Skip to content

Commit

Permalink
improve FeatureRepository impl performance
Browse files Browse the repository at this point in the history
reason: the backing data structure for features was an ArrayList, which has O(n) worst-case
look-up performance. since the method findFeature(...) is frequently used, this has negative
implications on the performance of the library. A HashMap is better suited for feature look-up.
  • Loading branch information
goekay committed Apr 16, 2019
1 parent 2ee40bc commit d1a4a83
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 37 deletions.
58 changes: 21 additions & 37 deletions ocpp-common/src/main/java/eu/chargetime/ocpp/FeatureRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,14 @@ of this software and associated documentation files (the "Software"), to deal
import eu.chargetime.ocpp.model.Request;
import eu.chargetime.ocpp.utilities.MoreObjects;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

public class FeatureRepository implements IFeatureRepository {

private ArrayList<Feature> featureList;

public FeatureRepository() {
this.featureList = new ArrayList<>();
}
private final Map<String, Feature> actionMap = new HashMap<>();
private final Map<Class<?>, Feature> classMap = new HashMap<>();

/**
* Add {@link Profile} to support a group of features.
Expand All @@ -50,64 +47,51 @@ public FeatureRepository() {
* @see Profile
*/
public void addFeatureProfile(Profile profile) {
Feature[] features = profile.getFeatureList();
Collections.addAll(featureList, features);
for (Feature feature : profile.getFeatureList()) {
addFeature(feature);
}
}

/**
* Add {@link Feature} to support.
* @param feature supported {@link Feature}.
*/
public void addFeature(Feature feature) {
featureList.add(feature);
actionMap.put(feature.getAction(), feature);
classMap.put(feature.getRequestType(), feature);
classMap.put(feature.getConfirmationType(), feature);
}

/**
* Search for supported features added with the addProfile.
* If no supported feature is found, null is returned
* If no supported feature is found, {@link Optional#empty()} is returned
* <p>
* Can take multiple inputs:
* {@link String}, search for the action name of the feature.
* {@link Request}/{@link Confirmation}, search for a feature that matches.
* Anything else will return null.
* Anything else will return {@link Optional#empty()}.
*
* @param needle Object supports {@link String}, {@link Request} or {@link Confirmation}
* @return Optional of instance of the supported Feature
*/
@Override
public Optional<Feature> findFeature(Object needle) {
for (Feature feature : featureList) {
if (featureContains(feature, needle)) {
return Optional.of(feature);
}
if (needle instanceof String) {
return Optional.ofNullable(actionMap.get(needle));
}

return Optional.empty();
}
if ((needle instanceof Request) || (needle instanceof Confirmation)) {
return Optional.ofNullable(classMap.get((needle.getClass())));
}

/**
* Tries to match {@link Feature} with an {@link Object}.
* Different outcome based on the type:
* {@link String}, matches the action name of the feature.
* {@link Request}, matches the request type of the feature.
* {@link Confirmation}, matches the confirmation type of the feature.
* Other wise returns false.
*
* @param feature to match
* @param object to match with, supports {@link String}, {@link Request} or {@link Confirmation}
* @return true if the {@link Feature} matches the {@link Object}
*/
private boolean featureContains(Feature feature, Object object) {
boolean contains = false;
contains |= object instanceof String && feature.getAction().equals(object);
contains |= object instanceof Request && feature.getRequestType() == object.getClass();
contains |= object instanceof Confirmation && feature.getConfirmationType() == object.getClass();
return contains;
return Optional.empty();
}

@Override
public String toString() {
return MoreObjects.toStringHelper("FeatureRepository")
.add("featureList", featureList)
.add("actionMap", actionMap)
.add("classMap", classMap)
.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package eu.chargetime.ocpp.test;

import eu.chargetime.ocpp.FeatureRepository;
import eu.chargetime.ocpp.feature.Feature;
import eu.chargetime.ocpp.model.Confirmation;
import eu.chargetime.ocpp.model.Request;
import eu.chargetime.ocpp.model.TestConfirmation;
import eu.chargetime.ocpp.model.TestRequest;
import org.junit.Test;

import java.util.Optional;
import java.util.UUID;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class FeatureRepositoryTest {

private final TestFeature feature = new TestFeature();
private static final String ACTION_NAME = "TestFeature";

@Test
public void testFind() {
FeatureRepository f = new FeatureRepository();
f.addFeature(feature);

assertFalse(f.findFeature("TestFeatureFail").isPresent());
assertFalse(f.findFeature(3).isPresent());

assertWhenFound(f.findFeature(ACTION_NAME));
assertWhenFound(f.findFeature(new TestRequest()));
assertWhenFound(f.findFeature(new TestConfirmation()));
}

private void assertWhenFound(Optional<Feature> dummyFeature) {
assertTrue(dummyFeature.isPresent());
assertEquals(dummyFeature.get(), feature);
}

private static class TestFeature implements Feature {

@Override
public Confirmation handleRequest(UUID sessionIndex, Request request) {
throw new RuntimeException("Do not call this method");
}

@Override
public Class<? extends Request> getRequestType() {
return TestRequest.class;
}

@Override
public Class<? extends Confirmation> getConfirmationType() {
return TestConfirmation.class;
}

@Override
public String getAction() {
return ACTION_NAME;
}
}
}

0 comments on commit d1a4a83

Please sign in to comment.