Add Alphametics exercise. Fixes #182 #225

ijanos · 2016-11-16T19:42:26Z

I'm not sure where should we put this in problems.md. I think it belongs towards the end of "Getting Rusty". What do you think?

ijanos · 2016-11-16T19:47:59Z

Hmm, it looks like travis fails on compiling example.rs due to missing dependencies. How should I fix that? Why is it even tries to compile that file?

IanWhitney · 2016-11-19T17:04:21Z

Gigasecond is a good example of an exercise that requires crates. You need to list them in your Cargo.toml file

https://github.com/exercism/xrust/blob/3c48af8880211d3da2284f549bf45e13099bcab9/exercises/gigasecond/Cargo.toml

Sorry that it took me so long to get to this. Vacation + News = Me Avoiding My Computer.

ijanos · 2016-11-19T18:47:04Z

The exercise is not about any specific crate. I just used two convenient creates for the example code. I feel providing a pre-filled [dependencies] would make people think they should use those and I want to avoid that.

IanWhitney · 2016-11-19T20:24:49Z

Sure, but since your example code uses external crates it will not pass CI until those crates are declared as dependencies in the Cargo.toml file.

When the CI runs it compiles your example.rs file and ensures that it passes all of the test cases. And that compilation can't happen unless the dependencies are declared.

ijanos · 2016-11-19T20:37:04Z

I see that, but what should I do if I don't want those dependencies to show up in Cargo.toml when other people download the exercise? Because when you exercism fetch it will not download example.rs but it will download Cargo.toml and I don't see why should this file contain references to something that is not even downloaded, I fear that it will misguide people into the same solution.

Maybe there should be a Cargo-example.toml file? I feel like this pull request is not the right place to discuss that, do you want me to open an issue somewhere for this? Or do You think this is a non-issue and those dependencies for example code should be there for everyone?

petertseng · 2016-11-20T12:31:29Z

Funnily enough, I too have limited access so expect delays in responses from me.

I feel providing a pre-filled [dependencies] would make people think they should use those and I want to avoid that.

I would prefer presenting a clean Cargo.toml to students, because I agree with your reasoning.

If there is a strong desire to have this PR merged quickly, I would find acceptable a Cargo.toml with the dependencies for now if an issue is filed to discuss removing it and we at least come up with a plan.

I feel like this pull request is not the right place to discuss that, do you want me to open an issue somewhere for this?

It does seem to make sense to open a separate issue in this repo. Despite the fact that this PR is the first place where this problem comes up, it can be applicable to others as well so makes sense to have the separate issue.

I would wait to post the following info in that issue, but since I have limited availability, I'll just say it now:

In the Haskell track, package.yaml plays a similar role to Cargo.toml: You can declare dependencies there. I have some history to share.

In exercism/haskell#189 (comment) , I noted that an example was depending on a package, and this package would show up in the delivered package.yaml to students.

Eventually we have exercism/haskell#420 and presented a clean package.yaml to students, without example's dependencies (example uses a separate package.yaml file)

We don't need to use the exact same solution, it only suffices that the example can use a separate Cargo.toml and that https://github.com/exercism/xrust/blob/master/_test/check-exercises.sh understands how to use it - I would suggest that if Cargo-example.toml exists, move it to Cargo.toml, otherwise do nothing - that way we don't have to maintain a whole bunch of unnecessary Cargo.toml files; we only have to maintain the ones that actually need a separate one.

petertseng · 2016-11-25T04:27:09Z

I think it belongs towards the end of "Getting Rusty"

I'll agree.

I can't think of many topics to add to https://github.com/exercism/xrust/blob/master/problems.md , I only have "string parsing", "math", "combinations" or something like that.

petertseng · 2016-11-25T04:23:51Z

exercises/alphametics/example.rs

+            char::from_digit(n as u32,10).unwrap()
+        } else {
+            // Otherwise just copy over the character
+            c


do you ever expect this to happen if we call test_equation? If it does happen, then the parse::<u32>().unwrap() later on would fail, right? If so, I might consider just unwrap() here as well, or have an unreachable! in here.

The else branch will run every time an equation is parsed. This will convert a String like "A + B == C" to another String which looks like this "1 + 2 == 3". The the function chops up this string later and parses the individual numbers.

Oh, so it is for the + and = and spaces. Got it.

petertseng · 2016-11-25T04:24:49Z

exercises/alphametics/example.rs

+    // All available numbers for substitution
+    let numbers: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
+
+    // Iterate every combination with the length of uniqe letters in the puzzle


minor: spelling (uniqe -> unique)

petertseng · 2016-11-25T04:35:40Z

I'll be able to take a closer look in about 96 hours.

How are we doing on runtime? I only see in the test output that the last test ran for more than 60 seconds; is there someting that tell us how long does the entire thing take?

ijanos · 2016-11-25T09:38:07Z

As mentioned in the comment on the top of the file, this is a rather naive brute force example which will run for a long time on large inputs, like the last test. On top of that I did not do any tricks to make this solution faster in the hope of better readability.

It is not that bad on my machine though, definitely under 10 seconds. Running cargo test --release helps, I'm not sure which if travis runs tests in debug or release mode.

petertseng

Codewise, I have no changes to propose. 👍 on this front.

I have some minor formatting comments, but we have no requirement for format, so I have very little ground to stand on in making these comments. #143 did not get taken. So it could get merged without fixing them if nobody else cares.

If looking at https://travis-ci.org/exercism/xrust/builds/178855887, and recalling from https://github.com/exercism/xrust/blob/master/_test/check-exercises.sh that DENYWARNINGS doesn't actually run the tests, we can look at the difference between the DENYWARNINGS and the standard runs to try to see how long the tests take.

I cannot explain why beta only has a difference of 40 seconds, yet stable and nightly have differences of about 4 minutes.

Again, we have no standards on runtimes, so I have no ground to stand on to request anything faster. So let's not block merging this PR for this reason. If we start reviewing PRs and start hurting because Travis is taking too long, we may have to take some or all of the following measures:

run Travis tests with --release (for all exercises? for only designated ones including alphametics?)
don't run some of the long-running tests in Travis (the 8 letter and 10 letter ones, perhaps. How do we do that? TBD).
solve column-by-column in the example solution to speed it up

petertseng · 2016-11-28T10:07:43Z

exercises/alphametics/tests/alphametics.rs

+extern crate alphametics;
+use std::collections::HashMap;
+
+fn test_helper(puzzle: &str, solution:  &[(char, u8)])  {


perhaps just a single space between the : and &?

petertseng · 2016-11-28T10:08:33Z

exercises/alphametics/tests/alphametics.rs

+extern crate alphametics;
+use std::collections::HashMap;
+
+fn test_helper(puzzle: &str, solution:  &[(char, u8)])  {


is there (should there) be a better name for test_helper?

Then again, I don't have a particularly better one. assert_solution_matches?

petertseng · 2016-11-28T10:08:49Z

exercises/alphametics/example.rs

+    }
+    // If we tested every combination and did not found a solution then return None
+    None
+}


newline after?

petertseng · 2016-11-28T10:09:08Z

exercises/alphametics/example.rs

+        if let Some(&n) = substitutions.get(&c) {
+            // If the character is in the substitutions, get the number and
+            // convert it to a char
+            char::from_digit(n as u32,10).unwrap()


space after the comma?

petertseng · 2016-11-28T10:09:30Z

exercises/alphametics/Cargo-example.toml

+
+[dependencies]
+itertools = "0.5"
+permutohedron = "0.2"


newline after?

petertseng · 2016-11-28T17:28:28Z

OK, since Ian's good and everything I said is dealt with, let's merge. I'll squash.

petertseng · 2016-11-28T17:29:34Z

Thanks!

IanWhitney · 2016-11-29T02:19:00Z

Thanks, @ijanos!

ijanos added 2 commits November 16, 2016 20:31

create alphametics rust project

e2ac07f

add alphametics to config.json

ac73592

ijanos mentioned this pull request Nov 20, 2016

Use example.rs specific Cargo.toml file during CI #227

Merged

Add Cargo-example.toml

8ae01a5

petertseng reviewed Nov 25, 2016

View reviewed changes

ijanos added 2 commits November 25, 2016 15:08

fix a typo

b5eacce

Place the exercise in "Getting Rusty"

8523029

IanWhitney approved these changes Nov 27, 2016

View reviewed changes

petertseng approved these changes Nov 28, 2016

View reviewed changes

ijanos added 3 commits November 28, 2016 18:11

add newline at the end of file

ff83051

run rustfmt on rust source code

44171e5

Rename test_helper function to something more meaningful

d3a5435

petertseng merged commit 531434e into exercism:master Nov 28, 2016

ijanos deleted the alphametics branch November 28, 2016 17:42

petertseng mentioned this pull request Mar 9, 2023

External Crates in Alphametics #1645

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Alphametics exercise. Fixes #182 #225

Add Alphametics exercise. Fixes #182 #225

ijanos commented Nov 16, 2016

ijanos commented Nov 16, 2016

IanWhitney commented Nov 19, 2016

ijanos commented Nov 19, 2016

IanWhitney commented Nov 19, 2016

ijanos commented Nov 19, 2016

petertseng commented Nov 20, 2016

petertseng commented Nov 25, 2016

petertseng Nov 25, 2016

ijanos Nov 25, 2016

petertseng Nov 25, 2016

petertseng Nov 25, 2016

petertseng commented Nov 25, 2016 •

edited

Loading

ijanos commented Nov 25, 2016

petertseng left a comment

petertseng Nov 28, 2016

petertseng Nov 28, 2016

petertseng Nov 28, 2016

petertseng Nov 28, 2016

petertseng Nov 28, 2016

petertseng commented Nov 28, 2016

petertseng commented Nov 28, 2016

IanWhitney commented Nov 29, 2016 •

edited

Loading

Add Alphametics exercise. Fixes #182 #225

Add Alphametics exercise. Fixes #182 #225

Conversation

ijanos commented Nov 16, 2016

ijanos commented Nov 16, 2016

IanWhitney commented Nov 19, 2016

ijanos commented Nov 19, 2016

IanWhitney commented Nov 19, 2016

ijanos commented Nov 19, 2016

petertseng commented Nov 20, 2016

petertseng commented Nov 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng commented Nov 25, 2016 • edited Loading

ijanos commented Nov 25, 2016

petertseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petertseng commented Nov 28, 2016

petertseng commented Nov 28, 2016

IanWhitney commented Nov 29, 2016 • edited Loading

petertseng commented Nov 25, 2016 •

edited

Loading

IanWhitney commented Nov 29, 2016 •

edited

Loading