5 kyu

Factoradic class

29catechni
Description
Loading description...
Algorithms
Mathematics
View
AllIssues4Questions1Suggestions1Show Resolved
  • Please sign in or sign up to leave a comment.
  • Voile Avatar
     
    test.assert_equals(str(Factoradic('1000000') / Factoradic('310')), "20200", "test 10")

    This is an incorrect use of Python operators: / is always floating point division (truediv), while // is always integer division (floordiv). So there are no reasons to use / instead of // here.

    • catechni Avatar

      The kata description calls for the class to be able to handle simple division using '/', so it absolutely should test using that symbol. Depending on how the class is implemented, using '//' could cause an issue for someone following the specification.

      An int / int returns a float, but a Factoradic / Factoradic should return a Factoradic.

      How the class should handle a non-integer result is not specified (round-up, round-down, fractional factoradics?), but all the tests divide exactly.

      This issue has been flagged (and marked resolved) before, so I am hesitant to just mark it resolved, in case it is raised again and again. I am of the thinking that a note on the description on how it should handle division would help, but I am open to suggestions.

    • Blind4Basics Avatar

      I think Voile is right. Especially considering the specs call for a "replacement for ints", sort of, so the behavior should be consistent when compared to ints. One way out of this:

      • specify that only integer arithmetic will be handled
      • use only // in the tests
      • make sure to test it with non divisible values
    • Voile Avatar

      but a Factoradic / Factoradic should return a Factoradic.

      Yes, but factorial base can also be used to express non-integral value like decimal base. There was nothing stopping one from supporting non-integral values for Factoradic.

      By using / you're implying much more work for us to do than only supporting //.

    • catechni Avatar

      This comment has been hidden.

    • catechni Avatar

      Switched / to //

      Issue marked resolved by catechni 2 years ago
  • Blind4Basics Avatar

    This comment has been hidden.

  • natan Avatar

    There are two tasks here. The representation conversion, and then the class boilerplate. Would this be better as only the conversion part?

    • Blind4Basics Avatar

      would be a duplicate, then

    • natan Avatar

      Then I'll argue this is already a duplicate, as all the operations here can be performed with int, and the rest is conversion.

    • Blind4Basics Avatar

      yesn't. It's not that simple. See my first issue below

    • Fbasham Avatar

      Codewars needs to be more explicit about what is considered a duplicate. The idea of conversion is certainly a duplicate, but this kata is not by definition a duplicate.

    • Blind4Basics Avatar

      the factoradic conversion has already been covered (back and forth). It's not just an idea.

    • Fbasham Avatar

      I'm guessing you're referring to this one? My point stands though. Do the docs even contain a section on the issue of duplicate katas or what the definition of a duplicate is? Last time I checked they didn't and it seems like more of an unwritten rule that new kata creators don't know about until it's too late and they're downvoted.

      EDIT: I guess the docs do:

      "Avoid creating blatant duplicates, and use the idea which has not yet been widely used to create a kata."

      But what is a 'blatant duplicate'? I'm a native english speaker, and to me, this would only mean an exact copy of the original. Also, "an idea that hasn't been widely used". To me, one kata outside of beta doesn't constitue "widely used".

    • Blind4Basics Avatar

      I don't have an answer for you sorry. Part of the problem is: all this is community driven, and nobody agrees, so no clear definition will arise (it's actually the exact same problem with the rank creep thing)

    • catechni Avatar

      I didn't see that kata, when looking before submission. In fact, I couldn't find it without your link.

      I wouldn't say it was a blatant duplicate since it's not immediately obvoius that they are the same problem from the description alone.

      (And also since blatant implies deliberate and intentional - which it wasn't. I solved it myself after reading xkcd and thought it would make a good kata).

    • Fbasham Avatar

      Seeing as how there isn't a consensus on whether this is a duplicate, I think the voters will have to decide on the fate of this kata (if you're going to re-publish it OP).

    • dfhwze Avatar

      Is there specific arithmetic available on factoradic numbers without converting to int first? And can this be optimised for huge numbers, so its much faster than dealing with huge integers? If this is the case, this kata could be turned into a performance version that would time out solutions that use int for underlying operations. If not, this is a "blatant" duplicate imo.

    • Blind4Basics Avatar

      imo that would be the worst thing to do because the task is about building a class, ie an interface, rather than a highly specific-hyper-veloce-one-job-function. Moreover, we're talking about python, so any hand written logic will probably be solwer than an approach making extensive use of builtins. In the end, that would just turn the kata into a micro-optimization nightmare.

  • mauro-1 Avatar
     
    class Factoradic()

    should be

    class Factoradic:
  • mauro-1 Avatar

    Shouldn't it be 

    8  ->  110

    ?

  • Blind4Basics Avatar

    oh, btw, the initial solution doesn't compile (syntax error)

  • Blind4Basics Avatar

    Hi,

    • all the example tests should be present at the beginning of the full test suite
    • missing fixed test where a negative value is passed as string
    • the tests use true division while integer results are expected. This must be:
      • either specified
      • or changed in the tests to only use floor div
    • missing fixed tests with double negations (my solution is wrong about that)

    I have very mixed feelings about the general task. We already have kata about factoriadics, so the conversion part is already covered. What's left is the class part, which is not really interesting, except maybe if one goes "full meta" (there are some quirck to handle there), problem being:

    • not all methods are covered
    • adding more method makes the kata more annoying rather than more interesting (resp. "more tedious rather..." if "full meta" approach)

    I don't really know what to suggest here...

    Cheers

    • catechni Avatar

      I didn't see any the other katas. The conversion is, as you say, the main part. I didn't want it to get too tedious - hence not going full meta - but the basic arithmetic was a petty easy add. Fixed the tests.

    • catechni Avatar

      Not sure what you mean about division. The kata specifies that simple division should be possible, and tests simple division. All the tests have integer results (because fractions are undefined), so it's not like anyone is being caught out here.

    • Blind4Basics Avatar

      yeah, I guess you can scratch that part out (I got into some troubles because I had messed the neg part, at some point).

    • catechni Avatar

      Marking as resolved

      Issue marked resolved by catechni 2 years ago