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 type to record internal solution #205

Merged
merged 2 commits into from
Mar 10, 2020

Conversation

CodeBear801
Copy link

@CodeBear801 CodeBear801 commented Mar 8, 2020

Issue

issue: #143
solutionformat implements internal data format for charge solutions, it isolate internal algorithm with external restful api. A function of dump2RestResult will be added to solution later.

Tasklist

  • code
  • review

@CodeBear801 CodeBear801 added the Prototype Proof of concept label Mar 8, 2020
- Internal algorithm will calculate result in this format.
- In the end, oasis will use this format to generate restful response.
*/
package solutionformat

Choose a reason for hiding this comment

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

I feel solution will be better, even it causes solution.Solution. How's your idea?

Copy link
Author

@CodeBear801 CodeBear801 Mar 9, 2020

Choose a reason for hiding this comment

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

I like shorter name.

My original thinking is, solution is better contains not only type definition but also solution logic, which is how those type is constructed, such as the calculation logic.

Currently, I put constructing logic inside stationgraph, in the end of its calculation it will generate result in solutionformat, which could be dumped to external format. That's what I try to limit scope with solutionformat or solutiontype

But as a reviewer, I think solution is more clear than solutionformat or solutiontype.

Copy link
Author

@CodeBear801 CodeBear801 Mar 9, 2020

Choose a reason for hiding this comment

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

The following comment I think is worth to record: Avoid to use package name as variable name, otherwise you will get strange errors.

After I rename package name from solutionformat to solution, there are some existing code use this package create a variable solution there. (this code is not in this pull, they exists in the pull request #203)

solution := &solution.Solution{}
solution.ChargeStations = make([]*solution.ChargeStation, 0)

I met following error:
image

I think compiler confuse on whether solution is a package name or a variable name. After I rename variable solution to sol, problem solved. But it took me a while to figure it out. I original thought it is related with package renaming.

@CodeBear801 CodeBear801 mentioned this pull request Mar 9, 2020
9 tasks
@CodeBear801 CodeBear801 merged commit a4c6369 into master Mar 10, 2020
@CodeBear801 CodeBear801 deleted the feature/oasis-integration2-patch2 branch March 10, 2020 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prototype Proof of concept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants