-
Notifications
You must be signed in to change notification settings - Fork 10
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
The swap method is inconsistent and buggy #22
Comments
There's only one documented way to do a swap. Here's a quick example of using swap with an array: var myArray = [ 'bread', 'bacon', 'lettuce', 'tomato', 'bread' ];
var myArrayTome = Tome.conjure(myArray);
myArrayTome.swap(1, myArrayTome[2]); I can throw some more informative exceptions when the syntax is not matched if that will help. |
Right. The swap method does seem to have fallback logic for the single argument case. The idea is nice, but doesn't actually work right now (as the code implies it should). Taking out this fallback mechanism would be an acceptable way to fix that I guess. |
It's just that when we tried to read the code with @ronkorving it seemed to support some other ways to do it. I guess yeah, removing the extra code (like when you only provide a key without target) and changing the errors would work nicely. Would be nice to support |
A array.swap(key1, key2) or array.swap(val1, val2) could be convenient so I pushed a swap modification than make as this. |
When using the
swap
method between 2 elements of an array, the only way it works is by calling:myArrayTome.swap(someKey, someValue)
and it swaps the items at keysomeKey
with the valuesomeValue
, but when trying to domyArrayTome.swap(someKey, someOtherKey)
ormyArrayTome.swap(someValue, someOtherValue)
we start getting either exceptions or crashes.Moreover, calling
myValue.swap(someOtherValue)
also crashes.The text was updated successfully, but these errors were encountered: