http://www.perlmonks.org?node_id=133229


in reply to How do you critique another person's code?

However, how can I politely provide factual feedback without "attacking" the style of programming?

"Style" IMHO means whether I use 2 or 4 space indentation, whether or not I cuddle my else's, etc. That code is crap, to put it politely (but put it as politely as possible in the code review, as others suggest...).

Update: Re: Dominus' reply: While the book you mentioned talks about good and bad programming style, I don't think of the code at the top of this thread as being in bad programming style - it's just bad programming, period. Saying that that programming has any style at all would be giving it too much credit, and if someone says that one's programming "has style," well, that's good in and of itself, isn't it?

When I think about style, I tend to think more along the lines of what's in or out of fashion (think goto, and looking at those example FORTRAN programs in the book you mention (and boy, do those bring back memories...), I think indentation is a valid example of an element of style, though I admit, not an incredibly substantive element, and the book does discuss how to cuddle your if's, then's, and else's {no braces, those must be a modern invention}); anyway, I didn't mean for anyone to take my 'definition' of style so literally. Sorry for the misunderstanding :)

Another update: Aww heck, now that I think about it, call it whatever you like - bad style, bad habits, bad technique, bad programming, or a bad hair day, its fine by me, and not all that much worth arguing about. (I predict that 'bad hair day' will become computer jargon for writing programs that suck, and although it will generally only refer to a temporary condition, the phrase "He's on a permananent bad hair day" will also take hold, and use of these phrases will proliferate after the publication of the book "Coding and Bad Hair Days Don't Mix" - and there is a 47% probability that it will be a Dilbert book :)

Replies are listed 'Best First'.
Re: How do you critique another person's code?
by Dominus (Parson) on Dec 20, 2001 at 09:08 UTC
    Says runrig:
    "Style" IMHO means whether I use 2 or 4 space indentation, whether or not I cuddle my else's, etc.
    Well, it didn't always mean that. For example, if you look at Programming Style: Examples and Counterexamples by Brian Kernighan and P.J. Plaugher, you'll find that it's concerned with the sort of issues that Rhose raises, and not with trivial matters like indentation or brace placement.

    I think it's really unfortunate that the programming community's discussions of programming style have been sidetracked into meaningless arguments about indentation, and I'd like to see the useful term 'style' applied to more substantive issues.

    --
    Mark Dominus
    Perl Paraphernalia

      Style is always more than formatting.

      Look at 'Code Complete', a weighty tome all about formatting that covers a lot more ground than just where to put braces and the merits of tabs over spaces.

      Beyond that look at something like the 'Chicago Style Manual for Writers'. It also deals with style and is more than just formatting. It addresses passive sentences vs. active sentences, word order, proper grammar, etc.

      The array slices the unnamed programmer was using are bad Perl grammar, as egregious as "That's something I ain't puttin' up with" in the English language. Let's all say it together: 'Style is NOT formatting.'

      Ok, I'll relinquish the soap box now.
        runrig stammers: "Style...is...NOT...formatting"

        There, I said it. But it does include formatting to some degree, and the 'etc.' that I listed implicitly includes all the other stuff :)