LINUX.ORG.RU
Ответ на: комментарий от anonymous

Да, я в курсе. Забыл исправить, но уже поздно.

И ещё s/islife/isalive/g

itn ★★★
() автор топика

life/death

почему не enum?

anonymous
()

Сам стиль кода хорош.

Свитч с getopt_long лучше всё-таки вынести в отдельную процедуру.

Массив массивов - ненужное здесь решение. Достаточно одномерного массива размерности width*height, доступ к ячейке - map[x + y * width]. И не очень хорошо, что она квадратная.

islife() - ужасен. Стена if-ов, каждый, по сути, делает одно и то же действие. Всё это можно заменить двойным циклом [x-1..x+1]*[y-1..y+1], и уже внутри учитывать переход через границы. Или хотя бы свернуть в одно выражение, в котором суммируют эти 8 значений - даже так меньше строк и проще читать.

А ещё взгляни на ncurses.

E ★★★
()

int ***map

Один астериск таки можно выкинуть. Ну и да, регистрант выше сказал про одномерный массив.

anonymous
()

gcc -Wall должен выдать массу информации. Стремление к хорошему стилю это хорошо, но явные предупреждения компилера надо убирать.

MKuznetsov ★★★★★
()
void life(void)
{
        int i;
        int **map;
        for (i = 1; i <= max_gen; i++) {
                if (i == 1) {
                        map = init_map();
                        random_map(&map);
                        printf("\e[34m шаг %d\n", i);
                        print_step(map);
                } else {
                        next_step(&map);
                        printf("\e[34m шаг %d\n", i);
                        print_step(map);
                }
        }
        free_map(map);
}

ты издеваешься?

qulinxao ★★☆
()

Мне кажется, нет смысла использовать вложенные массивы, насколько я знаю, обычно обходятся одним буффером и array[row_index * columns_count + column_index].

Строковые литералы с русским текстом - скорее всего, плохая идея.

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

Я бы не пердавал по коду int**, а завернул бы это в структурку (например, Life) и все функции для работы с ней начинал бы с префикса (life_@@@). Особенно пугающе выглядит с тремя звездами в void next_step(int ***map).

Есть парочка глобальных переменных, лучше бы их все убрать.

В названиях функций islife и isborder нет '_', но в остальных функциях он есть.

В init_map есть неиспользуемая перменная, простой gcc -Wall это ловит.

Смешанные объявления перменных и код пойдут для новых стандартов Си, но не для трушного Си - error: ISO C90 forbids mixed declarations and code. Хотя, раз ты не писал для старого Си, то можно было бы писать for (int i = 0; ...) вместо int i = 0; for(i = 0;...).

ozkriff
()
Ответ на: комментарий от qulinxao
void life(void)
{//max_gen>=1
        int i;
        int **map;
        map = init_map();
        random_map(&map);
        for (i = 1; i < max_gen; i++) {
                printf("\e[34m шаг %d\n", i);
                print_step(map);
                next_step(&map);
        }
        printf("\e[34m шаг %d\n", max_gen);
        print_step(map);
        free_map(map);
}
qulinxao ★★☆
()
                map[i] = calloc(size_m, sizeof(int));
                if (map[i] == NULL) {
                        fprintf(stderr, "%s: ошибка выделения памяти\n", name);
                        exit(0);
                }

в больших программах лучше оберни calloc и вызывай abort. так сделано в gtk и c++, никто не жалуется.

crowbar
()

и кстати, ОНО вообще работает ?

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

MKuznetsov ★★★★★
()

Субъективное

Нравится идея тренироваться на «жизни», нравится getopt вместо велосипедов (безотносительно к хорошести getopt как такового).

Не нравится calloc на каждом ходу (для bmap) — решается либо выделением bmap заранее, либо кодированием «умирающих» и «родившихся» ячеек отдельными значениями в «основной» карте.

Не очень нравится «двумерный массив» как указатель на указатели; понятно, что при динамическом размере массива так приходится делать, если мы хотим красивый синтаксис [x][y], но я бы наплевал на красивый синтаксис и сделал бы плоский кусок памяти с ручным пересчётом индексов.

Местами (next_step, random_map) без необходимости передаётся указатель на указатель на указатели, при том что функция меняет только содержимое игрового поля, и с таким же успехом можно было бы передавать указатель на указатели (int **map вместо int ***map).

isborder неудачно названа (clamp_to_border?), islife (даже если isalive) тоже намекает на булевский результат, что не вполне соответствует идее определять символические константы LIFE и DEAD (это не про работу кода, а про эстетику). ESC[34m — баловство, либо ncurses (slang, или прочее умеющее terminfo/termcap), либо не раскрашивать бы вообще.

Подсчет соседей в islife я бы оставил в виде цикла, пусть компилятор разворачивает (восемь проверок долго читать глазами, если там была бы опечатка — легко было бы её пропустить).

Постановка задачи для бесконечного поля («пока хватит памяти», ну пусть координаты в пределах SIZE_MAX) была бы веселее. Компромиссная постановка задачи — следить за выходом за границы поля, обнаруживая ситуацию, когда ограниченность поля влияет на результат, и предупреждая об этом (для случайного заполнения поля с плотностью 1/2 в этом смысла нет, так как ограниченность почти всегда будет влиять на результат; но если, скажем, программа сможет читать стартовую конфигурацию и «крутить» именно её — станет осмысленно).

LeninGad
()
Ответ на: комментарий от beastie

Из замеченных неточностей:

0 - это числовой ноль, поинтер ноль — это NULL

аргументы с вдоеточием имеют аргументы, проверять имеют ли они их на самом деле лишнее.

Всей этой проверкой занимается сам getopt.

т.е. было:

{"size", 1, 0, 's'},
/* ... */
getopt_long(argc, argv, "sc", long_options, &option_index))
надо:
{"size", 1, NULL, 's'},
/* ... */
getopt_long(argc, argv, "s:c:", long_options, &option_index))

Далее. В libc есть преопределённая extern char *__progname, вырывать название проги из argv[0] — моветон.

Ну и слишком много скобок.

И ещё: для уведомлений/ошибок использух errx()/warnx().

beastie ★★★★★
()
Последнее исправление: beastie (всего исправлений: 2)
Ответ на: комментарий от beastie

У меня там баг закрался: в case 'c' usage() и warnx() перепутанны местами.

beastie ★★★★★
()
Ответ на: комментарий от beastie

0 - это числовой ноль, поинтер ноль — это NULL

Очередной икзперд.

Стандарт С, п. 6.2.2.3

An integral constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant.

Да, предложение убивать за отступы табами уже поступало?

alegz ★★★★
()
Ответ на: комментарий от beastie

В libc есть преопределённая extern char *__progname

а использование нестандартных расширений без причины не моветон?

anonymous
()
Ответ на: комментарий от alegz

Убивать надо за пробелы! :D Only true tabs, nothing else!

И таки, читать и понимать who-is-who легче NULL а не аморфный 0. (То, как оно определено, я в курсе.)

beastie ★★★★★
()
Последнее исправление: beastie (всего исправлений: 1)
Ответ на: комментарий от anonymous

По крайней мере я не встречал ни одной системы, где __progname не было бы. (Да, изначально это BSD-изм, но он же теперь и GNU-изм и давольно таки давно.)

beastie ★★★★★
()

qulinxao, ого какой промах. Понял ошибку только когда ткнул носом.

ozkriff, насчёт неизменяемых переменных внутри функций я знаю, но каждый раз писать example(const int x, const int y, и т.д.)... И да, нет единообразия в названиях процедур. gcc -Wall приму к сведенью.

LeninGad, bash color - это баловство для наглядности. Насчёт выделения цельного куска памяти... это же никак и ни на что не влияет... или влияет? В свою защиту скажу что так как я сделал намного наглядней.

beastie, спасибо многого не знал про getopt и libc.

alegz, табы мне удобны.

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

itn ★★★
() автор топика
Ответ на: комментарий от beastie

0 - это числовой ноль, поинтер ноль — это NULL

NULL, тащемта, макрос на тот самый «числовой 0», объявленный в стандартных либах (да да,в пустом майне он не сработает). То что ты имеешь в вилду называется std::nullptr и находится в std=c++11

comp00 ★★★★
()

Код не читал, посмотрел лишь оформление, лично меня бесят {} с новой строки: слишком размашистый код становится. Да, IDE умеет скрывать содержимое, но не всегда IDE нужно и не всегда оно будет под рукой.

comp00 ★★★★
()
Ответ на: комментарий от IIIypuk

На хабре же читал.
IIIypuk ★ (01.09.2014 23:40:00) школота

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

comp00 ★★★★
()
Ответ на: комментарий от itn

Насчёт цельного куска памяти — это влияет на производительность, в основном двумя путями: (1) malloc бывает тяжёлым сам по себе, и (2) доступ к элементу через лишнее разыменование указателя может быть тяжелее, чем через арифметику. В этой задаче (1) было бы по барабану, если бы на каждом ходу не выделялся новый bmap (вот не надо так делать), а заметность (2) будет зависеть от размера поля.

Насчёт наглядности — приходится выбирать между наглядностью доступа к элементу и наглядностью выделения и освобождения памяти. Выбрать первое можно потому, что доступ к элементу бывает на каждом шагу, а выделение памяти раз в жизни (но сверните цикл по соседям — и доступ уже не на каждом шагу); выбрать второе можно потому, что в доступе к элементу труднее ошибиться, чем в управлении памятью.

LeninGad
()
Ответ на: комментарий от anonymous

Поддерживаю. Кодить «для себя» лучше на go/D. Гораздо больше удовольствия. Кстати, кто знает, на go можно писать .so ? Чтобы вызывать их из c/c++.

menangen ★★★★★
()

Внесите Царя.

anonymous
()
Ответ на: комментарий от alegz

это все знают, вообще-то имеется в виду, что хорошим стилем является использование NULL вместо 0, чтобы подчеркнуть, что речь идёт об указателе

next_time ★★★★★
()
Ответ на: комментарий от alegz

Да, предложение убивать за отступы табами уже поступало?

Нет, ты будешь первым отметившимся дурачком.

anonymous
()
Ответ на: комментарий от IIIypuk

автор на хабре либо врал, либо писал не про Россию (ходят слухи, что в Индии и Китае так делают, но мне про эти страны точно не известно, что у них внутри с программированием)

next_time ★★★★★
()
Ответ на: комментарий от comp00

Языки лучше для написания программ прикладных.

menangen ★★★★★
()
Ответ на: комментарий от menangen

Кстати, кто знает, на go можно писать .so ?

Нет.

anonymous
()
Ответ на: комментарий от ozkriff

Есть парочка глобальных переменных, лучше бы их все убрать.

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

int __size_m = 5;
int __max_gen = 2;
Чтобы сразу было видна что это такое
int i, j;
for (i = 0; i < __size_m; i++) {
	for (j = 0; j < __size_m; j++) {
		map[i][j] = (rand() % 2) ? ALIVE : DEAD;
	}
}

itn ★★★
() автор топика
Ответ на: Субъективное от LeninGad

Нравится идея тренироваться на «жизни»

это ему домашку задали на первом курсе, лол

anonymous
()

Почитай про GObject и посмотри на

        int **map;
        for (i = 1; i <= max_gen; i++) {
                if (i == 1) {
                        map = init_map();
                        random_map(&map);
                        printf("\e[34m шаг %d\n", i);
                        print_step(map);
                } else {
                        next_step(&map);
                        printf("\e[34m шаг %d\n", i);
                        print_step(map);
                }
        }
        free_map(map);

Ты по сути свёл всё к работе с объектом. Тогда определи структуру Map и переименуй/подправь функции к виду

Map* map_create();
void map_free(Map *map);
void map_fill_random(Map *map);
Map* map_create_random();
void map_live_step(Map *map);
void map_print(Map *map);

AlexVR ★★★★★
()
Ответ на: комментарий от itn

насчёт неизменяемых переменных внутри функций я знаю, но каждый раз писать example(const int x, const int y, и т.д.)...

Если параметр передается по копированию, то это ладно. Но для указателей я бы таки писал const везде, где можно.

gcc -Wall приму к сведению.

Лучше gcc -Wall -Wextra -pedantic и -std=c89 или -std=c99. И еще каким-нибудь строгим статическим анализатором заодно)

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

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

Тем более я теперь все глобальные переменные буду именовать с двумя подчёркиваниями в начале.

Хм, я видел, что если они все-таки нужны, им просто префикс g_ дают. По крайней мере парочка первых попавшихся кодстайлов в гугле с этим согласна и вот: http://stackoverflow.com/questions/1722112/what-are-the-most-common-naming-co...

Global variables: g_lowerCase or g_lower_case (searchable by g_ prefix)

Кстати, из принятого ответа оттуда :) :

Global variables: just don't use global variables. They are evil.

ozkriff
()
Последнее исправление: ozkriff (всего исправлений: 1)
Ответ на: комментарий от qulinxao

Буду благодарен за критику.

ты издеваешься?

Это ещё вопрос, кто из вас двоих издевается =)

Deleted
()
Ответ на: комментарий от comp00

Программистам платят за сданные дедлайны.

+9000, именно так... к индусам не относится

I-Love-Microsoft ★★★★★
()
Ответ на: комментарий от beastie

И ещё: для уведомлений/ошибок использух errx()/warnx().

These functions are non-standard BSD extensions.

Только POSIX, только хардкор :D

// но таки да, почти всюду эти функции, лично у меня, работали :)

joy4eg ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.