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 easy chaining of requests #4

Open
aabizri opened this issue May 10, 2017 · 2 comments
Open

Add easy chaining of requests #4

aabizri opened this issue May 10, 2017 · 2 comments

Comments

@aabizri
Copy link
Collaborator

aabizri commented May 10, 2017

Reasoning

Most resource endpoints (e.g ../stop_areas/{stop_area}) have sub-endpoints (e.g ../routes) following HATEOS principle. Currently you have to take the resource's ID and call a different method.

Possible solutions

We could simplify this these ways:

For results type having multiple results in them:

- Have a `func (r {Results}) Related() map[ID]map[string](func(context.Context,*Session) Results)` method on the results type, we'll have to create an exported interface `Results` for this to work intelligently.
- Have a single function `func Related(r {Results}) map[ID]map[string](func(context.Context, *Session) Results)` much like the method.

For results type having a single result in them:

- Have each `Results` type that returns only one result implement have an `Explore(ctx context.Context, s *Session, selector string)` method, allowing us easier chaining. (i.e `session.Coords(ctx, coords).Explore(ctx,session,"stop_areas")`).

There may be other ways, I'm open to suggestion !

@aabizri aabizri added this to the v1 release milestone May 10, 2017
@aabizri aabizri self-assigned this May 10, 2017
@adelcasse
Copy link

adelcasse commented May 15, 2017

What about something like that ? Ridygo@b50b197

Could be used like this :

scope := session.Scope(types.ID("default"))

params := navitia.PlacesNearbyRequest{
	Depth: 2,
	Distance: 500,
	Types: []string{"stop_area"},
}
coords := types.Coordinates{Latitude: dep.Coordinates[1].(float64), Longitude: dep.Coordinates[0].(float64)}

ctx := context.Background()

// Get stop areas near the departure point
res, err := scope.PlacesNearby(ctx, coords, params)
// fmt.Println(res)
if err != nil {
	fmt.Println(err)
	fmt.Println(res)
}

for _, place := range scope.RelatedContainers(res.Places) {

	opts := navitia.ExploreRequest{
		// ODTLevel: "scheduled",
	}

	// get lines in this stop area
	// lines, err2 := scope.ExploreResource(ctx, place.ID, "lines", opts)

	lines2, err3 := place.Explore(ctx, "lines", opts)
	fmt.Println("LINES :")
	fmt.Println(lines2)
	fmt.Println(err3)
}

(it's a little bit messy right now and needs to be implemented on more types than just Containers)

@aabizri
Copy link
Collaborator Author

aabizri commented May 18, 2017

I like this general direction, though I have a few questions:

On Results

So Results is a type holding common info for a future request, is that right ? In that case it might be better to name it Proposal or something like this.

Superficial note: a Scope already contains a *Session, no need to store both, and in case where both could be used, it is best to use Scope.

A question: is the separation between SingleValueResults and MultiValuesResults necessary ?

On RelatedContainer

Regarding the RelatedContainers: if I understand correctly this returns a slice of Results which then allows you to call Explore on each result. I like this concept very much, though as you say it needs to be cleaned up still.

On MultiValueResults.Explore

That, I don't understand what you're intending to do. My first impression is, as some XXXResults types can lead to different requests (for example Regions), then we need to be able to explore further per dataset for example, or per disruption. Is that what you're trying to solve ?

The way forward ?

First off, thanks for the input, that's certainly a better way to go about it than the proposed solution in the original post, which either requires the library to have a To{Type} method on results to work easily, or require the user to type-assert, which is bad.

Until your response, I'll see if I can think up something in the same vision as your proposal.

Thanks !

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

No branches or pull requests

2 participants