Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm working on a VB.Net WinForms project and found myself writing code like this:

this.Fizz.Enabled = this.Buzz.Enabled = someCondition;

I couldn't decide whether that was bad code or not. Are there any .NET guidelines for when/when-not to do assignment chaining?

share|improve this question
1  
Related: stackoverflow.com/questions/5590392/… –  Job May 8 '13 at 22:20
    
Specifically in the context of Winforms - if you have just two buttons/controls, then write out two lines. If you have a bunch of them, then perhaps save the logical groups of them in a set, and then apply an attribute change to the whole set with aid of a helper function. Still, as Robert said, try to minimize state! Frankly, try to simplify the UI first. You can also logically separate controls by groupboxes, or give them a specific Tag, and then can change all controls in a given GroupBox with tag = "foo" to have .Enabled = someCondition. Speed will matter less than clarity. –  Job May 8 '13 at 22:25
add comment

3 Answers

up vote 18 down vote accepted

I never do it. I always put each assignment on its own line. Clarity is king.

In practice, I seldom use enough state variables to make chaining assignments necessary. If I get to that point, I start looking for ways to trim the amount of state I am using.

share|improve this answer
1  
+1 having enough state hanging around that you can't spare an extra line in your method is a sign you're juggling too much in a single method. Time to break it into multiple methods. –  Jimmy Hoffa May 8 '13 at 22:59
add comment

It depends on the programming language. If you use a language where variable types are not enforced, then this could happen.

x = 1;
y = 494.0;
z = new Object();
x = y = z = "hello";

Now x,y,z are all a string. This might be confusing if found later on in the code.

The other problem is operator overloads. Some languages allow you to change what happens for the left-hand assignment of a property. So the following could happen.

x = y.something = 0;

Where y.something doesn't return 0. It returns null.

Otherwise, I don't have a problem with it. If it's a strongly typed language and there is no risk in the above happening. Then no problem.

EDIT:

The other problem is in how you read the source code.

 a = b = c = new Object();

Is not the same as.

 a = new Object();
 b = new Object();
 c = new Object();

It's really like this.

 c = new Object();
 b = c;
 a = b;

It might not be clear that all three variables have the same reference.

share|improve this answer
6  
So, other than those catastrophic problems... :) –  Robert Harvey May 8 '13 at 22:18
    
Two of his mentioned problems only show up in OO languages and only when chaining assignments with objects. One of his mentioned problems only shows up in dynamically typed languages. That doesn't seem catastrophic to me. –  Michael Shaw May 9 '13 at 0:02
    
@MichaelShaw: The problems are so easily avoided, though. –  Robert Harvey May 9 '13 at 14:41
    
The question was if it was bad code, and when to do it and not to do it. Answer: it's not bad code but there are many cases where you should not do it. I'll try to think up some examples of when it's good to use it, but I've been thinking about that for a while now and can't think of one. lol –  Mathew Foscarini May 9 '13 at 15:19
    
@RobertHarvey: They're so easily avoided that they don't exist in many languages and you'd have to be doing something goofy in languages where it might cause problems. –  Michael Shaw May 9 '13 at 16:03
show 4 more comments

It's not necessarily bad code, but it does make your code significantly less readable. Typically, I would strongly recommend against it, although I suppose there are a few cases where it might be acceptable (or merely less bad). For example, when setting a few local variables to constant value:

// accumulators
int j = 0, k = 0;
for(int i = 0; ...)
{
    ...
    if(reset)
    {
        // oops we have to start from the beginning
        i = j = k = 0;
    }
}

In this case the line, and more importantly the comment that accompanies it, make it clear what the authors intention was when writing it. This code also contains a compound declaration which I also generally discourage, but are more acceptable here for the same reasons.

Still, I would recommend against it in most cases. There is never any harm in writing more explicit code (as long as it's functionally identical).

share|improve this answer
2  
It seems more readable to me. If you have separate assignments to the same value, verifying that they all get set to that value takes a little extra time compared to chaining them. –  Michael Shaw May 8 '13 at 23:58
    
@MichaelShaw Sometimes, that's true--and that is kind of the point I was trying to make: the choice to use chained assignments should be driven first and foremost by readability. –  p.s.w.g May 9 '13 at 0:04
    
Having small functions, one could also throw an exception when something bad happens and have the caller decide whether the function should be called again or not - makes it easier to do a limited number of retries. –  Job May 9 '13 at 3:12
1  
@Job Perhaps my somethingBadHappened was a bit confusing. The code I wrote was just an example of some hypothetical number crunching where this sort of thing would be more acceptable. If this were higher-level code that needed to handle exceptions, I would structure it much differently. –  p.s.w.g May 9 '13 at 14:56
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.