-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port is_subset toTreeMap
#112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit about indentation, otherwise looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I added a shorter version, but I'd be fine with what you have.
for (k, v) in self.map.iter() { | ||
if v.is_empty() { | ||
continue; | ||
} | ||
match other.map.get(k) { | ||
None => return false, | ||
Some(other_v) => { | ||
if !v.is_subset(other_v) { | ||
return false; | ||
} | ||
} | ||
} | ||
} | ||
true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on how you feel, noting that this could be written more shortly as
self.map.iter().all(|(key, lhs)| {
lhs.is_empty() || other.map.get(key).map_or(false, |rhs| lhs.is_subset(rhs))
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if it's accepted, let's merge it?
Thanks for the contribution @ariesdevil! This will be part of 1.0 containing some future-proof, yet breaking changes. |
Port
is_subset
toTreeMap