Проходя собеседование в одну из компаний, я столкнулся с задачей, которая выделялась на фоне стандартных вопросов про алгоритмы и структуры данных. Вместо привычной реализации алгоритма мне предложили отревьюить легаси-код. Это было гораздо легче, чем алгоритмы, но не менее эффективно для проверки широты знаний. С тех пор я не расстаюсь с этой задачей уже как собеседующий. Работает гораздо лучше, чем вопросы. Это дает гораздо более реалистичное представление о навыках кандидата, чем типичные вопросы о сортировках или деревьях.
Задача заключалась в том, чтобы найти как можно больше плохих практик (далее “вонючек”) в коде и предложить варианты улучшений, не ломая существующие интерфейсы. Код не использовался в реальном проекте, но специально наполнен анти-паттернами и типичными проблемами, с которыми можно столкнуться при поддержке старых проектов. Это дало возможность продемонстрировать свои знания не только в области разработки, но и в вопросах рефакторинга и улучшения качества кода.
Почему мне нравится такое задание больше, чем задачи на алгоритмы
Метод задачи на ревью кода, по моему опыту, оказался гораздо более интересным и эффективным способом проверки знаний кандидатов, чем традиционные задачи на алгоритмы. Вот почему:
- Приближенность к реальной работе. В реальных проектах редко приходится ежедневно реализовывать сложные алгоритмы. Гораздо чаще разработчики сталкиваются с задачами по поддержке и улучшению существующего кода. Именно это задание дало возможность продемонстрировать умение ориентироваться в реальных проектах, где не всё идеально, и находить решения в рамках существующей системы.
- Комплексный подход. При ревью кода кандидат сталкивается с задачами из разных областей: архитектура приложения, работа с базой данных, асинхронные операции, поддержка уже существующей логики. Важно уметь решать такие задачи на стыке нескольких дисциплин.
- Критическое мышление. В этом задании требовалось не просто написать код, а внимательно анализировать его, находить потенциальные проблемы и предлагать улучшения. Это включает как простые недочёты, вроде дублирования кода, так и более серьёзные архитектурные проблемы.
- Практическая проверка культуры кода. Важно было не просто улучшить код, а сделать это так, чтобы не сломать существующие интерфейсы и поддерживать читаемость для других разработчиков. Это проверяет, насколько кандидат готов поддерживать и улучшать код в реальной команде.
А еще в задачах про алгоритмы обычно нет души. Сейчас поясню, что я имею в виду. Входные данные не имеют никаких ассоциативных связей с реальным миром. Набор несвязанных букв в строке, названия функций и сама постановка задачи лишена смысла. Обойти дерево - а зачем?
Перейдем к коду
Код для онлайн-магазина GigaStore, из которого выдернули несколько методов в разных слоях бизнес-логики. Задача - ревью и рефакторинг. Пример кода на C#, хотя некоторые концепции легко обнаружить, даже если вы не писали на этом языке. Для удобства ревью код содержит сразу несколько классов (что само по себе не самая распространенная практика). Строки подключения к БД, устройство моделей и DbContext тоже можно пропустить при ревью. Все остальное можно нещадно ревьюить.
Обычно в зависимости от скорости кандидата и его уровня предлагаю несколько режимов:
- Устное ревью, я записываю замечания в виде TODO комментариев (тудушек)
- Кандидат рассказывает и записывает тудушки сам
- Кандидат ревьюит и по возможности тут же рефакторит
- Кандидат рефакторит и учитывает факт, что кодом уже могут пользоваться
На задание дается 20 минут. А вот и код:
using System.Data.SqlClient;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Authorization;
using Microsoft.EntityFrameworkCore;
namespace GigaStore
{
public class UserSession
{
public static List<string> UserNames { get; set; } = new List<string>();
public static void SaveUser(string userName)
{
UserNames.Add(userName);
}
public static bool IsUserLoggedIn(HttpRequest request)
{
request.Cookies.TryGetValue("User", out var userName);
return UserNames.Contains(userName);
}
}
public class RetailController : ControllerBase
{
private DataAccessLayer _dataAccessLayer;
private BackgroundService _backgroundService;
public EmailService EmailService;
public RetailController()
{
_dataAccessLayer = new DataAccessLayer();
_backgroundService = new BackgroundService();
EmailService = new EmailService();
}
[AllowAnonymous]
[HttpGet]
[Route("PlaceOrder/{customerId}")]
public IActionResult PlaceOrder(int customerId, [FromBody] Order order)
{
if (UserSession.IsUserLoggedIn(Request))
{
_dataAccessLayer.AddOrder(customerId, order);
_backgroundService.UpdateInventory(order);
EmailService.SendOrderConfirmationEmailAsync(customerId, order);
return Ok();
}
return BadRequest();
}
}
public class DataAccessLayer
{
public void AddOrder(int customerId, Order order)
{
using (var context = new RetailDbContext())
{
var customer = context.Customers.FirstOrDefault(c => c.Id == customerId);
var customerOrders = context.Orders.Where(o => o.CustomerId == customerId).ToList();
customerOrders.Add(order);
context.Orders.Add(order);
context.SaveChanges();
}
}
}
public class BackgroundService
{
public void UpdateInventory(Order order)
{
using (var connection = new SqlConnection("..."))
{
connection.Open();
var currentDate = DateTime.Now;
var query = $"UPDATE Inventory SET Quantity = Quantity - {order.Quantity},
LastUpdated = '{currentDate}' WHERE ProductId = {order.ProductId}";
using (var command = new SqlCommand(query, connection))
{
command.ExecuteNonQuery();
}
}
}
}
public class EmailService
{
public async void SendOrderConfirmationEmailAsync(int customerId, Order order)
{
try
{
var emailContent = $"Order confirmation for order ID: {order.Id}.";
var email = new Dictionary<string, string>
{
{ "to", $"customer{customerId}@example.com" },
{ "subject", "Order Confirmation" },
{ "body", emailContent }
};
var content = new FormUrlEncodedContent(email);
var httpClient = new HttpClient();
var response = httpClient.PostAsync("https://email-api.example.com/send", content).Result;
if (!response.IsSuccessStatusCode)
{
throw new InvalidOperationException("Failed to send order confirmation email.");
}
}
catch (Exception ex)
{
Console.WriteLine($"Error sending email: {ex.Message}");
}
}
}
public class RetailDbContext : DbContext
{
public RetailDbContext() :
base(new DbContextOptionsBuilder().UseSqlite("...").Options)
{
}
public DbSet<Product> Products { get; set; }
public DbSet<Order> Orders { get; set; }
public DbSet<Customer> Customers { get; set; }
public DbSet<Inventory> Inventories { get; set; }
}
public class Product
{
}
public class Inventory
{
}
public class Customer
{
public int Id { get; set; }
}
public class Order
{
public object Quantity { get; set; }
public object ProductId { get; set; }
public object Id { get; set; }
public int CustomerId { get; set; }
}
}
В качестве подсказки, примерный список классов проблем, которые присутствуют в коде:
- Статические члены класса
- Проблемы с асинхронностью
- SQL-инъекции
- Проблемы с масштабируемостью
- Нарушение принципов REST
- Неоптимальное использование базы данных (Entity Framework)
- Проблемы с обработкой исключений
- Нарушение принципов SOLID
- Хардкодинг строк и данных
- Неоптимальные наименования переменных
- Отсутствие использования зависимостей через DI (Dependency Injection)
- Логика бизнес-процессов в контроллере
- Отсутствие валидации данных
- Проблемы с тестируемостью
- Отсутствие логирования
- Проблемы с управлением состоянием (State Management)
- Отсутствие транзакций
Попробуйте найти как можно больше “вонючек” в коде за ограниченное время. Это отличная практика для того, чтобы развить навыки рефакторинга и улучшения кода.