exercism / java-analyzer

GNU Affero General Public License v3.0
8 stars 14 forks source link

Need for Speed Exercise, Java Analyzer giving weird feedback #216

Open SabuWolf96 opened 2 weeks ago

SabuWolf96 commented 2 weeks ago

While Submiting my Solution the Analyzer gave me following feedback.

Instead of using a loop, consider returning a single expression using the <= operator. You can re-use the other methods already implemented in the class.

If this automated feedback doesn't look right, please open an issue in the exercism/java-analyzer repository.

I then looked at a good amount of Community Solutions and couldn't find any that don't use a loop similiar to my Solution. So for me it seems like theres a problem with the Analyzer, cause you can't simply return a single expression in the way the two Classes are settup right now.

My Solution for reference:

kahgoh commented 1 week ago

Actually, it is certainly possible to solve it without loops! It takes some maths - can you figure out how far the car can travel with the remaining battery? or perhaps how much battery do you need to finish?

The exercise has been through some changes. The early version encouraged using the loops and I believe the ones you saw are for the earlier version. It is also possible for people to still be submitting a version using loops (we don't stop people from doing that), but the newer submissions are more likely to use the maths approach and there are definitely some that uses the maths approach (such as this one and this one).

SabuWolf96 commented 1 week ago

Ok yeah with Math and using the fields directly with the use of a less restricted access modifier (public in the solutions you posted), you can use a single expression. I didn't look that deep into the Solutions then.

Still I wanna point out a thing in my original post.

This comes from the Analyzer message:

You can re-use the other methods already implemented in the class.

  1. The way it's setup or mainly expected, there are no Methods just Fields to access the required information needed

  2. I feel like encouraging, a public (or I think protected would also work) access modifier on the Fields to get the information is more hurting the Code Quality than encouraging the loop. I guess this could be fixed by pointing towards using getters, but that creates longer code... (Honestly don't know what's best here).

  3. The Code example in the Screenshot below (will add it in codeblock too) still leads to the believe that you have to let the car drive the required distance before checking if it can finsh the race. So some form of loop making the car drive seems to be intended.

image

int speed = 5;
int batteryDrain = 2;
var car = new NeedForSpeed(speed, batteryDrain);

int distance = 100;
var race = new RaceTrack(distance);

car.distanceDriven()
// => 0

race.canFinishRace(car);
// => true

car.distanceDriven()
// => 100

It's cool that thinking outside the box is encouraged, but I still believe that the feedback isn't necessarily correct.

kahgoh commented 1 week ago

Oh gosh, you're right the example isn't consistent. We shouldn't have distanceDriven() in the examples (although it might also be good to include another example where car can't finish).

The way it's setup or mainly expected, there are no Methods just Fields to access the required information needed

I think you're right... The analyzer comment says you can "reuse methods added earlier", but I don't think the the instructions guide the students to add all needed ones.

@manumafe98, I'd like to get your thoughts on this one.

manumafe98 commented 1 week ago

Hey there! good point, I agree that we have to update the following instructions and probably rephrase the following comment in case the user/student uses public fields without getters, currently this is our "ideal" solution with private fields and getters