1
\$\begingroup\$

I want to get two distinct sets of schema from a database without having to create multiple Connection/PreparedStatement/ResultSet objects.

Currently, I have a helper method that queries the database, gets each individual schema, and then returns them together in one ArrayList - please refer to the code below.

private ArrayList<ArrayList<?>> getAttributesAndGroups() {
    ArrayList<ArrayList<?>> retList = new ArrayList<ArrayList<?>>();

    Connection conn = null;
    try {
        Class.forName("com.mysql.cj.jdbc.Driver");
        conn = DriverManager.getConnection(jdbcUrl, userId, password);

        ArrayList<Attribute> listAttrs = new ArrayList<Attribute>();
        String query = "select aid, aname from tbl_attributes order by aid";
        PreparedStatement ps = conn.prepareStatement(query);
        ResultSet rs = ps.executeQuery();
        while (rs.next()) {
            int aid = rs.getInt(1);
            String aname = rs.getString(2);
            Attribute attr = new Attribute(aid, aname);
            listAttrs.add(attr);
        }
        retList.add(listAttrs);

        ArrayList<Group> listGroups = new ArrayList<Group>();
        query = "select gid, gname from tbl_groups order by gid";
        ps = conn.prepareStatement(query);
        rs = ps.executeQuery();
        while (rs.next()) {
            int gid = rs.getInt(1);
            String gname = rs.getString(2);
            Group grp = new Group(gid, gname);
            listGroups.add(grp);
        }
        retList.add(listGroups);

        rs.close();
        ps.close();

    } catch (Exception e) {
        log.error("Exception", e);
    } finally {
        if (conn != null) {
            try {
                conn.close();
            } catch (Exception e) {
                //
            }
        }
    }

    return retList;
}

Where I call my helper method, I do the following:

    ArrayList<ArrayList<?>> lists = getAttributesAndGroups();

    ArrayList<Attribute> listAttrs = (ArrayList<Attribute>) lists.get(ATTRIBUTES);
    request.setAttribute("listAttrs", listAttrs);

    ArrayList<Group> listGroups = (ArrayList<Group>) lists.get(GROUPS);
    request.setAttribute("listGroups", listGroups);

    request.getRequestDispatcher("addUser.jsp").forward(request, response);

(ATTRIBUTES and GROUPS are simple integer constants representing 0 and 1 respectively.)

However, my code is giving me warnings of Type safety: Unchecked cast from ArrayList<capture#1-of ?> to ArrayList<Attribute> (ditto for the other line).

Is there a better approach for what I want to do?

\$\endgroup\$
1
  • \$\begingroup\$ For future reference, if you had provided more context (specifically the class definition for Request), I could have written out another alternative solution. \$\endgroup\$
    – mdfst13
    Commented Jan 21, 2019 at 8:09

1 Answer 1

0
\$\begingroup\$

Generics

When you say, ArrayList<ArrayList<?>>, you are saying that there is a type but that you don't know what it is. So you write it as a ?.

But in your code, you are putting ArrayList<Attribute> and ArrayList<Group> into the larger list. These are not one type. So it has to find a type that covers both Attribute and Group. Absent both extending or implementing some common class or interface, that will be Object. So ? is actually Object in your code.

It is natural to think that ? is a wildcard that can match multiple things simultaneously. But it is not. It represents exactly one type at a time. You just avoid having to say which type. As such, I don't think that it is what you want to use here.

Cast by element not collection

The general way to fix this warning on something like

    ArrayList<Attribute> listAttrs = (ArrayList<Attribute>) lists.get(ATTRIBUTES);

would be something like

    List<Attribute> listAttrs = new ArrayList<>();
    for (Object attr : lists.get(ATTRIBUTES)) {
        if (attr instanceof Attribute) {
            listAttrs.add((Attribute) attr);
        }
    }

You can often leave off the instanceof check because you know what the result should be.

I changed from the implementation (ArrayList) to the interface (List), as is customary in Java when you aren't using implementation specific methods unavailable to the interface.

But I wouldn't recommend that code for this specific problem. You don't really need to cast here. You control the code at both ends and in between. Just write it without casts.

Custom return type

The most direct way to fix this specific problem is to replace your return value with a custom class.

class AttributesAndGroups {

    private List<Attribute> attributes;

    private List<Group> groups;

    public void setAttributes(List<Attribute> attributes) {
        this.attributes = attributes;
    }

    public List<Attribute> getAttributes() {
        return attributes;
    }

    public void setGroups(List<Group> groups) {
        this.groups = groups;
    }

    public List<Group> getGroups() {
        return groups;
    }

}

You can use this like

        AttributesAndGroups result = new AttributesAndGroups();
        result.setAttributes(listAttrs);
        result.setGroups(listGroups);
        return result;

and in the caller

        AttributesAndGroups lists = getAttributesAndGroups();
        List<Attribute> listAttrs = lists.getAttributes();
        List<Group> listGroups = lists.getGroups();

No casts (unchecked or otherwise).

There's a lot of boilerplate code in the class. It's possible to avoid that here.

Pass to the method

Another alternative would be to pass the lists into the helper method rather than have the method initialize them.

        List<Attribute> listAttrs = new ArrayList<>();
        List<Group> listGroups = new ArrayList<>();
        fillAttributesAndGroups(listAttrs, listGroups);
        request.setAttribute("listAttrs", listAttrs);
        request.setAttribute("listGroups", listGroups);

I changed the name to be more representative of the revised implementation.

And then the method signature would be

private void fillAttributesAndGroups(List<Attribute> listAttrs, List<Group> listGroups) {

Now just delete

    ArrayList<ArrayList<?>> retList = new ArrayList<ArrayList<?>>();
        ArrayList<Attribute> listAttrs = new ArrayList<Attribute>();
        retList.add(listAttrs);
        ArrayList<Group> listGroups = new ArrayList<Group>();
        retList.add(listGroups);
    return retList;

And the remainder of your code should function normally.

Again, no casts needed. And this code is much terser, particularly if this is the only time you call it.

Another alternative would be to pass request into the method, but I won't try to implement that here. That would allow you to make the lists inside the method again.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ In the end I reorganized my code, so that instead of having one helper method do all the work, I call two methods in two separate DAO objects. Thanks for the concise answer though, great reference. \$\endgroup\$ Commented Jan 23, 2019 at 6:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.