Skip to content
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

alternative to 'apply' method #135

Open
nyuichi opened this issue Apr 7, 2013 · 11 comments
Open

alternative to 'apply' method #135

nyuichi opened this issue Apr 7, 2013 · 11 comments

Comments

@nyuichi
Copy link
Member

nyuichi commented Apr 7, 2013

In JavaScript, we can push more than 2 values into an array by writing only one line using `Array.prototype.push.apply'.

var ary = [];
Array.prototype.push.apply(ary, [1,2,3]);

However there is no equivalent feature in JSX and we have to rewrite it into much more redundant code by using for statement and Array#push.
This is also the case with other methods in Array such as splice or unshift.
Do you have any plan to introduce new language feature to solve this problem?
For example, Python's star '*' operator is enough reasonable to be worth considering.

var values = [1,2,3];
ary.push(*values);
@gfx
Copy link
Member

gfx commented Apr 7, 2013

It seems good for performance, but how can we translate the star operator into JavaScript?
push(*values) is easy but how about splice(0, 1, *values)?

I suppose adding Array#push(values : T[]) as a special alias to Array.prototype.push.apply(a, values) looks good enough.

@nyuichi
Copy link
Member Author

nyuichi commented Apr 8, 2013

Performance is another problem. I think that's not what we have to disscuss here (because this proposal is only a draft, and the detail of its implementation is something the developers should decide when we adopt the issue).
But IMO, as you said, it's true that splice method will be compiled to much slower code. The output code will be like below:

Array.prototype.splice.apply(ary, [0, 1].concat(values));

Maybe some optimization techniques can be used and if neccesary it is possible to convert this expression into much more efficient code such as combination of for-statements or other engine-specific ways.

Regarding overloading:
I don't like the idea because,

  1. users have to write almost similar methods doubly when define native class interface.
  2. changing the API from JS is not good even if it looks trivial.
  3. it simply is hard to understand!

The reason of No.3 is that using overloaded 'push' against an array of an array may cause serious problems. Some dangerous codes which fails to be compile by far will pass the compiler check, and the caused bug will be so hard to find. That's not a very good scenario.

@kazuho
Copy link
Member

kazuho commented Apr 16, 2013

My understanding is that the issue is actually is a request for a destructive Array#concat. In other words, IMO it should better not be named as push or such.

How about adding a function with a different name that gets translated into a call to Array.prototype.push.apply?

var x = [ 1, 2 ];
x.pushElements([ 3, 4 ]); // x becomes [1,2,3,4]

We could also introduce Array#spliceElements and Array#unshiftElements as well.

@nyuichi
Copy link
Member Author

nyuichi commented Apr 16, 2013

My understanding is that the issue is actually is a request for a destructive Array#concat.

That's right about Array#push. But I do want the alternative version of Array#splice, too.

How about adding a function with a different name that gets translated into a call to Array.prototype.push.apply?

If you add some methods to Array class such as pushElements or spliceElements, it would be better to add a new utility class called 'ArrayUtilities'. This way would also be good for the compatibility to JS.

class ArrayUtilities {
    static function pushElements.<T> (ary : Array.<T>, values : Array.<T>) : Array.<T>;
    static function spliceElements.<T> (ary : Array.<T>, start : int, num : int, values : Array.<T>) : Array.<T>;
}

But this cannot be generalized to other native classes, so introducing star operator is still worth considering, I think. (Of course star operator is just an example.)

@gfx
Copy link
Member

gfx commented Apr 17, 2013

I agree with @wasabiz. Array-expand operator looks better because there are a lot of native methods which requires vararg, not only in built-in.jsx but in web.jsx.

@kazuho
Copy link
Member

kazuho commented Apr 17, 2013

@wasabiz @gfx
IMO the fact that certain number of native functions exist that might benefit from the extension does not necessarily mean that the language should be extended. Another approach would be to extend the native attribute so that it could be used to map JSX-level declarations to calls to Function.prototype.apply at the JavaScript emitter level (i.e. provide a syntax to map Array#pushElements to Array.prototype.push.apply).

How does it sound?

@nyuichi
Copy link
Member Author

nyuichi commented Apr 18, 2013

@kazuho I don't like the idea. I suppose the way of adding another version of variable-length methods automatically you suggested is not easy to understand. It is not good that something you don't explicitly write by the code is gonna happen. If you don't want to extend the language further more, why don't you introduce the new utility class? It won't solve the problem essentially but still is enough good for practical use.

@kazuho
Copy link
Member

kazuho commented Apr 19, 2013

@wasabiz
What I am saying is that it might be a good idea to extend the native attribute so that it would be possible to write like (I do not have any concrete idea about the syntax though):

class Array {
  ...
  native function pushElements(elements : Array.<T>) : void = "Array.prototype.push.apply($this, $1)"
}

I do not mind whether the function is defined as a static function of a utility class or as a member function of the Array class (although the former reminds me of the infamous System.arraycopy of Java).

Regarding your proposal, what is the exact behavior of the * operator. Is it to be allowed for positions where the formal arguments are "..." (i.e. variable length)?

@nyuichi
Copy link
Member Author

nyuichi commented Apr 22, 2013

@kazuho Well, I think your suggestion (extension of the native attibute) is aiming at something that can be extended for other functionalities as well as for alternatives to apply, but I can't come up with any ideas about such functionalities. If exists, extending native would be much better than mine.

Unlike Java, JSX has to follow the traditional convention made by JavaScript so far. Breaking the old API from JS is not a good idea, I think.

Regarding * operator:
You're right. It has so high readability, can be automatically deduced from the existing native class, and requires only one change to the language design. That's why I like the idea.

@kazuho
Copy link
Member

kazuho commented Apr 22, 2013

@wasabiz

I agree that the merit of introducing * operator is that it would be easy to understand for developers familiar with JavaScript.

Regarding * operator:
You're right. It has so high readability, can be automatically deduced from the existing native class, and requires only one change to the language design. That's why I like the idea.

Would it be possible to support creating a concatenated array in the form: [ *a, *b ] which gets compiled into [].concat(a, b)?

@kazuho
Copy link
Member

kazuho commented Apr 23, 2013

PS. IMO if we are to introduce "*" operator, then we should support [ *a, *b ] as well, since the meaning of the operator would be to splat an array into a list of homogeneously typed sequence (which is both the case of array initializer and variable length arguments of function calls).

Or else, I still think it is not a good idea to introduce a syntax-level extension for the purpose (since the scope is too narrow; my belief is that it is generally a bad thing to extend the language for tiny reasons). And in this case, introducing pushArray and alike (as discussed above), or supporting expressions like: a.push.apply(b) (like JavaScript but implicitly passes this object) could be the candidates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants