Careful with those member variables, Eugene

OOP (Object oriented programming,) or just OO, is a wonderful way of programming. I’m quite happy to say that since I’ll be honest and say I didn’t really get it properly until 2 or 3 years ago. Sure, I’ve been programming with Java and C# since 1998, but that doesn’t really mean I was doing OO all that time. A lot of people use Java or C# to write ‘object procedural’ or ‘class oriented’ programming. If you understand OO, you probably know already what I’m talking about, and if you don’t…. well I’ll try and explain.

There are a few pointers that code has not been written in the spirit of OO, and I’m going to talk about just one such small smell here. This smell is what I like to call ‘member variable madness’.

OO is to a large extent about encapsulating state. The state in OO is held in the member fields of objects. Those fields may be encapsulated data, or references to other objects. Member fields are used so that the object can keep track of data, or references, between calls to its public methods. Member fields consist of member constants (readonly in C# and final in Java) and member variables (i.e. not a constant.) Member constants are used when a piece of member data is immutable, or when a reference is a constructor-initialised dependency on another object.

Member variables are not meant to be a temporary scratchpad between public and private methods. This is not state that exists between externally stimulated method calls. What’s bad about using member variables as an internal scratchpad? I think there are at least a couple of reasons:

  • Clarity. When I look at a class for the first time, I look at its member fields. This tells me what the object is encapsulating as part of the whole program. If there are a bunch of scratchpad variables here that confuses this concept.
  • Correctness. Everytime you introduce a new member variable to a class, you are introducing another ‘degree of freedom’ of all the objects of that class. This means to be sure you haven’t broken anything you have more testing to do. Correctness can be broken in at least 2 ways:
    • Thread safety / interference. If you have 2 concurrent threads accessing one object, they will share the member variables of that object. There is a big chance they could interfere with each other leading to hard-to-find bugs
    • Initial value. If you use local variables, you know you need to initialise it every time you define it (otherwise the compiler will complain). However, with a member variable you need to only initialise it once, somewhere in the class. How do other usages know it has been initialised? Do they need to re-initialise? Might that actually break the required behaviour?

Thankfully, this smell is an easy one to fix. Simply replace the member variable with a local variable in the scope where it is initialised, then pass the variable as an argument to any other methods that require it. You may find that you now have methods with long parameter lists, but this may suggest that you need to introduce new classes to encapsulate such groups of variables anyway.

If you’re interested more in this area I recommend Ivan Moore’s ‘Programming in the Small’ series on his blog . Pair-programming with Ivan taught me more about OO programming than any other time in my career.

Advertisements