[skybomb0]

I was wondering what the best way to divide up code is. Here is an example bit of code that balances right parenthesis if there are too many left parenthesis:

string command = textBox.Text;
int leftOccurances = getInstances(command, "(");
int rightOccurances = getInstances(command, ")");

if(leftOccurances > rightOccurances)
{
	for(int i = 0; i < leftOccurances - rightOccurances; i++)
	{
		command += ")";
	}
	textBox.Text = command;
}

What is the best way to write this as far as style and readability go? Should I:

• Leave it as is
  
• Add comments above main lines
  
• Add comments to the end of main lines
  
• Put #region #endregion tags around it
  
• Move the code into a separate method with a descriptive name such as "balanceParenthesis"

How would you guys do it?

This post has been edited by skybomb0: 19 May 2010 - 04:52 PM

[Mion]

Personally I'd leave it mostly as is except : -

*a bit of commenting ont he lines above major code.
*possibly region tags around the appropriate things (variable definitions maybe?)
*Refactoring into a method of its own would also be a good move.

...

So I guess I'm not leaving it as is at all >.>

But really just usual generic good programming practices apply...

[PsychoCoder]

For personal preference I would change this bit of code

for(int i = 0; i < leftOccurances - rightOccurances; i++)
{
     command += ")";
}

To this

for(int i = 0; i < leftOccurances - rightOccurances; i++)
    command += ")";

Whenever there's just one act in a for loop (or even an if statement) I dont use curly braces, I just think it's more readable to me

[KYA]

I would move the subtraction in your conditional to just before the loop and assign its value to a temp variable. Then it's a simple comparison rather then a subtraction each iteration. Although there won't be any noticeable performance gains.

[Momerath]

I'd rewrite it as:

int diff = GetOccuranceCount(textBox.Text, "(") - GetOccuranceCount(textBox.Text, ")");
if (diff > 0) {
    textBox.Text = textBox.Text + String.Empty.PadLeft(diff, ')';
}

No loop, rename the method to be PascalCase and more descriptive

This post has been edited by Momerath: 19 May 2010 - 08:45 PM

[Riggs109]

I'd leave it as is, but I would add comments before any major code. I am the guy the comments everything so if someone else looks at my code they know exactly what is happening even if they don't know the particular language a piece of code is written in.

[MentalFloss]

It took me a minute to actually understand the point of this, but it's going to go something like this (correct me if I'm wrong):

Example Text:
(a(b(g BECOMES (a(b(g))).

Example Text:
a(b(g(d)) BECOMES a(b(g(d)))


Then, I would rewrite it. You will never have anything in between parenthesis it seems (now, if I'm wrong about this, then obviously the algorithm does change) so why bother with the extra nonsense?

• Get total count of left parentheses in string input.
  
• Get total count of right parenthesis in string input.
  
• Right pad the string with right parenthesis with Abs(left parenthesis count - right parenthesis count)
  
• Done.

Example:

a(b(g)d(e(f))

Total left parentheses: 4
Total right parentheses: 3
Required pad: 1

Right pad 1 parenthesis to string:

a(b(g)d(e(f)))


No loops required.

EDIT: C parenthesis turns into copyright notice...

EDIT 2: Just realized Momerath has already addressed exactly what I was trying to convey.

This post has been edited by MentalFloss: 19 May 2010 - 08:58 PM

[skybomb0]

Thanks for all the comments. So, from what I've heard, the best thing to do would be to:

• Alter the code so I can remove the loop
  
• Move the new code into a new method
  
• Add a comment (or method header?) ot explain the method

Thanks a lot for the suggestions guys!

[AdamSpeight2008]

Getting the delta between the number of occurrences of ( and ) doesn't indicate the parenthesis are balanced.
For example )(

[skybomb0]

This is true. But this section of code is just balancing the number of right parenthesis with the number of left parenthesis. Later in my full code, an error is returned if the string cannot be parsed such as an invalid combination of parenthesis.