Skip to content

Provide function signatures #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ijanos opened this issue Apr 28, 2016 · 13 comments
Closed

Provide function signatures #117

ijanos opened this issue Apr 28, 2016 · 13 comments

Comments

@ijanos
Copy link
Contributor

ijanos commented Apr 28, 2016

When solving Rust exercises the first thing I do is figuring out the name and the right type signature and I'm wondering about the usefulness of this. This process is busywork, I doubt anybody is learning from it and in some cases it requires reverse engineering the thought process of the exercise creator.
I think the the signatures could be added to the readme or maybe a skeleton src/lib.rs could be provided with the exercises.

Look at the Parallel letter frequency exercise for example. The solution has the signature of frequency(&[&str], u32) -> HashMap<char, u32> but nowhere in the readme or in the test cases are mentioned what is the second parameter supposed to be. It can be assumed from the name of one test case that it is the number of workers.
I was confused by it at first. This confusing could have been removed if the readme had a line similar to this:

Your task is to implement the following function: fn frequency(text: &[&str], workers: u32) -> HashMap<char, u32>

What is your opinion on this?

@IanWhitney
Copy link
Contributor

I'm entirely on the fence. And I think whichever way we go there's going to be a group of students that don't like it.

For me, I like not having the signature. I'm entirely new to Rust and static typing and I've found that I like puzzling out what I have to implement by looking at the tests. But that might just be me!

As for parallel letter frequency, I think the API could be improved. I find the current implementation to be hard to parse. Again, that might just be me.

@kytrinyx
Copy link
Member

I'm also on the fence. In the Go track we've provided stub files early on, and then we phase them out after the first few. For the most part reverse engineering function signatures doesn't seem to be difficult in Go, though, and I don't have experience with Rust, so I can't speak to how much busywork that adds here.

@ijanos
Copy link
Contributor Author

ijanos commented Apr 29, 2016

Take a look at the output of cargo test if I run an exercise with empty source file and another one with an empty function with a wrong type signature:

https://gist.github.com/ijanos/59644c68af57e38d5fd81ba6b1baee27

Now compare it to the output once you get the type signature right:

https://gist.github.com/ijanos/53cb04393b6c4e80e5ecbcd908bae0c2

Interpreting the errors is definitely not hard but it can be intimidating at first and mildly annoying if someone doesn't get the types right at the first try.

I think extending the README is a good middle ground to this issue. People still have the option to figure out the function signature while others can just copy paste it and start solving the real exercise.

@kytrinyx
Copy link
Member

We can extend the README by adding a HINTS.md file in the exercise directory.

@IanWhitney
Copy link
Contributor

Let's try the HINTS.md approach in an early exercise and see how it looks.

@ijanos
Copy link
Contributor Author

ijanos commented May 1, 2016

I'm fine with this.

Would the HINTS file contain anything else?

@kytrinyx
Copy link
Member

kytrinyx commented May 1, 2016

It's going to show up as part of the README, so maybe a sentence explaining that this is the expected function signature?

@ijanos
Copy link
Contributor Author

ijanos commented May 1, 2016

Yeah, that would be nice.

@tpitale
Copy link

tpitale commented Jun 17, 2016

I agree with @IanWhitney in that it's helpful/fun to puzzle out the function signatures (and struct/impl too).

I think it's very useful, but I think it should be made explicit. I think exercises to teach about writing the signatures may be a good approach.

Until those exercises get created, though … I think having hints in certain exercises would be a good approach. Possibly in the test file itself. There were very few where I was 😖

@jonasbb
Copy link
Contributor

jonasbb commented Jun 26, 2016

I spent lots of time this weekend to work on this track. I found it really annoying to figure out which functions the test cases do expect. It also conflicts with the way how it is intended to fix the tests. You cannot ignore all the tests marked with ignore, because it results in build errors. So you need to inspect all test cases anyway.
Even then, it is easy to overlook some function and then you see only the huge errors created by marco expansion.
Therefore, I am in strong favor of providing function signatures. I would even put them into a stub src/lib.rs file. (Also I found it annoying to always run mkdir src; touch src/lib.rs. Why is the file not just created?)

I found the Forth exercise to be a pleasant surprise to already contain a lib.rs file. Slightly less convenient, but still better than the rest is the Robot Name exercise which contains these information in the test file.

Whatever this discussion will result in, I think the Forth and Robot Name exercises should follow the same pattern than the rest. It does not really make sense for those middle and late exercises to introduce function signatures.

Would you consider merging stub files with function signatures, if I create a pull request for them?

@IanWhitney
Copy link
Contributor

I think stub files with signatures are great. Thanks! I'm also bugged by the compiler failing due to ignored tests.

@ijanos
Copy link
Contributor Author

ijanos commented Jun 26, 2016

I'm in favor of a at least an empty src/lib.rs making the directory and the file is busywork.

IanWhitney pushed a commit to IanWhitney/xrust that referenced this issue Jun 27, 2016
Tests come from exercism/problem-specifications#282

As discussed in exercism#117 we are
providing a stub implementation so that ignored tests do not fail when a
student hasn't yet implemented the functionality they exercise.
IanWhitney pushed a commit to IanWhitney/xrust that referenced this issue Jul 7, 2016
Tests come from
https://github.com/exercism/x-common/blob/183934754b1847612809db410a8aeb640678f188/robot-simulator.json

As discussed in exercism#117 we are
providing a stub implementation so that ignored tests do not fail when a
student hasn't yet implemented the functionality they exercise.

A lot of excellent feedback and help provided by

- jonasbb
- petertseng
- steveklabnik
- Ryman
- Dr-Emann

Full details of the design discussion at

exercism#146

--

Some details that people may care about:

----

Robots are immutable because:

- Immutability is the default in Rust
- No other problem (that I know of) really features Rust's default immutability
- Immutability is not my natural inclination

I figure if I don't expect immutability then other programmers with an
OO-focused background probably don't expect immutability. So a problem
that forces immutability may make me (and them) think about
immutability. Which I think is good.

Immutability brings other benefits, though they aren't exposed by the
tests. It would be very easy to trace your Robot's path, for example.
Re-winding to a specific point on the path would be trivial. All very
nice, but not part of the test suite.

----

The example code has the `build` function because:

There's still some awkwardness around the `new` function, since it takes
x/y and all the internals use Position.

I have to preserve the `new(x, y, direction)` function signature because
of the tests (and because the tests should follow the example of Queen
Attack, which also passes `x` and `y` in as separate parameters.

But that function becomes a pain once I'm working inside the Robot and I
have a Position.

The new `build` function creates a robot using a Position and Direction,
so it can be easily used by all of the Robot's internals. And `new` now
just wraps around `build`.

This is kind of the exact opposite way I'd normally do this. I'd expect
a `build` function to be the public API, do the data manipulation and
then call a private `new` function.

But if I were to do that here then all of the tests would contain:

```
Robot::build(x, y, &direction)
```

Which I think would be non-idiomatic and weird to students.

----
@IanWhitney
Copy link
Contributor

Now that we're starting to add src/lib.rs stub files, I'm going to close this issue. Thanks for the suggestion, @ijanos !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants