Uh oh!
There was an error while loading. Please reload this page.
Feature/atomic factory - #59
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the AtomicFactory pattern to provide a more structured way to handle API responses through inheritance. The main changes include creating a new abstract base class for custom response handlers and refactoring the API to use more conventional method naming.
- New
AtomicFactoryabstract class enabling custom response wrappers via inheritance - Renamed
getResponse()toexecute()for better API semantics - Added example test implementation demonstrating the factory pattern with
thenApplytransformation
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/fr/sandro642/github/provider/AtomicFactory.java | New abstract class providing base functionality for custom API response wrappers |
| src/main/java/fr/sandro642/github/jobs/JobGetInfos.java | Renamed method from getResponse() to execute() for clearer intent |
| src/test/java/fr/sandro642/github/test/MainTest.java | Added test class demonstrating AtomicFactory usage and updated all method calls to use execute() |
| src/main/java/fr/sandro642/github/example/ExampleUsages.java | Removed example file (all lines deleted) |
| build.gradle | Version bumped from 0.4.3.1-STABLE to 0.4.4-DEV_BUILD |
| .github/workflows/work-jar.yml | Updated branch name from Feature/AtomicFactory to feature/AtomicFactory for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.apiFactory = apiFactory; | ||
| } | ||
| protected Map<?,?> rawPhysx() { |
There was a problem hiding this comment.
The rawPhysx() method doesn't check if apiFactory is null before calling getRawData(). If this method is called before getPhysx() is invoked, it will throw a NullPointerException. Consider adding a null check or initializing the field appropriately.
| protectedMap<?,?> rawPhysx() { | |
| protectedMap<?,?> rawPhysx() { | |
| if (apiFactory == null) { | |
| thrownewIllegalStateException("ApiFactory has not been initialized. Call getPhysx() first."); | |
| } |
| .thenApply(ClassheritFromFactory::new); | ||
| System.out.println("Response: " + apiFactory.display()); | ||
| ClassheritFromFactory classheritFromFactory = apiFactoryCompletableFuture.get(5, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Typo in variable name: "classherit" should be "classInherit" to match proper spelling and be consistent with the class name correction.
| //.webServices(8080, "TestDashboard"); | ||
| CompletableFuture<ApiFactory> apiFactoryCompletableFuture = connectLib.JobGetInfos() | ||
| CompletableFuture<ClassheritFromFactory> apiFactoryCompletableFuture = connectLib.JobGetInfos() |
There was a problem hiding this comment.
Typo in type reference: "ClassheritFromFactory" should be "ClassInheritFromFactory" to match proper spelling.
| ApiFactory apiFactory = apiFactoryCompletableFuture.get(5, TimeUnit.SECONDS); | ||
| .urlBranch(ExampleUrlBranch.POST_PROD) | ||
| .execute() | ||
| .thenApply(ClassheritFromFactory::new); |
There was a problem hiding this comment.
Typo in type reference: "ClassheritFromFactory" should be "ClassInheritFromFactory" to match proper spelling.
| public class ClassheritFromFactory extends AtomicFactory { | ||
| public ClassheritFromFactory(ApiFactory apiFactory) { |
There was a problem hiding this comment.
Typo in class name: "Classherit" should be "ClassInherit" or "ClassThatInherits". The current name appears to be missing "In" between "Class" and "herit".
| publicclassClassheritFromFactoryextendsAtomicFactory { | |
| publicClassheritFromFactory(ApiFactoryapiFactory) { | |
| publicclassClassInheritFromFactoryextendsAtomicFactory { | |
| publicClassInheritFromFactory(ApiFactoryapiFactory) { |
| } | ||
| } | ||
| public class ClassheritFromFactory extends AtomicFactory { |
There was a problem hiding this comment.
ClassheritFromFactory should be made static, since the enclosing instance is not used.
| publicclassClassheritFromFactoryextendsAtomicFactory { | |
| publicstaticclassClassheritFromFactoryextendsAtomicFactory { |
No description provided.