views:

49

answers:

3

The following code illustrates a pattern I sometimes see, whereby an object is transformed implicitly as it is passed as a parameter across a number of method calls.

var o = new MyReferenceType();
DoSomeWorkAndPossiblyModifyO(o);
DoYetMoreWorkAndPossiblyFurtherModifyO(o);

//now use o...

This feels wrong to me (it hardly feels object oriented). Is it acceptable?

+1  A: 

It depends entirely on what the methods actually do, besides modifying that object.

For instance, an object primarily related to keeping some state in memory might for instance not have anything related to persisting that state anywhere.

The methods could for instance load data from a database, and update the object with that information.

However! Since I program mostly in C# and thus .NET, which is a wholly object-oriented language, I would actually write your code like this:

var o = new MyReferenceType();
SomeOtherClass.DoSomeWorkAndPossiblyModifyO(o);
SomeOtherClass.DoYetMoreWorkAndPossiblyFurtherModifyO(o);

//now use o...

In which case the actual name of that other class (or those other classes if there's 2 involved) would give me a big clue as to what is actually happening and/or the context.

Example:

Person p = new Person();
DatabaseContext.FetchAllLazilyLoadedProperties(p);
DatabaseContext.Save(p); // updates primary key property with new ID
Lasse V. Karlsen
+2  A: 

Based on your method names, I would argue that there is nothing implicit in the transformation. This pattern would be acceptable. If, on the other hand your methods had names like printO(o) or compareTo(o), but actually modified the Object o, the design would be bad.

akf
+1  A: 

It is acceptable but usually bad style. The usual "good" approach is:

DoSomeWorkAndModify(&o);  // explicit reference means we will be accepting changes

o = DoSomeWorkAndReturnModified(o); // much more elastic because you often want to keep original.

The approach you presented makes sense when o is huge, and making a copy of it in memory is out of question, or if it's a function you (and nobody else = private) use very frequently and don't want to bother with the & syntax. Otherwise it's laziness that results in some really difficult to detect bugs.

SF.
Thanks - this articulates my original thought well.
Ben Aston