https://issues.apache.org/bugzilla/show_bug.cgi?id=46905





--- Comment #35 from Vincent Hennebert <vhenneb...@gmail.com>  2009-07-07 
04:31:23 PST ---
Hi Andreas,

(In reply to comment #32)
> (In reply to comment #31)
> 
> Hi Vincent,
> 
<snip/>
> > And a bit of nit-picking:
> > - in BlockStackingLM: in the getKeep*Property methods, I chose to throw
> > IllegalStateExceptions because the only LMs that don't override those 
> > methods
> > are LMs to which keeps don't apply. So calling those methods on such LMs is 
> > a
> > genuine programming mistake, and not a TODONotYetImplementedException.
> 
> Initially, I had pushed the getKeep*Property() method up to the LayoutManager
> interface, and wanted to use a similar pattern as some standard Java
> interfaces. The subclass can choose to implement it, but if it does not, it is
> allowed to signal this with an UnsupportedOperationException. It would indeed
> be a mistake, just like it is a mistake to call remove() on an arbitrary
> Iterator, because remove() is an optional operation. A concrete iterator is 
> not
> obliged to implement it, and if it doesn't, it should throw an exception.
> Whether it's an IllegalStateException or an UnsupportedOperationException is
> really all the same to me. Both are unchecked runtime exceptions. Just found
> the latter more appropriate...

That's two different things. remove() is semantically correct on an Iterator;
the fact that some iterators don't support it really is an implementation
limitation, and UnsupportedOperationException is applicable here. In the case
of LayoutManager, getKeep*Property shouldn't even be defined in that interface,
since not all descendants accept keep properties. For example, keeps make no
sense on an fo:static-content element. Calling the keep methods on its
corresponding StaticContentLayoutManager therefore is an error in the logic,
not an implementation limitation issue.

Actually, those methods shouldn't even be declared on those layout managers for
which they are not applicable. That way it wouldn't even be possible to make a
logic error. Unfortunately that implies changes in the class hierarchy that are
beyond the scope of this patch.


<snip/>
> > - in PageBreakingAlgorithm.createFootnotePages: tmpLength can be declared
> > inside the while loop
> 
> No idea whether it's still relevant, but I always tend to avoid stuff like:
> 
> while (someCondition) {
>   int intVar = intValue;
>   ...
> }
> 
> Instead, use:
> 
> int intVar = -1;
> while (someCondition) {
>   intVar = intValue;
>   ...
> }
> 
> which, I guess, is almost equivalent to:
> 
> while (someCondition) {
>   final int intVar = intValue;
>   ...
> }
> 
> apart from the fact that the variable is available outside of the loop.
> IOW: loop only the assignment, not the declaration. There really is no reason
> to declare (=allocate space for) a new variable on every iteration. Maybe 
> using
> the final modifier would work here too, since we don't need the variable 
> scoped
> outside of the loop...

The 'declaration' only applies at compilation time, and is used to perform type
checking. At runtime there is no space allocated whatsoever. A value is simply
pushed onto the operand stack [1]. Actually, declaring the variable outside the
loop results into more boilerplate bytecode, so technically is less efficient
(although very probably unnoticeable). OTOH, declaring the variable inside the
loop is cleaner and safer (no risk to mistakenly use it outside the loop).

[1]
http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#28851


<snip/>

Vincent

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to