#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>
#include <random>
#include <map>
#include <math.h>
#include <cstring>
using namespace std;

class MathClass {
private:
    size_t current_capacity;
    double* logfact;
    bool inited = false;
    
    MathClass() {
        current_capacity = 0;
        logfact = new double[1];
        logfact[0] = 0;
    }
    
    void calculateLogFact(int n) {
        if (current_capacity >= n) return;
        double* newLogfact = new double[n 1];
        for (int i=0; i<=current_capacity; i  ) newLogfact[i] = logfact[i];
        for (int i=current_capacity 1; i<=n; i  ) newLogfact[i] = newLogfact[i-1]   log(double(i));        
        delete[] logfact;
        logfact = newLogfact;
    }
    
    double factorial(int n) {
        cout << "n = " << n << "\n";
        calculateLogFact(n);
        for (int i=0; i<=n; i  ) cout << int64_t(round(exp(logfact[i]))) << " ";
        cout << "\n";
        return exp(logfact[n]);
    }
    
public:

    static double factorial2n(int n) {
        static MathClass singleton;
        return singleton.factorial(2*n);
    }
};

int main(int argc, char** argv)
{    
    cout << MathClass::factorial2n(10) << "\n";
    return 0;
}

My library need to use an expensive function that needs to be initialized once before use (to pre-calculate some expensive values so that we don't have to calculate them every time). Currently, I use the singleton method above for this.

However, there are 2 problems:

1. Multi-threading: this will cause race conditions if 2 different threads call this function.
2. People don't like singleton
3. Other problems that I'm not aware of

What other design can I use to solve this problem? Pre-computing values is a must since this function needs to be fast.

CodePudding user response：

I agree with comments: Why hide the fact that MathClass caches results from the user? I, as a potential user, see no real benefit, rather potential confusion. If I want to reuse previously cached results stored in an instance I can do that. You need not wrap the whole class in a singleton for me to enable that. Also there is no need to manually manage a dynamic array when you can use std::vector.

In short: The alternative to using a singleton is to not use a singleton.

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <algorithm>
#include <random>
#include <map>
#include <math.h>
#include <cstring>
using namespace std;

class MathClass {
private:
    size_t current_capacity;
    std::vector<double> logfact;
    bool inited = false;
    
    void calculateLogFact(int n) {
        if (logfact.size() >= n) return;
        auto old_size= logfact.size();
        logfact.resize(n);
        for (int i=old_size; i<n; i  ) logfact.push_back(logfact.back()   log(double(i)));        
    }
    
    double factorial(int n) {
        cout << "n = " << n << "\n";
        calculateLogFact(n);
        for (int i=0; i<=n; i  ) cout << int64_t(round(exp(logfact[i]))) << " ";
        cout << "\n";
        return exp(logfact[n]);
    }
    
public:
    MathClass() {
        logfact.push_back(0);
    }
    double factorial2n(int n) {
       return factorial(2*n);
    }
};

void foo(MathClass& mc) { // some function using previously calculated results
    std::cout << mc.factorial2n(2);
}

int main(int argc, char** argv)
{    
    MathClass mc;
    cout << mc.factorial2n(10) << "\n";
    foo(mc);
}

I am not sure if the maths is correct, I didn't bother to check. Also inited and most of the includes seem to be unused.

Concerning "Multi-threading: this will cause race conditions if 2 different threads call this function." I would also not bother too much to bake the thread-safety into the type itself. When I want to use it single-threaded I do not need thread-safety, and I don't want to pay for it. When I want to use it multi-threaded, I can do that by using my own std::mutex to protect access to the mc instance.

PS: Frankly, I think the whole issue is caused by a misconception. Your MathClass is not a "function only" class. It is a class with state and member functions, just like any other class too. The "misconception" is to hide the state from the user and pretend that there is no state when in fact there is state. When using this class I would want to be in conctrol what results I can query because they are already cached and which results need to be computed first. In other words, I would provide more access to the class state, rather than less.