How best to modify and return a C# argument?
Asked Answered
T

4

5

I need to take a Widget that already has several property values set. I need to change the Widget's name. I'm drawn toward Option 3, but I'm having a hard time articulating why.

public void Do(Widget widget) { // 1
    widget.Name = "new name";
}

public void Do(ref Widget widget) { // 2
    widget.Name = "new name";
}

public Widget Do(Widget widget) { // 3
    widget.Name = "new name";
    return widget;
}

I'd like to play Devil's Advocate with a few questions and gather responses, to help me explain why I'm drawn to Option 3.

Option 1 : Why not just modify the Widget that's passed in? You're only "returning" one object. Why not just use the object that's passed in?

Option 2 : Why not return void? Why not just communicate in the signature that you'll be using the actual memory pointer to the parameter object itself?

Option 3 : Isn't it weird to you that you're returning the same object you're passing in?

Teenyweeny answered 22/9, 2011 at 15:50 Comment(3)
It is hard to have much of an opinion without more context.Receptacle
@Lance: The formulation of your question, examples, and your comments about each option, suggests a confusion about how reference types behave in .NET. For instance, the use of ref in the second option is entirely superflous.Miceli
@Miceli Agreed. My understanding is that Option 2 would be much more interesting if I were assigning to widget, not one of its properties, but I think Option 2, as it's shown, reflects the confusion that many developers (myself perhaps included, at times) have about the proper use of the ref keyword.Teenyweeny
E
12

Option 1: This is the most common approach - you do not need ref if you don't want to modify the reference itself. Make sure that you name your method appropriately so the expectation is that the passed object is indeed modified.

Option 2: This is only useful if you want to change the passed reference itself, i.e. create a new Widget instance or set the reference to point to an existing widget (this might be useful if you want to keep overall number of instances low if they all have the same properties, see Flyweight pattern, generally the returned widgets should be immutable in this case though and you would use a factory instead). In your case this does not seem appropriate.

Option 3: This allows for a fluent "builder" approach - also has its benefits, i.e chaining of changes to the Widget which some people consider more expressive and self-documenting. Also see Fluent interface

Esthonia answered 22/9, 2011 at 15:52 Comment(4)
Although the caller doesn't specify, it's possible for Widget to be a struct. In which case, only option's 2 and 3 would make sense.Miceli
@LBushkin: That's true indeed - I find that highly unlikely though - if so he's probably got other problems.Esthonia
@Esthonia What naming convention would you recommend for setting the expectation that the passed object is modified?Teenyweeny
@lance: Depends on the context - How about UpdateName() ? But it seems you are setting the name to some fixed value? Most important thing (imo) is make it readable - when you look over the method call you should be immediately able to gather what it doesEsthonia
Q
6

Option 1:

Fine. Exactly what I'd use. You can mutate the content of the widget, but not the widget itself.

Option 2:

No. No. No. You don't modify the reference itself, so you don't need to use ref. If you modified the reference itself (for example widget = new Widget() then out/ref are the correct choices, but in my experience that's rarely necessary.

Option 3:
Similar to option 1. But can be chained in a fluent style API. Personally I don't like this. I only use that signature if I return a copy and leave the original object untouched.


But the most important thing here is how you name the method. The name needs to clearly imply that the original object is mutated.

In many situations I'd opt for Option 4: Make the type immutable and return a copy. But with widgets which obviously are entity and not value like, this makes no sense.

Quantum answered 22/9, 2011 at 15:53 Comment(5)
Option 1 won't modify the widget in the code that's calling this function because it's being passed by value, not by reference.Sight
@Sight If widget is a reference type it will modify the content of the widget. What you're saying only applies to structs/value-types, and somebody who makes a widget a value type in C# is insane.Quantum
@David: Your statement would only be true if Widget is a struct. If it's a reference type (as one would generally assume), then a reference to the Widget instance is passed by value, allowing the caller to observe changes made in the method Do().Miceli
You're right. Deleting my original answer because I made the same mistake there. Thanks to everyone who commented!Sight
@CodeInChaos: It's not really constructive to judge the sanity of the OP based on the use of the name Widget for their type - that name is far too non-specific to assume that it's obvious that that a Widget couldn't be a value type ... however, likely it's not (since the OP doesn't mention that). However, given the general confusion of the OP about the nature of ref, it's possible there are some other conceptual errors at play as well.Miceli
C
4

There isn't actually any functional difference between the 3 options. (There ARE differences, just none that are relevant to your question.) Keep it simple - use option 1.

Cancroid answered 22/9, 2011 at 15:53 Comment(0)
E
1

I think Option 1 and Option 3 are both viable options. Option 3 has the benefit of being self-documenting in that it implies that you are modifying Widget in the method. I think the worst option is Option 2. The ref keyword to me implies that you are modifying the reference of the object, which you are most certainly not doing.

Emulsify answered 22/9, 2011 at 15:56 Comment(2)
I don't see Option 3 as self documenting. In most cases this signature implies that you don't change the parameter, but return a modified copy. The advantage of Option 3 is that it allows chaining, but I think it's less self documenting.Quantum
@CodeInChaos - In practice I use Option 1, but I can see people understanding that properties on the object may change in Option 3, where as they may not expect it in Option 1. For example: public void Log(MyClass a) { a.IWasLogged = true; Logger.Log(a); } a was changed here, but may not be obvious to the developer. I am not sure it is worth going for Option 3, but wanted to highlight that fact I suppose.Emulsify

© 2022 - 2024 — McMap. All rights reserved.