What is the best implementation of a state toggle in Java?

Question

What is the best (in terms of flexibility) object-oriented implementation for an alternating state toggle in Java? The implementations I have listed are only what I have come up with and are not exhaustive.

Note: The answer to this question is not subjective. By the principles of object-oriented programming, the context of usage for this implementation should be irrelevant.

[Edit] The focus here is on the structure of the code. Obviously the actual functionality is so simple as to not even warrant the effort of a dedicated implementation.

---

public class ImpureToggle<T> implements Supplier<T> {
    //false represents state a, true represents state b
    private boolean state;
    private final T a;
    private final T b;

    public ImpureToggle(T a, T b) {
        this.a = a;
        this.b = b;
    }

    // returns a different reference depending on internal state
    @Override
    public T get() {
        return state ? b : a;
    }
    public void toggle() {
        state = !state;
    }
}

---

public class ConsumerToggle<T> implements Consumer<Consumer<T>> {
    private final T a;
    private final T b;
    //false represents state a, true represents state b
    private boolean state;

    public ConsumerToggle(T a, T b) {
        this.a = a;
        this.b = b;
    }

    @Override
    public void accept(Consumer<T> t) {
        t.accept(state ? b : a);
    }

    public void toggle() {
        state = !state;
    }
}

---

public interface ImpureStaticToggle {
    // reassigns parameter 'state'
    static <T> void toggle(T state, T a, T b) {
        state = state == a ? b : a;
    }
}

---

public interface PureStaticToggle {
    // returns a different reference depending exclusively on external input
    static <T> T toggle(boolean state, T a, T b) {
        //false represents state a, true represents state b
        return state ? b : a;
    }
}

---

/*
Just as an example of an unarguably bad implementation:
*/
public class MutableToggle<T> implements Supplier<T> {
    private T state;
    private final T a;
    private final T b;

    public MutableToggle(T a, T b) {
        state = a;
        this.a = a;
        this.b = b;
    }

    // exposes a mutable reference, which could completely break the logic of this
    // object and others
    @Override
    public T get() {
        return state;
    }

    public void toggle() {
        state = state == a ? b : a;
    }
}

---

[Edit] ternary for inverting boolean (was done for consistency) replaced with logical complement operator as per @gargkshitiz.

Answer 1

Your first implementation (ImpureToggle) looks okay. Just change the toggle method to be:

state = !state

But having such a Toggler with a public toggle method looks like an overkill. Either use the whole class with proper access modifiers OR instead use a local method to limit the scope and the potential bugs.

Answer 2

By the principles of object-oriented programming, the context of usage for this implementation should be irrelevant.

Not sure what this means, and you seem firm on not giving context, but I'll try my best to give deeper insight into why I feel what you're doing doesn't make much sense.

---

Don't pass booleans as arguments¹

broadly speaking if there is a parameter passed into a function that selects specific behaviour to be executed then further step-wise refinement is required; Breaking up this function in to smaller functions will produce more highly cohesive ones

---

The problem with a parameter passed in as you describe, is that the function is doing more than two things; it may or may not check the users access rights depending on the state of the Boolean parameter, then depending on that decision tree it will carry out a piece of functionality.

---

It would be better to separate the concerns of Access Control from the concerns of Task, Action or Command.

Take, for example, String#regionMatches. It has an overload for ignoring case.

public boolean regionMatches(boolean ignoreCase, int toffset, String other, int ooffset, int len) {
    if (!ignoreCase) {
        return regionMatches(toffset, other, ooffset, len);
    }
    // Note: toffset, ooffset, or len might be near -1>>>1.
    if ((ooffset < 0) || (toffset < 0)
            || (toffset > (long)length() - len)
            || (ooffset > (long)other.length() - len)) {
        return false;
    }
    byte tv[] = value;
    byte ov[] = other.value;
    if (coder() == other.coder()) {
        return isLatin1()
          ? StringLatin1.regionMatchesCI(tv, toffset, ov, ooffset, len)
          : StringUTF16.regionMatchesCI(tv, toffset, ov, ooffset, len);
    }
    return isLatin1()
          ? StringLatin1.regionMatchesCI_UTF16(tv, toffset, ov, ooffset, len)
          : StringUTF16.regionMatchesCI_Latin1(tv, toffset, ov, ooffset, len);
}

This is a clear example, from the standard library, of why you should avoid boolean parameters for your behaviors.

Notice how the boolean determines which implementation should be used: one which ignores casing, or one which doesn't.

This is a cheap trick typically used to make choosing an implementation less verbose:

for(int i = 0; i < 100; i++) {
    boolean even = i % 2 == 0;
    boolean matches = text.regionMatches(even, ...);

    // use matches
}

However, at a glance, it's not clear exactly what that condition is determining. We're forced to open the documentation (or worse: the implementation).

Compare that to:

for(int i = 0; i < 100; i++) {
    boolean even = i % 2 == 0;
    boolean matches = false;

    if(even)
        matches = text.regionMatchesIgnoreCase(...);
    else
        matches = text.regionMatches(...);

    // use matches
}

Or

for(int i = 0; i < 100; i++) {
    boolean even = i % 2 == 0;
    boolean matches = even ? text.regionMatchesIgnoreCase(...) : text.regionMatches(...);

    // use matches
}

It's more verbose, but it's clearer as to what the condition is for: determining whether casing should be ignored.

regionMatchesIgnoreCase would be easier to comprehend at a glance, rather than needing to read the documentation to determine what the boolean represents.

Comprehension is important for avoiding time waste when fixing critical bugs. Assuming you want to blindly apply principles, this knocks out PureStaticToggle.

---

Don't use interfaces as utility classes¹ (easy fix)

This seems to me to cut against the grain of interfaces. One would have to look around the API to determine that there are no classes that implement this interface, and that there are no producers or consumers of this interface

---

If you look at the new Java 8 APIs, you'll see that the final class idiom is still used despite the ability to add static methods on interfaces.

This would remove both interface alternatives. It can be easily worked around: You can ignore this principle, or use a regular class.

But, what would this utility type be for? What other methods would be in the utility type? 1 type per utility implementation seems excessive, bloats the namespace.

Using an interface doesn't make your code OOP. Interfaces in general are not an OOP concept. However, their initial/primary function (before static and private methods) were OOP. Java supports multiple paradigms, hence the exposure of static methods in interfaces.

---

Design by contract¹

Software designers should define formal, precise and verifiable interface specifications for software components, which extend the ordinary definition of abstract data types with preconditions, postconditions and invariants.

Assuming you want sturdy interfaces for your implementations, you should expose contracts.

If you aren't familiar with contracts, they're a set of rules followed by both the client of the code & the code itself. If the code doesn't work based on what it states in the contract, it's considered to be bugged.

In Java, they're typically defined by JavaDocs. However, no matter how you choose to expose your contracts to users, the point here is that clients should know what that piece of code will and won't do, and code should define how the user should use the code.

How would your contract look for the types you've proposed?

Contracts are built based off requirements. From the code shown, the requirements aren't clear. In fact, the interface approaches

---

In terms of OOP, getters violate encapsulation¹

It is not encapsulation and [using] Lombok [to generate getters & setters] is just making to work with procedural code less painful

---

And data structure is not an object

---

You should encapsulate state and implementation details so that object has full control on that. Logic will be focused inside object and will not be spread all over the codebase

Getters are procedural, not object oriented.

In OOP, objects communicate via behaviors. When you expose getters, you are exposing properties of the object.

The reason why OOP prefers hidding the properties of objects can vary, with some being obvious: Properties are eventually used in logic somewhere, and the logic/behavior which relies on that property won't be easily apparent if exposed.

Using a call-back to handle the logic of the property, especially how you're doing it in ConsumerToggle, is not much different from exposing a getter.

---

ImpureStaticToggle won't work as it is (easy fix)

Java is pass by value.

String s = "first";
toggle(s, "second", "third");
System.out.println(s); // prints "first"

The value of s will remain unchanged. It can be fixed with a return statement & assignment when calling the function:

<T> T toggle(T state, T a, T b) {
    return state == a ? b : a;
}

T value = toggle(value, a, b);

However, this approach is still flawed for reasons mentioned in some of the sections above.

Last notes

ImpureStaticToggle and PureStaticToggle are different.

• The former determines a return value based on the type of a reference
• The latter determines a return value based on the result of any condition.

You can use PureStaticToggle to achieve what ImpureStaticToggle does. But, you cannot use ImpureStaticToggle to do what PureStaticToggle can do. They aren't completely interchangable, and those details should impact your choice.

What you are ultimately doing with the code you've shown is changing the implementation based on a condition. That's all that's going on here.

I hate to say it, but if your goal is to follow OOP principles by "tossing the book" at your code, then all your alternatives violate commonly practiced OOP principles.

Don't overcomplicate things. I don't see any benefit in encapsulating/hiding the use of a ternary. Use the ternary as is, when needed. Invest the time you would have invested in this design into something important.

Also, for your interfaces, toggle isn't the best name either, since the behavior isn't actually toggling anything - a better name would be chooseValue or determineValue, as that's what the method is actually doing.