Documentation Change Request: Best Practices

[derekgreer]

I have a few recommendations for changes to the Best Practices documentation. Overall I agree with the spirit of what is currently reflected, but in a few places I feel like it may actually serve to encourage bad practices rather than good ones.

My first recommendation concerns the item: "Prefer nested context classes over inherited classes". While I don't necessarily disagree with the statement in and of itself, I would submit that both techniques are to be avoided. So yes, nested contexts would be preferable to inheritance, but I would submit that we should prefer repeating shared context over either the use of nested contexts or inherited classes. I do agree with the warning that too much use of nested contexts can be confusing (i.e. leads to test obscurity), but I don't think the warning goes far enough to discourage the practice.

The second recommendation concerns the absence of any recommendations against the use of behaviors. For the same reasons as the first recommendation, use of behaviors should be discouraged. As I'm sure you are aware, Aaron Jensen's opinion was that this feature should never have been included. The fact that the document calls out best practices around naming Behaviors kind of legitimizes their use. It would be best to have an explicit recommendation to not use behaviors and perhaps call out that the are maintained solely for backward compatibility.

The third recommendation concerns the absence of a recommendation for Because to be a single statement. Context/Specification is about performing a single action for an established context, so it's arguably more important for the Because to be a single statement than for the It in terms of adherence to the spirit of context/specification (although I agree that both should be a single line).

The forth recommendation concerns the item: "Put lambda expressions on new line". This feels more like a personal preference that sneaked in as a "best practice" rather than being reflective of some general consensus within the mspec user community. For expressions that have no need to wrap, I would argue that it's actually more readable to keep both Because, It, and Cleanup expressions on the same line. In the event that the statements are lengthy, you'd certainly want to place them on a separate line, but I feel like the guidance for both is more of a general coding recommendation rather than anything specific to Mspec.

[robertcoltheart]

Thanks for the feedback, this is very constructive. I'll try and address your 4 points below:

Prefer nested context classes over inherited classes

Understand what you are saying, I'll strengthen the recommendation to use caution when nesting or inheriting.

Use of behaviors

Yep, totally agree with you. We don't use behaviors at all in my workplace where we can help it, and I think they should be deprecated in an upcoming major release. This is an oversight on my part, I'll address the documentation to discourage their use. I'd be interested to hear any ideas you had around a substitute that helps reduce repeated It clauses? Allowing It specs in abstract classes and using inherited classes is obviously one approach, as is an Examples feature as outlined here: https://github.com/machine/machine.specifications/issues/141.

Because to be a single statement

100% agree, I'll add that in.

Put lambda expressions on new line

I agree there is an element of preference here, so I'm happy to soften the language and simply recommend the format, since it aligns with (most) of the tests in the MSpec repo. However it's my opinion that the advice stands because:

1. It clauses tend to be quite verbose and usually do not fit on one line, necessitating a line-break. To keep the classes consistent it makes sense to line-break all It, Because and Establish clauses, which helps guide the developers eyes when reading the specs.

2. MSpec has a very unique style of coding and I feel expression-bodied clauses on a new line add to the uniqueness of the framework.

3. A non-exhaustive search over some open-source repos on GitHub shows many people using a similar style.

[robertcoltheart]

I'll close this for now, as I'll assume the above addresses your issue.