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\$
6
  • \$\begingroup\$ Is this from your own code, or is it purely an example? The latter is off-topic. \$\endgroup\$
    – Jamal
    Jul 15, 2014 at 19:57
  • \$\begingroup\$ Own code, however simplified function, just looking for confirmation on the exact sequence of replacements. \$\endgroup\$
    – Paul
    Jul 15, 2014 at 20:01
  • 2
    \$\begingroup\$ Write a test to see. Try this: unescape("//+") and see what you get. \$\endgroup\$
    – Kylar
    Jul 15, 2014 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, 2014 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, 2014 at 20:09

1 Answer 1

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\$
2
  • \$\begingroup\$ Thanks for showing me so clearly that multiple passes is indeed a bad idea. \$\endgroup\$
    – Paul
    Jul 16, 2014 at 11:38
  • 1
    \$\begingroup\$ I never realized that working from left to right is so important in escape functions. \$\endgroup\$
    – Paul
    Jul 16, 2014 at 19:04

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.