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

first project Dronnn #4

Closed
wants to merge 12 commits into from
Closed

first project Dronnn #4

wants to merge 12 commits into from

Conversation

Dronnn
Copy link

@Dronnn Dronnn commented Oct 30, 2012

first project

first project
@aspcartman
Copy link
Contributor

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

изменил метод для мутации
и
убрал дублирующееся объявление временного массива
Исправил недочёты на которые мне указали
@arabesc
Copy link

arabesc commented Nov 5, 2012

main.m:
Добавьте, пожалуйста, мутацию и для второй клетки, в соответствии с п.8 задания.

Категорию mutator желательно реализовать в отдельных файлах.

Cell.h(11):
Объявлять глобальные переменные в хидере плохая идея.

Cell.m(15):
При каждой инициализации Cell переинициализируется глобальная переменная ATGC. Не ошибка, но как-то криво.

Cell.m(20):
Не ошибка, но в Си принято итерировать циклы от 0 до границы, особенно, если тело цикла не зависит от счетчика, так просто привычнее читать, например: for (int i = 0; i != 100; ++i)

Cell.m(48):
Не ошибка, но ветка else лишняя. Если условие не сработало, дальнейший код можно просто выполнять, без else.

Cell.m(50):
Нет гарантии, что random не даст несколько одинаковых идексов за всю серию, что, во-первых, приведет к мутации одного элемента более одного раза, а во-вторых, изменит меньший процент элементов, чем требуется, что не соответствует п.7 задания.

@aspcartman
Copy link
Contributor

За i != 100 яб тюкнул =).

Делаем гденить в цикле i+=чтонить и бам мы за пределами. Супер, безудержное веселье =)

@arabesc
Copy link

arabesc commented Nov 5, 2012

А не надо в теле цикла делать i+=чтонить :) Все равно i не видно вне тела цикла, а для прерывания итераций есть break. Для i+=чтонить есть, например, более подходящий while. Если уж очень надо, всегда можно написать <, а запись != подчеркивает строгость условия.

@aspcartman
Copy link
Contributor

@arabesc
Подчеркивает это лишь то, что i не должна быть равной 100 и только. Т.е мы можем гулять как хотим (например реализовать внутри алгоритм бинарного поиска - спокойно прыгать от 1 до 200 и обратно, минуя сотню), а как только i=100 прям как на мину наступили.

Нет, если речь идет о простом переборе, то только i<100 или i<=100 и никак иначе. Условие for должно выполнятся там и только там, где должно по алгоритму.

@arabesc
Copy link

arabesc commented Nov 5, 2012

@aspcartman, вывод Ваш совершенно никак не следует из предыдущего абзаца. Если это простой перебор, то тем более нет никаких причин перескочить за границу цикла.
Спорить не вижу смысла. Объективных аргументов против != для случая простого перебора нет, есть у некоторых субъективные страхи вроде "а вдруг однажды перескочит?!" и остаются личные предпочтения, но тут на вкус и цвет все фломастеры разные.
Аргумент за != могу привести. Если вдруг что-то меняется в логике и счетчик перескакивает за границу цикла, причем, изначально это не подразумевалось, когда писали именно != вместо <, то программа с большей вероятностью начнет вести себя неадекватно и может быть даже упадет где-то рядом с телом цикла или внутри него, что позволит быстро локализовать и исправить ошибку. Если писать < без необходимости, это значит задать изначально более гибкие условия, можно пропустить часть ошибок, которые будет труднее локализовать, т.к., например, цикл всего лишь проработает меньшее количество итераций, чем нужно, обратится к меньшему, чем нужно, диапазону адресов, что не вызовет фатальных последствий, но нарушит логику. Пример: в теле цикла по i нужно модифицировать переменную j, но в результате опечатки программист меняет ту же i - вот ошибка, представьте последствия для разных вариантов условия и как быстро они станут заметны.

Ысчо один маленький косячокс
@aspcartman
Copy link
Contributor

Условие for должно выполнятся там и только там, где должно по алгоритму.

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

то программа с большей вероятностью начнет вести себя неадекватно и может быть даже упадет где-то рядом с телом цикла или внутри него

Может быть даже упадет где-то рядом с телом - понравилось. =) Ага. Щас. Программа с большей вероятностью побьет данные. Если такой код уйдет в релиз и из-за него кто-то потеряет важные данные - вам будет очень плохо. Очень, и лучше не проверять. Так-же это прекрасная дырка для очередного эксплойта. Программирование это достаточно строгая наука, давай-те в матане интегрировать по области так-же? =)

В тему

Вот еще пример небезопасного кода, специально сбегал за слайдами институтских лекций, думаю полезно будет.

/* Участок памяти ядра содержит данные доступные пользователю */ 
#define KSIZE 1024 
char kbuf[KSIZE]; 

/* Копируем не более maxlen байт из области ядра в буфер пользователя */ 
int copy_from_kernel(void *user_dest, int maxlen) { 
 /* Счётчик байт len – минимальное из размера буфера и maxlen */ 
 int len = KSIZE < maxlen ? KSIZE : maxlen; 
 memcpy(user_dest, kbuf, len); 
 return len; 
}

Вроде бы все прекрасно, да? Ничего не предвещает беды. Типичное использование:

#define MSIZE 528 
void getstuff() { 
 char mybuf[MSIZE]; 
 copy_from_kernel(mybuf, MSIZE); 
 printf(“%s\n”, mybuf); 
}

Однако же что будет, если мы поставим перед MBSIZE минус?

copy_from_kernel(mybuf,-MBSIZE);

А вот что - memcpy берет как аргумент unsigned int и получив отрицательное значение молча воспримет его как положительное (можете сами посчитать какая здоровая цифра получается в итоге) и спокойненько так скопирует вам что "попросили". Вот вам и не ограничили область как следует, и в итоге - настоящая дырень.

С вашим i != 10 можно тоже придумать много примеров, когда программист зазевается и не заметит возможность перескока, как не заметили разработчики ядра FreeBSD (откуда этот кусок и выдран), а ведь люди не глупые.

@arabesc
Copy link

arabesc commented Nov 5, 2012

Вы предлагаете для того, чтобы проверить исправность подвески расшатать в велосипеде гайки.

Программа, это не механизм, где есть материалы и сопромат, это детерминированный алгоритм, поведение которого должно быть подчинено логике, а не складываться из совокупности широких допущений, что не исключает возникновение ошибок, но лишь несколько снижает вероятность их проявления. Т.е. это такая своеобразная "хорошая мина при плохой игре" - ошибки внутрь программы прокрались, курсируют по структурам данных, но до поры получается их скрывать.
Встречаются такие программы, которые однажды почему-то начинают работать не так хорошо, как раньше, вместо того, чтобы упасть и сказать, что вот есть такая проблема, надо исправить. Наверняка в них накопились многоуровневые слои сглаживающего ошибки кода, из под которого эти ошибки уже никто и никогда не воковыряет и не поймет, что же именно и где начинает идти не так.

Аргумент "и так должно работать" или "это поможет заметить багу" не существенны.

Не "и так должно работать", а "именно и только так должно работать". Если случилось что-то другое, а разработчик этого не хотел, значит, проблема должна проявиться как можно скорее, чтобы ее можно было решить в зародыше, а не тихо размазаться в недрах программы. Про ненужность "это поможет заметить багу" тоже понравилось :) Вы должно быть и ассерты не ставите?

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

По Вашей логике получается, что пусть лучше программа уйдет в релиз с незаметным багом и долго по чуть-чуть в редких и непонятных ситуациях будет портить данные пользователя, чем обнаружит свои недостатки максимально быстро и явно, может даже еще на этапе QA.

Программирование это достаточно строгая наука

И сами себе противоречите. Пишете про строгую науку, но защищаетесь от каких-то чудес, когда детерминированный алгоритм вдруг начнет себя вести не так, как ему предписывал разработчик.
Если, например, вдруг космические частицы прошили память и консистентность ее содержимого нарушилась, то пусть уж лучше программа быстрее упадет, сигнализируя об аварии, и не вводит в заблуждение арбитра, сравнивающего показания с дублерами, чем будет максимально долго агонизировать, перемалывая чепуху.

Однако же что будет, если мы поставим перед MBSIZE минус?

Не пришлось дочитывать до этого места, чтобы понять, где подвох. Вы предлагаете грубые ошибки не выявлять на раннем этапе и исправлять путем правильного дизайна программы и верификации ее данных, а амортизировать ущерб от них путем расстановки бесчисленных смягчающих возможные негативные последствия проверок?

@aspcartman
Copy link
Contributor

Вам не кажется, что вы излишне экстраполируете? Мне, и не мне одному, видится что вы притягиваете аргументы за уши.

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

И давайте не будем переходить на личности, это не красиво и только мешает (я про ассерты) =).

Стоит определить четкие критерии, по которым можно сравнивать подходы. Ваша аргументация дает подходу право на жизнь, но я бы не советовал его и сам бы не начал использовать, тк он может привести к потере важных данных в случае чего, и никого не будет волновать, что это вы так багу ловите, на живца. А ведь бага может всплыть и не от вас, а от сторонней либы, части которой использовались в цикле. Нет, данные пользователя - святое. На баге мы должны либо вывалиться, либо оперативно его обработать, если это возможно. Ваш подход если и помогает вывалиться, то за счет данных, а это, считаю, не допустимо.

От @2k00l

  1. с точки зрения конвенций и читабельности, "!=" проигрывает. < используют практически все, людям нужно будет думать зачем это сделано.
  2. с точки зрения отладки, не вижу как это помогает отлаживать код, нужно просто уметь ассертики правильно расставить.
  3. с точки зрения скорости, можно конечно мерить, но я не думаю, что разница будет какая-то
  4. с точки зрения "я повыебываться хочу", "!=" конечно выигрывает тут

На пункт 4 думаю стоит закрыть глаза.

@arabesc
Copy link

arabesc commented Nov 5, 2012

Вам не кажется, что вы излишне экстраполируете?

Не кажется. Я вам про простой перебор, где != достаточное условие, а мне в ответ про чудеса с внезапно прыгающим счетчиком, где < уже необходимое условие.

и не мне одному

Кто здесь?!

Когда я ставлю четкие условия я знаю, что i никуда за границы дозволенного не выйдет

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

особенно это важно если внутри цикла она притерпевает изменения

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

@aspcartman
Copy link
Contributor

Здесь - команда hexlet в неполном составе =)

Похоже я действительно экстраполировал сам. Да, если внутри цикла i не должна изменятся и имеет стандартный шаг - то метод себя, должно быть, оправдывает. Контраргументов не вижу, кроме ухудшения читаемости и случая с избитием данных при пропрыге из-за непредвиденных обстоятельств =).

@aspcartman
Copy link
Contributor

Ой мы тут в коммите у @Dronnn устроили. Не хорошо.

@arabesc
Copy link

arabesc commented Nov 5, 2012

Наверное, еще и письмами с уведомлениями заспамили :) Можно поудалять лишнее, вроде более менее к консенсусу удалось прийти.

@aspcartman
Copy link
Contributor

Да думаю пусть будет, если владелец не против. Пусть люди читают :)

@arabesc
Copy link

arabesc commented Nov 6, 2012

По теме, @Dronnn:
Хорошо бы все же реализовать категорию mutator в отдельных файлах. Смысл сего может быть в работе с данными, которые сейчас доступны через глобальную переменную ATGC, наличие которой заднию не противоерчит, но является не совсем удачным подходом с точки зрения ООП, в частности, нарушает принцип инкапсуляции.

Cell.m(50):
Скорее нужно так:

for (int i = 1 ; i <= percentM; i++)

если precentM равен 0, итераций быть не должно.

Алгоритм mutate рабочий, но не очень красивый. Представьте, что при заданной 100% мутации обработано вот уже 99 индексов, но никак не выпадает нужный 100-й, сколько может быть пустых итераций? Не так много, конечно, но это процессорное время. Если есть желание, подумайте над алгоритмом получше или посмотрите в других работах, на оценку это все равно не влияет.

Замечание номер раз: Хорошо бы все же реализовать категорию mutator в
отдельных файлах.
Ответ: Так было сделано на лекции. Поэтому я и сделал так.
Результат: Исправлено.

Замечание номер два: наличие глобальной переменной заданию не
противоречит, но является не совсем удачным подходом с точки зрения ООП.
Ответ: Сделать глобальную переменную мне посоветовал один из
проверяющих. В книге Аарона Хилегаса "Objective-C. Программирование для
iOS и MacOS" использование глобальных переменных вполне допускается и
даже рекомендуется, например, как замена #define
Результат: Исправлено.

Метод mutate: полностью изменён.
@Dronnn
Copy link
Author

Dronnn commented Nov 6, 2012

Привет.
Замечание номер раз: Хорошо бы все же реализовать категорию mutator в отдельных файлах.
Ответ: Так было сделано на лекции. Поэтому я и сделал так.
Результат: Исправлено.

Замечание номер два: наличие глобальной переменной заданию не противоречит, но является не совсем удачным подходом с точки зрения ООП.
Ответ: Сделать глобальную переменную мне посоветовал один из проверяющих. В книге Аарона Хилегаса "Objective-C. Программирование для iOS и MacOS" использование глобальных переменных вполне допускается и даже рекомендуется, например, как замена #define
Результат: Исправлено.

Метод mutate: полностью изменён.

Исправлено: Проценты считаются не только при длинне цепочки = 100
@arabesc
Copy link

arabesc commented Nov 6, 2012

Сделать глобальную переменную мне посоветовал один из проверяющих.

Можно сделать глобальную статическую переменную в Cell.m, таким образом, доступ к ней будет только в контексте Cell. Если потребуется доступ к этим данным снаружи, например, из категории mutator при реализации в отдельных файлах, можно в Cell добавить статический метод (метод класса), возвращающий указатель на данные.

@Dronnn Dronnn closed this Feb 15, 2018
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

3 participants