myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: [VOTE] technical and design code issues
Date Thu, 15 Nov 2012 01:24:23 GMT
Hi

1.) How many inner classes do we like to use?

As many as necessary to get the job done. There is already a
checkstyle rule in myfaces that limits the file size to 3000 lines.
Everything below the limit is valid!!!.

 2.) hashCode as key

I created an alternate vote to explain this point in deep. It is
something that depends on the context.

regards,

Leonardo Uribe

2012/11/14 Mike Kienenberger <mkienenb@gmail.com>:
>> 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