Здравствуйте, dilmah, Вы писали:
D>и зачем всем этим кодерским онанизмом заниматься?? ИМХО и так понятный и простой код. Вы его только усложняете.
Я с вами глубоко несогласен, до кодерского онанизма предложенным изменениям очень и очень далеко.
И после моих изменений код остается достаточно простым и понятным, но при этом код, который не делает ничего интересного, перестает занимать много места => На экране легче видно, какое именно место занимают эти действия в методе. Кроме того, после таких изменений, код становится проще для модификации.
Здравствуйте, d4rkme551ah, Вы писали:
D>Мне нужно ваше мнение, что бы вы сказали о моих знаниях глядя на следующий код?
Код написан сишником недавно начавшим учить C#. Почему?
1) Названия функций и переменных
2) Перехват исключений и использование возвращаемых значений, типа true/false для индикации ошибок. Очень сильно захламляет код. Достаточно перехватывать на самом верху, ведь при исключении все равно продолжение работы невозможно.
3) Куча классов в одном файле
4) Неиспользование IDisposable и using
5) Совершенно непонятно зачем нужно было городить функции open/close/add_record и member variables типа FileStream, когда данные и так грузятся целиком в память после parse. Логичнее было бы вызвать одну единственную функцию db.WriteRecords(data)
6) Код нуждается в серьезном рефакторинге, он очень плохо читаем — extract method (функции на пару экранов), rename variables (куча сокращений и непонятных названий, типа i1). Да вообще разбивку по классам надо переделывать, тот же метод Parse надо выносить в отдельный класс, типа DataParser. Уверен, что после рефакторинга удастся избежать кучи повторного кода.
Вообщем для джуниора хорошо, потому что код рабочий. Но если бы я брал тебя в команду, то вначале очень подробно ставил бы задачи, вплоть до того какие классы должны быть и с какими методами. Плюс коммитить разрешил бы только после code review. Думаю, что через полгодика у тебя бы все стало получаться на уровне неплохого разработчика.
Здравствуйте, d4rkme551ah, Вы писали:
D>Меня вызвали на собеседование по .NET и сказали привезти с собой проекты. Но я так волнуюсь, у меня их мало, всего 2, и немножко стесняюсь показать им свой проект. Мне нужно ваше мнение, что бы вы сказали о моих знаниях глядя на следующий код? D>http://d4rkme551ah.hostzi.com/DBManager.cs D>http://d4rkme551ah.hostzi.com/PlxDevice.cs
Напишите комментарий на каждый класс и на каждый метод, чего этот класс делает, а на метод дополнительно — чего ожидает на входе и как реагирует на особые/ошибочные случаи, if any
И сверху шапочку напишите с названием модуля, автором/копирайтом, датой написания и коротким описанием, что этот модуль делает
я сами проекты не смотрела, у меня общее соображение — если эти проекты делались для какой-то компании, то наверное не очень хорошо их целиком показывать. Можно показать рабочее приложение и какую-то часть кода, или только часть кода. В общем, убедитесь сначала, что вы имеете право это кому-то показывать. Вполне нормально прямо на собеседовании сказать, что весь проект целиком вы не имеете права показывать.
Здравствуйте, d4rkme551ah, Вы писали:
D>А вот за кририку огромно спасибо, теперь я хотя бы знаю в каком направлении работать и где у меня слабые места. Но мне что-то расхотелось идти на собеседование
а вот это зря — сходить на собеседование и пообщаться всегда полезно.
Здравствуйте, Roman Odaisky, Вы писали:
IS>>поменьше бы использовал if return RO>т. е. if(нечто) { return что-нибудь; }? А в чём проблема?
один или два if-а это нормально, но вот такие конструкции:
{
if (!build_op_tree(root, '-'))
return false;
if (!build_op_tree(root, '+'))
return false;
if (!build_op_tree(root, '/'))
return false;
if (!build_op_tree(root, '*'))
return false;
}
надо убирать.
Почему?
На например при отладке придется ставить несколько точек останова чтобы посмотреть откуда процедура выходит, это сложно потому как присматривать за чем-то одним проче чем за нескольким. ну и еще у студии есть свойство тормозить как стадо гипопотамов если точек останова в солюшене много.
Да и вообще код с большим количеством условий увеличивает его сложность (cyclomatic complexity?).
Ксати, что я заметил — кореляция между опытом человека и простотой его кода, как бы это выразиться — нелинейная.
кто-то пишет почти сразу более менее простой код, хотя и не всегда рабочий — а кто-то вроде бы и научился с опытом
наводить марафет на код, и работает все, но код все равно излишне сложен (не интуитивен) для развития и понимания.
Здравствуйте, d4rkme551ah, Вы писали:
D>Меня вызвали на собеседование по .NET и сказали привезти с собой проекты. Но я так волнуюсь, у меня их мало, всего 2, и немножко стесняюсь показать им свой проект. Мне нужно ваше мнение, что бы вы сказали о моих знаниях глядя на следующий код? D>http://d4rkme551ah.hostzi.com/DBManager.cs D>http://d4rkme551ah.hostzi.com/PlxDevice.cs
Посмотрел очень бегло. Из хорошего — код отформатирован, у многих и этого не наблюдается. Из плохого — методы очень длинные (методы parseExpr, parseFile это вообще жесть, насмотрелся уже на такое — если встречаю — переписываю). Куча вложенных if наблюдается, magic numbers в константы не вынес (что такое 4, что такое 6). Как минимум к публичным методам сверху комментарии, к классам тоже — что делает, какие параметры и т.д. Закомментированный код наблюдается (справедливости сказать его мало, но он есть). Много copy-paste, не заметил ни одного private метода для вынесения общей логики, утилитными методами для конвертации типов (и других частых операций) не пользуешься — все на месте делаешь. Можно много чего еще накопать.
Резюме — типичный джуниор, на среднего разработчика не тянешь. Что-то серьезное давать опасно, я бы посадил вначале на багфиксинг, бросил бы в пару с опытным разработчиком чтобы он показал как надо писать, через год-полтора возможно будет отдача. Если не попадешь с пару с опытным разработчиком, то будешь писать так же и через 5 лет — проверено. Для начала надо прочитать Макконела "Совершенный код", Фаулера "Рефакторинг" — немного стиль это улучшает.
Здравствуйте, dilmah, Вы писали:
D>и зачем всем этим кодерским онанизмом заниматься?? ИМХО и так понятный и простой код. Вы его только усложняете.
Понятный для тех, кто сам так пишет, кто привык к такому коду, и не видел никогда нормального. Да, я видел код гораздо хуже, хуже на порядки, но — вначале это и был как раз именно вот такой понятный и простой код. Народ тоже считал рефакторинг кодерским онанизмом, при увеличении требований добавляли еще один if — как результат, через несколько лет проект полностью падает под собственной тяжестью. Там, где требовалось бы 3 человека на месяц — набирают где-то сотню человек и делают тоже самое, но за 2 года. От такого стиля выигрывают только менеджеры, у которых зарплата от количества подчиненных зависит — тут да, возможности осваивать бюджет будут большие. Все шишки от такого кода полетят потом на разработчиков, которым этот код впоследствии передадут — они окажутся крайними, знаю, плавали. Кстати да, для того, чтобы стать незаменимым сотрудником код слишком простой и понятный — потому рекомендую названия методов и переменных урезать до одного — двух символов, убрать форматирование, и накрутить кучу мертвого кода для объема.
RO>Я б такое убрал только из-за того, что это ^C^V. Если бы это был язык типа Пайтона, я бы вообще написал в том месте RO>
return all(build_op_tree(root, op) for op in"-+/*")
на C# один из вариантов будет такой (может на LINQ и короче будет):
foreach(var op in"-+/*")
{
if (!build_op_tree(root, op))
{
return false;
}
}
код можно вытянуть в одну строку убрав две пары необязательных с точки зрения языка curly brackets
RO>(там return true дальше по течению), а если бы во всех if были существенно разные выражения, что тогда?
тогда ничего не надо делать — существенно разные выражения явно показывают сущесвенно разный intent. Ну разве только что добавить коментариев.
Здравствуйте, d4rkme551ah, Вы писали:
D>Меня вызвали на собеседование по .NET и сказали привезти с собой проекты. Но я так волнуюсь, у меня их мало, всего 2, и немножко стесняюсь показать им свой проект. Мне нужно ваше мнение, что бы вы сказали о моих знаниях глядя на следующий код? D>http://d4rkme551ah.hostzi.com/DBManager.cs D>http://d4rkme551ah.hostzi.com/PlxDevice.cs
* thriving in a production environment *
