Bad Quality Project

Recap

The Code Analysis report gives metrics on code quality, and we want to improve the overall quality of our code over time. We put great importance on the Code Analysis report even though we haven’t done anything to address the issues. It’s just that certain managers keep hyping up how important it is.

New Project

An important project is going into the Main branch which will be released next month. I was asked to review the code. I was originally told the review would be sent by the end of the day, but it was delayed a day. I proactively found their code branch and saw it was out of date by a few months, which isn’t a good sign. (They should be merging Main into their project branch to minimise code conflicts).

So after they finally updated it to the latest version and sent the review, a manager said they “really need to get the project reviewed and approved by 5pm so they can start the regression testing tomorrow”. 

I jestingly asked if the expectation is that ‘I click the “Approve” button without looking at the code’ – if I criticised their code in any way, then they would miss the deadline since they would have to make the changes and retest those features. It was true though, the deadline was about 3 hours time.

Since we are more ethical than that, a few of us ripped it apart. Initially, we ended up with around 100 comments which I think is the largest I’ve seen on a review. The next day, we added even more to get it up to around 130.

If they really wanted to cut corners, then it’s up to the managers to decide whether they want to skip the review and check it in anyway.

I thought the pressure they were putting on the reviewers was unfair. It wasn’t just that initial message (“need to get the project reviewed and approved by 5pm”), but they kept on sending direct messages to each of us asking when we will be finished because we are pushing time.

Code Critique

Instead of moving some old code to a new location, and refactoring it so it is reusable with the new features (utilising C#’s object-oriented style (base and derived classes), then fixing up the existing issues to meet our coding standards)… they decided to copy and paste it instead.

So now we have some old, bad code; and some new, bad code. When the Code Analysis report runs, it is going to flag up issues in both files, and if it successfully identifies the duplication, we will have loads of Code Duplication errors too. Brilliant.

I expressed my concern to the team’s manager and he gave me a response along the lines of “we were a bit pushed for time, so decided to take the easy route”.

On the review, I questioned some of the messages we were displaying to the user, and the response I got was “this is an existing message”. Well, fair enough to a point, but why didn’t anyone question if it should be changed. You are revamping that area of the system, why don’t you improve the quality and user experience whilst you are there? The project has enough testers allocated to it, and they will be testing these changes in detail. It’s a great opportunity to rewrite it.

There were several files where their response was “actually, this file isn’t used, so I’m deleting it”. I’m not sure how that can happen with several files. Why only realise now?

Closing Thoughts

After all those discussions about how our team is supposed to have Quality as our main focus, then we proceed to make a mess of things!

It would be nice to have some consistency. If I check in some code that shows up on the report, then I get an email from someone. It doesn’t matter if I have reduced the problem count by 50, but added 1 new problem – they make out it is a problem. 

Why is someone manually emailing about what a report shows, rather than having it as an automated check as part of the review? Maybe it’s a licencing issue. If they were that serious about this report then they would pay for it and enforce it properly.

There’s been a few times where projects were supposed to be merged in, and the people doing the code review didn’t like the code. If you are doing a project, maybe get an expert to review your code earlier in the project. That way, you don’t get loads of issues to fix on the day you are supposed to merge it.

Work Hours #2: Working From Home

When we worked in the office, I found that if I had finished my assigned work and it was towards the end of the day – I was reluctant to start something new, but I wouldn’t have the guts to go home (I thought my manager or colleagues would question why I was walking out 30-60 minutes earlier than you should). So instead, I’d stay at my desk and talk to a colleague or browse some programming websites, casually look through recent bug reports, or bug fixes. But either way, I wouldn’t be working.

The thing is, I would rarely walk away from my desk during the day. Yet there were other people that would go for a break for 10-20 minutes on a regular basis. Then there was Derek that took so many breaks – he would only work for about 4 hours instead of his contracted 7.5 hours per day! I explained this in my previous blog on Work Hours.

When I thought about it, if it was acceptable for Derek to work like that, there shouldn’t be anything stopping me walking off after 4 productive hours at my desk.

When we moved to permanently working from home, I had my usual office mindset where I would rarely leave my desk and wouldn’t shut my laptop off until it got to 5pm. Then a few months in, I started taking a few more breaks, but still wouldn’t leave until 5pm. Sometimes if my colleagues asked for help around 5pm, I’d happily help them out for 30 minutes or so.

If you think about it, I was probably working more at home. There’s no one distracting you by walking past your desk, no temptation to look at what other people are doing, no office banter to distract you, no one coming to your desk unannounced.

A few weeks ago, it got to 4pm, and I started wasting time – having a nosey at bug reports and recent code changes. Then I thought “what am I doing? Why am I wasting time until it gets to 5pm. No one is going to see me leave”. So I walked over to my PC and started playing games. I left my laptop on, and if people messaged me, I would rush over and respond. Sometimes they wanted help and I would call them. Other times they were sending me unimportant stuff, so back to games.

Over the last month or so, we have cut down the amount of bug fixes we are putting in the releases. I have quite a few fixes waiting to be checked in, but I’m not allowed to commit it because of the restricted releases. So I’m on target for the work I’ve been assigned. I’ve sent a message to my manager, requesting more work. While waiting for a reply – I go play some games. He could take up to an hour to respond, because he could be in a meeting. No point wasting time and waiting around.

I have a high standard of ethics, or like to think so. I don’t see a problem with this new way of working. I’m getting the assigned work done, and I’m having fun. It’s not like I am actually slacking off. Also, I did say I always have my laptop on during work hours and will promptly respond. There’s times where there will be something important and I’ll work past my contracted hours, so it kind of cancels out anyway. I don’t complain if I work 2 extra hours some days to investigate/fix some important issue, because I know I’ve taken the time up front.

Additionally, sometimes you have those meetings where you don’t really need to participate so you end up casually listening. So sometimes I have whipped out the 3DS or Kindle and played some casual games whilst listening to the meeting. Multitasking.

Some days I’ve kept a book at my desk. When there’s something that blocks me for 5 mins or more, like I have to rebuild our software, or I need to wait for someone to finish their meeting – then I can read.

Managers are always going on about how important it is to have a good work-life balance, and how we need to be aware of our mental health. You may as well take advantage of the new way of working. It has its disadvantages but it also has advantages like this.

Dumb Code List

There was one code review I was having a nosey at, and the Lead Developer tore it apart. It had so many mistakes in it, I thought I’d make a note of the best ones.

  1. string.IsNullOrEmpty(null)

We are passing “null” into a method and asking “is this null”? Yes it is. 100% guaranteed to be.

  1. new FilingContainer(fEvent) ?? new FilingContainer(fEvent, null); 

This code means: “create a new FilingContainer. If this is null, then create a new FilingContainer (but with slightly different parameters”

Lead Developer’s comment: “how does this constructor return null?”

A constructor can only return a new object, or throw an exception. You can never return a “null” value. So the second part of the statement would never execute.

  1. There was a massive constructor defined with many parameters of the same type. So like

public  Ticket(
String firstName,
String secondName,
Int userId,
String userName,
Int jobCategoryId,
String jobCategoryName)

But then when he went to use it, he had the parameters in the wrong order:

new Ticket(
firstName,
secondName,
jobCategoryId,
jobCategoryName,
userId,
userName)

Lead Developer’s comment: “You are setting the JobCategoryId to something called UserId! Which is a hard coded value from your development database, and cannot be relied on to be the same in Live! You are setting the JobCategoryName to the User’s Name!”

So not only were the parameters in the wrong order, he should have queried the database for the correct JobCategoryId in his code. But he had queried it manually, then hardcoded the value in his code, so it would only work on his machine.

  1. Title + surname.toTitleCase( ) + “ ?” + firstname;

Lead Developer’s comment: this is “MrSmith ?DAVE” Also, is title-case correct? What if someone’s name is McDonald? Should it be Mcdonald?

Pretty much everything is wrong with the output!

  1. Lead Developer’s comment: “Sex or gender, use one or the other”.

Some files they had named a variable “sex” and in others “gender”. Obviously, only one name is correct.

  1. Age = (DateTime.Today.Year - messageDetails.User.DateOfBirth.Year).ToString());

Lead Developer’s comment: “Born 31/12/2000 Today 1/1/2001… does not make you 1!”

I’m not sure what sort of people think age is a calculation based on year only, but with that lack of attention, you really need to consider if this is the job for you. Programming is about thinking logically.

  1. FullName = Title + Surname; 

Lead Developer’s comment: FullName is just “MrSmith” (nospaces) is that correct? 

They reply “yes there will be no space”

Is that really in the requirements? I highly doubt it. Are they that stupid that they think the Lead Developer is asking how their code currently behaves, rather than questioning if the behaviour is actually correct?

  1. There was a variable named “refferalDescription”. Which is an incorrect spelling.

Lead Developer’s comment: “referralDescription?” 

They reply “refferalDescription is new property for code which is used to display in the referral document.” 

I was thinking “How can you write that response and not realise that the spelling is different in his comment to what you wrote?”

  1. Referral.Code ?? null. 

This code means, If it is null, then use null. A completely redundant line of code.

Missing Feature

Cassie: Can I ask you a question?

Me: What is it?

Cassie: A user has complained that a feature has been removed in version 7.5, can you check why? I’ve recreated it on my system.

I check on my system and the feature is there. I look at the code and it is specifically for users in England.

Me:  Is the Region of the organisation set to England?

Cassie: It will be, also I’ve recreated the issue on a second system which is set to Scotland

Me: Yes, this is an England only feature

Cassie: Ah, doesn’t explain why it doesn’t show on the other test system though

Me: Can you check the region?

Cassie: It is Wales

Me: This doesn’t explain why the user doesn’t have the feature though.

Cassie: They are in Scotland

Now I’d like to know if the user has really complained that a feature has been removed. They should never have had it in the first place. I think there has been some miscommunication here, and the user has probably read about the feature and questioned why they don’t have it.

University Learning

When you tell people that you went to university and studied some Computer Science related degree, people think you are a genius.

They also assume you must be a better programmer than self-taught developers, but that isn’t the case.

I heard someone rant on a podcast that some people in the industry try to “gatekeep” by saying that “Software Engineers” are university taught, whereas self-taught people are merely “Software Developers”. So a “developer” is a  pejorative term. I’m not sure if that is an American thing, because I have never heard of that before, and it’s not something I have witnessed in England. Personally, I use the terms as synonyms, and if I did hear someone trying to differentiate between them, I would certainly argue against it.

I was taught programming at College and didn’t learn much. I went to University and the standard of teaching (when it came to programming) was actually very low. 

Part of the problem is that you do some coursework based on something you are studying on the course, and by the time your code is starting to look complex – then you have completed the task. Your attention moves onto the next set of coursework and the pattern repeats. So essentially you are writing code using basic constructs like “If statements” and “For loops”, then moving onto another basic task using the same constructs.

Another problem is that when the coursework is graded, they mainly care about the written report. Often you had to simply demonstrate your program running to prove you didn’t just pretend.

The other problem is that it didn’t matter if your code was efficient or clear. As long as you could explain the theory, then it is fine. You were encouraged to litter your code with code-comments, explaining every line. When you actually learn how to write good code, you read about the idea of “clean code”, which means writing code in a logical and easy-to-read manner. This is the opposite of how universities teach it.

Your code at uni is a mess. There’s no automated tests, there’s no design patterns, the methods and classes aren’t split into small logical groups. It’s loads of messy code crammed into a few files (or maybe just 1 file); it’s not readable or very maintainable, and then the file is double the size because you have spammed it with code comments.

At that point in my life, I found that I learnt the most in my first job, simply by making mistakes and thinking about what I could have done instead. After I left that job, I spent time reading and working on the most ambitious idea I had. It was at this point, I felt that I finally understood how to be a developer.

Colin Promotion

I did mention that Colin joined this team because he saw it as a good opportunity for promotion. I thought he was absolutely nuts because he didn’t even deserve his Senior position, yet he was aiming to be among the “big dogs” in the company (often known as Principal Developers). I’m sure you will agree that he has ‘ideas above his station’ if you read the stories I write about him.

Well, now I have egg on my face – because he has been promoted yet again, and I’m still not a Senior. He is two ranks above me now, can you believe that?

I sent messages to some of his old colleagues to inform them of the news. Usually, if you tell someone about a promotion, you will get neutral responses like “ok, now I know who to talk to about X”, or maybe some positive responses like “brilliant, he really deserved it”. Out of 7 responses, what percentage were positive or neutral?

0% positive, 0% neutral. 100% negative. Some people even accused me of lying. I don’t normally put swearing on the blog, but I’ll include this one. Here are the responses:

“What!? I don’t know what to say to that.”

“haahaha are you fucking joking?”

“wtf..what is wrong with the managers?”

“Be prepared for more mistakes”

:joy:Is he your team leader? That should work out well :face_with_rolling_eyes:

 “wow, how did that happen? God help the world”

 “Don’t lie”

I’d like to point out all I said was “Colin has been promoted”. I didn’t tell them what I thought, or preceded it with any statement to influence their opinion.

So the promotion definitely doesn’t have the backing of his colleagues. It makes you wonder how a promotion can go ahead when it is unanimously deemed a bad idea. Well, I’ll keep trying to think of how to make an accurate and fair promotion process.

I did have a conversation with my manager to ask why my promotion didn’t happen. He gave me the usual response I’ve heard in the past to do with budgets and no free spaces for that role. I pointed out that Colin has just vacated a Senior position which I can have.

He was still adamant that the budget was gone and there’s no free spaces anyway. He promised he was doing as much as he could to promote me, and he said he was constantly asking the Head of Development to promote me.

I have to put faith in him, because later that day the Head of Development sent me a message saying how she understands “I am doing a great job and it is appreciated”. 

The funny thing is, I’m not even doing a good job at this current moment. I’ve had lots of annual leave, and haven’t had much work assigned since we decided to cut the scope of our recent releases. I’ll probably write about this in more detail shortly.

Stupid meeting times

I had a 30 minute meeting starting at 4:30 which overran by about 30 minutes – so it actually finished at 5:30. It’s a problem when I finish at 5, but that’s work I suppose.

The next day, I logged in 5 minutes late at 9:05. I received a message from the Tester on my team:

“are you joining this meeting?”

“What meeting?”

So I opened my emails and saw a request for a 9am meeting which was sent at 5:45pm last night. Ridiculous.

We have this concept of flexible working so the “core hours” are 10-4. This is because you can basically work 8-4, 9-5, or 10-6. Therefore meetings can only really be scheduled between 10-4 if you want everyone to turn up. A 9am meeting isn’t bad for me, but it is when you haven’t had the chance to even acknowledge the invite, and haven’t had a chance to even consider or act upon what was discussed the day before.

The Invoice

I was working on fixing an important bug and one of the managers called me to ask how I was getting on. I gave him an update, and he said that he had been informed that a customer had complained about this particular bug – and even had sent an invoice to try to bill us for time lost  because of this bug. With their calculations, they wanted £6k in compensation.

I often think about the impact of software bugs. Ideally, the software should always work and help the user do their jobs. A bug will cause frustration and slow the user down. If a company has loads of users that are all impacted by the same bug, then the impact is multiplied. If we end up taking a month to fix it, then the impact is multiplied over that time.

We had no intention of giving into the user’s demands though. We were just fixing the bug as urgently as we could.

Wrong Rob

I was on annual leave, but there was supposed to be a meeting to update on my project. I knew that Rob could do the update himself because he had all the information to pass on to the managers. We have had multiple meetings with these managers, so they know who we are.

I received the following email about the meeting.

Hi,
We had Rob Anderson on the call just now but it’s clear we needed Rob Baker – I will reschedule as soon as possible.

Thanks
Danny

Brilliant. I assume it was simply clicking the wrong Rob in the address book. The thing is Rob Anderson attended the call and was confused what it was all about. I wish I was there to hear the conversation that took place. It must have been pure bantz. I give this the “Email of the Week” award.