3

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)

1 Answer 1

4

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.

Sign up to request clarification or add additional context in comments.

2 Comments

This was more fun than I expected. Thank you both very much.
Thank you, @juanpa.arrivillaga, that was a bad call on my part. I've updated the answer to make that clear, leaving it in because success with this sort of stuff may lead to a presumption it will work in the problematic cases.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.