0

I am trying to build a method that will take a variety of numeric types and preprocess them for a second method. I am not sure if I should be simply overloading or using a generic method. I tried to use a generic method but the method does not seem to recognize the parameter type. The code is below. Can someone please explain to me whether it is better to overload or use a generic method in this case? Also, if I wanted to use a generic method to do this, how could I make it work? Thank you very much.

 public static class math
 {
      public static int nextpow2<T>(T a)
      {
           double w;
           if ( a.GetType() is sbyte   ||
                a.GetType() is byte    ||
                a.GetType() is short   ||
                a.GetType() is ushort  ||
                a.GetType() is int     ||
                a.GetType() is uint    ||
                a.GetType() is long    ||
                a.GetType() is ulong   ||
                a.GetType() is float   ||
                a.GetType() is double  ||
                a.GetType() is decimal
           ) w = (double)Convert.ChangeType(a, typeof(double));
           else
                throw new System.ArgumentException("Internal error in nextpow2: argument a is not a number!");

           return _nextpow2(w);
      }

      private static int _nextpow2(double a)
      {
           double index = Math.Abs(a);
           int p = (index > 1) ? (int)Math.Ceiling( Math.Log( index, 2.0) ) : 0;

           return p;
      }

I am calling the method as follows:

 int indx = 0;
 int p = math.nextpow2(indx);

The code fails to compile. I get the following error:

Internal error in nextpow2: argument a is not a number!

Can someone please explain what I am doing wrong? Thank you.

6
  • 1
    Why not just use the non-generic signature nextpow2(double a)? Many of the types you want to support are already implicitly convertible to double. Commented Mar 19, 2013 at 19:09
  • 1
    I'd get very upset with someone changing my decimal to a double as well. Commented Mar 19, 2013 at 19:13
  • I am trying to make a general method. It is possible someone may pass in a parameter that does not have an implicit conversion to double and I want to catch that case. Commented Mar 19, 2013 at 19:15
  • Very good point on the conversion of decimal to double. Thank you. Commented Mar 19, 2013 at 19:16
  • Even if you're writing both the method and its calling code, it's good practice to imagine the method being used by a caller unaware of its internal implementation. In this case, the method is difficult to use because it imposes unknown rules on its input. If you want the method to operate on double, make the parameter type double and let the caller worry about ensuring the input is suitable. Commented Mar 19, 2013 at 19:18

2 Answers 2

3

Can someone please explain what I am doing wrong?

Sure. You're checking whether a Type object is an sbyte, or a byte, etc. You're not checking whether the type represents the type of sbyte etc... you're asking whether the value is an sbyte. That's never, ever the case. (Which is why you're getting a compile-time error.)

You could use:

if (a.GetType() == typeof(byte) ||
    // etc)

But I probably wouldn't. I wouldn't make this a generic method at all. If you really want the functionality, I'd write:

private static readonly HashSet<Type> ValidTypes = new HashSet<Type>
{
    typeof(sbyte), typeof(byte), /* etc */
};

public static double ConvertToDouble(object x)
{
    if (x == null)
    {
        throw new ArgumentNullException("x");
    }
    if (!ValidTypes.Contains(x.GetType())
    {
        throw new ArgumentException("...");
    }
    return Convert.ChangeType(x, typeof(double));
}

Note how this is doing one thing: converting the argument to double. There's no point in making it then call _nextpow2 - it's easy enough for the caller to do that if they want to.

Additionally, you've written two contradictory things:

The code fails to compile. I get the following error:
Internal error in nextpow2: argument a is not a number!

If it fails to compile (which is what I'd expect, btw) then you can't be running the code, which means you can't be getting the exception. You need to differentiate between compile-time errors like this:

warning CS0184: The given expression is never of the provided ('sbyte') type

and execution-time exceptions.

Also, both your class and method names violate normal .NET naming conventions.

Sign up to request clarification or add additional context in comments.

10 Comments

Duh. That makes complete sense. I don't know how I missed that one. Might I ask why you wouldn't?
Also recommended, if you want to go down the route of "generic math": "Miscellaneous Utility Library", by Skeet (and Marc, right Jon?) : yoda.arachsys.com/csharp/miscutil
@PBrenek: You're really not writing a generic method in the normal sense. It's not like it can really deal with any type, and it's not like you're handling that value as a value of that type anyway - all you're doing is calling Convert.ChangeType. What benefit do you think there is to making it generic?
@JonSkeet: You are missing a return type! O_o ;p
@Jon Skeet This is very nice information. Thank you. It is overkill for this problem but it is certainly something I did not know and need to learn. Also, thank you for pointing out the incorrect naming conventions. I will keep this information in mid for the future. Thank you very much.
|
2

You can either say

a is sbyte

or say

a.GetType() == typeof(sbyte)

but it makes no sense to mix it into a.GetType() is sbyte. You should get a compiler warning about that! For a.GetType() will be an object which derives from System.Type (actually it will be System.RuntimeType), and therefore it can never derive from sbyte (since sbyte is sealed, being a struct, and sbyte itself derives only from ValueType and Object).

A better solution than a generic method, is to have:

public static int Nextpow2(double a)
{
    ...
}

All numereric types except decimal are implicitly convertible to double. So with the above signature, you can use it the way you want:

int indx = 0;
int p = math.Nextpow2(indx);

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.