Привет всем. Читаю тут время от времени про то, что людям вот дают тестовые задания. Нет, не так начну..
Решил я тут сходить на собеседование в одну компанию. Позиция — ведущий .NET разработчик. Позвонили, говорят:
резюме неплохое у вас, но на эту позицию у нас предусмотрено тестовое задание небольшое, часов на 10-12. Не хотите?
Я подумал — 10 часов не убудет и согласился, задание было действительно небольшим, я его сделал, отправил, через 4(!) часа
после этого мне пришел ответ с отказом и комментариями тамошнего эксперта. Мне лично(наверное, больное самолюбие) комментарии
к отказу показались недостаточно существенными. Если вы посмотрите задание, решение и причины отказа, буду крайне благодарен.
Просто на мой взгляд задание я выполнил неплохо, и мне интересно насколько глубоко я ошибаюсь.
1. Задание http://files.rsdn.ru/19230/task.txt
2. Решение http://files.rsdn.ru/19230/HttpTraveller.zip
3. Причины отказа http://files.rsdn.ru/19230/decline.txt
Здравствуйте, Karn, Вы писали:
K>Просто на мой взгляд задание я выполнил неплохо, и мне интересно насколько глубоко я ошибаюсь.
ИМХО, уж слишком многословно, отсюда и ошибки. Может быть, вас сбили упоминания DI и прочего в задании. По сути, требовалось два функционала — закачака/парсинг файлов и вывод результатов (тут я не совсем допёр, что значит "В последовательность файлов размером не более N Мбайт (конфигурируется)" — то есть вывод ссылк в последовательность файлов — им что ли нужен логер на подобии ролапа из log4net?).
Так вот, к заданию. Многопоточность вы там решили добавить — что ж, похвально. Только вот алгоритмы с
в топку. Ради чего? Неужели нельзя без колдунства?
Да и зачем вам собственные потоки? Чем стандартный пул не устроил? В нём достаточное количество потоков для работы на среднестатистическом компьютере. А, вообще, взяли бы таски, раз уж четвёртый фреймворк разрешили.
// если дошли до порога рекурсии - абортим все треды кроме той где
А ради чего требуется "доходить до порога рекурсии" и "абортить"? Почему нельзя, "дойдя до порога" просто не лезть в глубь, обработать всё на текущем уровне и успокоиться с чуством выполненого долга?
Совет: попробуйте реализовать задание используя минимальное количество кода, сущностей. Проведите декомпозию: ссылка/документ/ссылки/ссылка. Выделите в метод (а не в интерфейс + класс-реализация — у вас нет требований по расширяемости, зато есть требования по тестирабельности) решение подзадачи. Следите, что бы набор аргументов и возвращаемое значение были бы необходимыми и достаточными.
Ну и по мелочам не могу не докопаться: ссылки в документе всё же надо искать через DOM, а не простым сканом. Например, в значениях атрибутов элемиентов.
var request = WebRequest.Create(address) as HttpWebRequest;
if (request == null) return null;
var response = request.GetResponse() as HttpWebResponse;
if (response == null) return null;
using (var stream = response.GetResponseStream())
{
if (stream == null) return null;
using (var reader = new StreamReader(stream))
{
return reader.ReadToEnd();
}
}
Для чего там as и приведение типов вообще? Для чего проверки на null, то есть в каких случаях WebRequest.Create/request.GetResponse/response.GetResponseStream могут вернуть null? В документации об этом не говорится. Но даже если и так, то WebResponse надо диспозить в любом случае диспозить, даже если он вам и не подошёл вдруг.
В любом случае не расстраивайтесь: просто компании требовались какие-то другие особенности от кандидатов нежели те, что, они разглядели, как им показалось, в вас. Это же обычные смотрины с повезёт / не повезёт. Если компания так себе, то таких полно, обязательно найдёте свою. Если какая-то супер-очень… Ну представьте себе, что у вас случилось вдруг свидание со Скарлет Йохансон (или кто вам больше нравится?), а она на следующий день перезванивает и говорит: "Знаешь, больше не звони мне, мы не подходим друг другу." Ну совершенно это не стоит того, что бы огорчаться — она всё равно рассматривала тебя в качестве серьёздного кандидата
Help will always be given at Hogwarts to those who ask for it.
Здравствуйте, _FRED_, Вы писали: _FR>Ну и по мелочам не могу не докопаться: ссылки в документе всё же надо искать через DOM, а не простым сканом. Например, в значениях атрибутов элемиентов.
Имхо регуляркой сильно быстрее, чем парсить миллионы документов ради ссылок.
Здравствуйте, Karn, Вы писали:
K>Если вы посмотрите задание, решение и причины отказа, буду крайне благодарен. K>Просто на мой взгляд задание я выполнил неплохо, и мне интересно насколько глубоко я ошибаюсь.
В общем, по делу там в комментариях написано. В таких тестовых заданиях обычно смотрят не столько на крутизну использованных подходов, сколько на побочные эффекты, например, на то, как быстро посторонний сможет "въехать" в твоё решение. А в твоём случае довольно тяжело как уяснить сам код, так и прочесть пояснительную записку (каждое предложение "по традиции" приходится читать по три раза).
Относительно архитектуры присоединюсь к "предыдущему оратору": зачем здесь DI-контейнеры? Обычные ссылки были бы проще, а плюс к тому — ты показал бы как раз искомый OOP/OOD вместо искусства использования DI-контейнера. Зачем было огород городить? В общем, лучше бы ты потратил оставшееся время на предельное упрощение архитектуры и использованных решений, руководствуясь принципом: "если без вещи можно обойтись — она не нужна". Соответственно упростилось бы и всё остальное — от имён методов, до поясниловки и юнит-тестов.
Я знаю только две бесконечные вещи — Вселенную и человеческую глупость, и я не совсем уверен насчёт Вселенной. (c) А. Эйнштейн
P.S.: Винодельческие провинции — это есть рулез!
Здравствуйте, Karn, Вы писали:
K>Привет всем. Читаю тут время от времени про то, что людям вот дают тестовые задания. Нет, не так начну.. K>Решил я тут сходить на собеседование в одну компанию. Позиция — ведущий .NET разработчик. Позвонили, говорят: K>резюме неплохое у вас, но на эту позицию у нас предусмотрено тестовое задание небольшое, часов на 10-12. Не хотите? K>Я подумал — 10 часов не убудет и согласился, задание было действительно небольшим, я его сделал, отправил, через 4(!) часа K>после этого мне пришел ответ с отказом и комментариями тамошнего эксперта. Мне лично(наверное, больное самолюбие) комментарии K>к отказу показались недостаточно существенными. Если вы посмотрите задание, решение и причины отказа, буду крайне благодарен. K>Просто на мой взгляд задание я выполнил неплохо, и мне интересно насколько глубоко я ошибаюсь. K>1. Задание K>http://files.rsdn.ru/19230/task.txt K>2. Решение K>http://files.rsdn.ru/19230/HttpTraveller.zip K>3. Причины отказа K>http://files.rsdn.ru/19230/decline.txt
Сильно не разбирался, но семантика интерфейсов меня поставила в тупик
public interface IArchiveManager
{
bool IsAddressUniqueOnPrevious(string address);
void SendGenerationToStorage(string[] items, int storgeIndex);
}
долго думал, прежде чем предположил возможное поведение реализации. почему в интерфейс пролез номер хранилища, вроде как вещь сугубо завязана на имплементации, а не контракте.
Далее что-то очень похожее но не более понятное
public interface IGenerationStorage
{
bool IsAddressInStorage(string address, int storageIndex);
void DumpGenerationToStorage(string[] items, int storageIndex);
}
Опять мне надо ковыряться с каким-то непонятными номерами хранилищ в интерфейсе, при этом сущности отдельного хранилища нет.
Напрашивающихся абстракций документа, ссылки нет. Зато есть непонятные интерфейсы типа
Это тебе они кажутся несущественными, но ты не забывай, что ты выступаешь "одним из многих", соответственно, из общего числа кандидатов выбран был тот, у кого не было подобных замечаний.
Я знаю только две бесконечные вещи — Вселенную и человеческую глупость, и я не совсем уверен насчёт Вселенной. (c) А. Эйнштейн
P.S.: Винодельческие провинции — это есть рулез!
Ну да, вы все во многом правы (хотя не со всем я согласен Просто мне показалось, что замечания проверяющего не являются следствием каких-то догм и практик, а скорее следствием "его видения" того, как это должно быть реализовано и реальная "дельта" того что есть и того как должно не настолько значительна, чтобы отказываться от продолжения. Теперь я правда начинаю сомневаться в своем первом впечатлении
Здравствуйте, Karn, Вы писали:
K>Просто на мой взгляд задание я выполнил неплохо, и мне интересно насколько глубоко я ошибаюсь.
Посмотрел ваше задание и работу. У вас "болезнь начинающего программиста". Думаю, через это все в свое время прошли.
Когда только начинаешь -- хочешь применить все супер-модные технологии о которых только слышал, причем засунуть их везде -- и где можно и где неможна. Хочется максимально усложнить архитектуру, ввести десятки не особо нужных слоев-шелупаек...
А потом только осознаешь всю ценность простоты. Думаешь не как сделать крутую архитектуру -- а наоборот, как сделать МАКСИМАЛЬНО ПРОСТО и выбрать наиболее простые и подходящие решения.
Для простой задачи вы настолько усложнили решение, что даже я (более 7 лет опыта в .Net, и более 10 в программинге вообще) с налету нифига не понял. Мне кажется и ваш экзаменатор не до конца понял как оно все устроено. Ну зачем вы это делаете? А что если вам дать сложное задание? Кто тогда поймет что вы там написали?
И! Самое главное! В простейших вещах вы делаете грубейшие ошибки:
public virtual string GetSource(string address)
{
try
{
var request = WebRequest.Create(address) as HttpWebRequest;
if (request == null) return null; // Что это? Накой оно?var response = request.GetResponse() as HttpWebResponse;
if (response == null) return null; // Что это? Накой оно?using (var stream = response.GetResponseStream())
{
if (stream == null) return null; // Что это? Накой оно?using (var reader = new StreamReader(stream))
{
return reader.ReadToEnd();
}
}
}
catch (WebException)
{
// Нельзя прятать. Если, к примеру, страница не найдена (404) -- вы будете что делать (подсказка -- нужно вычеркнуть эту ссылку)? А если превышен интервал ожидания (подсказка -- повторить попытку)?
}
catch (Exception exception)
{
// То что вы написали этот блок говорит о том, что вы боитесь класса WebRequest. Вы не знаете как он работает. Ваша неуверенность заставила вписать здесь лишний код.
EnterpriseLibraryContainer.Current.GetInstance<LogWriter>().Write(exception);
throw;
}
return String.Empty; // Что это? В каком случае возникнет (подсказка -- при WebException)? Как вы это будете обрабатывать?
}
Надо же, тебе код-ревью на целую страницу написали, не поленились. Прогресс!..
В моем случае код-ревью выглядел так:
1. Процедурный стиль.
2. Небрежное использование асинхронных делегатов.
3. Отсутствие юнит тестов.
4. Плохие имена практически всех классов, методов, полей и переменных.
В переводе это означает:
1. Я должен был вместо одного класса с двумя методами (20 строк кода) сделать 10 классов с 50-ю методами. Как у тебя примерно.
2. Ревьювер не знал, что исключения в асинхронных делегатах регенерируются в EndInvoke(), и хотел увидеть тело метода обернутым в try{}catch(){}.
3. В солюшене из 3-х проектов ревьювер не нашел проекта с тестами. Ну не заметил просто. Ну бывает, хотел увидеть NUnit-тесты, а там MSTest оказался.
4. Эээ... не переводится
Я все это к чему: на Лабораторию Касперского нужно просто забить. Их просто ddos-ят. Ревьюверы просто не в состоянии переработать такое большое количество материала. Так что не огорчайся
Здравствуйте, _FRED_, Вы писали:
_FR>Для чего проверки на null,
Это все решарпер.
_FR>то есть в каких случаях WebRequest.Create/request.GetResponse/response.GetResponseStream могут вернуть null?
/// <devdoc>
/// <para>Gets the stream used for reading the body of the response from the
/// server.</para>
/// </devdoc>public override Stream GetResponseStream() {
if(Logging.On)Logging.Enter(Logging.Web, this, "GetResponseStream", "");
CheckDisposed();
if (!CanGetResponseStream())
{
// give a blank stream in the HEAD case, which = 0 bytes of dataif(Logging.On)Logging.Exit(Logging.Web, this, "GetResponseStream", Stream.Null);
returnStream.Null;
}
if(Logging.On)Logging.PrintInfo(Logging.Web, "ContentLength="+m_ContentLength);
if(Logging.On)Logging.Exit(Logging.Web, this, "GetResponseStream", m_ConnectStream);
return m_ConnectStream;
}
public abstract class Stream : MarshalByRefObject, IDisposable
{
public static readonly Stream Null;
}
То есть действительно может быть null;
_FR>В документации об этом не говорится.
Здравствуйте, Karn, Вы писали:
K>Ну да, вы все во многом правы (хотя не со всем я согласен Просто мне показалось, что замечания проверяющего не являются следствием каких-то догм и практик, а скорее следствием "его видения" того, как это должно быть реализовано и реальная "дельта" того что есть и того как должно не настолько значительна, чтобы отказываться от продолжения. Теперь я правда начинаю сомневаться в своем первом впечатлении
Самый простой тест на качество кода — дать его прочесть кому то другому, если он не врубится как он в основном работает, значит есть проблемы. Код должен читаться, в Ваш код я например до конца так и не врубился. DI нужно в первую очередь что бы *построить* дерево объектов, а не пихать использование контейнера в каждый класс. В тестах зависимости строятся явным образом.
Здравствуйте, _Raz_, Вы писали:
_FR>>Для чего проверки на null, _R_>Это все решарпер.
В инквизицию с такими оправданиями. У решарпера очень неплохо вроде как была размечена стандартная библиотека на этот счёт. Так что им надо багу написать.
_FR>>то есть в каких случаях WebRequest.Create/request.GetResponse/response.GetResponseStream могут вернуть null?
_R_> returnStream.Null;
_R_>То есть действительно может быть null;
Не поверите: не может. Изучите пожалуйста внимательнее данный вопрос. Вот "as" да, может вернуть в этом случае, но никаких проблем с этим быть не должно.
Help will always be given at Hogwarts to those who ask for it.
Здравствуйте, _FRED_, Вы писали:
_FR>Да и зачем вам собственные потоки? Чем стандартный пул не устроил? В нём достаточное количество потоков для работы на среднестатистическом компьютере. А, вообще, взяли бы таски, раз уж четвёртый фреймворк разрешили.
А если компьютер — 4х ядерный i7 c 24Gb ОП и 100Mb/c каналом, какое решение будет самым производительным в этой задаче? И допустим если усложнить — нужно в одной операции грузить не 1 страницу, а 5 например.
Здравствуйте, michael_isu, Вы писали:
_FR>>Да и зачем вам собственные потоки? Чем стандартный пул не устроил? В нём достаточное количество потоков для работы на среднестатистическом компьютере. А, вообще, взяли бы таски, раз уж четвёртый фреймворк разрешили.
_>А если компьютер — 4х ядерный i7 c 24Gb ОП и 100Mb/c каналом, какое решение будет самым производительным в этой задаче?
Это имеет какое-то отношение к использованию или нет пула?
_>И допустим если усложнить — нужно в одной операции грузить не 1 страницу, а 5 например.
Что значит "5 в одной операции"?
Help will always be given at Hogwarts to those who ask for it.
Здравствуйте, _FRED_, Вы писали:
_FR>Здравствуйте, michael_isu, Вы писали:
_FR>>>Да и зачем вам собственные потоки? Чем стандартный пул не устроил? В нём достаточное количество потоков для работы на среднестатистическом компьютере. А, вообще, взяли бы таски, раз уж четвёртый фреймворк разрешили.
_>>А если компьютер — 4х ядерный i7 c 24Gb ОП и 100Mb/c каналом, какое решение будет самым производительным в этой задаче?
_FR>Это имеет какое-то отношение к использованию или нет пула?
Вы сказали про среднестатистический компьютер, ну а я попытался привести пример несреднестатистического и хотелось узнать что тогда лучше использовать — пул или же собственные потоки.
_>>И допустим если усложнить — нужно в одной операции грузить не 1 страницу, а 5 например.
_FR>Что значит "5 в одной операции"?
Ну т.е. в методе, который выполняется в отдельном потоке, нужно загрузить не 1 страницу, а обойти по какому-то алгоритму несколько страниц сайта. Вопрос к тому, что будет лучше — каждый такой метод выполнять в своем потоке, либо же ещё и внутри метода каждую операцию загрузки страницы делать в отдельном потоке?
Здравствуйте, _FRED_, Вы писали:
_FR>В инквизицию с такими оправданиями. У решарпера очень неплохо вроде как была размечена стандартная библиотека на этот счёт. Так что им надо багу написать. http://youtrack.jetbrains.net/issue/RSRP-217580
Здравствуйте, michael_isu, Вы писали:
_FR>>>>Да и зачем вам собственные потоки? Чем стандартный пул не устроил? В нём достаточное количество потоков для работы на среднестатистическом компьютере. А, вообще, взяли бы таски, раз уж четвёртый фреймворк разрешили. _>>>А если компьютер — 4х ядерный i7 c 24Gb ОП и 100Mb/c каналом, какое решение будет самым производительным в этой задаче? _FR>>Это имеет какое-то отношение к использованию или нет пула? _>Вы сказали про среднестатистический компьютер, ну а я попытался привести пример несреднестатистического
Не среднестатистический — это сервер c огромным количеством процессоров. Бытовые компьютеры в этом плане все можно считать "среднестатистическими".
_>… и хотелось узнать что тогда лучше использовать — пул или же собственные потоки.
Собственные потоки нужны тогда, когда вам необходимо контролировать время жизни потока, какие-то его специфические свойства (приоритет, апартамент и прочее — менять эти параметры для "общих" потоков из пула — не красивый шаг). В данной же задачи требуется лишь выполнить какие-то действия в отдельном потоке. Что с этим потоком было до этих действий, что будет после — не так уж и важно.
_>>>И допустим если усложнить — нужно в одной операции грузить не 1 страницу, а 5 например. _FR>>Что значит "5 в одной операции"? _>Ну т.е. в методе, который выполняется в отдельном потоке, нужно загрузить не 1 страницу, а обойти по какому-то алгоритму несколько страниц сайта. Вопрос к тому, что будет лучше — каждый такой метод выполнять в своем потоке, либо же ещё и внутри метода каждую операцию загрузки страницы делать в отдельном потоке?
Если использовать высокоуровневые библиотеки, спроектированные как раз для таких задач — то разница (в програмировании) не велика. Это может зависеть от множества параметров: например, надёжности и скорости соединения (по которому закачиваются данные). думаю, в данной задачи большую часть времени будет занимать загрузка страницы. Насколько оптимальным и разумным вообще бедет попытка закачать одновременно пять-десять страниц заместот последовательной закачки каждой?
Если вернуться к примерному описанию задачи "ссылка/документ/ссылки/ссылка". "ссылка/документ" — это скачивание. Оно может быть последовательно (смотря, на сколько скачивание быстрее-медленнее разбора страницы) или параллельно, если у вас очень широкий канал. "документ/ссылки" — это парсинг. Тут имеет смысл распараллелить работу. Опять же хорошая библиотека сама за вас выберит оптимальное количество одновременно выполняющихся задач. "ссылки/ссылка" — логирование (например, так же по очереди, нет смысла параллельно писать в один файл).
В общем, если постараться и хорошо разбить задачу на подзадачи, то выбирать — параллельно или последовательно выполнять тот или иной кусок кода — это уже не сложно, главное что бы сам код задач небыл бы от этого [сильно] зависим. Таким образом решение заключается в правильном разбитии — а что с этим уже делать дальше — параллелить, тестировать, конфигурировать — становится второзначимым вопросом.
Help will always be given at Hogwarts to those who ask for it.
Спасибо.
_FR>Если использовать высокоуровневые библиотеки, спроектированные как раз для таких задач — то разница (в програмировании) не велика. Это может зависеть от множества параметров: например, надёжности и скорости соединения (по которому закачиваются данные). думаю, в данной задачи большую часть времени будет занимать загрузка страницы. Насколько оптимальным и разумным вообще бедет попытка закачать одновременно пять-десять страниц заместот последовательной закачки каждой?
Смысл есть, т.к. канал при загрузке страницы занимается не полностью, поэтому если одновременно грузить 100 страниц в 100 потоках — загрузятся все за не намногим большее время, чем если грузить 1 страницу.
Ну совершенно это не стоит того, что бы огорчаться — она всё равно рассматривала тебя в качестве серьёздного кандидата 
.
