Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl: the Markov chain saw
 
PerlMonks  

RFC: Module - GSM::Nbit

by techcode (Hermit)
on May 13, 2010 at 12:22 UTC ( [id://839828]=perlquestion: print w/replies, xml ) Need Help??

techcode has asked for the wisdom of the Perl Monks concerning the following question:

Dear fellow monks,

I worked on an module and getting ready to release it on CPAN. It's going to be my first release on CPAN, so please let me know if I did something really obviously stupid :) Another module that will depend on this one is also being done.

You can get it here: http://www.techcode.net/perl/GSM-Nbit-0.06.tar.gz, just untar and install as usual - might want to use INSTALL_BASE=/tmp/ for now ;)

-- README EXCERPT ---

GSM::Nbit - GSM 7bit and 8bit data encoder and decoder.

Throughout GSM world "special" encodings called 7bit and 8bit are used.
Encoding in 8bit is just plain HEX value and is provided here for completeness
and ease of use, 7bit packs 8bit data into 7bit HEX value by limiting it to the
lower 127 characters - and hence gaining 1 extra char every 8 characters.

That's how you get 160 characters limit on plain text (ASCII + few Greek chars)
messages with only 140 bytes for data.

Since many modules need such encodings in them, those functions are refactored
here. It's released as separate module and not part of some other distribution
exactly for that reason.

I personally needed to use it in few different modules.

INSTALLATION

To install this module, run the following commands:

	perl Makefile.PL
	make
	make test
	make install

Thanks!


Have you tried freelancing/outsourcing? Check out Scriptlance - I work there since 2003. For more info about Scriptlance and freelancing in general check out my home node.

Replies are listed 'Best First'.
Re: RFC: Module - GSM::Nbit
by toolic (Bishop) on May 13, 2010 at 17:45 UTC
    I don't see any big problems at all. Here are some minor suggestions for improvements.

    I believe you have a typo in your warn message: change 'characted' to 'character'.

    You should indent the code in the 'Code Sample' section of the SYNOPSIS in your POD. When I use perldoc, it renders poorly for me. You should also remove the tab characters in that code example because it still renders poorly even after I indented it.

    You should add more data validation for your input. If I do something that you probably don't expect, I do get lots of warnings, but they could be more helpful. I tried this (probably) illegal thing, and I got about 20 warnings:

    my $foo = $gsm->decode_7bit_wlen(5);
    Of course, that means you should add tests to your test suite. The Perl Best Practices book has a nice, succinct checklist of general-purpose things to test.

    As a user, I absolutely love it when a CPAN author includes a script I can run out-of-the-box, preferably in the bin directory, or at least in the examples directory. This could be nothing more than the code from the SYNOPSIS.

    There are plenty more style suggestions if you run the code through perlcritic.

    Update: Do this before you upload your tar file: Fix CPAN uploads for world writable files

    You should delete the INSTALLATION section from the POD. There is no need to tell someone how to install something which has already been installed.

    Another typo: change 'costructor' to 'constructor'. I guess you should run your code through a spell-checker.

      Yeah - there are some typos as I still need to run it through a spell checker (and perlcritic) - any tools for that?

      EDIT: perlcritic does a nice job of spell checking as well :)

      I had the code indented, but then in man pages it looked like double indented from being inside a section + code indentation itself. But I agree it looks better that way so reverting + removing spaces between variable, = and value.

      Yes for more data validation - I also "unified" so everything just does "return unless xyz;" instead of having both that and "return '' unless xyz;" in a few places.

      Agree on examples directory :)

      As of world writable files - that would be a bugger to figure out on my own. But looking at my folder structure I have standard chmod 644 for files and 755 for folders. So I'm not sure what's wrong?

      First point I don't completely agree on - I believe INSTALLATION section is there for the CPAN web page?

      Thanks!


      Have you tried freelancing/outsourcing? Check out Scriptlance - I work there since 2003. For more info about Scriptlance and freelancing in general check out my home node.
        As of world writable files - that would be a bugger to figure out on my own. But looking at my folder structure I have standard chmod 644 for files and 755 for folders. So I'm not sure what's wrong?
        To be honest, I do not understand why this is necessary. But, I do know that one of my uploaded tar files was rejected by CPAN testers because of a permission problem. When I ran bart's script on my tar file, the problem disappeared. I have used it on every upload since, and I have not had a repeat of the problem. It's easy to run, and it helps avoid hassles. Why not just do it?
        First point I don't completely agree on - I believe INSTALLATION section is there for the CPAN web page?
        INSTALLATION belongs in the README file, and you already have it there. I see no reason to duplicate it in your POD. Look at the POD for your 10 favorite modules. Do you see an INSTALLATION section in any of them?
        perldoc List::MoreUtils

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://839828]
Approved by Hue-Bond
Front-paged by ww
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others wandering the Monastery: (7)
As of 2024-04-23 11:07 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found