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()
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 )
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;
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;
23: sections = new Section[numDataSections];
24: fs.Position = dataSections;
25: for( int i = 0; i < numDataSections; i++ )
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;
35: fs.Position += 16;
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 firstname.lastname@example.org.