Find Max of columns in 2D array

[cafegeo]

Hello everyone. I am trying to find the max values of each column in this 5 column array. I have skipped the first column because it is just the product name. The other 4 columns contain revenue of that product. The function I wrote below works for col 1, 2 and 4, but column 3 always shows the same results as column 2. I cannot figure out why. I would appreciate any help you can offer. Thank you in advance.

Here is the function and data file:

void quartprofit(string dis[], double array[][4]) 
 {
      int qu;
      int row=10;
      int col=1;
      string dismx;
      double mx=0;
             
        for (int a=0;a<4;a++){
         for(int i=0;i<row;i++){
          for(int qu=0;qu<col;qu++)
      
           if(mx<array[i][qu])    {   
			mx = array[i][qu];
			dismx=dis[i];    
                                  }
                               }
      cout << "\n";
      cout << col <<" Quarter: "<< dismx << "  " << mx << "\n"; 
      col++;	
                             }
 }

Data File:

Memory 5 15 7 10
CPU 10 21 0 11
Monitor 2 17 4 42
HD 5 -8 -10 -4
Keyboards 7 10 15 -9
Mice 3 -10 3 10
Drives -1 -11 1 -8
Software 3 5 10 8
Optical 5 7 -8 10
Printers 11 -19 7 3

[VernonDozier]

Line 11 doesn't make much sense to me. col is 1 and never changes. That means the "loop" will only happen once, so not much of a loop. qu is always 0. Therefore I am quite surprised by this statement...

The function I wrote below works for col 1, 2 and 4

I would expect it to not work for 2 and 4 (I assume these correspond to indexes 1 and 3 in array[][4]).

Also, since you have negative numbers, you may want to initialize line 7 to the smallest possible double value rather than 0, or just something smaller than anything you'll have in your file.

http://stackoverflow.com/questions/1153548/minimum-double-value-in-c-c

[cafegeo]

Col increments by 1 in line 20. This allows the loop in line 9 to analyze the next column.

Point taken on the line 7 initialization. For now I will set to -32000.

Why would you expect col 2 and 4 not to work? My output gives correct info for Col 1,2 and 4.

Could you advise on any changes I would need to make to my loops or the code in general. I believe I can accomplish this with two For loops, but just cannot grasp how.
Thank you

[VernonDozier]

>> Col increments by 1 in line 20

Indeed it does. I missed that. Are you missing a bracket somewhere? It looked like lines 18 and on weren't part of a loop, so I didn't look that far. I guess the bracket on line 22 ends the outer-most for-loop rather than the function.

I think I'm not understanding the logic of the inner-most loop. Here's the goal from your first post.

I am trying to find the max values of each column

Key word is "each". It looks like you are comparing values from DIFFERENT columns maybe. The "a" variable appears to never be used in any indexing. Why three loops and not two?

Seems to me it should be more like this...

for(col = 0; col < 4; col++)
{
    // initialize max to so some really low number
    for(int row = 0; row < 10; row++)
    {
        // check to see if there is a new max for column col
    }
    // display the max for column col.
}

[VernonDozier]

>> My output gives correct info for Col 1,2 and 4.

I am wondering whether this is just luck based on this particular file. Try changing the numbers around. I see no numbers in column 3 greater than 21, the max from column 2. Try changing one of the numbers in column 3 to 22. See if everything displays right then. Then change that number to 100 and see if column 4 now incorrectly displays 100 (since 100 is greater than anything in column 4).

[cafegeo]

Vernon, thank you very much for the advice. I did try messing around with the numbers in my data file, but no changes.

I knew I could do this with two loops rather than one. I was also able to display the product name next to the profit by placing it within the If statement.

Thanks again for your patience. I am kind of new to c++, haven't touched it in 15 years..

[VernonDozier]

You are welcome. It looks like you weren't missing any brackets after all, but I re-indented as well as added a few brackets to make the code a bit clearer. Let's analyze.

void quartprofit(string dis[], double array[][4]) 
{
    int qu;
    int row=10;
    int col=1;
    string dismx;
    double mx=0;
             
    for (int a=0;a<4;a++)
    {
        for(int i=0;i<row;i++)
        {
            for(int qu=0;qu<col;qu++)
            {
                if(mx<array[i][qu])    
                {   
                    mx = array[i][qu];
                    dismx=dis[i];    
                }
            }
        }
        cout << "\n";
        cout << col <<" Quarter: "<< dismx << "  " << mx << "\n"; 
        col++;	
    }
}

First let's change a few names so it's a little easier to read. I'm going to change the 4 to NUM_COLS and the 10 to NUM_ROWS.

void quartprofit(string dis[], double array[][4]) 
{
    int qu;
    const int NUM_ROWS = 10;
    const int NUM_COLS = 4;
    int col=1;
    string dismx;
    double mx=0;
             
    for (int a=0;a<NUM_ROWS;a++)
    {
        for(int i=0;i<NUM_COLS;i++)
        {
            for(int qu=0;qu<col;qu++)
            {
                if(mx<array[i][qu])    
                {   
                    mx = array[i][qu];
                    dismx=dis[i];    
                }
            }
        }
        cout << "\n";
        cout << col <<" Quarter: "<< dismx << "  " << mx << "\n"; 
        col++;	
    }
}

Now let's rename a to row. We had a "row" before, but changed it to NUM_ROWS, so we can use "row" now.

void quartprofit(string dis[], double array[][4]) 
{
    int qu;
    const int NUM_ROWS = 10;
    const int NUM_COLS = 4;
    int col=1;
    string dismx;
    double mx=0;
             
    for (int row=0;row<NUM_ROWS;a++)
    {
        for(int i=0;i<NUM_COLS;i++)
        {
            for(int qu=0;qu<col;qu++)
            {
                if(mx<array[i][qu])    
                {   
                    mx = array[i][qu];
                    dismx=dis[i];    
                }
            }
        }
        cout << "\n";
        cout << col <<" Quarter: "<< dismx << "  " << mx << "\n"; 
        col++;	
    }
}

Now let's change any row indexes we have to make sure they use the "row" variable. So we'll try to find any variables with the variable "array" in it. "row" represents the row number, so we should be using it as an index. I see "array" on lines 16 and 18. Let's see what we're using "a" as the row variable and change it to "row".

Not seeing it. I'm seeing "i" as the row index variable. In the code, "i" loops through all the COLUMNS. So why am I seeing it as a row index? I shouldn't be. "i" should represent a column index, not a row index. There lies (one of) the problems. Why did I want to change the variable names? So that problems like this immediately stand out. I can easily mistake "a" and "i" and stick them in the wrong spot. Mistakenly switch "row", "col", "NUM_ROWS", or "NUM_COLS" and the error pops out immediately just by reading the code. Similarly, the indentation is important. I immediately know what is inside of what.

Why is array[][] an array of doubles? I assume "qu" means "quantity"? or is it "quarter"? Hell, "quarter" is even more descriptive than "row". "mx" is "maximum"? This is some sort of inventory system or sales chart? Can I ever have or sell 1.5 of something? If not, perhaps change the array[][] to an integer array.

Try re-writing this function using variables as descriptively as possible. I think that it will really clarify the thinking process. What does the array represent? Does it represent the amount of inventory in stock? Perhaps rename it inventoryInStock[][]. Stick the keyword "const" in front of anything that should never change and capitalize them. When you name extremely carefully, it forces you to clarify in your mind what everything does. The code almost writes itself.

[VernonDozier]

Just looked at the function name. Okay, so it's "quarter" and "profit". Profit presumably would be in dollars and cents, so making the array an array of doubles is good. My missing that makes my point actually. I now know much more about the function, so it's easier to read it and write it.

[cafegeo]

All very good points! I am in the process of rewriting this to clean it up and make it much more legible. It is a sales data report for a company based on quarterly revenue.

I will keep this in mind if I have to post again.

This is a very rough first attempt for me. Thank you again for your patience.