In a previous blog, I was criticising some work done by an Apprentice: Apprentice Insanity
What actually happened with this is that it got deprioritised so was just shelved for a couple of months. Which is weird, because the original developer made out it was very urgent and wanted to check it in at the last minute.
When it was decided it was needed again, who got asked to finish it off?
Yeah! me! What are the chances of that?
The Fix
The work was to show some messages to the user why a feature isn’t enabled for them. There can be several different reasons, but we never told the user why, until now.
So I load up our program with a valid user… I can’t use the feature anymore, and it tells me 5 reasons why I can’t use the feature. It’s completely broken then. A valid user should have allowed me to use the feature, and obviously show 0 reasons.
So I fixed a few bugs, and now I can use the feature, but there are still 3 reasons displayed. So it says I can’t use the feature; but I can, and I know I should be able to.
I spent a lot of time looking through the code, and I saw that there is an existing file that did the validation, but the apprentice had created another file with some other validation rules in. If it is the same rules, then we are checking them twice? If they are different rules, then you end up in a situation like this; where I can use a feature, but the other file’s validation tells me I can’t.
So I end up deleting this extra file, and make the existing file be the code that returns the failed rules.
The new dialog he added looked nice enough, although the dialog was a bit small, and a few aspects of it wouldn’t actually meet our UX standards. When I looked at it, I saw that he had positioned all the controls by hand for a particular size, and so I had to do a lot of rework to get the controls to work on a larger dialog.
He also had code with complicated logic to adjust the size further. Complete hacky. So I got rid of all that too.
There was some other hacky code where he even had a comment along the lines of “for some reason the control can be duplicated, so if this happens, then we need to remove it”. After some experimentation and debugging, I eventually worked it out. What was happening was that the initialisation code was calling his new method twice. You would think that means the control was always added twice, but it wasn’t.
It turns out that there was some existing code that checked if a control was in position 1 in the grid, and if it was; it was then removed. Then there was some logic to re-add it based on a condition. So the control in position one is optional.
| Position 1 | Control that sometimes gets displayed. Put new control here? |
| Position 2 | Control that is always displayed |
| Position 3 | Another Control that is always displayed |
The Apprentice was adding his control to position 1 in the grid, but if the other control was there, then he added it to position 2 instead. So what was happening is – if he added his control to position 1, then the existing code then removed it, then when his logic is executed the second time, then it gets added to position 1 once more. So it displays correctly, but works “by coincidence”.
If the existing control is added to position 1, his method then adds the new control to position 2, so when it executes the second time, it adds a duplicate to position 2. So then his hacky code kicks in and removes the duplicate. The simple solution; just create a new row in the grid for his new control. Delete the duplicate call. Delete the hack.
So it seems like he got into a mess by trying to cut corners, put in a hack when it didn’t work, but then it only worked intermittently and he didn’t understand why… so then put in some more hacks to cover it up. What you end up with is nonsense.
In the end, I had completely rewritten it, made the UI nicer, and got rid of all the bugs. Result.
The Demo
We had a demo arranged with someone who was basically representing a user. I was going to demo my code, but, an hour before the meeting, I got an important meeting invite. As a joke, I asked the Apprentice if he could cover me. He said he would be happy to do so. I told him I was joking, but if he really wanted to do it, then it would be appreciated. Like a nutcase, he agreed. With only about 45 minutes to prepare for the demo, I told him I had completely rewritten his code, so he may be a bit lost.
He chose to demo from his existing, broken code.
My meeting turned out to be really short, so I joined the demo as the Apprentice was trying to explain a feature, after it crashed and somehow wiped the contents of an existing dialog, much to the confusion of the Representative.
I saved the day at that point and demoed the proper, working code.
Afterwards, one of my colleagues said to me
“How the hell can you demo something THAT broken?”
concerned colleague
I said he is “a complete legend”.
Stranger Coding Tings 