HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Validating integer or string input

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inputstringintegervalidating

Problem

I'm at uni and working on an assignment in c# that takes some user input (from they keyboard via console).

The input validation required:

  • numbers must be within range (range will vary depending on which menu they're in)



  • strings must not be empty/null



  • string must be no longer than 30 characters in length



So I've chosen to write just one method (will be a static method of a 'validator' class) that can handle validation for both and am unsure if this is a good idea - having a method that essentially does more than one thing.

The method:

public static bool isValid(string input, string type, int min = 0, int max = 0)
  {
     if (String.IsNullOrEmpty(input))
        return false;
     else
     {
        switch (type.ToLower())
        {
           case "integer":
              int i;
              if (Int32.TryParse(input, out i))
                 return ((i >= min) && (i <= max));
              else
                 return false;
           case "string":
              return input.Length < charLimit; // charLimit defined as 30 above
           default:
              return false;
        }
     }
  }


And a sample call:

if(isValid(userInput, "integer", 0, 5)
{
   // do something awesome
}


I've run some tests to check the edge cases and passing in empty strings etc and it passed them, but for some reason this feels kind of wrong.

Should I separate string and integer validation into separate methods?

Solution

You are clearly violating the Single Responsibility Principle (SRP) because the method is doing two things. You should use two methods where each validates one type only.

You will see also that for this task you gain the advantage that the methods will be smaller and easier to read and understand.

A small side note: In .NET methods are named using PascalCase, so isValid should be IsValid.

Don't omit braces {} although they might be optional. By always using them your code will become less error prone.

Another quick remark about the method in question. You have defined optional parameters min = 0 and max = 0. I would expect if parameters are optional, that they could return true for a bool method. Using this method without specifying min and max will result in false which is unexpected.

Integrating the said points into extension methods would look like so

public static bool IsInRange(this int value, int min, int max)
{
    return min <= value && value <= max;
}  

public static bool HasValidLength(this string value, int maximumLength)
{
    if (value == null) { throw new ArgumentNullException("value"); } 

    return value.Length < maximumLength;
}


which could then be called through this methods

public static bool IsValidInteger(this string value, int min = int.MinValue, int max = int.MaxValue)
{
   int v = 0;
   return int.TryParse(value, out v) && v.IsInRange(min, max);
}  

public static bool IsValidString(this string value, int maximumLength = 30)
{
    return value != null && value.HasValidLength(maximumLength);
}


like e.g

if(userInput.IsValidInteger(0, 5))
{
   // do something awesome
}

Code Snippets

public static bool IsInRange(this int value, int min, int max)
{
    return min <= value && value <= max;
}  

public static bool HasValidLength(this string value, int maximumLength)
{
    if (value == null) { throw new ArgumentNullException("value"); } 

    return value.Length < maximumLength;
}
public static bool IsValidInteger(this string value, int min = int.MinValue, int max = int.MaxValue)
{
   int v = 0;
   return int.TryParse(value, out v) && v.IsInRange(min, max);
}  

public static bool IsValidString(this string value, int maximumLength = 30)
{
    return value != null && value.HasValidLength(maximumLength);
}
if(userInput.IsValidInteger(0, 5))
{
   // do something awesome
}

Context

StackExchange Code Review Q#132258, answer score: 11

Revisions (0)

No revisions yet.