What is the best way to store actions as data in java?

Question

I have a class (Activity) in an android application that has a bunch of example math problems on it. You can move to a different question by pressing a button which changes a question counter and displays a new question. You can see how to solve a question by clicking a 'show work' button, which displays a bunch of information on the screen. My problem is I have a bunch of methods that look like this:

public void showWorkButtonClicked()
{
    if (questionCounter == 1)
        showWork1();
    else if (questionCounter == 2)
        showWork2();
    else if (questionCounter == 3)
        showWork3();
    //for how ever many questions are available
}

Obviously these if-statement are terrible design. I could store the data needed for each function in a class, which might work in this case (but would probably just make things confusing), but what if each showWork method was sufficiently unique to make this impractical. I know if I was making the app in C#, I could simply put delegates into a list and would have an elegant solution. If anybody has a better solution (that ideally uses less code), I would love to hear it.

See functors in Java [Help with understanding a function object or functor in Java](http://stackoverflow.com/questions/7369460/help-with-understanding-a-function-object-or-functor-in-java) as well as this PDF of a slide package [Functors and the Command Pattern](http://www.buyya.com/254/Patterns/Command-2pp.pdf). Also see https://www.cs.odu.edu/~zeil/cs330/f15zeil/Public/functors/index.html — Richard Chambers, May 18 '16 at 21:01

score 4 · Answer 1 · answered May 18 '16 at 20:40

4

Use a Map<Integer, Runnable> to store your actions:

private static Map<Integer, Runnable> actions = new HashMap<Integer, Runnable>() {{
    put(1, () -> showWork1());
    put(2, () -> showWork2());
    put(3, () -> showWork3());
}};

then look them up:

public void showWorkButtonClicked() {
    actions.getOrDefault(questionCounter, () -> {}).run();
}

Here I use a "do nothing" runnable to avoid an NPE if there's no action for a number. Alternatively, you could:

public void showWorkButtonClicked() {
    Optional.of(questionCounter)
       .map(actions::get)
       .orElseThrow(IllegalArgumentException::new)
       .run();
}

answered May 18 '16 at 20:40

Bohemian

412,405
93
575
722

If these `int` values form a contiguous range, a `Runnable[]` array will do as well. The advantage of using an array is that you can truly initialize it with a curly brace syntax instead of using the *obfuscated anonymous inner class* anti-pattern. – Holger May 20 '16 at 09:25
@Holger this is not an inner class. It's a *static* inner class - big difference, because there's only one instance. And it's not obfuscated (it's obvious), or an anti pattern (it's just fine). And an array doesn't solve the general case, which is mapping from any number (contiguous integers is an edge case). – Bohemian May 20 '16 at 09:37
It’s an obsolete additional class whether `static` or not. Creating a superfluous additional class just to save a few characters *is* an anti-pattern. A clear contradiction to the “prefer composition over inheritance” recommendation. And it’s obfuscated as you placed the curly braces carefully to make it look different than a nested class with nested initializer. – Holger May 20 '16 at 09:41
Looking at the questions code, it seems to be exactly about the “edge case” of contiguous integers. That’s what I would expect from a variable named “…Counter”. – Holger May 20 '16 at 09:43
@holger I'm thinking of future visitors (which is the site's premise) who may have a similar problem, but not a range. Also, it's a very neat way to initialise an unmodifiable collection - see code block 2 of [this popular answer](http://stackoverflow.com/a/6763606/256196) – Bohemian May 20 '16 at 10:48
There are higher rated answer telling even worse things. That doesn’t prove anything. Besides that, consider what [this](http://stackoverflow.com/a/27521360/2711488), much higher voted, answer tells you: “*Every time someone uses double brace initialization, a kitten gets killed*” And in case you don’t like kittens anyway, consider [this](http://stackoverflow.com/a/924326/2711488) – Holger May 20 '16 at 11:07
@Holger Those posts discuss *inner* classes, not *static inner* classes. There's no possible memory leak with a `static` map, because there's nothing to leak. Yes, there's one extra class (with a singleton instance) generated, but that is just the way that the compiler handles an anonymous class. Why does it matter if there's an extra class file lying around that has a dollar sign in it? Who cares? Why should anyone care? I do agree that anonymous classes (which create inner classes that have an implicit reference to an instance) are dangerous, but static in-line initializers are harmless. – Bohemian May 20 '16 at 12:47
I doubt that people developing the habit of using that anti-pattern are always careful enough to recognize whether the context is `static` or not (as that’s not visible from the code pattern itself). The reader of your answer is not aware that you are using something that shouldn’t be used once the `static` modifier is removed. And is saving 20 characters in source code really worth having an additional class file on your disk and in all deployed copies of your software? And what about the kitten? You said nothing about the kitten. – Holger May 20 '16 at 12:55

score 0 · Answer 2 · answered May 18 '16 at 20:47

0

You should not put any data in your code nor you should use dedicated functions to display something different. If you want to display data depending on a counter use a List or a Map to store it and read it in a single function for display.

answered May 18 '16 at 20:47

Thomas

1,622
17
24

What is the best way to store actions as data in java?

2 Answers2