LINUX.ORG.RU

Говнокод ли?

 


1

2

Что-то мне посказывает, что я тут наговнокодил. Слишком много раз проверяю imageXX [!=]= SML_NOTASSIGNED. Как сократить?

int SmlWindowIconSet(int index, int image16, int image32)
{

    if ((warehouse.win.count <= index) ||
        (warehouse.win.alive[index]) == SML_DEAD)
    {
        return SML_ERR_BADWINDOW;
    }

    if ((image16  == SML_NOTASSIGNED) &&
        (image32  == SML_NOTASSIGNED))
        return SML_ERR_BADIMAGE;

    if ((image16 != SML_NOTASSIGNED) &&
        ((warehouse.img.count <= image16) ||
         (warehouse.img.alive[image16] == SML_DEAD) ||
        (warehouse.img.ptr[image16]->width  != 16) ||
        (warehouse.img.ptr[image16]->height != 16)))
    {
        return SML_ERR_BADIMAGE;
    }

    if ((image32 != SML_NOTASSIGNED) &&
        ((warehouse.img.count <= image32) ||
         (warehouse.img.alive[image32] == SML_DEAD) ||
        (warehouse.img.ptr[image32]->width  != 32) ||
        (warehouse.img.ptr[image32]->height != 32)))
    {
        return SML_ERR_BADIMAGE;
    }

    int memsize = 0;

    if (image16  != SML_NOTASSIGNED) memsize += 2 + 16  * 16;
    if (image32  != SML_NOTASSIGNED) memsize += 2 + 32  * 32;

    int * data = malloc(memsize * 4);

    if (data == NULL)
        return SML_ERR_BADALLOC;

    int count = 0;

    if (image16 != SML_NOTASSIGNED)
    {
        data[count++] = 16;
        data[count++] = 16;
        memcpy(data + count, warehouse.img.ptr[image16]->data, 16 * 16 * 4);
        count += 16 * 16;
    }

    if (image32 != SML_NOTASSIGNED)
    {
        data[count++] = 32;
        data[count++] = 32;
        memcpy(data + count, warehouse.img.ptr[image32]->data, 32 * 32 * 4);
        count += 32 * 32;
    }

    int err = XChangeProperty(warehouse.display,
                              warehouse.win.ptr[index].window,
                              warehouse.atoms.wmicon,
                              warehouse.atoms.cardinal,
                              32,  PropModeReplace,
                              (const unsigned char *) data, count);

    SML_CHECKERROR(err);

    return SML_ERR_SUCCESS;
}


Последнее исправление: inn (всего исправлений: 1)

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

if (image?? != SML_NOTASSIGNED)
, которые можно объединить, просто запихав в контейнер и датакаунты и вывод ерроров

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

В смысле? Немного не распарсил.

inn
() автор топика
bool image16_assigned = image16 != SML_NOTASSIGNED;
bool image32_assigned = image32 != SML_NOTASSIGNED;

чуток улучшит читаемость.

Legioner ★★★★★
()

Выходит двойное дублирование кода, для 16 и 32. Его нужно заменить ф-ий func(int index, int image, size_t size); И лучше

warehouse.img.
заменить на более короткий алиас img.

И так делать не нужно (во многих местах встречается)

 int * data = malloc(memsize * 4);
4 <- sizeof(int)

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

Ради прописанных в хедере true, false,1 и 0 либу подключать? Если совсем с 0 и !=0 проблема, тогда да, жесткие нынче дешевы, линукс и так жирный

minakov ★★★★★
()

И с data не всё понятно. С одной стороны это

int * data = malloc(memsize * 4);
С другой char*
(const unsigned char *) data
Нужно юзать тип конкретной длины uint32_t или какой-то другой тип, который рекомендуют в данном случае. Явно это не int

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

Выходит двойное дублирование кода, для 16 и 32. Его нужно заменить ф-ий func(int index, int image, size_t size);

Не выйдет. XChangeProperty поддерживает append, но тогда нужно отслеживать первую установку и держать ради этого флаг. Первая установка - PropModeReplace, вторая - PropModeAppend. И не факт, что получится.

warehouse.img. заменить на более короткий алиас img.

А как тогда разбираться, где локальная переменная, где хранилище? Там на складе с десяток различных хреновин лежит, от событий до таймеров.

inn
() автор топика
Ответ на: комментарий от mashina

Вообще эта штука - это цвета. Причем первые два значения перед картой должны быть размерами: 16, 16, 0xFFFF0000, 0xFF00FFFF... например. char * в функции ибо она универсальная для каждого свойства и придумана не мной, все претензии в xlib. Насчет int -> unsigned int согласен, исправлю.

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

Есть какие то раритетные юниксы, на которых int занимает 8 байтов, например. Поэтому чтобы на лоре не бурчали, надо не unsigned int использовать, а именно uint32_t.

Legioner ★★★★★
()
static bool IsBadImage(int image, int size)
{
	if ((image != SML_NOTASSIGNED) &&
			((warehouse.img.count <= image) ||
			 (warehouse.img.alive[image] == SML_DEAD) ||
			 (warehouse.img.ptr[image]->width  != size) ||
			 (warehouse.img.ptr[image]->height != size)))
	{
		return true;
	}

	return false;
}


static int *SetData(int image, int size, int *data)
{

	if (image != SML_NOTASSIGNED)
	{
		*data++ = size;
		*data++ = size;
		memcpy(data, warehouse.img.ptr[image]->data, size * size * sizeof(*data));
		data += size * size;
	}

	return data;
}

int SmlWindowIconSet(int index, int image16, int image32)
{

	if ((warehouse.win.count <= index) ||
			(warehouse.win.alive[index]) == SML_DEAD)
	{
		return SML_ERR_BADWINDOW;
	}

	if ((image16  == SML_NOTASSIGNED) &&
			(image32  == SML_NOTASSIGNED))
		return SML_ERR_BADIMAGE;

	if (IsBadImage(image16, 16) || IsBadImage(image32, 32))
		return SML_ERR_BADIMAGE;

	int memsize = 0;

	if (image16  != SML_NOTASSIGNED) memsize += 2 + 16  * 16;
	if (image32  != SML_NOTASSIGNED) memsize += 2 + 32  * 32;

	int * data = malloc(memsize * sizeof(*data));

	if (data == NULL)
		return SML_ERR_BADALLOC;

	int *p = SetData(image16, 16, data);
	SetData(image32, 32, p);

	int err = XChangeProperty(warehouse.display,
			warehouse.win.ptr[index].window,
			warehouse.atoms.wmicon,
			warehouse.atoms.cardinal,
			32,  PropModeReplace,
			(const unsigned char *) data, count);

	SML_CHECKERROR(err);

	return SML_ERR_SUCCESS;
}

И все еще плохо.

AptGet ★★★
()

Ы?

int get_icon_mem_size(int image, int dimension)
{
    if((image == SML_NOTASSIGNED) &&
            !((warehouse.img.count <= image) ||
              (warehouse.img.alive[image] == SML_DEAD) ||
              (warehouse.img.ptr[image]->width  != dimension) ||
              (warehouse.img.ptr[image]->height != dimension)))
        return 2 + dimension  * dimension;

    return 0;
}

void update_icon_data(int image, int dimension, int *data, int *count)
{
    if (image != SML_NOTASSIGNED)
    {
        data[*count++] = dimension;
        data[*count++] = dimension;
        memcpy(data + count, warehouse.img.ptr[image]->data, dimension * dimension * sizeof(*data));
        *count += dimension * dimension;
    }
}

int SmlWindowIconSet(int index, int image16, int image32)
{

    if ((warehouse.win.count <= index) ||
        (warehouse.win.alive[index]) == SML_DEAD)
    {
        return SML_ERR_BADWINDOW;
    }

    int memsize = 0;
    memsize += get_icon_mem_size(image16, 16);
    memsize += get_icon_mem_size(image32, 32);

    if(!memsize)
        return SML_ERR_BADIMAGE;

    int * data = malloc(memsize * sizeof(int));

    if (data == NULL)
        return SML_ERR_BADALLOC;

    int count = 0;

    update_icon_data(image16, 16, data, &count);
    update_icon_data(image32, 32, data, &count);

    int err = XChangeProperty(warehouse.display,
                              warehouse.win.ptr[index].window,
                              warehouse.atoms.wmicon,
                              warehouse.atoms.cardinal,
                              32,  PropModeReplace,
                              (const unsigned char *) data, count);

    SML_CHECKERROR(err);

    return SML_ERR_SUCCESS;
}
anonymous
()
Ответ на: комментарий от anonymous
Поправка
в get_icon_mem_size 
замени
if((image == SML_NOTASSIGNED)
на
if((image != SML_NOTASSIGNED)
anonymous
()
Ответ на: комментарий от anonymous

а если image16 не верный по любому из этих условий:

       ((warehouse.img.count <= image16) ||
         (warehouse.img.alive[image16] == SML_DEAD) ||
        (warehouse.img.ptr[image16]->width  != 16) ||
        (warehouse.img.ptr[image16]->height != 16)))
, а image32 верный.

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

а если image16 не верный
а image32 верный

...
    int memsize = 0;
    int tmp = 0;
    
    if(!(tmp = get_icon_mem_size(image16, 16)))
        return SML_ERR_BADIMAGE;
    memsize += tmp;
    
    if(!(tmp = get_icon_mem_size(image16, 16)))
        return SML_ERR_BADIMAGE;
    memsize += tmp;

    int * data = malloc(memsize * sizeof(int));
...

Сам догадаешься, где исправить?

anonymous
()

Пиши на c++ будет куда лучше

anonymous
()

Вы реально упоролись чтоли? Все и тотально - идите мести улицы.

Каким же надо быть упоротым чтобы сделать такое говно - он взял смержил 2 функции, которые отличаются константой? Риально?

Для упортых объясняю -

  if((warehouse.win.count <= index) ||
     (warehouse.win.alive[index] == SML_DEAD)) {
    return SML_ERR_BADWINDOW;
  }
Это говно общие для 2-х функций - выносится в другую функцию, которая вызвается до вызова твоей «общей функции». Говно ниже можно тоже вынести, если оно юзается часто, а не форфан.

int err = XChangeProperty(warehouse.display,
                              warehouse.win.ptr[index].window,
                              warehouse.atoms.wmicon,
                              warehouse.atoms.cardinal,
                              32,  PropModeReplace,
                              (const unsigned char *) data, count);

Как это говно будет узнавать, что у тебя: image16+/image32 или наоборот? Тыб описал чтоли что бывает в твоей функции.

Общая функция - эта вся твоя 32/16 копипаста собранная вместе, а 32/16 - это аргумент.

    int * data = malloc(memsize * 4);

    if (data == NULL)
        return SML_ERR_BADALLOC;

Вам не лень эту гниль постоянно писать? Напиши макрос.

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

пост из разряда «БЕРЕМ ЕБАНОЕ МОЛОКО И ЗАЛИВАЕМ АДСКИЕ ХЛОПЬЯ ЭТИМ КИПЯЩИМ ДЕРЬМОМ!111»

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

Ты меня унизил. Выбирай оружие :3

Не хотел, честно.

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

реально упоролись

Все и тотально - идите мести улицы

говно

говно

говно

форфан

В тред врывается суперхаккиллер, всем лежать, лалки!

false ★★★★★
()
Последнее исправление: false (всего исправлений: 1)

если работает - поставь галочку FIXME и не трогай пока не сделаешь остальное.

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

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

если работает - поставь галочку FIXME и не трогай пока не сделаешь остальное.

кстати, да

сначала заставь работать. причёсывание, оптимизация и красота - всё после.

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

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

anonymous
()

Куча точек возврата из функции - зачем? Постоянные проверки image* на SML_NOTASSIGNED - почему не консолидировать их? Постоянное warehouse.img - тебе что, за многабукав платят?

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

Куча точек возврата из функции - зачем?

А что, в сях придумали исключения?

Постоянные проверки image* на SML_NOTASSIGNED - почему не консолидировать их?

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

inn
() автор топика
Ответ на: комментарий от tailgunner

Постоянное warehouse.img - тебе что, за многабукав платят?

Компилятор все равно свернет это в пару байт адреса, зато так понятней, откуда эта переменная пришла вообще.

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

Куча точек возврата из функции - зачем?

А что, в сях придумали исключения?

В Си придумали if и goto.

Постоянные проверки image* на SML_NOTASSIGNED - почему не консолидировать их?

В первый раз проверяется [...], во второй - [...], в третий [...]

Интересно, если будет четвертый и далее разы - ты всё равно будешь тупо копипастить? Ни до макроса, ни до локальной переменной-флага не додумаешься? О том, чтобы изменить структуру кода, я даже не спрашиваю.

Постоянное warehouse.img - тебе что, за многабукав платят?

Компилятор все равно свернет это в пару байт адреса

А, так ты пишешь программы для компилятора...

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

В Си придумали if и goto.

Тогда это будет макаронный код.

ни до локальной переменной-флага не додумаешься?

Хосспаде, а разница? Что я заведу локальную переменную, что я буду проверять такую же переменную. Смысл? Ввести новые сущности?

А, так ты пишешь программы для компилятора...

А читать дальше одного предложения не обучены?

Компилятор все равно свернет это в пару байт адреса, зато так понятней, откуда эта переменная пришла вообще.

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

В Си придумали if и goto.

Тогда это будет макаронный код.

Простыня, из которой торачат return - это и есть макаронный код.

Ни до макроса, ни до локальной переменной-флага не додумаешься?

Хосспаде, а разница? Что я заведу локальную переменную, что я буду проверять такую же переменную.

То есть ты не видишь разницы между тем, что напишешь одно выражение трех местах и тем, что напишешь его в одном месте, запомнишь результат, и используешь в трех местах? Смени профессию.

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

То есть ты не видишь разницы между тем, что напишешь одно выражение трех местах и тем, что напишешь его в одном месте, запомнишь результат, и используешь в трех местах? Смени профессию.

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

anonymous
()
Ответ на: комментарий от tailgunner
int func(int a, int b)
{
   int c = (a != DONT);
   int d = (b != DONT);


   if (c && d) {...}

   if (c) {...}
   if (d) {...}

   if (c) {...}
   if (d) {...}
}
int func(int a, int b)
{
  if (a != DONT) && (b != DONT) {...}

  if (a != DONT) {...}
  if (b != DONT) {...}

  if (a != DONT) {...}
  if (b != DONT) {...}
}

И какой сакральный смысл в создании лишней переменной? Оккам нервно плачет в углу.

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

Оккам нервно плачет в углу.

Не поминал бы ты Оккама всуе:

    if ((image16 != SML_NOTASSIGNED) &&
        ((warehouse.img.count <= image16) ||
         (warehouse.img.alive[image16] == SML_DEAD) ||
        (warehouse.img.ptr[image16]->width  != 16) ||
        (warehouse.img.ptr[image16]->height != 16)))

    if ((image32 != SML_NOTASSIGNED) &&
        ((warehouse.img.count <= image32) ||
         (warehouse.img.alive[image32] == SML_DEAD) ||
        (warehouse.img.ptr[image32]->width  != 32) ||
        (warehouse.img.ptr[image32]->height != 32)))

->

static int bad_image(int img_idx, int img_bits)
{
     return img_idx != SML_NOTASSIGNED &&
            (warehouse.img.count <= img_idx ||
             warehouse.img.alive[img_idx] == SML_DEAD ||
             warehouse.img.ptr[img_idx]->width  != img_bits ||
             warehouse.img.ptr[img_idx]->height != img_bits)
}

Но ты можешь продолжать писать быдлокод методом копипаста и вопрошать о сакральном смысле.

tailgunner ★★★★★
()

warehouse.img.ptr[image16]->width

Чух-чух!

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

SmlWindowIconSet - моя

Отлично. Что тогда менает переписать её с сигнатурой

int SmlWindowIconSet(int index, int image, int size);
Я это ниже уже предлагал и ты что-то ответил нерелевантное к показанному коду.

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

То, что есть проблемы присваивания свойства XSetAttribute. Оно может либо затереть его (в данном случае - иконку), либо приаппендить. Второй случай нигде не рассматривается,скорее всего не поддерживается wm'ами для данного войства.

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

Это я ниже читал, какое отношение имеет к коду выше (где нет XSetAttribute) и возможности переписывания ф-ии с нормализованными аргументами?

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

Ок, либо я, либо вы чего то не понимаем.

Что написал я:

SmlWindowIconSet(mywin1, icon16x16, SML_NOT_ASSIGNED);
// либо
SmlWindowIconSet(mywin1, SML_NOT_ASSIGNED, icon32x32);
// либо
SmlWindowIconSet(mywin1, icon16x16, icon32x32);
При каждом вызове иконка окна будет полностью перезаписана сервером (PropReplace).

Что предлагаете вы (как я понял)

SmlWindowIconSet(mywin1, icon16x16, 16);
SmlWindowIconSet(mywin1, icon32x32, 32);

При этом нужно не перезатереть а приаппендить иконку. Насколько я понял, так никто с помощью XSetAttribute не делает, по крайней мере, работающего кода я не обнаружил.

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

Ясно. Вроде пишут, что PropModeAppend можно делать несуществующему проперти, т.е. запоминать ничего не нужно, можно всегда ресетить + много раз аппендить.

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

Как вы предлагаете разбираться, когда аппендить, а когда ресетить. И вообще, как ресеттить, если есть только Replace и Append.

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