multidimensional array help

This is a discussion on multidimensional array help within the C Programming forums, part of the General Programming Boards category; Hi! I'm trying to write a program using a two-dimensional array (for school), and I think I've got it, but ...

  1. #1
    Registered User
    Join Date
    Feb 2009
    Posts
    4

    Question multidimensional array help

    Hi!

    I'm trying to write a program using a two-dimensional array (for school), and I think I've got it, but my teacher sent back the code I sent to him, saying there is a "grave mistake" in it. I've been trying to figure it out for the past two hours, but I don't get what he's getting at, so I thought I'd ask around here.

    What I want to accomplish is this:
    I give the program an 'n' number, which is the number of both the rows and the colums of a matrix, after which I'm asked to input further n*n numbers for the matrix. After this, the program is supposed to multiply these numbers by two then write the matrix to the screen.

    What I've come up with for this is:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    int main()
    {
    	int n,i,j;	//n is the number of rows and columns in the matrix, i and j are for the for cycles
    	int k,l;	//k and l are used so that the program starts asking for 1st row 1st number and not 0r 0n
    	int temp1,temp2;
    	int **p=NULL; 
    	printf("n="); scanf("%d", &n);	//inputting n
    	p=(int**)malloc(n*sizeof(int));		//allocating memory for the matrix
    	for(i=0;i<n;i++)
    	{
    		p[i]=(int*)malloc(n*sizeof(int));	
    	}
    	for(i=0;i<n;i++)
    	{
    		for(j=0;j<n;j++)
    		{
    			k=i+1;
    			l=j+1;
    			printf("%d. row %d. number: ",k,l);	//asking for the numbers in the matrix
    			scanf("%d",&temp1);		//inputting the numbers into the first temporary variable
    			p[i][j]=temp1;	//putting it into the matrix
    		}
    	}
    	for(i=0;i<n;i++)
    	{
    		for(j=0;j<n;j++)
    		{
    			temp2=p[i][j];	//putting each number in the matrix into a second temporary variable
    			temp2=2*temp2;	//multiplying by two
    			p[i][j]=temp2;	//putting it back into the matrix
    			printf("%3d",p[i][j]);	//printing out the matix 
    			if(j==(n-1)) {printf("\n");}	//if the end of a row is reached, start a new row
    		}
    	}
    }
    As you can see, I'm not exactly experienced here. After I thought about it a bit, I've figured out that using temp2 is unnecessary and that I could put the second for cycle inside the first (though I don't think this makes the program any better). Also, I figured that I'd set every variable I'm using to 0, so there can be no problem there.

    So this is my second version:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    int main()
    {
    	int n=0,i,j;	
    	int k=0,l=0;	
    	int temp1=0;
    	printf("n="); scanf("%d", &n);
    	int **p=(int**)malloc(n*sizeof(int)); 
    	for(i=0;i<n;i++)
    	{
    		p[i]=(int*)malloc(n*sizeof(int));	
    	}
    	for(i=0;i<n;i++)
    	{
    		for(j=0;j<n;j++)
    		{
    			k=i+1;
    			l=j+1;
    			printf("%d. row %d. number: ",k,l);	
    			scanf("%d",&temp1);
    			temp1=2*temp1;
    			p[i][j]=temp1;
    			if((i==(n-1))&(j==(n-1)))	//only start writing the matrix once every number has been read and multiplied 
    			{
    				for(i=0;i<n;i++)
    				{
    					for(j=0;j<n;j++)
    					{
    						printf("%3d",p[i][j]);
    						if(j==(n-1)) {printf("\n");}
    					}
    				}
    			}
    		}
    	}
    }
    Is this any better than the first? What was the "grave mistake" in the first one, and did I accidentally solve it with the second one? Also, the teacher mentioned that there was a simpler way of doing this. If anyone could help, I would much appreciate it.

    Thank you very much, in advance.
    Last edited by dg862; 02-14-2009 at 03:56 PM.

  2. #2
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Quote Originally Posted by dg862 View Post
    I'm trying to write a program using a two-dimensional array (for school), and I think I've got it, but my teacher sent back the code I sent to him, saying there is a "grave mistake" in it.
    I have a university degree and your prof is 1) a jerk 2) a jerk, and 3) a jerk, and I would bet money if you ask other students or even other professors they will confirm that. Don't take a class with him/her again unless you have to, and don't waste your brain trying to satisfy this person when you almost certainly have other and better opportunities to actually learn.

    Your second version works fine, hand it in -- except I would add "return 0" at the very end.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  3. #3
    Registered User
    Join Date
    Feb 2009
    Posts
    4
    Wow, I didn't expect such a fast answer. Thanks. Yeah, I think'll hand it in like you said. And the teacher is pretty young, so he probably wants to show that he can do it "properly". Anyway, thanks again for the quick answer.

  4. #4
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Well, I don't want to mislead you -- my degree is not in computing. But if I walked into a prof's office during his/her consultation hours with a question or whatever and received that kind of non-advice, I wouldn't bother asking for any more advice. If he believes this is the extent of his job, he must be aware that he himself would easily be replaced by:

    - some textbooks
    - a C compiler
    - an assignment submission site

    I think you are paying for more than that, tho.

    w/r/t your code you might also want to free() what you malloc at the end. It doesn't really matter here, but at least this will demonstrate that you understand the need.

    Also, gcc -pendantic advises:
    test2.c:10: warning: ISO C90 forbids mixed declarations and code
    which means you should swap line 9 and 10 around. It is much nicer to put all your declarations first, since then you know exactly where they are when your code gets longer. I believe the compiler prefers this too, someone in the know here may explain.
    Last edited by MK27; 02-14-2009 at 04:26 PM.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  5. #5
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    975
    Code:
    p=(int**)malloc(n*sizeof(int));
    This is the only "real" problem I see with the code. You're allocating pointers here, not ints, so you should multiply by sizeof(int *); or, perhaps somewhat nicer:
    Code:
    p = malloc(n * sizeof *p);
    On systems where sizeof(int) is the same as sizeof(int *) your code works, but it's still not correct, because there's no guarantee that the sizes are the same with all compilers.

    As for the second vs. the first, the second is a lot less clear. If you want to do something after your loops are done, do it after the loops. That is to say, the first example makes more sense than the second one.

    It's true that you don't need a temporary in order to multiply each value by two when printing out, and so that should be done as in the second example. Also, when you're printing out newlines for each row, it makes a lot more sense to do it like this:
    Code:
    for(i = 0; i < n; i++)
    {
      for(j = 0; j < n; j++)
      {
        printf("%3d", p[i][j]);
      }
      putchar('\n');
    }
    No need to test j; just print out a newline each time a row is completed.

    Expressions can also be used in places where you put variables. What I mean is this:
    Code:
    k=i+1;
    l=j+1;
    printf("%d. row %d. number: ",k,l);
    
    /* That is cumbersome compared to: */
    printf("%d. row %d. number: ", i + 1, j + 1);
    And, in fact, you can remove your temp1 variable using the same idea:
    Code:
    scanf("%d", &p[i][j]);
    p[i][j] *= 2;
    Whether you should multiply by two when reading in or printing out depends on the requirements. For this program it really doesn't matter, unless your professor cares.

  6. #6
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Yay cas!
    Quote Originally Posted by cas View Post
    Code:
    p=(int**)malloc(n*sizeof(int));
    This is the only "real" problem I see with the code. You're allocating pointers here, not ints, so you should multiply by sizeof(int *); or, perhaps somewhat nicer:
    Code:
    p = malloc(n * sizeof *p);
    I had not noticed this, but it certainly would constitute a "grave mistake" in the sense that it will work (since an int is at least the size of a pointer) but if these had been chars, you would have created an overflow.

    It only applies to the array declaration. You are not using a "pure" 2D array which would be this:
    Code:
    int matrix[10][10];
    You are using a 1D array of pointers to more 1D arrays. That's why these two lines should be different:
    Code:
    	int **p=(int**)malloc(n*sizeof(*int)); 
    	for(i=0;i<n;i++)
    	{
    		p[i]=(int*)malloc(n*sizeof(int));	
    	}
    I would still stick with the second version but separate the loops as cas says.
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  7. #7
    Registered User
    Join Date
    Feb 2009
    Posts
    4
    Also, gcc -pendantic advises:
    test2.c:10: warning: ISO C90 forbids mixed declarations and code
    which means you should swap line 9 and 10 around. It is much nicer to put all your declarations first, since then you know exactly where they are when your code gets longer. I believe the compiler prefers this too, someone in the know here may explain.
    You're right, I tried running it like I posted it and Visual Studio didn't like it very much, but swapping those lines works.

    Quote Originally Posted by cas View Post
    Code:
    p=(int**)malloc(n*sizeof(int));
    This is the only "real" problem I see with the code. You're allocating pointers here, not ints, so you should multiply by sizeof(int *); or, perhaps somewhat nicer:
    Code:
    p = malloc(n * sizeof *p);
    Yes, that makes sense, now that I think about it. I felt there was something strange about that, just couldn't put my finger on it.

    All your other suggestions are helpful too. It's funny I didn't spot that I could do away with k,l and temp1 as well. Hopefully this will satisfy my prof., though even if it doesn't I think I've at least learned some handy things here. I guess these are rather basic for a more experienced programmer, but I'm only a newbie yet.

  8. #8
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,439
    Two more issues for completeness.

    1. You should check each malloc call for success. If malloc returns NULL, you need to take care of that.

    2. You should call free as well, first a loop for each p[i], then free p
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  9. #9
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,047
    imo you need only two loops; one for malloc()'ing memory / initializing array elements etc. and another loop for printing out the elements of the 2D array; and make use of the dereferencing operator "*" instead of using the p[i][j] syntax; and i see no "grave mistake" other than the one cas has already pointed out.
    Last edited by itCbitC; 02-15-2009 at 02:51 PM.

  10. #10
    Registered User
    Join Date
    Feb 2009
    Posts
    4
    Thank you for all your responses. I think this will be the final version:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    int main()
    {
    	int n,i,j;	
    	int **p=NULL;	
    	printf("n="); scanf("%d",&n);	
    	p=malloc(n*sizeof(*p));	
    	for(i=0;i<n;i++)
    	{
    		p[i]=(int*)malloc(n*sizeof(int));	
    	}
    	for(i=0;i<n;i++)
    	{
    		for(j=0;j<n;j++)
    		{
    			printf("%d. row %d. number: ",i+1,j+1);	
    			scanf("%d",&p[i][j]);	
    			p[i][j]*=2;		
    		}
    	}
    	for(i=0;i<n;i++)
    	{
    		for(j=0;j<n;j++)
    		{
    			printf("%d ",p[i][j]);	
    		}
    		putchar('\n');	
    	}
    	for(i=0;i<n;i++)
    	{
    		free(p[i]);		
    	}
    	free(p);
    	return 0;
    }
    I changed the memory allocation (nice one spotting that, by the way), left out the unnecessary variables and I'm using free at the end too. Left the for cycles separate in the end.

    imo you need only two loops; one for malloc()'ing memory / initializing array elements etc. and another loop for printing out the elements of the 2D array
    You mean like this:
    Code:
    p=malloc(n*sizeof(*p));
    	for(i=0;i<n;i++)
    	{p[i]=(int*)malloc(n*sizeof(int));	
    		for(j=0;j<n;j++)
    		{
    			printf("%d. row %d. number: ",i+1,j+1);
    			scanf("%d",&p[i][j]);
    			p[i][j]*=2;	
    		}
    	}
    Heh, that's true. Another thing I didn't notice. Though I think I'm fine with more for cycles, if only to better see what I'm doing in each one.

    1. You should check each malloc call for success. If malloc returns NULL, you need to take care of that.
    Yeah, I know that's how it's properly done, but the task was writing a program that works right, so it's not supposed to give an error message at all.

  11. #11
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Re: error-checking malloc, you can do this:
    Code:
    void *ec_malloc (size_t bytes) {
    	void *assign;
    	assign = malloc(bytes);		
    	if (assign == NULL) {
                 puts("OUT OF MEMORY!??!");
                 /* maybe you want to do other different stuff here now */
                 return NULL;
            }
    	return assign;
    }
    ec_malloc() can then be substituted for malloc():
    Code:
    int *ray=ec_malloc(12*sizeof(int));
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  12. #12
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,047
    Note that p[i] is the same as &p[i][j] and p[i][j] *=2 can be written as *p[i] *= 2. This is based on the fact that x[n][n] is equivalent to y[n*n] as in.
    Code:
    p=malloc(n*sizeof(int*));	
    for(i=0;i<n*n;i++)
    {
        p[i]=(int*)malloc(sizeof(int));	
        scanf("%d",p[i]);	
        *p[i] *=2;		
    }
    Based on this the two inner for loops can be eliminated. That's just my 2c, though you have to decide if code conciseness is to be preferred over clarity.

  13. #13
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,294
    Quote Originally Posted by itCbitC View Post
    Note that p[i] is the same as &p[i][j] and p[i][j] *=2 can be written as *p[i] *= 2. This is based on the fact that x[n][n] is equivalent to y[n*n] as in.
    Code:
    p=malloc(n*sizeof(int*));	
    for(i=0;i<n*n;i++)
    {
        p[i]=(int*)malloc(sizeof(int));	
        scanf("%d",p[i]);	
        *p[i] *=2;		
    }
    Based on this the two inner for loops can be eliminated. That's just my 2c, though you have to decide if code conciseness is to be preferred over clarity.
    The above advice and code is completely wrong! p[i] is definitely not the same as &p[i][j]

    There are two obvious ways of representing a 3D array. One is the way dg862 has used, the other way is the bitmap scanline type of approach where you store each row in consecutive memory, all in one big array. Some code for this way would actually be:
    Code:
    int *p=malloc(<n*n*sizeof(int));	
    for (i=0; i<n*n; i++)
    {
        scanf("%d", &p[i]);	
        p[i] *=2;		
    }
    Last edited by iMalc; 02-15-2009 at 11:09 PM.

  14. #14
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,047
    Ah! yes I forgot to square the array rank while malloc()'ing memory so here's the one w/o the typo.
    Code:
    p=malloc((n*n)*sizeof(int*));	
    for(i=0;i<n*n;i++)
    {
        p[i]=(int*)malloc(sizeof(int));	
        scanf("%d",p[i]);	
        *p[i] *=2;		
    }
    The rest stays as is because this is a 2D array so p[i] == &p[i][j] except if it were a 3D.

  15. #15
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by itCbitC View Post
    Ah! yes I forgot to square the array rank while malloc()'ing memory so here's the one w/o the typo.
    Code:
    p=malloc((n*n)*sizeof(int*));	
    for(i=0;i<n*n;i++)
    {
        p[i]=(int*)malloc(sizeof(int));	
        scanf("%d",p[i]);	
        *p[i] *=2;		
    }
    The rest stays as is because this is a 2D array so p[i] == &p[i][j] except if it were a 3D.
    I think you mean that p[i][j] (for normal people) here is represented as p[n*i + j]. (Because naturally p[i] cannot possibly equal p[i][j] for all j.)

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 16
    Last Post: 05-29-2009, 07:25 PM
  2. Help -- allocating memory in a multidimensional array
    By jonathan.plumb in forum C Programming
    Replies: 10
    Last Post: 05-20-2009, 11:04 PM
  3. Pointer to multidimensional array
    By ejohns85 in forum C++ Programming
    Replies: 4
    Last Post: 03-24-2009, 11:17 AM
  4. Type and nontype parameters w/overloading
    By Mr_LJ in forum C++ Programming
    Replies: 3
    Last Post: 01-02-2004, 12:01 AM
  5. Help with an Array
    By omalleys in forum C Programming
    Replies: 1
    Last Post: 07-01-2002, 08:31 AM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21