Implementing a Treeview component using React Today after watching a getting started course about React, I tried to code my very first React component, a "treeview" as the title says. It does work, but as a newbie I know there are many improvement that can be done.

It receives a data array like the one below:

[{
    descp: 'UNE',
    url: '/tree/une',
    icon: 'icon-list',
    children: [
        {
            descp: 'ATI',
            url: '/tree/une/ati',
            icon: 'icon-layers',
            children: [{
                    descp: 'ATI-VC',
                    url: '/tree/une/ati/ativc',
                    icon: 'icon-pin',
                    children: []
                },
                {
                    descp: 'ATI-CMG',
                    url: '/tree/une/ati/aticmg',
                    icon: 'icon-pin',
                    children: []
                }
            ]
        },
        {
            descp: 'EMGEF',
            url: '/tree/une/emgef',
            icon: 'icon-layers',
            children: []
        },
        {
            descp: 'ECIE',
            url: '/tree/une/ecie',
            icon: 'icon-layers',
            children: []
        },
        {
            descp: 'GEYSEL',
            url: '/tree/une/geysel',
            icon: 'icon-layers',
            children: []
        }
    ]
}]

Maybe the data format can be improve as well but the real deal is the component:

const MyTree = (props) => {
    const { list } = props;
    return (
        list.map((child,i)=>
            <MyTreeNestedItems key={i} data={child} />
        )
    );
};

const MyTreeNestedItems = (props) => {
    const { data } = props;

    const createChildren = (list) => {
        return (
            list.map((child,i)=>
                <MyTreeNestedItems key={i} data={child} />
            )
        )
    }

    let children = null;
    if (data.children.length) {
        children = createChildren(data.children);
    }

    return (

                 {" "}
                { data.descp }

{children}

    );
};

render(<MyTree list={tree} />,document.querySelector('.js-mytree'));

Any tips would be gratefully appreciated.

You might notice that createChildren is almost the same as MyTree -- the only difference is that createChildren takes its argument directly instead of as a props object. So with a small change we can remove createChildren entirely:

const MyTreeNestedItems = (props) => {
    const { data } = props;

    let children = null;
    if (data.children.length) {
      children = <MyTree list={data.children} />;
    }

    return (

                 {" "}
                { data.descp }

{children}

    );
}

This can be a bit more compact as so:

const MyTreeNestedItems = ({ data }) => {
    return (

                 {" "}
                { data.descp }

                {data.children.length ? <MyTree list={data.children} /> : null}

    );
};

Currently this code renders invalid HTML -- it renders with a
 as the top level element but
 should only be contained in a
,
, or <menu>.

Looking at the code so far, <MyTree> is rendered in two places -- it may be tempting to just add another
 around <MyTree list={tree} /> in the render() call. But then we would have two occurrences of a structure like this:
<MyTree/>
. It'd be easier to just move the
 into the <MyTree/>.

With that, the code now looks like this:

const MyTree = ({ list }) => {
    return (

            {list.map((child,i)=> <MyTreeNestedItems key={i} data={child} />)}

    );
};

const MyTreeNestedItems = ({ data }) => {
    return (

                 {" "}
                { data.descp }

            {data.children.length ? <MyTree list={data.children} /> : null}

    );
};

render(<MyTree list={tree} />, document.querySelector('.js-mytree'));

From here it's a matter of taste. Both of the components are just directly returning a value, so we can use short arrow function syntax now: (...) => (value) instead of (...) => { return (value); }

Also, the components are now simple enough that I would just combine the two.

All in, that leaves us with this final version of the code:

const MyTree = ({ list }) => (

        {list.map(({icon, descp, children}, i) => (

                     {descp}

                {children.length ? <MyTree list={children} /> : null}

        ))}

);

render(<MyTree list={tree} />, document.querySelector('.js-mytree'));


Creating SOLID immutable and mutable Location types I have a Location type:

public abstract class Location implements Locatable {
    private int x, y;

    protected final void setX(int x) {
         this.x = x;
    }

    protected final void setY(int y) {
         this.y = y;
    }

    public final int getX() {
        return x;
    }

    public final int getY() {
        return y;
    }
}

interface Locatable {
    int getX();
    int getY();
}

Location has protected mutator methods, to ensure I can provide an MutableLocation type, but I'm not sure if this violates the Interface Seggregation principle when creating an ImmutableLocation.

I feel it doesn't, since the client will never have access to the setters through ImmutableLocation (cannot call it from an instance, cannot call it from a subclass of ImmutableLocation). So this was my attempt at a SOLID location type, which I plan on using in a project.

class ImmutableLocation extends Location {
    //...
 }

class MutableLocation extends Location {
    public void adjustX(int amount, IncremeantOperation op) {
        int x = op.adjust(x, amount);

        setX(x);
    }

    public void adjustY(int amount, IncremeantOperation op) {
        int y = op.adjust(getY(), amount);

        setY(y);
    }
}

enum IncremeantOperation {
    INCREASE {
        public int adjust(int target, int amount) {
             return target + amount;
        }
    },
    DECREASE {
        public int adjust(int target, int amount) {
            return target - amount;
        }
    };

    public int adjust(int target, int amount) {
        throw new UnsupportedOperationException("Cannot call adjust(int, int) on unknown operation");
    }
}

Please give your thoughts. My main concerns are:

I'm not sure if the enum is an overkill
If the setters are a violation of SOLID; I'm not sure how else I should do it.
If UnsupportedOperationException is best fit for the adjust method

interface Locatable {
    Location getLocation();
}

public abstract class Location {
    //may add fields, which explains the class

    public abstract int getX();
    public abstract int getY();
}

public final class ImmutableLocation extends Location {
    private int x, y;

    public ImmutableLocation(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }
}

public final class MutableLocation extends Location {
    private int x, y;

    public MutableLocation(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public void adjustX(int x) {
         //adjust x
    }

    public void adjustY(int y) {
         //adjust y
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }
}

Locatable would be implemented by anything with a location, such as Entity:

abstract class Entity implements Locatable {
    private Location location;

    public Location getLocation() {
         return location;
    }
}

It doesn't make sense to have setters in an immutable location.
Consider this alternative:

interface Location {
    int getX();
    int getY();
}

class MutableLocation implements Location {

    private int x;
    private int y;

    public MutableLocation(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public int getY() {
        return y;
    }

    public void adjustX(int x) {
        //adjust x
    }

    public void adjustY(int y) {
        //adjust y
    }
}

final class ImmutableLocation implements Location {
    private final Location location;

    public ImmutableLocation(int x, int y) {
        location = new MutableLocation(x, y);
    }

    @Override
    public int getX() {
        return location.getX();
    }

    @Override
    public int getY() {
        return location.getY();
    }
}

Your questions

  I'm not sure if the enum is an overkill

Not necessarily. Maybe it is, maybe it isn't. It will depend on the rest of the code in your project. From here it can go both ways.

  If the setters are a violation of SOLID; I'm not sure how else I should do it.

They violate SOLID, in particular the interface segregation principle:
the immutable class inherits setters but doesn't need them.
See my proposed cleaner alternatives above.

  If UnsupportedOperationException is best fit for the adjust method

For unsupported operations, this is fine, but your purpose is not clear.
Do you intend to add values in this enum without implementing adjust?
If yes, then your solution is fine.
But if you never intend to add values without implementing adjust,
then it would be better to make the method abstract.
Also you should use @Override in the implementations:

enum IncrementOperation {
    INCREASE {
        @Override
        public int adjust(int target, int amount) {
            return target + amount;
        }
    },
    DECREASE {
        @Override
        public int adjust(int target, int amount) {
            return target - amount;
        }
    };

    public abstract int adjust(int target, int amount);
}


Find out whether number is of Power of Two The task

is taken from leetcode

  Given an integer, write a function to determine if it is a power of
  two.

  Example 1:

  Input: 1

  Output: true

  Explanation: 20 = 1

  Example 2:

  Input: 16

  Output: true

  Explanation: 24 = 16

  Example 3:

  Input: 218

  Output: false

My solution

/**
 * @param {number} n
 * @return {boolean}
 */
var isPowerOfTwo = function(n) {
  if (n <= 0 ) { return false; }
  if (n <= 2) { return true; }
  let num = n;
  do {
    const x = num / 2;
    if ((x | 0) !== x) { return false; }
    if (x === 2) { return true; }
    num = x;
  } while(num);
  return false;
};

I mean, I would use a library to do this. Namely, Math.log2

function isPowerOfTwo(n) {
  return Math.log2(n) % 1 === 0;
}

console.log(isPowerOfTwo(3)); //false
console.log(isPowerOfTwo(4)); //true
console.log(isPowerOfTwo(7)); //false
console.log(isPowerOfTwo(8)); //true
console.log(isPowerOfTwo(-1));//false


Pool of two objects for a web service I am creating a web service that uses a component I've licensed that is restricted to two concurrent instances at any one time. Creating a new instance of the component takes a long time (seconds) so I want to maintain a pool of objects for re-use. Here's my first (working) attempt, but it seems fairly inelegant, and also likely that I've made some mistakes (although it hasn't failed in my own tests):

public class MyWebService : System.Web.Services.WebService
{
    static RestrictedClass rc1, rc2;

    [WebMethod]
    public byte[] MyMethod(byte[] Stream1, byte[] Stream2)
    {
        RestrictedClass rc = null;
        Mutex m1 = null, m2 = null;
        if (rc1 == null)
        {
            m1 = new Mutex(true, "rc1");
            m1.WaitOne();
            rc1 = new RestrictedClass();
            rc = rc1;
        }
        else if (rc2 == null)
        {
            m2 = new Mutex(true, "rc2");
            m2.WaitOne();
            rc2 = new RestrictedClass();
            rc = rc2;
        }
        else
        {
            bool m1Granted, m2Granted;
            m1 = new Mutex(true, "rc1", out m1Granted);
            m2 = new Mutex(true, "rc2", out m2Granted);
            int idx;
            if (m1Granted)
            {
                idx = 0;
            }
            else if (m2Granted)
            {
                idx = 1;
            }
            else
            {
                idx = Mutex.WaitAny(new Mutex[] { m1, m2 });
            }
            if (idx == 0)
            {
                rc = rc1;
                m2.Close();
            }
            else if (idx == 1)
            {
                rc = rc2;
                m1.Close();
            }
        }

        Result = rc.DoStuff();

        if (m1 != null && rc == rc1)
            m1.ReleaseMutex();
        if (m2 != null && rc == rc2)
            m2.ReleaseMutex();
        return Result.ToArray();
    }
}

Please pick it apart and suggest any improvements you like.

I have plenty of things to say here..

You should apply separation of concerns by isolating the pooling behavior from the logic of the webmethod itself.
you might want to create both instances at startup
blocking inside the web methods will seriously limit throughput, consider using async methods
What happens if there is an exception and you don't release the mutex?
The pooling can be implemented much simpler using higher level concurrency features, for example a BlockingCollection:

public class ObjectPool<T>
{
    private readonly BlockingCollection<Lazy<T>> _pool;

    public ObjectPool(int size, Func<T> constructor)
    {
        var resources = Enumerable.Repeat(new Lazy<T>(constructor), size);
        var stack = new ConcurrentStack<Lazy<T>>(resources);
        _pool = new BlockingCollection<Lazy<T>>(stack);
    }

    public IDisposable Aquire(out T resource)
    {
        Lazy<T> lazy = _pool.Take();
        resource = lazy.Value;
        return new Disposable(() => _pool.Add(lazy));
    }

    public class Disposable : IDisposable
    {
        private readonly Action _dispose;

        public Disposable(Action dispose)
        {
            _dispose = dispose;
        }

        public void Dispose()
        {
            _dispose.Invoke();
        }
    }
}

The call to Aquire will block until an item is available in the pool, it's very important to call Dispose on the returned object, this will release the resouce back into the pool. You can use try/finally or a using statement:

    private static readonly ObjectPool<Random> pool
        = new ObjectPool<Random>(2, () => new Random());

    static int CalculateRandomNumber()
    {
        using (pool.Aquire(out var rnd))
        {
            return rnd.Next();
        }
    }

Notice how a Stack is used in combination with Lazy. This ensures that you will never create more instances than are in use at the same time.


Comparing data from two data tables I have this code which compares data from two data tables.  It does this by using a primary column and inserting the new record from table2 to table1. This loop continues for a large number of tables. I want to optimize it. Kindly give suggestions.

foreach (DataRow drRow in table2.Rows)
{
    if(!table1.Rows.Contains(drRow["KeyId"]))
    {
        table1.ImportRow(drRow);
    }
}

For .net framework 3.5, you can have following code improvements:

foreach (var drRow in table2.Rows.Cast<DataRow>()
                      .Where(drRow => !table1.Rows.Contains(drRow["KeyId"])))
{
    table1.ImportRow(drRow);
}

part of body can be converted to LINQ-expression
use "var" (implicitly typed) instead of "DataRow" (explicitly typed)


Secure math expressions using PHP eval() PHP's eval() is general considered insecure with any user input.

However, with filtering of input (i.e. [-+/*0-9]) you can make eval() secure for a small subset of PHP; mathematical expressions.

I've tried to think of a way to extend that by allowing a limited set of math functions and I believe I've found a secure way of doing it.

function expression($expression) {
    static $function_map = array(
        'floor'     => 'floor',
        'ceil'      => 'ceil',
        'round'     => 'round',
        'sin'       => 'sin',
        'cos'       => 'cos',
        'tan'       => 'tan',
        'asin'      => 'asin',
        'acos'      => 'acos',
        'atan'      => 'atan',
        'abs'       => 'abs',
        'log'       => 'log',
        'pi'        => 'pi',
        'exp'       => 'exp',
        'min'       => 'min',
        'max'       => 'max',
        'rand'      => 'rand',
        'fmod'      => 'fmod',
        'sqrt'      => 'sqrt',
        'deg2rad'   => 'deg2rad',
        'rad2deg'   => 'rad2deg',
    );

    // Remove any whitespace
    $expression = strtolower(preg_replace('~\s+~', '', $expression));

    // Empty expression
    if ($expression === '') {
        trigger_error('Empty expression', E_USER_ERROR);
        return null;
    }

    // Illegal function
    $expression = preg_replace_callback('~\b[a-z]\w*\b~', function($match) use($function_map) {
        $function = $match[0];
        if (!isset($function_map[$function])) {
            trigger_error("Illegal function '{$match[0]}'", E_USER_ERROR);
            return '';
        }
        return $function_map[$function];
    }, $expression);

    // Invalid function calls
    if (preg_match('~[a-z]\w*(?![\(\w])~', $expression, $match) > 0) {
        trigger_error("Invalid function call '{$match[0]}'", E_USER_ERROR);
        return null;
    }

    // Legal characters
    if (preg_match('~[^-+/%*&|<>!=.()0-9a-z,]~', $expression, $match) > 0) {
        trigger_error("Illegal character '{$match[0]}'", E_USER_ERROR);
        return null;
    }

    return eval("return({$expression});");
}

It's use would be simple: `$result = expression('floor(3 / 2)') and anything but math/boolean operators, numbers and a limited set of functions should be filtered out.

I can't find a way of hacking this expression() function, but I'd really like to have a second opinion on this.

Is the above expression function secure for user input?

It looks pretty reasonable to me. A couple of points:

You seem to be assuming that a function start with a letter. Functions CAN start with an underscore. Are you sure there is no such function that can be exploited?
The regex in the section 'Invalid function calls' seems like it could be simpler. You are trying to check that any 'word' ends with a '(', is this right? If so, I don't think you need the second \w

Finally although this seems like a good job, the ideally ideal way to do something like this would be to make your own tokenizer. A tokenizer looks at a string char by char and decides at each point whether it's looking at part of an identifier, an operator, etc. You would simply need to call your checks at each point that it decides it has identified a complete token. The advantage of this is that it would be easy to extend the syntax you accept and if needed to make translations from the language you accept to valid PHP for eval().


Merges two lists into one list The code works as it is written, however; I was wondering if there are any easier or more effective ways to merge two lists.

import java.util.*;
public class MergeTwoSortedLists {
    public static void main(String[] args)
    {
     java.util.Scanner input = new java.util.Scanner(System.in);

     // Enter values for list1
     System.out.println("Enter the amount of numbers in the first list:");
     // Assign user defined length to the array
     int lengthOfArray = input.nextInt();
     int[] list1 = new int[lengthOfArray];

     // i increments and the user values are assigned to the array until the length is exceeded
     System.out.println("Enter " + list1.length + " values: ");
     for (int i = 0; i < list1.length; i++)
      list1[i] = input.nextInt();

     // Enter values for list2
     System.out.println("Enter the amount of numbers in the second list:");
     // Assign user defined length to the array
     int lengthOfArray2 = input.nextInt();
     int[] list2 = new int[lengthOfArray2];

     // i increments and the user values are assigned to the array until the length is exceeded
     System.out.println("Enter " + list2.length + " values: ");
     for (int i = 0; i < list2.length; i++)
      list2[i] = input.nextInt();

     // Lists one and two are merged into one list
     int[] list3 = merge(list1, list2);
     // Sorts the lists in numerical order
     Arrays.sort(list3);
     // This accesses the actual user defined integers and not their locations, printing them in an assorted array
     System.out.println("The merged list is: " +Arrays.toString(list3));

    }
    public static int[] merge(int[] list1, int[] list2)
    {
        int[] list3 = new int[list1.length + list2.length];
        // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
        for (int i = 0, p=0 ; i < list1.length || i < list2.length ; i++) {
            if (i < list1.length) {
                list3[p++] = list1[i];
            }
            if (i < list2.length) {
                list3[p++] = list2[i];
            }
        }
        return list3;

    }

}

Formatting

Now let's talk about formatting. Your code is hard to read. Format it. Done.

Actually, if you want some tips:

import java.util.*;
public class MergeTwoSortedLists {
    public static void main(String[] args)
    {

These lines of code have a couple of problems:

Line 1-2: Space between import statements and class declaration

Line 2-3: Space between class declaration and first line of code inside class

Line 3-4: Java conventions require that braces are on the same line as declaration

Result:

import java.util.*;

public class MergeTwoSortedLists {

    public static void main(String[] args) {

This code:

 java.util.Scanner input = new java.util.Scanner(System.in);

 // Enter values for list1
 System.out.println("Enter the amount of numbers in the first list:");
 // Assign user defined length to the array
 int lengthOfArray = input.nextInt();
 int[] list1 = new int[lengthOfArray];

 // i increments and the user values are assigned to the array until the length is exceeded
 System.out.println("Enter " + list1.length + " values: ");
 for (int i = 0; i < list1.length; i++) // line 11
  list1[i] = input.nextInt();

 // Enter values for list2
 System.out.println("Enter the amount of numbers in the second list:");
 // Assign user defined length to the array
 int lengthOfArray2 = input.nextInt();
 int[] list2 = new int[lengthOfArray2];

 // i increments and the user values are assigned to the array until the length is exceeded
 System.out.println("Enter " + list2.length + " values: ");
 for (int i = 0; i < list2.length; i++) // line 23
  list2[i] = input.nextInt();

 // Lists one and two are merged into one list
 int[] list3 = merge(list1, list2);
 // Sorts the lists in numerical order
 Arrays.sort(list3);
 // This accesses the actual user defined integers and not their locations, printing them in an assorted array
 System.out.println("The merged list is: " +Arrays.toString(list3));

all share a problem. That problem is that you are indenting it by only 1 space, Java conventions recommend 4-space tabs, or just 4 spaces.

Some other things:

Line 11 (as marked by comment): ALWAYS put braces to group if, else, while, do-while, for, and similar statements. Here is why.

Line 13 or 14: Two empty lines won't improve readability. Remove one.

Line 23 (as marked by a comment): Same thing as Line 11

Last line: Space before and after +

Result:

    java.util.Scanner input = new java.util.Scanner(System.in);

    // Enter values for list1
    System.out.println("Enter the amount of numbers in the first list:");
    // Assign user defined length to the array
    int lengthOfArray = input.nextInt();
    int[] list1 = new int[lengthOfArray];

    // i increments and the user values are assigned to the array until the length is exceeded
    System.out.println("Enter " + list1.length + " values: ");
    for (int i = 0; i < list1.length; i++) {
        list1[i] = input.nextInt();
    }

    // Enter values for list2
    System.out.println("Enter the amount of numbers in the second list:");
    // Assign user defined length to the array
    int lengthOfArray2 = input.nextInt();
    int[] list2 = new int[lengthOfArray2];

    // i increments and the user values are assigned to the array until the length is exceeded
    System.out.println("Enter " + list2.length + " values: ");
    for (int i = 0; i < list2.length; i++) {
        list2[i] = input.nextInt();
    }

    // Lists one and two are merged into one list
    int[] list3 = merge(list1, list2);
    // Sorts the lists in numerical order
    Arrays.sort(list3);
    // This accesses the actual user defined integers and not their locations, printing them in an assorted array
    System.out.println("The merged list is: " + Arrays.toString(list3));

Here:

public static int[] merge(int[] list1, int[] list2)
{
    int[] list3 = new int[list1.length + list2.length];
    // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
    for (int i = 0, p=0 ; i < list1.length || i < list2.length ; i++) {
        if (i < list1.length) {
            list3[p++] = list1[i];
        }
        if (i < list2.length) {
            list3[p++] = list2[i];
        }
    }
    return list3;
    // line 15
}

Looks pretty good, but some points:

Line 1: Space between methods

Line 1-2: Java conventions require that braces are on the same line as declaration

Line 4: Space before and after =
Line 4: Usually no space is required before ;

Line 15: Extra newline at the end of the method is not necessary

Result:

public static int[] merge(int[] list1, int[] list2) {
    int[] list3 = new int[list1.length + list2.length];
    // Both p and i increment, and list 3 is assigned the user defined integers from both list 1 and 2
    for (int i = 0, p = 0; i < list1.length || i < list2.length; i++) {
        if (i < list1.length) {
            list3[p++] = list1[i];
        }
        if (i < list2.length) {
            list3[p++] = list2[i];
        }
    }
    return list3;
}

Review

import java.util.*;

Never import an entire package, even if you are using a lot of the package. In this case, all you need is `Scanner, so that's even worse.

import java.util.Scanner;

is what you should do.

java.util.Scanner input = new java.util.Scanner(System.in);

Why are you using the full name when you just imported it?

Scanner input = new Scanner(System.in);

Now, to your merge method:

The method should be private
Since you are sorting the array anyways, it is more efficient to sort the arrays, then merge them, like the final step of merge sort:

private static int[] merge(int[] array1, int[] array2) {
    int length1 = array1.length;
    int length2 = array2.length;

    int[] array = new int[length1 + length2];

    int index1 = 0;
    int index2 = 0;

    while (index1 < length1 && index2 < length2) {
        array[index1 + index2] = array1[index1] < array2[index2]
                ? array1[index1++]
                : array2[index2++];
    }
    while (index1 < length1) {
        array[index1 + index2] = array1[index1++];
    }
    while (index2 < length2) {
        array[index1 + index2] = array2[index2++];
    }
    return array;
}

Final Code:

import java.util.Scanner;

public class MergeTwoSortedLists {

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);

        // Enter values for list1
        System.out.println("Enter the amount of numbers in the first list:");
        // Assign user defined length to the array
        int lengthOfArray = input.nextInt();
        int[] list1 = new int[lengthOfArray];

        // i increments and the user values are assigned to the array until the length is exceeded
        System.out.println("Enter " + list1.length + " values: ");
        for (int i = 0; i < list1.length; i++) {
            list1[i] = input.nextInt();
        }

        // Enter values for list2
        System.out.println("Enter the amount of numbers in the second list:");
        // Assign user defined length to the array
        int lengthOfArray2 = input.nextInt();
        int[] list2 = new int[lengthOfArray2];

        // i increments and the user values are assigned to the array until the length is exceeded
        System.out.println("Enter " + list2.length + " values: ");
        for (int i = 0; i < list2.length; i++) {
            list2[i] = input.nextInt();
        }
        // Sorts the lists in numerical order
        Arrays.sort(list1);
        Arrays.sort(list2);

        // Lists one and two are merged into one list
        int[] list3 = merge(list1, list2);
        // This accesses the actual user defined integers and not their locations, printing them in an assorted array
        System.out.println("The merged list is: " + Arrays.toString(list3));
    }

    private static int[] merge(int[] array1, int[] array2) {
        int length1 = array1.length;
        int length2 = array2.length;

        int[] array = new int[length1 + length2];

        int index1 = 0;
        int index2 = 0;

        while (index1 < length1 && index2 < length2) {
            array[index1 + index2] = array1[index1] < array2[index2]
                    ? array1[index1++]
                    : array2[index2++];
        }
        while (index1 < length1) {
            array[index1 + index2] = array1[index1++];
        }
        while (index2 < length2) {
            array[index1 + index2] = array2[index2++];
        }
        return array;
    }

}


Abstracting a Password Service I want to use Microsoft.AspNet.Identity's PasswordHasher but I don't want to pollute my Domain layer with knowledge of it. As such I created an interface in my Domain layer

public interface IPasswordService
{
    string HashPassword(string password);

    bool VerifyPassword(string hashedPassword, string testPassword);
}

So I can use DI for both IPasswordHasher and IPasswordService I created an adaptor:

public class PasswordServiceAdaptor : IPasswordService, IPasswordHasher
{
    private readonly PasswordHasher hasher;

    public PasswordServiceAdaptor()
    {
        this.hasher = new PasswordHasher();
    }

    string IPasswordHasher.HashPassword(string password)
    {
        return this.hasher.HashPassword(password);
    }

    PasswordVerificationResult IPasswordHasher.VerifyHashedPassword(
        string hashedPassword,
        string providedPassword)
    {
        return this.hasher.VerifyHashedPassword(hashedPassword, providedPassword);
    }

    string IPasswordService.HashPassword(string password)
    {
        return this.AsPasswordHasher().HashPassword(password);
    }

    bool IPasswordService.VerifyPassword(string hashedPassword, string testPassword)
    {
        var result = this.AsPasswordHasher().VerifyHashedPassword(
                         hashedPassword,
                         testPassword);

        return result == PasswordVerificationResult.Success;
    }

    private IPasswordHasher AsPasswordHasher()
    {
        return this;
    }
}

Is this a reasonable way to go about this? Are there any better patterns I can use?

This looks quite good, but as usual improvements can be made.

  private readonly PasswordHasher hasher;

instead of using the implementation, you should use the interface

private readonly IPasswordHasher hasher;

I don't see why you want to use both IPasswordHasher and IPasswordService. If your application is built upon IPasswordService you can just skip the implementation of the IPasswordHasher interface like

public class PasswordServiceAdaptor : IPasswordService
{
    private readonly IPasswordHasher hasher;

    public PasswordServiceAdaptor()
    {
        this.hasher = new PasswordHasher();
    }

    string IPasswordService.HashPassword(string password)
    {
        return this.hasher.HashPassword(password);
    }

    bool IPasswordService.VerifyPassword(string hashedPassword, string testPassword)
    {
        var result = this.hasher.VerifyHashedPassword(hashedPassword, providedPassword);

        return result == PasswordVerificationResult.Success;
    }
}

As it seems from the comments that you need to implement both interfaces, you should consider to just call the hasher directly without the call to AsPasswordHasher().

public class PasswordServiceAdaptor : IPasswordService, IPasswordHasher
{
    private readonly IPasswordHasher hasher;

    public PasswordServiceAdaptor()
    {
        this.hasher = new PasswordHasher();
    }

    string IPasswordHasher.HashPassword(string password)
    {
        return this.hasher.HashPassword(password);
    }

    PasswordVerificationResult IPasswordHasher.VerifyHashedPassword(
        string hashedPassword,
        string providedPassword)
    {
        return this.hasher.VerifyHashedPassword(hashedPassword, providedPassword);
    }

    string IPasswordService.HashPassword(string password)
    {
        return this.hasher.HashPassword(password);
    }

    bool IPasswordService.VerifyPassword(string hashedPassword, string testPassword)
    {
        var result = this.hasher.VerifyHashedPassword(
                         hashedPassword,
                         testPassword);

        return result == PasswordVerificationResult.Success;
    }
}


Scroll related inline image resizing I have been working the some project and I'm struggling to find the answer. Right now I'm coding with Angular and there some code that makes me write twice. There only two arrays now but if I have lots of arrays the code will be really big. The thing makes me stop always that I have to write the IDs one by one and I could not find the answer.

Please help me simplify the code even more.

$scope.allTheCont = [{
    styleClass: 'lftMn',
    theContId: 'theIdOne',
    pageNum: 'views/1.html',
    theOpn: function() {
        if ($scope.theIdOne) {
            return true
        };
    },
    imgClass: 'mainLeftImg',
    imgSrc: 'img/1.jpg',
    imgName: 'Traveling with sport',
    contClass: 'lftCnt',
    openCont: function() {
        $scope.all = false;
        $scope.theIdOne = true;
        $('.lftMn').addClass('lftMnOpnd')
        $('.lftCnt').addClass('mnCntOpnd')
        $('.mainLeftImg').addClass('mnImgOpnd')
        $location.hash('main');
        $anchorScroll();
    },
    closeCont: function() {
        $scope.all = true;
        $scope.theIdOne = false;
        $('.lftMn').removeClass('lftMnOpnd')
        $('.lftCnt').removeClass('mnCntOpnd')
        $('.mainLeftImg').removeClass('mnImgOpnd')
        $location.hash('theIdOne');
        $anchorScroll();
    }
}, {
    styleClass: 'mnRgtImg',
    theContId: 'theIdTwo',
    pageNum: 'views/2.html',
    theOpn: function() {
        if ($scope.theIdTwo) {
            return true
        };
    },
    imgClass: 'mnRgtImgs',
    imgSrc: 'img/2.jpg',
    imgName: 'Traveling with sport',
    contClass: 'mnRgtCnt',
    openCont: function() {
        $scope.all = false;
        $scope.theIdTwo = true;
        $('.mnRgtImg').addClass('mnRtOpnd')

        $('.mnRgtCnt').addClass('mnRtCntOpnd')
        $('.mnRgtImgs').addClass('mnRgtImgOpnd')
        $location.hash('main');
        $anchorScroll();
    },
    closeCont: function() {
        $scope.all = true;
        $scope.theIdTwo = false;
        $('.mnRgtImg').removeClass('mnRtOpnd')

        $('.mnRgtCnt').removeClass('mnRtCntOpnd')
        $('.mnRgtImgs').removeClass('mnRgtImgOpnd')
        $location.hash('theIdTwo');
        $anchorScroll();
    }

}]

You have a couple of methods that repeat themselves for the most part that could be factored out.  For example;

the openCont and closeCont are nearly identical, so you could factor them out to be their own function, for example,

var _openCont = function(theId, classesToSet) {
    $scope.all = false;
    $scope[theId] = true;
    for (var key in classesToSet) {
      $(key).addClass(classesToSet[key]);
    }
    $location.hash('main');
    $anchorScroll();
};

Which would be called like:

_openCont('theIdOne', {
  '.lftMn': 'lftMnOpnd',
  '.lftCnt': 'mnCntOpnd',
  '.mainLeftImg': 'mnImgOpnd'
});

Along with this, I'd recommend naming your variables more clearly.  Ideally code should be able to be read very easily even to a new set of eyes.  This will make maintainability far easier down the road.


Counting occurrences of values in C Array (Shannon Entropy) I have written the following C code for calculating the Shannon Entropy of a distribution of 8-bit ints. But obviously this is very inefficient for small arrays and won't work with say 32-bit integers, because that would require literally gigabytes of memory. I am not very experienced in C and don't know what would be the best approach here. If it would simplify things, I could use C++ or Objective-C...

Also, please tell me about any other issues with the code you may find :-)

double entropyOfDistribution(uint8_t *x, size_t length)
{
    double entropy = 0.0;

    //Counting number of occurrences of a number (using "buckets")
    double *probabilityOfX = calloc(sizeof(double), 256);
    for (int i = 0; i < length; i++)
        probabilityOfX[x[i]] += 1.0;

    //Calculating the probabilities
    for (int i = 0; i < 256; i++)
        probabilityOfX[x[i]] /= length;

    //Calculating the sum of p(x)*lg(p(x)) for all X
    double sum = 0.0;
    for (int i = 0; i < 256; i++)
        if (probabilityOfX[i] > 0.0)
            sum += probabilityOfX[i] * log2(probabilityOfX[i]);

    entropy = -1.0 * sum;

    free(probabilityOfX);

    return entropy;
}

btw, this is the formula I implemented:  $$ H(x) = -\sum_{x} p(x) \log(p(x))$$

Firstly, you know the size of the number of buckets you want to use here, so there is no reason to use dynamic allocation (specifically, double *probabilityOfX = calloc(sizeof(double), 256);). This could simply be:

double probabilityOfX[256];
memset(&probabilityOfX, 0.0, 256);

You don't need to free this memory at the end then, either, reducing the possibility for memory leaks.

Of course, this is fine for small values (like what can fit in a uint8_t), however, using a uint32_t (or larger), this will pre-allocate a large array which could potentially be very sparse. In this case, what you actually want is a dictionary data structure (like a hashmap). Since C doesn't have anything like this inbuilt, I'm going to switch over to C++ so we can use std::unordered_map and some other nice things like std::vector (instead of raw uintx_t pointers):

double entropyOfDistribution(const std::vector<uint32_t>& vec)
{
    std::unordered_map<uint32_t, unsigned> counts;

    // Store the number of counts
    for(uint32_t value : vec) {
        ++counts[value];
    }

    double sum = 0.0;
    // Note the cast as otherwise we'll be doing integer
    // division and hence rounding to an int -
    // thanks @syb0rg for pointing that out.
    const double num_samples = static_cast<double>(vec.size());

    for(auto it = counts.begin(); it != counts.end(); ++it) {
        double probability = it->second / num_samples;
        sum += probability * log2(probability);
    }

    return -1.0 * sum;
}


dispatch event listener problem after click My code is working fine...
but I want to remove below those(1) lines of code from my example code(2) (JS Code). If I remove those code then a tag is not working. (a tag do not open link or href is not working).
Is it a good approach to done it by writing those code? ( if any problem will occur for ajax request for this ) ? & Is there any way to work a tag by removing those line of code ? Please help me.
Wanna Remove Those Code(1)
 if (e.target.nextElementSibling == null) {

    if (e.target.getAttribute('target') != null) {
        window.open(e.target.getAttribute('href'), e.target.getAttribute('target'));
    } else {
        window.location.href = e.target.getAttribute('href');
    }

Example Code(2)

const lists = document.querySelectorAll('.sidenav nav ul ul')
lists.forEach(function (el) {
    el.className = 'sub-menu';
    el.style.height = 0 + "px";
})
document.querySelectorAll(".sidenav nav  ul li a ").forEach(el => el.addEventListener('click', function (e) {

    e.preventDefault();
    e.stopPropagation();

    if (e.target.nextElementSibling == null) {

        if (e.target.getAttribute('target') != null) {
            window.open(e.target.getAttribute('href'), e.target.getAttribute('target'));
        } else {
            window.location.href = e.target.getAttribute('href');
        }
    }

    let el = e.target.parentElement.children[1];
    let ul = e.target.parentElement.closest('ul');
    if (ul) {
        ul.querySelectorAll('ul').forEach(function (item) {
            item.style.height = 0 + 'px';
        })
    }

    if (typeof el == 'undefined') return;

    if (parseInt(getComputedStyle(el).height) > 0) {
        el.style.height = 0 + "px";
    } else {
        el.style.height = el.scrollHeight + "px";
    }
    return false;

}));
*,*::after,*::before{
    box-sizing: border-box;
}
html,body{
    margin: 0;
    padding: 0;
}
nav{
    padding: 20px;
    height: 4rem;
    width: 250px;
}
nav ul {
     margin: 0;
     padding: 0;
     list-style: none;
     display: flex;
     flex-direction: column;
}
nav ul li {

      background: tomato;
     width: 100%;

}
nav ul li a{
    display: block;
    padding: 10px;
    text-decoration: none;

}
nav ul ul {
  margin-left: 20px;
  height: 0;
    transition: all .2s ease-in ;
}

nav ul ul li {
      width: 100%;
    background: aqua;
}

Home

Cat

Cat 1

Cat 1

about

about

about

e.preventDefault();
e.stopPropagation();

if (e.target.nextElementSibling == null) {

    if (e.target.getAttribute('target') != null) {
        window.open(e.target.getAttribute('href'), e.target.getAttribute('target'));
    } else {
        window.location.href = e.target.getAttribute('href');
    }
}

Change that part to:
if (e.target.nextElementSibling == null) {
    return;
}

e.preventDefault();
e.stopPropagation();


Print all possible combinations of size r, from an array of size n This is my working solution for the following problem: given an array of integers of size n, print all possible combinations of size r.

Before I proceed to the solution, I have the following question: combination means that the order does not matter, right? I.e. printing {1, 2} is the same as {2, 1}, so I want to avoid repetitions?

If yes, here is what I do to avoid printing duplicate combinations: presort the given array arr, and if for some i I have that arr[i] == arr[i+1] I just skip this iteration.

Is my approach correct? Is there better solution?

package recursion;

import java.util.Arrays;

public class Combinations {
    public static void main(String[] args) {
        int[] arr = {1, 2, 3, 4, 4, 5};
        int r = 3;
        Arrays.sort(arr);
        combine(arr, r);
    }

    private static void combine(int[] arr, int r) {
        int[] res = new int[r];
        doCombine(arr, res, 0, 0, r);
    }

    private static void doCombine(int[] arr, int[] res, int currIndex, int level, int r) {
        if(level == r){
            printArray(res);
            return;
        }
        for (int i = currIndex; i < arr.length; i++) {
            res[level] = arr[i];
            doCombine(arr, res, i+1, level+1, r);
            //way to avoid printing duplicates
            if(i < arr.length-1 && arr[i] == arr[i+1]){
                i++;
            }
        }
    }

    private static void printArray(int[] res) {
        for (int i = 0; i < res.length; i++) {
            System.out.print(res[i] + " ");
        }
        System.out.println();
    }
}

I think that by avoiding duplicates, you are essentially changing the definition of the question at hand.

If that is indeed the definition of question, then instead of sorting the array and avoiding duplicates, you can simply get rid of the duplicates to begin with by using a set instead of an array:

Set<Integer> set = new HashSet<Integer>();
for (int i : arr)
    set.add(i);

In addition, for better modularity, I think that you should separate the algorithm itself from the printing:

Create an empty List<List<Integer>> instance
Pass it to the doCombine method, to fill it in with all combinations
Print it in the main method (the combine method looks kind of redundant)


Checking in four directions for board game win I have the following code which I would like to simplify in Haskell, although I'm not sure how. I recall that I can use monads to simplify a case chain when the result of the case leads onto a next bit of code but in my example, I return from the function when I get a value, and continue to the next bit of code if I don't get a value (Nothing).

checkWon :: Board -> Maybe Col
checkWon board = let moves = pieces board
                     n     = target board
                     col   = snd (moves !! 0) in
    case east moves col n of
        Just c  -> Just c
        Nothing ->
            case west moves col n of
                Just c -> Just c
                Nothing ->
                    case north moves col n of
                        Just c -> Just c
                        Nothing ->
                            case south moves col n of
                                Just c -> Just c
                                Nothing -> Nothing

The only other thing I've tried is using if statements where I calculate all the values of the functions before-hand and test against them in a if-else branch; but this still feels verbose. Is there any syntactic sugar in Haskell to simplify this, or is my current solution the best option? Maybe I would need to structure my code so I don't fall into this example?

Nice question! This is a little trickier than the usual “clean up a string of Maybes” in that you want short-circuiting on the Just case, and continued computation on Nothing. There is—of course—a function for that. Skip to the bottom for the answer, or read through for how you might find or derive it.

Finding the Answer

Hoogle. Pretty much always Hoogle. Think about what you've got and what you need, then start testing types against Hoogle. In this case you've got a sequence of Maybe a values that you want to reduce to a single value. The key insight is that the sequence must all be of the same Maybe a type, so it's possible (and thus probably necessary) to create a list (an actual [Maybe a]) of them. Thus our candidate type signature—

? :: [Maybe a] -> Maybe a

Pop that type signature into Hoogle and the very first thing that comes up is—

-- "base" Control.Monad
-- | This generalizes the list-based concat function.
msum :: MonadPlus m => [m a] -> m a

Test it in GHCi and—

> msum [Nothing, Just 1, Just 2]
Just 1
> msum [Just 1, Nothing, Just 2]
Just 1
> msum [Nothing] :: Maybe Int
Nothing

Yep, that's what we want.

Deriving the Answer

There are probably two different ways you might come up with your own solution. Either start at the concrete and generalize, or cobble a solution from typeclasses down.

From the Concrete

Consider again the type of the function you need.

? :: [Maybe a] -> Maybe a

A no-frills solution to this with primitive recursion and pattern matching should be self-obvious (or will be with experience).

firstJust :: [Maybe a] -> Maybe a
firstJust []             = Nothing
firstJust (Nothing:ms)   = firstJust ms
firstJust (j@(Just _):_) = j

From there we can deploy our usual bag of tricks higher-order functions to write this more idiomatically.

firstJust :: [Maybe a] -> Maybe a
firstJust = foldr orElse Nothing
    where
        orElse :: Maybe a -> Maybe a -> Maybe a
        orElse Nothing    m = m
        orElse j@(Just _) _ = j

Now a few facts may percolate to the top of your brain and strike you as insight.

Maybe encapsulates a notion of “failure”.
orElse provides an alternative value in the case of failure.
Alternative is a typeclass that exists.

So we get a spiffy one-liner—

firstJust :: [Maybe a] -> Maybe a
firstJust = foldr (<|>) Nothing

From the Typeclass Down

This all depends on which tools you're most familiar with. If you're a Monoid fan and used to relying on newtypes you may recognize the pattern of condensing a list of values as Control.Monoid.mconcat. Then you just need to write or choose an appropriate Monoid instance, and of course if you're really on top of the ball you'll know that one already exists.

import Control.Monoid
import Data.Coerce (coerce) -- GHC 7.8.1

firstJust :: [Maybe a] -> Maybe a
firstJust = getFirst . mconcat . coerce
-- Compatible with older GHC but slower, `getFirst . mconcat . map First`

Just the Answer

This version relies on MonadPlus, which ties up all of the other concepts we used and provides various handy functions to leverage their combined power. This version is as terse as it's gonna get.

import Control.Monad (msum) -- Or, Control.Applicative.asum

checkWon :: Board -> Maybe col
checkWon board = msum [ east moves col n
                      , west moves col n
                      , north moves col n
                      , south moves col n
                      ]
    where
        moves = pieces board
        n     = target board
        col   = snd (moves !! 0)


Rearrange characters of a string into a palindrome, if possible I have written Python code which works absolutely fine but I am looking for other ways to do this:

import itertools
def permutation(word):
   for i in range(len(word)+1):
      perms=list(itertools.permutations(word))
      for i in perms:
         j=(''.join(i))
         j=j.casefold()
         k=reversed(j)
         if list(j)==list(k):
            print("P")
            print(j)
word='Aabbccdd'
permutation(word)

All that is technically required of a palindrome is that exactly one or zero of the character counts are odd and the rest are even. Then from this character count information you can reconstruct all of the palindromic words. Here is some python code to compute this method:

from collections import Counter
from itertools import permutations, repeat

def get_palindromes(word, ignore_case=True):
    if ignore_case:
        word = word.lower()
    char_counts = Counter(word)
    num_odd_counts = sum(int(c % 2 != 0) for c in char_counts.values())
    if num_odd_counts > 1:
        return []
    odd_char = '' if num_odd_counts == 0 else [
        c for char, count in char_counts.items() if count % 2 == 1][0]
    even_chars = [rep_char for char, count in char_counts.items()
                  for rep_char in repeat(char, count / 2) if count % 2 == 0]
    return [perm_word + odd_char + perm_word[::-1]
            for perm_word in set(
                    ''.join(char_perm)
                    for char_perm in permutations(even_chars))]

If there are no palindromes this function will return an empty list, but you can obviously return any value you like in this case.


Using an Edit box in a table views header section to insert the values into the table using swift Allow me to provide some context. I have a UITableViewCell that contains an EditText and a Button. This cell is then inserted into the self.tableView.tableHeaderView property of the table to achieve the following result:

Below is my implementation for the view controller:

class ViewController: UITableViewController {

     var data = ["Apple", "Apricot"] as NSMutableArray

    override func viewDidLoad() {
        super.viewDidLoad()
        self.title = "Testing"
        createHeader()
    }

    func createHeader(){

        let headerViewCell = NSBundle.mainBundle().loadNibNamed("InputToListCell", owner: nil, options: nil)[0] as! InputToListCell
        headerViewCell.setTableViewWithData(self,data: data)
        self.tableView.tableHeaderView = headerViewCell

    }

    override func didReceiveMemoryWarning() {
        super.didReceiveMemoryWarning()
        // Dispose of any resources that can be recreated.
    }

    override func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return data.count
    }

    override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {

        let cell = tableView.dequeueReusableCellWithIdentifier("rowCell", forIndexPath: indexPath)
        cell.textLabel?.text = data[indexPath.row] as? String
        return cell
    }
}

Here is my implementation for the UITableViewCell that is inserted into the table:

class InputToListCell: UITableViewCell {

    @IBOutlet weak var inputEditText: UITextField!
    var tableViewController: ViewController!
    var arrayForTable: NSMutableArray!

    override func awakeFromNib() {
        super.awakeFromNib()
        // Initialization code
    }

    func setTableViewWithData(inputTableViewController:ViewController, data:NSMutableArray){
        tableViewController = inputTableViewController
        arrayForTable = data
    }

    override func setSelected(selected: Bool, animated: Bool) {
        super.setSelected(selected, animated: animated)
        // Configure the view for the selected state
    }

    @IBAction func addValueToArray(sender: AnyObject) {
        let inputFromEditText = inputEditText.text!
        arrayForTable.addObject(inputFromEditText)
        tableViewController.data = arrayForTable
        tableViewController.tableView.reloadData()
        tableViewController.createHeader()
    }
}

I have just started with iOS development and I dont have access to a senior developer to review my code to see if things could be improved or done in a better/best practices sort of way. Any help or suggestions would be great.

Random Fixes

You don't need to pre-pend with self in self.title = "Testing" or self.tableView.

var data = ["Apple", "Apricot"] as NSMutableArray

This doesn't need to be declared as an NSMutableArray.  Swift defines a mutable array like this
    var data = ["Apple", "Apricot"]
and if you wanted it to be non-mutable, then declare it with let

Remove the functions that Xcode auto populated your code with if you aren't actually doing something in them.  This includes didReceiveMemoryWarning, awakeFromNib, and setSelected.

Subclassing

I think you're incorrectly labeling the parts of a UITableView in your description.  Going by your comp, the UITextField and Add button are contained inside a tableHeaderView.  This shouldn't be subclassed as a UITableViewCell - if you look at the documentation for the tableHeaderView property of a UITableView you'll see its a UIView.  Let's just rename the InputToListCell as InputToListView and change it to a UIView.

Passing Data From the Header View to the TableView

As a general rule in iOS, a view that is a component of a larger system shouldn't know anything about nor should it be able to control that larger system.  Here, the component is the InputToListView and its parent is the UITableView.  The InputToListView shouldn't have an array of its own.  When it is declared in the ViewController, its button property should add a selector that is called inside the ViewController.  Delete both tableView and array properties from it.

func createHeader(){
    headerView = NSBundle.mainBundle().loadNibNamed("InputToListView", owner: nil, options: nil) as! InputToListView
    // delete the next line - the cell shouldn't need to know anything about what the table view has or doesn't have
    // the tableview already has its own array that it is using as its source of data
    //headerViewCell.setTableViewWithData(self,data: data)
    tableView.tableHeaderView = headerViewCell
    headerView.button.addTarget(self, action: "addButtonPressed", forControlEvents: UIControlEvents.TouchUpInside)
}

func addButtonPressed() {
// 1 access the headerView's textField property
// 2 check to see if the the textField has text by unwrapping it and if it has characters to display in the table view
// 3 add that text to the tableView's data array
// 4 either insert a new row into the tableView (preferred) or reload the tableView (messy)
}

Notice that the headerView should be a property of the ViewController.  That way you can access its textView property to use its text to add to the ViewController's array.  Conversely, you can declare a callback closure in the addButtonPressed: function of the header view and pass in the textView's text.


Tokenizing a file with a generator I'm parsing (specifically tokenizing) a file, line-by-line. I've a method tokenize, that takes a string (one line of code, it can't take the whole file at once), breaks it into parts, and returns a generator that yields those parts until it reaches the end of line. This is all given. I implemented some methods, that will give me all of the file parts (i.e. feeding each line to tokenize, and sending each line parts). Here's what I did, I feel it's very cumbersome and ugly.

class tokenizer:
    def __init__(self,read_file):
        self.file = read_file
        self.line = self.file.readline()
        self.gen = tokenize(self.line)
        self.token = Token("","","","")

    def advance(self):
        return self.token

    def hasMoreTokens(self):
        try:
            self.token = self.gen.__next__()
        except StopIteration:
            self.line = self.file.readline()
            if self.line == '':
                return False
            self.gen = tokenize(self.line)
            self.token = self.gen.__next__()
        return True

with open(sys.argv[1],'r') as file:
    tknzr = tokenizer(file)
    while tknzr.hasMoreTokens():
        print(tknzr.advance())

Could you advise me on how to make a more logical and sensible implementation?

Python already constructs a generator when you loop over a file object with a for loop. So you could simplify your code to:

with open(sys.argv[1],'r') as f:
    for line in f:
        for token in tokenize(line):
            print(token)

Note that I renamed the file variable to f, to avoid shadowing the built-in file.

If you really need to implement a class, you should implement the iterator protocol, allowing Python to iterate over your object. First, let me define a dummy tokenize function for testing purposes:

def tokenize(line):
    return line.split()

Then, let's define the class. Note that I renamed it in PascalCase, to adhere to Python's official style-guide, PEP8.

It has two important methods, first the __iter__ method, which just returns self. This just tells Python that this class is the actual iterator which it can iterate over. It is important for if you nest iter calls, namely iter(iter(tokenizer)) == iter(tokenizer).

The second important method is the __next__ method, which, just like the name suggests, tells Python how to get the next element from the iterator. It is similar to your implementation, only I use the iterator interface of the file. This method is called a second time if we got to a new line. It will stop at the end of the file, because then the unguarded next(self.file_it) will raise StopIteration, which the for loop will catch and stop iterating.

Note that since we call iter on the output of the tokenize function, it is enough for tokenize to return an iterable (this can be a list, like  here, but it can also be an iterator itself).

class Tokenizer:
    def __init__(self, f):
        self.file_it = iter(f)
        self.token_it = None

    def __next__(self):
        if self.token_it is None:
            self.token_it = iter(tokenize(next(self.file_it)))
        try:
            return next(self.token_it)
        except StopIteration:
            self.token_it = None
            return next(self)

    def __iter__(self):
        return self

if __name__ == "__main__":
    with open(sys.argv[1],'r') as f:
        for token in Tokenizer(f):
            print(token)

I also added a if __name__ == "__main__": guard around your code running the tokenizer to allow importing this class from other scripts.

Normally I would expect the tokenize function to be a method of the Tokenizer. Either directly defined:

class Tokenizer:
    ...
    def tokenize(self, line):
        return line.split()
    ...

Or, using the strategy pattern, plugged in at creation time:

def tokenize(line):
    return line.split()

class Tokenizer:
    def __init__(self, f, tokenize):
        self.f_it = iter(f)
        self.tokenize = tokenize
        self.token_it = None
    ...

Or, using inheritance:

class Tokenizer:
    ...
    def tokenize(self, line):
        raise NotImplementedError

class SplitTokenizer(Tokenizer):
    def tokenize(self, line):
        return line.split()


Project Euler #119 : Digit power sum From problem #119 of the Project Euler :

The number 512 is interesting because it is equal to the sum of its digits raised to some power: 5 + 1 + 2 = 8, and 83 = 512. Another example of a number with this property is 614656 = 284.
We shall define an to be the nth term of this sequence and insist that a number must contain at least two digits to have a sum.
You are given that a2 = 512 and a10 = 614656.
Find a30.

The number 512 fits because :

5+1+2 = 8
and 83 = 512.

The number 17576 fits because :

1+7+5+7+6 = 26
and 263 = 17576.

There's my solution but it's extremely inefficient. I need the variable c to reach 25 but I haven't seen past 16.
    public static int c = 0;
    public static void TryAll(long x, long y)
    {
        for (int i = 2; i < 10; i++)
        {
            double powered = Math.Pow(y, i);
            if (x % y == 0 && powered == x && x % 10 != 0)
            {
                c++;
                Console.WriteLine("----- {0}", c);
                Console.WriteLine(powered);
                Console.WriteLine(y);
                Console.WriteLine(i);
            }
        }
    }
    public static void Main(string[] args)
    {
        int baseNum = 0;
        for (c = c; c < 26; baseNum++)
        {

            if (baseNum > 9)
            {
                int sum = 0;
                int baseNumD = baseNum;
                while (baseNumD != 0)
                {
                    sum += baseNumD % 10;
                    baseNumD /= 10;
                }
                TryAll(baseNum, sum);
            }
        }
    }

If you don't need to print out results in order, you can inverse the logic and compute power value first, and check if the result equals to sum of digits. It's super fast:

using System;

public class Program
{
    private const int MaxCount = 25;

    public static void Main(string[] args)
    {
        var c = 1;
        for (var i = 2; i < 100; i++)
        {
            for (var j = 2; j < 1000; j++)
            {
                var pow = (long)Math.Pow(j, i);
                var sum = SumOfDigits(pow);
                if (sum != j)
                {
                    continue;
                }
                Console.WriteLine($"{c}: {pow} = {sum} ^ {i}");
                c++;
                if (c > MaxCount)
                {
                    break;
                }
            }
            if (c > MaxCount)
            {
                break;
            }
        }
        Console.ReadLine();
    }

    private static long SumOfDigits(long value)
    {
        long sum = 0;
        while (value != 0)
        {
            sum = sum + (value % 10);
            value = value / 10;
        }

        return sum;
    }
}

Please note loop limit values are arbitrary, large enough to satisfy requirement of 25 results.


Simple version of the Game Of Life This is just a very simple version of the Game of Life made in Java. Please let me know how I could have improved this. I am also hoping for some feedback on how the formatting and conventions are because I would like my code to be readable and neat.

public class GameOfLife
{
    private boolean[][] board;
    private int maxRow, maxCol;

    public GameOfLife(boolean[][] board)
    {
        if(board != null)
            this.board = board;
        else
            this.board = new boolean[1][1];

        this.maxRow = this.board.length;
        this.maxCol = this.board[0].length;
    }

    public void calculateNextGeneration()
    {
        boolean[][] nextBoard = new boolean[maxRow][maxCol];

        for(int row = 0; row < maxRow; row++)
        {
            for(int col = 0; col < maxCol; col++)
            {
                nextBoard[row][col] = nextState(row, col);
            }
        }

        board = nextBoard;
    }

    public void print()
    {
        for(int row = 0; row < maxRow; row++)
        {
            for(int col = 0; col < maxCol; col++)
            {
                if(isAlive(row, col))
                    System.out.print("*");
                else
                    System.out.print(".");
            }
            System.out.println();
        }
    }

    public void setAlive(int row, int col, boolean isAlive)
    {
        if(inBounds(row, col))
            board[row][col] = isAlive;
    }

    public boolean isAlive(int row, int col)
    {
        if(inBounds(row, col))
            return board[row][col];
        else
            return false;
    }

    private boolean nextState(int rowPos, int colPos)
    {
        int alive = 0;

        for(int row = rowPos - 1; row <= rowPos + 1; row++)
        {
            for(int col = colPos - 1; col <= colPos + 1; col++)
            {
                if(!inBounds(row, col) || (row == rowPos && col == colPos))
                    continue;

                if(isAlive(row, col))
                    alive++;
            }
        }

        if(alive == 3)
            return true;
        else if(isAlive(rowPos, colPos) && alive == 2)
            return true;
        else
            return false;
    }

    private boolean inBounds(int row, int col)
    {
        if(row < 0 || row >= maxRow || col < 0 || col >= maxCol)
            return false;
        else
            return true;
    }
}

just some minor things...

private boolean inBounds(int row, int col)
{
    if(row < 0 || row >= maxRow || col < 0 || col >= maxCol)
        return false;
    else
        return true;
}

could be more clear if you directly return the value

private boolean inBounds(int row, int col)
{
    return (row >= 0 && row < maxRow && col >= 0 && col < maxCol)
}

the same matters for isAlive()

public boolean isAlive(int row, int col)
{
    return inBounds(row, col) && board[row][col];
}

another thing is the name of the variable maxRows and maxColumns is not exactly what they are, i would use the simple forms for them rows and columns since they are rows and columns...

that would not even make conflicts for your iterations for(int row = 0; row < rows; row++)...

remove your whitespace, that makes reading easier...


Check whether all checkboxes are checked and then execute a function How could I make this code dry?

$("#btnSubmitToCheckOut").click(function(event) {
        var isSelected = [];

        $(".age_agree :checkbox").each(function() {
            if ($(this).is(":checked")) {
                isSelected.push("true");
            } else {
                isSelected.push("false");
            }
        });

        if ($.inArray("false", isSelected) < 0) {
            $(this).parents('form').submit();
        } else {
            alert('Please accept all the Terms and Conditions');
        }

    });

I was thinking something along these lines. The condition checks whether there are no unchecked checkboxes (length of the selection is 0) and then executes the appropriate branch.

$("#btnSubmitToCheckOut").click(function(event) {
    if ($('.age_agree input[type="checkbox"]').not(':checked').length == 0) {
        $(this).parents('form').submit();
    } else {
        alert('Please accept all the Terms and Conditions');
    }
});


Displaying all single posts I think there are some confusing and weird indentation problems in this WordPress file (single.php). Any suggestions on improving indentation and readability?

<?php
/**
 * The Template for displaying all single posts.
 *
 * @package WordPress
 * @subpackage Starkers
 * @since Starkers 3.0
 */

get_header(); ?>

<?php get_sidebar(); ?>

    <?php // Set and display custom field
        $intro_image = get_post_meta($post->ID, 'Intro Image', true); ?>

            <img src="<?php echo $intro_image; ?>" alt="" />

 <?php
    ?>

    <?php // Start The Loop
        if ( have_posts() ) while ( have_posts() ) : the_post();
    ?>

            <?php the_content(); ?>

    <?php endwhile; // end of the loop. ?>

    <?php //  Display previous and next posts thumbnails ?>

        <?php // Set and display custom field
            $mainbar_left_title = get_post_meta($post->ID, 'Mainbar Left Title', true);
            $mainbar_left_image = get_post_meta($post->ID, 'Mainbar Left Image', true); ?>

<?php echo $mainbar_left_title; ?>

                <img src="<?php echo $mainbar_left_image ?>" alt="" />

 <?php
        ?>

        <?php // Set and display custom field
            $mainbar_right_title = get_post_meta($post->ID, 'Mainbar Right Title', true);
            $mainbar_right_image = get_post_meta($post->ID, 'Mainbar Right Image', true); ?>

<?php echo $mainbar_right_title; ?>

                <img src="<?php echo $mainbar_right_image ?>" alt="" />

 <?php
        ?>

    <?php // Set and display custom field
        $mainbar_right_title = get_post_meta($post->ID, 'Mainbar Right Title', true);
        $mainbar_right_image = get_post_meta($post->ID, 'Mainbar Right Image', true); ?>

<?php echo $mainbar_right_title; ?>

            <img src="<?php echo $mainbar_right_image ?>" alt="" />

 <?php
    ?>

    <?php // Start The Loop
        if ( have_posts() ) while ( have_posts() ) : the_post();
    ?>

            <?php the_content(); ?>

    <?php endwhile; // end of the loop. ?>

    <?php get_sidebar('secondary'); ?>

<?php get_footer(); ?>

You could you PHP alternate syntax

<?php
/**
 * The Template for displaying all single posts.
 *
 * @package WordPress
 * @subpackage Starkers
 * @since Starkers 3.0
 */
?>

<?php get_header();
<?php get_sidebar(); ?>

    <?php $intro_image = get_post_meta($post->ID, 'Intro Image', true); ?>

        <img src="<?php echo $intro_image; ?>" alt="" />

    <?php if ( have_posts() ? while ( have_posts() ) : the_post()): ?>

            <?php the_content(); ?>

    <?php endwhile; ?>

        <?php $mainbar_left_title = get_post_meta($post->ID, 'Mainbar Left Title', true); ?>
        <?php $mainbar_left_image = get_post_meta($post->ID, 'Mainbar Left Image', true); ?>

<?php echo $mainbar_left_title; ?>

            <img src="<?php echo $mainbar_left_image ?>" alt="" />

            <?php $mainbar_right_title = get_post_meta($post->ID, 'Mainbar Right Title', true); ?>
            <?php $mainbar_right_image = get_post_meta($post->ID, 'Mainbar Right Image', true); ?>

<?php echo $mainbar_right_title; ?>

            <img src="<?php echo $mainbar_right_image ?>" alt="" />

    <?php $mainbar_right_title = get_post_meta($post->ID, 'Mainbar Right Title', true); ?>
    <?php $mainbar_right_image = get_post_meta($post->ID, 'Mainbar Right Image', true); ?>

<?php echo $mainbar_right_title; ?>

        <img src="<?php echo $mainbar_right_image ?>" alt="" />

    <?php if ( have_posts() ? while ( have_posts() ) : the_post()): ?>

            <?php the_content(); ?>

    <?php endwhile; ?>

    <?php get_sidebar('secondary'); ?>

<?php get_footer(); ?>

This follows all WP coding standards from what I can tell and is readable.
I would also suggest changing sections like this:

<?php $mainbar_right_title = get_post_meta($post->ID, 'Mainbar Right Title', true); ?>
<?php $mainbar_right_image = get_post_meta($post->ID, 'Mainbar Right Image', true); ?>

<?php echo $mainbar_right_title; ?>

    <img src="<?php echo $mainbar_right_image ?>" alt="" />

To:

<?php echo get_post_meta($post->ID, 'Mainbar Right Title', true); ?>

    <img src="<?php echo get_post_meta($post->ID, 'Mainbar Right Image', true) ?>" alt="" />

Although that's just personal preference, I find it much more readable.


jQuery Fallback Support for the CSS Property "background-attachment: local" I have made some jQuery Fallback Support for the CSS Property "background-attachment: local" and I am mainly a CSS developer. I do some jQuery, but I thought I would check to see if I am doing this in the more efficient way, as I may have multiple versions of this on one page.

CODEPEN DEMO

HTML

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

hello

CSS

@import "http://fonts.googleapis.com/css?family=Open+Sans:400,600,700";
* {
    -moz-box-sizing: border-box;
    -webkit-box-sizing: border-box;
    box-sizing: border-box;
}
body {
    background: none repeat scroll 0 0 #fff;
    font-family: "Open Sans";
    line-height: 26px;
    margin: 20px;
}

.container {
  position: relative;
  margin-top: 30px;
}

.data-holder {
  border-top: 1px solid #eee;
  border-bottom: 1px solid #eee;
  width: 200px;
  height: 300px;
  overflow: auto;
}

#shadowtop,
#shadowbottom {
  position: absolute;
  left: 0;
  height: 12px;
  width: 180px;
  z-index: 9999;
  display: none;
  background-size: 100% 12px;

}
#shadowtop {
  top: 0;
  background:   radial-gradient(farthest-side at 50% 0, rgba(0,0,0,.15), rgba(0,0,0,0)) 0 100%;
}
#shadowbottom {
  bottom: 0;
  background:   radial-gradient(farthest-side at 50% 100%, rgba(0,0,0,.15), rgba(0,0,0,0)) 0 100%;
}

.block {
  background: #fff;
  border-bottom: 1px solid #f4f4f4;
  padding: 10px;
}
.block:last-child {
  border-bottom: 0 none;
}

jQuery

$(document).ready(function(){
  $('.data-holder').scroll(function(){
    var scrollValue = $(this).scrollTop();
    var elementHeight = $(this).height();
    if(scrollValue == 0){
      $("#shadowtop").fadeOut(200);
      console.log("at top");  //should remove
    } else if (scrollValue == ($(this).get(0).scrollHeight - $(this).height())){
      $("#shadowbottom").fadeOut(200);
      console.log("at bottom"); //should remove
    } else {
      console.log("in middle"); //should remove
      $("#shadowtop").fadeIn(200);
      $("#shadowbottom").fadeIn(200);
    }
  });
  var scrollValue = $('.data-holder').scrollTop();
  if(scrollValue < ($('.data-holder').get(0).scrollHeight - $('.data-holder').height())){
     $("#shadowbottom").show();
  }
});

This code annoyed me for a while, the part outside of the initialization does not belong and it seemed silly to add it to the listener. Plus, the code outside of the listener will mess up when you have multiple data-holder elements.

Then I realized that I could take a different approach, do this in 2 steps:

Determine whether I should hide or show the top
Determine whether I should hide or show the bottom

Then, I do not need the piece of code outside of the listener, and it becomes cleaner.
Also, the 200 has to be a constant, also I would take only the #shadowbottom and #shadowtop belonging to the .data-holder.

So, in all, I would counter propose:

$(document).ready(function(){
  var fadeSpeed = 200;
  $('.data-holder').scroll(function(){
    var scrollValue = $(this).scrollTop(),
        elementHeight = $(this).height(),
        $top = $(this).children('#shadowtop'),
        $bottom = $(this).children('#shadowbottom'),
        atTop = (scrollValue == 0),
        atBottom = (scrollValue == ( this.scrollHeight - elementHeight));
    (atTop)   ? $top.fadeOut( fadeSpeed )    : $top.fadeIn( fadeSpeed );
    (atBottom)? $bottom.fadeOut( fadeSpeed ) : $bottom.fadeIn( fadeSpeed );
  }).scroll();
});

I was mildly worried that with each scroll event the top / bottom might flicker because of the repeated fadeIn() calls, but I can tell from your CodePen that does not happen, so there is nothing to worry about there.

Finally, I did not look at your CSS as you actually are a CSS developer and also I did not test my code, but I am confident you get where I am going with this.


Using Mockito to verify that an object is saved We've been reviewing some legacy code and have found differences in the preferred approach to how to write tests using Mockito.

Both the following tests pass but which one is better and why?

@Test
public void verifyObjectSaved() throws PersistQueueException {
    final UserTokenVo userTokenVo = new UserTokenVo();

    String token = vodafoneSubscriptionService.processTokenRequest(userTokenVo);

    Mockito.verify(userTokenDao).savePayload(isA(String.class), eq(userTokenVo));

    assertTrue(StringUtils.isNotBlank(token));
}

and

@Test
public void verifyObjectSaved() throws PersistQueueException {
    final UserTokenVo userTokenVo = new UserTokenVo();

    doAnswer(new Answer() {
        public Object answer(InvocationOnMock invocation) {
            return "completed";
        }
    }).when(userTokenDao).savePayload(any(String.class), eq(userTokenVo));

    String token = vodafoneSubscriptionService.processTokenRequest(userTokenVo);

    assertTrue(StringUtils.isNotBlank(token));

    verify(userTokenDao, times(1)).savePayload(any(String.class), eq(userTokenVo));
}

Here's the production code it tests — it's not very complicated:

public String processTokenRequest(UserTokenVo userTokenVo) {
    String token;

    try {
        token = new String(TokenFactory.getToken());
        if (token != null) {
            userTokenDao.savePayload(token, userTokenVo);
        }
    } catch (Exception e) {
        LOGGER.error("failed to save token");
        token = null;
    }
    return token;
}

Looking at your production code, my positive test case would check the getToken() and savePayload() methods were invoked; and the same string returned from getToken() was also returned by processTokenRequest().

Some points:

I wouldn't use the any matcher with verify(). Explicitly state what you want.
No need to return value from a method when you don't use the returned value i.e. doAnswer for savePayload() is unnecessary.
As mentioned, times(1) is the default for verify() and used in your first test.
Tests shouldn't throw exceptions.
Static imports often make short code easy to read.
Is isNotBlank() enough?


A program for fun to see how red and blue appears blended to purple due to resolution I do some art both analog and digital. This time I did some color test digitally. To work on my programming skill so that it's shareable I'm asking for help to get some code review.

How's the readability?
Is it easy to follow line of thought?
Suggestion as to how to improve?

I'm using John Zelles graphics.py for this

#A program for fun to see how red, blue and purple appears at resolution

from math import *
from graphics import *

def hole(win, centerx, centery, radius): #draws an inverted sphere
    for circle in range(radius, 0, -1):
        c = Circle(Point(centerx, centery), circle)
        c.setWidth(0)
        coloratangle = int(sin(circle/radius*pi/2)*255) #max resolution from black to white is 255 steps
        c.setFill(color_rgb(coloratangle * (circle % 2), 0, coloratangle * ((circle + 1) % 2))) # Modulus instead of Boolean? Modulus in c.setFill or in another line?
        c.draw(win)

windowheight = 1350
windowwidth = 730
win = GraphWin("My Circle", windowheight, windowwidth)
win.setBackground(color_rgb(255, 100, 100))

centerx = int(windowheight / 2)
centery = int(windowwidth / 2) #intetger
radius = int(sqrt(centerx**2 + centery**2)) #pythagoras

hole(win, centerx, centery, radius)

win.getMouse() # Pause to view result
win.close()    # Close window when done

I actually ran this. Good thing I don't have epilepsy, because wow. The flashing is intense.

There are many ways to improve this, depending on how technical you want to get. Move to a more advanced graphics library that's able to draw circles in an off-screen memory buffer before showing them in tkinter, your current windowing framework. This will speed up the rendering significantly and won't create that flashing (unless that's part of your art?).

There's a bug where moving your window creates even more flashing - this time, instead of being due to the concentric circles being rendered from the outside in, I suspect it's from the circles being re-rendered by graphics.py rather than flattening the representation of the image to a rasterized buffer. Rendering to an off-screen buffer will fix this, too.

There's another bug: closing the window produces this stack trace -

  File "/opt/pycharm-community-2019.2.1/helpers/pydev/pydevd.py", line 2060, in <module>
    main()
  File "/opt/pycharm-community-2019.2.1/helpers/pydev/pydevd.py", line 2054, in main
    globals = debugger.run(setup['file'], None, None, is_module)
  File "/opt/pycharm-community-2019.2.1/helpers/pydev/pydevd.py", line 1405, in run
    return self._exec(is_module, entry_point_fn, module_name, file, globals, locals)
  File "/opt/pycharm-community-2019.2.1/helpers/pydev/pydevd.py", line 1412, in _exec
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/opt/pycharm-community-2019.2.1/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/home/gtoombs/PycharmProjects/zelles/zelles.py", line 25, in <module>
    win.getMouse() # Pause to view result
  File "/home/gtoombs/.virtualenvs/zelles/lib/python3.7/site-packages/graphics/__init__.py", line 316, in getMouse
    if self.isClosed(): raise GraphicsError("getMouse in closed window")
graphics.GraphicsError: getMouse in closed window

which means that you're not handling shut-down gracefully. See if you can change your exit condition to be either when the mouse event is received or the window is closed.


Determine whether a list contains a particular sequence Allowing wraparound, the sequence [2, 3, 5, 7, 11] contains the sequence [7, 11, 2].

I'm looking for compact/pythonic way to code this, rather than an optimally-efficient way. (Unless I need the efficiency, I usually gravitate to the path of least ASCII).

An obvious method would be as follows (I provide three different ways to code it):

def Contains(L, s):
    L_extended = L+L[0:len(s)]

    for i in xrange( len(L) ):
        if L_extended[i:i+len(s)] == s:
            return True

    return False

def Contains2(L, s):
    L_extended = L+L[0:len(s)]

    return next( \
        ( True for i in xrange(len(L))  if  L_extended[i:i+len(s)] == s ), \
        False \
        )

def Contains3(L, s):
    L_extended = L+L[0:len(s)]

    try:
        next( i  for  i in xrange(len(L))  if  L_extended[i:i+len(s)] == s )
    except StopIteration:
        return False

    return True

print Contains3( [1,2,3,4,5], [1,3] )   # False
print Contains3( [1,2,3,4,5], [2,3] )   # True
print Contains3( [1,2,3,4,5], [4,5,1] ) # True

My preference is for the first, but Python is full of pleasant surprises -- can anyone see a cleaner way to do it?

As you said in your comments above, the first is the cleanest way you came up with.

Instead of writing your own logic, Python has a built-in library difflib that contains the SequenceMatcher class. You can use this to find the longest match between two iterables:

from difflib import SequenceMatcher

def contains(main_list, sub_list, junk=None):
    sub_len = len(sub_list)
    extended = main_list + main_list[:sub_len]

    sequence = SequenceMatcher(junk, extended, sub_list)
    return sequence.get_longest_match(0, len(extended), 0, sub_len).size == sub_len

My only qualm with get_longest_match is that its parameters are ALL positional arguments and are thus required. I would prefer the API assume we want the entire length of both iterables searched. Instead of using get_longest_match you could instead use the get_matching_blocks function (which returns tuples) then iterate through those until the corresponding tuple value is equal to sub_len.

By using SequenceMatcher you also get the flexibility to declare what 'junk' you want the class to ignore. So say you want to find the longest match between the two lists, but you don't care about 1 or 2. You would simply pass the lambda:

lambda num: num in [1, 2]

as the junk parameter.

Now that I've given my structure recommendation, I want to give some style pointers.

First of all, take a look at PEP8, the official Python style guide. It gives a great reference to how your code should look. Here are some places your code strays from the Python conventions:

Use descriptive variable names. Single letter variable names are typically frowned-upon. When naming, err on the side of being too descriptive instead of too brief.
Use underscores_in_names for basically everything in Python. The only aberration from this rule is class names which are written in PascalCase.

Also, variable and function/method names should be lowercase. Conventionally, uppercase names are saved for constants. So by convention, you L variable is considered constant.
Your spacing is quite nice; except for a few occasions where you place a single space after an opening paren and before the close paren. PEP8 actually labels that style as a pet peeve.
Multi-line statement happen. PEP8 purposefully takes a neutral stance on how those should be formatted. However, Python is quite lenient in terms of newlines in statements, so more than likely, there is a better way to format your multi-line statements without escaping the newlines.

def Contains2(L, s):
    L_extended = L+L[0:len(s)]

    return next((True for i in xrange(len(L))
                 if  L_extended[i:i+len(s)] == s), False)


Project Euler #7: Nth prime number I was solving Project Euler #7 on Hackerrank
 but for two cases, execution time is more than 2 sec.
Can you help with optimizing? I'm new to programming.

#include <stdio.h>
#include <stdbool.h>
/*INPUT EXAMPLE
2 -> no of test cases
3 -> 1st test case
5 -> 2nd test case
*/
bool IsPrime(unsigned long);

int main(){
  int n;
  scanf("%d", &n);

  while (n != 0) {
    n--;
    unsigned long m, l = 2, k = 1;
    scanf("%lu", &m);
    if (m == 1) {
      printf("%d\n", 2);
    } else if(m == 2){
      printf("%d\n", 3);
    }else{
    while(1){
      if(IsPrime(6*k-1) == true){
        l++;
        if(l == m){
          printf("%lu\n", 6*k-1);
          break;
        }
      }
      if(IsPrime(6*k+1) == true){
        l++;
        if(l == m){
          printf("%lu\n", 6*k+1);
          break;
        }
      }
      k++;
    }
  }
  }
}
bool IsPrime(unsigned long n){
    unsigned long k;
    for(k = 2; k*k <= n; k++){
        if(n % k == 0){
            return false;
        }
    }
    return true;
}

bool IsPrime(unsigned long n){
    unsigned long k;
    for(k = 2; k*k <= n; k++){
        if(n % k == 0){
            return false;
        }
    }
    return true;
}

A couple things off the bat.

bool isPrime(unsigned long n) {
    for (unsigned long k = 2; k*k <= n; k++) {
        if (n % k == 0) {
            return false;
        }
    }

    return true;
}

The general standard is to name functions either camelCase or snake_case.  You used PascalCase, which is generally only used for classes and structs.

It is standard to declare loop variables that are not used outside the loop in the for declaration.  That's one of the advantages of a for loop, that it allows you to do that.

But neither of those would cause performance problems.  This code checks if each number from 2 to \$\sqrt{n}\$ is a factor of n.  But you don't need to check all the numbers.  You only need to check the primes.  And of course, you generate a list of primes, so it's easy enough to save that and check just the primes.

Something like

bool isPrime(unsigned long n, unsigned long *primes) {
    for (size_t k = 0; primes[k]*primes[k] <= n; k++) {
        if (n % primes[k] == 0) {
            return false;
        }
    }

    return true;
}

Back in the calling function, you will need to add some code to handle primes.

Before the loop.

unsigned long *primes = malloc(2 * sizeof *primes);
if (!primes) {
    // panic:  exit, return, or whatever seems reasonable
}
primes[0] = 2;
primes[1] = 3;

size_t length = 2;

Then take

    unsigned long m, l = 2, k = 1;
    scanf("%lu", &m);
    if (m == 1) {
      printf("%d\n", 2);
    } else if(m == 2){
      printf("%d\n", 3);
    }else{
    while(1){

and replace it with

    size_t m;
    scanf("%u", &m);
    if (m > length) {
        unsigned long *temp = realloc(primes, m * sizeof *primes);

        if (!temp) {
            free(primes);
            // panic:  same way as last time
        }

        primes = temp;
    }
    m--;

    unsigned long candidate = 5;
    unsigned long increment = 4;
    while (length <= m) {
         increment = 6 - increment;

This changes the array index to size_t.  This will easily support 10,000, which is the maximum m (what HackerRank calls \$N\$).

This also changes l to a size_t, since it has to be smaller than m and it will index an array.

This expands the primes array as necessary to handle the inputs.

I moved declarations to when they are initialized.

Your k code has some problems:

You do an increment, a multiplication, a subtraction, and an addition every two iterations.
You do two iterations of l per loop iteration.
You have to duplicate code to handle both the same.
You have to loop forever since the exit criterion is hidden.
You have to check two conditions before the loop to handle the first two primes.

This version of the code prepares to do

One iteration per loop iteration.
The exit criterion is in the loop declaration.
This does one subtraction and one addition on every iteration.
The code no longer needs duplicated.
No special cases are needed for the first two primes.
The solution can be printed after the loop, once rather than in four different places.
If some previous test looked for a later prime, we remember that and don't enter the loop at all.

The candidate variable will have the same values as 6*k-1 and 6*k+1.  The increment will alternate between 2 and 4 (6-2=4 and 6-4=2).  So candidate will be 5, 7, 11, 13, 17, ...  Just as previously.

The rest of the loop can be

        if (isPrime(candidate, primes)) {
            primes[length] = candidate;
            length++;
        }

        candidate += increment;
    }

And then

    printf("%lu\n", primes[m]);

If m was originally 1, it will now be 0 and primes[0] is 2.  If m was originally 2, it will now be 1 and primes[1] is 3.  So there's the special cases.

Outside the outer loop, you should

free(primes);

The big performance improvements are

Not recalculating the same primes for each test case.
Not checking for non-prime factors in isPrime.


SQL query on relational schema I'm trying to find the best solutions for operations on the following database:

  Employee (EmployeeId, Surname, Salary, DepartmentId)
  Department (DepartmentId, DeptName, City, Director)
  Project (ProjectId, ProjectName, Budget, Director)
  Works (EmpoyeeId, ProjectId)

My queries:

1 SQL query to find the surname of the employees who are directors of a department.

SELECT Surname
FROM Employee
WHERE Employee.EmployeeId IN (
SELECT Department.Director
FROM Department
);

2 SQL query to find the employee surname and salary of all employees who work in Haarlem.

SELECT Surname, Salary, City
FROM Employee,Department
WHERE Department.City = ‘Haarlem’;

3 SQL query to find the names of the projects whose budget is higher than 100 and the surnames of the employees who work on them.

SELECT ProjectName, Budget, Surname
FROM Project, Employee, Works
WHERE Project.Budget > 100
AND Works.EmployeeId = Employee.EmployeeId
AND Works.ProjectId = Project.ProjectId;

Are there any better ways to get the information needed?

SELECT Surname
FROM Employee
JOIN Department ON Department.Director = Employee.EmployeeId;

Other than not requiring a subquery, this is generally a better thing to do because you can then easily access the matching Department table if you ever need to select a column from it.
Are you sure that this even works? You aren't joining the employee's department to the department ID. This should likely be:

SELECT Surname, Salary, City
FROM Employee
JOIN Department ON Department.DepartmentId = Employee.DepartmentId
               AND Department.City = 'Haarlem';

SELECT ProjectName, Budget, Surname
FROM Employee
JOIN Works ON Works.EmployeeId = Employee.EmployeeId
JOIN Project ON Project.ProjectId = Works.ProjectId
            AND Project.Budget > 100;

This should be functionally equivalent but uses cleaner syntax.


Scraper for a lazy-loading webpage I've written a script in VBA in combination with selenium which is able to scrape all the 1000 links from a lazy-loading webpage which displays it's content 20 at a time and doesn't display the full content until it reaches the bottom of that page. However, my script can make this webpage scroll to the end. After collecting all the links from main page, it then follows each individual link to scrape the Name of CEO and Web address of that organization. It is working perfectly now. I tried to make the whole thing flawless. Here is the full code:

Sub Fortune_parser()

Dim driver As New WebDriver
Dim posts As Object, post As Object, items As Object, item As Object

driver.Start "chrome", "http://fortune.com/fortune500"
driver.get "/list/"

driver.Wait 500

For y = 0 To 200

    driver.ExecuteScript ("window.scrollTo(0, document.body.scrollHeight);")
    driver.Wait 500

Next y

Set posts = driver.FindElementsByXPath("//li[contains(concat(' ', @class, ' '), ' small-12 ')]")

For Each post In posts

    i = i + 1
    item_links = post.FindElementByXPath(".//a").Attribute("href")

    driver.Start "chrome", ""
    driver.get item_links

    driver.Wait 500

    Set items = driver.FindElementsByXPath("//div[contains(@class,'company-info-card-table')]")

    For Each item In items
        x = x + 1
        Cells(x, 1) = item.FindElementByXPath(".//div[contains(@class,'small-7')]/p[@class='remove-bottom-margin']").Text
        Cells(x, 2) = item.FindElementByXPath(".//div[contains(@class,'small-9')]/a").Attribute("href")
        driver.Quit
    Next item
Next post

End Sub

Sub Fortune_parser()

The procedure is implicitly Public; I'd make it explicitly so. It's also using a rather uncommon casing style I don't even have a name for. In every single VBA type library out there[citation needed], public members are PascalCase.

Note that classes that expose public members that have underscores in their names, cannot be used with the Implements keyword; the underscore has a special meaning in VBA, and thus its use should be avoided in member names.

Dim driver As New WebDriver

If you had Rubberduck installed, this would pop an inspection result about driver being auto-instantiated. The way VBA treats those is rather unsettling (and frankly annoying) - essentially, you can't destroy an object that was declared with As New. Best split declaration and assignment into two distinct instructions:

Dim driver As WebDriver
Set driver = New WebDriver

I see you've avoided the classic beginner trap:

Dim posts As Object, post As Object, items As Object, item As Object

Well done! However posts is declared there, and then assigned 8 instructions later. post is used further down, and items+item are even further. I wouldn't declare all variables at the top of the procedure; by doing that it's easy to declare a variable and then forget about it, and then you're stuck with dangling declarations for things you're not using anywhere.

Consider:

Dim driver As WebDriver
Set driver = New WebDriver
driver.Start "chrome", "http://fortune.com/fortune500"
driver.Get "/list/"
driver.Wait 500

'...

Dim posts As Object
Set posts = driver.FindElementsByXPath("//li[contains(concat(' ', @class, ' '), ' small-12 ')]")

Dim post As Object
For Each post In posts
    '...
Next

That last loop is interesting. Where are i, item_links and x declared? Are you not using Option Explicit?

I think I would make a With block hold the reference to the driver object; that way I'm sure it's destroyed proper.

So scratch the part above about splitting declaration and assignment; don't even declare it, and wrap the whole procedure body with this:

With New WebDriver
    .Start "chrome", "http://fortune.com/fortune500"
    '...
    .Quit
End With

You're not consistent about parentheses; these ones are redundant:

driver.ExecuteScript ("window.scrollTo(0, document.body.scrollHeight);")

Notice the whitespace between the method name and the opening parens; these parentheses are telling VBA to take that string literal and pass it by value to the ExecuteScript method.

Should be:

driver.ExecuteScript "window.scrollTo(0, document.body.scrollHeight);"

That way this method call looks like every other method call you have, and doesn't force any argument to be passed ByVal.

There's some implicit code here:

Cells(x, 1) = item.FindElementByXPath(".//div[contains(@class,'small-7')]/p[@class='remove-bottom-margin']").Text
Cells(x, 2) = item.FindElementByXPath(".//div[contains(@class,'small-9')]/a").Attribute("href")

Unqualified Cells calls like these, implicitly refer to the ActiveSheet, which may or may not be set to a valid reference, and may or may not be the sheet you expect. You're also implicitly assigning to the default property of that Range object (the reference returned by Cells).

Explicit code would be:

ActiveSheet.Cells(x, 1).Value = item.FindElementByXPath(".//div[contains(@class,'small-7')]/p[@class='remove-bottom-margin']").Text
ActiveSheet.Cells(x, 2).Value = item.FindElementByXPath(".//div[contains(@class,'small-9')]/a").Attribute("href")

Your code will work fine, until the scraped website changes their CSS classes. I would extract the CSS class names from the string literals and into a local constant, e.g.:

Const CSS_CLASS_HREF As String = "small-9"

And then concatenate the constant into the xpath query:

item.FindElementByXPath(".//div[contains(@class,'" & CSS_CLASS_HREF & "')]/a").Attribute("href")

That way if/when the CSS changes, you don't need to dig up any specific instructions, you just update your CSS class names constants to mirror the website's changes.

Error handling is missing. What happens when the site is down? When FindElementByXPath returns Nothing? A flawless procedure would handle these sticky situations gracefully.