The Artima Developer Community
Sponsored Link

Weblogs Forum
Why Your Code Sucks

21 replies on 2 pages. Most recent reply: May 17, 2012 1:07 PM by elaine morrison

Welcome Guest
  Sign In

Go back to the topic listing  Back to Topic List Click to reply to this topic  Reply to this Topic Click to search messages in this forum  Search Forum Click for a threaded view of the topic  Threaded View   
Previous Topic   Next Topic
Flat View: This topic has 21 replies on 2 pages [ 1 2 | » ]
Dave Astels

Posts: 32
Nickname: dastels
Registered: Mar, 2003

Why Your Code Sucks (View in Weblogs)
Posted: Sep 27, 2004 12:32 PM
Reply to this message Reply
Summary
If you are like most, and possibly even all, programmers (this author humblely included) then your code sucks. Maybe not all of it, and maybe not all the time, but certainly some of it, some of the time.
Advertisement

Your code sucks if it doesn't work.

Does your code work? How do you know? Can you prove it?

If you don't have tests for your code, it sucks. And I mean comprehensive, fine grained, programmer tests (aka something like unit tests), as well as higher level functional and integration tests. Tests that are automated. Tests that are run routinely. With the programmer tests run after any change to the code.

Without adequate tests (in terms of both quantity and quality) you simply can't have confidence that your codes works as required. And even if you are confident with your work... why should anyone else be? What about people who work on the code later? How do they know it still works after they make their changes?

Your code sucks if it isn't testable.

OK, so you decide to add tests to your code. That typically isn't so easy. The odds are that your code wasn't designed and/or implemented to be easily testable. That means you will have to make changes to it to make it testable... without breaking any of it (this is usually called refactoring these days). But how can you quickly and easily tell if you've broken anything without that comprehensive set of fine grained tests? Tricky.

It's even trickier than that. If you take the approach that you'll write tests for all your code after you write the code, you have to go to extra effort to try and design/write the code to be testable.

Maintaining an adequate level of testing when writing tests after the code being tested takes time and planning. And if you have to do additional work to make your code testable... well... that sucks.

Writing tests before you write the code means that the code is testable by definition. If you work one test at a time, you reap even more benefit, as each test only requires incremental changes to code that you know is solid.

Your code sucks if it's hard to read.

Code should be easy to read. Steve McConnell made the statement at his SD West '04 keynote that code should be convienent to read, not convienent to write.

There are several things involved here. Choosing good names that are self-explainatory is a good place to start. Strive for simple solutions, even if they are more verbose or inefficient. Whether inefficiency is a problem won't be known until profiling is done later in the project, in a real production situation.

Choose style and formating conventions early in the project, and conform to them. Require that everyone conform to them. This will be easier to do if everyone has a hand in deciding on the conventions and buys into them. This will make the code uniform and thus easier to read.

And get rid of any and all duplication. More on this later.

Your code sucks if it's not understandable.

This is closely related to the previous point. Code can be readable, but still not easily understood. Maybe it reads well but is misleading. Maybe you took the time to pick a good name for that variable, but the way it's used and what it means has changed, but the name hasn't. That's worse than using a cryptic name... at least in the latter case, the reader knows the name is meaningless. If the name is missleading, the reader can make ungrounded assumptions about what's happening in the code, leading to misunderstanding, and eventually bugs.

Your code sucks if it dogmatically conforms to a trendy framework at the cost of following good design/implimentation practices.

For example, Bob Martin recently raised the issue of dogmatically using private fields and getters/setters for a simple data structure (e.g. a DTO). If a field is transparently readable and writable why not simply make the field public? In most languages you can do that. Granted, in some you can't. For example, traditionally in Smalltalk all fields are private and all methods are public.

In general it's a good thing whenever you can throw out, or avoid writing, some code. Using a heavy framework generally requires that you must write a significant amount of code that has no business value.

There are a variety of lightweight frameworks for Java that are a response to the heavyweight frameworks (e.g. EJB) that have become matters of dogma lately. O'Reilly has a new book out on this topic, coauthored by Bruce Tate.

When making framework decisions, consider if a lighter framework will do the required job. Using something like Hibernate, Prevayler, Spring, PicoContainer, NakedObjects, etc. can be a real win in many situations. Never blindly adopt a heavy framework just because it's the current bandwagon. Likewise, don't blindly adopt a lightweight framework in defiance. Always give due consideration to your choices.

Your code sucks if it has duplication.

Duplication is probably the single, most important thing to banish from your code.

Duplication can take several forms:

textual
This is the simplest and often the easiest to find. This is when you have sections of code that are identical or nearly so. This is often a result of copy & paste programming. Several techniques can be used to eliminate it, including: extracting methods and reusing them, making a method into a template method, pulling it up into a superclass, and providing the variations in subclasses, and other basic refactorings. See Fowler's "Refactoring" for more detail.
functional
This is a special case of textual duplication and is present when you have multiple functions/methods that serve the same purspose. This is often seen in projects that are split up between groups that don't communicate enough. This could be a result of copy & paste, or it could be the result of ignorance of existing code or libraries. Practices and policies that encourage and promote knowledge of the entire codebase and all libraries and frameworks being used can help avoid functional duplication.
temporal
This is a bit odder, more obscure, and not as easy to find. You have temporal duplication when the same work is done repeatedly when it doesn't have to be. This isn't a problem in the same way as the previous two forms of duplication since it doesn't involve having extra code laying about. It can become a problem as the system scales and the repeated work starts taking too much time. Various caching techques can be used to address this problem.

Of the three types I've identified above, textual duplication is the worst. It is the quickest to bite you as the system grows. It has a nasty habit of being viral: when you see code duplicated in the system, it can be tempted to think "Oh, well then, it's not a problem if I just nic this code and put a copy of it over here and fiddle it a bit. Hey, everyone else is doing it!" It's a lot like the Broken Window that Dave Thomas & Andy Hunt talk about. If you don't keep on top of the little things as they appear, the whole thing will soon go down the toilet.

Summary

So, chances are pretty good that some of your code sucks.

What are you going to do about it? Well, first you have to recognise the fact (and hopefully this article has helped with that) and admit that there's a problem.

Then you have a choice. You can choose to do something about it. You can learn new ways to work... ways that help you write better code. That's great. If only one programmer out there does this, I'll be happy.

Your other option is to do nothing about it, and continue writing code the way you always have.

It's your choice. Choose wisely.


Chris Dailey

Posts: 56
Nickname: mouse
Registered: Dec, 2002

Re: Why Your Code Sucks Posted: Sep 27, 2004 2:17 PM
Reply to this message Reply
Your code sucks if it isn't testable.

So this begs the question ... what makes code testable?

The best answer I found deals with complexity due to method paths and dependencies.

For method paths, any information on McCabe's cyclomatic complexity calculations is extremely helpful. Google is your friend here, as are programs such as JavaNCSS and devMetrics (C#).

Two good sources of information about dependencies are:

1. Large Scale C++ Software Design by John Lakos

2. Compuware's OptimalAdvisor program, which is free (cost, not free libre). They also have lots of good articles at the site.
http://javacentral.compuware.com/products/optimaladvisor/

I don't know of any dependency analysis software available for .NET yet.

I believe that easy analysis of depencencies in an automated fashion depends on statically typed languages. Perhaps it can be done in a dynamically typed language, but it would be a much harder problem.

Are there any other factors that you know of that contribute to testable software?

Hristo Deshev

Posts: 1
Nickname: muslon
Registered: Sep, 2004

Re: What makes code testable? Posted: Sep 27, 2004 2:34 PM
Reply to this message Reply
Hi Chris,

I have seen a really good definition of testable code in Michael Feathers' blog:

I don't care how good you think your design is. If I can't walk in and write a test for an arbitrary method of yours in five minutes its not as good as you think it is, and whether you know it or not, you're paying a price for it.

You can find the whole post at http://www.artima.com/weblogs/viewpost.jsp?thread=42486

Have fun.

Hristo Deshev
http://muslon.blogspot.com

Chris Dailey

Posts: 56
Nickname: mouse
Registered: Dec, 2002

Re: What makes code testable? Posted: Sep 27, 2004 4:27 PM
Reply to this message Reply
It sounds like you're talking about the effects of testable code. You know when you're writing tests for it whether it's testable. And if you're writing tests at the same time as development, then you keep reworking the code until it is testable.

But what about going back and writing tests for code that already exists? What makes the class testable or not? What rules of thumb did programmer A apply that caused his/her classes to be more easily testable vs those that programmer B used that were not easily testable?

Basically, my question is this: What are the patterns and practices that facilitate testable code? That's the heart of the question I tried above (but apparently failed) to ask and then partially answer.

Jason Yip

Posts: 31
Nickname: jchyip
Registered: Mar, 2003

Re: What makes code testable? Posted: Sep 27, 2004 7:19 PM
Reply to this message Reply
...
> But what about going back and writing tests for code that
> already exists? What makes the class testable or not?
> What rules of thumb did programmer A apply that caused
> d his/her classes to be more easily testable vs those that
> programmer B used that were not easily testable?

The rule of thumb that programmer A used was probably to write the test(s) before writing the code.

But you're talking about pre-existing code...

> Basically, my question is this: What are the patterns and
> practices that facilitate testable code? That's the heart
> of the question I tried above (but apparently failed) to
> ask and then partially answer.

Short answer is high cohesion and low coupling... which doesn't really say much. I would suggest that the most direct and effective way to learn what properties testable code should have would be to practice test-driven development (TDD) and observe the result. I'm not aware of any papers/articles that are specific and thorough about the design properties resulting from TDD.

Steven E. Newton

Posts: 137
Nickname: cm
Registered: Apr, 2003

Re: What makes code testable? Posted: Sep 27, 2004 7:24 PM
Reply to this message Reply
Well Brett Pettichord has a paper titled "Design For Testability" that's a good resource. It's at http://www.io.com/~wazmo/papers/design_for_testability_PNSQC.pdf

Kirk Knoernschild

Posts: 46
Nickname: kirkk
Registered: Feb, 2004

Re: Why Your Code Sucks Posted: Sep 27, 2004 9:38 PM
Reply to this message Reply
>Your code sucks if it has duplication.

I agree with most everything you have to say. However, while the DRY principle is almost universally applicable, like most principles, there are times when violation is acceptable. The most notable is when you want to manage dependencies. If avoiding duplication means creating a heavy dependency on another component, then one might consider copying the code in favor of creating the dependency.

Certainly, refactoring can help in this situation, but there are some situations, such as when the dependency would be on a 3rd party component, where refactoring isn't always an option.

Principles, patterns, heuristics, guidelines are all valuable, but as developers, we still have to think about when they're appropriate, and when they aren't. Unfortunately, I think we too often rely on principles, etc. to do the thinking for us.

--kirk

Marc Clifton

Posts: 1
Nickname: cliftonm1
Registered: Sep, 2004

Unit Test Patterns Posted: Sep 28, 2004 7:23 AM
Reply to this message Reply
Well, everyone seems to be commenting about the difficulties of testing, so I'll mention my article on unit test patterns, part V on a series on Advanced Unit Testing (also note that AUT is an open source project hosted on Tigris).

Article: http://www.codeproject.com/gen/design/autp5.asp

Marc

phil jones

Posts: 14
Nickname: interstar
Registered: Apr, 2003

Re: Why Your Code Sucks Posted: Sep 28, 2004 8:53 PM
Reply to this message Reply
"Choose style and formating conventions early in the project, and conform to them. Require that everyone conform to them. This will be easier to do if everyone has a hand in deciding on the conventions and buys into them. This will make the code uniform and thus easier to read."

I'm still skeptical of this. Of course, I'm in favour of sensible naming and layout. And I think most experienced programmers already do this. But I don't believe this should automatically imply setting standards across a project. Particularly for indentation and line-length.

The problem with standardizing those is that they depend a lot on the screen you're using. Are you using a 15 inch laptop or a 21 inch desktop? Do you prefer your windows split vertically or horizontally? Demanding a convention there is tantamount to demanding conformity of the whole screen layout. I simply don't believe any benefits of standard line length outweigh the cost in this case.

Ben Griffiths

Posts: 1
Nickname: aycrum
Registered: Sep, 2004

Re: Why Your Code Sucks Posted: Sep 29, 2004 8:44 PM
Reply to this message Reply
Great article.


Most code will suck at some point during its development and improving code is an iterative process, I like to state the process as:


1. Prove that it works.
"Your code sucks if it doesn't work."
"Your code sucks if it isn't testable."


2. Make it express itself clearly unless that conflicts with rule 1.
"Your code sucks if it's hard to read."
"Your code sucks if it's not understandable."


3. Remove duplication unless that conflicts with rule 2.
"Your code sucks if it has duplication."


4. Make it efficient unless that conflicts with rule 3.
"You have temporal duplication when the same work is done repeatedly when it doesn't have to be."


In practice, meaningful conflicts between the rules hardly ever happen – getting 1 and 2 right mean that you often get 3 and 4 for free. I think that if you find yourself running into conflicts very often then your code really sucks and at that point it's worth getting a colleague to sense-check your early decisions.

Another nuance is that you can make your code suck less - it's not an either/or decision.

The difference between great programmers and good hackers is that in watching a great programmer code you get the sense that they can see the arc that code must take from working but sucking to it being great code. Hackers tend to stop when it works.

Andrew Byshenk

Posts: 1
Nickname: abyshenk
Registered: Sep, 2004

Re: Why Your Code Sucks Posted: Sep 30, 2004 8:21 PM
Reply to this message Reply
Okay, I admit that at least some of the code I have written in my life did not live up to the high standard I demand of myself. ;-)

I like much of the content of Dave's treatise, but there are a few items that bear further discussion.

A statement that bothers me is:
>>>Your code sucks if it doesn't work.
While that may sound like some obvious tenet on its face, I humbly submit that it is not true at least inasmuch as by using logic, the contrapositive is questionable. The contrapositive of the statement above is:
>>>Your code works if it does not suck.
That doesn't seem like it could really be true. Your code may be incorrect if it doesn't work, but it may actually be good (non-sucking) code if it follows several of the other rules outlined in the treatise. In particular, if your code is testable and readable, defects can be readily found, fixed, and verified. Moreover, if it is truly understandable (and that is a high standard), it may even be refactored as necessary.

Have you ever tried to refactor code that you didn't understand? What people really do when they try to refactor code that they don't understand is either a) break it or b) rewrite it or c) first break it and then rewrite it. Rewriting is not the same as refactoring. Rewriting tends to result in such problems as functional duplication. Refactoring does not.


Another problematic portion of the treatise, in my opinion, is found in the two competing desires:
>>>Your code sucks if it's not understandable.
>>>Your code sucks if it has duplication.
I propose that if your code has sections that are similar, that is not necessarily undesirable duplication; it may actually be a desirable pattern-based design. It is true that in many such cases, a programmer can eliminate the duplication by applying one or more techniques (including those itemized in the treatise). However, it is not always desirable to do so. I warn all programmers that in many cases, applying such refactoring techniques increases the complexity of the static model, and increasing the complexity of the static model beyond certain limited bounds actually decreases understanding. For example, the deeper a type hierarchy gets (past 5 levels or so), the less most programmers can retain it. Or, the more functionality is generalized at an abstraction and specialized by the concrete, the less most programmers follow the logic and interactions.

I'm all for choosing wisely...

robin bakkerus

Posts: 1
Nickname: jrb
Registered: Oct, 2004

Re: Why Your Code Sucks Posted: Oct 3, 2004 8:58 AM
Reply to this message Reply
I fully agree with all your comments, except for one:
"Why not use public fields, instead of private fiels plus getters and setters".
Your argument is correct that in many cases these getters and setters don't do anything extra. But the point is that a number of frameworks rely on the fact that there is a getter and setter, for example Hibernate BeanUtils, Struts etc etc.
To create all these getters/setters is just two mouseclicks (in Exclipse at least) and Java will handle these getters as efficiently as public fields.

Dave Astels

Posts: 32
Nickname: dastels
Registered: Mar, 2003

Re: Why Your Code Sucks Posted: Oct 6, 2004 11:34 AM
Reply to this message Reply
> But the point is that a number of frameworks rely on the
> fact that there is a getter and setter, for example
> Hibernate BeanUtils, Struts etc etc.

Yes. and in those cases you need to add getters/setters. but when all you want is a publically accessible variable... just make it public.. and, of course, question why you want a publically accessible variable in the first place.

Dave

Steven

Posts: 1
Nickname: smb77
Registered: Oct, 2004

Re: Why Your Code Sucks Posted: Oct 9, 2004 2:08 PM
Reply to this message Reply
Great article (and yes I have written code that sucks in many ways), although I will take issue with the idea of haveing publically accessible variables for two reasons.

1. Consistency and Readability.
There will be, in almost all cases, some getter and setter methods that do more than simply set and return the variable. Rather than trying to remember wich variables are public and which are available through getter and setter methods, simply make them all getters and setters.

2. Maintainability.
I have on more than one occation changed a getter and/or setter from simply passing the value to actually doing some formatting/parsing or value checking. If the variable is available publically I cannot do this without breaking other code.

Dave Astels

Posts: 32
Nickname: dastels
Registered: Mar, 2003

Re: Why Your Code Sucks Posted: Oct 12, 2004 11:27 AM
Reply to this message Reply
> Great article

Thanks

> 1. Consistency and Readability.
> There will be, in almost all cases, some getter and setter
> methods that do more than simply set and return the
> variable. Rather than trying to remember wich variables
> are public and which are available through getter and
> setter methods, simply make them all getters and setters.

I'd argue that you should try to not use getters and setters at all if you can avoid them. Many programmers (I'm tempted to say most.. based on my sampling) still haven't grokked the real idea behind encapsulation. This is especially true in the Java world where getters/setters are central parts of various paradigms & frameworks (e.g. JavaBeans). Go read some really good OO code. If you put the behaviour where it belongs you won't need nearly as many getters/setters.

> 2. Maintainability.
> I have on more than one occation changed a getter and/or
> setter from simply passing the value to actually doing
> some formatting/parsing or value checking. If the
> variable is available publically I cannot do this without
> breaking other code.

I don't accept that as an excuse anymore.. with tools like Eclipse & IDEA. They can find references to the vars.. and often even do the switch to a getter/setter for you.

Alan Holub had a great article in JavaWorld a while ago (http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html) that discussed this. Here are a couple of significant quotes:

"an experienced OO designer could probably eliminate 99 percent of the accessors currently in your code without much difficulty"

and:

"Getter/setter methods often make their way in code because the coder was thinking procedurally. The best way to break out of that procedural mindset is to think in terms of a conversation between objects that have well-defined responsibilities."

What's needed is a solid understanding of OO. I'm constantly amazed that, after 37 years, that's still mostly absent!

Maybe I should change my mind regarding certification & licensing of programmers.

Dave

Flat View: This topic has 21 replies on 2 pages [ 1  2 | » ]
Topic: The Management Myth Previous Topic   Next Topic Topic: Revisiting: Why salary bonus and other incentives fail to meet their object


Sponsored Links



Google
  Web Artima.com   

Copyright © 1996-2014 Artima, Inc. All Rights Reserved. - Privacy Policy - Terms of Use - Advertise with Us