-
Notifications
You must be signed in to change notification settings - Fork 66
Sigma balls #23
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
Sigma balls #23
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| HOST_NAME=https://ntfy.fungover.org |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,22 +1,68 @@ | ||||||||
| package com.example; | ||||||||
|
|
||||||||
| import javafx.event.ActionEvent; | ||||||||
| import javafx.fxml.FXML; | ||||||||
| import javafx.scene.control.Label; | ||||||||
| import javafx.scene.control.ListView; | ||||||||
| import javafx.scene.control.TextField; | ||||||||
| import javafx.stage.FileChooser; | ||||||||
| import javafx.stage.Stage; | ||||||||
|
|
||||||||
| import java.io.File; | ||||||||
|
|
||||||||
| /** | ||||||||
| * Controller layer: mediates between the view (FXML) and the model. | ||||||||
| */ | ||||||||
| public class HelloController { | ||||||||
|
|
||||||||
| private final HelloModel model = new HelloModel(); | ||||||||
| private final HelloModel model = new HelloModel(new NtfyConnectionImpl()); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Direct instantiation breaks testability and violates dependency injection principles. Creating
This is particularly problematic since tests like Consider one of these approaches: Option 1: Constructor injection (preferred) public class HelloController {
- private final HelloModel model = new HelloModel(new NtfyConnectionImpl());
+ private final HelloModel model;
+
+ public HelloController() {
+ this(new NtfyConnectionImpl());
+ }
+
+ // Package-private for testing
+ HelloController(NtfyConnection connection) {
+ this.model = new HelloModel(connection);
+ }Option 2: Lazy initialization with factory method public class HelloController {
- private final HelloModel model = new HelloModel(new NtfyConnectionImpl());
+ private HelloModel model;
@FXML
private void initialize() {
+ if (model == null) {
+ model = createModel();
+ }
// ... rest of initialization
+ }
+
+ // Protected for testing/override
+ protected HelloModel createModel() {
+ return new HelloModel(new NtfyConnectionImpl());
}
🤖 Prompt for AI Agents |
||||||||
|
|
||||||||
| @FXML | ||||||||
| public ListView<NtfyMessageDto> messageView; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Make messageView field private for proper encapsulation. The Apply this diff: @FXML
- public ListView<NtfyMessageDto> messageView;
+ private ListView<NtfyMessageDto> messageView;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
|
|
||||||||
| @FXML | ||||||||
| private Label messageLabel; | ||||||||
|
|
||||||||
| @FXML | ||||||||
| private TextField messageInput; | ||||||||
|
|
||||||||
| @FXML | ||||||||
| private void initialize() { | ||||||||
| if (messageLabel != null) { | ||||||||
| messageLabel.setText(model.getGreeting()); | ||||||||
| } | ||||||||
| messageView.setItems(model.getMessages()); | ||||||||
|
|
||||||||
|
|
||||||||
| model.messageToSendProperty().bindBidirectional(messageInput.textProperty()); | ||||||||
|
|
||||||||
|
|
||||||||
| messageView.setCellFactory(lv -> new javafx.scene.control.ListCell<NtfyMessageDto>() { | ||||||||
| @Override | ||||||||
| protected void updateItem(NtfyMessageDto item, boolean empty) { | ||||||||
| super.updateItem(item, empty); | ||||||||
| if (empty || item == null) { | ||||||||
| setText(null); | ||||||||
| } else { | ||||||||
| setText(item.message() != null ? item.message() : "(No message content)"); | ||||||||
| } | ||||||||
| } | ||||||||
| }); | ||||||||
| } | ||||||||
|
|
||||||||
| public void sendMessage(ActionEvent actionEvent) { | ||||||||
| model.sendMessage(); | ||||||||
|
|
||||||||
| messageInput.clear(); | ||||||||
| } | ||||||||
|
|
||||||||
| public void attachFile(ActionEvent actionEvent) { | ||||||||
| FileChooser fileChooser = new FileChooser(); | ||||||||
| fileChooser.setTitle("Select File to Attach"); | ||||||||
| File file = fileChooser.showOpenDialog(new Stage()); | ||||||||
|
|
||||||||
| if (file != null) { | ||||||||
| model.sendFile(file); | ||||||||
| } | ||||||||
| } | ||||||||
|
Comment on lines
+59
to
67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the controller's window instead of creating a new Stage. Line 62 creates a new Apply this diff to get the owner window from an existing scene node: public void attachFile(ActionEvent actionEvent) {
FileChooser fileChooser = new FileChooser();
fileChooser.setTitle("Select File to Attach");
- File file = fileChooser.showOpenDialog(new Stage());
+ Stage ownerStage = (Stage) messageInput.getScene().getWindow();
+ File file = fileChooser.showOpenDialog(ownerStage);
if (file != null) {
model.sendFile(file);
}
}
🤖 Prompt for AI Agents |
||||||||
| } | ||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,83 @@ | ||
| package com.example; | ||
|
|
||
| import javafx.application.Platform; | ||
| import javafx.beans.property.SimpleStringProperty; | ||
| import javafx.beans.property.StringProperty; | ||
| import javafx.collections.FXCollections; | ||
| import javafx.collections.ObservableList; | ||
|
|
||
| import java.io.File; | ||
|
|
||
| /** | ||
| * Model layer: encapsulates application data and business logic. | ||
| */ | ||
| public class HelloModel { | ||
|
|
||
| private final NtfyConnection connection; | ||
| private final ObservableList<NtfyMessageDto> messages = FXCollections.observableArrayList(); | ||
| private final StringProperty messageToSend = new SimpleStringProperty(); | ||
|
|
||
| public HelloModel(NtfyConnection connection) { | ||
| this.connection = connection; | ||
| startReceivingMessages(); | ||
| } | ||
|
|
||
| public ObservableList<NtfyMessageDto> getMessages() { | ||
| return messages; | ||
| } | ||
|
|
||
| public String getMessageToSend() { | ||
| return messageToSend.get(); | ||
| } | ||
|
|
||
| public StringProperty messageToSendProperty() { | ||
| return messageToSend; | ||
| } | ||
|
|
||
| public void setMessageToSend(String message) { | ||
| messageToSend.set(message); | ||
| } | ||
|
|
||
| /** | ||
| * Returns a greeting based on the current Java and JavaFX versions. | ||
| */ | ||
| public String getGreeting() { | ||
| String javaVersion = System.getProperty("java.version"); | ||
| String javafxVersion = System.getProperty("javafx.version"); | ||
| return "Hello, JavaFX " + javafxVersion + ", running on Java " + javaVersion + "."; | ||
| return "Welcome to JavaFX Chat App " + javafxVersion + ", running on Java " + javaVersion + "."; | ||
| } | ||
|
|
||
| public boolean sendMessage() { | ||
| String message = getMessageToSend(); | ||
| if (message == null || message.trim().isEmpty()) { | ||
| return false; | ||
| } | ||
| return connection.send(message.trim()); | ||
| } | ||
|
|
||
| public boolean sendFile(File file) { | ||
| if (file == null || !file.exists()) { | ||
| return false; | ||
| } | ||
| return connection.sendFile(file); | ||
| } | ||
|
Comment on lines
+50
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainConsider message clearing and return value semantics after send/sendFile. Two small behavior points worth deciding explicitly:
If you want to double‑check how the boolean is used, you can scan the controller: 🏁 Script executed: #!/bin/bash
rg -n "sendMessage\(" src/main/java
rg -n "sendFile\(" src/main/javaLength of output: 695 I need to examine the controller methods more closely to verify how the return values are actually being handled. Length of output: 378 I can see that Length of output: 383 Use the return value to gate input clearing in sendMessage() and provide feedback for sendFile(). Verification shows the boolean return values are being discarded entirely. In
🤖 Prompt for AI Agents |
||
|
|
||
| private void startReceivingMessages() { | ||
| connection.receive(this::addMessageToUI); | ||
| } | ||
|
|
||
| private void addMessageToUI(NtfyMessageDto message) { | ||
| // Check if we're on JavaFX application thread, if not use Platform.runLater | ||
| if (Platform.isFxApplicationThread()) { | ||
| messages.add(message); | ||
| } else { | ||
| Platform.runLater(() -> messages.add(message)); | ||
| } | ||
| } | ||
|
|
||
| // Test helper method - package private for testing | ||
| void addTestMessage(NtfyMessageDto message) { | ||
| // Direct add for testing (bypasses Platform.runLater) | ||
| messages.add(message); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||
| package com.example; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public class ManyParameters { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public ManyParameters(String computerName, int timeout, | ||||||||||||||||||||||||||||||||||||||
| String method, int size, byte[] data) { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Constructor accepts parameters but doesn't store them. The constructor receives 5 parameters but has an empty body. This makes the
Apply this diff to store the parameters: + private final String computerName;
+ private final int timeout;
+ private final String method;
+ private final int size;
+ private final byte[] data;
+
public ManyParameters(String computerName, int timeout,
String method, int size, byte[] data) {
-
+ this.computerName = computerName;
+ this.timeout = timeout;
+ this.method = method;
+ this.size = size;
+ this.data = data;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| static void main() { | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-standard main method signature. The method signature Apply this diff to fix the signature: - static void main() {
+ public static void main(String[] args) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| ManyParametersBuilder builder = new ManyParametersBuilder(); | ||||||||||||||||||||||||||||||||||||||
| builder | ||||||||||||||||||||||||||||||||||||||
| .setComputerName("localhost") //Fluent API | ||||||||||||||||||||||||||||||||||||||
| .setTimeout(10) | ||||||||||||||||||||||||||||||||||||||
| .setSize(0) | ||||||||||||||||||||||||||||||||||||||
| .createManyParameters(); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||||||||||||
| package com.example; | ||||||||||||||||
|
|
||||||||||||||||
| public class ManyParametersBuilder { | ||||||||||||||||
| private String computerName; | ||||||||||||||||
| private int timeout = 0; | ||||||||||||||||
| private String method; | ||||||||||||||||
| private int size = 0; | ||||||||||||||||
| private byte[] data = null; | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParametersBuilder setComputerName(String computerName) { | ||||||||||||||||
| this.computerName = computerName; | ||||||||||||||||
| return this; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParametersBuilder setTimeout(int timeout) { | ||||||||||||||||
| this.timeout = timeout; | ||||||||||||||||
| return this; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParametersBuilder setMethod(String method) { | ||||||||||||||||
| this.method = method; | ||||||||||||||||
| return this; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParametersBuilder setSize(int size) { | ||||||||||||||||
| this.size = size; | ||||||||||||||||
| return this; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParametersBuilder setData(byte[] data) { | ||||||||||||||||
| this.data = data; | ||||||||||||||||
| return this; | ||||||||||||||||
|
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No defensive copy of byte array parameter. The builder stores the Apply this diff to make a defensive copy: public ManyParametersBuilder setData(byte[] data) {
- this.data = data;
+ this.data = data != null ? data.clone() : null;
return this;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| public ManyParameters createManyParameters() { | ||||||||||||||||
| return new ManyParameters(computerName, timeout, method, size, data); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package com.example; | ||
|
|
||
| import java.io.File; | ||
| import java.util.function.Consumer; | ||
|
|
||
| public interface NtfyConnection { | ||
| boolean send(String message); | ||
| void receive(Consumer<NtfyMessageDto> messageHandler); | ||
| boolean sendFile(File file); | ||
| } |
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.
Incorrect groupId for jackson-databind dependency.
The groupId
tools.jackson.coreappears to be incorrect. The standard Jackson groupId iscom.fasterxml.jackson.core. This incorrect groupId will likely cause the dependency to fail to resolve from Maven Central.Apply this diff to use the correct groupId:
Additionally, verify that version 3.0.1 is appropriate for your Java version, as Jackson 3.x requires Java 17+.
📝 Committable suggestion
🤖 Prompt for AI Agents