Return Assignment Expression

Question

While I was working on some old C # code, I came across code that annoyed me.

Without much noise, this happens something like this:

private string foo(string _text) { /* some manipulation on _text */ return _text = Server.HtmlDecode(_text); } 

This is the last line that annoys me; I am from C background and I understand that the code is really trying to return the decoded _text variable. Also the value of the assignment operator is the left operand, so I see it.

But I still find it unpleasant.

Is ordinate practice in C # that I need to get used to?

For me, the last line should be simple

 return Server.HtmlDecode(_text); 

and is not an expression of assignment. Is there a deeper C # function that I don't know about?

Answer 1

While I was working on some old C # code, I came across code that annoyed me.

There are many unpleasant problems. Let me list them all.

 private string foo(string _text) { /* some manipulation on _text */ return _text = Server.HtmlDecode(_text); } 

This is the last line that annoys me

The comment is also depressed. Local variables are cheap. There is no need to erase the original _text value. Instead, create a new local variable and manipulate it. Thus, when debugging at any time, you can find out what the original argument is. (Remember that the original argument may have the right to garbage collection at the moment the variable is overwritten, and therefore may be lost forever.)

Do not write a formal parameter without good reason. This makes debugging difficult.

the value of the assignment operator is the left operand, so I see it.

This is correct in this case, but overall incorrectly incorrect; the value of the assignment operator in C # is the value of the correct operand after being converted to the type associated with the left side. Remember that the left side may not matter; this may be a write-only property.

Is ordinate practice in C # that I need to get used to?

There is standard practice here, yes. Bizarre in this regard is (1) that the selected variable is formal, and (2) that the assignment is combined with return .

It would be common practice in C # to say:

 string decoded = Server.HtmlDecode(_text); return decoded; 

Now you may wonder how the irresistible benefits of this depend on what you offer:

 return Server.HtmlDecode(_text); 

Answer: before Visual Studio 2013, there was no object in the debugger to check the return value of the method call! Therefore, if you want to know what value returned by HtmlDecode was during debugging, you had the following options:

• Assembly level debugging and EAX content browsing
• Step in HtmlDecode and check its status
• Exit the current method and examine what return value has been assigned
• Assign the result to another useless local variable , and then examine the local one in the debugger

Since the first three are terrible, and the last is easy, something that many C # programmers are used to doing.

If you do this and then do not use the resulting local, the C # compiler knows that this is common practice and deliberately suppresses "you wrote a local message that you never read." It gives this warning only if the local user had a permanent record, in which case you already knew what was at compile time, and usually you do not need to check it in the debugger.

We hope that VS2013 will finally support this frequently requested function, this type of templates will gradually disappear.

Answer 2

This instruction is redundant and not C # practice.

By doing this while ReSharper is active, a warning will also be issued

The assigned value is not used in any execution path.

As you mentioned, this code will really be best practice

 return Server.HtmlDecode(_text); 

In addition, since decoding is part of the manipulation of _text, it would also be useful to separate the assignment and return operator in order to keep the logic in the same block:

 /* Other manipulations on _text */ _text = Server.HtmlDecode(_text); return _text; 

Answer 3

No, this is a stupid thing, and it sacrifices readability in the name of overflowing more code on one line. This is almost always bad.

In this case, it actually has no purpose. _text is a method parameter, and changing it in the body of the method does nothing. The string passed to the method will not be changed.

Answer 4

They have the same result. The most common are the latter, or:

 _text = Server.HtmlDecode(_text); return _text; 

(I like either the above or return Server.HtmlDecode(_text); but not like in the code you are reading)

Answer 5

No, there is no deeper C # function in this case, doing the job in the return simply pointless. This only complicates the execution of the code to be executed.

The only difference from what you suggest is that the decoded value is assigned to the _text variable, but since this variable is no longer used, it does not matter what value it has when the method ends. A parameter is a local variable inside a method that does not affect anything outside the method.