PDF Stamper Method to Add Field and Script to PDF File

Question

This is related to my (unanswered) question here, but the code has changed considerably since I asked that question. I have a method that takes the filename of a PDF file and a Script object, and applies the Script to the PDF. If you are not familiar, PDFs support JavaScript to perform various tasks such as changing the content of documents, and scripts can be executed on various events such as on printing or saving.

A Script has a ScriptText, a ScriptField, and a ScriptTiming. The text is the JavaScript itself, the field is an object with a string Title and four integers representing the coordinates of the field on the document, and the timing is an enum value used to determine when the script should be executed.

The method to add the script to the PDF is as follows:

private void AddScriptToPdf(string filename, Script script)
{
    try
    {
        using (var pdfReader = new iTextSharp.text.pdf.PdfReader(filename))
        {
            using (var pdfStamper = new iTextSharp.text.pdf.PdfStamper(
                    pdfReader,
                    new System.IO.FileStream(
                        GetOutputPath(filename),
                        System.IO.FileMode.Create)))
            {
                var parentField
                    = iTextSharp.text.pdf.PdfFormField.CreateTextField(
                        pdfStamper.Writer,
                        false,
                        false,
                        0);
                parentField.FieldName = script.ScriptField.Title;
                for (var pageNumber = PdfFirstPageNumber;
                    pageNumber <= pdfReader.NumberOfPages;
                    pageNumber++)
                {
                    var pdfContentByte = pdfStamper.GetOverContent(
                        pageNumber);
                    var textField = new iTextSharp.text.pdf.TextField(
                            pdfStamper.Writer,
                            new iTextSharp.text.Rectangle(
                                script.ScriptField.TopLeftX,
                                script.ScriptField.TopLeftY,
                                script.ScriptField.BottomRightX,
                                script.ScriptField.BottomRightY),
                            null);
                    var childField = textField.GetTextField();
                    parentField.AddKid(childField);
                    childField.PlaceInPage = pageNumber;
                }
                pdfStamper.AddAnnotation(parentField, 1);
                var pdfAction
                    = iTextSharp.text.pdf.PdfAction.JavaScript(
                        script.ScriptText,
                        pdfStamper.Writer);
                iTextSharp.text.pdf.PdfName actionType = null;
                switch (script.ScriptTiming)
                {
                    case ScriptTiming.DidPrint:
                        actionType
                            = iTextSharp.text.pdf.PdfWriter.DID_PRINT;
                        break;

                    case ScriptTiming.DidSave:
                        actionType
                            = iTextSharp.text.pdf.PdfWriter.DID_SAVE;
                        break;

                    case ScriptTiming.WillPrint:
                        actionType
                            = iTextSharp.text.pdf.PdfWriter.WILL_PRINT;
                        break;

                    case ScriptTiming.WillSave:
                        actionType
                            = iTextSharp.text.pdf.PdfWriter.WILL_SAVE;
                        break;
                }
                pdfStamper.Writer.SetAdditionalAction(
                    actionType,
                    pdfAction);
            }
        }
    }
    catch (iTextSharp.text.exceptions.InvalidPdfException e)
    {
        ShowMessage(e.Message + filename);
    }
}

The method is long, but requires very little logic. It's pretty much step-by-step instructions. I know I could split it into several smaller methods, but considering they only ever run in sequence, I don't see the point of that. Is there any way I could actually make it better (or shorter)?

Answer 1

• There is an excessive amount of new line after parameter delimiter(,) and assignment operator(=). At the same time, there is no new line between logical blocks of the method. Together, it makes the code difficult to read when glancing over.

• When using two directly nested using, you can write it as the following to reduce the level of indentation :

  using (var pdfReader = new PdfReader(filename))
  using (var pdfStamper = new PdfStamper(pdfReader, new FileStream(GetOutputPath(filename), FileMode.Create)))
  {
      // ...
  }
  

• The switch can be replaced by a dictionary to remove the boilerplate breaks :

  var actionMapping = new Dictionary<ScriptTiming, PdfName>
  {
      [ScriptTiming.DidPrint] = PdfWriter.DID_PRINT,
      [ScriptTiming.DidSave] = PdfWriter.DID_SAVE,
      [ScriptTiming.WillPrint] = PdfWriter.WILL_PRINT,
      [ScriptTiming.WillSave] = PdfWriter.WILL_SAVE,
  };
  

• You have some very-very-very-lonnnng name due to lack of using namespace :

  new iTextSharp.text.Rectangle(...)
  iTextSharp.text.pdf.PdfFormField.CreateTextField(...)
  
  // which can be replaced by this
  new Rectangle(...)
  PdfFormField.CreateTextField(...)
  
  // if you add this line on the top of the file
  using iTextSharp.text;
  using iTextSharp.text.pdf;
  
  // if the compiler complains that Rectangle is ambiguous, use this instead
  //using Rectangle = iTextSharp.text.Rectangle;
  

---

Code :

using iTextSharp.text;
using iTextSharp.text.pdf;

---

private void AddScriptToPdf(string filename, Script script)
{
    try
    {
        using (var pdfReader = new PdfReader(filename))
        using (var pdfStamper = new PdfStamper(pdfReader, File.Create(GetOutputPath(filename))))
        {
            var parentField = PdfFormField.CreateTextField(pdfStamper.Writer, false, false, 0);
            parentField.FieldName = script.ScriptField.Title;

            /* Some space here*/
            for (var pageNumber = PdfFirstPageNumber; pageNumber <= pdfReader.NumberOfPages; pageNumber++)
            {
                //var pdfContentByte = pdfStamper.GetOverContent(pageNumber); // <-- as @Heslacher pointed out, this is never unsed
                var textField = new TextField(
                    pdfStamper.Writer,
                    new Rectangle(
                        script.ScriptField.TopLeftX,
                        script.ScriptField.TopLeftY,
                        script.ScriptField.BottomRightX,
                        script.ScriptField.BottomRightY),
                    null);

                var childField = textField.GetTextField();
                childField.PlaceInPage = pageNumber;
                parentField.AddKid(childField);
            }

            /* Optional: Some space here */
            pdfStamper.AddAnnotation(parentField, 1);

            /* Some more space here */
            var pdfAction = PdfAction.JavaScript(script.ScriptText, pdfStamper.Writer);
            var actionMapping = new Dictionary<ScriptTiming, PdfName>
            {
                [ScriptTiming.DidPrint] = PdfWriter.DID_PRINT,
                [ScriptTiming.DidSave] = PdfWriter.DID_SAVE,
                [ScriptTiming.WillPrint] = PdfWriter.WILL_PRINT,
                [ScriptTiming.WillSave] = PdfWriter.WILL_SAVE,
            };
            // if null needs to be passed for any other cases, use the commented lines instead
            pdfStamper.Writer.SetAdditionalAction(actionType, actionMapping[script.ScriptTiming]);
//            pdfStamper.Writer.SetAdditionalAction(actionType, actionMapping
//                .Where(x => x.Key == script.ScriptTiming)
//                .Select(x => x.Value)
//                .FirstOrDefault());
        }
    }
    catch (InvalidPdfException e)
    {
        ShowMessage(e.Message + filename);
    }
}

Answer 2

It seems that

var pdfContentByte = pdfStamper.GetOverContent(
                    pageNumber);  

is never used so you should get rid of it.

---

Although this code is always executed in sequence it is better to extract separate stuff to methods. This will help if you need to find a bug or if you are trying to refactor it.

In its current state that method is doing to much. It is constructing and adding the textfields (annotations) and is asigning the javascript functionality.

---

The readability of that method should be improved by adding vertical spaces(new lines). Using vertical space will help to group related code and makes it more structured.