“it’s your job to defend the code”

As I read “Clean Code” by Robert C Martin, I was particular drawn to the passage about why unrealistic commitments should not be an excuse for writing bad code.  He points out that while it is a manager’s job to “defend the schedule and requirements”, it is a developer’s “job to defend the code with equal passion.”

What management says vs what management wants

One way this becomes a problem is when a developer hears management say “I want it by Tuesday.”  They may want it by Tuesday, but does that mean they are getting it by Tuesday.  The manager also wants it to work correctly and meet all the requirements. A manager isn’t going to be happy if we claim it is done on Tuesday and then a whole pile of rework needs to be done afterwards.  This really means the task wasn’t done by Tuesday and we just claimed it was.

“Done”

The manager also doesn’t want this one date to be met at the cost of meeting other dates. One way this comes up is by saying the task is done while silently thinking that parts of it will be done “later.”  Regardless of whether “later” ever comes about, the task isn’t REALLY done.  When I disagree with someone on whether a task is done, I’ll ask if they need anymore time to work on things related to it.  This may bring up parts of the task that aren’t really done.  Sometimes it brings up additional tasks that were discovered during this task.  Either way, it is often a productive answer.

Trust

When unrealistic estimates come up, there are often two responses.  One is to silently accept it knowing the date will not be met (or will be met at the cost of other things) because it is “what the manager wants.”  The other is to discuss why the estimate will not be met with management.  Of course, the later requires much more energy at first.  However, it pays off in getting everyone to have a realistic understanding of the project.

Schedule gambling

These other desires tend to be implicit, so we only hear the deadline. That doesn’t mean that the manager wants it done by Tuesday at the expense of all else.  I’m fond of saying that management would rather meet the production date than a development date.  So if we are meeting the development date at the expense of something that affects the production date, management will really be less happy.  This may be sloppiness that makes other tasks take longer or leaving parts of the task incomplete.

What all of these things have in common is that it is our responsibility as developers to keep management up to date and provide them with accurate visibility into what is going on within the project.

—-

This coming week we will be hosting Uncle Bob (Robert C Martin) at JavaRanch in the Agile and Other Processes forum.  Come on in, ask him a question or join the discussion.  You might even win a free copy of the book Clean Code: A Handbook of Agile Software Craftsmanship.

premature optimization

Donald Knuth is often quoted as saying “premature optimization is the root of all evil.”

Often in the JavaRanch forums, we see questions like:
which is faster stringBuilder.append(‘a’) or “a” + “b”
(The answer is these two expressions are supposed to generate to the same byte code if used on the same line.)

My first thought when I see this is who cares. I’d be very surprised if the bottleneck in an application was being caused by this line. I’d be even more surprised if the poster had done the analysis to prove this line is the performance bottleneck before posting.

What got me thinking about this is that I think I just wrote some premature optimization on a pet project.

/* Only do search and replace if one or more images in value. This is likely premature optimization but it's faster to add the if statement than find out. */
private String convert(String value) {
  String result = value;
  if (result .contains("<img src=")) {
    for (String imageName : setOfTwentyElements()) {
      // do a regular expression search and replace
      result = result.replaceAll(imageTag,  strippedOutImage);
    }
  }
  return result;
}

The fact that I commented it means I’m aware of doing it. Which is a step ahead of what usually happens where we write the optimization assuming it will be important and later (if ever) finding out otherwise.

In this case, I expect to run the method over a million times and the value not containing an image over half the time. After that the containing code will not live on. I also know the containing code contains a database update which means the code is not the bottleneck. To me, the extra if statement seems like a low cost to avoid doing some work at all. It also feels like a code smell for premature optimization. If this were a real application and not a pet project, I would invest some time in finding out how long the code actually takes to run for my data patterns/length.

Here I’m more interested in whether it is an example of premature optimization. I think I might have to leave it a philosophical question. Comments one way or the other are certainly welcome.  My leaning is that it is still premature optimization and this post is a rationalization.

copy & paste

Virtually all of us are guilty of copy and paste “reuse.”  There are different types of reuse though and each come with their own set of problems and gotchas.

Copy/paste a method or large code snippet

This is the kind of reuse that almost everyone will agree is bad.  When all is said and done there are two copies of the code.  If a defect is found, it is likely to be fixed in one and not the other.  Which is bad because an avoidable defect now stays in the code.  This practice is hinting we should extract a new method and call it from the original code and our new code.

Copy/paste a code snippet and then change some values

While less blatant than the copy/paste a method code, it is still a code smell.  This practice is hinting we should be refactoring into a new method that takes some parameters.

Copy/paste a code snippet and then change the body

This approach tends to be used for boilerplate code.  One common example is copying the Java code for a regular expression replace loop and then changing the middle where the replace logic.  In languages with closures, the need for refactoring is more glaring.  In Java, it becomes a call on whether the reuse is worth it.  For tiny examples, it tends to not be.  For more complex examples, an inner class or inheritance is often the answer.

Copy/paste an idea

The hardest type of copy/paste reuse to detect is when it is on a conceptual level.  A copy paste detection tool is unlikely to find it as the code wasn’t copy/pasted.  This kind of copy/paste reuse occurs when someone is trying to figure out how to accomplish a task – say develop a DAO.  Suppose the developer is new and doesn’t know what to do.  The developer sees some code that loops through a result set and decide to use that idea.  Then the developer sees some code that says getString() and copies that for all the types.  This is all well and good until the data types aren’t varchars.  Now the developer has getString() being used to get numbers and Dates too.  Worse yet, this will not crash and appear to work.  However, it will create other problems such as figuring out how to parse the date.  If you were to ask the developer why they are using getString() instead of getDate(), the developer won’t know.  This is fairly low level example.  The same can happen on a higher level such as why the developer is using JDBC instead of JPA/entity beans/etc.

JavaRanch’s statement on Not being a code mill gets into this.  Experts from the page “We are not, however, a code mill and make a point not to give out working code to someone who wants to … dump it into their project without knowing what it does. … that will help you to learn how to solve your problem using best practices.”  I think this is an important point in all levels of reuse.  While copy/paste reuse may appear to work at first, it doesn’t help in the long run.

When the copy/paste code causes defects in the future or needs to be modified, it needs to be understood.  “I did it that way because some other code does” doesn’t make for a helpful reason to the team down the road.