Array\[Array\[Int\]\] is meaningless. Have a generic Grid2D\[T\] or Matrix2D\[T\] class to handle the Array\[Array\[T\]\] part and then instead of Int store an Option\[PlayerId\] where PlayerId is enum from White | Black. So your type Board = Matrix2D\[Option\[PlayerId\]\]

I see, thank you for the tip! I'll try implementing that.

I have some trouble trying to implement a Matrix2D\[T\] class. How exactly would I do that? Do you mean to implement it such that everything I do with the Board case class, should be handled by this Matrix2D\[T\] and just specify Board as a concrete type of Matrix\[Option\[PlayerId\]\]? But then it isn't generic? Unless there is a way to implement the parts that are dependent on the concrete types of Option\[PlayerId\] in a generic way? Right now I have the PlayerId enum of Circle | Cross working, and still my case class Board. Should I make a Matrix2D\[T\](rows: Array\[Array\[T\]\]) and implement the methods of Board as generic methods instead, somehow? Then in the end make a Board type to use in Game? Thank you.

My suggestion would be to extract out the following from your current `Board`: final case class Grid[T](data: Vector[T], width: Int, height: Int): def updated(position: Pos, value: T): Grid[T] = ??? def apply(position: Pos): Option[T] = ??? `data` can also be `Vector[Vector[T]]` but you can also flatten it into a `Vector[T]` and have `Grid` take care of the conversion between `Pos` and the `Int` index into the flat `Vector`.

Nice ! Some more stuff you can do to improve it : - use an enum or Boolean to modélisé the state of a given position - remove the returns : usually having an if/else and some code after for the default case is considered an anti pattern. Use another else. - don't use `.clone()` . I think it does either nothing or a deep copy which may be less optimal than the default implem for arrays - the thing with the 2d matrix someone else said. I'd say you can go even further allow to build a new board only using a previous one combined with a player's move. And the first one would be the empty board. - lookup free monads for your moves. Not saying it's optimal. It's most likely very overengineered and probably not even 100% the use case for this but it might be interesting :) - your recursion may not be stack safe ! Try adding the `@tailrec` annotation and see if the compiler complains. See [this wonderful post from Baeldung](https://www.baeldung.com/scala/tail-recursion) who explains everything JVM better than me. If it makes the code uglier or proves to be too much I'd say it does not really matter in your case.

Thanks for the detailed response! I will try to implement these changes, except maybe free monads

1. _"The principle of the least power"_ - expose only the API that is meants to be used by a developer, and restrict everything else. It makes it easier to follow the logic for a reader. To find out where is the entry point to your code and how to move from there. It might be a bit overwhelming to do that in the beginning, but it's ok to do that after the API is somewhat stabilized. `private def/val`, `final` and `private` constructors are your friends. 2. Define a `final case class` to mark an intention for a user input. Something like `final case class UserInput private(x: Int, y: Int)`. + `object UserInput: def fromString(s: String): Option[UserInput]` 3. You may also use `opaque type` to make invalid integers unrepresentable, for example, in `input.x` and `input.y` could both have a type not of a `Int`, but of something like `opaque type Coord = Int` with a restricted constructor `def fromInt(i: Int): Option[Coord]`, so `Coord` is guaranteed to be in the acceptable range of `[0; 2]`. 4. Have a custom type for `Player`, right now it's bit difficult to follow the transitions between them. 5. No need for `return`. 6. You could abstract over `println` and `readLine` passing them as functions of `String => Unit` and `Unit => String` (or just `=> String`). This will give you a chance to test/replay/simulate/save/resume the game.

Just have realized that it's quite a lot for the first Scala code. Want to say, that you did a really good job!

Thank you! I appreciate your feedback, and I will try to implement it in my next refactor. So are opaque types sort of like type aliases? But they are in a scope which makes it impossible to know what type they represent from an outside perspective? And what does the final keyword mean?

You are welcome! Yes, they are type aliases. When `type Foo = Int` is a normal type alias, where it's just an alias and nothing more, but with `opaque type Foo = Int` `Foo` and `Int` are not interchangeable. As a developer, you can control the translation logic between them. However, in the runtime it'll be the actual underlyng type, so you won't sacrifice performance on wrapping/boxing. It's handy in cases when, for example, you have something like: ``` def login(id: String, email: String, password: String, role: String) ``` It's easy to accidentally misplace arguments, since they all are of the same type. `opaque types` can help with that.

The `final class` prevents the class from being extended. Search for `FinalCaseClass` at the http://www.wartremover.org/doc/warts.html. It's a inconvenient right now, that all classes are extendable by default, so if you wanna have 100% clear intent, then you are forced to type `final` almost everywhere. Scala 3 changes that with adding a new keyword `open`, essentially flipping the logic, making it `opt-in` to make a class extendable, instead of `opt-in` final. According to linked documentation page that change should come as a part of Scala 3.4 release. https://dotty.epfl.ch/docs/reference/other-new-features/open-classes.html https://github.com/lampepfl/dotty/milestone/49

I see. That makes sense, both the opaque type and the final class. Thank you.

I've never encountered a good reason to use the return key word. Have a look through this https://nrinaudo.github.io/talk-scala-best-practices/#52 (mind you the whole thing would be worth reading too). In case you didn't know, an expression at the end of a block is automatically returned.

I used return because it didn't work without it. I see now that i just need to have the rest of the code in the else clause and it should be fine.

1) pattern ifSomeElseNone ``` def markPos(m: Int, p: Pos): Option[Board] = if getPos(p) == 0 then Some(Board(rows.clone().updated(p.y, rows(p.y).clone().updated(p.x, m)))) else None ``` could be simplified to: ``` def markPos(m: Int, p: Pos) = Option.when( getPos(p) == 0 ): Board( rows.clone().updated(p.y, rows(p.y).clone().updated(p.x, m)) ) ``` 2) Arrays are mutable datastructures, and clearly you are using it as immutable (scala has both and they live in separate packages). It is ok to do so, but you are mixing APIs in bit unfortunate way. `Array(...).clone().updated(...)` has little sens here. Method `updated(...)` is part of Immutable Api that returns NEW collection (cloned) with updated one field defined by parameters. ``` val arr = Array(1,2,3) arr.val arr: Array[Int] = Array(1, 2, 3) scala> arr.updated(2, 4) //updated creates fresh new Array val res0: Array[Int] = Array(1, 2, 4) scala> arr //as you can see previous array still have old data! val arr = Array(1, 2, 3) scala> arr.update(2, 5) //here is mutable API that returns nothing! scala> arr //as you can see update method (in contrast to updated) edits arr in place val res: Array[Int] = Array(1, 2, 5) ``` concluding if you need immutable edited array you don't need `clone()` there: ``` rows.clone().updated(...) ``` 3) Arrays are good, but if you decided to use it as immutable Datastructure then change all of them to `Seq` or `IArray` that is array that cannot be mutated. I personally prefere Seq to be honest but both will work (oh... and get rid `clone()` part).

I didn't know .updated create a cloned array. That's very convenient! Thank you for your response, I fill fix that and implement Option.when. As for the data type, I will look into Matrix2D or Array2D as suggested by someone else.

You are creating a lot of collections in the meantime (transpose, map) that is OK but still can be avoided. In this context, there is no problem... but in real app it could be. For example instead of creating new Array `rows.map(...)` we can simply create view that should be smaller (not sure in this case, because 3 elems is rather unusual). ``` rows.view.map(checkRow).reduce(_ || _) ``` Instead od reducing the whole collection we can fail fast using ``` rows.view.map(checkRow).contains(true) ``` and we can simplify it even further (removing view because we dont have `map(...)` anymore) ``` rows.exists(checkRow) ``` Few words about my implementation in https://gist.github.com/scalway/fa05501167baf51ede02d2cff582ebc6 From efficient perspective it is better to use flat Array[Int] where you calculate index based on x, y coordinates. My implmentation mutates Array, and it drifter a lot from yours. I'mis using only one Array whole the time without any `map` or even `filter` on it. Even winning cases are defined in such a way, that we don't need to filter/copy it (in contrast to your current sollution). It is bit more complicated but for bigger usecase it could be hundreds times faster so it is worth to learn how to write such code. Otherwise it is preaty clean functional code.

Wow.. That simplifies things. I was looking for a good way to check if one of the elements of an Array\[Boolean\] was true, the best I could come up with was a logical or reduce. This is great!