Subj : Re: Newb question on style and choices
To   : borland.public.cpp.borlandcpp
From : "Ed Mulroy [TeamB]" <dont_email_me@bitbuc.ket>
Date : Sun Nov 30 2003 11:58 am

Any response to your question is going to include opinions on technique and
opinions vary among people so consider that when reading my reply.

Consider changing your method of formatting the code.  Formatting is to make
things stand out - be more understandable.  For instance, neither of the
class member functions Validate and ChangePassword return a value in all
cases but it is difficult to see that in the code you posted.

I suggest that the end of an 'if' statement be a newline and the target of
an 'if' statement be indented.  If a target is a block (code enclosed in
curly braces) even if only one statement, then all one-statement targets
should be in blocks.

It helps clarity to be consistent in formatting.  If 'return 0;' is used why
is 'return (-1);' instead of 'return -1;' used?   People often use
parentheses in a return statement when what is being returned is an
expression, although some always used them.  Consistency throughout the code
is what is important.

Note how in the second version of this function below the fact that there is
no return value for string lengths < 5 or > 8 is clear but in the first
version it is not clear.  (also-why are return 0; and 'return(-1);'
followed by comments saying 'return zero' and 'return -1' ?)

---------------
int Password::Validate(char *pass)        // validator
{
  if(strlen(pass) >= 5 && strlen(pass) <= 8 )    // check for correct length
  if( isalpha(pass[0] ))            // check if first char is alpha
  {  return 0 ; }                       // return zero if passed
  else return(-1) ;                   // return -1 if failed
}

---------------

---------------
int Password::Validate(char *pass)        // validator
{
  if(strlen(pass) >= 5 && strlen(pass) <= 8 )    // check for correct length
  {
      if( isalpha(pass[0] ))         // check if first char is alpha
      {  return 0 ; }                // return zero if passed
      else
      {  return -1 ;  }             // return -1 if failed
  }
}

---------------

The member function ChangePassword shares this problem.

When you allocate a data item the memory for the item is used along with
whatever memory is needed for the memory manager table entry for that item.
Dynamicly allocating a 9 byte item probably takes much more memory for the
allocation call and memory usage than just defining the array to have 9
characters.

This gets to a technique called RAII or Resource Allocation Is
Initialization.  The idea is that allocation of a class instance carries
with it the allocation of the members.  If an exception is thrown on the one
allocation then all items associated with the class are cleaned up and no
extra deallocation is required.

You could do this:

class Password
{
     :
  char Password[9];
  char Password1[9];
  char Password2[9];
  char Password3[9];
  char Newp[50];
     :
};


Note that each data item is on its own line.  The file may be bigger but
when you are in a debug session looking for what may be wrong, none of the
data items are overlooked by being buried in the middle of a long,
multi-element line.

In general it has been useful to make data items private in a class and then
provide functions to access or change them.  There are many reasons but some
of the simpler ones are that you can set a breakpoint to trap when a data
item is being changed or accessed and you can introduce code to alter what
the data in the item is (for instance, limiting the number of characters
copied to the number the data element can hold independent of how many
characters the user typed in).

Over-commenting clutters code.  Labeling a char* declaration with the
comment '// data item' probably does not help.

..  Ed

"Base8" <Base8@base8.com> wrote in message
news:3fc928dc$1@newsgroups.borland.com...
> ...
> I'm using BC++ 4.0 and I'm looking for some feedback on style
> and the choices I've made in the class included below.  Please
> comment on anything you'd like that helps me advance in my
> understanding of C++ (I haven't had any classes or courses on
> it).  What I'm trying to do here is create a tiny executable that
> will simply act as a passthrough to an MVS system to which
> the user only has text mode access (CLI) so they don't have to
> delve into DOS.  This class will check the information the user
> entered against the requirements and concatenate the passwords
> as required for changing.  Is this too klugy for this simple task?
> Am I on the right track? Are there obvious choices I should have
> made, but didn't?  All constructive replies appreciated.

.