Skip to content

Commit

Permalink
Improve performance once again, and a few refactorings as well.
Browse files Browse the repository at this point in the history
  • Loading branch information
@simonharrer
simonharrer committed Apr 5, 2016
1 parent 237aa1b commit 480dae7
Show file tree
Hide file tree
Showing 7 changed files with 299 additions and 277 deletions.
158 changes: 64 additions & 94 deletions src/main/java/net/sf/jabref/bibtex/comparator/FieldComparator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,43 +44,63 @@
*/
public class FieldComparator implements Comparator<BibEntry> {

private static Collator collator;
private static final Collator COLLATOR = getCollator();

static {
private static Collator getCollator() {
try {
FieldComparator.collator = new RuleBasedCollator(
return new RuleBasedCollator(
((RuleBasedCollator) Collator.getInstance()).getRules().replace("<'\u005f'", "<' '<'\u005f'"));
} catch (ParseException e) {
FieldComparator.collator = Collator.getInstance();
return Collator.getInstance();
}
}



enum FieldType {
NAME, TYPE, YEAR, MONTH, OTHER;
}

private final String[] field;
private final String fieldName;

private final boolean isNameField;
private final boolean isTypeHeader;
private final boolean isYearField;
private final boolean isMonthField;
private final FieldType fieldType;
private final boolean isNumeric;

private final int multiplier;


public FieldComparator(String field) {
this(field, false);
}

public FieldComparator(String field, boolean reversed) {
this.fieldName = Objects.requireNonNull(field);
this.field = field.split(MainTableFormat.COL_DEFINITION_FIELD_SEPARATOR);
multiplier = reversed ? -1 : 1;
isTypeHeader = this.field[0].equals(BibEntry.TYPE_HEADER);
isNameField = "author".equals(this.field[0])
|| "editor".equals(this.field[0]);
isYearField = "year".equals(this.field[0]);
isMonthField = "month".equals(this.field[0]);
this.field = fieldName.split(MainTableFormat.COL_DEFINITION_FIELD_SEPARATOR);
fieldType = determineFieldType(this.field[0]);
isNumeric = InternalBibtexFields.isNumeric(this.field[0]);

if(fieldType == FieldType.MONTH) {
/*
* [ 1598777 ] Month sorting
*
* http://sourceforge.net/tracker/index.php?func=detail&aid=1598777&group_id=92314&atid=600306
*/
multiplier = reversed ? 1 : -1;
} else {
multiplier = reversed ? -1 : 1;
}
}

private FieldType determineFieldType(String s) {
if(BibEntry.TYPE_HEADER.equals(this.field[0])) {
return FieldType.TYPE;
} else if("author".equals(this.field[0]) || "editor".equals(this.field[0])) {
return FieldType.NAME;
} else if ("year".equals(this.field[0])) {
return FieldType.YEAR;
} else if("month".equals(this.field[0])) {
return FieldType.MONTH;
} else {
return FieldType.OTHER;
}
}

public FieldComparator(SaveOrderConfig.SortCriterion sortCriterion) {
Expand All @@ -89,116 +109,66 @@ public FieldComparator(SaveOrderConfig.SortCriterion sortCriterion) {

@Override
public int compare(BibEntry e1, BibEntry e2) {
Object f1;
Object f2;
String f1;
String f2;

if (isTypeHeader) {
if (fieldType == FieldType.TYPE) {
// Sort by type.
f1 = e1.getType();
f2 = e2.getType();
} else {

// If the field is author or editor, we rearrange names so they are
// sorted according to last name.
f1 = getField(e1);
f2 = getField(e2);
}

/*
* [ 1598777 ] Month sorting
*
* http://sourceforge.net/tracker/index.php?func=detail&aid=1598777&group_id=92314&atid=600306
*/
int localMultiplier = multiplier;
if (isMonthField) {
localMultiplier = -localMultiplier;
}

// Catch all cases involving null:
if (f1 == null) {
return f2 == null ? 0 : localMultiplier;
}

if (f2 == null) {
return -localMultiplier;
if (f1 == null && f2 == null) {
return 0;
} else if(f1 == null) {
return multiplier;
} else if (f2 == null) {
return -multiplier;
}

// Now we now that both f1 and f2 are != null
if (isNameField) {
f1 = AuthorList.fixAuthorForAlphabetization((String) f1);
f2 = AuthorList.fixAuthorForAlphabetization((String) f2);
} else if (isYearField) {
/*
* [ 1285977 ] Impossible to properly sort a numeric field
*
* http://sourceforge.net/tracker/index.php?func=detail&aid=1285977&group_id=92314&atid=600307
*/
f1 = YearUtil.toFourDigitYear((String) f1);
f2 = YearUtil.toFourDigitYear((String) f2);
} else if (isMonthField) {
/*
* [ 1535044 ] Month sorting
*
* http://sourceforge.net/tracker/index.php?func=detail&aid=1535044&group_id=92314&atid=600306
*/
f1 = MonthUtil.getMonth((String) f1).number;
f2 = MonthUtil.getMonth((String) f2).number;
if (fieldType == FieldType.NAME) {
f1 = AuthorList.fixAuthorForAlphabetization(f1);
f2 = AuthorList.fixAuthorForAlphabetization(f2);
} else if (fieldType == FieldType.YEAR) {
return Integer.compare(YearUtil.toFourDigitYearWithInts(f1), YearUtil.toFourDigitYearWithInts(f2)) * multiplier;
} else if (fieldType == FieldType.MONTH) {
return Integer.compare(MonthUtil.getMonth(f1).number, MonthUtil.getMonth(f2).number) * multiplier;
}

if (isNumeric) {
Integer i1 = null;
Integer i2 = null;
try {
i1 = StringUtil.intValueOf((String) f1);
} catch (NumberFormatException ex) {
// Parsing failed.
}

try {
i2 = StringUtil.intValueOf((String) f2);
} catch (NumberFormatException ex) {
// Parsing failed.
}
Integer i1 = StringUtil.intValueOfWithNull(f1);
Integer i2 = StringUtil.intValueOfWithNull(f2);

if ((i2 != null) && (i1 != null)) {
// Ok, parsing was successful. Update f1 and f2:
f1 = i1;
f2 = i2;
return i1.compareTo(i2) * multiplier;
} else if (i1 != null) {
// The first one was parseable, but not the second one.
// This means we consider one < two
f1 = i1;
f2 = i1 + 1;
return -1 * multiplier;
} else if (i2 != null) {
// The second one was parseable, but not the first one.
// This means we consider one > two
f2 = i2;
f1 = i2 + 1;
return 1 * multiplier;
}
// Else none of them were parseable, and we can fall back on comparing strings.
}

int result;
if ((f1 instanceof Integer) && (f2 instanceof Integer)) {
result = ((Integer) f1).compareTo((Integer) f2);
} else if (f2 instanceof Integer) {
Integer f1AsInteger = Integer.valueOf(f1.toString());
result = -f1AsInteger.compareTo((Integer) f2);
} else if (f1 instanceof Integer) {
Integer f2AsInteger = Integer.valueOf(f2.toString());
result = -((Integer) f1).compareTo(f2AsInteger);
} else {
String ours = ((String) f1).toLowerCase();
String theirs = ((String) f2).toLowerCase();
result = FieldComparator.collator.compare(ours, theirs);
}

return result * localMultiplier;
String ours = ((String) f1).toLowerCase();
String theirs = ((String) f2).toLowerCase();
return COLLATOR.compare(ours, theirs) * multiplier;
}

private Object getField(BibEntry entry) {
private String getField(BibEntry entry) {
for (String aField : field) {
Object o = entry.getFieldOrAlias(aField);
String o = entry.getFieldOrAlias(aField);
if (o != null) {
return o;
}
Expand Down
31 changes: 6 additions & 25 deletions .../net/sf/jabref/gui/GlazedEntrySorter.java → ...abref/gui/maintable/ListSynchronizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,19 @@
with this program; if not, write to the Free Software Foundation, Inc.,
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
package net.sf.jabref.gui;
package net.sf.jabref.gui.maintable;

import java.util.Collections;
import java.util.List;

import net.sf.jabref.model.entry.BibEntry;
import ca.odell.glazedlists.EventList;
import net.sf.jabref.model.database.DatabaseChangeEvent;
import net.sf.jabref.model.database.DatabaseChangeListener;
import net.sf.jabref.logic.id.IdComparator;
import ca.odell.glazedlists.BasicEventList;
import ca.odell.glazedlists.EventList;
import net.sf.jabref.model.entry.BibEntry;

public class GlazedEntrySorter implements DatabaseChangeListener {
public class ListSynchronizer implements DatabaseChangeListener {

private final EventList<BibEntry> list;

public GlazedEntrySorter(List<BibEntry> entries) {
list = new BasicEventList<>();
try {
list.getReadWriteLock().writeLock().lock();
list.addAll(entries);

// Sort the list so it is ordered according to creation (or read) order
// when the table is unsorted.
Collections.sort(list, new IdComparator());
} finally {
list.getReadWriteLock().writeLock().unlock();
}
}

public EventList<BibEntry> getTheList() {
return list;
public ListSynchronizer(EventList<BibEntry> list) {
this.list = list;
}

@Override
Expand Down
22 changes: 9 additions & 13 deletions src/main/java/net/sf/jabref/gui/maintable/MainTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
import javax.swing.table.TableCellRenderer;
import javax.swing.table.TableColumnModel;

import net.sf.jabref.groups.GroupMatcher;
import net.sf.jabref.gui.*;
import net.sf.jabref.gui.renderer.CompleteRenderer;
import net.sf.jabref.gui.renderer.GeneralRenderer;
import net.sf.jabref.gui.renderer.IncompleteRenderer;
import net.sf.jabref.gui.search.matchers.SearchMatcher;
import net.sf.jabref.gui.util.comparator.FirstColumnComparator;
import net.sf.jabref.gui.util.comparator.IconComparator;
import net.sf.jabref.gui.util.comparator.RankingFieldComparator;
Expand Down Expand Up @@ -150,7 +152,7 @@ public MainTable(MainTableFormat tableFormat, MainTableDataModel model, JabRefFr

this.setTableHeader(new PreventDraggingJTableHeader(this, tableFormat));

comparatorChooser = this.createTableComparatorChooser(this, model.getSortedForTable(),
comparatorChooser = this.createTableComparatorChooser(this, model.getSortedForUserDefinedTableColumnSorting(),
TableComparatorChooser.MULTIPLE_COLUMN_KEYBOARD);

this.tableColumnListener = new PersistenceTableColumnListener(this);
Expand All @@ -168,7 +170,7 @@ public MainTable(MainTableFormat tableFormat, MainTableDataModel model, JabRefFr
pane.setTransferHandler(xfer);

setupComparatorChooser();
model.refreshSorting();
model.updateMarkingState(Globals.prefs.getBoolean(JabRefPreferences.FLOAT_MARKED_ENTRIES));
setWidths();
}

Expand Down Expand Up @@ -208,10 +210,10 @@ public TableCellRenderer getCellRenderer(int row, int column) {

CellRendererMode status = getCellStatus(row, column);

if (!(model.getSearchState() == MainTableDataModel.DisplayOption.FLOAT) || matches(row, model.getSearchMatcher())) {
if (!(model.getSearchState() == MainTableDataModel.DisplayOption.FLOAT) || matches(row, model.getSearchState() != MainTableDataModel.DisplayOption.DISABLED ? SearchMatcher.INSTANCE : null)) {
score++;
}
if (!(model.getGroupingState() == MainTableDataModel.DisplayOption.FLOAT) || matches(row, model.getGroupMatcher())) {
if (!(model.getGroupingState() == MainTableDataModel.DisplayOption.FLOAT) || matches(row, model.getGroupingState() != MainTableDataModel.DisplayOption.DISABLED ? GroupMatcher.INSTANCE : null)) {

This comment has been minimized.

Sign in to view

Copy link
@oscargus

oscargus Jul 26, 2016

Contributor

@simonharrer Passing null here seems to be a bad idea since the matcher is directly dereferences in matches. Or am I (and Find bugs) missing something?

This comment has been minimized.

Sign in to view

Copy link
@oscargus

oscargus Jul 26, 2016

Contributor

Same three line above.
score += 2;
}

Expand Down Expand Up @@ -383,7 +385,7 @@ private void setupComparatorChooser() {
Globals.prefs.getBoolean(JabRefPreferences.TABLE_TERTIARY_SORT_DESCENDING)
}; // descending

model.getSortedForTable().getReadWriteLock().writeLock().lock();
model.getSortedForUserDefinedTableColumnSorting().getReadWriteLock().writeLock().lock();
try {
for (int i = 0; i < sortFields.length; i++) {
int index = -1;
Expand All @@ -405,7 +407,7 @@ private void setupComparatorChooser() {
}
}
} finally {
model.getSortedForTable().getReadWriteLock().writeLock().unlock();
model.getSortedForUserDefinedTableColumnSorting().getReadWriteLock().writeLock().unlock();
}

// Add action listener so we can remember the sort order:
Expand Down Expand Up @@ -634,13 +636,7 @@ private static Color mixColors(Color one, Color two) {

private TableComparatorChooser<BibEntry> createTableComparatorChooser(JTable table, SortedList<BibEntry> list,
Object sortingStrategy) {
final TableComparatorChooser<BibEntry> result = TableComparatorChooser.install(table, list, sortingStrategy);

// We need to reset the stack of sorted list each time sorting order
// changes, or the sorting breaks down:
result.addSortActionListener(e -> model.refreshSorting());

return result;
return TableComparatorChooser.install(table, list, sortingStrategy);
}

/**
Expand Down
Loading

0 comments on commit 480dae7

Please sign in to comment.