3
\$\begingroup\$

I have a list of conversion histories, where information about user channels is stored. For example user may have app for iOS and Android installed. Each time user logs in, I store a conversion history, so that there might be multiple rows with shared data (e.g. client is always iOS).

I need to get the latest usage of iOS and Android in this case, as you can see code is pretty much in 'serial' way, so I wonder how this part can be optimized. Any help is appreciated.

private List<UserChannel.AppUser> findLatestAppClients(
            List<Conversion> conversions
    ) {
        Optional<Conversion> iosClient =
                conversions.stream().filter(c -> IOS.clientName().equals(c.getClient())).findFirst();

        Optional<Conversion> androidClient =
                conversions.stream().filter(c -> ANDROID.clientName().equals(c.getClient())).findFirst();

        List<UserChannel.AppUser> appUsers = new ArrayList<>();

        if (iosClient.isPresent()) {
            appUsers.add(new UserChannel.AppUser(iosClient.get().getClient(), iosClient.get().getClientId()));
        }

        if (androidClient.isPresent()) {
            appUsers.add(new UserChannel.AppUser(androidClient.get().getClient(), androidClient.get().getClientId()));
        }

        return appUsers;
    }
\$\endgroup\$
5
\$\begingroup\$

There's no problem with the performance. Java 8 filtering actually doesn't scan the whole stream, so the processing will be done only until the first element meeting the predicate is hit. So you're fine this way. However, I would do a bit of refractoring here:

private List<UserChannel.AppUser> findLatestAppClients(
            List<Conversion> conversions) {
    Optional<Conversion> iosClient = findFirstWithName(IOS.clientName());

    Optional<Conversion> androidClient = findFirstWithName(ANDROID.clientName());

    List<UserChannel.AppUser> appUsers = new ArrayList<>();

    addIfPresent(appUsers, iosClient);

    addIfPresent(appUsers, androidClient);

    return appUsers;
}

private Optional<Conversion> findFirstWithName(String name){
    return conversions.stream().filter(c -> name.equals(c.getClient())).findFirst();
}

private void addIfPresent(List<UserChannel.AppUser> appUsers, Optional<Conversion> conversion){
    if (conversion.isPresent()) {
        appUsers.add(new UserChannel.AppUser(conversion.get().getClient(), 
            conversion.get().getClientId()));
    }
}
\$\endgroup\$
  • \$\begingroup\$ Nice refactor, but consider going further and putting the findFirstWithName(...) calls inside the addIfPresent(...) to become: addIfPresent(appUsers, findFirstWithName(IOS.clientName())) \$\endgroup\$ – rolfl May 2 '16 at 11:53
  • \$\begingroup\$ You can even call directly ifPresent on the Optional to make it shorter. \$\endgroup\$ – Tunaki May 2 '16 at 12:00

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.