> Unnecessary goto Or the most straightforward and obvious way to break out of a switch in a loop.
> variables defined far away from where they are used, Variables defined predictably at the start of the function, as the convention is in BSD code. Yes, they can be a little far if the function is long. You also always know exactly where to find them. > monster function Two hundred lines is hardly long, and size matters less than readability. If a long function reads neatly from top to bottom, you can split it into a few dozen procedures, requiring the reader to jump back and forth to figure out what the code *actually* does. You then also need to shovel data back and forth between these functions. This is how you turn simple and straightforward into a clusterfuck. > variables are not commented what they do, > variables named well enough If you're familiar with the fts api and know what command line flags are, then half of these variables are immediately obvious. For the rest, the name is a good hint and if you spent a few seconds looking at where and how each variable is used, these are perfectly understandable too. Short names don't get in the way too much. How would you improve these names? It sounds like you're just trying to take snippets of code out of context and declare them bad because you're not reading the rest of the code? Of course you could comment these, but the code is simple enough that such excess verbiage would likely just get in the way. Much in the same manner that splitting the function into a few dozen would only make it harder to see the code for what it is. Mind you, the code isn't written for a first-year comp sci course where the students need annotated twenty-line snippets to help them come to grips with the basic syntax and structure of a computer program. > not all functions are commented what they do.. Again trying to please some quality metric? Usage does not need to be documented. And main() is essentially the whole program, which is extensively documented in a man page. It's not called by another function; there is no internal API to document. What comment would you add there? What value would that comment add? > To me, it looks like that there is no intention to optimize readability and > testability. Instead it looks like there is put effort to minimize amount of > functions or something. It looks like you're not actually reading the code, you're just looking at snippets of it, taken out of context. And then you're applying some cargo- cult pseudo-science to make statements about these snippets. Goto is evil! > Something to aim for: > -Less linearly independent paths in module > -High cohesion > -Low coupling Did you intend to achieve better cohesion and lower coupling by breaking the function into a dozen small functions that appease whatever metric you're using? Of course, I'm a nobody to say how this code should look, but it looks fine to me; and so far as I can tell, it's not broken. Don't fix it if it's not broken. But if you think it can be improved, I'm sure someone here might take a look at the diff once you send it. But if your comments about the current code are any indication of what your ideal version would look like, I'm not sure there are many who would be willing to commit it. But don't let me discourage you; I'm a nobody here. -Henri
