external-repos/forge
1
0
Fork 0
mirror of https://github.com/Card-Forge/forge.git synced 2025-11-14 01:38:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Renamed and optimised implementation for getTheLatestOfAllTheOriginalEditionsOfCardsIn + tests

Browse Source 
`getTheLatestOfAllTheOriginalEditionsOfCardsIn` is the new name for `getEarliestEditionWithAllCards` which better clarifies the intent of the method.
The implementation has been optimised, according to the new CardDb behaviour and APIs.

The tests have been extended to verify that the output of the method is always consistent regardless of the edition of cards in the input CardPool.
This commit is contained in:
leriomaggio
2021-07-28 15:56:17 +01:00
parent 1697c1205f
commit 1eaf644e5b
2 changed files with 38 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
forge-core/src/main/java/forge/card/CardEdition.java
View File

@@ -791,34 +791,27 @@ public final class CardEdition implements Comparable<CardEdition> {
};
}
/* @leriomaggio [Note to self]
I am sure this method does not work the way it is expected - do some tests
The name is also misleading!
/* @leriomaggio
The original name "getEarliestEditionWithAllCards" was completely misleading, as it did
not reflect at all what the method really does (and what's the original goal).
What the method **really** does is to return the **latest** (as in the most recent)
Card Edition among the different (see Set<String>) sets found
that includes cards in the pool, picked by an "Old Art First" policy.
There is definitely no-guarantee that **all** cards in the pool will be
included in the set returned!
CounterExample: All cards from LEA, one card from USG.
USG will be returned!
See CardEditionCollectionTestCase.getEarliestEditionWithAllCardsNotWorkingAsExpected
test case that tackles exactly this example!
What I would do instead is to return the set containing the majority of the cards in
the pool (still picked with the same "Old Art First" policy!
A good renaming would be "getEarlierEditionWithMostOfTheCards"
What the method does is to return the **latest** (as in the most recent)
Card Edition among all the different "Original" sets (as in "first print") were cards
in the Pool can be found.
Therefore, nothing to do with an Edition "including" all the cards.
*/
public CardEdition getEarliestEditionWithAllCards(CardPool cards) {
public CardEdition getTheLatestOfAllTheOriginalEditionsOfCardsIn(CardPool cards) {
Set<String> minEditions = new HashSet<>();
CardDb db = StaticData.instance().getCommonCards();
for (Entry<PaperCard, Integer> k : cards) {
// NOTE: Even if we do force a very stringent Policy on Editions
// (which only considers core, expansions, and reprint editions), the fetch method
// is flexible enough to relax the constraint automatically, if no card can be found
// under those conditions (i.e. ORIGINAL_ART_ALL_EDITIONS will be automatically used instead).
PaperCard cp = db.getCardFromEditions(k.getKey().getName(),
CardArtPreference.ORIGINAL_ART_CORE_EXPANSIONS_REPRINT_ONLY);
if (cp == null)
cp = k.getKey(); // it's unlikely, this code will ever run
if (cp == null) // it's unlikely, this code will ever run. Only Happens if card does not exist.
cp = k.getKey();
minEditions.add(cp.getEdition());
}
for (CardEdition ed : getOrderedEditions()) {
@@ -827,15 +820,6 @@ public final class CardEdition implements Comparable<CardEdition> {
}
return UNKNOWN;
}
public Date getEarliestDateWithAllCards(CardPool cardPool) {
CardEdition earliestSet = StaticData.instance().getEditions().getEarliestEditionWithAllCards(cardPool);
Calendar cal = Calendar.getInstance();
cal.setTime(earliestSet.getDate());
cal.add(Calendar.DATE, 1);
return cal.getTime();
}
}
public static class Predicates {

26
forge-gui-desktop/src/test/java/forge/card/CardEditionCollectionTestCase.java
View File

@@ -13,7 +13,7 @@ import static org.testng.Assert.assertEquals;
public class CardEditionCollectionTestCase extends ForgeCardMockTestCase {
@Test
public void getEarliestEditionWithAllCardsNotWorkingAsExpected(){
public void testGetTheLatestOfAllTheOriginalEditionsOfCardsInPoolWithOriginalSets(){
CardEdition.Collection editions = FModel.getMagicDb().getEditions();
CardDb cardDb = FModel.getMagicDb().getCommonCards();
@@ -30,7 +30,29 @@ public class CardEditionCollectionTestCase extends ForgeCardMockTestCase {
CardPool pool = new CardPool();
pool.add(cards);
CardEdition ed = editions.getEarliestEditionWithAllCards(pool);
CardEdition ed = editions.getTheLatestOfAllTheOriginalEditionsOfCardsIn(pool);
assertEquals(ed.getCode(), "ALL");
}
@Test
public void testGetTheLatestOfAllTheOriginalEditionsOfCardsInPoolWithLatestArtSets(){
CardEdition.Collection editions = FModel.getMagicDb().getEditions();
CardDb cardDb = FModel.getMagicDb().getCommonCards();
String[] cardNames = {"Shivan Dragon", "Animate Wall", "Balance", "Blessing", "Force of Will"};
String[] expectedSets = {"M20", "MED", "SLD", "M14", "2XM"};
List<PaperCard> cards = new ArrayList<>();
for (int i=0; i < 5; i++){
String cardName = cardNames[i];
String expectedSet = expectedSets[i];
PaperCard card = cardDb.getCardFromEditions(cardName, CardDb.CardArtPreference.LATEST_ART_ALL_EDITIONS);
assertEquals(card.getEdition(), expectedSet, "Assertion Failed for "+cardName);
cards.add(card);
}
CardPool pool = new CardPool();
pool.add(cards);
CardEdition ed = editions.getTheLatestOfAllTheOriginalEditionsOfCardsIn(pool);
assertEquals(ed.getCode(), "ALL");
}
}