Home > General Software Development, Refactoring, Software Development, TDD > Please question the need for whitespace

Please question the need for whitespace

I have blogged about this before but I think it is a common problem that is worth restating since it affect developers across our industry.  I noticed the following method recently and again the curious separation of sections by whitespace popped into my head:

   1:  private void CalcHeaderOffsets()
   2:  {
   3:      this.fs = new FileStream( assemblyPath, FileMode.Open, FileAccess.Read );
   4:      this.rdr = new BinaryReader( fs );
   5:      dos_magic = rdr.ReadUInt16();
   6:      if ( dos_magic == 0x5a4d )
   7:      {
   8:          fs.Position = 0x3c;
   9:          peHeader = rdr.ReadUInt32();
  10:          fileHeader = peHeader + 4;
  11:          optionalHeader = fileHeader + 20;
  12:          dataDirectory = optionalHeader + 96;
  13:          // dotNetDirectoryEntry = dataDirectory + 14 * 8;
  14:   
  15:          fs.Position = peHeader;
  16:          pe_signature = rdr.ReadUInt32();
  17:          rdr.ReadUInt16(); // machine
  18:          numDataSections = rdr.ReadUInt16();
  19:          fs.Position += 12;
  20:          optionalHeaderSize = rdr.ReadUInt16();
  21:          dataSections = optionalHeader + optionalHeaderSize;
  22:   
  23:          sections = new Section[numDataSections];
  24:          fs.Position = dataSections;
  25:          for( int i = 0; i < numDataSections; i++ )
  26:          {
  27:              fs.Position += 8;
  28:              sections[i].virtualSize = rdr.ReadUInt32();
  29:              sections[i].virtualAddress = rdr.ReadUInt32();
  30:              uint rawDataSize = rdr.ReadUInt32();
  31:              sections[i].fileOffset = rdr.ReadUInt32();
  32:              if ( sections[i].virtualSize == 0 )
  33:                  sections[i].virtualSize = rawDataSize;
  34:   
  35:              fs.Position += 16;
  36:          }
  37:      }
  38:  }

.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: consolas, “Courier New”, courier, monospace;
background-color: #ffffff;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #006080; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }

What is the purpose of the new lines at 14, 22 and 34?  Are these separate blocks of logic?  Could they be better represented using “Extract Method“?

One of the first things that I do with code when refactoring is remove all the line breaks and make sure I am comfortable with the logical flow of the code.  If lines don’t logically flow together then I ask myself if the two pieces belong next to each other or if they should be different responsibilities in different methods.

Ask yourself next time you leave a new line whether it is really necessary or is it a codesmell indicating poorly separated unrelated functionality?

FYI:  The code above is taken from NUnit (AssemblyReader.cs) and is free software licensed under the NUnit license (Copyright 2007, Charlie Poole, http://nunit.org/?p=license&r=2.4).

 

We are hiring!  Do you want to write beautiful code in a Test Driven, Refactored, Agile .NET software company in the heart of Washington DC and work on cool products
Take the code test and send your resume along with why you want to join Thycotic to
tddjobs@thycotic.com.

  1. Jens
    March 18, 2008 at 7:24 am

    adding empty lines is like using commas and periods in your sentences if you don’t the text will be much harder to read readability isn’t improved by extracting every 5-10 lines into methods with parameters going back and forth

  2. http://
    March 18, 2008 at 8:45 am

    You should compare it with reading a book. A book without paragraphs reads badly and you will most likely to miss a lot of things. However a badly paragraphed book, isn’t good either as you will loose the connection between lines and it won’t get clearer either.

    White spaces are a good thing, if used properly. And adding a whitespace is definatly NOT just a sign that you could extract a new method from it.

  3. http://
    March 18, 2008 at 8:00 pm

    I agree with the above comments. It makes the code readable.

    I often add empty lines in logical places whenever I have to change some code I didn’t write myself.

    It always easier to read code you’ve written yourself… think about your coworkers.

  4. Ken Cox [MVP]
    March 18, 2008 at 9:16 pm

    I think whitespace is essential to make code readable. Leaving space helps the cognitive process because the mind can easily form groups of code chunks and recognize sections at a glance.

    It’s basic Gestalt theory at work.

  5. http://
    March 19, 2008 at 1:13 pm

    Sometimes extracting a method does not make things more clear because the method needs to have lots of parameters and returns.

    Often, the human who is writing the code feels that adding a little space makes it easier for him/her or the next human to understand the code. The machine has no problem ignoring the space.

  6. John
    March 20, 2008 at 2:37 am

    Well – I don’t agree with the thought of removing all white space.

    It does not get in the way of the compile process and it does not necessarily designate a break in code flow. In the code example you show – I am not sure I would have the line breaks where they are – but I sure would not remove them willy-nilly.

    I teach my students to use white space consistently and where it will help to organize. I could be wrong, but I think this is the point you were trying to make.

    I write code for humans and giving the eye a break as you scan enables you to focus on successive chunks. Writing code is about organizing and certainly breaking code out into other methods is one possible way to organize – but not always.

    A good example I can pass along from my own daily work – I add a break every 5 lines when I am setting a significant number of properties or parameters. If you have data tables with 30, 50, 100+ columns in it and you are performing insert/update – setting those values in scrunched up blocks of text – 30, 50, 100+ straight lines of code just causes the eyes to go cross-eyed as you are scanning through.

    I have learned from designers who work in a variety of media (e.g. print, web, film, etc) – and from that I have gained valuable knowledge in what makes for easy reading. White space plays a significant role in that.

    I can tell you from reviewing a lot of student code and those of my co-workers… Code written for humans is easy on the eyes. Being able to view in chunks is faster to scan, and comprehend, and enables me (at least) to spot inconsistencies.

    Use it wisely.

  7. thycotic
    March 21, 2008 at 4:39 am

    Thanks everyone for your feedback. It has been an interesting discussion.

    I still think you will find less dependence on whitespace if you make your methods shorter and about one thing. :)

  8. http://
    March 23, 2008 at 4:08 pm

    Interesting how a single exclamation point throws you off in this post “What makes some code confusing?” but having all of your code compressed into having no whitespace is easier for you?? That doesn’t make much sense. White space != code smell. Extract method != prettier code.

  9. thycotic
    March 23, 2008 at 4:40 pm

    @Snacks: Your comment is somewhat of an oversimplification and doesn’t really add much else to comment on. I would encourage you to discuss and explain rather than simply make statements.

  10. LudovicoVan
    March 27, 2008 at 7:04 pm

    Jens:
    > adding empty lines is like using commas and periods in your sentences if you don’t the text will be much harder to read readability isn’t improved by extracting every 5-10 lines into methods with parameters going back and forth
    I second this and think nothing more there is to add on the matter other than even more explicit critics.
    READABILITY is what we need and the number 1 key factor in any decent SE book. The rest is just misguiding and misinformation, including all the buzz on that logical nonsense called “refactoring” and the whole Agility bandwagon.
    Those guys are simply reinventing the wheel, and they are reinventing it wrong, because they didn’t bother to check what had already been done. They have their “time to market” before all.
    -LV

  11. LudovicoVan
    March 28, 2008 at 10:33 am

    Well, thank you very much for not publishing my comment, I see you really are after genuine questioning,
    open discussion and frank explanation.

    Keep up the good work!

    -LV

  12. thycotic
    March 28, 2008 at 10:53 am

    @LudovicoVan: The only reason your comment wasn’t originally published was that CommunityServer tagged it as “Possible Spam”. I don’t censor opinions, however I don’t think your comment contributed much.

    Bashing refactoring without any real explanation doesn’t come off as enlightened.

    The number #1 goal of refactoring *is* “READABILITY” as you put it.

    According to Bob Martin, code should do 3 things:
    * should work (hopefully this means that your unit tests pass)
    * should be easy to change
    * should be easy to read and communicates its purpose clearly

  13. LudovicoVan
    March 28, 2008 at 3:13 pm

    Hi back Jonathan,

    First thanks for salvaging it, this makes me wander how the CS spam filters are coded.

    Anyways, as to the primality of Readability, even with respect to Maintainability, I have recently written something on my blog. It’s not a full SE book there, so not all connections are made explicit, but it should at least make quite clear in which sense (IMO, but I’ll dare say in any respectable SE treatise) Readability is No 1 and the rest is simply a consequence. There you’ll indeed find hints as to how (again, IMO) Refactoring is a dead end, along with the whole general concept of Optimization (at least when taken naively): http://architectando.blogspot.com/2008/03/on-readability-pure-induction-in-action.html

    As to the relevance of tests to the concept of “working software”, I do strongly agree on that and I have indeed something about the concept of Correctness in engineering in another blog post, where you might find that after all our positions are not that far: http://architectando.blogspot.com/2008/02/on-correctness.html

    This all said, about Mr Martin’s, I would just shrink his list to the first and last point from the list you show above, and then our views would be in perfect accordance.

    You are right in that “positions” here should be reasonably justified, although please let’s take also into account the fact the it’s not that easy to write a book within this very small space…

    In any case, just thanks for letting it happen.

    -LV

  1. No trackbacks yet.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: