Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some spotbugs issues #3060

Merged
merged 3 commits into from
Aug 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ private Globals() {
}

// Key binding preferences
public static KeyBindingRepository getKeyPrefs() {
public static synchronized KeyBindingRepository getKeyPrefs() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why synchronized here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect lazy initialization
https://stackoverflow.com/a/6782690/3450689

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refs item 3 of effective java, doesn't it?

if (keyBindingRepository == null) {
keyBindingRepository = prefs.getKeyBindingRepository();
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/jabref/gui/BasePanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -2144,7 +2144,9 @@ public void action() {
if ((focused != null) && (focused instanceof FieldEditor) && focused.hasFocus()) {
// User is currently editing a field:
// Check if it is the preamble:
if ((preambleEditor != null) && (focused == preambleEditor.getFieldEditor())) {

FieldEditor fieldEditor = (FieldEditor) focused;
if ((preambleEditor != null) && (fieldEditor.equals(preambleEditor.getFieldEditor()))) {
preambleEditor.storeCurrentEdit();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ public void openFiles(List<Path> filesToOpen, boolean raisePanel) {
Path file = iterator.next();
for (int i = 0; i < frame.getTabbedPane().getTabCount(); i++) {
BasePanel basePanel = frame.getBasePanelAt(i);
if ((basePanel.getBibDatabaseContext().getDatabaseFile().isPresent())
&& basePanel.getBibDatabaseContext().getDatabaseFile().get().equals(file)) {
if ((basePanel.getBibDatabaseContext().getDatabasePath().isPresent())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path equals file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the bug I fixed

&& basePanel.getBibDatabaseContext().getDatabasePath().get().equals(file)) {
iterator.remove();
removed++;
// See if we removed the final one. If so, we must perhaps
Expand Down
16 changes: 11 additions & 5 deletions src/main/java/org/jabref/logic/citationstyle/CitationStyle.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,22 @@ public String toString() {
}

@Override
public boolean equals(Object other) {
if (this == other) {
public boolean equals(Object o) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other is a pretty good name for the parameter imho

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed it to be consistent with the other implementations we have.
And there other is used as the name of the casted instance, see the line below

if (this == o) {
return true;
}
if (other == null || getClass() != other.getClass()) {
if ((o == null) || (getClass() != o.getClass())) {
return false;
}

CitationStyle that = (CitationStyle) other;
return source != null ? source.equals(that.source) : that.source == null;
CitationStyle other = (CitationStyle) o;
return Objects.equals(source, other.source);
}

@Override
public int hashCode()
{
return Objects.hash(source);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ private String constructQuery(String queryWithTitle) {
URI uri = null;
try {
uri = builder.build();
return uri.toString();
} catch (URISyntaxException e) {
LOGGER.error(e.getMessage(), e);
}
System.out.println("Query: " + uri.toString());
return uri.toString();
return "";
}
}
18 changes: 9 additions & 9 deletions src/main/java/org/jabref/model/groups/WordKeywordGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ public class WordKeywordGroup extends KeywordGroup implements GroupEntryChanger
private final boolean onlySplitWordsAtSeparator;

public WordKeywordGroup(String name, GroupHierarchyType context, String searchField,
String searchExpression, boolean caseSensitive, Character keywordSeparator,
boolean onlySplitWordsAtSeparator) {
String searchExpression, boolean caseSensitive, Character keywordSeparator,
boolean onlySplitWordsAtSeparator) {
super(name, context, searchField, searchExpression, caseSensitive);

this.keywordSeparator = keywordSeparator;
Expand Down Expand Up @@ -92,13 +92,13 @@ public boolean equals(Object o) {
return false;
}
WordKeywordGroup other = (WordKeywordGroup) o;
return getName().equals(other.getName())
&& (getHierarchicalContext() == other.getHierarchicalContext())
&& searchField.equals(other.searchField)
&& searchExpression.equals(other.searchExpression)
&& (caseSensitive == other.caseSensitive)
&& keywordSeparator == other.keywordSeparator
&& onlySplitWordsAtSeparator == other.onlySplitWordsAtSeparator;
return Objects.equals(getName(), other.getName())
&& Objects.equals(getHierarchicalContext(), other.getHierarchicalContext())
&& Objects.equals(searchField, other.searchField)
&& Objects.equals(searchExpression, other.searchExpression)
&& Objects.equals(caseSensitive, other.caseSensitive)
&& Objects.equals(keywordSeparator, other.keywordSeparator)
&& Objects.equals(onlySplitWordsAtSeparator, other.onlySplitWordsAtSeparator);
}

@Override
Expand Down