0

I've read that a greedy algorithm only take cares for the optimal solution that is trying to reach at that moment, but is this the only criteria I should consider if I want to create a greedy algorithm? Aslo, how can I know if I have created a greedy algorithm or not? I mean, I created the following code for the problem of the change in C++:

#include <iostream>

using namespace std;

int greedyChange(int coinSet[], int lenCoinSet,  int money){
    int change = 0;
    static int i = 0;

    if(i >= lenCoinSet){
        return 0;
    }
    while(money - coinSet[i] >= 0){
        change++;
        money -= coinSet[i];
    }
    i++;
    return change + greedyChange(coinSet, lenCoinSet, money);
}

int main(int argc, char const *argv[]){

    int coinSet[]={20, 15, 10, 5, 1};
    int lenCoinSet = sizeof(coinSet)/sizeof(coinSet[0]);
    int money = 30;

    cout << "The minimun number of coins to get your change is: " 
            << greedyChange(coinSet, lenCoinSet, money)<<endl;
    return 0;
}

And I think it is greedy, but I'm not sure. I would apreciate if you could explain me if the code I wrote is greedy or not. Besides, if it isn't, do you have another possible solution you could share or maybe some advices to improve this code? Finally, if there is documentation you could recommend me, I'll be very thankful.

Anthony Luzquiños
  • 739
  • 11
  • 23

2 Answers2

1

Yes, it's greedy.

There are 2 problems, though.

Firstly, instead of loop, you should have used a simple division:

while(money - coinSet[i] >= 0){
    change++;
    money -= coinSet[i];
}

can be easily replaced with:

coins = money / coinSet[i]
change += coins
money -= coins * coinSet[i]

Secondly, your program uses recursion with a static variable -- this is usually frowned upon. You should replace this with a simple loop over i instead of calling recursively.

lenik
  • 23,228
  • 4
  • 34
  • 43
1

Yes, this is greedy – you take as much as possible of the first denomination, then as much as possible of the next, and so on.

There are a couple of problems, though.

The most important problem is that your function will only work once, due to the local static variable. (Mutable state and recursion is not a pleasant combination.)

You could solve this by also passing the current index as an argument, but I would reverse the array and go backwards instead.

The second problem is a small inefficiency with this loop:

    while(money - coinSet[index] >= 0){
        change++;
        money -= coinSet[index];
    }

which you can replace with division and multiplication.

int greedyChange(int coinSet[], int index,  int money){
    if(index < 0 || money <= 0){
        return 0;
    }
    int coins = money / coinSet[index];
    return coins + greedyChange(coinSet, index - 1, money - coins * coinSet[index]);
}


int main(int argc, char const *argv[]){

    int coinSet[]={1, 5, 10, 15, 20};
    int lenCoinSet = sizeof(coinSet)/sizeof(coinSet[0]);
    int money = 30;

    cout << "The minimun number of coins to get your change is: " 
            << greedyChange(coinSet, lenCoinSet - 1, money)<<endl;
    return 0;
}
molbdnilo
  • 64,751
  • 3
  • 43
  • 82