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

feat:implement local station finder based on google:s2 indexer #278

Merged
merged 3 commits into from
Apr 14, 2020

Conversation

CodeBear801
Copy link

Issue

#240

Major changes

  • change the name of tnsearchfinder to cloudfinder
  • implement local station finder based on google:s2 indexer

Minor changes

  • change name of NewTnSearchStationFinder to New
  • add protection code in go func() in unit test of orig_station_finder_test.go
  • change return value of IterateNearbyStations() from <-chan stationfindertype.ChargeStationInfo to <-chan *stationfindertype.ChargeStationInfo, original strategy is easy to create bugs while convert channel result into pointers https://play.golang.org/p/gWgJpEOSuj6

To do

  • Change spatialindexer.PointInfo to spatialindexer.PlaceInfo
  • Replace spatialindexer.Location to nav.Location
  • Integrate Stop functionality into Station Finder's algorithms

Tasklist

  • update relevant code
  • add tests
  • review

@CodeBear801 CodeBear801 added NewFeature New feature or feature improvement Prototype Proof of concept labels Apr 13, 2020
@CodeBear801 CodeBear801 self-assigned this Apr 13, 2020
@CodeBear801
Copy link
Author

Seems there is something wrong with github's actions, CI check also failed for clean branch

image

@wangyoucao577
Copy link

Seems there is something wrong with github's actions, CI check also failed for clean branch

image

Poor Github!!! Has re-run the jobs, let's see whether it's ok now.
image

-1)
sf.bf.getNearbyChargeStations(req)
return
}

func (sf *lowEnergyLocationStationFinder) IterateNearbyStations() <-chan stationfindertype.ChargeStationInfo {
func (sf *lowEnergyLocationStationFinder) IterateNearbyStations() <-chan *stationfindertype.ChargeStationInfo {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: use l instead of sf might be better as convention.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy that, use l or meaningful name.

var r []stationfindertype.ChargeStationInfo
var r []*stationfindertype.ChargeStationInfo

isdoneC := make(chan bool)
go func() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not remove go func(), go with the for loop directly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will go with that.

return nil, err
}

switch finderType {

case TNSearchFinder:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unify name?

)

// MockChargeStationInfo1 mocks array of *ChargeStationInfo which is compatible with spatialindexer.MockPlaceInfo1
var MockChargeStationInfo1 = []*ChargeStationInfo{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why export the mock var?

@@ -1,6 +1,7 @@
package spatialindexer

var mockPlaceInfo1 = []*PointInfo{
// MockPlaceInfo1 contains 10 PointInfo items
var MockPlaceInfo1 = []*PointInfo{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why export the mock var?

Copy link
Author

@CodeBear801 CodeBear801 Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to use which to evaluate result

package A defines some common data structures

package B use that common data structures and returns some result, I use package A's mock result to evaluate whether result is the same as expected.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the B will strongly depends on A, which may should not. Maybe you could write some test packages to test the scenario like this?

@CodeBear801 CodeBear801 merged commit 04d3930 into master Apr 14, 2020
@CodeBear801 CodeBear801 deleted the feature/implement-finder-by-localsearch2 branch April 14, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NewFeature New feature or feature improvement Prototype Proof of concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants