CODEWITHSUNDEEP
c#coding-styledesign-guidelines
Ian
Ian

Reputation: 5725

Pass by reference: Which is more readable/right?

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;
}

Upvotes: 9

Views: 286

Answers (8)

fastcodejava
fastcodejava

Reputation: 41087

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.

Upvotes: 1

shahkalpesh
shahkalpesh

Reputation: 33474

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;
}

Upvotes: 0

k3b
k3b

Reputation: 14755

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("....)

Upvotes: 0

Can Gencer
Can Gencer

Reputation: 8885

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.

Upvotes: 1

Stephen Chung
Stephen Chung

Reputation: 14605

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.

Upvotes: 1

Teo Klestrup Röijezon
Teo Klestrup Röijezon

Reputation: 5247

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?

Upvotes: 0

Ritch Melton
Ritch Melton

Reputation: 11608

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.

Upvotes: 0

Corey Sunwold
Corey Sunwold

Reputation: 10254

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;
}

Upvotes: 12

Related Questions