Hi all,
I see this started a few days ago, but I just came back from a couple of
weeks of vacation (and a couple of months of being totally buried in
work), so before I go back to the office, I'll at least let you know I'm
still around and even reading the list now and then :-)
I also recognize some of my code....
J.Pietschmann wrote:
> Hello all,
> I just took a more extended look at CVS HEAD. There are
> some style issues which caught my eye.
> 1. I'd appreciate if indentation uses spaces instead of
> tabs. And because I can avoid using tabs, I expect
> everyone else avoiding tabs too.
> 2. I'd really, really appreciate if assignments were
> not made operands of other expressions, especially
> not operands of comparisions in if statements:
> if ((bp = getNextBreakPoss(childLC, null)) != null) {
> I'd make some exceptions for more commonly used idioms
> in while statements
> while ((curLM = getChildLM()) != null) {
> but IMO this still s*cks.
Probably comes from long years of writing C and C++ and avoiding extra
lines of code. Must be taste since I don't find it that bad, but if we
all vote it out in the style rules, I'll agree to banish it!
> 3. Exceptions should never, ever replace simpler flow
> controls:
> try {
> return super.nextChar();
> }
> catch (NoSuchElementException e) {
> if (bEndBoundary) {
> bEndBoundary=false;
> return CharUtilities.CODE_EOT;
> }
> else throw e;
> }
> Apart from the potential for confusion, throwing
> exceptions is an expensive operation and should really
> be reserved for rare and non-local exits. (In the
> particular case above, the discrepancy between hasNext()
> and getNext() seems to indicate a design bug that should
> be fixed rather than worked around).
Agreed, it would be better to recheck super.hasNext() rather than
letting it throw the exception. However, you'll note that the exception
is only propagated from this method in the case where it's hasNext()
returns false, which is the normal behavior for an Iterator.
> 4. Some identifiers make me a bit uneasy, like "BreakPoss".
> IMO random abbreviations should be avoided unless the
> identifiers become really long and unwieldy, at least
> in class names and preferably also in method names.
> Furthermore, I think abbreviations should be used
> consistently at least across class and method names:
> public void setStructHandler(StructureHandler sh) {
> public BreakPoss getBP() {
> protected Position getPos(Object nextObj) {
> Is it really worth to save a few characters this way?
> Also, capitalization should be consistent:
> class LMiter
> interface BPLayoutManager
Mea culpa. I quite agree with you that these names are not the best. All
I can say is that I was writing code in bits and pieces over a fairly
long period of time and wasn't paying enough attention to being
consistent with my BP (BreakPossibility) and BreakPosition (just
Position). I'm all for changing the names to something less confusing.
> 5. I don't think naming styles should be mixed without good
> (and preferably explained) reason:
> boolean m_bInited = false;
> Yuck!
> 6. There seems to be a drive to use hungarian notation.
> While I don't have a strong feelings for or against it,
> I'd like to note that in all projects I've watched such
> an efford was ultimately wasted. Usage universally
> degraded over time, and causal and inconsistent usage
> just makes the code look ugly.
Like most everyone else that's responded, I say, let's just pick a rule
that makes it clear what are member or static variables and which are
local and stick to it. Personally I tend to prefer m_xxx to this.xxx,
with or without hungarian.
...
> It would be nice to get some agreements nailed down and
> published.
>
> J.Pietschmann
>
>
All for that.
Regards,
Karen
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
