Memory Error during string replacement in python

user_7663410 0
Viewed 602

I wanted to answer a rather easy looking question on stackoverflow. Original code (not mine):

def to_alternating_case(string):
    for char in string:
        if ord(char) in range(97, 123):
            string.replace(char, string.upper())
        elif ord(char) in range (65,91):
            string.replace(char, string.lower())
        elif ord(char) in range (32, 48):
            continue
        else:
            return '//Non-alphabetical characters are unaffected'
    return string

Of course, because strings are immutable and string.replace cannot change the original string, the return value is still the original string (lower and upper case should have been swapped by the intention of the original programmer). So I wanted to suggest the following simple fix:

def to_alternating_case(string):
    for char in string:
        if ord(char) in range(97, 123):
            string = string.replace(char, string.upper())
        elif ord(char) in range (65,91):
            string = string.replace(char, string.lower())
        elif ord(char) in range (32, 48):
            continue
        else:
            return '//Non-alphabetical characters are unaffected'
    return string


print(to_alternating_case("Guten Tag"))

But instead of new hard earned reputation points and a warm 'thank you' I get this exception:

Traceback (most recent call last):
  File "main.py", line 14, in <module>
    print(to_alternating_case("Guten Tag"))
  File "main.py", line 4, in to_alternating_case
    string = string.replace(char, string.upper())
MemoryError

At the moment I am truly puzzled. What is going on? I did not check the numbers, so I currently would expect any string - but not a MemoryError.

Disclaimer: Yes, I know the original issue can be solved with the one-liner to_alternating_case = lambda s: ''.join(c.lower() if c.isupper() else c.upper() for c in s)

python
1 Answers

You should possibly rethink this:

string = string.replace(char, string.upper())

This replaces the character with an upper-case version of the entire string, probably not what you wanted. It's going to get very big, very fast, and possibly run out of memory. One would hope Python would raise a signal of some description should that occur :-)

And, just as an aside, you should be very circumspect about the possibility of modifying things that you're currently iterating over, that tends to lead to either double-processing of items, or missing of items, the classic case being:

for item in someList:
    if item < 0:
        someList.remove(item)

That's not actually the case in your code since you're simply rebinding string to a new object after you've started iterating over the thing it originally pointed to, but it's still a good habit to avoid in case it gives you a false sense of security.

In those cases (where it matters), it's better to create a new item and then, once built assign back to the original somehow.

user_148600
Related