1
\$\begingroup\$

I have function that convert binary number to unary number based on a Markov algorithm. In what ways can I improve the code?

The algorithm goes as follows:

  1. Replace all 0l with 0l

    "|0" -> "0||"

  2. Replace all l0 with 0ll

    "1" -> "0|"

  3. Major Step 3: Remove the 0

    "0" -> ""

    public List BinaryToUnary(List value)
    {
        var temp = new List();
        var temp2 = new List();
        var temp3 = new List();
        string hold = "";
        int count = 1;


        TxtToDisPlay = "Major Step 1: replace All 1 with 0l userInput " + string.Join("", value) + "\n\n";

        foreach (var x in value)
        {
            if (x.Equals("1"))
            {
                temp.Add("0l");
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + x + " = " + "0l" + "\n";
            }
            else
            {
                temp.Add(x);
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + x + " = " + x + "\n";
            }
            count++;
        }
        TxtToDisPlay = TxtToDisPlay + "\n\nMajor Step 2: Replace all l0 with 0ll  userInput = " + string.Join("", temp) + " \n\n";

        foreach (var x in string.Join("", temp))
        {
            hold = hold + x;
            if (hold.Equals("l0"))
            {
                temp2.Add("0ll");
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + hold + " = " + "0ll" + "\n";
                hold = "";
            }
            if (hold.Equals("0"))
            {
                temp2.Add(hold);
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + hold + " = " + hold + "\n";
                hold = "";
            }
            count++;
        }
        if (hold.Equals("l"))
        {
            temp2.Add("l");
        }


        hold = "";

        foreach (var y in string.Join("", temp2))
        {
            if (temp2.Count == 1)
            {
                temp3.Add("l");
                break;
            }

            hold = hold + y;
            if (hold.Equals("l0"))
            {
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + hold + " = " + "0ll" + "\n";
                temp3.Add("0ll");
                hold = "";
            }
            if (hold.Equals("0") || hold.Equals("ll"))
            {
                if (hold.Equals("ll"))
                {
                    TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + hold + " = " + hold + "\n";
                    temp3.Add("l");
                    hold = "l";
                }
                else
                {
                    TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + hold + " = " + hold + "\n";
                    temp3.Add(hold);
                    hold = "";
                }

            }
        }
        if (value.Count == 3 && hold.Equals("l"))
        {
            temp3.Add("l");
            temp3.Add("l");
        }
        else if (value.Count == 3 || hold.Equals("l"))
        {
            temp3.Add("l");

        }

        TxtToDisPlay = TxtToDisPlay + "\n\nMajor Step 3 : get rid of the 0 from  userInput = " + string.Join("", temp3) + " \n\n";
        var answer = new List();
        foreach (var x in temp3)
        {
            if (x.Contains("0ll"))
            {
                foreach (var y in x.Where(y => y.Equals('l')))
                {
                    answer.Add("l");
                    TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + "x = l so " + "Add l To new Array result = " + string.Join("", answer) + "\n";
                }
            }
            else if (x.Contains("l"))
            {
                TxtToDisPlay = TxtToDisPlay + "Step " + count + ": " + "Add l To new Array result = " + string.Join("", answer) + "\n";
                answer.Add(x);
            }
            count++;
        }

        return answer;
        }
\$\endgroup\$

closed as off-topic by mjolka, Legato, mleyfman, BCdotWEB, Ben Aaronson Apr 20 '15 at 8:27

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – mjolka, Legato, mleyfman, BCdotWEB, Ben Aaronson
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Could you perhaps provide a plain-English description of the "algorithm"? \$\endgroup\$ – nhgrif Apr 20 '15 at 1:17
  • \$\begingroup\$ i add the plain version of the algorithm \$\endgroup\$ – DacorieS Apr 20 '15 at 1:56
  • \$\begingroup\$ As far as I can tell, this code does not work as intended. For example, BinaryToUnary(new List<string> { "1", "0", "0", "0" }) returns 1, 1, 1 when it should return 1, 1, 1, 1, 1, 1, 1, 1. \$\endgroup\$ – mjolka Apr 20 '15 at 2:24
  • 1
    \$\begingroup\$ A good explanation of the algorithm is here en.wikipedia.org/wiki/Markov_algorithm. I would use a string and not a list since then you can search for substrings easily. You only need one loop. In each iteration you check if any pattern is contained in the string and if so you replace it according to the rules. \$\endgroup\$ – SpiderPig Apr 20 '15 at 3:15
1
\$\begingroup\$

You are not supposed to replace all matches but only the first one. And then repeat the process.

Here is some javascript code that implements the markov algorithm exactly the way it is described on the wikipedia page. Now all you have to do is translate that to C#.

function binaryToUnary(value) {
  var matchFound = false,
      rules = [["|0", "0||"],
               ["1",  "0|" ],
               ["0",  ""   ]];
  do {
    matchFound = false;
    for(var i = 0; i < rules.length; i++) {
      var rule = rules[i],
          pattern = rule[0], replacement = rule[1],
          index = value.indexOf(pattern);
      if(index >= 0) {
        value = value.substring(0, index) +
                replacement +
                value.substring(index + pattern.length);
        matchFound = true;
        break; //break because only the first pattern that matches should be replaced
      }
    }
  } while(matchFound);
  return value;
}
\$\endgroup\$

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