Помню, как меня впервые попросили прособеседовать по WPF молодого человека – я несколько часов составлял список того что стоит спросить (и перепроверял ответы, чтобы самому не ударить лицом в грязь) чтобы хоть с какой-то то долей уверенности потом сказать, нужен нам в компании такой человек или нет. И вот, вооружившись 10-15 вопросами, я вхожу в переговорную, представляюсь, задаю пару общих вопросов и между прочим уточняю:
— А сколько у вас лет опыта разработки с использованием WPF?
— Я не знаю WPF….
— …
Этот неловкий момент когда ты понимаешь, что предусмотрел всё, кроме самого очевидного…
Другой, не менее неожиданный для меня поворот был, когда в CV соискателя был указан опыт разработки 5 лет и перечислены куча интригующих описаний проектов, а по факту, человек с трудом смог объяснить, чем отличаются ссылочные типы от значимых, а про сборку мусора сказал, что знает лишь то, что о памяти в .Net думать не нужно…
Что же можно спросить, чтобы можно было положиться на ответ, как на что-то значимое?
— Спрашивать, какие книги он и по сколько раз прочитал? Но заученные формулировки не говорят о том, что человек способен решать реальные задачи, постановка которых отличается от книжных и порой требует отклонения от общих принципов разработки по тем или иным причинам.
— Выписать все технические нюансы среды и спрашивать о них? Но по правде говоря, кому хоть раз пригодилось знание о том, как работает сборка мусора (которая к тому же и меняется от версии к версии) и сколько там поколений? Я не говорю, что это лишнее знание – отнюдь, но знание или незнание этой особенности не позволят определить «качество» разработчика.
— Просить показать пример кода? Но какой код вам покажут? Сколько людей его уже правило? В каких условиях он был написан? Что если эти блестящие 300 строк писались месяц под шум прибоя атлантического океана в сезон дождей? Сможем ли мы потом воссоздать «рабочую» атмосферу, чтобы получить следующие 300 блестящих строк?
Я хочу поделится своей идей и услышать конструктивную критику такого подхода к проведению собеседований. Моя идея заключается в том, чтобы показывать «СВОЙ» код и слушать. За вечер я набросал пример ужасного кода, включив в него самые распространённые «ошибки». Я ожидаю, что старший разработчик, с реальным опытом разработки от 4 лет, должен идентифицировать более 80% процентов ошибок и указать на существующие проблемы в гипотетической архитектуре.
И так, собственно код:
1 using System;
2 using System.Collections.Generic;
3
4 namespace App.Services
5 {
6 public enum LoginResult
7 {
8 Unknown = 0,
9 Success = 1,
10 WrongLogin = -1,
11 WrongPass = -2,
12 Error
13 }
14
15 public class LoginService
16 {
17 public string LastError = string.Empty;
18
19 /// <summary>
20 /// Allow to login new user
21 /// </summary>
22 /// <param name="login">login</param>
23 /// <param name="password">password</param>
24 /// <param name="asAdmin">asAdmin</param>
25 /// <returns>login result</returns>
26 public LoginResult Login(string login, string password)
27 {
28 List<Login> dbLogins = new List<Login>();
29 try
30 {
31 dbLogins.AddRange(
32 DAL.GetItems<Login>(
33 "select * from db.Login where Name='" + login + "'"));
34 }
35 catch (Exception ex)
36 {
37 lock ((object)777)
38 {
39 LastError = ex.Message;
40 }
41 throw ex;
42 }
43 if (dbLogins.Count < 1)
44 {
45 return LoginResult.WrongLogin;
46 }
47
48 var prevUser = App.CurrentUser;
49 App.CurrentUser = dbLogins[0];
50 if (password.CompareTo(App.CurrentUser.Password) != 0)
51 {
52 App.CurrentUser = prevUser;
53 return LoginResult.WrongPass;
54 }
55
56 var log = System.IO.File.AppendText(App.LogFile);
57 log.WriteLine("New user loggined. Login=" + App.CurrentUser.Name);
58
59 if (!(bool)((EventService)App.Service).SendWithConfirm(prevUser))
60 {
61 log.Write("Error sending to user.");
62 }
63
64 GC.Collect();
65 GC.Collect();
66
67 return LoginResult.Success;
68 }
69 }
70 }
Ошибки ( номер строки и описание, которое я ожидаю услышать от соискателя):
12 – Значение для «Error» будет «-1», что продублирует уже существующее и не позволит в будущем отличить одно от другого.
17 (1) – Public field. По правилам хорошего тона не рекомендуется делать поля публично доступными.
17 (2) – Запись в переменную далее по коду «реализована» через lock, но внешний потребитель может и не знать о том что обращение к переменной следует синхронизировать с кем-либо.
20 – Бессмысленные комментарии.
24 – Комментарий не соответствующий действительности.
28 – Публичный метод принял параметры из вне, но не выполнил проверку на их корректность (как минимум на null).
32 – Сильная связанность.
33 (1) – Потенциальная место для использования SQL инъекции. Так как для формирования запроса используется конкатенация а не параметры. А во вторых – не параметризованные запросы не кэшируются сиквел сервером (если СУБД сиквел).
33 (2) – Подобный стиль формирования запросов «привязывает» приложение к конкретной СУБД).
33 (3) – Конкатенация строк таким образом не самое эффективное решение.
35 – По правилам хорошего тона следует перехватывать ошибки, которые могут быть обработаны, а не всё подряд.
37 – Этот lock не будет работать.
39 – Бессмысленное значение в переменной и никакой пользы для разработчиков при диагностики.
41 – Актуальный стэк трйс, который мог бы быть полезен при диагностике ошибки безвозвратно потерян при таком порождении исключения.
49 (1) – Разрушение состояния приложения, так как при ошибке ниже (null пароль) в поле текущего пользователя будет уже новый логин.
49 (2) – Неявная бизнес логика. Почему отсекли другие логины?
50 (1) – Пароль не проверен на null и может быть исключение которое приведёт к разрушению состояния приложения.
50 (2) – Пароль хранится и используется в открытом виде.
53 – По правилам хорошего тона – не следует сообщать столь подробную информацию о причинах ошибки аутентификации.
56 (1) – Отсутствие продуманного механизма логирования как такового.
56 (2) – Открывается поток, но его закрытие и уничтожение не выполнены, данные могут быть и не записаны в конечный файл.
56 (3) – Операция работы с файлами может породить исключение, которое уровнем выше может быть трактовано как ошибка аутентификации, хотя по факту – логин уже прошёл.
57 – Бессмысленная информация, которая никак не упростит жизнь анализирующего лог файл, если он вдруг будет создан и кому ни будь потребуется.
59 (1) – Кроме связанности, вложенные вызовы с «завышенными» ожиданиями касательно возвращаемого результата.
59 (2) – Неочевидная бизнес логика.
61 – Бессмысленная запись в лог.
64,65 – Признак больших проблем с памятью в приложении. (Также я ожидаю услышать почему именно два вызова подряд и почему так делать всётаки не стоит)
Может, кто-то уже применял такой подход и может поделиться своим опытом?
This entry passed through the Full-Text RSS service — if this is your content and you're reading it on someone else's site, please read the FAQ at fivefilters.org/content-only/faq.php#publishers. Five Filters recommends:
- Massacres That Matter - Part 1 - 'Responsibility To Protect' In Egypt, Libya And Syria
- Massacres That Matter - Part 2 - The Media Response On Egypt, Libya And Syria
- National demonstration: No attack on Syria - Saturday 31 August, 12 noon, Temple Place, London, UK
Комментариев нет:
Отправить комментарий