YAGNI / This Code Could be Useful In The Future

I remember the days of being a junior software developer where you write code because “it might be useful at some point in the future”. The thing is, this means you end up writing way more code than is necessary, and could lead to overcomplicated designs.

There is a concept called YAGNI: “You Ain’t Gonna Need It”

Swizec sums this up well here with a strong hint of sarcasm…

And that brings us to the other YAGNI: Building stuff you think you’ll need before you need it.

You think of a wonderful abstraction. A wonderful little feature that’s gonna help oh so much in the future.

You build it.

Months pass. You tweak the code here and there. Nobody’s using it, but you have to maintain otherwise it’s gonna break. Whenever you implement a feature, you have to think about how it impacts this code you’ve got.

It’s slowing you down. Making you think. And you aren’t even using it.

But one day! One glorious day, your PM gives you a task. A task from the gods. You are about to use this code you predicted 6 months ago!

You are a god of prediction! You knew it all along!

https://swizec.com/blog/dry-is-a-footgun-remember-to-yagni/

A simple example I came across recently was this:

public static bool IsModalDialogOpen()
{
  Boolean isModalDialogOpen = Application.AnyModalDialogOpen;
  if (isModalDialogOpen)
  {
     throw new DialogException("A dialog is already open");
  }
  return isModalDialogOpen;
}

So they have written a method that checks if a dialog is already open. If there is, then an exception is thrown (depending on their error handling, it would either crash, or display some kind of error to the user), otherwise it returns false. Since this is new code, the only places that called this method was in his new code. However, he never checked the return value of the method. Since it is defined as a Boolean/bool then it could only return true or false, but his logic only ever returned false… and all the calling code never used the value. Our exchange went like this:

Me: Why do we have a return type for this method when the return value is not used above?
Developer: If any one in future want to know the value it will be useful because of that I have added.
Me: https://rules.sonarsource.com/csharp/RSPEC-3241
       Also: it will never return true
Developer: OK, removed the return type.
Me: It needs an appropriate method name now

So at first he says it will be useful in future – which is definitely false – YANGI definitely applied in this situation. Then when he agreed that the boolean value wasn’t useful, he left the method name as IsModalDialogOpen which implies it should return a boolean. In the end, we were left with this:

public static void ThrowExceptionIfModalDialogOpen()
{
   if (Application.AnyModalDialogOpen)
   {
     throw new DialogException("A dialog is already open");
   }
}

In another recent change, there was this humorous exchange between two Senior Developers, George and Paul:

George
This looks like dead code...

Paul
Yes on the off chance it was used, we now redirect to the new stored procedure

George
How did you test it?

Paul
We spoke to the Data guys and they said it wasnt used, so no testing required. I am sure.

George
So you leave it in case it is used, but don't test it because it isn't?

Paul
Yes sort of. In case this was ever ressurrected the change was there for them to use

George
It won't be

So Paul had been migrating a database “into the Cloud”, and so went through all the features where that old database was used. He found some old code which he believed to be obsolete, but made the change there anyway. George then flags it up, and Paul says he changed it just in case it was actually used, even though other developers also agreed it wasn’t used. He didn’t test it because it wasn’t used, but the code is there if someone decides it should be used, but it might not actually work because he never tested it.

You could make a variation on that Swizec quote at the start of the blog. It’s extra code that you have to maintain and slows you down, but the day that code is finally used; Paul has saved people time! (Apart from all the developers who will end up looking at the obsolete code in the meantime).

Coding Tales #1

I haven’t written many blogs talking about code for a while, but my notes are stacking up; so time to post some blogs discussing code! I’ll try to explain them the best I can for the non-programmers, or those that find C# hard to understand.

There was one bug fix where the developer took several attempts to fix it. Taking several attempts to fix a bug illustrates that you either – don’t have a good understanding of:

  • the requirements,
  • or the codebase that you are changing.

If you make a change and it introduces a bug, then you make another change and it introduces another – instead of playing “whack-a-mole”, it’s worth considering if you are actually writing the correct fix. From my experience, you can often just take a step back, ask for help, and come up with a much simpler, and less risky fix.

In one of their changes, I saw evidence that it was possible that they didn’t understand what they were changing, or taking care in their work.

case ParentRecordRelationship.Grouped:
//when a record is grouped, the associated text is NOT altered - so this method should not be called
   throw new ArgumentException(string.Concat("Invalid parent record relationship supplied: ", parentRecordRelationship));

So we have a comment, explaining that the existing requirement is that the “associated text” is not modified, so it is completely invalid to call this particular method which would change the associated text (other code not shown). Therefore, we throw an exception which would cause the system to show an error to the user.

The developer changed the code to this:

case ParentRecordRelationship.Grouped:
    newAssociatedTextValue = CreateAssociatedTextValue(string.Format(_groupedWithFormatText, parentrecordOriginalTerm));
     break;
//when a record is grouped, the associated text is NOT altered - so this method should not be called
//throw new ArgumentException(string.Concat("Invalid parent record relationship supplied: ", parentRecordRelationship));  

So now he has allowed the text to be updated in this situation. Unless that really is a change in requirements, then this is the wrong thing to do. Also, instead of deleting the code that throws an exception, he has just “commented” it out (which is why it now shows in green), and also left in the actual comment saying that this scenario is invalid; which now adds to the confusion when the next developer reads it.

To me, that shows that the developer is uncertain it is the right thing to do, otherwise you would remove the comment and the code. Just “commenting” the old implementation out basically acts as a reminder that you might want to restore it. (You could easily just look at the history of the file to see the old code anyway, it isn’t like it is lost if you actually delete it in the first place.)

He also added to the confusion with this change

//take the 1st 12 characters of the format text
itemValueStartsWith = _combinedWithFormatText.Substring(0, 12);

it is now

//take the 1st 12 characters of the format text                      
itemValueStartsWith = _combinedWithFormatText.Substring(0, 9);

This is a good example of why writing comments for simple code is bad. The comment explains a requirement that it should take 12 characters, but you can see that it takes 9.

To me, this shows a lack of care, and lack of attention to detail – which are basic traits that I think are important for good software developers.

T-Shaped Engineer

Recently, I came across a new jargon term, the T-Shaped Engineer. I think the general idea is that you used to have a “specialist” or a “generalist”. Now the perception is that it is good to have a compromise between them.

A generalist is like the adage, “A jack of all trades, but a master of none”. You learn new technologies but not achieve a deeper understanding – since mastering anything requires you to dedicate massive amounts of time to the craft. When challenging work needs to be done, engineers with deeper knowledge are needed; aka the specialist.

Specialists devote their time to a narrow set of technologies. They probably don’t learn the newest technology unless it falls into their specialist domain.

Work was often prioritised based on what resources were available. If a feature required more developers with a certain skill-set, and they weren’t available, then the development had to be postponed. If the company hires more developers with the required skill, when priorities shift, you can end up with spare/unassigned developers. This was a hard balancing act for the Product Managers. Sometimes, the developers could end up being asked to take tasks they wouldn’t normally do.

This has led to a new kind of engineer – the “T-Shaped” engineer. This describes a person whose knowledge distribution looks like the letter T. The horizontal line represents a broad knowledge in multiple areas, and the vertical one represents a specialisation of a topic.

From I-Shaped to T-Shaped – Why DevOps Professionals Need to be Multi-Skilled

Theoretically, having a full team of T-Shaped engineers with their own specialisation means that work can be prioritised. Whilst they may have a broad general knowledge, managers need to remember they can’t perform exceptionally everywhere. The concept isn’t a silver-bullet.

If “Pi-shaped” and “comb-shaped” developers exist, then you would think those would be the developers to hire. I suppose if you do find them, then they will be rare and demand large wages.

References:

What are T Shaped People? Youtube video

https://alexkondov.com/the-t-shaped-engineer/

https://www.devopsinstitute.com/from-i-shaped-to-t-shaped-why-devops-professionals-need-to-be-multi-skilled/

Recruiting Graduates #4: The Applicants

Introduction

Other blogs in this series Recruiting Graduates #1, Recruiting Graduates #2, Recruiting Graduates #3.

Fredrik Christenson (I didn’t make a note of the exact video it was from) once stated good developers can be judged by:

  1. How they reason about a problem
  2. Their way of communicating with people
  3. How they voice their concerns/make decisions. Asking the right questions, pushing back in the right circumstances in order to get the right requirements; have “Guiding conversations”.

I think the general set-up of our interview can lead to this. The first part is for them to present the take-home test we gave them. They have to write a simple application with a UI and present data it retrieved through an API. The second part is to do some live coding, and define a few technical terms. Then the final section was some behavioural questions led by Colin.

Colin’s Update

Colin had interviewed several candidates for the Graduate Developer role in India. I asked him if the interviews had been going well, and he said the Graduates “couldn’t write code“. He said there was someone with a Masters degree, who then said they

just learnt the course material a few days before the university exams then instantly forgot it”.

applicant

I guess at that point in a programming interview, when the interviewers are asking why you can’t write any code, then there is probably no way to get yourself out of that situation.

Saying you are forgetful is just instant rejection, although points for honesty, I suppose.

My Preparation

When it was my turn to do some interviews for the UK-based applicants, I was worried. I hadn’t been given any training or guidance, and the questions we had come up with were poor and I didn’t feel comfortable asking them. I was also scrutinising their applications and was trying to come up with questions based on their descriptions of their university course, or small amount of work experience. However, when it actually came to interviewing, there was actually someone else leading the interview, and Colin turned up too. So Colin had misled me that I would have a big part in the interviews. Also, Colin said he never asks questions about their CV, and sticks to coding or behavioural questions – so I’d wasted my time.

To be honest, I’d wasted my time anyway – because the first 2 interviews I was lined up to do were cancelled with half a day’s notice. Not sure if the HR team were slack or if the candidates backed out late. Colin speculated that people were dropping out because they couldn’t do the take-home test we gave them (which they would present in the interview). Maybe that means it is a good test. Colin said that he didn’t give the take-home test to the applicants in India, which could explain why he ended up interviewing people that couldn’t write code at all. 

He didn’t give them the test due to our Indian Senior Developers saying the test was too hard for Indian Graduates to do (see Recruiting Graduates #1). To me, this seems like an admission that Indians are generally inferior, especially if they are happy for us to give the challenge to the UK applicants. This seems a very strange admission indeed. 

The Cheats

Colin also said many of the applicants were caught cheating. On the questions where they were asked to define technical terms, they were reading the definitions from a website. He said some of them hid it well in terms of eye/arm movements (since they had to have their webcams on). However, the phrasing was a bit suspect, so Colin Googled the response and found the website they were reading it from.

He said another candidate seemed to pause whilst they thought about a question, then all of a sudden pasted in a block of code plagiarised from a website. 

“HERE’S SOMETHING I PREPARED EARLIER”

fictional quote, inspired by the classic Blue Peter line.

Scoring

Colin did actually hire a few of the Indian applicants though, but when I looked through their Interview Feedback, the interviewers were scoring them 2/5 on most sections, but then a few sections were 0. So the scores were clearly a fail but we were hiring them anyway.

I also noticed that candidates who provided the programming answers in Python were scoring lower, even though, when I looked at the code – it looked correct. The problem is that it is a C# job and the interviewers only know C#. However, many of the Universities we were targeting used Python as their main language and we knew this up front.

So for the same job, we have a stricter process for UK candidates, and are also biassed towards C# candidates even though we only ask for knowledge of programming and don’t require prior C# knowledge.

Applicants

Time Magazine’s “Person of the Year 2006”

I did read through the CV’s of the people we hired. We hired Time Magazine’s “Person of the Year 2006”. I did wonder if Colin or the other interviewers had read that because it’s an instant red flag to me. It doesn’t sound like a legitimate claim, but then if you research who the award went to, it went to YOU. To put that on your CV is a joke though, unless you really did contribute significantly to online content. So if the candidate couldn’t justify it, I would just reject them.

Game Developer

For one of the guys that dropped out, I had some concerns with his application. Maybe he was legitimate but I think his claims weren’t backed up by the evidence he provided.

The first thing I noticed is that he stated “deep knowledge and proficiency in Java, Python, HTML, CSS, and JavaScript.” but I didn’t see any examples on his GitHub page, and there’s no commercial experience listed in his Work History. I wanted to ask him:

 “Can you tell me what kinds of applications you have developed using these?”

My proposed question

He did say that his GitHub profile “included group projects.” but which ones were Group Projects? Working on group projects is fine, but I think it needs to be clearer so you can judge what he has done on his own. If they had used source control properly, you would see the commits by each user – which is where I saw the major red flag. All the commits were actually done by someone else. The only thing the applicant had done on the entire repository in his name was to initialise the repo, and fix a build issue at the end.

Your profile has another “contributor”, can you tell me who that is?  – They have committed all the games to this repo.

My proposed question

Many of the games on the repository were Unity games, and I downloaded them, looked through the source code and play-tested them. For someone who had done a Games Degree, I was disappointed in the quality. Most of them I felt I could have made, and there were some really basic game design mistakes, and he said one of his University modules was “Advanced Game Design”. I wanted to ask this:

“If you worked on these games again: to fix bugs or improve the game design, what would you change about them?”

My proposed question

Example answers would be: In the Platforming game, you can stick to walls and the sides of platforms if you hold down the arrow key – which seems like an obvious bug. In his Snake game, it wasn’t clear what was happening, and you can move over enemy spawn positions to instantly kill them as they spawn in. 

For his experience working in Home Bargains, he claims he

“Engaged with customers and built relationships to advise customers on products they’d like to know about”.

Candidate, clearly blagging

Home Bargains is basically just a supermarket, and he is hyping it up as if it is a sales job and personally gets customers returning. That doesn’t sound like a valid description of this role. I shop at Home Bargains and I don’t ever recall seeing anyone talk to the staff there unless there is some kind of dispute. 

Python, Machine Learning Expert

The first guy I actually interviewed was about to finish his Masters degree, and has a Machine Learning degree. He seemed decent enough at Python to be able to do the take-home assignment, which he said he rushed through in a few hours because that was the only time he could dedicate to it. However, he seemed to struggle on the live coding part. I tried to prompt the candidate to talk us through his code, but he seemed to prefer coding in silence. I even helped him out by pointing out a bug in his code, but he ignored me, then wasted another 5 minutes working it out. I did wonder if he understood what I was saying. His CV claimed “Good communication and Presentation skills” but his presentation was full of “erms”, and during the live coding, he barely said anything, even when I reminded him to explain his thought process in order to gain more marks. By our scoring system, he scored very average but he has good credentials and I think there is potential there. You would think with his credentials, he would aim for a fancier job which could utilise machine learning though.

Possible Cheat

The second guy I interviewed got off to a good start. The program he demonstrated looked very complex. He said he was new to C#, and had good knowledge of C++, but he used our task as a good opportunity to learn C#. He actually used ASP.Net with Razor. It was mainly C# but it also contained some Javascript. He explained the program reasonably well, but then suddenly seemed to doubt himself and say that his program wasn’t very good. I assured him it was actually very impressive. However, when we moved onto the technical definitions, he couldn’t answer them very well at all. He really struggled with the live coding questions, then reminded us he is a C++ developer. However, after letting him re-attempt using C++, he still struggled to get something even close to what we asked. I find it hard to read C++ but he came up with some strange approaches and couldn’t explain his thought process at all. It made me think that he got someone else to write the take-home test.

3rd Guy

The third guy I interviewed seemed like a much well-rounded person. He explained his code well, asked questions when he was unsure of what we were asking, and showed he could debug (well, to a basic level anyway) when his code didn’t work. I made it clear to Colin that I thought he was the best candidate, but sadly he got offered a job elsewhere for way more money than what we could offer.

WE HAVE FLEXIBLE 
WORKING HOURS. 
INTERVIEWER 
NO. 
WE DECIDE YOUR 
WORKING HOURS. 
J) 
WorkChronicIes.com 
Fo//ow me on 
GREAT! 
SO 1 DECIDE MY 
WORKING HOURS ? 
WE STRETCH IT AS 
MUCH AS WE CAN. 
IT'S VERY FLEXIBLE. 
b

Employee Profiles: Steve

I found loads of chat logs from work, and additionally found a few quotes I wrote down from various employees. So today we are going to discuss the legendary employee Steve.

“Steve looks like a confused garden gnome that lost his hat”

Adam – colleague

Introduction

Steve joined as a Software Developer, and I think Steve’s carefree attitude meant his code was a bit inconsistent in quality. I liked working with Steve though, he was often quiet and just got on with his work, not really paying attention to anything else. It’s a pro and con really. Work got done (though often you had to prompt him to tidy parts up, or spot mistakes for him to perfect it), but then he didn’t know who many employees were because of the lack of attention around him. When he joined in the banter, he was quite “laddish”. A simple northern lad, Steve was well known to like his food and beer.

I just looked on his Facebook – and under “Political Views” it says “Democratic Alliance for the Betterment of Hong Kong.”

Andy

Sometimes he seemed to be quite unlucky and trivial events became more hilarious. Once, Steve opened up the window to let a butterfly out, and another one came in.

He always wore a t-shirt and even when the aircon was chilly, he loved opening the window. Sometimes even had the fan on whilst sat next to the open window.

Me 13:13:
how come Steve has a different body temperature to everyone else? Sometimes I think he isn't human
Andy 13:13:
haha, has he got his fan on?
Me 13:14:
he has the window open. Last week the air was directed at us so we were freezing. Now we have got him to open a different window, it's not so bad. Although, after a while, it does get freezing but Steve insists he isn't cold. Meanwhile Liam just said "I'm wearing my headset to keep my ears warm"

Knowledge of Colleagues

“I know people that are relevant to me”

Steve
Matt: "Do you remember Colin?"
Steve: "No, of course I don't"

“out of all the companies I’ve worked at before, I can only remember about 2 names”

Steve
Me: "Why is Simon leaving?"
Steve: "I don't care"

After our team member Paula moved up to Scotland and worked in our office there, Steve asked if Paula “was on her own, or if there was someone else on her team up there“. Paula is on our team, therefore if someone else was on her team, they would be on our team.

Charlotte asked Steve who wanted all these database changes and he said “John Bundy”. There is no colleague called John Bundy, and there never has been.

Matt: "The documents work item needs moving off the board because the Documents team are doing it"
Steve: "who is doing that? is it Gary?"
Matt: "No, it's Tony. You emailed him about it last week"
Steve: "Oh yeah, I did"

In his update, Steve once said “A chap called Jon Reaves has made some changes”. Jon had worked there for several years and is well known to everyone. Saying “a chap called” suggests he had never heard of him and thought he was new.

Food

Having 2 glasses of wine a week is unhealthy. You should be aiming for 30 units a week, mainly from beer.

Steve

“I’d rather eat my own feet than a KFC”

Steve

“Giving up beer and pizza is never a good idea”

Steve

Tracey was explaining how she went to London and had a fancy meal in Gordon Ramsey‘s restaurant. Steve chimes in:

“I went to Sheffield and had a kebab”

Steve
Matt: "I tried loads of stuff in Vietnam, no idea what it was"
Steve:[loud and affirmatively] "Bollocks"

It could have been testicles, Matt was explaining the interesting and different meals they have there, but it was funnier the way he said it like he had no doubt it was that.

“Four pints is what I call breakfast”

Steve

Steve was complaining that the office canteen has had “Toad in the Hole” for 2 days running. I said “I bet you ate it anyway”. Then he replies in a passive-aggressive tone:

“what else am I gonna do? eat the vegetarian option? Not likely.”

Steve

We once had 2 offices located close together. Our team had moved to the other office but we received a mass email from Mark stating he had brought cake in and placed it in the kitchen. Steve started walking to our kitchen (in the different office), Matt told him it’s not in that kitchen… but Steve checked anyway! He was desperate for that cake.

“Chickens come from seed which comes from oil”

Steve

Matt was originally talking about cars. Then Steve said all food comes from oil, then said that. I was instantly lost.

Software Development/Attitude to Work

“Matt! Myself and Phil are having a bit of a disagreement, and it’s about to turn to blows”

Steve
Matt: "Steve, have you done your Information Governance training?"
Steve: "I did it last year"
Matt: "what does the email say?"
Steve: "It said it is fine"
Matt: "Read it again"

In our team “Retrospective” meeting, we had to vote for “Team Member of the Sprint”. Steve voted for me. Matt asked him for the reason and he said

“I was hoping there wasn’t a second round of questioning”

Steve

A few weeks after finishing the Online Request project:

“Do you know how to switch on ‘Online Requests’?”

Steve
Me 13:41
guess how many unread emails Steve has. It's like he has been on holiday for weeks
Dan 13:42:
100
Me 13:42:
way higher
Dan 13:42:
500
Me 13:42:
closer, higher!
Dan 13:42:
I give up
Me 13:43:
550

No wonder he didn’t know what was going on.

His manager, Matt once stood at his desk and simply stated “Steve”, and Steve was baffled. I correctly assumed it was his one-to-one meeting. Even after Matt told him to check his calendar, Steve was still baffled what it could be. Classic Steve. Probably a meeting request in one of his unread emails.

We once had a meeting located in the main office. All our team dialled in remotely apart from Steve. From the video feed, we saw him walk into the meeting room late and say something to Adam.

Andy 12:36:
did you see Steve randomly turn up to the meeting?
funny as can be
Me 12:36:
yeah
Andy 12:36:
Mia had tears streaming down her face
Me 12:36:
why?
Andy 12:36:
cos why did he turn up when everyone else on his team dialled in
Me 12:37:
did he ask Adam if he was at the right meeting?
Andy12:37:
yeah!

“I was thinking of going for ‘Looks Good’ because there’s too many files”

Steve on doing Code Reviews. Too many files gets instant approval.
Charlotte: "what did everyone think of the meeting yesterday?"
Steve: "What meeting?"
Charlotte: "the meeting with Ronnie"
Steve: "oh, that. I'll be honest with you. I wasn't listening. I have no idea what was said"
Steve took an extended lunch break, and then later he went for a long walk. Matt challenged him on it "Didn't you go out for lunch as well?"
Steve said "yes" with a right cheesy grin
Doesn't care.

“Soon, I’m gonna be introducing lots of bugs. I’ve nearly finished my work; and I’m not dev-testing it”

Steve
Dan 16:18:
is he… what!? is he trying to get fired in the same way you'd act like a jerk to encourage your partner to split up so you get to feel morally superior?
Me 16:18:
haha, great example

A similar example…

Matt: "Steve, are you sure these changes haven't broken anything?"
Me (with fake confidence): "Yeah, because he ran the unit tests"
Steve: "Have I? I only ran the build"

Steve wrote a unit test with the following test data (Michael Jackson).

string doesNotContainsNumeric = "you know I'm bad, I'm bad, you know it, I'm bad";

He often used his name in variable names. He was supposed to choose good names before submitting it to review, but he sometimes forgot. Examples:

boolSteve
strSteve
SuppressMessage("Microsoft.Naming", "CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "IOn", Justification = "steve") 
Me 13:53:
user.Surname = "O'Cake";
user.GivenName = "Pat";
Andy M13:53:
i'm sure i've seen that before
Me 13:54:
reminds you of your days in Pre-school
singing children's songs
Me 14:14:
OMG STEVE IS HILARIOUS
Matt googled Pat O'Cake and its a character from Bottom. He asked Steve about it, and he said "I wouldn't Google them all though, sometimes I use pornstar names"

Then a week later:

private const string _vouchingUser = "Bearstrangler McGee"; 
Andy 10:52: 
wtf
Me 10:52: 
Steve special
I never dare search for anything Steve puts in unit tests after he said "I sometimes use porn star names"
Andy 10:54: 
haha
i hope that's not the name of porn star

Then there were some interesting reasons:

Me 14:48:
"CancellationReason\": \"patient has lost keys to handcuffs\"
why is Steve different?
Andy 14:48:
what the hell?
Me1 4:48:
const string cancellationReason = "patient was visiting a massage parlour";
Andy 14:49:
is he checking this stuff in?!
Me14:49:
yes, it's in our branch
Andy14:50:
he's an absolute lunatic

Steve was working on fixing a bug that Matt was also fixing (but we didn’t know it at the time). The next day Matt and Steve were both on annual leave, so Matt had handed his work over to me, and Steve handed his over to Jim. I finished my work, and Jim even passed my code review without even realising the similarity. It’s like a comedy show sometimes.

Steve had completed a feature, but his changes had broken Matt’s last bug fix.

“it worked for my user story”

Steve. It’s like the classic “it worked on my machine” that software developers love to say

Steve completed the work for saving Users to the database. I just tried it and it crashed. We asked him how much testing he did and he claimed it was all working. I showed him and he said “I forgot about that way”. There are only two scenarios, add from existing user, and add new user.

“I don’t think the Database Tool is working. I think it is completely goosed”

Steve

I just caught Steve smurf naming even though in his last code review, Phil told him not to.
So then he looks up Smurfs on wikipedia. He clicks Smurfette and says “I’ll see if she is fit“.

I have no idea who brought a “dunce hat” in, but we decided that if you somehow break the build, then you wear the hat. Steve wore the hat quite a bit.

“I don’t need to wear the hat; I haven’t broken the build. I’ve just broken the product”

Steve

Not sure how he did it, but Steve once sent code to review which had the same title as the previous change he did. It also had the wrong User Story linked to it. (-‸ლ)

I told Steve that he was supposed to roll back one of his work items. After a few seconds he said it was done. I was sceptical. He said that I had already deleted the other part of the change. So I looked, and I hadn’t. He then said

“to be honest, I didn’t even look at it. I didn’t even compile it”

Steve

Miscellaneous

“Any advice that starts with ‘do not expose’ is good advice”

Steve

Liam was telling Steve that an angry resident left him a note on his car telling him not to park there again. Steve then comes out with this…

“Just piss through their letterbox”

Steve

We were playing badminton after work, and Steve said he had to rush off. Mike asked “are you doing something interesting?”. He said his parents were coming over later and he had a massive stash of weed to hide or smoke.

“I accidentally googled porn with my mum on mother’s day”

Steve

He was helping her with a crossword and the clue was “goddess of nature” and he wrote “goddess of mature

Farcical Development: Templates

The Plan

A developer in my team, Isobel, was free to pick up some work, and she knew another team had too many items assigned, so their Team Lead contacted us to ask if we could pick up the work instead. 

I wasn’t opposed to the idea, but we didn’t know what the enhancement was at the time, so didn’t know how long it would take and didn’t know if our Testers would be available to test it, or would the other team test it? It’s unclear, but their Team Lead wanted me to commit and accept reassigning it to us.

When I looked at the enhancement details, there was loads of confusion because it had loads of code changes linked to it, and was logged about a year and a half ago. It turns out, a year ago, a developer had done the work, checked the code in, but then got told to “roll it back” due to lack of testing resources. Then instead of going into the next release, it was somehow delayed a year… Now we have a lack of development and testing resources! 

There was this comment too:

The change will then need to go out as an urgent/emergency release. As the functionality is deemed potentially unsafe, we only have 7 days to get it out, it’s actually 6 now as it was logged yesterday.

1.5 years later… 

There was another item I was assigned recently, and that had similar chaos with how long it sat on our backlog

10th November 2020 - bug logged
15th September 2021 - assigned to the team

No wonder our users are often reluctant to report bugs because we don’t seem bothered about fixing them. Then our release process is also really long so sometimes there’s a 6 month lead time after we fix it.

I'll try and quickly explain the feature: Users can create these Templates which are composed of components. There's these special "calculation" components which use data added to the record and give you a score. Users can add data-entry components to the template which can be used by the calculation components. However, it's not clear which data is used, and we can change the calculation formula at any time; which makes it “unsafe”. You can also group several components together to make a Group Component. So the plan is basically to stop users from adding these calculator components, and they have to use our own Group Components which will have the calculator and the prerequisite data-entry components with it. For existing templates, we just have to show a message, telling the users their template isn’t recommended to be used.

I was invited to a meeting along with Isobel, and the managers tell us that all we need to do is take the old code, update some user-facing text which the UX team will confirm, then it just needs to be tested, but it should all work since it was ready for testing over a year ago. So in terms of development work, it sounds like we’d spend more time in meetings and generally discussing the work – than actually doing the work. (I later find out this is not true).

Assigned to Isobel

I tell Isobel to go through the changes and make sure they really do meet the requirements. A few days later, Isobel says she is on annual leave for 2 week but the changes are fine. The next day, I’m told I should look at it instead.

Assigned to Me

After an hour of testing it out, I find that there was:

  • A control partially truncated
  • Some extra spacing in one of the error messages
  • Some components that were disabled that shouldn’t be
  • Inconsistency in behaviour between “Templates” and “Templates Pro”
  • Group Item logic was completely wrong
  • Blank warnings were appearing for all other components

So how did the original developer think this was ready? Why did Isobel think it was ready?

So I start to fix the issues and I find copy-and-pasted code, redundant code, unclear code, code which could easily be unit tested but isn’t. I spent around 3 weeks sorting it out and it still wasn’t perfect. Meanwhile, I was invited to other meetings to say they changed their mind about some features. I had to undo some changes, change more UI text, and disable a few more components. In hindsight, I think I may as well have binned it, and started it again from scratch.

When I thought it was ready, I had the Pull Request created in the first week of January, ready to be checked-in for testing, however, there were no testers free, so it sat there for a month.

Eventually, the testers begin testing it and find a few problems with it. I fixed 3 out of 4 issues, but the last one seemed to be impossible to fix due to another bug which really needed to go to the specialist team that dealt with that area. 

The actual template knows where it came from, but the Group item inside doesn’t. There was this interesting variable name that made me smile.

isOneOfOurs

I showed it to a colleague

“seems like Britain First wrote this”

Bants from colleague

There was some code where we set the originID to either the user’s or our organisationID. However, it set it to 0 which then assumed it was one of ours. I tried looking at one of the other properties which was a different type of ID; a GUID, but it was blank, so it was broken there too. 

I couldn’t see a simple way to fix this. It would be far too risky for me to change, and I definitely didn’t have time. So I got told to abandon it and it would be reprioritised.

I think it was around 6 weeks later, it was assigned to another team. So it is now it’s with its 4th team, approaching 2 years later. Maybe we can call this a “pass the parcel” enhancement.

Assigned to Kumar

I was aware that the development re-started (along with some other requirement changes) when I saw a Pull Request for it. It was from Kumar, a developer in India that is absolutely rubbish. Not only that, it is quite hard to help him because his English is fairly poor. I also can’t tell if he is trolling, or if he really is that bad.

I would have thought that Kumar would have been told to speak to me about the work so I can “hand it over”, or at least he should have seen my name and comments on the item and shown some initiative and asked me about it. As it goes, this new change was something I already had fixed, it just wasn’t checked in. I could tell his fix wouldn’t work just by reading his changes. I message him telling him this.

He later responds with this conversation:

Kumar  10:11
Hi Mate
10:12
With the changes i have raised as a PR, I created a page as Group item and section as well with itself containing the components as Group as well as non Group items and it seems working  .
can i share you the screen mate ?
Me  10:14
so a user authored page/section Group item which contains one of the components - shows the message
and a Officially-authored page/section Group item which contains one of the calculators - does not show the message

Kumar  10:14 Exactly mate Kumar  12:34 Hi Mate, Shall i comment out that the fix has completed the scenario we discussed here ?

I didn’t believe him, so I checked his code out, built it and tested it out myself. Obviously broken as expected.

Me  12:50
just trying it now and it looks broken to me
got an Officially Authored Group Page and all the components have warnings next to them
Kumar  13:10
Ok dude, I will have a look on it.
Kumar  13:31
Hi Mate, As u said it was showing up all the warning
I might have not removed the Pages and section flag check for showing warnings
will debug the code mate,

The next day

Kumar  07:12
Hi Mate
Good Morning Mate

I look at the latest Pull Requests and see his new changes. Instead of taking my code that I know works, he has come up with his own solution. I think it might work, just harder to read.

Me  08:59 did you test my changes or just write that yourself? 
Kumar  09:11
no mate i have took it from latest service branch of my team
was everything fine mate ?
i tested that locally and working fine
Me  09:13
it is similar to what I had done, just more lines of code
Kumar  09:14
haha, ok mate i will make the changes as suggested in the comments
Thanks Mate

It’s not really funny. You just wasted a full day’s work because you didn’t just use my code.

I explained to him that this item is a bit of a nightmare since there’s multiple places you need to change due to “Templates” and “Templates Pro” which doesn’t share much code. Then there’s some existing bugs, and many different combinations of templates you can create. He doesn’t seem to test his work at all, so I think he had no chance of getting this working. I was trying to emphasise how much testing needs doing with every change to try and get him to put some effort in. Unit testing would help alleviate some of the manual testing. The next day…

Kumar  13:27
Hi mate, Good noon
It is becoming a night mare as u said :joy:, i have completed with the unit test case also have fixed other area like Warnings were happening in the “Admin org” too. i have fixed that now. Kindly review and guide me to proceed further
Me  15:58
<send him picture> that should have the main yellow banner at the top shouldn't it?
Kumar  15:58
If it was a “Admin org” org, it should not have mate
Me  15:59
it's not
Kumar  15:59
Then it should have i believe
haah, one more PR patch upcoming ..?
:exploding_head: literally with this work
Me  16:17
I suggested 2 unit tests. You have only done 1. I think it is the other scenario where it doesn't work
Kumar  16:32
Ok , but if the method will not execute if the template was Group template mate, so do i need to do that as well ? in turn it returns empty
Me  16:40
isn't your requirement that it shows the message regardless if it is Officially-authored or User-authored
Kumar  16:41
yes, that is the requirement
i will check on the code once again mate
haah lil confusing

Although the overall work is confusing, this is one of the simplest parts of it: Show a message regardless of who created it. If the method is “returning empty” then that is the bug, it should return a message.

There was a line of code like

if (!configEnabled && activeSubscription)

and he changed it to

if ((!configEnabled && activeSubscription)
||
(configEnabled && activeSubscription))

so I wrote: “so just activeSubscription then

Kumar: I am not able to get ur point here, kindly guide me

Me: “true or false” is always true isn’t it?

Kumar
yes mate, so shall it be framed like this
if ((configEnabled || !configEnabled) && activeSubscription)

NO! That still says “true or false”. I was trying to think of how I can write a response without telling him he doesn’t understand the very basics of programming. This is like Day 1 of learning how to write code.

Kumar: haah, I got it, just “activeSubscription” is enough isn’t it?

I was glad he seemed to understand in the end, because I was tempted to tell him to change his career…then he adds:

“correct me if I am wrong”

He has zero confidence.

The very next day he then tells me there is another requirement to remove the banner that he has been changing, so “is there any point carrying on?”. He sends me a screenshot of the requirement rather than giving me a link. He is definitely sent here to troll.

I’m sure he must have known about this, and I don’t know why he either:

  • didn’t make the deletions first (it would reduce the amount of code and reduce confusion)
  • not change any code he knew he was going to delete

So he made changes trying to fix a feature he knew he was going to remove. I had invested time reading the code, manually testing it, and all this back and forth communication. What a waste of my time.

Unassigned?

The good news is that he is leaving so he is going to be another company’s problem. The bad news is this enhancement is going to be reassigned to someone else.

I’ll probably write another blog about Kumar; I’ve got some more notes on previous development work he has done. If this enhancement ever goes live, maybe I will write a follow-up blog on it too.

Game Review: Game Dev Tycoon

Since Game Dev Tycoon is about software development, I decided to review it and put it on this blog.

Game Dev Tycoon is a simulation game with a game development theme. You start off as a solo developer in your garage, making simple games in order to raise money to expand. Once you have accumulated enough wealth, you can purchase a new office which allows you to hire more developers and work on more complicated games. Later, a larger office is available which allows you to hire even more staff, and even have a Research, and Hardware labs (as long as you have a Design and Technology specialist).

When you start to make a game, you choose the game’s name, pick the Size (you start off with Small, then unlock further sizes as you progress), Topic (theme), Genre and Platform. A later upgrade allows you to choose the target audience; Young, Everyone, Mature. Then there’s 3 stages of development where you move sliders and choose extra features. 

  1. Engine, Gameplay, Story
  2. Dialogues, Level design, AI
  3. World Design, Graphic, Sound.

You move the sliders based on your assumption of the genre (RPGs have an emphasis on Story, Simulation requires Engine), and by the results and feedback of your released games. After release, you can then generate a game report which will give you hints on which aspects are important, and if the target audience and platform fit the style of game and theme.

For medium sized projects and above, you have to assign a developer to lead the development for each aspect. Adding additional features (e.g. Soundtrack, Surround Sound) and using game engines comes at a cost, but the more features usually means a better quality game. It’s not clear how random a lot of these elements are though, or if certain tech affects the genres differently.

Once you have created your game, you can wait a bit longer to put the finishing touches. The number of bugs will decrease and you can add extra points to Design, Technology. When you click “Release”, you will be given a review score and your staff experience within all aspects will increase. 

Money will come in from the sales with declining sales over time, and then the game is removed from the market. Your staff wages increase as they level up, and the general cost of development means it’s a good idea to have a large surplus of money if you can. If you do drop into negative figures you can get offered a short term loan but with extremely high interest. If you can’t afford that, then you are bankrupt. You are allowed to restart from your last save, the last office move, or you can just start again. The game can be tricky if you don’t land those big revenues at the right time.

As time progresses, games take longer to develop, require more staff and cost way more money. This is like real life when early computer games were developed by tiny teams over a few months. Those games may have been text only, or had primitive graphics with colours, but that was what the consoles could handle back then. Today’s “AAA” games take years to develop but have cutting-edge graphics and sound. So in this game you need to hire more staff, keep training their skills, and make sure you have the staff in the right areas. You can only have 1 game in development at any time.

As time progresses, new technologies become available for research, and Platforms are released or removed from the market. After a new Platform has been released, you can then buy a Development Licence to make games on that Platform (it would have been a nice idea to be able to work on games for the actual console launch though, then you would have the challenge of rushing your game out). The names and images of the consoles are slight variations to avoid copyright issues (or add humour to the game) but it’s easy to recognise what they are supposed to be. Each console is biassed towards certain genres and audiences. You start with the PC and Commodore, and it progresses to modern day with the Xbox Series and PS5.

You can take on simple contract work which gives you a small amount of money for a small amount of your time. The main advantage is to acquire more Research points. You can also develop games for Publishers but must create the game at a certain quality in order to receive the full amount. These contracts are more beneficial when you haven’t accumulated enough fans. Later on when you do have many fans, even mediocre games seem to sell themselves.

With your Research points, you can send your employees on training courses. You can Research new topics (which seems like a randomly ordered large list which is unlocked in batches), and technology. Once you have researched some more technology (mainly the improved graphics), it’s often beneficial to create a new Game Engine so you can use these features.

The Research room takes a lot of money to use. Even moving the slider slightly will cost $300k or so per month. You can research a Steam-like platform, the ability to make your own console in the Hardware Lab, create MMOs and a few other options. It’s quite hard to acquire these aspects before the game “ends”. You can carry on playing but new consoles won’t be released and there’s no “events” that trigger. 

There’s not many Genres but there are loads of Topics. However, I think some of the Topics should be classed as Genres such as “Racing” or “Sports”. Two other Genres that seem obvious to include would be Platforming and Puzzle.

I also wondered how the developer interpreted the genres especially when you unlock the ability to choose 2 genres (you cannot have multiple topics though). For example: what does “Adventure” mean? My assumption would be a point-and-click adventure such as Monkey Island, but then what does RPG-Adventure combo mean? Is such a combo doomed to fail, or do the developers have a different idea what this actually means? Maybe a certain Topic could fit this genre well? Casual is also down as a Genre, but I’d say this is closely tied to the size of a game. You don’t get major budget Casual games, although something mid-range like Animal Crossing. For the most part, the decisions seemed logical, but there’s probably some Topic-Genre combos that you won’t agree with because it is fairly subjective.

Just like most simulation games, I found myself playing for hours at a time and found it hard to put it down. It’s simplistic and definitely not for everyone, but I found it addictive and satisfying watching those number-bubbles float to the top and increment those total counts as your developers work on the game.

8/10

Configuration Manager tool – Text Matching

When we add new, optional features, we often put in a flag to enable or disable the feature for certain users. This allows us to slowly roll-out the feature, or only enable it for customers that pay the premium. If there’s problems, you can also disable the feature quickly without pushing out a new version of our software.

One team had decided to rename their module, and therefore were updating the configuration flag’s name.

A lead developer, who reviewed the change, questioned if they could do that without running into incompatibility issues. The project team’s lead stated:

“No, we have the feature validation at source and target separately before we do anything. So, there should not be any compatibility issues.”

Project Lead

However, I was convinced the lead developer was correct. We have multiple versions of our software deployed, but we only have one version of the Configuration Manager tool.

So let’s say in Version1, the new module is called “User Manager“, but in Version2 they want the module to now be called “Staff Management” – and so they update the main software and the Configuration Manager tool to use this new name.

When we use the Configuration Management tool for new users that are using Version1, we update their config to use the new name “Staff Management“, however Version1‘s software will be looking for “User Manager” and will not find it, so will think the module is disabled.

Existing users on Version1 with the old flag in their configuration will work as normal, but it won’t work for new users. For Version2 users, the Configuration will have to be redeployed since their config will have the old name, but Version2 will be looking for the new name.

If the Configuration Management tool used ID’s rather than matching on text; it wouldn’t be a problem, so we have screwed ourselves over there. Matching on text is rarely a good idea due to possible spelling mistakes, case sensitivity (is “User Manager” the same as “user manager“?), and usually less efficient matching on something else like a number ID.

I spent a while trying to think of ways around this issue. Ideas that I thought of involved writing complex database scripts, running scripts outside the release process and getting other people involved. But then I think all my ideas still wouldn’t solve the incompatibility issues and it seemed way too much work for something trivial.

The team were adamant they wanted to rename it though, but it didn’t really matter too much. Only our staff see the Configuration Management tool, and we can update the main software so the users see the new name. It just adds confusion if someone tells you to enable “Staff Management” but you can’t see the option, so they have to correct themselves and ask for “User Manager” instead.

I would have thought the project team would have ran through different scenarios to test if their idea was feasible for new and existing users. But even after questioning it was feasible, they were adamant there wouldn’t be any compatibility issues so I had to explain the scenarios to them.

Related Blogs:

Configuration Switches
String Comparisons

Software Performance Tales

Looking through my draft blogs, I have quite a few to do with performance, so I have grouped them together to make a complete blog entry.

“the whirling circle thing is just whirling”

User annoyed at the slowness of our system

Do we care about performance?

I’ve listened to some Tech podcasts that have discussed Big O notation which is a way of classifying the performance of an algorithm such as O(n), O(n^2), but outside of studying Computer Science at university, I personally have never heard anyone else reference this. I think with some programming jobs, the efficiency of algorithms is crucial. For the most part, I don’t think people care unless you are doing something that turns out incredibly slow. In the age of Cloud Computing where you can be charged by the millisecond, then it will become increasingly important for server-side algorithms.

Where I work, we are very reactionary rather than proactive. Recently, we have had loads of complaints about our performance, so then we have identified the slowest areas and addressed them.

Thinking about performance upfront would mean the user gets the best experience and doesn’t complain. However, there’s plenty of examples of code which runs fine on your machine but runs poorly in live. This can be because you have a faster computer, the network speeds are faster, lower latency between the servers (when developing, the client and server is on the same computer so there is no latency), or you don’t have a database which represents live – you have 100s of rows rather than 100,000s. I think this last reason is often a cause for us, in addition to concurrency (thousands of live users simultaneously using the system, compared to a few testers using a test server).

An example of how we don’t often consider performance is as follows:

Example 1

I showed some initiative recently and essentially rewrote a project that was inefficient. 

  1. Some server calls had a check to see if a feature was enabled before attempting to retrieve data from the database, but some didn’t. Therefore these code paths ran queries against the database before returning no data. 
  2. When the feature was enabled, there were other parts of the code that were making the same server call twice in the same sequence of actions,
  3. and other places that made a server call even though the client already had the data from a previous call. 

It was a mess basically.

The functionality behaves correctly and the performance didn’t seem out of the ordinary, so the Software Testers never flagged it. We were probably talking an extra 50ms to 200ms in total when testing internally, although this would be worse when deployed live. It should have been obvious to the Development team that there were inefficiencies if they were thinking about possible improvements to their codebase, or if they were paying attention to the server calls and noticed their new server calls when the feature was switched off. 

Volume and Performance Testing

We have loads of test environments – virtual machines with databases and servers where the Software Testers can test our code changes. I would have thought these would be fine to do performance testing, and spam these test servers with calls, but a Test Environment Engineer once sent an angry email:

Our standard test environments are Absolutely not to be used for Volume and Performance testing. This can impact the whole devtest infrastructure, as well as some live services that share the same infrastructure. Doing this could cause a Major Incident.

Test Environment Engineer

This seems bizarre to me. Why would you share resources with the live production environment? Surely the Test Environment should be in its own isolated sandbox, where you can create, update, and delete with the safety and freedom. 

Example 2

We had another performance issue where a large amount of data was attempted to be retrieved. When the developer analysed it, we weren’t actually trying to retrieve a large amount of data afterall.

“There were 142,858 Id’s passed into the comma separated string, I found that there were only 64 distinct ID’s once I removed the duplicates. After the duplicates were removed it went from 76minutes duration to under 1 second”.

Software Developer

What a performance improvement! 

I wonder if passing the data in a form that isn’t a comma-separated string would also help. If you are passing in “12, 156, 201, 202, 284”, then, to get the ID’s, you need to remove the commas and place the numbers in a table. If you just pass it in as a table, then you are cutting out a step on the database layer.

Example 3: SQLite

One part of our system used a local file cache (loads of files were downloaded to the users machine, then our software would just load up these xml files rather than going to the server every time). It has worked fairly well, but overtime, the number of files has grown.

I think the performance issue comes from the fact that all the files are then loaded and are kept in memory, so it is a memory hog, and sometimes leading to “Out Of Memory” errors. Maybe the solution is to work out if we can dynamically load these files in full only when they are needed, while only having the “metadata” in memory at all times.

Colin came up with an idea to move the files into a local database using SQLite. This innovative idea impressed the managers and were eager to do it. I was sceptical because I think we would still end up loading all the files, just the place where they are stored has changed.

Day 1 of release, the following Major Incident was raised:

SQLite corruption. Out of memory

After that was fixed, I believe the changes caused 3 additional Major Incidents. Colin still swears by it. He thinks it was an amazing idea. I ain’t convinced.

User Feedback

It’s rare that I come across comments from users about our software, but they do often make me laugh. Part of the problem is that they use software, but aren’t really technical enough to use the correct terms, so when complaining about a performance issue, one user remarked:

“the whirling circle thing is just whirling”

User annoyed at the slowness of our system

Another thing about user complaints is that I tend to only hear about the real angry complaints and not just generic comments. I think Support had told a user there was a large amount of data on that particular record, and the more data on the record – the longer it is going to take to load.

“Regarding larger records, this is a predictable problem. What mitigations are being put in place to offset that? I still have 20 years of working life yet, don’t tell me that by 2035 I will have to wait 20 minutes to load a record???!”

Angry User

It’s a valid complaint. We need to come up with a different way of loading the record so they aren’t sitting around waiting for the full record to load.

Excuse: Performing “Analysis”

I think I have discovered a new excuse to use in order to buy time; “Performing Analysis”.

Example 1

There was a Major Incident on Friday which I debugged and suggested a simple fix. However, instead of me going ahead with this fix, the managers wanted the team responsible for introducing it; to take accountability for it.

After the manager sent the email and Cc’d me in, the team responded with “The Team is analysing this issue; will update you shortly”. Then, after the entire day has passed, they send a follow-up email “this will be ready by close of play Tuesday.”

It was currently Friday, and we wanted it released for Monday (since it was a Major Incident) – not released on Wednesday. So I had to sort it out.

Example 2

Dave commented on a Code Review that he thought the Developer had changed too many files; and there was a simpler way of introducing their fix. They responded: “we need to perform some analysis” and took a day or so before coming up with new changes. It would have just taken 10 minutes to rewrite it; I didn’t see any scope for this so-called “analysis”.

I’ll keep an eye out for more examples of this.