Recursive os.scandir() for copying/moving files preserving subdirectories This is modified from the general recursive solution here: https://stackoverflow.com/a/54544211/42346 to, more specifically, copy/move files with a given extension from one directory to another whilst preserving subdirectories.

I'm reluctant to use .rename() for this purpose as it seems slightly less intuitive than copying or moving.

Any other ideas you have about how to clean this up are greatly appreciated.

import os, shutil
from pathlib import Path

def recur(path,destination,file_ext,is_subdir=False):
    if not is_subdir:
        os.chdir(path)
    for entry in os.scandir(path):
        if os.path.splitext(entry.name)[-1].lower() == file_ext:
            subdir_path = entry.path.replace(os.getcwd() + os.sep,'')
            new_path = Path(destination) / subdir_path
            os.makedirs(os.path.dirname(new_path), exist_ok=True)
            shutil.copy(entry.path,new_path)
        if entry.is_dir():
            recur(Path(path).joinpath(entry.name),destination,file_ext,is_subdir=True)

You use pathlib.Path, which is great, but it can do a lot more:

Operations like os.path.splitext should be modernized using the suffix attribute of Path objects; for a full list of those designated replacements, see the documentation.
os.scandir can in this case be replaced by Path.rglob. This is where the recursion you mentioned comes into play. But that's it: Recursive globbing is an irrelevant implementation detail here. The point is that we do not have to do it "manually". rglob takes a pattern as its string argument, which can be the suffix you mentioned.
Further, os.makedirs can be Path.mkdir with parents=True for the latter. This is not as big a win as the others, but nice.

Eventually, it turns out the os import can be done without altogether, and recursion is neatly tucked away:

from pathlib import Path
from shutil import copy2  # "identical to copy() but attempts to preserve to file metadata"

COPY_FROM = Path("source")
COPY_TO = Path("dest")

SUFFIX = "*.py"

for source in COPY_FROM.rglob(SUFFIX):
    subpath = source.relative_to(COPY_FROM)
    destination = COPY_TO.joinpath(subpath)
    destination.parent.mkdir(parents=True, exist_ok=True)
    copy2(source, destination)

Note the use of copy2, which is identical to copy but attempts to preserve metadata.
This can be convenient for e.g. music files.

The parent attribute is the logical parent path to the found file, so essentially all path elements except for the found file itself.
Like in your solution, those path elements (directories) have to be created first.
In mkdir, exist_ok=False is the default and would prohibit us from most desired operations, like copying files in subdirectories.

The above gives the following result, which is hopefully what you are aiming for:

~$ tree
.
├── dest
│   ├── file1.py
│   ├── test1
│   │   ├── file2.py
│   │   └── file3.py
│   └── test2
│       ├── file4.py
│       └── test3
│           └── file5.py
└── source
    ├── file1.py
    ├── test1
    │   ├── file2.py
    │   └── file3.py
    └── test2
        ├── file4.py
        └── test3
            └── file5.py


Find the common element in two int arrays I am preparing for a interview for a junior level c# position.  I take two arrays and find the common int between both of them.  Let me know if you have any feedback.

using System;
using System.Collections.Generic;

namespace CommonElement
{
    class MainClass
    {
        static void EnterArray1IntoHashSet (int[] array1, HashSet<int> hs){
            foreach (int num in array1)
            {
                if (!hs.Contains (num))
                {
                    hs.Add (num);
                }
            }
        }

        static int FindCommonInt(int[] array2, HashSet<int> hs){
            foreach (int num in array2)
            {
                if (hs.Contains (num))
                {
                    return num;
                }
            }
            return -1;
        }

        public static void Main (string[] args)
        {
            int[] array1 = new int[7]{ 5, 6, 7, 8, 1, 2, 3 };
            int[] array2 = new int[7]{ 13, 22, 3, 45, 67, 73, 85};
            HashSet<int> hs = new HashSet<int> ();
            EnterArray1IntoHashSet (array1, hs);
            int commonElement = FindCommonInt(array2, hs);
            if (commonElement != -1)
            {
                Console.WriteLine (commonElement);
            }
            else
            {
                Console.WriteLine ("No Common Element");
            }
        }
    }
}

Improving EnterArray1IntoHashSet

Why do you pass a hash set to EnterArray1IntoHashSet ?
It would be better to let the method create, populate and return the hash set.

You don't need to check if an element is inside a hash set before adding it:
if the element already exists, it will be simply ignored.

The name of this function is unnecessarily tedious.
It has a nice general purpose,
and you can name it accordingly.

static HashSet<int> ToHashSet(int[] array1)
{
    HashSet<int> set = new HashSet<int>();
    foreach (int num in array1)
    {
        set.Add(num);
    }
    return set;
}

Coding style

The placement of opening braces { is inconsistent:
sometimes you put it on the next line,
sometimes on the same line.
The .NET convention is to put it on the next line.

It's also unusual to put a space between method name and the ( when calling them, for example this:

Console.WriteLine (commonElement);

Just omit the unnecessary space:

Console.WriteLine(commonElement);


Regex match for a string in a URL I feel like there is too much repetitive code going on here. All I am doing is doing a basic regex match for a string in the URL.

If a match is found, I find a li with a class (.index, .grid or .type) and add the active class. This is just for my main nav in an attempt to make it somewhat dynamic.

However, I feel like there is a more efficient way to code this:

$(document).ready(function() {
  var myLocation = window.location.href;
  var index = /index/i;
  var grid = /grid/i;
  var type = /type/i;
  var urlIndex = convertURL.match(index);
  var urlGrid = convertURL.match(grid);
  var urlType = convertURL.match(type);

  if (urlIndex) {
    $('.index').addClass('active');
  } else if (urlGrid) {
    $('.grid').addClass('active');
  } else if (urlType) {
    $('.type').addClass('active');
  }
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

Much like elclanrs' solution, just slightly more "plain"

$(function () {
  var classes = ['index', 'grid', 'type'],
      url = window.location.href.toLowerCase();
  for(var i = 0, l = classes.length ; i < l ; i++ ) {
    if(url.indexOf(classes[i]) !== -1) {
      $('.' + classes[i]).addClass('active');
      break; // only first matching class is considered
    }
  }
});


Proper way of a testable method with multiple return points I have the following piece of java method code:

@Override
public void applyStates(int timestamp, State[] states) {

    // Calculate the time diff between current time and the time stamp of the given states.
    int diff = getNetworkTime() - timestamp;

    if (diff <= 0) {
        Log.e("Ignoring states that seem to come from the future!");
        return;
    }

    if (diff >= DIFF_THRESHOLD) {
        Log.w("Ignoring states that are too old. Diff is: " + diff);
        return;
    }

    // Do the real processing here.
}

I'd like this code to be testable (by unit tests), however since it has a flow that's comprised of multiple return statements, it's not so easy for me to do so.

I'd like to verify for example, that depending on the input, i go into the 2 different validation clauses in the beginning of the method.

Is there a better design than this? how can this method be better structured for testabilty?

You've got a Policy abstraction that is asking to be teased out.  The Policy provides a method that tests "is it valid to do this operation now".  In production, the Policy instance checks the time difference to see if it is valid.  In test, however, you specify an instance of the policy object that always says no when you want to test that code path.

You've a bit of refactoring to do to make that possible.  To make testing easier, you want to inject an instance of policy into the object, so that your test can control which implementation is being used.

@Override
public void applyStates(int timestamp, State[] states) {
    if (! this.policy.isValid(timestamp)) {
        return;
    }

    // ....

So for testing purposes, you might inject a "policy.isValid always returns false" object, and then run your tests and verify that none of the states have been applied (whatever that means).


Comparing similar types that don't share any interface I have two types that don't share any interface but have similar properties and that I need to compare for equality.

One of them is comming from a 3rd party library and one of them is mine. I obviously cannot add any interface to the foreign type and honestly I don't want to create any wrapper for both of them in order to just use it once for some linq. I prefer to have something that lasts longer and can be reused for other projects.

I used these to types for testing but they might of course have many other properties - and as a matter of fact my two actual types have exactly the same properties as this example:

class PersonLib1
{
  public string FirstName { get; set; }

  public string LastName { get; set; }
}

class PersonLib2
{
  public string FirstName { get; set; }

  public string LastName { get; set; }
}

API

I solved this problem by implementing the IEqualityComparer<object> as a DuckEqualityComparer - its name comes from the idea of Duck Typing - this is, since both types share some common or similar properties they must be the same.

My comparer allows the user to specify the exact types and their comparison logic. The order of the types is not relevant and it tries both combinations.

I used additional blocks of {} inside the Equals method to be able to reuse the short variable names.

class DuckEqualityComparer<Tx, Ty> : IEqualityComparer<object>
{
    private readonly Func<Tx, Ty, bool> _equals;
    private readonly Func<Tx, int> _getHashCodeX;
    private readonly Func<Ty, int> _getHashCodeY;

    private DuckEqualityComparer(Func<Tx, Ty, bool> equals, Func<Tx, int> getHashCodeX, Func<Ty, int> getHashCodeY)
    {
        _equals = equals;
        _getHashCodeX = getHashCodeX;
        _getHashCodeY = getHashCodeY;
    }

    public new bool Equals(object x, object y)
    {
        if (ReferenceEquals(x, y)) return true;
        if (ReferenceEquals(x, null)) return false;
        if (ReferenceEquals(y, null)) return false;
        {
            if (x is Tx tx && y is Ty ty && Equals(tx, ty)) return true;
        }
        {
            if (x is Ty ty && y is Tx tx && Equals(tx, ty)) return true;
        }
        return false;
    }

    private bool Equals(Tx x, Ty y)
    {
        if (ReferenceEquals(x, null)) return false;
        if (ReferenceEquals(y, null)) return false;
        return _equals(x, y);
    }

    public int GetHashCode(object obj)
    {
        switch (obj)
        {
            case Tx x: return _getHashCodeX(x);
            case Ty y: return _getHashCodeY(y);
            default: return 0;
        }
    }

    public static IEqualityComparer<object> Create(Func<Tx, Ty, bool> equals, Func<Tx, int> getHashCodeX, Func<Ty, int> getHashCodeY)
    {

        return new DuckEqualityComparer<Tx, Ty>(equals, getHashCodeX, getHashCodeY);
    }
}

Why didn't I use tuples? Because they don't allow me to use other then the default comparer on the property level.

Is this an acceptable solution or is there anything that could be done better?

Example

This is how I use it:

var p1 = new PersonLib1 { FirstName = "John", LastName = "Doe" };
var p2 = new PersonLib2 { FirstName = "JOHN", LastName = "Doe" };
var p3 = new PersonLib2 { FirstName = "Joh", LastName = "Doe" };

var stringComparer = StringComparer.OrdinalIgnoreCase;
var personComparer = DuckEqualityComparer<PersonLib1, PersonLib2>.Create(
    equals: (x, y) =>
        stringComparer.Equals(x.FirstName, y.FirstName) &&
        stringComparer.Equals(x.LastName, y.LastName),
    getHashCodeX: x =>
        stringComparer.GetHashCode(x.FirstName) +
        stringComparer.GetHashCode(x.LastName),
    getHashCodeY: y =>
        stringComparer.GetHashCode(y.FirstName) +
        stringComparer.GetHashCode(y.LastName)
);

personComparer.Equals(p1, p2).Dump();
personComparer.Equals(p2, p1).Dump();

personComparer.Equals(p1, p3).Dump();
personComparer.Equals(p3, p1).Dump();

You can shorten the syntax a bit and increase readability by using the is or == operators instead of ReferenceEquals:

public new bool Equals(object x, object y)
{
    if (ReferenceEquals(x, y)) return true;
    if (x is null || y is null) return false;
    {
        if (x is Tx tx && y is Ty ty && Equals(tx, ty)) return true;
    }
    {
        if (x is Ty ty && y is Tx tx && Equals(tx, ty)) return true;
    }
    return false;
}

The extra braces to make the variables local to their body is clever, however I cant exactly say I like it, it's very easy to consider the first braces to be part of the if statement above them and it can be made much more readable too if you extract it to a method:

public new bool Equals(object x, object y)
{
    if (ReferenceEquals(x, y)) return true;
    if (x is null || y is null) return false;
    return CompareObjects(x, y) || CompareObjects(y, x);
}

protected bool CompareObjects(object x, object y)
{
    return x is Tx tx && y is Ty ty && Equals(tx, ty);
}

You can also define 2 generic type arguments for the CompareObjects methods, to make it more explicit, that you're swapping the types.

private bool Equals(Tx x, Ty y)

Feels weird and out of place in my opinion, you're checking if the values of x and y are null, but you've already done so in public new bool Equals(object x, object y) and since it's a private method, that doesn't make much sense, removing this check makes it a simple call to _equals, which makes the whole method redundant, you are better off making it protected virtual, or just delete it all together

protected virtual bool Equals(Tx x, Ty y)
{
    return x != null && y != null && _equals(x, y);
}

You also lack null checks for the delegates in the Create method, but I assume you've omitted those for simplicity/


Find lowest leaf nodes in a binary tree So this is probably a popular problem, and I decided to take a crack at it today using C#.

I came up with the following solution:

private  BinaryTree<int>.BinaryTreeNode FindLowestNodes(BinaryTree<int>.BinaryTreeNode node, int depth, ref int biggestDepth) {
    this.WriteResult("Entering recursiorn for node.....{0}", node.Value.ToString());
    BinaryTree<int>.BinaryTreeNode lowestNode = null;
    if (node.Left != null) {
        var n = FindLowestNodes(node.  Left, depth + 1, ref biggestDepth);
        if (n != null) lowestNode = n;
    }
    if (node.Right != null) {
        var n = FindLowestNodes(node.Right, depth + 1, ref biggestDepth);
        if (n != null)
            lowestNode = n;
    }
    if (node.Left == null && node.Right == null) {
        if (depth > biggestDepth) {
            lowestNode = node;
            biggestDepth = depth;
        }
    }
    return lowestNode;
}

This is a recursive function using a depth parameter, and biggestDepth parameter that helps me determine if the node is indeed the lowest since the tree can have multiple depths. What I don't like about passing an int by reference is boxing...

Could this solution be written more efficiently?

Also, would it be easier to parallelize using a functional language like Clojure, or is that not really a factor as long as variables are not passed by reference?

Passing an int with the ref keyword does not involve any boxing at all. Do not confuse this with "passing an int by reference" (as in, a function with return type Object returning an int,) which is quite a different thing, and it does involve boxing.

Your code seems quite efficient to me, I have nothing to comment on.


Time complexity of finding a triangle in undirected graph Recently I posted the following solution on Stack Overflow and comments made me to rethink about the time complexity of my solution which I am sure is \$O(V+E)\$. Please suggest reasons as to why I am wrong.

import java.util.*;
class Triangle
{
 static ArrayList<LinkedList<Integer>> g;
 static int []visited;
 static int []level;
 static int []parent;
 static Queue<Integer> q=new LinkedList<Integer>();
 public static void create_graph()
 {
     int n,i,k,j;
     Scanner s=new Scanner(System.in);
     System.out.println("enter the value of number of vertices");
     n=s.nextInt();
     visited=new int[n];
     level=new int[n];
     parent=new int[n];
     for(i=0;i<n;++i)
      {visited[i]=0;level[i]=-1;}
     g=new ArrayList<LinkedList<Integer>>();
     for(i=0;i<n;++i)
     {
         g.add(new LinkedList<Integer>());
         System.out.println("enter the number of vertices adjacent to "+ (i)+" and what are they?" );
         k=s.nextInt();
         for(j=1;j<=k;++j)
           g.get(i).add(s.nextInt());
     }
 }
 public static boolean BFS(int so,int l)
 {
        int i;
        visited[so]=1;
        q.add(so);
        level[so]=l;
        while(q.size()!=0)
        {
            i=q.poll();
            for(int e:g.get(i))
             {
                if(visited[e]==0)
                {visited[e]=1;
                 level[e]=level[i]+1;
                 q.add(e);
                 parent[e]=i;
                }
                else if(level[e]==level[i]&&parent[e]==parent[i])
                 return true;
             }
        }
        return false;
 }
 public static void main(String []args)
 {
      int so,i;
      Scanner s=new Scanner(System.in);
      create_graph();
      if(BFS(0,0))
       System.out.println("The graph contains a triangle");
      else  System.out.println("The graph does not contain a triangle");
 }
}

Before I'll get to your timecomplexity let me tell you something:

Your code screams for a lint:

class Triangle
{
 static ArrayList<LinkedList<Integer>> g;

this part should become:

class Triangle {
    static ArrayList<LinkedList<Integer>> g;

no wait scratch that:

Whenever possible - specify visibility
Program against interfaces
Use speaking variables

Okay let's try again

public class Triangle {
    private static List<List<Integer>> graph;

same goes for your int[]s. On a very related note, you should encapsulate state into a class and not into the static context, where everything can modify it. Drop the static modifiers you use.

Use the final keyword to signal immutability. Use it wherever possible to reduce risk of misassigning variables.

The initialization blocks become:

private final List<List<Integer>> graph = new ArrayList<List<Integer>>();
private int[] visited;
private int[] level;
private int[] parent;
private final Queue<Integer> queue = new LinkedList<Integer>();

I usually prefer to add an empty line before a method head to help in recognizing it as new method. And also respect Java naming conventions for method names. This means: no snake_case and no SHOUTCASE. camelCase is the way to go:

public void createGraph() {

Also respect the braces placement of Java conventions. On a related note keep nice spacing around your operators. Cramping code together doesn't help readabiltiy and the times of scarce memory, where every unnecessary space was a wasted Byte are over:

Scanner s = new Scanner();

is preferrable over:

Scanner s=new Scanner();

same goes for all the operations you're doing...

While we're at Scanner. One of these Scanners is not used, neither is ever closed. Close your resources with try-with-resources, remove all of your unused variables and declare variables as close as possible to their usage.

This gets rid of:

int so, i;
Scanner s=new Scanner(System.in);
int n,i,k,j;

To make things clear, this is what your code looks like in my IDE right now:

import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
import java.util.Queue;
import java.util.Scanner;

class Triangle {

    private final List<List<Integer>> graph = new ArrayList<List<Integer>>();
    private int[] visited;
    private int[] level;
    private int[] parent;
    private final Queue<Integer> queue = new LinkedList<Integer>();

    public void createGraph() {
        try (Scanner scanner = new Scanner(System.in)) {
            System.out.println("enter the value of number of vertices");
            int vertexCount = scanner.nextInt();
            visited = new int[vertexCount];
            level = new int[vertexCount];
            parent = new int[vertexCount];
            for (int i = 0; i < vertexCount; ++i) {
                visited[i] = 0;
                level[i] = -1;
            }
            for (int i = 0; i < vertexCount; ++i) {
                graph.add(new LinkedList<Integer>());
                System.out.println("enter the number of vertices adjacent to " + (i) + " and what are they?");
                int k = scanner.nextInt();
                for (int j = 1; j <= k; ++j) {
                    graph.get(i).add(scanner.nextInt());
                }
            }
        }
    }

    public boolean triangleCheck(int so, int l) {
        visited[so] = 1;
        queue.add(so);
        level[so] = l;
        while (queue.size() != 0) {
            int i = queue.poll();
            for (int e : graph.get(i)) {
                if (visited[e] == 0) {
                    visited[e] = 1;
                    level[e] = level[i] + 1;
                    queue.add(e);
                    parent[e] = i;
                }
                else if (level[e] == level[i] && parent[e] == parent[i])
                    return true;
            }
        }
        return false;
    }

    public static void main(String[] args) {
        Triangle tri = new Triangle();
        tri.createGraph();
        if (tri.triangleCheck(0, 0)) {
            System.out.println("The graph contains a triangle");
        }
        else {
            System.out.println("The graph does not contain a triangle");
        }
    }
}

By now you're probably asking yourself "Why did I even come here, I wanted to know about time-complexity"... The answer is. I couldn't even understand your code until just now.

Honestly I still now have problems, bit I think you should get down to   \$O(V)\$, under the assumption that this code actually produces correct results..


Combining four functions for toggling gallery displays Here is my HTML:

            Ken
            Kina

This is some jQuery I set up to have the divs fadeIn/FadeOut. Is there a way to combine all of these together or a more elegant way of writing it? I feel I'm repeating things too much.

    $('.ken-gallery .gallery-home').click(function( e ){
        e.preventDefault();
        $('.ken-gallery').fadeOut(function() {
            $('.artists-home').fadeIn('slow');
        });
    });

    $('.kina-gallery .gallery-home').click(function( e ){
        e.preventDefault();
        $('.kina-gallery').fadeOut(function() {
            $('.artists-home').fadeIn('slow');
        });
    });

    $('.artist-ken').click(function( e ){
        e.preventDefault();
        $('.artists-home').fadeOut(function() {
            $('.ken-gallery').fadeIn('slow');
        });
    });

    $('.artist-kina').click(function( e ){
        e.preventDefault();
        $('.artists-home').fadeOut(function() {
            $('.kina-gallery').fadeIn('slow');
        });
    });

The way I have read the code, it looks like you can see 1 gallery at a time or the home links but not both.

It's not much of a saving in code, but you could simplify it a little and assume that if artists-home is clicked that all galleries fade out.

            Ken
            Kina

   $('.gallery-home').click(function( e ){
        e.preventDefault();
        $('.gallery').fadeOut(function() {
            $('.artists-home').fadeIn('slow');
        });
    });

Then for showing the gallery we could use a data tag instead of class names to identify the links and galleries, then you could easily extend the code to include more galleries, with no extra js

$('.artist').click(function( e ){
    e.preventDefault();

    var artist = $(this).data('artist');

    $('.artists-home').fadeOut(function() {
        $('.gallery[data-artist' + artist + ']).fadeIn('slow');
    });
});

Please note, this code is not tested, I might have missed something


TODO in React w/ Redux Please critique my TODO App in React w/ Redux:

// React Component
const TodoApp = ({todos, inputText, onInputChange, onAdd, onDelete}) => {
    const numbers = range(todos.length);
    const zipped  = zip(numbers, todos);

    return (

TODO!

{zipped.map( (z) =>

                        {z[1].text}
                        <button onClick={() => onDelete(z[0])}>delete</button>

                )}

                    <input type="text" value={inputText} onChange={onInputChange}/>
                    <button onClick={onAdd}>add</button>

    );
};

// Redux (State)
const ADD = 'ADD';
const UPDATE_INPUT_TEXT = 'UPDATE_INPUT_TEXT';
const DELETE='DELETE';

function todo(state = {todos: [], inputText: ""}, action) {
    switch(action.type) {
        case ADD:
            return {
              todos: state.todos.concat( {
                text: state.inputText
              }),
              inputText: ''
            };
        case UPDATE_INPUT_TEXT:
            return {
                todos: state.todos,
                inputText: action.newInputText
            };
        case DELETE:
            const filtered = state.todos.filter( (e, index) => index !== action.index);
            return {
                todos: filtered,
                inputText: ''
            };
        default:
            return state;
    }
}

const store = createStore(todo);

const renderTodo = () => {
    const st = store.getState();
    ReactDOM.render(
        <TodoApp todos={st.todos}
                 inputText={st.inputText}
                 onInputChange={event =>
                     store.dispatch({type: UPDATE_INPUT_TEXT, newInputText: event.target.value})
                 }
                 onAdd={() => store.dispatch({type: ADD})}
                 onDelete={(index) => store.dispatch({type: DELETE, index: index})}

        />,
        document.getElementById('root')
    )
};

store.subscribe(renderTodo);
renderTodo();

I'm not exactly sure of your environment (I'm going to assume you're using Node), and with this assumption, I want to address a few things starting with specific things, then more general:

Specific

You shouldn't be keeping inputText in the global store. This creates lots of store dispatches and updates which are completely unnecessary and costly. Instead, pass the input value once you want to add a todo. This would entail the use of state in TodoApp to control the input so you'd have to change to an ES6 class, but that's favorable to the unnecessary updating of the Redux store.
There's no need to do the two lines:

const numbers = range(todos.length);
const zipped  = zip(numbers, todos);

You can shorten your Array#map callback to this without those two lines:

{
  todos.map(({ text }, index) =>

      {text}
      <button onClick={() => onDelete(index)}>delete</button>

  )
}

General

Organization - Separate your Redux reducers, actions, etc. into different files for organization. I'm not sure if your project is currently in one file or not, but this is general advice. This app may be a minor one, but it's good practice for larger apps to keep things organized. For example, a directory structure I would use in this case looks like this:

src/
├── actions/
│   ├── actions.js
│   └── actionCreators.js
├── reducers/
│   └── todos.js
├── components/
│   └── Todos.js
└── index.js

Where action.js contains all your actions, exported using ES6 modules (which I name accordingly for uniformity):

export const DELETE_TODO = 'DELETE_TODO';
export const ADD_TODO = 'ADD_TODO';

And actionCreators.js contains all the action creators, in other words, returns an action to be dispatched. Right now, you're using object literals left and right, which isn't very consistent. It also doesn't convey a message at a glance. I would use action creators instead, for example, if you're adding a todo:

export const addTodo = text => ({
  type: ADD_TODO,
  text
});

Then you can dispatch the action like so:

dispatch(addTodo('foo'));

Instead of like this everytime:

dispatch({ type: ADD_TODO, text: 'foo' });

This can be applied also to another action creator named deleteTodo. And in this new structure, todos.js would be a separate file for your reducer. This is good practice in preparation for larger apps, where you may have more than one reducer. Finally Todos.js is your actual React component and index.js is where you create the store and call ReactDOM.render.
react-redux - Don't subscribe to state like that -- sure, it works but there's a more canonical pattern to use. That's where the react-redux comes in. Redux doesn't have anything to with React itself, but integrates well with React with the react-redux package. This is because Redux emits updates (via store.subscribe) when you dispatch and action and generate new state, which react-redux can take advantage to readably connect your components to state without having to recall ReactDOM.render every time the store changes. This allows for scaling to larger applications with more components.

So in react-redux there's the connect higher-order component that provides state to a certain child, connecting it to the Redux store. You provide it two arguments: mapStateToProps and mapDispatchToProps. What each does can be found in more detail at the GitHub page, but they essentially take values and the dispatch function from the Redux store and pass them as props to a certain component, the same as what you're doing now, except all in a neat package. But connect has to get the store from somewhere, so react-redux also provides a <Provider> component to pass down the Redux store to ancestors while not having to physically do store.getState(). This will get you this in index.js:

import { Provider } from 'react-redux';
import Todos from './components/Todos.js';
import todos from './reducers/todos.js';

const store = createStore(todos);

ReactDOM.render(
  <Provider store={store}>
    <Todos />
  </Provider>,
  …
);

Then in Todos.js:

class Todos extends React.Component {
  constructor() {
    this.state = {
      inputText: ''
    };
  }

  handleChange(e) {
    this.setState({
      inputText: e.target.value
    });
  }

  render() {
    const { todos, onAdd, onDelete } = this.props;

    return (

TODO!

          {
            todos.map(({ text }, index) => (

                {text}
                <button onClick={() => onDelete(index)}>delete</button>

            )
          }

            <input type="text" value={this.state.inputText} onChange={e => this.handleChange(e)}/>
            <button onClick={() => onAdd(this.state.inputText)}>add</button>

    );
  }
}

const mapStateToProps = ({ todos }) => ({ todos }); //provides this.props.todos
const mapDispatchToProps = dispatch => ({
  onAdd(text) {
    dispatch(addTodo(text)); //import this from ../actions/actionCreators.js
  },
  onDelete(index) {
    dispatch(deleteTodo(index));
  }
});

export default connect(mapStateToProps, mapDispatchToProps)(Todos); //connects component to Redux store

Finally, this will clean up the reducer in todos.js for you:

export default function todos(state = { todos: [] }, action) {
  switch(action.type) {
    case ADD_TODO: //import from ../actions/actions.js
      return {
        todos: state.todos.concat({
          text: action.text
        })
      };
    case DELETE_TODO:
      const filtered = state.todos.filter((e, index) => index !== action.index);
      return {
        todos: filtered
      };
    default:
      return state;
  }
}


Zepto / jQuery plugin base I am looking for a starting point to code plugins, primarily  for Zepto.js (with fall back for jQuery). These will provide reusable functions for Tumblr theming.

However, I can't seem to find anything in depth about writing a plugin (with Zepto) as the example, apart from a small snippet on the Zepto site: http://zeptojs.com/

The code below is pieced together from reading various 'jQuery' plugin tutorials, but I am not 100% sure how solid it is.

;(function($){
    $.extend($.fn, {
        pluginName: function(el, options) {
            // Set defaults
            this.defaults = {
                option: 'option',
                onComplete: function() {}
            };
            // Combine defaults / options
            var settings = $.extend({}, this.defaults, options);
            // Do stuff
            $.each(this, function() {

            });
            // Call back if needed
            settings.onComplete.call(this);
            // Return `this` for chain ability
            return this;
        }
    });
}(window.Zepto || window.jQuery));

According to the Zepto documentation, my plugins method will be added to the Zepto object. Is this acceptable?

I would also like the option to use private / public functions contained in the plugin for flexibility.

I apologise if my questions come across as confusing or novice, but any help / improvements would be greatly appreciated.

This looks fine to me, but then again it's sample (boilerplate) code ;)

Consider "use strict";
If it was not for the onComplete, you could simply return the result of $.each

I like your options handling, graceful fallback to jQuery, style, it's all good.


Printing all the permutations of a string in alphabetical order-ver1.1 I adapted most of the suggestions from answers for my previous question Printing all the permutations of a string in alphabetical order to rewrote my solution again. Please let me know any more changes required or any more suggestions.

#include <iostream>
#include<fstream>
#include<algorithm>
#include<string>

void create_permute(  std::string& record )
{
    std::sort( record.begin(), record.end() );
    std::cout<<record;

    while( std::next_permutation( record.begin(), record.end() ) )
    {
        std::cout<<","<<record;
    }
    std::cout << "\n";
}
void readInputFile( std::string filename )
{
    std::ifstream infile(filename);
    std::string record;
    while( std::getline( infile,record ) )
    {
       create_permute( record );
    }
    infile.close();
}

int main( int argc, char* argv[] )
{
    if( argc < 2 )
    {
        std::cout << "usage: filesize filename" << "\n";
        exit( 0 );
    }
    std::ios_base::sync_with_stdio( false );
    readInputFile( argv[ 1 ] );

    return 0;
}

Nothing I would bother about complaing about in real code review.

But for argument's sake:

Pass by const ref

Rather than passing by value. It usually means that an extra copy will be removed. In this case that's not true but I think its a good habit.

void readInputFile( std::string const& filename )

No need to return 0 from main

If main() can not return anything else. Then I don't bother with a return statement in main (it's special). The compiler adds a return 0 if no return is provided for main.

Personally I use this (that face that I have not added a return) to indicate that the application has no real failure state.


Utilization of the Multiton pattern See Wikipedia - Multiton Pattern for details on the intent.

Here's the class:

/**
 * Holds a thread-safe map of unique create-once items.
 *
 * Contract:
 *
 * Only one object will be made for each key presented.
 *
 * Thread safe.
 *
 * @param <K>
 * @param <V>
 */
public class Multiton<K, V> {
  // Map from the key to the futures of the items.
  private final ConcurrentMap<K, Future<V>> multitons = new ConcurrentHashMap<K, Future<V>>();
  // A Prayer is used to construct the Callable that is attached to the FutureTask.
  private final Prayer prayer;

  public Multiton(Creator<K, V> creator) {
    // Create the prayer to the creator.
    this.prayer = new Prayer(creator);
  }

  /**
   * There can be only one.
   *
   * Use a FutureTask to do the creation to ensure only one construction.
   *
   * @param key
   * @return
   * @throws InterruptedException
   * @throws ExecutionException
   */
  public V getInstance(final K key) throws InterruptedException, ExecutionException {
    // Already made?
    Future<V> f = multitons.get(key);
    if (f == null) {
      // Plan the future but do not create as yet.
      FutureTask<V> ft = new FutureTask<V>(prayer.pray(key));
      // Store it.
      f = multitons.putIfAbsent(key, ft);
      if (f == null) {
        // It was successfully stored - it is the first (and only)
        f = ft;
        // Make it happen.
        ft.run();
      }
    }
    // Wait for it to finish construction and return the constructed.
    return f.get();
  }

  /**
   * Use a Prayer to pass the key to the Creator.
   *
   * @param <K>
   * @param <V>
   */
  private class Prayer {
    private final Creator<K, V> creator;

    public Prayer(Creator<K, V> creator) {
      this.creator = creator;
    }

    public Callable<V> pray(final K key) {
      return new Callable<V>() {

        @Override
        public V call() throws Exception {
          return creator.create(key);
        }

      };
    }

  }

  /**
   * User provides one of these to do the construction.
   *
   * @param <K>
   * @param <V>
   */
  public abstract static class Creator<K, V> {
    // Return a new item under the key.
    abstract V create(K key) throws ExecutionException;

  }

}

I use it to ensure I only create one JAXBContex for each class:

/**
 * Create just one JAXB context for each class.
 */
private static final Multiton<Class, JAXBContext> contexts = new Multiton<Class, JAXBContext>(new Creator<Class, JAXBContext>() {

  @Override
  JAXBContext create(Class key) throws ExecutionException {
    try {
      // Get it out of my Multiton
      return JAXBContext.newInstance(key);
    } catch (JAXBException ex) {
      // Pass it back as an Execution Exception.
      throw new ExecutionException("Failed to create JAXB Context for class "+key.getName(), ex);
    }
  }

});

/**
 * Uses a Multiton to create a JAXB context for the object.
 *
 * @param it - The object to create the context for.
 * @return - the unique JAXB context.
 * @throws InterruptedException - Interrupted.
 * @throws ExecutionException - Something else went wrong (cause is wrapped)
 */
private static JAXBContext createJAXBContext(Object it) throws InterruptedException, ExecutionException {
  return contexts.getInstance(it.getClass());
}

My Questions:

Is this a Multiton as described by the article? I.e. Will it only ever create just one instance of the object?
Is it thread-safe?
Am I being offensive using a Prayer object to request an object from the Creator?
If the answer to 3 is no - should I make my create method of the Creator take a Prayer as a parameter rather than a K key - just for the belly laugh of course?

NB: The idea of using a FutureTask came from a nice answer by assylias on thread-safe multitons. All I've really done is make it generic and plumbed in the ability to pass a Creator.

I think your use-case and the Wiki description are different. Technically, the Multiton should return the exact same intance for the exact same key, always, but, because your multitons Map is not a static instance, you can simply create multiple instances of the Multiton and each one will generate different instances for the same key.

This is OK in your case, because you create a private static final anonymous implementation of the class.... so you handle that problem at a higher level.

I think, otherwise, the algorithm is good.... (i.e. it will work as advertised).

There are two changes I can recommend....

Why is the Creator<K,V> a public abstract static class? This is a broken pattern, and it should simply be an interface public interface Creator<K,V>
This is complicated, the rest of the answer is about change 2....

I believe you have unnecessary indirection. Your inner class:

  private class Prayer {
    private final Creator<K, V> creator;

    public Prayer(Creator<K, V> creator) {
      this.creator = creator;
    }

    public Callable<V> pray(final K key) {
      return new Callable<V>() {

        @Override
        public V call() throws Exception {
          return creator.create(key);
        }

      };
    }

  }

This is overkill.... if you make the Prayer class Extend Callable, you win:

  private class Prayer implements Callable<V> {
    private final K key;

    public Prayer(K key) {
      this.key = key;
    }

    @Override
    public V call() throws Exception {
      // creator is accessible as a final field on the Multiton instance.
      return creator.create(key);
    }

  }

Then, instead of keeping a final instance to Prayer in your main class:

private final Prayer prayer;

you should instead hold the simple Creator:

private final Creator<K,V> creator;

And, instead of calling prayer.pray(), you only need do:

FutureTask<V> ft = new FutureTask<V>(new Prayer(key));

Note: It is much better to request a new Prayer(...) than it is to demand one with prayer.pray() !!!

Hope this all makes sense.... I have reworked your original code as:

import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;

public class Multiton<K, V> {
  // Map from the key to the futures of the items.
  private final ConcurrentMap<K, Future<V>> multitons = new ConcurrentHashMap<K, Future<V>>();
  // A Creator is used to construct the Callable that is attached to the FutureTask.
  private final Creator<K,V> creator;

  public Multiton(Creator<K, V> creator) {
    this.creator = creator;
  }

  /**
   * There can be only one.
   *
   * Use a FutureTask to do the creation to ensure only one construction.
   *
   * @param key
   * @return
   * @throws InterruptedException
   * @throws ExecutionException
   */
  public V getInstance(final K key) throws InterruptedException, ExecutionException {
    // Already made?
    Future<V> f = multitons.get(key);
    if (f == null) {
      // Plan the future but do not create as yet.
      FutureTask<V> ft = new FutureTask<V>(new Prayer(key));
      // Store it.
      f = multitons.putIfAbsent(key, ft);
      if (f == null) {
        // It was successfully stored - it is the first (and only)
        f = ft;
        // Make it happen.
        ft.run();
      }
    }
    // Wait for it to finish construction and return the constructed.
    return f.get();
  }

  /**
   * Use a Prayer to pass the key to the Creator.
   *
   * @param <K>
   * @param <V>
   */
  private class Prayer implements Callable<V> {
    private final K key;

    public Prayer(K key) {
      this.key = key;
    }

    @Override
    public V call() throws Exception {
      return creator.create(key);
    }

  }

  /**
   * User provides one of these to do the construction.
   *
   * @param <K>
   * @param <V>
   */
  public interface Creator<K, V> {
    // Return a new item under the key.
    abstract V create(K key) throws ExecutionException;

  }

}


Generalization of any() function with switchable default parameter for empty iterables I have some use-cases where I want the behaviour of the build-in any() function to be slightly different, namely to return True if the iterable is empty.
Therefore I came up with the following override:
_any = any  # Alias built-in any.

def any(iterable, *, default=False):
    """Generalization of the built-in any() function."""

    if not default:
        return _any(iterable)

    count = 0

    for count, item in enumerate(iterable, start=1):
        if item:
            return True

    return count == 0

The code I used to test this is:
EMPTY_TUPLE = ()
FALSEY_TUPLE = (False, None, 0, '')
TRUEY_TUPLE = (False, None, 1, '')

def empty_iterables():
    """Yields empty iterables."""

    yield list(EMPTY_TUPLE)
    yield EMPTY_TUPLE
    yield {item: item for item in EMPTY_TUPLE}
    yield set(EMPTY_TUPLE)
    yield (i for i in EMPTY_TUPLE)

def falsey_iterables():
    """Yields iterables that any() should assert as False."""

    yield list(FALSEY_TUPLE)
    yield FALSEY_TUPLE
    yield {item: item for item in FALSEY_TUPLE}
    yield set(FALSEY_TUPLE)
    yield (i for i in FALSEY_TUPLE)

def truey_iterables():
    """Yields iterables that any() should assert as True."""

    yield list(TRUEY_TUPLE)
    yield TRUEY_TUPLE
    yield {item: item for item in TRUEY_TUPLE}
    yield set(TRUEY_TUPLE)
    yield (i for i in TRUEY_TUPLE)

def test():
    """Tests the custom any() function."""

    # Built-in behaviour.
    for iterable in empty_iterables():
        assert any(iterable) is False

    for iterable in falsey_iterables():
        assert any(iterable) is False

    for iterable in truey_iterables():
        assert any(iterable) is True

    # Custom behaviour.
    for iterable in empty_iterables():
        assert any(iterable, default=True) is True

    for iterable in falsey_iterables():
        assert any(iterable, default=True) is False

    for iterable in truey_iterables():
        assert any(iterable, default=True) is True

if __name__ == '__main__':
    test()

I wonder whether there's a more elegant way to implement the custom any() function with the aforementioned behaviour.
Use case
I have a custom monitoring system, that determines the state of a system using check records from a MariaDB database. I use peewee as an ORM:
def check_customer_system(system):
    """Returns the customer online check for the respective system."""

    end = datetime.now()
    start = end - CUSTOMER_INTERVAL
    condition = OnlineCheck.system == system
    condition &= OnlineCheck.timestamp >= start
    condition &= OnlineCheck.timestamp <= end
    query = OnlineCheck.select().where(condition)

    if query:
        return any(online_check.online for online_check in query)

    raise NotChecked(system, OnlineCheck)

In the current implementation, I need to check, whether the amount of records in the query in non-empty by doing if query:.
However, this will execute the query an load all records into RAM, which I want to avoid. Peewee has the optional .iterator() method on queries, which will return an iterator, that will not load all records into RAM at once. However, if I use that, if query: will always be True since it's testing the truth value of a generator object then.
Hence, I came up with the idea to implement a function, that returns True if any value of an iterable is True or the iterable is empty.
And I just realized that this is indeed an XY-Problem, since I'd loose the ability to distinguish between a checked and a non-checked system.
Thanks @Eric Duminil.

I really think this is a bad idea. A function called any returning True if there not only isn't anything true but even not anything at all... that's just wrong. It sure isn't a "generalization". Shadowing the built-in is not a good idea, either. Maybe just call yours any_or_empty (without a default)?
For the emptiness check, the counting seems like overkill and I think a Boolean is clearer (and probably faster, due to not going through enumerate):
    empty = True

    for item in iterable:
        if item:
            return True
        empty = False

    return empty

Or with a sentinel, to get rid of the assignment in the loop:
    item = sentinel = object()

    for item in iterable:
        if item:
            return True

    return item is sentinel

For best speed, you could just handle the first item yourself and then let the built-in handle the rest:
def any(iterable, *, default=False):
    """Modification of the built-in any() function."""

    it = iter(iterable)
    sentinel = object()
    first = next(it, sentinel)
    if first is sentinel:
        return default
    return bool(first) or _any(it)

Your test code uses the words "truey" and "falsey". That's not Python terminology. See the Python docs, you'll never find those words. It's true and false, as you can for example see at if, Boolean operations and Truth Value Testing.
The name falsey_iterables is ambiguous if not misleading, as it doesn't yield any false iterables. They're all true. They just don't contain anything true. I'd suggest iterables_without_trues. And iterables_with_true instead of truey_iterables for consistency.
Did you have this idea because next, min and max offer a default parameter? In their cases it makes sense, as they rather need non-empty inputs. And meaningful defaults are possible there, for example min with default=float('inf') could be a useful default for the code using the result.


FizzBuzz Solution Rags to Different Rags I took some code and reviewed it, but it got me thinking, so I re wrote the code a little bit and would like to ask what you think of it and why it's bad to do this, even though it works.

public class FizzBuzz{
    public static void main(String[] args){
        for(int i = 1 ; i < 101; i++)
            if(i % (5*3) == 0)
                System.out.println("FizzBuzz");
            else if(i % 3 == 0)
                System.out.println("Fizz");
            else if(i % 5 == 0)
                System.out.println("Buzz");
            else
                System.out.println(i);
    }
}

I removed braces because these are all essentially one-line statements and don't need them, so what does this say about my Java coding style?

Taking your question at face value:

  I removed braces because these are all essentially one-line statements and don't need them, so what does this say about my Java coding style?

They are not essentially 1-line statements. Your assumptions are incorrect.

An if is a statement, and a System.out.println is another statement. Thus, you have two statements there... now, the code segment:

        if(i % (5*3) == 0)
            System.out.println("FizzBuzz");

represents two statements on two lines... but, the statement:

       else if(i % 3 == 0)

technically merges one if statement (with an else clause) in to a second if statement. You thus have two statements on one line.

What conclusion can you draw from there? That code style does not depend on the number of statements on one line, because it is normal, and acceptable to have an else and a new if on a single line.

Thus, the fact that it is a 1-liner or not is not the issue. The issue relates to readability, and maintainability.

Your code is readable, sure, but is it maintainable? No.

Maintenance requires making changes. Now, the base reason I have against the un-braced 1-liners, is that maintenance often requires breaking a single statement in to multiple.

Thus, a line like:

        if(i % (5*3) == 0)
            System.out.println("FizzBuzz");
        else if (....)

may need to become:

        if(i % (5*3) == 0) {
            System.out.println("FizzBuzz");
            dualmatch++;
        } else if (....)

Note, now, that to make a single change to count the number of times that the double-modulo has happened (FizzBuzz), we have added a single line of code, but also modified 2 other lines.

Thus, we have had to modify two lines, and add a third, just to add one line.

Now, what does this mean?

It means someone looking through a 'diff' to see what changed, sees a diff on the line:

        if(i % (5*3) == 0) {

and also on the line:

        } else if (....)

when, in fact, nothing in those lines of any consequence changed.....

By not adding the braces when you created the code, you have made the maintainability of the code much more difficult, because each maintenance requires a lot more effort, and a lot more confusion when trying to identify code progression.

What saved you 2 seconds when you wrote the code, is going to create 5 minutes of effort later for every single person who investigates why you changed lines of code, that you did not actually change.... and those 'unchanged' but different lines are the most complicated parts of the code...

By not adding the braces, you are essentially saying, "My 2-seconds saved is worth more than the hours? of time that someone else has to consume understanding revisions of my code).

Now, if that is not enough, whenever work is done, bugs can be introduced, and the amount of bugs is typically proportional to the complication and effort required to implement a change....

By making changes to your code more difficult, you are increasing the amount of bugs that result from your laziness, because other people need to correct both the code, and your laziness in the future.

Note here is the diff for adding in a count of the number of FizzBuzz's using your code:

Now compare that diff with what it looks like if the bracing is right to start with:


Basic JSON Representation in Rust This week I started reading "The Rust Programming Language". As I reached the chapters on enumerations and pattern matching I felt I had enough material to put together a simple representation of JSON in Rust, just to play around and get a better feeling of the language.

This is what I came up with:

use std::fmt;

pub enum Json {
    Obj(Vec<(String, JsonVal)>),
    Arr(Vec<JsonVal>),
}

pub enum JsonVal {
    Str(String),
    Num(f64),
    Composite(Json),
    Bool(bool),
    Null,
}

#[allow(unused_must_use)]
impl fmt::Display for Json {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            Json::Obj(ref obj) => {
                "{".fmt(f);
                for (n, prop) in obj.iter().enumerate() {
                    if n != 0 {
                        ",".fmt(f);
                    }
                    "\"".fmt(f);
                    prop.0.fmt(f);
                    "\":".fmt(f);
                    prop.1.fmt(f);
                }
                "}".fmt(f);
                Result::Ok(())
            }
            Json::Arr(ref arr) => {
                "[".fmt(f);
                for (n, item) in arr.iter().enumerate() {
                    if n != 0 {
                        ",".fmt(f);
                    }
                    item.fmt(f);
                }
                "]".fmt(f);
                Result::Ok(())
            }
        }
    }
}

impl fmt::Display for JsonVal {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f,
               "{}",
               match *self {
                   JsonVal::Str(ref string) => format!("\"{}\"", string),
                   JsonVal::Num(number) => number.to_string(),
                   JsonVal::Composite(ref json) => json.to_string(),
                   JsonVal::Bool(boolean) => boolean.to_string(),
                   JsonVal::Null => String::from("null"),
               })
    }
}

The things that "smell" a little bit, in my humble opinion, are three:

The warning suppression before the implementation of the Display trait for Json: I tried aggregating the operations in Vectors and then folding them together but the resulting code looked unnecessarily garbled, so I just ignored the Results. Is there a better way to do this?
In the Display implementation of JsonVal I use the format! macro to basically make a copy of the string (a "technique" which smells by itself); however, as I'm not touching the string in any way, it would be great to just hand out the string itself to the write! macro; returning *string wouldn't make sense as I would be trying to perform a move on a borrowed reference, but just returning the reference would mean that in other branches I'd have to return a reference as well, which gets rejected because the result of the calls to the to_string method go out of scope after the pattern matching. Is there a way out of this, perhaps using lifetimes?
In the Display implementation of JsonVal I use the String::from associated function to return the value of JSON's null. However, if I'm not mistaken, this would mean that I would be creating a new instance of the String every time I have to format a null. A str, being 'static (again, if I'm not mistaken), would solve the problem. Is the something like a static String or a way to be more efficient, perhaps using (again) lifetimes?

Also, I put all the refs because the compiler hinted me to do so and I understand more or less that it has to do with the fact that I'm just borrowing what's inside the struct to use it, but it's not completely clear to me. However, I still have to encounter the ref keyword on the book, so don't bother explaining if you feel like I'm better off RTFM.

You can find an updated version of this code on my Github repo.

There are no tests. That means that all of the changes I show compile, but may not do what I say. Rust makes is very easy to write basic tests, so I'd encourage you to write them. Especially as you are learning a new language and especially when you ask for feedback or refactor the code.
You should never ignore Results. If you don't believe it's possible for an error to occur, use unwrap to cause the program to abort on error. Even better would be to use try! to return an error in case of error.
Result::Ok(()) is duplicated at the end of each method; move it after the match.
Result::Ok is imported via the prelude, you can just say Ok.
Multiple consecutive fmt calls can be combined with the write! macro and a format string.
Instead of using to_string, embed the write! call inside the match arms. Then there are no additional heap allocations.
Consider dealing with the middle commas in a different way. Pop off the first element, then unconditionally add the comma for all subsequent ones. Theres a possibility of performance difference here, so you should test.
Consider using an Option instead of creating a Null type. Option is a pervasive type from the standard library and people innately know how to use it.
I think you will have a bug if a string contains embedded quotes, as there's no escaping happening.

use std::fmt;

pub enum Json {
    Obj(Vec<(String, JsonVal)>),
    Arr(Vec<JsonVal>),
}

pub enum JsonVal {
    Str(String),
    Num(f64),
    Composite(Json),
    Bool(bool),
    Null,
}

impl fmt::Display for Json {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            Json::Obj(ref obj) => {
                try!("{".fmt(f));
                let mut props = obj.iter();
                if let Some(prop) = props.next() {
                    try!(write!(f, r#""{}":{}"#, prop.0, prop.1));
                }
                for prop in props {
                    try!(write!(f, r#","{}":{}"#, prop.0, prop.1));
                }
                try!("}".fmt(f));
            }
            Json::Arr(ref arr) => {
                try!("[".fmt(f));
                let mut items = arr.iter();
                if let Some(item) = items.next() {
                    try!(item.fmt(f));
                }
                for item in items {
                    try!(write!(f, ",{}", item));
                }
                try!("]".fmt(f));
            }
        }

        Ok(())
    }
}

impl fmt::Display for JsonVal {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            JsonVal::Str(ref string) => write!(f, r#""{}""#, string),
            JsonVal::Num(number) => number.fmt(f),
            JsonVal::Composite(ref json) => json.fmt(f),
            JsonVal::Bool(boolean) => boolean.fmt(f),
            JsonVal::Null => "null".fmt(f),
        }
    }
}

fn main() {

}

  the warning suppression before the implementation of the Display trait

We mentioned this one already; good call on thinking that something was wonky here.

  In the Display implementation of JsonVal I use the format! macro to basically make a copy of the string
  In the Display implementation of JsonVal I use the String::from associated function to return the value of JSON's null.

Again, good call on both of these points. Creating an allocated object is sometimes the right thing, but in this case we can just delegate to the existing Display implementations of various types. In case you weren't aware, the {} corresponds to items that implement Display. That's what makes it possible to just call out to those.

  I put all the refs because the compiler hinted me to do so and I understand more or less that it has to do with the fact that I'm just borrowing what's inside the struct to use it

Yes, this is the right intuition. When formatting something, you don't have ownership of the item, only a reference. That means you can't take ownership of a struct member, only further references. The ref keyword in a pattern match is conceptually equivalent to taking another reference on the right side:

struct Foo(String);
let f = Foo("hello".to_string());

let String(ref s) = f;
let s = &f.0; // equivalent

The problem is that you can't always write code like the second case, like in a multi-way pattern match or in one arm of a match, so the ref keyword is needed.


Webservice methods using EclipseLink and database entities I am wondering if this is a good and correct way to write the implementation of two web service methods (get/set methods).

The setPerson method can be called from different threads from a pool, so I think the best way is to make the method synchronized. Or is it possible to lock the table when editing to avoid the synchronization on Java?

@Override
public synchronized void setPerson(Person person) {
    EntityManagerFactory emf = Persistence.createEntityManagerFactory("PersonLibPU");
    EntityManager em = emf.createEntityManager();
    if(!em.getTransaction().isActive()) {
       em.getTransaction().begin();
    }
    try {
         em.merge(person);
         em.getTransaction().commit();
         emf.getCache().evict(Person.class);
    } catch (Exception ex) {
         if(em.getTransaction().isActive())
            em.getTransaction().rollback();
    } finally {
        em.close();
    }
}

And here follows the getPerson method. I didn't make it synchronized since it reads from the JPA cache.

@Override
public Person getPerson(int personId) {
    EntityManagerFactory emf = Persistence.createEntityManagerFactory("PersonLibPU");
    EntityManager em = emf.createEntityManager();
    try {
        return (Person)em.getReference(Person.class, personId);
    } catch (javax.persistence.EntityNotFoundException ex) {
        return new Person(); // return empty person
    } finally {
        em.close();
    }
}

Just a few notes:

if(!em.getTransaction().isActive()) {
   em.getTransaction().begin();
}
try {
     em.merge(person);
     em.getTransaction().commit();
     emf.getCache().evict(Person.class);
} catch (Exception ex) {
     if(em.getTransaction().isActive())
        em.getTransaction().rollback();
} finally {
    em.close();
}

I'd create a local variable for em.getTransaction() here and move the isActive check inside the try block.

try {
    EntityTransaction transaction = em.getTransaction();
    try {
        if (!transaction.isActive()) {
            transaction.begin();
        }
        em.merge(person);
        transaction.commit();
        emf.getCache().evict(Person.class);
    } finally {
        if (transaction.isActive()) {
            em.transaction.rollback();
        }
    }
} finally {
    em.close();
}

It closes the EntityManager in every code path as well as the transaction.

I've removed the exception catching. If you catch it consider at least logging it.

Evicting the whole Person cache every time you save a Person entity does not seem a good idea for performance.

Please note that if something call this method with an active transaction (I'm not sure if it's possible at all) it will returns with a committed or rollbacked, but not active transaction.


Recursive JavaScript permutations function I have a recursive function that calculates the permutations of a given list/array list. Although a similar implementation works great in Python, this JavaScript implementation cannot handle a list of more than 7 elements before its webpage kills it.
I realise this is a natural effect of the recursion, but I am only making \$n\$ recursive calls for a list of size \$n\$. Is there any JavaScript performance magic that could be used to save this function, even if only for a handful more cases?
function permute(list) {
    if (list.length == 1) { return [list] }
    let permutations = []
    let subpermutations = permute(list.slice(1, list.length))
    for (index in subpermutations) {
        let sublist = subpermutations[index]
        for (let pos = 0; pos < sublist.length+1; pos++) {
             permutations.push(sublist.slice(0, pos)
                         .concat([list[0]])
                         .concat(sublist.slice(pos, sublist.length)));
        }
     }
     return permutations;
}

Remarks

The algorithm used seems vastly ineffective (lots of sliceing and concating greatly contributes to it),
The strict equality operator shall be used in place of the equality operator wherever applicable,
Use the for...of statement instead of for...in.

Rewrite (ES2015+)
The below rewrite is my implementation of the non-recursive variant of the Heap's algorithm, which might be the most effective algorithm for the job. Another noteworthy one is the "Steinhaus–Johnson–Trotter algorithm", but it's more complicated and less performant in practice.
Please note, many portions of the below code could have been written in a far more idiomatic way, e.g. the swap function could have been completely replaced with a destructuring assignment which is far more concise, but that would come at a price of performance, which in this case is a critical point.
const permute = arr => {
  const permutations = [];

  const swap = (a, b) => {
    const tmp = arr[a];
    arr[a] = arr[b];
    arr[b] = tmp;
  };

  const c = new Array(arr.length).fill(0);
  permutations.push(arr.slice());

  let i = 0;
  while (i < arr.length) {
    if (c[i] < i) {
      swap(i, i % 2 ? c[i] : 0);
      permutations.push(arr.slice());

      c[i] += 1;
      i = 0;
    }
    else {
      c[i] = 0;
      i += 1;
    }
  }

  return permutations;
};

Benchmark
The times change, try it yourself!

Original ― 69,330.36 ops/s ± 1.05% (93.9% slower)
The above implementation ― 1,135,764.38 ops/s ±2.29% (fastest)


A custom Java collection which doesn't support nulls I am making a GWT application and have to prevent client code from using null values in collections.
I found an answer but GWT doesn't support Queue and its implementations as the GWT documentation says.

So, I had to make my own implementation.

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;

/**
 * This implementation of {@link ArrayList} does not allow to add null values
 *
 * @author Maxim Dmitriev
 *
 * @param <E>
 */
public class NonNullArrayList<E> extends ArrayList<E> implements Serializable {

    private static final String NULL_VALUES_ARE_PROHIBITED = "Null values are prohibited";
    private static final String CLONING_IS_UNSUPPORTED = "Cloning is unsupported. Please support if necessary";

    public NonNullArrayList() {
        // For serialization. super() is called automatically.
    }

    public NonNullArrayList(int initialCapacity) {
        super(initialCapacity);
    }

    public NonNullArrayList(Collection<? extends E> c) {
    super(c);
        if (contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
    }

    /**
     *
     */
    private static final long serialVersionUID = 7716772103636691126L;

    public boolean add(E e) {
        if (e == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.add(e);
    };

    @Override
    public boolean addAll(Collection<? extends E> c) {
        if (c.contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.addAll(c);
    }

    public void add(int index, E element) {
        if (element == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        super.add(index, element);
    };

    @Override
    public boolean addAll(int index, Collection<? extends E> c) {
        if (c.contains(null)) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.addAll(index, c);
    }

    public E set(int index, E element) {
        if (element == null) {
            throw new NullPointerException(NULL_VALUES_ARE_PROHIBITED);
        }
        return super.set(index, element);
    };

    @Override
    public Object clone() {
        throw new Error(CLONING_IS_UNSUPPORTED);
    }

}

Besides I want to prevent code from using the clone() method because I just don't need it.

Is the implementation good or not?

Throwing NullPointerException from methods like add, set are OK, cause the element to be added can't be null. But throwing NPE from addAll method is not cool. From Javadoc

  Throws: NullPointerException - if the specified collection is null

that means if there exists a null value in the Collection throwing NPE doesn't make sense. You can throw IllegalArgumentException or a custom exception like FoundNullValueInCollectionException(I think this is better).

In your defense you can point towards the Map.containsKey Javadoc, where it throws NPE if the specified key is null and this map does not permit null keys (optional).

  I want to prevent code from using the clone() method

So you can throw a UnsupportedOperationException or CloneNotSupportedException(as JohnMark13 suggested).


Using a dependency container to load a parent/child class In PHP, I have a parent/child class and am using dependency container to load them.  I am trying to use the same database connection on all of them without duplicating the object.

I used $this->_db to pull data from the database in a child class method (and my class stores that in the property $this->_db->result). I then went into the parent class and print_r($this->_db->result) and it worked. That leads me to believe that I am passing the database object and not duplicating it (maybe?).

However, would still be nice if someone could tell me why my code would not be considered OOP, as someone had commented that it was not OOP. Can someone who understands this stuff better than me please have a look and let me know if I'm doing it right and if not, what I need to change in my code?

<?php

    include('mysqliClass.php');
    $db = new mysqliObject();

    depCon::$_database = $db;
    $myObject = depCon::createNewObject();

    class depCon {

      public static $_database;

      public static function createNewObject() {

        $newObject = new myChild();
        $newObject->setDatabase(self::$_database);

        return $newObject;
      }
    }

    class myParent {

      private $_db;

       public function __construct() {
        }

      public function setDatabase($databaseConnection) {
        $this->_db = $databaseConnection;
      }

    }

    class myChild extends myParent {

        private $_db;

        public function __construct() {
            parent::__construct();
        }

        public function setDatabase($databaseConnection) {
            $this->_db = $databaseConnection;
            parent::setDatabase($this->_db);
        }

    }

    ?>

Don't use a private attribute for the database, use a protected one. That way both classes can share the same $_db, and you only need one setDatabase.

As for your code being OO or not, there simply isn't enough code here to make the claim one way or the other.


Implementation of a dynamic list-like array I needed to address an issue that I met with quite a bit, which has sort of a bag of data that I can easily add items and remove them, and iterate over them at any point without an issue.

First thing that came to mind was to use a linked list: easy to add, easy to remove, without having blank spaces in between, but the use of a linked list didn't look optimal to me.

I made a class in Java called BagArray that behaves like a list in terms of adding and removing items, and is easily iterated over with a for-each without having blank spaces.

It is a mix of a normal array to store the data, and a stack that stores indices of the normal array in which it is free to add an item without deleting another one.

import java.util.Iterator;

public class BagArray<T> implements Iterable<T> {

    private final Object mItems[];
    private final int mSlots[];
    private int mSlotsTop = -1;

    public final int size;

    public BagArray(int size) {
        this.size = size;
        mItems = new Object[size];
        mSlots = new int[size];
        for (int i = 0; i < size; i++)
            mSlots[i] = i;
        mSlotsTop = size - 1;
    }

    public boolean add(T item) {
        if (mSlotsTop < 0)
            return false;
        mItems[popSlot()] = item;
        return true;
    }

    @SuppressWarnings("unchecked")
    public T get(int i) {
        if (i < 0 || i >= size)
            throw new IndexOutOfBoundsException();
        return (T) mItems[i];
    }

    public void remove(int i) {
        if (i < 0 || i > size)
            return;
        if (mItems[i] == null)
            return;
        mItems[i] = null;
        pushSlot(i);
    }

    public void remove(T item) {
        remove(indexOf(item));
    }

    public boolean isFull() {
        return mSlotsTop == -1;
    }

    public boolean isEmpty() {
        return mSlotsTop == size - 1;
    }

    public int numFreeSlots() {
        return mSlotsTop + 1;
    }

    public int indexOf(T item) {
        for (int i = 0; i < size; i++)
            if (item == mItems[i])
                return i;
        return -1;
    }

    private int popSlot() {
        return mSlots[mSlotsTop--];
    }

    private void pushSlot(int s) {
        mSlots[++mSlotsTop] = s;
    }

    @Override
    public Iterator<T> iterator() {
        return new Iterator<T>() {
            private int index = -1;

            @Override
            public boolean hasNext() {
                do {
                    index++;
                    if (index >= size)
                        return false;
                } while (mItems[index] == null);
                return true;
            }

            @Override
            @SuppressWarnings("unchecked")
            public T next() {
                return (T) mItems[index];
            }
        };
    }
}

Important thing to notice here is that the time complexity of the add(), get() and remove(index) methods are \$O(1)\$, which are the essential utilities of this class.

It is also Iterable while the for-each will skip blank spaces in the array.

This makes the use of the class very easy when we need to just "throw some items into a bag or replace them" and iterate over them quickly.

What are your opinions? Is this a good implementation, or is there another way?

This is an interesting concept, a size-limited "bucket" of not-guaranteed-order items. You are right that the add and remove are O(1) operations, but there are a number of concerns I have.... some of them are severe.

If someone adds a null value, you are broken in the iterator. You need to accommodate null, or throw an exception.
The get(int) and remove(int) methods are useless - how does the user know what int to use? The first item added is placed at index size - 1, and so on, so how does that work?

Then, there's a trick with Collections that have a generic type. The general concensus in Java is that you should pass the class definition in to the constructor. The standard Java collections can't do that for backward compatibility reasons, but, consider this:

public class BagArray<T> implements Iterable<T> {

    private final T[] mItems;
    public BagArray(Class<T> tClass, int size) {
        @SuppressWarnings("unchecked")
        mItems = (T[]) Array.newInstance(tClass, size);

Note two things there...

In Java, by convention, you declare arrays as T[] mItems, and not T mItems[]
The array now has the correct type, and there's only one place you need to cast the values, and that is in the array creation.

One last thing, in Java 8, a nice trick for creating an array populated with sequential numbers is:

int[] values = IntStream.range(startInclusive, endExclusive).toArray();


OpenFile function in C++ I am in the process of learning basic C++ and one of the practice questions I came across was to write a function to open a file in C++.

I did understand how to use the streams but I think I went overboard a little bit by making sure that it works. I would appreciate any feedback on my code. The objective was also to keep prompting the user to open a file if it doesn't work in the first go.

Code:

void OpenFile(std::ifstream& inputFileStream) {
    while(true) {
        inputFileStream.clear();
        std::stringstream fileNameStream;
        std::string fileName;
        std::cout << "Enter the file name to open: ";
        if(!getline(std::cin, fileName)) {
            std::cout << "Please enter a valid name." << std::endl;
        } else {
            fileNameStream << fileName;
            std::string fileToOpen;
            std::string remaining;
            if(fileNameStream >> fileToOpen) {
                if(fileNameStream >> remaining) {
                    std::cout << "Unexpected character: " << remaining << std::endl;
                } else {
                    inputFileStream.open(fileToOpen.c_str());
                    if (!inputFileStream.is_open()) {
                        std::cout << "File could not be opened. Please try again.";
                        std::cout << std::endl;
                    } else {
                        break;
                    }
                }
            }
        }
    }
}

Prefer returning a result instead of passing it via a reference

The result of your function is a std::ifstream object, so make that explicit:

std::ifstream OpenFile() {
  while(true) {
    // read filename

    std::ifstream file(fileName);

    if (file.is_open())
      return file;
    else
      // handle error
  }
}

The advantage is that instead of having to write:

std::ifstream file;
OpenFile(file);

You can write:

std::ifstream file = OpenFile();

Or even:

auto file = OpenFile();

Avoid using std::stringstream if it's not necessary

It might seem convenient, but it's rather inefficient, and often there are other ways to accomplish what you want. It seems from your code that you want to forbid filenames with whitespace characters in them. Consider using std::find_first_of() to check if there are certain characters in a string.

Apart from that, many filesystems, including those on mainstream operating systems such as Linux, Windows and macOS, allow spaces in their filenames. And surely, when you call open(), the operating system will return an error if you pass it an invalid filename, so you don't have to do this error checking yourself.

Avoid casting std::string to C-strings

Since C++11 you can pass a std::string directly to a std::ifstream's constructor or to its open() function.

Use '\n' instead of std::endl

Avoid using std::endl. It is equivalent to '\n' plus a call to flush(). The flush is usually unnecessary and might hurt performance. So for example, instead of:

std::cout << "File could not be opened. Please try again.";
std::cout << std::endl;

Write:

std::cout << "File could not be opened. Please try again.\n";

Pass along the error message from the operating system

There are many reasons why opening a file might fail. It will benefit the user to learn the exact reason why it failed. For example, it could be that the file doesn't exist, or that it's permissions don't allow it being opened by the user. To do this, call strerror(errno), like so:

std::cout << "File could not be opened: " << strerror(errno) << "\nPlease try again.\n";

Add a way for the user to cancel opening a file

The function loops forever until a file has succesfully been opened. What if the user decides that it doesn't want any file to be opened at this time, but wants the application to cancel this operation but do something else? If you don't allow a way out of this function, the only option available to the user is closing the application and restarting it.

Consider making the function more generic

While you shouldn't add bells and whistles to a function like this if you are not going to use them, you might want to consider whether you want to make this function a bit more widely applicable. For example, you could have it open a file either for reading, writing or both, or have a way to give a custom prompt. For example:

std::fstream OpenFile(ios_base::openmode mode = ios_base::in, const std::string &prompt = "Enter a file name: ") {
  while(true) {
    std::cout << prompt;
    // read filename

    std::fstream file(filename, mode);

    if (file.is_open())
      return file;
    else
      // handle error
  }
}


Project Euler 50: Consecutive prime sum I just finished solving Project Euler's 50th problem, but it's awfully slow. I'd like to hear your thoughts on my code's efficiency and practices.
Problem Statement
The prime 41, can be written as the sum of six consecutive primes:
\$41 = 2 + 3 + 5 + 7 + 11 + 13\$
This is the longest sum of consecutive primes that adds to a prime below one-hundred.
The longest sum of consecutive primes below one-thousand that adds to a prime, contains 21 terms, and is equal to 953.
Which prime, below one-million, can be written as the sum of the most consecutive primes?
Code

let primeNumbers = [];

function isPrime(number) { // checks whether number is prime or not
    for(let i = 2; i <= number / 2; i++) { // stops checking at 1/2 of number
        if (number % i === 0) return false;
    }
    return true;
}

function storePrimes(count) {
    for(let i = 2; i < count; i++) { // starts at 2
        if (isPrime(i)) {
            primeNumbers.push(i);
        }
    }
}

function findLargestSum() {
    let termsCount = 0;
    let sumOfTerms = 0;

    primeNumbers.forEach(currentSum => { // keeps track of possible sum
        primeNumbers.forEach((startNumber, startIndex) => { // keeps track of start index
            let consecutiveCount = 0;
            let consecutiveSum = 0;
            primeNumbers.forEach((prime, primeIndex) => { // iterates through primes
                if (primeIndex >= startIndex) { // applies start index
                    consecutiveCount++;
                    consecutiveSum += prime;
                    if (consecutiveCount > termsCount && consecutiveSum === currentSum) {
                        termsCount = consecutiveCount;
                        sumOfTerms = consecutiveSum;
                    }
                }
            })
        })
    })

    return {largestSum: sumOfTerms, termsCount: termsCount};
}

function findPrimes(count) {
    storePrimes(count)
    let results = findLargestSum();
    console.log("Largest sum'o'primes of prime consecutives under " + count + " is: " + results.largestSum + " with " + results.termsCount + " terms.");
}

findPrimes(1000000);

Improvement in function isprime:
for(let i = 2; i <= number / 2; i++)

can be
for(let i = 2; i <= Math.round(Math.sqrt(number)) + 1 ; i++)

Otherwise, the best easy to understand approach(in accordance to my knowledge) is to use the Sieve of Eratosthenes. Your problem can be a subset of the following problem Sieve of Eratosthenes JavaScript implementation - performance very slow over a certain number. Credits of the code below goes to the owner of this post.
function getPrimesUnder(number) {
  var start = new Date().getTime();

  var numbers = [2];
  var sqNum = Math.sqrt(number);
  var i, x;
  for (i = 3; i < number; i = i + 2) {
    numbers.push(i);
  }
  for (x = 0; numbers[x] < sqNum; x++) {
    for (i = 0; i < numbers.length ; i++){
       if (numbers[i] > numbers[x]) {
        if(numbers[i] % numbers[x] === 0){
          numbers.splice(i, 1)
        }
      }
    }
  }
  var end = new Date().getTime();
  var time = end - start;
  alert('Execution time: ' + time/1000 + ' seconds');
  return numbers;

}

There is something much more efficient (Which is the fastest algorithm to find prime numbers?) known as Sieve of Atkin. You can do more research on it.


Permutations II from leetcode solution Given a collection of numbers that might contain duplicates, return
  all possible unique permutations.

  For example, [1,1,2] have the following unique permutations:
  [[1,1,2], [1,2,1], [2,1,1]]

There are many better solutions out there but I am interested in just the code review and how it can be made better.

Solution is exactly described here

Let's use input abc as an example.

Start off with just the last element (c) in a set (["c"]), then add the second last element (b) to its front, end and every possible positions in the middle, making it ["bc", "cb"] and then in the same manner it will add the next element from the back (a) to each string in the set making it:

"a" + "bc" = ["abc", "bac", "bca"]  and  "a" + "cb" = ["acb" ,"cab", "cba"]
Thus entire permutation:

["abc", "bac", "bca","acb" ,"cab", "cba"]

class Solution(object):
    def permuteUnique(self, nums):
        """
        :type nums: List[int]
        :rtype: List[List[int]]
        """
        if not nums or len(nums) == 1:
            return [nums]
        output_list, output_list_copy, temp_output = [], [], []
        for num in nums:
            if not output_list:
                output_list = [[num]]
                continue
            for t in output_list:
                assigned, already_seen = False, None
                for j in range(len(t)+1):
                    if already_seen == num and assigned:
                        continue
                    t1 = t[0:j] + [num] + t[j:]
                    if j < len(t) and t[j] == num:
                        assigned, already_seen = True, num
                    temp_output.append(t1)
                output_list_copy += temp_output
                temp_output = []
            output_list = output_list_copy
            output_list_copy = []
        return output_list

I have a few pointers,

Use libraries when possible,  you are currently reinventing the wheel of itertools.permutations
You could use a set() to have \$O(N)\$ lookup when finding items that are already seen.
You should use generators instead of keeping everything in a list, this avoids high memory usage when working with large numbers

Reinventing the wheel

class Solution:
    def permuteUnique(self, nums):
        """
        :type nums: List[int]
        :rtype: List[List[int]]
        """

        def unique_perms(elements, unique):
            if len(elements) <=1:
                yield elements
            else:
                for perm in unique_perms(elements[1:], unique):
                    for i in range(len(elements)):
                        next_perm = perm[:i] + elements[0:1] + perm[i:]
                        if tuple(next_perm) not in unique:
                            unique.add(tuple(next_perm))
                            yield next_perm

        return list(unique_perms(nums, set()))

Using modules

from itertools import permutations

def permute_unqiue(nums, r):
    return list(set(permutations(nums, r)))

This works by making a set() of the permutations. That removes all duplicate values, since they will hash to the same thing. Afterwards you can return the list() of unique permutations.


Deep copy linked list with arbitrary nodes The original question can be found here.

I implemented method 2:

Please comment about style, unit testing, code complexity, time and memory space and complexity.

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

namespace ConsoleApplication2
{
    public class LinkedListArbitraryPointer
    {
        public ArbNode Root { get; set; }

        public ArbNode DeepCopyLinkedList(ArbNode root)
        {
            if(root ==null)
            {
              return null;
            }

            //1) Create the copy of node 1 and insert it between node 1 & node 2 in original Linked List, create the copy of 2 and insert it between 2 & 3..
            //Continue in this fashion, add the copy of N afte the Nth node
            ArbNode temp = root;
            while (temp != null)
            {
                ArbNode swap = temp.Next;
                temp.Next = new ArbNode(temp.Index);
                temp.Next.Next = swap;
                temp = temp.Next.Next;
            }

            // 2) Now copy the arbitrary link in this fashion
            //  original->next->arbitrary = original->arbitrary->next;  /*TRAVERSE TWO NODES*/

            temp = root;
            while (temp != null)
            {
                temp.Next.Arb = temp.Arb.Next;
                temp = temp.Next.Next;
            }

            //3) Now restore the original and copy linked lists in this fashion in a single loop.
            temp = root;
            ArbNode current = root.Next;
            ArbNode storeList = root.Next;
            while (temp != null)
            {
                temp.Next = temp.Next.Next;
                if (current.Next != null)
                {
                    current.Next = current.Next.Next;
                }
                temp = temp.Next;
                current = current.Next;
            }

            return storeList;
        }

    }

    public class ArbNode
    {
        public ArbNode(int index)
        {
            Index = index;
            Next = null;
            Arb = null;
        }

        public ArbNode()
        {
            Next = null;
            Arb = null;
        }
        public int Index { get; set; }
        public ArbNode Next { get; set; }
        public ArbNode Arb { get; set; }
    }
}

Unit Test

using System;
using System.Linq;
using ConsoleApplication2;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace UnitTests
{
    [TestClass]
    public class LinkedListArbPointerUnitTest
    {
        [TestMethod]
        public void TestMethod1()
        {
            LinkedListArbitraryPointer list = new LinkedListArbitraryPointer();

            list.Root = new ArbNode(1);

            list.Root.Next = new ArbNode(2);

            //2->1
            list.Root.Next.Arb = list.Root;

            list.Root.Next.Next = new ArbNode(3);

            //1 -> 3
            list.Root.Arb = list.Root.Next.Next;

            list.Root.Next.Next.Next = new ArbNode(4);

            //4->3
            list.Root.Next.Next.Next.Arb = list.Root.Next.Next;

            list.Root.Next.Next.Next.Next = new ArbNode(5);

            //5->2
            list.Root.Next.Next.Next.Next.Arb = list.Root.Next;

            //3->5
            list.Root.Next.Next.Arb = list.Root.Next.Next.Next.Next;
            //var res = list.DeepCopyLinkedListWithAuxArray(list.Root);
            var res = list.DeepCopyLinkedList(list.Root);
            Assert.AreEqual(1,res.Index);
            Assert.AreEqual(3, res.Arb.Index);
        }
    }
}

Var

Use the var keyword when defining local variables where the right hand side of the definition makes the type obvious. This looks cleaner and saves time when it comes to changing types during refactoring.

e.g.

LinkedListArbitraryPointer list = new LinkedListArbitraryPointer();

should be

var list = new LinkedListArbitraryPointer();

You actually do use var at points, but not consistently.

Naming

Avoid using shortened names for your variables, at a glance res is not entirely clear what it refers to, prefer actual or deepCopiedList instead. The same goes for ArbNode.

Definitely don't name your test methods TestMethodX! Find a naming style you like that's concise, detailed and consistent and stick with it.

Comments

Your comments seem to describe what you're doing (which is the job of your code) and not why you are doing it. I often find if there is some chunk of code that needs a comment explaining what it does, it can often be refactored to its own method, or made clearer somehow.

Design

The property 'Root' seems entirely out of place and unused by your code, remove it.

Additionally, I would consider making ArbNode implement IEnumerable so you can replace all your:

        ArbNode temp = root;
        while (temp != null)
        {
            ...
            temp = temp.Next;
        }

with a cleaner:

foreach(var node in root)
{
    ...
}


C++ Read istream into string with exceptions Did I cover all the bases? Can this be improved (best practice)?

Edit... I've come up with some improvements. When I tried edit the code, the software complained about too much code and too little blather (paraphrasing). So, here I am blathering. When I have blathered enough, the code below will be the new stuff.

//#include <libdj/istream.h>

#include <istream>
#include <string>
#include <stdexcept>

namespace dj {
    inline
    void append_istream_onto_string(std::istream& inp, std::string &outp) {
        // Uses inp.seekg(), and inp.tellg() to determine size
        // Throws (std::) invalid_argument, runtime_error, bad_alloc, length_error
        if (!inp.good()) {
            throw std::invalid_argument
              ("Input stream is not valid. (read_istream_into_string)");
        }
        if (!inp.seekg(0, std::ios::end)) {
            throw std::runtime_error
            ("Cannot deduce length of input stream. (read_istream_into_string)");
        }
        outp.reserve(outp.size() + inp.tellg()); // Throws bad_alloc and length_error
        if (!inp.seekg(0, std::ios::beg)) {
            throw std::runtime_error
              ("Input stream seek failed. (read_istream_into_string)");
        }
        outp.append((std::istreambuf_iterator<char>(inp)),
            std::istreambuf_iterator<char>());
    }

    inline
    std::string to_string(std::istream& inp) {
        std::string ret;
        append_istream_onto_string(inp, ret);
        return ret;
    }
}
#include <fstream>

int main() {

    std::ifstream inp("junk.txt");

    std::string buffer;
    buffer = "// junk.txt\n";
    try {
        //dj::append_istream_onto_string(inp, buffer);
        buffer = dj::to_string(inp);
    } catch (const std::exception &ex) {
        std::cout << ex.what() << std::endl;
    }
    std::cout << buffer << std::endl;
}

Streams are awful. Don't use them. (Joking, but I wish I had a viable alternative to recommend for basic file input. C++ is pathetic in this area.)

[code:] tellg() returns a signed type, with -1 as an error value. You might like to check that, and then static_cast it before using it in reserve().
[code:] Using std::string::resize() and std::istream::read() to copy from the stream might be faster than using std::string::append() with std::istreambuf_iterators.
[design(bug?):] Seeking to the end of the file like that may not work. On Windows, for example, if a file is opened in text mode, the ctrl-z (alt026) character is treated as the end of the file (even though it isn't). As I understand it, you have to use ignore() to find the real end of the file instead.
[design:] I doubt it's worth it to create a function to append a string directly. Loading into a string, then appending is probably fast enough. (What if you want to prepend or insert... suddenly you need more functions that basically do the same thing).
Write unit tests! Something like this needs a whole bunch of tests that save a string to a file, then load a string and compare the results (binary mode, text mode, special characters, etc.).

The whole article in the link above is useful, but the summary section at the end describes 3 reasonable alternatives for reading from a file stream:

Put the istream's rdbuf into a stringstream, then use the .str() function to get a string. This involves extra copying, but is simplest, and should be quick enough.
Use .ignore() to find the end of the file, then read the whole thing at once. No extra copies in memory (good for large files), but involves reading the file twice.
Read chunks into a std::deque and then copy. Works where it's not possible to seek in the stream.