Truth Table Calculator In of my software engineering classes, I had to write a truth table calculator in the language of my choice.  This is what I came up with, and I would like to know what I should do better next time:

import copy
import sys

print("This program reads boolean equations and outputs the truth table.")
print("You may use the operators ~, ∨, and ∧, or")
print("their text equivalents, not, or, and and.")
print("You may use parenthesis, braces, and brackets to group your equations,")
print("and you must use single letters for your variable names, as 'a∨b'\n")

equation = input()
equation_copy = equation
equation_vars = []
equation = " " + equation + " "
equation = equation.replace("~", " not ")
equation = equation.replace("∨", " or ")
equation = equation.replace("∧", " and ")
equation = equation.replace("[", "(")
equation = equation.replace("]", ")")
equation = equation.replace("{", "(")
equation = equation.replace("}", ")")

for index in range(len(equation)):
    if equation[index] in " ()^":
        continue
    if equation[index + 1] in " ()" and equation[index - 1] in " ()" and equation[index] not in equation_vars:
        equation_vars.append(equation[index])

equation_vars.sort()

for equationIndex in equation_vars:
    sys.stdout.write(equationIndex + '\t')

print(equation_copy)

for equationIndex in range(pow(2,len(equation_vars))):
    vals = str(bin(equationIndex)).replace('0b','').zfill(len(equation_vars))

    for value in vals:
        sys.stdout.write(value + '\t')

    equation_vars_copy = copy.deepcopy(equation_vars)
    for index in range(len(equation_vars_copy)):
        equation_vars_copy[index] += " = " + vals[index]
        exec(equation_vars_copy[index])

    truthTable = "result = equation"
    truthTable = truthTable.replace('equation', equation)
    exec(truthTable)
    if result:print(1)
    else:print(0)

I have a working version at IdeOne.

Let's proceed top-down.  I'll suggest cleverer ways to do a bunch of stuff, but the review will culminate in the two most important points:

this is a huge security hole, and
you should organize your code into functions.

Introductory message

A good way to write a multiline string is using """the longstring""" syntax.

In this case, though, that introductory message could just as well be the docstring for the program.  You could use the special __doc__ variable to fetch the docstring by introspection.

"""This program reads boolean expressions and outputs the truth table.
You may use the operators ~, ∨, and ∧, or
their text equivalents, not, or, and and.
You may use parenthesis, braces, and brackets to group your expressions,
and you must use single letters for your variable names, as 'a∨b'"""

print(__doc__)

Note that "equation" is not the right word: you don't want a string with an equals sign.  You just want a "boolean expression".

Canonicalizing the expression

Doing string substitutions in multiple passes is bad practice.  In the best case, you waste a bit of effort making multiple passes over the string.  In the worst case, passing previously substituted text through another round could be incorrect — though fortunately that's not the case here.

I recommend doing the substitution in one pass using a regular expression.

REPLACEMENTS = {
   '~': ' not ',
   'v': ' or ',
   '^': ' and ',
   '[': '(',
   ']': ')',
   '{': '(',
   '}': ')',
}
expr = re.sub('|'.join(re.escape(sym) for sym in REPLACEMENTS.keys()),
              lambda sym: REPLACEMENTS[sym.group(0)],
              expr).strip()

Extracting the variables

This, too, can be simplified using a regular expression.  \b[A-Za-z]\b looks for any single letter bounded by word boundaries on both sides.

In addition, deduplication is more elegant using a set.

vars = sorted(set(re.findall(r'\b[A-Za-z]\b', expr)))

Generating the boolean values

This is a perfect job for itertools.product(), and in fact the documentation has just such an example:

product(range(2), repeat=len(vars))

Evaluating the expression

First of all, you should use eval(), not exec(), since you want to evaluate the expression and get its value, rather than running some code for its side-effect.  result = eval(expr) is better than exec('result = ' + expr).

Most importantly, though, calling eval() or exec() like that is a huge security hole — it lets the user execute arbitrary Python code.  For example, the user might enter this to be executed:

__import__('os').system('rm -rfi /')

The consensus is that Python can't be safely sandboxed.  We can try to do better by evaluating the expression in a scope that contains just the relevant variables.

NO_GLOBALS = {'__builtins__': {}}
for vals in product(range(2), repeat=len(vars)):
    locals = dict(zip(vars, vals))
    result = eval(expr, NO_GLOBALS, locals)

However, I suspect that even that won't be secure.  The lambda keyword is still available, for example, and that makes me nervous.  Ultimately, the only safe approach may be to write your own code to parse and evaluate the expression — Python itself is too powerful.

Suggested solution

As a final remark: the problem can be divided neatly into functions.  Naming these chunks of work make your code easier to follow.

"""This program reads boolean expressions and outputs the truth table.
You may use the operators ~, ∨, and ∧, or
their text equivalents, not, or, and and.
You may use parenthesis, braces, and brackets to group your expressions,
and you must use single letters for your variable names, as 'a∨b'"""

from itertools import product
import re

def canonicalize(expr):
    REPLACEMENTS = {
       '~': ' not ',
       'v': ' or ',
       '^': ' and ',
       '[': '(',
       ']': ')',
       '{': '(',
       '}': ')',
    }
    return re.sub('|'.join(re.escape(sym) for sym in REPLACEMENTS.keys()),
                  lambda sym: REPLACEMENTS[sym.group(0)],
                  expr).strip()

def extract_variables(expr):
    return sorted(set(re.findall(r'\b[A-Za-z]\b', expr)))

def truth_table(expr):
    expr = canonicalize(expr)
    vars = extract_variables(expr)
    NO_GLOBALS = {'__builtins__': {}}

    # Print header
    print('\t'.join(vars + [expr]))

    # Print body
    for vals in product(range(2), repeat=len(vars)):
        locals = dict(zip(vars, vals))
        result = eval(expr, NO_GLOBALS, locals)
        print('\t'.join([str(v) for v in vals] + [str(result)]))

def prompt_expr():
    print(__doc__)
    print()
    return input()

if __name__ == '__main__':
    truth_table(prompt_expr())


List of Files in any Folder - Scala I have written a code to count the number of files in a folder and if there are any folder inside folder it will count the files in that folder too.

import java.io.File
class GetFileCount {
  def getFilesCount(dir: String):Int = {
    val file = new File(dir)
    require(file.exists() && file.isDirectory)
    def inDirectoryFiles(inDir:List[File]):Int={
      if(inDir.filter(_.isDirectory).nonEmpty)
        inDir.filter(_.isFile).length + inDirectoryFiles(inDir.flatMap(_.listFiles.toList))
      else
        inDir.filter(_.isFile).length
    }
    file.listFiles.filter(_.isFile).toList.length + inDirectoryFiles(file.listFiles.filter(_.isDirectory).toList)
  }
}

I feel like it could be done more nicely.

If you are allowed to, I would strongly suggest using https://github.com/pathikrit/better-files to do this. The code would look like :

import better.files._
class GetFileCount {
  def getFilesCount(dir: String):Int = {
    File(dir).glob("**").count(_.isRegularFile)
  }
}

If you can't or won't use better-files, you can probably improve your code using just nio:

import java.nio.file.{ FileSystems, Paths, Path, Files }
import java.util.function.Predicate
import java.util.stream.Stream
object GetFileCount{
  val regularFilePredicate = new Predicate[Path] {
    override def test(path: Path) = Files.isRegularFile(path)
  }
}
class GetFileCount {
  import GetFileCount._
  def getFilesCount(dir: String): Int = {
    val path: Path = FileSystems.getDefault.getPath("/tmp/d1")
    val filter: Stream[Path] = Files.walk(path).filter(regularFilePredicate)
    filter.toArray.length
  }
}


Generate next available filename based on current date I have a bit of code that I would like your collective opinions on. Currently, my working code has two return statements. I'm not sure how I feel about having multiple points of exit, however, the alternative introduces the inclusion of else, which looks like it will eventually be harder to maintain if more logic is added to the code later on.

My current code (Feel free to comment on other aspects as well):

function generateFileName() {
    var k = 0;
    var today = new Date();
    today = (today.getMonth() + 1).toString() + '-' + today.getDate().toString()
            + '-' + today.getFullYear().toString();
    while (true) {
        if (!fs.existsSync('./'+ today + '.pdf')) {
            return './' + today + '.pdf';
        }

        if (!fs.existsSync('./' + today + '(' + k + ').pdf')) {
            return './' + today + '(' + k + ').pdf';
        }

        k++;
    }
}

As you see, here there are two different return statements. If the file does not exist in the system, a file will be generated with the name 5-19-2017.pdf, i.e., the current date. If there is already a file named that, the file will be named 5-19-2017 (0).pdf, then 5-19-2017(1).pdf, and so on...

This code works just fine, however - I am wondering if maybe a single point of exit might be better, such as :

function generateFileName() {
    var k = 0;
    var today = new Date();
    today = (today.getMonth() + 1).toString() + '-' + today.getDate().toString() + '-' +today.getFullYear().toString();
    var result = today;
    var stop = false;
    while (!stop) {
        if (fs.existsSync('./'+ today + '.pdf')) {
            if (fs.existsSync('./' + today + '( ' + k + ' ).pdf')){
                k++;
            } else {
                result = today + '( ' + k + ' )';
                stop = true;
            }
        }
        else {
            stop = true;
        }
    }
    return result;
}

Now, the code has a single point of exit, however the added else statements make the code unbearable to read, in my opinion.

Thoughts?

Your code can be made much simpler:

var basename = today;
var filename;
for (var k = 0; fs.existsSync(filename = './' + basename + '.pdf'); k++) {
    basename = today + ' (' + k + ')';
}
return filename;


Mapper and service call I have an entity called Travel which has list of costCenter. I have created a code for handling CRUD operation for cost centers

protected void assignCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long detailsId) throws BusinessException {
    CostCenter costCenter = null;
    if (costCenterDTO.getCostCenters() != null) {
        Center center = commonDao.find(costCenterDTO.getCostCenters().getId());
        costCenter = costCenterDTOMapper.mapToDTO(costCenterDTO.getCostCenters(), travel, detailsId, center);
        if (costCenter != null && !travel.getCostCenters().contains(costCenter)) {
            travel.getCostCenters().add(costCenter);
        }
    } else {
        travel.setCostCenters(travel.getCostCenters().stream().filter(costCenter1 -> !(costCenter1.getDetailId().equals(detailsId))).collect(Collectors.toList()));
    }
}

And in the mapper

 public CostCenter mapToCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long claimDetail, Center center) throws BusinessException {
        CostCenter costCenter = new CostCenter();
        CostCenter costCenterToUpdate = travel.getCostCenters()
                .stream()
                .filter(costCenter1 -> costCenter1.getDetailId().equals(claimDetail))
                .findFirst()
                .orElse(null);
        if (costCenterToUpdate == null) {
            costCenter.setCenter(center);
            costCenter.setTravel(travel);
            costCenter.setDetailId(claimDetail);
        } else {
            costCenterToUpdate.setCenter(center);
            return costCenterToUpdate;
        }
        return costCenter;
    }

The functionality is that if new CostCenter is assigned, I fetch Center from db which was assigned and pass to mapper. In mapper I call travel.getCostCenters to see if there is no costCenter already assigned with same detail id if there is I just update it, if there is no I create new and return, and add to list if doesn't exist. Also if CostCenterDTO comes as null then I remove from list costCenter by detailId. My question is how I could refactor it because it doesn't look tidy?

I think the mapper looks ok, but in the cost centers management method, you violate encapsulation by directly manipulating the travel's cost centers list.

I'd rather leave the list management to the travel class:

class Travel {
    ...
    void addCostCenter(CostCenter c) {
       ...
    }

    void removeCostCenter(CostCenter c) {
       ...
    }

    boolean containsCostCenter(CostCenter c) {
        ...
    }

    CostCenter findByDetails(Long detailsId) {
        ....
    }
}

... and use it like this:

protected void assignCostCenter(CostCenterDTO costCenterDTO, Travel travel, Long detailsId) throws BusinessException {
    // note: moved outside declaration of CostCenter to smaller scope
    if (costCenterDTO.getCostCenters() != null) {
        Center center = commonDao.find(costCenterDTO.getCostCenters().getId());
        CostCenter costCenter = costCenterDTOMapper.mapToDTO(costCenterDTO.getCostCenters(), travel, detailsId, center);
        // can costCenterDTOMapper.mapToDTO actually return null? If not, declare it like that and
        // remove the ugly null-check
        if (costCenter != null && !travel.containsCostCenter(costCenter)) {
            // depending on the business logic, you might also consider
            //    travel.addCostCenterIfNotExists(costCenter)
            // and skip the if
            travel.addCostCenter(costCenter);
        }
    } else {
        travel.removeCostCenter(travel.findByDetails(detailsId));
    }
}


C++ EventHandler Class I need to have some thread-safe event handler (like C#'s EventHandler), so I made one. Here is my event handler class and sample code (compiled with VS2013). Is this code OK to be operated in multithreaded environment?

#include <functional>
#include <memory>
#include <atomic>
#include <list>
#include <conio.h>

////////////////////////////////////////////////////////////////////////////
// Event Handler class

template <typename _EventArg>
class EventHandler
{
public:
    EventHandler()
    {
        m_flag.clear();
    }

    EventHandler(const EventHandler& other)
    {
        m_flag.clear();
        m_ptr = other.m_ptr;
    }

    EventHandler& operator= (const EventHandler& other)
    {
        m_flag.clear();
        m_ptr = other.m_ptr;
        return *this;
    }

    virtual ~EventHandler()
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
    }

    template <typename _Type>
    EventHandler(void (_Type::*memFn)(_EventArg), _Type *obj)
    {
        m_flag.clear();
        m_ptr.reset(new std::function<void(_EventArg)>(std::bind(memFn, obj, std::placeholders::_1)));
    }

    void operator() (_EventArg e)
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
        if (m_ptr.get()) { (*m_ptr)(e); }
        m_flag.clear(std::memory_order_release);
    }

    bool operator== (const EventHandler& other)
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
        bool equal = (m_ptr == other.m_ptr);
        m_flag.clear(std::memory_order_release);
        return equal;
    }

private:
    std::atomic_flag m_flag;
    std::shared_ptr<std::function<void(_EventArg)>> m_ptr;
};

template <typename _Type>
class IEvent
{
public:
    IEvent()
    {
        m_flag.clear();
    }

    virtual ~IEvent()
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
    }

    void operator+= (const EventHandler<_Type>& handler)
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
        m_list.push_back(handler);
        m_flag.clear(std::memory_order_release);
    }

    void operator-= (const EventHandler<_Type>& handler)
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
        m_list.remove(handler);
        m_flag.clear(std::memory_order_release);
    }

    void operator() (_Type arg)
    {
        while (m_flag.test_and_set(std::memory_order_acquire));
        for (auto& f : m_list) { f(arg); }
        m_flag.clear(std::memory_order_release);
    }

private:
    std::atomic_flag m_flag;
    std::list<EventHandler<_Type>> m_list;

    // make non-copyable
    IEvent(const IEvent&);
    IEvent& operator= (const IEvent&);
};

////////////////////////////////////////////////////////////////////////////

// Event source class
class CSource {
public:
    void RaiseEvent(int value)
    {
        MyEvent(value); // Raise event
    }

    IEvent<int> MyEvent;
};

// Event receiver class
class CReceiver {
public:
    void MyHandler1(int e)
    {
        printf("CReceiver::MyHandler1 called with value %d\n", e);
    }

    void MyHandler2(int e)
    {
        printf("CReceiver::MyHandler2 called with value %d\n", e);
    }
};

void main()
{
    CSource source;
    CReceiver receiver;

    // define event handler
    EventHandler<int> handler1 = EventHandler<int>(&CReceiver::MyHandler1, &receiver);
    EventHandler<int> handler2 = EventHandler<int>(&CReceiver::MyHandler2, &receiver);

    // add event handler
    source.MyEvent += handler1;
    source.MyEvent += handler2;

    // raise event with value 1234!
    source.RaiseEvent(1234);

    // remove event handler
    source.MyEvent -= handler1;
    source.MyEvent -= handler2;

    _getch();
}

RAII

Whenever you need to acquire a resource, hold it for a while, and then release it deterministically - think resource acquisition is initialization. RAII! When I look at your equality comparison:

bool operator== (const EventHandler& other)
{
    while (m_flag.test_and_set(std::memory_order_acquire));
    bool equal = (m_ptr == other.m_ptr);
    m_flag.clear(std::memory_order_release);
    return equal;
}

We're really doing two things. We're acquiring and releasing a lock on m_flag, and then we're doing the equality comparison. So let's make that a lot more explicit.

std::lock_guard is an RAII lock, templated on a BasicLockable - which is just a class that supports a lock() and unlock(). So instead of holding a std::atomic_flag member, we can hold a member of type:

struct FlagMutex {
    void lock() {
        while (m_flag.test_and_set(std::memory_order_acquire));
    }

    void unlock() {
        m_flag.clear(std::memory_order_release);
    }

    /*
     * note that you were not initializing your flag correctly!
     */
    std::atomic_flag m_flag = ATOMIC_INIT_FLAG;
};

And then our equality operator becomes:

bool operator== (const EventHandler& other) {
    std::lock_guard<FlagMutex> lk(m_mutex);
    return m_ptr == other.m_ptr;
}

Keep that in mind throughout. Furthermore, if you do it this way, then FlagMutex can become a template argument for EventHandler so you can actually test the assertion that a std::atomic_flag is faster than a std::mutex.

Generalize Your Class

Right now, your event handler is constructible with:

template <typename _Type>
EventHandler(void (_Type::*memFn)(_EventArg), _Type *obj)

But why limit to just member functions? Your class has a std::function member. That can support free functions. Just generalize it.

Additionally, prefer make_shared to just new:

using FuncType = std::function<void(_EventArg)>;

template <typename F,
          typename = std::enable_if_t<
              std::is_constructible<FuncType(F&&)>::value
          >>
EventHandler(F&& func)
{
    m_ptr = std::make_shared<FuncType>(std::forward<F>(func));
}

The user can pass in a bind expression if they want, or a lambda, or any number of other things. Just lets you do more things.

Also _EventArg is a reserved name.


How to handle JSON's undefined value in a better way? I'm working on Google Book API to post a book's basic information. I notice that some of the books will always miss a value or two under volumeInfo, like this book (Fiddle) that has no publisher value. Do I need to repeat if [object] undefined statement several times for the missing values (e.g Publisher,Average Rating,Rating Count)? Can I do it in a more concise way?

 $.ajax({

    url: 'http://query.yahooapis.com/v1/public/yql?q=SELECT%20*%20FROM%20json%20WHERE%20url%3D%22https%3A%2F%2Fwww.googleapis.com%2Fbooks%2Fv1%2Fvolumes%3Fq%3Disbn%3A1879505215%22&format=json&diagnostics=true&callback=?',
    dataType: 'json',
    success: function (data) {

 $(data.query.results.json.items).each(function (index, item) {

    var isbn10;

 $.each(item.volumeInfo.industryIdentifiers, function(index, item) {

    if (item.type === "ISBN_10") {
    isbn10 = item.identifier;
    return false;
    }

    });
    var Rating = item.volumeInfo.averageRating;
    var RatingCount = item.volumeInfo.ratingsCount;
    var item_html = '';

    item_html += '

<img src="' +item.volumeInfo.imageLinks.thumbnail+'

Author: ' + item.volumeInfo.authors + '

Publisher ' + item.volumeInfo.publisher + '

PublishDate: '+item.volumeInfo.publishedDate+'

Category: ' + item.volumeInfo.categories + '

Page Count: ' + item.volumeInfo.pageCount +'

ISBN: ISBN10_' + isbn10 +'

Description:  ' + item.volumeInfo.description +'
';

    if (Rating != undefined) {
    item_html += '
Average Rating:' + Rating +'
';
    }

    if (RatingCount != undefined) {
    item_html += '
Rating Count:' + RatingCount +'
';
    }

    var title_html ='' +item.volumeInfo.title + '';

    $('#bookhead').append(title_html);

    $('#bookdetail ul').append(item_html);

    });

    },
    error: function () {}

    });

You can define a hash of the possible field names and loop through them:

var fieldTitles = {
    description: "Description",
    Rating: "Average Rating",
    RatingCount: "Rating Count"
};

var volumeInfo = item.volumeInfo;
for (var key in fieldTitles) {
    if (volumeInfo[key]) {
        item_html += '
' + fieldTitles[key] + ':' + volumeInfo[key] +'
';
    }
}

Additional notes:

store item.volumeInfo to a local variable so you don't have to access it through item object all the time.
Most of the time it's better and simpler to check for existence of a value with if (someVar) rather than comparing directly with undefined.


Implementation of fixed size hash map I'm trying to brush up on my algorithms and data structures and wrote a fixed sized hash map assuming both key and value will be only integers. This is a significant simplification of the real case and is supposed to be so. Any comments regarding bugs, best practices, style is gladly appreciated.

#include <vector>
#include <iostream>
#include <exception>
#include <stdexcept>
#include <assert.h>
#define SIZE 10

class HashEntry {
public:
    int _k;
    int _v;
    HashEntry(int k, int v) : _k(k), _v(v) {};
};

class HashMap {
private:
    std::vector<HashEntry> buckets[SIZE];
    int hashFunc(int key);
    std::vector<HashEntry>& getBucket(int key);

public:
    bool keyExists(int k);
    HashEntry get(int k);
    bool put(int k, int v);
    bool remove(int k);
};

int HashMap::hashFunc(int key) {
    return key % SIZE;
}

std::vector<HashEntry>& HashMap::getBucket(int key) {
    if (key < 0) {
        throw std::runtime_error("key should be >= 0");
    }
    return buckets[hashFunc(key)];
}

HashEntry HashMap::get(int k) {
    std::vector<HashEntry>& bucket = getBucket(k);
    for (HashEntry entry : bucket) {
        if (entry._k == k) {
            return entry;
        }
    }
    throw std::runtime_error("Key not found");
}
bool HashMap::keyExists(int k) {
    std::vector<HashEntry>& bucket = getBucket(k);
    for (HashEntry entry : bucket) {
        if (entry._k == k) {

            // key already exists
            return true;
        }
    }
    return false;
}

bool HashMap::put(int k, int v) {
    std::vector<HashEntry>& bucket = getBucket(k);
    if (keyExists(k)) return false;
    bucket.push_back(HashEntry(k, v));
    return true;
}

bool HashMap::remove(int k) {
    std::vector<HashEntry>& bucket = getBucket(k);
    if (!(keyExists(k))) return false;
    for (auto itr = bucket.begin(); itr != bucket.end(); ++itr) {
        HashEntry entry = static_cast<HashEntry>(*itr);
        if (entry._k == k) {
            bucket.erase(itr);
            return true;
            break;
        }
    }
    return false;
}

int main() {
    HashMap map;
    for (int i=0; i < 10; ++i) {
        assert(map.put(i, i*10));
    }
    for (int i=0; i < 10; ++i) {
        assert(map.keyExists(i) == true);
    }
    assert(map.keyExists(0) == true);
    assert(map.get(0)._v == 0);
    assert(map.remove(0) == true);
    assert(map.remove(0) == false);
    assert(map.keyExists(0) == false);

    bool exceptionThrown = false;
    try {
        map.get(0);
    } catch (std::exception &e) {
        exceptionThrown = true;
    };
    assert(exceptionThrown);
    return 1;
}

I would nest HashEntry inside of HashMap as just Entry since it's not very likely to ever stand on its own.

The _ prefix is a bit dangerous as the standard reserves the underscore prefix in a few situations (though not this situation). Still though, it's a bit awkward for it to be there not only in case of bad habits, but also because it serves no purpose. And, map.get(3)._k reads a little oddly.

I would call them key and value. It's just a few more characters to type, and it makes the code read a lot clearer (map.get(3).key).

hashFunc is a bit of an odd name. I would just call it hash instead since it's clearly a function.

std::invalid_argument fits a bit better in getBucket than runtime_error does. The more the type of an exception can tell you, the better (in general -- don't take it to an extreme).

for (HashEntry entry : bucket) {

It probably doesn't matter in practice since HashEntry's are fairly cheap, but this creates a copy of each bucket entry. If HashEntry weren't guaranteed to be such a trivial type, this could be devastating to performance. It should be for (const HashEntry& entry : bucket) {.

Any method that doesn't permute that map should be const (keyExists, get, getBucket). Const correctness is an important part of C++, and it should always be kept in mind when designing/implementing classes.

Why does main return 1? It seems the flow that reaches main is a successful execution, which means that the return value should be 0 (or EXIT_SUCCESS).

You do a lot of the same "loop over and find the correct bucket slot" logic. That should be pulled into a function. That can be done by either making a Bucket type that has the concept searching built in, or, more simply, just write a function that loops over a vector<HashEntry> and returns an iterator to the HashEntry (or end()).

When possible, constants in the form of macros should typically be avoided in favor of const constants. const int SIZE = 10 -- or, since it's being used an array size/index, you might find const std::size_t SIZE = 10; more fitting (I would just stick with an int).

Macros do not have a type, and their names are typically lost as debugging symbols. A const variable has either of those drawbacks, and it also doesn't sacrifice anything.

Since getBucket is a very straightforward constant time operation, it doesn't really matter, but containers activate my super picky mode with regards to performance :).

Try to avoid doing the same work twice. For example, consider your implementation of put. It calls getBucket and it calls keyExists, which behind the scenes calls getBucket. This implies that it might be good to have an additional method behind the scenes and leverage that for checking if keys exists:

bool HashMap::keyExists(const std::vector<HashEntry>& bucket, int k) const {
    for (const HashEntry& entry : bucket) {
        if (entry.key == k) {
            return true;
        }
    }
    return false;
}

bool HashMap::keyExists(int k) const {
    return keyExists(getBucket(k), k);
}

bool HashMap::put(int k, int v) {
    std::vector<HashEntry>& bucket = getBucket(k);
    if (keyExists(bucket, k)) return false;
    bucket.push_back(HashEntry(k, v));
    return true;
}

While I'm being a bit performance paranoid, it would also be better to call getBucket after keyExists in your current implementation. That way, for the case when keys don't exist, you don't bother doing the work of getting the bucket.

HashEntry entry = static_cast<HashEntry>(*itr);

Is this cast necessary? I can't imagine that it is. Also, HashEntry entry = static_cast<HashEntry>(*itr) should be const HashEntry& entry = ... (and really it should be const HashEntry& entry = *itr; or const auto& entry = *itr;).

I also wouldn't bother deferencing the iterator:

if (itr->key == k) {
    bucket.erase(itr);
    return true;
}

A break; immediately following a return is just noise. Since the function has already been exited, the loop will have been too.

In the spirit of more performance paranoia, it would be nice to allow a single operation for both checking the existence of a key and retrieving the value of it if it does exist. This allows you to avoid the same loop over the bucket twice.

Something like bool find(int key, int& value); or std::pair<bool, int> find(int key) would do the trick (provided it was implemented to take advantage of the ability to do only a single pass).

If you had the concept of an iterator, you could also do this in the form of having a find() that returns an iterator either to the element or to the end(). It's actually a bit icky to do this without iterators since you don't really have anything meaningful to return if you want to avoid a reference parameter. You would have to have either a std::pair<bool, int*> will a null value, or you'd have to have a junk value in the int component of a std::pair<bool, int>. With an int, the junk value approach works pretty well since you can just throw a 0 in it, but if this were to become generic, it becomes more of a concern.

The key in HashEntry should be const since it should never change after construction. Currently this doesn't matter, but if you were ever to return a reference to the HashEntry so that the value could be changed, it would be necessary to maintain the contract of the map. This is why the value_type of std::map<K, V> and std::unordered_map<K, V> is std::pair<const K, V> instead of std::pair<K, V>.

You probably realize this already, but this HashMap is not really a hash map in the strict sense. In particular, it does not honor the expected O(1) lookup and insertion run times. Instead, all of the meaningful operations are all O(n). The unbounded linear scans over the vectors kill it.

There are essentially three main components to a hash table: look up, insertion, and deletion.

The run time of insertion is of course T(lookup) + T(insertion), and with deletion, it's T(lookup) + T(deletion). This means that lookup, insertion, and deletion all have to be constant time operations for insertion and deletion to be constant time.

This means that hashing must be constant time, and it means that whatever is done in the case of a collision must be constant time. In your case, the buckets grow indefinitely, so the lookup across them is linear time. Provided randomly distributed data (which is likely to happen, but bear with me), if you have n keys and S buckets, each bucket will have n/S items in it. A linear scan over that is O(n/S) = O(1) time for constant S.

The easiest solution to this is to make S non-constant. How you do this actually turns out to be relatively important since pathological cases can exploit most approaches. For example, ensuring S >= N/c for some constant c gives you a decent average case, but if keys are not distributed well across buckets, you're essentially back to linear scans.

Another approach might be to decide that whenever a bucket goes over a certain size, double the size of buckets. This would work well at first glance since it would mean resizing and rehashing the map rarely, but once again, pathological cases could purposely target the same bucket so a relatively small amount of keys could cause a hugely sparse map. (Provided the bucket size case was constant, a linear number of keys could cause quadratic growth in the number of buckets).

So, where am I going with this? Go find a well known, well understood, commonly used approach to implementing a hash map, and implement it. I might do linear probing since it's pretty easy to implement and it's relatively drawback free. Other viable approaches can be found on Wikipedia, in a well known implementation (e.g. clang or gcc's unordered_map, or Oracle Java's java.util.HashMap), or in an algorithms text book.


Custom output stream that acts like std::cout I wrote a simple output stream object that from the user point of view should act exactly like the classic std::cout. I need it because I want to customize the printing behavior of my program.

Can you review my code? Do you see anything wrong in it?

File Utils.h:

namespace Utils
{
   static std::ostream& Log;
}

File Utils.cpp:

struct MyStream : std::ostream, std::streambuf
{
public:
   MyStream() : std::ostream(this) { }
   std::streambuf::int_type overflow(std::streambuf::int_type c)
   {
      std::cout << c;
      // My custom operations
      return 0;
   }
};
std::ostream& Utils::Log = MyStream();

Construction

The construction of a MyStream object will proceed like this:

Construct std::ostream part with pointer to not-yet-constructed std::streambuf part.
Construct std::streambuf part.
Construct MyStream part.

Thus, suggestion: Reorder inheritance list like this:

struct MyStream : std::streambuf, std::ostream

override specifier

Nitpick: Add override specifier for overflow() function:

std::streambuf::int_type overflow(std::streambuf::int_type c) override

struct vs class

Another nitpick. I'm noticing that you are using struct instead of class. It is subjective, but I like to distinguish between struct and class like this:

If the resulting object will mostly be used by calling its methods - it's a class.
If the resulting object will mostly be used by accessing its fields - it's a struct.

Thus, for MyStream I would use class instead of struct. But, as I said, it is subjective.


Python Desktop Activity Log App Code Review

I've recently (within the last month) started learning Python and am now building a few basic apps. One of the apps I've created is a sort of desktop monitoring tool, that attempts to build a list of all applications used within a specified time period.

Process -

Check whether the current time falls between the monitoring period
Create a record of what is currently being used
Save that record / entry to the log file
Repeat the process until the current time falls outside of the monitoring period
Once outside of monitoring period the Log file is renamed
End

I'd like to know whether some of you could spare a few minutes to review my code - Commenting on it's efficiency (or lack of), thoughts on how it could be improved and more. I do have some additional features to add (currently working on a UI). Right now I'm just looking comments.

Thanks in advance and apologies if I've posted in the wrong area.

from datetime import datetime
import os
import time
import psutil
import win32process

# When to start monitoring
start_time = datetime.now().strftime("%H:%M:%S")
# End Time - When to end monitoring
end_time = "08:55:00"
# Directory - Where to store activity log
directory = r"C:\Users\username\Documents"

def activity_log(s_time, e_time, dir_t):

    # act_app is used to return the active app/window
    act_app = ""

    # entry number/id
    num = 1000

    # monitoring start time
    p_start_time = s_time

    # monitoring end time
    p_end_time = e_time

    # File path / File name
    log_path = dir_t + "\\Log.csv"

    # Look into creating outer look to review current time or use out IF condition
    while p_start_time <= datetime.now().strftime("%H:%M:%S") <= p_end_time:

        # New record will be create IF different from the previous record
        if act_app != GetWindowText(GetForegroundWindow()) and act_app != " ":

            act_app = str.replace(GetWindowText(GetForegroundWindow()), " = ", ", ")
            today = datetime.today()
            num = num + 1
            pid = win32process.GetWindowThreadProcessId(GetForegroundWindow())
            app_name = psutil.Process(pid[-1]).name()

            with open(log_path, "a", encoding="utf-8") as LogFile:
                LogFile.write(f"{num}, {str.replace(act_app, ''''''', ' - ').replace(',', '')}, "
                              f"{app_name}, {today.strftime('''%d/%m/%Y''')}, "
                              f"{today.strftime('''%H:%M:%S''')} \n ")

        time.sleep(2)
        # Sleep/wait two seconds before attempting to create a new entry
    else:
        if os.path.isfile(log_path):
            # need to check if file exists before opening file
            with open(log_path, "a") as LogFile:
                LogFile.write(f"\n\n\n---- Desktop Activity Between {p_start_time} and {p_end_time} ----")
                # rename log file at the end of monitoring period
            os.rename(log_path, f"{dir_t}\\DesktopJ_ActLog_{datetime.now().strftime('''%d%m%Y_%H%M%S''')}.csv")
        else:
            print("No log created.")
            return "No log created"

activity_log(start_time, end_time, directory)

Welcome to CodeReview. Great first question!

Stringly-typed dates

Rather than holding onto dates as strings, you should be holding onto them as actual datetime objects. So this:

# When to start monitoring
start_time = datetime.now().strftime("%H:%M:%S")
# End Time - When to end monitoring
end_time = "08:55:00"

should instead be

# When to start monitoring
start_time = datetime.now()
# End Time - When to end monitoring
# This assumes that we are running early in the morning of the same
# day as the end_time
end_time = datetime.now().replace(hour=8, minute=55)

and

p_start_time <= datetime.now().strftime("%H:%M:%S") <= p_end_time:

should be

p_start_time <= datetime.now() <= p_end_time:

Parametric paths

# Directory - Where to store activity log
directory = r"C:\Users\username\Documents"

should not be hard-coded. Instead, accept it as a command-line argument, or an environmental variable, or store it in a config file.

Path management

# File path / File name
log_path = dir_t + "\\Log.csv"

is much easier with pathlib:

from pathlib import Path
# ...
dir_t = Path(...)
log_path = dir_t / 'Log.csv'

Logging

Strongly consider using the actual logging module instead of rolling your own. This will lend you more flexibility, and access to a more feature-rich logging framework. You can do everything you're doing now - have a file handler; have a custom formatter with your own chosen fields; etc.

In-place addition

        num = num + 1

can be

        num += 1


Three-function calculator I'm pretty new to C# and was just wondering if there is a better way I can write this calculator that I made:

        using System;
        using System.Collections.Generic;
        using System.Linq;
        using System.Text;
        using System.Threading.Tasks;

        namespace AnthonyCalculator
               {
        class Program
          {
        static void Main(string[] args)
        {
            double num01;
            double num02;
            int userChoice = 1;

            Console.WriteLine ("What is your first number?: ");
            num01 = double.Parse (Console.ReadLine());

            Console.WriteLine ("\nPlease select one of the following (1-4)");
            Console.WriteLine ("1 - Add");
            Console.WriteLine ("2 - Subtract");
            Console.WriteLine ("3 - Divide");

            Console.Write("Select number 1-3: ");
            userChoice = int.Parse (Console.ReadLine ());

            switch (userChoice)
            {
                case 1:
                    Console.WriteLine ("\nAddition Selected: ");
                    Console.Write ("\nSecond number: ");
                    num02 = double.Parse (Console.ReadLine());
                    Console.WriteLine ("\nTotal of " + num01 + " + " + num02 + "      = " + (num01 + num02));
                    break;

                case 2:
                    Console.WriteLine ("Subtract Selected: ");
                    Console.Write("\nSecond number: ");
                    num02 = double.Parse(Console.ReadLine());
                    Console.WriteLine("\nTotal of " + num01 + " - " + num02 + " = " + (num01 - num02));
                    break;

                case 3:
                    Console.WriteLine("Division Selected: ");
                    Console.Write("\nSecond number: ");
                    num02 = double.Parse(Console.ReadLine());
                    Console.WriteLine("\n Total of " + num01 + " / " + num02 + " = " + (num01 / num02));
                    break;

                default:
                    Console.WriteLine("Sorry, incorrect input");
                    break;
            }
            Console.ReadLine();

        }
    }
}

I'll blame the extra-weird broken indentation on copy/paste difficulties, and ignore that.

There are as many ways to write a calculator as there are programmers willing to write one. So I'm not going to suggest other ways, instead I'm going to hint at how you could improve the flexibility of what you've got.

The first concern you have, is getting user input. Your code works when the sky is blue and the user isn't being too clever.

  QA engineer walks into a bar. Orders a beer. Orders 0 beers. Orders 999999999 beers. Orders a lizard. Orders -1 beers. Orders a sfdeljknesv.

  https://twitter.com/JimFraeErskine/status/527564769302433792

What happens if you play the QA engineer here?

Console.WriteLine ("What is your first number?: ");
num01 = double.Parse (Console.ReadLine());

Your code is making the assumption that the user will enter a valid double! Look into TryParse, and write a separate function with the sole purpose of turning a string into a double. I'd actually make that function return a double? (or Nullable<double>, so that the calculator knows the number isn't valid when that function returns a null).

Console.WriteLine ("\nPlease select one of the following (1-4)");
Console.WriteLine ("1 - Add");
Console.WriteLine ("2 - Subtract");
Console.WriteLine ("3 - Divide");

Console.Write("Select number 1-3: ");

That's ...rather interesting. It looks like at one point it was intended to support 4 - Multiply, but the idea was dropped and now 4 is an invalid input despite the instructions at the top saying (1-4), and the instructions at the bottom saying 1-3!

How about you take the concern of creating the menu out of there, find a way to encapsulate each operation into its own strategy object, and generate the menu based on how many available strategies you have?

Say you had an enum:

public enum CalculatorOperation
{
    Add,
    Subtract,
    Divide,
    Multiply
}

You could then "map" each enum to a function taking two double values and returning another - that's a Func<double,double,double>:

var strategies = new Dictionary<CalculatorOperation,Func<double,double,double>>
{
    { CalculatorOperation.Add,      (x, y) => x + y },
    { CalculatorOperation.Subtract, (x, y) => x - y },
    { CalculatorOperation.Divide,   (x, y) => x / y },
    { CalculatorOperation.Multiply, (x, y) => x * y },
};

What just happened here? You've given yourself the tools to eliminate that switch block - given userChoice is parsed into a CalculatorOperation instead of just an int, you could do this:

var result = strategies[userChoice](num01, num02);

How does that work? Delegates, my friend. And generics. But delegates, my friend. You see, this:

(x, y) => x * y

Is shorthand for:

delegate(double x, double y) { return x * y; }

Which could just as well be this:

private double Multiply(double x, double y)
{
    return x * y;
}

And, if you had one such method for each operator, you could just as well define your dictionary like so:

var strategies = new Dictionary<CalculatorOperation,Func<double,double,double>>
{
    { CalculatorOperation.Add, Add },
    { CalculatorOperation.Subtract, Subtract },
    { CalculatorOperation.Divide, Divide },
    { CalculatorOperation.Multiply, Multiply },
};

Because the signature double Multiply(double x, double y) matches that of a delegate taking two double parameters and returning a double - that's really all delegates are: functions that you can encapsulate, and pass around as parameters, or store inside a dictionary to execute later.

You could then generate the menu items simply by iterating the dictionary keys.

Now, you'll still have to worry about division by zero and overflows (what happens if you add double.MaxValue and double.MaxValue?), and then separate the concerns.

One of the most important principles, IMO, is the Single Responsibility Principle. Your current code has user input/feedback, validation(?) and the actual logic intertwined, which makes it harder to maintain/extend than it needs to be.

Identify what the concerns are, and extract them into their own methods; monolithic blocks of code are the easiest to write, but the hardest to maintain.


TPL Task tracker which keeps track of tasks This class keeps track of tasks which are added and tries to remove them after certain delay. I tried doing it with CustomTaskScheduler but didn't get much in return.

Here is my related question on SO.

I would be glad to know if there are other ways to track tasks.

public class TaskCompletedNotifier
{
    public delegate void AllTasksCompletedHandler(object sender);
    public delegate void TaskStartedHandler();
    public event AllTasksCompletedHandler AllTasksCompleted;
    public event TaskStartedHandler TaskStarted;

    private object syncLock = new object();
    private SynchronizedCollection<Task> tasksCollection;
    private bool isTaskStartedNotified;
    private readonly uint delay;

    public TaskCompletedNotifier(uint delayBeforeRemovingTasks)
    {
        tasksCollection = new SynchronizedCollection<Task>();
        delay = delayBeforeRemovingTasks;
    }

    public void Add(Task task)
    {

        if (!isTaskStartedNotified)
        {
            isTaskStartedNotified = true;
            TaskStarted?.Invoke();
        }

        task.ContinueWith(t =>
        {
            RemoveTask(t);
        });

        tasksCollection.Add(task);
    }

    private async void RemoveTask(Task task)
    {
        await Task.Delay(300);
        await Task.Run(() =>
        {
            tasksCollection.Remove(task);

            if (tasksCollection.Count == 0)
            {
                isTaskStartedNotified = false;
                AllTasksCompleted?.Invoke(tasksCollection);
            }
        });
    }
}

I don't think you want a TaskScheduler and this is a better idea.  Creating a custom TaskScheduler isn't for the faint of heart and the ThreadPoolTaskScheduler does a lot of things and it's better to just let it do it's own thing.

Your code is headed down the correct path but I'd make a couple of changes.  One for the delay I wouldn't use a uint I'd use a TimeSpan.  That easier to understand and can use Factory method like TimeSpan.FromMilliseconds to indicate what is the delay.  I don't like passing in numbers.  Is it milliseconds? seconds?  Also from your code you are not using the delay.

Also why create delegates for the events and not just use the standard EventHandler? Also when sending out the source of the event you are sending our your collection.  This seems like a very bad idea.  What if someone would clear your collection or add to it outside your class?

You have a synclock that you are not using.  Using the SynchronizedCollection wouldn't require a lock. But that being said I would probably ditch the SyncronizedCollection.  That will do a lot more locking than what is required for this application.  I would just use a HashSet and and lock on that since it's going to be internal.

This now becomes the class and the constructor

public class TaskCompletedNotifier
{
    public event EventHandler AllTasksCompleted;
    public event EventHandler TaskStarted;
    private readonly ISet<Task> _tasksCollection = new HashSet<Task>();
    private readonly TimeSpan _delay;

    public TaskCompletedNotifier(TimeSpan delayBeforeRemovingTasks)
    {
        if (delayBeforeRemovingTasks.TotalMilliseconds < 1)
        {
            throw new ArgumentOutOfRangeException(nameof(delayBeforeRemovingTasks));
        }

        _delay = delayBeforeRemovingTasks;
    }

Now when adding a task to the collection we just need to account for the delay and we can do that with the Task.Delay and combo that up with Task.WhenAny.  Also any time there is an async void that is a code smell.  I would just run the remove from the ContinueWith and not get task involved.  I wouldn't think the overhead of the thread switching would be that great for just removing an item from a hashset.

public void Add(Task task)
{
    // short-cut why add a task that is already done?
    if (!task.IsCompleted)
    {
        // Create a task that is done when either the time has passed or when it's completed
        var newTask = Task.WhenAny(task, Task.Delay(_delay));
        var fireEvent = false;
        lock (_tasksCollection)
        {
            if (_tasksCollection.Add(newTask))
            {
                // set if we need to fire event
                fireEvent = (_tasksCollection.Count == 1);
            }
        }

        // fire event outside of locker
        if (fireEvent)
        {
            // Don't pass in the internal collection as we don't want anyone modifying our data outside our self
            TaskStarted?.Invoke(this, EventArgs.Empty);
        }

        // add the continuation to remove it from the collection
        //     after we might have fired the event to ensure the events run in correct order
        newTask.ContinueWith(RemoveTask);
    }
}

private void RemoveTask(Task task)
{
    var fireEvent = false;
    lock (_tasksCollection)
    {
        {
            if (_tasksCollection.Remove(task))
            {
                fireEvent = (_tasksCollection.Count == 0);
            }
        }
    }

    if (fireEvent)
    {
        AllTasksCompleted?.Invoke(this, EventArgs.Empty);
    }
}

It's important to note that you should still await the task that is passed into this class outside of this class to handle any faults.


LSB steganography with pure Python As this is pure Python so using loop with big files make the code very slow
any improvements or suggestions.

Hide Code

 # Consts
HEADER_SIZE = 54
DELIMITER = "$"

class LSBEncrypter(object):
    def __init__(self):
        self.image_byte_counter = 0
        self.new_image_data = ''
        self.original_image = ''
        self.text_to_hide = ''

    def open_image(self):
        with open(ImageFile, "rb") as f:
            self.original_image = ''.join(map(chr, f.read()))
    def read_header(self):
        for x in range(0, HEADER_SIZE):
            self.new_image_data += self.original_image[x]
            self.image_byte_counter += 1
    def hide_text_size(self):
        sz = len(self.text_to_hide)
        s_sz = str(sz)
        s_sz += DELIMITER
        self.do_steg(s_sz)

    def do_steg(self, steg_text):

        for ch in range(0, len(steg_text)):

            current_char = steg_text[ch]
            current_char_binary = '{0:08b}'.format(ord(current_char))
            for bit in range(0, len(current_char_binary)):
                new_byte_binary = ''

                current_image_binary = '{0:08b}'.format(ord(self.original_image[self.image_byte_counter]))

                new_byte_binary = current_image_binary[:7]

                new_byte_binary += current_char_binary[bit]

                new_byte = chr(int(new_byte_binary, 2))

                self.new_image_data += new_byte
                self.image_byte_counter += 1
    def copy_rest(self):
        self.new_image_data += self.original_image[self.image_byte_counter:]

    def close_file(self):
        with open(StegImageFile, "wb") as out:
            out.write(bytearray(map(ord, self.new_image_data)))

    def run(self, stega_text):
        self.text_to_hide = stega_text
        self.open_image()
        self.read_header()
        self.hide_text_size()
        self.do_steg(self.text_to_hide)
        self.copy_rest()
        self.close_file()

    def hide_(self, Texte, image, steg):
        global TextToHide, ImageFile, StegImageFile
        TextToHide = Texte
        ImageFile = image
        StegImageFile = steg
        stega_instance = LSBEncrypter()
        stega_instance.run(TextToHide)

Reveal Code

 import binascii

HEADER_SIZE = 54
DELIMITER = "$"

class LSBDecrypter:
    def __init__(self):
        self.StegImageFile = ''
        # self.fh = open(StegImageFile, 'rb')
        self.number_of_chars_in_text = 0
        self.original_text = ''

    def read_header(self):
        self.fh = open(StegImageFile, 'rb')
        for i in range(0, HEADER_SIZE):
            byte = self.fh.read(1)

    def get_char(self):
        new_byte = ''

        for bit in range(0, 8):
            byte = self.fh.read(1)

            new_byte += str(ord(byte) & 0x01)

        n = int(new_byte, 2)
        desteg_char = binascii.unhexlify('%x' % n)
        desteg_char = ''.join(map(chr, desteg_char))
        return desteg_char

    def get_text_size(self):
        curr_ch = self.get_char()

        s_sz = ''

        while curr_ch != DELIMITER:
            s_sz += curr_ch
            curr_ch = self.get_char()

        if (s_sz != ''):
            self.number_of_chars_in_text = int(s_sz)

    def read_stega_text(self):
        decoded_chars = 0;
        while decoded_chars < self.number_of_chars_in_text:
            self.original_text += self.get_char()
            decoded_chars += 1

    def close_file(self):
        self.fh.close();

    def get_text(self):
        self.read_header()
        self.get_text_size()
        self.read_stega_text()
        self.close_file()
        return self.original_text

    def reveal_(self, stego):
        global StegImageFile
        StegImageFile = stego
        destag_insta = LSBDecrypter()
        text = destag_insta.get_text()
        print (text)

Performance:
def get_char(self):
    new_byte = ''

    for bit in range(0, 8):
        byte = self.fh.read(1)

        new_byte += str(ord(byte) & 0x01)

    n = int(new_byte, 2)
    desteg_char = binascii.unhexlify('%x' % n)
    desteg_char = ''.join(map(chr, desteg_char))
    return desteg_char

This is not as simple as it could be. You are reading 8 bits as strings to build a single byte, converting it to an integer, then to a hex string (string format before unhexlify), then to a hexadecimal string, then mapping to a character, and then joining them together. I recommend doing more math and less string conversion and concatenation.
Style:
1. Use consistent string delimiters

You have ' in most places, like open(StegImageFile, 'rb'), but you also have DELIMITER = "$" and open(StegImageFile, "wb"). Use ' everywhere if that's what you're using.

2. Remove unnecessary parentheses

if (s_sz != ''): should be if s_sz != '':, or even more concise: if s_sz:.

3. Consider meaningful variable names

s_sz isn't a helpful variable name, and I don't know what it is at a glance, other than that it's the string form of sz (also not helpful).

4. Remove the space in print (text)

You don't use spaces after any other function names.

5. Remove weird leading spaces

The space before # Consts

The space before import binascii


Website to Track Project's Progress This ASP.NET MVC website is for my professor to track my progress on my Checkers app.  This is the first time I have build an ASP.NET website entirely on my own, and I would like a review to make sure I am doing everything in the best way possible.

First, my HomeController:

public class HomeController : Controller
{
    private T Deserialize<T>(string filePath)
    {
        var jss = new System.Web.Script.Serialization.JavaScriptSerializer();
        var data = System.IO.File.ReadAllText(filePath);
        return jss.Deserialize<T>(data);
    }

    public ActionResult Index()
    {
        var announcements = Deserialize<ListOfItems<Announcement>>(Server.MapPath("~/App_Data/Announcements.json"));
        var latestAnnouncement = announcements.Items.Last();
        if (DateTime.Now.Date.Subtract(TimeSpan.FromDays(7)) > latestAnnouncement.AnnouncementDate)
        {
            latestAnnouncement = null;
        }

        return View(latestAnnouncement);
    }

    public ActionResult About()
    {
        return View();
    }

    public ActionResult Contact()
    {
        return View();
    }

    public ActionResult Reports()
    {
        var reports = Deserialize<ListOfItems<BiweeklyReport>>(Server.MapPath("~/App_Data/Reports.json"));

        return View(reports.Items);
    }

    public ActionResult Resources()
    {
        var resources = Deserialize<ListOfItems<Resource>>(Server.MapPath("~/App_Data/Resources.json"));

        return View(resources.Items);
    }

    public ActionResult Announcements()
    {
        var announcements = Deserialize<ListOfItems<Announcement>>(Server.MapPath("~/App_Data/Announcements.json"));

        return View(announcements.Items);
    }
}

My models are as follows:

First, a generic class for easy serialization of a list of items:

[Serializable]
public class ListOfItems<T>
{
    public List<T> Items { get; set; }
}

And a set of items to serialize.  To keep this post slim, I will only post one:

[Serializable]
public class Announcement
{
    public string Title { get; set; }
    public DateTime AnnouncementDate { get; set; }
    public string Content { get; set; }
}

My views are all either static information or a simple iteration over a list of data; here is my view for the above model class:

@model List<Announcement>
@using ITM3804Website.Models

@{
    ViewBag.Title = "Announcements";
}

@ViewBag.Title

@{
    var orderedAnnouncements = Model.OrderByDescending(x => x.AnnouncementDate);
    foreach (var announcement in orderedAnnouncements)
    {

             @announcement.Title

             @announcement.AnnouncementDate.ToString("MMMM d, yyyy")

             @announcement.Content

        if (announcement != orderedAnnouncements.Last())
        {

         }
     }
}

I want to talk about your View while I'm on mobile and thinking about it. Just like regular C# code we should definitely define structure for our Razor code. The first thing that rubs me wrong is that @using statement. I really don't like it because you only need it for the @model declaration at the top of the file. I would just get rid of it and explicitly specify the model full type.

Next, I really don't like the @{ } block with the foreach and such in it. Generally I just prefer to prefix each code block with @whatever instead. This helps create a more consistent convention throughout. (Right now you mix code and @code in your view which is generally bad.)

As far as the HTML itself, we've started leaning more on CSS to do the styling for things, so elements like strong and i should be span class="title" then in your CSS .title would have font-weight: bold;.

Let's talk about your serialization:

private T Deserialize<T>(string filePath)
{
    var jss = new System.Web.Script.Serialization.JavaScriptSerializer();
    var data = System.IO.File.ReadAllText(filePath);
    return jss.Deserialize<T>(data);
}

You always call it as Deserialize<T>(Server.MapPath(...)), so let's encapsulate that:

private T DeserializeAppData<T>(string file) =>
    Deserialize<T>(Server.MapPath($"~/App_Data/{file}.json"));

Then call it as DeserializeAppData<T>("Reports"). Much simpler, straightforward and to the point. You still have the other Deserialize<T> method, in case you need it for other things.


Code to write a C template file I am writing code to write a C template file; the executable takes two arguments (main) <file name> and <no of questions> and outputs a file with a file named argv[1] and inside which there are n no of functions with n+1 switch cases in main() calling those functions; where n = argv[2].

Problem

My problem is with the allocation of memory to the char*'s, it feels so messy and roundabout.
The formatting of strings some times gives no seg faults other times it is not possible. For example, ass = fopen(("./%s",fname),"w"); works fine; but strncat(swic,("    case %d : q%d();break;\n",i,i),s/n); does not work. For it to work I have to use format() a user function defined as follows:

char* format(char* s,...){
    va_list args;
    size_t siz=strlen(s)+sizeof(size_t);
    char* res=(char*)malloc(siz);
    va_start(args,s);
    vsnprintf(res, siz, s, args);
    va_end(args);
    return(res);
}

Another problem in above function is that I am unable to get the size or no. of passed arguments which I need to correctly allocate char *res which contains the resultant formatted string.
Also, during debugging I found that during fclose(ass) in main(); vscode gives two faults received SIGTRAP, Trace/breakpoint trap. in ntdll!RtlpNtMakeTemporaryKeyreceived signal ?, Unknown signal., then exits.

Click to view the complete code

    char* tail(n)
    {
        char *main="\nint main(){\nint choi;\nprintf(\"\\n*note: enter 0 to diplay all Q\\n\\n  Display Q no. \");\nscanf(\"  %%d\",&choi);\nswitch(choi){\n";
        char *naim="    default: printf(\"! invalid choice !\");\n}\nreturn 0;\n}";

        size_t f=strlen(format("void q%d(){printf(\"\\n ---- Q %d -----\\n\");\n}\n",n,n))*n, s=strlen(format("    case %d : q%d();break;\n",n,n))*n, s0=strlen(format("q%d();",n))*n,c0=strlen("    case 0 : break;\n");
        char *func=(char*)malloc(f);
        // strcpy(func,"");
        char *swic0=(char*)malloc(s0);
        char *swic=(char*)malloc(s);
        // strcpy(swic,"");
        for(int i=1;i<=n;i++){
            if(i==1){
            strcpy(func,format("void q%d(){printf(\"\\n ---- Q %d -----\\n\");\n}\n",i,i));
            strcpy(swic,format("    case %d : q%d();break;\n",i,i));
            strcpy(swic0,format("q%d();",i));
            }
            else{
                strncat(func,format("void q%d(){printf(\"\\n ---- Q %d -----\\n\");\n}\n",i,i),f/n);
                strncat(swic,format("    case %d : q%d();break;\n",i,i),s/n);
                strncat(swic0,format("q%d();",i),s0/n);
            }
        }
        // return strcat(strcat(strcat(func,main),swic),naim);
        char *case0=(char*)malloc(c0+s0);
        sprintf(case0,"    case 0 : %sbreak;\n",swic0);
        char *res=(char*)malloc(strlen(format("%s%s",main,naim))+f+s+c0+s0);
        sprintf(res,"%s%s%s%s%s",func,main,case0,swic,naim);
        return res;

    }

Example: output file if argv[2]=="2"

    //##########  Loops assigment

    #include<stdio.h>

    int abs(int n){
        if(n<0) return n*-1;
        else return n;
    }

    // answer functions --
    void q1(){printf("\n ---- Q 1 -----\n");
    }
    void q2(){printf("\n ---- Q 2 -----\n");
    }

    int main(){
    int choi;
    printf("\n*note: enter 0 to diplay all Q\n\n  Display Q no. ");
    scanf("  %d",&choi);
    switch(choi){
        case 0 : q1();q2();break;
        case 1 : q1();break;
        case 2 : q2();break;
        default: printf("! invalid choice !");
    }
    return 0;
    }

We need to be more defensive in these areas:

malloc() and family can return a null pointer.
scanf() and family can return fewer than requested conversions.

Additionally, there's no need to cast the result of malloc() - if you've correctly included <stdlib.h>, then it returns a void*, which is assignable to any pointer type.

Writing the unnecessary cast makes the code harder to review, because all casts indicate areas that need close attention by the reviewer.


Handling session resources in MY_Controller with Codeigniter I've added a function that checks if the user is trying to use/access something which requires a session. I'd love to hear some criticisms of my choice of design.

class MY_Controller extends CI_Controller {

    function __construct() {
        parent::__construct();
        if(!$this->input->is_ajax_request()) {
            if(!$this->session->userdata('is_logged_in')) {
                $this->load->view('header_public_view');
                if($this->login_required())
                  //do something, redirect etc..
            }
            else {
                $this->load->model('user_model');
                $data['user'] = $this->user_model->get_user($this->session->userdata('userid'));
                $this->load->view('header_user_view', $data);
            }
        }

    }

    function login_required() {
        if($this->uri->total_segments() > 3)
            $request = $this->uri->segment(3);
        else
            $request = $this->uri->segment(2);
        foreach(unserialize(SESSION_RESOURCES) as $required) {
            if($request == $required)
                return true;
        }
    }

}

You don't really need to check for is_logged_in.  If you destroy/build the session data correctly all you should care about is "Does a user id exist in the session?", then you can let your main controller handle the rest.

class MY_Controller extends CI_Controller{

    //here we just build some constants to check against values
    //in the permissions array
    const PERM_READ = "read";
    const PERM_EDIT = "update";
    const PERM_DELETE = "delete";

    //here we let the main controller take care of our auth/permissions/roles etc
    //first check if a user id exists in the session
    //if so assign a user, group, permissions and check for authentication where
    //you need it ( ie: child controllers)

    protected $_user, $_group, $_permissions = array();

    public function __construct(){
        parent::__construct();

        //check for a user id in the session
        $this->user = ( $this->session->userdata('uid') )
                    ? User::find_by_id( $this->session->userdata('uid') )
                    : NULL;

        //if user exists assign permissions and group
          if ($this->user !== NULL) {
              $this->_assign_group();
              $this->_assign_permissions();
              $this->_check_for_banned_users();
          }

    }

    protected function _assign_group() {
        return $this->group = $this->user->group->name;
    }

    // {["read", "update", "delete"]}
    protected function _assign_permissions() {
        return  $this->permissions = json_decode($this->user->permissions);
    }

    protected function _check_for_banned_users() {
        if ($this->group === 'banned') {
            show_error('You have been banned from this website!');
            return;
        }
    }

    protected function _can_read(){
        return (bool) ( in_array( self::PERM_READ, $this->permissions) );
    }

    public function _can_edit(){
        return (bool) ( in_array( self::PERM_EDIT, $this->permissions) );
    }

    public function _can_delete(){
        return (bool) ( in_array( self::PERM_DELETE, $this->permissions) );
    }
}

This might give you some idea, as your user table won't look like that, I'm sure.

Now, you have control over any child classes.

class some_child extends MY_Controller{

     public function __construct(){
         parent::__construct();
     }
     public function show_something(){

          if($this->is_ajax_request()){
              if($this->user && $this->_can_read()){
              //yes a user exists and is logged in,
              //yes he has permission to read from this section
              $this->load->view('some_view');
              }
          }
          else
             show_404();
     }
}

Just be careful to destroy your session properly.

function logout(){
    $this->session->set_userdata(array(
         'uid'  => 0,
         //any others you created upon user login
    ));
    return (bool) $this->session->sess_destroy();
}


Display git commit statistics in bash I have scripts to display commit statistics and merge statistics of my repos, and they work. I wrote them for my personal usage, and because I was interested in finding trends in my git repos.

This script reports statistics about commits (number, average length in words, etc.). Relevant commits can be selected using git-rev-list options.

Features and times

count: report number of commits (not a performance issue)
len: report length in words of commit message and commit hash (~20s for 1561 commits)
len min, len max, and len avg: report minimum, maximum, or average commit message length in words and commit hash (~10-15s for the same)

  Benchmarks run with bash's time on my dotfiles repo

A previous implementation using for-loops had similar performance.

Obviously, the algorithms are O(n). They are still too slow for every-day usage.

#! /usr/bin/env bash

set -euo pipefail

USAGE='[-h] (count | len [min|max|avg]) [rev-list options]

Display commit statistics
Filter commits based on [rev-list options]'
SUBDIRECTORY_OK=true

# source git-sh-setup for some helpers
set +u
source "$(git --exec-path)"/git-sh-setup
set -u

SIZER=(
  wc
  # count words
  -w
)

size() {
  local commit="$1"
  git log "$commit" -1 --format=%B | "${SIZER[@]}" | tr -d ' '
}

commits_list() {
  command=(
    git
    rev-list
    # start somewhere
    --all
  )
  if (($# > 0)) ; then
    command+=("$@")
  fi
  "${command[@]}" 2>/dev/null
}

commit_count() {
  commits_list "$@" | wc -l | tr -d ' '
}

commit_len() {
  commits_list "$@" |
    while read c ; do
      size "$c" | tr -d '[:space:]'
      printf ' %s\n' "$c"
    done
}

commit_len_min() {
  commit_len "$@" |
    sort -n |
    head -n 1
}

commit_len_max() {
  commit_len "$@" |
    sort -rn |
    head -n 1
}

commit_len_avg() {
  local num=0
  {
    printf '%s\n' '5k'
    while read c ; do
      ((++num))
      size "$c"
      ((num >= 2)) && printf '%s\n' '+'
    done < <(commits_list "$@")
    printf '%s\n' "$num" '/p'
  } | dc
}

main() {
  (($# >= 1)) || usage
  case "$1" in
    count) commit_count "${@:2}" ;;
    len)
      if (($# >= 2)); then
        case "$2" in
          max|min|avg) commit_len_"$2" "${@:3}" ;;
          *) commit_len "${@:2}" ;;
        esac
      else
        commit_len "${@:2}"
      fi
      ;;
    *) usage ;;
  esac
}

main "$@"

Shell scripts being hard to profile, I've been unable to identify the bottleneck (though commit_len seems like a good place to start).

I run shellcheck regularly.

Some suggestions:

shellcheck should give you a few suggestions. I won't mention things I expect it to find.
Uppercase names are by convention only used for exported variables.
SUBDIRECTORY_OK is unused. If it's a magic variable this probably should be mentioned.
SIZER is only used once, so it should be inlined.
wc -w 8.30 from GNU coreutils, at least, does not output any spaces, so tr -d ' ' might be unnecessary.
(($# > 0)) would usually be written [[ "$#" -gt 0 ]].
Throwing away standard error means the script will be harder to debug. If there's specific output there you want to hide you can use cmd 2> >(grep -v … >&2)
commit_len is slow because for each commit you run a git command & more to count the number of commits before it. Which means you traverse the Git history N times. I think you'll get the same result by running size "$1".
You can use shift to simplify things like "${@:2}" to just "$@".
dc is not a tool I'm familiar with, but it will certainly be faster to count using something like awk to gobble the whole stream in one command. while read is actually surprisingly slow.


Logging with date in Java I write my log messages in JTextArea. Each message should have a time added.
I created a following method:

public void log(String message) {
    logTextArea.append(String.format("%s : %s%n",DateFormatUtils.ISO_TIME_NO_T_FORMAT.format(new Date()), message));
}

It uses DateFormatUtils from Apache Commons Lang.
Each time log() method is invoked, it creates a new instance of Date.

Is this the most elegant method?

Elegant and efficient are different things.

SimpleDateFormat instances are not thread safe. It is common in my code to have a few utility classes, and one method I often have on the utility class is:

private static final ThreadLocal<SimpleDateFormat> localDateFormat = new ThreadLocal<SimpleDateFormat>() {

    @Override
    protected SimpleDateFormat initialValue() {
             return new SimpleDateFormat(....);
    }

};

public static SimpleDateFormat getDateFormatter() {
    return localDateFormat.get();
}

That way, anyone calling getDateFormatter() gets an instance of the standard formatter for my system, and it is local to the current thread. I don't let them 'hang around', so you can't assign them to class fields, etc.

The benefit is that there's at most one instance per thread, it is thread safe (if you don't do silly things with it), and it is neat:

public void log(String message) {
    logTextArea.append(String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message));
}

Unfortunately, I don't think you have taken this far enough. You need another utility method like:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message);
}

and then all your various places that modify things like this can reuse the code:

public void log(String message) {
    logTextArea.append(Utils.formatLogMesssage(message));
}

Update: I somewhat misread your specific question, and in response to your comment:

creating a new short-term new Date() is not perfect, but is still very fast (it will be cycled out by the garbage collector very quickly, and not impact that very much at all). There are perhaps better ways though...
I don't use the commons library, so was not aware that the FastDateFormat class was thread-safe (it probably does what I suggest above), but, at the same time, the extra log-method is a good idea because it brings all the different log places in to one spot so you can change formats, etc. with just one changed line of code.

Additionally, if you are concerned about the actual performance of the new Date(), (and if you actually read the full Javadoc of the FastDateFormat class you sent me... ;-) you can replace that one method:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message);
}

with:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           DateFormatUtils.ISO_TIME_NO_T_FORMAT.format(System.currentTimeMillis()), message);
}


Object-oriented design of list of primes Is there a better way to implement this class, or does it not constitute implementation in the first place?  This is one of my first few tries at OOP.  Any suggestions will be much appreciated.

#include <iostream>
#include <vector>
#include <cmath>

class PrimeList
{
    public:
        std::vector<int> vPrime;
        void initVector(int n)
        {
            for (int i=2; i <= n; ++i)
                vPrime.push_back(i);
        }
};

int main()
{
    // Instantiate an object "primes" of type PrimeList.
    PrimeList primes;

    // Get the input variable to pass to the initVector method.
    std::cout << "Enter the upper limit of the list of primes you want     to generate:" << std::endl;
    int n;
    std::cin >> n;

    // Create a vector filled with every integer from 2 to n.
    primes.initVector(n);

    // Begin the sieve
    for (int a=0; a < sqrt(n); ++a) {
        for (int b=a+1; b < primes.vPrime.size(); ++b) {
            if (primes.vPrime[b] % primes.vPrime[a] == 0) {
                primes.vPrime.erase(primes.vPrime.begin() + b);
            }
        }
    }

    // Print the list of primes from 2 to n.
    for (int i=0; i < primes.vPrime.size(); ++i)
        std::cout << primes.vPrime[i] << std::endl;
    return 0;
}

As a general rule for classes: data members (variables), such as vPrime, should be private.  They can either be declared under private or outside of public (a class is private by default).
For a simple use of classes here, you don't need a mutator.  Just declare your object with an argument:

PrimeList primes(n);

Since this would make n (or let's just make it number) a data member, modify your constructor as such and initialize the object like this:

// 'n' is the parameter and 'number' is the class's data member
PrimeList::PrimeList(int n) : number(n) {initVector();}

As you can see, the constructor now calls initVector().  With number now as a data member, initVector() no longer needs an argument.  Since this function is not part of the interface, make it private.
The display for-loop in main() can also be put into a class method:

void display() const; // header declaration

// .cpp implementation

void PrimeList::display() const
{
    // your object is no longer referenced inside the class
    for (int i = 0; i < vPrime.size(); ++i)
    {
        std::cout << vPrime[i] << std::endl;
    }
}

Then simple call it in main() with your object:

primes.display();

Note the use of const fin this function.  If you're not changing any data members, create your functions as such.  If you accidentally violate this, your compiler will stop you.  This is to ensure that your data members are not unintentionally modified, depending on the functions's purpose.
Your sieve calculations can also be put into a class method.  Get the idea now?  Again, it should be private.  It will NOT be const since you're changing the vector inside the class.  You can call it in initVector() since they're both non-const (you cannot call const functions in non-const functions and vice-versa).


Array-based queue implementation using C I wrote this header for generic use of a queue. The one thing I'm wondering is if I understood the usage of void*. I hope that if somebody teach me some conventions when coding in C.

/*
*   array-based queue implementation by using void*
*   written by kidkkr
*   May 6 '16
*/

#ifndef QUEUE_H
#define QUEUE_H
#define QUEUE_CAPACITY 10
#include <stdio.h>

typedef struct {
    void* data[QUEUE_CAPACITY];
    int head;
    int tail;
    int size;
} Queue;

void initQueue(Queue* pQueue)
{
    pQueue->head = 0;
    pQueue->tail = -1;
    pQueue->size = 0;
}

void enqueue(Queue* pQueue, void* item)
{
    if (pQueue->size == QUEUE_CAPACITY) // when queue is full
    {
        printf("Queue is full\n");
        return;
    }
    else
    {
        (pQueue->tail)++;
        (pQueue->tail) %= QUEUE_CAPACITY;
        (pQueue->data)[pQueue->tail] = item;
        (pQueue->size)++;
    }
}

void* dequeue(Queue* pQueue)
{
    // Return NULL when queue is empty
    // Return (void*)item at the head otherwise.
    void* item = NULL;
    if (isEmpty(&pQueue))
    {
        printf("Queue is empty\n");
    }
    else
    {
        item = (pQueue->data)[pQueue->head];
        (pQueue->head)++;
        (pQueue->head) %= QUEUE_CAPACITY;
        (pQueue->size)--;
    }
    return item;
}

int isEmpty(Queue* pQueue)
{
    return pQueue->size == 0;
}

#endif

if (isEmpty(&pQueue)) is wrong. It should be pQueue. You also need to have the prototype in scope before you use it. Put int isEmpty(Queue* pQueue) at the top of your file.


Sending an uptime to a remote server through SSH with the method pxssh I have this code that I'm putting into a library script.

The aim of the code is the following:

The method is to connect to a remote server and run uptime on the remote server. The uptime will be displayed on the host machine, to see for how long the server has been running.

The aim in a near future, is to uptime-check several servers and have the results of my program, on the running machine.

I put the the method under a main to not run it when the library is imported.

from pexpect import pxssh
import pexpect

# conn_up is a function for connecting to a remote server and doing an uptime.

def conn_up():
    try:
        s = pxssh.pxssh()
        hostname = '192.168.1.32'
        username = 'pi'
        s.login(hostname, username,ssh_key='/home/miaou/priv_keys')
        s.sendline('uptime')   # run a command
        s.prompt()             # match the prompt
        print(s.before.decode())        # print everything before the prompt.
    except pxssh.ExceptionPxssh as e:
        print("pxssh failed on login.")
        print(e)

if __name__ == '__main__':
    conn_up()

Currently I have only one method, conn_up().

But if I want to create additional methods to broaden the tools available for my administration, should I list them all of them under __name__ == '__main__' or is there a simpler way to call all the future methods all at once?

It sounds like the problem is not the code but how to package it up in a such a way that only functions that you want to call get called when you want to call them.

A part of the solution is as you have defined it ...

if __name__ == '__main__':
    conn_up()

... this would ensure that conn_up() gets called, but you could just treat conn_up as a constructor and pull all the code from it and put it in another function which you then call on demand.

From further discussion with you on discord it seems that the issue is also in part about extending this in the future so I would advise packaging this up in to a class that you might add functions to, then can add to your "if __name__ == '__main__':" block additional functions as you add them, or through some other logic in a new "onStart()" function or something.

There's a great post on this here ...

Hope this helps.


C++  convenience functions I have a set of functions in my program for conveniently generating random number in various distributions. I'm interested in the questions listed below, though other comments are also welcome.

Am I using C++'s random tools correctly and clearly?
From watching rand() Considered Harmful and reading this answer, I learned that RNG calls are not thread-safe. I changed my code just by adding the specifier thread_local to the generator and distribution variables. Is this all that's needed to make the functions thread-safe? These are called within std::async procedures in my project.
Does it matter whether one std::mt19937_64 generator is used for all the functions or if each function has its own copy?

In Random.h

#ifndef RANDOM_H
#define RANDOM_H

#include <random>
#include <algorithm>

namespace Random
{
    // Random number with normal distribution and mean of zero
    double random_normal(double standard_deviation);

    // Random number with inclusive range
    double random_real(double min, double max);
    int random_integer(int min, int max);

    // Return true with probability 50%
    bool coin_flip();

    // Return true with given probability
    bool success_probability(double probability);

    namespace
    {
        thread_local std::mt19937_64 generator(std::random_device{}());
    }

    // Shuffles the order of the list
    template<class List>
    void shuffle(List& list)
    {
        std::shuffle(list.begin(), list.end(), generator);
    }
}
#endif // RANDOM_H

In Random.cpp

#include "Random.h"

#include <random>

int Random::random_integer(int min, int max)
{
    using uid = std::uniform_int_distribution<int>;
    thread_local static auto dist = uid{};
    return dist(generator, uid::param_type{min, max});
}

double Random::random_normal(double standard_deviation)
{
    using nd = std::normal_distribution<double>;
    thread_local static auto dist = nd{};
    return dist(generator, nd::param_type{0.0, standard_deviation});
}

double Random::random_real(double min, double max)
{
    using urd = std::uniform_real_distribution<double>;
    thread_local static auto dist = urd{};
    return dist(generator, urd::param_type{min, max});
}

bool Random::coin_flip()
{
    return success_probability(0.5);
}

bool Random::success_probability(double probability)
{
    return random_real(0.0, 1.0) < probability;
}

An example usage:

#include <iostream>
#include <future>

#include "Random.h"

double random_walk(int number_of_steps, double step_size)
{
    double position = 0;
    for(int i = 0; i < number_of_steps; ++i)
    {
        position += Random::random_normal(step_size);
    }
    return position;
}

int main()
{
    // Simulate 1,000,000-step continuous random walks
    const int number_of_walks = 100; // adjust to suit your computer
    const int number_of_steps = 1000000;
    const double step_size = 1.0;
    std::vector<std::future<double>> results;
    for(int i = 0; i < number_of_walks; ++i)
    {
        results.emplace_back(std::async(random_walk, number_of_steps, step_size));
    }

    for(auto& result : results)
    {
        std::cout << result.get() << std::endl;;
    }

    return 0;
}

To answer your questions:

Sure looks like it.
No, see next point.
No, it is not safe. std::mersenne_twister_engine::operator() reads and increments its internal state. AFAIK, this makes it not thread safe at all.

So, here are a few suggestions:

An extra semicolon hid itself in your code:

std::cout << result.get() << std::endl;;
                                       ^

Don't use std::endl. It implicitly uses std::flush to flush the stream. This is not very cheap, so it is better for performance if you just output '\n'.

If you do want to flush the stream, it would be better to state your intent and use std::flush, but this depends on you.
Mark functions that do not throw noexcept. That way, you state your intent that the functions will not throw, and it helps the compiler optimize a bit.
Currently, your shuffle takes a container. While that is desirable in most cases, sometimes, you just need to shuffle a subrange of a container. It's for exactly this reason that all the standard algorithms take an iterator pair instead of a container.
If you try to include Random.h more than once, you will get symbol collision for the variable generator. The same thing happens for functions. Here's what you can do:

Use C++17, and mark the variable inline, which behaves exactly like in functions:

thread_local inline std::mt19937_64 generator(std::random_device{}());

Mark the variable extern, and put its definition in Random.cpp.

You don't need return 0; at the end of main(). The compiler implicitly adds it for you.
I like to use types as parameter that try to match the preconditions of a function. For example, I would use unsigned integers everywhere where you use signed integers, because I do not see the point in calling let's say random_walk with a negative number_of_steps.
You can shorten the definitions in Random.cpp by removing Random:: if you wrap them around namespace Random, like in the declarations.


The veiled guise of binary and string Challenge

Conceal a binary code within a word.

Specifications

The first argument is a path to a file.
The file contains multiple lines.
Each line is a test case represented by a space separated word and a binary code.
For each test case, print the binary masked word.
The mask alters the word according to the following:

If the bit is 1, change the letter to upper case.
If the bit is 0, leave the letter as is.

Constraints

Words are from 1 to 20 letters long.
The length of each word is equal to the length of the binary code.
Words are always in lower case.
The number of test cases is 40.

Sample Input

  hello 11001
  world 10000
  cba 111

Sample Output

  HEllO
  World
  CBA

Source

My Solution

#include <stdio.h>
#include <ctype.h>

#define LINE_BUFFER 42

int main(int argc, char *args[]) {
    if (argc < 2) {
        puts("File path not provided.");
        return 1;
    }

    if (argc > 2) {
        puts("Excessive arguments, only the first will be considered.");
    }

    FILE *file = fopen(args[1], "r");
    if (file == NULL) {
        puts("Could not access file / file not found.");
        return 1;
    }

    char line[LINE_BUFFER];
    while (fgets(line, LINE_BUFFER, file)) {
        for (int i = 0, word_length = 0; ; i++, word_length++) {
            if (line[i] == ' ') {
                line[i++] = '\0';
                for (int j = 0; j < word_length; j++, i++) {
                    if (line[i] == '1') {
                        line[j] = toupper(line[j]);
                    }
                }
                printf("%s\n", line);
                break;
            }
        }
    }

    fclose(file);
}

42

#define LINE_BUFFER 42

42 is not a buffer. Besides being a meaning of life, the universe, and everything, it is size. Consider renaming it to LINE_BUFFER_LENGTH.
No naked loops

Every loop represents some algorithm, and therefore deservers a name. Consider

while (fgets(line, LINE_BUFFER, file)) {
    char * mask = find_separator(line);
    *mask++ = '\0';
    convert_word_according_to_mask(line, mask);
    printf("%s\n", line);
}

Errors (like fopen returning NULL) are better handled with perror() or strerror(errno). Both functions precisely describe what went wrong.


jQuery scroll-prevention plugin This plugin is for article pages that has a very long main content (left side) and a shorter sidebar. When the user scrolls down mostly the right part will be white space.

What this plugin does is make the sidebar stick or make it fixed past a point and will stop sticking when it collides with the bottom part or a footer. The functionality will not work if the main content is short.

Anyway, I'm asking feedback on how to improve my JS code.

;(function ( $, window, document, undefined ) {

// Create the defaults once
var pluginName = "scrollMinUntil",
defaults = {
    article: null,
    topPadding: 0
};

// The actual plugin constructor
var Plugin = function ( element, options ) {
this.element = element;
this.options = $.extend( {}, defaults, options);
this._defaults = defaults;
this._name = pluginName;
this.init();
}

Plugin.prototype = {

init: function() {

    if($(this.element).length <= 0)
      return null;

    this.scroll(this.element, this.options);
},

scroll: function(el, options) {
    var article = options.article,
        topPadding = options.topPadding,
        w = $(window),
        wrap = $(el).children('.wrap'),
        startingPos = wrap.offset().top,
        fixedPosTrigger = wrap.offset().top + wrap.outerHeight(),
        stopMovingPosTrigger = 0,
        staticMarginTop = 0,
        scrollTop = 0,
        elHeight = 0,
        stopMovingPos = 0,
        fixed = false,
        staticFlag = false;

    if(options.article.outerHeight() > 1500) {
      w.scroll(function() {

        // Calculate dynamic values
        scrollTop = $(this).scrollTop(),
        elHeight = wrap.outerHeight(),
        fixed = wrap.hasClass('fixed'),
        staticFlag = wrap.hasClass('static'),
        stopMovingPos = article.outerHeight() + article.offset().top;

        // Check if scrolling before or after the fixed trigger, set stopMovingPosTrigger accordingly
        (scrollTop < fixedPosTrigger) ? stopMovingPosTrigger = scrollTop : stopMovingPosTrigger = scrollTop + elHeight;

        // Making sure that fixed is triggered when scrolling pass the trigger position from the original area
        if(scrollTop > fixedPosTrigger && !staticFlag)
          wrap
            .addClass('fixed')
            .css({
              'padding-top': topPadding + 'px'
            });

        // Fixed positioned area, setting paddings and margins, and transition scrolling from the static area
        if((scrollTop > fixedPosTrigger) && (stopMovingPosTrigger < stopMovingPos)){
          wrap
            .removeClass('static')
            .addClass('fixed')
            .css({
              'margin-top': 0
            });
        }

        // Static positioned area, setting dynamically the margin-top for the element
        else if((stopMovingPosTrigger > stopMovingPos) && fixed) {

          // Calculate top margin to be added to the static element
          staticMarginTop = (stopMovingPos - startingPos) - elHeight;

          wrap
            .removeClass('fixed')
            .addClass('static')
            .css({
              'margin-top': staticMarginTop
            });
        }

        // Original area before the fixed position area
        else if(scrollTop < fixedPosTrigger) {
          wrap
            .removeClass('fixed static')
            .css({
              'padding-top': 0
            });
        }
      });
    }
}
};

// A really lightweight plugin wrapper around the constructor,
// preventing against multiple instantiations
$.fn[pluginName] = function ( options ) {
return this.each(function () {
    if (!$.data(this, "plugin_" + pluginName)) {
        $.data(this, "plugin_" + pluginName,
        new Plugin( this, options ));
    }
});
};

})( jQuery, window, document );

This part is pretty odd:

    // Calculate dynamic values
    scrollTop = $(this).scrollTop(),
    elHeight = wrap.outerHeight(),
    fixed = wrap.hasClass('fixed'),
    staticFlag = wrap.hasClass('static'),
    stopMovingPos = article.outerHeight() + article.offset().top;

The 5 assignments are separated by commas, instead of semi-colons.
It would be better to end each of the first 4 lines with a semi-colon.

This is even worse, using assignment in the right-hand sides of a ternary operator:

    (scrollTop < fixedPosTrigger) ? stopMovingPosTrigger = scrollTop : stopMovingPosTrigger = scrollTop + elHeight;

It would be shorter and better to move stopMovingPosTrigger to the left, and use the ternary at the right-hand side of the assignment:

    stopMovingPosTrigger = scrollTop < fixedPosTrigger ? scrollTop : scrollTop + elHeight;

Inside the scroll function, these conditions and their negated values appear multiple times:

scrollTop > fixedPosTrigger
stopMovingPosTrigger > stopMovingPos

To reduce the duplication which may lead to errors,
I suggest giving these conditions a name by storing them in variables, like this:

var fixedPositionArea = scrollTop > fixedPosTrigger;
var staticPositionArea = stopMovingPosTrigger > stopMovingPos;

This way, instead of re-typing scrollTop > fixedPosTrigger multiple times,
you could use fixedPositionArea and !fixedPositionArea in your conditions.
Of course, !fixedPositionArea is not exactly the same as scrollTop < fixedPosTrigger,
but it seems to me this small discrepancy won't break your script.

Using these variables the code becomes:

if (fixedPositionArea && !staticFlag) {
    wrap
        .addClass('fixed')
        .css({ 'padding-top': topPadding + 'px' });
}

// Fixed positioned area, setting paddings and margins, and transition scrolling from the static area
if (fixedPositionArea && !staticPositionArea) {
    wrap
        .removeClass('static')
        .addClass('fixed')
        .css({ 'margin-top': 0 });
}

// Static positioned area, setting dynamically the margin-top for the element
else if (staticPositionArea && fixed) {
    // Calculate top margin to be added to the static element
    staticMarginTop = stopMovingPos - startingPos - elHeight;

    wrap
        .removeClass('fixed')
        .addClass('static')
        .css({ 'margin-top': staticMarginTop });
}

// Original area before the fixed position area
else if (!fixedPositionArea) {
    wrap
        .removeClass('fixed static')
        .css({ 'padding-top': 0 });
}

Some blocks are not indented well, for example:

var Plugin = function ( element, options ) {
this.element = element;
this.options = $.extend( {}, defaults, options);
this._defaults = defaults;
this._name = pluginName;
this.init();
}

It would be better to indent the body inside a { ... } block, like this:

var Plugin = function ( element, options ) {
    this.element = element;
    this.options = $.extend( {}, defaults, options);
    this._defaults = defaults;
    this._name = pluginName;
    this.init();
};

Also, since this is an assignment, the right-hand side should be terminted with a semi-colon.

Review the rest of the code and correct all the under-indented blocks.

It would be also good to use consistent amount of indents.
In most places you seem to use 4 spaces, while in others only 2.
I suggest to pick one width and use it consistently everywhere.