C# Windows Forms custom exception problems

Question

I'm trying to get this to throw an exception if year entered in TextBoxCopyrightYear.Text is higher than the current year, but it does not seem to be doing it.

The question is... why is it not throwing the exception when I input something higher than 2011?

I created custom exception class CopyrightYearOutOfRange:

public class CopyrightYearOutOfRange : Exception
{
    private LibraryBook book;
    private int year;

    public CopyrightYearOutOfRange(LibraryBook book, int year)
        : base("Year is beyond current year. This is impossible.")
    {
        this.book = book;
        this.year = year;
    }

    public LibraryBook Book { get { return book; } }
    public int Year { get { return year; } }

    public CopyrightYearOutOfRange(string message)
        : base(message)
    { 
    }
}

And it is thrown in this section in my LibraryBook class as this:

public LibraryBook(string title, string author, int copyrightYear)
{
    Title = title;
    Author = author;
    if (isValidYear(copyrightYear))
    {
        CopyrightYear = copyrightYear;
    }
    else
    {
        throw new CopyrightYearOutOfRange(this, copyrightYear);
    }
}

private bool isValidYear(int year)
{
    return year <= 2011;
}

So I created this code to check if the user input is valid...

// makes sure data is valid...
private bool validateData()
{
    int year;
    int errorCount = 0;
    string errorHeader = "";
    string errorMessage = "";
    string errorTitle = "";
    string plural = "";

    if (textBoxTitle.Text.Length == 0)
    {
        errorMessage += "\nEnter a title";
        errorCount++;
    }

    if (!int.TryParse(textBoxCopyrightYear.Text, out year) || year < 1)
    {
        errorMessage += "\nEnter year as a positive number";
        errorCount++;
    }
    else
    {
        try
        {
            // Is this where I'm making a mistake?
            int.TryParse(textBoxCopyrightYear.Text, out year);
        }
        catch (CopyrightYearOutOfRange ex)
        {
            MessageBox.Show(
                string.Format("{0}", ex.Message)
                , "Copyright out of range exception"
                , MessageBoxButtons.OK
                , MessageBoxIcon.Exclamation
                );
        }
    }

and here is the ok button code:

private void buttonOK_Click(object sender, EventArgs e)
{
    if (validateData())
    {
        controlsToObject();
        this.DialogResult = DialogResult.OK;
    }
}

edit: Here is the code that creates the object. Is this where I am supposed to put the try catch block?

    private void controlsToObject()
    {
        if (libraryBook == null)
        {
            libraryBook = new LibraryBook();
        }

        libraryBook.Title = textBoxTitle.Text;
        libraryBook.Author = textBoxAuthor.Text;
        libraryBook.CopyrightYear = int.Parse(textBoxCopyrightYear.Text);
    }

AGAIN: The question is... why is it not throwing the exception when I input something higher than 2011?

You are throwing the exception in LibraryBook constructor, but where exactly objects of that class are created? I don't see it in the code... — Bartosz, Nov 14 '11 at 08:36

score 2 · Answer 1 · answered Nov 14 '11 at 08:36

2

Well, the exception is thrown from the constructor of the LibraryBook class - but you're not calling that anywhere. You're converting the year number from string to int. No reason for the exception to be raised there.

answered Nov 14 '11 at 08:36

Vilx-

104,512
87
279
422

score 1 · Accepted Answer · answered Nov 14 '11 at 08:40

1

A solution for you is creating a LibraryBook object from your try block, so that the data gets validated in the constructor.

put this in your try:

var b = new LibraryBook(textBoxTitle.Text, textBoxAuthor.Text, int.Parse(textBoxCopyrightYear.Text));

answered Nov 14 '11 at 08:40

Dmitry Kruglov

318
1
9

Thank you for that. I understand that this creates the LibraryBook object. But is the location of my try-catch block correct? Or should it be in the edit in the post? – lorenzoid Nov 14 '11 at 08:55
See, you have your validation logic in two different places. In the constructor of LibraryBook, and in the validateData function. It would make more sense if all of your validation logic was in the constructor. If so, in validateData you will just have that try-catch block. – Dmitry Kruglov Nov 14 '11 at 09:09
But if you will create LibraryBook object *just* to validate data (and won't be using that particular instance anywhere else), then it means there is something wrong with how the code is designed. – Bartosz Nov 14 '11 at 09:10
Well, there are two different approaches to validation. Either you validate inside the constructor, or before invoking one. In my opinion validation in the constructor is not a bad thing, because it prevents us from creating invalid objects at all. The only drawback is that it gives a little overhead due to instantiating objects. For not complex objects, like LibraryBook that is not a problem, and only gives us a simple way to validate data and avoid duplicating code. – Dmitry Kruglov Nov 14 '11 at 09:27

score 0 · Answer 3 · answered Nov 14 '11 at 08:37

You call int.TryParse. This method returns false, if it cannot parse the value - it will never throw an exception. If you want to have an exception, you have to take int.Parse(). But this method will never throw your custom exception CopyrightYearOutOfRange!

score 0 · Answer 4 · answered Nov 14 '11 at 08:39

Because you don't create LibraryBook instance here:

        try
        {
            int.TryParse(textBoxCopyrightYear.Text, out year);
        }
        catch (CopyrightYearOutOfRange ex)
        {
            MessageBox.Show(
                string.Format("{0}", ex.Message)
                , "Copyright out of range exception"
                , MessageBoxButtons.OK
                , MessageBoxIcon.Exclamation
                );
        }

You just check if text in textBoxCopyrightYear is integer value

So how would I pass the int and check it there? – lorenzoid Nov 14 '11 at 08:52 — lorenzoid, Nov 14 '11 at 08:52

Bartosz · Answer 5 · 2011-11-14T08:44:35.960

try
{
    // Is this where I'm making a mistake?
    int.TryParse(textBoxCopyrightYear.Text, out year);
}

Exactly, you are parsing an integer here, and not instantiate LibraryBook. So your validation code is not executed, and your exception is never thrown.

In your case you don't need to use such validation in constructor, because you can just add if clause after int.TryParse.

if(int.TryParse(textBoxCopyrightYear.Text, out year))
if(!isValidYear(year))
    MessageBox.Show(
        string.Format("{0}", ex.Message)
        , "Copyright out of range exception"
        , MessageBoxButtons.OK
        , MessageBoxIcon.Exclamation
        );

C# Windows Forms custom exception problems

5 Answers5