8 наиболее распространенных ошибок C# программистов

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

Я иногда почитываю dzone и периодически нахожу там довольно интересные статьи, и вот вчера, в разделе "Popular on DZone" наткнулся на любопытную статью под названием “8 Most common mistakes C# developers make”.

Я предлагаю вначале полистать вам ее самому, а уже потом смотреть мои комментарии.

.

.

.

.

.

.

Итак, в статье представлены 8 наиболее распространенных ошибок, которые допускают программисты на языке C#, давайте рассмотрим их более подробно.

1. String concatenation instead of StringBuilder

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

Буквальное следование подобным советам и параноидальная боязнь конкатенации строк приводит к тому, что StringBuilder начинают использовать даже для объединения нескольких строк, что приводит к падению читабельности без какого-либо выигрыша в производительности.

2. LINQ - 'Where' with 'First' instead of FirstOrDefault

Совет сводится к тому, чтобы нужно использовать метод FirstOrDefault вместо метода First или, в крайнем случае, использовать метод First, принимающий предикат, вместо использования пары методов Where/First.

С первого взгляда совет может показаться неплохим, но это не так. Как я уже неоднократно писал, код сам по себе не может быть корректным или не корректным; все зависит от того, что этот код должен делать.

Говорить о том, что следует использовать FirstOrDefault вместо First все равно, что советовать использовать ‘as’ вместо приведения типов ‘(T)’. У этих методов разная семантика, поэтому если мы знаем, что отсутствие определенного значения в последовательности является ошибкой, то метод First как раз то, что нам нужно.

Если же главной проблемой по мнению автора, заключается в использовании пары методов Where/First, то это другой вопрос. Во-первых, это совет из разряда микрооптимизаций; при написании подобного кода мы должны сосредоточиться прежде всего на его читабельности, а уже потом мерять производительность с помощью профилирования. Во-вторых, совет не совсем корректный, поскольку использование Where с последующим вызовом First обычно даже более эффективно по сравнению с вызовом метода First, принимающего предикат.

Все дело в том, что в методе Where используются специализированные итераторы для некоторых типов коллекций. Так, при использовании списков или массивов именно пара Where/First будет эффективнее по сравнению с методом First, принимающим делегат. Если же специализированного итератора нет, то эффективность обоих вариантов будет одинаковой.

// Список содержит довольно много элементов
var list = new List<int>() { };
// Первый вариант более эффективный
var value1 = list.Where(x => x == 42).FirstOrDefault();
var value2 = list.FirstOrDefault(x => x == 42);

В общем, совет достаточно бестолковый, да еще и нарушает Single Responsibility Principle, поскольку описывает две проблемы одновременно, при этом не описывает ни одну из них достаточно полно.

3. Casting by means of '(T)' instead of 'as (T)' when possibly not castable

Это совет, как и предыдущий, очень спорен. Он сводится к тому, что мы должны предпочесть использование преобразования типов с помощью ‘as’ вместо стандартного преобразования вида ‘(T)’, аргументируя это тем, что ‘as’ более эффективный и может применяться даже тогда, когда стандартное преобразование типов невозможно.

Во-первых, существует очень мало случаев, когда мы можем использовать преобразование типов с помощью ‘as’, но не можем использовать преобразование типов с помощью ‘(T)’: например, при использовании обобщений (о чем совсем недавно писал Эрик Липперт). Поскольку это явно крайний случай, на его основе не следует делать выводы об использовании одного способа преобразования типов по сравнению с другим.

Во-вторых, мы, опять таки, не можем говорить о корректности одного варианта преобразования типов над другим, поскольку они предназначены для разных целей. Использование ‘(T)’ говорит читателю нашего кода, что вы «знаете» о конкретном типе объекта, и если он не совпадает, то это является ошибкой. ‘as’ является безопасным преобразованием типов, при этом у нас появляется возможность “справиться” с неизвестным типом самостоятельно.

Я бы, например, дал противоположный совет: подумайте об использовании ‘(T)’ вместо ‘as’, поскольку очень часто используется «безопасное» приведение типов без последующей проверки результата на null.

EventArgs e = null;
var a = e as CustomEventArgs;
Console.WriteLine(a.Value);

Наш код должен максимально четко передавать намерения программиста и использование правильных методов (First vs FirstOrDefault) или языковых конструкций (‘as’ vs ‘(T)’) являются ключевым механизмом в этом деле. При этом я бы советовал делать контракты максимально жесткими и не захламлять свой код десятками проверок на null там, где делать этого не следует.

4. Not using mapping for rewriting properties

Наличие или отсутствие рекомендуемых мапперов является вопросом дизайна (причем достаточно субъективным) и может быть актуальным лишь для определенного класса задач. Отсутствие мапперов само по себе не является проблемой, если это не приводит к дублированию кода или падению сопровождаемости. Вот нарушение принципа DRY (Don’t Repeat Yourself) и интенсивное использование паттерна “Copy-Paste” являются более общей проблемой, на которую следует обратить внимание.

Мне не нравятся подобные советы из-за их субъективности и узости применения. Подобных частных случаев слишком много, чтобы стараться их запомнить; значительно эффективнее следовать более общим принципам проектирования и стараться использовать свои знания повторно.

5. Incorrect exception re-throwing

Один из немногих советов, к которому у меня нет никаких вопросов.

6. Not using ‘using’ for objects disposal

Это второй адекватный совет, пусть и из серии КО.

7. Using ‘foreach’ instead of ‘for’ for anything else than collections

Мне казалось, что споры типа for vs foreach уже давно в прошлом, поэтому я искренне удивлен увидеть подобный совет в статье 2013 (!) года, аргументируя совершенно невнятной статьей 2004-го года.

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

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

8. Retrieving or saving data to DB in more than 1 call

Общеизвестные принципы проектирования, такие как связность и связанность (coupling и cohesion) можно применять не только при проектировании классов и модулей, но и при написании технических статей.

У этой статьи явная проблема со связностью (она у нее очень низкая), поскольку советы слабо коррелируют между собой. Совет о правильной работой с базами данных выглядит подозрительным среди советов об обработке исключений и циклами foreach. И хотя рекомендация сама по себе выглядит разумной она слишком общая, чтобы можно было ее каким-то образом применять на практике.

Заключение

Печально видеть в 2013 году советы из разряда «используйте for вместо foreach», особенно на вполне адекватных сайтах, типа dzone. Основная цель моей статьи в том, чтобы вы думали, а не принимали советы на веру (да, и мои советы в том числе).

У большинства рекомендаций есть исключения и важно понимать основные «за» и «против». Тем более с осторожностью следует относиться к советам, связанных с эффективностью или корректностью применения тех или иных языковых средств без соответствующего контекста, поскольку это позднее может сыграть с вами злую шутку.