Need help rewriting a C macro as a function

Question

Hi, I need some help rewriting the following line in a more type safe way and rewriting it as a function, however the fact that this code is defined inside a function is making it hard for me to think of a clever way of doing it because apparently it would involve declaring several arguments.

#define CHECK(id) if(table->cells[id]) isgood[table->cells[id]-1] = 0;

where table is a struct and isgood is an int.

Thanks a lot in advance for your help.

Answer 1

Why not just a function that receives table and id and does this?

void foo(TableType & t, int id)
{
    if (t.cells[id]) 
        isgood[t.cells[id]-1] = 0;
}

p.s.

It's a bad macro indeed. The name is very misleading.

p.p.s.

The whole thing is rather weird and the logic of this function escapes me. What exactly is this supposed to achieve?

Answer 2

Straight translation (if table->cells[id] is int):

void check(int id, int*isgood) { if (id) isgood[id-1] = 0; }

Call with:

check(table->cells[id], isgood);

However, I'd rename/rework this a bit. I'd especially change the name. There is also no error checking - ie, if table->cells[id]==0, you're going to try to set isgood[-1], which would be bad.

Answer 3

It is generally a good idea not to refer to variables in a macro.

First, make sure the name is meaningful. what are you checking for? And is there a side effect?

void update_valid_cells(Table& table, int id, BoolArray& validArray)
{
     if(table.cells[id]==NULL) return;
     validArray[id]-1=false;
}

Answer 4

apparently it would involve declaring several arguments

What is wrong with that?

Answer 5

If you're working in C++, I'd consider making check a member function of the table, which seems like a good candidate for a class:

class Table {
    //...
    public bool check(int id) {
        if (this->cells[id]) {
            this->isGood[id] = 0;
            // the line you have, isgood[table->cells[id]-1] = 0 looks buggy:
            // you treat table->cells[id] as a true/false value one line ago;
            // probably not a valid array index? I'm taking a stab at what to do.
        }
    }
}

Answer 6

I think C99 can qualify functions as inline so you get the speedup of no-function-call without using macros. Also, most C compilers support extensions such as __inline for this purpose.