andishgar commented on PR #46229: URL: https://github.com/apache/arrow/pull/46229#issuecomment-3094577874
@pitrou I apply two suggestions Two notes 1-Regarding [this comment](https://github.com/apache/arrow/pull/46229#discussion_r2208278290), my current algorithm for adding intervals even updates `interval.start`, so I believe the suggestion to use a `std::map` doesn't apply in this case. However, if you're still in favor of using `std::map`, please let me know, and I’ll adjust the logic accordingly. 2- I use a lot of `if` statements in `TryFastInsert` for `MergeOrInsertInterval`, and the tests I wrote cover all the branches. However, if you believe the number of `if` statements should be reduced, it is possible, since most of them are independent of each other. That said, it's also possible to increase the number of `if` statements—for example, in every place I use `std::max`, it could be broken down into several `if`s. Another example is the "less than" case in `TryFastInsert`, which I mentioned in my comments. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
