[SERVER-42081] Map helpers for strict use Created: 03/Jul/19 Updated: 10/Sep/19 Resolved: 10/Sep/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Benjamin Caimano (Inactive) | Assignee: | Billy Donahue |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | Dev Tools 2019-09-09, Dev Tools 2019-09-09, Dev Tools 2019-09-23 |
| Participants: |
| Description |
|
I recently ended up creating translation unit-static functions for more strict access patterns with stdx::unordered_map/_set. I suspect that a set of helpers with invariants might be more useful to the codebase at large:
This set of operations allows a developer to confidently use a map or set to contain a unique set of initialized objects. |
| Comments |
| Comment by Billy Donahue [ 10/Sep/19 ] |
|
My feeling on functions like this is that you're better off writing the code out where it's needed. I overhauled Google's util/gtl "helpers" collection and it was like this huge pile of fiddly little functions all with slightly different behaviors. In the end, C+11, particularly `unique_ptr`, `auto`, and `rvalue references`, left most of the helpers as being far more trouble than they were worth. They had caveats that caused a few bugs (like FindWithDefault's dangling reference on the `default` argument), and developers relied on them too much, and didn't develop their skills working with C+ containers as well as they should have. There are issues here like: What should be the exact signature of getOrInvariant? Is the key a `const Map::key_type&`? That ignores heterogeneous lookup, or hinted find. Pretty soon they might want countOrInvariant, equalRangeOrInvariant.... emplaceOrInvariant has more problems, the argument list is easier (M&, const_iterator, Args&&...). Does it take a hint? But then will we soon want emplaceHintOrInvariant(M&, K&, Args&&...), tryEmplaceOrInvariant(M&, const_iterator, Args&&...), insertOrInvariant(M&, const value_type&), and insertOrInvariant(M&, value_type&&) ? What happens if an exception is thrown while emplacing? Is that allowed to propagate out or is Invariant the only possibility if the emplace doesn't succeed. It gets to be a mouthful. When I'm reading code and encounter helper function, I have to think about the signature, return type, and possible caveats of the helper function. Whereas if I read straight container-manipulating code I just have to know the container API and a few idioms. In practice I think the manipulator functions are only saving about 2 or 3 lines of code, and you can't really be sure they were the only functions used to manipulate an arbitrary container. It will have back doors unless you encapsulate it and make the invariant enforcement part of the capsule, so then the capsule is the thing we'd need to design, and IMO it's very hard to do it in a way that anticipates the diversity in user requirements. Like, what's so special about `invariant`? Why isn't the "or" action customizable to other things? Okay how do we specify those other actions? You start to get into this maze of twisty little passages. I don't want to be down on this idea, but I have seen a very similar collection of algorithms at Google, and worked on an assignment for several months on paring it down (some things had 50k+ callers) and getting rid of it, and I just don't think anyone who looked at the problems up close was really convinced that the convenience of those helpers was worthwhile post-C++11. When I have a special map that needs special restrictions, I like to make a little class or derived type containing that map, and have it expose only those operations I want to allow. But making it a public type in a util/ directory is tough because you can't really account for all the variations the users will want and perhaps even expect it to have. My feeling is that this ticket should probably be closed. |
| Comment by Benjamin Caimano (Inactive) [ 23/Jul/19 ] |
|
Yeah, that would be fine by me. I've already caught one runtime bug with these helpers in the last week. |
| Comment by Billy Donahue [ 22/Jul/19 ] |
|
Maybe you need a facade type around the map to enforce this discipline for On Mon, Jul 22, 2019 at 2:42 PM Andrew Morrow (Jira) <jira@mongodb.org> |