10
\$\begingroup\$

Rules: You should create a function that receives a string with brackets, parentheses, or curly brackets, an example:

test('{A}')

The function must check that the opening and closing of the brackets are correct. If they are correct it should return true, else it should return false.

Examples:

'{A}'   // true
'[A]'   // true
'(AB)'  // true
'({A})' // true
'((A))' // true
'[(A)]' // true
'[A}'   // false
'(A]'   // false
'({A])' // false
'([A))' // false

This is the code I did to resolve this problem:

const bracketList = ['{', '[', '('];


// This function return the reverse bracket. For example '[' returns ']';
const getReverseBracket = (bracket) => {
    if (bracket == '{') return '}';
    if (bracket == '[') return ']';
    if (bracket == '(') return ')';
}

// This function return the initial bracket. For example if the string starts with '{', then it returns '{'.
// In case the string does not start with any bracket, then return null
const starterBracket = (string) => {
    for (brak of bracketList) {
        if (string[0] == brak) return brak
    }

    return null
}

// This function is called when any starter bracket has been detected.

// if the string start with '(', then it checks if the reverse bracket is in the end of the string.
// If this is true, then returns true, else returns false
const bracketOk = (string, bracket) => {
    let reverseBracket = getReverseBracket(bracket);
    if (string.slice(-1) == reverseBracket) return true
    else return false;
}

// This function removes the brackets of the strings
const removeBrackets = (string, bracket) => {
    let reverseBracket = getReverseBracket(bracket);

    return string.replace(bracket, '').replace(reverseBracket, '');
}

// This function checks if the string contains any reverse or normal bracket. Depending of the result it return a number that means:
// ! 0: if the string contains reverse bracket but no Starter
// * 1: if the string contain starter bracket
// ? 2: if the string does not contain any bracket
const containAnyBracket = (string) => {
    const rbracketList = ['}', ']', ')'];

    for (brak of bracketList) {
        if (string.includes(brak)) return 1
    }

    for (rbrak of rbracketList) {
        if (string.includes(rbrak)) return 0
    }

    return 2;
}


// this function calls all other functions to find out if the string meets the requirements.
function testString(string) {
    let bracket = starterBracket(string)
    if (bracket) {
        if (bracketOk(string, bracket)) {
            let noBracketString = removeBrackets(string, bracket);

            let containBracket = containAnyBracket(noBracketString);
            if (containBracket == 2) return true;
            else if (containBracket == 1) {
                bracket = starterBracket(noBracketString);
                if (bracketOk(noBracketString, bracket)) return true;
                else return false;
            }
            else if (containBracket == 0) return false;
        } else return false;
    } else return false;
}

How would you improve the solution code?. I honestly think i could do it better. How you would solve this?

Do you have any advice for me about good practices?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ This is too simple to be an answer: you can replace getReverseBracket with a plain object that holds the opening bracket as the key, and the closing as the value, like this: const reverseBracket = {'(': ')'} (of course, you'd need to add all the missing mappings) \$\endgroup\$
    – jmm
    Nov 29, 2021 at 16:00
  • 1
    \$\begingroup\$ When you say for (name of list) {...}, you're creating a global variable called name (name is just a placeholder for your multiple times of doing this). Use for (const name of list) {...}. Another thing is you're inconsistent with your use of semicolons. In your bracketOk and containAnyBracket functions, you don't use ; after return statements, but you do everywhere else. In your getReverseBracket function, I would either make it else if for the second and third if, or make it a switch statement. \$\endgroup\$ Nov 29, 2021 at 16:44

1 Answer 1

11
\$\begingroup\$

Correctness

Your code falsely identifies (hi)there as having unbalanced brackets.

Formatting

The else return false clauses can be replaced with a single return false at the end of the function. There is no need to treat the 0 as a special case. testString() is not a very descriptive name. getReverseBracket() could be unecessary if you restructured the bracket data. The magic numbers would better be named constants, but they are not necessary anyway.

Alternative Solution

A stack is all you need to solve this. If you see an opening bracket, add to the stack. If you see the wrong closing bracket, fail. If you see the correct closing bracket, pop the stack. When you get all the way through the string, there should be no brackets left to close.

const bracketPairs = { '[':']', '{':'}', '(':')' }                              
const closingBrackets = new Set(Object.values(bracketPairs))                    
                                                                                
function bracketsAreBalanced(text) {                                            
    const open = [] // stack of (closing) brackets that need to be closed                       
    for (char of text) {                                                        
        if (closingBrackets.has(char)) {                                        
            if (char === open[open.length-1]) open.pop()                        
            else return false;                                                  
        }                                                                       
        if (char in bracketPairs) open.push(bracketPairs[char])                 
    }                                                                           
    return open.length === 0                                                    
}                                                                               
                                                                                
console.log(bracketsAreBalanced('(hi)there')) 
\$\endgroup\$
5
  • 7
    \$\begingroup\$ This is the classic problem for introducing stacks, either explicitly (in an imperative language) or implicitly (using tail recursion in a functional language, keeping information on the call stack). If the solution doesn't use a stack, it is pretty much guaranteed to be more complex than necessary. (In the simpler case where there is only one type of brackets, you don't even need the stack, you only need its depth.) So, I would argue that this is not just an alternative solution but THE solution. \$\endgroup\$ Nov 29, 2021 at 11:05
  • 1
    \$\begingroup\$ Why const open = []? You intend to modify it, so wouldn't var be more appropriate? \$\endgroup\$
    – Andrakis
    Nov 29, 2021 at 11:55
  • 6
    \$\begingroup\$ @Andrakis const doesn't mean you can't modify the object referenced by the variable, but rather that the reference will not change (e.g. open = [] later would be illegal). If it were meant to be a read-only object, you'd use Object.freeze for shallow modifications, or a read-only Proxy for deep modifications. \$\endgroup\$
    – phyrfox
    Nov 29, 2021 at 12:57
  • 3
    \$\begingroup\$ The content of the array is modified, but the variable won't be assigned a new array, const is more appropriate. \$\endgroup\$ Nov 29, 2021 at 13:04
  • \$\begingroup\$ Your code also has the issue that the original post has: global variables in the for loops and inconsistent use of semicolons. See my main comment on the post for more details \$\endgroup\$ Nov 29, 2021 at 16:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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