• some guy (unregistered)

    I didn't even know that an assignment returned a value in TS. And it shouldn't.

  • (nodebb)

    Now, TypeScript, being a Microsoft language, allows properties to be just, well, properties, or it allows them to be functions with getters and setters.

    The (mis)use of vocabulary here is ... disappointing. See, I thought that what is visually a member variable access (x.somevar) could actually be just that, a member variable access, or it could be an invocation of a property, which generates a call to the relevant get or set function of the property. Unless it's some specific weirdness of TypeScript's vocabulary.

  • Rob (unregistered)

    If the function is really called as if (!this.checkAndPick) then the body will never be invoked. this.checkAndPick is a function, so it can only be falsy if the function isn't defined. If it is defined, the function itself is truthy. To get a false value out of it, it needs to be invoked: if (!this.checkAndPick()).

  • (nodebb)

    It's bonkers that a function marked "get" is even allowed to modify any state. Shouldn't your object be const from that context? Or does TypeScript not actually enforce proper type safety?

  • (nodebb) in reply to Rob

    I think the get keyword changes that; the body is actually invoked when doing this.checkAndPick.

  • (author) in reply to Rob

    You're touching upon the confusion point of properties- when you add get and set keywords into function definitions like this, you're telling TypeScript to evaluate the function when you access it like a property. The emitted JavaScript will invoke the function- this.checkAndPick gets transpiled to this.get_checkAndPick() (with some name mangling in there, but this is the core idea).

  • Rob (unregistered) in reply to Remy Porter

    @Mr. TA, @Remy: I missed the get part in the method names.

  • (nodebb) in reply to Remy Porter

    Getters and setters in classes have been a JavaScript functionality for long (ES6, 2015: https://262.ecma-international.org/6.0/index.html#sec-method-definitions). Given that Object.defineProperty has been there for even longer (ES5, 2009), I would expect that TypeScript never actually transpiled that down to a function call.

  • (nodebb)

    I love when developers write code to force test results to be favorable while ignoring the required functions of the business process. I guess we can blame it on 'return code.vendor || microSoft.ToString()'

  • Tinkle (unregistered)

    The code in that function is not a huge WTF. It sets _hasPicked to true and returns the old value. Having it as a property getter is the WTF.

    The way it is implemented is a little bit like code golf, but some decent comments and a decent function name would solve that issue.

  • (nodebb) in reply to Tinkle

    My thoughts exactly. What the function does is a classic test-and-set (without the thread-safety) rather than innately a WTF... But how it's done is the WTF (both the code-golfy implementation and the putting it in property getter).

  • MaxiTB (unregistered)

    It is actually fine if getters mutate state as long as it doesnt have side effects. That is literally what getter/setters are for otherwise you could use fields in the first place.

    So the correct statement would be: Never ABUSE properties by changing a state causing side effects.

  • MaxiTB (unregistered) in reply to some guy

    Ever correct implemented C-like language allows operator chaining, it is after all a core language concept. There's nothing wrong with having functional features in a language too IMO.

  • (nodebb) in reply to MaxiTB

    Here a simple example for how to make a property which changes state but has no side effects, for those wondering:

    private get hasPicked() {
	++this._pickCount;
    return this._hasPicked;
}

private get getPickCount() {
    return this._pickCount;
}

    The second getter is just there as a contractual enforcer of keeping the state protected ironically :-)

  • (nodebb) in reply to MaxiTB

    I would strongly question why you would ever care how many times a getter has been called (other than debugging or profiling)

  • NoLand (unregistered)

    Now, I expect there to be much more serious problems with this code base.

    I actually think, the last change was faithful to the original intention, as in, "every piece of code that checks this property in order to handle that object will have to set it to 'true', so do it just in place." It achieves this by returning the current state and also forcing the property value to true. In the previous iteration, however, where the getter always returned true, any such checks, like in the provided example (if (!this.checkAndPick) …), would fail and the code meant to handle the case would never be executed. Effectively, the body guarded by any such condition became dead code. How did this even continue to do what it was meant to do?

  • (nodebb) in reply to MaxiTB

    Here a simple example for how to make a property which changes state but has no side effects,

    Nah. The getter clearly has a side effect: it increases _pickCount by 1. Changing state is a side effect. I don't know how you can seriously argue otherwise.

    Furthermore, your example appears to me to be particularly unhelpful. If a compiler is allowed to assume that getting a property does not have side effects (including changing state) then it can make a number of optimisations. For example:

    x = object.hasPicked;
y = object.hasPicked;

    There's an obvious compiler optimisation there, if you can assume the person who wrote the getter didn't change anything, you can optimise out one call of the getter.

    And never mind the actual compiler why should developers have to be aware of what the implementation of the getter is?

    These should be functionally equivalent

    objectHasPicked = foo.hasPicked;
for something in someCollection 
{
    // do stuff not involving foo
    if (objectHasPicked)
    {
        // do other stuff  not involving foo
    }
}

    and

    for something in someCollection 
{
    // do stuff not involving foo
    if (foo.hasPicked)
    {
        // do other stuff not involving foo
    }
}

    Look at the above two fragments and ask yourself what your counter means. It means nothing.

  • (nodebb) in reply to jeremypnet

    Nah. The getter clearly has a side effect: it increases _pickCount by 1. Changing state is a side effect. I don't know how you can seriously argue otherwise.

    It's not in context of _hasPicked. It's classical state composition, you have to break it by cross referencing the state which would make it having side effects. If you have two separate objects with their own states or one object with two separate states doesn't matter.

    There's an obvious compiler optimisation there, if you can assume the person who wrote the getter didn't change anything, you can optimise out one call of the getter.

    Keep in mind, we talk JavaScript here and pretty much all JavaScript vendor versions are interpreters and only few have PGOs in limited fashion. So there is no inlining and without inlining the interpreter never knows if the function only acts as a pure getter. From a performance point of view the function call itself is 10-50x slower than the implementation, so there's no noticeable performance impact as well (which is the normal cost of a function call compared to an atomic operation depending again on vendor version).

    And never mind the actual compiler why should developers have to be aware of what the implementation of the getter is?

    If you have your getter doesn't have side-effects, it doesn't matter what is implemented. The hasPicked getter works the same as without incrementing the count. In your example, YOU as developer chose to retrieve the property once, so the code works still as expected, in fact, as state by other readers if the intended purpose of _pickCount is telemetry you gave a prime example of one of it's use case :-)

  • lzsiga (unregistered)

    In the final version the 'getter' is actually a setter, which returns the old value. It could have a pair called 'checkAndUnpick' (for setting false value). Or there could be a proper setter with a 'newValue' parameter.

  • Loren Pechtel (unregistered)

    A scenario:

    Original: hasPicked is a property, doing a reasonably expensive search for picked items.

    Update: Perhaps the author of hasPicked wrote a setter to discard the assignment, or perhaps it continues to return true on subsequent calls. Bug--but that could be hidden by code that would never call it again unless it was true (part of a loop conditional.)

    Second update: Someone had a case where it was called again after returning false, and got the corrupted true.

Leave a comment on “Property Flippers”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article