Привет, Хабр. Меня зовут Рогатнев Сергей. Я работаю в Контуре ведущим разработчиком уже более 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_SuccessWhen_HandleProcessingResultTransactionEvent_Should_OkValidate_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 в своём проекте, то автоматизируйте его: настройте правила, проверки, подсказки, автоформатирование… Не заставляйте людей зубрить его.
