Tests should not use Assert.True and Assert.False: it doesn't show what the original expected and actual values are (all they give is "this flag is not expected").
Resource is implicitly assumed to be distinguished by Name, but there are nothing in the code that assures this (e.g Equals and GetHashCode are not overridden).
Certain operations have success/failure semantics but are no explained, e.g Quiver.AddArrows can fail, but what does it mean by a failure? What happens in this case? Do we not add any arrows, or only add as much as possible?
These cases are never explained, and tests never touch them because they only consists of shallow tests of the simplest scenarios.
Certain designs in the kata that makes no sense. e.g Why IHunter doesn't have a backpack if it is a core function of a hunter to have backpack to store things? How does one even expect Backpack-related methods on IHunter to work?
These are for some reasons implemented onto instances of IHunter instead, which makes no sense whatsoever. If an IHunter is supposed to have a backpack, then it should have this property in the interface. If it doesn't have one, then Aloy should implement from a separate interface IHasBackpack, IArcher or something that provides said hunter methods of interacting with a backpack.
There are a lot of errors in the initial code, such as
src/Solution.cs(305,23): error CS0508: 'BodyPart.AddComponent(BodyComponent)': return type must be 'void' to match overridden member 'BodyComponent.AddComponent(BodyComponent)'
src/Solution.cs(329,23): error CS0508: 'BodyRemovableComponent.AddComponent(BodyComponent)': return type must be 'void' to match overridden member 'BodyComponent.AddComponent(BodyComponent)'
And some errors from the sample test code:
tests/Fixture.cs(145,27): error CS0103: The name 'GetResources' does not exist in the current context
This is not tested.
There are no random tests.
Tests should not use
Assert.True
andAssert.False
: it doesn't show what the original expected and actual values are (all they give is "this flag is not expected").Resource
is implicitly assumed to be distinguished byName
, but there are nothing in the code that assures this (e.gEquals
andGetHashCode
are not overridden).This applies to every other class as well.
Certain operations have success/failure semantics but are no explained, e.g
Quiver.AddArrows
can fail, but what does it mean by a failure? What happens in this case? Do we not add any arrows, or only add as much as possible?These cases are never explained, and tests never touch them because they only consists of shallow tests of the simplest scenarios.
Certain designs in the kata that makes no sense. e.g Why
IHunter
doesn't have a backpack if it is a core function of a hunter to have backpack to store things? How does one even expectBackpack
-related methods onIHunter
to work?These are for some reasons implemented onto instances of
IHunter
instead, which makes no sense whatsoever. If anIHunter
is supposed to have a backpack, then it should have this property in the interface. If it doesn't have one, thenAloy
should implement from a separate interfaceIHasBackpack, IArcher
or something that provides said hunter methods of interacting with a backpack.There are a lot of errors in the initial code, such as
And some errors from the sample test code:
Required
using
s in initial code are also missing.