Is it bad practice to embed method parameter expressions?

Question

Is it generally considered bad practice to structure code with embedded expressions in method parameters? Should variables be declared instead?

(Android code snippet for an example)

((EditText)view.findViewById(R.id.fooEditText))
  .setText(
    someExpression
      ? getResources().getString(R.string.true_expression_text)
      : getResources().getString(R.string.false_expression_text)
  );

Personally I think it looks fine, but am just wondering if this is considered repulsing :)

Answer 1

I would almost certainly simplify that, in a number of ways:

EditText editText = (EditText) view.findViewById(R.id.fooEditText);
String resourceName = someExpression ? R.string.true_expression_text
                                     : R.string.false_expression_text;
editText.setText(getResources().getString(resourceName));

Doing it all in one statement makes it harder to read and harder to debug, IMO. Note that I've also removed duplication here, but using the fact that you were calling getResources().getString(...) in both operands of the conditional operator, just with different resource names.

My main beef with the original code is calling a method on the result of a cast - aside from anything else, it introduces more brackets than you need, which is generally confusing.

Answer 2

I'd say this depends on the situation, for instance.

player.setName(User.getName());

Would be fine, however, train wrecking such as below...

player.setName(getGroup().getUsers().get(0).getName());

I'd say is bad practice and is mentioned in Clean Code by Bob Martin regarding the dangers of train wrecks. Also duplicate calls as mentioned by @Jon Skeet is another reason to use a variable rather than a method call.

Answer 3

The word "repulsing" was yours, but it certainly describes my reaction. I can't focus on what this statement is doing because it has an if statement, a search, and at least 5 dereferences happening before it gets started.

I find the trinary operator particularly pernicious, since I have to hold two disjoint sets of state in my mind while I parse everything else. Some folks prefer brevity to local variables (I'm not one of them) but trinary operators (or any other branch) embedded in other statements are especially unloveable. If you ignore the rest of Clean Code or similar works because you enjoy complex statements, at least separate the conditionals out.