Hello Dominik,

may I commit the patch to be considered for the release candidate?
Until which time of day will changes/patches be accepted for the
latter? I'd like to provide a fix for the PdfOutlineItem crash also,
and for podofocolor passing through unsupported colours, instead of
throwing an exception and losing info about them from the input.
I'm thankfully looking forward to your answer in time.

Best regards, mabri
 
> Matthew Brincke <ma...@mailbox.org> wrote on 26 January 2018 at 18:08:
> 
> 
> Hello zyx, hello all,
> 
> > zyx <z...@gmx.us> has written on 14 January 2018 at 11:40:
> > 
> > 
> > On Thu, 2018-01-04 at 00:59 +0100, Matthew Brincke wrote:
> > > > the internal function name PODOFO_Base14FontDef_FindBuiltinData
> > > > looks like it should be in a namespace and a class IMHO, therefore
> > > > I've converted it to a private method with access only from
> > > > PdfFontFactory.
> > 
> >     Hi,
> > I'm sorry, but what is the idea behind the change, please? If only the
> > above, then, well, from my point of view, it doesn't make much sense. I
> > see that it makes many things more complicated, defines classes inside
> 
> no, that was only my original idea, before I knew how to do it in C++.
> The reasons why I've made it so "complicated" are that IMHO it's important
> to isolate an internal "function" (static method, so "private" applies) as
> much as possible (a tenet of OOP AFAIK) and the C++ syntax rules. I've left
> off PODOFO_LOCAL only because that's (probably by far) not not the only
> location to insert it and that such a change would be advisable later anyway
> (by Mattia Rizzolo [1]).
> 
> > classes (while possible, I do not like it myself), requirement of
> > 'friend' classes, which also kind of points into a bad/suboptimal API
> 
> Without the "friend" declarations I'd have a design with much less isolation:
> One "friend function" declaration was there before, but C++ forbids them for
> private methods, so I would have had to either make the method public (not
> acceptable for me because:
> - that would've added to the API whereas I'd like to keep it internal and
> - I had hoped that it would be easier to get a not-to-API change committed
> - (PODOFO_Base14FontDef_FindBuiltinData isn't in the PoDoFo API, I hope?),
> - I have doubts it should be public because of OOP and its implementation
> - because of these reasons I couldn't get rid of that "friend" declaration)
> 
> or change it to declare the whole class PdfFontFactory a "friend" which
> would be worse because AFAIK the best design with a "friend" is (the) one
> which makes the breach to isolation minimal, that is, (in this case) which
> gives access to the minimum number of methods (one in this case). I didn't
> want to have the new internal class needed for this to be at namespace level
> because I think that is for public API classes only.
> The "friend PdfFontFactory" declaration in there (nested class Builtin) is
> necessary because otherwise the FindMetrics() method would need to be public
> (C++ gives free access from nested to outer class, not the other way around).
> 
> The other nested class is required to avoid giving the (public API?) class
> PdfFontCache access to the internal Base14 font metrics ("needs to know"
> principle). The "friend" declaration there is give access only to the class
> which needs it (PdfFontCache, restricting it to method level would've been
> more complicated still, so much that I ruled it inelegant and avoided it).
> 
> > design, and also:
> > 
> > > > Should someone desire access to any of this info outside PoDoFo,
> > > > some workaround is possible: e.g. create the font, query its
> > > > metrics and check if they can be converted (dynamic_cast) to
> > > > PdfFontMetricsBase14.
> > 
> > which is quite discouraging for me.
> 
> Why is it discouraging for you, please?
> I only think of it as an interim workaround before API will be introduced
> to handle Base14 font metrics better, that is, without giving access (to
> manipulate, even, with PdfFont::GetFontMetrics2(), at least a copy)
> to the internal values. At that time, it should be deprecated IMHO. 
> 
> > 
> > Just my opinion. If the maintainer will wish to commit the change, then
> > I'm not against it, I'm just not in favor of it myself.
> 
> I'd like to discuss a roadmap to API changes with Dominik, project leader,
> anyway, do you mean him? I thank you in advance for a not-put-off answer.
> 
> >     Bye,
> >     zyx
> > 
> 
> Best regards, mabri
> 
> [1] https://sourceforge.net/p/podofo/mailman/message/36112580/ (last sentence)
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Podofo-users mailing list
> Podofo-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/podofo-users

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to