What makes something a horrible hack?

Over in Moodle bug MDL-42974, Derek Chirnside asked "What is it about a hack that makes it 'horrible'??". I had described the code sam wrote to fix that issue in those terms, while at the same time submitting it for integration. It was a fair enough comment. I had helped sam create the code, and it was the kind of code you only write to make things work in Internet Explorer 8.

Although "Horrible hack" is clearly an aesthetic judgement, and therefore rather subjective, I think I can give a definition. However, it is easier to start by defining the opposite term. What is "Good code"? Good code should have properties like this:

1. It works: It does what it is supposed to.
2. It is readable: Another developer can read it and see what it is supposed to do.
3. It is logical: It does what it is supposed to do in a way that makes sense. It is not just that a developer can puzzle out what it does, but it is clear that it does just that and nothing else.
4. It is succinct: This is a companion to point 3).
5. It is maintainable: It is clear that the code will go in working in the future, or if circumstances do change, it is clear how the code could be modified to adapt to it.

Note that property 1) is really just a starting point. It is not enough on its own.

A horrible hack is code that manages little more than property 1. I think sam's patch on MDL-42974 scores a full et.

2. It is not at all obvious what the added code it for. Sam tried to mitigate that by adding a long comment to explain, but that is just a workaround to the hackiness of the code.
3. There is no logical reason why the given change makes things work in IE <= 8. We were just fiddling around in Firebug to try to find out how IE was going wrong. Changing the display property on one div appeared to solve the display problem, so we turned that into code. We still don't really understand why. Another sign of the illogicality is the two setTimeout calls. Why do we need those two delays to make it work? No idea, but they are necessary.
4. The whole chunk of added code should be unnecessary. Without the addition, it works in any other browser. We are adding some code that should be redundant just to make things work in IE.
5. We don't understand why this code works, so we cannot understand if it will go on working. In this case, lack of maintainability is not too serious. The code only executes on IE8 or below. In due course we know we can just delete it.

Normally, you would wish to avoid code like this, but in this case it is OK because:

• The hack is confined in one small area of the code.
• There is a comment to explain what is going on.
• It is clear that we will be able to remove this code in future, once usage of IE8 has dropped to a low enough level.

At least, we hope that the Moodle integration team agree that this code is acceptable for now. Otherwise, we wasted our time writing it.

The Good, the Bad and the Ugly

It was the best of times, it was the worst of times, ... It has certainly been a mixed week.

The good

... was that I helped three OU developers submit their first bug fix through the new Moodle development process: MDL-27631, MDL-28517 and MDL-28620. Hopefully those fixes all get through integration review next week.

The Bad

... was that the time had finally come to deal with a hot potato that we have been tossing around for some months; and, to mix metaphors, when the buck stopped, I was in the the wrong place at the wrong time.

As part of some new question types we are developing, we want students to be able to type responses that include superscripts and subscripts. For example 3×10⁸ ms^-1 or SO₄^2-. We have an old implementation of this, done six years ago for OpenMark (for example this or this), but that never worked in Safari, and is a bit dodgy generally. We want a new, reliable implementation that works in IE, Firefox, Chrome and Safari.

Plan A: back near the start of spring, I quickly knocked up a partial solution using the YUI 2 Rich Text Editor library. It mostly worked, but there were issues. It did not work consistently across browsers, and it lets you nest superscripts inside subscripts inside superscripts which just gets confusing, so we want to prevent that.

I had a sneaking suspicion how hard it would be to get from my quick partial solution to a robust implementation. Therefore I moved on to other things, and tried to unload this job onto three other people in turn. There were plenty of other more urgent tasks on our todo list.

Time passed, and many of the other things got done, so at the start of the week I realised that creating this input widget could not be put off any longer. I also felt it was unfair to expect other developers to deal with a crappy job that I was not prepared to do myself, so I decided to have another go.

The other thing that had changed is that while attempting to implement this, Colin and Wale had both eliminated some blind alleys, and suggested some promising ideas. Therefore, I was continuing from a far better place than where I left off. Even so, it was a long week.

The Ugly

Plan B: Although we had a partial implementation in YUI 2, I did not want to continue with that. Moodle is trying to move away from YUI 2 and to YUI 3 as soon as possible. So, my first attempt was to use the YUI 3(.3) Rich Text Editor. As the docs make clear. That is not finished yet. It is also not terribly well documented. With hindsight, I now realise that it provides only a very thin rapper around the native editing facilities provided by web browsers. Therefore it does not really help with browser inconsistencies.

Plan C: Since the Rich Text Editor is only beta, I decided to have a look at what was new in YUI 3.4, which is due for release soon. The answer is that they have made quite a lot of progress - in the sense that if you are trying to walk from London to Edinburgh, you have made quite at lot of progress by the time you reach Milton Keynes. Compounded with the fact that it is hard to find any documentation for pre-release version of YUI, this approach also failed.

At this point, I decided to do a bit of reading. I found two excellent articles from Opera that explained exactly how contentEditable works in web browsers. I also found a good cross-browser compatibility table. That made me realise that YUI was hardly doing anything to help with cross-browser differences.

Plan D: Now that I knew roughly what the browsers were doing, I briefly toyed with the idea of implementing the widget entirely myself in plain JavaScript. Once again, getting something basic working was not too hard, but I had not even started to tackle the cross-browser differences.

Then I realised that TinyMCE, which Moodle uses, tends to work really well across browsers. It is a bit slow to load, because it is a huge mass of code, but perhaps all that code is there for a reason. A quick play with superscript and subscript in the Moodle HTML editor in various browsers confirmed that TinyMCE must be working around most of the problems. So I dived into the TinyMCE code with the original idea of stealing the bits I needed to make Plan D work. It did not take much looking for me to develop a new-found respect for how hard TinyMCE is working to keep different web browsers in line. I did not want to have to replicate all that.

Plan E: So I finally concluded I should just use TinyMCE directly. That is using a very large sledge-hammer to crack a nut, but at least it should work. Indeed, it was mostly just a matter of setting the right configuration options. What made it particularly good is that there is an option to limit which tags can be nested inside other tags. That robustly prevents people from nesting superscript inside subscript, etc.

I was very nearly there, but there were two more requirements. We did not want pressing enter to insert a line-break, and because we were only dealing with a single line of input, we wanted to use the up and down arrow keys as shortcuts for superscript and subscript. The only way I could find to do that was to write a simple TinyMCE plugin. Fortunately, that is well documented.

The end

I got there eventually. The code needs to be cleaned up, tested some more, and integrated into the question types we are building, but I don't foresee any problems doing that.

I would like to thank the Moxie Code, who make TinyMCE, even though they have completely ignored the patch I sent them some time ago in relation to MDL-27890. I would also like to thank Olav Junker Kjær, who wrote the Oracle blog posts, which were the most useful thing I read. Also, the team behind Firebug. I can't imagine doing JavaScript development without that debugging tool. Finally Colin, Wale and Jamie, who I tried to dump this on, and who in return gave me helpful ideas.