# Should we check libraries before using them? MudBlazor helps us find the answer

There was a need in our company to use a library for Blazor components\. We chose MudBlazor and checked its code quality before implementation\. The result is a number of strange things and even a reproducible crash\. Want to know more? Keep on reading this article\.

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

[MudBlazor](https://mudblazor.com/) is one of the most popular libraries for Blazor applications\. In many ways, we chose it because of the number of components and the beauty of their implementation\. The library's source code is written in C\#, so we decided to analyze it before using it directly\. For analysis, we took the [source files of the v6\.1\.7 version](https://github.com/MudBlazor/MudBlazor/tree/v6.1.7)\. Next, let's talk about the errors found\.

## Questionable call

**Issue 1**

```cpp
public EndSlopeSpline(....):base(....)
{
  m = new Matrix(n);
  gauss = new MatrixSolver(n, m);
  a = new double[n];
  b = new double[n];
  c = new double[n];
  d = new double[n];
  h = new double[n];

  CalcParameters(firstSlopeDegrees, lastSlopeDegrees);
  Integrate();                                            // <=
  Interpolate();
}
```

The PVS\-Studio warning: [V3010](https://pvs-studio.com/en/docs/warnings/v3010/) The return value of function 'Integrate' is required to be utilized\. EndSlopeSpline\.cs 26

To understand the essence of the warning, we need to look at the implementation of the _Integrate_ method:

```cpp
public double Integrate()
{
  double integral = 0;
  for (int i = 0; i < h.Length; i++)
  {
    double termA = a[i] * h[i];
    double termB = b[i] * Math.Pow(h[i], 2) / 2.0;
    double termC = c[i] * Math.Pow(h[i], 3) / 3.0;
    double termD = d[i] * Math.Pow(h[i], 4) / 4.0;
    integral += termA + termB + termC + termD;
  }
  return integral;
}
```

In the method, the state of members does not change \(e\.g\. fields or properties\)\. Therefore, we can conclude that the only purpose of this method is to calculate and then return the value of the _integral_ variable\. This makes it pointless to call _Integrate_ without using the return value\.

Note that the _CalcParameters_ and _Interpolate_ methods \(called next to _Integrate_\) exactly change the states of a number of properties and fields\.

**Issue 2**

```cpp
internal void DateValueChanged(DateTime? value)
{
  _valueDate = value;

  if (value != null)
  {
    var date = value.Value.Date;

    // get the time component and add it to the date.
    if (_valueTime != null)
    {
      date.Add(_valueTime.Value);                    // <=
    }

    _filterDefinition.Value = date;
  }
  else
    _filterDefinition.Value = value;

  _dataGrid.GroupItems();
}
```

The PVS\-Studio warning: [V3010](https://pvs-studio.com/en/docs/warnings/v3010/) The return value of function 'Add' is required to be utilized\. Filter\.cs 140

The warning is similar to the previous one\. The return value of the _Add_ method called on a _DateTime_ variable was not used here\. Most likely, the developer wanted to add the _\_valueTime\.Value_ value to _date_\. However, they did not take into account that the _Add_ method for the _DateTime_ object returns the result of the addition, and does not change the original object\. The result is a useless call that was probably meant to have an impact\. 

## Unexpected NullReferenceException

**Issue 3**

```cpp
public static bool operator ==(ResizeOptions l, ResizeOptions r) 
                                                       => l.Equals(r);

public static bool operator !=(ResizeOptions l, ResizeOptions r) 
                                                       => !l.Equals(r);
```

The PVS\-Studio warnings:

* [V3115](https://pvs-studio.com/en/docs/warnings/v3115/) Passing 'null' to '\=\=' operator should not result in 'NullReferenceException'\. ResizeOptions\.cs 34
* [V3115](https://pvs-studio.com/en/docs/warnings/v3115/) Passing 'null' to '\!\=' operator should not result in 'NullReferenceException'\. ResizeOptions\.cs 35

When overloading the equality and inequality operators, it is worth considering the possibility that one of the operands may have the _null_ value\. [Microsoft documentation](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type) gives similar recommendations\.

However, in the implementation of '\=\=' and '\!\=' overloading, the left operand is not checked for _null_\. If the value of the _l_ parameter is _null_, an exception of the _NullReferenceException_ type will be thrown when the _Equals_ method is called\. The argument of the _Equals_ method, by the way, is also dereferenced without checking:

```cpp
public bool Equals(ResizeOptions other)
{
  if (ReportRate != other.ReportRate || ....)     // <=
  {
    return false;
  }
  ....
}
```

This behavior can be a very unpleasant surprise for the user if the class is public\. This is exactly what _ResizeOptions_ is\.

To make sure there is a problem, let's try to compare an object of the _ResizeOptions_ type with_ null_\.

![1032_MudBlazor/image2.png](https://import.viva64.com/docx/blog/1032_MudBlazor/image2.png)

Quite expectedly, we get an exception of the _NullReferenceException_ type\. If the right operand is _null_, the behavior will be similar\.

![1032_MudBlazor/image3.png](https://import.viva64.com/docx/blog/1032_MudBlazor/image3.png)

## Unreasonable assignment

**Issue 4**

```cpp
protected override void OnParametersSet()
{
  if (....)
  {
    ....
    foreach (....)
    {
      if (firstTime)
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      else
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      gridValueY = yValue;                                 // <=
      ....
    }
  }
  else
  {
    ....
  }
  ....
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'gridValueY' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 191, 189\. Line\.razor\.cs 191

Let's look at the nested _if_\. Some value is assigned to the _gridValueY_ variable in both branches of this operator\. It is worth noting that the value is the same in both the first and the second case\. It turns out that this assignment should have been taken out of the _if_\. However, this is not the main problem: the value of _gridValueY_ changes immediately after _if_, although it was not used\.

The _else_ block of the top\-level _if_ statement is similar to the _then_ block:

```cpp
foreach (....)
{
  if (firstTime)
  {
    ....
    gridValueY = verticalStartSpace;
  }
  else
  {
    ....
    gridValueY = verticalStartSpace;
  }
  ....
  gridValueY = boundHeight - (gridValueY + gridValue);   // <=
  ....
}
```

In this case, the value of _gridValueY_ is also overwritten immediately after _if_, but the old value is used\. 

## Wrong priorities

**Issue 5**

```cpp
internal async Task<bool> StartResizeColumn(....)
{
  ....
  // In case resize mode is column, we have to find any column right
  // of the current one that can also be resized and is not hidden.

  var nextResizableColumn = _columns.Skip(_....) + 1)
                                    .FirstOrDefault(c =>
                                        c.Resizable ?? true && !c.Hidden); // <=
  ....
}
```

The PVS\-Studio warning: [V3177](https://pvs-studio.com/en/docs/warnings/v3177/) The 'true' literal belongs to the '&&' operator with a higher priority\. It is possible the literal was intended to belong to '??' operator instead\. DataGridColumnResizeService\.cs 52

Let's look at the _c\.Resizable ?? true && \!c\.Hidden_ expression\. It is worth saying that the priority of '??' is lower than that of '&&'\. Based on this, if _c\.Resizable_ is _null_, then the null\-coalescing operator will return the result of the _true &&\!c\.Hidden_ operation\. The "&&" operation with the _true_ literal is pointless\. Most likely, the correct option looks as follows:

```cpp
(c.Resizable ?? true) && !c.Hidden
```

The difference between these variants will manifest itself if the values of _c\.Resizable_ and _c\.Hidden_ are _true_\.

There is another thing indicating that a mistake was made\. A line above has a comment that the column must be resizable \(information about this can be found in _c\.Resizable_\) and not hidden \(this can be found in _c\.Hidden_\)\. If _c\.Resizable_ is _true_, then the logic of the operation will be broken, since the value of _c\.Hidden_ will not affect the result in any way\.

## Summing up

It seems to me that each of the warnings we looked at may indicate an error that changes the logic of the program\. In one of the cases, we were able to prove that\. Before writing the article, I created an [issue](https://github.com/MudBlazor/MudBlazor/issues/6128) on GitHub\. However, I still haven't heard back from the developers\. I hope they will look into the problems listed and be able to improve the quality of their project code :\)\.

If you want to try the analyzer used to check MudBlazor, you can download it [here](https://pvs-studio.com/en/pvs-studio/try-free/)\. By the way, PVS\-Studio recently introduced the ability to analyze blocks of C\# code in Blazor components\. Some bugs found in such components are described in [this article](https://pvs-studio.com/en/blog/posts/csharp/1023/)\.