Beefy Boxes and Bandwidth Generously Provided by pair Networks
Problems? Is your data what you think it is?

Payment changer

by uksza (Abbot)
on Dec 28, 2004 at 09:03 UTC ( #417727=perlquestion: print w/replies, xml ) Need Help??
uksza has asked for the wisdom of the Perl Monks concerning the following question:


I'm still practise my skill in Perl. I walk slowly, but it's very nice travel by Camel's track.
So, I need some advices about style code.

Look please at this:
To try OO programing in Perl I've made module to calculate how many bills and coins I need to pay for example $133 = (100+20+10+2+1).
It's propably quite helpful for employers.
He can't get 5x$100 from bank for 6 workers ($135 + $80 + $66 + $150 + $69= $500),
but he can do this:
#!/usr/bin/perl use warnings; use strict; use payment; my @my_bills = qw (200 100 50 20 10 5 2 1); my @my_payments = qw{135 80 66 150 69}; my $mp = payment->new(); $mp->change(@my_bills); foreach (@my_payments) { $mp->add($_); } my %result = $mp->get(); foreach ( sort { $b <=> $a } keys(%result) ) { print "$_ \t: $result{$_}\n"; }
and this is code of
package payment; use warnings; use strict; sub new { my $class = shift; my $self = {}; $self->{units} = undef; $self->{bills} = undef; bless $self, $class; } sub change { my $self = shift; @{ $self->{units} } = @_; %{ $self->{bills} } = map { $_ => 0 } @_; } sub add { my $self = shift; my @units = @{ $self->{units} }; my %pieces = %{ $self->{bills} }; my $temp = shift; my $unit; my $i = 0; while ($temp) { $unit = $units[$i]; while ( $temp >= $unit ) { $temp -= $unit; $pieces{$unit} += 1; } last if $i == (@units); $unit = $units[ ++$i ]; } %{ $self->{bills} } = %pieces; } sub get { my $self = shift; return %{ $self->{bills}}; } 1;
So, what do you thing about this? My style is good? Have you any advices for me? If it's good, I'm going to add some features into my class. ;-)

Thanks for your's advices and time.

Replies are listed 'Best First'.
Re: Payment changer
by trammell (Priest) on Dec 28, 2004 at 16:01 UTC
    The code looks fine to me, but where's the documentation? You might want to add some POD or at least a few comments before adding new features.
Re: Payment changer
by Forsaken (Friar) on Dec 29, 2004 at 00:29 UTC
    1 small comment i'd like to make is on the following part:
    sub add { my $self = shift; my @units = @{ $self->{units} }; my %pieces = %{ $self->{bills} }; my $temp = shift;
    I personally use the guideline that it's best to clean out @_ and assign all the values that i need before doing anything else. 9 out of 10 times this is completely unnecessary, but consistently doing it does prevent me from leaving things in @_ only to have them changed later, yielding unpredictable results. so my advice would be to start off with my ($self, $temp) = @_; and then to proceed with the remainder of the sub.

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://417727]
Approved by pelagic
LanX where is the camera?
Tux => $HOME
[Eily]: you wrote boos instead of boss, and jedikaiti read that as boobs :)
[LanX]: Win claims only 8 MB left on C: how do I find out where the problem happens?
[Eily]: by Tux. And going home sounds like a good idea
[Eily]: LanX pretty much everywhere except in 8MB ?
[Eily]: s/by/bye/ someone hacked my keyboard I'm sure
[LanX]: good point! :)
[Eily]: LanX I don't understand the question though, you want to know what is taking so much space?
[Tanktalus]: Probably the swapfile :)

How do I use this? | Other CB clients
Other Users?
Others wandering the Monastery: (13)
As of 2017-09-20 16:42 GMT
Find Nodes?
    Voting Booth?
    During the recent solar eclipse, I:

    Results (237 votes). Check out past polls.