CODEWITHSUNDEEP
cmacrosc-preprocessor
David Tanner
David Tanner

Reputation:

Need help rewriting a C macro as a function

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.

Upvotes: 3

Views: 616

Answers (6)

Hernán
Hernán

Reputation: 4587

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.

Upvotes: 1

Reed Copsey
Reed Copsey

Reputation: 564403

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.

Upvotes: 5

ojrac
ojrac

Reputation: 13421

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.
        }
    }
}

Upvotes: 2

anon
anon

Reputation:

apparently it would involve declaring several arguments

What is wrong with that?

Upvotes: 4

Uri
Uri

Reputation: 89729

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;
}

Upvotes: 1

Assaf Lavie
Assaf Lavie

Reputation: 75973

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?

Upvotes: 2

Related Questions