Feature/config parser cleanup (#8)

* changed sysout to log

* deleted model from class ConfigParser

* deleted model and spyModel from ConfigParserTest.java

* renamed method parserConfig() to parseConfig()

* corrected title of parseConfigButton and removed unnecessary import statement in ControllerTest.java

* improved log statements

Author: mplieske

src/main/java/de/doubleslash/keeptime/common/ConfigParser.java

@@ -5,23 +5,23 @@
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
 import de.doubleslash.keeptime.controller.Controller;
-import de.doubleslash.keeptime.model.Model;
 import de.doubleslash.keeptime.model.Project;
 import javafx.scene.paint.Color;
 
 public class ConfigParser {
+   private final Logger LOG = LoggerFactory.getLogger(this.getClass());
 
-   Controller controller;
-   Model model;
+   private final Controller controller;
 
-   public ConfigParser(final Model model, final Controller controller) {
-      this.model = model;
+   public ConfigParser(final Controller controller) {
       this.controller = controller;
    }
 
@@ -30,15 +30,15 @@ public static boolean hasConfigFile(final String fileName) {
       return f.exists();
    }
 
-   public void parserConfig(final File inputFile) {
+   public void parseConfig(final File inputFile) {
+      LOG.info("Starting import of projects in file '{}'.", inputFile);
       try {
          final DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
          final DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
          final Document doc = dBuilder.parse(inputFile);
          doc.getDocumentElement().normalize();
-         System.out.println("Root element :" + doc.getDocumentElement().getNodeName());
+         LOG.debug("Root element '{}'.", doc.getDocumentElement().getNodeName());
          final NodeList nList = doc.getElementsByTagName("project");
-         System.out.println("----------------------------");
 
          // index makes sure to add new projects at the end
          int index = controller.getAvailableProjects().size();
@@ -51,26 +51,29 @@ public void parserConfig(final File inputFile) {
                final String isWork = eElement.getElementsByTagName("isWork").item(0).getTextContent();
                final String color = eElement.getElementsByTagName("color").item(0).getTextContent();
 
-               System.out.println("checking for: " + name);
+               LOG.debug("Testing if project with name '{}' already exists.", name);
                boolean exists = false;
 
                for (final Project p : controller.getAvailableProjects()) {
                   if (name.equals(p.getName())) {
-                     System.out.println(name + "    " + Boolean.toString(exists));
                      exists = true;
+                     break;
                   }
                }
                if (!exists) {
+                  LOG.info("Adding project '{}'.", name);
                   final Color colorTemp = Color.valueOf(color);
                   controller.addNewProject(name, Boolean.parseBoolean(isWork), colorTemp, index);
                   index++;
+               } else {
+                  LOG.debug("Project '{}' already exists", name);
                }
 
             }
          }
-
+         LOG.info("Import of '{}' finished.", inputFile);
       } catch (final Exception e) {
-         e.printStackTrace();
+         LOG.error("There was an error while importing projects from config.xml", e);
       }
    }
 }

src/main/java/de/doubleslash/keeptime/view/SettingsController.java

@@ -98,8 +98,8 @@ private void initialize() {
          @Override
          public void handle(final ActionEvent actionEvent) {
             if (ConfigParser.hasConfigFile(INPUT_FILE)) {
-               final ConfigParser parser = new ConfigParser(model, controller);
-               parser.parserConfig(new File(INPUT_FILE));
+               final ConfigParser parser = new ConfigParser(controller);
+               parser.parseConfig(new File(INPUT_FILE));
             }
          }
       });

src/main/resources/settings.fxml

@@ -118,7 +118,7 @@
                <children>
                   <Button fx:id="saveButton" mnemonicParsing="false" text="Save" />
                   <Button fx:id="cancelButton" layoutX="10.0" layoutY="10.0" mnemonicParsing="false" text="Cancel" />
-                  <Button fx:id="parseConfigButton" mnemonicParsing="false" text="pars config.xml" />
+                  <Button fx:id="parseConfigButton" mnemonicParsing="false" text="parse config.xml" />
                </children>
             </HBox>
          </children>

src/test/java/de/doubleslash/keeptime/common/ConfigParserTest.java

@@ -10,7 +10,6 @@
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -19,18 +18,13 @@
 
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import de.doubleslash.keeptime.controller.Controller;
-import de.doubleslash.keeptime.model.Model;
 import de.doubleslash.keeptime.model.Project;
-import de.doubleslash.keeptime.model.repos.ProjectRepository;
-import de.doubleslash.keeptime.model.repos.SettingsRepository;
-import de.doubleslash.keeptime.model.repos.WorkRepository;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.scene.paint.Color;
@@ -47,16 +41,10 @@ public class ConfigParserTest {
    private final Project hinzUndKunz = new Project("Hinz und Kunz", Color.DARKRED, true, 3);
    private final ObservableList<Project> projects = FXCollections.observableArrayList();
 
-   private Model model;
-   private ObservableList<Project> spyModel;
    private final Controller controller = mock(Controller.class);
 
    @Before
    public void beforeTest() {
-      model = new Model(Mockito.mock(ProjectRepository.class), Mockito.mock(WorkRepository.class),
-            Mockito.mock(SettingsRepository.class));
-      spyModel = spy(model.availableProjects);
-
       projects.add(idle);
       projects.add(heinz);
       projects.add(peter);
@@ -66,7 +54,6 @@ public void beforeTest() {
    @Test
    public void testImportFromFileOnce() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {
@@ -88,15 +75,14 @@ public Object answer(final InvocationOnMock invocation) {
    }
 
    private void loadConfigFile(final String configFile) {
-      final ConfigParser testSubject = new ConfigParser(model, controller);
+      final ConfigParser testSubject = new ConfigParser(controller);
       final File inputfile = new File(configFile);
-      testSubject.parserConfig(inputfile);
+      testSubject.parseConfig(inputfile);
    }
 
    @Test
    public void testDoNotImportIdle() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {
@@ -117,7 +103,6 @@ public Object answer(final InvocationOnMock invocation) {
    @Test
    public void testImportNonexistingProjects() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {
@@ -138,7 +123,6 @@ public Object answer(final InvocationOnMock invocation) {
    @Test
    public void testIndexIsRight() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {
@@ -159,7 +143,6 @@ public Object answer(final InvocationOnMock invocation) {
    @Test
    public void testProject1IsParsedCorrectly() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {
@@ -179,7 +162,6 @@ public Object answer(final InvocationOnMock invocation) {
    @Test
    public void testProject2IsParsedCorrectly() {
       when(controller.getAvailableProjects()).thenReturn(projects);
-      when(spyModel.size()).thenReturn(4);
       doAnswer(new Answer() {
          @Override
          public Object answer(final InvocationOnMock invocation) {