From e0fb36aad7cf6d4dcb9d69edc3f21572a5745359 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@gmail.com>
Date: Fri, 11 Jun 2021 20:05:53 +0100
Subject: Solitaire: Iterate the foundation stacks and inline move_card()

Keeping this as a separate commit as I'm not certain whether this
is a good change or not. The repeated if-else for each Foundation
stack bothered me a bit, though more so before I reduced the code
in the {}. But maybe the ifs are clearer than the loop?

Doing that also meant I could inline the move_card() code instead
of needing to make it a lambda. Again, maybe it would be better as
a lambda? I'm still figuring out the style Serenity uses, and I
know Andreas is big on expressiveness, and move_card() is more
expressive than just having the code in the loop.
---
 Userland/Games/Solitaire/Game.cpp | 52 +++++++++++++++++----------------------
 Userland/Games/Solitaire/Game.h   |  2 +-
 2 files changed, 23 insertions(+), 31 deletions(-)

(limited to 'Userland/Games/Solitaire')

diff --git a/Userland/Games/Solitaire/Game.cpp b/Userland/Games/Solitaire/Game.cpp
index a23f1ef059..bd6459636e 100644
--- a/Userland/Games/Solitaire/Game.cpp
+++ b/Userland/Games/Solitaire/Game.cpp
@@ -332,24 +332,6 @@ void Game::check_for_game_over()
     start_game_over_animation();
 }
 
-void Game::move_card(CardStack& from, CardStack& to)
-{
-    update(from.bounding_box());
-
-    auto card = from.pop();
-
-    mark_intersecting_stacks_dirty(card);
-    to.push(card);
-
-    NonnullRefPtrVector<Card> moved_card;
-    moved_card.append(card);
-    remember_move_for_undo(from, to, moved_card);
-
-    score_move(from, to);
-
-    update(to.bounding_box());
-}
-
 void Game::draw_cards()
 {
     auto& waste = stack(Waste);
@@ -441,18 +423,28 @@ bool Game::attempt_to_move_card_to_foundations(CardStack& from)
 
     bool card_was_moved = false;
 
-    if (stack(Foundation1).is_allowed_to_push(top_card)) {
-        move_card(from, stack(Foundation1));
-        card_was_moved = true;
-    } else if (stack(Foundation2).is_allowed_to_push(top_card)) {
-        move_card(from, stack(Foundation2));
-        card_was_moved = true;
-    } else if (stack(Foundation3).is_allowed_to_push(top_card)) {
-        move_card(from, stack(Foundation3));
-        card_was_moved = true;
-    } else if (stack(Foundation4).is_allowed_to_push(top_card)) {
-        move_card(from, stack(Foundation4));
-        card_was_moved = true;
+    for (auto foundationID : foundations) {
+        auto& foundation = stack(foundationID);
+
+        if (foundation.is_allowed_to_push(top_card)) {
+            update(from.bounding_box());
+
+            auto card = from.pop();
+
+            mark_intersecting_stacks_dirty(card);
+            foundation.push(card);
+
+            NonnullRefPtrVector<Card> moved_card;
+            moved_card.append(card);
+            remember_move_for_undo(from, foundation, moved_card);
+
+            score_move(from, foundation);
+
+            update(foundation.bounding_box());
+
+            card_was_moved = true;
+            break;
+        }
     }
 
     if (card_was_moved && (from.type() == CardStack::Type::Play))
diff --git a/Userland/Games/Solitaire/Game.h b/Userland/Games/Solitaire/Game.h
index 191a25caf6..9dfcff24a2 100644
--- a/Userland/Games/Solitaire/Game.h
+++ b/Userland/Games/Solitaire/Game.h
@@ -137,6 +137,7 @@ private:
         __Count
     };
     static constexpr Array piles = { Pile1, Pile2, Pile3, Pile4, Pile5, Pile6, Pile7 };
+    static constexpr Array foundations = { Foundation1, Foundation2, Foundation3, Foundation4 };
 
     ALWAYS_INLINE const WasteRecycleRules& recycle_rules()
     {
@@ -161,7 +162,6 @@ private:
     void remember_move_for_undo(CardStack& from, CardStack& to, NonnullRefPtrVector<Card> moved_cards);
     void remember_flip_for_undo(Card& card);
     void update_score(int to_add);
-    void move_card(CardStack& from, CardStack& to);
     void draw_cards();
     void pop_waste_to_play_stack();
     bool attempt_to_move_card_to_foundations(CardStack& from);
-- 
cgit v1.2.3