Documentation and spec fixes in lists, queue, sets, ordsets and gb_sets#10844
Documentation and spec fixes in lists, queue, sets, ordsets and gb_sets#10844juhlig wants to merge 1 commit intoerlang:masterfrom
lists, queue, sets, ordsets and gb_sets#10844Conversation
* gb_sets * lists * ordsets * queue * sets
CT Test Results 2 files 99 suites 1h 9m 42s ⏱️ Results for commit cc9b295. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
|
|
||
| ```erlang | ||
| 1> S0 = gb_sets:from_ordset(lists:seq(1, 100)). | ||
| 1> S0 = gb_sets:from_list(lists:seq(1, 100)). |
There was a problem hiding this comment.
While the usage of from_ordset is not wrong here, it may be confusing to readers. Everybody knows from_list and can intuitively guess at what it does. OTOH, the usage of from_ordset assumes that the reader knows the assumption that the given list is sorted and free of duplicates.
| Returns an iterator over members of `Set` in the given `Order`, starting | ||
| from `Element` or, if absent, the first member that follows in the | ||
| iteration order, if any; see `next/1`. |
There was a problem hiding this comment.
I believe this is easier to read, and that the references to iterator/1,2 are not needed. Also, the passage saying "... the first element greater than or equal to..." in the original text was only correct for the ordered case, while in the reversed case it actually is "... the first element less than or equal to...".
| SetList :: [set(Element),...], | ||
| SetList :: [set(Element)], |
There was a problem hiding this comment.
union([]) is allowed and results in an empty set, so the argument list may be empty.
| This function is equivalent to `gb_sets:intersection(Set1, Set2) =:= []`, | ||
| This function is equivalent to `gb_sets:is_empty(gb_sets:intersection(Set1, Set2))`, |
There was a problem hiding this comment.
gb_sets:intersection will return a gb_set, not an empty list, so the comparison with [] will always be false even if the two sets are disjoint.
| gb_sets:from_list(lists:filtermap(Fun, Set1)). | ||
| gb_sets:from_list(lists:filtermap(Fun, gb_sets:to_list(Set1))). |
There was a problem hiding this comment.
Assuming that Set1 is a gb_set, lists:filtermap(Fun, Set1) would not work here.
| -spec nthtail(N, List) -> Tail when | ||
| N :: non_neg_integer(), | ||
| List :: [T,...], | ||
| List :: [T], |
There was a problem hiding this comment.
nthtail(0, []) works and returns [], so empty lists are actually allowed.
| ``` | ||
| """. | ||
| -spec new() -> []. | ||
| -spec new() -> ordset(none()). |
|
|
||
| This function is equivalent to `ordsets:intersection(Ordset1, Ordset2) | ||
| =:= []`, but faster. | ||
| This function is equivalent to `ordsets:is_empty(ordsets:intersection(Ordset1, Ordset2))`, but faster. |
There was a problem hiding this comment.
While not wrong and the representation backing ordsets is not opaque, I think using lists and ordsets ambiguosly should not be encouraged, but rather the usage of the respective API functions.
| Fun :: fun((Element1 :: T1) -> boolean | ({true, Element2 :: T2})), | ||
| Fun :: fun((Element1 :: T1) -> boolean() | {true, Element2 :: T2}), |
There was a problem hiding this comment.
boolean (atom) instead of boolean() (type)
| 4> Queue1 = queue:filtermap(fun (E) -> {true, E+100} end, Queue). | ||
| {"ihg","ef"} | ||
| 5> queue:to_list(Queue1). | ||
| "efghi | ||
| 4> Queue2 = queue:filtermap(fun (E) -> {true, E - 100} end, Queue). | ||
| {[-95,-96,-97],[-99,-98]} | ||
| 5> queue:to_list(Queue2). | ||
| [-99,-98,-97,-96,-95] |
There was a problem hiding this comment.
The result of the filtermap and subsequent to_list calls ended up prettified to "strings", which might be confusing to readers.
| - `from_list/1` | ||
| - `intersection/1` | ||
| - `intersection/2` | ||
| - `is_disjoint/2` |
There was a problem hiding this comment.
All set implementations export this function, with same semantics (sans matching vs comparing equal, as usual).
bjorng
changed the title
lists, queue, sets, ordests and gb_setslists, queue, sets, ordsets and gb_sets
bjorng
left a comment
bjorng
left a comment
There was a problem hiding this comment.
Thanks for your pull request.
2 participants
This PR contains a number of non-code fixes for the
lists,queue,sets,ordsetsandgb_setsmodules. See exemplary inline comments.