Can this sampling code be vectorized or optimized? I have some code that is slow because it loops over every row in an input matrix Y. Obviously, this code does not scale with the number of rows. I don't think it's critical to understand what the code does, but for the curious, it is a Gibbs sampling step for sampling the dispersion parameter r for the negative binomial distribution.

Is there anything I can do to make this faster? I've tried for y in Y_j[Y_j > 0] in the crt_sum function, but that's not actually faster according to a simple speed test. I suspect because in many cases, the code loops over every row in the Y_jth column of Y in C and then does it again in Python.

import numpy as np

def sample_r(Y, P, R, e0=1e-2, f0=1e-2):
    """Sample negative binomial dispersion parameter `r` based on
    (Zhou 2020). See:

    - http://people.ee.duke.edu/~mz1/Papers/Mingyuan_NBP_NIPS2012.pdf
    - https://mingyuanzhou.github.io/Softwares/LGNB_Regression_v0.zip
    """
    J = Y.shape[1]
    for j in range(J):
        L = crt_sum(Y, R, j)
        A = e0 + L
        # `maximum` is element-wise, while `max` is not.
        maxes = np.maximum(1 - P[:, j], -np.inf)
        B = 1. / (f0 - np.sum(np.log(maxes)))
        R[j] = np.random.gamma(A, B)
    # `R` cannot be zero.
    inds = np.isclose(R, 0)
    R[inds] = 0.0000001
    return R

def crt_sum(Y, R, j):
    """Sum independent Chinese restaurant table random variables.
    """
    Y_j = Y[:, j]
    r   = R[j]
    L   = 0
    tbl = r / (r + np.arange(Y_j.max()))
    for y in Y_j:
        if y > 0:
            u = np.random.uniform(0, 1, size=y)
            inds = np.arange(y)
            L += (u <= tbl[inds]).sum()
    return L

Synthetic data should be sufficient. Basically, Y is a matrix of count data (non-negative integers), P is a matrix of numbers in the range [0, 1], and R is a matrix of positive (nonzero) real numbers. This should be sufficient to generate realistic data:

def sigmoid(x):
    return 1 / (1 + np.exp(-x))

N = 100
J = 10
Y = np.arange(N*J).reshape(N, J)
P = sigmoid(np.random.random((N, J)))
R = np.arange(J, dtype=float)+1

sample_r(Y, P, R)

I currently don't have any tests (sorry, lonely researcher code), but assuming my implementation is correct, this should pass

assert(sample_r(Y, P, R) == sample_r_fast(Y, P, R))

where sample_r_fast a faster implementation. Thanks in advance.

Yes, it is possible to vectorize the code. Does that make it faster? That depends on the spread of values in your data Y.

Computing the scale parameter can be vectorized without issues and should be faster.

For the shape parameter - I haven't read the papers - you seem to use something that looks quite similar to rejection sampling. You compute a probability distribution tbl over NxJ, then draw Y[n, j] many samples from a uniform distribution and check how often you land within tbl[n,j]. shape is then the sum of "hits" along N.

The naive approach to vectorizing this would be to draw the same amount of samples for each Y[n, j]. In this case at least max(Y) many. This can create a lot of overhead if your data has a few really large values but otherwise small ones. If the values are fairly close together, this will make things faster. If you have a way to quickly (pre-)generate large quantities of uniform numbers this limitation doesn't matter.

Here is the code:

def sample_r_vec(Y, P, R, e0=1e-2, f0=1e-2, random_numbers=None):
    """Sample negative binomial dispersion parameter `r` based on
    (Zhou 2020). See:

    - http://people.ee.duke.edu/~mz1/Papers/Mingyuan_NBP_NIPS2012.pdf
    - https://mingyuanzhou.github.io/Softwares/LGNB_Regression_v0.zip
    """

    if random_numbers is None:
        random_numbers = np.random.uniform(0, 1, size=(*Y.shape, np.max(Y) + 1))

    # compute shape
    Y_max_vec = np.arange(np.max(Y) + 1).reshape((-1, 1))
    R_vec = R.reshape((1, -1))
    tbl = (R_vec / (R_vec + Y_max_vec))
    tbl = tbl.reshape((*tbl.shape, 1))
    N_vec = np.arange(Y.shape[0]).reshape(-1, 1)
    J_vec = np.arange(Y.shape[1]).reshape(1, -1)
    sum_hits = np.cumsum(random_numbers <= tbl.T, axis=2)[N_vec, J_vec, Y - 1]
    sum_hits[Y == 0] = 0
    shape = e0 + np.sum(sum_hits, axis=0)

    # compute scale
    maxes = np.maximum(1 - P, -np.inf)
    scale = 1. / (f0 - np.sum(np.log(maxes), axis=0))

    # sample
    R = np.random.gamma(shape, scale)
    R[R < 1e-7] = 1e-7
    return R

Edit: Based on the comment, I did find a different bug that is now fixed.

Edit: Demonstration that both versions perform equally.

Firstly, we need a different test to assert equality of the code; try assert(sample_r(Y, P, R) == sample_r(Y, P, R)) and you will quickly see the problem. There are many reasons why a different test is needed: (1) sample_r and sample_r_fast return vectors and not scalars. The truth value of a vector is undefined (numpy also warns for that). (2) your code (sample_r) modifies R in place, which means that the input to sample_r_fast will be different to the input of sample_r. Logically, the outputs will differ due to the unwanted side effect. (3) Both functions are expected to create random samples and compute results based on that. assert tests for exact equality and will hence fail even if both versions are correct. Providing the same random seed won't be enough either, because the order in which the samples are used may differ, which will change the results. (4) This is a numerics problem; even in the deterministic part of the code it is only accurate up to a tolerance. (5) The code estimates parameters for a distribution and then samples once from the estimated distribution; these samples are then compared. If we want to know whether or not the two versions estimate the same distributions it seems much more efficient to directly compare the parameters.

To fix all of this, I modified the code in the following way:

Added an optional parameter to the function that can be used as random numbers; generate them if none.
Made sure the same random numbers are used for comparisons each time.
Removed the sampling of R at the end and instead return the estimated shape and scale directly.
It is also nice to test if the new code is actually faster, so I added a speed test (excluding RNG).

My full script looks like this:

import numpy as np

def sample_r(Y, P, R, e0=1e-2, f0=1e-2, random_numbers=None):
    """Sample negative binomial dispersion parameter `r` based on
    (Zhou 2020). See:

    - http://people.ee.duke.edu/~mz1/Papers/Mingyuan_NBP_NIPS2012.pdf
    - https://mingyuanzhou.github.io/Softwares/LGNB_Regression_v0.zip
    """

    if random_numbers is None:
        random_numbers = np.random.uniform(0, 1, size=(*Y.shape, np.max(Y) + 1))

    A_vec = np.zeros_like(R)
    B_vec = np.zeros_like(R)
    J = Y.shape[1]
    for j in range(J):
        L = crt_sum(Y, R, j, random_numbers)
        A = e0 + L
        A_vec[j] = A

        # `maximum` is element-wise, while `max` is not.
        maxes = np.maximum(1 - P[:, j], -np.inf)
        B = 1. / (f0 - np.sum(np.log(maxes)))
        B_vec[j] = B

        # R[j] = np.random.gamma(A, B)
    # `R` cannot be zero.
    # inds = np.isclose(R, 0)
    # R[inds] = 0.0000001
    return A_vec, B_vec

def crt_sum(Y, R, j, random_numbers):
    """Sum independent Chinese restaurant table random variables.
    """

    Y_j = Y[:, j]
    r   = R[j]
    L   = 0
    tbl = r / (r + np.arange(Y_j.max()))
    for n_idx, y in enumerate(Y_j):
        if y > 0:
            relevant_numbers = random_numbers[n_idx, j, :y]
            inds = np.arange(y)
            L += (relevant_numbers <= tbl[inds]).sum()
    return L

def sample_r_vec(Y, P, R, e0=1e-2, f0=1e-2, random_numbers=None):
    """Sample negative binomial dispersion parameter `r` based on
    (Zhou 2020). See:

    - http://people.ee.duke.edu/~mz1/Papers/Mingyuan_NBP_NIPS2012.pdf
    - https://mingyuanzhou.github.io/Softwares/LGNB_Regression_v0.zip
    """

    if random_numbers is None:
        random_numbers = np.random.uniform(0, 1, size=(*Y.shape, np.max(Y) + 1))

    # compute shape
    Y_max_vec = np.arange(np.max(Y) + 1).reshape((-1, 1))
    R_vec = R.reshape((1, -1))
    tbl = (R_vec / (R_vec + Y_max_vec))
    tbl = tbl.reshape((*tbl.shape, 1))
    N_vec = np.arange(Y.shape[0]).reshape(-1, 1)
    J_vec = np.arange(Y.shape[1]).reshape(1, -1)
    sum_hits = np.cumsum(random_numbers <= tbl.T, axis=2)[N_vec, J_vec, Y - 1]
    sum_hits[Y == 0] = 0
    shape = e0 + np.sum(sum_hits, axis=0)

    # compute scale
    maxes = np.maximum(1 - P, -np.inf)
    scale = 1. / (f0 - np.sum(np.log(maxes), axis=0))

    return shape, scale

if __name__ == "__main__":
    def sigmoid(x):
        return 1 / (1 + np.exp(-x))

    np.random.seed(1337)

    N = 100
    J = 10
    Y = np.arange(N*J, dtype=np.int32).reshape(N, J)
    P = sigmoid(np.random.random((N, J)))
    # use test case from comments
    R = np.ones(J, dtype=np.float32); R[J-1] = 5000
    random_numbers = np.random.uniform(0, 1, size=(*Y.shape, np.max(Y) + 1))

    shape_normal, scale_normal = sample_r(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers)
    shape_vec, scale_vec = sample_r_vec(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers)

    assert np.all(np.isclose(scale_normal, scale_vec))
    assert np.all(np.isclose(shape_normal, shape_vec))

    #speed test
    import timeit
    t1 = timeit.timeit(lambda: sample_r(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers), number=100)
    t2 = timeit.timeit(lambda: sample_r_vec(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers), number=100)
    print(f"Original version total time {t1:.2f}. Vector Version total time {t2:.2f}.")

    N = 1000
    J = 10
    Y = 100*np.ones(N*J, dtype=np.int32).reshape(N, J)
    P = sigmoid(np.random.random((N, J)))
    R = np.arange(J)+1
    random_numbers = np.random.uniform(0, 1, size=(*Y.shape, np.max(Y) + 1))
    t1 = timeit.timeit(lambda: sample_r(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers), number=100)
    t2 = timeit.timeit(lambda: sample_r_vec(Y.copy(), P.copy(), R.copy(), random_numbers=random_numbers), number=100)
    print(f"Original version total time {t1:.2f}. Vector Version total time {t2:.2f}.")

Note that this now has a python 3.7+ dependency due to format strings in the functional test, the relevant code does not have that dependency.

Output:

Original version total time 1.29. Vector Version total time 1.05.
Original version total time 8.55. Vector Version total time 0.98.

I did the following modifications to your code: To return the estimated parameters, I am stacking them up in a vector as they are being computed. To deal with the random sampling, I generate a whole bunch of random numbers (Y.max() many) for each Y[n, j], and then select the first y=Y[n,j] of them. This is the same idea I used to vectorize the code; it 'wastes' Y.max() - Y[n, j] many random samples at each step, because the code generates them but then doesn't use them; it's a trick to match the shapes for vectorization. This is the main reason why the vectorized version will only be faster if you either (1) pre-generate the random numbers, or (2) have a situation where the different Y[n, j] don't differ too much, so that generating the 'waste' doesn't take more time than is saved by hardware acceleration.

I hope this explains what is going on.

PS: Please, please use informative variable names for code in the future. L,n,j,A,B, ect. are not wise choices if others are to read your code or if you try to understand it 3 years from now.


fizzbu| - Fizzbuzz with dynamic height Here is a variation on a theme: fizzbuzz. After providing this answer recently, I decided to practice some VueJS skills with outputting the values and conditionally applying styles based on the value. The specific styles are as follows:

red border for Fizz
blue border for Buzz
purple border for FizzBuzz

And then I decided to allow the user to change the height of the container, in case he/she wanted a smaller viewport.

What, if anything, would you change?

const getIndex = (number, index) => index + 1;
const app = new Vue({
  el: "#app",
  data: {
    numbers: new Array(100).fill(1).map(getIndex),
    height: 3000
  },
  filters: {
    getOutput: function(number) {
      if (number % 3 === 0 && number % 5 === 0) return 'FizzBuzz';
      if (number % 3 === 0) return 'Fizz';
      if (number % 5 === 0) return 'Buzz';
      return number;
    }
  },
  methods: {
    getClass: function(number) {
      const output = this.$options.filters.getOutput(number);
      if (isNaN(parseInt(output, 10))) {
       return output;
      }
      return '';
    }
  }
});
body {
  padding: 4px;
  font-family: serif;
}
h1 {
   font: 400 20px cursive;
}
input[type="range"] {
  width: 100%;
}
#listContainer {
  overflow-y: hidden;
}

li {
  background: #fff;
  color: #000000;
  border-radius: 4px;
  border: 2px solid #6a737c;
  padding: 3px;
}

li.FizzBuzz {
  border-color: #f800ff;
}

li.Fizz {
  border-color: #f80000;
}

li.Buzz {
  border-color: #0000ff;
}

    FizzBuzz with dynamic height container

   Height: {{ height }}

  <input type="range" min="200" max="3000" v-model="height" />

        {{ number | getOutput }}

Found a little inconsistency in your Vue template

Using v-bind: syntax here,

... but using : shorthand here:

Useless variable app

const app = new Vue({ /* ... */ });

Do I need to say more?

EDIT: YES.
From ESLint: "Variables that are declared and not used anywhere in the code are most likely an error due to incomplete refactoring. Such variables take up space in the code and can lead to confusion by readers."

Object method notation shorthand

Instead of using:

getOutput: function(number) { /* ... */ }
// ...
getClass: function(number) { /* ... */ }

You could use:

getOutput(number) { /* ... */ }
// ...
getClass(number) { /* ... */ }

Too specific CSS selector

You're specifying these selectors,

li.FizzBuzz { /* ... */ }
li.Fizz { /* ... */ }
li.Buzz { /* ... */ }

but there aren't any elements that would have these classes (FizzBuzz, Fizz and Buzz) other than li's. This means you could simplify it to:

.FizzBuzz { /* ... */ }
.Fizz { /* ... */ }
.Buzz { /* ... */ }


Finding an index in an array of objects with 3 possible outcomes currentUserPosts will have a post object with an id that either matches a tempId, an Id, or nothing.

If there's a tempId match there won't be an id match and vice versa.

This code works but there's something about it I don't like. How can it be refactored?

let postIndex;
postIndex = currentUserPosts.findIndex(post => post.id === upsertParams.tempId);
if (postIndex === -1) {
  postIndex = currentUserPosts.findIndex(post => post.id === upsertParams.id);
}
if (postIndex === -1) {
  currentUserPosts.push(upsertParams);
} else {
  currentUserPosts[postIndex] = upsertParams;
}

Complete function:

const upsertIntoApolloCache = (upsertParams) => {
  try {
    const data = client.readQuery({
      query: USER_POSTS_QUERY,
    });
    const currentUserPosts = data.currentUser.posts;

    const postIndex = currentUserPosts.findIndex(
      post => post.id === upsertParams.tempId || post.id === upsertParams.id
    );
    if (postIndex === -1) {
      currentUserPosts.push(upsertParams);
    } else {
      currentUserPosts[postIndex] = upsertParams;
    }

    client.writeQuery({
      query: USER_POSTS_QUERY,
      data,
    });
  } catch (error) {
    console.log('!!ERROR in upsertIntoApolloCache!!', error);
  }
};

You can group the two conditions for finding into a single || condition, since the post.id can match only one of tempId or id.

let postIndex = currentUserPosts.findIndex(post => {
  return post.id === upsertParams.id || post.id === upsertParams.tempId
});

if (postIndex === -1) {
  currentUserPosts.push(upsertParams);
} else {
  currentUserPosts[postIndex] = upsertParams;
}

You could, if possible, also change the currentUserPosts from array to a dictionary mapping your post.id to the post itself. This entirely depends on your further usage of the currentUserPosts across application.

Based on updated question content:

let currentUserPosts = {}
for (let post of data.currentUser.posts) {
  currentUserPosts[post.id] = post
}

This way, you'd only have to check against id using the in operation:

let newId = upsertParams.id || upsertParams.tempId
if (newId in currentUserPosts)
  // do unspeakable things


Shortening function calls with closures I wrote this code that makes a fairly long function call multiple times using some of the same parameters

match key {
    Key { code: KeyCode::Left, .. } => self.map.move_object_by(PLAYER, -1, 0),
    Key { code: KeyCode::Right, .. } => self.map.move_object_by(PLAYER, 1, 0),
    Key { code: KeyCode::Up, .. } => self.map.move_object_by(PLAYER, 0, -1),
    Key { code: KeyCode::Down, .. } => self.map.move_object_by(PLAYER, 0, 1),
}

I was looking for a way to shortened this function call with something simlar to C/C++ macros. I looked into Rust Macros but they are clearly very different. The solution I came up with is creating a function pointer to a closure that calls the function self.map.move_object_by

let mut move_player = |x, y| self.map.move_object_by(PLAYER, x, y);

match key {
    Key { code: KeyCode::Left, .. } => move_player(-1, 0),
    Key { code: KeyCode::Right, .. } => move_player(1, 0),
    Key { code: KeyCode::Up, .. } => move_player(0, -1),
    Key { code: KeyCode::Down, .. } => move_player(0, 1),
}

Is this bad? Will this cause any real slow downs or bugs? Are there other/better ways of doing this?

The Rust compiler will probably inline the code, and generate the exact same code in both situations. So there isn't a performance issue.

However, I think there is a better way to write it:

if let Some((x,y)) = match key.code {
   KeyCode::Left => Some((-1, 0)),
   KeyCode::Right => Some((1, 0)),
   KeyCode::Up => Some((0, -1)),
   KeyCode::Down => Some((0, 1)),
   _ => None
} {
   self.map.move_object_by(PLAYER, x, y);
}


Subtract ordered lists Newbie OCaml review:

Given lists xs and ys that are both ordered in the same way (e.g. monotonically ordered integers), I want to return a list containing all elements of xs not in ys, with order preserved.  I can assume that the elements in ys are a (perhaps improper) subset of xs.

The key is that I have to walk two lists, but they are different lengths and I don't necessarily move through them at the same rate, so I can't use one of the fold2 functions, as far as I can see.

let rec subtract_list xs ys =
  match xs with
  | [] -> []
  | x::more_xs -> match ys with
                  | [] -> xs
                  | y::more_ys ->
                      if x = y
                      then subtract_list more_xs more_ys
                      else x::(subtract_list more_xs ys)

This is the best method I came up with, but I wonder if there's some better strategy that doesn't involve embedding a match inside a match or that's simpler or more idiomatic in some other way.  This version isn't tail recursive, but that's OK.  Comments on indentation or other formatting issues are welcome too.

You can merge all the testing into one match expression:

let rec subtract_list xs ys =
  match xs, ys with
  | [], _ -> []
  | _, [] -> xs
  | x :: xs', y :: ys' when x = y ->
      subtract_list xs' ys'
  | x :: xs', _ ->
      x :: (subtract_list xs' ys)

This also gets rid of the if expression.


Implementing IDisposable correctly to return TCP connection back to pool I'm trying to implement TCP connection pooling and return a connection back to the pool using IDisposable. I'm wondering if my implementation is correct. It seems to be working but I think because the base class also implements IDisposable and finalize, my code might be leaky.

public class BaseClass : IDisposable
{
    internal bool IsDisposed { get; set; }
    private object someResource;

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~BaseClass()
    {
        Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (someResource != null)
        {
            // some clearn up
            return;
        }

        if (disposing)
        {
            //dispose un managed resources

        }
    }
}

public class ChildClass : BaseClass
{
    // adds some functionality
}

public class MyClass : ChildClass, IDisposable
{
    MyPoolManager manager = null;

    public MyClass(MyPoolManager manager)
    {
        this.manager = manager;

    }
    void IDisposable.Dispose()
    {
        manager.ReturnPooledConnection(this);
    }
}

public class MyPoolManager
{
    private static MyPoolManager instance = new MyPoolManager();
    private static object objLock = new object();

    private static Queue<MyClass> que = null;
    private string name;

    static MyPoolManager()
    {
        que = new Queue<MyClass>();
        // enqueue some instances of MyClass here
        MyClass client = new MyClass(instance);
        que.Enqueue(client);
    }

    private MyPoolManager() { }

    public MyPoolManager(string name)
    {
        this.name = name;
    }

    public MyClass GetPooledConnection()
    {
        lock (objLock)
        {
            while (que.Count == 0)
            {
                if (!Monitor.Wait(objLock, 1000))
                    throw new TimeoutException("Connection timeout");
            }
            return que.Dequeue();
        }
    }

    public void ReturnPooledConnection(MyClass client)
    {
        lock (objLock)
        {
            que.Enqueue(client);
            Monitor.Pulse(objLock);
        }
    }
}

Use it like this in your program:

MyPoolManager pool = new MyPoolManager();
using (var conn = pool.GetPooledConnection())
{
    // use the conn here
}

// when you reach here the conn should have returned back to the pool

The days of using IDisposable for things like this only came about because of the lack of functional programming concepts in C#. Now that Action and Func<T> exist there's absolutely no reason to use IDisposable for this situation.

A pool of connections/resources can expose a much nicer and simpler interface that removes the need for the using statement altogether and as a result avoids the possibility of the caller "forgetting" to give the resource back to the pool. Just because something implements IDisposable it doesn't mean that the caller will dispose of it properly.

A few points on your implementation:

Your BaseClass is completely unnecessary and can be removed.
The manager class has a poor implementation of what's already in the .NET framework: ConcurrentQueue. It's not only a better option because you don't have to write it, but it's lock-free and much more efficient.
The manager class's lock forces serialised access to one thread at a time, even if there are plenty of resources available. If you have 30 connections, you should let 30 threads access your resource pool at once, not just 1. Use a Sempahore instead.

So with this in mind I'd like to make a couple of suggestions.

Generalise your resource pool. The resource pool shouldn't be coded to a specific type of thing. Anything of any type can be pooled.
Avoid the use of disposable and use higher order functions to wrap up the lifetime of the resource. The consumer can not possible forget to give the resource back to the pool because you're doing it all for them.
Use proper .NET collections to solve the problem, not your own implementation.
Avoid a straight lock which serialises access when you can instead use a semaphore.
Don't push the logic of resource creation into the pool. Pass in a function that can do the creation for you, then you can have something that can create objects of arbitrary complexity.

Here's a quick implementation I've just thrown together (haven't compiled it, it's for reference and for ideas only):

public class Pool<TResource>
{
  private const int PoolWaitTimeoutMs = 1000;

  private readonly BlockingCollection<TResource> _resources;

  public Pool(int size, Func<TResource> producer)
  {
    _resources = new BlockingCollection<TResource>();

    for(var i = 0; i < size; ++i)
    {
      _resources.Add(producer());
    }
  }

  public void Consume(Action<TResource> consumer)
  {
    // This is a helper for when consumers don't produce a result
    Consume(new Func<TResource, object>(res => { consumer(res); return null; }));
  }

  public TResult Consume(Func<TResource, TResult> consumer)
  {

    TResource res;

    try
    {
      // wait for a resource to become available
      if (!_resource.TryTake(out res, PoolWaitTimeoutMs))
      {
        throw new Exception("Unable to acquire resource");
      }

      // invoke the consumer function and return the result
      return consumer(res);
    }
    finally
    {
      // put it back in the queue if we managed to get one
      if(res != null)
      {
        _resources.Add(res);
      }
    }
  }
}

Using the pool them becomes as simple as this:

public class SomePooledResource
{
  public void DoStuff() { ... };
  public List<string> GetStuff() { ... };
  // other stuff
}

// create a pool of 10 resources
var pool = new Pool<SomePooledResource>(10, () => new SomePooledResource);

// use the resource without returning something
pool.Consume(res => res.DoStuff());

// use the resource returning a result
var results1 = pool.Consume(res => res.GetStuff());

// use the resource to do both
var results2 = pool.Consume(res =>
    {
      res.DoStuff();
      return res.GetStuff();
    });


Two tables for storing comments I have two tables for storing comments

comments

CREATE TABLE `comments` (
 `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
 `text` text NOT NULL,
 `author` int(10) unsigned NOT NULL,
 `time` datetime DEFAULT NULL,
 PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8

comments_ratings

CREATE TABLE `comments_ratings` (
 `comment` int(10) unsigned NOT NULL,
 `user` int(10) unsigned NOT NULL,
 `vote` tinyint(4) NOT NULL,
 PRIMARY KEY (`comment`,`user`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8

users

CREATE TABLE `users` (
 `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
 `email` varchar(255) NOT NULL,
 PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8

The goal is to select comments along with rating and information about the author as well as whether the logged in user has voted or not and if yes their vote's value.

SELECT
c.id,
c.text,
c.time,
c.author,
u.email AS author_name,
SUM(cr.vote) AS rating,
MAX(if(cr.user = ?, cr.vote, NULL)) AS voted

FROM comments c
LEFT JOIN users u ON u.id = c.author
LEFT JOIN comments_ratings cr ON c.id = cr.comment

GROUP BY c.id
ORDER BY c.id DESC

I used to do a second join on comments_ratings in order to get the vote of the logged in user but I recently switched to this thing that I'm not fully sure on to how it works, I just have my guesses. I wrote it with the help of another question I asked and some luck.

Ultimately what I want to ask is, are there any possible issues I can run into with

MAX(if(cr.user = ?, cr.vote, NULL)) AS voted

And I'm asking performance-wise. I know it may not be very easy to read but all I care about now is performance. Is this alright or I should definitely switch back to an additional join on commments_ratings like LEFT JOIN comments_ratings cr2 ON c.id = cr2.comment AND c.author = cr2.user to get the value of vote?

Disclaimer: I work with MSSQL most of the time, so my syntax might be a bit off.

Looking at your code : as you do GROUP BY c.id you will get 1 record per comment. (**) Each line will get the total sum of cr.vote values found for each comment. So far so good.

But then it gets a bit muddy. It seems you want to add the author-information too, but you use a LEFT OUTER JOIN, meaning that you expect/prepare for some comments to have an author id that is not present in the users table?! Is this by design? If not I'd suggest to change the OUTER JOIN into an inner JOIN and additionally to put a FOREIGN KEY from the author field towards the users-id field. This way you prevent 'bad data' and it also tells the RDBMS that there always will be a matching record.

In MSSQL, a SUM() or MAX() on purely NULLs (= what will get returned from the LEFT OUTER JOIN in case no matching comments_ratings records exist) will return a NULL. You seem to try to catch this by using the IF() construction but I'd rather suggest to a 'COALESCE()` with some default value, probably zero.

As such you'd get:

SELECT c.id,
       c.text,
       c.time,
       c.author,
       u.email AS author_name,
       COALESCE(SUM(cr.vote), 0) AS rating,
       COALESCE(MAX(cr.vote), 0) AS voted

  FROM comments c
 JOIN users u ON u.id = c.author
 LEFT OUTER JOIN comments_ratings cr ON c.id = cr.comment
GROUP BY c.id, c.text, c.time, c.author, u.email

**: As you can see I've added all fields that are not aggregated to the GROUP BY clause, in MSSQL this is a requirement for the syntax to be valid; I believe this is not the case for mysql, but I see it as a good habit in all cases. (YMMV =)


Minimum number of coins problem using dynamic programming - follow up I previously posted a code here. It turns out that the code is not following the correct dynamic programming principles and instead it was based on a greedy approach. Hence, I started again, keeping in mind helpful guidelines from here and here. I would now like to know if my code is efficient, or if it can be further optimized in terms of time and space complexity.

#include <stdio.h>
#include <stdlib.h>

int i , no_of_denominations , denominations [ 10000 ] , total_cost , cost [ 10000 ] ;

int min ( int idx )
{
    int min_cost = 99999 , j ;
    cost [ 0 ] = 0 ;
    for ( j = 0 ; j < no_of_denominations ; j++ )
    {
        if( ( i >= denominations [ j ] ) && ( min_cost > cost [ i - denominations [ j ] ] ) )
        {
            min_cost = cost [ i - denominations [ j ] ] ;
        }
    }
    return min_cost ;
}

void input()
{
    printf ( "Coin Change with min no. of coins\nEnter the total change you want: " ) ;
    scanf ( "%d" , &total_cost ) ;

    printf ( "Enter the no. of different denominations of coins available: " ) ;
    scanf ( "%d" , &no_of_denominations ) ;

    printf ( "Enter the different denominations in ascending order: \n" ) ;
    for ( i = 0 ; i < no_of_denominations ; i++ )
    {
        scanf ( "%d" , &denominations [ i ] ) ;
    }
}

void calc_print ()
{
    for ( i = 1 ; i <= total_cost ; i++ )
     {
         cost [ i ] = 1 + min ( i ) ;
     }

     printf ( "min no of coins = %d" , cost [ total_cost ] ) ;
}

int main()
{
    input () ;
    calc_print () ;
    return 0;
}

I advised you that it is called main and not everything, and you did a good job of moving code out of main.  Now main looks a lot closer to what I'd expect to see.

But, we still have functions that are doing too much.  Plus, we've made a bad problem worse.  We've replaced doing everything in the main function with global variables.  And we don't want to do that.

One indication that we have functions that are doing too much is that our functions don't have very great names and there's not a good name to give to them.

For example, void input().  It's not very clear what exactly this function does (from name alone), and we can't really give it a more clear name without it being miles long (and filled with ands... "and" and "or" in function names is generally a code smell).  From the name alone, we'd guess that this function gets input from the user.  But what does it do with that input?  Who knows?  Because this returns void and it doesn't take anything by reference to return either.

So let me repeat the method from earlier more clearly (this you can and should copy-and-paste this exactly and use it):

int getUserInput(const char * prompt) {
    int input;
    printf(prompt);
    scanf("%d", input);
    return input;
}

Do not mistake this.  You're repeatedly getting integer input from the user.  This is a clear indication that you should have a function for doing this.  If you don't have this in your next post, you're doing something very wrong.

The section in which we gather inputs from the user should look basically exactly like this:

int denominationCount = getUserInput("How many different denominations? ");
for (int i = 0; i < denominationCount; ++i) {
    denominations[i] = getUserInput("Enter the next denomination: ");
}
int makingChangeFor = getUserInput("How much are we making change for? ");

And now you've got all the values for which to do the calculation.  You can feed them to the function that does the calculation and get an output from that.  And you don't need the global variables.

Until all of your functions that are accessing the global variables are both taking and returning arguments, you shouldn't be comfortable using the global variables (as a rough guideline).  Global variables are almost always unnecessary.  Global variables are the path to rats nests, madness, and lunacy.


Return correct path if windows or linux I have been trying to code something simple where I call a class with different functions to different paths,
if I call etc:
from lib.utils import GetAllPaths

path = GetAllPaths()
path.test()

with following code:
class GetAllPaths:
    """
    Return file and paths directions
    """

    if platform == "win32":

        def test(self):
            return "C:\\Users\\test.py"

        def test2(self):
            return "C:\\Users\\test2.py"

        def test_flag(self):
            # new_manual_url.flag
            return "C:\\Users\\*test.flag"

        def kill_flag(self):
            # kill_*.txt
            return "C:\\Users\\*kill_*.flag"

        def restart(self):
            # start_*.txt
            return "C:\\Users\\*start_*.flag"

    elif platform == "linux" or platform == "linux2":

        def test(self):
            return "/home/test.py"

        def test2(self):
            return "/home/test2.py"

        def test_flag(self):
            return "/home/*test.flag"

        def kill_flag(self):
            return "/home/*kill_*.flag"

        def restart(self):
            return "/home/*start_*.flag"

My problem here is that I run sometimes my code through Linux and sometimes through Windows, so I have created a if else statement if the script is ran through Linux or Windows,
however I do see huge potential to short this code by alot but I don't know how to apply it where I can choose etc if its linux then use the correct path of "test" for linux or if its windows then use the correct path of "test" for windows

I think you can lean on a class-variable to create your base paths, and then define the functions to use that variable:
import os
from sys import platform

class GetAllPaths:
    """
    Return file and paths directions
    """

    if platform == "win32":
        root = 'C:\\Users'
    elif platform == 'linux':
        root = '/home'
    else:
        # Probably raise an exception here
        raise Exception(f"Platform {platform} not supported")

    def test(self):
        return os.path.join(self.root, "test.py")

    def test2(self):
        return os.path.join(self.root, "test2.py")

    def test_flag(self):
        # new_manual_url.flag
        return os.path.join(self.root, "*test.flag")

    def kill_flag(self):
        # kill_*.txt
        return os.path.join(self.root, "*kill_*.flag")

    def restart(self):
        # start_*.txt
        return os.path.join(self.root, "*start_*.flag")

Furthermore, you'll notice that I'm using os.path.join, this way you don't have to worry about hard-coding the path separators.
Even more DRY would be to use a decorator on each of these functions which cuts down on repeated code:
import os
from functools import wraps
from sys import platform

class GetAllPaths:
    if platform == "win32":
        root = 'C:\\Users'
    elif platform == 'linux':
        root = '/home'
    else:
        # Probably raise an exception here
        raise Exception(f"Platform {platform} not supported")

    def _add_platform(f):
        @wraps(f)
        def wrapper(self):
            path = f(self)
            return os.path.join(self.root, path)
        return wrapper

    @_add_platform
    def test(self):
        return "test.py"

    @_add_platform
    def test2(self):
        return "test2.py"

    @_add_platform
    def test_flag(self):
        # new_manual_url.flag
        return "*test.flag"

    @_add_platform
    def kill_flag(self):
        # kill_*.txt
        return "*kill_*.flag"

    @_add_platform
    def restart(self):
        # start_*.txt
        return "*start_*.flag"

Ditch the class
Realistically, you don't need the class here. You can identify this by looking at how your functions are utilized. There's not any class or instance specific information being passed to your functions (decorator notwithstanding). This means that your class and a module are fairly indistinguishable. To see this without a class, you might put all of this code into a new file, say get_all_paths.py:
# get_all_paths.py
import os
from functools import wraps
from sys import platform

if platform == "win32":
    root = 'C:\\Users'
elif platform == 'linux':
    root = '/home'
else:
    # Probably raise an exception here
    raise Exception(f"Platform {platform} not supported")

def add_platform(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        path = f(*args, **kwargs)
        return os.path.join(root, path)
    return wrapper

@add_platform
def test():
    return "test.py"

@add_platform
def test2():
    return "test2.py"

@add_platform
def test_flag():
    # new_manual_url.flag
    return "*test.flag"

@add_platform
def kill_flag():
    # kill_*.txt
    return "*kill_*.flag"

@add_platform
def restart():
    # start_*.txt
    return "*start_*.flag"

Where now, you can use your functions through a global import:
import get_all_paths as g

g.test()
'/home/test.py'
```


Tic Tac Toe in C - Version 1 This was the first game I made not too long ago (2018), and while I'm working on another, more complex version, I want to know how good or bad I did this one and what can be improved.

Note: the change of players and detection of free spaces are the things I did with help of a friend, everything else I did it myself.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

int printBoard();
int clear(int h, int w);
int freeSpace();
int player();
int checkLine(int x, int y, int z);
int winner();

int player_input;
char board[9] = {' ', ' ', ' ',
                 ' ', ' ', ' ',
                 ' ', ' ', ' '};

int player_num = 1;
char symbol = 'X';

int main(void)
{
    char start;

    clear(24, 80);

    printf("\nTotito game on a 3x3 board:\n");
    printf("Enter number from 1 to 9 to mark the respective spaces in the board.\n");
    printf("Press a number to start the game: ");
    scanf("%c", &start);

    clear(24, 80);
    printBoard();

    while (true)
    {
        player();
        clear(24, 80);
        printBoard();
        winner();

        player_num = (player_num == 1)? 2: 1;
        symbol = (player_num == 1)? 'X': 'O';
    }
}

int printBoard() // 3x3 board
{
    int lines = 7;
    int columns = 7;
    int n = 0;

    for (int i = 1; i <= lines; i++)
    {
        printf("    ");

        for (int j = 0; j < columns; j++)
        {
            if (i % 2 != 0)
            {
                if (j % 2 == 0)
                {
                    printf("+");
                }

                else
                {
                    printf("---");
                }
            }

            else
            {
                if (j % 2 == 0)
                {
                    printf("|");
                }

                else
                {
                    printf(" %c ", board[n]);
                    n++;
                }
            }
        }

        printf("\n");
    }
}

int clear(int h, int w)
{
    printf("\033[0;0H");

    for (int i = 0; i < w; i++)
    {
        for (int j = 0; j < h; j++)
        {
            printf(" ");
        }
    }

    printf("\033[0;0H");
}

int freeSpace()
{
    for (int i = 0; i < sizeof(board); i++)
    {
        if (board[i] == ' ')
        {
            return true;
        }
    }
}

int player()
{
    printf("Player %d: ", player_num);

    if (freeSpace())
    {
        scanf("%d", &player_input);
    }

    while (board[player_input - 1] != ' ')
    {
        printf("\033[1APlayer %d: Not empty, try again: ", player_num);
        scanf("%d", &player_input);
    }

    board[player_input - 1] = symbol;
}

int checkLine(int x, int y, int z)
{
    if ((board[x] == symbol) && (board[x] == board[y]) && (board[y] == board[z]))
    {
        printf("Player %d wins!\n", player_num);
        exit(0);
    }
}

int winner()
{
    checkLine(0, 1, 2); //first horizontal

    checkLine(3, 4, 5); //second horizontal

    checkLine(6, 7, 8); //third horizontal

    checkLine(0, 3, 6); //first vertical

    checkLine(1, 4, 7); //second vertical

    checkLine(2, 5, 8); //third vertical

    checkLine(0, 4, 8); //first diagonal

    checkLine(2, 4, 6); //second diagonal
}

Compiler Options and Warning Checking
It might be a good idea to use the -wall compiler switch. It will provide warning messages that may indicate possible logic errors in some cases. When compiled with -wall this program yields the following warning messages:

D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(89) : warning C4716: 'printBoard': must return a value
D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(104) : warning C4716: 'clear': must return a value
D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(115) : warning C4715: 'freeSpace': not all control paths return a value
D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(133) : warning C4716: 'player': must return a value
D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(142) : warning C4715: 'checkLine': not all control paths return a value
D:\ProjectsNfwsi\CodeReview\tictactoev1\main.c(161) : warning C4716: 'winner': must return a value

In some cases the functions should be declared void rather than int, in other cases there might be a bug when not all control paths return a value, this is certainly true in freeSpace.
Global Variables
Global variables make programs very, very difficult to write, read, debug and maintain. In the C programming language they can cause modules (other .c files) not link if the global variable is declared in multiple modules. Due to the nature of global variables, it is sometimes very difficult to find where they are changed in order to remove a bug.
There is a discussion about global variables in this stackoverflow question
It is much better to pass a variables into functions where they are needed.
Avoid Using exit()
For a number of reasons the use of the exit() function should be avoided. The exit() function should be used when there is a non-recoverable error in a program. Since it is not being used for this purpose in this program it should definitely be avoided. It would be much better if the functions checkLine() and winner() returned values that indicated status.


Project Euler #1 - Multiples of 3 and 5 - First Program Taking a problem from the website, "Project Euler #1", I have created my first C# program. The problem I have used is as follows:

If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.
Find the sum of all the multiples of 3 or 5 below 1000.

The program I have made:
using System;

namespace _5_multiples
{
    public class MainClass
    {
        public static void Main()
        {
            int currentNumber = 1000;
            int allSum = 0;

            currentNumber = currentNumber - 1;
            while (currentNumber > 0)
            {
                if (currentNumber % 3 == 0)
                {
                    allSum = allSum + currentNumber;
                }
                else
                {
                    if (currentNumber % 5 == 0)
                    {
                        allSum = allSum + currentNumber;
                    }
                }
                currentNumber = currentNumber - 1;
            }
            Console.WriteLine(allSum);
        }
    }
}

I understand there is probably a lot to improve - this is my first C# program after all. I did run into a few problems such as referencing non static members from a static context and no suitable entry method. Rather than finding a definitive fix, the solution I implemented was more of a workaround; hence why I feel there may be a lot to improve.
namespace _5_multiples - this doesn't look right? Name of the program is actually 3_5_multiples - perhaps a badly chosen name?
Anyway, please let me know on anything that could be improved.

Here's what I would change:

I'd use a for loop instead of a while loop here: for (int currentNumber = 999; currentNumber > 0; currentNumber--). This keeps all loop-related things grouped together. Loop bodies can be skipped or broken out of, so if code that must be executed every cycle is put at the end of a loop body it's easily broken when the loop body grows more complex and someone adds a continue or break in there.
Actually, there is no need to count down - the result will be the same either way. Counting up is often a little easier to understand: for (int currentNumber = 0; currentNumber < 1000; currentNumber++). Note that using < here means you don't need to decrement currentNumber by one before the loop starts.
You can write a += b instead of a = a + b (the same goes for various other operators).
else { if { .. } } can be simplified to else if { ... }.
Though in this case, a single if statement is sufficient: if (currentNumber % 3 == 0 || currentNumber % 5 == 0) { ... }. The 'or' operator (||) short-circuits if the first expression is true, so if a number can be divided by 3 it won't also try to divide it by 5.
C# naming conventions typically write class, method and namespace names in PascalCase.

Regarding your questions:

Static things belong to a class, non-static things belong to instances of that class. Classes are to instances what blueprints are to actual buildings. Trying to access a non-static (instance) members from a static context is like trying to open a door on a blueprint. The blueprint doesn't contain any real doors, only actual buildings do.
Programs need a place to start executing, and in C# the starting point is a static Main method. Putting everything in there is fine for a small program like this.
Identifier names in C# cannot start with a number, so 3_5_multiples isn't a valid name. I'd probably use EulerProblem1 or something like that here.
Namespaces are useful for organizing larger programs (namespaces to classes are a bit like folders to files). The root namespace of a project is usually the name of that project itself, and child namespaces often correspond to folders.

EDIT: Part of the fun on Project Euler is coming up with more efficient solutions. Can you find a way to solve this without checking every number below 1000?


Merge k sorted lists in Java I hava a question similiar with mege in Python but I am trying to write Java code.
Here is my code:
The input is: The first line - a number of arrays (k); Each next line - the first number is the array size, next numbers are elements.

Max k is 1024. Max array size is 10*k. All numbers between 0 and 100. Memory limit - 10MB, time limit - 1s. Recommended complexity is k ⋅ log(k) ⋅ n, where n is an array length.

Example input:

4
6 2 26 64 88 96 96
4 8 20 65 86
7 1 4 16 42 58 61 69
1 84

Example output:

1 2 4 8 16 20 26 42 58 61 64 65 69 84 86 88 96 96

I get memory limit errors.
I use System.gc() to clear the memory but I recieve timeout error

Here is my code:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
public class TaskFSolution {
    public static void main(String[] args) throws IOException {
        BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));

        int k = Integer.valueOf(reader.readLine());

        short[][] arrays = new short[k][];

        int arrayLength = 0;
        int[] minPositionInArrays = new int[k];
        for (int i = 0; i < k; i++) {
            String str = reader.readLine();
            String[] elements = str.split(" ");
            arrays[i] = new short[Short.valueOf(elements[0]) + 1];
            for (int j = 0; j < elements.length; j++) {
                arrays[i][j] = Short.valueOf(elements[j]);
            }
            arrayLength += Short.valueOf(elements[0]);
            minPositionInArrays[i] = 1;

            if (arrayLength % 100 == 0) {
                System.gc();
            }
        }

        reader.close();

        StringBuilder sb = new StringBuilder();
        int currentArrayNumber = 0;
        int currentPositionInArray = minPositionInArrays[currentArrayNumber];
        while (arrayLength > 0) {
            short currentMinValue = 101;
            for (int i = 0; i < k; i++) {
                if (minPositionInArrays[i] <= arrays[i][0] && currentMinValue >= arrays[i][minPositionInArrays[i]]) {
                    currentMinValue = arrays[i][minPositionInArrays[i]];
                    currentArrayNumber = i;
                    currentPositionInArray = minPositionInArrays[i] + 1;
                }
            }
            sb.append(currentMinValue + " ");
            minPositionInArrays[currentArrayNumber] = currentPositionInArray;
            arrayLength--;
        }

        System.out.println(sb.toString().trim());
    }
}

Are you getting the correct output with the recommended complexity? It's been a long time since I've written my own sort but start by confirming that before any of my recommendations below.

I suspect str.split(" ") is causing problems for you because you're creating lots of strings which you're about to parse and then throw away. How about parsing the integers directly from str in a single pass instead?

Try changing sb.append(currentMinValue + " ") to sb.append(currentMinValue).append(" "). It might be a small optimization but it's easy to do.

How about splitting your main into two methods? The first for everything that uses reader, the second for everything that uses sb. Besides improving the structure of the program this provides a natural point for garbage collection.

What command-line args are you giving to the JVM? Since you know your memory limit try allocating it all at the start to save time allocating more memory later. You might also play with different garbage collection strategies. As you're discovering, forcing garbage collection isn't a good idea. If you're correctly setting the maximum in the JVM it should automatically garbage collect as needed to prevent running out of memory.

Hope at least one of these suggestions helps. I'm curious what gives the biggest improvement.


Make two random enemies shoot projectiles every second Just finished my canvas game that is more or less replica of the old school arcade game "Space Invaders".
The code below chooses two random enemies every second (approximately) from the enemies array to shoot projectiles. If there are two or more enemies left in the array I want two random enemies to shoot every second and if there is only one enemy left only that enemy will shoot every second.
I have two solutions for this problem. Both of them work. However the first one seems quite unnecessary repetitive and the second solution although short and elegant is probably unnecessary more "expensive". Keep in mind I am beginner and I might be wrong here with my assumptions and conclusions.
First solution:
let req;
let timer = 0
function animate() {
  req = requestAnimationFrame(animate);
  ctx.clearRect(0, 0, canvas.width, canvas.height);
  // other game logic

  // two random enemies shoot every second
  if (timer % 60 === 0 && timer !== 0) {
    let enemiesLength = enemies.length;
    if (enemiesLength > 2) {
      let rndIndex_1;
      let rndIndex_2;
      rndIndex_1 = Math.floor(Math.random() * enemiesLength);
      do {
        rndIndex_2 = Math.floor(Math.random() * enemiesLength)
      } while (rndIndex_1 == rndIndex_2);
      enemies[rndIndex_1].shootProjectile();
      enemies[rndIndex_2].shootProjectile();
    } else if (enemiesLength === 2) {
      enemies[0].shootProjectile();
      enemies[1].shootProjectile();
    } else if (enemiesLength === 1) {
      enemies[0].shootProjectile();
    }
  }

  // other game logic
  timer++
}

animate();

Second solution:
let req;
let timer = 0
function animate() {
  req = requestAnimationFrame(animate);
  ctx.clearRect(0, 0, canvas.width, canvas.height);
  // other game logic

  // two random enemies shoot every second
  if (timer % 60 === 0 && timer !== 0) {
    enemies.sort(() => 0.5 - Math.random()).slice(0, 2).forEach(e => {
      e.shootProjectile();
    });
  }

  // other game logic
  timer++
}

animate();

Any other solutions and help is much appreciated.
EDIT:
In response to CertainPerformance answer. I changed the answer to this so it would work:
   if (timer % 60 === 0 && timer !== 0) {
    const enemiesCopy = [...enemies];
    const enemiesToShoot = [];

    while (enemiesToShoot.length < 2 && enemiesCopy.length) {
        const [enemy] = enemiesCopy.splice(Math.floor(Math.random() * enemiesCopy.length), 1);
        enemiesToShoot.push(enemy);
    }
    for (const enemy of enemiesToShoot) {
        enemy.shootProjectile();
    }
}

It works fine like this. Is this how it should be? Some clarification please.

Games
Performance
Writing games using JavaScript presents some unique problems.

Jank and slowdown due to memory management. Though there have been vast improvements to what was the norm just a few years back, memory management remains a major point of performance degradation.
Games must always be aware of the issues of memory management. Many common pattern used in JavaScript are not suitable for games.

Platform variance. JavaScript games will run a vast range of platforms, from low end hand held devices, to power hungry overclocked water cooled desktop towers. The average consumer's device is generally of lower spec than the average developer's device.
The success of a browser based game is highly dependent on social networking, a game that does not have a strong focus on performance will not be as successful. Even minor performance problems can make the game unplayable on many more devices than would be  expected on a linear curve as the majority of devices are to the lower end of capability.
Bad reviews are worse than good in a market dominated by word of mouth (social networking) Poor performance is a major driver of bad reviews.

To get the best performance you must get a good understanding of what is slow and what is fast. This is true right down to the most trivial operations need to consider performance.
A look at performance.
Arrays are slow.
Creating and populating arrays are slow due to many factors.

Allocation on heap
Iteration to fill
De-allocation at end of execution (is scoped to execution content) or as a GC deferred slowdown if part of scope that is not released at execution idle.

The following snippet shows 3 examples, two from your question and a 3rd I created,
To ensure that the optimizer does not remove code that has no side effects the enemy shoot function returns a random value that is added to a global soak. All 3 methods use the same enemy, enemies and soak.
This test concentrates only on the shooting ignoring the time test.

    const enemyCount = 60;
    const enemy = { shootProjectile(){ return Math.random() }  }
    const enemies = $setOf(enemyCount,() => ({...enemy}));

    var soak = 0;
    function testA() {
        enemies.sort(() => 0.5 - Math.random()).slice(0, 2).forEach(e => {
          soak += e.shootProjectile();
    }

    function testB() {
        const enemiesCopy = [...enemies];
        const enemiesToShoot = [];

        while (enemiesToShoot.length < 2 && enemiesCopy.length) {
            const [enemy] = enemiesCopy.splice(Math.floor(Math.random() * enemiesCopy.length), 1);
            enemiesToShoot.push(enemy);
        }
        for (const enemy of enemiesToShoot) {
            soak += enemy.shootProjectile();
        }
    }

    function testC() {
        const shooterIdx = Math.random() * enemies.length | 0;
        var shooterIdx1 = Math.random() * enemies.length | 0;
        if (shooterIdx1 === shooterIdx) { shooterIdx1 = (shooterIdx1 + 1) % enemies.length }
        soak += enemies[shooterIdx].shootProjectile();
        soak += enemies[shooterIdx1].shootProjectile();
    }

Shooters: 60. Shots per call: 2

Test Function
Mean time
Max diff
OPS*
Relative Performance *1

TestA
31.929µs
±17.298µs
31,319
1%

TestB
0.609µs
±0.604µs
1,642,819
27%

TestC
0.166µs
±0.226µs
6,025,389
100%

µs millisecond. 1 millionth of second.
(*) OPS is calculated operations per second, where the function call is considered the operation
(*1) Percentage of best

As higher enemy counts will negatively effect the outcome of testA and testB and lower enemy counts will negatively effect the outcome of testC the following table is with an enemy count of 6
Shooters: 6. Shots per call: 2

Test Function
Mean time
Max diff
OPS
Relative Performance

TestA
1.208µs
±0.284µs
828,136
14%

TestB
0.490µs
±0.123µs
2,039,537
34%

TestC
0.168µs
±0.266µs
5,959,047
100%

Game compromises
I have been in the games industry for a long time and one thing I have learnt is that pushing for ideal implementations will hurt performance, while compromising in favor of performance goes completely unnoticed.
Selecting many from many
There is another factor that effects the performance of the code and that is the number of shooters. As function textC can only handle 2 shooters at a time we must rewrite it to work with any number of shooters.
At this point we make a sacrifice. Finding shooters randomly means that we need to test against shooter firing at the same time. This overhead should be ignored, rather we flag shooters that get to shoot so that we do not shoot twice.
The compromise
The function TestC will shoot about 7% less than the others giving it a slight advantage, but that advantage pales when compared to the 3* faster than testB and a huge 30* faster than testA
The updated code

const enemyCount = 60;
const enemyShoot = 10;
const enemy = {
    shoot: 0,
    shootProjectile(){ return Math.random() }
}
const enemies = $setOf(enemyCount,() => ({...enemy}));

var soak = 0;
var time = 0;
function testA() {
    enemies.sort(() => 0.5 - Math.random()).slice(0, enemyShoot).forEach(e => {
      soak += e.shootProjectile();
    });
    time ++;
}

function testB() {
    const enemiesCopy = [...enemies];
    const enemiesToShoot = [];
    while (enemiesToShoot.length < enemyShoot && enemiesCopy.length) {
        const [enemy] = enemiesCopy.splice(Math.floor(Math.random() * enemiesCopy.length), 1);
        enemiesToShoot.push(enemy);
    }
    for (const enemy of enemiesToShoot) {
        soak += enemy.shootProjectile();
    }
    time ++;
}

function testC() {
    var count = enemyShoot + 1;
    while (count--) {
        const shooter = enemies[Math.random() * enemies.length | 0];
        soak += shooter.shoot !== time ? (shooter.shoot = time, shooter.shootProjectile()) : 0;
    }
    time ++;
}

Shooters: 60. Shots per call: 10

Test Function
Mean time
Max diff
OPS
Relative Performance

TestA
26.669µs
±8.919µs
37,497
3%

TestB
2.293µs
±1.023µs
436,149
33%

TestC
0.762µs
±0.834µs
1,313,125
100%

Arrays the outcome
As you can see the results show that using arrays in this case to select items to perform and action is much slower than avoiding the need to hold shooters.
With that said arrays are not bad if used correctly.
Part of the performance penalty of arrays is the allocation and de allocation process.
As memory is currently very cheap compared to CPU cycles you can leave arrays populated and keep a separate counter to track how many active items it holds. This eliminates the need to allocate and de-allocate arrays each frame.
The overhead of iteration
Each time you iterate an array you add the overhead of the loop logic. A general rule is iterate twice only, once for update, once for display. Actions that you would normally do in a separate loop should be done in the update loop.
In this case as you would already be iterating each enemy to move them, this is where the logic to select a shooter should be.
I would not try for 2 shooters every 1 seconds, I would give all shooters odds to shoot that are equivalent. So if there are 60 enemy at 2 shoots total every 1 second the odds for any shooter shooting per frame is 2 / (60 * 60) = 5.556e-4
Thus in the main enemy loop you add Math.random() < 5.556e-4 && enemy.shootProjectile(); complete eliminating the extra code finding shooters, and in my view creating a far more realistic AI.
 All tests on Chrome 87. 64Bit i9 windows laptop, cooling set to active.


Using another function on modified dictionary This is based upon an exercise in chapter 5 of Automate the Boring Stuff with Python.

This code contains functions addToInventory which modifies a given inventory of items to add new items as well as update  existing items and displayInventory which displays the items and their values. The main operation is adding the contents of a list dragonloot to inventory inv.

I have tried to do this in two ways, one by calling addToInventory and after that calling displayInventory. This method leads to errors. Another way I tried was by printing the new inventory values within addtoInventory. This works fine. Why doesn't the first method work? I have put both the methods as comments within my code, indicated by """ """.

I know C++ pretty well, but I'm new to Python. Expected result:
Inventory:
gold coin: 45 rope:1 dagger:1 ruby:1 Total number of items is 48

def addToInventory(inventory, addedItems):
    draft = {}
    for i in addedItems:
        if (i not in draft):
            draft[i]=1
        else:
            draft [i]= draft [i] + 1
    for i in draft:
        if i not in inventory:
            inventory[i]=draft[i]
        else:
            inventory[i] = inventory[i]+draft[i]
    """for i in inventory:
        print(str(inventory[i]) + " " + str(i))"""

def displayInventory(inventory):
    print ("Inventory: ")
    total=0
    for i in inventory.items():
        total+=1
        print (str(inventory[i])+" "+str(i))
    print("Total number of items is"+str(total))

    inv = {'gold coin': 42, 'rope': 1}
    dragonLoot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
    inv = addToInventory(inv, dragonLoot)
    """displayInventory(inv) when I use this line in place of above comment I get errors"""

You overwrite inv with the return value of addToInventory

inv = addToInventory(inv, dragonLoot)

Yet add to inventory does not return anything.
In order to first get your code to work, just return the inventory:

def addToInventory(inventory, addedItems):
    draft = {}
    for i in addedItems:
        if (i not in draft):
            draft[i]=1
        else:
            draft [i]= draft [i] + 1
    for i in draft:
        if i not in inventory:
            inventory[i]=draft[i]
        else:
            inventory[i] = inventory[i]+draft[i]
    return inventory

And then lets improve that function using the collections library such as:

from collections import Counter

def addToInventory(inv, loot):
    loot_counter = Counter(loot)
    return dict(loot_counter + Counter(inv))

Further, for the second displayInventory function, you iterate over dict.items(), yet you then try to use the item itself as an index. Instead iterate over dict.keys() and you will be fine:

def displayInventory(inventory):
    print ("Inventory: ")
    total=0
    for i in inventory.keys():
        total+=1
        print (str(inventory[i])+" "+str(i))
    print("Total number of items is"+str(total))


Speeding up a length calculation for a QGIS plugin In a QGIS 2.0 plugin for windows, I calculate length and update a field with the values. It works, but takes a lot of time. Execution for this part only takes about ten minutes. Can anyone suggest anything to speed things up?

def calculatelength(self):
    idx = self.resLayer.fieldNameIndex('Longueur')
    caps = self.resLayer.dataProvider().capabilities()
    features = self.resLayer.getFeatures()
    for feature in features:
        if caps & QgsVectorDataProvider.ChangeAttributeValues:
            fid = feature.id()
            flen = feature.geometry().length()
            attrs = {idx : flen}
            self.resLayer.dataProvider().changeAttributeValues({fid : attrs})

I'm not familiar with the libraries in question, but it seems to me that you could take a couple of things out from the for loop:

The if condition does not look like it would change between iterations.
As changeAttributeValues takes a dictionary argument, I would expect it to handle a dictionary with multiple keys as well. Construct a dict with all fid : attrs pairs in the loop and make just one function call after.

My proposal:

def calculatelength(self):
    idx = self.resLayer.fieldNameIndex('Longueur')
    caps = self.resLayer.dataProvider().capabilities()
    if caps & QgsVectorDataProvider.ChangeAttributeValues:
        features = self.resLayer.getFeatures()
        feature_attrs = {}
        for feature in features:
            fid = feature.id()
            flen = feature.geometry().length()
            feature_attrs[fid] = {idx : flen}
        self.resLayer.dataProvider().changeAttributeValues(feature_attrs)


C solution to "99 bottles" kata I typically use high level programming languages like C# and JavaScript, but today I was doing the "99 bottles of beer" kata for an event using C. Below is my solution, which seems to work. Please let me know if there's any egregious mistakes or non-idiomatic pieces in there?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXBOTTLES 9999

#define MAX_NR_OF_DIGIT_OCCURENCES 2 // Max nr of placeholders in any of the sentence formats
#define SENTENCE_FORMAT_BASE "%i bottles of beer on the wall, %i bottles of beer\nTake one down, pass it around, %i bottles of beer on the wall...\n"
#define SENTENCE_FORMAT_1 "1 bottle of beer on the wall, 1 bottle of beer\nTake one down, pass it around, no more bottles of beer on the wall...\n"
#define SENTENCE_FORMAT_0 "No more bottles of beer on the wall, no more bottles of beer.\nGo to the store and buy some more, %i bottles of beer on the wall..."

int getNumberOfDigits(int number) {
    int numberWidth = 0;

    while (number) {
        numberWidth++;
        number /= 10;
    }

    return numberWidth;
}

char* createCouplet(const int i, char* format) {
    int length = getNumberOfDigits(i) * MAX_NR_OF_DIGIT_OCCURENCES + strlen(format);
    char* coupletBuffer = malloc(length * sizeof(char));

    sprintf(coupletBuffer, format, i, i, i-1);

    return coupletBuffer;
}

int main(int argc, char *argv[]) {
    int songSize = 13; // TODO: Retrieve from argv
    int limit = songSize + 1;

    if (songSize >= MAXBOTTLES) {
        return 42;
    }

    char* song[limit];

    for (int i = 2; i < limit; i++) {
        song[limit - i - 1] = createCouplet(i, SENTENCE_FORMAT_BASE);
    }

    song[limit - 2] = malloc(2500 * sizeof(char));
    strcpy(song[limit - 2], SENTENCE_FORMAT_1);

    song[limit - 1] = createCouplet(songSize, SENTENCE_FORMAT_0);

    for (int i = 0; i < limit; i++) {
        printf("%s\n", song[i]);
        free(song[i]);
    }

    return 0;
}

I welcome any feedback of course, but I'm particularly worried that I do silly things around buffers and handling memory correctly.

Logic

I find it counterintuitive that your main loop doesn't look like this:

for (int bottles = limit; bottles >= 0; bottles--) {
    …
}

The way you build the song backwards (with some index arithmetic), followed by two special cases, makes the code hard to follow.

You have a pluralization bug:

2 bottles of beer on the wall, 2 bottles of beer
Take one down, pass it around, 1 bottles of beer on the wall...

You used a special case for SENTENCE_FORMAT_1. Because the text happens to be constant, you call malloc() and strcpy() instead of createCouplet().  I don't recommend doing anything so drastically different. In fact, createCouplet() should be made responsible for choosing the right template to use based on the bottle count, such that the function "just works" for any i.

Memory management

I don't understand the appeal of storing all of the verses in a variable-length array. If you want to have the entire song at once, it would be better to build one huge contiguous string. Alternatively, if you are willing to write a loop that prints each verse (like my countdown loop above), then you don't even need to have the entire song in memory at any given time; you can just work one verse at a time.

In particular, the one-verse-at-a-time approach gives you the option to use one buffer, whose contents are overwritten repeatedly. I would just use a static buffer. In general any design in C that avoids malloc() altogether is highly appealing.

I'm not a fan of the technique you used to calculate the length of the output. It's a very manual process, requiring you to tell it how many numbers appear in the template, and calculating how many digits should be rendered. Rather, what you could do is take advantage of the return value of snprintf(), which tells you how long the output should be — and that technique works in general. (The whole issue would be a moot point with a static buffer: you just generously estimate the required space for any verse.)

Suggested design

  int bottleVerse(int bottles, int init_bottles, char *buf, size_t bufsize)

  This function generates one verse from the song 99 Bottles of Beer on the Wall, writing it to the given buffer.  The output ends with a newline and a NUL terminator.

  Parameters

  bottles: The number of bottles of beer at the start of the verse
  init_bottles: The initial number of bottles (traditionally 99)
  buf: Pointer to a buffer to which the output should be written (possibly NULL, to obtain the output length without writing the output anywhere).
  bufsize: The size of the buffer

  Return value

  On success, a positive value is returned, indicating the length of the string that was written to the buffer.  A negative value indicates the number of bytes by which the buffer is undersized.

This design mimics the pattern followed by many functions in the C library.  The caller is responsible for providing the buffer and its size; the function merely fills it in.  The return value acts as a status indicator.

The memory management is very in-your-face.  It seems more inconvenient to use than your createCouplet().  However, it has many advantages:

This function obviously allocates no memory.  It's very clear what the memory ownership policy is: the caller provides and "owns" the memory.
The caller has the flexibility to provide several kinds of memory:

It could be a static buffer, reused between verses.
The caller could malloc() a buffer.
The caller could append this verse to the previous verse in one huge buffer for the song.

It can avoid some drawbacks of malloc() and free():

malloc() is a mildly expensive operation.
malloc() might allocate memory in non-consecutive chunks, reducing the efficiency of the memory cache.
Allocating many small chunks of memory with malloc() can lead to memory fragmentation, wasting memory.
Forgetting to free() a newly allocated string would cause a memory leak.
Accidentally using memory that has already been free()d could cause a security vulnerability.

Many of these issues are negligible concerns for a tiny program like this.  However, if many functions in a large program were designed to use malloc(), then you would have a chaotic mess.  For these reasons, idiomatic C functions generally have this kind of design where the caller provides the memory.

Here is an example of what the code might look like if you wanted to compose the entire song as one long string.  Using a static buffer to print one verse at a time would result in a slightly simpler program.

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>

// Left as an implementation exercise for you, using snprintf()
int bottleVerse(int bottles, int init_bottles, char *buf, size_t bufsize);

int main() {
    const int initBottles = 99;
    char *song, *verse;

    // Overestimate the length by one verse
    // (better than underestimating by one byte!)
    int estimatedVerseLen = -bottleVerse(initBottles, initBottles, NULL, 0);
    int bufsize = estimatedVerseLen * (initBottles + 2) + 1;

    if (NULL == (verse = song = malloc(bufsize))) {
        return 1;
    }
    for (int bottles = initBottles; bottles >= 0; bottles--) {
        int len = bottleVerse(bottles, initBottles, verse, bufsize);
        assert(len >= 0);
        verse += len;
        bufsize -= len;
    }
    fputs(song, stdout);
    free(song);
}


Program to test for divisibility of numbers (EDIT: here is the follow up qusetion)

Using this program to test for the smallest number where its permutations or itself is divisible by 10 or less numbers is twice as slow as the fastest program I found doing this (I don't have the source code). How can this program be twice as slow, what optimizations can I do without messing with the compiler flags?

#include <stdio.h>
#include <string.h>
#define MAX 1000000

int main() {
  int N,p[10],n[100];
  int mask[MAX];
  register int i, j, k;

  scanf("%d", &N);
  for(i=0;i<N;i++) scanf("%d", &n[i]);
  memset(mask,0,sizeof(mask));
  i = MAX+1;
  while(--i) {
    k=i;
    p[0]=0;
    p[1]=0;
    p[2]=0;
    p[3]=0;
    p[4]=0;
    p[5]=0;
    p[6]=0;
    p[7]=0;
    p[8]=0;
    p[9]=0;
    while (k>0) {p[k%10]++; k/=10;}

    for(j=1;j<10;j++) if(p[j]>0) {k=j; p[j]--; break;}
    for(j=0;j<10;j++) while (p[j]>0) {k=10*k+j; p[j]--;}
    for(j=0;j<N;j++) if(i%n[j]==0) mask[k]|=(1<<j);
  }
  for(i=1;i<MAX;i++) if(mask[i]==(1<<N)-1) break;
  fprintf(stdout, "%d\n", i);
}

This program uses bitmasks to test all numbers between 1 and MAX for divisibility with the given numbers and then prints out the smallest number whose permutations or itself is divisible by all given numbers. It is not fast enough!

Example input

7
164 278 293 382 483 598 23

This will test all numbers and check which ones are divisible by the given numbers and the output should be 102246.

It is compiled using the GCC compiler with the following flags:

-g -O2 -std=gnu99 -static -lm

Test the code here.

for loop

i = MAX+1;
while(--i) {

This should be converted to a for loop, it will become more readable.

Smallest scope possible

Maybe in this challenge C99 is not an option, but I would like to reccomend declaring variables in loop declarations, like:

for(int i=1;i<MAX;i++)

Avoiding repetition

Please, avoid repetion:

p[0]=0;
p[1]=0;
p[2]=0;
p[3]=0;
p[4]=0;
p[5]=0;
p[6]=0;
p[7]=0;
p[8]=0;
p[9]=0;

Should be:

for (int i = 0; i <= 9; i++) {
    p[i] = 0;
}

and it becomes so much clearer (think what would have happened if you had to set 100 of such items).


Summations and products in F# I was bored, and looking to do something that involved anonymous functions, and it was suggested by @Quill, that I create a summation function. I decided to also include a product function as well to top the whole thing off.

For those who don't know, a summation is usually defined like this:

  $$\sum_{n=a}^{b}f(n)$$

And a product is usually defined like this:

  $$\prod_{n=a}^{b}f(n)$$

I'd like to know the following things:

Am I doing this in a proper functional way?
Is there a way to reduce the repetitiveness of the code?
Is it okay to use List.fold to multiply all the elements of a list together? Is there a better way?
Am I violation any style guidelines?
Anything else?

Here's the code:

/// <summary>
/// Find the summation in a specific range, given
/// a specific function.
/// </summary>
let summation func low high =
    let function_result_set: int list = [for x in low..high do yield x |> func]
    let result = function_result_set |> List.sum
    result

/// <summary>
/// Find the product in a specific range, given
/// a specific function.
/// </summary>
let product func low high =
    let function_result_set: int list = [for x in low..high do yield x |> func]
    let result = function_result_set |> List.fold (*) 1
    result

And finally, here's a few tests:

[<EntryPoint>]
let main argv =
    System.Console.WriteLine(summation (fun n -> n) 1 10)
    System.Console.WriteLine(product (fun n -> n) 1 10)
    System.Console.ReadKey() |> ignore
    0

And here's the desired output from the above tests:

55
3628800

Starting with summation,

let summation func low high =
    let function_result_set: int list = [for x in low..high do yield x |> func]
    let result = function_result_set |> List.sum
    result

First of all, we can remove result

let summation func low high =
    let function_result_set: int list = [for x in low..high do yield x |> func]
    function_result_set |> List.sum

Next, we don't need to construct a list containing func(low) .. func(high); we can instead operate over a sequence, meaning we don't have to have all those values in memory at once.

let summation func low high =
    let function_result_set = Seq.map func { low .. high }
    function_result_set |> Seq.sum

Now we can make it a bit more succinct

let summation func low high =
    Seq.map func { low .. high } |> Seq.sum

Finally, we can make it more generic by using statically resolved type parameters. At the moment, if we have a call to summation like this:

printfn "%d" <| summation id 1 10

then summation will be constrained to be of type (int -> int) -> int -> int -> int. We can fix this by making the function inline:

let inline summation func low high =
    Seq.map func { low .. high } |> Seq.sum

Now  summation has type ('a -> 'b) -> 'a -> 'a -> 'b and the following code will compile:

printfn "%d" <| summation id 1 10
printfn "%f" <| summation id 1.0 10.0

Similar comments hold for product, but with two additional things:

To make it generic we need to get the One property of the type ^a, and
I've used the checked version of * to check for overflow

let inline product func low high : ^a =
    Seq.map func { low .. high } |> Seq.fold Checked.(*) LanguagePrimitives.GenericOne< ^a >


Date/Time picker for browsers that don't support the datetimelocal input I have this code that shows date pickers when the browser doesn't support <input type="datetime-local">.
Any tips on optimizing this code?
if (!Modernizr.inputtypes.date) {
    $(function () {
        var d = Date.parse($('#StartDatePicker').val());
        $('#StartDatePicker').val($.format.date(d, "yyyy-MM-dd hh:mm a"));
        d = Date.parse($('#EndDatePicker').val());
        $('#EndDatePicker').val($.format.date(d, "yyyy-MM-dd hh:mm a"));

        var startDateTextBox = $('#StartDatePicker');
        var endDateTextBox = $('#EndDatePicker');
        startDateTextBox.datetimepicker({
            dateFormat: "yy-mm-dd",
            timeFormat: "hh:mm TT",
            minuteGrid: 10,
            addSliderAccess: true,
            sliderAccessArgs: {
                touchonly: false
            },
            onClose: function (dateText, inst) {
                if (endDateTextBox.val() != '') {
                    var testStartDate = startDateTextBox.datetimepicker('getDate');
                    var testEndDate = endDateTextBox.datetimepicker('getDate');
                    if (testStartDate > testEndDate) endDateTextBox.datetimepicker('setDate', testStartDate);
                } else {
                    endDateTextBox.val(dateText);
                }
            },
            onSelect: function (selectedDateTime) {
                endDateTextBox.datetimepicker('option', 'minDate', startDateTextBox.datetimepicker('getDate'));
            }
        });

        endDateTextBox.datetimepicker({
            dateFormat: "yy-mm-dd",
            timeFormat: "hh:mm TT",
            minuteGrid: 10,
            addSliderAccess: true,
            sliderAccessArgs: {
                touchonly: false
            },
            onClose: function (dateText, inst) {
                if (startDateTextBox.val() != '') {
                    var testStartDate = startDateTextBox.datetimepicker('getDate');
                    var testEndDate = endDateTextBox.datetimepicker('getDate');
                    if (testStartDate > testEndDate) startDateTextBox.datetimepicker('setDate', testEndDate);
                } else {
                    startDateTextBox.val(dateText);
                }
            },
            onSelect: function (selectedDateTime) {
                startDateTextBox.datetimepicker('option', 'maxDate', endDateTextBox.datetimepicker('getDate'));
            }
        });

    });
}

Yes there is a fair amount of repeated code - e.g. the options:

        dateFormat: "yy-mm-dd",
        timeFormat: "hh:mm TT",
        minuteGrid: 10,
        addSliderAccess: true,
        sliderAccessArgs: {
            touchonly: false
        },

These could be abstracted to a common object that gets extended with each call to datetimepicker() using Object.assign()
const commonOptions = {
    dateFormat: "yy-mm-dd",
    timeFormat: "hh:mm TT",
    minuteGrid: 10,
    addSliderAccess: true,
    sliderAccessArgs: {
        touchonly: false
    }
};

startDateTextBox.datetimepicker(Object.assign({
    onClose: function (dateText, inst) {
        if (endDateTextBox.val() != '') {
            const testStartDate = startDateTextBox.datetimepicker('getDate');
            const testEndDate = endDateTextBox.datetimepicker('getDate');
            if (testStartDate > testEndDate) endDateTextBox.datetimepicker('setDate', testStartDate);
        } else {
            endDateTextBox.val(dateText);
        }
    },
    onSelect: function (selectedDateTime) {
        endDateTextBox.datetimepicker('option', 'minDate', startDateTextBox.datetimepicker('getDate'));
    }
}, commonOptions));
endDateTextBox.datetimepicker(Object.assign({
    onClose: function (dateText, inst) {
        if (startDateTextBox.val() != '') {
            const testStartDate = startDateTextBox.datetimepicker('getDate');
            const testEndDate = endDateTextBox.datetimepicker('getDate');
            if (testStartDate > testEndDate) startDateTextBox.datetimepicker('setDate', testEndDate);
        } else {
            startDateTextBox.val(dateText);
        }
    },
    onSelect: function (selectedDateTime) {
        startDateTextBox.datetimepicker('option', 'maxDate', endDateTextBox.datetimepicker('getDate'));
    }
}, commonOptions));

The onClose function could be abstracted to a function that accepts the picker to modify as a parameter.
It appears there is an option in the datetimepicker documentation for a Range between date selection - it uses just the onShow option to set the min and max dates:

jQuery(function(){
 jQuery('#date_timepicker_start').datetimepicker({
  format:'Y/m/d',
  onShow:function( ct ){
   this.setOptions({
    maxDate:jQuery('#date_timepicker_end').val()?jQuery('#date_timepicker_end').val():false
   })
  },
  timepicker:false
 });
 jQuery('#date_timepicker_end').datetimepicker({
  format:'Y/m/d',
  onShow:function( ct ){
   this.setOptions({
    minDate:jQuery('#date_timepicker_start').val()?jQuery('#date_timepicker_start').val():false
   })
  },
  timepicker:false
 });
});


Making CRUD abstracted class in PHP I did a working CRUD abstracted class, and I am not sure that this is a good way for it, so I would like to hear your opinion.

protected function properties(){
    $properties = array();
    foreach (self::$table_fields as $db_field){
        if(property_exists($this, $db_field)){
            $properties[$db_field] = $this->$db_field;
        }
    }
    return $properties;
}

public function create(){
    $db = db::getConnection()->conn;
    $properties = $this->properties();

    $key = implode(",",array_keys($properties));
    $value = implode(",:",array_keys($properties));

    $sql = "INSERT INTO ".self::$table."(" . $key . ") ";
    $sql.= "VALUES (:" . $value . ")";
    $stmt = $db->prepare($sql);
    foreach($properties as $key=>$value){
        $stmt->bindParam($key,$value);
    }
    if($stmt->execute()){
        print_r($stmt);
        $this->id = db::the_insert_id();
        return true;
    }else{
        print_r($stmt);
        return false;
    }
    return $stmt;
}

This is working well: I am getting TRUE as response and it creates a user in database, but I am just not sure about it.

Format all identifiers in order to avoid a syntax error. Assuming it's mysql, use backticks for the purpose:

$key = "`".implode("`,`",array_keys($properties))."`";

You are going to repeat the second half of the code in the every crud method.  To avoid that, add a query() method to your db class

public function query($sql, $params = []) {
    $stmt = self::getConnection()->conn->prepare($sql);
    $stmt->execute($params);
    return $stmt;
}

so you'll be able to make your crud methods much more concise.

public function create(){
    $properties = $this->properties();

    $key = "`".implode("`,`",array_keys($properties))."`";
    $value = implode(",:",array_keys($properties));

    $sql = "INSERT INTO `".self::$table."` (" . $key . ") ";
    $sql.= "VALUES (:" . $value . ")";
    db::query($sql, $properties);
    $this->id = db::the_insert_id();
}

There is no point in returning true or false from such a method, the only reason for it to return false is a database error and such an error should be thrown in the form of Exception and handled elsewhere, making all this true false stuff rather useless.
Your current setup is following Active Record pattern, making a data object to contain all the database interaction related code. Consider switching to Data Mapper pattern where you have two classes - your data class and a mapper that is responsible for all the database interactions.


Unique nucleotide permutations, Python itertools product I'm searching for all the unique possible permutations of nucleotide given a defined length. By unique, I mean the reverse complement would not be counted.

  ACGT

For example, permutations of length 1 would be:

A:T
C:G
G:C
T:A

But the last two wouldn't be included because they would already be seen.

Length 2 would be:

AA:TT
AC:GT
AG:CT
AT:AT
CA:TG
CC:GG
CG:CG
CT:AG
GA:TC
GC:GC
GG:CC
GT:AC
TA:TA
TC:GA
TG:CA
TT:AA

So, 8, 11, 12, 14, 15, 16 wouldn't be included for the same reason, and so on.

def data_count(n, trantab):
    nn = int(n)
    count = 0
    seen = set()

    with open('file.txt','w') as f:
        for number in range(1, nn+1):
            for p in itertools.product('ACGT', repeat=number):
                print(''.join(p)+':'+''.join(p)[::-1].translate(trantab))
                if ''.join(p) in seen:
                    pass
                else:
                    seen.add(''.join(p)[::-1].translate(trantab))
                    count += 1
            #print(x)
            f.write(str(number)+' = '+str(count)+'\n')
            count = 0

It does the job pretty well until 10 or 12, but beyond that it takes a lot of time to output results.

n is the length of the search and trantab is:

intab = 'ACGT'
outtab = 'TGCA'
trantab = str.maketrans(intab, outtab)

to get the reverse complement of the nucleotide.

Is there any way to improve this one?

To get the best performance depends on what you want.
If you want to only write to the file you should use maths.
If you want to generate them all, well that's almost the best you'll get.

And so if you want to do the latter I'd remove all of the former from the function.
And just create:

def data_count(data, dethp):
    translation = str.maketrans(data, data[::-1])
    for number in range(1, int(depth)+1):
        for product in itertools.product(data, repeat=number):
            product = ''.join(product)
            yield product, product[::-1].translate(translation)

The other bit is a little harder.
What you want to do is find a common factor.
For repeat=2, we get the bullet points in your question.
And so we should we should group them, there's:

Groups that add to seen.
Groups that aren't counted due to seen.
Groups that are the same both sides.

# group 1
('AA', 'TT')
('AC', 'GT')
('AG', 'CT')
('CA', 'TG')
('CC', 'GG')
('GA', 'TC')

# group 2
('TT', 'AA')
('GT', 'AC')
('CT', 'AG')
('TG', 'CA')
('GG', 'CC')
('TC', 'GA')

# group 3
('TA', 'TA')
('GC', 'GC')
('CG', 'CG')
('AT', 'AT')

We can then use the above function to find out more about these groups.
And so we can get the following table:

1 + 2 + 3   | 1 + 3     | 3
------------+-----------+-----
4           | 2         | 0
16          | 10        | 4
64          | 32        | 0
256         | 136       | 16
1024        | 512       | 0
4096        | 2080      | 64
16384       | 8192      | 0
65536       | 32896     | 256
262144      | 131072    | 0
1048576     | 524800    | 1024
4194304     | 2097152   | 0
16777216    | 8390656   | 4096

From this we can see that 1 + 3 is the same as (1 + 2 + 3 + 3) // 2.
We can also find a pattern for 1 + 2 + 3 and for 3.
The first number is \$4 ^ \text{num}\$. The second is almost \$2 ^ \text{num}\$.
And so we can use this to find 1 + 3:

def data_count(depth):
    for n in range(1, int(depth)+1):
        yield n, (4 ** n + (0 if n % 2 else 2 ** n)) // 2

But if you want both, then you should use both functions.


Simple XmlHttpRequest object I would like some eyes on this:

/* Little XHR
 * by: rlemon        http://github.com/rlemon/
 * see README for useage.
 * */
var xhr = {
    xmlhttp: (function() {
        var xmlhttp;
        try {
            xmlhttp = new XMLHttpRequest();
        } catch (e) {
            try {
                xmlhttp = new ActiveXObject('Msxml2.XMLHTTP');
            } catch (er) {
                try {
                    xmlhttp = new ActiveXObject('Microsoft.XMLHTTP');
                } catch (err) {
                    xmlhttp = false;
                }
            }
        }
        return xmlhttp;
    }()),
     /* https://github.com/Titani/SO-ChatBot/blob/ccf6cfe827aee2af7b2832e48720a8e24a8feeed/source/bot.js#L110 */
    urlstringify: (function() {
        var simplies = {
            'number': true,
            'string': true,
            'boolean': true
        };
        var singularStringify = function(thing) {
            if (typeof thing in simplies) {
                return encodeURIComponent(thing.toString());
            }
            return '';
        };
        var arrayStringify = function(array, keyName) {
            keyName = singularStringify(keyName);
            return array.map(function(thing) {
                return keyName + '=' + singularStringify(thing);
            });
        };
        return function(obj) {
            return Object.keys(obj).map(function(key) {
                var val = obj[key];
                if (Array.isArray(val)) {
                    return arrayStringify(val, key);
                } else {
                    return singularStringify(key) + '=' + singularStringify(val);
                }
            }).join('&');
        };
    }()),
    post: function(options) {
        this.request.apply(this, ['POST', options]);
    },
    get: function(options) {
        this.request.apply(this, ['GET', options]);
    },
    request: function(type, options) {
        if (this.xmlhttp && options && 'url' in options) {
            var xhr = this.xmlhttp,
                enctype = ('enctype' in options) ? options.enctype : 'application/x-www-form-urlencoded';
            xhr.open(type, options.url, true);
            xhr.setRequestHeader('Content-Type', enctype);
            xhr.onreadystatechange = function() {
                if (xhr.readyState == 4) {
                    if (xhr.status == 200) {
                        if ('success' in options && options.success.apply) {
                            options.success.apply(this, [xhr]);
                        }
                    } else if (xhr.status && xhr.status != 200) {
                        if ('failure' in options && options.failure.apply) {
                            options.failure.apply(this, [xhr]);
                        }
                    }
                }
            };
            var data = null;
            if ('data' in options) {
                data = this.urlstringify.apply(this, [options.data]);
            }
            xhr.send(data);
        }
    }
};

For a barebones object; have I missed anything important? Should I change how any of this is organized or layed out? I have also tested in a limited number of browsers and so far so good! However I am unsure about older browsers and compatibility.

Any input would be great! thanks!

Its interface might not be obvious enough for junior programmers in a hurry. I suppose there's some assistance in your README. Anyway the code is quite clear given a little experience or effort. It doesn't use exactly the subset of JavaScript that Douglas Crockford recommended in JavaScript: The Good Parts, but that doesn't always matter.

I like this code. It has enough complexity for the real world without taking on responsibilities extraneous to wrapping XmlHttpRequest (or its equivalents). Nice use of closures. I really like that you incorporated a hyperlink to a line of github code that you found helpful.