Conversation
codynhat
left a comment
codynhat
left a comment
There was a problem hiding this comment.
The overall structure of the package looks good. Additional test cases can be:
- A "base" price with custom prices overriding certain days
- A per mile/kilometer price for driving further than a specified distance
codynhat
left a comment
codynhat
left a comment
There was a problem hiding this comment.
Looking good! I think just adding some test assertions for the actual prices would be good. All these cases look good.
|
|
||
| const ligoAgreementState: LigoAgreementState = { | ||
| startOdometer: { | ||
| value: 1000, |
There was a problem hiding this comment.
This should include some specific distance unit. The case 2 PriceSpecification has a price per km, but what if the odometer reading is in miles?
There was a problem hiding this comment.
For km, this can just be
{
value: 1000,
unitCode: "KMT"
}
There was a problem hiding this comment.
Added "SMI" for Statute Mile.
There was a problem hiding this comment.
Can you add a test case for "SMI" as well? And also add either unit code to this agreement state?
There was a problem hiding this comment.
See my comment about the _totalPriceKM function. This LigoAgreementState needs to specify some unitCode and there should be test cases for both SMI and KMT
| return (await this._dateDiffInDays(a, b)) * _priceSpecification.price; | ||
| } | ||
|
|
||
| private async _totalPriceKM( |
There was a problem hiding this comment.
Is this function assuming that the unitCode for the startOdometer and endOdometer is the same as the PriceSpecification? This isn't a safe assumption. What should be done if the units are not the same?
To keep it simple, I think a per distance fee should only be charged if there is a PriceSpecification for the correct unit and leave it up to the host to published a per distance fee for each unit separately.
There was a problem hiding this comment.
I have added conversion for SMI to KMT. Let me know your thoughts on it.
2 participants
No description provided.