3
\$\begingroup\$

I wrote some code to put stamps on all pages of a PDF file with the name and company passed as parameters. It works as intended, but as I am a beginner in Java, I am sure there are some best practices that I am not following.

I'm using the iText library.

Main.java:

public class Main {

public static void main(String[] args) throws IOException, DocumentException {

    Helper.sortposition(args[0], args[1], args[2], args[3], args[4]);

}
}

Helper.java:

public class Helper {

public static void sortposition(String name, String company, String originalpdf, String newpdf, String report) throws IOException {
    Properties prop = new Properties();
    InputStream in = null;
    String hxStr, hyStr, fxStr, fyStr, fhStr, ffStr;
    Float hx, hy, fx, fy, fh, ff;
        /*  hx= Header "x" position
            hy= Header "y" position
            fx= Footer "x" position
            fy= Footer "y" position
            fh= Font size on the Header (Font Header)
            ff= Font size on the Footer (Font Footer) */

    String colour;

    try {
        in = Helper.class.getResourceAsStream("files/config.properties");
        prop.load(in);

        switch (report) {
            case "1":
                fhStr = prop.getProperty("fh1");
                ffStr = prop.getProperty("ff1");
                colour = prop.getProperty("type1rgb");
                hxStr = prop.getProperty("type1hx");
                hyStr = prop.getProperty("type1hy");
                fxStr = prop.getProperty("type1fx");
                fyStr = prop.getProperty("type1fy");
                fh = Float.parseFloat(fhStr);
                ff = Float.parseFloat(ffStr);
                hx = Float.parseFloat(hxStr);
                hy = Float.parseFloat(hyStr);
                fx = Float.parseFloat(fxStr);
                fy = Float.parseFloat(fyStr);
                break;
            case "2":
                fhStr = prop.getProperty("fh2");
                ffStr = prop.getProperty("ff2");
                colour = prop.getProperty("type2rgb");
                hxStr = prop.getProperty("type2hx");
                hyStr = prop.getProperty("type2hy");
                fxStr = prop.getProperty("type2fx");
                fyStr = prop.getProperty("type2fy");
                fh = Float.parseFloat(fhStr);
                ff = Float.parseFloat(ffStr);
                hx = Float.parseFloat(hxStr);
                hy = Float.parseFloat(hyStr);
                fx = Float.parseFloat(fxStr);
                fy = Float.parseFloat(fyStr);
                break;
            case "3":
                fhStr = prop.getProperty("fh3");
                ffStr = prop.getProperty("ff3");
                colour = prop.getProperty("type3rgb");
                hxStr = prop.getProperty("type3hx");
                hyStr = prop.getProperty("type3hy");
                fxStr = prop.getProperty("type3fx");
                fyStr = prop.getProperty("type3fy");
                fh = Float.parseFloat(fhStr);
                ff = Float.parseFloat(ffStr);
                hx = Float.parseFloat(hxStr);
                hy = Float.parseFloat(hyStr);
                fx = Float.parseFloat(fxStr);
                fy = Float.parseFloat(fyStr);
                break;
            case "4":
                fhStr = prop.getProperty("fh4");
                ffStr = prop.getProperty("ff4");
                colour = prop.getProperty("type4rgb");
                hxStr = prop.getProperty("type4hx");
                hyStr = prop.getProperty("type4hy");
                fxStr = prop.getProperty("type4fx");
                fyStr = prop.getProperty("type4fy");
                fh = Float.parseFloat(fhStr);
                ff = Float.parseFloat(ffStr);
                hx = Float.parseFloat(hxStr);
                hy = Float.parseFloat(hyStr);
                fx = Float.parseFloat(fxStr);
                fy = Float.parseFloat(fyStr);
                break;
            default:
                fhStr = prop.getProperty("fh5");
                ffStr = prop.getProperty("ff5");
                colour = prop.getProperty("type5rgb");
                hxStr = prop.getProperty("type5hx");
                hyStr = prop.getProperty("type5hy");
                fxStr = prop.getProperty("type5fx");
                fyStr = prop.getProperty("type5fy");
                fh = Float.parseFloat(fhStr);
                ff = Float.parseFloat(ffStr);
                hx = Float.parseFloat(hxStr);
                hy = Float.parseFloat(hyStr);
                fx = Float.parseFloat(fxStr);
                fy = Float.parseFloat(fyStr);
        }

        String delims = "[,]+";
        String[] colourstr = colour.split(delims);
        int[] rgb = new int[colourstr.length];
        for (int i = 0; i < colourstr.length; ) {
            rgb[i] = Integer.parseInt(colourstr[i]);
            i++;
            Stamper.putStamp(name, company, originalpdf, newpdf, rgb, hx, hy, fx, fy, fh, ff);
        }

    } catch (IOException e) {
        e.printStackTrace();
    } finally {
        if (in != null) {
            try {
                in.close();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}
}

Stamper.java:

public class Stamper {

public static void putStamp(String name, String company, String originalpdf, String newpdf, int[] rgb, Float hx, Float hy, Float fx, Float fy, Float fh, Float ff) {
    PdfReader pdfReader;
    PdfStamper pdfStamper = null;
    Properties prop = new Properties();
    InputStream in = null;

    try {
        in = Helper.class.getResourceAsStream("files/config.properties");
        prop.load(in);

        pdfReader = new PdfReader(originalpdf);
        pdfStamper = new PdfStamper(pdfReader,
                new FileOutputStream(newpdf));

        Font font = FontFactory.getFont("files/LTe50327.ttf",
                BaseFont.IDENTITY_H, BaseFont.EMBEDDED, 0.8f, Font.BOLD);
        BaseFont bf = font.getBaseFont();

        int pages = pdfReader.getNumberOfPages();
        for (int i = 1; i <= pages; i++) {
            PdfContentByte pageContentByte = pdfStamper.getOverContent(i);
            pageContentByte.beginText();
            pageContentByte.setFontAndSize(bf, fh);
            pageContentByte.setRGBColorFill(rgb[0], rgb[1], rgb[2]);
            pageContentByte.setTextMatrix(hx, hy);
            pageContentByte.showText("This report belongs to " + name
                    + " from " + company);
            pageContentByte.endText();
        }

        for (int i = 1; i <= pages; i++) {
            PdfContentByte pageContentByte = pdfStamper.getOverContent(i);
            pageContentByte.beginText();
            pageContentByte.setFontAndSize(bf, ff);
            pageContentByte.setRGBColorFill(rgb[0], rgb[1], rgb[2]);
            pageContentByte.setTextMatrix(fx, fy);
            pageContentByte.showText("This report belongs to " + name
                    + " from " + company);
            pageContentByte.endText();
        }

    } catch (IOException e) {
        System.out.println("File does not exist");
        e.printStackTrace();
    } catch (DocumentException e) {
        e.printStackTrace();
    } finally {
        try {
            if (pdfStamper != null) {
                pdfStamper.close();
            }
            if (in != null) {
                in.close();
            }
        } catch (DocumentException | IOException e) {
            e.printStackTrace();
        }
    }
}
}
\$\endgroup\$
3
\$\begingroup\$

[I'll update my answer until it is done.]

Good start and welcome to Java programming!

General observations

You have a lot of repeating code. This is an indicator that something can be abstracted away, put in a method or other solution to prevent repeating yourself. Don't Repeat Yourself. The main reason to prevent repetition is that repetition leads to mistakes (like forgetting to edit one repitition, etc)

Main

  • You should check the arguments, and print some message when they are not ok.
  • Don't just throw error from main, catch them and handle them.

Helper

  • Give your variables sensible names; so you can read them and know what they do.
  • Use native types whenever possible, so please user float instead of Float.
  • Decide what to do if the float's don't parse.
  • You can re-write your switch loop to a method that gets the report as parameter.
  • You set a lot of variables that belong together, just to pass them around. You can put those in a class. As I do not yet get what your variables do, I'd call it StamperProperties. It would look somewhat like this:

Here:

class StamperProperties
{
    String hxStr, hyStr, fxStr, fyStr, fhStr, ffStr;
    float hx, hy, fx, fy, fh, ff;
}

I would also make this StamperProperties be able to load itself from the config.

You can then pass an instance of these properties to your stamper, which will be:

StamperProperties stamperProperties = new StamperProperties();
stamperProperties.loadFromConfig(...)

Stamper.putStamp(name, company, originalpdf, newpdf, rgb, stamperProperties)

Stamper

To prevent repetition, you can use methods. For example:

for (int i = 1; i <= pages; i++) {
    PdfContentByte pageContentByte = pdfStamper.getOverContent(i);
    String waterMark = "This report belongs to " + name
            + " from " + company;
    putWaterMark(waterMark, hx, hy);
    putWaterMark(waterMark, fx, fy);

}

public void putWaterMark(PdfContentByte pageContentByte, String text, float x, float y)
{
    pageContentByte.beginText();
    pageContentByte.setFontAndSize(bf, fh);
    pageContentByte.setRGBColorFill(rgb[0], rgb[1], rgb[2]);
    pageContentByte.setTextMatrix(x, y);
    pageContentByte.showText(text);
    pageContentByte.endText();

}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your answer! I will have a look at what you said and implement it on my code, will let you know how it worked out! Thank you once again! Those were very good points! \$\endgroup\$ – Teio Sep 9 '15 at 13:44
3
\$\begingroup\$

Admittedly, I am unfamiliar with the iText library in Java. However, I think it would be good practice to include comments in your code. For example, it isn't clear to me what Float hx, hy, fx, fy, fh, ff are being used for.

But, keep up the good work. You started to do a good job with try/catch for error checking, but you could do better if you handled errors in main by printing messages corresponding to what went wrong. For example:

if(args.length() < 5){
    System.err.println("Not enough input arguments");

}else{
    Helper.sortposition(args[0], args[1], args[2], args[3], args[4]);
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for you answer! The comments for the Float variables are in the config.properties file that I didn't include here. Perhaps it would a good idea to include it in the code too! As a reference (I'll include OP too): hx= Header "x" position hy= Header "y" position fx= Footer "x" position fy=Footer "y" position fh= Font size on the Header (Font Header) ff= Font size on the Footer (Font Footer) About the error handling on the main, that was a good one too, one which I have implemented and it's working! Thanks again! \$\endgroup\$ – Teio Sep 8 '15 at 15:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.