1
\$\begingroup\$

Will you review the syntax, structure, and logic of my code. It can be tested using the main.

using iTextSharp.text.pdf;
using iTextSharp.text;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace AddCopyRightPDF
{
    class Program
    {
        static void Main(string[] args)
        {
            string PDFPath = "D:\\SamplePDF\\SamplePDF.PDF";
            string PDFPathUpdated = "D:\\SamplePDF\\SamplePDFUpdated.PDF";

            byte[] b = AddCopyrighttoPDF(PDFPath);


            System.IO.File.WriteAllBytes(PDFPathUpdated, b);

        }

        public static byte[] AddCopyrighttoPDF(string path)
        {
            byte[] b = null;
            string CopyRighText;
            try
            {

                CopyRighText = "Copyright © " + DateTime.Now.Year + " All Rights Reserved.";
                PdfStamper pdfStamper = null;
                PdfReader pdfReader = null;
                try
                {
                    using (MemoryStream ms = new MemoryStream())
                    {
                        pdfReader = new PdfReader(new RandomAccessFileOrArray(path), System.Text.ASCIIEncoding.UTF8.GetBytes("1111"));
                        pdfStamper = new PdfStamper(pdfReader, ms);
                        for (int i = 1; i <= pdfReader.NumberOfPages; i++)
                        {
                            if (i > 1)
                            {
                                Rectangle pageSizeWithRotation = pdfReader.GetPageSizeWithRotation(i);
                                PdfContentByte overContent = pdfStamper.GetOverContent(i);
                                overContent.BeginText();
                                BaseFont baseFont = BaseFont.CreateFont("Helvetica", "Cp1250", false);
                                overContent.SetFontAndSize(baseFont, 7F);
                                overContent.SetRGBColorFill(0, 0, 0);
                                float n2 = 15F;
                                float n3 = pageSizeWithRotation.Height - 10F;
                                overContent.ShowTextAligned(0, CopyRighText, n2, n3, 0F);
                                overContent.EndText();


                            }
                        }

                        pdfStamper.FormFlattening = true;
                        pdfStamper.Close();
                        b = ms.ToArray();

                        ms.Flush();
                        ms.Close();
                    }
                }
                catch
                {
                    b = null;
                }
                finally
                {
                    if (pdfReader != null) { pdfReader.Close(); }
                }
            }
            catch { b = null; }
            return b;

        }
    }
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

From top to bottom

Since AddCopyrighttoPDF() is a public method you should validate its argument path.

Variables should be named using camelCase casing and should be spelled correctly string CopyRighText; -> string copyrightText;

try
{

    CopyRighText = "Copyright © " + DateTime.Now.Year + " All Rights Reserved.";
    PdfStamper pdfStamper = null;
    PdfReader pdfReader = null;  

why are these 3 lines of code inside a try..catch block? Which exception could here happen? The try..cacth block can be removed.

You are using an using statement for the MemoryStream which is a good thing, but both PdfReader and PdfStamper are implementing the IDisposable interface as well hence they should be enclosed in an using block as well.

for (int i = 1; i <= pdfReader.NumberOfPages; i++)
{
    if (i > 1)
    {  

If the loop would start at int i = 2 the if block would become superflous which saves one indentation-level of thecode.

BaseFont baseFont should be created outside of the loop and reused.

float n2 = 15F; should be named better and should be a constant.

float n3 should be named better.

overContent.ShowTextAligned(0, CopyRighText, n2, n3, 0F); here 0F should be extracted into a constant as well.

Disposing of the MemoryStream, which happens when the end of the using block is reached will Flush() and Close() it as well.

Implementing the mentioned points will lead to

private const float copyrightFontHeight = 7F;
private const float copyrightHorizontalPosition = 15F;
private const float copyrightVerticalBorder = 10F;
private const float copyrightRotationNone = 0F;
public static byte[] AddCopyrighttoPDF(string path)
{
    if (string.IsNullOrEmpty(path)) { return null; }

    string copyrightText = "Copyright © " + DateTime.Now.Year + " All Rights Reserved.";

    try
    {
        BaseFont baseFont = BaseFont.CreateFont("Helvetica", "Cp1250", false);
        using (MemoryStream ms = new MemoryStream())
        using (PdfReader pdfReader = new PdfReader(new RandomAccessFileOrArray(path), System.Text.ASCIIEncoding.UTF8.GetBytes("1111")))
        using (PdfStamper = new PdfStamper(pdfReader, ms))
        {
            for (int i = 2; i <= pdfReader.NumberOfPages; i++)
            {
                Rectangle pageSizeWithRotation = pdfReader.GetPageSizeWithRotation(i);
                PdfContentByte overContent = pdfStamper.GetOverContent(i);
                overContent.BeginText();
                overContent.SetFontAndSize(baseFont, copyrightFontHeight);
                overContent.SetRGBColorFill(0, 0, 0);
                float copyrightVerticalPosition = pageSizeWithRotation.Height - copyrightVerticalBorder;
                overContent.ShowTextAligned(0, copyrightText, copyrightHorizontalPosition, copyrightVerticalPosition, copyrightRotationNone);
                overContent.EndText();
            }

            pdfStamper.FormFlattening = true;
            pdfStamper.Close();
            return ms.ToArray();
        }
    }
    catch { } //empty because if we just want to swallow the exception

    return null;
}  

This should behave exactly like your former code. Thats why its not throwing an ArguemntNullException if path == null and why its not throwing an ArgumentException if path is whitespace only.

I couldn't find the C# documentation of BaseFont but if BaseFont is implementing IDisposable as well, it should be enclosed in an using block as well.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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