Efficiency of code

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

I would say I'm fairly acquainted with Perl, with considerable room to improve. My experience is limited to mostly small projects, nothing fancy. One system I have is for my university's newspaper site. When I planned the system I unfortunately didn't have any database experience. Eventually I will switch to a MySQL-based system, but not just yet.

In the mean time, I've been using MySQL for smaller, personal projects. I whipped up one today that displays the grades for my courses, with a computed average and such. The table is maintained via phpMyAdmin simply because I was too lazy to implement my own when one was readily available on the hosting company I use. What I'm getting at here is I want to improve the quality of the code. Especially before I write the database system for the newspaper. I would say the goal is mostly to get people's feedback on the Perl code, but feel free to throw MySQL suggestions (albeit there isn't a great deal of either).

I would appreciate any suggestions for improving any aspect of the code, in terms of efficiency or elegance. I'm almost positive there has to be a better way of getting the letter grade (maybe a hash with a range of values, not sure how to do that though).

-----

# declare mod usage
use DBI;
use Echo;
print Echo::carp;    # content-type header

# predefined vars
@courses = ('COSC 314', 'COSC 321', 'COSC 444', 'COSC 423', 'MATH 319'
+);
$htdocs = Echo::path("htdocs");
$current = 0;
$| = 1;

# open body template
$file = "$htdocs/html/grades/body.txt";
open(READ, $file) or die "can't open $file: $!";
  $body = join("", <READ>);
close(READ);

# connect to mysql db
dbh = DBI->connect("DBI:mysql:host=localhost;database=DATABASE", "USER
+", "PASS", {PrintError => 0, RaiseError => 1});

# cycle through courses
foreach $course (@courses) {

  # keep track of current class
  $current++;

  # execute query
  $cursor = $dbh->prepare("SELECT * FROM grades WHERE className='$cour
+se'");
  $cursor->execute();

  # reset vars
  $earned = $possible = 0;
  $assignments = "";

  # loop through result
  while (@result = $cursor->fetchrow_array()) {

    # open assignment template
    $file = "$htdocs/html/grades/assignment.txt";
    open(READ, $file) or die "can't open $file: $!";
      $assignment = join("", <READ>);
    close(READ);

    # substitutions
    $assignment =~ s/VAR_ASSIGNMENTTITLE/$result[1]/;
    $assignment =~ s/VAR_POINTSEARNED/$result[2]/;
    $assignment =~ s/VAR_POINTSPOSSIBLE/$result[3]/;
    $assignments .= $assignment . "\n";

    # add up points
    $earned += $result[2];
    $possible += $result[3];
  }

  # add padding to last TR
  $assignments =~ s/(.*)<tr>/$1<tr class=last>/s;

  # calculate average and grade
  if ($possible ) { $average = sprintf("%2.2f", ($earned / $possible) 
+* 100); } else { $average = "?"; }
  if ($average >= 95) { $grade = "A"; }
    elsif ($average >= 90) { $grade = "A-"; }
    elsif ($average >= 87) { $grade = "B+"; }
    elsif ($average >= 84) { $grade = "B"; }
    elsif ($average >= 80) { $grade = "B-"; }
    elsif ($average >= 77) { $grade = "C+"; }
    elsif ($average >= 74) { $grade = "C"; }
    elsif ($average >= 70) { $grade = "C-"; }
    elsif ($average >= 67) { $grade = "D+"; }
    elsif ($average >= 64) { $grade = "D"; }
    elsif ($average >= 60) { $grade = "D-"; }
    elsif ($average eq "?") { $grade = "?"; }
    elsif ($average < 60) { $grade = "E"; }

  # fix points
  if ($possible == 0) { $earned = $possible = "?"; }
  
  # open class template
  $file = "$htdocs/html/grades/class.txt";
  open(READ, $file) or die "can't open $file: $!";
    $class = join("", <READ>);
  close(READ);

  # substitutions
  $class =~ s/VAR_CLASSNAME/$course/;
  $class =~ s/VAR_POINTSEARNED/$earned\/$possible/;
  $class =~ s/VAR_AVERAGE/$average/;
  $class =~ s/VAR_GRADE/$grade/;
  $class =~ s/\t\tVAR_ASSIGNMENT/$assignments/;
  $body =~ s/\t      VAR_CLASS$current/$class/;
}

# clean up
$cursor->finish();
$dbh->disconnect();

# substitutions
$body =~ s/VAR_SEMESTER/Fall 2004/g;

# print
print $body;
[download]

Comment on Efficiency of code Download Code

Replies are listed 'Best First'.
Re: Efficiency of code by ikegami (Patriarch) on Sep 25, 2004 at 01:33 UTC
Looks good. The if is a bit shaky, and I don't mean the one you think: if ($possible) { $average = sprintf("%2.2f", ($earned / $possible) * 100); if ($average >= 95) { $grade = "A"; } elsif ($average >= 90) { $grade = "A-"; } elsif ($average >= 87) { $grade = "B+"; } elsif ($average >= 84) { $grade = "B"; } elsif ($average >= 80) { $grade = "B-"; } elsif ($average >= 77) { $grade = "C+"; } elsif ($average >= 74) { $grade = "C"; } elsif ($average >= 70) { $grade = "C-"; } elsif ($average >= 67) { $grade = "D+"; } elsif ($average >= 64) { $grade = "D"; } elsif ($average >= 60) { $grade = "D-"; } elsif ($average < 60) { $grade = "E"; } } else { $grade = $average = $earned = $possible = "?"; } [download] One way to get rid of the big if: `@GRADES = qw( E D- D D+ C- C C+ B- B B+ A- A ); @BRACKETS = ( 60,64,67,70,74,77,80,84,87,90,95 ); if ($possible) { $average = sprintf("%2.2f", ($earned / $possible) * 100); $grade = 0; foreach (@BRACKETS) { $grade++ if ($average >= $_); } $grade = $GRADES[$grade]; } else { $grade = $average = $earned = $possible = "?"; }` [download] And how about only preparing the query only once: `# prepare query $cursor = $dbh->prepare('SELECT * FROM grades WHERE className=?'); # cycle through courses foreach $course (@courses) { # execute query $cursor->execute($course); ... } ...` [download]	[reply] [d/l] [select]
Re: Efficiency of code by saberworks (Curate) on Sep 25, 2004 at 20:39 UTC
The database server, in this case, MySQL, is perfectly capable of calculating averages and whatnot as you're pulling data out of the database. Check out the MySQL Manual Numeric Functions. And as always, the absolute most important thing to do is always `use strict;` and `use warnings;`	[reply] [d/l] [select]