Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm writing a list of inputs and outputs for to be compared in unit tests.

var equals = [
                //input                         //output
                ["name:(John Smith)",           "name:(John~ Smith~)"   ],
                ["name:Jon~0.1",                "name:Jon~0.1"          ],
                ["Jon",                         "Jon~"                  ],
                ["Jon Smith~0.1",               "Jon~ Smith~0.1"        ],
                ["Jon AND Bob",                 "Jon~ AND Bob~"         ],
                ["Jon AND Bob~0.1",             "Jon~ AND Bob~0.1"      ],
                ["Steve^9 Jon", ]               "Steve^9 Jon~"          ]

            ];

I've formatted it as such, so it's easy to compare and read. However, it's unconventional.

Is this a bad idea?

share|improve this question
    
I imagine one possible problem would be that it might spill over on different editors/screen sizes, and look awful. –  dwjohnston Jan 23 at 3:19

3 Answers 3

An argument against table-style alignment in code

Except when the editor/IDE helps maintain alignment with little work from the programmer, and all those who work with the code have that same facility, horizontal, table-like alignment in code is more trouble than it's worth, for these reasons:

  • Search/replace operations on the entire code-base (as when renaming a variable or method) will tend to leave such code in a mess that needs manual cleanup.

  • Anyone whose editor/IDE does not make horizontal alignment easy will spend a lot of time hitting space or DEL to make things line up when adding or deleting an entry.

  • When the entire table needs to be realigned (as when adding a new row that causes a column to grow), the version control system will show that the entire table changed.

For those reasons, I'm inclined to put up with a little bit of ugly:

 var equals = [ //input, output
   ["name:(John Smith)", "name:(John~ Smith~)"],
   ["name:Jon~0.1", "name:Jon~0.1"],
   //...
 ];

or, as @ChrisW suggests:

 var equals = [ //input, output
   [
     "name:(John Smith)",
     "name:(John~ Smith~)"
   ],
   [
     "name:Jon~0.1",
     "name:Jon~0.1"
   ],
   //...
 ];

But maybe data doesn't want to be in the code

Sometimes data wants its own home, a place where "rows and columns" are more natural than in code. Perhaps that's a table in the database. In your case, where the table is input to a test, a .csv file might be good. A non-technical user could then view and edit the data with a spreadsheet program.

share|improve this answer
2  
Is there a second alternative you'd also find acceptable, which left-aligns each output string underneath its input string? IMO that might make it easier to visually compare each output with its input. –  ChrisW Jan 22 at 23:02
1  
For example, ["Jon" newline , "Jon~"] so that the , is under the [? –  ChrisW Jan 22 at 23:04
3  
I disagree with this answer. Usually, code is read more often than written, so the extra effort is worth it. Tabular data should have a tabular layout in code. Good formatting makes it easier to spot errors. –  amon Jan 22 at 23:06
2  
+1 That looks good (better than my suggestion). You could additionally perhaps save vertical space, by having the ], [ immediately after (on the same line as) each output. –  ChrisW Jan 22 at 23:44
1  
Good answer thanks Wayne. Here is a programmers.SE question on the same topic where I mention putting it in to CSV. Is there a better way of writing unit tests than a series of 'AssertEquals'? –  dwjohnston Jan 23 at 21:33

An opinion question, always tricky.

From a DRY perspective, your code should be usable to generate documentation, like 'what use cases does this routine support' or 'What are the mapping values between y and z'.

So I definitely align my tables, however I also tend to align the comma's and I tend to align 2 spaces in, not all the way to the right of var.

In other words, something like this :

var equals =                                            
  [                                                       
    //input               //output
    ["name:(John Smith)", "name:(John~ Smith~)"],
    ["name:Jon~0.1"     , "name:Jon~0.1"       ],
    ["Jon"              , "Jon~"               ],
    ["Jon Smith~0.1"    , "Jon~ Smith~0.1"     ],
    ["Jon AND Bob"      , "Jon~ AND Bob~"      ],
    ["Jon AND Bob~0.1"  , "Jon~ AND Bob~0.1"   ],
    ["Steve^9 Jon"      , "Steve^9 Jon~"       ]         
  ];       
share|improve this answer
    
+1 for aligning commas. I saw that, and it was bugging me. :) –  dwjohnston Jan 23 at 0:13
    
Can you state any advantage which this has over the other answer? –  ChrisW Jan 23 at 0:42
    
@ChrisW Aesthetics (which is subjective and a matter of personal taste). I like this version as well, although I tend to not align commas, because spaces before commas feel like improper punctuation to me and create that annoying pressure point in my head, just like unaligned tables. Not annoying me is an important advantage my code needs to have. :-) I tend to work in very small teams, though. (Objective disadvantages are well-stated in another answer.) –  Andrey Tarantsov Jan 23 at 3:03
1  
@ChrisW As per my answer, documentation, especially documentation that I provide to non-technical people, they understand this format whereas the other answer is ungrokkable for non-technical people. –  konijn Jan 23 at 3:04
    
I do not generally align commas since I do not like how it looks. It does however have the advantage that it makes it much easier to use block-selection to copy any column of the declaration to for example a default assignment or a validation code section. –  Boise Jan 23 at 7:55

From a pure code review perspective, not addressing the direct question of "Should I format my unit test data in a tabular way", but instead, is this the best way to write this data in the first place?

If you think about writing a unit test, is it more readable/maintainable to do:

    var equals =                                            
  [                                                       
    //input               //output
    ["name:(John Smith)", "name:(John~ Smith~)"],
    ["name:Jon~0.1"     , "name:Jon~0.1"       ],
    ["Jon"              , "Jon~"               ],
    ["Jon Smith~0.1"    , "Jon~ Smith~0.1"     ],
    ["Jon AND Bob"      , "Jon~ AND Bob~"      ],
    ["Jon AND Bob~0.1"  , "Jon~ AND Bob~0.1"   ],
    ["Steve^9 Jon"      , "Steve^9 Jon~"       ]         
  ]; 

foreach(var item in equals){
Assert.Equal(Sut.Process(item.Key), item.Value)
}

Or perhaps you should simply write out your assertions/tests:

Assert.Equal(Sut.Process("name:(John Smith)"), "name:(John~ Smith~)")
Assert.Equal(Sut.Process("name:Jon~0.1"), "name:Jon~0.1")
etc

If you feel you should use a list, use a structured type as opposed to an array, as although it's more typing, it's easier to read:

    var params =
    [{
         Input: "name:(John Smith)",
         Output: "name:(John~ Smith~)"
    },{
         Input: "name:Jon~0.1",
         Output: "name:Jon~0.1"
    }]

for(var expect in params){
    Assert.Equal(Sut.Process(expect.Input), expect.Output)
}

There is no advantage to formatting your data the way that you do UNLESS you are using an editor that manages this for you. The readability is not greatly enhanced, (considering if I have lower screen resolution than you I may not even see it) and the work to maintain such formatting far outweighs the benefits of doing so. Agreed, tests should be as readable as possible, which is why an array is probably not the best option for writing this data in the first place.

share|improve this answer

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.