srogatnev вчера в 07:06

Мне всё равно, какой у вас код-стайл

Простой

5 мин

8.2K

Блог компании КонтурC#*Программирование*

Мнение

Привет, Хабр. Меня зовут Рогатнев Сергей. Я работаю в Контуре ведущим разработчиком уже более 7 лет. За это время я поработал как минимум над десятью разными проектами в разных командах. Это были и проекты с историей на 10 лет, и стартапы, делающие свои первые шаги. Где-то я был всего 2–3 месяца, а где-то задерживался на пару лет. Такой формат работы позволил мне увидеть совершенно разные подходы к работе и написанию кода. За это время я адаптировался к переходам и смене команд, но мой собственный code style практически исчез, потому что нет двух команд с одинаковым стилем. Каждая команда обоснованно выбирает какой-то свой подход, довольно жестко придерживается этого подхода и очень редко пересматривает соглашения по code style, так как это приводит к каким-то холиварам.

В этой статье я хочу показать вам примеры таких холиваров, которые я встретил, работая над разными C#-проектами. И это не критика этих проектов, команд или их код-стайла. Моё отношение к этим холиварам примерно такое: «Давайте любой вариант, без разницы, не будем тратить время».

Скобочки

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

if (!includeDeleted)
    query = query.Where(f => f.State != FormState.Deleted);

А в других проектах – обязательно нужно ставить:

if (int.TryParse(match.Groups[groupIndex].Value, out var val))
{
    return val;
}

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

Переносы

Где-то можно так:

roles = organization.Roles.Select(role => role.ToString()).ToArray();

А где-то обязательно только так:

return rules
        .Select(CreateProvider)
        .ToArray();

Сюда же можно отнести любые переносы, не только в цепочках вызова LINQ-методов.

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

В целом, у меня такое отношение к любым переносам. Для меня код-стайл – это не переносы. Сам уже никогда не пишу на ревью про них.

Длина строчки кода…

…не больше 120 символов. Или 160 символов. Или 100. Или любого другого числа. Посмотрите, сколько у вас в проекте и знайте, что в соседнем будет другое значение.

Сам смотрю только на читаемость кода.

this

Обязательно ставим для методов класса:

this.Register(initialMessageBuilder);
this.Register(messageBuilder);

Или не ставим:

var manager = FindManager(form);

Как по мне – никакой разницы. Просто выберите любой вариант и автоматизируйте его.

Названия тестов

Тут безграничное количество вариантов:

Create_FlWithAttachedIpOrganization_Success
When_HandleProcessingResultTransactionEvent_Should_Ok
Validate_WhenNotFoundForm_ReturnNotFoundFormWithNullForm

Так же не встречал еще двух проектов, где был одинаковый подход к именованию тестов. Принимаю любой вариант, который есть в проекте. Во многом название теста для меня превратилось в идентификатор. Сам пишу примерно так ConvertAutoTransportTest. Из отличительного тут – слово Test в конце названия теста, чтобы в автокомплите или быстрее найти, или не спутать с «настоящим» методом.

Get/Read/Search

Много различий в использовании глаголов для работы с источниками данных. Например, методы репозиториев. Get, Read, Fetch, Search, Find, GetBatch, FindMany, GetAll, TryGet, TryRead, TryFetch, TrySearch… Сможете сказать, какой метод вернет 1 элемент, а какой коллекцию? Какой метод выкинет исключение, а какой нет? Разочарую вас и скажу, что однозначного ответа нет, и опыт в одном проекте никак не помогает в другом.

Any() vs Count

Пустую коллекцию обязательно проверяем через свойство Count:

public bool Empty()
{
    return properties.Count == 0;
}

А где-то только через LINQ:

if (uploads.Any())
{
    ...
}

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

IEnumerable

Используем тип IEnumerable для работы с коллекциями:

public interface IStorage
{
    Task<IEnumerable<Item>> BatchGet(IEnumerable<Guid> ids);
    Task BatchCreate(IEnumerable<Item> items);
}

Или используем только конкретный тип, например List<T>:

public interface IModulesExecutor
{
    Task<List<Offers>> Execute(List<Client> clients, List<OfferModule> module);
    Task<List<ComplexOffers>> Execute(List<Client> clients, List<ComplexOfferModule> module);
}

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

var items = await storage.BatchGet(ids);
var itemsList = items.ToList();

Или:

var clients = new Client[] { ... };
await modules.Execute(clients.ToList(), modules);

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

Порядок

У нас строгий порядок полей, свойств, методов в классе:

internal sealed class SyncRepository : ISyncRepository
{
    private static readonly IBinaryConverter<OperationError> ErrorConverter = new BinaryConverter<OperationError>();
 
    private readonly IClient client;
    private readonly string tableName;
 
    public SyncRepository(IClient client, SyncScheme tableScheme)
    {
        ...
    }
    ...
}

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

public class GraphiteReportingTask : PeriodicTask
{
    public GraphiteReportingTask(IGraphiteClient graphiteClient, IProgressMarkerStorage progressMarkerStorage)
    {
        ...
    }
 
    ...
 
    private readonly IGraphiteClient graphiteClient;
    private readonly IProgressMarkerStorage progressMarkerStorage;
    private readonly string graphitePath;
}

А еще интерфейсы мы храним в отдельной папочке рядом с классом:

А мы описываем их в том же файле, что и сам класс:

public interface IFormsReader
{
    Form[] Find(Parameters parameters);
}

public class FormsReader : IFormsReader
{
    ...
}

Service/Manager/Handler/Helper

Если класс управляет аккаунтами, то это AccountService или AccountManager, или… ну вы поняли.

Сам не использую названия типа Helper, так как он очень общий. Чаще это Service или Handler.

Get[Async]

Всегда пишем суффикс Async для асинхронных методов:

public interface IAccountService
{
    Task<Account> GetAsync(Guid accountId, CancellationToken token);
}

Или не пишем, если все методы асинхронные:

public interface IUserService
{
    Task<UserInfo?> Get(Guid userId);
}

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

Так что же лучше?

Вот те примеры, с которыми я сталкиваюсь меняя проекты. Возможно, какие-то из них вызвали у вас возмущение и внутреннее неприятие, возможно вы даже воскликнули: «Да как так можно код писать?!» Да можно. Ничего плохого в этом коде нет. И тратить силы на споры о единственно верном способе расстановки переносов мне видится совсем не продуктивным.

В целом, я считаю, что если вы очень ревностно относитесь к code style в своём проекте, то автоматизируйте его: настройте правила, проверки, подсказки, автоформатирование… Не заставляйте людей зубрить его.

Теги:

Хабы: