1

A simple question as to whether or not this would be a proper way to combine logical statements for readability and organization purposes.

The 'standard' way I was taught to write code:

txtbox_vendor.Visible = false;
txtbox_vendor.Enabled = false;
txtbox_vendorCity.Visible = false;
txtbox_vendorCity.Enabled = false;
txtbox_vendorState.Visible = false;
txtbox_vendorState.Enabled = false;

Now obviously if I have a huge amount of textboxes or any objects that are being being switched on and off, would it be better to write it like this:

txtbox_vendor.Visible = false; txtbox_vendor.Enabled = false;
txtbox_vendorCity.Visible = false; txtbox_vendorCity.Enabled = false;
txtbox_vendorState.Visible = false; txtbox_vendorState.Enabled = false;

Obviously, performance isn't going to be an issue.

But is the second option considered a correct way to write code for legibility? Or is it frowned upon?

  • 6
    Generally speaking I tend to have one semicolon per line (unless it's a for loop). One line, one statement. – Matthew Sep 1 '16 at 19:47
  • Matthew, this was my thinking as well. It was just a curious thought I had and came across looking at some legacy code. – Gary.Taylor717 Sep 1 '16 at 19:49
  • 7
    Another option to increase legibility may be to create a method that accepts a textbox and sets the .Visible and .Enabled property in it. – Matthew Sep 1 '16 at 19:49
  • I find the second way much harder to read. – Loren Pechtel Sep 2 '16 at 0:39
8

The standard is one code statement per line. If a statement is too long, it should be broken across several lines in the file. The reverse is not true.

Why?

  • Automated code merges rely on one statement per line
  • Stack traces identify code position by line number; with more than one statement per line, a line number is ambiguous
  • One statement per line allows easy reading of nesting via indentation
  • One statement per line is consistent with code that is generated by most code wizards (other than minifiers).
  • File diff'ing tools such as Beyond Compare are much easier to use with one statement per line

It used to be the following idiom was not common but somewhat acceptable:

txtbox_vendor.Visible = txtbox_vendor.Enabled = txtbox_vendorCity.Visible = false;

Technically this is one statement. This works because the output of the = (assignment) operator is the value being assigned. I find it confusing and it is no longer common or encouraged.

On a side note, if you are coming across a lot of situations where you are even considering using multiple statements per line, as in your example, I have to wonder if your code suffers from other more serious stylistic and structural issues, e.g. a lack of modularity or failure to honor the single responsibility principle.

P.S. You do not need to set Enabled=false if you are setting Visible=false. If you insist on setting both, consider using an extension method to combine them into one call, and therefore one line.

  • Thank you for the in depth explanation. I wasn't writing the code this way by choice at first, I was simply following the standard a set of legacy code I was working with had. But after reading these answers and doing some investigating, I decided to refactor and change the program structure. SRP really does help and DRY as well. – Gary.Taylor717 Sep 2 '16 at 19:00
8

Groups of things that are the same can be placed in an array/collection and iterated over to avoid large amounts of identical-seeming code.

var textboxes = new[] {txtbox_vendor, txtbox_vendorCity, txtbox_vendorState};
foreach (var textbox in textboxes)
{
    textbox.Enabled = false;
    textbox.Visible = false;
}

In this way you might avoid the temptation to save space by placing multiple statements on a single line (which is indeed frowned upon.)

  • 1
    This addresses the OP's example but not his question, which could include lines of code that are not simple assignment or which involve classes that do not share a supertype. – John Wu Sep 2 '16 at 17:48
0

You can still set all properties with only one value and keep the one statement per line rule by doing this:

txtbox_vendor.Visible = 
txtbox_vendor.Enabled =
txtbox_vendorCity.Visible = 
txtbox_vendorCity.Enabled =
txtbox_vendorState.Visible = 
txtbox_vendorState.Enabled = false;

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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