{"action":"created","issue":{"url":"https://api.github.com/repos/hamstergem/hamster/issues/246","repository_url":"https://api.github.com/repos/hamstergem/hamster","labels_url":"https://api.github.com/repos/hamstergem/hamster/issues/246/labels{/name}","comments_url":"https://api.github.com/repos/hamstergem/hamster/issues/246/comments","events_url":"https://api.github.com/repos/hamstergem/hamster/issues/246/events","html_url":"https://github.com/hamstergem/hamster/issues/246","id":379564099,"node_id":"MDU6SXNzdWUzNzk1NjQwOTk=","number":246,"title":"<=> is not the same as ==","user":{"login":"tdreyno","id":233,"node_id":"MDQ6VXNlcjIzMw==","avatar_url":"https://avatars3.githubusercontent.com/u/233?v=4","gravatar_id":"","url":"https://api.github.com/users/tdreyno","html_url":"https://github.com/tdreyno","followers_url":"https://api.github.com/users/tdreyno/followers","following_url":"https://api.github.com/users/tdreyno/following{/other_user}","gists_url":"https://api.github.com/users/tdreyno/gists{/gist_id}","starred_url":"https://api.github.com/users/tdreyno/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tdreyno/subscriptions","organizations_url":"https://api.github.com/users/tdreyno/orgs","repos_url":"https://api.github.com/users/tdreyno/repos","events_url":"https://api.github.com/users/tdreyno/events{/privacy}","received_events_url":"https://api.github.com/users/tdreyno/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-11-11T21:15:39Z","updated_at":"2018-11-16T18:49:19Z","closed_at":null,"author_association":"NONE","body":"When using a `SortedSet`, there is a method called `uniq_by_comparator!`: https://github.com/hamstergem/hamster/blob/core/lib/hamster/sorted_set.rb#L88-L103\r\n\r\nIt assumes that items with the same sort position, `a <=> b == 0`, are actually the same object. That's not true. This causes `SortedSet`s with non-unique sort orders to not work correctly.\r\n\r\nImagine a comparator of `.size` where we are sorting on the length of some class. If the size is ever the same, they will be considered equal by Hamster.\r\n\r\nFor now, I'm working around this by adding `object_id` to my comparator, but that's definitely code smell:\r\n\r\n```\r\n      def <=>(other)\r\n        [self.priority, self.object_id] <=> [other.priority, other.object_id]\r\n      end\r\n```"},"comment":{"url":"https://api.github.com/repos/hamstergem/hamster/issues/comments/439489805","html_url":"https://github.com/hamstergem/hamster/issues/246#issuecomment-439489805","issue_url":"https://api.github.com/repos/hamstergem/hamster/issues/246","id":439489805,"node_id":"MDEyOklzc3VlQ29tbWVudDQzOTQ4OTgwNQ==","user":{"login":"alexdowad","id":1067359,"node_id":"MDQ6VXNlcjEwNjczNTk=","avatar_url":"https://avatars3.githubusercontent.com/u/1067359?v=4","gravatar_id":"","url":"https://api.github.com/users/alexdowad","html_url":"https://github.com/alexdowad","followers_url":"https://api.github.com/users/alexdowad/followers","following_url":"https://api.github.com/users/alexdowad/following{/other_user}","gists_url":"https://api.github.com/users/alexdowad/gists{/gist_id}","starred_url":"https://api.github.com/users/alexdowad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexdowad/subscriptions","organizations_url":"https://api.github.com/users/alexdowad/orgs","repos_url":"https://api.github.com/users/alexdowad/repos","events_url":"https://api.github.com/users/alexdowad/events{/privacy}","received_events_url":"https://api.github.com/users/alexdowad/received_events","type":"User","site_admin":false},"created_at":"2018-11-16T18:49:19Z","updated_at":"2018-11-16T18:49:19Z","author_association":"CONTRIBUTOR","body":"Dear @tdreyno, your proposal is very interesting. Although the proposal\njust involves adding to the API, rather than changing the existing API, I\nwould like to be very sure that this is the right thing to do. (Once\nsomething is published to the world, it becomes difficult to change or\nremove it, for fear of breaking other people's code.)\n\nAs for why `Hamster::SortedSet` is fast, see:\nhttps://en.wikipedia.org/wiki/AVL_tree. Interestingly, while red-black\ntrees are a more commonly used type of balanced tree, and can easily be\nimplemented in an immutable, persistent manner, I found that AVL trees were\nactually better and faster for this application. For a mutable data\nstructure, using red-black trees reduces the cost of modifying the\nstructure, since they don't need to be rebalanced as frequently. But for an\nimmutable structure, that doesn't help. Actually, it hurts, since the path\nto the root needs to be copied on each modification, and maintaining strict\nbalance means the number of nodes copied for each such update is minimized.\n\nBack to the original topic, it would be interesting and helpful to see an\nimplementation of your proposed structure. If the final decision is not to\nadd it to the library, you could release it as a gem which extends the\nHamster namespace with an additional class.\n\nA simple (and not at all bad) implementation choice would be a class which\nholds both a Hamster::Set and a Hamster::Vector. Use the Set for checking\ninclusion, and the Vector for iteration and retrieving items by index. When\nitems are added or removed, update both the Set and the Vector.\n\nThanks!\n\nOn Wed, Nov 14, 2018 at 11:57 PM Thomas Reynolds <notifications@github.com>\nwrote:\n\n> The performance issue is a good one. I was actually digging through the\n> code to figure out how it was fast, while constantly sorting.\n>\n> What if, you made it an option. Give users a way of specifying Set\n> equality (defaulting to :== for Set and <=> for SortedSet), but with a\n> hook for those of us who would prefer to match the core SortedSet library\n> (but be immutable) and are willing to take the performance hit. My sets\n> have only a couple hundred items AND I'm using them to avoid disk I/O which\n> is way more costly for my use case.\n>\n> That said, my hack of sorting on multiple parameters (using object_id to\n> break the tie) also works.\n>\n> —\n> You are receiving this because you commented.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/hamstergem/hamster/issues/246#issuecomment-438833780>,\n> or mute the thread\n> <https://github.com/notifications/unsubscribe-auth/ABBJX_9XgZI3sSsuWKqZ9UWXQhMCC3Yrks5uvJHYgaJpZM4YYsKq>\n> .\n>\n"}}
{"actor":{"display_login":"alexdowad"}}