Want to win a PS4? Go Premium and enter to win our High-Tech Treats giveaway. Enter to Win

x
?
Solved

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

Posted on 2000-03-27
3
Medium Priority
?
235 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
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

Ask an Anonymous Question!

Don't feel intimidated by what you don't know. Ask your question anonymously. It's easy! Learn more and upgrade.

Question has a verified solution.

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

Introduction: Finishing the grid – keyboard support for arrow keys to manoeuvre, entering the numbers.  The PreTranslateMessage function is to be used to intercept and respond to keyboard events. Continuing from the fourth article about sudoku. …
Introduction: The undo support, implementing a stack. Continuing from the eigth article about sudoku.   We need a mechanism to keep track of the digits entered so as to implement an undo mechanism.  This should be a ‘Last In First Out’ collec…
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.
How to fix incompatible JVM issue while installing Eclipse While installing Eclipse in windows, got one error like above and unable to proceed with the installation. This video describes how to successfully install Eclipse. How to solve incompa…

604 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