I have an event handler that creates objects for which it needs data from both before and after an event. The event handler itself implements Selenium's WebDriverEventListener
interface and gets registered accordingly. That is, I have no control over how the class is being invoked.
In order to create the objects, I use a mutable field. Here is a simple example (in Java):
public class MyEventHandler {
private MyType myMutableField;
public void beforeEvent(DataA a) {
myMutableField = new MyType();
myMutableField.setA(a);
}
public void afterEvent(DataB b) {
myMutableField.setB(b);
// Do something with myMutableField...
}
}
To prevent inappropriate method invokations (e.g. consecutive invokations of beforeEvent
or afterEvent
), I do the following:
public class MyEventHandler {
private MyType myMutableField;
public void beforeEvent(DataA a) {
if ( myMutableField != null ) {
throw new IllegalStateException();
}
// ...
}
public void afterEvent(DataB b) {
// ...
myMutableField = null;
}
}
I have also implemented mechanisms such that setters can be invoked only once.
However, I'm not really happy with this design approach and I wonder if there is a pattern that helps me to a) avoid the mutable field and/or b) make MyType
immutable.
2 Answers 2
I have also implemented mechanisms such that setters can be invoked only once.
Why don't' we change the rationale? Let's make it executable "at least once" and make it idempotent so that we don't have to be worried about the number of subsequent calls.
public class MyEventHandler {
private MyType myMutableField;
public void beforeEvent(DataA a) {
if ( stateBeforeEvent ) {
runMyCode(a);
}
}
public void afterEvent(DataB b) {
if (stateAfterEvent) {
runMyCode(a,b);
}
}
}
For brevity, I didn't give any implementation for stateBeforeEvent
and stateAfterEvent
. How to obtain and hold both states (conditions) depends on your resources at hand.
Note that I have removed the exception throwing. Unless you need to break the whole execution path I found it to be quite "defensive". Too strict. If you still need to throw an exception, we need to make some changes
public class MyEventHandler {
private MyType myMutableField;
public void beforeEvent(DataA a) {
if ( !stateBeforeEvent ) {
throw IllegalStateException("BeforeEvent has been already executed!");
}
runMyCode(a);
}
public void afterEvent(DataB b) {
if (!stateAfterEvent) {
throw IllegalStateException("AfterEvent has been already executed!");
}
runMyCode(a,b);
}
}
However, I'm not sure if we can label these methods as "idempotent" any more. Probably not.
Related links
-
+1 although it doesn't remove the mutable field, I like the approach.beatngu13– beatngu132018年12月12日 20:43:17 +00:00Commented Dec 12, 2018 at 20:43
-
Perhaps, you could create MyType only when you have both Data objects (A and B). Can not you wait untill afterEvent is called?
new MyType (a,b)
?Laiv– Laiv2018年12月12日 21:38:53 +00:00Commented Dec 12, 2018 at 21:38 -
Instead of using
MyType
as a mutable field, I could use a field of typeDataA
and then create an immutable version ofMyType
inafterEvent
. But the reference of that field can't befinal
either.beatngu13– beatngu132018年12月12日 21:45:00 +00:00Commented Dec 12, 2018 at 21:45 -
Inmutable and final are different things. You could use a builder and make builder's methods idempotent. At the end, when
build()
is called, a single MyType is returned. Just make MyType' setters protected and let the builder to be the only one allowed to call these methods.Laiv– Laiv2018年12月12日 21:58:10 +00:00Commented Dec 12, 2018 at 21:58 -
1"Immutable and final are different things." Yep, see the original question: a)
final
field, b) immutableMyType
. Regarding the builder: I already tried a step builder, which addresses the immutability ofMyType
.beatngu13– beatngu132018年12月12日 22:04:52 +00:00Commented Dec 12, 2018 at 22:04
It seems your using code will look like this:
MyEventHandler meh = new MyEventHandler();
meh.beforeEvent(a);
DataB b = someCalculation();
meh.afterEvent(b);
When it could have looked like this:
MyEventHandler meh = new MyEventHandler(a);
DataB b = someCalculation();
meh.afterEvent(b);
Done that way everything can be immutable. This is the same trick closures pull off in functional languages. This is just how the trick looks in an object oriented language.
In either case the trick is basically saving something you know now to use with something you'll learn later. Possibly somewhere else.
Disclaimer: I'm only using these names so you can match them to your original code. Please improve them before you use this.
-
Is not the handler itself a whole "AfterEvent"? Handlers come into play when "something" already happened. Given your answer, I wonder if we could not just make something like
new MyEventHandler(a, someCalculation())
, I don't why butafterEvent
andbeforeEvent
seems both code smell to me.Laiv– Laiv2018年12月12日 10:48:17 +00:00Commented Dec 12, 2018 at 10:48 -
Unfortunately, I have no control over how the class is being invoked (edited the question accordingly). Sorry, should have mentioned this earlier.beatngu13– beatngu132018年12月12日 10:59:17 +00:00Commented Dec 12, 2018 at 10:59
-
@beatngu13 I've noted your edit but still don't see your using code. Show me what I'm working with.candied_orange– candied_orange2018年12月12日 20:05:37 +00:00Commented Dec 12, 2018 at 20:05
-
What exactly would you like to see? I have tried to simplify the code example as good as possible, and the handler is all that is under my control. I just hand the implementation over to the Selenium framework and it gets invoked whenever the corresponding events occur.beatngu13– beatngu132018年12月12日 20:18:56 +00:00Commented Dec 12, 2018 at 20:18
serverAboutToReboot
/serverRebooting
/serverRebooted
DataA
. But you might also be better served by looking into a proper state machine library.serverAboutToReboot / serverRebooting
these are not events because they are still happening!. These are 'states", not events.