Allow empty layers in LayeredArchitecture & OnionArchitecture when configured

[hankem]

Resolves #267 & #271:

After #177 had prevented empty layers by default, this can now be overridden to re-allow empty layers in LayeredArchitecture & OnionArchitecture.

[ghost]

DeepCode's analysis on #e9f87d found:

• ❌ 0 critical issues. ⚠️ 0 warnings and 0 minor issues. ✔️ 0 issues were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.

---

☺️ If you want to provide feedback on our bot, here is how to contact us.

[codecholeric]

Kewl, thanks a lot for your support 😄 I added a couple of thoughts in some places, tell me what you think. Also I'd be really interested in your input on #267 (comment)

[codecholeric]

I'm wondering, if it would be better to have either two tests, or specify given and the assertion in the data point. I find conditionals in assertions harder to read and harder to maintain in the long run. What do you think?
I would see as alternatives to either extract the common given into a method or pass more into the data point, e.g. pass a function pair given{ architecture -> architecture.allowEmptyLayers(true) } and expected{ failureReport -> assertThat(failureReport.isEmpty()).as("failure report").isTrue(); } as data point.

[hankem]

Alternatively, we can also extract helper methods such as

• private OnionArchitecture anOnionArchitectureWithEmptyLayers()
• private void assertFailureOnionArchitectureWithEmptyLayers(EvaluationResult result)

and explicitly unroll the data points into

• @Test public void onion_architecture_rejects_empty_layers_by_default()
• @Test public void onion_architecture_allows_empty_layers_if_configured_to_allow()
• @Test public void onion_architecture_rejects_empty_layers_if_explicitly_configured_to_not_allow()

[codecholeric]

Same as below. For me it is also hard to immediately grasp at the first look, that those two conditional blocks relate to each other and are only set in conjunction...

[hankem]

There's no 1:1 correspondence, but an explicit test is probably still better than this implicit construction.

[codecholeric]

@hankem do you have any strong opinion regarding the discussion of #278 (the discussion if the new method should also become part of the description)? To me it seems that either both of these features, or none of them should add additional information to the description, since they are of similar type...

[hankem]

No, I don't have a strong opinion.

I initially considered allowEmptyLayers rather as a technical configuration of the ArchRule, similar to @AnalyzeClasses's packages or importOptions, which have IMO a similar relevance (Whether layers are empty or not obviously depends on which classes are analyzed...), but are not included in the test report either, so I hadn't even thought about it.

A difference between those and this configuration specific to LayeredArchictecture or OnionArchitecture is, however, that we could easily adapt the description in this case, so if @roxspring has good reasons to do so, I can try to adapt the description for allowEmptyLayers as well.

[codecholeric]

Looks noice now, thank you so much 😄
Glad we can take away the pain the increased restrictions have caused!