HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Can I jump like this?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thiscanlikejump

Problem

In my nested for loops below, it is sometimes necessary to exit the outer loop and then continue program execution. Since loop labels and other goto-like constructions are heavily discouraged, I'm a bit afraid of raptors. Is this usage OK? Is there a better way to structure the program?

my @objects = load_objects();
OUTER: for my $obj ( @objects ) {
    for my $element ( @{$obj->elements} ) {
        if ( is_case_1( $element ) ) {
            ...
        } elsif ( is_case_2( $element ) ) {
            ...
        } else {
            warn "invalid element: $element";
            last OUTER;
        }
        stuff_that_assumes_valid_element( $element );
    }
    stuff_that_assumes_valid_object( $obj );
}
store_objects( @objects );

Solution

In general, the code is OK, i.e. I don't think it must be changed. However, to increase clarity, I would do a couple of things differently:

It's probably better to write an is_valid() method for your objects, so that whole inner loop is a single function call. Then, you can use grep to shorten the outer loop like this:

my @objects = load_objects();
    my @valid_objects = grep { $_->is_valid() } @objects;
    stuff_that_assumes_valid_object($_) for @valid_objects;
    store_objects(@valid_objects);


Also, depending on what stuff_that_assumes_valid_element does, you can either put that call into is_valid() or into stuff_that_assumes_valid_object. Either way, your top-level code works with objects and thus shouldn't know too much about single elements inside those objects, that's something the object itself should know and do (see Law of Demeter for reasoning).

Code Snippets

my @objects = load_objects();
    my @valid_objects = grep { $_->is_valid() } @objects;
    stuff_that_assumes_valid_object($_) for @valid_objects;
    store_objects(@valid_objects);

Context

StackExchange Code Review Q#2060, answer score: 3

Revisions (0)

No revisions yet.