Raindrops in PythonReview my prime factorization function. Is it complete?Project Euler #12 - highly divisible triangular numberProject Euler #3 (Largest prime factor) in SwiftHackerEarth Girlfriend's Demand challenge, solved two waysRaindrops in JavaPrimes & SquaresCircular array rotation JavaFirstDuplicate Finder(Leetcode) Sliding puzzle in PythonPDF page & word count, recursive searching of directory tree, output to excel

What is the source of this clause, often used to mark the completion of something?

Why did Windows 95 crash the whole system but newer Windows only crashed programs?

Why are we moving in circles with a tandem kayak?

How close to the Sun would you have to be to hear it?

What is my clock telling me to do?

Embedded C - Most elegant way to insert a delay

Move arrows along a contour

How to have poached eggs in "sphere form"?

Is Ear Protection Necessary For General Aviation Airplanes?

How does Asimov's second law deal with contradictory orders from different people?

Why tantalum for the Hayabusa bullets?

Would people understand me speaking German all over Europe?

Why are subdominants unstable?

Why don't short runways use ramps for takeoff?

Is it unprofessional to mention your cover letter and resume are best viewed in Chrome?

What clothes would flying-people wear?

Can living where Earth magnet ore is abundent provide any protection?

Should I intervene when a colleague in a different department makes students run laps as part of their grade?

Why did I lose on time with 3 pawns vs Knight. Shouldn't it be a draw?

What language is Raven using for her attack in the new 52?

My employer is refusing to give me the pay that was advertised after an internal job move

Why put copper in between battery contacts and clamps?

when to use "wait" and when "busy" mouse cursor

Can a US President, after impeachment and removal, be re-elected or re-appointed?



Raindrops in Python


Review my prime factorization function. Is it complete?Project Euler #12 - highly divisible triangular numberProject Euler #3 (Largest prime factor) in SwiftHackerEarth Girlfriend's Demand challenge, solved two waysRaindrops in JavaPrimes & SquaresCircular array rotation JavaFirstDuplicate Finder(Leetcode) Sliding puzzle in PythonPDF page & word count, recursive searching of directory tree, output to excel






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








4












$begingroup$


There's this simple coding challenge that feels like a twist on FizzBuzz. Your task is to convert a number to a string, the contents of which depends on the number's factors.




  • If the number has 3 as a factor, output 'Pling'.

  • If the number has 5 as a factor, output 'Plang'.

  • If the number has 7 as a factor, output 'Plong'.

  • If the number does not have 3, 5, or 7 as a factor, just pass the number's digits straight through.



For example, the sample output would look like this:




28's factors are 1, 2, 4, 7, 14, 28. In raindrop-speak, this would be
a simple "Plong".



30's factors are 1, 2, 3, 5, 6, 10, 15, 30. In
raindrop-speak, this would be a "PlingPlang".




I've come up with a working solution (based on the test suite results). However, I'm not really happy with it, as I feel this could be simplified. Thus, I'd appreciate some feedback.



My solution:



SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"
FACTORS = (3, 5, 7)

def is_divisible_by(number, divisior):
return number % divisior == 0


def raidndrops(number):
return [SOUNDS[factor] for factor in FACTORS if is_divisible_by(number, factor)]


def convert(number):
speak = raidndrops(number)
return "".join(speak) if speak else str(number)


The test suite is here:



import unittest

from raindrops import convert

class RaindropsTest(unittest.TestCase):
def test_the_sound_for_1_is_1(self):
self.assertEqual(convert(1), "1")

def test_the_sound_for_3_is_pling(self):
self.assertEqual(convert(3), "Pling")

def test_the_sound_for_5_is_plang(self):
self.assertEqual(convert(5), "Plang")

def test_the_sound_for_7_is_plong(self):
self.assertEqual(convert(7), "Plong")

def test_the_sound_for_6_is_pling(self):
self.assertEqual(convert(6), "Pling")

def test_2_to_the_power_3_does_not_make_sound(self):
self.assertEqual(convert(8), "8")

def test_the_sound_for_9_is_pling(self):
self.assertEqual(convert(9), "Pling")

def test_the_sound_for_10_is_plang(self):
self.assertEqual(convert(10), "Plang")

def test_the_sound_for_14_is_plong(self):
self.assertEqual(convert(14), "Plong")

def test_the_sound_for_15_is_plingplang(self):
self.assertEqual(convert(15), "PlingPlang")

def test_the_sound_for_21_is_plingplong(self):
self.assertEqual(convert(21), "PlingPlong")

def test_the_sound_for_25_is_plang(self):
self.assertEqual(convert(25), "Plang")

def test_the_sound_for_27_is_pling(self):
self.assertEqual(convert(27), "Pling")

def test_the_sound_for_35_is_plangplong(self):
self.assertEqual(convert(35), "PlangPlong")

def test_the_sound_for_49_is_plong(self):
self.assertEqual(convert(49), "Plong")

def test_the_sound_for_52_is_52(self):
self.assertEqual(convert(52), "52")

def test_the_sound_for_105_is_plingplangplong(self):
self.assertEqual(convert(105), "PlingPlangPlong")

def test_the_sound_for_12121_is_12121(self):
self.assertEqual(convert(12121), "12121")


if __name__ == '__main__':
unittest.main()









share|improve this question











$endgroup$




















    4












    $begingroup$


    There's this simple coding challenge that feels like a twist on FizzBuzz. Your task is to convert a number to a string, the contents of which depends on the number's factors.




    • If the number has 3 as a factor, output 'Pling'.

    • If the number has 5 as a factor, output 'Plang'.

    • If the number has 7 as a factor, output 'Plong'.

    • If the number does not have 3, 5, or 7 as a factor, just pass the number's digits straight through.



    For example, the sample output would look like this:




    28's factors are 1, 2, 4, 7, 14, 28. In raindrop-speak, this would be
    a simple "Plong".



    30's factors are 1, 2, 3, 5, 6, 10, 15, 30. In
    raindrop-speak, this would be a "PlingPlang".




    I've come up with a working solution (based on the test suite results). However, I'm not really happy with it, as I feel this could be simplified. Thus, I'd appreciate some feedback.



    My solution:



    SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"
    FACTORS = (3, 5, 7)

    def is_divisible_by(number, divisior):
    return number % divisior == 0


    def raidndrops(number):
    return [SOUNDS[factor] for factor in FACTORS if is_divisible_by(number, factor)]


    def convert(number):
    speak = raidndrops(number)
    return "".join(speak) if speak else str(number)


    The test suite is here:



    import unittest

    from raindrops import convert

    class RaindropsTest(unittest.TestCase):
    def test_the_sound_for_1_is_1(self):
    self.assertEqual(convert(1), "1")

    def test_the_sound_for_3_is_pling(self):
    self.assertEqual(convert(3), "Pling")

    def test_the_sound_for_5_is_plang(self):
    self.assertEqual(convert(5), "Plang")

    def test_the_sound_for_7_is_plong(self):
    self.assertEqual(convert(7), "Plong")

    def test_the_sound_for_6_is_pling(self):
    self.assertEqual(convert(6), "Pling")

    def test_2_to_the_power_3_does_not_make_sound(self):
    self.assertEqual(convert(8), "8")

    def test_the_sound_for_9_is_pling(self):
    self.assertEqual(convert(9), "Pling")

    def test_the_sound_for_10_is_plang(self):
    self.assertEqual(convert(10), "Plang")

    def test_the_sound_for_14_is_plong(self):
    self.assertEqual(convert(14), "Plong")

    def test_the_sound_for_15_is_plingplang(self):
    self.assertEqual(convert(15), "PlingPlang")

    def test_the_sound_for_21_is_plingplong(self):
    self.assertEqual(convert(21), "PlingPlong")

    def test_the_sound_for_25_is_plang(self):
    self.assertEqual(convert(25), "Plang")

    def test_the_sound_for_27_is_pling(self):
    self.assertEqual(convert(27), "Pling")

    def test_the_sound_for_35_is_plangplong(self):
    self.assertEqual(convert(35), "PlangPlong")

    def test_the_sound_for_49_is_plong(self):
    self.assertEqual(convert(49), "Plong")

    def test_the_sound_for_52_is_52(self):
    self.assertEqual(convert(52), "52")

    def test_the_sound_for_105_is_plingplangplong(self):
    self.assertEqual(convert(105), "PlingPlangPlong")

    def test_the_sound_for_12121_is_12121(self):
    self.assertEqual(convert(12121), "12121")


    if __name__ == '__main__':
    unittest.main()









    share|improve this question











    $endgroup$
















      4












      4








      4





      $begingroup$


      There's this simple coding challenge that feels like a twist on FizzBuzz. Your task is to convert a number to a string, the contents of which depends on the number's factors.




      • If the number has 3 as a factor, output 'Pling'.

      • If the number has 5 as a factor, output 'Plang'.

      • If the number has 7 as a factor, output 'Plong'.

      • If the number does not have 3, 5, or 7 as a factor, just pass the number's digits straight through.



      For example, the sample output would look like this:




      28's factors are 1, 2, 4, 7, 14, 28. In raindrop-speak, this would be
      a simple "Plong".



      30's factors are 1, 2, 3, 5, 6, 10, 15, 30. In
      raindrop-speak, this would be a "PlingPlang".




      I've come up with a working solution (based on the test suite results). However, I'm not really happy with it, as I feel this could be simplified. Thus, I'd appreciate some feedback.



      My solution:



      SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"
      FACTORS = (3, 5, 7)

      def is_divisible_by(number, divisior):
      return number % divisior == 0


      def raidndrops(number):
      return [SOUNDS[factor] for factor in FACTORS if is_divisible_by(number, factor)]


      def convert(number):
      speak = raidndrops(number)
      return "".join(speak) if speak else str(number)


      The test suite is here:



      import unittest

      from raindrops import convert

      class RaindropsTest(unittest.TestCase):
      def test_the_sound_for_1_is_1(self):
      self.assertEqual(convert(1), "1")

      def test_the_sound_for_3_is_pling(self):
      self.assertEqual(convert(3), "Pling")

      def test_the_sound_for_5_is_plang(self):
      self.assertEqual(convert(5), "Plang")

      def test_the_sound_for_7_is_plong(self):
      self.assertEqual(convert(7), "Plong")

      def test_the_sound_for_6_is_pling(self):
      self.assertEqual(convert(6), "Pling")

      def test_2_to_the_power_3_does_not_make_sound(self):
      self.assertEqual(convert(8), "8")

      def test_the_sound_for_9_is_pling(self):
      self.assertEqual(convert(9), "Pling")

      def test_the_sound_for_10_is_plang(self):
      self.assertEqual(convert(10), "Plang")

      def test_the_sound_for_14_is_plong(self):
      self.assertEqual(convert(14), "Plong")

      def test_the_sound_for_15_is_plingplang(self):
      self.assertEqual(convert(15), "PlingPlang")

      def test_the_sound_for_21_is_plingplong(self):
      self.assertEqual(convert(21), "PlingPlong")

      def test_the_sound_for_25_is_plang(self):
      self.assertEqual(convert(25), "Plang")

      def test_the_sound_for_27_is_pling(self):
      self.assertEqual(convert(27), "Pling")

      def test_the_sound_for_35_is_plangplong(self):
      self.assertEqual(convert(35), "PlangPlong")

      def test_the_sound_for_49_is_plong(self):
      self.assertEqual(convert(49), "Plong")

      def test_the_sound_for_52_is_52(self):
      self.assertEqual(convert(52), "52")

      def test_the_sound_for_105_is_plingplangplong(self):
      self.assertEqual(convert(105), "PlingPlangPlong")

      def test_the_sound_for_12121_is_12121(self):
      self.assertEqual(convert(12121), "12121")


      if __name__ == '__main__':
      unittest.main()









      share|improve this question











      $endgroup$




      There's this simple coding challenge that feels like a twist on FizzBuzz. Your task is to convert a number to a string, the contents of which depends on the number's factors.




      • If the number has 3 as a factor, output 'Pling'.

      • If the number has 5 as a factor, output 'Plang'.

      • If the number has 7 as a factor, output 'Plong'.

      • If the number does not have 3, 5, or 7 as a factor, just pass the number's digits straight through.



      For example, the sample output would look like this:




      28's factors are 1, 2, 4, 7, 14, 28. In raindrop-speak, this would be
      a simple "Plong".



      30's factors are 1, 2, 3, 5, 6, 10, 15, 30. In
      raindrop-speak, this would be a "PlingPlang".




      I've come up with a working solution (based on the test suite results). However, I'm not really happy with it, as I feel this could be simplified. Thus, I'd appreciate some feedback.



      My solution:



      SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"
      FACTORS = (3, 5, 7)

      def is_divisible_by(number, divisior):
      return number % divisior == 0


      def raidndrops(number):
      return [SOUNDS[factor] for factor in FACTORS if is_divisible_by(number, factor)]


      def convert(number):
      speak = raidndrops(number)
      return "".join(speak) if speak else str(number)


      The test suite is here:



      import unittest

      from raindrops import convert

      class RaindropsTest(unittest.TestCase):
      def test_the_sound_for_1_is_1(self):
      self.assertEqual(convert(1), "1")

      def test_the_sound_for_3_is_pling(self):
      self.assertEqual(convert(3), "Pling")

      def test_the_sound_for_5_is_plang(self):
      self.assertEqual(convert(5), "Plang")

      def test_the_sound_for_7_is_plong(self):
      self.assertEqual(convert(7), "Plong")

      def test_the_sound_for_6_is_pling(self):
      self.assertEqual(convert(6), "Pling")

      def test_2_to_the_power_3_does_not_make_sound(self):
      self.assertEqual(convert(8), "8")

      def test_the_sound_for_9_is_pling(self):
      self.assertEqual(convert(9), "Pling")

      def test_the_sound_for_10_is_plang(self):
      self.assertEqual(convert(10), "Plang")

      def test_the_sound_for_14_is_plong(self):
      self.assertEqual(convert(14), "Plong")

      def test_the_sound_for_15_is_plingplang(self):
      self.assertEqual(convert(15), "PlingPlang")

      def test_the_sound_for_21_is_plingplong(self):
      self.assertEqual(convert(21), "PlingPlong")

      def test_the_sound_for_25_is_plang(self):
      self.assertEqual(convert(25), "Plang")

      def test_the_sound_for_27_is_pling(self):
      self.assertEqual(convert(27), "Pling")

      def test_the_sound_for_35_is_plangplong(self):
      self.assertEqual(convert(35), "PlangPlong")

      def test_the_sound_for_49_is_plong(self):
      self.assertEqual(convert(49), "Plong")

      def test_the_sound_for_52_is_52(self):
      self.assertEqual(convert(52), "52")

      def test_the_sound_for_105_is_plingplangplong(self):
      self.assertEqual(convert(105), "PlingPlangPlong")

      def test_the_sound_for_12121_is_12121(self):
      self.assertEqual(convert(12121), "12121")


      if __name__ == '__main__':
      unittest.main()






      python python-3.x programming-challenge






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited 8 hours ago







      baduker

















      asked 9 hours ago









      badukerbaduker

      3111 silver badge11 bronze badges




      3111 silver badge11 bronze badges























          1 Answer
          1






          active

          oldest

          votes


















          7












          $begingroup$

          Modular division is a well-known concept to almost everyone who has done any programming. In my opinion delegating it to a is_divisible_by function is not needed and only introduces unnecessary overhead by generating an additional function call. It's not like you are ever going to use any other implementation than using modular division. Instead I would simply inline it.



          While I am a fan of clear variable names, and PEP8 recommends against single letter variables, using n for a generic number (and i for a generic counting integer) is IMO acceptable and helps keeping lines short.



          Your FACTORS variable is only needed for the order, since it is just the keys of the dictionary. Since Python 3.7 the order of dictionaries is guaranteed to be insertion order (implemented since CPython 3.6), so you also don't need it for the order if you are using a modern version of Python.



          You have a spelling error in raindrops (but at least it is also present when calling it).



          The convert function can be a bit simplified by using or.



          SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"

          def raindrops(n):
          return [sound for d, sound in SOUNDS.items() if n % d == 0]


          def convert(n):
          return "".join(raindrops(n)) or str(n)


          Instead of having a lot of testcases, which takes a lot of manual typing to setup, group similar testcases together and use unittest.TestCase.subTest:



          class RaindropsTest(unittest.TestCase):
          def test_known_results(self):
          test_cases = [(1, "1"), (3, "Pling"), ...]
          for n, expected in test_cases:
          with self.subTest(n=n, expected=expected)):
          self.assertEqual(convert(n), expected)


          For successful test cases this reports as one test case, but if it fails the additional information passed as keyword arguments is shown (here purposefully broken by supplying the wrong expected output):



          ======================================================================
          FAIL: test_known_results (__main__.RaindropsTest) (expected='5', n=5)
          ----------------------------------------------------------------------
          Traceback (most recent call last):
          File "/tmp/test.py", line 15, in test_known_results
          self.assertEqual(convert(n), expected)
          AssertionError: 'Plang' != '5'
          - Plang
          + 5


          ----------------------------------------------------------------------
          Ran 1 test in 0.000s

          FAILED (failures=1)





          share|improve this answer









          $endgroup$

















            Your Answer






            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader:
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            ,
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );













            draft saved

            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f225275%2fraindrops-in-python%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown

























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            7












            $begingroup$

            Modular division is a well-known concept to almost everyone who has done any programming. In my opinion delegating it to a is_divisible_by function is not needed and only introduces unnecessary overhead by generating an additional function call. It's not like you are ever going to use any other implementation than using modular division. Instead I would simply inline it.



            While I am a fan of clear variable names, and PEP8 recommends against single letter variables, using n for a generic number (and i for a generic counting integer) is IMO acceptable and helps keeping lines short.



            Your FACTORS variable is only needed for the order, since it is just the keys of the dictionary. Since Python 3.7 the order of dictionaries is guaranteed to be insertion order (implemented since CPython 3.6), so you also don't need it for the order if you are using a modern version of Python.



            You have a spelling error in raindrops (but at least it is also present when calling it).



            The convert function can be a bit simplified by using or.



            SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"

            def raindrops(n):
            return [sound for d, sound in SOUNDS.items() if n % d == 0]


            def convert(n):
            return "".join(raindrops(n)) or str(n)


            Instead of having a lot of testcases, which takes a lot of manual typing to setup, group similar testcases together and use unittest.TestCase.subTest:



            class RaindropsTest(unittest.TestCase):
            def test_known_results(self):
            test_cases = [(1, "1"), (3, "Pling"), ...]
            for n, expected in test_cases:
            with self.subTest(n=n, expected=expected)):
            self.assertEqual(convert(n), expected)


            For successful test cases this reports as one test case, but if it fails the additional information passed as keyword arguments is shown (here purposefully broken by supplying the wrong expected output):



            ======================================================================
            FAIL: test_known_results (__main__.RaindropsTest) (expected='5', n=5)
            ----------------------------------------------------------------------
            Traceback (most recent call last):
            File "/tmp/test.py", line 15, in test_known_results
            self.assertEqual(convert(n), expected)
            AssertionError: 'Plang' != '5'
            - Plang
            + 5


            ----------------------------------------------------------------------
            Ran 1 test in 0.000s

            FAILED (failures=1)





            share|improve this answer









            $endgroup$



















              7












              $begingroup$

              Modular division is a well-known concept to almost everyone who has done any programming. In my opinion delegating it to a is_divisible_by function is not needed and only introduces unnecessary overhead by generating an additional function call. It's not like you are ever going to use any other implementation than using modular division. Instead I would simply inline it.



              While I am a fan of clear variable names, and PEP8 recommends against single letter variables, using n for a generic number (and i for a generic counting integer) is IMO acceptable and helps keeping lines short.



              Your FACTORS variable is only needed for the order, since it is just the keys of the dictionary. Since Python 3.7 the order of dictionaries is guaranteed to be insertion order (implemented since CPython 3.6), so you also don't need it for the order if you are using a modern version of Python.



              You have a spelling error in raindrops (but at least it is also present when calling it).



              The convert function can be a bit simplified by using or.



              SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"

              def raindrops(n):
              return [sound for d, sound in SOUNDS.items() if n % d == 0]


              def convert(n):
              return "".join(raindrops(n)) or str(n)


              Instead of having a lot of testcases, which takes a lot of manual typing to setup, group similar testcases together and use unittest.TestCase.subTest:



              class RaindropsTest(unittest.TestCase):
              def test_known_results(self):
              test_cases = [(1, "1"), (3, "Pling"), ...]
              for n, expected in test_cases:
              with self.subTest(n=n, expected=expected)):
              self.assertEqual(convert(n), expected)


              For successful test cases this reports as one test case, but if it fails the additional information passed as keyword arguments is shown (here purposefully broken by supplying the wrong expected output):



              ======================================================================
              FAIL: test_known_results (__main__.RaindropsTest) (expected='5', n=5)
              ----------------------------------------------------------------------
              Traceback (most recent call last):
              File "/tmp/test.py", line 15, in test_known_results
              self.assertEqual(convert(n), expected)
              AssertionError: 'Plang' != '5'
              - Plang
              + 5


              ----------------------------------------------------------------------
              Ran 1 test in 0.000s

              FAILED (failures=1)





              share|improve this answer









              $endgroup$

















                7












                7








                7





                $begingroup$

                Modular division is a well-known concept to almost everyone who has done any programming. In my opinion delegating it to a is_divisible_by function is not needed and only introduces unnecessary overhead by generating an additional function call. It's not like you are ever going to use any other implementation than using modular division. Instead I would simply inline it.



                While I am a fan of clear variable names, and PEP8 recommends against single letter variables, using n for a generic number (and i for a generic counting integer) is IMO acceptable and helps keeping lines short.



                Your FACTORS variable is only needed for the order, since it is just the keys of the dictionary. Since Python 3.7 the order of dictionaries is guaranteed to be insertion order (implemented since CPython 3.6), so you also don't need it for the order if you are using a modern version of Python.



                You have a spelling error in raindrops (but at least it is also present when calling it).



                The convert function can be a bit simplified by using or.



                SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"

                def raindrops(n):
                return [sound for d, sound in SOUNDS.items() if n % d == 0]


                def convert(n):
                return "".join(raindrops(n)) or str(n)


                Instead of having a lot of testcases, which takes a lot of manual typing to setup, group similar testcases together and use unittest.TestCase.subTest:



                class RaindropsTest(unittest.TestCase):
                def test_known_results(self):
                test_cases = [(1, "1"), (3, "Pling"), ...]
                for n, expected in test_cases:
                with self.subTest(n=n, expected=expected)):
                self.assertEqual(convert(n), expected)


                For successful test cases this reports as one test case, but if it fails the additional information passed as keyword arguments is shown (here purposefully broken by supplying the wrong expected output):



                ======================================================================
                FAIL: test_known_results (__main__.RaindropsTest) (expected='5', n=5)
                ----------------------------------------------------------------------
                Traceback (most recent call last):
                File "/tmp/test.py", line 15, in test_known_results
                self.assertEqual(convert(n), expected)
                AssertionError: 'Plang' != '5'
                - Plang
                + 5


                ----------------------------------------------------------------------
                Ran 1 test in 0.000s

                FAILED (failures=1)





                share|improve this answer









                $endgroup$



                Modular division is a well-known concept to almost everyone who has done any programming. In my opinion delegating it to a is_divisible_by function is not needed and only introduces unnecessary overhead by generating an additional function call. It's not like you are ever going to use any other implementation than using modular division. Instead I would simply inline it.



                While I am a fan of clear variable names, and PEP8 recommends against single letter variables, using n for a generic number (and i for a generic counting integer) is IMO acceptable and helps keeping lines short.



                Your FACTORS variable is only needed for the order, since it is just the keys of the dictionary. Since Python 3.7 the order of dictionaries is guaranteed to be insertion order (implemented since CPython 3.6), so you also don't need it for the order if you are using a modern version of Python.



                You have a spelling error in raindrops (but at least it is also present when calling it).



                The convert function can be a bit simplified by using or.



                SOUNDS = 3: "Pling", 5: "Plang", 7: "Plong"

                def raindrops(n):
                return [sound for d, sound in SOUNDS.items() if n % d == 0]


                def convert(n):
                return "".join(raindrops(n)) or str(n)


                Instead of having a lot of testcases, which takes a lot of manual typing to setup, group similar testcases together and use unittest.TestCase.subTest:



                class RaindropsTest(unittest.TestCase):
                def test_known_results(self):
                test_cases = [(1, "1"), (3, "Pling"), ...]
                for n, expected in test_cases:
                with self.subTest(n=n, expected=expected)):
                self.assertEqual(convert(n), expected)


                For successful test cases this reports as one test case, but if it fails the additional information passed as keyword arguments is shown (here purposefully broken by supplying the wrong expected output):



                ======================================================================
                FAIL: test_known_results (__main__.RaindropsTest) (expected='5', n=5)
                ----------------------------------------------------------------------
                Traceback (most recent call last):
                File "/tmp/test.py", line 15, in test_known_results
                self.assertEqual(convert(n), expected)
                AssertionError: 'Plang' != '5'
                - Plang
                + 5


                ----------------------------------------------------------------------
                Ran 1 test in 0.000s

                FAILED (failures=1)






                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered 8 hours ago









                GraipherGraipher

                29.4k5 gold badges46 silver badges103 bronze badges




                29.4k5 gold badges46 silver badges103 bronze badges






























                    draft saved

                    draft discarded
















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid


                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.

                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f225275%2fraindrops-in-python%23new-answer', 'question_page');

                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Invision Community Contents History See also References External links Navigation menuProprietaryinvisioncommunity.comIPS Community ForumsIPS Community Forumsthis blog entry"License Changes, IP.Board 3.4, and the Future""Interview -- Matt Mecham of Ibforums""CEO Invision Power Board, Matt Mecham Is a Liar, Thief!"IPB License Explanation 1.3, 1.3.1, 2.0, and 2.1ArchivedSecurity Fixes, Updates And Enhancements For IPB 1.3.1Archived"New Demo Accounts - Invision Power Services"the original"New Default Skin"the original"Invision Power Board 3.0.0 and Applications Released"the original"Archived copy"the original"Perpetual licenses being done away with""Release Notes - Invision Power Services""Introducing: IPS Community Suite 4!"Invision Community Release Notes

                    Canceling a color specificationRandomly assigning color to Graphics3D objects?Default color for Filling in Mathematica 9Coloring specific elements of sets with a prime modified order in an array plotHow to pick a color differing significantly from the colors already in a given color list?Detection of the text colorColor numbers based on their valueCan color schemes for use with ColorData include opacity specification?My dynamic color schemes

                    Ласкавець круглолистий Зміст Опис | Поширення | Галерея | Примітки | Посилання | Навігаційне меню58171138361-22960890446Bupleurum rotundifoliumEuro+Med PlantbasePlants of the World Online — Kew ScienceGermplasm Resources Information Network (GRIN)Ласкавецькн. VI : Літери Ком — Левиправивши або дописавши її