On 21/10/2011 15:13, Glenn Adams wrote:
Chris,

Hi Glenn,


I would really like to see an acknowledgement from Glenn that there are some imperfections that need addressing.

I wasn't aware I had given anyone the impression of presenting a perfect submission. In fact, one of my favorite quotes is Voltaire's /le mieux est l'ennemi du bien/ "the best [perfect] is the enemy of the good", which a former colleague Charlie Sandbank (now deceased) used to love to cite at least once a day in ITU committee meetings.

Many thanks for taking the time to write this long e-mail. I do appreciate it. I reached this impression because I see Vincent and Peter giving feedback but I've yet to see any of the suggestions actioned. It could be that some of their suggestions aren't appropriate, but I do believe some of them are good points.


My coding philosophy is by and large based on a step-wise refinement process:

(1) make it (some subset of possible features) work
(2) make sure its correct (and regression testable) - or if following BDD, then do this first (3) make it (more) understandable/maintainable, i.e., re-factor, improve comments, documentation, etc
(4) optionally, make it faster and smaller
(5) optionally, add more features
(6) go to (1)

Right now, I've finished steps (1) and (2) to a certain degree for an initial set of features, a degree that I think is sufficient to merit moving it into trunk. I have not yet seriously addressed (3) through (6). It would seem strange to expect that all these points have been addressed at this point in the process.

Thanks for clarifying. Just to be clear, I didn't mean to say that reaching the end of development was a pre-requisite to merging to trunk. It just seemed like a major milestone and therefore seemed like a suitable prompt for the opening of a discussion about our concerns.


If this work goes into the trunk, it will provide greater exposure to help drive and prioritize the remaining steps. There are trade-offs in time and money about how to spend my effort on steps (3) to (6). I have a well defined set of issues already waiting for item (5) [1], so, post merge, I can spend my time on these features or work on (3) or (4), or could attempt to divide my time between them. The bottom line is that it is a process that started some time ago and will continue for an indefinite time into the future. The current request for merging is just one step in that process.

That makes sense to me.


I expect to be maintaining this code and feature set for the indefinite future, according to the desires of my sponsor, Basis Technologies, who has a definite interest in the production use of an internationalized FOP, as well as others who have expressed a similar interest. As a consequence, there is little chance that any other FOP dev is going to have to work on this code any time soon. Of course, if they want to do so, I would certainly welcome community contributions to additional features, testing, optimization, documentation, etc., in this area. I have no personal need to *own* this code if others wish to help, and I certainly encourage it; however, at the same time, I do have a sponsor who is willing to continue investing in improving FOP in this area, and that willingness should not be discounted.

Since Thunderhead also needs this feature we are willing to invest some time into it too. Currently my team are telling me it would take 9 person months to port this code into our branch of FOP, partly because of some merge conflicts, but also partly because we are not comfortable with some aspects of the code as it has already been pointed out. The estimate would include the time to refactor long files into small ones, deal with the variable names and restructuring the code.


[1] http://skynav.trac.cvsdude.com/fop/report/1

Regarding the comments about line length, file length etc., I will note that, at least with line length, I have maintained the existing (but arbitrary) limit of 100 in existing files. In the case of new files, I have chosen to use a longer line length that works better for my development process. I use GNU EMACS on a wide-screen MacBook Pro that has an effective width of 200 columns, and which, when this limit is exceeded, wraps the line (on display only). I find that arbitrary line lengths like 100 curtail my coding style, and as I've been coding for 40+ years, it's a pretty well established style (though back in the days when I wrote Fortran IV using an IBM 026 card punch <http://en.wikipedia.org/wiki/IBM_026#IBM_024.2C_026_Card_Punches>, I had to stick with 80 columns). [If line length followed Moore's Law, we would be using lines of length 1760, starting from 1967 with 80 columns.]

I personally don't have a problem with line length, but I think the File length is a small issue that we would like to address. I think the code would be easier to maintain if the larger classes were broken into several smaller classes.

By the way, I would note that the FOP Coding Conventions [2] do not specify a maximum line length. In searching through the FOP DEV archives, I also don't see an explicit discussion about line length (though I may have missed it). What I do see is the initial introduction of the checkstyle target by Jeremias in 2002 [4], where it appears he basically adjusted the checkstyle rules to pass most of the extant code at that time.

[2] http://xmlgraphics.apache.org/fop/dev/conventions.html
[3] http://marc.info/?l=fop-dev&w=2&r=1&s=%2Bcheckstyle+%2Bline+%2Blength&q=b <http://marc.info/?l=fop-dev&w=2&r=1&s=%2Bcheckstyle+%2Bline+%2Blength&q=b> [4] http://marc.info/?l=fop-dev&m=103124169719869&w=2 <http://marc.info/?l=fop-dev&m=103124169719869&w=2>

My opinion about the various length limits (parameter count, method length, file length, line length) as reported by checkstyle is that these should interpreted as guidelines, and not hard rules. In the case of existing files, it makes sense to attempt to adhere to them when possible (and I have done this), while recognizing that some exceptions are inevitable. In the case of new files, I agree they are reasonable targets, but I would not readily agree to slavish adherence. Some latitude should be afforded to individual coders, particularly when they are adding a substantial amount of code in new files.

I think that is a fair statement, but breaking the rules should be the exception not the rule. Otherwise we need to review the rule as a community or consider another code structure.


Regarding identifier length, neither the FOP coding conventions [2] nor checkstyle indicates or checks for any kind of limitation; so I will respectfully decline to change my code based on other author's subjective notions of what is necessary or sufficient. If folks want to have a discussion about this in order to reach a consensus, then I would not object to adhering to a consensus that emerges; but IMO, there are more important things to do than hammer out such a consensus in an objectively verifiable rule. What I will agree to do is add (over a reasonable period of time) comments at the point of defining all static, instance, or local variables that adequately spell out the meaning of any 'short identifier' save for the obvious indexers, i, j, k, etc, and basic type variables (String s) and enumerators (incidentally, where would one draw the line with an objective rule?).

It's true that there is currently no rule about this in the coding conventions. I think the community should seriously consider adding such a rule. I think the reason for it absence is that no one used the short identifiers on mass before. I agree variables used as iterators are an exception to the rule; Peter, Vincent nor I are trying to suggest that i,j,k are not allowed as iterators. I think that is a well known convention. Here is an example of what we mean:

int gi = 0;

Where gi really means glyphIndex. In this case we believe the variable should be named "glyphIndex"

I appreciate your commitment to add comments to short identifiers declarations, so at least it will be easier for the team to translate the short variables to longer equivalents. Just so we are clear on what you propose, do you mean this:

int gi = 0; // Glyph Index

Thanks,

Chris


Regards,
Glenn

On Fri, Oct 21, 2011 at 5:50 PM, Chris Bowditch <bowditch_ch...@hotmail.com <mailto:bowditch_ch...@hotmail.com>> wrote:

    On 21/10/2011 09:36, Simon Pepping wrote:

    Hi Simon,

        I am pleased to learn that you are also in need of this new
        functionality.

        I share some of Vincent and Peter's concerns about technical
        points of
        the code. On the other hand, this is the only implementation of
        complex scripts we have, created by Glenn, in the style of
        Glenn. It
        is an initial implementation, and it is normal that it requires
        further work, maybe even design changes to make it more
        flexible. Does
        keeping it in a branch make that further work easier? Merging
        it into
        trunk will enhance its visibility, and make it available to more
        users.


    I'm not opposing the merge, I simply saw it as an appropriate
    milesone at which to open the debate on our concerns. It feels
    like we are making some progress here, so thanks for helping the
    debate along. I would really like to see an acknowledgement from
    Glenn that there are some imperfections that need addressing. If I
    saw that then I would give my full backing, but even without that
    I would vote +0 for the merge for the reasons you highlight above.

    Thanks,

    Chris



        Simon

        On Thu, Oct 20, 2011 at 02:02:10PM +0100, Chris Bowditch wrote:

            On 19/10/2011 19:32, Simon Pepping wrote:

            Hi Simon,

            I think you misunderstood my mail. I don't want to stop
            the merge. I
            simply thought it was an appropriate time to discuss some
            concerns
            that Vincent and Peter had identified. You are preaching
            to the
            converted about the need for supporting Complex scripts.
            It is an
            urgent requirement for us too.

            If we don't discuss our concerns over the code now, then
            when do we
            discuss it?

            Vincent and Peter will be replying to this thread shortly
            and will
            outline their primary concerns then.

            Thanks,

            Chris





Reply via email to