Windows form that generates invoices

I have a C# program that I would like to have reviewed. I am new to programming and want to know how it could be improved. It is a Windows Forms project that generates a PDF using the iTextSharp dll. I have tried to leave off the repetitive parts (multiple textboxes, etc) as well as the part that uses iTextSharp.

```
namespace InvoiceGenerator
{
public partial class InvoiceGenerator : Form
{
List state = new List();
public InvoiceGenerator()
{
InitializeComponent();

string[] states = new string[] { "Alabama", "Alaska", "Arizona", "Arkansas", "California", "Colorado",
"Connecticut", "Delaware", "Florida", "Georgia", "Hawaii", "Idaho", "Illinois", "Indiana", "Iowa", "Kansas",
"Kentucky", "Louisiana", "Maine", "Maryland", "Massachusetts", "Michigan", "Minnesota", "Mississippi",
"Missouri", "Montana", "Nebraska", "Nevada", "New Hampshire", "New Jersey", "New Mexico", "New York",
"North Carolina", "North Dakota", "Ohio", "Oklahoma", "Oregon", "Pennsylvania", "Rhode Island", "South Carolina",
"South Dakota", "Tennessee", "Texas", "Utah", "Vermont", "Virginia", "Washington", "West Virginia", "Wisconsin",
"Wyoming" };
comboBoxState.Items.AddRange(states);
}

private void InvoiceGenerator_Load(object sender, EventArgs e)
{
maskedTextBoxDate.Text = DateTime.Today.ToString("MM/dd/yyyy");
maskedTextBoxDate.ValidatingType = typeof(System.DateTime);
lblTotalPrice.Text = "$0.00";
}

public static bool IsFileLocked(FileInfo file)
{
FileStream stream = null;
try
{
stream = file.Open(FileMode.Open, FileAccess.ReadWrite, FileShare.None);
}
catch (IOException)
{
return true;
}
finally
{
if (stream != null)
stream.Close();
}
return false;
}

private void btnGenerate_Click(object sender, EventA

• Moving the states list to some sort of data store (could be a text / xml file, or database) is preferable; this way you won't have to re-compile if you want to use your application in countries other than the US.

• (A positive point) I see that IsFileLocked is copied from a SO answer about file locking. using proven and tested code (i.e from a good source online) is a very good practice. however, why is this method public?

• In FillForm, if the file is locked, you can just return from the function after displaying the message to the user; there's no need for the else clause.

Personally, I always advocate verification at the beginning, and returning immediately if it fails. This saves levels of nesting. (this comment also applies to checkBoxFull_CheckedChanged and numericUpDownFull_ValueChanged and maskedTextBoxDate_TypeValidationCompleted)

• Is invoiceFull a class member? if so, I would recommend naming it as such (either with a _ or m_ prefix). It's also recommended to declare all members at the top of the class.

Also, I fail to see where it's used.

• If numericUpDownFull.Value == 0 , then total will also will always be 0. no need to calculate it.

• The member invoiceName is never used for reading...

StackExchange Code Review Q#25190, answer score: 3