Market Actor v3 #1010
Market Actor v3 #1010
Conversation
| self.0.delete(key)?; | ||
|
|
||
| Ok(()) | ||
| pub fn delete(&mut self, key: &[u8]) -> Result<Option<()>, Error> { |
There was a problem hiding this comment.
Option<()> can definitely be simplified to be a bool, can't think of a benefit to having it as an option
There was a problem hiding this comment.
I like the option because then we can do the method chaining when handling the error instead of checking if its Some(false). What do you think?
There was a problem hiding this comment.
Then what is the need to erase the return in the function? This may lead to having to do unnecessary work if you need to retrieve the value first?
There was a problem hiding this comment.
Good catch. gunna return the byteskey
There was a problem hiding this comment.
Actually... Does that make sense? we should already have the key.... The value of the underlying map is always () because this is a Set that's backed by a hamt
| self.0.delete(key)?; | ||
|
|
||
| Ok(()) | ||
| pub fn delete(&mut self, key: &[u8]) -> Result<Option<()>, Error> { |
There was a problem hiding this comment.
Then what is the need to erase the return in the function? This may lead to having to do unnecessary work if you need to retrieve the value first?
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links