3 ms·

I know, I just couldn't find a cross-db solution since I can't seem to find any standardized error codes in the exception. Do you know of a better way of doing

I know, I just couldn't find a cross-db solution since I can't seem to find any standardized error codes in the exception. Do you know of a better way of doing this?

midasz 9y ago
How about adding a "GetByID" function? The check is searching the entity, if NULL - save - if NOT NULL - update. I don't believe it is useful to save a trip to the database, the database is there for a reason.
- biarity 9y ago
  Yes, that is the usual way of doing it. However, the point of this post was to showcase some ways of reducing duplicate code in large projects. Doing it normally (by checking if the entity already exists) can get repetitive if you have many controllers/entities. If you know of a better way of doing this, I'd be happy to update my article :)
- deleted 9y ago
  [deleted]
czechdeveloper 9y ago
I would just delete that part if it can't be solved. Smaller issues with code are, that you create new string just to compare it (ToLower makes new instance), when you can achieve this just by using proper comparer such as ex.InnerException.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1 Good idea would also be to check for existence of Inner exception at all ex.InnerException?.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1 But I would just live without that check to prevent magical behavior.
- biarity 9y ago
  I myself really dislike that piece of code (searching in exception strings isn't secure for many reasons, for example someone could use this to reverse-engineer information about my database). But the only alternative that comes to mind is to check if an entity exists before attempting to create it - which I don't like because it leads to repetitive code in large projects (and I might forget to write checks). Any other ideas?
- Dayshine 9y ago
  Yes, because the performance impact of a single string creation in an Exception handler is important.
  - mattmanser 9y ago
    I can't upvote this enough. I done a fair bit of performance problem fixing now in my career and I have never seen a .ToLower be the cause of any client's performance problems. Or really any string manipulation opinion like stringbuilder vs += and all that nonsense.
    - LandR 9y ago
      If you want to get really picky you should be converting to uppercase and ToUpperInvariant in particular: From C# via the CLR: Important If you want to change the case of a string's characters before performing an ordinal comparison, you should use String’s ToUpperInvariant or ToLowerInvariant method. When normalizing strings, it is highly recommended that you use ToUpperInvariant instead of ToLowerInvariant because Microsoft has optimized the code for performing uppercase comparisons. In fact, the FCL internally normalizes strings to uppercase prior to performing case insensitive comparisons. We use ToUpperInvariant and ToLowerInvariant methods because the String class does not offer ToUpperOrdinal and ToLowerOrdinal methods. We do not use the ToUpper and ToLower methods because these are culture sensitive. And from MS CA1308: https://docs.microsoft.com/en-gb/visualstudio/code-quality/ca1308-normalize-strings-to-uppercase https://docs.microsoft.com/en-gb/visualstudio/code-quality/c...
- mattmanser 9y ago
  Your code is harder to read and the performance implications are so trivial, a classic example of pre-optimization.
  - tw1010 9y ago
    I agree with you. But I also think you're sacrificing clarity over brevity yourself by not expanding "pre-optimization" into "premature optimization" ;)
caleblloyd 9y ago
I do not know of a good Cross DB solution. It may be more elegant to check specific exception types for the DBMSs that you do use. For example, MySQL would be: https://github.com/mysql-net/MySqlConnector/blob/master/src/MySqlConnector/MySql.Data.MySqlClient/MySqlException.cs https://github.com/mysql-net/MySqlConnector/blob/master/src/... Check that MySqlException.Number == 1062 for Duplicate entry. Add one check per DBMS that you use.

I know, I just couldn't find a cross-db solution since I can't seem to find any standardized error codes in the exception. Do you know of a better way of doing this?

midasz 9y ago

How about adding a "GetByID" function? The check is searching the entity, if NULL - save - if NOT NULL - update. I don't believe it is useful to save a trip to the database, the database is there for a reason.

biarity 9y ago
Yes, that is the usual way of doing it. However, the point of this post was to showcase some ways of reducing duplicate code in large projects. Doing it normally (by checking if the entity already exists) can get repetitive if you have many controllers/entities. If you know of a better way of doing this, I'd be happy to update my article :)
deleted 9y ago
[deleted]

czechdeveloper 9y ago

I would just delete that part if it can't be solved. Smaller issues with code are, that you create new string just to compare it (ToLower makes new instance), when you can achieve this just by using proper comparer such as ex.InnerException.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1 Good idea would also be to check for existence of Inner exception at all ex.InnerException?.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1 But I would just live without that check to prevent magical behavior.

biarity 9y ago
I myself really dislike that piece of code (searching in exception strings isn't secure for many reasons, for example someone could use this to reverse-engineer information about my database). But the only alternative that comes to mind is to check if an entity exists before attempting to create it - which I don't like because it leads to repetitive code in large projects (and I might forget to write checks). Any other ideas?
Dayshine 9y ago
Yes, because the performance impact of a single string creation in an Exception handler is important.
- mattmanser 9y ago
  I can't upvote this enough. I done a fair bit of performance problem fixing now in my career and I have never seen a .ToLower be the cause of any client's performance problems. Or really any string manipulation opinion like stringbuilder vs += and all that nonsense.
  - LandR 9y ago
    If you want to get really picky you should be converting to uppercase and ToUpperInvariant in particular: From C# via the CLR: Important If you want to change the case of a string's characters before performing an ordinal comparison, you should use String’s ToUpperInvariant or ToLowerInvariant method. When normalizing strings, it is highly recommended that you use ToUpperInvariant instead of ToLowerInvariant because Microsoft has optimized the code for performing uppercase comparisons. In fact, the FCL internally normalizes strings to uppercase prior to performing case insensitive comparisons. We use ToUpperInvariant and ToLowerInvariant methods because the String class does not offer ToUpperOrdinal and ToLowerOrdinal methods. We do not use the ToUpper and ToLower methods because these are culture sensitive. And from MS CA1308: https://docs.microsoft.com/en-gb/visualstudio/code-quality/ca1308-normalize-strings-to-uppercase https://docs.microsoft.com/en-gb/visualstudio/code-quality/c...
mattmanser 9y ago
Your code is harder to read and the performance implications are so trivial, a classic example of pre-optimization.
- tw1010 9y ago
  I agree with you. But I also think you're sacrificing clarity over brevity yourself by not expanding "pre-optimization" into "premature optimization" ;)

caleblloyd 9y ago

I do not know of a good Cross DB solution. It may be more elegant to check specific exception types for the DBMSs that you do use. For example, MySQL would be: https://github.com/mysql-net/MySqlConnector/blob/master/src/MySqlConnector/MySql.Data.MySqlClient/MySqlException.cs https://github.com/mysql-net/MySqlConnector/blob/master/src/... Check that MySqlException.Number == 1062 for Duplicate entry. Add one check per DBMS that you use.