Yahtzee Code Kata in Python I tried a Yahtzee kata in Python the other day. I'm not particularly experienced in Python (C# is my main language) so I'd be interested in any feedback as to how I could have done things better. In particular, I wonder whether my Count and HighestRepeated methods could be a little more elegant.

import unittest

#helpers
def Count(dice, number):
    return len([y for y in dice if y == number])

def HighestRepeated(dice, minRepeats):
    unique = set(dice)
    repeats = [x for x in unique if Count(dice, x) >= minRepeats]
    return max(repeats) if repeats else 0

def OfAKind(dice, n):
    return HighestRepeated(dice,n) * n

def SumOfSingle(dice, selected):
    return sum([x for x in dice if x == selected])

#strategies
def Chance(dice):
    return sum(dice)

def Pair(dice):
    return OfAKind(dice, 2)

def ThreeOfAKind(dice):
    return OfAKind(dice, 3)

def FourOfAKind(dice):
    return OfAKind(dice, 4)

def SmallStraight(dice):
    return 15 if tuple(sorted(dice)) == (1,2,3,4,5) else 0

def LargeStraight(dice):
    return 20 if tuple(sorted(dice)) == (2,3,4,5,6) else 0

def Ones(dice):
    return SumOfSingle(dice,1)

def Twos(dice):
    return SumOfSingle(dice,2)

def Threes(dice):
    return SumOfSingle(dice,3)

def Fours(dice):
    return SumOfSingle(dice,4)

def Fives(dice):
    return SumOfSingle(dice,5)

def Sixes(dice):
    return SumOfSingle(dice,6)

def Yahtzee(dice):
    return 50 if len(dice) == 5 and len(set(dice)) == 1 else 0

class YahtzeeTest(unittest.TestCase):
    testCases = (
        ((1,2,3,4,5), 1, Ones),
        ((1,2,3,4,5), 2, Twos),
        ((3,2,3,4,3), 9, Threes),
        ((3,2,3,4,3), 0, Sixes),
        ((1,2,3,4,5), 0, Pair), # no pairs found
        ((1,5,3,4,5), 10, Pair), # one pair found
        ((2,2,6,6,4), 12, Pair), # picks highest
        ((2,3,1,3,3), 6, Pair), # only counts two
        ((2,2,6,6,6), 18, ThreeOfAKind),
        ((2,2,4,6,6), 0, ThreeOfAKind), # no threes found
        ((5,5,5,5,5), 15, ThreeOfAKind), # only counts three
        ((6,2,6,6,6), 24, FourOfAKind),
        ((2,6,4,6,6), 0, FourOfAKind), # no fours found
        ((5,5,5,5,5), 20, FourOfAKind), # only counts four
        ((1,2,5,4,3), 15, SmallStraight),
        ((1,2,5,1,3), 0, SmallStraight),
        ((6,2,5,4,3), 20, LargeStraight),
        ((1,2,5,1,3), 0, LargeStraight),
        ((5,5,5,5,5), 50, Yahtzee),
        ((1,5,5,5,5), 0, Yahtzee),
        ((1,2,3,4,5), 15, Chance),
        )

    def testRunAll(self):
        for (dice, expected, strategy) in self.testCases:
            score = strategy(dice)
            self.assertEquals(expected, score, "got {0} expected {1}, testing with {2} on {3}".format(score, expected, strategy.__name__, dice))
        print 'ran {0} test cases'.format(len(self.testCases))

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

import unittest

#helpers
def Count(dice, number):

The python style guide recommends lowercase_with_underscores for global functions.

    return len([y for y in dice if y == number])

This can be written as

    return dice.count(number)

def HighestRepeated(dice, minRepeats):
    unique = set(dice)
    repeats = [x for x in unique if Count(dice, x) >= minRepeats]
    return max(repeats) if repeats else 0

This will be somewhat clearer if you find the highest repeated count and then check it against minRepeats. You can also use the counter class:

counts = collections.Counter(dice)
element, count = counts.most_common()[0]
return count if minRepeats <= count else 0

def OfAKind(dice, n):
    return HighestRepeated(dice,n) * n

def SumOfSingle(dice, selected):
    return sum([x for x in dice if x == selected])

Could be written as

    return collections.Counter(dice)[selected]

#strategies
def Chance(dice):
    return sum(dice)

def Pair(dice):
    return OfAKind(dice, 2)

def ThreeOfAKind(dice):
    return OfAKind(dice, 3)

def FourOfAKind(dice):
    return OfAKind(dice, 4)

def SmallStraight(dice):
    return 15 if tuple(sorted(dice)) == (1,2,3,4,5) else 0

Shouldn't a small straight consist of 4 elements in a row?

def LargeStraight(dice):
    return 20 if tuple(sorted(dice)) == (2,3,4,5,6) else 0

Can't a large straight start with 1?

def Ones(dice):
    return SumOfSingle(dice,1)

def Twos(dice):
    return SumOfSingle(dice,2)

def Threes(dice):
    return SumOfSingle(dice,3)

def Fours(dice):
    return SumOfSingle(dice,4)

def Fives(dice):
    return SumOfSingle(dice,5)

def Sixes(dice):
    return SumOfSingle(dice,6)

Writing so many functions that differ only in a parameter they pass to another function is odd.

def Yahtzee(dice):
    return 50 if len(dice) == 5 and len(set(dice)) == 1 else 0

Can't you assume that there are always 5 dice? Also, why don't you use OfAKind?

class YahtzeeTest(unittest.TestCase):
    testCases = (
        ((1,2,3,4,5), 1, Ones),
        ((1,2,3,4,5), 2, Twos),
        ((3,2,3,4,3), 9, Threes),
        ((3,2,3,4,3), 0, Sixes),
        ((1,2,3,4,5), 0, Pair), # no pairs found
        ((1,5,3,4,5), 10, Pair), # one pair found
        ((2,2,6,6,4), 12, Pair), # picks highest
        ((2,3,1,3,3), 6, Pair), # only counts two
        ((2,2,6,6,6), 18, ThreeOfAKind),
        ((2,2,4,6,6), 0, ThreeOfAKind), # no threes found
        ((5,5,5,5,5), 15, ThreeOfAKind), # only counts three
        ((6,2,6,6,6), 24, FourOfAKind),
        ((2,6,4,6,6), 0, FourOfAKind), # no fours found
        ((5,5,5,5,5), 20, FourOfAKind), # only counts four
        ((1,2,5,4,3), 15, SmallStraight),
        ((1,2,5,1,3), 0, SmallStraight),
        ((6,2,5,4,3), 20, LargeStraight),
        ((1,2,5,1,3), 0, LargeStraight),
        ((5,5,5,5,5), 50, Yahtzee),
        ((1,5,5,5,5), 0, Yahtzee),
        ((1,2,3,4,5), 15, Chance),
        )

    def testRunAll(self):
        for (dice, expected, strategy) in self.testCases:
            score = strategy(dice)
            self.assertEquals(expected, score, "got {0} expected {1}, testing with {2} on {3}".format(score, expected, strategy.__name__, dice))
        print 'ran {0} test cases'.format(len(self.testCases))

You are duplicating some of the logic of the unit test system here. I recommend using nose tests which would allow this one function to generate a number of tests each of which is considered an independent test by the unit testing system. Then you wouldn't need to spend so much effort.

I'd also consider writing functions for each of your tests rather then putting them in a huge list. The list technique is really nice but its best used for calling the same function. Using it to test so many different functions is odd.

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


D minMax function implementation I happened to need to find both the maximum and the minimum of an array, so I decided to implement a minMax function in D.

Questions:

Is this an efficient implementation?
Is it readable?
Any general feedback is appreciated.

Thanks!

import std.range;
import std.traits;
import std.typecons;
import std.functional;

class EmptyContainerException : Exception {
    this(string msg, string file = __FILE__, size_t line = __LINE__) {
        super(msg, file, line);
    }
}

// returns a tuple containing the minimum and maximum elements of a finite InputRange
template minMax(alias pred = "a") {
    alias map = unaryFun!pred;

    auto minMax(Range)(Range r)
    if(isInputRange!Range && !isInfinite!Range && is(typeof(map(r.front)))) {
        if(r.empty) {
            throw new EmptyContainerException(
                "minMax expected a container with at least 1 element, got an empty container."
            );
        }
        auto front = r.front;
        auto frontMap = map(front);

        auto min = front;
        auto max = front;
        auto minMap = frontMap;
        auto maxMap = frontMap;

        r.popFront;
        while(!r.empty) {
            front = r.front;
            frontMap = map(front);
            if(frontMap > maxMap) {
                max = front;
                maxMap = frontMap;
            }
            if(frontMap < minMap) {
                min = front;
                minMap = frontMap;
            }
            r.popFront;
        }
        return tuple!("min", "max")(min, max);
    }
}

unittest {
    import std.exception : assertThrown;

    int[] test1 = [1, 2, 3, 4, 5];
    auto test1result = test1.minMax;
    assert(test1result.min == 1 && test1result.max == 5);

    string test2 = "Hello, World!";
    auto test2result = test2.minMax;
    assert(test2result.min == ' ' && test2result.max == 'r');

    int[] test3 = [];
    assertThrown!EmptyContainerException(test3.minMax);

    string[] test4 = ["who", "is", "the", "longest", "word"];
    auto test4result = test4.minMax!"a.length";
    assert(test4result.min == "is" && test4result.max == "longest");
}

Is this an efficient implementation?

Yup. It calls map exactly once per item, and otherwise does nothing but the regular range calls. A specialized implementation could be more efficient in some cases (but probably only if using vector instructions), as could a parallel implementation, but neither of these have the genericity that your code does.

  Is it readable?

Absolutely, it's some of the most readable code I've seen. The content of the loop is somewhat dense. You could perhaps make it less dense like this:

    auto front = tuple!("elem", "map")(r.front, map(r.front));
    auto min = front;
    auto max = front;

    r.popFront;
    foreach (e; r) {
        front = tuple(e, map(e));
        if(front.map > max.map) max = front;
        if(front.map < min.map) min = front;
    }
    return tuple!("min", "max")(min.elem, max.elem);

Apart from that, I got nuthin'.

  Any general feedback is appreciated.

As hinted at in the section above, instead of while (!empty), you might want to use foreach. This reduces your code by one line (r.popFront()), and makes it clearer that you're going to iterate over every element of the range. It shouldn't change the performance of the code in any way.

You should put an if (is(typeof(unaryFun!pred))) constraint on the template, so the user of the function gets an error message on the line where he or she tries to instantiate it with arr.minMax!"invalid string".

Also, the name pred is wrong - a predicate is a boolean-valued function, like equal, same color or contains bees. I'd call it Fn or Fun.

As a minor nit, I'd also change the name of map, probably to fn or fun (note the difference in capitalization). This because I kept conflating your map with std.algorithm.map. The concept is correct, but essentially the name is already taken.

All in all, very good - my only comments are essentially nitpicks.


Find the center of n squares which together build a rectangle I have a rectangle with size w and height h. Now I want to split this rectangle into n new rectangles that are as similar as possible to a square. Afterwards I'd like to calculated the center of each square.

public static List<Point> getCenters(int number, double width, double height)
    {
        List<Point> points = new List<Point>();
        int originalNumberOfSquares = number;
        int numberOfSquares = number;
        if (numberOfSquares % 2 == 1)
        {
            numberOfSquares++;
        }
        int rectangleWidth = Convert.ToInt32(width);
        int rectangleHeight = Convert.ToInt32(height);

        double minDistance = Double.MaxValue;
        int nSquaresInRow = -1;
        int nSquaresInColumn = -1;

        for (int i = 0; i <= numberOfSquares; i++)
        {
            for (int j = 0; j <= numberOfSquares; j++)
            {
                if (i * j == numberOfSquares)
                {
                    if (Math.Abs(i - j) < minDistance)
                    {
                        minDistance = Math.Abs(i - j);
                        nSquaresInRow = i;
                        nSquaresInColumn = j;
                    }
                }
            }
        }

        int squareWidth = rectangleWidth / nSquaresInColumn;
        int squareHeight = rectangleHeight / nSquaresInRow;

        for (int r = 0; r < originalNumberOfSquares; r++)
        {
            int xSquareCenter = (((r + 1) * 2) - 1) * (squareWidth / 2);
            while (xSquareCenter > rectangleWidth)
            {
                xSquareCenter = xSquareCenter - rectangleWidth;
            }

            int row = (r / nSquaresInColumn) + 1;
            int ySquareCenter = ((2 * row) - 1) * (squareHeight / 2);

            points.Add(new Point(Convert.ToDouble(xSquareCenter), Convert.ToDouble(ySquareCenter)));
        }

        return points;
    }

Now the code works, but I think it's a little bit ugly. Any hints on how I can improve it?

A couple of notes:

The width, height and number of sub-rectangles should be parameters, not local variables. If you should, for example, want to print out the centres for differing values of w, h and n, calling the method in a loop with different arguments is more convenient than changing the code and rerunning it multiple times.
Determining the size of the sub-rectangles and collecting their centres could be done in two separate methods because a) they can easily be separated as they're not intertwined b) it makes it immediately obvious which part is done by which code and c) they might be useful on their own.
Rather than outputting the centres, you should return them in an array or list. This way you can also easily use the method in a context where you don't want to print the centres or want to do something to them before printing. Also it's generally a good practice to separate IO code from logic code.
Since both i and j are ints and thus minDistance will only ever hold integer values, it should have type int, not double. Otherwise it leaves the impression that it could possibly have a non-integer value.
On a similar note you should probably make the type of width and height int as well since the first thing you do is to truncate them to ints. Having their type be double might make the user of your method think the results will be more accurate than they really are.


Hook to switch the linter binaries in Emacs Lisp according to virtual environment Problem

I use Emacs for Python development along with several linters. When I activate a Python virtual environment (venv) from within Emacs, I would like to set the linter binaries according to the following rules:

If the venv has a particular linter installed, use it
If the venv does not have a particular linter, use the one in a pre-specified, default venv
If the linter does not exist in either the active venv or the default venv, set the linter binary to nil

For example, let's say I activate a venv called my_venv that has pylint installed, but no flake8. flake8 is however installed in my default linters venv. After activating my_venv, the linters that will be used are

pylint -- my_venv
flake8 -- linters

Purpose

The reason for implementing this is that I develop Python on multiple machines that share a single, version-controlled init.el file. I do not want to guarantee that I have the same Python binaries and venvs across these machines; this implementation helps decouple my Emacs setup from the Python venvs that are present on a machine.

Additional info

The code will go inside my init.el file
I use flycheck as the interface between Emacs and the linters
I use pyvenv for Python virtual environments in Emacs

Code

(defvar linter-execs '((flycheck-python-flake8-executable "bin/flake8")
                       (flycheck-python-pylint-executable "bin/pylint")
                       (flycheck-python-pycompile-executable "bin/python")))
(defvar default-linter-venv-path (concat (getenv "WORKON_HOME") "/linters/"))

(defun switch-linters ()
  "Switch linter executables to those in the current venv.

If the venv does not have any linter packages, then they will be
set to those in the `default-linter-venv-path` venv.  If these do
not exist, then no linter will be set."
  (dolist (exec linter-execs)
    (let ((venv-linter-bin    (concat pyvenv-virtual-env (nth 1 exec)))
          (default-linter-bin (concat default-linter-venv-path (nth 1 exec)))
          (flycheck-var       (nth 0 exec)))
      (cond ((file-exists-p venv-linter-bin)
             (set flycheck-var venv-linter-bin))
            ((file-exists-p default-linter-bin)
             (set flycheck-var default-linter-bin))
            (t (set flycheck-var nil))))))

(add-hook 'pyvenv-post-activate-hooks 'switch-linters)

Explanation

linter-execs is a list of two-element lists. The first element of an entry is the flycheck variable that contains the path of a linter binary. The second element is the relative path of the binary from within the venv.
The default linter venv is $WORKON_HOME/linters
switch-linters is the call-back function attached to pyvenv-post-activate-hooks
The conditional checks for the presence of the linter binary files, first in the current venv and next in the default venv. Failing these, it sets the binary to nil in the line (t (set flycheck-var nil))

Specific questions

Is this idiomatic elisp, or is it too "Pythonic"?
Is linter-execs the proper way to implement a list of tuples in elisp?

It would be a good idea to write a docstring for the defvar'd variables like linter-execs. The explanations in the post would make an excellent start, for example:

(defvar linter-execs '((flycheck-python-flake8-executable "bin/flake8")
                       (flycheck-python-pylint-executable "bin/pylint")
                       (flycheck-python-pycompile-executable "bin/python"))
  "The linter executables, as list of two-element lists. The
first element of an entry is the flycheck variable that contains
the path of a linter executable. The second element is the
relative path of the executable from within the venv.")

Writing (nth 0 exec) and (nth 1 exec) makes it hard to understand the meaning of these items of data. In Python you'd use tuple unpacking to give meaningful names to each element of the exec data structure, like this:

flycheck_var, path = exec

In Emacs Lisp you can use cl-destructuring-bind in a similar way:

(dolist (exec linter-execs)
  (cl-destructuring-bind (flycheck-var path) exec
    (let ((venv-linter-bin    (concat pyvenv-virtual-env path))
      ;; etc

But exec comes from a list that you are looping over, so you could use the cl-loop macro instead of dolist, and cl-loop has destructuring built in:

(cl-loop for (flycheck-var path) in linter-execs do
  (let ((venv-linter-bin    (concat pyvenv-virtual-env path))
    ;; etc

You'll need (require 'cl-macs) to use cl-destructuring-bind or cl-loop.
The two directories pyvenv-virtual-env and default-linter-venv-path are treated in exactly the same way: first we do (let ((Y (concat X path)) and then (file-exists-p Y) and finally (set flycheck-var Y). This repetition could be factored out into a loop:

(defun switch-linters ()
  "Switch linter executables to those in the current venv.

If the venv does not have any linter packages, then they will be
set to those in the `default-linter-venv-path` venv.  If these do
not exist, then the linter will be set to nil."
  (cl-loop with dirs = (list pyvenv-virtual-env default-linter-venv-path)
           for (flycheck-var path) in linter-execs
           do (set flycheck-var
                   (cl-loop for directory in dirs
                            for executable = (concat directory path)
                            if (file-exists-p executable) return executable)))

Instead of file-exists-p, you probably want to use file-executable-p.
Looking for a file in a list of directories is built into Emacs as the function locate-file. Using this, we get:

(defun switch-linters ()
  "Switch linter executables to those in the current venv.

If the venv does not have any linter packages, then they will be
set to those in the `default-linter-venv-path` venv.  If these do
not exist, then the linter will be set to nil."
  (cl-loop with dirs = (list pyvenv-virtual-env default-linter-venv-path)
           for (flycheck-var path) in linter-execs
           do (set flycheck-var (locate-file path dirs nil 'file-executable-p))))


Release the acquired lock after specific timeout I'm trying to release a lock after some timeout. The lock is acquired to do any database changes so that multiple requests will not update the database simultaneously.

Below is the code

public static readonly _lockObject = new Object();

public bool UpdateData(ModifiedData modifiedData)
{
    var success = false;
    lock(_lockObject)
    {
        var thread = new Thread(()=>
        {
            try
            {
                SessionManager.Instance.OpenConversation(true); // Starts a session using nHibernate
                //Modifications

                success = true;
                SessionManager.Instance.EndConversation(true);

            }
            catch(ThreadAbortException ex)
            {
                //Do I need to handle any condition here
            }
        });
        thread.Start();
        if (!thread.Join(TimeSpan.FromMilliseconds(1000)))  // Timeout
        {
           if (!success)
              thread.Abort();
           else
           {
             thread.Join();
           }
        }
        else
        {
            if (!success)
             {
                    throw new Exception("Should Not occur");
             }
        }
    }
}

are there any better approaches available?? I tried with Action.BeginInvoke but in that not able to kill or stop action execution after some timeout.

First you shouldn't make your lock object public if it's for internal working of the class.  Typically they are marked as private.

The lock statement is just wrapper around the Monitor class.  The Monitor class has a built in timeout.   To use it we need to write a bit more code but less than what you currently have

private static readonly object _lockObject = new object();

public bool UpdateData()
{
    bool lockWasTaken = false;
    try
    {
        Monitor.TryEnter(_lockObject, TimeSpan.FromSeconds(1), ref lockWasTaken);
        if (lockWasTaken)
        {
            // do work here
        }

    }
    finally
    {
        if (lockWasTaken)
        {
            Monitor.Exit(_lockObject);
        }
    }

    return lockWasTaken;
}

Also if you are going to throw an exception you should make it useful and shouldn't normally use the standard Exception class.  If it's timing out then throw the TimeoutException.  This make it easier for callers to know it timed out and they can decide if they can capture it and retry.


Traversing and printing complex Dictionary types (Scripting.Dictionary) - Dictionary inside of a Dictionary Based on this SO post - also reposted on vba4all.com with a few more details and explanations.

Please notice there currently is no error handling whatsoever as I didn't analyse and consider any traps yet. Although feel free to supply details/code based on your assumptions. For Example - the code fails when you try to load duplicate keys into the dictionary... this can easily be handled with the OERN approach...

So the tree would look like this for example

I have created a simple procedure TraverseDictionary() which traverses any structure of dictionaries.

The simplest version that prints somehow a logical structure to the Immediate Window:

Sub Main()

    Dim dict As New Dictionary
    Dim subDict As New Dictionary
    Dim lvlDict As New Dictionary

    lvlDict.Add "LVL KEY", "LVL ITEM"

    subDict.Add "HELLO", ":)"
    subDict.Add "WORLD", ":("
    subDict.Add "OTHER", lvlDict

    dict.Add "FOO", "BAR"
    dict.Add "BOO", subDict

    TraverseDictionary dict

End Sub

Private Sub TraverseDictionary(d As Dictionary)

    For Each Key In d.Keys
        Debug.Print "KEY: " & Key
        If VarType(d(Key)) = 9 Then
            TraverseDictionary d(Key)
        Else
            Debug.Print "ITEM: " & d(Key)
        End If
    Next
End Sub

with a result:

and w/ a print out to sheet with somehow even more logical view

Private i As Long
Private depth As Long

Sub Main()
Cells.ClearContents

    Dim dict As New Dictionary
    Dim subDict As New Dictionary
    Dim lvlDict As New Dictionary

    lvlDict.Add "LVL KEY", "LVL ITEM"

    subDict.Add "HELLO", ":)"
    subDict.Add "WORLD", ":("
    subDict.Add "OTHER", lvlDict

    dict.Add "FOO", "BAR"
    dict.Add "BOO", subDict

    i = 1
    depth = 0
    TraverseDictionary dict

    Columns.AutoFit

End Sub

Private Sub TraverseDictionary(d As Dictionary)

    For Each Key In d.Keys
        Range("A" & i).Offset(0, depth) = "KEY: " & Key
        If VarType(d(Key)) = 9 Then
            depth = depth + 1
            TraverseDictionary d(Key)
        Else
            Range("B" & i).Offset(0, depth) = "ITEM: " & d(Key)
        End If
        i = i + 1
    Next
End Sub

As mr. Ioannis noticed

  There is a small risk, if an Item of a dictionary is itself an object
  but not a dictionary. Specifically, VarType(d(Key)) = 9 holds true for
  Objects of any kind (I tried it with custom classes and it returned a
  type mismatch error). I would suggest after checking for VarType, also
  checking for dictionary specifically (eg, try to access a property and
  trap the error)

Ok, so currently the code works with the assumption that the Dictionary is actually of VarType = 9 but that breaks because of Objects which also return 9.

If we check TypeName(d(key)) = "Dictionary"  then in theory it should only work with Dictionaries but can you think of any other approaches to actually check & possibly validate the Dictionary.

Side note: In the future I am most likely to create a class wrapper for this and accumulate all the checks in there along with printing at different locations and to different views.

A couple of things, which you've really more or less pointed out already.

You've shown us two different implementations of the same routine that outputs two two different places. I think what you really need is a class that gets initialized with an IOuput member. Then you can have different implementations of the IOuput interface, but TraverseDictionary only has to deal with IOutput and one routine can print out to anywhere.
You're correct that VarType = 9 is unsafe and TypeName = "Dictionary" is safer, but I'm not sure it's entirely safe. What if there's a namespace conflict because I implemented a VBAProject.Dictionary? TypeName of my custom dict would also be "Dictionary" and who knows what would happen then. TypeOf is another option though. I've not tested it, but I believe you can do this.

If TypeOf d(Key) Is Scripting.Dictionary Then

But that means you'll also have to check IsObject(d(key)) prior to checking its actual type as well.

Update:

I finally had time to test this. I created an empty Dictionary class and ran the code below. It works.

Sub test()
    Dim dict As New Scripting.Dictionary
    Dim myDict As New VBAProject.Dictionary

    Debug.Print TypeOf dict Is Scripting.Dictionary
    'true
    Debug.Print TypeOf myDict Is Scripting.Dictionary
    'false
    Debug.Print TypeOf dict Is VBAProject.Dictionary
    'false
    Debug.Print TypeOf myDict Is VBAProject.Dictionary
    'true
End Sub


Bringing content from other pages without refreshing the entire page I wrote my first AJAX script that brings content from other pages without refreshing the entire page.  I just want to see if improvements can be added to it, or if I'm doing something not recommended.

$(function(){
    // content container content loader
    $("#nav-tabs-list a").click(function() {
        var page = this.hash.substr(1);
        //$("#sidebar-content-container").html(""); // displays content only after fully loaded
        $.get("content/" + page + ".php", function(gotHtml) {
            $("#content-container").html(gotHtml);
        });
    });
    if ( location.hash ) {
        $("a[href="+location.hash+"]").click();
    } else {
        $("#nav-tabs-list a").click();
    }

    // sidebar content loader
    $("#sidebar-container #sidebar-tabs-container a").click(function() {
        var page = this.hash.substr(1);
        //$("#sidebar-content-container").html(""); // displays content only after fully loaded
        $.get("content/sidebar-content/" + page + ".php", function(gotHtml) {
            $("#sidebar-content-container").html(gotHtml);
        });
    });
    if ( location.hash ) {
        $("a[href="+location.hash+"]").click();
    } else {
        $("#sidebar-container #sidebar-tabs-container a").click();
    }
});

You seem to have some copy pasted code there.

How about

function enableLinks( selector , path , container )
{
  /* Allow ajax to work */
  $( selector ).click( function()
  {
    var page = this.hash.substr(1);
    $.get( path + page + ".php", function( html )
    {
      $( container ).html( html );
    });
  });
  /* Allow bookmarks to work */
  if ( location.hash )
    $("a[href="+location.hash+"]").click();
  else
    $(selector).click();
}

$(function()
{
  enableLinks( "#nav-tabs-list a" , "content/" , "#content-container" );
  enableLinks( "#sidebar-container #sidebar-tabs-container a" ,
               "content/sidebar-content/" ,
               "#sidebar-content-container" );
});

The things you do with location.hash are a bit of a mystery to me.


Implementing cat function in Julia I wanted to implement the cat function in Julia and I ran a few test cases but I am not sure if I am missing anything:

for line in eachline(STDIN)
    write(line)
end

It's really simple code and I tested it with the following test cases:

$ cat hello.txt | julia cat.jl
first line
second line
third line

$ cat hello.jpg | julia cat.jl > hello2.jpg
$

and hello2.jpg was the same as hello.jpg.

I think this is a good approach.

Stylistically I think you've done a good job as well; you've use a descriptive variable name (line), good indentation (4 spaces is standard for Julia), and have made use of a function that returns an iterator, which will be more efficient for this purpose than, say, one that returns an array.

The only thing I would do differently is to replace write with print. I tend to reserve the former for writing to a file and use the latter for writing to STDOUT.

There are two alternative ways to do this that I can think of. I'll provide them for completeness.

Read the entire input stream as a single string and print it to STDOUT. Note that this is quite impractical for large input streams.

print(readall(STDIN))

Read and print each character from the stream one at a time.

while !eof(STDIN)
    print(read(STDIN, Char))
end

This may provide some benefit in efficiency over your approach if the input is very large and contains few to no newlines, since eachline returns an iterator over the lines of the input, where lines are delimited by newlines. If I'm not mistaken, all lines are read before the loop body executes.

Overall though, for practical purposes I think this is a fine approach. Nice job.


UPenn CIS 194 Homework 4: Higher-order programming I am working through UPenn CIS 194: Introduction to Haskell (Spring 2013). Since I am not able to take the course for real I am asking for CR (feedback) as it could be from teacher in that course.

HW4 - Higher-order programming - Full description

import Data.List

-- Exercise 1
-- Reimplement each of the following functions in a more idiomatic Haskell style

fun1 :: [Integer] -> Integer
fun1 [] = 1
fun1 (x:xs)
  | even x = (x - 2) * fun1 xs
  | otherwise = fun1 xs

fun1' :: [Integer] -> Integer
fun1' = product . map (\x -> x - 2) . filter even

fun2 :: Integer -> Integer
fun2 1 = 0
fun2 n | even n = n + fun2 (n `div` 2)
       | otherwise = fun2 (3 * n + 1)

fun2' :: Integer -> Integer
fun2' = sum . filter even . takeWhile (\x -> x > 1) . iterate (\x -> if (even x) then div x 2 else 3 * x + 1)

-- Exercise 2
-- Generates a balanced binary tree from a list of values using foldr

data Tree a = Leaf
            | Node Integer (Tree a) a (Tree a)
  deriving (Show, Eq)

foldTree :: [a] -> Tree a
foldTree = foldr makeTree Leaf

makeTree :: a -> Tree a -> Tree a
makeTree a Leaf = Node 0 Leaf a Leaf
makeTree a (Node h l m r)
   | height l <= height r = Node (height (makeTree a l) + 1) (makeTree a l) m r
   | otherwise = Node (height (makeTree a r) + 1) l m (makeTree a r)
  where
    height Leaf = -1
    height (Node h _ _ _) = h

-- Exercise 3
-- More folds:

-- implement a function which returns True if and only if there are a odd number of True values contained in the input list
xor :: [Bool] -> Bool
xor = odd . foldr (\x s -> if (x) then 1 + s else s) 0

-- map’ should behave identically to the standard map function
map' :: (a -> b) -> [a] -> [b]
map' f = foldr (\x xs -> (f x) : xs) []

-- implement foldl using foldr
myFoldl :: (a -> b -> a) -> a -> [b] -> a
myFoldl f base xs = foldr (\x s -> f s x) base (reverse xs)

-- Exercise 4
-- Implement the Sieve of Sundaram algorithm using function composition

sieveSundaram :: Integer -> [Integer]
sieveSundaram n = map (\x -> 2 * x + 1) (genSieve n)
  where
    valid x = (i <= j) && (i + j + 2 * i * j <= n) where (i, j) = x
    genCrossed m = map (\(i, j) -> (i + j + 2 * i * j)) (filter valid (cartProd [1..m] [1..m]))
    genSieve n = (\\) [1..n] (genCrossed n)

cartProd :: [a] -> [b] -> [(a, b)]
cartProd xs ys = [(x,y) | x <- xs, y <- ys]

hlint

hlint is a great tool for improving your mastery of Haskell idioms and syntax.

For example, here is one thing (of several) it found in your code:

input.hs:20:40: Warning: Avoid lambda
Found:
  \ x -> x > 1
Why not:
  (> 1)

I would study the output of hlint.

fun1'

Looks fine.

fun2'

Stylistically I like to avoid long inline expressions. Use let and where to define meaningful subexpressions. I find this more readable (also
combining the hlint suggestion):

fun2' = sum . filter even . takeWhile (> 1) . iterate collatz
  where collatz x = if (even x) then div x 2 else 3 * x + 1

collatz could even be a top-level function because it is generally useful.

makeTree

You are repeating the expressions makeTree a l and makeTree a r.
GHC might be able to detect the common sub-expression and unify the two
so that they are only evaluated once.

ghci doesn't do any CSE, so the same call will be executed twice.

In most cases it's best to use a where or let to ensure the expression
is only evaluated once:

| height l <= height r =
    let left = makeTree a l
    in  Node (height left + 1) left m r

More info on GHC and CSE: https://wiki.haskell.org/GHC/FAQ#Does_GHC_do_common_subexpression_elimination.3F

Also, height looks like it would be generally useful - why not expose it at the top level?

xor

Looks like it will work, but since you are not specifying a type for s
it will likely default to being an Integer.

But you don't need a whole Integer to keep track of the parity of a number. In fact, you can get by with simply a Bool.

As a extension to this exercise, try to write xor without using addition.

myFoldl

Try to write it without using reverse.

Hint: You have to build up a higher-order function.

Your answer will look like:

foldl f a xs =  (some big function) a

where (some big function) will be a foldr (\g x -> ...) g0 xs.

Another hint: With a fold like:

foldr h y xs

you know that y = foldr h y [], so use this to determine what g0 is.

Sundram

Stylistically I would use list comprehensions as much as possible - replacing map and filter.

Instead of:

sieveSundaram n = map (\x -> 2 * x + 1) (genSieve n)

I would write:

sieveSundaram n = [ 2*x+1 | x <- genSieve n]

and instead of:

genCrossed m = map (\(i, j) -> (i + j + 2 * i * j)) (filter valid (cartProd [1..m] [1..m]))

I would write:

genCrossed m = [ i+j+2*i*j | (i,j) <- cartProd [1..m] [1..m], valid (i,j) ]

Also, you are computing the expression i + j + 2 * i * j twice. Can you think of a way of organizing your code so that it is only computed once?

valid

The where clause in the valid function can be dropped if you define it this way:

valid (i,j) = ...

Given a value for m, how many pairs is valid examining? Can you think of a way to reduce that number by about half?


Multiple conditions for same table in EF I am getting products using given parameters (name of real entity and columns changed for illustration purposes),

    public IQueryable<Product> GetProductsByPara(string price, string weight, string dimensions, string trend)
    {
        var result = dbContext.Products.Where(a =>
            a.Price.ToLower().Equals(price.ToLower()) &&
            a.Weight.ToLower().Equals(weight.ToLower()) &&
            a.Dimensions.ToLower().Equals(dimensions.ToLower()) &&
            a.Trend.ToLower().Equals(trend.ToLower()));
        if (result.Any())
        {
            return result;
        }

        if (!string.IsNullOrWhiteSpace(price))
        {
            result = dbContext.Products.Where(a =>
                a.Price.ToLower().Equals(price.ToLower()) &&
                a.Weight.ToLower().Equals(weight.ToLower()) &&
                a.Trend.ToLower().Equals(trend.ToLower()));
            if (result.Any())
            {
                return result;
            }
        }

        if (!string.IsNullOrEmpty(dimensions))
        {
            result = dbContext.Products.Where(a =>
                a.Dimensions.ToLower().Equals(dimensions.ToLower()) &&
                a.Weight.ToLower().Equals(weight.ToLower()) &&
                a.Trend.ToLower().Equals(trend.ToLower()));
            if (result.Any())
            {
                return result;
            }
        }

        return result;
    }

Here is an approach to get rid of some of the redundancy:

First write an extension Method

static IQueryable<Product> FilterForParameters
    (this IQueryable<Product> query, string price, string weight, string dimensions, string trend)
{
    if (price != null)
        query = query.Where(p => p.Price.ToLower().Equals(price.ToLower()));

    if (weight != null)
        query = query.Where(p => p.Weight.ToLower().Equals(weight.ToLower()));

    if (dimensions != null)
        query = query.Where(p => p.Dimension.ToLower().Equals(dimensions.ToLower()));

    if (trend != null)
        query = query.Where(p => p.Trend.ToLower().Equals(trend.ToLower()));

    return query;
}

Then change your method

public IQueryable<Product> GetProductsByPara(string price, string weight, string dimensions, string trend)
{
    var products = dbContext.Products;

    var productsFilterdByAll = products.FilterForParameters(price, weight, dimensions, trend);
    if (productsFilterdByAll.Any())
        return productsFilterdByAll;

    var productsFilterdByAllButDimnensions = products.FilterForParameters(price, weight, null, trend);
    if (productsFilterdByAllButDimnensions.Any())
        return productsFilterdByAllButDimnensions;

    var productsFilterdByAllButPrice = products.FilterForParameters(null, weight, dimensions, trend);
    return productsFilterdByAllButPrice;
}

Depending on the context, it would also be a good idea to check your parameters for null at the beginning of the method.


Arduino driver for 7 segment display with bit-shift register This code counts down from 30 to -999 on a 7-segment 4-digit display; it works but I don't know if it is a good way to do it or if it follows best practices.  I want to start writing libraries for components that are not specifically made for Arduino.  This is my first attempt.  Please tell me if there are any problems with it or if there is some way for me to improve it.  I am working on also making it able to show decimals but am not done yet.

//This is code to put out any signed integer to a seven segment display using a bit shift register.

int number[12]   = {80,95,50,22,29,148,144,94,16,28,191,16};  //Values 0 through 9 on a 7-segment LED display, negative symbol is 10 and subtract 11 from any number to put a decimal
int shiftPins[3] = {7,8,9};  // (clock pin, data pin, latch pin)
int digitPins[4] = {2,3,4,6}; // (ones, tens, hundreds, thousands)
int show[4]; // (thousands, hundreds, tens, ones)

void setup(){
  pinMode(digitPins[0], OUTPUT);
  pinMode(digitPins[1], OUTPUT);
  pinMode(digitPins[2], OUTPUT);
  pinMode(digitPins[3], OUTPUT);

  pinMode(shiftPins[0], OUTPUT);
  pinMode(shiftPins[1], OUTPUT);
  pinMode(shiftPins[2], OUTPUT);
}

void loop(){

  print2LEDs(30-millis()/1000);

}

void print2LEDs(int number2show){

  if(number2show >= 0){

    show[0] = ((int)number2show/1000);
    show[1] = ((int)number2show%1000)/100;
    show[3] = (((int)number2show%1000)%100)/10;
    show[2] = ((((int)number2show%1000)%100)%10);
    }
  else {
    number2show = -(number2show);
    show[0] = 10;
    show[1] = ((int)number2show%1000)/100;
    show[3] = (((int)number2show%1000)%100)/10;
    show[2] = ((((int)number2show%1000)%100)%10);
  }
  showDigit(show[0],digitPins[0]);
  showDigit(show[1],digitPins[1]);
  showDigit(show[2],digitPins[2]);
  showDigit(show[3],digitPins[3]);
}

void showDigit(int i, int f){

  digitalWrite(shiftPins[2], 0);
  digitalWrite(f, 1);

  shiftOut(shiftPins[1], shiftPins[0], LSBFIRST, number[i]);

  digitalWrite(f, 0);
  digitalWrite(shiftPins[2], 1);
}

What's really in show[2]?`

According to your comment:

int show[4]; // (thousands, hundreds, tens, ones)

the digits in show are in descending order.  But according to your code:

show[0] = ((int)number2show/1000);
show[1] = ((int)number2show%1000)/100;
show[3] = (((int)number2show%1000)%100)/10;
show[2] = ((((int)number2show%1000)%100)%10);

you swapped show[2] and show[3].  Was this a copy paste error?

Simplification

Your code for setting the show array can be simplified.

There's no need for an int cast because everything is already an int.
(((n % 1000) % 100) % 10) is equivalent to n % 10
Three of the four lines in the if and else case are common.

I would rewrite it like this:

    if (number2show >= 0) {
        show[0] = (number2show / 1000) % 10;
    } else {
        show[0] = MINUS_SIGN;
        number2show = -number2show;
    }
    show[1] = (number2show / 100) % 10;
    show[2] = (number2show /  10) % 10;
    show[3] = number2show % 10;
    for (int i=0; i < 4; i++)
        showDigit(show[i], digitPins[i]);

Variable scopes

I'm not sure if you use show anywhere else, but if you don't then you should make it be a local variable instead of a global.  Also, your other global arrays can be marked const since they are read-only.


Extracting the IP addresses of Docker containers using JSON API The following piece of code is imported in another file to get the status of the docker containers. I am printing the required information in a proper tabular form. I am using next() for finding out the only key available in the dictionary. This particular key will change hence used next() to find out the key. But the next() raises an exception when it reaches to the end. Currently I a handling that using PASS.

My question is "Is there a better to way to handle the exception caused by next () : StopIteration" ?

import requests
from prettytable import PrettyTable

def container_status(status):

    url = None
    if status == "all": url = "http://127.0.0.1:6000/containers/json?all=1"
    elif status == "running" : url = "http://127.0.0.1:6000/containers/json?all"
    else: raise ValueError("status should be either 'all' or 'running'")

    return requests.get(url)

def active_containers(status):

    response = container_status(status)
    table = PrettyTable(["Container Name", "Container ID", "Status", "IP ADDR"])
    for i in response.json():
        try:
            table.add_row([i["Names"][0].encode('utf-8').replace('/', ''),
                          i['Id'].encode('utf-8')[:12],
                          i["State"],
                          i["NetworkSettings"]["Networks"][next(iter(i["NetworkSettings"]["Networks"]))]["IPAddress"]])

        except StopIteration:
            pass
    print(table)

What are the other possibilities where I can improve the code?

Dictionaries, as any iterable in Python can be unpacked. It means that, if you know the length of said iterable, you can use as many variables to hold each item of the iterable independently.

For instance:

>>> l = [1, 2, 3]
>>> a, b, c = l
>>> a
1
>>> b
2
>>> c
3
>>> d = {"one": 1, "two": 2}
>>> a, b = d
>>> a
"two"
>>> b
"one"

(order may vary for the dictionary)

If the number of elements in the iterable and the number of variables mismatch, you get a ValueError:

>>> d = {"one": 1}
>>> a, b = d
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: not enough values to unpack (expected 2, got 1)

So, if you know that a dictionary has a single key, you can

network_settings, = i["NetworkSettings"]["Networks"]

and get it that way. If it appears that the dictionary may not contain that key, you have no other choice but to use a try: ... except ValueError: pass.

However, you only care about the values of this dictionary. So I wouldn't use this directly on the dictionary but on its .values() (.itervalues() in Python 2). So the loop would look like:

for container in response.json():
    network_settings, = container['NetworkSettings']['Networks'].values()
    table.add_row(
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])

or

for container in response.json():
    try:
        network_settings, = container['NetworkSettings']['Networks'].values()
    except ValueError:
        continue
    table.add_row(
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])

In case of possible missing keys.

Now, for the rest of the code. I wouldn't use any of the prettytable stuff within active_containers. It may be interesting, for reusability, to only extract relevant informations in this function, and let the caller format it.

I would also change the meaning of the container_status function a bit. Looking at the URLs, it seems to me that the status parameter is only there to filter out inactive containers. So it should be more obvious using a binary choice:

def container_status(include_stopped=False):
    url = 'http://127.0.0.1:6000/containers/json?all'
    if include_stopped:
        url += '=1'
    return requests.get(url)

You get to choose the default based on your most common needs.

Full code would look like:

import requests
from prettytable import PrettyTable

def container_status(include_stopped=False):
    url = 'http://127.0.0.1:6000/containers/json?all'
    if include_stopped:
        url += '=1'
    return requests.get(url)

def active_containers():
    for container in container_status().json():
        network_settings, = container['NetworkSettings']['Networks'].values()
        yield (
            container['Names'][0].encode('utf-8').replace('/', ''),
            container['Id'].encode('utf-8')[:12],
            container['State'],
            network_settings['IPAddress'])

def main():
    table = PrettyTable(['Container Name', 'Container ID', 'Status', 'IP ADDR'])
    for container_infos in active_containers():
        table.add_row(*container_infos)
    print(table)

if __name__ == '__main__':
    main()

(Just assuming that active_containers here means you only want running ones, main and active_containers may need an include_stopped parameter otherwise.)


Maximum product of a triplet in array in linear time Given an array of integers, find the highest product you can get from three of the integers.

  Input:  [1, -4, 3, -6, 7, 0]

  Output: 168

I would love to hear from how this might be refactored/improved.

GitHub

public static int highestProduct(int[] numbers) {
    int largest, secondLargest, thirdLargest;
    int smallest, secondSmallest;

    largest = secondLargest = thirdLargest = Integer.MIN_VALUE;
    smallest = secondSmallest = Integer.MAX_VALUE;

    for (int number : numbers) {

        // logic to find the largest numbers
        if (number > largest) {
            thirdLargest = secondLargest;
            secondLargest = largest;
            largest = number;
        } else if (number > secondLargest) {
            thirdLargest = secondLargest;
            secondLargest = number;
        } else if (number > thirdLargest) {
            thirdLargest = number;
        }

        // logic to find the smallest numbers
        if (number < smallest) {
            secondSmallest = smallest;
            smallest = number;
        } else if (number < secondSmallest) {
            secondSmallest = number;
        }
    }

    return Math.max(largest * secondLargest * thirdLargest,
            largest * smallest * secondSmallest);
}

   @Rule
public ExpectedException thrown = ExpectedException.none();

@Test
public void invalidInput() {
    thrown.expect(InvalidArgumentException.class);
    HighestProductOfThree.highestProduct(new int[]{1, 2});
}

@Test
public void allPositiveNumbers() {
    Assert.assertEquals(30000, HighestProductOfThree.highestProduct(new int[]{1, 3, 2, 100, 100}));
    Assert.assertEquals(40, HighestProductOfThree.highestProduct(new int[]{1, 2, 4, 5}));
}

@Test
public void allNegativeNumbers() {
    Assert.assertEquals(-30, HighestProductOfThree.highestProduct(new int[]{-20, -10, -5, -3, -2}));
}

@Test
public void mixedNumbers() {
    Assert.assertEquals(300, HighestProductOfThree.highestProduct(new int[]{-10, -10, 1, 3, 2}));
    Assert.assertEquals(168, HighestProductOfThree.highestProduct(new int[]{1, -4, 3, -6, 7, 0}));
}

Your code is straightforward and easy to read. It also doesn't do anything unnecessary. You chose the variable names perfectly, got the indentation correct and provided good test cases. Since you mentioned in the title that there code needs to run in linear time, it is perfect.

I don't see where you throw the exception to make the first of your tests succeed. I bet that test fails currently. That test also imports InvalidArgumentException from a strange package instead of using the standard IllegalArgumentException. Are you coming from a .NET background?

Your code is rather long. If you prefer maintainability and readability over a few nanoseconds of execution time (complexity \$\mathcal O(n\log n)\$), you should rather write:

int[] nums = numbers.clone();
Arrays.sort(nums);

int n = nums.length;
return Math.max(
    nums[n - 3] * nums[n - 2] * nums[n - 1],
    nums[0] * nums[1] * nums[n - 1]);


Finding the sum of all the multiples of 3 or 5 below 1000 As a Lisp (and functional programming) newbie, I wrote the following code that finds the sum of all the multiples of 3 or 5 below 1000, and I suspect that it is lousy:

(defun multiple-of-3-or-5 (x)
    (or (eq (mod x 3) 0) (eq (mod x 5) 0)) )

(defun generate-list (min max)
    (cond ((> min max) nil)
          (t (cons min (generate-list (+ min 1) max )))))

(defun my-sum (my-list)
    (cond ((atom my-list) 0)
          (t (+ (cond ((multiple-of-3-or-5 (car my-list)) (car my-list))
              (t 0))
            (my-sum (cdr my-list))))))

Any general advice is most welcome. Specifically, I have these questions (other than what you could say):

I had to extend the stack multiple times (using LispWorks) to get a result. Is my use of recursion wrong?
Should I prefer iterative approach over recursion in Lisp too? (as in imperative languages)
Should I use the loop macro if I am to solve it in iterative fashion?

You are not using tail recursion in my-sum, so the compiler cannot easily turn it into iteration.
It depends on the specific problem; in this case I would probably do it iteratively.
Sure, loop is useful and convenient.

Here is my attempt:

(defun multiple-of-3-or-5 (x)
    (or (zerop (mod x 3))
        (zerop (mod x 5))))
(defun my-sum (from to)
  (loop :for i :from from :to to
    :when (multiple-of-3-or-5 i)
    :sum i))


Organizing JavaScript code for a website I'm in the process of building a website for the company I work for and I'm trying to think of the best way to namespace and organize the site's JavaScript code.

I've written several jQuery/JavaScript plugins for the big chunks of code, but now I'd like a namespaced place to "call" said plugins and any other random JavaScript that's needed throughout the site.

I'm considering using a singleton (NAMESPACE.js):

// http://stackoverflow.com/a/1479341/922323
var NAMESPACE = (function() {

    //var privateVar = '';

    function weather() {

        var $weather = $('#weather');

        if ($weather.length) {

            // Do jQuery stuff with $weather here.

        }

    } // weather()

    function navigation() {

        var $baz = $('#baz'),
        $html = $('html');

        $baz
            .myOtherPlugin({
                cloneRemove : 'li > div',
                cloneId     : false
            })
            .anotherPlugin({
                eventType      : 'hoverIntent',
                onStartOutside : function() {
                    $html.addClass('mw-outside');
                },
                onEndOutside   : function() {
                    $html.removeClass('mw-outside');
                }
            });

    } // navigation()

    function footer() {

        $('footer ul').myPlugin({
            //openTab : true,
            currentPage : true
        });

    } // footer()

    return {

        init : function() {

            weather();

            navigation();

            footer();

            // Add more here as project grows...

        } // init()

    };

})();

HTML would look like so (in foot of document):

<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js"></script>
<script src="plugins.js"></script>
<script src="NAMESPACE.js"></script>

<script type="text/javascript">
    <!--

        $(document).ready(function() {

            NAMESPACE.init();

        });

    //-->
</script>

Question(s):

Do you see anything out of the ordianary with my (pseudo) code above?
Am I headed in the right direction? If not, could you suggest an alternative?
Am I over-complicating things?
I really like how Yahoo! does: YAHOO.namespace("myProject"); YAHOO.myProject.myModule = function () { ... } ... it seems like if I did something similar, that would allow me to easily add/remove "modules" on a "per page" basis. Would anyone recommend this approach? If so, how could I modify my example to mimic YUI's functionality (I'm going to research YUI's namespace() code and post back my results if I figure it out).

One problem in your code is that the "modules" aren't standalone. You'd be adding the "modules" for calling into the init every time a new on is made.

To avoid doing so, each module should register itself for init. You can do this by adding a "register" functionality in your singleton.

(function(exports){

    var registry = []; //collection of module

    //adds module to collection
    exports.register = function(moduleDeclaration){
        registry.push(moduleDeclaration);
    }

    //executes every module
    exports.init = function(){
        var registryLength,i;

        registryLength = registry.length

        //loop through each module and execute
        for(i=0;i<registry.length;i++){
            registry[i].call(this);
        }
    }

//use existing namespace or make a new object of that namespace
}(window.Lib = window.Lib || {}));

//To register a module
Lib.register(function(){
    //do what you have to do here
});

//executing the init
Lib.init();

See? The modules register themselves and don't need to be added manually for init. A developer can create a module on a separate file, add it into the page without editing the main Lib singleton.

Also, you'd have to consider what you'd do if you include a module dynamically after init is called. You should have some internal flag to indicate that init was already called, and that dynamically added module should register and execute immediately.

A vaguely known library called KernelJS already does this and contains many more features like core abstraction, pub-sub inter-module communication, register-remove modules, loose coupling pattern and so on. I suggest you try it or base from it.

As an answer to your add-on question of exposing modules, sure you can. However, according to scalability principles, modules or "widgets" as they call them, should not know each exists. This is to prevent tight-coupling and avoid breakage when, for example, you take out a certain module out.

Also, naming modules isn't good for portability as you are setting a fixed name for a set of code. What if you changed it's name? You'd have to look for all code that depends on that module and change the name.

However, if you want to have named modules and fetching badly, you could expose another function to retrieve a module and return that module if it exists, or undefined or a custom error if it doesn't. Here's modified code for Lib. Since we are using named modules, instead of an array, registry is now an object.

var registry = {}

exports.register = function(name,declaration){
    registry[name] = declaration;
}

exports.init = function(){
    var module,i;

    registryLength = registry.length

    //loop through each module and execute
    for(name in registry){
        if(registry.hasOwnProperty(name)){
            registry[name].call(this);
        }
    }
}

exports.getModule = function(moduleName){

    //default to undefined
    var module;

    //if module exists, assign
    if(registry.hasOwnProperty('moduleName')){
        module = registry[moduleName];
    }

    return module
}

//to register a module
Lib.register('moduleName',function(){
    //module code
});

Additionally, you can call each module's init from Lib's init. Just make sure each module exposes an init function.


Join a JSON object of arrays and reduce down to one key Given that I have a dataset as follows:

{ 1: ['Extremely Tall'], 2: ['Tall'], 3: ['Medium', 'Low'], 4: ['Dwarf'] }

and I receive a value (e.g. Tall or Medium) I want to determine the associated key. By the way feel free to change the dataset, i.e. it doesn't have to be a json object... so long as:

Medium -> 3
Low -> 3
Tall -> 2

My solution is below, anyone have a better way of doing it?

const R = require('ramda')
const heightsMap = { 1: ['Extremely Tall'], 2: ['Tall'], 3: ['Medium', 'Low'], 4: ['Dwarf'] }
const height = 'Medium'

const associatedNumber = R.keys(heightsMap).filter(key => heightsMap[key].includes(height))[0]

console.log(associatedNumber)

Review

I'm not a big fan of importing other libraries when a built-in alternative is available.

const R = require('ramda')

R.keys could be replaced by Object.keys:

const associatedNumber = Object.keys(heightsMap)
  .filter(key => heightsMap[key]
  .includes(height))[0]

Two improvements can be made. First, you're using filter which iterates all elements. This is a pity, since you're only interested in the first item [0]. Find is the method you were looking for. Second, You're traversing keys to perform another lookup heightsMap[key]. This could be done in a single iteration using Object.entries.

Proposed Rewrite

const associatedNumber = Object.entries(heightsMap)
  .find(([key, value]) => value.includes(height))[0];

console.log(associatedNumber);

Note that [0] in the refactored code gets the key of [key, value].


Find value from other dataset based on 6 variables that must be equal in both datasets I have two datasets that are both within a worksheet, call them Data and IBES. The code checks whether the 6 variables are the same in each dataset and then writes the value from a specific column to the other dataset. To find this value the code runs through 288503 lines which is dramatically slow.

How can I speed up this code?

Public Function GetRightValue()

Dim i As Integer
Dim j As Long

Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False
Application.DisplayStatusBar = False
Application.EnableEvents = False
ActiveSheet.DisplayPageBreaks = False

For i = 2 To 1511           'Loop over all values from total dataset
        For j = 2 To 288503      'Loop over all values from IBES file
            If Worksheets("Data").Cells(i, 3) = Worksheets("IBES").Cells(j, 1) Then
                If Worksheets("Data").Cells(i, 7) = Worksheets("IBES").Cells(j, 6) Then
                    If Worksheets("Data").Cells(i, 10) = Worksheets("IBES").Cells(j, 9) Then
                        If Worksheets("Data").Cells(i, 13) = Worksheets("IBES").Cells(j, 11) Then
                            If Worksheets("Data").Cells(i, 8) = Worksheets("IBES").Cells(j, 7) Then
                                If Worksheets("Data").Cells(i, 14).Text = Worksheets("IBES").Cells(j, 13).Text Then
                                    Worksheets("Data").Cells(i, 12) = Worksheets("IBES").Cells(j, 10).Text
                                    Worksheets("Data").Cells(i, 18) = Worksheets("IBES").Cells(j, 16).Text
                                End If
                            End If
                        End If
                    End If
                End If
            End If
        Next j
Next i

Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
Application.DisplayStatusBar = True
Application.EnableEvents = True
ActiveSheet.DisplayPageBreaks = True

End Function

Speed

When working with data at any kind of scale, the golden rule is Do not interact directly with the worksheet. Accessing worksheet objects has huge computational overhead, and you are accessing it every time you check a cell's contents.

Dirk's answer demonstrates how to put a range of values into an Array. Operating on an Array is typically 10-100x faster than operating on a worksheet. When you're finished, just set (Range) = (Array) to print your data back to your sheet.

Magic Variables

Not the good kind of magic either. A magic variable is any hard-coded value. E.G.

  For i = 2 To 1511           'Loop over all values from total dataset
    For j = 2 To 288503       'Loop over all values from IBES file

2 to 1511, 2 to 288503, Magic Variables. Where have those numbers come from? How do you know they're still up to date? I gather they're the final row(s) in your data series. Why not determine them dynamically?

Dim finalRow as long

finalRow = ws.Cells(Rows.Count, colNum).End(xlUp).Row

For i = 2 to finalRow '/ +1 to avoid Headers

    ...

Same goes for your column numbers. They should be named variables along the lines of condition1ColNum, condition2ColNum etc. Additionally, if you know what their headers are, I would start by iterating over your header row, and dynamically determining which columns they're actually in.

The key to all this is: How easy is it to break your Macro? If the answer is just "Add an extra row" or "Add/move a column" then it's not very good.

Also, you see this: Worksheets("Data").Cells(i, 3). You have that sheetName hardcoded 8 times. What if someone renames the sheet? Do you really want to go through and re-type it every time? Instead, use the VBa object model and create a proper Worksheet object:

Dim wsData as Worksheet, wsIbes as Worksheet

Set wsData = sheets("Data")
Set wsIbes = sheets("IBES")

Then you can use it like so:

wsData.Cells(i, 3)

And should the name change, you only have to change it in one place.

Even better, have you ever heard of CodeNames? In VBA, every worksheet object has a "Name" which is displayed in Excel and the user can rename at will unless it's protected. Each sheet also has a "CodeName", which can only be seen / modified in the VBE Window. Additionally, a sheet's CodeName actsas a Worksheet variable. E.G. If I have a sheet called "Data" which has CodeName "wsData" then I can simply write

wsData.Cells(i, 3)

Without having to declare anything. And now, if the user renames your sheet to "Something Something Data", your code won't break, because the codeName will remain unchanged.

For your main loop, you shouldn't be using nested If statements. It's messy and leads to Arrow Code. I would do it something like so:

For i = 2 To wsDataFinalRow 'Loop over all values from total dataset
    For j = 2 To wsIbesFinalRow 'Loop over all values from IBES file
        boolean1 = (Worksheets("Data").Cells(i, condition1DataColNum) = Worksheets("IBES").Cells(j, condition1IbesColNum))
        boolean2 = (Worksheets("Data").Cells(i, condition2DataColNum) = Worksheets("IBES").Cells(j, condition2IbesColNum))
        boolean3 = ...
        ...
        passedTest = boolean1 And boolean2 And boolean3 And ...

        If passedTest then
            ...
        End If

    next j
next i

And now it's much clearer what's going on, and it's much easier to add/move/re-arrange your test conditions at will, and you can add more actions based on a subset of conditions being true *without* having to write out a whole extra If ... If ... If ... If ... block.


Simple taxi emulator This app is a simple taxi emulator. The only feedback I've got on it was "bad internal architecture", that's why I would be grateful to receive more adequate and humiliating review.

Simply-speaking, it should be a static lib, all the functionality must execute in the same thread (but! thread-safe). There should be a taxi station with cash desk, drivers and cabs. Each driver has its own car and own cash desk. One can re-assign the car to another driver. Car breaks after each 50 000 km.  The DoMainProcessing function should wake every 2 seconds from outside, check if driver already took passenger to his place, fill in cash desk and possibly fix cars.

Full description

The weakest place in my code, I think, is DoMainProcessing. It just goes through all drivers and cars checking their condition. But I just had no better idea to do this in one thread.

Main class header:

#pragma once
#include "CashBox.h"
#include "Driver.h"
#include "Car.h"
#include <unordered_map>
#include <unordered_set>
#include <memory>

static const std::string DRIVER_CAR_LOCK = "DriverCar";

static const int NO_ERR = 0;
static const int CAR_IS_BUSY = 1;
static const int DRIVER_IS_BUSY = 2;
static const int NO_DRIVER_FOUND = 3;
static const int CAR_IS_BROKEN = 4;

class TaxiPark
{
public:
    //Basic function. Checks if which trips are finished
    // and repairs broken cars
    void DoMainProcessing();

    ~TaxiPark(void){};
    TaxiPark(void){};

    //Cars and drivers management
    int AddDriver(DWORD ID, std::shared_ptr<Car> car);
    int AddCar(DWORD ID);
    int ChangeCar(DWORD driverID, std::shared_ptr<Car> newCar);

    //Get driver/car info by ID
    const std::shared_ptr<Driver> GetDriverById(DWORD ID);
    const std::shared_ptr<Car> GetCarById(DWORD ID);

    //Get all ID's
    const std::vector<DWORD> GetAllCarID();
    const std::vector<DWORD> GetAllDriverID();

    //Try to set driver status as sick/on vacation if driver is not on trip
    int SetDriverStatus(DWORD driverID, bool onVacation, bool isSick);

    //Try to start trip for chosen passenger with chosen driver if it possible
    //(driver isn't already busy/on vacation/sick, car isn't broken)
    int TakeClients(Passenger client, DWORD driverID, DWORD distance, DWORD speed );

private:
    CashBox cashBox;

    std::unordered_map<DWORD, std::shared_ptr<Driver>> Drivers;
    std::unordered_map<DWORD, std::shared_ptr<Car>> Cars;
};

DoMainProcessing function:

void TaxiPark::DoMainProcessing()
{
    auto mut = CreateMutex(NULL, FALSE, DRIVER_CAR_LOCK.c_str());

    WaitForSingleObject(mut, INFINITE);
    {
        std::for_each(Drivers.begin(), Drivers.end(), [this] (std::pair<DWORD, std::shared_ptr<Driver>> it)
        {
            std::shared_ptr<Driver> driver(it.second);
            auto driverID = it.first;
            if (driver->IsTripFinished())
            {
                Drivers.erase(driverID);
                driver->FinishTrip();
                Drivers.emplace(std::make_pair<DWORD,std::shared_ptr<Driver>>(driverID, driver));
            }
        });
        std::for_each(Cars.begin(), Cars.end(), [this] (std::pair<DWORD, std::shared_ptr<Car>> it)
        {
            std::shared_ptr<Car> car(it.second);
            auto carID = it.first;
            if (car->IsBroken())
            {
                Cars.erase(carID);
                if (cashBox.GetBalance() >= CAR_REPAIR_COST)
                {
                    cashBox.TakeCash(CAR_REPAIR_COST);
                    car->Repair();
                }
                Cars.emplace(std::make_pair<DWORD,std::shared_ptr<Car>>(carID, car));
            }
        });
    }
}

Here's the full code listing.

P.S.: I know that unordered_map is a bad idea in this case. It gives no advantages in terms of speed, but requires exceed work in need of changing elements.

Use standard #include guards, not the non-standard #pragma once.
Use standard data types, not DWORD.
Change

this:

static const int NO_ERR = 0;
static const int CAR_IS_BUSY = 1;
static const int DRIVER_IS_BUSY = 2;
static const int NO_DRIVER_FOUND = 3;
static const int CAR_IS_BROKEN = 4;

into an enum:

enum CarStatus { NO_ERROR, CAR_IS_BUSY, DRIVER_IS_BUSY, NO_DRIVER_FOUND, CAR_IS_BROKEN };

DoMainProcessing is a horrible name and sounds like a function that does way too much. (Update: Okay, that's part of the exercise. Oh, well.)
Consider typedef std::shared_ptr<Car> CarPtr or something, to make the code less cluttered.
Avoid bool in interfaces, as it leads to poor readability and is error prone.

Instead of:

int SetDriverStatus(DWORD driverID, bool onVacation, bool isSick);

Use an enum indicator (or a string) instead:

enum DriverStatus { /* ... */ };
void SetDriverStatus(DriverID driver, DriverStatus newStatus)
{
    // ...
}

The function should be void. (If you are currently returning the old state: Stop doing that, and have a separate function for that. If you are returning a status number to indicate success or not: Use exceptions instead.)

You should probably use the C++ threading library instead of your CreateMutex. (Update: Again, your exercise hinders you from doing the right thing.)
Your use of for_each should be substituted with a range-based for loop.
The exercise states:

  There must be some function, we call it DoMainProcessing, that periodically
  gets called by an external source using the emulator (for example, every second or two).
  It should detect when the driver made the carriage of the customer (depending
  distance and average speed) and is ready to perform the next one. Then, it should
  repair the broken car (subject of availability of sufficient
  number of funds).

The way I understand this, you should update the state of each of your cars and drivers in this function. You should probably record the time between each call, and add the distance traveled to a counter.

Very roughly something like this:

void TaxiPark::DoMainProcessing()
{
    // Use a real library for time, I'm just illustrating.
    Time now = getTime();
    TimeDiff time_difference = now - previous_processing_time;
    previous_processing_time = now;

    for (auto& driver : Drivers) {
        driver.update(time_difference);
    }

    // Updates for cars ...
}

Then, in Driver::update(), you should increase distance traveled based on speed and time difference. If the distance is greater than or equal to the distance of the current ride, the ride is done. Do other state updates as necessary.


Synonymiser refactoring I am interested in seeing how people would refactor this code to make it more readable, remove local variables, etc. The code checks a string against a number of arrays. The first (0) field of each array is the root word and the rest of the words are its synonyms. The idea is to replace any word in the user-provided string with the root word or provide the user-supplied word if none were found.

import java.util.Arrays;

public class Synonymiser {

    private final static int ROOT_WORD = 0;

    private String[][] mSynonyms;

    public Synonymiser(String[]... synonyms) {
        super();
        this.mSynonyms = synonyms;
    }

    public String normaliseString(String inStr){
        String[] inArr = inStr.split(" ");
        boolean matchFound;

        for (int i = 0; i < inArr.length; i++){
            matchFound=false;
            for (String[] synonymArr : mSynonyms){
                 for(int j = 0 ; j < synonymArr.length; j++){
                     if (inArr[i].equalsIgnoreCase(synonymArr[j])){
                         inArr[i] = synonymArr[ROOT_WORD];
                         matchFound = true;
                     }
                     if (matchFound) break;
                 }
                 if (matchFound)  break;
            }
        }
        return Arrays.toString(inArr);
    }

    public static void main(String [] args){
        String[] ar1 = new String[] {"one", "two", "three", "four", "five"};
        String[] ar2 = new String[] {"six", "seven", "eight"};
        Synonymiser s = new Synonymiser(ar1, ar2);

        System.out.println(s.normaliseString("one two seven bulb"));
    }

}

I would like to go beyond just making the code prettier, and change the method that you are using. Instead of looping through the arrays looking for matches, you can put the synonyms in a hashed list. That will give you a performance close to O(1) for each lookup, instead of O(n).

Example in C#:

public class Synonymiser {

  private Dictionary<string, string> _synonyms = new Dictionary<string, string>();

  public Synonymiser AddWord(string word, params string[] synonyms) {
    foreach (string synonym in synonyms) {
      _synonyms.Add(synonym.ToUpper(), word);
    }
    return this;
  }

  public string NormaliseString(string inStr){
    string replacement;
    return String.Join(
      " ",
      inStr.Split(' ')
      .Select(w => _synonyms.TryGetValue(w.ToUpper(), out replacement) ? replacement : w)
    );
  }

}

Usage:

Synonymiser s =
  new Synonymiser()
  .AddWord("one", "two", "three", "four", "five")
  .AddWord("six", "seven", "eight");

Console.WriteLine(s.NormaliseString("one two seven bulb"));

Output:

one one six bulb


Ways to express proc inside a method I'm interested in seeing different ways the proc can be expressed inside a method that takes args.

This method is meant to be a sort of FizzBuzz that will dynamically replace "Fizz" or "Buzz" as well as the traditional divisors 3 & 5.

def fizz_buzz_custom(string_1="Fizz", string_2="Buzz", num_1=3, num_2=5)
  proc do |length|
    index = length + 1
    val = if (index % 3 == 0 && index & 5 == 0)
      (string_1+string_2)
    elsif index % 3 == 0
      string_1
    elsif index % 5 == 0
      string_2
    else
      index
    end
  end
end

Here are some examples with test output:

fizz_buzz_custom[15]                         # returns 16
fizz_buzz_custom[44]                         # returns "FizzBuzz" (45 is divisible by 3 and 5)
fizz_buzz_custom('Hey', 'There')[25]         # returns 26
fizz_buzz_custom('Hey', 'There')[11]         # returns "Hey" (12 is divisible by 3)
fizz_buzz_custom("What's ", "up?", 3, 7)[80] # returns "What's " (81 is divisible by 3)

To answer your question directly, you can express a proc/block/lambda as

Proc.new do |...|
# or
lambda do |...|
# or
-> (...) do

Either syntax (and your current one) should work fine for this. To invoke the block, you can use square brackets like now, or use call.

Really, that's about it, unless I'm misinterpreting your question.

However, I see some other issues with your code:

Use a hash for the arguments. Please. The current construction is very un-Ruby-like. A hash will also "force" you to write the method (or block, in this case) in a more generic manner, since a hash can have any number of key/value pairs.
Why length? It's just a number. Whose to say it's the length of anything?
Also, why add 1 to length? Just wondering. If anything, going from an array's length to an index usually involves subtracting 1.

Something like this seems cleaner (though it's as much about FizzBuzz as it is about procs):

def custom_fizz_buzz(divisors = {"Fizz" => 3, "Buzz" => 5})
  divisors = divisors.sort_by { |_, value| value }
  -> (number) do
    output = divisors.map { |key, value| key if (number % value).zero? }.compact.join
    output.empty? ? number : output
  end
end

which'll give you

custom_fizz_buzz[7]                              # => 7
custom_fizz_buzz.call(9)                         # => "Fizz"
custom_fizz_buzz[15]                             # => "FizzBuzz"
custom_fizz_buzz("What's " => 3, "up?" => 7)[21] # => "What's up?"

or even

1.upto(100).map(&custom_fizz_buzz) # => [1, 2, "Fizz", 4, "Buzz", .... etc.]


Adding augmented maps to a vector based on regex text grabbing This is my latest brain child for clojure, could it be done with less code, cleaner, or in a more re applicable way? I am at a pretty early stage in my clojure learning so an in depth explanation of how and why my code is faulty would help me get a handle on what I am doing wrong so I do not build bad habits.

(def next-apendage
  #(hash-map :name (clojure.string/replace (:name %1) #"[0-9]" (str (+ 1 %2 (read-string (re-find #"[0-9]" (:name %1))))))))

(defn add-apendages
  [times part]
  (loop [done 0
         final-apendages [part]]
    (if (= done times)
      final-apendages
      (recur
       (inc done)
       (conj final-apendages
             (next-apendage part done))))))

(add-apendages 2 {:name "tentacle-3"})
;;OUTPUT->[{:name "tentacle-3"} {:name "tentacle-4"} {:name "tentacle-5"}]

That really ugly looking first function takes two arguments a map and a number. The number is added (with a compensation for done starting at 0) to the :name string of the map. The second function condenses the new appendages along side the original into a list. The third section is a call to the second function.

Some things that stand out from your code:

(def next-appendage #()) is really (defn next-appendage [part done])
Do not use anonymous functions except for really really short functions. You next-appendage function is long for Clojure standards.
Instead of hash-map you can use the map literal directly `{:name (clojure.string/replace ,,,}
Even though it seems cool when you are starting Clojure, one liners are very hard to read. You already know that is "really ugly" :)
It is unusual to use loop. Most of the time you can replace it with a simpler map or similar
Do not use read-string, unless you know what you are doing

My version would look like:

(defn add-apendages [times part]
  (let [ ;; find the prefix and initial number
        [_ prefix number-str] (first (re-seq #"(.*)-([0-9]+)$" (:name part)))
         ;; parse the number, using Java interop
        initial-number (Integer/parseInt number-str)]
    ;; Build the list of results using a simple map operation
    (map (fn [n] {:name (str prefix "-" n)})
         (range initial-number (+ 1 initial-number times)))))


Bot-like Spring Java service for checking advertisements I am in the middle of developing a Spring Boot application, which has a service acting like a bot.

advertisementService.update(Advertisement ad) is a method which parses some HTML. It is important that it cannot execute periodically. For now it is pseudo random, but in the future there will be some logic returning the time depending on information read in previous parsing.

Immediately after the start of the app, it invokes the method scheduleAllChecks().
If a new Advertisement comes up, then it calls scheduleCheck(0, ad);

I am wondering if the solution (I mean the whole service class) is proper and if it can be beter?

package com.gadomski.service;

import com.gadomski.model.Advertisement;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.Random;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

/**
 * Created by Krzysiek on 21.08.2016.
 */

@Service
public class CheckerService {
    private final Logger log = LoggerFactory.getLogger(this.getClass());
    private final int CHECKER_THREADS = 100;
    @Autowired
    AdvertisementService advertisementService;
    private ScheduledExecutorService scheduledExecutorService;

    CheckerService() {
        this.scheduledExecutorService = Executors.newScheduledThreadPool(CHECKER_THREADS);
    }

    public void scheduleCheck(int delaySeconds, Advertisement advertisement) {
        if (advertisement.isActive()) {
            ScheduledFuture<?> check = this.scheduledExecutorService.schedule(
                    () -> {
                        scheduleCheck(getDelayInSeconds(), advertisement);
                        advertisementService.updateAdvertisement(advertisement);
                        log.info("Scheaduled next check for " + advertisement.getUrl() + " for user with id: " + advertisement.getUserId());
                    },
                    delaySeconds, TimeUnit.SECONDS);
        }
    }

    private int getDelayInSeconds() {
        Random rand = new Random();
        return (8 + rand.nextInt(10)) * 60 + rand.nextInt(60);
    }

    public void scheduleAllChecks() {
        Random random = new Random();
        advertisementService.getAllActvieAdvertisements().forEach(a -> scheduleCheck(random.nextInt(3 * 60), a));
    }
}

It appears that you are using Random across multiple threads. I guess you plan to replace it in the future, but still, according to the documentation:

  Instances of java.util.Random are threadsafe. However, the concurrent use of the same java.util.Random instance across threads may encounter contention and consequent poor performance. Consider instead using ThreadLocalRandom in multithreaded designs.

You should probably rewrite this to use java.util.concurrent.ThreadLocalRandom. As the docs say:

  When applicable, use of ThreadLocalRandom rather than shared Random objects in concurrent programs will typically encounter much less overhead and contention.


Easier ways to make an RPG dice roller code? I'm new to programming and this is the first thing I'm doing on my own. I would appreciate it if you could point me ways to optimize this RPG dice roller code in Python!
import random

def dice_reader():

##this should read messages like '2 d 6 + 3, 5 d 20 + 2 or just 3 d 8'##
    message = input('>')
    reader = message.split(' ')
    times = reader[0]
    sides = reader[2]
    output_with_modifier = []
    result = []

    if len(reader) == 5:
        modifier = reader[4]
    else:
        modifier = 0

    for output in range(int(times)):
        output = random.randint(1, int(sides))
        result.append(output)
        output_with_modifier = [(int(x) + int(modifier)) for x in result]
    print(f' Dice rolls: {tuple(result)}')
    if modifier != 0:
        print(f' With modifier: {tuple(output_with_modifier)}')

    end = False
    while end == False:
        dice_reader()
        end_message = input('Again? ')
        if end_message.lower() == 'no':
            end = True
    else:
        pass

Mystery inputs
It's clear that there's an implied structure to this input:
message = input('>')
reader = message.split(' ')

requiring between four and five tokens. I have no idea what those are, and neither does your user. Replace '>' with an actual description of what's expected here.
Unpacking
times = reader[0]
sides = reader[2]

can be
times, _, sides = reader[:3]

though the discarded second item is suspicious. You do need to show what this is, and it probably shouldn't be there.
Looping and overwrites
This:
for output in range(int(times)):
    output_with_modifier = [(int(x) + int(modifier)) for x in result]

does not make sense. If you ask for 100 times, output_with_modifier will be calculated 100 times and thrown away 99 of them. Only the last value will be kept. You probably want to de-indent that last assignment so that it happens outside of the loop.
More looping
end = False
while end == False:
    dice_reader()
    end_message = input('Again? ')
    if end_message.lower() == 'no':
        end = True
else:
    pass

First, delete that else; pass - it isn't doing anything. Also, end == False should be not end; but you shouldn't be using a termination variable at all. If you find a no, simply break.
Suggested code
Some of this may challenge a beginner, but I like to think that CodeReview is for "aspiring advanced programmers". I've tried to comment it extensively, but feel free to ask questions in the comments.
import re
from random import randint
from re import Pattern
from typing import ClassVar, Iterable

class Dice:
    """
    One specification for dice rolls in Dungeons & Dragons-like format.
    """

    def __init__(self, times: int, sides: int, modifier: int = 0):
        if times < 1:
            raise ValueError(f'times={times} is not a positive integer')
        if sides < 1:
            raise ValueError(f'sides={sides} is not a positive integer')

        self.times, self.sides, self.modifier = times, sides, modifier

    # This is a class variable (basically a "static") that only has one copy
    # for the entire class type, rather than a copy for every class instance
    # It is a regular expression pattern that will allow us to parse user
    # input.
    INPUT_PAT: ClassVar[Pattern] = re.compile(
        # From the start, maybe some whitespace, then a group named "times"
        # that contains one or more digits
        r'^\s*(?P<times>\d+)'

        # Maybe some whitespace, then the letter "d"
        r'\s*d'

        # Maybe some whitespace, then a group named "sides" that contains one
        # or more digits
        r'\s*(?P<sides>\d+)'

        # The beginning of a group that we do not store.
        r'(?:'

            # Maybe some whitespace, then a "+" character
            r'\s*\+'

            # Maybe some whitespace, then a group named "modifier" that
            # contains one or more digits
            r'\s*(?P<modifier>\d+)'

        # End of the group that we do not store; mark it optional
        r')?'

        # Maybe some whitespace, then the end.
        r'\s*$',

        # We might use "d" or "D"
        re.IGNORECASE
    )

    # This can only be called on the class type, not a class instance. It
    # returns a new class instance, so it acts as a secondary constructor.
    @classmethod
    def parse(cls, message: str) -> 'Rolls':
        match = cls.INPUT_PAT.match(message)
        if match is None:
            raise ValueError(f'Invalid dice specification string "{message}"')

        # Make a new instance of this class based on the matched regular
        # expression.
        return cls(
            int(match['times']),
            int(match['sides']),
            # If there was no modifier specified, pass 0.
            0 if match['modifier'] is None else int(match['modifier']),
        )

    @classmethod
    def from_stdin(cls) -> 'Rolls':
        """
        Parse and return a new Rolls instance from stdin.
        """

        while True:
            try:
                message = input(
                    'Enter your dice specification, of the form\n'
                    '<times>d<sides> [+ modifier], e.g. 3d6 or 4d12 + 1:\n'
                )
                return cls.parse(message)
            except ValueError as v:
                print(v)
                print('Please try again.')

    def roll(self, with_modifier: bool = False) -> Iterable[int]:
        """
        Return a generator of rolls. This is "lazy" and will only execute the
        rolls that are consumed by the caller, because it returns a generator
        (not a list or a tuple).
        """
        mod = self.modifier if with_modifier else 0
        return (
            randint(1, self.sides) + mod
            for _ in range(self.times)
        )

    def print_roll(self):
        print(
            'Dice rolls:',
            ', '.join(str(x) for x in self.roll()),
        )

        if self.modifier != 0:
            print(
                'With modifier:',
                ', '.join(str(x) for x in self.roll(with_modifier=True)),
            )

def test():
    """
    This is an automated test method that does some sanity checks on the Dice
    implementation.
    """

    d = Dice.parse('3 d 6')
    assert d.times == 3
    assert d.sides == 6
    assert d.modifier == 0

    d = Dice.parse('3D6 + 2')
    assert d.times == 3
    assert d.sides == 6
    assert d.modifier == 2

    try:
        Dice.parse('nonsense')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == 'Invalid dice specification string "nonsense"'

    try:
        Dice.parse('-2d5')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == 'Invalid dice specification string "-2d5"'

    try:
        Dice.parse('0d6')
        raise AssertionError()
    except ValueError as v:
        assert str(v) == "times=0 is not a positive integer"

    d = Dice.parse('100 d 12+3')
    n = 0
    for x in d.roll(True):
        assert 4 <= x <= 15
        n += 1
    assert n == 100

def main():
    test()

    dice = Dice.from_stdin()
    dice.print_roll()

if __name__ == '__main__':
    main()


Script which migrates files to secondary storage and symlinks them I wrote this code to clean up some of the space on our file server. We've got 15 years of legacy data that nobody accesses or changes or cares about that we have to keep regardless. I'd rather have it not sitting on our main file server so that I don't have to add an additional TB to it annually. This script goes through the file structure, finds files that haven't been touched in 4 years and copies them to slower storage, then replaces the source file with a symlink.

I'm new to PowerShell, so any pointers regarding style or a better way to do things would be nice. I know that my way of bracing things is wrong and that closing curly braces should be indented one less level, but I don't care and find this easier to deal with.

param(
    [string]$Dir = "",
    [string]$ArchiveDrive = ""
    )

if ($ArchiveDrive -eq ""){
    $hostname = hostname
    $ArchiveDrive = "\\Archives\"+$hostname+"\"+$Dir[0]+"\"
    }

import-module PSCX
import-module new-symlink

$FileList = @()

$SourceDrive = $dir[0] + ":\"

$date = Get-Date -Format yyyy-MM-dd
$ErrLog = "C:\ErrorLog $date.txt"
$DelLog = "C:\DelLog $date.txt"
$PathWarning = "C:\_PROBLEMS DETECTED.txt"

function BuildLists($dir){
    $FileList = @()
    $DirList = (dir $dir -recurse)
    foreach ($item in $DirList){
        if ( ((get-date).Subtract($item.LastWriteTime).Days-gt 1460) -eq $True) {

            $FileList += $item
            }
        #else {write-host "$item is modified recently"}
        }
    return $FileList
    }

function CheckPathLength($file){
    if ($File.FullName.Length -ge 220){
        copy $PathWarning $File.DirectoryName}
    }

function ArchiveFile($SourceFile){
    $DestFile = ($SourceFile.fullname.replace($SourceDrive, $ArchiveDrive))
    $DestDir = ($SourceFile.DirectoryName.replace($SourceDrive, $ArchiveDrive))
    mkdir -Path $DestDir 2>$ErrLog
    copy $SourceFile.FullName $DestFile
    }

function HashCheckFile($SourceFile){
    $DestFile = $SourceFile.FullName.replace($SourceDrive, $ArchiveDrive)
    $SourceHash = get-hash($SourceFile.fullname)
    $DestHash = get-hash("$DestFile")
    return $SourceHash.HashString -eq $DestHash.HashString
    }

function DeleteFIle($File){
    del $file.fullname
    }

function LinkFile($Sourcefile){
    $SourceFilePath = $Sourcefile.fullname
    $DestFile = ($sourcefilepath.replace($SourceDrive, $ArchiveDrive))
    New-Symlink -path $DestFile $SourceFile.fullname -file 1>$errlog
    }

function CheckPathLength($file){
    if ($File.FullName.Length -ge 220){
        copy $PathWarning $File.DirectoryName}
    }

function ReplicateFile($file){
    if ($file.Attributes -eq "Directory"){continue}
    ArchiveFile($File)
    if (HashCheckFile($File)){
        DeleteFile($File)
        LinkFile($File)
        }
    }

function Archive($FileList){
    foreach ($File in $FileList){
        CheckPathLength($file)
        ReplicateFile($File)
        }
    }

function RunArchiving($dir){
    $FileList = BuildLists($dir)
    Archive($FileList)
    }

function UnArchive(){}

RunArchiving($dir)

Not bad at all. Nice to see functions but perhaps you have too many.

Do you really need this as a function?

function DeleteFIle($File){
    del $file.fullname
}

This function is listed twice but is only called once. It could have probably stayed in the Archive function.

function CheckPathLength($file){
    if ($File.FullName.Length -ge 220){
        copy $PathWarning $File.DirectoryName
    }
}