Extension method to return a default value for any null value I'm creating an extension method that can be used on any type.  The idea is that if this method is called, it checks if value is null.  If null, it needs to return a default instance of the specified type.

public static dynamic CreateDefaultIfNull<T>(this object item)
    {
        if(item == null)
        {
            var type = typeof(T);

            if(type.IsArray)
            {
                return Array.CreateInstance(type.GetElementType(), 0);
            }

            if(type.GetConstructor(Type.EmptyTypes) == null)
            {
                var paramCount =  type.GetConstructors().First().GetParameters().Count();

                var paramNullList = new List<object>();

                for(int i = 0; i < paramCount; i++)
                {
                    paramNullList.Add(null);
                }

                return Activator.CreateInstance(type, paramNullList.ToArray());
            }

            return Activator.CreateInstance(type);
        }

        return (T)item;
    }

I'm looking for a review of my code, and any pitfalls that I may encounter or may have missed.

The idea is that this method should be used anytime that a null value cannot be used and a default must be returned.

I'm not worried about null values being passed into parameterized constructors, as this is doing.  The constructor of those objects should be handling if null values are passed into their constructors.

EDIT: Based on the information provided, I've updated my code.  The current version is below:

public static dynamic CreateDefaultIfNull<T>(this T item)
    {
        if (item != null)
            return item;

        var type = typeof(T);

        if (type.IsArray)
        {
            return Array.CreateInstance(type.GetElementType(), 0);
        }

        if (type.GetConstructor(Type.EmptyTypes) == null)
        {
            var paramCount = type.GetConstructors().Min(construct => construct.GetParameters().Count());
            var constructorToUse = type.GetConstructors().Where(construct => construct.GetParameters().Count() == paramCount).First();

            var paramNullList = new object[paramCount];

            var parameters = constructorToUse.GetParameters();
            for (int i = 0; i < paramCount; i++)
            {
                paramNullList[i] = parameters[i].ParameterType.IsValueType ? Activator.CreateInstance(parameters[i].ParameterType) : null;
            }

            return Activator.CreateInstance(type, paramNullList);
        }

        return Activator.CreateInstance(type);
    }

Took a quick look at your code and a few things came to mind.

As Magus mentioned in the comments it seems odd that you have a generic extension method on the object class, which returns a dynamic type. You should (with a few modifications) be able to use the generic type instead:

public static T CreateDefaultIfNull<T>(this T item)

Since it rarely makes sense to check value types for null, you could also restrict it to reference types only.

public static T CreateDefaultIfNull<T>(this T item) where T : class

You will have to cast the result of CreateInstance before returning.

The name is bothering me. The default value for a reference type is null, so if I saw your method used somewhere I would wonder what exactly it returns. (I don't know of a better name though. This method could return all kinds of things.)

I would invert the first if statement. It would reduce some of the indentation and I think it looks cleaner:

if(item != null)
{
    return item;
}
...

I haven't worked much with the GetConstructors method, but in what order is the constructors returned? If there is no guarantee, can you be sure that the first constructor is the most appropriate?

I have often seen constructors that throw exceptions when given null. Is that the behavior you want?

EDIT: If you look here, you will see that there is no guarantee of the order and it is not recommended to depend on the order.

  The GetConstructors method does not return constructors in a particular order, such as declaration order. Your code must not depend on the order in which constructors are returned, because that order varies.

Also, as mentioned in the comments, you should probably look into the situation mentioned by Jesse in the comments.

  What will happen if the "first" constructor's parameter list has value types (such as int)..

It might/might not work as you expect in all situations. (I don't know what the defined behavior is)

You should be able to change

var paramNullList = new List<object>();
for(int i = 0; i < paramCount; i++)
{
    paramNullList.Add(null);
}
return Activator.CreateInstance(type, paramNullList.ToArray());

to

var nullParams = new object[paramCount]; //All elements null by default.
return Activator.CreateInstance(type, nullParams);

Finally, your method doesn't work if there is no public constructor. What should happen in that case? You can't select the first constructor if there are none, so First would throw an exception (InvalidOperationException if I remember correctly).


iterative binary_search Python implementation I did a Python implementation of binary search implementation
iteratively. It passes all of the test and I was wondering if I can
get any feedback.

#!python

def binary_search_iterative(array, item):
    """iteratively return the index of the item with binary search or None if not found"""
    # Sorted array that has to be the condition
    # go through several iteration until we found this
    # First, set the lower_index = 0
    # upper_index as the length of array -1
    lower_index = 0
    upper_index = len(array) - 1
    # iteration until it is found
    while lower_index <= upper_index:
        # calculate the middle index by sum the lower_index and upper_index and divide by 2
        middle_index = (lower_index + upper_index) // 2 # // is int division
        # check if the item is found
        if array[middle_index] == item:
            return middle_index
        elif array[middle_index] < item:
            lower_index = middle_index + 1
        elif array[middle_index] > item:
            upper_index = middle_index - 1

    # Did not find
    return None

def binary_search(array, item):
    """return the index of item in sorted array or None if item is not found"""
    # change this to call your implementation to verify it passes all tests
    return binary_search_iterative(array, item)

link to my linear search code:
https://gist.github.com/anonymous/be81dc4ab1a26328785ed8cffce33346

def linear_search(array, item):
    for index, value in enumerate(array):
        if item == value:
            return index  # found
    return None  # not found

#!python

from search import linear_search, binary_search
import unittest

class SearchTest(unittest.TestCase):

    def test_binary_search_with_items_in_list(self):
        # binary search requires list values to be in sorted order
        names = ['Alex', 'James', 'Julia', 'Matthew', 'Nathan', 'Conrad', 'John']
        # binary search should return the index of each item in the list
        assert binary_search(names, 'Alex') == 0
        assert binary_search(names, 'James') == 1
        assert binary_search(names, 'Julia') == 2
        assert binary_search(names, 'Matthew') == 3
        assert binary_search(names, 'Nathan') == 4
        assert binary_search(names, 'Conrad') == 5
        assert binary_search(names, 'John') == 6

    def test_binary_search_with_items_in_list2(self):
        # binary search requires list values to be in sorted order
        names = ['Person0', 'Person1', 'Person2', 'Person3', 'Person4', 'Person5', 'Person6']
        # binary search should return the index of each item in the list
        assert linear_search(names, 'Person0') == 0
        assert linear_search(names, 'Person1') == 1
        assert linear_search(names, 'Person2') == 2
        assert linear_search(names, 'Person3') == 3
        assert linear_search(names, 'Person4') == 4
        assert linear_search(names, 'Person5') == 5
        assert linear_search(names, 'Person6') == 6

    def test_binary_search_with_items_not_in_list(self):
        # binary search requires list values to be in sorted order
        names = ['Alex', 'James', 'Julia', 'Matthew', 'Nathan', 'Conrad', 'John']
        # binary search should return None for any item not in the list
        assert binary_search(names, 'Jeffrey') is None
        assert binary_search(names, 'nobody') is None

    def test_binary_search_with_items_not_in_list2(self):
        # binary search requires list values to be in sorted order
        names = ['Person5', 'Person2', 'Person4', 'Person1', 'Person3', 'Person6', 'Person0']
        # binary search should return None for any item not in the list
        assert binary_search(names, 'Jack-in-Box') is None
        assert binary_search(names, 'nobody') is None

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

Be sure to check bisect the Python's standard binary search module.

This is a good implementation of Binary Search; I just have a few nitpicks:

How about a high and low overload?

Instead of declaring your lower and higher index like this:

lower_index = 0
upper_index = len(array) - 1

You could give them as argument overloads

def binary_search_iterative(array, item, low=0, high=None):
    if high is None:
        high = len(array)
    ...

Change your comments accordingly

Don't mix comments and docstrings, stick with docstrings if you want to explain arguments

"""iteratively return the index of the item with binary search or None if not found"""
# Sorted array that has to be the condition
# go through several iteration until we found this
# First, set the lower_index = 0
# upper_index as the length of array -1
lower_index = 0

That could be this:

"""Return the index of the item with binary search or None if not found

Keyword arguments:
array -- the list to search in
item -- the item to search for
high -- the highest index to search in between (default highest index of the array)
low -- the lowest index to search in between (default 0)
"""

This uses the PEP257 Docstring style, but you may use your own as long as it is clear.
Remove redundant comment

Commenting is good, but don't overdo it; your code is good enough to speak for itself. These comments feel redundant:

# calculate the middle index by sum the lower_index and upper_index and divide by 2
# check if the item is found
# Did not find


Limiting Q promise concurrency in JavaScript I wrote a helper to limit Q promise concurrency.
If I have a promise-returning function promiseSomething, writing

promiseSomething = PromiseScheduler.limitConcurrency(promiseSomething, 5);

ensures no more than 5 promises are pending at the same time.

I'm coming to JS from .NET world, and I welcome critique of this code.
It seems to work fine, but I wonder if it can be made shorter / more concise / plain better.

One of the thing worth remembering is that Q always schedules continuations on the next tick.

'use strict';

var q = require('q');

/**
 * Constructs a function that proxies to promiseFactory
 * limiting the count of promises that can run simultaneously.
 * @param promiseFactory function that returns promises.
 * @param limit how many promises are allowed to be running at the same time.
 * @returns function that returns a promise that eventually proxies to promiseFactory.
 */
function limitConcurrency(promiseFactory, limit) {
  var running = 0,
      semaphore;

  function scheduleNextJob() {
    if (running < limit) {
      running++;
      return q();
    }

    if (!semaphore) {
      semaphore = q.defer();
    }

    return semaphore.promise
      .finally(scheduleNextJob);
  }

  function processScheduledJobs() {
    running--;

    if (semaphore && running < limit) {
      semaphore.resolve();
      semaphore = null;
    }
  }

  return function () {
    var _this = this,
        args = arguments;

    function runJob() {
      return promiseFactory.apply(_this, args);
    }

    return scheduleNextJob()
      .then(runJob)
      .finally(processScheduledJobs);
  };
}

module.exports = {
  limitConcurrency: limitConcurrency
};

Your code looks good to me. Promises/A are a great pattern - however it can be hard to grasp in the beginning, I personally still got problems in understanding the Q library and Promises/A spec to wrap my head around

Your implementation looks familiar to control flow patterns presented here:
http://book.mixu.net/node/ch7.html

Here is another nice presentation on Promises and control flow issues:
http://trevorburnham.com/presentations/flow-control-with-promises/#/

and here is a list of other Promise/A spec implementations:

JQuery

http://api.jquery.com/promise The #1 choice in jQuery-land

Promises/A+

http://github.com/kriskowal/ The #1 choice in Node-land
http://github.com/tildeio/rsvp.js The most lightweight choice
http://github.com/cujojs/when The speediest choice
http://github.com/promises-aplus/promises-spec/blob/master/implementations.md


Check syntax (string and character comparison) I would like to know if my function that checks if a string syntax is correct. It's currently very messy, but seems to do the job.

//Example Valid String
[12345678]
{
something
whatever
}

Function:

private static bool CheckIfSyntaxIsValid(string input)
        {
            int mode = 1;
            char last = ' ';
            foreach (char c in input)
            {
                if (last == '{' && c == '}')
                {
                    last = ' ';
                    mode = 1;
                    continue;
                }
                if (last == '{' && c == '{')
                    return false;

                if (last == '[' && c == ']')
                {
                    last = ' ';
                    mode = 2;
                    continue;
                }
                if (last == '[' && !Char.IsDigit(c))
                    return false;

                if (c == '[')
                {
                    if (mode == 2)
                        return false;
                    last = c;
                    mode = 1;
                }
                if (c == '{')
                {
                    if (mode == 1)
                        return false;

                    last = c;
                    mode = 2;
                }

                if (c == ']' || c == '}')
                    return false;
            }

            if (last != ' ')
                return false;

            return true;

        }

The rule is basically:

[] must contain only numbers/digits
It must start with [] before {}, and they must come after each other

So, [] {} [] {} etc. Anything else is simply invalid.

Okay, not everything else is invalid. Text outside of those are ignored,
so the only thing that matters is that the bracket orders are correct, etc.

I suggest making the code more expressive by encapsulating the conditions as lambdas before the loop.

You should avoid magic numbers and for the mode create constants or an enum.

In the following example the {} stand for a block and [] for an attribute.

private static bool CheckIfSyntaxIsValid(string input)
{
    var mode = 1;
    var last = ' ';

    var isEndOfEmptyBlock = new Func<char, bool>(c => last == '{' && c == '}');
    var isBeginOfNestedBlock = new Func<char, bool>(c => last == '{' && c == '{');
    var isEndOfEmptyAttribute = new Func<char, bool>(c => last == '[' && c == ']');
    var isNotDigitInsideAttribute = new Func<char, bool>(c => last == '[' && !Char.IsDigit(c));
    var isAttributeBegin = new Func<char, bool>(c => c == '[');
    var isAttributeEnd = new Func<char, bool>(c => c == ']');
    var isBlockBegin = new Func<char, bool>(c => c == '{');
    var isBlockEnd = new Func<char, bool>(c => c == '}');

    foreach (char c in input)
    {
        if (isEndOfEmptyBlock(c))
        {
            last = ' ';
            mode = 1;
            continue;
        }

        if (isBeginOfNestedBlock(c))
        {
            return false;
        }

        if (isEndOfEmptyAttribute(c))
        {
            last = ' ';
            mode = 2;
            continue;
        }

        if (isNotDigitInsideAttribute(c))
        {
            return false;
        }

        if (isAttributeBegin(c))
        {
            if (mode == 2)
            {
                return false;
            }
            last = c;
            mode = 1;
        }

        if (isBlockBegin(c))
        {
            if (mode == 1)
            {
                return false;
            }

            last = c;
            mode = 2;
        }

        if (isAttributeEnd(c) || isBlockEnd(c))
        {
            return false;
        }
    }

    if (last != ' ')
    {
        return false;
    }

    return true;
}


Sum all numbers in a range I have implemented the "Sum All Numbers in a Range" challenge from Free Code Camp quoted below.
The code works like a charm, but I don't think it's idiomatic. The challenge hints imply I should be using Math.max(), Math.min() and Array.reduce(). In addition I used the spread operator from ES6 to simplify both Math calls.
I'm looking for a more idiomatic way to solve this. I think it can be done in a one-liner, something like:
return arr.sort((function(a, b) { return a - b; })).reduce(function(a, b) { /* */ });

But I'm pretty sure I'm heading the wrong way with the above.
The challenge:

We'll pass you an array of two numbers. Return the sum of those two numbers and all numbers between them.
The lowest number will not always come first.

The code:
function sumAll(arr) {
  var out = 0;
  for (var i = Math.min(...arr); i <= Math.max(...arr); i++) {
    out += i;
  }
  return out;
}

Test cases:
sumAll([1, 4])
sumAll([4, 1])
sumAll([5, 10])
sumAll([10, 5])

Expected output:
10
10
45
45

I don't know what you would use reduce() for to be honest. But you don't need to loop. We just have a simple arithmetic sequence with a straightforward formula:

$$\sum_{i=min}^{max} i = \frac{(max-min+1)(min+max)}2$$

We just need to pick out which one is larger:

function sumFrom(min, max) {
    return (max-min+1) * (min+max) / 2;
}

function sumAll(arr) {
    return sumFrom(Math.min(...arr), Math.max(...arr));
}

Or we could generalize to any arithmetic sequence:

function sumArithmetic(a1, n, d) {
    return n*(2*a1 + (n-1)*d) / 2;
}

function sumAll(arr) {
    var min = Math.min(...arr);
    return sumArithmetic(min, Math.max(...arr) - min + 1, 1);
}

I guess if you really want reduce() you'd do it this way (not using ES6 because I don't know how to get freecodecamp to use it):

function sumAll(arr) {
  var min = Math.min.apply(null, arr);
  var max = Math.max.apply(null, arr);

  return Array.apply(null, Array(max-min+1))   // get array of correct size
      .map(function(_, b) { return b+min; })   // change it to have correct values
      .reduce(function(a, b) { return a+b; }); // and sum it
}


Project Euler #35 in Common Lisp To start with Common Lisp I am doing Project Euler using this language. Usually I manage to solve problems but I am quite sure that my code is not as efficient as it could be in Common Lisp. That is why I need a review from experienced lispers.

This is my code for problem 35. Please offer any improvements.

(defun prime-p (n)
  (cond
    ((= n 1) nil)
    ((= n 2) t)
    ((evenp n) nil)
    (t (loop for i from 3 to (isqrt n) by 2
      never (zerop (mod n i))))))

(defun list->num (lst)
  (loop for i in lst
       for p = (- (length lst) 1) then (- p 1)
       sum (* i (expt 10 p))))

(defun num->list (n)
  (loop for c across (write-to-string n)
       collect (parse-integer (string c))))

(defun rotate (lst)
  (append (last lst) (butlast lst)))

(defun number-rotations (n)
  (let* ((digits (num->list n))
         (digits-count (length digits)))
    (loop repeat digits-count
       for rotated = digits then (rotate rotated)
       collect (list->num rotated))))

(defun problem-35 (limit)
  (let ((hash-primes (make-hash-table)))
    (loop for n from 1 to limit
       if (prime-p n)
       do (setf (gethash n hash-primes) t))
    (loop for p being the hash-keys in hash-primes
       if (loop for n in (number-rotations p)
         always (gethash n hash-primes))
       collect p)))

In list->num you can count down with something like for i downfrom n.

(defun num->list (n)
  (loop for c across (write-to-string n)
        collect (parse-integer (string c))))

In above function you can just collect (digit-char-p c). The function returns the digit value as a number.


Queue class with enqueue and dequeue I am just learning swift and tried to make a queue class with enqueue and dequeue.

The node holds the data for each element
The enqueue adds an element to the "queue"
The dequeue deletes the first element in the queue and returns the data
Head and Tail are references to node elements, which also tells if the queue is empty when they are nil (null)

Now I am wondering, have I done this correctly, both the theory and can the code be improved in Swift (used to code in Java)?

class Queue {

    class Node {
        // variables
        var data: String
        var prev: Node?

        init(data: String){
            self.data = data
        }
    }

    // variables
    var head: Node?
    var tail: Node?

    func enqueue(e: String){
        if head == nil && tail == nil {
            head = Node(data: e)
            head?.prev = nil
            tail = head
        } else {
            let temp = Node(data: e)
            tail?.prev = temp
            tail = temp

        }
    }

    func dequeue() -> String{
        let result = head?.data

        if head === tail {
            head = nil
            tail = nil
        } else {
            head = head?.prev
        }
        if (result == nil){
            return "empty"
        }
        return result!
    }

}

Testing the program:

var q: Queue
q = Queue()

q.enqueue("One")
q.enqueue("Two")
q.enqueue("Three")

print(q.dequeue())
print(q.dequeue())
print(q.dequeue())
print(q.dequeue())

Output:

One
Two
Three
empty

Your code is correct as far as I can see. However, there are some
things which can be improved or simplified.

Let's start with the class Node definition:

Strictly speaking, data and prev are properties.
prev is – as I understand it – a pointer to the node that was added after this one, so I would call it next instead.
The class should be private because it is not indented to be
used outside of Queue.
data should be a constant because it is not mutated after the
creation of a node (attributions go to @Feldur who noticed that in
a comment).
Minor note: There should be a space before curly braces.

Then we have:

private class Node {
    // properties
    let data: String
    var next: Node?

    init(data: String) {
        self.data = data
    }
}

Consequently, head and tail are private properties as well:

// properties
private var head: Node?
private var tail: Node?

The enqueue() method can be simplified. head and tail are both
nil or not, so there is no need to check both. Here it makes sense
to check the tail via optional binding, so that a new node can be
appended if the list was non-empty, or a single-node list created
otherwise:

func enqueue(e: String) {
    let node = Node(data: e)
    if let lastNode = tail {
        lastNode.next = node
    } else {
        head = node
    }
    tail = node
}

The dequeue() method returns the string "empty" if the queue is
empty. This is a "magic string" and makes it impossible to store the
string "empty" itself in the queue. Swift optionals were make
for this very purpose: The method should return an optional String?
which is nil if the queue is empty.

The method can be simplified in a similar manner as enqueue().
Now we check the initial node via optional binding. Note that there
is no forced unwrapping (!)  anymore, which should generally be
avoided:

func dequeue() -> String? {
    if let firstNode = head {
        head = firstNode.next
        if head == nil {
            tail = nil
        }
        return firstNode.data
    } else {
        return nil
    }
}

In your test code, the queue variable can be defined and initialized in
a single statement:

var q = Queue()

Since dequeue() now returns an optional, we can remove elements in
a loop like this:

q.enqueue("One")
q.enqueue("Two")
q.enqueue("Three")

while let s = q.dequeue() {
    print(s)
}

Finally: There is nothing in the implementation which is particular
to strings. You can make the class generic so that it can be used
with other data types as well. Only minor modifications are
necessary:

Replace Node by Node<T> (and move it outside of Queue, as
Swift currently does not allow nested types in generic classes).
Replace Queue by Queue<T>.
Replace String by T.

Then it looks like this:

private class Node<T> {
    let data: T
    var next: Node?

    init(data: T) {
        self.data = data
    }
}

class Queue<T> {

    private var head: Node<T>?
    private var tail: Node<T>?

    func enqueue(e: T) {
        let node = Node(data: e)
        if let lastNode = tail {
            lastNode.next = node
        } else {
            head = node
        }
        tail = node
    }

    func dequeue() -> T? {
        if let firstNode = head {
            head = firstNode.next
            if head == nil {
                tail = nil
            }
            return firstNode.data
        } else {
            return nil
        }
    }
}

Test code:

var q = Queue<Int>()

q.enqueue(1)
q.enqueue(2)
q.enqueue(3)

while let i = q.dequeue() {
    print(i)
}


Unit testing needs only optional methods of protocol in Swift I have a protocol which has quite more methods, it looks like:

  protocol MessageService {

    //configuration
    var fetchOfflineMessagesOnLogin: Bool {get set}//default true
    var maxCharsInMessage: UInt {get set}//default 400

    //connection
    var connectionState: ConnectionState { get }
    func login(hostName: String, port: UInt, username: String, password: String)
    func logoutFromService()

    //contacts
    func sendContactRequest(to peerUId: String)
    func acceptContactRequest(from peerUId: String)
    func removeContact(peerUId: String)
  }

I am unit testing the controller class which takes this protocol and operates on it according to various states.
There are some functions in this controller class that only interacts with only one method out of many protocol methods, and I while testing those methods, I wrote following unit test:

func testSendContactRequest() {
    class MessageServiceMock: MessageService {
        func sendContactRequest(to peerUId: String) {
            //tests
        }
    }
    //unit test setup
    let controller = Controller(service: MessageServiceMock())
    //...
}

func testAcceptContactRequest() {
    class MessageServiceMock: MessageService {
        func acceptContactRequest(from peerUId: String) {
            //tests
        }
    }
    //unit test setup
    let controller = Controller(service: MessageServiceMock())
    //...
}

But, apparently, compiler throws error

  MessageServiceMock does not conform to protocol

So I tried to make all the protocol methods optional by adding extension to it as (in test class file only):

fileprivate extension MessageService {

    var fetchOfflineMessagesOnLogin: Bool {
      get { return true }
      set {}
    }
    var maxCharsInMessage: UInt {
      get { return 400 }
      set {}
    }

    var connectionState: ConnectionState {
      return .connecting
    }
    func login(hostName: String, port: UInt, username: String, password: String) {}
    func logoutFromService() {}

    func sendContactRequest(to peerUId: String) {}
    func acceptContactRequest(from peerUId: String) {}
    func removeContact(peerUId: String) {}
}

Now error is gone and everything works fine.

I know other way to make protocol methods optional is using @objc optional keywords, but I can't use those as MessageService contains some Swift value types those are not convertible to ObjC objects.

So, is this solution is acceptable?
Are there any things that I can improve?

There are plenty of approaches that can fix your problem. But the main issue is how you build your classes. So IMHO you should focus on that.

Recommendation 1:  You should build your classes with composition of protocols. Loggable, Runnable, Flyable etc. If you build your class with combination of many protocols, the problem would be solved. And since Swift enables default implementations for protocols you can write a shorter class that is build by protocols. In this case you can test these protocols and their implementations separately.

Recommendation 2: There is a code smell in this design. You say no need to test for most of the methods. But you still put them in the same interface. And inject a class that conforms to this interface. You should change the class structure or injection interface. As in first recommendation I offer you to separate those methods to several interfaces and inject each other separately.

let controller = Controller(service: ABCProtocolMock(), anotherService: XYZProtocolMock())

Recommendation 3: Code Complete 2nd Ed. offers classes should have no more than 7 methods. If a class has more methods they should be separated through many considerations including SOLID.

Recommendation 4: I don't support this solution, because its another hacky code smell. But as a lazy developer you can think about if only one method will be testable/replacable why do you inject a whole interface. You can just inject a small interface that provides that method only.

Recommendation 5: Using utility functions can be another hacky solution. Since you want to test just one method of the class and if this method's implementation can be abstracted from internals of your class. You can forward this method's implementation to a utility function and write unit tests for this utility function.

Recommendation 6: Another hacky solution is building your classes as NSObject childs, and using Method Swizzling for testing purposes

As I mentioned above I don't know what is your exact situation, but Recommendation 1-2-3 are the correct solutions for a solid rock architecture. Others are just hacks that can save your day, but through your situation you can prefer to choose one of them.

For this decision I can offer you to check; if this is a one-time issue you can use a hacky solution. But if this a common situation that effects your applications architecture you should definitely use Recommendation 1-2-3.


Simple Password Utility I am somewhat new to ruby (but not to programming) and I want to develop a small web-app with Sinatra. I started creating a small password utility script that hashes and secures entered password. I am not sure it is finished. I would like to know if there are any ruby tricks to make my code better or more verbose, and what else needs to be done / changed to make passwords more secure. Any criticism is appreciated. here is my code:

require 'digest'

def random_salt
    hash = ''
    for i in 1..64
        rand_seed = rand(0..61)
        hash << 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890'[rand_seed]
    end
    return hash
end

# password format: salt + password + length_of_salt_and_hash
def hash_password(password)
    salt = random_salt
    hashed_pass = Digest::SHA512.hexdigest password.chomp
    length = salt.length + hashed_pass.length

    return salt + hashed_pass + length.to_s
end

# just a test
print 'Enter a password: '
word = gets
new_word = hash_password word
puts new_word

My critique is mostly on the crypto.

Never invent your own crypto. It's difficult to do correctly, and there are already good libraries for this sort of thing. See sinatra-authentication or bcrypt-ruby.
rand() is not a cryptographically secure random number generator, so it's a poor choice for use in crypto. Also see point #1.
Your password salting is incorrect. The salt must be mixed with the password before hashing, not after. Also see point #1.
Why append the length_of_salt_and_hash? That seems pointless.
Why append salt + hashed_pass? These are usually stored as separate fields in a database record.
Avoid magic numbers like 61 in your code. What does Roger Maris have to do with this? It's unclear and redundant that this 61 refers to the length of the string on the next line.
You ask for "better or more verbose." Those are often opposite goals. More verbose code means more code to read and more code to maintain. (That does not mean you should prefer cryptic one-liners.)


BoltDB Lookup and Add methods and advice on unit testing I am writing a Go package that handles communication with BoltDB as a part of some larger project. I have 2 methods for interacting with DB.
func Lookup(path string) (string, bool) {}
func RegisterUrl(path, url string) error {}

Lookup makes a request to database to retrieve corresponding url, whereas RegisterUrl writes path:url key-value pair to DB.
I would like to ask for a review of my implementation and some advice on how to implement unit tests.
Unit Testing
I've included the skeleton for unit tests that I would like to have for above 2 functions. However I have trouble figuring what is the best way to implement them.

I wouldn't like to test methods using the "prod" database/bucket, rather then that I would prefer to swap prod DB to test DB in the database_test.go file.
I thought about making dbName and bucketName public variables in database.go and then change them in _test file for test db, but this seems like I would be breaking the encapsulation.
I also considered passing test database/bucket to functions as functional options, but this seems like it will make code more complicated and I am not sure if this is the best approach.

I would be happy to hear your advice.
database.go
// Package database handles communication with BoltDB.
package database

import (
    "log"
    "time"

    "github.com/boltdb/bolt"
)

var dbOptions *bolt.Options = &bolt.Options{
    Timeout: 1 * time.Second,
}

const (
    dbName     = "urls.db"
    bucketName = "UrlFromPath"
)

func init() {
    // Make sure that the database exists.
    db, err := bolt.Open(dbName, 0600, dbOptions)
    if err != nil {
        log.Fatal(err)
    }
    defer db.Close()
    // Make sure that UrlFromPath bucket exists.
    err = db.Update(func(tx *bolt.Tx) error {
        _, err := tx.CreateBucketIfNotExists([]byte(bucketName))
        if err != nil {
            return err
        }
        return nil
    })
    if err != nil {
        log.Fatal(err)
    }
}

// Lookup checks if `path` key exists in database and returns related `url` value if found. If not
// found second return value `ok` is set to false.
func Lookup(path string) (url string, ok bool) { // TODO: return
    // Open connection to DB.
    db, err := bolt.Open(dbName, 0600, dbOptions)
    if err != nil {
        log.Fatalf("Cannot connect to DB: %s", err)
        return "", false
    }
    defer db.Close()

    // Retreive url from DB.
    var url_bytes []byte
    err = db.View(func(tx *bolt.Tx) error {
        b := tx.Bucket([]byte(bucketName))
        url_bytes = b.Get([]byte(path))
        return nil
    })
    if err != nil {
        return "", false
    }
    if url_bytes != nil {
        return string(url_bytes), true
    } else {
        return "", false
    }

}

// RegisterUrl saves provided key:value (path:url) pair to database.
func RegisterUrl(path, url string) error {
    // Open connection to DB.
    db, err := bolt.Open(dbName, 0600, dbOptions)
    if err != nil {
        return err
    }
    defer db.Close()

    // Save path and url.
    err = db.Update(func(tx *bolt.Tx) error {
        b := tx.Bucket([]byte(bucketName))
        err := b.Put([]byte(path), []byte(url))
        return err
    })
    if err != nil {
        return err
    }

    log.Printf("Database: %s, %s registered.", path, url)
    return nil
}

database_test.go
package database

import "testing"

func setup() {
    // Init test db.
    // Init test bucket.
}

func cleanup() {
    // Remove test db.
}

func TestLookup(t *testing.T) {
    // T1. Path exists in db. Correct url is retrieved.

    // T2. Path doesn't exist in db. false is returned.

}

func TestRegisterUrl(t *testing.T) {
    // T1. Add path:url to db. Correct path:url pair has been added to db.
}
```

I think the code could benefit a struct for the database instead of global state.
You can use a New method instead of init() where you pass in the params, which would differet from test and production.
I also demonstrated that you can keep the connection open and reuse it within the struct. If you prefer a lazy approach, you can omit this and create a the connection in each call if you want.
type Database struct {
   bucketName string
   boltDB *bolt.DB
}

func New(dbName, bucketName string, opts *bolt.Options) (db *Database, error) {
   db, err := bolt.Open(dbName, 0600, opts)
   return &Database{
      boltDb: db,
      bucketName: bucketName
   }, nil
}

func(db *Database) Lookup(path string) (url string, ok bool) { // lookup logic }
func(db *Database) RegisterUrl(path, url string) error { //register logic}
```


Counting Duplicates - Code War This is a problem from CodeWar.

  Write a function that will return the count of distinct
  case-insensitive alphabetic characters and numeric digits that occur
  more than once in the input string. The input string can be assumed to
  contain only alphabets (both uppercase and lowercase) and numeric
  digits.

  EX: "abba" -> 2, "aabBcde" -> 2.

Here is my solution in Java:

public class CountingDuplicates {
      public static int duplicateCount(String text) {
      String textLower = text.toLowerCase();
      char[] charArray = textLower.toCharArray();
      String uniqueRepeats = ""; //Will keep track of unique repeats.
      int count = 0;

      for(int i = 0; i < charArray.length - 1; i ++) {
      String restOfString = textLower.substring(i + 1);

      //Convert single char to String to be used in method.
      String character = Character.toString(charArray[i]);

      //If not in uniqueRepeats, check if it is a repeat.
      if(!uniqueRepeats.contains(character)) {
          if(restOfString.indexOf(character) != -1) {
     //If it is a repeat, increase count and concat it to uniqueRepeats
              count++;
              uniqueRepeats += character;
     }
    }
   }
  return count;
  }
}

Some improvements that I can think of is using a HashMap to store the repeats so that I do not have to create a new string object for every repeat
If you guys have any suggestions on code readability, code structure, or better implementation for this problem, please leave me a comment. Thank you!
EDIT - Here is the link to the problem: https://www.codewars.com/kata/counting-duplicates/train/java

Let's see what is the Big O of your code.

In new versions of Java substring creates a new String and performs in \$O(n)\$ (In older versions this is \$O(1)\$)
uniqueRepeats.contains(character) results in \$O(m)\$ where \$m\$ is count of unique characters.
restOfString.indexOf(character) != -1 - this is again \$O(n)\$
uniqueRepeats += character Depending on the JVM this might end up creating set of StringBuilder objects or set of String objects that are discarded. So this is probably \$O(m^2)\$
You are doing this for \$n\$ characters.

So time complexity is - \$O(n *(n + m + m^2))\$ in some situations \$m\$ can be as large as \$n\$. So if we simplyfy things we get \$O(n^3)\$.

Ideas:

This is where your HashMap idea would've been better. We can simply store Char and an Integer count. Then iterate over your map to count all elements that has more than \$1\$ element. LinkedHashMap is very useful for a situation like this.
Why is LinkedHashMap better? Because it has an internal linked list that allows faster iteration.
We can also use an array if this is ASCII only. (But it is easier with a Map).
This will result in armotized \$O(n)\$. Which is lot better.
Either way all these are theoretical and it is always better to profile things.

Code

//Will keep track of unique repeats.
//Convert single char to String to be used in method.
//If not in uniqueRepeats, check if it is a repeat.
//If it is a repeat, increase count and concat it to uniqueRepeats

You have mentioned what you are doing in comments. We can also understand that from code itself. So it is better to include why comments.
I recommend that you use an IDE to indent code. Code is clearly not indented accurately.

if(!uniqueRepeats.contains(character)) {
    if(restOfString.indexOf(character) != -1) {

Be consistent. We can use contains to both above statements.

References

https://stackoverflow.com/a/1532483/1355145
http://java-performance.info/changes-to-string-java-1-7-0_06/
https://stackoverflow.com/questions/26311776/why-iteration-through-buckets-in-linkedhashmap-is-faster-than-hashmap


Minecraft server querier that generates an image of online players This little chunk of code is run every 10 seconds on my desktop. It generates a PNG with up to 6 player faces based on the list of online players.

Is there any way for this code to execute faster? Any comments?

Each player has a PNG of their avatar's face with the title: "username_face.png" These are pasted into the main image when that player is online.

I wrote this script to compile their faces into a single image for display in a program called conky, allowing me to display the online players as part of my desktop.

from mcstatus import MinecraftServer
import os
from PIL import Image

#Query the server and get player names.
server = MinecraftServer("123.123.123.123", 25565)
query = server.query()
nameList = query.players.names

#Sort the list of player names.
nameList.sort()

#Open our temporary file for storing player names.
tempFile = open('server1_previous_players.txt', 'r')
oldNameList = tempFile.read().splitlines()
tempFile.close()

def createImage(players):
    #create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    if len(players) > 0:
        for x in range(0, 6):
            try:
                face = Image.open(str(nameList[x]).lower() + "_face.png", 'r')
                baseImage.paste(face, slotList[x])
            except:
                break

    baseImage.save("server1_online.png")

#If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")
    pass

#Else, if no-one is on, but the text file says people are on, empty the text file.
elif len(nameList) == 0 and len(oldNameList) > 0:
    print("everyone left :'(")
    tempFile = open('server1_previous_players.txt', 'w')
    createImage(nameList)

#Else, if at least 1 person is on, generate a new image accordingly.
elif len(nameList) > 0:
    tempFile = open('server1_previous_players.txt', 'w')
    print('\n'.join(nameList), file=tempFile)
    createImage(nameList)

else:
    pass

Maybe it will not increase the performance, but I have some general tips:
Use while loops instead of for-loops with break. It will make your code more readable.
I have rewritten the createImage function for you:

def createImage(players):
    # create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    x = 0
    while x < len(players) and x < 6:
        face = Image.open(str(players[x]).lower() + "_face.png", 'r')
        baseImage.paste(face, slotList[x])
        x += 1
    baseImage.save("server1_online.png")

If you prefer for over while, you can use:

def createImage(players):
    # create a blank transparent image with the dimensions: 212x147px.
    baseImage = Image.new("RGBA", (212, 147))
    slotList = [(17, 17), (82, 17), (147, 17), (17, 82), (82, 82), (147, 82)]

    for slot, player in zip(slotList, players):
        face = Image.open(str(player).lower() + "_face.png", 'r')
        baseImage.paste(face, slot)
    baseImage.save("server1_online.png")

Read more about zip here

You need to close the files you open:

# If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")

# Else, if no-one is on, but the text file says people are on, empty the text file.
elif len(nameList) == 0 and len(oldNameList) > 0:
    print("everyone left :'(")
    tempFile = open('server1_previous_players.txt', 'w')
    tempFile.write("")
    tempFile.close()
    createImage(nameList)

# Else, if at least 1 person is on, generate a new image accordingly.
elif len(nameList) > 0:
    tempFile = open('server1_previous_players.txt', 'w')
    # write should be used instead of print
    tempFile.write('\n'.join(nameList))
    tempFile.close()
    createImage(nameList)

An if-statement can be used without an else, so if there is only pass in there you don't need it.

If you want to create an empty / transparent image when nobody is on, you can combine both elif-statements:

# If nothing changed, pass!
if nameList == oldNameList:
    print("same thing!")

# else generate image and write new player list to file
else:
    if len(nameList) == 0 and len(oldNameList) > 0:
        print("everybody left")
    tempFile = open('server1_previous_players.txt', 'w')
    tempFile.write('\n'.join(nameList))  # writes "" for an empty player list
    tempFile.close()
    createImage(nameList)


Transcribe .docx files via python-docx to modify font and font size. Need to reconstruct paragraphs in target files The intention is to transcribe .docx files to have modified font and font sizes while keeping the run attributes such as bold, underline, italic etc. I'll then add some headers and graphics to the newly created target.docx files

How to reconstruct paragraphs from runs? Each one, currently, gets it's own separate line!

from docx import Document
from docx.shared import Pt

def main(filename):
    try:
        src_doc = Document(filename)
        trg_doc = Document()

        style = trg_doc.styles['Normal']
        font = style.font
        font.name = 'Times'
        font.size = Pt(11)

        for p_cnt in range(len(src_doc.paragraphs)):
            for r_cnt in range(len(src_doc.paragraphs[p_cnt].runs)):
                curr_run = src_doc.paragraphs[p_cnt].runs[r_cnt]
                print('Run: ', curr_run.text)
                paragraph = trg_doc.add_paragraph()

                if curr_run.bold:
                    paragraph.add_run(curr_run.text).bold = True
                elif curr_run.italic:
                    paragraph.add_run(curr_run.text).italic = True
                elif curr_run.underline:
                    paragraph.add_run(curr_run.text).underline = True
                else:
                    paragraph.add_run(curr_run.text)

        trg_doc.save('../Output/the_target.docx')

    except IOError:
        print('There was an error opening the file')

if __name__ == '__main__':
    main("../Input/Current_File.docx")

nput:

1.0 PURPOSE The purpose of this procedure is to ensure all feedback is logged, documented and any resulting complaints are received, evaluated, and reviewed in accordance with 21 CFR Part 820 and ISO 13485

Output:

PURPOSE The purpose of this procedure is to ensure

all feedback is logged,

documented and any resulting complaints are received,

evaluated, and reviewed

in accordance with 21 CFR P art 820

and ISO 13485 .

This modification resolves this issue.
Thanks to Scanny at Stack Overflow

for src_paragraph in src_doc.paragraphs:
            tgt_paragraph = tgt_doc.add_paragraph()
            for src_run in src_paragraph.runs:
                print('Run: ', src_run.text)
                tgt_run = tgt_paragraph.add_run(src_run.text)
                if src_run.bold:
                    tgt_run.bold = True
                if src_run.italic:
                    tgt_run.italic = True
                if src_run.underline:
                    tgt_run.underline = True


Cleaner Way to Structure Asynchronous Code The purpose of the code is to allow the user to select directories that will be crawled recursively in order to find particular files or file types to analyze en masse. The thing to bear in mind is that if C:\ is already selected, then any child path of C:\ that the user tries to select will be discarded. The entire process of the script can be broken down into the following steps:

The user selects a directory or directories via a dialog box.
The script then compares the selection to whatever directories might already be stored in an array in the configuration file, which is a JSON object.
If the selection is invalid, the selection is discarded. / If the selection is valid, the configuration file is modified accordingly.

Obviously promises are ideal, so I'm using Node's fs.promises. However, the tricky bit is that, although they're asynchronous, parts of the code must behave synchronously. For example, it would be a waste of resources to retrieve the array from the configuration file if the dialog box is cancelled by the user.

I'm completely new to promises and everything else that entails, so I'm wondering whether or not there's an even better way to structure the asynchronous code or the code in general? The code works fine the way it is, but can its structure/readability and or efficiency be improved?

async function userSelectsArchives()
{
  try {
    const r1 = await dialog.showOpenDialog(remote.getCurrentWindow(), {
      title: 'Select Archives to Analyze',
      buttonLabel: 'Select Archives',
      defaultPath: 'c:\\',
      properties: [ 'openDirectory', 'multiSelections' ]
    })

    if (r1.canceled) throw new Error('Dialog cancelled.')

    const
      r2 = await fsp.readFile('./config.json', 'utf8'),
      config = JSON.parse(r2),
      archivesOG = config.archives

    for (const dir of r1.filePaths){
      // Disregard user input if parent path or exact match already exists.
      if (
        config.archives.reduce((a, v) => {
          return a || dir.indexOf(v.slice(-1) === '\\' ? v : (v + '\\')) === 0
        }, false)
        || config.archives.includes(dir)
      ) continue
      // Remove child paths of user input in config.
      config.archives = config.archives.filter(
        v => v.indexOf(dir.slice(-1) === '\\' ? dir : (dir + '\\')) !== 0
      )
      // Add user input to archive.
      config.archives.push(dir)
    }

    // Verify that changes have occurred or throw error.
    if (archivesOG === config.archives) throw new Error('Invalid selection.')

    await fsp.writeFile('./config.json', JSON.stringify(config, null, 2), 'utf8')

    displayDirs(config.archives)
  }
  catch (error) {
    console.error(error)
  }
}

Tips for JS code optimization:

Variable names

Give your variables meaningful names:

const r1 = await dialog.showOpenDialog(...) can be renamed to dirs or dirList (representing selected directories)
const r2 = await fsp.readFile('./config.json', 'utf8'), config = JSON.parse(r2), where r2 is supposed to be a config file path. Let's rename it to say const config_file = await fsp.readFile('./config.json', 'utf8'), config = JSON.parse(config_file)

Extract function:

Repeated expression <string>.slice(-1) === '\\' ? <string> : (<string> + '\\') can be extracted into a reusable routine (named or anonymous function):

var asDirPath = function(path) {
    return path.slice(-1) === '\\' ? path : (path + '\\');
};

Substitute algorithm:

The whole condition (combination of array.reduce and array.includes) can be easily substituted with a more efficient Array.some() approach (to check for any match between current directory path dir and config.archives file/dir paths):

for (const dir of dirList.filePaths){
    // Disregard user input if parent path or exact match already exists.
    if (config.archives.some(path => {
            return dir.indexOf(asDirPath(path)) === 0 || path === dir
    })) continue


Generate simple sinusoidal sounds and plot signal/fft I made a small python script to create an audio sound used as stimuli. It's one of my first uses of super(), and I do not know if I did it right. I'm looking for any improvement in both optimization and style you could suggest.
import pyaudio
import numpy as np

class SoundStimuli:
    def __init__(self, fs=44100, duration=1.0):
        self.fs = fs
        self.duration = duration
        self.t = np.linspace(0, duration, int(duration*fs), endpoint=False)
        self.signal = None

    def play(self):
        if self.signal is None:
            print ('WARNING: No audio signal to play.')
            return

        p = pyaudio.PyAudio()

        # for paFloat32 sample values must be in range [-1.0, 1.0]
        stream = p.open(format=pyaudio.paFloat32,
                        channels=1,
                        rate=self.fs,
                        output=True)

        # play. May repeat with different volume values (if done interactively)
        stream.write(self.signal)

        stream.stop_stream()
        stream.close()
        p.terminate()

    def convert2float32(self):
        self.signal = self.signal.astype(np.float32)

    def plot_signal(self, ax, **plt_kwargs):
        if self.signal is None:
            print ('WARNING: No audio signal to plot.')
            return

        ax.plot(self.t, self.signal, **plt_kwargs)

    def plot_signal_fft(self, ax, **plt_kwargs):
        fft_freq = np.fft.rfftfreq(self.t.shape[0], 1.0/self.fs)
        fft = np.abs(np.fft.rfft(self.signal))
        ax.plot(fft_freq, fft, **plt_kwargs)

class ASSR(SoundStimuli):
    def __init__(self, fc, fm, fs=44100, duration=1.0):
        super().__init__(fs, duration)
        self.fc = fc
        self.fm = fm

    def classical_AM(self):
        self.assr_amplitude = (1-np.sin(2*np.pi*self.fm*self.t))
        self.signal = self.assr_amplitude * np.sin(2*np.pi*self.fc*self.t).astype(np.float32)

    def DSBSC_AM(self):
        self.assr_amplitude = np.sin(2*np.pi*self.fm*self.t)
        self.signal = self.assr_amplitude * np.sin(2*np.pi*self.fc*self.t).astype(np.float32)

    def plot_signal(self, ax, amplitude_linestyle='--', amplitude_color='crimson', **plt_kwargs):
        super().plot_signal(ax, **plt_kwargs)
        ax.plot(self.t, self.assr_amplitude, linestyle=amplitude_linestyle, color=amplitude_color)

The class ASSR is imported in a second script where the stimuli is created:
sound = ASSR(fs=44100, duration=1.0, fc=1000, fm=40)
sound.classical_AM()

However, I do not play it using the .play() method because it looks like pyaudio doesn't accept the PyAudio and stream to be open/close outside the scope of .write(). And since I do not want to open a new stream every time I play a sound, I ended up using .play() to test the sound; and then create in the main program where ASSR is imported a new audio stream with different .write(sound.signal) calls.
# Initialize audio stream
p = pyaudio.PyAudio()
stream = p.open(format=pyaudio.paFloat32,
                channels=1,
                rate=sound.fs,
                output=True)

# Do stuff
stream.write(sound.signal)
# Do stuff
stream.write(sound.signal)

stream.stop_stream()
stream.close()
p.terminate()

A lot of things here are already good, like:

fs, duration and t being members on SoundStimuli
The choice of which things to initialize in the first __init__ (mostly)

Things that could be improved:

SoundStimuli is arguably SoundStimulus
self.signal should not be an optional member variable; the base class should be made effectively abstract and the signal should be returned from a getter or property whose base implementation raises NotImplementedError.
In play, the absence of a signal should be fatal, so you should naively access self.signal, and let any exceptions fall through to the caller
After p.open, wrap the rest in a try, and put the close() in a finally
convert2float is not a great idea - it replaces a member with a different version of the same member. That means that before this is called, the instance is by definition in an invalid state, which should be avoided.


Create all lags of given columns I'm creating a pandas.DataFrame out of lags from an existing pandas.DataFrame, using DataFrame.shift and pandas.concat. There are a two aspects I don't like about this code:

Using np.ones_like to create constants
Building up a list of lag tables and using pandas.concat to combine them

"""
>>> df = pd.DataFrame({'GDP': [100, 110, 120, 130, 140, 150],
...                    'INV': [0, 10, 20, 30, 40, 50]})

>>> lagged_dataframe(df, lags=3)
   GDP  INV  GDP_Int  INV_Int  GDP_Lag01  INV_Lag01  GDP_Lag02  INV_Lag02  \\
0  100    0        1        1        NaN        NaN        NaN        NaN
1  110   10        1        1      100.0        0.0        NaN        NaN
2  120   20        1        1      110.0       10.0      100.0        0.0
3  130   30        1        1      120.0       20.0      110.0       10.0
4  140   40        1        1      130.0       30.0      120.0       20.0
5  150   50        1        1      140.0       40.0      130.0       30.0
<BLANKLINE>
   GDP_Lag03  INV_Lag03
0        NaN        NaN
1        NaN        NaN
2        NaN        NaN
3      100.0        0.0
4      110.0       10.0
5      120.0       20.0

"""

import numpy as np
import pandas as pd

def constants(df):
    new_columns = ["{}_Int".format(variable) for variable in df.columns]
    return pd.DataFrame(np.ones_like(df.loc[:,:]), index=df.index, columns=new_columns)

def lag(df, n):
    new_columns = ["{}_Lag{:02d}".format(variable, n) for variable in df.columns]
    new_df = df.shift(n)
    new_df.columns = new_columns
    return new_df

def lagged_dataframe(df, lags=1):
    data_frames = [df]
    data_frames.append(constants(df))
    data_frames.extend([lag(df, i) for i in range(1, lags + 1)])
    return pd.concat(data_frames, axis=1)

if __name__ == "__main__":
    import doctest
    doctest.testmod(verbose=True, optionflags=doctest.NORMALIZE_WHITESPACE)

Well you've written you don't like it, but why?  Is this ever used with relevantly large input?  If so, you might have to consider allocating the right size of data frame from the start and assign the values afterwards.

If not, I can only see some small things that could be a little bit more succinct, and perhaps np.ones_like(df) instead of with df.loc[:,:] ... but otherwise this seems fine?

Like the following removes a bit of duplicated code, but is not inherently better:

def suffixed_columns(df, suffix):
    return ["{}{}".format(column, suffix) for column in df.columns]

def constants(df):
    return pd.DataFrame(np.ones_like(df), index=df.index, columns=suffixed_columns(df, "_Int"))

def lag(df, n):
    new_df = df.shift(n)
    new_df.columns = suffixed_columns(df, "_Lag{:02d}".format(n))
    return new_df

def lagged_dataframe(df, lags=1):
    return pd.concat([df, constants(df)] + [lag(df, i) for i in range(1, lags + 1)], axis=1)


AngularJS Login Check I have states that have a requiredlogin object set as true, then I have Angular listen on stateChangeStart, but things end up pretty ugly.

Here is a state that requires a login. I think this actually pretty good and simple.

.state('dashboard',
  url: '/dashboard',
  templateUrl: 'partials/dashboard/main.jade',
  data:
    requireLogin: true
)

This is where it gets ugly. I'll eventually just change this into a loginCheck() function but I hate the logic so much; too many if statements. Also, this barely works, so I could use the improvements.

$rootScope.$on('$stateChangeStart', (event, toState) ->
    # Checks if a page or a page's parent page requires login
    if _.has(toState.data, 'requireLogin')
      if _.has($window.sessionStorage, 'userInfo')
    # If it does check for stored user information
        userInfo = JSON.parse($window.sessionStorage["userInfo"]) or null
      # Check if user info has an ID property if it does try to authorize it
        if _.has(userInfo, '_id')
          AuthService.isAuthentic(userInfo._id).then (d) ->
            if d.data == "true"
              $rootScope.userInfo = userInfo
              $rootScope.userAuth = true
            else
          $rootScope.userInfo = null
          $rootScope.userAuth = false
          $state.go('login')
    else
      event.preventDefault()
      $state.go('login')
  else
    $window.sessionStorage["userInfo"] = null
    $state.go('login')
)

I think I would make a state object and a user object to represent this delema.

We will make a user class to encapsulate working with the user api

var User = function(sessionStorage, authService){
    var self = this,
       userInfo = buildUserInfo(sessionStorage["userInfo"]);

    self.isAuthentic = function(cb){
       if(!hasActiveLogin) return;

          //THIS is where the "doesn't work" part of your code is
          //this service looks to be an async service. and you are using it in a synced way
       authService.isAuthentic(userInfo._id).then(function(d){
           userInfo.isAuthenticated = d.data == "true";
           cb(d);
       });
    }

    self.hasActiveLogin = function(){
        var valid = userInfo != null;
            valid = valid && userInfo._id;
        return valid;
    }

    self.isAuthenticated = function(){
       return self.hasActiveLogin() && userInfo.isAuthenticated;
    }

    self.getUserInfo = function(){
       userInfo;
    }

    self.clearUserInfo = function(){
        sessionStorage["userInfo"] = null;
    }

    function buildUserInfo(userInfoData){
       return userInfoData ? JSON.parse(userInfoData) : null;
    }

};

now the root scope can simply handle that user

This implementation is not perfect... but it does show how to use javascript to encapsulate these kinds of issues into manageable objects.

$rootScope.$on('$stateChangeStart', function(event, toState){

     if(!_.has(toState.data, 'requireLogin')) return;

     var user = new User($window.sessionStorage,

    if(!user.hasActiveLogin()){
       event.preventDefault();
       $state.go('login');
    }

    //im not exactly sure how the service works so this peice of encapsulation
    // may be over kill
    if(user.isAuthenticThen(function(d)){
       if(!user.isAuthenticated()){
          user.clearUserInfo();
          event.preventDefault();
          $state.go('login');
       }

       $rootScope.userInfo = user.getUserInfo();
       $rootScope.userAuth = user.getUserInfo().isAuthenticated || false;
    }
});


.htaccess using public folder and SEO friendly URL .htaccess is not my strong suit. Is there any there can optimize this code?

RewriteEngine on
RewriteCond %{REQUEST_URI} !^/(public)
RewriteRule (.*) /public/$1

Options +FollowSymLinks -MultiViews
# Turn mod_rewrite on
RewriteEngine On
RewriteBase /

RewriteCond %{REQUEST_FILENAME}.php -f
RewriteCond %{REQUEST_URI} !/$
RewriteRule (.*) $1\.php [L]

ErrorDocument 500 "Sorry, our script crashed. Oh dear"
ErrorDocument 500 /cgi-bin/crash-recover
ErrorDocument 500 http://error.example.com/server_error.html
ErrorDocument 404 /errors/not_found.html
ErrorDocument 401 /subscription/how_to_subscribe.html

don't know if it necessary but here are an example of the folder system

app
public

index.php
contact.php
forum

last_news.php
users

user.php

ask.php

.htaccess

Redundant RewriteEngine declarations
Unused match group in (public)
Escaping (using \) is not needed in the new URLs
A personal touch, but I prefer to keep the core directives at the top, and put module specific directives inside <IfModule> body, so that I won't receive errors if the modules are disabled.
Multiple declarations for ErrorDocument 500

Options +FollowSymLinks -MultiViews

ErrorDocument 401 /subscription/how_to_subscribe.html# Turn mod_rewrite on
ErrorDocument 404 /errors/not_found.html
ErrorDocument 500 http://error.example.com/server_error.html

<IfModule mod_rewrite.c>
    RewriteEngine on
    RewriteBase /

    RewriteCond %{REQUEST_URI} !^/(public)
    RewriteRule ^ /%1%{REQUEST_URI}

    RewriteCond %{REQUEST_FILENAME}.php -f
    RewriteCond %{REQUEST_URI} !/$
    RewriteRule ^(.*)$ $1.php [L]
</IfModule>


VBA - XMLHTTP web scraping I navigate with IE, do various things, then select all results option from a list and fire on click event. Once all results have been listed, I loop through their URLs, using the following code to gather data inside these URLs.

What can I do to improve speed of data scraping (apart from VB Script
multithreading)?

With

IE navigation, visible = false and no images I had 1000 links extracted in 3min 45sec
msxml2.XMLHTTP in 3min 30sec
msxml2.serverXMLHTTP 1min 40sec - .setRequestHeader "Content-Type", "text/xml" and .setRequestHeader "Accept-Encoding", "compress, gzip" didn't affect performance

Is it better to declare and create objects within this Sub (meaning inside the loop for 42,000 links) or outside and make them public variables?

Public Sub PrintCompanyData()
Dim oDom As Object: Set oDom = CreateObject("htmlFile")
Dim htmlelePopUp As IHTMLElement
Dim unformattedData As String

With CreateObject("msxml2.serverXMLHTTP") 'CreateObject("msxml2.xmlhttp")
    .Open "GET", Link, False
    '.setRequestHeader "Content-Type", "text/xml"
    '.setRequestHeader "Accept-Encoding", "compress, gzip"
    .send

    If .ReadyState = 4 And .Status = 200 Then

        oDom.body.innerHTML = .responseText

        With WS
            For Each htmlelePopUp In oDom.getElementsByTagName("tbody")
                unformattedData = htmlelePopUp.Children(htmlelePopUp.Children.Length - 1).innerText
                .Range("a" & ItemCount + 1).Value2 = Replace(unformattedData, Chr(10), vbNullString)
            Next htmlelePopUp
        End With

    End If

End With

Set oDom = Nothing

End Sub

It's likely not a huge improvement, because the vast majority of this program's time will be spent going over the network, but there is a micro-optimization you can make here.

Instead of latebinding like this

With CreateObject("msxml2.serverXMLHTTP")

You can early bind your objects, eliminating the time COM takes to do a dynamic lookup. However, the time this saves will be minuscule compared to the total runtime.

I'm not bashing on VBA, I <3 VBA, but it's just not the right tool for the job if you need it to run faster than this. What you really need is a language that's capable of multi-threading and asynchronous programming to make the most of the CPU time while you're waiting for those Http requests to return. Sounds like it's time to learn some C# or F# to me. Even Powershell may be a good option.


Pokémon style battle game I haven't been learning Python for too long and I was just wondering how this Pokémon style battle looks? It's based off of this:
Turn Based Pokémon Style Game. It's my first proper time using classes so I'd love advice or critique on the usage. Also, where I make the CPU more likely to use heal when under 35 health, there must surely be a better way to do that.

# Simple battle simulator in the style of Pokemon.
# author: Prendy

import random

moves = {"tackle": range(18, 26),
         "thundershock": range(10, 36),
         "heal": range(10, 20)}

class Character:
    """ Define our general Character which we base our player and enemy off """
    def __init__(self, health):
        self.health = health

    def attack(self, other):
        raise NotImplementedError

class Player(Character):
    """ The player, they start with 100 health and have the choice of three moves """
    def __init__(self, health=100):
        super().__init__(health)

    def attack(self, other):
        while True:
            choice = str.lower(input("\nWhat move would you like to make? (Tackle, Thundershock, or Heal)"))

            if choice == "heal":
                self.health += int(random.choice(moves[choice]))
                print("\nYour health is now {0.health}.".format(self))
                break
            if choice == "tackle" or choice == "thundershock":
                damage = int(random.choice(moves[choice]))
                other.health -= damage
                print("\nYou attack with {0}, dealing {1} damage.".format(choice, damage))
                break
            else:
                print("Not a valid move, try again!")

class Enemy(Character):
    """ The enemy, also starts with 100 health and chooses moves at random """
    def __init__(self, health=100):
        super().__init__(health)

    def attack(self, other):
        if self.health <= 35:
            # increasing probability of heal when under 35 health, bit janky
            moves_1 = ["tackle", "thundershock", "heal", "heal", "heal", "heal", "heal"]
            cpu_choice = random.choice(moves_1)
        else:
            cpu_choice = random.choice(list(moves))
        if cpu_choice == "tackle" or cpu_choice == "thundershock":
            damage = int(random.choice(moves[cpu_choice]))
            other.health -= damage
            print("\nThe CPU attacks with {0}, dealing {1} damage.".format(cpu_choice, damage))
        if cpu_choice == "heal":
            self.health += int(random.choice(moves[cpu_choice]))
            print("\nThe CPU uses heal and its health is now {0.health}.".format(self))

def battle(player, enemy):
    print("An enemy CPU enters...")
    while player.health > 0 and enemy.health > 0:
        player.attack(enemy)
        if enemy.health <= 0:
            break
        print("\nThe health of the CPU is now {0.health}.".format(enemy))
        enemy.attack(player)
        if player.health <= 0:
            break
        print("\nYour health is now {0.health}.".format(player))
    # outcome
    if player.health > 0:
        print("You defeated the CPU!")
    if enemy.health > 0:
        print("You were defeated by the CPU!")

if __name__ == '__main__':
    battle(Player(), Enemy())

Magic numbers

Right off the bat I see some magic numbers

moves = {"tackle": range(18, 26),
         "thundershock": range(10, 36),
         "heal": range(10, 20)}

What does that mean?  Being familiar with Pokemon I'd assume damage, or something, but that won't necessarily be apparent to the user.

ABCs

You have your base class, Character.  It would benefit from being an abstract base class

For example,

import abc

class Character(metaclass=abc.ABCMeta):

    def __init__(self, starting_health):
        self.current_health = starting_health

    @abc.abstractmethod
    def attack(self, other):
        raise NotImplementedError

I've done a few things here.  For one, I've used more clear variable names.  The input parameter health is actually the starting_health of the character, while self.health is actually referring to the current_health of the character.  Better variable names make code easier to read.

The meat of this is the abc stuff.  By giving the Character class a metaclass of abc.ABCMeta (don't worry about what a metaclass is) we're saying that it cannot be instantiated directly if it has any abstract methods or properties.  With this definition, if you then tried to do this

char = Character(100)

you would get the following error:

TypeError: Can't instantiate abstract class Character with abstract methods attack

This carries into sub-types as well.  It is a way of guaranteeing that all classes you instantiate that should override a method do override it.

Moves

Your moves should probably be classes.  This will make it much, much easier to extend this, and simplify some other behaviors.  I'd look at something like this

from enum import Enum

DamageTypes = Enum('DamageTypes', 'DAMAGING HEALING STATUS')
Types = Enum('Types', 'ELECTRIC NORMAL')

class Move(metaclass=abc.ABCMeta):

    @abc.abstractproperty
    def damage_type(self):
        return NotImplemented

    @abc.abstractproperty
    def move_type(self):
        return NotImplemented

    @abc.abstractmethod
    def health_change(self, modifiers=None):
        return NotImplemented

class Thundershock(Move):
    _max = 36
    _min = 10

    @property
    def damage_type(self):
        return MoveTypes.DAMAGING

    @property
    def move_type(self):
        return Types.ELECTRIC

    def health_change(self, modifiers=None):
        if modifiers is None:
            return random.randint(self._min, self._max)
        else:
            # Do something here if they have some ability that reduces electric damage, or whatever

I did a few things here.  The first was the Enums.  Enums let you group related constants together.  For example, now I can do something like

if move.move_type is Types.ELECTRIC:
    # Has lightningrod ability, immune to electric moves
    return 0

without having a magic number.  I then gave the class some properties (i.e. attributes that I can get/set without using parentheses) as well as some methods (attributes that are functions).  This gives you a slightly more well organized codebase, and is easy to extend.  Just add a new subclass.

Now in your Player class you can do something like

def attack(self, opponent)
    while True:
        try:
            move = moves[str.lower(input("stuff"))]
        except KeyError:
            print("Not a valid move, try again!")
        else:
            if move.move_type is MoveTypes.HEAL:
                self.health += move.health_change(None)
            elif move.move_type is MoveTypes.DAMAGIN:
                opponent.health -= move.health_change(None)
            else:
                opponent.status = move.status_effect(None)
            break

which to me is much cleaner, and also easier to extend if you add move types.  It doesn't rely on the specific strings of the names (can you imagine typing all ~600 moves that exist in Pokemon?) just on what sort of effect they have.

Weighted randomness

I won't repeat everything here, but Ned Batchelder has a good suggestion for how to handle weighted randomness here

Attack order

You should randomise this.  Good guys don't always go first :)

String formatting

Instead of doing

print("The health of the CPu is now {0.health}.".format(enemy)

just do

print("The health of the CPU is now {}.".format(enemy.health)

Improving your base class

As per @200_success's answer below, you should expand the functionality of your Character base class.  For example, attacking, healing, and taking damage are all shared between characters (mostly).  What if you did this?

class Character(metaclass=abc.ABCMeta):

    def __init__(self, starting_health):
        self.current_health = starting_health

    def attack(self, other, modifiers):
        move = self.get_move()
        if move.move_type is MoveTypes.DAMAGING:
            other.damage(move.health_change(modifiers))
        elif move.move_type is MoveTypes.HEAL:
            self.heal(move.health_change(modifiers))
        elif move.move_type is MoveTypes.STATUS:
            other.status(move.status_effect(modifiers))
        else:
            raise NotImplementedError

    @abc.abstractmethod
    def get_move(self):
        raise NotImplementedError

I've consolidated the act of attacking and abstracted away the only part that changes - how they pick their move.  Now you can do

 class Player(Character):

     def get_move(self):
         while True:
             move = moves[str.lower(input(""))]
         except KeyError:
             print("No such move")
         else:
             return move

 class Enemy(Character):

     def get_move(self):
         # do some pseudo random stuff
         return move

And you don't have to repeat the attacking logic, and new types of characters only need to override how they pick a move.


Peer to Peer GraphQL with Relay over WebSocket Here is my Relay Network:
async function fetchQuery(
    operation,
    variables,
) {
    // hash graphql query for unique listener
    const hash = await digestMessage(operation.text);

    // send query to websocket
    doSend(JSON.stringify({
        message: 'request',
        query: operation.text,
        hash,
        variables,
    }));

    // listen on response from WebSocket, events to promise
    return await response({ eventEmitter, hash })
}

Here is my WebSocket listener:
function onMessage (evt: MessageEvent) {
  const payload = JSON.parse(evt.data)

  if (payload.message === 'request') {
    // run graphql query if WebSocket message is a 'request'
    graphql(schema, payload.query).then(result => {
      doSend(
        JSON.stringify({
          message: 'response',
          hash: payload.hash,
          data: result.data
        })
      )
    })
  } else if (payload.message === 'response') {
    // emit local event with results of graphql resolver if WebSocket message is a 'response'
    eventEmitter.emit(payload.hash, { data: payload.data })
  }
}

Design Goals:

GraphQL support with minimal backing services, here just a WebSocket server.
Use client devices, the browser, for persistence instead of a server.

Design Constraints / GraphQL and Relay Goals:

Provide data in a form that is useful to product developers building
a view.
Fetch data more efficiently.
Decouple product code and server logic.

Attempted Approaches:

WAMP: Needs a server for router.
Web-RTC: Needs a WebSocket server for signaling. For a broadcast, each peer needs to connect to all peers without a server or use WebTorrent / DHT
WebSocket in React useEffect: No React reconciliation. No Relay declarative data-fetching.

Work in Progress:

Load balancing: Limiting only one response, and deciding who responds next.
Versioning: If peers have an old schema or resolver logic changes.
Authentication: Verifying OAuth tokens in browser.
Local storage security and risk: Browser now stores credentials, like payment processing tokens, instead of server.
Abuse: Just anyone joins WebSocket and sends out bad data.

Feedback welcome. I looked for similar projects and found a lack of references.

Don't!
After working on this for too long, I'm moving onto something else, and don't suggest you do this. Typical over reach. When building an internet application, you have to ask yourself, are internet servers and the internet itself really the problem? Here are some specifics:

Are Servers Really a Problem? WebRTC requires signaling. Free platforms or WebSocket servers can still be DDoS, requiring something else in front of it. Servers are an easy, often free utility compared to storage and compute on an abstraction of client devices.
Who's Really Offline Today? Mobile and Wifi are ubiquitous. There's web workers. 5G has billions invested in using client devices in conjugation with data providers down to the chips with efficient power consumption.
Peer Discovery A lot of the products that bill themselves as decentralized are surprisingly centralized when it comes to peer discovery.
Authoritative Data Anyone can resolve a query with anything. Requests require complex validation, dropping the peer, and reporting to a signaling server.
Dedicated Coverage No peer instances require a server. Headless browsers may drop WebRTC support.
Private data Servers provide an area of mitigated security, as in, if someone's rooted your server and reads API secrets, you have bigger problems. Peers require specialization, as in adding a Stripe secret in local storage, and announcing checkout support.
HTTP Cookie This was the final deal breaker. Cookies with the secure header prevent JavaScript access to secrets. Server required, non-negotiable.

At best, you have a very complex and insecure infrastructure still running servers.
A better alternative is to find a graphql platform. Then run fail2ban for DDoS protection on a free tier Google Cloud Engine server, and put that in front of the graphql platform.
For more scaling, like expensive compute, setup redis as a queue, and process on a local private network.
Rest here is left as a reference:

I created an npm based on this concept, with github repo.. Feedback and contributions welcomed.
Here's some highlights after about a month's work
Load Balancing

Create a random delay on each peer from 0 to 3s in intervals of 20ms.
During delay, use GraphQL's local state management to save incoming responses.
After delay, check local state for response. Clear state if exists then short circuit exit.

Source Authentication

Use a public key crypto library like nacl.
When sending a response, sign the hash, and provide public key.
When receiving a response, verify the signature.

Validation

Use io-ts to validate the WebSocket message. My hash, query, and signature properties are strings. My variable property is a record of strings and my uri property, is either "request" or "response".
graphql runs the request's query in browser with variables so no injection.
A custom graphql-tools codegen takes the graphql schema, and generates io-ts types. These types runtime decode websocket response data.

Authoritative Data
Even with all this, there is no authoritative data sources. If you build an online store, someone can join and start responding with {price: 0.00}.
Load balancing times them out, authentication tags them, validation makes them do some work, so there's momentum working here. It's not just opening a URL. Still it's possible to open a browser window and send bad data.
My plan is to use brain.js. Train a test set of good and bad source detection. Bundle with the app. Property, value, and source should be enough for consensus and alerting the peer network of bad sources. Other ideas are transparency with good UI and let the user detect bots, and also references. Send a supplier list along with the price, and let the app independently validate. All these are domain specific so I left it out of graphql networking implementation.
Rendering, Suspense API
Work in progress, will report back soon.
Update: 10/21/20
RelayEnvironmentProvider component suspends. Since this at the top level of the app, the entire app suspends as oppose to individual components. The relay issue-tracker demo has a custom react experimental version. I'm creating a npm to see if I can drop this functionality into Relay networking.
Bundle Size
react, react-relay, relay-runtime, and graphql are all 100kb+ each. brain.js is at ~580kb. Working in the browser, and built for the browser are two different things.
To keep bundle size small, I webpack these as external modules loaded from the vendor's CDN, excluded from webpack build.
Other
graphql-tools has limited browser support. file includes do not work. Instead I went with compile time codegen while building io-ts runtime decoding from graphql schema.
jest has limited support too, especially with create-react-app. I wanted to import all files in a folder automatically. browser supports this but jest doesn't. If I want that extensibility, compile time codegen would be the best.
Also jest does not support TextEncoder, which relay uses, and was used but no longer in my nacl implementation. I used StandardLib instead.
I looked into modeling a state machine for this. xState and graphql is redundant. graphql is really nice, the best state management there is. It's best to avoid complex state as most as possible. Straight through pure functions with just input and output, best you can do.


Comparison methods for Photo objects I'm unsure of how to Name and clearly distinguish between methods that deals with comparisons.

I know of the following two established concepts:

.Equals: A method to see if two objects refer to the same instance

.Compare: A method often used when determining sort order among objects

But what to name a method when you are interested to see if two different object instances refer to the same "logical" object, or when comparing values of two different object instances? Let me give you an example with a class named Photo.

public class Photo : IEquatable<Photo>
{
    public string FileName { get; private set; }
    public string InFolder { get; private set; }
    public string FullFilePath { get; private set; }
    public string FileExtension { get; private set; }
    public long FileSize { get; private set; }
    public DateTime FileLastModified { get; private set; }
    public DateTime FileCreated { get; private set; }
    public ExifData EXIF
    {
        get
        {
            if (this._exif == null)
                LoadExtendedProperties();
            return this._exif;
        }
        set { this._exif = value; }
    }
    public Photo(string fullFilePath)
    {
        //...
    }
    public override int GetHashCode()
    {
        return this.FullFilePath.GetHashCode();
    }
    private void LoadExtendedProperties()
    {
        //...
    }
    public bool Equals(Photo other)
    {
        if (object.ReferenceEquals(this, null) || object.ReferenceEquals(other, null))
            return false;
        if (object.ReferenceEquals(this, other))
            return true;
        else
            return false;
    }
    /// <summary>
    /// Do they refer to the same photo (file)
    /// </summary>
    public bool UnnamedCompareMethodA(Photo other)
    {
        if(object.ReferenceEquals(other, null))
            return false;
        if (this.FullFilePath == other.FullFilePath)
            return true;
        else
            return false;
    }
    /// <summary>
    /// Do all properties have the same value?
    /// Usefull when determining if a Photo has been modified since last loaded
    /// </summary>
    public bool UnnamedCompareMethodB(Photo other)
    {
        // Note: Instead of manually adding fields; Reflection might be a better choice.
        // Though that might bring other challenges as some fields should be left out when comparing.

        // If any object is null then they are NOT considered equal
        if (object.ReferenceEquals(this, null) || object.ReferenceEquals(other, null))
            return false;
        if (this.FileLastModified != other.FileLastModified)
            return false;
        if (this.FullFilePath != other.FullFilePath)
            return false;

        if (this.FileSize != other.FileSize)
            return false;
        if (this.EXIF.Title != other.EXIF.Title)
            return false;
        if (this.EXIF.Rating != other.EXIF.Rating)
            return false;
        if (this.EXIF.PictureTaken != this.EXIF.PictureTaken)
            return false;
        if (!this.EXIF.Orientation.Equals(other.EXIF.Orientation))
            return false;
        if (!this.EXIF.Dimensions.Equals(other.EXIF.Dimensions))
            return false;
        if (!this.EXIF.Location.Equals(other.EXIF.Location))
            return false;

        return true;
    }
}

Question:
What to name UnnamedCompareMethodA() and UnnamedCompareMethodB()? I would like to have "Equal" somewhere in the name but I feel like that would create confusion as they are not related at all with .Equals().

You seem confused.  Given two objects, a.Equals(b) tests whether they have equivalent content.  Basically, what you wrote as UnnamedCompareMethodB() should actually be the Equals() method.

I wouldn't bother with defining an UnnamedCompareMethodA() at all.  If someone wants to compare objects by FullFilePath, they can just write a.FullFilePath.Equals(b.FullFilePath).

Furthermore, calling object.ReferenceEquals(this, null) makes little sense.  If the object were indeed null, the code would have already crashed with a NullReferenceException before then.


A random relative prime number based on the modulus I am implementing RSA in java using BigInteger and I have a method to return a random relative prime number based on the modulus m.

The method looks like this:

BigInteger prime = new BigInteger(m.bitLength(),rnd);

    while (prime.compareTo(BigInteger.ZERO) <= 0 || !prime.gcd(m).equals(BigInteger.ONE) )
    {
        prime = new BigInteger(m.bitLength(),rnd);
    }

Is there anyway to make this more efficient? I dont know if recreating an object like this is bad or not.

What is m.bitLength, and how often do you get a <= 0 or an gcd-1?

Why do you think creating an object in the loop could be bad? Performance? Memory wise?

An object is ready for garbage collection as soon as it is unreachable and out of scope.

So the second object, if the first one fits the condition, will hide the first one - the first one is out of scope, and unreachable.

For a cheap object like a BigInteger, you may worry if you produce millions and millions of Objects in one second, over and over again. Maybe then, if you experience a performance problem, you can measure and find out, where it happens.


Macro to distribute data within the sheets based on tabs name I wrote a macro which works, but given the large amount of data, it takes a lot of time. I wonder if there is a way to speed it up. Here is a summary of what it does:

In "Summary (All)" tab I have global sheet with all the data.
I have several tabs corresponding to the each months.
I want to have the data distributed to those tabs, based on the value in one column (should match the tab name).

My code

Option Explicit

Sub CopyDataOutToSheets()
    Dim sh As Worksheet
    Dim SourceSh As Worksheet
    Dim Last As Long
    Dim shLast As Long
    Dim CopyRng As Range
    Dim StartRow As Long
    Dim lrow As Long
    Dim r As Long

    With Application
        .ScreenUpdating = False
        .EnableEvents = False
    End With

    Set SourceSh = ActiveWorkbook.Worksheets("Summary (All)")
    Application.DisplayAlerts = False
    On Error Resume Next
    On Error GoTo 0

    For Each sh In ActiveWorkbook.Worksheets
        If IsError(Application.Match(sh.Name, _
                                     Array(SourceSh.Name, "List Data", _
                                     "Lists", "Summary (Filtered)"), 0)) Then
            lrow = lastRow(sh)
            If lrow > 6 Then
                sh.Rows("7:" & lrow).Delete
            End If

            If lastRow(SourceSh) < 7 Then
                MsgBox ("Nothing to move")
                Exit Sub
            End If

            For r = lastRow(SourceSh) To 7 Step -1
                'Finding the first empty row in column A on destination worksheet
                If SourceSh.Range("N" & r).Value = sh.Name Then
                    SourceSh.Rows(r).Copy Destination:= _
                                                  sh.Range("A" & lastRow(sh) + 1)
                End If
            Next r
        End If
    Next

ExitTheSub:
    Application.Goto SourceSh.Cells(1)
    Application.DisplayAlerts = True
    With Application
        .ScreenUpdating = True
        .EnableEvents = True
    End With
End Sub

There are few thousands rows in global tab, so I'm expecting to have roughly 1000 in each tab after distribution. Could you please let me know what would be the best way to make it happen more quickly?

Use any method that make a "directly-interation" with worksheet like copy, paste, range.value = something (...) intensifies the runtime of the code. To solve this you need to create an array to store the SourceSh.Rows(r) values. Before the loop change spreadsheet you parse all values stored in the array to cells using the method that I will show in the example. When the loop change spreadsheet you clear the array.

With that I think that you will have at least 65% better performance.

I created an example comparing runtime of the array method with the single value method,
to 20k cells. See the difference?

[

Sub Compare()

Dim arr() As Variant
Dim max As Long
Dim start As Double

start = Timer

max = 20000
ReDim arr(max)

Application.ScreenUpdating = False

'Using the array method

For i = 1 To max

    arr(i) = i

Next i

Sheets("Planilha1").Range("A1:A" & UBound(arr) + 1) = WorksheetFunction.Transpose(arr)
Debug.Print "Array method in seconds: " & Round(Timer - start, 2)

'Parsing single values method

start = Timer

For i = 1 To UBound(arr)

    Sheets("Planilha1").Range("A1:A" & UBound(arr) + 1) = i

Next i

Debug.Print "Value method in seconds: " & Round(Timer - start, 2)

Application.ScreenUpdating = True

End Sub


Scraping through product pages I'm working through a scraping function where pages of results lead to product pages.  I've added a default maximum number of results pages, and pages per set of results, to prevent a simple mistake of crawling the whole site.

Here's what I have so far.  Does the way I'm implementing the maximums with the for loops make sense?  Is there a more "pythonic" way? I'm coming at this from a completely learning perspective.

def my_crawler(url, max_pages = 1, max_items = 1):

    for page_number in range(1, max_pages + 1):
        source_code = requests.get(url + str(page_number)).text

        products = SoupStrainer(class_ = 'productTags')
        soup = BeautifulSoup(source_code, 'html.parser', parse_only=products)

        for item_number, a in enumerate(soup.find_all('a')):
            print(str(item_number) + ': ' + a['href'])

            if item_number == max_items - 1: break

my_crawler('http://www.thesite.com/productResults.aspx?&No=')

There's not a lot to say about such a simple program, mainly nitpicks.

You are violating PEP8 at a few places.
The code reformatted to follow PEP8 would look like this:

def my_crawler(url, max_pages=1, max_items=1):
    for page_number in range(1, max_pages + 1):
        source_code = requests.get(url + str(page_number)).text

        products = SoupStrainer(class_='productTags')
        soup = BeautifulSoup(source_code, 'html.parser', parse_only=products)

        for item_number, a in enumerate(soup.find_all('a')):
            print(str(item_number) + ': ' + a['href'])

            if item_number == max_items - 1:
                break

In particular:

Write method parameters compactly as max_pages=1 instead of max_pages = 1
Break line after :

Instead of range(1, max_pages + 1) I would find range(max_pages) simpler if you only need to do max_pages + 1 in the code. So I'd rewrite the above as:

for page_number in range(max_pages):
    source_code = requests.get(url + str(page_number + 1)).text

And as a mild case of paranoia,
I'd prefer the if statement for breaking out of the loop this way:

if item_number >= max_items - 1:
    break

And yes, it's fine to break out of the loop this way.
I don't think there's a better way.

When you print the item number and the link,
I think this is slightly more readable:

print('{}: {}'.format(item_number, a['href']))

Finally, don't execute code in global scope,
it's recommended to wrap code in an if __name__ == '__name__': guard:

if __name__ == '__name__':
    my_crawler('http://www.thesite.com/productResults.aspx?&No=')

These are mainly just nitpicks though. You're doing fine.