tamasvajk / SkyScanner

MIT License
17 stars 9 forks source link

Return intermediate results of queries #3

Closed tamasvajk closed 9 years ago

tamasvajk commented 9 years ago

We should be able to get the results of each ping in the Flight and Booking queries.

Some random ideas:

jochenvanwylick commented 9 years ago

@tamasvajk : Could you please review https://github.com/jochenvanwylick/SkyScanner/commit/674481890bdd3144a52316effa4295bb53422f48 ? I've added an event to ResponsePinger.cs that fires when an HTTP 200 is recieved but response.Succeeded == false. IScanner has an extra overload for the QueryFlight method: Task<List<Itinerary>> QueryFlight(FlightQuerySettings flightQuerySettings, Action<List<Itinerary>> callback);

I haven't looked into whether to emit deltas or not, I'm just experimenting with where to hook up the events.

Let me know what you think!

jochenvanwylick commented 9 years ago

Hey - I implemented your feedback: https://github.com/jochenvanwylick/SkyScanner/commit/b0b24c55137d01c63af249981ae533de3fbc7f2d Let me know what you think!

jochenvanwylick commented 9 years ago

@tamasvajk OK - I implemented your feedback. The callback that the caller provides now has the signature: private static Action<InterimItineraryCollection> TheCallBack(). This InterimItineraryCollection holds the total list of Itineraries, and makes the additions and updates explicit. E.g.:

private static Action<InterimItineraryCollection> WriteToDebug()
{            
    return list => Debug.WriteLine($"Interim results recieved ! {list.AllItineraries.Count()} "
                                + $"total itineraries, {list.NewItineraries.Count()} new "
                                + $"and {list.UpdatedItineraries.Count()} updates");
}

Does that make more sense?

tamasvajk commented 9 years ago

@jochenvanwylick Yep, I think so. One additional thought, the same interim result returning could be done for the booking service as well. Even if it is not implemented now, it might be worth making this collection generic, like InterimCollection<Itinerary>, so the public API of the flight service won't have to be changed whenever the booking service will be modified. Also InterimChangeSet<Itinerary> would probably be a better name. WDYT?

jochenvanwylick commented 9 years ago

Good one - will do. I'll prepare a pull-request after then, OK? And I can do the booking later ( maybe new issue ).

tamasvajk commented 9 years ago

Sure, thanks. Feel free to create the issue.

jochenvanwylick commented 9 years ago

@tamasvajk - this one can be closed, right?