On Wed, Mar 12, 2008 at 8:43 AM, Moon, John <[EMAIL PROTECTED]> wrote:
> sub IFormat_num{
> #
> # value = number to format, required
> # places= decimal places, required
> # float = character to prefix string with
> # credit= credit format, (<1>,2,3) = ("surround string with <>",
> "CR suffix", "- suffix")
> #
Since these comments talk about "credit formats", it seems safe to
assume that you're doing something financial. I'm curious, then, why
you use a parameter for decimal places, instead of a constant value of
2. Is there some decimal currency that doesn't use two digits after
the decimal point? Or are you (probably wisely) allowing for such
currencies to arise in the future? Or are you even dealing directly
with dimes or mills in your current application?
> my ($value, $places, $float, $credit) = @_;
> $credit = 1 unless $credit =~ /[1-3]/;
It looks here like you're trying to make credit format 1 be the
default, in case the caller has specified anything else. But what if
the specified format is 21, or -1, or 5.1? Also, I see in the code
below an if-elsif-else whose ordering would make the third case the
default. Which should it be?
$credit = 1 unless $credit == 2 or $credit == 3;
> my $is_credit = 1 if $value < 0;
Although this code is allowed, it's often considered poor form to
attach a conditional expression to a statement with a 'my'
declaration, partly because many programmers don't understand what
happens when the statement is not executed. In this case, since
$is_credit looks like it's trying to be a Boolean value, it's simple
to use the one that Perl provides:
my $is_credit = $value < 0;
> my $format = '%.' . $places . 'f';
Why not interpolate? Is it because of the trailing "f"? Use curly braces:
my $format = "%.${places}f";
> my $formated = $float . &commify(sprintf($format,abs($value)));
It is quite confusing to my little mind to have a variable named
$float in code which is working so intimately with floating point
values, and yet the variable itself is no floating point value at all.
Wouldn't a potential maintainer of your code be happier if $float
became $output_prefix? And I know I'd be happier if $credit became
$credit_format and $formated became $formatted. But maybe that's just
me.
> if ($credit == 1) {
> return $is_credit?'<' . $formated . '>':$formated . ' ';
> }
> elsif ($credit == 2) {
> return $is_credit?$formated . 'CR':$formated;
> }
> else {
> return $is_credit?$formated . '-':$formated . ' ';
> }
Again, interpolation would make this clearer. And oops I spelled it again:
if ($credit_format == 1) {
return $is_credit ? "<$formatted>" : "$formatted ";
} elsif ($credit_format == 2) {
return $is_credit ? "${formatted}CR" : $formatted;
} else {
return $is_credit ? "$formatted-" : "$formatted ";
}
Why do some formats add trailing spaces for positive numbers, but some
don't? And the question I hate to ask: Are you or your manager going
to spend much of next month wishing you'd started this task with a
more precise specification of your goals, when your client asks you
how to align the decimal points in a vertical column of these
formatted values?
> -----Original Message-----
Hey! If I'd known you were posting upside down, I would have stood on
my head to read your message. Good thing you didn't say anything
important. Except....
> Subject: Re: Sum not producing zero
Oh, right. Hey, if using this subroutine is the "solution", does that
mean that you're adding up your financial numbers with accumulating
round-off error and then using this code's rounding off to fake-up the
final answer? Say it ain't so! Use floating point values when and
where you have to, but calculate every financial value to an integer
numbers of cents (usually) if you want the books to balance at the end
of the day.
Hope this helps!
--Tom Phoenix
Stonehenge Perl Training
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
