kinow commented on a change in pull request #296:
URL: https://github.com/apache/commons-io/pull/296#discussion_r741526857



##########
File path: 
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -46,9 +46,53 @@
  *
  * @since 1.4
  */
-public class FileWriterWithEncoding extends Writer {
-    // Cannot extend ProxyWriter, as requires writer to be
-    // known when super() is called
+public class FileWriterWithEncoding extends ProxyWriter {
+
+    private static final Writer NULL_WRITER = new Writer() {

Review comment:
       Not sure if it's handling `null`s, or being used when some value is 
`null`. But maybe `NOOP_WRITER` would be a better name?

##########
File path: 
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -160,7 +209,11 @@ public FileWriterWithEncoding(final File file, final 
String charsetName) throws
      * @throws IOException in case of an I/O error.
      */
     public FileWriterWithEncoding(final File file, final String charsetName, 
final boolean append) throws IOException {
-        this.out = initWriter(file, charsetName, append);
+        super(NULL_WRITER);
+
+        super.out = initWriter(file, charsetName, append);

Review comment:
       Ah, I think now I understand this pull request.
   
   @wodencafe I think we pass the `NULL_WRITER` to the parent `ProxyWriter` as 
a way to give it a valid object. But in reality, that `NULL_WRITER` is then 
replaced here when we call `super.out = ....`.
   
   While I guess that would work, but not sure if modifying setting `super.out` 
twice (once after the `super(NULL_WRITER)`, then here :point_up: ) would be the 
best approach.

##########
File path: 
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -46,9 +46,53 @@
  *
  * @since 1.4
  */
-public class FileWriterWithEncoding extends Writer {
-    // Cannot extend ProxyWriter, as requires writer to be
-    // known when super() is called
+public class FileWriterWithEncoding extends ProxyWriter {
+
+    private static final Writer NULL_WRITER = new Writer() {

Review comment:
       Just saw your comment about the `Writer.nullWriter()`, I guess that's 
where you got the name from :+1: no need to change it.

##########
File path: 
src/main/java/org/apache/commons/io/output/FileWriterWithEncoding.java
##########
@@ -245,7 +240,7 @@ public FileWriterWithEncoding(final String fileName, final 
String charsetName, f
      */

Review comment:
       @wodencafe do we need to override the methods below? Could we just use 
the parent methods directly?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to