Fix #2

Guify/src/code/GuiAbstractions/Implementations/JFrameFactory.java

@@ -8,44 +8,8 @@ import views.*;
  */
 public class JFrameFactory implements IFrameFactory {
 	
-	public static Object createJFrame(int frameType) throws Exception {
-		
-		switch(frameType) {
-		
-		case LOGIN:
-			throw new Exception("Frame Login requires additional parameters. "
-					+ "Call createJFrame(int frameType, Object additionalParameters) instead");
-			
-		case DESKTOP:
-			throw new Exception("Frame Desktop requires additional parameters. "
-					+ "Call createJFrame(int frameType, Object additionalParameters) instead");
-			
-		case QUEUE:
-			return new Queue();
-			
-		case NOTEPAD:
-			throw new Exception("Frame Notepad requires additional parameters. "
-					+ "Call createJFrame(int frameType, Object additionalParameters) instead");
-			
-		case FIND_AND_REPLACE:
-			throw new Exception("Frame FindAndReplace requires additional parameters. "
-					+ "Call createJFrame(int frameType, Object additionalParameters) instead");
-			
-		default:
-			throw new Exception("Invalid frame name");
-		}
-	}
-	
 	public static Object createJFrame(int frameType, Object controller) throws Exception{
 		
-		if(	frameType != NOTEPAD 
-			&& frameType != FIND_AND_REPLACE 
-			&& frameType != LOGIN 
-			&& frameType != DESKTOP) {
-				System.err.println("additionalParams ignored for this frame");
-				return createJFrame(frameType);
-		}
-		
 		switch(frameType) {
 		
 		case LOGIN:
@@ -59,6 +23,9 @@ public class JFrameFactory implements IFrameFactory {
 			
 		case FIND_AND_REPLACE:
 			return new FindAndReplace(controller);
+			
+		case QUEUE:
+			return new Queue(controller);
 		
 		default:
 			throw new Exception("Invalid frame name");

Guify/src/controllers/QueueController.java

@@ -2,37 +2,32 @@ package controllers;
 
 import java.util.Observable;
 import java.util.Observer;
+import javax.swing.SwingUtilities;
 import code.QueueEventManager;
 import code.TransferProgress;
 import code.GuiAbstractions.Implementations.JFrameFactory;
 import code.GuiAbstractions.Interfaces.IFrameFactory;
 import views.interfaces.IQueueFrame;
+import java.util.HashMap;
 
-import java.util.concurrent.ConcurrentHashMap;
+@SuppressWarnings("deprecation")
-import javax.swing.SwingUtilities;
+public class QueueController implements Observer{
-
+	
-import com.jcraft.jsch.SftpProgressMonitor;
+	/*
-
+	 * ========== BEGIN Attributes ==========
-@SuppressWarnings("deprecation") // Observer/Observable objects are okay here
+	 */
-public class QueueController implements Observer {
 	
 	private IQueueFrame frame;
-	// A HashMap containing the Transfer Progress entry and the index of said entry
+	private HashMap<TransferProgress, Integer> indexHashMap = new HashMap<>();
-	// in the table.
+	
-	// We need a ConcurrentHashMap instead of a simple HashMap because it
+	/*
-	// is accessed by multiple threads. In particular the threads executing update()
+	 * ========== END Attributes ==========
-	// and the Event Dispatch Thread.
+	 */
-	// Alternatively, the HashMap could've been managed by the view itself without the need
-	// to concern over threading/concurrent problems.
-	// TODO Make the HashMap to be handled by the view itself (EDT thread) rather than concurrent threads
-	// to enhance readability and understanding
-	private ConcurrentHashMap<TransferProgress, Integer> indexAssociationMap = new ConcurrentHashMap<>();
-	private final static int HASHMAP_DUMMY_VALUE = -1;
 	
 	// Executed by the EDT
 	public QueueController() {
 		try {
-			frame = (IQueueFrame) JFrameFactory.createJFrame(IFrameFactory.QUEUE);
+			frame = (IQueueFrame) JFrameFactory.createJFrame(IFrameFactory.QUEUE, this);
 		} 
 		catch (Exception e) {}
 		
@@ -43,126 +38,55 @@ public class QueueController implements Observer {
 		// transfers could go lost
 		TransferProgress[] queued = QueueEventManager.getInstance().getQueue();
 		for(TransferProgress transferProgress : queued) {
-			// It is possible that while iterating on this for, the element
+			SwingUtilities.invokeLater(new Runnable() {
-			// has already been inserted into the indexAssociationMap from
+			      @Override
-			// another thread executing update(), hence, we check if the key is contained 
+			      public void run() {
-			// already.
+			    	  frame.manageTransferProgress(transferProgress);
-			if (indexAssociationMap.putIfAbsent(transferProgress, HASHMAP_DUMMY_VALUE) == null) {
+			      }
-				int percentage = (int) Math.floor( ((transferProgress.getTransferredBytes() * 100) / transferProgress.getTotalBytes()) );
+			});
-				int rowIndex = frame.addRow(transferProgress.getSource(), transferProgress.getDestination(), transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", percentage);
-		        indexAssociationMap.put(transferProgress, rowIndex);
-			}
 		}
 	}
 	
-	// Updated by QueueEventManager. Can run simultaneously
+	// Executed by different threads
-	// on multiple threads.
 	@Override
 	public void update(Observable o, Object arg) {
-		TransferProgress transferProgressObj = (TransferProgress)arg;
+		TransferProgress transferProgress = (TransferProgress)arg;
-		if(transferProgressObj.getTransferStatus() == TransferProgress.INIT) {
+		SwingUtilities.invokeLater(new Runnable() {
-			// Since the Runnable in SwingUtilities.invokeLater contained in this if
+		      @Override
-			// might run *after* a subsequent execution of
+		      public void run() {
-			// update() having a TransferProgress whose status is UPDATING, said subsequent update()
+		    	  frame.manageTransferProgress(transferProgress);
-			// must know that this specific transferProgressObj was already inserted in the HashMap,
+		      }
-			// otherwise the if (indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null)
+		});
-			// (contained in the if checking whether the status is UPDATING)
+	}
-			// would return true and add it again to the table, because indexAssociationMap.put(transferProgressObj, rowIndex);
+	
-			// has not been completed yet, being in the Runnable.
+	public int computePercentage(TransferProgress transferProgress) {
-			//
+		// Avoid division by zero
-			// We call putIfAbsent() instead of just put() because it is possible that this transferProgressObj is also
+		if(transferProgress.getTotalBytes() == 0) {
-			// in the initial for present in Queue() if this transfer was initiated in a time x where
+			// The percentage is 100% if ∀ byte in the file, byte was transferred.
-			// t1 < x < t2, where t1 is the time of completion of QueueEventManager.getInstance().addObserver(this);
+			// If there are no bytes in the file, this logic proposition holds true (vacuous truth)
-			// and t2 is the time of completion of QueueEventManager.getInstance().getQueue(); 
+			return 100;
-			//
-			// To sum it up:
-			// 1) update() receives transferProgressObj whose status is "INIT". Puts "DUMMY_VALUE"
-			// and schedules the EDT for a table insertion;
-			// 2) update() receives the same transferProgressObj with the status "UPDATING".
-			// It successfully sees that indexAssociationMap contains this specific transferProgressObj,
-			// whose value is DUMMY_VALUE, so it does not schedule an insert, but rather an update. While
-			// updating it will see that the value is not valid (if(rowIndex != HASHMAP_DUMMY_VALUE)) and will
-			// not perform an update as well.
-			// 3) The EDT runs and puts the correct index in the HashMap.
-			//
-			// Remember that if an update() receives a transferProgress whose status is INIT, that update() will always run
-			// before an update() on the same transferProgress whose status is UPDATING, as the thread handling
-			// each transferProgress is one and one only.
-			if(indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null) {
-				// We need SwingUtilities.invokeLater because we
-				// are not on the Event Dispatch Thread (the thread
-				// responsible for GUI management), but rather on the
-				// thread created in SshEngine
-				SwingUtilities.invokeLater(new Runnable() {
-				      @Override
-				      public void run() {
-				    	  int rowIndex = frame.addRow(transferProgressObj.getSource(), transferProgressObj.getDestination(), transferProgressObj.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", 0);
-				    	  indexAssociationMap.put(transferProgressObj, rowIndex);
-				      }
-				});
-			}
 		}
-		else if(transferProgressObj.getTransferStatus() == TransferProgress.UPDATING) {
+		
-			int percentage;
+		// Avoid "stuck at 99%" due to precision issues
-			// Avoid division by zero
+		else if(transferProgress.getTotalBytes() == transferProgress.getTransferredBytes()) {
-			if(transferProgressObj.getTotalBytes() == 0) {
+			return 100;
-				// The percentage is 100% if ∀ byte in the file, byte was transferred.
-				// If there are no bytes in the file, this logic proposition holds true (vacuous truth)
-				percentage = 100;
-			}
-			else {
-				percentage = (int) Math.floor( ((transferProgressObj.getTransferredBytes() * 100) / transferProgressObj.getTotalBytes()) );
-			}
-			// It is possible to receive TransferProgress.UPDATING without receiving
-			// a TransferProgress.INIT (when this controller gets created when the transferring
-			// was already occurring) and before "for(TransferProgress transferProgress : queued)"
-			// gets executed on this element
-			if (indexAssociationMap.putIfAbsent(transferProgressObj, HASHMAP_DUMMY_VALUE) == null) {
-				// We need SwingUtilities.invokeLater because we
-				// are not on the Event Dispatch Thread (the thread
-				// responsible for GUI management), but rather on the
-				// thread created in SshEngine
-				SwingUtilities.invokeLater(new Runnable() {
-				      @Override
-				      public void run() {
-				    	  int rowIndex = frame.addRow(transferProgressObj.getSource(), transferProgressObj.getDestination(), transferProgressObj.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", percentage);
-				    	  indexAssociationMap.put(transferProgressObj, rowIndex);
-				      }
-				});
-			}
-			else {
-				int rowIndex = indexAssociationMap.get(transferProgressObj);
-				// It is possible that rowIndex is a DUMMY_VALUE if the insertion
-				// into the table has been scheduled with SwingUtilities.invokeLater
-				// but has not ran yet
-				if(rowIndex != HASHMAP_DUMMY_VALUE) {
-					// We need SwingUtilities.invokeLater because we
-					// are not on the Event Dispatch Thread (the thread
-					// responsible for GUI management), but rather on the
-					// thread created in SshEngine
-					SwingUtilities.invokeLater(new Runnable() {
-					      @Override
-					      public void run() {
-					    	  frame.updateRow(rowIndex, percentage);
-					      }
-					});
-				}
-				else {
-					// TODO If the file is small enough, in all the (few)
-					// updates, rowIndex could always be HASHMAP_DUMMY_VALUE
-					// because when SwingUtilities tries to insert the row the first time,
-					// the EDT task will not execute soon enough before the termination
-					// of the transfer, so we will see that the percentage it's fixed
-					// to a specific value.
-				}
-			}
 		}
-		else if(transferProgressObj.getTransferStatus() == TransferProgress.END) {
+		
-			// We choose not to remove the element from the table once it has finished
+		else {
+			return (int) Math.floor( ((transferProgress.getTransferredBytes() * 100) / transferProgress.getTotalBytes()) );
 		}
 	}
 	
+	public Integer getTableIndex(TransferProgress transferProgress) {
+		return indexHashMap.get(transferProgress);
+	}
+	
+	public void putTableIndex(TransferProgress transferProgress, Integer index) {
+		indexHashMap.put(transferProgress, index);
+	}
+	
 	public void showFrame(boolean visible) {
-		frame.setVisible(true);
+		frame.setVisible(visible);
 	}
+	
 }

Guify/src/views/Queue.java

@@ -11,19 +11,24 @@ import javax.swing.table.DefaultTableCellRenderer;
 import javax.swing.table.DefaultTableModel;
 import javax.swing.table.TableColumn;
 
+import com.jcraft.jsch.SftpProgressMonitor;
+
 import code.Constants;
+import code.TransferProgress;
+import controllers.QueueController;
 import views.interfaces.IQueueFrame;
 public class Queue extends JFrame implements IQueueFrame {
 
 	private static final long serialVersionUID = 1L;
-
+	private QueueController controller;
+	
 	/**
 	 *
 	 * Custom cell renderer in order to be able to display
 	 * a progress bar in the JTable
 	 *
 	 */
-	public static class ProgressBarTableCellRenderer extends DefaultTableCellRenderer {
+ 	public static class ProgressBarTableCellRenderer extends DefaultTableCellRenderer {
 		private static final long serialVersionUID = 1L;
 		private JProgressBar progressBar;
 		
@@ -52,7 +57,8 @@ public class Queue extends JFrame implements IQueueFrame {
 	
 	public DefaultTableModel tableModel;
 	
-	public Queue() {
+	public Queue(Object controller) {
+		this.controller = (QueueController) controller;
 		setTitle("Queue");
 		String[] columnNames = {"Source", "Destination", "Operation", "Percentage"};
         tableModel = new DefaultTableModel(columnNames, 0);
@@ -81,4 +87,37 @@ public class Queue extends JFrame implements IQueueFrame {
 			tableModel.setValueAt(percentage, rowIndex, 3);
 		}
 	}
+
+	@Override
+	public void manageTransferProgress(TransferProgress transferProgress) {
+		
+		if(transferProgress.getTransferStatus() == TransferProgress.INIT) {
+			if(controller.getTableIndex(transferProgress) == null) {
+				controller.putTableIndex(transferProgress, 
+								addRow(transferProgress.getSource(), 
+								transferProgress.getDestination(), 
+								transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", 
+								0));
+			}
+		}
+		
+		else if(transferProgress.getTransferStatus() == TransferProgress.UPDATING) {
+			Integer tableIndex = controller.getTableIndex(transferProgress);
+			if(tableIndex == null) {
+				controller.putTableIndex(transferProgress, 
+								addRow(transferProgress.getSource(), 
+								transferProgress.getDestination(), 
+								transferProgress.getOperation() == SftpProgressMonitor.GET? "Download" : "Upload", 
+								controller.computePercentage(transferProgress)));
+			}
+			else {
+				updateRow(tableIndex, controller.computePercentage(transferProgress));
+			}
+		}
+		
+		else if(transferProgress.getTransferStatus() == TransferProgress.END) {
+			// Do nothing
+		}
+		
+	}
 }

Guify/src/views/interfaces/IQueueFrame.java

@@ -1,7 +1,8 @@
 package views.interfaces;
 
+import code.TransferProgress;
+
 public interface IQueueFrame {
 	public void setVisible(boolean visible);
-	int addRow(String source, String destination, String operation, int percentage);
+	public void manageTransferProgress(TransferProgress transferProgress);
-	void updateRow(int rowIndex, int percentage);
 }