Skip to content

Complete Trees-2#1570

Open
paridhimalviya wants to merge 1 commit into
super30admin:masterfrom
paridhimalviya:master
Open

Complete Trees-2#1570
paridhimalviya wants to merge 1 commit into
super30admin:masterfrom
paridhimalviya:master

Conversation

@paridhimalviya
Copy link
Copy Markdown

@super30admin
Copy link
Copy Markdown
Owner

Strengths:

  • You correctly identified the approach of using a hash map to store inorder indices for O(1) lookups.
  • The recursive function correctly handles the construction of the tree by first building the right subtree and then the left subtree, which is essential for postorder traversal.
  • The base case for the recursion is correctly implemented.

Areas for improvement:

  • The postOrderIdx should not be a class property because it will be shared across multiple calls to buildTree. If the same instance is used to build multiple trees, the index will not reset correctly. Instead, you should pass the index by reference (e.g., using an inout parameter) or restructure the code to avoid state. Alternatively, you can make postOrderIdx a local variable and pass it by reference to the helper function.

  • The helper function constructTree should be marked as private to avoid exposing it as part of the public API.

  • The class name BinaryTreeFromInorderAndPostorder is descriptive but a bit verbose. Consider shortening it to something like Solution if that aligns with the problem context.

  • The submission includes an unrelated file RootToLeafSum.swift. For the purpose of this problem, only the relevant code should be submitted. Ensure that your solution is focused on the problem at hand.

  • In Swift, it's common to use lowerCamelCase for parameter names. The parameter inMap should be inMap (which it is), but inStart and inEnd should be inStart and inEnd to follow Swift naming conventions.

Revised Solution Suggestion:
To fix the issue with postOrderIdx, you can make it a local variable in buildTree and pass it as an inout parameter to the helper function. Alternatively, you can define the helper function inside buildTree to capture the index. Here's a revised version:

class Solution {
    func buildTree(_ inorder: [Int], _ postorder: [Int]) -> TreeNode? {
        var inMap = [Int: Int]()
        for (index, value) in inorder.enumerated() {
            inMap[value] = index
        }
        var postIndex = postorder.count - 1
        return buildTreeHelper(postorder, &postIndex, inMap, 0, inorder.count - 1)
    }
    
    private func buildTreeHelper(_ postorder: [Int], _ index: inout Int, _ inMap: [Int: Int], _ inStart: Int, _ inEnd: Int) -> TreeNode? {
        if inStart > inEnd { return nil }
        let rootVal = postorder[index]
        index -= 1
        let root = TreeNode(rootVal)
        let rootIndex = inMap[rootVal]!
        root.right = buildTreeHelper(postorder, &index, inMap, rootIndex + 1, inEnd)
        root.left = buildTreeHelper(postorder, &index, inMap, inStart, rootIndex - 1)
        return root
    }
}

@super30admin
Copy link
Copy Markdown
Owner

Your solution is on the right track and demonstrates a good understanding of the problem. You correctly used a hash map to store inorder indices for efficient root location, and you correctly traversed the postorder array from the end to the beginning. The recursion order (right subtree first, then left) is correct for postorder traversal.

However, there are a few issues to address:

  1. The TreeNode initializer you used does not match the provided definition. The provided TreeNode class does not have an initializer with named parameters. You should use TreeNode(rootVal) for the node without children, or TreeNode(rootVal, left, right) when specifying children. In your code, you are creating a node with no children, so you can simply use TreeNode(rootVal).

  2. The class-level variable postOrderIdx is reset in buildTree, which is good. However, consider that if the same instance is used to build multiple trees, the variable might be modified concurrently. While the problem does not require thread safety, it is generally better to avoid statefulness when possible. You could make the variable local to the method and pass it by reference (e.g., as an inout parameter) in Swift, or use a nested function that captures a variable. For example, you could define a local variable var postOrderIdx = postorder.count - 1 in buildTree and then have a nested function that uses and updates it.

  3. You included an unrelated file (RootToLeafSum.swift) in your submission. This file is not relevant to the current problem and should be removed to keep the solution focused.

  4. The function constructTree is well-named, but the parameters inStart and inEnd are clear. However, consider using more descriptive names like inorderStart and inorderEnd for clarity.

To fix the TreeNode initialization, change:

let root = TreeNode(val: rootVal, left: nil, right: nil)

to:

let root = TreeNode(rootVal)

Alternatively, if you want to explicitly set left and right to nil, you can use:

let root = TreeNode(rootVal, nil, nil)

But the first option is simpler.

Here is a revised version of your solution without the stateful variable (using a nested function):

class Solution {
    func buildTree(_ inorder: [Int], _ postorder: [Int]) -> TreeNode? {
        var inMap = [Int: Int]()
        for (index, value) in inorder.enumerated() {
            inMap[value] = index
        }
        var postOrderIdx = postorder.count - 1
        func constructTree(_ inStart: Int, _ inEnd: Int) -> TreeNode? {
            if inStart > inEnd { return nil }
            let rootVal = postorder[postOrderIdx]
            guard let rootIdx = inMap[rootVal] else { return nil }
            postOrderIdx -= 1
            let root = TreeNode(rootVal)
            root.right = constructTree(rootIdx + 1, inEnd)
            root.left = constructTree(inStart, rootIdx - 1)
            return root
        }
        return constructTree(0, inorder.count - 1)
    }
}

This avoids the class-level variable and uses a nested function to capture postOrderIdx and inMap.

Overall, your solution is almost correct but for the compilation error. With that fixed, it would work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants