Back in university, I had to implement a reverse polish notation calculator in a software engineering class. Overall the assignment was pretty stupid, and I entertained myself by generating writing a very compact implementation. It worked perfectly, but I got a 25/40 (62.5%) grade on it. That mark was well deserved, although I did not think so at the time.
The grading remarks were actually some of best feedback that I ever received, and also really funny to boot. I don’t know the name of this old now-nameless TA anymore, but I took his advice to heart, and kept his grading remarks on my wall in my IBM office for years. That served as an excellent reminder not to write over complicated code.
Today, I found those remarks again, and am posting them for posterity. Enjoy!
Transcription for easy reading
- It is obvious that are a very clever person, but this program is is like a big puzzle, and in understanding it, I appreciated it and enjoyed it, because of your cleverness. However much I enjoyed, it is none the less a very poorly designed program.
- A program should be constructed in the easiest and simplest to understand manner because when you construct very large programs the “complexity” of them will increase greatly.
- A program should not be an intricate puzzle, where you show off how clever you are.
- Your string class is an elephant gun trying to kill a mouse.
- macros Build_binary_op and Binary_op are the worst examples of programming style I have ever seen in my entire life! Veru c;ever. bit a cardinal sin of programming style.
- Your binary_expr constructor does all the computation. Not good style.
- Your “expr” class is a baroque mess.
- Although I enjoyed your program, Never write a program like this in your life again. As your T.A., I have to pushish you so that you do not develop bad habits in the future. I hate to do it, but I can only give you 25/40 for this “clever puzzle”.
The only part of this feedback that I would refute was the comment about the string class. That was a actually a pretty good string implementation. I didn’t write it because I was a viscous mouse hunter, but because I hit a porting issue with pre-std:: C. In particular, we had two sets of Solaris machines available to us, and I was using one that had a compiler that included a nice C++ string class. So, naturally I used it. For submission, our code had to compile an run on a different Solaris machine, and lo and behold, the string class that all my code was based on was not available.
What I should have done (20/20 hindsight), was throw out my horrendous code, and start over from scratch. However, I took the more fun approach, and wrote my own string class so that my machine would compile on either machine.
Amusingly, when I worked on IBM LUW, there was a part of the query optimizer code seemed to have learned all it’s tricks from the ugly macros and token pasting that I did in this assignment. It was truly gross, but there was 10000x more of it than my assignment. Having been thoroughly punished for my atrocities, I easily recognized this code for the evil it was. The only way that you could debug that optimizer code, was by running it through the preprocessor, cut and pasting the results, and filtering that cut and paste through something like cindent (these days you would probably use clang-format.) That code was brutal, and I always wished that it’s authors had had the good luck of having a TA like mine. That code is probably still part of LUW terrorizing developers. Apparently the justification for it was that it was originally written by an IBM researcher using templates, but templates couldn’t be used in DB2 code because we didn’t have compiler on all platforms that supported them at the time.
I have used token pasting macros very judiciously and sparingly in the 26 years since I originally used them in this assignment, and I do think that there are a few good uses for that sort of generative code. However, if you do have to write that sort of code, I think it’s better to write perl (or some other language) code that generates understandable code that can be debugged, instead of relying on token pasting.