From 6988e983071cebf78f7fbadc6e1e7bc94d348826 Mon Sep 17 00:00:00 2001 From: dcolazin Date: Thu, 22 Dec 2022 18:51:40 +0100 Subject: [PATCH 1/7] issue 99 failing test & small fixes --- .../bhlangonijr/chesslib/game/Game.java | 4 ++-- .../chesslib/game/GameFactory.java | 8 ++++---- .../bhlangonijr/chesslib/PgnIteratorTest.java | 20 +++++++++++++++++++ src/test/resources/pgn_order.pgn | 20 +++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 src/test/resources/pgn_order.pgn diff --git a/src/main/java/com/github/bhlangonijr/chesslib/game/Game.java b/src/main/java/com/github/bhlangonijr/chesslib/game/Game.java index f1782b9..bbabe58 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/game/Game.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/game/Game.java @@ -555,7 +555,7 @@ private int translateVariation(StringBuilder sb, MoveList variation, int parent, @Override public String toString() { try { - return toPgn(true, true); + return toPgn(true, true); //TODO this throws NPE in debugger sometimes } catch (MoveConversionException e) { return null; } @@ -801,7 +801,7 @@ public void loadMoveText(StringBuilder moveText) throws Exception { onCommentBlock = false; if (comment != null) { if (getComments() == null) { - setComments(new HashMap()); + setComments(new HashMap<>()); } getComments().put(variantIndex, comment.toString()); } diff --git a/src/main/java/com/github/bhlangonijr/chesslib/game/GameFactory.java b/src/main/java/com/github/bhlangonijr/chesslib/game/GameFactory.java index e1af63a..f886597 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/game/GameFactory.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/game/GameFactory.java @@ -60,9 +60,7 @@ public static Round newRound(Event event, int number) { */ public static Game newGame(String gameId, Round round) { - Game game = new Game(gameId, round); - - return game; + return new Game(gameId, round); } /** @@ -73,7 +71,9 @@ public static Game newGame(String gameId, Round round) { * @return he newly created instance of a player */ public static Player newPlayer(PlayerType type, String name) { - return new GenericPlayer(name, name); + Player player = new GenericPlayer(name, name); + player.setType(type); + return player; } diff --git a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java index 2533755..ac117fb 100644 --- a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java +++ b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java @@ -1,6 +1,7 @@ package com.github.bhlangonijr.chesslib; import com.github.bhlangonijr.chesslib.game.Game; +import com.github.bhlangonijr.chesslib.game.GameResult; import com.github.bhlangonijr.chesslib.move.Move; import com.github.bhlangonijr.chesslib.move.MoveList; import com.github.bhlangonijr.chesslib.pgn.PgnIterator; @@ -18,6 +19,25 @@ */ public class PgnIteratorTest { + @Test + public void testPGNOrder() throws Exception { + PgnIterator games = new PgnIterator("src/test/resources/pgn_order.pgn"); + Game game = games.iterator().next(); + assertEquals("1st World U20 ch", game.getRound().getEvent().getName()); + assertEquals("Birmingham ENG", game.getRound().getEvent().getSite()); + assertEquals("1951.06.20", game.getDate()); + //assertEquals("1951.??.??", game.getRound().getEvent().getStartDate()); + assertEquals(9, game.getRound().getNumber()); + //assertEquals(GameResult.WHITE_WON, game.getResult()); //TODO this is the issue + assertEquals("Bent Larsen", game.getWhitePlayer().getName()); + assertEquals("Lionel Joyner", game.getBlackPlayer().getName()); + assertEquals("C30", game.getEco()); + //assertEquals("?", game.getWhitePlayer().getElo()); //TODO have a null (not inserted), vs unknown (?) ? + //assertEquals("?", game.getBlackPlayer().getElo()); + assertEquals("63", game.getPlyCount()); //TODO why is it a string? + //assertEquals("...", game.getHalfMoves().toString()); //TODO + } + @Test public void testPGNIteration() throws Exception { diff --git a/src/test/resources/pgn_order.pgn b/src/test/resources/pgn_order.pgn new file mode 100644 index 0000000..356a466 --- /dev/null +++ b/src/test/resources/pgn_order.pgn @@ -0,0 +1,20 @@ +[Event "1st World U20 ch"] +[Site "Birmingham ENG"] +[Date "1951.06.20"] +[EventDate "1951.??.??"] +[Round "9"] +[Result "1-0"] +[White "Bent Larsen"] +[Black "Lionel Joyner"] +[ECO "C30"] +[WhiteElo "?"] +[BlackElo "?"] +[PlyCount "63"] + +1. e4 e5 2. f4 Bc5 3. Nf3 d6 4. c3 Bg4 5. fxe5 dxe5 6. Qa4+ Bd7 +7. Qc2 Nc6 8. b4 Bd6 9. Be2 Qe7 10. Na3 a5 11. b5 Nd8 12. Nc4 f6 +13. O-O Nh6 14. d4 Nhf7 15. a4 O-O 16. Nxd6 Nxd6 17. Ba3 N8f7 +18. c4 exd4 19. c5 Qxe4 20. Bd3 Qe3+ 21. Kh1 Nxb5 +22. axb5 c6 23. Bxh7+ Kh8 24. Nh4 Ne5 25. Rae1 Qh6 26. Ng6+ +Nxg6 27. Bxg6 Be8 28. Bf5 cxb5 29. c6 b4 30. Bc1 g5 31. c7 Bc6 +32. Re7 1-0 From 70afa7cfe94bc455b6f0e6d6281e27262302f804 Mon Sep 17 00:00:00 2001 From: dcolazin Date: Thu, 22 Dec 2022 19:26:05 +0100 Subject: [PATCH 2/7] extracted methods from loadNextGame --- .../bhlangonijr/chesslib/pgn/GameLoader.java | 399 +++++++++--------- .../bhlangonijr/chesslib/PgnIteratorTest.java | 2 + 2 files changed, 204 insertions(+), 197 deletions(-) diff --git a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java index e5be205..acd7f42 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java @@ -16,14 +16,6 @@ package com.github.bhlangonijr.chesslib.pgn; -import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.UTF8_BOM; -import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.isProperty; -import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.parsePgnProperty; - -import java.util.HashMap; -import java.util.Iterator; -import java.util.UUID; - import com.github.bhlangonijr.chesslib.game.Event; import com.github.bhlangonijr.chesslib.game.Game; import com.github.bhlangonijr.chesslib.game.GameFactory; @@ -35,6 +27,14 @@ import com.github.bhlangonijr.chesslib.game.TimeControl; import com.github.bhlangonijr.chesslib.util.StringUtil; +import java.util.HashMap; +import java.util.Iterator; +import java.util.UUID; + +import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.UTF8_BOM; +import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.isProperty; +import static com.github.bhlangonijr.chesslib.pgn.PgnProperty.parsePgnProperty; + /** * A convenient loader to extract a chess game and its metadata from an iterator over the lines of the PGN file. *

@@ -51,13 +51,7 @@ public class GameLoader { */ public static Game loadNextGame(Iterator iterator) { - Event event = null; - Round round = null; - Game game = null; - Player whitePlayer = null; - Player blackPlayer = null; - StringBuilder moveText = null; - boolean moveTextParsing = false; + PgnTempContainer container = new PgnTempContainer(); while (iterator.hasNext()) { String line = iterator.next(); @@ -67,202 +61,213 @@ public static Game loadNextGame(Iterator iterator) { line = line.substring(1); } if (isProperty(line)) { - PgnProperty p = parsePgnProperty(line); - if (p != null) { - String tag = p.name.toLowerCase().trim(); - //begin - switch (tag) { - case "event": - if (moveTextParsing && game != null && game.getHalfMoves().size() == 0) { - setMoveText(game, moveText); - } - game = null; - round = null; - whitePlayer = null; - blackPlayer = null; - if (event == null) { - event = GameFactory.newEvent(p.value); - } - moveText = new StringBuilder(); - - break; - case "site": - if (event != null) { - event.setSite(p.value); - } - break; - case "date": - if (event != null) { - event.setStartDate(p.value); - } - break; - case "round": - if (event != null) { - int r = 1; - try { - r = Integer.parseInt(p.value); - } catch (Exception e1) { - } - r = Math.max(0, r); - round = event.getRound().get(r); - if (round == null) { - round = GameFactory.newRound(event, r); - event.getRound().put(r, round); - } - } - break; - case "white": { - if (round == null) { - round = GameFactory.newRound(event, 1); - event.getRound().put(1, round); - } - if (game == null) { - game = GameFactory.newGame(UUID.randomUUID().toString(), round); - game.setDate(event.getStartDate()); - round.getGame().add(game); - } + addProperty(line, container); + } else if (!line.equals("") && container.moveText != null) { + addMoveText(line, container); + if (isEndGame(line)) { + if (container.game != null) { + setMoveText(container.game, container.moveText); + } + return container.game; + } + } + } catch (Exception e) { //TODO stricter exceptions + String name = container.event != null ? container.event.getName() : ""; + int r = container.round != null ? container.round.getNumber() : 0; + throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); + } + } + return container.game; + } - Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); - player.setId(p.value); - player.setDescription(p.value); + private static void addProperty(String line, PgnTempContainer container) throws Exception { + PgnProperty p = parsePgnProperty(line); + if (p == null) { + return; + } + String tag = p.name.toLowerCase().trim(); + //begin + switch (tag) { + case "event": + if (container.moveTextParsing && container.game != null && container.game.getHalfMoves().size() == 0) { + setMoveText(container.game, container.moveText); + } + container.game = null; + container.round = null; + container.whitePlayer = null; + container.blackPlayer = null; + if (container.event == null) { + container.event = GameFactory.newEvent(p.value); + } + container.moveText = new StringBuilder(); + break; + case "site": + if (container.event != null) { + container.event.setSite(p.value); + } + break; + case "date": + if (container.event != null) { + container.event.setStartDate(p.value); + } + break; + case "round": + if (container.event != null) { + int r = 1; + try { + r = Integer.parseInt(p.value); + } catch (Exception e1) { + } + r = Math.max(0, r); + container.round = container.event.getRound().get(r); + if (container.round == null) { + container.round = GameFactory.newRound(container.event, r); + container.event.getRound().put(r, container.round); + } + } + break; + case "white": { + if (container.round == null) { + container.round = GameFactory.newRound(container.event, 1); + container.event.getRound().put(1, container.round); + } + if (container.game == null) { + container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); + container.game.setDate(container.event.getStartDate()); + container.round.getGame().add(container.game); + } - game.setWhitePlayer(player); - whitePlayer = player; + Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); + player.setId(p.value); + player.setDescription(p.value); - break; - } - case "black": { - if (round == null) { - round = GameFactory.newRound(event, 1); - event.getRound().put(1, round); - } - if (game == null) { - game = GameFactory.newGame(UUID.randomUUID().toString(), round); - game.setDate(event.getStartDate()); - round.getGame().add(game); - } - Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); - player.setId(p.value); - player.setDescription(p.value); + container.game.setWhitePlayer(player); + container.whitePlayer = player; - game.setBlackPlayer(player); - blackPlayer = player; + break; + } + case "black": { + if (container.round == null) { + container.round = GameFactory.newRound(container.event, 1); + container.event.getRound().put(1, container.round); + } + if (container.game == null) { + container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); + container.game.setDate(container.event.getStartDate()); + container.round.getGame().add(container.game); + } + Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); + player.setId(p.value); + player.setDescription(p.value); - break; - } - case "result": - if (game != null) { - GameResult r = GameResult.fromNotation(p.value); - game.setResult(r); - } - break; - case "plycount": - if (game != null) { - game.setPlyCount(p.value); - } - break; - case "termination": - if (game != null) { - try { - game.setTermination(Termination.fromValue(p.value.toUpperCase())); - } catch (Exception e1) { - game.setTermination(Termination.UNTERMINATED); - } - } - break; - case "timecontrol": - if (event != null && event.getTimeControl() == null) { - try { - event.setTimeControl(TimeControl.parseFromString(p.value.toUpperCase())); - } catch (Exception e1) { - //ignore errors in time control tag as it's not required by standards - } - } - break; - case "annotator": - if (game != null) { - game.setAnnotator(p.value); - } - break; - case "fen": - if (game != null) { - game.setFen(p.value); - } - break; - case "eco": - if (game != null) { - game.setEco(p.value); - } - break; - case "opening": - if (game != null) { - game.setOpening(p.value); - } - break; - case "variation": - if (game != null) { - game.setVariation(p.value); - } - break; - case "whiteelo": - if (whitePlayer != null) { - try { - whitePlayer.setElo(Integer.parseInt(p.value)); - } catch (NumberFormatException e) { + container.game.setBlackPlayer(player); + container.blackPlayer = player; + break; + } + case "result": + if (container.game != null) { + GameResult r = GameResult.fromNotation(p.value); + container.game.setResult(r); + } + break; + case "plycount": + if (container.game != null) { + container.game.setPlyCount(p.value); + } + break; + case "termination": + if (container.game != null) { + try { + container.game.setTermination(Termination.fromValue(p.value.toUpperCase())); + } catch (Exception e1) { + container.game.setTermination(Termination.UNTERMINATED); + } + } + break; + case "timecontrol": + if (container.event != null && container.event.getTimeControl() == null) { + try { + container.event.setTimeControl(TimeControl.parseFromString(p.value.toUpperCase())); + } catch (Exception e1) { + //ignore errors in time control tag as it's not required by standards + } + } + break; + case "annotator": + if (container.game != null) { + container.game.setAnnotator(p.value); + } + break; + case "fen": + if (container.game != null) { + container.game.setFen(p.value); + } + break; + case "eco": + if (container.game != null) { + container.game.setEco(p.value); + } + break; + case "opening": + if (container.game != null) { + container.game.setOpening(p.value); + } + break; + case "variation": + if (container.game != null) { + container.game.setVariation(p.value); + } + break; + case "whiteelo": + if (container.whitePlayer != null) { + try { + container.whitePlayer.setElo(Integer.parseInt(p.value)); + } catch (NumberFormatException e) { - } - } - break; - case "blackelo": - if (blackPlayer != null) { - try { - blackPlayer.setElo(Integer.parseInt(p.value)); - } catch (NumberFormatException e) { + } + } + break; + case "blackelo": + if (container.blackPlayer != null) { + try { + container.blackPlayer.setElo(Integer.parseInt(p.value)); + } catch (NumberFormatException e) { - } - } - break; - default: - if (game != null) { - if (game.getProperty() == null) { - game.setProperty(new HashMap()); - } - game.getProperty().put(p.name, p.value); - } - break; - } } - } else if (!line.trim().equals("") && moveText != null) { - moveText.append(line); - moveText.append('\n'); - moveTextParsing = true; - if (line.endsWith("1-0") || - line.endsWith("0-1") || - line.endsWith("1/2-1/2") || - line.endsWith("*")) { - //end of PGN - if (game != null) { - setMoveText(game, moveText); - } - break; + } + break; + default: + if (container.game != null) { + if (container.game.getProperty() == null) { + container.game.setProperty(new HashMap()); } + container.game.getProperty().put(p.name, p.value); } + break; + } + } - } catch (Exception e) { - String name = ""; - int r = 0; - try { - r = round.getNumber(); - name = event.getName(); - } catch (Exception e2) { + private static void addMoveText(String line, PgnTempContainer container) { + container.moveText.append(line); + container.moveText.append('\n'); + container.moveTextParsing = true; + } - } - throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); - } + private static boolean isEndGame(String line) { + return line.endsWith("1-0") || line.endsWith("0-1") || line.endsWith("1/2-1/2") || line.endsWith("*"); + } - } + private static class PgnTempContainer { + + Event event; + Round round; + Game game; + Player whitePlayer; + Player blackPlayer; + StringBuilder moveText; + boolean moveTextParsing; - return game; + PgnTempContainer() {} } private static void setMoveText(Game game, StringBuilder moveText) throws Exception { diff --git a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java index ac117fb..e140419 100644 --- a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java +++ b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java @@ -19,6 +19,8 @@ */ public class PgnIteratorTest { + //TODO test exception... + @Test public void testPGNOrder() throws Exception { PgnIterator games = new PgnIterator("src/test/resources/pgn_order.pgn"); From 22f9837d72ae480214088db78628df799fb87b0f Mon Sep 17 00:00:00 2001 From: dcolazin Date: Tue, 17 Jan 2023 06:11:32 +0100 Subject: [PATCH 3/7] init event in GameLoader --- .../bhlangonijr/chesslib/game/Event.java | 2 +- .../bhlangonijr/chesslib/pgn/GameLoader.java | 91 ++++++++----------- 2 files changed, 41 insertions(+), 52 deletions(-) diff --git a/src/main/java/com/github/bhlangonijr/chesslib/game/Event.java b/src/main/java/com/github/bhlangonijr/chesslib/game/Event.java index c4acc26..9285160 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/game/Event.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/game/Event.java @@ -26,7 +26,7 @@ */ public class Event { - private final Map round = new HashMap(); + private final Map round = new HashMap<>(); private String id; private String name; private EventType eventType; diff --git a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java index acd7f42..099c49e 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java @@ -72,7 +72,7 @@ public static Game loadNextGame(Iterator iterator) { } } } catch (Exception e) { //TODO stricter exceptions - String name = container.event != null ? container.event.getName() : ""; + String name = container.event.getName(); int r = container.round != null ? container.round.getNumber() : 0; throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); } @@ -81,49 +81,38 @@ public static Game loadNextGame(Iterator iterator) { } private static void addProperty(String line, PgnTempContainer container) throws Exception { - PgnProperty p = parsePgnProperty(line); - if (p == null) { + PgnProperty property = parsePgnProperty(line); + if (property == null) { return; } - String tag = p.name.toLowerCase().trim(); + String tag = property.name.toLowerCase().trim(); //begin switch (tag) { case "event": if (container.moveTextParsing && container.game != null && container.game.getHalfMoves().size() == 0) { setMoveText(container.game, container.moveText); } - container.game = null; - container.round = null; - container.whitePlayer = null; - container.blackPlayer = null; - if (container.event == null) { - container.event = GameFactory.newEvent(p.value); - } - container.moveText = new StringBuilder(); + container.event.setName(property.value); + container.event.setId(property.value); + container.moveText = new StringBuilder(); //TODO initialize this break; case "site": - if (container.event != null) { - container.event.setSite(p.value); - } + container.event.setSite(property.value); break; case "date": - if (container.event != null) { - container.event.setStartDate(p.value); - } + container.event.setStartDate(property.value); break; case "round": - if (container.event != null) { - int r = 1; - try { - r = Integer.parseInt(p.value); - } catch (Exception e1) { - } - r = Math.max(0, r); - container.round = container.event.getRound().get(r); - if (container.round == null) { - container.round = GameFactory.newRound(container.event, r); - container.event.getRound().put(r, container.round); - } + int r = 1; + try { + r = Integer.parseInt(property.value); //TODO isParseable + } catch (Exception e1) { + } + r = Math.max(0, r); + container.round = container.event.getRound().get(r); + if (container.round == null) { + container.round = GameFactory.newRound(container.event, r); + container.event.getRound().put(r, container.round); } break; case "white": { @@ -137,9 +126,9 @@ private static void addProperty(String line, PgnTempContainer container) throws container.round.getGame().add(container.game); } - Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); - player.setId(p.value); - player.setDescription(p.value); + Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); + player.setId(property.value); + player.setDescription(property.value); container.game.setWhitePlayer(player); container.whitePlayer = player; @@ -156,9 +145,9 @@ private static void addProperty(String line, PgnTempContainer container) throws container.game.setDate(container.event.getStartDate()); container.round.getGame().add(container.game); } - Player player = GameFactory.newPlayer(PlayerType.HUMAN, p.value); - player.setId(p.value); - player.setDescription(p.value); + Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); + player.setId(property.value); + player.setDescription(property.value); container.game.setBlackPlayer(player); container.blackPlayer = player; @@ -166,28 +155,28 @@ private static void addProperty(String line, PgnTempContainer container) throws } case "result": if (container.game != null) { - GameResult r = GameResult.fromNotation(p.value); - container.game.setResult(r); + GameResult result = GameResult.fromNotation(property.value); + container.game.setResult(result); } break; case "plycount": if (container.game != null) { - container.game.setPlyCount(p.value); + container.game.setPlyCount(property.value); } break; case "termination": if (container.game != null) { try { - container.game.setTermination(Termination.fromValue(p.value.toUpperCase())); + container.game.setTermination(Termination.fromValue(property.value.toUpperCase())); } catch (Exception e1) { container.game.setTermination(Termination.UNTERMINATED); } } break; case "timecontrol": - if (container.event != null && container.event.getTimeControl() == null) { + if (container.event.getTimeControl() == null) { try { - container.event.setTimeControl(TimeControl.parseFromString(p.value.toUpperCase())); + container.event.setTimeControl(TimeControl.parseFromString(property.value.toUpperCase())); } catch (Exception e1) { //ignore errors in time control tag as it's not required by standards } @@ -195,33 +184,33 @@ private static void addProperty(String line, PgnTempContainer container) throws break; case "annotator": if (container.game != null) { - container.game.setAnnotator(p.value); + container.game.setAnnotator(property.value); } break; case "fen": if (container.game != null) { - container.game.setFen(p.value); + container.game.setFen(property.value); } break; case "eco": if (container.game != null) { - container.game.setEco(p.value); + container.game.setEco(property.value); } break; case "opening": if (container.game != null) { - container.game.setOpening(p.value); + container.game.setOpening(property.value); } break; case "variation": if (container.game != null) { - container.game.setVariation(p.value); + container.game.setVariation(property.value); } break; case "whiteelo": if (container.whitePlayer != null) { try { - container.whitePlayer.setElo(Integer.parseInt(p.value)); + container.whitePlayer.setElo(Integer.parseInt(property.value)); } catch (NumberFormatException e) { } @@ -230,7 +219,7 @@ private static void addProperty(String line, PgnTempContainer container) throws case "blackelo": if (container.blackPlayer != null) { try { - container.blackPlayer.setElo(Integer.parseInt(p.value)); + container.blackPlayer.setElo(Integer.parseInt(property.value)); } catch (NumberFormatException e) { } @@ -241,7 +230,7 @@ private static void addProperty(String line, PgnTempContainer container) throws if (container.game.getProperty() == null) { container.game.setProperty(new HashMap()); } - container.game.getProperty().put(p.name, p.value); + container.game.getProperty().put(property.name, property.value); } break; } @@ -259,7 +248,7 @@ private static boolean isEndGame(String line) { private static class PgnTempContainer { - Event event; + Event event = new Event(); Round round; Game game; Player whitePlayer; From 97526078581d438edc5795d43dcd26633eb035c0 Mon Sep 17 00:00:00 2001 From: dcolazin Date: Tue, 17 Jan 2023 06:22:32 +0100 Subject: [PATCH 4/7] init round in GameLoader --- .../bhlangonijr/chesslib/game/Round.java | 2 +- .../bhlangonijr/chesslib/pgn/GameLoader.java | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/github/bhlangonijr/chesslib/game/Round.java b/src/main/java/com/github/bhlangonijr/chesslib/game/Round.java index 2167f0e..eac4245 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/game/Round.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/game/Round.java @@ -23,7 +23,7 @@ * A round of a chess event. */ public class Round { - private final List game = new ArrayList(); + private final List game = new ArrayList<>(); private final Event event; private int number; diff --git a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java index 099c49e..d0df22d 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java @@ -73,7 +73,7 @@ public static Game loadNextGame(Iterator iterator) { } } catch (Exception e) { //TODO stricter exceptions String name = container.event.getName(); - int r = container.round != null ? container.round.getNumber() : 0; + int r = container.round.getNumber(); throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); } } @@ -109,16 +109,14 @@ private static void addProperty(String line, PgnTempContainer container) throws } catch (Exception e1) { } r = Math.max(0, r); - container.round = container.event.getRound().get(r); - if (container.round == null) { - container.round = GameFactory.newRound(container.event, r); + container.round.setNumber(r); + if (!container.event.getRound().containsKey(r)) { container.event.getRound().put(r, container.round); } break; case "white": { - if (container.round == null) { - container.round = GameFactory.newRound(container.event, 1); - container.event.getRound().put(1, container.round); + if (container.round.getNumber() < 1) { + container.round.setNumber(1); //TODO this is just to have the same behaviour as before... } if (container.game == null) { container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); @@ -136,9 +134,8 @@ private static void addProperty(String line, PgnTempContainer container) throws break; } case "black": { - if (container.round == null) { - container.round = GameFactory.newRound(container.event, 1); - container.event.getRound().put(1, container.round); + if (container.round.getNumber() < 1) { + container.round.setNumber(1); //TODO this just to have the same behaviour as before... } if (container.game == null) { container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); @@ -249,7 +246,7 @@ private static boolean isEndGame(String line) { private static class PgnTempContainer { Event event = new Event(); - Round round; + Round round = new Round(event); Game game; Player whitePlayer; Player blackPlayer; From cf68677eac6d6a84c8ef286aadbddf11d3155e4c Mon Sep 17 00:00:00 2001 From: dcolazin Date: Tue, 17 Jan 2023 06:51:39 +0100 Subject: [PATCH 5/7] init game in GameLoader --- .../bhlangonijr/chesslib/pgn/GameLoader.java | 91 ++++++++----------- .../bhlangonijr/chesslib/PgnIteratorTest.java | 2 +- 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java index d0df22d..502e055 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java @@ -51,6 +51,10 @@ public class GameLoader { */ public static Game loadNextGame(Iterator iterator) { + if (!iterator.hasNext()) { + return null; + } + PgnTempContainer container = new PgnTempContainer(); while (iterator.hasNext()) { @@ -65,10 +69,8 @@ public static Game loadNextGame(Iterator iterator) { } else if (!line.equals("") && container.moveText != null) { addMoveText(line, container); if (isEndGame(line)) { - if (container.game != null) { - setMoveText(container.game, container.moveText); - } - return container.game; + setMoveText(container.game, container.moveText); + return container.initGame ? container.game : null; } } } catch (Exception e) { //TODO stricter exceptions @@ -77,7 +79,7 @@ public static Game loadNextGame(Iterator iterator) { throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); } } - return container.game; + return container.initGame ? container.game : null; } private static void addProperty(String line, PgnTempContainer container) throws Exception { @@ -85,11 +87,12 @@ private static void addProperty(String line, PgnTempContainer container) throws if (property == null) { return; } + container.initGame = true; String tag = property.name.toLowerCase().trim(); //begin switch (tag) { case "event": - if (container.moveTextParsing && container.game != null && container.game.getHalfMoves().size() == 0) { + if (container.moveTextParsing && container.game.getHalfMoves().size() == 0) { setMoveText(container.game, container.moveText); } container.event.setName(property.value); @@ -118,11 +121,9 @@ private static void addProperty(String line, PgnTempContainer container) throws if (container.round.getNumber() < 1) { container.round.setNumber(1); //TODO this is just to have the same behaviour as before... } - if (container.game == null) { - container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); - container.game.setDate(container.event.getStartDate()); - container.round.getGame().add(container.game); - } + + container.game.setDate(container.event.getStartDate()); //TODO this should be done only once + Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); player.setId(property.value); @@ -137,11 +138,9 @@ private static void addProperty(String line, PgnTempContainer container) throws if (container.round.getNumber() < 1) { container.round.setNumber(1); //TODO this just to have the same behaviour as before... } - if (container.game == null) { - container.game = GameFactory.newGame(UUID.randomUUID().toString(), container.round); - container.game.setDate(container.event.getStartDate()); - container.round.getGame().add(container.game); - } + + container.game.setDate(container.event.getStartDate()); //TODO this should be done only once + Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); player.setId(property.value); player.setDescription(property.value); @@ -151,23 +150,16 @@ private static void addProperty(String line, PgnTempContainer container) throws break; } case "result": - if (container.game != null) { - GameResult result = GameResult.fromNotation(property.value); - container.game.setResult(result); - } + container.game.setResult(GameResult.fromNotation(property.value)); break; case "plycount": - if (container.game != null) { - container.game.setPlyCount(property.value); - } + container.game.setPlyCount(property.value); break; case "termination": - if (container.game != null) { - try { - container.game.setTermination(Termination.fromValue(property.value.toUpperCase())); - } catch (Exception e1) { - container.game.setTermination(Termination.UNTERMINATED); - } + try { + container.game.setTermination(Termination.fromValue(property.value.toUpperCase())); + } catch (Exception e1) { + container.game.setTermination(Termination.UNTERMINATED); } break; case "timecontrol": @@ -180,29 +172,19 @@ private static void addProperty(String line, PgnTempContainer container) throws } break; case "annotator": - if (container.game != null) { - container.game.setAnnotator(property.value); - } + container.game.setAnnotator(property.value); break; case "fen": - if (container.game != null) { - container.game.setFen(property.value); - } + container.game.setFen(property.value); break; case "eco": - if (container.game != null) { - container.game.setEco(property.value); - } + container.game.setEco(property.value); break; case "opening": - if (container.game != null) { - container.game.setOpening(property.value); - } + container.game.setOpening(property.value); break; case "variation": - if (container.game != null) { - container.game.setVariation(property.value); - } + container.game.setVariation(property.value); break; case "whiteelo": if (container.whitePlayer != null) { @@ -223,17 +205,16 @@ private static void addProperty(String line, PgnTempContainer container) throws } break; default: - if (container.game != null) { - if (container.game.getProperty() == null) { - container.game.setProperty(new HashMap()); - } - container.game.getProperty().put(property.name, property.value); + if (container.game.getProperty() == null) { + container.game.setProperty(new HashMap<>()); } + container.game.getProperty().put(property.name, property.value); break; } } private static void addMoveText(String line, PgnTempContainer container) { + container.initGame = true; container.moveText.append(line); container.moveText.append('\n'); container.moveTextParsing = true; @@ -245,15 +226,21 @@ private static boolean isEndGame(String line) { private static class PgnTempContainer { - Event event = new Event(); - Round round = new Round(event); + Event event; + Round round; Game game; Player whitePlayer; Player blackPlayer; StringBuilder moveText; boolean moveTextParsing; + boolean initGame; - PgnTempContainer() {} + PgnTempContainer() { + this.event = new Event(); + this.round = new Round(event); + this.game = new Game(UUID.randomUUID().toString(), round); + this.round.getGame().add(this.game); + } } private static void setMoveText(Game game, StringBuilder moveText) throws Exception { diff --git a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java index e140419..d36007a 100644 --- a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java +++ b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java @@ -30,7 +30,7 @@ public void testPGNOrder() throws Exception { assertEquals("1951.06.20", game.getDate()); //assertEquals("1951.??.??", game.getRound().getEvent().getStartDate()); assertEquals(9, game.getRound().getNumber()); - //assertEquals(GameResult.WHITE_WON, game.getResult()); //TODO this is the issue + assertEquals(GameResult.WHITE_WON, game.getResult()); //TODO this is the issue assertEquals("Bent Larsen", game.getWhitePlayer().getName()); assertEquals("Lionel Joyner", game.getBlackPlayer().getName()); assertEquals("C30", game.getEco()); From 2c2dc7fadb4f5a5071dafdd74fb6681b8629d8d4 Mon Sep 17 00:00:00 2001 From: dcolazin Date: Tue, 17 Jan 2023 19:49:49 +0100 Subject: [PATCH 6/7] init players in GameLoader --- .../bhlangonijr/chesslib/pgn/GameLoader.java | 68 +++++++++---------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java index 502e055..73c4edc 100644 --- a/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java +++ b/src/main/java/com/github/bhlangonijr/chesslib/pgn/GameLoader.java @@ -20,6 +20,7 @@ import com.github.bhlangonijr.chesslib.game.Game; import com.github.bhlangonijr.chesslib.game.GameFactory; import com.github.bhlangonijr.chesslib.game.GameResult; +import com.github.bhlangonijr.chesslib.game.GenericPlayer; import com.github.bhlangonijr.chesslib.game.Player; import com.github.bhlangonijr.chesslib.game.PlayerType; import com.github.bhlangonijr.chesslib.game.Round; @@ -58,15 +59,14 @@ public static Game loadNextGame(Iterator iterator) { PgnTempContainer container = new PgnTempContainer(); while (iterator.hasNext()) { - String line = iterator.next(); + String line = iterator.next().trim(); + if (line.startsWith(UTF8_BOM)) { + line = line.substring(1); + } try { - line = line.trim(); - if (line.startsWith(UTF8_BOM)) { - line = line.substring(1); - } if (isProperty(line)) { addProperty(line, container); - } else if (!line.equals("") && container.moveText != null) { + } else if (!line.equals("")) { addMoveText(line, container); if (isEndGame(line)) { setMoveText(container.game, container.moveText); @@ -76,7 +76,7 @@ public static Game loadNextGame(Iterator iterator) { } catch (Exception e) { //TODO stricter exceptions String name = container.event.getName(); int r = container.round.getNumber(); - throw new PgnException("Error parsing PGN[" + r + ", " + name + "]: ", e); + throw new PgnException(String.format("Error parsing PGN[%d, %s]: ", r, name), e); } } return container.initGame ? container.game : null; @@ -97,7 +97,6 @@ private static void addProperty(String line, PgnTempContainer container) throws } container.event.setName(property.value); container.event.setId(property.value); - container.moveText = new StringBuilder(); //TODO initialize this break; case "site": container.event.setSite(property.value); @@ -124,14 +123,9 @@ private static void addProperty(String line, PgnTempContainer container) throws container.game.setDate(container.event.getStartDate()); //TODO this should be done only once - - Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); - player.setId(property.value); - player.setDescription(property.value); - - container.game.setWhitePlayer(player); - container.whitePlayer = player; - + container.whitePlayer.setId(property.value); + container.whitePlayer.setName(property.value); + container.whitePlayer.setDescription(property.value); break; } case "black": { @@ -141,12 +135,9 @@ private static void addProperty(String line, PgnTempContainer container) throws container.game.setDate(container.event.getStartDate()); //TODO this should be done only once - Player player = GameFactory.newPlayer(PlayerType.HUMAN, property.value); - player.setId(property.value); - player.setDescription(property.value); - - container.game.setBlackPlayer(player); - container.blackPlayer = player; + container.blackPlayer.setId(property.value); + container.blackPlayer.setName(property.value); + container.blackPlayer.setDescription(property.value); break; } case "result": @@ -187,21 +178,17 @@ private static void addProperty(String line, PgnTempContainer container) throws container.game.setVariation(property.value); break; case "whiteelo": - if (container.whitePlayer != null) { - try { - container.whitePlayer.setElo(Integer.parseInt(property.value)); - } catch (NumberFormatException e) { + try { + container.whitePlayer.setElo(Integer.parseInt(property.value)); + } catch (NumberFormatException e) { - } } break; case "blackelo": - if (container.blackPlayer != null) { - try { - container.blackPlayer.setElo(Integer.parseInt(property.value)); - } catch (NumberFormatException e) { + try { + container.blackPlayer.setElo(Integer.parseInt(property.value)); + } catch (NumberFormatException e) { - } } break; default: @@ -226,12 +213,14 @@ private static boolean isEndGame(String line) { private static class PgnTempContainer { - Event event; - Round round; - Game game; + //TODO many of this stuff can be accessed through game + + final Event event; + final Round round; + final Game game; Player whitePlayer; Player blackPlayer; - StringBuilder moveText; + final StringBuilder moveText; boolean moveTextParsing; boolean initGame; @@ -240,6 +229,13 @@ private static class PgnTempContainer { this.round = new Round(event); this.game = new Game(UUID.randomUUID().toString(), round); this.round.getGame().add(this.game); + this.whitePlayer = new GenericPlayer(); + this.whitePlayer.setType(PlayerType.HUMAN); + this.game.setWhitePlayer(this.whitePlayer); + this.blackPlayer = new GenericPlayer(); + this.blackPlayer.setType(PlayerType.HUMAN); + this.game.setBlackPlayer(this.blackPlayer); + this.moveText = new StringBuilder(); } } From b43b2f6a677f139c5132a4f665de7e5b95fb47b3 Mon Sep 17 00:00:00 2001 From: dcolazin Date: Tue, 17 Jan 2023 20:20:14 +0100 Subject: [PATCH 7/7] clean pgn order test --- .../github/bhlangonijr/chesslib/PgnIteratorTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java index d36007a..d0709aa 100644 --- a/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java +++ b/src/test/java/com/github/bhlangonijr/chesslib/PgnIteratorTest.java @@ -19,8 +19,6 @@ */ public class PgnIteratorTest { - //TODO test exception... - @Test public void testPGNOrder() throws Exception { PgnIterator games = new PgnIterator("src/test/resources/pgn_order.pgn"); @@ -28,14 +26,14 @@ public void testPGNOrder() throws Exception { assertEquals("1st World U20 ch", game.getRound().getEvent().getName()); assertEquals("Birmingham ENG", game.getRound().getEvent().getSite()); assertEquals("1951.06.20", game.getDate()); - //assertEquals("1951.??.??", game.getRound().getEvent().getStartDate()); + assertEquals("1951.06.20", game.getRound().getEvent().getStartDate()); assertEquals(9, game.getRound().getNumber()); - assertEquals(GameResult.WHITE_WON, game.getResult()); //TODO this is the issue + assertEquals(GameResult.WHITE_WON, game.getResult()); assertEquals("Bent Larsen", game.getWhitePlayer().getName()); assertEquals("Lionel Joyner", game.getBlackPlayer().getName()); assertEquals("C30", game.getEco()); - //assertEquals("?", game.getWhitePlayer().getElo()); //TODO have a null (not inserted), vs unknown (?) ? - //assertEquals("?", game.getBlackPlayer().getElo()); + assertEquals(0, game.getWhitePlayer().getElo()); //TODO have a null (not inserted), vs unknown (?) ? + assertEquals(0, game.getBlackPlayer().getElo()); assertEquals("63", game.getPlyCount()); //TODO why is it a string? //assertEquals("...", game.getHalfMoves().toString()); //TODO }