use Perl

• Code smell heuristic (Score:2)

  by jdavidb (1361)

  conditional checks in OO code prove themselves to be code smells.

  Thanks for the heuristic! I never realized that before! I think this will help improve my coding.

  (If there's a larger source where you picked that up, I'd be benefitted by a pointer.)

  --
  J. David works really hard, has a passion for writing good software, and knows many of the world's best Perl programmers
  • Re: (Score:2)

    by Ovid (2709)

    I figured a concrete example would help. Imagine this awful method from some Catalyst code I was working on:

    sub _set_group_by : Private {
        my ( $self, $c ) = @_;
        my $group_by = 'day';
        my $period = $c->stash->{report_metadata}{period};
    
        my $report_range
                = $c->stash->{report_metadata}{start}->day_month_year
                . ' – '
             
    • Re: (Score:1)

      by theguvnor (1723)

      Thanks for the example, but I'm still confused... take pity on this dim one, and explain why the conditional check hasn't just moved elsewhere in your code where you decide which method to call? Or is that the entire point?
      • Re:Code smell heuristic (Score:2)

        by Ovid (2709) on 2008.04.29 16:42 (#62483) Homepage Journal

        In this case, it's done via a customer selecting a URL, so it's up to the customer to decide what they want and we don't have to.

        A better example would be with Java:

        public void setName(String name) {
            this.name = name;
        }
        
        public void setName(String first, String last) {
            this.name = first + " " + last;
        }

        In regular Perl (no modules), it sometimes gets mistakenly rewritten as this:

        sub name {
            my ( $self, $first, $last ) = @_;
            $self->{name} = $last ? "$first $last" : $first;
        }

        While that's a silly example, it means that you cannot set the last name to a value which evaluates as false. Variants of this bug occur all the time because every conditional you embed has a great chance of adding bugs. With Java, conditional argument handling bugs are much less common (but the methods bind at compile time instead of runtime, so it's a trade-off, but that's another rant :).

        Reply to This

        Parent