1
\$\begingroup\$

I have implemented class which manages communication with measuring device, which sends list of assignments, like variableName=value. These assignments are always separated by space. There are two options, the first is simple assignment variableName=value (without space around equal sign), the second is "array assignment", like variableName[num]= value_1 ... value_num (here is space after equal sign and between single values). Here is my code for parser:

    /// <summary>
    /// Parse variable value "y" from assignment pair "x=y"
    /// </summary>
    /// <param name="s">string where "x=y" pairs are separated by space</param>
    /// <param name="varName">left side of assignment</param>
    /// <returns>right side of assignment</returns>
    private static string ParseEqualValue(string s, string varName)
    {
        int startPos = s.IndexOf("=", s.IndexOf(varName)) + 1;
        int endPos = s.IndexOf(" ", startPos);
        return s.Substring(startPos, endPos - startPos);
    }

    /// <summary>
    /// Parse variable values after "=" from string
    /// </summary>
    /// <param name="s">string in format: varName[num]= x1 .. xnum</param>
    /// <param name="varName">left side of assignment</param>
    /// <returns>right side of assignment in the list</returns>
    private static List<string> ParseEqualValues(string s, string varName)
    {
        List<string> list = new List<string>();

        // position of ShapeX..
        int varPos = s.IndexOf(varName);

        // parsing of numPts value in []
        int numStartPos = s.IndexOf("[", varPos) + 1;
        int numEndPos = s.IndexOf("]", numStartPos);
        int num = 0;
        int.TryParse(s.Substring(numStartPos, numEndPos - numStartPos), out num);

        int startPos = s.IndexOf("=", varPos) + 2;

        for (int i = 0; i < num; i++)
        {
            int endPos = s.IndexOf(" ", startPos);
            if (endPos < 0)
                endPos = s.Length;

            list.Add(s.Substring(startPos, endPos - startPos));
            startPos = endPos + 1;
        }

        return list;
    }

And here how I use it:

    /// <summary>
    /// Create ColdTest cut from ProfRoll answer
    /// </summary>
    /// <param name="s">Answer from ProfRoll</param>
    /// <returns></returns>
    public static Cut CreateColdTestCut(string s)
    {
        Cut cut = new Cut();
        cut.pts = new List<PointF>();

        int numShapes;

        double.TryParse(ParseEqualValue(s, "AvgTemp"), out cut.pyroTemp);
        double.TryParse(ParseEqualValue(s, "MinTemp"), out cut.tempMin);
        double.TryParse(ParseEqualValue(s, "MaxTemp"), out cut.tempMax);

        int.TryParse(ParseEqualValue(s, "NumShapes"), out numShapes);

        List<int> numPts = new List<int>();

        for (int i = 0; i < numShapes; i++)
        {
            List<string> xPts = ParseEqualValues(s, string.Format("ShapeX{{0}}", i));
            List<string> yPts = ParseEqualValues(s, string.Format("ShapeY{{0}}", i));

            if (xPts.Count != yPts.Count)
                break;

            numPts.Add(xPts.Count);

            for (int j = 0; j < xPts.Count; j++)
            {
                try
                {
                    cut.pts.Add(new PointF(float.Parse(xPts[j]), float.Parse(yPts[j])));
                }
                catch { }
            }
        }

        cut.isColdTest = true;
        cut.time = DateTime.Now;
        cut.numCurves = cut.numPts.Length;

        return cut;
    }

I will be glad for any critical comments.

\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

I'm not sure exactly where to start with reviewing this code. There are many things that jump out as unsafe or awkwardly approached.

If my source string contained:

foo1=bar1 foo=bar

Then querying for 'foo'

int startPos = s.IndexOf("=", s.IndexOf(varName)) + 1;

Will incorrectly return the index of 'bar1' not 'bar'.

Use positional data to locate the keys within the string. All keys begin immediately after a space, and end immediately before an equals sign, and contain no spaces or equal signs.

Start by splitting the string on the '='. Your first key will be result[0]. Find the last index of a ' ' character in result[1]. Everything before that is your first value. Iterate your split results to build a Dictionary<string, string> and a Dictionary<string, string[]>, processing the value of key as appropriate based on the assignment type. You can then check if a particular key is defined by looking for it in the dictionaries, and get the value directly.

Is there a purpose to specifying the number of array elements in the key, when you already know the number from the value? How do you propose resolving the difference in the case of an incorrectly constructed string?

"a[1]= x y z"

Should either be a[3]= x y z or a[1]= x, but which takes precedence. If you can control the incoming format, and are not dealing with something beyond your control, use a[]= x y z and don't rely on the count being placed in the key.

\$\endgroup\$
1
  • \$\begingroup\$ I definitely like idea of using Dictionary here, thanks! Constructing correct string with number of elements relies on measuring device class, so if it constructs a[1]= x y z for some reason, I want to take a[1]= x, maybe it sends me all measured results and tells me that only the first on is valid, who knows. \$\endgroup\$
    – Majak
    Commented Nov 24, 2015 at 8:18

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.