Bad Code – Snippets from production

True, false … or maybe?

Lets start with something on the funny side. Some of the code that I have come across in our system.

boolean isFutureDeal = Boolean.parseBoolean(request.getParameter("loadFuture"));
if(isFutureDeal != true){
       isFutureDeal = false;
}

If it’s not true, set it to false. I guess this is to make sure it isn’t set to that other boolean value – true, false, maybe.

Give me that.  What?!  I don’t need that.

public ShoppingCart createNewCart(ShoppingCart cart, HttpSession session,
                     MiniUserBean member) throws Exception {

              if (cart == null) {
                     session.setAttribute("shoppingCart", new ShoppingCart());
                     cart = (ShoppingCart) session.getAttribute("shoppingCart");
                     cart.setCurrentRedemptionType(member.getRedemptiontype());
              }
              cart = (ShoppingCart) session.getAttribute("shoppingCart");

The cart object is passed in, but the first thing we do is read it from the session, so why even pass it?

Day of the week calculation

[pseudo-code...actually seen in production]
if (date == "11/28/2016" || date == "12/5/2016" || date == "12/12/2016" 
     || date == "12/19/2016" ...) {
          dayOfWeek = "Monday";
} else if ( date=="11/29/2016"...

You get the idea.  There were about 1/2 year’s dates in the if statements.  I found out about this gem when a customer called and complained that their script was not setting the day of the week any longer.  This was in perl, and I replaced that huge block with a few lines that would forever compute the day of the week.

Learning in the real world

How many times have you heard a kid say, “when am I going to use this in the real world”? How many times have you said it?

My niece is taking some programming courses in college and she told me she was having difficulty with one of the classes. When I asked, she said she just didn’t understand how what she was learning would be applied.  The professor had forgotten to tell the class the why of what they were learning.

This happens throughout our learning lifetime.  I wrote a post a while ago (Searching for the why) where I talked about making sure to include the why when talking about processes as well as coding techniques.  Knowing the why helps us to apply the how better.

Does knowing the why help us learn too?

I think understanding why we are learning a new skill helps motivate us to learn it.  Let’s take for example, learning a new language.  I have tried many times to learn a new language, and I have only been partially successful.  I was most successful learning some Spanish when I was planning on going to a Spanish speaking country.  Since then, it has been on and off.

I don’t know anyone who speaks Spanish.  I live in the midwest and we don’t have a lot of native speakers in the area.  I have little to no reason to learn it, other than I have this fantasy in my head of me being able to speak multiple languages fluently.

Sometimes the skill that we are learning is simply a stepping stone to something more complex, and having a concrete why for that may be difficult at best.  This happens a lot throughout our school years.  But, that is the why right there.  Because it is crucial to understanding something greater.

I encourage our teachers, our mentors and our parents to help the learners of the world to understand the whys of what they are learning as well as the whats and the hows.  I encourage our learners to ask the teachers they whys as well.

Design Patterns: Facade

What are Design Patterns?

Most of you already know what a design pattern is, but I want to make sure this blog is accessible to everyone.  A design pattern is a set of “best practices” for how to structure your code to solve certain problems.  It is not a completed library that you an plug in to your code base, but more like a template helping you determine how to structure the code you are writing.

There are a couple of good books out there describing Design Patterns.  The one most people will point you to is the Gang of Four (Design Patterns: Elements of Reusable Object-oriented Software) book.  I know it is probably blasphemy to admit, but I have actually not read that one…I have, however, read the Head First Design Patterns book and I highly recommend it.

Today we are going to dive into the Facade pattern.

Facade Pattern

The definition of the word facade helps me remember what this pattern is used for:

  • an outward appearance that is maintained to conceal a less pleasant or creditable reality.

We often use the facade pattern to hide either a complex part of the system (or subsystem) or to simplify the use of some subsystem. And, sometimes we use it to simply hide a subsystem, third-party library, etc.

It is obvious why we want to create a class that understands the complexity of a subsystem and keep that complexity out of the other parts of your code.  However, it is probably less obvious why you would want to simply hide a subsystem or third-party library behind a facade.

If you hide your subsystem, or third-party library behind a facade, it will allow you to more easily switch this system out for a new one, or even a new version without having to do major refactoring.  If there is only one place that is dealing with this system, there is only one place where you need to make changes.

Secondly, using a facade can help you to mock out systems that you interact with for testing.  Sometimes it can be difficult to fake out a system you are interacting with, but if every part of your system uses the facade, then you simply have to mock out the facade in the rest of the application, and only worry about figuring out how to mock the real system out for the tests around the facade.

 

Lastly, when you hide things behind a facade it allows you to delay making major architectural decisions.  You can determine what you want the API to this system to look like, and even fake out the interactions until you have had a chance to decide how you want to implement the subsystem.

* Photo by Zacharie Gaudrillot-Roy

Content is not good enough

I have been fortunate enough to have a job that I was truly excited to go to every day.  I loved what I was doing and who I was working with.  Before that I was content at many of my jobs, but wasn’t truly happy/truly in love with the job.  Since then I have been more picky about where I work.

This is going to sound hokey, but just bare with me.  When I work somewhere that I really enjoy and do something I really enjoy, I get energy back from it.  I don’t dread going to work and rarely come home feeling drained.

 

 

Since we spend so much of our lives at work, why wouldn’t you find a place that you are truly happy?

I think I have said this before, but when you are interviewing at a company, it is not just an opportunity for them to learn about you, but an opportunity for you to learn about them.  See if you can talk to some of your potential colleagues.  Ask them what they like about the company and what they would change.  This can give you some good insight into whether you would be happy.

Take some time and look at you current job and previous jobs.  What worked well at each place?  What didn’t work well?  What made your life easier?  What were your biggest pain points?  Now you have a good feel of what your ideal job would look like.  Now when you are looking for a new position, you can use this information to help you decide whether it is right for you.  Good luck!

 

Should I apply?

Recently, I have not been very happy at work, so I was keeping my eye out for another position that looked interesting.  I hadn’t applied to any of the listings I saw simply because they hadn’t piqued my interest quite enough.

Now, I am laid off and I am applying to some of those positions I passed on previously.  As I have been looking at listings, I ask myself if I really want to apply to this position.

Sometimes, after a little research on the company it is obvious that I do not want to apply (glassdoor.com is your friend, they have employee review of the company).  But, often I am still left wondering whether I would be happy there…should I apply, or just walk away now?

While being laid off may be influencing my thought process, I have realized that the best way to determine if a place is going to be right for me is to actually apply and interview there.

Nothing can replace the experience of visiting a workplace and talking to the people who work there.  You get a chance to see the workspace.  I have visited workplaces and decided I was not interested after visiting.  Remember, this is where you are going to be spending 8 hours a day.

The interview is a chance to get a feel for some of the people you will be working with.  Your colleagues can make or break a job.  Pay attention to who interviews you and what sorts of questions they ask.  You can get a good feel for their culture.

Remember, even if the company offers you a job, you are not required to say yes.  It is your decision whether this is the right position for you.  The interview process is not just a chance for the company to find out if you are a fit for them, but also an opportunity for you to determine if they are a fit for you.

Code Coverage

We all want to achieve 100% code coverage.  If we have 100% of our code executed in tests, we can feel confident that we are testing our code well. Right?

Not so fast!

We wanted to share some examples of tests that, while they may boost your code coverage, aren’t testing much.  Below are some examples we have actually seen (code has been changed to protect the guilty).

Trust, but verify

This is a code snippet using Mockito.

public void testSomething() {
   classInTest.doStuff();
   verify(classInTest).doStuff();
}

For those of you unfamiliar with what verify does in Mockito…it will pass if the method in question was called.  That’s right…this test is verifying that we called the “doStuff” method…which we did in the previous line.

Unfortunately for us, our code coverage tools will look at this and see that we executed lines inside the doStuff method and count them as covered, even though we didn’t actually test anything!

Preparation is key

public void testStuff() {
  when(thing.doStuff()).thenReturn("hi");
  when(otherThing.doStuff()).thenReturn("there");
  .... lots more setup ....

  classInTest.methodInTest();

}

That’s right…they took the time to do a whole bunch of setup and even called the method in test…but forgot to actually test anything!  There are no asserts or verifies.  Again, our helpful code coverage will show that lines inside of methodInTest were covered since they were executed in a test, even though we didn’t test a single thing.

Green means Go

public void testTest() {
  ... improper setup ...
  ... call method ...
  ... assert the wrong thing ...
}

Sometimes we make mistakes in our setup code and when we run the test, it fails, even though we are sure that the code is working correctly.  That leaves us with two options.  1) Figure out why our test is failing…step through, debug, print stuff out, etc.  See if it is a bug in the production code that we hadn’t noticed or if there is an issue with our test.  OR 2) Change the expected values in our test.

Option 2 is obviously the easier answer.  Yes, we’ve seen this.  No, this is not correct.  Tests are supposed to give you confidence that your code is working correctly.  It can also act as a sort of documentation for future developers.  Do you want future coders to look at the test and think that what you are testing is expected behavior?

What have we learned?

Code coverage is not actually telling the whole story.  As we have seen above, if you have poorly written tests lines of code covered don’t mean anything.  Anyone can be clever and find ways to execute more code in their tests…the smart programmer figures out how to actually test the code.

Please be vigilant in writing your tests and actually ensuring that your code is doing what you expect it to.  When doing code reviews, if you see any blunders like we show above, or other not so great tests, please help the other person write better tests.  You, your team and your code will be better off for it.

photo credit: <a href=”http://www.flickr.com/photos/62975503@N04/8227882239″>Facepalm</a&gt; via <a href=”http://photopin.com”>photopin</a&gt; <a href=”https://creativecommons.org/licenses/by/2.0/”>(license)</a&gt;

 

Starting an agile project with a new team

At the first job where we were trying to do agile, none of us knew a lot about what that meant.  We didn’t break down our work into stories, therefor we didn’t estimate any stories.  We simply divided up work that needed to be done among the pairs and did pair programming and test driven development.

Then, the next job where I was doing agile, we didn’t estimate our stories using story points.  We estimated in hours, kind of.  We used a set of hours that we could pick from, it was either 1,2,4,8,16,32 or 64 hours.  This allowed us to stop fooling ourselves that we could be very accurate in estimating a task that was going to take longer than a couple of hours.  In order to determine how much a pair could accomplish in a sprint (which were one week long), we simply found 32 hours worth of work and BINGO.

In my current job, I am trying to bring more agile processes to bear.  I am in charge of a new project at work and have decided to run it in a completely agile manner.  I am the scrum master (which I have only kind of done before, mostly for myself).  The project has only one developer resource.  We have access to a QA resource as well as our user experience designer, though neither of them have any dedicated cards.

The developer on this project is a contractor who is not familiar with the code base, nor with the system that we will be integrating with.  We have not worked with him before.

Our first task was to introduce him to the project and then estimate the cards that we had created.  Rather than going with hours, I decided I wanted to use story points.  Our scale is 1,2,4,8.  Anything that is sized over an 8 needs to be broken down.

After a long estimation session, we needed to determine what work would go into our first 2 week sprint.  In an existing team this is fairly easy…just look at the average velocity and put that many points in the sprint, adjusting for any absences.  But, what do you do when you have a new team and have no idea what the velocity is?

You guess.

That’s right.  Look at the cards that you want to play and start adding things to the sprint until the team says stop.  Then, make sure your backlog is groomed well so if the team guesses wrong and can pull more work in than they thought, they can pull from the top of the backlog.

If the team doesn’t get all of the work done that they committed to in this first sprint, don’t hold it against them.  The first couple of sprints are learning sprints.  The team is learning the code base.  The team is learning the process.  The team is learning what their velocity is.

After a few sprints you can take the average velocity and start using that to plan out the work.  If your team is consistent with their estimation, and is fairly consistent in their velocity, you should be able to fairly accurately estimate when different features will be complete.