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?

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
Gary.Taylor717
  • 114
  • 1
  • 7
  • 6
    Generally speaking I tend to have one semicolon per line (unless it's a `for` loop). One line, one statement. – Matthew Sep 01 '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 01 '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 01 '16 at 19:49
  • Possible duplicate of [How would you know if you've written readable and easily maintainable code?](http://programmers.stackexchange.com/questions/141005/how-would-you-know-if-youve-written-readable-and-easily-maintainable-code) – gnat Sep 01 '16 at 19:57
  • I find the second way much harder to read. – Loren Pechtel Sep 02 '16 at 00:39

3 Answers3

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

Justin T.
  • 97
  • 2
  • 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 02 '16 at 17:48
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.

Community
  • 1
John Wu
  • 26,032
  • 10
  • 63
  • 84
  • 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 02 '16 at 19:00
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;
t3chb0t
  • 2,515
  • 2
  • 21
  • 34