{ANNOUNCE} Games::Quake::Log

Andrew Wilson andrew at soto.kasei.com
Fri Dec 28 05:30:30 CST 2001


Hi Marc

On Fri, Dec 28, 2001 at 10:55:03AM +0000, Stray Toaster wrote:
> OK, so it ain't monkey brains, but it does what it is supposed to. Yes,
> and I know the code is pretty sucky. That is what you get for coding in
> a rush one Sunday afternoon, then being too lazy to actually rewrite it
> and tidy it up. It is only for fun, dontcha know.
> 
> And I am in a defensive mood, so no critisim of it, Yet. Give me a few
> days, then get tore in.
> 
> c'est la vie. And my vie at that.
> 
> m.
> 
> Oh, go on, I know you want to tear it to bits....

Well if you insist.

I see several things that could be improved

1) It has no tests
2) It has no tests [0]
3) You see that init method.  That's too big that is.
4) It doesn't seem very OO to me, all those 

  if ($action eq foo) {
    $bar = $wibble
  } 
  
  sections should probably be methods called foo   
5) how would I go about subclassing this to override a small piece of
   functionality?  I wouldn't easily, that's a hint that your methods
   are doing to much (a common BlackStar failing if I remember
   correctly, wea always tried to do far too much in one method).
6) If you make the all simpler method calls (see point 40 it will be
   easier to write tests for them. [1]
7) Data structures.  You're quite mad.  You're also not going to
   start listening to my data structure rants at this point, are you?
   :-)

Nice module by the way.

cheers

Andrew

[0] Technically the same point, yes, but such a good point it's worth
    mentioning twice ;-)
[1] Ok, it's actually worth mentioning three times. [2]
[2] do the footnotes count, ah ... that's five times I've mentioned it
    has no tests.[3]
[3] Or is that six ... AAaarrrrrggggghhhhhhhhh!!!!!!



More information about the Belfast-pm mailing list