Risk battle simulator v1

I've written a program to find the odds of either side winning a battle in Risk, and how many men they would have left. I've finished, but I feel that it can be improved since it doesn't function if the battle gets too big (with "too big" being ridiculously small, to the point of being near-useless). Can anybody find any ways to improve it?

```
#include
#include
#include
#include

using namespace std;

struct Fraction { //Fraction class used for adding/multiplying the odds of rolls together
unsigned long long top;
unsigned long long bottom;
};

struct FractionMathException : public exception { // Error code (to keep the system from creating bad numbers)
const char * what () const throw ()
{
return "C++ Exception";
}
};

void sorter(int attackers,int defenders,Fraction odds); //Declaring functions to use later
void a1d1(int attackers,int defenders,Fraction odds);
void a1d2(int attackers,int defenders,Fraction odds);
void a2d1(int attackers,int defenders,Fraction odds);
void a2d2(int attackers,int defenders,Fraction odds);
void a3d1(int attackers,int defenders,Fraction odds);
void a3d2(int attackers,int defenders,Fraction odds);
unsigned long long LCM(unsigned long long a, unsigned long long b);
Fraction Add(Fraction a, Fraction b);
Fraction Multiply(Fraction a, Fraction b);
Fraction Frac(int a,int b);

vector attack_wins,defend_wins; //Keeps track of how many times the attackers and defenders win. Which slot is determined by how many men are left standing.

int main(){
int attackers,defenders;

cout > attackers;
cout > defenders;

attack_wins = vector(attackers,Frac(0,1)); // Set attack_wins and defend_wins to the correct sizes, and set every slot to 0/1 (so 0)
defend_wins = vector(defenders,Frac(0,1));

try{
sorter(attackers,defenders,Frac(1,1)); // Run the main part of the program using the number of attackers, defenders, and a 100% chance of this happening.

cout =0; loop--){

using namespace std;

Don't do that. You're not really saving much typing, and Risk easily avoidable name collisions. If there's one thing I know of c++, it's that using namespace std is a bad habit to lose.

void a1d1(int attackers,int defenders,Fraction odds);
void a1d2(int attackers,int defenders,Fraction odds);
void a2d1(int attackers,int defenders,Fraction odds);
void a2d2(int attackers,int defenders,Fraction odds);
void a3d1(int attackers,int defenders,Fraction odds);
void a3d2(int attackers,int defenders,Fraction odds);

It's not very clear what these methods mean at first glance. ...at second glance either. You should use meaningful identifiers - always. One shouldn't need to read the comments you put up on the implementations to know what they stand for.

That said, I think it might have been better (and a bit harder, but better) to do something more like this:

void calculate(int attackerArmies, int attackerDice, int defenderArmies, int defenderDice, Fraction odds);

And then, perhaps regroup armies and dice together into an appropriate data structure - note, I don't do c++, so take this recommendation with a grain of salt - the point I'm making is that both the attacker and the defender have armies and dice, and this looks like a great [missed] opportunity for an abstraction; this could turn the signature into something like this:

void calculate(std::tuple attacker, std::tuple defender, Fraction odds);

Most likely there's a more appropriate data type for this, but you get the idea. Perhaps something like this:

class Player
{
    int armies;
    int dice;

public:
    Player(int armies, int dice):
        armies(armies),
        dice(dice)
    {}
};

Which turns the signature into this:

void calculate(Player attacker, Player defender, Fraction odds);

Now, you're calling this a function, but it's returning void, and there's no indication that you're altering the parameters (this might be a language thing though, ...but it doesn't feel right anyway). How about another abstraction?

class BattleSettings
{
    Player attacker;
    Player defender;

public:
    BattleSettings(Player attacker, Player defender):
        attacker(attacker),
        defender(defender)
    {}
};

Now you could have this:

BattleSettings calculate(BattleSettings players, Fraction odds);

..and the implementation wouldn't sneakily modify the parameter values, but return a new BattleSettings value instead, containing the state (armies and dice) of each player.

As for the odds themselves, you've essentially hard-coded the whole thing - and these magic numbers stick out like a sore thumb. Best would be to actually calculate them.

StackExchange Code Review Q#107005, answer score: 12