how to write less messy code

fguihen

Junior Contributor
Joined
Nov 10, 2003
Messages
248
Location
Eire
i have a habbit of writing code as i go along, and tackling problems as i meet them, but my code is a bit of a mess because of this, and difficult to read when you come back to it after a few weeks. does it make much of a diference to plan out your coding strategy first? the thing is that im not awfully expirienced, and to plan something , you need to know how it all works first and i learn as i go along. how do you all keep your code tidier?
 
When I am doing something I am not experianced with, I program as I go along, and my code is generally rather messy at first, but once I understand what I'm doing I go back and pretty up my functions. I also try to orginize my code in class modules. Sometimes a class isn't necesary and a regular module will work for a collection of unrelated functions or whathave you.

For example if I am writing two functions, I will write one and get it working. Then I will start on the second, and when I need a break from the second function I go back and pretty up my first function. If that makes sense.
 
Here are the two biggest things you can do to help (in my experience):
(1)Comment. Anything you think won't be obvious a week from now (to you, for the time being forget about other programmers) should be commented.
(2)Refactor. Later, go through and organize code more logically. Extract functions where you think it will be more readable, or inline them if you think it makes things simpler. Rename variables, put them into more appropriate scope, change functions to properties when it makes more sense, etc. And when you refactor, comment for yourself as well as anyone else who will see your code.

That is how I program and produce something readable. And by using that process, during the refactoring stage (where you depend on the comments you made earlier) you can notice trends in your programming that you need to refactor repeatedly, change your style, and hence improve your programming.
 
Generally, when entering unknown territory, prototyping is a good course to take. Any time "lost" doing a rewrite will be more than made up for supporting your organized code as opposed to your initial spaghetti.
 
I'd like to add to the comment tip of marble_eater.

Often you see that the comment describes what happens, often that isnt what you want (you can find out what happens by reading the code / debugging those few lines), a lot of time the interesting fact is WHY did you put that code there.

So for instace
Code:
' Increase the counter by one
counter = counter + 1

will be less than usefull as the code pretty much explains itself but

Code:
'Counter is 1 off because we're using custom 1 based collection instead of 0 base
counter = counter + 1

gives a lot more information about what is going on at that line of code.

The refactor / prototyping belong together: dont be afraid to re-write some pieces of code if they look awfull. It might cost you half a day to re-write it, but if you have to fix a problem in that code in about 2 months, it will cost a lot more.
 
Also regarding comments, often a lot of simple comments could be removed just by using sensible variable, control and method names. Many times in the past I've encountered code along the lines of
C#:
//Customer collection
System.Collections.ArrayList cl = new  ArrayList();
if a variable is going to hold a list of customers give it a name like, being radical here, CustomerList. This means the code is more readable rather than the comments being a required read, plus if you do modify the code you do not have to worry about the code / comments getting out of sync.

Sensible comments like those suggested by Wile however are a must - explain the 'why we do this', 'what the expected parameters are' and 'desired results' so you can read over code and identify what a function is / does etc without having to read each line.

Also it is very, very hard to produce good quality readable code while developing the code - refactoring as mentioned by marble_eater above is an essential skill; write code, check if it can be improved, tidied up or simplified and if it can do so before moving on.

Using some form of additional tools like FxCop to check naming conventions etc will help. Unit tests with NUnit or similar mean you produce code that passes a set of tests and when you refactor you can guarentee that you haven't introduced bugs by re-running your tests. I can definately recommend This book as a good starter on using unit tests and getting into the refactoring habbit while coding.
 
Probably the single best way to improve your code is to adhere to the DRY principle -- Don't Repeat Yourself. At the simplest level this means don't copy and paste code around. If you use code in more than one place, make a function for the code and call the function. The same goes for comments as Wile pointed out. Your comments shouldn't repeat what your code says -- if you're going to have comments, they should add something to help clarify.

At the biggest level the DRY principle applies to everything in your system. This is very well practiced in databases but isn't usually thought of in other areas -- normalization. Take the easy "person's age" example. Say you have a person object and you need to know how old the person is. It is better to store the person's birth date and then calculate the age rather than storing the just age or both the age and birth date. Storing the age doesn't make any sense because you now have an opportunity for your data to become old or out of sync with reality. By storing the birth date you can always determine the age and always be right. PLUS through the power of .Net, you can declare Age as a read only property and you'll never even worry whether age is derived or stored while you're coding. Same thing with something like IsFull -- don't store a boolean if your list is full, calculate it on the fly and hide the calculation in a read only property.

DRY: "Every piece of knowledge must have a single, unambiguous, authoritative representation within a system."

For more on this many other excellent practices, I highly recommend The Pragmatic Programmer: From Journeyman to Master. This is probably the single most influential software book I have ever read.
 
I agree that it's important to refactor. I'd also recommend this book.

If you have to comment code to make it readable, it's probably not written in the best way. Comments have problems in that they are difficult to maintain. If you need to fix a problem urgently at a later stage, you're unlikely to find the time to update the comments as well.

If you're using NUnit or similar, as PlausiblyDamp suggests, the unit tests are the best place for comments, and can act as comments themselves as they explain what the code should do. They will break if they are not kept up to date.
 
The funny thing about unit testing -- unit tests are still code. So, the advice for how to write less messy code is to write more code...I just thought it was kind of funny, that's all.
 
A lot can be gained by designing the system first instead of rushing to code. That is probably one of the biggest falicies out there. If you design your system first, in say UML and using good requirement managment techniques, then by the time you get to doing code, coding become more of a dictation of turning requirements documents and pictures (diagrams) into words and requires very little effort. Obviously this is overkill for a quick throw away app, but for anything serious it should be done, and for coding itself, the techniques mentioned above are very helpful (along with updating comments when the code changes - that gets missed a lot), but my point is, that if you design your system first, you won't have to make a lot of changes.
 
I find it best to write code from top to bottom, instead of the way most coders do, which is bottom to top. This means instead of writing functional code as step 1, step 2, etc I design my application via a series of empty verbose & well commented methods.

Say I'm writing a program for users on a network. At first I design a UI in the designer to think about how a program works, plus it gives you something quick to show people. After I feel the UI is well designed and possibly set in stone, I jump to the code and the first kind of stuff I write would be...

PopulateFormWithDefaults();

I create that method and it would contain something like...

GetAppSettings();
PopulateUsername();
PopulateState();
...

I then create empty methods for all of these. Keep in mind I write an XML comment for each method that explains what it does. Admit to yourself that if you don't write comments as you write code (even an empty method is code) then you never will get around to it.

I usually keep going on with this until I get to the smallest parts of my app, often as small as trivial methods like IsNumeric() or whatever. From there I begin coding whatever method grabs my attention first but usually I code it with the top-most methods. This way my application begins to have some functionality initially, all the time commenting every line of code (I'm a firm believer that if every line of code was removed, my app could be totally rewritten as is from comments alone).

By creating apps this way, you begin to look at your app from a functionality standpoint, which I feel is very OOP like. It helps prevent you from writing long chunks of code as every aspect of the app is broken into it's own method. I find this format as a good way at preventing sloppy code.

I learned this method from a professional programmer who now works at Microsoft. Considering he's been coder much longer than myself, and the companies he has worked for, and now works at, tells me it's a very good methodology.
 
I agree with travisowens's approach but I'd like to add a step.

If you were going to write unit tests (which you should no matter how much fun I poked at the advice in its current context), the best time to do it would be right after you finish writing your XML comments and before you do anything else. At that point you have a fully specified method and, assuming you've written extensive enough tests (hopefully based on the english description written in the XML comments), you will know when you've got the method implimented correctly. Test Driven Development :cool:. After you have your unit tests set, you can make all the changes to make your code less messy as you want and you'll always know that you have a working system -- or at least if something went wrong.

The big trick that I found laughable earlier is that Unit Tests won't make your code less messy by themselves AND you can still write unit tests that are messy code. To write less messy code I stand by my advice earlier -- DRY Principle on all levels of your system. I also thought that readability was good advice as was thinking everything through (planning) which a few people said earlier too.
 
If you are going the Unit Tests route (and you really should IMNSHO) then as mskeel says treat the test code as if it was part of the source code and apply the same principles to it as you do to the main 'trunk' of the application. Having the tests means you can reorganise and tidy your code knowing that you aren't making it any worse functionally while making it tidier / more readable etc. having this knowledge will give you more confidence in actually performing the refactoring itself, this is only of any real benefit if you can read, understand and maintain the tests themselves though.
Get in the habit of doing this as you go i.e. code something and then when you are sure it works rather than move on to something else examine what you have done (both application code and test code) : Is it readable? Is it clear what the code is doing? Do the variable / method / parameter / class names give a clear indication of their meaning? Are the comments sufficient / clear / accurate / needed? If any of these are not a definite yes then fix things now rather than letting poor quality code get embedded into the system. Then retest.

Similar to the DRY principle mskeel mentions a lot of TDD developers go by the term OAOO (Once And Only Once), however that is generally the 2nd most important rule to apply to your code - the 1st being 'Does the code clearly express it's intent'. i.e. If duplication of a piece of code makes the code clearer to read then the duplication is fine, if the duplication doesn't add to the programmers understanding then it needs to be removed; a less efficient (but adequately performing algorithm) may be better than a more performant but less understandable one depending on the situation.
Do this as part of your routine - when you check your code like I suggest above also check for duplications etc and if you can identify methods / interfaces / base classes that could be defined to simplify the code even more do it now. Then retest.

Also the idea of keeping things simple i.e. only solve the problem in front of you - do not create massively complex architectures 'just in case' they are needed later; if they are needed later then code them later - if they are never needed think of the time saved. This method also means because you never make any great leaps of faith in code changes you are going from a working state to another working state without ever causing major problems.

One of the biggest causes of poor quality code is that once written code is rarely maintained unless things go wrong - then it is normally a patch to the code (often of lower quality than the original code) rather than a proper revision. Routine maintenance of any other system (production lines, electrical installations, cars etc.) is taken for granted - why should source code be any different?
 
Last edited:
I agree, production code should be maintained, but I think the reason it's taken for granted is because 90-99% of the time if 'code' fails, nobody dies; however, if a car fails people could be injured.
 
Nate Bross said:
I agree, production code should be maintained, but I think the reason it's taken for granted is because 90-99% of the time if 'code' fails, nobody dies; however, if a car fails people could be injured.
If it's important it will get done otherwise it won't. It's up to you to make quality a priority. No one should live in a house with broken windows.
 
Ha! Good question, Cags... My opinion is no becuase if you say that cracked windows are ok then pretty soon you'll have a lot of cracked windows (that's just the way people are) and then you'll end up with a very fragile system where once the windows start to break you'll be in a heap of trouble.

The whole goal is to avoid the situation where one more crack or break doesn't really mean anything. If everything is pristine, and you even scratch something...well, you just don't do that sort of thing becuase everything looks so good -- you don't want to be that guy. You know That Guy; he's the one that writes all the ugly, not reliable, unmaintainable code. And nobody likes working with That Guy.
 
1 - Add comments. (Large ones if you need to).
2 - Use #Regions in every function.
3 - Don't be afraid to use long object names.
4 - Identify the type of variables your declaring.

On my current home project (a RPG maker, which most likely I will never finish it as I keep adding new fesatures to it, over and over) have a structure like this.

[1]
Add comments to every thing
Code:
public class c_DirectX
{
	public static Device dxDevice1 = null; //Draws on pnl_map
	public static Device dxDevice2 = null; //Draws on pnl_selected	
        public static Device dxDevice3 = null; //Draws on pnl_graphics

	public static Texture texTemp; //Stores temporarely textures

	public static string sLayerText = "BOTTOM LAYER"; //Default layer text to be shown
	public static object oColorLayerText = Color.FromArgb(0,255,0); //Default text color
	public static object oColorLayerBackground = Color.Black; //Fixed color of the text shadow
 
	public const int ScreenWidth = 1024; //Default Horizontal screen size
	public const int ScreenHeight = 768; //Default Vertical screen size

	public static int iGMapXPos = 0; //Stores the X pos of the current selected tile in the Graphics panel
	public static int iGMapYPos = 0; //Stores the y pos of the current selected tile in the Graphics panel
	public static string[,] sArrGraphicsPanel = new string[1000,1000];

	//private Texture spriteTexture;
	public static Texture texDX2;
	public static Texture[] texDeleteTile = new Texture[5]; //Stores cursor's delete pictures
	public static Texture[] texRegTile = new Texture[5]; //Stores cursor's registering pictures

[2]
Add regions to your functions

Regions.jpg


[3]
Everyone keeps telling me to use short names for objects, but I don't. I totaly refuse to apply such method. I use large names instead. Most of the times I don't actually need to comment what I'm doing since the code itself is way too much explanatory. I rather take a few more seconds to write bigger names and have my code more explicit, then code faster and loosing plenty of time to figure out what was I trying to do weeks ago when I last touched the code.

[4]
This is what I do to identify the variables I declare;

use string sName = "";
then string name = "";

use int iJoinDate = 0;
then int joinDate = 0;

use bool bContinue = false;
then bool continue = false;

use double dTotal = 0.0;
then double total = 0.0;

use string sArrMap[,] = new string[100,100]
then string map[,] = new string[100,100]

etc...

I hope this helps
 
Last edited:
First of all, experiment and find what works for you. Secondly, my comments on EFileTahi's preferences. Since that is what works for him, by all means he should keep doing that, but there are some points you should consider.

[1]Commenting everything: Many people would argue that too many comments are a bad thing (though not as bad as no comments). It takes focus away from what absolutely must be commented. Use descriptive names and code so that intent is usually apparent. Keep Plausibly's and mskeel's suggestions in mind: comment on what code does (or better yet, why it does it) only when it isn't obvious from its surrounding context. Maybe comment everything for starters and when you go back later, you will discover which comments you find necessary, and which ones you don't.

[2]Regions on functions: Functions are collapsible anyways. My suggestion would be to add a descriptive comment for each function and collapse the function.

[3]Long object names: Do what works for you. If short names adequately describe a variables purpose to you, use a short name. If a long name is necessary, by all means use a long name. (I keep my names shorter because I find that I have an easy time remembering what variables do, and long names clutter my code and hurt my head. That's just me.)

[4]Prefix Variables: Many poeple argue that Hungarian notation is a bad thing for a few reasons. Yes, it helps identify what the type of the variable is, but it can also take from the readability. This might apply more to VB, where you often get statements that are almost gramatically correct English. Also, people who don't like Hungarian notation argue that the type should be implied by the purpose of the variable, which should be evident from its name, i.e. instead of bContinue, use ShouldContinue, or WillContinue, or CanContinue. It not only implies that Continue is a boolean (it wouldn't make sense to store WillContinue as a string, integer, etc.) , but also more accurately describes what that boolean value indicates. This doesn't mean that prefixing your variable is a bad thing. It just means that some poeple prefer a different coding style.
 
Some adicional notes on my post, as marble_eater made me see some poinst of mine were not too much explicit.

[1] - My bad, when I said comment everything I was not refering to comment every single line, but to keep on track what your code does. In other words comment everything you think might be confused for later paused re-visits.

[2] - Keep in mind that collapsing funtions will be expanded next time you load your .NET application (at least in v1.1). This, (at least for me) is STRONGLY annoying to I see loads of functions/lines of code messing around with my eyes when I'm scrolling down through the code to focus on a X funtion. Like not Regions .NET will keep them collpased/expanded the way you let them by the time you exit .NET platafform. This is also great because you can easly know what was your last coding move by looking at the only expanded function. I always leave expanded the last function I was making/changing.

[3] - No adicional notes to add here :)

[4] - Just some side notes on this one. Continue variable could be controling a loop or if the player ran out of action points to continue with his turn. In other words, it could be bool or int type variable, bContinue or iContinue.
Alternatively you could say WillContinue and EndPlayerTurn, but then again, try to "guess" which type of variable are both WillContinue and EndPlayerTurn. Int or bool? I could say EndPlayerTurn = 1 or EndPlayerTurn = true. All in all I also could do the following: bWillContinue and iEndPlayerTurn, using both logic names and iditifying its types. :)
Just to finish, applying logic is slower then following rules. We immediately know that a bContinue variable is a bool by simply looking at the variable's starting letter, but, by looking at WillContinue I will have to understand the purpose of the funtion in which the variable is to know exactly what kind of value is need their. Of course this can be a vague explanation by having in count many other circustances/factors, but I think I got my point here.

Now let me reforce Marble's opinion on "do what suits best for you". Persons should try all the ideias/methods post here and see what suits best for them or simply make their own methods as I did. :)
 
Last edited:
Back
Top