Add analyzer design for jedliks toy car #3047
Conversation
xinri
force-pushed
the
add-analyzer-design-for-jedliks-toy-car
branch
3 times, most recently
from
71c5e76 to
3be4665
Compare
| - `essential`: Verify that the solution has the fields in the class for battery percentage and the driving distance. | ||
| - `essential`: Verify that the solution updates the variables in the drive method. | ||
| - `essential`: Verify that the solution is not updating values when the battery is empty. |
There was a problem hiding this comment.
essential: Verify that the solution has the fields in the class for battery percentage and the driving distance.
I don't think we should be insisting that there are specific fields for battery percentage and driving distance. To be honest, I'm not sure how reliably an analyzer implementation can tell which fields is for battery percentage and driving distance. Instead, I'd suggest simply checking that the class has fields.
essential: Verify that the solution updates the variables in the drive method.
Did you mean "updates the fields in the class"?
essential: Verify that the solution is not updating values when the battery is empty.
I don't think we need this one - I think the last two tests cover this.
There was a problem hiding this comment.
Sorry yes I mean fields.
There was a problem hiding this comment.
I don't think we should be insisting that there are specific fields for battery percentage and driving distance. To be honest, I'm not sure how reliably an analyzer implementation can tell which fields is for battery percentage and driving distance. Instead, I'd suggest simply checking that the class has fields.
mmmh ok. thanks for taking account the analyzer
| - `actionable`: If the solution defines the fields as `public`, instruct the student to use `private` and explain the encapsulation principle. | ||
| - `informative`: If the solution does not use a primitive as a type for the fields, inform the student to use it. | ||
| Explain that the values cannot be null and it is less error-prone | ||
| - `informative`: If the solution updates variables outside the drive function, instruct the student to move there. |
There was a problem hiding this comment.
I didn't quite understand what you mean by "move there". Is it checking that the fields are only changed in the drive method?
There was a problem hiding this comment.
I will remove this, I saw the unit tests that just create the instance with the new (and not the buy) and will test the battery and the distance. I thought perhaps the mentee can do a -- for battery or ++ for the distance when calling the function batterydisplay and distanceDisplay
xinri
force-pushed
the
add-analyzer-design-for-jedliks-toy-car
branch
from
3be4665 to
9834189
Compare
xinri
force-pushed
the
add-analyzer-design-for-jedliks-toy-car
branch
from
9834189 to
94b707f
Compare
|
Great, thank you very much for the review. I will try to check this analyzer ;) |
2 participants
pull request
Add the design.md file for the jedliks-toy-car for the analyzer.
Fixes: #2676
Reviewer Resources:
Track Policies