Duplicates are being inserted into the year table. Why?

Question

3.00/5 (2 votes)

See more:

This code adds a new year into the years table. If the same year was inserted before it should show an error message? My code doesn't work and duplicates are inserted into the years table. Where am I going wrong?

Thanks in advance!

C#

//checks for duplicate year
if (txt_add_year.Text != "")
{
    //gets years list
    DataAccess connection = new DataAccess(); //my ado class collection
    DataTable dt = new DataTable();
    string query = "SELECT * FROM years;";
    dt = connection.from_database(query);

    foreach(DataRow row in dt.Rows)
    {
        //if new year is in database
        if(txt_add_year.Text == row["year_name"])
        {
            //error massage
            MessageBox.Show("سال وارد شده موجود می باشد","خطا",MessageBoxButtons.OK,MessageBoxIcon.Stop);
            return;
        }
    }

    //adds new year
    query = "INSERT INTO years(year_name) VALUES('{0}');";
    query = string.Format(query, txt_add_year.Text);

    connection.to_database(query);

    //gets school list
    query = "SELECT * from school;";
    dt = null;
    dt = connection.from_database(query);

    //gets new year id
    // but in duplication state gets f***ing x year id
    query = "SELECT year_id FROM years;";
    DataTable dt2 = new DataTable();
    dt2 = connection.from_database(query);

    //updates schoolyear table
    //insert new rows into school year so each school has a row for new year
    foreach (DataRow row in dt.Rows)
    {
        query = "INSERT INTO schoolyear(school_id, year_id)" +
                "VALUES({0}, {1});";
        query = string.Format(query, row["school_id"], dt2.Rows[0]["year_id"]);
        connection.to_database(query);
    }
    this.Text = "سال جدید ثبت شد";
}

Posted 13-Feb-11 4:36am

siaswar

Updated 13-Feb-11 9:14am

Henry Minute

v3

Add a Solution

3 solutions

Solution 1

You are comparing the year values using string comparison. This will not work as you expect unless you trim off trailing space from the left and right edges of the strings. Both the string the user entered, and the strings stored in the database.

I would suggest that a better option would be to store the year as an int in the database. Then you can make a more stable comparison that way. You may also consider redesigning your database table so as to not allow insertions of duplicate column values (by making it a unique field).

EDIT: I missed the string to object comparison (see Sandeep's answer). But even if you call ToString, you need to be aware of trimming related errors.

Posted 13-Feb-11 5:13am

Smitha Nishant

Updated 13-Feb-11 6:05am

v3

Comments

Sandeep Mewara 13-Feb-11 11:47am

I disagree and agree.

Actual reason was object comparison and it fails everytime.

Smitha Nishant 13-Feb-11 12:03pm

Yes, I missed that - thanks. But I will still recommend to the OP to follow the advice in my 2nd paragraph.

Sandeep Mewara 13-Feb-11 12:05pm

I agreed on that already. :)

[no name] 7-Jul-13 16:20pm

HI Smitha, thanks for editing my tutorial "Java Thread Tutorial", I appreciate the work, Thanks in advance :D

Solution 3

Depending on the amount of data in your table you might find it quicker to use something like the following in place of your loop:

C#

string selectString = string.Format("year_name = {0}", txt_add_year.Text);
DataRow[] yearsFound = dt.Select(selectString);
if (yearsFound.Length > 0)
{
  // the year is already there.
}
else
{
  // it's not there, so add it.
}

Posted 13-Feb-11 9:38am

Henry Minute

Comments

Sandeep Mewara 14-Feb-11 13:44pm

Nice one Henry! 5!

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sandeep Mewara · Accepted Answer · 2011-02-13T05:46:00

Did you try to debug at all?

Check this line: if(txt_add_year.Text == row["year_name"])
Do you see anything wrong with it?

You are trying to compare a text with a Row object here. And it *will* fail always as there is a reference check here which would be different. And thus, it would move ahead in the method leading to duplication.

What you need to check is if the values are same or not.

Just make the following change and it should do:
if(txt_add_year.Text == row["year_name"].ToString())
When you do the ToString() call, you are then doing string comparison.