> Well to say the truth it took me less than a few seconds to see what the code was doing, it was perfectly readable, so I classify this one as good not bad. If one can see what the code is doing at a glance, I don't think it should matter to split into multiple lines (and actually he did, doing the operation in 2 steps).
> Anyway it shows 'bad and good' code is also a relative thing and personal preference at times. OTOH I also classify as bad for not having mdot.
>
You are correct, good code/bad code is subjective because I would still say that it is bad code due to the style it has been written in. I think I should be able to look at a line of code and work out
instantly what it is doing and if not then some comments would be nice. I think everyone should be able to work out what this code is doing in a few seconds but it really shouldn't take that long for such a simple operation.
I used to work with someone (he was my boss at the time but not anymore) who prided himself on being able to write really complicated code in one line, where people would take multiple lines for the same operation. He believed that this achievement made him better than the other coders when in fact everyone else hated debugging his code because it was so atrociously written. Never mind everything being bunched up, he also liked single letter variable names and PUBLIC variables abound and it made his code possibly the most hated in the company.
The code in question executes the
AT(",",fstr,6)
part 3 times and the
AT(",",fstr,5)
part twice in just two lines. The coder did split the calculation up into two operations but I think that may have slightly slowed it down without really making it more readable because the code could also have been written as:
reccnt= VAL(SUBSTR(fstr,AT(",",fstr,6)+1,AT(",",fstr,7)-AT(",",fstr,6)-1)) + ;
VAL(SUBSTR(fstr,AT(",",fstr,5)+1,AT(",",fstr,6)-AT(",",fstr,5)-1))
which would still split it up for readability but also remove an operation. IMO splitting the code up into even more lines would not only have made it more readable but it may have made it execute slightly faster by not executing repeated code but maybe I am wrong about that second part.
So I have a question. Would
lnFifthComma = AT(",", fstr, 5)
lnSixthComma = AT(",", fstr, 6)
reccnt = VAL(SUBSTR(fstr, lnSixthComma + 1, AT(",", fstr, 7) - lnSixthComma - 1)) + ;
VAL(SUBSTR(fstr, lnFifthComma + 1, lnSixthComma - lnFifthComma - 1))
actually be slightly faster than the original code because of the removal of the repeated calls? I am genuinely asking that question as I would be interested to know.
Of course once GETWORDNUM became available to coders the snippet could have been rewritten as
reccnt = VAL(GETWORDNUM(fstr, 7, ",")) + VAL(GETWORDNUM(fstr, 6, ","))
Much more readable AND fewer lines proving that improvements in VFP do allow us to improve ourselves.