Re: Newbie: out of memory issues

On Thu, 21 Aug 2008 10:12:37 -0500, trx32@xxxxxxxxx wrote:

Yes ,

Heres the problem code:: Lots of array handling with objects..
I'm sure there's a better way to init arrays of strings(below).


thanks, Tony C.

int CMyFx::Calc_Charts_And_Houses(CChart Chart1,int intCht)
{
double X[6] = {0,0,0,0,0,0};
double Y[6] = {0,0,0,0,0,0};
double cusp[13];
double ascmc[10];
int cal = 103;
string ss(16,' ');
****
Using std::string in MFC is usually a very bad idea. You should use CString.
****
char out1[31];
char out2[31];
char out3[31];
****
The above are mistakes. They are the 'char' type, which is obsolete. They are character
buffers of fixed size, which should be an obsolete concept except in very rare and exotic
circumstances, of which this is not an example. They should be
CString out1;
CString out2;
CString out3;
and that's all you should need. If you ever declare a 'char' array of fixed size
anywhere, and most especially on the stack, you can consider it a fundamental programming
mistake except in EXTREMELY rare and exotic circumstances, of which this is clearly not
one.
****


double h, tjd_ut, et_flag, tjd_et, t2;
****
Avoid commas in declaration lists. One variable, one declaration line. Makes the code
much easier to read.
****
int olen, retval, ret_flag;
int32 iflag;
string strUT;
****
If programming in MFC, do not use 'string', use 'CString'

There is rarely a need to use the obsolete C style where all variables are declared at the
front of the function; you declare them as they are needed.
****


h = Chart1.dbHour + Chart1.intMin / 60.0;

olen = ss.length();

tjd_ut = swe_julday(Chart1.intYear, Chart1.intMonth, Chart1.intDay,
h, 1);

retval = swe_date_conversion(Chart1.intYear, Chart1.intMonth,
Chart1.intDay, h, cal, &tjd_ut);
****
You should not be passing pointers in unless the function is willing to accept a NULL
pointer. The prototye of this function should be
BOOL swe_date_conversion(int year, int month, int day, int hour, int cal, double &
ut);

and not use the & operator on the variable. You are programming in 1975 C in 2008. Use
references most of the time. One way to tell if you should have used a reference instead
of a pointer is if you ever write
if(ut == NULL)
return ...some error indication...;
If you can't accept a NULL, do not give the user the opportunity to pass one.
****


if (retval != 0) {
return -1;
****
If you have only two return values, why is the prototype not a BOOL?
****
}


tjd_et = tjd_ut + swe_deltat(tjd_ut);

t2 = tjd_ut - 2415018.5;

if (t2 < 0 )
{
t2 = t2;
}

if (tjd_ut == tjd_et)
{
strUT = " ";
****
strUT = _T("");
or
strUT.Empty();
****
}
else
{
strUT = " UT= "; // + tjd_ut
****
strUT.Format(_T(" UT= %g"), tjd_ut);
I presume from the "+ tjd_ut" the goal was to print this out. For debugging, %g is
usually "good enough" but you can use %f or %e formats if you want.
****
}


int intX;
intX = 1;
****
WHy two lines?
int intX = 1;
****

//**************************************************************************************
// MAIN LOOP HERE BASICALY FROM 1 TO 11 :::
//**************************************************************************************

for (int planet = SE_SUN; planet <= SE_TRUE_NODE; planet++)
{


iflag = SEFLG_SPEED + SEFLG_SWIEPH;



char* strErr;
//CString strErr;
char* strPlname;
strErr = new char[256];
****
char and char* are obsolete and should not be used. You are programming for 1975, not
2008. In fact, the commented-out CString is the correct declaration. Why are you
allocating a string on the heap? Why are you hard-wiring the apparent random constant
256? Why are you using this constant hardwired in more than one place? This may be part
of your leak, by the way. There is no need to use 'new' here!
****

strPlname = new char[21];
****
Here is a 'new'. What does it do? Why is it here? It serves no useful purpose.
****

//*****************************************************************************
// PROBABLY A BETTER WAY TO DO THE BELOW ROUTINES?
//*****************************************************************************
for ( int i = 1; i < 256; i++) {
strErr[i] = ' ';
}
*****
I have no idea what the purpose is for this. There would be no sane reason to set a
string that contains nothing to hold 256 spaces, and in any case, if you wanted to write a
loop like this, it would have been
static const UINT MAX_LENGTH = 256;
strerr = new char[MAX_LENGTH];
for(int i = 0; i < MAX_LENGTH; i++)
strErr[i] = _T(' ');
strErr[MAX_LENGTH] = _T('\0');

but fundamentally, code like the above is unnecessary. The simple declaration
CString strErr;
is sufficient. You do not need to initialize it to anything because the default CString
constructor creates an empty string
*****

for ( int i = 1; i < 21; i++) {
strPlname[i] = ' ';
}
****
Ditto. Where did the magical value 21 come from? Why is it hardwired in two different
places? Why is there a 'new' involved in this at all?
****
//*******************************************************************************


//************************************************************************
// MAIN CALCULATION HERE:
ret_flag = swe_calc(tjd_et, planet, iflag, X, strErr);

//************************************************************************

//strErr = TrimRight(strErr, ' ');
****
This doesn't even make sense. strErr has not been set and therefore there would be no
need to trim it; in fact, the trim would defeat the initialization loop you did, meaning
the entire initialization loop is a waste of time, space, and programming effort.
****

swe_get_planet_name(planet, strPlname);
****
The prototype for this function should be
void swe_get_planet_name(planet, CString & strPlname);
or
CString swe_get_planet_name(planet);
and store the result as
strPlname = swe_get_planet_name(planet);
****

strPlname = TrimRight(strPlname, ' ');
****
This should be
strPlName.Trim();
since you probably want to delete all leading and trailing spaces
****

if (intCht = 1)
****
Either this is erroneous code because you mean to write
if(intCht == 1)
or it is erroneous code because you meant to write
intCht = 1;
if(intCht)

(never, ever embed an assignment in an if-statement; that when, when you see a piece of
code that has an =, you know immediately that it is incorrect code and it should have been
==)
***
{


NPlanets[intX].intBody_Num = intX;
NPlanets[intX].dbRawDegree = X[0];
NPlanets[intX].intSign =getIntSign(NPlanets[intX].dbRawDegree);
NPlanets[intX].strSign = getStrSign(NPlanets[intX].dbRawDegree);
NPlanets[intX].dbNormDegree
=getChoppedDegree(NPlanets[intX].dbRawDegree);
NPlanets[intX].strBody_Name = strPlname;
NPlanets[intX].dbEcLat = X[1];


iflag = SEFLG_SPEED + SEFLG_SWIEPH + SEFLG_SIDEREAL;

swe_set_sid_mode(0, 0, 0);

ret_flag = swe_calc(tjd_et, planet, iflag, Y, strErr);

strErr = TrimRight(strErr,' ');
****
The prototype of swe_calc should be
BOOL swe_calc(double et, int planet, DWORD iflag, double Y, CString & strErr);

Also, if strErr is a CString, you should write
strErr.Trim();
altnough it is not clear why the function would gratuitously place spaces front or back,
but if it does, you only need the Trim.

Pretend you never heard of the 'char' data type or arrays of char. You will be far more
productive, your code will be more robust, be much easier to write and debug.
*****


NPlanets[intX].dbDeclination = Y[1];

strcpy(out1,strPlname);
****
You should not be using strcpy, which is both dangerous and obsolete. Assume TCHAR is
your basic character data type. But what is out1 used for, and why is it not a CString so
you can just write
out1 = strPlname;
You declared a fixed-size array on the stac, and this is immediately an example of very,
very bad coding style except in the aforementioned extremely rare and exotic
circumstances, and this isn't even CLOSE to rare-and-exotic. So don't do it.
****
}
else
{

N2Planets[intX].intBody_Num = intX;
N2Planets[intX].dbRawDegree = X[0];
N2Planets[intX].intSign = getIntSign(N2Planets[intX].dbRawDegree);
N2Planets[intX].strSign = getStrSign(N2Planets[intX].dbRawDegree);
N2Planets[intX].dbNormDegree =
getChoppedDegree(N2Planets[intX].dbRawDegree);
N2Planets[intX].strBody_Name = strPlname;
N2Planets[intX].dbEcLat = X[1];


iflag = SEFLG_SPEED + SEFLG_SWIEPH + SEFLG_SIDEREAL;

swe_set_sid_mode(0, 0, 0);

ret_flag = swe_calc(tjd_et, planet, iflag, Y, strErr);

strErr = TrimRight(strErr,' ');
****
Same issue about swe_calc. I note that although you store the result, you don't actually
test it to see if the call succeeded...
****


N2Planets[intX].dbDeclination = Y[1];

strcpy(out1,strPlname);
****
Why strcpy to a buffer? Same question: why is out1 not a CString?

Hint: if you EVER find yourself writing strcpy or strcat, you have a fundamental
programming error. Once you are a highly-experienced programmer, you may, under rare
conditions that hardly ever come up in practice, find a need to write strcpy_s or
strcat_s, but the conditions under which these would appear are vanishingly small and
would not apply in most programs. Nor would the equivalent _tcscpy and _tcscat methods,
which are also dangerous and therefore must be thought of as obsolete.
joe

****



}







intX++;



}


ret_flag = swe_houses_ex(tjd_ut, iflag, Chart1.dbLat, Chart1.dbLon,
int('P'), cusp, ascmc);
for ( int i = 1; i < 13; i++) {


//MyHouses are of type 'double'

MyHouses[i] = cusp[i];

}





}
Joseph M. Newcomer [MVP]
email: newcomer@xxxxxxxxxxxx
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
.

Relevant Pages

  • Re: socket communication: send & receive doesnt work right
    ... Maybe start from sending simple byte array to check what happen with byte ... I did a test of sending two doubles: ... public void send_doubles(double vals, int len) throws IOException ... char *result; ...
    (microsoft.public.win32.programmer.networks)
  • Re: Memory Allocation Problem, please help
    ... typedef struct word_tag{ ... array is not an array. ... static int total_word_count; ... static int word_index(const char *word); ...
    (comp.lang.c)
  • Re: C code
    ... Which Richard are you addressing, ... an array and a pointer to the first element of an array have, ... char input; ... mainreturns an int, so you should return an int. ...
    (comp.lang.c)
  • Doing things the hard way...accessing an array as different data types
    ... of data-types of my choosing (eg an array of char, short, or int). ... single void* cast to the proper type as the return value of getter ...
    (comp.lang.cpp)
  • Re: conversion of cstring to char[]
    ... You cannot "add" strings together like VB. ... You need to make your character array large enough to hold the entire string ... your using CString, your obviously using MFC, so why not just use a CString ... If you absolutely must use a char[], then you can either allocate the array ...
    (microsoft.public.vc.language)