4
\$\begingroup\$

I have a pretty unusual situation (please, don't ask why) where I need to compare objects which might be a string or double to string which might be a string or a double.ToString(). I've come up with something which I really don't like, so please feel free to criticize it and suggest a better solution.

public bool AreEqual(string stringValue, object objectValue)
{
    double numericValue;
    return (stringValue == (objectValue ?? string.Empty).ToString()) || // trying to compare values as strings
           (double.TryParse(stringValue, out numericValue) && (numericValue == objectValue as double?)); // otherwise - as double
}

To make it clear what I expect to get from that function:

[TestCase("a", "a", true)]
[TestCase("a", "b", false)]
[TestCase("10", 10, true)]
[TestCase("11", 10, false)]
[TestCase("11", "11.0", true)]
[TestCase("11", "11.5", false)]
public void ComparisionTest(string stringValue, object objectValue, bool areEqual)
{
     bool result = AreEqual(stringValue, objectValue);
     Assert.That(result == areEqual)
}
\$\endgroup\$
1
  • \$\begingroup\$ Some additional test cases: "1", "1." and "a.0", "a". You should also test with negative numbers, positive numbers with a + sign, scientific notation like 1E+9, thousands separator, NaN, infinities, signed zero, numbers really close to zero, numbers large enough for double to lose accuracy but not larger than its maximal finite value, numbers larger than the maximum value double can represent and probably a few more I forgot. \$\endgroup\$ – CodesInChaos Aug 28 '14 at 20:10
5
\$\begingroup\$

Your clarification made it a lot clearer what you want. I think you're trying too much to make it all fit on two lines and you can make it must more readable by splitting it out a little.

Following straightforward logic (really just translating your description) I came to this which works for all 4 tests (note that argument 10 is an integer, not a double).

public bool AreEqual(string stringValue, object objectValue)
{
  if (objectValue is string)
  {
      double parsedValue;
      if (double.TryParse(objectValue.ToString(), NumberStyles.Any, CultureInfo.InvariantCulture, out parsedValue))
      {
          return Math.Abs(double.Parse(stringValue, CultureInfo.InvariantCulture) - parsedValue) < 0.00000001;
      }

      return stringValue == objectValue as string;
  }

  if (objectValue is double)
  {
      return double.Parse(stringValue, CultureInfo.InvariantCulture) == objectValue as double?;
  }

  throw new ArgumentException("The object must either be a string or a double");
}

Note that you can also use a direct cast instead of as since it is guaranteed to be a possible cast, but I like to read as myself (the performance impact is minimal and not relevant in our scenario).

Note that I also use double.Parse instead of double.TryParse because from your description I can assume it is either a double or a string. If it isn't, I'm throwing an exception anyway at the end so I don't feel compelled to use TryParse.


I have made changes that take care of the added test cases which you specified later. Alongside that it also takes care of the culture-specific notation (dot vs comma) and floating point errors.

It's more lengthy than your solution but it is also more robust code and a lot easier to digest.

This takes care of the following testcases:

AreEqual("a", "a") // true
AreEqual("a", "b") // false
AreEqual("10", 10.0) // true
AreEqual("11", 10.0) // false
AreEqual("11", "11.0") // true
AreEqual("11", "11.5") // false
AreEqual("11.0", "11") // true
\$\endgroup\$
2
  • \$\begingroup\$ Just realized that I missed one testcase. Question updated. \$\endgroup\$ – anatoliiG Aug 28 '14 at 12:05
  • \$\begingroup\$ Now it's working, but not sure whether it looks better than my code. ;) \$\endgroup\$ – anatoliiG Aug 29 '14 at 7:44

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.