440821: code clean up and refactoring

Change-Id: Ibd10632a80957c9d56bc71e72dfbbdbf16d26e7b
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=440821

6 files changed, 196 insertions, 300 deletions

diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/actions/AddLineCommentToFileAction.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/actions/AddLineCommentToFileAction.java
index 07e516850..f1a63c089 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/actions/AddLineCommentToFileAction.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/actions/AddLineCommentToFileAction.java
@@ -92,7 +92,12 @@ public class AddLineCommentToFileAction extends AbstractReviewAction {
 	protected LineRange getSelectedRange() {
 		//if its the action from the compare editor, get currently selected lines
 		if (compareSourceViewer != null) {
-			return compareSourceViewer.getSelection();
+			LineRange selectedRange = compareSourceViewer.getSelection();
+			int maxNumberOfLines = compareSourceViewer.getAnnotationModel().getDocument().getNumberOfLines();
+			if (selectedRange.getStartLine() == maxNumberOfLines) {
+				selectedRange = new LineRange(maxNumberOfLines - 1, 1);
+			}
+			return selectedRange;
 		} else {
 			return getJavaEditorSelection(getEditorInput());
 		}
@@ -102,12 +107,8 @@ public class AddLineCommentToFileAction extends AbstractReviewAction {
 		return editorInput;
 	}
 
-	public ILocation getLocation() {
+	private ILocation getLocation() {
 		LineRange selectedRange = getSelectedRange();
-		int maxNumberOfLines = compareSourceViewer.getAnnotationModel().getDocument().getNumberOfLines();
-		if (selectedRange.getStartLine() == maxNumberOfLines) {
-			selectedRange = new LineRange(maxNumberOfLines - 1, 1);
-		}
 		ILineLocation location = ReviewsFactory.eINSTANCE.createLineLocation();
 		ILineRange range = ReviewsFactory.eINSTANCE.createLineRange();
 		range.setStart(selectedRange.getStartLine());
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentPopupDialog.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentPopupDialog.java
index 426c7d3d9..ab27ca9e7 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentPopupDialog.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/CommentPopupDialog.java
@@ -12,8 +12,8 @@
 
 package org.eclipse.mylyn.internal.reviews.ui.annotations;
 
-import java.util.HashMap;
-import java.util.Map;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.eclipse.jface.action.Action;
 import org.eclipse.jface.dialogs.PopupDialog;
@@ -202,28 +202,20 @@ public class CommentPopupDialog extends PopupDialog implements IReviewActionList
 
 			currentPopupDialog = this;
 
-			final Map<Integer, Comment> commentList = new HashMap<Integer, Comment>();
-
-			int count = 0;
+			final List<Comment> commentList = new ArrayList<Comment>();
 			for (CommentAnnotation annotation : annotationInput.getAnnotations()) {
 
 				if ((reviewitem != null)
 						&& reviewitem.getReview() != null
 						&& reviewitem.getReview().getRepository() != null
 						&& reviewitem.getReview().getRepository().getAccount() != null
-						&& reviewitem.getReview()
-								.getRepository()
-								.getAccount()
-								.toString()
-								.compareTo(((Comment) annotation.getComment()).getAuthor().toString()) != 0
+						&& reviewitem.getReview().getRepository().getAccount() != ((Comment) annotation.getComment()).getAuthor()
 						&& ((Comment) annotation.getComment()).isDraft()) {
 					continue;
 				}
 
 				CommentPart part = new CommentPart(annotation.getComment(), annotationInput.getBehavior());
-				count++;
-
-				commentList.put(new Integer(count), (Comment) annotation.getComment());
+				commentList.add((Comment) annotation.getComment());
 				part.hookCustomActionRunListener(this);
 				Control control = part.createControl(composite, toolkit);
 				toolkit.adapt(control, true, true);
@@ -238,11 +230,7 @@ public class CommentPopupDialog extends PopupDialog implements IReviewActionList
 			focusLabel.setForeground(toolkit.getColors().getColor(IFormColors.TITLE));
 			GridDataFactory.fillDefaults().align(SWT.RIGHT, SWT.CENTER).applyTo(focusLabel);
 
-			PixelConverter Pc = new PixelConverter(composite.getFont());
-
-			final int avgHeight = Pc.convertHeightInCharsToPixels(1);
-
-			focusLabel.getParent().addMouseTrackListener(mouseTrackAdapter(commentList, avgHeight));
+			focusLabel.getParent().addMouseTrackListener(mouseTrackAdapter(commentList));
 
 		} else {
 			input = null;
@@ -250,7 +238,9 @@ public class CommentPopupDialog extends PopupDialog implements IReviewActionList
 
 	}
 
-	private MouseTrackAdapter mouseTrackAdapter(final Map<Integer, Comment> commentList, final int avgHeight) {
+	private MouseTrackAdapter mouseTrackAdapter(final List<Comment> commentList) {
+		PixelConverter Pc = new PixelConverter(composite.getFont());
+		final int lineHeight = Pc.convertHeightInCharsToPixels(1);
 		return new MouseTrackAdapter() {
 
 			@Override
@@ -271,9 +261,9 @@ public class CommentPopupDialog extends PopupDialog implements IReviewActionList
 
 					// adjust size to display maximum of 15 lines, which means 5 comments
 					if (annotationInput.getAnnotations().size() < 5) {
-						size.y = size.y + (annotationInput.getAnnotations().size() * 3 * avgHeight);
+						size.y = size.y + (annotationInput.getAnnotations().size() * 3 * lineHeight);
 					} else {
-						size.y = size.y + (15 * avgHeight);
+						size.y = size.y + (15 * lineHeight);
 					}
 					currentCommentInputDialog.getShell().setSize(size);
 					currentCommentInputDialog.open();
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
index e9daa8765..274fd9e2c 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/ReviewAnnotationModel.java
@@ -227,34 +227,16 @@ public class ReviewAnnotationModel implements IAnnotationModel {
 	}
 
 	private void createCommentAnnotations(IDocument document, AnnotationModelEvent event, IComment comment) {
-		int startLine = 0;
-		int endLine = 0;
 		//TODO We need to ensure that this works properly with cases where 0 or many locations exist.
 		for (ILocation location : comment.getLocations()) {
 			if (location instanceof ILineLocation) {
-				ILineLocation lineLocation = (ILineLocation) location;
 				try {
-					startLine = lineLocation.getRangeMin();
-					endLine = lineLocation.getRangeMax();
-
-					int offset = 0;
-					int length = 0;
-					if (startLine != 0 && startLine <= document.getNumberOfLines()) {
-
-						offset = document.getLineOffset(startLine - 1);
-						if (endLine == 0) {
-							endLine = startLine;
-						}
-						length = Math.max(document.getLineOffset(endLine - 1) - offset, 0);
-
-					}
-					length = Math.max(1, length);
-					CommentAnnotation ca = new CommentAnnotation(offset, length, comment);
+					CommentAnnotation ca = createCommentAnnotation(document, comment, (ILineLocation) location);
 					if (annotations.add(ca)) {
 						event.annotationAdded(ca);
 					}
 				} catch (BadLocationException e) {
-					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Unable to add annotation.",
+					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Unable to add annotation.", //$NON-NLS-1$
 							e));
 				}
 			}
@@ -262,37 +244,17 @@ public class ReviewAnnotationModel implements IAnnotationModel {
 	}
 
 	private void removeCommentAnnotations(IDocument document, AnnotationModelEvent event, IComment comment) {
-		int startLine = 0;
-		int endLine = 0;
 		for (ILocation location : comment.getLocations()) {
 			if (location instanceof ILineLocation) {
-				ILineLocation lineLocation = (ILineLocation) location;
 				try {
-					startLine = lineLocation.getRangeMin();
-					endLine = lineLocation.getRangeMax();
-
-					int offset = 0;
-					int length = 0;
-					if (startLine != 0 && startLine <= document.getNumberOfLines()) {
-
-						offset = document.getLineOffset(startLine - 1);
-						if (endLine == 0) {
-							endLine = startLine;
-						}
-						length = Math.max(document.getLineOffset(endLine - 1) - offset, 0);
-
-					}
-					length = Math.max(1, length);
-					CommentAnnotation ca = new CommentAnnotation(offset, length, comment);
-
+					CommentAnnotation ca = createCommentAnnotation(document, comment, (ILineLocation) location);
 					annotations.remove(ca);
-					List<IComment> lst = reviewItem.getComments();
-					lst.remove(comment);
+					reviewItem.getComments().remove(comment);
 					event.annotationRemoved(ca);
 
 				} catch (BadLocationException e) {
 					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
-							"Unable to remove annotation.", e));
+							"Unable to remove annotation.", e)); //$NON-NLS-1$
 				}
 			}
 		}
@@ -300,42 +262,41 @@ public class ReviewAnnotationModel implements IAnnotationModel {
 
 	private void modifyCommentAnnotations(IDocument document, AnnotationModelEvent event, IComment oldcomment,
 			IComment comment) {
-		int startLine = 0;
-		int endLine = 0;
 		for (ILocation location : comment.getLocations()) {
 			if (location instanceof ILineLocation) {
-				ILineLocation lineLocation = (ILineLocation) location;
 				try {
-					startLine = lineLocation.getRangeMin();
-					endLine = lineLocation.getRangeMax();
-
-					int offset = 0;
-					int length = 0;
-					if (startLine != 0 && startLine <= document.getNumberOfLines()) {
-
-						offset = document.getLineOffset(startLine - 1);
-						if (endLine == 0) {
-							endLine = startLine;
-						}
-						length = Math.max(document.getLineOffset(endLine - 1) - offset, 0);
-
-					}
-					length = Math.max(1, length);
-					CommentAnnotation oldca = new CommentAnnotation(offset, length, oldcomment);
-					CommentAnnotation ca = new CommentAnnotation(offset, length, comment);
-					annotations.remove(oldca);
+					CommentAnnotation oldCa = createCommentAnnotation(document, oldcomment, (ILineLocation) location);
+					CommentAnnotation ca = new CommentAnnotation(oldCa.getPosition().offset,
+							oldCa.getPosition().length, comment);
+					annotations.remove(oldCa);
 					annotations.add(ca);
-					event.annotationRemoved(oldca);
+					event.annotationRemoved(oldCa);
 					event.annotationChanged(ca);
 
 				} catch (BadLocationException e) {
 					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
-							"Unable to modify annotation.", e));
+							"Unable to modify annotation.", e)); //$NON-NLS-1$
 				}
 			}
 		}
 	}
 
+	private CommentAnnotation createCommentAnnotation(IDocument document, IComment comment, ILineLocation lineLocation)
+			throws BadLocationException {
+		int startLine = lineLocation.getRangeMin();
+		int endLine = lineLocation.getRangeMax();
+		int offset = 0;
+		int length = 1;
+		if (startLine != 0 && startLine <= document.getNumberOfLines()) {
+			offset = document.getLineOffset(startLine - 1);
+			if (endLine == 0) {
+				endLine = startLine;
+			}
+			length = Math.max(document.getLineOffset(endLine - 1) - offset, 1);
+		}
+		return new CommentAnnotation(offset, length, comment);
+	}
+
 	protected void clear() {
 		AnnotationModelEvent event = new AnnotationModelEvent(this);
 		clear(event);
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/messages.properties b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/messages.properties
index 69742f4bb..f7eb48990 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/messages.properties
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/annotations/messages.properties
@@ -10,5 +10,5 @@
 ###############################################################################
 CommentAnnotation_X_dash_Y={0} - {1}
 CommentAnnotationHover_Multiple_comments=There are multiple comments on this line
-CommentInputDialog_LineNumber=Comment for line number {0}
+CommentInputDialog_LineNumber=Comments for line {0}
 
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/compare/ReviewCompareAnnotationSupport.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/compare/ReviewCompareAnnotationSupport.java
index 0fc248976..d25153b66 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/compare/ReviewCompareAnnotationSupport.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/compare/ReviewCompareAnnotationSupport.java
@@ -50,6 +50,7 @@ import org.eclipse.mylyn.reviews.core.model.IReviewItem;
 import org.eclipse.mylyn.reviews.ui.ReviewBehavior;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.StyledText;
+import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.ui.texteditor.AbstractTextEditor;
 
@@ -180,7 +181,7 @@ public class ReviewCompareAnnotationSupport {
 
 	public boolean hasAnnotation(Direction direction) {
 		Position rightPosition = new Position(0, 0);
-		return findAnnotation(rightSourceViewer, direction, rightPosition, rightAnnotationModel) == null ? false : true;
+		return findAnnotation(rightSourceViewer, direction, rightPosition, rightAnnotationModel) != null;
 	}
 
 	/**
@@ -194,11 +195,7 @@ public class ReviewCompareAnnotationSupport {
 		if (leftSourceViewer == null) {
 			return null;
 		}
-		ITextSelection selection = (ITextSelection) leftSourceViewer.getSourceViewer()
-				.getSelectionProvider()
-				.getSelection();
-		int currentLeftOffset = selection.getOffset();
-		selection = (ITextSelection) rightSourceViewer.getSourceViewer().getSelectionProvider().getSelection();
+		int currentLeftOffset = getSelection(leftSourceViewer).getOffset();
 
 		Position nextLeftPosition = new Position(0, 0);
 		Annotation leftAnnotation = findAnnotation(leftSourceViewer, direction, nextLeftPosition, leftAnnotationModel);
@@ -212,30 +209,11 @@ public class ReviewCompareAnnotationSupport {
 			selectAndReveal(leftSourceViewer, nextLeftPosition);
 			return leftAnnotation;
 		} else if (leftAnnotation != null && rightAnnotation != null) {
-			int line, offset = 0;
-			try {
-				line = leftAnnotationModel.getDocument().getLineOfOffset(nextLeftPosition.offset);
-				offset = leftAnnotationModel.getDocument().getLineOffset(line);
-			} catch (BadLocationException e) {
-				StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e)); //$NON-NLS-1$
-			}
-			nextLeftPosition.offset = offset;
+			nextLeftPosition.offset = getLineOffset(leftAnnotationModel, nextLeftPosition.offset);
 			nextLeftPosition.length = 1;
-			try {
-				line = rightAnnotationModel.getDocument().getLineOfOffset(nextRightPosition.offset);
-				offset = rightAnnotationModel.getDocument().getLineOffset(line);
-			} catch (BadLocationException e) {
-				StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e)); //$NON-NLS-1$
-			}
-			nextRightPosition.offset = offset;
+			nextRightPosition.offset = getLineOffset(rightAnnotationModel, nextRightPosition.offset);
 			nextRightPosition.length = 1;
-			try {
-				line = leftAnnotationModel.getDocument().getLineOfOffset(currentLeftOffset);
-				offset = leftAnnotationModel.getDocument().getLineOffset(line);
-			} catch (BadLocationException e) {
-				StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e)); //$NON-NLS-1$
-			}
-			currentLeftOffset = offset;
+			currentLeftOffset = getLineOffset(leftAnnotationModel, currentLeftOffset);
 
 			if (calculateNextAnnotation(direction, nextLeftPosition, nextRightPosition, currentLeftOffset) == LEFT_SIDE) {
 				return leftAnnotation;
@@ -246,6 +224,16 @@ public class ReviewCompareAnnotationSupport {
 		return null;
 	}
 
+	private int getLineOffset(ReviewAnnotationModel annotationModel, int offset) {
+		try {
+			int line = annotationModel.getDocument().getLineOfOffset(offset);
+			return annotationModel.getDocument().getLineOffset(line);
+		} catch (BadLocationException e) {
+			StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e)); //$NON-NLS-1$
+		}
+		return 0;
+	}
+
 	public Side calculateNextAnnotation(Direction direction, Position nextLeftPosition, Position nextRightPosition,
 			Integer currentLeftOffset) {
 		if (direction == Direction.FORWARDS) {
@@ -256,92 +244,73 @@ public class ReviewCompareAnnotationSupport {
 				rightSourceViewer.getSourceViewer().setSelectedRange(nextLeftPosition.offset - 1,
 						nextLeftPosition.length - 1);
 				return LEFT_SIDE;
-			} else if ((nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset < currentLeftOffset)) {
+			} else if ((nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset < currentLeftOffset)
+					|| (nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset > currentLeftOffset)) {
 				if ((nextLeftPosition.offset < nextRightPosition.offset)) {
-					moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-					return LEFT_SIDE;
+					return moveToLeftAnnotation(nextLeftPosition);
 				} else {
-					moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-					return RIGHT_SIDE;
-				}
-			} else if ((nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset > currentLeftOffset)) {
-				if ((nextLeftPosition.offset < nextRightPosition.offset)) {
-					moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-					return LEFT_SIDE;
-				} else {
-					moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-					return RIGHT_SIDE;
+					return moveToRightAnnotation(nextRightPosition);
 				}
 			} else if ((nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset > currentLeftOffset)) {
-				moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-				return RIGHT_SIDE;
-
+				return moveToRightAnnotation(nextRightPosition);
 			} else if ((nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset < currentLeftOffset)) {
-				moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-				return LEFT_SIDE;
+				return moveToLeftAnnotation(nextLeftPosition);
 			} else if (nextRightPosition.offset == currentLeftOffset) {
-				moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-				return LEFT_SIDE;
+				return moveToLeftAnnotation(nextLeftPosition);
 			} else {
-				moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-				return RIGHT_SIDE;
+				return moveToRightAnnotation(nextRightPosition);
 			}
 
 		} else { // backwards
 			if (nextLeftPosition.offset == nextRightPosition.offset) {
 				moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-				int line;
-				IRegion region;
-				int offset = 0;
-				int length = 0;
-				try {
-					line = rightAnnotationModel.getDocument().getLineOfOffset(nextRightPosition.offset);
-					region = rightAnnotationModel.getDocument().getLineInformation(line + 1);
-					offset = region.getOffset();
-					length = region.getLength();
-				} catch (BadLocationException e) {
-					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
-							"Error displaying comment", e)); //$NON-NLS-1$
-				}
-				nextLeftPosition.offset = offset;
-				nextLeftPosition.length = length;
-				leftSourceViewer.getSourceViewer().revealRange(nextLeftPosition.offset, nextLeftPosition.length);
-				leftSourceViewer.getSourceViewer().setSelectedRange(nextLeftPosition.offset, nextLeftPosition.length);
+				Position position = getNextLine(nextRightPosition.offset);
+				leftSourceViewer.getSourceViewer().revealRange(position.offset, position.length);
+				leftSourceViewer.getSourceViewer().setSelectedRange(position.offset, position.length);
 				return RIGHT_SIDE;
-			} else if ((nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset > currentLeftOffset)) {
-				if ((nextLeftPosition.offset > nextRightPosition.offset)) {
-					moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-					return LEFT_SIDE;
-				} else {
-					moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-					return RIGHT_SIDE;
-				}
-			} else if ((nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset < currentLeftOffset)) {
+			} else if ((nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset > currentLeftOffset)
+					|| (nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset < currentLeftOffset)) {
 				if ((nextLeftPosition.offset > nextRightPosition.offset)) {
-					moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-					return LEFT_SIDE;
+					return moveToLeftAnnotation(nextLeftPosition);
 				} else {
-					moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-					return RIGHT_SIDE;
+					return moveToRightAnnotation(nextRightPosition);
 				}
 			} else if ((nextLeftPosition.offset > currentLeftOffset && nextRightPosition.offset < currentLeftOffset)) {
-				moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-				return RIGHT_SIDE;
-
+				return moveToRightAnnotation(nextRightPosition);
 			} else if ((nextLeftPosition.offset < currentLeftOffset && nextRightPosition.offset > currentLeftOffset)) {
-				moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-				return LEFT_SIDE;
+				return moveToLeftAnnotation(nextLeftPosition);
 			} else if (nextRightPosition.offset == currentLeftOffset) {
-				moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
-				return LEFT_SIDE;
+				return moveToLeftAnnotation(nextLeftPosition);
 			} else {
-				moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
-				return RIGHT_SIDE;
+				return moveToRightAnnotation(nextRightPosition);
 			}
 
 		}
 	}
 
+	private Position getNextLine(int offset) {
+		Position position = new Position(0, 0);
+		try {
+			int line = rightAnnotationModel.getDocument().getLineOfOffset(offset);
+			IRegion region = rightAnnotationModel.getDocument().getLineInformation(line + 1);
+			position.offset = region.getOffset();
+			position.length = region.getLength();
+		} catch (BadLocationException e) {
+			StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e)); //$NON-NLS-1$
+		}
+		return position;
+	}
+
+	private Side moveToLeftAnnotation(Position nextLeftPosition) {
+		moveToAnnotation(rightSourceViewer, leftSourceViewer, nextLeftPosition);
+		return LEFT_SIDE;
+	}
+
+	private Side moveToRightAnnotation(Position nextRightPosition) {
+		moveToAnnotation(leftSourceViewer, rightSourceViewer, nextRightPosition);
+		return RIGHT_SIDE;
+	}
+
 	public void moveToAnnotation(MergeSourceViewer adjacentViewer, MergeSourceViewer annotationViewer, Position position) {
 		adjacentViewer.getSourceViewer().revealRange(position.offset, position.length);
 		adjacentViewer.getSourceViewer().setSelectedRange(position.offset, position.length);
@@ -352,98 +321,87 @@ public class ReviewCompareAnnotationSupport {
 	private void selectAndReveal(MergeSourceViewer sourceViewer, Position position) {
 		StyledText widget = sourceViewer.getSourceViewer().getTextWidget();
 		widget.setRedraw(false);
-		{
-			int currentLineNumber = 0;
-			try {
-				currentLineNumber = sourceViewer.getSourceViewer().getDocument().getLineOfOffset(position.getOffset());
-			} catch (BadLocationException e1) {
-				StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e1)); //$NON-NLS-1$
-			}
-			adjustHighlightRange(sourceViewer.getSourceViewer(), position.offset, position.length);
-			sourceViewer.getSourceViewer().revealRange(position.offset, position.length);
-			sourceViewer.getSourceViewer().setSelectedRange(position.offset, position.length);
-			SourceViewer srcViewer = sourceViewer.getSourceViewer();
-
-			IReviewItem reviewitem = ((ReviewAnnotationModel) srcViewer.getAnnotationModel()).getItem();
-
-			List<CommentAnnotation> comments = getAnnotationsForLine(srcViewer, position.offset);
-
-			org.eclipse.swt.graphics.Point p = sourceViewer.getLineRange(position, sourceViewer.getSourceViewer()
-					.getSelectedRange());
-			LineRange range = new LineRange(p.x + 1, p.y);
-
-			if (commentPopupDialog != null) {
-				if (watchDog != null) {
-					try {
-						synchronized (myMonitorObject) {
-							sentinel = true;
-							commentPopupDialog.dispose();
-							myMonitorObject.wait();
-						}
-					} catch (InterruptedException e) {
-						StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
-								"Error destroying dialog", e)); //$NON-NLS-1$
-					}
-				}
-			}
-
-			commentPopupDialog = new CommentPopupDialog(ReviewsUiPlugin.getDefault()
-					.getWorkbench()
-					.getActiveWorkbenchWindow()
-					.getShell(), SWT.NO_FOCUS | SWT.ON_TOP, false, reviewitem, range);
-
-			CommentAnnotationHoverInput input = new CommentAnnotationHoverInput(comments,
-					((ReviewAnnotationModel) srcViewer.getAnnotationModel()).getBehavior());
-
-			commentPopupDialog.create();
-			commentPopupDialog.setInput(input);
-
-			commentPopupDialog.setLineNumber(currentLineNumber);
-
-			commentPopupDialog.setSize(50, 150);
+		int currentLineNumber = 0;
+		try {
+			currentLineNumber = sourceViewer.getSourceViewer().getDocument().getLineOfOffset(position.getOffset());
+		} catch (BadLocationException e1) {
+			StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error displaying comment", e1)); //$NON-NLS-1$
+		}
+		adjustHighlightRange(sourceViewer.getSourceViewer(), position.offset, position.length);
+		sourceViewer.getSourceViewer().revealRange(position.offset, position.length);
+		sourceViewer.getSourceViewer().setSelectedRange(position.offset, position.length);
+		SourceViewer srcViewer = sourceViewer.getSourceViewer();
 
-			org.eclipse.swt.graphics.Point location = sourceViewer.getSourceViewer().getControl().getLocation();
-			location = Display.getCurrent().getCursorLocation();
+		IReviewItem reviewitem = ((ReviewAnnotationModel) srcViewer.getAnnotationModel()).getItem();
 
-			location.y = location.y + (sourceViewer.getViewportHeight() / 2);
+		List<CommentAnnotation> comments = getAnnotationsForLine(srcViewer, position.offset);
 
-			commentPopupDialog.setLocation(location);
+		Point p = sourceViewer.getLineRange(position, sourceViewer.getSourceViewer().getSelectedRange());
+		LineRange range = new LineRange(p.x + 1, p.y);
 
-			commentPopupDialog.open();
+		if (commentPopupDialog != null) {
+			if (watchDog != null) {
+				try {
+					synchronized (myMonitorObject) {
+						sentinel = true;
+						commentPopupDialog.dispose();
+						myMonitorObject.wait();
+					}
+				} catch (InterruptedException e) {
+					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID, "Error destroying dialog", e)); //$NON-NLS-1$
+				}
+			}
+		}
 
-			watchDog = new Thread(new Runnable() {
-				public void run() {
-					try {
-						for (int x = 0; x < INTERRUPT_INTERVAL; x++) {
-							if (sentinel) {
-								Thread.currentThread().interrupt();
-								break;
-							}
-							Thread.sleep(PAUSE_DELAY);
+		commentPopupDialog = new CommentPopupDialog(ReviewsUiPlugin.getDefault()
+				.getWorkbench()
+				.getActiveWorkbenchWindow()
+				.getShell(), SWT.NO_FOCUS | SWT.ON_TOP, false, reviewitem, range);
+		CommentAnnotationHoverInput input = new CommentAnnotationHoverInput(comments,
+				((ReviewAnnotationModel) srcViewer.getAnnotationModel()).getBehavior());
+		commentPopupDialog.create();
+		commentPopupDialog.setInput(input);
+		commentPopupDialog.setLineNumber(currentLineNumber);
+		commentPopupDialog.setSize(50, 150);
+
+		Point location = sourceViewer.getSourceViewer().getControl().getLocation();
+		location = Display.getCurrent().getCursorLocation();
+		location.y = location.y + (sourceViewer.getViewportHeight() / 2);
+		commentPopupDialog.setLocation(location);
+		commentPopupDialog.open();
+
+		watchDog = new Thread(new Runnable() {
+			public void run() {
+				try {
+					for (int x = 0; x < INTERRUPT_INTERVAL; x++) {
+						if (sentinel) {
+							Thread.currentThread().interrupt();
+							break;
 						}
-					} catch (Exception e) {
-						StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
-								"Error interrupting thread", e)); //$NON-NLS-1$
+						Thread.sleep(PAUSE_DELAY);
 					}
-					if (!sentinel) {
-						Display.getDefault().asyncExec(new Runnable() {
-							public void run() {
-								if (commentPopupDialog != null) {
-									commentPopupDialog.dispose();
-									commentPopupDialog = null;
-								}
+				} catch (Exception e) {
+					StatusHandler.log(new Status(IStatus.ERROR, ReviewsUiPlugin.PLUGIN_ID,
+							"Error interrupting thread", e)); //$NON-NLS-1$
+				}
+				if (!sentinel) {
+					Display.getDefault().asyncExec(new Runnable() {
+						public void run() {
+							if (commentPopupDialog != null) {
+								commentPopupDialog.dispose();
+								commentPopupDialog = null;
 							}
-						});
-					}
-					synchronized (myMonitorObject) {
-						myMonitorObject.notify();
-					}
+						}
+					});
 				}
-			});
-			sentinel = false;
+				synchronized (myMonitorObject) {
+					myMonitorObject.notify();
+				}
+			}
+		});
+		sentinel = false;
 
-			watchDog.start();
-		}
+		watchDog.start();
 		widget.setRedraw(true);
 	}
 
@@ -467,29 +425,20 @@ public class ReviewCompareAnnotationSupport {
 		for (Iterator<Annotation> it = model.getAnnotationIterator(); it.hasNext();) {
 			Annotation annotation = it.next();
 			Position position = model.getPosition(annotation);
-			if (position == null) {
-				continue;
-			}
-
-			if (!isRulerLine(position, document, line)) {
+			if (position == null || !isPositionOnLine(position, line, document)) {
 				continue;
 			}
-
 			if (annotation instanceof AnnotationBag) {
 				AnnotationBag bag = (AnnotationBag) annotation;
 				Iterator<Annotation> e = bag.iterator();
 				while (e.hasNext()) {
 					annotation = e.next();
 					position = model.getPosition(annotation);
-					if (position != null && includeAnnotation(annotation, position, commentAnnotations)
-							&& annotation instanceof CommentAnnotation) {
+					if (position != null && includeAnnotation(annotation, position, commentAnnotations)) {
 						commentAnnotations.add((CommentAnnotation) annotation);
 					}
 				}
-				continue;
-			}
-
-			if (includeAnnotation(annotation, position, commentAnnotations) && annotation instanceof CommentAnnotation) {
+			} else if (includeAnnotation(annotation, position, commentAnnotations)) {
 				commentAnnotations.add((CommentAnnotation) annotation);
 			}
 		}
@@ -498,14 +447,10 @@ public class ReviewCompareAnnotationSupport {
 	}
 
 	private boolean includeAnnotation(Annotation annotation, Position position, List<CommentAnnotation> annotations) {
-		if (!(annotation instanceof CommentAnnotation)) {
-			return false;
-		}
-
-		return (annotation != null && !annotations.contains(annotation));
+		return annotation instanceof CommentAnnotation && !annotations.contains(annotation);
 	}
 
-	private boolean isRulerLine(Position position, IDocument document, int line) {
+	private boolean isPositionOnLine(Position position, int line, IDocument document) {
 		if (position.getOffset() > -1 && position.getLength() > -1) {
 			try {
 				return line == document.getLineOfOffset(position.getOffset());
@@ -589,7 +534,7 @@ public class ReviewCompareAnnotationSupport {
 		if (viewer == null) {
 			return null;
 		}
-		ITextSelection selection = (ITextSelection) viewer.getSourceViewer().getSelectionProvider().getSelection();
+		ITextSelection selection = getSelection(viewer);
 		final int offset = selection.getOffset();
 		final int length = selection.getLength();
 
@@ -668,4 +613,8 @@ public class ReviewCompareAnnotationSupport {
 		return nextAnnotation;
 	}
 
+	private ITextSelection getSelection(MergeSourceViewer viewer) {
+		return (ITextSelection) viewer.getSourceViewer().getSelectionProvider().getSelection();
+	}
+
 }
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/CommentInputDialog.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/CommentInputDialog.java
index 50622d209..3109ad1f8 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/CommentInputDialog.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/CommentInputDialog.java
@@ -16,7 +16,6 @@ package org.eclipse.mylyn.internal.reviews.ui.dialogs;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.commons.lang.StringUtils;
@@ -96,7 +95,7 @@ public class CommentInputDialog extends FormDialog {
 
 	private final LineRange range;
 
-	private Map<Integer, Comment> commentList;
+	private List<Comment> commentList;
 
 	private boolean isUpdate = false;
 
@@ -335,28 +334,24 @@ public class CommentInputDialog extends FormDialog {
 		composite.setLayout(layout);
 		GridData textGridData = null;
 
-		for (int i = 0; i < commentList.size(); i++) {
+		for (Comment comment : commentList) {
 			final Button button = new Button(composite, SWT.RADIO);
-
 			button.setBackground(composite.getBackground());
 
-			@SuppressWarnings("restriction")
-			Comment currentComment = commentList.get(new Integer(i + 1));
-
-			final String uuid = currentComment.getId();
-			final boolean isDraft = currentComment.isDraft();
-			final String commentText = currentComment.getDescription();
+			final String uuid = comment.getId();
+			final boolean isDraft = comment.isDraft();
+			final String commentText = comment.getDescription();
 			final String authorAndDate;
-			IUser author = currentComment.getAuthor();
+			IUser author = comment.getAuthor();
 			if (author != null) {
-				authorAndDate = currentComment.getAuthor().getDisplayName() + " " //$NON-NLS-1$
-						+ currentComment.getCreationDate().toString();
+				authorAndDate = comment.getAuthor().getDisplayName() + " " //$NON-NLS-1$
+						+ comment.getCreationDate().toString();
 			} else {
 				authorAndDate = Messages.CommentInputDialog_No_author + " " //$NON-NLS-1$
-						+ currentComment.getCreationDate().toString();
+						+ comment.getCreationDate().toString();
 			}
 
-			String commentPrefix = StringUtils.abbreviate(currentComment.getDescription(), 50);
+			String commentPrefix = StringUtils.abbreviate(comment.getDescription(), 50);
 			if (isDraft) {
 				button.setText(NLS.bind(Messages.CommentInputDialog_Draft, authorAndDate, commentPrefix));
 			} else {
@@ -445,7 +440,7 @@ public class CommentInputDialog extends FormDialog {
 		return true;
 	}
 
-	public void setComments(Map<Integer, Comment> commentList) {
+	public void setComments(List<Comment> commentList) {
 		this.commentList = commentList;
 	}