coding standards: lining up large arrays with padding

Current standards:
http://drupal.org/coding-standards#array

My opinion:
I disagree with this idea. This kind of whitespace is not very maintainable. If you add another item that is longer, you have to go edit all the lines even though really only one line has changed, if you want to maintain the lining up. I also don't think it improves code readability.

I think our standard should be like what is currently on the standards page, and that we should add text there to suggest *not* lining things up (for the reason above). And we should update Coder to detect/fix this.

But really... Before we adopt one standard or the other, we should survey other coding standards out there and find out what they suggest. If someone has time to do some searching, that would be good.

Also, do you want to advocate other things lining up, like:

  $variable_name_1 = 'whatever';
  $name_2          = 'something else';

Because that's really the same thing....
[EDIT: the dangers of trying to edit code to line up while using a variable-width font in the editing window]

I think they look better lined up but I don't know if it's necessarily more readable. For me, it's a matter of having things lined up properly and in their own boxes and such. Neat and orderly. :)

I guess the big question is, how much of a pain is it when you have a patch that moves a bunch of others to line things up again? Is the "unrelated" change a hassle? If so, then looking nice isn't as important as making life easier for committers. :)

Edit: I like variables lined up as well.

Michelle

I like it lined up, especially in JS.

half of it come from Symfony
ack ' {2,}=>' core/vendor/Symfony/ | wc -l
139

ack ' {2,}=>' | wc -l
287

Yeah, Symfony doesn't count - we don't apply our coding standards to third-party code. :)

So far, in comments above, it looks like the "I like it lined up" camp is winning over the "It's hard to maintain" camp.

But we apprarently only have 148 lines in arrays that are lined up in core [not counting Symfony], as compared to countless lines like this:

  // hook_theme()
   'system_themes_page' => array(
      'variables' => array('theme_groups' => NULL),
      'file' => 'system.admin.inc',
    ),

  // hook_menu()
  $items['system/files'] = array(
    'title' => 'File download',
    'page callback' => 'file_download',
    'page arguments' => array('private'),
    'access callback' => TRUE,
    'type' => MENU_CALLBACK,
  );

  // form generation
  $form['timezone'] = array(
    '#type' => 'fieldset',
    '#title' => t('Locale settings'),
    '#weight' => 6,
    '#collapsible' => TRUE,
  );

 

etc. [I don't have this "ack" thing and when I tried to install it, it was something having to do with Kanji Japanese characters?!? so didn't count all the not-lined-up lines...]

Anyway, our overwhelming standard in core is definitely *not* to line things up, and I don't think there is any way we're going to go through core and line up all those non-lined-up arrays. Nor do I think we would want to maintain them lined up.

So what standard would you propose adopting? "Line things up if you feel like it"? Can anyone articulate a reason that some arrays should be lined up, while others don't need to be, that is consistent with how it's being used in core now?

+1 #1

Think we should convert those 148 lined up cases to "not lined up" and thus enforce the current standard.

Sorry to disagree joachim, but I think a less ambiguous standard and more homogeneous code is better. Also, between the 2 possibilities, I choose "not lined up" (current standard), sharing the mentioned motives.

my 0.0001c ;)

hugs

EDIT: After double checking the current standard, I see that it doesn't explicitly indicate the use of only one space next to the =>, so I propose we change "and spaces around the => key association operator" to "and a space around the => key association operator"

I think we should pick one or the other and fix code as it comes up rather than trying to go in and fix it all at once, unless someone has a lot of time on their hands. :)

+1 #8. my own exp most large PHP projects do the same way. (suggest padding to promote readability but not a requirement)

@jhodgdon,

ACK:
http://betterthangrep.com/

simply way to install it is
curl http://betterthangrep.com/ack-standalone > ~/bin/ack && chmod 0755 !#:3

make sure you have Perl 5 installed on your platform (linux, win, mac, etc...)

or use GIT:
git grep -e '\s\{2,\}=>' | wc -l

Can someone who is advocating for lining things up please try and articulate a standard for when it would make things better or more readable and when it won't? Because honestly I have not yet understood the difference between the situations where you think it would and where you think it wouldn't. Anything in arrays is probably related, right?

Anyway, if you want us to adopt a "we advocate lining up" standard, we need to articulate when and when not. Can someone who wants to adopt this please write up a proposed standard that we can consider for adoption, as an alternative to #1? Or some coding standards in other projects that advocate this type of thing?

I like the look of lining things up but it's not a super big deal to me. Honestly, I'm in favor of leaving this one up to personal preference but I know that can get messy in core when you're dealing with patches if one person leaves them staggered and another goes and lines them up. But, anyway, I think I'm too ambivalent about it to be much help so I'm dropping out of this one. :)

It's personal taste, and I'd never mark someone down for keeping things ragged and compact as they see fit, but I WOULD take offense if someone "fixed" my code that was aligned for readability to convert it to an un-aligned version.

I say:
Ragged is default - fine.
Aligned is not worse and is legal.

Let there be no holy wars about this.

Aside: I format my code like this automatically, by choice, as a pastime while my brain is churning on other stuff. Gardening code for aesthetic effect is meditative to me.
When I do code reviews (which I do often) - seeing intentionally formatted code tells me that this person cares about their code, and cares about those who come after them. Aligned code is fodder for peer review. Unaligned - while not wrong - indicates less pride and attention to detail. To me.

I side with Steve McConnell's assessment in "Code Complete" (which is a damn good software design bible) : Chapter 18.5
He recommends alignment when appropriate

The main justification is that these statements belong together and the alignment scheme shows that they belong together.

Points above (eg Joachims right above) agree that it's not such a big deal when looking at FAPI, but there are other cases - when the items or the assignments are isomorphic - that the "aligned" style is beneficial to readability and understanding (although potentially a little troublesome to maintenance if you don't have a helpful IDE)

Neither is wrong. Let neither be prohibited. But "easier to read" is more equal than others - in my book.

Coding standards are not about religious wars. Coding standards are about adopting standard ways for everyone to write our project's code, so that we can have uniform-looking, consistent code that everyone can maintain. I am sure that there are items in the Drupal coding standards that each of us has had trouble adopting (I know that is the case for me). But it's still important to articulate what the standards are, and for each of us (with the help of Coder) to follow them as much as possible.

So once again... If the development community as a whole wants to have a standard that assignments and arrays should be lined up when items "belong together", then we should adopt that. Obviously, there are people here that feel strongly about it, and there is support from excellent code construction books out there (I agree, McConnel's reference is excellent) for this standard.

But 99% of our Drupal code will then be out of compliance. Given that, I do not think this is a standard that the community as a whole has adopted or believes in. The current de facto standard that 99% of our code is using is that we don't line up assignments and arrays, even if the items "belong together". I also don't think Coder could distinguish between assignments/arrays that "belong together" and those that don't, so a standard that says "Line things up if you think they belong together" cannot be checked by Coder or automatically reformatted by Coder Update (or similar tools).

Which is why I think we either need to adopt a standard saying "Line up all arrays and consecutive assignments", or we need to adopt a standard saying "Don't line up any". Either one of these is clear without involving a value judgement on a developer, and either one could be checked/updated by Coder* modules.

After poking around some of the core instances where "line up" occurs, one question jumps out to me, "Is it worth NOT having a standard, measurable way to define proper commenting structures?" I know that I was very discouraged in the early days of my development by finding core instances where comments were aligned, while being directed to not align in my own comments. However, after working with the community more I've come to understand the need for measurable standards that promote forward and backward migration of code (as mentioned by jhodgdon).

On that note, I have to throw my hat in the ring for deciding one way or the other on this issue. Good commenting is something that the Drupal community needs much more of and having yet another "judgement call" standard doesn't feel to me like it promotes readable comments.

It may also be worth mentioning classical conditioning in this instance. If we decide on and properly implement one specific method of "proper" commenting, then the community will become familiar with it over time.

Just my humble opinions on the matter having spent hours upon hours beating my head against the api docs.

Throw my hat in the ring of "leave this up to developers' personal preferences." I actually /don't/ think we need standards for /every/ single thing, especially when it comes to optional whitespace. If PEAR doesn't care, neither should we.

It's been almost 2 years with this issue. I'm going to close it for now. If someone feels strongly or PEAR changes it's standards then let's consider re-opening it.