Want to protect your cyber security and still get fast solutions? Ask a secure question today.Go Premium

x
?
Solved

Experts-please review my function and see if it is "good" enough..!

Posted on 2000-03-27
3
Medium Priority
?
237 Views
Last Modified: 2013-11-20
hi guys,

I have code a function that reads lines from a file into a buffer based on this file format:

H,1,2,3,4,5,6,7,8,9,0
D,1,2,3,4,5
T,3

H,1,2,3,4,5,6,7,8,9,0
D,1,2,3,4,5
T,3

...more records...
where it can also be:
H,1,2,3,4,
5,6,7,8,9,0
D,1,2
,3,4,5
T,3

where the number of fields is FIXED..(ie. H has 10 data fields, D has 5, T has 1)

My function reads one "record" into a buffer:

iErr = DoGetBuffer(szFile,SZ_NOFIELD1, szBuffer1); /* for H */
iErr = DoGetBuffer(szFile,SZ_NOFIELD2, szBuffer2); /* for D */
iErr = DoGetBuffer(szFile,SZ_NOFIELD3, szBuffer3); /* for T */

Note that I use the no of commas as an indicator. This function is very important and there cannot be any bugs in it... can u guys please go through it and comment/improve upon it so that it is "perfect" for production use?

Thanzs!

--------------------------------------
int CMyClass::DoGetBuffer(CFile * myFile, int NoFields, char *buffer)
{
      int iComma;
      char szElement;
      DWORD dwRead;
      int iLine;
      // initialize
      iLine = 0;
      iComma = 0;
      szElement = NULL;

    do
    {
        dwRead =  myFile->Read(&szElement, 1);
            
            // check if end of record
            if ( \
                  ((szElement == '\r')&&(iComma == (NoFields - 1)))|| \
                  ((dwRead  <= 0)&&(iComma == (NoFields - 1))) \
               )      
            {      // should check for record_type in the next line
                        buffer[iLine] = '\0';
                        return 0;
            }

            // ignore newline and cr
            if ((szElement != '\n')&&(szElement != '\r'))
            {
            buffer[iLine] = szElement;
            if (buffer[iLine] == ',')  // one comma add
                  iComma++;
            iLine++;
            };
            

    }
    while (dwRead > 0);
      // dwRead returned <= 0
      return -1;

}
----------------------------------
0
Comment
Question by:Haho
3 Comments
 
LVL 9

Expert Comment

by:ShaunWilde
ID: 2663699
you may want to add some exception handling as CFile::Read(...) can throw a CFileException .

You may also wish to put in some tests on buffer i.e. make sure it is not NULL and that you do not exceed its bounds (you may have to pass in the size of the buffer)
0
 

Accepted Solution

by:
wasan earned 300 total points
ID: 2664956
Hi,

Make sure that you have only one return
statement in your function.

You can make the following change to do it.

int iRc = -1;


    do
    {
        dwRead =  myFile->Read(&szElement, 1);

// check if end of record
if ( \
((szElement == '\r')&&(iComma == (NoFields - 1)))|| \
((dwRead  <= 0)&&(iComma == (NoFields - 1))) \
   )
{ // should check for record_type in the next line
buffer[iLine] = '\0';
iRc = 0;  // changed by wasan
}
//follwing two lines added by wasan
if ( iRc == 0 )
break;
// ignore newline and cr
if ((szElement != '\n')&&(szElement != '\r'))
{
buffer[iLine] = szElement;
if (buffer[iLine] == ',')  // one comma add
iComma++;
iLine++;
};

    }
    while (dwRead > 0);
// dwRead returned <= 0
return iRc; //changed by wasan

bye,
wasan.

   
0
 
LVL 2

Expert Comment

by:DKostov
ID: 2665520
Why you don't make "char* buffer" input
parameter "std::string &buffer" or with MFC-"CString &buffer"? In this
case you'll never exceed the buffer's memory limits.
0

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Introduction: Dynamic window placements and drawing on a form, simple usage of windows registry as a storage place for information. Continuing from the first article about sudoku.  There we have designed the application and put a lot of user int…
Ready to get certified? Check out some courses that help you prepare for third-party exams.
This video will show you how to get GIT to work in Eclipse.   It will walk you through how to install the EGit plugin in eclipse and how to checkout an existing repository.
When cloud platforms entered the scene, users and companies jumped on board to take advantage of the many benefits, like the ability to work and connect with company information from various locations. What many didn't foresee was the increased risk…
Suggested Courses

571 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question