Skip to content

Simplify removeById #550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 26, 2024
Merged

Simplify removeById #550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89cf7af
Simplify removeById
Masynchin May 25, 2024
704252b
Fix format
Masynchin May 25, 2024
1233991
Add tests for HasId.removeById
lenguyenthanh May 26, 2024
ae55f1a
Better test for remove first item
lenguyenthanh May 26, 2024
b246088
Add comment to HasId.removeById
lenguyenthanh May 26, 2024
c5f9adf
Use tailrec to eliminate recursive
lenguyenthanh May 26, 2024
2ce4537
Run prepare
lenguyenthanh May 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions core/src/main/scala/HasId.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,21 @@ trait HasId[A, Id]:
inline def hasId(id: Id): Boolean = a.id == id

extension (xs: List[A])
def remove(v: A): List[A] =
final def remove(v: A): List[A] =
xs.removeById(v.id)

def removeById(id: Id): List[A] =
xs.foldLeft((false, List.empty[A])) { (acc, v) =>
if acc._1 then (acc._1, v :: acc._2)
else if v.hasId(id) then (true, acc._2)
else (false, v :: acc._2)
}._2
.reverse
// Remove first item with the given id
// if there is no match return the original list
// This behavior is to accomodate the lila study tree current implementation
// We should change it after We finally migrate it to this new tree
final def removeById(id: Id): List[A] =
@tailrec
def loop(acc: List[A], rest: List[A]): List[A] =
rest match
case (v :: vs) if v.hasId(id) => acc ++ vs
case (v :: vs) => loop(acc :+ v, vs)
case Nil => acc
loop(Nil, xs)

trait Mergeable[A]:

Expand Down
17 changes: 13 additions & 4 deletions test-kit/src/test/scala/HasIdTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
package chess

import munit.ScalaCheckSuite
import org.scalacheck.Prop.forAll
import org.scalacheck.Prop.{ forAll, propBoolean }

class HasIdTest extends ScalaCheckSuite:

given HasId[Int, Int] with
extension (a: Int) def id: Int = a

test("removeById.size <= size"):
test("if there is no id, removeById does nothing"):
forAll: (xs: List[Int], id: Int) =>
val removed = xs.removeById(id)
xs.size == removed.size || xs.size == removed.size + 1
xs.indexOf(id) == -1 ==> (xs.removeById(id) == xs)

test("removeById.size <= size"):
forAll: (xs: List[Int], id: Int) =>
val removed = xs.removeById(id)
val epsilon = if xs.find(_ == id).isDefined then 1 else 0
xs.size == removed.size + epsilon

test("removeById reserves order"):
forAll: (xs: List[Int], id: Int) =>
val sorted = xs.sorted
val removed = sorted.removeById(id)
removed == removed.sorted

test("removeById only remove first items"):
val xs = List(1, 2, 1)
xs.removeById(1) == List(2, 1)