SDK-116 Udate server interface: Add query params; use `Result` type from stdlib by matthewhammer · Pull Request #342 · caffeinelabs/motoko

matthewhammer · 2019-04-24T15:41:47Z

No description provided.

and instead of somehow creating code that can handle this unreachable case, simply loop. One could go a step further and never even add something of type `Non` to the environment, and simply loop (or `Unreachable`) that code. But for a quick fix this should work.

…ript into result-type

matthewhammer · 2019-04-24T17:04:23Z

Sorry for the messy commit history @paulyoung, I hit a snag with the compiler, now resolved.
I think I'm ready to merge this now, whenever you can take a look to review.

nomeata · 2019-04-24T17:11:34Z

Sorry for the messy commit history

If nobody else is building on your branch and the commit history is not worth preserving, consider doing a squash merge (manually, or with automerge-squash).

stdlib/examples/produce-exchange/serverModel.as

paulyoung · 2019-04-24T17:45:11Z

stdlib/examples/produce-exchange/serverActor.as

-  ) : async ?TruckTypeId {
-    getModel()
-      .truckTypeTable.addInfoGetId(
+  ) : async Result<TruckTypeId, ()> {


How about Result<TruckTypeId, None> ?

What's the intended meaning of None vs ()?

I just avoid None if I can, since it seems prone to issues (e.g., the recent compiler issue).

I think None in ActorScript is the equivalent of Void in other languages, meaning:

() a type with a single inhabitant

None a type with no inhabitants

To borrow from the PureScript docs for Void:

An uninhabited data type.

Void is useful to eliminate the possibility of a value being created.
For example, a value of type Either Void Boolean can never have
a Left value created in PureScript.

I don't see the advantage of ever using None over plain old () for a function that returns nothing.

Further, None seems confusing to me, since I always think that the meaning of None should be "inhabited" (no values of this type at all), not "returns nothing".

The distinction for me is that with None we can prove that there can never be an error. With () there can be an error but it can only error in one way (which can be useful for knowing that there was an error but you don't know or care why)

Ah, OK. Thanks for clarifying this is what you meant. Yeah, I see what you mean, and why you wanted None.

I added a commit to address this, hopefully to your satisfaction.

Maybe I misunderstood the intention here. Is the intention that there can be an error, but because we've created a Result from an Option there's no error info?

Just looking at optionUnwrapResult and seeing that we're dealing with an optional value, which suggests that this can fail.

...in which case I think () or some meaningful error type (representing an error code or message) is better after all.

@matthewhammer just wanted to follow up here in case we were doing the wrong thing

stdlib/examples/produce-exchange/serverModel.as

paulyoung · 2019-04-24T17:55:56Z

stdlib/result.as

+/**
+ `bind`
+ -------
+ bind operation in result monad.


I imagine we'll need map sooner rather than later too. Maybe even apply :)

I assume so as well; let's have them as needed

…lt-type

paulyoung

I think we need to separate out the changes for queries into another PR and merge that first, since the JS client doesn't know about variants yet.

Ideally we'd be able do some JS work in the app for querying without being blocked on changes to the JS client.

matthewhammer · 2019-04-24T18:49:14Z

I agree that it's unfortunate that this PR is doing more than one thing at a time.

Logically, it sort of makes some sense, though: Many of the queries have a new result type (for the error codes); one query has a couple of new parameters, to support the retailer query UI.

matthewhammer · 2019-04-24T18:50:05Z

(I'd rather not refactor this PR this time, if we can avoid it.)

paulyoung · 2019-04-24T18:55:44Z

That's fair enough. I think it makes sense not to separate the two then.

result-type

d1993a6

matthewhammer requested a review from paulyoung April 24, 2019 15:41

AS syntax error

62a5fc5

matthewhammer changed the title ~~SDK-116 Return Result<A,Error> on failure instead of null~~ WIP: SDK-116 Return Result<A,Error> on failure instead of null Apr 24, 2019

matthewhammer and others added 6 commits April 24, 2019 10:12

darn

cbbdc1e

compiler barfs on this input for some reason

ff1f9db

Merge branch 'joachim/calling-none' of github.com:dfinity-lab/actorsc…

41a29d5

…ript into result-type

thanks @nomeata

83ce93f

unfix compiler

e9cb6f9

matthewhammer added the automerge label Apr 24, 2019

optional parameters for produce and query window constraints

e1009d1

matthewhammer added automerge-squash When ready, merge (using squash) and removed automerge labels Apr 24, 2019

matthewhammer commented Apr 24, 2019

View reviewed changes

stdlib/examples/produce-exchange/serverModel.as Show resolved Hide resolved

matthewhammer commented Apr 24, 2019

View reviewed changes

stdlib/examples/produce-exchange/serverModel.as Outdated Show resolved Hide resolved

paulyoung reviewed Apr 24, 2019

View reviewed changes

stdlib/examples/produce-exchange/serverModel.as Outdated Show resolved Hide resolved

rename query params; use query date param

31fdc95

paulyoung reviewed Apr 24, 2019

View reviewed changes

Merge branch 'master' of github.com:dfinity-lab/actorscript into resu…

201f92b

…lt-type

paulyoung previously requested changes Apr 24, 2019

View reviewed changes

remove stale calls to isValidUser

001f57f

matthewhammer changed the title ~~WIP: SDK-116 Return Result<A,Error> on failure instead of null~~ WIP: SDK-116 Udate server interface: Add query params; use Result type from stdlib Apr 24, 2019

use Result<_,None> idiom

9da391e

Conversation

matthewhammer commented Apr 24, 2019

Uh oh!

matthewhammer commented Apr 24, 2019

Uh oh!

nomeata commented Apr 24, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulyoung Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewhammer Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulyoung left a comment

Choose a reason for hiding this comment

Uh oh!

matthewhammer commented Apr 24, 2019

Uh oh!

matthewhammer commented Apr 24, 2019

Uh oh!

paulyoung commented Apr 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paulyoung Apr 24, 2019 •

edited

Loading

matthewhammer Apr 24, 2019 •

edited

Loading