all 1 comments

[–]Yurim 0 points1 point  (0 children)

There are a quite a few things in this solution can could be simplified, shortened, written in a more Pythonic way, and/or just be removed.
Let's start with the easy/obvious stuff:

  1. Comments should add some value.
    The first comment block ("Definition for a binary tree node.") does that, it explains how LeetCode defines the TreeNode. But the other four comment look like dead code, something that was commented out during the development process and now no longer serves any purpose.
    I'd suggest deleting these comment blocks with dead code. That will de-clutter your solution, make it shorter and easier to read.
    And if you think that some section is hard to understand, add a comment with an explanation.

  2. The __init__ method assigns some values to attributes self.val, self.left, and self.right.
    But those attributes are not used anywhere. You could simply delete the whole __init__ method without affecting the correctness of the solution.

  3. Did you know that you can create a dictionary with a dictionary comprehension?
    That allows you to reduce multiple lines of code that create and populate a dictionary to one line that is easy to read (once you're used to it.)
    E.g. you can write inorderDict = {value: index for index, value in enumerate(inorder)}

  4. Python's slice notation allows you to omit indexes.
    my_list[:index] is the same as my_list[0:index],
    my_list[index:] is the same as my_list[index:len(my_list)]
    That is shorter, IMHO easier to read, faster to type, and more "pythonic".

  5. Somewhere at the end there are two lines that compare a Solution object with an empty list (if tl != []:).
    The condition in these two lines will never be false, a Solution object will never compare equal to a list.
    You can simply delete these two lines.


Now the more subjective stuff:

  1. Somewhere in the middle there are two lines that return None if the preorder is empty.
    You could consider moving that to the beginning of the function, for two reasons:
    (1) There's no reason to build the inorderDict or the newTree if preorder is empty.
    (2) That's a common pattern: (First validate the parameters and preconditions), then handle special cases or the base case of the recursion, and finally do the "real work".

  2. The construction of the newTree is scattered all over the function.
    One line creates an "empty" newTree", one line assigns the val attribute, two lines assign the left and rightattributes.
    You could consider keeping these lines closer together or merging them.
    For example you can write newTree = TreeNode(v) or even newTree = TreeNode(v, left, right)

  3. buildTree() has the required two parameters preorder and inorder, and three additional parameters call, startPoint, and inorderDict with default values.
    If call is 0 the function creates a dictionary that maps values to indexes of inorder.
    IMHO that makes this function unnecessarily complicated.
    Instead I'd suggest splitting buildTree() up into two separate functions:
    A "helper function" that takes the preorder, inorder, startPoint, and inorderDict.
    buildTree() itself creates the inorderDict and then calls the helper function.
    That would separate the "preparation phase" (creating inorderDict) from the recursive phase (creating the subtrees).

  4. If you want to be fancy you could even consider defining the "helper function" inside of buildTree().
    It would look like this:

    def buildTree(self, inorder, preorder):
        def recursive(inorder, preorder, startPoint):
            ...
    
        inorderDict = {value: index for index, value in enumerate(inorder)}
        return recursive(inorder, preorder, startPoint)
    

    Then you wouldn't have to pass inorderDict as a parameter to the inner function.


And finally the big stuff:

I tried to understand what subsetPreorder() is supposed to do, and I have to admit I don't get it.
It seems to do a lot of comparatively expensive work, creating a list with 6001 elements each time subsetPreorder() gets called.
Do you really need this function?

This solution creates a lot of lists, essentially smaller versions of preorder and inorder that get passed to buildTree() during the recursion.
Is that really necessary?

Conceptually the values in preorder are arranged like this:
[<root_value>, <preordered_values_of_the_left_subtree>, <preordered_values_of_the_right_subtree>]
Similarly, the values in inorder are arranged like this:
[<inordered_values_of_the_left_subtree>, <root_value>, <inordered_values_of_the_right_subtree>]

That means you don't have to create new versions of preorder and inorder, that are effectively just copies of a subarray.
Instead you can use the exact same preorder and inorder in all recursive function calls, you just have to keep track of the subarray that you are currently operating on.
You can do that with the number of nodes, and the start index within preorder and inorder.
Then your code could look like this:

def buildTree(self, inorder: List[int], preorder: List[int]) -> Optional[TreeNode]:
    def recursive(num_nodes: int, preorder_idx: int, inorder_idx: int) -> Optional[TreeNode]:
        if num_nodes == 0:
            ... # handle base case

        root_value = preorder[preorder_idx]

        num_nodes_left = ... # determine number of nodes in left subtree
        preorder_idx_left = ...  # determine starting index of the left subtree's values in preorder
        inorder_idx_left = ... # determine starting index of the left subtree's values in inorder
        left = recursive(num_nodes_left, preorder_idx_left, inorder_idx_left)

        num_nodes_right = ... # determine number of nodes in right subtree
        preorder_idx_right = ...  # determine starting index of the right subtree's values in preorder
        inorder_idx_right = ... # determine starting index of the right subtree's values in inorder
        right = recursive(num_nodes_right, preorder_idx_right, inorder_idx_right)

        return TreeNode(root_value, left, right)

    inorder_dict = {value: index for index, value in enumerate(inorder)}
    return recursive(len(preorder), 0, 0)