Здравствуйте

решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так.

Прошу не стесняясь попинайте код пожалуйста.

https://github.com/dmitry-kolesov/TCBspline

Задание было следующим:

"1. Создать визуальный редактор сплайнов Кочанека-Бартельса
(tension/continuity/bias splines).

Функциональные требования:

* Работоспособность в MS Windows (7/8) или Linux.
* Визуализация сплайнов.
* Разделение кода графического интерфейса и логики приложения.
* Визуальное редактирование сплайнов:
— добавление управляющих точек,
— изменение значений tension/continuity/bias для управляющих точек,
— перемещение управляющих точек,
— удаление управляющих точек.

Бонусы:

* Сохранение и загрузка сплайнов,
* Функции undo/redo,
* Поддержка одновременного редактирования произвольного количества сплайнов.
"

Здравствуйте, amid493, Вы писали:

A>Здравствуйте

A>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так.
A>Прошу не стесняясь попинайте код пожалуйста.
A>https://github.com/dmitry-kolesov/TCBspline

Часть коммитов датируется двумя годами назад, Вы задание два года делали?

Здравствуйте, Handie, Вы писали:

H>Часть коммитов датируется двумя годами назад, Вы задание два года делали?

да это же задание было два года назад, сейчс кое что перерефакторил, сделал пару бонусов (undo/redo и сохранение)
ответ был тот же причем без пояснений к сожалению.

Здравствуйте, amid493, Вы писали:

A>Здравствуйте

A>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так.

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

потому что
1. если это стартап и берут человека со стороны, то скорее всего качество кода либо еще не важно, либо уже не важно, а значит тестовое задание ничего не значит и набор идет по др критериям которым вы не соответствуете
2. если это развитой корпоративный проект, то есть деньги и примеры для того, чтобы натаскать любого человека с головой не ниже среднего, и значит вы не прошли по др критериям
3. если это суперпроект, где важно качество кода, то туда берут совсем не по тестовому заданию, а по другим профессиональным критериям и в массы объеву кидают просто наугад чтобы случайно поймать суперспеца без работы, и ам тоже найм идет не по тестовому заданию
4. если это ни одно из перечисленного (стартап в активной фазе роста, неразвитой корпоративный проект, суперпроект являющийся таковым лишь в головах авторов) и работодатели верят, что они могут таким образом отсеять правильных людей, то они некомпетентны

просто идите искать работу дальше

Здравствуйте, watcher, Вы писали:

W>просто идите искать работу дальше

возможно я не совсем точно выразился
если кратко, то в 7-8 случаях из 10 качество кода либо не имеет принципиального значения либо есть ресурсы, чтобы натаскать любого
поэтому на эту тему париться не стоит

Здравствуйте, amid493, Вы писали:

A>Прошу не стесняясь попинайте код пожалуйста.
A>https://github.com/dmitry-kolesov/TCBspline


Я, так, сбоку по стилю пройдусь.
— А тесты где? У вас же куча математики в сторонних классах. Ну уж можно пару тестов написать для вида. К тому же Вас попросили отделить логику от представления.
— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/DrawHelper.cs — зачем он тут?
— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Common/XmlHelper.cs:

            try
            { ... }
            catch (Exception ex)
            {
                // log ex;
                throw ex;
            }

Зачем? Нет, я знаю зачем это иногда бывает надо (было надо), но в данном то примере зачем такой try-catch?

— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Common/SplineCalculator.cs

        initPoints = points;
                this.points = points;

Вы уж как-нибудь в одном стиле пишите.

— Там же, в конструкторе

            if (points != null && points.Length > 2)
            {
....
            }
            else
            {
        throw new Exception(...);
            }

А если points == null? А если измерения отрицательные? А если еще что-то?
Вы либо валидируете по-нормальному и толковые исключения бросайте, либо не трогайте. К тому же Вы перед вызовом конструктора SplineCalculator уже эту проверку выше по стэку делаете (в DrawController)

— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Controller/DrawerController.cs

       internal DrawerController(int _pictureBoxWidth, int _pictureBoxHeight)
        {
            InitPictureBox(_pictureBoxWidth, _pictureBoxHeight);
        }

        internal void InitPictureBox(int _pictureBoxWidth, int _pictureBoxHeight)
        { ...

Определитесь со стилем. Вы в конструкторе инициализируете поля или в отдельном init-like методе. В других местах у Вас по-другому.

— Там же. Куча Brush'ей, Pen'ов и Graphics'ов без using'a гуляет.

— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Controller/MainController.cs

 class MainController
    {
        private List<MyPointF> points = new List<MyPointF>();
  internal MainController(PictureBox pbx)
        {
            toUndoActionsList = new List<PointAction>();
        ...

Будьте последовательны. Инициализируете одно в одном месте (например, в конструкторе), там же и другое инициализируйте.

— https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Model/MyPointF.cs
Какой может быть Index у точки?
Вот вы создали свою точку, даже свою коллекцию точек создали, даже в контроллерах там где-то пытаетесь найти свою точку в коллекции. Наверное лучше было бы вместо кучи вспомогательных методов Equals() и GetHashCode() переопределить.

https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Model/PointsCollection.cs
— Я не понял смысл этого класса.

Здравствуйте, watcher, Вы писали:

W>Здравствуйте, watcher, Вы писали:

W>>просто идите искать работу дальше

W>возможно я не совсем точно выразился
W>если кратко, то в 7-8 случаях из 10 качество кода либо не имеет принципиального значения либо есть ресурсы, чтобы натаскать любого
W>поэтому на эту тему париться не стоит

а в оставшихся 2-3 найм ведется совсем не по тестовым заданиям
и если даже объява в открытом доступе и задания раздаются, то это пустая формальность

Здравствуйте, amid493, Вы писали:

A>Здравствуйте

A>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так.

Может им просто код твой нужен был?

из трех случаев где я все-таки решился сделать тестовое задание (ибо заняться было особо не чем)
ни в одно место не устроился в итоге. работодатели не нравились — все какие-то мутные и/или с закидонами.
сделал для себя определенные выводы.
пс. а ваше задание вообще навороченное. писать много, а зачем — не понятно.

if (points != null && points.Length > 2)
{
......
}
else
{
if (points.Length > 2)
{
MessageBox.Show("Please select points");
throw new Exception("Points was not selected");
}
}

1. Что будет если points == null ? на строчке if (points.Length > 2)

2. catch (Exception ex)
{
// log ex;
throw ex;
}
Теряете стек трейс. throw ex vs throw;

3.

Graphics g = Graphics.FromImage(graphBmp);
DrawPoints(points, g);
UpdateImage(graphBmp);
g.Dispose();


Если проиходит exception в DrawPoints(points, g); или UpdateImage(graphBmp); то g.Dispose(); не вызовется..

лучше так:

using ( Graphics g = Graphics.FromImage(graphBmp))
{
DrawPoints(points, g);
UpdateImage(graphBmp);
}

Здравствуйте, watcher, Вы писали:

W>Здравствуйте, watcher, Вы писали:

W>>Здравствуйте, watcher, Вы писали:

W>>>просто идите искать работу дальше

W>>возможно я не совсем точно выразился
W>>если кратко, то в 7-8 случаях из 10 качество кода либо не имеет принципиального значения либо есть ресурсы, чтобы натаскать любого
W>>поэтому на эту тему париться не стоит

W>а в оставшихся 2-3 найм ведется совсем не по тестовым заданиям
W>и если даже объява в открытом доступе и задания раздаются, то это пустая формальность

Спасибо за поддержку, я особо не заморачиваюсь просто странно, что не дают обратной связи.

Здравствуйте, andrey.t, Вы писали:

AT>Здравствуйте, amid493, Вы писали:

A>>Прошу не стесняясь попинайте код пожалуйста.
A>>https://github.com/dmitry-kolesov/TCBspline


AT>Я, так, сбоку по стилю пройдусь.
AT>- А тесты где? У вас же куча математики в сторонних классах. Ну уж можно пару тестов написать для вида. К тому же Вас попросили отделить логику от представления.
AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/DrawHelper.cs — зачем он тут?
AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Common/XmlHelper.cs:
AT>

AT>            try
AT>            { ... }
AT>            catch (Exception ex)
AT>            {
AT>                // log ex;
AT>                throw ex;
AT>            }
AT>

AT> Зачем? Нет, я знаю зачем это иногда бывает надо (было надо), но в данном то примере зачем такой try-catch?

AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Common/SplineCalculator.cs

AT>

AT>        initPoints = points;
AT>                this.points = points;
AT>

AT>Вы уж как-нибудь в одном стиле пишите.

AT>- Там же, в конструкторе
AT>

AT>            if (points != null && points.Length > 2)
AT>            {
AT>....
AT>            }
AT>            else
AT>            {
AT>        throw new Exception(...);
AT>            }
AT>

AT>А если points == null? А если измерения отрицательные? А если еще что-то?
AT>Вы либо валидируете по-нормальному и толковые исключения бросайте, либо не трогайте. К тому же Вы перед вызовом конструктора SplineCalculator уже эту проверку выше по стэку делаете (в DrawController)

AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Controller/DrawerController.cs
AT>

AT>       internal DrawerController(int _pictureBoxWidth, int _pictureBoxHeight)
AT>        {
AT>            InitPictureBox(_pictureBoxWidth, _pictureBoxHeight);
AT>        }

AT>        internal void InitPictureBox(int _pictureBoxWidth, int _pictureBoxHeight)
AT>        { ...
AT>

AT>Определитесь со стилем. Вы в конструкторе инициализируете поля или в отдельном init-like методе. В других местах у Вас по-другому.

AT>- Там же. Куча Brush'ей, Pen'ов и Graphics'ов без using'a гуляет.

AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Controller/MainController.cs
AT>

AT> class MainController
AT>    {
AT>        private List<MyPointF> points = new List<MyPointF>();
AT>  internal MainController(PictureBox pbx)
AT>        {
AT>            toUndoActionsList = new List<PointAction>();
AT>        ...
AT>

AT>Будьте последовательны. Инициализируете одно в одном месте (например, в конструкторе), там же и другое инициализируйте.

AT>- https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Model/MyPointF.cs
AT>Какой может быть Index у точки?
AT>Вот вы создали свою точку, даже свою коллекцию точек создали, даже в контроллерах там где-то пытаетесь найти свою точку в коллекции. Наверное лучше было бы вместо кучи вспомогательных методов Equals() и GetHashCode() переопределить.

AT>https://github.com/dmitry-kolesov/TCBspline/blob/master/TCBspline/Model/PointsCollection.cs
AT> — Я не понял смысл этого класса.


Спасибо за подробный ответ)

Это запросто могла быть чья-то лаба или курсовая, а тебя просто использовали.

К>Это запросто могла быть чья-то лаба или курсовая, а тебя просто использовали.

Сейчас посмотрел задание .. это вообще чей то диплом скорее. Такая большая работа не может быть тестовым заданием.

Здравствуйте, amid493, Вы писали:

A>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так.

Зря, для решения реальных задач логично было взять opencascade. Выполнять сложные тестовые задания при поиске работы только терять время. Большинство людей сами не знают чего они хотят.

Здравствуйте, amid493, Вы писали:

A>* Работоспособность в MS Windows (7/8) или Linux.

Вот эта строчка смутила. Точно они код на .NET хотели?
Ты хоть выяснил чем они занимаются и какие у них потребности?

Здравствуйте, amid493, Вы писали:

	Скрытый текст
	A>Здравствуйте A>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так. A>Прошу не стесняясь попинайте код пожалуйста. A>https://github.com/dmitry-kolesov/TCBspline A>Задание было следующим: A>"1. Создать визуальный редактор сплайнов Кочанека-Бартельса A>(tension/continuity/bias splines). A>Функциональные требования: A>* Работоспособность в MS Windows (7/8) или Linux. A>* Визуализация сплайнов. A>* Разделение кода графического интерфейса и логики приложения. A>* Визуальное редактирование сплайнов: A> — добавление управляющих точек, A> — изменение значений tension/continuity/bias для управляющих точек, A> — перемещение управляющих точек, A> — удаление управляющих точек. A>Бонусы: A>* Сохранение и загрузка сплайнов, A>* Функции undo/redo, A>* Поддержка одновременного редактирования произвольного количества сплайнов. A>"

Слишком жирно для тестового задания.

ИМХО, как писали выше, им был нужен твой код. Гнилая контора. Либо тебя кинули хитрые студенты.

Здравствуйте, vpchelko, Вы писали:

V>Здравствуйте, amid493, Вы писали:
V>

	Скрытый текст
	A>>Здравствуйте A>>решал тестовое задание и получил ответ, что типа вы нам не подходите без объяснения что в коде не так. A>>Прошу не стесняясь попинайте код пожалуйста. A>>https://github.com/dmitry-kolesov/TCBspline A>>Задание было следующим: A>>"1. Создать визуальный редактор сплайнов Кочанека-Бартельса A>>(tension/continuity/bias splines). A>>Функциональные требования: A>>* Работоспособность в MS Windows (7/8) или Linux. A>>* Визуализация сплайнов. A>>* Разделение кода графического интерфейса и логики приложения. A>>* Визуальное редактирование сплайнов: A>> — добавление управляющих точек, A>> — изменение значений tension/continuity/bias для управляющих точек, A>> — перемещение управляющих точек, A>> — удаление управляющих точек. A>>Бонусы: A>>* Сохранение и загрузка сплайнов, A>>* Функции undo/redo, A>>* Поддержка одновременного редактирования произвольного количества сплайнов. A>>"

V>Слишком жирно для тестового задания.

V>ИМХО, как писали выше, им был нужен твой код. Гнилая контора. Либо тебя кинули хитрые студенты.

Здравствуйте, vpchelko, Вы писали:

V>Слишком жирно для тестового задания.

Вполне нормальное задание. Когда в своё время в серьёзные конторы собеседовался такого же масштаба работы делал.

D>Вполне нормальное задание. Когда в своё время в серьёзные конторы собеседовался такого же масштаба работы делал.

Обычно таким образом набирают студентов.