6
\$\begingroup\$

Is this example of an escape - unescape function correct?

ESCAPE = '/'

def escape(s):
    rv = s
    rv = rv.replace(ESCAPE, ESCAPE + ESCAPE)
    rv = rv.replace('+', ESCAPE + 'PLUS')
    rv = rv.replace('-', ESCAPE + 'MINUS')
    return rv

def unescape(s):
    rv = s
    rv = rv.replace(ESCAPE + 'MINUS', '-')
    rv = rv.replace(ESCAPE + 'PLUS', '+')
    rv = rv.replace(ESCAPE + ESCAPE, ESCAPE)
    return rv
\$\endgroup\$
  • \$\begingroup\$ Is this from your own code, or is it purely an example? The latter is off-topic. \$\endgroup\$ – Jamal Jul 15 '14 at 19:57
  • \$\begingroup\$ Own code, however simplified function, just looking for confirmation on the exact sequence of replacements. \$\endgroup\$ – Paul Jul 15 '14 at 20:01
  • 2
    \$\begingroup\$ Write a test to see. Try this: unescape("//+") and see what you get. \$\endgroup\$ – Kylar Jul 15 '14 at 20:02
  • \$\begingroup\$ I wrote tests, I think it's ok. @Kylar I'm not sure if the unescape function should except the + sign to unescape... \$\endgroup\$ – Paul Jul 15 '14 at 20:03
  • 1
    \$\begingroup\$ Exactly my point. Escaping/Unescaping is tricky. You may want to iterate through the string by yourself, and keep a state machine. It will make things much easier when you hit unexpected data. \$\endgroup\$ – Kylar Jul 15 '14 at 20:09
8
\$\begingroup\$

Performing string substitutions using multiple passes is almost always a bad idea, not only for performance, but for correctness. In this case, there is indeed a bug: unescape('//PLUS/PLUS') should produce '/PLUS+', but instead produces '/++'.

Therefore, you have to parse the input string in one pass:

import re

def unescape(s):
    def replacement(match):
        return {
            '/': '/',
            'PLUS': '+',
            'MINUS': '-',
        }[match.group(1)]
    return re.sub('/(/|PLUS|MINUS)', replacement, s)

In your original code and in the implementation above, there is a lot of repetition. If there are more symbols being escaped than just + and -, then it makes sense to specify the mapping just once.

ESCAPE_CHAR = '/'
ESCAPE = {
    ESCAPE_CHAR: ESCAPE_CHAR,
    '+': 'PLUS',
    '-': 'MINUS',
}
ESCAPE_RE = re.compile('|'.join(map(re.escape, ESCAPE.keys())))

UNESCAPE = dict((v, k) for (k, v) in ESCAPE.items())
UNESCAPE_RE = re.compile(
    re.escape(ESCAPE_CHAR) + '(' +
        '|'.join(map(re.escape, UNESCAPE.keys())) +
    ')')

def escape(s):
    return ESCAPE_RE.sub(lambda match: ESCAPE_CHAR + ESCAPE[match.group(0)], s)

def unescape(s):
    return UNESCAPE_RE.sub(lambda match: UNESCAPE[match.group(1)], s)
\$\endgroup\$
  • \$\begingroup\$ Thanks for showing me so clearly that multiple passes is indeed a bad idea. \$\endgroup\$ – Paul Jul 16 '14 at 11:38
  • 1
    \$\begingroup\$ I never realized that working from left to right is so important in escape functions. \$\endgroup\$ – Paul Jul 16 '14 at 19:04

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.