Даже если так, то ничего плохого я не вижу. Люди посмотрели, примерили разные алгоритмы и паттерны, поделились своим мнением. Все вынесли какие то уроки для себя, произошло развитие. Ну а я, как автор исходной статьи, не стал бы пытаться выгораживать свой код и оправдываться если бы хотел лишь получить улучшенный вариант.
Спасибо за рекомендации. Я глядел, как на образец, как сделан парсинг в исходниках .Net (подробно в моём комментариии выше). Почти везде там парсинг свой (без регулярок и даже без int.Parse). Разбиение на токены применяется только в сложных комплексных форматах.
Цикл по миллисекундам ничего не делает кроме проверки флага в массиве, тут итерации нет смысла считать если их не много тысяч. Итераций где идут изменения даты/времени — не более чем 283 в самом худшем случая когда минимальный год сравнивают с максимальным.
Всё таки возможен случай если заявят экстремальные требования по памяти и парсинг нужно будет перенести прямо в методы поиска ближайшего времени, запоминая в конструкторе только исходную строку.
К сожалению, оценка «в разы лучше вашего творчества» осталась нераскрытой. Тут ведь тоже не наблюдается наследование, полиморфизм, инкапсуляция и работа с паттернами. И тоже код не идеален и подлежит облагораживанию. Почему же «вы живете в совершенно разных измерениях»?
Так давайте разоблачим само-лесть! Давайте конкретно сравним. Вот метод из исходников .Net: RegexParser.ScanReplacement(), который используется для парсинга регулярок. По смысловой нагрузке он примерно сравним с моим парсингом строки расписания.
А без конкретики, ваши слова выглядят как снобизм.
Спасибо за оценку. Главное в том, что я не верно оценил сопроводительную фразу заказчика «Обращаю Ваше внимание, что класс должен быть эффективным и не использовать много памяти и ресурсов». Остальное — последствия. И последствий стало много ввиду сложности задания. Я всё написал в стиле исходников самого .Net. (не применял регулярки, string.Split() и int.Parse()). Если бы заказчик хотя бы намекнул, что требуется показать «опыт с наследованием, полиморфизмом и инкапсуляцией» и «опыт работы с паттернами», то я бы делал задание совсем по другому.
Не соглашусь про «календарь это не тот случай». Тут заказчик как раз чётко сказал «очень много значений с шагом в одну миллисекунду» и это дополнительно увело меня в сторону оптимизации памяти и ресурсов.
Мне бы помогла обратная связь с заказчиком, но в ней было отказано.
В некоторых рекомендациях просят не вызвать функции/методы внутри if-выражения. Вот например статья C# Coding Standards, раздел 4.3 Flow Control пункт 33. Avoid invoking methods within a conditional expression.
P.S.: я переписывал мой исходный вариант, а не предложенный неработающий с инвертированием
Первым делом огромное спасибо за потраченное на реализацию время! К сожалению, у вас где то закралась ошибка, тесты не проходят. Например, если на входе строка где все звёздочки, то последние элементы в массивах years/days/months/... оказываются в ошибочном значении false.
Давайте сравним с моим вариантом. Все выводы, конечно же, представляют только моё личное мнение.
По объёму кода получилось у нас почти одинаково. Для сравнения я позволил себе привести форматирование вашего кода с своему стандарту (тело if всегда в фигурных скобках, каждая фигурная скобка в отдельной строке, после if или цикла - пустая строка) и убрал все комментарии. Ничья.
Ваш код более мелко нарезан на методы, соответственно методы более маленькие и более понятные. Тоже самое можно сделать и с моим кодом, от способа разбора строки это не зависит. Я посчитал, что формат строки не столь сложен чтобы выделять методы. Вам плюсик, а я учту на будущее.
Для парсинга чисел вы используете int.TryParse(str, out val), а это прямой призыв выстрелить себе в ногу. Посмотрите, например, какие неожиданные бывают результаты. Чтобы не получить проблем, надо указывать стиль или культуру. И при дальнейшем сопровождении кода могут снова забыть их указать. Вам минусик.
Использование string.Split() и int.TryParse() не характерно для системных библиотек. В исходном коде самого .Net их можно найти только в высокоуровневых подсистемах. В большинстве случаев сделано так, как у меня. Подробный список я привёл в другом моём комментарии.
Некоторые кусочки вашего кода также непонятны, как и мои. Например:
return (parts.Length == 4) ?
TryParseInterval (parts[3], 1000, out milliseconds) :
TryParseInterval ("0", 1000, out milliseconds);
Поэтому не могу вам дать плюс за решение всех проблем понятности кода.
Итого, ваш код немного более понятен, чем мой. Но не принципиально. И есть свои недостатки.
в конструкторе же делаете свой "быстрый" велосипед даже для парсинга строки в число...
смысла отказываться от регулярных выражений небыло
Я, как на образец, всегда смотрю в исходный код самого .Net. Там регулярки используются только в очень высокоуровневом коде типа UI или СУБД. Примеры где парсинг делается также, как у меня (ищите в них '0' или * 10):
Спасибо за мнение. Вот я в поисках "краткого и понятного" кода гляжу метод GetTimeAfter() в библиотеке Quartz, который делает то же самое что у меня. Делал уже несколько подходов, но полностью логику пока не понял. Зато не много вложенных if и выход из цикла показан в последней его строке (которая примерно на 10 экранов отстоит от начала цикла). Мой цикл виден на одном экране, а вложенность if отражает натуральную вложенность компонентов даты (месяцы вложены в года, дни вложены в месяцы и т.д.) Я посчитал такой вариант вполне пригодным для понимания. Но я, конечно, могу понять что в чьих то правилах может существовать жёсткий запрет на вложенные if.
Никто не просил создавать массивы с флагами. Вы могли создать простую структуру в несколько байт и функции Nearest() бы работали просто и без циклов с milliseconds++. Понимаете? Вы как-то неправильно втюхали про класс-календарь, усложнили без необходимости
В одной строке расписания заданы списки, диапазоны и периоды. Тут простой структурой не обойдёшься. Если я не прав, покажите подробнее, про что вы говорите.
Истекло время редактирования комментария, продолжу. У меня в цикле вложены элементы даты (строка год, строка месяц, строка день, строка час и т.д.) и вся вложенность выглядит как естественная иерархия элементов даты (ведь месяц вложен в год, а не идёт последовательно за ним). Лично для меня это повышало понятность алгоритма. Вы вот не разбирались в смысле кода, а зря. Я бы не согласился, что после правок стало принципиально читабельнее.
Спасибо за конкретику. Моя ошибка в том, что мне алгоритм был виден яснее именно в виде вложенных if на одном экране. Я не знал, что количество вложений if считается страшным злом даже если строки короткие и понятные.
Но вы где то ошиблись (видимо по мелочи). Алгоритм нарушен, тесты зависают.
«Регекс это грубо говоря стандарт. Его используют куча людей...Ваш вариант это вещь в себе. Спросить если что можно только у вас» — тут вы передёргиваете (так говорят когда тайно меняют карту в карточной игре). Мой вариант — это C#, который гораздо легче понять, отладить и подработать. Справится даже не профильный специалист, а например джававед. А Regex — сложная штука, в которой трудно разбираться и даже опытные легко могут сделать плохой Regex.
--- добавлено ---
«Ваш вариант» сказали не мне. Что не отменяет моих выводов про Regex!
Не беспокойтесь про «задеть». Хотелось бы больше конкретики, что именно делает мой код выглядящим как «код неопытного программиста». Вот давайте, например, сравним с системным кодом Guid.TryParseGuid() где тоже идёт разбор строки.
Даже если так, то ничего плохого я не вижу. Люди посмотрели, примерили разные алгоритмы и паттерны, поделились своим мнением. Все вынесли какие то уроки для себя, произошло развитие. Ну а я, как автор исходной статьи, не стал бы пытаться выгораживать свой код и оправдываться если бы хотел лишь получить улучшенный вариант.
Спасибо за рекомендации. Я глядел, как на образец, как сделан парсинг в исходниках .Net (подробно в моём комментариии выше). Почти везде там парсинг свой (без регулярок и даже без int.Parse). Разбиение на токены применяется только в сложных комплексных форматах.
Цикл по миллисекундам ничего не делает кроме проверки флага в массиве, тут итерации нет смысла считать если их не много тысяч. Итераций где идут изменения даты/времени — не более чем 283 в самом худшем случая когда минимальный год сравнивают с максимальным.
Всё таки возможен случай если заявят экстремальные требования по памяти и парсинг нужно будет перенести прямо в методы поиска ближайшего времени, запоминая в конструкторе только исходную строку.
У кого есть опыт по тестовым заданиям, скажите, как заказчики отнесутся к реализации парсинга через комбинаторы из библиотеки Pidgin?
(уже сказали выше)
К сожалению, оценка «в разы лучше вашего творчества» осталась нераскрытой. Тут ведь тоже не наблюдается наследование, полиморфизм, инкапсуляция и работа с паттернами. И тоже код не идеален и подлежит облагораживанию. Почему же «вы живете в совершенно разных измерениях»?
Так давайте разоблачим само-лесть! Давайте конкретно сравним. Вот метод из исходников .Net: RegexParser.ScanReplacement(), который используется для парсинга регулярок. По смысловой нагрузке он примерно сравним с моим парсингом строки расписания.
А без конкретики, ваши слова выглядят как снобизм.
Спасибо за оценку. Главное в том, что я не верно оценил сопроводительную фразу заказчика «Обращаю Ваше внимание, что класс должен быть эффективным и не использовать много памяти и ресурсов». Остальное — последствия. И последствий стало много ввиду сложности задания. Я всё написал в стиле исходников самого .Net. (не применял регулярки, string.Split() и int.Parse()). Если бы заказчик хотя бы намекнул, что требуется показать «опыт с наследованием, полиморфизмом и инкапсуляцией» и «опыт работы с паттернами», то я бы делал задание совсем по другому.
Не соглашусь про «календарь это не тот случай». Тут заказчик как раз чётко сказал «очень много значений с шагом в одну миллисекунду» и это дополнительно увело меня в сторону оптимизации памяти и ресурсов.
Мне бы помогла обратная связь с заказчиком, но в ней было отказано.
В некоторых рекомендациях просят не вызвать функции/методы внутри if-выражения. Вот например статья C# Coding Standards, раздел 4.3 Flow Control пункт 33. Avoid invoking methods within a conditional expression.
P.S.: я переписывал мой исходный вариант, а не предложенный неработающий с инвертированием
Вы про то, что надо было просто разбить на строки так:
или что то другое?
Первым делом огромное спасибо за потраченное на реализацию время! К сожалению, у вас где то закралась ошибка, тесты не проходят. Например, если на входе строка где все звёздочки, то последние элементы в массивах years/days/months/... оказываются в ошибочном значении false.
Давайте сравним с моим вариантом. Все выводы, конечно же, представляют только моё личное мнение.
По объёму кода получилось у нас почти одинаково. Для сравнения я позволил себе привести форматирование вашего кода с своему стандарту (тело if всегда в фигурных скобках, каждая фигурная скобка в отдельной строке, после if или цикла - пустая строка) и убрал все комментарии. Ничья.
Ваш код более мелко нарезан на методы, соответственно методы более маленькие и более понятные. Тоже самое можно сделать и с моим кодом, от способа разбора строки это не зависит. Я посчитал, что формат строки не столь сложен чтобы выделять методы. Вам плюсик, а я учту на будущее.
Для парсинга чисел вы используете int.TryParse(str, out val), а это прямой призыв выстрелить себе в ногу. Посмотрите, например, какие неожиданные бывают результаты. Чтобы не получить проблем, надо указывать стиль или культуру. И при дальнейшем сопровождении кода могут снова забыть их указать. Вам минусик.
Использование string.Split() и int.TryParse() не характерно для системных библиотек. В исходном коде самого .Net их можно найти только в высокоуровневых подсистемах. В большинстве случаев сделано так, как у меня. Подробный список я привёл в другом моём комментарии.
Некоторые кусочки вашего кода также непонятны, как и мои. Например:
Поэтому не могу вам дать плюс за решение всех проблем понятности кода.
Итого, ваш код немного более понятен, чем мой. Но не принципиально. И есть свои недостатки.
Я, как на образец, всегда смотрю в исходный код самого .Net. Там регулярки используются только в очень высокоуровневом коде типа UI или СУБД. Примеры где парсинг делается также, как у меня (ищите в них '0' или * 10):
System.Text.RegularExpressions.RegexParser
System.Numerics.BigNumber
System.Globalization.TimeSpanTokenizer
System.Uri
System.Xml.XmlTextReaderImpl
System.Text.StringBuilder
System.IPv4AddressHelper
System.DateTimeParse
Кое где они напрямую пишут что хотели реализацию без регулярок.
Спасибо за мнение. Вот я в поисках "краткого и понятного" кода гляжу метод GetTimeAfter() в библиотеке Quartz, который делает то же самое что у меня. Делал уже несколько подходов, но полностью логику пока не понял. Зато не много вложенных if и выход из цикла показан в последней его строке (которая примерно на 10 экранов отстоит от начала цикла). Мой цикл виден на одном экране, а вложенность if отражает натуральную вложенность компонентов даты (месяцы вложены в года, дни вложены в месяцы и т.д.) Я посчитал такой вариант вполне пригодным для понимания. Но я, конечно, могу понять что в чьих то правилах может существовать жёсткий запрет на вложенные if.
В одной строке расписания заданы списки, диапазоны и периоды. Тут простой структурой не обойдёшься. Если я не прав, покажите подробнее, про что вы говорите.
Истекло время редактирования комментария, продолжу. У меня в цикле вложены элементы даты (строка год, строка месяц, строка день, строка час и т.д.) и вся вложенность выглядит как естественная иерархия элементов даты (ведь месяц вложен в год, а не идёт последовательно за ним). Лично для меня это повышало понятность алгоритма. Вы вот не разбирались в смысле кода, а зря. Я бы не согласился, что после правок стало принципиально читабельнее.
Спасибо за конкретику. Моя ошибка в том, что мне алгоритм был виден яснее именно в виде вложенных if на одном экране. Я не знал, что количество вложений if считается страшным злом даже если строки короткие и понятные.
Но вы где то ошиблись (видимо по мелочи). Алгоритм нарушен, тесты зависают.
Тогда извините, не сориентировался в потоке комментариев
«Регекс это грубо говоря стандарт. Его используют куча людей...Ваш вариант это вещь в себе. Спросить если что можно только у вас» — тут вы передёргиваете (так говорят когда тайно меняют карту в карточной игре). Мой вариант — это C#, который гораздо легче понять, отладить и подработать. Справится даже не профильный специалист, а например джававед. А Regex — сложная штука, в которой трудно разбираться и даже опытные легко могут сделать плохой Regex.
--- добавлено ---
«Ваш вариант» сказали не мне. Что не отменяет моих выводов про Regex!
«предложить конкретные улучшения». Вот это — разумный подход!
Не беспокойтесь про «задеть». Хотелось бы больше конкретики, что именно делает мой код выглядящим как «код неопытного программиста». Вот давайте, например, сравним с системным кодом Guid.TryParseGuid() где тоже идёт разбор строки.