Bye Derek?

Recently, Derek has been “Working” From Home due to not being able to drive to work. After we finished the development of our project, the developers needed to chip in with Regression testing. We had a chart to track our progress, and I thought that would motivate Derek to work; but that wasn’t the case.

After 3 days, I looked at the chart. I had ran 35 tests, and Derek had run 3. What has he being doing? Obviously not working. Anyway, days later, he announces he is leaving the company. OK, that makes sense, he was out doing job interviews in company time.

Our company has many employees that have left and then come back at some point. So even though it is sad that Derek is leaving, I believe he will be back. I’ll give him 3 months. I assume he will be on a probation period, and I’m sure if a company thinks they are getting a Senior and then Derek turns up, they will be backing out of that deal. Then Derek will come crawling back.

The two main inspirations for the Colin character have moved teams, so in that regard; the blog has taken a massive hit this month.

On the positive side, I’ll be starting a new project soon and will most likely be working with new people with new technologies.

It’s quite possible the blog will be a bit quiet for the time being. But hopefully it’s not the end.

Care

After I write some code, I always test and inspect it thoroughly. I’ll write Unit Tests, running manual tests, constantly reading and re-reading the code. Any slight change I make, I then end up repeating the process. Even when I’m about to send it to review, I get paranoid that I’ve missed something, so end up looking at the files again to make sure.

It’s often rare that I get any (serious) feedback to change my code, and it’s rare that Testers will find anything wrong with it. But then sometimes I wonder if I’ve spent a little too long in the development stage.

However, Colin and Derek seem to take the opposite approach and end up coming out with something more quick and dirty.

Derek always seems to come up with a solution close to a hack which I’ll tear apart in the Code Review. After, it will end up being rejected by Testing and he will put in another bodge.

Colin spends time writing Unit Tests but often misses some scenarios. His methods often need refactoring down more, and methods/variables/classes may not have the best names. His solutions still come across as rushed, and he seems to cut more corners when the deadline approaches. When I point out his mistakes, he often says “the deadline is tomorrow night, so I just have to get it checked in”.

But surely it’s useless if it doesn’t work? In a similar fashion to “it’s better late than never”, you could say “it’s better that it is late and works, rather than getting it early and broken”.

He got some comments last week to refactor some of his methods. He quickly changes them and submits his new method for review. It was a one-line method, and somehow there were 3 mistakes in it. The thing is, he had Unit Tests covering every possible scenario of that method; he just didn’t run them. Unit Tests take a several seconds for a large batch of them to run; why would you cut corners so much that you don’t even run the unit tests? If you had full coverage, it will allow you to cut manual testing, but to cut testing completely? Madness.

Anyway, his code was something like this:

public string DisplayInfo(bool isScotland)
{
 return DisplayInfo(Configuration.Scotland==Configuration.Scotland); 
}

The 3 things that are wrong:

he is passing in a parameter “isScotland” then not using it
where he should be using “isScotland”, he is calculating it…but wrongly. He is comparing a value to itself; so it always evaluates to “True”
The method just calls itself, which calls itself, which calls itself…until it crashes with a StackOverflowException because it’s an infinite loop.

Absolute Junior-level coding.

Delegating/Shirking Responsibility

As a developer, we are paid to fix bugs or introduce new features. So as long as you are asked to write in the language you know, it seems silly to turn work down. The exception to this is if part of the code-base requires a high amount of domain-specific knowledge.

We do have a few areas that are like that, and we end up keeping that knowledge with a small group of developers, and in some extreme cases; just a single developer. If that developer leaves the company, then there’s trouble.

I think there has only been one situation where I’ve been asked to fix something, and I have declared it to be a bad idea. It was in an area of code that basically works in 99% of the situations, and if you change it; you are almost guaranteed to introduce a new bug (or many). It’s a really important area of the system, a manager was stating it had to be fixed within a few days; piling on the pressure. I put up a good argument to wait a day for the Lead Developer to return from holiday. A gamble; but I could take the pressure off myself, reduce risk of mistakes, and I could pass on the responsibility.

It was a brilliant idea in hindsight. He came in, I explained the problem and he managed to come up with a really understandable solution within an hour. Worked perfectly; crisis averted.

There’s been plenty of times where there’s been really standard, low risk bugs and Derek has been asked to fix them and he has said “I don’t know much about that area, so I’m not best placed to do it”. But it’s a bit of a paradox, because if you don’t look at the code, and don’t learn about functionality, then you will never know it, and will always come up with that excuse.

“that’s the limit of my knowledge, so I draw a line under it and move on.”
Derek

A few days ago, in the stand-up meeting, our Team Lead asked Colin if he could help pick up one of the bugs. Again, it was a standard bug and he said “I don’t think I should pick it up, I think Sam should do it.” The thing is, Colin is a Senior and Sam is a Junior; a couple of ranks below him. When you think of it in those terms; it is just absurd. Sure, I understand Colin’s point that Sam may know which files the problem may reside in; maybe Sam has even introduced it! But, is there really a situation where a Junior is in a better position to fix something than a Senior?

In my opinion, delegating down makes you appear silly. Delegating up alerts people to the severity of the problem. You’ll just look silly if it is actually a simple problem.

Mini Musing #1

In C#, when you create a Winforms form, you get a “.cs” file where you can write your own code, then a “.Designer.cs” file which is autogenerated. Every time you make a change using the Design View, it will regenerate the “.Designer.cs” file, so you should never manually change it.

    #region Windows Form Designer generated code

    /// <summary>
    /// Required method for Designer support - do not modify
    /// the contents of this method with the code editor.
    /// </summary>
    private void InitializeComponent()

As you see in the example, it has a region which says “generated code“, then the next part is a comment that says “do not modify the contents“. So even if you don’t previously know it’s a generate file (as I described in the first paragraph); you sure do now.

Today, I got a code review where the InitalizeComponent method had been manually changed. Idiots.

Overtime

In a follow-up to my blog on Work Hours, I wanted to discuss Overtime. From my experience, Overtime has rarely been worth it for the employer, and my strong guilty conscience prevents me from taking advantage.

I haven’t looked into the topic, but the widely adopted “9-5 work hours, 5 days a week” must have come into play after years of theorising and research. Working past this limit will lead to burnout. After working for a few months in the 9-5 structure without additional time off, I do come to the point where I need time away. Working past this point will just push me to that point earlier.

Recently, my project has been approaching the final deadline, so overtime has been available. Some team members have taken advantage of this, whereas others have avoided it completely.

One day, I was helping out testing and was finding appropriate Test Cases to link to the Work Item. I saw that some tests I had previously wrote now were incorrect. I checked the history, and the tester that was doing overtime (I’ll call him Brian) had changed them on Sunday, late at night. He turns up late to work on Monday, looking really fatigued, and several times during the day, he calls me over to explain Work Items to him; which was unusual. So basically, the employer paid him a full days work on Sunday, and then he fails to perform on a Monday when he is actually contracted to work. So it seemed that we gained a day, but then lost it straight away.

There was another Work Item I was looking at, and I saw there was an Overtime Task from Colin assigned to it. Weird, since the tests were written by Brian, someone else wrote the fix, and it hadn’t been tested yet. I check the history, and Brian had removed all the tests that Colin had written, and rewrote them during work time. Colin was claiming 7 hours overtime on it. A full day at the weekend for no benefit.

There was a day where I overheard Colin talking to the Product Owner, saying that he had lost his work over the weekend. Now, I didn’t hear his reason, but I was thinking “how the hell can you lose your work?”. Let’s say it was a hard-disc failure; well, you wouldn’t be complaining that you lost your work – you would be moaning about losing everything. Let’s say it was a server failure and his Shelveset was gone; many other people would be complaining. So the only thing I can think of is that he didn’t create a Shelveset, then undid his changes; which is just incompetence. The only other alternative, is that he had nothing to show for his several hours of development, so came up with a “dog ate my homework” excuse to try and get paid anyway.

Yesterday, I was sat with Colin discussing the work we had left. He then told me that he was sick of the project and felt overworked. Since we are roughly on track, he was gonna take a couple of days off work. He then asks me why I don’t do overtime. Well, I think this proves my point.

We were seemingly behind, he does all this overtime and the end result is that we’d probably be in the same position if there was no overtime available, but now we have fatigued, demotivated team members.

I don’t know what the end result is going to be, but there was a time were promotions were dished out, but I didn’t get one despite it being a widely accepted fact that I was among the highest performers in the team. In a meeting with my line manager, he said the reason why Tim was picked over me, was that Tim did overtime when the company really needed it. I was like “WTF, I’m contracted to do a job, and I do it in normal hours, or stay behind and get it done. Tim wastes time doing person shopping on Amazon, then when his work isn’t done, he volunteers to do overtime at an extra cost to the company”. Nice guys finish last.

Work Hours

I always wonder what is an acceptable amount of breaks because people have their own views on what is efficient. Some people like regular breaks, whereas others prefer fewer but longer breaks.

Often, managers say:

“I’m not bothered if the work gets done”

but it would be strange if someone did 3 hours of perfect work, then left for the day.

Personally, I rarely leave my desk, but sometimes procrastinate by peeking at other team’s work to learn what else is going on.

Derek takes 20 minute smoke breaks at certain times of the day. He will also have a 20 minute break with his best work-mate, then take an hour lunch even though contractually we get 30 minutes. On a good day, he will have 2.5 hours out of his 7.5 hour day away from his desk, but on bad days he can have up to 4 hours.

Now, the amount of output we get from Derek isn’t actually the lowest in the team, although often you can question the quality of work. My view is that since Derek spends so much time trying to avoid work, he isn’t very good at it. So when he actually tries doing work, he has to try and cut corners and cobble something together.

Meanwhile, Colin comes into work late, takes way fewer breaks, but his breaks are longer. He is known to work obscure hours, remoting in at home. When available, he will also claim as much overtime as possible, so he appears dedicated. His output is much higher than Derek, but the quality is often erratic.

If I was a manager, I think I would much prefer people to be around their desks, so even if they are chilling out; they are free to answer questions from team members that are trying to get something done. Ultimately, I think the person’s overall output should be analysed to have evidence of their approach. Low quality, or low quantity is poor; so I think Derek and Colin’s approach needs to be questioned.

Polymorphism

This blog entry probably requires some basic understanding of programming in an Object Oriented language.

A technique in Object Oriented programming is to take advantages of Base Classes or Interfaces to make code more reusable.

So you could define an Interface with a method on it, lets say IAnimate, and you could create a class, let’s say “Animator” which takes in some kind of object that can be animated. So the constructor would look something like this:

private IAnimate  animatedObject;

public Animator(IAnimate animatedObject)
{
_animatedObject = animatedObject;
}

Then let’s say this class also has a method that calls the method defined in the interface.

public void PlayAnimation()
{
animatedObject.Animate()
}

As long as your defined objects implement the IAnimate interface, you can pass them into the Animator class. The Animator class doesn’t need to know anything about what objects implement it. It doesn’t have any dependency on Cat, Dog, Fish etc. In the future, more animals can be added to the program, and the Animator never needs to change. Simple to understand for a Junior Programmer.

Colin came up with a class which completely rendered this concept pointless.

private Dog _dog;
private Cat _cat;
private Fish _fish;

public Animator(IAnimate animatedObject)
{
_dog = animatedObject as dog;
 
_cat = animatedObject as cat; 
 
_fish = animatedObject as fish; 
}
 
public void PlayCatAnimation()
{
_cat.Animate()
} 
 
public void PlayDogAnimation()
{
_dog.Animate()
} 
 
public void PlayFishAnimation()
{
_fish.Animate()
}

So with this design, we are still passing in an interface, but there is some bizarre casting going on in the constructor, and we save the result to the appropriate variable. We also have a method for each implementation of IAnimate, so there’s a separate method for Cats, Dog, Fish, even though they do exactly the same thing. Whatever class actually uses Animator has to know what type of animal it is for, because creating an Animator for a fish, then calling PlayDogAnimation() will crash. In the future, when new animals are added, Animator will not work without extra code changes. You would need a new variable and new method for each type of animal.

Any advantage that Polymorphism brings has been completely eradicated with this design. All that has happened is that there’s extra complexity and crashes. It shows a complete misunderstanding of such a fundamental technique to Object Oriented Programming.

Inverted Logic

A few months back, Derek was writing some logic to decide if something was valid or not. This would then be used to display warning messages to the user.

He ended up writing a method like this:

public bool
NotValid()

Now, if you aren’t a programmer, this sounds fine. But when you come to use it, things can get confusing. When you want the Valid case, you will have to invert it using the logical negation operator “!” (i.e. “!NotValid()”) which then reads “if not not valid”. A double negative.

I flagged this up on his Code Review, but Derek left it in. This led to a bug, because he had used it in a method and didn’t use the “!” to invert it.

I was at his desk the following week discussing a problem he was having. I saw a method that was work in progress which had the same double negative problem (still hadn’t learned his lesson). I was about to comment on it, but then he turned to me and said “I know you don’t like that negative logic, but I find it easier”.

How can it be easier? Having a method called “Valid()” then using “!Valid()” if you want the opposite is perfectly fine. It is easy to understand and easy to read: “Valid” and “Not Valid”; completely fine. Versus “Not Valid” and “Not Not Valid”; confusing. I don’t see any aspect you can use to justify it is easier.

On a similar note, there was a Code Review from Colin that was to check one date wasn’t greater than the other. The common validation scenario where you have two Date controls, and want to enter a date range; From date, and To date.

public boolean IsFutureDate(DateTime startDate, DateTime endDate)
{ 
  return !(startDate <= endDate) 
}

Here, the method signature is fine (maybe their naming was better than mine), but the logic inside is using a negative approach which makes it hard to read. “Start date not less than or equal to end date”. Why not simply “start date greater than end date”?

return startDate
> endDate

Lessons Learned

Logic within programs can be naturally complex, but the skillful programmer can break this down into small, manageable functions. There is no point adding in additional complexity by writing simple statements in an illogical, hard to read way.

Time In Ints

I wanted to keep my stories anonymous, but there is a very specific developer I want to write many stories about. I want to attribute those stories to the same person to build up his character, so I’ll always blog about him under the name Derek. Any other story, I’ll blog under the name Colin, and collectively, maybe Colin’s persona will match the insanity of Derek.

So here is a Colin story.

Just a background for those not familiar with C# or programming languages. If you want to store a number, you will often use the type “int” which is short for Integer; a whole number.

Colin had to program a countdown timer which took the time from a config file. Here was the Property he chose:

public int TimeInInts

The second word is the return type, so as we begin to read this, we know the Property returns an integer. What does this integer represent? A time. Fair enough, but in what unit? Ints.

Ints!

Absolute useless! Is it hours, minutes, seconds, milliseconds, something else? We don’t know without reading more code. That’s the telling sign of a badly named Variable/Property/Method; if you have to dig deeper to work out how you can use it.

I assumed it was minutes, but I look in the same file and there is another method.

public int GetTimeInMinutes() 
{
  return TimeInInts * 60 * 1000; 
}

So GetTimeInMinutes uses TimeInInts in order to calculate minutes, so TimeInInts isn’t minutes.

Hang on, “* 60 * 1000“. That’s the calculation to convert minutes to milliseconds. So TimeInInts IS time in minutes and GetTimeInMinutes returns the time in milliseconds!

What.

The.

Hell.

How can you even write that, use it, and possibly get the feature working when everything you are dealing with has the wrong name?

Unbelievable.

Anyway, if you wondered where my username came from; now you know.

Lessons Learned

Always try to come up with the best variable names possible. Anything ambiguous, misleading or wrong will lead to confusion and bugs.