in reply to Re^6: Documenting Methods/Subs
in thread Documenting Methods/Subs

Part of the problem here is that your new could be improved. Personally I'm not a fan of having a 'new' that takes arguments; I prefer to leave that to factory methods. 
sub new {
  my $class = shift;
  return bless {}, $class;
}

sub new_with_logger {
  my $proto = shift;
  my $logger = shift;
  return $proto->new->set_logger($logger);
}

sub set_logger {
  my $self = shift;
  my $logger = shift;
  # If you insist...
  eval { $logger->isa('Logger') } or 
    croak "New logger must be an instance of Logger class"
  $self->{logger} = $logger;
  return $self;
}

[download]
Personally, I could do without the assertion at this point, relying instead on programmer's ability to read; okay, so the errors may happen some way away, but the error should be explicit enough for the programmer to realise what's amiss. However, if you insist on asserting that the logger is a Logger, the correct place to do this is definitely within the setting method itself, that way you catch all errors when someone tries to set the logger to an illegal value (assuming they don't just monkey directly with the hash, in which case they deserve all that's coming to them.)

Your comment about attempting to assert that 'this has to be a logger object in all subclasses' being impossible seems to miss the point. After all, a later subclass could take as its logger argument either a Logger or a logspec, which can be used to build an appropriate logger. All of a sudden your assertion about the logger argument is incorrect.

Replies are listed 'Best First'.
Re^8: Documenting Methods/Subs
by adrianh (Chancellor) on Jan 19, 2003 at 09:38 UTC
    Part of the problem here is that your new could be improved.

    Well, I was trying to keep the example simple :-)

    After all, a later subclass could take as its logger argument either a Logger or a logspec, which can be used to build an appropriate logger. All of a sudden your assertion about the logger argument is incorrect.

    In which case, I would change it.

    My aim when I do this sort of thing is to make the assumptions (or contracts if you prefer) of my classes to be explicit. Where possible I like to make them explicit using code rather than comments or documentation.

    If my contracts are wrong, I fix them. Having them in code makes it very obvious when they're wrong - which is a plus in my book :-)

[reply]
      After all, a later subclass could take as its logger argument either a Logger or a logspec, which can be used to build an appropriate logger. All of a sudden your assertion about the logger argument is incorrect.
      In which case, I would change it.
      Yick! I hate having to deal with dependent changes like that.

      Shall we agree to differ?

[reply]
        Yick! I hate having to deal with dependent changes like that.

        To me it's not a "bad" dependent change. A documented (in code) design/implementation decision was incorrect. It needs to be fixed.

        Having these decisions explicit makes it easier (for me) to spot when they wrong.

        Shall we agree to differ?

        Okay doke :-)

[reply]

In Section Meditations