Splitting methods in separate files

[eNeRGy164]

Just a suggestion.

As I am currently working in TypeAssertions.cs (1500+ lines) and TypeAssertionSpecs.cs (5500+ lines) these files are huge.

Would it be an idea to split these files in separate files for maintainability?
Making the static class partial should not change the public signature.

I do this for my projects that contain large amounts of extension methods, f.e. PlantUml.Builder/StringBuilderExtensions, PlantUml.Builder.Tests/StringBuilderExtensions.

Depending on the file structure I have a dedicated folder per class, or I name my files like StringBuilderExtensions.SkinParameter.cs containing all the .SkinParameter(...) methods and overloads.

Some of the benefits:

• Smaller files
• With changes, the filename gives context (a change in TypeAssertions.cs does not reflect which of the 55! methods is modified, a change in TypeAssertions.HaveProperty.cs does give it away)
• You can have methods open side-by-side in the IDE
• Comparisons/changes are easier shown (f.e. in GitHub)
• No regions needed anymore

[jnyrup]

I'm very positive.
It would also make history search easier with the fewer changes per file.

For our unit tests I've also thought about the approach described in http://haacked.com/archive/2012/01/02/structuring-unit-tests.aspx/ where each method under test has it's own subclass.
With the functional naming style we practice I sometimes have to get "creative" with the method naming to avoid conflicts.
E.g. When_passing_in_a_null_sequence_it_should_throw could then be re-used for many tests asserting on parameter validation.

[dennisdoomen]

I'm not very fond of partial files for the production code, especially because I always use Rider's Structure Window to see where a new method should go, or use it for reordering. However, I do acknowledge the problem with our test files. There it may make sense to group them in a better way or use partial classes.

[jnyrup]

Okay, let's try it out with test classes for now.

Here are some suggestions for the structure

Types\
  TypeAssertionsSpecs.cs
  TypeAssertionsSpecs.Be.cs (both Be and NotBe in here)

2. Affirmative and negative assertions in separate files

Types\
  TypeAssertionsSpecs.cs
  TypeAssertionsSpecs.Be.cs
  TypeAssertionsSpecs.NotBe.cs

3. Subfolder per class.
   E.g. GenericCollectionAssertions will have 44 different partial classes

Types\
  TypeAssertions\
    TypeAssertionsSpecs.cs
    TypeAssertionsSpecs.Be.cs
    TypeAssertionsSpecs.NotBe.cs

I would vote against simply naming them Be.cs as that would give us 18 classes with that name, which would harm the "goto file" experience in any IDE.

[dennisdoomen]

Haha. Well, that's why I think option 1 is good enough. It keeps the positive and negative tests together, but still splits them by API.

[eNeRGy164]

I'd love to pick it up, but maybe my current PR is already getting a bit out of scope i think 😄

[eNeRGy164]

I would also expect the generic versions to be combined with the non generics as well for now. Although '`1' is allowed as file name.

[dennisdoomen]

I'd love to pick it up, but maybe my current PR is already getting a bit out of scope i think

Nothing stops you from creating more PRs ;-)

[eNeRGy164]

Well, not if it's splitting up the file that is in my other PR. I will happily wait till that one is merged.

[eNeRGy164]

Funny, I see DataEquivalencySpecs is already doing this

[dennisdoomen]

Yeah, that was an external contributor. Since we didn't align on our "convention" yet, we accepted it.

[dennisdoomen]

So did we settle on option 1 @jnyrup @eNeRGy164 ?

[eNeRGy164]

Yes, option 1

See:

• https://github.com/fluentassertions/fluentassertions/tree/master/Tests/FluentAssertions.Specs/Types
• https://github.com/fluentassertions/fluentassertions/tree/master/Tests/FluentAssertions.Specs/Collections

[jnyrup]

yes

[dennisdoomen]

Then we also need to think of a way to improve the equivalency specs where all of them use the same method, BeEquivalentTo. We currently use regions, but that doesn't help in the test explorer. I've tried using a nested class, e.g. Mapping and that seems to work well in Rider.

[jnyrup]

Using nested classes also helps reducing the headache of duplicate method names.