Go Track - Need for Speed - suggestion for a test scenario

Hi all,

I’ve recently solved the Need for Speed challenge and realised it’s missing an important testing use-case for the CanFinish function.

The README #4 reads:

Assume that you are currently at the starting line of the race and start the engine of the car for the race. Take into account that the car’s battery might not necessarily be fully charged when starting the race:

Assume you’re currently at the starting line, but in the test scenarios, cars not always start at distance: 0.

{
	name: "Car can easily finish the track distance.",
	car: Car{
		speed:        5,
		batteryDrain: 2,
		battery:      100,
		distance:     20,
	},
	track: Track{
		distance: 30,
	},
	expected: true,
},

However, if the car.distance is not zero, that means the car is not in the starting line when this function is called. As it says “the starting line”, most of community solutions - mine included - did not consider the car.distance at all when implementing the function. But if the car could be anywhere in the track, then I suggest replacing the current instructions to something like this:

Assume that you are anywhere in the track, either the starting line or just before crossing the finish line.

And the test updated to:

{
	name: "Car can easily finish the track distance.",
	car: Car{
		speed:        5,
		batteryDrain: 5,
		battery:      10,
		distance:     90,
	},
	track: Track{
		distance: 100,
	},
	expected: true,
},

That car would make, but depending on how the function is implemented it won’t return true.

Hope that makes sense.

Thanks all! :slight_smile:

  • Does this change fix something which is broken?
  • Does this change better help students better learn the concept (structs)?
  • Is this change worth breaking all the published solutions?

To be clear, I appreciate the input and suggestion and think it has merit! And, to be clear, I’m not a Go track maintainer. These are just some questions that may be worth thinking about and guessing at when suggesting this type of change.

Hey @IsaacG

Yeah, that makes sense. I don’t think it would help students better learn the concept. I think it’s more like a cohesion thing. The instructions would be coherent with what we see in the test scenarios. It’s also important to write relevant and coherent tests too.

But I do agree it’s not worth breaking the published solutions if it’s not wrong nor helpful for the purpose of the challenge.

If you guys think it’s worth it, perhaps a middle ground would be updating the Challenge description only. That wouldn’t break any solution. Please ignore this if it’s just not relevant at all.

Thanks for the reply! :slight_smile:

As the tests and the description seem to mismatch, changing the instructions would seem to add clarity. If we can avoid changing the tests, that would be good.

(cc @andrerfcsantos)

1 Like

I had the same doubts as @rafaelmdurante , and though I agree that it is not related to the concept the exercise should demonstrate, it still would be nice to make it less ambiguous. For me, confusion comes from misunderstanding “finishing a track” as something that is done already after the car has started the track. Maybe some different wording in instructions and Go Doc would make it more clear

  1. Could we update Go Doc Comment from
// CanFinish checks if a car is able to finish a certain track.

to

> // CanFinish checks if a car can finish a certain track from start to finish

?

  1. A suggestion that would make the description clearer for me would be rephrase

if the car can finish the race

which is ambiguous with regard how to handle car.distance to either

if the car can finish the race from start to finish

or

if the car can travel the whole distance of a track

1 Like

(cc @andrerfcsantos @junedev )

This assumes that car.distance is the distance the car drove in the “current track”, but that is never stated. Here car.distance behaves like a car odometer, registering the total distance traveled by the car in its lifetime.

But I can see that since that is never clarified in the description, the assumption that car.distance is the distance driven in the current track is one that is possible, although not correct.

Provably one thing we can do is that when presenting the fields the struct should have, we should also explain exactly what each field is, maybe that will help clarify some of the confusion.

In that task, we only care about if the car can do the whole track. I don’t see why thinking of the car as being in an arbitrary position on the track can help answer that question.

The line:

Assume that you are currently at the starting line of the race and start the engine of the car for the race.

Is an attempt of pushing the solutions in te right direction and making it ignore car.distance. The rationale is that, while the car might have some car.distance != 0, ignore it and assume the car is just at the starting line.

I like both ideas a lot! They are simple changes to the description but I can see them massively reducing the ambiguity about car.distance

So, my suggestion here would be:

  • Do not change the tests - touch descriptions/comments only
  • Clarify what element of the struct is, with special care to clarify that the distance field is not tied to any specific track and that behaves like a regular car’s odometer
  • Make the changes @seriar suggested
  • Review task’s hints copy to avoid this ambiguity there too.

Anyone is free to make a PR with these changes, otherwise, I’ll make one this weekend.

@andrerfcsantos i’ve created PR but it was closed as i am not a part of an org.

Fixed the commit message since then to follow the guidelines, but did not re-create PR to avoid the noise

1 Like

Thanks! :slight_smile:

Don’t worry about the commit message guidelines. While it’s good to have good commit messages, we usually merge squash the commits, and when we do that, we can change the commit message of the commit that eventually ends up in main that represents the whole PR.

However, I’ll ask you to re-create the PR if you can. Not sure why, but Github is not letting me reopen the PR saying that the branch was force-pushed or recreated. Re-creating the PR is fine and will likely fix this. it’ll be closed as this one, but I’ll reopen it for you.

1 Like

Yeah, it is the case - after rewording my last commit i force-pushed to my forked branch. New PR is here