Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

if node[i] and node[i+1] are present in the neighbor[i], then store the 'i' position of node and print the 'i' value of node.

this is also done by reversing the node array(only) and same type of check and print is done with the neighbor[i].

This code is written by me and works well, is there any other efficient method to perform this.

#include <stdio.h>
int main()
{
    int node[5] = {44,5,4,6,40};
    int neighbor[4]= {40,3,4,6};
    int i=0,j=0,k=0;
    int found_indices[5]; // array used to store indices of found entries..
    int count = 0; //n entries found;
    int fwd_count=0;
    int postn;
    int find;
    // to find in forward direction
     for (i=0; i<4; i++) {
        for (j=0; j<4; j++) {
            if (node[i]==neighbor[j]) {
                postn=node[i];
                for (k=0; k<4; k++) {
                    if (node[i+1]==neighbor[k]) {
                      found_indices[fwd_count ++] = postn; // storing the index of found entry


                    }
                }
            }
        }
    }

   if (fwd_count!=0) {  
       for (i=0; i<fwd_count; i++)
           printf("forward relay for ==%d\n", found_indices[i]);

   } else{
     printf("Relay not found in forward\n");
  }



 // to find in backward direction
    for (i=4; i>0; i--) {
        for (j=0; j<4; j++) {
            if (node[i]==neighbor[j]) {
                postn=node[i];
                for (k=0; k<4; k++) {
                    if (node[i-1]==neighbor[k]) {
                      found_indices[count ++] = postn; // storing the index of found entry


                    }
                }
            }
        }
    }

   if (count!=0) {  
       for (i=0; i<count; i++)
           printf("backward relayy for ==%d\n", found_indices[i]);

   }else{
      printf("Relay not found in backward \n");
   }

}
share|improve this question
    
Does it really work? Can I suggest that you separate the two cases (forward/reverse) into two functions, remove the assumed array sizes (pass them to the functions) and then define and run some (many) more test cases. Also explain more carefully what you expect as results. –  William Morris Feb 16 '13 at 23:38
add comment

1 Answer

up vote 1 down vote accepted

They key here is to to be able to abstract common functionality away. You're not expressing "node[i] is in the neighbor array" nicely. Simply write a function which does it for you:

int node_in_neighbors(int node, int neighbor[], int length) {
    int i;

    for(i = 0; i < length; i++) {
        if (neighbor[i] == node) { 
            return 1;              
        }
    }
    return 0;
}

In a more "high-level" language, you wouldn't have to write this function, and it would be in the library. You can now rewrite your forward/backward loops easily:

// to find in forward direction
for (i = 0; i < 4; i++) {
    // keep a node if his successor and him are in the neighbors array
    if (node_in_neighbors(node[i], neighbor, 4)
            && node_in_neighbors(node[i+1], neighbor, 4)) {
        found_indices[fwd_count ++] = node[i]; 
    }
}                                                                      

// to find in backward direction
for (i = 4; i > 0; i--) {
    // keep a node if his predecessor and him are in the neighbors array
    if (node_in_neighbors(node[i], neighbor, 4)
            && node_in_neighbors(node[i-1], neighbor, 4)) {
        found_indices[count ++] = node[i];
    }
}

This makes the code clearer, since it's way easier to understand the logic now.

One issue is that you still have lots of 4 around. You should replace those in the loop with node_length - 1 and those in the node_in_neighbors call by neighbor_length, otherwise you could confuse them.

Define those lengths like this:

int node_length = sizeof(node) / sizeof(node[0])
int neighbor_length = sizeof(neighbor) / sizeof(neighbor[0])

This trick only works on static arrays, be careful.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.