I have programmed this alarm application in Java using JavaFX for the GUI. I'd like these questions covered in the review:Is my code efficient? Can it be shortened?Are there any flaws in my code?Do I violate any OOP principles? If so, how, where and how do I rectify them?App UI:Alarm.java:/* * Program developed by Hassan Althaf. * Copyright  2015, Hassan Althaf. * Website: http://hassanalthaf.com */package com.HassanAlthaf;import javafx.application.Application;import javafx.fxml.FXMLLoader;import javafx.scene.Parent;import javafx.scene.Scene;import javafx.stage.Stage;public class Alarm extends Application {    @Override    public void start(Stage stage) throws Exception {        Parent root = FXMLLoader.load(getClass().getResource(MainWindow.fxml));        Scene scene = new Scene(root);        stage.setScene(scene);        stage.setResizable(false);        stage.setTitle(Alarm Application - Hassan Althaf);        stage.show();    }    public static void main(String[] args) {        launch(args);    }}MainView.java:/* * Program developed by Hassan Althaf. * Copyright  2015, Hassan Althaf. * Website: http://hassanalthaf.com */package com.HassanAlthaf;import java.net.URL;import java.util.ResourceBundle;import java.util.Timer;import java.util.TimerTask;import javafx.event.ActionEvent;import javafx.fxml.FXML;import javafx.fxml.Initializable;import javafx.scene.control.Alert;import javafx.scene.control.Alert.AlertType;import javafx.scene.control.TextField;/** * * @author hassan */public class MainView implements Initializable {    @FXML    private TextField hoursField;    @FXML    private TextField minutesField;    @FXML    private TextField secondsField;    private final AlarmController alarmController;    private Timer timer;    private boolean timerOn = false;    private boolean alarmPlayed = false;    private boolean timerValidation = true;    @FXML    private void startAlarm(ActionEvent event) {        if(!timerOn) {            this.alarmPlayed = false;            this.timerValidation = true;            String hoursString = this.hoursField.getText();            String minutesString = this.minutesField.getText();            String secondsString = this.secondsField.getText();            if(hoursString.equals()) {                hoursString = 0;            }            if(minutesString.equals()) {                minutesString = 0;            }            if(secondsString.equals()) {                secondsString = 0;            }            int hours = 0;            int minutes = 0;            int seconds = 0;            try {                hours = Integer.parseInt(hoursString);                minutes = Integer.parseInt(minutesString);                seconds = Integer.parseInt(secondsString);            } catch(Exception ex) {                this.timerValidation = false;                this.showWarning(Please enter a number only for the times!, Invalid input received);            }            int totalTime = seconds + (minutes * 60) + (hours * 3600);            if(totalTime == 0) {                this.timerValidation = false;                this.showWarning(Please set a time for the alarm before starting!, Time not found!);            }            if(this.timerValidation) {                TimerTask task = new TimerTask() {                    private final int ALARM_TIME = totalTime;                    private int secondCounter = 0;                    @Override                    public void run()                    {                        if(secondCounter < ALARM_TIME) {                            secondCounter++;                        } else {                            if(timerOn) {                                alarmController.playAlarm();                                alarmPlayed = true;                            }                            timerOn = false;                            cancel();                        }                    }                };                this.timer = new Timer();                this.timer.schedule(task, 0, 1000);                this.timerOn = true;            }        } else {            this.showWarning(An alarm is already running at the moment! If you wish to override the current alarm, please stop it first!, Cannot override current alarm.);        }    }    public void showWarning(String message, String title) {        Alert alert = new Alert(AlertType.WARNING);        alert.setTitle(title);        alert.setHeaderText(null);        alert.setContentText(message);        alert.showAndWait();    }    @FXML    private void stopAlarm(ActionEvent event) {        if(this.timerOn) {            this.timer.cancel();            this.timer.purge();            this.timerOn = false;        }        if(this.alarmPlayed) {            this.alarmController.stopAlarm();            this.alarmPlayed = false;        }    }    public MainView() {        this.alarmController = new AlarmController();    }    @Override    public void initialize(URL url, ResourceBundle rb) {    }    }MainWindow.fxml:<?xml version=1.0 encoding=UTF-8?><?import javafx.scene.text.*?><?import java.lang.*?><?import java.util.*?><?import javafx.scene.*?><?import javafx.scene.control.*?><?import javafx.scene.layout.*?><AnchorPane id=AnchorPane maxHeight=300.0 maxWidth=500.0 minHeight=300.0 minWidth=500.0 prefHeight=300.0 prefWidth=500.0 xmlns=http://javafx.com/javafx/8 xmlns:fx=http://javafx.com/fxml/1 fx:controller=com.HassanAlthaf.MainView>   <children>      <Label layoutX=171.0 layoutY=30.0 text=Alarm Application textFill=#404040>         <font>            <Font name=Lato Regular size=20.0 />         </font>      </Label>      <Label layoutX=150.0 layoutY=68.0 text=Set the timer for your alarm below />      <TextField fx:id=hoursField layoutX=193.0 layoutY=108.0 />      <Label layoutX=150.0 layoutY=113.0 text=Hours:  />      <Label layoutX=138.0 layoutY=150.0 text=Minutes:  />      <TextField fx:id=minutesField layoutX=193.0 layoutY=145.0 />      <TextField fx:id=secondsField layoutX=193.0 layoutY=182.0 />      <Label layoutX=133.0 layoutY=187.0 text=Seconds:  />      <Button layoutX=251.0 layoutY=222.0 mnemonicParsing=false onAction=#startAlarm text=Start />      <Button layoutX=309.0 layoutY=222.0 mnemonicParsing=false onAction=#stopAlarm text=Stop />   </children></AnchorPane>AlarmController.java:/* * Program developed by Hassan Althaf. * Copyright  2015, Hassan Althaf. * Website: http://hassanalthaf.com */package com.HassanAlthaf;import javafx.scene.media.Media;import javafx.scene.media.MediaPlayer;/** * * @author hassan */public class AlarmController {    private MediaPlayer mediaPlayer;    public void playAlarm()    {        Media media = new Media(getClass().getResource(alarm.mp3).toString());        this.mediaPlayer = new MediaPlayer(media);        this.mediaPlayer.play();    }    public void stopAlarm()    {        this.mediaPlayer.stop();    }}
An alarm application in Java
java;gui;javafx
It seems good overall.I'm not familiar with JavaFX threading, but it seems that you at least have a threading bug for the shared variables which you modify within your TimerTask.run().  The flags alarmPlayed and timerOn should be declared AtomicBoolean.  I checked the doc of MediaPlayer to see if it can be started and stopped from different threads. I did not find an answer, but I would guess it does.I would invert the two lines:   this.timer.schedule(task, 0, 1000); this.timerOn = true;but more generally, I think I would get rid of this.timerOn and instead just set this.timer = null to indicate there is no timer.It not clear why you are waking up every second to check if the time is done instead of just setting the timer once for the final time.  The later would simplify your code a lot.  My guess is that maybe you were planning to add some animation at each second in the future.private final int ALARM_TIME = totalTime is not needed.  You can use totalTime directly within TimerTask.run().  You might have to declare it final.You sometimes use C bracket placement.Answers to Questions in the Commments:Multi-threading (also known as concurrency) is a difficult and subtle topic.  The best reference is Java Concurrency In Practice; you can also take a look at the tutorial.  The problem is that modern cpus don't have a single copy of each variable in memory, but have many copies since each cpu also has some cache.  Thread-safe code means that you make sure that when a thread modifies a variable, the other threads see that change.  You don't want all variables to be shared between threads, only those that do need to be shared, since keeping variables up-to-date between cores has some cost since the values in the various core caches must be synchronized.In your case, you have two different threads: 1) the thread that runs the JavaFX UI and 2) the timer thread which runs TimerTask.run().  The two variables alarmPlayed and timerOn are used by both threads.  What can happen is that your code in the timer can change those variables, but that change might never be seen by the code in the JavaFX UI thread.  Where things get even more tricky is that although your program might have a bug formally, it can happen in practice that for the particular cpu you are using, this bug might never arise.  Where things get even trickier, is that it is even possible that the timer has some inner synchronization that makes it unnecessary to add any other explicit synchronization controls.  One would have to understand the Java Memory Model to tell if it's the case or not, but I don't think anyone fully understands it except the guys who wrote Java.AtomicBoolean, or volatile boolean, means that whenever such a variable is modified by one thread, all threads immediately see the updated value (the caches between all cpu cores are synchronized).For your question about the timer not waking every single second, there are also methods in Timer which instead of calling a repeating task, just call the task a single time.  See this version of Timer.schedule which takes as argument a task and the number of milliseconds after which the task will be executed, a single time.public class A { // Java bracket placement     public void method()     {  // C bracket placement (should be at the end of the above line)        ...     }}