tags:

views:

462

answers:

4

I'm attempting to create a function where I can pass names and values that will be put into a ContentValues. The problem i'm having is that I'm trying to allow passing more that one key/value pair at a time, so keys can be an array of strings, but values have to be objects and ContentValues.put() doesn't allow Objects, they need to be cast to String, Double, Float, etc. Is there a way I can determine the type of the object and cast it so it will call the correct put()? Below is one method that does work, but it requires me to add an if for every possible type of value.

 public long create(String[] names, Object[] values) {
        ContentValues initialValues = new ContentValues();

        for (int i = 0; i < names.length; i++)
        {
         String type = values[i].getClass().getName();
         if (type.equals("java.lang.Double"))
         {
          initialValues.put(names[i], (Double)values[i]);
         }
         else if (type.equals("java.lang.String")) {
          initialValues.put(names[i], (String)values[i]);
   }
         else
         {
          throw new InvalidParameterException("Unable to convert type:"+type);
         }
        }

        return mDb.insert(this.getTableName(), null, initialValues);
    }
+2  A: 

You have two options:

  1. Create a list of if-else blocks like you're doing. If you do that I wouldn't do it that way though. I would do "ob instanceof String" instead; or
  2. Use reflection to find the correct Method for the type passed in.

One thing to consider is null values. If a passed in value is null there is no class so you can't determine the type, if thats relevant to your usage.

cletus
why using instanceof? primitive's wrappers are always final.. there is no need to check subclasses
dfa
In this case, instanceof should be faster than a String.equals() call. In general, we cannot assume that all the types are final. Indeed, if there are classloaders involved, comparing class names may not be enough to distinguish classes.
Stephen C
You could also do (object.getClass() == String.class) for the primitives instead of instanceof. That may be what was being implied instead of the .equals().
Robin
A: 

Arguably ContentValues should have provided a putObject() method along the lines of what you are about to write. I might be inclined to Extend or otherwise wrap ContentValue with a class that offers that method, hence making the code reusable.

The if/else approach you are using may feel annoyingly prosaic but will explicitly say what you mean, and production of simple, maintainable code is high on my priorities. Using reflection would also be possible but be more complex to implement. I would stick with what you are doing.

djna
+3  A: 

Your approach is the right one, though you should use instanceof instead of getClass().getName(). ie:

if (values[i] == null) {
  initialValues.putNull(names[i]);
} else if (values[i] instanceof Boolean) {
  initialValues.put(names[i], (Boolean)values[i]);
} else if (values[i] instanceof Byte) {
  initialValues.put(names[i], (Byte)values[i]);
} else if (values[i] instanceof Double) {
  initialValues.put(names[i], (Double)values[i]);
} else if (values[i] instanceof Float) {
  initialValues.put(names[i], (Float)values[i]);
} else if (values[i] instanceof Integer) {
  initialValues.put(names[i], (Integer)values[i]);
} else if (values[i] instanceof Long) {
  initialValues.put(names[i], (Long)values[i]);
} else if (values[i] instanceof Short) {
  initialValues.put(names[i], (Short)values[i]);
} else if (values[i] instanceof String) {
  initialValues.put(names[i], (String)values[i]);
} else if (values[i] instanceof byte[]) {
  initialValues.put(names[i], (byte[])values[i]);
} else if (values[i] instanceof ContentValues) {
  initialValues.putAll(names[i], (ContentValues)values[i]);
} else {
  throw new IllegalArgumentException(
      "can't put " + values[i].getClass().getName() + " in ContentValues.");
}

Another option would be to use reflection, but I consider reflection to be a method of last resort.

If ContentValues wasn't final another option would be to extend it and add a method that takes an Object.

Laurence Gonsalves
Why do you consider reflection to be a method of last resort? It looks like a good method to me.
Asa Ayers
@Asa: it is too expensive.
Stephen C
why using instanceof? primitive's wrappers are always final.. there is no need to check subclasses
dfa
dfa: instanceof is more general and more concise. Why *wouldn't* you use instanceof?
Laurence Gonsalves
@Asa: in addition to being expensive, reflection generally leads to code that's harder to maintain and that breaks static analysis (including compile time type checking, refactoring tools, etc.). In general, if you can accomplish the same thing without using reflection, don't use reflection.
Laurence Gonsalves
A: 

This is about as neat, efficient and correct as you can make it, IMO. It is possible that the compiler(s) can optimize the typecasts, but it doesn't matter much.

public long create(String[] names, Object[] values) {
    ContentValues initialValues = new ContentValues();
    for (int i = 0; i < names.length; i++) {
        Object v = values[i];
        if (v instanceof Double) {
            initialValues.put(names[i], (Double) v);
        } else if (v instanceof String) {
            initialValues.put(names[i], (String) v);
        } else {
            // You could test for v == null here ... otherwise you'll
            // get an NPE 
            throw new InvalidParameterException(
                    "Unable to convert type: " + v.getClass());
        }
    }
    return mDb.insert(this.getTableName(), null, initialValues);
}

Some notes:

  1. EDIT: In general, you probably don't want to test for an exact class C. More likely you want to test for "C or a subclass of C". You cannot test this using class names. Use instanceof for this.
  2. If you test class names, be sure that you use the right one. For example Double.class.getName() actually gives "java/lang/Double" not "java.lang.Double".
  3. As well as being slower than instanceof, comparing class names won't always give the right answer. If something is doing classloader tricks, different classes can have the same name as reported by getName() and getCanonicalName().
  4. Avoid using reflection for things like this. It is inevitably slower and the compiler won't be able to pick up a lot of your mistakes.
Stephen C