Good, flexible and secure MVC router PHP I have developed a new router after having learnt a lot from my previous attempts and I will be using this in sites I make from now on.

In a previous question, the issue of REQUEST_URI being too inflexible made me wonder if I could make adjustments to allow it to route $_POST and $_GET requests too?

Please let me know what you think and how it could be improved as I am still learning:

class Router {

public $offset = 9 ; // Adjust according to subdir length.
public $start_page = 'Index' ; // Change to your home/default page.
private $classAliases = array(
    'pretty-url' => 'UglyClassName',
) ;

private $methodAliases = array(
    'pretty-method' => 'uglyMethodName',
) ;

function __construct()
{
    $url = substr(rtrim($_SERVER['REQUEST_URI'], '/'), $this->offset) ;

    $command = explode('/', $url, 3) ;

    if(isset($command[0]))
    {
        $class = ucfirst($this->autoMap($command[0]), $this->classAliases) ;
    }

    if(class_exists($class) && preg_match('/[^A-Za-z0-9]/', $class) == false)
    {
        $controller = new $class ;

        if($method = (isset($command[1]) ? $command[1] : NULL))
        {
            $method = (strstr($method, '?') ? substr($url, 0, strrpos($method, '?')) : $method) ;
            $method = $this->autoMap($method, $this->methodAliases) ;
            if(method_exists($class, $method) && preg_match('/[^A-Za-z0-9]/', $method) == false)
            {
                $params = array() ;
                if(stristr($url, '?')) // Parameters passed the conventional way...
                {
                    $queryString = substr($url, strrpos($url, '?')+1) ;
                    parse_str($queryString, $params) ;
                }
                elseif(isset($command[2])) // ...or the clean URL way.
                {
                    $params = explode('/', $command[2]) ;
                }
                call_user_func_array(array($controller, $method), $params) ;
            }
            elseif($method)
            {
                $this->throwError('Method '.$method.' does not exist.') ;
            }
        } else { // Default index method.
            if(method_exists($class, 'index'))
            {
                $controller->index() ;
            } else {
                $this->throwError('Class '.$class.' has no index method.') ;
            }
        }
    }
    elseif(!$class)
    {
        $controller = new $this->start_page ;
        $controller->index() ;
    } else {
        $this->throwError('Class '.$class.' does not exist.') ;
    }
}

private function throwError($e)
{
    if(isset($_SERVER['HTTP_X_REQUESTED_WITH'])) // Ajax request...
    {
        echo $e ;
    } else { // ...or normal request.
        new Error($e) ;
    }
}

private function autoMap($alias, $routes)
{
    if(array_key_exists($alias, $routes))
    {
        return $routes[$alias] ;
    }
    return $alias ;
}

}

Where you have:

$class = ucfirst($this->autoMap($command[0]), $this->classAliases);

I think it should be:

$class = ucfirst($this->autoMap($command[0], $this->classAliases));

The closing ) is in the wrong place.

Besides that I like your router, maybe it would be even better if it had a module control (so you could have something like: module/controller/method). This way you can have separated modules for admin, debug, default, etc.


Reading from a text file into a structure and posting to list boxes I am trying to stay ahead of my Year 12 Software class. Starting to work with records and arrays. I have answered the question, but the solution feels very clunky. I am hoping someone has suggestions/links for completing this task in a more efficient way.
The task: read in lines from a text file and into a structure, and then loop through that, populating four list boxes if an animal hasn't been vaccinated.
Imports System.IO
Public Class Form1
    'Set up the variables - customer record, total pets not vaccinated, total records in the file, and a streamreader for the file.
    Structure PetsRecord
        Dim custName As String
        Dim address As String
        Dim petType As String
        Dim vacced As String
    End Structure
    Dim totNotVac As Integer
    Dim totalRecCount As Integer
    Dim PetFile As IO.StreamReader

    Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load

    End Sub

    Private Sub btnLoad_Click(sender As Object, e As EventArgs) Handles btnLoad.Click

        'set an array of records to store each record as it comes in. Limitation: you need to know how many records in the file. Set the array at 15 to allow for adding more in later.
        Dim PetArray(15) As PetsRecord
        'variables that let me read in a line and split it into sections.
        Dim lineoftext As String
        Dim i As Integer
        Dim arytextfile() As String
        'tell them what text file to read
        PetFile = New IO.StreamReader("patients.txt")
        totNotVac = 0

        Try
            totalRecCount = 0
            ' read each line in and split the lines into fields for the records. Then assign the fields from the array. Finally, reset the array and loop.
            Do Until PetFile.Peek = -1
                'read in a line of text
                lineoftext = PetFile.ReadLine()
                'split that line into bits separated by commas. these will go into the array.
                arytextfile = lineoftext.Split(",")
                'dunno whether this is the best way to do it, but stick the array bits into the record, and then clear the array to start again.
                PetArray(totalRecCount).custName = arytextfile(0)
                PetArray(totalRecCount).address = arytextfile(1)
                PetArray(totalRecCount).petType = arytextfile(2)
                PetArray(totalRecCount).vacced = arytextfile(3)

                totalRecCount += 1
                Array.Clear(arytextfile, 0, arytextfile.Length)
            Loop

            For i = 0 To PetArray.GetUpperBound(0)

                If PetArray(i).vacced = "No" Then
                    lstVaccinated.Items.Add(PetArray(i).vacced)
                    lstCustomer.Items.Add(PetArray(i).custName)
                    lstAddress.Items.Add(PetArray(i).address)
                    lstPetType.Items.Add(PetArray(i).petType)

                    totNotVac += 1
                    lblVacTotal.Text = "The total number of unvaccinated animals is " & CStr(totNotVac)
                End If

            Next
        Catch ex As Exception
            MsgBox("Something went wrong with the file")
        End Try
        PetFile.Close()

    End Sub

    Private Sub btnExit_Click(sender As Object, e As EventArgs) Handles btnExit.Click
        Close()

    End Sub
End Class

And one line from the patient.txt file:
    Richard Gere,16 Sunset Blvd,Gerbil,No

Opinion: there is no benefit to building an array like this. I agree it is a clunky use of arrays which I think is unnecessary (and old-school). For instance you don't even need to know in advance how many records, nor do you need to dimension the array to a predetermined size. There are more flexible data types available than arrays for the job.
When adding items to controls like a ListView or ListBox using .Items.Add you can use BeginUpdate at the beginning of your loop and EndUpdate at the end as you are populating the items from a loop.
The aim is to make the UI more responsive - if you have a large file and you're adding lots of items, the interface may freeze while the loop is running.
Although I would probably bind the list to a datatable instead, or another structure. Just append items to the datatable and let the control update itself.
So here is a refactored example: I have converted your structure to a class and I use List Of instead of an array (note the addition of a binding source). For this to work I need to use properties to expose class members (so I think), which is why I am using a class instead of a structure, but there should be a way to retain a structure if you wish.
There are several ways of reading a file, in this example I am using StreamReader. Note the use of a context manager (the Using block), which means I don't have to worry about closing the file, including in case of an exception. This happens automatically past the End Using statement.
In your code PetFile.Close() will not be executed if an exception occurs. Your procedure will exit after MsgBox. But what you can do is move PetFile.Close() to a Finally section within your Try-Catch block. But if you're attempting to close a file that isn't even open, this will raise an exception within the exception. The best solution is not to bother with it and use the context manager.
Note that each line in the file has to contain at least 4 comma-separated fields or the code will crash. So it is advisable to verify for each line that fields.Count = 4 and skip lines that don't match this criterion.
Here I am populating only one listbox, you can figure out the rest easily hopefully.
Imports System.IO

Public Class Form1

    Public Class PetsRecord
        Private m_CustomerName As String
        Private m_CustomerAddress As String
        Private m_PetType As String
        Private m_Vaccinated As String

        Public Property CustomerName() As String
            Get
                Return Me.m_CustomerName
            End Get
            Set(ByVal value As String)
                Me.m_CustomerName = value
            End Set
        End Property

        Public Property CustomerAddress() As String
            Get
                Return Me.m_CustomerAddress
            End Get
            Set(ByVal value As String)
                Me.m_CustomerAddress = value
            End Set
        End Property

        Public Property PetType() As String
            Get
                Return Me.m_PetType
            End Get
            Set(ByVal value As String)
                Me.m_PetType = value
            End Set
        End Property

        Public Property Vaccinated() As String
            Get
                Return Me.m_Vaccinated
            End Get
            Set(ByVal value As String)
                Me.m_Vaccinated = value
            End Set
        End Property
    End Class 'Structure

    Private Sub btnLoad_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnLoad.Click
        Dim PetArray As New List(Of PetsRecord)
        Dim bs As New BindingSource()
        Dim PetFile As String = "C:\Users\test\Documents\pets.txt"

        Using r As StreamReader = New StreamReader(PetFile)
            Dim line As String
            line = r.ReadLine
            Do While (Not line Is Nothing)

                ' split the line into fields
                Dim fields As New List(Of String)(line.Split(","c))
                If fields.Count < 4 Then
                    Console.WriteLine("Skipping line (contains less than 4 fields): " & line)
                Else
                    ' flag this line
                    If fields(3) = "No" Then
                        Dim pet As New PetsRecord
                        pet.CustomerName = fields(0)
                        pet.CustomerAddress = fields(1)
                        pet.PetType = fields(2)
                        pet.Vaccinated = fields(3)
                        PetArray.Add(pet)
                    End If
                End If

                line = r.ReadLine
            Loop
        End Using

        ' bind the listbox to PetArray
        bs.DataSource = PetArray
        Me.lstCustomer.DisplayMember = "CustomerName"
        Me.lstCustomer.DataSource = bs

    End Sub
End Class


Simple thread pool in C++ I wrote a simple thread pool, which works pretty well. I would like to see your review of it.

One important feature that I needed in the pool is the ability to wait until all the tasks that I sent to the pool are complete (so that later I could send other tasks that are dependent on the result of the previous tasks).

This is the basic task which you inherit from, to send tasks:

class Task
{
public :
    virtual void ExecuteTask() = 0;
};

This is the .h file of the Thread pool:

#include<time.h>
#include <Windows.h>
#include<list>

using namespace std;

class ThreadPool
{
public :
    ThreadPool(int threadCount=0);
    bool ScheduleTask(Task* task);
    bool WaitForWorkToBeFinished(DWORD dwMilliseconds);
    ~ThreadPool();

private :
    static DWORD WINAPI ThreadStart(LPVOID threadParameters);
    DWORD ThreadMain();

    list<Task*> mTasks;
    HANDLE* mThreads;
    int mThreadCount;
    CRITICAL_SECTION mCriticalSection;
    CONDITION_VARIABLE mConditionVariable;
    CONDITION_VARIABLE mConditionVariableTaskFinished;
    int mNumberOfTasksNotFinished;
    bool mConsum;
};

This is the CPP file:

#include "ThreadPool.h"

ThreadPool::ThreadPool(int threadCount)
{
    mThreadCount=threadCount;
    mThreads = new HANDLE[mThreadCount]; //This handles will be used to wait upon them

    InitializeCriticalSection(&mCriticalSection);
    InitializeConditionVariable(&mConditionVariable);
    InitializeConditionVariable(&mConditionVariableTaskFinished);
    mConsum = true;
    mNumberOfTasksNotFinished = 0;
    for(int i = 0; i< mThreadCount;i++)
        mThreads[i] = CreateThread(NULL, 0, ThreadStart, (void*)this, 0,NULL);
}

bool ThreadPool::ScheduleTask(Task* task)
{
    EnterCriticalSection(&mCriticalSection);
    mTasks.push_back(task); //This is a simple std::list
    mNumberOfTasksNotFinished++;
    LeaveCriticalSection(&mCriticalSection);

    WakeConditionVariable(&mConditionVariable); //Waking up the threads so they will know there is a job to do
    return true;
}

DWORD WINAPI ThreadPool::ThreadStart(LPVOID threadParameters) //Just a static function to help me use member function as a thread
{
    ThreadPool* referenceToThis = (ThreadPool*)threadParameters;
    return referenceToThis->ThreadMain();
}

DWORD ThreadPool::ThreadMain() //The main thread function
{
    do
    {
        Task* currentTask;
        EnterCriticalSection(&mCriticalSection);

        while(mTasks.size() ==0 && mConsum)
            SleepConditionVariableCS(&mConditionVariable,&mCriticalSection, INFINITE);

        if(!mConsum) //Destructor ordered on abort
        {
            LeaveCriticalSection(&mCriticalSection);
            return 0;
        }

        //If we got here, we successfully aquired the lock and the list<Task> is not empty
        currentTask = mTasks.front();
        mTasks.pop_front();

        LeaveCriticalSection(&mCriticalSection);

        currentTask->ExecuteTask();
        //delete currentTask;

        EnterCriticalSection(&mCriticalSection);
        mNumberOfTasksNotFinished--;
        LeaveCriticalSection(&mCriticalSection);

        WakeConditionVariable(&mConditionVariableTaskFinished);
    }while(mConsum);

    return 0;
}
//This function is very important, it gives the user the ability to send 10 tasks to the thread pool
// then to wait till all the tasks completed, and give the next 10 which are dependand on the result of the previous ones.
bool ThreadPool::WaitForWorkToBeFinished(DWORD dwMilliseconds)
{
    EnterCriticalSection(&mCriticalSection);
    while(mNumberOfTasksNotFinished!=0)
        SleepConditionVariableCS(&mConditionVariableTaskFinished,&mCriticalSection, INFINITE);
    LeaveCriticalSection(&mCriticalSection);

    return true;
}

ThreadPool::~ThreadPool()
{
    mConsum = false;
    WakeAllConditionVariable (&mConditionVariable);
    WaitForMultipleObjects(mThreadCount,mThreads, true, INFINITE);
    DeleteCriticalSection(&mCriticalSection);
}

A few points:

Your EnterCriticalSection/LeaveCriticalSection code should be placed into a scoped RAII object. This saves you from explicitly calling leave, makes your code smaller, and avoids a deadlock if the synchronized code throws.

Do something similar with everything that requires cleanup (like mThreads = new HANDLE[mThreadCount];).

The interface of your class should receive the tasks by reference and store them by pointer within. This way, you make it clear to client code that the class doesn't take ownership of the tasks it receives. (see Third Edit below).

Use reinterpret_cast instead of C-style casts (because C-style casts = bad, mmmkay?!)

using namespace std;

Don't do this. It greatly restricts what client code (whoever includes your class) can write, and the errors generated in client code are almost always obscure. You're better off repeating that in local scopes (if you must) or (better still) not using it (consider writing using std::string; and similar, in the scopes you use the type, or simply declaring std::string x;).

Edit: If bool ThreadPool::ScheduleTask(Task* task) never returns false, why does it return bool? Shouldn't it be void?

Second Edit: Consder using an unsigned int for the number of threads (mThreadCount).

Third edit ( Hi Loki :) )

Regarding ownership of the threads and object lifetime management (how to implement safely and indicate it in the public interface of the class).

Consider this client code:

ThreadPool runner();

// bad bad code: don't call new with raw pointers like this in practice
// but easy to write to make my point below.
BackgroundTask *t = new BackgroundTask(); // class BackgroundTask: public Task
                                          // defined elsewhere

runner.ScheduleTask(t);

What you don't know looking at this code: should you delete t after the ThreadPool completes running? Will runner delete the task itself? If it does, t will point to deleted memory after runner finishes. If it doesn't, you will have to remember to check when runner has finished, and call delete explicitly, every single time you use the thread pool.

You could implement the ThreadPool to delete all tasks after execution, but then, an enterprising programmer (maybe yourself) could write:

ThreadPool runner();
BackgroundTask t();
runner.ScheduleTask(&t); // pass address of automatic storage object

and this will crash your program when your ThreadPool deletes the task.

Another client code alternative, this one much much worse:

void crashy(ThreadPool& pool) {
    BackgroundTask t;
    pool.ScheduleTask(&t);
} // t goes out of scope here and is destroyed but p still holds it's address

ThreadPool p;
crashy(p); // some compilers tend to happily accept this (maybe with a warning)
// p now has a pointer to a deleted BackgroundTask instance

The interface doesn't indicate ownership and makes you interrupt your coding, to look at the implementation, just so you know how to write client code that doesn't leak or crash.

Solutions:

You could change your interface to take a Task& instead. (Similar to passing by pointer), the only drawback is that you need to make sure the tasks you pass to ScheduleTask must still be in scope until the ThreadPool stops running (in other words, you can still write the nice crashy function).

This is better, as at least you don't need to wonder if it will delete the task (which passing by pointer may, or may not suggest).

Better yet, change your interface to take a std::unique_ptr instead:

class ThreadPool
{
public :
    bool ScheduleTask(std::unique_ptr<Task> task) // << std::unique_ptr by val.
    {
        EnterCriticalSection(&mCriticalSection);
        mTasks.push_back( std::move(task) ); // << notice std::move
        mNumberOfTasksNotFinished++;
        LeaveCriticalSection(&mCriticalSection); // also notice non-raii unlock;
                                                 // if push_back throws on
                                                 // allocating new list node,
                                                 // your whole app is deadlocked

        WakeConditionVariable(&mConditionVariable);
        return true;
    }

private :

    list<std::unique_ptr<Task>> mTasks; // << notice std::unique_ptr
    ...
};

Client code:

ThreadPool runner();
std::unique_ptr<Task> p(new BackgroundTask());
runner.ScheduleTask(std::move(p)); // works, no lifetime management problems
                                   // and no ambguity either

Since the function takes a unique_ptr now, you now know you need to pass a pointer and you also know you are passing ownership as well.


Python style with-statement in C I thought about using a macro to auto close FILE pointers at the end of a block. My solution so far is:

FILE *fopen_safe(const char *filename, const char *mode) {
    FILE *fp = fopen(filename, mode);

    if (!fp) {
        perror("fopen");
        exit(EXIT_FAILURE);
    }

    return fp;
}

#define with_fopen(fp, filename, mode) \
    for (FILE *fp = fopen_safe(filename, mode), _invariant = { ._r = 1 }; _invariant._r--; fclose(fp))

// Usage:
with_fopen(fp, "test", "w") {
    fprintf(fp, "test\n");
}

This solution is not elegant as i am using an int in the struct to create my loop invariant.

Is there another way to auto close files? Can a block be associated with a macro in a smarter way?

There are few problems with this macro.

It relies on an undocumented layout of FILE structure
It cannot be nested (try it with -Wshadow)
It assumes that the block is always left through the end
It may create suprises:

    for (....) {
        with_fopen(fp, ...) {
            int rc = do_something(fp);
            if (rc == -1) {
                break;
            }
        }
    }

The programmer (with or without Python background) expects the outer loop to be broken. Instead the loop hidden inside with is.

Nice try however.


Is the code prone to SQL injection or to any other kind of threat I am very new to PHP coding, hence just wanted to know if my coding is prone to any SQL injection attack or any other kind of threat. Below is the code for adding client to the database:

<?php
$response = array();
include 'db/db_connect.php';
include 'functions.php';

//Get the input request parameters
$inputJSON = file_get_contents('php://input');
$input = json_decode($inputJSON, TRUE); //convert JSON into array

//Check for Mandatory parameters
if(isset($input['clientName']) && isset($input['siteName'])){
    $clientName = $input['clientName'];
  $siteName = $input['siteName'];
    //Check if client already exist
    if(!clientExists($clientName, $siteName)){
        //Query to add new material
        $insertQuery  = "INSERT INTO clients(clientName, siteName) VALUES (?,?)";
        if($stmt = $con->prepare($insertQuery)){
            $stmt->bind_param("ss",$clientName,$siteName);
            $stmt->execute();
            $response["status"] = 0;
            $response["message"] = "Client created";
            $stmt->close();
        }
    }
    else{
        $response["status"] = 1;
        $response["message"] = "Client exists";
    }
}
else{
    $response["status"] = 2;
    $response["message"] = "Missing mandatory parameters";
}
echo json_encode($response);
?>

Code for clientExists() function is as follows:

function clientExists($clientName,$siteName){
    $query = "SELECT clientName FROM clients WHERE clientName = ? AND siteName=?";
    global $con;
    if($stmt = $con->prepare($query)){
        $stmt->bind_param("ss",$clientName,$siteName);
        $stmt->execute();
        $stmt->store_result();
        $stmt->fetch();
        if($stmt->num_rows == 1){
            $stmt->close();
            return true;
        }
        $stmt->close();
    }
    return false;
}

SQL injection should not be a problem, as long as you pass any user input to queries through placeholders, which you do.

I have a few other remarks though

Single Responsibility Principle

In the first code snippet you do too much things. You decode request json, you talk to db. you handle the logic, you maintain response structure. It would be wise to separate those things into their own scopes (functions/classes). Of course if this is all you ever intend to have there may be no point. But in general, in future you will want to reuse the individual pieces.

Global Scope vs. Classes and Dependency Injection

You may want to avoid definind global variables. It's generaly considered bad practise for several reason which I'm not gonna repeat here as there is plenty of information sources for this already.

You are probably not very familiar with the concept of OOP, but wrapping those function into class scopes and working with objects may be something for you to learn.

If you want to stick with procedural style, maybe at least get rid of the globals by passing the db connection always as function parameter.

if (clientExists($connection, $clientName)) {...}

It is annoying but it is still better then globals, in some ways anyway.
That where the objects could help if you get used to it.

$users->exists($clientName)

Data Validation

You check that $input['clientName'] is set. Is it enough? What if it is empty? What if it is array or number? Is it ok? What if $input is not an array in the first place? All that should be handled.

Validation is btw, yet another responsibility that might be separated to its own function.

Useless Statements

In the clientExists() function I think you fetch the row for now reason. It should be able to tell count without fetching it.
You can also simplify the select from SELECT clientName to SELECT 1 which is, IMO, cleaner as you could actualy select any of the table's column for it to work and if that column ever changes name this would have to change too.

Public API

At this point your api is public in whichever network in which it is deployed. As this network is probably going to be the internet (I suppose), anyone will be able to call it. Is it ok if any anonymous client can succesfully call this endpoint and so create db entries? You may want to have some way of authentication, but this depends on your use-case.


Octree code performance I'm using a simple octree in my program and wanted to know if my implementation could be faster. It is part of this program.

Octree.H:

#ifndef OCTREE_H
#define OCTREE_H

#include "Rendera.H"

class Octree
{
public:
  struct node_t
  {
    float value;
    struct node_t *child[8];
  };

  Octree();
  virtual ~Octree();

  void clear(struct node_t *);
  void write(const int &, const int &, const int &, const float &);
  void writePath(const int &, const int &, const int &, const float &);
  float read(const int &, const int &, const int &);

  struct node_t *root;
};

#endif

Octree.cxx:

#include "Octree.H"

Octree::Octree()
{
  root = new node_t;
  root->value = 0;

  for(int i = 0; i < 8; i++)
    root->child[i] = 0;
}

Octree::~Octree()
{
  clear(root);
}

void Octree::clear(struct node_t *node)
{
  for(int i = 0; i < 8; i++)
    if(node->child[i])
      clear(node->child[i]);

  delete node;
}

void Octree::write(const int &r, const int &g, const int &b,
                   const float &value)
{
  struct node_t *node = root;

  for(int i = 7; i >= 0; i--)
  {
    const int index = ((r >> i) & 1) << 0 |
                      ((g >> i) & 1) << 1 |
                      ((b >> i) & 1) << 2;

    if(!node->child[index])
    {
      node->child[index] = new node_t;
      node = node->child[index];
      node->value = 0;

      for(int j = 0; j < 8; j++)
        node->child[j] = 0;
    }
    else
    {
      node = node->child[index];
    }
  }

  node->value = value;
}

// Sets entire path to value.
// This allows the octree to be used in a different way.
// (Needed for palette lookup.)
void Octree::writePath(const int &r, const int &g, const int &b,
                       const float &value)
{
  struct node_t *node = root;

  for(int i = 7; i >= 0; i--)
  {
    const int index = ((r >> i) & 1) << 0 |
                      ((g >> i) & 1) << 1 |
                      ((b >> i) & 1) << 2;

    if(!node->child[index])
    {
      node->child[index] = new node_t;
      node = node->child[index];
      node->value = value;

      for(int j = 0; j < 8; j++)
        node->child[j] = 0;
    }
    else
    {
      node = node->child[index];
    }
  }
}

float Octree::read(const int &r, const int &g, const int &b)
{
  struct node_t *node = root;

  for(int i = 7; i >= 0; i--)
  {
    const int index = ((r >> i) & 1) << 0 |
                      ((g >> i) & 1) << 1 |
                      ((b >> i) & 1) << 2;

    if(node->child[index])
      node = node->child[index];
    else
      break;
  }

  return node->value;
}

Performance
(Without any profiling data, I'm guessing blindly here)
You have a linked data structure (a tree), traversing the tree has poor cache locality and this is where your performance will likely suffer. Because every node could be a cache miss at worst. But please do measure this first, if you see that most of your time is spent getting the next node or close to a line of code that does, you're having cache issues.
One thing you can do to try to improve your cache performance is to encode your tree into a linear sequence (std::vector preferrably). And then measure to see if it actually is faster.
As for the encoding, we'll refer to a level of the tree as k and the root has k=0. This means that the level k has at most 8^k nodes. If we refer to each node in a level by it's grid coordinate (x,y, z) then the node n(k,x,y,z) (at level k and position (x,y,z)) will have it's child nodes on:

n(k+1, 2*x, 2*y, 2*z)
... (all combinations of +1 to the x,y,z) ...
n(k+1, 2*x + 1, 2*y + 1, 2*z + 1)

We can order all the nodes into a linear random access sequence v (std::vector) like this:
level_base = ipow(8,k) - 1;
level_width = ipow(2,k);
grid_index = x + level_width * y + level_width*level_width*z;
n(k, x, y) = v[level_base + grid_index];

Note that the width of the grid at level k is 2^k (example: 1, 2, 4, 8). Here ipow is an optimized integer power function. If ipow proves to be a bottle neck, you can use a LUT for the powers of 8 and 2.
Benefits

The benefits to doing this is that Breadth First Traversal/Search becomes a linear access pattern which your CPU's pre-fetcher will love you for. This is very fast.
If you know before hand which node you want (k,x,y,z), you can get it directly.
You can get all the neighbours easily.
Given your (r,g,b) triplet, you may be able to just directly compute the index of the destination node, write the value and you're done. No iteration to the right node.
Less memory fragmentation.
For dense trees this actually takes less memory as you don't have the heap bookkeeping overhead etc.
You do not need the child[] array making your data structure smaller and more of them can fit in each cache line.

Drawbacks

The std::vector is dense, if your tree is very sparse, you may end up allocating a lot more nodes than you need. On the other hand, if you have the memory it might be acceptable.
It's a more complex indexing which can result in bugs or more complex code if proper caution isn't taken.
If you need to distinguish between empty and non-empty nodes (as opposed to having value == 0), you need to introduce a has_data flag. But the nodes are still smaller than having the child array.
Not as resilient against off-by-one type of bugs. In a linked-tree type structure if you manage to somehow write outside of the node's structure you'll write into the canary values on the heap or in the heap book-keeping. Under debug mode your RT should let you know you have an issue. With this approach you will silently corrupt nearby nodes. Not necessarily likely but something to be aware of.


Quick sort in Python How can this be improved?

def quick_sort(temp_list):
#Check if the array is having one element or not
    if len(temp_list) == 1:
        return temp_list
    elif len(temp_list) >= 2:
        l_list = list()
        g_list = list()
#Choose a pivot
        if len(temp_list) % 2 == 0:
            pivot = temp_list[len(temp_list)/2 - 1]
        else:
            pivot = temp_list[len(temp_list)/2]
#Make two list greater and lesser than the pivot
        for e in temp_list:
            if e < pivot:
                l_list.append(e)
            elif e > pivot:
                g_list.append(e)
#Merge the two list with the pivot
        return merge(quick_sort(l_list),pivot,quick_sort(g_list))

def merge(list1,mid,list2):
    newlist = list()
    if not list1 and list2:
        newlist.append(mid)
        newlist.extend(list2)
    if not list2 and list1:
        newlist.extend(list1)
        newlist.append(mid)
    if list1 and list2 :
        newlist.extend(list1)
        newlist.append(mid)
        newlist.extend(list2)
    return newlist

On Style

“Quicksort” is customarily written as one word – the method should be called quicksort, not quick_sort. What’s more, this is an implementation detail and a public API shouldn’t reveal this (it’s subject to change), hence sort would be an even better name.
Some of your comments are redundant: “Check if the array is having one element or not” – that is (and should be!) obvious from the code. Don’t comment here. Reserve comments to explain why you do something. The next comment (“choose a pivot”) is better since that’s not immediately obvious from the code. However, it would be better to make it obvious in code and maybe have a comment describe how you choose the pivot (e.g. simple median).

The other comments are redundant again.
Furthermore, indenting the comments to the correct level would increase readability. As it stands, the comments visually hack the code into pieces.
Names: l_list and g_list contain unnecessary information that would easily be inferred from context – list, and their actual function is codified in just one letter. Rename them to lesser and greater or similar.

temp_list as a parameter is actually wrong: it’s not really temporary, it’s just the input. And again, list is a bit redundant. In fact, a one-letter identifier is acceptable here (but that’s the exception!). How about a for “array”?

Code

Use the idiomatic [] instead of list() to initialise lists.
The first elif is unnecessary, and causes the subsequent code to be indented too much. Remove it. (Also, it should logically be an else, not an elif).
Your choice of the pivot is a bit weird. Why two cases? Why not just use the middle? pivot = a[len(a) // 2]
Your code discards duplicate elements: quick_sort([1, 2, 3, 2]) will yield [1, 2, 3].
blufox has already remarked that your merge routine is more convoluted than necessary. In fact, you could just use concatenation here, lesser + [pivot] + greater (but this again discards duplicates)
It’s also a good idea to explicitly comment invariants in such algorithmic code.

Putting It Together

import itertools

def sort(a):
    def swap(i, j):
        tmp = a[i]
        a[i] = a[j]
        a[j] = tmp

    if len(a) < 2:
        return a

    lesser = []
    greater = []
    # Swap pivot to front so we can safely ignore it afterwards.
    swap(0, len(a) // 2)
    pivot = a[0]

    # Invariant: lesser < pivot <= greater
    for x in itertools.islice(a, 1, None):
        if x < pivot:
            lesser.append(x)
        else:
            greater.append(x)

    return sort(lesser) + [pivot] + sort(greater)

Bonus: Algorithmic Considerations

As blufox noted, this implementation requires more additional memory than necessary.  This is acceptable when working with immutable data structures, but becomes quite expensive when used with mutable arrays.

A normal, mutable quicksort implementation consists of two steps:

in-place partitioning along a pivot
recursive sorting of the two partitions

The second step is trivial; the partitioning and choice of pivot however require some sophistication, otherwise they will have degenerate worst cases (even without considering the worst-case input “sorted array”). The definitive paper in this regard is Engineering a sort function by Bentley and McIlroy. It is one of the clearest, most thorough computer science papers ever written and I heartily recommend its reading.

That said, no “toy” implementation will ever replace an industry-strength implementation and attempting to do so is largely a waste of time, so it’s entirely reasonable to deliberately simplify an implementation by ignoring some performance considerations.

In this regard the implementation above fares quite well. Just for the gained understanding I would still suggest that you attempt writing a variant which uses an in-place partitioning step.


How to make this ping test with timer more efficient? I wrote a small ducktaped VBA script which pings servers every 15 mins or so.  If a server's status is anything other than "Alive", the server and timestamp is written to another worksheet called "Log".

Sub Countup()
    Dim CountDown As Date
    CountDown = Now + TimeValue("00:00:01")
    Application.OnTime CountDown, "Auto_Open"
End Sub

Sub Auto_Open()
    Dim count As Range

    Set count = Worksheets("Servers").Range("A1:A1")
    count.Value = count.Value - TimeSerial(0, 0, 1)
    If count <= 0 Then
        count = Worksheets("Servers").Range("C1:C1")
        Call GetComputerToPing
        Call Countup

        Exit Sub
    End If
    Call Countup
End Sub

Public Sub addDataToTable(ByVal strTableName As String, ByVal strData As String, ByVal Col As Integer)
    Dim lLastRow As Long
    Dim iHeader As Integer

    With ActiveSheet.ListObjects(strTableName)
        'find the last row of the list
        lLastRow = ActiveSheet.ListObjects(strTableName).ListRows.count
        'shift from an extra row if list has header
        If .Sort.Header = xlYes Then
            iHeader = 1
        Else
            iHeader = 0
        End If
    End With
    'add the data a row after the end of the list
    ActiveSheet.Cells(lLastRow + 1 + iHeader, Col).Value = strData
End Sub

'Requires references to Microsoft Scripting Runtime and Windows Script Host Object Model.
'Set these in Tools - References in VB Editor.

Function sPing(sHost) As String
    On Error Resume Next
    sHost = Trim(sHost)
    Dim ipaddress As String
    Dim computername As String
    Dim Model As String
    Dim memory As Long

    Dim oPing As Object, oRetStatus As Object
    Set oPing = GetObject("winmgmts:{impersonationLevel=impersonate}")
    Set oPing = oPing.execquery("select * from win32_pingstatus where address ='" & sHost & "'")
    For Each oRetStatus In oPing
        If IsNull(oRetStatus.statuscode) Then
            sPing = "Dead"
        ElseIf oRetStatus.statuscode = 11010 Then
            sPing = "Request Timed Out"
        ElseIf oRetStatus.statuscode = 11013 Then
            sPing = "Destination Host Unreachable"
        Else
            sPing = "Alive"
        End If
    Next
    Set oPing = Nothing
    Set oRetStatus = Nothing
End Function

Sub GetComputerToPing()
    Application.DisplayAlerts = False
    'On Error Resume Next
    Dim applicationobject As Object
    Dim i As Integer

    i = 3 'row to start checking servers from
    Do Until Cells(i, 1) = ""
        'If Cells(i, 1) <> "" Then
            'If Cells(i, 2) = "Request Timed Out" Or Cells(i, 2) = "" Or Cells(i, 2) = "Dead" Then
                Cells(i, 2) = sPing(Cells(i, 1))
                Cells(i, 3) = Now()

                'log it to Log
                If Cells(i, 2).Value <> "Alive" Then
                    Call copytest(i)
                End If
            'End If
        'End If
        i = i + 1
    Loop
Set applicationobject = Nothing
End Sub

Function findlast_Row() As Long
    Dim ws As Worksheet
    Set ws = ThisWorkbook.Sheets("Log")

    With ws
        findlast_Row = .Range("A" & .Rows.count).End(xlUp).Row
    End With
End Function
Sub copytest(ByVal intRow As Integer)
    'screens for last row in log sheet
    iLastRow = findlast_Row() + 1
    Worksheets("Log").Range("A" & CStr(iLastRow) & ":E" & CStr(iLastRow)).Value = Worksheets("Servers").Range("A" & CStr(intRow) & ":E" & CStr(intRow)).Value
End Sub

Is there another way (or better way) to do the countdown timer?

Using Application.OnTime is a very neat way of implementing your timer, but I have a hard time figuring out how "pings servers every 15 mins or so" translates to Now + TimeValue("00:00:01"). Note that since VBA is single-threaded, OnTime doesn't mean your code will run at that specific time, rather that when Excel isn't busy doing something else, it will queue your method for synchronous execution, like any other event handler code.

That said, your naming and access modifiers lack consistency (I guess that's what you meant with "duct-taped"?).

Observations

You're using ListObject.ListRows.Count, but you're puzzling your way into adding a new row - why aren't you using ListObject.ListRows.Add() which returns the added row?
In sPing function (awful name), I would refactor this ElseIf construct into some Private Function GetPingResult(StatusCode As Long) As String, and assign the function's return value once; also you made the default value "Alive" - if there's an error code you haven't accounted for, your function returns "Alive" which is possibly wrong.

Nitpicks

This is VBA - stick to PascalCasing for all identifiers. It will make your code read consistently with the language itself.
The default access modifier for Sub and Function is Public - thus, either specify it or leave it out, but don't do both. If the unspecified ones are supposed to be Private, be explicit about it.
The language's convention for underscores in procedure names, is only for methods that implement interface methods (e.g. event handlers). Avoid it.
Worse than Hungarian notation (strTableName As String), is inconsistent Hungarian notation, especially in the same method signature (why isn't it iCol As Integer then?). Avoid it (Hungarian notation, that is!).

I know this is an incomplete review, but I think you have enough meat here anyway.


Find only unique number in vector Find the number occuring only once in a vector.
Given a vector A, containing N pairs (1, 1, 2, 3, 3 ... etc.) and one unique number, find and return the one unique number.

I wrote the following code for this problem.

As I don't have a lot of experience I would be grateful for any kind of feedback regarding my solution so it can be improved.

#include <iostream>
#include <unordered_map>
#include <vector>

uint32_t find_unique(std::vector<uint32_t> &A)
{
    std::unordered_map<uint32_t, uint32_t> seen;

    for (const auto &it : A)
    {
        if (++seen[it] > 1)
        {
            seen.erase(it);
        }
    }

    return seen.begin()->first;
}

int main()
{
    std::vector<uint32_t> v{1, 1, 6, 2, 2, 3, 3, 4, 4, 5, 5};

    std::cout << find_unique(v) << "\n";

    return 0;
}

You can use a more efficient algorithm. It's clear that the unique value is equal to the xor of all numbers in the vector. One can write a nice one-liner to compute it:

std::accumulate(v.begin(), v.end(), (uint32_t)0, [](uint32_t a, uint32_t b) { return a ^ b; })

The time complexity is the same (as a hash table works in O(1) on average), but this code is likely to work faster in practice. It also requires O(1) extra memory, while your solution needs O(n), so it's more space efficient.

I'd also say that your code is little bit confusing (it inserts keys implicitly via [] operator but erases them explicitly). It took me a while to figure out what's going on. I think that the solution with a set is a little bit more clear as it makes all insertions and deletions explicitly:

unordered_set<uint32_t> seen;
for (const auto& elem : v) {
     if (seen.count(elem))
         seen.erase(elem);
     else
         seen.insert(elem);
}
return *seen->begin();

You can also omit return 0 in the main function (it returns 0 by default, anyway).


Simple counter jsx component This is my first ES6 & JSX React Component. It's just a simple counter with disabling/enabling buttons on certain conditions with notice.

I would like to know if this code is OK or if you could've done something another way. I want to avoid bad practices right from the beginning.

import React from 'react';

export default class Counter extends React.Component{
  constructor(props) {
    super(props);
    this.state = {
      number: 0,
    };

    this.decrement = this.decrement.bind(this);
    this.increment = this.increment.bind(this);
  }

  decrement() {
    this.setState({
      number: this.state.number - 1
    })
  }

  increment() {
    this.setState({
      number: this.state.number + 1
    })
  }

  disableIncrement() {
    if(this.state.number >= 10) {
      return true;
    } else {
      return false;
    }
  }

  disableDecrement() {
    if(this.state.number <= 0) {
      return true;
    } else {
      return false;
    }
  }

  notice() {
    if(this.disableIncrement()) {
      return 'You\'ve reached N° 10.';
    }
    if(this.disableDecrement()) {
      return 'You can\'t decrement. You\'re at N° 0';
    }
    return 'You can do both.';
  }

  render() {
    return(

{this.state.number}

        <button disabled={this.disableIncrement()} onClick={this.increment}>+</button>
        <button disabled={this.disableDecrement()} onClick={this.decrement}>-</button>

{this.notice()}

    )
  }
}

First, you need to bind this to the notice function as well

this:

if(this.state.number <= 0) {
  return true;
} else {
  return false;
}

Can be written as:

this.state.number <= 0

as It will return those booleans anyway

in which case you could write:

notice() {
  const { number } = this.state

  if(number >= 10) {
    return 'You\'ve reached N° 10.';
  }

  if(number <= 0) {
    return 'You can\'t decrement. You\'re at N° 0';
  }

  return 'You can do both.';
}

Note the destructuring pattern:

const { number } = this.state
// is the same as
const number = this.state.number


Timsort implementation in Python I wanted to implement a Timsort in python.

Here's what I've come up with:

from typing import List

RUN = 32

def insertionSort(unsorted: List[int], left: int, right: int) -> None:
    """ Sorts the array using insertion sort """

    for i in range(left + 1, right + 1):
        temp = unsorted[i]
        j = i-1

        while unsorted[j] > temp and j >= left:
            unsorted[j+1] = unsorted[j]
            j -= 1

        unsorted[j+1] = temp

def merge(unsorted: List[int], left: int, middle: int, right: int) -> None:
    """ Merges two sorted arrays """

    len1 = middle - left + 1
    len2 = right - middle

    left_side  = unsorted[left : left + len1]
    right_side = unsorted[middle + 1 : middle + len2 + 1]

    i = j = 0
    k = left

    while i < len1 and j < len2:
        if left_side[i] <= right_side[j]:
            unsorted[k] = left_side[i]
            i += 1
        else:
            unsorted[k] = right_side[j]
            j += 1

        k += 1

    while i < len1:
        unsorted[k] = left_side[i]
        i += 1
        k += 1

    while j < len2:
        unsorted[k] = right_side[j]
        j += 1
        k += 1

def timSort(unsorted: List[int], length: int) -> None:
    """

    Sorts every 'RUN' elements by insertion sort as
            insertion sort is faster for smaller array lengths

    """

    # Sorts every RUN elements using insertion sort
    for i in range(0, length, RUN):
        insertionSort(unsorted, i, min(i + 31, length - 1))

    size = RUN

    # Merges every sorted subarray using Merge Sort
    while size < length:
        left = 0

        while left < length:
            mid = min(left + size - 1, length - 1)
            right = min(left + 2 * size - 1, length - 1)

            merge(unsorted, left, mid, right)

            left += size * 2

        size *= 2

if __name__ == '__main__':
    unsorted = list(map(int, input('Enter space seperated elements: ').split()))
    n = len(unsorted)

    timSort(unsorted, n)

    print('Here\'s the sorted array: ')
    print(*unsorted)

I think it looks good, but I want to make it even better.

PS: What would be the optimal length for RUN?

The easiest way to implement Timsort in Python is of course to use the built-in version:
def timsort(x):
    return sorted(x)

Since you don't learn anything from that, implementing it yourself is of course nice.
First, some general comments.

Python has an official style-guide, PEP8. It recommends using lower_case for variables and functions. It also recommends using empty lines sparingly.
You don't need to pass the length to the algorithm. Every indexable Python object has a length, so just use len(unsorted).
Your type hints say that this function can only take lists of integers. It works perfectly fine with lists of any comparable type, just like the built-in sorted.
Choosing to modify the input list in-place is perfectly fine for lists (after all, list.sort does the same thing). But if you do that, it should be noted in the docstring. You could, however, also choose to conform closer to sorted and first copy the iterable into a list (simply copying if it already is a list), sorting it in-place and returning it. This would change the signature to tim_sort(x: Iterable[Any]) -> List[Any].
The built-in sorting functions all take a key argument, which is called once per argument. This results in effectively sorting [key(element) for element in x] and afterwards returning only the second element from each inner tuple.

Now, let's take a closer look at your actual code.
You use while loops quite a lot. While (pun intended) they almost always work, they are not always the most efficient (in readability) way to accomplish your goal. In the merge function you can replace the last two while loops with list slice assignments. I would also simplify the setting of left_side and right_side by using left, middle and right instead of left, middle, len1 and len2.
def merge(unsorted: List[int], left: int, middle: int, right: int) -> None:
    """ Merges two sorted arrays """
    len1 = middle - left + 1
    len2 = right - middle
    left_side  = unsorted[left:middle + 1]
    right_side = unsorted[middle + 1:right + 1]

    i = j = 0
    k = left
    while i < len1 and j < len2:
        if left_side[i] <= right_side[j]:
            unsorted[k] = left_side[i]
            i += 1
        else:
            unsorted[k] = right_side[j]
            j += 1
        k += 1

    unsorted[k:] = left_side[i:]
    unsorted[k + len1 - i:] = right_side[j:]

Instead of slices, you can also create a new list and use extend:
def merge(unsorted: List[int], left: int, middle: int, right: int) -> None:
    """ Merges two sorted arrays """
    len1 = middle - left + 1
    len2 = right - middle
    left_side  = unsorted[left:middle + 1]
    right_side = unsorted[middle + 1:right + 1]

    i = j = 0
    k = left
    out = []
    while i < len1 and j < len2:
        if left_side[i] <= right_side[j]:
            out.append(left_side[i])
            i += 1
        else:
            out.append(right_side[j])
            j += 1
        k += 1
    out.extend(left_side[i:])
    out.extend(right_side[j:])
    unsorted[:] = out

The first while loop could also use iterators instead of indices, but that is left as an exercise for now.


Ajax function with a callback I have Ajax function with a callback which fetches all the Patient data however I have some perfomance issues and trying to figure out what it might be, does anyone have idea? That's my Code

var patient = new Array();
function sync(arg, callback){   //ajax result
    $('.loader').show();
    $.ajax({
        method: 'GET',
        url: 'sync/active.php',
        dataType: 'json',
        data: arg, // argument schicken und aktualisieren
        success: function(data, status, xhr){
         $('.loader').hide();
         callback(data);
        // setTimeout(sync, ajaxDelay);
        },
        error: function(xhr, ajaxOptions, thrownError){
            console.log(thrownError);
        }
   });
}
function onPatientCallback(data) {
        var res = data;
        for(var i=0; i<res.length;i++){
            for(var key in res[i]){
                var value = res[i][key];
                switch(key){
                    case "id":
                        res[i][key] = parseInt(value);
                        break;
                    case "kundennr":
                        res[i][key] = parseInt(value);
                        break;
                    case "client":
                        res[i][key] = value;
                        break;
                    case "start":
                        res[i][key] = new Date(value);
                        break;
                    case "end":
                        res[i][key] = new Date(value);
                        break;
                    case "title":
                        res[i][key] = value;
                        break;
                    case "description":
                        res[i][key] = value;
                        break;

                    case "termart":
                        res[i][key] = parseInt(value);
                        break;

                    case "userId":
                        res[i][key] = parseInt(value);
                        break;

                    default:
                        console.log("unbekannter Datentyp "+key);
                }
            }
        }
        patient = res;

}

I use the function to fill the patient variable and call it in another js file like that  sync({calling: "patient"}, onPatientCallback);

From a short review;

patient is a global variable, global variables are bad
var patient = [] is more idiomatic than var patient = new Array();
Comments should be all German or all English (I would go for all English, its the common language of the developers)
You can group switch labels, this is valid JavaScript:

        switch(key){
            case "id":
            case "kundennr":
            case "termart":
            case "userId":
                res[i][key] = parseInt(value);
                break;
            case "client":
            case "title":
            case "description":
                res[i][key] = value;
                break;
            case "start":
            case "end":
                res[i][key] = new Date(value);
                break;
            default:
                console.log("unbekannter Datentyp " + key);

Your data fields should be either all English or all German (you have userId, but also kundennr), here as well I would go for all English
Since value already contains res[i][key] for "client", "title", etc., you should probably comment on that
Since this is real, you should not log to console for an unknown data type, you should call a REST service that will log this entry in a database table, and even possibly send out an email to developers
var key in Object.keys(res[i]) is safer than var key in res[i], you never know when someone decides to extend Object
Your indenting is off, consider using a beautifier


Laravel transactions with a database I'm really concerned about my transaction with a database and my code structure. I have created more methods to insert, update, delete data to the database but sometimes some of them will break or miss. That's why I try to use Laravel transaction and PHP catch and throw exceptions to rollback and avoid any errors happening during those method processes.

By they way, I don't yet understand how try/catch and Laravel transaction catch those errors and return or throw to the end user.

However, I've tried to find all kind of errors will happen and how I can throw those error types to the end users in the catch block.

This function work find but I want to check if any errors happen and how I can return those type of errors to the end user by using Laravel transaction and try.

public function Dosomething($id)
    {
     if (Request::ajax()) {
        $data = [
          'till_account_id' => Request::input('till_account_id'),
          'from_account' => Request::input('from_account'),
          'to_account' => Request::input('to_account'),
          'till_user_id' => Request::input('till_user_id'),
          'branch_id' => Request::input('branch_id'),
          'operate_by' => Request::input('operate_by'),
          'type' => Request::input('type'),
         'tranx_time' => date('Y-m-d H:m:s', time())
       ];
      DB::beginTransaction();
     try {
         $update = Teller::where('id', '=', $id)->update(['balance' => Request::input('balanceCommision')]);
         if (!empty($update)) {
             $data['cash_out'] = Request::input('cash_out');
             $trans_cashIn = DB::table('till_transaction')->insertGetId($data);

             $data['cash_in'] = Request::input('cash_in');
             $data['cash_out'] = 0;
             $trans_cashOut = DB::table('till_transaction')->insertGetId($data);
        }
            $test = DB::commit();
            return ['up' => $update, 'insCashIn' => $trans_cashIn, 'insCashOut' => $trans_cashOut, 'data' => $data, 'cash_out' => $data['cash_out'],'commit'=>$test];
       } catch (Exception $e) {
           DB::rollBack();
           throw $e;
      }
   }
}

Overall, you've got decent structure there. However, there are a few things I can see from my limited experience.

You defined an associative array $data at the beginning. From experience, it usually works best to define it fully up front instead of in bits and pieces. This is not required in PHP like it is in Java, but it usually works better because it keeps your code looking clean and organized. It's hard to know what exactly is in your array if you add to it in multiple places.

From what I can see, you don't have any specific delimiters or checks to see if the inputs exist, this should be the first thing you do. If something is null, there should be red flags going up.

That being said, inside the try statement you insert into the database twice. Both times you pass $data to the insertGetId() magic method. While you do want to pass an associative array, you must make sure that all required fields are in that array. Effectively, you're duplicating your data with just an extra column.

Reading your $data array, you have a column named 'till_account_id'. If I understand the project correctly, this field should be unique as no one person should share an account. There may be cosigners, but it isn't technically their account. That being said, I think the 'till_account_id' should be your primary key, which means your insert should look like this:

DB::table('till_transaction')->insertGetId($data,'till_account_id');

Be very careful when you return data. You return several variables defined inside of an if statement. If the update fails, it will return null which means !empty($update) will fail because you're trying to read a null object. The same goes for further down, when you return, all the values that were defined inside the if statement wouldn't exist, thus returning null.

Good job catching the exception and then rethrowing it to the caller. Most people would catch it and forget to handle it. That being said, though, depending on how specific of an app you're developing, it might be beneficial to make a custom exception class and do something like this:

catch (Exception $e) {
    //respond here as needed
    throw new CustomException('some message describing error',0,$e);
}

This would do whatever you need to do, then rethrow the exception in a more manageable fashion. You would have a message specific to what you need to explain - details are very important in debugging, you would have the error code, and you would have the previous exception passed to the new exception for the trace.


Deserialize string into property array of the deserializing type using json.net I have the input JSON

{"QtyEqualsQtyWorked":0,"ReasonCode":"IN"}

and my class is

public class LoadForMovementOpt
{
    public bool QtyEqualsQtyWorked { get; set; }
    public string[] ReasonCode { get; set; }
}

The property ReasonCode has to be array for some reasons I cannot argue with, but in json it can be both string and string[]. So I wrote a custom converter that actually works, but maybe it can improved:

class LoadForMovementConverter : JsonConverter
{
    public override bool CanConvert(Type objectType) { return true; }
    public override bool CanWrite
    {
        get
        {
            return false;
        }
    }
    public override bool CanRead
    {
        get
        {
            return true;
        }
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) { }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        JToken token = JToken.Load(reader);
        var reasonToken = token["ReasonCode"];

        if(reasonToken == null)
        {
            return token.ToObject(objectType);
        }

        if(reasonToken.Type == JTokenType.Array)
        {
            return token.ToObject(objectType);
        }
        else
        {
            var QtyEqualsQtyWorkedToken = token["QtyEqualsQtyWorked"];
            return
            new LoadForMovementOpt
            {
                QtyEqualsQtyWorked =
                    QtyEqualsQtyWorkedToken
                        .ToObject<bool>(),
                ReasonCode =
                    new string[]
                    {
                        reasonToken.ToObject<string>()
                    }
            };
        }
    }
}

Is there any way to improve it?

Do you have access the JsonReader and JToken classes? If so, consider the following APIs. There is a lot of logic tied into the current ReadJson method which could be (IMO) better separated out of there. You would of course have to create/implement/override
the appropriate base classes and methods: Notice how a lot of the clutter is separated and moved out to a factory with the following APIs? And the respective JTokens will know exactly what to do when you send the ToObject method to it - there is no need to check for the type, or to check if it's an array because the factory will take care of that logic. The only negative is that it might be a little annoying if you have to roll your own JsonReader implementation.

// A so-so API that is marginally better
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    JToken token = JToken.LoadMovementConverter(reader);
    token.ToObject();
}

    // even more ideal API:
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
    JToken token = reader.GetMovementConverterJToken();
    token.ToObject();
}

    // create new JToken types:
public class JTokenNullReason : JToken
{
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

public class JTokenArray : JToken
{
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

public class JTokenEqualsQtyWorked : JToken
{
    public override object ToObject()
    {
        throw new NotImplementedException();
    }
}

And then the abstract class something like this:

public abstract class JToken
{
    public abstract object ToObject();

    public static JToken Load(JsonReader reader )
    {
            // not ideal because an abstract ReasonCode class instance method
            // could return the required JToken via a factory method.
            // you could certainly refactor this code, or, even better:
            // ask the reader itself to get you the correct JToken.

          string reasonCode = reader.GetReasonCode();
          switch (reasonCode)
          {
              case null:
                  return new JTokenNullReason();
                  break;
              case "Array":
                  return new JTokenArray();
                  break;
              default:
                  return new JTokenEqualsQtyWorked();
                  break;
          }
    }
}


Optimize sorting of Array according to Distance (CLLocation) Currently my below code takes an average 9.27300 every time I run it. This is an iOS application and I can't expect the user to sit there for roughly 10 seconds.

This code takes an array (originalData) of placeObjects and calculates the distance from the selected location (CLLocation* searched). It does by [searched distanceFromLocation:currentLoc];
Each time the distance is calculated, the distance is stored into keysArray.
After the first for loop the numbers are then sorted from ascending order.

After that another for loop is made to line up the originalData to follow the same ascending order as the keysArray.

A side note: I am currently doing this for 500 objects in originalData.

I am calling this method [self sortLocations]; from viewDidLoad.

-(void)sortLocations{
    NSLog(@"Start");
    [keysArray removeAllObjects];
    //originalData is an NSMutableArray
    for (int i = 0; i < [originalData count]; i++) {
        /* f is global but its:
         * f = [[NSNumberFormatter alloc] init];
         * [f setNumberStyle:NSNumberFormatterDecimalStyle];
         */
        NSNumber* cLat = [f numberFromString:[[originalData objectAtIndex:i] latitude]];
        NSNumber* cLng = [f numberFromString:[[originalData objectAtIndex:i] longitude]];
        CLLocation* currentLoc = [[CLLocation alloc] initWithLatitude:[cLat doubleValue] longitude:[cLng doubleValue]];
        //searched is a CLLocation
        CLLocationDistance distance = [searched distanceFromLocation:currentLoc];
        NSNumber* num = [NSNumber numberWithInt:distance];
        [keysArray addObject:num];
    }
    //lowToHight is this:
    // lowToHigh = [NSSortDescriptor sortDescriptorWithKey:@"self" ascending:YES];
    [keysArray sortUsingDescriptors:[NSArray arrayWithObject:lowToHigh]];
    [orderedArray removeAllObjects];
    for (int i = 0; i < [keysArray count]; i++) {
        NSNumber* keyNum = [keysArray objectAtIndex:i];
        for (placeObjects* x in originalData){
            NSNumber* cLat = [f numberFromString:[x latitude]];
            NSNumber* cLng = [f numberFromString:[x longitude]];
            CLLocation *currentLoc = [[CLLocation alloc] initWithLatitude:[cLat doubleValue] longitude:[cLng doubleValue]];
            CLLocationDistance distance = [searched distanceFromLocation:currentLoc];
            NSNumber* num = [NSNumber numberWithInt:distance];
            if ([num intValue] == [keyNum intValue]) {
                [orderedArray addObject:x];
            }
        }
    }
    NSLog(@"End");
}

Winston's answer points out some improvements to your algorithm.  There are other ways in which we can slightly improve the speed of this.

In Objective-C, for loops (and while and do...while) loops are handled one iteration at a time and the exit condition is checked on each iteration of the loop.  Meanwhile, a forin loop is handled in batches.  When we're dealing with an array for example, a forin loop might cache the first 8 objects and work with them before it goes back and grabs more objects.  In this way, we save time by calling fewer messages on the collection.  Moreover, when it caches these objects, you've already got a reference to the object, so you don't have to waste yet another message to the collection to grab a reference to the object.  Long story short, forin loops are faster than for loops in Objective-C, and you should use them when you can, even if you don't need the reference to the objects in the collection...

In this case though, we're actually wasting tons of execution time making calls back to the collection.

Moreover, NSString has a method called doubleValue which shouldn't have any trouble pulling a double value out of a string...

So let's clean up the first loop to look like this:

for (id object in originalData) {
    double latitude = [[object latitude] doubleValue];
    double longitude = [[object longitude] doubleValue];
    CLLocation* currentLoc = [[CLLocation alloc] initWithLatitude:latitude
                                                        longitude:longitude];

    CLLocationDistance distance = [searched distanceFromLocation:currentLoc];
    NSNumber* num = [NSNumber numberWithInt:distance];
    [keysArray addObject:num];
}

There are some things that are curious to me however.

What's the data type of the objects in the array?  Why does it have properties latitude and longitude which you clearly need to do math things with yet they're stored as NSString objects?  These should be stored as NSNumber objects or just double primitives preferably.

CLLocationDistance is a typedef for double.  Why are we then taking this double and instantiating an NSNumber object with numberWithInt:?


Get K most frequent tokens from a token stream This is an interview problem:

  Given a token stream, find the K most frequent tokens in real time (the order of these k tokens does not matter).

My approach:

Use a hash table (unordered_map in c++) to maintain the frequency of tokens.
Use a min heap to maintain the k most frequent tokens.
Update the hash table and the min heap dynamically.

I feel my code is not concise enough and I am not experienced using some new features in C++11. I would be very appreciative if anyone could any suggestions on the algorithm, coding style, object-oriented design, etc.

    class TokenCounter{
    public:
    void MaxKFrequent(int k) {
      std::unordered_map<std::string, TokenInfo::TokenInfoPtr> hash_table;
      std::deque<TokenInfo::TokenInfoPtr> min_heap;

      string token;
      while(std::cin >> token) {
        // update hashtable
        if(hash_table.find(token) == hash_table.end()) {
          hash_table[token] = TokenInfo::TokenInfoPtr(new TokenInfo(token));
        } else {
          hash_table[token]->increase_cnt();
        }

        // update min-heap
        TokenInfo::TokenInfoPtr& token_info = hash_table[token];
        // if already in the heap -> heaplify directly
        if(token_info->get_in_heap()) {
          std::make_heap(min_heap.begin(), min_heap.end(), TokenInfoCmp());
        } else {
        // if not in the heap
          if(min_heap.size() < k) {
          // if heap is not full -> insert into heap directly
            token_info->set_in_heap(true);
            min_heap.push_back(token_info);
            std::push_heap(min_heap.begin(), min_heap.end(), TokenInfoCmp());
          } else {
          // if heap is full
              // if count > heap top -> delete heap top & insert into heap
            if(token_info->get_cnt() > min_heap.front()->get_cnt()) {
              token_info->set_in_heap(true);
              min_heap.push_back(token_info);

              min_heap[0]->set_in_heap(false);
              min_heap.pop_front();

              std::make_heap(min_heap.begin(), min_heap.end(), TokenInfoCmp());
            }
          }
        }

        for(auto& token_ptr : min_heap) {
          std::cout << token_ptr->get_token() << " ";
        }
        std::cout << std::endl;
      }
    }

    private:
    class TokenInfo {
      public:
        using TokenInfoPtr = std::shared_ptr<TokenInfo>;
        TokenInfo(std::string token) : token_(token) {}

        int get_cnt() const {return cnt_;}
        void increase_cnt() {cnt_ += 1;}

        bool get_in_heap() const {return in_heap_;}
        void set_in_heap(bool in_heap) {in_heap_ = in_heap;}

        std::string get_token() const {return token_;}

      private:
        std::string token_;
        int cnt_ = 1;
        bool in_heap_ = false;
    };

    struct TokenInfoCmp {
      bool operator() (TokenInfo::TokenInfoPtr a, TokenInfo::TokenInfoPtr b) const {
        return a->get_cnt() > b->get_cnt();
      }
    };
  };

I feel my code is not concise enough

The feeling is mutual.

I feel that this can be expressed much more succinctly.

  I am not experienced using some new features in C++11.

You seem to be using range based for so not bad in that regards.

But. You do "probably" have a memory leak.

hash_table[token] = TokenInfo::TokenInfoPtr(new TokenInfo(token))
                                            ^^^^

Don't see a delete anywhere in the code. To be honest I doubt you actually need that new at all. If you just made your container hold objects all these problems would go away.

  I would be very appreciative if anyone could any suggestions on the algorithm, coding style, object-oriented design, etc.

Not sure that heap is the correct structure.

Sure it does sound correct initially. But you can increase the count of items that are already in the heap (which means you have to rebuild it from scratch).

I would just keep a sorted list of the top n items. When a count increases you at most need to move one item in the list.

I can't really review more than that because your code snipet is missing the most important thing. TYPE information.


2 Queens on N x N board I am trying to write an algorithm to solve a variation of the N queens problem. There are only 2 queens on an N x N board. I simply need to find the number of solutions, don't need to know what the solutions are.

Below is the code I have so far, and it works fine for N < 100000. However I need to solve it for N > 130456789 so I'm looking for feedback on how to make it more efficient.

public class Chess {

    long size;
    long n;
    long board;
    long cRow;
    long cCol;
    long maxRow;
    long maxCol;
    long safes = 0;

    public static void main(String[] args) {
        Chess a = new Chess(6);
        a.calculateMoves();
    }

public Chess(long n) {
        size = n * n;
        this.n = n;
        maxRow = n - 1;
        maxCol = n - 1;
    }

    public void calculateMoves() {
        iterate();
    }

    public void iterate() {
        for (long i = 0; i < n; i++) {
            for (long j = 0; j < n; j++) {
                cRow = i;
                cCol = j;
                calculateSafePlaces();
            }
        }
        System.out.println(safes/2);
    }

    private void calculateSafePlaces() {
        long notSafePlaces = (2 * n) - 1;

        notSafePlaces += Nw();
        notSafePlaces += Ne();
        notSafePlaces += Se();
        notSafePlaces += Sw();

        safes += (n * n) - notSafePlaces;
    }

    public long Nw() {
        long x = cRow;
        long y = cCol;

        if (x == 0) {
            return 0;
        }

        if (y == 0) {
            return 0;
        }

        if (x == y) {
            return x;
        }

        if (x > y) {
            return y;
        }

        if (y > x) {
            return x;
        }
        return 0;
    }

    public long Ne() {
        long x = cRow;
        long y = cCol;

        long count = 0;
        boolean condition = true;

        if (x == 0 || y == maxCol) {
            return 0;
        }

        long distance_to_east = maxCol - y;
        long distance_to_north = x;
        if (distance_to_north > distance_to_east) {
            return distance_to_east;
        }

        if (distance_to_east > distance_to_north) {
            return distance_to_north;
        }

        if (distance_to_north == distance_to_east) {
            return distance_to_east;
        }
        return 0;
    }

    public long Se() {
        long x = cRow;
        long y = cCol;

        if (x == maxRow) {
            return 0;
        }

        if (y == maxCol) {
            return 0;
        }

        long distance_to_south = maxRow - x;
        long distance_to_east = maxCol - y;

        if (distance_to_south > distance_to_east) {
            return distance_to_east;
        }

        if (distance_to_east > distance_to_south) {
            return distance_to_south;
        }

        if (distance_to_south == distance_to_east) {
            return distance_to_south;
        }
        return 0;
    }

    public long Sw() {
        long x = cRow;
        long y = cCol;

        long count = 0;
        boolean condition = true;

        if (x == maxRow) {
            return 0;
        }

        if (y == 0) {
            return 0;
        }

        long distance_to_south = maxRow - x;
        long distance_to_west = y;

        if (distance_to_south > distance_to_west) {
            return distance_to_west;
        }

        if (distance_to_west > distance_to_south) {
            return distance_to_south;
        }

        if (distance_to_south == distance_to_west) {
            return distance_to_west;
        }
        return 0;
    }
}

This can be done by hand in constant time using a bit of math. Just break the board up into two regions to place the first queen in:

The queen is in a corner/edge. For each corner, this eliminates the n-1 squares on the same row, column, and one or two diagonals, plus one for the one the queen is on. There are 4*(n-1) edge pieces, so there are 4*(n-1)*(n^2-(3(n-1)+1)) possibilities.
The queen is in the middle, not on an edge. Each of the (n-2)^2 positions that the queen can be in eliminates 4*(n-1)+1 squares (row, column, two diagonals, and the one square for the queen), for a total of (n-2)^2*(n^2-4*(n-1)+1).

Add these two and you should have your answer (assuming I didn't make any math mistakes).


Creating and manipulating a variant number of bufferedImages for vision algorithm I'm developing an algorithm that takes structured light patterns to determine 3D properties of objects.  This class builds the structured light objects.  It creates a buffered image, and based upon initialization parameters and the needed iteration step, manipulates the image to form a structured light pattern that will be be projected.  After the total space has been iteratively exhausted a 3D model of the surface will be constructed.  The projection process is then repeated.

The manipulation step of this class constructs different versions of the same image (the pixels will converge towards the center at each step).

Any suggestions on how I can speed this up, make it smoother, or more readable?

class ImageCollection implements Iterable<BufferedImage> {

private ArrayList<BufferedImage> ImageList = new ArrayList<>();
private static int width;
private static int height;
private static double pointXResolution;
private static double pointYResolution;
private static final Color red = new Color(255, 0, 0);
private static final Color green = new Color(0, 255, 0);
private static final Color blue = new Color(0, 0, 255);
private static final Color white = new Color(255, 255, 255);
private static final ArrayList<Color> colorList = new ArrayList<>();
private static double amountOfSets;

public ImageCollection(int numberOfRows, int numberOfColumns, double xResolution, double yResolution) {
    int typeIntRGB = 1;
    colorList.add(red);
    colorList.add(green);
    colorList.add(blue);
    colorList.add(white);
    ImageCollection.pointYResolution = Math.floor(yResolution / numberOfRows);
    ImageCollection.pointXResolution = Math.floor(xResolution / numberOfColumns);
    ImageCollection.width = (int) xResolution;
    ImageCollection.height = (int) yResolution;
    ImageCollection.height = (int) yResolution;
    int amountOfXPoints = numberOfRows;
    double amountOfYPoints = numberOfColumns;
    int rightPosition = width;
    int leftPosition = 0;
    ImageCollection.amountOfSets = Math.floor(height / (2 * (colorList.size()) * pointYResolution));
    for (int i = 0; i < amountOfSets; i++) {
        BufferedImage image = new BufferedImage(width, height, typeIntRGB);
        for (Color currentColor : colorList) {
            rightPosition-=pointXResolution;
            leftPosition+=pointXResolution;
            int otherPointer = 0;
            for (int k = 1; k < amountOfYPoints + 1; k++) {
                image.setRGB(leftPosition, (int) (k * pointYResolution), currentColor.getRGB());
                image.setRGB(rightPosition, (int) (k * pointYResolution), currentColor.getRGB());
            }
        }
        ImageList.add(image);
    }
}

@Override
public Iterator<BufferedImage> iterator() {
    Iterator<BufferedImage> nextImage = ImageList.iterator();
    return nextImage;
}
}

Skip the object wrapper. Generate the images list in a static function or a method and return it as is or return its iterator or stream if that is how you like it. Or return a custom iterator or stream that generates the images.

Consider re-factoring the looping code. Three loops is a bit too much in the same place. Extract the outer one to a function/method.

Several values are computed multiple times. '(k * pointYResolution)' and 'currentColor.getRGB()' for example. Store them in variables.

'ImageList' should be 'imageList' in java naming.

Add final to all variables that re not changing.

JavaDoc do you use it?

Make sure buffered image can hold the size of data you need. Some graphics buffers will scale up to a rectangle with a side power of 2 greater than what you ask and can potentially eat a lot of memory. I have no idea if this is applicable to this case here.


Does my controller code look good? I read many times about controller code mustn't be too complicated and so on. I was developing new features in my simple project. I added a function, which allow users to get access to news only in one specified category. Now, if a user writes some of this URL:

/news/common
/news/sport
/news/finance

only news from specified category would be shown.

I was thinking about how to do this through another actions, but realized that I can do it in index action. I need just to check if user entered category, but not id (id can
contain only digits), which I've done.

Controller:

public function indexAction() {
    $objectManager = $this->getServiceLocator()->get('Doctrine\ORM\EntityManager');

    $options = array();

    $categoryUrl = (string)$this->params('category');

    if($categoryUrl) { // add category to the 'where'
        $category = $objectManager
            ->getRepository('\News\Entity\Category')
            ->findOneByUrl($categoryUrl);
        if(!$category) {
            return $this->redirect()->toRoute('news');
        }
        $options['category'] = $category->getId();
        $categoryName = $category->getName();
    }

    $news = $objectManager
        ->getRepository('\News\Entity\Item')
        ->findBy($options, array('created'=>'DESC'));

    $items = array();
    foreach ($news as $item) {
        $buffer = $item->getArrayCopy();
        $buffer['category'] = $item->getCategory()->getName();
        $buffer['user'] = $item->getUser()->getDisplayName();
        $items[] = $buffer;
    }

    $view = new ViewModel(array(
        'news' => $items,
        'categoryName' => $categoryName,
    ));

    return $view;
}

What I am doing here?

receive category from URL

if category specified I add clause to the $options array and set $categoryName as category name
if category is not specified I don't do anything with $options (so it will be blank after this part) and don't set flag (so it is not set, undefined)

get news items (function pass $options array)
return $categoryName and news array to the view

View:

<?
    if($categoryName) {
        $title = $categoryName;
    } else {
        $title = "News list";
    }
    $this->headTitle($title);
?>
// html code, some conditions etc

There is an if condition. If $categoryName is specified, $title will have the same contents as $categoryName. If $categoryName is not specified, $title will be just News list.

Questions

Is this the correct approach at all? Should I create new actions and handle this case in it?
Is it correct to set flags, as I did, send to the view, handle it etc?
Is my controller "fat" now?
How can I improve this code?

In addition, you can find the full code of files on GitHub:

NewsController.php (controller)
index.phtml (view)

Note: Some words in files are in Russian.

On the surface the code in your question seems fine, but I can tell you from experience that once this door is cracked just a bit, it will continue to creak open wider over time.

"I'm just checking if we have an ID or category name."
"This just adds a few meta tags."
"It's already 200 lines; 50 more won't matter."

The linked controller, however, is doing way too much work. It should be passing data off to a model class (not the entity manager directly), placing whatever the view needs into the ViewModel, and that's it. Controllers are glue code. As you have it, you'll need to copy all of this code and modify it slightly to expose the CRUD interface in another form.

For the code you posted, I would prefer to separate the actions so each handles one specific use case: all items, items matching a category, and one item (not in the code but you mentioned it). Create a regex route for the last two. The beauty of this is that you don't need to do all the conditional checks--the dispatcher does it for you.

// see miscellaneous tips below
public function init() {
    $this->objectManager = $this->getServiceLocator()
        ->get('Doctrine\ORM\EntityManager');
}

public function allAction() {
    return new ViewModel(array(
        'news' => $this->loadItems(),
        'categoryName' => null,
    ));
}

public function categoryAction() {
    $category = $this->objectManager
        ->getRepository('\News\Entity\Category')
        ->findOneByUrl((string) $this->params('category'));
    if (!$category) {
        return $this->redirect()->toRoute('news');
    }
    return new ViewModel(array(
        'news' => $this->loadItems(array('category' => $category->getId())),
        'categoryName' => $categoryName,
    ));
}

private function loadItems($options = array()) {
    $news = $this->objectManager
        ->getRepository('\News\Entity\Item')
        ->findBy($options, array('created' => 'DESC'));
    $items = array();
    foreach ($news as $item) {
        $buffer = $item->getArrayCopy();
        $buffer['category'] = $item->getCategory()->getName();
        $buffer['user'] = $item->getUser()->getDisplayName();
        $items[] = $buffer;
    }
    return $items;
}

This is about the same length of the original, but it's far less complicated. Each action is easy to follow and clearly lays out what it requires.

Miscellaneous

And here are a few tips after looking at your linked controller and view code:

You are accessing the entity manager in every action (sometimes pulling it from the registry twice in the same method). Do this once by storing it in an instance property in init.
The index and list actions are nearly identical. Refactor these to extract the common code into a new private method. It looks like this applies to some of the other actions, e.g., converting a news item into an array for the view with its associated category and user names.
You can simplify the title-setting with the Elvis operator: $title = categoryName ?: "Список новостей".
Every page should have an H1--even the "all news" index page.
An non-empty array is truthy in PHP. if(count($this->news) != 0): can be shortened to if($this->news):. If you want to be explicit, at least use if(!empty($this->news)):.
If $this->news will be an empty array instead of null or false when there are no items, you don't even need the if since looping over an empty array is a no-op.
You don't need the ; in <?=...?> since it's an expression instead of a statement. You also don't need it for one-line statements in <?php ... ?>, but we still use it to avoid bugs when someone adds a line.


Full-page background in HTML I am trying to make a page background for my website. Is this a good way to do it?

This is my HTML:

And this is my CSS:

#page-background {
    position: fixed;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
}

The first small comment is that there's no need to put this extra div tag. Simply apply the page-background id to your img tag. It's still nice that you put it in the HTML code.

The other two issues are image ratio (do you want it to be preserved?) and browser support (willing to support IE6 and old Android and iPhones?)

position: fixed is not supported before iOS 5 and Android 4.0. Doesn't work on IE6 either. You could use a method that has been tested for every desktop browser since IE5 and will also work on any phone (tested on my old Android).
If you want to preserve image ratio and don't care about old desktop browsers, you could use the CSS3 background-size:cover property supported since IE9, Android 2.2 and iOS4.
If you want to preserve image ratio and support every browser, there's the usual JavaScript fallback. Use Modernizr to detect the CSS3 property and this jQuery script to simulate background-size: cover.


Inverse Document Frequency (IDF) implementation I would like to share my class IDF. Any suggestions/modification?

public class IDFMeasure
{
    private readonly string[] _docs;
    private readonly int _numDocs;
    private int _numTerms;
    private List<string> _terms;
    private float[][] _InverseDocFreq;
    private int[] _docFreq;

    private readonly Dictionary<string, int> _wordsIndex = new Dictionary<string, int>();

    public IDFMeasure(string[] documents)
    {
        _docs = documents;
        _numDocs = documents.Length;
        MyInit();
    }

    private List<string> GenerateTerms(string[] docs)
    {
        return docs.SelectMany(doc => ProcessDocument(doc)).Distinct().ToList();
    }

    private IEnumerable<string> ProcessDocument(string doc)
    {
        return doc.Split(' ')
            .GroupBy(word => word)
            .OrderByDescending(g => g.Count())
            .Select(g => g.Key)
    }

    private int GetTermIndex(string term)
    {
        return _wordsIndex[term];
    }

    private void MyInit()
    {
        _terms = GenerateTerms(_docs);
        _numTerms = _terms.Count;

        _docFreq = new int[_numTerms];

        for (var i = 0; i < _terms.Count; i++)
        {
            _wordsIndex.Add(_terms[i], i);
        }

        GenerateDocumentFrequency();
        GenerateInverseDocfrequency();
    }

    private float Log(float num)
    {
        return (float) Math.Log(num); //log2
    }

    private void GenerateDocumentFrequency()
    {
        _InverseDocFreq = new float[_numDocs][];

        for (var i = 0; i < _numDocs; i++)
        {
            _InverseDocFreq[i] = new float[_numTerms];

            var curDoc = _docs[i];
            var freq = GetWordFrequency(curDoc);

            foreach (var entry in freq)
            {
                var word = entry.Key;

                var termIndex = GetTermIndex(word);

                _docFreq[termIndex]++;
            }
        }
    }

    private void GenerateInverseDocfrequency()
    {
        for (var i = 0; i < _numDocs; i++)
        {
            var curDoc = _docs[i];
            var freq = GetWordFrequency(curDoc);

            foreach (var entry in freq)
            {
                var word = entry.Key;

                var termIndex = GetTermIndex(word);

                _InverseDocFreq[i][termIndex] = Log((_numDocs)/
                                                    (float) _docFreq[termIndex]);
            }
        }
    }

    private Dictionary<string, int> GetWordFrequency(string input)
    {
        return input.Split(' ').GroupBy(x => x)
            .OrderByDescending(g => g.Count())
            .ToDictionary(g => g.Key, g => g.Count());
    }
}

The layout and naming conventions look good.  Thank you for that.

_InverseDocFreq should be renamed _inverseDocFreq to keep with conventions and also stay consistent in your code.

I also think _terms should be an IList.  This just eliminates a bunch of the unneeded operations.

you are missing a ; after this statement in ProcessDocument

return doc.Split(' ')
            .GroupBy(word => word)
            .OrderByDescending(g => g.Count())
            .Select(g => g.Key)

I'm not sure why you are using MyInit, the constructor is the only place that is called.  I would move the code into the constructor, then you can make _numTerms, _terms, and _docFreq readonly.

I would rename _wordsIndex to _wordIndex because it is really only and index for each word.  I would also move the new into the constructor.

The for loop where you are filling the terms into the _wordIndex dictionary could be moved to its own method, I called it FillWordIndexWithTerms(), and it looks like

private void FillWordIndexWithTerms()
{
    for (var termIndex = 0; termIndex < _terms.Count; termIndex++)
    {
        _wordIndex.Add(_terms[termIndex], termIndex);
    }
}

I renamed i to termIndex to give it a bit better meaning in your code, and make it easier to read.

The constructor now looks like

public IDFMeasure(string[] documents)
{
    _docs = documents;
    _numDocs = documents.Length;

    _terms = GenerateTerms(_docs);
    _numTerms = _terms.Count;

    _docFreq = new int[_numTerms];
    _wordIndex = new Dictionary<string, int>();

    FillWordIndexWithTerms();
    GenerateDocumentFrequency();
    GenerateInverseDocfrequency();
}

In the GenerateTerms method, I would change the input parameter to an IEnumerable.  You could also use a method group in the linq statement, you don't need the lamba.  The function can also be made static.

private static IList<string> GenerateTerms(IEnumerable<string> docs)
{
    return docs.SelectMany(ProcessDocument).Distinct().ToList();
}

ProcessDocument looks good.

GetTermIndex looks good.

Log could be made static.  Other than that it looks good.

I would move the inner ForEach in GenerateDocumentFrequency to its own method.  This will reduce nesting.

private void GenerateDocumentFrequency()
{
    _inverseDocFreq = new float[_numDocs][];

    for (var i = 0; i < _numDocs; i++)
    {
        _inverseDocFreq[i] = new float[_numTerms];

        var curDoc = _docs[i];
        var freq = GetWordFrequency(curDoc);

        CalculateTermFrequency(freq);
    }
}

The ForEach can also be simplified using Linq.

private void CalculateTermFrequency(Dictionary<string, int> freq)
{
    foreach (var termIndex in freq.Select(entry => GetTermIndex(entry.Key)))
    {
        _docFreq[termIndex]++;
    }
}

In GenerateInverseDocfrequency I would rename i to currentDocument.  The nested ForEach should be moved to its own method, CalculateInverseDocumentFrequency.

private void GenerateInverseDocfrequency()
{
    for (var currentDocument = 0; currentDocument < _numDocs; currentDocument++)
    {
        var curDoc = _docs[currentDocument];
        var freq = GetWordFrequency(curDoc);

        CalculateInverseDocumentFrequency(currentDocument, freq);
    }
}

private void CalculateInverseDocumentFrequency(int currentDocument, Dictionary<string, int> freq)
{
    foreach (var termIndex in freq.Select(entry => entry.Key).Select(GetTermIndex))
    {
        _inverseDocFreq[currentDocument][termIndex] = Log((_numDocs)/
                                                            (float) _docFreq[termIndex]);
    }
}

GetWordFrequency could be made static.  Other than that it looks good.

Overall, nice code to read, the suggestions are just minor clean-up.  I really liked the way you used methods to separate different operations.


Move all zeros to the left with TDD approach The task

  Given an integer array, move all elements that are equal to 0 to the
  left while maintaining the order of other elements in the array. Let's
  look at the following integer array.

  After moving all zero elements to the left, the array should look like
  this. We need to maintain the order of non-zero elements.

0 0 0 1 10 20 59 63 88

My approach, I used TDD and hoped my solution would become just as good as the sample solution.

test('contains no element', () => {
  expect(moveZeroToLeft([])).toStrictEqual(undefined);
});

test('contains only a 0 element', () => {
  expect(moveZeroToLeft([0])).toStrictEqual([0]);
});

test('contains only a non 0 element', () => {
  expect(moveZeroToLeft([1])).toStrictEqual([1]);
});

test('contains more than one element with a 0', () => {
  expect(moveZeroToLeft([1, 0])).toStrictEqual([0, 1]);
});

test('contains more than one element without 0', () => {
  expect(moveZeroToLeft([1, 3])).toStrictEqual([1, 3]);
});

test('contains more than one element with a 0 somewhere in the middle', () => {
  expect(moveZeroToLeft([1, 0, 3])).toStrictEqual([0, 1, 3]);
});

test('contains random number of zeros and non-zero elements', () => {
  expect(moveZeroToLeft([1, 0, 3, 0, 0, 44, 1, 0, 2])).toStrictEqual([0, 0, 0, 0, 1, 3, 44, 1, 2]);
});

My solution

function moveZeroToLeft(arr) {
  if (arr.length < 2) {
    return arr;
  }
  if (arr.some(x => x === 0)) {
    let zeroCounter = 0;
    const res = arr.filter(el => {
      if (el !== 0) {
        return true;
      }
      zeroCounter++;
    });
    const leadingZeros = Array(zeroCounter).fill(0);
    return leadingZeros.concat(res);
  }
  return arr;
}

But the sample solution is much more elegant than mine:

let move_zeros_to_left = function(A) {
  if (A.length < 1) {
    return;
  }

  let lengthA = A.length;
  let write_index = lengthA - 1;
  let read_index = lengthA - 1;

  while (read_index >= 0) {
    if (A[read_index] != 0) {
      A[write_index] = A[read_index];
      write_index--;
    }

    read_index--;
  }

  while (write_index >= 0) {
    A[write_index] = 0;
    write_index--;
  }
};

How could I have come to the sample solution with a TDD approach?

TDD is not for finding "elegant" solutions, but for finding working solutions that cover all edge cases. Without edge cases, solutions are often too naive and in the end won't work.

You missed some edge cases; the task states: "order of other elements in the array" (that are integer). How about Infinity and -Infinity? Are they numbers? :) isNaN(Infinity)?

Another thing is, that beauty lies in the eye of the beholder. For me elegant code, is code, that is easy to grasp and not overly complex. Whenever a problem can be solved in such a generic way, that edge case do not need any special handling this is good code. :)

Speaking of which - your problem is clearly about transforming an array - with JavaScript you have a good set of Array.fns() at you disposal and fat arrow functions. Thus I'd intuitively never use loops, but only array functions.

The original problem sounds very constructed to me - I guess some stupid code interview stuff. I guess it is about thinking of how to find zeros in an array and move them around.

However the solution is quite trivial if you are thinking outside the box (which you are!), that with the elements being numbers, the solution is not about moving around zeros (and thus maintain their reference, if it where not zeros but objects), but in filtering and reconstructing them.

const move_zeros_to_left = function(unsortedArray) {
  const numberOfZeros = unsortedArray.filter(item => item === 0).length;
  const nonZeroArray = unsortedArray.filter(item => item !== 0);
  return [...Array.from({length: numberOfZeros}).fill(0), ...nonZeroArray];
}

Covers all the edge cases and is readable. Your provided sample solution is the worst of all solution, because it neglects the fact, that JavaScript has a rich set of Array functions and thus it is neither readable nor idiomatic and I'd say thus not elegant all. :)