0
\$\begingroup\$

I just write a simple method which returns a string based on the number in accepts as a parameter. I used if/elif/esle here but I think for this simple scenario I wrote long code. I want to know is it possible to shorten this code in any way except using a lambda method. Also, I'm interested to know about the performance of if/elif/else. Is there any other solution which can make faster this method?

def oneTwoMany(n):
    if n == 1:
        return 'one'
    elif n == 2:
        return 'two'
    else:
        return 'many'
\$\endgroup\$

closed as off-topic by dfhwze, Mast, VisualMelon, 23fc9a62-56de-47fb-97b4-737890, IEatBagels Jul 19 at 15:15

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – dfhwze, Mast, VisualMelon, 23fc9a62-56de-47fb-97b4-737890, IEatBagels
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ This has received close-votes because it lacks context. You could improve it by adding more information: where will this method be used? How is the output used? Is it likely to change in the future? Are there many of these methods, all slightly different, which you need to maintain? What range of values should it accept? (e.g. zero) \$\endgroup\$ – VisualMelon Jul 19 at 8:19
4
\$\begingroup\$

You can instead use a dictionary to define your n values:

nums = {1: 'one', 2: 'two'}

Then, when you want to use this, you can use .get(). .get() has a default argument which is returned when a dict doesn't have a key you've requested; here we use 'many' to be returned.

def oneTwoMany(n):
    nums = {1: 'one', 2: 'two'}
    return nums.get(n, 'many')

If you really want to be concise you can stop defining nums:

def oneTwoMany(n):
    return {1: 'one', 2: 'two'}.get(n, 'many')

As an aside to the downvoters of the question; it may be a simple question but I don't see how it breaks any rules.

EDIT: incorporating some other answers which (quite rightly) suggest catching certain invalid inputs.

def oneTwoMany(n):
    if type(n) is not int:
        raise TypeError('Invalid input, value must be integer')
    elif n < 1:
        raise ValueError('Invalid input, value is below 1')
    else:
        return {1: 'one', 2: 'two'}.get(n, 'many')
\$\endgroup\$
  • \$\begingroup\$ @Thank you dear for your nice solution and support. As you mentioned, this seems a simple question but in my opinion, can help me a lot and my others to think different and learn new flexible and file solutions. You're code and solution is really nice. \$\endgroup\$ – Ibrahim Rahimi Jul 4 at 18:11
  • \$\begingroup\$ This solution, just like the original, breaks on corner cases. Try oneTwoMany(0) and oneTwoMany(-1). \$\endgroup\$ – Richard Neumann Jul 5 at 9:30
  • \$\begingroup\$ @Richard Neumann, What do you suggest in that case? Should we extend the if/elif/ clauses? I should mention that this solution in my scenario which is a simple task is enough but as you mentioned in real and more complex cases it causes the program to break. What we can to do to improve it? \$\endgroup\$ – Ibrahim Rahimi Jul 5 at 9:43
  • \$\begingroup\$ Maybe say 'none' on 0 and raise a ValueError on negative numbers. \$\endgroup\$ – Richard Neumann Jul 5 at 9:59
3
\$\begingroup\$

I'd question your objectives here. Yes, it's admirable to keep code concise, but we should also strive to make it robust. As it currently stands, we will return 'many' for any of these inputs:

  • 0
  • -5
  • 0.1
  • True
  • None
  • "foobar"
  • []

Consider throwing an exception if the input is not a positive integer.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.