Почему «код довольно сложно читать» (тестовое задание)?

Question

[FastJony]

Пилил тестовое задание на позицию "Experienced C# Developer".

Задача:
1. Консольное приложение на C# для поблочного сжатия и распаковки файлов с помощью "GzipStream".
2. Исходный файл делим на блоки одинакового размера по 1 метру.
3. Каждый блок сжимается и записывается в выходной файл независимо от остальных блоков.
4. Размер файлов может превышает размер оперативки.
5. Можно юзать: (Thread, Manual/AutoResetEvent, Monitor, Semaphor, Mutex)
6. Нельзя юзать: (async/await, ThreadPool, BackgroundWorker, TPL).
7. Запуск след. образом: GZipTest.exe compress/decompress [имя исходного файла] [имя результирующего файла].
8. Формат архива не важен, соответствие формату GZIP опционально.

Решение:
https://yadi.sk/d/ZULN7SW1yWq6Tw

Почему решение некачественное (по мнению проверяющего):
1. Скорость хорошая.
2. Код довольно сложно читать.
3. Почему-то от файла осталась только маленькая часть после распаковки.

Вопрос к опытным разрабам:
1. Глянув на проект, действительно ли код нечитабелен?
2. Есть ли в нем явные косяки?
3. "от файла осталась только маленькая часть после распаковки"
- сколько тестировал, проблем не было. Видит ли кто косяк в коде?

Косяк нашёл сам, всем спасибо за советы по поводу "красоты кода" :*

Comments

[FastJony]

Dance Macabre, согласен, зря быканул с(:

[#]

а зачем манифест, который требует повышения привилегий?

[FastJony]

#, жать и разжимать файлецкие в директории, доступные только админу.

[#]

FastJony, за уши притянуто.. при доступе в таковые папки, у вас и так попросят повышение привилегий..

а вот при попытке запуска клона вашей репы, меня это вполне стремануло, пришлось копнуть

.. к коду теперь вернусь когда еще будет время

[#]

FastJony, .. не вернешь репу - пущу ищеек по следу

тук тук.. https://github.com/FastJony

[FastJony]

#,
1) Что такое репа и зачем мне её тебе возвращать?
2) Тук тук не рабит, ошибку нашёл, всё поправил, (смотрим вопрос), смысла валяться проекту тут более нет.
3) Если конкретно тебе нужен проект, пиши в личку, скину, могу в манифесте убрать просьбу об админских правах :3

[FastJony]

#, я тут прознал, что репа - это репозиторий (век живи, век учись)
и Бога ради, не натравливай на меня киберколлекторов, я ещё так молод.
Не смог в личку написать (сложна), ссылка сверху обновлена, но на яде, жду оценки коду :*

[#]

FastJony, шутка же ))
я просто теранул проект при общей зачистке темпов

[#]

FastJony, а вообще интересно померять скорость - если сопоставима с rar я бы заюзал )))
в личку на гите не пишу, не замыкаю с тостером

а вот в моем профиле есть тележка, так что пожалуйста, дай поиграть ))
а с манифестом то справлюсь и сам ;)))

[#]

FastJony, .. или че?.. )))

[#]

FastJony, ... я иногда за себя не отвечаю.. (с) .. ;))

[FastJony]

#, я прилепил же, ссыль поменял на яндекс диск :3 юзай, отпишись как по скорости потом, я не сравнивал.

[#]

FastJony, о! нашел! ))
спсб! ))
потестю ))
.. ну и если че, то и по коду.. уже не отвертишься ;)))

Answer 1

[Дмитрий Беляев]

Я бы в первую очередь отметил комментарии, лучше бы их не было, чем такие...
И ладно они на русском, но они рассказывают, что я по идее и так должен понять читая код.
Комментарий, если он есть, то должен отвечать не на вопрос "что делает код?", а на вопрос "зачем здесь этот код вообще нужен?". А если из самого кода не понятно "что делает код" - то код плохой.

Ну и так по мелочи, вроде единого стиля и огромных пустых пространств...

Comments

[FastJony]

Лучше писать комменты только на иглише? Или есть какая-то стратегия выбора языка комментов?

Разве здесь коммент не рассказывает "что делает код" (метод)?

[Дмитрий Беляев]

FastJony, здесь скорее документация, а не коммент, это разные вещи
Я же имел в виду такое:
https://github.com/FastJony/GZipTest/blob/master/G...
https://github.com/FastJony/GZipTest/blob/master/G...
https://github.com/FastJony/GZipTest/blob/master/G...

Ну а насчет языка - да, English is international
Просто предположите, насколько Вам удобно будет читать комментарии, скажем, на китайском?

[Griboks]

Дмитрий Беляев, хотелось бы заметить, что русский язык можно и нужно использовать в таких случаях.

[Владимир Коротенко]

Мой бог блин тут у всех что ли флюен инглиш и вы таки его без напряга понимаете? И маки дураки переводят документацию руками адаптируя? Мое мнение код должен быть откомментирован а в лучшем случае даже с примерами. Это относится к коду который использует 1+ человек. И да самодокументированный код это говно! Прежде всего потому что человек не смог сформулировать что его код делает

Answer 2

[GavriKos]

Открыл Main.cs. Куча непонятных отступов. Часть блоков под условием - с фигурными скобками, часть - без. Читать действительно сложно. Часть комментов на гитхабе даже уехала за пределы зоны просмотра.

Comments

[FastJony]

Вы бы пилили всё с фигурными скобками? Как лучше?

[GavriKos]

FastJony, лучше - единый стиль. Почитайте какие нибудь стандартны оформления кода

[FastJony]

GavriKos, Благодарю за совет

[GavriKos]

FastJony, ну и местами код прям совсем не найс. Длиннющий иф в проверке хейдеров, например.

[FastJony]

GavriKos, Блин, мне наоборот казалось, что если поместить всё в структурку например, сравнить её с идеальной структуркой хэдера будет да, красиво, но я процессор занимаю не тем делом... Разве красота кода настолько важнее его скорости? Или нужно искать золотую серединку?

[GavriKos]

FastJony, а причем тут структура и вот это все? Есть много других методов решения.

А вы уверены что знаете, во что будет преобразован ваш код и как JIT его оптимизнет, что так лихо забиваете на ЧИТАБЕЛЬНОСТЬ?

[FastJony]

GavriKos, как JIT его оптимизнёт дело JITa, я не лезу в его исходники (скрой реализацию, дай человеческий интерфейс). Я оперирую строчечками, написанными на C#, и стараюсь дать ей такую строчечку, чтобы она тратила меньше ресурсов и давала больший профит. Это как залезть в реализацию "System.Collections.Generic.List<>" и сказать: а шо по красоте кода, пацаны, почему реализация некрасивая, JIT оптимизирует, не ссы.

Answer 3

[Петр]

Код то читабельный, но видно что написан новичком в программировании.
Из самого простого:

...
if ((uint)e.HResult == 0x80070002) actualException = "Ошибка : Не удается найти файл - " + ((FileNotFoundException)e).FileName + ".";
                        else if ((uint)e.HResult == 0x80070057) actualException = "Ошибка : Неверно задан путь к файлу или вы ссылаетесь на устройство нефайлового типа.";
                        else if ((uint)e.HResult == 0x80070003) actualException = "Ошибка : Неверно задан путь к файлу.";
                        else if ((uint)e.HResult == 0x80030003) actualException = "Ошибка : Неверно задан путь к файлу.";
...

Что мешало:
1. использовать switch,
2. писать просто текст ошибки и уже при выводе в консоль добавлять "Ошибка : "

Comments

[FastJony]

swith в такой ситуации - правило хорошего тона?

[Петр]

FastJony, Нет. Но у вас просто все захламлено длинным выражением (uint)e.HResult

[FastJony]

Петр, На полном серьёзе могут осудить на джуна не по качеству исполнения самого функционала, а по захламленным длинным выражениям для вывода в консольку? Это так важно?

[Griboks]

Switch занимает на 1 строчку больше для каждого условия. Это только уменьшает читабельность.

[Петр]

Griboks, Его стилистика писать в строку не отразилась бы на читабельности

case 0x80030003: 
case 0x80070057:  actualException = "Неверно задан путь к файлу."; break;

[Петр]

FastJony, джуна - нет. Но вы хотели на Experienced

[Griboks]

Петр, лишний оператор, нет скобок, чтобы различать условие и реакцию.

[Петр]

Griboks, В git код быстро правиться под наши замечания. Похоже ревью по заданию, в компании куда он желает устроиться, еще небыло )))

[FastJony]

Петр, уже было, правлю для себя

[FastJony]

Петр, вы против моих правок? Почему так? :( компанию я еще сначала указал, ее модеры потерли, хош, тыкну коммент, что исправил по твоей инициативе? :D Но если код будет менее читабелен, то не тыкну, ты, главное, заранее предупреди :*

Answer 4

[#]

Косяк нашёл сам, всем спасибо за советы по поводу "красоты кода" :*

не не не.. где же обещанное

ЗЫ. Кто пилит такое же задание, пользуйтесь :3

верни репу!.. ну или мне копию хотя бы.. критику кода по желанию - хочешь, получишь (можно в личку.. и репу, и критику. см профиль

Comments

[FastJony]

Дык качали да пользовались :)
Что значит верни репу? :D
Критику получил, выводы для себя сделал. Косяк нашёл сам. (Подсказка: бошки у гзип, когда жмутся тем же гзип, при распаковке заставляют тебя страдать) Тебе в личку проект кинуть?

[#]

FastJony, а еще очень советую https://marketplace.visualstudio.com/items?itemNam...

.. а то вырвиглаз.. ну право слово

я именно про русский язык, там разобраться не сложно

[#]

FastJony, исключения это для разрабов либ

типа чувак, вот такая фигня очень даже может случиться, но думать что с ней делать должен ты

но не надо этим грузить юзера

ps ты же ни где не бросаешь свои исключения.. так что..
https://docs.microsoft.com/ru-ru/dotnet/csharp/lan...

[#]

FastJony, https://www.sendspace.com/file/q1inzi .. я ток начал чистить.. и не бенчил.. вообще не пробовал

[#]

FastJony,

очередной подход к снаряду

ну и в общем беру назад все острословие, местами выглядит даже педантично )) .. продолжение истории точно будет ))

ps но каменты придется вычистить .. а то читать невозможно.. а до нормального дебага руки так и не доходят ))

[#]

FastJony, напомни.. только один файл обрабатываем? папки рекурсивно не научились?

[FastJony]

#, да, только один, папки любые вбить можно, там в майне коммент с примером