refactor: Modernize ChaincodeBase class #357
Conversation
Signed-off-by: Bhaskar Ram <[email protected]>
fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/shim/ChaincodeBase.java
Show resolved
Hide resolved
fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/shim/ChaincodeBase.java
Outdated
Show resolved
Hide resolved
| Optional.ofNullable(this.getClass().getPackage()) | ||
| .ifPresentOrElse( | ||
| pkg -> Logger.getLogger(pkg.getName()).setLevel(chaincodeLogLevel), | ||
| () -> Logger.getLogger("").setLevel(chaincodeLogLevel) | ||
| ); |
There was a problem hiding this comment.
I don't see any real benefit to using an optional to perform an if/else, and I don't think this is available in Java 8
There was a problem hiding this comment.
Replaced the Optional usage with a simple null check.
Uses a standard if/else statement, which is more readable and familiar to most Java developers. Now in the latest commit It Is compatible with Java 8 and earlier versions.
| return switch (level.toUpperCase().trim()) { | ||
| case "CRITICAL", "ERROR" -> Level.SEVERE; | ||
| case "WARNING", "WARN" -> Level.WARNING; | ||
| case "INFO" -> Level.INFO; | ||
| case "NOTICE" -> Level.CONFIG; | ||
| case "DEBUG" -> Level.FINEST; | ||
| default -> Level.INFO; | ||
| }; |
There was a problem hiding this comment.
This looks reasonable but the project uses the Java 8 language level, which does not support switch expressions
There was a problem hiding this comment.
Updated in latest commit.
| Optional.ofNullable(System.getenv(CORE_PEER_ADDRESS)) | ||
| .ifPresent(address -> { | ||
| String[] hostArr = address.split(":"); | ||
| if (hostArr.length == 2) { | ||
| this.port = Integer.parseInt(hostArr[1].trim()); | ||
| this.host = hostArr[0].trim(); | ||
| } else { | ||
| LOGGER.severe(() -> String.format("peer address argument should be in host:port format, ignoring current %s", address)); | ||
| } | ||
| }); |
There was a problem hiding this comment.
I don't see any real benefit to using an optional only to perform a null check
There was a problem hiding this comment.
Replaced the Optional.ofNullable().ifPresent() with a simple null check using an if statement. Improved readability by using a more straightforward and familiar Java construct.
| LOGGER.info(() -> String.format(""" | ||
| <<<<<<<<<<<<<Environment options>>>>>>>>>>>> | ||
| CORE_CHAINCODE_ID_NAME: %s | ||
| CORE_PEER_ADDRESS: %s | ||
| CORE_PEER_TLS_ENABLED: %s | ||
| CORE_PEER_TLS_ROOTCERT_FILE: %s | ||
| CORE_TLS_CLIENT_KEY_PATH: %s | ||
| CORE_TLS_CLIENT_CERT_PATH: %s | ||
| CORE_TLS_CLIENT_KEY_FILE: %s | ||
| CORE_TLS_CLIENT_CERT_FILE: %s | ||
| CORE_PEER_LOCALMSPID: %s | ||
| CHAINCODE_SERVER_ADDRESS: %s | ||
| LOGLEVEL: %s | ||
| """, |
There was a problem hiding this comment.
Text blocks are not available in Java 8
There was a problem hiding this comment.
Latest Commit is now fully compatible with Java 8.
Signed-off-by: Bhaskar Ram <[email protected]>
|
|
@bestbeforetoday I've added recent changes for compatibility for Java 8, please review it. |
|
The build is failing since the code does not even compile. Looking at the current set of changes (after the use of language features not available in Java 8 are reverted), the vast majority of the changes are reformatting of the code. A couple of variables are renamed and, in a couple of cases, the logic flow of the code is slightly reordered. What issue is this change addressing? |
|
No response so closing. |
refactor: Modernize ChaincodeBase class
Signed-off-by: Bhaskar Allam [email protected]