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 <[email protected]>: >> 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 <[email protected]> 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
