Problem with Checking in methods if ISBN is valid and currency is valid in C#

Question

So, I have the problem with implementation method for validating currency and validating isbn number. I give below my code with summary description. I believe that somebody can help me and explain which I have done bad. Validation for currency ISO

        /// <summary>
        /// Determines whether a specified string is a valid ISO currency symbol.
        /// </summary>
        /// <param name="currency">Currency string to check.</param>
        /// <returns>
        /// <see langword="true"/> if <paramref name="currency"/> is a valid ISO currency 
         symbol; <see langword="false"/> otherwise.
        /// </returns>
        /// <exception cref="ArgumentNullException">Thrown if currency is null.</exception>
        public static bool IsValid(string? currency)
        {
            if (currency == null)
            {
                throw new ArgumentNullException(nameof(currency));
            }

            Regex regex = new Regex(@"^\d*\.?\d?\d?$");
            bool y = regex.IsMatch(currency);
            return y;
        }

Validation for ISBN number

        /// <summary>
        /// Verifies if the string representation of number is a valid ISBN-10 or ISBN-13 
         identification number of book.
        /// </summary>
        /// <param name="isbn">The string representation of book's isbn.</param>
        /// <returns>true if number is a valid ISBN-10 or ISBN-13 identification number of 
        book, false otherwise.</returns>
        /// <exception cref="ArgumentNullException">Thrown if isbn is null.</exception>
        public static bool IsValid(string? isbn)
        {
            if (isbn == null)
            {
                throw new ArgumentNullException(nameof(isbn));
            }

            int n = isbn.Length;

            if (n != 10)
            {
                return false;
            }

            int sum = 0;
            for (int i = 0; i < 9; i++)
            {
                int digit = isbn[i] - '0';

                if (digit < 0 || digit > 9)
                {
                    return false;
                }

                sum += digit * (10 - i);
            }

            char last = isbn[9];

            if (last != 'X' && (last < '0'
                             || last > '9'))
            {
                return false;
            }

            sum += (last == 'X') ? 10 :
                              (last - '0');

            return sum % 11 == 0;
        }

I would be grateful for some help. I cannot decide it will be better resolve by using regex or different ways are more complex. My solutions are not good enough, so I believe in some help or explanation.

Shouldn't your `IsValid` function return `false` instead of throw if `currency` is null? Especially as you've explicitly declared that `currency` _can_ be `null` (because you have `String?` not `String`). — Dai, Apr 15 '22 at 00:39
See the second example [here](https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex?view=net-6.0#examples). You should be using the culture of the current thread to validate whether the currency value is correct. In general, it is best to have one overload that uses the culture of the current thread and one that allows you to pass in an `IFormatProvider` instance so you can specify the culture explicitly. — NightOwl888, Apr 15 '22 at 03:48

Jerome Petrov · Answer 1 · 2022-04-15T01:21:09.197

First of all your methods both have the same signature and thus will look the same to the compiler, even if they differ by the name of their parameter if they are implemented in the same class/struct/record. I would recommend giving them more specific names like ValidCurrencyString and ValidISBN.

You also might want to consider not using nullable parameters (as you apparently are in #nullable context) unless strictly necessary.

I understand what you are trying to do with your method for validating currency strings but it probably would be best to look into answers for the exact same problem as it is rather unnecessary to write that regex again.

public static bool ValidCurrencyString(string? str)
{
    // If it is null, then it is not a valid string.
    if (str is null) return false;

    // Replace pattern with whichever regex suits you.
    return Regex.IsMatch(str, pattern);
}

or (most likely) better provided #nullable is enabled:

public static bool ValidCurrencyString(string str) => Regex.IsMatch(str, pattern);

Verifying an ISBN is not as easy but you can (and probably should) also use a regular expression others have already written. If you decide to do it that way this method would also basically look like ValidCurrencyString with another regular expression.

Problem with Checking in methods if ISBN is valid and currency is valid in C#

1 Answers1