Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??

sub index_menu { return [ map { anchor(textify($_) => {href => $_}) } sort { article_sort($a,$b) } grep { m/^[A-Z]./ && -f $_ } file_list(shift() // '.') ]; }

Why I did it this way:

  • return [...];: Makes very obvious what is about to happen; we're returning an array ref.
  • Braces aligned both left and right: similar things should look similar. Why? Because if they suddenly look dissimilar it means I've made a mistake; missing closing brace, for example.
  • grep first operation to reduce the sort problem space. The problem space may be small today but it's just a good habit.
  • Call file_list() directly and pass its response into grep: Why store in an explicit intermediate variable if you're already chaining all the rest of the operations? This reduces the amount of state one has to be concerned with.
  • shift() // '.': This may be controversial; we're not unpacking args at the top of the sub. But unpacking is the first thing that happens, and it happens within the first statement. The advantage we get is that one doesn't have to remember yet another variable or be concerned with what it may have act upon it. The brevity reduces the amount of state one needs to keep in-head. But yes, this last one is probably controversial since it would probably violate a PBP rule.

Also m/^[A-Z]./ has the same effect here as m/^[A-Z].+/; either way you're matching any string that starts with A-Z and contains at least one more character. But the one without the quantifier doesn't have to read forward to the end of the string unnecessarily.

Finally, the abstraction may be wrong altogether. Here we're splitting hairs on how to format the code or what order sort and grep should come in, but overlooking the fact that anchor(textify(...), {href => ...}) looks a lot like HTML generation, and that you would probably be better served with a proper MVC framework and at very least with the use of a templating system. At minimum a template system to generate the HTML would be cleaner, and could likely be dropped into your code without other major refactoring.


Dave


In reply to Re: Refactoring just to refactor? by davido
in thread Refactoring just to refactor? by Lady_Aleena

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 about the Monastery: (2)
As of 2024-04-26 00:27 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found