Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

comment on

( #3333=superdoc: print w/replies, xml ) Need Help??

anita2R:

Your code looks pretty nice, I'll have to try it out and check the E2PROM on some boards I have. I took a quick look at your code and have a couple suggestions (in no particular order):

  • You can simplify this statement: if ( ( $i + 1 ) / 16 == int( ( $i + 1 ) / 16 ) ) by using the modulo operator (%)--it does an integer division and gives you the remainder. Using it, you could write your statement as:
    if ( ($i+1)%16 == 0).
  • Generally you should declare your variables close to where you use them, in the innermost scope that makes sense. That limits the amount of code you need to review if you're looking for how the variable is used. This is causing you to double-comment certain things, kinda like:
    Line 38:
    my ($i2cBus, $i2cAddr);        # bus ID number & device address
    Line 54:
    #setup bus number, EEPROM address and max eeprom size in bytes $i2cBus = BB_I2C_PERI_1;

    If you declare the variable at the second point, you need only comment it once.

  • You're also commenting on things that need no comment. In the code below the comment we can see that it's setting the baud rate to 100kHz (due to the well-named value). In fact, with well-named variables, many comments are superfluous.
    # set baudrate to 100kHz (AT24C32 at 3.3v, max speed = 100kHz)) $objI2c->set_baudrate( $i2cBus, BB_I2C_CLOCK_100_KHZ );

    The comment about the part's limitations is something I'd put in the comment. If you decide later to use 50kHz because some flakey parts, you don't have to update your comment.

Keep in mind that these are just some trivial suggestions. If one of my colleagues gave me this code, I'd be perfectly happy with it going into production as it is (assuming that it does what it's supposed to). If you review some of my posts here, you'll notice that I frequently post code that could do with improvement as well.

...roboticus

When your only tool is a hammer, all problems look like your thumb.


In reply to Re: EEPROM on i2c Real Time Clock Modules by roboticus
in thread EEPROM on i2c Real Time Clock Modules by anita2R

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



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.
  • Log In?
    Username:
    Password:

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

    How do I use this? | Other CB clients
    Other Users?
    Others pondering the Monastery: (7)
    As of 2021-01-26 16:29 GMT
    Sections?
    Information?
    Find Nodes?
    Leftovers?
      Voting Booth?
      Notices?