Skip to main content
Fixing a missunderstanding of mine
Source Link
Gilles
  • 158
  • 2
  • 8

The code looks nice, but is see a few issues in it which I'll try to list here:

  • This is supposedly a header file. However, you've included in here both declarations and definitions of your functions and structures. Normally, only the structures and the declarations of the functions should go into the header file, while the definitions of the functions should go into a source file (a .c). The way it is done at the moment, if you try to include your file into two different compilations units, you will have a problem at link time for multiple definitions of the functions...
  • Normally, you define first your structures and typedef them after (or at the same time) but not before. Actually, I'm not sure the way you did it is compliant with the C standard (I could e wrong here).
  • You have a potential major issue in the code for with the way you allocate your structures: by using a static variable into the "constructors" and returning a pointer to it, you actually reuse again again and againtake the same address from one callrisk of storing references to the nextsame structure over and over again. This means that allcould lead to tricky bugs and memory leaks. Fortunately, since in the calling side you neurons are actuallydereference the same onepointer returned immediately, and that you loose the references to all the allocated memories butactually store a copy of the last ones..structure. But then, why returning pointers in the first place? Just allocateuse a new neuron per calllocal structure for result and return a pointer to it (idemby value. Complexity-wise, this is the same for all the other structures)code, but this is safer and cleaner.
  • Performance-wise, it is hard to tell anything. Nothing strikes me as problematic, but the good way of doing is to profile your code and possibly optimize the hottest parts when identified.

The code looks nice, but is see a few issues in it which I'll try to list here:

  • This is supposedly a header file. However, you've included in here both declarations and definitions of your functions and structures. Normally, only the structures and the declarations of the functions should go into the header file, while the definitions of the functions should go into a source file (a .c). The way it is done at the moment, if you try to include your file into two different compilations units, you will have a problem at link time for multiple definitions of the functions...
  • Normally, you define first your structures and typedef them after (or at the same time) but not before. Actually, I'm not sure the way you did it is compliant with the C standard (I could e wrong here).
  • You have a major issue in the code for with the way you allocate your structures: by using a static variable into the "constructors" and returning a pointer to it, you actually reuse again again and again the same address from one call to the next. This means that all you neurons are actually the same one, and that you loose the references to all the allocated memories but the last ones... Just allocate a new neuron per call and return a pointer to it (idem for all the other structures).
  • Performance-wise, it is hard to tell anything. Nothing strikes me as problematic, but the good way of doing is to profile your code and possibly optimize the hottest parts when identified.

The code looks nice, but is see a few issues in it which I'll try to list here:

  • This is supposedly a header file. However, you've included in here both declarations and definitions of your functions and structures. Normally, only the structures and the declarations of the functions should go into the header file, while the definitions of the functions should go into a source file (a .c). The way it is done at the moment, if you try to include your file into two different compilations units, you will have a problem at link time for multiple definitions of the functions...
  • Normally, you define first your structures and typedef them after (or at the same time) but not before. Actually, I'm not sure the way you did it is compliant with the C standard (I could e wrong here).
  • You have a potential major issue in the code with the way you allocate your structures: by using a static variable into the "constructors" and returning a pointer to it, you take the risk of storing references to the same structure over and over again. This could lead to tricky bugs and memory leaks. Fortunately, since in the calling side you dereference the pointer returned immediately, you actually store a copy of the structure. But then, why returning pointers in the first place? Just use a local structure for result and return it by value. Complexity-wise, this is the same for the code, but this is safer and cleaner.
  • Performance-wise, it is hard to tell anything. Nothing strikes me as problematic, but the good way of doing is to profile your code and possibly optimize the hottest parts when identified.
Source Link
Gilles
  • 158
  • 2
  • 8

The code looks nice, but is see a few issues in it which I'll try to list here:

  • This is supposedly a header file. However, you've included in here both declarations and definitions of your functions and structures. Normally, only the structures and the declarations of the functions should go into the header file, while the definitions of the functions should go into a source file (a .c). The way it is done at the moment, if you try to include your file into two different compilations units, you will have a problem at link time for multiple definitions of the functions...
  • Normally, you define first your structures and typedef them after (or at the same time) but not before. Actually, I'm not sure the way you did it is compliant with the C standard (I could e wrong here).
  • You have a major issue in the code for with the way you allocate your structures: by using a static variable into the "constructors" and returning a pointer to it, you actually reuse again again and again the same address from one call to the next. This means that all you neurons are actually the same one, and that you loose the references to all the allocated memories but the last ones... Just allocate a new neuron per call and return a pointer to it (idem for all the other structures).
  • Performance-wise, it is hard to tell anything. Nothing strikes me as problematic, but the good way of doing is to profile your code and possibly optimize the hottest parts when identified.