Multiple target method invocation wrapper I'm currently working on a system using WCF to communicate between a Windows Service and one or multiple clients. Service is required to answer clients calls, as well as notify them of certain events, which is why I implemented a callback feature using simplified subscriber model. Callback interface has methods with various signatures and I'd like to use a single method to invoke one of them for all subscribers (placing enumerating over them and exception handling in one place). At first I tried approach with an enum containing interface methods and a switch statement to invoke particular method.

private enum CallbackFunc { Method1, Method2, Method3 }
private static void InvokeCallback(CallbackFunc callbackFunc, params object[] args)
{
    foreach (var subscriber in subscribers)
    {
        try
        {
            switch (callbackFunc)
            {
                case CallbackFunc.Method1:
                    subscriber.Method1((int)args[0]);
                    break;
                case CallbackFunc.Method2:
                    Foo foo = args[0] as Foo;
                    if (foo != null)
                        subscriber.Method2(foo , args.Length > 1 ? args[1] as List<Bar> : null);
                    break;
                case CallbackFunc.Method3:
                    subscriber.Method3((Bar)args[0], (Baz)args[1]);
                    break;
            }
        }
        catch
        {
            //error handling
        }
    }
}

This solution works, but I don't like it very much... it's missing finesse. I figured I could do it more universally using delegates, at which I failed because of different method signatures. Next step was to use reflection with the following result:

private static void InvokeCallback(System.Reflection.MethodInfo func, params object[] args)
{
    foreach (var subscriber in subscribers)
    {
        try
        {
            func.Invoke(subscriber, args);
        }
        catch
        {
            //error handling
        }
    }
}

As you can see the code is much nicer, but I began to doubt if this is actually a better solution (for example because of a little reflection overhead, less nice function call).

public static void Method1(int a)
{
    InvokeCallback(typeof(ICallback).GetMethod("Method1"), a);
    InvokeCallback(CallbackFunc.Method1, a);
}
public static void Method2(Foo foo, List<Bar> bars)
{
    InvokeCallback(typeof(ICallback).GetMethod("Method2"), foo, bars);
    InvokeCallback(CallbackFunc.Method2, foo, bars);
}
public static void Method3(Bar bar, Baz baz)
{
    InvokeCallback(typeof(ICallback).GetMethod("Method3"), bar, baz);
    InvokeCallback(CallbackFunc.Method3, bar, baz);
}

I'd like to ask you which of these approaches is better or suggest a better solution. Any help will be much appreciated, thanks in advance.

Why not expose a lambda expression like this (assuming that _subscribers is a field of type List<ICallback>):

private void InvokeCallbacks(Action<ICallback> callbackFunc)
{
    _subscribers.ForEach(callbackFunc);
}

and usage would be:

public void Method1(int a)
{
    InvokeCallbacks(callback => callback.Method1(a));
}

public void Method2(Foo foo, List<Bar> bars)
{
    InvokeCallbacks(callback => callback.Method2(foo, bars));
}

public void Method3(Bar bar, Baz baz)
{
    InvokeCallbacks(callback => callback.Method3(bar, baz));
}


Sort by value in multidimensional array I have a multidimensional array of house properties:

$array = array(
    array('apn' => 345345353, 'sqft' => 1121, 'address' => '12 Lincoln Ave.'),
    array('apn' => 345345351, 'sqft' => 1643, 'address' => '13 Lincoln Ave.'),
    array('apn' => 345345352, 'sqft' => 1222, 'address' => '14 Lincoln Ave.')
    array('apn' => 345345358, 'sqft' => 1226, 'address' => '6 Olark Ave.')
    array('apn' => 345345345, 'sqft' => 1200, 'address' => '323 Pacific Ave.'),
    array('apn' => 345345342, 'sqft' => 1421, 'address' => '324 Pacific Ave.'),
    array('apn' => 345345347, 'sqft' => 1093, 'address' => '328 Pacific Ave.'),
);

What I want to create from it is an array with just the lowest and highest address for each street. If a street has only one address then it shouldn't show up in the array.

array('323 Pacific Ave.', '328 Pacific Ave.', '12 Lincoln Ave.', '14 Lincoln Ave.')

Currently I have the following function that works find but it is very verbose and I'm trying to figure out a simple way to sort by value in a multidimensional array.

function minMaxAddress($data) {
    $addressRanges = array();
    $streets = array();
    $numProperties = count($data);
    $i = 0;
    foreach ($data as $key => $listing) {
        $address = $listing['address'];
        $street = getStreetName($address);
        if ($i == 0) {
          $addressRanges[] = $address;
        }
        else if ($street != end($streets)) {
          $addressRanges[] = $lastAddress;
          $addressRanges[] = $address;
        }
        if ($i == $numProperties - 1) {
            $addressRanges[] = $address;
        }
        $streets[] = $street;
        $lastAddress = $address;
        $i++;
    }
    $addressRanges = array_diff($addressRanges, array_diff_assoc($addressRanges, array_unique($addressRanges)));
    $addressRanges = array_values($addressRanges);
    return $addressRanges;
}

While this task can be written as a single loop over your data, it would involve unnecessarily convoluted conditionals and temporary variables which will likely be an irritation to future developers (that person might actually be you) that may need to read your code.

I respect your desire to keep things terse, but great code isn't just brief, it is easily comprehensible.  I feel that two loops will be the smartest approach.  In the first loop, create a grouping array with the street name as keys and push each number into its respective street's subarray.

Then filter the group array for qualifying data (2 or more numbers in a group), extract the highest and lowest number, then append the bookend numbers to the street name.

Code: (Demo)

$array = [
    ['apn' => 345345353, 'sqft' => 1121, 'address' => '12 Lincoln Ave.'],
    ['apn' => 345345351, 'sqft' => 1643, 'address' => '13 Lincoln Ave.'],
    ['apn' => 345345352, 'sqft' => 1222, 'address' => '14 Lincoln Ave.'],
    ['apn' => 345345358, 'sqft' => 1226, 'address' => '6 Olark Ave.'],
    ['apn' => 345345345, 'sqft' => 1200, 'address' => '323 Pacific Ave.'],
    ['apn' => 345345342, 'sqft' => 1421, 'address' => '324 Pacific Ave.'],
    ['apn' => 345345347, 'sqft' => 1093, 'address' => '328 Pacific Ave.'],
];

foreach ($array as $row) {
    [$number, $street] = explode(' ', $row['address'], 2);
    $group[$street][] = $number;
}

$result = [];
foreach ($group as $street => $numbers) {
    if (sizeof($numbers) > 1) {
        array_push($result, min($numbers) . ' ' . $street, max($numbers) . ' ' . $street);
    }
}

var_export($result);

Output:

array (
  0 => '12 Lincoln Ave.',
  1 => '14 Lincoln Ave.',
  2 => '323 Pacific Ave.',
  3 => '328 Pacific Ave.',
)


Writing an array to a worksheet When I run my code, I get an "Excel not responding" error at this sub as I step through the code, more specifically the write portion of this code. Can anyone help optimize this? I think this is because the loop is writing so many records cell by cell (50k) records and it's causing my Excel sheet to become unresponsive.

 'writes the nRows and nCols from arr to startCell
    'if fromTop is true it starts reading from the first element of arr, otherwise it starts fromm the last element
    Public Sub writeArrToWS(arr() As Variant, startCell As Range, fromTop As Boolean, nRows As Long, nCols As Long)

        Dim i As Long, j As Long, startRow As Long, startCol As Long
        Dim thisWS As Worksheet
        Dim writeVal As Variant

        Set thisWS = startCell.Worksheet

        startRow = startCell.row
        startCol = startCell.Column

        'clear
        For i = 1 To nRows
            For j = 1 To nCols
                thisWS.Cells(startRow + i - 1, startCol + j - 1).value = ""
            Next j
        Next i

        'write
        For i = 1 To WorksheetFunction.Min(nRows, UBound(arr, 1))
            For j = 1 To nCols
                If fromTop Then writeVal = arr(i, j) Else writeVal = arr(UBound(arr, 1) - i + 1, j)
                thisWS.Cells(startRow + i - 1, startCol + j - 1).value = writeVal
            Next j
        Next i

    End Sub

This minimizes the interaction with the range (as mentioned by @M.Doerner)
and clears the previous data in one operation (mentioned by @Patrick)

Determines the range to work with

Minimum between arr().Rows and nRows, and arr().Cols and nCols
Establishes the UsedRange based on startCell, and Cells(nRows, nCols)

Clears entire UsedRange
If fromTop is True places arr() into the UsedRange
Else, reverses rows in arr() into variant array result, then places it into UsedRange

Rows: 100,001; Cols: 20; fromTop: True  - Time: 4.461 sec
Rows: 100,001; Cols: 20; fromTop: False - Time: 4.734 sec

Option Explicit

'writes the 2-D arr to startCell; if fromTop is False it starts from the last row
Public Sub WriteArrToWS(ByRef arr() As Variant, ByRef startCell As Range, _
                        ByVal fromTop As Boolean, nRows As Long, nCols As Long)

    nRows = IIf(nRows < UBound(arr, 1), nRows, UBound(arr, 1))
    nCols = IIf(nCols < UBound(arr, 2), nCols, UBound(arr, 2))

    Dim ws As Worksheet, fr As Long, fc As Long, ur As Range

    Set ws = startCell.Worksheet
    fr = startCell.Row
    fc = startCell.Column
    Set ur = ws.Range(ws.Cells(fr, fc), ws.Cells(fr + nRows - 1, fc + nCols - 1))
    ur.ClearContents

    Dim result As Variant, r As Long, c As Long, lr As Long

    result = ur
    result = arr            'Defaults to fromTop = True
    If Not fromTop Then     'If fromTop = False, reverses the rows in the array
        lr = nRows
        For r = 1 To nRows
            For c = 1 To nCols
                result(r, c) = arr(lr, c)
            Next
            lr = lr - 1
        Next
    End If
    ur = result
End Sub


Calculating IRR (Internal Rate of Return) in Ruby using recursion Is there a way to improve this code? (specially how I built the recursion and the breaking out of the loops and conditionals)

# Returns sum of present value of a sequence of cash flows
# rate = interest rate (month)
# initial amount = balance at the beginning
# *amounts = sequence of cash flows (1 or more)

def present_value_of_series(rate, initial_amount, *amounts)
  present_value = initial_amount
  i = 0
  while i < amounts.length
    present_value += amounts[i] / ( (1+rate)**(i+1) )
    present_value
    i +=1
  end
  return present_value
end

# Returns the IRR (Internal Rate of Return) for a sequence of cash flows
#
# IRR definition: the IRR is the rate at which the present value of a sequence of cash flows equals zero (or close to zero). It is reached by trial and error, using the steps below:
#
# 1. Define lower limit and upper limit for the rate - this is the range that will be tested
# 2. Define the rate increment at each iteration
# 3. Calculate the present value of a sequence of cash flows starting at the lower limit and keep incrementing according to defined increment
# 4. When the present value found is below zero, it means that a new lower and upper limit have been found
# 5. Increase calculation precision using a recursive function, this time incrementing at 10x more finer increments.

def irr(lower_guess, higher_guess, increment, *amounts)
  rate = lower_guess
  precision_level = 1
  while rate <= higher_guess
    if present_value_of_series(rate, *amounts) < 0
      if present_value_of_series(rate, *amounts) >= -precision_level
        #debug puts "present value = #{present_value_of_series(rate, *amounts)}"
        puts "rate = #{rate}"
        puts "rate = #{(rate*100).round(2)}% (rounded)"
        # returns annual rate
        annual_rate = (1+rate)**12-1
        puts "annual rate = #{annual_rate}"
        puts "annual rate = #{(annual_rate*100).round(2)}% (rounded)"
        break
      else
        irr((rate-increment), rate, increment/10, *amounts)
        break
      end
    else
      rate += increment
    end
  end
end

# Usage Example:
# 30000 loan to be paid in 12 installments of different values
# lower_guess = 0.005 (0.5%)
# higher_guess = 0.5 (50%)
# increment = 0.01 (1%)

puts irr(0.005, 0.50, 0.01, -30000.00, 2986.55, 2954.11, 2921.68, 2889.25, 2856.82, 2824.38, 2791.94, 2759.51, 2727.07, 2694.65, 2662.21, 2629.77)

Sanity checking
You should probably add some sanity checks. It's entirely possible to provide a range that'll never produce a result.
Separate output from calculation
Your test code ends up printing an extra blank line because #irr doesn't actually return anything. It'd be nicer to let #irr actually return the rate it found; you can do whatever you want with it afterward. Right now it just prints.
Too many splats
Variadic arguments are neat, but aren't really that necessary here. It'd be simpler and more straightforward to pass an actual array for the amounts.
Repetition
No need to do call present_value_of_series two-three times in a row, like you do here:

if present_value_of_series(rate, *amounts) < 0
  if present_value_of_series(rate, *amounts) >= -precision_level
    #debug puts "present value = #{present_value_of_series(rate, amounts)}"

Just stick it in a variable.
#present_value_of_series
This can be simplified by not treating the initial value as a separate argument, and using #reduce to do the sum:

def present_value_of_series(rate, amounts)
  amounts.each_with_index.reduce(0) do |sum, (amount, index)|
    sum + amount / (rate + 1)**index
  end
end

Note that this also matches your other code where you do treat the amounts as an array. No need for * splats.

As a general note, you rarely need while or for loops when iterating arrays in Ruby. There's almost always a neater way.
#irr
Since you're dealing with a range, it'd be nice to use, well, a Range. A range also gives you a #step iterator, which is useful here.

Here's what I came up with. It works the same as yours (recursion), but I can't say I'm super happy with it - returning from inside the #detect block feel wrong, but it avoids recalculating the present value afterward to check it against -1.

def irr(range, step, amounts)
  rate = range.step(step).detect do |rate|
    present_value = present_value_of_series(rate, amounts)
    if present_value < 0
      return rate if present_value >= -1
      true
    end
  end

  if rate.nil?
    raise "No solution"
  else
    irr((rate-step..rate), step / 10, amounts)
  end
end

Still needs extra sanity checks, though.

A possible optimization (trading some memory for computation) would be to use Array#bsearch instead of #detect. It does however mean that the range has to be made into an array first (e.g. range.step(step).to_a.bsearch ...).

Speaking of binary searching, you could also do:

def irr(min_rate, max_rate, amounts)
  range = max_rate - min_rate
  raise "No solution" if range <= Float::EPSILON * 2

  rate = range.fdiv(2) + min_rate
  present_value = present_value_of_series(rate, amounts)

  if present_value > 0
    irr(rate, max_rate, amounts)
  elsif present_value < -1
    irr(min_rate, rate, amounts)
  else
    rate
  end
end

Basic idea is that it keeps guessing in the direction of the answer by testing a rate midway between min_rate and max_rate. It gives up if the difference between min_rate and max_rate would introduce floating point precision loss.

It's the fastest solution of the bunch, though it could perhaps be smarter about checking the initial min/max before starting the iteration (it could do that up front by calculating present values for max and min, and only then, if the range makes sense, handle the iteration in separate method). It should probably have a precision argument as well, but that's trivial.


STL vector implementation I've implemented a simple vector-like structure.  I would appreciate all criticism relevant to code, style, flow, camelCase vs underscore, and so forth.

template <class T>
class  Vector {
public:

    typedef T* Iterator;

    Vector();
    Vector(unsigned int size);
    Vector(unsigned int size, const T & initial);
    Vector(const Vector<T>& v);
    ~Vector();

    unsigned int capacity() const;
    unsigned int size() const;
    bool empty() const;
    Iterator begin();
    Iterator end();
    T& front();
    T& back();
    void push_back(const T& value);
    void pop_back();

    void reserve(unsigned int capacity);
    void resize(unsigned int size);

    T & operator[](unsigned int index);
    Vector<T> & operator = (const Vector<T> &);
    void clear();
private:
    unsigned int _size;
    unsigned int _capacity;
    unsigned int Log;
    T* buffer;
};

template<class T>
Vector<T>::Vector() {
    _capacity = 0;
    _size = 0;
    buffer = 0;
    Log = 0;
}

template<class T>
Vector<T>::Vector(const Vector<T> & v) {
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;
    buffer = new T[_size];
    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
}

template<class T>
Vector<T>::Vector(unsigned int size) {
    _size = size;
    Log = ceil(log((double) size) / log(2.0));
    _capacity = 1 << Log;
    buffer = new T[_capacity];
}

template <class T>
bool Vector<T>:: empty() const {
    return _size == 0;
}

template<class T>
Vector<T>::Vector(unsigned int size, const T& initial) {
    _size = size;
    Log = ceil(log((double) size) / log(2.0));
    _capacity = 1 << Log;
    buffer = new T [_capacity];
    for (unsigned int i = 0; i < size; i++)
        buffer[i] = initial;
}

template<class T>
Vector<T>& Vector<T>::operator = (const Vector<T> & v) {
    delete[] buffer;
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;
    buffer = new T [_capacity];
    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
    return *this;
}

template<class T>
typename Vector<T>::Iterator Vector<T>::begin() {
    return buffer;
}

template<class T>
typename Vector<T>::Iterator Vector<T>::end() {
    return buffer + size();
}

template<class T>
T& Vector<T>::front() {
    return buffer[0];
}

template<class T>
T& Vector<T>::back() {
    return buffer[_size - 1];
}

template<class T>
void Vector<T>::push_back(const T & v) {
    /*
        Incidentally, one common way of regrowing an array is to double the size as needed.
        This is so that if you are inserting n items at most only O(log n) regrowths are performed
        and at most O(n) space is wasted.
    */
    if (_size >= _capacity) {
        reserve(1 << Log);
        Log++;
    }
    buffer [_size++] = v;
}

template<class T>
void Vector<T>::pop_back() {
    _size--;
}

template<class T>
void Vector<T>::reserve(unsigned int capacity) {
    T * newBuffer = new T[capacity];

    for (unsigned int i = 0; i < _size; i++)
        newBuffer[i] = buffer[i];

    _capacity = capacity;
    delete[] buffer;
    buffer = newBuffer;
}

template<class T>
unsigned int Vector<T>::size() const {
    return _size;
}

template<class T>
void Vector<T>::resize(unsigned int size) {
    Log = ceil(log((double) size) / log(2.0));
    reserve(1 << Log);
    _size = size;
}

template<class T>
T& Vector<T>::operator[](unsigned int index) {
    return buffer[index];
}

template<class T>
unsigned int Vector<T>::capacity()const {
    return _capacity;
}

template<class T>
Vector<T>::~Vector() {
    delete[] buffer;
}

template <class T>
void Vector<T>::clear() {
    _capacity = 0;
    _size = 0;
    buffer = 0;
    Log = 0;
}

Your code is C++03 like (ie there are no move constructors or move assignment operators). You should definitely think about updating your class to be move aware.

The first thing about the interface is that the assignment operator is not close to the constructors. Since these are highly linked I like to place the assignment operators very close to the constructors (see rule of three/five).

Using _ as a prefix is a bad idea. You don't break any rules (but was this on purpose or just an accident?). The rules are sufficiently complex that prefix _ on identifiers is a bad idea for user space code. Others have suggested a prefix like m_ personally I think that is old school advice. As long as the naming is clear there should be no issues.

My personal convention with identifiers is that anything that can be an object starts with a lowercase letter. Anything that is a type begins with an uppercase letter. Others will disagree with this convention as the standard does not follow it and this can lead to other conflicts.

Here is one of those exceptions to my rules.

    typedef T* Iterator;

You should probably have iterator with a lowercase i. This is because a lot of the algorithms will use the type.

Good start.

    Iterator begin();
    Iterator end();
    T& front();
    T& back();
    T & operator[](unsigned int index);

But you will need const versions of all these (and a const_iterator type) to be a fully compliant container.

You are going to have a hard time making this work if the buffer is of type T. Every time you expand the buffer all the elements in the buffer will be initialized with T constructor. For int this is not a problem. But if T has a non trivial constructor then you are going to pay a heavy price initializing elements that may never be used.

T* buffer;

Really the buffer should be something that does not have a constructor.

char* buffer;

Prefer the initializer list.

template<class T>
Vector<T>::Vector() {
    _capacity = 0;
    _size = 0;
    buffer = 0;
    Log = 0;
}

// This will also warn about out of order initialization.
// Not a big thing but it keeps your thinking logical.

template<class T>
Vector<T>::Vector()
    : _size(0)
    , _capacity(0)
    , Log(0)
    // Personally I would always allocate some space for the buffer
    // even if the size is zero. This way I do not have to worry about
    // the special case of buffer ever being a NULL pointer.
    //
    // Having a special case of NULL will affect all the methods and make
    // the code much more complex. Reduce code complexity means easier to
    // maintain code.
    //
    , buffer(new char[sizeof(T) * capacity])
    // Note this assumes buffer is (char* as described above)
{}

Again. Use an initializer list.

template<class T>
Vector<T>::Vector(const Vector<T> & v) {
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;

Do you really mean to allocate _size memebers to the buffer. Should this not be _capacity?

    buffer = new T[_size];

    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
}

Also use the internal algorithms to help you.

template<class T>
Vector<T>::Vector(Vector<T> const& v)
 // Note. I put cost here   ^^^^^   most of the time it does not matter.
 // There is one corner case where it does.
    : _size(v._size)
    , _capacity(v._capacity)
    , Log(v.Log)
    , buffer(new char[sizeof(T) * _capacity])
{
    for (unsigned int i = 0; i < _size; i++)
    {
        // Now you can use placement new to copy the elements from
        // source into the current vector.
        new (buffer + sizeof(T) * i) T(v[i]);
    }
}

Again use initializer list.

template<class T>
Vector<T>::Vector(unsigned int size) {
    _size = size;

This seems to be a constant. I would just make this a constant of the class and calculate it once by hand. The use of these maths functions seems a bit like overkill to me.

    Log = ceil(log((double) size) / log(2.0));
    _capacity = 1 << Log;
    buffer = new T[_capacity];
}

Personally one liners like this.

template <class T>
bool Vector<T>:: empty() const {
    return _size == 0;
}

I just put inline into the class.

The assignment operator problem has been covered by @liv902

template<class T>
Vector<T>& Vector<T>::operator = (const Vector<T> & v) {
    delete[] buffer;
    _size = v._size;
    Log = v.Log;
    _capacity = v._capacity;
    buffer = new T [_capacity];
    for (unsigned int i = 0; i < _size; i++)
        buffer[i] = v.buffer[i];
    return *this;
}

Basically you should not change the state of your object until you have guranteed that there are no expceptions can be thrown and leave your object in an incosistent state. This means your assignemnt should proceed in three stages.

Copy the state of the src class into temporary objects.
This is because making the copy can throw an exception and thus you should not put your object into a state wheere it can be invalid.
Swap the state of you object with the temporary values.
Swapping is an exception safe operation.
Now you can destroy the old state (which is in the temporary values).
This can also throw an exception, but your object will be in a consistent state.

This techniques means you provide the "Strong (Transaction) Exception Guarantee" (You can look it up in google).

The easiest way to implement the above is to use the "Copy and Swap Idiom".

When you remove something from a std::vector.

template<class T>
void Vector<T>::pop_back() {
    _size--;
}

The destructor for that object is also called. This allows you free resources for non trivial resources (say T was a std::shared_ptr<int>) not calling the destructor means you are effectively retaining a reference to an object that could be freed.

It is perfectly valid to manually call the destructor of an object (In fact this is very normal when pared with placement new operation).

template<class T>
void Vector<T>::pop_back() {

    // You way want to do a check for empty() before calling the destructor.

    // Call the destructor.
    (reinterpret_cast<T*>(buffer)[_size-1]).~T();

    // It is usual to prefer the pre decrement (and pre increment).
    --_size;
}

Just like a pop.
When you clear a vector you generally expect the destructor to all the members to be called.

template <class T>
void Vector<T>::clear() {
    _capacity = 0;
    _size = 0;
    buffer = 0;
    Log = 0;
}


Initializing values of a singly linked list with a constructor I have to create a constructor that allows a new linked list to be populated with ten consecutive values, starting at 0. Then I need to print the list! So i want to check if the functions I wrote for those are ok.

#include <iostream>
#include <string>
#include <stdexcept>

using namespace std;

template <typename E> class SLinkedList;    // forward declaration to be used when declaring SNode

template <typename E>
class SNode {
private:
    E elem;
    SNode<E> *next;
    friend class SLinkedList<E>;
};

template <typename E>
class SLinkedList {
public:
    SLinkedList();
    SLinkedList(SNode<E>* v);   //What I need help with
    ~SLinkedList();
    bool empty() const;
    E& front();
    void printList(SLinkedList<E> &list); //what i need help with
    void addFront(const E& e);
    void removeFront();
    int size() const;
private:
    SNode<E>* head;
    int     n;                          // number of items
};

template <typename E>
SLinkedList<E>::SLinkedList()           // constructor
    : head(NULL), n(0) { }

template <typename E>
SLinkedList<E>::SLinkedList(SNode<E>* v){ //WHat I  Need Help With
    SNode<E>* v = new SNode<E>;
    for (int i = 0; i < 10; i++)
        v->elem = i;
}

template <typename E>
bool SLinkedList<E>::empty() const
{
    return head == NULL; // can also use return (n == 0);
}

template <typename E>
E& SLinkedList<E>::front()
{
    if (empty()) throw length_error("empty list");
    return head->elem;
}

template <typename E>
SLinkedList<E>::~SLinkedList()
{
    while (!empty()) removeFront();
}

template<typename E>
void SLinkedList<E>::printList(SLinkedList<E> &list) //What I need help with
{
    for (int i = 0; i < list.size(); i++)
    {
        cout << list << " ";
    }
    cout << endl;
}

template <typename E>
void SLinkedList<E>::addFront(const E& e) {
    SNode<E>* v = new SNode<E>;     // create new node
    v->elem = e;                // store data
    v->next = head;             // head now follows v
    head = v;               // v is now the head
    n++;
}

template <typename E>
void SLinkedList<E>::removeFront() {
    if (empty()) throw length_error("empty list");
    SNode<E>* old = head;
    head = old->next;
    delete old;
    n--;
}

template <typename E>
int SLinkedList<E>::size() const {
    return n;
}

Don't do this:
using namespace std;

Bringing everything from std into the global namespace causes more issues than it is worth. I know every book on the subject does it. But they do it because of size limitations in their medium. You should never do this in real code.
See: Why is “using namespace std” considered bad practice?
This should not be a public class:
template <typename E>
class SNode {
private:
    E elem;
    SNode<E> *next;
    friend class SLinkedList<E>;
};

Make it a private member class of SLinkedList. Then it gets smaller. You can make it struct, and you won't need the friend declaration or the <E> sprinkled everywhere.
That does not look correct:
SLinkedList(SNode<E>* v);   //What I need help with

Why would somebody want to create a linked list with a node? Why are you passing a pointer to the node? What is the ownership semantics of the pointer? So many unanswerable questions. Just get rid of this constructor.
I don't see any of the normal constructors I would expect for a container that is doing memory management (and you are doing memory management). Please look up the rule of three.
// I would expect to see
SLinkedList(SNode<E> const& copy);
SLinkedList& operator=(SLinkedList const& copy);

For bonus points add move semantics:
SLinkedList(SLinkedList&& move) noexcept;
SLinkedList& operator(SLinkedList&& move) noexcept;
void swap(SLinkedList& other) noexcept;

Yeahh. Stop using NULL; that's an old hangover for C (we don't like C here). C++ has a better identifier. Use nullptr; it has better type information associated with it (so it will not be converted to a integer type accidentally).
Sure you can throw.
template <typename E>
E& SLinkedList<E>::front()
{
    if (empty()) throw length_error("empty list");
    return head->elem;
}

But do you really want to?
I am an expert user and will always check that list is not empty before calling front(). But you are making me pay a penalty (the extra check (I just checked)) because there are idiots out there that don't check. Why are you penalizing me (the good guy) when the bad guys are the ones you should be penalizing?
Sure have a checking version. But the standard version should not check.
OK. This is obviously not working:
template<typename E>
void SLinkedList<E>::printList(SLinkedList<E> &list) //What I need help with
{
    for (int i = 0; i < list.size(); i++)
    {
        cout << list << " ";
    }
    cout << endl;
}

You want:
template<typename E>
void SLinkedList<E>::printList(SLinkedList<E> &list) //What I need help with
{
    for (auto item = head; item != nullptr; item = item->next)
    {
        cout << item->elem << " ";
    }
    cout << "\n";
}

Prefer '\n' to std::endl.
The only difference between the two is that std::endl forces a stream to flush. This is never the correct thing to do (unless you are an expert and understand stream flushing to byte level (or you are debugging - in which case you should be using an unbuffered stream like std::cerr)).
Incorrectly flushing the buffer is the MAIN cause of inefficiency in C++ I/O application. The standard streams are quite good at correctly flushing for efficiency; use the code somebody else has spent hours perfecting for your system.
This all looks fine:
SNode<E>* v = new SNode<E>;     // create new node
v->elem = e;                // store data
v->next = head;             // head now follows v
head = v;               // v is now the head
n++;

But would it not look a lot nicer if it was:
head = new SNode(e, head);
++n;

Also prefer to use prefix increment over postfix. The majority of the time it will make no difference. But with iterators (it can have a tiny impact). Since iterators are prevalent all over C++ code this could make a difference. There is no cost to prefix operations.
See: How to overload the operator++ in two different ways for postfix a++ and prefix ++a?
Additions

I have to create a constructor that allows a new linked list to be populated with ten consecutive values, starting at 0.

A special constructor for adding ten items is a bit strange. But we can do it. Let's create a marker enum:
enum Marker { MakeTenItems };
SLinkedList(Marker)
    : head(nullptr)
{
    for(int loop = 0; loop < 10; ++loop)
    {
        addFront(10 - 1 - loop);  // This will construct an E with an int
                                  // So E must have an appropriate constructor.
    }
}

Then to use it:
SLinkedList<int>  list(MakeTenItems);


Hosting a game on a server I have the following code but it's a bit messy and hard to look at. I could include a function to host a game so that block of code isn't repeating, but could this be optimized any further?

server.em.addListener('hostGame', function(settings)
{
    if (Object.keys(hostingGames).length != 0) //If nobody is hosting a game
        {
            for (i in hostingGames) //If there is more than 1 game being hosted, scroll through each one
            {
                if(hostingGames[i].userId === settings.userId) //Check if any of them are by the current user
                {
                    break; //if there is, dont let them host a new one
                }
                else
                {
                    var hostGame = new HostGame(); //If the player isnt hosting a game, host a new game for them

                    hostGame.initialise(settings.userId, settings.userName, settings.boardSize, settings.gameMode, settings.gameNote);
                    hostingGames[settings.userId] = hostGame;
                    server.consoleLog('APP', settings.userName + ' is hosting a game. ID: ' + settings.userId);
                    server.updateGamesList(hostingGames);
                }
            }
        }
        else //If there is no games being hosted, the user must not be hosting any, so let them host a new game
        {
            var hostGame = new HostGame();

            hostGame.initialise(settings.userId, settings.userName, settings.boardSize, settings.gameMode, settings.gameNote);
            hostingGames[settings.userId] = hostGame;
            server.consoleLog('APP', settings.userName + ' is hosting a game. ID: ' + settings.userId);
            server.updateGamesList(hostingGames);
        }
});

Your indents could use a little work.
Typical JS use the collapsed braces or "egyptian braces", where the opening brace is on the same line as the opening statement.
Use template literals to construct strings.
Comments be their own line. That way, they're readable from the very start and don't run off to the right.
Do the "early return" pattern, where you return on invalid cases until you can assume validity. That way, you avoid deeply nested conditions.

Here's a more concise version:

server.em.addListener('hostGame', function(settings){

  // Get essential values. You can inline them, but I prefer putting them in
  // variables for clarity.
  const gameKeys = Object.keys(hostingGames);
  const gameUids = gameKeys.map(key => hostingGames[key].userId);
  const userHasGame = gameUids.includes(settings.userId);

  // The only time the code ever bails out is when the user has a game.
  if(userHasGame) return;

  // userHasGame can also be false if there are no games since gameKeys will
  // be empty, causing gameUids to be empty, causing includes to be false.

  // Otherwise, a game is created.
  const hostGame = new HostGame();
  hostGame.initialise(settings.userId, settings.userName, settings.boardSize, settings.gameMode, settings.gameNote);
  hostingGames[settings.userId] = hostGame;
  server.updateGamesList(hostingGames);

  server.consoleLog('APP', `${settings.userName} is hosting a game. ID: ${settings.userId}`);
});


Implementing an open/close function in Prolog I'm going through the Adventure in Prolog tutorial and I'm trying to implement an open/close rule for each of the doors I have.

  Add an open/closed status for each of the doors. Write open and close predicates that do the obvious

:- dynamic here/1, location/2, have/1, turned_on/1, turned_off/1.
room(kitchen).
room(office).
room(hall).
room('dining room').
room(cellar).

location(desk, office).
location(apple, kitchen).
location(flashlight, desk).
location('washing machine', cellar).
location( nani,'washing machine' ).
location(broccoli, kitchen).
location(crackers, kitchen).
location(computer, office).

door(office, hall).
door(kitchen, office).
door(kitchen, cellar).
door(hall, 'dining room').
door('dining room', kitchen).

edible(apple).
edible(crackers).

tastes_yucky(broccoli).

turned_off(flashlight).

here(kitchen).

connect(X, Y) :- door(X, Y).
connect(X, Y) :- door(Y, X).

list_things(Place) :-
  location(X, Place),
  tab(2),
  write(X), nl,
  fail.
list_things(AnyPlace).

list_connections(Conn) :-
  connect(Conn, X),
  tab(2),
  write(X),
  nl,
  fail.
list_connections(_).

look :-
  here(X),
  write('you are in the '), write(X), nl,
  write('you can see:'), nl,
  list_things(X),
  write('you can go to:'), nl,
  list_connections(X).

goto(Place) :-
  can_go(Place),
  move(Place),
  look.

can_go(Room) :-
  here(X),
  connect(X, Room).
can_go(Room) :-
  write('You can\'t go there from here'),nl,
  fail.
move(Place):-
  retract(here(X)),
  asserta(here(Place)).

take_object(X) :-
  retract(location(X, _)),
  asserta(have(X)).
  write('taken'), nl.

put(X) :-
  retract(have(X)),
  asserta(location(X,_)).
inventory:-
  have(X),
  write('you have the following items:'), nl,
  write(X).
turn_on(X):-
  turned_off(X),
  retract(turned_off(X)),
  asserta(turned_on(X)),
  write('flash light turned on').
turn_on(X):-
  write('you can\'t turn that on').

turn_off(X):-
  turned_on(X),
  retract(turned_on(X)),
  asserta(turned_off(X)),
  write('flash light is now off').
turn_off(X):-
  write('you can\'t turn that off').

Now to actually add the rules:

door(office, hall).
door(kitchen, office).
door(kitchen, cellar).
door(hall, 'dining room').
door('dining room', kitchen).

opened(office, hall).
closed(kitchen, office).
opened(kitchen, cellar).
closed(hall, 'dining room').
opened('dining room', kitchen).

open(X, Y) :-
  closed(X, Y),
  retract(closed(X, Y)),
  asserta(opened(X, Y)),
  writeln('door opened').
open(X, Y) :-
  closed(Y, X),
  retract(closed(Y, X)),
  asserta(opened(Y, X)),
  writeln('door opened').
close(X, Y) :-
  opened(X, Y),
  retract(opened(X, Y)),
  asserta(closed(X, Y)),
  writeln('door closed').
close(X, Y) :-
  opened(Y, X),
  retract(opened(Y, X)),
  asserta(closed(Y, X)),
  writeln('door closed').

I'm only concerned  that I'm using the open(X,Y) twice (also the close).  Is there a better way?

Always try hard to avoid assert/1 and retract/1: Using these predicates makes it impossible to reason about your other predicates in isolation, and therefore make debugging and extending your code much harder.

In this case, there is a clear declarative solution:

Represent the state of all doors as a Prolog term, and thread this state through all predicates that need it. For example, a simple representation could be:

[door_state(door1, open), door_state(door2, closed), door_state(door3, open)]

You can thread such a state through as follows:

state0_action_state(S0, Action, S) :-
    ...

This predicate relates an initital state to the next state, depending on Action. Given the example representation above, an action like close(door1) would result in the next state:

[door_state(door1, closed), door_state(door2, closed), door_state(door3, open)]

Thus, your predicates can all be tested in isolation, and do not depend on any implicit state.


Recursive Fibonacci in Java I just started learning recursion and memoization, so I decided to give a simple implementation a shot. Works with some simple test cases I came up with. Is there anything I can add to make this more interesting? I'm not sure if I can update the return type with generics somehow depending on the input "n" for long, BigInteger, etc.

public static int fibonacci(int n) {
    if (n < 0) throw new IllegalArgumentException("n must be non-negative.");
    if (n == 0) return 0;
    if (n == 1) return 1;
    // if n >= 2
    Map<Integer, Integer> hm = new HashMap<Integer, Integer>(); // cache
    hm.put(0, 0);
    hm.put(1, 1);
    return fibHelper(n-1, hm) + fibHelper(n-2, hm);
}

static int fibHelper(int n, Map<Integer, Integer> hm) {
    if (hm.containsKey(n)) return hm.get(n);
    int fn = fibHelper(n-1, hm) + fibHelper(n-2, hm); // fn = fib of n
    hm.put(n, fn);
    return fn;
}

Have you tried running this with a "big" input? Try it with fibonacci(10000) for example. At least on my machine with default settings this gives a StackOverflowError.

This is because java doesn't handle recursion well. Since you're going to recalculate all the values for each call to ´fibonacci(n)` you might as well solve it with a simple for loop instead.

public static int fibonacci(int n) {
    if (n < 0) throw new IllegalArgumentException("n must be non-negative.");
    if (n == 0) return 0;
    if (n == 1) return 1;

    int f1 = 0;
    int f2 = 1;
    for(int i = 2; i <= n; i++){
        int temp = f2;
        f2 += f1;
        f1 = temp;
    }
    return f2;
}

It's not possible to change the return type dynamically based on a runtime value. Seeing how fibonacci numbers grow incredibly fast, you might want to consider returning BigInteger by default. (For ints, you can't even get to number 50).

public static BigInteger fibonacci(int n) {
    if (n < 0) throw new IllegalArgumentException("n must be non-negative.");
    if (n == 0) return BigInteger.ZERO;
    if (n == 1) return BigInteger.ONE;

    BigInteger f1 = BigInteger.ZERO;
    BigInteger f2 = BigInteger.ONE;
    for(int i = 2; i <= n; i++){
        BigInteger temp = f2;
        f2 = f2.add(f1);
        f1 = temp;
    }
    return f2;
}

Personally I would also not memoize fibonacci numbers further than these 2 temp veriables to keep track of the last 2 values. If your program is intended to request fibonacci numbers over and over and you're willing to get a little bit of speed in exchange for some memory you could also precompute a lot of those numbers. Except for learning purposes I don't think it's actually worth doing though.


Size of the largest connected component in a grid in Python Given an R x C grid of 1s and 0s (or True and False values), I need a function that can find the size of the largest connected component of 1s. For example, for the following grid,

grid = [[0, 1, 0, 1],
        [1, 1, 1, 0],
        [0, 1, 0, 0],
        [0, 0, 0, 1]]

The answer is 5.

Here is my implementation:

def largest_connected_component(nrows, ncols, grid):
    """Find largest connected component of 1s on a grid."""

    def traverse_component(i, j):
        """Returns no. of unseen elements connected to (i,j)."""
        seen[i][j] = True
        result = 1

        # Check all four neighbours
        if i > 0 and grid[i-1][j] and not seen[i-1][j]:
            result += traverse_component(i-1, j)
        if j > 0 and grid[i][j-1] and not seen[i][j-1]:
            result += traverse_component(i, j-1)
        if i < len(grid)-1 and grid[i+1][j] and not seen[i+1][j]:
            result += traverse_component(i+1, j)
        if j < len(grid[0])-1 and grid[i][j+1] and not seen[i][j+1]:
            result += traverse_component(i, j+1)
        return result

    seen = [[False] * ncols for _ in range(nrows)]

    # Tracks size of largest connected component found
    component_size = 0

    for i in range(nrows):
        for j in range(ncols):
            if grid[i][j] and not seen[i][j]:
                temp = traverse_component(i, j)
                if temp > component_size:
                    component_size = temp

    return component_size

Feel free to use the following code to generate random grids to test the function,

from random import randint

N = 20
grid = [[randint(0,1) for _ in range(N)] for _ in range(N)]

Problem: My implementation runs too slow (by about a factor of 3). Since I wrote this as a naive approach by myself, I am guessing there are clever optimizations that can be made.

Context: This is for solving the Gridception problem from Round 2 of Google Codejam 2018. My goal is to solve the problem in Python 3. As a result, there is a hard constraint of using only the Python 3 standard library.

I have figured out that this particular portion of the full solution is my performance bottleneck and thus, my solution fails to clear the Large Input due to being too slow.

Thank you so much!

Edit: Adding some timeit benchmarks

For a randomly generated 20 x 20 grid, my implementation takes 219 +/- 41 μs (a grid of 0s takes 30 μs, and a grid of 1s takes 380 μs).

In programming challenges, proper performances improvement usually come from a smarter algorithm. Unfortunately, I have no algorithm better than the one you have implemented.

I have found a single trick to shave off some time.

Remove all the logic around seen

In all places where you access elements from grid and seen, we have basically: if grid[pos] and not seen[pos].

An idea could be to update grid in place to remove seen elements from it. From an engineering point of view, it is not very nice: I would not expect an function computing the size of the biggest connected components to update the provided input. For a programming challenge, we can probably accept such a thing...

We'd get:

def largest_connected_component(nrows, ncols, grid):
    """Find largest connected component of 1s on a grid."""

    def traverse_component(i, j):
        """Returns no. of unseen elements connected to (i,j)."""
        grid[i][j] = False
        result = 1

        # Check all four neighbours
        if i > 0 and grid[i-1][j]:
            result += traverse_component(i-1, j)
        if j > 0 and grid[i][j-1]:
            result += traverse_component(i, j-1)
        if i < len(grid)-1 and grid[i+1][j]:
            result += traverse_component(i+1, j)
        if j < len(grid[0])-1 and grid[i][j+1]:
            result += traverse_component(i, j+1)
        return result

    # Tracks size of largest connected component found
    component_size = 0

    for i in range(nrows):
        for j in range(ncols):
            if grid[i][j]:
                temp = traverse_component(i, j)
                if temp > component_size:
                    component_size = temp

    return component_size

Another idea in order to do the same type of things without changing grid could be to store "positive" elements in a set. This also remove the need to check for edge cases of the grid. The great thing is that we can populate that set with less array accesses. This is still pretty hackish:

def largest_connected_component(nrows, ncols, grid):
    """Find largest connected component of 1s on a grid."""

    def traverse_component(pos):
        """Returns no. of unseen elements connected to (i,j)."""
        elements.remove(pos)
        i, j = pos
        result = 1

        # Check all four neighbours
        for new_pos in [(i-1, j), (i+1, j), (i, j-1), (i, j+1)]:
            if new_pos in elements:
                result += traverse_component(new_pos)
        return result

    # Tracks size of largest connected component found
    elements = set()

    for i, line in enumerate(grid):
        for j, cell in enumerate(line):
            if cell:
                elements.add((i, j))

    return max(traverse_component(pos) for pos in set(elements) if pos in elements)

Edit: rewriting the solution to avoid the copy of elements, we have a slightly faster solution:

def largest_connected_component(nrows, ncols, grid):
    """Find largest connected component of 1s on a grid."""

    def traverse_component(pos):
        """Returns no. of unseen elements connected to (i,j)."""
        i, j = pos
        result = 1

        # Check all four neighbours
        for new_pos in [(i-1, j), (i+1, j), (i, j-1), (i, j+1)]:
            if new_pos in elements:
                elements.remove(new_pos)
                result += traverse_component(new_pos)
        return result

    # Tracks size of largest connected component found
    elements = set((i, j) for i, line in enumerate(grid) for j, cell in enumerate(line) if cell)
    largest = 0
    while elements:
        pos = elements.pop()
        largest = max(largest, traverse_component(pos))
    return largest


Simple pool of threads which calculate the sum of a given number from the main process I have to make a program which creates a number of threads that are waiting at the beginning, then the main process creates a doubly linked list where random numbers are put and then a signal tells the threads that a number has been put in the list, then the threads should "process" these numbers (is simply calculate the sum from 0 to the given number), graphically it looks like this:

And here is the code, I wonder if I'm doing what I'm doing fine...

Note: I'm working with 4 threads and the sleep(1) was to see the output...

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <signal.h>
#include <time.h>
#include <semaphore.h>

#include "double_linked_list.h"

void *sum(void *ptr);

sem_t semaphore;
pthread_mutex_t mutex;

int main() {
    sem_init(&semaphore, 0, 0);

    int threads = 4;
    int number, i;

    pthread_t *threads_tab;
    int *thread_args;

    threads_tab = malloc(threads * sizeof(pthread_t));
    thread_args = malloc(threads * sizeof(int));

    srand(time(NULL));

    head = NULL;
    tail = NULL;

    for (i = 0; i < threads; i++) {
        thread_args[i] = i;
        pthread_create(&threads_tab[i], NULL, sum, (void*) &thread_args[i]);
    }

    while (1) {
        number = rand() % 100;

        printf("Insertando: %d\n", number);
        InsertAtHead(number);

        sem_post(&semaphore);

        sleep(1);
    }

    return 0;
}

void *sum(void *ptr) {
    int i, acumulator, number;

    while (1) {
        sem_wait(&semaphore);

        acumulator = 0;

        pthread_mutex_lock(&mutex);
        number = tail->data;
        PopTail();
        pthread_mutex_unlock(&mutex);

        for (i = 0; i < number + 1; i++)
            acumulator += i;

        printf("The number is: %d and the result is: %d from thread: %d\n", number, acumulator, *(int *)ptr);
    }
}

Apparently it's working well, it doesn't generate any errors and the outputs are the expected ones, but I would like to hear more opinions, because I am new in this, perhaps I'm making some mistakes without realizing...

It's hard to tell without seeing the code for InsertHead and PopTail, but I would not be surprised if you've got a race condition between the main thread's InsertHead and the worker thread's PopTail. The workers take mutex in order to prevent data races among themselves; but the main thread blithely pushes new data onto the list without taking mutex, which seems like it would cause a race in the case that the list had only one element.

Stylistically, I'd replace

int threads = 4;
int number, i;

pthread_t *threads_tab;
int *thread_args;

threads_tab = malloc(threads * sizeof(pthread_t));
thread_args = malloc(threads * sizeof(int));

with

pthread_t threads_tab[4];
int thread_args[4];

to save those heap allocations (which, by the way, you never free).

The variable i should be declared within the for loop:

for (int i = 0; i < threads; i++) {

and the variable number should be declared where it's used:

int number = rand() % 100;

thread_args[i] = i;
pthread_create(&threads_tab[i], NULL, sum, (void*) &thread_args[i]);
[...]
printf(..., *(int *)ptr);

This is fine (and 100% correct, because it avoids implementation-defined casts!), but you should know that in practice most codebases will do simply

pthread_create(&threads_tab[i], NULL, sum, (void*) i);
[...]
printf(..., (int)ptr);

The latter is technically implementation-defined, but on common platforms it works, and it works without any memory loads or stores, which is nice, so that's why people end up doing it.

You fail to check the return value of pthread_create, which is a pretty big deal in real code; it's easy for a process to run out of threads. See man pthread_create.

In English, accumulator has two cs.


Print last Sunday of each month in a given year I'm an experienced programmer trying to learn OCaml. I figured a good way to do that would be to pick a simple task on Rosetta code that didn't yet have an OCaml example and do it myself. With that in mind, I wrote this program to find the last sunday of each month in a given year. It works by using Gauss's algorithm to determine what day of the week the last day in each month of the year is and if it's not a Sunday it moves backwards until it is. I run it with "./ocaml sundays.ml N" where N is the year in question. Are there any spots where my code is written in a way unusual for OCaml developers or where there is a more OCamlish way of doing things?

let is_leap_year y =
  (* See OCaml solution on Rosetta Code for
     determing if it's a leap year *)
   if (y mod 100) = 0
     then (y mod 400) = 0
     else (y mod 4) = 0;;

let get_days aYear =
  if is_leap_year aYear
    then
      [31;29;31;30;31;30;31;31;30;31;30;31]
    else
      [31;28;31;30;31;30;31;31;30;31;30;31];;

let print_date y m d =
  print_int y;
  print_char '/';
  print_int m;
  print_char '/';
  print_int d;
  print_endline "";;

let get_day_of_week y m d =
  let y = if m > 2 then y else y - 1 in
  let c = y / 100 in
  let y = y mod 100 in
  let m_shifted = float_of_int ( ((m + 9) mod 12) + 1) in
  let m_factor = int_of_float (2.6 *. m_shifted -. 0.2) in
  let leap_factor = 5 * (y mod 4) + 3 * (y mod 7) + 5 * (c mod 4) in
  (d + m_factor + leap_factor) mod 7;;

let get_shift y m last_day =
  get_day_of_week y m last_day;;

let print_last_sunday y m =
  let days = get_days y in
  let last_day = List.nth days (m - 1) in
  let last_sunday = last_day - (get_shift  y m last_day) in
  print_date y m last_sunday;;

let print_last_sundays y =
  let months = [1;2;3;4;5;6;7;8;9;10;11;12] in
  List.iter (print_last_sunday y) months;;

match (Array.length Sys.argv ) with
  2 -> print_last_sundays( int_of_string (Sys.argv.(1)));
 |_ -> invalid_arg "Please enter a year";

You do not need to put ;; everywhere. They're artifact of the interpreter.
print_date can be rewritten using printf:

let print_date = Printf.printf "%d/%d/%d"

It is better to store days in an array. List is not a good container for random access. Also, syntactically it would be easier to read: days.(2) is better than List.nth 2. Also, just FYI, you can have arrays in OCaml indexed from one, e.g., let days = Bigarray.(Array1.create int fortran_layout 12). Although, using them here would be overkill, I think.
You do not need parens here:

let last_sunday = last_day - (get_shift  y m last_day) in


Bootstrap responsive layout I'm creating a responsive page with bootstrap. I tested my code with my browser (Firefox and Chrome) and it works. Is the code clean or is there a better way to code it?

<%= name %>

<%= adresse %>

        <button class="btn btn-default btn-block" type="submit">Choose Address</button>

            <button class="btn-xs btn-default btn-block" id="editBtn" type="submit">Edit</button>

            <button class="btn-xs btn-warning btn-block" id="delBtn" type="submit">Delete</button>

This is a small piece of code but few things I'm going to highlight here

When using the col class, it's best practice to have this class under a row class. This creates a grid structure e.g

.col-sm-4

.col-sm-4

.col-sm-4

One thing to note here is that in bootstrap grid structure the total number of columns you can have is 12 . You can read more on that Bootstrap Grids . So these lines defines more than 12 columns and it's abusing bootstrap usage

I can see some Inline css declarations floating about, there isn't anything bad with that but it becomes cumbersome when you have many div's in your page. I suggest defining your css in an external stylesheet

I hope this helps


Displaying sorted results of a web crawl The issue I have with this class is that most of the methods are almost the same. I would like for this code to be more pythonic.

Note: I plan on replacing all the for loop constructs with list comprehensions.

class Sorter(BaseCrawler):

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

    def by_size(self, *options):
        '''
        Sort files by size
        '''
        file_list = []
        for fn in self.files(*options):
            file_list.append((fn, float(os.path.getsize(fn))))
        file_list.sort(key=lambda fn: fn[1])
        return file_list

    def by_mtime(self, *options):
        '''
        Sort files by modified time.
        '''
        file_list = []
        for fn in self.files(*options):
            file_list.append((fn, os.path.getmtime(fn)))
        file_list.sort(key=lambda fn: fn[1])
        return file_list

    def by_ctime(self, *options):
        '''
        Sort files by creation time.
        '''
        file_list = []
        for fn in self.files(*options):
            file_list.append((fn, os.path.getctime(fn)))
        file_list.sort(key=lambda fn: fn[1])
        return file_list

    def by_atime(self, *options):
        '''
        Sort files by access time.
        '''
        file_list = []
        for fn in self.files(*options):
            file_list.append((fn, os.path.getatime(fn)))
        file_list.sort(key=lambda fn: fn[1])
        return file_list

    def last_modified(self):
        '''
        Return the last modified file.
        '''
        return self.by_mtime()[0]

    def last_accessed(self):
        '''
        Return the last accessed file.
        '''
        return self.by_atime()[0]

    def last_created(self):
        '''
        Return the last created file.
        '''
        return self.by_ctime()[0]

    def average_fsize(self):
        '''
        Return the average file size.
        '''
        sizes = []
        for name, size in c.by_size():
            sizes.append(size)
        return sum(sizes) / len(sizes)

    def above_avg_size(self, times):
        '''
        Yield files that have an above average file size.
        '''
        avg = self.average_fsize()
        for f, s in c.by_size():
            if s > avg * times:
                yield f, s

Removing duplication is a very worthy goal.  Just "factor out" the feature-extraction functions ("f-e-f"s):

import operator

class Sorter(BaseCrawler):

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

    def _sort(self, fef, *options):
        file_list = [(fn, fef(fn)) for fn in self.files(*options)]
        return sorted(file_list, key=operator.itemgetter(1))

    def by_size(self, *options):
        def fef(fn): return float(os.path.getsize(fn))
        return self._sort(fef, *options)

    def by_mtime(self, *options):
        return self._sort(os.path.getmtime, *options)

etc, etc.

Not sure why by_size needs that float rather than the natural int returned by os.path.getsize (I'd rather do the float cast, if needed, downstream), but I've kept that dubious choice to show how best to deal with a f-e-f that's not already a plain existing function (hint: not with a lambda:-).

Other improvements are feasible which don't deal with duplication but with efficiency.  For example, consider:

def last_modified(self):
    '''
    Return the last modified file.
    '''
    return self.by_mtime()[0]

this does a lot of work (list building and sorting) that it doesn't need to do (and actually does not meet its name and docstring: it returns the file modified earliest, because the sort in by_mtime is in ascending order).  How much simpler to do...:

def last_modified(self):
    '''
    Return the last modified file.
    '''
    return max(self.files(), key=os.path.getmtime)

(this one fixes the bug; to maintain the bug, use min in lieu of max).


Decoding and Encoding packet layers using decorator pattern I am trying to use decorator pattern to decode and encode packet layers in my application. I've supposed my packet has three layers: Header, Level2, Level3. So far I've come up with the following code. For the sake of simplicity, I've supposed that the data of each layer is the first 10 bytes of the input packet. I've also intentionally ignored memory leak problems.

#include <iostream>
#include "Decoder.h"

int main()
{
    Header* header = new Header("HeaderData");
    Level2* l2 = new Level2(header, "Level2Data");
    Level3* l3 = new Level3(l2, "Level3Data");

    std::cout << header->Encode() << std::endl;
    std::cout << l2->Encode() << std::endl;
    std::cout << l3->Encode() << std::endl;

    std::string packet = l3->Encode();

    header->Decode(packet);
    l2->Decode(header->mNextData);
    l3->Decode(l2->mNextData);

    std::cout << ((Header*)header)->mData << std::endl;
    std::cout << ((Level2*)l2)->mData << std::endl;
    std::cout << ((Level3*)l3)->mData << std::endl;
}

Decoder.h file:

#ifndef _DECODER_H_
#define _DECODER_H_

#include <string>

class DecoderBase
{
public:
    virtual void Decode(std::string &packet) = 0;
    virtual std::string Encode() = 0;
protected:
    DecoderBase* mInnerLayer;
};

class Header : public DecoderBase
{
public:
    Header(const std::string &data);
    virtual ~Header();
    virtual void Decode(std::string &packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

class Level2 : public DecoderBase
{
public:
    Level2(DecoderBase* innerLayer, const std::string &data);
    ~Level2();
    virtual void Decode(std::string &packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

class Level3 : public DecoderBase
{
public:
    Level3(DecoderBase* innerLayer, const std::string &data);
    ~Level3();
    virtual void Decode(std::string &packet);
    virtual std::string Encode();
    std::string mData;
    std::string mNextData;
};

#endif

Decoder.cpp file:

#include "Decoder.h"

Header::Header(const std::string &data)
{
    mData = data;
}

Header::~Header(){}

void Header::Decode(std::string &packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Header::Encode()
{
    return mData;
}

Level2::Level2(DecoderBase *innerLayer, const std::string &data)
{
    mInnerLayer = innerLayer;
    mData = data;
}

void Level2::Decode(std::string &packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Level2::Encode()
{
    return mInnerLayer->Encode() + mData;
}

Level3::Level3(DecoderBase *innerLayer, const std::string &data)
{
    mInnerLayer = innerLayer;
    mData = data;
}

void Level3::Decode(std::string &packet)
{
    mData = packet.substr(0, 10);
    mNextData = packet.substr(10);
}

std::string Level3::Encode()
{
    return mInnerLayer->Encode() + mData;
}

Output:

  HeaderData
  HeaderDataLevel2Data
  HeaderDataLevel2DataLevel3Data
  HeaderData
  Level2Data
  Level3Data

Am I using the decorator pattern correctly to decode and encode packets?
Who is responsible for calling the Decode method of the next layer? Am I calling the Decode method of each layer correctly? I thought maybe l2->Decode() should be called from within header->Decode().

Any advice on improving the above code would be appreciated!

I don't know about the decorator pattern, but there are things you can improve anyway:

Identifiers that begin with an underscore followed by a capital letter are reserved identifiers in C++. Therefore, _DECODER_H_ is not a valid identifier; that name is reserved to the implementation. You should change it to DECODER_H_.
You never modify the string that you pass to Decode. Therefore, you should pass it a const reference instead of a simple non-const one:

virtual void Decode(const std::string &packet) = 0;

All of your class members are public. Therefore, you could simply change them to structs instead and remove all the public qualifications. The only difference between class and struct is that all is public by default (even inheritance) while all is private by default within a class.
You don't seem to need pointers in main, regular automatic variables would be enough. Actually, try to use pointers only when you have to in C++, only where it makes sense. Otherwise, try to always use value and/or reference semantics. That way, you may avoid many problems.
DecoderBase is probably meant to be a base class to be used for some polymorphic behaviour. Give it a virtual destructor. Otherwise, deleting a derived instance through a base pointer will be undefined behaviour.
Try to use constructor initialization lists whenever possible:

Level2::Level2(DecoderBase *innerLayer, const std::string &data):
    mInnerLayer(InnerLayer),
    mData(data)
{}

There are several functions that could be const-qualified (try to do so whenever a function does not modify the members of the class). The Encode methods look like they should be const-qualified everywhere:

std::string Header::Encode() const
{
    return mData;
}


Generic Fenwick Tree A Fenwick Tree, or a Binary Indexed tree, is an interesting data structure that can efficiently update its elements. You can read more about it in this paper.

Here is a generic implementation of a Fenwick Tree, which could take any two reversible operations:

class FenwickTree<T> {

    private let count   : Int
    private let neutral : T
    private let forward : (T,T) -> T
    private let reverse : (T,T) -> T

    private var data    : [T]
    private var tree    : [T]

    init(
        count: Int,
        neutralElement: T,
        forward: @escaping (T,T) -> T,
        reverse: @escaping (T,T) -> T
        ) {
        self.count = count
        self.neutral = neutralElement
        self.forward = forward
        self.reverse = reverse
        self.data = Array(repeating: neutralElement, count: count)
        self.tree = Array(repeating: neutralElement, count: count + 1)
    }

    func update(index: Int, with newValue: T) {
        let oldValue = data[index];
        let delta = reverse(newValue, oldValue)
        data[index] = newValue
        var treeIndex = index + 1
        while treeIndex <= count {
            tree[treeIndex] = forward(tree[treeIndex], delta)
            treeIndex += treeIndex & -treeIndex
        }
    }

    func accumulated(at index: Int) -> T {
        var sum = neutral
        var treeIndex = index + 1
        while 0 < treeIndex {
            sum = forward(tree[treeIndex], sum)
            treeIndex -= treeIndex & -treeIndex
        }
        return sum
    }

    func accumulated(in range: Range<Int>) -> T {
        let low = range.lowerBound, high = range.upperBound - 1
        let cumulatedLow = low == 0 ? neutral : accumulated(at: low - 1)
        let cumulatedHigh = accumulated(at: high)
        return low == high ? data[low] : reverse(cumulatedHigh,cumulatedLow)
    }
}

Here are some example usecases:

let n = 10

let additiveTree = FenwickTree(count: n, neutralElement: 0, forward: +, reverse: -)

for i in 0..<n { additiveTree.update(index: i, with: i + 1) }

additiveTree.accumulated(at: 5)
additiveTree.accumulated(at: 9)
additiveTree.accumulated(in: 8..<10)

let multiplicativeTree = FenwickTree(count: n, neutralElement: 1, forward: *, reverse: /)

for i in 0..<n { multiplicativeTree.update(index: i, with: i + 1) }

multiplicativeTree.accumulated(at: 5)
multiplicativeTree.accumulated(at: 9)
multiplicativeTree.accumulated(in: 8..<10)

I am mainly concerned about the naming of functions and variables:

neutral: This is the neutral element for forward and reverse
forward and reverse should be transitive verbs since they have side effects. Preferably, the names should have the same number of letters.
accumulated(at:) is an unusual name, more familiar would be foldLeft from Scala, or foldl from Haskell.
accumulated(at:) and accumulated(in:) have too similar names.

I can see some possible improvements:

Performance: accumulated(in:) could have better time complexity at the expense of some auxiliary space;
Enforcing the reversibility of forward and reverse in code;

Other areas of improvement are welcome.

Choosing the generic type (and parameters)

The Fenwick tree stores cumulative values of its elements, and is based on the ability to compute the sum and difference of those elements. You made a very general implementation, where “sum” and “difference” are provided as closure parameters. But it cannot be ensured that these are proper inverse operations of each other, and a slight variation of your multiplicativeTree example demonstrates the problem:

let multiplicativeTree = FenwickTree(count: 4, neutralElement: 1,
                                     forward: *, reverse: /)

multiplicativeTree.update(index: 0, with: 5)
print(multiplicativeTree.accumulated(at: 0))    // 5   OK

multiplicativeTree.update(index: 0, with: 10)
print(multiplicativeTree.accumulated(at: 0))    // 10  still OK

multiplicativeTree.update(index: 0, with: 13)
print(multiplicativeTree.accumulated(at: 0))    // 10  what?

multiplicativeTree.update(index: 0, with: 11)
print(multiplicativeTree.accumulated(at: 0))    // 0   what???

I would require the elements to conform to the AdditiveArithmetic protocol instead. That protocol

requires addition and subtraction, making the forward and reverse parameter obsolete,
requires a .zero element, making the neutralElement parameter obsolete, and
provides semantics: according to SE-0223, it “roughly corresponds to the mathematic notion of an additive group.”

AdditiveArithmetic is adopted by all binary integer and floating point types, that probably covers most use-cases. If you really need a data structure where updating a value is done via multiplication then you can define a custom type which implements AdditiveArithmetic accordingly.

The declaration and initialization then simplifies to

class FenwickTree<T: AdditiveArithmetic>  {

    private let count   : Int

    private var data    : [T]
    private var tree    : [T]

    init(count: Int) {
        self.count = count
        self.data = Array(repeating: .zero, count: count)
        self.tree = Array(repeating: .zero, count: count + 1)
    }

    // ...
}

and sums/differences can just be computed with + and -, making the code even better readable.

Accessing the elements

There is a

func update(index: Int, with newValue: T) {

to set new value at an index, but no method to get a single value. I would suggest to implement the subscript method instead, with both a getter and a setter:

subscript(index: Int) -> T {
    get {
        return data[index]
    }
    set {
        // ... update `data` and `tree`
    }
}

And not much more work is needed to make FenwickTree a (mutable, random access)  Collection.

A wider range of ranges ...

The accumulated(in:) method becomes much more flexible if it takes a RangeExpression parameter instead of a Range:

func accumulated<R>(in range: R) -> T
where R: RangeExpression, R.Bound == Int
{
    let realRange = range.relative(to: 0..<count)
    let cumulatedLow = accumulated(at: realRange.lowerBound - 1)
    let cumulatedHigh = accumulated(at: realRange.upperBound - 1)
    return cumulatedHigh - cumulatedLow
}

Now you can call it with half-open, closed, or partial ranges:

additiveTree.accumulated(in: 1..<2))
additiveTree.accumulated(in: 1...3))
additiveTree.accumulated(in: 2...))
additiveTree.accumulated(in: ..<4))
additiveTree.accumulated(in: ...3))

The last line is equivalent to your

additiveTree.accumulated(at: 3))

which means that accumulated(at:) is no longer needed as a public method (which solves the problem of the similar names of those methods.)

Further suggestions

The index operations

 treeIndex += treeIndex & -treeIndex
 treeIndex -= treeIndex & -treeIndex

look like magic to the first reader of your code. I would move that to helper methods (which the compiler inlines) and document it.

The tree[0] element is never accessed. With slight modifications of the index calculations you can save one element in that array, i.e. initialize it as

  self.tree = Array(repeating: .zero, count: count)

For example, the accumulation would be

func accumulated(at index: Int) -> T {
    guard index >= 0 else { return .zero }
    var sum = tree[0]
    var treeIndex = index
    while treeIndex > 0 {
        sum += tree[treeIndex]
        treeIndex -= treeIndex & -treeIndex
    }
    return sum
}

Finally, a single element can be retrieved from the tree array alone, by computing the difference of two tree nodes (this is also described in the Fenwick article). Therefore, at the cost of slightly more computation, you can get rid of the self.data array and save memory.


Function for shuffling cards Assignment from here

  Q6. Define a function shuffle that takes a list with an even number of elements (cards) and creates a new list that interleaves the elements of the first half with the elements of the second half:

def card(n):
    """Return the playing card type for a positive n <= 13."""
    assert type(n) == int and n > 0 and n <= 13, "Bad card n"
    specials = {1: 'A', 11: 'J', 12: 'Q', 13: 'K'}
    return specials.get(n, str(n))

def shuffle(cards):
    """Return a shuffled list that interleaves the two halves of cards.

    >>> suits = ['♡', '♢', '♤', '♧']
    >>> cards = [card(n) + suit for n in range(1,14) for suit in suits]
    >>> cards[:12]
    ['A♡', 'A♢', 'A♤', 'A♧', '2♡', '2♢', '2♤', '2♧', '3♡', '3♢', '3♤', '3♧']
    >>> cards[26:30]
    ['7♤', '7♧', '8♡', '8♢']
    >>> shuffle(cards)[:12]
    ['A♡', '7♤', 'A♢', '7♧', 'A♤', '8♡', 'A♧', '8♢', '2♡', '8♤', '2♢', '8♧']
    >>> shuffle(shuffle(cards))[:12]
    ['A♡', '4♢', '7♤', '10♧', 'A♢', '4♤', '7♧', 'J♡', 'A♤', '4♧', '8♡', 'J♢']
    >>> cards[:12]  # Should not be changed
    ['A♡', 'A♢', 'A♤', 'A♧', '2♡', '2♢', '2♤', '2♧', '3♡', '3♢', '3♤', '3♧']
    >>> repeated(shuffle, 8)(cards) == cards
    True
    """
    assert len(cards) % 2 == 0, 'len(cards) must be even'
    "*** YOUR CODE HERE ***"

Solution:

def card(n):
    """Return the playing card type for a positive n <= 13."""
    assert type(n) == int and n > 0 and n <= 13, "Bad card n"
    specials = {1: 'A', 11: 'J', 12: 'Q', 13: 'K'}
    return specials.get(n, str(n))

def shuffle(cards, index = 1, incr = 0):
    """Return a shuffled list that interleaves the two halves of cards.

    >>> suits = ['♡', '♢', '♤', '♧']
    >>> cards = [card(n) + suit for n in range(1,14) for suit in suits]
    >>> cards[:12]
    ['A♡', 'A♢', 'A♤', 'A♧', '2♡', '2♢', '2♤', '2♧', '3♡', '3♢', '3♤', '3♧']
    >>> cards[26:30]
    ['7♤', '7♧', '8♡', '8♢']
    >>> shuffle(cards)[:12]
    ['A♡', '7♤', 'A♢', '7♧', 'A♤', '8♡', 'A♧', '8♢', '2♡', '8♤', '2♢', '8♧']
    """
    assert len(cards) % 2 == 0, 'len(cards) must be even'
    if index > (len(cards) // 2):
        element = cards.pop((len(cards) // 2) + incr)
        cards.insert(index, element)
        return cards
    else:
        element = cards.pop((len(cards) // 2) + incr)
        cards.insert(index, element)
        return shuffle(cards, index + 2, incr + 1)

Can I improve this solution?

First of all, you are not supposed to modify the list in-place. Instead, return a shuffled copy of the original list. This is also checked by this doc test:

>>> cards[:12]  # Should not be changed
['A♡', 'A♢', 'A♤', 'A♧', '2♡', '2♢', '2♤', '2♧', '3♡', '3♢', '3♤', '3♧']

Furthermore, using recursion makes this overly complicated. After all, the solution to get the desired result is pretty simple: Cut the list in half and then take one item alternatively from each sublist. The first part is simple splicing, and ways to do the latter is covered in this question. So you could end up with this:

def shuffle (cards):
    # split the list in halves
    first, second = cards[:len(cards) // 2], cards[len(cards) // 2:]

    # combine them alternatively, e.g.
    return sum(map(list, zip(first, second)), [])

You can also do it without splitting the list by just iterating it alternatively from the beginning and from the middle:

def shuffle (cards):
    result = []
    half = len(cards) // 2
    for i in range(half):
        result.append(cards[i])
        result.append(cards[i + half])
    return result


Finding string roots This is the following Python code that I have coded for an algorithmic problem online.  For each line of input, the goal is to find the greatest number of repeating blocks into which the string can be split.  For example, if the input is abcabcabcabc, then the output should be 4, because the string is equal to 4 repetitions of abc.

The online compiler says "Time Limit Exceeded". Where in the following code can I optimize to make it much more efficient in terms of time?

inputString = raw_input()
while (inputString !='*'):
   scanned_string = ""
   matched_characters = ""
   frequency = 0
   if (len(inputString) == 1):
     print frequency
   for current_symbol in inputString:
     if not scanned_string:
        scanned_string = scanned_string + current_symbol
     if scanned_string:
        matched_characters = matched_characters + current_symbol
        if (scanned_string.startswith(matched_characters)):
            if (len(scanned_string) == len(matched_characters)):
                frequency = frequency + 1
                matched_characters = ""
        else:
            scanned_string = scanned_string+matched_characters
            matched_characters = ""
            frequency = 1
   print frequency
   inputString = raw_input()

Notice that:

The length of a root must be a divisor of the length of the input string: you could skip other lengths. For example for the input string "abcabcabc" (length = 9), "ab" (length = 2) or "abcd" (length = 4) cannot be a root, so no need to check those
The length of a root must be less than equal to half the length of the input string, otherwise it's the input string itself (frequency = 1)

Taking the above into consideration, this implementation should be faster:

def sroot(text):
    text_len = len(text)
    for segment_len in range(1, text_len // 2 + 1):
        if text_len % segment_len != 0:
            continue
        frequency = text_len // segment_len
        if text[:segment_len] * frequency == text:
            return frequency
    return 1

This can be further improved too.
For example, instead of iterating until text_len // 2,
it would be enough to iterate until math.sqrt(text_len),
and when finding a divisor, also try its factor pair as a possible segment.
For example for text_len = 100,
when finding segment_len = 5,
also try segment_len = 100 / 5 = 20.
Iterating until 10 will be a lot faster than iterating until 50 as in the example implementation above.

There are several coding style issues in your code:

It's good to put the main logic inside a function. It makes it easier to test. For example you can add assertions like this to test your solution:

assert sroot('abcabcabcabc') == 4
assert sroot('abcdefgh012') == 1
assert sroot('aaaaaaaaaa') == 10
assert sroot('abcdefgh012abcdefgh012') == 2
assert sroot('abcdefgh012abcdefgh012x') == 1

Lots of unnecessary parentheses, for example in:

while (inputString !='*'):
if (len(inputString) == 1):

This can be simplified:

 if not scanned_string:
    scanned_string = scanned_string + current_symbol

to this:

 if not scanned_string:
    scanned_string = current_symbol

This can be simplified:

matched_characters = matched_characters + current_symbol

to this:

matched_characters += current_symbol


Project Euler problem 1 in Python - Multiples of 3 and 5 I'd like suggestions for optimizing this brute force solution to problem 1.  The algorithm currently checks every integer between 3 and 1000.  I'd like to cut as many unnecessary calls to isMultiple as possible:

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

Find the sum of all the multiples of 3 or 5 below 1000.
'''

end = 1000

def Solution01():
    '''
        Solved by brute force
        #OPTIMIZE
    '''
    sum = 0
    for i in range(3, end):
        if isMultiple(i):
            sum += i
    print(sum)

def isMultiple(i):
    return (i % 3 == 0) or (i % 5 == 0)

The sum 3+6+9+12+...+999 = 3(1+2+3+...+333) = 3 (n(n+1))/2 for n = 333. And 333 = 1000/3, where "/" is integral arithmetic.

Also, note that multiples of 15 are counted twice.

So

def sum_factors_of_n_below_k(k, n):
    m = (k-1) // n
    return n * m * (m+1) // 2

def solution_01():
    return (sum_factors_of_n_below_k(1000, 3) +
            sum_factors_of_n_below_k(1000, 5) -
            sum_factors_of_n_below_k(1000, 15))


Execute .NET Core 3.1 background worker tasks simultaneously I have following code:
public class Worker : BackgroundService
{

    public Worker()
    {
    }

    protected async override Task ExecuteAsync(CancellationToken stoppingToken)
    {
        while (!stoppingToken.IsCancellationRequested)
        {
            var task1 = Task.Run(() =>
                MethodOneAsync(stoppingToken)
            ).ContinueWith(_ => Debug.WriteLine($"MethodOneAsync finished @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}"));
            var task2 = Task.Run(() =>
                MethodTwoAsync(stoppingToken)
            ).ContinueWith(_ => Debug.WriteLine($"MethodTwoAsync finished @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}"));

            await Task.WhenAll(task1, task2);

            await Task.Delay(10000);
        }
    }

    private async Task<bool> MethodOneAsync(CancellationToken stoppingToken)
    {
        Debug.WriteLine($"MethodOneAsync started @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
        await Task.Delay(1000); // emulating some long work

        return true;
    }

    private async Task<bool> MethodTwoAsync(CancellationToken stoppingToken)
    {
        Debug.WriteLine($"MethodTwoAsync started @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
        await Task.Delay(1000); // emulating some long work

        return true;
    }
}

What I want to achieve is:

to create background worker that will call MethodOneAsync and MethodTwoAsync on separate threads (start both tasks simultaneously)
log time each task started and finished
wait for them both to finish
then wait another 10 seconds
repeat all over again

The code I wrote does its job as expected.
Since I am still trying to get a grasp of .NET Core tasks, I would like to know is this the right way to do it?

Async method out-of-the-box is using multiple threads for continuations. (Everything after await is continuation). Exception of this rule if existing SynchronizationContext. Don't Task.Run asynchronous I/O-bound operation, do it only for synchronous CPU-bound one. Asynchronous programming, SynchrinizationContext & TaskScheduler.
Mixing .ContinueWith and await is not recommended as it doing the same but using different coding pattern. Mixing patterns with no reason makes the code more complicated.

If I were you, the code could look like this:
public class Worker : BackgroundService
{
    protected async override Task ExecuteAsync(CancellationToken stoppingToken)
    {
        // all methods will throw OperationCanceledException if cancellation was happened,
        // then checking if it was canceled is a kind of redundancy here.
        while (true)
        {
            var task1 = MethodOneAsync(stoppingToken);
            var task2 = MethodTwoAsync(stoppingToken);
            await Task.WhenAll(task1, task2, Task.Delay(10000, stoppingToken)); // inlining Task.Delay here changes the logic: 10 seconds counted between starts of two iterations not between finish of previous and start of next.
        }
    }

    private async Task MethodOneAsync(CancellationToken stoppingToken)
    {
        Debug.WriteLine($"MethodOneAsync started @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
        await Task.Delay(1000, stoppingToken);
        Debug.WriteLine($"MethodOneAsync finished @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
    }

    private async Task MethodTwoAsync(CancellationToken stoppingToken)
    {
        Debug.WriteLine($"MethodTwoAsync started @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
        await Task.Delay(1000, stoppingToken);
        Debug.WriteLine($"MethodTwoAsync finished @ {DateTime.Now:dd.MM.yyyy HH:mm:ss.ffff}");
    }
}

About Worker class purpose. It looks like Scheduler not Worker because it repeats some work continuously. Consider to change its name.


Simple Calculator with switch statement I just start learning to code in C#.
Is there any other short way to make calculator like this

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Calculator
{
   class Program
{

    static void Main(string[] args)
    {
        float result = 0;
        //Getting Input of First number
        Console.WriteLine("Enter first number");
        float num1 = float.Parse(Console.ReadLine());

        //Getting Input of second number
        Console.WriteLine("Enter second number");
        float num2 = float.Parse(Console.ReadLine());

        //Getting the math operator
        Console.WriteLine("Enter operator");
        string op = Console.ReadLine();

        switch (op) {

            case "+" : result = num1 + num2;
                break;
            case "-":
                result = num1 - num2;
                break;
            case "*":
                result = num1 * num2;
                break;
            case "/":
                result = num1 / num2;
                break;
            case "%":
                result = num1 % num2;
                break;

        }
        Console.WriteLine("Result = " + result);

    }
  }
}

Is there any other short way to make calculator

Without trying to be facetious, of course there are many different ways to try and do the same thing.

I assume this is a beginner exercise; so I'll review it with that in mind, giving you some tips on what to tackle next as well.

The coding style

The variables are acceptably named, but I'd refrain from using shorthand such as op, instead favoring operation. This makes no technical difference but it enhances readability. The current program is simple enough that it's not an issue, but more complex applications will suffer more from these unclear abbreviations.
Well structured paragraphs (although I personally would have put the last line on a separate paragraph, but I'm nitpicking).
You should really separate some of this logic into separate methods(/classes), but I do concede that the current code is short enough that's it's not a big issue right now.

The code itself

On a technical level, everything is as it should be.
You have not accounted for nonsensical input. Ask yourself what happens if the user enters "batman" as a number value.
Similarly, you have not accounted for non-existing operators. If I enter "batman" as the operator; your application will tell me that the result is 0.

As a user, I'm unable to differentiate whether this is correct (e.g. if I entered 5 - 5) or incorrect (if I use an unknown operator). That's not good. Suppose I get 0. That would make me paranoid and require me to backtrack what I gave as input to confirm whether this is a real answer or not.
Instead, your application should clearly tell the user that they used an unknown operator, e.g. Error - "batman" is not a known mathematical operation!

You've avoided the int division problem.
You've abstracted your result into a result variable instead of creating a different Console.WriteLine() for every operation. That is a good application of reusability.
You have not avoided division by zero. This is a fairly important fringe case to account for in a calculator application.

Improvements and future challenges

Implement a failsafe to prevent division by zero.
Implement clear user feedback in case the user enters nonsensical data.
The current application performs a single calculation and then exits. What if the user wants to calculate a few different things? A better approach would be to loop the application code until the user enters "exit", at which point the application closes.
Although the current code is simple enough that it's not necessary yet, I would suggest you abstract the number entry into a method, so your code looks like this: float num1 = AskForNumber(); float num2 = AskForNumber(); (feel free to use a different method name).
String comparison is not as type safe as enum comparison. It would be a good exercise to create an enum Operation { Add, Subtract, Multiply, Divide } and then use that in your switch case instead of directly checking string values. Note: The current code is simple enough that the enum adds more code than it's currently worth, but it is a good exercise and preparation for the future.
Since you're learning C#, I would suggest you try an OOP approach, e.g. a Calculator class which implements the calculations, so that your Program class doesn't contain any of the math logic. Again, the current example is simple enough that it's not quite necessary, but it's a good exercise. If you only start using OOP when the code is already complicated; that's not going to help you understand the core principles.
Currently, the user is asked for a number, another number, and then an operation. This is not intuitive from the perspective of a user. Notice how every real world calculator asks for the operation before it asks for the second number.

It would also be nice if the user could enter the entire calculation in a single go (1 + 2), and your code extract the needed num1, num2 and operation values.
Try allowing for multiple operations in a single statement (1 + 2 - 3).
Don't forget to apply the correct order of operations! (e.g. 1 + 2 * 3 is equal to 7, not 9).
Add functionality for () parentheses to influence the order of operations.
More operators: ^ for powers (e.g. 2 ^ 3 = 2³ = 8)

Small remark

  Is there any other short way

Shortness is not a measure of good code. If anything, excessively short code is often bad due to inlining several operations and generally lowering code readability.

I know that you meant "simple", i.e. you didn't want a counterexample that is massively more complicated than your exercise.

But I just want to call your attention to the distinction between short code and simple code. These are not freely interchangeable; and more often end up being opposites of each other.


Newton's Method Polynomial solver in Ruby I am learning Ruby programming from "Learn Ruby the Hard way" and I am doing the "Ruby koans".

I have heard a little bit about "Idiomatic" Ruby but I don't know much about it. How can I make it more idiomatic?

# Program to solve polynomials

def solve(coeff)
    coefficients = []
    coeff.each{ |a|
        coefficients.push a.to_f
    }
    diffrentiate = []
    n = 0
    coefficients.reverse_each { |a|
        diffrentiate.unshift n*a
        n += 1
    }
    diffrentiate.pop
    return newton(coefficients,diffrentiate,-1)
end

def newton(coefficients,diffrentiate,initialvalue)
    x = initialvalue
    10.times { |n|
        x -= evaluate_polynomial(coefficients,x)/evaluate_polynomial(diffrentiate,x)
    }
    return x
end

def evaluate_polynomial(coefficients,x)
    n = 0
    value = 0
    coefficients.reverse_each { |a|
        value += a*x**n
        n+=1
    }
    return value
end

puts solve ([1,0,0,0,-1,1])

Use do...end instead of {...} for multi-line blocks.
return is implicit at the end of a method.
differentiate is a verb. derivatives is the plural noun you're looking for when naming the array.
Use Ruby's array methods (map, reduce, and friends) to your advantage.
Don't modify external variables from inside a block if you can avoid it. I.e. don't do things like n += 1, or x -= .... A combination of with_index and reduce should do the same trick more neatly.
There's some repetition in your handling of the order of coefficients (i.e. the n += 1 in the blocks). It'd be nice to get rid of that.
It'd be nice to specify the number of iterations the approximation should use.

Here's a pretty direct translation of your solution:

# Add a method to Array which zips the array with the index
# of each element in reverse order
class Array
  def zip_reverse_index
    zip((0...count).to_a.reverse)
  end
end

# Use a *-splat to make the method variadic
def solve(*coefficients)
  coefficients.map!(&:to_f)
  derivatives = coefficients.zip_reverse_index.map { |n, i| n * i }
  newton(coefficients, derivatives[0...-1])
end

# Added some (optional) arguments
def newton(coefficients, derivatives, iterations = 10, initial = -1)
  iterations.times.reduce(initial) do |x, i|
    x -= evaluate_polynomial(coefficients, x) / evaluate_polynomial(derivatives, x)
  end
end

# A simpler implementation using reduce
def evaluate_polynomial(coefficients, x)
  coefficients.zip_reverse_index.reduce(0) do |sum, (c, n)|
    sum += c * x ** n
  end
end

Here's a different version that keeps everything in 1 method:

def newton_raphson(coefficients, intial = -1, iterations = 10)
  evaluate = -> (array, x) do
    array.reduce(0) { |sum, (c, ord)| sum += c * x ** ord }
  end

  coefficients = coefficients
    .map(&:to_f)
    .zip((0...coefficients.count).to_a.reverse)

  derivatives = coefficients.map { |c, ord| [c * ord, ord - 1] }[0...-1]

  iterations.times.reduce(intial) do |approx, _|
    approx -= evaluate[coefficients, approx] / evaluate[derivatives, approx]
  end
end

It's pretty long for a Ruby method, but some of the methods in the other implementation seemed too specific to keep around in the global scope (and the Array mixin also seemed a little heavy-handed).

Edit: as @jQweirdy mentioned in the comments, this could also be solved with a class, but I was hesitant to add a class just for one method. But there are a few things here, that might be generically useful for Polynomial class. So here's an example:

class Polynomial
  attr_reader :coefficients

  def initialize(*coefficients)
    @coefficients = coefficients
  end

  def evaluate(x)
    coefficients.zip(ordinals).reduce(0) do |sum, (n, i)|
      sum += n * x ** i
    end
  end

  def derivative
    @derivative ||= begin
      derivatives = coefficients.zip(ordinals)[0...-1].map { |n, i| n * i }
      self.class.new(*derivatives)
    end
  end

  def order
    coefficients.count - 1
  end

  def ordinals
    (0..order).to_a.reverse
  end

  def newton_raphson_approximation(iterations = 1, initial = -1)
    iterations.times.reduce(initial) do |x, _|
      x -= evaluate(x).fdiv(derivative.evaluate(x))
    end
  end
end


Speed up loop through JSON I have the following function that takes JSON and turns it into table rows:

function addDataFromJson(json) {
    var data = json.data;
    var rows = '';
    $('.uiGridContent tbody').html('');

    for (var i = 0; i < data.length; i++) // for each row
    {
        var columns = '';

        for (var b = 0; b < Object.keys(data[i]).length; b++) // for each column
        {
            var content = data[i][Object.keys(data[i])[b]];

            columns += '' + content + '	';
        }

        var row = columns;

        row = '
\
                    <input type="checkbox">	\
                    \

\
                            Actions\

\
                    	\
                    ' + columns + '\

';

        rows =+ row;

    }

    $(rows).appendTo('.uiGridContent tbody');
}

The JSON looks like this (for those curious):

"data": [{
        "id": 1,
        "name": "Sheffield University",
        "departments": ["Software", "Recruitment", "Consulting"],
        "locations": ["Sheffield", "Rotherham", "London", "New York"]
    }, {
        "id": 2,
        "name": "Sheffield College",
        "departments": "",
        "locations": ["Hillsborough", "City Centre", "Crystal Peaks"]
    }, {
        "id": 3,
        "name": "Sheffield High School",
        "departments": ["Medical", "Family", "Criminal"],
        "locations": ["Sheffield", "Rotherham"]
    }...

However if I pass back 1000+ records then the JavaScript struggles and it takes a few seconds to do it and causes the browser to lock up.

Any ideas on how to improve this code?

It seems to be the line where I append the rows to the page:

$(rows).appendTo('.uiGridContent tbody');

As if I comment that line out, then it's really fast...

Fun question,

there is really nothing much you can do about this, adding a thousand rows in an HTML table will take a few second and might lock your browser. Browser were not meant for that. Moore's law is on your side though, so at it'll be fine at some point. Consider implementing paging ( with say 100 items per page ), and you app will be much snappier.

Other than that;

rows =+ row; <- This drove me nuts, it is rows += row;, your code does not work
HTML in your JavaScript, say it aint so. You could use either true HTML 5 templates or write your own  as a template
You are retrieving Object.keys far too many times, I would suggest you get it ones for the first row and cache it
It is considered better practice to have all your var's in ` place, and chain them

This is my counter proposal:

var data = { "data": [{
        "id": 1,
        "name": "Sheffield University",
        "departments": ["Software", "Recruitment", "Consulting"],
        "locations": ["Sheffield", "Rotherham", "London", "New York"]
    }, {
        "id": 2,
        "name": "Sheffield College",
        "departments": "",
        "locations": ["Hillsborough", "City Centre", "Crystal Peaks"]
    }, {
        "id": 3,
        "name": "Sheffield High School",
        "departments": ["Medical", "Family", "Criminal"],
        "locations": ["Sheffield", "Rotherham"]
    }]};

addDataFromJson( data );

function addDataFromJson(json) {
    var template = $('#mytemplate').html(),
        data = json.data,
        keys = Object.keys( data[0] ),
        rows = '',
        cells;

    $('body').html('');

    for (i = 0; i < data.length; i++) // for each row
    {
        cells =  keys.map( function(key){ return data[i][key]; } ).join('	\n');
        rows += template.replace( 'DataFromJson' , cells );
    }

    $(rows.trim()).appendTo('body');
}

<template id="mytemplate" style="display:none">

  <input type="checkbox">

          Actions

  DataFromJson

</template>