On the abuse of properties


One thing that I see occasionally is the abuse of property syntax. IDispatch and CLR objects (and C++ objects if you want to avail yourself of a Microsoft-specific extension) support "properties", which syntactically look like fields but internally are treated as a pair of methods ("get" and "put"). An important principle is that given an object o and a property p, the lines

(void)o.p;
o.p = o.p;

should be effectively nops. (Mind you, they might be really inefficient nops.)

My favorite (or perhaps most hated) example of violating this principle is an object I saw many years ago that had a "print" property, which if set to true, caused the object to send itself to the printer. If you did

o.print = true;
o.print = true;

then two copies of the object were printed.

Property syntax mimics field syntax for a reason: Properties should behave like fields.

(Plenty more guidance on properties can be found in the .NET Framework Property Usage Guidelines.)

[While Raymond was on vacation, the autopilot stopped working due to a power outage. This entry has been backdated.]

Comments (15)
  1. Gabe says:

    One interesting fallacy that I have seen is that many people expect that if you perform o.p = x, then o.p == x should be true.

    This is false because one of the reasons to use a property versus a regular public field is to prevent out-of-bounds or to normalize values. For example, you might have an Angle property that is always between 0 and 360. If you set it to 540, you’ll get 180. Another example is setting a window’s position. If you set it to some location off-screen, in some cases the correct thing to do is move it so that it’s still visible.

  2. Michael Golubev says:

    As for me, side effects of setting a property should not be considered as problem. In fact, it is probably common to expect that setting a new value for property may fire broadcasting a change event to listeners and in turn force a lot of things happens. I do not see any differences between "send itself to a printer" and "update property inspector component to show new value" side-effects.

    I think that main reason, why this o.print syntax should be avoided is not its side-effects, but the fact that it has only one valid value. If we would have some abstraction which accepts both o.print = true, and o.print = false, and if the meaning of this call would be clear for reasonable maintainer, then "print" should be considered as normal write-only property.

    (Sorry for my English)

  3. Jim Arnold says:

    "As for me, side effects of setting a property should not be considered as problem"

    The problem is not the side-effects per se, rather the API design. If you want to provide the ability to print an object, then make it a method and call it print()!

    Having said that, I dislike properties in general. The main rationale for employing them is, traditionally, that if you need to introduce some behaviour around field access in the future you won’t have a versioning problem. I have never, ever seen a real example of this.

    In addition, many developers are under the impression that they are "encapsulating" their objects’ fields by simply wrapping them in properties. Poppycock!

    Jim

  4. Mike says:

    Isn’t that just a remnant of the old Visual Basic VBX interface that only provided support for properties? Such magic properties were necessary to provide method-like semantics.

    I certainly remember using lots of VBXs that had such properties.

  5. tzagotta says:

    Jim, I program in C#, and use properties all day long. We have a couple of cases that come up often that will illustrate why properties are important to us.

    1. One object fires an event to notify other objects of a changing value. Sure, each caller could do this after setting a field value, but it is a much cleaner design to have the object itself do the work.

    2. A document object sets its internal "changed" or "dirty" flag when one of its properties is changed. Again, could be done externally, but is a lot cleaner to have the object take care of itself.

    I’m a very practical programmer, and I’ve adopted the idea of NEVER exposing public fields. Properties are cheap and easy, and provide some nice benefits.

  6. Derek says:

    Jim, you seem to be assuming that the only things exposed via properties are fields. While that’s probably the most common case, it’s not the only one. A property might retrieve a value from a database, or calculate a value from a number of fields, or request a value from an internal state object, etc. There are plenty of examples of when a property might appear to simply return a field, when in actuality, it’s doing something more complex. I’ve seen it a number of times. (I’m especially fond of the internal state object.)

    Properties allow all accessors to have a consistent interface, without resorting to a bunch of gets and sets. (It accomplishes the same thing, but with a little less hassle, and a little less typing.)

  7. James Curran says:

    Now I’m wondering about that object with the misguided "print" property. What happens if you set it to false?

    o.print = false;

    Does it check the value for "true" or does any assigment cause it to print?

  8. J says:

    The statement "o.print = false" causes the printer to take back the document it just printed and erase the ink off of it, producing the original clean sheet of paper.

  9. Mike Swaim says:

    "Having said that, I dislike properties in general. The main rationale for employing them is, traditionally, that if you need to introduce some behaviour around field access in the future you won’t have a versioning problem. I have never, ever seen a real example of this."

    For me, one of the main reasons for exposing properties is the side effects. Stuff like

    buttonx.visible = false

    Sure, you could use set_visible() and get_visible(), but it looks ugly, and if you’re worried about your object’s state, you need to use set_ and get_ for just about every variable in your class, which gets annoying really quickly.

  10. David says:

    When we wrote VBX’s for VB3, we had to use the

    o.SendToPrinter = True

    because the VBX model didn’t support the creation of custom methods, only properties and events. This probably qualifies for "many years ago".. :-)

  11. Jonathan says:

    Obviously,

    o.print(true) sends a copy to the printer

    o.print(false) doesn’t send a copy to the printer!

    Much like the guy which sleep with a glass of water and an empty glass on the nightstand, because when he wakes up, sometimes he’s thirsty and sometimes he’s not.

  12. DavidE says:

    I think that the VBX guys are probably right. I remember somewhere in the Utopia (BOB) toolkit there were a couple of controls where you would set multiple properties as arguments and then set a property to True to trigger the code. Man, that was nasty!

  13. Cheong says:

    DavidE,

    I’ve imagined what the call should look like, and think that it’d seems to be like calling a function in assembly language – a lot of push to "push" the parameters to stack, then a trigger "call" to do the actual thing.

    Funny. XD

  14. Craig Ringer says:

    Gabe:

    I’d expect

    o.x = 1;

    assert(o.x == 1);

    to always succeed <i>or throw a suitable exception</i>. If it should not, then I’d expect to see something like:

    bool valid;

    o.setX(1, valid);

    (where `valid’ is a bool& argument, or take a bool* and use &valid)

    or

    bool valid = o.setx(1);

    instead. The caller can always ignore the optional out parameter / discard the return if they don’t care. If you want to force them to care, throw an exception.

    Of course, I can’t actually imagine working in plain-old-C…

  15. Gabe says:

    For the record, I consider Craig to be definitively wrong. One should never assume that

    o.x = 1;

    assert(o.x == 1);

    will always succeed or throw. In the example,

    o.Angle = 540;

    assert(o.Angle == 540);

    540 is not an invalid angle, so the assignment shouldn’t throw an exception. Likewise,

    bool valid = o.setAngle(540);

    wouldn’t return false just because o.Angle == 180. Are you suggesting that I should be programming like this:

    int actualAngle = o.setAngle(540);

    so that I can find out that Angle really gets set to 180? That’s pointless because I could always just write this:

    o.Angle = 540;

    int actualAngle = o.Angle;

    if I really cared that Angle might not have the exact value I put in it.

    Also, consider that in most cases you want the value you set to be coerced to a correct value instead of having an exception thrown. If you try to scroll past the bottom of a window, you want it to just stop scrolling, not beep at you or put up an error message box. Similarly, if you call o.ScrollPos = 101 and the valid value are 0 to 100, you want ScrollPos silently set to 100.

Comments are closed.

Skip to main content