Differ between a compiled Indexand an IndexBuilder

[smyrman]

EDIT: The orginal issue was to return a concrete type from NewIndex. The issue is now about differing between an IndexBuilder and a compiled Index which serve different usage patterns and requirements. See comments for more details

---

Original idea

@rs, I am raising this as a potential enchantment only.

It appears to be an idiom in Go to return concrete types, and accept interfaces. resource.NewIndex() is breaking this idiom. I see the following benefits for correcting it:

• index.Compile() would no longer be a "hidden" function, so user would not have to type-cast the returned index to a Compiler instance if using the returned Index without the rest handlers.
• The documentation in editors (i.e. with go-to-definitions and auto-complete) would be able to display help-text when calling methods on the returned index.

These are not huge benefits, but if there is no obvious downside to the change, except for a limited change to the public API, it might be worth-while. There are also some consequences that could come out of how we fix this that we need to consider.

• resource.Resource implements the Index interface. Could/should Bind accept a version of the Index interface? (probably not)
• Where the Index interface is used, du we require all of the functions, or generally just the Getter functions? Should the interface be split?

Background: General information on the idiom

Some blogs on the subject:

• http://idiomaticgo.com/post/best-practice/accept-interfaces-return-structs/
• https://medium.com/@cep21/what-accept-interfaces-return-structs-means-in-go-2fe879e25ee8

TL;DR with some added personal experience:

Generally, returning the concrete type makes sense because:

• When we write a function, we always know what type it will return (when we don't, such as for errors, returning an interface still make sense).
• When we return the concrete type, users can access all public functions on the returned type.
• Documentation in auto-complete and go-to-definition is generally better.

Likewise, it makes sense to accept interfaces because:

• We allow for a wider usage of the API, as users may provide their own implementations, or wrapped implementations.
• We achieve better documentation, since the function declaration itself can document which methods we are interested in using on the passed in type.
• We allow for mocking during testing.

Sometimes these points doesn't match with our goals, and then it's probably correct to break the idiom.

[rs]

+1

[smyrman]

I have been thinking a little bit about this for a while, and I have just come to a perhaps slightly unexpected conclusion.

I think that an uncompiled Index and a compiled Index is not really the same thing, and does not have the same interface. I will collaborate a little-bit on this...

An uncompiled Index has the following use-case and properties:

• You WANT to be able to bind new resources against it.
• You WANT to eventually compile it.
• You DON'T want to be fetching any resources from it (except for during compilation, but that's internal, not external).
• You DON'T want to be fetching any actual resources items from it.

A compiled Index has the following use-cases and properties:

• You DON'T want to be able to bind new resources against it (you want it to be immutable).
• You DON'T want to compile it (again).
• You WANT to fetch resources from it.
• You WANT to fetch actual resources items from it.

The same can in principal be said about Resource and perhaps even Scheam and FieldValidator implementations, but we can set a line on how far we want to drag this to not create to much work to ourselves. For this issue at least, let's limit it to only touch the resource package.

Following from this, we could imagine we have the following types & interfaces:

• type IndexBuilder struct: An uncompiled resource index where you can bind resources and call Compile to return a new compiled index (as a concrete type that implements the Index interface). No interface is needed for IndexBuilder.
• type Index interface: A compiled index only allows resource lookup, no binding or compiling.

Optionally, we do the same for Resource, and define a ResourceBuilder type that contains the Bind and Compile methods. Either way, removing Bind from the Index inteface allows the Resource type to fully implement the Index interface, which could be very handy.

[smyrman]

PS! This design also completely guards against users using an uncompiled index by mistake, as doing so would now be a compiler error.

This of course also eases the documentation, as we no longer need to put warnings in the README alá:

if you use index without using rest.NewHandler or grahql.NewHandler, you must remember to Compile the index yourself.

[rs]

I like the idea.