0
\$\begingroup\$

If the prefix/suffix does not match the beginning or the end of a string, then, depending on how I call these functions, they should either raise an exception or return the original text unmodified.

I am using these inside of a simple script for now, so there was no need to make these member functions of some class. I am hoping to find a way to simplify the logic, extract any common code into private function(s), improve the code style (although I think there are several different standards).

def remove_prefix(text, prefix, raise_if_no_match=True):
    exc_msg = 'Text "{}" does not start with a prefix "{}".'.format(text, prefix)
    if not prefix or not text:
        if not raise_if_no_match:
            return text
        if len(prefix) == len(text):
            return ''
        raise ValueError(exc_msg)
    if text.startswith(prefix):
        return text[len(prefix):]
    if raise_if_no_match:
        raise ValueError(exc_msg)
    return text

def remove_suffix(text, suffix, raise_if_no_match=True):
    exc_msg = 'Text "{}" does not end with a suffix "{}".'.format(text, suffix)
    if not suffix or not text:
        if not raise_if_no_match:
            return text
        if len(suffix) == len(text):
            return ''
        raise ValueError(exc_msg)
    if text.endswith(suffix):
        return text[:-len(suffix):]
    if raise_if_no_match:
        raise ValueError(exc_msg)
    return text

print remove_prefix('Hello, World', 'Hello, ')
# ValueError: Text "Hello, World" does not start with a prefix "Hello, Hello".
#print remove_prefix('Hello, World', 'Hello, Hello')
print remove_prefix('Hello, World', 'Hello, Hello', raise_if_no_match=False)

print remove_suffix('I am singing in the rain', ' in the rain')
# ValueError: Text "I am singing in the rain" does not end with a suffix "swinging in the rain".
#print remove_suffix('I am singing in the rain', 'swinging in the rain')
print remove_suffix('I am singing in the rain', 'swinging in the rain', raise_if_no_match=False)

Output:

World
Hello, World
I am singing
I am singing in the rain
\$\endgroup\$
  • \$\begingroup\$ It seems to me that you could just delete code from first if to first raise. The rest is ok. \$\endgroup\$ – Janne Karila Nov 5 '13 at 21:36
1
\$\begingroup\$

Firstly I would say turn your print statements into a test. That way you can change the implementation with confidence that you have not broken anything.

Here is what those debug prints are as a test:

class TestRemove(unittest.TestCase):

    def test_remove_prefix(self):
        hello = 'Hello, World'
        value = remove_prefix(hello, 'Hello, ')
        self.assertEqual(value, 'World')
        value = remove_prefix(hello, 'Hello, Hello', raise_if_no_match=False)
        self.assertEqual(value, hello)
        self.assertRaises(ValueError, remove_prefix, hello, 'Hello, Hello')

    def test_remove_suffix(self):
        singing = 'I am singing in the rain'
        value = remove_suffix(singing, ' in the rain')
        self.assertEqual(value, 'I am singing')
        value = remove_suffix(singing, 'swinging in the rain', raise_if_no_match=False)
        self.assertEqual(value, singing)
        self.assertRaises(ValueError, remove_suffix, singing, 'swinging in the rain')

I think you should be using the the built in string operations which can be found here. Namely startswith(), endswith(), spilt() and rsplit().

You can also reassign variable names to give a more clear flow through the function. Here is my version of remove_prefix().

def remove_prefix(text, prefix, raise_if_no_match=True):
    if (text.startswith(prefix)):
        text = text.split(prefix, 1)[1]
    else:
        if (raise_if_no_match):
            msg_fmt = 'Text "{}" does not end with a prefix "{}".'
            raise ValueError(msg_fmt.format(text, prefix))
    return text

From that I'm sure you can change remove_suffix().

Also I would add docstrings to the functions so the arguments and exceptions raised are documented.

\$\endgroup\$
  • 1
    \$\begingroup\$ I'd omit the ()s around the conditions; this is Python, not a C-decendent language ... I use them sometimes to spread a more complex condition over several lines, though, and thus avoid backslashes. \$\endgroup\$ – Tobias Nov 5 '13 at 23:45
  • \$\begingroup\$ Looks good, thanks. One thing - I would rename the following variable to msg_fmt instead because that is what it is now: exc_msg = 'Text "{}" does not end with a prefix "{}".' \$\endgroup\$ – Leonid Nov 6 '13 at 0:50
  • \$\begingroup\$ @Tobias I like the consistency of always having the brackets whether they are needed for multi-line conditions or not. \$\endgroup\$ – davidcorne Nov 6 '13 at 9:33
  • \$\begingroup\$ @Leonid I agree, that is a better name, changed. \$\endgroup\$ – davidcorne Nov 6 '13 at 9:35

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.