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

add haxe.EitherType (see HaxeFoundation/hxnodejs#15) #3465

Merged
merged 1 commit into from
Oct 10, 2014

Conversation

nadako
Copy link
Member

@nadako nadako commented Oct 10, 2014

No description provided.

@nadako
Copy link
Member Author

nadako commented Oct 10, 2014

@waneck proposes adding left:T1 and right:T2 methods/properties to the abstract so one could extract the value of proper type without explicit casting. Should we add that? I think this could be quite useful.

@Gama11
Copy link
Member

Gama11 commented Oct 10, 2014

Wouldn't first and second be more appropriate? There's not really anything that indicates a "direction" here, but via T1 and T2 the types basically already have a number.

@andyli
Copy link
Member

andyli commented Oct 10, 2014

About left/right prop, I'm not sure if it is a good idea, since I think casting would be more readable.

var value = someFunction().left; //what is the type of value?
var value:Int = someFunction(); //ok, it is an Int

//nested EitherType
var value = someFunction2().left.left.right; //what?

Also since this is mainly used for extern, and nowadays we used to generate them from doc. It is possible the order of T1 and T2 are inconsistent.

@:overload(function(v:Int):EitherType<Int, String>{})
public function fun():EitherType<String, Int>;
/* ... */
$type(fun().left) //String
$type(fun(123).left) //Int

@nadako
Copy link
Member Author

nadako commented Oct 10, 2014

Hmm, that's a valid argument, @andyli. I thought simply using left/right would be cleaner, but now looking at the actual code I see that it just causes confusion, especially with nested types.

nadako added a commit that referenced this pull request Oct 10, 2014
@nadako nadako merged commit 4695407 into HaxeFoundation:development Oct 10, 2014
@nadako
Copy link
Member Author

nadako commented Oct 10, 2014

Merging it as is.

@nadako nadako deleted the eithertype branch October 10, 2014 20:01
@ncannasse
Copy link
Member

Could we shorten it to Either only ?

@nadako
Copy link
Member Author

nadako commented Oct 10, 2014

@frabbit
Copy link
Member

frabbit commented Oct 11, 2014

Couldn't we move it in a subpackage like 'haxe.externs' to make clear that its a helper class for externs? Btw. I like the name Choice for it.

@Simn
Copy link
Member

Simn commented Oct 11, 2014

That's an interesting idea, it could live next to haxe.Rest and perhaps even ArrayAccess.

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

Successfully merging this pull request may close these issues.

6 participants