# Пять забавных странностей в коде Entity Framework Core
Разработкой Entity Framework Core занимаются профессионалы, не допускающие ошибок благодаря опыту, ревью и мощной системе тестирования\. Впрочем, даже в таком проекте не без приколов\. Перед вами статья о странностях, укрывшихся от взгляда разработчиков высшего класса\.
## Мини\-введение
Обычно мы пишем статьи, в которых разбираем целый список различных ошибок, найденных с помощью PVS\-Studio в открытым проекте\. Однако в этот раз такая статья не получится\. Код в проекте действительно хорошо написан :\)\. Так что, когда я писал про профессионализм, это был не сарказм\.
Это тот случай, когда я действительно проверил проект, но не могу написать классическую статью про разбор ошибок\. Есть интересные аномалии, про которые я расскажу, но в целом код выглядит очень чистым, качественным, и анализатор почти не выдаёт срабатываний, указывающих на явные проблемы\.
Для поиска странностей и потенциальных ошибок я использовал анализатор PVS\-Studio 7\.26\. Версия самого Entity Framework Core — [7\.0\.10](https://github.com/dotnet/efcore/releases/tag/v7.0.10)\. Топ составлялся совершено субъективно, поэтому если ваше мнение по какому\-то вопросу отличается — пишите в комментариях, обсудим\.
## 5 место\. Несогласованность
```cpp
protected virtual ModelBuilder VisitDatabaseModel(....)
{
....
if (!string.IsNullOrEmpty(databaseModel.DatabaseName)) // <=
{
modelBuilder.Model.SetDatabaseName(
!_options.UseDatabaseNames
&& !string.IsNullOrEmpty(databaseModel.DatabaseName) // <=
? _candidateNamingService.GenerateCandidateIdentifier(....)
: databaseModel.DatabaseName
);
}
....
}
```
Первое срабатывание из топа сразу же отражает идею, лежащую в основе большинства странностей, которые я нашёл в Entity Framework Core\. А именно — удивительная несогласованность кода, причём именно фрагментов, находящихся буквально по соседству\.
Сначала проверяется, не является ли _databaseModel\.DatabaseName_ пустой строкой или _null_\. Если там лежит вполне нормальное строковое значение, код решает проверить значение свойства ещё раз\. Зачем? Чего разработчик хотел этим добиться?
И ведь что самое странное — разница между этими проверками буквально 3 строчки\! Ну как так\-то?
Для тех, кто решил посчитать разницу в строчках
В приведённом мною примере разница — 4 строчки \(о да, это многое меняет :D\)\. Однако это лишь оттого, что для статьи код пришлось слегка отформатировать\. В "оригинале" вызовы _\!\_options\.UseDatabaseNames _и лишняя проверка _DatabaseName_ записаны в одну строку\.
Данный фрагмент найден с помощью следующего предупреждения анализатора:
[V3063](https://pvs-studio.ru/ru/docs/warnings/v3063/) A part of conditional expression is always true if it is evaluated: \!string\.IsNullOrEmpty\(databaseModel\.DatabaseName\)\.
## 4 место\. Модели нет, возьмите из неё схему
```cpp
protected virtual IEnumerable
GenerateModificationCommands(InsertDataOperation operation,
IModel? model)
{
....
if ( operation.ColumnTypes != null
&& operation.Columns.Length != operation.ColumnTypes.Length)
{
throw new InvalidOperationException(
RelationalStrings.InsertDataOperationTypesCountMismatch(
....,
FormatTable(operation.Table,
operation.Schema ?? model?.GetDefaultSchema())));
}
if (operation.ColumnTypes == null && model == null)
{
throw new InvalidOperationException(
RelationalStrings.InsertDataOperationNoModel(
FormatTable(operation.Table,
operation.Schema ?? model?.GetDefaultSchema())));
}
....
}
```
Ещё одно замечательное проявление несогласованности и недостатка вдумчивости при написании кода\. Действительно, какая там разница, про что вообще исключение, если можно просто скопипастить :\)\. В проверке мы удостоверились, что _model_ равна _null_, после чего кидаем исключение про то, что **модели нет**\. Почему бы при его конструировании не попробовать обратиться к модели? :\)
Погоди, там же использован оператор '?\\\.', всё ок\\\!
Дело не в том, что обращение к модели приведёт тут к выбрасыванию исключения\. Дело в том, что программист \(кажется\) написал этот код, не задумываясь о его смысле\. Учитывая, что мы говорим об одной из наиболее известных и широко используемых библиотек под \.NET, подобный подход к разработке смущает\.
Другая теория — возможно, в проверке допущена ошибка, и вместо '&&' нужно использовать '\|\|'\. Но это уже домыслы\.
Описанную тут странность я нашёл благодаря предупреждению правила [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/):
Expression 'model' is always null\. The operator '?\.' is meaningless\.
## 3 место\. Двойной максимум
```cpp
public virtual SqlExpression? Translate(....)
{
if (SupportedMethods.TryGetValue(method, out var sqlFunctionName))
{
RelationalTypeMapping? typeMapping;
List? newArguments = null;
if (sqlFunctionName == "max" || sqlFunctionName == "max") // <=
{
typeMapping = ExpressionExtensions.InferTypeMapping(arguments[0],
arguments[1]);
newArguments = new List
{
....
};
}
else
{
typeMapping = arguments[0].TypeMapping;
}
var finalArguments = newArguments ?? arguments;
return _sqlExpressionFactory.Function(....);
}
return null;
}
```
Вот тут уже может быть опаснее\. Похоже, для функции _max_ реализована какая\-то особая обработка\. Ну и подобное же хотели сделать для какой\-то ещё функции, судя по всему\. Предположу, что для функции _min_, но сложно сказать точно\.
Увы, но сейчас код выглядит так, будто переспрашивает: "А может, имя функции всё\-таки max?" \(в случае если первое сравнение вернуло _false_\)\. Не, ну а вдруг и правда что\-то поменялось? :\)
Использованный для поиска этой странности PVS\-Studio даёт по этому поводу такой комментарий:
[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'sqlFunctionName \=\= "max"' to the left and to the right of the '\|\|' operator\.
## 2 место\. Подозрительный успех
```cpp
public override Expression Visit(Expression expression)
{
....
if ( TryGetPartitionKeyProperty(entityType,
out var partitionKeyProperty)
&& entityTypePrimaryKeyProperties.SequenceEqual(queryProperties)
&& (partitionKeyProperty == null || ....)
&& ....)
{
var propertyParameterList = queryProperties.Zip(....);
var readItemExpression = new ReadItemExpression(entityType,
propertyParameterList);
return ....;
}
....
}
```
Предлагаю внимательно вглядеться в код\. Видите ли вы странность, скрытую в нём?
Ну, я тоже не вижу :\)\. На самом деле какой\-то ошибки тут, может, и правда нет, но всё\-таки случай любопытный\.
Необычность здесь состоит в вызове функции _TryGetPartitionKeyProperty_\. По идее, она работает по обычному паттерну Try\-методов — возвращает _true_, если операция успешна, и _false_ в противном случае\.
Если вы подумали так же, как и я, то поздравляю — код вас обманул\. Ведь метод _TryGetPartitionKeyProperty_ выглядит так:
```cpp
static bool TryGetPartitionKeyProperty(IEntityType entityType,
out IProperty partitionKeyProperty)
{
var partitionKeyPropertyName = entityType.GetPartitionKeyPropertyName();
if (partitionKeyPropertyName is null)
{
partitionKeyProperty = null;
return true;
}
partitionKeyProperty = entityType.FindProperty(partitionKeyPropertyName);
return true;
}
```
Ну как бы да, функции, написанные успешными разработчиками, всегда отрабатывают успешно :\)\.
Спорный момент тут состоит в следующем\. Если значение _partitionKeyPropertyName_ не получено, то в out\-параметр явно записывается _null\._ При этом всё равно считается, что метод _TryGetPartitionKeyProperty_ отработал успешно \(что звучит странно, если вчитаться\)\. На этот момент можно взглянуть с двух сторон\.
С одной стороны, тут ничего страшного, ведь вызывающий код вполне готов к тому, что в out\-параметр будет записан _null_:
```cpp
if ( TryGetPartitionKeyProperty(entityType,
out var partitionKeyProperty)
&& entityTypePrimaryKeyProperties.SequenceEqual(queryProperties)
&& (partitionKeyProperty == null || ....)
&& ....)
```
Да, метод можно было бы переписать так, чтобы он просто возвращал _IProperty_ \(безо всяких Try, out\-параметров и возвращения логического значения\)\. Но как бы вроде и не страшно\.
С другой стороны, не совсем понятно, на что именно рассчитывает эта проверка на _null_\. К примеру, она могла быть добавлена на случай, если _null_ вернёт вызов _entityType\.FindProperty\(partitionKeyPropertyName\)_\.
Я склоняюсь к тому, что тут что\-то не так\. Ну очень уж странно, что метод с названием _TryGetPartitionKeyProperty_ возвращает _true _\(как бы говоря: "Да, мы всё успешно получили\!"\), но при этом сам выставляет _null_ в соответствующий out\-параметр\.
На этот спорный случай я наткнулся благодаря предупреждению PVS\-Studio:
[V3063](https://pvs-studio.ru/ru/docs/warnings/v3063/) A part of conditional expression is always true if it is evaluated: TryGetPartitionKeyProperty\(entityType, out var partitionKeyProperty\)\.
## 1 место\. Да кому нужен этот ef\.exe?
```cpp
protected override int Execute(string[] _)
{
....
var targetPlatformIdentifier = startupProject.TargetPlatformIdentifier!;
if ( targetPlatformIdentifier.Length != 0
&& !string.Equals(targetPlatformIdentifier, "Windows", ....))
{
executable = Path.Combine(
toolsPath,
"net461",
startupProject.PlatformTarget switch
{
"x86" => "win-x86",
"ARM64" => "win-arm64",
_ => "any"
},
"ef.exe");
}
executable = "dotnet";
args.Add("exec");
args.Add("--depsfile");
args.Add(depsFile);
....
return Exe.Run(executable, args, startupProject.ProjectDir);
}
```
Я бы сказал, что этот фрагмент позабавил больше других\. Сначала идёт суровая серьёзная проверка различных значений, после чего идёт не менее серьёзный функционал формирования пути с помощью сложного вызова _Path\.Combine_\. А затем результаты всего этого просто затираются :\)\. Теперь просто 'dotnet' и всё\.
И я ничего не имею против, но выглядит странно и забавно\. Если тут нет ошибки, то опять имеем какую\-то дикую несогласованность логики, будто в код вставляются рандомные строчки без оглядки на то, что происходит по соседству\. Возможно, это последствия неудачного слияния веток\.
Нашёл я этот фрагмент благодаря сообщению от правила [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/):
The 'executable' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 139, 127\.
## Мини\-заключение
Несмотря на все мои замечания, стоит признать, что Entity Framework и правда пишут мощные профессионалы\. Почти ни одно из представленных предупреждений не выглядит на 100% ошибкой, а я, можете поверить, очень старался найти самое лучшее\.
Конечно, можно сказать, что тут просто анализатор не очень справился, но как\-то так вышло, что в других проектах он вполне находит немало более очевидных проблем\. Кстати, если будет интересно попробовать этот инструмент, то его можно бесплатно получить [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.
Ну а меня на этом всё, надеюсь, вам было интересно :\)\.