Github user kinow commented on a diff in the pull request:

    https://github.com/apache/commons-imaging/pull/35#discussion_r210523866
  
    --- Diff: 
src/main/java/org/apache/commons/imaging/formats/psd/PsdHeaderInfo.java ---
    @@ -45,9 +50,14 @@ public PsdHeaderInfo(final int version, final byte[] 
reserved, final int channel
         }
     
         public void dump() {
    -        final PrintWriter pw = new PrintWriter(new 
OutputStreamWriter(System.out, Charset.defaultCharset()));
    -        dump(pw);
    -        pw.flush();
    +        try (StringWriter sw = new StringWriter(); PrintWriter pw = new 
PrintWriter(sw)) {
    --- End diff --
    
    @garydgregory I'm not sure how to refactor it. If you look at the `dump` 
methods, you will see they are scattered all over the place, but there's no 
`@Override` tags, as they are not defined in any parent class.........
    
    I am ignoring this for now, as we could have users coming from Sanselan 
using these methods. But for 2.x I would like to tackle this problem and either 
remove the `dump` methods, or have a design with a base class defining the 
signature of these methods.
    
    That would give us a place to implement a method that could remove this 
duplication.
    
    The alternative that I see here, is to add a `Util` class to the project, 
and add a method to remove the duplication. But I am not very fond of `Util` 
classes for a case like this one (where a proper design would remove the need 
for it).
    
    I hope this makes sense. Do you think we could ignore this duplication for 
now, and fix in the upcoming (hopefully soon) next releases?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to