-
Notifications
You must be signed in to change notification settings - Fork 25
HPCC4J-370 Remove Observable pattern from codebase #884
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
base: candidate-9.8.x
Are you sure you want to change the base?
HPCC4J-370 Remove Observable pattern from codebase #884
Conversation
Co-authored-by: jpmcmu <[email protected]>
| @@ -0,0 +1,126 @@ | |||
| /******************************************************************************* | |||
| * Copyright (c) 2024 HPCC Systems. All rights reserved. This program and the accompanying materials are made available | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This should 2025 not 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated copyright year to 2025 in both Observable.java and Observer.java (commit 575e66b)
| @@ -0,0 +1,23 @@ | |||
| /******************************************************************************* | |||
| * Copyright (c) 2024 HPCC Systems. All rights reserved. This program and the accompanying materials are made available | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This should be 2025 not 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already updated to 2025 in commit 575e66b
Co-authored-by: jpmcmu <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| import java.util.Map; | ||
| import java.util.Observable; | ||
|
|
||
| public class DataSingletonCollection extends Observable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Lets remove the dependency on the Observable pattern here entirely, the functionality is no longer needed. All locations in the code base the depend on the pattern should be updated to remove the observer functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the Observable pattern entirely. Deleted Observable.java and Observer.java files, removed extends Observable from DataSingleton and DataSingletonCollection, and removed all setChanged() and notifyObservers() calls from all platform classes (commit 7a05c21)
| @@ -0,0 +1,126 @@ | |||
| /******************************************************************************* | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This class should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already removed in commit 7a05c21
Co-authored-by: jpmcmu <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| @@ -0,0 +1,23 @@ | |||
| /******************************************************************************* | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This class should also be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already removed in commit 7a05c21
|
|
||
| void refresh(boolean full) | ||
| { | ||
| if (countObservers() > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This logic can be removed and fastRefresh() can be called directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified by removing unnecessary isComplete() check in refresh logic (commit 6900c24)
| boolean retVal = false; | ||
| if (wu != null && info.getWuid().equals(wu.getWuid()) && !info.equals(wu)) | ||
| { | ||
| if (updateState(wu)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot There is a logical error that had existed prior to these changes, retVal should be set to true inside if the if(wu != null... block and then if any of the update*( functions fail it should be set to false, please correct that issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed retVal logic - now starts as true inside the if block and becomes false if any update fails (commit 6900c24)
| boolean retVal = false; | ||
| if (dfuWorkunitWrapper != null && info.getID().equals(dfuWorkunitWrapper.getID()) && !info.equals(dfuWorkunitWrapper)) | ||
| { | ||
| if (updateState(dfuWorkunitWrapper)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot The same logical error exists here as well, please correct the logic with retVal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed retVal logic - now starts as true inside the if block and becomes false if any update fails (commit 6900c24)
Co-authored-by: jpmcmu <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.