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

Reply via email to