Beefy Boxes and Bandwidth Generously Provided by pair Networks
Come for the quick hacks, stay for the epiphanies.
 
PerlMonks  

comment on

( [id://3333]=superdoc: print w/replies, xml ) Need Help??
This is perhaps a prime demonstration of several factors of releasing code too early. Most of these suggestions are excellent suggestions. A few I was aware of, but failed to act on.

I was wound up on this code. Perhaps, as merlyn would say, I had too much emotional attachment to it. I got it written, it worked, I was ready to release it. I failed to take the time to apply several things that I conciously know, or decided that it didn't warrant fixing them. As an example of code for others in the catacombs, it should set good examples. I've replied to tilly's post with my reasons or excuses (take your pick) about why certain things were done.

To make it perfectly clear, and be perfectly honest, I did not and do not take tillys suggestions as attacks on my code. I was a little mortified about a couple of them slipping past a veteran programmer, and I had a couple of 'So what? It's my code! Who *is* this guy?' thoughts to myself. But I let go, got objective, and realized that most of the suggestions were very good.

So, let's take a tour of what I did wrong, why I might have done it, and what I did about it.

  1. Don't use "my" for things you are using as global variables. Instead "use vars". This gives more context to what you mean, and the habit will avoid some problems if you ever use mod_perl.

    You're correct about this. I have a bad habit when doing quick hacks to use 'my' for globals. Corrected.

  2. Keep lines down to 80 characters. Perl has no rule about how many lines a single line of code takes, so you can and should break a line of code across several actual lines for readability. This makes it easier to read and easier to print.

    Sorry, gotta disagree on this one. I use a modern editor that allows horizontal scrolling, and have a good sized desktop. I format my code the way I like to edit it, and that doesn't involve unnecessary line breaks. It's not my habit to print code for debugging, so I don't sweat long lines. Matter of personal style.

  3. Your "scalar $#array+1" construct is redundant. Either use ($#array+1) or use "scalar @array".

    Entirely correct. I think I had a keys() there before, and forgot to remove the scalar. Corrected.

  4. If you are going to check parameters then use Carp, and confess to the problem, not die. That will give you a stack backtrace which makes the actual mistake far easier to track down. (In general aim to have every error message have enough information for debugging.)

    Some of the dies() aren't meant to produce an error messag, per se, and include the \n to prevent the line number. The rest could benefit from croak, and have been changed. In more complicated code, I probably would have used croak more liberally. As it is, this never gets very deep, and the die messages tell me enough information. But, in the interest of fostering better programing, I agree. Corrected.

  5. Put the expected action first. For instance in write_file do an "or die" because you don't expect to. (Suggestion straight from perlstyle.)

    I guess it's a matter of my thought processes on this one. I think to myself "OK, we're gonna die() if...", so I write it that way. However, in most of the cases it does make more sense. Corrected.

  6. In write_file you can and should use a hash slice instead of writing the hash lookup 5 times.

    Not sure about this one. The print() method requires an array referrence, and to get the fields in the correct order in the CSV file requires extracting them in correct order. Perhaps I miss your point, and you could demonstrate what you mean.

    Update:Added tyes patch from node 26404[] to the code, and understand what tilly was getting at.

  7. Personally I don't like your formatting for reasons explained in Code Complete. Namely that it right-shifts very fast, and it is a maintainance nightmare if anyone changes a line of code. (Because then you have to change several others.) Instead I suggest using whatever your standard block indent is.

    Code Complete is an excellent book. However, I do not agree with *everything* in it, and this is one of those points. I prefer heavy indention. Since I can block shift with my editor, I don't feel that I run into this problem, and am comfortable with it. Sorry. <G>

Thanks to tilly for some excellent suggestions, and for caring enough to post those suggestions. And thanks for the way they were worded. You could have been a S.O.B., and really fired some nasty comments about it. As it was, the presentation made it very clear that these were suggestions, and not "You're a moron, what are you doing writing code?" attacks.

--Chris

e-mail jcwren

In reply to (jcwren) RE: RE: luke_repwalker.pl by jcwren
in thread luke_repwalker.pl by jcwren

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 chilling in the Monastery: (6)
As of 2024-03-28 13:40 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found