patterncMinor
Matching square brackets
Viewed 0 times
matchingbracketssquare
Problem
I wrote this program to match square brackets as part of a Brainf**k interpreter. I'm aware that it gets caught in an infinite loop if brackets are entered like this:
```
//Matches an opening or closing bracket to a corresponding opening or closing bracket
int getMatchingBraceIndex(unsigned int braceIndex, char* str) {
//Current nesting level
int level = 0;
//Matching brace's nesting level
//(Matching braces have to be at the same level)
int returnLvl = -1;
/*****\
Note:
If brace given is open brace, iterate through string from beginning to end
If brace given is close brace, iterate through string from end to beginning
\*****/
//If brace given is open brace
if (str[braceIndex] == '[') {
//Iterator
int i = 0;
//Iterate through given string
for (i; i = 0; i--) {
//If current character of string is close brace
if (str[i] == ']') {
//Increment nesting level
level++;
//If current character is given brace, assume that matching brace is on the same nesting level
if (i == braceIndex) {
returnLvl = level;
}
//If current character of string is open brace
} else if (str[i] == '[') {
//If nesting level is nesting level of matching brace, return current index
if (level == returnLvl) {
return i;
//Otherwise, decrement nesting level and keep go
][.- Are there any performance improvements I can make?
- Should I change any variable names?
- Are there other syntax errors that will break the code?
- How can I fix said errors?
- Anything else I did wrong?
```
//Matches an opening or closing bracket to a corresponding opening or closing bracket
int getMatchingBraceIndex(unsigned int braceIndex, char* str) {
//Current nesting level
int level = 0;
//Matching brace's nesting level
//(Matching braces have to be at the same level)
int returnLvl = -1;
/*****\
Note:
If brace given is open brace, iterate through string from beginning to end
If brace given is close brace, iterate through string from end to beginning
\*****/
//If brace given is open brace
if (str[braceIndex] == '[') {
//Iterator
int i = 0;
//Iterate through given string
for (i; i = 0; i--) {
//If current character of string is close brace
if (str[i] == ']') {
//Increment nesting level
level++;
//If current character is given brace, assume that matching brace is on the same nesting level
if (i == braceIndex) {
returnLvl = level;
}
//If current character of string is open brace
} else if (str[i] == '[') {
//If nesting level is nesting level of matching brace, return current index
if (level == returnLvl) {
return i;
//Otherwise, decrement nesting level and keep go
Solution
The outer
This will also make it easier to determine if that code itself should be refactored because you'll just have one occurrence of it. Overall, putting duplicate code into a separate function helps with maintainability, readability, and DRY (Don't Repeat Yourself).
if/else if blocks have a lot of duplicate code. This should be put into a separate function to be called by both conditionals and given the relevant arguments.This will also make it easier to determine if that code itself should be refactored because you'll just have one occurrence of it. Overall, putting duplicate code into a separate function helps with maintainability, readability, and DRY (Don't Repeat Yourself).
Context
StackExchange Code Review Q#48205, answer score: 3
Revisions (0)
No revisions yet.