Procedural World in Unity3D The idea here is that rather than placing objects directly into the scene, these building objects are instantiated at run time and placed in the correct locations.  This gives me full control over where and how to place the buildings.

The workflow is still a little involved. I have to import the model and texture image, drag it out into the scene, scale it, add a Mesh collider to it, and then add all of that to a prefab object that I also need to create. Then I need to add the name of that prefab to the script below. This must be done for each model that I want to use in the game.  This approach is still a whole lot better than having to manually place and position each object in the Unity scene though.

GameModelScript.cs

using UnityEngine;
using System;
using System.Collections;
using System.Collections.Generic;

public class GameModelScript : MonoBehaviour {

    public int worldSize;

    private System.Random random = new System.Random();

    void Start () {
        List<string> names = new List<string> ();
        names.Add ("arco01_2_prefab");
        names.Add ("arco03_prefab");
        names.Add ("arco04_prefab");
        names.Add ("arco05_prefab");
        names.Add ("ballTower01_prefab");
        names.Add ("bank01_prefab");
        names.Add ("barn01_prefab");
        names.Add ("bigApartments05_prefab");
        names.Add ("bigFactory06_prefab");
        names.Add ("bigMatterCube01_prefab");
        names.Add ("busDepot01_prefab");
        names.Add ("casino01_prefab");

        for (int y = 0; y < worldSize; y+=30) {
            for (int x = 0; x < worldSize; x+=30) {
                int randomNumber = random.Next(0, names.Count);
                GameObject tileObject = (GameObject)GameObject.Instantiate (Resources.Load (names[randomNumber]));
                tileObject.transform.position = new Vector3(x, 10, y);
            }
        }
    }

    void Update () {
    }
}

My plan is to use this foundation to build some kind of superhero city exploration game.  Here is a link to a very early playable version: Super Voxel

And here is a little screenshot from up high:

Unlike Java Lists, C# lists can be pre-initialized with values, like this:

List<string> names = new List<string>()
{
    "arco01_2_prefab",
    ...
};

This eliminates the need to call .Add, and improves code readability as well.

In the future as well, if you ever plan to add more buildings, it'll be a pain to edit this script to include the new building names. Instead, you could declare a public variable which allows you to drag and drop the model prefabs instead:

public List<GameObject> buildings;

This also eliminates the need to call Resources.Load.

In addition, the namespace UnityEngine provides a class Random which can be used over the default System.Random. Unlike System.Random, UnityEngine.Random is a static class doesn't need to be instantiated, and you can just call Random.Range like this:

int randomNumber = (int)Random.Range(0, names.Count);

Other than that, there are just a few things I want to nitpick.

You don't need to call Instantiate from the GameObject class. It can just be called with out the GameObject. prefix.
In addition, the accepted Unity style for storing an instantiated GameObject is like this:

GameObject gameObject = Instantiate( ... ) as GameObject;

If you're feeling up to dynamic typing, this also works as well (Thanks @Kroltan):

var gameObject = Instantiate( ... ) as GameObject;

You can also remove the Update function. It doesn't do anything, and can decrease performance, due to the fact that it's running many times per frame.
The general style for braces in C# is to put them on the next line, not like Java braces.
You aren't using System.Collections, so you can remove the using System.Collections;.


Universal File Duplicator I made a simple Universal-File-Duplicator (Example: make 125 duplicates of one file).
Very useful if you want to fill a whole USB flash drive or an old harddisk with an important file (Example: Bitcoin wallet.dat or privatekey) and you don't want to hit Ctrl+V all the time...

Suggestions? Improvements?

#include "stdafx.h"
#include "stdio.h"
#include <stdlib.h>
#include <strstream>
#include <string>
#include <iostream>
#include <fstream>
#include <conio.h>
using namespace std;

int main(int argc, char* argv[])
{

    char nummer[1000000], sourcefile[255], targetfile[255], targetfileneu[255], endung[255];
    int i = 0;
    int anzahl = 0;
    ifstream Quelldatei;

    printf("Welcome! You are using UNIVERSAL-DATEI-DUPLIKATOR v1.0\n");
    printf("Filename? (Example: test.txt)\n");
    gets(sourcefile);
    printf("New Filename without ending? (Example: filename)\n");
    gets(targetfile);
    printf("Ending? (Example: .txt)\n");
    gets(endung);

    printf("How many times do you want to create the file?\n");
    scanf("%d", &anzahl);

    for (i = 0; i < anzahl; i++)
    {
        sprintf(nummer, "%ld", i);
        strcpy(targetfileneu, targetfile);
        strcat(targetfileneu, nummer);
        strcat(targetfileneu, endung);
        printf("Targetfilename: %s\n", targetfileneu);

        Quelldatei.open(sourcefile, ios::binary);
        if (!Quelldatei)
        {
            cerr << "ERROR!\n";
            return 0;
        }
        ofstream Zieldatei(targetfileneu, ios::binary);
        if (!Zieldatei)
        {
            cerr << "ERROR!\n";
                    return 0;
        }
        else
        {
            int c;
        while ((c = Quelldatei.get()) >= 0) { Zieldatei.put(c); }
        }
        memset(&targetfileneu[0], 0, sizeof(targetfileneu));
        Quelldatei.close();
        Zieldatei.close();
    }
    printf("SUCCESS!\n");
    system("pause");
    return 0;
}

You've got a using namespace std; in your code. Don't do that. (No, really).

gets is dangerous; don't use it. Streams are so much more convenient anyway! For instance, you could write:

std::string sourcefile, targetfile, endung;
std::cout << "Welcome! You are using UNIVERSAL-DATEI-DUPLIKATOR v1.0\n"
          << "Filename? (Example: test.txt)\n";
std::cin >> sourcefile;
std::cout << "New Filename without ending? (Example: filename)\n";
std::cin >> targetfile;
std::cout << "Ending? (Example: .txt)\n";
std::cin >> endung;

int anzahl;
std::cout << "How many times do you want to create the file?\n";
std::cin >> anzahl;

Opening the input file only once and resetting the stream to the beginning for each copy may allow the stream to make better use of buffering, reducing the number of seeks required.

Also, streams can be copied directly to streambufs using operator>>.

std::ifstream Quelldatei(sourcefile, std::ios::binary);
for (int i = 0; i < anzahl; ++i) {
    Quelldatei.seekg(0);
    std::ostringstream output_filename(targetfile);
    output_filename << i << endung;
    std::ofstream Zieldatei(output_filename, std::ios::binary);
    Quelldatei >> Zieldatei.rdbuf();
    Zieldatei.close();
}
std::cout << "SUCCESS!" << std::endl;
system("pause");


Unit tests: to cast or not to cast? Suppose the following example unit test for an ASP.NET MVC controller:

[Test]
public void Delete_Will_Redirect_To_Index_1()
{
    bookRepository.Add(someBook);
    var result = booksController.Delete(someBook.Id) as RedirectToRouteResult;
    Assert.That(result, Is.Not.Null);
    Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}

Resharper unwittingly complains on the fourth line of code that result.RouteValues may cause a possible NullReferenceException. I hate those squiggly lines, so I figured I could just as easily write it as follows:

[Test]
public void Delete_Will_Redirect_To_Index_2()
{
    bookRepository.Add(someBook);
    var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
    Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}

This has some advantages:

One LoC less;
More to the point;

It also has one main disadvantage:

Code will fail with a cast exception if the result is of unexpected type. This seems(?) less clear than a failed assert.

Some other alternatives I've considered include :

[Test]
public void Delete_Will_Redirect_To_Index_3()
{
    bookRepository.Add(someBook);
    var result = booksController.Delete(someBook.Id);
    Assert.That(result, Is.InstanceOf<RedirectToRouteResult>());
    var redirectResult = (RedirectToRouteResult)result;
    Assert.That(redirectResult.RouteValues["Action"], Is.EqualTo("Index"));
}

This is five lines of code, but has two very clear asserts.

I've even considered this version:

[Test]
public void Delete_Will_Redirect()
{
    bookRepository.Add(someBook);
    var result = booksController.Delete(someBook.Id);
    Assert.That(result, Is.InstanceOf<RedirectToRouteResult>());
}

[Test]
public void Delete_Will_Redirect_To_Index_4()
{
    bookRepository.Add(someBook);
    var redirectResult = (RedirectToRouteResult)booksController.Delete(someBook.Id);
    Assert.That(redirectResult.RouteValues["Action"], Is.EqualTo("Index"));
}

All in all I see the following points in play to judge the various forms:

number of lines of code;
readability of the code;
clarity of failing-unit-test feedback;
how pragmatic and painless it is to write a test;

Did I miss any important points? Did I miss a scenario that has the best of all points? What can I do to get the best of all scenarios?

PS. I'd prefer a framework-agnostic answer, but if it matters, I'm using C# NUnit tests for an ASP.NET MVC5 controller.

You could make a strong argument that you need 2 tests.

General theory has a unit test (as opposed to an integration test) containing only one assert, see e.g. Programmers.StackExchange

So you probably want:

[Test]
public void Delete_WithValidBook_ReturnsRedirectToRouteResult()
{
    bookRepository.Add(someBook);
    var result = booksController.Delete(someBook.Id);
    Assert.That(result.Type, Is.EqualTo(typeof(RedirectToRouteResult)));
}

[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
    bookRepository.Add(someBook);
    var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
    Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}

This has the added advantage that it is much easier to find the bug later based on the name of the failed test alone.

For example, if your type test fails, presumably your redirect test will also fail, but I know which I'd look at first during debugging!

If, on the other hand, the Delete method is guaranteed (by the framework) to return a RedirectToRouteResult, then you only want one test:

[Test]
public void Delete_WithValidBook_RedirectsToValidIndex()
{
    bookRepository.Add(someBook);
    var result = (RedirectToRouteResult)booksController.Delete(someBook.Id);
    Assert.That(result.RouteValues["Action"], Is.EqualTo("Index"));
}

If you have multiple tests, or a guard assertion, the reader (i.e. you at some point in the future) will assume that it must be possible to get, say, a null result, which will mean you may feel the need to add null checks, etc, to other parts of the code that may not be required.


PHP checking for array key or using default value I currently use this code pattern fairly frequently

if(array_key_exists('possible_key', $array)) {
    $my_variable = $array['possible_key'];
} else {
    $my_variable = 'my default value';
}

Is there a better way to be writing this?

One quick change you can do is making use of ternary operator instead of writing if else block.

$my_variable = array_key_exists('possible_key', $array)
               ? $array['possible_key']
               : 'my default value';


Connect 4 refine the diagonal check I am creating a connect 4 game in python and I have managed to write the code so that it runs. However I have a lot of duplicated data in it and was just wondering if anyone could help me to refine it to minimise this?

Here is my code so far:

def CreateBoard(r, c, b, n):

  for i in range(1,(r+1)):
    n.append(str(i))
  b.append(n)

  for i in range(c):
    b.append(['*']*(r))
  return(b)

"""The Following PrintBoard function takes the parameter board(b), prints
the board and then returns the board."""

def printBoard(b):
  for lst in b:
    print(" ".join(lst))
  return b

"""The following Check function takes the parameter board. Within this  """
def check(board, n):
  n = []
  for i in range(1,len(board[1])+1):
    if board[1][i-1] == '*':
      n.append(i)
  print(n)

  user = (input('Enter column: '))

  if(user.isdigit() == True):
    user = int(user)
    if (user in n):
      return(user)
  print('Invalid input')
  return check(board, n)

def WinningCon(b, r, u, c):
  loop1 = True
  rowCon = ""
  colCon = ""
  for i in range(0,len(b[1])):
    rowCon += b[r][i]
    if('X'*4 in rowCon) or('O'*4 in rowCon):
      return(True)
  for i in range(1,c+1):
    colCon += b[i][u-1]
    if('X'*4 in colCon) or('O'*4 in colCon):
      return(True)

def Diag2(u, r, b, column, row):
  utemp1 = u-1
  utemp2 = u-1
  rtemp1 = r
  rtemp2 = r
  end = ""
  beg = ""
  while(True):

    beg += b[rtemp1][utemp1]
    rtemp1 -= 1
    utemp1 += 1
    if(rtemp1 == 1):
      break
    elif(utemp1 >= column - 1):
      break
  while(True):
    end += b[rtemp2][utemp2]
    rtemp2 +=1
    utemp2 -=1
    if(rtemp2 >= row):
      break
    elif(utemp2 == 0):
      break
  beg = beg[::-1]
  fullstring = beg+end
  if('X'*4 in fullstring) or('O'*4 in fullstring):
    return(True)

def Diag1(u, r, b, column, row):
  utemp1 = u-1
  utemp2 = u-1
  rtemp1 = r
  rtemp2 = r
  end = ""
  beg = ""
  while(True):
    beg += b[rtemp1][utemp1]
    rtemp1 -= 1
    utemp1 -= 1
    if(rtemp1 == 1):
      break
    elif(utemp1 == 0):
      break
  while(True):
    end += b[rtemp2][utemp2]
    rtemp2 +=1
    utemp2 +=1
    if(rtemp2 >= row):
      break
    elif(utemp2 >= column-1):
      break
  beg = beg[::-1]
  fullstring = beg+end
  if('X'*4 in fullstring) or('O'*4 in fullstring):
    return(True)

def ProcessInput(u, s, b, r):
  rowNum = r
  u = u-1
  while(not b[rowNum][u] == '*'):
    rowNum -= 1
  b[rowNum][u] = s
  return(rowNum)

def EndGame(g, b, p):
  printBoard(b)
  print("Congrats %s, you've won!" %p)
  replayGame = input('Would you like to play again? Yes or No?\n').lower()
  if replayGame == 'yes':
    MainGame()
  else:
    g = False
    return(g)

def MainGame():
  column = int(input('Enter number of columns'))
  row = int(input('Enter number of rows'))
  board = []
  nums = []
  board = CreateBoard(column, row, board, nums)
  player1 = 'K'#input('Enter name: ')
  player2 = 'A'#input('Enter name: ')
  turn = 1
  GameOn = True
  while(GameOn):
    board = printBoard(board)
    user = check(board, nums)
    if(turn%2 != 0):
      print("It's %s's turn" %player1)
      rc = ProcessInput(user, "X", board, row)
    else:
      print("It's %s's turn" %player2)
      rc = ProcessInput(user, "O", board, row)

    if(WinningCon(board, rc , user, row) == True):
      GameOn = EndGame(GameOn, board, player1)
    elif(Diag1(user, rc, board, column, row) == True):
      GameOn = EndGame(GameOn, board, player1)
    elif(Diag2(user, rc, board, column, row) == True):
      GameOn = EndGame(GameOn, board, player1)

    turn +=1

MainGame()

Severe code readability issues

Honestly, your code is extremely difficult to follow and I gave up. The most important part about code is readability. Readable code is easier to reason about, easier to maintain, easier to debug, and, especially relevant here, easier to get other people's opinions on. I would very much like to give you advice on how to improve your functions - but I can't. I have no idea what they're doing now. They're simply inscrutable.

It also isn't enough that you understand what this code does right now. What if you need to go back to this code in a week? In 3 months? In a year? Will you remember what it does then? Even well-commented code with good names sometimes becomes hard to understand once you forget the context of when it was written. Poorly named, uncommented code is just setting yourself up for lots of confusion and frustration. At best.

I cannot stress this enough. Read up on the python style guide, especially for its conventions for naming and comments.

As far as examples of the above. We have:

def CreateBoard(r, c, b, n):

What are r, c, b, and n? I can't even tell from the body of the function. Avoid single-letter variable names unless used in a context where they are truly trivial (e.g. iterating over a range with i or _ is fine).

Function names should be snake_case, not CamelCase, and should describe what the function actually does. Certainly Diag1 and Diag2 are not helpful at all. Which one checks which kind of diagonal? What arguments do they take? What do they return?

docstrings belong inside the function:

def print_board(b):
    """This function takes the parameter board(b), prints
    the board and then returns the board."""

When you need a comment to describe what your variable name names, that's a good indicator that you should just change your variable name. Once you do that, that doc string becomes pointless:

def print_board(board):
    ...

It's pretty clear that a function called print_board that takes a board will print it. And if it doesn't do that, then it should be renamed. somewhere.

The meaning of this docstring:

"""The following Check function takes the parameter board. Within this  """
def check(board, n):

eludes me completely. Also check suggests that we're checking something, but check is doing all sorts of things, involving taking user input. It also takes an argument n which it immediately shadows internally, and it's unclear if that's intentional or not.

WinningCon either explicitly returns True or implicitly returns None - it gives no indication as to who won. It has a variable (loop1) that isn't used.

I would strongly, strongly suggest rewriting your code with PEP-8 in mind, providing better variable names and comments, and using 4-space indentation throughout. Once you do that, you can repost and I will hopefully be able to provide more specific comments as to the actual functionality.


Elementwise perfect forwarding of a member range Perfect forwarding is usually done with std::forward. E.g.,

template<typename T>
void foo( T&& t ) {
    bar(std::forward<T>(t));
}

for some function bar(). Sometimes, only a member of t, say t.x, needs to be forwarded. This is also a solved problem

template<typename T>
void foo( T&& t ) {
    bar(std::forward<T>(t).x);
}

...But what if t has a range member, say t.range, and bar takes an element from said range? E.g.,

template<typename T>
void foo( T&& t ) {
    for (auto& r : t.range)
        bar(???(r));
}

Here, ??? is a placeholder for a cast which forwards the type of r as the type of t itself would be forwarded.

Note that simply writing

template<typename T>
void foo( T&& t ) {
    for (auto&& r : std::forward<T>(t).range)
        bar(r);
}

won't work as r is still named and will always pass by value or l-value reference but never by r-value reference. To solve this I introduce forward_as<T>

////////////////////////////////////////////////////////////////////////////////
/// Forward As
///
/// Forwards u of type U as T would be forwarded. I.e., u is forwarded
/// like some t would be forwarded by std::forward<T>(t). The cv-qualifiers of
/// U are preserved. E.g., forward_as<T, std::string>(u) casts u to:
///
///  (1) std::string& if T is int&
///  (2) std::string& if T is const int&
///  (3) std::string&& if T is int&&
///
/// Analogously, forward_as<T, const std::string>(u) casts u to:
///
///  (1) const std::string& if T is int&
///  (2) const std::string& if T is const int&
///  (3) const std::string&& if T is int&&
///
////////////////////////////////////////////////////////////////////////////////
template<typename T, typename U>
auto forward_as( U&& u ) -> std::conditional_t<
        std::is_reference<T>::value,
        std::remove_reference_t<U>&,
        std::remove_reference_t<U>&&>
{
    return static_cast<std::conditional_t<
        std::is_reference<T>::value,
        std::remove_reference_t<U>&,
        std::remove_reference_t<U>&&>>(u);
}

Note that I use auto with a trailing return type. This is because my compiler is old. It should work just fine with decltype(auto) on a C++14 compliant compiler. Also note that I've chosen to preserve cv-qualifiers of the given type.

Now the problem from before can be solved by

template<typename T>
void foo( T&& t ) {
    for (auto& r : t.range)
        bar(forward_as<T>(r));
}

Direct member forwarding is also possible using forward_as<T>(t.x).

Have I missed any edge cases? Is there an alternative to all of this? Any suggestions for interface/naming/style improvements?

Edit: I've made a code sample. The syntax is slightly more verbose since the ideone compiler only supports C++11. The underlying principle is the same.

I'll start with some direct review of what you've implemented. After the bullets, however, I go into some concerns about whether this should be used. Thanks for the interesting idea; reviewing it made me re-examine several things I thought I already knew.

Like std::forward you should consider making forward_as a constexpr function.
You could avoid some of the pre-C++14 verbosity by using decltype inside forward_as (although the auto return type sounds better still):

    return static_cast<decltype(forward_as<T, U>(u))>(u);

How do you know whether for (auto&& r : t.range) will iterate over objects owned by t?
Admittedly a bikeshed point, I don't like the way the name forward_as<T>(u) reads; perhaps forward_if<T>(u) or even move_if_rvalue<T>(u) would help this not look like it returns an expression of a T-based type.

However like Kerrek SB's comment on the referenced question, I worry about the state of the main object when forwarding or moving its members. What are the rules for doing this safely? Your examples seem to show forwarding only a single member, but what are the rules when forwarding different parts of the same object? What are the code review guidelines to ensure that problems are not introduced in later edits?

To sum up, I mistrust this. It might just be my own lack of familiarity with such a use case. But it also reflects an uncertainty that optimizing for move is likely to be worth it in these cases. If this is actually a bottleneck or potential bottleneck in the code, are there better alternatives? Does bar even overload on r-value types? Should foo receive t.range instead of t, or will std::for_each be able to generate r-values automatically:

    std::for_each(std::begin(t.range), std::end(t.range), bar)

(If so, I haven't figured out the syntax to support it in your sample with bar as a function template. Using instead a struct with overloaded operator(), it appears to suggest we shouldn't assume the range contains r-values.)


Tortoise and hare cycle detection algorithm using iterators in Python Reading the article in Wikipedia about cycle detection I came across a relatively small code snipped of Floyd's cycle-finding algorithm. I'll put it here to keep everything in one place:

def floyd(f, x0):
    # The main phase of the algorithm, finding a repetition x_mu = x_2mu
    # The hare moves twice as quickly as the tortoise
    tortoise = f(x0) # f(x0) is the element/node next to x0.
    hare = f(f(x0))
    while tortoise != hare:
        tortoise = f(tortoise)
        hare = f(f(hare))

    # at this point the start of the loop is equi-distant from current tortoise
    # position and x0, so hare moving in circle and tortoise (set to x0 )
    # moving towards circle, will intersect at the beginning of the circle.

    # Find the position of the first repetition of length mu
    # The hare and tortoise move at the same speeds
    mu = 0
    tortoise = x0
    while tortoise != hare:
        tortoise = f(tortoise)
        hare = f(hare)
        mu += 1

    # Find the length of the shortest cycle starting from x_mu
    # The hare moves while the tortoise stays still
    lam = 1
    hare = f(tortoise)
    while tortoise != hare:
        hare = f(hare)
        lam += 1

    return lam, mu

But then I decided, why not rewrite this algorithm using iterators.

The result wasn't as good as I had hoped it to be.

In some parts of the code I needed the values that the iterators have already returned. So I had keep the values apart from iterators themselves. And I introduced the class animal to keep them in one place.

To my mind, the code got very hefty and ugly. Here it is:

# -*- coding: utf-8 -*-

from __future__ import print_function
from itertools import islice,cycle,chain

def detect_cycle(iterable):

    class animal(object):
        '''Holds the iterator and keeps the last value returned by the iterator'''
        def __init__(self,iterable):
            self.it = iter(iterable)
            self.value = next(self.it) #get the first value

        def next(self):
            self.value = next(self.it)
            return self.value

        def __iter__(self):
            return self

    tortoise = animal(iterable)
    hare = animal(islice(iter(iterable),None,None,2))  #two steps at a time

    while True:
        if next(tortoise) == next(hare):
            break

    hare = tortoise #put hare in the place of tortoise
    tortoise = animal(iterable) #start tortoise from the very beginning
    mu = 0
    while True:
        if hare.value == tortoise.value:
            break
        next(hare)
        next(tortoise)
        mu += 1

    lamb = 0
    tortoise_val = tortoise.value #save the value of tortoise as the object is reassigned to hare
    hare = tortoise
    while True:
        next(hare)
        lamb += 1
        if hare.value == tortoise_val: #check AFTER the first step by hare
            break

    print('mu: {}'.format(mu))
    print('lamb: {}'.format(lamb))

if __name__ == '__main__':

    class iterable(object):
        '''Emulate the object returning iterators having cycles'''
        def __init__(self,beginning,cycling):
            '''
beginning: the beginning part with no cycles (its length corresponds to mu)
cycling: the part which will be cycling (its length corresponds to lamb)
'''
            self.beginning = beginning
            self.cycling = cycling

        def __iter__(self):
            return chain(iter(self.beginning),cycle(iter(self.cycling)))

    detect_cycle(iterable('1234','678'))

Is there anything that can be done to improve the code?
Is it the best that can be done using iterators?

# -*- coding: utf-8 -*-

from __future__ import print_function
from itertools import islice,cycle,chain

def detect_cycle(iterable):

    class animal(object):

Putting classes inside function is usually not all that helpful. Also, the name doesn't really suggest what this class does.

        '''Holds the iterator and keeps the last value returned by the iterator'''
        def __init__(self,iterable):
            self.it = iter(iterable)

I'd do something like iter, just because it isn't a really obvious abbreviation

            self.value = next(self.it) #get the first value

        def next(self):
            self.value = next(self.it)
            return self.value

        def __iter__(self):
            return self

    tortoise = animal(iterable)
    hare = animal(islice(iter(iterable),None,None,2))  #two steps at a time

You end up create two iterators from the same iterable. That might be problematic as you can't be sure those two iterators are independent.

    while True:
        if next(tortoise) == next(hare):
            break

You get most of the benefit of iterators when you can use for loops. In this case, I might use a for loop with itertools.izip.

    hare = tortoise #put hare in the place of tortoise
    tortoise = animal(iterable) #start tortoise from the very beginning
    mu = 0
    while True:
        if hare.value == tortoise.value:
            break
        next(hare)
        next(tortoise)
        mu += 1

You do this basic thing three times. You loop through both iterators, until they match. Write a function that takes the two iterators, does that and then returns the count of them.

    lamb = 0
    tortoise_val = tortoise.value #save the value of tortoise as the object is reassigned to hare
    hare = tortoise
    while True:
        next(hare)
        lamb += 1
        if hare.value == tortoise_val: #check AFTER the first step by hare
            break

    print('mu: {}'.format(mu))
    print('lamb: {}'.format(lamb))


C++ reduce repetitions of code for similar classes I have the following two classes to make appending strings - and wstrings respectively - easier:

#include <string>
#include <sstream>

class StringBuilder
{
public:
    template <typename T>
    StringBuilder& operator<<(const T& t)
    {
        this->stream << t;
        return *this;
    }

    operator std::string() const;

private:
    std::ostringstream stream;
};

StringBuilder::operator std::string() const
{
    return this->stream.str();
}

class WStringBuilder
{
public:
    template <typename T>
    WStringBuilder& operator<<(const T& t)
    {
        this->stream << t;
        return *this;
    }

    explicit operator std::wstring() const;

private:
    std::wstringstream stream;
};

WStringBuilder::operator std::wstring() const
{
    return this->stream.str();
}

Now, as you see, pretty much all of the code (except for the concrete string and stringstream types) is repeated, especially the operators.

The question is:

Is there any way to reduce the duplicated code? I thought about a base class, templates and specialization, but the problem is I don't know how to determine the correct stringstream type depending on the string type then.

Yes, there is. In fact, a very nice way to solve this code redundancy issue comes to mind if you consider that std::string is a just type alias for std::basic_string<char> and std::wstring an alias for std::basic_string<wchar_t>. Likewise, std::stringstream and std::wstringstream are also just aliases for std::basic_stringstream<char> and std::basic_stringstream<wchar_t>, respectively. This allows for an easy template-based approach to the problem.

What really suggests itself to me is keeping this alias-of-template-base-type pattern from the STL (although there may be other valid approaches as well), an implementation of which could look something like this:

#include <sstream>
#include <string>

namespace impl {

    template <typename CharType>
    class StringBuilderImpl {

        std::basic_stringstream<CharType> stream;

        public:
        template <typename T>
        StringBuilderImpl& operator<<(T const& t) {
            stream << t;
            return *this;
        }

        operator std::basic_string<CharType>() const {
            return stream.str();
        }
    };
}

using StringBuilder = impl::StringBuilderImpl<char>;
using WStringBuilder = impl::StringBuilderImpl<wchar_t>;

StringBuilderImpl corresponds somewhat to std::basic_[string/stringstream] and takes the underlying character type as a template argument. Your former classes StringBuilder and WstringBuilder are now only aliases for certain template specializations, just as std::[string/wstring] and std::[stringstream/wstringstream] are.

(In my code, StringBuilderImpl is kind of hidden away inside an impl namespace. If it serves your need, however, you could also make it part of the public interface of your program library, allowing further specifications such as StringBuilderImpl<char16_t>)


Functional linked list I've been implementing functional linked lists in different languages, just as a learning exercise. I'm looking for feedback in general: code correctness, best practices, design patterns; everything you think about this. Is it bad code? Where can it be improved?

/**
*
* Constructing Linked Lists
*
* Our linked list data structure consists of two fundamental building blocks: Nil and cons. Nil represents the
* empty list and serves as a sentinel for more complex lists. The cons operation extends a list at the front by
* inserting a new value.
*
* The lists we construct using this method consist internally of nested arrays of 2 items. For example, the list [1, 2, 3]
* is represented by the expression cons(1, cons(2, cons(3, Nil))) which evaluates to the nested arrays [1, [2, [3, Nil]]].
*
*/

var Nil = "Nil";

/** Extends a list at the front by inserting a new value. */
function cons(head, tail) {
    tail = typeof tail === "undefined" ? Nil : tail;
    return [head, tail];
}

/**
* Define list instances using a more convenient syntax and without deeply nested cons calls.
* lst() === Nil
* lst(1, 2, 3) === [1, [2, [3, Nil]]]
*/
function lst() {
    var arg_array = Array.prototype.slice.call(arguments);  // arguments cast from object to array
    if (arg_array.length === 0) return Nil;
    else return cons(arg_array[0], lst.apply(this, arg_array.slice(1)));
}

/** Returns the first element of a list */
function head(xs) {
    return xs[0];
}

/** Returns a list containing all elements except the first. */
function tail(xs) {
    return xs[1];
}

/** Returns True if the list contains zero elements. */
function isEmpty(xs) {
    return xs === Nil;
}

/** Returns number of elements in a given list. */
function length(xs) {
    if (isEmpty(xs)) return 0;
    else return 1 + length(tail(xs));
}

/** Concatenates two lists. */
function concat(xs, ys) {
    if (isEmpty(xs)) return ys;
    else return cons(head(xs), concat(tail(xs), ys));
}

/** Returns the last element of a non-empty list. */
function last(xs) {
    if (isEmpty(tail(xs))) return head(xs);
    else return last(tail(xs));
}

/** Returns all elements except the last one. */
function init(xs) {
    if (isEmpty(tail(tail(xs)))) return cons(head(xs));
    else return cons(head(xs), init(tail(xs)));
}

/** Returns the input list, reversed. */
function reverse(xs) {
    if (isEmpty(xs)) return xs;
    else return concat(reverse(tail(xs)), cons(head(xs), Nil));
}

/** Returns the first n elements of the given list. */
function take(n, xs){
    if (n < 1) return Nil;
    else return cons(head(xs), take(n - 1, tail(xs)));
}

/** Returns a copy of input list, without the first n elements. */
function drop(n, xs){
    if (n < 1) return xs;
    else return drop(n - 1, tail(xs));
}

/**  Returns a subset of the input list that includes the items in the range from, to. */
function piece(from, to, xs){
    return take(to - from, drop(from, xs));
}

/** Removes the element of the list at given index */
function removeOne(index, xs){
    return concat(take(index - 1, xs), drop(index, xs));
}

Your code runs into a lot of problems because it chooses a weird value for Nil, and because it fails to check for that value. What is the correct result for head(lst())? "N", Nil, or a type error?

Because "Nil" === "Nil" holds (value type), but [] === [] does not (reference type), I suggest you use the empty array to represent Nil (this is also closer to Lisp semantics). The advantage of this is that your cons can then contain any value in the tail position – the string "Nil" was previously forbidden.

Another good possibility would be to use a false value, this would even remove the need for isEmpty! I will assume var Nil = undefined for the rest of this answer, even though now undefined and Nil cannot be distinguished.

As already mentioned, you fail to check for Nil, therefore your code can be considered incorrect. E.g. head could be:

function head(xs) {
  if (!xs) return Nil;
  return xs[0];
}

or even

function head(xs) {
  if (!xs || !(xs instanceof Array)) {
    throw "Expected pair";
  }
  return xs[0];
}

Other functions that build on these primitives are in turn correct, although the new value for Nil simplifies the code a bit, e.g.

function length(xs) {
    if (!xs) return 0;
    return 1 + length(tail(xs));
}

Notice that I removed the unnecessary else, but this is a matter of personal style.

You may want to specify what exactly the output of take(3, Nil) and similar functions should be. It seems you are assuming that all indices will always be inside the correct bounds. Better be explicit about that. In this case, valid interpretations of var n = 3; take(n, Nil) would be to:

Use the n as an upper limit, and return the empty list/Nil.
Use n as a strict limit, and fill up the surplus cons with Nil in the head.
Throw an error that the index is out of bounds.

The first solution is most DWIM, it would seem to me.

Your piece function does not document if the from and to bounds are inclusive or exclusive. Because you will drop(from, xs), the from index is not included in the output. The to is exclusive as well. This counterintuitive behaviour is not made clear in the documentation!

Your removeOne is even weirder by using one-based indexing instead of zero-based indexing, like piece does.

Interestingly, you don't have an indexing function, e.g. at(1, lst(0, 1, 2)) === 1.

Summary

Your code looks fine, but you haven't carefully considered (or tested) the actual behaviour of your functions. When writing complex code, a few well-chosen test cases can help detect errors early.


Browser resize service In AngularJS, I currently have a directive and 2 services which permit me to bind the window resize event, and broadcast it to my application.

I would like to know if you could see anything that could be improved in my code, in term of optimization, naming or possible cross browser issues.

I have:

A directive which binds resize event. It will broadcast the event 300ms after the last resize (to avoid flood).
A factory to get window's dimensions
A service to detect browser's name (chrome, safari, etc...). In this case, it is used to target Safari which doesn't calculate window's dimensions the same way.

Here is a plunkr to illustrate the code.

Detect Browser Service

/*
 * Detects on which browser the user is navigating
 *
 * Usage:
 * var browser = detectBrowser();
 *
 */
commonApp.service('detectBrowser', ['$window',
    function( $window ) {

        // http://stackoverflow.com/questions/22947535/how-to-detect-browser-using-angular
        return function() {
            var userAgent = $window.navigator.userAgent,
                browsers  = {
                    chrome  : /chrome/i,
                    safari  : /safari/i,
                    firefox : /firefox/i,
                    ie      : /internet explorer/i
                };

            for ( var key in browsers ) {
                if ( browsers[key].test(userAgent) ) {
                    return key;
                }
            }

            return 'unknown';
        }
    }]);

Window Dimensions Factory

/*
 * Get window height and width
 *
 * Usage:
 * windowDimensions.height();
 * windowDimensions.width();
 *
 */
commonApp.factory('windowDimensions', ['$window', 'detectBrowser',
    function( $window, detectBrowser ) {
        var browser = detectBrowser();

        return {
            height: function() {
                return ( browser === 'safari' )
                          ? document.documentElement.clientHeight
                          : window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight
            },

            width : function() {
                return ( browser === 'safari' )
                          ? document.documentElement.clientWidth
                          : window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth
            }
        }
    }]);

Window Resize Event Directive

/*
 * Watch window resizing event to set new window dimensions,
 * and broadcast the event to the app
 *
 * Usage:
 * <html watch-window-resize>...</html>
 *
 * Bind the resize event:
   $scope.$on('watchWindowResize::resize', function() {
       // Do something
   });
 *
 */
app.directive('watchWindowResize', ['$window', '$timeout', 'windowDimensions',
    function( $window, $timeout, windowDimensions ) {

        return {
            link: function( $scope ) {
                // Get window's dimensions
                $scope.getDimensions = function() {

                    // Namespacing events with name of directive + event to avoid collisions
                    // http://stackoverflow.com/questions/23272169/what-is-the-best-way-to-bind-to-a-global-event-in-a-angularjs-directive
                    $scope.$broadcast('watchWindowResize::resize', {
                        height: windowDimensions.height(),
                        width : windowDimensions.width()
                    });
                }

                // On window resize...
                angular.element($window).on('resize', function( e ) {

                    // Reset timeout
                    $timeout.cancel($scope.resizing);

                    // Add a timeout to not call the resizing function every pixel
                    $scope.resizing = $timeout( function() {

                        $scope.getDimensions();
                    }, 300);
                });
            }
        }
    }]);

Fun question,

I am not sure why you think you need a special treatment for Safari, I could not find any reference to that on the webs. So if you only need the browser code for that part, then I would drop it and simply go for

/*
 * Get window height and width
 *
 * Usage:
 * windowDimensions.height();
 * windowDimensions.width();
 *
 */
app.factory('windowDimensions', ['$window',
  function($window) {
    return {
      height: function() {
        return window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight;
      },
      width: function() {
        return window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
      }
    };
  }
]);

From a naming perspective I would suggest these changes:

getDimensions -> broadcastDimensions (since that is really what you are doing there)
$scope.resizing -> $scope.resizingTimer (to reduce potential confusion, resizing is a verb so one would expect resizing to do something instead of merely being a timer)
300 -> var delayTime = 300; //Broadcast 300ms after the last resize to avoid flood

Other than that, all is well besides a few missing semicolons.
You can test my fork on Safari if you wish, it worked for me:
http://plnkr.co/edit/SY8pnX3ae4YtWErno5f0?p=preview


Uniquely digitized 5-digit multiples This is a programming problem:

  Write a program that finds and displays all pairs of 5-digit numbers
  that between them use the digits 0 through 9 once each, such that the
  first number divided by the second is equal to an integer N so that:

abcde / fghij = N

  where each letter represents a different digit. The first digit of one
  of the numerals is allowed to be zero.

  Input

  Each line of the input file consists of a valid integer N. An input of
  zero is to terminate the program.

  Output

  Your program have to display ALL qualifying pairs of numerals, sorted
  by increasing numerator (and, of course, denominator). Your output
  should be in the following general form:

xxxxx / xxxxx = N

xxxxx / xxxxx = N

  In case there are no pairs of numerals satisfying the condition, you
  must write "There are no solutions for N.". Separate the output for
  two different values of N by a blank line.

This keeps giving me TLE. How can I speed it up?

#include <iostream>
#include <algorithm>

using namespace std;

int i[10] = {0,1,2,3,4,5,6,7,8,9};

int check(int helper, int n)
{
    int a[5] = {i[5],i[6],i[7],i[8],i[9]};
    sort(a,a+5);
    int p = 0;
    int comp = helper*n;
    while (p < 120)
    {
        next_permutation(a,a+5);
        if(comp == a[0]*10000 + a[1]*1000 + a[2]*100 + a[3]*10 + a[4])
            return 1;
        p++;
    }
    return 0;
}

int main()
{
    int n;
    int helper = 0;
    int boo = 0;
    string str;
    int j = 0;
    int a1,a2,a3,a4,a5;
    while (cin >> n)
    {
        while(j < 10)
        {
            i[j] = j;
            j++;
        }
        do
        {
            a1 = i[0];
            a2 = i[1];
            a3 = i[2];
            a4 = i[3];
            a5 = i[4];
            while (a1 == i[0] && a2 == i[1] && a3 == i[2] && a4 == i[3] && a5 == i[4])
                next_permutation(i,i+10);
            helper = i[0]*10000 + i[1]*1000 + i[2]*100 + i[3]*10 + i[4];
            if(check(helper,n))
            {
                cout << helper * n << "/" << i[0] << i[1] << i[2] << i[3] << i[4] << endl;
                boo++;
            }
        }while (helper * n <= 100000);
        if (boo == 0)
            cout << "There are no solutions for " << n << "." << endl;
        boo = 0;
    }
    return 0;
}

C++ is not my normal haunt,  but there are a few significant observations on an algorithmic level that can be made here.

Bugs

The first obvious bug is that your code does not run multiple times. You need to reset the j variable in the outside loop, because the system will otherwise never start again from the beginning.
Your code enters an infinite loop if the user inputs 1. The code is supposed to output the result There are no solutions for 1.. Instead, since all 5-digit numbers are less than 100000 and since helper will never be more than 5 digits, the helper * 1 < 100000 exit condition for the while loop will never fail.

This leads on to a number of code-style problems...

Style

Namespaces:

using namespace std;

Don't do that. It is too easy to have issues with pollution in your namespace.
Variable declarations:

Declare variables in the scope they are used. This would have solved the bug, for the record. The j variable is declared outside it's use-scope. This is how you declare j:

int j = 0;
int a1,a2,a3,a4,a5;
while (cin >> n)
{
    while(j < 10)
    {
        i[j] = j;
        j++;
    }

But this is how it should be declared:

int a1,a2,a3,a4,a5;
while (cin >> n)
{
    int j = 0;
    while(j < 10)
    {
        i[j] = j;
        j++;
    }

Bracing C++ styles generally recommend the brace at line end: e.g. Google C++ style guide Local standards override this, though, so if your local dev team has a standard they follow, that's the one that should be followed.

Algorithm

First, this problem is a 2-part problem. The first is that you have to find all 5-digit numbers that do not have the same digit twice. The second is that you have to find the right multiple of that number that uses the other digits.

Your solution is pretty poor.... you use all combinations of essentially all 10 digits to find the numbers, and you skip many, many combinations. Consider this:

while (a1 == i[0] && a2 == i[1] && a3 == i[2] && a4 == i[3] && a5 == i[4])
    next_permutation(i,i+10);

Here you have taken a copy of the i array, and put it in the a array. Then you calculate all permutations of the remaining 5 digits, until the head-5-digits of the number change. I calculate millions of iterations in that loop that are wasted.

A better solution will use some mathematical properties to limit the problem significantly. For example, the basic formula you have, is:

$$
A \times N = B
$$

where A and B are both 5-digit numbers, and N is an integer. Additionally, none of the digits in A are in B. Using this, we know that:

the largest B can be is 98765
the largest A can be is 98765 / N
the smallest N can be is 2 (N == 1 is impossible because then A == B and we know they cannot have the same digits).

A smarter algorithm will be:

obtain N
find the maximum limit of A (98765 / N)
find all combinations of unique digits less than (or equal to) the A limit.
find the N'th multiple (B) of that digit.
determine whether the digits in B are unique (when combined with A).

Working code example:

#include <iostream>

bool digitsUnique(long value) {
  int count[10] = {};
  // Move 0 digits to the end
  while (value < 10000) {
    value *= 10;
  }
  while (value > 0) {
    if (count[value % 10]++ > 0) {
      return false;
    }
    value /= 10;
  }
  return true;
}

bool combinedUnique(long low, long high) {
  int count[10] = {};
  // Move 0 digits to the end
  while (low < 10000) {
    low *= 10;
  }
  while (high < 10000) {
    high *= 10;
  }
  while (low > 0) {
    if (count[low % 10]++ > 0) {
      return false;
    }
    low /= 10;
  }
  while (high > 0) {
    if (count[high % 10]++ > 0) {
      return false;
    }
    high /= 10;
  }
  return true;
}

void compute(int mult) {
  int count = 0;
  // 10 will require releating digits,
  // and more than 10 requires multiple 0 digits
  if (mult > 1 && mult < 10) {
    long limit = 98765 / mult;
    for (long low = 1234; low <= limit; low++) {
      if (digitsUnique(low)) {
        long high = low * mult;
        if (combinedUnique(low, high)) {
          count++;
          std::cout << high << "/" << low << " = " << mult << "\n";
        }
      }
    }
  }
  if (count == 0) {
    std::cout << "There are no solutions for " << mult << ".\n";
  }
}

int main() {
  int n;
  // std::cout << "Enter Multiple: ";
  while (std::cin >> n) {
    compute(n);
    // std::cout << "Enter Multiple: ";
  }
  return 0;
}

Note, when I run your solution on my computer, and use the input values 2 through 11 like:

time ./nfact < inputs

I get the time result of:

.....
97524/10836
There are no solutions for 10.
There are no solutions for 11.

real    0m0.815s
user    0m0.808s
sys     0m0.008s

When I run my version above, I get:

95823/10647 = 9
97524/10836 = 9
There are no solutions for 10.
There are no solutions for 11.

real    0m0.019s
user    0m0.016s
sys     0m0.000s

Note that it is about 42 times faster.... I like 42.


Create composition instead of inheritance [DataContract(Name = "jobDateTimeFilterRequest", Namespace = Constants.ManagementNamespace)]
public class JobDateTimeFilterRequest : TimeFrame
{
    [DataMember(Name = "jobType")]
    public BackgroundJobType JobType { get; set; }
}

[DataContract(Name = "timeFrame", Namespace = Constants.ManagementNamespace)]
public class TimeFrame
{
    [DataMember(Name = "from")]
    public DateTime From { get; set; }

    [DataMember(Name = "to")]
    public DateTime To { get; set; }
}

I would like to use composition instead of inheritance. To do this I will need to create:

[DataContract(Name = "jobFilterRequest", Namespace = Constants.ManagementNamespace)]
public class JobFilterRequest
{
    [DataMember(Name = "jobType")]
    public BackgroundJobType JobType { get; set; }
}

And JobDateTimeFilterRequest will be like this:

[DataContract(Name = "jobDateTimeFilterRequest", Namespace = Constants.ManagementNamespace)]
public class JobDateTimeFilterRequest
{
    [DataMember(Name = "jobType")]
    public JobFilterRequest JobFilter { get; set; }

    [DataMember(Name = "timeFrame")]
    public TimeFrame TimeFrame{ get; set; }
}

I'm I right? Or it's incorrect understanding of composition?

"... instead of inheritance.." I expected the JobFilterRequest class to encapsulate the TimeFrame object by wrapping the contained TimeFrame object's methods in its own. Thus the client code is not aware of a distinct inner object - the Law of Demeter a.k.a. the least knowledge principle.

That encapsulation implies there is no public getter; and I would prefer a constructor parameter over the public setter.

If we inherit from TimeFrame there is no contained object, so good encapsulation makes composition "feel" the same for the client code. Class types are different of course, but we're still calling the same methods and getting the same behavior.


Constructing a binary tree in java I am constructing a binary tree. Let me know if this is a right way to do it. If not please tell me how to?? I could not find a proper link where constructing a general binary tree has been coded. Everywhere BST is coded.

  3
 / \
1   4
   / \
  2   5

This is the binary tree which i want to make.I should be able to do all the tree traversals.Simple stuff.

public class Binarytreenode
{
    public Binarytreenode left;
    public Binarytreenode right;
    public int data;

    public Binarytreenode(int data)
    {
        this.data=data;
    }

    public void printNode()
    {
        System.out.println(data);
    }

    public static void main(String ar[])
    {
        Binarytreenode root = new Binarytreenode(3);
        Binarytreenode n1 = new Binarytreenode(1);
        Binarytreenode n2 = new Binarytreenode(4);
        Binarytreenode n3 = new Binarytreenode(2);
        Binarytreenode n4 = new Binarytreenode(5);

        root.left = n1;
        root.right = n2;
        root.right.left = n3;
        root.right.right = n4;
    }
}

Your code does represent the tree that you drew.  However, the code doesn't implement the logic for determining where to place nodes within a binary search tree; instead, you hard-coded that logic in your main() function.  A useful implementation of Binarytreenode would let you write something like

BinaryTree t = new BinaryTree();
t.add(3);
t.add(1);
t.add(4);
t.add(2);
t.add(5);

… to produce the same tree structure.

You want to design the code as a class that maintains the tree in a self-consistent state, supporting operations such as .add(int value) and .find(int value).  The member fields left, right, and data should generally not be have public access, which would let other code modify the tree in an invalid way.  It could be acceptable, however, to let data remain public if you also mark it as final.


SQL query C++ code (My first C++ application) I would really welcome some advice, and notes about my code. I am a long time Python user, and some days ago i decided i should try C++. So a couple of days passed, and i wrote my first application which has a practical use. I still find C++ very complicated (my current perspective is over complicated) but i can clearly say it is faster then my same python implementation.

The code:

/*
 * File:   main.cpp
 * Author: gerdos
 *
 * Created on October 10, 2016, 4:23 PM
 */

#include <string>
#include <iostream>
#include <unistd.h>
#include <list>
#include <vector>
#include <algorithm>

#include <mysql_connection.h>
#include <cppconn/driver.h>
#include <cppconn/exception.h>
#include <cppconn/resultset.h>
#include <cppconn/statement.h>
#include <cppconn/resultset_metadata.h>

using namespace sql;
using namespace std;

int main(int argc, char* argv[]){
    int opt;
    vector<string> column_name_list;

    // Shut GetOpt error messages down (return '?'):
    opterr = 0;

    // Retrieve the options:
    while ( (opt = getopt(argc, argv, "F:")) != -1 ) {  // for each option...
        if (opt == 'F'){
            column_name_list.insert(column_name_list.end(), string(optarg));
        }
    }
    string queries;
    for (unsigned i = 1; i < argc; i++){
        if (string(argv[i]) != "-F" && find(column_name_list.begin(), column_name_list.end(), argv[i]) == column_name_list.end()){
            queries += "'" + string(argv[i]) + "',";
        }
    }

    string columns;
    for (unsigned i = 0; i < column_name_list.size(); i++){
        columns += column_name_list[i] + ",";
    }
    columns.erase(columns.size()-1);
    queries.erase(queries.size()-1);

    try{
        sql::Driver *driver;
        sql::Connection *con;
        sql::Statement *stmt;
        sql::ResultSet *res;

        driver = get_driver_instance();

        con = driver->connect("sql_adress", "user", "nope"); // Join a server
        con->setSchema("database_name"); // Use a database

        stmt = con->createStatement();
        res = stmt->executeQuery(string("select " + columns + " from table_name where accession in (" + queries + ")"));

        sql::ResultSetMetaData *res_meta = res -> getMetaData();
        int cols = res_meta -> getColumnCount();
        if (res->rowsCount() == 0){
            cout << "Query not found!" << endl;
            return 0;
        }
        while (res->next()) {
            for (unsigned i = 1; i <= cols; i++){
                cout << res->getString(i) << endl;
            }
        }
        delete res;
        delete stmt;
        delete con;
    }
    // No idea about this part yet, copied it from a tutorial
    catch (sql::SQLException &e){
        cout << "# ERR: SQLException in " << __FILE__;
        cout << "(" << __FUNCTION__ << ") on line " << __LINE__ << endl;
        cout << "# ERR: " << e.what();
        cout << " (MySQL error code: " << e.getErrorCode();
        cout << ", SQLState: " << e.getSQLState() << " )" << endl;
    }
    return 0;
}

I found this code very difficult to understand, and I feel that I won't be alone. I can't refactor it (for the aforementioned reason) nevertheless some advice which you might find instructive:

Please use names which people will make sense of: column_name_list, opt, "F", opterr etc etc - this makes reading code very difficult.
formatting - please use good formatting it all makes a difference.
break things up into methods so that each method is doing only one thing.
Add better comments if things are not self-explanatory.

My two cents.


Simple python code takes command line argument for file location and tokenizes text The usage of this Python program is:

python ~/test.py test.txt

Here is the source:

import sys
#Open file passed by terminal
if len(sys.argv)==2:

    try:
        open(sys.argv[1])
        with open(sys.argv[1]) as inputfile:
            #Iterate through lines
            for line in inputfile:
                #tokenize
                words=line.split()
                #print tokens separated by commas
                print(",".join(words))
        inputfile.close
        sys.exit(0)

    except:
        sys.exit(-1)
sys.exit(-1)

This is the "test.txt"

\ / input("       ")
input(print("some text"))

             one        two                 three
other                    things                  here
             more stuff on this    line

garbage
\n
%20

and it should output (which it does):

\,/,input(",")
input(print("some,text"))

one,two,three
other,things,here
more,stuff,on,this,line

garbage
\n
%20

But I'm wondering if there is some unexpected way to break it. Is there a way to nest carriage returns or something somewhere? Oh, and I realize that I can accomplish the same thing with:

print(",".join(line.split()))

But for this specific instance, I would like to separate the two steps.

You have a few redundant and unnecessary calls in there.

Calling open(sys.argv[1]) right before with open(sys.argv[1]) as inputfile: does not do anything.
The with statement already takes care of closing the file. So you don't need the inputfile.close
If with did not close your file, it would not be closed, because you actually need to call that method (so inputfile.close())
You should never have a bare except. This catches more than you want, including, for example, the user pressing Ctrl+C. Always use as specific an exception as possible. Here you probably want to except the file missing, so use either IOError (Python 2.x) or FileNotFoundError (Python 3.x)

You should also have a look at Python's official style-guide, PEP8. It recommends surrounding operators with spaces, so if len(sys.argv) == 2:.

If you want this script to do something more, you should start to split the responsibilities into functions. Here you could have something like this:

import sys

def parse_lines(file_name):
    """Parse the lines of file_names into comma-separated lists"""
    with open(file_name) as inputfile:
        for line in inputfile:
            yield ",".join(line.split())

if __name__ == "__main__":
    # Open file passed by terminal
    if len(sys.argv) == 2:
        try:
            for line in parse_lines(sys.argv[1]):
                print(line)
        except FileNotFoundError:
            print(f"File {sys.argv[1]} not found")
            sys.exit(-1)

Here I put the parsing into a function, which is actually a generator. It yields the parsed lines one by one. I also added a docstring describing the function.

I also made your error handling code more specific. Your code actually makes debugging your program harder, because the program returning -1 (regardless of the error encountered) is  a lot less clear than Python creating a stacktrace. If you do catch errors, you should handle them (and give the user feedback if necessary). This uses Python 3.6's new f-strings.


Newton-Pepys, common birthdays and other probability simulations in Python 3 I have been learning Probability from Statistics 110 and trying to simulate problems that have an unintuitive answer

from numpy import cumsum
from statistics import mean
from numpy.random import exponential
from random import randint, sample, uniform
from bisect import bisect_left

def mean_of_experiments(experiment_func, N=100000):
    '''Decorator to repeat any Bernoulli trial N times and return probability of success'''
    def wrapper(*args, **kwargs):
        return round(mean(experiment_func(*args, **kwargs) for _ in range(N)), 3)
    return wrapper

@mean_of_experiments
def common_birthday(k):
    '''Simulates an experiment to generate k independent uniformly random birthdays and check if there are any repeat birthdays'''
    rands = [randint(1, 365) for _ in range(k)]
    return len(rands) != len(set(rands))

@mean_of_experiments
def matching(k=52):
    '''Simulates an experiment to permute 'k' cards and check if any jth card's value is j'''
    idx_labels = enumerate(sample(range(k), k))
    return any(idx == label for idx, label in idx_labels)

@mean_of_experiments
def dice(n, c):
    '''Simulates an experiment to roll 'n' dice and and check if count of 6's is at least c'''
    return [randint(1, 6) for _ in range(n)].count(6) >= c

def boardings(scale=5.0, N=1_00_000):
    '''Simulates an experiment where arrival of buses at stop follows a Poisson process and finds avg. inter-arrival time at a random instant'''
    arrivals = cumsum([exponential(scale=scale) for _ in range(N)])

    @mean_of_experiments
    def wait():
        boarding_idx = bisect_left(arrivals, uniform(0, arrivals[-1]))
        missed_bus = 0 if boarding_idx == 0 else arrivals[boarding_idx - 1]
        return arrivals[boarding_idx] - missed_bus

    return wait()

Probability Problems:

common_birthday: Given k people, what's the probability that any 2 people share a birthday? For 23 people, it's >50% and for 50 people, >97%.
matching: Given 52 cards with a unique label in 0...n-1, they are shuffled. What's the probability that any j^th card's label is j? Answer's very close to 1-1/e
dice: Problem posed by a gambler to Newton; dice(6, 1) > dice(12, 2) > dice(18, 3)
boardings: Given that a bus company's buses follow a Poisson process, if a person visits a stop at some random time, what was that inter-arrival time? From Tsitsiklis' lectures; answer is 2*scale for any value of scale.

What I wish I could do in my code is dynamically set the value of N in the decorator mean_of_experiments. Is that possible?

1. Design

I think that using the @mean_of_experiments decorator is not the best approach to this problem:

There might be cases where you want to run the underlying function by itself, for example in order to test it, but the decorator prevents this.
You have to choose the number of experiments, N, when you define the function. This is restrictive: it means that you can't run the function with different values of N.
The only result you get back is the mean. But what if you sometimes want another statistic, such as the standard deviation?

My preferred design would use the principle of separation of concerns. For example, we might write:

def mean_of_experiments(n, f, *args, **kwargs):
    "Return the mean of n calls to f(*args, **kwargs)."
    return mean(f(*args, **kwargs) for _ in range(n))

This meets all my objections:

I can call common_birthday(23) to carry out one experiment, or mean_of_experiments(1000, common_birthday, 23) to find the mean of 1000 experiments.
I can choose a different value of n for each call to mean_of_experiments, I don't have to commit to a particular value in advance.
If I need the standard deviation, I can write std_of_experiments, and this can coexist in the same program with mean_of_experiments.

2. Review

The code is very clear and well documented! In the top 1% of questions on Code Review.
If you're going to use NumPy at all, I think you might as well take advantage of it everywhere, instead of mixing and matching with the Python standard library. For example, in common_birthday you could use numpy.random.randint and numpy.unique:

def common_birthday(k, days=365):
    "Return True iff there are any repeats among k random birthdays."
    return k != np.unique(np.random.randint(1, days, size=k)).size

Similarly, in matching, you could use numpy.random.permutation, numpy.arange, and numpy.any:

def matching(k=52):
    """Return True iff there's any j such that card j appears in the j'th
    place of a shuffled deck of k cards.

    """
    return (np.random.permutation(k) == np.arange(k)).any()

The docstring for boardings doesn't explain the meaning of the scale and N arguments.
The scale argument to boardings seems unnecessary, since the randomly chosen instants will scale along with the arrival times.


Computing all Tic-Tac-Toe moves Recently I started to learn Haskell. I did this by making exercises on the Internet. The problem of making these exercises is that I never know if I solved it the correct way. I currently wrote a function which I think looks really ugly, but I don't know how to improve it. I hope that someone could give me some tips to improve it.

In this exercise I need to make a Tic-Tac-Toe game.

The question of the function I wrote is as follows:

  Exercise 8. Write a function moves :: Player -> Board -> [Board] that,
  given the current player and the current state of the board, returns
  all possible moves that player can make expressed as a list of
  resulting boards. (For now, you should continue making moves, even if
  one of the players has already won.)

I wrote the following code:

moves :: Player -> Board -> [Board]
moves p ((a,b,c),(d,e,f), (g,h,i)) = (moves' a (((symbol p),b,c), (d,e,f), (g,h,i))) ++
                                     (moves' b ((a,(symbol p),c), (d,e,f), (g,h,i))) ++
                                     (moves' c ((a,b,(symbol p)), (d,e,f), (g,h,i))) ++
                                     (moves' d ((a,b,c), ((symbol p),e,f), (g,h,i))) ++
                                     (moves' e ((a,b,c), (d,(symbol p),f), (g,h,i))) ++
                                     (moves' f ((a,b,c), (d,e,(symbol p)), (g,h,i))) ++
                                     (moves' g ((a,b,c), (d,e,f), ((symbol p),h,i))) ++
                                     (moves' h ((a,b,c), (d,e,f), (g,(symbol p),i))) ++
                                     (moves' i ((a,b,c), (d,e,f), (g,h,(symbol p))))

moves' :: Field -> Board -> [Board]
moves' c m = if c == B then [m] else []

Is there a way to improve my code?

First of all, you can use a where binding to clear this up:

moves :: Player -> Board -> [Board]
moves p ((a,b,c),(d,e,f), (g,h,i)) = (moves' a ((s,b,c), (d,e,f), (g,h,i))) ++
                                     (moves' b ((a,s,c), (d,e,f), (g,h,i))) ++
                                     (moves' c ((a,b,s), (d,e,f), (g,h,i))) ++
                                     (moves' d ((a,b,c), (s,e,f), (g,h,i))) ++
                                     (moves' e ((a,b,c), (d,s,f), (g,h,i))) ++
                                     (moves' f ((a,b,c), (d,e,s), (g,h,i))) ++
                                     (moves' g ((a,b,c), (d,e,f), (s,h,i))) ++
                                     (moves' h ((a,b,c), (d,e,f), (g,s,i))) ++
                                     (moves' i ((a,b,c), (d,e,f), (g,h,s)))
    where
        s = symbol p

Next, I would recommend changing your Board type to use a flat list instead of nested tuples, it's a lot easier to work with:

type Board = [Field]

For whatever your Field type is (for future reference please include all relevant definitions in your questions on SO, this goes for any language).  Now you can write this a bit differently:

moves :: Player -> Board -> [Board]
moves p [a, b, c, d, e, f, g, h, i] = moves' a [s, b, c, d, e, f, g, h, i] ++
                                      moves' b [a, s, c, d, e, f, g, h, i] ++
                                      moves' c [a, b, s, d, e, f, g, h, i] ++
                                      moves' d [a, b, c, s, e, f, g, h, i] ++
                                      moves' e [a, b, c, d, s, f, g, h, i] ++
                                      moves' f [a, b, c, d, e, s, g, h, i] ++
                                      moves' g [a, b, c, d, e, f, s, h, i] ++
                                      moves' h [a, b, c, d, e, f, g, s, i] ++
                                      moves' i [a, b, c, d, e, f, g, h, s]
    where
        s = symbol p

Now I recognize a zipWith here between the first and second arguments to moves', although we'll need a concat after the zipWith is done:

moves :: Player -> Board -> [Board]
moves p board@[a, b, c, d, e, f, g, h, i]
    = concat
    $ zipWith moves'
        board
        [[s, b, c, d, e, f, g, h, i],
         [a, s, c, d, e, f, g, h, i],
         [a, b, s, d, e, f, g, h, i],
         [a, b, c, s, e, f, g, h, i],
         [a, b, c, d, s, f, g, h, i],
         [a, b, c, d, e, s, g, h, i],
         [a, b, c, d, e, f, s, h, i],
         [a, b, c, d, e, f, g, s, i],
         [a, b, c, d, e, f, g, h, s]]
    where
        s = symbol p

Now, this still leaves the big block of different boards.  There is definitely a pattern here.  If you have a replace function with the type replace :: Int -> a -> [a] -> [a] (I'll leave this as an exercise to the reader) then you could write this as

[replace 0 s board,
 replace 1 s board,
 replace 2 s board,
 replace 3 s board,
 replace 4 s board,
 replace 5 s board,
 replace 6 s board,
 replace 7 s board,
 replace 8 s board]

Or more simply

map (\i -> replace i s board) [0..8]

So your entire function becomes

moves p board
    = concat $ zipWith moves' board
    $ map (\i -> replace i s board) [0..8]
    where
        s = symbol p
        replace i a xs = undefined
        moves' B m = [m]
        moves' _ _ = []


Python code to check if an array has a sequence (1,3,4) I recently applied for a job as a Python coder but was rejected.

This was the problem:

  Write a python code to check if an array has a sequence (1,3,4)

Assuming they were looking for expert Python programmers, what could I have done better?

# Tested with Python 2.7
import unittest

# Runtime: O(n)

def doesSeqAppear(int_arr):
    #check if input is a list
    if not isinstance(int_arr, list):
        raise TypeError("Input shall be of type array.")

    # check all elements are of type int
    if not all(isinstance(item, int) for item in int_arr) :
        raise ValueError("All elements in array shall be of type int.")

    arr_len = len(int_arr)
    if arr_len < 3:
        return False

    # Loop through elements
    for i in range(arr_len-2):
        if int_arr[i] == 1 and \
            int_arr[i+1] == 3 and \
            int_arr[i+2] == 4 :
            return True
    return False

class TestMethodDoesSeqAppear(unittest.TestCase):
    def test_only_single_seq(self):
        #Single time
        assert doesSeqAppear([1,3,4]) == True

    def test_multiple_seq(self):
        #multiple
        assert doesSeqAppear([2,2,1,3,4,2,1,3,4]) == True

    def test_neg_seq(self):
        #multiple
        assert doesSeqAppear([9,-1,1,3,4,-4,4]) == True

    def test_only_empty_seq(self):
        #empty
        assert doesSeqAppear([]) == False
    def test_only_single_elem_seq(self):
        #Single element
        assert doesSeqAppear([1]) == False

    def test_input_is_none(self):
        self.assertRaises(TypeError, doesSeqAppear, None)

    def test_raises_type_error(self):
        self.assertRaises(TypeError, doesSeqAppear, "string")

    def test_raises_value_error(self):
        self.assertRaises(ValueError, doesSeqAppear, [1,2,'a', 'b'])

if __name__ == '__main__':
    unittest.main()

#

By PEP 8, doesSeqAppear should be does_seq_appear. You used the right naming convention for your unit tests, though. Personally, I would prefer def contains_seq(arr, seq=[1, 3, 4]).

Your arr_len < 3 test is superfluous and should therefore be eliminated. Don't write a special case when the regular case works correctly and just as quickly.

Your all(isinstance(item, int) for item in int_arr) check was not specified in the problem, and is therefore harmful. The question does not say that doesSeqAppear([3.1, 1, 3, 4]) should return False, nor does it say that it should fail with an exception. In fact, by my interpretation, it does contain the magic sequence and should therefore return True. In any case, you have wasted a complete iteration of the list just to perform a check that wasn't asked for.

Checking isinstance(int_arr, list) is un-Pythonic, since duck-typing is the norm in Python. In any case, the code would likely fail naturally if it is not a list.

After cutting all that excess, you should drop the # Loop through elements comment as well.


Simplifying multiple mouseover and mouseout functions I am trying to hide and show tooltips depending on what element is hovered over.  This works as expected and I could continue to rinse and repeat. However, I am wondering if there is a good way to simplify this or make it more compact. Also I was assuming I could use .hover(), but that didn't seem to work for me. So instead I am using mouseover and mouseout.

$('.remove').eq(0).mouseover(function(){
    $('.tooltip').eq(0).show();
});

$('.remove').eq(0).mouseout(function(){
    $('.tooltip').eq(0).hide();
});

$('.remove').eq(1).mouseover(function(){
    $('.tooltip').eq(1).show();
});

$('.remove').eq(1).mouseout(function(){
    $('.tooltip').eq(1).hide();
});

Use an .each "loop":

var tooltips = $('.tooltip');

$('.remove').each(function(i) {
    var tooltip = tooltips.eq(i);

    $(this).on({
        mouseover: function() { tooltip.show(); },
        mouseout:  function() { tooltip.hide(); }
    });
});


Finding minimum scalar product using ST Monad Inspired by a Stack Overflow question, I decided to practice my Haskell by taking a crack at the Google Code Jam's Minimum Scalar Product problem:

  Given two vectors \$\mathbf{v_1}=(x_1,x_2,\ldots,x_n)\$ and \$\mathbf{v_2}=(y_1,y_2,\ldots,y_n)\$. If you could permute the coordinates of each vector, what is the minimum scalar product \$\mathbf{v_1} \cdot \mathbf{v_2}\$?

  Constraints:
  \$100 \le n \le 800\$
  \$-100000 \le x_i, y_i \le 100000\$

I'm not claiming any algorithmic awesomeness (this is just a reference implementation for checking correctness later).

This is my first time using Vectors and the ST monad, so what I really want is a sanity check that I'm using both correctly, and that I'm using the correct tools for the job.

module MinimumScalarProduct where

import Control.Monad (replicateM, forM_, (>=>))
import Control.Monad.ST (runST, ST)
import Data.Vector (thaw, fromList, MVector, Vector, zipWith, foldl', length, (!))
import Data.Vector.Generic.Mutable (read, swap)
import Prelude hiding (read, zipWith, length)

-- sequnce of transpoitions to yield all permutations of n elts
-- http://en.wikipedia.org/wiki/Steinhaus-Johnson-Trotter_algorithm
transpositions :: Int -> [(Int, Int)]
transpositions n = runST $ do
          -- p maps index to element
          p <- thaw $ fromList [0 .. n-1]
          -- q maps element to index
          q <- thaw $ fromList [0 .. n-1]
          -- let the prefixes define themselves recursively
          foldr ((>=>) . extend p q) return [2..n] []

extend :: MVector s Int -> MVector s Int -> Int -> [(Int,Int)] -> ST s [(Int,Int)]
extend p q n ts = fmap ((ts++) . concat) . replicateM (n-1) $ do
  -- replace the element in the (n-1)'th position with its predecessor
  a <- read p (n-1)
  i <- read q (a-1)
  swap p (n-1) i
  swap q a (a-1)
  -- replay the earlier transpositions
  forM_ ts $ \(m,j) -> do
    b <- read p m
    c <- read p j
    swap p m j
    swap q b c
  return $ (n-1,i) : ts

-- reference implementation, takes O(n!)
msp :: Vector Int -> Vector Int -> Int
msp u v | length u /= length v = 0
msp u v = runST $ do
  let x = foldl' (+) 0 $ zipWith (*) u v
  let n = length u
  u' <- thaw u
  -- check each permutation of u'
  let steps = map (adjust u' v) $ transpositions n
  fmap minimum . sequence $ scanl (>>=) (return x) steps

-- adjust the current scalar product for the transposition of u
adjust :: MVector s Int -> Vector Int -> (Int, Int) -> Int -> ST s Int
adjust u v (i,j) x = do
  a <- read u i
  b <- read u j
  let c = v ! i
  let d = v ! j
  swap u i j
  return $ x - (a*c + b*d) + (a*d + b*c)

Somewhat old question, but still it's a pity it hasn't been answered :). Putting aside the asymptotically better solution suggested in the comments, and focusing just on the code:

Pluses: Top-level types for all funtcions, that's very good, as well as comments. The code shows good understanding of various Haskell functions and idioms.

My most general comment is that there is disproportion between code verbosity and complexity. Usually more complex code should be more verbose and more descriptive. Visually, most of the code is just simple read/write/swap ST operations, but the important part is squeezed into not-so-easy-to-understand and uncommon combinations of folds/scans/sequence/>=>/>>= etc. I'd probably separate the simple parts (read/swap) into helper functions and expand/simplify the complex parts a bit.

Function adjust somewhat mixes pure and mutable approaches. While it mutates its first argument, it keeps the scalar product pure and passes it as an argument in and out. This is somewhat confusing and complicates the computation of the minimum (scanl with >>=). If the product were a ST variable too, the code gets simpler (untested):

adjust :: MVector s Int -> Vector Int -> STRef s Int -> (Int, Int) -> ST s Int
...
msp = ...
  x <- newSTRef $ foldl' (+) 0 $ zipWith (*) u v
  fmap minimum . traverse (adjust u' v x) . transpositions $ n

Alternatively you could even keep the result in an ST variable.

Also foldr with >=> can be replaced with foldM (untested):

foldM (flip $ extend p q) [] [2..n]

(Then it'd make sense to refactor the type of extend to get rid of the flip.)

Some questions:

Is replaying all ts in extend necessary? As it's executed repeatedly, it'd make sense to compute the combined permutation of ts separately or once and then just apply it.
Which variant of the algorithm is actually used? Using two vectors doesn't seem to match anything in the linked Wikipedia article. While the general idea of the algorithm is clear, it'd be easier for someone not familiar with it to get more precise guidelines.

Nit: The order of functions is somewhat unnatural: transpositions uses extend, msp uses transpositions and adjust. There is no visible order in this. One good option is that functions are always defined before being used (with the obvious exception of mutually-recursive functions). Or the other way around - the main/exported functions are first and helper functions follow. Or, separate functions into (possibly nested) sections, then ordering of functions doesn't really matter, ordering of sections becomes important.


Batch file to delete calling program and then itself There are times when I want to run a one-off executable on an end-user's machine that cleans up after itself, i.e. deletes itself without a trace. An exe can't delete itself because an exe can't be deleted while it's running. However, a batch file can delete itself, presumably because its contents get copied so the file can close before the code gets run.

So what if there was a general-purpose batch file that deletes the calling executable and then deletes itself? I couldn't find such a batch file so I made one, thinking maybe it could be useful to others. And obviously I want to see if it can be improved upon, of course.

This is particularly useful for an NSIS installer because the batch file can be embedded in the exe and then extracted.

Make sure you copy the batch file before you test it because it will delete itself!

@ECHO OFF

SET filename=%~nx1

IF "%filename%"=="" GOTO:EOF

TASKKILL /IM "%filename%" /F

:LOOP
TASKLIST | FIND /I "%filename%" >NUL 2>&1
IF ERRORLEVEL 1 (
  GOTO CONTINUE
) ELSE (
  ECHO %filename% is still running
  PING -n 6 127.0.0.1>NUL
  GOTO LOOP
)

:CONTINUE

DEL /F "%~f1"
DEL /F "%~f0"

I figured the easiest way to get the path of the calling program is to just have it passed in as a parameter. The IF "%filename%"=="" GOTO:EOF is very important because if the first parameter isn't defined then DEL /F "%~f1" will automatically act like a * is there and try to delete the contents of the whole folder.

TASKKILL /IM "%filename%" /F

It makes sense that you might want to do this, but I expect the file would be more useful if it waited for its calling program to end on its own. That way we can be sure the calling executable is done with everything else, including releasing any resources it's holding. Because you already have looping code checking for the process, this should be as simple as removing the line.

Comments would be great.

For example, I understand that PING -n 6 127.0.0.1>NUL is a fairly common delaying tactic, but it's still downright odd if you haven't seen it before.

Likewise filename manipulations (%~nx1 and such) will always be arcane. A comment saying that it pulls out the name and extension would be appreciated.

There's an alternative here, which is a bit of a cheat but worth mentioning as potentially safer in some cases. Because the use case of this batch file is that it's created by the executable that wants cleaning up, that executable could do the text substitution and put its own filename directly into the relevant code of the batch file. Obviously this depends on the language that the executable is written in having good string manipulation capabilties, but it would help protect against deleting whole folders and other such disasters.

GOTO CONTINUE

Personally, I would not have called it 'CONTINUE' because in many C family languages continue would continue with the next iteration of a loop, rather than break out of it. Obviously it makes no difference to the script, and it's not something that a pure batch programmer would trip over. I just found it slightly jarring, and thought I'd mention it.

) ELSE (

This is not clear cut, but some people find it easier to read if the else is omitted when the if side breaks, returns, or otherwise interupts the control flow. That is

:LOOP
TASKLIST | FIND /I "%filename%" >NUL 2>&1
IF ERRORLEVEL 1 (
  GOTO CONTINUE
)
ECHO %filename% is still running
PING -n 6 127.0.0.1>NUL
GOTO LOOP

Alternatively, you could swap the test around and avoid the goto CONTINUE altogether.

:LOOP
TASKLIST | FIND /I "%filename%" >NUL 2>&1
IF ERRORLEVEL 0 (
  ECHO %filename% is still running
  PING -n 6 127.0.0.1>NUL
  GOTO LOOP
)

I would personally find that easier to read as the batch magic equivalent of a conventional spin lock style construction.


Lists' intersection keeping elements' order I could use the function Intersection, but it returns a sorted list. That's why I have to do my own, but it looks too big. I hope it could be done shorter.

lists = {{1, 2, 3, 4, 5}, {1, 2, 3, 4}, {2, 3, 4, 5}};
Fold[ Function[ {a, b},
  Select[b, MemberQ[a, #] &]
  ], lists // First, lists // Rest]

This function deletes from the first list all the elements not contained in the intersection, thus returning what you want:

f[l_List]:= DeleteCases[First@l, Except[Alternatives @@ (Intersection @@ l)]]

f[{{1, 2, 3, 4, 5}, {1, 2, 3, 4}, {2, 3, 4, 5}}]
->{2,3,4}

f[{{5, 7, 6, 3}, {5, 6, 1, 3}}]
->{5,6,3}


Random retrievable number based on day and month I wrote a function to get a 2-digit random number for each day, but it should be retrievable.
For example, today's random number is 72, but later I can provide "day and month" and get this 72 again.
function ranDay($day,$month){

    $pow1 = round(pow($day,$day))*$month;
    $pow2 = round(pow($day,$day))+$month;

    $dig1 = intval(substr($pow1,0,1));
    $dig2 = intval(substr($pow2,-1));

    echo $dig1.$dig2;
}

Now if I provide 27 and 1, I get 48 and I can get this number again later.
Am I doing this right?
The only issue I see in my code is that I can't get 0 for the first digit, and it would be nice to cover that too.
Please review this and tell me if I should consider doing something else.
EDIT:
Based on the accepted answer in ended up using this code instead:
function hashDay($day, $month) {
    echo substr(hexdec(substr(hash('sha256',($month.$day)),0,15)),-2);
}

reason i used 2 substr is amazingly when i used -2 instead of 0,15 in first substr, i got 1-3 digits instead of always 2 digits...

I wrote a function to get a 2-digit random number for each day, but it should be retrievable.
For example, today's random number is 72, but later I can provide "day and month" and get this 72 again.

This isn't random.  This is a hashing function.  For a given input, it always returns the same output.  The normal name for something like this is a hashing function.
You don't explain why you want to do this.  If you ask again, try to include that context.  We may suggest alternatives.

function ranDay($day,$month){

If this were a random function, I'd write out the name:  randomDay.  If you really must abbreviate, randDay would be better.  However, as I noted earlier, this isn't random.  You are just mapping one input to another, so a name like hashDay would be better.  There may be an even better name.
function hashDay($day, $month) {
    if ( ! is_numeric($day) || ! is_numeric($month) || 0 > $day || 0 > $month || 12 < $month || 31 < $day ) {
        return;
    }

I also added additional whitespace for readability.
Since this is only supposed to work for valid day and month values, I added some basic checks.  Even with these, it will still allow dates like the 31st of February, but this restricts the range somewhat.  I just return an undefined value implicitly, but you could explicitly return null (which would do the same thing) or do something like throwing an exception.  Note that this also handles if the month or day is given as a string (e.g. January) rather than a number.
Handling zero values with different returns than a value of one is possible:
    $day++;

With that, a zero and a one will return different values.

$dig1 = intval(substr($pow1,0,1));
$dig2 = intval(substr($pow2,-1));

I'd write out $digit1 and $digit2.
Also, you don't need the intval.  You immediately use string concatenation on it, which will throw it back into strings.  So the only thing that the intval can do is hide bad input from you.

echo $dig1.$dig2;

Rather than echoing the result, it would probably be better to return it.  That way you can display it or check for invalid values or whatever.  This can also be seen as a violation of the Single Responsibility Principle:  you are calculating the hash value; don't display it as well.  If you really want to display it, consider changing the name to something like printDayHash.
return $digit1 . $digit2;

Your function will have collisions.  There are only 100 possible results, but there are at least 366 valid day/month combinations.  Running it on all 416 allowed day/month combinations (including zero values and weird dates like the 31st of February) shows that some results are more likely than others.  For example, 91 and 92 (and eight others) are never returned while 16 is returned a whopping twenty times.  For most uses, it would be better for this to be more evenly distributed.
My test code:
$count_of = array();
for ( $i = 0; $i < 10; $i++ ) {
    for ( $j = 0; $j < 10; $j++ ) {
        $count_of[$i.$j] = 0;
    }
}

for ( $m = 0; $m <= 12; $m++ ) {
    for ( $d = 0; $d <= 31; $d++ ) {
         $count_of[hashDay($d, $m)]++;
    }
}

asort($count_of);
print_r($count_of);

Note how returning the value rather than displaying it makes this test much easier.


Text justification program I'm looking for code review comments for text justification problem described here.

Problem:

Given an array of words and a length L, format the text such that each line has exactly L characters and is fully (left and right) justified.

You should pack your words in a greedy approach; that is, pack as many words as you can in each line. Pad extra spaces ' ' when necessary so that each line has exactly L characters.

Extra spaces between words should be distributed as evenly as possible. If the number of spaces on a line do not divide evenly between words, the empty slots on the left will be assigned more spaces than the slots on the right.

For the last line of text, it should be left justified and no extra space is inserted between words.

For example,
words: ["This", "is", "an", "example", "of", "text", "justification."]
L: 16.

Response:

[
   "This    is    an",
   "example  of text",
   "justification.  "
]

/**
 * @param {string[]} words
 * @param {number} maxWidth
 * @return {string[]}
 */
var fullJustify = function(words, maxWidth) {
    let text = new Text(maxWidth);

    text.parse(words);

    return text.toStringArr();
};

function Text (maxWidth){
    this.lines = [];
    this.currLineIndex = -1;

    Text.prototype.parse = parse;
    Text.prototype.toStringArr = toStringArr;

    function parse(words) {
        words.forEach(function(word)  {
            let line = this.lines[this.currLineIndex];

            if(line && line.canAddWord(word)){
                line.add(word);
           } else {

               // Justify current row, if exists
               if(line) {
                line.justify();
               }

               // Create new row and add element to that
               this.currLineIndex++;
               line = new Line(maxWidth);
               this.lines[this.currLineIndex] = line;

               line.add(word);
           }
        }, this);

        // Justify last line
        let lastLine = this.lines[this.currLineIndex];
        lastLine.justify(true);
    }

    function toStringArr() {
        let result = [];

        this.lines.forEach(function(line) {
            result.push(line.toString());
        });

        return result;
    }
}
function Line (maxLen){
    this.words = [];
    this.spaces = [];   //
    this.size = 0;   // Initial size of array
    this.maxLen = maxLen;

    function add(word) {
        if(this.canAddWord(word)) {
            this.words.push(word);
            this.size += word.length;
            return true;
        } else {
            return false;
        }
    }

    function toString() {
        let result = [];
        this.words.forEach(function(word, i) {
            result.push(word + " ".repeat(this.spaces[i]));
        }, this);

        return result.join("");
    }

    function justify(isLastLine) {
        let spacesCount = this.words.length - 1;
        let extraSpaces = this.maxLen - this.size;

        let quotient = extraSpaces / spacesCount;
        let mod = extraSpaces % spacesCount;

        if(isLastLine) {
            // Add one space between each word
            for(let i = 0; i < this.words.length - 1; i++) {
                this.spaces[i] = 1;
            }
        } else {
            // Distribute (maxLen - size) in $(spacesCount) evenly
            for(let i = 0; i < this.words.length - 1; i++) {
                this.spaces[i] = quotient + ((mod-- > 0)? 1 : 0);
            }
        }
    }

    function canAddWord(word) {
        return (this.size + (this.words.length - 1) + word.length <= this.maxLen)
    }

    Line.prototype.add=  add;
    Line.prototype.toString = toString;
    Line.prototype.justify = justify;
    Line.prototype.canAddWord = canAddWord;
}

Interesting problem.

I think your answer is quite a bit longer than it needs to be.  Consider rethinking what the solution entails, essentially.  Also, your code will be quite a bit more readable if you avoid nesting statement -- especially thing like loops nested inside conditionals, such as you have here:

  } else {
        // Distribute (maxLen - size) in $(spacesCount) evenly
        for(let i = 0; i < this.words.length - 1; i++) {
            this.spaces[i] = quotient + ((mod-- > 0)? 1 : 0);
        }
    }

You have the right idea to break the problem down into the subproblems of line breaks and justification, but trying to do so with objects, rather than simple functions, seems to have added complexity for you.

Consider this rewrite, which expresses the line breaking problem using recursion, and also takes advantage of es6 features.  It's about a third the size of the original, even with lots of intermediate variables which exist only for the sake of readability

function fullJustify(words, maxLen) {
  return asLines(words, maxLen).map(x => justify(x, maxLen))
}

function asLines(words, maxLen, curLine=[], charCount = 0, lines = []) {

  if (!words.length)
    return lines.concat([curLine])

  const nextWord        = words[0]
  const remainingWords  = words.slice(1)
  const additionalChars = nextWord.length + (curLine.length ? 1 : 0)
  const nextCharCount   = charCount + additionalChars
  const breakLine       = nextCharCount > maxLen

  if (breakLine)
    return asLines(words, maxLen, [], 0, lines.concat([curLine]))

  return asLines( remainingWords, maxLen, curLine.concat(nextWord),
    nextCharCount, lines )
}

function justify(words, len) {
  if (words.length == 1)
    return words[0] + ' '.repeat(len - words[0].length)

  const numPaddedWords  = words.length - 1
  const totalChars      = words.reduce((m, w) => m + w.length, 0)
  const extraChars      = len - totalChars
  const spaceBetween    = Math.floor(extraChars / numPaddedWords)
  const spacer          = ' '.repeat(spaceBetween)
  const extraSpaces     = extraChars - spaceBetween * numPaddedWords
  const leftPaddedWords = words.slice(1).map(
    (w, i) => spacer + (i < extraSpaces ? ' ' : '') + w
  )
  return [words[0], ...leftPaddedWords].join('')
}

additional improvement

my asLines function above is still too complex for my taste, so i did one more improvement, using a few utility functions from ramda.js:

function asLines(words, len, lines=[]) {

  if (!words.length) return lines

  let charCount    = -1  // bc the first word is not left-padded
  const fitsOnLine = w => (charCount += w.length + 1) < len
  const nextLine   = takeWhile(fitsOnLine, words)
  const remaining  = drop(nextLine.length, words)
  return asLines(remaining, len, lines.concat([nextLine]))
}


Returning rental invoice data transfer object When trying to follow single responsibility principle, I often find myself in a position where there is so much to do that I do not know where to start. Maybe there is there a rule of thumb that I could use?

A concrete example originating from my test project.

    // doing way too many things
    public InvoiceDto GetInvoice(CartDto cart)
    {
        if (cart == null)
            throw new ArgumentNullException(nameof(cart));                                      // validation

        var cartequipments = _carts.GetById(cart.CartId).CartEquipments.ToList();               // quering
        var rentals = cartequipments.Select(o =>                                                // mapping
            new RentalDto                                                                       // object creation
            {
                Name = o.Equipment.EquipmentName,
                Price = _mapPriceCalculatorLogic                                                // buisiness logic
                    .Create(o.Equipment.EquipmentType)
                    .Calculate(o.RentDurationDays)
            }).ToList();
        var loyaltyPoints = _loyaltyPointsService                                               // buisiness logic
            .GetLoyaltyPoints(cartequipments.Select(o => o.Equipment.EquipmentType));
        var total = rentals.Sum(o => o.Price);                                                  // aggregation

        var invoiceDto = new InvoiceDto                                                         // object creation
        {
            Title = $"Invoice id : {cart.CartId}",
            Rentals = rentals,
            LoyaltyPoints = loyaltyPoints,
            TotalPrice = total
        };

        return invoiceDto;
    }

My question is how would you clean this up and if possible what would the result look like (helper methods sources not needed).

Make separate methods for getting equipment from the cart, creating rentals from the equipment, getting loyalty points from the equipment, and creating the invoice from the rentals and loyalty points.