Path: csiph.com!eternal-september.org!reader02.eternal-september.org!.POSTED!not-for-mail
From: Tim Rentsch <tr.17687@z991.linuxsc.com>
Newsgroups: comp.lang.c
Subject: Re: Can this program be improved?
Date: Sat, 25 Dec 2021 11:13:49 -0800
Organization: A noiseless patient Spider
Lines: 122
Message-ID: <868rw81or6.fsf@linuxsc.com>
References: <sq63mi$gom$1@dont-email.me> <sq6b3t$ce5$1@dont-email.me>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Injection-Info: reader02.eternal-september.org; posting-host="1fe31837ad76f46de1d32de2fe408cda"; logging-data="3928"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1/g8c8ugGmsnidodNUTVugN9ISRSxgJNwM="
User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.4 (gnu/linux)
Cancel-Lock: sha1:hyKhrfg25vmGT1zBTFXtO5zoDr8= sha1:HTemm04td317th7e+61s1z8wI5g=
Xref: csiph.com comp.lang.c:164065

Andrey Tarasevich <andreytarasevich@hotmail.com> writes:

> On 12/24/2021 5:00 PM, Manu Raju wrote:
>
>> [revised to repair white space]
>>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <math.h>
>>
>> int main()
>> {
>>     float P = 100000; // Loan Amount
>>     float r = 7.5; // Advertised Interest Rate
>>     int n = 12; // Loan Period
>>     float payBack = (P * r / 1200 * pow(1 + r / 1200, n))
>>         / (pow((1 + r / 1200), n) - 1);
>>     float Opening = P; // Opening Balance
>>     float closing = 0; // Closing Balance
>>     float interest = 0;
>>     float Total = 0;
>>     float repaid = 0;
>>     
>>     printf("\nMonthly payment is: %.2f\n", payBack);
>>
>>     printf("%10s %14s %13s %10s %14s %12s\n", "Month", "Beginning",
>>         "Interest", "Total", "Repayment", "Balance");
>>
>>     for (int i = 1; i <= n; i++)
>>     {
>>         for (int j = 0; j <= n; j++)
>>         {
>>             interest = Opening * r / 1200;
>>             Total = Opening + interest;
>>             repaid = payBack;
>>             closing = Total - payBack;
>>         }
>>         printf("%10d %14.0f %13.0f %10.0f %14.0f %12.0f\n", i,
>>             Opening, interest, Total, repaid, closing);
>>         Opening = closing;
>>     }
>>     return 0;
>> }
>
> Certainly.
>
> 1. int main(void)

Check.

> 2. Stop using `float` for local variables.  "Default" floating-point
> type in C is `double`. Everything else is used only if you have a
> good reason to do that.  In this case you don't.

Check.

> 3. Stop using "regular" floating point types for representing
> monetary quantities.  They are not good for that purpose.

For this level of programming I think floating point is okay.  A
problem that is important is the program makes no effort to round
(to the nearest cent, for example), so the calculation gives
funny results typical of calculations done with floating point.
But that problem can be addressed without throwing out floating
point altogether.

> 4. It appears that you are actually trying to use `float` to
> represent _integer_ quantities (judging by your `printf`).  Why?

To me that just looks like what the print formats are, not what
the kinds of quantities are.

> 5. Stop using `pow` for calculating integer powers.  You don't
> really need `<math.h>` here.

I have to vote against this comment.  Using pow() makes it
obvious what is being done, the exponent being integral
notwithstanding.

> 6. Stop piling up all variable declarations at the beginning of
> the function.  Declare variables as locally as possible.

Good advice here I would say, but this rule is a point of style
with differing viewpoints.

> 7. Stop using dummy initializers for your variables.  It is better
> to leave them uninitialized than initialize them to dummy zeros.
> This point is actually connected to the previous one:  once you
> start declaring variables as locally as possible, you'll normally
> have a meaningful initializers for them at the point of declaration.

Check.

> 8. What's going on with capitalization in your variables names?
> Opening`, `closing`, `interest`, `Total`?  Is this a convention of
> some sort?

Check.

> 9.  "Magical constants"?  What on Earth is `1200`?

Check.  However I think it should be added that the "magicness"
of the constant(s) can be addressed by rephrasing the expressions
where they are used, without necessarily taking the constants
out of those expressions.

> 10.  Repetitive subexpressions, like `1 + r / 1200` (some explicit,
> some slightly obfuscated) are probably a matter of style.... But
> anyway:  DRY - do not repeat yourself.

More significant here, I think, is that defining a variable with
the value of this expression offers an opportunity to give some
sort of descriptive name to the quantity.


Two additional points:

The #include <stdlib.h> can be removed.

The inner loop (with 'j' as the index variable) does the same
calculation over and over again.  There is no reason to make
that a loop - just write the body.