Add Day 15 write-up #781
Add Day 15 write-up #781
Conversation
|
I put a call for reviewers on Discord — let's merge in 24 hours unless there is unresolved review feedback at that point |
SethTisue
left a comment
SethTisue
left a comment
There was a problem hiding this comment.
(none of this is terribly important, but just some thoughts)
| input.flatMap(_.map(Direction.ofChar)).toSeq | ||
|
|
||
| def parse(inputFile: String): (Warehouse, Seq[Direction]) = | ||
| val file = io.Source.fromFile(inputFile) |
There was a problem hiding this comment.
maybe Using instead of try/finally? I don't have a strong opinion here, just wondering if you'd considered it
There was a problem hiding this comment.
Is it more idiomatic? I'd have to look up how to use it 😅 but I could
twentylemon
left a comment
twentylemon
left a comment
There was a problem hiding this comment.
Hello, I am a random person from said discord. 🫡
LGTM, clean solution! 🚀
| val newCells = cells.map(_.toArray).toArray | ||
| // mutate it, | ||
| updates.foreach{ case ((row, col), cell) => newCells(row)(col) = cell } | ||
| // and construct a new ArraySeq backed by it. Possibly unsafe if the | ||
| // backing array remains accessible outside of the ArraySeq, but safe | ||
| // here because the method only returns the ArraySeq. | ||
| ArraySeq.unsafeWrapArray(newCells).map(ArraySeq.unsafeWrapArray) |
There was a problem hiding this comment.
Minor, might be able to avoid these unsafes by using Seq everywhere instead of the ArraySeq implementation type, and using .toSeq instead of the .toArrays. Seq is still immutable, so it won't compromise your morals!
There was a problem hiding this comment.
Ah but I know the size ahead of time and I know I will be doing a lot of random accesses. Seq doesn’t guarantee anything about that, right? (I’ll add this to my reasoning note)
There was a problem hiding this comment.
Yeah, but the typical implementation of Seq is the Vector, which offers basically O(1) everything. See docs.
There was a problem hiding this comment.
the default implementation of Seq is List, but .toIndexedSeq is worth considering (not sure what my opinion is). I think most of us tend to forget that .toIndexedSeq even exists (I forgot, until I suddenly remembered).
There was a problem hiding this comment.
My personal approach is to not rely on what the typical/default implementation is, but rather to only assume what the docs guarantee, and Seq and IndexedSeq would not quite guarantee enough. I would also have thought that Vectors provide O(1) random access but the Scala 3 docs say it's O(log n).
There was a problem hiding this comment.
(The collections guide calls that "effectively constant" since the base of the log is quite high, so that it can never take more than a small number of steps.)
3 participants
No description provided.