Move Icebnb to a geo-search model #3
Conversation
robertmogos
force-pushed
the
icebnb_geosearch
branch
from
1577e06 to
998e5ad
Compare
Icebnb/Icebnb/Constants.swift
Outdated
| } | ||
|
|
||
| struct Geo { | ||
| static let minimalDistance: Double = 250 |
| removeAnnotations(annotations) | ||
|
|
||
| let searchParams = userInfo["params"] as? SearchParameters | ||
| if searchParams == nil || searchParams?.page == 0 { |
There was a problem hiding this comment.
Probably an opportunity here to add some helper to check if there are search parameters or not. We can do a better developer experience for geo-search related features (we already discussed this live, maybe it's a good idea to add an Asana task
Icebnb/Icebnb/MapViewWidget.swift
Outdated
| import MapKit | ||
| import InstantSearch | ||
| import InstantSearchCore | ||
| import AlgoliaSearch |
There was a problem hiding this comment.
Would be best to hide AlgoliaSearch from developers (unless they really want to dig in deep). All "high level" functionalities should be surfaced from InstantSearch (stating the obvious I know...)
| // PRGRoundRangeSlider.swift | ||
| // PRGRoundSliderTest | ||
| // | ||
| // Created by John Spiropoulos on 20/03/2017. |
There was a problem hiding this comment.
I guess we're keeping those to give credit? not sure here
There was a problem hiding this comment.
Yes, it's for giving credit
|
|
||
| class RadiusController: UIViewController { | ||
| @IBOutlet weak var slider: PRGRoundSlider! | ||
| let searcher: Searcher |
There was a problem hiding this comment.
We're sure to have a searcher (or this component does not make sense). So you can force unwrap like you did for the slider
There was a problem hiding this comment.
No need for force unwrapping since it's initialised in the constructer
| slider.value = CGFloat((existingValue - Geo.startRadius) / (Geo.endRadius - Geo.startRadius)) | ||
| slider.messageForValue = { [weak searcher] value in | ||
| let meters = round(Geo.startRadius + Double(value) * (Geo.endRadius - Geo.startRadius)) | ||
| searcher?.params.aroundRadius = Query.AroundRadius.explicit(UInt(meters)) |
There was a problem hiding this comment.
remove the "?" after force unwrapping
There was a problem hiding this comment.
I passed a weak reference to the searcher in the closure, hence the ?.
Icebnb/Icebnb/RadiusController.swift
Outdated
| formatter.string(fromValue: meters, unit: .meter) | ||
| } | ||
|
|
||
| // Do any additional setup after loading the view. |
|
|
||
| override func viewWillDisappear(_ animated: Bool) { | ||
| super.viewWillDisappear(animated) | ||
| searcher.search() |
There was a problem hiding this comment.
isn't it better to load results everytime the radius finishes changing value, so that we can update the map in realtime?
| newLocation.distance(from: lastSearchLocation!) >= Geo.minimalDistance { | ||
| lastSearchLocation = locations.last | ||
| updateResultsWithLocation() | ||
| return |
No description provided.