[
https://issues.apache.org/jira/browse/PDFBOX-2576?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14519875#comment-14519875
]
Tilman Hausherr commented on PDFBOX-2576:
-----------------------------------------
[~jahewson] I felt that it was more readable after the change, as the new code
shows that there are three possibilities to get the encoding: from font, from
dictionary and from name. (I began to understand what this code was doing only
after the refactoring!) The current method doesn't fit in my 24" screen - are
you using a tiny font and/or is your screen in portrait position? The method
has 72 lines!
But you're right about "state-mutating", I should have returned an encoding in
the two new methods and not set "encoding".
The rest is a matter of opinion / work-habit, i.e. you prefer to have one huge,
fat method that does all. I know what you feel because I have many such methods
in my own closed-source software. These huge methods may be good for the
author, but not for outside people who want to understand what happens. The
more complicated our code is, the less outside people will dare to debug it and
submit patches.
Anyway, I'll just leave it unchanged, but I won't discuss such things first
(this would take too much time, and the wait for an answer is annoying), I'll
rather revert or accept reverts without much discussion (i.e. unlike this
time), and will probably avoid splitting code you did (even if there is no such
thing as "code ownership").
> Improve code quality
> --------------------
>
> Key: PDFBOX-2576
> URL: https://issues.apache.org/jira/browse/PDFBOX-2576
> Project: PDFBox
> Issue Type: Task
> Affects Versions: 2.0.0
> Reporter: Tilman Hausherr
> Attachments: ExtractText.2.patch, ExtractText.patch,
> GraphicsOperatorProcessor.patch, SecuryHandlerFactory.patch,
> Type5ShadingContext.patch, examples.arrayclone.patch,
> fontbox.arrayclone.patch, org.apache.fontbox.afm.patch,
> org.apache.fontbox.cff.cffparser.patch, org.apache.fontbox.cff.patch,
> org.apache.fontbox.cmap.patch,
> org.apache.pdfbox.contentstream.operator.state.patch,
> org.apache.pdfbox.cos.patch, org.apache.pdfbox.filter-2.patch,
> org.apache.pdfbox.filter.patch, org.apache.pdfbox.pdfwriter.COSWriter.patch,
> org.apache.pdfbox.pdmodel.documentinterchange.logicalstructure.patch,
> org.apache.pdfbox.pdmodel.documentinterchange.patch,
> org.apache.pdfbox.preflight.graphic.patch, org.apache.pdfbox.resource.patch,
> org.apache.pdfbox.text.testtextstripper.patch, pdfbox-override-patch.txt,
> pdfbox-raw-type-patch.txt, pdfbox.arrayclone.patch,
> pdfcloneutility-patch.txt, pdftextstripperbyarea-patch.txt,
> ttfsubsetter-2.patch, ttfsubsetter-3.patch, ttfsubsetter-patch.txt
>
>
> This is a longterm issue for the task to improve code quality, by using the
> [SonarQube
> report|https://analysis.apache.org/dashboard/index/org.apache.pdfbox:pdfbox-reactor],
> hints in different IDEs, the FindBugs tool and other code quality tools.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pdfbox.apache.org
For additional commands, e-mail: dev-h...@pdfbox.apache.org
