3

The premise is using a language (e.g. C#, javascript) which passes an object by reference into a method.

Assume there is an object "Player" which has a property "Level" and there is a method takes Player as parameter and its function is to increase the player's level by 1.

The method can be designed something like (c# syntax):

void AddLevelBy1(Player player)
{
 player.Level +=1;
}

The concern was this method changed the value of the argument and I could not find a generic way to indicate (or may prevent?) this.

One might get some hints from the method name "AddLevelBy1". But such a way is not reliable and if the method was name "foo" then it becomes even more ambiguous.

So is there a solution / convention for this case (indicate whether arguments get changed)? Or such method design was considered a bad practice?

Added:

Try to describe my case more clearly:

  1. I have an object "Player" which was not defined by myself (cannot change).

  2. I wrote a method (AddLevelBy1() in this example) which takes "Player" as parameter (I know what happened).

  3. I also pass this "Player" to some other methods (e.g. foo()) which were not written by me (so this is no way to know whether the "Player" got changed).

Thanks for some answers below but what if the codes were designed in the manner I described above.

I have added a demo of the case I described: http://jsfiddle.net/54sun/srhHt/9/

  • 1
    A method implies a receiver. You'd model this as player.addLevelBy1() if I understand your question correctly. – elclanrs Mar 14 '14 at 10:02
5

If you have method like that, then such method should actually be part of Player class. This then clearly indicates the method might change the state of the object.

class Player
{
    void AddLevelBy1()
    {
        this.level += 1;
    }
}

player.AddLevelBy1()

The problems start when you have more than 1 objects being changed, but there is no general solution to that.

If using c#, the "out" keyword can be used to indicate the change.

This is wrong. out keyword is to indicate something completely else. See more here: https://stackoverflow.com/questions/9875333/c-sharp-passing-reference-type-directly-vs-out-parameter

  • Indeed, the out keyword means "I'm changing the value of the parameter", not "I'm changing something in the object identified by the parameter". – Ross Patterson Mar 14 '14 at 10:54
  • Sorry about the "out" keyword. I was wrong on it and I have corrected the descriptions in my question. Thanks for pointing it out as I took it for granted from its name "out". – Sun Mar 15 '14 at 20:21
1

Although your example is simplistic, and may be modeled differently (like @Euphoric suggested, or by using extension methods if the class is not in your code-base), there are a lot of use-cases where it is legitimate for a method to change the state of their parameters.

For example, builder processes, your recursive algorithms, like DFS detecting cycles.

You can use the Law of Demeter as a rule of thumb of what should a method do and not do:

  • You can play with yourself.
  • You can play with your own toys (but you can’t take them apart),
  • You can play with toys that were given to you.
  • And you can play with toys you’ve made yourself.

Of course, when a method has side-effects, it should be declared in its name: IncrementPlayerLevel(Player player) plainly states that it is going to change the level of the player...

On the other side of the design considerations, sometimes it is critical that methods do not change the state. For example - implementing an Actor model for parallel computations. When considering this, you are better off designing your state objects as immutable, so you don't need to ask yourself whether any method has changed your objects without you being aware of it.

  • I think designing an object immutable is more language-dependent and I don't think it solved the problem in my case (as change should be allowed). I'm also agree with what you suggested but that relies on good manner of a programmer. So do we have a mechanism to cope with such a situation? – Sun Mar 15 '14 at 21:32
  • What situation are you referring to? – Uri Agassi Mar 16 '14 at 6:53
  • I mean the situation that I described in the Added points (1,2,3). If the codes were designed like that, was it a bad design that one should avoid or there was a pattern to design in this way? – Sun Mar 18 '14 at 13:32
  • I've already suggested an expressive name to the function (IncrementPlayerLevel), if extension is not possible (several programming languages allow for extensions) for point #2; for point #3, if you want to make sure that your object is not changed, you can pass a copy of that element. – Uri Agassi Mar 18 '14 at 14:46
  • Thanks for the answers. May I wrap them as: 1) The function name should indicate whether it changes the argument; 2) If the function caller want to prevent any change to the argument passed in, a deep copy is the ONLY way to protect its original object; 3) We can consider defining a method (without properly named) which changes its arguments is a bad practice. Sorry if this seems a stupid question. – Sun Mar 20 '14 at 12:47
1

As mentioned in Uri's answer, ideally your types would be immutable. Mutability complicates programs. But mainstream languages have poor support for immutability, and we have a long history of mutation which goes all the way back to the assembly days, so there's no getting around dealing with mutability sometimes. Sometimes mutating an argument is unavoidable - for example, if the class is not under your control, and it provides no way of cloning (i.e. deep copying) its values. So, it may not be a best practice, but sometimes you can't get around it.

Some have suggested making the mutator a method of the class. Even if the class is under your control, this doesn't necessarily make sense - there's an infinite number of things you might want to do with your mutable class, and thus an infinite number of functions that may need to mutate it. Secondly, not everything is a method. If you have a commutative operation, there is no "recipient" for the function; either argument is equally valid. If you have a function that operates on arguments of two different types, why should the function belong to one class or the other? Sometimes this makes no sense. If you don't need access to the class's private members, it doesn't make sense to make the function a method.

Ok, so you need to mutate the argument. What can you do to mitigate the potential damage? As you pointed out, it's critical that the function name unambiguously informs you that that's going to happen. Secondly, you should avoid creating functions that are accessors and mutators at the same time. If a function's purpose is to return some information about an object, people are going to be surprised if it also mutates that object or any other.

Uri brings up the Law of Demeter, which is an excellent guideline, but be warned that it's relevant only to abstractions. The idea is that an abstraction should hide its implementation, so chains like foo.bar.baz() are usually a sign that you're not doing a very good job there. But not everything is an abstraction! Some types (e.g. tuples, records) are simply data containers, and accessing their members is not a violation of the law, even if the end result is still a dotted chain. (Of course, if your abstraction shouldn't be exposing that tuple in the first place, it's still a problem.)

  • Whether the code belongs in the object to be mutated or something else depends upon the purpose of the thing being mutated. If code needs to process the coordinates of a large number of objects, using a method void GetLocation(Point pt) which copies an object's location into the supplied Point may be much more efficient than having a method Point GetLocation() which needs to construct a new Point instance every time it is invoked. The latter will likely be more convenient in cases where the caller just needs one or two points, but the former can be much better if there are thousands. – supercat Nov 22 '14 at 23:48
  • Note that even though GetLocation acts upon an instance of Point, that doesn't mean it should be a member of that type. Instead, a Point should be viewed not as an object with behaviors, but rather as a container for storing two numbers. The purpose of GetLocation is to store an object's coordinates into a container; containers are not usually responsible for acquiring the things to go in them. Instead, whatever has the things that should go into the containers is generally given the responsibility of putting them there. – supercat Nov 22 '14 at 23:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.