Title: Letters to the Editor

On reading the various answers for the code critique competition 27 in C Vu Vol 16 No 3 (yes, I'm an issue behind at the moment!), involving "ugly" numbers, I feel I must point out what I find to be some poor advice from Francis in his commentary. He develops a solution, as most of us did, with an IsUgly(), or is_ugly() function. This he gets to a near working stage, i.e. it detects numbers whose prime factors are not all 2, 3 or 5. Then he realises that the spec said that ugly numbers have to have at least 2 prime factors, i.e. they cannot be 2, 3, and 5 themselves. Fine, except for the implementation. He realises that he need not worry testing numbers below 6, oh, except for 4, because that is 2x2.

I can't decide whether this is a form of premature optimisation or not. Either way, the most significant problem is that we've now introduced the literals 4 and 6 into the code. Why? Because 6 is bigger that the biggest of the ugly prime factors, and 4 is the only number between them which can be made by more than one factor of one of them. That to me is not the kind of complicated derived logic you should have in code.

Suppose we made the definition of ugly such that its prime factors were 2, 7, 23. Now, what happens to the 4 and 6? Hmm, we need to tackle it generically this time, or else make special cases of 4, 8, 14, 16.

Simon Sebright <simonsebright@hotmail.com>

I put this to Francis and had this back..

We can spend an awful lot of time trying for generic solutions to specialised problems. In my opinion the time for considering generic solutions is when we are actually presented with several similar problems. At that stage it may be worth looking for a suitable abstraction.

I contend that human time is one of the most expensive resources and we need to develop work habits that reduce that cost. We can argue interminably about how to do this and that in itself is a waste of precious human resources. The degree to which a solution is specialised will always be a judgement call and as such different people will draw the line in different places.

By the way, you see the logic of my code as complicated, I don't but here again we differ and I am certainly not going to lose any sleep about such differences of opinion.

Which in turn led to...

Of course, there is always a line to be drawn on the amount of time you can scrutinise a piece of code. In a production environment, there'll be thousands or even millions of lines in a project, and spending half an hour looking at 40 lines of code could have a dramatic effect on productivity.

But, in the context of a student code critique (real or ficticious), I think investing time is a good thing. Not discouraging bad habits or shortcuts at this stage has to be a bad thing. Making the novice aware of more expressive, or generic ways of doing things gives them a greater toolkit later on, and then it's going to be more likely that future code reviews won't need such fine-scale attention. I've seen plenty of these kinds of shortcuts cause bugs later in a program's life, and they can be very hard to track down.

I'm increasingly of the opinion that maintenance starts the moment you hit the compile button, not just two years later. The current project I'm on has about 1 million lines of code, has been in development for 3-4 years, is on the third or fourth generation of programmers, and won't be finished and out of the door for a number of months. Equipping students with the awareness of issues in these environments is very important.

Applying that to this example, I'd rather see logic or assumptions in the code than the comments. E.g.

const int small_ugly_exceptions[] = { 4 };
const int smallest_possible_ugly = 6;

and then we don't need comments. I note that there was no comment on the < 6 test, which I'd like to see a put in in a production environment (except that I advocate coding it so comments are not necessary).

I'll leave it there. As always, if you wish to comment, feel free.