Update index.js #29
Update index.js #29
Conversation
The swap method has been updated, for be able of to swap two elements using the keys or the values, something as array.swap(key1, key2) or array.swap(someKey, someValue).
Now we can make:
var myArray = [ 'bread', 'bacon', 'lettuce', 'tomato', 'egg','cheese' ];
myArrayTome.swap(0,1);
myArrayTome.swap('cheese','egg');
myArrayTome.swap(1,'tomato');
myArrayTome.swap('bread',2);
|
Please make sure all the tests pass. If you add new behavior, please add appropriate tests for the new behavior. |
The swap.js test file is updated for to check the new swap method behavior. The swap method was updated without update the swap test file, so the swap test was launching validation errors.
|
I changed the swap method but I didn't change the swap test. Sorry. I made a new pull, changing only the swap test. |
|
We want to be able to swap 2 tomes by either referencing the key or the tome itself. We would like to be able to do all of the following: var myArray = [ 'bread', 'bacon', 'lettuce', 'tomato', 'egg', 'cheese' ];
myArrayTome = Tome.conjure(myArray);
myArrayTome.swap(0,1);
myArrayTome.swap(0, myArrayTome[1]);
myArrayTome.swap(myArrayTome[0], 1);
myArrayTome.swap(myArrayTome[0], myArrayTome[1]); |
|
Using the pushed swap method, I ran the next code for test (the console.log output is on the right): var myArray = [ 'bread', 'bacon', 'lettuce', 'tomato', 'egg','cheese' ]; myArrayTome.swap(0,1); var b = { b: { c: 1 }, d: { e: 1}, x: { y: 1}, y: { x: 1} }; b.swap('b','d'); You can use it over arrays and over objects. The matter can be when you have a 'b' key and a 'b' target. The pushed swap uses swap(arg1,arg2). |
|
Swapping tomes based on their values is very problematic. Case in point: var a = { b: 'a', c: 'a', d: 'b' };
var b = Tome.conjure(a);
b.swap('a', b.d);
console.log(JSON.stringify(b));
> {"b":"b","c":"a","d":"a"}I have no idea which tome I am going to swap b.d with, it could be b.b or b.c |
|
If there are many times the same value, swap checks the keys for to find the value and uses the first key. I agree with you about swapping values can be very problematic, in similar situations I prefer to use only keys. But after to read the issue #22, I see a logic about being able to make swap(key1,key2) or swap(value1,value2). I prefer don't use values, but If there is general agreement about this, the swap values capacity could remain and you uses "at your own risk". |
|
I don't want it to swap values, I only want it to swap keys and tomes. |
Swap method updated, for to allow only keys or Tomes.
Swap method has been updated for to use only keys or Tomes. Swap test file has been updated for to check than you can't use other kind of parameters, as values.
|
I modified the swap method for to swap only keys or tomes, and pushed the changes. To swap any combination of key/tomes is possible, but if you try to swap a value you will obtain a exception. |
|
Before I get into the actual code that you've prepared, could you format the whitespace to match the style of the whitespace in the project? While you're at it, see if there's not a more elegant way of doing this, multiple nested if statements are hard to follow. |
|
Yes, of course. |
Updated swap method, for for to eliminate the multiple nested if statements and to format the whitespaces.
|
Updated swap method, for for to eliminate the multiple nested if statements and to format the whitespaces. |
| var arg2IsKey = false; | ||
| var arg2IsTome = false; | ||
| var obj2; | ||
| var key2; |
There was a problem hiding this comment.
I think you're having a lot of indentation issues on this PR. This project uses single tab indentation.
|
I see that, but it is a surprise for me. I edit the file using gedit, using only tab for indentation , from the beginning of lines, no spaces. When I finish, I paste the text in Github using the "Edit" Github option over the file text. When I edit the file, I see the same indentation at my lines and at the other lines. And after to put the file in Github, it is the same. I see this. By default gedit is using 8 spaces for tabulation. Could this be the problem?. Should I change the tabulation spaces value?. At the styleguide there is not info about a indentation space value. |
|
That explains the title of your pull request :) Please set up git on your development machine and try to use git instead of pasting code into github. GitHub supports this, but it's really not meant for this kind of contribution. It's useful when making a one-character change in a Readme file for example, but not for code. You can find a free e-book on using Git here: http://git-scm.com/book |
|
Thank you very much, ronkorving. About git, I use it at the work. Push, branch.... no problem, but about to install and to configure a git repository, I made once looooooong time ago, and I forgot. I supposed than, for to edit node code and to push, a simple text editor and Git facilities were going to be the less problematic. Finally, it isn't so. I apologize for the inconvenience. I'll make to maky using git as soon as possible, but now I am having a pair of days very busy, so maybe it delays a bit. |
|
Finally, I updated the indentation and pushed using only git, not github. I was very busy with the classical very urgent project, so I couldn't make the changes before. I changed the push name to a more descriptive name, too. Now it is Swap method updated. |
The swap method has been updated, for be able of to swap two elements using the keys, the tomes, or any combination of them.
Now is possible to make:
var myArray = [ 'bread', 'bacon', 'lettuce', 'tomato', 'egg', 'cheese' ];
myArrayTome = Tome.conjure(myArray);
myArrayTome.swap(0, 1);
myArrayTome.swap(myArrayTome[0], myArrayTome[1]);
myArrayTome.swap(0, myArrayTome[1]);
myArrayTome.swap(myArrayTome[0], 1);