5
\$\begingroup\$

This is an update from here. This tool is a Windows Form application that can add fields and scripts to PDF files, as well as convert Microsoft Office Word documents to PDF.

The usage is as follows:

  1. User opens program and presses "Select Files" button.
  2. A dialog opens allowing the user to select one or more Word documents or PDF files.
  3. The user presses a button to either add a field/script (for now, each field/script this tool supports has its own button) or convert to PDF only.
  4. The back end handles the processing. Any Word documents are first converted to PDF and placed in a "Processing" folder.
  5. If a field/script was selected, the appropriate field and script are added to the PDF files.
  6. All PDF files, including those in processing, are saved to an "Output" folder.

enter image description here

An overview of the classes:

  • Program.cs is the auto-generated Windows Form main file and opens the Windows Form. (Code omitted from this post)
  • PdfScript.cs is the Windows Form itself. It contains all the UI elements including button listeners, and passes commands to the back end.
  • PdfProcessor.cs is the back end. It does all the work, and reports progress to the front end.
  • Field.cs is a class to configure PDF fields.
  • ProgressReport.cs is a class to handle progress reporting in the form of a current, total, and percent.
  • Script.cs is a class to configure PDF scripts.

PdfScriptTool

namespace PdfScriptTool
{
    using System.Linq;
    using Action = System.Action;
    using DialogResult = System.Windows.Forms.DialogResult;
    using EventArgs = System.EventArgs;
    using Exception = System.Exception;
    using Form = System.Windows.Forms.Form;
    using Func = System.Func<System.Threading.Tasks.Task>;
    using IProgress = System.IProgress<ProgressReport>;
    using MessageBox = System.Windows.Forms.MessageBox;
    using Resources = Properties.Resources;
    using Task = System.Threading.Tasks.Task;

    /// <summary>
    /// The main application window.
    /// </summary>
    internal partial class PdfScriptTool : Form, IProgress
    {
        /// <summary>
        /// Whether or not files in the file view should be automatically
        /// checked (selected).
        /// </summary>
        private const bool FileViewFileIsChecked = true;

        /// <summary>
        /// Whether or not the open file dialog should allow selection of
        /// multiple files.
        /// </summary>
        private const bool OpenFileDialogAllowMultiple = true;

        /// <summary>
        /// The PDF Processor that does the back end work.
        /// </summary>
        private PdfProcessor pdfProcessor;

        /// <summary>
        /// Initializes a new instance of the <see cref="PdfScriptTool"/>
        /// class.
        /// </summary>
        internal PdfScriptTool()
        {
            InitializeComponent();
            InitializeOpenFileDialog();
            pdfProcessor = new PdfProcessor();
        }

        /// <summary>
        /// Reports the progress of the current task.
        /// </summary>
        /// <param name="progressReport">The progress report containing a
        /// current count, total count, and percent.</param>
        public void Report(ProgressReport progressReport)
        {
            if (InvokeRequired)
            {
                Invoke((Action)(() => Report(progressReport)));
            }
            else
            {
                progressBar.Value = progressReport.Percent;
            }
        }

        /// <summary>
        /// Performs a specified task in the backend.
        /// </summary>
        /// <param name="function">The task to perform.</param>
        /// <returns>The completed task.</returns>
        internal async Task PerformTask(Func function)
        {
            if (fileView.CheckedItems.Count > 0)
            {
                Enabled = false;
                try
                {
                    pdfProcessor.Files =
                        fileView.CheckedItems.OfType<string>().ToList();
                    await function();
                }
                catch (Exception e)
                {
                    ShowException(e);
                }

                ShowMessage(Resources.FilesSavedInMessage +
                    PdfProcessor.OutputRootPath);
                progressBar.Value = 0;
                Enabled = true;
            }
            else
            {
                ShowMessage(Resources.NoFilesSelectedErrorMessage);
            }
        }

        /// <summary>
        /// Shows an exception in a message box.
        /// </summary>
        /// <param name="e">The exception to show.</param>
        private static void ShowException(Exception e)
        {
            ShowMessage(e.Message);
        }

        /// <summary>
        /// Shows a message in a message box.
        /// </summary>
        /// <param name="message">The message to show.</param>
        private static void ShowMessage(string message)
        {
            MessageBox.Show(message);
        }

        /// <summary>
        /// Listener for the "Convert to PDF Only" button.
        /// </summary>
        /// <param name="sender">The object that triggered the event.</param>
        /// <param name="e">The event arguments.</param>
        private async void ConvertOnly_Click(object sender, EventArgs e)
        {
            await PerformTask(() => pdfProcessor.ProcessPdfs(this));
        }

        /// <summary>
        /// Sets attributes for the open file dialog.
        /// </summary>
        private void InitializeOpenFileDialog()
        {
            openFileDialog.Filter = Resources.OpenFileDialogFilter;
            openFileDialog.Multiselect = OpenFileDialogAllowMultiple;
            openFileDialog.Title = Resources.OpenFileDialogTitle;
        }

        /// <summary>
        /// Listener for the "Select Files" button. Shows the select files
        /// dialog and adds all selected files to the files view, locking each
        /// file to prevent editing until released.
        /// </summary>
        /// <param name="sender">The object that triggered the event.</param>
        /// <param name="e">The event arguments.</param>
        private void SelectFiles_Click(object sender, EventArgs e)
        {
            var dialogResult = openFileDialog.ShowDialog();
            if (dialogResult == DialogResult.OK)
            {
                foreach (var filename in openFileDialog.FileNames)
                {
                    fileView.Items.Add(filename, FileViewFileIsChecked);
                }
            }
        }

        /// <summary>
        /// Listener for the "Timestamp 24 Hours" button.
        /// </summary>
        /// <param name="sender">The object that triggered the event.</param>
        /// <param name="e">The event arguments.</param>
        private async void TimeStampDefaultDay_Click(
            object sender, EventArgs e)
        {
            await PerformTask(() => pdfProcessor.ProcessPdfs(
                this,
                Field.DefaultTimeStampField,
                Script.TimeStampOnPrintDefaultDayScript));
        }

        /// <summary>
        /// Listener for the "Timestamp 30 Days" button.
        /// </summary>
        /// <param name="sender">The object that triggered the event.</param>
        /// <param name="e">The event arguments.</param>
        private async void TimeStampDefaultMonth_Click(
            object sender, EventArgs e)
        {
            await PerformTask(() => pdfProcessor.ProcessPdfs(
                this,
                Field.DefaultTimeStampField,
                Script.TimeStampOnPrintDefaultMonthScript));
        }
    }
}

PdfProcessor

namespace PdfScriptTool
{
    using Application = Microsoft.Office.Interop.Word.Application;
    using Directory = System.IO.Directory;
    using Environment = System.Environment;
    using File = System.IO.File;
    using FileMode = System.IO.FileMode;
    using FileStream = System.IO.FileStream;
    using IProgress = System.IProgress<ProgressReport>;
    using List = System.Collections.Generic.List<string>;
    using Path = System.IO.Path;
    using PdfAction = iTextSharp.text.pdf.PdfAction;
    using PdfFormField = iTextSharp.text.pdf.PdfFormField;
    using PdfName = iTextSharp.text.pdf.PdfName;
    using PdfReader = iTextSharp.text.pdf.PdfReader;
    using PdfStamper = iTextSharp.text.pdf.PdfStamper;
    using PdfWriter = iTextSharp.text.pdf.PdfWriter;
    using Rectangle = iTextSharp.text.Rectangle;
    using Resources = Properties.Resources;
    using SpecialFolder = System.Environment.SpecialFolder;
    using StringComparison = System.StringComparison;
    using Task = System.Threading.Tasks.Task;
    using TextField = iTextSharp.text.pdf.TextField;
    using WdExportFormat = Microsoft.Office.Interop.Word.WdExportFormat;

    /// <summary>
    /// The back end of the PDF Script Tool.
    /// </summary>
    internal class PdfProcessor
    {
        /// <summary>
        /// Increment of two to skip a page.
        /// </summary>
        private const int EveryOtherPage = 2;

        /// <summary>
        /// Increment of one to get every page.
        /// </summary>
        private const int EveryPage = 1;

        /// <summary>
        /// Page one should be the first page.
        /// </summary>
        private const int FirstPageNumber = 1;

        /// <summary>
        /// Page two should be the first page.
        /// </summary>
        private const int SecondPageNumber = 2;

        /// <summary>
        /// Initializes a new instance of the <see cref="PdfProcessor"/> class.
        /// </summary>
        internal PdfProcessor()
        {
            Directory.CreateDirectory(RootPath);
            Directory.CreateDirectory(OutputRootPath);
            Directory.CreateDirectory(ProcessingPath);
        }

        /// <summary>
        /// Gets the folder output is stored in.
        /// "Output" in the root folder.
        /// </summary>
        internal static string OutputRootPath
        {
            get
            {
                return Path.Combine(RootPath, Resources.OutputFolderName);
            }
        }

        /// <summary>
        /// Gets the folder processing files are stored in.
        /// "Processing" in the root folder.
        /// </summary>
        internal static string ProcessingPath
        {
            get
            {
                return Path.Combine(RootPath, Resources.ProcessingFolderName);
            }
        }

        /// <summary>
        /// Gets the root folder the program works in.
        /// "PDF Script Tool" in the user's "My Documents" folder.
        /// </summary>
        internal static string RootPath
        {
            get
            {
                return Path.Combine(
                    Environment.GetFolderPath(SpecialFolder.MyDocuments),
                    Resources.RootFolderName);
            }
        }

        /// <summary>
        /// Gets or sets the list of files (paths) to be processed.
        /// </summary>
        internal List Files { get; set; }

        /// <summary>
        /// Adds a field and a script to the currently selected files.
        /// </summary>
        /// <param name="progress">
        /// The object to which progress is reported.
        /// </param>
        /// <param name="field"> The field to be added to the files.</param>
        /// <param name="script">The script to be added to the files.</param>
        /// <returns>The completed task.</returns>
        internal async Task ProcessPdfs(
            IProgress progress, Field field = null, Script script = null)
        {
            await Task.Run(() =>
            {
                for (int i = 0; i < Files.Count; i++)
                {
                    var currentFile = Files[i];
                    if (!IsPdf(currentFile))
                    {
                        currentFile = ConvertToPdf(currentFile);
                    }

                    if (field != null || script != null)
                    {
                        ProcessPdf(currentFile, field, script);
                    }
                    else
                    {
                        MovePdfToOutput(currentFile);
                    }

                    progress.Report(new ProgressReport
                    {
                        Total = Files.Count,
                        CurrentCount = i + 1
                    });
                }
            });
        }

        /// <summary>
        /// Adds a field to a page of a PDF document.
        /// </summary>
        /// <param name="field">The field to add.</param>
        /// <param name="pageNumber">
        /// The page number on which the field will be added.
        /// </param>
        /// <param name="pdfStamper">The PDF stamper for the document.</param>
        /// <param name="parentField">The parent field.</param>
        private static void AddFieldToPage(
            Field field,
            int pageNumber,
            PdfStamper pdfStamper,
            PdfFormField parentField)
        {
            var textField = new TextField(
                pdfStamper.Writer,
                new Rectangle(
                    field.TopLeftX,
                    field.TopLeftY,
                    field.BottomRightX,
                    field.BottomRightY),
                null);
            var childField = textField.GetTextField();
            parentField.AddKid(childField);
            childField.PlaceInPage = pageNumber;
        }

        /// <summary>
        /// Adds a field to a PDF document.
        /// </summary>
        /// <param name="field">The field to add.</param>
        /// <param name="pdfStamper">The PDF stamper for the document.</param>
        /// <param name="numberOfPages">
        /// The number of pages in the document.
        /// </param>
        private static void AddFieldToPdf(
            Field field, PdfStamper pdfStamper, int numberOfPages)
        {
            var parentField = PdfFormField.CreateTextField(
                pdfStamper.Writer, false, false, 0);
            parentField.FieldName = field.Title;
            int pageNumber = field.Pages == Pages.Last ?
                numberOfPages : FirstPageNumber;
            if (field.Pages == Pages.First || field.Pages == Pages.Last)
            {
                AddFieldToPage(
                    field,
                    pageNumber,
                    pdfStamper,
                    parentField);
            }
            else
            {
                int increment = field.Pages == Pages.All ?
                    EveryPage : EveryOtherPage;
                if (field.Pages == Pages.Even)
                {
                    pageNumber += 1;
                }

                for (; pageNumber <= numberOfPages; pageNumber += increment)
                {
                    AddFieldToPage(
                        field,
                        pageNumber,
                        pdfStamper,
                        parentField);
                }
            }

            pdfStamper.AddAnnotation(parentField, FirstPageNumber);
        }

        /// <summary>
        /// Adds a script to a PDF document.
        /// </summary>
        /// <param name="script">The script to add.</param>
        /// <param name="pdfStamper">The PDF stamper for the document.</param>
        private static void AddScriptToPdf(
            Script script, PdfStamper pdfStamper)
        {
            var pdfAction = PdfAction.JavaScript(
                script.ScriptText, pdfStamper.Writer);
            PdfName actionType = null;
            switch (script.ScriptEvent)
            {
                case ScriptEvent.DidPrint:
                    actionType = PdfWriter.DID_PRINT;
                    break;

                case ScriptEvent.DidSave:
                    actionType = PdfWriter.DID_SAVE;
                    break;

                case ScriptEvent.WillPrint:
                    actionType = PdfWriter.WILL_PRINT;
                    break;

                case ScriptEvent.WillSave:
                    actionType = PdfWriter.WILL_SAVE;
                    break;
            }

            pdfStamper.Writer.SetAdditionalAction(
                actionType, pdfAction);
        }

        /// <summary>
        /// Converts a file to a PDF document.
        /// </summary>
        /// <param name="filename">The path of the file to convert.</param>
        /// <returns>The path of the converted PDF document.</returns>
        private static string ConvertToPdf(string filename)
        {
            var outputFilename = Path.GetFileNameWithoutExtension(filename)
                + Resources.PdfFileExtension;
            var outputPath = Path.Combine(ProcessingPath, outputFilename);
            var wordApplication = new Application();
            var wordDocument = wordApplication.Documents.Open(filename);
            var exportFormat = WdExportFormat.wdExportFormatPDF;
            wordDocument.ExportAsFixedFormat(outputPath, exportFormat);
            wordDocument.Close(false);
            wordApplication.Quit();
            return outputPath;
        }

        /// <summary>
        /// Gets the output path for a specified input file.
        /// </summary>
        /// <param name="inputPath">The input file path.</param>
        /// <returns>The output path for the file.</returns>
        private static string GetOutputPath(string inputPath)
        {
            return Path.Combine(OutputRootPath, Path.GetFileName(inputPath));
        }

        /// <summary>
        /// Checks if a file is a PDF document.
        /// </summary>
        /// <param name="filename">The path of the file to check.</param>
        /// <returns>Whether or not the file is a PDF document.</returns>
        private static bool IsPdf(string filename)
        {
            return string.Equals(
                Path.GetExtension(filename),
                Resources.PdfFileExtension,
                StringComparison.InvariantCultureIgnoreCase);
        }

        /// <summary>
        /// Moves a PDF file to the output folder.
        /// </summary>
        /// <param name="filename">The PDF file to move.</param>
        private static void MovePdfToOutput(string filename)
        {
            File.Move(filename, GetOutputPath(filename));
        }

        /// <summary>
        /// Adds features to a PDF document.
        /// </summary>
        /// <param name="filename">The path of the PDF document.</param>
        /// <param name="field">The field to add.</param>
        /// <param name="script">The script to add.</param>
        private static void ProcessPdf(
            string filename, Field field, Script script)
        {
            using (var pdfReader = new PdfReader(filename))
            {
                using (var pdfStamper = new PdfStamper(
                    pdfReader,
                    new FileStream(GetOutputPath(filename), FileMode.Create)))
                {
                    if (field != null)
                    {
                        AddFieldToPdf(
                            field,
                            pdfStamper,
                            pdfReader.NumberOfPages);
                    }

                    if (script != null)
                    {
                        AddScriptToPdf(script, pdfStamper);
                    }
                }
            }
        }
    }
}

Field

namespace PdfScriptTool
{
    using static Properties.Resources;

    /// <summary>
    /// The allowed specifications for which pages a field can be placed on.
    /// </summary>
    internal enum Pages
    {
        /// <summary>
        /// All pages in the document.
        /// </summary>
        All,

        /// <summary>
        /// Odd pages in the document.
        /// </summary>
        Odd,

        /// <summary>
        /// Even pages in the document.
        /// </summary>
        Even,

        /// <summary>
        /// The first page of the document.
        /// </summary>
        First,

        /// <summary>
        /// The last page of the document.
        /// </summary>
        Last
    }

    /// <summary>
    /// Fields are used to design text fields for PDF files.
    /// </summary>
    internal class Field
    {
        /// <summary>
        /// The default field used for "time stamp on print" methods.
        /// </summary>
        internal static readonly Field DefaultTimeStampField
            = new Field(
                DefaultTimestampFieldTitle,
                DefaultTopLeftX,
                DefaultTopLeftY,
                DefaultBottomRightX,
                DefaultBottomRightY,
                Pages.All);

        /// <summary>
        /// The x coordinate of the default bottom right field corner.
        /// </summary>
        private const int DefaultBottomRightX = 576;

        /// <summary>
        /// The y coordinate of the default bottom right field corner.
        /// </summary>
        private const int DefaultBottomRightY = 756;

        /// <summary>
        /// The x coordinate of the default top left field corner.
        /// </summary>
        private const int DefaultTopLeftX = 36;

        /// <summary>
        /// The y coordinate of the default top left field corner.
        /// </summary>
        private const int DefaultTopLeftY = 792;

        /// <summary>
        /// The maximum y coordinate in points on a portrait-oriented PDF.
        /// Top edge of document.
        /// </summary>
        private const int TYMax = 792;

        /// <summary>
        /// The maximum x coordinate in points on a portrait-oriented PDF.
        /// Right edge of document.
        /// </summary>
        private const int XMax = 612;

        /// <summary>
        /// The minimum x coordinate in points on a PDF.
        /// Left edge of document.
        /// </summary>
        private const int XMin = 0;

        /// <summary>
        /// The minimum y coordinate in points on a PDF.
        /// Bottom edge of document.
        /// </summary>
        private const int YMin = 0;

        /// <summary>
        /// The four coordinates that comprise the top left and bottom right
        /// corners of the field.
        /// </summary>
        private int[] coordinates;

        /// <summary>
        /// Initializes a new instance of the <see cref="Field" /> class.
        /// </summary>
        /// <param name="title">
        /// The title of the field.
        /// </param>
        /// <param name="topLeftX">
        /// The top left corner x coordinate of the field.
        /// </param>
        /// <param name="topLeftY">
        /// The top left corner y coordinate of the field.
        /// </param>
        /// <param name="bottomRightX">
        /// The bottom right x coordinate of the field.
        /// </param>
        /// <param name="bottomRightY">
        /// The bottom right y coordinate of the field.
        /// </param>
        /// <param name="pages">The pages the field should be placed on.
        /// </param>
        internal Field(
            string title,
            int topLeftX,
            int topLeftY,
            int bottomRightX,
            int bottomRightY,
            Pages pages)
        {
            Title = title;
            coordinates = new int[4];
            coordinates[0] = topLeftX;
            coordinates[1] = topLeftY;
            coordinates[2] = bottomRightX;
            coordinates[3] = bottomRightY;
            Pages = pages;
        }

        /// <summary>
        /// Gets the bottom right corner x coordinate of the field.
        /// </summary>
        internal int BottomRightX
        {
            get
            {
                return coordinates[2];
            }
        }

        /// <summary>
        /// Gets the bottom right corner y coordinate of the field.
        /// </summary>
        internal int BottomRightY
        {
            get
            {
                return coordinates[3];
            }
        }

        /// <summary>
        /// Gets or sets the pages the field should be placed on.
        /// </summary>
        internal Pages Pages { get; set; }

        /// <summary>
        /// Gets or sets the title of the field.
        /// </summary>
        internal string Title { get; set; }

        /// <summary>
        /// Gets the top left corner x coordinate of the field.
        /// </summary>
        internal int TopLeftX
        {
            get
            {
                return coordinates[0];
            }
        }

        /// <summary>
        /// Gets the top left corner y coordinate of the field.
        /// </summary>
        internal int TopLeftY
        {
            get
            {
                return coordinates[1];
            }
        }
    }
}

ProgressReport

namespace PdfScriptTool
{
    /// <summary>
    /// Allows for reporting progress as both a percent and a count.
    /// </summary>
    internal class ProgressReport
    {
        /// <summary>
        /// Converts a decimal number to a percent value.
        /// </summary>
        private const int PercentMultiplier = 100;

        /// <summary>
        /// Gets or sets the number of tasks completed.
        /// </summary>
        internal int CurrentCount { get; set; }

        /// <summary>
        /// Gets the percent of tasks completed.
        /// </summary>
        internal int Percent
        {
            get
            {
                return PercentMultiplier * CurrentCount / Total;
            }
        }

        /// <summary>
        /// Gets or sets the total number of tasks.
        /// </summary>
        internal int Total { get; set; }
    }
}

Script.cs

namespace PdfScriptTool
{
    using static Properties.Resources;

    /// <summary>
    /// The event that triggers the execution of the script.
    /// </summary>
    internal enum ScriptEvent
    {
        /// <summary>
        /// The document is preparing to print.
        /// </summary>
        WillPrint,

        /// <summary>
        /// The document is preparing to save.
        /// </summary>
        WillSave,

        /// <summary>
        /// The document was printed.
        /// </summary>
        DidPrint,

        /// <summary>
        /// The document was saved.
        /// </summary>
        DidSave
    }

    /// <summary>
    /// Scripts define JavaScript text that can be added to a PDF file.
    /// </summary>
    internal class Script
    {
        /// <summary>
        /// The default "time stamp on print" script, valid for a day.
        /// </summary>
        internal static readonly Script TimeStampOnPrintDefaultDayScript =
            new Script(TimeStampOnPrintDefaultDay, ScriptEvent.WillPrint);

        /// <summary>
        /// The default "time stamp on print" script, valid for a month.
        /// </summary>
        internal static readonly Script TimeStampOnPrintDefaultMonthScript =
            new Script(TimeStampOnPrintDefaultMonth, ScriptEvent.WillPrint);

        /// <summary>
        /// Initializes a new instance of the <see cref="Script"/> class.
        /// </summary>
        /// <param name="scriptText">The JavaScript text of the script.</param>
        /// <param name="scriptEvent">The event that triggers the execution of
        /// the script.</param>
        internal Script(string scriptText, ScriptEvent scriptEvent)
        {
            ScriptText = scriptText;
            ScriptEvent = scriptEvent;
        }

        /// <summary>
        /// Gets or sets the JavaScript text of the script.
        /// </summary>
        internal string ScriptText { get; set; }

        /// <summary>
        /// Gets or sets the event that triggers the execution of the script.
        /// </summary>
        internal ScriptEvent ScriptEvent { get; set; }
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ This question has been mentioned on Meta \$\endgroup\$
    – Malachi
    Jun 6, 2016 at 12:32
  • \$\begingroup\$ Michael, preemptive perhaps, but remember not to take criticism too harshly. I wish I could get a summer intern who already codes as well as you. You'll be a fine dev someday. \$\endgroup\$
    – RubberDuck
    Jun 6, 2016 at 16:09
  • 1
    \$\begingroup\$ @RubberDuck Thank you for your compliment. I hope I have not seemed to take any criticism too harshly. I understand my skills are very limited (my C# knowledge is entirely self/Stack Exchange taught) and I appreciate all of the comments and criticisms. On a side note, if you really do need an intern for next summer... ;) \$\endgroup\$ Jun 6, 2016 at 16:15

3 Answers 3

8
\$\begingroup\$
using Application = Microsoft.Office.Interop.Word.Application;
using Directory = System.IO.Directory;
using Environment = System.Environment;
using File = System.IO.File;
using FileMode = System.IO.FileMode;
using FileStream = System.IO.FileStream;
using IProgress = System.IProgress<ProgressReport>;
using List = System.Collections.Generic.List<string>;
using Path = System.IO.Path;
using PdfAction = iTextSharp.text.pdf.PdfAction;
using PdfFormField = iTextSharp.text.pdf.PdfFormField;
using PdfName = iTextSharp.text.pdf.PdfName;
using PdfReader = iTextSharp.text.pdf.PdfReader;
using PdfStamper = iTextSharp.text.pdf.PdfStamper;
using PdfWriter = iTextSharp.text.pdf.PdfWriter;
using Rectangle = iTextSharp.text.Rectangle;
using Resources = Properties.Resources;
using SpecialFolder = System.Environment.SpecialFolder;
using StringComparison = System.StringComparison;
using Task = System.Threading.Tasks.Task;
using TextField = iTextSharp.text.pdf.TextField;
using WdExportFormat = Microsoft.Office.Interop.Word.WdExportFormat;

Boy, that's a lot of using statements. A number of things don't look right here:

  • Why have them within the namespace scope instead of at the very top of the code file, where every single C# developer is used to see them? That's where every VS project templates puts them, too. StyleCop? StyleCop is wrong, if you have name clashes then you're doing something wrong, reducing the scope of using statements amounts to putting lipstick on a pig - the problem would be bad naming, not the placement of using statements. Scott Hanselman blogged about this in 2008.
  • using List = System.Collections.Generic.List<string>; - that one is actually outright harmful.
  • Why define an alias for every single type you're using? Seems to me your using block could be reduced to this:

    using System;
    using System.IO;
    using System.Collections.Generic;
    using System.Threading.Task;
    using Microsoft.Office.Interop.Word;
    using Properties;
    using iTextSharp.text.pdf;
    

Aliasing every single type you're using buys you exactly nothing, and adds to the mental load without having read even a single line of actual code - one needs to remember that List is actually a List<string>, that IProgress is actually an IProgress<ProgressReport>; these aliases are harmful, because they hide the actual type and make the reader work harder than necessary to truly understand what's going on. I suppose the rest of the code doesn't use implicit typing (var). Let's see...


These shouldn't be constants:

    /// <summary>
    /// Whether or not files in the file view should be automatically
    /// checked (selected).
    /// </summary>
    private const bool FileViewFileIsChecked = true;

    /// <summary>
    /// Whether or not the open file dialog should allow selection of
    /// multiple files.
    /// </summary>
    private const bool OpenFileDialogAllowMultiple = true;

They're configuration settings, not constants whose value should be burned into the call sites at compilation. Good thing they're private, so changing their value isn't going to require recompiling anything that depends on it. But still configuration settings belong in a configuration settings file, not as const values in your code. If you really want them in code, then make them private static readonly. Making them const is abusing the semantic meaning of "constant".

Also, it appears resources are being misused as well:

    private void InitializeOpenFileDialog()
    {
        openFileDialog.Filter = Resources.OpenFileDialogFilter;
        openFileDialog.Multiselect = OpenFileDialogAllowMultiple;
        openFileDialog.Title = Resources.OpenFileDialogTitle;
    }

A file dialog filter isn't a resource string; you wouldn't have a different filter if your app ran with a different Culture setting - again that would be configuration if anything... but then it's not like *.pdf is going to change anytime soon: the extension of a .PDF file (assuming that's what the filter is) is much closer to the semantics of a private const than what you used private const for.


Missed opportunity here:

    /// <summary>
    /// The PDF Processor that does the back end work.
    /// </summary>
    private PdfProcessor pdfProcessor;

    /// <summary>
    /// Initializes a new instance of the <see cref="PdfScriptTool"/>
    /// class.
    /// </summary>
    internal PdfScriptTool()
    {
        InitializeComponent();
        InitializeOpenFileDialog();
        pdfProcessor = new PdfProcessor();
    }

The field is only ever assigned in the constructor and should be readonly. And then you just painted yourself in a corner where the UI is going to run the show and new up its dependencies - if PdfProcessor has its own dependencies and you want to decouple them, you'll need the form to create them here.

This is a "Smart UI" [anti-]pattern. Oh, it's great for prototyping. But not so much for extensible production code.

You need to give the Program.cs (i.e. the app's entry point) back the control of its wiring, get the application logic out of the form's code, and into a class that's responsible for coordinating between the view, the model and the presentation - the Model-View-Presenter pattern (MVP) plays very well with apps, and allows you to decouple your components and invert the dependency chain so that you control each components' dependencies from the outside, which in turn allows you to properly test each component.


This is dangerous:

    /// <summary>
    /// Initializes a new instance of the <see cref="PdfProcessor"/> class.
    /// </summary>
    internal PdfProcessor()
    {
        Directory.CreateDirectory(RootPath);
        Directory.CreateDirectory(OutputRootPath);
        Directory.CreateDirectory(ProcessingPath);
    }

You're making I/O work inside a constructor. And that constructor is called in your UI's constructor, which means by merely instantiating a form, you've [tried to] created 3 folders. Talk about surprising side-effects!

\$\endgroup\$
8
  • \$\begingroup\$ If I changed List to ListString would that make it better? xD Anyway, I have had discussions with others about the using directives before. In a nutshell, I think it is better to specify exactly what class is being used. I used to fully qualify them in the body of the code itself (I still do that for C++), but I got fussed at on here for doing that, and so I switched to aliasing. \$\endgroup\$ Jun 6, 2016 at 14:40
  • 1
    \$\begingroup\$ The compiled IL code is rigorously identical (see the link to Scott Hanselman's blog), and these aliases are hurting readability. There's a reason multiple reviewers point out the using statements... \$\endgroup\$ Jun 6, 2016 at 14:43
  • 3
    \$\begingroup\$ Also what's up with the casing of iTextSharp.text.pdf? Is that a 3rd-party namespace? A good alias for it would be ITextSharp.Text.Pdf. \$\endgroup\$ Jun 6, 2016 at 14:45
  • 1
    \$\begingroup\$ .net namespaces are PascalCase, the "i" implies an interface on a type, not on a namespace. If your using statements involved namespaces and not types, it wouldn't be confusing at all. If these folders are a prerequesite for the application to run, then they should be created before the form is instantiated, by whoever instantiates the form. Not sure how possible that is without inverting the dependency chain and stop having a UI that runs the show. Or their presence could be verified when they are accessed, and created if they're not present. But I/O work in a ctor is a big no-no ;-) \$\endgroup\$ Jun 6, 2016 at 14:52
  • 1
    \$\begingroup\$ lol! You can use named parameters instead of declaring constants: DoSomething(isFoo:true, isBar:false); - avoiding magic values is an excellent kind of OCD to have (gosh, I think I might have the same affliction!), but named parameters are a better solution IMO. \$\endgroup\$ Jun 6, 2016 at 15:03
6
\$\begingroup\$
    internal async Task ProcessPdfs(
        IProgress progress, Field field = null, Script script = null)
    {
        await Task.Run(() =>

This method looks like it runs asynchronously, but it doesn't. By awaiting the Action in Task.Run(), you're essentially blocking. Your ProcessPdfs method returns a Task and so does Task.Run(), so you could just return Task.Run() instead of awaiting it.

\$\endgroup\$
0
3
\$\begingroup\$

One thing that I am certain that everyone is thinking the first time they look at this code is that you have too many summary comments.

/// <summary>
/// The allowed specifications for which pages a field can be placed on.
/// </summary>
internal enum Pages
{
    /// <summary>
    /// All pages in the document.
    /// </summary>
    All,

    /// <summary>
    /// Odd pages in the document.
    /// </summary>
    Odd,

    /// <summary>
    /// Even pages in the document.
    /// </summary>
    Even,

    /// <summary>
    /// The first page of the document.
    /// </summary>
    First,

    /// <summary>
    /// The last page of the document.
    /// </summary>
    Last
}

Just write it like this

internal enum Pages
{
    All,
    Odd,
    Even,
    First,
    Last
}

I think that this code is pretty straight forward and that anyone that looks through this piece of code or anywhere that this enum is used is going to know exactly what this enum is being used for.

Save all the typing and unnecessary clutter.

I think that a single summary comment for the Pages enum should be sufficient to document what this element of the code is doing.

You mentioned, in a comment to this post, that you are using StyleCop and from what you commented it is unclear whether or not you are being forced to use StyleCop. One option might be to not use StyleCop and instead use something else like ReSharper instead, which is what VS2015 recommends using, it doesn't conflict with the IDE's Styling and doesn't require extraneous comment blocks that can become monotonous even to the point of actually ignoring them when you read through the code. Another option is to suppress the warnings requiring the extraneous comment blocks or change some of the rules so that they better fit your needs in the code.

My first thought to your comment

StyleCop mandates all of the summary comments I incorporated in the code.

Was that you are only doing it because StyleCop says so, and not because it is mandated by some higher power (like your development team or your boss), if you are using StyleCop just to make sure that your code is clean you can actually change the rules or suppress some of the warnings that are not helpful.

How to suppress a StyleCop warning?

\$\endgroup\$
7
  • 1
    \$\begingroup\$ StyleCop mandates all of the summary comments I incorporated in the code. \$\endgroup\$ Jun 3, 2016 at 13:35
  • 2
    \$\begingroup\$ you should mention that you are using StyleCop in your post. this is a valid review of your code. personally if I had to use a summary comment it would go above the declaration and not above every single element of the enumeration. I just don't think that you need all those summary comments, they are just not needed, maybe you should change the rules or suppress those warnings \$\endgroup\$
    – Malachi
    Jun 3, 2016 at 13:46
  • 3
    \$\begingroup\$ +1 Comments that dont add value should be avoided because they just decrease redability. \$\endgroup\$
    – JanDotNet
    Jun 3, 2016 at 13:50
  • 3
    \$\begingroup\$ FWIW I feel that most of OP's comments are good ones, but the ones pointed out in this answer are not. +1 \$\endgroup\$
    – RubberDuck
    Jun 6, 2016 at 12:59
  • 1
    \$\begingroup\$ +1 for suppressing superfluous warnings. We had a real scenario recently where we weren't awaiting something which caused runtime errors. This was flagged up as a warning in VS2015, but we didn't see it because we had 2,000 StyleCop warnings hiding it, even though our application was actually following .NET standard guidelines in terms of style. Treat all warnings as errors. \$\endgroup\$
    – Dan
    Jun 6, 2016 at 13:13

Your Answer

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

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