Let me preface the discussion by saying that I understand that there is a good reason the globals exist (they go way back).
The following happened on Monday and illustrates one of the problems with globals:
I was making last minute changes to a product. I was setting an Rng variable in the product, but by the time the output code ran, that boolean always had a reverse value. I searched the code for all places that changed that global, but didn't find any that I wasn't already aware of. To temporarily solve the problem I implemented a hack.
Cindy later explained to me why the global seemed to be changing, but no matter; the root problem is globals themselves.
I'm sure you're all aware of all or part of the following, but I wanted to put together a good list of reasons why globals are bad with few exceptions:
- They make the code less modular; in fact they make it anti-modular - everything is tied to the globals.
- When globals are used, it's exponentially more difficult to determine what to change. When all variables are in their own (smaller) object, you can find what needs to change much more quickly.
- Using globals leads to duplicated and unused variables (of which Cindy and I have found several just since I've been here).
- Better memory footprint: more objects, but fewer in memory at any given time.
- Much easier debugging. Even when you have a simple "dummy" member variable with an accessor at least you can set a breakpoint on the accessor and easily track down all callers.
- This from a portion of the coding standards that Rob referenced in a previous post (and another reason to always have accessors even if they're just dummy accessors): "If you avoid exposing fields directly to the developer, classes can be versioned more easily because a field cannot be changed to a property while maintaining binary compatibility. Consider providing get and set property accessors for fields instead of making them public. The presence of executable code in get and set property accessors allows later improvements, such as creation of an object on demand, upon usage of the property, or upon a property change notification."
- They make test driven development (even though we may never actually adopt this) virtually impossible because you can't test objects individually; they all need globals that are manipulated by other parts of the main app.
- For more, see Code Complete | Chapter 13 | 13.3: Global Data.
In the specific case outlined above, I should have added a member variable to the product class and accessed it from the output code (even though it's a little funky to access the product object at that point).
4 comments:
I'm adding this comment from Cindy (with permission) because this started as an email discussion:
The person I most want feedback from is Steve. I view the Rng/Inp/Outp variables as being part of the skeleton of the program. He's the one who originally set it up the way it is (way before object-oriented programming) and there are benefits to having some consistency throughout the program. For instance, when he converted Rng into an object recently, we ran into problems with some of the annuity code because it did access the product outside of the calculations. If we're going to move away from that design on the annuity side, he should be aware of it. I do agree with you however. If we were to build the app today using object-oriented design principles, the product attributes would only be accessible via the product object.
I agree a product should set its own member fields and be self contained. I think we should get rid of Rng altogether. The Rng variables are currently used for basically 2 purposes: (1) for properties related to the product itself and (2) for properties related to the inputs. I believe the ones related to the product itself should become true properties of the product and that the ones related to inputs should be properties of various input classes. Many of the Rng variables now share double duty and should be broken up.
Again, in a couple of weeks Dennis and I will discuss plans for the input and rng variables related to the input. Rng variables only needed as properties of the product could be moved at any time. Keep in mind, however, that there may currently be some code in the GUI that refers to a Rng variable that might generate an access error if you made it a property of a product and no product was selected.
Bilbo wrote:
The Rng/Inp/Outp variables are not global to the program but are passed en masse between objects.
I might be misunderstanding your point, but a collection of globals referenced by everyone and individual globals are 2 sides of the same coin.
We have a hierarchy of products (good) using collections of globals to manage their state in addition to the state of other objects(bad).
A product should manage it's own state, so instead of setting Rng.Whatever, it should set it's own member field and be self contained.
Bilbo wrote:
I agree a product should set its own member fields and be self contained. I think we should get rid of Rng altogether.
Great - so now give me direction: I want to remove the variable that started this discussion from Rng. It turns out it is only used by annuity products and in involves guaranteed rates.
Thus, I want to create an object that deals with interest rates (what is the guaranteed rate if applicable, get current rate, etc...) and have it owned by the base annuity object. It would take over this and no doubt several other Rng vars.
Such a refactoring could be done at just the annuity level. If life wanted to emulate later, it could. But then again, if there would be alot of code duplication, it wouldn't be a good idea to do it unless it's used across the board. I don't know the answer to the question of whether there would be significant duplication...
Post a Comment