Get Word Doc from SQL table, save temp file, edit that file, save new doc in a different table

Question

I have Frankensteined the crap out of this method and just wanted to make sure of two things.

1. I'd like to make sure that this is the best route to take. If it isn't, I would welcome with open arms an alternative.

2. I don't typically use SQL in the code behind. I always use it in the markup. So I just need to make sure there are no redundancies, or SQL injections.

Any help would be greatly appreciated. So here is Frankenstein himself:

protected void Button1_Click(object sender, EventArgs e)
{
    Byte[] byt = null;
    string query = "SELECT Letter FROM LetterTemplates WHERE LetterName = '[REDACTED].docx'";
    SqlCommand cmd = new SqlCommand(query);
    String strConnString = System.Configuration.ConfigurationManager.ConnectionStrings["[REDACTED]ConnectionString"].ConnectionString;
    SqlConnection con = new SqlConnection(strConnString);
    cmd.Connection = con;
    con.Open();
    SqlDataReader dr = cmd.ExecuteReader();
    while(dr.Read())
    {
        byt = (byte[])dr["Letter"];
    }
    if(byt != null)
    {
        FileStream fs = new FileStream("C:\\Temp\\[REDACTED].docx", FileMode.OpenOrCreate, FileAccess.Write);
        BinaryWriter br = new BinaryWriter(fs);
        br.Write(byt);
        fs.Dispose();
        try
        {
            foreach (var process in Process.GetProcessesByName("WINWORD"))
            {
                process.Kill();
            }
            object missing = Missing.Value;
            Word.Application wordApp = new Word.ApplicationClass();
            Word.Document aDoc = null;
            object filename = "C:\\Temp\\[REDACTED].docx";
            if (File.Exists((string)filename))
            {
                object readOnly = false;
                object isVisible = false;
                wordApp.Visible = false;
                aDoc = wordApp.Documents.Open(ref filename, ref missing,
            ref readOnly, ref missing, ref missing, ref missing,
            ref missing, ref missing, ref missing, ref missing,
            ref missing, ref isVisible, ref missing, ref missing,
            ref missing, ref missing);
                aDoc.Activate();
                this.FindAndReplace(wordApp, "<TodaysDate>", DateTime.Today.ToShortDateString());
                this.FindAndReplace(wordApp, "<ClaimantAddress>", TextBox1.Text.Trim());
                this.FindAndReplace(wordApp, "<ClaimantLastName>", TextBox2.Text.Trim());
                this.FindAndReplace(wordApp, "<DateReceived>", TextBox3.Text.Trim());
                aDoc.Save();
                foreach (var process in Process.GetProcessesByName("WINWORD"))
                {
                    process.Kill();
                }
            }
        }
        catch (Exception)
        {

        }
        string uploadFilename = "C:\\Temp\\[REDACTED].docx";
        string ext = Path.GetExtension(uploadFilename);
        string contentType = String.Empty;
        switch (ext)
        {
            case ".doc":
                contentType = "application/vnd.ms-word";
                break;
            case ".docx":
                contentType = "application/vnd.ms-word";
                break;
        }
        if (contentType != String.Empty)
        {
            Stream newFS = new FileStream("C:\\Temp\\[REDACTED].docx", FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
            BinaryReader newBR = new BinaryReader(newFS);
            Byte[] bytes = newBR.ReadBytes((Int32)newFS.Length);
            string appID = "a3679061-a06b-4897-bdc9-1b097e28dba2";
            string appNumber = "[REDACTED]";
            string strQuery = "INSERT INTO GeneratedLetters(ID, ApplicationID, ApplicationNumber, LetterName, Letter) VALUES (NEWID(), @ApplicationID, @ApplicationNumber, @LetterName, @Letter)";
            SqlCommand newCMD = new SqlCommand(strQuery);

            newCMD.Parameters.Add("@ApplicationID", SqlDbType.UniqueIdentifier).Value = new Guid(appID);
            newCMD.Parameters.Add("@ApplicationNumber", SqlDbType.VarChar).Value = appNumber;
            newCMD.Parameters.Add("@LetterName", SqlDbType.VarChar).Value = uploadFilename;
            newCMD.Parameters.Add("@Letter", SqlDbType.Binary).Value = bytes;
            InsertUpdateData(newCMD);
            Label1.ForeColor = System.Drawing.Color.Green;
            Label1.Text = "File Uploaded Successfully";
        }
        else
        {
            Label1.ForeColor = System.Drawing.Color.Red;
            Label1.Text = "File format not recognized";
        }
    }
}
private void FindAndReplace(Word.Application wordApp, object findText, object replaceText)
{
    object matchCase = true;
    object matchWholeWord = true;
    object matchWildCards = false;
    object matchSoundsLike = false;
    object matchAllWordForms = false;
    object forward = true;
    object format = false;
    object matchKashida = false;
    object matchDiacritics = false;
    object matchAlefHamza = false;
    object matchControl = false;
    object read_only = false;
    object visible = true;
    object replace = 2;
    object wrap = 1;
    wordApp.Selection.Find.Execute(ref findText, ref matchCase,
        ref matchWholeWord, ref matchWildCards, ref matchSoundsLike,
        ref matchAllWordForms, ref forward, ref wrap, ref format,
        ref replaceText, ref replace, ref matchKashida,
                ref matchDiacritics,
        ref matchAlefHamza, ref matchControl);
}
private Boolean InsertUpdateData(SqlCommand cmd)
{
    String strConnString = System.Configuration.ConfigurationManager.ConnectionStrings["[REDACTED]ConnectionString"].ConnectionString;
    SqlConnection con = new SqlConnection(strConnString);
    cmd.CommandType = CommandType.Text;
    cmd.Connection = con;
    try
    {
        con.Open();
        cmd.ExecuteNonQuery();
        return true;
    }
    catch (Exception ex)
    {
        Response.Write(ex.Message);
        return false;
    }
    finally
    {
        con.Close();
        con.Dispose();
    }
}

Answer 1

Most of what happens in Button1_Click should not be happening there, it should be happening in a separate layer. I'd suggest you look into multi-tier architecture and split up your code into at least three tiers:

• Presentation tier
• Application tier (business logic, logic tier, or middle tier)
• Data tier

Splitting up the logic will result in smaller methods and much easier to maintain code. Right now you have three methods over 150 lines of code and so much happens in those three methods it is hard to keep an overview.

---

Use meaningful names: byt is meaningless and also wrong since it is a byte array.

---

Classes that implement IDisposable like SqlConnection should be used inside using statements.

Even better: look into using an ORM like NHibernate or Entity Framework instead of "raw ADO". That way you don't have to repeatedly write code that involves SqlConnection etc. Moreover:

I’ll repeat this little gem one last time, if you’re writing ADO.Net code by hand, you’re stealing from your employer or client.

Answer 2

You've tagged Asp.Net so I'm going to assume this is a website... Have you considered what happens if more than one person clicks this button1 at the same time?

foreach (var process in Process.GetProcessesByName("WINWORD"))
{
    process.Kill();
}

Hmmm, the second user could kill the first user's word instance. That seems like a massive bug waiting to happen. There is a quit method on Word.Application you should use instead of killing the processes.

---

You should also read up on disposing anything that implements IDisposable. In short you can do:

IDisposable something;
try
{
    something = new DisposableObject();
    something.DoSomething();
}
finally
{
    something.Dispose();
}

You'll notice that's a bit cumbersome, luckily there is a shorter version:

using (var something = new DisposableObject())
{
     something.DoSomething();
}

All of your ADO.Net objects implement IDisposable: SqlConnection, SqlCommand, SqlDataReader.

---

You should make sure your code is stating its intent clearly:

while(dr.Read())
{
    byt = (byte[])dr["Letter"];
}

Why are you overwriting? Your code implies only a single row:

using (IDataReader reader = cmd.ExecuteReader())
{
    if (dr.Read())
    {
        bytes = (byte[])dr["Letter"];
    }
}

You'll notice that I've also declared the reader as an IDataReader. It's generally better to code against an interface if you can.

---

Your code is littered with magic strings, you should move them to named fields.

e.g.

private static readonly string Filename = "C:\\Temp\\[REDACTED].docx";

It would be even better as an app setting.

---

You're mixing aliases and typenames. E.g. object vs String vs string. Choose one and stick with it (and preferably aliases).

---

Add usings to simplify type names:

System.Configuration.ConfigurationManager.ConnectionStrings["[REDACTED]ConnectionString"].ConnectionString;
SqlConnection con = new SqlConnection(strConnString);

Is better as:

// using System.Configuration;

string strConnString = ConfigurationManager.ConnectionStrings["[REDACTED]ConnectionString"].ConnectionString;

---

Word interop is pretty horrendous. I notice that your extension is .docx - have you considered using a library to manipulate the file as openxml and throwing out the interop?