What Makes Code Unbearable to Work With

Technorati Tags: Standards

I liked this post by Jeremy D. Miller about what really makes our jobs tough because of others or departments who don’t care to have any sort of process, standards, or feature priority. If only other developers would take the time to care and really work towards coding good quality code, the entire team becomes 5x more productive. This includes telling the business NO at times or slowing down the business (in the case of code & run shops) so that the quality of code and just sanity of work and ability to think while you code, debug errors and TEST!!! is possible so that the entire team and the business works like a well oiled machine, not something that’s hacked which causes a lot of pain. Jeremy points out a list which I know from experience is a direct relation and result to Code & Run shops:

What makes a system hard to work with?

My own list of unacceptable team “Sins”:

Inline Styles – for God sakes, we’re in 2009 people. This is one of the most annoying and inefficient things you can do in a UI. Who wants to update inline styles all over the place. It makes debugging unbearable as well with CSS. Use of this rather than learning and using CSS classes and ids for EVERYTHING that you do to style your UI is just plain laziness. I can’t tolerate it and most good teams I’ve worked on in my career also cannot and will not tolerate it. And if you resist learning tableless design, I feel sorry for you. I’ve found it to now be easier than using tables to control layout which restrict the hell out of me and make my mark-up a complete pile and footprint on the web much bigger for my website. I’m sorry but the argument that “tables are easier” no longer applies to me personally after starting to get real comfortable with divs and other elements lately. Make yourself get into tableless design. You’ll find out in about a month that it’s not that bad after you struggle a bit just like anything else!

No Business Layer (physical and non-physical). Don’t have a BL project? Shame on you…not good. Don’t have BL classes related to entities? This is also not acceptable. Putting all your basically business layer type methods into one class which basically serves as your BL, again is not acceptable. There are about 100 obvious reasons why this is not ok to skip having a BL in your applications…makes life for all developers using it hell. In fact I just cannot stand it when I don’t see a BL in mid-to-large applications. It just boggles my mind. It also renders your application non-unit testable. I don’t have to go on to explain this, it’s obvious. If you’re too lazy to stub one out, not cool. The mindset should not be “does this application really need that much separation”. The mindset should be SOLID and “I don’t care if it doesn’t need it now, but will it in the future? We don’t know but at least create the separation for extensibility regardless”.

No XML or C# comments – again, laziness. And I don’t buy into “the business wanted it now, so I did not have time”. That’s bogus. Tell the business tough, I need some time to code well and test my damn code. The business CAN wait. If you think that you have to run to every person who calls up and wants something NOW, then you have definitely not had experience in other environments which would look at this as complete chaos and completely inefficient and ridiculous. The business should not drive stupidity and hacky code. That’s not what they should be paying you to do for them with these unrealistic deadlines/expectations.

No Source Control at All – that’s self explanatory. It’s a HUGE burden to the development environment for a ton of reasons and it does NOT matter how big the team is.

No Source Control Check-in Commenting or Sufficient Commenting –and when I say non-sufficient, I’m talking about comments like “updated page”, “changed x method”. Be thorough in those comments. Nobody wants to look at every piece of code you updated in order to “see what changed”. I hear developers give this excuse when not putting in comments saying “just look at the diff file”. Hey, guess what, other developers do not have the time to review your entire change set idiot. So put in a comment that explains the change set and a decent summary (3-4 lines or whatever) to allow someone glancing at it to get a little more than “added a feature”. Give 3-4 sentences per check-in. Trust me, it’s not that much time consuming and it’s a disrespect to others on the team. You’re not the only one using that codebase and others need to know quickly what each change set represents when looking at history to decide which change sets may pertain to them or affect them

Not Checking Code Into Source Control Often – who wants to merge a ton of sh%% later 2-3 weeks or a month from now when you are just about to deploy to QA. The entire point of source control is to get latest and to work against that codebase. If fellow developers are not checking in often, what’s the point of using source control then. You should not worry about your code being perfect. That’s not what source control is about. Sure, make sure it’s not going to break when others compile your code and make sure you’re not introducing show stopping errors. But otherwise stop freaking out and just check in often. It’s not that bad really.

Developers Constantly Checking in Breaking Code – if you do not have continuous integration using Cruise Control, you need to start NOW for the benefit of you and the entire team.

These are not “picky”. These are effective requirements for a development team to function efficiently which correlates to developers enjoying what they do, and developers able to meet deadlines and to produce code that is reliable for the business later on because the process has standards defined and expectations defined for the team and system as a whole.

It’s called SANITY, instead of Code & Run CHOAS. It saves a huge amount of time for all if these standards/expectations/practices are in place. All the points above together adds value to the team, the code base, and maintenance thereof. I can say these things from experience being in many environments, so this is not just rambling or rants here. It’s common sense and it’s called having good teamwork, a good process, and good standards in any development environment tiny or big…doesn’t matter.

Print | posted on Thursday, March 12, 2009 9:51 PM

Comments on this post

# re: What Makes Code Unbearable to Work With

I can buy most of those, but the no comments one i do not agree with. To me excessive comments are a code smell. There was once a time where I thought EVERY method needed header comments (xml ones) that fully described the intent of the method.

Then as i started to write smaller, more tightly defined methods that did ONLY one thing (SRP) I quickly realized that i was simply restating the the code in 'english' text. If you cannot read the code to understand the intent then shame on you.

Also, if my method names are not descriptive then shame on me, but comments are not the answer.

I will stop short of saying comments are bad in all cases. Comments should be used if the intent of the code cannot be clearly conveyed by simply reading it. Or if there is some sort of 'hack' that is in there and you want/need to explain why it is a hack.

Left by Derik Whittaker on Mar 13, 2009 7:01 AM

# re: What Makes Code Unbearable to Work With

@Derik

Notice I stated "NO" comments. I did not say that every method has to be commented. Obviously some methods clearly speak for themselves. However, let me also say that for XmL documentation, it's nice to get a list of documented methods, even for the inferred...so you can see a complete listing of all method information using XML. However, I would not require all methods to be documented if they are very small and don't do much and the name is very clear. But...even if the name is very clear, a method could still be about 10 lines and not only just what the thing does, but also there could be other intent that the developer needed to add to its description other than just what goes in and what comes out. In those cases, even if the method name is clear and the method is only 3-4 lines of code, a developer may desire to put in an XML comment for other reasons...maybe a good FYI or whatever the case may be. But again, I never advocated all methods to be commented. It's bad when only 10% are commented...or 1% or none! That's ridiculous. I personally do not comment the methods that if commented you're just spitting back the same thing (input and output). Yea, that would be lame. But I never advocated that.

I also did not say comments are the answer to badly named methods. The two go hand in hand. Good comments and good method names are compliments.

And I don't agree that comments should only be conveyed if the intent of the code is not clear. Again, don't decorate all methods but surely at least 70% should have them for XML Documentation especially if it's an API.

If there is some "hack", refactor it now.

I also don't like the XML Comments to be more than 2-3 lines. There are just some elements that you don't need that I agree do clutter the page.

Left by espresso on Mar 13, 2009 7:42 AM

# re: What Makes Code Unbearable to Work With

@Dave,

Yea i can see that we do not see eye to eye on this. I would NOT want to see 70% of my methods commented, maybe 10-15%. Also, 'Good comments and good method names are compliments' is the wrong idea. If the method name is clear then u do not need to comment it.

Lets just agree to disagree (or agree that I am right and you are wrong :) )

Left by Derik Whittaker on Mar 14, 2009 8:28 PM

# re: What Makes Code Unbearable to Work With

Ok, fine. I agree 70% is a lot and I retract that. But 10% is not enough. And I would like to see an example of methods you think need comments. Again, even some method names may be obvious. And for the most part they probably are and should not be commented. However again:

1) XML comments give you a nice comment file to work with for your entire API
2) Even if a method name is obvious, there may be more to it than just the name....context may play into it.

If you are going to only comment of 10-15% of an entire application, then you better be adding 10-15% inline C# comments as well as a whole to make things quickly clear again, where it makes sense in methods and classes. Nobody wants to step through your entire code. Granted there are many times we have to obviously to see what's going on but sometimes the intent and even syntax may not be clear to some developers in your group especially if it's hacky or more complex for some of your Jr. Developers. You can't assume that what you know is inferred to all else. Lets say for example you created a lot of generic classes that inherited multiple interfaces. You better be putting inline comments and better be putting in some sort of comments on your methods if you have a lot of generics and Interfaces going on.

Also, do you realized even good method names and clear code can't always tell you the "business terms" of what's going on? A developer who's looking at a class or classes for the first time, may or may not get how it relates to the business and you know what, comments are good for the team.

Again, nobody wants to look at ALL of your code and step through every method EVERY TIME. In fact there are a lot of times I'm just checking out some classes to see what they do. Sure, I can tell mostly just by looking at the code somewhat briefly but method names are not a substitute for a reasonable amount of code commenting. Again keep it short, simple, and put the comments in the right places. But surely 10-15%...if you are only commenting to that level, it's not enough.

And if you keep your comment template very simple (require only 2-3 lines...method summary, inputs/outputs) then your classes are not going to be so cluttered like you are telling me. Because if your classes are that long, then maybe the class is designed poorly and so you can make that case as well.

And one more time, I talked about NO comments. I retract my statement about 70%. Lets focus on what I stated in the post. I said NO COMMENTS is bad. And yes, I think 10% sucks (both XML AND inline C#) as a whole in any application).

Use of:

1) C# Regions
(using them smartly...having a good standard across the team, and clean logical and general section use of regions, not overdone)
2) 30-40% of inline and XML comments
3) Method names that make sense

all contribute to a more readable code base outside of the times you are debugging

And if you're going to be providing an API to be consumed by the outside, you should have comments on ALL methods for that API for documentation purposes so that you can provide your customers some good MSDN style help section for your API.

>>>Also, 'Good comments and good method names are compliments' is the wrong idea. If the method name is clear then u do not need to comment it.

Please tell me what you thought I was saying...because I guess you did not understand. What I mean is, they compliment indirectly as both good standards to have and when you have both, the code is must easier to read. I do not mean you must have good comments on method names that are obvious. I am saying if you have comments and good methods names, together (whether the same method or not) they are good things to have.

Left by Dave Schinkel on Mar 14, 2009 9:15 PM

# re: What Makes Code Unbearable to Work With

Gentlemen,

Although, you both have some valid points here is my perspective in regards to code comments. I think that regardless of the comments, code should be self describing and have meaningful names, not use of short lazy abbreviations (using o instead of order for an Order class). Intellisense and Resharper save typing, no excuses. It is given that complex code should be commented. And if you have very complex code, that is usually a red flag that it needs to be refactored. I am a firm believer that a picture is worth a thousand words, especially in the context of software design. Class diagrams and package diagrams illustrate the logical static structure of a system and are good for understanding the various layers from a high level. The dynamic aspects of a system are represented by sequence and activity diagrams. Sequence diagrams provide a structured representation of behavior as a series of sequential steps over time. Activity diagrams are used to model the behaviors of a system, and the way in which these behaviors are related in an overall flow of the system. These are great for more complex business rules. Of course the challenge is to keep the code and models in synch and linked. In the past third party products like Enterprise Architect have provided these facilities. Slowly, Visual Studio has begun incorporating these features in, although somewhat limited as of yet. Visual Studio 2010 has the ability to auto generate sequence diagrams from your code. My point is there is a limit as to what too much commenting can provide. The effort is often better focused on a better overall design, with self-describing code, and architecture diagrams.

Left by MFS on Mar 16, 2009 8:33 AM