HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Harmonic partial sum calculator with multithreading

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
harmonicwithpartialsummultithreadingcalculator

Problem

I wrote a program which computes the harmonic partial sum to N terms with multithreading capability. I've been working on this to sharpen my C++ skills for the upcoming semester. Just wondering if there's anything that doesn't follow the C++ standard and/or glaringly wrong. Also wondering if I can make any further optimizations.

Basically I have my program set up to only use threads if the input is divisible by 4, since I am using 4 threads due to my quad core CPU. I want this to be more flexible but I haven't thought of a better solution yet. If anyone has a suggestions, that'd be appreciated.

#include 
#include 
#include 
#include 
#include 

using namespace std;

const int NUM_THREADS = 4;

// H(N) is defined to be the harmonic partial sum to N terms.
// calculateHarmonic(): function given to threads to compute H(N).
// unsigned long start: starting point for the loop; different for each thread.
// unsigned long N: amount of terms to compute; different for each thread; divided evenly by # of threads.
// double *sum: pointer to a double which holds a thread's individual sum for H(N).
void calculateHarmonic(long long start, long long N, double *sum)
{
    for(; start > N;
    assert(N > 0 && "N must be a nonzero positive integer.");
    cout  NUM_THREADS && N % 4 == 0)
    {
        cout  NUM_THREADS && N % 4 != 0)
    {
        cout << "Calculating without threads..." << endl;
        executeNoThreads(N);
    }
    else
    {
        cout << "Calculating without threads due to small N..." << endl;
        executeNoThreads(N);
    }

    return 0;
}


Compiled with:

g++ -std=c++11 -pthread -Ofast harm_sum.cpp -o harm.out ; ./harm.out

Solution

-
Try not to use using namespace std in global scope. If you must use it, keep it in local scope, such as in a function. More info about that here.

-
I don't need any need to use printf() over std::cout here. You are also using C-style casting and no std:: for clock_t, so some of this code still looks C-like.

If you're using printf() because you're having trouble with formatting, then take a look at the ` library for something useful.

-
Unless you're debugging,
assert may be unnecessary. You can easily output an error message to std::cerr and return EXIT_FAILURE from main().

-
The value
4 used in main() appears to be a magic number and should be a constant.

-
Each use of
std::endl flushes the buffer and takes extra time. Luckily, you're not using it in the timed section. It's still not needed in most cases and you can also output a newline with "\n"`, which doesn't do a flush as well. More info about that here.

Context

StackExchange Code Review Q#77898, answer score: 5

Revisions (0)

No revisions yet.