Return to Answer

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

## EDIT

Also

This:

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

EDIT

Also

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

EDIT

Also

This:

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

## EDIT

Also

This:

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

EDIT

Also

This:

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already done at an upper layer.

Code like this is bound to be a bit hairy.

However

You aren't using parameters

This is a big security vulnerability. What's that SanitiseForMySql method doing? Is it doing everything by the book? Does it handle every possible obscure corner case? Let ADO the MySQL connector manipulate your query, especially if there's a chance you're handling values supplied by the user.

MySQL can perform the so-called MERGE (also called UPSERT)

You can do it via INSERT ... ON DUPLICATE KEY UPDATE statement. In case you are not aware what that is, it's basically a way to either insert or update in case the INSERT fails because of a duplicate key error. It's for sure faster than making a separate query to retrieve the records that are currently present.

By the way, code like this leaves your code open to race conditions in case another user inserts a record exactly between when you read the current records and the moment you execute your INSERT. This is bad, and you're not currently guarding against it. MySQL can do it for you, take advantage of it.

EDIT

Also

This:

    if (fr.IsGoneaway)
    {
        sqlTargetFlag = sqlTargetFlag + "Goneaway";
    }

are you sure this is correct? Concatenating a column name to the other like that? Are you assuming the column name will be passed in followed by a comma? And anyway, isn't it going to blow up with a syntax error when you do the UPDATE? Or will the column name get passed as an empty string? Might be a good idea to check what garbage you are receiving through your parameters, assuming said checks aren't already in place at an upper layer. How can you be sure all callers will remember to pass your arguments exactly the way you expect them? Plus, just my two cents, but if my assumptions are correct, this is not a very fun API to work with.