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

Re: [RFC] Review of module code and POD

by GrandFather (Saint)
on Apr 01, 2021 at 00:05 UTC ( [id://11130651]=note: print w/replies, xml ) Need Help??


in reply to [RFC] Review of module code and POD

As a general thing lines that are very long, especially if there is important stuff at the end, and even worse of the structure is convoluted, compared with short lines are hard to understand. Keep your lines short, or at least wrap them sensibly. For example:

return $self->{'dbh'}->selectrow_array("SELECT COUNT(*) FROM $Bod: +:Variables::M_Table WHERE $Bod::Variables::C_Table_id$Bod::Variables: +:C_Table = ?, $Bod::Variables::B_Table_id$Bod::Variables::B_Table = ? + AND Permission_idPermission = ? AND date > NOW() - INTERVAL 2 SECOND +", undef, $contact, $business, $permission);

is easier to maintain written:

return $self->{'dbh'}->selectrow_array( "SELECT COUNT(*) FROM $Bod::Variables::M_Table WHERE $Bod::Variables::C_Table_id$Bod::Variables::C_Table += ?, $Bod::Variables::B_Table_id$Bod::Variables::B_Tabl +e = ? AND Permission_idPermission = ? AND date > NOW() - INTERVAL 2 SECOND", undef, $contact, $business, $permission );

Consider using spaces instead of tabs for indentation.

Consider having new return an instance of Bod::CRM with its error field set, or throw an exception, for a DB connection failure.

Consider removing warnings in lower level code. Low level code may not have enough context to be really helpful and low level warnings can end up generating a noisy log. Calling code can emit warnings if appropriate and can generally provide a better semantic context.

Consider using exceptions for handling all errors so that calling code can chunk error handling into blocks of related code rather than having to deal with handling errors on all function calls. That extends quite well to having DBI code throw exceptions rather than having fine grain error checking at a low level (see RaiseWarn and RaiseError in DBI).

Consider passing named parameters to find and add. That way your existing hash ref gets passed in as a hash with no other change, but a simple find doesn't need to have a hash ref spun up. That makes it easier to write the code and easier to read and check:

my %fields = ( firstname => 'John', lastname => 'Smith', pri_email => 'john.smith@example.com', ); $crm->add(%fields); $crm->add(firstname => 'John', lastname => 'Smith', pri_email => ' +john@xxxx.com'); my @contacts = $crm->find(firstname => 'John');
Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond

Replies are listed 'Best First'.
Re^2: [RFC] Review of module code and POD
by hippo (Bishop) on Apr 01, 2021 at 10:05 UTC
    Consider using spaces instead of tabs for indentation.

    8 simple words to start a holy war.


    🦛

      From that node subtitled "My arguement in favor of tabs": "Reputation: -34 (+44 -78)" - says it all really.

      Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond
      Consider using spaces instead of tabs for indentation

      I never cease to be amazed at how The Monastery reveals that things I thought were standard practice...just aren't!

      The only formal education I have ever had relating to writing code was a short C++ course as part of my undergrad degree course and an even shorter Java course at another university as a small part of wider study. In both courses in the 1990's we were told to use tabs as indents and I didn't question why. Since then I have never questioned that anyone might do anything else...

      Is this a case of academia being out of touch with working practices or has the favour of spaces changes in the last quarter of a century?

        ... and I didn't question why.

        And thus is born tradition based on ..., actually I'm not sure what it's based on. Much bad process is a result of "because that's how I was told to do it" or, even worse, "that is how we did it at ...". From coding to coffee making to many other fields, progress is only made when people ask "Why?". The question shouldn't be judgmental or intended to create contention, but just for understanding and considering options. Emotive answers are almost always unhelpful, but white space and indentation issues often head that way.

        Optimising for fewest key strokes only makes sense transmitting to Pluto or beyond

        You might think it doesn't matter whether you use spaces or tabs. Yet be very wary when using the make build automation tool. In what Eric S Raymond described as "one of the worst design botches in the history of Unix", semantic meaning was assigned to a TAB character in column one of makefiles!

        This tragic UI blunder has cost thousands of hours of lost productivity over many years. I've personally suffered, puzzling over cryptic error messages from make after my editor automatically converted TABs to spaces ... so I naturally singled it out as an example of a UI mistake when I wrote On Interfaces and APIs:

        The Unix make utility decrees that the actions of a rule must start with a tab. If you accidentally insert a space before the leading tab, look out! Ditto if your editor or other tool is configured to automatically convert tabs to spaces. This unfortunate design choice is a violation of the principle of least astonishment because most programs treat spaces and tabs the same way. Moreover, when you hit a (typically cryptic) error message for using a space instead of a tab, it may take a long time to figure out what the problem is because tabs and spaces look the same in most editors.

        Is this a case of academia being out of touch with working practices or has the favour of spaces changes in the last quarter of a century?
        It probably just happened to be the personal preference of your two lecturers. An analysis of the top 400,000 GitHub repositories reported that spaces were more popular than tabs in all languages except C and golang ... with tab indentation dominating golang presumably due to the gofmt command mandating:
        Gofmt formats Go programs. It uses tabs for indentation and blanks for alignment.

        > In both courses in the 1990's ... Is this a case of academia being out of touch

        The problems of the tab character "\t" were not as apparent in the 1990s like they were later.

        Different contexts have different indentation settings, while most people agree on 4 columns nowadays, do most apps default to 8 columns, which used to be standard.

        The chaos starts if people mix tab with space and everything looks perfectly indented in their own editor, but once it's opened with another editor ... or posted to perlmonks, it becomes a mess (yes browsers default to 8).

        I actually use the tab-key for indentation, but my editor has a setting to insert space. And a command to "untabify" foreign code.

        One benefit of "\t" is that it can be easily reverted with one backspace instead of four. But again, this can be fixed with modern editors and I use automatic indentation anyway.

        And I wasn't talking about semantic effects.

        Perl code is thankfully resilient against "\t" vs " " confusion, and whitespace rarely matters.

        But ask the Pythonistas what they think was Guido's biggest design error...

        In short: Academics in the 2020 wouldn't say this anymore.

        Cheers Rolf
        (addicted to the Perl Programming Language :)
        Wikisyntax for the Monastery

Re^2: [RFC] Review of module code and POD
by Bod (Parson) on Apr 02, 2021 at 21:23 UTC
    Consider passing named parameters to find and add. That way your existing hash ref gets passed in as a hash with no other change, but a simple find doesn't need to have a hash ref spun up.

    Thank you - I don't know why I didn't do it exactly like that because most of the time it will be a hash, not a hashref that gets passed in. Except there are a lot of parameters which could change...hence why I read the table columns out of the information_schema in the new method.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://11130651]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others exploiting the Monastery: (5)
As of 2024-03-29 15:32 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found