Ad
  • Custom User Avatar

    There are no actual random tests: the ones testing the values all have fixed params, and the only random ones are testing property getter/setters and never actually runs the scheme even once.

  • Custom User Avatar

    IMHO the fact that the tests (which are not random) failed with this exact message AttributeError: 'PokemonType' object has no attribute 'name' in case name was not implemented clearly mean that the thing is tested for, and this should be marked as a Suggestion, not an issue. Btw I now have added 2 more explicit test cases.

  • Default User Avatar

    Oh yeah duh, I didn't even think how dict recognizes keys in python. xD

    I even used it myself to use list as keys sometimes. x)

  • Custom User Avatar

    This comment is hidden because it contains spoiler information about the solution

  • Custom User Avatar

    It is not tested that the instance attribute is really defined as name, only accidentally (because part of random tests explode if it isn't)

  • Custom User Avatar
  • Custom User Avatar
    1. The constant is labeled ATTACK_MULTIPLIERS so it is consistent that it contains details from an attacking POV. Flying attacks Ground with 1x damage (nothing to specify), while Ground attacks Flying with a 2x damage. This is not always the case that if Type_A does 2x dmg Type_B then Type_B does 0.5x dmg Type_A.
    2. Ok, I didn't want to argue on the fact that test should be done properly, but that I've found others approved kata with just 3 or 4 lines of tests (on the samples) and I really appreciated them more because of the freedom and accessibility they provided than formally long tests samples not so easy to edit. Thus IMO, inside the test sample is a proper use (or at least a permitted one, since there are katas with high satisfaction votes out there that use just test.expect something) of the testing suite
    3. I think I still don't get it: in the full test suite all the tests are implemented with decorators and everything, eg
    @test.describe("Configuration Checks")
    def tests():
        @test.it("All types are correctly initialized")
        def it():
    
    1. So people do actually prefer to get their mind-cpu busy assuming instead of just trying to press submit and see what happens? (not a salty question) I personally always try test AND submit with the real first attempt I do to solve a kata (or to have info without bothering to look back at the description). Btw I've integrated your suggestions on this (and the related point 2)
    2. Ok I see, few additional words in the description can be worth to be integrated to explain that detail.
    get(name, default = None): Get a single type instance
    -->
    get(name, default = None): Get a single type instance. If no instance is found returns either 'None' or whatever the consumer has provided via the optional param 'default'.
  • Custom User Avatar
    1. oops, right, copy/paste mistake, sorry. Cannot get my hand on the original assertion (I changed my code in the meantime), but basically, the dict is "directed" (ex: flying doesn't contain info about ground, but the opposite is true). So my point stil stands: the initial data structure is not consistent.
    2. This has nothing to do with readability. Writing the tests without proper use of test framework is just a "no" for the approval of the kata. And as the author of the kata you are responsible for writing it properly. Otherwise, there is no point in authoring kata.
    3. the stroke is definitely there, because point 3 refers to point 2 which refers to the use of the test framework (which, in present context, refers to the use of @test.desribe & @test.it decorator. I realize now you mixed this up with the assertion thing (which is also a problem, but not the same. My mistake again).
    4. ok, I see. Please note, tho:
    • writing the tests in different fahsion (full test suite vs example tests) has a cost in maintainability => when possible, the example tests should just be a copy paste of some initial part of the full test suite.
    • if the user sees test.expect in the example tests, they will assume the invisible tests will be written that way. That doesn't encourage in thinking they'll have a good experience with the kata.
    1. You didn't understand what I meant. I was talking about the behaviours related to the presence or not of a value for the default argument. there you only talked about implementation details, which are effectively already visible in the code. The problem is more about:
    • why None as default instead of, for example, "Normal"?
    • what is the user supposed to do when they get None as default?
      • throw an error?
      • use something else?
      • is it guaraanteed to never happen? (in that case, it must be specified and it would be better to not use None as default value)
  • Custom User Avatar

    Hi, this time (to answer the "...again") I don't think you nail any real point tbh.

    1. the relevant part you provide shows details for 'Flying' class when you comment a test that expects 'Ghost' to be immune to 'Normal'. So, to answer you, yes this line is present in the dictionary: "Normal": {"Ghost": 0} i.e. "Normal" does 0 damage to "Ghost".
    2. Sample tests were really readable, concise and I didn't feel it needed extra verbosity because the users can add it if they feel they need it.
    3. You should remove the stroke from the "I guess" part because there isn't (and wasn't) any test.expect nor test.assert_equals without error description in the internal test case. If you found some other type of misleading error description please provide me something to watch for and I'll fix it
    4. As said at point 2, imho test.expect(PT.get("Ghost").is_immune_to("Normal")) in the sample tests where users can add/comment out/edit as they need to (if they actually experience errors) is perfectly fine, readable and clear. I also don't think test.assert_equals was properly used, as per your observation, since it missed the verbose description. I've still added message description for every error (both expect and assert_equals) in the example test case, althought they make the test case code box less clean and more annoying to edit for the users imo, but whatever.
    5. The DefaultType was declared this way DefaultType = TypeVar("DefaultType") meaning 'DefaultType is a generic type with name "DefaultType"'. I've still changed it to DefaultType = TypeVar("T") to be even more clear that it's just a generic type used to specify that the return type of get should be either PokemonType or any type the default param is. I think that what's on my mind is also on the initial solution, so it doesn't really need to be verbosely explained. Do you really think I should explain this inside the description?

    PS I still wait for your response to mark the issue as solved

  • Custom User Avatar

    Nice catch, pretty coherent the bugs were in the Bug isn't it?
    Fixed, thank you. I have also double checked others and didn't find any additional error.

  • Custom User Avatar

    Hi,

    The dictionary contains all the needed data for the interactions between types

    Are you sure about that...?

    test.expect(PT.get("Ghost").is_immune_to("Normal"))
    ('Flying', {'Fight': 2, 'Rock': 0.5, 'Bug': 2, 'Steel': 0.5, 'Grass': 2, 'Electric': 0.5})
    => no "Normal" in there while the default multiplier leads to "Not resistant" instead of "Immune"
    

    • sample tests are not using the test framework properly (...again)
    • I guess it's the same with the full test suite?
    • DON'T use test.expect unless you provide a useful error message. Tip: just use test.assert_equals...
    • the rules about the DefaultType are never explained anywhere (that's full specs. The user shouldn't have to guess what's on your mind)

    Cheers

  • Custom User Avatar

    This kata requires some domain knowledge, but the final equations are relatively straightforward -- not unlike the various math and music theory katas on Codewars.

    The error handling, especially the careful treatment around the implosion boundary, justifies a rating around 5kyu, IMHO. I rated this as 4kyu as, per Codewars guidelines, it involves Understanding intricate business requirements.

  • Custom User Avatar

    I'm pretty sure your Bug/Fairy multipliers are wrong. I'm doing my own implmentation of the multiplier and only failing on Bug/Fairy comparisons. Check to see if your're missing something or have incorrect values.

    Also: Bug/Dark.

  • Custom User Avatar

    This comment is hidden because it contains spoiler information about the solution

  • Custom User Avatar

    I think the current approach is a reasonable tradeoff. Applying a "promised rate of return" to a monetary value always entails an accounting convention to deal with rounding, which is currently neatly encapsulated by the "black box" functions.

    In theory, floats also introduce a possible loss of precision, such as int(12345678987654321 * 1.0) == 12345678987654320, although this would require an astronomically large Ponzi scheme to be relevant. Still, I agree with hobovsky that the truly "best practices" implementation would eschew floats altogether, e.g., by providing the rates as basis points, and assuming a downwards-rounding convention.

  • Loading more items...