Coding style: assignments inside expressions?
Quick question asking for insight from this community: Which one is preferable?
Option ①
// How many spaces are there in the beginning of string? (and remove them)
int spaces = text.Length;
text = text.TrimStart(' ');
spaces -= text.Length;
- Advantage: Assignment on a separate line, thus side-effect is explicit
- Disadvantage: The first line looks nonsensical by itself; you have to notice the third line to understand it
Option ②
// How many spaces are there in the beginning of string? (and remove them)
int spaces = text.Length - (text = text.TrimStart(' ')).Length;
开发者_如何学JAVA
- Advantage: Statement makes sense in terms of the computation it performs
- Disadvantage: Assignment kinda hidden inside the expression; side-effect can be overlooked
I don't like either of them. Some guidelines for writing clear code:
- The meaning of a variable should remain the same throughout the lifetime of the variable.
Option (1) violates this guideline; the variable "spaces" is commented as meaning "how many spaces are in text" but it at no time actually has this meaning! It begins its lifetime by being the number of characters in text, and ends its lifetime as being the number of spaces that used to be in text. It means two different things throughout its lifetime and neither of them is what it is documented to mean.
An expression statement has exactly one side effect. (An "expression statement" is a statement that consists of a single expression; in C# the legal statement expressions are method calls, object constructions, increments, decrements and assignments.)
An expression has no side effects, except when the expression is the single side effect of an expression statement.
Option (2) obviously violates these guidelines. Expression statements that do multiple side effects are hard to reason about, they're hard to debug because you can't put the breakpoints where you want them, it's all bad.
I would rewrite your fragment to follow these guidelines.
string originalText = text;
string trimmedText = originalText.TrimStart(' ');
int removedSpaces = originalText.Length - trimmedText.Length;
text = trimmedText;
One side effect per line, and every variable means exactly the same thing throughout its entire lifetime.
I'd do option 1b:
int initial_length = text.Length;
text = text.TrimStart(' ');
int spaces = initial_length - text.Length;
Sure, it's almost a duplicate of option one, but it's a little clearer (and you might need the initial length of your string later on).
I personally prefer option 1. Although option 2 is more concise, and works correctly, I think of the guy who has to maintain this after I've moved on and I want to make my code as understandable as possible. I may know that an assignment as an expression evaluates to the value assigned, but the next guy may not.
What about an overload?
public static string TrimStart(this string s, char c, out int numCharsTrimmed)
{
numCharsTrimmed = s.Length;
s = s.TrimStart(c);
numCharsTrimmed -= s.Length;
}
Option ① All day. It's readable. Option ② is much more difficult to maintain.
From the point of view of your question itself, I'd say not to do assignments within expressions because it's not supported in all languages, Python for example, so if you want to remain consistent in your own personal coding style, you could stick with the traditional assignments.
精彩评论