GAE datastore JOIN + GROUP BY

I had posted this here and was asked to move it to code review since my code works and am looking for the best possible solution : original post

NOTE: I have seen the JOIN in GAE related posts but GROUP BY in my requirements made me make a new post and I have made a solution of my own which I wanted to check how good is it.

Datastore Entities properties and types (database)

-
Entity : PartyEntity let's call this as a

• String partyName

• String partyId

• BlobKey image

-
Entity : InsertEntity let's call this as b

• String partyIdentifier

• String name

• String constituency

Objective

SELECT * FROM a JOIN b on a.partyId = b.partyIdentifier GROUP BY b.constituency;


The solution I made is as follows. Do suggest any changes or better ideas.

```
// SELECT FROM a (PartyEntity holds unique partyId) and for each partyId SELECT FROM b

private List initData() {

DatastoreService datastore = DatastoreServiceFactory
.getDatastoreService();

List data = new ArrayList();

Query query = new Query("PartyEntity");
PreparedQuery preparedQuery = datastore.prepare(query);
List entities = preparedQuery.asList(FetchOptions.Builder
.withDefaults());

for (Entity entity : entities) {

String PARTY_ID = (String) entity.getProperty("partyId");

// fetch from another
Query query2 = new Query("InsertEntity");
Filter filterByPartyId = new Query.FilterPredicate(
"partyIdentifier", FilterOperator.EQUAL, PARTY_ID);
query2.setFilter(filterByPartyId);
PreparedQuery preparedQuery2 = datastore.prepare(query2);

List entities2 = preparedQuery2.asList(FetchOptions.Builder
.withDefaults());

for (Entity entity2 : entities2) {
ListDisplay display = new ListDisplay();
display.setPartyId((String) entity.getProperty("partyId"));
display.setPartyName((String) entity.getProperty("partyName"));
d

Just a few generic notes, I'm not familiar with Google App Engine:

-
HashMap reference types could be Map:

public Map> getData() {

See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

-
I'd rename this variable to result to express its purpose:

List data = new ArrayList();

-
remove is unnecesary here, put will override it:

data.remove(d.getConstituency());
data.put(d.getConstituency(), hashlist);

-
Consider replacing the following with a Multimap:

// if null add
if (data.get(d.getConstituency()) == null) {
    List internal = new ArrayList<>();
    internal.add(d);
    data.put(d.getConstituency(), internal);
}
// else modify
else {
    List hashlist = data.get(d.getConstituency());
    hashlist.add(d);
    data.remove(d.getConstituency());
    data.put(d.getConstituency(), hashlist);
}

The following uses Google Guava's Multimap and much simpler:

final ArrayListMultimap data = ArrayListMultimap.create();

for (ListDisplay d : list) {
    data.put(d.getConstituency(), d);
}

If you really need Map return type you can convert it back to Map with data.asMap() which returns a Map> but you cast it down safely to Map>. (ArrayListMultimap javadoc supports this as well as these Stack Overflow posts.)

-
Variable names in Java usually camelCase.

String PARTY_ID = (String) entity.getProperty("partyId");

-
Comments on the closing curly braces are unnecessary and disturbing. Modern IDEs and editors could show blocks.

} // end loop entity2

“// …” comments at end of code block after } - good or bad?

StackExchange Code Review Q#45464, answer score: 3