Is this Perl program well-written?

Question

This is a perl program intended for my chemistry class. Since I am just learning to write in Perl, which is my first language, can you tell me if you think that this is well written? If not, could you tell me what is wrong with it or what could be improved.

## Name:Titration
## Beginning of Program

#Input Variables
system "clear";     # Clear the Screen

print "Milliliters Solute: ";
chomp ($milliliters_solute=<>);

print "Milliliters Solvent: ";
chomp ($milliliters_solvent=<>);

print "Molarity of Solvent: ";
chomp ($molarity_solvent=<>);

print "Moles of Solute: ";
chomp ($moles_solute=<>);

print "Moles of Solvent: ";
chomp ($moles_solvent=<>);

system "clear";     # Clear the Screen

#Calculate Answer
$liters_solute=$milliliters_solute/1000;
$liters_solvent=$milliliters_solvent/1000;
$molarity=($liters_solute/$liters_solvent)*($molarity_solvent/1)*($moles_solvent/$moles_solute);

#Print Answer
system "clear";     # Clear the Screen
print "Molarity is $molarity \n";

## End of Program

EDIT: One year later. I've learned bash shell scripting since, and have written a lot of scripts in that, but haven't done as much work in Perl. Nonetheless, I've improved my little program to have functions, something I learned from bash, and made other small improvements.

#!/usr/bin/perl
# Name: Titration
# Author: Jonathan Bondhus

use strict; # Use strict perl parser

# Define the variables
my($milliliters_solute) = 0;
my($milliliters_solvent) = 0;
my($molarity_solvent) = 0;
my($moles_solute) = 0;
my($moles_solvent) = 0;
my($liters_solute) = 0;
my($liters_solvent) = 0;
my($molarity) = 0;

# Define the functions
sub my_define;
sub my_calculate;
sub my_print;

# Call the functions
&my_define;
&my_calculate;
&my_print;

sub my_define{      # Input Variables
    system "clear";     # Clear the Screen
    # Input milliliteres of solute
    print "Milliliters Solute: ";
    chomp ($milliliters_solute=<>);
    # Input milliliters of solvent
    print "Milliliters Solvent: ";
    chomp ($milliliters_solvent=<>);
    # Input molarity of solvent
    print "Molarity of Solvent: ";
    chomp ($molarity_solvent=<>);
    # Input moles of solute
    print "Moles of Solute: ";
    chomp ($moles_solute=<>);
    # Input moles of solvent
    print "Moles of Solvent: ";
    chomp ($moles_solvent=<>);

    system "clear";     # Clear the Screen
}
sub my_calculate{   #Calculate Answer
    $molarity=((($milliliters_solute/1000)/($milliliters_solvent))*($molarity_solvent)*($moles_solvent/$moles_solute));
}
sub my_print{#Print Answer
system "clear";     # Clear the Screen
print "Molarity is $molarity \n";   # Print the molarity
}

exit 0  ## End of Program

Answer 1

One suggestion, use

use warnings;

use strict;

Will force you to be a better perl programmer, by enforcing more rigidity to your scripts, and have the intepreter catch any nasty issues that might be otherwise hard to debug.

Answer 2

Use these. At first they will make things harder, because you'll get a few error messages, you'll need to declare all variables with my, etc. But very quickly they will make your life better.

use strict;
use warnings;

Whenever you see repetitive code, consider writing a subroutine.

sub get_user_input {
    my $message = shift;
    print $message, ': ';
    chomp(my $reply = <>);
    return $reply;
}

Example usage.

my $milliliters_solute  = get_user_input('Milliliters Solute');
my $milliliters_solvent = get_user_input('Milliliters Solvent');

Good luck with your studies. One of my favorite teachers went by the name The Mole Man -- you can tell what subject he taught.

Answer 3

Beyond what's been mentioned, I can suggest following a few formatting conventions to make your life easier in the long run:

• Keep lines to a reasonable length, inserting line breaks at logical points where necessary. This helps avoid having to scroll horizontally, as well as improving readability. 80 columns is generally accepted as the standard right margin for most code.

• Insert spaces around infix operators to improve legibility:

  $liters_solute = $milliliters_solute / 1000;
  

• Don't put extra whitespace around escapes in strings, because...well, it's extra, and it's significant, and while it doesn't really matter here (at the end of a line), it's a good thing to pay attention to in the future.

  print "Molarity is $molarity\n";
  

• Don't put extra whitespace before function arguments:

  chomp($milliliters_solute = <>);
  

Answer 4

You've made an excellent start. Your comments are clear and your variable names are good.

I've made a much fancier version of your code that you might find useful to look at.

First thing I did was add a 'she-bang' line: #!/blah/blah/blah. On unix systems, this is used to identify the interpreter. On windows its mostly useless, but windows perls can detect command line options that may be specified there.

Next I laid in some POD describing your program. My chemistry knowledge sucks, so I probably screwed up the description horribly.

Then I added the 'strict' and 'warnings' pragmas, which make perl much pickier about things that are often indicators of bugs.

Next I put in some stuff to handle printing a usage message when someone says script --help or script -?. Now the script will print some useful info.

Then I took a bunch of repeated stuff (your print/chomp input collection) and wrapped it into subroutines.

I also wrapped up the calculation, but used a common idiom that gives subroutines named, rather than positional parameters. This is good thing to do when you have many parameters to pass.

Finally, I slipped in a subroutine to handle displaying the results. I used an interesting bit of relatively advanced technology to format the output.

#!/usr/bin/perl

=head1 Name

titration

=head1 SYNOPSIS

A simple titration calculator.

Interactively requests information from the user and 
calculates the molarity of a solution.

=cut

use strict;
use warnings;

use Pod::Usage;
use Getopt::Long;


# Handle command line args / help requests.
my $help;

GetOptions( 'help|?' => \$help )
    or pod2usage(2);

pod2usage(0)  if ($help);

# The real program.  I go back and forth over having a 'main' sub like a C program.

my %raw_data = collect_user_data();

my $molarity = calculate_molarity(
    ml_solvent       => $raw_data{'Milliliters Solvent'},
    ml_solute        => $raw_data{'Milliliters Solute'},
    molarity_solvent => $raw_data{'Molarity Solvent'},
    moles_solvent    => $raw_data{'Moles Solvent'},
    moles_solute     => $raw_data{'Moles Solvent'},
);

display_result( "Molarity is $molarity" );

exit;

# Subroutines ------------------------------------------

# Print an message and get a response.
sub get_user_response {
    my $question = shift;

    chomp $question;  # Make sure we don't have any messy newlines.
    print "$question: ";

    my $answer = <STDIN>;
    chomp $answer;

    return $answer;
}


# Ask for all the user data and store them in a hash.
sub collect_user_data {

    system "clear";     # Clear the Screen

    my %data;

    for my $unit (
       'Milliliters Solute', 'Milliliters Solvent',
                             'Molarity Solvent',
       'Moles Solute',       'Moles Solvent'
    ) {
        $data{$unit} = get_user_response( "Please enter the $unit" );
    }

    system "clear";     # Clear the Screen

    return %data;
}

sub display_result {

    system "clear";     # Clear the Screen

    # this is confusing magic that prints each argument with a trailing "\n".
    # Understand this and you have learned MUCH.
    print map "$_\n", @_;
}

# Do the molarity calculation
# Expects name / value pairs of arguments.
# 
# Arguments must include:
#    molarity_solvent
#    ml_solute 
#    ml_solvent 
#    moles_solvent 
#    moles_solute

sub calculate_molarity {
    my %arg = @_;

    my $molarity = $arg{molarity_solvent}
                 * ( $arg{ml_solute}     / $arg{ml_solvent}   )
                 * ( $arg{moles_solvent} / $arg{moles_solute} )
                   ;

    return $molarity;
}

"Oh, gee, thanks, but how the heck am I going to understand this?" you may be thinking. Well, perldoc is a great help. As is asking questions. perldoc.perl.org is a great resource for searching and reading Perl's documentation.

Check out the perlvar article for information on special variables. Of the many available, four are worth learning IMO. ($_, @_, $! and $@, if you care) For everything else, there's perlvar.

perlfunc is the function reference. My favorite section as a newbie to Perl was Functions by Category.

Don't forget about Stack Overflow, and most especially Perlmonks.org. These sites are great places to ask for help. Perlmonks is the great grand-daddy of SO, and such sites, it's decade-plus library of Q&A has a lot to offer, plus the people there are very friendly and helpful.

Answer 5

There isn't much to it but as a first program in any language it looks fine - reasonable use of comments (could be a little more verbose with them but it isn't really necessary for such a short application), code is readable.

If I were to suggest a single change it would be to learn to style things in functions rather than a pure linear flow of code. This will make your code easier to reuse later and for more complicated programs, easier to reuse portions within them.