Pass by reference: Which is more readable/right?

Question

I have the following class:

public class Person
{
     public String Name { get; set; }
}

I have a method that takes in Person and a String as parameters:

public void ChangeName(Person p, String name)
{
     p.Name = name;
}

Since Person was passed by reference, it should change the Name of the passed instance.

But is this method more readable than the one above?

public Person ChangeName(Person p, String name)
{
     p.Name = name;
     return p;
}

Technically, `p` is not passed by reference in any of those examples. — Amber, Mar 11 '11 at 06:07
`p` *contains* and *passes* a reference, but is not *passed by reference* - there is a distinct difference between the two. Pass by reference (by declaring the arguments as `ChangeName(ref Person p, ...)` would allow something like `p = foo` to then completely change which Person the caller's variable was pointing at, not just the contents of the currently pointed-to Person. — Amber, Mar 11 '11 at 06:10
For more details, see http://msdn.microsoft.com/en-us/library/0f66670z(v=vs.71).aspx examples #4 and #5. — Amber, Mar 11 '11 at 06:12

score 12 · Accepted Answer · answered Mar 11 '11 at 06:05

Is it more readable? No. In fact you may be doing more harm them good.

By having it return a Person object, it might lead you to believe that instead of modifying the Person parameter, it is actually creating a new Person based on p but with a different name and someone could incorrectly assume that p is never changed.

Either way, if you have a method that has no affect on the class it is apart of it should probably be static. That helps you know for sure that it doesn't affect its class. Only have the method return a value if you need it to return a value.

So here is my recommendation for this method:

public static void ChangeName(Person p, String name)
{
    p.Name = name;
}

score 1 · Answer 2 · answered Mar 11 '11 at 06:10

There isn't anything right/wrong with either approach. Depends on what your program needs.

Returning the parameter passed into a method is rarely needed as it is always possible for the user to just use the variable passed as argument instead.

It, however, gives you the flexibility of eventually overriding this implementation, or passing this implementation into another function which accepts delegates with similar signatures. Then you can pass in other implementations that does not return the same Person object.

Do it only if you really need the flexibility.

score 1 · Answer 3 · answered Mar 11 '11 at 06:58

I would suggest you use one of the following for best readability:

public static void ChangeName(Person p, String name)
{
    p.Name = name;
}

public static Person WithName(Person p, String name)
{
    return new Person(p) { Name = name };    
}

The second one treats the Person object as immutable and does not change the state of the object. The ChangeName function explicitly changes the state of the input object. I think it's important to make a clear distinction between the two types of methods. A good rule of thumb to follow is that a method should not change the state of an object AND return one at the same time.

fastcodejava · Answer 4 · 2011-03-11T07:28:14.000

1

first of all p is not being passed by reference in the first example. Your second method makes one believe that it is returning a new reference which it is not. So I don't think the second one is any clearer than the first one.

edited Mar 11 '11 at 07:28

answered Mar 11 '11 at 07:21

fastcodejava

39,895
28
133
186

score 0 · Answer 5 · answered Mar 11 '11 at 06:09

In the case you've described, I would say neither. Its not really clear what you are trying to do with this method. Just use the object and set the property. Inserting a method into the execution path just complicates understanding and creates another dependency on the Person object and its underlying value.

If you are asking a meta question that involves some design over and above the code you have posted, then I am missing it.

score 0 · Answer 6 · answered Mar 11 '11 at 06:10

0

The first one is better, because of that the second one might lead you to believe that p is immutable. But, the whole method is useless since it just calls the setter. Why not just call the setter directly?

answered Mar 11 '11 at 06:10

Teo Klestrup Röijezon

5,097
3
29
37

score 0 · Answer 7 · answered Mar 11 '11 at 07:15

I beleve that your 2nd approach is not more readable YAGNI. But if you change it like this

public static class PersonExtensions 
{
public static Person ChangeName(this Person p, String name)
{
 p.Name = name;
 return p;
}

you will have an extensionmethod for a fluent interface a la

new Person().ChangeName("Peter Smith").SendEmail().Subject("Test Mail").Receiver("....)

score 0 · Answer 8 · answered Mar 11 '11 at 07:21

Here is the definitive reference to understand passing parameters by value/reference.

Looking at the code, why don't you use property?

public string Name
{
   set {name = value;}
   get { return name; }
}

EDIT: Auto implemented properties

public string Name
{
   set;
   get;
}

Pass by reference: Which is more readable/right?

8 Answers8