# What popular ORMs for C\# hide: Let's check RepoDB and SqlSugar

When it comes to ORMs, most C\# developers immediately think of the powerful Entity Framework Core or the lightweight Dapper\. What about RepoDB and SqlSugar, though? While they may not be as well\-known, these ORMs are actually being used in real\-world projects and are still developing\. Let's take a look at what issues we can uncover in their source code with the help of a static analyzer\.

![1411_dotnet_orm_errors/image1.png](https://import.viva64.com/docx/blog/1411_dotnet_orm_errors/image1.png)

## Introduction

It's hard to imagine a modern application without an ORM: this technology helps developers by reducing the amount of code they need to write and by eliminating the need to work directly with SQL\. 

The ORM manages all interactions with the database, including processing read and write queries, transactions, and object\-to\-SQL mapping\. Errors in such tools can lead to serious consequences: from reduced performance to application malfunctions and even security issues\.

In the \.NET world, [Entity Framework Core](https://github.com/dotnet/efcore) and [Dapper](https://github.com/DapperLib/Dapper) are the industry standards\. However, many less popular tools are also used in real\-world projects\. [RepoDB](https://github.com/mikependon/RepoDB) and [SqlSugar](https://github.com/DotNetNext/SqlSugar) are among these ORMs\.

In this article, we'll look at them through the lens of PVS\-Studio static analyzer\. Instead of comparing API capabilities or performance, we'll check the projects' source code for potential bugs and suspicious constructs\.

## SqlSugar

Let's move on to discussing the most exciting parts of the SqlSugar code\. The source code comes from [this commit](https://github.com/DotNetNext/SqlSugar/tree/d221c7b14ab4bc4c81ce53b6a450c96e70091168)\.

### The forgotten CancellationToken

#### Code fragment 1

```cpp
public Task<int> ExecuteCommandAsync(string sql, object parameters, 
  CancellationToken cancellationToken) 
{
  this.CancellationToken = CancellationToken;
  return ExecuteCommandAsync(sql,parameters);
}
```

The PVS\-Studio warning: [V3005](https://pvs-studio.com/en/docs/warnings/v3005/) The 'this\.CancellationToken' variable is assigned to itself\. [AdoProvider\.cs 1472](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar/Abstract/AdoProvider/AdoProvider.cs#L1472)

The `CancellationToken` auto\-property is assigned its own value\. Most likely, this property should be assigned the value of the `cancellationToken` parameter instead\.

### A pointless string check

#### Code fragment 2

```cpp
private static void AppColumns(SqlInfo result, 
  ISugarQueryable<object> queryable, 
  string columnName)
{
  var selectPkName = queryable.SqlBuilder.GetTranslationColumnName(columnName);
  if (result.IsSelectNav) 
  {
    if (   result.SelectString != null 
        && !result.SelectString
                  .ToLower()
                  .Contains($" {selectPkName.ToLower()}
                              AS {selectPkName.ToLower()}"))            // <=
    {
      result.SelectString = result.SelectString + "," 
        + (selectPkName + " AS " + selectPkName);
    }
  }
  ....
}
```

The PVS\-Studio warning: [V3122](https://pvs-studio.com/en/docs/warnings/v3122/) Lowercase string is compared with a different mixed case string\. [NavigatManager\.cs 1131](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar/Abstract/QueryableProvider/NavigatManager.cs#L1131)

Let's take a closer look at what's going wrong here\. First, the `result.SelectString` string is converted to lowercase using the `ToLower` method, and then the resulting string is checked whether it contains `$" {selectPkName.ToLower()} AS {selectPkName.ToLower()}"`\. The substring we're looking for contains uppercase letters \(`AS`\)\. As a result, the `Contains` method will always return `false`\.

To fix the code, we can simply convert all the characters to lowercase:

```cpp
!result.SelectString
       .ToLower()
       .Contains($" {selectPkName.ToLower()} as {selectPkName.ToLower()}"))
```

### Recurring conditions

#### Code fragment 3

```cpp
public override string ToSqlString()
{
  ....
  if (it.InsertServerTime || it.InsertSql.HasValue()) 
  {
    return GetDbColumn(it,null);
  }
  object value = null;
  if (it.Value is DateTime)
  {
    ....
  }
  else if (   it.Value is int 
           || it.Value is long 
           || it.Value is short                                    // <=
           || it.Value is short                                    // <=
           || it.Value is byte 
           || it.Value is double)
  {
    return  it.Value;
  }
  
  ....
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'it\.Value is short' to the left and to the right of the '\|\|' operator\. [QuestDBInsertBuilder\.cs 89](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar/Realization/QuestDB/SqlBuilder/QuestDBInsertBuilder.cs#L89)

Using the `it.Value is short` sub\-expression twice is pointless\. Perhaps `short` should be replaced with another type, such as `decimal` or `float`\.

#### Code fragment 4

```cpp
public static Func<string, object> GetTypeConvert(object value)
{
  if (   value is int 
      || value is uint 
      || value is int? 
      || value is uint?)
  {
    return x => Convert.ToInt32(x);
  }
  else if (   value is short 
           || value is ushort 
           || value is short? 
           || value is ushort?)
  {
    return x => Convert.ToInt16(x);
  }
  else if (   value is long 
           || value is long?                                       // <=
           || value is ulong? 
           || value is long?)                                      // <=
  {
    return x => Convert.ToInt64(x);
  }
  ....
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'value is long?' to the left and to the right of the '\|\|' operator\. [UtilMethods\.cs 288](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar.MySqlConnector/Tools/UtilMethods.cs#L288)

The developers used the `value is long?` condition twice here\. Based on the code above, we can assume that the second identical condition should be replaced with `value is ulong`\.

### An unused parameter

#### Code fragment 5

```cpp
private string GetName(ExpressionParameter parameter, 
                       MemberExpression expression, 
                       bool? isLeft, 
                       bool isSingle)
{
  if (isSingle)
  {
    return GetSingleName(parameter, expression, IsLeft);
  }
  else
  {
    return GetMultipleName(parameter, expression, IsLeft);
  }
}
```

The PVS\-Studio warning: [V3196](https://pvs-studio.com/en/docs/warnings/v3196/) The 'isLeft' parameter is not utilized inside the method body, but an identifier with a similar name is used inside the same method\. [MemberExpressionResolve\.cs 770](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar/ExpressionsToSql/ResolveItems/MemberExpressionResolve.cs#L770)

The `isLeft` parameter isn't used, but the method accesses a property whose name differs from the parameter name only in the capitalization of the first letter\.

The developers might have wanted to use `isLeft` parameter of the `GetName` method intended of the `IsLeft` property\.

### The missing num

#### Code fragment 6

```cpp
public string GetValue(Expression expression)
{
  var numExp = (expression as MethodCallExpression).Arguments[0];
  var num =1;
  if (ExpressionTool.GetParameters(numExp).Any()) 
  { 
    var copyContext = this.Context.GetCopyContextWithMapping();
    copyContext.IsSingle = false;
    copyContext.Resolve(numExp, ResolveExpressType.WhereMultiple);
    copyContext.Result.GetString();                              // <=
  }
  else 
  {
    num = ExpressionTool.DynamicInvoke(numExp).ObjToInt();
  }
  var take = (expression as MethodCallExpression); 
  if (....)
  {
    return "TOP " + num;
  }
  else if (this.Context is OracleExpressionContext)
  {
    return (HasWhere ? "AND" : "WHERE") + " ROWNUM<=" + num;
  }
  else if (....)
  {
    return "limit " + num;
  }
  else if (this.Context.GetLimit() != null)
  {
    if (this?.Context?.Case != null)
    {
      this.Context.Case.HasWhere = this.HasWhere;
      this.Context.Case.Num = num;
    }
    return this.Context.GetLimit();
  }
  else
  {
    return "limit " + num;
  }
}
```

The PVS\-Studio warning: [V3010](https://pvs-studio.com/en/docs/warnings/v3010/) The return value of function 'GetString' is required to be utilized\.  [SubTake\.cs 64](https://github.com/DotNetNext/SqlSugar/blob/d221c7b14ab4bc4c81ce53b6a450c96e70091168/Src/Asp.Net/SqlSugar/ExpressionsToSql/Subquery/Items/SubTake.cs#L64)

The analyzer reports that the return value of the `GetString` method isn't used\. At the same time, the `GetString` implementation reveals that the method only builds and returns a string representation of `_Result`\. It doesn't modify the object's state or perform any actions whose results are used later in the code:

```cpp
public string GetString()
{
  if (_Result == null) return null;
  if (IsUpper)
    return
  _Result.ToString()
         .ToUpper()
         .Replace(UtilConstants.ReplaceCommaKey,",")
         .TrimEnd(',');
  else
    return _Result.ToString()
                  .Replace(UtilConstants.ReplaceCommaKey, ",")
                  .TrimEnd(',');
}
```

So, the result of the `GetString` call is discarded in the current code\. The developers probably forgot to use the return value when calculating `num`\.

The surrounding code also supports this assumption: every return from the method uses the `num` variable\. Following this logic, both branches of the first condition should calculate the same value and assign it to the `num` variable\.

The correct code may look like this:

```cpp
copyContext.Resolve(numExp, ResolveExpressType.WhereMultiple);
num = copyContext.Result.GetString();
```

## RepoDB

Now let's look at some errors and issues in the RepoDB project\. The source code comes from [this commit](https://github.com/mikependon/RepoDB/tree/58004d4b05a99b0332e8eb3bbe74d366030b8924)\.

### Pointless checks

#### Code fragment 1

```cpp
public static object GetValue(this ConditionalExpression expression)
{
  var test = expression.Test.GetValue();
  var trueValue = expression.IfTrue.GetValue();
  if (expression.Test.NodeType == ExpressionType.Equal)
  {
    return test == trueValue ? trueValue : expression.IfFalse.GetValue();
  }
  else if (expression.Test.NodeType == ExpressionType.NotEqual)
  {
    return test != trueValue ? trueValue : expression.IfFalse.GetValue();
  }
  else if (expression.Test.NodeType > ExpressionType.GreaterThan)
  {
    ....
  }
  else if (expression.Test.NodeType > ExpressionType.GreaterThanOrEqual)// <=
  {
    ....  
  }
  else if (expression.Test.NodeType > ExpressionType.LessThan)          // <=
  {
    ....
  }
    else if (expression.Test.NodeType > ExpressionType.LessThanOrEqual) // <=
  {
    ....
  }
    throw new NotSupportedException(....);
}
```

The PVS\-Studio warnings:

* V3022 Expression 'expression\.Test\.NodeType \> ExpressionType\.GreaterThanOrEqual' is always false\. [ExpressionExtension\.cs 456](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/Extensions/ExpressionExtension.cs#L456)
* V3022 Expression 'expression\.Test\.NodeType \> ExpressionType\.LessThan' is always false\. [ExpressionExtension\.cs 460](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/Extensions/ExpressionExtension.cs#L460)
* V3022 Expression 'expression\.Test\.NodeType \> ExpressionType\.LessThanOrEqual' is always false\. [ExpressionExtension\.cs 464](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/Extensions/ExpressionExtension.cs#L464)

In the `GetValue` method, `expression.Test.NodeType` is successively compared against different values from the `ExpressionType` enumeration\. However, the last four checks use the `>` operator instead of the equality operator\.

The `else if` chain executes sequentially, and the enumeration values in the lower branches are greater than those in the branches above them\. 

```cpp
public enum ExpressionType
{
  ....
  GreaterThan = 15,
  GreaterThanOrEqual = 16,
  ....
  LessThan = 20,
  LessThanOrEqual = 21,
  ....
}
```

The `expression.Test.NodeType > ExpressionType.GreaterThan` condition covers all the conditions that follow\. 

So, if `expression.Test.NodeType > ExpressionType.GreaterThan` evaluates to `true`, the subsequent conditions aren't checked\. If it evaluates to `false`, the following conditions are `false`\.

### A copy\-paste error

#### Code fragment 2

```cpp
public override int GetHashCode()
{
  // Make sure to return if it is already provided
  if (this.hashCode != null)
  {
    return this.hashCode.Value;
  }

  // Get first the entity hash code
  var hashCode = HashCode.Combine(base.GetHashCode(), Name, ".UpdateAll");

  // Get the fields
  if (Fields != null)
  {
    foreach (var field in Fields)                           // <=
    {
      hashCode = HashCode.Combine(hashCode, field);
    }
  }

  // Get the qualifier <see cref="Field"/> objects
  if (Fields != null)                                       // <=
  {
    foreach (var field in Qualifiers)
    {
      hashCode = HashCode.Combine(hashCode, field);
    }
  }

  ....
}
```

The PVS\-Studio warning: [V3127](https://pvs-studio.com/en/docs/warnings/v3127/) Two similar code fragments were found\. Perhaps, this is a typo and 'Qualifiers' variable should be used instead of 'Fields'\. [UpdateAllRequest\.cs 124](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/Requests/UpdateAllRequest.cs#L124)

`Fields` is checked for `null` twice\. Most likely, the second check should use `Qualifiers` instead of `Fields`, since this collection is used in `foreach`\.

#### Code fragment 3

```cpp
public static Task<int> UpdateAllAsync<TEntity>(....
    Expression<Func<TEntity, object>> qualifiers,
    ....,
    IEnumerable<Field> fields = null,
    ....)
    where TEntity : class
{
  return UpdateAllAsyncInternal<TEntity>(connection: connection,
    tableName: tableName,
    entities: entities,
    qualifiers: fields,                                      // <=
    batchSize: batchSize,
    fields: fields,                                          // <=
    hints: hints,
    commandTimeout: commandTimeout,
    traceKey: traceKey,
    transaction: transaction,
    trace: trace,
    statementBuilder: statementBuilder,
    cancellationToken: cancellationToken);
}
```

The PVS\-Studio warning: [V3038](https://pvs-studio.com/en/docs/warnings/v3038/) The argument was passed to method several times\. It is possible that other argument should be passed instead\. [UpdateAll\.cs 619](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/Operations/DbConnection/UpdateAll.cs#L619)

Here, the `fields` parameter is passed to the `UpdateAllAsyncInternal` method twice, while the `qualifiers` parameter isn't used at all\.

The developers might have wanted to pass `qualifiers` as follows:

```cpp
qualifiers: Field.Parse<TEntity>(qualifiers)
```

### An empty collection

#### Code fragment 4

```cpp
public override string CreateBatchQuery(string tableName,
  IEnumerable<Field> fields,
  int page,
  int rowsPerBatch,
  IEnumerable<OrderField> orderBy = null,
  QueryGroup where = null,
  string hints = null)
{
  // Ensure with guards
  GuardTableName(tableName);

  // Validate the hints
  GuardHints(hints);

  // There should be fields
  if (fields?.Any() != true)
  {
    throw new MissingFieldsException(fields?.Select(f => f.Name));
  }
  ....
}
```

The PVS\-Studio warning: [V3191](https://pvs-studio.com/en/docs/warnings/v3191/) Iteration through the 'fields' collection makes no sense because it is always empty\. [SqlServerStatementBuilder\.cs 70](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.SqlServer/RepoDb.SqlServer/StatementBuilders/SqlServerStatementBuilder.cs#L70)

The `fields?.Any() != true` condition is true when the `fields` collection is empty or `null`\. In that case, `fields?.Select(f => f.Name)` serves no purpose, as it will always return an empty collection or `null`\.

### The treacherous null

#### Code fragment 5

```cpp
public override Guid GetGuid(int i)
{
  ThrowExceptionIfNotAvailable();
  return Guid.Parse(GetValue(i)?.ToString());
}
```

The PVS\-Studio warning: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The result of null\-conditional operator is passed as the first argument to the 'Parse' method and is not expected to be null\. [DataEntityDataReader\.cs 405](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.Core/RepoDb/DataEntityDataReader.cs#L405)

Here, `GetValue(i)` may return `null`, which will be passed to the `Guid.Parse` method because of the `?` operator\. And `Guid.Parse` will throw an `ArgumentNullException` when passed the `null` argument\.

#### Code fragment 6

```cpp
public static void AssertMembersEquality(
  object obj, 
  IDictionary<string, object> dictionary)
{
  ....
  var value1 = property.GetValue(obj);
  var value2 = dictionary[property.Name];
  if (value1 is byte[] b1 && value2 is byte[] b2)
  {
    ....
  else
  {
    var propertyType = property.PropertyType.GetUnderlyingType();
    if (propertyType == typeof(TimeSpan) && value2 is DateTime dateTime)
    {
      value2 = dateTime.TimeOfDay;
    }
    else if (propertyType == typeof(string) && value2 is DateTime)
    {
      value1 = DateTime.Parse(value1?.ToString());                // <=
    }
  ....
}
```

The PVS\-Studio warning: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The result of null\-conditional operator is passed as the first argument to the 'Parse' method and is not expected to be null\. [Helper\.cs 157](https://github.com/mikependon/RepoDB/blob/58004d4b05a99b0332e8eb3bbe74d366030b8924/RepoDb.SqLite/RepoDb.SqLite.IntegrationTests/Helper.cs#L157)

Again, if `value1` is `null`, the `ArgumentNullException` will be thrown\.

## Conclusion

Despite the substantial amount of code and extensive functionality of the projects, the analysis revealed only a few suspicious code fragments\. This suggests that both projects are well written and maintained at a high standard\.

Still, perfect code doesn't exist\. Even mature and popular libraries can contain copy\-paste errors, forgotten parameters, incorrect conditions, and other minor issues that are easy to overlook during development and testing\. This is where static analysis is particularly useful: it can highlight potential issues before they become real problems\.

If you'd like to analyze your own project with PVS\-Studio, you can try it via this [link](https://pvs-studio.com/en/pvs-studio/try-free/)\.

Take care of yourself and your code\!