George George - 1 month ago 7
C Question

why is this else block executed twice?

I have the following code which correctly calculates the jaccard similarity between an input

char array
and an existing array of char arrays.
jacc_sim_rec[]
is used to record the similarities which satisfy a minimum threshold value. The for loop is used to iterate through the multidimensional array and the loop is supposed to continue checking similarity if minimum threshold is not satisfied at
if (jacc_sim < SIM_THRESHOLD);
else record the result at

else
{
jacc_sim_rec[j] = jacc_sim;//keep record of similarity
++j;//record number of highly similar elements
}


my problem is, the whole statements in the
else
block is executed twice every time the threshold value is satisfied.

int j=0;

void calc_jac_sim( char*INCOMING, int grp)
{
unsigned long i, j11 = 0, j01 = 0, j10 = 0,m=0;
char *m11, *m01, *m10;
float jacc_sim = 0.0;
char r1[SBF_LEN] = { NULL };
char r2[SBF_LEN] = { NULL };
char r3[SBF_LEN] = { NULL };
int cnt = SBF_LEN - 1;

clear_jacc_sim_info();

for (int i = 0; i <= SBF_REC[grp]; ++i)
{
while (cnt >= 0)
{
r1[cnt] = SBF[grp][i][cnt] & INCOMING[cnt];
r2[cnt] = ~SBF[grp][i][cnt] & INCOMING[cnt];
r3[cnt] = SBF[grp][i][cnt] & ~INCOMING[cnt];
cnt--;
}
m11 = ( char*)r1;
m01 = ( char*)r2;
m10 = ( char*)r3;

for (m = SBF_LEN * sizeof( char); m--;
j11 += NumberOfSetBits(*m11++),
j01 += NumberOfSetBits(*m01++),
j10 += NumberOfSetBits(*m10++));

jacc_sim = j11 / (float)(j11 + j01 + j10);

if (jacc_sim < SIM_THRESHOLD);
//continue;//do nothing
else
{
jacc_sim_rec[j] = jacc_sim;//keep record of similarity
++j;//record number of highly similar elements
}
}

}

Answer Source

I don't understand the code, but I'll bet the problem is that you're not reinitializing cnt each time through the for loop, so you only fill in r1, r2, and r3 when i = 0.

Change that loop to:

    for (int cnt = SBF_LEN - 1; cnt >= 0; cnt--)
    {
        r1[cnt] = SBF[grp][i][cnt] & INCOMING[cnt];
        r2[cnt] = ~SBF[grp][i][cnt] & INCOMING[cnt];
        r3[cnt] = SBF[grp][i][cnt] & ~INCOMING[cnt];
    }

I'm also not sure why this needs to count down instead of up, like a typical loop, but it shouldn't make a difference.