Sample review

gistfile1.txt

<sartak> this the way you wrote it:

    sub _build_message {
        my $self = shift;
        my $line1 = "Attribute (".$self->attribute_name.") does not pass the type constraint because: ";
     
        if( $self->new_member ) {
            $line1 = "A new member value for foo does not pass its type constraint because: "
        }
     
        $line1.$self->type_constraint_message;
    }

<sartak> this the way I'd write it, which I think is clearer:

    sub _build_message {
        my $self = shift;
        my $line1;
     
        if ($self->new_member) {
            $line1 = "A new member value for foo does not pass its type constraint because: "
        }
        else {
            $line1 = "Attribute (".$self->attribute_name.") does not pass the type constraint because: ";
        }
     
        return $line1 . $self->type_constraint_message;
    }

<sartak> assigning a value to a variable only once makes things more understandable
<sweet_kid> yeah
<sartak> if you reassign a variable it's a little bit harder to understand the flow
<sartak> and, I like explicitly including "return" when the method body is anything but very trivial
<sweet_kid> I skip return statements
<sweet_kid> sorry
<sweet_kid> will include it from now onwards
<sartak> you don't have to include explicit "return" in all exceptions
<sweet_kid> okay
<sartak> maybe a good metric is: is there anything more complicated than variable assignment and string concatenation
<sartak> this had an "if" statement so it is more complicated
<sweet_kid> okay
<sweet_kid> you're right