atom feed17 messages in org.apache.myfaces.devRe: [DISCUSS] how to get rid of tons ...
FromSent OnAttachments
Mark StrubergOct 22, 2011 4:08 pm.txt
Jakob KorherrOct 23, 2011 2:59 am 
Mark StrubergOct 23, 2011 8:55 am 
Jakob KorherrOct 23, 2011 9:14 am 
Leonardo UribeOct 23, 2011 11:27 am 
Leonardo UribeOct 23, 2011 12:08 pm 
Mark StrubergOct 23, 2011 3:37 pm 
Leonardo UribeOct 23, 2011 7:35 pm 
Mark StrubergOct 24, 2011 1:37 am 
Leonardo UribeOct 24, 2011 10:17 am 
Mark StrubergOct 24, 2011 3:14 pm 
Leonardo UribeOct 24, 2011 3:29 pm 
Leonardo UribeOct 24, 2011 7:02 pm 
Mark StrubergOct 24, 2011 11:38 pm 
Mark StrubergOct 25, 2011 4:32 am 
Leonardo UribeOct 25, 2011 8:47 am 
Mark StrubergOct 25, 2011 9:23 am 
Subject:Re: [DISCUSS] how to get rid of tons of duplicated code
From:Mark Struberg (stru@yahoo.de)
Date:Oct 23, 2011 8:55:02 am
List:org.apache.myfaces.dev

With this approach each MyFaces subproject would be able to pick out only the stuff it really needs.

This is not needed if we use the maven-shade-plugin <minimizeJar> option [1] !

With <minimizeJar> enabled the dependency classes are analyzed and only the
classes which really got used will get shaded into the destination jar. The only
limitation is when some classes get loaded via Class.forName() or similar. But
as long as there is a bytecode invocation it will work smoothly.

Thus I really see no reason why we cannot use maven-shared these days!

Thus also a formal +1 from me.

LieGrue, strub

[1]
http://maven.apache.org/plugins/maven-shade-plugin/shade-mojo.html#minimizeJar

----- Original Message -----

From: Jakob Korherr <jako@gmail.com> To: MyFaces Development <de@myfaces.apache.org>; Mark Struberg
<stru@yahoo.de> Cc: Sent: Sunday, October 23, 2011 12:00 PM Subject: Re: [DISCUSS] how to get rid of tons of duplicated code

Hi Mark,

+1 - that's exactly what I have been trying to accomplish some time ago (introducing common-shades [1]). Unfortunately, I was not successful back then.

However, there is a slight problem with moving all this stuff into MyFaces shared, which I want to point out: code size. If we really put all the code that is shared across any MyFaces subproject into shared, it will get fat and ugly (even more than it is right now). In addition, if we continue including the whole shared project into MyFaces core, MyFaces core impl will get bigger and bigger.

Thus I'd like to suggest something similar which I wanted to accomplish with common-shades: Introduce a new shared module, which consists of many submodules that each handle a specific functionality instead of being one fat module. With this approach each MyFaces subproject would be able to pick out only the stuff it really needs. Furthermore we would see more easily which code in shared is not used anymore (I guess at the moment there is a lot of it), just by checking which modules are still used in our poms.

Regards, Jakob

[1] https://svn.apache.org/repos/asf/myfaces/common-shades/

2011/10/23 Mark Struberg <stru@yahoo.de>:

Hi! While working on the mafyces-commons cleanup I figured that we have tons of duplicated code spread over MyFaces.

As an example I like to mention myfaces-commons-resourcehandler. There are 43 classes in total, and 35 of them are just 1:1 copied from other projects to provide resource management, zip, etc. For me this is an absolute no-go. Those classes have neither tests nor any documentation where they got forked from. Nor will any bug which gets fixed in another module make it's way over to all the other projects containing that very forked code. That's just ... unbelievable unmaintainable.

There are 2 different ways to solve this (depending on the problem):

A.) drop the functionality and provide a generalized solution. The GZIP of myfaces-commons-resourcehandleris an obvious example: We now copy this code over the 4th time or even more. Instead of doing this, we should rather do it in the classic unix fashion: do one thing, but do it well. Which means I'd rather see all the GZIP stuff factored out into an own mf-commons module as a Servlet Filter. This can then get applied to what ever other mechanism you like. This could also (commonly) cover cases like detecting http UserAgents which are not able to handle zipped resources, etc. That way we could provide this logic ONCE and have complete freedom over the configuration.

B.) code reusable components once and use them in other projects (ev via shading it in). ClassLoaderResourceLoader would be a perfect candidate! I grepped through only the few pits which I have checked out locally and found this class 7 SEVEN times! I just can't believe that we can't move this stuff to a shared modul...

Same for FacesServletMapping. 6 times copied around, WebConfigProviderFactory 5 times, ... There are whole packages with 10++ classes which got copied 1:1!

I really could cry seeing this :(

What can we do to solve this?

Theoretically myfaces-shared should contain this stuff. This is exactly what it is for! Historically there have been some hand forged tweeks and ugly hacks, but nowadays we have the maven-shade-plugin to make our live easier.

So the suggestion is:

1.) cleanup myfaces-shared. mf-shared has almost no checkstyle rules applied. 2.) add unit tests for myfaces-shared. Currently there are not many... 3.) move the shared code parts back to myfaces-shared and add unit tests. 4.) import myfaces-shared via maven dependency and use <minimizeJar> and <relocations> to package the stuff

[+1] fine go ahead (ideally: yes, what parts can I help with?) [0] dont care [-1] wont work because ...

I've attached a file which contains all Classes which name exists multiple times in MyFaces. The number is the cound how often they exist in MyFaces. I excluded current20. Please note that classes with the same name do not necessarily have the same content - but quite a lot actually do have! (scroll to the bottom of the file ...)

LieGrue, strub

-- Jakob Korherr

blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at