6
\$\begingroup\$

The goal here is to, add a credit card image, and remove all other credit card images, of a certain input field. I had a even larger code, with a lot of addClass() and removeClass()... then, I shrink it until I have this. Still, I feel it's still a huge block of code.

The point here is to short things up, but NOT until it's hard to understand what's going on.

if (event.isValid) {

    if (event.card.type == 'visa') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-visa'));

    } else if (event.card.type == 'master-card') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-master-card'));

    } else if (event.card.type == 'e-american-express') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-american-express'));

    } else if (event.card.type == 'diners-club') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-diners-club'));

    } else if (event.card.type == 'discover') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-discover'));

    } else if (event.card.type == 'jcb') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-jcb'));

    } else if (event.card.type == 'unionpay') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-unionpay'));

    } else if (event.card.type == 'maestro') {

        $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-maestro'));
    }

} else {

    $("#card-number").removeClass(CREDIT_CARD_LIST);
}
\$\endgroup\$
  • \$\begingroup\$ Is } else if (event.card.type == 'e-american-express') { a typo in that it should be == 'american-express'. \$\endgroup\$ – Quill Sep 3 '15 at 9:44
  • \$\begingroup\$ I don't understand CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-visa') wouldn't it be the same as just adding 'e-visa' ? \$\endgroup\$ – the_lotus Sep 3 '15 at 12:58
5
\$\begingroup\$

Create a map from card.type to what class it should be:

var cardTypeToClass = {  'visa': 'e-visa',
                         'master-card': 'e-master-card',
                         //...
                       };

This creates an associative array so if you give it a key value then you can get out the associated value.

in your case that is done like:

if (event.isValid) {
    $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, cardTypeToClass[event.card.type]));
} else {
    $("#card-number").removeClass(CREDIT_CARD_LIST);
}

This concentrates the mapping into a single section and separates out what the code actually does.

\$\endgroup\$
  • \$\begingroup\$ I don't understand how "cardTypeToClass[event.card.type]" will know what kind it is that we have to show. Care to explain a little further? \$\endgroup\$ – user82648 Sep 3 '15 at 8:34
  • \$\begingroup\$ I would extract a variable: var cardType = cardTypeToClass[event.card.type]; $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, cardType)); \$\endgroup\$ – ANeves Sep 3 '15 at 9:40
4
\$\begingroup\$

It seems the value you're trying to replace for is really just the event.card.type prefixed by e-, so just string concatenate them together.

However, if some of the replace variables do not follow this pattern, it'd be best to use @ratchet freak's solution.

if (event.isValid) {
    $("#card-number").addClass(CREDIT_CARD_LIST.replace(CREDIT_CARD_LIST, 'e-' + event.card.type));
} else {
    $("#card-number").removeClass(CREDIT_CARD_LIST);
}
\$\endgroup\$
  • \$\begingroup\$ e-american-express doesn't follow the pattern \$\endgroup\$ – ratchet freak Sep 3 '15 at 9:43
  • \$\begingroup\$ That looks like a typo though. \$\endgroup\$ – Quill Sep 3 '15 at 9:44
  • 7
    \$\begingroup\$ If it doesn't follow the pattern, you should strongly consider making it follow the pattern rather than accommodating special cases. \$\endgroup\$ – 200_success Sep 3 '15 at 10:29

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.