exercism / swift

Exercism exercises in Swift.
https://exercism.org/tracks/swift
MIT License
114 stars 158 forks source link

[Swift] Code checker script fails correct code and might confuse new students. #757

Closed 1MiloAC closed 5 months ago

1MiloAC commented 5 months ago

In the Swift / Exercises / Lasagna layer track, on line 50, 55, and 60 of the LasagnaTests.swift file the code is written as follows: XCTAssertEqual(totalTimeInMinutes(elapsedMinutes: placeholder, layers: placeholder), placeholder)

The issue that I have with this code, is that it is explicitly checks that the totalTimeInMinutes function is passed the argumentlabels in the order of: layers: elapsedMinutes

The issue with this order, is that this will falsely fail code in which the arguments are passed in a different order like elapsedMinutes: layers

While this may not seem like a big issue this could be a major issue for new students as in the exercise the previous function's variables are defined in the order of elapsedMinutes: then layers:

So consequently new students may want to pass the arguments to the final function in this order, which sadly would fail students who had just completed the code correctly.

Also I would like to state that it is never explicitly stated on the website which order to pass the arguments, so this is more of a design issue than a user miss interpreting something.

Also I don't care how small of an edge case this may seem, it is important. Code incorrectly throwing errors is a major issue, especially if it could hinder students. Please don't ignore this as a complaint and actually fix the issue by check for both instances.

meatball133 commented 5 months ago

Hi!

So, if you work in the online editor, you will see the test file after you run the tests. This means that you should be able to solve the exercise only by what is given to you from the instructions and introduction to the concept.

And an example of the function being called is provided in the instructions:

## 4. Calculate the total working time in minutes

Define the function `totalTimeInMinutes(layers:elapsedMinutes:)` that takes two arguments: the `layers` parameter is the number of layers you added to the lasagna, and the `elapsedMinutes` parameter is the number of minutes the lasagna has been in the oven.
The function should return the total number of minutes you've worked on cooking the lasagna, which is the sum of the preparation time in minutes and the time the lasagna has spent in the oven at the moment.

```swift
totalTimeInMinutes(layers: 3, elapsedMinutes: 20)
// Returns 26


As well does the Swift compiler tell you if you put the arguments in the wrong order:

![image](https://github.com/exercism/swift/assets/69751659/ee2f50f7-773f-4620-ad99-d4cb887f36bc)

But if you want me to flip the order of the arguments, I have no problem doing that, but I think that is the extent to which the tests should be changed.
meatball133 commented 5 months ago

Most other tracks (Python, Ruby, etc) feature the same order as the Swift tracks, but Swift will throw a compile error if you use the wrong order. Personally, I think that is better since it tells the student exactly what is wrong with their solution. Compile errors are often a lot nicer than run-time errors.

glaxxie commented 5 months ago

This has been resolved from a thread on discord. Issue can be closed now.