Static types #228
Static types #228
Conversation
| XValue | ||
|
|
||
| ToString() XString | ||
| ToBool() XBool |
There was a problem hiding this comment.
ToString and ToBool are somewhat different to other conversions since every type should be able to render itself as these without blowing up
excellent/types/base.go
Outdated
| case XObject: | ||
| return x.Reduce().ToString() | ||
| } | ||
| panic(fmt.Sprintf("can't convert type %v to a string", value)) |
There was a problem hiding this comment.
using panics here because if those goes wrong we've coded something wrong - i.e. its more serious than giving the user back an XError
excellent/types/base.go
Outdated
| XTypeArray | ||
| XTypeObject | ||
| XTypeError | ||
| XTypeNil |
There was a problem hiding this comment.
Do we need XType anymore? This was mostly added for equality and we are really only supporting equality in excellent for bool and numeric types, so that's vastly simplified. Seems we could probably remove these and just use normal type assertions instead.
| @@ -41,51 +42,51 @@ var XFUNCTIONS = map[string]XFunction{ | |||
| "legacy_add": LegacyAdd, | |||
|
|
|||
| "round": Round, | |||
| "round_up": RoundUp, | |||
| "round_down": RoundDown, | |||
| "round_up": OneNumberFunction("round_up", RoundUp), | |||
There was a problem hiding this comment.
@nicpottier here's what functions look like using wrappers - can brainstorm if there's a better way to do this! Does suck having to repeat the function name so that's something to improve.
| lookup, err := types.ToString(v.env, v.Visit(ctx.Expression())) | ||
| if err != nil { | ||
| return err | ||
| context := toXValue(v.Visit(ctx.Atom())) |
There was a problem hiding this comment.
@nicpottier would be nice to replace these runtime checks with compile time checks
| vars["int1"] = 1 | ||
| vars["string1"] = "string1" | ||
| vars["int2"] = 2 | ||
| vars := types.NewXMap(map[string]types.XValue{ |
There was a problem hiding this comment.
unlike XArray, users can't create these but we use them internally
| } | ||
|
|
||
| // NewXResolveError creates a new XError when a key can't be resolved on an XResolvable | ||
| func NewXResolveError(resolvable XResolvable, key string) XError { |
There was a problem hiding this comment.
we could create several of these factory methods, so that errors of same type are created the same way. Then at some point we could add error codes or the like to make it easier for consumers like the simulator to localize these
excellent/types/conversions.go
Outdated
| arg1Str := fmt.Sprintf("%v", arg1) | ||
| arg2Str := fmt.Sprintf("%v", arg2) | ||
| // RequireMarshalToXString calls json.Marshal in the given value and panics in the case of an error | ||
| func RequireMarshalToXString(x interface{}) XString { |
There was a problem hiding this comment.
I like this, though I think the golang convention is must instead of require
There was a problem hiding this comment.
can change - copied Require from decimal.RequireFromString
There was a problem hiding this comment.
Oh, well maybe I'm wrong, maybe worth checking what is used more, no biggie either way.
There was a problem hiding this comment.
I think you're right about Must being more standard - and MustMarshalToXString sounds marginally better
Addresses: #215, #218 and #221