refactor(common): 3879 change return types for insert and remove in numerated intervals tree #4454
Conversation
grishasobol
added
A4-insubstantial
grishasobol
changed the title
…move-in-numeratedintervalstree
common/numerated/src/tree.rs
Outdated
| pub fn insert<I: Into<IntervalIterator<T>>>(&mut self, interval: I) { | ||
| /// | ||
| /// Returns: | ||
| /// - false: if `interval` is non-empty and for each `p` ∈ `interval` ⇒ `p` ∈ `self` |
There was a problem hiding this comment.
append info that it's before insertion
common/numerated/src/tree.rs
Outdated
| let Some((start, end)) = Self::into_start_end(interval) else { | ||
| // Empty interval - nothing to insert. | ||
| return; | ||
| return true; |
There was a problem hiding this comment.
I think (may be I'm not right) that it's simplier to define the return as:
Returns whether at least one point was added in `self`.
In that case we should return false here because of trying to add empty interval.
common/numerated/src/tree.rs
Outdated
| let Some((start, end)) = Self::into_start_end(interval) else { | ||
| // Empty interval - nothing to remove. | ||
| return; | ||
| return true; |
There was a problem hiding this comment.
The same as in insert - may be better to define return as:
Returns whether at least one point was removed from `self`.
There was a problem hiding this comment.
Or maybe better for both insert and remove:
Returns whether `self` has been changed.
There was a problem hiding this comment.
In core/src/memory.rs we will return InvalidFree error if remove method returns false:
if page < self.static_pages || page >= self.max_pages || !self.allocations.remove(page){
Err(AllocError::InvalidFree(page))
}But in my opinion empty free empty free is allowed (like free(NULL) in C). If it is not allows i will agree to change the functions behaviour.
There was a problem hiding this comment.
we also allow to remove empty interval. But return in that case that nothing has been removed . free in C does not return anything anyway
…atedintervalstree' of https://github.com/gear-tech/gear into 3879-change-return-types-for-insert-and-remove-in-numeratedintervalstree
Resolves #3879
insert: return false ifintervalis non-empty and for eachp∈interval⇒p∈self, In other cases - trueremove: return false ifintervalis not empty and for eachp∈interval⇒p∉self. In other cases - true@grishasobol