Select multiple rows using multiple data attribute I am hiding rows for given ids using this code:

var idArray = ['A123','B456']; // getting from web services
for (var i=0;i<idArray.length;i++) {
   var id = idArray[i];
   $("#eTable tr[data-empid=" + id + "]").hide();
}

HTML (Just for example doesn't needs improvement)

        Peter
        Vogel
        Principal

        Jan
        Vogel
        General Manager

        Jason
        van de Velde
        Company nurse

How can we improve this code as it is selecting same table again and again to get row one by one instead of getting them once?

The performance benefits vary depending on the browser's implementation of ECMAScript, but you can usually get a modest performance improvement by selecting the parent element first, then selecting only child elements of that parent element. This jsPerf test somewhat demonstrates the difference.

In native JavaScript, this is done by selecting the parent element with document.querySelector, document.getElementById, or document.getElementsByTagName. You can then call a similar selector method on that element instead of on document to limit your search to only child elements.

Native

var idArray = ['A123','B456'];
var table = document.querySelector("#eTable");
for(var i = 0; i < idArray.length; i++){
    table.querySelector("tr[data-empid='"+idArray[i]+"']").style.display = "none";
}

In jQuery, you can achieve the same using jQuery(parentElement).find(selector) or $(parentElement).find(selector)

jQuery

var idArray = ['A123','B456'];
var table = $("#eTable");
for(var i = 0; i < idArray.length; i++){
   $(table).find("tr[data-empid='"+idArray[i]+"']").hide();
}


Number of divisors of a factorial I was solving the DIVFACT problem from Sphere Online Judge:

  Given a number, find the total number of divisors of the factorial of the number.
  Since the answer can be very large, print the answer modulo \$10^9+7\$.

  Input

  The first line contains \$T\$, the number of test cases

  \$T\$ lines follows each containing the number \$N\$.

  Constraints:

  \$ 1 \le T \le 500\$

  \$ 0 \le N \le 50000\$

To solve this I took the following approach: I first calculated the primes up to 50000 using a sieve, then for each prime \$p\$ computed the power of \$p\$ in \$N!\$ given by:

$$ \mathrm{power}(p) = \frac{n}{p} + \frac{n}{p^2} + \frac{n}{p^3} + \ldots $$

… until I got a \$p^\mathrm{power}\$ greater than \$N\$.

Finally I computed the answer for each prime \$p\$:

$$ (\mathrm{power}(p_1)+1) \cdot (\mathrm{power}(p_2)+1) \cdot \ldots \cdot (\mathrm{power}(p_k)+1)$$

assuming there are \$k\$ prime numbers less than 50000.

def prime_numbers(limit=50001):
    '''Prime number generator. Yields the series
    2, 3, 5, 7, 11, 13, 17, 19, 23, 29 ...
    using Sieve of Eratosthenes.
    '''
    yield 2
    sub_limit = int(limit**0.5)
    flags = [True, True] + [False] * (limit - 2)
    # Step through all the odd numbers
    for i in range(3, limit, 2):
        if flags[i]:
            continue
        yield i
        # Exclude further multiples of the current prime number
        if i <= sub_limit:
            for j in range(i*i, limit, i<<1):
                flags[j] = True

p=list(prime_numbers())
# print p

for i in range(int(raw_input())):
    n=int(raw_input())
    d=[]
    pr=1
    for num in p:
        if num<=n:
            exp=1
            s=0
            while n>=num**exp:
                s+=int(n/(num**exp))
                exp+=1
            pr*=(s+1)
            # d.append(s)
        else:
            break
    print pr%1000000007

but this code and algo is not fast enough to pass the test cases in the given time limit.Is there any way to improve this code to make it faster

Things should be fine if you perform strength reduction on n / num**exp. That not only makes things faster, it also gets rid of the messiness inherent in Python's exponentiation operator (so that you don't have to find out how it actually works under the hood).

In the kth iteration of the inner loop you have a quotient q_k that is equal to n / p**k (i.e. n / num**exp in your code).

To obtain n / p**(k+1) in the next iteration, all you have to do is q_k / p because that makes it (n / p**k) / p, which is equal to n / p**(k+1) as desired. In other words, instead of computing the power of the prime anew and then performing a division, simply divide the old quotient by the prime again.

Here's the complete loop, coded in a more pedestrian language:

static ushort[] primes_up_to_LIMIT = U16.SmallPrimesUpTo(50000).ToArray();

...

long divisor_count = 1;

foreach (int prime in primes)
{
    if (prime > n)
        break;

    int e = 0;

    for (int q = n; q >= prime; )
        e += q /= prime;

    divisor_count = (divisor_count * (e + 1)) % 1000000007;
}

return (int)divisor_count;

It isn't very fast - it clocked 0.12 s on SPOJ - but it should show the principle well enough and it can serve as the basis of a more performant solution.

Note: detailed investigation shows that for once, the lackluster performance of my C# submission on SPOJ is not due to usual problems inherent in .Net (e.g. poor input/output performance) but rather to some strange problem with Mono. On my laptop a worst-case input file clocks 13.7 ms under .Net and 136.4 ms under Mono.

Here is an attempt at rending the loop in Python (renaming some things for clarity):

divisor_count = 1

for current_prime in primes:

    if current_prime > n:
        break

    q = n
    e = 0
    while q >= current_prime:
        q /= current_prime
        e += q

    divisor_count *= e + 1

print divisor_count % 1000000007

One thing worth looking into is the underlying data type of the divisor count (pr in the original code). This can grow quickly, far beyond anything representable in a plain integer, and hence it will go big integer internally. This makes the multiplication pr *= s + 1 more expensive, and the statement has a cost multiplier of max(T) * pi(max(N)) = 500 * 5133.

Reducing the division count modulo 1000000007 after every update by changing

divisor_count *= e + 1

to

divisor_count = (divisor_count * (e + 1)) % 1000000007

improved the time for a worst-case input file from 1.92 s to 1.39 s on my box, so it's something worth keeping in mind. For comparison: the original code clocked 2.64 s for this file.

Belated addition: a few days after the fact an idea hit me out of the blue while I was having a smoke, and I'm reporting it here because it can speed up things by a factor of 20 for the SPOJ limit of 50000 (much more for higher inputs). This idea is based on the observation that half of the primes p satisfy

(n / 2) < p <= n

and hence

1 <= (n / p) < 2

which is just a coy way of saying

n / p = 1

Knowing that there are k primes that satisfy this condition (bisect_right() to the rescue) I can compute the contribution for all these primes in one fell swoop as

(1 + 1)**k

This readily extends to other small quotients in the remaining range of primes, and the fun only stops when the primes get so small that their squares appear as divisors (i.e. p <= sqrt(n)).

Here's how you explain this to your computer in the pythonic manner:

def v6b (n):
    """number of divisors of n! (SPOJ DIVFACT)"""
    divisor_count = 1
    hi = bisect.bisect_right(primes, n)
    for m in range(2, int(math.sqrt(n))):
        lo = bisect.bisect_right(primes, n / m, 0, hi)
        k  = hi - lo
        hi = lo
        divisor_count = (divisor_count * m**k) % 1000000007
        if k < 2:
            break
    # the rest is algorithm v5b but without regular modulo trimming since growth is contained
    for current_prime in primes[0:hi]:
        q = n
        e = 0
        while q >= current_prime:
            q /= current_prime
            e += q
        divisor_count *= e + 1
    return divisor_count % 1000000007

Here are the timings for the usual 500 worst-case inputs, first for the earlier version (here called 'v5b') and then for this new version. I went beyond the SPOJ limit of 50000 to give some idea of the growth behaviour.

# timing v5b ...
500 @    500:    0.016  // cksum 230711749546
500 @   1000:    0.016  // cksum 256088767231
500 @   5000:    0.078  // cksum 248659270107
500 @  10000:    0.156  // cksum 255839576411
500 @  50000:    1.281  // cksum 252317442043
500 @ 100000:    3.313  // cksum 246710058695
500 @ 500000:   44.376  // cksum 244700225412

# timing v6b ...
500 @    500:    0.000  // cksum 230711749546
500 @   1000:    0.015  // cksum 256088767231
500 @   5000:    0.016  // cksum 248659270107
500 @  10000:    0.031  // cksum 255839576411
500 @  50000:    0.063  // cksum 252317442043
500 @ 100000:    0.078  // cksum 246710058695
500 @ 500000:    0.266  // cksum 244700225412

Beyond 100000 or thereabouts the code gets faster with a hand-coded powering function (via repeated squaring) but the builtin exponentiation operator is faster for lower limits, and hence on SPOJ. In C++ and C# things were faster when bailing out of the first loop for k < 12 but in Python it worked best with k < 2.

Note: the time for 500 test cases at the SPOJ limit of 50000 is virtually the same as for the C# version running under Mono on my box, which is no mean feat for an interpreted language...


Performance of object-to-string conversion Using an OR/M I map a lot of rows from database to array of objects (~300k). In these objects some properties are marked with special attribute [Signed]. Row-by-row I combine them in string, calculate salted MD5 of string and compare to hash I received from database.

Everything was working pretty okay for a long time, but now my website started to work quite slow and I run a profiler. It says the slowest method of whole website is ValueToString. I optimized it as much as I can. Is it possible to optimize it more? I want to mention I do not need exact string representation so feel free to change formatters.

Another question if I will redesign everything from string to byte[] - will it speed up the process, what do you think?

static string ValueToString(object value)
{
    if (value == null)
        return "";

    string value_string = value as string;
    if (value_string != null)
        return value_string;

    int value_int;
    if (value is Enum) {
        value_int = (int)value;
        if (value_int >= 0 && value_int <= 255)
            return hexStringTable[((int)value)];
        if (value_int == -1)
            return "FFFFFFFF";
        // Actually, we are never here.
        return value_int.ToString("X");
    }

    decimal value_decimal;
    if (TryCast(value, out value_decimal))
        return value_decimal.ToString().TrimEnd('0');

    if (TryCast(value, out value_int))
        return value_int.ToString("X");

    DateTime value_DateTime;
    if (TryCast(value, out value_DateTime))
        return value_DateTime.Ticks.ToString("X");

    // We are never here for slow table.
    return Convert.ToString(value,
        CultureInfo.InvariantCulture);
}

static bool TryCast<T>(object o, out T r)
{
    if (o is T) {
        r = (T)o;
        return true;
    }

    r = default(T);
    return false;
}

static readonly string[] hexStringTable = new string[]
{
    "00", "01", "02", "03", "04", "05", "06", "07", "08", "09", "0A", "0B", "0C", "0D", "0E", "0F",
    "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "1A", "1B", "1C", "1D", "1E", "1F",
    "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "2A", "2B", "2C", "2D", "2E", "2F",
    "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "3A", "3B", "3C", "3D", "3E", "3F",
    "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "4A", "4B", "4C", "4D", "4E", "4F",
    "50", "51", "52", "53", "54", "55", "56", "57", "58", "59", "5A", "5B", "5C", "5D", "5E", "5F",
    "60", "61", "62", "63", "64", "65", "66", "67", "68", "69", "6A", "6B", "6C", "6D", "6E", "6F",
    "70", "71", "72", "73", "74", "75", "76", "77", "78", "79", "7A", "7B", "7C", "7D", "7E", "7F",
    "80", "81", "82", "83", "84", "85", "86", "87", "88", "89", "8A", "8B", "8C", "8D", "8E", "8F",
    "90", "91", "92", "93", "94", "95", "96", "97", "98", "99", "9A", "9B", "9C", "9D", "9E", "9F",
    "A0", "A1", "A2", "A3", "A4", "A5", "A6", "A7", "A8", "A9", "AA", "AB", "AC", "AD", "AE", "AF",
    "B0", "B1", "B2", "B3", "B4", "B5", "B6", "B7", "B8", "B9", "BA", "BB", "BC", "BD", "BE", "BF",
    "C0", "C1", "C2", "C3", "C4", "C5", "C6", "C7", "C8", "C9", "CA", "CB", "CC", "CD", "CE", "CF",
    "D0", "D1", "D2", "D3", "D4", "D5", "D6", "D7", "D8", "D9", "DA", "DB", "DC", "DD", "DE", "DF",
    "E0", "E1", "E2", "E3", "E4", "E5", "E6", "E7", "E8", "E9", "EA", "EB", "EC", "ED", "EE", "EF",
    "F0", "F1", "F2", "F3", "F4", "F5", "F6", "F7", "F8", "F9", "FA", "FB", "FC", "FD", "FE", "FF"
};

(I've made a few assumptions, so please correct me if I'm wrong.)

Let's suppose we have the following types:

public class Foo
{
    public int Id { get; set; }

    [Signed]
    public DateTime Timestamp { get; set; }

    [Signed]
    public string Name { get; set; }

    [Signed]
    public decimal Price { get; set; }

    [Signed]
    public SomeEnum Thing { get; set; }
}

public enum SomeEnum
{
    One = 1,
    Two,
    Three
}

public class SignedAttribute : Attribute
{
}

We want to hash the Signed properties of an instance of Foo (actually, many instances of Foo).

I assume you're currently getting these properties via reflection. Something like this:

var properties = typeof(Foo)
    .GetProperties()
    .Where(p => Attribute.IsDefined(p, typeof(SignedAttribute)))
    .OrderBy(p => p.Name)
    .ToArray();

// ...

var sb = new StringBuilder();
foreach (var property in properties)
{
    sb.Append(ValueToString(property.GetValue(foo)));
}

var bytes = Encoding.UTF8.GetBytes(sb.ToString());
md5.TransformBlock(bytes, 0, bytes.Length, bytes, 0);

Let's set up a test to see how long this takes

var foo = new Foo
{
    Id = 1,
    Name = "Thing",
    Price = 123.45m,
    Thing = SomeEnum.Two,
    Timestamp = new DateTime(2015, 04, 07)
};

byte[] hash;
var sw = Stopwatch.StartNew();
using (var md5 = MD5.Create())
{
    for (var i = 0; i < 300000; i++)
    {
        var sb = new StringBuilder();
        foreach (var property in properties)
        {
            sb.Append(ValueToString(property.GetValue(foo)));
        }

        var bytes = Encoding.UTF8.GetBytes(sb.ToString());
        md5.TransformBlock(bytes, 0, bytes.Length, bytes, 0);
    }

    md5.TransformFinalBlock(new byte[0], 0, 0);
    hash = md5.Hash;
}

sw.Stop();

Here ValueToString is the code as posted in your question. On my machine, I get a time of 0.8s.

How much better can we do? Suppose we could cheat and use the following method:

private static string FooToString(Foo foo)
{
    var sb = new StringBuilder();
    sb.Append(foo.Name);
    sb.Append(foo.Price.ToString(
        "0.############################",
        CultureInfo.InvariantCulture));
    sb.Append(((int) foo.Thing).ToString("X"));
    sb.Append(foo.Timestamp.Ticks.ToString("X"));
    return sb.ToString();
}

So now our loop looks like this:

for (var i = 0; i < 300000; i++)
{
    var value = FooToString(foo);
    var bytes = Encoding.UTF8.GetBytes(value);
    md5.TransformBlock(bytes, 0, bytes.Length, bytes, 0);
}

I get a time of 0.36s, which I'll take as the baseline.

Great, but let's assume we can't write FooToString. We can create an equivalent method dynamically.

We introduce some helper methods:

public static string ValueToString(string value)
{
    return value ?? string.Empty;
}

public static string ValueToString(Enum value)
{
    return Convert.ToInt32(value).ToString("X");
}

public static string ValueToString(decimal value)
{
    return value.ToString("0.############################", CultureInfo.InvariantCulture);
}

public static string ValueToString(DateTime value)
{
    return value.Ticks.ToString("X");
}

And then create the method dynamically, using reflection to find the properties of Foo that have the Signed attribute.

var properties = (typeof(Foo))
    .GetProperties()
    .Where(p => Attribute.IsDefined(p, typeof(SignedAttribute)))
    .OrderBy(p => p.Name)
    .ToArray();

var getSignatureMethod = new DynamicMethod(
    "GetSignature",
    typeof(string),
    new[] { typeof(Foo) },
    typeof(Foo).Module);

var generator = getSignatureMethod.GetILGenerator();
generator.DeclareLocal(typeof(StringBuilder));
generator.Emit(OpCodes.Newobj, typeof(StringBuilder).GetConstructor(Type.EmptyTypes));
generator.Emit(OpCodes.Stloc_0);
generator.Emit(OpCodes.Ldloc_0);

var append = typeof(StringBuilder).GetMethod("Append", new[] { typeof (string) });
foreach (var property in properties)
{
    generator.Emit(OpCodes.Ldarg_0);
    generator.Emit(OpCodes.Callvirt, property.GetGetMethod());
    if (property.PropertyType.BaseType == typeof(Enum))
    {
        generator.Emit(OpCodes.Box, property.PropertyType);
    }

    generator.Emit(OpCodes.Call, typeof(Program).GetMethod("ValueToString", new[] { property.PropertyType }));
    generator.Emit(OpCodes.Callvirt, append);
}

generator.Emit(OpCodes.Pop);
generator.Emit(OpCodes.Ldloc_0);
generator.Emit(OpCodes.Callvirt, typeof(object).GetMethod("ToString", new Type[] { }));
generator.Emit(OpCodes.Ret);

var getSignature = (Func<Foo, string>)
    getSignatureMethod.CreateDelegate(typeof(Func<Foo, string>));

(I haven't worked with dynamically generated code before, there might be some things that could be improved.)

Now our loop looks like this:

for (var i = 0; i < 300000; i++)
{
    var value = getSignature(foo);
    var bytes = Encoding.UTF8.GetBytes(value);
    md5.TransformBlock(bytes, 0, bytes.Length, bytes, 0);
}

And on my machine, it takes 0.39s, which is pretty close to the baseline.

I've put the code up on Github if you want to play around with it.


Reverse words in a string without affecting space I came across a problem that needs to reverse each word in a string without affecting space in it. For example, if a string has two spaces between the word, the answer needs to have space between two words. These are example test cases, I have been given
/This is a sentence/
/ This  is a Sentence /
My approach is:

string oreverse(string s){
    for(int i=0,n=s.size();i<n/2;++i)
        swap(s[i],s[n-i-1]);
    return s;
}
string reverseWords(string s) {
    string re;
    for(auto i=0;i<s.size();){
        if(s[i]!=' '||s[i]=='/0'){
            int _t = s.find_first_of(' ',i);
            re+=oreverse(s.substr(i,_t-i));
            i=_t;
        }else{
            re+=s[i];
            i++;
        }
    }
    return re;
}

I like to know, is there any further optimisation can be done or any other approaches with increased efficiency.

Use compiler warnings

You should always enable compilers warning in c++. For your code, clang & g++ gives the following warnings (live demo):

prog.cc:14:29: warning: multi-character character constant [-Wmultichar]
        if(s[i]!=' '||s[i]=='/0'){
                            ^
prog.cc:13:19: warning: comparison of integers of different signs: 'int' and '...' (aka 'unsigned long') [-Wsign-compare]
    for(auto i=0;i<s.size();){
                 ~^~~~~~~~~
prog.cc:14:27: warning: result of comparison of constant 12080 with expression of type '...' (aka 'char') is always false [-Wtautological-constant-out-of-range-compare]
        if(s[i]!=' '||s[i]=='/0'){
                      ~~~~^ ~~~~

The ones about s[i]=='/0' are especially concerning. You probably meant s[i]=='\0'. I don't know if you want to handle other possible separators of a string (like tabs, line feeds). If it's the case, you should have a look at std::isspace or std::isblank.

The one about the for suggests that the type int (deduced for auto) might not be the most suited as a string/vector index. According to this post, the most portable index type to use would be std::string::size_type.

Avoid std::string copy

Your code is performing a lot of string construction / copy / concatenation. They all can cause memory allocation/deallocation, which can be quite costly compared to the rest of your code. You should be able to avoid most of them.

Output: generate the result directly in a single std::string

You know exactly the size of the output string re (same size as input). You can preallocate it with std::string::reserve.
Pass a reference to re to void oreverse(std::string& output,/*InputType*/ input), and directly push_back the characters of the input in reverse order.

Input: work directly on the input string

pass your input string by (const) references in both functions: const std::string&.
avoid std::string::substr:

C++17: you can use std::string_view instead of std::string. It's substr method won't do any copy.
Pre-C++17: pass iterators to the input string to oreverse. Or a (const) reference to the input string, with start/end indices.

Use "static" for global functions when applicable

Declaring a global function as "static" indicates to your compiler that your function is used only in the current translation unit (current *.cpp file). The compiler can use this information to do better inlining decisions (see for example gcc/g++: -finline-functions-called-once option).


Print factors of number using fencepost loop I was wondering if this code could be optimized in any way? For instance, could I use a for each loop here? Are there other ways I could write this which would make it easier for me, or the person reading it to understand?

public static void printFactors(int factorsOf) {
    for (int i = 1; i < factorsOf; i++) {
        if(factorsOf % i == 0) {
            System.out.print(i + " and ");
        }
    }
    System.out.print(factorsOf);
}

Just a tiny note:

  In number theory, integer factorization is the decomposition of a composite number into a product of smaller integers.

So a factorization of 12 may be

6 * 2

or

3 * 2 * 2

but not

1 and 2 and 3 and 4 and 6 and 12

What you did is finding all divisors and you should name your method appropriately.

As always, a method both computing and printing something is rather unusable. You should always compute the result in one method, so that you can later either print it or process it further.

For simple methods like yours it doesn't matter much, but it's good habit.


Length-Based Frame Stream En-/Decoding in Go I'm relatively new to Go, which is why I decided to post some code here. My goal is to en-/decode a TCP stream. The individual data packets length-based. The first two bytes are the unsigned size in little-endian byte order. The size includes the size field itself. The methods I wrote seem to en-/decode the packets as expected.

Is there a more efficient way to achieve this?
Does Go perhaps already implement such methods somewhere.

func GetPacketByLength(source io.Reader) ([]byte, error) {
    s := make([]byte, 2)
    if _, err := io.ReadFull(source, s); err != nil {
        return nil, err
    }

    // Little-endian uint16
    size := uint16(s[0]) | uint16(s[1])<<8 - 2

    p := make([]byte, size)
    if _, err := io.ReadFull(source, p); err != nil {
        return nil, err
    }

    return p, nil
}

func PutPacketByLength(target io.Writer, p []byte) error {
    s := make([]byte, 2)

    // Little-endian uint16
    s[0] = byte((len(p) + 2))
    s[1] = byte((len(p) + 2) >> 8)

    if _, err := target.Write(s); err != nil {
        return err
    }
    if _, err := target.Write(p); err != nil {
        return err
    }

    return nil
}

You may consider to use binary.Read. I find this more readable and self documented. See this example.

var len uint16
err := binary.Read(buf, binary.LittleEndian, &len)

The writing part:

len := uint16(len(p))
err := binary.Write(buf, binary.LittleEndian, len)

Also don't forget to check that len(p) doesn't overflow uint16 (less than 65535).

Apart from that your code is fine.


HackerRank Sherlock and GCD challenge I've written an implementation for the Sherlock GCD HackerRank Challenge. As the running time of my algorithm is something like \$O(kn^2)\$, I reckon that there's a more efficient algorithm that could be used instead. Are there any improvements that can be made to my implementation to reduce the running time?

Problem statement (as shown on HackerRank)

  Sherlock is stuck while solving a problem: Given an array \$A={a1,a2,⋯,aN}\$, he wants to know if there exists a subset \$B\$ of this array which follows these statements:

  \$B\$ is a non-empty subset.
  There exists no integer \$x(x>1)\$ which divides all elements of \$B\$.
  There are no elements of \$B\$ which are equal to another.

  Print YES if such a subset exists; otherwise, print NO

Answer:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

public class SherlockGCD {
    public static void main(String[] args) {
        try(BufferedReader input = new BufferedReader(new InputStreamReader(System.in))){
            int numTestCases = Integer.parseInt(input.readLine());

            for(int i = 0; i < numTestCases; i++){
                int N = Integer.parseInt(input.readLine());
                String[] array = input.readLine().split(" ");
                Set<Integer> distinctElements = new HashSet<>(N);
                for(int j = 0; j < N; j++){
                    distinctElements.add(Integer.valueOf(array[j]));
                }
                List<Integer> numbers = new ArrayList<>(distinctElements);
                System.out.println(doesSubsetExist(distinctElements, numbers));
            }
        }catch(IOException e){
            e.printStackTrace();
        }
    }

    /**
     * Determines whether a subset that satisfies the constraints in the problem statement exists.
     *
     * Looks for two elements that satisfy the conditions set out in the problem statement. If two such
     * elements exist, then there exists at least one subset that satisfies the conditions. Else, there doesn't exist a
     * subset.
     *
     * @param set        A set of distinct numbers. Passed in because .contains() is O(1) for a Set vs O(n) for an
     *                   ArrayList
     * @param numbers    A list of distinct numbers
     * @return "YES" if such a subset exists. "NO" if not.
     */
    private static String doesSubsetExist(Set<Integer> set, List<Integer> numbers){
        if(set.contains(1)){
            return "YES";
        }

        // If there's only one distinct element and that element isn't the integer 1, then the second constraint is not
        // satisfied.
        if(numbers.size() == 1){
            return "NO";
        }

        for(int i = 0; i < numbers.size(); i++){
            for(int j = i + 1; j < numbers.size(); j++){
                for(int k = 2; k <= Math.min(numbers.get(i), numbers.get(j)); k++){
                    if(numbers.get(i) % k == 0 && numbers.get(j) % k == 0){
                        break;
                    }

                    if(k == Math.min(numbers.get(i), numbers.get(j))){
                        return "YES";
                    }
                }
            }
        }
        return "NO";
    }
}

There is a simple efficient solution for this problem which uses the following observation: if \$ A ⊆ B \$, then \$ \gcd(A) ≥ \gcd(B) \$. Proof: if \$ g = \gcd(B) \$ then \$ g \$ divides all elements of \$ B \$. But all elements from \$ A \$ are in \$ B \$, thus \$ g \$ divides all elements in \$ A \$. Hence, \$ \gcd(A) ≥ g \$.

This means that we can simply check that the GCD of all elements of the given array is \$ 1 \$.

What about duplicates? They don't matter because they do not affect the value of the greatest common divisor.

How to compute the GCD of an array? We can use the following property: $$ \gcd(a_0, a_1, a_2, \dotsc, a_{n - 1}) = \gcd(\gcd(a_0, a_1), a_2, \dotsc, a_{n - 1}). $$ To compute the GCD of two numbers one can use Euclid's algorithm.

The time complexity is \$ O(n + \log \max A) \$, where \$ \max A \$ is the maximum element of the array \$A\$.


Insert JSON data into rendered template after AJAX call I have an already rendered template and I want to update it with some JSON data I obtain via AJAX.

The template as it is before rendering looks like this (using jinja2 template engine):

     Time
     Destination
     Bus number

{% for bus in buses %}

     bus[0]
     bus[1]
     bus[2]

{% endfor %}

And I want to send AJAX requests to update the buses var with new values. The way I do it is deleting the tbody content when new data arrives, and rendering everything again with the new data.

function executeQuery() {
    $.ajax({
        method: 'get',
        url: '/data/json',
        data: { bus_id : {{ bus_id }} },
        success: function(data) {
            data = JSON.parse(data);
            $('#table-body').html('');
            for (i in data){
                $('#table-body').append(
                    "
" +
                    "" + data[i][0] + "	" +
                    "" + data[i][1] + "	" +
                    "" + data[i][2] + "	" +
                    "
");
                };
        }
      });

How can I improve this code? (FWIW I'm using Flask)

If you're using a jQuery collection more than once, make sure to set a variable, this means you don't have to do unnecessary DOM lookups:

 var $table = $('#table-body');

You can probably improve your append code to make it more legible. Personally I prefer utilising the jQuery methods where possible, such as .text().

 data = JSON.parse(data);
 $table.empty(); // empty is more explicit
 // iterate over the data
 data.forEach(function (buses) {
   // create a new table row and append it
   var $row = $('<tr>')
     .appendTo($table);
   // iterate over the buses
   buses.forEach(function (bus) {
     // create the cell, set the text,
     // and append it to the row
     $('<td>')
       .text(bus)
       .appendTo($row);
   });

 });

Since you're just doing a simple get, you can use jQuery shorthand $.get function:

$.get('/data/json', { bus_id : {{ bus_id }} })
  .success(function (data) {
    // do render
  });


Simple random number generator I have been wanting to learn programming for some time now and decided to really give it a shot today. I have edited and read lots of things in Java to fix small issues but never written from scratch. I've tried C and Java but both seemed just a bit too much to teach myself as a first language, so I'm going with Python first.

Can anyone look through this little sample random number generator I made and point out anything I should be doing differently or correctly? It is executing without issues for me but I'm sure I've got extra nonsense in here. I just have all the prints to see a result of each step and I know this is not what a real random number generator would be like.  I'm just testing and learning my way around different aspects of the language.

import random

x = int(raw_input("Please enter a number up to 100. "))
z = random.randint(40,100)
""" x and z can both be used below without errors -- testing/learning"""

randomNumber = random.randint(1,z)
print "first random",randomNumber

if 35 >= randomNumber:
    newRandom = randomNumber * .5
    newRandom = newRandom + 9
    print "second random " "%.f" % newRandom

elif 36 <= randomNumber <= 93:
    newRandom = randomNumber + 7
    print "second random " "%.f" % newRandom

else:
    newRandom = randomNumber
    print "second random",newRandom

Code organization

Rather than having your code in the global namespace, it's better to wrap it into a method, for example:

def gen_random_number():
    randomNumber = random.randint(1, z)
    print "first random", randomNumber
    # ...

And call the method like this:

if __name__ == '__main__':
    gen_random_number()

Magic numbers

Avoid using "magic numbers": the numbers 40, 100, 35, 93 are "magic" because it's unclear what they are and why. If you put them in capitalized constants near the top of the file with a good name, it becomes much clearer what they are and their purpose, for example:

LOWER_BOUND = 40
UPPER_BOUND = 100

z = random.randint(LOWER_BOUND, UPPER_BOUND)

For 40 and 100 it was easy enough to guess what they are and give these names. But for 35, 93 I'm not sure. If you had given them a name, I wouldn't have to wonder.

Formatting

There is an official standard Python style guide called PEP8. I suggest to read and follow that, it will make it easier for others to review your code. There is command line utility called pep8 that can detect coding style violations. You can install it with pip install pep8, and run it on your script (or even an entire directory) simply with pep8 path/to/your/code.

Your code reformatted and variables renamed to follow PEP8 would look like this:

random_number = random.randint(1, z)
print "first random", random_number

if 35 >= random_number:
    new_random = random_number * .5
    new_random += 9
    print "second random " "%.f" % new_random

elif 36 <= random_number <= 93:
    new_random = random_number + 7
    print "second random " "%.f" % new_random

else:
    new_random = random_number
    print "second random", new_random

Although PEP8 doesn't complain about this:

print "second random " "%.f" % new_random

There's no need to put a space between those two strings, so you can write it simply:

print "second random %.f" % new_random

You could further simplify this:

new_random = random_number * .5
new_random += 9

to this:

new_random = random_number * .5 + 9


Pointer class/handle Please review my pointer class.

template<typename T>
class Ptr {
public:
    Ptr(T* t, int s = 1) {
        sz = s;
        p = new T[sz];
        for (int i = 0; i < sz; ++i) {
            p[i] = t[i];
        }
    }

    Ptr(const Ptr&) = delete;
    Ptr& operator=(const Ptr&) = delete;

    Ptr(Ptr &&t) :p{t.p}, sz{t.sz} {
        t.p = nullptr;
        t.sz = 0;
    }

    Ptr& operator=(Ptr &&t) {
        std::swap(t.p,p);
        std::swap(t.sz,sz);
        return *this;
    }

    T& operator*() {
        check_range();
        return *p;
    }

    T& operator[](int i) {
        check_range(i);
        return p[i];
    }

    Ptr& operator+=(int i) {
        check_range(index+i);
        index += i;
        p+= i;
        return *this;
    }

    Ptr& operator-=(int i) {
        check_range(index-i);
        index -= i;
        p -= i;
        return *this;
    }

    Ptr& operator+(int i) {
        Ptr old{*this};
        return old+=1;
    }

    Ptr& operator-(int i) {
        Ptr old{*this};
        return old-=1;
    }

    Ptr& operator++() {
        return operator+=(1);
    }

    Ptr operator++(int) {
        Ptr<T> old{p};
        operator++();
        return old;
    }

    Ptr& operator--() {
        return operator-=(1);
    }

    Ptr operator--(int) {
        Ptr<T> old{p};
        operator--();
        return old;
    }

    ~Ptr() {
        while (index < sz-1) {
            operator++();
        }
        while (index != 0) {
            delete p;
            operator--();
        }
        delete p;
    }

private:
    T* p;
    int sz;
    int index = 0;
    void check_range(int i) {
        if (i < 0 || i > sz-1) {
            throw std::out_of_range("out of range");
        }
    }
    void check_range() {
        if (p == nullptr) {
            throw std::out_of_range("null pointer");
        }
    }
};

Only need to point out one thing.

Constructor:

    p = new T[sz];

Destructor:

    delete p;

This code is broken. If you allocation with new [] you MUST destroy with delete [].

In the operator+= and operator-=

Ptr& operator+=(int i) {
    check_range(index+i);
    index += i;
    p+= i;
    return *this;
}

Ptr& operator-=(int i) {
    check_range(index-i);
    index -= i;
    p -= i;
    return *this;
}

Thus moving p during the lifetime of the object is not a good idea (because you must call delete on the pointer returned by new). So if you are going to move p you need to keep track of the original pointer in another member. But I would not move it all. You are adjusting index so why adjust p?

These can't return by reference:

Ptr& operator+(int i) {
    Ptr old{*this};
    return old+=1;
}

Ptr& operator-(int i) {
    Ptr old{*this};
    return old-=1;
}

You have to return by value because you are creating new values.

I am surprised that this works:

    Ptr old{*this};

Because you have deleted the copy constructor.

Ptr(const Ptr&) = delete;

If it compiles I am not sure what is happening. But it is definitely not good.

In the constructor you should prefer initializer list.

Ptr(T* t, int s = 1) {
    sz = s;              // Initializer list
    p = new T[sz];       // Initializer list.

    // This seems inefficient.
    // Since the line above has just called the constructor on each element
    // element in the array. You are now calling the assignment operator
    // on each element in the array.
    for (int i = 0; i < sz; ++i) {
        p[i] = t[i];
    }
}

You can optimize the above

Ptr(T* t, int s = 1)
    : sz(s)
    , p(reinterpret_cast<T*>(new char[s * sizeof(T)])  // Allocate aligned but uninitialized memory.
{
    for (int i = 0; i < sz; ++i) {
        new (p + i) T(t[i]);       // Its called placement new.
                                   // The `(p + i)` is the address where you want the
                                   // construction to take place (ie don't allocate)
                                   // Then we use the copy constructor to create the
                                   // copy
    }
}


Writing a chunk of a file to a socket in Swift I'm writing a chunk of a file from an offset to a socket.

This is my implementation and it's working OK from my tests.

Any comments or speed improvements?

private func sendChunk(_ source: Int32, _ target: Int32, _ offset: off_t, _ count: UInt64) -> Int64 {
    let bufferSize:Int = 1024
    var buffer = [UInt8](repeating: 0, count: bufferSize)
    var writed:UInt64 = 0
    var read:Int = 0
    while true {
        let remaining = Int(count - writed)
        if remaining < bufferSize {
            read = pread(source, &buffer, remaining, offset + Int64(writed))
        } else {
            read = pread(source, &buffer, buffer.count, offset + Int64(writed))
        }
        guard read > 0 else {return Int64(read)}
        var writeCounter = 0
        while writeCounter < read, writed < count {
        let writeResult = write(target, &buffer + writeCounter, read - writeCounter)
            guard writeResult > 0 else {return Int64(writeResult)}
            writeCounter = writeCounter + writeResult
            writed += UInt64(writeCounter)
        }
    }
}

The type annotations in

let bufferSize:Int = 1024
var read:Int = 0

are not necessary, this can be simplified to

let bufferSize = 1024
var read = 0

The variable names can be improved:
read is not ideal since there is a system call
read() with the same name. The past participle of "to write" is
"written", not "writed", a better variable name might be totalBytesWritten since it contains the accumulated number of
bytes written to the target file.

The conversion to Int in

    let remaining = Int(count - writed)

can overflow on a 32-bit architecture. If the number of bytes to
read is computed as

    let readAmount = Int(min(count - totalBytesWritten, Int64(bufferSize)))

instead then it can not overflow and only one call to pread()
is needed.

There is a bug in

        writed += UInt64(writeCounter)

which should be

        writed += UInt64(writeResult)

otherwise the number of bytes written is added multiple times to
the total counter if multiple write() calls were necessary
(and then count - writed can crash).

With the outer loop

while true { ... }

pread() is called once more if all data has been copied. That can be
avoided by changing the outer loop to

while totalBytesWritten < count { ... }

Then also the logic becomes clearer and the condition in

    while writeCounter < read, writed < count { ... }

can be simplified.

Your function can only return 0 (on success) or -1 (on error),
therefore the return type Int64 makes no sense.

One possibility would be to change the return type to Bool to indicate
success or failure. Another possibility is to return the total number
of bytes copied in the success case (which can be less than the count
parameter if end-of-file was encountered), or nil on failure.
Yet another possibility is to throw an error, that allows to report
details on the problem to the caller.

I would not use empty external parameter names (_), a call

sendChunk(source: sfd, target: dfd, offset: 2, count: 5)

is more descriptive than

sendChunk(sfd, dfd, 2, 5)

The offset parameter could be defined with an default value zero.

Putting it all together the function could look like this:

func sendChunk(source: Int32, target: Int32, offset: off_t = 0, count: UInt64) -> UInt64? {

    let bufferSize = 1024
    var buffer = [UInt8](repeating: 0, count: bufferSize)

    var totalBytesWritten: UInt64 = 0
    while totalBytesWritten < count {
        let readAmount = Int(min(count - totalBytesWritten, UInt64(bufferSize)))
        let bytesRead = pread(source, &buffer, readAmount, offset + off_t(totalBytesWritten))
        guard bytesRead > 0 else {
            return bytesRead == 0 ? totalBytesWritten : nil
        }

        var bytesWritten = 0
        while bytesWritten < bytesRead {
            let amount = write(target, &buffer + bytesWritten, bytesRead - bytesWritten)
            guard amount > 0 else {
                return nil
            }
            bytesWritten += amount
        }
        totalBytesWritten += UInt64(bytesWritten)
    }

    return totalBytesWritten
}

Possible speed improvements: Most time will be spent in the read/write
system calls. One thing that you can do is to increase the
buffer size.


Recurring tasks in ASP.NET MVC I have 2 alternatives for executing recurring tasks in ASP.NET MVC.

We just need to add some code in Global.asax

First alternative:

private static void ThreadSendNextMail()
{
  while (true)
  {
    HostingEnvironment.QueueBackgroundWorkItem(async cancellationToken =>
    {
      using (var db = new EFDbContext())
      {
        try
        {
          //demo test for periodically tasks
          int paymentCount = await db.Payments.AsNoTracking().CountAsync();
          logger.Info(paymentCount);
        }
        catch (Exception ex)
        {
          logger.Error(ex.ToString());
        }
      }
    });
    Thread.Sleep(20000);
  }
}

private Thread mailThread { get; set; }

protected void Application_Start()
{
  //other code removed for clarity
  mailThread = new Thread(new ThreadStart(ThreadSendNextMail));
  mailThread.Start();
}

Second alternative:

private void AddTask(Action<string, object, CacheItemRemovedReason> target, int seconds)
{
  HttpRuntime.Cache.Insert(target.Method.Name + DateTime.Now.ToString(), seconds, null, DateTime.Now.AddSeconds(seconds), Cache.NoSlidingExpiration, CacheItemPriority.NotRemovable, new CacheItemRemovedCallback(target));
}

public void SendNextMail(string key, object seconds, CacheItemRemovedReason reason)
{
  HostingEnvironment.QueueBackgroundWorkItem(async cancellationToken =>
  {
    using (var db = new EFDbContext())
    {
      try
      {
        //demo test for periodically tasks
        int paymentCount = await db.Payments.AsNoTracking().CountAsync();
        logger.Info(paymentCount);
      }
      catch (Exception ex)
      {
        logger.Error(ex.ToString());
      }
    }
  });

  AddTask(SendNextMail, Convert.ToInt32(seconds));
}

protected void Application_Start()
{
  //other code removed for clarity

  //Periodically tasks
  AddTask(SendNextMail, 20);
}

I use .NET 4.5.2 and MVC 5.2. For logging I use NLog.

Both methods work perfectly (You can check this by watch log time in log file). Which is more preferable and better for performance, if I have many recurring tasks in the same time?

Do you see any improvement / issue?

As already mentioned by Anders, the second approach is better, however there are several problems when implementing async Tasks that run within an IIS worker process:

1) unhandled exceptions in the async threads/tasks kill the entire process. This should not be a problem in your code, since you catch all exceptions and normally, NLog handle does not have unhandled exception.

2) worker process may suddenly stop for various reasons such as web.config, binaries, idle recycling and the task might be stopped in the middle of its work. This should not happen in your case, since the operation seems pretty short and ASP.NET will try to delay the shutdown of the process to allow background workers to finish their jobs.

I also remark the possible optimization brought by using an Async function along with await on the same instruction.

A small improvement is to use UnitOfWork pattern for your database context, but the benefit comes when more complex things happen in your worker:

/// <summary>
/// unit of work store
/// </summary>
public static class UnitOfWorkStore
{
    public static object GetData(string key)
    {
        if (HttpContext.Current != null)
            return HttpContext.Current.Items[key];
        return CallContext.GetData(key);
    }

    public static void SetData(string key, object data)
    {
        if (HttpContext.Current != null)
            HttpContext.Current.Items[key] = data;
        else
            CallContext.SetData(key, data);
    }
}

public partial class EFDbContext
{
    #region Members
    private static readonly object objSync = new object();
    private static readonly string DATACONTEXT_KEY = "EFDbContext_UserInstance";
    #endregion

    /// <summary>
    /// Uses a UnitOfWorkStore to return the same instance of Context, that is unique
    /// per user, per postback (or CallContext, if not in an HttpContext environment, such as unit tests or console apps)
    /// </summary>
    public static EFDbContext Instance
    {
        get
        {
            // Dirty (non thread-safe) check
            if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
            {
                lock (objSync)
                {
                    // Thread-safe check
                    if (UnitOfWorkStore.GetData(DATACONTEXT_KEY) == null)
                    {
                        var context = new EFDbContext();
                        UnitOfWorkStore.SetData(DATACONTEXT_KEY, context);
                    }
                }
            }
            return (EFDbContext)UnitOfWorkStore.GetData(DATACONTEXT_KEY);
        }
    }
}

So, that generating a new context can be done using EFDbContext.Instance and unit of work pattern will take care to return the same instance if called in the same "context". Also, it looks like database context disposal is not really necessary.

[later edit]

Just remembered a couple of things:

3) AsNoTracking() is a good option when fetching read-only data. It is faster and allocates less memory, so its usage is welcomed (however, I rarely see it used).

4) Idle timeout - by default, IIS will stop the application pool if it has not activity. This means that no processing will be made during some periods (e.g. during week-ends)


Mapping file data to environment variables This is a follow-on to my previous question: Enforcing set environment variables

While learning more about JavaScript, node, and the bluemix environment, I have been using the loading of process environment variables as a starting point for understanding how the systems work.

This part of the system checks to see if a 'default' value exists for an environment variable. If there is a default, and the variable is not set, then the variable is set to the default's value. Default values are stored in a file in a pre-defined directory (the same folder as the script file).

After taking in the review suggestions, and some other suggestions from colleagues, I have changed the code mechanism to use a map/filter/process system on the files. This makes the code quite a bit neater.

Note: this code is a file/module in a node.js application deployed in a Bluemix host.

Again I am looking for any further insights in to how the code can be improved, and if any edge cases exist. Performance is not critical, but as I am just learning JavaScript I would appreciate any insights in to any bad practices that should be avoided so that issues are avoided later on.

In particular, I am concerned that the code is not as asynchronous as it could be, though it is a requirement that all the variables are set before the code terminates (nothing asynchronous can be incomplete).

/*jshint node:true*/

/*
 * Load various pre-determined environment variables
 * (files in this folder with .env extension).
 * Only if they have not previously been set in the environment.
 *
 * This makes the setting of Bluemix style variables quite easy.
 */

var fs = require('fs');
var path = require('path');

var env = /\.env$/;

function isEnv(fileName) {
    return fileName.match(env);
}

function processEnv(fileName) {
    var key = fileName.replace(env, "");
    if (process.env.hasOwnProperty(key)) {
        return;
    }
    var filePath = path.join(__dirname, fileName);
    var data = fs.readFileSync(filePath, 'utf8');
    process.env[key] = data;
}

fs.readdirSync(__dirname)
    .filter(isEnv)
    .forEach(processEnv);

This code is very clean and is definitely a clear improvement from your last version.

Your code is easy to understand and follow, now that's it's more broken up (aside from one big block of code).

I recommend storing your regexular expression for locating the extension .env like this:

var env = new RegExp("\.env$/", "");

The reason is for this: RegExp.test.

The test method of a RegExp object does the following:

Returns true if the regexular expression matches the input string.
Returns false if the regexular expression does not match the input string.

Now, your isEnv function does this:

return env.test(fileName);

Why is this better? Well...

The test method of RegExp only has to worry about the regexular expression matching at all.
The match method of a string has to worry about the regexular expression matching AND it has to worry about creating and returning an array of all the matches.

That being said, the test method is obviously faster than using match.


Concatenate strings (originally binary) in 8-character chunks, 2 per row Here is the code I would like reviewed:

int lines = 4; //Example
string input = "81111111822222228333333384444444" //Example

//Use a StringBuilder to make it easier when producing the formatted code
var stringBuilder = new StringBuilder();

//Loop through the ASM Hex code and extract every 8 character and put them in 2 by 2 rows
//Also add a NOP as needed depending if it's Odd or Even
for (int i = 0; i < lines; i++)
{
    string tempString = input.Substring(i * 8, 8);

    if (i % 2 == 1)
        stringBuilder.AppendLine(tempString);
    else
        stringBuilder.Append(tempString + " ");
}

//End Code with "000000" or "60000000 00000000" if empty line, needed for Gecko
if (lines % 2 == 1)
    stringBuilder.Append("00000000").AppendLine();
else
    stringBuilder.Append("60000000 00000000").AppendLine(); //This would be True in this Example (4 = Equal number)

    //Return the result
    return stringBuilder.ToString();

//stringBuilder.ToString() =
//           "81111111 82222222
//            83333333 84444444
//            60000000 00000000"

What I am doing is basically what the comments says. What might be worth knowing is that originally the string that I want to format is read from a file as hex.

//"hex" is "input" in the first part.
var ba = File.ReadAllBytes($"temp{i}.o");

string hex = BitConverter.ToString(ba).Replace("-", "");
hex = hex.Substring(104, lines * 8); //Remove Garbage data and read only what we want (8 in length * rows of ASM)

I'm not sure if there is a more efficient way of handling the data more directly via that.

I don't think my approach now is bad, but it feels a bit "overkill". It feels like I could probably just read the data I want directly and split up the bytes or something. Sadly, I don't have the skills to know if that is something one should be doing in this case.

lines is an integer which contains the rows from a text. It's simply used for the formatting as one line = 8 characters of the wanted text (hex).

An example of hex text would be something like:

81111111822222228333333384444444855555558666666687777777

It's simplified so it's easier to tell (each Code starts with 8 in this example).

What I want it to turn into in this case would be:

81111111 82222222
83333333 84444444
85555555 86666666
87777777 00000000

And if (8xxxxxxx) actually took up both 2 lines in the row, I would add:

60000000 00000000

The original code is of no interested as it's completely different (Assembly). The only useful part is the rows of code because one row of Assembly = 8 hex characters.

You need to separate splitting the string into bytes from creating lines.

By following the Single Responsibility Principle it's much easier to optmize your code because you can focus on one unit at a time. Other benefits are: easier maintenance and easier testing.

(I call it just byte because it's 8 elements long)

Ok, so here you'll need two methods. The first one just creates a list of strings and can be greatly simplified because we can add each substring directly to the list.

public static IEnumerable<string> Bytify(this string value)
{
    var byteLength = 8;
    for (var i = 0; i < value.Length; i += byteLength)
    {
        yield return value.Substring(i, byteLength);
    }
}

There are many other methods for splitting the string into batches but that's not the point. It's about the SRP. If you have separate methods you can exchange the algorithm for another one without breaking or adjusting everything else. The method creating the lines will still work because it doesn't need to know how you create the list, it just requires one.

Done. Now you can work with the bytes.

The other method can build the lines. With some LINQ it's very easy to create each one. Notice that I also flipped the condition to be values.Count % 2 == 0 that I find is easier to understand.

public static string ToBytePairs(this IList<string> values)
{
    var text = new StringBuilder();
    var lineWidth = 2;
    var byteSeparator = " ";

    for (var i = 0; i < values.Count; i += lineWidth)
    {
        text.AppendLine(string.Join(byteSeparator, values.SkipFast(i).Take(lineWidth)));
    }

    if (values.Count % 2 == 0)
    {
        text.AppendLine(string.Join(byteSeparator, new[] { "60000000", "00000000" }));
    }
    else
    {
        // There is line break at the and already so we need to insert this one before it.
        text.Insert(text.Length - 1, $"{byteSeparator}00000000");
    }

    return text.ToString();
}

As a bonus a faster skip extension so that the entire list doesn't have to be enumerated.

private static IEnumerable<T> SkipFast<T>(this IList<T> values, int index)
{
    for (var i = index; i < values.Count; i++)
    {
        yield return values[i];
    }
}

The default Skip is not a super fast solution but it's enough for short lists. You can easily find Skip optimized for IList that uses indexes if your strings a very big and every second matters. Just don't optimize too much if you don't need to. Premature optimization is the root of all evil.

Example:

var input = "8111111182222222833333338444444499999999";
var bytes = input.Bytify().ToList();
bytes.ToBytePairs().Dump(); // Dump() is LINQPad's method.

Result

// 8111111182222222833333338444444499999999

81111111 82222222
83333333 84444444
99999999 00000000

or

// 81111111822222228333333384444444

81111111 82222222
83333333 84444444
60000000 00000000


3D mathematical vector class I've been working on a 3D mathematical vector class which should be as streamlined as possible for use in numerical simulations. It will be used to model 3D-physical vectors.

Here, 3D-vector should be taken in mathematical sense, meaning a tuple (a,b,c).

I hoped to design it in a modern and fast way - but one is never perfect. So, I would be interested in some input from your side.  Any tips for making this faster?

//threevector.h
#ifndef threevector_h_
#define threevector_h_

#include <fstream>
#include <cmath>
#include <array>

template <class T>
class threevector
{
private:
  static const int dim = 3; //dimension of vector
  std::array<T,dim> container;

public:

  //constructors and assignment
  threevector(const double a = 0, const double b = 0, const double c = 0):
  container({{a,b,c}}) {}; //standard constructor
  threevector(const threevector& a): container(a.container) {};
  //copy constructor

// add once gcc 4.7 is used
//   threevector(threevector&& other): threevector() {swap(*this, other);}
  // move constructor

  threevector& operator=(threevector rhs) //assignment
  {
    swap(*this, rhs);
    return *this;
  }

  void swap(threevector& first, threevector& second)
  {first.container.swap(second.container);}

  //operators
  threevector& operator+=(const threevector& rhs)
  {
    container[0] += rhs.container[0];
    container[1] += rhs.container[1];
    container[2] += rhs.container[2];
    return *this;
  }

  threevector& operator-=(const threevector& rhs)
  {
    *this += -rhs;
    return *this;
  }

  threevector& operator*=(const double rhs) //scalar multiplication assignment
  {
    container[0] *= rhs;
    container[1] *= rhs;
    container[2] *= rhs;
    return *this;
  }

  threevector& operator/=(const double rhs) //scalar division assignment
  {
    *this *= 1./rhs;
    return *this;
  }

  threevector operator+() const //unary plus
  {
    threevector a(*this);
    return a;
  }

  threevector operator-() const //unary minus
  {
    threevector a(*this);
    a *= -1;
    return a;
  }

  T& operator[](const int input) {return container[input];} //access operator

  const T& operator[](const int input) const {return container[input];}
  // const access operator

  //utility functions
  double abs() const
  {return sqrt(container[0]*container[0]+container[1]*container[1]+
   container[2]*container[2]);}
  double abs_sq() const {return pow(abs(),2);}
  void reset() { container[0] = 0; container[1] = 0; container[2] = 0;}
};

//output operator
template<class T>
std::ostream& operator<<(std::ostream& os, const threevector<T>& obj)
{
  os << std::fixed << "(" << obj[0] << "," << obj[1] << "," << obj[2] << ")";
  return os;
}

//addition operator
template<class T>
inline threevector<T> operator+(threevector<T> lhs, const threevector<T>& rhs)
{
  lhs += rhs;
  return lhs;
}

//subtraction operator
template<class T>
inline threevector<T> operator-(threevector<T> lhs, const threevector<T>& rhs)
{
  lhs -= rhs;
  return lhs;
}

//scalar product
template<class T>
inline double operator*(const threevector<T>& lhs, const threevector<T>& rhs)
{return lhs[0] * rhs[0] + lhs[1] * rhs[1] + lhs[2] * rhs[2];}

//product with scalar
template<class T>
inline threevector<T> operator*(const double lhs, threevector<T> rhs)
{
  rhs[0] *= lhs;
  rhs[1] *= lhs;
  rhs[2] *= lhs;
  return rhs;
}

//product with scalar
template<class T>
inline threevector<T> operator*(threevector<T> lhs, const double rhs){return rhs*lhs;}

// scalar division
template<class T>
inline threevector<T> operator/(threevector<T> lhs, const double rhs){return lhs*(1./rhs);};

#endif

Your design is a little bit confusing. You define your class as a template class, but just about everything takes and returns a double. Generally, you should try and be consistent: will this class work for any numeric-like T? Then make sure it has constructors that take a T instead of a double:

threevector(T a = T(), T b = T(), T c = T())

Similarly for the overloaded operators that multiply/divide by a scalar, and so on.

The name threevector is awkward. At the least it should be three_vector, but that's still a little awkward. vector_3d is more of the "standard" name for this kind of thing.

Your include guards should generally be in all-caps. This is because they are global in scope, and you want to do as much as possible to minimize the possibility of potential name clashes.

#ifndef threevector_h_ // should be THREE_VECTOR_H_
#define threevector_h_ // or VECTOR_3D_H_

You have some semi-colons where they aren't needed:

threevector(const double a = 0, const double b = 0, const double c = 0):
container({{a,b,c}}) {}; //standard constructor
threevector(const threevector& a): container(a.container) {}; // <--- Not needed

Comments like //standard constructor are obvious and don't really need to be there. Further, your copy constructor and copy assignment operator are redundant; the compiler generated ones are sufficient.

It's often easiest to write operator+ (and -, * etc) in terms of operator+=:

vector_3d operator+(const vector_3d& a, const vector_3d& b)
{
     vector_3d result(a);
     result += b;
     return result;
}

For something this simple, it's unlikely you'll be able to speed it up much. The only potential that I can see is replacing the call to pow() in:

double abs_sq() const {return pow(abs(),2);}

with something like:

T abs_sq() const
{
    const T ab = abs();  // abs() should also return a T
    return ab * ab;
}

which avoids the overhead of a function call to pow. In practice, this is unlikely to make much of a difference, however.


Internal method to calculate the distance between all nodes in Kruskal's Importance (Driver) Algorithm I've been trying to reduce the indentation from a method in my code. The solution will probably be unrelated to the language, in this case python, although it may rely on some python specific libraries e.g. itertools.

The code below is part of this implementation of Kruskal's algorithm.

def generate_diff(self, ndarr, arr):
    """
    Internal method to calculate the difference between all points
    """
    l = ndarr.shape[1]
    pij = np.empty((l,l,)) * np.nan
    pijm = np.empty((l,l,l)) * np.nan
    for i in range(l):
        for j in range(l):
            if i != j:
                pij[i, j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j]]))
                for m in range(l):
                    if m != i and m != j and m > j:
                        pijm[m, i, j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j], ndarr[:, m]]))
    return (l, pij, pijm)

As you can see it's looping through the length of the shape 3 times! (in creating i, j, m).

ndarr and arr are just a two dimensional numpy array and a one dimensional numpy array. The specs in the file will enable you to hit the method very quickly if you add an assert False and a run py.test --pdb from the command.

I've tried changing the first line to for i, j in it.combinations(range(l), 2): and doing both sides of the square of the matrix simulataneously with:

pij[i, j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j]]))
pij[j, i] = self.pcor_squared(np.array([ndarr[:,j], arr, ndarr[:,i]]))

but it's a bit off (i.e. gives wrong numbers in the spec).

In short I'm trying to reduce the complexity by making the code linear and hopefully making it more performant by removing the looping. A better understanding of matrices may be key to this.

You can use comprehensions.

A comprehension is a special way to write a for loop, or nested for loops.
This is a functional part of the language and is derived from set-builder notation.
If we look at Wikipedia we'll see:

\$S = \{2 \dot{} x | x \in \mathbb{N}, x^2 > 3\}\$

In imperative Python this would be:

from itertools import count

S = []
for x in count(0):
    if x ** 2 > 3:
        S.append(2 * x)
...

Ignoring the inability for this function to complete, that's a bit long winded.
Instead we can use a list comprehension:

S = [
    2 * x
    for x in count(0)
    if x ** 2 > 3
]

This allows us to write what would be multiple indentations of code in one line.
And so take the code:

for i in range(l):
    for j in range(l):
        if i != j:
            # Do stuff

Using comprehensions we can change it to:

for i, j in [
        (i, j)
        for i in range(l)
        for j in range(l)
        if i != j]:
    # Do stuff

This allows a large reduction in the amount of tabs.
As we can just use two comprehensions.

for i, j in [
        (i, j)
        for i in range(l)
        for j in range(l)
        if i != j]:
    pij[i,j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j]]))
    for m in [m for m in range(l) if m != i and m != j and m > j]:
        pijm[m,i,j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j], ndarr[:,m]]))

I'd like to point out two ways to improve on this.
As we are currently using list comprehensions that build an entire list before using the first item we can change them to generator comprehensions.
This is as simple as changing the [] to ().
This would allow us to use the list \$\{2 \dot{} x | x \in \mathbb{N}, x^2 > 3\}\$, for example:

import itertools

S = (2 * x for x in itertools.count(0) if x ** 2 > 3)
print(list(itertools.islice(S, 10)))

The second way to improve the above is to move the two checks m != j and m > j into the range.
This would change the range to range(j + 1, l) and just leave the m != i.

And so you can get:

for i, j in (
        (i, j)
        for i in range(l)
        for j in range(l)
        if i != j):
    pij[i,j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j]]))
    for m in (m for m in range(j + 1, l) if m != i):
        pijm[m,i,j] = self.pcor_squared(np.array([ndarr[:,i], arr, ndarr[:,j], ndarr[:,m]]))


OO design of Reverse Polish Notation Calculator This solution focuses on the design of classes, not the actual push and pop stack part. I will include that part of the code as well. The current code has 2 operators (plus and minus).

  If we add another subclass under Token, is there a way that we don't need to check in processInput() what each operator is and perform the right calculation?

I didn't know how to answer that question. I looked into Java Reflection, but still don't know how reflection can help me in this case. Can anyone shed some light on how I can make this design better?

I want to allow people to add new operators, multiply and divide, and maybe make their own definition for special operators.

This application will take input string like "1 2 + 1 -" and output 2 because "1 2 + 1 -" is the reverse Polish notation for  (1+2)-1 = 2.

import java.util.*;
public class RPNCalculator {
    public static Stack<Integer> stack;
    //assume this string has integers and legal operators +,- and deliminated by space
    // an example would be "12 2 + 1 -"
    private static String input;
    static String operators ="+-";

    public RPNCalculator(String userInput){
        input = userInput;
        stack = new Stack<Integer>();
    }

    public void processInput(){
        StringTokenizer st = new StringTokenizer(input, " ");
        while (st.hasMoreTokens()){
            String str = st.nextToken();
            if(!operators.contains(str)){
                stack.push(Integer.parseInt(str));
            }
            else{
                if(str.equals("+")){
                    Plus obj = new Plus();
                    obj.calc();
                }
                else if(str.equals("-")){
                    Minus obj2 = new Minus();
                    obj2.calc();
                }
            }
        }
        System.out.println(stack.pop());
    }
}

public abstract class Token {
    abstract void calc();
}
public class Minus extends Token {

    @Override
    void calc() {
        int a = RPNCalculator.stack.pop();
        int b = RPNCalculator.stack.pop();
        RPNCalculator.stack.push(b-a);

    }

}

public class Plus extends Token {

    @Override
    void calc() {
        int a = RPNCalculator.stack.pop();
        int b = RPNCalculator.stack.pop();
        RPNCalculator.stack.push(a+b);
    }

}
public class RPNDriver {
       public static void main(String[] args) {
           System.out.println("starting calculator...");
           RPNCalculator rpn = new RPNCalculator("1 2 +");
           rpn.processInput();
       }
}

You can add more flexibility by storing a mapping from a String to a Token in a Map. After populating the map, you'll just need to look up for the token in it instead of writing a a long sequence of if-else blocks.

That is, you can define and populate it like this:

Map<String, Token> operators = new HashMap<>();
operators.add("+", new Plus());
// other operators go here

and then use it like this:
operators.get(str).calc()

This way, you just need to define a new subclass of the Token and add its instance to the operators map to define a new operator.

You can make the code even more flexible by passing this map as a parameter to the processInput method to decouple reading the input from the computations (this way, a client will be able to define his own subclass of the Token and pass it to this method).

I'd also rename the Token. Operator or Operation seems more appropriate to me (because a number is also a token, so this name is more precise).

There's also no point in having static members in the RPNCalculator class. It makes it non-reusable and might create a bunch of issues in a multithreaded environment. I'd suggest to make all members non-static and encapsulate them properly (and pass a reference to an instance of this class to the constructor of the concrete Token subclasses).

You can also decouple the operators from the calculator completely by changing the signature of the calc method to public int calc(int a, int b) (it might be an issue for non-binary operators, though).

You can improve your code by handling the errors more carefully. For instance, if the number of tokens on the stack is more than one in the end of the evaluation, your code will return some value, but it should be an error ("1 2" is not a valid expression, is it?). You can check such cases and throw an appropriate exception.


Get random sets of 5 from 50 From a deck of 50 need to get sets of 5 as random as possible and as fast as possible.  The thought here is to shuffle to get 10 sets of five at a time with no collisions.

Int1 and Int2 will not change during a run.  If they do other bad stuff would happen.

    public MainWindow()
    {
        InitializeComponent();
        Stopwatch sw = new Stopwatch();
        sw.Start();
        for(int i = 0; i < 100000; i++ )
        {
            //Debug.WriteLine(string.Join(", ", NextFive(11, 12)));
            int count = NextFive(11, 12).Count();
        }
        sw.Stop();
        Debug.WriteLine(sw.ElapsedMilliseconds.ToString("N0"));
    }
    Random rand = new Random();
    private int[] deckBase;
    int nextFiveLastStart = 45;
    private IEnumerable<int> NextFive(int int1, int int2)
    {
        if (int1 == int2)
            throw new IndexOutOfRangeException("int1 == int2");
        if (int1 > 51 || int2 > 51)
            throw new IndexOutOfRangeException("int1 > 51 || int2 > 51");
        if (deckBase == null)
        {
            deckBase = new int[50];
            int j = 0;
            for (int i = 0; i < 52; i++)
            {
                if (i == int1 || i == int2)
                    continue;
                deckBase[j] = i;
                j++;
            }
        }
        if (nextFiveLastStart >= 45)
        {
            nextFiveLastStart = 0;
            // Yates shuffle
            for (int i = 49; i >= 1; i--)
            {
                int j = rand.Next(i + 1);
                if (j != i)
                {   // exchange values
                    int curVal = deckBase[i];
                    deckBase[i] = deckBase[j];
                    deckBase[j] = curVal;
                }
            }
        }
        else
            nextFiveLastStart += 5;
        return deckBase.Skip(nextFiveLastStart).Take(5);
    }

private IEnumerable<int> NextFive(int int1, int int2)

The names int1 and int2 give absolutely no clue about what they are good for. I tried to ready the code but I cannot figure it out. Looking at if (i == int1 || i == int2) I guess it means exclude but who knows.

throw new IndexOutOfRangeException("int1 == int2");

This isn't IndexOutOfRangeException but rather ArgumentException. The argumetns have invalid values but they might be within the allowed range.

throw new IndexOutOfRangeException("int1 > 51 || int2 > 51");

I agree this is the right type of the exception but I preferred it checking each value separately to give the user a hint which one of the parameters is incorrect.

Random rand = new Random();
private int[] deckBase;
int nextFiveLastStart = 45;

Inconsistent access modifier usage. private implicit, private explicit, private implicit... Pick one ;-)

int j = 0;
for (int i = 0; i < 52; i++)
{
    if (i == int1 || i == int2)
        continue;
    deckBase[j] = i;
    j++;
}

There's no need for the int j = 0; you can put it inside the for:

for (int i = 0, j = 0; i < 52; i++)
{
    if (i == int1 || i == int2)
        continue;
    deckBase[j] = i;
    j++;
}


jQuery Shorten Switch Statement I have a switch statement that checks whether or not one string matched another that was stored in an array. However, there are multiple cases in which the same result is returned for multiple matches. I originally had a large amount of if/else checks, but tried to shorten that down with the switch statement. I was wondering, is there anyway to shorten down this switch statement?

function checkLoc(xstreets) {
            var address;
            if(xstreets[2] == undefined) {
                address = xstreets[0]+" & "+xstreets[1]+" Chicago IL";
            }
            else {
                switch(xstreets[2]) {
                    case "aon":
                        address = "Aon Center Chicago IL";
                        break;
                    case "trump":
                        address = "401 North Wabash Avenue, Chicago, IL";
                        break;
                    case "pritzker":
                        address = "Pritzker Park Chicago IL";
                        break;
                    case "hyde":
                    case "hydepark":
                    case "uofc":
                    case "uchicago":
                        address = "5801 South Ellis Avenue, Chicago, IL";
                        break;
                    case "reg":
                        address = "1100 E 57th St, Chicago, IL 60637";
                        break;
                    case "willis":
                    case "wttw":
                        address = "233 South Wacker Drive, Chicago, IL";
                        break;
                    case "600":
                        address = "600 West Chicago Avenue, Chicago, IL";
                        break;
                    default:
                        address = xstreets[0]+" & "+xstreets[1]+" Chicago IL";
                        break;
                }
            }
            return address;
        }

You can convert it to two array. The first will normalize multiple "aliases" of the same key into one and second will have your address values.

var aliases = {
  "hyde" : "hydepark",
  "uofc" : "hydepark",
  "uchicago" : "hydepark",
  "wttw" : "willis"
};

var addresses = {
  "hydepark" : "5801 South Ellis Avenue, Chicago, IL",
  ...
};

var alias = aliases[xstreets[2]];
if(alias == undefined) {
  alias = xstreets[2]
}
var address = addresses[alias];
if(address == undefined) {
  return xstreets[0]+" & "+xstreets[1]+" Chicago IL";
}
return address;


Storing variables to a map by splitting a text file I'm currently making a social media app that contains profile details such as interests in a text file. The following code works by using markers in the text file to upload these details to map data structures. I'm wondering if there is a more efficient way of doing this.

textList = ReadFile(FileName);
System.out.println("File Length" + textList.size());
for (int i = 0; i < textList.size(); i++){
    String[] split = splitTxt(textList.get(i), "\\s+");
    List<String> friends = new ArrayList<>();
    List<String> interests = new ArrayList<>();
    if (split.length >= 2){
        String[] interestArray = splitTxt(split[2], ";");
        for(int j = 0; j < interestArray.length; j++){
            interests.add(interestArray[j]);
        }
        Accounts.put(split[0],split[1]);
        intrestDic.put(split[0],interests);
        for(int j= 3; j < split.length; j++){
            friends.add(split[j]);

        }
        FriendsDic.put(split[0],friends);
        System.out.println(FriendsDic.get(split[0]));
    }

}

I've created a split function that looks for marker to split it on

public static String[] splitTxt(String text, String type){
    String[] parsedText = text.split(type);
    return parsedText;
}

Range-based for loop

for (int i = 0; i < textList.size(); i++){
    String[] split = splitTxt(textList.get(i), "\\s+");

You can simplify this with a range-based for loop.

for (String text : textList) {
    String[] split = text.split("\\s+");

No need to manage i manually.

I also eliminated the call to splitTxt, which makes the code smaller and easier to read.

Bug

    List<String> friends = new ArrayList<>();
    List<String> interests = new ArrayList<>();
    if (split.length >= 2){
        String[] interestArray = splitTxt(split[2], ";");

This will cause an array index out of bounds when the split.length is 2.

    if (split.length < 3) {
        continue;
    }

    String[] interestArray = splitTxt(split[2], ";");
    List<String> friends = new ArrayList<>();
    List<String> interests = new ArrayList<>();

When split is of length 2, only 0 and 1 are valid indexes.  So split[2] would be out of bounds.  Making it a 3 instead of a 2 would fix that.

Switching to an early exit changes from >= to < and allows us to reduce the level of indent.  It works here because the entire code for an iteration after that point is inside the if.

There is no point to declaring and initializing friends and interests if we're not going to use them.  So do those tasks after the check, not prior.

asList

        String[] interestArray = splitTxt(split[2], ";");
        for(int j = 0; j < interestArray.length; j++){
            interests.add(interestArray[j]);
        }
        Accounts.put(split[0],split[1]);
        intrestDic.put(split[0],interests);

You don't need to manually copy one by one.

        Accounts.put(split[0],split[1]);
        intrestDic.put(split[0], new ArrayList<String>(Arrays.asList(split[2].split(";"))));

Using a copy constructor with Arrays.asList will allow the whole thing to be copied at once.

Summary

private static final int HEADER_SIZE = 3;

and later

for (String text : ReadFile(FileName)) {
    String[] split = text.split("\\s+");
    if (split.length < HEADER_SIZE) {
        continue;
    }

    Accounts.put(split[0], split[1]);
    intrestDic.put(split[0], new ArrayList<String>(Arrays.asList(split[2].split(";"))));
    List<String> friends = Arrays.asList(split).subList(HEADER_SIZE, split.length));
    FriendsDic.put(split[0], new ArrayList<String>(friends);
}

This isn't necessarily more efficient, although it could be (depending on how Java optimizes it).  The primary advantage of this version is that it leaves maintenance to the Java engine.

Hibernate

Hibernate (and Java Persistence Annotations in general) is for exactly this task.  It maps classes to storage and vice versa.  Now, that might be a heavyweight solution, but it is one that exactly addresses the problem.

You can write persistence code manually, without annotations.  But I would generally still use a database if you intend the code to scale at all.  You don't want to read all the records every time you want one record.  Rather than reinventing that wheel, just use the existing solution:  a database.

Hibernate makes it comparatively easy to switch from one database to another.


DailyProgrammer 280 Easy: Counting on Fingers We are trying to count to 100 on two hands!

Input is given like so: LP  LR  LM  LI  LT  RT  RI  RM  RR  RP (i.e. 0111011100)

Where:

L = Left, R = Right, P = Pinky, R = Ring Finger, M = Middle Finger, I = Index Finger, T = Thumb

Our job is to make sure the code is valid according to this counting scheme, and then show what the number represented is.

Original post can be found here.

My solution validates the input by checking to see if there is a "1" followed by a "0" for the "left" hand, and vice-versa for the right. I have it set up so that positions 3,4, and 5 aren't checked (isn't necessary as they can't invalidate the input).

Looking for feedback on best practice, convention, better ways to solve problem, and really just anything :)

EDIT: I am not interested in checking for incorrect input such as "0123456789"

#include <iostream>
#include <string>

int main () {
    bool isValid = true;
    int total = 0;
    std::string input = "0111011100"; // This one is 37

    // Divide by 2 because there is two hands; subtract 1 to zero index
    int oneHandLength = (input.length() / 2) - 1;

    // Verify
    for(int a = 0; a <= oneHandLength; ++a){

        // Left
        if(((input[a] - '0') > (input[a + 1] - '0')) && ((a + 1) < oneHandLength)){
            isValid = false;
            break;
        }

        // Right
        if(((input[a + oneHandLength] - '0') < (input[(a + oneHandLength) + 1] - '0')) && (a + oneHandLength > oneHandLength + 1)){
            isValid = false;
            break;
        }
    }

    // Count up the total
    if(isValid){

        for(int b = 0; b < input.length(); b++){
            if ((input[b] - '0')){
                if(b < oneHandLength) total += 10;
                else if(b == oneHandLength) total += 50;
                else if(b == oneHandLength + 1) total += 5;
                else total += 1;
            }
        }

        std::cout << input << " is valid: " << total;
    }
    else std::cout << input << " isn't valid!";

    return 0;
}

Overall, your code is really good :) Good job.

The thing that bothers me is the inconsistent weird indentation, but that's probably because of copying and pasting.

The 2 verifying conditions do the same thing, you can put them together, which reduces maintenance in the long term, although for such a small project it's not really important.

if (((input[a] - '0') > (input[a + 1] - '0') && (a + 1) < oneHandLength) ||
    ((input[a + oneHandLength] - '0') < (input[(a + oneHandLength) + 1] - '0') && a + oneHandLength > oneHandLength + 1));

Depending on the architecture, int is not guaranteed to cover the entire range of std::size_t, so you should use that as a type instead of int to avoid overflowing. This is not a problem now because you only have 10 fingers, but if you decide to add more... :)

std::size_t oneHandLength = (input.length() / 2) - 1;
//'auto' would also be possible if you want to

Don't use integers as booleans. It is better to clearly state that you want to check if the integer is greater than 0 (or equal to 1 in this case):

if (input[b] - '0' == 1);

Although using parenthesis reduces confusion, I think you are using too many of them:

if ((input[b] - '0')){
    ^              ^

if(((input[a] - '0') > (input[a + 1] - '0')) && ((a + 1) < oneHandLength)){
   ^^              ^   ^                  ^^    ^^     ^                ^

You have inconsistent spacing, choose one and be consistent:

for(int a = 0; a <= oneHandLength; ++a){
int main () {
if ((input[b] - '0')){
if(isValid){

You aren't including a newline after the last output, which can be a "problem" when run in a shell:

$ ./hands
$ 0111011100 is valid: 37 $ (command here - on same line!)

Minor thing: You don't need to return 0;, so you can remove it if you want to.


Security of code snippets Last time I made a website using PHP, I didn't know of PDO, so someone dropped all my tables. I think I've made an improvement now. I'm sorry if there's a lot to go through, but I want to make sure its safe (most importantly) and also using the best practices.

Here are some snippets:

1)

$form = $_POST;     $name = $form[ 'name' ];    $desc = $form[ 'desc' ]; $link = $form[ 'link' ];   $tag = $form[ 'tag' ];  $category = $form[ 'category' ];

$sql = "INSERT INTO items ( `name`, `desc`, `link`, `tag`, `category` ) VALUES ( :name, :desc, :link, :tag, :category )";

$query = $db->prepare( $sql );  $query->execute( array( ':name'=>$name, ':desc'=>$desc, ':link'=>$link, ':tag'=>$tag, ':category'=>$category ) );

2)

$db = new PDO('mysql:host=mywebsite.com;dbname=thisdb;charset=utf8','root', '');

3)

if (isset($_GET['search'])) {
   $searchStmt = $db->prepare('SELECT * FROM items WHERE tag LIKE    :usersearch'); $searchStmt->execute(['usersearch' =>    "%".$_GET['search']."%"]);

   foreach ($searchStmt as $row) { ?> <?php    include('includes/results-layout.php'); ?> <?php }} else {    header('Location: http://localhost/photo'); }
   if ($_GET['search'] == "") {     header('Location:    http://mywebsite.com/photo');  }
   if ($searchStmt->rowCount() === 0) {     echo "Sorry, your search for    \"".$_GET['search']."\" was not found...";   }

4)

$page1Stmt = $db->query("SELECT * FROM items WHERE category = 1 ORDER BY id DESC");

5)

$downloadStmt = $db->prepare('SELECT * FROM items WHERE id = :downloadID'); $downloadStmt->execute(['downloadID' => $_GET['id']]); ?> $tags = explode(" ",$row['tag']);
foreach($tags as $tag) {echo "".$tag."";} $relatedStmt = $db->prepare("SELECT * FROM items WHERE tag = (select tag from items where id = :downloadID)");
$relatedStmt->execute(['downloadID' => $_GET['id']]); ?>

6)

$pageStmt = $db->query("SELECT * FROM items ORDER BY id DESC");
foreach ($pageStmt as $row) { ?>
<?php include('includes/results-layout.php'); ?>
<?php } ?>

Okay

All of them

Don't have multiple statements on the same line like that.
Use either spaces or tabs, don't use both.
<?php something; ?> <?php somethingElse; ?> is pointless. Don't do that.
Be consistent. Sometimes you're doing funcName( $param ) and sometimes funcName($param). Pick one and stick with it.

1)

Looks good security wise. See above All of them for improving.

2)

Looks good, but too little information to know. If you haven't already, set PDO::ATTR_ERRMODE to PDO::ERRMODE_EXCEPTION and PDO::ATTR_EMULATE_PREPARES to false. See `PDO::setAttribute().

3)

Sometimes you are redirecting to localhost, sometimes to mywebsite. Use a variable. See $_SERVER.
}} is horrible.

4)

Very good. Queries that don't use parameters don't need to be prepared. Note however that there's a very fine line! Every time you need a variable inside of a query, you want a prepared statement.

5)

You are vulnerable to XSS attacks on your tags. This can be prevented by escaping your tags for HTML output.

6)

Look at the all of them list for details on how to improve.

All in all, you must improve your code-style. Your code is extremely unreadable and will be hard to maintain in the future.

Aside from that one XSS vulnerability, it looks good to me, but again, it's hard to tell just with the code you've given.


Clearing files from directories My code does what I want, which is remove any files from these specific folders. Is there a smoother way to do this?

# Test Script to remove any items

Remove-Item -Path 'C:\users\test_removal_account\desktop\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\contacts\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\documents\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\Downloads\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\Music\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\Pictures\*.*' -Recurse
Remove-Item -Path 'C:\users\test_removal_account\Videos\*.*' -Recurse

There's an important principal in programming called Don't Repeat Yourself (or DRY). This says that if there is any repeated code, then that is usually a bad thing. So let's identify the non-repeated code, and start with that. This is the stuff that changes:

desktop
contacts
documents
Downloads
Music
Pictures
Videos

We could put that in a file, but that's probably overkill here, so let's just put that in an array:

$dirNames =
    "desktop",
    "contacts",
    "documents",
    "Downloads",
    "Music",
    "Pictures",
    "Videos"

Now we can just operate on the array:

foreach ($dirName in $dirNames)
{
    Remove-Item -Path "C:\users\test_removal_account\$dirName\*" -Recurse -WhatIf
}

Notice I put -WhatIf on the Remove-Item. That just makes the command tell you what it is going to do instead of actually doing it. This is useful for testing purposes. You can remove it when you are happy the script does what you want.

This script as it stands is dangerous. Recursive deletes must be treated with respect. If you accidentally blow away someone's work, you could ruin their life. You may even accidentally blow away all your own work and ruin your own life. You have to think about this sort of thing as a responsible engineer.

Of course, the "test_removal_account" makes the script pretty benign as it stands, but I don't know what you are going to replace that with in the real script.

I don't think a script ought do anything destructive if you just run it.

If it were me, I would put in a confirmation check before deleting. I would get the user to type in something, such as "really delete" before doing the delete.


Form submission handler with terms and conditions checkbox and double-click prevention I'm experienced with JavaScript, but new to refactoring.  Sonar specified that the below function is too complex.  I'm looking for suggestions on how to simplify it, mainly the conditional statements.

_executeWhenReady: function() {
    if ($('#applyButton').length) {
        $('#applyButton').on('click', function(e) {
            if (appProcessing.isTermsChecked() && !$('#applyButton').hasClass('noClick')) {
                $('#applyButton').addClass('noClick');
                appProcessing.proceedToApp();
            } else {
                e.preventDefault();
                $("#termsConditionsChBx").removeClass('checkbox-checked').removeClass('checkbox').removeClass('checkbox-error').addClass('checkbox-error');
                if ($('.checkboxErrorText').length) {
                    $('.checkboxErrorText:first').removeClass('hidden');
                }
                return false;
            }
        });
    }

    if ($('#btnBenefitsCancel').length) {
        $('#btnBenefitsCancel').on('click', function() {
            $('#benefitsCancel').attr('value', true);
        });
    }

    if ($("#termsConditionsChBx").length) {
        $('#termsConditionsChBx').on('click', function() {
            appProcessing._eConsentCheck("termsConditionsChBx");
        });
    }
    $("#termsConditionsChBx_Native").on('change', function() {
            appProcessing._eConsentCheck("termsConditionsChBx");
    });
}

A couple thoughts:

Don't keep re-querying the DOM with jQuery selectors. This is poor from a performance standpoint.  Put the jQuery collection into a variable and don't keep re-querying the DOM every time you need to work with the collection.

For example:

var $applyButton = $('#applyButton');
var $btnBenefitsCancel = $('#btnBenefitsCancel');

Or for case when you need to find one element in collection like your :first selector:

var $checkboxErrorText = $(.checkboxErrorText);
if($checkboxErrorText.length) {
    $checkboxErrorText.first().removeClass('hidden');
}

A good rule of thumb is if you find yourself using the selector (or some subselection of it) more than once, put it into a variable.

Why do you need to check if there are any elements that meet the condition for adding the click handlers?  You could easily remove the $(*).length checks and your code would function exactly the same (and probably make Sonar very happy at the same time). Same thing applies to the checkboxErrorText example. It is OK to try to operate against a zero length jQuery collection in most cases. I would reserve such tests of collection size to those cases where you need to have alternate code behavior if the selector returns an empty collection. This is not really the case for the use cases you have shown (though it could be if you needed to do something like log an error or alert the end user of an empty selection result).
Be more consistent in how you name your DOM element id's. It is confusing to have one button name like applyButton and another like btnBenefitsCancel. You can read up on semantic naming for your HTML id's, class names, etc. to formulate a good approach on having a consistent naming strategy.
You have some really long line of code, which make your code hard to read.

For example, take this line of code:

            $("#termsConditionsChBx").removeClass('checkbox-checked').removeClass('checkbox').removeClass('checkbox-error').addClass('checkbox-error');

You could make this more readable in several ways.  First just breaking up across lines:

            $("#termsConditionsChBx")
                .removeClass('checkbox-checked')
                .removeClass('checkbox')
                .removeClass('checkbox-error')
                .addClass('checkbox-error');

But you can also pass multiple class name into removeClass() and addClass(), so you could actually implement like this if you like:

            $("#termsConditionsChBx")
                .removeClass('checkbox checkbox-checked checkbox-error')
                .addClass('checkbox-error');

This approach would actually perform better as well, as the collection would only be iterated a single time to remove all those classes as opposed to iterating three times due to having having three different removeClass() calls. If you are working with large collections, this could add up.

The same approach of splitting longer lines of code across multiple lines could also be applied to longer conditionals like this line of code:

        if (appProcessing.isTermsChecked() && !$('#applyButton').hasClass('noClick')) {

Could be:

        if (
            appProcessing.isTermsChecked() &&
            !$('#applyButton').hasClass('noClick')
        ) {

If you should consider incorporating style checks into your static code analysis such that you can automatically flag style issues like lines of code that are too long (typically ~80 characters).