C++ multi-threading demo I recently started learning C++ and here's a simple program I wrote to demonstrate how multi-threading work. This program simply fetches JSON strings from a whether API for a list of cities read from a text file.
Can you guys take a look? And I really appreciate professionals' views.

#include <iostream>
#include <ctime>
#include <typeinfo>
#include <thread>
#include <mutex>
#include <vector>
#include <fstream>
#include <string>
#include <curl/curl.h>

static size_t WriteCallback(void *contents, size_t size, size_t nmemb, void *userp);
void fetchJson(int);
void getCityList();
void fetch_weather_async();

const char *URL_BASE = "https://www.metaweather.com/api/location/search/?query=";
const char *CITY_LIST_FILE = "city_list.txt";
std::vector<std::string> cities;
int current_city = -1;
std::mutex city_counter_lock;
std::time_t start;
std::time_t finish;

int main(void){
    getCityList();
    std::cout << "Read " << cities.size() << " cities from file..." << std::endl;

    std::cout << "Using 2 threads to fetch..." << std::endl;
    std::thread thread_1(&fetch_weather_async);
    std::thread thread_2(&fetch_weather_async);

    start = std::time(nullptr);
    thread_1.join();
    thread_2.join();
    finish = std::time(nullptr);

    std::cout << "Duration " << (finish - start) << "s" << std::endl;

    return 0;
}

void fetch_weather_async(){
    while (current_city + 1 < (int) cities.size()){
        city_counter_lock.lock();
        int city_index = 0;
        if (current_city + 1 < (int) cities.size()) {
            city_index = ++current_city;
        }
        city_counter_lock.unlock();

        std::cout << "requesting " << city_index << std::endl;
        fetchJson(city_index);
        std::cout << "acquired " << city_index << std::endl;
    }
}

void getCityList(){
    std::ifstream idFile(CITY_LIST_FILE);
    std::string currentLine;

    if(idFile.is_open()){
        while(! idFile.eof()){
            getline (idFile, currentLine);
            cities.push_back(currentLine);
        }
    }
    idFile.close();
}

void fetchJson(int i){
    CURL *curl;
    CURLcode res;
    std::string readBuffer;

    curl = curl_easy_init();
    if(curl) {
        curl_easy_setopt(curl, CURLOPT_URL, (URL_BASE + cities[i]).c_str());
        curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteCallback);
        curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer);
        res = curl_easy_perform(curl);
        curl_easy_cleanup(curl);

        std::cout << i << " : " << readBuffer << std::endl;
    }
}

static size_t WriteCallback(void *contents, size_t size, size_t nmemb, void *userp)
{
    ((std::string*)userp)->append((char*)contents, size * nmemb);
    return size * nmemb;
}

The city list file (city_list.txt) contains:

Adelaide
Amsterdam
Ankara
Athens
Atlanta
Auckland
Baghdad
Bangkok
Beijing
Belfast
Berlin
Blackpool
Bridgeport
Brussels
Bucharest
Budapest
Busan
Cairo
Caracas
Cardiff
Casablanca
Charlotte
Copenhagen
Dallas
Damascus
Dhaka
Dublin
Edinburgh
Glasgow
Helsinki
Jakarta
Kiev
Kinshasa
Lagos
Lahore
Lima
Lisbon
London
Madrid
Manila
Milan
Minsk
Moscow
Nairobi
Newark
Newcastle
Oakland
Oslo
Paris
Philadelphia
Plymouth
Portland
Portsmouth
Prague
Pyongyang
Riyadh
Rome
Santander
Santiago
Santorini
Seoul
Singapore
Sofia
Stockholm
Sunderland
Taipei
Tokyo
Vienna
Warsaw
Wellington
Windhoek
Zagreb

Prefer using a lock guard:

int city_index = 0;
{
    std::lock_guard<std::mutex> guard(city_counter_lock);
    if (current_city + 1 < (int) cities.size()) {
        city_index = ++current_city;
    }
}

This makes sure that an exception will not leave the mutex locked;

You will need to handle failures to get the webpage. If res != CURLE_OK then a failure occurred somewhere.

You are encouraged to reuse the CURL handle you can do so by creating it inside fetch_weather_async before the loop and passing it into fetchJson

void fetch_weather_async(){
    CURL *curl;

    curl = curl_easy_init();

    if(!curl) return;
    while (current_city + 1 < (int) cities.size()){
        int city_index = 0;
        {
            std::lock_guard<std::mutex> guard(city_counter_lock);
            if (current_city + 1 < (int) cities.size()) {
                city_index = ++current_city;
            }
        }

        std::cout << "requesting " << city_index << std::endl;
        fetchJson(city_index, curl);
        std::cout << "acquired " << city_index << std::endl;
    }
    curl_easy_cleanup(curl);
}

void fetchJson(int i, CURL *curl){
    CURLcode res;
    std::string readBuffer;

    curl_easy_setopt(curl, CURLOPT_URL, (URL_BASE + cities[i]).c_str());
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteCallback);
    curl_easy_setopt(curl, CURLOPT_WRITEDATA, &readBuffer);
    res = curl_easy_perform(curl);
    if(res == CURLE_OK)
        std::cout << i << " : " << readBuffer << std::endl;

}


this is my realloc version, is it good ? any suggestion are welcomed I am trying to recode some c functions myself.
This is an my implementation of realloc function, any updates or suggestion about how to improve it are welcomed.
void    *ft_realloc(void *old_ptr, size_t old_size, size_t size) {
    void *new_ptr;

    if (size == 0 && old_ptr != NULL)
    {
        free (old_ptr);
        return (NULL);
    }
    else if (old_ptr == NULL)
    {
        if (!(old_ptr = malloc (size)))
            return (NULL);
        ft_memset(old_ptr, 0, size);
        return (old_ptr);
    }
    else if (size >= old_size) {
        if (!(new_ptr = malloc (size)))
            return (NULL);
        ft_memset(new_ptr, 0, size);
        ft_strncpy (new_ptr, old_ptr, old_size);
        free (old_ptr);
        return (new_ptr);
    }
    else if (size < old_size)
    {
        if (!(new_ptr = malloc(size)))
            return (NULL);
        ft_memset(new_ptr, 0, size);
        ft_strncpy (new_ptr, old_ptr, size);
        free (old_ptr);
        return (new_ptr);
    }
    return (NULL);
}

ft_memset and ft_strncpy are just my implementation of memset and strncpy they behave the same way!

#include <stddef.h>
#include <stdlib.h>

void    *ft_realloc(void *old_ptr, size_t old_size, size_t size) {
    if (size == 0 && old_ptr != NULL)
    {
        free (old_ptr);
    }
    else if (old_ptr == NULL)
    {
        if (old_ptr = malloc(size))
        {
            ft_memset(old_ptr, 0, size);
            return (old_ptr);
        }
    }
    else
    {
        void *new_ptr;
        if (new_ptr = malloc(size))
        {
            ft_memset(new_ptr, 0, size);

            if (size >= old_size)
                ft_strncpy (new_ptr, old_ptr, old_size);
            else
                ft_strncpy (new_ptr, old_ptr, size);

            free (old_ptr);
            return (new_ptr);
        }
    }
    return (NULL);
}

My insightful observation: OP's code has redundancies.
My answer: I removed some redundancies, which makes the code possible faster and easier to maintain.


Thread-safe singleton cache (Python) In a service I'm developing I'm using three cloud services (five if you count additional two streaming services I have to create that are used by those cloud services in turn) and to orchestrate bits and pieces incoming I need a cache for objects relating data from all those services. That cache has to be is safe to access from various Tornado handlers in an atomic manner.

Now, since Tornado is based on I/O loop, theoretically I do not need to use locks or some other synchronization primitives... but that's obviously relying on implementation detail (GIL) and it can reportedly cause problems under some circumstances anyway, so to be rather safe than sorry I try to develop the cache that is all:

Singleton
Thread-safe
Creates a default object available under a key

I'm approaching the subject with some trepidation as so far I have not done such stuff much. This is what I've come up with so far:

UPDATE

I've realized I had a stupid mistake in the previous implementation: the setdefault method called default_factory every time. Updated version works but it's kinda ugly:

class ThreadSafeSingletonCache(object):
    __me = None
    __cache = None
    __lock = threading.Lock()

    def __new__(cls, _):
        if cls.__me is None:
            cls.__cache = {}
            cls.__me = super(ThreadSafeSingletonCache, cls).__new__(cls)
        return cls.__me

    def __init__(self, default_factory):
        self.default_factory = default_factory

    def obj_setattr(self, cache_key, obj_attr, value):
        with self.__lock:
            item = self.__like_setdefault(cache_key)
            setattr(item, obj_attr, value)

    def __like_setdefault(self, cache_key):
        ''' Unfortunately I cannot use .setdefault as default_factory function gets called on each setdefault use'''
        item = self.__cache.get(cache_key)
        if not item:
            item = self.default_factory()
            self.__cache[cache_key] = item
        return item

    def get(self, cache_key):
        with self.__lock:
            return self.__like_setdefault(cache_key)

    def set(self, cache_key, obj):
        with self.__lock:
            self.__cache[cache_key] = obj

Please point out problems and potential improvements.

I would consider using get(cache_key) within obj_setattr, rather than re-implementing it.

Another thing to look at would be to use a decorator to activate the lock, instead of writing the with every time. This would scale better in case you want to do some preprocessing / postprocessing later in time.


Signal simulation through Python scripts using shared memory for testing a C application This is bit long, but I need to explain something before I can ask for actual reviews/advice.

I have to test generated C code from Matlab Simulink model. I can create a executable binary from those.  I need to test this application using tests developed using Python scripting.

I came up with following approach: inter-process communication using shared memory.

I planned to write a DLL or a shared object, which enables the communication between the tests scripts and the application.  This DLL will be used via both Python scripts and application to interact with each other

Here's my DLL code, which uses boost::interprocess shared memory.

int create_shmem(const char* shmem_name, long long length  )
{
    try{
       // remove_shmem(shmem_name);
        boost::interprocess::shared_memory_object shm
                          (boost::interprocess::create_only,
                           shmem_name,
                           boost::interprocess::read_write);
      //Set size
      shm.truncate(length);
      return 1;
      }
      catch(...){
        std::cerr << "Error Creating Shared Memory : " << shmem_name << std::endl;
      }
      return 0;
}

int remove_shmem(const char* shmem_name )
{
    try {
        boost::interprocess::shared_memory_object::remove(shmem_name);
        return 1;
    }
    catch(...){
        std::cerr << "Error Removing Shared Memory : "<< shmem_name << std::endl;
    }
    return 0;
}

int write_shmem(const char* shmem_name, char* shmem)
{
      try{
            boost::interprocess::shared_memory_object shm
            (boost::interprocess::open_only, //Open if already created
                 shmem_name,
             boost::interprocess::read_write);

      //Map the whole shared memory in this process
            boost::interprocess::mapped_region region
                (shm,
                boost::interprocess::read_write
                );

            std::memcpy(region.get_address(), shmem, region.get_size());
            return 1;
      }
      catch(...){
         std::cerr << "Error Writing to Shared Memory " << shmem_name << std::endl;
      }
      return 0;
}

int read_shmem( const char* shmem_name, char *shmem )
{
      //Open already created shared memory object.
     // Same process might not be calling it, so create new object
     try{
            boost::interprocess::shared_memory_object shm (
                boost::interprocess::open_only,
                    shmem_name,
                    boost::interprocess::read_only);

      //Map the whole shared memory in this process
            boost::interprocess::mapped_region region
                (shm,
                     boost::interprocess::read_only
                );

      //CAUTION : No check is performed
    std::memcpy(shmem, reinterpret_cast<char*> (region.get_address()),
                       region.get_size());

      return 1;
      }

      catch(...){
      std::cerr << "Error Reading from Shared Memory " << shmem_name << std::endl;
      }

   return 0;
}

The application needs simulation of around 1.5K different signals of different data types all packed in a C struct.

My initial though is to created a shared memory and write the simulated signals on to it which the application can fetch and accordingly shows the output by writing back to same shared memory which is again read from test scripts.

Any help/advice regarding underlying issues/failure and improvement will be much appreciated.

I can post my ctypes Python interface if required.

It is short and to the point. Looks fine.

  used via both Python scripts and application to interact with each other

You didn't describe the interaction, in English nor in code. Maybe the two sides routinely manipulate lots of data structures (1500 signals didn't quite sound like "a lot") that the other side uses less than 1% of the time, so your code or the multiprocessing support that ppperry highlighted would be appropriate. But the interaction is going to require condition variables or mutexes or higher level coordination such as queues. Since you've not specified the performance characteristics, I would just start with a TCP connection, or pub-sub, and go from there, see how things shake out. Localhost TCP offers very high bandwidth, and low latency. Profiling might reveal a need for shmem, but I'm skeptical, and it's better to cross that bridge after it's clear that you need to.


The importance of consistency in percentage returns when investing It is said that

  Being consistent in percentage annual returns leads to larger amount over time.

That is for any given principal, getting 20% successively for 2 years is better than 30% for first year and 10% for second year even though both percentages add to 40. First case yields 44% while the later one yields 43%.

I have tried to simulate this for arbitrary number of years and arbitrary percentages using Python 3. The obvious added constraint is that sum of percentages is constant (otherwise, inconsistent arbitrarily high percentages will obviously beat consistent low percentages).

from operator import mul
from functools import reduce
import numpy as np
from collections import namedtuple
from typing import List, Tuple

def avg_cagr(percents: List[int]) -> float:
    '''Given (successive) % annual growth rates, returns average Compound Annual Growth Rate'''
    amount = reduce(mul, [1+p/100 for p in percents])
    return (amount**(1/len(percents)) - 1)*100

def dirichlet(n: int = 5, amount: float = 100) -> List[float]:
    '''Returns n random numbers which sum to "amount"'''
    random_returns = np.random.dirichlet(alpha=np.ones(n), size=1)[0]*amount
    return random_returns

def simulate_returns(n: int = 5, amount: float = 100, iterations: int = 40) -> List[Tuple[float, float]]:
    '''Generate bunch of random percentages that sum to a constant and compare average CAGR with their deviation'''
    net_returns = namedtuple('net_returns', 'deviation CAGR')
    results = []
    for _ in range(iterations):
        random_returns = dirichlet(n, amount)
        avg_returns = avg_cagr(random_returns)
        deviation = np.std(random_returns)
        results.append(net_returns(deviation, avg_returns))
    return sorted(results, key=lambda x: x.CAGR, reverse=True)

if __name__ == '__main__':
    results = simulate_returns()
    print('\n'.join(('dev=' + str(round(result.deviation, 2)) + ', ' + 'CAGR=' + str(round(result.CAGR, 2))) for result in results))

Key Ideas in the code:

Dirichlet distribution is used to generate bunch of random numbers that sum to a constant
Standard deviation is used to illustrate consistency

Result: Simulation agrees with maxim.

For me the code looks rather okay, I have just a few small remarks:

 Formatting

The line

print('\n'.join(('dev=' + str(round(result.deviation, 2)) + ', ' + 'CAGR=' + str(round(result.CAGR, 2))) for result in results))

is very hard to read, could you possibly split it or use a function?

def format_result(result):
    dev = round(result.deviation,2)
    cagr = round(result.CAGR,2)
    return f'dev={str(dev)}, CAGR={str(cagr)}'

and then just

print('\n'.join(format_result(result) for result in results))

 Performance

I'm unsure whether you need an extreme performance but it seems that potentially you might want to run the code large number of iterations.

namedtuple

namedtuple datastructure is extremely handy if you want to use it to make your code looks better. However, if you care about performance (i.e. you will be using many of them) you might be better off with usual tuple.
Reference: link1, link2, link3.

list.append

In this section I'll be referring to results.append(net_returns(deviation, avg_returns)). append is rather a slow operation if you know a list size before the appending. I think your code would be faster if you allocated results list first and then just put each net return on it's place.

reverse=True

As tempting it is, it might have a performance hit as well. As one might see in source code the list is actually reversed to keep the stability. It should be better just to pass a negative value to your lambda:

return sorted(results, key=lambda x: -x.CAGR)


Yet another quicksort Code review my quicksort! It isn't supposed to be superfast or anything and doesn't basecase out to insertion sort or anything. It's optimized for clarity and only sorts arrays of pointers.

#define MISC_XCHG(a, b) ({ __typeof__(a) MISC_XCHG__A = (a); (a) = (b); (b) = MISC_XCHG__A; })

static void misc_qsort_internal(void const** A, void const** Z, bool (*cmp)(void const*, void const*))
{
  while ((Z - A) > 1) {
    /*
      quicksort is a bit tricky. the basic idea of just partitioning
      into < and >= partitions and recursing on each isn't guaranteed
      to terminate, because everything can end up in the >= part.

      the key to termination is to make sure at least one element
      drops out.

      the element that drops out must be equal to the pivot, so that
      it can be >= everything in the < part and <= to everything in
      the >= part. most simply, this element can be the pivot, which
      should be inserted between the < and the >= parts.

      so as we build up the regions we keep the pivot at the start of
      the array and later exchange it with the last element in the <
      part.
    */
    void const* pivot = *A;

    /* S points to the first element in the >= part */
    void const** S = A+1;

    for (void const** I = S; I < Z; I++) {
      if (cmp(*I, pivot)) {
        /* when *I < pivot */
        /* we must exchange with the first element in the >= part */
        MISC_XCHG(*I, *S);
        /* and shift the >= part over by one */
        S++;
      } else {
        /* when *I >= pivot */
        /* nothing to do, the >= part grows "automatically" */
      }
    }

    /* now do the business of exchanging the pivot */
    void const** R = (S - 1);
    *A = *R;
    *R = pivot;

    /* recursively sort (>=A, <R) and (>=S, <Z) */
    /* recurse on the smaller partition */
    /* this keeps stack usage always at or below O(log n) */
    if ((R - A) < (Z - S)) {
      misc_qsort_internal(A, R, cmp);
      A = S; /* loop to sort (>=S, <Z) */
    } else {
      misc_qsort_internal(S, Z, cmp);
      Z = R; /* loop to sort (>=A, <R) */
    }
  }
}

/*
  cmp should return true iff [arg1] < [arg2] (strictly).
*/
static INLINE void misc_qsort(void const** A, uintptr_t L, bool (*cmp)(void const*, void const*))
{
  misc_qsort_internal(A, (A + L), cmp);
}

#define MISC_QSORT_VARIANT(s, t)                                        \
  static INLINE void misc_qsort_##s(t** A, uintptr_t L, bool (*cmp)(t const*, t const*)) \
  {                                                                     \
    misc_qsort(((void const**)(A)), L, ((bool (*)(void const*, void const*))(cmp))); \
  }

I think instead of writing the macro MISC_XCHG and using only once, I would just write the swap directly in place.
The use of upper-case for the single-letter locals is unconventional, and looks odd.
I haven't measured performance, but it seems that sorting already-ordered (or nearly-ordered) input is likely to suffer as a consequence of always picking the first element as pivot.  A popular strategy is to pick the median of the first, last and central elements.  It doesn't matter that these are not necessarily distinct when we get to the very smallest partitions.
I do like the code that chooses the smaller partition for the recursive step, and uses iteration on the larger partition.
I don't understand why L is a uintptr_t - I would have thought size_t the obvious choice, the same as standard qsort().
The cast of cmp from  bool(*)(t const*, t const*) to (bool(*)(void const*, void const*) leads to Undefined Behaviour when it's called as the wrong type.  The correct way to provide a type-specific comparator is to convert the arguments:
#define DEFINE_QSORT_COMPARATOR(s,t,cmp) \
  static bool qsort_compare_##s(void const *a, void const *b) \
  { t const *x = a; t const *y = b; return cmp(*x, *y); }


Class that represents a conditional expression I have a class with which I want to persist a conditional expression:

boolean x = a [GT] i

I am only concerned with the boolean outcome of that comparison. 'a' '[GT]' and 'i' are supplied as arguments at runtime.

public class RuleClauseCondition {
    RuleClauseOperator operator;
    Double             threshold;
    Double             operand;

    private RuleClauseCondition()
    {
    }

    public RuleClauseCondition(RuleClauseOperator operator, Double threshold, Double operand) {
        this.operator = operator;
        this.threshold = threshold;
        this.operand = operand;
    }

    boolean isValid() {
        return operator.isValid(operand, threshold);
    }

}

public enum RuleClauseOperator{
    EQ {
        @Override
        public boolean isValid(Double inputValue, Double thresholdValue) {
            return inputValue.equals(thresholdValue);
        }

        @Override
        public String toString() {
            return " == ";
        }
    }, LT {
        @Override
        public boolean isValid(Double inputValue, Double thresholdValue) {
            return inputValue < thresholdValue;
        }

        @Override
        public String toString() {
            return " < ";
        }
    }, GT {
        @Override
        public boolean isValid(Double inputValue, Double thresholdValue) {
            return inputValue > thresholdValue;
        }

        @Override
        public String toString() {
            return " > ";
        }
    }

I am looking for better names to represent the left and right side of the operator as well as the class name.

Are current names intuitive and self documenting, or do I need to improve them?

Well, being the class a binary comparator, and being that the enum contains comparison operators, I would suggest to call them BinaryComparator and ComparisonOperator. Unless you have a naming convention or something else that would make those names not usable, obviously.

I would suggest naming classes by what they represent, if possible it should be a name not related to the context (if such names don't make the code less readable or less understandable).


Generating a list (or a string) with random elements During implementation and testing of different small hobby projects I often reach the point where I need (at least I like it more) some random data beyond the usual foo-bar strings or lists. Random here does not mean really mathematical random data but just something that looks random to humans.

Therefore I came up with the following:

;;;; random.lisp
;;; Time-stamp: <2019-05-27 14:53:38 m.buchmann>
;;;
;;; A short sketch of some random data generating functions.
;;;

(ql:quickload "alexandria")

;; * Choosing a random element from a given list.
(defun random-draw (list &aux (len (length list)) (pos (random len)))
  "Returns a random element from LIST."
  (nth pos list))

;;; * Generating a random element
(defun random-element (type &key (radix 10) (case :up))
  "Returns a random element of type :digit (depending on :radix) or
  :character (case given by :case :up, :down or :both). Limited to
7bit ASCII characters from A to z."
  (let ((char-range (ecase case
                      (:up   (alexandria:iota 26 :start 65))
                      (:down (alexandria:iota 26 :start 97))
                      (:both (append (alexandria:iota 26 :start 65)
                                     (alexandria:iota 26 :start 97))))))
      (ecase type
        (:digit (random radix))
        (:character (code-char (random-draw char-range))))))

(defun random-list (len &key (type :digit) (radix 10) (case :up))
  "Returns a list of length LEN filled with random elements of TYPE :digit or :character."
  (loop :for i from 0 below len
        :collect (random-element type :radix radix :case case)))

(defun random-string (len &key (case :up) &aux (result (make-array len :element-type 'character)))
  "Returns a random string of length LEN and :case (:up, :down or :both)."
  (loop :for i :from 0 :below len
        :do (setf (aref result i) (random-element :character :case case)))
  result)

I did not pack it in a proper package etc. yet because my use case is pretty simple and usually temporary. I was wondering if other people have implemented similar things or if the need I felt was just so individual that no one else really cares about it. I did not find other libraries for this.

I think it could be easily extended to supply random-arrays, hash-tables and so on. Also the character encoding could be improved to deliver more than 7bit ASCII and this in a portable way.

Any comments on the usability, style etc. are gratefully acknowledged.

Within python there is the faker package creating "human" data. It's often used for testing and can easily be extended to your personal needs:

Link to github

It also has a command line interface. Here is the example from the official documentation:

$ faker address
968 Bahringer Garden Apt. 722
Kristinaland, NJ 09890

$ faker -l de_DE address
Samira-Niemeier-Allee 56
94812 Biedenkopf

$ faker profile ssn,birthdate
{'ssn': u'628-10-1085', 'birthdate': '2008-03-29'}

$ faker -r=3 -s=";" name
Willam Kertzmann;
Josiah Maggio;
Gayla Schmitt;


Matched Brackets challenge I am attempting the matched brackets problem. I have written a Python program which seems to do the job but is very slow and thus exceeds the time limit. Please help me making this code more efficient. I have explained my code in the comments.

length = int(input())
inp = input().split(' ')
inp = [ int(x) for x in inp ]
position = 0 ##where our cursor is.
highest_depth =0
buffer = 0
stretch = 0 ##max distance b/w two matched brackets.
bar = 0 ## jst a buffer for calculating max distance between two brackets.
stretch_pos = 0
for l in inp:
    position += 1
    bar += 1
    '''if the cursor is currently at open bracket(1) add 1 to buffer'''
    if l == 1:
        buffer += 1
    else:
        '''if the cursor is at a closed bracket(2) we need to stop calculating
        higest depth and assign it to highest_depth only if the current value
        is smaller'''
        if highest_depth < buffer:
             highest_depth = buffer
             hpos = position -1 ##position of highest depth.
        buffer -= 1
        '''if buffer is now 0  due to the above line it must mean that all the
           brackets opened have been closed.'''
        if buffer == 0:
            ''' bar which calculates the max distance between two matched brackets
                must be assigned now and reset to zero'''
            if bar > stretch:
                stretch = bar
                stretch_pos = position - stretch + 1
            bar = 0
print(highest_depth , hpos,stretch, stretch_pos)

I think your algorithm is fast enough to get accepted. Contest times are sometimes not well calibrated for python speed.

One part that could be optimized is splitting the string and converting it to integers, which is now creating a new string for every number, possibly up to \$10^5\$ strings. Since the string is always gonna be 1-digit numbers separated by spaces, you could instead iterate on the string without parsing it or converting it to integers.

Something like:

str = "1 2 3 4"
for index in range(0, len(str), 2):
  num = str[index]
  # use num here


Store CMTimeRange into Core Data I wanted to store CMTimeRange structures inside managed objects, so I've worked out the following:

@interface Clip : NSManagedObject

@property (nonatomic, retain) NSData * primitiveRange;
@property CMTimeRange range;

@end

Within the data model editor I've set the type of primitiveRange to "Transformable".

The implementation:

@implementation Clip
{
    CMTimeRange _range;
}
@dynamic primitiveRange;

// pre-calculated get
- (void)awakeFromFetch
{
    [super awakeFromFetch];

    NSData *data = [self primitiveValueForKey:@"primitiveRange"];
    if (data) {
        _range = CMTimeRangeMakeFromDictionary((__bridge CFDictionaryRef)[NSKeyedUnarchiver unarchiveObjectWithData:data]);
    }
}

- (CMTimeRange)range
{
    [self willAccessValueForKey:@"primitiveRange"];
    CMTimeRange value = _range;
    [self didAccessValueForKey:@"primitiveRange"];

    return value;
}

// immediate-update set
- (void)setRange:(CMTimeRange)range
{
    NSData *value = [NSKeyedArchiver archivedDataWithRootObject:CFBridgingRelease(CMTimeRangeCopyAsDictionary(range, kCFAllocatorDefault))];

    [self willChangeValueForKey:@"primitiveRange"];
    [self setPrimitiveValue:value forKey:@"primitiveRange"];
    [self didChangeValueForKey:@"primitiveRange"];

    _range = range;
}

This works fine, but is this the proper way of doing things?

Objective-C doesn't have namespaces.  As such, we need to be much more careful with our class names.  The common way for dealing with this problem in Objective-C is to come up with a 2 or 3 letter prefix to put in front of all of your classes.  Apple does this with their classes.

NS for NeXTSTEP
CF for CoreFoundation
UI for UserInterface
SK for SpriteKit
CM for CoreMotion (but I think it overlaps something else maybe)
CA for CoreAnimation
CG for CoreGraphics

And there are more.

If it weren't for this naming convention, Apple would have a hard time creating class like NSView for OS X development and UIView for iOS development.

Following in this path, other good Objective-C developers come up with their own acronyms to prefix their various classes, structs, and typealiases with.  And you should do the same.  Clip just will not work as a class name (plus, it's not particularly expressive of what the class actually represents... is this like a movie clip or audio clip or something?).

I believe most Objective-C programmers prefer Egyptian braces.  You're consistent-ish with your braces though, that's fine-ish and not a necessary change.  I say consistent-ish because you use Egyptian braces for your if, but not for your methods.  Due to the way Xcode folds code, I consider Egyptian style to be vastly superior if you're developing in Xcode.

Finally... we often talk about (especially in Code Reviews), magic numbers.  You don't have any of these, but I'm going to go ahead and call @"primitiveRange" a magic string.

We should define a constant, and perhaps use reverse domain lookup for your keys:

NSString * const kKEY_PrimitiveRange = @"com.yourDomain.clip.primitiveRange";

Your using this string in 6 places.  What happens if, for whatever reason, you decide to change the string?  Now you have to change it in 6 different places.  If we'd just use a string constant, if we ever change the string, we only have to change it in one place.


Slow performing subquery This query executes very slowly and I wonder if it can be improved?

We have an Access database split to front end / back end with around 50 users. BE is in a network folder and FE is on users' hard drives.

Data entered into the FE is stored in FE tables until the user has finished inputting all the required data for a record. They then click a button to send the data to the BE in one go, to  identical tables as in the FE. In the sql below the BE table is suffixed with '_Share'.

The table contains 2 keys: QuoteID and OptionID. There is a one-to-many between the two e.g.:

QuoteID   OptionID

1234      1
1234      2

3333      1
3333      2
3333      3

As they user works they are creating data for new Options to go with existing Quotes. What the code is doing is to check whether a QuoteID on the BE already has an OptionID created by the user on the FE, if not then the data for that OptionID is appended to the BE.

INSERT INTO T_Option_Category_Benefits_Share
SELECT T_Option_Category_Benefits.*, *
FROM T_Option_Category_Benefits
WHERE (((T_Option_Category_Benefits.QuoteID)=1971)
AND ((T_Option_Category_Benefits.OptionID)
NOT IN (SELECT T_Option_Category_Benefits_Share.OptionID FROM T_Option_Category_Benefits_Share WHERE T_Option_Category_Benefits_Share.QuoteID=1971)));

The key fields are not indexed. The BE table contains 18 columns with around 100k rows. The network is generally quite slow at busy times but time of day doesn't have much effect. Amount of rows to append is only around 300 on average. We're using Office 365 on Windows 10.

Here are a few tips:

Do not use select *, instead type out all the columns you need to insert into. It should be done in both your select and insert statements.
For the subquery in the where clause, you can store the result from that query in a temp table and then use that temp table for the where clause so it does not need to check while inserting the data.
Try to add an index to your tables which are unique so you can do faster and more efficient queries.


Hacker rank - Left rotation - PHP code feedback for Timeout Here is the problem description: https://www.hackerrank.com/challenges/ctci-array-left-rotation

  A left rotation operation on an array of size \$n\$ shifts each of the array's elements 1 unit to the left. For example, if 2 left rotations are performed on array \$[1,2,3,4,5]\$, then the array would become \$[3,4,5,1,2]\$.

  Given an array of \$n\$ integers and a number, \$d\$, perform \$d\$ left rotations on the array. Then print the updated array as a single line of space-separated integers.

My code passes all test cases but is stuck on Hacker rank timeout.  I want to know where is the part that takes too long to execute, in order to optimize my code.

<?php

function rotateOnce($a)
{
    if ($a[0] >= 1 && $a[0] <= 1000000) {
        $left = array_shift($a);
        $a[] = $left;
    }
    return $a;

}

function checkConstrains($d, $n)
{
    if ($d >= 1 && $d <= $n && $n >= 1 && $n <= 10 ^ 5)
        return true;

    return false;
}

// Complete the rotLeft function below.
function rotLeft($a, $d)
{
    global $n;
    if (checkConstrains($d, $n)) {
        for ($i = 0; $i < $d; $i++) {
            $a = rotateOnce($a);
        }
    }
    return $a;

}

$fptr = fopen(getenv("OUTPUT_PATH"), "w");

$stdin = fopen("php://stdin", "r");

fscanf($stdin, "%[^\n]", $nd_temp);
$nd = explode(' ', $nd_temp);

$n = intval($nd[0]);

$d = intval($nd[1]);

fscanf($stdin, "%[^\n]", $a_temp);

$a = array_map('intval', preg_split('/ /', $a_temp, -1, PREG_SPLIT_NO_EMPTY));

$result = rotLeft($a, $d);

fwrite($fptr, implode(" ", $result) . "\n");

fclose($stdin);
fclose($fptr);

Efficiency

You do a lot of manual shifting and pushing with your array. This is ok for small inputs, but as soon, as the distance $d and the size of your array $n grow, this becomes inefficient.

The main overhead however is coming from calling rotateOnce. Parameters are passed by value by default. That means the array is copied every time the function is called. You could pass it by reference:

function rotateOnce(&$a) {}

or simply include the two lines in your original function:

function rotLeft($a, $d) {
    $n = count($a);

    for ($i = 0; $i < $d; $i++) {
        $left = array_shift($a);
        $a[] = $left;
    }

    return $a;
}

I would guess that constraints on HackerRank mean, that you can expect values in that range and don't need to test inputs yourself.

This is now significant faster, but still slow especially for large distances $d.

That being said, I would take a look at PHP's internal array functions and think of a way, how to use them in combination to increase performance.

My naive approach would be something like this

if d is 0 or the same as the array's size, return the original
else split the array into two chunks at index $d
combine both arrays and return the result

The function could look like this Don't hover if you don't want to get spoiled:

 function rotLeft($a, $d) {
     if (count($a) == $d || $d === 0) {
         return $a;
     }

     $chunk1 = array_slice($a, 0, $d);
     $chunk2 = array_slice($a, $d);

     return array_merge($chunk2, $chunk1);
 }

For this input:

$a = range(1, 100000);
$n = count($a);
$d = 500;

… I've measured these times on my local machine*:

original: 1.8130s
optimized: 0.4294
rewritten: 0.0054s

Exponential expression vs. bitwise Operators

Your program has a flaw. It won't calculate the result correctly for large array sizes, because of this:

$n <= 10 ^ 5

^ is bitwise Xor and not pow:

pow(10, 5);
10 ** 5;

Try to avoid globals

I can see, that n is an input parameter that is not part of the function's given signature. However, I would try to avoid globals and get the value manually, if needed:

$n = count($a);

You can read more about this here:

PHP global in functions

Are global variables in PHP considered bad practice? If so, why?

* macOS 10.13, I7 2.5 GHz, 16GB RAM, MAMP PHP 7.2.1


Rotating `n` random items in an array Honestly, it should be an incredibly simplistic function, yet it has become the following atrocity:

function rotate(arr, n){

  // step 1: pick n different elements in the array
  var allIndexes = [];
  for(var i = 0; i<arr.length; i++){
    allIndexes.push(i);
  }

  var pickedIndexesToRotate = [];
  var valuesOfPickedIndexes = [];
  for(var i = 0; i<n; i++){
    var randomIndex = Math.floor(Math.random()*allIndexes.length);
    valuesOfPickedIndexes.push(arr[allIndexes[randomIndex]]);
    pickedIndexesToRotate.push(allIndexes.splice(randomIndex,1)[0]);
  }

  // step 2: actually rotate those elements...
  //         so if n = 3 and pickedIndexesToRotate = [5, 1, 2]
  //         we get [10, 60, 20, 40, 50, 30]
  for(var i = 1; i<=n; i++){
    arr[pickedIndexesToRotate[i == n ? 0 : i]] = valuesOfPickedIndexes.shift();
  }

  return arr;

}
rotate([10,20,30,40,50,60], 3);

And as it's just for a weekend project and it works I should be happy... but honestly, I can not say I am. It feels like the function is both far too complex for the simplicity of the task and far too inefficient. Looking forward to any and all criticism and advice.

(And just for the record: I wouldn't normally have those comments in my code as I believe code should be self explanatory, but it seemed useful in this case (as in, on codereview.SE) to make the intent of the function clear.)

First of all, I wouldn't call it "rotate" as rotate has a significant meaning in programming. What you're doing though is picking up n random values and rotating them by themselves, and not in context to the array itself.

Anyways, I'll just explain in the code for the rest.

function rotate(arr, n){

  // I wouldn't want to mutate the original array in the operation
  // so instead I operate on a shallow copy of the array.
  var arrToOperate = arr.slice();

  // Then we select indices by random.
  // The | 0 at the end truncates the decimal part, like floor.
  // We sort, since rotation needs to know who is last/first.
  var selectedIndices = [];
  for(var i = n; i--;) selectedIndices.push((Math.random()*arr.length) | 0);
  selectedIndices.sort();

  // Now the actual rotation. If value isn't the zeroth number, get the next
  // otherwise get the one from the other end.
  for(var j = n; j--;) arrToOperate[selectedIndices[j]] = arr[selectedIndices[(!j ? n : j) - 1]];

  return arrToOperate;

}

alert(JSON.stringify(rotate([10,20,30,40,50,60], 3)));

Looping backwards is personal preference, because I don't have to use comparison operators, and I can always use 0 as the terminator value.

The | 0 is also a personal preference, as it is shorter (and sometimes faster) than Math.floor.


Average spam confidence Exercise 7.2 from Python for Informatics:

  Write a program to prompt for a file name, and then read through
  the file and look for lines of the form:

  X-DSPAM-Confidence: 0.8475

  When you encounter a line that starts with “X-DSPAM-Confidence:” pull apart
  the line to extract the floating-point number on the line. Count these lines and then compute the total of the spam confidence values from these lines. When you reach the end of the file, print out the average spam confidence.

Enter the file name: mbox.txt
Average spam confidence: 0.894128046745

Enter the file name: mbox-short.txt
Average spam confidence: 0.750718518519

  Test your file on the mbox.txt and mbox-short.txt files.

My code works. It's a bit more forgiving with the amount of decimals on both input and output, but that's as intended.

What I'm not happy about are the ask_file_name and retrieve_values functions. The first uses a hacky method of input validation (the Path module seems appropriate here, but that would be overkill), the second uses iteration while it shouldn't. Perhaps a reduce  would be more appropriate, or something different altogether.

This is written in modern Python. Which means it's written in Python 3 (3.5.2 to be exact, Python 3.7.x is incoming) and has plenty of docstrings. It should be up to standards, but I'd like to have that verified.

AverageSpamConfidence.py

#! /usr/bin/env python3
# coding: utf-8

# Sample data from http://www.py4inf.com/code/

import re

def ask_file_name():
    """
    Ask user for file name.

    Check for FileNotFound & IsADirectory errors.
    Keyword arguments:
    -
    """
    while True:
        try:
            user_input = input("Enter the file name: ")
            # Is the file there and can we open it?
            with open(user_input, "r") as test_input:
                pass
        except FileNotFoundError:
            print("Input empty or file does not exist.")
            continue
        except IsADirectoryError:
            print("That's not a directory, not a file.")
            continue
        else:
            return user_input

def find_occurences_in_file(file_name):
    """
    Find all occurences in target file.

    Keyword arguments:
    file_name -- name of (and path to) target file (example: mbox.txt)
    """
    with open(file_name, "r") as input_file:
        return re.findall(
            'X-DSPAM-Confidence: 0.[0-9]+', str(input_file.readlines())
        )

def retrieve_values(input_list):
    """
    Return relevant values from list.

    Keyword arguments:
    input_list -- list to results to retrieve values from
    """
    return_list = []
    for line in input_list:
        return_list.append(float(line.split()[-1]))
    return return_list

def average(input_list):
    """
    Calculate average of list provided.

    Keyword arguments:
    input_list -- list of numerical values
    """
    return sum(input_list) / len(input_list)

def main():
    """
    Exercise 7.2 (Python for Informatics, Charles Severance)
    Write a program to prompt for a file name,
    and then read through the file and look for lines of the form:

    X-DSPAM-Confidence: 0.8475

    When you encounter a line that starts with “X-DSPAM-Confidence:” pull apart
    the line to extract the floating-point number on the line.
    Count these lines and then compute the total of the spam confidence values
    from these lines. When you reach the end of the file, print out
    the average spam confidence.
    """
    file_name = ask_file_name()
    occurences = find_occurences_in_file(file_name)
    values = retrieve_values(occurences)
    print(average(values))

if __name__ == '__main__':
    main()

Not duplicating any of @Peilonrayz's code review points ...

Stop reading entire files into memory when you can process the file line by line in one pass, and stop creating huge lists in memory which are then iterated over exactly once.  Both of these things creates a huge unnecessary memory pressure which can be avoided by looping and/or using generator expressions.

Using a simple loop over all lines in the file:

def average_spam_confidence(filename):
    with open(filename) as file:
        count = 0
        total = 0
        for line in file:
            if line.startswith("X-DSPAM-Confidence: "):
                try:
                    total += float(line[20:])
                    count += 1
                except:
                    pass   # Line ended with garbage
        return total / count

No regex.  No reading entire file in memory.  No creating a list of individual confidence values to sum up afterwards.

Plus, we've fixed a bug!  If "X-DSPAM-Confidence:" appears in the middle of a line, instead of at the start as required by the problem text, we don't try to process it.

But perhaps you wanted a more functional way of programming, where you:

find all the confidence lines,
extract the confidence values, and
compute the average

... all as separate steps which you can compose together, and reuse to solve other problems.  Fear not!  We can do that too!  Enter generator expressions:

First, let's open the file, and read all the matching lines into a list, using list comprehension:

with open(filename) as file:
    lines = [line for line in file if line.startswith("X-DSPAM-Confidence: ")]

That second statement loops through each line in the file, checks if the line begins with the desired text, and if so, includes it in the list that is being constructed.  We can later iterator over lines to further process each line individually.

That is almost what we want to do.  Well, that is exactly what we want to do, but we don't want to do it all at once.  If we change the [...] to (...), we move from list comprehension to a generator expression.

with open(filename) as file:
    lines = (line for line in file if line.startswith("X-DSPAM-Confidence: "))

Now, the second statement has done ... nothing.  We haven't read the first character of the file yet.  What we have returned is a generator expression, which when we ask for the first value will start reading lines until it finds one that matches, and then it will pause its execution and return that value.

Ok.  Let's extract just the confidence values:

    values = [ line[20:] for line in lines ]

Whoops!  That's list comprehension.  It will loop over all the lines that the lines generator can produce, skip over the prefix and return the rest.  Again, change those [...] to (...):

    values = ( line[20:] for line in lines )

Better!  Now those are still strings, so we'll need to convert them into float point values.  Too easy.  Just map them:

    confidences = map(float, values)

confidences is a generator.  If you said list(confidences), you'd create that in-memory list of float values for all the "X-DSPAM-Confidence:" values in the file.  And you could then sum(...) and len(...) the list of values to compute the average.  But we don't want to realize the list in memory, so ...

def average(data):
    total = 0
    count = 0
    for value in data:
        total += value
        count += 1
    return total / count

average_confidence = average(confidences)

... we ask for values from the confidences generator, add them up one at a time, counting as we go.  When the generator is exhausted, the for loop ends, and we return the average.

Putting it all together:

def average(data):
    total = 0
    count = 0
    for value in data:
        total += value
        count += 1
    return total / count

def average_spam_confidence(filename):
    with open(filename) as file:
        lines = (line for line in file if line.startswith("X-DSPAM-Confidence: "))
        values = ( line[20:] for line in lines )
        confidences = map(float, values)
        return average(confidences)

Or more simply:

import statistics

def average_spam_confidence(filename):
    with open(filename) as file:
        values = (line[20:] for line in file
                            if line.startswith("X-DSPAM-Confidence: "))
        return statistics.mean(map(float, values))

Note: the non-generator solution was more robust converting the confidence value strings into floats, via a try...except block.  The generator expression solution shown above omits that.  The robustness may be improved by using a more precise matching when searching for the "X-DSPAM" lines (regex).  Alternately, a generator function could be used, which discards the non-float values.

def map_to_float(data):
    for value in data:
        try:
            yield float(value)
        except:
            pass

confidences = map_to_float(values)

Note (from @Roland Illig's comment): Because generator expressions delay the execution of their operation, any resources they use must remain available until their processing has been finished.  They cannot be used to compute the average spam confidence if the file they are reading from is closed before the average has been computed.  In the above examples, the generator expressions are fully consumed within the body of the with open(...) as file: block, so the file was kept open.

This does not mean the generator expressions must all occur within the with statement.  They can be spread out across many functions, but their execution must be constrained to the interval when the file is open:

def find_occurrences_in_file(input_file):
    """Return a generator for occurrences of X-DSPAM lines"""
    return (line for line in input_file if line.startswith("X-DSPAM-Confidence: "))

def retrieve_values(input_list):
    """Return a generator for float values from a list"""
    return map(float, (line.split()[-1] for line in input_list))

def average(input_list):
    """Compute average of a list/sequence/iterable/generator of values..."""
    return statistics.mean(input_list)

def average_spam_confidence(file_name):
    with open(file_name) as file:
        # File is open for this entire with statement.
        occurrences = find_occurrences_in_file(file)
        values = retrieve_values(occurrences)
        print(average(values))
        # File is closed here - but "average(...)" has exhaustively read
        # from all of the generators already.


Unit test using mocks As some part of my application needs connection to database I wanted to create tests that use mocks to avoid running special instance of database just for testing. That gave me a glimpse of how can I use mocks and I played with them a bit.
I wanted to test express router:
UserRouter.post("/users", async ({ body }: Request, res: Response) => {
    try {
        const userRepository = getCustomRepository(UserRepository);
        const user = userRepository.create({ ...body } as User);
        await userRepository.save(user);
        res.send(user.toJSON());
    } catch (error) {
        res.status(400).send();
    }
});

To do so I created mocks:
jest.mock("typeorm", () => ({
    __esModule: true,
    getCustomRepository: jest.fn(),
    PrimaryGeneratedColumn: jest.fn(),
    Column: jest.fn(),
    CreateDateColumn: jest.fn(),
    UpdateDateColumn: jest.fn(),
    Entity: jest.fn(),
    EntityRepository: jest.fn(),
    Repository: jest.fn(),
}));

const getCustomRepositoryMock = mocked(getCustomRepository);
const MockedRepository = {
    create: jest.fn(),
    save: jest.fn(),
};
const MockedEntity = {
    toJSON: jest.fn(),
};

beforeEach(() => {
    jest.resetAllMocks();
    getCustomRepositoryMock.mockReturnValueOnce(MockedRepository);
});

Then instead of passing and requiring actual data to be return I relayed of testing whether mocks were used, what data were passed to their calls etc. Here is an example:
it("should call save on UserRepository with value returned by call create on UserRepository", async () => {
    MockedRepository.create.mockReturnValue(MockedEntity);
    await request(app).post(path).send();
    expect(MockedRepository.save).toBeCalledWith(MockedEntity);
});

Commit with all my tests is here.

As long as the real implementation that is being mocked here is also unit tested. And the mock is configured to match the expected behavior of the real implementation. You should be fine.
However you gotta remember to change all mock configuration when expectations of the real implementation change. This should not happen often for a library (BC break) but it is much more common in applications.
You are also depending on a specific implementation and if in future the tested object is changed to use different approach/calling other methods of other classes etc, your tests will have to change.
But anyway there is something your test does not test. It does not check that the correct json is returned by the endpoint. And it does not test what happens when exception is thrown. Although the latter should probably be a separate test.
One note for the route, I'm not sure whether save method throwing an exception should be considered a client error (400). It is not client's fault that your db failed.


A program to find out the number of odd and even Fibonacci numbers between given range I built a program in C that can tell how many odd and even Fibonacci numbers there are between a given range.

  Input Specification

  First line of the input contains T, representing the number of test cases (1 ≤ T ≤ 50). Each test case contains two integers N and M (1 ≤ N ≤ M ≤ 1018) and (|N-M| ≤ 105), where N is the Nth Fibonacci number and M is the Mth Fibonacci number of the sequence.

  Output Specification

  Case T:
  Odd = total number of odd Fibonacchi numbers between N and M
  Even = total number of even Fibonacchi numbers between N and M

The full code

#include <stdio.h>
#include <math.h>

int main()
{
    int T, i, j, k;
    scanf("%d", &T);

    for (i=0; i<T; i++)
    {
        int N, M, val;
        scanf("%d%d", &N,&M);
        val = abs(N-M)+1;

        int n,m;
        if (M>N) { n=N; m=M; }
        if (N>M) { n=M; m=N; }

        int b_tri, e_tri, even=0, odd=0;
        if (n%3==1) { b_tri=3; even++; odd+=2; }
        if (n%3==2) { b_tri=2; odd+=2; }
        if (n%3==0) { b_tri=1; odd++;  }

        if (m%3==1) { e_tri=1; even++; }
        if (m%3==2) { e_tri=2; odd++;  even++; }
        if (m%3==0) { e_tri=3; odd+=2; even++; }

        val-=(e_tri+b_tri);
        val/=3;

        for (j=0; j<val; j++)
        {
            even++;
            odd+=2;
        }

        printf("Case %d:\nOdd = %d\nEven = %d\n", i+1, odd, even);
    }

    return 0;
}

Logic

1st Fibonacci number is 0 (even).
2nd Fibonacci number is 1 (odd).
3rd Fibonacci number is 1 (even + odd = odd).
4th Fibonacci number is 2 (odd  + odd = even).
5th Fibonacci number is 3 (odd + even = odd).
6th Fibonacci number is 5 (even + odd = odd).
7th Fibonacci number is 8 (even).
8th Fibonacci number is 13 (odd).
9th Fibonacci number is 21 (odd).

I'd like advice on:

Using mathematical formulas instead of loops to calculate this
Making the code more efficient

I'm going to assume you tested it for several inputs and got the right values. Your logic seems right so if a couple test cases work, it probably works. But I didn't look carefully for any off-by-one like errors.

There is one logic issue that I see:

You don't set n and  m if N = M. Double check your two if statements in the beginning of the loop; you only check for strict inequality on both sides. One of them should change to a >= or <=, or better yet just use if-else instead of two ifs.

As for optimization...

In terms of an average computer, your program is good enough for any N and M because you specify N,M < 1018. Even if you don't, for-loops by themselves are pretty cheap.

However, there are still optimizations/cleanups.

The biggest one is your last for loop:

for (j=0; j<val; j++)
{
    even++;
    odd+=2;
}

This can just be:

even+=val;
odd+=2*val

Your two if chains can turn into if, else if, else instead of three ifs (same thing for your first if pair, which should just be if else)

if (...) { }
else if (...) { }
else (...) { }

although the compiler will most likely optimize that for you.

I think that's the best you can do for your current algorithm.

However, it seems odd to me that you are checking for all mod cases for n and m; I think your idea is that you know the pattern for the large chunks in between N and M but you want to trim off the edge cases. Now, you won't need to trim down the case when n%3==1 or m%3==0, because you can just treat that as the even++, odd+=2 chunk.

n=m case

In programming you can always cop out :)

Here is how I would address the n=m case even though it's not too beautiful:

if (N == M)
{
  printf("Odd: 0, Even: 0"); //format this to match your output
  break;  // break ends the current for loop iteration in this context
}
else if (N > M)
{
  n = M; m = N;
}
else
{
  n = N; m = M;
}


Aggregating Conditional Sums I have recently forked/rewritten a SEDE query (here) that aims at figuring out where a site stands in terms of avid users and distribution of reputation scores, compared to a specific target (number of avid users in each major rep cluster).

declare @target_20k float
declare @target_10k float
declare @target_3k float
declare @target_2k float

set @target_20k = 3.0
set @target_10k = 10.0
set @target_3k = 80.0
set @target_2k = 120.0

/* only look at users that visited the site recently */
declare @lastActiveDays int
set @lastActiveDays = 60

select [20K], [10K], [3K], [2K],
       [TotalSiteRep],
       [UsersCount],
       round([TotalSiteRep]/cast([UsersCount] as float),2) [AvgRep],
       [AvidUsersRep],
       round([AvidUsersRep]/cast([TotalSiteRep] as float),2) [%AvidRep],
       [AvidUsersCount],
       round([AvidUsersCount]/cast([UsersCount] as float),2) [%AvidUsers],
       round([AvidUsersRep]/cast(AvidUsersCount as float),2) [AvgAvidRep]
from (

select round(sum([20K])/@target_20k, 2) [20K],
       round(sum([10K])/@target_10k,2) [10K],
       round(sum([3K])/@target_3k,2) [3K],
       round(sum([2K])/@target_2k,2) [2K],
       sum(Reputation) [TotalSiteRep],
       count(*) [UsersCount],
       sum(AvidRep) [AvidUsersRep],
       sum(AvidUser) [AvidUsersCount]
from (
  select Reputation,
    case when Reputation >= 20000 then 1 else 0 end [20K],
    case when Reputation >= 10000 then 1 else 0 end [10K],
    case when Reputation >= 3000 then 1 else 0 end [3K],
    case when Reputation >= 2000 then 1 else 0 end [2K],
    case when Reputation >= 150 then Reputation else 0 end [AvidRep],
    case when Reputation >= 150 then 1 else 0 end [AvidUser]
  from Users
  where datediff(d, LastAccessDate, getdate()) <= @lastActiveDays
) q
) r

I'm not crazy about the case when ... then .. else ... end part and about the sub-querying, but it seems to get the job done.

Is there a better way to aggregate conditional sums? (feel free to review everything else, ...except the broken indentation of the q query - that's already something I know I have to fix ;)

Here are the query's results for Code Review, as of the latest SEDE update:

It's a good habit to end each statement with an explicit semicolon.

The innermost query q generates one row for each recent user.  Since you have no interest in individual users, and only want counts and sums, you can collapse the two innermost queries q and r — both poorly named, by the way — into one.  Use COUNT() for counting.

The lastActiveDays filtering is being done inefficiently:

where datediff(d, LastAccessDate, getdate()) <= @lastActiveDays

To accomplish that, the server would have to run datediff() on every single row in the Users table.  Instead, you should calculate the cutoff date just once:

where LastAccessDate >= cast(dateadd(day, -@lastActiveDays, getdate()) as Date)

(The cast truncates the result to midnight.)

I prefer to use Common Table Expressions for readability.  Since you are not running a correlated subquery, a top-down layout is better than nesting.

The UserStats CTE below is roughly equivalent to subquery q.  I've deferred the … / @target… divisions for readability and rearranged the columns in a more logical order.  It could be written as just one SELECT, but I think that splitting it up with a CTE makes it easier to understand.

declare @target_20k float;
declare @target_10k float;
declare @target_3k float;
declare @target_2k float;

set @target_20k = 3.0;
set @target_10k = 10.0;
set @target_3k = 80.0;
set @target_2k = 120.0;

declare @lastActiveDays int;
set @lastActiveDays = 60;

with UserStats as (
  select
      count(case when Reputation >= 20000 then 1 else null end) [20KUsersCount],
      count(case when Reputation >= 10000 then 1 else null end) [10KUsersCount],
      count(case when Reputation >=  3000 then 1 else null end) [3KUsersCount],
      count(case when Reputation >=  2000 then 1 else null end) [2KUsersCount],
      count(case when Reputation >=   150 then 1 else null end) [AvidUsersCount],
      count(Id) [UsersCount],
      sum(case when Reputation >= 150 then Reputation else 0 end) [AvidUsersRep],
      sum(Reputation) [TotalSiteRep]
    from Users
    where LastAccessDate >= cast(dateadd(day, -@lastActiveDays, getdate()) as Date)
)
select
    round([20KUsersCount] / @target_20k, 2) [20K],
    round([10KUsersCount] / @target_10k, 2) [10K],
    round([3KUsersCount]  / @target_3k,  2) [3K],
    round([2KUsersCount]  / @target_2k,  2) [2K],
    [TotalSiteRep],
    [UsersCount],
    round([TotalSiteRep] / cast([UsersCount] as float), 2) [AvgRep],
    [AvidUsersRep],
    round([AvidUsersRep] / cast([TotalSiteRep] as float), 2) [%AvidRep],
    [AvidUsersCount],
    round([AvidUsersCount] / cast([UsersCount] as float), 2) [%AvidUsers],
    round([AvidUsersRep] / cast(AvidUsersCount as float), 2) [AvgAvidRep]
  from UserStats;


Generating tables of numbers in PHP I have this code and this works fine but I think it's too lengthy. I want to know if there can be a less lengthy code. I've heard from people that lengthy code is often not considered good.

<title>Table</title>

<form method = "Post" action = "Table.php" >
Enter a number between 2 and 20 to see its tables:
<input type = "text" name = "number" />
<input type = "hidden" value = "true" name = "checker" />
</form>
<?PHP
if ( isset($_POST['checker'])) {
$number = $_POST['number'];
switch ($number) {
case 2:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 3:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 4:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 5:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 6:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 7:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 8:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 9:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
case 10:
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ;
echo "$number x $counter = $temp
";
}
break;
default:
echo "Please enter a number between 2 and 10";
}
}
?>

Already gave in the wrong placed question, anyway here is my solution: codepad.viper-7.com/zTLliA
Copy&paste is never a good idea.
if (isset($_POST['number']) && is_numeric($_POST['number']) && (($_POST['number'] > 0) && ($_POST['number'] <= 20))) {
    $number = $_POST['number'];
    for ( $counter = 1 ; $counter <=10; $counter++ ) {
        $temp = $number * $counter ;
        echo "$number x $counter = $temp<br />";
    }
} else {
    echo "Please enter a number between 1 and 20";
}

A single for is enough. In the if statement we check if the $_POST['number'] exists, if it's actually a number and if it is between 1 and 20.


Array of Objects or Array of arrays? I build phone apps and I use JSON a lot in my APIs. The problem is sometimes I don't know how to format the JSON data.

For example, an iOS UITableView takes NSArray as datasource.

{
    "contact_id": [
        "455",
        "464"
    ],
    "contact_name": [
        "Administrator",
        "Main contact"
    ]
}

For me it makes sense since I don't have to loop through anything, but I was told it is not safe to rely on the indices matching the position.

I remember, while asking questions, people bringing to my attention to be careful not to separate records into different arrays, and instead I should return array of objects:

[
    {
        "contact_id": "455",
        "contact_name": "Administrator"
    },
    {
        "contact_id": "464",
        "contact_name": "Main contact"
    }
]

I would like to know the best practice to formatting JSON.

Keep related data together.

Does your application need a list of contacts? Keep each contact intact with the second approach. Merging data to create and pass a single contact takes work and is error-prone.
Does it need a list of contact IDs? Do the same or return just that list. With the latter approach you can easily produce a limited array. Going the other way is much harder (more code).
Does it need a list of contact names in a drop-down that loads the full contact for one ID? Use the latter since you get everything up-front.

The short answer is Keep related data together because it's generally easier to pull it apart than merge it back together.


Pure Python script that saves an HTML page with all images Here is a pure Python script that saves an HTML page without CSS but with all images on it and replaces all hrefs with a path of an image on the hard drive.

I know that there are great libraries like BeautifulSoup and others but I would like to try myself with pure Python.

Actually there is no practical usage of this script, it was just test task from one of companies where I applied.

Once again what it does:

This script can pre-launched from command line, which takes 2 arguments:

Address of web page to save (this one is required)
Name of folder where images from page should be saved (this one is optional)
Script saves html content of page(without CSS) and also it looks for all images on page and save them too, replacing their href attribute to actual path of image on hard drive.

How can I improve it?

import random
import string
import sys
import urllib2
import os
import re
from urlparse import urlparse

def page_loader(url_name, dir_name='imgs'):

    page_to_open = urllib2.urlopen(url_name)
    target_page = page_to_open.read()
    base_dir = os.path.dirname(os.path.realpath(__file__))
    dir_to_save = os.path.join(base_dir, dir_name)
    new_file_name = '%s.html' % ''.join(random.choice(string.ascii_uppercase + string.ascii_lowercase) for _ in range(10))
    if not os.path.exists(dir_to_save):
        os.makedirs(dir_to_save)

    images_on_page = re.findall('img .*?src="(.*?)"', target_page)
    internal_images = [img for img in images_on_page if img.startswith('/')]
    external_images = [img for img in images_on_page if not img.startswith('/')]

    for image in internal_images:
        image_url = '%s%s' % (page_to_open.geturl()[:-1], image)
        new_image_name = urlparse(image_url).path.split('/')[-1]
        with open(os.path.join(dir_to_save, new_image_name), 'w') as new_image:
            new_image.write(urllib2.urlopen(image_url).read())
            target_page = re.sub(image, new_image.name, target_page)

    for image_url in external_images:
        new_image_name = urlparse(image_url).path.split('/')[-1]
        with open(os.path.join(dir_to_save, new_image_name), 'w') as new_image:
            new_image.write(urllib2.urlopen(image_url).read())
            target_page = re.sub(image_url, new_image.name, target_page)

    with open(os.path.join(base_dir, new_file_name), 'w') as new_file:
        new_file.write(target_page)

if __name__ == '__main__':
    target_url = sys.argv[1]
    if len(sys.argv) > 2:
        dir_name = sys.argv[2]
        page_loader(target_url, dir_name)
    else:
        page_loader(target_url)

Your code is pretty good! I do have a few tips regarding style, and such.

As mentioned in the comments, even though it looks like you aren't, you shouldn't be trying to parse HTML with Regexes.
Where are your comments? While good code can be pretty readable, comments are still a valuable asset. You should probably flesh out your page_loader function with a docstring, and any unclear blocks of code with an inline comment.
If you're only going to use part of a module, e.g, one function or variable, you should do from ... import ....

That's about all I can really think of right now. If there's anything else you want me to comment on, just mention it below, and I'll see if I can cover it. Hope this helps!


Python class that implements the Newton method Here is a python function I wrote to implement the Newton method for optimization for the case where you are trying to optimize a function that takes a vector input and gives a scalar output.  I use numdifftools to approximate the hessian and the gradient of the given function then perform the newton method iteration.

import numpy as np
import numdifftools as nd

class multivariate_newton(object):

    def __init__(self,func,start_point,step_size=0.8,num_iter=100,tol=0.000001):
        '''
        func: function to be optimized. Takes a vector argument as input and returns
              a scalar output
        step_size: step size in newton method update step
        num_iter: number of iterations for newton method to run
        tol: tolerance to determine convergence
        '''
        self.func=func
        self.start_point=np.array(start_point)
        self.num_iter=num_iter
        self.step_size=step_size
        self.tol=tol

    def newton_method(self):
        '''
        perform multivariate newton method for function with vector input
        and scalar output
        '''
        x_t=self.start_point
        #Get an approximation to hessian of function
        H=nd.Hessian(self.func)
        #Get an approximation of Gradient of function
        g=nd.Gradient(self.func)

        for i in range(self.num_iter):
            x_tplus1=x_t-self.step_size*np.dot(np.linalg.inv(H(x_t)),g(x_t))
            #check for convergence
            if abs(max(x_tplus1-x_t))<self.tol:
                break
            x_t=x_tplus1

        self.crit_point=x_tplus1
        self.max_min=self.func(x_t)

        return self

    def critical_point(self):
        '''
        print critical point found in newton_method function. newton_method function
        must be called first.
        '''
        print self.crit_point

Pathological cases exist where Newton's method will not converge on a solution.  Yet, there is no obvious way for the caller to tell whether convergence was achieved.  You should distinguish between whether the loop terminated via the break (success) or by exhaustion of the loop condition (failure).

for _ in range(self.num_iter):
    x_tplus1 = x_t - self.step_size * np.dot(np.linalg.inv(H(x_t)), g(x_t))
    #check for convergence
    if abs(max(x_tplus1-x_t))<self.tol:
        break
    x_t = x_tplus1
else:
    raise SolutionNotFound, "No convergence after %d iterations" % (self.num_iter)


Instantiating shapes using the Factory Design Pattern in Java Trying to learn  the factory design pattern, came up with some code based on a Shape Factory (found this example here). Is this the right way to implement the factory pattern?

interface Shape{
    void draw();
}

class Rectangle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A Rectangle!");

    }

}

class Circle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A cIRCLE!");

    }

}

class Triangle implements Shape{

    @Override
    public void draw() {
        System.out.println(" Drawing A Triangle!");

    }

}

class ShapeFactory{

    public static Shape getShape(String shapeType)
    {
        Shape shape = null;
        switch(shapeType.toUpperCase()){

        case "CIRCLE":
        shape = new Circle();
        break;

        case "RECTANGLE":
            shape = new Rectangle();
            break;

         case "TRIANGLE":
            shape = new Triangle();
            break;
        }

            return shape;
    }

}

public class FactoryExample {

    public static void main(String[] args) {
        Shape s = ShapeFactory.getShape("rectangle");
         s.draw();
         s = ShapeFactory.getShape("triangle");
        s.draw();
    }

}

You could rewrite getShape as follows:

public static Shape getShape(String shapeType) {
    switch (shapeType.trim().toUpperCase()) {
        case "CIRCLE":
            return new Circle();

        case "RECTANGLE":
            return new Rectangle();

        case "TRIANGLE":
            return new Triangle();

        default:
            return null;
    }
}


Is this safe against major XSS attacks? I am building a classifieds website here in Portugal, and I'm now in the security phase. So until now, I already made what I think is a good measure against SQL injection:

$firstname = chunk_split(mysql_real_escape_string($_POST[firstname]),1,'.');
$lastname = chunk_split(mysql_real_escape_string($_POST[lastname]),1,'.');
$email = chunk_split(mysql_real_escape_string($_POST[email]),1,'.');

etc...

This will save valid and invalid emails, for example, like this:

good email: u.s.e.r.@.e.m.a.i.l.h.o.s.t...c.o.m.
bad email:

Example of text in the email field:

Y';
 UPDATE table
  SET email = 'hacker@ymail.com'
  WHERE email = 'joe@ymail.com';

and after stored in the database:

Y.\.'.;. .U.P.D.A.T.E. .t.a.b.l.e. .S.E.T. .e.m.a.i.l. .=. .\.'.h.a.c.k.e.r.@.y.m.a.i.l...c.o.m.\'. .W.H.E.R.E. .e.m.a.i.l. .=. .\.'.j.o.e.@.y.m.a.i.l...c.o.m.\.'.;.

So, I don't care if the size of the stored string is bigger because I think this is a solid approach and a very fast one. And if I had to use other ways, the scripts would probably consume the same time that is here exchanged by size.

But now I have the serious problem of XSS attacks. I'm trying to prevent only attacks based on text, not images or JavaScript because I will not have untrusted data there.

The solution I find is based on the last example, and looks like this:

$str = "";
$ad_title = chunk_split($str,1,".");

So when the page loads the users will see the inserted text and the alert will not work:

This is the source code:

<.s.c.r.i.p.t.>.a.l.e.r.t.(.'.f.a.b.i.o.'.).;.<./.s.c.r.i.p.t.>.

Without being concerned about all the code it produces, my question is: is this enough or actually safe?

Is it safe?  Maybe.  Is it the right way to do it?  No.

Even if it were safe, there’s a big problem with the SQL, and that’s that you’re inserting the dots after escaping, rather than before.  I notice that for the input ', it will be escaped to \', and then your dot-insertion turns it into \.'..  Before, the \ was escaping the ', but now it isn’t.  Now it’s escaping the ..  I don’t know if a malicious entity could do anything bad with that, but I wouldn’t take any chances.

And here are some problems with the HTML:

Copy-and-pasting will catch those dots.
Search engines will catch those dots.
You break any Unicode characters, e.g. you turn 日本語 into ?.?.?.?.?.?.?.?.?.. (But I only have an old version of PHP installed; maybe newer versions are more intelligent, but in that case, you might have other problems like putting a combining character onto your >)

And that’s not to mention the gigantic size increase, and potentially having invalid HTML… all around, it’s just not a very good way to do it.

So what’s the right way to do it?  Well, for inserting data into the database, you should be using PDO with prepared statements.  Then you don’t have to deal with SQL escaping at all: you prepare a query with placeholders, and send in the placeholder data, and since the data doesn’t have to touch the query, you don’t have to worry about that at all.

Sidebar: using PDO

You said that it’d be a lot of work to use PDO.  Well, I’ve never found it particularly difficult.  Your code perhaps looks like this:

mysql_connect('localhost', 'myapp', 'letmein');
mysql_select_db('myapp');
// ...
mysql_query("insert into users (firstname, lastname, email) values ('$firstname', '$lastname', '$email')");

But it’s not actually that hard to use PDO.  That code could be translated to use prepared statements and PDO like so:

$db = new PDO('mysql:host=localhost;dbname=myapp', 'myapp', 'letmein');
// ...
$stmt = $db->prepare('insert into users (firstname, lastname, email) values (:firstname, :lastname, :email)');
$stmt->bindValue('firstname', $_POST['firstname']);  // look ma, no escaping!
$stmt->bindValue('lastname', $_POST['lastname']);
$stmt->bindValue('email', $_POST['email']);
$stmt->execute();

And besides being more secure, you’re also moving onto something that the PHP developers have committed to keep in place: the mysql_* functions will be removed, as the PHP documentation says:

  Warning: This extension is deprecated as of PHP 5.5.0, and will be removed in the future. Instead, the MySQLi or PDO_MySQL extension should be used. See also MySQL: choosing an API guide and related FAQ for more information.

End of sidebar

On the HTML side, using htmlentities or htmlspecialchars is the standard way to do it.

Then, finally, I’d like to point out one thing about how you’re accessing POST data: you’re currently using $_POST[firstname].  It’s supposed to be an expression that’s between the brackets, but you have the name of the field literally in there.  It turns out that that’s okay currently, as undefined constants evaluate to their name.  But that does generate a notice, and depending on your error reporting level, you will probably see lots of notices in your error log saying that this is bad and/or deprecated.  You should explicitly quote it: $_POST['firstname'].


Filtering JSON by object property value I am attempting to filter JSON into an array of arrays containing a property value based on the product.  The form is as follows:

[{
    "product": "T0013415_",
    "ordered": 48,
    "shipped": 48,
    "year": 2016,
    "month": 6
}, {
    "product": "T0054113_",
    "ordered": 379,
    "shipped": 379,
    "year": 2016,
    "month": 7
},......]

I am first filtering this JSON into an array of arrays of objects seperated by product with:

var firstPassArray = [];
for(var key in _self.productList)
{
    firstPassArray.push(_self.turnoverMonthlyData.filter(function (el) {
        return el.product == _self.productList[key].product;
    }))
}

The result being an array of arrays of objects:

[[Object, Object, ...],[Object, Object, ...],[Object, Object, ...],...]

I am then getting the actual property values into arrays with

var finalPassArray = [];
for (var key2 in firstPassArray)
{
    var insideArray = [];
    for (var index = 0; index < firstPassArray[key2].length ; index++)
    {
        insideArray.push(firstPassArray[key2][index].ordered);
    }
    finalPassArray.push(insideArray);
}

So that my final results looks like

[[48,264,24,67,54],[4567,511,24,18,5],[4,57,6,11,544],[78,98,77,88,77],[454,54,4,6,8]]

Ignoring bad variable names, is there a better way to do this? Like returning a object property value in the filter() method rather than an object?

It would seem that a simple optimization would be to do it in a single step, i.e. something like:

var finalPassArray = [];
for(var key in _self.productList)
{
    finalPassArray.push(_self.turnoverMonthlyData.filter(function (el) {
        return el.product == _self.productList[key].product;
    }).map( function(el) {{ el.ordered } ) )
}

But I would be tempted to use something like reduce:

var results = _self.turnoverMonthlyData.reduce( function(list, el) {
                if (!list[el.product]) list[el.product] = [];
                list[el.product].push(el.ordered);
                return list;
              }, {} );

Which I tweaked slightly to give results as:

{ T0000001_: [48,264,24,67,54],
  T0000002_ :[4567,511,24,18,5],
  T0000003_: [4,57,6,11,544],
  ...

which might be easier to work with.


Three-way join in Entity Framework Core to retrieve all roles that a user has I'm very new to Entity Framework Core, so I'm concerned about whether I've done this query in the most efficient way possible.

The database I'm querying looks like this:

Basically, UserRoles is a a "linking table" between the User and the Roles in order to create a one-to-many relationship. If a user exists at all, they will always have at least one role, but they could have several roles.

I want to get the role name based on the NT ID of the user.

This is something I plan to do fairly often, so I separated it out into a separate utility (rather than having it in the controller).

In SQL, I'd just do two inner joins. However, what I did here is to query UserRoles (rather than using Join).

public class UserUtilities
{
    public static List<Roles> GetRoles(string ntid)
    {
        var ctx = new MyLiveDocsContext();
        List<Roles> roles =
            (from ur in ctx.UserRoles
             where ur.User.Ntid == ntid
             select ur.Role).ToList();

        return roles;
    }
}

Ive tested this and it does what I want, but I'm wondering if this is the best way to do this. For example, would it be better to use an explicit join instead?

For this isolated piece of code, this is the way to go. Generally, in ORMs that support LINQ, navigation properties (like ur.Roles) should be used wherever possible. Usingjoin should be reserved for custom joins where no navigation properties exist.

Reasons:

Joins are far more verbose than navigation properties, making code harder to read and causing repetitive code.
Joins are error-prone. It's easy to match the wrong fields. A navigation property, once configured correctly, will always generate a correct SQL join.
Navigation properties, if well-named, are more expressive. In a join the cardinality of a relationship isn't always obvious. In navigation properties it is. You know what you're looking at when reading Role vs Roles. Unfortunately, sometimes people tend to be sloppy in this area and they just use the generated class and property names as an ORM generated them, which may result in any mixture of plural class names, plural reference property names, and singular collection property names.

One exception to this rule is when you want to overrule the ORM's SQL generation. Entity Framework will generate an OUTER JOIN for non-required navigation properties (i.e. where the "1"-side is optional, 0..1-n, 0..1-1). There may be cases where INNER JOIN performs better and the results with null navigation properties aren't interesting anyway. In those cases, using a manual join, overriding a navigation property, could be considered.

Side note: as said in a comment, it's recommended to use disposable objects, like a DbContext, in a using statement.


Dot product, ported from C to Swift This is how I posted a method from C to Swift. I tested it well, it seems to work. It's there a way to write this better, following good practices of Swift? The original is in C and the target must be in Swift.

Original C Code:

inline float mac(const float *a, const float *b, unsigned int size)
{
    float sum = 0;
    unsigned int i;

    for (i = 0; i < size; i++)
        sum += (*a++) * (*b++);
    return sum;
}

and the translation to Swift:

func mac(a : [Float], b : [Float], size : Int) -> Float
{
    var sum : Float = 0;

    for (var i = 0; i < size; i++){
        sum += (a[i]) * (b[i]);
    }
return sum;
}

You can remove the size parameter as arrays know their own size in Swift, and use the zip, map and reduce functions to perform the computation rather than the C-style for loop, which is now unavailable in the latest versions of Swift (as is i++):

func mac(a: [Float], b: [Float]) -> Float {
    return zip(a, b).map(*).reduce(0, +)
}

The zip function takes two arrays and makes a single array of tuples, with each tuple containing the nth element from each array.
The map function changes each element of the array with a user-supplied function. In this case, the multiplication operator is passed in, which multiplies each pair.
The reduce function performs the task of summing each result (starting at 0).


Capitalize the first character of all words (even when following a '-') I got it working, all variations are displayed Sint-Anna as should be, but I wonder, is there a simpler way to this, since it looks very cluttered?

String.prototype.capitalize = function(){
    var sa = this.replace(/-/g,' ');
    var saa = sa.toLowerCase();
    var sb = saa.replace( /(^|\s)([a-z])/g , function(m,p1,p2){ return p1+p2.toUpperCase(); } );
    var sc = sb.replace(/\s+/g, '-');
    return sc;
};

console.log('sint-anna'.capitalize());
console.log('sint anna'.capitalize());
console.log('sint-Anna'.capitalize());
console.log('Sint Anna'.capitalize());
console.log('SiNt anna'.capitalize());
console.log('SINT ANNA'.capitalize());

Console:

Sint-Anna
Sint-Anna
Sint-Anna
Sint-Anna
Sint-Anna
Sint-Anna

There are no ways that there is a wrong input like Si ntAn na resulting in Si-Ntan-Na.

Your problem is an interesting one,but your solution is not a great use of regular expressions, since there's the better option of using the 'word boundary' anchor \b.

The boundary anchor matches a transition from a word, to a non-word, or vice versa. For this purpose, using the \w match to target word-characters after the \b is the right solution.

Note that the replace-function you give to the replace operation only needs the m argument now, there's no longer any sub-groups to process.

If your code was written as:

    String.prototype.capitalize = function(){
        return this.toLowerCase().replace( /\b\w/g, function (m) {
            return m.toUpperCase();
        });
    };

then you would convert-to-lowercase, then locate the first letter of any word, and convert that to uppercase.

Putting that in to action, like:

    <input id="myInput" type="text" value="" size="50" />

type something in the box above


Replace an element of a list to get maximum sum of subsequence I was practicing for interview questions, and my friend recommended this one:

Given an initial list of integers L, and a number N, find the smallest member M of L, which on being replaced by N, gives the subsequence with the largest sum S for any subsequence within the modified list.
Examples:
L = [1, 2, -1, 3, 4], N = 4 => M = -1, S = 14
L = [1, 2, 3, 4], N = 4 => M = 1, S = 13
L = [4, 4, 4, 4], N = 4 => M = 4, S = 16
L = [1, 2, 10, -3, -10, 8, 5], N = 4 => M = -10, S = 27
L = [1, 3, 2, -1, 4], N = -2 => M=-1, S = 8

How can I improve this code?
def find_replaceable_element(arr, n):
    max_sum = [0] * len(arr)
    low = [-10000] * len(arr)
    current_sum = 0
    for index, val in enumerate(arr):
        current_sum += val
        max_sum[index] = max(current_sum, val)
        if index >= 1:
            if low[index-1] > val:
                low[index] = val
            else:
                low[index] = low[index-1]
        else:
            low[index] = val

        if val > current_sum:
            current_sum = val
            low[index] = val
    print low
    print max_sum
    max_got = 0
    lowest = 0
    for i, val in enumerate(max_sum):
        cur_max = max_sum[i] - low[i] + n
        if max_got < cur_max:
            max_got = cur_max
            lowest = low[i]
    return lowest, max_got

arr = [1, 2, 10, -3, -10, 8, 5]
print find_replaceable_element(arr, 8)

It's actually a really nice programming quiz.

You tried to implement a linear (\$\mathcal{O}(n)\$ time) solution, and this is the correct final aim, but, I think, you agree that the current solution is hard to understand especially without any comments.

So let's start from something easy-to-understand and not very efficient. But this will allow us to check a correctness of your solution in a first place.

Brute-force solution (\$\mathcal{O}(n^2)\$ time)

It is clear that this task is related to the maximum subarray problem except the condition of replacing by N. So let's make a loop through all positions of the original array; on every iteration let's replace the current element of the array by N and find the largest sum by Kadane's algorithm.

'''
Kadane's algorithm for the Maximum subarray problem
Returns maximum sum
'''
def max_subarray(arr):
    max_sum = -sys.maxint
    cur_sum = 0
    for a in arr:
        cur_sum += a
        max_sum = max(cur_sum, max_sum)
        cur_sum = max(cur_sum, 0)
    return max_sum

def find_replaceable_element0(arr, n):
    max_sum = arr[0]
    max_sum_index = 0
    cur_sum = 0
    for i, a in enumerate(arr):
        arr[i] = n #replace by N in position i
        cur_sum = max_subarray(arr)
        arr[i] = a #change back
        if (cur_sum > max_sum or
                (cur_sum == max_sum and arr[max_sum_index] > arr[i])): #find the smallest
            max_sum = cur_sum
            max_sum_index = i
    return arr[max_sum_index], max_sum

Now we can check correctness of your solution.

import random
random.seed(123)
for i in range(10000):
    arr = random.sample(range(-10, 10), 5)
    n = random.randint(-5, 5)
    if (find_replaceable_element0(arr, n) != find_replaceable_element(arr, n)):
        print(find_replaceable_element0(arr, n))
        print(find_replaceable_element(arr, n))
        print(arr, n)
        break

And for this parameters L = [-9, 9, -3, 8, 4], N = -5 your solution gives M = -3, S = 16. That's wrong because changing -9 on N you can get S = 18.

For sure this task can be solved in \$\mathcal{O}(n)\$ time and \$\mathcal{O}(1)\$ additional memory, but extensive testing should be priority number one.


Displaying a table using DataTables and AngularJS I am using DataTables to format and display a table. I started the project using AngularJS with a PHP backend but I needed to inject some Vanilla JS and JQuery code in order to use DataTables.

Is this approach/organization fine, or should I be trying to use AngularJS modules like ngTable to keep the project "pure".

AngularJS + JQuery snippet for pulling data in DataTable

$http.post('requests.php', data, config).then(
function(response) {
        var df = response.data['data'];

        // Put data into table
        $("#UserTable").DataTable( {
            pagingType: "full_numbers",
            processing : true,
            data: df,
            order: [[ 0, "desc" ]],
            columns:
       [
          { data: "user" },
          { data: "name" }
       ]
    } );
});

PHP BACKEND

if (isset($params['req']) && $params['req'] == "users") {

  $json = array();
  foreach ($cma_DB->query($sql) as $row) {

  //  MAKE ARRAY ASSOCIATIVE; THIS WAS BROKEN IN OTHER VERSIONS OF DATATABLES

    $json[] = array(
      'user'  => $row['user'],
      'name'  => $row['first_name'] . " " . $row['last_name']
    );
  }
    // MAKE RESPONSE HAVE 'data' ENTRY //
    $response = array();
    $response['success'] = true;
    $response['data'] = $json;

    echo json_encode($response);
}

VERSUS

Something written using ngTable

I think your question is architectural in its nature, and it's rather hard to tell whether you want to prerender as much as you can on server side (meaning via PHP) or do most of the work on the client (i.e. via Angular.js). Each approach has its pros and cons.

Here are a few sources that may help you form your opinion about whether to go

client-heavy (would be my default pick)
server-heavy
spread complexity across layers (I'm really not a fan of having bits and pieces of scattered business logic).

To get a more definitive answer about which way to go, you would really need to provide the list of both functional and non-functional requirements to your system.


Most efficient way to validate multiple textboxes against a tolerance value I have a series of text boxes in a table to gather input as below:

The user will input a target and actual value for each measurement point they require. I would then like to validate the actual values against the target values based upon the tolerance inputted in the tolerance textbox. The tolerance will be the same for all measurement points inputted, however the user will not always input all 10 measurement points.

I have also created a very basic class containing a function that accepts the target, actual and tolerance values then returns a Boolean depending on whether the actual value is within tolerance. I realise I could use this with a ruck load of if statements to check each textbox for input then use the class to perform the validation, however this seems like a lot of code repetition and a bit crude. My question being is there a better way I can perform this validation?

Class content

Public Class TolerenceHelper

Public Function IsInTolerence(ByVal target As Integer, ByVal actual As Integer, ByVal tolerence As Integer) As Boolean

    Dim upper As Integer = target + tolerence
    Dim lower As Integer = target - tolerence

    If actual < lower OrElse actual > upper Then
        Return False
    Else
        Return True
    End If
End Function

Calling the function as below:

Dim m1 As TolerenceHelper
Dim flag As Boolean = True

m1 = New TolerenceHelper

If m1.IsInTolerence(Integer.Parse(txtT1.Text), Integer.Parse(txtA1.Text), Integer.Parse(txtTolerance.Text)) = False Then
    flag = False
End If

If flag = False Then
    lblTest.Text = "Out of tolerance"
Else
    lblTest.Text = "In tolerance"
End If

I would write it like this

Dim m1 As TolerenceHelper
Dim flag As Boolean = True

m1 = New TolerenceHelper

flag = m1.IsInTolerence(Integer.Parse(txtT1.Text), Integer.Parse(txtA1.Text), Integer.Parse(txtTolerance.Text))

If flag = False Then
    lblTest.Text = "Out of tolerance"
Else
    lblTest.Text = "In tolerance"
End If

that function returns a boolean so you can just assign it the flag or you could do it like this and avoid creating the flag boolean as well

Dim m1 As TolerenceHelper
m1 = New TolerenceHelper

If m1.IsInTolerence(Integer.Parse(txtT1.Text), Integer.Parse(txtA1.Text), Integer.Parse(txtTolerance.Text)) Then
    lblTest.Text = "In tolerance"
Else
    lblTest.Text = "Out of tolerance"
End If

In your Function IsInTolerence you could eliminate some variables by writing it like this:

Public Function IsInTolerence(ByVal target As Integer, ByVal actual As Integer, ByVal tolerence As Integer) As Boolean

    If (target - tolerence <= actual AND actual =< target + tolerence)
        Return True
    Else
        Return False
    End If
End Function

Straight to the point.

check if it is true first, and then dump out in all other cases, this would cover a null or something else.

Remove Code Duplication with a list of TextBox Objects

I am thinking that you create a list of the text boxes, then you should be able to loop through that list, checking each text box for the following.

if it has a value entered
if the tolerance is met
some kind of highlighting to show that the tolerance was not met.

I am not sure on the syntax, but this would be a good way to do it, because you would only have to type out the logic once and then loop through the list of TextBoxes,  and it would be more maintainable because you could just add TextBoxes any time you like and it shouldn't affect the any of the other code.

dim TextBoxListActual as New List(Of TextBox)
TextBoxListActual.Add(txtT1)
TextBoxListActual.Add(txtT2)
TextBoxListActual.Add(txtT3)

Dim TextBoxListTarget as New List(Of TextBox)
TextBoxListTarget.Add(txtA1)
TextBoxListTarget.Add(txtA2)
TextBoxListTarget.Add(txtA3)

dim m1 As TolerenceHelper
m1 = New TolerenceHelper

For index As Integer = 0 To TextBoxListActual.Count
    If m1.IsInTolerence(Integer.Parse(TextBoxListTarget(index).value), Integer.Parse(TextBoxListActual(index).value), Integer.Parse(txtTolerance.Text))
        lblTest.Text = "In Tolerance"
    Else
        lblTest.Text = "Out of Tolerance"
    End If
Next

little bit of an example.  probably not perfect.