Giter Club home page Giter Club logo

Comments (14)

petertseng avatar petertseng commented on September 25, 2024 1

To me it sounds like there are two points of view that make sense based on the context.


In isolation, one may see the tuple as an unneeded extra level of grouping. If all the Queen tests are just testing queens with certain ranks and files that are sourced form somewhere we need not care about, why should it be that they values passed to us get packed up in a tuple? Especially if I'm just going to have to unpack the tuple immediately to operate on its two members anyway?


On the other hand, as part of a larger system that may pass coordinate points around, tuples make sense if tuples are the representation of the coordinate points (and they indeed are a very reasonable representation).

If I had a list of coordinates that I wanted to turn into queens, it's likely that I would want to have them as a list of (i8, i8), rather than two lists of i8.

And only if Queen::new takes a single (i8, i8) can you do something like Queen::new(give_me_some_coordinates()) (obviously give_me_some_coordinates() would be some function that gives you an (i8, i8)). I currently do not know of a way to make this work if Queen::new was (rank: i8, file: i8), and my attempts on the playground were unsuccessful; I don't think Rust has a splat operator that will splat the tuple in (making the commented line work).

from rust.

petertseng avatar petertseng commented on September 25, 2024 1

Yeah, you've essentially got it. Note that, whereas currently Queen::new returns a Result, I would think ChessCoord (which I might bikeshed as Position, but eh) return a Result. Queen::new would take an unconditional ChessCoord and unconditionally return a Queen, so the assumption is ChessCoords can only hold valid chess coordinates.

from rust.

petertseng avatar petertseng commented on September 25, 2024

For me personally I think y'all have convinced me. Passing two tuples to a function might make sense to show the more logical grouping, but a single tuple seems an unnecessary extra level of grouping (and I still believe that despite the fact that you can destructure in the argument list)

from rust.

IanWhitney avatar IanWhitney commented on September 25, 2024

Here was my thought process when making that a tuple.

  1. A co-ordinate on the board is the same as a point on a grid.
  2. A point on a grid is the canonical example of a data clump
  3. You get rid of data clumps by encapsulating them.
  4. The simplest way to do that in this exercise was with a tuple.

I don't quite follow the argument that passing in a single tuple is odd. This could be an idiomatic thing in Rust (or systems languages) that I'm unfamiliar with. I feel the same way about passing tuples into new. Seems fine to me, but there may be an idiomatic reason to not do so.

Also, from a design perspective, one of the more interesting things about the current Queen Attack is what you do with the tuple once you pass it in. Yes, you can just use it as a tuple, or you can encapsulate it, name it and give it behavior

from rust.

petertseng avatar petertseng commented on September 25, 2024

Another potential thought: Maybe I didn't want to internally represent a position as a tuple! Notice that earlier I said (with emphasis added):

tuples make sense if tuples are the representation of the coordinate points

What if the conditional in that sentence were false? What if I wanted to represent my position as a struct with named members instead of a tuple? I'm out of luck, it has to be a tuple since that's what Queen::new expects.

Of course I can change the representation I use internally, but the representation I have to accept from external callers is constrained by the test suite. That's perhaps not too bad (you often have to be liberal in what you accept), but if it is too bad, then here is another idea:

If the exercise would separate out the "Create and validate a position given a rank and file" versus "Given a queen at a certain position, can it attack a queen at this other position", we could envision a world where students are free to choose whatever internal representation of position makes sense, whether that be tuple or struct or anything else! (a single int with the value of rank*8 + file?!)

My memory is getting a bit fuzzy, but I think that was one of the iterations considered when Queen Attack was implemented for Rust but there was a concern about - how do we order the tests in a good way to allow the student to get to the first passing test quickly and iterate quickly from there?

I'll review the discussion around the initial implementation and see if I get any ideas.

E: Hmm, actually it was just that the final iteration we landed on is not too far from allowing this suggestion to work, since our first tests are fundamentally about testing valid and invalid positions. So we could consider that (Note: the tests would get a bit more verbose)

from rust.

kytrinyx avatar kytrinyx commented on September 25, 2024

we could envision a world where students are free to choose whatever internal representation of position makes sense

That's the winning argument as far as I'm concerned. (Of course, I wouldn't be surprised if you had an argument tomorrow that convinced me of the opposite, but still. This is a good one.)

the tests would get a bit more verbose

I'm personally OK with that, if it allows more freedom at the level of the implementation.

from rust.

IanWhitney avatar IanWhitney commented on September 25, 2024

we could envision a world where students are free to choose whatever internal representation of position makes sense

That's the winning argument as far as I'm concerned. (Of course, I wouldn't be surprised if you had an argument tomorrow that convinced me of the opposite, but still. This is a good one.)

I'm not sure I follow. How would this look? Since the tests would include functions with specific signatures, I don't see how students could choose their own approach without also changing the test suite.

Like, if we stick with tests containing the code Queen::new((1,2)) then students can't write their new implementation to accept rank and file parameters, right?

Edit:

Ok, after re-reading, I think I might follow.

fn test_coordinate_constructor() {
  let rank = 1;
  let file = 2;
  let coordinate = ChessCoord::new(rank, file)
  assert_eq!(coordinate.rank(), 1);
  assert_eq!(coordinate.file(), 2);
}

//test here for invalid chess coordinates

fn test_queen_construction() {
  let queen = Queen::new(ChessCoord::new(1,2))
}

Is that the proposal? Because it would let students store their representation of a coordinate in any way they choose?

from rust.

IanWhitney avatar IanWhitney commented on September 25, 2024

Coord is a terrible name, I just threw it there for an example. I can put together a sample of this and we can see if we like it.

from rust.

IanWhitney avatar IanWhitney commented on September 25, 2024

@petertseng When you say that Queen::new takes an unconditional Position, I'm not sure exactly what you mean. Do you think the tests will look like:

fn test_queen_creation() {
  let position = ChessPosition::new(1,2);
  let queen = Queen::new(position.unwrap());
  //etc.
}

from rust.

petertseng avatar petertseng commented on September 25, 2024

When you say that Queen::new takes an unconditional Position, I'm not sure exactly what you mean. Do you think the tests will look like:

Yup, this tells me I should accompany my suggestions with code (I would have were I not otherwise occupied), but you do have the right of it.

from rust.

IanWhitney avatar IanWhitney commented on September 25, 2024

What do you think, @ijanos?

from rust.

petertseng avatar petertseng commented on September 25, 2024

I am usually cautious about speaking for others, but notice the thumbs up on #118 (comment) and who it comes from

from rust.

ijanos avatar ijanos commented on September 25, 2024

Sorry for the late response, I'm from Europe and I guess that is responsible for the longer delays between messages, you are sleeping while I'm awake an vice versa :)

@IanWhitney I like your proposed code, I think it is a better API for this exercise.

from rust.

kytrinyx avatar kytrinyx commented on September 25, 2024

BTW: I mentioned this conversation in today's "Behind the Scenes" newsletter. http://tinyletter.com/exercism/letters/exercism-it-s-about-the-conversations

from rust.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.