myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Kienenberger <mkien...@gmail.com>
Subject Re: [VOTE] technical and design code issues
Date Wed, 14 Nov 2012 23:35:25 GMT
> 1.) How many inner classes do we like to use?

Well, your first item isn't a technical or design issue.   It's a style issue.

My preference is that the code be readable, and if these inner classes
are making it less readable, then they should be moved.

So as a general, but not absolute rule,   I haven't looked at the code
in question, so I am not saying you're right about it.

[X] Inner classes [should] only be used for private data holders and
similar helper classes. Max 2 in a class, not longer than 50 LOC each.


> 2.) hashCode as key

Obviously, we don't need votes on whether we want to write broken
code.    You have a specific issue in mind, and this is what we should
be considering.

I haven't closely followed the discussion, and I only have a few
minutes to spare tonight.  However....

Leonardo said that the viewid algorithm isn't relying on the hash
code, at which point you changed the topic to inner classes.

If it is relying on hashcode, and hashcode is not something we control
-- viewids are Strings, correct? -- then we should not assume that
hashcode is good enough.   Hashocode + equals needs to be used.

I agree with Leonardo that any time we can trim a few bytes off the
state, we need to try to do it.    Not every site cares about
encryption.   Or perhaps they are providing a custom encryption.



On Wed, Nov 14, 2012 at 5:34 PM, Mark Struberg <struberg@yahoo.de> wrote:
> Hi!
>
> While working on a few parts of the MyFaces codebase the last few times I saw increasingly
bad style imo. Some of it might be a personal taste, others is technically funded. Let's start
with the design parts
>
>
>
> 1.) How many inner classes do we like to use?
>  I found 10++ inner classes per class. For no whatever reason - just to make the code
unmaintainable it seems. One could argue with private access, but hey that's bollocks as everyone
can tweak private classes via reflections anyway. It' just stinks and is not maintainable
that way. Also this leads to bad design as it's not easy to see what classes we have in there.
In ServerSideStateCacheImpl there has been 10++ inner classes with 1000 LOC which might have
EASILY filled an own package!
>
> [ ] Inner classes must only be used for private data holders and similar helper classes.
Max 2 in a class, not longer than 50 LOC each.
>
> [ ] I don't care
> [ ] Oh yes, inner classes for the win, even if a class hits 5000 LOC because of that!
>
>
> 2.) hashCode as key
> a hashCode is NOT unique! Using it for a key which must be unique is just dumb.
>
> [ ] Use hashCode + original value for keys. First checking the hashCode and if it is
the same equals on the original value
> [ ] I don't care
> [ ] Yea, pleas gimme the full random key collission stuff. I like to get random errors
in production which are not reproducible - makes my days so sparkling!
>
>
> LieGrue,
> strub
>
> Oh, here is my vote:
> [X ] Inner classes must only be used for private data holders and similar helper classes.
Max 2 in a class, not longer than 50 LOC each.
> [X ] Use hashCode + original value for keys. First checking the hashCode and if it is
the same equals on the original value

Mime
View raw message