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;
$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()
python python-3.x programming-challenge
$endgroup$
add a comment |
$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()
python python-3.x programming-challenge
$endgroup$
add a comment |
$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()
python python-3.x programming-challenge
$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
python python-3.x programming-challenge
edited 8 hours ago
baduker
asked 9 hours ago
badukerbaduker
3111 silver badge11 bronze badges
3111 silver badge11 bronze badges
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
$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)
$endgroup$
add a comment |
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
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
$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)
$endgroup$
add a comment |
$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)
$endgroup$
add a comment |
$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)
$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)
answered 8 hours ago
GraipherGraipher
29.4k5 gold badges46 silver badges103 bronze badges
29.4k5 gold badges46 silver badges103 bronze badges
add a comment |
add a comment |
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.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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