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;
}
}
}
1 Answer 1
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.