7
\$\begingroup\$

The below is the C# implementation for the a function for given parameters int a, int b - both represent characters 'A' and 'B' respectively, where the function should return a string containing both characters 'A' and 'B' occurring a times and b times respectively but neither 'A' nor 'B' repeating consecutively for more than 2 times. Both values for a and b are given in a way that a string can be build using those numbers - so eg: Foo(0,3) or Foo(1,7) shall not be invoked.

eg:

Foo(3,3) returns "BBAABA" or "AABBAB"

Foo(4,1) returns "AABAA"

Foo(3,5) returns "BAABBABB" or "BBAABBAB"

Code:

static string Foo(int a, int b)
{
    int total = a + b;
    StringBuilder sb = new StringBuilder();
    char charToPrint = a > b ? 'A' : 'B';
    int flag = 0;

    for(int x =0; x< total; x++)
    {
        if(flag == 2)
        {
            flag = 0;
            charToPrint = SwapChar(charToPrint);
        }
        if(charToPrint == 'A' && a == 0 || charToPrint == 'B' && b == 0)
        {
            charToPrint = SwapChar(charToPrint);
        }

        if (charToPrint == 'A')
            a--;
        else
            b--;

        sb.Append(charToPrint);
        flag++;
    }
    return sb.ToString();
}

static char SwapChar(char thisChar)
{
    return thisChar == 'A' ? 'B' : 'A';
}

This is working but I would like to receive some feedback on this code.

\$\endgroup\$
2
  • \$\begingroup\$ It looks like you've forgotten to include your SwapChar method. What is the code supposed to do when it is impossible to create a string with the requirements? E.g. Foo(0, 3) is impossible but your code returns "BBB" which violates the last rule. \$\endgroup\$ – RobH Aug 5 '20 at 15:16
  • \$\begingroup\$ @RobH added the SwapChar method. Btw, I just edited the question to include the Foo(0,3) shall be called. \$\endgroup\$ – Isham Mohamed Aug 5 '20 at 15:24
9
\$\begingroup\$

Your algorithm has a flaw - it's too greedy. Take this example:

Foo(6, 2) // "AABBAAAA"

Alas, it should return "AABAABAA".

In terms of a review, your method shouldn't be called Foo - give it a descriptive name.

Great use of StringBuilder. You could initialize the length to a+b in the constructor as you know what size your finished string will be.

Expression bodied members can greatly improve readability in my opinion:

static char SwapChar(char c) => 
    c == 'A' ? 'B' : 'A';

Try to avoid variables like flag. I had to read the whole loop before I knew what that variable meant. I'd call it something like repeatCount.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback and indicating the flaw \$\endgroup\$ – Isham Mohamed Aug 6 '20 at 2:54

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.