On Thu, Feb 18, 2016 at 2:13 PM, Remko Popma <[email protected]> wrote:
> I think preserving binary compatibility on its own is a strong reason for > doing this, but it's more than that. > OK, since org.apache.logging.log4j.message.MessageFactory is in log4j-api that's important. I can buy that. BUT, we are also adding methods to Logger so that would break some things too. I guess less breakage is better than more in this case! Overall, I not convinced that this is the best approach but I can appreciate that you seem to feel about it stronger that I do. > > Having a separate factory for flow messages makes both factories more > cohesive (single responsibility principle). No need for one factory to > extend the other in my view. > The distinction is pretty subtle here IMO. We are still talking about creating messages, but I get your point. For me, the only reason for this is to minimize the risk of API breakage, a nobe goal for the log4j-api module, if not a requirement. > > The logger would have separate instances so users can configure them > separately: lower coupling. > OK. So now we have: org.apache.logging.log4j.LogManager.getLogger(Class<?>, MessageFactory) org.apache.logging.log4j.LogManager.getLogger(Object, MessageFactory) org.apache.logging.log4j.LogManager.getLogger(String, MessageFactory) We would add: org.apache.logging.log4j.LogManager.getLogger(Class<?>, MessageFactory, FlowMessageFactory) org.apache.logging.log4j.LogManager.getLogger(Object, MessageFactory, FlowMessageFactory) org.apache.logging.log4j.LogManager.getLogger(String, MessageFactory, FlowMessageFactory) Right? Any other places? > > These are both desirable properties so I believe it would improve the > design. > > Does this make sense? > Sure, even though I am less gun-ho about it than you are. I'd say go ahead, see how it looks and feels after you refactor. We can keep discussing it once your changes hits the repo if need be. Thank you for putting in the work! Gary Remko > > Sent from my iPhone > > On 2016/02/19, at 2:24, Gary Gregory <[email protected]> wrote: > > Is a flow message factory a kind of message factory or a different kind of > factory? > > Does a logger need instances of both or just the one? > > Since entry message extends message, should the factory do so as well? > > Gary, phone, typos. > On Feb 18, 2016 8:44 AM, "Remko Popma" <[email protected]> wrote: > >> Would anyone mind terribly if I factored out the FlowMessage creation >> methods from MessageFactory to a new interface FlowMessageFactory? >> >> Concretely, this interface would contain the methods introduced in >> LOG4J2-1255: >> >> EntryMessage newEntryMessage(Message message); >> ExitMessage newExitMessage(Object object, Message message); >> ExitMessage newExitMessage(EntryMessage message); >> ExitMessage newExitMessage(Object object, EntryMessage message); >> >> I think flow messages are different enough from normal Messages that a >> separate factory makes sense. >> >> It would also insulate users who created a custom MessageFactory from the >> changes we made in LOG4J2-1255. >> >> Thoughts? >> >> -Remko >> >> >> -- E-Mail: [email protected] | [email protected] Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/> Spring Batch in Action <http://www.manning.com/templier/> Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
