3
\$\begingroup\$

I've got a string that consists of an arbitrary combination of text and {} delimited python code, for instance, A plus b is {a + b}. However, braces are used for dictionary and set literals in python, so You chose the {{1:"first", 2:"second"}[choice]} option should also be interpretted correctly. It's also valid to have more than one python expression in the input, so You picked {choice1} and {choice2} is valid.

Here's my current code:

protected String ParseStringForVariable([NotNull] String str)
{
    PythonEngine.PythonEngine pythonEngine = GetCurrentCore().PythonEngine;

    for (int i = 0; i < str.Length; i++)
    {
        if (str[i] != '{')
        {
            continue;
        }

        int opening = i;
        foreach (var expression in from closing in str.IndexesWhere('}'.Equals)
                                   where closing > opening
                                   select new
                                          {
                                              Template = str.Substring(opening, closing - opening + 1),
                                              Code = str.Substring(opening + 1, closing - opening - 1)
                                          })
        {
            PythonByteCode compiled;
            try
            {
                compiled = pythonEngine.Compile(expression.Code, PythonByteCode.SourceCodeType.Expression);
            }
            catch (PythonParseException)
            {
                // not valid python, try next expression
                continue;
            }
            String result = pythonEngine.Evaluate(compiled).ToString();
            str = str.Replace(expression.Template, result);
            break;
        }
    }

    return str;
}

It works by looking at progressively longer strings, attempting to parse them, and ignoring them if it's not valid python. This is done with the PythonEngine class, which is a wrapper around the IronPython interpretter, and has an extensive test suite, so can assumed to be correct.

  1. It appears to burp slightly if the value of the first of multiple python expression contains an opening brace: {"{"} {"}"}, what's the best way to prevent that?

  2. Resharper is complaining about access to modified closures, can you provide a test case that exposes why that is an issue?

  3. It works under all inputs I've tested it with, but what edge cases should be included in the test suite? (It's intended behaviour that invalid python code is left as-is). Current thoughts include:

    • Empty string
    • without braces
    • empty braces
    • valid python in braces
    • invalid python
    • both valid and invalid
    • both valid and empty
    • both invalid and empty
    • valid, nested braces
    • invalid, nested braces
    • valid, evaluated to contain open brace
    • valid, evaluated to contain close brace - irrelevant, as parsing is LTR?
    • valid, evaluated to contain open brace followed by further invalid
    • valid, evaluated to contain open brace followed by further empty
    • unmatched open brace
    • unmatched close brace
  4. Are there any improvements that jump out? Clarity, performance, style?

\$\endgroup\$
11
  • \$\begingroup\$ What are you using to execute the Python code? I can't seem to find types like PythonByteCode anywhere. \$\endgroup\$
    – svick
    Commented Apr 10, 2013 at 18:35
  • \$\begingroup\$ @svick Post edited to clarify. \$\endgroup\$
    – RoadieRich
    Commented Apr 11, 2013 at 8:04
  • \$\begingroup\$ reg 3.) Could you please add your test cases. \$\endgroup\$ Commented Apr 11, 2013 at 9:31
  • \$\begingroup\$ @mnhg clarified slightly, but added. \$\endgroup\$
    – RoadieRich
    Commented Apr 11, 2013 at 10:27
  • \$\begingroup\$ Regarding #2: stackoverflow.com/q/8898925/298754 \$\endgroup\$
    – Bobson
    Commented Apr 11, 2013 at 15:01

1 Answer 1

2
\$\begingroup\$

I see an extra unnecessary variable that you can get rid of, or change another variable so that it makes more sense.

int opening = i;

you use this inside of a for loop. You can do 2 things with this.

  1. Just use the variable i where ever you had the variable opening
  2. change the i variable to opening in the for loop like

    for (int opening = 0; opening < str.Length; opening++)
    

hopefully this shows you how unnecessary that extra variable is, it isn't used anywhere else for anything other than the loops.


Other than that I would need to play with it and spend some time with it to see if it plays well with others.

\$\endgroup\$

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.