Skip to content

Commit d7fe212

Browse files
paudelritijpaudelritij
committed
Apply boy scout principle
- Refactor ImportEntriesViewModel and ImportEntriesDialog to use Optional for fetcher and query.
1 parent 51b5f6d commit d7fe212

File tree

2 files changed

+77
-71
lines changed

2 files changed

+77
-71
lines changed

‎jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java‎

Lines changed: 41 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ public class ImportEntriesDialog extends BaseDialog<Boolean> {
7777
private final BackgroundTask<ParserResult> task;
7878
private final BibDatabaseContext database;
7979
private ImportEntriesViewModel viewModel;
80-
private final SearchBasedFetcher searchBasedFetcher;
81-
private final String query;
80+
private final Optional<SearchBasedFetcher> searchBasedFetcher;
81+
private final Optional<String> query;
8282

8383
@Inject private TaskExecutor taskExecutor;
8484
@Inject private DialogService dialogService;
@@ -97,7 +97,12 @@ public class ImportEntriesDialog extends BaseDialog<Boolean> {
9797
* @param task the task executed for parsing the selected files(s).
9898
*/
9999
public ImportEntriesDialog(BibDatabaseContext database, BackgroundTask<ParserResult> task) {
100-
this(database, task, null, null);
100+
this.database = database;
101+
this.task = task;
102+
this.searchBasedFetcher = Optional.empty();
103+
this.query = Optional.empty();
104+
105+
initializeDialog();
101106
}
102107

103108
/**
@@ -112,32 +117,10 @@ public ImportEntriesDialog(BibDatabaseContext database, BackgroundTask<ParserRes
112117
public ImportEntriesDialog(BibDatabaseContext database, BackgroundTask<ParserResult> task, SearchBasedFetcher fetcher, String query) {
113118
this.database = database;
114119
this.task = task;
115-
this.searchBasedFetcher = fetcher;
116-
this.query = query;
117-
118-
ViewLoader.view(this)
119-
.load()
120-
.setAsDialogPane(this);
121-
122-
boolean showPagination = (searchBasedFetcher != null) && (query != null);
123-
paginationBox.setVisible(showPagination);
124-
paginationBox.setManaged(showPagination);
125-
126-
BooleanBinding booleanBind = Bindings.isEmpty(entriesListView.getCheckModel().getCheckedItems());
127-
Button btn = (Button) this.getDialogPane().lookupButton(importButton);
128-
btn.disableProperty().bind(booleanBind);
120+
this.searchBasedFetcher = Optional.of(fetcher);
121+
this.query = Optional.of(query);
129122

130-
downloadLinkedOnlineFiles.setSelected(preferences.getFilePreferences().shouldDownloadLinkedFiles());
131-
132-
setResultConverter(button -> {
133-
if (button == importButton) {
134-
viewModel.importEntries(viewModel.getCheckedEntries().stream().toList(), downloadLinkedOnlineFiles.isSelected());
135-
} else {
136-
dialogService.notify(Localization.lang("Import canceled"));
137-
}
138-
139-
return false;
140-
});
123+
initializeDialog();
141124
}
142125

143126
@FXML
@@ -231,12 +214,37 @@ private void initialize() {
231214
totalItems.textProperty().bind(Bindings.size(viewModel.getAllEntries()).asString());
232215
entriesListView.setSelectionModel(new NoSelectionModel<>());
233216
initBibTeX();
234-
if (searchBasedFetcher != null) {
217+
if (searchBasedFetcher.isPresent()) {
235218
updatePageUI();
236219
setupPaginationBindings();
237220
}
238221
}
239222

223+
private void initializeDialog() {
224+
ViewLoader.view(this)
225+
.load()
226+
.setAsDialogPane(this);
227+
228+
paginationBox.setVisible(searchBasedFetcher.isPresent());
229+
paginationBox.setManaged(searchBasedFetcher.isPresent());
230+
231+
BooleanBinding booleanBind = Bindings.isEmpty(entriesListView.getCheckModel().getCheckedItems());
232+
Button btn = (Button) this.getDialogPane().lookupButton(importButton);
233+
btn.disableProperty().bind(booleanBind);
234+
235+
downloadLinkedOnlineFiles.setSelected(preferences.getFilePreferences().shouldDownloadLinkedFiles());
236+
237+
setResultConverter(button -> {
238+
if (button == importButton) {
239+
viewModel.importEntries(viewModel.getCheckedEntries().stream().toList(), downloadLinkedOnlineFiles.isSelected());
240+
} else {
241+
dialogService.notify(Localization.lang("Import canceled"));
242+
}
243+
244+
return false;
245+
});
246+
}
247+
240248
private void setupPaginationBindings() {
241249
BooleanProperty loading = viewModel.loadingProperty();
242250
BooleanProperty initialLoadComplete = viewModel.initialLoadCompleteProperty();
@@ -248,7 +256,7 @@ private void setupPaginationBindings() {
248256
}, viewModel.currentPageProperty(), viewModel.totalPagesProperty());
249257

250258
BooleanBinding isPagedFetcher = Bindings.createBooleanBinding(() ->
251-
searchBasedFetcher instanceof PagedSearchBasedFetcher
259+
searchBasedFetcher.isPresent() && searchBasedFetcher.get() instanceof PagedSearchBasedFetcher
252260
);
253261

254262
// Disable: during loading OR when on the last page for non-paged fetchers
@@ -306,7 +314,7 @@ private void setupPaginationBindings() {
306314
)
307315
);
308316

309-
loading.addListener((obs, oldVal, newVal) -> {
317+
loading.addListener((_, _, newVal) -> {
310318
getDialogPane().getScene().setCursor(newVal ? Cursor.WAIT : Cursor.DEFAULT);
311319
});
312320

@@ -382,7 +390,7 @@ public void selectAllEntries() {
382390
}
383391

384392
private boolean isOnLastPageAndPagedFetcher() {
385-
if (!(searchBasedFetcher instanceof PagedSearchBasedFetcher)) {
393+
if (searchBasedFetcher.isEmpty() || !(searchBasedFetcher.get() instanceof PagedSearchBasedFetcher)) {
386394
return false;
387395
}
388396

@@ -400,7 +408,7 @@ private void onPrevPage() {
400408
@FXML
401409
private void onNextPage() {
402410
if (isOnLastPageAndPagedFetcher()) {
403-
viewModel.fetchMoreEntriesFromLastPage();
411+
viewModel.fetchMoreEntries();
404412
} else {
405413
viewModel.goToNextPage();
406414
}

‎jabgui/src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ public class ImportEntriesViewModel extends AbstractViewModel {
7070
private final ObservableList<BibEntry> pagedEntries = FXCollections.observableArrayList();
7171
private final ObservableSet<BibEntry> checkedEntries = FXCollections.observableSet();
7272
private final ObservableList<BibEntry> allEntries = FXCollections.observableArrayList();
73-
private final SearchBasedFetcher fetcher;
74-
private final String query;
73+
private final Optional<SearchBasedFetcher> fetcher;
74+
private final Optional<String> query;
7575

7676
/**
7777
* @param databaseContext the database to import into
@@ -86,8 +86,8 @@ public ImportEntriesViewModel(BackgroundTask<ParserResult> task,
8686
StateManager stateManager,
8787
BibEntryTypesManager entryTypesManager,
8888
FileUpdateMonitor fileUpdateMonitor,
89-
SearchBasedFetcher fetcher,
90-
String query) {
89+
Optional<SearchBasedFetcher> fetcher,
90+
Optional<String> query) {
9191
this.taskExecutor = taskExecutor;
9292
this.databaseContext = databaseContext;
9393
this.dialogService = dialogService;
@@ -226,13 +226,6 @@ private Optional<BibEntry> findInternalDuplicate(BibEntry entry) {
226226
return Optional.empty();
227227
}
228228

229-
public void fetchMoreEntriesFromLastPage() {
230-
if (fetcher instanceof PagedSearchBasedFetcher pagedFetcher && !loading.get()) {
231-
loading.set(true);
232-
fetchMoreEntries(pagedFetcher);
233-
}
234-
}
235-
236229
public void goToPrevPage() {
237230
if (hasPrevPage()) {
238231
currentPageProperty.set(currentPageProperty.get() - 1);
@@ -269,7 +262,7 @@ public void updateTotalPages() {
269262
}
270263

271264
private boolean isFromWebSearch() {
272-
return fetcher != null && query != null;
265+
return fetcher.isPresent() && query.isPresent();
273266
}
274267

275268
private void updatePagedEntries() {
@@ -284,31 +277,36 @@ private void updatePagedEntries() {
284277
pagedEntries.setAll(allEntries.subList(fromIdx, toIdx));
285278
}
286279

287-
private void fetchMoreEntries(PagedSearchBasedFetcher pagedFetcher) {
288-
BackgroundTask<ArrayList<BibEntry>> fetchTask = BackgroundTask
289-
.wrap(() -> {
290-
LOGGER.info("Fetching entries from {} for page {}", fetcher.getName(), currentPageProperty.get() + 2);
291-
return new ArrayList<>(pagedFetcher.performSearchPaged(query, currentPageProperty.get() + 1).getContent());
292-
})
293-
.onSuccess(newEntries -> {
294-
if (newEntries != null && !newEntries.isEmpty()) {
295-
allEntries.addAll(newEntries);
296-
updateTotalPages();
297-
} else {
298-
LOGGER.warn("No new entries fetched from {} for page {}", fetcher.getName(), currentPageProperty.get() + 2);
299-
dialogService.notify(Localization.lang("No new entries found from %0", fetcher.getName()));
300-
}
301-
loading.set(false);
302-
})
303-
.onFailure(exception -> {
304-
loading.set(false);
305-
dialogService.showErrorDialogAndWait(
306-
Localization.lang("Error fetching entries"),
307-
Localization.lang("An error occurred while fetching entries from %0: %1",
308-
fetcher.getName(), exception.getMessage())
309-
);
310-
});
311-
312-
fetchTask.executeWith(taskExecutor);
280+
public void fetchMoreEntries() {
281+
if (fetcher.isPresent() &&
282+
fetcher.get() instanceof PagedSearchBasedFetcher pagedFetcher &&
283+
query.isPresent() && !loading.get()) {
284+
loading.set(true);
285+
BackgroundTask<ArrayList<BibEntry>> fetchTask = BackgroundTask
286+
.wrap(() -> {
287+
LOGGER.info("Fetching entries from {} for page {}", fetcher.get().getName(), currentPageProperty.get() + 2);
288+
return new ArrayList<>(pagedFetcher.performSearchPaged(query.get(), currentPageProperty.get() + 1).getContent());
289+
})
290+
.onSuccess(newEntries -> {
291+
if (newEntries != null && !newEntries.isEmpty()) {
292+
allEntries.addAll(newEntries);
293+
updateTotalPages();
294+
} else {
295+
LOGGER.warn("No new entries fetched from {} for page {}", fetcher.get().getName(), currentPageProperty.get() + 2);
296+
dialogService.notify(Localization.lang("No new entries found from %0", fetcher.get().getName()));
297+
}
298+
loading.set(false);
299+
})
300+
.onFailure(exception -> {
301+
loading.set(false);
302+
dialogService.showErrorDialogAndWait(
303+
Localization.lang("Error fetching entries"),
304+
Localization.lang("An error occurred while fetching entries from %0: %1",
305+
fetcher.get().getName(), exception.getMessage())
306+
);
307+
});
308+
309+
fetchTask.executeWith(taskExecutor);
310+
}
313311
}
314312
}

0 commit comments

Comments
 (0)