Merging (not removing) duplicated elements in a vector There's a struct named Player containing name and stat:

struct Player {
    name: String,
    stat: Stat,
}

struct Stat {
    points: u32,
    fouls: u32,
}

And I have a Vec<Player>. What is interesting is that there can be multiple players with same name and different stats like the following:

let players = vec![
    Player {
        name: String::from("player1"),
        stat: Stat {
            points: 10,
            fouls: 1,
        },
    },
    Player {
        name: String::from("player2"),
        stat: Stat {
            points: 30,
            fouls: 3,
        },
    },
    Player {
        name: String::from("player1"),
        stat: Stat {
            points: 5,
            fouls: 1,
        },
    },
];

The first and the third elements have same name, but with different stat values. What I want to do is merging those elements into one by adding all stat values. After the merging process, the output vector will have 2 elements like the following:

[
    Player {
        name: String::from("player2"),
        stat: Stat {
            points: 30,
            fouls: 3,
        },
    },
    Player {
        name: String::from("player1"),
        stat: Stat {
            points: 15,
            fouls: 2,
        },
    },
]

(The order of output vector is not important in this case.)

And the code below is my solution:

let dup_merged: Vec<_> = players
    .into_iter()
    .fold(
        HashMap::new(),
        |mut acc: HashMap<String, Player>, curr: Player| {
            match acc.get(&curr.name) {
                Some(prev) => {
                    // Is it better to modify prev.stat
                    // instead of inserting new Player instance?
                    // HashMap<String, &Player> required to modification?
                    acc.insert(
                        curr.name.clone(),
                        Player {
                            name: curr.name,
                            stat: Stat {
                                points: prev.stat.points + curr.stat.points,
                                fouls: prev.stat.fouls + curr.stat.fouls,
                            },
                        },
                    );
                }
                None => {
                    acc.insert(curr.name.clone(), curr);
                }
            }
            acc
        },
    )
    .into_iter() // Hope there's better way to get a vector of a hash map
    .map(|(_, v)| v)
    .collect();

Playground link

I used HashMap (key: player.name: String, value: player: Player) to accumulate stat values for players with same name, then  converted the HashMap to Vec<Player> again. I think my solution can be simpler, more idiomatic and refined better.

Looks pretty good already! Here's some ideas for improvement.

You can make implement Add and AddAssign for Stat so that you can use + and += on those. Here's an AddAssign implementation:

impl std::ops::AddAssign for Stat {
    fn add_assign(&mut self, other: Self) {
        self.points += other.points;
        self.fouls += other.fouls;
    }
}

I like that you reached for a HashMap and fold, those are definitely how I'd do this. However, you only need to store the name and stat (HashMap<String, Stat>), which will make it so you don't have to clone the name each time and also reduce size. You can simply reconstruct the players at the end, before the collection.

Another thing is that you currently perform two lookups in the hashmap for every insertion. The Entry API is a great tool to learn for interacting with maps. It lets you interact with a possibly missing element of the map and only perform one lookup.

Here's a implementation with what I've mentioned so far.

use std::collections::hash_map::Entry;

let dup_merged: Vec<_> = players
    .into_iter()
    .fold(
        HashMap::new(),
        |mut acc: HashMap<String, Stat>, curr: Player| {
            match acc.entry(curr.name) {
                Entry::Occupied(mut occ) => {
                    // This player already exists, increase its stats
                    *occ.get_mut() += curr.stat;
                }
                Entry::Vacant(vac) => {
                    // No such player exists, insert these stats
                    vac.insert(curr.stat);
                }
            }
            acc
        },
    )
    .into_iter()
    .map(|(k, v)| Player { name: k, stat: v })
    .collect();

There's another simplification that can be made, due to Stat being a small, cheap struct with sensible default values: You can have Stat implement Default, and then instead of inserting an entry if its missing, you can just have it default to a zero'd Stat and always add:

#[derive(Default, Debug)]
struct Stat {
    points: u32,
    fouls: u32,
}
// ...
let dup_merged: Vec<_> = players
    .into_iter()
    .fold(
        HashMap::new(),
        |mut acc: HashMap<String, Stat>, curr: Player| {
            *acc.entry(curr.name).or_default() += curr.stat;
            acc
        },
    )
    .into_iter()
    .map(|(k, v)| Player { name: k, stat: v })
    .collect();


ShoppingCart and Item Classes I've built a class for an e-commerce Item, and the ShoppingCart class for checkout functions.

The thing I'm most concerned about is my use of pointers.  I think I've used them properly in this situation, but I could be wrong.

Item.hpp

#ifndef Item_hpp
#define Item_hpp

#include <iostream>
using namespace std;

class Item {

private:
    string name;
    double price;
    int quantity;

public:
    void setName(string);
    void setPrice(double);
    void setQuantity(int);
    string getName();
    double getPrice();
    int getQuantity();

    // Constructors
    Item();

    Item(string, double, int);

};

#endif

Item.cpp

#include "Item.hpp"
#include <iostream>
using namespace std;

Item::Item() {
    name = "";
    price = 0.0;
    quantity = 0;
}

Item::Item(string itemName, double itemPrice, int itemQuantity) {
    setName(itemName);
    setPrice(itemPrice);
    setQuantity(itemQuantity);
}

void Item::setName(string itemName) {
    name = itemName;
}

void Item::setPrice(double itemPrice) {
    price = itemPrice;
}

void Item::setQuantity(int itemQuantity) {
    quantity = itemQuantity;
}

string Item::getName() {
    return name;
}

double Item::getPrice() {
    return price;
}

int Item::getQuantity() {
    return quantity;
}

ShoppingCart.hpp

#ifndef ShoppingCart_hpp
#define ShoppingCart_hpp

#include "Item.hpp"
#include <iostream>
using namespace std;

class ShoppingCart {

private:
    Item* itemArray[100];
    int arrayEnd;

public:
    void addItem(Item *);
    double totalPrice();

    // Constructors
    ShoppingCart();

};
#endif

ShoppingCart.cpp

#include "ShoppingCart.hpp"
#include "Item.hpp"
#include <iostream>
using namespace std;

ShoppingCart::ShoppingCart() {
    for (int i=0; i<100; i++) {
        itemArray[i] = {NULL};
    }
    arrayEnd = 0;
}

void ShoppingCart::addItem(Item *currentItem) {
    itemArray[arrayEnd] = currentItem;
    arrayEnd++;
}

double ShoppingCart::totalPrice() {
    double totalPrice;

    for (int i=0; i<100; i++) {
        if (itemArray[i] != NULL) {
            totalPrice += itemArray[i]->getPrice() * itemArray[i]->getQuantity();
        } else {
            break;
        }
    }

    return totalPrice;
}

using namespace std is considered a bad practice.
Unrestricted getters and setters are as good as making Item's members public. In fact, if there is no class invariant to maintain, do make them public.
arrayEnd is not a particularly good name. Consider calling it items (or items_count) instead. As a side note, use it!

for (int i = 0; i < items; i++)
    total_price += whatever;

This way the shopping cart constructor reduces to

ShoppingCart() {
    items = 0;
}

totalPrice is not initialized.
Having pointers in the shopping card is a dubious decision. You must answer the question who owns items. In other words, who's responsibility is to destroy Item, and what happens to the cart when it is destroyed? I strongly recommend to either have the cart to keep objects, or make cart responsible for their construction and destruction.


Applying a function to columns of two matrices producing their combinations I made a short snippet for applying a function f that takes two arguments (vectors) to columns of two matrices producing all possible combinations of those columns.

function V = cross_apply(f, V1, V2)
% Apply function f to columns of V1 and V2 producing
% a matrix where V(i, j) = f(V1(:,i), V2(:,j))
    s1 = size(V1, 2);
    s2 = size(V2, 2);
    V = zeros(s1, s2);
    for i = 1:s1
        for j = 1:s2
            V(i, j) = f(V1(:, i), V2(:, j));
        end
    end
end

An example of a use of this function could include approximating a function with a polynomial using a custom inner product (not a traditional dot product) here ip and defined as a kind of integral.

close all
f = @(x) log(x);
P = @(x) x.^(0:3);
ip = @(x, y) trapz(x.*y) / (size(x, 1)-1);

t = linspace(1, 2, 10)';
V = P(t);

% Inner product matrices
A = cross_apply(ip, V, V);
b = cross_apply(ip, V, f(t));

% Coefficients for the polynomial
rr = rref([A, b])
coef = rr(:, end);

% Plot ln(t) and polynomial fit
t_ = linspace(1, 3, 1000)';
figure()
plot(t_, P(t_)*coef, 'r--')
grid on, hold on
plot(t_, f(t_), 'b:')
legend('fit', 'f(t)', 'Location', 'best')

Not using this function, one would write a whole bunch of inner products (ip in the code) for matrices A and b. I didn't find a built-in function that does this, so I'm after two things:

Is this already an implemented function in Matlab built-ins or some toolbox?
Is there some improvement to be made to the function?

Your code is clear and concise, there is not much to complain about it, except for one small thing:

t = linspace(1, 2, 10)';

In MATLAB, ' is the complex conjugate transpose, whereas .' is the non-conjugate transpose, the simple re-ordering of dimensions. The array it is applied to here is real-valued, so there is no difference, but it is good practice to use .' always when the intention is to change the orientation of a vector. Bugs do appear at times because of the improper use of the ' operator.

As I've said before, loops are no longer slow in MATLAB, but often it's still possible to remove them ("vectorize the code") for a time gain. If the vectorization doesn't involve creating intermediate copies of the data, and doesn't involve complex indexing, then vectorization is still profitable.

In this particular case, vectorization involves calling a function one time vs calling it 10x10=100 times, and function calls still have a significant overhead in MATLAB. So cutting down on function calls will be beneficial.

Producing all possible combinations of two values can be accomplished with the bsxfun function in MATLAB. But in this case, we want to produce all combinations of two columns, and there is no function for that. However, you could use it to generate the product of all columns (the argument to trapz in your function ip). This requires a large intermediate matrix to be generated, but in this case (100 vectors of length 4), this is a very mild amount of data, and well worth the price if it means not calling a function 100 times.

Note that bsxfun is, since R2016b, no longer necessary. Everything you could previously do with bsxfun can now be done without it. For example, given x=1:10, you can do:

y = x .* x.';

to produce a 10x10 matrix with the product of all combinations of elements of x. This implicit singleton expansion happens automatically when the two arguments to any operator have compatible dimensions. That means that their sizes are identical along each dimension, or one of them has size 1 along a dimension (a singleton dimension). That singleton dimension gets repeated virtually to match the size of the other matrix.

Your function ip is written such that, given two input matrices x and y of compatible dimensions, it will compute your custom inner product along the first dimension, as long as that dimension has a size larger than 1. To make this function safer in use, you can explicitly tell trapz along which dimension to operate:

ip = @(x, y) trapz(x.*y, 1) / (size(x, 1)-1);

Now, ip(x,y) will compute many custom inner products at once, if x and y are matrices. However, ip(V, V) will compute the inner product of each column in V with itself, not all combinations of columns. For that, we need to do a little bit of reshaping of the inputs:

function V = cross_apply2(f, V1, V2)
s1 = size(V1, 2);
s2 = size(V2, 2);
V2 = reshape(V2, [], 1, s2);
V = f(V1, V2);
V = reshape(V, s1, s2);

What this code does is convert the second input matrix of size NxM to a matrix of size Nx1xM (this can be done without copying the data). Now the function f (really ip the way you call it) will use implicit singleton expansion to compute the custom inner product of each combination of columns of V1 and V2.

You can verify that cross_apply and cross_apply2 yield exactly the same result:

A = cross_apply(ip, V, V);
b = cross_apply(ip, V, f(t));

A2 = cross_apply2(ip, V, V);
assert(isequal(A,A2))
b2 = cross_apply2(ip, V, f(t));
assert(isequal(b,b2))

And we can also see what the time savings are for calling the function ip once instead of 100 times:

timeit(@()cross_apply(ip, V, V))
timeit(@()cross_apply2(ip, V, V))

On my computer this is 1.5016e-04 vs 2.9650e-05, a 5x speedup.

But cross_apply2 is not as general as cross_apply, as it imposes a requirement on the function to be applied to be vectorized.

trapz is a funny function. In your case, where it is called without an x input argument, it sets x to 1. The trapezoid rule now is simply the sum of the elements of the input vector, minus half the first and half the last element. I don't know what you application even is, but have you considered replacing this with the simple sum? Computing the true dot product is a lot faster.

You can do edit trapz to see how trapz is implemented. For the way you call it, it does:

z = sum((y(1:end-1,:) + y(2:end,:)), 1)/2;

This is the same, as I said before, as:

z = sum(y(:,:)) - y(1,:)/2 - y(end,:)/2;

But this second form is twice as fast for large arrays: The first form copies the data in y twice (minus one row), adds these results and sums them up, whereas the second form simply adds up all elements, then subtracts half of the first row and half the last row. For a random large array y:

y = rand(1000,100,100);
z1 = sum((y(1:end-1,:) + y(2:end,:)), 1)/2;
z2 = sum(y(:,:)) - y(1,:)/2 - y(end,:)/2;
max(abs(z1(:)-z2(:)))
timeit(@()sum((y(1:end-1,:) + y(2:end,:)), 1)/2)
timeit(@()sum(y(:,:)) - y(1,:)/2 - y(end,:)/2)

This shows that the largest difference between the two results is rounding errors (4.8317e-13), and the timings are 0.0778 vs 0.0341.


Add many objects to a collection in a JavaScript object I'm trying to add a lot of objects manually into a collection.  The best I can figure out is to define an object, create an add method, and call that add method multiple times in the object's constructor.

function FieldMap(){
    FieldMap.prototype.add=function(listField,formField,fieldType){
        this[listField]={
            'formField':formField,
            'fieldType':fieldType
        }
    }

    this.add('Status','StatusName','select');
    this.add('Builder','Build Org','select');
    this.add('Route','RouteName','input');
    this.add('DocVersion','Doc Version','input');

    //...  and so on for maybe 20 entries
}

I know that it would be easier to read the data in from a file, but for various reasons, that's not an option here.  Is there a more efficient way to do this?

Before I answer your question, I want to clean some things up first.

this[listField]={
    'formField':formField,
    'fieldType':fieldType
}

Whoa. You are creating an entire new property here based on input a caller gave you. Then, you are setting this to a uniformly designed object every time.

Not good practice
We can OOP this up.

Every time this method is called, an object is created with a certain "shape" (certain properties, etc.)

For that, we create a new class:

function ListField(name, formField, fieldType) {
    ... fill in the rest with property setting ...
}

Now our data is more easily help together.

Time to fix the bad practice mentioned above.

Instead of modifying the very structure of the parent object every time add is called, you should instead push the new ListField object into an array.

This will keep all the added ListField objects in one, easy to access place. This all removes all bad practices.

  Is there a more efficient way to do this?

Sure.

Considering you know that you are going to be sending in a large amount of ListFields, then why not let add take an array?

FieldMap.prototype.add = function(arr) { // use better name
    for(var i = 0; i < arr.length; i++) {
        this.listFields.push(arr[i]);
    }
}

Then, to call, just pass in an array of the various FieldMaps.

No, this will not making typing more easily (well, it will by a little bit). However, since the data you are providing is arbitrary, then there is no way you can do this with patterns and loops.

However, this will speed up your code. Instead of calling a method 20+ times in a row, you are calling this method a single time and this method is using a loop 20+ times; a much more efficient task.


A command-line utility to solve a system of linear equations I was re-checking my answers for a Math paper on my laptop. I needed to verify my solution for a system of linear equations. Unfortunately, I found no quick way to do that. That's why I wrote a (hopefully) simple Python program (using NumPy) to do this job quickly. General comments on the code are appreciated.

linearsolver.py

from sys import argv, exit
from fractions import Fraction
import numpy as np
import string

def solve(A, B, vars=['x', 'y']):
    X = np.linalg.inv(A) * B.transpose()
    values = []
    for item in X:
        values.append(Fraction(item[0, 0]).limit_denominator())
    return dict(zip(vars, values))

def vars(len):
    alph = list(string.ascii_lowercase)
    # x is the 24th alphabet
    start = 23 if len <= 3 else 0
    return list(alph[start:start + len])

def interact():
    print "======================\nLinear Equation Solver\n======================\nHow many equations (and variables) ? ",

    len = int(raw_input())

    # Input the variable names
    vars = []
    print "Enter the variables involved :"
    for i in range(0, len):
        vars.append(raw_input())

    # Input the coefficients
    a = []
    print "Enter the coefficients of the variables in order, for each equation:"
    for i in range(1, len + 1):
        print "-----%d-----" % i
        temp = []
        for i in range(0, len):
            temp.append(float(raw_input()))
        a.append(temp)

    # Input the values they equal to
    print "Enter the constants for the RHS :"
    b = []
    for i in range(0, len):
        b.append(float(raw_input()))

    A = np.matrix(a)
    B = np.matrix(b)
    answer = solve(A, B, vars)
    print_answer(answer)

def print_answer(answer):
    for variable, value in answer.items():
        print "%4s = %4s" % (variable, value)

def main(argv):
    num = len(argv)
    varlist = None
    if num == 1:
        # Only the script name: enter interactive mode
        interact()
        exit()
    elif num == 3:
        script, a, b = argv
    elif num == 4:
        script, a, varlist, b = argv
    else:
        print "Too many arguments."
        sys.exit()

    A = np.matrix(a)
    B = np.matrix(b)

    rows = A.shape[0]
    cols = A.shape[1]

    if rows != cols:
        print "Number of rows must be equal to number of columns."
        exit()

    if varlist is None:
        answer = solve(A, B, vars(rows))
    else:
        answer = solve(A, B, list(varlist))
    print_answer(answer)

if __name__ == '__main__':
    main(argv)

Some example runs:

$ python linearsolver.py "2 5 -9 3; 5 6 -4 2; 3 -4 2 7; 11 7 4 -8" "151 103 16 -32"
   a =    3
   c =  -11
   b =    5
   d =    7
$ python linearsolver.py "5 -11 9; 13 7 -10; -6 5 -8" "gkm" "-125 2 60"
   k =   12
   m =    3
   g =   -4
$ python linearsolver.py
======================
Linear Equation Solver
======================
How many equations (and variables) ?  7
Enter the variables involved :
s
t
u
v
w
x
y
Enter the coefficients of the variables in order, for each equation:
-----1-----
2
-7
3
5
-7
11
8
-----2-----
5
7
-1
9
-6
3
-5
-----3-----
13
11
8
7
4
2
1
-----4-----
12
1
6
5
9
3
11
-----5-----
1
-4
7
-3
9
-2
6
-----6-----
4
1
-6
-3
8
-7
-4
-----7-----
-7
-2
5
1
9
3
-4
Enter the constants for the RHS :
114
-11
-25
-37
4
-91
-50
   s =    5
   u =    8
   t =   -7
   w =   -9
   v =   -6
   y =   -3
   x =    2

List comprehensions

List comprehensions are a cool feature that could shorten your code, at many places.
For example instead of this:

    values = []
    for item in X:
        values.append(Fraction(item[0, 0]).limit_denominator())

You can write like this:

    values = [Fraction(item[0, 0]).limit_denominator() for item in X]

Another example:

    vars = []
    for i in range(0, len):
        vars.append(raw_input())

Better this way:

    vars = [raw_input() for _ in range(len)]

There are some more tips in there:

range(len) is equivalent to range(0, len)
When you don't use the loop variable in the loop body, the convention is to name the loop variable _

There are other places in your code that you could shorten using list comprehensions, I suggest to rewrite them all.

Magic values

The number 23 is a magic value here:

    # x is the 24th alphabet
    start = 23 if len <= 3 else 0

Although the comment explains it, it would be better to put the value in a constant with a name that describes its purpose.
And instead of the hard-coded number, I suggest to express it in a programmatic way, for example:

POS_X = ord('x') - ord('a')

When written this way, we don't even have to know what the value really is (23), the important thing is its relative position from the letter 'a'.

Code duplication

The solve calls are duplicated here, except for the 3rd parameter:

    if varlist is None:
        answer = solve(A, B, vars(rows))
    else:
        answer = solve(A, B, list(varlist))
    print_answer(answer)

It would be better to reorganize the code to eliminate such duplication:

    if varlist is None:
        varlist = vars(rows)

    answer = solve(A, B, varlist)
    print_answer(answer)

Of course, in your current implementation varlist is not really a list.
It would be better to fix that, and make it a list when you assign it earlier, for example:

elif num == 4:
    script, a, symbols, b = argv
    varlist = list(symbols)


C++ linked lists with smart pointers I want to know about smart pointers in C++11, so I started to write my own linked list implementation:

#include <iostream>
#include <memory>

template<typename T>
struct link
{
    T data;
    std::shared_ptr<link> next;

    link(T value) : data(value) {}
    void add_to_end(T value)
    {
        if (next)
            next->add_to_end(value);
        else
            next.reset(new link(value));
    }

};

template<typename T>
class linked_list
{
public:
    linked_list() : head(nullptr) {};
    ~linked_list() {};

    void push(T elem);
    void show(void);
    void reverse(void);
private:
    std::shared_ptr<link<T>> head;
};

template<typename T>
void linked_list<T>::push(T elem)
{
    if (head == nullptr)
        head.reset(new link<T>(elem));
    else
        head->add_to_end(elem);
}

template<typename T>
void linked_list<T>::reverse(void)
{
    std::shared_ptr<link<T>> prev;
    std::shared_ptr<link<T>> next;

    if (head == nullptr) return;

    while (head->next)
    {
        next = head->next;
        head->next = prev;
        prev = head;
        head = next;
    }
    head->next = prev;
}

template<typename T>
void linked_list<T>::show(void)
{
    std::shared_ptr<link<T>> current(head);
    while (current)
    {
        std::cout << current->data << std::endl;
        current = current->next;
    }
}

int main(int argc, char *argv[])
{
    linked_list<int> list;

    list.push(1);
    list.push(2);
    list.push(3);

    list.show();
    list.reverse();
    list.show();

    return 0;
}

I don't like to use shared pointers because they are used when multiple ownership is possible. But in linked list every node owns only the next one node. So I want to use unique_ptr. How can I use them? Unique pointers don't allow assignment which I use in my code.
I perform adding element into the end of list using link::add_to_end call. It looks not very good. How can I improve this code? What changes should I do if I want to write my push_front method to add into the head?
How else can I improve my code? What other problems does it have?

I get a warning from g++ -Weffc++:

144018.cpp: In instantiation of ‘link<T>::link(T) [with T = int]’:
144018.cpp:39:20:   required from ‘void linked_list<T>::push(T) [with T = int]’
144018.cpp:78:16:   required from here
144018.cpp:10:5: warning: ‘link<int>::next’ should be initialized in the member initialization list [-Weffc++]
     link(T value) : data(value) {}
     ^~~~

It's worth building with all the warnings you can stand.  This one is easily fixed by declaring it as

std::shared_ptr<link> next = {};

I found it easier to experiment with your code by encapsulating the specific pointer type within the classes:

template<typename T>
struct link
{
    using pointer = std::shared_ptr<link>;

    T data;
    pointer next = {};
};

template<typename T>
class linked_list
{
    using link_pointer = typename link<T>::pointer;

private:
    link_pointer head = {};
};

Then you can use pointer or link_pointer anywhere that auto won't do.

If I change link::pointer to be a std::unique_ptr, the errors I get are all within reverse() and show() (both of which I would implement outside of the class - the first as a specialization of std::reverse).  But I'm guessing you implemented them as members for the educational value, so I'll continue.  For show, you want to traverse the pointers without transferring ownership, so you can't use unique pointers for your traversal.  But you know you can safely use raw pointers as long as you don't let them leak outside the objects' lifetimes:

template<typename T>
void linked_list<T>::show()
{
    auto current = head.get();
    while (current)
    {
        std::cout << current->data << std::endl;
        current = current->next.get();
    }
}

Your reverse() method does want to take ownership, and can use std::move() to do so:

template<typename T>
void linked_list<T>::reverse()
{
    if (!head) return;

    link_pointer prev;
    link_pointer next;

    while (head->next)
    {
        next = std::move(head->next);
        head->next = std::move(prev);
        prev = std::move(head);
        head = std::move(next);
    }
    head->next = std::move(prev);
}

To add elements in O(1) time rather than O(n) time, maintain a (non-owning) pointer to the tail of the list:

template<typename T>
class linked_list
{
    using link_pointer = typename link<T>::pointer;

private:
    link_pointer head = {};
    link<T> *tail = nullptr;
};

template<typename T>
void linked_list<T>::push(T elem)
{
    auto new_link = new link<T>(std::move(elem));

    if (tail)
        tail->next.reset(new_link);
    else
        head.reset(new_link);
    tail = new_link;
}

Minor style points:

You can write empty argument lists as () rather than C-style (void).
The show() method can be const.
It's a bit more idiomatic to allow (raw or smart) pointer types to convert to bool than to test against nullptr:

{
    if (head)
        head->add_to_end(elem);
    else
        head.reset(new link<T>(elem));
}

Constructors that immediately store their arguments can move from the argument:

link(T value) : data{std::move(value)} {}

This reduces copying (which may become more important when T is bigger than the int used in the test program).

Putting it all together, I get:

#include <iostream>
#include <memory>

template<typename T>
struct link
{
    using pointer = std::unique_ptr<link>;

    T data;
    pointer next = {};

    link(T value) : data{std::move(value)} {}
};

template<typename T>
class linked_list
{
    using link_pointer = typename link<T>::pointer;

public:
    linked_list() = default;
    ~linked_list() = default;

    void push(T elem);
    void show() const;
    void reverse();
private:
    link_pointer head = {};
    link<T> *tail = nullptr;
};

template<typename T>
void linked_list<T>::push(T elem)
{
    auto new_link = new link<T>(std::move(elem));

    if (tail)
        tail->next.reset(new_link);
    else
        head.reset(new_link);
    tail = new_link;
}

template<typename T>
void linked_list<T>::reverse()
{
    if (!head) return;

    link_pointer prev;
    link_pointer next;

    while (head->next)
    {
        next = std::move(head->next);
        head->next = std::move(prev);
        prev = std::move(head);
        head = std::move(next);
    }
    head->next = std::move(prev);
}

template<typename T>
void linked_list<T>::show() const
{
    auto current = head.get();
    while (current)
    {
        std::cout << current->data << std::endl;
        current = current->next.get();
    }
}

int main()
{
    linked_list<int> list;

    list.push(1);
    list.push(2);
    list.push(3);

    list.show();
    list.reverse();
    list.show();
}

For your next steps, I think you'll want to add an iterator type and suitable begin() and end().  Then you'll be able to remove show() and replace it with a simple for (const auto& e: list) std::cout << e;

P.S. Thank you for providing the test program with the code - that certainly makes trying out changes easier!


Looping through cells and deleting column A quick description of what I want:

Loop through a particular cell row and it's columns
If any cell contains 0, then delete the entire column

So let's say 4,4 was 0, then delete that column (D)

My code works but takes a while

Dim lColumn As Long
Dim iCntr As Long
lColumn = 120
For iCntr = lColumn To 4 Step -1
If Cells(4, iCntr) = 0 Then
Columns(iCntr).Delete
End If
Next

Surely there is a faster way to do this.

This version is about 33 times faster:

Dim oUnion As Range
Dim oCell As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
For Each oCell In Range("D4:DS4")
    If oCell.Value = 0 Then
        If oUnion Is Nothing Then
        Set oUnion = oCell
        Else
        Set oUnion = Union(oCell, oUnion)
        End If
    End If
Next
If Not oUnion Is Nothing Then
    oUnion.EntireColumn.Delete
End If
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic

Why is this faster?

Minimize the number of interactions with Excel.

This procedure first gathers a set of all cells that comply with the condition (=0). Then it deletes those cells (columns) in one command, rather than deleting them one at the time. The routine could be made even faster by first reading all values into an array and testing the values of the array in the loop.

For Each loops are faster when you are traversing objects

If you are running through all objects in a collection, a for each loop is generally faster than a counted loop.

Time, time and time again.

If you are optimising code for performance never just assume. Time your code. And time it multiple times too, as other programs (Windows!) might affect your results. It is a good idea to time variants A and B of your routine multiple times in random order. Suspect the result? Time again.


HttpClient error handling I would be happy to hear feedback on my implementation of HttpClient. I am not so strong with error handling so I want to improve. My thought process behind this was that IOExceptions can be recoverable and should be tried again. Where as client exception isn't. Hope to hear your thoughts.

private HttpResponse execute(int httpVerb, boolean authHeader) throws NoInternetConnectionError, IOException, ClientProtocolException {

    try {
        Log.i(TAG, "setting header and executing");
        switch (httpVerb) {
        case HTTP_POST:
            if (authHeader){
                httpPost.addHeader(header);
            }
            return httpClient.execute(httpPost);
        case HTTP_PUT:
            if (authHeader){
                httpPut.addHeader(header);
            }
            return httpClient.execute(httpPut);
        case HTTP_GET:
            if (authHeader){
                httpGet.addHeader(header);
            }
            return httpClient.execute(httpGet);
        default:
            return null;
        }
    } catch (IOException e) {
        InternetChecker internetChecker = new InternetChecker(context);
        if (internetChecker.isInternetConnected() == false) {
            throw new NoInternetConnectionError("internet not connected");
        } else {
            numberOfTries ++;
            if (numberOfTries > 2) {
                throw new IOException(e.getMessage());
            } else {
                Log.i(TAG, "executing for the " + String.valueOf(numberOfTries) + " time");
                return execute(httpVerb, authHeader);
            }
        }
    } finally {
        numberOfTries = 0;
        Log.i(TAG, "finally block run");
    }
}

In the end there was no need to retry manually httpclient has a retry handler.

    HttpRequestRetryHandler retryhandler = new DefaultHttpRequestRetryHandler(6, true);

    httpClient.setHttpRequestRetryHandler(retryhandler);


Looking up a hostname for a given path and client ID I have this code in my library and I wanted to see whether we can avoid duplicating stuff.

private String getAddress(final String path, final int clientId) {
    if (TestUtils.isEmpty(mapping) || TestUtils.isEmpty(mapping.get(path))  || TestUtils.isEmpty(mapping.get(path).get(clientId))) {
        logger.logError("mapping must not be empty. full path= ", path, ", clientId= ", clientId, ", Mapping= ",
                mapping);
        return null;
    }
    final int localId = mapping.get(path).get(clientId);
    final String hostname = getHostname(path, localId);
    return hostname;
}

Here mapping is defined as,

private final Map<String, Map<Integer, Integer>> mapping;

Is there anything we can improve in the above code? I do see duplicated stuff as I am checking in the if statement and then using same thing just below it.

Just don't duplicate it:

Map<Integer, Integer> intermediate = mapping.get(path);
if (intermediate != null) {
    Integer localId = intermediate.get(clientId);
    if (localId != null) {
        return getHostname(path, localId);
    }
}

// log error here
return null;

Alternatively, you could throw the above into a helper function:

private Integer getLocalId(String path, int clientId)
{
    Map<Integer, Integer> intermediate = mapping.get(path);
    if (intermediate != null) {
        return intermediate.get(clientId);
    }
    return null;
}

And then have just the one check:

private String getAddress(final String path, final int clientId) {
    Integer localId = getLocalId(path, clientId);
    if (localId == null) {
        // error
        return null;
    }

    return getHostname(path, localId);
}


Enforcing a constraint that a user may vote on each post at most once I have the following table in a PostgreSQL database:

    creator uuid not null,
    post    uuid not null,
    type    text not null,
    content text not null,
    constraint check_type check
           (type = 'comment' or
               (type = 'vote' and
                   (content = 'up' or
                    content = 'down')))

In order to enforce the constraint that there is no duplicate pair of (creator, post, type) where type = 'vote' (ie. a specific user can only vote on a specific post once) I can either create a unique index like so:

create unique index interactions_index on posts.interactions
    (creator, post, type, content) where type = 'vote';

Or I can query the table on in the server code and do a check there.

My questions:

which one is more efficient, or should I include both for safeguards? How much will having a unique index impact performance (say 100,000 posts @ 1000 likes/post = 100,000,000 database lines)?
I am using this to store "interactions" with a post (ie. likes, comments), is this a good/sustainable way to approach this, or should I be doing something different?

which one is more efficient

Almost surely the unique index will be. Querying back and forth between the server and database is very expensive, and the query that you would have to write to check the table beforehand would do the same thing as the unique constraint - just far less efficiently. The best case is that you'd have to still add an index to the database to get vaguely similar performance, and the connection round-trip expense between the server and database would then be the limiting factor.

All of that said: you aren't asking the more important question, which is

which method gives me a better guarantee of correctness?

Under most circumstances, it's useless to do performance tuning when your data are wrong. The database can guarantee that the data remain valid if you give it a constraint to enforce. If the server is responsible for enforcing the constraint, and it has a race condition or other nasty concurrency edge case, you're setting yourself up for a very difficult debugging session trying to find why duplicates are getting into your database.


C# TCP Socket for sending images Here is my TCP socket that sends eight images per second to a connected Client.
The images are taken from a thermal camera.

static void HandleServer()
        {
            Console.WriteLine("Server is starting...");
            byte[] data = new byte[1024];
            IPEndPoint ipep = new IPEndPoint(IPAddress.Any, 9050);
        Socket newsock = new Socket(AddressFamily.InterNetwork,
                        SocketType.Stream, ProtocolType.Tcp);
            newsock.Bind(ipep);
            newsock.Listen(10);
            Console.WriteLine("Waiting for a client...");

            Socket client = newsock.Accept();
            IPEndPoint newclient = (IPEndPoint)client.RemoteEndPoint;
            Console.WriteLine("Connected with {0} at port {1}",
                            newclient.Address, newclient.Port);
        HandleImage(client, newsock);

    }

    private static void HandleImage(Socket client, Socket s)
    {
        int sent;
        int imageCount = 0;
        double totalSize = 0;
        try
        {
            while (true)
            {

                Image bmp = getImage();
                MemoryStream ms = new MemoryStream();
                // Save to memory using the bmp format
                bmp.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);

                byte[] bmpBytes = ms.ToArray();
                bmp.Dispose();
                ms.Close();

                sent = SendVarData(client, bmpBytes);
                imageCount++;
                totalSize += sent;
                Console.WriteLine("Sent " + sent + " bytes");
                Thread.Sleep(125);

            }
        }
        catch (Exception e)
        {
            Console.WriteLine("Exception catched, probably an interrupted connection. Restarting server...");
            s.Close();
            Console.WriteLine("Transferred {0} images with a total size of {1}", imageCount, totalSize);
            Console.WriteLine("Average JPEG size: " + totalSize / imageCount + " byte");
            HandleServer();
        }
    }
    private static int SendVarData(Socket s, byte[] data)
    {
        int total = 0;
        int size = data.Length;
        int dataleft = size;
        int sent;

        byte[] datasize = new byte[4];
        datasize = BitConverter.GetBytes(size);
        sent = s.Send(datasize);

        while (total < size)
        {
            sent = s.Send(data, total, dataleft, SocketFlags.None);
            total += sent;
            dataleft -= sent;
        }
        return total;
    }
}
}

Any feedback is appreciated:)

Update: getImage simply calls the driver DLL of the thermal camera and returns the actual Image.
I do this every 125ms because the camera is specified with 8fps and does NOT provide a method or property to check whether there is a new image available.

This isn't quite true:

  sends eight images per second

The 125ms sleep means that your code will sit around blocking a thread for about ~1/8th of a second (Thread.Sleep shouldn't be use for accurate timing ever) after it sends every image. This means that the time between images is ~125ms plus the time to actually send an image (i.e. no more than ~8 images per second, could be much fewer). This pretty much guarantees that you will drop a frame now and then.

You say that getImage() may or may not be providing a new image, and there is no nice way to check if a new image is available; however, you might consider interrogating the image received and checking if it is the same the one you last sent (and then don't send it); you can do this cheaply enough by taking a hash of the data stream. If getImage() itself isn't too expensive, then this would enable a quicker response rate, and ensure 8fps, rather than the 7-or-8 fps that a method like this will give you (assuming sending the image data is reasonably fast). If this makes sense will depend on the resource available, and how much you care about receiving every frame.

It would also be nice if you could avoid locking up a whole thread, as that sort of methodology does not scale (you run out of threads). An asynchronous HandleServer/HandleImage combination which makes use of Task.Delay() to schedule work might be a good solution, and would not significantly increase the complexity of the implementation.

Don't leave comments to rot:

// Save to memory using the bmp format
bmp.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);

Such a comment is probably not worth the space it consumes, and it's unhelpful when it is out of date like this.

Calling HandleServer() within HandleImage() means that every exception will consume a little more of the stack frame, until eventually the program crashes. You might consider putting the logic for handling exceptions outside of both of these methods, and putting it in a loop; for example, something like:

while (keepRunning)
{
    try
    {
        HandleServer();
    }
    catch (Exception ex)
    {
        // do whatever
    }
}

Not only will this save slowly filling the stack, but it means that HandleImage can focus on handling images and not restarting the server when it goes wrong.

The difficulty with this is that you currently print some information when catching an exception, to which this alternative does not lend itself. (you could catch the expcetion, print everything out, and then throw it again: this way HandleImage is still only concerned with sending lots of images, and the code above it takes the role of deciding how to deal with other failures (e.g. network errors)).

You include the exception in your catch clause, but you don't report anything about it. It would be much better to print out the exception rather than the assumption that it is a network error: that way, when something goes wrong in production there is some hope of debugging it.

This code needlessly allocates an extra byte[].

byte[] datasize = new byte[4];
datasize = BitConverter.GetBytes(size);

It can be simplified to this:

byte[] dataSize = BitConverter.GetBytes(size);

Your code to send the data to the server is suspect: when sending the size-data, you don't check the return value, but you have a corresponding loop to send the image data. I'm not sure how socket and TCP get along, but if you don't trust data to send in one go, you should be giving sizeData the same treatment (you can put the loop in it's own method to keep it simple).

Consider using the using statement (as suggested by Jesse C. Slicer in the comments) to ensure proper disposal of disposables. For example, in HandleImage:

byte[] bmpBytes;

using (Image bmp = getImage())
using (MemoryStream ms = new MemoryStream())
{
    bmp.Save(ms, System.Drawing.Imaging.ImageFormat.Jpeg);
    bmpBytes = ms.ToArray();
}

This will dispose the resources in the event that an exception if thrown in there somehow, and moves the released resources into a different scope, which means they can't be misused as easily (you can't forget to dispose them, and you can't use once when are disposed).

It's generally a good idea to define variables as close to their usage as possible. sent in HandleImage, for example, is declared on the first line of the method, but only used inside the while loop; it's purpose would be clearer if it was defined and assigned on the same line, and it would be impossible to use outside of its meaningful scope. total in SendVarData is also initialised first thing, but has no role in first piece of logic in the method.

These methods would really benefit from some inline-documentation (///) explaining what their role and behaviour is: I would not assume a method called HandleImage() could ever start a TCP server, so this needs to be written down somewhere (though ideally that would be changed, either as suggested above or through some other refactoring).

Small things:

Your 'bytes sent' reporting doesn't include the 4 bytes sent for the length-prefix.
The past-participle of 'catch' is 'caught', rather than 'catched'.
The variable data in HandleServer is never used.
Be consistent with your use of white-space (e.g. the gaps between methods, and indentation in HandleServer, etc.)
You might consider reusing the memory stream; we don't know how large the files are, but taking a new memory stream for each image may ruin your program's memory characteristics, and will necessarily degrade performance to some extent.
static void HandleServer() is missing an accessibility modifier (which are otherwise present)
double totalSize = 0; should not be a double (64bit float-point); did you mean long (64-bit integer)?


Finding the max width of the tree 1
    /  \
   2    3
 /  \     \
4    5     8
          /  \
         6    7

  Width of a tree is maximum of widths of all levels.

  For the above tree:

  width of level 1 is 1
  width of level 2 is 2
  width of level 3 is 3
  width of level 4 is 2

  So the maximum width of the tree is 3.

This code is attributed to GeeksForGeeks. I'm looking for code-review, best practices and optimizations. I'm verifying space complexity to be \$O(n)\$.

public class MaxWidthOfTree<T> {

    private TreeNode<T> root;

    public MaxWidthOfTree(List<T> items) {
        create(items);
    };

    private void create(List<T> items) {
        if (items.isEmpty())  {
            throw new NullPointerException("The input array is not empty.");
        }

        root = new TreeNode<>(items.get(0));

        final Queue<TreeNode<T>> queue = new LinkedList<TreeNode<T>>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode<T> current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode<>(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode<>(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    private static class TreeNode<T> {
        private TreeNode<T> left;
        private T item;
        private TreeNode<T> right;

        TreeNode(T item) {
            this.item = item;
        }
    }

    public int getMaxWidth() {
        if (root == null) {
            throw new IllegalArgumentException("The root is empty");
        }

        int h = getHeight(root);
        int[] arr = new int[h + 1]; // NOTE: h + 1

        computeWidth(root, arr, 0);

        return maxElementOfArray (arr);
    }

    private int maxElementOfArray(int[] arr) {
        int max = Integer.MIN_VALUE;
        for (int i = 0; i < arr.length; i++) {
            if (arr[i] > max) {
                max = arr[i];
            }
        }
        return max;
    }

    private void computeWidth(TreeNode<T> node, int[] arr, int level) {
        if (node == null) return;
        arr[level]++;  // NOTE: you can place this as preorder, inorder or postorder, does not matter.
        computeWidth(node.left, arr, level + 1);
        computeWidth(node.right, arr, level + 1);
    }

    public int getHeight(TreeNode<T> node) {
        if (node == null) return -1;
        return Math.max(getHeight(node.left), getHeight(node.right)) + 1;
    }
}

public class MaxWidthOfTreeTest {

    @Test
    public void testLevel3() {
        MaxWidthOfTree<Integer> mwot1 = new MaxWidthOfTree<>(Arrays.asList(1, 2, 3, 4, 5, null, 7, null, null, null, null, null, null, 14, 15));
        assertEquals(3, mwot1.getMaxWidth());
    }

    @Test
    public void testTestLevel4() {
        MaxWidthOfTree<Integer> mwot2 = new MaxWidthOfTree<>(Arrays.asList(1, 2, 3, 4, 5, null, 7, null, null, 14, 15, null, null, 16, 17));
        assertEquals(4, mwot2.getMaxWidth());
    }
}

Similar code, so similar answer.

Please take these things to heart; I feel like I'm wasting my time otherwise.

    return maxElementOfArray (arr);

You have a space between the function name and the parentheses.

Finalize your arguments if you're not gonna do anything with them. Final your locals if you're not gonna do anything with them.

I feel your approach is completely wrong. Why load the whole tree structure into memory? Because you're copying the same code over and over. And that turns you into a blind programmer.

Instead, try this:

Add depth to TreeNode. private int depth;

Add depth to the constructor of TreeNode as the second argument.

Then declare root to be at depth 1.

Declare an array of int (we'll call it widths). Size 32; you can't get deeper with your syntax for defining a tree. (Maybe you can, I'm not sure how well a list containing 2^32 items will function).

Next, parse the list.

Whenever you add a new node to the queue, widths[current.depth+1]++. Whenever you create a new node, pass current.depth+1 as the second argument.

When you're done parsing the list, return maxElementOfArray(widths).

That might be considered cheating though; we're not really using a tree. Anyway.

    if (items.isEmpty())  {
        throw new NullPointerException("The input array is not empty.");
    }

Erm... yes it is. Maybe "The input list shouldn't be empty". Also NullPointerException is NOT valid here; use IllegalArgumentException instead (I didn't pass null! I passed empty list! If I get NullPointerException back when I pass in non-null objects then I spend a lot of time debugging, trying to find my null.)

    int h = getHeight(root);
    int[] arr = new int[h + 1]; // NOTE: h + 1

Use int height, it's more descriptive. Get rid of the comment or write WHY. Right now it could have been //<--- important! too, and that doesn't help anyone. Maybe consider using int[] arr = new int[getHeight(root) + 1]; as you don't reuse this h anywhere.

Your maxElementOfArray function returns Integer.MIN_VALUE. There's no such thing as a negative width. Return 0 instead; the top level contains 0 items. There are no other levels.

You perform three traversals of the entire tree; create is one, getHeight is two, computeWidth is three. I'd combine getHeight and computeWidth. You'll have to rewrite a couple things for this... but I believe I gave some pointers already.


Link to WinRT XAML code optimization I need a method which will parse text to RichTextBlock-compatible XAML markup with clickable links.

Example:

Input:

"Hello, check new service for students! Check edusty.ru and http://edusty.ru/test?text1=abc&args=123 sites. Check edusty.ru/privacy.html to read privacy policy."

Output:

<Paragraph xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" TextAlignment="Left" FontSize="20"  FontWeight="Normal" FontStyle="Normal" FontStretch="Normal" >
     <Run Text="Hello, check new service for students! Check "/>
     <Hyperlink NavigateUri="http://edusty.ru">edusty.ru</Hyperlink>
     <Run Text=" and "/>
     <Hyperlink NavigateUri="http://edusty.ru/test?text1=abc&amp;args=123">http://edusty.ru/test?text1=abc&amp;args=123</Hyperlink>
     <Run Text=" sites. Check "/>
     <Hyperlink NavigateUri="http://edusty.ru/privacy.html">edusty.ru/privacy.html</Hyperlink>
     <Run Text=" to read privacy policy."></Run>
</Paragraph>

I wrote this method:

public static string ParseLinksToXaml(this string source)
        {
            var transformedText = Regex.Replace(source,
                @"((https?:\/\/)?(ftps?:\/\/)?[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,6}(:[0-9]{1,5})?(\/\S*)?)",
                @"""/> <Hyperlink NavigateUri=""$1"">$1</Hyperlink> <Run Text=""", RegexOptions.IgnoreCase);
            transformedText = WebUtility.HtmlEncode(transformedText);
            var matches = Regex.Matches(transformedText, @"&quot;\/&gt;\s&lt;Hyperlink NavigateUri=&quot;.*?&quot;&gt;|&lt;\/Hyperlink&gt;\s&lt;Run Text=&quot;");
            foreach (var match in matches)
                transformedText = transformedText.Replace(match.ToString(), WebUtility.HtmlDecode(match.ToString()));

            matches = Regex.Matches(transformedText, @"NavigateUri=\""(.*?)"">");
            foreach (Match match in matches)
            {
                var link = match.Groups[1].ToString();
                link = link.Replace("&", "&amp;");
                link = link.Contains("://") ? link : "http://" + link;
                transformedText = transformedText.ReplaceFirst("\"" + match.Groups[1] + "\"", "\"" + link + "\"");
            }
            var xaml = @"<Paragraph xmlns=""http://schemas.microsoft.com/winfx/2006/xaml/presentation"" TextAlignment=""Left"" FontSize=""20""  FontWeight=""Normal"" FontStyle=""Normal"" FontStretch=""Normal"" >
                                    <Run Text=""" + transformedText + @"""></Run></Paragraph>";
            return xaml;
        }

It works, but the code really looks bad. Can anyone help rewrite/optimize this?

Whenever you're concatenating strings inside a loop's body, +-concatenation should be ruled out; transformedText would be a StringBuilder instance, to which each iteration would Append.

However XAML is a subset of XML; as such, there is no reason to treat your output as purely strings: using the classes in the System.Linq.Xml namespace (otherwise known as "Linq-to-Xml"), you can rewrite this in a much more elegant and object-oriented way.

Extending System.String like this is a bit overkill, too. I'm not sure I'd like working in a code base where every single string out there has this ParseLinksToXaml extension method: that functionality is very specialized, and IMO deserves its own class where the client code is actually explicit about what's going on:

var parser = new StringLinksToXamlConverter();
var xaml = parser.ParseLinksToXaml(myString);

The string-building stuff going on under the hood should be an implementation detail - the returned value would be potentially much more useful to the client code if an XDocument was returned, leaving it up to the client code to query or output as a string.


Help me make my Python code clearer Here's my solution to Project Euler problem 40.

import operator
import math

def levels(n):
    return 9 * n * 10 ** (n - 1)

def face_value(i):
    def level_filler(i):
        n = 1
        yield 1

        while i > levels(n):
            yield int(levels(n)/n * 1.0)
            i -= levels(n)
            n += 1

        yield int(math.ceil(i / n * 1.0))

    if i == 1: return 1
    else:
        return sum(level_filler(i))

def d(i):
    j = str(face_value(i))
    k = len(j)
    l = sum(levels(x) for x in xrange(1, k)) + 1
    m = (i-l)%k

    return int(j[m])

# d(1) * d(10) * d(100) * d(1000) * d(10000) * d(100000) * d(1000000)
print(reduce(operator.mul, (d(10**i) for i in xrange(7))))

It answers without building the actual string, and gets the value of d(i) in (quasi-)linear time.

The problem is that it's messy and I feel it's not easy to read. I cannot even find proper names for my variables.

I was wondering if anyone could give me a code review tell me how my ideas are unclear and how can I express them better.

(I tested the code on Python 2.6)

1. Bug

Your program contains a bug. The second digit of 123456... is 2, but:

>>> d(2)
3

However, solving Project Euler problem 40 only relies on you having the correct result for d(10**i), and this you got right.

2. Improving your solution

There are no docstrings, so even though I have solved Project Euler problem 40 myself, it took me a while to figure out what your functions are doing. Even if you struggle to pick good names for functions (and this can be very hard), you can always make things clearer with a brief description and some examples.

There are also no doctests. Even a handful of doctests might well have found the bug.

The discipline of writing down what each function actually does will make it easier for you to choose good names. For example, once you have written the docstring for your function levels, it should be clear that a better name would be something like total_digits:

def total_digits(n):
    """
    Return the total number of digits among all the `n`-digit decimal
    numbers.

    >>> total_digits(1) # 1 to 9 (note that 0 is not included)
    9
    >>> total_digits(2) # 10 to 99
    180
    """
    return n * 9 * 10 ** (n - 1)

Choosing good names for variables is particularly hard when writing mathematical code (as when solving problems from Project Euler). It's typical of mathematical code that variables contain a number whose meaning is hard to capture in a short name. Long names would end up making your code hard to read, so it's often the case that the best approach is to give your variables arbitrary names like i, j, m, and n, and to explain their meaning in longer comments.
There's no need for level_filler to be a local function inside face_value (it doesn't make use of any context from face_value). If you moved it out to the top level then it would be easier to test it. (But in fact, it will be better just to remove it, as I will explain below.)
The expression int(levels(n)/n * 1.0) is the same as int((levels(n) / n) * 1.0) since multiplication and division have the same precedence and associate from left to right. So this expression converts levels(n) / n to a float and then back to an int, which is pointless. Also, since levels(n) is n * 9 * 10 ** (n - 1), when divided by n the result is always 9 * 10 ** (n - 1). It would be better to write this directly and avoid the useless multiplication by n followed immediately by division by n.
Similarly the expression int(math.ceil(i / n * 1.0)) doesn't do what you think it does: the division happens before the multiplication, so in Python 2 (where / truncates the result) this expression is actually the same as i // n and evaluates to the floor of the division, not the ceiling.
You make three calls to levels(n) in succession. It would be better to make just one call and remember the result.
The purpose of face_value(i) function is to return the number which contains the ith digit. This is where the bug lives:

>>> face_value(2)
3

In some sense you knew that this was incorrect, because you compensate for the error with your test if i == 1: return 1!

3. A better solution?

My own approach to this problem would be to simplify the structure as much as possible. The function levels is so simple (just a single expression) that we can easily inline it into face_value. And then various other simplifications become possible. Here's my version of your face_value:

def number_containing(i):
    """
    Return the number containing the `i`th digit in the sequence
    123456789101112...

    >>> number_containing(5)
    5
    >>> number_containing(13)
    11
    >>> number_containing(99)
    54
    >>> number_containing(100)
    55
    """
    n = 1                       # Number of digits.
    first = 1                   # The first n-digit number.
    count = 9                   # Count of n-digit numbers.
    while True:
        # The i-th digit is in the j-th n-digit number (but not if
        # there are more than j n-digit numbers).
        j = (i + n - 1) // n
        if j <= count:
            return first + j - 1
        first += count
        i -= n * count
        n += 1
        count *= 10

Having computed which number contains the ith digit, we then have to work out which digit it is, which you do using another sum over levels. In fact, it is much simpler to work out which digit you want at the same time as you work out which number this digit comes from. If you look at my function above, you'll see that all the information is already present at the point where we return the result. So it's easy just to return the digit at this point:

def d(i):
    """
    Return the `i`th digit of the sequence 123456789101112...

    >>> d(5)
    5
    >>> d(15)
    2
    >>> d(100)
    5
    >>> sequence = ''.join(str(i) for i in xrange(1, 10001))
    >>> all(d(i + 1) == int(digit) for i, digit in enumerate(sequence))
    True
    """
    n = 1                       # Number of digits.
    first = 1                   # The first n-digit number.
    count = 9                   # Count of n-digit numbers.
    while True:
        # Check the loop invariants.
        assert first == 10 ** (n - 1)
        assert count == 9 * first

        # The i-th digit is in the j-th n-digit number (but not if
        # there are more than j n-digit numbers).
        j = (i + n - 1) // n
        if j <= count:
            return int(str(first + j - 1)[(i - 1) % n])
        first += count
        i -= n * count
        n += 1
        count *= 10

Note that the last doctest is quite thorough!


Remove elements compared with other elements in a Set based on a condition Simplified the actual problem to focus on comparing/removing performance.

Given a set S with n elements , find the most optimal way to compare each element to all others until a condition is met that decides if and which item to remove. I experience long runtimes when n is high (e.g. n=3000 has a runtime of 2500ms on an intel i5-6500 cpu)

An element is a Tuple object (itv, t) where itv holds an integer interval value (between 0 and 4, inclusive) and t holds an integer total benefit value.

The condition: Given Tuple s and another Tuple s', if itv == itv', then if t <= t' remove s, else remove s'.

A naïve example. S={(1,20),(0,40),(1,35)}

Compare (1,20) with (0,40): itv != itv', continue.
Compare (1,20) with (1,35): itv == itv', t < t', remove (1,20) from S.
Compare (0,40) with (1,34): itv != itv', return S.

Due to Java's constraints on removing elements while iterating, for-loops are wildly inefficient and I currently use a nested foreach-loop inside an Iterator loop. However, I still feel like it underperforms. This thought is mostly because the code looks terrible, in my opinion.

Below is my implementation. It tries to reduce the amount of unnecessary comparisons as much as possible. My code uses an outer Iterator for comparing the main Tuple to the others. In the case that the interval matches and the main Tuple's total benefit is worse, the inner loop is stopped, the main Tuple is removed and we continue with the next Iterator item. If the inner Tuple is worse, however, then the inner Tuple is marked for deletion and will be deleted at the end of the outer loop iteration.

Does anyone see any glaring mistakes, have any comments about performance (expensive calls) or refactoring for code clarity?

public static HashSet<Tuple> compareAndRemove(HashSet<Tuple> set) {
    HashSet<Tuple> bestTuples = new HashSet<>(set.size()); //Return set
    Collection<Tuple> tupleRemovals = new HashSet<>(); //Tuples to remove this iteration
    Iterator<Tuple> s_iter;

    do {
        set.removeAll(tupleRemovals);
        removedElements += tupleRemovals.size();
        tupleRemovals = new HashSet<>();

        s_iter = set.iterator();
        if (s_iter.hasNext()) {
            Tuple s = s_iter.next();
            if (!s_iter.hasNext()) { //Last element; nothing to compare anymore
                bestTuples.add(s);
                s_iter.remove();
                break;
            }

            //Compare with other
            boolean removeMainTuple = false;
            for (Tuple s_prime : set) {
                boolean equalIntervals = true;
                if (s.equals(s_prime)) continue; //Skip same element

                //Check interval
                if (s.itv == s_prime.itv) {
                    //Decide which Tuple to discard
                    if (s.t <= s_prime.t) {
                        removeMainTuple = true; //Remove Iterator element
                        break;
                    } else {
                        tupleRemovals.add(s_prime); //Remove foreach element
                    }
                }
            }
            //Move current item to return set if it was the best
            if (!removeMainTuple) {
                bestTuples.add(s);
            }
            s_iter.remove();
        }
    } while (s_iter.hasNext());

    bestTuples.addAll(set);

    return bestTuples;
}

In the case where you just want to find the best tuples, the code can be much simpler:

find the Tuple with the best t for each value of itv (sounds like a job for Map)
put the best bunch of Tuple into a Set, and return it

public static Set<Tuple> bestTuples(Set<Tuple> set) {

    Map<Integer, Tuple> bestTuplesByItv = new HashMap<Integer, Tuple>();
    Tuple bestTupleSoFar;
    for (Tuple t: set) {
        if ((bestTupleSoFar = bestTuplesByItv.get(t.itv)) == null || t.t > bestTupleSoFar.t) {
            bestTuplesByItv.put(t.itv, t);
        }
    }

    Set<Tuple> bestTuples = new HashSet<Tuple>();
    bestTuples.addAll(bestTuplesByItv.values());

    return bestTuples;

}

This should also be faster - your solution has some nested loops, O(n^2), but the suggestion above just iterates through the entire set once. My gut tells me that removing things from a Set is slower than just making a new set with the things we want, but I'm not sure about HashSet.

Since the values for the Map are coming from a HashSet, all the values are guaranteed to have unique hash value. Since that's the case, we don't need a LinkedHashMap - a plain HashMap will work just fine (LinkedHashMap creates linked lists to deal with hash collisions - but that's unnecessary as we won't have any).

Your code seems to be mutating the Set that is passed in as a parameter though - if that's desired behaviour, it's a trivial modification from the code above - just clear the values in set and add the values we want before returning:

public static Set<Tuple> compareAndRemove(Set<Tuple> set) {

    Map<Integer, Tuple> bestTuplesByItv = new HashMap<Integer, Tuple>();
    Tuple bestTupleSoFar;
    for (Tuple t: set) {
        if ((bestTupleSoFar = bestTuplesByItv.get(t.itv)) == null || t.t > bestTupleSoFar.t) {
            bestTuplesByItv.put(t.itv, t);
        }
    }

    set.clear();
    set.addAll(bestTuplesByItv.values());

    return set;

}


Compare File with ignore some line/last line We have two log files in which one is generated from a process. We compare one file(golden file) to another file to check if file correct or not. It should have same value. We generally use diff utility to compare two files. I have got enhancement to add machine information into process generated File. So I want to compare upto previous line and ignore  new changes.
Could Anyone provide me any utility which I can use in python.

Golden File

CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -rise -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.06 -slew 0.1 -temp -40 -rise -clock
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -fall -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -fall -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.06 -slew 0.1 -temp -40 -rise -clock
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -rise -data
Temp1   Temp2   Temp3   Temp4   Temp5   Temp6
-31.00     -19.00      -3.00       -8.00      43.00      61.00

Process File

CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -rise -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.06 -slew 0.1 -temp -40 -rise -clock
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -fall -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -fall -data
CMD gen -vdd 0.99 -vss 0 -sinps 0.06 -slew 0.1 -temp -40 -rise -clock
CMD gen -vdd 0.99 -vss 0 -sinps 0.02 -slew 0.1 -temp -40 -rise -data
Temp1   Temp2   Temp3   Temp4   Temp5   Temp6
-31.00     -19.00      -3.00       -8.00      43.00      61.00
 Adding machine name( ignore machine name)

I have write code in following.Can we better way for improve code

data = None
with open("Golden_File",'r+') as f:
    data = f.readlines()
del data[-1]

data_1 = None
with open("cp.log",'r+') as f:
    data_1 = f.readlines()
del data_1[-1]
print cmp(data, data_1)

[Question]: Does cmp function works fine in list also. I have used first time and not sure how internally works.

There is no need for python here, you can use the head command to filter out the last line of a file:

diff <(head -n -1 "Golden_File") <(head -n -1 "cp.log")


Bash function to find contents of a file with a certain extension I have to read a lot of code and search through many code bases. I frequently find myself using something like grep -r --include=.C "test string" . to find files containing a certain string or regular expression. However, to simplify this and expedite the process, I want to create a function that allows me to specify what I am looking for, what are acceptable file extensions and what are inacceptable file extensions.

The function I have written is below. I am not sure if this follows best practices. Also, the hack using the brace expansion with the .\? is bad. I really appreciate any feedback that you have.

findInFile() {
  # Function to be placed in bash profile
  # Allows user to specify what contents of a file they are looking for
  # in what file types and in what NOT file types
  # Iterate over the number of scripts arugments
  # TODO: How can I create documentation for this function outside of code comments?
  while [[ "$#" -gt 0 ]]
  do
    case $1 in
      -ft|--fileTypes)
        local fileTypes=$2
        ;;
      -et|--excludeTypes)
        local excludeTypes=$2
        ;;
      *) # Catch the case where user does not specify
         # these arguments
        local searchTerm=$1
        ;;
    esac
    shift
  done

  echo "fileTypes: $fileTypes"
  echo "excludeTypes: $excludeTypes"
  echo "searchTerm: $searchTerm"

  # TODO: Should probably clean up with case statement
  # TODO: I am using this \? in the include and exclude as a hack
  # to catch the case where only one file type is provided.
  if [ -n "$fileTypes" ] && [ -n "$excludeTypes" ]
  then
    #searchString="grep -r --include=\*{$fileTypes} --exclude=\*{$excludeTypes} "$searchTerm" ."
    searchString="grep -r --include=\*{$fileTypes,.\?} --exclude=\*{$excludeTypes,.\?} "$searchTerm" ."

  elif [ -n "$fileTypes" ]
    then
      #searchString="grep -r --include=\*{$fileTypes} "$searchTerm" ."
      searchString="grep -r --include=\*{$fileTypes,.\?} "$searchTerm" ."
  elif [ -n "$excludeTypes" ]
    then
      #searchString="grep -r --exclude=\*{$excludeTypes} "$searchTerm" ."
      searchString="grep -r --exclude=\*{$excludeTypes,.\?} "$searchTerm" ."
  else
      searchString="grep -r "$searchTerm" ."
  fi

  #searchString="grep -r --include=\*{$fileTypes} "$searchTerm" ."
  echo "searchString: $searchString"
  eval $searchString

  # TODO: Allow the user to type a number to then programmatically jump to that
  # file in the text editor of their choice

}

You can use [[ .. ]] instead of [ .. ] to do tests.  The former is a bash builtin and saves a fork.

You don't need to eval anything since you're just building a couple of optional switches to grep.  Start with empty strings and populate variables or an array, and pass the result to an invocation of grep as variables.  This also avoids enumerating every possible combination of arguments (already 4 combos with 2 options -- that approach quickly becomes unsustainable).

There's no need to absorb searchTerm.  Just leave it in the arguments and pass all those to grep, which allows you to include grep switches too, like -i.

Use set +x to see what's going on.

Tying it all together:

findInFile() {
    :<<_comment_
        Function to be placed in bash profile
        Allows user to specify what contents of a file they are looking for
        in what file types and in what NOT file types
        Iterate over the number of scripts arugments
_comment_
    declare -a select
    while [[ "$#" -gt 0 ]]
    do
        if [[ $1 =~ ^(-ft|--fileTypes|-et|--excludeTypes)$ ]]
        then
            local type="$2"
            [[ "$type" == *,* ]] && type="{$type}"
            if [[ $1 == *-f* ]]
            then
                select+=( "--include=*$type" )
            else
                select+=( "--exclude=*$type" )
            fi
            shift 2
        else
            break
        fi
    done
    set -x
    grep -r ${select[@]} "$@" .
    { set +x; } 2>/dev/null
}

You can include long comments as here-docs piped to the null operator :.


Numpy array slicing/reshape/concatination I'm sure my question exists on the internet, i just don't know the right formulations.

I have a data-sample as input for a NN. This sample has the shape (1, 8, 28, 80).
Basically it is 80 timesteps of an image. -> y=8, x=28, time=80

i can extract the image at time=0 with:

np_img = image.data.numpy()   # shape (1, 8, 28, 80)
t0 = np_img[:, :, :, 0][0]

in order to be able to plot the images at each timestamp below each other, resulting in an array of (640, 28), ergo concatenating along the y-axis I do:

amount_timeslots = img.shape[-1]
new_array = img[:, :, :, 0][0]

for i in range(1, amount_timeslots):
    ti = img[:, :, :, i][0]
    new_array = np.concatenate((new_array, ti))

new_array.shape  # (640, 28)

Is there a more pythonic way by using build in numpy magic to do this?

concatenate

There is no need to do the concatenation pairwise np.concatenate([img[:, :, :, i][0] for i in range(img.shape[-1])]) should improve the speed already

numpy.moveaxis

You can use numpy.moveaxis

new_array2 = np.concatenate(np.moveaxis(img[0],(0,1,2), (1,2,0)))

To check whether the result is the same:

assert np.array_equal(new_array, new_array2)

Other improvements

Since you don't use the first axis of the image, you can do np_img = image.data.numpy()[0] so prevent you repeating the [0] in all subsequent code


Handle OnClick in Jquery or C# I am working on a webforms project using c#/aspx/jquery. I'm not sure where to handle my Onclick. Both options (doing it in C# or jquery) seem feasible. Which would be better practice?

Here is the scenario:

I want to build a checkbox which will toggle a textbox's textmode, altering it between Password and SingleLine.

Currently, I have this following code for my markup since I am currently handling it in C#:

<asp:Checkbox ID="CheckBox1" OnCheckedChanged="CheckBox1_OnCheckedChanged"/>

The code:

protected virtual void OnCheckedChange(EventArgs e)
{
     If (CheckBox1.TextMode == TextMode.Password)
          CheckBox1.TextMode = TextMode.SingleLine;
     else
          CheckBox1.TextMode = TextMode.Password;
}

The use case of this is: A user is entering his password. On the same page, he may choose to keep the password hidden or not by checking the checkbox.

You cannot change the type of the input field on client side in some or all browsers. I would create a custom hiding method in Javascript and leave the input type in text mode (SingleLine), but maybe this has some drawbacks in security.

Other solution is to remove the original input and create a new one on the fly with Javascipt but i think not all browser will let you to read out the value of a password type input.

Third version is to send the field to the server side and get back the new HTML fields but in this way you may send multiple times the password to the server which can be a security hole.


PHP - Importing columns from uploaded CSV file to SQL database (Laravel 5.7) The goal is to upload the columns from the csv into the SQL Pro database. I am able to select the column names that have been uploaded from the csv file, the columns from the csv file are then uploaded into the option dropdown.  In return I need to submit the selections that have been selected from the options dropdown, into the database. I am writing this in PHP. What I have is definitely not correct.

PHP

public function csvProcess(Request $request)
    {

        $contact = newContact();
        $contact->first_name = $rowProperties[$mapping['column_first_name']];
        $contact->last_name = $rowProperties[$mapping['column_last_name']];
        //$contact->newContact()->attach($first_name[$mapping['column_first_name']]);
        store(newContact()->where->$columnNames=$firstrow());
        $result = create($this(newContact()));

        $contact->save();
}

HTML

 Contact Fields

    {{field}}
    <select :id="field" :name="field" v-model="mapping[field]"required>
      <option value="">Please Select</option>
      <option v-for="columnName in columnNames" :name="columnName" :value="columnName"  value="">{{columnName}}</option>
    </select>

Javascript

     submitFiles()
        {
          //Form validation that will not allow a user to submit the form by upon the @click event
          if(!this.$refs.csvFile.checkValidity())
          {
            return null;
          }
          let formData = new FormData();
          formData.append('file', this.file);
          axios.post('http://localhost:8080/api/contacts/upload-csv',
          formData,
          {
          headers:
            {
              'Content-Type': 'multipart/form-data'
            }
          }
          )
          .then(function(response){
            console.log('SUCCESS!!');
            console.log(response);

          })
          .catch(function(response){
            console.log('FAILURE!!');
            console.log(response);

          })
        }

      }
    }
    </script>

I defined the fields that I wanted to be required first. Then, I defined a One-to-Many relationship between an account table and a contacts table where the contact id is the account's name in the account's table in the SQL database

        $account = new Account();
                $mapping = $request->$mapping;
                $account->name = $rowProperties[$mapping['account_name']];

                //One-To-Many Eloquent Relationship that links a table of Account Names in the Account's
                //table to contact Account_ID's in the Contact tables
                //$contact->id = $account->id;

                $account->save();

                $contact = new Contact();
                $contact->id = $account->id;
                $contact->contact_id = $rowProperties[$mapping['contact_account_name']];
                $contact->first_name = $rowProperties[$mapping['contact_first_name']];
                $contact->last_name = $rowProperties[$mapping['contact_last_name']];

    account = new Account();
                $account->name = $rowProperties[$mapping['account_name']];
                $this->account->hasMany('contact_account_names');
                //$accountid->contact_account_name
                $account->save();


Simple rock-paper-scissors game This is a simple rock-paper-scissors program implemented using C++. Any feedback or suggestions for improvement are welcome.

Code:

main.cpp:

#include <iostream>
#include <string>
#include "rock-paper-scissorss.h"
using namespace std;

void whoWon(string player_move, string computer_move){
    if(player_move == "rock"){
        if(computer_move == "paper")
            cout<<"You chose rock and the computer chose paper, the computer wins!"<<endl;

        else if(computer_move == "scissors")
            cout<<"You chose rock and the computer chose scissors, you win!"<<endl;

        else
            cout<<"You chose rock and the computer chose rock, it's a tie!"<<endl;
    }

    else if(player_move == "paper"){
        if(computer_move == "scissors")
            cout<<"You chose paper and the computer chose scissors, the computer wins!"<<endl;

        else if(computer_move == "rock")
            cout<<"You chose paper and the computer chose rock, you win!"<<endl;

        else
            cout<< "You chose paper and the computer chose paper, it's a tie!"<<endl;
    }

    else{
        if(computer_move == "rock")
            cout<<"You chose scissors and the computer chose rock, the computer wins!"<<endl;

        else if(computer_move == "paper")
            cout<<"You chose scissors and the computer chose paper, you win!"<<endl;

        else
            cout<<"You chose scissors and the computer chose scissors, it's a tie!"<<endl;
    }
}

int main(int argc, char** argv) {
  \
  char play_again = 'y';
  cout<<"Please enter your name: "<<endl;
  string name;
  getline(cin,name);
  cout<<"Hello "<<name<<", welcome to Rock-Paper-Scissors!"<<endl;

    Player player_1(name);
  while(play_again == 'y'){

    cout<<"Please enter the move you wish to make: "<<endl;
    string move;
    getline(cin,move);
    while(1){
        if((move == "rock") || (move == "paper") || (move == "scissors")){

            player_1.setMoveChoice(move);
            break;
        }

        else{
            cout<<"Invalid move choice! Please enter rock, paper, or scissors"<<endl;
            getline(cin,move);
        }
    }

    Computer com_1;
    com_1.setMoveChoice();

   string p_move = player_1.getMoveChoice();
   string c_move = com_1.getMoveChoice();

   whoWon(p_move,c_move);

   cout<<"Would you like to play again?(y/n)"<<endl;

   cin>>play_again;
   cin.ignore();

    }

 return 0;
}

rock-paper-scissors.cpp:

#include "rock-paper-scissorss.h"
#include <cstdlib>

Computer :: Computer(){
    num_generated = rand() % 3 + 1;
}

void Computer ::  setMoveChoice(){
   if(num_generated == 1)
       move_selected = "rock";

   else if(num_generated == 2)
       move_selected = "paper";

   else
       move_selected = "scissors";

}

string Computer :: getMoveChoice(){
    return move_selected;
}

Player :: Player(string name){
    player_name = name;

}

void Player :: setMoveChoice(string choice){
    move_selected = choice;
}

string Player :: getMoveChoice(){
    return move_selected;
}

rock-paper-scissorss.h:

#ifndef ROCK_PAPER_SCISSORSS_H
#define ROCK_PAPER_SCISSORSS_H

#include <iostream>
#include <string>
using namespace std;

class Computer{

private:
    int num_generated;
    string move_selected;

public:
    Computer();
    void setMoveChoice();
    string getMoveChoice();

};

class Player{

private:
    string move_selected;
    string player_name;

public:
    Player(string);
    void setMoveChoice(string move);
    string getMoveChoice();

};

#endif /* ROCK_PAPER_SCISSORSS_H */

Welcome to codereview. Here's a selection of suggestions.

num_generated = rand() % 3 + 1;

This is probably fine for this use case. Be aware that

rand() is generally agreed not to be random enough for many uses where you need a random number. Consider instead using the <random> header.
Using %3 to condense the spread of random numbers into 0 through 2 slightly biases the result. Consider what happens if rand() uniformly and randomly produced numbers between 0 and 4 and you used this approach: getting low values would be slightly more likely than getting high values.
rand() requires seeding with srand() or the results won't vary from run to run.
The + 1 goes from a range of 0 through 2 to a range of 1 through 3. That's not inherently a problem (it doesn't cause any issues with this program) but the very strong convention is that counting starts from 0. If, say, you had an array of moves and you used this random number to look up the array, one of the moves should be move 0.

Putting that in the constructor, specifically

Computer :: Computer(){
    num_generated = rand() % 3 + 1;
}

means that a given Computer object only ever produced one move. It would probably be better if you could reuse the same object.

public:
    Computer();
    void setMoveChoice();
    string getMoveChoice();
};

Consider setting up some sort of inheritance structure. If there was a base class Player which HumanPlayer and ComputerPlayer inherited from, it would be easier to write the program flexibly. You could have the same game logic, and swap in different sorts of players for PvP games, CvC simulations, and potentially new additions such as playing against another player on the internet.

move_selected = "rock";

For a problem like this, it's better to use integer (or better still enum) types. Keep strings for when the task is inherently about the text (including text input and output) rather than the thing the text represents. Among other things, strings are expensive to work with and tend to confuse the compiler in places where it would otherwise be able to help point out bugs.

On the topic of strings, there is a lot of redundancy in whoWon. Consider separating out the logic of working out who won from the logic of displaying it. Every output has the form.

cout<< "You chose " << player_move << " and the computer chose "<< computer_move <<", " << result <<endl;

A c++ specific issue:

using namespace std;

It is generally agreed that this should be avoided, especially in header files. Better to write std::string and such like in full. Otherwise there can be ambiguity (including in files that at some level include that header, which the programmer is not fully aware of) between functionality defined by the standard library and functionality defined by the programmer or third party libraries.

endl

C++ programmers generally prefer putting a newline (\n) into the string rather than using endl: endl both pushes a newline and forces the output buffer to flush. Only use it when you need the buffer to flush and the text to display immediately.

while(play_again == 'y'){

If you want to use a loop that always runs once, and then checks whether to run again at the end, consider a do { ... } while () loop instead. This avoids having to force the repeat condition before starting.


An acceptable way of sorting List I had a problem where I needed to find the largest amount of passengers in multiple cars/vans etc, so I got the totals from each vehicle and added them to a list, I then created this method to sort the list...

static List<int> SortList(List<int> tobesorted)
{
    var j = 0;
    int JP = 0;

    int value1 = Convert.ToInt32(tobesorted[j]);
    int value2 = Convert.ToInt32(tobesorted[JP]);

    for (var i = 0; i < tobesorted.Count-1; i++)
    {
        for (j = 0; j < tobesorted.Count-1; j++)
        {
            JP = (j + 1);
            value1 = Convert.ToInt32(tobesorted[j]);
            value2 = Convert.ToInt32(tobesorted[JP]);

            if (value2 > value1)
            {
                tobesorted[j] = value2;
                tobesorted[JP] = value1;
            }
        }
        j = 0;
        JP = 0;
    }

    return tobesorted;
}

I don't understand is this is acceptable, and if not what methods are? It seems quick enough, I know of sorting algorithms, but haven't ever been forced to use any.

I needed to find the largest amount of passengers

If you need just the largest amount, you don't need to sort the whole list (which is slow*), you can instead walk though the list, remember the largest amount found so far and update that as you iterate the list.

LINQ already contains a method that does just that, called Max().

* Sorting is \$\mathcal{O}(n \log n)\$ with a good algorithm, or \$\mathcal{O}(n^2)\$ with a trivial algorithm like the one you used; finding maximum is just \$\mathcal{O}(n)\$.


Node router for marketplace routes After cleaning up & some refactoring, one module of my code became like below.
However, I'm feeling like there's still some unnecessary redundancy, and it can be improved further and look neater.

But I can't come up with exactly how to do it.

'use strict';

// === showing the execution context ==============

const express = require('express');
const router = express.Router();

const moment = require("moment");
const momentDurationSetup = require("moment-duration-format");
const DB = require('./DBOperator');
const _ = require('lodash');

const Promise = require("bluebird");
const request = require('superagent-bluebird-promise');

const utils = require('../utils');

// ========== main concern area ===============

function realTime(queryFunction, payload, req, res, next) {
    queryFunction((err, results) => {
        if (err) {
            console.log(err);
            next();
        } else {
            res.json({data: _.get(results[0].rows[0], payload, -1)});
        }
    });
}

router.get('/online-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinebuyerNumQuery, 'onlinebuyerNum', req, res, next);
});

router.get('/offline-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.offlinebuyerNumQuery, 'offlinebuyerNum', req, res, next);
});

router.get('/online-seller-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinesellerNumQuery, 'onlinesellerNum', req, res, next);
});

router.get('/offline-seller-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.offlinesellerNumQuery, 'offlinesellerNum', req, res, next);
});

/ ===========================

module.exports = router;

Partial application

One way to simplify the code is to eliminate the lambda/anonymous functions using Function.bind() to create Partially applied functions. For instance:

router.get('/online-buyer-count', utils.mcache(15), (req, res, next) => {
    realTime(DB.onlinebuyerNumQuery, 'onlinebuyerNum', req, res, next);
});

can be simplified to

router.get('/online-buyer-count', realTime.bind(null, DB.onlinebuyerNumQuery, 'onlinebuyerNum'));

And similarly for the other three routes, which reduces 12 lines to 4.

looping over the routes

The redundancy could also be simplified using Array.forEach()

['online', 'offline'].forEach(mode => {
    ['buyer', 'seller'].forEach(role => {
        router.get('/'+mode+'-'+role+'-count', realTime.bind(null, DB[mode+role+'NumQuery'], mode+role+'Num'));
    });
});

Taking that one step further using more partially-applied functions, the callback function of the inner forEach can be pulled out to a separate function:

const addRoute = (mode, role) => router.get('/'+mode+'-'+role+'-count', realTime.bind(null, DB[mode+role+'NumQuery'], mode+role+'Num'));

And then that function can be made into a partial and used within the nested forEach:

['online', 'offline'].forEach(mode => ['buyer', 'seller'].forEach(addRoute.bind(null, mode)));

So that can reduce those 4 lines to 2. It may be desirable to break those up into multiple lines for readability.


C++ Calculate euclidean distance between tuples, for specified indices I wrote a small utility function that will take two tuples and returns the Euclidean distance for the given indices only:

#include <cmath>
#include <tuple>

namespace {
    template <typename T> constexpr auto sq(const T& val) { return val * val; }
}

template <int... I, typename... Tp>
constexpr auto euclidean_distance(const std::tuple<Tp...>& ta, const std::tuple<Tp...>& tb) {
    return static_cast<std::common_type_t<std::tuple_element_t<I, std::decay_t<decltype(ta)>>...>>(
        sqrt(
                (
                    sq(
                        static_cast<std::common_type_t<std::tuple_element_t<I, std::decay_t<decltype(ta)>>...>>(std::get<I>(tb)) -
                        static_cast<std::common_type_t<std::tuple_element_t<I, std::decay_t<decltype(ta)>>...>>(std::get<I>(ta))
                    ) + ...
                )
            )
        );
}

constexpr std::tuple a { (int8_t)-10, -20, (uint16_t)50, "String", 1.2f };
constexpr std::tuple b { (int8_t)20, 120, (uint16_t)150, "Another", 9.1f };

int main() {
    volatile auto res_i = euclidean_distance<0,1,2>(a, b);
    volatile auto res_f = euclidean_distance<0,1,2,4>(a, b);
}

Right now it only takes tuples of completely equal type, but the template logic could be adapted to use tuples that only share similar types for the used indices.
I'd love to hear any comments and critiques!

Nice work! There's not much to improve here, but still:

Don't repeat yourself

You keep repeating the same type declaration over and over again. A simple type alias at the top of the function makes things so much cleaner:

using result_t = std::common_type_t<std::tuple_element_t<I, std::decay_t<decltype(ta)>>...>;

A little too trigger happy on the casts

I think this would be better without the final cast. std::sqrt() returns a double when passed integral types, and that's often desirable. Letting your users cast to an integer if they want after your function returns would be more flexible.

Prefer the namespaced version of C functions

std::sqrt is preferable over sqrt

Anonymous namespaces in headers mean almost nothing.

Your anonymous namespace may not generate a link-time symbol, but it still pollutes the global namespace for anything that includes it. sq() would probably be better off as a lambda inside of euclidean_distance() itself, or be in an actual named namespace.

Possible improvement:

It would be nice if this could be generalized to arbitrary norms. Passing functors that default to std::sqrt and sq would be really nice. Or perhaps a trait type that defines both the raising and the lowering operations in a single norm type might make sense.