Hello zyx, my comments below.
On Mon, 6 Feb 2023 at 09:18, zyx <z...@gmx.us> wrote: > From my point of view the "base" PdfPainter > should do just what the PDF standard drawing does, to expose all those > APIs in 1:1 manner. After that the "extensions" (or better a > PafPainter, which derives from the PdfPanterBase - or some better name) > can add wrapper functions, which will build on top of the base, adding > higher level API for common operations. It felt like your > PdfPainterExtensions aims to. First let's clarify what PdfPainterExtensions is right now: really it's a bunch of quarantined functions that were once in PdfPainter and needs to be cleaned/reviewed (if it will ever happen) before being reintegrated in PdfPainter once again. Secondly: somehow I am now feeling intrigued by your proposal that I translate to "Let's have a base class/interface of PdfPanter with just methods that match PDF operators 1:1". I think it could be a very interesting idea to have for power users, if we ensure the inherited PdfPainter will still be easy to user for most users and performing the same validations, and still have convenience methods like it has now (for example to draw circles/ellipses/etc). Let me work on it on a separate branch and show it to you what I have in mind. > That the wrappers/contexts need to access private members of the > PdfPainter makes me feel like not a great API design. The usefulness of the helper classes PdfPainterPathContext and PdfPainterTextContext is really just to conceptually separate these functions from the others in PdfPainter because they need a Beging()/End() semantics plus the other reasons I explained in my first email and the friendship is in fact used only to directly call private functions with same signature in PdfPainter through conveniently named public member instances painter.Path and painter.Text. If we agree that helpers class can be used to perform such conceptual separation, and we move to use them this way, I don't think we can come up with a design that's more friendly/convenient from the API user perspective. > I'd still reconsider whether the subobjects and all those "AddSometing()" > methods are needed (the 'Add' prefix of the functions feels unnecessary > and awful to me). I believe there must be a distinction between DrawSomething() functions that actually perform some stroking and/or filling and just adding to a path. If you noticed, DrawSomething() are just in PdfPainter and AddSomething() is just in PdfPainterPathContext. This is a similar approach from .NET Graphics[1] and GraphicsPath[2] (but the latter don't have "prolong to" stateful methods). I think you may want to imagine having no Draw/Add prefix at all or you imagine just the "Draw" prefix in PdfPainter and no prefix at all in PdfPainterPathContext. That would be more confusing to me. > I'm missing PdfPainter::/PdfPainterPathContext::getCurrentPath(), it's > needed for some drawing operators. Return it back, please. > First I explain why I removed it: I was trying to avoid double caching the raw string stream being built. So I ask you: - Why you need to access a raw string stream with just the content of the path? Consider that logically the paths may stack up, even if in the PDF content stream semantics maybe they don't do it really. This opens further questions that I don't really want to answer; - Later we may consider adding to the API logical paths, meaning a class PdfGraphicsPath that has similar methods to PdfPainterPathContext but logically contains the path being built and that can be re-used, for example with a function PdfPainter::DrawPath(path). In this sense it would be very similar to NET GraphicsPath class[2]. Would this fit better your use case? - For now, would direct access to the global string stream as a backdoor to be configured during PdfPainter construction (eg. "PdfPainter painter(stream);" be enough for you? I'm thinking adding it as well as discussed this issue[3]. Or access to the interface with all PDF operators as discussed above. > By the way, what is the reason to repeat in the class definition that > some code is "private:", while you already declared that the current > declaration section is "private:"? Similarly with the "public:" > section. Unneeded redundancy in the code, is it not? > It's a stylistic choice I invented to help me keeping disciplined and organized. Few examples I separate private functions that's called just inside the class and private functions that can be called outside by means of friendship, or default/deleted members or inline getters/setters. In general I like to keep a discipline in the code, but it must be lightweight, so of course it's redundant but I believe it's not that terrible. That's the reason I use Hungarian notation like "m_" or "s_" prefixes for members or static variables but I don't like full Hungarian notation itself, which it's just too heavy/redundant. Cheers, Francesco [1] https://learn.microsoft.com/en-us/dotnet/api/system.drawing.graphics [2] https://learn.microsoft.com/en-us/dotnet/api/system.drawing.drawing2d.graphicspath [3] https://github.com/podofo/podofo/issues/41#issuecomment-1418281159 _______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
