Skip to content

Stand-alone app for save-and-restore search results#3279

Closed
georgweiss wants to merge 3 commits intomasterfrom
CSSTUDIO-2733
Closed

Stand-alone app for save-and-restore search results#3279
georgweiss wants to merge 3 commits intomasterfrom
CSSTUDIO-2733

Conversation

@georgweiss
Copy link
Copy Markdown
Collaborator

@georgweiss georgweiss commented Feb 18, 2025

As per user request:

When an action is opening a save-and-restore filter, the regular save-and-restore UI should not be used. Instead a dedicated app/view should launch to display only the search result associated with the filter.

The new app reuses same view as the existing search and filter view in the main save and restore app. It offers same functionality with respect to the context menu. Refactoring was necessary to achieve a result where - in the spirit of resource spending control - the UI and its controller is reused between apps.

Screenshot 2025-02-18 at 14 15 42

SaveAndRestoreApplication saveAndRestoreApplication = (SaveAndRestoreApplication) appDescriptor;
AppDescriptor saveAndRestoreAppDescriptor = ApplicationService.findApplication(SaveAndRestoreApplication.NAME);
if (saveAndRestoreAppDescriptor != null && saveAndRestoreAppDescriptor instanceof SaveAndRestoreApplication) {
SaveAndRestoreApplication saveAndRestoreApplication = (SaveAndRestoreApplication) saveAndRestoreAppDescriptor;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The cast can be made type-safe by changing the the if-statement to
if (saveAndRestoreAppDescriptor instanceof SaveAndRestoreApplication saveAndRestoreApplication) { ...

Copy link
Copy Markdown
Collaborator Author

@georgweiss georgweiss Feb 18, 2025

Choose a reason for hiding this comment

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

How is it not type-safe the way it stands?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if (x instance of ABC)
{
   ABC abc = (ABC) x;

is type-safe.
.. except one could have typos and accidentally do this:

if (x instance of ABC)
{
   XYZ abc = (XYZ) x;

In the newfangled syntax, the type is only written once

if (x instance ABC abc)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a downside, the new syntax is ugly.
While at it, the null-check isn't necessary. if (x instanceof XXX) is false for x=null, so can be shorter and more readable .. which does free up space for the new ugly syntax

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@georgweiss: You are right that casting (in Java) is type-safe. However, a runtime-exception will be thrown if the cast doesn't succeed, and the compiler doesn't check that the runtime exception is caught.

With the pattern matching construct the compiler can give better guarantees: in the body of the then-clause of the if-expression it is known that the cast succeeded and that the variable has the correct type. If the cast didn't succeed for some reason, this can, e.g., be handled in an else-clause (where the variable that was bound in the then-clause isn't bound). This allows the programmer to handle all cases and for the compiler to type-check the program without any risk of ClassCastException from the cast at runtime.

In simple cases such as this, the main advantage is to guard against typos as @kasemir pointed out. In more complicated cases it can help the programmer to know that all cases that can arise have been covered in the code.

AppDescriptor saveAndRestoreAppDescriptor = ApplicationService.findApplication(SaveAndRestoreApplication.NAME);
if (saveAndRestoreAppDescriptor != null && saveAndRestoreAppDescriptor instanceof SaveAndRestoreApplication) {
SaveAndRestoreApplication saveAndRestoreApplication = (SaveAndRestoreApplication) saveAndRestoreAppDescriptor;
SaveAndRestoreInstance saveAndRestoreInstance = (SaveAndRestoreInstance) saveAndRestoreApplication.getInstance();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This cast can be made type-safe by changing the return type of SaveAndRestoreApplication.getInstance() to SaveAndRestoreInstance instead of AppInstance.


AppDescriptor filterViewAppDescriptor = ApplicationService.findApplication(FilterViewApplication.NAME);
if (filterViewAppDescriptor != null && filterViewAppDescriptor instanceof FilterViewApplication) {
FilterViewApplication filterViewApplication = (FilterViewApplication) filterViewAppDescriptor;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As above, this cast can be made type-safe by changing the if-statement to: if (filterViewAppDescriptor instanceof FilterViewApplication filterViewApplication) { ...

AppDescriptor filterViewAppDescriptor = ApplicationService.findApplication(FilterViewApplication.NAME);
if (filterViewAppDescriptor != null && filterViewAppDescriptor instanceof FilterViewApplication) {
FilterViewApplication filterViewApplication = (FilterViewApplication) filterViewAppDescriptor;
FilterViewInstance filterViewInstance = (FilterViewInstance) filterViewApplication.getInstance();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As above, this can be made type-safe by changing the return type of FilterViewInstance.getInstance() to FilterViewInstance.

import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unused import.

import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unused import.

import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unused import.

import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
import org.phoebus.ui.javafx.ImageCache;

import java.util.function.Consumer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unused import.

import org.phoebus.applications.saveandrestore.model.Node;
import org.phoebus.applications.saveandrestore.model.NodeType;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreBaseController;
import org.phoebus.applications.saveandrestore.ui.SaveAndRestoreController;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unused import.

@georgweiss georgweiss closed this Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants