Qbasicnews.com

Full Version: C++ comments/advice on overall structure
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Hi guys...as an exercise, I've written what I believe is a pretty strong PseudoRandomNumberGenerator in c++. I've implemented it in a symmetric file encrypter. The files are in J_Crypt.zip:
http://home.bellsouth.net/p/PWP-brightwave

j_crypt.cpp is the file to be compiled.

To use, drop the file to be (un)encrypted onto the executable and follow the menus)...the original file will *not* be modified.

Anyway...I was wanting feedback not on the encryption or hashing algorithms, but on my use of header files, standard c++, and general organizational issues(from a c++ progger perspective).

Thanks in advance
Okay, first off Im a C programmer not a C++ programmer, but heres are few pointers ;-)

You shouldn't have header files including source files, the correct way to compile a multi source file project is to compile each of the source files individually and then link them together to create a single executable. They way to do this varies based on the compiler/IDE used but an example with gcc is:
Code:
$ gcc file1.c file2.c file3.c -o exuctable_file

You shouldn't have functions inside a header file, you should only define functions in headers for example:
Code:
/* File.c */
#include "file.h"

int func(int a) {
  /* Do stuff */
}

/* File.h */
int func(int);
If you have a function body in a header file and the header is included by more than one source file you will get errors because the function will be defined twice. If you need to put something in a header that may get defined twice you can enclose it in an ifdef block, for example:
Code:
#ifndef MYTYPE
#define MYTYPE
typedef struct {
  int a;
  int b;
} myType_t;
#endif /* MYTYPE */
Then if the MYTYPE token is already defined then myType_t won't get redefined.

As for style, C and C++ let you put whitespace just about anywhere you want, some of your code is a little cluttered (IMHO) for example:
Code:
void Slicer::bounce(int steps){
   for(int z = 0; z < steps; ++z){      
      a_dup[0] = ((a[a_size-1] << 1)|(a[a_size-1] >> (bits-1))) ^ (a[0] | a[1]);        // Roll last row to align with first row.
      for(int i = 1; i < a_size-1; ++i)       // Starts with a[1]...a[0] has already been done.
         a_dup[i] = a[i-1] ^ (a[i] | a[i+1]); // a slight optimization could be had by incrementing i in the lvalue (a_dup[i++] and leaving the increment blank in the for loop...not done for clarity, and because speed-gain would be nominal.  
      a_dup[a_size - 1] = a[a_size-2] ^ (a[a_size-1] | (a[0] >> 1)|(a[0] << (bits-1)));   // Roll 1st row to align with last row.
      swap(a, a_dup);
      toggler = !toggler;           // if toggler is set to 0 upon initialization, the first bit of data will be offset.
   }
   for (int i = offset * toggler; i < a_size; i += spacer)  //update key.  (offset * toggler) toggles between 0 and offset.
      key[i/spacer] = a[i];  // (i/spacer) increments by one because 'i' increments as i += spacer.
  
   keypos = 0;   // Resets to 0 after each update.
}
Is much harder to read than:
Code:
void Slicer::bounce(int steps) {
for(int z = 0; z < steps; ++z) {      
   // Roll last row to align with first row.
   a_dup[0] = ((a[a_size-1] << 1) |
               (a[a_size-1] >> (bits-1))) ^
               (a[0] | a[1]);        

    // Starts with a[1]...a[0] has already been done.
    for(int i = 1; i < a_size-1; ++i) {
      /*
       * a slight optimization could be had by
       * incrementing i in the lvalue (a_dup[i++]
       * and leaving the increment blank in the for
       * loop...not done for clarity, and because
       * speed-gain would be nominal.
       */
      a_dup[i] = a[i-1] ^ (a[i] | a[i+1]);
    }
  
    // Roll 1st row to align with last row.
    a_dup[a_size - 1] = a[a_size-2] ^
                        (a[a_size-1] |
                        (a[0] >> 1) |
                        (a[0] << (bits-1)));

    swap(a, a_dup);

    /*
     * if toggler is set to 0 upon initialization,
     * the first bit of data will be offset.
     */
    toggler = !toggler;
  }
  
  //update key.  (offset * toggler) toggles between 0 and offset.
  for(int i = offset * toggler; i < a_size; i += spacer) {
    /*
     * (i/spacer) increments by one because 'i'
     * increments as i += spacer.
     */
    key[i/spacer] = a[i];    
  }
  keypos = 0;   // Resets to 0 after each update.
}

Other than those few things, nice work.
Quote:Okay, first off Im a C programmer not a C++ programmer, but heres are few pointers ;-)

You shouldn't have header files including source files, the correct way to compile a multi source file project is to compile each of the source files individually and then link them together to create a single executable. They way to do this varies based on the compiler/IDE used but an example with gcc is:
Code:
$ gcc file1.c file2.c file3.c -o exuctable_file
LC...first, thanks for taking the time. I knew I wasn't quite using header files correctly, but didn't get how it should be. My compiler is the gcc-based IDE dev-c++. As an IDE, the commands sent to the compiler are hidden from me. What exactly do you mean "compiled individually"? What is the extension of these "individually compiled" units? What I'm doing now is working with the source, and then, when I compile the "main-containing" file, it has all the rest of the code #included. Are you suggesting that the #included files should be some binary format, and not the ascii source code?
Quote:You shouldn't have functions inside a header file, you should only define functions in headers for example:
Code:
/* File.c */
#include "file.h"

int func(int a) {
  /* Do stuff */
}

/* File.h */
int func(int);
I see...so I've been doing this backwards...I should include *.cpp files from main, then have the *.cpp include files contain the *.h declarations. Does this mean that the main file that is compiled should look like
Code:
//main.h
#include <iostream>
#include <fstream>

using namespace std;

#include "myhelperfunctions.cpp"
#include "myclass1.cpp"
#include "myclass2.cpp"
#include "main.cpp"
//end of main.h
rather than:
Code:
//main.cpp
#include <iostream>
#include <fstream>

using namespace std;
#include "main.h"

int main(){
  //main code...
  return 0;
}
Do I understand you correctly?

Quote:
If you have a function body in a header file and the header is included by more than one source file you will get errors because the function will be defined twice. If you need to put something in a header that may get defined twice you can enclose it in an ifdef block, for example:
Code:
#ifndef MYTYPE
#define MYTYPE
typedef struct {
  int a;
  int b;
} myType_t;
#endif /* MYTYPE */
Then if the MYTYPE token is already defined then myType_t won't get redefined.

When I do it this way, do I call "MYTYPE", or do I call "myType_t"?? If it is the latter, what is the purpose of "MYTYPE"? Each time I use the
Code:
#ifndef MYTYPE
#define MYTYPE
typedef struct {
} myType_t;
#endif /* MYTYPE */
idiom, does the label "MYTYPT" need to have a uniqe name, or does "myType_t" need the unique name?

Thanks for the clarity hints...I agree that your modifications are much more readable.

I really appreciate your help,

Joe
I updated the files with the prefered header-file format...although I left out the
#ifndef
until I understand it better.
thanks
Making an executable from a number of source files involves a number of steps, usually each of the steps is hidden from you by the compiler. To break it into two major steps we first have compiling, this takes a single source file and produces an object file. Object files usually have a .o or .obj extension. Several object files can then be linked together to produce an executable. Your ide probably has some sort of support for projects or makefiles that you will need to look at to work out how to compile programs with more than one source file.

When you have code like:
Code:
#include "header.h"
When the pre-processor is run (before any compiling takes place) the contents of the file header.h is substituted in place of the above line. Header files are used for defining functions and constants that may be used in multiple files, for example:

Code:
/* error.h */

#define ERROR_MEM
#define ERROR_STACK

void printError(int);
Code:
/* main.c */

#include <stdio.h>
#include "error.h"

int errno;

int main(int argc, char **argv) {
  while(1) {
    /* Stuff */
  }

  /* Got here, an error occured, code in errno */
  printError(errno);
  exit(-1);
}
Code:
/* error.c */
#include <stdio.h>
#include "error.h"

void printError(int errCode) {
  switch(errCode) {
  case ERROR_MEM:
    fprintf(stderr, "Error: Out of memory\n");
    break;
  case ERROR_STACK:
    fprintf(stderr, "Error: Out of stack space\n");
    break;
  default:
    fprintf(stderr, "Error: Unknown error\n");
    break;
  }
}

The two source files, main.c and error.c both need the declaration for the function printError, instead of defining it in both source files, we define it in one place (error.h) and then included it in both source files. When we want to change something, it only needs to be changed in one place.

Quote:I see...so I've been doing this backwards...I should include *.cpp files from main, then have the *.cpp include files contain the *.h declarations.
No, you shouldn't include .cpp files (unless you are doing something weird). Each .cpp file should include the header files that contain any declarations or constants it requires.
Quote:No, you shouldn't include .cpp files (unless you are doing something weird). Each .cpp file should include the header files that contain any declarations or constants it requires.

Indeed, I was doing something weird...I was lumping all my code into one file by #including all the *.cpp code into one file, then compiling that all at once.

If you have a minute, will you see if the revisions I've made to the way the prog uses header files makes more sense?

The files are located in J_Crypt1003.zip at:

http://home.bellsouth.net/p/PWP-brightwave

int main(){}

is located in

j_crypt.cpp

Thanks