2
\$\begingroup\$

I've got a NumericUpDown control for each denomination of money and all of the events run the exact same code save for a single line so I'm trying to condense them into 1 event. I was originally going to use a switch but you can't switch on a non-value object so I just did an if tree. Is there a better way to do this?

private void BankInChanged(object sender, EventArgs e)
    {
        var objSender = (NumericUpDown) sender;

        if(objSender == bkI_Ls_Quarters)
        {
            currBoat.inBank.setQuarters(Convert.ToInt32(bkI_Ls_Quarters.Value));
        }
        else if(objSender == bkI_Rl_Quarters)
        {
            currBoat.inBank.setQuarterRolls(Convert.ToInt32(bkI_Rl_Quarters.Value));
        }
        else if(objSender == bkI_Ls_Nickels)
        {
            currBoat.inBank.setNickels(Convert.ToInt32(bkI_Ls_Nickels.Value));
        }
        else if(objSender == bkI_Rl_Nickels)
        {
            currBoat.inBank.setNickelRolls(Convert.ToInt32(bkI_Rl_Nickels.Value));
        }
        else if(objSender == bkI_Ls_Dimes)
        {
            currBoat.inBank.setDimes(Convert.ToInt32(bkI_Ls_Dimes.Value));
        }
        else if(objSender == bkI_Rl_Dimes)
        {
            currBoat.inBank.setDimeRolls(Convert.ToInt32(bkI_Rl_Dimes.Value));
        }
        else if(objSender == bkI_Ls_Pennies)
        {
            currBoat.inBank.setPennies(Convert.ToInt32(bkI_Ls_Pennies.Value));
        }
        else if(objSender == bkI_Rl_Pennies)
        {
            currBoat.inBank.setPennyRolls(Convert.ToInt32(bkI_Rl_Pennies.Value));
        }
        else if(objSender == bkI_Ones)
        {
            currBoat.inBank.setOnes(Convert.ToInt32(bkI_Ones.Value));
        }
        else if(objSender == bkI_Fives)
        {
            currBoat.inBank.setFives(Convert.ToInt32(bkI_Fives.Value));
        }
        else if(objSender == bkI_Tens)
        {
            currBoat.inBank.setTens(Convert.ToInt32(bkI_Tens.Value));
        }
        else if(objSender == bkI_Twenties)
        {
            currBoat.inBank.setTwenties(Convert.ToInt32(bkI_Twenties.Value));
        }
        else if(objSender == bkI_Fifties)
        {
            currBoat.inBank.setFifties(Convert.ToInt32(bkI_Fifties.Value));
        }
        else if(objSender == bkI_Hundreds)
        {
            currBoat.inBank.setHundreds(Convert.ToInt32(bkI_Hundreds.Value));
        }
        currBoat.updateTotals();
        updateStatDisplay();
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ Upon loading and just briefly seeing your code, NO. \$\endgroup\$ – Jeff Mercado Aug 23 '12 at 1:47
5
\$\begingroup\$

The problem is not in switching between senders but the fact that each branch calls a different method on inBank. I would suggest creating an enum for the denominations:

enum Denomination {
  Quarters,
  QuarterRolls,
  Nickels,
  NickelRolls,
  ...
}

and having a single setter in inBank:

void set(Denomination denomination, int value);

Then you can create a dictionary of controls to denominations:

IDictionary<NumericUpDown, Denomination> dict = new Dictionary<NumericUpDown, Denomination>();
dict[bkI_Ls_Quarters] = Denomination.Quarters;
dict[bkI_Rl_Quarters] = Denomination.QuarterRolls;
...

and your event handler would simply be:

private void BankInChanged(object sender, EventArgs e)
{
    var objSender = (NumericUpDown) sender;
    Denomination denomination = dict[objSender];

    currBoat.inBank.set(denomination, Convert.ToInt32(objSender.Value));
    currBoat.updateTotals();
    updateStatDisplay();
}
\$\endgroup\$
  • \$\begingroup\$ That's awesome, thanks! I'm still pretty new and I've never used a Dictionary before. Will definitely be making use of this. \$\endgroup\$ – JDiPierro Aug 23 '12 at 12:37
  • \$\begingroup\$ I think using collection initializer for your dictionary would make the code even better. Something like new Dictionary<NumericUpDown, Denomination> { { bkI_Ls_Quarters, Denomination.Quarters }, … }; \$\endgroup\$ – svick Aug 23 '12 at 14:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.