Title: Student Code Critique Competition

Before writing a single line of code you need to have a clear idea about what you are trying to produce. In this case it seems that the objective is to produce a file of values generated by some form of generator. I am certain that the actual code does not do what the programmer thinks because it is littered with quirky bits. If it did actually generate the desired values it would be pure chance.

Fundamentally there are two ways to create an output file. The first way is to have the program create the required file. The second way that is often forgotten is to simply capture the program output into a file. What you do not do (or only very rarely) is to capture the output from the screen and paste it into a file. It seems that the student has been trying to use this last method.

There are times when we have to fall back on implementation specific features and library functionality but they are much less common than most students believe.

That almost speaks for itself so I will just leave it as a thought to put alongside 'Do things in time.'

Here is my rewritten code up to the student's first comment:

# include<iomanip>
# include<iostream>
int main() {
  static int sum[5000][5000];
  sum[1][1]=25000;
  sum[2][1]=35000;

Two of the header files go. We do not need conio.h because we are not going to use the screen as some intermediary for getting the output where we want it. Looking forward I realise that the programmer only needed math.h to give him access to the declaration of pow(). Using that function to compute successive powers of two is overkill. It is the kind of overkill that inexperienced programmers are very prone to. They think 'power of two' and forget that what they needed was 'next power of two' which is simply two times the current one.

While I was about it I replaced the header files with the equivalent Standard C++ headers.

Next I tidied up the declaration of main() to return the required int, and threw out the definitions of i etc. because this is far too early to be declaring single letter variables (I have nothing against them but they should always be declared in the context of their use.) Even C now allows that. clrscr() went because it serves no useful purpose.

I think the student was getting away with an implicit int in the declaration of Sum, which I have amended to sum because I cannot abide gratuitous use of upper case. In case you are wondering, removing the static storage class specifier would result in undefined behaviour because the array would not have been initialised. Of course there are other ways of ensuring that an array is fully initialised.

In passing, I feel profoundly uncomfortable with that requirement for storage for twenty-five million ints. I am sure that a correct analysis of the required computation will demonstrate that a much less memory demanding approach is viable.

Now let me look at the computation section. This definitely can do with some restructuring because, frankly, it is currently a hacked together mess.

int i_limit = 4;
for(int j=2; j != 12; ++j, i_limit *=2) {
  sum[1][j] = sum[1][j-1] + 25000;
  sum[2][j] = sum[1][j-1] + 35000;
  for(int i=3; i<= i_limit; i += 2) {
    sum[i][j] = sum[2][j-1] + 25000;
    sum[i+1][j] = sum[(i+1)/2][j-1] + 35000;
  }
}

Where has the r gone? Careful analysis of the control flow of the original code showed that r was always 2 when used. I find that very suspicious and think that it probably should have been equal to i. However the point I would make to the student is that by cleaning up the code we can see what is happening. This will make it much easier to correct errors in the code. That '2' where r was sticks out like a sore thumb. Indeed I suspect that the correct code should be:

int i_limit = 4;
for(int j=2; j != 12; ++j, i_limit *=2) {
  for(int i=1; i<= i_limit; i += 2) {
    sum[i][j] = sum[i][j-1] + 25000;
    sum[i+1][j] = sum[(i+1)/2][j-1] + 35000;
  }
}

Once we have it down to this level, we can see that the original dimensions of sum are ridiculous. In fact he has 10 sets of results, each set solely dependant on the immediately prior set. Allowing indexing from 1 rather than 0 and not even attempting to optimise storage we get:

static int sum[2049][12];

as being entirely sufficient. This reduces the storage requirement to something reasonable on most hardware (around about a hundred thousand bytes on a platform using 4-byte ints.

Finally let me look at the output section. This also has its oddities. Here is my rewritten version:

i_limit = 2; // reset to initial value
for(int j=1; j != 12; ++j, i_limit *= 2) {
  for(int i=1; i <= i_limit ; ++i){
    std::cout << i << j << std::setw(10)
              << sum[i][j] << '\n';
    std::cout << i << j << std::setw(10)
              << sum[i][j] << '\n';
  }
}

Now why the student wants to output the values of i and j without an intermediate space, and why he wants to output each result twice will for ever remain a mystery but the process of cleaning up his code makes it quite clear that that is exactly what he does. By the way, for consistency I changed the output loop so that it will display the initial data as well as the computed data.

Now I suspect that all the code mangling came about while the student struggled to solve the problem of getting the output into a text file. His mind was so focused on trying to cope with this that everything else went up in flames. So how to get the data into a file? Simply, invoke the program with:

program > data.txt

Which I think makes it clear that the student actually needed a very different answer from the one he was asking for.

I am little confused about return values of sizeof operator. Here is a simple C program which is putting me in doubt:

#include <stdio.h>
int main() {
  int a[10];
  int *p =(int *) malloc (10);
  printf("Size of a = %d \n",sizeof(a));
  printf("Size of p = %d \n",sizeof(p));
}

Output is :

Size of a = 40
Size of p = 4

My understanding says even array name is a pointer. If so why it does not show sizeof(a) as 4? Or if sizeof shows the total allocated memory then why sizeof(p) does not show 10?

There are numerous errors in both the code and the student's understanding. Please address these comprehensively, perhaps including places where your explanation would be different if this were a C++ program.

I am getting an error linker error. Here is the code:

// Define Libraries
#include <stdio.h>
#include <math.h>
//Start Of Main Program
main(){
  double hypo, base, height;
/* Enter base and height */
  printf("Enter base:");
  scanf("%f", &base);
  printf("Enter height:");
  scanf("%f", &height);
  hypo = sqrt(pow(base, 2)+pow(height, 2));
  printf("hypotenuse is %f", &hypotenuse);
}

Ignore the question asked by the student and address the serious problems with the code itself.