Skip to content
This repository has been archived by the owner on Jan 12, 2022. It is now read-only.

first request #6

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

first request #6

wants to merge 9 commits into from

Conversation

averveiko
Copy link

No description provided.

@aspcartman
Copy link
Contributor

Ждем исправлений! :) Если у вас какие-то вопросы - не стесняйтесь открывать тему в обсуждениях на хекслет, писать мне на почту или же тут в коментариях.

@rdtft
Copy link

rdtft commented Oct 31, 2012

Я бы посоветовал выносить категории в отдельный файл.

A common naming convention is that the base file name of the category is the name of the class the category extends followed by “+” followed by the name of the category

подробней почитать можно тут

#CHANGES:
+ Новая реализация mutate с использованием вспомогательного метода
shuffleArray;
+ Добавлен вспомогательный метод shuffleArray перемешивающий случайным
образом NSArray;
+ При замене символа ДНК проверяется отличие нового символа от
заменяемого;
+ Все константы обьявлены в #define;
+ Перевод "процентов в штуки";
+ Категория вынесена в отдельный файл;
+ getRandomDNASymbol больше не нужна, перенесена в init;
+ Использование новых плюшек Xcode и Objective-C (обьявление, доступ к
массивам через []).
@averveiko
Copy link
Author

ryuk, Спасибо за совет, так и сделал.

CHANGES:
Устранены мелкие баги
Массив чисел в mutate создается только при первом обращении у методу
Упрощена реализация выбора символа ДНК отличного от имеющегося
(Выбирается случайно пока не попадется отличный,
в данном случае это быстрее создания массива и его полная перетасовка
как ьыло ранее (есть еще несколько вариантов но они думаю оверхед,
попасть на тот же символ вероятность мала))
Инит вынесен отдельно

Добавлен вывод для удобной проверки (раскомментируйте // DEBUG в
Cell+mutator.m)
CHANGES:
Новая улучшенная реализация выбора случайного символа для замены;
Проверка параметра percent;
Добавлены комментарии;
Готово для сдачи.

Для удобной проверки раскомментируйте // DEBUG: в Cell+mutator.m
Сдеал свойства readonly
@dmitrii-davidov
Copy link

Интересная реализация. В целом претензий не имею.

@averveiko
Copy link
Author

Спасибо) Долго пилил.

@dmitrii-davidov
Copy link

По-моему просто переусложнили)

@averveiko
Copy link
Author

Дык первый коммит очень простой, усложнял отталкиваясь от комментариев преподов)
И например если следовать всем рекомендациям https://u.hexlet.org/courses/4/discussion_topics/26
то есть куда еще усложнять: убрать свойства и сделать новый метод для доступа к элементам DNA.
Но не стал уже, хватит)

@averveiko
Copy link
Author

И кстати, если я Вам назначен для проверки,чтоб оценку поставить надо ткнуть Show Rubric в Peer Review)

@dmitrii-davidov
Copy link

А вот за это прям огромное спасибо, растерялся)

@averveiko
Copy link
Author

Вам спасибо, за проверку) Сейчас сам сижу проверяю назначенных.

if (percent == 0) return;

// Переводим процент в количество элементов
int count = DNA_LENGHT * percent / 100;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Некорректное определение кол-ва элементов для замены, для массива со 100 элементами сработает, если указать, например, 170 - то нет.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно подробнее почему не работает?
Запись эквивалентна записи int count = round(DNA_LENGHT * percent / 100);
Т.к. числа целые округляется и без round.
Или я не прав?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Н-р при 170 элементах массива нам надо найти 15%
DNA_LENGHT * percent / 100 выдаст 25
round(DNA_LENGHT * percent / 100) выдаст 25

Copy link

@slebedev slebedev Nov 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Запись не эквивалентна. В первом случае будет просто целая часть дробного числа, а во втором - округленное число.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лыжы не едут)
Долго думал - не додумал, подскажите при каких условия будет неверное значение.
Вот код для теста:
for (int proc=0; proc<100; proc++) { int a = DNA_LENGHT * proc / 100; //Моя реализация int b = round(DNA_LENGHT * proc / 100); //с раунд int c = (float)DNA_LENGHT / 100 * proc; // подсмотрел у l0rda NSLog(@"%i, %i, %i", a, b, c); }
Выдает одинаковые значения для диапазона процентов от 0 до 100 (DNA_LENGHT = 170)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут забавная штука на самом деле :)
В вашем варианте работать будет(не факт, что во всех случаях), а вот моем не работало, из-за того что делил, а потом умножал.
Код для примера выдаст разные результаты:
int numberIndexesToReplace = (float)DNA_LEN / 100 * mutatePercent;
int numberIndexesToReplace1 = DNA_LEN / 100 * mutatePercent;
NSLog(@"%d", numberIndexesToReplace);
NSLog(@"%d", numberIndexesToReplace1);

если деление и умножение поменять местами, то результат будет одинаков :)
Это питоновская привычка, при делении переводить числитель в float:

271 / 100 * 70
140
271 * 70 / 100
189
271.0 / 100 * 70
189.7

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вы как-то синхронно напали, думал и правда не верно.
Да, если разобраться это не арифметическое округление как в школе учили, в моем случае дробная часть просто отбрасывается.
round - по идее банковское округление (половина округляется к ближайшему четному)

В задании не сказано как именно нужно - вариантов много ( в большую сторону, арифметическое, попеременное).
По этому и не стал ничего пока выдумывать, пока не уточнили.

Copy link

@slebedev slebedev Nov 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как хотите, так и сделайте, просто имейте четкое представление, как у вас делается. Не считаю отсутствие округления поводом снижать оценку. Но вы должны понимать, почему работает так, а не иначе.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Согласен, с round будет именно так "как учили в школе". Сейчас запилю.

Добавлен round для тру-округления
Довел до ума округление
Причесал еще код.
Оценка уже получена.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants