Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

Weird C++ Array Issue

  1. May 21, 2013 #1
    Hello all,

    I'm not really a programmer at all, but I learned c++ for a while so I can do basic stuff. Having just finished my linear algebra class I suddenly had an urge to write myself a simple matrix solver. I knew it wouldn't be easy (especially because of my lack of skill) but I was curious.

    It was going great until I noticed that large matrices would crash the program. It's very weird. If the number of entries in my matrix array is less than 10 then everything works fine, but the second it hits 10 it stops working. I get the following message:

    I was hoping someone could help me figure out why in gods name this is happening. I apologize in advance for the messiness of my code, I haven't yet made it nice (I was going to once I got the matrix entry down), but I put in a couple of notes to try and help.

    Thank you very much :)

    Edit: Something I should add: it doesn't crash when I made the size > 10 entries, it crashes when I enter the value for the 10th entry.

    Code (Text):
    // Matrix.cpp : main project file.

    #include "stdafx.h"
    #include <iostream>

    int main()
    {
        using namespace std;

        // definition of array values
        struct Entry
        {
            long double Value;
            int mVal;
            int nVal;
            bool pivot;
        };

        // beginning of program
        int mSize;
        int nSize;
        int tSize;
        Entry *toSize = new Entry[tSize]; // creating array
            cout << "Height of Matrix (M-Val):" << endl;
            cin >> mSize;
            cout << "Width of Matrix (N-Val):" << endl;
            cin >> nSize;
            tSize = (nSize * mSize);
        int B;
        int Num = 1;
        int modN = 1;
        int modM = 1;

            // entering values
                while (B < tSize)
                {
                    long double value;
                    cout << "What do you want entry " << "(" << modM << "," << modN << ")" << " to be?" << endl;
                    cin >> value;
                    toSize[B].Value = value;
                        if (modN < nSize)
                        {
                            toSize[B].nVal = modN;
                            toSize[B].mVal = modM;
                        }

                        else
                        {
                            toSize[B].nVal = modN;
                            modN = 0;
                            toSize[B].mVal = modM;
                            modM ++;
                        }  
                    B ++;
                    Num ++;
                    modN ++;
                }
        int T;
        cout << "\n \n" << endl;
        // printing values and placements
            while (T < tSize)
            {
                cout << "Value is:" << toSize[T].Value << endl;
                cout << "M-Placement is:  " << toSize[T].mVal << endl;
                cout << "N-Placement is:  " << toSize[T].nVal << endl;
                cout << "\n" << endl;
                T++;
            }

    // so it doesn't close immediately
        cin.clear();
        cin.ignore(255, '\n');
        cin.get();
     
    }
     
  2. jcsd
  3. May 21, 2013 #2

    AlephZero

    User Avatar
    Science Advisor
    Homework Helper

    You allocated the array of Entriy structures before you know the value of tsize.

    The fact that you can enter 10 elements before the program crashes is just luck - the first few entries didn't overwrite anything important enough to cause a crash.
     
  4. May 21, 2013 #3

    mfb

    User Avatar
    2016 Award

    Staff: Mentor

    To fix this issue, it should be sufficient to move that one line down.

    Just wondering... why don't you use a two-dimensional array to store two-dimensional data?
     
  5. May 21, 2013 #4

    Mark44

    Staff: Mentor

    Aleph_zero nailed the problem you're having, so I would like to address your indentation style. Of course, the compiler cares not a whit about how things are indented, but human readers do.

    Your indentation is for the most part good, except that there are some blocks of code that are indented too much when they shouldn't be. The downside of this is that too much indenting can make some lines either wrap to the next line or stretch beyond the right margin so that a reader has to scroll to the right to see the rest of the line. A good rule of thumb is that two statements that execute the same number of times should have the same level of indentation. This means, for example, that the line with "if" or "while" or "for" should not be indented, but that the body of each of these statements should be indented.

    With this in mind, here is the revised version of your code.
     
  6. May 21, 2013 #5
    To Mark, makes sense. It was indented differently before I posted I added indents for increased clarity, but I definitely prefer what you were doing to mine!

    By the way I had forgotten to post here earlier, Aleph you nailed it so thank you! It was really interesting it consistently worked up until 10, I wonder what it was that it finally overwrote, but after I moved it it worked perfectly.

    As for the multidimensional array.....because I'm stupid.
     
  7. May 21, 2013 #6
    multidimensional arrays aren't that nice anyways IMO. It's always best to either use a one dimensional array or some kind of structure, such as a vector.
     
  8. May 21, 2013 #7
    I'm just figuring that out.

    I rewrote the whole thing in 2-D Arrays but I just got frustrated enough to close it for the night.
    I'm planning on re-writing it again tomorrow or the next day using 1-D arrays but in a nicer style/one that would be able to be expanded on easier.

    Thanks for the help!
     
  9. May 22, 2013 #8

    D H

    User Avatar
    Staff Emeritus
    Science Advisor

    I disagree vehemently. Just because the language is weak (arguably very weak) in its support for multidimensional arrays doesn't mean you shouldn't use them. If the underlying math uses a multidimensional array, the best thing to do is to use a multidimensional array in the code. Because the language is weak in this regard, that means either developing the requisite tools in house or finding an existing package that does so. IMO, std::vector is not a good choice. If the math says to use a fixed sized NxM matrix, the last thing one should use is a mechanism that lets the size change. Changing the size of a fixed size NxM matrix is domain specific undefined behavior. The language already offers way to many avenues to achieve undefined behavior. There's no reason to add domain specific UB to the huge mountain of UB that comes with the language.

    If you have a C++11 compiler (and if you can use it; a lot of projects mandate C++03), std::array is oftentimes a better choice. An array of arrays provides true support for NxM matrices, but at the cost of significant performance degradation unless compiled at a very high optimization level (inlining enabled). That leads to another issue: A lot of projects forbid compiling at this high a level of optimization.

    The problem with this is that inlining exposes a large number of undefined behaviors that are benign in a non-inlined setting. Undefined behavior is another pitfall of C and C++. The key problem with undefined behavior is that programmers don't know that their code invokes it. The UB-invoking code can compile clean, even with the highest warning levels enabled.
     
  10. May 22, 2013 #9
    Well look, I'm not a computer science major, but from practical experience it's easier IMO to just define a one dimensional array and index it like,

    Code (Text):
    array[i + j*WIDTH]
    as opposed to

    Code (Text):
    array[i][j]
    which are exactly equivalent in the way the memory is actually accessed and indexed. The nice thing about 1-d array is that you can simply pass it to functions by its name. To pass a 2-d array or higher you must declare the size of the array in the argument of the function which is receiving it, which can be a pain.

    Can you give me an explicit example of where a 2-d array offers any tangible benefits over a 1-d array?

    Also I don't see your point about vectors not having a fixed-size... how is the size of your vector going to change unless you explicitly resize it or attempt to go out of bounds? You are allowed to index an array out of bounds too... so if your code is doing something wrong with vector it's going to do the equally wrong thing with an array too.
     
  11. May 22, 2013 #10
    Hey, sorry to bother you guys again.

    Work's coming along steadily on this next incarnation (and a hell of a lot cleaner). But could one of you explain how I should go about passing arrays to functions (or how I achieve this affect; I've come to understand functions cannot take arrays as arguments)? It isn't a requirement but it would make my life a lot easier (and clean up main() a whole lot). I tried searching around but the web didn't help much.

    Thanks :)
     
  12. May 22, 2013 #11
    I've just figured out that solution (or solved it at least), so I'm good for now :)
     
  13. May 22, 2013 #12
    Sorry for the triple post in advance.

    I had finished both the matrix entry, storage and printing, and had built my transpose function. I soon realized that the way my matrix print function worked (printed numbers in sequential order from their array) wasn't going to let my transpose function work properly (and also was quite limited).

    As such, I began to rewrite the Print Matrix function so that it printed numbers based off of the (m/n) values of the array entries (because each entry is a struct) not off of their order. This works (or at least it seems to print correctly, I won't know until I have this next problem fixed), but I'm having a very annoying problem.

    I have an if statement in the print function, basically saying if the entry's n-value is the n-width of the matrix, print a newline afterwords (so it prints like a matrix), I've done this a couple times before and it works perfectly.

    The problem is that for some reason I've been rattling my brains over, no matter what the n-value is the function always prints a newline after. It's very weird. Here is the function (I'm really sure the problem is here) and I've bolded the if statement in question.

    Thank you again :)

    Code (Text):
    void PrintMatrix(Entry *CurrentMatrix, int tSize ([B]number of entries in array[/B]), int nSize ([B]width of matrix[/B]))
    {
        int A;
        int mCount = 1;
        int nCount = 1;

        while (A < tSize)
    {
        int Count;
    while (Count < tSize)
        {
        [B]if (CurrentMatrix[Count].mVal = mCount, CurrentMatrix[Count].nVal = nCount)
            {
                if (CurrentMatrix[Count].nVal = nSize)
                    cout << CurrentMatrix[Count].Value << "\n" << endl;
                else
                    cout << CurrentMatrix[Count].Value << "  ";
            Count ++;
            }[/B]
        else
                    Count ++;
        }
            if (nCount = nSize)
            {
                nCount = 1;
                mCount ++;
            }
            else
                nCount ++;
            A ++;
           

        }
    }
    I've tried to make the indents a bit more friendly for the forum, sorry if it's not.
     
    Last edited by a moderator: May 23, 2013
  14. May 23, 2013 #13

    mfb

    User Avatar
    2016 Award

    Staff: Mentor

    Here you have undefined behavior again. "A" could be anything, as you do not set it to some value before you call it.
    Same issue.

    That is wrong for several reasons:
    • a single "=" assigns the value of the right hand side to the left hand side. Instead of comparing two things, you modify the Count entry of CurrentMatrix. Use "==" for comparisons
    • Those assignments always return "true" (means: they were successful), unless there is some error in the code. It is pointless to use them in an if-statement like that.
    • If you separate multiple statements with commas, they are all executed, but only the last one is evaluated to see if the "if" condition is true. Do not use commas in if-statements. If you want two conditions at the same time, use the logical "and": "if(condition1 && condition2)"
    • I am not sure why you want to compare those parameters at all.
     
  15. May 23, 2013 #14

    Mark44

    Staff: Mentor

    Not always. An assignment returns false if zero is the value being assigned.
     
  16. May 23, 2013 #15
    Thank you so much. The reason I didn't set those to values (and I had done that several other times throughout my code) is that I had mistakenly thought that doing so implicitly set the value at zero, likewise I had forgotten about the =/== distinction, so thank you.

    The reason I am comparing those parameters is that my idea was that the printer would look though every entry in the matrix for the entry with the designated m/n values, print it if both m and n were equal to the ones you were looking for, then increase m/n by one (the code at the bottom makes it follow matrices' numbering correctly) and look for that one. This way the position of the entries in the array shouldn't correspond to the position of the entries in a printed matrix (or that's the idea), which should make things like transposes easier.

    I've gotten the printmatrix function to work now, thank you all so much, right now I'm just troubleshooting the transpose.

    Edit: Strike that, Transpose works like a charm!
     
  17. May 23, 2013 #16

    Mark44

    Staff: Mentor

    Some tips on indentation again.
    I don't use tab spacing for indenting, as it tends to put deeply embedded control structures so far to the right that you have to scroll across the page to see the end of the line. As part of my job I format code that appears in documentation on the web, and we strive to make it fit on the page as much as possible.

    You are doing something with your if statements that can lead to unexpected behavior - not enclosing if bodies with braces. Although C and C++ don't require braces for if, while, etc. bodies that consist of a single statement, it's very easy to come along later and stick in another line, and think it will execute as part of the if body.

    Here's an example.
    Code (Text):

    int p = 0, q = 0;
    int quotient;

    if (q != 0)
       printf("q is not 0.");
       quotient = p/q;
    else
       printf("Division by zero is undefined.");
     
    To the casual observer, it appears that the code guards against dividing by zero, but this isn't the case.

    For this reason, some companies enforce a policy that programmers have to include braces for all if/for/while/etc bodies.

    Here is how I would indent your code - pretty much the same as before. On a line that is the start of a while loop, it's not good practice to indent the left brace AND the first line of the loop body.
    Code (Text):
    void PrintMatrix(Entry *CurrentMatrix, int tSize ([B]number of entries in array[/B]), int nSize ([B]width of matrix[/B]))
    {
       int A;
       int mCount = 1;
       int nCount = 1;

       while (A < tSize)
       {
          int Count;
          while (Count < tSize)
          {
             if (CurrentMatrix[Count].mVal = mCount, CurrentMatrix[Count].nVal = nCount)
            {
               if (CurrentMatrix[Count].nVal = nSize)
                  cout << CurrentMatrix[Count].Value << "\n" << endl;
              else
        cout << CurrentMatrix[Count].Value << "  ";
        Count ++;
            }
            else
               Count ++;
       }
       if (nCount = nSize)
       {
          nCount = 1;
          mCount ++;
        }
        else
           nCount ++;
        A ++;    

      }
    }
     
Know someone interested in this topic? Share this thread via Reddit, Google+, Twitter, or Facebook




Similar Discussions: Weird C++ Array Issue
  1. Arrays in C++ (Replies: 10)

  2. Arrays c++ (Replies: 14)

  3. C++ Array (Replies: 10)

  4. C# arrays (Replies: 1)

Loading...