Виталий Алферов

10 Фев 2016

Теги:

#CSharp

Проверяем исходный код WPF примеров от компании Infragistics

10 Фев 2016

Автор: Виталий Алферов

WPF ошибки
Неспециализированные C# ошибки
Заключение

Мы продолжаем проверять различные C#-проекты с целью демонстрации возможностей статического анализатора кода PVS-Studio. В этой статье мы рассмотрим результаты проверки WPF примеров от компании Infragistics. Сама компания Infragistics является глобальным поставщиком программного обеспечения, основанная в 1989 году. Компания сделала себе имя на разработке компонентов пользовательских интерфейсов для сторонних разработчиков на всех платформах, включая .NET.

В PVS-Studio 6.00 мы в основном реализовали C# диагностики общего плана, опираясь на опыт созданного нами ранее C++ анализатора. Начиная с PVS-Studio 6.01, мы приступили к созданию диагностик, специализированных именно для языка С#. Для старта были выбраны свойства зависимости (DependencyProperty), использующиеся в WPF проектах. На DependencyProperty выбор пал по причине сложности их создания. Сложность в том, что очень легко допустить опечатку в однотипном коде, к которому тяготеют WPF проекты. Нами был разработан ряд диагностик [3044, 3045, 3046, 3047, 3048, 3049], специализирующихся именно на анализе и проверке подобных свойств.

Как известно, одной из особенностей DependencyProperty является то, что в большинстве случаев любая ошибка при их регистрации приводит к падению программы на этапе выполнения (в runtime). Волей-неволей программисты исправляют подобные ошибки, вновь и вновь запуская программу. Тем самым на поиски опечатки в шаблонном коде создания DependencyProperty, тратятся драгоценные минуты, а суммарно - целые часы. При этом, как показала практика проверки WPF проектов, не все ошибки в свойствах зависимости очевидны после первого запуска программы.

Первым испытуемым кодом для данных диагностик стал код тестовых примеров от компании Infragistics. Архив был загружен 02.02.2016, количество проектов достигает 11-ти штук, но их можно скачать все одним архивом.

Проверка исходного кода выполнена с помощью статического анализатора PVS-Studio 6.01.

WPF ошибки

Многие проекты построены на основе общего переиспользуемого кода, в котором и найдено большинство ошибок.

Ошибка N1

В проекте "IGExtensions.Common.WPF" в файле "LambertConformalConic.cs" была найдена следующая строка регистрации "DependencyProperty":

public static readonly DependencyProperty CentralMeridianProperty
 = DependencyProperty.Register("CentralMeridianProperty",
    typeof(double), typeof(LambertConformalConic),
      new PropertyMetadata(0.0,
        new PropertyChangedCallback(UpdateConstants)));

V3045 WPF: the names of the registered property 'CentralMeridianProperty', and of the property 'CentralMeridian', do not correspond with each other. LambertConformalConic.cs 130

Как можно заметить, при регистрации DependencyProperty в его имени было указанно "CentralMeridianProperty" вместо "CentralMeridian". Ошибка совершается довольно часто, вследствие копирования имени переменной, но таит в себе следующую опасность.

А именно - для записи/чтения в свойство зависимости из C# кода создают следующее свойство-прослойку:

public double CentralMeridian {
  get { return (double)GetValue(CentralMeridianProperty);  }
  set { SetValue(CentralMeridianProperty, value); } 
}

При обращении из xaml разметки, binding пишут на свойство "CentralMeridian". WPF достаточно умный, чтобы найти свойство CentralMeridian и прочитать изначально значение оттуда, но вот изменения значения в CentralMeridian, естественно, подхватываться не будут.

Ошибка N2

Продолжая тему опечаток в именах регистрируемых свойств зависимости рассмотрим следующую ошибку в файле "TransverseMercator.cs" проекта "IGExtensions.Common.WPF".

public static readonly DependencyProperty CentralMeridianProperty
  = DependencyProperty.Register("LongitudeOrigin", typeof(double),
     typeof(TransverseMercator), new PropertyMetadata(0.0,
       new PropertyChangedCallback(UpdateConstants)));

public double CentralMeridian { .... }

V3045 WPF: the names of the registered property 'LongitudeOrigin', and of the property 'CentralMeridian', do not correspond with each other. TransverseMercator.cs 95

Как показывает практика, свойства зависимостей прописывают скопом, с помощью множественного дублирования одной строки и последующими правками. Другими словами, с помощью Copy-Paste. И нередко где-то в этом блоке однотипного кода пропускают одну переменную и прописывают ей другое имя, которое было рядом в списке. А учитывая, что сам список обычно находится где-нибудь в блокноте [Notepad, Notepad++, Sublime Text и т.д.] рядом в другом окне, проверить создание всех необходимых объектов можно только глазами. Выявлять подобные ошибки очень сложно, т.к. код вполне рабочий, но правда, лишь на половину.

Ошибка N3

С ошибками в именах регистрируемых свойств вроде всё ясно, но где еще можно ошибиться при создании DependencyProperty? Ну, например, в типах значений, которые должны в них содержаться, как в файле "PropertyBrushColorEditor.cs" всё того же проекта "IGExtensions.Common.WPF".

public static readonly DependencyProperty BrushColorProperty = 
  DependencyProperty.Register(BrushColorPropertyName, 
    typeof(Brush), typeof(PropertyBrushColorEditor), 
      new PropertyMetadata(null, (sender, e) => 
      {....})
);

public SolidColorBrush BrushColor
{
 get { return (SolidColorBrush)GetValue(BrushColorProperty); }
 set { SetValue(BrushColorProperty, value); }
}

V3046 WPF: the type registered for DependencyProperty does not correspond with the type of the property used to access it.

Если у вас не возникает вопросов, почему неправильно указывать родительских класс " Brush" при регистрации, а в обращении через свойство "BrushColor" указывать наследника "SolidColorBrush", то это хорошо. В противном случаем опишем упрощенный случай такой "игры" с хранимыми типами.

Например, представим простой случай. Создадим простой WPF проект и добавим в класс окна следующее свойство зависимости:

public static DependencyProperty MyIndexProperty =
  DependencyProperty.Register("MyIndex", typeof(int),
  typeof(MainWindow), new FrameworkPropertyMetadata(1));

int MyIndex
{
 get { return (int)GetValue(MyIndexProperty); }
 set { SetValue(MyIndexProperty, value); }
}

В xaml разметке напишем следующее:

....

Title="MainWindow" Height="350" Width="525"
DataContext="{Binding RelativeSource = 
               {RelativeSource Mode=Self}}">
<Grid>
  <Grid.RowDefinitions>
      <RowDefinition Height="Auto" />
      <RowDefinition Height="Auto" />
      <RowDefinition Height="Auto" />
  </Grid.RowDefinitions>
  <TextBlock Grid.Row="0" Text="{Binding Path=MyIndex}"/>
  <Slider Grid.Row="1" Name="slider1" 
    Value="{Binding Path=MyIndex}" Maximum="100" />
    <Button Grid.Row="2" Click="Button_Click">
      Прочитать значение
    </Button>
</Grid>

И добавим в класс окна код на событие нажатия кнопки:

private void Button_Click(object sender, RoutedEventArgs e)
{
  this.Title = this.MyIndex.ToString(); 
}

Всё. Как можно убедиться, всё работает? Двигаем слайдер, число изменяется. Нажимаем на кнопку, и заголовок окна тут же поменял свой текст на текущее значение в слайдере. Кстати, как можно заметить, в TextBlock-е отображаются целочисленные значения.

А теперь заменим при регистрируемом DependencyProperty тип с "int" на общий тип "object":

public static DependencyProperty MyIndexProperty =
  DependencyProperty.Register("MyIndex", typeof(object),
  typeof(MainWindow), new FrameworkPropertyMetadata(1));

Остальное оставим без изменений и снова запустим программу.

Программа запустилась и теперь, когда мы двигаем слайдер, в TextBlock-е отображаются вещественные значения. Как легко догадаться, если мы попробуем нажать кнопку - программа просто упадет, т.к. она не сможет сконвертировать вещественное значение в MyIndexProperty в целочисленное в свойстве MyIndex. Казалось бы, мелочь, но она привела к печальным последствиям.

Ошибка N4

Кроме вышеприведенных ошибок, которые относятся к большинству проектов (что особенно печально, ибо их ни разу не заметили и не исправили ни в одном проекте), есть ошибки локальные для одного проекта IGEquityTrading.WPF:

public static readonly DependencyProperty
 AxisFinancialIndicatorYTemplateProperty =
  DependencyProperty.Register("AxisFinancialIndicatorYTemplate",
    typeof(DataTemplate),
    typeof(DataChartEx),
    new PropertyMetadata(default(DataTemplate)));

public DataTemplate AxisCategoryYTemplate{
 get { return (DataTemplate)
  GetValue(AxisFinancialIndicatorYTemplateProperty); }
 set { 
  SetValue(AxisFinancialIndicatorYTemplateProperty, value); }
}

V3045 WPF: the names of the property registered for DependencyProperty, and of the property used to access it, do not correspond with each other. DataChartEx.cs 469

И снова Infragistics наступает на те же грабли, вместо регистрируемого имени "AxisFinancialIndicatorYTemplate" создают свойство с именем "AxisCategoryYTemplate".

Ошибка N5

public static readonly DependencyProperty
 FinancialIndicatorSeriesTemplateProperty =
  DependencyProperty.Register("FinancialIndicatorTemplate",
    typeof(DataTemplate),
    typeof(DataChartEx),
    new PropertyMetadata(default(DataTemplate)));

public DataTemplate FinancialIndicatorSeriesTemplate {
 get { return (DataTemplate)
    GetValue(FinancialIndicatorSeriesTemplateProperty); }
 set { 
    SetValue(FinancialIndicatorSeriesTemplateProperty, value); }
}

V3045 WPF: the names of the property registered for DependencyProperty, and of the property used to access it, do not correspond with each other. DataChartEx.cs 344

В последнем случае ошибка возникла, скорее всего, после рефакторинга, когда решили конкретизировать переменную, и вставили в середину фразы "FinancialIndicatorTemplate" слово "Series". Самое интересное, что поменяли везде, даже в XAML разметке и в "#region", но вот в имени регистрируемого свойства забыли.

....\Infra\EquityTrading\IGEquityTrading.WPF\App.xaml(123): <DataTemplate x:Key="FinancialIndicatorSeriesTemplate">
....\Infra\EquityTrading\IGEquityTrading.WPF\App.xaml(214): FinancialIndicatorSeriesTemplate="{StaticResource FinancialIndicatorSeriesTemplate}"
....\Infra\EquityTrading\IGEquityTrading.WPF\Controls\DataChartEx.cs(189): var financialIndicator = FinancialIndicatorSeriesTemplate.LoadContent() as Series;
....\Infra\EquityTrading\IGEquityTrading.WPF\Controls\DataChartEx.cs(330): #region FinancialIndicatorSeriesTemplate (DependencyProperty)
....\Infra\EquityTrading\IGEquityTrading.WPF\Controls\DataChartEx.cs(336): public DataTemplate FinancialIndicatorSeriesTemplate
....\Infra\EquityTrading\IGEquityTrading.WPF\Controls\DataChartEx.cs(349): #endregion FinancialIndicatorSeriesTemplate (DependencyProperty)
....\Infra\EquityTrading\IGEquityTrading.WPF\Controls\StockHistoryChart.xaml(646): FinancialIndicatorSeriesTemplate="{StaticResource FinancialIndicatorSeriesTemplate}"

При этом зарегистрированное имя "FinancialIndicatorTemplate" нигде не используется, а к чему приводит такая оплошность, мы уже знаем.

Неспециализированные C# ошибки

В данных сборках от компании Infragistics не нашлось больше WPF ошибок. Как уже говорилось, большинство WPF диагностик рассчитаны именно на предварительное нахождение ошибок еще до компиляции и запуска проекта. А данные проекты с примерами использования компонентов уже были проверены программистами и QA специалистами. Дополнительно проекты смотрели пользователи, которые оценили качество и удобство использования продукта именно по тестовым примерам. Думаю, если они замечали ошибку, то, скорее всего, уведомляли разработчиков.

Естественно, кроме WPF ошибок, в данных сборках есть и другие. Анализатор выдал в сумме несколько сотен предупреждений. Далеко не все из сообщений указывают на настоящие ошибки. Многие предупреждения (например, про сравнение типа double с константой) просто не актуальны для данного типа проектов. Это не страшно, так как анализатор предоставляет несколько механизмов подавления неинтересных сообщений.

В любом случае предупреждений много, и большая их часть свидетельствует об аномалиях в коде. Это настоящие ошибки или код "с запахом". Поэтому рекомендуем авторам проекта самостоятельно выполнить проверку проекта и изучить предупреждения. Здесь же мы пройдемся только по самым интересным из них:

public bool IsValid
{
get {
  var valid = 
    double.IsNaN(Latitude) || double.IsNaN(Latitude) ||
    this.Weather.DateTime == Weather.DateTimeInitial;
  return valid;
 }
}

V3001 There are identical sub-expressions 'double.IsNaN(Latitude)' to the left and to the right of the '||' operator. WeatherStation.cs 25

Программистам жить тяжело. Они должны разбираться не только в самом программировании, но и в той области, где должна работать программа. Вот и получается, что они должны разобраться в предметной области и знать такие слова, как "вира" (Вверх), "майна" (Вниз), "Latitude"(Широта), "Longitude"(Долгота) и т.д. Это только добавляет сложности, особенно если понятия схожи по написанию. Вот и получается, что ошибочно пишем проверку одной и той же переменной: double.IsNaN(Latitude) || double.IsNaN(Latitude).

Следующая ошибка:

private static int clipSegment(....)
{
 if (xmax > rc.Right && xmax > rc.Right)
 {
   return -1;
 }
}

V3001 There are identical sub-expressions 'xmax > rc.Right' to the left and to the right of the '&&' operator. Geometry. Geometry.CubicSpline.cs 529

Проверка того, в каких пределах лежит переменная - дело привычное, но вот ошибиться в знаках, а после - и в переменной, которую нужно написать, довольно легко. На самом деле для исключения подобных ошибок просто нужно придерживаться следующего паттерна: общая переменная в условии пишется с разных сторон в выражениях.

if (xmin < rc.Right && rc.Right < xmax)

Ошибиться сложнее и читается гораздо легче.

P.S. Правда подобный фокус не работает в Entity Framework, при конвертации LINQ кода в SQL-запрос программа упадет. Вот такие вот дела :)

На самом деле в данном проекте Infragistics что-то перемудрил с такими проверками. Кроме выше приведённого кода, подобная ошибка повторилась в следующих строчках:

private static int clipSegment(....)
{
  ....
  if (ymin < rc.Top && ymin < rc.Top) // <= здесь
  ....
  if (ymax > rc.Bottom && ymax > rc.Bottom) // <= и здесь
  ....
}

Диагностике V3001 мало этих ошибок, и она продолжает экспансию по проекту :). Вот очередной пример её срабатывания:

private static bool IsInDesignModeStatic(this Application app)
{
 ....
  if (_isInDesignMode != null && _isInDesignMode.HasValue) 
   return _isInDesignMode.Value;
 ....
}

V3001 There are identical sub-expressions '_isInDesignMode != null' to the left and to the right of the '&&' operator. NavigationApp.cs 415

В данном случае мы имеем дело не с ошибкой, а с избыточным кодом. Достаточно было написать:

if (_isInDesignMode.HasValue)

Еще одно срабатывание V3001 отправляет нас к проекту "IgWord.Infrastructure":

void ParagraphSettingsPreviewAdapter_PropertyChanged(
 object sender, PropertyChangedEventArgs e) {
 ....
 if (LineSpacingType == Infrastructure.LineSpacingTypes.Exactly 
  || LineSpacingType == Infrastructure.LineSpacingTypes.Exactly){
 ....
}

V3001 There are identical sub-expressions 'LineSpacingType == Infrastructure.LineSpacingTypes.Exactly' to the left and to the right of the '||' operator. ParagraphSettingsPreviewAdapter.cs 268

Что конкретно здесь хотел сделать разработчик, не очень понятно, но явно не то, что написано.

От V3001, в порядке очереди, прейдем к V3010.

В проекте "IGEarthQuake.WPF", есть пара вызовов функций:

public MapViewModel() {
  ....
  WeakPropertyChangedListener.CreateIfNecessary(_service, this);
  ....
}

V3010 The return value of function 'CreateIfNecessary' is required to be utilized. MapViewModel.cs 42

public TimeLineViewModel(){
  ....
  WeakPropertyChangedListener.CreateIfNecessary(_service, this);
  ....
}

V3010 The return value of function 'CreateIfNecessary' is required to be utilized. TimeLineViewModel.cs 50

Собственно, в обоих случаях вызывается одна функция, и она довольно простая. Давайте посмотрим на её реализацию:

public static 
WeakPropertyChangedListener CreateIfNecessary(object source,
IPropertyChangedListener listener){
  INotifyPropertyChanged inpc = source as INotifyPropertyChanged;
  return inpc != null ? 
    new WeakPropertyChangedListener(inpc, listener) : null;
}

Действительно, как можно увидеть, никаких глобальных изменений функция не производит, и её результат тоже не используется. Вот и возникает вопрос - а зачем её вообще звали? Подозрительно всё это...

Подобный пример нас ждет и в проекте "IGHospitalFloorPlan.WPF":

private void ParseAllShapefiles() {
  ....
  this.ShapeFilesMaxBounds.Expand(new Thickness(10, 10, 10, 10));
  ....
}

V3010 The return value of function 'Expand' is required to be utilized. HospitalView.xaml.cs 52

Её реализация чуть хитрее, но в итоге просто возвращает новый объект, который нигде не используется.

Мы находимся в середине статьи. Вот посмотрите на картинку. Расслабьтесь, а потом мы продолжим.

Одним из самых распространённых типов ошибок является неудачный Copy-Paste кода:

public static EsriMapImageryView 
   GetImageryView(EsriMapImageryStyle imageryStyle){
 ....
  if (imageryStyle ==
    EsriMapImageryStyle.UsaPopulationChange2010Overlay)
 return EsriMapImageryViews.UsaPopulationChange2010Overlay;
  if (imageryStyle ==
    EsriMapImageryStyle.UsaPopulationChange2010Overlay)
 return EsriMapImageryViews.UsaPopulationChange2010Overlay;
 ....
}

V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless EsriMapImageryView.cs 97

В данном случае, под одним и тем же условием находится одинаковый код. На данном этапе ошибка представляет собой неудачный (избыточный) Copy-Paste-е кода. Но после рефакторинга может возникнуть ситуация, что программист изменит тело нижестоящей инструкции if, которая никогда не выполняется, что приведет к ошибкам в логике программы.

И посмотрим, какие еще ошибки встречаются в коде компании Infragistics.

На каждую ниже идущую строку было выдано предупреждение V3022:

public static double GenerateTemperature(GeoLocation location){
  ....
  else if (location.Latitude > 10 || location.Latitude < 25) 
  ....
  else if (location.Latitude > -40 || location.Latitude < 10)
  ....
}

public static WeatherCondition GenerateWeatherCondition(....){
  ....
  else if (location.Latitude > 10 || location.Latitude < 25)
  ....
  else if (location.Latitude > -40 || location.Latitude < 10)
  ....
}

Всё ошибки сводятся к следующему сообщению:

V3022 Expression 'location.Latitude > -40 || location.Latitude < 10' is always true. Probably the '&&' operator should be used here.

Что тут можно сказать? Наверно то же самое, что и выше, при описании одной из ошибок V3001. Полезно использовать паттерн, когда одна и та же переменная написана с разных сторон в выражении:

if (xmin < rc.Right && rc.Right < xmax)

На этом описание ошибок первого уровня закончим и перейдем ко второму и третьему, т.к. один и тот же номер сообщения, в зависимости от ситуации, имеет различный приоритет.

На третьем уровне собраны диагностические сообщения, которые выдаёт анализатор, когда слабо уверен в своей правоте. Также на 3 уровень попадают диагностики, которые актуальны не для всех проектов.

На практике предупреждения 3 уровня редко выявляют настоящие ошибки. Часто это всё же ложные срабатывания или сообщения, которые могут указывать хотя и на корректный код, но имеющий "запах". В любом случае, если есть время, эти диагностики стоит изучить и провести рефакторинг кода.

Начнем с кода с двумя одинаковыми функциями:

// 0 reference
public static double Ramp(double a) {
  return a - Math.Floor(a);
}

// 1 reference
public static double Frac(double a) {
  return a - Math.Floor(a);
}

V3013 It is odd that the body of 'Ramp' function is fully equivalent to the body of 'Frac' function (28, line 33). Math.cs 28

Если функция Frac имеет хоть какой-то смысл, т.к. подобная функция была в Pascal, но вот функция Ramp не имеет никакого аналога, или я их не нашёл. Собственно, счетчики количества мест, где используются функции, говорят сами за себя (см. комментарии).

Дабы далеко не уходить от ошибки V3013, рассмотрим случай, когда данная ошибка появилась на втором уровне.

public void StartCurrent()
{
  StartTask("Current");
}
public void StopCurrent()
{
  StartTask("Current");
}

V3013 It is odd that the body of 'StartCurrent' function is fully equivalent to the body of 'StopCurrent' function (503, line 507). DataViewModel.cs 503

По всей видимости, во втором случае перепутали функцию "StartTask" с "StopTask". Эти обе функции есть в коде, и действуют они довольно однозначно и согласно их названиям.

Далее рассмотрим серию сообщений, связанных с нижеприведенным кодом:

{
  IsUpdating = true;
  ....
  IsUpdating = false;
}

Подобный код встречается аж 4-х местах (в каждой из сборок)

V3008 The 'IsUpdating' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 201, 195. GeoRegion.cs 201
V3008 The 'IsUpdating' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 212, 205. GeoRegion.cs 212
V3008 The 'IsUpdating' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 226, 216. GeoRegion.cs 226
V3008 The 'IsUpdating' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 244, 236. GeoRegion.cs 244

Изначально кажется, что данная переменная используется для межпотокового взаимодействия. Но как выяснилось, на практике данная переменная нигде не встречается, кроме строк, для которых сработала диагностика.

Хорошо, предположим, что эту переменную решат использовать для межпотоковой синхронизации. И вот тут как раз нас ждет неприятный сюрприз. Объявление переменой выглядит следующим образом:

protected bool IsUpdating = false;

Как видите, нет ключевого слова "volatile", а как следствие, компилятор с успехом её оптимизирует и работать будет совершенно не так, как хотелось бы.

Что еще интересного нашлось в коде? Например, лишние вычисления:

Пример 1:

public static void Normalize(....)
{
  var x = rect.X < boundingRect.X ? boundingRect.X : rect.X;
  x = (rect.X + rect.Width) > boundingRect.Right ? 
     boundingRect.X : rect.X;
}

V3008 The 'x' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 96, 95. RectEx.cs

Пример 2:

private static GradientStopCollection fromInterpolation(....){
 ....
 Color color=ColorTool.FromAHSV(ahsv[0], 
                                ahsv[1], 
                                ahsv[2], 
                                ahsv[3]);
 color = ColorTool.FromARGBInterpolation(min, p, max[i].Color);
 ....
}

V3008 The 'color' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 165, 163. BrushTool.cs

Иногда встречаются вообще забавные фрагменты кода:

private void UpdateAutoSavedState() {
  AutoSaved = true;
  AutoSaved = false;
}

V3008 The 'AutoSaved' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 691, 690. ShellViewModel.cs 691

Для тех, кто сомневается в данном коде привожу объявление свойства:

private bool autoSaved;
public bool AutoSaved
{
  get { return autoSaved; }
  set { autoSaved = value; }
}

И опять ни "volatile", ни чего-то подобно, что говорило бы о скрытом смысле данного действия - нет.

Перейдем еще к одной группе строк с ошибкой V3029:

public void OnPropertyChanged(PropertyChangedEventArgs ea) {
 ....
 var index = this.SelectedBrushCollectionIndex;
 ....
 if (index >= 0) 
  DebugManager.LogData(this.BrushCollectionList[index].ToText());
 if (index >= 0) 
  this.SelectedBrushCollectionIndex = index;
 ....
}

V3029 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 338, 339.

public static void EnableSeriesMouseDoubleClick(
  this XamGeographicMap geoMap, bool isEnabled = true){
  ....
  if (geoMap != null) geoMap.SeriesMouseLeftButtonDown +=
    OnSeriesMouseLeftButtomDown;
  if (geoMap != null) geoMap.SeriesMouseLeftButtonUp +=
    OnSeriesMouseLeftButtonUp;
  ....
  if (geoMap != null) geoMap.SeriesMouseLeftButtonDown -=
    OnSeriesMouseLeftButtomDown;
  if (geoMap != null) geoMap.SeriesMouseLeftButtonUp -=
    OnSeriesMouseLeftButtonUp;
  ....
}

V3029 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 92, 93. GeoMapAdapter.cs 92

V3029 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 100, 101. GeoMapAdapter.cs 100

public void SyncSeriesViewPropertyChanges() {
  if (this.SeriesView != null) 
    this.SeriesView.PropertyUpdated += OnSeriesViewPropertyUpdated;
  if (this.SeriesView != null) 
    this.SeriesView.PropertyChanged += OnSeriesViewPropertyChanged;
}

V3029 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 342, 343. GeoSeriesLayer.cs 342

Как говорится - "на всякий случай, а то вдруг"...

Хотя это и не ошибки, но повторные поверки загромождают код и усложняют его понимание.

А вот избыточный код, который возник скорее всего во время рефакторинга.

public Frame NavigationTarget
{
 get { return (Frame)this.GetValue(NavigationTargetProperty); }
 set {
  var targetFrame = value as Frame;
  if (targetFrame != null)
    this.SetValue(NavigationTargetProperty, value);
 }
}

"value" и так имеет тип Frame, приведение бессмысленно. Но данном случае здесь надо рассматривать ситуацию в более в широком смысле. Infragistics производит проверку на null при записи в DependencyProperty. Для подобных проверок разработчиками свойств зависимости была предусмотрена функция обратного вызова "ValidateValueCallback". Данная функция проверяет значения, которые записываются в DependencyProperty, а задается она при регистрации свойства зависимости.

Заключение

В очередной раз наш радужный единорог в сияющих доспехах выявил немалое количество проблемных мест (в статье перечислены далеко не все ошибки, которые мы обнаружили). Да, теперь разработчики могут исправить код и сделать его более качественным, чем был... Чем был, когда его писали... Чем был, когда его тестировали... Чем был, когда его переписывали, запускали, а он вновь и вновь падал или работал не так, как нужно...

В мой практике на предыдущей работе, были авралы по выходным и по ночам, за несколько дней до дедлайна, когда нужно было сделать очень много и очень быстро. Вся команда понимала, что нужно делать, но спешка и усталость давали о себе знать, и больше времени уходило именно на отладку. Т.е. мы пишем код, запускаемся, а он работает не так, как задумывалось. Останавливаем всё, ставим брейкпоинт и снова запускаемся. Выполняем все действия для повторения, приходим в точку останова и строчку за строчкой проверяем и смотрим, что в ней происходит. Прыгая по коду туда-сюда и просматривая значения в переменных. А в итоге оказывается, что где-то в условии перепутали переменную или знак... Вот и получается, что на банальную опечатку при Copy-Paste-е уходит 15 минут чистого времени...

Так что, сколько бы мы не проверяли проекты, сколько бы мы в них не находили ошибок, это всего лишь меленькая вершина огромного айсберга проблем, которые возникают при написании кода.

Никто не застрахован от ошибок. Даже создавая код, который должен служить образцом для подражания и визитной карточкой компании, невозможно избежать ошибок.

Мой вам искренний совет - пользуйтесь анализатором PVS-Studio постоянно. Для это в нем есть все инструменты. Например, есть режим, в котором он проверяет изменённые файлы, вам даже не надо его запускать, анализатор сам проверит, что нужно и сообщит о проблемах в них.

#CSharp