HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

PDF Script Tool - adds JavaScripts to PDF files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptpdfjavascriptsfilesaddstool

Problem

This is a Windows Forms application that adds JavaScripts to PDF files. It supports multiple files at the same time. Right now, the only supported script is a "time-stamp on print" script, but any others I add in the future would work exactly the same in principle.

My company uses the "time-stamp on print" script for time-sensitive documents. We originally manually added the script to each individual PDF document, then switched to an Adobe Acrobat Pro batch process script. The licenses for Acrobat Pro are expensive, and it can be buggy/slow, so we are looking for a better solution. While I was working on that, figured I might as well make it a GUI app with room to expand for future needs (and open-source third-party use).

I tried a somewhat new method for limiting the length of lines in this project. In short, I inserted a line break before each parameter to a method. Let me know if the code is readable or not (because of the line breaking or any other reasons).

PdfScriptTool

```
using iTextSharp.text.exceptions;
using iTextSharp.text.pdf;
using iTextSharp.text.pdf.draw;
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;
using static PdfScriptTool.PdfScriptToolConstants;

namespace PdfScriptTool
{
internal partial class PdfScriptTool : Form, IProgress
{
#region Folders

private static string RootPath = Path.Combine(
Environment.GetFolderPath(
Environment.SpecialFolder.MyDocuments),
RootFolderName);

private static string ConfigurationPath = Path.Combine(
RootPath,
ConfigurationFolderName);

private static string OutputRootPath = Path.Combine(
RootPath,
OutputFolderName);

private static string ProcessingPath = Path.Combine(
RootPath,
ProcessingFolderName);

private static string TimeStampScriptPath = Path.Combine(
Configurat

Solution

It's time to get through separation of concerns.

Whatever you are trying to do with your PDFs, your form is not the place to do it.
At the very least you should move the functionality to a different class with static methods.

If you do that you will realise that TimeStampPdfs() will need two parameters.
One is the list of the files that you need to process, and the other is the component that does the report on IProgress interface,
in this case that is your form.

Continuing, will also realise that if sometime will want to use this class on web for instance you can not have any
MessageBox showing on TimeStampPdf, so we will just remove that try catch

lineSeparator and pdfContentByte are never being used there. Remove them.

Some folks like to write nested using statements without defining a new scope (increasing the indentation),
I learned with those folks and I now prefer that way.

I will not clutter my answer with a lot of code since you should be able to do most of the refactoring,
but I will still give an implementation of TimeStampPdfs

public static Task>  TimeStampPdfs(IEnumerable files, 
    IProgress progress) => Task.Run(() =>
{
    int i = 0;
    int count = files.Count();
    var list = new List();
    foreach (var file in files)
    {
        string outFile = file;
        if (!".pdf".Equals(Path.GetExtension(file),
             StringComparison.InvariantCultureIgnoreCase))
        {
            outFile = ConvertToPdf(file);
        }
        list.Add(outFile);
        TimeStampPdf(outFile);
        progress.Report(new ProgressReport
        {
            Total = count,
            CurrentCount = ++i
        });
    }
    return list;
});


The usage on the form should be something along the lines of:

private async void timeStampDocuments_Click(object sender, EventArgs e)
{
    var files = documentsView.CheckedItems.Cast().Select(o => o.ToString());
    try{
        await PerformTask(TimeStampPdfs(files));
    }catch(Exception e){
        //when you work with tasks you normally get AggregateExceptions
        //catch your exception according to its type 
        //at the moment you don't have a way to tell which file failed, I will leave that on your side
        MessageBox.Show(e.Message);
    }
    //my answer to your previous question also missed the opportunity to show the message here
    MessageBox.Show("Timestamped all files.");
}

Code Snippets

public static Task<IEnumerable<string>>  TimeStampPdfs(IEnumerable<string> files, 
    IProgress<ProgressReport> progress) => Task.Run(() =>
{
    int i = 0;
    int count = files.Count();
    var list = new List<string>();
    foreach (var file in files)
    {
        string outFile = file;
        if (!".pdf".Equals(Path.GetExtension(file),
             StringComparison.InvariantCultureIgnoreCase))
        {
            outFile = ConvertToPdf(file);
        }
        list.Add(outFile);
        TimeStampPdf(outFile);
        progress.Report(new ProgressReport
        {
            Total = count,
            CurrentCount = ++i
        });
    }
    return list;
});
private async void timeStampDocuments_Click(object sender, EventArgs e)
{
    var files = documentsView.CheckedItems.Cast<object>().Select(o => o.ToString());
    try{
        await PerformTask(TimeStampPdfs(files));
    }catch(Exception e){
        //when you work with tasks you normally get AggregateExceptions
        //catch your exception according to its type 
        //at the moment you don't have a way to tell which file failed, I will leave that on your side
        MessageBox.Show(e.Message);
    }
    //my answer to your previous question also missed the opportunity to show the message here
    MessageBox.Show("Timestamped all files.");
}

Context

StackExchange Code Review Q#129687, answer score: 4

Revisions (0)

No revisions yet.