CODEWITHSUNDEEP
c#dry
Mike J
Mike J

Reputation: 425

C# best practice with using multiple similar methods with different properties?

I'm trying to simplify and reduce my code.

I have a method like this:

private bool FindMatchingValue(string value, string response, string dataType) {
  bool isFound = false;
  switch (dataType) {
    case "Date":
      var parsedDate = DateTimeOffset.Parse(response).ToString("yyyy-MM-dd");
      isFound = checkEquality(value, parsedDate);
      break;
    case "Time":
      var parsedTime = DateTimeOffset.Parse(response).ToString("HH:mm:00.000");
      isFound = checkEquality(value, parsedTime);
      break;
  }
  return isFound;
}

I have another method which takes a list of strings like this:

private bool FindMatchingValue(List<string> values, string response, string dataType) {
  bool isFound = false;
  switch (dataType) {
    case "Date":
      var parsedDate = DateTimeOffset.Parse(response).ToString("yyyy-MM-dd");
      isFound = checkEquality(values, parsedDate);
      break;
    case "Time":
      var parsedTime = DateTimeOffset.Parse(response).ToString("HH:mm:00.000");
      isFound = checkEquality(values, parsedTime);
      break;
  }
  return isFound;
}

The only real difference with these methods is that one property. Is there a better approach I can use, so I don't have to repeat the repeating switch case code?

Upvotes: 0

Views: 217

Answers (3)

phuzi
phuzi

Reputation: 13060

You already have a version that handles multiple values. Handling a single value is a special case of that so

private bool FindMatchingValue(string value, string response, string dataType) {
  bool isFound = false;
  switch (dataType) {
    case "Date":
      var parsedDate = DateTimeOffset.Parse(response).ToString("yyyy-MM-dd");
      isFound = checkEquality(value, parsedDate);
      break;
    case "Time":
      var parsedTime = DateTimeOffset.Parse(response).ToString("HH:mm:00.000");
      isFound = checkEquality(value, parsedTime);
      break;
  }
  return isFound;
}

Could just be

private bool FindMatchingValue(string value, string response, string dataType) {
    return FindMatchingValue(new List{ value }, response, dataType);
}

This way you don't need to duplicate the code, just provide an alternate method signature that can be called depending upon what is easier at the time. It's fairly common (in my experience) to see overloads like this all refer to a single method that has the necessary business logic.

Equally the List version could be replaced with a call to the original version of the single value

private bool FindMatchingValue(List<string> values, string response, string dataType) {
    return values.Any(value => FindMatchingValue, response, dataType);
}

But the choice is yours.

Upvotes: 7

Josh Heaps
Josh Heaps

Reputation: 296

For the first method that only has the one string, you could do this:

private bool FindMatchingValue(string value, string response, string dataType) => return FindMatchingValue(new List<String>{ value }, response, dataType)

It's a single line function that just calls the other

Upvotes: 1

Stig
Stig

Reputation: 2086

Use the list-version and flip the arguments around, and make it a params. It will now works for both.

private bool FindMatchingValue(string response, string dataType, params string[] values) {
  bool isFound = false;
  switch (dataType) {
    case "Date":
      var parsedDate = DateTimeOffset.Parse(response).ToString("yyyy-MM-dd");
      isFound = checkEquality(values, parsedDate);
      break;
    case "Time":
      var parsedTime = DateTimeOffset.Parse(response).ToString("HH:mm:00.000");
      isFound = checkEquality(values, parsedTime);
      break;
  }
  return isFound;
}

Upvotes: 1

Related Questions