Create a gist now

Instantly share code, notes, and snippets.

Sample review
<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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment