Rework UI building #24

jaghaimo · 2021-04-25T22:03:04Z

Closes #22

src/main/java/stelnet/market/MarketQueryBoard.java

src/main/java/stelnet/ui/Renderable.java

src/main/java/stelnet/ui/Cargo.java

src/main/java/stelnet/market/view/QueryRow.java

src/main/java/stelnet/storage/data/SharedData.java

mnukka

A bit of history on lombok:

The granural control with @Getter @Setter was the first thing project lombok offered. However later on they added @Data annotation.

From my experience it is better to have granural control only when you really need it. Whenever possible, write least amount of code. In otherwords, use annotations on top of the class.

As you read the code from top to down, you will see right away on the class that "ok there are getters and setters", instead of going through full class and finding out for yourself.

For example Spring framework (REST applications / Dependency injection) has most of its annotations on class level. I think it is like in 80% of the java applications today so java devs are very used to such practice.

mnukka · 2021-04-27T11:40:28Z

src/main/java/stelnet/commodity/CommodityBoard.java

@@ -33,7 +33,9 @@ private CommodityTab(String title) {
        }
    }

+    @Setter


For future, if you end up with a class where you have 3-4 setters and maybe some instance variables you are not necessarily exposing already then it is ok practice to just add @Data on the class itself not micromanaging individual properties with Getter/Setter functionality.

If there are some properties that you do not want to be made accessible through getters/setters then you can use

@Getter(value = AccessLevel.NONE) @Setter(value = AccessLevel.NONE) private Datatype myVariableName;

As the code is generated then having lombok generate a method that is not used vs human is wasting time on reading dead code - there is a difference. I would rather read less and have some dead functions in compile time. Most of the time it will not hurt anybody.

There are exceptions to everything of course. When you are writing a library for example then obviously the exposed api has to be thought through.

mnukka · 2021-04-27T11:58:30Z

src/main/java/stelnet/helper/LogHelper.java

@@ -28,7 +26,7 @@ public static void fatal(Object logObject) {
    }

    private static Logger getLogger() {


I am unsure if this helper is necessary. Logging elevation should be switched via configuration files. As a developer, you have full control over that file. In release build you can toggle logging elevation to be OFF.

This is generally how it is used and appropriate configuration settings are then in log4j.properties file. https://docs.oracle.com/cd/E29578_01/webhelp/cas_webcrawler/src/cwcg_config_log4j_file.html

public class SomeClass { private static final Logger LOG = Logger.getLogger(SomeClass.class);

This is an interesting workaround :D (below in the same file)

return Class.forName(Thread.currentThread().getStackTrace()[4].getClassName());

PS: I could do this change myself later on.

Yes please. I wanted to have a no-brains logger without having to declare the logger in each class. Ideally, I'd like to have both class name, method and line number of the log:

fqdnClass.method(line number): message stelnet.ui.Renderer.render(24): I have so much data but actual message is useless.

Yes please. I wanted to have a no-brains logger without having to declare the logger in each class.

I see. In that case this helper justifies itself. I am used to writing that extra line myself to show the intent. Matter of preference.

mnukka · 2021-04-27T12:07:48Z

src/main/java/stelnet/storage/ButtonManager.java

@@ -25,12 +26,14 @@

 public class ButtonManager {

+    @Getter


Here I would use class level @Getter instead

@Getter public class ButtonManager {

mnukka · 2021-04-27T12:08:29Z

src/main/java/stelnet/storage/FilterManager.java

@@ -3,12 +3,16 @@
 import java.util.HashSet;
 import java.util.Set;

+import lombok.Getter;
 import stelnet.filter.cargostack.CargoStackFilter;
 import stelnet.filter.fleetmember.FleetMemberFilter;

 public class FilterManager {


Here I would use class level @Getter instead

@Getter public class FilterManager {

mnukka · 2021-04-27T12:10:54Z

src/main/java/stelnet/ui/Button.java


-    private final Size size;
-    private final String title;
+public class Button extends Renderable implements ButtonHandler {


Ditch the @Getter @Setter and use

@Data public class Button extends Renderable implements ButtonHandler {

If you really really do not want to expose title and handler via setters then you can:

@Setter(value = AccessLevel.NONE) private int mySecret;

mnukka · 2021-04-27T12:12:53Z

src/main/java/stelnet/ui/Location.java

+    };
+
+    @Getter
+    private int horizontalDirection;


I think you can add here also @Getter on top of the enum itself.

mnukka · 2021-04-27T12:13:22Z

src/main/java/stelnet/ui/Position.java

@@ -1,8 +1,12 @@
 package stelnet.ui;

+import lombok.Getter;
+
 public class Position {


Class level @Getter

mnukka · 2021-04-27T12:14:58Z

src/main/java/stelnet/ui/Renderable.java

+    public Size getSize() {
+        if (size == null) {
+            LogHelper.warn("Null size found");
+        }
+        return size;
+    }
+
+    public Location getLocation() {
+        return location;
+    }
+
+    public Position getOffset() {
+        return offset;
+    }
+
+    public void setLocation(Location location) {
+        this.location = location;
+    }
+
+    public void setOffset(Position offset) {
+        this.offset = offset;
+    }


You could add here class level @Getter and keep custom implementation of getSize

mnukka · 2021-04-27T12:15:55Z

src/main/java/stelnet/ui/SimpleHandler.java

@@ -3,18 +3,18 @@
 import com.fs.starfarer.api.ui.IntelUIAPI;
 import com.fs.starfarer.api.ui.TooltipMakerAPI;

-public class SimpleCallback implements Callable {
+public class SimpleHandler implements ButtonHandler {


SimpleHandler vs EventHandler

mnukka · 2021-04-27T12:16:13Z

src/main/java/stelnet/ui/Size.java

@@ -1,23 +1,19 @@
 package stelnet.ui;

+import lombok.Getter;
+
 public class Size {


Class level getter annotation

jaghaimo · 2021-04-27T15:56:12Z

A bit of history on lombok:

The granural control with @Getter @Setter was the first thing project lombok offered. However later on they added @Data annotation.

From my experience it is better to have granural control only when you really need it. Whenever possible, write least amount of code. In otherwords, use annotations on top of the class.

As you read the code from top to down, you will see right away on the class that "ok there are getters and setters", instead of going through full class and finding out for yourself.

For example Spring framework (REST applications / Dependency injection) has most of its annotations on class level. I think it is like in 80% of the java applications today so java devs are very used to such practice.

Happy with @DaTa. I'll change it in another MR as I don't want to hold you up and won't have time today to make the change.

jaghaimo added 2 commits April 25, 2021 22:57

Rework market and storage UI building

1951b79

Simplify implementation

577f751

mnukka reviewed Apr 26, 2021

View reviewed changes

jaghaimo added 2 commits April 26, 2021 15:35

Change Commodity intel creation and remove GridRenderer

bb4b776

Use constant-case

6a91556

mnukka approved these changes Apr 26, 2021

View reviewed changes

jaghaimo added 2 commits April 27, 2021 10:59

Add Lombok

e7de888

Replace Callbable with ButtonHandler

acd02df

jaghaimo mentioned this pull request Apr 27, 2021

Fix SharedData #26

Closed

jaghaimo added 3 commits April 27, 2021 11:34

Replace default getters/setters with annotations

c72a23b

Split Queries into two classes

7ade2b2

Add and use RenderableView

f198fb7

mnukka reviewed Apr 27, 2021

View reviewed changes

jaghaimo merged commit aeed121 into master Apr 27, 2021

jaghaimo deleted the 22-ui branch April 27, 2021 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework UI building #24

Rework UI building #24

jaghaimo commented Apr 25, 2021

mnukka left a comment •

edited

Loading

mnukka Apr 27, 2021

mnukka Apr 27, 2021

jaghaimo Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

mnukka Apr 27, 2021

jaghaimo commented Apr 27, 2021

@@ @@ -33,7 +33,9 @@ private CommodityTab(String title) { @@
                       }
                   }
+                  @Setter

		@@ -28,7 +26,7 @@ public static void fatal(Object logObject) {
		}

		private static Logger getLogger() {

Rework UI building #24

Rework UI building #24

Conversation

jaghaimo commented Apr 25, 2021

mnukka left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaghaimo commented Apr 27, 2021

mnukka left a comment •

edited

Loading