Understanding Structures through Variable Names

[tedv]

Something to keep in mind is the deep nested nature of your hashes (objects). You have a list of hash refs. You call the tool() function on these objects (without parentheses) which returns a reference-to-hash. But you need a list of values in the hash itself. These values are, themselves, objects which you only care about so you can call the "count" function on them (once again without parentheses).

I feel like somewhere far back on the design board, the data structures and APIs were not designed properly. First, it's not even clear what kind of data you have after you run values %{$l->tool}. Yes, it's a list... But a list of what??? Suppose your had a function called "get_tool_list". Then you can write something like... foreach my $tool ( $l->get_tool_list() ) { ... }. Your code would be much better off if you forced yourself to use meaningful variable and function names. If the function names no longer match their purpose, either redesign the API, the class, or the function.

On the subject of variable naming, $l is not a recommended variable name. Not only does it look like $1, but it doesn't give you any clues about what the variable is used for. You'd be much better off writing something like...

my $count = 0;
foreach my $drill (@drills) {
    foreach my $tool ($drill->get_tools()) {
        $count += $tool->count();
    }
}

Now that code looks much cleaner. You might be able to clean up the code a little more, now that you know what everything does, using some other looping syntax.

The second block of code is not that bad, although clearly using 1 grep is better than 2 greps. It's only bad if you need to run that grep every single time you access an element. The code structure makes me think that @layer_list won't change between calls, so it's best to pre-compute that data once and then directly access @layer_list (instead of recomputing it on each function call). Of course, I'm making a large number of assertions about the surrounding context of this code, so use your better judgement.

-Ted