Skip to content

Conversation

@perNyfelt
Copy link
Member

This pull request introduces several improvements focused on code quality, error handling, and robustness across the codebase. The most significant changes include replacing direct System.err.println() calls with a proper logging framework (SLF4J), standardizing null handling and exception policies, delegating resource URL resolution to a single utility method, and adding headless environment detection to prevent crashes in non-GUI environments.

Logging and Error Handling Improvements:

  • Replaced all System.err.println() and printStackTrace() calls in gi-fx/InOut.groovy, gi-fx/Viewer.groovy, gi-swing/InOut.groovy, and gi-console/InOut.groovy with SLF4J logging calls (log.warn, log.error). This provides consistent, configurable logging and better error reporting. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]
  • Added SLF4J API as a dependency in gi-common/build.gradle to enable logging support throughout the project.

Code Quality and Refactoring:

  • Refactored AbstractInOut.getResourceUrl() to delegate to FileUtils.getResourceUrl(), eliminating code duplication and centralizing resource lookup logic.
  • Updated the TODO.md roadmap to mark logging, duplication removal, headless detection, and null handling standardization as completed. [1] [2]

Robustness and Environment Detection:

  • Added headless environment detection in both gi-fx/InOut.groovy and gi-swing/InOut.groovy. These modules now throw UnsupportedOperationException if instantiated in a headless environment, preventing runtime crashes and providing clear guidance to use the console version instead. [1] [2]

API Documentation and Policy Standardization:

  • Updated GuiInteraction.groovy interface documentation to clearly specify null handling and exception policies for all methods, favoring exceptions for invalid input and null returns only for user cancellation or missing resources. Also updated method signatures and documentation to reflect these policies. [1] [2]

per added 3 commits December 16, 2025 23:09
- 4.1 Replace 12+ `System.err.println()` calls with a proper logging framework (SLF4J/Logback); locations include `gi-fx/InOut.groovy`, `gi-fx/Viewer.groovy`, `gi-swing/InOut.groovy`.
- 4.2 Fix `printStackTrace()` usage in `gi-swing/InOut.groovy:268`; use logger instead.
- 4.3 Remove code duplication: `AbstractInOut.getResourceUrl()` and `FileUtils.getResourceUrl()` have nearly identical logic; make one delegate to the other.
- 4.4 Add headless environment detection (`GraphicsEnvironment.isHeadless()`) in gi-fx and gi-swing to fail gracefully instead of crashing on headless systems.
- 4.5 Document and standardise null handling policy across all methods to favour exceptions; some return null on failure, others throw exceptions.
Copilot AI review requested due to automatic review settings December 16, 2025 22:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements comprehensive quality improvements focused on logging infrastructure, error handling standardization, and environment robustness. The changes replace ad-hoc error output with a proper SLF4J logging framework, eliminate code duplication, add headless environment detection to prevent crashes, and formalize null handling policies across the API.

Key Changes:

  • Migrated from System.err.println() and printStackTrace() to SLF4J logging with parameterized messages across all modules
  • Added headless environment detection in GUI modules (gi-fx, gi-swing) to fail early with clear error messages
  • Refactored AbstractInOut.getResourceUrl() to delegate to FileUtils.getResourceUrl(), eliminating duplicate resource resolution logic
  • Formalized and documented null handling and exception policies in the GuiInteraction interface

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
gi-swing/InOut.groovy Added SLF4J logger, headless detection in static initializer, replaced System.err calls with log.error/log.warn
gi-fx/Viewer.groovy Added SLF4J logger, replaced System.err calls with log.error/log.warn
gi-fx/InOut.groovy Added SLF4J logger, headless detection in static initializer, replaced System.err calls with log.error/log.warn
gi-console/InOut.groovy Added SLF4J logger, replaced System.err warning with log.warn
GuiInteraction.groovy Enhanced documentation with formal null handling and exception policies, clarified IllegalArgumentException usage for promptSelect
AbstractInOut.groovy Refactored getResourceUrl() to delegate to FileUtils.getResourceUrl(), removing duplicate code
gi-common/build.gradle Added SLF4J API 2.0.17 as api dependency
TODO.md Marked completed tasks (4.1-4.5) as done, added version 0.2.0 header

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@perNyfelt perNyfelt merged commit 54a29f8 into main Dec 16, 2025
2 checks passed
@perNyfelt perNyfelt deleted the quality_refactoring branch December 16, 2025 22:34
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.

2 participants