myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: [DISCUSS] why do we overcomplicate our code soooo much?
Date Fri, 28 Oct 2011 01:17:08 GMT
Hi

I think you have the wrong impression, basically because you haven't
tried to solve each one of the problems that the code try to solve. Is
like the blind that try to see the elephant and say that is like a
tree, because it touching its leg.

To keep things simple, I'll describe and explain one by one each
problem "without mercy". Please don't get me wrong.

1. Which features should be included: The original functionality
proposed by Jakob on the first mail was this:

- relative paths between resources (css files referencing images
without using #resource['..'])
- caching resources in the client (disabled if ProjectStage == Development)
- GZIP compression and local cache in tmp dir (disabled if
ProjectStage == Development)
- i18n (supporting country code and language).

The current list is this:

- GZIP compression and local cache in tmp dir.
- i18n (supporting country code and language).
- Prevent use #{resource['...']} on css files, using prefix mapping
- Prefix mapping automatic detection
- Override request path generation using EL expressions
- Redirect library names to prevent duplicate usage against other JSF libraries.

If you want to get rid a feature by whatever reason please send a vote
describing the reasons why that feature is not worth to include it and
should be removed on the next version.

The first thing we need to do is come to an agreement about how that
should looks like. As simple as that.

2. You have the impression that the original code provided by Jakob or
the code in apache extras works correctly. I doubt that. Probably it
works on simple cases but the code has 'holes' that can't be fixed
without take the code in myfaces core about resource handler and reuse
it. To support this point check this:

    @Override
    public Resource createResource(String resourceName, String
libraryName, String contentType)
    {
        FacesContext facesContext = FacesContext.getCurrentInstance();
        String requestedLocalePrefix = null;

        // check if libraryName is null and the current request is a
resource request
        // (FIXME this must be done in a better way when changing
JSF's own ResourceHandler in JSF 2.2)
        if (libraryName == null &&
facesContext.getApplication().getResourceHandler().isResourceRequest(facesContext))
        {
            // extract the libraryName and the locale from the resourceName
            // --> valid resource url: de/library/Name/resourceName.css

            // trim any slashes at begin or end
            resourceName = ResourceUtils.trimSlashes(resourceName);

Can you see what's wrong? Inclusive it has a note that says:

        // (FIXME this must be done in a better way when changing
JSF's own ResourceHandler in JSF 2.2)

The problem is the code try to extract the libraryName from
resourceName field. Why?, because the original proposal try to change
the way a resource request path is generated, and make use of a "side
effect" when that request path is interpreted by the default
ResourceHandler, which put everything on resourceName without decode
it.

If you want to do it right, and have control about what's going on it
is necessary to override ResourceHandler.handleResourceRequest()
method. That means reuse a lot of code that comes from the default
resourcehandler implementation and change it so libraryName and
resourceName can be decoded correctly and createResource() method can
be called with the right params !!!!!!.

As you can see, all metrics you mention doesn't say anything about
correctness, and that's the most important thing. This fact, makes all
previous conclusions you claim pure lies, that does not resist any
analysis.

3. ".... containing lots of functionality which we NEVER will need!
..." who says that? you? do you have any probe about that? Please be
more specific, otherwise it is not possible to take this kind of
comments seriously.

I think the best way to solve this is discuss on this thread point by
point which features should be excluded and the reasons behind that.
Then, when we have an agreement, I invite you to try to create a
resource handler that implement such features and then compare it with
the proposed implementation with the same features. In that way we
have a fair comparison and finally the community will benefit of such
effort to make things better.

regards,

Leonardo Uribe

2011/10/27 Mark Struberg <struberg@yahoo.de>:
>   Hi folks!
>
> I'm just comparing the original proposal of the resource handler from Jakob (which works
fine)
>
> http://code.google.com/a/apache-extras.org/p/relative-resource-handler/source/checkout
>
> and what we do have now in myfaces-commons-resource-handler
>
> https://svn.apache.org/repos/asf/myfaces/commons/trunk/myfaces-commons-resourcehandler
(has problems with multiple servlets - fixed that itmt)
>
>
> and to be honest - I'm freaking out a bit!
> Blowing up the code (with almost the same functionality) to 10 times the amount of code
and copying tons of code from our other projects over cannot be the right solution. Really,
please be honest and just compare the functionality
>
> original config parsing: 110 LOC, working well (java6 specific)
>
> new version: 20 classes with ~1800 LOC!
>
> And all that just to support java5? I cannot believe that! Even if I need to hack all
this myself (without any JAX parser or other utility), I don't need more than 200 LOC!
>
> -> if there is no really good explanation then lets throw the crap away.
>
>
> next stop:
>
>
> package org.apache.myfaces.commons.resourcehandler.resource;
>
> original: 3 classes, ~1000 LOC, working
>
> myfaces-commons-resource-handler: 13 classes, ~ 2400 LOC
>
> containing lots of functionality which we NEVER will need!
>
> But at least this code is undocumented (almost no single javadoc) and contains no unit
tests </sarcasm>
>
>
> I'm really in the mood to rollback this project and start if all over again...
>
>
> Really, please, the projects intention was to REPLACE the overloaded crap we have in
the JFS spec as resource handler with a LIGHT and powerful alternative. Thus this shall not
get bloated and filled with crap copied all over the place.
>
>
> LieGrue,
> strub
>
>

Mime
View raw message