Why is this returning Garbage?

Question

I am making a calculator for a game.
I usually first make sure the code is working in C++, and then convert it to the desired language. I had made this program. In this program, some of the times, optDura stores the correct value and other times, it stores garbage.

Here is the code :-

#include <iostream>
using namespace std;

/* 
    cd - current Durability
    se - smith efficiency in decimal
    sc - smith charges in decimal
    sE - smith efficiency in %
    sC - smith charges in %
    ic - initial cost
    md - maximum durability
    tmd - temporary maximum durability
    tD - temporary durability
    td - total durability
    rc - repair cost
    cpb - cost per battle
*/
int main(){

    long int currDura, maxDura, tempMaxDura, tempDura, totDura, optDura;
    double iniCost, repCost;
    long int  smithEffi, smithCharge;
    double se, sc;
    double totCostTillNow, costPerBattle = 0, minCPB;
    int i;
    long int repCount = 1;
    iniCost = 25500;
    currDura = 58;
    maxDura = 59;
    repCost = 27414;
    se = 0.90;
    sc = 0.85;
    tempMaxDura = maxDura;
    tempDura = currDura;
    totDura = tempDura;
    totCostTillNow = iniCost;
    costPerBattle = totCostTillNow / totDura;
    minCPB = costPerBattle;
    cout<<"\n Cost without any repair = "<<costPerBattle;

    for(i=1; i<=maxDura; i++)
    {
        totCostTillNow += (double) repCost * sc;
        tempDura = tempMaxDura * se;
        totDura += tempDura;
        costPerBattle = (double) (totCostTillNow / totDura);
        tempMaxDura -= 1;
        if ( minCPB >=  costPerBattle )
        {
            minCPB = costPerBattle;
            optDura = tempMaxDura;
        }
    }
    cout<<"\n Minimum cost per battle = "<<minCPB<<" & sell at 0/"<<optDura;
    return 0;
}

The problem occurs when repCost is >= 27414. For values less than that, it gives the desired result. I am unable to figure out why this is happening.

Thanks a lot

It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: **[How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/)** — NathanOliver, Jun 02 '16 at 12:40
@NathanOliver if someone doesn't know about floating point errors, debugging wouldn't help a lot. One should comment though that it's good practice to minimize the scope of variables and *not* split declaration from initialization. If the declaration was `double repCost = 27414;` it would be obvious that `repCost` is a double and thus subject to inaccuracies — Panagiotis Kanavos, Jun 02 '16 at 12:43
Looks to me like, given a different set of initial input, that's not shown here, `optDura` may not ever get initialized. So, what did you expect to happen, then? — Sam Varshavchik, Jun 02 '16 at 12:43
What is the exact input (edit: I guess the exact input in the program, good), exact output, and expected output. — eerorika, Jun 02 '16 at 12:43
@PanagiotisKanavos Could you please help me solving this problem? — dknight, Jun 02 '16 at 12:49
@dknight help them understand so that they can help you fix your problem ! see what user2079303 said?? do it — Khalil Khalaf, Jun 02 '16 at 13:10

molbdnilo · Accepted Answer · 2016-06-02T13:16:57.587

If you rewrite to intialise your variables instead of using the "declare and assign" anti-"pattern" (I also removed unused variables):

int main(){  
    long int currDura = 58;
    long int maxDura = 59;
    long int tempMaxDura = maxDura;
    long int tempDura = currDura;
    long int totDura = tempDura;
    long int optDura; 
    double iniCost = 25500;
    double repCost = 27414;
    double se = 0.90;
    double sc = 0.85;
    double totCostTillNow = iniCost;
    double costPerBattle = totCostTillNow / totDura;
    double minCPB = costPerBattle;

    cout<<"\n Cost without any repair = "<<costPerBattle;

    for(int i=1; i<=maxDura; i++)
    {
        totCostTillNow += repCost * sc;
        tempDura = tempMaxDura * se;
        totDura += tempDura;
        costPerBattle = totCostTillNow / totDura;
        tempMaxDura -= 1;
        if ( minCPB >=  costPerBattle )
        {
            minCPB = costPerBattle;
            optDura = tempMaxDura;
        }
    }
    cout<<"\n Minimum cost per battle = "<<minCPB<<" & sell at 0/"<<optDura;
    return 0;
}

then optDura sticks out as the only one missing initialisation.
The only time it's assigned a value is if minCPB >= costPerBattle, so if that condition never holds you're left with an indeterminate value.

Initialise it to something sensible.

Why is this returning Garbage?

1 Answers1