2
\$\begingroup\$

This is a helper class that helps fill models with data obtained from the database.

public class Util {

    public static <M> ArrayList<M> createModel(M model, Cursor cursor) throws IOException {

        try {
            final Class<?> cls = model.getClass();

            final ArrayList<M> result = new ArrayList<>();
            while (cursor.next()) {

                final Class<?> item = Class.forName(cls.getName());
                final M obj = (M) item.newInstance();

                final Field[] fields = item.getDeclaredFields();

                for (Field field : fields) {
                    field.setAccessible(true);

                    final Class<?> type = field.getType();

                    switch (type.toString()) {

                        case "int": {
                            final int value = cursor.getInt(getFieldName(field), 0);
                            field.setInt(obj, value);
                            break;
                        }

                        default:
                            //throw new IOException();
                            continue;
                    }
                }

                result.add(obj);
            }

            return result;
        } catch (IllegalAccessException | InstantiationException | ClassNotFoundException e) {
            e.printStackTrace();
            throw new IOException();
        }
    }

    private static String getFieldName(Field field) {
        final fieldName annotation = field.getDeclaredAnnotation(fieldName.class);
        return annotation == null ? field.getName() : annotation.name();
    }
}

All code here(pastebin.com)

I think my version is very bad, so I want to improve it, but I haven’t yet figured out how to do it.

Usage example:

String query = "SELECT * FROM table_name WHERE;";
Cursor cursor = helper.query(query);
List<Test> test = Util.createModel(new Test(), cursor);
\$\endgroup\$
6
  • 2
    \$\begingroup\$ Your PasteBin isn't that long; you should just include all of the code directly in the question itself, so that the question makes sense. \$\endgroup\$ – 200_success Jan 24 '19 at 14:38
  • \$\begingroup\$ Can you provide an example of how you would use this? I find it more likely that you would want a specialised "collector" or "adapter" class for each specific table in database that actually uses the column names to transform a table row into an instance of a specific class. \$\endgroup\$ – Imus Jan 24 '19 at 15:41
  • \$\begingroup\$ @200_success, I could not insert all the code here, as the system did not allow to create a question. And asked to write more text. \$\endgroup\$ – MadLax Jan 25 '19 at 2:28
  • \$\begingroup\$ @Imus, Example of use added to the question. \$\endgroup\$ – MadLax Jan 25 '19 at 2:32
  • \$\begingroup\$ The length limit is 64 kiB, which is definitely long enough for all of your code to fit. If it's complaining about your text being too short, then you should tell us more about your code. \$\endgroup\$ – 200_success Jan 25 '19 at 2:36
3
\$\begingroup\$

Why do you think that your version in "very bad" ?

There are of course some little improvements that you can do. Like declaring List<M> instead of ArrayList<M> as the return type. Also you can pass Class<M> cls instead of an empty instance of your model.

Another think to improve the readability of your code is to split your main method into many little methods. Like one for the mapping of a class.

Many dislike the switch and specially for a long one like that. If you want you can replace it with a Map<String, BiConsumer<Cursor, Field>> that associate a type and a consumer to set the value to a field. It is usually a wrong parctive to silently ignore an edge case. You may consider to throw an exception or at least log something instead of continuing in the default case.

Instead of testing the name of the type, you can compare the types and add the support for the wrapper types.

Finally for the exception. I am not sure that IOException is the best type, you can also use a logger instead of e.printStackTrace() because this one will pollute your output without any means to control it.

if ( Integer.class.equals(type) || Integer.TYPE.equals(type) ) {
  // ...
} else if ( .. ) {
  // ...  
} else {
  throw new UnsupportedTypeException("Type "+type.getName()+" cannot be mapped.");
}
\$\endgroup\$
3
  • \$\begingroup\$ Thank you, made changes to your suggestions. \$\endgroup\$ – MadLax Jan 25 '19 at 3:01
  • \$\begingroup\$ Regarding your usage example you can also create a method that execute the query and map the result because this pattern (Create query, Execute query, Map cursor) will occurs a lot of time. You can create a decorator over your helper to expose this method new SimpleOrm(helper).<M>selectAs(String query, Class<M> type):List<M> \$\endgroup\$ – gervais.b Jan 25 '19 at 8:10
  • \$\begingroup\$ int.class == type is fine. \$\endgroup\$ – Joop Eggen Oct 22 '19 at 6:40

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.