0
\$\begingroup\$

I have the following entity, I am getting it in response of REST API.

class Account {
    private String ownerId;
    private List<String> coOwners;
}

REST API accepts UUID and returns linked Account.

Now my task here is to call this REST API for multiple bank accounts owned by the same person, combine coOwner and owner and return it back.

For combining coOwners and owner I have created a separate model.

class LinkedOwners {
    private String ownerId;
    private List<String> coOwners;

    // getters and setters
}

My solution:

public LinkedOwners getLinkedOwners(List<UUID> uuids) {
    LinkedOwners linkedOwners = new LinkedOwners();
    String ownerId = null;
    Set<String> coOwners = new HashSet<>();
    for (UUID uuid : uuids) {
        Account account = // call REST API with uuid
        ownerId = account.getOwnerId();
        coOwners.addAll(account.getCoOwners());
    }
    linkedOwners.setOwnerId(ownerId);
    linkedOwners.setCoOwners(coOwners);
    return linkedOwners;
}

Now the problem here is, as I am calling the API with UUIDs belonging to the same user. each Account will have the same owner, and ownerId = account.getOwnerId(); will set the same value again and again and it looks hacky.

The other solution would be to create List<Account> and perform groupBy stream operation on it.

is there a better/elegant way to merge owner and coOwners? Unfortunately, I cannot change the REST API definition as it's a third-party service.

\$\endgroup\$
2
\$\begingroup\$

Note on how to publish here: In general, it's better to try to post complete code which will compile (and ideally run) rather than fragments of code. Later in this post I show an example with test data that's runnable, you could and should have done the same thing.

Now to discuss the issue at hand.

I assume from the description that you know that your set of UUIDs relate to the same ownerId, in which case I'm a little surprised that you don't know the relevant ownerid.

However, assuming that you don't know that, then I think that the processing really needs to handle the first returned Account slightly different from the rest.

(Aside: I'm not sure Streams add much here, but then I'm not so familiar with them that I automatically reach for a Stream-based solution.)

I think your LinkedOwners class (I don't like this name, so called it MergedAccount in my solution, though I'm not sure that name is much better) should take care of the Collection of coOwners, rather than building it separately and then storing it.

Here's my approach, which has at least had some basic test data through it:

import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
    public class Govinda {
      private static class Account {
        private String ownerId;
        private List<String> coOwners;
        Account(String ownerId, List<String> coOwners) {
          this.ownerId = ownerId;
          this.coOwners = coOwners;
        }
        public String getOwnerId() {
          return ownerId;
        }
        public List<String> getCoOwners() {
          return coOwners;
        }
      }
    
  // Haven't got a REST API, so just set up some test data
  private static final List<Account> testAccounts = Arrays.asList(
      new Account("Fred", Arrays.asList("Joe", "Margo")),
      new Account("Fred", Arrays.asList("Joe", "Margo")),
      new Account("Fred", Arrays.asList("Bill", "Karen", "Pete")),
      new Account("Fred", Arrays.asList("Mark")));

  private static class MergedAccount {
    private String ownerId;
    private Set<String> coOwners;
    MergedAccount(String ownerId) {
      this.ownerId = ownerId;
      coOwners = new TreeSet<>();  // Sorted 
    }
    public String getOwnerId() {
      return ownerId;
    }
    public Set<String> getCoOwners() {
      return coOwners;
    }
    public MergedAccount addCowners(List<String> coOwnersToAdd) {
      coOwners.addAll(coOwnersToAdd);
      return this; // easy to chain with constructor
    }
  }

  public static void main(String[] args) {
    Iterator<Account> accountIterator = testAccounts.iterator();
    if (accountIterator.hasNext()) {
      Account currentAccount = accountIterator.next();
      MergedAccount mergedAccount = new MergedAccount(currentAccount.getOwnerId()).addCowners(currentAccount.getCoOwners());
      while (accountIterator.hasNext()) {
        currentAccount = accountIterator.next();
        mergedAccount.addCowners(currentAccount.getCoOwners());
      }

      System.out.format("OwnerId: %s CoOwners: %s%n", mergedAccount.getOwnerId(), String.valueOf(mergedAccount.getCoOwners()));
    }
  }
}
\$\endgroup\$
1
\$\begingroup\$

It's a bit unclear to me what the UUID you're passing to the rest service represents. Is it the ID for an account, the ID for an owner, the ID for an owner/account relationship. The answer is probably in the source of the UUID, which isn't included in the question, but presumably it's been returned from the REST service at some point. Does it really matter... well, maybe.

If there's a 1-1 mapping between UUID and ownerId, how come you don't already know the ownerId? Do you just not store it because it seems like redundant data? Are you just acting as a pass-through where your clients are responsible for knowing things like the UUID? Do your clients really need to know ownerId, rather than the UUID? What information do you/they need for future calls to the API?

Setting the ownerid over and over again with what you think is the same value and then using the last value may seem hacky, but it does avoid branching logic that doesn't add any value (if(ownerId==null)). However, you may want to consider validating your assumptions.

The rest API is a third party service, which you can't change, so have no control over. Perhaps it's really well documented and you're completely confident it does what you're expecting, however if not what happens? If the UUID represents an account, is it possible that the particular owner you are querying could be returned in the co-owner list, instead of the owner field? If this happens on the last account retrieved it will appear as if the co-owner owns all of the returned accounts.

Your clients generally are going to blame you for any mistakes that are passed back to them, so where possible consider adding in basic validation around your assumptions. In this case, validating that all of the returned accounts have the same ownerId would seem to be reasonable. If they don't, then the UUIDs are wrong, the client service isn't behaving, or your assumptions about how it works are incorrect. However if any of these happen it seems likely you'd want to know.

As an aside... it also seems a little odd to me that the Account returned from the API doesn't appear to have anything like an account identifier in it, but I guess that's not your API.

\$\endgroup\$
0
\$\begingroup\$

You could clean it up by splitting into multiple steps. It feels wrong to make a class for ownerId and coOwners... Don't you know the ownerId at the time of making the request? (How do you know that accounts belong to the same owner?)

public List<String> fetchDistinctCoOwners(List<UUID> accountIds) {
  List<Account> accounts = fetchAccounts(accountIds);
  return getDistinctCoOwners(accounts);
}

private List<Account> fetchAccounts(List<UUID> accountIds) {
  return accountIds.stream()
    .map(id -> // call API)
    .collect(Collectors.toList());
}

private List<String> getDistinctCoOwners(List<Account> accounts) {
  return uuids.stream()
      .map(Account::getCoOwners)
      .flatMap(Collections::stream)
      .distinct()
      .collect(Collectors.toList());
}

It feels odd and awkward to have accountIds and not ownerId of the account at the beginning. If you are doing an API a good design for this seems something like 'owners/:ownerId/co-owners', where the result is a list of coowners. But again, I do not know the whole context.

\$\endgroup\$

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.