Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??
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

In reply to Understanding Structures through Variable Names by tedv
in thread Am I developing poor style? by el-moe

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post; it's "PerlMonks-approved HTML":



  • Are you posting in the right place? Check out Where do I post X? to know for sure.
  • Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
    <code> <a> <b> <big> <blockquote> <br /> <dd> <dl> <dt> <em> <font> <h1> <h2> <h3> <h4> <h5> <h6> <hr /> <i> <li> <nbsp> <ol> <p> <small> <strike> <strong> <sub> <sup> <table> <td> <th> <tr> <tt> <u> <ul>
  • Snippets of code should be wrapped in <code> tags not <pre> tags. In fact, <pre> tags should generally be avoided. If they must be used, extreme care should be taken to ensure that their contents do not have long lines (<70 chars), in order to prevent horizontal scrolling (and possible janitor intervention).
  • Want more info? How to link or How to display code and escape characters are good places to start.
Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others cooling their heels in the Monastery: (6)
As of 2024-04-18 15:04 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found