Hot answers tagged

7

Import import java.util.*; The class does not need to import the whole java.util package. Instead it only needs: import java.util.AbstractList; import java.util.RandomAccess; The benefits are: avoid namespace collisions better readability, because you know the dependencies at a glance faster compilation Comments Variable Declaration private Node&...


6

Here are my thoughts: Don't mutate function parameters unless there is good reason to do so. node.depth = level; The above statement basically breaks the contract of the function. The function claims to locate the deepestNode, but in fact it does findDeepestNodeAndSetDeepestPropOnAllNodes (silly, but you get the idea). This could lead to confusing, subtle ...


6

Review I see no reason to have 3 constructors, each invoking the other, when two of them are private and only called by another constructor. Keep it simple. You check for if (Nodes == null) in the constructor. Is there a scenario where this reference could have already been created upfront? Constructor arguments should be camel cased. There is no point in ...


3

A class Solution with only public members can be written struct Solution. But with only a single member function (that never uses the this pointer), why not a free function? Presumably your vector is intended to be a std::vector, in which case we need #include <vector> and the correct namespace qualifier. Don't pass a reference to a mutable object ...


2

This is overall well-thought-out, well-organized, and well-written. There's at least one bug, and one oversight that I would consider a bug: In traverse your recursive call is passing the wrong second parameter: } else if (mode == Node.Traversal.DepthFirst) { callback(this); this.children.forEach(n => n.traverse(callback, false)); } should read }...


2

Review Your code is compact and well organised. Reusing traverse and reduce allows for easy extensibility. Your tree could be a graph, or even worse a cyclic one: traverse could iterate to infinity. By adding additional methods such as descendants and ancestors you could guard that the structure remains a tree. When adding a node, it cannot have a parent, ...


2

Firstly, use a PEP8 checker. It will raise a lot of issues about whitespace and also tell you to change if visited[k[0]-1] == False: to either if visited[k[0]-1] is False: or if not visited[k[0]-1]: The DFS is far more complicated than necessary: We don't need visited: given that we know that the graph is a tree, it ...


2

Use brackets for one-liner blocks (opinionated, but pretty strong consensus for this) Use shorthand object notation { val: val } -> { val } Use default value syntax Prefer const to let (this goes for your test too) Prefer ternary to if (not true if readability suffers) Let TreeNode take right and left as parameters module.exports = { deepestNode(node) { ...


2

Choosing the generic type (and parameters) The Fenwick tree stores cumulative values of its elements, and is based on the ability to compute the sum and difference of those elements. You made a very general implementation, where “sum” and “difference” are provided as closure parameters. But it cannot be ensured that these are proper inverse operations of ...


2

Specific suggestions: It is idiomatic to wrap the stuff after if __name__ == "__main__": in a main function. Rather than the generic data I would suggest figuring out exactly which information you want to attach to each Tree and creating fields for each of them rather than a fully generic list of stuff. This will make it much less painful to work with ...


1

Literature Since you never worked with a Tree structure before, I recommend reading about tree traversal strategies. Depth-First Breadth-First Order (bottom-up, top-down, in order) Bug Alert If you set the same value as is already set and the value is ITreeNodeAware<T>, it will end up with Node is null, while it should have the current instance set....


Only top voted, non community-wiki answers of a minimum length are eligible