Double printing values in nested for loop

Question

I have some C code that is double printing some values in my code, effectively I have stored some strings in a buffer, these strings are items such as:

GPU
ROUTER

My code has already stored and saved these items into a buffer. I am writing a simple exchange program that prints BUY and SELL orders for the items in the buffer. I have saved all the orders from the user into my exchange.all_orders buffer.

Here is a brief example:

When the user types

1. BUY 30 GPU 500 (0th element in  all_orders array)

The system will print out

BUY 30 GPU 500  (1 order)

When the user types same order again i.e:

BUY 30 GPU 500

The system should print out, note the order quantity is incremented.

BUY 60 GPU 500

Subsequently when a unique order is placed BUY 30 GPU 501, then the system will print out

BUY 60 GPU 500 (2 orders)
BUY 30 GPU 501 (1 order)

Right now my code does not do this, here is what I have so far

Struct Definitions

    struct exchange_order_book {
    int number_of_orders;
    int exchange_fees;
    struct exchange_order all_orders[100];
};


struct exchange_order {
    int quantity;
    int price;
    char action[5];
    char item[20];
    int traderId;
    int orderId;
};

    void print_buy_levels(){
    for(int i = 0; i<buffer_size; ++i){
        int buys = 0, sells = 0;
        double buy_prices[100] = {0}; // assuming at most 100 unique prices
        double sell_prices[100] = {0}; // assuming at most 100 unique prices
        int buy_count = 0, sell_count = 0; 

        // iterate through all orders in the exchange
        for(int j = 0; j<exchange.number_of_orders; ++j){
            if(strcmp(buffer[i],exchange.all_orders[j].item) == 0){
                if(strcmp(exchange.all_orders[j].action,"BUY") == 0){
                    // check if this is a unique buy price
                    int found = 0;
                    for(int k = 0; k < buy_count; ++k){
                        if(exchange.all_orders[j].price == buy_prices[k]){
                            found = 1;
                            break;
                        }
                    }
                    if(!found){
                        buy_prices[buy_count++] = exchange.all_orders[j].price;
                        ++buys;
                    }
                }else if(strcmp(exchange.all_orders[j].action,"SELL") == 0)
                {
                    // check if this is a unique sell price
                    int found = 0;
                    for(int k = 0; k < sell_count; ++k){
                        if(exchange.all_orders[j].price == sell_prices[k]){
                            found = 1;
                            break;
                        }
                    } 
                    if(!found){
                        sell_prices[sell_count++] = exchange.all_orders[j].price;
                        ++sells;
                    }
                }
            }
        }
        printf("%s\tProduct: %s; Buy levels: %d; Sell levels: %d\n",LOG_PREFIX,buffer[i],buys,sells);
    } 

}

Here are the cases where my code fails:

When the user enters BUY 30 GPU 500, it SUCCESSFULLY prints

BUY 30 GPU 500 (1 order)

When the user enters BUY 30 GPU 501, it SUCCESSFULLY prints

BUY 30 GPU 500 (1 order)
BUY 30 GPU 501 (1 order)

When the user enters BUY 30 GPU 501, AGAIN my code fails

BUY 30 GPU 500 (1 order)
BUY 60 GPU 501 (2 order)
BUY 30 GPU 501 (1 order)

Although it increments the quantity and count correctly, it prints the new added order into std.

Now I think I need to find a way to keep track of seen orders

I have been working on this for a long while and any help will be greatly appreciated.

As you can see this is the step my code is failing

This is how it is suppose to look like

Ignore the other input, only input that matters is COMMAND and the subsequent print statements for the orders

parse command, parses the command and uses sscanf to store it into the struct variables.

EDITED CODE AFTER SUGGESTION

    void print_orders() {
    for (int i = 0; i < buffer_size; ++i) {
        for (int j = 0; j < exchange.number_of_orders; ++j) {
            if ( !exchange.all_orders[j].duplicate && strcmp(buffer[i], exchange.all_orders[j].item) == 0) {
                struct exchange_order current_order = exchange.all_orders[j];
                int count = 1;
                int quantity = current_order.quantity;
                int is_duplicate = 0;


                for (int k = j + 1; k < exchange.number_of_orders; ++k) {
                    struct exchange_order next_order = exchange.all_orders[k];
                    if (current_order.price == next_order.price &&
                        strcmp(current_order.item, next_order.item) == 0) {
                        ++count;
                        quantity += next_order.quantity;
                        is_duplicate = 1;
                        next_order.duplicate = true;
                    }
                }

                if (!is_duplicate) {
                    printf("%s\t\t%s %d @ $%d (%d order)\n", LOG_PREFIX, current_order.action, quantity,
                           current_order.price, count);
                } else {
                    printf("%s\t\t%s %d @ $%d (%d orders)\n", LOG_PREFIX, current_order.action, quantity,
                           current_order.price, count);
                }
            }
        }
    }
}


struct exchange_order {
        int quantity;
        int price;
        char action[5];
        char item[20];
        int traderId;
        int orderId;
        bool duplicate;
    };

And exchange_order is unchanged

@H.S

Suggest an alternate method

"To check duplicate, one way is to compare the current processing order with the already processed (previous) orders in the list and if found duplicate then skip it. If the list is long then this is going to add a lot of unnecessary iteration and may impact the performance."

My list will be short, maybe this method might work

Have you tried running your code line-by-line in a debugger while monitoring the control flow and the values of all variables, in order to determine in which line your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) — Andreas Wenzel, May 10 '23 at 11:16
Yes I already know what the issue is, I just don't know how to fix it When the user enters a order that has already been updated with the amended quantity that same order is printed agian. I do not know how to solve this — HyperCoderSuperion, May 10 '23 at 11:19
If an order has been included in the duplicated check you need to somehow flag it to be skipped in the later check. — Anthony Kelly, May 10 '23 at 11:26
yes I have thought of that but I do not exactly know how to do that, do I need to declare a flag in my struct — HyperCoderSuperion, May 10 '23 at 11:29
I spent all day trying to figure out such a trivial problem, please help me — HyperCoderSuperion, May 10 '23 at 11:30
@chux-ReinstateMonica You can be certain that there is no lurking new line in my buffer, if there was it wouldnt of printed anything at all. I am certain of no new line characters — HyperCoderSuperion, May 10 '23 at 11:32
@HyperCoderSuperion Your code is comparing the current item with the items existing further in the list. Once a duplicate item is found, it combine them with current processing item and print but when processing the list further, in your code, you are nowhere checking whether the item is duplicate or not. While processing an item in the list, either check if current item is duplicate of any of the previous items processed or add a field in structure to mark whether the item is duplicate or not and mark the item duplicate when found and based on result then print them. — H.S., May 10 '23 at 12:01

H.S. · Answer 1 · 2023-05-10T12:57:12.533

While processing an order, in your code, you are checking whether any duplicate of current processing order exists further in the list. If it exists you are combining it in current processing order, which means, those duplicate order(s) has been processed. When processing list further, those duplicate order(s) processed again because, in your existing implementation, you are not checking whether current processing order is a duplicate or not i.e. whether it is combined with a previous processed order or not.

To check duplicate, one way is to compare the current processing order with the already processed (previous) orders in the list and if found duplicate then skip it. If the list is long then this is going to add a lot of unnecessary iteration and may impact the performance.
Other way could be to add a field in exchange_order structure, which will indicate whether an order is duplicate or not. If it is duplicate, don't process it as it has been already combined (processed) while processing a previous order.

Add a field in exchange_order structure to indicate whether an item is duplicate or not:

struct exchange_order {
    int quantity;
    int price;
    char action[5];
    char item[20];
    int traderId;
    int orderId;
    bool duplicate;   //new field, this require stdbool.h header file
};

While initialising a variable of type exchange_order, make sure to initialise duplicate member with false.

In the print_orders() function, add check for duplicate:

if ((!exchange.all_orders[j].duplicate) && 
    (strcmp(buffer[i], exchange.all_orders[j].item) == 0)) {

and here, mark an item duplicate:

                for (int k = j + 1; k < exchange.number_of_orders; ++k) {
                    struct exchange_order next_order = exchange.all_orders[k];
                    if (current_order.price == next_order.price &&
                        strcmp(current_order.item, next_order.item) == 0) {
                        next_order.duplicate = true;   // mark it duplicate 
                        ......
                        ......

Exactly same as the code you have shown in your question, just do the minor changes I have shown in my post. Yes, you would need `is_duplicate` flag as it is used to decide the print statement. — H.S., May 10 '23 at 12:56
Your original code had function `print_orders()` which you have replaced with `print_buy_levels()` function. Moreover, I do not see `duplicate` field in the structure `exchange_order`. Don't change the code in the question, keep the original code as it is and add the new code under `EDIT`. — H.S., May 10 '23 at 13:22
"To check duplicate, one way is to compare the current processing order with the already processed (previous) orders in the list and if found duplicate then skip it. If the list is long then this is going to add a lot of unnecessary iteration and may impact the performance." Maybe we could try this method, my list wont be long it will be pretty short <10 — HyperCoderSuperion, May 10 '23 at 13:35

Double printing values in nested for loop

1 Answers1