Skip to content

Commit 8d67c42

Browse files
Review checked exceptions softening in layout module
1 parent 5fc4165 commit 8d67c42

File tree

4 files changed

+50
-0
lines changed

4 files changed

+50
-0
lines changed

layout/findbugs-filter.xml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,34 @@
3232
<Or>
3333
<And>
3434
<Class name="com.itextpdf.layout.font.FontProvider"/>
35+
<!--
36+
FontProvider is usually used in highlevel API, which requests fonts in deep underlying logic.
37+
IOException would mean that font is chosen and it is supposed to exist, however it cannot be read.
38+
Using fallbacks in such situations would make FontProvider less intuitive.
39+
40+
Even though softening of checked exceptions can be handled at higher levels in order to let
41+
the caller of this method know that font creation failed, we prefer to avoid bloating highlevel API
42+
and avoid making higher level code depend on low-level code because of the exceptions handling.
43+
-->
3544
<Method name="getPdfFont" params="com.itextpdf.layout.font.FontInfo, com.itextpdf.layout.font.FontSet" returns="com.itextpdf.kernel.font.PdfFont"/>
3645
</And>
3746
<And>
47+
<!--
48+
Here creating of specific XMLReader is requested for processing patterns in hyphenation.
49+
If it fails, hyphenation processing cannot be recovered.
50+
-->
3851
<Class name="com.itextpdf.layout.hyphenation.PatternParser"/>
3952
<Method name="createParser"/>
4053
</And>
4154
<And>
4255
<Class name="com.itextpdf.layout.renderer.TypographyUtils"/>
4356
<Or>
57+
<!--
58+
If typography utils throws an exception at this point, we consider it as unrecoverable situation for
59+
its callers (layouting methods). Presence of typography module in class path is checked before.
60+
It's might be more suitable to wrap checked exceptions at a bit higher level, but we do it here for
61+
the sake of convenience.
62+
-->
4463
<Method name="callConstructor"/>
4564
<Method name="callMethod" params="java.lang.String, java.lang.String, java.lang.Object, java.lang.Class[], java.lang.Object[]" returns="java.lang.Object"/>
4665
</Or>

layout/src/main/java/com/itextpdf/layout/font/FontProvider.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,15 @@ public PdfFont getPdfFont(FontInfo fontInfo, FontSet tempFonts) {
353353
pdfFont = PdfFontFactory.createFont(fontProgram, encoding, getDefaultEmbeddingFlag());
354354

355355
} catch (IOException e) {
356+
// Converting checked exceptions to unchecked RuntimeException (java-specific comment).
357+
//
358+
// FontProvider is usually used in highlevel API, which requests fonts in deep underlying logic.
359+
// IOException would mean that font is chosen and it is supposed to exist, however it cannot be read.
360+
// Using fallbacks in such situations would make FontProvider less intuitive.
361+
//
362+
// Even though softening of checked exceptions can be handled at higher levels in order to let
363+
// the caller of this method know that font creation failed, we prefer to avoid bloating highlevel API
364+
// and avoid making higher level code depend on low-level code because of the exceptions handling.
356365
throw new PdfException(PdfException.IoExceptionWhileCreatingFont, e);
357366
}
358367

layout/src/main/java/com/itextpdf/layout/hyphenation/PatternParser.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ static XMLReader createParser() {
119119
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
120120
return factory.newSAXParser().getXMLReader();
121121
} catch (Exception e) {
122+
// Converting checked exceptions to unchecked RuntimeException (java-specific comment).
123+
//
124+
// Here creating of specific XMLReader is requested for processing patterns in hyphenation.
125+
// If it fails, hyphenation processing cannot be recovered.
122126
throw new RuntimeException("Couldn't create XMLReader: " + e.getMessage());
123127
}
124128
}

layout/src/main/java/com/itextpdf/layout/renderer/TypographyUtils.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,15 @@ private static Object callMethod(String className, String methodName, Object tar
266266
} catch (IllegalArgumentException e) {
267267
logger.warn(MessageFormatUtil.format("Illegal arguments passed to {0}#{1} method call: {2}", className, methodName, e.getMessage()));
268268
} catch (Exception e) {
269+
// Converting checked exceptions to unchecked RuntimeException (java-specific comment).
270+
//
271+
// If typography utils throws an exception at this point, we consider it as unrecoverable situation for
272+
// its callers (layouting methods). Presence of typography module in class path is checked before.
273+
// It's might be more suitable to wrap checked exceptions at a bit higher level, but we do it here for
274+
// the sake of convenience.
275+
//
276+
// The RuntimeException exception is used instead of, for example, PdfException, because failure here is
277+
// unexpected and is not connected to PDF documents processing.
269278
throw new RuntimeException(e.toString(), e);
270279
}
271280
return null;
@@ -280,6 +289,15 @@ private static Object callConstructor(String className, Class[] parameterTypes,
280289
} catch (ClassNotFoundException e) {
281290
logger.warn(MessageFormatUtil.format("Cannot find class {0}", className));
282291
} catch (Exception exc) {
292+
// Converting checked exceptions to unchecked RuntimeException (java-specific comment).
293+
//
294+
// If typography utils throws an exception at this point, we consider it as unrecoverable situation for
295+
// its callers (layouting methods). Presence of typography module in class path is checked before.
296+
// It's might be more suitable to wrap checked exceptions at a bit higher level, but we do it here for
297+
// the sake of convenience.
298+
//
299+
// The RuntimeException exception is used instead of, for example, PdfException, because failure here is
300+
// unexpected and is not connected to PDF documents processing.
283301
throw new RuntimeException(exc.toString(), exc);
284302
}
285303
return null;

0 commit comments

Comments
 (0)