logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <ralph.go...@dslextreme.com>
Subject Re: Proposed change to MarkerManager API
Date Mon, 07 Apr 2014 06:58:09 GMT
I have mixed feelings about this.  I dislike the ambiguity of getMarker()  creating a Marker
with the specified parents if it doesn’t exist or just returning the existing Marker, even
if the parents are different.  However, I don’t really see anything in your proposal that
fixes that.  I really dislike that the second cal to define would override the first. I believe
it should throw an exception because what is happening is clearly wrong and should be fixed.

After thinking about this the only sensible thing I can think of is to get rid of the getMarker
calls that accept parents and totally rely on the add method to create the parents.  That
happens to also be how SLF4J works.  But, of course, that will break backward compatibility.

Ralph

On Apr 6, 2014, at 7:35 PM, Bruce Brouwer <bruce.brouwer@gmail.com> wrote:

> I hate changing API as much as the next guy, but there is an API in MarkerManager that
I think could be improved. 
> 
> MarkerManager.getMarker(name) gets a marker, optionally creating it if it doesn't exist.
I support this with no change.
> 
> MarkerManager.getMarker(name, parents...) I have an issue with. It does not in all cases
return me a marker that has the parents specified. If the marker already existed, it is simply
returned with no changes made to the parents. I propose removing this method and replacing
it with...
> 
> MarkerManager.define(name, parents...) This method will create the marker with the specified
parents if it does not exist. If it does exist, it will change the parent list to be the list
specified. 
> 
> Here's another reason I want to change this. Consider these two classes:
> 
> public class A {
>   private final static Marker m = MarkerManager.getMarker("m", "p");
> }
> 
> public class B {
>   private final static Marker m = MarkerManager.getMarker("m");
> }
> 
> If class A gets loaded first, marker "m" will have parent "p". But if class B gets loaded
first, then marker "m" will have no parents. I generally don't like relying on the exact order
that classes are loaded when the two classes aren't related. 
> 
> But if we consider my proposed change:
> 
> public class A {
>   private final static Marker m = MarkerManager.define("m", "p");
> }
> 
> public class B {
>   private final static Marker m = MarkerManager.getMarker("m");
> }
> 
> Now the behavior is the same, no matter which class is loaded first. It looks clearer
to me as the definition of the marker happens in one place, while class B is only interested
in a reference to a potentially already defined marker, but makes no statement about what
parents it has. 
> 
> I also don't like the idea of the define method throwing an exception if the marker already
exists or getMarker returning null if the marker doesn't exist; again for the reasons that
I don't want class loading order to impact behavior. 
> 
> I understand that if there are two statements defining a marker with different parents,
whichever one runs last is going to be the winner. It's not great if that happens, but I shouldn't
be defining a marker in two places. In reality, the best thing would be to make class B reference
the marker field in class A. If I do that, then the current implementation would work fine;
I'm just thinking of the case where somebody doesn't (or can't) do the best thing. Then, a
distinction between get and define could be helpful.
> 
> Does this sound like an acceptable change that we could get into log4j-api before GA?
> 
> -- 
> 
> Bruce Brouwer


---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message