Suggestions in improving and writing user input based program

[jui]

Hello,
I am working on the code for a while but I could hardly make this better but some changes manually in the code which is very inefficient and error prone when I need to use more input and output for testing and verify the result. I am looking for some experts suggestions to make this code better. My programming experience is new but I am eager to improve this for writing a better code. Any suggestions and comments as well as advice will be highly appreciated

[melkiy]

What conserns the memory...
Replace each memory allocation like
float *predCol = (float *)malloc(sizeof(float)*rdata.frames);
with
float *predCol = new float[rdata.frames];

new operator of this type requires number of elements in brackets, not number of bytes.
Take into account that operator new not always returns NULL in case of failure. Verstion of new from standard C++ library throws bad_alloc exception.

Replace this
free(predCol);
with
delete[] preCol;

[jui]

Thank you very much for your kind help, Melkiy.

I tried to do this according to your advice as:

data = new float(hdr.sampSize*hdr.nSamples);
delete [] data;
predCol= new float(tframes); curCol = new float(tframes);
delete [] predCol; delete [] curCol;
instead data = (float *)malloc(hdr.sampSize*hdr.nSamples); and predCol = (float *)malloc(sizeof(float)*tframes); free(predCol); curCol = (float *)malloc(sizeof(float)*tframes);
free(curCol)

Hopefully I followed this right.

Now, I see some problems and I will appreciate any help and advice,

The loop number in a 2 dimension data file starts from number 5 row because 1 to 5 is zero and I need to avoid zero. The two dimension data look like as follows:
.
.
2: 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000
0.000 0.000 -0.151
3: 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000
0.000 0.000 -0.151
4: 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000 0.000
0.000 0.000 -0.151
5: -2.778 -6.403 4.641 -6.572 6.769 -7.476 2.578 -5.711 5.012 -6.111
4.399 -3.106 0.901
6: -2.612 -5.778 5.451 -5.569 8.488 -7.375 2.698 -5.647 6.403 -7.455
5.122 -4.319 0.985
7: -0.948 -2.702 9.042 -0.920 13.847 -3.659 5.798 -2.127 11.096 -6.141
8.997 -3.309 1.000
8: 7.406 7.580 14.425 4.389 9.297 1.981 8.568 -0.703 4.459 1.386
6.138 -0.332 0.960
9: 4.906 7.771 16.425 2.042 14.991 2.381 1.123 -8.163 -0.299 -3.751
5.068 4.930 0.860 .......
.
.

and the same structure in other two dimension data.
The loop I am using as follows so that it reads each number(coeffs) in each row(5, 6, 7, .....549)

predCol[5] = d(5, 5, coeffs, tdata, pdata);

//cout << predCol[5] <<endl;
//for(p = 13; p < templateframes; p++)
//cout <<" p " << p << endl;
predCol[p] = predCol[5] + d(5, 6, coeffs, tdata, pdata);

/**
//cout <<"d(12, 13, coeffs, tdata, pdata)" << d(12, 13, coeffs, tdata, pdata) << endl;

//cout << predCol[p] <<endl;
**/

for(p = 7; p < templateframes; p++)
predCol[p] = predCol[p - 1] + d (6, p, coeffs, tdata, pdata);

cout << " d (6, p, coeffs, tdata, pdata)"
<< d (6, p, coeffs, tdata, pdata) << endl;
cout << "predCol[p] " << predCol[p] << endl;
for(i = 6; i < patternframes; i++)
{
curCol[5] = predCol[5] + d(6, 5, coeffs, tdata, pdata);
for(j = 6; j < templateframes; j++)
curCol[j] = (min(predCol[j], predCol[j - 1], curCol[j - 1])
+ d(i, j, coeffs, tdata, pdata));

delete[] predCol;
predCol = curCol;
curCol = new float (templateframes);
}

lowestGlobal = predCol[templateframes - 1];

cout << "lowestGlobal " << lowestGlobal << endl;

}


But this is not giving me result as expected but some negative numbers which can not be true since I am calculting Euclidean distance between the elementents in each row in the two dmension two data file.

I will appreciate any help and advice in this regard.

I look forward to hearing from you.

With sincere thanks,
Jui

[melkiy]

I have not even read the whole your post, but you misuse operator new!

Here is incorrectness (thought compilabe, but that is even worse...):
data = new float(hdr.sampSize*hdr.nSamples);

This operator creates only one float object with the specified value!

If you intend to allocate memory for ARRAY, use SQUARE brackets
data = new float[hdr.sampSize*hdr.nSamples];

Be careful!
And, please, enclose your big code in the code tags (see small Format-bar to the right of the box you type your message in).

[jui]

Hello,

I am having memory allocation for a while. I attach a part of my attempt for your kind review and advice. I have error as
error:

malloc: *** error for object 0xbffff370: Non-aligned pointer being freed (2)
*** set a breakpoint in malloc_error_break to debug


So far, I have learned memory deallocation is essential part. But in my task (even this is not completed and not doing what is expected yet,) the programs run without giving me error when I do not use delete [] localdist. But I have a recursion computation and this is quite large ( more than 3000 float computation). Later at one stage, I noticed I had to force quit and reboot my MacBook since I am using MacOS. Please advice me if the memory allocation is reasonable here.
Thank you for your attention and response. (Hope I am able to post the message correctly this time.)

float *localdist[reference.n];
	
	
	try{
	for (int i = 0; i < reference.n; i++){
		localdist[i]  = new (nothrow) float(ByteSwapFloat(reference.data[i]));
		}
	}	
	catch (bad_alloc){
		cerr << "error in memory allocation "<<endl;
		exit(1);
	}

delete [] local dist;
delete [] reference.data;

[firedraco]

I think the problem is you are using:

delete [] local dist;

Try removing the space and see if it works. Also, you do know that by creating localdist like this:

float *localdist[reference.n];

You are creating a *pointer* to an array, not an array. Just use [] if you want to make an array by itself.

[Zhuge]

@jui: If you use new with nothrow, there is no point in trying to catcha badalloc exception since you made it unable to throw exceptions.

[Zaita]

Errr....

Your code doesn't fully illustrate your intention. At first glance it appears you are trying to create an array of pointers to floats (essentially a Matrix/2D Array). But I think you've trying to create a dynamic array correct?

Dynamic Array code:

 float *localdist = new float[reference.n];

 for (int i = 0; i < reference.n; ++i) {
  localdist[i] = ByteSwapFloat(reference.data[i]);
 }

 delete [] localdist;
 // Why are you deleting reference.data here? It should be deleted in destructor of object 

[jui]

Thank you very much to Firedraco, Zhuge and Zaita. The advice is very helpful.

The memory allocation works at the moment only

float *localdist = new float[reference.n];
delete [] localdist;

I have another question and some hints will be very helpful.

I have a prototype asspeech_form readmfc(char *filename)

I need to pass a list of files in two phases in the mentioned prototype: in one phase filename will be change and there will be more files(i.e., now this is 20 files) and in the second phase the filename is same which will be used for some computations by the files listed in the first phase: that is
argv[1] - argv[2] where argv[1] will be the diskstorage files which will be changed by some file names for some files but argv[2] will be the input argument(cin) and this will remain the same for one test and this continues where argv[2] will be changed while there are more tests (and argv[2] will be changed as input when the test changes but argv[1] will be changed in each test several times i.e., file1 - argv[2], file2- argv[2] ....file20-argv[2]). I was trying this by putting the file names manually typing but it seems very inefficient poor work and I am asked to make this automatic so the program shows the output when one can give speech input on the microphone. because this is an speech related application If the explanation is not clear and confusing; perhaps I can post my effort for your kind suggestions for making this good.

Thank you for all the support.

[Zaita]

That prototype doesn't support passing in a list. Only a string (char *).

e.g readmfc("my file");

Again, you need to simplify how you are asking your question down to specific points. It's very difficult to comprehend your intention.

From what I can extract, your asking how you can auto-load a list of files? You want to put them into a directory, or give them a unique extension. From here you can use Filesystem API's to scan for these files in the target location and load them dynamically.

http://www.boost.org/doc/libs/1_35_0/libs/filesystem/doc/index.htm
might be a good read for you. Since you haven't specified your platform, boost is multi-platform.

[jui]

Thank you very much for the help, Zaita. I am going through the suggested site. I was thinking to use something in the STL library to read all the files contents one file at a time in a directory not just listing the filename. My present program just gives me the list of the file names in a directory( including some . .. syntax). I am using MacOS Leopard 10.5.5 but the result of the task has to be platform independent.

I have some confusion in memory allocation and deallocation. I do not understand when to deallocate memory because as a rule of thumb when I try to deallocate memory, I receive error message. I do not know if my program is taking my memory away due to my present lack of understanding about this memory management. Is there any away to know how the memory is missued or leaking or causing some problems on the diskspace. I do not understand this at the moment because my new Macbook has 2GH memory and more than 100 G harddrive space available right now; so this is fast now. But continuous memory leaking in my program perhaps harmful since I am using many speech files which takes huge space. How the damage or memory loss can be notified earlier than last moment. I understand that there is some problems in my elementary but improving programming skills but how can I learn more and deppt about dynamic memory management in C++ data structure problems for an elegant program?

Thank you.

[firedraco]

Well, theoretically memory leaks will "go away" if you restart your computer. Although, obviously it's better to never have them happen in the first place XD. As for notifying of memory leaks...well, you can't...the best you can do it proofread your own code or switch over to smart pointers (Boost is good here)/automatically managed containers (std::vector, etc).

[Zaita]

You only need to de-allocate what you allocate manually. So unless your using the new keyword there is no need to de-allocate. However, every variable you create has a useful lifetime. You de-allocate the memory once it's usefulness is complete. I typically allocate variables in a constructor, and de-allocate them in the destructor.

@firedraco: Actually, there are many profiling tools that will notify you of memory leaks. I use a professional tool called AQTime, but unfortunately it has a hefty price tag. There are some free tools out there, but I have not tried any.

[jui]

Thank you very much for the info and advice, Firedraco and Zaita. This gives me some direction to proceede.