Repeat operation if signal is received I have a data synchronisation operation that is observing a database. This is a fairly key piece of code for the system, so I want to make sure I handle the concurrency correctly.

The problem

The operation manipulates sync state flags in the database and should
have zero concurrency
The operation must be cancellable
The operation should repeat an additional time if signals are received while it is running

Behavior

The service is started in response to changes to the underlying data
The service is started in response to changes in connectivity
The service is started periodically

Current design

    bool syncRequired = false;

    private async void StartSynchronisation()
    {
        // the cancellation token operates as the flag to indicate if the operation is running
        if (this.cancellationTokenSource != null)
        {
            syncRequired = true;
            return;
        }

        try
        {
            this.cancellationTokenSource = new System.Threading.CancellationTokenSource();

            // stop the time that periodically prompt the operation
            this.synchronisationTimer.Stop();

            bool dataWasSynchronised = false;

            // Repeated synchronisation and replication until no updates are made
            do
            {
                syncRequired = false;
                // Only attempt synchronisation if the service is actually available
                if (this.connectivityMonitor.IsServiceAvailable)
                {
                    dataWasSynchronised = await DoSynchronisation().ConfiguredAwait(false);
                }
            }
            while (dataWasSynchronised || syncRequired );
        }
        catch (Exception ex)
        {
            // handle exception using injected policy handler
        }
        finally
        {
            this.synchronisationTimer.Start();
            this.cancellationTokenSource.Dispose();
            this.cancellationTokenSource = null;
        }
    }

I bring you this code because it is missing any thread synchronisation and it probably needs it. It currently relies on the atomicity of read/writes to a reference (cancellationTokenSource) and bool (syncRequired) but this doesn't cover anything really. As a critical piece of code, I don't want to miss an event and need to wait for the next timer interval to start the operation. It will continue working, but our test department won't like it. My issue is, thread concurrency isn't my strongest area and I'm already burned out on this feature - I'm struggling to reason about it without tying my head in knots.

What synchronisation primitives do I need to make sure all events are handled as intended? Is there a more favourable pattern using concurrent queues or reactive extensions that I can throw in here? I have been considering some use of Reactive Extension's BehaviorSubject<T>.

My understanding:

The first if statement is meant to check whether a thread is already executing the loop by checking whether cancellationTokenSource is null; if it's not null, then signal that a sync is required by setting syncRequired to true and returning. The thread executing the loop will then run another loop due to the while (dataWasSynchronised || syncRequired ) condition.

If cancellationTokenSource is null, then the sync loop isn't being executed, so this thread goes on to do just that.

private async void StartSynchronisation()
{
    // the cancellation token operates as the flag to indicate if the operation is running
    if (this.cancellationTokenSource != null)
    {
        syncRequired = true;
        return;
    }

    try
    {
        this.cancellationTokenSource = new System.Threading.CancellationTokenSource();

So the first problem is that that check (the first if statement) isn't synchronised. One thread can see cancellationTokenSource is null and move on to execute the sync loop, first setting cancellationTokenSource to a new object. Before it assigns the new object - say, after setting syncRequired to true - there's a context switch, and another thread executes the same code. It too sees that cancellationTokenSource is null, sets syncRequired to true, assigns a new object to cancellationTokenSource, and starts running the loop. Then there's another context switch and our original thread is back again, assigns a new object to cancellationTokenSource, and starts executing the loop as well. Voila, you've now violated your first requirement of having zero concurrency.

The fix is simple: the check, as well as any objects being altered to affect the outcome of the check, must be synchronised.

A basic way to achieve this:

class MyClass
{
    private readonly object syncLock = new object();
    private volatile bool syncRequired = false;
    private volatile bool syncRunning = false;

    private void MySyncFunc()
    {
        lock (syncLock)
        {
            if (syncRunning)
            {
                syncRequired = true;
                return;
            }

            syncRunning = true;
        }

        while (true)
        {
            syncRequired = false;

            // your sync operations here

            lock (syncLock)
            {
                if (!syncRequired)
                {
                    syncRunning = false;
                    break;
                }
            }
        }
    }
}

I'll quickly point out this isn't the most efficient or pretty, but it's a simple (and hopefully accurate) way to show and do it.

First of all notice you can't even check whether your sync loop is running without getting the lock. So only one thread at a time can check. If a thread determines it isn't running, it sets the value to true (to signal that it's now running) and exits the lock. The second thread that arrived on the scene in the earlier example, if it was context-switched to during that time, would not have been able to proceed because it couldn't get the lock. Once the first thread exits the lock, the second thread gets the lock. Now it sees that syncRunning is true, so it sets syncRequired to true and returns. No sync problems there.

Now let's look at the loop. Nevermind that it's an infinite loop, I just wanted to get the checks out of the loop condition. So first thing it does is set syncRequired to false. This doesn't need to be synchronised because even if another thread had set it to true right before, you're gonna go and do the actual synchronisation right now anyway.

So after your own sync stuff completes, it's time to check whether you can exit the loop or need to repeat it. (I've left out checking the result of your sync operation as it's not relevant to this.) Again you got to get the lock. Why? Because the thread could check syncRequired and see it's false, and move on to set syncRunning = false. But before it's set, context switch, another thread checks and sees that syncRunning == true, so sets syncRequired to true and returns. Meanwhile the first thread continues to set syncRunning to false and returns as well. Now you have syncRequired == true but no thread running the loop. Whoops, you missed an event! Take a look at your original code and you'll see this problem is there, if syncRequired is set to true after the loop exits and before cancellationTokenSource is set to null in the finally block.

So what the code above does instead is it gets a lock on the same object again and checks whether a sync is required. If it is, fine, the lock is exited and the loop starts over. But if it's not, then it sets syncRunning to false before it exits the lock. Since any other thread that wants to check if syncRunning == true has to wait until the lock in the loop is exited, they will see the correct value when they finally do get the lock, because if the first thread will exit the loop, it will set syncRunning to false while it has the lock and before it exits the lock. Whether the next thread gets the lock and checks, it will see that syncRunning == false, and go on to run the loop itself. No more lost events!

Couple other issues:

If you're going to use a member variable in a loop, but not modify its value in the loop (or only ever set it to a single value on each iteration such that there is no apparent point in doing so, even though you know that some other thread can modify the value), then mark it volatile. This will result in the value being read from memory each time, so you pick up changes made from other threads. Otherwise the compiler could optimise the read out of the loop; after all, it's never changing during the lifetime of the loop.

And your code isn't cancellable. Sure you create a CancellationTokenSource, but you never actually use it - hence why I simply substituted it with a bool in my example. What you need to be doing is checking the token each iteration (or possibly pass it to the function being called if that's the part that should allow cancellation of its task, though you'd then have to alter the function to accept and know how to use the token) and seeing whether a cancellation has been requested; then if so, throw a OperationCanceledException (or however you want to abort what's currently running).


Square-tree using maps and recursion Define a procedure square-tree analogous to the
  square-list procedure of exercise
  2.21. That is, square-list should behave as follows:

(square-tree  (list 1
        (list 2 (list 3 4) 5)
        (list 6 7))) (1 (4 (9 16) 25) (36 49))

  Define square-tree both directly
  (i.e., without using any higher-order
  procedures) and also by using map and
  recursion.

I wrote this solution.  What do you think?

(define (square x) (* x x))

(define (square-tree tree)
  (cond ((null? tree) null)
        ((pair? tree)
         (cons (square-tree (car tree))
               (square-tree (cdr tree))))
        (else (square tree))))

(define (map-square-tree tree)
  (map (lambda (subtree)
         (if (pair? subtree)
             (cons (square-tree (car subtree))
                   (square-tree (cdr subtree)))
             (square subtree)))
       tree))

(define a (list 1 1 (list (list 2 3) 1 2)))

EDIT: This is a much better solution for map-square-tree.

(define (square x) (* x x))

(define (square-tree tree)
  (cond ((null? tree) null)
        ((pair? tree)
         (cons (square-tree (car tree))
               (square-tree (cdr tree))))
        (else (square tree))))

(define (map-square-tree tree)
  (map (lambda (subtree)
         ((if (pair? subtree) map-square-tree square) subtree))
       tree))

(define a (list 1 1 (list (list 2 3) 1 2)))

Your direct definition of square-tree is correct.

Your definition using map calls square-tree; to make it properly recursive, call map-square-tree instead.  Further, you may recurse on the subtree itself.  This will make your code succinct.

(define (map-square-tree tree)
  (map (lambda (subtree)
         ((if (pair? subtree) map-square-tree square) subtree))
       tree))


Removing elements in an array I have a piece of code where I compare two arrays for similar values, then remove the similar values from one of the arrays. Is there a better way of writing this piece of code?

UpdateAddedTasksAfterDelete() {
    var tasksToRemove = [];
    var updatedTasks = [];

    for (var i = 0; i < this.deletedTasks.length; i++) {
        if (this.addedTasks.indexOf(this.deletedTasks[i]) > -1) { tasksToRemove.push(this.deletedTasks[i]); }
    }

    for (var i = 0; i < this.addedTasks.length; i++) {
        if (this.addedTasks.indexOf(tasksToRemove[i]) == -1) { updatedTasks.push(this.addedTasks[i]); }
    }

    this.addedTasks = updatedTasks;
}

I'm a little unclear as to what your goal is here, but here are a few options, depending on which outcome you are hoping for...

If your goal is to take two arrays, combine them, and discard any duplicate values, using a Set will automatically discard the values that appear more than once for you.

Simply use:

var mySet = new Set(array1)
array2.forEach((item)=>{mySet.add(item)});

Then convert the set back to an array if you need it specifically in array form, using the spread operator:

var finalArray = [...mySet]

'finalArray' will now hold each unique value from the combination of your two original arrays.

If your goal is to compare two arrays, removing values from the first array that are present in the second, use filter:

var finalArray = array1.filter((item)=>{return !array2.includes(item)})

'finalArray' will now hold all values of array1 that do not also exist in array2.


Generic logging class for tracing the function calls #include<iostream>
#include<string>
using namespace std;

// Generic class for tracing the function call
class functionlogging {
private:
    string name;
    string in{ ">>" };
    string out{ "<<" };
public:
    functionlogging(string x) :name(x) {
        cout << in << name << endl;
    }
    ~functionlogging(){
        cout << out << name << endl;
    }
};

//uses
void generic_function() {
    functionlogging logger{ __func__ };
    int a = 10;
    int b = 100;
    a += b;
}

int main() {
    generic_function();
}

Many times, we do need to log when function call starts and ends for better understanding.  With modern C++ concepts like __funct__/RAII, I wrote the small class which can be used to achieve trace. With this I was able to achieve this for above "generic_function()".

Output

>>generic_function
<<generic_function

I wanted others opinion about the functionlogging class and how to make it in such a way that user of this class would have to do the minimal work/code changes in his/her code base.

At present user of this class needs to create a object of this class and pass the __func__ information. Is it acceptable or in a way can it be embed inside something by which user of the class almost do not require to do anything?.

Stop doing this:

using namespace std;

see: Why is “using namespace std;” considered bad practice?

These two are not unique to the class.

string in{ ">>" };
string out{ "<<" };

May as well make them static. Also they re never modified so make them const.

No need to force a flush with std::endl

    cout << in << name << endl;

Prefer "\n" when you don't need a flush (also cerr or clog may be better choices of output stream).

That seems like an overly verbose and error prone way of use.

void generic_function() {
    functionlogging logger{ __func__ };

You have to use up a variable and remember __func__. Can't we get a macro in here to do all the for you? I actually need to look up to see if func is a standard macro.

Just checked the standard:

8.4.1 In general
Paragrah 8:
The function-local predefined variable __func__ is defined as if a definition of the form

static const char __func__[] = "function-name ";

So I would define the macros:

#define LOG_ENTRY_EXIT_FOR(x)       functionlogging  SomeLongNameThatIsNotLikelyToBeUsedInTheFunctionlogger(x)
#define LOG_ENTRY_EXIT              LOG_ENTRY_EXIT_FOR(__func__)

Then usage is simply:

void generic_function() {
    LOG_ENTRY_EXIT;

    int a = 10;
    int b = 100;
    a += b;
}


Listing all primes up to a given limit I started writing Rust by doing a simple program that will print all prime numbers until it reaches the upper bound that is provided through standard input. Here is my code

use std::io;

fn main() {
    println!("Enter a number: ");
    let mut limit = String::new();
    io::stdin().read_line(&mut limit).expect("failed to read line");
    let limit: u64 = limit.trim().parse().expect("Please enter a number.");
    if limit <= 0 {
        println!("Number must be greater than or equal to 0.");
    } else {
        let mut is_prime = 0;
        for n in 2..limit+1 {
            if n < 4 {
                is_prime = 1;
            }
            if n % 2 == 0 && n != 2 {
                continue;
            }
            if n % 3 == 0 && n != 3 {
                continue;
            }
            is_prime = 1;
            let mut m = 5;
            while m <= (n as f64).sqrt() as u64 {
                if n % m == 0 {
                    is_prime = 0;
                    break;
                }
                if n % (m + 2) == 0 {
                    is_prime = 0;
                    break;
                }
                m += 6;
            }
            if is_prime == 1 {
                println!("{}", n);
            }
        }
    }
}

While the program does compile and run without any issues, there are two warnings that come up when I compile. This isn't the biggest concern of my program, but is merely just worth mentioning.

$ rustc primes.rs
primes.rs:11:13: 11:25 warning: value assigned to `is_prime` is never read, #[warn(unused_assignments)] on by default
primes.rs:11         let mut is_prime = 0;
                         ^~~~~~~~~~~~
primes.rs:14:17: 14:25 warning: value assigned to `is_prime` is never read, #[warn(unused_assignments)] on by default
primes.rs:14                 is_prime = 1;
                             ^~~~~~~~

primes.rs:11:13: 11:25 warning: value assigned to `is_prime` is never read, #[warn(unused_assignments)] on by default
primes.rs:11         let mut is_prime = 0;
                         ^~~~~~~~~~~~

You don't have to initialize a variable when you declare it. That is, you can write

let mut is_prime;

You just have to initialize it before reading from it; if the compiler cannot guarantee that the variable will be initialized at that point, it will raise an error.

primes.rs:14:17: 14:25 warning: value assigned to `is_prime` is never read, #[warn(unused_assignments)] on by default
primes.rs:14                 is_prime = 1;
                             ^~~~~~~~

That warning comes from this block:

if n < 4 {
    is_prime = 1;
}

This block is useless, because you are unconditionally setting is_prime to 1 after a couple of other if blocks, and you don't read back the variable between these assignments.

is_prime should be a boolean instead of an integer. Rust has a built-in boolean type, bool, and the keywords true and false, which you can use instead of 1 and 0.

Instead of having a mutable is_prime variable, I'd factor out the code that decides if a number is prime in a separate function. We can use early returns to avoid defining a mutable variable.

use std::io;

fn is_prime(n: u64) -> bool {
    if n < 4 {
        // Skip all the following checks, we know it's a prime!
        return true;
    }

    // We don't need to check if n is 2 or 3 anymore,
    // since if n is one of these values, we won't reach here.
    if n % 2 == 0 {
        return false;
    }

    if n % 3 == 0 {
        return false;
    }

    let mut m = 5;
    while m <= (n as f64).sqrt() as u64 {
        if n % m == 0 {
            return false;
        }

        if n % (m + 2) == 0 {
            return false;
        }

        m += 6;
    }

    // Implicit return
    true
}

fn main() {
    println!("Enter a number: ");
    let mut limit = String::new();
    io::stdin().read_line(&mut limit).expect("failed to read line");
    let limit: u64 = limit.trim().parse().expect("Please enter a number.");
    if limit <= 0 {
        println!("Number must be greater than or equal to 0.");
    } else {
        for n in 2..limit + 1 {
            if is_prime(n) {
                println!("{}", n);
            }
        }
    }
}


Argument parser for a word count and/or line count program How to make code dry in this example? My program is working, but I want to improve it by shorting my code.
.

parser = argparse.ArgumentParser()
parser.add_argument('--words', '-w', action='store_true')
parser.add_argument('--lines', '-l', action='store_true')
parser.add_argument('filename')
args = parser.parse_args()

if args.words:
    print "Words in a file: {}" .format(print_words(args.filename))

elif args.lines:
    print "Lines in a file: {}" .format(print_lines(args.filename))

else:
    print "Words in a file: {}" .format(print_words(args.filename))
    print "Lines in a file: {}" .format(print_lines(args.filename))

I guess by adding a new function and putting the function call in the else statement.

I tried with the print_all function, I added the function call in the else statement.

def print_all(filename, args):
    for a in read_file(filename):
        if a in args:
            print "All word counts :", a

When I run the program using the print_all function I get:

Namaspace(words=False, filename='someFile.txt', lines=False)

Do you notice something fishy on this line?

print "Words in a file: {}" .format(print_words(args.filename))

If I highlight just the fishy part:

print print_words(...)

The fishy part is printing something returned by print_words.
The name of print_words suggests that it prints something.
And printing functions typically don't return a value.
So this is all very confusing.

My guess is that print_words doesn't actually print words,
but returns a word count. So a name like get_word_count would be less confusing.
The same goes for print_lines.

With these suggested renames, now we can introduce some real printing functions, and make the code more DRY:

if args.words:
    print_words(args.filename)

elif args.lines:
    print_lines(args.filename)

else:
    print_words(args.filename)
    print_lines(args.filename)

It won't get much DRY-er than this, and I don't think it needs to.


Exceptions handling, what would you do? I know that there are several ways to handle exceptions and probably a good way is to keep the handling consistently and use the pattern which feels comfortable for himself, but I would like to get some advice or hints, what would you do in following situation or how I could improve such a case.

I created a custom URLClassLoader which checks the certification and some advanced stuff. With following function I load the X509 certificate into the instance:

public boolean loadX509Certificate(File file) {
    try {
        CertificateFactory cf = CertificateFactory.getInstance("X.509");
        InputStream is = new FileInputStream(file);
        this.setCertificate(cf.generateCertificate(is));
        is.close();
    } catch (CertificateException e) {
        return false;
    } catch (FileNotFoundException e) {
        return false;
    } catch (SecurityException e) {
        return false;
    } catch (IOException e) {
        return false;
    }
    return true;
}

In that function I catch four exceptions, what would you do in this situation? Return false if the certificate cannot load, rethrow the exception or create a custom exception which tells the user what went wrong?

In generally the Class will work without the certification, but if the value doesn't exist, the class will not check the classes and resources signature.

First, I would separate loading the certificate from storing it.

They are independent concerns.
It allows multiple ways to load a certificate.
It improves testability.

As Kinjal suggests, logging the error is a good alternative when failure is recoverable. However, I would throw a custom exception (or use CertificateException) and let the caller decide to log and continue or terminate. I really don't like returning success codes when I can avoid it because it muddies the code with checks.

Finally, instead of setting a certificate or leaving the field blank consider modeling a certificate checking strategy. This would require a simple one-method interface CertificateCheck with two implementations: NullCertificateCheck and BasicCertificateCheck. Again this improves testability by providing a seam for mocking, separating concerns, etc. It also removes the need for if (this.certificate != null) checks and allows for more strategies and certificate types.

public void setupCertificate() {
    try {
        certificateCheck = loadX590Certificate(...);
    }
    catch (InvalidCertificateException e) {
        LOG.warn("Skipping certificate check", e);
        certificateCheck = new NullCertificateCheck();
    }
}

private CertificateCheck loadX509Certificate(File file) throws CertificateException {
    InputStream is = null;
    try {
        CertificateFactory cf = CertificateFactory.getInstance("X.509");
        is = new FileInputStream(file);
        return new BasicCertificateCheck(cf.generateCertificate(is));
    }
    catch (FileNotFoundException e) {
        throw new CertificateException("Certificate file not found", e);
    }
    ... more exceptions ...
    finally {
        IOUtils.closeQuietly(is);
    }
}

Update: Since you're storing the certificate (or not) so you can check its validity, I assumed you were checking it at a later time and ideally multiple times. If this is the case, extract the logic into a CertificateCheck hierarchy using the Strategy pattern. If the certificate is checked once in isolation of any other data, this isn't necessary and I see no reason to store the certificate in an instance field.

  Note: I'm using Checkable as a stand-in for whatever contains the information that must be checked against the certificate. Your implementation would need to pass whatever is appropriate.

public interface CertificateCheck {
    boolean matches(Checkable thing);
}

// in the absence of a certificate, everything matches
public class NullCertificateCheck implements CertificateCheck {
    public boolean matches(Checkable thing) {
        return true;
    }
}

public class BasicCertificateCheck implements CertificateCheck {
    private final Certificate cert;

    public BasicCertificateCheck(Certificate cert) {
        this.cert = cert;
    }

    public boolean matches(Checkable thing) {
        // move the current checks you have here
    }
}


Bloom filter implementation in C++ This is a bloom filter implementation in C++. To instantiate the BloomFilter class, supply it with the number of bool cells, and a HashFunction vector. The method addElement() adds a string to the set of strings the bloom filter test element membership against. The method searchElement() checks set membership against the aforementioned set.

#include <iostream>
#include <string>
#include <vector>
#include <iterator>

// bloom filter implementation

typedef unsigned int (*HashFunction)(std::string);

class BloomFilter {
    unsigned int numberOfCells;
    unsigned int numberOfFunctions;
    bool* cell;
    std::vector<HashFunction> hashFunctions;

public:

    BloomFilter(unsigned int numbCells, std::vector<HashFunction> funcs) : numberOfCells(numbCells), hashFunctions(funcs) {
        cell = (bool*)calloc(numbCells, sizeof(bool));
    }

    void addElement(std::string str) {
        for (std::vector<HashFunction>::iterator iter = hashFunctions.begin(); iter != hashFunctions.end(); iter++) {
            cell[(*iter)(str) % numberOfCells] = true;
        }
    }

    bool searchElement(std::string str) {
        bool strInSet = true;

        for (std::vector<HashFunction>::iterator iter = hashFunctions.begin(); iter != hashFunctions.end(); iter++) {
            if (cell[(*iter)(str) % numberOfCells] == false) {
                strInSet = false;
                break;
            }
        }

        return strInSet;
    }

    ~BloomFilter() {
        free(cell);
        cell = NULL;
    }
};

// implementing a couple of hash functions for testing

unsigned int djb2(std::string str) {
    unsigned int hash = 5381;

    for (std::string::iterator iter = str.begin(); iter != str.end(); iter++) {
        hash = ((hash << 5) + hash) + *iter;
    }

    return hash;
}

unsigned int sdbm(std::string str) {
    unsigned int hash = 0;

    for (std::string::iterator iter = str.begin(); iter != str.end(); iter++) {
        hash = ((hash << 6) + (hash << 16) - hash) + *iter;
    }

    return hash;
}

int main() {
    // create bloom filter
    std::vector<HashFunction> hashFunctions;
    hashFunctions.push_back(djb2);
    hashFunctions.push_back(sdbm);

    BloomFilter bf(1024, hashFunctions);

    // insert words into the filter
    std::vector<std::string> setOfStrings({
        "Hello World!",
        "sweet potato",
        "C++",
        "alpha",
        "beta",
        "gamma"
    });

    std::cout << "Bloom filter created." << std::endl;
    std::cout << "Bloom filter tests for the following set of strings:" << std::endl;

    for (std::vector<std::string>::iterator iter = setOfStrings.begin(); iter != setOfStrings.end(); iter++) {
        bf.addElement(*iter);
        std::cout << "\t" + *iter << std::endl;
    }

    // testing a set of strings against the bloom filter
    std::vector<std::string> testSetOfStrings({
        "Hello World!",
        "sweet potato",
        "C++",
        "alpha",
        "beta",
        "gamma",
        "delta",
        "where am i?",
        "foo",
        "bar"
    });

    std::cout << "Testing set inclusion for the following strings:" << std::endl;
    std::cout << std::boolalpha;

    for (std::vector<std::string>::iterator iter = testSetOfStrings.begin(); iter != testSetOfStrings.end(); iter++) {
        std::cout << "\t" + *iter + ": " << bf.searchElement(*iter) << std::endl;
    }

    getchar();
}

Thanks. I have been trying to learn about bloom filters. Had to do some research before the review :-)

Code Review

Hash Function

This is an old C style typedef.

typedef unsigned int (*HashFunction)(std::string);

Though not wrong I prefer the C++ style as they are easier to read.

typedef std::function<unsigned int(std::string)>   HashFunction;

Or using the newer C++11 using directive (I am beginning to use more of this myself).

using HashFunction = std::function<unsigned int(std::string)>;

While we are on the hash function. I note that you are passing std::string as a value and thus inducing a copy of the string. To avoid the copy pass by const reference.

using HashFunction = std::function<unsigned int(std::string const&)>;

Members

Looking at the data members of BloomFilter.

    // This member is not used anywhere.
    // but should also be the same value as hashFunctions.size()
    // So I think we can remove this.
    unsigned int numberOfFunctions;

    // Here we have a pointer data member. This is usually a bad
    // idea. We want to encourage the rule of zero. So we want to
    // use classes that do their own memory management.
    bool* cell;
    // More about this in notes on the destructor.

So I would use:

    std::vector<bool>   cell;

Constructor

Just like normal variable declarations, prefer to initialize members one per line. There are no prizes for using the least number of rows possible.

Also like the hash function you are passing the parameters by value (and thus inducing a copy). Here we can pass funcs by const reference.

    BloomFilter(unsigned int numbCells, std::vector<HashFunction> funcs) : numberOfCells(numbCells), hashFunctions(funcs) {
        cell = (bool*)calloc(numbCells, sizeof(bool));
    }

    // Can be

    BloomFilter(unsigned int numbCells, std::vector<HashFunction> const& funcs)
        : numberOfCells(numbCells)
        , hashFunctions(funcs)
        , cell(numbCells)          // assuming we change cell to vector
    {}

As a side note:

        // Please don't use C-Casts.
        // These are completely unchecked. You are basically
        // telling the compiler to ignore all errors.
        cell = (bool*)calloc(numbCells, sizeof(bool));
        // Also there is no check to see if calloc() failed.

It should look more like this:

        cell = static_cast<bool*>(calloc(number, sizeof(bool));
        if (cell == null) {
            throw bad_alloc;  // maybe another exception.
        }

        // or better yet use new/delete
        cell = new bool[number]();      // need a call to delete [] cell; in destructor: The braces force value initialization that is false.

        // or better yet use a type like vector that does memory management correctly.
           // No action needed as you did it all in the initializer list.

addElement

Passing parameter by value again. Prefer to pass by const reference.

    void addElement(std::string str) {
    // So prefer
    void addElement(std::string const& str) {

When using iterators prefer to use the pre-increment ++iter rather than the post increment iter++. There is slight performance environment.

So you use the standard for loop from C++03

        for (std::vector<HashFunction>::iterator iter = hashFunctions.begin(); iter != hashFunctions.end(); iter++) {
            cell[(*iter)(str) % numberOfCells] = true;
        }

We can improve this in C++11 using auto.

        for (auto iter = hashFunctions.begin(); iter != hashFunctions.end(); ++iter) {
            cell[(*iter)(str) % numberOfCells] = true;
        }

In C++14 we now have the range based for loop.

        for (auto const& item: hashFunctions) {
            cell[item(str) % numberOfCells] = true;
        }

searchElement

Passing parameter by value again. Prefer to pass by const reference.

    bool searchElement(std::string str) {
    // should be
    bool searchElement(std::string const & str) {

The first note I want to make is the name. A searchX() function usually return a reference to something. I would name it checkX().

In this case rather than using a for loop you can use one of the standard function. In this case we can use find(). You are looking for a failure (but that's a thing).

    auto f = std::find(std::begin(hashFunctions), std::end(hashFunctions),
                       [&cell, &str](HashFunction hash){
                           return !cell[hash(str)];
                       });
    return f != std::end(hashFunctions);

Destructor

We should note that in C++11 we moved to using nullptr rather than NULL.  It is type safe.

    ~BloomFilter() {
        free(cell);
        cell = NULL;  // This is actually a bad idea.
                      // It hides errors that can be detected by the
                      // debug system.
    }

One of the issues you can get into with doing your own memory management is called the double free. Usually debug version of the standard library detects double free so you can catch it during testing. But by setting the value to nullptr in the destructor prevents that check.

The other thing you need to note is that by default the compiler will generate four methods for a class if you don't define them.

  BloomFilter();                             // Constructor.
  BloomFilter(BloomFilter const&);           // Copy constructor.
  BloomFilter& operator=(BloomFilter const&);// Assignment operator
  ~BloomFilter();                            // Destrutor.

You define the Constructor and Destructor but the other two you don't define. So the compiler will generate the other two for you. Usually these work fine. BUT if your class contains an owned RAW pointer then the default compiler generated versions don't work as you would expect.

So you should look up the rule of three.
If you have to define one of Copy Constructor/Copy Assignment or destructor you probably need to do all three. If your class only uses objects that obey the rule of three already then your class will obey the rule of zero automatically (look them up).

Design Review

Asking user to provide a large set of hash functions is unreasonable. Hash functions are hard to define in the first place and even harder to get a good distribution with over a set of them. You need to define some way that in most cases the hash is provided (though being able to provide your own is a nice touch).


Generating the N-bonacci numbers I'm new to Haskell, and this is one of the more interesting programs I've made with it so far. I understand there are already solutions to this problem that use much less code, but to me this one seems fairly intuitive. Are there any ways I can improve this code to be more readable or efficient?
fib :: Int -> [Integer]
fib n = (n-1) `replicate` 0 !: 1 ++ (+) `zipPoly` repFor tail (fib n) n
(!:) :: [x] -> x -> [x]
[] !: a = [a]
(l:ls) !: a = l : ls !: a
zipPoly :: (x -> x -> x) -> [[x]] -> [x]
zipPoly _ [] = []
zipPoly _ [a] = a
zipPoly f (a:b:c) = zipPoly f $ zipWith f a b : c
repFor :: (x -> x) -> x -> Int -> [x]
repFor _ _ 0 = []
repFor f a n = a : repFor f (f a) (n-1)
main = do
    s <- getLine
    putStrLn "Starting..."
    putStr $ unlines $ map show $ fib (read s :: Int)

Explanations of functions:

fib n is a list containing the n-bonacci numbers, starting with n-1 zeros and then a one.
list !: elem concatenates elem to the end of list.
zipPoly func lists is equivalent to zipWith but with more than two lists.
repFor func start amount is a list of length amount where the first element is start and every element after it is func applied to the element before it  ([x, f x, f (f x), f (f (f x))...]).

Spacing! Separate function definitions with blank lines.
I'd use a, b, c for type variables and x, y, z for variables.
A . looks cleaner than a $.
putStr . unlines . map show . fib $ read s :: Int

I would add another pair of parentheses to make the order of application absolutely obvious.
(l:ls) !: a = l : (ls !: a)

For similar reasons, I prefer the following.
fib n = fibBase ++ zipPoly (+) fibTails
    where fibBase = replicate (n-1) 0 ++ [1]
          fibTails = repFor tail (fib n) n

I'd use cs for the remainder of the list instead of c. Perhaps a where helps, too.
zipPoly f (a:b:cs) = zipPoly f $ c:cs
    where c = zipWith f a b

zipPoly needs the associativity of f, if I'm not mistaken. Perhaps its worth a comment.
Perhaps use a fold?
zipPoly f [] = []
zipPoly f (xs:xss) = foldl (zipWith f) xs xss

Don't reinvent the wheel too much :). Use Hoogle to search for a function using its signature.
For example, !: is snoc. Since you only use it once, I'd replace it with a ++. Similarly, repFor f x0 n is take n . iterate f $ x0.


Refactor selective item by highest create date or update date I have to check for the Highest Date from CostRepository whether it is create date or edit date then i have to get value in that field. edit date can be null. i don't know how to fix the codesmell.

var allCostsInThisPo = CostRepository
                       .GetAll()
                       .WhereInPo(po.po_surr_key);

if (allCostsInThisPo.Any())
{
    var costWithMaxCreatedDate = allCostsInThisPo.OrderByDescending(item => item.create_date).First();
    var allEditedCosts = allCostsInThisPo.Where(item=>item.update_date != null);

    if (allEditedCosts.Any())
    {
        var costWithMaxEditedDate = allEditedCosts.OrderByDescending(item => item.update_date).First();

        if (costWithMaxCreatedDate.create_date > costWithMaxEditedDate.update_date)
        {
            history.AllocationInfo = BuilkConstant.LogHistorySetting.GetAllocationHistoryLogDisplay(costWithMaxCreatedDate.CreatedByUser.user_fullname, costWithMaxCreatedDate.create_date);
        }
        else
        {
            history.AllocationInfo = BuilkConstant.LogHistorySetting.GetAllocationHistoryLogDisplay(costWithMaxEditedDate.EditedByUser.user_fullname, costWithMaxEditedDate.update_date.Value);
        }

    }
    else
    {
        history.AllocationInfo = BuilkConstant.LogHistorySetting.GetAllocationHistoryLogDisplay(costWithMaxCreatedDate.CreatedByUser.user_fullname, costWithMaxCreatedDate.create_date);
    }

}

You could try and make LINQ do more, to me it is more readable but it may not be to your style.

var allCostsInThisPo = CostRepository
                       .GetAll()
                       .WhereInPo(po.po_surr_key);

if (allCostsInThisPo.Any())
{
    var costWithMaxCreatedDate = allCostsInThisPo.OrderByDescending(item => item.create_date)
                                                        .Select(c => new { Name = c.CreatedByUser.user_fullname, Date = c.create_date})
                                                        .First();

    // if there is a max edited date greated than the cost created date then this will
    // be populated, otherwise will be null.
    var costWithMaxEditedDate = allCostsInThisPo.Where(item => item.update_date.HasValue && item.update_date.Value > costWithMaxCreatedDate.Date)
                                                          .OrderByDescending(c => item.update_date.Value)
                                                          .Select(c => new { Name = c.EditedByUser.user_fullname, Date = c.update_date.Value})
                                                          .FirstOrDefault();

    var maxCost = costWithMaxEditedDate ?? costWithMaxCreatedDate;

    history.AllocationInfo = BuilkConstant.LogHistorySetting.GetAllocationHistoryLogDisplay(maxCost.Name, maxCost.Date);
}


Reach C++ policy-based common interface with different template parameters from one container Please review my solution for the following problem. I am interested in:

What do you think of the design
Improvement tips
Usability
Efficiency

Problem: Need to be store policy-based objects with different template parameters in one container, and reach the common interface.

Solution:
C++ Shell

#include <exception>
#include <iostream>
#include <memory>
#include <string>
#include <vector>

class SinglePolicy
{
public:
    SinglePolicy() = default;

    void push(int data)
    {
        std::cout << "set to SinglePolicy\n";
        buffer_ = data;
    }

    int pop()
    {
        std::cout << "return from SinglePolicy\n";
        int data = buffer_;

        buffer_ = -1;

        return data;
    }

protected:
    int buffer_;
};

class VectorPolicy
{
public:
    VectorPolicy() = default;

    virtual void push(int data)
    {
        std::cout << "emplace_back to VectorPolicy\n";
        buffer_.emplace_back(data);
    }

    virtual int pop()
    {
        std::cout << "pop front from VectorPolicy\n";
        if(buffer_.empty())
            return -1;

        int data = buffer_[0];
        buffer_.erase(buffer_.begin());

        return data;
    }

protected:
    std::vector<int> buffer_;
};

template<class BufferPolicy>
class PolicyBasedBuffer
        : public BufferPolicy
{
public:
    PolicyBasedBuffer()
        : BufferPolicy()
    {}
};

typedef std::shared_ptr<PolicyBasedBuffer<SinglePolicy>> SinglePolicyPtr;
typedef std::shared_ptr<PolicyBasedBuffer<VectorPolicy>> VectorPolicyPtr;

class PolicyInterface
{
public:
    PolicyInterface(const std::function<void(int)>& pushArg, const std::function<int(void)>& popArg)
        : push(pushArg)
        , pop(popArg)
    {}

    const std::function<void(int)> push;
    const std::function<int(void)> pop;
};

typedef std::shared_ptr<PolicyInterface> PolicyInterfacePtr;

PolicyInterfacePtr saveToPolicyInterface(const SinglePolicyPtr& aPolicy)
{
    return std::make_shared<PolicyInterface>([aPolicy](int data) { aPolicy->push(data); }, [aPolicy]() { return aPolicy->pop(); });
}

PolicyInterfacePtr saveToPolicyInterface(const VectorPolicyPtr& aPolicy)
{
    return std::make_shared<PolicyInterface>([aPolicy](int data) { aPolicy->push(data); }, [aPolicy]() { return aPolicy->pop(); });
}

int main()
{
    auto singleBuffer = std::make_shared<PolicyBasedBuffer<SinglePolicy>>();
    auto vectorBuffer = std::make_shared<PolicyBasedBuffer<VectorPolicy>>();

    const int seven = 7;

    std::cout << "PolicyBasedBuffer test:\n";
    singleBuffer->push(seven);
    if(seven != singleBuffer->pop())
        throw std::runtime_error("SinglePolicy :: wrong return data");

    vectorBuffer->push(seven);
    if(seven != vectorBuffer->pop())
        throw std::runtime_error("VectorPolicy :: wrong return data");

    std::vector<PolicyInterfacePtr> policy;

    policy.push_back(saveToPolicyInterface(std::make_shared<PolicyBasedBuffer<SinglePolicy>>()));
    policy.push_back(saveToPolicyInterface(std::make_shared<PolicyBasedBuffer<VectorPolicy>>()));

    std::cout << "\nPolicyInterface test:\n";

    for(size_t i=0; i < policy.size(); ++i)
    {
        policy[i]->push(seven);
        if(seven != policy[i]->pop())
            throw std::runtime_error("policyInterface :: wrong return data");
    }

    std::cout << std::flush;
}

Few minor notes.

You have deliberately assigned -1 to represent undef, correct? The question whether you'll ever need to store -1 aside, you should probably define a named constant to get rid of magic number (it also helps when you decide to accept -1s). BTW, the minimal int (-2³¹ or so) is maybe a better alternative.
Every constructor and factory function accepts arguments by const ref. While acceptable for shared_ptr args, it makes sense to accept others (such as closures) by value and then move them inside the object's members.
const_cast is a threat to PolicyInterface's public push and pop methods.
VectorPolicy pushes elements to its right end but pops them on the left. If it should work as a LILO queue, std::deque is a better alternative to std::vector. Otherwise, VectorPolicy should follow stack (LIFO) idiom, and elements popped at the end.


Is this a sufficient way to prevent script injections and other bad stuff in strings Would this function be sufficient enough to remove all malicious code and foreign characters from a string?

//Clean the string
$out = ltrim($do);
$out = rtrim($out);
$out = preg_replace('/[^(\x20-\x7F)]*/','', strip_tags($out));

This data is not going into a SQL database, so I dont have to worry about sql injection attempts. Is there any way to improve my code and make it more efficient?

This function would clean any user inputted data (forms, and ?), and then save it do a database. This would be used in a global sanitizing function.

That one is quite simple (for me at least) since there is a very general answer :)

NO

There is no way you can ever really safely "repair" user input.

  "Please provide a list of everything you shouldn't to with a hammer"

is way harder than

  "list all appropriate uses of a hammer".

You might forget one or two but no harm done there if you go back and add them.

It might sound harsh but something will always byte you and if it's only EVAL(UNHEX(ASD23426363)) or something like that. (Sql example even so you did say it not sql but whatever).

Of course you might want to do things like strip out tags out of input for html context anyways so that a hole in your other code is not as easily exploited and less damage will be done but i shouldn't be your only defense.

Someone that said Filter Input, Escape Output way better than i could. Terry Chay

In short: Whatever you are doing there, find the appropriate escaping function and use it.

Shell context ? escapeshellarg

Html context ? htmlspecialchars

Database context ? Use prepared statements and never worry about sql injection again

Little edit:
I know what i said and the blog post contradict a little but thats fine with me. 'In practice' will always differ from general advice and sometimes you want to do everything you can ;)


Displaying a Sudoku Board For my programming class, we have been tasked with creating a Sudoku solver; the first step of the project was to create a program which would display the game board based on input provided by the user.

import math #To be used later

def board_filler():
    """Creates the sudoku board from user input"""
    board = [[], [], [], [], [], [], [], [], [], [], []]
    for x in enumerate(board):
        #If it is one of the rows that have lines, add them
        if ((x[0] + 1) % 4) == 0:
           for y in range(11):
               board[x[0]].append("-")
        else:
            for y in range(11):
                #If it is a column that has lines in it, add them
                if ((y + 1) % 4) == 0:
                    board[x[0]].append("|")
                else:
                    #Repeat until an inout has been entered
                    z = True
                    while z:
                        z = False
                        if x[0] > 7:
                            xRead = x[0] - 1
                        elif x[0] > 3:
                            xRead = x[0]
                        else:
                            xRead = x[0] + 1
                        if y > 7:
                            yRead = y - 1
                        elif y > 3:
                            yRead = y
                        else:
                            yRead = y + 1
                        number = input("Please enter a number for the square in column %s and in row %s, if there is no number, just hit enter:" %(xRead, yRead))
                        #Trys to make it a number, then checks to see if it is a number 1 to 9
                        try:
                            number = int(number)
                            if number > 9 or number < 1:
                                z = True
                                print("Please enter a number between 1 and 9")
                            else:
                                board[x[0]].append(number)
                        #If it is not a number, check if its empty
                        except (TypeError, ValueError):
                            #If its empty, add a space
                            if len(number) == 0:
                                board[x[0]].append(" ")
                            #If not ask for a number
                            else:
                                z = True
                                print("Please enter a number")
    return board

def board_printer(board):
    """Prints the sudoku board"""
    #Turns board into str to make the .join work
    for x in enumerate(board):
        for y in enumerate(board):
            board[x[0]][y[0]] = str(board[x[0]][y[0]])
    #Prints the board
    for x in enumerate(board):
        print(" ".join(board[x[0]]))

board_printer(board_filler())

A few notes for review:

The board_filler function feels quite bloated.
I don't like the way I implemented xRead and yRead.

How can I improve my code?

First of all, let the code speak! Don't unnecessarily add comments unless they are explaining a really complicated piece of code.

Basic definitions:

A magic number is defined to be:

  Unique values with unexplained meaning or multiple occurrences which could (preferably) be replaced with named constants

Can you find the magic numbers in your code?
Yes that's right, the culprits are 11 and 4.

Let's just define variables for those!

ROWS = 11
COLS = 11
GRID_ROWS = 4
GRID_COLS = 4

Next, let's replace every magic number with the appropriate variable!

Making the code shorter!

Function board_printer

Why use enumerate? Why make everything a str?

Your whole function can just be rewritten as the following:

def board_printer(board):
    """Prints the sudoku board"""

    for row in board:
        print(*row)

Function board_filler

You don't need to use for x in enumerate(board) at all!
Just use for x in range(ROWS) and change every instance of x[0] to x accordingly

Instead of

if ((x[0] + 1) % GRID_ROWS) == 0:
    for y in range(11):
        board[x].append("-")

Use

if ((x[0] + 1) % GRID_ROWS) == 0:
    board[x] = ["-"] * COLS

Use formatting!

for taking input of number, use number = input("Please enter a number for the square in row {xRead} and in column {yRead} (hit enter for no number): ") instead of using %s

Why use a dummy variable z?

Just do:

while True:
    number = input("Please enter a number for the square in column {x[0] + 1} and in row {y + 1} (hit enter for no number): ")

    try:
        number = int(number)

        if number > 9 or number < 1:
            raise ValueError
        else:
            board[x].append(number)

        break

    except (TypeError, ValueError):
        if not number:
            board[x].append(" ")
        else:
            print("Please enter an integer between 1 and 9")

Essentially, it tries to take an input, check if the value is an integer between 1 and 9, else raises an exception. If the number was indeed valid, it makes it to the end of the try statement, which causes the loop to break

Misc

Run board_printer(board_filler()) inside if __name__ == __main__:. This causes board_printer(board_filler()) not to run if it was imported from another module.
Try using list(map(int, input(f'Enter {COLS - (COLS // GRID_COLS)} space separated integers: ').split())) for each row. It is tiring to fill every single cell by hand! So ask input for a whole row. It's your personal preference though!

Hope this helps!

EDIT:

This part is to solely dedicated to removing xRead and yRead and is not related to the above improvements

First we have to change
ROWS = COLS = 11 to ROWS = COLS = 9 and
GRID_ROWS = GRID_COLS = 4 to GRID_ROWS = GRID_COLS = 3

Do not append - or | to board at all!

Just remove all statements that append - or | and also remove xRead and yRead. Now, the board would look like a sudoku board without - or |

In the board_filler use

def board_printer(board):
    """Prints the sudoku board"""

    for row in range(ROWS):
        s = ''

        for col in range(COLS):
            s += str(board[row][col]) + ' '

            if not (col + 1) % GRID_COLS:
                s += '| '

        print(s)

        if not (row + 1) % GRID_ROWS:
            print('-' * len(s))

This will print - or | according to the row or column!

Final code including all above mentioned improvements

ROWS = COLS = 9
GRID_ROWS = GRID_COLS = 3

def board_filler():
    """Creates the sudoku board from user input"""

    board = [[] for _ in range(ROWS)]

    for x in range(ROWS):
        for y in range(COLS):
            while True:
                number = input(f"Please enter an integer for the square in column {x + 1} and in row {y + 1} (hit enter for no number): ")

                try:
                    number = int(number)

                    if number > 9 or number < 1:
                        raise ValueError
                    else:
                        board[x].append(number)

                    break

                except (TypeError, ValueError):
                    if not number:
                        board[x].append(" ")
                    else:
                        print("Please enter an integer between 1 and 9")

    return board

def board_printer(board):
    """Prints the sudoku board"""

    print()

    for row in range(ROWS):
        s = ''

        for col in range(COLS):
            s += str(board[row][col]) + ' '

            if not (col + 1) % GRID_COLS:
                s += '| '

        s = s[:-1] # Removes trailing space

        print(s)

        if not (row + 1) % GRID_ROWS:
            print('-' * len(s))

if __name__ == '__main__':
    board_printer(board_filler())

NOTE: board_filler doesn't look as bloated either! The code kills 2 birds with one stone!


Defensive programming type-checking I have issues with dynamically typed languages, and I tend to worry about type a lot.

Numpy has different behaviour depending on if something is a matrix or a plain ndarray, or a list.  I didn't originally have all these asserts, but I inserted them while trying to debug a type error.

from numpy import *
sigmoid = vectorize(lambda(x): 1.0/(1.0+exp(-x)))

def prob_hOn_givenV (v,b,w):
    assert isinstance(v,matrix)
    assert isinstance(w,matrix)
    assert isinstance(b,matrix)
    assert shape(v)[1]==shape(w)[0]
    #print("|v|="+str(shape(v)) +"|w|="+str(shape(w)) )
    return sigmoid(b+v*w) #sum up rows (v is a column vector)

def prob_vOn_givenH (h,a,w):
    assert isinstance(h,matrix)
    assert isinstance(a,matrix)
    assert isinstance(w,matrix)

    assert shape(h)[1]==shape(w.T)[0]

    return sigmoid(a+h*w.T) #sum up columns. (h is a row vector)

Is there a better way? Perhaps a defensive programming toolkit?  There is also duplication in the code, but I can't see a nice way of removing it while maintaining meaning.  People familiar with Restricted Boltzman Machines will recognise the conditional formula used for Gibbs sampling for contrastive divergence.

First, I recommend avoiding the matrix type altogether. The problems it causes outweighs the minor syntactical convenience it provides, in my (and quite a few others') opinion.

Second, the idiomatic way to do this kind of thing in numpy is not to assert that the inputs are particular types, but to coerce them to an ndarray. np.asarray() does this efficiently; if the input is an ndarray, the input passes through untouched; if it is an ndarray subclass, then a new ndarray will be created but only as a view on the original data so no data will be copied.

import numpy as np

def sigmoid(x):
    # No need to use vectorize() for this. It's already vectorized.
    return 1.0 / (1.0 + np.exp(-x))

def prob_hOn_givenV(v, b, w):
    v = np.asarray(v)
    b = np.asarray(b)
    w = np.asarray(w)
    # Don't bother checking the shape. `np.dot()` will do that for us.
    # Strictly speaking, the `asarray(v)` and `asarray(w)` aren't necessary
    # since `np.dot(v, w)` already does that internally. But it does no harm.
    return sigmoid(b + np.dot(v, w))


Number of prime numbers in a given range I've written a method that should return the numbers of prime numbers in a given range:

public static int numberOfPrim(int up){                 // Returns amount of primnumbers from 0 to up
    if(up<=0){
        return 0;
    }
    int primAmount = 0;
    for(int i = 1; i<=up ; i++){
        if(checkIfPrim(i) == true){
           primAmount++;
        }
    }
    return primAmount;
}

public static boolean checkIfPrim(int number){         // If number is prim -> return true
    if (number < 2){
        return false;
    }
    boolean prim = true;
    if(number == 2){
        return prim;
    }
    boolean[] con = new boolean[number];
    for(int i = 2; i < number; i++){
        if(number %i == 0) {
            con[i] = true;
        }
    }
    for(int j = 0; j < number; j++){
        if(con[j] == true){
            prim = false;
        }
    }
    return prim;
}

The code is working like I want it to. For example, numberOfPrim(10) should return 4 (2, 3, 5, 7). I'm just asking if there are any locations where I can improve my code, like shorten it or if there are any other possibilities to write such a method.

What is the purpose of shortening the term "Prime(s)" to Prim in your method names? IMO numberOfPrimes and checkIfPrime are more readable. The same is true for primAmount etc.
if(checkIfPrim(i) == true) can simply be if (checkIfPrim(i))
Your spacing is inconsistent. Generally, it is recommended to leave a space between a control structure (for, if, while etc.) and its following open paren, which you adhere to exactly once in checkIfPrim, but not elsewhere. The same applies to operators: Write up <= 0 instead of up<=0.
Your checkIfPrim method is grossly overcomplicated. First of all, you check every number up to number for divisibility, while it should be obvious that there can be no divisor greater than number / 2. Still, if you think about the problem, you will soon arrive at another conclusion, which is that you only need to check numbers up to sqrt(number), because you can write a * b = number (with a and b being positive integers), which is to say that a and b are divisors of number. Now, letting sqrt(number) = c * c, and by thinking about how multiplication behaves, we can conclude that either a or b must always be lesser or equal to  while the other must be greater or equal to c, because when either a or b is smaller than c the total product would shrink unless the other one grows to counteract this fact.
The next problem is your use of a boolean array, which is completely redundant. You do not need to store the result of the division test for each number separately, but you only have to keep track of whether you have found a divisor for number (=> number is not prime) or not (=> number is prime). All in all, I would rewrite the method to something like this:

public static boolean checkIfPrime(int number) {
    if (number < 2) {
        return false;
    }

    if (number == 2) {
        return true;
    }

    int root = (int) Math.sqrt(number);
    for (int i = 2; i <= root; i++) {
        if (number % i == 0) {
            return false;
        }
    }

    return true;
}

Although this code should be reasonably efficient for relatively small numbers, there are faster algorithms to solve this problem. However, most of these are not as easy to implement, and if you are not planning on doing this for very large integers, then you should be fine with the changes I suggested. If you are still interested in improving effiency, I point you to the Wikipedia article about the prime-counting function for a start.


Handling shared state between actix and a parallel thread In order to learn rust and concurrency , im trying to share a common hashmap between actix web server and a parallel thread

Theres one thread which updates a hashmap with the current timestamp every 10 seconds
theres a GET API which returns the latest timestamp provided in the hashmap

heres my code
use actix_web::{web, App, HttpServer};
use std::collections::HashMap;
use chrono::prelude::*;
use std::sync::{Arc, Mutex};
use std::thread;
use std::time::Duration;

async fn get_latest_timestamp(data: web::Data<Arc<Mutex<HashMap<String, String>>>>) -> String {

    let wrapped_data = data.lock().unwrap();

    match wrapped_data.get("time_now") {
        Some(i) => String::from(i),
        None => String::from("None")
    }
}

fn update_timestamps(data: Arc<Mutex<HashMap<String, String>>>) -> () {

    loop {
        thread::sleep(Duration::from_secs(10));
        println!("Updating time.... ");
        let now: DateTime<Local> = Local::now();

        let mut data = data.lock().unwrap();

        data.insert(String::from("time_now"), now.to_rfc2822());
    }
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {

    let mut hash = HashMap::new();
    hash.insert(String::from("time_now"), Local::now().to_rfc2822());

    let data = Arc::new(Mutex::new(hash));

    let hash_clone = Arc::clone(&data);
    thread::spawn(move || {
        update_timestamps(hash_clone)
    });

    HttpServer::new( move || {
        App::new()
            .data(data.clone())
            .route("/", web::get().to(get_latest_timestamp))
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

Is there anything i could do handle better. A couple of questions i do have

How does one generally handle errors that occur on the parallel thread? is the main thread notified in someway?

the code in update_timestamps runs in a continous loop - is there a better way to handle it so that it cleanly exits?

Answering your questions:

Depends on the nature of the error.

Panics will make the thread exit. If the creator called join on the result of thread::spawn, the result of that join will be Err(boxed_any). The boxed_any is the argument given to panic!. This is one common way to signal and handle an error.
If the panic occurs while a mutex's lock is in scope, the mutex will be poisoned and therefore return Err for all locking attempts on all threads indefinitely.
In case you'd like to notify another thread about an event, let's say, an occurence of a meaningful error, it's best to use channels. You can read more in "Using Message Passing to Transfer Data Between Threads": https://doc.rust-lang.org/book/ch16-02-message-passing.html

I'm not sure what you meant by exiting cleanly. You need the thread to run indefinitely. The thread will exit "cleanly" when the server is closed (and so when the main thread exits).

Minor points:

calls to Arc::clone can be simply .clone(). Quoting the docs:

// The two syntaxes below are equivalent.
let a = foo.clone();
let b = Arc::clone(&foo);

the second (and last) data.clone() is unnecessary. This is often seen in code. Just pass the value.

You still have ownership of that original value. That original value can be spent by passing it to actix.

-> () in a function signature is never meaningful; just cut it out.


Rvalue copy and and assignment for secure array class I have a C++03 library with lots of classes. I'm interested in trying to take advantage of Rvalues and move semantics under C++11 for two or three critical classes. Once of the critical classes is a SecBlock, which is effectively a secure array class. It has a secure allocator and deallocator, and its used nearly everywhere.

SecBlock looks like so, where T might be a byte or an int:

template <class T, class A = AllocatorWithCleanup<T> >
class SecBlock
{
   ...
private:
    A m_alloc;
    size_type m_size;
    T *m_ptr;
}

If I am reading Thomas Becker's "C++ Rvalue References" treatise correctly, I can use a simple swap because SecBlock does not have a base class (no need for Base(std::move(rhs)):

template <class T, class A = AllocatorWithCleanup<T> >
class SecBlock
{
   ...

#if CXX11_AVAILABLE
    SecBlock(SecBlock&& t)
    {
        std::swap(*this, t);
    }
    SecBlock& operator=(SecBlock&& t)
    {
        if(this != &t)
            std::swap(*this, t);
        return *this;
    }
#endif

private:
    A m_alloc;
    size_type m_size;
    T *m_ptr;
}

We guard on CXX11_AVAILABLE because the library is a C++03 library at heart.

It's probably obvious I have some open questions. For instance, do I have to guard for self assignment in operator=? (I suspect not, but I think its best to show the code and take the comments).

I'm also interested in knowing if a derived class must implement the semantics also, even if they add no members.

Is the code above correct, or are there gaps that need to be addressed? If there are gaps, what are they?

This code does not function.

Let's take a look at your move constructor:

SecBlock(SecBlock&& t)
{
    std::swap(*this, t);
}

This caused a stack overflow on my system. Why? Imagine if the swap function was implemented like this:

template <typename T>
void swap (T &t1, T &t2)
{
    T temp = std::move (t1) ;
    t1 = std::move (t2) ;
    t2 = std::move (temp) ;
}

In fact, this is how it is implemented by Visual Studio 2012. The first line T temp = std::move (t1) ;, calls your move constructor. This causes you to recursively call your move constructor until your program inevitably crashes.

Another problem is that before the swap, m_size and m_ptr for *this will be uninitialized. So even if you fixed your move constructor, you will be transferring uninitialized data to t. Assuming you're destructor does some clean-up, this could lead to undefined behavior.

All in all, both member-functions are a mess.
Check out the rule of 5 for more help.


Testing if numbers in the array can be added up to equal the largest number in the array Okay, here's a challenge at Coderbyte that completely stumped me. I came up with a solution, but I know it is flawed.

  Have the function ArrayAdditionI(arr) take the array of numbers stored in arr and return the string true if any combination of numbers in the array can be added up to equal the largest number in the array, otherwise return the string false. For example: if arr contains [4, 6, 23, 10, 1, 3] the output should return true because 4 + 6 + 10 + 3 = 23. The array will not be empty, will not contain all the same elements, and may contain negative numbers.

It is easy enough to find the highest value in the array.  If the length of the array was fixed, I could probably come up with a way to test various combinations, but I'm not sure how to handle different arrays. I wonder if the solution involves recursion? Or perhaps there is a way to call a function a number of times depending on the size of the array. Anyway, my "solution" is below. I would love to see a real solution, or if anyone wants to give me some hints, I am happy to give it another try.

function ArrayAdditionI(arr) {
  var greatest = -100;
  var index = -1;
  var sum = 0;

  for (var i = 0; i<arr.length; i++){
    if (arr[i] > greatest){
      greatest = arr[i];
      index = i;
    }
  }
    arr.splice(i,1)
      for (var i = 0; i<arr.length; i++){
        sum += arr[i];
        if (greatest = sum){
          return true;
        }else{
          return false;
        }
      }
  }

First, let's fix the two bugs in the code.

If the array would contain only numbers below -100, your code would think that -100 was the greatest number, located at index -1. That would make the rest of the code remove the last item of the array instead of the greatest one, and try to sum up the remaining to be -100.

Start with the first item as the greatest instead of -100:

var greatest = arr[0];
var index = 0;

for (var i = 1; i < arr.length; i++){
  if (arr[i] > greatest){
    greatest = arr[i];
    index = i;
  }
}

(That bug would however not change the outcome, as an array of numbers below -100 can't be combined to give the sum -100, and an array of all negative numbers can't form a sum according to the specification.)

The splice is using the variable i instead of index. It should be:

arr.splice(index, 1)

Using recursion is a good idea to find a combination. You can make a function that determines if any combination of items from an array can give a specific sum. The function would loop through the items in the array, and call itself to determine if the remaining items could give the sum when the item is subtracted.

Another alternative is to use a bitmask to represent the combinations. If you count from 1 to 2^arr.length-1 you will get all possible combinations. The bitmask 11101 (decimal 29) would give the winning combination [4, 6, 10, 3] from the array [4, 6, 10, 1, 3].


Break music player into usable components in SwiftUI I created a music player using swiftUI.I need to ask a question when I break up my player into usable components. How should I take this approach with the music player? because the music player is one view can't break up?
import SwiftUI

struct ContentView: View {
    var body: some View {
        VStack {
            Image("albumCover")
            HStack{
                VStack {
                    Text("Album name")
                    Text("Artist name")
                }
                Spacer()
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName: "heart")
                }
            }.padding()
            HStack{
                Text("0")
                Slider(value: /*@START_MENU_TOKEN@*//*@PLACEHOLDER=Value@*/.constant(10)/*@END_MENU_TOKEN@*/)
                Text("-5:00")
            }.padding()

            HStack{
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName: "shuffle")
                }
                Spacer()
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName:"backward.end")
                }
                Spacer()
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName: "play")
                }
                Spacer()
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName: "forward.end")
                }
                Spacer()
                Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                     Image(systemName: "repeat")
                }

            }.padding()

                HStack{
                    Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                         Image(systemName: "hifispeaker")
                    }

                    Spacer()
                    Button(action: /*@START_MENU_TOKEN@*/{}/*@END_MENU_TOKEN@*/) {
                         Image(systemName: "music.note.list")
                    }

                }.padding()

        }
    }
}

struct ContentView_Previews: PreviewProvider {
    static var previews: some View {
        ContentView()
    }
}

I need to ask a question when I break up my player into usable
components. How should I take this approach

The idea is to break a long and not so readable code into smaller parts. Also, each individual part needs to access variables etc.
For this you can use a private extension (unless it's in another file, then skip private):
struct ContentView: View {
    @State private var sliderValue = 1.0

    var body: some View {
        VStack {
            Image("albumCover")
            infoView
            sliderView
            actionsView
            bottomView
        }
    }
}

private extension ContentView {
    var infoView: some View {
        HStack {
            VStack {
                Text("Album name")
                Text("Artist name")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "heart")
            }
        }
        .padding()
    }
}

private extension ContentView {
    var sliderView: some View {
        HStack {
            Text("0")
            Slider(value: $sliderValue)
            Text("-5:00")
        }
        .padding()
    }
}

private extension ContentView {
    var actionsView: some View {
        HStack {
            Button(action: {}) {
                Image(systemName: "shuffle")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "backward.end")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "play")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "forward.end")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "repeat")
            }
        }
        .padding()
    }
}

private extension ContentView {
    var bottomView: some View {
        HStack {
            Button(action: {}) {
                Image(systemName: "hifispeaker")
            }
            Spacer()
            Button(action: {}) {
                Image(systemName: "music.note.list")
            }
        }
        .padding()
    }
}

Note that you can declare sliderValue directly in ContentView:
struct ContentView: View {
    @State private var sliderValue = 1.0

and access it in the extension:
private extension ContentView {
    var sliderView: some View {
        Slider(value: $sliderValue)

But the code above still doesn't address the problem of repeating blocks of code in your ContentView. See how often this block is reused:
Button(action: {}) {
    Image(systemName: "hifispeaker")
}

What if you want to apply a modifier to every Image?
A solution may be to extract the repeating code to one function:
private extension ContentView {
    func customButton(imageName: String, action: @escaping () -> Void) -> some View {
        Button(action: action) {
            Image(systemName: imageName)
        }
    }
}

and use it like this:
private extension ContentView {
    var bottomView: some View {
        HStack {
            customButton(imageName: "hifispeaker", action: {})
            customButton(imageName: "music.note.list", action: {})
        }
        .padding()
    }
}


Direct Mapped Cache simulation This is my attempt to build a simple simulation of a direct mapped cache, I'm new to C and trying to learn some low level concepts. I had some difficulties with the design, because I'm used to program in a object-oriented style.

#include <stdio.h>

int create_mask(int num_of_bits, int pos) {
    return (1 << num_of_bits)-1 << pos;
}

int get_window_bits(int num, int window_size, int pos) {
    return (create_mask(window_size, pos) & num) >> pos;
}

int is_valid(char cache[4][4], int set, int tag) {
    return cache[set][0] && (cache[set][1] == tag);
}

void load_to_cache(int address, char memory[8][2], char cache[4][4], int set, int tag, int offset) {
    int memory_index = get_window_bits(address, 3, 1);

    cache[set][0] = 1;
    cache[set][1] = tag;
    cache[set][2] = memory[memory_index][0];
    cache[set][3] = memory[memory_index][1];
}

int fetch_from_cache(int address, char cache[4][4], char memory[8][2]) {
    int set = get_window_bits(address, 2, 1);
    int offset = get_window_bits(address, 1, 0);
    int tag = get_window_bits(address, 1, 3);

    if (!is_valid(cache, set, tag)) {
        printf("cache miss\n");
        load_to_cache(address, memory, cache, set, tag, offset);
    } else {
        printf("cache hit\n");
    }

    return cache[set][2+offset];
}

int main() {

    /* init memory - 8 blocks of 2 bytes */
    char memory[8][2] = {{'A','B'},
                        {'C','D'},
                        {'E','F'},
                        {'G','H'},
                        {'I','J'},
                        {'K','L'},
                        {'M','N'},
                        {'O','P'}};

    /* init cache - 4 lines of 2 bytes storage*/
    char cache[4][4] = {{0,0,0,0},
                        {0,0,0,0},
                        {0,0,0,0},
                        {0,0,0,0}};

    fetch_from_cache(0, cache, memory);
    fetch_from_cache(1, cache, memory);

    return 0;
}

Structure:

The program consists of a 2d array that represents the memory, it's size is 16 bytes:

char memory[8][2] = {{'A','B'},
                    {'C','D'},
                    {'E','F'},
                    {'G','H'},
                    {'I','J'},
                    {'K','L'},
                    {'M','N'},
                    {'O','P'}};

another array which represents the cache of 4 lines, each line can store 2 bytes
the first byte of the line is for the valid bit, the second is for the tag, and the rest are for the data.

char cache[4][4] = {{0,0,0,0},
                    {0,0,0,0},
                    {0,0,0,0},
                    {0,0,0,0}};

The function get_window_bits is used to extract the tag, set and offset of a memory address.

int get_window_bits(int num, int window_size, int pos) {
    return (create_mask(window_size, pos) & num) >> pos;
}

I hope everything else is clear.

Here are some things that may help you improve your program.

Use parentheses to clarify expressions

The code contains this line:

return (1 << num_of_bits)-1 << pos;

A programmer reading it might wonder which of the following is the correct interpretation:

return ((1 << num_of_bits)-1) << pos;
return (1 << num_of_bits)-(1 << pos);

To interpret correctly (the first one), one would have to remember that according to C operator precedence, the - operator has higher precedence than the << operator.  We can easily avoid this potential confusion by simply writing the code with parentheses, even though, technically, they're redunandant.

Eliminate unused variables

The load_to_cache function includes unused parameter offset.  If it's not used, it probably should be removed.

Be wary of plain char

Whether char is signed or unsigned is implementation defined.  I'd suggest instead using uint8_t from <stdint.h> to make sure that there aren't any surprises when doing right shifts.  A short example shows the difference:

#include <stdio.h>
int main() {
    signed char a = -1;
    unsigned char b = -1;
    printf("%x\n", a>>2);    // prints ffffffff
    printf("%x\n", b>>2);    // prints 3f
}

For similar reasons, most of the places the code is using int should probably be either unsigned or size_t.

Use a struct where appropriate

The cache is just memory, of course, but with a simulation, the bytes have structure and meaning.  The code has these three lines:

cache[set][0] = 1;
cache[set][1] = tag;
cache[set][2] = memory[memory_index][0];
cache[set][3] = memory[memory_index][1];

This suggests the following struct definition:

#define CACHESIZE 2
typedef struct cacheline_s {
    uint8_t valid;
    uint8_t tag;
    uint8_t mem[CACHESIZE];
} cacheline;

Now the lines above become this:

cache[set].valid = 1;
cache[set].tag = tag;
cache[set].mem[0] = memory[memory_index][0];
cache[set].mem[1] = memory[memory_index][1];

Reconsider the interfaces

Instead of this:

if (!is_valid(cache, set, tag)) {

I think it might make more sense to pass only a single cache line.  Using the struct defined above, the function now looks like this:

int is_valid(const cacheline *cache, int tag) {
    return cache->valid && (cache->tag == tag);
}

Note too that we're passing a const pointer, advertising the fact that the underlying cacheline will not be changed by this function.  A similar interface change could be made to load_to_cache.

Eliminate "magic numbers"

This code has a number of inscrutable "magic numbers," that is, unnamed constants such as 2, 4, 8, etc.  Generally it's better to avoid that and give such constants meaningful names.  That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "2" and then trying to determine if this particular 2 is relevant to the desired change or if it is some other constant that happens to have the same value.  This is especially beneficial in cases like this in which some numbers depend on others.  For example, we can use these to make sure that the two constants are easily changed and kept consistent.  Only the first variable needs to be touched.

#define CACHESIZE_BITS 1
#define CACHESIZE (1u << CACHESIZE_BITS)


Probabilistic matchmaking simulation The following is a problem I found on this wiki. What can I do to optimize my algorithm, and make this code more C++11?

  Write a program to discover the answer to this puzzle:"Let's say men
  and women are paid equally (from the same uniform distribution). If
  women date randomly and marry the first man with a higher salary, what
  fraction of the population will get married?"

#include <iostream>
#include <algorithm>
#include <vector>
#include <ctime>
//#include <cstdlib>

using std::cin;
using std::cout;
using std::endl;
using std::random_shuffle;

const int MIN = 1;
const int MAX = 1000000;
const int MALES = 100;
const int FEMALES = 100;

class Female{
public:
    Female(){isMarried = false;}

    bool isMarried;
    int salary;
};

class Male{
public:
    Male(){isMarried = false;}

    bool isMarried;
    int salary;
};

void marriageTest(std::vector<double> &results){

    double count = 0;
    bool done = false;

    Male male[MALES];
    Female female[FEMALES];

    //Fill array of Females and Males with random salaries ranging from 1 to 10
    for(int i=0; i<100; i++){
        int output = MIN + (rand() % (int)(MAX - MIN + 1));
        i[male].salary = output; //( ͡° ͜ʖ ͡°)
    }
    for(int i=0; i<100; i++){
        int output = MIN + (rand() % (int)(MAX - MIN + 1));
        i[female].salary = output;
    }

    //Start dating
    //Keep dating until the maximum salary of males is lower than minimum salary of females

    do{
        random_shuffle(&male[0], &male[MALES]);               //Shuffle array of males
        random_shuffle(&female[0], &female[FEMALES]);           //Shuffle array of females

        for(int i=0; i<100; i++){                              //Compare a female and male from both arrays
            if(female[i].salary < male[i].salary)
                if(!female[i].isMarried && !male[i].isMarried){
                    count++;
                    female[i].isMarried = true;
                    male[i].isMarried = true;
                }
        }

        //Check for Maximum Male Salary
        int maxMen = MIN;
        for(int i=0; i<100; i++){
            if(male[i].salary > maxMen && !male[i].isMarried)
                maxMen = male[i].salary;
        }

        //Check for Minimum Female Salary
        int minWomen = MAX;
        for(int i=0; i<100; i++){
            if(female[i].salary < minWomen && !female[i].isMarried)
                minWomen = female[i].salary;
        }

        if(maxMen <= minWomen)
            done = true;

    }while(!done);

    results.push_back(count/100);
}

void runTrials(int num_trials, std::vector<double> &results){

    for(int i=0; i<num_trials;i++)
        marriageTest(results);
}

double Percentage(std::vector<double> &results){

    double final = 0;
    int trials = 0;
    for(auto i : results){
        final += i;
        ++trials;
    }

    return final/trials;
}

int main()
{
    int num_trials;
    std::vector<double> results;

    srand(time(NULL));

    cout << "Number of trials: ";
    cin >> num_trials;

    runTrials(num_trials,results);

    double final = Percentage(results);

    cout << "Percentage: " << final << endl;

    return 0;
}

There doesn't seem to be a need for separate Male and Female structures as they hold the same exact data.  Instead, just have one named Person and create male and female instances of it.

You also don't need classes if you're not going to have any private data; just use structs.

struct Person
{
    bool isMarried;
    int salary;

    Person() : isMarried(false) {}
};

Person males[MALES];
Person females[FEMALES];

Regarding the use of C++11, you can start by removing both std::srand() and std::rand() and replacing them with functionality from the <random> library, particularly std::shuffle().


Binding data to a ComboBox and then showing ValueMember in another TextBox I have a comboBox and a TextBox in the View and when the view is loaded, the ComboBox should be populated. After that when a user selects particular item from the ComboBox, its related ValueMember should be displayed in the TextBox.

The View doesn't know about DAL or Presenter and, it only has properties and events.

VIEW

public partial class frmBankAccount : Form, IBankAccountView
{

    public event EventHandler OnLoadEmployees;

    public BindingSource LoadingEmployees
    {
        set { cmbName.DataSource = value; cmbName.DisplayMember = "Name"; cmbName.ValueMember = "Emp_ID"; }
    }

    private void frmBankAccount_Load(object sender, EventArgs e)
    {
         OnLoadEmployees(sender, e);
    }

    private void cmbName_SelectedValueChanged(object sender, EventArgs e)
    {
        txtEID.Text = cmbName.SelectedValue.ToString();
    }

}

When the event fires, Presenter gets a DataTable from DataService and binds it to the ComboBox through the property.

PRESENTER

class BankAccountPresenter : BasePresenter
{
    public BankAccountPresenter(IBankAccount model, IBankAccountView view, IDataService dataService )
    {
        _DataService = dataService;
        _Model = model;
        _View = view;

        this.WireUpViewEvents();
    }

    private void WireUpViewEvents()
    {
        _View.OnLoadEmployees += new EventHandler(_View_OnLoadEmployees);
    }

    private void _View_OnLoadEmployees(object sender, EventArgs e)
    {
        LoadEmployeeNames();
    }

    private void LoadEmployeeNames()
    {
        BindingSource bs = new BindingSource();
        bs.DataSource = _DataService.LoadEmplyees(); //DataService returns a DataTabel with "Emp_ID" and "Name"
        _View.LoadingEmployees = bs;
    }

    //Main window will call this method to load the View
    public void Show()
    {
        _View.ShowDialog();
    }

}

NOTE: When populating ComboBox since I need only two fields (Emp_ID and Name), The DataService returns a DataTable instead of the Model as the model has about 10 other fields as well. The Model will be used in subsequent tasks like Search, Insert, Update etc.

Could you please review this code and provide your feedback? You may give your attention to the way that I've used properties to set DisplayMember and ValueMember of the Combo. If there is anything which is not acceptable kindly let me know with an alternate solution.

It looks pretty clean. As far as I can tell, you have a good separation of concerns. Everything is properly cased and your functions have verb-noun names. All good things.

The only nitpick I have is the bs variable. I'm not a fan of ultra short names of any kind. You could call it source or some such thing. Seriously though, that's a huge nitpick. I just couldn't find anything else to comment on.


C++ Fast Fourier transform attempt 2 This is a follow up for C++ Fast Fourier transform . I did end up changing the algorithm slightly to reduce the memory usage.
#include <vector>
#include <iostream>
#include <complex>
#include <cmath>
#include <algorithm>

#define N 1048576 // Problem size, must be a power of 2
#define PI 3.14159265358979323846

typedef std::vector<std::complex<double>> complexVec;

/*
Calculates the N'th roots of unity.
*/
complexVec rootsOfUnityCalculator() {
    complexVec table;
    table.resize(N);

    for (size_t k = 0; k < N; k++) {
        table[k] = std::complex<double>(std::cos(-2.0 * PI * k / N), std::sin(-2.0 * PI * k / N));
    }

    return table;
}

/*
Permutes an input vector of size length - a power of 2 - according
to the bit reversal permutation.

TESTED: SUCCESS
*/
void bitReversal(complexVec& input) {
    std::vector<size_t> permutation;
    permutation.resize(N);

    permutation[0] = 0;
    size_t k = 2;

    // Calculating the permutation according to recursive relation
    while (k <= N) {
        for (size_t j = 0; j < k / 2; j++) {
            permutation[j] *= 2; // permutation_k(j) = 2 * permutation_(k/2)(j)
            permutation[j + k / 2] = permutation[j] + 1; // permutation_k(j + k/2) = 2 * permutation_(k/2)(j) + 1
        }
        k *= 2;
    }

    // Permute the input
    for (size_t j = 0; j < N; j++) {
        if (j < permutation[j]) {
            std::swap(input[j], input[ permutation[j] ]);
        }
    }
}

void unorderedFFT(complexVec& input, const complexVec& table) {
    size_t k = 2;

    while (k <= N) {
        for (size_t r = 0; r < N / k; r++) {
            for (size_t j = 0; j < k / 2; j++) {
                std::complex<double> plusMinus = input[r * k + j + k / 2] * table[N / k * j]; // omega_k^j = (omega_N^(N/k))^j = omega_N^(Nj/k)
                input[r * k + j + k / 2] = input[r * k + j] - plusMinus;
                input[r * k + j] += plusMinus;
            }
        }

        k *= 2;
    }
}

/*
Calculates the FFT

TESTED: SUCCESS
*/
void FFT(complexVec& input, const complexVec& table) {
    bitReversal(input);
    unorderedFFT(input, table);
}

// Prints array of N complex numbers
void printArray(complexVec& array) {
    for (size_t k = 0; k < N - 1; k++) {
        std::cout << array[k] << ", ";
    }

    std::cout << array[N - 1] << std::endl;
}

int main() {
    complexVec input;
    input.resize(N);

    // Initialize the input to all 0, 1, ..., N
    for (size_t k = 0; k < N; k++) {
        input[k] = std::complex<double>(k, 0.0);
    }

    const complexVec table = rootsOfUnityCalculator();

    FFT(input, table);
}

Profiling results
Using the profiler in Visual Studio, on my machine (Intel Haswell, 4770K) I get a breakdown roughly like this:

unorderedFFT 107ms
rootsOfUnityCalculator 21ms
bitReversal 17ms

So let's improve all of those.
Roots of unity
The N'th roots of unity in the complex plane is a set of equally spaced points on the unit circle. Their coordinates can be calculated using sine and cosine, but an alternative is starting at 1 + 0i and successively applying a small vector rotation to it (alternative view: the first non-trivial root is calculated, and then iteratively raised to the powers of 0, 1, 2 .. N-1). That way, only one pair of sin/cos is necessary (by the way, std::polar can be used to replace such a sin/cos pair with simpler code). A downside is that it may be less precise. It tends to be OK, but it will perturb the results slightly.
For example, like this:
complexVec rootsOfUnityCalculator() {
    complexVec table;
    table.resize(N);

    auto step = std::polar(1.0, -2.0 * PI / N);
    std::complex<double> root = 1.0;
    for (std::size_t k = 0; k < N; k++) {
        table[k] = root;
        root *= step;
    }

    return table;
}

With this, the time it takes has gone down to 7.5ms, so a factor of 2.3, which is certainly significant. This could be done even faster than that by unrolling the loop and removing the dependency between the copies of the loop body, for example if unrolling by 2 then there would be a separate root1 and root2 that are each multiplied by stepSquared so that their calculations are independent (improving instruction-level parallelism). Doing that reduced the time to 6ms.
Bit-reversal permutation
This is a well-studied permutation, because it is of practical interest, namely this exact application. Various more efficient techniques are known, including some variants of "increment a reversed integer". If an integer i and its reverse rev are both available, calculating the next values for both of them is efficiently possible.
Of course i can just be incremented. rev needs some more work, but there is hope: the XOR of an integer and its successor is a "nice mask" consisting of a contiguous range of ones and a contiguous range of zeroes, with no "mess". This results from how, in the increment, the carry goes through the trailing ones, flipping each of them, then finally the first zero that is found is also flipped and the carry is "absorbed" by that zero - so the bits the flipped start at the least significant bit and form a contiguous range. That is relevant because a "nice mask" can be reversed by shifting it, and then XORing rev by it results in a "bit-reversed increment". A variant of that trick is using a trailing-zero-count instruction to find how long the contiguous range of flipped bits is, and then basing a mask directly on that. Let's go with that:
void bitReversal(complexVec& input) {
    std::size_t maskLen = std::countr_zero(unsigned(N));

    // Permute the input
    for (std::size_t j = 0, rev = 0; j < N; j++) {
        if (j < rev)
            std::swap(input[j], input[rev]);
        std::size_t maskLen = std::countr_zero(j + 1) + 1;
        rev ^= N - (N >> maskLen);
    }
}

This uses the very new std::countr_zero from the <bit> header, a C++20 feature. There are other ways to write that too, for example _tzcnt_u64. unsigned(N) is a bit unfortunate, but required for countr_zero, which refuses to work on signed types. Preferably this would be avoided by making N a constant of type size_t instead of #define-ing it, which I recommend anyway.
Using this trick, bitReversal has gone down from taking 17ms to 14ms (additionally, the memory used by the permutation vector is saved). It's a bit better, but nothing as great as improving the calculation of the roots of unity.
This is not the best way to do it. The if corresponds to a badly-predicted branch, and the memory access pattern is semi-random. There are various useful papers about the technique I used here and further improvements, for example practically efficient methods for performing bit-reversed permutation in C++11 on the x86-64 architecture.
The actual FFT
The real meat of the algorithm. Since you asked in the previous question not to bother with suggestions for different algorithms, I won't. However, I can still suggest a performance improvement: use SSE3. SSE2 is actually sufficient, but SSE3 adds the ADDSUBPD instruction which is handy for complex multiplication:
__m128d multiplyComplex(__m128d A, __m128d B)
{
    __m128d ARealReal = _mm_shuffle_pd(A, A, 0);
    __m128d AImagImag = _mm_shuffle_pd(A, A, 3);
    __m128d BRealImag = B;
    __m128d BImagReal = _mm_shuffle_pd(B, B, 1);
    return _mm_addsub_pd(_mm_mul_pd(ARealReal, BRealImag), _mm_mul_pd(AImagImag, BImagReal));
}

Which could be used like this:
void unorderedFFT2(complexVec& input, const complexVec& table) {
    std::size_t k = 2;

    while (k <= N) {
        for (std::size_t r = 0; r < N / k; r++) {
            for (std::size_t j = 0; j < k / 2; j++) {
                __m128d input0 = _mm_loadu_pd((double*)&input[r * k + j + k / 2]);
                __m128d input1 = _mm_loadu_pd((double*)&input[r * k + j]);
                __m128d twiddle = _mm_loadu_pd((double*)&table[N / k * j]);
                __m128d product = multiplyComplex(input0, twiddle);
                _mm_storeu_pd((double*)&input[r * k + j + k / 2], _mm_sub_pd(input1, product));
                _mm_storeu_pd((double*)&input[r * k + j], _mm_add_pd(input1, product));
            }
        }

        k *= 2;
    }
}

Casting those pointers looks scary, but it seems to be allowed according to the paragraph about Array Oriented Access of std::complex. reinterpret_cast could be used if you dislike C-style casts.
Admittedly, using SSE is not very beginner-friendly. But it does help significantly, the time of unorderedFFT went down to 70ms, the biggest improvement in absolute terms.
Using intrinsic functions like this requires including the relevant header, for example #include <tmmintrin.h> (or newer) in this case. Depending on the compiler, it may also require passing certain command line options (GCC and Clang need that, MSVC does not).