[pietomb00]

Hi All,

I'm sure none of you ever do write untidy code But when you come across some that you've got to work with that's completely unstructured and hard to understand where do you start with tidying it up?

Do you have a process that you follow, or just dive in? Or even do you re-write it if you have time?

I've got some hideous code that needs altering, but there's really no logic to it, almost all of it is in a few methods split between 3 classes, can't see any reason for where each bit has been put. In general where would you start?

[modi123_1]

I may if I have time.. but frankly any code written by you later than six months ago is darn near indistinguishable from the piles of crud your coworkers write. It might as well have been written by some barely upright ape in the savannah. It typically gets flagged as "when you are bored and have an afternoon free".

[tlhIn`toq]

It depends on what you mean by 'untidy'. Sloppy logic, or just having methods and properties scattered all over?

ReSharper has a really nice code clean-up feature that moves all the same types together, surrounds with #regions etc. There are other code cleaning plug-ins in the Visual Studio gallery.



But nothing beats sticking to a good naming convention in my opinion. I like the 'military style' of naming things by most significant element first.

All the buttons will group together alphabetically if you name them with this pattern

• btnSave
  
• btnLoad
  
• btnExit

Where they are scattered if you do this:

• SaveButton
  
• LoadButton
  
• ExitButton

Same with methods:

• PreferencesLoad()
  
• PreferencesSaveXML()
  
• PreferencesSaveDB()
  
• PreferencesGetDefault()
  
• PreferencesChanged()

Puts everything related to preferences together.

[Skydiver]

And if you really need to change the code, having a good set of automated unit tests will be invaluable. Yes, it may seem like you are taking extra time to write tests instead of jumping right in to do the refactoring/code re-organization, but you are saving yourself time long term when something does start misbehaving and you have no idea what changes in your series of changes may have caused it. With your set of unit tests, you'll have the confidence to make a code change and be assured that it still behaves the same way.

I used to be in the camp that used to believe writing the unit tests was a waste of time since you're going to do acceptance and regression testing anyways, right? After trying things with unit tests being written first, I found that it actually accelerated my development time. Taking time to write the tests let me know what were the expected inputs and outputs, understand how it works, and know the corner cases. So when it was actually time to make the code change, it was a lot easier. It also gave me a lot more freedom to do more radical surgery on the code knowing that there's a safety net that will tell me within seconds whether I screwed up or not.

[cfoley]

There are lots of ways of cleaning up code. Refactoring is a rigerous one. First you make sure that the code you are restructuring works. Do that with some unit tests. With any luck, there will already be a comprehensive test suite but in the real world, you often have to top it up. Then you change the code using baby steps. Run your test suite after each change to make sure you haven't broken anything. If you get tests failing, roll back and figure out what went wrong.

For more information, google refactoring and code smells.

[pietomb00]

I think I'll have to take a look at refactoring, I've got a couple of unit tests but no doubt they're not sufficient.

[cfoley]

Martin Fowler's book is really good.

[Lemur]

Never dive in without a plan of action. 2 hours of planning is 20 hours saved in code hell.

Make sure to write out everything. Write out the structure of the code, including the methods and what they do in plain english. A sketch book, a ruler, and a good pencil are invaluable when planning a method of attack.

[jon.kiparsky]

tlhIn`toq, on 10 August 2012 - 11:25 AM, said:

All the buttons will group together alphabetically if you name them with this pattern

• btnSave
  
• btnLoad
  
• btnExit

Except Hungarian notation defeats the purpose of variable names, because it makes the names themselves more difficult to read.
If you find it hard to keep track of your buttons, then the better approach would be to put your buttons into a map and do something like

ButtonMap.get("save");

if you ever really need to access a particular button. But once you've defined a particular button you shouldn't ever need to know its name, so you could even just add it to the GUI and leave it in most cases.

[cfoley]

I agree with you about the Hungarian notation but I don't think that accessing your variables via a map is the answer. The biggest problem is that if you make a typo (or even use a refactoring tool to change the variable's name), you are swapping compile time errors for runtime errors. I know when I would prefer to find out about my mistakes!

As an aside, Hungarian notation has a bad reputation because people abused it by labelling the variable names with the data type. When used properly, it can be a really useful tool. Good uses include units for physical quantities (e.g. temp_c and temp_f when your application needs to switch between the two) and frames of reference (e.g. cursorY_screen and cursorY_document) when you have scrolling.

[Skydiver]

When Hungarian notation was invented, its purpose was to denote the datatype of variables.

Quote

Quantities are named by their type possibly followed by a qualifier.

From the inventor himself: http://msdn.microsof...6(v=vs.60).aspx

How can you say that it is abusing the usage of Hungarian notation?

So there are two flavors of Hungarian: Windows Hungarian and Office Hungarian. Windows Hungarian is very strict about the type prefix matching the memory representation. Office Hungarian is looser and would prefer that prefix denote the "intent" or "context" of the data. So with your example, of the temperatures, Windows Hungarian would have something like floatTempC, floatTempF, while Office Hungarian would have something closer to what you had: tempC, tempF.

Why divergence within even a single organization? I don't really know, but I can make a few guesses. For Office, code being written is more domain driven. You don't really need to know the underlying type, but you do need to know what operations can be done on in. For Windows, when you are in the middle of a debugging session with kd or windbg, you'd like to know right away whether that variable is a word, a dword, a character, or an array of something so that you can do a memory dump and interpret the bytes correctly. More mechanics of the underlying data, and less domain context.

Personally, I prefer Office Hungarian rather than Windows Hungarian, except for the case of structures that get read/written directly from/to disk or the wire. In those cases I want to know what the underlying type is, and not be guessing whether offsetData is a WORD, DWORD or a __int64.

This post has been edited by Skydiver: 16 August 2012 - 12:38 PM

[cfoley]

Thanks for the link. It was very interesting and informative. The history I had previously read claimed that Hungarian notation was invented while writing office, and that all the inclusion of data types in names was a misguided application. It looks like that's not quite accurate. Thanks again!