views:

109

answers:

3

I have set up a class to validate credit card numbers. The credit card type and number are selected on a form in a separate class. I'm trying to figure out how to get the credit card type and number that are selected in the other class (frmPayment) in to my credit card class algorithm:

public enum CardType
{
    MasterCard, Visa, AmericanExpress
}

public sealed class CardValidator
{
    public static string SelectedCardType { get; private set; }
    public static string CardNumber { get; private set; }

    private CardValidator(string selectedCardType, string cardNumber) 
    {
        SelectedCardType = selectedCardType;
        CardNumber = cardNumber;
    }

    public static bool Validate(CardType cardType, string cardNumber)
{
   byte[] number = new byte[16];


  int length = 0;
  for (int i = 0; i < cardNumber.Length; i++)
  {
      if (char.IsDigit(cardNumber, i))
      {
          if (length == 16) return false;
          number[length++] = byte.Parse(cardNumber[i]); //not working.  find different way to parse
      }
  }

  switch(cardType)
  {
     case CardType.MasterCard:
        if(length != 16)
           return false;
        if(number[0] != 5 || number[1] == 0 || number[1] > 5)
           return false;
        break;

     case CardType.Visa:
        if(length != 16 & length != 13)
           return false;
        if(number[0] != 4)
           return false;
        break;

     case CardType.AmericanExpress:
        if(length != 15)
           return false;
        if(number[0] != 3 || (number[1] != 4 & number[1] != 7))
           return false;
        break;

  }

  // Use Luhn Algorithm to validate
  int sum = 0;
  for(int i = length - 1; i >= 0; i--)
  {
     if(i % 2 == length % 2)
     {
        int n = number[i] * 2;
        sum += (n / 10) + (n % 10);
     }
     else
        sum += number[i];
  }
  return (sum % 10 == 0);

} }

+1  A: 

In frmPayments btnValidate_click (or similar) event handler, just call CardValidator.Validate method.

However, the CardValidator constructor has parameters even though it's private, and never called? And why have you added properties to the class?

Edit: missed some bits of your sample code.

ho1
+1 on the second paragraph. You probably don't want the ctor taking args, then possibly ignoring them in the method. If it were me I'd make the only ctor private, drop the two properties, and keep the entire class static. Then when I wanted to validate call CardValidator.Validate(type, number);
fatcat1111
it's not quite done yet. i was just getting to where i wanted to start trying to call it from my frmPayment class, but couldn't quite figure out a way to do it. that's why my constructor isn't quite there yet. i just haven't done one quite like this before
EvanRyan
+2  A: 

For starters, I'd use Regex to do the simple validation (all digits, specific length.)

But to the point of your question, I'm not sure I understand what the issue is. From what you've posted here, it looks like you should probably drop the constructor and make the whole thing a static class.

public enum CardType
{
    MasterCard,
    Visa,
    AmericanExpress,
}

public static class CardValidator
{
    public static bool Validate(CardType cardType, string cardNumber)
    {
        string strippedCardNumber = Regex.Replace(cardNumber, @"\D", String.Empty);


        ICardValidator validator = SelectCardValidator(cardType);

        return validator.Validate(strippedCardNumber);
    }

    private static ICardValidator SelectCardValidator(CardType cardType)
    {
        switch (cardType)
        {
            case CardType.MasterCard:
                return new MasterCardValidator();
            case CardType.Visa:
                return new VisaValidator();
            case CardType.AmericanExpress:
                return new AmericanExpressValidator();
            default:
                return new UnknownCardTypeValidator();
        }
    }

    private interface ICardValidator
    {
        bool Validate(string cardNumber);
    }

    private class UnknownCardTypeValidator : ICardValidator
    {
        #region ICardValidator Members

        public bool Validate(string cardNumber)
        {
            return false;
        }

        #endregion
    }

    private abstract class LuhnAlgorithmValidator : ICardValidator
    {

        #region ICardValidator Members

        public virtual bool Validate(string cardNumber)
        {
            // Implement Luhn Algorithm here

            return false;
        }

        #endregion
    }

    private class MasterCardValidator : LuhnAlgorithmValidator
    {
        public override bool Validate(string cardNumber)
        {
            bool isValid = false; // replace with MasterCard validation
            return isValid && base.Validate(cardNumber);
        }
    }

    private class VisaValidator : LuhnAlgorithmValidator
    {
        public override bool Validate(string cardNumber)
        {
            bool isValid = false; // replace with Visa validation
            return isValid && base.Validate(cardNumber);
        }
    }

    private class AmericanExpressValidator : LuhnAlgorithmValidator
    {
        public override bool Validate(string cardNumber)
        {
            bool isValid = false; // replace with AmEx validation
            return isValid && base.Validate(cardNumber);
        }
    }
}
Toby
yeah, that seems right. i'm still a very beginner programmer, so i'm still making pretty basic mistakes quite often
EvanRyan
+3  A: 

You're missing a great opportunity for better OOP and cleaner code.

class CreditCard
{
   public CreditCard(string number, string expiration, string cvv2) {...}

   public virtual bool IsValid()
   {
       /* put common validation logic here */
   }

   /* factory for actual cards */
   public static CreditCard GetCardByType (CardType card, string number, string expiration, string cvv2)
   {
        switch (card)
        {
             case  CardType.Visa:
                  return new VisaCreditCard(...);

             ...
        }
   }
}

class VisaCreditCard : CreditCard
{
   public VisaCreditCard (string number, string expiration, string cvv2 )
      : base (number, expiration, cvv2)
   {...}

   public override bool IsValid()
   {
       /* check Visa rules... */
       bool isValid = ...

       return isValid & base.IsValid();
   }
}
Austin Salonen
This is a really clean solution.
Tyler