About two weeks ago I added the postfixop check to krazy. This checks for postfix uses of ++/– usage. Initially I constrained the check to for-loops. However this lead to a huge amount of issues because also postfix usage of ++/– operators on simple types where marked as an issue. After some discussion with Eckhart Woerner and Allen Winterz I relaxed the check more so that usage on simple types is also not taken in account anymore.
Although postfix usage on simple types won’t gain you any/much I still think it’s a good thing to consequently use prefix wherever appropriate. However that has more a cosmetic reason (which I why I took it out actually) then a perfomance reason. Consider for example the next piece of code (and yes I’ve seen something like this somewhere in the KDE codebase):
QList::Iterator it = list.begin();
for( int i = someNumber(); it != list.end(); ++it, i++ )
I realize that it’s partly a matter of taste if you like this kind of code snippets in your code base or not. However for people who never have had some in depth programming course it might look like if it is arbitrary what you choose.
Another thing I’ve been thinking about last weeks is "What is the importance of cosmetic changes in relation to code quality". When browsing through the source of KMail while fixing issues reported by krazy I see a lot of things which make me cry. For example defining variables which are used only ~25 lines later or in 3 different context. These kind of things make my fingers itch to rewrite such parts. Well, don’t be scared, I didn’t….. if it ain’t broken don’t fix it, right? But I still can’t find a good answer on the question: is that sort of code broken? In some sense it is in my opinion. When I as a newcomer to the KMail code look into some sections I really have to read that twice or sometimes even more times to get some vague idea what is happening over there. I think that is something that can be avoided (to some extend of course, as not everything in life is as easy as we would like it to be). All this made me think that it would be a nice to have something in addition to the normal krazy checks which checks for situations where some cosmetic change might improve the readability (and therefore the quality?) of the code.
Well, leave me a comment if you have some noteworthy ideas about these topics.
p.s. KMail only served as example here, I guess there are more places which have similar issues. It’s just that I was browsing through KMails code while these thoughts came up.