Skip to content

Conversation

@riftEmber
Copy link

No description provided.

@riftEmber riftEmber force-pushed the add-state-commands branch 2 times, most recently from d4c9754 to a2cc083 Compare September 28, 2016 03:39
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a guava MultiMap instead? Is there a reason we can't have multiple state commands for each state?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCommandForBinding() is here so it provides a more helpful error message

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between this and just setting the map?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we want to add more later

@riftEmber riftEmber force-pushed the add-state-commands branch 2 times, most recently from ca0d840 to b2a5cef Compare September 30, 2016 01:37
Command stateCommand = getCommandForBinding(commandEntry.getValue());
stateCommands.put(commandEntry.getKey(), stateCommand);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably take a MultiMap. I think there's a Multimaps.newMultiMap method which can convert a map to a multimap. Or you could just use https://github.com/FasterXML/jackson-datatypes-collections and have a Multimap in the config

logger.debug("Starting stepper");
stepper.start();

autoCommand = commandStore.getCommand("AutoInit");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You remove this here, but you never add it to the config

@riftEmber riftEmber force-pushed the add-state-commands branch 2 times, most recently from 8b3da0e to 45d3543 Compare October 8, 2016 19:53
@vpzomtrrfrt vpzomtrrfrt dismissed stale reviews from amikhalev and sentientspud October 15, 2016 19:45

Appears to be done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants