haskell-graphql / graphql-api

Write type-safe GraphQL services in Haskell
BSD 3-Clause "New" or "Revised" License
406 stars 35 forks source link

Rename 'Values' to 'Const' #50

Closed jml closed 7 years ago

jml commented 7 years ago

Or "Constant". Or "Constants". We talked a bit about this in #19. The thing about our "Values" module is that it only refers to constants, not variables. This is a super useful distinction that becomes more obvious when looking at validation code.

Accompanying change should consider changing ToValue and FromValue to also use "constant" (or whatever) in the name.

jml commented 7 years ago

@teh thoughts?

teh commented 7 years ago

I'm still not convinced that's a good idea name-wise (like with literals). The distinguishing feature of Values IMO is that they are dynamically typed, not that they are constant?

I may be too old-school but for me constants and literals are values that are typed directly into the program code, e.g. like in http://www.cplusplus.com/doc/tutorial/constants/ - skimming the spec I think graphql thinks about constants similarly (e.g. in me(arg: string = "hello") "hello" is a const)

There are other traditions (like physics) where constants are forever-fixed values (like ℏ).

The spec also talks about "values" (input value, float values).

Maybe I'm misunderstanding what you are proposing?

jml commented 7 years ago

distinguishing feature of Values IMO is that they are dynamically typed

That distinguishes them from Haskell code, but not from the rest of the GraphQL AST.

constants and literals are values that are typed directly into the program code

Literals are that. Constants are more complicated.

e.g. 3.1416 is a literal. const float pi = 3.1416 defines a constant. pi is a constant.

graphql thinks about constants similarly (e.g. in me(arg: string = "hello") "hello" is a const)

Yes, but in the following, me(arg: string = $greeting), $greeting is a variable and also a value (both in spec terms -- https://facebook.github.io/graphql/#Value -- and in parser terms https://github.com/jml/graphql-api/blob/master/src/GraphQL/Internal/AST.hs#L189). When we do variable substitution, it becomes me(arg: string = "hello"), where "hello" is a non-variable value. Having a term for "values that aren't variables" that isn't "Value" would be very useful. GValue.Value is definitely not self-explanatory.

Note also that the spec uses "Const" as a qualifier on the kinds of Value, and uses that qualifier to restrict what a Default value can be (see https://facebook.github.io/graphql/#DefaultValue).

All that said, I don't want to convince you against your will. If you find a proposed name misleading, we shouldn't use it.

teh commented 7 years ago

From reading I'm not sure we're talking about the same issue but I'm also not sure where the misunderstanding happens. I'm thinking about e.g. BooleanValue here - would you rename that to BooleanConst?

Maybe let's do this IRL :)

jml commented 7 years ago

I'm talking about renaming:

jml commented 7 years ago

66 does enough of this to keep me happy.