15
\$\begingroup\$

Recently, a certain user has asked three times for code reviews in quick succession, for different parts of a project he is doing. This in itself is perfectly fine. However, he does not seem to have learned from previous reviews.

Examples

E.g. in his first question, he had

my ($t, $d) = @_; # Element, stack

which I critizised in my review:

You have a knack for single letter variable names. Heck, you even have a comment explaining what each variable means: my ($t, $d) = @_; # Element, stack. This is completely and utterly silly, just name them my ($element, $stack) already. Code ought to be self-documenting.

In his second question (posted only a few minutes later), he did the same thing:

my $e; # Element, Attribute = animation
my $a; # Attributes

which was called out by this answer:

A key contributing factor to the unreadability of your code is naming. You have…

  • $a, $e
  • $A, $E ($E is used twice in the global scope. $A is used inside your first for loop as a boolean meaning "animatable", then as an iteration dummy variable for presentation attributes, then as a hashref to a data structure for all attributes.)
  • cc()
  • data()

It's not 1960, where every byte counts. Just saying camelCase() instead of cc() would help reduce mental workload. Short, cryptic variable names should only be used as iteration dummy variables in simple loops where it is absolutely obvious what it means. Your code doesn't qualify for that.

Now in this third question (posted 22 hours later), this problem is present again:

my ($t, $d) = @_; # Element, stack

Any further review would just regurgitate this and many other similar issues, e.g. his non-use of sensible parsers for HTML -.-

Question

How do I deal with a user that seems unwilling to learn? He does not seem to be interested in best practices at all? What justification do I choose when closing this question?

  • It is duplicate in a sense that possible reviews would have significant overlap with the previous ones
  • On the other hand, this is a new piece of code, which also exhibits new problems that weren't discussed in prior reviews.

For the time being, I left a comment explaining my views and asking him to apply the prior criticism before he asks for another review.

\$\endgroup\$
  • 2
    \$\begingroup\$ Did he at least respond when you've left the comment? If nothing else was said, then I'd probably worry. The comments section is important for maintaining an appropriate bond between the OP and the answerer(s). \$\endgroup\$ – Jamal Sep 12 '13 at 21:37
  • 1
    \$\begingroup\$ @Jamal He did respond, and disagreed to my criticism: “The advantage of short names is that they are at an end point[…]. I did actually include a number of your observations where I felt that they clearly added value” – there are indeed some few minor improvements I could see after a third pass. \$\endgroup\$ – amon Sep 12 '13 at 21:45
  • 3
    \$\begingroup\$ Okay, I've just seen that comment section. I do disagree with him. Yes, short names are nice, but one-letter names (unless a simple loop counter) mean nothing. You'll need to add comments for each variable, thereby flooding your code with unneeded comments. That's a no-no. \$\endgroup\$ – Jamal Sep 12 '13 at 21:49
  • 6
    \$\begingroup\$ I think his "about me" tells it all - My first step towards functional programming ‘programming without variables’ is to shorten variable names as much as possible. \$\endgroup\$ – Anirban Nag 'tintinmj' Sep 13 '13 at 15:09
13
\$\begingroup\$

Pretend you never answered the previous review requests. Other reviewers might not have seen those and will point it out as well, so you could also ignore/walk away from his posts if you find it irritating, but odds are he's pasting different parts of his codebase into different review requests and that's perfectly fine. Though at one point he should take the time to rename his variables...

\$\endgroup\$
  • 2
    \$\begingroup\$ Thanks for your response. I now realize that my problem is that I just felt hurt after seeing how much of my (and the other) answer – in which I invested some effort – have been ignored, while on most other reviews, I do not have any feedback at all. I'll take this as a chance to fine-tune my answer style, and let it rest for the night before taking further action. \$\endgroup\$ – amon Sep 12 '13 at 21:52
  • 2
    \$\begingroup\$ See the glass half-full: no news, good news! Most likely your contributions are appreciated, if not by the OP, by the community! That user seems new with SE altogether, might not be familiar with the concepts of upvoting and accepting answers yet :) \$\endgroup\$ – Mathieu Guindon Sep 12 '13 at 21:56
  • 6
    \$\begingroup\$ Oh, just saw that comment where he did respond... bah, at the end of the day you're not the one stuck with that ugly code base... \$\endgroup\$ – Mathieu Guindon Sep 12 '13 at 21:58
  • 2
    \$\begingroup\$ That I can drink to! Thanks for your insights. \$\endgroup\$ – amon Sep 12 '13 at 22:01

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .