# Osherove TDD Kata 1: String Calculator

I implemented Roy Osherove's TDD Kata 1: String Calculator using Python 3.5.2 and the unittest module. The full description can be found at the link, it's a bit lengthy for this post.

The jest of it is to implement various string parsing and simple numbers logic to add numbers together which are passed as a delimited string, e.g.: "1,2,3" returns 6.

I followed true TDD principles to the best of my knowledge, in that for each requirement I first wrote a failing test, then made it pass, rinse & repeat, refactoring along the way.

Note that test_incorrect_input_types was not part of the requirement, but rather is a test that I do out of habit to make sure the correct type is passed to the function.

I'm interested in any and all ways I could improve this code. In particular, I'm looking for ways to make the TDD approach as clear and solid as possible.

# Unit tests:

import unittest

"""Tests for add_string_numbers function."""

def test_incorrect_input_types(self) -> None:
"""Test that non-string inputs fail"""
incorrect_inputs = [
None,
42,
3.1415,
[],
{}
]
for input_ in incorrect_inputs:

def test_empty_string(self) -> None:
"""Test empty string as input"""
input_ = ""
expected = 0
self.assertEqual(expected, actual)

def test_1_number(self) -> None:
"""Test a single number as input"""
input_ = "42"
expected = 42
self.assertEqual(expected, actual)

def test_2_numbers(self) -> None:
"""Test 2 numbers as input"""
input_ = "42,99"
expected = 141
self.assertEqual(expected, actual)

def test_many_numbers(self) -> None:
"""Test many numbers as input"""
input_ = "1,2,3,4,5,6,7,8,9,10"
expected = 55
self.assertEqual(expected, actual)

def test_negative_numbers(self) -> None:
"""Test that input with negative numbers fails"""
input_ = "-1"
input_ = "-1,2,-3"

def test_numbers_greater_than_1000(self) -> None:
"""Test that numbers greater than 1000 are ignored"""
input_ = "1,1001,2"
expected = 3
self.assertEqual(expected, actual)
input_ = "1001,2001,3001"
expected = 0
self.assertEqual(expected, actual)

def test_newline_delimiter(self) -> None:
"""Test input separated by newlines instead of commas"""
input_ = "1\n2\n3"
expected = 6
self.assertEqual(expected, actual)
input_ = """1
2
3"""
expected = 6
self.assertEqual(expected, actual)

def test_invalid_delimiters(self) -> None:
"""Test that input with invalid delimiter placement fails"""
input_ = "1,,2"
input_ = "1,2,"
input_ = ",1,2"

def test_custom_delimiters(self) -> None:
"""Test that custom delimiters can be used with //[delim1][delim2]\n syntax"""
input_ = "//[***]\n1***2***3"
expected = 6
self.assertEqual(expected, actual)
input_ = "//[***][&&]\n1***2&&3"
expected = 6
self.assertEqual(expected, actual)


# Implementation:

from typing import List
import re

"""
Add numbers passed in the form of a delimited string of numbers.
For example, "1,2,3" returns 6.

Delimiters are handled in the following ways:
- The standard delimiter is a comma ,
- Newlines \n are always acceptable delimiters, e.g.:
1\n2\n3
- Additional delimiters can be declared with the following syntax
added before the beginning of a delimited string:
//[delim1][delim2]\n
...then used as delimiters. For example:
//[***][&&]\n1***2&&3
which will be parsed to:
1,2,3
"""
if not isinstance(numbers, str):
raise TypeError("Input must be of string type")
if numbers == "":
return 0
number_list = parse_into_list(numbers)
result = 0
negatives = []
for num in number_list:
try:
num = int(num)
except ValueError:
raise ValueError("Input could not be converted to int")
if num < 0:
negatives.append(num)
else:
if num <= 1000:
result += num
if negatives:
raise ValueError("Negatives not allowed: {0}".format(",".join(map(str, negatives))))
return result

def parse_into_list(numbers: str) -> List:
"""Parse a string of numbers into a list"""
numbers = handle_delimiters(numbers)
return numbers.split(",")

def handle_delimiters(numbers: str) -> str:
"""
Handle delimiters according to the add function requirements.
"""
alt_delims = ["\n"]
if numbers.startswith("//"):
for delim in delims:
alt_delims.append(delim)
for delim in alt_delims:
numbers = numbers.replace(delim, ",")
return numbers

"""Helper to handle the //...\n header and return the section containing the delimiters"""
//    # match beginning delimiter
(.*)  # capturing group of 0 or more of any char
\n    # match ending delimiter
""", re.VERBOSE)

"""Helper to handle each individual delimiter and return a list containing them"""
re_delims = re.compile(r"""
$# match a literal [ ( # start capturing group [^]]* # match anything except a closing ] ) # close capturing group$      # match a literal ]
""", re.VERBOSE)

def cleanup_numbers_string(original: str, delims_header: str) -> str:
"""Helper to remove the delimiters header section and return only the delimited list"""


Personally I think your code is good. I personally would do things slightly differently. However I don't think there are really any major, or minor, problems. However I don't really know testing in Python, and so won't comment on that.

Some things I'd do differently are:

• I personally don't like erroring whilst handling another error. Even more so when both raise the same error, but with different messages. And so I'd just stick to using int without a try-except.
• Personally I'd prefer to use a comprehension than a for loop. And so I'd change lines 26 to 37 to three comprehensions.
• I prefer using sum over manually adding numbers. sum(i for i in numbers if i <= 1000) is easy to read and, IIRC, faster.
• If I came across parse_into_list in the wild, I'd think it should take a delimiter argument. However that's still in the numbers argument. Instead I'd go for a split_calculator_input function. This would return just numbers. And so I'd use something like the below, which I think is slightly easier to read than your functions.

def split_calculator_input(input):
delimiter, numbers = re.match(r'(?s)^(?://$([^\n]+)$\n)?(.*)$', input).groups() delimiter = '|'.join((delimiter or r',][\n').split('][')) return re.split(delimiter, numbers)  I had a go at doing this myself, since it's a one-time thing I didn't split the code into a lot of functions, but came up with the following: def add_string_numbers(input): if not input: return 0 delimiter, numbers = re.match(r'(?s)^(?://$([^\n]+)$\n)?(.*)$', input).groups()
delimiter = '|'.join((delimiter or r',][\n').split(']['))
numbers = [int(i) for i in re.split(delimiter, numbers)]
negatives = [i for i in numbers if i < 0]
if negatives:
raise ValueError('Negatives not allowed {} found'.format(negatives))
return sum(i for i in numbers if i <= 1000)