my $self = { $x => shift, $y => shift, $w => shift, $h => shift };
[download]

This has many problems. Apart from using four, uninitialised, package variables; $self actually ends up with this value:

{ "" => $_[4] }
[download]

Well seen, kcott++. I really read

my $self = { x => shift, y => shift, w => shift, h => shift };
[download]

instead.

Another thing with this line - original or as misread by me - is order of evaluation. I don't know by heart if Perl guarantees left-to-right order in this case. I can't find a reason why it should not, and if I could get my brain out of stand-by mode, I would probably compare with %hash=( %oldhash, 'this' => 'overwrites', 'anything' => 'found', 'in' => '%oldhash' ); and consider this harmless.

But the point is: I have to think about it. Maybe I have to RTFM. And apart from that, it is not elegant at all.

In our code review system, I would mark this line as "bad style, probably wrong", and it would have to be changed (or explained by some comments). I would not spend more time considering if it is really wrong. It is sufficient that it is not clearly right.

Our review system uses peer review, i.e. one developer reads the code of another developer, and the roles of coder and reviewer are swapped quite often. Understanding data flow and algorithms used is hard enough, and finding errors in there is even harder. Errors in our code can have the potential to hurt or - in the worst case - kill people, and so we want clean code.

That does not mean that we can't use some tricks in our code. But if we do, those tricks are commented to explain what happens. Tricks are quite rare, because our target hardware usually has some spare CPU cycles and some spare memory. So instead of using tricks, we prefer code that wastes a little bit of memory and / or CPU, but is easier to read.

Yes, I'm aware that games rarely kill real people, but code that is clean and easy to read makes it harder to accidentally hide bugs.

Alexander