kesac / Syllabore

A fantasy name generator that does not use pre-made lists of names
MIT License
40 stars 2 forks source link

NameFilterTests refactor #11

Closed monerofglory closed 1 year ago

monerofglory commented 1 year ago

What is changing?

Why does the change need to happen?

Other ways to improve:

Issue 1

Issue 2

Look into the AAA structure for setting up unit tests. It stands for Arrange, Act, Assert and can be used to make your testing much easier to read and structure.

E.g:

// Arrange
sut.UsingFilter(x => x.DoNotAllow("a"));

// Act
var result = sut.Next()

// Assert
Assert.NotNull(result)

As always, feel free to reach out if you want me to go through any of these. Thanks for reviewing!

kesac commented 1 year ago

Thanks for taking the initiative in getting the project to adopt best test practices!

Looks good to me and I will read up on the AAA structure as you recommend.

For issue 1, the call to UsingFilter() sets and also replaces the filter of the generator.

So this setup:

generator.UsingFilter(x => x.DoNotAllow("a"));
generator.UsingFilter(x => x.DoNotAllow("e"));

would be better expressed as:

generator.UsingFilter(x => x
    .DoNotAllow("a")
    .DoNotAllow("e"));

I will make sure this clearer in the XML documention