-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Description
The combineWith and extendWith methods in both TransitionFunctionOne and TransitionFunctionZero rely on reference equality checks against statically initialized instances within their respective classes. However, these classes are not singletons—users can instantiate them using their public constructors.
Static Initialization Example
public class TransitionFunctionOne implements TransitionFunction {
private static final @NonNull TransitionFunctionOne one = new TransitionFunctionOne();
public TransitionFunctionOne() {
}
public static TransitionFunctionOne one() {
return one;
}
...
}Despite providing a static instance via one(), the public constructor allows users to create additional instances of TransitionFunctionOne.
Problematic Equality Check
public @NonNull Weight combineWith(@NonNull Weight other) {
// `other` may be an instance of TransitionFunctionOne but not equal to the static instance from `one()`
if (!(other instanceof TransitionFunction)) {
throw new RuntimeException();
} else if (this.equals(TransitionFunctionZero.zero())) {
return other;
} else if (other.equals(TransitionFunctionZero.zero())) {
return this;
} else if (other.equals(one()) && this.equals(one())) {
return one();
} else {
...
}
}This implementation assumes that only the static instance returned from one() (or zero()) will ever be used, which is not enforced or guaranteed by the type system. If a user constructs a new instance using new TransitionFunctionOne(), the equals(one()) check fails, and the method may incorrectly proceed to cast the object to TransitionFunctionImpl and call getValues(), resulting in a runtime exception.
Why this is problematic
-
This design is fragile and error-prone.
-
It assumes internal invariants (like singleton usage) that are not enforced by the API.
-
It relies on reference equality to control logic, which fails for valid instances created outside of the static factory method.