You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@eventmesh.apache.org by "Pil0tXia (via GitHub)" <gi...@apache.org> on 2023/07/23 07:16:50 UTC
Re: [PR] [Enhancement] Method may fail to clean up stream or resource[WatchFileManagerTest] #4098 (eventmesh)
Pil0tXia commented on code in PR #4274:
URL: https://github.com/apache/eventmesh/pull/4274#discussion_r1271393737
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/file/WatchFileManagerTest.java:
##########
@@ -50,11 +50,18 @@ public boolean support(FileChangeContext changeContext) {
};
WatchFileManager.registerFileChangeListener(f.getParent(), fileChangeListener);
- Properties properties = new Properties();
- properties.load(new BufferedReader(new FileReader(file)));
- properties.setProperty("eventMesh.server.newAdd", "newAdd");
- FileWriter fw = new FileWriter(file);
- properties.store(fw, "newAdd");
+ try (FileReader fr = new FileReader(file);
+ BufferedReader br = new BufferedReader(fr);
+ FileWriter fw = new FileWriter(file)) {
+
+ Properties properties = new Properties();
+ properties.load(br);
+
+ properties.setProperty("eventMesh.server.newAdd", "newAdd");
+ properties.store(fw, "newAdd");
+ } catch (IOException e) {
+ e.printStackTrace();
Review Comment:
Please use log4j instead.
##########
eventmesh-common/src/test/java/org/apache/eventmesh/common/file/WatchFileManagerTest.java:
##########
@@ -50,11 +50,18 @@ public boolean support(FileChangeContext changeContext) {
};
WatchFileManager.registerFileChangeListener(f.getParent(), fileChangeListener);
- Properties properties = new Properties();
- properties.load(new BufferedReader(new FileReader(file)));
- properties.setProperty("eventMesh.server.newAdd", "newAdd");
- FileWriter fw = new FileWriter(file);
- properties.store(fw, "newAdd");
+ try (FileReader fr = new FileReader(file);
+ BufferedReader br = new BufferedReader(fr);
+ FileWriter fw = new FileWriter(file)) {
Review Comment:
Good job.
When using `.close()` to close nested streams, we only need to close the outermost stream, because the close request propagates from the outermost stream to the innermost stream.
To have try-with-resources close them, you assigned the streams to variables as you open them, instead of nesting. If you use nesting, an exception during construction of one of the later streams (e.g. `GZIPOutputStream`) will leave any stream constructed by the nested calls inside it open.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org