atom feed30 messages in org.apache.commons.devRe: [imaging] IMAGING-154 remove Debu...
FromSent OnAttachments
Bruno P. KinoshitaFeb 5, 2018 4:20 am 
Otto FowlerFeb 5, 2018 4:41 am 
GillesFeb 5, 2018 5:47 am 
Bruno P. KinoshitaFeb 5, 2018 10:27 pm 
Jörg SchaibleFeb 6, 2018 12:21 am 
Bruno P. KinoshitaFeb 6, 2018 1:52 am 
sebbFeb 6, 2018 2:06 am 
Bruno P. KinoshitaFeb 6, 2018 2:30 am 
Bruno P. KinoshitaAug 12, 2018 1:56 am 
GillesAug 12, 2018 5:21 am 
Remko PopmaAug 12, 2018 7:00 am 
Gary GregoryAug 12, 2018 7:12 am 
Matt SickerAug 12, 2018 8:17 am 
Remko PopmaAug 12, 2018 2:49 pm 
Bruno P. KinoshitaAug 13, 2018 1:40 am 
Bruno P. KinoshitaAug 13, 2018 1:56 am 
Bruno P. KinoshitaAug 13, 2018 1:59 am 
Bruno P. KinoshitaAug 13, 2018 2:12 am 
Bruno P. KinoshitaAug 13, 2018 2:22 am 
GillesAug 13, 2018 3:40 am 
Remko PopmaAug 13, 2018 4:55 am 
Bruno P. KinoshitaAug 13, 2018 5:05 am 
Remko PopmaAug 13, 2018 6:57 am 
sebbAug 13, 2018 7:05 am 
Gary GregoryAug 13, 2018 7:50 am 
Matt SickerAug 13, 2018 12:01 pm 
sebbAug 14, 2018 3:56 am 
Matt SickerAug 14, 2018 2:19 pm 
Bruno P. KinoshitaAug 15, 2018 5:26 am 
Bruno P. KinoshitaAug 17, 2018 1:21 am 
Subject:Re: [imaging] IMAGING-154 remove Debug class
From:Bruno P. Kinoshita (brun@yahoo.com.br.INVALID)
Date:Feb 6, 2018 2:30:39 am
List:org.apache.commons.dev

Hi sebb,

Another aspect of debugging is ensuring that methods are small and

easily tested independently. However this is difficult to do, and care must be taken to ensure that the public API is not unnecessarily extended..

A very good point.

The parsers in commons-imaging expose some #dump... methods
(https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/ImageParser.java#L794).

While I can see that parsers may need to dump the data they are holding in some
structured way for inspecting, reporting, serializing, etc, it looks like some
other classes were affected by it too. For example...

A JPEG Segment has a #dump() method

https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/formats/jpeg/segments/Segment.java#L34

which gets defined in each subclass of Segment. It can be confusing to have a
method such as #dump() in a Segment, from the point of view of someone writing a
photo editor for example. The user could use that to pass his/her own logger's
PrintWriter, which would make removing or changing logging in the future in
commons-imaging.

If we keep the Debug class, and make it internal, there would still be these
methods to take care. And there are some methods where users can provide a
PrintWriter, while others instead use System.out
(e.g.https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/FormatCompliance.java#L70).

Cheers Bruno

________________________________ From: sebb <seb@gmail.com> To: Commons Developers List <de@commons.apache.org>; Bruno P. Kinoshita
<brun@yahoo.com.br> Sent: Tuesday, 6 February 2018 11:06 PM Subject: Re: [imaging] IMAGING-154 remove Debug class

On 6 February 2018 at 09:52, Bruno P. Kinoshita <brun@yahoo.com.br.invalid> wrote:

Hi Jorg,

I'd be fine with that solution too. I think this one would cause the smaller
change to the code as is.

I believe my preference is still to remove the Debug class. But between logging
and making Debug internal only, I'd choose making it internal.

+1

I think making it internal means it can still be dropped later.

Looking forward to hearing what others think about these options.

Another aspect of debugging is ensuring that methods are small and easily tested independently. However this is difficult to do, and care must be taken to ensure that the public API is not unnecessarily extended..

Thanks Bruno

________________________________ From: Jörg Schaible <joer@bpm-inspire.com> To: de@commons.apache.org Sent: Tuesday, 6 February 2018 9:24 PM Subject: Re: [imaging] IMAGING-154 remove Debug class

Hi Bruno,

if it might also be helpful to our users, why not keep and provide it. As

I understand it, the Debug class is a tool helping in development to

analyze some behavior.

Nothing stops us from declaring this class internal (we might even put it

into a package "internal" or "debug") that might be changed without

further comment. Nobody may rely on it in production code, but during

development it might be helpful. With such an approach we might not have

a need to find a better interface to provide this functionality.

Just my 2¢,

Jörg

Am Mon, 05 Feb 2018 12:20:58 +0000 schrieb Bruno P. Kinoshita:

Hello,

If memory serves me well, some time ago we had a discussion around

sanselan & commons-imaging 1.0. One of the issues with commons-imaging

1.0 was the Debug class.

I finished the pull request, but Gilles raised an important point, about

discussing other alternatives first.

Initially I am against logging in low level libraries, especially

commons components. But some time ago I had to debug TIFF issues in

commons-imaging, and having the dump methods was a tremendous help.

The issue is that some imaging algorithms/processing have a lot of

variables that can be altered. And keeping an eye on all of them in the

debugger can be quite hard - though not impossible.

So all in all, now I am more confident to proceed without the Debug

class. But some users could have a hard time investigating possible

issues in the library without seeing what's going on within the library.

IMO, that could be solved with the logging/dump features... or through a

better design, especially around exception handling/throwing. The latter

is my preferred approach. Instead of logging, I prefer - whenever

possible - that low level libraries throw exceptions and let me handle

the logging.

So, any thoughts? :) I'm +1 to remove the Debug class, and +0 to a

logging added to commons-imaging.

---------------------------------------------------------------------

To unsubscribe, e-mail: dev-@commons.apache.org

For additional commands, e-mail: dev-@commons.apache.org