HTTP download with dynamic reallocation and retries This code is for downloading a file via winhttp lib, It's invoked by StartDownload function which tries to download n times before returning false if maximum tries exceeded. I'm worried about the dynamic realloc part in Download function, is my approach here have any flaws, or is there any improvements that can be made?

#include "HttpDownload.h"
#include <Windows.h>
#include <Winhttp.h>
#pragma comment(lib, "winhttp.lib")

bool bResults = FALSE;
HINTERNET hSession = 0, hConnect = 0, hRequest = 0;
int RepeatedTimes = 0;

void HttpDownload::CloseHandles() {
    if (hRequest) WinHttpCloseHandle(hRequest);
    if (hConnect) WinHttpCloseHandle(hConnect);
    if (hSession) WinHttpCloseHandle(hSession);
}

bool HttpDownload::InitHttp() {
    if (!(hSession = WinHttpOpen(0, 0, 0, 0, 0))) return false;
    if (!(hConnect = WinHttpConnect(hSession, L"127.0.0.1", 80, 0))) return false;
    if (!(hRequest = WinHttpOpenRequest(hConnect, L"GET", L"/file.ext", 0, 0, 0, 0))) return false;
    if (!(bResults = WinHttpSendRequest(hRequest, 0, 0, 0, 0, 0, 0))) return false;
    if (!(bResults = WinHttpReceiveResponse(hRequest, 0))) return false;
    return true;
}

bool HttpDownload::Download() {
    if (!bResults) return false;
    unsigned long dwSize = 0;
    unsigned long dwDownloaded = 0;
    void *buf;
    bool failed = false;
    int curalloc = 8192;
    unsigned char *jar = (unsigned char*)malloc(curalloc);
    int totalDownloaded = 0;
    #define failbreak failed = true; break;
    while (1) {
        dwSize = 0;
        if (!WinHttpQueryDataAvailable(hRequest, &dwSize)) { failbreak }
        if (dwSize == 0) break;
        buf = VirtualAlloc(0, dwSize, MEM_COMMIT, PAGE_READWRITE);
        if (!buf) { failbreak }
        else {
            if (!WinHttpReadData(hRequest, buf, dwSize, &dwDownloaded)) { failbreak }
            else {
                realloc:
                if ((totalDownloaded + dwDownloaded) > curalloc) {
                    unsigned char *m = (unsigned char*)malloc(curalloc * 2);
                    curalloc = curalloc * 2;
                    memcpy(m, jar, totalDownloaded);
                    free(jar);
                    jar = m;
                    goto realloc;
                }
                memcpy((jar + totalDownloaded), buf, dwDownloaded);
                totalDownloaded += dwDownloaded;
            }
            VirtualFree(buf, 0, MEM_RELEASE);
        }
    } if (failed) return false;
    CloseHandles();
    FinalResult = (unsigned char*)malloc(totalDownloaded);
    memcpy(FinalResult, jar, totalDownloaded);
    free(jar);
    FinalDownloadedSize = totalDownloaded;
    return true;
}

bool HttpDownload::StartDownload() {
    CloseHandles();
    if (RepeatedTimes > 4) { return false; }
    RepeatedTimes++;
    bool r = InitHttp();
    if (!r) StartDownload();
    r = Download();
    if (!r) StartDownload();
    return true;
}

at first StartDownload use recursion, when it absolute not need here. enough simply loop like -

ULONG nTry = 4, dwError;
do {
    dwError = Download();
} while(dwError && --Try);

next - you not need recreate all handles - hSession, hConnect, hRequest on send request fail, but only hRequest. the hSession we usual must open only once. the hConnect - once per url. note that WinHttpConnect really not connect to server. it simply remember server url and port in internal structures. if WinHttpOpen or WinHttpConnect - no sense try call it again (this is offline calls)

so code for synchronous (which is bad) processing must look like:

void test()
{
    if (HINTERNET hSession = WinHttpOpen( 0,
        WINHTTP_ACCESS_TYPE_NO_PROXY,
        WINHTTP_NO_PROXY_NAME,
        WINHTTP_NO_PROXY_BYPASS, 0))
    {
        if (HINTERNET hConnect = WinHttpConnect(hSession, L"stackoverflow.com", INTERNET_DEFAULT_HTTPS_PORT, 0))
        {
            int nTry = 4;
            ULONG dwError;
            do
            {
                if (HINTERNET hRequest = WinHttpOpenRequest(hConnect, L"GET", NULL,
                    NULL, WINHTTP_NO_REFERER,
                    WINHTTP_DEFAULT_ACCEPT_TYPES,
                    WINHTTP_FLAG_REFRESH|WINHTTP_FLAG_SECURE))
                {
                    if (WinHttpSendRequest(hRequest,
                        WINHTTP_NO_ADDITIONAL_HEADERS,
                        0, WINHTTP_NO_REQUEST_DATA, 0,
                        0, 0) && WinHttpReceiveResponse(hRequest, 0))
                    {
                        dwError = Download(hRequest);
                    }
                    else
                    {
                        dwError = GetLastError();
                    }

                    WinHttpCloseHandle(hRequest);
                }
                else
                {
                    dwError = GetLastError();
                }

            } while (dwError && --nTry);

            WinHttpCloseHandle(hConnect);
        }

        WinHttpCloseHandle(hSession);
    }
}

now look for you Download implementation - it contains mistakes and amazing not effective.

VirtualAlloc(0, dwSize, MEM_COMMIT, PAGE_READWRITE);

you always, unconditionally allocate temporary memory buffer (buf), even if you main buffer jar containing enough free space. than reallocate jar if need and copy temporary buffer to it memcpy((jar + totalDownloaded), buf, dwDownloaded);

also note that you have potential memory leak here - WinHttpReadData fail - you just break loop, without free temporary buf.

really we not need allocate any temporary buffer. we need just read data to main buffer jar - WinHttpReadData(hRequest, (PBYTE)jar + totalDownloaded, dwSize, &dwSize). if jar not have enough free space - we need reallocate it before WinHttpReadData call.

now look how you reallocate jar - it's just inexplicable for me

realloc: ** goto realloc;

for what this loop ?!? why need curalloc = curalloc * 2; and multiple time allocate, copy, free data ?!?. when we can just set

curalloc = totalDownloaded + dwDownloaded;

more better of course allocate some more memory than totalDownloaded + dwDownloaded. so code for Download can look like:

ULONG Download(HINTERNET hRequest)
{
    ULONG curalloc = 0x10000, dwSize, totalDownloaded = 0, cbNeed;

    HANDLE hHeap = GetProcessHeap();

    ULONG dwError = NOERROR;

    if (PVOID jar = HeapAlloc(hHeap, 0, curalloc))
    {
        do
        {
            if (!WinHttpQueryDataAvailable(hRequest, &dwSize))
            {
                dwError = GetLastError();
                break;
            }

            cbNeed = totalDownloaded + dwSize;

            if (cbNeed > curalloc)
            {
                cbNeed = (cbNeed + 0xffff) & ~0xffff;// round to 64kb

                if (PVOID buf = HeapReAlloc(hHeap, 0, jar, cbNeed))
                {
                    curalloc = cbNeed, jar = buf;
                }
                else
                {
                    dwSize = ERROR_OUTOFMEMORY;
                    break;
                }
            }

            if (!WinHttpReadData(hRequest, (PBYTE)jar + totalDownloaded, dwSize, &dwSize))
            {
                dwError = GetLastError();
                break;
            }

            totalDownloaded += dwSize;

        } while (dwSize);

        if (!dwError)
        {
            DbgPrint("download %u Ok\n", totalDownloaded);
        }

        HeapFree(hHeap, 0, jar);
    }

    return GetLastError();
}

this is already ok, but not best. windows let reserve memory space. so good solution - at begin reserve large enough memory space (in 32bit we can reserve how minimum n*10Mb, in 64bit - gigabytes). and then allocate memory from this reserved range. with this we not need reallocate and copy data. class for this dynamic allocation:

class DynamicBuffer
{
    PBYTE _BaseAddress;
    SIZE_T _dwReserve, _dwSize, _dwCommit;

    static SIZE_T RoundSize(SIZE_T size)
    {
        static SIZE_T s_dwAllocationGranularity;

        if (!s_dwAllocationGranularity)
        {
            SYSTEM_INFO si;
            GetSystemInfo(&si);
            s_dwAllocationGranularity = si.dwAllocationGranularity - 1;
        }

        return (size + s_dwAllocationGranularity) & ~s_dwAllocationGranularity;
    }

public:

    DynamicBuffer()
    {
        _BaseAddress = 0, _dwReserve = 0, _dwSize = 0, _dwCommit = 0;
    }

    ~DynamicBuffer()
    {
        Reset();
    }

    ULONG Create(SIZE_T dwSize)
    {
        if (_BaseAddress = (PBYTE)VirtualAlloc(0, dwSize = RoundSize(dwSize), MEM_RESERVE, PAGE_READWRITE))
        {
            _dwReserve = dwSize;
            return NOERROR;
        }

        return GetLastError();
    }

    ULONG AllocBuffer(PVOID* ppv, SIZE_T cb)
    {
        if (_dwReserve - _dwSize < cb)
        {
            return ERROR_OUTOFMEMORY;
        }

        SIZE_T dwSize = _dwSize + cb;

        if (dwSize > _dwCommit)
        {
            SIZE_T dwCommit = RoundSize(dwSize);

            if (!VirtualAlloc(_BaseAddress + _dwCommit, dwCommit - _dwCommit, MEM_COMMIT, PAGE_READWRITE))
            {
                return GetLastError();
            }

            _dwCommit = dwCommit;
        }

        *ppv = _BaseAddress + _dwSize;

        return NOERROR;
    }

    void AddData(SIZE_T cb)
    {
        _dwSize += cb;

        if (_dwSize > _dwCommit)
        {
            __debugbreak();
        }
    }

    PVOID getData()
    {
        return _BaseAddress;
    }

    SIZE_T getDataSize()
    {
        return _dwSize;
    }

    SIZE_T getFreeSpace()
    {
        return _dwReserve - _dwSize;
    }

    void Reset()
    {
        if (_BaseAddress)
        {
            VirtualFree(_BaseAddress, 0, MEM_RELEASE);
            _BaseAddress = 0;
        }
        _dwReserve = 0, _dwSize = 0, _dwCommit = 0;
    }
};

with this we can rewrite Download :

ULONG Download(HINTERNET hRequest)
{
    DynamicBuffer buf;

    ULONG dwError = NOERROR;

    if (dwError = buf.Create(0x4000000)) // 64Mb reserve
    {
        return dwError;
    }

    ULONG dwSize;

    do
    {
        if (!WinHttpQueryDataAvailable(hRequest, &dwSize))
        {
            dwError = GetLastError();
            break;
        }

        PVOID pv;
        if (dwError = buf.AllocBuffer(&pv, dwSize))
        {
            break;
        }

        if (!WinHttpReadData(hRequest, pv, dwSize, &dwSize))
        {
            dwError = GetLastError();
            break;
        }

        buf.AddData(dwSize);

    } while (dwSize);

    if (!dwError)
    {
        DbgPrint("download %u Ok\n", buf.getDataSize());
    }

    return GetLastError();
}

but however always much better do asynchronous download. this can be done say in next way:

class __declspec(novtable) InternetHandle
{
    HINTERNET _hInternet;
    LONG _dwRef;

protected:

    virtual ~InternetHandle()
    {
        if (_hInternet)
        {
            WinHttpCloseHandle(_hInternet);
        }
        DbgPrint("%s<%p>\n", __FUNCTION__, this);
    }

    InternetHandle()
    {
        _dwRef = 1;
        _hInternet = 0;
        DbgPrint("%s<%p>\n", __FUNCTION__, this);
    }

public:

    void AddRef()
    {
        InterlockedIncrement(&_dwRef);
    }

    void Release()
    {
        if (!InterlockedDecrement(&_dwRef)) delete this;
    }

    HINTERNET get_handle() { return _hInternet; }

    HINTERNET set_handle(HINTERNET hInternet) {
        return InterlockedExchangePointer(&_hInternet, hInternet);
    }
};

class CSession : public InternetHandle
{
public:
    ULONG Open(LPCWSTR pszAgentW = 0)
    {
        if (HINTERNET hSession = WinHttpOpen( pszAgentW,
            WINHTTP_ACCESS_TYPE_NO_PROXY,
            WINHTTP_NO_PROXY_NAME,
            WINHTTP_NO_PROXY_BYPASS, WINHTTP_FLAG_ASYNC))
        {
            set_handle(hSession);

            return NOERROR;
        }

        return GetLastError();
    }
};

class CTarget : public InternetHandle
{
    CSession* _pSession;

    virtual ~CTarget()
    {
        _pSession->Release();
    }
public:

    ULONG Set(PCWSTR pswzServerName, INTERNET_PORT nServerPort)
    {
        if (HINTERNET hConnect = WinHttpConnect(_pSession->get_handle(), pswzServerName, nServerPort, 0))
        {
            set_handle(hConnect);
            return NOERROR;
        }

        return GetLastError();
    }

    CTarget(CSession* pSession) : _pSession(pSession)
    {
        pSession->AddRef();
    }
};

class DownloadCtx : public InternetHandle, public DynamicBuffer, public WINHTTP_ASYNC_RESULT
{
    CTarget* _pTarget;
    ULONG _nTryCount;
    ULONG _dwThreadId;

    virtual ~DownloadCtx()
    {
        DbgPrint("<%u,%u> %u\n", dwResult, dwError, getDataSize());

        _pTarget->Release();

        PostThreadMessage(_dwThreadId, WM_QUIT, dwResult, dwError);
    }

public:
    DownloadCtx(CTarget* pTarget, ULONG nTryCount) : _pTarget(pTarget), _nTryCount(nTryCount)
    {
        pTarget->AddRef();
        dwError = ERROR_IO_PENDING, dwResult = 0;
        _dwThreadId = GetCurrentThreadId();
    }

    ULONG SendRequest()
    {
        if (get_handle()) __debugbreak();

        if (!dwError)
        {
            return NOERROR;
        }

        if (!_nTryCount--)
        {
            return dwError;
        }

        dwError = ERROR_IO_PENDING, dwResult = 0;

        Reset();

        DbgPrint("=========== %x ============\n", _nTryCount);

        if (HINTERNET hRequest = WinHttpOpenRequest(_pTarget->get_handle(), L"GET", NULL,
            NULL, WINHTTP_NO_REFERER,
            WINHTTP_DEFAULT_ACCEPT_TYPES,
            WINHTTP_FLAG_REFRESH|WINHTTP_FLAG_SECURE))
        {
            PVOID Context = this;

            if (WinHttpSetOption(hRequest, WINHTTP_OPTION_CONTEXT_VALUE, &Context, sizeof(Context)))
            {
                AddRef();

                set_handle(hRequest);

                WinHttpSetStatusCallback(
                    hRequest,
                    _StatusCallback,
                    WINHTTP_CALLBACK_FLAG_ALL_NOTIFICATIONS,
                    NULL );

                if (!WinHttpSendRequest(hRequest,
                    WINHTTP_NO_ADDITIONAL_HEADERS,
                    0, WINHTTP_NO_REQUEST_DATA, 0,
                    0, (DWORD_PTR)this))
                {
                    Close(GetLastError(), API_SEND_REQUEST);
                }

                return NOERROR;
            }

            WinHttpCloseHandle(hRequest);
        }

        return GetLastError();
    }

    ULONG Read(HINTERNET hRequest)
    {
        if (DWORD dwNumberOfBytesToRead = (DWORD)min(0x10000, getFreeSpace()))
        {
            PVOID Buf;
            if (ULONG err = AllocBuffer(&Buf, dwNumberOfBytesToRead))
            {
                return err;
            }

            DbgPrint("Begin Read %u\n", dwNumberOfBytesToRead);

            return WinHttpReadData(hRequest, Buf, 0x10000, 0) ? NOERROR : GetLastError();
        }

        return ERROR_BUFFER_OVERFLOW;
    }

    void Close(ULONG err, DWORD_PTR from)
    {
        if (HINTERNET hRequest = set_handle(0))
        {
            dwError = err, dwResult = from;
            WinHttpCloseHandle(hRequest);
        }
    }

    static void CALLBACK _StatusCallback(
        __in  HINTERNET hRequest,
        __in  DWORD_PTR Context,
        __in  DWORD dwInternetStatus,
        __in  LPVOID lpvStatusInformation,
        __in  DWORD dwStatusInformationLength
        )
    {
        reinterpret_cast<DownloadCtx*>(Context)->StatusCallback(
            hRequest, dwInternetStatus, lpvStatusInformation, dwStatusInformationLength);
    }

    void StatusCallback(
        __in  HINTERNET hRequest,
        __in  DWORD dwInternetStatus,
        __in  LPVOID lpvStatusInformation,
        __in  DWORD dwStatusInformationLength
        );
};

void DownloadCtx::StatusCallback(
                                      __in  HINTERNET hRequest,
                                      __in  DWORD dwInternetStatus,
                                      __in  LPVOID lpvStatusInformation,
                                      __in  DWORD dwStatusInformationLength
                                      )
{
    DbgPrint("%x>%p %08x %p %x\n", GetCurrentThreadId(), hRequest,
        dwInternetStatus, lpvStatusInformation, dwStatusInformationLength);

    ULONG Error;
    DWORD_PTR Result;

    switch (dwInternetStatus)
    {
    default: return;

    case WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING:
        SendRequest();
        Release();
        return;

    case WINHTTP_CALLBACK_STATUS_REQUEST_ERROR:
        Error = reinterpret_cast<WINHTTP_ASYNC_RESULT*>(lpvStatusInformation)->dwError;
        Result = reinterpret_cast<WINHTTP_ASYNC_RESULT*>(lpvStatusInformation)->dwResult;
        break;

    case WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE:
        if (WinHttpReceiveResponse(hRequest, NULL))
        {
            return;
        }

        Error = GetLastError(), Result = API_RECEIVE_RESPONSE;
        break;

    case WINHTTP_CALLBACK_STATUS_READ_COMPLETE:

        DbgPrint("READ_COMPLETE:%u\n",dwStatusInformationLength);

        if (dwStatusInformationLength)
        {
            AddData(dwStatusInformationLength);

            if (Error = Read(hRequest))
            {
                Result = API_READ_DATA;
                break;
            }
            return ;
        }

        Error = NOERROR, Result = 0;
        break;

    case WINHTTP_CALLBACK_STATUS_HEADERS_AVAILABLE:

        static volatile UCHAR guz;
        union {
            PWSTR sz;
            PVOID buf;
        };
        PVOID stack = _alloca(guz);
        ULONG cb = 0, rcb = 32;
        do
        {
            if (cb < rcb)
            {
                rcb = cb = (ULONG)((PBYTE)stack - (PBYTE)(buf = _alloca(rcb - cb)));
            }

            if (WinHttpQueryHeaders(hRequest, WINHTTP_QUERY_CONTENT_LENGTH, 0, buf, &rcb, 0))
            {
                Error = NOERROR;

                if (cb = wcstoul(sz, &sz, 10))
                {
                    DbgPrint("Content-Length: %u\n", cb);
                    cb++;
                }
                else
                {
                    Error = ERROR_NO_DATA;
                }
                break;
            }
        } while ((Error = GetLastError()) == ERROR_INSUFFICIENT_BUFFER);

        Result = 0;

        switch (Error)
        {
        case ERROR_WINHTTP_HEADER_NOT_FOUND:
            cb = 0x4000000;// 64 mb reserve
        case NOERROR:
            if (Error = Create(cb))
            {
                break;
            }

            if (Error = Read(hRequest))
            {
                Result = API_READ_DATA;
                break;
            }

            return ;
        }
        break;
    }

    Close(Error, Result);
}

void winhttp()
{
    if (CSession* pSession = new CSession)
    {
        if (!pSession->Open())
        {
            if (CTarget* pTarget = new CTarget(pSession))
            {
                if (!pTarget->Set(L"stackoverflow.com", INTERNET_DEFAULT_HTTPS_PORT))
                {
                    if (DownloadCtx* Ctx = new DownloadCtx(pTarget, 4))
                    {
                        Ctx->SendRequest();

                        Ctx->Release();
                    }
                }
                pTarget->Release();
            }
        }

        pSession->Release();
    }

    MessageBoxW(0,0,L"for demo",0);
}


C# Blackjack prototype I would like some feedback on my code, I'm new to coding with C# but I do have some knowledge on Lua and Python. Is there anything that I need to change/clean up to make it more simplified?
using System;
using System.Linq;

class MainClass {
  public static void Main () {
    int[] cards = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 10, 10, 10};

    Console.WriteLine("Welcome to Blackjack. Here are your draws.");

    Random drawCard = new Random();
    int draw1 = drawCard.Next(cards.Min(), cards.Max());
    int draw2 = drawCard.Next(cards.Min(), cards.Max());

    Console.WriteLine("You recieved a " + draw1 + " card!");
    Console.WriteLine("You recieved a " + draw2 + " card!");

    int sum1 = draw1 + draw2;

    if (sum1 == 21) //Blackjack Ending
    {
      Console.WriteLine("Congratulations! You got " + sum1 + "!");
    }

    else if (sum1 >= 11) //Choice of 3rd draw
    {
      Console.WriteLine("Is " + sum1 + " enough?");
      bool cont1 = false;

      drawChoice(cont1); //Call the draw choice function

      if (cont1 == true)
      {
        int draw3 = drawCard.Next(cards.Min(), cards.Max());
        Console.WriteLine("You drawed a " + draw3 + " card!");

        int sum2 = draw3 + sum1;
        Console.WriteLine("You have a total of " + sum2 + ".");

        if (sum2 > 21) Console.WriteLine("Game Over!");
      }

      else //NPC's turn starts
      {

      }
    }

    else //Player has less than 11 cards, auto draw
    {
      Console.WriteLine("You have a total of " + sum1 + ".");
      Console.WriteLine("You will be forced to draw another card.");

      int draw3 = drawCard.Next(cards.Min(), cards.Max());
      Console.WriteLine("You drawed a " + draw3 + " card!");

      int sum2 = draw3 + sum1;
      Console.WriteLine("You have a total of " + sum2 + ".");
    }
  }

  static void drawChoice(bool contChoice) //Function for player to choose whether to draw
  {
    Console.WriteLine("Would you like to draw another card? Y/N");
    string choice1 = Console.ReadLine();

    if (choice1 == "Y" || choice1 == "y")
    {
      contChoice = true;
      Console.WriteLine(contChoice);
    }

    else if (choice1 == "N" || choice1 == "n")
    {
      contChoice = false;
      Console.WriteLine(contChoice);
    }
  }
}

Just a few notes so you can improve your code:

int sum1 = draw1 + draw2;

if (sum1 == 21) //Blackjack Ending
{
  Console.WriteLine("Congratulations! You got " + sum1 + "!");
}

This won't ever be true, because

you have nowhere in the cards an ace meaning having a possible value of eleven.
the maxValue in Random.Next(int minValue, int maxValue) is the exclusive upper bound of the random number returned.

In void drawChoice(bool contChoice) the method argument contChoice is a value type. You can't modify it like you think. After leaving the method regardless wether the user typed y or n the value of cont1 is still false. You should change the method signature to have no arguments but to return a bool.

if (cont1 == true)

because cont1 already is a bool you won't need to compare it with a bool. You can simply use it as condition like if (cont1) and if  you would need to check wether a bool variable is false you would use if (!cont1).


Balanced centrifuge configurations Given an input N as the size of a centrifuge, I need to find out the number of balanced configurations. The full description is here.

  Centrifuge is a piece of equipment that puts multiple test tubes in rotation at very high speeds. It consists of a cylindrical rotor with holes situated evenly along the circumference of the rotor.
  Because the device is operating at high speeds, the center of mass of all tubes must coincide with the center of the rotor. We assume all tubes have the same mass.

  For the sake of simplicity, we also assume that tubes are point masses, fixed firmly to the rotor. All at the same distance from the center. You may safely assume the (x,y) coordinates of tube k are \$R\sin a, R\cos a\$, where \$a = 2\pi\frac{k}{n}\$

  The problem is: Given a centrifuge with N holes and K tubes, is it possible to balance it?

  example 1: given N = 6 possible values for K are 2,3,4,6.

  example 2: given N = 12, it is possible to balance 5 tubes: put 3 evenly dispersed (every 4th hole), then find a pair of opposite unoccupied holes and insert remaining 2 tubes there.

  example 3: given N = 10, it is impossible to balance 7 tubes: put 5 evenly dispersed, and there's no opposite unoccupied holes left!

  Input

  Line 1: An integer N for the capacity of the centrifuge.

  Output

  Line 1: An integer M for the number of different possible values of K.

This is my code:

import numpy as np

N=int(input())
angle = 2*np.pi/N
z = complex(np.cos(angle), np.sin(angle))

import itertools
combs = []
lst = range(1,N)
for i in range(2, N-1):
    combs.append(i)
    els = [list(x) for x in itertools.combinations(lst, i)]
    combs.append(els)

count=1
lis = []
while count<=len(combs):
    for a in range(len(combs[count])):
        s=0
        for b in range(len(combs[count][a])):
            s+=z**combs[count][a][b]
        if abs(s)<1e-10:
            lis.append(combs[count][a])
    count+=2

lengths = []
for i in lis:
    if len(i) not in lengths:
        lengths.append(len(i))

print(len(lengths)+1)

The code works fine, but slows down after input size of above 20, and is practically unusable after the input grows to 50 or above, due to the for loops. How do I optimize it?

This is not a review, but an extended comment.

The code cannot be meaningfully optimized. Few notes though.

First, thou shalt not bruteforce. It is almost inevitably wrong.
Next, you must realize that it is a number-theoretical problem. Even the names of the test cases suggest so. Among other things it means that testing the sum against 1e-10 is not right.
Finally, do watch the video referenced in the problem. It is a big spoiler, and it doesn't provide a proof of the claim. The proof is far from trivial; if you are interested in the proof, follow up with this is a fascinating reading (trigger warning: heavy-duty math).


Statistical function which computes the average work time in different months It's a task from a Ruby-course, in which I'm currently enrolled:
Ruby Course - Page

Precisely it's one of the assignments for the first week.

Following idea:
Your are given a list in which finished work-tasks are logged.
[
  {work: "item 1", date: "2017-04-26", time: 20},
  {work: "item 2", date: "2017-04-27", time: 27},
  ...

You shall write a function which computes the daily work-time for the different months.
Means: The average daily work-time in April, avg. time in May, ... in June. And so on ...

A data-structure, to work upon, was given. Even the result, which is expected for that data-structure: { "2017-04" => 40, "2017-05" => 14 }.

I was able to write a function, which passed all unit-tests.
Here it is:

#!/usr/bin/ruby
tasks = [
  {work: "item 1", date: "2017-04-26", time: 20},
  {work: "item 2", date: "2017-04-27", time: 27},
  {work: "item 3", date: "2017-04-27", time: 33},
  {work: "item 4", date: "2017-05-05", time: 20},
  {work: "item 5", date: "2017-05-06", time: 12},
  {work: "item 6", date: "2017-05-14", time: 10},
]

# Expected result : { "2017-04" => 40, "2017-05" => 14 }
def work_per_month(tasks)
  days_aggregate = {}

  tasks.each do | task |
    key = task[:date]

    if days_aggregate.key?(key)
      days_aggregate[key][0] = days_aggregate[key][0] + task[:time]
    else
      arr = []
      arr[0] = task[:time]

      days_aggregate[key] = arr
    end
  end

  months_aggregate = {}

  days_aggregate.each do | key, task |
    parts = key.split("-")
    k = "#{parts[0]}-#{parts[1]}"

    if months_aggregate.key?(k)
      months_aggregate[k][0] = months_aggregate[k][0] + task[0]
      months_aggregate[k][1] = months_aggregate[k][1] + 1
    else
      arr = []
      arr[0] = task[0]
      arr[1] = 1

      months_aggregate[k] = arr
    end
  end

  avg_hours_month = {}

  months_aggregate.each do | key, data |
    avg_hours_month[key] = data[0] / data[1]
  end

  avg_hours_month
end

puts work_per_month(tasks)  # Returns {"2017-04"=>40, "2017-05"=>14}

Please take into account that I started Ruby programming just a week ago.

It works and it has passed the tests. But I'm aware that it is clumsy.

Is there are more elegant way to solve the described task?

Without having this sequence of loops?

Rubocop Report

Rubocop was able to solve minor layout issues regarding the use of white spaces. One note-worthy change it proposed is:

[Corrected] Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

days_aggregate.each do | key, task |
  parts = key.split("-")

 parts = key.split('-')

A bigger complexity issue it found was:

Metrics/AbcSize: Assignment Branch Condition size for work_per_month is too high. [33/15]
Metrics/MethodLength: Method has too many lines. [30/10]

This means your method work_per_month is doing way too much. It has more than double the AbcSize than the suggested threshold [33/15]. You should split this method in sub-routines.


Sherlock and The Beast This is my solution to this question from Hackerrank.

  Given N (<= 100000), find the largest N-digit number such that:

  The number has only 3 and 5 as its digits.
  Number of times 3 appears is divisible by 5.
  Number of times 5 appears is divisible by 3.

  If no such number exists, produce -1.

Please guide me. Is there any optimization or improvement that can be done to the code?

a=0
items=[]

a=int(raw_input(''))
for i in range(0,a):
    items.append(int(raw_input('')))

def Sherlock_and_The_Beast(items):
    for i in items:

        if i<3:
            print -1
        elif i%3==0:
            print int(str(5)*i)
        elif i==5:
            print int(str(3)*i)

        else:
            flag=False
            for j in range(i/3,-1,-1):
                if (i-(3*j))%5==0:
                    flag=True
                    print int(str(5)*(3*j)+str(3)*(i-(3*j)))
                    break
            if not flag:
                print -1

Sherlock_and_The_Beast(items)

I think your code could use some restructuring:

The name Sherlock_and_The_Beast is more applicable as a module name instead of a function name. Now, even though I said 'more applicable', it does not mean I would suggest naming your Python script this. I would instead call it sherlock.py.*
Your function should be named something like generate_decent_number. This gives a better description of what it actually does.
I would have your function take a single integer instead of a list and then return the decent number. Doing so, removes the need for a flag variable.
Use xrange instead of range.

There are also a few style notes to point out:

Use descriptive variable names. Even though i (and to some extent j) have been conventionalized as indexing names, I prefer to be more descriptive. i should be item and j should be num.
Use whitespace appropriately. Put single spaces around assignment or conditional operators:

foo = 'bar'
if foo == 'bar'

I prefer to use not when I can. Because 0 evaluates to False in conditional statements, you can use:

elif not item % 5:

instead of:

elif item%5 == 0:

This is mostly a personal preference. Some people abhor checking for negative conditions.

Past these notes above, your code is simple and straightforward which are both Pythonic conventions.

Here is my version:

def get_decent_number(num_digits):
    if num_digits < 3:
        return -1

    three = str(3)
    five = str(5)
    if not num_digits % 3:
        return five*num_digits
    elif num_digits == 5:
        return three*num_digits

    for num in xrange(num_digits/3, -1, -1):
        diff = num_digits - 3*num
        if not diff % 5:
            return five*3*num + three*diff

    return -1


Tests for palindromes in C and C++ A question I've been asked on interactive phone screens is to code a test to determine whether a string is
a palindrome or not. I am providing two examples here: one in C and one in C++. They both work and they both compile
without errors or warnings. I believe my C example is good, with the possible exception of variable names
(some of you may not like the register char* declarations, but they are valid in C).

My goal was an \$O(n)\$ solution with out reversing the string.

My question is about the C++ version, and I miss using iterators. Is there a way to make this more C++ like?

I can think in C; I can't necessarily think in C++.

Palindrome checker in C

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

bool palindromeChecker(char *possiblePalindrome)
{
    bool isPalindrome = true;
    size_t length = strlen(possiblePalindrome);

    if (length < 3)
    {
        isPalindrome = false;
    }
    else
    {
        register char *leftSide = possiblePalindrome;
        register char *rightSide = &possiblePalindrome[length-1];

        while ((leftSide < rightSide) && isPalindrome)
        {
            if (*leftSide != *rightSide)
            {
                isPalindrome = false;
            }
            leftSide++;
            --rightSide;
        }
    }

    return isPalindrome;
}

int main(int argc, const char * argv[]) {
    char testString[BUFSIZ];
    bool iSPalendrome;

    printf("Enter a test string to be checked for if it is a palindrome\n");
    scanf("%s", testString);
    iSPalendrome = palindromeChecker(testString);
    printf("The test string %s is %s a palindrome\n", testString, (iSPalendrome? "" : "Not"));

}

Palindrome checker in C++

#include <iostream>
#include <string>

bool IsPalindrome(std::string PossiblePalindrome)
{
    bool Palidrime = true;

    if (PossiblePalindrome.size() < 3)
    {
        Palidrime = false;
    }
    else
    {
        std::string::iterator leftSide = PossiblePalindrome.begin();
        std::string::iterator rightSide = PossiblePalindrome.end();
        rightSide--; // end is past the end of the possible palindrome so decrement to point to a character.

        while ((leftSide < rightSide) && Palidrime)
        {
            if (*leftSide != *rightSide)
            {
                Palidrime = false;
            }
            leftSide++;
            rightSide--;
        }
    }

    return Palidrime;
}

int main(int argc, const char * argv[]) {
    // insert code here...
    std::string PossiblePalindrome;
    std::cout << "Enter a string to test to see if it is a palindrome.\n";
    std::cin >> PossiblePalindrome;

    std::cout << "The string " << PossiblePalindrome;
    if (IsPalindrome(PossiblePalindrome))
    {
        std::cout << " is";
    }
    else
    {
        std::cout << " is not";
    }
    std::cout << " a palindrome" << std::endl;
}

A special case length < 3 seems like a bug. A string aa is a palindrome, and one could successfully argue that a single-letter string is also a palindrome.
A boolean isPalindrome is redundant, and forces the code to test the effectively same condition twice. An early return is perfectly fine here.
A traditional idiomatic (and arguably more performant) way to iterate forward is *first++. Similarly, an idiom for iterating backwards is *--last (that also avoids subtracting 1 beforehand):

    last = possiblePalindrome.end();
    while (first < last) {
        if (*first++ != *--last) {
            return false;
        }
    }
    return true;

The naming is very inconsistent. I don't see the reason to spell palindrome in 3 different ways. A capitalization in iSPalendrome is also strange.


refactor ruby method i have 3 method which do almost the same thing,they different models to achieve the same      goal.I did small refactoring but its still not looks like
   ruby,can we improve it?
   Thanks in advance.

after:

def get_list(id,list_name,attribute_id,attribute,locale_string)
  map={ "resentact" => ResenAct }, "stageact" => StageAct , "payact" => PayAct" }
  @temp=""
  @list=map[list_name.downcase].find(:all, :conditions => {'id' => "#      {id}".chop_space})
   @list.each do |item|
      @temp = @temp + get_data(item.attribute_id,item) + " " + item.attribute + " " +   locale_string+ ", "
   end
   return  @temp.pop(2) if @temp.length > 1
  end

method call : get_list(id,payact,payact_id,date,"some_string")

before:

 def get_resentact_list(id)
 @temp=""
  @resentacts=ResentAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space})
   @resentacts.each do |item|
      @temp = @temp + get_data(item.shop_id,"resentact") + " " + item.cost.to_s + " " + "string1" + ", "
   end
   @temp = @temp[0,@temp.length-2] if @temp.length > 1
   return @temp

end

 def get_stageact_list(id)
  @temp=""

    @stageacts=StageAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space})
    @stageacts.each do |item|
       @temp = @temp + "string1" + ": " + get_data(item.type_id,"type") + ", " + "str2" + ": " + item.summ + " " + "str3" + ", "
    end
    @temp = @temp[0,@temp.length-2] if @temp.length > 1
    return @temp
 end

def get_payact_list(id)
  @temp=""

   @payacts=PayAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space})
      @payacts.each do |item|
         @temp = @temp + "str1" + ": " + item.date + ", " + "str2" + ": " + item.summ + " " + "str3" + ", "
      end
      @temp = @temp[0,@temp.length-2] if @temp.length > 1
      return @temp
  end

def get_payact_list(id)
  ## Dont use class instance variables ( prefixed with @ ) for method scoped variables
  @temp="" # temp = ""
  ## In this case, you dont need the temporary variable, but lets keep it, but turn it into an array.
  # temp = []

  ## No need for the temp variable here, and still using class instance variables.
    @payacts=PayAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space})
    @payacts.each do |item|
  # PayAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space}).each do |item|

      @temp = @temp + "str1" + ": " + item.date + ", " + "str2" + ": " + item.summ + " " + "str3" + ", "
      ## Lets simply push each item onto the array.
      ## While we are at it, we remove the trailing comma string.
      # temp << "str1" + ": " + item.date + ", " + "str2" + ": " + item.summ + " " + "str3"

    end
    # No need to mangle the end of the string, as we can simply join the temp array with a comma, which will not add to the end.
    @temp = @temp[0,@temp.length-2] if @temp.length > 1
    # temp.join ", "

    # no need for an explicit return. Ruby will return the value of the last expression.
  return @temp
end

Complete example, without changing too much.

def get_payact_list(id)
  temp = []
  PayAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space}).each do |item|
    temp << "str1" + ": " + item.date + ", " + "str2" + ": " + item.summ + " " + "str3"
  end
  temp.join ", " # is implicitly returned.
end

We can do away with the temp variable by using collect() instead of each() and pushing to an array.

def get_payact_list(id)
  list = PayAct.find(:all, :conditions => {'product_id' => "#{id}".chop_space}).collect { |item|

    "str1" + ": " + item.date + ", " + "str2" + ": " + item.summ + " " + "str3"
  }
  list.join ", "
end

Lets go the whole way, here are our new methods

This still looks and feels like duplication, but without a bit more
metaprogramming than I want to go into right now, it will do.
At least the separate concerns are factored out.

  def get_payact_list id
    format_act_list PayAct.find_by_product_id(id).collect { |item|
      [item.date, item.summ]
    }
  end

  # Note: the get_data method should be encapsulated on the item, leaving as is for now.
  def get_stageact_list id
    format_act_list StageAct.find_by_product_id(id).collect { |item|
      [ get_data(item.type_id,"type"), item.summ ]
    }
  end

  def get_resentact_list id
     format_act_list ResentAct.find_by_product_id(id).collect { |item|
       [ get_data(item.shop_id,"resentact"), item.cost ]
     }
  end

  def format_act_list list
    list.collect { |item| format_tuple(item) }.join ', '
  end

  # Takes an array and correctly formats the output
  # Ideally this would live in a presenter or a view.
  def format_tuple tuple
    "str1: %s, str2: %s str3" % [ tuple[0],  tuple[1] ]
  end

And this is the supporting code.

class Act
  # I've moved this common finder method up the (fake) chain of inheritance I invented.
  # It could also live in each of the child objects i.e ( PayAct StageAct ResentAct )
  def self.find_by_product_id (product_id)
    self.find(:all, :conditions => {'product_id' => "#{product_id}".chop_space})
  end
end

class PayAct < Act;end
class StageAct < Act; end
class ResentAct < Act; end


Create a table from MySQL using PHP PDO I am trying to simply present a table in HTML that is stored in a MySQL database. I would like to use Object Oriented PHP to access and fetch the data for the table. I have spent some time learning the different elements and have tried to put together a generic template I can use to access the tables in the database.

Questions:

Is there anything wrong with the code below?
Is there a better way to do this?
Are there any redundancies in the code? Is a more generally preferred/standard way of doing this? I've seen foreach and while being used...

    field1
    field2
    field3
    field4
    field5

<?php
require_once 'db_config.php';
$dbh = new PDO($dsn, $dbuser, $dbpass);
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$sth = $dbh->prepare("SELECT * FROM a_temp");
$sth->execute();
$result = $sth->fetch(PDO::FETCH_ASSOC);
?>
<?php foreach($result as $index => $row) : ?>

    <?php echo $row[field1]; ?>
    <?php echo $row[field2]; ?>
    <?php echo $row[field3]; ?>
    <?php echo $row[field4]; ?>
    <?php echo $row[field5]; ?>

<?php endforeach;?>

The fetch() call only calls a single row from result set.
Your foreach loop will, as a result, just go in the first result and do nothing. It may even produce an error/notice for undefined index in $row.
While using a SELECT statement in PHP, always include the specific columns that you want to fetch. This is among good practices.

$sth = $dbh->prepare("SELECT `field1`, `field2`, `field3`, `field4`, `field5` FROM a_temp");
$sth->execute();
?>
<?php foreach($sth->fetch(PDO::FETCH_ASSOC) as $row) : ?>

    <?php echo $row['field1']; ?>
    <?php echo $row['field2']; ?>
    <?php echo $row['field3']; ?>
    <?php echo $row['field4']; ?>
    <?php echo $row['field5']; ?>

<?php endforeach;?>

Or, if you want to use a while loop:

$sth = $dbh->prepare("SELECT `field1`, `field2`, `field3`, `field4`, `field5` FROM a_temp");
$sth->execute();
?>
<?php while( $row = $sth->fetch(PDO::FETCH_ASSOC) ) { ?>

    <?php echo $row['field1']; ?>
    <?php echo $row['field2']; ?>
    <?php echo $row['field3']; ?>
    <?php echo $row['field4']; ?>
    <?php echo $row['field5']; ?>

<?php } ?>


Compute the height of a tower made of buckets I need to speed up my algorithm. It is about finding the height of a tower. The tower is built from buckets. Each bucket has height and radius (1 <= height,radius <= 1000). Variable bucketCount describe how many buckets are placed on tower (1 <= bucketCount <= 10^6). We set buckets in sequence. Thickness of bucket is 0 (for simplicity)

Image of example tower:

I decided to use a stack. My algorithm for each bucket:

If stack is empty, then push bucket to stack.
If the bucket I hold is narrower then bucket on top then put it on the stack.
Else while bucket I hold is wider: pop and find maximum height (for new bucket ground), after that push bucket which I hold.

For each bucket I added additional an variable ground, which specifies the height at which the bucket is placed. Meantime I keep maximum height of tower in variable.

This task is automatically checked and gives output: time limit exceeded. I tried to optimize code but nothing gives positive result.

Example input: 2 20 20 30 30
Output: 50

#include <iostream>
#include <stack>

using namespace std;

struct Bucket{
public:
    int height;
    int radius;
    int ground;
};

int main()
{
    int ile;
    cin >> ile;
    for(int c = 0; c < ile; c++){
    stack<Bucket> tower;
    int hightestPoint = 0;
    int bucketCount;
    cin >> bucketCount;
    Bucket temp;
    Bucket temporary; //Used to speed up a bit
    int maksimum;
    int sum;
    int counter = 0; //Its size of the Stack. Used it because thought its faster then empty() function
    for(int i = 0; i < bucketCount; i++){
        cin >> temp.radius >> temp.height;
        maksimum = -1;
        sum = 0;
        if(counter == 0){
            temp.ground = 0;
            tower.push(temp);
            counter++;
        } else {
            if(temp.radius < tower.top().radius){ //If bucket is narrower then push it
                temp.ground = tower.top().ground;
                tower.push(temp);
                counter++;
            } else { //If bucket is wider
                while((counter > 0) && temp.radius >= (temporary = tower.top()).radius){ //Pop and search for new ground (maximum)
                    sum = temporary.height + temporary.ground;
                    if(maksimum < sum){
                        maksimum = sum;
                    }
                    tower.pop();
                    counter--;
                }
                temp.ground = maksimum; //Set ground for new bucket
                tower.push(temp);
                counter++;
            }
        }
        sum = temp.height + temp.ground; //Meantime find highest point in stack
        if(hightestPoint < sum){
            hightestPoint = sum;
        }
    }
    cout << hightestPoint << endl;
    }

    return 0;
}

You don't need to remember every single bucket you've seen.

Once a bucket is added to the top of the pile, any wider bucket that has its rim lower than the rim of the new bucket becomes irrelevant, as do all the narrower buckets it sits on.

So a better data structure would just store the altitudes of all rims currently in play; this can be initialised to (0,0).  When you add a bucket, find the widest rim not greater than the new bucket's width; we now set height[0] to that height and height[bucket_width] to height[0]+bucket_height.  Remove all narrower buckets, and all other buckets up to a the new height - they can no longer affect the result.

I've added a working demonstration, with tests, below.

Other items for review:

using namespace std;

Bringing all names in from a namespace is problematic; namespace std particularly so. See Why is “using namespace std” considered bad practice?.
Spellings: highestPoint, maximum.
Naming: temp and temporary - these are declared far from their use, so it's hard to appreciate the difference.
You can omit return 0; in main().

Working solution

#include <algorithm>
#include <iostream>
#include <istream>
#include <iterator>
#include <map>
#include <sstream>

// Assume that unsigned int is large enough to represent the tallest
// tower
unsigned tower_height(std::istream& input)
{
    unsigned count;
    input >> count;
    if (!input) { std::cerr << "reading count" << std::endl; return 0; }

    // start with a point at 0,0 on which everything balances
    std::map<unsigned,unsigned> m{ {0,0} }; // radius -> altitude

    while (count-->0) {
        unsigned radius, height;
        input >> radius >> height;
        if (!input) { std::cerr << "reading size" << std::endl; return 0; }
        // find the widest bucket this sits on top of
        auto smaller = std::prev(m.upper_bound(radius));
        // floor height is now smaller's height
        m[0] = smaller->second;
        // floor extends out over smaller
        if (smaller != m.begin()) {
            m.erase(std::next(m.begin()), std::next(smaller));
        }
        // put this bucket in place (it will always be second)
        auto current = m.insert({radius, m[0]+height}).first;
        // find the first wider bucket that's taller
        auto height_test = [current](auto const& e){ return e.second > current->second; };
        auto wider = std::find_if(current, m.end(), height_test);
        // and remove values up to it
        m.erase(std::next(current), wider);
    }

    return m.rbegin()->second;
}

static bool test(unsigned expected, const std::string& input)
{
    std::istringstream is{input};
    auto actual = tower_height(is);
    if (actual != expected) {
        std::cerr << "Got " << actual
                  << " instead of " << expected
                  << " for input " << input
                  << std::endl;
        return false;
    }
    return true;
}

int main()
{
    int errors = 0;
    errors += !test(0, "0");
    errors += !test(10, "1  20 10");
    errors += !test(20, "2  20 10  20 10");
    errors += !test(15, "2  20 10  10 15");
    errors += !test(25, "2  10 15  20 10");
    errors += !test(25, "3  10 15  20 10   5 10");
    errors += !test(20, "3  20 10   5  5  10 15");
    return errors;
}

A note on complexity

At first glance, we might think that the code here is O(n²), because we used std::find_if() each time we add a bucket.  But find_if() returns at the first match, and we then remove all the elements we've seen, thus reducing work for subsequent iterations.  Therefore on average, we scale as O(n), and that's demonstrable:

std::mt19937 gen{std::random_device()()};
std::uniform_int_distribution dist{1, 1000};
for (unsigned n: {10000, 100000, 1000000}) {
    std::stringstream s;
    s << n;
    for (unsigned i = 0;  i < n;  ++i) {
        s << ' ' << dist(gen) << ' ' << dist(gen);
    }
    auto start_time = std::chrono::high_resolution_clock::now();
    auto height = tower_height(s);
    auto end_time = std::chrono::high_resolution_clock::now();
    auto millis = std::chrono::duration_cast<std::chrono::milliseconds>(end_time - start_time);
    std::clog << "Measured " << n << "-bucket tower as "
              << height << " in " << millis.count() << "ms"
              << std::endl;
}

Measured 10000-bucket tower as 2897237 in 1ms
Measured 100000-bucket tower as 28587991 in 13ms
Measured 1000000-bucket tower as 285125976 in 136ms

The very worst case is where each bucket is narrower and shorter than the previous, meaning that the map continuously grows in size.  This introduces an O(log n) term, since that's how map operations scale.  So we have worst-case performance that scales as O(n log n):

for (unsigned n: {10000, 100000, 1000000}) {
    std::stringstream s;
    s << n;
    for (unsigned i = n;  i > 0;  --i) {
        s << ' ' << i << ' ' << i;
    }
    auto start_time = std::chrono::high_resolution_clock::now();
    auto height = tower_height(s);
    auto end_time = std::chrono::high_resolution_clock::now();
    auto millis = std::chrono::duration_cast<std::chrono::milliseconds>(end_time - start_time);
    std::clog << "Measured " << n << "-bucket tower as "
              << height << " in " << millis.count() << "ms"
              << std::endl;
}

Measured 10000-bucket tower as 10000 in 2ms
Measured 100000-bucket tower as 100000 in 36ms
Measured 1000000-bucket tower as 1000000 in 539ms


Improve performance while Traversing List I was trying to write a Java program to compare previous records while traversing. It returns true if element exists previously, else false.

e.g.

if elements are {"Raj","Jas","Kam","Jas"} then return true (since Jas element available previously)
if elements are {"Raj","Jas","Kam","Tas"} then return false

I have written the below code, but its complexity is n*n, which is not good.  How could I write clean code to improve the performance?

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

public class CompareList {
    private List<String> listNames;
    private List<String> listTempName = new ArrayList<>();

    public CompareList(String[] vListArray) {
        listNames = Arrays.asList(vListArray);
    }
    public CompareList(List<String> vListName) {
        listNames = vListName;
    }
    public boolean isPreviousNameExist() {
        String tempName = null;
        for (int i = 0; i < listNames.size(); i++) {
            tempName = listNames.get(i);
            if (listTempName.contains(tempName)) {
                return true;
            } else {
                listTempName.add(tempName);
            }

        }
        return false;
    }
    public static void main(String[] args) {
        String[] nameArray = {"A", "B", "C", "D", "E", "F"};
        CompareList compareList = new CompareList(nameArray);
        System.out.println("==>" + compareList.isPreviousNameExist());
    }
}

UPDATED

I have updated the code and tried using a recursive way, same way how we sort using MergeSort and find its performance better for a large number of records, compared to HashSet. I believe its complexity is O(log n).

public class NameComparator {

    private String[] listNames;

    private String[] listTempNames;

    private boolean compareFlag = false;

    private int size;
    public boolean isNameAlreadyExist(String[] vListNames) {
        this.listNames = vListNames;
        size = vListNames.length;
        this.listTempNames = new String[size];
        checkAndSort(0, size - 1);
        return compareFlag;
    }
    private void checkAndSort(int low, int high) {

        //Use the merge sort
        }
    }
    ..............................
}

You can actually change the performance to O(n) by changing one line of code here (and adding a couple of imports):

import java.util.Set;
import java.util.HashSet;

...

private Set<String> listTempName = new HashSet<>();

Although this works, we can simplify the code a little bit. Effectively, you want to see if the list of elements in the given List are unique. There's a really easy way to do this: add them all to a Set. If the size of the set and the size of the list are equal, then there are no duplicates. If the sizes are different, then there must be 1 or more duplicate elements. With this, we can simplify your code to:

public boolean isPreviousNameExist()
{
    for(String name : listNames) {
        listTempName.add(name);
    }
    return listNames.size() != listTempName.size();
}

A slight improvement based on comment by @bowmore:

public boolean isPreviousNameExist()
{
    for(String name : listNames) {
        if(!listTempName.add(name)) {
            return true;
        }
     }
     return false;
}


Selenium Java: errorRecovery I have the following code to select an image using Selenium, only now I have added an errorRecovery.  Is there is a better way to right the code in the catch?

    public static void selectHeader() {
    try {
        WebDriverWait wait = new WebDriverWait(driver, 20);
        wait.until(ExpectedConditions.visibilityOfElementLocated(By
                .xpath("//*[contains(@id, 'picture-und-0-select')]")));
        driver.findElement(
                By.xpath("//a[contains(@id, 'picture-und-0-select')]"))
                .click();
        driver.switchTo().frame("mediaBrowser");
        wait.until(ExpectedConditions.presenceOfElementLocated(By
                .linkText("LIBRARY")));
        driver.findElement(By.linkText("LIBRARY")).click();
        driver.findElement(By.tagName("img")).click();
        driver.findElement(By.linkText("Submit")).click();
        driver.switchTo().defaultContent();
    } catch (UnhandledAlertException alert) {
        errorRecovery error = _TestSuite.errorRecovery;
        if (error.getCount() == 0) {
            error.addCount();
            selectHeader();
        } else {
            error.resetCount();
            throw new UnhandledAlertException(alert.toString());
        }
    }
}

Judging by the overall structure it looks quite good, I would change a few things though with respect to formatting:

Your public static void selectHeader() { line seems to be off, might've been due to putting the code here, you should remove 4 blanks.
Take as example this line:

wait.until(ExpectedConditions.visibilityOfElementLocated(By
       .xpath("//*[contains(@id, 'picture-und-0-select')]")));

I think it would read better as:

wait.until(ExpectedConditions.visibilityOfElementLocated(
       By.xpath("//*[contains(@id, 'picture-und-0-select')]")));

Note that now every argument is on one line.
Similar situations a few lines below.

Then onto the exception handling:

First I would change UnhandledAlertException alert to UnhandledAlertException alertEx, to denote that it is about an exception.
errorRecovery error = _TestSuite.errorRecovery confuses me to no ends, hence these suggestions:

_TestSuite should not have the underscore in front of it, I am still assuming it is a class though.
errorRecovery appears to be a class, which really should and needs to be upper-cased to prevent any confusion I had so far.

Your header_afbeelding_selecteer() method call is in Dutch and not in camelCase, it should be something like headerImageSelect().
Lastly, your ErrorRecovery error is not an error, it is an recovery or an errorRecovery, please name your variable like that.

I do not fully get what your error handling code does though, there also seems to be logic ongoing which is not neccessary, all I see that you are really using is the method call to headerImageSelect().

Therefore I suggest to add the following to your ErrorRecovery class:

public void handleRecovery(final Runnable methodToRun, final String exceptionMessage) throws UnhandledAlertException {
    if (getCount() == 0) {
        addCount();
        methodToRun.run();
    }
    else {
        resetCount();
        throw new UnhandledAletException(exceptionMessage);
    }
}

Which can then be called as:

errorRecovery.handleRecovery(new Runnable() {
    @Override
    public void run() {
        headerImageSelect();
    }, alertEx.toString());

Or if you are using Java 8, just:

errorRecovery.handleRecovery(() -> headerImageSelect(), alertEx.toString());

or even better:

errorRecovery.handleRecovery(this::headerImageSelect, alertEx.toString());

The design still seems weird to me though, which perhaps now is even more obvious, why does your error recovery method throw an UnhandledAlertException? Indicating that your error would need recovery again!


Shortest prime number generator we could come up with? I worked with a friend on the following prime number generator. Our goal was to make it as short and as fast as possible. This is what we came up with:

def p(n): # will print out primes between 0 and n
  s=[True]*(n/2)
  for i in range(int((n/2-1)/2) >> 1):
    for j in range((i*(i+3)<<1)+3,n/2,(i<<1)+3): s[j]=False
  return [2] + [((i<<1)+3) for i in range(n/2) if (s[i])]
print p(input()) # input number for n

Can it be made any faster? Also, it crashes due to memory errors if numbers larger than 1010 are inputted. Can this be fixed to make it more memory efficient?

0. Quick comments on your code

Clearer function names and
variables in your code. What does p mean, and what is s? I can
guess, but please.
Include a docstring or some indication of what your code does
Use the  if __name__ == "__main__": module.

I did some quick speed comparisons of your function which you can see below.

magic1            21.9095528103 ms
magic2            26.5589244423 ms
Joseph Farah    1030.95786404   ms

I did 10 runs with primes up to \$2\$ million. Now note the big gap between the optimized functions and yours. Even if you were to speed up your code tenfold you would still be 10 times slower than the other functions.

  You can add as many horses you want to your cart, but it will never beat a ferrari.

With this in mind i propose three solutions

Implement a better algorithm
Use Import
Give up

1. New algorithm

Wikipedia can shed some light on how to find larger primes.

  For the large primes used in cryptography, it is usual to use a
  modified form of sieving: a randomly chosen range of odd numbers of
  the desired size is sieved against a number of relatively small primes
  (typically all primes less than \$65,000\$). The remaining candidate
  primes are tested in random order with a standard probabilistic
  primality test such as the Baillie-PSW primality test or the
  Miller-Rabin primality test for probable primes.

So in short you use for an example Wheel factorization to find possible primes, then use the Rabin_primality_test to test whether the candidates are primes. Without going too technical the Rabin_primality_test is a probabilistic test. If the function returns True p might be prime, if False we are guaranteed it is not. If n is composite then the Miller–Rabin primality test declares n probably prime with a probability at most \$4^{−k}\$. A friend told me banks used tested primes 80 times. The chances of a composite number passing \$80\$ tests is \$4^{-80}\$ which is in incredibly low number.

A comparison of many different sieves and ways to find primes can be found here https://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n. I used the fastest one on my machine primesfrom2to to generate the results above

import numpy as np
def magic1(n):
    # https://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n-in-python/3035188#3035188
    """ Input n>=6, Returns a array of primes, 2 <= p < n """
    sieve = np.ones(n/3 + (n%6==2), dtype=np.bool)
    sieve[0] = False
    for i in xrange(int(n**0.5)/3+1):
        if sieve[i]:
            k=3*i+1|1
            sieve[      ((k*k)/3)      ::2*k] = False
            sieve[(k*k+4*k-2*k*(i&1))/3::2*k] = False
    return np.r_[2,3,((3*np.nonzero(sieve)[0]+1)|1)]

2. Use Import

Python is magical in the sense that you can find libaries which pretty much do anything. The best part is that installing new packages is easy as \$\pi\$. There are many libaries for primes, I will mention a few below

primesieve
primefac
pyprimesieve

primesieve] is Python bindings for the primesieve C++ library. It can generate list of primes at an incredible speed, and also has fast nthprime implementation. primefac is incredibly good at factoring primes, it can also generate primes, but is rather slow. pyprimesieve
has functions such as prime_sum and has many nifty specialized functions.

So if you want a golfed answer the best you can get in terms of speed is

from primesieve import generate_primes

if __name__ == '__main__':

    print generate_primes(2*10**6)

3. Give up

Even if you are in your prime finding and generating large primes are incredibly hard. However this is a good thing as primes are incredibly important in internet security. If by some miracle you were to find an incredibly, incredibly fast prime generator it would break all the large banks and make payments over the net unsafe.

So perhaps it is a good thing your code is slow ;)


Dynamic loading of Javascript dependency graph I have JavaScript module for loading dynamically script files by dependency graph:

var module = {};

module.loadScript = function (url, callback) {
    var script = document.createElement('script');
    script.type = 'text/javascript';
    script.src = url;
    script.async = true;
    script.charset = 'utf-8';
    document.body.appendChild(script);
    console.time(script.src + ' Load');

    module.addEvent(script, 'load', function () {
        console.timeEnd(this.src + ' Load');
        document.body.removeChild(this);

        if (typeof callback === "function") {
            callback();
        }
    });
};

module.loadJsTree = function (tree, callback) {
    _loadJsTreeInternal(tree, callback, 0, 0);
};

function _loadJsTreeInternal (tree, callback, _totalCount, _loadedCount) {
    tree = tree || {};

    var branches = Object.keys(tree);
    _totalCount += branches.length;

    branches.forEach(function (branch, index) {
        console.log("Start load ", branch, tree[branch]);

        module.loadScript(branch, function () {
            _loadedCount++;

            if (tree[branch] !== null && typeof tree[branch] === 'object') {
                _loadJsTreeInternal(tree[branch], callback, _totalCount, _loadedCount);
            }
            else if (_loadedCount == _totalCount) {
                if (typeof callback === "function") {
                    callback();
                }
            }
        });
    });
}

//RUN SCRIPT

var scripts = {
    '/script1.js': null,
    '/script2.js': {
        '/script2.plugin1.js': {
            '/script2.plugin1.plugin1.js': null
        },
        '/script2.plugin2.js': null
    }
};

module.loadJsTree(scripts, function () { alert("ALL SCRIPTS READY!") });

Do you see any other improvements / issues?

I have JavaScript module for loading dynamically script files by dependency graph:

But... why? There are libraries out there that already do this. Take for instance, RequireJS. Most libraries out there ship as UMD modules (defined as CommonJS, AMD, and global), you can easily drop them into RequireJS-like or Browserify-like workflows.

Also, if you already know the order at which the modules come in (because of your deps graph), why not take a step further and just use ES modules together with a bundler like Rollup? Together with Uglify, you can strip away unnecessary bloat. Aside from the tooling, ES modules explicitly define their dependencies. You don't need an external deps map.

Async loading of modules all at once only prevent render-blocking and make your site look fast. It does not speed up loading of the site. What makes sites faster is smaller or lesser dependencies, or async loading plus loading only what's needed at the moment.

Consider using the tools existing in the ecosystem instead of building your own.


Moving an element from one unordered_map to another This code moves one element from one unordered_map to another.

I would like to ask if my code below can be improved.

#include <iostream>
#include <string>
#include <vector>
#include <utility>
#include <unordered_map>

int main()
{
    struct Data
    {
        int foo;
        bool boo;
    };
    std::unordered_map<std::string, Data> map_A = {{"A", {1, true}}, {"B", {2, false}}};
    std::unordered_map<std::string, Data> map_B = {{"C", {3, false}}, {"D", {4, true}}};;

    std::string keyToMove = "C";
    map_A.insert({keyToMove, map_B[keyToMove]});
    map_B.erase(keyToMove);

    std::cout << "map_A" << std::endl;
    auto it = map_A.begin();
    while(it != map_A.end())
    {
        std::cout<<it->first<<" :: "<<it->second.foo<<std::endl;
        it++;
    }
    std::cout << "map_B" << std::endl;
    auto it2 = map_B.begin();
    while(it2 != map_B.end())
    {
        std::cout<<it2->first<<" :: "<<it2->second.foo<<std::endl;
        it2++;
    }
    return 0;
}

Result:

map_A
C :: 3
B :: 2
A :: 1
map_B
D :: 4
Program ended with exit code: 0

Unused Includes
The code is not using std:vector, nor is it using anything from utilities. It might be better not to include these, that can improve your compile time and decrease the size of the generated code.

Don't Repeat Yourself
There are two obvious functions that can be written to shorten the code in main(). One function prints the contents of an unordered map and the other moves the contents from one map to the other. When you see code repeating itself in a function that is a good indication that you can create a function to reduce the amount of code in the function to make it simpler.

Program Exit Status
In this particular program it isn't necessary to have return 0; in main() C++ will generate it for you. When you have programs that might fail it might be better to include cstdlib and use the symbolic constants EXIT_SUCCESS and EXIT_FAILURE to indicate success or failure to the calling program.


Fill a vector with uniformly distributed random complex numbers I want to fill an array of complex numbers using a uniform generator. I thought up the next code. (Complex is a simple fixed-point complex data type.)

std::generate(inputData.begin(), inputData.end(), []()-> Complex {
    static std::default_random_engine generator;
    static std::normal_distribution<double> distribution(0.0, 0.5); // mean = 0.0, stddev = 0.5
    return Complex(distribution(generator), distribution(generator));
});

So I though up using static variables within the lambda expression. Is that inefficient? Alternatively I would create them outside of the lambda, and put them on the capture list. But I don't need them outside of the lambda, so this seems cleaner to me.

The problem with this is that it leaks memory, although not in the traditional sense of leaking.

Function static variables live till the end of the program. So while generator and distribution are only created when this code is first run, they only get destroyed at the end of your program even though you never use them later. This eats up memory the entire time.

On the other hand, the amount of memory it eats up is pretty small and constant, so for almost all purposes, it shouldn't make too big of a difference. Even if you repeatedly called a function that had this code in it, the static variable would live and not be recreated. Just don't use this in a way that generates the lambda 100s of times, such as in intense template metaprogramming.

In C++17, if you were using this to initialize a big enough vector, you could make use of execution policies to parallelize this like so:

std::generate(std::execution::par_unseq, inputData.begin(), inputData.end(), []()-> Complex {
    thread_local std::default_random_engine generator; // thread_local so we don't have to do any locking
    thread_local std::normal_distribution<double> distribution(0.0, 0.5); // mean = 0.0, stddev = 0.5
    return Complex(distribution(generator), distribution(generator));
});

This has the added benefit that these variables will be destructed when the threads used finish, although when that happens is mostly unclear.


Transform this to one line arrow function ES6+ I am using one line arrow functions a lot, if they are well written I believe they can make code easier to read. I believe that the following code can be easily transformed into one liner but I can't figure out how. Any ideas please? I am asking more just from curiosity.
const getUID = async () => {
  let uid;
  await firebase.auth().onAuthStateChanged(async (user) => {
    if (!user) await firebase.auth().signInAnonymously();
    uid = user.uid;
  });
  return uid;
};

First of all, onAuthStateChanged expects a callback and can be fired multiple times. To resolve that you can use unsubscribe() after the first success.
Secondly, for your code to work, you need to wait for signInAnonymously. At the moment you assign it to a temporary variable, but I can't see it working. Because onAuthStateChanged accepts a callback that can be fired multiple times, it won't wait in the main scope. To solve this, I suggest using Promise constructor.
Finally, you call signInAnonymously, but you don't do anything with its response. You need to use user.uid if the user is logged in, otherwise you need to use signInAnonymously().user.uid
So the final solution:
const getUID = async () => new Promise(resolve => {
    const unsubscribe = firebase.auth().onAuthStateChanged(user => {
        unsubscribe();
        resolve(user ? user.uid : firebase.auth().signInAnonymously().then(v => v.user.uid))
    });
})

const uid = await getUID()

Explanation:

Wrap everything to Promise and resolve it when we get uid
Auth & listen on onAuthStateChanged
Unsubscribe on the first onAuthStateChanged callback trigger (because one trigger it's enough in your case)
Check if a user is defined (that means the user is signed in), resolve promise with user uid
If the user is not defined, use signInAnonymously to sign anonymous user, resolve that user `uid.


Curvature data to coordinates I want to determine a path (x and y coordinates) by using curvature data (radius, with constant distance (length_of_arc) travelled between data points. I want to repeat this calculation every 0.010 seconds and visualize the points in pyhon kivy (at the moment a raw calculation takes 0.006 - 0.014 seconds).

Assume sample data

radius = [68.96551724, 69.73500697, 71.78750897, 75.98784195, 81.96721311, 90.74410163, 102.6694045, 120.0480192,
          146.1988304, 187.9699248, 265.9574468, 458.7155963, 1694.915254, -1020.408163, -393.7007874, -244.4987775]

This is the code (inside a class) that I use to calculate the coordinates. Is there a way to increase performance / readability of the code. It is important to note that performance has higher priority.

EDIT: I already tried small angle approximation (sin x \approx x, cos x \approx 1 - 0.5 x^2) but that did only lead to insignificant changes.

# positive curvature is a right bend, negative curvature is a left bend
# scale_factor: used to scale the values for plotting them
# initial_displacement_x and initial_displacement_y: values are displaced such that the plot is in the Center of the screen.
@staticmethod
def calculate_coordinates_from_length_of_arc_and_radius(_radius, length_of_arc, scale_factor,
                                                    initial_displacement_x, initial_displacement_y):
    phi = 0  # accumulation of angle
    number_of_data_points = len(_radius)
    x_accumulated = number_of_data_points * [0]
    y_accumulated = number_of_data_points * [0]
    track_coordinates = 2 * number_of_data_points * [0] # x,y alternating needed for ploting values as line in python kivy

    for data_point in range(number_of_data_points - 1):
        delta_phi = - length_of_arc / _radius[data_point]
        phi = phi + delta_phi
        dx = - _radius[data_point] * (cos(delta_phi) - 1)
        dy = - _radius[data_point] * sin(delta_phi)

        if data_point != 0:
            dx_rotated = cos(phi) * dx - sin(phi) * dy # apply rotation matrix
            dy_rotated = sin(phi) * dx + cos(phi) * dy # apply rotation matrix
        else:
            dx_rotated = dx  # the displacements are not rotated
            dy_rotated = dy  # for the first data point

        x_accumulated[data_point + 1] = x_accumulated[data_point] + dx_rotated
        y_accumulated[data_point + 1] = y_accumulated[data_point] + dy_rotated
        track_coordinates[2 * data_point + 2] = scale_factor * (x_accumulated[data_point] + dx_rotated)
        track_coordinates[2 * data_point + 3] = scale_factor * (y_accumulated[data_point] + dy_rotated)

    for data_point in range(len(track_coordinates)):
        if data_point % 2:
            track_coordinates[data_point] = track_coordinates[data_point] + initial_displacement_x
        elif not data_point % 2:
            track_coordinates[data_point] = track_coordinates[data_point] + initial_displacement_y
    return track_coordinates

If you are doing a lot of numerical calculations in Python and want them to be fast, you should use numpy, a package specifically for this purpose.

It allows you to write your code in a vectorized way, making it easer to understand at the same time:

import numpy as np

@staticmethod
def arc_coordinates(_radius, length_of_arc, scale_factor, x0, y0):
    """
    positive curvature is a right bend, negative curvature is a left bend
    scale_factor: used to scale the values for plotting them
    x0 and y0: values are displaced such that the plot is in the center of the screen.
    """
    radius = np.array(_radius)
    delta_phi = - length_of_arc / radius
    phi = np.cumsum(delta_phi)
    dx = - radius * (np.cos(delta_phi) - 1)
    dy = - radius * np.sin(delta_phi)
    track_coordinates = np.vstack([dx, dy]).T
    track_coordinates[1:, 0] = np.cos(phi[1:]) * dx[1:] - np.sin(phi[1:]) * dy[1:]
    track_coordinates[1:, 1] = np.sin(phi[1:]) * dx[1:] + np.cos(phi[1:]) * dy[1:]
    track_coordinates = scale_factor * np.cumsum(track_coordinates, axis=0) + np.array([[y0, x0]])
    track_coordinates = np.concatenate([np.array([y0, x0]), track_coordinates.flatten()[:-2]])
    return track_coordinates

This gives the same results as your code. Note that it (just like your code) uses the weird convention that the output array is flat, alternating between x and y values, starting with a y-value.

On my machine this is not actually faster than your implementation, though:

%timeit arc_coordinates(radius, 1, 1, 0, 0)
39.4 µs ± 849 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
%timeit calculate_coordinates_from_length_of_arc_and_radius(radius, 1, 1, 0, 0)
21.5 µs ± 457 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

However, this probably has a few possibilities for improvement left (WIP).

I also renamed your function to a (hopefully) also clear but shorter name as well as the initial displacements. I also made your comment detailing the parameters a docstring.


Inner product of random floats written to file My goals are to make this code:

faster
more idiomatic C++

What the code does:

take an integer command-line argument N
make a vector of N random floating point values
make a matrix of N*N random floating point values
do inner product of vector * matrix
write the resulting vector to a file

Currently compiled with: g++ myFile.cpp -std=c++11 -O3

#include <vector>
#include <fstream>

const std::string outputName = "output/coutput.txt"; // the name of the file this program makes

std::ostream& operator<< (std::ostream& os, const std::vector<double>& v)
{ // pretty-print a vector of doubles using operator<<
  for (auto& i : v)
    i == *v.begin() ? os << i : os << " " << i;
  return os;
}

double genRand(void)
{
  return rand() / static_cast<double>(RAND_MAX);
}

int main(int argc, char* argv[])
{
  // file i/o
  std::ofstream result;
  result.open(outputName, std::ios::out);

  srand(1);
  long elems = std::atol(argv[1]); // command line param
  std::vector<double> m(elems * elems); // (elems * elems); // actually this is a vector of length (N*N)
  std::vector<double> a(elems);
  std::vector<double> r(elems, 0); // to store the result

  for (auto i = 0; i < elems; i++) // generate random vector
    a[i] = genRand();

  for (auto i = 0; i < elems * elems; i++) // generate random matrix
    m[i] = genRand();

  for (auto i = 0; i < m.size(); i++) {
    auto idx = i / a.size();
    r[idx] += a[idx] * m[i];
  }

  result << r << std::endl; // print results
  result.close();
}

If you're going to use C++11, you should firstly look at throwing out rand, and using the newer features found in <random>. The setup is a bit more complex, but the distributions are generally much better statistically. Also, instead of generating numbers one at a time, we're going to generate them in a batch:

template <typename Cont = std::vector<double>>
Cont generate_rand(std::size_t n)
{
    Cont results;
    results.reserve(n);

    std::random_device rd;
    std::mt19937 generator(rd());
    std::uniform_real_distribution<> dist;
    auto random = [&]() { return dist(generator); };
    std::generate_n(std::back_inserter(results), n, random);
    return results;
}

With this, we can generate a set of random values into a vector:

// Read into num however many values we want to generate from args
auto vec = generate_rand(num);

Working with an actual matrix is a bit easier, so let's generate that (instead of an n * n length vector):

std::vector<std::vector<double>> matrix;
for(auto i = 0; i < num; ++i) {
    matrix.emplace_back(generate_rand(num));
}

Something to hold our results, with the size correctly reserved:

std::vector<double> result;
result.reserve(num);

The algorithm library already has a function that will do what you want: std::inner_product:

for(auto it = matrix.begin(); it != matrix.end(); ++it) {
    auto v = std::inner_product(std::begin(vec), std::end(vec), std::begin(*it), 0.0);
    result.push_back(v);
}

Alternately, this is the place where a library like Eigen comes in really handy; with the correct includes, this can be written as follows:

#include <Eigen/Core>

typedef Eigen::MatrixXd Dynamic2d;

int main()
{
    // Initialize num from program args
    auto x = Dynamic2d::Random(num, num);
    auto y = Dynamic2d::Random(1, num);
    auto res = y * x;
}

(Note however that Random gives a range between [-1, 1] when called like this, so you'd need to potentially do a bit of work to get that distribution back to [0, 1]). This is likely to give you the maximum performance, as libraries like Eigen have had a lot of time spent on making them highly efficient.


Hooking with Python3 Decorators I wrote this prototype after reading the Wikipedia article on hooking.  I didn't bother to read any of the code examples listed there because, well, I just didn't.  I don't have a good excuse.

The documentation in this file should tell you enough about its purpose and intended usage.  I'm looking for a confirmation of whether I even understand what hooks are and how they should be used.  I'm also curious about any sort of preferred method of implementation or any way that I could improve these two objects (Hook and OnHook).

"""Implement hooks with a simple interface using decorators.

The Hook class creates a function that can be placed in code as an
anchoring point for as yet undetermined functionality, while the
OnHook function creates the interface to that functionality.

To create a hook, use the @Hook decorator on an empty function of
your chosen name.  The function's __call__ method will be generated
by the decorator.  Then, create a function that accepts a function
as an argument and adds it to the targets callback by using @OnHook.
OnHook expects the target hook as an argument.

Example:

    @Hook                           # decorator, no args
    def my_hook(): pass             # empty func, no params
    @OnHook(my_hook)                # decorator, hook as arg
    def on_my_hook(func): pass      # empty func, func as param

To add a callback to the new hook, use:

    on_my_hook(my_callback)

When the hook is executed, your callback will be executed along with
any other callbacks that have been registered.

Written 2014-02-02 by Jack Stout.

"""

class Hook:
    """Decorator to create a hook."""
    def __init__(self, func):
        self.callbacks = []

    def __call__(self):
        for callback in self.callbacks:
            callback()

def OnHook(target):
    """Decorator to create an interface to a hook.

    Requires a target hook as only argument.

    """
    def decorator(func):
        def add_callback(func):
            target.callbacks.append(func)
        return add_callback
    return decorator

# Here I've created two hooks with interfaces which would be used
# immediately before and after a hypothetical initiative() function.
@Hook
def pre_initiative(): pass
@OnHook(pre_initiative)
def on_pre_initiative(func): pass

@Hook
def post_initiative(): pass
@OnHook(post_initiative)
def on_post_initiative(func): pass

# Two dummy functions are created and are added to the hooks' callback lists.
def dummy_func_1():
    print("Inside pre_initiative!")
on_pre_initiative(dummy_func_1)

def dummy_func_2():
    print("Inside post_initiative!")
on_post_initiative(dummy_func_2)

# These print functions reveal what has been registered in the hook.
print(pre_initiative.callbacks)
# This function call is what we would see in production code.
pre_initiative()

print(post_initiative.callbacks)
post_initiative()

Less important but related:

This is a prototype but I'm working on a game project that could benefit from this method of quickly attaching function pointers to upcoming state changes.  While working on this file, I was imagining the first phase in a tabletop combat encounter where the players and game master roll initiatives and determine the combat order.  If a character were to gain a power that allowed them to adjust their initiative roll after everyone has rolled, it would require only a call to on_post_initiative(power_name).  When initiative() completes, post_initiative() would fire and execute power_name().

Before getting any feedback, if I were to revisit this code I would add the ability to include arguments to be used with the callback, i.e. on_post_initiative(power_name, character_name).

This appears to be an implementation of something akin to the observer pattern. Your implementation is straightforward enough, but there are two things that jump out at me.

First, I'm surprised that this involves two top-level decorators? I think I would rather see a pattern similar to what property provides. Namely, rather than exposing both @Hook and @OnHook, make @Hook add an attribute to the function that can itself, as a decorator, hook other functions to the first (implementation left as an exercise):

@Hook
def pre_initiative(): pass

@pre_initiative.when_called   # or preferred name
def dummy_func_1(): print("pre-initiative!")

Second, I'm a little confused about your larger use case. Do hooks have a chance to change the inputs or outputs of the hooked function? Are you working on this project with other people who may be more used to some particular approach? (If so, it's easier for them to work with if it matches the approach they expect.) Are these phases easier to implement as functions called on an instance or a subclass?

class Player:
    def pre_initiative(self): pass
    def post_initiative(self): pass

def encounter(players):
    for player in players:
        player.pre_initiative()
    for player in players:
        player.post_initiative()


Print diamond of any size I'm trying to print a diamond of asterisks of an arbitrary size. I'm new to F#, so I'm interested in virtually any feedback, but especially adherence to idioms, convention, and functional orientation.

module Diamond
open System

let rangeWithInverse (max) =
    List.zip [ 1 .. max ] [ max .. -1 .. 1]

let listAscDesc (list) =
    list @ (list |> List.rev |> List.skip 1)

let starsAndSpacesString (stars: int, spaces: int): string =
    String.replicate spaces " " + String.replicate ((stars * 2) - 1) "*"

let diamond =
    rangeWithInverse 18
        |> listAscDesc
        |> Seq.map starsAndSpacesString
        |> Seq.reduce (fun a b -> a + "\n" + b)

[<EntryPoint>]
let main argv =
    diamond |> printfn "%s"
    0

It's hard to comment on it. It seems OK to me - maybe a little to much list manipulation. I'm not sure if string concat with + is acceptable practice in F# but in general it's not. For fun I've tried with my own version with use of recursion:

let newDiamond n =
    let rec print nn f =
        if nn <= n && nn > 0 then
            let spaces = String.replicate ((n - nn) / 2) " "
            sprintf "%s%s%s\n%s" spaces (String.replicate nn "*") spaces (print (f nn) f)
        else
            ""
    let add x = x + 2
    let subt x = x - 2
    let dn = if n % 2 = 0 then 3 else 2
    sprintf "%s%s" (print 1 add) (print (n - dn) subt)

[<EntryPoint>]
let main argv =
    newDiamond 36 |> printfn "%s"
    Console.ReadLine() |> ignore
    0

Version with pattern match instead of if:

let newDiamond n =
    let rec print nn f =
        match nn with
        | x when x <= 0 || x >= n + 1 -> ""
        | _ -> let spaces = String.replicate ((n - nn) / 2) " "
               sprintf "%s%s%s\n%s" spaces (String.replicate nn "*") spaces (print (f nn) f)

    let add x = x + 2
    let subt x = x - 2
    let dn = if n % 2 = 0 then 3 else 2
    sprintf "%s%s" (print 1 add) (print (n - dn) subt)

It's possible but does it add any good?

EDIT:

A condensed solution could be:

let newDiamond n =
    let topNs = [1..2..n]
    let diamondNs = List.append topNs (topNs |> List.rev |> List.tail)

    let createLine x =
        sprintf "%s%s" (String.replicate ((n - x) / 2) " ") (String.replicate x "*")

    (diamondNs |> Seq.mapFold (fun strs x -> ("", sprintf "%s\n%s" strs (createLine x))) "") |> snd

However none of the solutions are very performant if diamond size is increased (> 2000) due to the concatenation of strings. In that respect a good old iterative solution is preferable:

let newDiamond n =
    let createLine = (fun x -> sprintf "%s%s" (String.replicate ((n - x) / 2) " ") (String.replicate x "*"))
    let topHalf = [ 1..2..n ]
    let indices = topHalf @ (topHalf |> List.rev |> List.tail)
    seq { for ln in indices do
            yield createLine ln } |> Seq.iter (printfn "%s")

A gentle "refactoring" of BadHeuristics code:

let rangeWithInverse max =
    List.zip [ 1 .. max ] [ max .. -1 .. 1]

let listAscDesc list =
    list @ (list |> List.rev |> List.tail) // tail instead of skip 1

let starsAndSpacesString (stars, spaces) =
    sprintf "%s%s" (String.replicate spaces " ") (String.replicate ((stars * 2) - 1) "*") // sprintf instead of ""+""

let diamond n =
    rangeWithInverse (n / 2)
        |> listAscDesc
        |> Seq.map starsAndSpacesString
        |> Seq.reduce (fun a b -> sprintf "%s\n%s" a b) // sprintf instead of ""+""


Writing a log file with threads Here are all the threads write the log file.  I want to know if the below script is fine, if it needs any modifications, and whether I am utilising threads properly to execute this code quickly.

open(LOG,"+>","log.txt")or die "cant open the file";

my $file_total= scalar @listOfFiles;

    my $div = $file_total/$thread_count;
    $div = ceil($div);
    my $start = 0;
    my $end = $div;

    for ($count = 1; $count <=$thread_count ; $count++)
    {
        my $thread = threads->new(\&register, $start, $end );
        push(@threads,$thread);
        $start = $end;
        $end = $end + $div;
        if ($end > $file_total)
        {
            $end = $file_total;
        }
    }

    foreach (@threads)
    {
       $_->join;
    }
    close(LOG);
    sub register
    {
         my $lstart = shift;
         my $lend = shift;

         for (my $index = $lstart; $index < $lend; ++$index)
         {
             print LOG $listOfFiles[$index];
             process......
             ..........
             .......
         }
     }

process... placeholder: I have a huge number of documents, say, 1 million.  I am collecting all the 1 million filenames, storing them in the listOfFiles array, and dividing the array index based on the number of threads the user wants.  I am passing every thread and the start and end indices to fetch the document name, process the document, and store the status of the output to a log file.

You can atomically write to a file if you are working with a threadsafe queue.

#!/usr/bin/env perl

use warnings FATAL => 'all';
use strict;
use Time::HiRes qw(time nanosleep);
use threads;
use Thread::Queue;

my $logQ = Thread::Queue->new();

my $consumer = threads->new(\&_logConsumer);

my @producer;
push(@producer, threads->new(\&_logProducer)) for (1..10);
$_->join() foreach @producer;

$logQ->enqueue('STOP');

$consumer->join();

sub _logProducer {

  for (1..10) {
    $logQ->enqueue(sprintf('[%-17s:%04i]', time(), threads->tid()));
    nanosleep(int(rand(10_000)));
  }

  return;
}

sub _logConsumer {

  open(my $fh, '>', '/tmp/atomic.log')
     or die 'Cannot open log file, stopped';

  my $running = 1;
  while($running) {
    if (defined(my $item = $logQ->dequeue())) {
      $running = undef if $item eq 'STOP';
      print $fh "$item\n";
    }
  }
  close($fh);

  return;
}

The output of /tmp/atomic.log looks like:

[1526568957.83079 :0011]
[1526568957.8308  :0006]
[1526568957.83081 :0009]
[1526568957.83082 :0010]
[1526568957.83087 :0008]


A PHP function that prints the url based on path I'm trying to come up with a smart wordpress inspired PHP function that builds an url for my CMS. It has some different requirements that are almost all fulfilled so far.

Must use correct domain
Must be able to use subfolder (Eg. /blog/index.php)
Must be able to specify protocol (if http or https)
Must accept queries (Eg. ?sort=date)

The purpose is to make it easier to print urls when developing through my site so that I don't have to type the whole url myself. It's also so the CMS can run on as many different websites as possible, both with subfolders and without.

My code so far:

function _url( $path = null, $args = "" ) {

    $protocol = "http";

    if ( isset($_SERVER['HTTP_HOST']) && !empty($_SERVER['HTTP_HOST']) ) {
        $domain = $_SERVER['HTTP_HOST'];
    } else {
        $domain = $_SERVER['SERVER_NAME'];
    }

    if ( isset($_SERVER['HTTPS']) ) {
        $protocol = ($_SERVER['HTTPS'] && $_SERVER['HTTPS'] != "off") ? "https" : "http";
    }

    if ( isset($_SERVER['PHP_SELF']) && strlen(dirname($_SERVER['PHP_SELF'])) > 1 ) {
        $subfolder = dirname($_SERVER['PHP_SELF']);
    } else {
        $subfolder = "";
    }

    if ( !$path ) {
        $path = str_replace( $subfolder, "", strtok($_SERVER["REQUEST_URI"],'?') );
    }

    $url = $protocol . "://" . $domain . $subfolder . $path . $args;

    return $url;
}

An example of use:

// A website where the domain is http://domain.com
echo _url( "/about.php" );

// A website where the domain path is https://domain.com/blog/
echo _url( "/users.php", "?sort=date");

Which prints:

http://domain.com/about.php
https://domain.com/blog/users.php?sort=date

I'm pretty satisfied so far with my code. But I'm 100% sure that there is a smarter way. I tried to dig out the functions that wordpress use, but I couldn't figure it out.

Any help or proposals are appreciated. I hope it makes sense.

I am not sure that $_SERVER['SERVER_NAME'] would be a valuable fallback for $_SERVER['HOST_NAME'] as this can be really any arbitrary string.
Keep your 'protocol' portion of code together.  Also you can streamline conditional for https case.

Example:

$protocol = 'http';
if (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] === "on") {
    $protocol = 'https';
};

You could also simplify here and just eliminate this https check if you simply build your URL string like //domain.com/uri with no protocol shown.  This would mimic current behavior of simply continuing current protocol.  You might consider adding parameter to force http vs. https if you ever need to move between the two protocols.

It does not make sense to me why the path of the currently executing script or the current request URI would have anything to do with formulating the URI portion of the URL when this URI value is being explicitly passed in as a parameter to this function.
I do not follow the logic on the null use case for URI.  Why would this case exist?  Is this just to link a page to itself?
You should add validation around parametric input.  In this case, I would guess you need to validate URI as null or non-zero-length string. You might need to validate that that $args is empty string or starts with ?.