Comments 10
Наверное требовалось русскоязычное наполнение, не всякие Джоны и Томсоны. А отечественные Ивановы и Сидоровы. Я для тестовой базы делал файлы: фамилий, имен, отчеств, названий фирм и улиц. После чего читал данные файлы и рэндомно создавал адреса, ФИО, и т.д И сохранял в csv после чего в PostgreSQL, писал через copy. Первичные ключи, тоже генерил и лил в базу. Плюсы, полный контроль за данными, можешь генерировать любые диапазоны.
Bogus можно расширять своими словарями и генераторами.
И всякие имена с адресами там вроде как из коробки могут генерироваться на нужном языке.
Делал однажды свой такой генератор правдоподобных автомобильных номеров.
и отсутствие опыта работы с asp net.как знания asp net должны помочь в рандомной генерирации данных, может речь шла о том, что нужно удалённо данные запросить у какого-то открытого api сервиса генерации данных.
Мне кажется, что тут просто очень формально к проверке подошли. В приложении вполне конкретно указано, что и куда должно выводиться. А судя по коду там в консоль лишние данные выводятся (ConsoleWriter не забыл убрать?) и ещё непонятно, как в json пишется (на рабочий стол нужно + в lowerCamelCase). Им может не хочется заморачиваться: что-то лишнее или не так в выводе - не прошёл. Тут задание всё-таки на джуна, наверняка ждут чёткую реализацию поставленной задачи.
Так-то задание в принципе подходит джуну: недолгое, проверяет базовые знания, а главное способность следовать ТЗ.
А по коду, может быть проблема в какой-то излишней переусложнённости. От джуна ожидаешь решение в лоб, а не класс под каждое действие.
Про Faker - в принципе хорошо, но, кажется, это не принципиально было для них, хватило бы любых рандомных значений, подходящих под типы. PersonsGenerator, генерирующий в конструкторе один раз - не очень решение, если уж генератор, то нужен метод типа Generate возвращающий список без сохранения в поле класса. Про сериализацию/десериализацию сложно что-то сказать. Как я понял JsonWriter|JsonReader - это кастомные классы? Не вижу, чтобы пути к файлу указывал. Если уж решил в ООП, то нужно передавать путь к файлу в них извне. ConsoleWriter даже не требовался по заданию. ValueCounter как-то тоже кажется лишним, там два простых Linq запроса. Это из того, что увидел (ссылки на исходники я чёт не нашёл).
Очень дельные замечания, со многим согласен, но я бы, пожалуй дополнил еще несколькими:
1) Генерируемые данные рассогласованы.
В задании нет четкого указания, на сколько корректными должны быть данные, генерируемые случайным образом. Но мне кажется, что стоило бы обеспечить минимальный уровень согласованности. Возможо, что автор задания хотел проверить детальность проработки задания. Наверно лучше уточнять такие детали перед выполнением задания. Вот, что я бы предположил:
Дата рождения не согласуется с возрастом, генерируются раздельно. Достаточно вычислять возраст на основе случайно сгенерированной даты рождения. Но я бы сделал Age вычисляемым свойством, что правда, противоречит предоставленной структуре класса Person.
Пол не согласуется с именем. Пускай это и в духе современных трендов, но мне кажется, что в данном случае это скорее не заранее продуманная фича.
Возраст детей не согласуется с возрастом родителя. Вполне вероятно, что случайные данные нагенерируют 12-летних детей у которых есть свои дети возрастом под 68 лет.
Фамилия детей не согласуется с фамилией родителя.
Возраст не согласован со статусом вступления в брак. 3 летние женатые дети.
Можно еще кучу ограничений придумать, это то, что пришло в голову с ходу. Возможно стоило бы генерировать сразу целые семьи с супругами и детьми, чтобы согласовывать эти данные.
Не понятно, должен ли идентификатор ребенка Child.Id быть согласованым с Person.Id. Но выглядит так, что скорее всего да. Тогда детей нужно создавать как Person, а родитель должен ссылаться на реальных Person с реальными Id.
2) Нет консистентности в названии классов.
Одни классы называются ***Generation, другие ***Generator. Думаю, что следует придерживаться одного подхода к именованию.
3) Есть такое правило, что если класс помимо конструктора содержит всего один метод, то стоит задуматься, а нужен ли вообще такой класс. Возможно стоит конвертировать его в просто в метод, чтобы не слишком усложнять код и не плодить сущности.
Про то, что плохая практика в конструкторе класса делать "тяжелые" операции, не связанные с инициализацией объекта уже написали.
4) Я бы предложил использовать автоматическое выведение типов (var) в конструкциях, вроде:
JsonWriter<Person> jsonWriter = new JsonWriter<Person>();
JsonReader<Person> jsonReader = new JsonReader<Person>();
Заменил бы на
var jsonWriter = new JsonWriter<Person>();
var jsonReader = new JsonReader<Person>();
Это лучше читается, для такого "масла масленного" этот сахар и был придуман.
5) Вызов метода Dispose() у класса PersonsGenerator.
Класс PersonsGenerator судя по коду из статьи не реализует ни интерфейс IDisposable, ни собственно метод Dispose(). Скорее всего это опечатка, иначе этот код просто бы не работал.
Не понятно, что вообще должен в данном случае делать этот метод, так как очистка PersonsGenerator.persons производится явно, вне данного метода. Вероятно, эту операцию как раз надо засунуть внутрь Dispose(). И тогда использовать генератор вместе с конструкцией using:
using (var personsGenerator = new PersonsGenerator()) { ... }
Но я бы вообще не стал хранить в генераторе сгенерированные данные. Ответственность генератора - генерировать сущености, а не хранить их.
6) Не стоит сохранять в финальном коде отладочные куски, я про это:
// Не обращаем внимание, так, для себя сделал
7) В дополнение к 6), для вывода небольших массивов в отладочной строке можно было бы воспользоваться методом string.Join(), тогда можно было бы элементы массивов выводить через разделитель, например через запятую.
Вообще мне кажется, что тут рефлексия не особенно нужна, можно было бы для класса Person переопределить метод ToString() и в явном виде форматировать данные. Или вообще для отладочного вывода просто печатать готовый json в котором уже произведена почти аналогичная сериализация данных, так сказать бесплатно и без лишнего кода.
8) Мне понравилась обертка для Faker, которая называется Bogus (https://github.com/bchavez/Bogus). Рекомендую посмотреть на ее реализацию создания декларативных генераторов, мне кажется, что это довольно красивое решение, которое иногда избавляет от необходимости написания своих классов-генераторов. Плюс эта библиотека содержит кучу дополнительных расширений, среди которых, например есть возможность генерировать данные для не-англоязычных сред, например имена на русском языке.
Все вышесказанное - мое имхо и не является истиной в последней инстанции, надеюсь какие-то из советов будут полезными, желаю удачи автору статьи с поиском работы!
Заменил бы на
var jsonWriter = new JsonWriter<Person>();
var jsonReader = new JsonReader<Person>();
В C# 10 и выше еще лучше:
JsonWriter<Person> jsonWriter = new();
JsonReader<Person> тако ужас, = new();А вообще код совершенно студенческий, что совсем необязателно плохо. Но если от автора ожидали увидеть код production уровня, то этот код совершенно не годится. Одна из причин почему я никогда и ни при каких условиях тестовые задания давно не делаю принципиально - даже при полной ясности задания никогда не ясно что же на самом деле от тебя ожидают увидеть.
И еще. Их английский настолько ужасен, что уже это должно оттолкнуть. Если хочешь писать на английском (или китайском, монгольском, албанском) делай это грамотно. Или не выеживайся и пиши на своем родном языке.
Дополню ещё.
Модификаторы доступа для полей не указаны, нужно всегда явно указывать. У классов тоже не у всех.
Если это private-поля, то не соблюдён кодстайл именования — должно быть
_phones,_childrenитдInt32,Int64вместоintиlongВообще типы все напрямую а не алиасами, как рекомендовано (
String>string, аналогично дляdouble,booleanитд)IsMarredвместоIsMarriedНет никакого смысла выводить вот эти вот
System.String[]. Это просто визуальный мусор, не несущий для человека информации.Вообще, кажется, в выводе есть лишнее. Самих Person не просили выводить, только записать и считать, как я понимаю. А вывести только общие показатели.
Оригинальный Faker идет с Ruby.
Он так же есть для Java, JS, PHP, python итд.
Вдруг кому пригодится.
Faker API для .NET — генерация случайных имен и других данных