Coding style: assignments inside expressions?

Question

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;

• Advantage: Statement makes sense in terms of the computation it performs
• Disadvantage: Assignment kinda hidden inside the expression; side-effect can be overlooked

---

Answer 1

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.

Answer 2

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