5
\$\begingroup\$

Given a string, return the sum of all the numbers in the string, 0 if none are present.

Below is my solution for the problem above. I feel like this can be turned into one line. I would really only like suggestions on the actual algorithm, but anything else is appreciated.

def sum_numbers(string: str) -> int:
    """
    Returns the sum of all the numbers in the string
    """
    num_sum = 0
    for char in string:
        if char.isnumeric():
            num_sum += int(char)
    return num_sum

if __name__ == '__main__':

    # Test Cases #

    assert sum_numbers("123") == 6
    assert sum_numbers("abc") == 0
    assert sum_numbers("4x7") == 11
    assert sum_numbers("wefbyug87iqu") == 15
    assert sum_numbers("123456789") == 45
\$\endgroup\$
  • \$\begingroup\$ @Downvoter, please explain what drew the downvote so I can fix it. \$\endgroup\$ – Linny Oct 10 at 20:55
  • 2
    \$\begingroup\$ I feel like a better title would be "Sum of all digits in a string." Digits imply "abc10def10" is 2, numbers implies 20. \$\endgroup\$ – rrauenza Oct 10 at 21:02
  • \$\begingroup\$ @rrauenza That would make more sense. Title changed accordingly. \$\endgroup\$ – Linny Oct 10 at 22:47
15
\$\begingroup\$

You're right, it can be turned into one line using comprehension syntax which would be your best option of you're looking for a slightly more efficient code.

  • isdecimal() is better to use in this case than isnumeric() because the isnumeric() accepts types like ½ and ¼ and n² which might produce some side effects you don't want if you're using this function for some real application. Apart from this the code looks good, can be improved using the comprehension syntax which is more efficient and shorter.

Improved version:

def sum_digits(chars: str) -> int:
    """Return sum of numbers in chars"""
    return sum(int(char) for char in chars if char.isdecimal())


if __name__ == '__main__':
    print(sum_digits('abcd173fg'))
    print(sum_digits('abcd'))
    print(sum_digits('173678'))
\$\endgroup\$
  • \$\begingroup\$ @AJNeufeld is it necessary to write functions in this form def func(x: some_type) -> another_type: ? \$\endgroup\$ – bullseye Oct 10 at 5:08
  • 3
    \$\begingroup\$ @brijeshkalkani what solution do you expect from the string '2.53.6'? It is not a valid float. There are several ways it could be interpreted: 2 + 5 + 3 + 6 = 16; 2.5 + 3.6 = 6.1; 2 + 53 + 6 = 61; 2.53 + 6 = 8.53 \$\endgroup\$ – Turksarama Oct 10 at 5:17
  • 1
    \$\begingroup\$ @IEatBagels Did you post your comment on the wrong answer? This answer only iterates through the string once. The answer by c4llmeco4ch iterates through the string multiple times. \$\endgroup\$ – AJNeufeld Oct 10 at 18:53
  • 2
    \$\begingroup\$ @IEatBagels to clarify AJNeufeld's comment, this code is using a generator expression instead of a list comprehension, so the code does not iterate through the string multiple times. \$\endgroup\$ – oobug Oct 10 at 18:55
  • 1
    \$\begingroup\$ @oobug Well you (I) learn something new everyday! \$\endgroup\$ – IEatBagels Oct 10 at 19:12
4
\$\begingroup\$

We could also do something like:

def sum_numbers(string: str) -> int:
    """Return sum of numbers in string"""
    return sum([string.count(str(i)) * i for i in range(10)])


if __name__ == '__main__':
    print(sum_numbers('hel3l4o55')) #should print 17
    print(sum_numbers('xdman'))     #should print 0
    print(sum_numbers('123456789')) #should print 45

This differs from your implementation due to the fact that my loop iterates over the digits while yours loops through the string itself. This means a few things:

  1. If a string has at least one of every digit, this implementation skips out on checking for 0s.

  2. Depending on how Python internally runs the list.count function, there is a chance this implementation is wildly inefficient overall. I likely loop through the strings more times than its worth, so this becomes worse as the length of the string becomes sizeable enough.

Ultimately, this is just a different idea on what we can loop around. As the number of things we need to search for (and in turn, the number of things we have to run a count on increases), the more times we search through the string, wasting time. However, if we are only looking for one thing or a small sample of possible results, list.count is a good option to have available.

EDIT: might also depend on how floats are looked at as mentioned in the above comments. If they are supposed to just be read as individual chars then this works fine. If you're expected to parse whole sections to determine what is and isn't a float/multi-char number, this falls apart.

\$\endgroup\$
  • 1
    \$\begingroup\$ Updated the edit to provide a bit more thought on when this would make sense. Hopefully that suffices but if not, let me know. Thanks and sorry for the trouble \$\endgroup\$ – c4llmeco4ch Oct 10 at 8:00
  • \$\begingroup\$ You should write a full code as an answer and explain why it might be better than the Op's, please edit your answer that most likely will attract negative energy here. \$\endgroup\$ – bullseye Oct 10 at 8:02
  • 2
    \$\begingroup\$ @bullseye These are current guidelines for answering with a minimum of content: codereview.meta.stackexchange.com/questions/9308/… \$\endgroup\$ – dfhwze Oct 10 at 8:03
  • 2
    \$\begingroup\$ @bullseye hopefully provided a bit more in line with what you both find reasonable. appreciate you guys catching me up to speed on what we're looking for here. \$\endgroup\$ – c4llmeco4ch Oct 10 at 8:22
  • 1
    \$\begingroup\$ Usually, alternative implementations are only valid as part of a review. They shouldn't be the start of a review. Point out what's wrong with the code first, and if you want, provide an alternative fixing that what's wrong further on. An alternative without pointing out what's wrong with the original isn't a review after all. Since this is Code Review, you know... \$\endgroup\$ – Mast Oct 10 at 9:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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