OOP AHK good practise - validation, setters, throw exception Topic is solved

[SirRFI]

Recently I started rewriting some of my code to AHK v2, using Object Oriented Programming (OOP, as in classes, getters/setters etc.). While on it, I want to ensure that inserted values/content (the setter part) meets variable type (in example: integer only) and other custom conditions, so properties are validated.
I wanted to ask few questions regarding this approach, also as AHK good practises.



1. Should I check variable type & other conditions in setters?
Here's a sample method:

Code: Select all

setWidth(width)
{
	if !(width is "integer" && width > 0) ; checks if passed argument is an integer and is bigger than 0
		throw Exception("Argument is not integer or the value is smaller than 1.") ; if it's not - throw an exception
	this.width := width ; assign the value to object's property
	return true ; return... something. See next question.
}

This ensures the value will be always valid, of course as long this method is used.


2. What should I return in the setter?
Look at the sample above - what should I return?:
a) true, because the method succeed. Probably not, because if it wouldn't - exception is thrown anyway.
b) width (the argument)
c) this.width (the propery, after assigning new value)
d) this.getWidth() (use getter method)
e) this (class instance itself)
I think getter makes most sens, just in case there's extra logic in it.


3. Should I use setters in constructor?

Code: Select all

__New(width)
{
	this.setWidth(width)
}

Looks logical choice, assuming we are checking if the argument is proper in the setter.


4. Maybe instead of this all, I should use __Get and __Set? Pros and cons, why yes or not.
Looks like saving afford on making methods that can be omitted anyway, thus automatically answer two previous questions. The thing is, I would probabably need to if/else each propery within this method.



5. What should I do if the argument doesn't meet requirements? (Sticking to doing this in setter)
Sticking to the first code snippet - is throwing an exception there valid in first place? What is the best practise? Specifically, I want to use some kind of general purpose messages, like "Invalid argument type: excepted 'integer', received 'string'", without having to have them inline everywhere or a global variable.
As far I can tell, Exception() is basically a function, that returns associated array with predefined keys to fill the throw error.
I was thinking about creating a custom class for exceptions and call it by either throw new MyClass("stuff") or throw MyClass.__New("stuff"). Why custom class? I could use class static for the messasing mentioned above.



Thanks for your input.

[swagfag]

Also, while on OOP approach, wanted to ask if I should return the argument variable (which has been just assigned to this.var anyway), the this.var or use getter (this.getVar) in setter. Check the sample for clarification.

return the instance to do this:

Code: Select all

class Box {
	setColor(color) {
		this.color := color
		return this
	}

	setMaterial(material) {
		this.material := material
		return this
	}

	setWeight(weight) {
		this.weight := weight
		return this
	}

	printIt() {
		MsgBox, % "Color: " . this.color
		. "`r`n" . "Material: " . this.material
		. "`r`n" . "Weight: " . this.weight . " lbs"
	}
}

heavyRedWoodenBox := new Box()
				.setWeight(200)
				.setMaterial("Anodized Aluminum")
				.setColor("Red")
				.printIt()

more than a gimmick than anything really.

id argue in favor of command/query separation and have the setters not return anything

[SirRFI]

Thanks for answer. Meanwhile I actually went deeper into the topic and decided to rewrite it whole.

Returning the class instance itself makes sense, because it allows to keep working on a one-liner.

[kczx3]

I’m also interested in the answers to your questions. I will comment that it is very common to return the instance to allow for method chaining.

[SirRFI]

Bringing this up for visibility.

[swagfag]

4. Maybe instead of this all, I should use __Get and __Set? Pros and cons, why yes or not.
Looks like saving afford on making methods that can be omitted anyway, thus automatically answer two previous questions. The thing is, I would probabably need to if/else each propery within this method.

properties can define their own get/setters, so no need to mess with meta functions

5. What should I do if the argument doesn't meet requirements? (Sticking to doing this in setter)
Sticking to the first code snippet - is throwing an exception there valid in first place? What is the best practise? Specifically, I want to use some kind of general purpose messages, like "Invalid argument type: excepted 'integer', received 'string'", without having to have them inline everywhere or a global variable.
As far I can tell, Exception() is basically a function, that returns associated array with predefined keys to fill the throw error.
I was thinking about creating a custom class for exceptions and call it by either throw new MyClass("stuff") or throw MyClass.__New("stuff"). Why custom class? I could use class static for the messasing mentioned above.

im not sure i understand this question entirely

[nnnik]

1. it helps finding bugs and keeps the uses of your code clean - I would reccomend doing it, but Im rather lazy myself

2. that is different for every programmer - whats important is that you stick with it. So far I have always returned width however I do not really consider at an advantage

3. yes - however there are rare exceptions

4. meta-functions cause more problems than they are worth therefore I wouldn't reccomend using them unless you really need to.
properties provide a lightweight alternative for meta functions however they are limited on several aspects - thats why I prefer getters and setters

5. Every class I wrote so far handled this differently. I do not know if there are advantages to either one or the other. Just experiment with it

[SirRFI]

properties can define their own get/setters, so no need to mess with meta functions

Can you elaborate?

5. What should I do if the argument doesn't meet requirements? (Sticking to doing this in setter)
Sticking to the first code snippet - is throwing an exception there valid in first place? What is the best practise? Specifically, I want to use some kind of general purpose messages, like "Invalid argument type: excepted 'integer', received 'string'", without having to have them inline everywhere or a global variable.
As far I can tell, Exception() is basically a function, that returns associated array with predefined keys to fill the throw error.
I was thinking about creating a custom class for exceptions and call it by either throw new MyClass("stuff") or throw MyClass.__New("stuff"). Why custom class? I could use class static for the messasing mentioned above.

im not sure i understand this question entirely[/quote]
Say I have class Triangle, where the properties are all the sides (a, b, c). Now, to ensure the class is used properly - I want to enforce the user (as in whoever uses the class in their code) to use only positive integers when setting each of the triangle's side. I see no better solution than checking just that in the property's setter method (see question #1).

Now, the question is: what should I do if the passed value isn't positive integer, but a string or something. Returning false or nothing doesn't notify anyone about the problem and allows the code to continue, likely causing another problem soon. It seems that throwing an exception does the trick, as it literally pops up an error and disables the code flow to go on.

Later I share my thoughts on what Exception() in AHK seems to be for me, and how to possibly expand it with things such as reuseable error message, rather than having the same string in every throw Exception() of same kind.

4. meta-functions cause more problems than they are worth therefore I wouldn't reccomend using them unless you really need to.

Aren't they called everytime this.width is used? As said, it would fix the "vulnerability" of ignoring getter/setter method and refer to the properly directly. What are problems that may come out of this, assuming I only want to validate input data before saving it.

properties provide a lightweight alternative for meta functions however they are limited on several aspects - thats why I prefer getters and setters

To my understanding, properties are variables within the class' instance - more precisely: these with this. prefix. Please correct me if I am wrong. Now, what "lightweight alternative" do they offer? Can't ask about limitations without knowing just this yet.



Thanks everyone for input. I'd like to see more, keep it coming.

[nnnik]

Using meta functions sounds easy in theory. In practice you will end up in the so called 'callback hell'.
Many bugs many issues and its just no fun debugging them.

properties are a specific syntax construct which allows you to define the __Get and __Set meta functions for a specific key:

Code: Select all

class propertyContainer {
	property[] {
		get {
			Msgbox tried to get property
			return whatever
		}
		set {
			Msgbox tried to set property to %value%
			return whatever
		}
	}
}

[lexikos]

[Edit: Somehow I overlooked the fact that SirRFI is not using properties.]
The return value of set should be either the value which was assigned or the new value of the property.

The return value of get or set becomes the result of the sub-expression which invoked the property. For example, val := obj.Property := 42 stores the return value of set in val.

The result of the sub-expression y := 1 is the variable y, so there is an expectation that the result of obj.Property := 42 will effectively be obj.Property. This allows assignments to be chained - i.e. assigning one value to multiple properties or variables, right to left, without having to repeat the value. Sometimes the value being assigned is not the same as the value the property's getter would return afterward. For example:

Code: Select all

MsgBox FileOpen("nul", "r").Encoding := "CP65001"  ; Shows "UTF-8"

Sometimes I take a shortcut and return the assigned value to avoid the cost of executing the getter (I can't think of an example right now). This is just as useful for chained assignments and might actually be less surprising in some cases.


I would generally consider passing invalid input to a function or property as a defect in the script. When AutoHotkey v2 detects this for the built-in functions, it throws an exception like "Invalid parameter #1". For properties and built-in variables, it throws "Invalid value." So I would say it is appropriate to do the same in scripts.

[SirRFI]

Apparently the topic is deeper/wider than I anticipated. I've read up on the getter/setters in PHP, but the discussions in StackOverflow quickly turned to cross-language. To summarize what was said:
• Creating getter/setter methods (such as setWidth()) came from Java and C++ due to their structure.
• Many languages have this implemented via "magic methods" (which are said to be "evil", including in this very thread) and properties.
• Properties are faster than methods, because methods are basically additional layer, which in the end use (by language design) property anyway. However, this does not mean that methods have any practical performance drop.
• Methods add up more code, so files and memory usage is bigger. Let alone they have to be implemented first.
• Methods help code maintenance/upkeeping. As an example, it's easier to find get and set usages, rather than searching by variable name, which returns both and more.
• Methods help encapsulation (protecting object's content from outside world).
• People have various preferences and opinions on this topic, but seemingly the methods one is favored due to benefits it gives the programmer.

I may have missed something, but that's about it.

These points seems valid for AHK as well. However, since AHK doesn't seem to have private variables/fields/whatever, one can ignore instance.setWidth() and assign to instance.width directly, which in terms of validation makes the setter method useless. That said, if the class designer wanted to offer methods approach (although can't enforce it, it seems), they would have to make the validation in propery setter, which is somewhat duplicating the code.

Please note that anytime I say "methods" in this post - I meant getter/setter methods only.



I inserted invalid data to my class on purpose, so exception is thrown. The error appears, but seemingly acts as just an error, doesn't terminate the script. Is it intended? The usual errors either terminate or load previous version of the script, if it was reloaded with newer.

[lexikos]

The error appears, but seemingly acts as just an error, doesn't terminate the script. Is it intended? The usual errors either terminate or load previous version of the script, if it was reloaded with newer.

"The usual errors" do not terminate the script. If they can be detected before the script starts, syntax errors and (mostly just in v1) invalid parameters will prevent the script from starting. If it never starts, it can't terminate.

During loading, the script is optimized and validated. Any syntax errors will be displayed, and they must be corrected before the script can run.
Source: AutoHotkey Scripts and Macros

Errors which are detected after the script starts, i.e. "at runtime", generally do not terminate the script. This is why the error message includes the text "The current thread will exit" or "The thread has exited". If that thread is the last one and the script is not persistent, the script will exit (just the same as if the thread completed normally or exited without error).

If you are reloading a script, the new instance always terminates the old instance before executing. If the script is able to throw, it was obviously able to be (re)loaded. Even if the old instance waited a short time to see if the new instance will throw an exception, it must stop waiting some time, and that could be moments before the new instance throws an exception. If the old instance did not terminate before the new instance starts, any hotkeys it has registered would fail to be registered by the new instance (and would therefore require the keyboard hook). Similar issues might occur depending on the script, such as if it refers to its own windows by title.

If the script cannot be reloaded -- perhaps because it has a syntax error -- the original instance of the script will continue running.
Source: Reload

It does not "load previous version" under any circumstance. If the new instance is unable to start, it exits and the old instance, which is still running, stops waiting and continues execution.

[SirRFI]

Setters: I am trying mentioned approaches, but there are problems as well.

Code: Select all

Class Testt
{
    width[]
    {
        get
        {
            return this.width
        }
        set
        {
            if !(value is "integer")
                throw Exception(A_ThisFunc ": Expected integer, got " Type(value))
            this.width := value
			return this ; or return this.width := value
        }
    }

    __New(width)
    {
        this.width := width
    }
}

asd := new Testt(123)
MsgBox(asd.width)

Crashes silently. This is caused by return this.width := value in the set property, leading to infinite recursion.
When asked about this on discord, I was told to do following (replace this.width to this._width in get & set):

Code: Select all

Class Testt
{
    width[]
    {
        get
        {
            return this._width
        }
        set
        {
            if !(value is "integer")
                throw Exception(A_ThisFunc ": Expected integer, got " Type(value))
            this._width := value
			return this ; or return this._width := value
        }
    }

    __New(width)
    {
        this.width := width
    }
}

asd := new Testt(123)
MsgBox(asd.width) ; 123

Sure it works, but it creates another variable called _width, which also can be accessed directly:

Code: Select all

; ...
asd := new Testt(123)
asd._width := 111
MsgBox(asd.width) ; 111

When pointed that out, I was given magic method counterpart, but it resulted in the same thing, just with an addition of extra ifs.

Spoiler

Code: Select all

Class Testt
{
    __get(key)
    {
        if (key = "width")
            return this.width
    }

    __set(key, value)
    {
        if (key = "width")
		{
            if !(value is "integer")
                throw Exception(A_ThisFunc " is wrong type - expected integer, got " Type(value))
            return this.width := value
        }
    }

    __New(width)
    {
        this.width := width
    }
}

asd := new Testt(333)
MsgBox(asd.width) ; 333

Code: Select all

Class Testt
{
    __get(key)
    {
        if (key = "width")
            return this._width
    }

    __set(key, value)
    {
        if (key = "width")
		{
            if !(value is "integer")
                throw Exception(A_ThisFunc " is wrong type - expected integer, got " Type(value))
            return this._width := value
        }
    }

    __New(width)
    {
        this.width := width
    }
}

asd := new Testt(333)
asd._width := 111
MsgBox(asd.width) ; 111

In the end, these are no different than enforcing to use setWidth() method (as setter), since width can be accessed directly either way. Guess what I am trying to achieve is simply not doable (at least without too much afford) without private variable scope. Is it right? If so, I'll stick to method getters/setters.

[SirRFI]

Exceptions: @lexikos I see. In that case I suppose throwing an exception to inform what and where is wrong should be enough anyway.

Okay, how should I connect validation and exceptions for best result? Assuming this is the right way, of course.

Code: Select all

Class Test1
{
    __New(width)
    {
        this.setWidth(width)
    }

	getWidth()
	{
		return this.width
	}

	setWidth(width)
	{
		if !(width is "integer")
			throw Exception("Expected integer, got " Type(width))
		if !(width >= 0 && width <= 100)
			throw Exception("Expected value between 0 and 100, got " width)
		this.width := width
	}
}

asd := new Test1(9000)

The above shows exact place where error was thrown, which is good. However, if I added height / setHeight following similar rules (such as checking if it's an integer), I'd have this string/expression in Exception() repeated - not cool. Let alone I have to retype "integer" in the string to match the if condition, leaving an opportunity to forget adjusting it, therefore having a misguiding error message.

Code: Select all

Class Test2
{
    __New(width)
    {
        this.setWidth(width)
    }

	getWidth()
	{
		return this.width
	}

	setWidth(width)
	{
		if (ValidateInteger(width) && ValidateRange(width, 0, 100))
			this.width := width
	}
}

ValidateInteger(value)
{
	if !(value is "integer")
	{
		throw Exception("Expected integer, got " Type(value))
		return false ; just in case?
	}
	return true
}

ValidateRange(value, min, max)
{
	; if value, min and max are numbers (int/float) ...
	if (min > max)
	{
		throw Exception("Range minimum value (" min ") cannot be bigger than maximum (" max ") !")
		return false ; just in case?
	}
	if (min >= value || max <= value)
	{
		throw Exception("Expected value between " min " and " max ", got " value)
		return false ; just in case?
	}
	return true
}

asd := new Test2(9001)

This on other hand points function's code to have an error, because the throw is there. As result, I could have multiple variables or/and objects, and I won't really know which one caused it.

Code: Select all

Class Test3
{
    __New(width)
    {
        this.setWidth(width)
    }

	getWidth()
	{
		return this.width
	}

	setWidth(width)
	{
		if !(width is "integer")
			throw TypeException(width, "integer")
		if !(width >= 0 && width <= 100)
			throw RangeException(width, 0, 100)
		this.width := width
	}
}

TypeException(provided, expected)
{
	; Exception("Expected " expected ", got " Type(provided)) ; ← causes "Unhandled exception" error
	; return Exception("Expected " expected ", got " Type(provided)) ; ← points to the function instead
	return {"message": "Expected " expected ", got " Type(provided)} ; ← works fine
}

RangeException(provided, expected_min, expected_max)
{
	; Exception("Expected value between " expected_min " and " expected_max ", got " provided) ; ← causes "Unhandled exception" error
	; return Exception("Expected value between " expected_min " and " expected_max ", got " provided) ; ← points to the function instead
	return {"message": "Expected value between " expected_min " and " expected_max ", got " provided} ; ← works fine
}


asd := new Test3(9000)

This is a hybrid of Test1 and Test2, but all it solves from Test1 is having the text in one place.


How to do it best?

[swagfag]

Setters: I am trying mentioned approaches, but there are problems as well.

its not documented very clearly imo https://autohotkey.com/boards/viewtopic ... 14#p213514

[kczx3]

EDIT: Ignore below. This would only be useful with test3.setWidth(100).setHeight(200) and not the property syntax.

I don't have an extensive history with OOP but I have always seen where a setter returns the instance, not the value which was set.

[lexikos]

"Property definitions allow a method to be executed whenever the script gets or sets a specific key."
"For example, [...] obj.Property := value would call set."

Did you have reason to believe that this.width := value would behave differently depending on whether it is inside or outside the property setter?

If you store a value against the key "width" (e.g. with ObjRawSet), the property "width" will no longer be called.

The workaround, using "_width" to store the value, does not create another variable, because "width" is not a variable in the first place. It has no value and is not stored in the object. If you want a value to be accessible, it will be accessible. You can't use it if you can't access it.

If it concerns you that a user might refer to this._width directly, either tell them not to, or use something even less obvious/convenient. You can use a unique object as a key (static width_key := {} or class width_key for use with this[width_key]), but this may make debugging more difficult, and the object needs to be accessible to all of your methods. Storing private values in a separate global/static array has similar drawbacks, but also requires manually removing those values from the array when the object is deleted, and prevents the standard enumerator from finding the key-value pair (probably a good thing).

You can use closures in v2 to hide variables, but you can't use standard class syntax. Unless the hidden variables are intended to be shared, each new instance of the object must be assigned a closure for each method. Each "method" must be implemented as a nested function.

Spoiler

Code: Select all

t := Testt()
t.setWidth(10)
MsgBox t.getWidth()

Testt() {
    width := 0
    setWidth(this, value) {
        return width := value
    }
    getWidth(this) {
        return width
    }
    return {
        setWidth: Func("setWidth"),
        getWidth: Func("getWidth"),
    }
}

or

Code: Select all

t := new Testt()
t.setWidth(10)
MsgBox t.getWidth()

class Testt {
    __new() {
        width := 0
        setWidth(this, value) {
            return width := value
        }
        getWidth(this) {
            return width
        }
        this.setWidth := Func("setWidth")
        this.getWidth := Func("getWidth")
    }
}

or for trivial cases

Code: Select all

t := new Testt()
t.setWidth(10)
MsgBox t.getWidth()

class Testt {
    __new() {
        width := 0
        this.setWidth := (this, value) => width := value
        this.getWidth := (this) => width
    }
}

To replace the method calls with property syntax, currently you must use meta-functions.

Implementing access specifiers such as private would increase the run-time cost of objects, since everything you do with an object is dynamic. Whenever you attempt to access a private member the object must determine whether the caller is allowed to access it.

Exceptions:
If you pass -1 for the second parameter of Exception, the error message will show the line which called the function. If you pass -2, it will show another step further up the call stack.

[lexikos]

An alternative to real access specifiers is to use two objects: the real object this, and the public object that outsiders use. The following script for v2 demonstrates the concept:

Code: Select all

class MyClass extends PrivateObject {
    myvar := 0
    getVar() {
        return this.myvar
    }
    setVar(value) {
        if (value & 1)
            throw Exception("Odd value", -1, value)
        return this.myvar := value
    }
}

obj := new MyClass()
MsgBox obj.getVar()  ; 0
obj.setVar(42)
MsgBox obj.getVar()  ; 42
MsgBox obj.myvar  ; Error

; ====================================================

class PrivateObject {
    __new() {
        return PublicHandle(this)
    }
}

PublicHandle(this) {
    return {(PrivateObject): this, base: PublicHandleBase(this.base)}
}

class PublicHandle {
    __get(m:="") {
        if m != "base"
            throw Exception("Unknown property", -1, m)
    }
    __set(m:="") {
        throw Exception("Unknown property", -1, m)
    }
    __call(m:="") {
        if !ObjHasKey(this.base, m)
            throw Exception("Unknown method", -1, m)
    }
}

PublicHandleBase(cls) {
    static pbs := {}
    if pb := pbs[cls]
        return pb
    if cls = PrivateObject
        return PublicHandle
    pb := ObjClone(PublicHandleBase(cls.base))
    enm := ObjNewEnum(cls)
    while enm.Next(k, v) {
        if type(v) = "Func" && (SubStr(k,1,1) != "_" || k = "_NewEnum") && !pb.HasKey(k)
            ObjRawSet(pb, k, ((f, t, p*) => f.call(t[PrivateObject], p*)).Bind(v))
    }
    pb.base := cls  ; For type identity.
    pbs[cls] := pb  ; One per class.
    return pb
}

This proof of concept has several limitations:

• Properties aren't implemented, only methods.
• All instance variables are assumed to be private.
• You must remember not to pass this to external code, but instead pass a public handle for this object.
• This object's public handle isn't stored anywhere, so it can't be reused from inside the class. Storing it requires additional complication to avoid a circular reference.
• Probably others.