tiamalist / myCreditCard

0 stars 0 forks source link

GetCreditCardVendor returns incorrect vendor #2

Closed kakarotto67 closed 8 years ago

kakarotto67 commented 8 years ago

Check tests below.

kakarotto67 commented 8 years ago
    [TestCase("343434343434343")]
    [TestCase("3434 3434 3434 343")]
    [TestCase("341134113411347")]
    [TestCase("3411 3411 3411 347")]
    [TestCase("378282246310005")]
    [TestCase("3782 8224 6310 005")]
    [TestCase("371449635398431")]
    [TestCase("3714 4963 5398 431")]
    public void GetCreditCardVendor_AmericanExpress(string number)
    {
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(AmericanExpress, vendor);
    }
kakarotto67 commented 8 years ago
    [TestCase("3530111333300000")]
    [TestCase("3530 1113 3330 0000")]
    [TestCase("3566002020360505")]
    [TestCase("3566 0020 2036 0505")]
    public void GetCreditCardVendor_JCB(string number)
    {
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(JCB, vendor);
    }
kakarotto67 commented 8 years ago
    [TestCase("5000000000000611")]
    [TestCase("5000 0000 0000 0611")]
    public void GetCreditCardVendor_Maestro(string number)
    {
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(Maestro, vendor);
    }
kakarotto67 commented 8 years ago
    [TestCase("5555555555554444")]
    [TestCase("5555 5555 5555 4444")]
    [TestCase("5105105105105100")]
    [TestCase("5105 1051 0510 5100")]
    public void GetCreditCardVendor_MasterCard(string number)
    {
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(MasterCard, vendor);
    }
kakarotto67 commented 8 years ago
    // extra character at the end
    [TestCase("35301113333000001")]
    [TestCase("41111111111111112")]
    [TestCase("3434343434343433")]
    [TestCase("50000000000006114")]

    // Diners
    [TestCase("36148900647913")]
    public void GetCreditCardVendor_Unknown(string number)
    {
        var isValid = CreditCardUtility.IsCreditCardNumberValid(number);
        //Assert.IsFalse(isValid);
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(Unknown, vendor);
    }
kakarotto67 commented 8 years ago
    [TestCase("4111111111111111")]
    [TestCase("4111 1111 1111 1111")]
    [TestCase("4012888888881881")]
    [TestCase("4012 8888 8888 1881")]
    [TestCase("4222222222222")]
    [TestCase("4222 2222 222 22")]
    [TestCase("4917610000000000003")]
    [TestCase("4911830000000")]
    public void GetCreditCardVendor_VISA(string number)
    {
        var vendor = CreditCardUtility.GetCreditCardVendor(number);
        StringAssert.AreEqualIgnoringCase(VISA, vendor);
    }
tiamalist commented 8 years ago

не зовсім зрозумів, необхідно перевірити вище вказані функції у моєму коді, чи замінити їх???

tiamalist commented 8 years ago

У всі наведених прикладах номери проходять перевірку вендора.

tiamalist commented 8 years ago

Якщо можна, поясни вчому суть вище вказаних тобою коментарів!!!

tiamalist commented 8 years ago

/ extra character at the end [TestCase("35301113333000001")] Результат: Unknown [TestCase("41111111111111112")] Результат: Unknown [TestCase("3434343434343433")] Результат: Unknown [TestCase("50000000000006114")] Результат: Maestro
/ Diners [TestCase("36148900647913")] Результат: Unknown

kakarotto67 commented 8 years ago

These are unit tests. You can use them to check your code. E.g., in the last your comment "Uknown" should be for all test cases, but you got "Maestro" for "50000000000006114".

tiamalist commented 8 years ago

50000000000006114 - це "Maestro" так як він починається на "50" а кількість символів входить в діапазон вендора "Maestro" від 12 до 19 символів. а вказаний приклад має 17 символів. Де я можу помилятись?

kakarotto67 commented 8 years ago

Are you sure? Then please just pass this number into your IsCreditCardNumberValid method and it will return false, so how can this number be valid? Also please check other unit tests.

tiamalist commented 8 years ago

У функції GetCreditCardVendor визначення вендора проходить успішно (Maestro) , а у функції IsCreditCardNumberValid повертається (False) cc4 Тема issues звучить "GetCreditCardVendor returns incorrect vendor", те що номер не валідний погоджуюсь, але визначення вендора правильне так як номер відповідає критеріям вендора.

kakarotto67 commented 8 years ago

Do you think that Maestro vendor will issue a card with invalid number in real world? Do you think that this is valid flow? Please, just stop proving this as this makes no sense. Despite this particular test you have 30+ tests that crash, so I can't mark you code as 'Passed'.

tiamalist commented 8 years ago

Я прекрасно розумію що в реальному світі, вендор не буде генерувати хибні номера. Але для того щоб якісно виконати поставлену задачу необхідно чітко сформувати "технічне завдання", а це не дуже схоже на якісно оформлене технічне завдання: https://gist.github.com/intrueder/530b2bee09debea47272c65b8b87b087 1-ша функція задача стояла повертати імя вендора 2-га функція перевірка валідності 3-я функція видати наступний валідний номер

tiamalist commented 8 years ago

Цитата із листа Тимур Муджири: Завдання самі по собі не пов'язані одне з одним, і можуть бути виконані незалежно.

Без звернення до функції IsCreditCardNumberValid, а із слів Тимура працюючи тільки у функції GetCreditCardVendor, як ти визначиш валідність даного номера 50000000000006114.?

tiamalist commented 8 years ago

І всі 30 вище вказаних номерів проходять визначення вендора та валідність і моєму коді.

kakarotto67 commented 8 years ago

We haven't specified all the cases to see how you will manage to find and cover them, not only to check your code. Real programmer should think about different business scenarious, not only about the code. In that particular case you've already agreed that this would be incorrect behaviour to issue such card in real world. And your code should resemble real world as much as possible, that's why all this style is called OOP. So, most of the people managed to cover that scenario without our help, but you didn't. Regarding other test cases, have you managed to run unit tests I've shared with you and checked the result?

tiamalist commented 8 years ago

Маєш на увазі ось ці уривки коду: public void GetCreditCardVendor_AmericanExpress(string number) { var vendor = CreditCardUtility.GetCreditCardVendor(number); StringAssert.AreEqualIgnoringCase(AmericanExpress, vendor); }

Модульне тестування ще не виконував

tiamalist commented 8 years ago

На прикладі мого коду, навіть якщо вендор (або ініціатор запуску програми) вкаже не валідний номер карти, враховуючи тільки код вендара номера та кількість символів, то функція GenerateNextCreditCardNumber поверне йому валідний номер, свого роду захист від людського фактору 1cc