RE: Java Programming Basics

[Dridi Seifeddine]

Who is that author? I hope you’re not referring to me Glenn. Well I came from a 
C/C++ background, and I confess that my Java knowledge is a little bit rusty 
but I’m always learning, faster than you may expect :)

 

If you look at my latest patch for whitespace management, you’ll see that I 
never do these mistakes.

 

All the best.

 

Seifeddine

 

P.S. Ignore this email if it was destined to someone else.

 

De : Glenn Adams [mailto:gl...@skynav.com] 
Envoyé : mardi 29 octobre 2013 15:02
À : FOP Developers
Objet : Java Programming Basics

 

I've been noticing code recently that makes it clear the author was a C/C++ 
programmer unfamiliar with Java. Whenever I see these things, I am fixing them 
on the spot:

 

(1) initializers are not required for class or instance members when their 
initial values are null, false, 0, etc;

 

e.g.

 

public class Foo {

  private Bar bar = null;

  private boolean done = false;

  private int value = 0;

}

 

should be written as:

 

public class Foo {

  private Bar bar;

  private boolean done;

  private int value;

}

 

(2) it is never necessary to invoke super() in a constructor, e.g., super() in 
the following is redundant:

 

public class Foo {

  public Foo() {

super();

  }

}

 

(3) it is never necessary to define a default constructor if there is no other 
defined constructor and it does nothing; e.g.,

 

public class Foo {

  public Foo() {

  }

}

 

should not define a default constructor; Java will always supply a default 
constructor if no other constructor is defined;

 

(4) however, if you want to prevent the generation of a default, public 
constructor, then you can define a private no-argument constructor:

 

public class CantInstantiate {

  private CantInstantiate() {

  }

}

RE: [jira] [Commented] (FOP-2293) Whitespace management extension

[Dridi Seifeddine]

I don’t believe so. Should I sign one and send it to the Apache administration?

 

De : Glenn Adams [mailto:gl...@skynav.com] 
Envoyé : jeudi 7 novembre 2013 22:36
À : FOP Developers
Objet : Re: [jira] [Commented] (FOP-2293) Whitespace management extension

 

Is there an CLA on file for Seiffidine?

 

On Thu, Nov 7, 2013 at 2:11 PM, Vincent Hennebert (JIRA) j...@apache.org 
wrote:


[ 
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel
 
https://issues.apache.org/jira/browse/FOP-2293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13816427#comment-13816427
 focusedCommentId=13816427#comment-13816427 ]

Vincent Hennebert commented on FOP-2293:


Hi Seifeddine,

thanks for your patch. I’ve applied it in [rev. 
1539809|http://svn.apache.org/r1539809]. I have the following questions and 
comments:

* FOPropertyMapping: The fox:fitting-strategy property is no longer needed is 
it? Same for Alternative.FittingStrategy.
* BestFit: same here?
* MultiCase.isActuated: not sure what you have in mind here? A multi-case 
cannot be actuated? Only a multi-toggle can. This method is used in 
MultiSwitch.finalizeNode, but I don’t get the point of that latter method?
* MultiToggle: why bother implementing it? AFAIU you won’t need it in your 
extension. If you implement it you have to test it...

I didn’t include the test case as I think it still needs a bit of work:
* I wouldn’t qualify nested multi-switch elements as simple test. Maybe you 
want to concentrate on the basics first and handle advanced use cases later.
* Instead of using space-after to artificially grow the size of a multi-case, 
you probably want to use a series of block elements. Space is subject to 
element resolution (See SpaceResolver.resolveElementList), and may disappear as 
the result of that. At any rate, it should in the present case since the space 
is conditional and would end a reference-area.
  All that to say: better use plain blocks and leave complex content for later, 
once the basics are working.
* It would be good if the test case were reflecting the choice of a variant at 
the bottom of a page. In this test the multi-switch elements are found in the 
middle of the page, which AFAIU you haven’t implemented yet.
* Please remove tabs from XML files and use a two-space indentation: tabs are 
as troublesome in XML as they are in Java.

When running the test case I noticed that the following warning is still 
issued: “The following feature isn’t implemented by Apache FOP, yet: 
fo:multi-switch”. You may want to fix that.

I noticed you tried a different approach at managing variants, by detecting 
page overflows and restarting the breaking algorithm at an earlier node after 
having discarded the current variant and moved on to the next one. That seems 
like a good idea, but the code is deceptively simple and I think you will get 
in trouble as you move foward in the implementation:
* Imagine the following test case: let’s call E the element just before the 
multi-switch, and assume that there is a legal break after that element. But 
that legal break results into a tooShortNode. Also, the first variant of the 
multi-switch leads to a tooLongNode, but the second variant perfectly fits on 
the page. When you are at the multi-switch, you will have a tooLongNode, the 
number of active nodes will drop to zero, which triggers the node recovery 
mechanism. Since there is a tooShortNode available, the algorithm will pick it 
(it’s better to have not enough content on a page than overflowing content that 
may be clipped). So the final break will be at element E, and the user will 
wonder why FOP broke the page there while there was enough room on the page to 
fit the second variant.
* Even if you find a fix for the preceding case, restarting at the node before 
every time a variant must be skipped is sub-optimal and creates unnecessary 
work for the breaking algorithm.
* The node recovery mechanism is particularly tricky and hard to understand, so 
fiddling with it is not advised unless there is no other option...

I suggest you carry on with the approach you had in your previous patches, 
which I think was integrating better with the algorithm, and in the end will 
lead to more robust and simpler code. You ‘just’ have to pass information to 
the KnuthPageNode about which variant of the multi-switch is associated to it, 
and all the way down the line until you are back in MultiSwitchLM.

Feel free to ask if you have any questions.

Thanks,
Vincent


 Whitespace management extension
 ---

 Key: FOP-2293
 URL: https://issues.apache.org/jira/browse/FOP-2293
 Project: Fop
  Issue Type: New Feature
  Components: general
Affects Versions: trunk
Reporter: Seifeddine Dridi
Priority: Minor
   

Unicode line breaking issues

[Dridi Seifeddine]

Hello,

 

In table 2 of the Unicode Line Breaking Algorithm, we see the following rule
NU ^ SY, which is a prohibited break. In other words, no break should occur
between a digit and a solidus ('/'). However, if we consider an example like
this: fo:page-number//fo:page-number, the NU ^ SY rule gets violated
since the text layout manager for the character '/' will have no access to
the type of content surrounding it (e.g. layout management is done
separately for each FO child). I wonder if you people are aware of this
issue, and if yes is there any workaround besides using the
keep-together.within-line property?

 

Thanks

 

Seifeddine