Implementing a Neural Network

As a pet project I implemented a neural network. I am looking for general advice, since I am a self tought programmer, but I have few specific questions I stated at the end of this post. Let's dig into it:

Code

Neuron.hpp:

//------------------------------------------------
//  NEURON.HPP
//------------------------------------------------

#ifndef NEURON_HPP
#define NEURON_HPP

// Libraries
#include 
#include 

// My Libraries
#include "fact.hpp"

// Typedefs and Structures

class neuron;

struct origin
{
    neuron& linked_neuron;
    double w;

    origin(neuron& ln, double win) : linked_neuron(ln), w(win) {};
};

struct target
{
    neuron& linked_neuron;
    double& w;

    target(neuron& ln, double& win) : linked_neuron(ln), w(win) {};
};

// Neuron : A single neutron in the network

class neuron
{
    private:
        double _act;
        double _input;
        double _delta;

        std::function _fact;
        std::function _dfact;

        std::vector _origins;
        std::vector _targets;

    public:
        double out;

        neuron(double (*fact)(double) = essential::fact_linear,
                double (*dfact)(double) = essential::dfact_linear )
            : _act(0), _input(0), _delta(0), _fact(fact), _dfact(dfact), out(0){};
        neuron& operator= (const neuron& rhs);

        void activate(void);

        void calculate_delta(void);
        void calculate_delta(double diff_to_example);

        void train_connection(double learning_rate);

        friend void link_neurons(neuron& predecessor, neuron& successor, double rate);
};

#endif

Neuron.cpp:

```
//------------------------------------------------
// NEURON.CPP
//------------------------------------------------

#include "neuron.hpp"

// Libraries
#include "config.hpp"

// CODE

using namespace std;

neuron& neuron::operator= (const neuron& rhs)
{
if(this != &rhs)
{
for (auto& rhso: rhs._origins)
{
_origins.push_back(origin(rhso.linked_

It's a long question then I'm sure many others will find something else. Here just few thoughts about your code.

You're using include guards (#ifndef) but I feel more easy with #pragma once. Yes, it's not standard but it's supported by all major compilers and less error prone (also slightly faster but nothing we may/should care).

In origin, target and neuron you have public fields. I think you seldom need a public uncontrolled field in your C++ code (a method set*() may be a good beginning but more explicative names like reset() are even better).

You have many references to primitive types (for example double&). I do not know in your architecture what pointer size is but if it's bigger than sizeof(double) then you're probably wasting memory and in any case you may prevent some compiler optimizations (because of pointer aliases, standard do not mandate how references must be implemented.) Yes, you may use const but it's so weak that AFAIK it's happily ignored by any optimizer.

Why neuron constructor is declared with double (*fact)(double) instead of std::function? Well it may even be a const& (but I think it'll have worse performance and more complicate interactions with lambdas).

neuron has overloaded assignment operator but I do not see any copy constructor (what about move constructor?) You even have a std::vector. Many words have been printed (elsewhere) about this.

Do not ever declare using namespace std;.

std::vector> seems little bit too much for me, maybe it's time to use a specific type for that? Boost has a multidimensional array, alternatively you may use template using multivector = std::vector> (or something similar...)

unsigned int isn't better than simply unsigned.

When you move around a std::vector you may want to make it const&. Compiler may (at least for small programs and simple code) be smart enough but it's also about communicating intent.

network::_initialize_random_rates() is too complex. Partly because you didn't split it in multiple functions and partly because you're using an enum. Why don't you use enum class? Better: you have delegates for many things, why don't you also use a class for this? To drop delegates will simplify your code, distribute responsibilities and it will make it easier to extend. The same is true for network::train_epoch().

EDIT: what's this suggested refactoring? You already delegate some features to an external entity (rne, for example). Extend the same concept to replace enum: in my opinion they're invaluable (better if using enum class) for small tasks where they slightly change method behavior but they start to smell when used to switch between big blocks of code (in multiple places). In your case it may be like this (proof of concept):

class neural_network_type {
public:
    virtual void initialize(network& net) = 0;
};

You will then have a feedforward_network_type. Note that you may have a parameter/property to specify neural_network_type concrete class (direct replacement of enum) or it may be a template argument:

template 
class network {
private:
    TType brainStructure;
};

Note that I removed the public accessible field. Even if you opt for the other solution does it make it sense to have it as ctor argument? Will you ever change its value again? If ctor starts to have too many parameters you may want to introduce a design struct will all required arguments, something like this:

struct network_description {
    neural_network_type type;
    vector neurons_per_layer;
    rne random_engine
};

With this ctor (but you may also want to use a factory method):

network::network(const network_description& description) {
}

The very next step is to have different network derived classes. One of each type you want to support and a factory method:

class network {
public:
    static network* create(const network_description& description) {
        // ...
    }

protected:
    virtual void initialize_random_rates() = 0;
};

class feedforward_network : public network {
    // ...
};

One last thing that I didn't note at first: do not start identifiers with an underscore, they're reserved. Simply do not prefix private methods.

About conditional compilation: it may be used or not (as you said to include some features in compiled code) but note that:

• Compiler is smart enough to remove unused code (then you won't have any gain but faster compilation.)

• If you're building a library then you force your users to recompile the library or to keep multiple versions of it.

• Code in header files won't be compiled at all if they're not included.

I usually use conditional compilation only in the following cases (but of course there isn't a rule):

• I want to include extra code for debugging/tracing purposes (for example #ifdef DEBUG_ALLOCATION to include verbose logging about memory allocation.)

• I want to include extra code during develop

StackExchange Code Review Q#145280, answer score: 5