My professor told me to optimize my perl code, but he wouldn't tell me what exactly - just make it simpler.

I changed some little things but nothing major. Am I missing something?

The task was:

Perl program that allows the user to simulate the simultaneous throwing of several cubes. The simulation result is then a total number of points, which is the sum of the numbers of the eyes results in single throws. If many of these throws are performed with several cubes, the frequency distribution for the total number of eyes is calculated. This frequency distribution should be in the console window in the form of a table and as a simple overview chart.

At the beginning the following entries should be requested by the user:

  • the number of dice he wants to roll.
  • the number of throws he wants to perform with the selected number of dice.

The frequency distribution for the total number of eye sets should be displayed in two different ways:

  • Tabular representation as a comparison of the number of points obtained and the associated frequency, both the absolute values ​​of the frequency and the normalized values ​​that result in the division of the absolute values ​​resulting from the total number of throws are issued.
  • Graphic representation as a simple (tilted by 90 degrees) bar chart (e.g. corresponding number of *)

Use subroutines.

use warnings;                   
use diagnostics;                
use GD::Graph::hbars;        
use GD;
use List::Util qw(max);
my $number_of_dice = 0;   
my $number_of_rolls  = 0;     
my $sum_of_throw = 0;     
my @total_sum_of_eyes;       
my @single_sum_of_eyes;   
my @frequency;            
my %count;                   
my $count =0;                 
my $number =0;                

print "\n Please insert the number_of_dice: ";   
print "\n Please instert the number_of_rolls: ";

while ($count< $number_of_rolls) {                  
representation ();

sub roll{ 
    $sum_of_throw = 0;
    for($i = 0; $i< $number_of_dice; $i++)      
        $number = int(rand(6) +1);                  

sub frequency{
    foreach my $eyes(sort{$a <=> $b} @total_sum_of_eyes){                       
sub representation{
    #Tabular representation
    print "\n the frequencys-distribution is: \n";
    print "\n total_sum_of_eyes\t frequency\t frequencys-distribution in %\n";
    foreach my $eyes (sort{$a <=> $b} keys %count){         
        push(@frequency, $count{$eyes});                #data for the bar chart     
        push(@single_sum_of_eyes, $eyes);               #data for the bar chart         
        printf "\n\t $eyes\t\t\t  $count{$eyes}\t\t%g",($count{$eyes}/$number_of_rolls)*100;
    print "\n";
    #bar chart
    my $graph = GD::Graph::hbars->new(1600, 600);       
    x_label             => 'total_sum_of_eyes',         
    y_label             => 'frequency',             
    title               => 'frequencys-distribution',   
    y_max_value         =>  max(@frequency)+1,          
    y_min_value         =>  0,                          
    y_tick_number       => 8,                           
    transparent         => 0,                           
    bgclr               => 'white',                     
    long_ticks          => 1,                           
) or die $graph->error;                                 
my @data = (\@single_sum_of_eyes,\@frequency);      
$graph->set( dclrs => [ qw(green) ] );   
my $gd = $graph->plot(\@data) or die $graph->error;  
open(IMG, '>gd_bars.gif') or die $!;
binmode IMG;
print IMG $gd->gif;

  • 3
    \$\begingroup\$ Welcome to Code Review! I reformatted your question for better readability and also reworded the title in order to state the task the code should accomplish. In the future, you should do this yourself. Have a look at our How to Ask page to get some hints how to get there. \$\endgroup\$ – AlexV Dec 11 '19 at 11:47
  • 1
    \$\begingroup\$ Use smallest possible scope for my variables. Also pass variables to functions as arguments, and use return $var to return result from functions. This is not perl specific but applies to almost every language. That sort for $count{$eyes}++ doesn't do anything useful. Use simpler, perl specific loops, for (1 .. $number_of_rolls) \$\endgroup\$ – mpapec Dec 12 '19 at 11:42
  • 1
    \$\begingroup\$ Dont use same variable names for $count, and %count. \$\endgroup\$ – mpapec Dec 12 '19 at 11:44
  • 1
    \$\begingroup\$ @mpapec thank you \$\endgroup\$ – alec Dec 12 '19 at 13:09
  • 2
    \$\begingroup\$ New versions of code should generally go into a new question. Having the code under review change makes life hard for folks trying to review it. \$\endgroup\$ – chicks Dec 12 '19 at 21:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.