RuleChain.of(...) #582

Closed
wants to merge 1 commit into
from

Projects

None yet

5 participants

@panchenko

The existing way of constructing RuleChain using outerRule() and around() calls seems too verbose without any additional benefits.
I would like to have simpler way of constructing a chain. From my point of view the most logical way is passing them all as parameters of the single method call.

@dsaff
JUnit member

Thanks for the idea.

While the current API is not necessarily ideal, it does have the benefit of clearly communicating the meaning of the ordering. I don't see enough reason to add a competing constructor scheme.

@panchenko

The rationale for me is that I want to "initialize these rules in the specific order", but for sure I am not thinking about it in a way like "construct the chain with one item being outer rule surrounding others". Probably we have different use cases, I don't know. We can use longer method name to "communicate the meaning of the ordering" and even deprecate the current API. Also, suggested change clearly communicates that chain should have 2+ components.

@dsaff
JUnit member

I'm still not a big fan, but could be swayed. Would you mind bringing up a thread on junit@yahoogroups.com? Let's see how it plays there.

@nealeu

I think the simplicity of RuleChain.of() is great, and that the Javadoc can cover the ordering.

If that's not sufficient, then RuleChain.outerToInner() or RuleChain.outerFirst() would be sufficient and a big improvement.

@nealeu nealeu commented on the diff Jan 31, 2013
src/main/java/org/junit/rules/RuleChain.java
@@ -67,6 +67,20 @@ public static RuleChain outerRule(TestRule outerRule) {
return emptyRuleChain().around(outerRule);
}
+ /**
+ * Returns a {@code RuleChain} containing the specified rules in the
+ * specified order.
@nealeu
nealeu Jan 31, 2013

This could be specific in saying that the order is outermost first.

@stefanbirkner

I prefer the current outerRule and around syntax, because it is more meaningful. Nevertheless I see the advantage of creating a RuleChain from a list of rules. But I'm not happy with the name of. It is shown as the rules are executed sequentially. But that's not true. Therefore we should use one of the names proposed by @nealeu: innerToOuter or outerToInner.

@panchenko

Actually, initialization is executed sequentially before the test, and cleanup is executed sequentially in the reverse order after the test.

Looks like different points of view here. For junit-developers the internal details like outer/inner/around are important. However, from a user point of view, it works like in my first sentence of this comment.

@dsaff
JUnit member

I'd consider this if we use outerToInner or innerToOuter.

@dsaff
JUnit member

@panchenko, are you still interested in this pull, given the new suggested nomenclature?

@panchenko

@dsaff The suggested syntax looked quite nice in one of my projects, that's why I am trying to share it.
It makes sense to improve the API, but I am not convinced with outerToInner/innerToOuter. Do you have other naming ideas? Perhaps, something with a verb?

@panchenko

My use case was setting up a few similar data structures (typically 2 or 3 per test class), however initialization order is important, that's why I used RuleChain:

private static final ProjectSetup A = new ProjectSetup(...);
private static final ProjectSetup B = new ProjectSetup(...);
private static final ProjectSetup C = new ProjectSetup(...);

@ClassRule
public static final RuleChain CLASS_RULE_CHAIN = RuleChain.of(A, B, C);

It is concise and "initialization in the specific order" intent is clear to the reader.
The method name can be different, but it should not be too long, as thinking more on it - it is the only static method which makes sense in the class (2 overloads, accepting varargs and List).
For me, RuleChain is kind of syntactic sugar for executing rules in the fixed order, I don't treat it as something, which makes sense on it's own, so why creating an empty chain and adding elements one by one? There is List for that.

@dsaff
JUnit member

@panchenko,

So, I'm on board with the general idea, but we need to improve the name. Consider this:

private static final ProjectSetup A = new ProjectTeardown(...);
private static final ProjectSetup B = new ProjectTeardown(...);
private static final ProjectSetup C = new ProjectTeardown(...);

@ClassRule
public static final RuleChain CLASS_RULE_CHAIN = RuleChain.of(A, B, C);

Given this implementation, the teardowns will likely be executed in the order C, B, A. Is this what the developer intended?

"of" is simple, but rules have a certain amount of irreducible complexity, since they are wrapping operations. If you don't like outerToInner or innerToOuter, how about innerFirst, or outerFirst, or do you have another suggestion? "of" isn't going to be committed.

@dsaff
JUnit member

@panchenko, still interested in moving this forward?

@dsaff
JUnit member

Closing for now. Happy to re-open when folks are interested.

@dsaff dsaff closed this Jun 24, 2013
@ittaiz

I have to say that as a user I think "of" says what it does (e.g. A.before,B.before,C.before,Test,C.after,B.after,A.after).
And I would also like this change in.

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