Ad
  • Custom User Avatar
  • Custom User Avatar

    The type hints in the Python setup code are misleading. Since None is a valid argument for the node argument in both functions, the type hint should be node: Node | None instead of node: Node.

    Also, since you're using type hints, you could make the Node class generic:

    from typing import Generic, TypeVar
    
    T = TypeVar('T')
    
    class Node(Generic[T]):
        @property
        def data(self) -> T: ...
        @property
        def next(self) -> 'Node[T] | None': ...
    

    Then count could look like the following:

    def count(node: Node[T] | None, data: T) -> int: ...
    
  • Custom User Avatar

    done

    • added explicit imports in initial code and tests
    • commented out the Node class in the initial code
    • added type hints in the initial code
    • moved utility functions out of preloaded
  • Custom User Avatar

    The tests in Python do not use the definition of Node from the solution. The class definition should either be removed or commented out of the setup code or be tested in the test suite.

  • Custom User Avatar

    BTW are you on Codewars Discord? I wanted to ask one thing, could you please poke me with a DM if you are there?

  • Custom User Avatar
  • Custom User Avatar

    thanks for the explanation. in hindsight it was foolish of me, of course the hash code should change if data is mutated. i changed GetHashCode() to simply return Data.GetHashCode();.

  • Custom User Avatar

    The contract of GetHashCode is based on two rules:

    • Two instances which are equal must produce the same hash code,
    • Two instances which are not equal should, but do not necessarily have to, produce different hash codes.

    The above rules enable a couple of possible patterns of implementing sane GetHashCode:

    • use the same data in GetHashCode and Equals,
    • in GetHashCode use a subset of data used in Equals,
    • ... a couple of others...

    Your implemntation of GetHashCode is good in a way that it conforms to above rules: it uses Data, which is also used by Equals. If Data of two elements will be different, Equals will return false, and also hash codes will be different, which is good (it might be not great for other reasons like performance, but it is at least not against rules).

    The "hash code should stay constant for the lifetime of the object" is something different, it's a pattern/rule not enforced by GetHashCode itself, but an advice which helps to avoid some nasty bugs. Generally it would translate to: "avoid hash codes calculated from mutable fields". However in case of this challenge and the Node class, it is not possible: the Data field is mutable, and it's a part of equality. If Data changes, then its equality with other instances might also change. This is why precalculated and cached hash code becomes wrong when Data changes.

    For the Node class, this means that one possible implementation for GetHashCode is return this.ToString().GetHashCode() (uses the same data as Equals, but not great performance wise), another is return Data.GetHashCode() (easy to calculate, might have poor distribution but should be good enough for a kata), but should not be cached because it will be wrong when Data changes.

  • Custom User Avatar

    C# warning in the preloaded section:

    src/Preloaded.cs(5,22): warning CS0659: 'Node' overrides Object.Equals(object o) but does not override Object.GetHashCode()
    
  • Custom User Avatar

    (OP was solving in JavaScript)

    your implementation of count() has a bug, it skips the last element of the list because your while(head.next) is not entered for the penultimate element. not a kata issue.

  • Custom User Avatar

    I added the following GetHashCode() implementation in preloaded, which cleared the warning. I don't know whether it is idiomatic enough though, I read that the hash code should stay constant for the lifetime of the object, which is not guaranteed here if we use the Node fields for the hashcode computation, as they are mutable. I tried to work around that.

    private int? _hashCode = null;
    public override int GetHashCode()
    {
    // should remain constant for the lifetime of the object
      if (_hashCode == null)
          _hashCode = Data.GetHashCode();
      return _hashCode.Value;
    }
    
  • Custom User Avatar
  • Custom User Avatar

    Same issue here, I wonder why it is still in draft...

  • Custom User Avatar

    Additionally: Node class is buggy because it ovrrides Equals but not GetHashCode.

  • Custom User Avatar

    C# test suite generates a warning, and the preloaded Node class is missing GetHashCode.

  • Loading more items...