Kata

Unfortunately, Crate needs to be redefined in the testing code to be inaccessable to the solver, so you only have access to the definition from preloaded. namedtuples can still be compared with __eq__ from two definitions, but I'm not sure if there's a way to compare them with is or with match statements.

It's an unfortunate effect from a poor choice made early on to use namedtuples, but I'm not sure what difference it makes when solving. There are no invalid inputs and you can still match on crate.color perfectly fine.

the test code could likewise read out the data and compare that instead of insisting on its own separate definition. could also get fancy and render the states like the drawings in the instuctions. or, only show the preloaded one to the user and then compare with a separately defined type for the expected value. codewars_test does do actual == expected though which is arguably wrong and it should instead do expected == actual

Thanks. Other than that, great kata.

There're no performance requirements in this kata, and your solution passes the tests in ~100 ms. What are you talking about?

im pretty sure there is 12k ms and deep recursion test is over 10k moves

same issue

10k is a very small number of operations - a reasonable solution can pass the "deep recursion" test in ~10 ms. If your solution times out, either you aren't using the max_steps limiter, or your code is genuinely 1200+ times slower than what other people have written which is unacceptably bad.

provide information or just close this issue. Thx

Provide info about what? How to choose a meaningful data representation? Which representation is fitting in this particular case? How to get rid of the delusion that "OOP is used to model real-life objects"?

Which representation is fitting in this particular case?

that would be enough

With the current setup Crate is a str (or namedtuple if the author really wants it to imitate a separate class) and Command is clearly a tuple/namedtuple.

I'll be honest, I'm not very familiar with namedtuples, but from reading, it does seem like a better choice. I originally went with an OOP aproach before deciding exactly how the Kata would work, and looking back see why it may not be a good choice.

I have some time today, so I'll give a shot to using them.

Thanks for the feedback!

Alright, I've replaced the Crates and Commands with namedtuple objects. Let me know if there are any errors in my attempt or any other issues.

Thanks!

And why are you inheriting from the namedtuple-classes instead of using them directly? This is pointless, and you're overriding __repr__ incorrectly anyway.

I was inheriting from them in order to override __repr__. Is there a better way to do this? Or would it be best to just leave the default __repr__ as is?

And why are you inheriting from the namedtuple-classes instead of using them directly?

I was inheriting from them in order to override __repr__

That was a rhetorical question... And you don't need to inherit from a class to change its __repr__ method.

Or would it be best to just leave the default __repr__ as is?

Do you know how __repr__ (and __str__) should be used at all? As I see, you don't, so you really should read the documentation.

Alright, I've removed the unnecessary inheritance. Let me know if there's anything else I should change. Thanks!

Awesome, thanks for the feedback.

Thanks for all the helpful feedback!

I've implemented your latter three points. As for the first one, I fixed the __eq__ method to account for non-crate objects, but I had a couple questions as to the rest:

• I'm not sure of the protocol for documenting preloaded in the description. Is it best to just include everything even if it's not necessarily relevant to solving? (e.g. __eq__ and __repr__) My concern was overcomplicating the description, but I can see how leaving it out could lead to confusion.
• For reimplementing the Crate and Command classes in the test suite, am I correct that this would invalidate any solution that attempted to construct its own crates? I wouldn't expect this to be a common approach, but I do see one current solution (here line 30) that makes use of this.

About documentation, you can just say what methods are defined for each class without giving the code (as long as it's working in all situations ;o ).

About redefining the classes: the users will use the classes in preloaded, that will have the exact same interface, hence behaviors, than those you'll copy/paste in the tests. So that shouldn't cause troubles as long as the users don't try to add methods on the fly or stuff like that (might be worth of adding a note about that, tho / about that solution, as far as I can see, they didn't touch to your classes and built a State class that is only relying on the data held by your classes, and not their "implementation").

Alright, I've updated the description and copied the classes to the test suite.

Let me know if there's anything else amiss. Thanks again for your help!

your __eq__ implementation still isn't correct. Use this instead:

return isinstance(other, Crate) and self.color == other.color

Oh, and you need to add the __hash__ method, since you provide __eq__ (that shouldn't be useful, but just so that the objects behaves properly if an user tries to build sets of dicts):

def __hash__(self):
    return hash(self.color)
    

Other than that, the kata looks ok.

That's the implementation I had for __eq__ at first, but it was causing a problem with a solution which constructed its own Crate object using the class in preloaded and compared it to a Crate object constructed in the test suite.

I took FArekkusu's note and switched to namedtuple objects. Am I correct in assuming that, as tuples, they should be hashable and comparable by default?

yes, namedtuples should resolve all the problems at once. But as FArekkusu pointed out, you're currently using them the wrong way ;)

It's a fun kata, and you've put alot of effort in the description and test cases. Well done :)