Implement Bracket Push

[IanWhitney]

I'm on the fence about using &str. If we don't provide the stub file, it will increase the difficulty of implementing from.

But if we include the stub file, then there's almost no extra work involved. Which also might reduce the amount students learn from the exercise.

Sometimes you gotta work with borrowed stuff, right? Writing a from implementation for a borrow seems like a useful thing to learn.

So I think my current wish is to:

• Write the tests to expect From<&'a str> (From<&'static str> would also work, I think)
• Don't include the stub file

[petertseng]

Oh, right, because we're making a Brackets from a string, so that will need to annotate its lifetime. Okay.

And what would you say to the suggestion to instead make the interface brackets::are_balanced("()") ?

Of course, internally I may use some structures that require me to track lifetimes, so don't worry that that's being lost.

[petertseng]

I will work on a solution soon to make sure everything is sane. I don't expect there will be anything groundbreaking in it so I'll simply wait until the exercise goes live to submit it, but I'll report whether there were any problems.

[petertseng]

is s better? or does the i stand for "input"?

I don't actually care, but I ask in case it matters to someone

[petertseng]

The current tests appear OK (implementation does fine on them)

[IanWhitney]

Re: Using the from trait. It might be unnecessary here. But recently I've started paying a lot more attention to Primitive Obsession smells. I want to get concepts out of primitives and into named concepts (objects, structs, whatever).

I think this approach works especially well with strong, static type languages. In Ruby I can get away with passing primitives around forever, thanks to its endless number of type-casting functions (both implicit and explicit).

This gets harder in Rust because it won't do implicit type changes. Eventually you'll end up with an i8 and a u8 and want to do some math and run into a type error, for example.

But if I get all primitives into types that I control, then I'm:

• Less likely to run into problems like the i8 vs u8 one.
• Giving the concept a domain name
• Providing a place to hang behavior

In short, I think it's good design.

Which all seems like extreme overkill in an exercise like this, I agree. There is no additional behavior, there is no 'domain', there is only passing the tests.

But if people are using these exercises to learn how to write Rust and the exercises encourage smelly Rust code, then won't most of the students go off and write smelly Rust code in production?

[petertseng]

Of the two choices of brackets::are_balanced("[]") and Brackets::from("[]").are_balanced(), I find either acceptable. If only one choice is acceptable to others, it obviously makes sense to proceed with this PR using the one that is acceptable to all. At this point, we could consider this decision made and move on to other aspects of the PR. I'll discuss it because there are points worth discussing.

Since the motivation here is only to gain understanding for myself, there's no need to wait for this discussion before moving forward with this PR, since an acceptable resolution is already known.

---

But recently I've started paying a lot more attention to Primitive Obsession smells. I want to get concepts out of primitives and into named concepts (objects, structs, whatever).

I wonder if I jokingly said "For every design principle there is an equal and opposite design principle" whether it would be true. I did find that "It is better to have 100 functions operate on one data structure than 10 functions on 10 data structures" and http://stackoverflow.com/questions/6016271/why-is-it-better-to-have-100-functions-operate-on-one-data-structure-than-10-fun attempted to explain why that would be the case.

How would that apply to the matter at hand? I'm not sure. I think a situation I can imagine is: I have a collection of strings, and I want to first filter out the ones that contain valid brackets, and of the ones that do, I want to do something else with them, maybe map(|x| x.len()) or something. But I can also imagine that it's not that much harder to do .filter(|x| Brackets.from(x).are_balanced()) than .filter(|x| brackets::are_balanced(x)). Perhaps another idea is: If I have a collection of Brackets, I can only do operations on them that Brackets allows, whereas if I had a collection of strings I can do many more things. Sometimes I want to be able to do that, sometimes not.

From my reading, a good use of a custom type rather than a primitive is when not all possible values the primitive can take are valid for the custom type. That seems to make sense here: Not all strings are bracketed strings. Unfortunately TryFrom will not be stable for a while, otherwise I would find that interface very good: BracketedString::try_from("[[") would give an error, while BracketedString::try_from("[]") would give something suitably successful. Alas, we shall have to make do without for now...

I think this approach works especially well with strong, static type languages.
Eventually you'll end up with an i8 and a u8 and want to do some math and run into a type error, for example.

Yes, I think this is often why I am interested in languages with static type systems, and taking advantage of their type systems. I like to avoid mistakes such as the adding of an i8 and a u8, or the mistake of adding two (int, int) when one of them is (x, y) and the other is (r, θ).

But if I get all primitives into types that I control, then I'm less likely to run into problems like the i8 vs u8 one.

I thought it would make them more likely, but you would be happy that you are running into the problems (happy that the compiler is catching the potential mistakes, rather than the code having weird bugs exposing themselves only at runtime)

But if people are using these exercises to learn how to write Rust and the exercises encourage smelly Rust code, then won't most of the students go off and write smelly Rust code in production?

Wholeheartedly agree.

[IanWhitney]

Thanks for the link to the SO question, that's a good perspective. I'm guessing this the sort of problem that Generics are trying to address? A single function can continue to work on your primitive types and all of your custom types.

"Good Design" certainly isn't a checklist, it's a series of tradeoffs. No doubt. In a real-world application that checks brackets we might find that using Brackets a bad idea, or it might be great. Hard to say. Without further knowledge I'm OK going with my first instinct of reducing direct Primitive usage.

[petertseng]

probably good to remove these from example

[petertseng]

placement:

If you would like to make a decision before being biased by mine, I put my final thoughts at the bottom.

topics are: iteration over a string's characters, one of (stack OR recursive descent parser). maybe something to do with hash maps (but it's easy to imagine a solution that doesn't use one).

I'll suggest either right between queen-attack and sublist. Can probably be easily convinced to place it anywhere between grade-school and wordy. Anywhere else will need more convincing.

[petertseng]

take a look at whether this can be expressed as an extend which I think cn work because the docs say impl<T> Extend<T> for Vec<T>.

the idea of letting the pairs determine what's unmatched seems OK.

[IanWhitney]

Huh! Extend! Learning things all the time. Good find.

[petertseng]

Everything seems to be coming together nicely. You mentioned you wanted to do something about unmatched (the duplication, I presume) which I would support. I think once that is done this will be good to go.

[IanWhitney]

Ok, I have been torturing this example code for far too long. It's fine. Really, it's just fine. I can save the code torture for when I submit solutions to exercism.

I think everyone is ok with the API (at least no one has complained openly). I still want to:

• Add a HINTS.md file
• Remove the stub file, as I think it trivializes one of the things this problem can teach
• Restore the #[ignore] directive on all-but-the-first test.

[petertseng]

I don't suppose you would find it too much to see if that MatchingBrackets' HashMap initialization can be done with collect?

use std::collections::HashMap;

fn main() {
    let v = vec![(1, 2), (3, 4), (5, 6)];
    let h1 = v.iter().cloned().collect::<HashMap<_, _>>();
    let h2 = v.into_iter().collect::<HashMap<_, _>>();
    println!("{:?}", h1);
    println!("{:?}", h2);
}

[petertseng]

I'm pretty sure I've said all I have to say 👍