alexbol99 / flatten-interval-tree

Interval binary search tree
MIT License
41 stars 21 forks source link

v0.3 return intervals as search response #7

Closed valoricDe closed 5 years ago

valoricDe commented 5 years ago

In my implementation I need not only the value of a interval but also the low, high offsets. Because I'm doing something like:

   const nodes = useInternalSearch(tree, offset, rangeHigh)

    nodes.forEach((node) => {
      const { value: { marks }, key, key: { low, high } } = node.item

      if (offset <= low && rangeHigh >= high) {
        // one block
        marks.push(getTextMarkFromRangeStyle(range))
        return
      }

Therefore I used the internal method tree_search_interval in:

function useInternalSearch (tree, low, high) {
  if (!tree.root) return []

  const resultNodes = []
  tree.tree_search_interval(tree.root, new Node([low, high]), resultNodes)
  return resultNodes
}

But therefore I need the Node class to be exposed.

So for my usecase I either need the Node class or a search function which take low, high and results in [low, high, value] pairs or simply both ^^,

alexbol99 commented 5 years ago

Hi, Velten @valoricDe

I don't want to make breaking change, some people use this method in the way it is implemented now. What do you think about adding a callback as second parameter to the search method? Callback will accept key,value as parameters and will enable to transform them to whatever user wants, and the search method will push the result of the callback into array of the search results. For example:

const searchResult = tree.search(interval, (key,value) => {return {key,value}})

So searchResult will be an array of {key,value} pairs.

Best regards, Alex

valoricDe commented 5 years ago

I also wouldn't introduce a breaking change I thought exposing a function like searchNodes eg. would be sufficient. But your idea also sounds very good. Would you only expose the key and value or also the other information of a node, like left and right side of a node? I don't need it, but maybe others would?

alexbol99 commented 5 years ago

Hi, Velten @valoricDe I merged your pull request but I changed mapping function to accept only (value,key), not a Node. I think it is more applicable. Otherwise user will have to know that he need to extract item from the node, and then key and value from the item, it is obsolete imho. Sorry it you will have to change your code because of that. Best regards and thank you for your collaboration. Alex

valoricDe commented 5 years ago

No problem. As I said I only need the key and value for now. Thx for merging and deploying