Closed kotarCreative closed 8 years ago
:+1: on the changes, super happy to have more tests. What do you think about renaming join
? Once that's decided this is good to merge. :shipit:
I'm not sure what your complaint is about the naming of "join". I think "concatenate" could be a little misleading, though it's probably not really any worse than "join". One other clarification could be to use "with separator" as the label of the second slot, instead of just "with". Or perhaps "using separator".
@CelticMinstrel Whose complaint? @kotarCreative's complaint about join
for sticking an array together as a string, or my complaint about join
to concatenate arrays?
I was referring to your inline comment. I haven't seen any such complaint from kotarCreative.
OK. Part of the PR was renaming the join block (for joining an array of strings into one string) to make string
and renaming concatenate (to join two arrays into one array) to join
. I'm OK with the first one, but am afraid the second one will be confusing to anyone who actually knows JavaScript and is using WB.
Ah, I see what happened here. I missed that @kotarCreative has, for some reason, changed "join" to perform "concatenate" instead.
Concatenation is an operation on two arrays. The code for this is essentially correct, but it has been shoved in under the name "join" which, as @dethe has noted, will confuse people who are more used to programming in various languages (not limited to just JavaScript). Also, the variable name s
is used which suggests that it's a string variable, when in fact it's an array. At least the <wb-*>
markup does correctly define it though. I agree with @dethe that this should probably be called "concatenate" rather than "join".
Having removed the old join function, @kotarCreative then proceeded to add it back... with fewer features. This new "make string" block joins all the elements of the array into a string with no separator, while the old "join" block joins them with a specified separator. It's a step backwards in functionality.
I'm OK with renaming things to be easier to understand for folks coming into programming for the first time, as long as we're not making it inadvertently confusing for those with more experience. For instance, I'm fine with following Python's lead (and LISP's to some extent, except that LISP's lists were linked-lists, not arrays) and calling arrays "lists". I had missed that the separator was dropped, we should keep that.
I would have to say I agree with everything that was just discussed. I made some changes to the wording of the blocks and added the lost functionality that I had missed. Let me know what you think.
I think this looks good. :shipit:
"combine" instead of "join"? I still prefer "concatenate", but I guess this works.
Eventually I want to test all the language with non-CS folk to see what works best.
Concatenate is a very topic specific word where as combine is more general. It is also more legible when read as "combine array1 with array2". I feel like it is more approachable.
Added unit tests and fixed some of the array blocks so they are more readable for the new programmers.