Finding the Least Common Ancestor of a Binary Tree

Introduction

I noticed that your demo binary tree is not sorted by values, so I assume that we are dealing with the unsorted version of the data structure.

Default values for reference fields

public Treenode(int value) {
    this.value = value;
    this.left = null;
    this.right = null;
}

In Java, the reference fields are initialized with the value null by default. You can simply write

public Treenode(int value) {
    this.value = value;
}

Actual algorithms

Your BFS and DFS seem like overkilling it. See Summa summarum for an alternative implementation.

Static vs. non-static

static boolean v1=false,v2=false;

Not good. Consider creating the two variables in the actual LCA routine.

Whitespace

Once again:

static boolean v1=false,v2=false;

You should have a single space before and after each binary operator:

static boolean v1 = false, v2 = false;

Hide the object

Since SearchTree is useless as an object, consider declaring a private constructor.

Summa summarum

All in all, I had this in mind:

Treenode.java:

public class Treenode {

    private final int value;
    private Treenode right;
    private Treenode left;

    public Treenode(final int value) {
        this.value = value;
    }

    public int getValue() {
        return this.value;
    }

    public Treenode getLeftChild() {
        return this.left;
    }

    public Treenode getRightChild() {
        return this.right;
    }

    public void setLeftChild(final Treenode child) {
        this.left = child;
    }

    public void setRightChild(final Treenode child) {
        this.right = child;
    }

    @Override
    public int hashCode() {
        return this.value;
    }

    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            return false;
        }

        if (getClass() != obj.getClass()) {
            return false;
        }

        final Treenode other = (Treenode) obj;

        return this.value == other.value;
    }

    @Override
    public String toString() {
        return "[" + this.value + "]";
    }
}

SearchTree.java:

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

public class SearchTree {

    private SearchTree() {}

    public static Treenode findLeastCommonAncestor(final Treenode root,
                                                   final Treenode node1,
                                                   final Treenode node2) {
        final List<Treenode> list1 = findPath(root, node1);

        if (list1 == null) {
            return null;
        }

        final List<Treenode> list2 = findPath(root, node2);

        if (list2 == null) {
            return null;
        }

        final Set<Treenode> visitedSet = new HashSet<>(list2);

        for (int i = list1.size() - 1; i >= 0; --i) {
            final Treenode node = list1.get(i);

            if (visitedSet.contains(node)) {
                return node;
            }
        }

        return null;
    }

    private static List<Treenode> findPath(final Treenode root,
                                           final Treenode node) {
        final Map<Treenode, Treenode> parentMap = new HashMap<>();
        dfs(root, null, parentMap);

        if (!parentMap.containsKey(node)) {
            return null;
        }

        final List<Treenode> path = new ArrayList<>();
        Treenode current = node;

        while (current != null) {
            path.add(current);
            current = parentMap.get(current);
        }

        Collections.<Treenode>reverse(path);
        return path;
    }

    private static void dfs(final Treenode currentTreenode,
                            final Treenode parentTreenode,
                            final Map<Treenode, Treenode> parents) {
        parents.put(currentTreenode, parentTreenode);

        if (currentTreenode.getLeftChild() != null) {
            dfs(currentTreenode.getLeftChild(), currentTreenode, parents);
        }

        if (currentTreenode.getRightChild() != null) {
            dfs(currentTreenode.getRightChild(), currentTreenode, parents);
        }
    }

    public static void main(String[] args) {
        /*
              n1                   5
             /  \                 / \
           n2    n3    ---->     2   4
          /  \                  / \
         n4  n5                1   3

        */
        Treenode n1 = new Treenode(5);
        Treenode n2 = new Treenode(2);
        Treenode n3 = new Treenode(4);
        Treenode n4 = new Treenode(1);
        Treenode n5 = new Treenode(3);
        Treenode disconnected = new Treenode(6);

        n1.setLeftChild(n2);
        n1.setRightChild(n3);
        n2.setLeftChild(n4);
        n2.setRightChild(n5);

        System.out.println(findLeastCommonAncestor(n1, n4, n3));
        System.out.println(findLeastCommonAncestor(n1, n2, disconnected));
    }
}

Hope that helps.


Are there any bugs or ways to make my divisor code better?

Here are some thoughts on this code:

Chose descriptive function names

The name programme is not really very descriptive of what the function does.  You might call it something like factor to give the reader of your code a better idea of what it actually does.

Document your code

Python supports the use of multiple different kinds of comments, including ones that can be made into unit tests.  Get in the habit of commenting your code carefully.

Separate I/O from calculation

The main purpose of the program is to factor numbers into divisors which is something that is potentially reusable.  Both to make it more clear as to what the program is doing and to allow for future re-use, I'd suggest extracting the factoring part into a separate function and the have the input from the user be done within the main routine or some other input-only function.

Separate timing from the thing being timed

On my machine, the timer always reports 0.0 seconds, which makes it hard to compare the effects of changes in the code.  For that reason, what's often done is to call the routine to be timed multiple times to get better resolution.  In order to do that, you'd have to remove the timing from the routine you're timing, which is good design practice anyway.  Right now, the print function is part of the code that's being timed.

Think of your user

Right now, the user has to enter "yes" or the equivalent and then enter another number to be factored.  However, the prompt doesn't suggest that "y" is a valid answer.  Adding that to the prompt would help the user understand what the computer is expecting.  Better still, would be t eliminate that question and to simply ask for the next number with a number '0' being the user's way to specify no more numbers.  Of course the prompt should be changed to tell the user of this fact.

Expand the range

Right now, the size of numbers that can be factored is limited to what will fit within an int, but it would be much more interesting to be able to factor really large numbers.

Use divmod() to save some time

Right now, for every successfully discovered divisor, the program performs three division operations:  one for %, and then two more to insert the number into numbers and again to print.  Instead, you could do the division once using the divmod function which gives you both the quotient and remainder at the same time.


This is one of the standard examples for algorithmic complexity.  This algorithm is very easy to understand since it matches the mathematical definition well, but performs miserably since it's trying to calculate a number that grows exponentially by adding 0s and 1s together.

The key observation is that to calculate, say, fibonacci(5), it has to recursively calculate fibonacci(4) and fibonacci(3).  Then to calculate fibonacci(4), it has to calculate fibonacci(3) again, which is completely wasteful.  There are several things we can do to go faster:

Build from the bottom up.  Start two variables a=0 and b=1, then run a loop where you set a=b and b=a+b (careful to not blast values as you're using them).  This is how you would do it by hand.
Use your recursive solution, but cache the results so that it doesn't repeat work.  This is called memoization.
Use the closed-form formula.


Here are some of the things I've noticed:

switching to argparse might make the argument parsing a bit more readable
the read_files() + remove_old_files() functions could make use of glob module with the **+recursive mode:

for filename in glob.iglob('./**/[0-9]+.csv', recursive=True):
    os.remove(filename)

avoid calling "magic" methods like __len__() when not necessary - you can use len() function directly
you can define first_col in one line:

first_col = sys.argv[2] if len(sys.argv) > 2 else None

c and f are not good variable names, think of something more descriptive - input_file and output_file?..
you can use an f-string to define the "filename" for a chunk
move the comments before the functions into proper docstrings

Also, what if you would slice the CSV in an iterative manner, something along these lines (other improvements applied):

import csv
import glob
import os
import sys
from itertools import islice

def remove_old_files():
    """Removing files which match certain pattern."""
    for filename in glob.iglob('./**/[0-9]+.csv', recursive=True):
        os.remove(filename)

def chunks(it, size):
    it = iter(it)
    return iter(lambda: tuple(islice(it, size)), ())

def split(csv_file, number_of_slices, first_column):
    """Split file into number_of_slices pieces."""
    with open(csv_file, 'r') as input_file:
        reader = csv.reader(input_file)

        for num_file, chunk in enumerate(chunks(reader, number_of_slices)):
            with open(f"{num_file}.csv", 'w') as output_file:
                writer = csv.writer(output_file)

                if first_column:
                    for row in chunk:
                        writer.writerow([first_column] + row)
                else:
                    writer.writerows(chunk)

if __name__ == "__main__":
    # TODO: argparse?
    if int(sys.argv[1]) <= 0:
        raise SystemExit("Piece count must be natural number greater than zero.")
    number_of_slices = int(sys.argv[1])
    first_column = sys.argv[2] if len(sys.argv) > 2 else None

    remove_old_files()

    for filename in glob.iglob('./**/*.csv', recursive=True):
        split(filename, number_of_slices, first_column)


RadixSort implementation design and performance

Improvement to radix_sort_32

One of the things that you were wondering about was whether radix_sort_msd or radix_sort_32 was better.  The former called max_element() in order to determine the max width of the values, and the latter always did 4 passes even when they weren't necessary.

I took radix_sort_32 and added this code after generating the counts:

// If this byte is zero for every value, skip the byte entirely.
if (count[0] == (size_t)(__last - __first))
    continue;

This is a quick check to find out if the entire input was full of zeroes for the current byte.  If so, it moves on to the next byte instead of wasting time making an exact copy of the input to the temp area.  It still uses more time than radix_sort_msd in the "values < 256" case, but it is much faster, because for 3 passes it can skip more than 50% of the work.

On my machine, radix_sort_32 got 40% faster in the "values < 256" case compared to before the change.  This might make you reconsider which version is better.

Hybrid radix sort

Another thing you could do to improve your radix sort is to use a so-called "hybrid" radix sort, which is a hybrid of MSD and LSD radix sorts.  The idea is that use you one pass of radix sort on the most significant byte.  This breaks the input into 256 "bins".  Then you run LSD radix sort (your current sort) on each of the 256 bins.

The advantage of this hybrid sort over a plain LSD radix sort is that the hybrid version is more cache friendly.  In the plain LSD sort, if your input doesn't fit in cache, then the second counting pass can't benefit from caching because by the end of the first counting pass, the start of the array will already be evicted from the cache.

With the hybrid sort, the input is first broken up into 256 parts (which hopefully do fit in the cache).  So sorting each bin should be faster due to caching.  However, there is extra overhead, because each of the 256 smaller sorts needs to do some fixed amount of work to deal with the counting bucket.  So we should only do the hybrid sort if the input is larger than our cache size, otherwise we will only have more overhead and no benefit.

I modified your code to do the hybrid sort and it ran faster than the original version on randomized 32-bit input.  I did not attempt to optimize it for the "values < 256" case, although there are definitely possibilities for improvements there, similar to what I mentioned in the previous section.  Here is the code:

// LSD RadixSort helper function for radix_sort() below.
template<typename Iter>
static void radix_sort_lsd(Iter __first, Iter __last, Iter __out, Iter __outEnd,
        int __msb, bool needsSwap)
{
    Iter __i, __j, __s;
    bool __swapped = false;

    for (int __shift = 0; __shift < __msb; __shift += 8) {

        // cycle input/auxiliar vectors
        if (__swapped) {
            __i = __out;
            __j = __outEnd;
            __s = __first;
        }
        else {
            __i = __first;
            __j = __last;
            __s = __out;
        }

        // counting_sort
        size_t count[0x100] = {};
        for (Iter __p = __i; __p != __j; __p++)
            count[(*__p >> __shift) & 0xFF]++;

        if (count[0] == (size_t)(__last - __first))
            continue;

        // prefix-sum
        size_t __m, __q = 0;
        for (int i = 0; i < 0x100; i++) {
            __m = count[i];
            count[i] = __q;
            __q += __m;
        }

        // filling result
        for (Iter __p = __i; __p != __j; __p++) {
            *(__s + count[(*__p >> __shift) & 0xFF]++) = *__p;
        }
        __swapped = !__swapped;
    }

    // if ended on auxiliar vector, copy to input vector
    if (__swapped != needsSwap) copy(__out, __outEnd, __first); //O(n)
}

// If the input exceeds this threshold (in bytes), we do one pass of MSD
// followed by the rest of the passes done by LSD.  This number should be
// an estimate of the cache size.
#define THRESHOLD 8000000

// RadixSort - works for values up to unsigned_int_max (32-bit)
template<typename Iter>
void radix_sort(Iter __first, Iter __last)
{
    typedef typename iterator_traits<Iter>::value_type value_type;
    size_t len = (size_t) (__last - __first);
    vector<value_type> out(len);

    // First, test if the input exceeds the caching threshold.  If the input
    // is smaller than the threshold, just do a straight LSD radix sort.
    if (len * sizeof(value_type) < THRESHOLD) {
        radix_sort_lsd(__first, __last, out.begin(), out.end(),
                sizeof(value_type) * 8, false);
        return;
    }

    // Set __shift to the most significant byte.
    int  __shift = (sizeof(value_type)-1) * 8;
    Iter __s     = out.begin();
    Iter __p     = __first;

    // counting_sort
    size_t count[0x100] = {};

    for (size_t i = 0; i < len; i++) {
        count[(*__p++ >> __shift) & 0xFF]++;
    }

    // prefix-sum
    size_t __m, __q = 0;
    for (int i = 0; i < 0x100; i++) {
        __m = count[i];
        count[i] = __q;
        __q += __m;
    }

    // filling result
    __p = __first;
    for (size_t i = 0; i < len; i++) {
        *(__s + count[(*__p >> __shift) & 0xFF]++) = *__p++;
    }

    // For each of the 256 bins, do a LSD radix sort on the bin.  The input
    // and auxiliary vectors have been swapped, so we pass needsSwap = true to
    // indicate that the LSD sort should end on the auxiliary vector instead.
    int startIndex = 0;
    for (int i = 0; i < 0x100; i++) {
        int endIndex = count[i];
        radix_sort_lsd(__s + startIndex, __s + endIndex, __first + startIndex,
                __first + endIndex, __shift, true);
        startIndex = endIndex;
    }
}

Other things

I think that vector<value_type> is an appropriate container for your auxiliary array.  I can't think of anything better than that.

As far as copy vs move, they should be equivalent for numeric types.  See this Stackoverflow question and its answers for some good explanations on why that is.


How can I improve performance of my selecting code

So pretty much everything will make a insignificant effect apart from the section where you are looping through each of the checkboxes. See my modifications and notes below.

$("a.action:not(.remove)").click(function (e) { // .remove => do not execute on download tasks page
     var selected = new Array(); // array to hold selections or clicks
     var obj = $(this),
         objData = obj.data(),

         // Moved these up here as they are not specific to the checkboxes
         packageId = objData.ajaxPackageid.removeSpecialChars().toUpperCase(),
         operation = objData.ajaxType;

     e.preventDefault();
     if (!obj.hasClass('disablelink')) {
         if (objData.ajaxMachineid) {
             selected.push(objData.ajaxMachineid);
         } else {

             // This seems a little convoluted to me, normally you would
             // use [type=checkbox]
             $("input[type=checkbox]:checked:not(.checkall)").each(function () {

                 var checkbox = $(this),
                     machineId = checkbox.val();

                 // No need to call .removeSpecialChars().toUpperCase() on packageId
                 var $row = $("#" + machineId + packageId);

                 if ($row.length) {

                     // Not 100% whether you need .length here
                     // It could also be beneficial to add a class to the row instead
                     // of calling this complex selector
                     $row.has("a[data-ajax-type=" + operation + "]:not(.hide)").length
                         ? selected.push(machineId)
                         : $(this).attr('checked', false);

                 }
             });
         }
         if (selected.length > 0) {
             // do something more here blah blah blah
         };
     }
     $("input.checkall").attr("checked", false);
 });

The obvious thing to do on top of my suggestions above is to cache the checkbox list, this means that every time it enters that section, the app will no longer need to go through every element in the DOM looking for all the checkboxes. Doing this will probably have the most meaningful impact on performance.

// in document ready
var checkboxes = $('input[type=checkbox]');

// When you need to find the checked check boxes
var checked = checkboxes.find(':checked:not(.checkall)');


Make website full responsive by using javascript

No, this is not the "normal" way to build a responsive website. "Normally" people use frameworks that are built specifically to remedy this problem.

Bootstrap, made by the guys who run Twitter, is the most popular responsive framework. Basically it's a set of styles and (opionally) javascript that you include in your sites and then you just add class names to your HTML depending on how you want things arranged. It's really easy to learn and use. Much easier than writing 1000s of lines of JS or CSS.


Reading and Storing Integer from Serial Monitor

Character literals

While this is technically correct:

if (received != 10) // Terminate if newline char detected

you're better off writing

if (received != '\n')

Much of embedded electronics assumes ASCII encoding, and this is no exception, but you're still better off using the symbol instead of the code.

IO management

You loop while serial I/O is available:

while (Serial.available())

But what if there is a pause in the availability of bytes in the middle of your integer? Your code will not do the right thing. Instead:

uint32_t number = 0;
while (true) {
  int received = Serial.read();
  if (received == -1) continue;
  if (received == '\n') break;
  // ...
}
Serial.println(number);

Do not use static for number
Don't need a counter
Keep looping if no data are available
Ensure that one execution of loop maps to one full output integer

Condition in loop

If you are using the ATmega328P, it has a dedicated MUL instruction that only takes two cycles. It's more complex and expensive to have your if (counter != 0) than it is to simply unconditionally multiply-and-add.


Euclidean algorithm to determine GCD (greatest common divisor) in C -- alternative method

Unnecessary function calls
My quick thought is that you are making an unnecessary function call.
unsigned temp;
while ((temp = a % b)) {
    a = b;
    b = temp;
}

The thing with the doubled parentheses is a hack to keep some compilers from warning you that you may be doing an unintentional assignment.  It should make no functional difference.
This does fewer assignments.  Three compared to four in your version.  And it makes no function calls.  Typically a function call is more involved than an assignment.  Because it has to first save the current state, then jump somewhere else and do work, and finally restore the original state.  The save and restore steps themselves often involve assignments.  And those may be relatively expensive assignments from register to cache or memory.  Whereas that loop can be done with three registers (or even two if the compiler optimizes out temp by doubling the code).
while ((a = a % b)) {
    if (!(b = b % a)) {
        return a;
    }
}

No temp in that version, but duplicate logic.  I wouldn't normally recommend writing that code.  But it would be a perfectly legitimate compiler optimization if there is a shortage of available registers.
Note:  if you only call this function a few times, this won't make a difference.  Use whatever you find more readable.  But if you are calling GCD in your main loop, it is possible that efficiency will matter.  If efficiency does matter, my revised version (the first one) will (absent compiler optimizations) probably be faster than your version.  While premature optimizations are to be avoided, there is little harm in picking the more efficient version if you have two working versions.  And GCD is exactly the kind of problem that may be subject to profiling that would lead to that kind of optimization.  So many sources may offer the best optimized algorithm rather than an alternative.
I find it a best practice to never use the statement form of control structures and always use the block form.  I also prefer the brackets on the same line as the control structure.  The latter is very much opinion, but the former is based on actual experience with bugs caused by editing.
Be careful of types
I also find it a bit risky that your variables are unsigned in the original function but int in the swap_numbers function.  This presumably works because the unsigned and int types use the same storage width.  But that's an implementation detail.  Is that implementation detail guaranteed always?  I don't know off-hand.  Perhaps you already did that work and do know.  But you don't say that you know and there's no comment explaining when you can and cannot use swap_numbers.
For example, what would happen if used with an unsigned long?  On some systems, that might work because int and long are the same width.  On other systems, you may only swap part of the numbers.  Possibly only some of the time (because if the bytes that you are swapping happen to be the ones with data, it might not matter).
You might fix this by switching from pointers to handles (pointers to pointers).
void swap_numbers(void **a, void **b)
{
  void *temp = *a;
  *a = *b;
  *b = temp;
}

unsigned euklid_iterative(unsigned *a, unsigned *b)
{
  while ((*a = *a % *b))
  {
    swap_numbers(&a, &b);
  }

  return *b;
}

But it might be easier just to write a swap_unsigned function.  Or write out the swap entirely as I did originally.
All code in this post is untested.  Not even for compilable syntax much less correct logic.  Use at your own risk.


Django ListView with MySQL query for the queryset

This is slow NOT because of your SQL.
The SQL in the post is a simple JOIN,
on id columns that are indexed by Django by default.

Without doubt, the cause of the slowness is this:

for row in queryset:
    pizza = Pizza.objects.get(id=row['pizza_id'])
    topping = Topping.objects.get(id=row['topping_id'])

For every row returned by the first query,
you load a Pizza and a Topping.
Django might be smart enough to not re-fetch the same topping multiple times,
but for every unique Pizza and every unique Topping,
Django will have to run an additional query.
With 700 pizzas, you're looking at at least 700 queries, which is clearly not efficient.

  Unfortunately, I can't just use Django's helpfulness and just give it the model to use (I'm skipping why for the sake of brevity)

It would be best to get to the bottom of this.
Take a look at the documentation of the select_related method on querysets,
I think the solution for your case should be somewhere around there.
I really think there's a solution within the realm of "Django's helpfulness",
without resorting to such queries, you just need to figure it out.
(You might want to ask some questions on that on stackoverflow.com)

Another workaround might be to stop calling Pizza.objects.get and Topping.objects.get:
include in your query all the fields you need for pizzas and toppings,
and build simple dictionaries from them.
Rest assured, when you stop calling Pizza.objects.get and Topping.objects.get 700 times you will notice a massive speed improvement.


Return Y integer with same bit sets as X & having | X - Y | minimum value

Your algorithm is logical, but a little trick can make it much simpler too. Because applying the trick makes the code so much shorter, it seems better to just point out some smaller issues in your code, and then show the improved algorithm later....

So, first up.... the method name: MinDiff .... really? Java convention is to have camelCase method names, not PascalCase. The first letter should be a lower-case.

Your variable names also are horrible.... y, x, a, and b. With x and y variables I am always looking for cartesian coordinates or something. Choose more meaningful names.

Your handling of 0 and -1 is also broken. Since there are no bits set in 0, and no unset bits in -1, they are impossible to provide a solution for, so the best thing would be to handle them as exceptions. Returning -1 for an input of 0 is broken, and overflowing on the input of -1 is inconsistent too.

So, having said that, the best algorithm to use is to identify the right-most change in bit values... where bitn and bitn-1 are different. Then, just swap them. A swap is easy to do with and XOR:

public static final int leastBitFlip(final int input) {
    if (input == 0 || input == -1) {
        throw new IllegalArgumentException("0 or -1 are not possible to compute as inputs");
    }
    int mask = 0b11;
    while (sameBits(input, mask)) {
        mask <<= 1;
    }
    // swap the bits.
    return input ^ mask;
}

private static boolean sameBits(final int input, final int mask) {
    final int bits = input & mask;
    final int swap = mask & ((bits << 1) | (bits >> 1));
    return bits == swap;
}

Now, as for determining whether two bits are the same, or not, I think that sameBits() function can be improved, but I have not really run the truth table on it. The code is simple enough to understand though, and readability is important in these things.

Using the hint from ErikR - this algorithm would be faster though too:

public static final int leastBitFlip(int input) {
    if (input == 0 || input == -1) {
        throw new IllegalArgumentException("0 or -1 are not possible to compute as inputs");
    }
    // right-most one-bit.
    int onebit = ((input ^ (input - 1)) + 1) >> 1;
    // right most zero-bit.
    int zerobit = ((input ^ (input + 1)) + 1) >> 1;
    // where the change happens
    int change = onebit > zerobit ? onebit : zerobit;
    // create a mask of the two bits either side of the change.
    int mask = change | (change >> 1);
    // swap them
    return input ^ mask;
}


Printing Gray code through modifying Samuel Beckett's stage instructions

I have some suggestions for your code.
Always add curly braces to loop & if
In my opinion, it's a bad practice to have a block of code not surrounded by curly braces; I saw so many bugs in my career related to that, if you forget to add the braces when adding code, you break the logic / semantic of the code.
Use java.lang.StringBuilder to concatenate String in a loop.
It's generally more efficient to use the builder in a loop, since the compiler is unable to make it efficient in a loop; since it creates a new String each iteration. There are lots of good explanations with more details on the subject.
before
String s = "";
for (int i = 0; i < n; i++) {
   s += "0";
}

after
StringBuilder s = new StringBuilder();
for (int i = 0; i < n; i++) {
   s.append("0");
}

Don’t use a loop when you want to repeat a string of x elements
The API that java give you offer a method to repeat the character x times; you can use java.lang.String#repeat (Java 11+)
before
StringBuilder s = new StringBuilder();
for (int i = 0; i < n; i++) {
   s.append("0");
}

after
String s = "0".repeat(n);

Extract the expression to variables when used multiple times.
In your code, you can extract the similar expressions into variables; this will make the code shorter and easier to read.
before
beckett(s, n - 1);
System.out.println(swap(s, "1", n));
s = swap(s, "1", n);
beckett(s, n - 1);

after
int previousNumber = n - 1;
beckett(s, previousNumber);
System.out.println(swap(s, "1", n));
s = swap(s, "1", n);
beckett(s, previousNumber);

Uses the increment and decrement operators when possible
Try to use the x++ / ++x, x-- or --x instead of x = x + 1 since they are more widely used and make the code easier to read.
before
n = n - 1;

after
n--;


In place solution to remove duplicates from a sorted list

One way that might speed up your solution slightly is to only place the value if necessary. Also note that only one of the two if conditions can be true, so just use else. Or even better, since there was a continue in the other case, just don't indent it.

i = 0  #slow-run pointer
for j in range(1, len(nums)):
    if nums[j] == nums[i]:
        continue
    # capture the result
    i += 1
    if i != j:
        nums[i] = nums[j] #in place overriden

You can also save half of the index lookups by iterating over the values. Of course it will need slightly more memory this way.

def removeDuplicates(nums):
    if len(nums) < 2:
        return len(nums)
    i = 0  #slow-run pointer
    for j, value in enumerate(nums):
        if value == nums[i]:
            continue
        # capture the result
        i += 1
        if i != j:
            nums[i] = value # in place overriden
    return i + 1

This can probably be further sped-up by saving nums[i] in a variable as well.

What is interesting is to see timing comparisons to using the itertools recipe unique_justseen (which I would recommend you use in production if you want to get duplicate free values in vanilla Python):

from itertools import groupby
from operator import itemgetter

def unique_justseen(iterable, key=None):
    "List unique elements, preserving order. Remember only the element just seen."
    # unique_justseen('AAAABBBCCDAABBB') --> A B C D A B
    # unique_justseen('ABBCcAD', str.lower) --> A B C A D
    return map(next, map(itemgetter(1), groupby(iterable, key)))

def remove_duplicates(nums):
    for i, value in enumerate(unique_justseen(nums)):
        nums[i] = value
    return i + 1

For nums = list(np.random.randint(100, size=10000)) they take almost the same time:

removeDuplicates took 0.0023s
remove_duplicates took 0.0026s


How can I optimize this hash string function?

Your hash_sequence() function is quite good. The only thing I would suggest about that function is rename your triplet variable to something more suitable like sequence. Currently, I expect triplet to contain strings of length 3. A name like sequence is more general and is a better representation of what will be stored in it.

Your intersects() function is where the vast improvement can be found. Each iteration of the for loop you are re-running the hash_sequence() function. If your strings are really long and k really small (i.e. 1), hash_sequence() would have to scan through the entirety of both strings each iteration of the loop!

Instead, we only have to hash one of the strings! Before the loop save the returned dict to a variable (this way the hash_sequence function is only ran once):

def intersects(string1, string2, k):
    dictionary= hash_sequence(string1, k)

Next, instead of iterating over the length of the dicts we can iterate over the keys in the dict, then we can check if that key is 'in' the other string:

for key in dictionary:
    # Check if the key (string of length k) is in the other string
    if key in string_two:
        return [key]

Here is my full version of your code:

def hash_sequence(string, length):
    dictionary = {}
    for i in range(len(string) - (length-1)):
        sequence = string[i:i+length]
        dictionary.setdefault(sequence, []).append(i)

    return dictionary

def intersects(string_one, string_two, length):
    dictionary = hash_sequence(string_one, length)

    for key in dictionary:
        if key in string_two:
            return [key]
    return None

As a final note, take a look at PEP8, the official Python style conventions. It will help make your code more Pythonic.


You may look at opencv Transparent API
#
imgUMat = cv2.UMat(img)
dst = cv2.fastNlMeansDenoising(imgUMat, 10,10,7,21)
#


Personally, I think you would be better off moving this code into a separate class as it's probably logic you are going to want in multiple places in your application and is most likely violating SRP (Single Responsibility Principle) wherever it is at the moment.

If we create a username class, we can encapsulate the logic in there and then re-use it from wherever we want in the application:

public class UserName
{
    private UserName() {}
    public string AccountName { get; private set; }
    public string DomainName { get; private set; }

    public static UserName Parse(string userName)
    {
        if (userName == null)
        {
            throw new ArgumentNullException("userName");
        }

        if (userName.Contains(@"\"))
        {
            var parts = userName.Split('\\');

            return new UserName { DomainName = parts[0], AccountName = parts[1] };
        }

        if (userName.Contains("@"))
        {
            var parts = userName.Split('@');

            return new UserName { DomainName = parts[1], AccountName = parts[0] };
        }

        return new UserName { AccountName = userName };
    }
}

You can then test the logic for this class in isolation:

public class UserNameTests
{
    public class WhenCallingParseWithANullUserName
    {
        [Fact]
        public void AnArgumentNullExceptionShouldBeThrown()
        {
            Assert.Throws<ArgumentNullException>(() => UserName.Parse(null));
        }
    }

    public class WhenCallingParseWithAUserNameWithNoDomainName
    {
        private UserName userName;

        public WhenCallingParseWithAUserNameWithNoDomainName()
        {
            userName = UserName.Parse("JohnSmith");
        }

        [Fact]
        public void TheDomainNameShouldBeNull()
        {
            Assert.Null(userName.DomainName);
        }

        [Fact]
        public void TheUserNameShouldEqualTheWholeUserName()
        {
            Assert.Equal("JohnSmith", userName.AccountName);
        }
    }

    public class WhenCallingParseWithDomainSlashUserName
    {
        private UserName userName;

        public WhenCallingParseWithDomainSlashUserName()
        {
            userName = UserName.Parse(@"Domain\JohnSmith");
        }

        [Fact]
        public void TheDomainNameShouldBeSetToTheDomainName()
        {
            Assert.Equal("Domain", userName.DomainName);
        }

        [Fact]
        public void TheUserNameShouldBeSetToTheUserName()
        {
            Assert.Equal("JohnSmith", userName.AccountName);
        }
    }

    public class WhenCallingParseWithUserNameAtDomain
    {
        private UserName userName;

        public WhenCallingParseWithUserNameAtDomain()
        {
            userName = UserName.Parse(@"JohnSmith@Domain");
        }

        [Fact]
        public void TheDomainNameShouldBeSetToTheDomainName()
        {
            Assert.Equal("Domain", userName.DomainName);
        }

        [Fact]
        public void TheUserNameShouldBeSetToTheUserName()
        {
            Assert.Equal("JohnSmith", userName.AccountName);
        }
    }
}


Approach to download files through POST request

Have you tried something like:

iframe.addEventListener('loadend', function() {
  document.body.removeChild(iframe);
});

The iframe has many other events (load, error, abort, progress) which might also be useful.


Parallel Calls to External WCF Service - ASP.NET Web Api

I think instead of creating an unknown amount of task using PLinq would make it simpler and have control over the number of task inflight/created
something like
var result = model.Materials
                .AsParallel()
                .WithDegreeOfParallelism(8) // whatever you want or leave it out
                .Select(material => _materials.Get(material.Number));

model.MaterialModels.AddRange(result);

This way it's not trying to make synchronous code async and can limit the amount of task inflight vs unlimited amount.


Simpler state manament alternative to Redux, MobX, Context API, etc

After looking at the implementation, this is not an implementation of global state management. The entire point of things like redux is so you can directly connect components to a global store. In your case, it looks like every instance of StateStore has its own unique data structure, which seems to imply that you are only going to use StateStore once.

You can only import it at the highest level and keep passing it down in order to persist the data. This has very heavy performance implications. Read up on how React decides on whether it should scan an entire component for any changes (rerenders can severely add up) and how the reconciliation algorithms relates to it.

Your implementation is essentially this.state but also handles functions. Technically the same functionality, but it's not a true global state like how the context api or redux works.


Login script that checks two users and their corresponding passwords

This is tagged programming-challenge, but just to be sure it's said: This obviously is not secure in any sense; the passwords are in the source.
Anyway, I'm assuming security isn't the point.

First of all: Spellcheck. "useranme" and "Your not chip/monty".

Secondly, my question is "why 2 users?" What if it's 3? Or 6,123,345? Right now you'd have to duplicate everything in your code to accommodate more users. Obviously that'll be extremely cumbersome.

A much better approach is to write code to check any given user/password against a list of users (rather than named variables).

So, first we'll need a list of users (or, in this case, a JS object):

var users = {
  'Monty': 'Cheese',
  'Chip':  'Gadget'
};

Then, some login logic (this can be a function, or several functions, or many other things - this is just a simple implementation)

var username = prompt('Enter Username:', 'username');
if( !username ) { // if username is blank, null or otherwise false'y
  return; // stop here if no name is given
}

// alert and stop if the username doesn't exists
if( !users.hasOwnProperty(username) ) {
  alert('Unknown username');
  return;
}

// now we can get and check the password
var password = prompt('Enter Password:','password');
if( users[username] === password ) {
  document.write('
Welcome to the site ' + username + '
');
} else {
  alert("You're not " + username + "!");
}

And done.


Writing a more Pythonic solution to the min triangle path

Your memo array is pretty inefficient, as it is of size \$n^2\$. There's a lot of wasted cells there, right? So one idea to improve the space complexity would be for each memo[i] to be just the right length. For example, memo[0] would be an array of length 1 instead of length n.. This would reduce space complexity from \$n^2\$ to \$n(n+1)/2\$. Still quadratic, but a small improvement.

But you can see that you don't need to keep all that information. All you care about is the previous row, so there's no need to keep more history than that. You could have an array of length n that represents the previous state and reuse it as you go through the triangle. This would result in a space complexity of \$O(n)\$, a big improvement. Of course, if you could change the input in place you could do a space complexity of \$O(1)\$ - no extra space required :)

I also like to use a bottom up approach to these problems. It's more beautiful, as it makes the solution just bubble up. It would rid you of the last for loop as well, which would make it just a bit faster.

Here's a sketch:

t = [v for v in triangle[-1]]
for i in range(len(triangle) - 2, -1 , -1):
  for j in range(len(triangle[i])):
    t[j] = min(t[j], t[j + 1]) + triangle[i][j]

return t[0]


MS VS solution structure w.r.t. dependency injection unit tests?

First, some quality/stylistic comments:

This won't do what you want/isn't legal:

  while (count < maxMeasuredValues )
  {
     return myDevice.Measure();
     count++;
  }

// non-deterministic (random) values
private readonly MyRandom myRandomVar = new Random(0);

Random initialized with a fixed seed is deterministic.  Even with no seed (current time) it is technically deterministic, but that's pickier.  Also, I don't see how you are using MyDevice in the code sample.

    double actual = 1;
    double expected = 21.068;

    TestDevice myDevice;
    CalculatingUnit myCalculatingUnit;

    myDevice = new TestDevice();
    myCalculatingUnit = new CalculatingUnit();

    actual = myCalculatingUnit.doMeasures(myDevice );

In general, prefer to declare and assign a value at the same time.  This will make the functions shorter and more readable.

Other than that, the organization looks fine.


Print numbers in a concentric square pattern

As has been said by @200_success, outputting individual characters at a time to the console is going to be relatively slow.  You'd be better off building up the string using a StringBuilder and then writing the whole thing at the end.

Assuming a maximum number of 9, you can predict the size of the required buffer which will prevent the buffer from needing to resize.  A safe value would be:

StringBuilder sb = new StringBuilder(N*2*N*2)

This number will need to be bigger if you have to support multiple digit numbers, however the StringBuilder will scale this for you if required.

Instead of calling System.out.print, you then append to the StringBuilder:

sb.append(j);

Adding new lines when required:

sb.append("\n");

And at the end, write out the entire buffer in one go:

System.out.print(sb);

Putting it together

public static void main(String[] args) {
    Scanner sc = new Scanner(System.in);
    int N = sc.nextInt();
    StringBuilder sb = new StringBuilder(N*2*N*2);
    for(int i = N; i>(1); i--) //Print upper part
    {
        for(int j = N; j >i; j--)
            sb.append(j);
        for(int j = 0; j<(i*2)-1; j++)
           sb.append(i);
        for(int j = i+1; j<=N; j++)
            sb.append(j);
        sb.append("\n");
    }
    for(int i = 1; i<=(N); i++)// print lower part
    {
        for(int j = N; j>i; j--)
            sb.append(j);
        for(int j = 0; j<(i*2)-1; j++)
            sb.append(i);
        for(int j = i+1; j <=N; j++)
            sb.append(j);
        sb.append("\n");
    }
    System.out.print(sb);
}


A function which collates data from various API calls. Help on design-pattern

Well, based on what I can get, here's some code that should work. Note that I use native Promises which is ES6 technology. It's handy, but I don't want to have jQuery tagging along. Use a polyfill for older browsers.

;(function(ns) {

  ns.generateReport = function(template, executionList) {

    // Because the operations can be async, we use promises instead. This allows
    // us to dictate when and what in the operation we resolve as a value.
    var promises = executionList.map(function(item) {
      return new Promise(function(resolve, reject) {

        //Assuming that everything is synchronous, the code most likely looks like this
        var result = search(item.search);
        var data = item.save.map(function(saveItem) {
          result.getValue(saveItem);
        });
        resolve(data);

        //If the operations are aync, then most likely it will look like
        search(item.search, function(result) {
          var data = item.save.map(function(saveItem) {
            result.getValue(saveItem);
          });
          resolve(data);
        });

      });
    });

    // We listen for all of the promises to resolve. When they do, the callback is
    // called with an array of resolved values from the promises we just mapped. They
    // will look like [[1],[2],[3,4]]
    Promises.all(promises).then(function(values) {

      // We concatenate the arrays inside the array using apply and concat. They'll
      // end up like [1,2,3,4]
      var params = Array.prototype.concat.apply([], values);

      // Add in the report template parameter so it'll look like ['template',1,2,3,4]
      params.unshift(template);
      saveToReportTemplate.apply(null, data);
    });

  }

// Uses an existing R or creates one
}(this.R = this.R || {}));

// Sample usage is this.
R.generateReport('report_template', [{
  search: 'transactions',
  save: ['transactions.done']
}, {
  search: 'company',
  save: ['transactions.done']
}, {
  search: 'company.investors',
  save: ['employee.name', 'employee.salary']
}]);


General notes:

I will focus mainly on your second question as I cannot imagine simpler solution. I would even suggest to do it a little bit more complex in order to make CSS more scalable (in the terms of resolution and content size).

For instance you can consider using width: auto; for your .popup class in order to make it scale itself depending on internal content.

Another approach would be to set width and height fixed and use overflow:auto; in order to use some scrollbars as internal content grows bigger.

If you would like to make it simpler for the user (if you mean an user of your code) you can wrap it inside some API like:

function showPopup(message) {
      const popupContainerClass = '.popupBg';
      document.querySelector(popupContainerClass).classList.toggle('hide');
      const popupContentClass = '.popup';
      const popupElement = document.querySelector(popupContentClass);
      popupElement.innerHTML = message;
      //... but please be aware of potential Cross Site Request Forgery vulnerability here!
      popupElement.focus();
}

Question2:

Your approach is about hiding an element that in fact is always present in DOM. It usually has advantages because displaying popup will not consume any processing power.

But in some cases (for instance when your popup presence would solely cause complex computations, or require lot of memory -like some videos or animations) it would be considerable to completely remove it from DOM instead.


Find Todos by Line Number (or any other reference) in Python - Revised as functions

Overall, pretty good job.  Here are some observations:

_get_files

path + f"/**/*{ext}"

Might cause trouble if path ends in a / and may be unsafe.  If a user inputs an empty path, this could start at the root of the file system, and try to scan every file.  Consider checking for such things and maybe using os.path.normpath or path.resolve() in pathlib.  pathlib also provides exists() and glob().

find_todos()

if exists(path) should probably come before it is used in the call to _get_files():

if exists(path):
    files = _get_files(path, ext,recursive=recursive)
    ...

A PermissionError might occur when a filed is opened, so I would tend to put the try:...except PermissionError: block in _find_todos_in_file()

_find_todos_in_file()

Each line is scanned at least twice, and maybe four times:

if comment_start in line and todo_token in line: # two scans
        ....
    comment_index = line.find(comment_start)     # one scan
    todo_index = line.find(todo_token)           # one scan

line.find(comment_start) returns -1 if the comment_start isn't in the line.  Also, find() takes a parameter that says where to start searching.  So the above code can be simplified and made more efficient like so:

comment_index = line.find(comment_start)
if comment_index >= 0:
    todo_index = line.find(todo_token, comment_index)

    if todo_index >= 0:
        temp_todos.append((line_no+1, line.strip))

I changed what gets appended.  The file name is redundant, it gets passed in as an argument, so the caller already knows what it is -- find_todos() uses it as a dict key.  And I put line_no and the text in a tuple rather than format them into a string.  Putting them in a string seems more like a job for an output function.  And keeping them as a tuple is more more flexible (e.g. you want to put them in a database, or see if TODOs show up more near the end of a file, etc).

other stuff

find_todos() and find_todos_in_file() basically iterate over all the lines in a bunch of files.  This is what the fileinput library does.  These two function could be combined and rewritten something like this:

def find_todos(path, ext, todo_token, comment_start, recursive):
    todos = defaultdict(list)

    files = glob("{path}/**/*{ext}", recursive=recursive)

    with fileinput.input(files=files) as f:
        for line in f:
            if f.isfirstline():
                filename = f.filename()
                print(f"Searching: {filename}")

            comment_index = line.find(comment_start)
            if comment_index >= 0:
                todo_index = line.find(todo_token, comment_index)

                if todo_index >= 0:
                    todos[filename].append((f.filelineno(), line.strip))

For a simple command line tool, the argparse library can help with parsing command parameters and options.  For more complicated or enhanced tools try a command line tool library like Click or Python Prompt.

def make_arg_parser():
    import argparse

    parser = argparse.ArgumentParser(
                        description='TODO finder/lister.')

    parser.add_argument('path',
                        default='.',
                        help="where to search for TODO's")
    parser.add_argument('ext',
                        default='py',
                        help="kind of files to search")
    parser.add_argument('comment',
                        default='#',
                        help="line comment marker")
    parser.add_argument('todo',
                        default='TODO',
                        help="text to mark TODO items")
    parser.add_argument('--norecurse',
                        action='store_false',
                        dest='recursive',
                        help="don't recurse into subdirs")
    parser.add_argument('--csv',
                        action='store_true',
                        help="save to a csv file")

    return parser

if __name__ == "__main__":
    cli_parser = make_arg_parser()
    args = cli_parser.parse_args()

    todos = find_todos(args.path,
                       ext=args.ext,
                       todo_token=args.todo,
                       comment_start=args.comment,
                       recursive=args.recursive)
show_todos(todos)

if args.csv:
    save_csv(todos, 'py')

The above is just an example.  It mirrors the call to find_todos() and adds an argument for todo_token and a flag to save to a csv file.


Trending Machine Learning topics for Alexa

Structure

Nothing! The code seems fairly straightforward and clear, and I was able to understand what you’re doing even if I didn't study flask too much. So I don’t think there’s anything wrong with the structure.

Code style

One small nitpick: some of your lines are too long (>79 characters). From PEP8:

  Limit all lines to a maximum of 79 characters.

Anyway, as a personal preference, I like to stick to PyCharm's recommendation and keep it < 120. But even with that in mind, you still have one line which doesn't fit.

Do this instead (for <120):

session.headers = {
    "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) "
                  "Chrome/59.0.3071.115 Safari/537.36"
}

Or:

session.headers = {
        "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5)"
                      "AppleWebKit/537.36 (KHTML, like Gecko) "
                      "Chrome/59.0.3071.115 Safari/537.36"
    }

Anyway, that's really subjective and it didn't bother me that you kept the entire User-Agent on the same line.

The same observation is also applicable for updateDate.

Flask-RESTful API

Apparently, you're trying to create some sort of RESTful API. Luckily, flask let's you do this by installing their Flask-RESTful API which you can do by running pip install flask-restful.

Flask-RESTful is a powerful extension for Flask that was designed to help you build complex RESTful APIs. It follows similar design principles to Flask so it is really easy (IMO) to pick up. Now, you might wonder why you should use this: well, your code was quite straightforward to implement but as the API grows in complexity it is easy to see this code becoming very complicated, very quickly!


Minimum perfect squares needed to sum up to a target

I won't specifically address the execution time concerns to begin with; there are other issues:

Don't use a class

There's no reason for this to be a class. You have one method and you don't even reference self. In theory you could remove self and mark it a @staticmethod, but really it should just be a function without a class.

Reuse variables

Make this variable -

i2 = i**2

since it's used twice. The same goes for min_coins_to_make[target].

Loop limit

    for coin in coins:
        if coin > n:
            break

That termination condition will be true if coin > n, but coin == i**2. i**2 > n will never be true, because in the previous loop,

        if i**2>n:
            break

So can't you just write for coin in coins without an interior termination condition?


Generic function, which populates a collection based on collection interface implementation

If you look closely there is actually four loops in your code. Two is explicit and other two from code like items.OfType<IStatusResult>() where if you look at the implementation of OfType<T> is looks something like below

foreach (object obj in source)
      {
        if (obj is TResult)
          yield return (TResult) obj;
      }

I would rather prefer to have one foreach loop to loop through all the items once and based on the type of each item, assign the appropriate properties something like below

foreach (var item in items)
            {
                if (typeof(ICurrencyResult).IsAssignableFrom(item.GetType()))
                {
                    item.CurrencyName = currencies[item.Currency];
                }
                else if (typeof(IStatusResult).IsAssignableFrom(item.GetType()))
                {
                    item.StatusName = statuses[item.Status];
                }
            }


Let's answer your questions first:

What about the memory issues?
If you are not limited to an older C++ standard in any way, it would be the best to use C++11 (or above), since it provides you with a good implementation of smart pointers (reference, in-depth-tutorial). This way, all your memory management becomes easier and more resilient. Also you do not need to manually delete anything and do not run into trouble if someone else is still accessing the element you just removed.

Caution: Smart pointers and raw pointers do not mix very well, so avoid raw pointers whenever possible.

What about type definitions?
In general, many people nowadays do frown upon typedefs. Firstly, they hide the actual composition of the type and may lead to confusion or wrong assumptions - especially in team projects. Second, in C++ you do have the using keyword which is a better choice here, especially since it can be combined with templates. (reference)

What functions should be provided?
If there is a push_back I would expect some kind of pop_back (also a push_front). Usually you have some methods to give you the number of elements in the list, check if the list is empty... You can take Qt's QList class as an inspiration ().

Suggestion: make node a templates class
Defining the inner class list<T>::node is valid code and gets the job done. However, you may find that it would be more convenient to pull it out of the list class and define a stand-alone class node<T>. A list<T> then contains a node<T>. Depending on your design, you then might find that your node class then is re-usable e.g. for queues.

Recommendation: Use namespaces
Namespaces are your #1 tool to structure your code. At least one level of namespaces should be used at all times to avoid cluttering the global namespace.

Suggestion: use #pragma once
If your compiler supports it, this is a good alternative to the include guards. Just put it into the first line of your header and you are good to go.

Hint: inline is not that important
First, do not waste brain capacity on whether to inline something while developing. Early optimization suffocates progress, diverts attention, and thus kills projects. Get it to run, then optimize one step at a time. Second, modern compilers know best what to inline and what not. Let them do it, they are really good at this. While learning, trust your compiler. You can argue with it later :)

Additional note: Combining templates and inline is used to address issues with the One Definition Rule (ODR). If you define a template, but do not fully specialize it, you may (but do not need to) inline. However if you do also want to provide a fully specialized case, you have to inline, otherwise you conflict with the ODR, since the compiler expands the template and thus already provides an implementation for the case you intended to cover "by hand". See also this question for details and examples. (Kudos to @Cody Gray)

Wrapping things up
I just sketched together a piece of code that sums up the made suggestions/ recommendations to give you an orientation (Note: I prefer the Java-style of camel-casing the type names with an initial upper case letter, but this is a question of personal preference).

#pragma once

#include <memory> // for smart pointers

namespace collection {

template<typename ValueType>
class Node<ValueType> {

private:
    ValueType _data;
    // alternatively store a smart pointer to the data
    // if the amount of data is large e.g. an image
    std::shared_ptr<Node<ValueType>> _next;
    // to "delete" the next pointer just reset it to nullptr
    // only hand out the shared pointer, never the thing it points to
    // the receiver has to check if the pointer is null before use
public:
     //... you know what to do
};

template<typename ValueType>
class List<ValueType> {

private:
    std::shared_ptr<Node<ValueType>> _head;
    // you can also keep track of the tail and whatever...
public:
    // ... all the functions
};

} // end namespace collection

Last but not least
Put your code somewhere accessible (e.g. github) which makes it easier to review code, suggest concrete changes, discuss details, and notify you about the bugs and issues that were found. Post a link and I will have a look at it :)


Adding or replacing a sheet in an Excel file

VBA does a lot of implicit things. And it can get confusing, and it can cause surprising bugs.

Your function is implicitly Public. Is it meant to be called from another module? Or is it only called from within the same module it's in? Being explicit about everything makes the intent of the code much easier to understand.

The parameters are implicitly passed ByRef, and yet, their values aren't reassigned in the body of the function (which is good!), so they could, and should, be passed ByVal. It's really a shame that ByRef is the implicit default in VB6/VBA.

The comment that's trailing after the function's signature, should be either immediately above, or immediately below the signature - avoid horizontal scrolling, and don't hide information from maintainers.

The comment could be clearer about why a Boolean return value is returned.

Private Function AddSheet(ByVal shtName As String, Optional ByVal readOnlyFlag As Boolean = True) As Boolean
'Appends a new worksheet to the active workbook. Returns True if sheet is successfully added.

It's not clear what readOnlyFlag does, even when reading the code. It probably could use a better name and/or a comment. It seems it determines whether the sheet should be replaced if it already exists?

The IsSheet function looks like it should be called IsExistingSheetName.

The instructions separator (colon) after With ThisWorkbook: serves no purpose and should be omitted; the Exit Function is redundant, too, since the code branches out of that block anyway.

There's an execution path where you're verifying whether the sheet name exists twice - I'd factor that check out of the If block.

As for the return value - I'd go even more explicit than that:

Dim result As Boolean

Dim isExisting As Boolean
isExisting = IsExistingSheetName(shtName)

If readOnlyFlag And isExisting Then
    result = False
ElseIf Not readOnlyFlag Then
    If isExisting Then DeleteSheet (shtName)
    With ThisWorkbook
        .Sheets.Add(After:=.Sheets(.Sheets.Count)).Name = shtName
    End With
    result = True
End If

AddSheet = result

Now, how helpful is that? How does the calling code access that new worksheet now? With Application.ActiveSheet? That's a dangerous assumption to make!

How about returning the Worksheet object instead? The function would return Nothing if no worksheet is added, and an object reference when it is.

Dim result As Worksheet

Dim isExisting As Boolean
isExisting = IsExistingSheetName(shtName)

If readOnlyFlag And isExisting Then
    'Set result = Nothing '' todo: remove this useless branch
ElseIf Not readOnlyFlag Then
    If isExisting Then DeleteSheet shtName
    With ThisWorkbook
        Set result = .Sheets.Add(After:=.Sheets(.Sheets.Count))
        If Not result Is Nothing Then result.Name = shtName
    End With
End If

Set AddSheet = result

Notice something? The code will blow up if .Add fails, because the function will return Nothing and then .Name cannot be set: you need to handle this.


There's a couple of points that are a possible C or Java background leaking through:

You end a lot of lines with an unnecessarry semicolon
You've named your methods and some variables using camelCase, whereas the prefered convention in Python is usually lowercase_with_underscores (the unittest module is an anomaly here, being one of a very few isolated stdlib modules using camelCase)
The getSets method is unnecessarry. Python convention is to expose attributes directly because if the implementation changes later, you can just make it a property. If you really don't want to do that, then you should rename self.sets to self._sets to emphasise that it isn't part of the documented API

I have a feeling that the following line:

self.sets.append([repr])

isn't actually doing what you want. It looks like you mean self.sets.append(repr), otherwise self.sets is full of single-length lists whose only item is a collection of some kind whereas it would make more sense for those collections to be in self.sets directly. You would then make a corresponding change down in findSet so that the test there reads:

if repr1 == oneSet:

Also think about what should happen if findSet fails to find the set. As written, it will fall off the end and impliiclty return None, which will almost certaintly lead to errors in short order. Instead, you should probably immediately raise ValueError with an appropriate error message. You may also want to use list.find to help with this.

You could then add that case to your unit tests.

repr is the name of a builtin function, so calling your arguments by that name shadows it. A better name might be subset, for example. You may also want to document whether it should be a set, a list or a tuple, especially if that ends up mattering to any of the optimisations you write later.

In test_simple, this looks like a typo:

expetctedSets = [[1, 2, 4, 5], [3]];

should be 'expected'.

Finally, you say that this class models disjoint sets, but you never enforce that. Indeed, this would work:

ds = Disjoint()
subset = {1,2,3}
ds.add(subset)
ds.add(subset)
ds.add(subset)

If you are assuming clients won't do things like that, that is OK (although it may make your class less useful), but you should document that. In fact, some docstrings in general wouldn't go astray.


php most elegant function call returning array

This question is not simply preference, the second method is better than the first for the following reasons:

In the first, you initialise a function test and it returns an array: test[]
So, alongside the string to test against 'short' in the memory, you now have those three things in the memory.

Whereas, in the second example, you don't return an array, and the 'short' string is only used as a parameter, by doing this you now can only have two things in the memory.

Therefore, the second code block is better than the first.

Also, in the spirit of Code Review, you can further improve the second code block, by not actually assigning $array, or using a switch.

public function test($myVar, $key) {
    return [
        'test1' => [
            'value' => 'Value of test1',
            'short' => 'Val 1'
        ]
    ][$myVar][$key];
}

$this->test('test1', 'short')  // Val 1

$myvar should also be in camelCase: $myVar


You are on the right track about trying to reduce trips to the database. I think the rule of thumb would be: whatever gives the best overall performance ;-)

A single query will almost always outperform N queries. Sometimes you may have the choice between two single queries: one that's extremely complex but loads exactly the data you need, and another that's simple but loads more data than you need and you have to filter the result yourself in code. When the database is heavily loaded, the simple query can be faster.

That said, your final $query is not optimal. This would be better:

SELECT col1, col2 FROM shelf
LEFT JOIN (
    SELECT tray.shelf_id, SUM(egg.weight) AS total_weight FROM tray
    LEFT JOIN egg USING (tray_id)
    GROUP BY shelf_id
) AS weight USING (shelf_id)
WHERE shop_id = ?

Of course col1 and col2 are not real columns, I just wanted to emphasize that you should always write the column names explicitly. It's faster to load only what you really need. It also protects you from changes in the column order.

Also notice that I removed one sub-query, which should also make it faster.


Removing commented dead code without removing the legitimate comments

echo | grep is unwarranted. bash understands regular expressions ("$line" =~ regex), and can do simple substitutions: line=${line#[[:space:]]*\/\/} removes leading whitespaces , followed by the comment, just what we are after.
Replacing the file while reading it looks suspiciously. I recommend to have a destination file, and copy valid lines (and don't copy undesired ones). A perk benefit is that forking sed is not needed anymore.

A side note: the script makes a false positive in cases like

    // Notice that
    // some_valid_c_code;
    // doesn't work, because etc

The part of the comment would be recognized as a dead code, and the output will be

    // Notice that
    // doesn't work, because etc


Converting HEX values to Unicode characters

Only use try-except when you need to. Your outer try should be useless.
You should also perform chr(int(code, 16)) once, as there is no gain to running it twice, it just costs cycles.

Your output is horrible, if there is no valid input you go from explicitly silencing it to returning an error.
And if somehow you have an array of items, but they don't get added to sym you return None.
You need to pick how your function works. Either remove invalid characters, or raise errors.

Doing the above removes half your code. I'd also change except to catch certain errors, e.g. ValueError, otherwise your program is prone to mask bugs.

Strings are immutable and so sym = sym + chr(int(code, 16)) can take a very long amount of time.
Instead build a list and ''.join it.

Finally return c_s.get('incorrect') is a massive red flag, remove these and raise exceptions instead.

This can get you:

def unicode_(msg):
    new_msg = []
    for char in msg:
        try:
            char = chr(int(char, 16))
        except ValueError:
            char = '?'
        new_msg.append(char)
    return ''.join(new_msg)


Calling variable methods to handle ajax requests

First and foremost, I would worry about getting rid of that awful code nesting you have. This nesting yields increased cyclomatic complexity which tends to lead to more buggy/fragile code and code that is harder to maintain.

So, at a minimum, think of re-writing your code like this:

if(empty($_POST['action'])) {
    echo json_encode(["0", "No request received"]);
    exit;
}

$action = strtolower($_POST['action']);
if($action === 'get_notification') {
    //  return session notification messages
    exit;
}

if(empty($_POST['module'])) {
    echo json_encode(["0", "No request received"]);
    exit;
}

require_once("libraries/class.module.php");
$module = new Module;
$moduleName = strtolower($_POST["module"]);

if(!$module->verify($moduleName)) {
    echo json_encode(["0", "No request received"]);
    exit;
}

// rest of code, now without nesting

Now, with that consideration out of the way.  Let's get to your main question. I am bothered by the fact that you call your script controller.php when what it is really doing is routing.  Your "modules" seem to really be your controllers and your "routing" is done in a very procedural fashion.

What you are really missing here is the concept of a router which can authoritatively map the request to a controller which can handle the request and to which request execution is then handed off. This is a concrete application of the Inversion of Control principle.  You are trying to keep control of request execution at the level of this script, which means you will have to continue to increase the complexity of this script as you add new use cases.

So to answer you question about using a switch. I would recommend that, rather than thinking about variable methods or switch as your alternatives, you think about proper routing and inversion of control as your alternative.

Envision a top level script here which looks like:

/**
 * An object that encapsulates the request data - URI, GET/POST data, etc.
 * that you can pass through your system
 */
$request = new Request();

/**
 * An object which can inspect the request and instantiate a controller to
 * fulfill it. It then hands off script control to the controller.
 */
$router = new Router();
$controller = $router->getController($request);
$controller->execute();

That's it.  That's all that needs to be in this top-level script file (with the exception of any script bootstrapping) if you embrace the concepts of routing and inversion of control.

The Router of course would need to hold the logic mapping request properties to controllers - oftentimes with that mapping be captured in a config file. That might look something like this (here I am using JSON representation, but this could take many forms):

{
    "action": {
        "get_notification": "NotificationController",
        "other_action": {
            "module": {
                "some_module": "OtherActionSomeModuleController",
                "other_module" "OtherActionOtherModuleController",
                ...
             },
             "other_module: { ... },
        },
        ...
    }
}

This could, for example, just map various actions, modules, etc. to different controller class names. Each of these controllers would need to be able to accept a Request object as a dependency and know what to do with it when execute() is called. The router would instantiate the appropriate class and then pass the Request to it.

As a developer wanting to add new action/behaviors to the system, all you would need to do is build a new controller for it, and then add the new controller to the route map as appropriate. You would not need to change one bit of code in the rest of the application (i.e. the top-level routing script) unless you were introducing a non-compatible change for some reason (i.e. your whole route map would need to change).

And in case you were wondering what happened to all the handling of invalid requests, that would be something that would live with the Router.  If it cannot successfully route the request, it could just instantiate an InvalidRequestController or similar to fulfill the request.

So your getController() method might look like:

// assume the config shown above has been loaded into a multi-dimensional array
// in $map property.
public function getController(Request $request) {
      if(!array_key_exists($request->action, $this->map) {
          return new InvalidRequestController($request);
      }
      $action = $this->map[$request->action];
      if(!is_array($action)) {
          return new {$action}($request);
      }
      if(!array_key_exists($request->module, $action)) {
          return new InvalidRequestController($request);
      }

      return new {$action[$request->module]}($request);
}

A few other considerations:

You really ought to consider using an autoloader (ideally a PSR-4 compliant one).  Having a bunch of include/require statements littered throughout your code to load classes is really a very old way of doing things.
You really should consider sending appropriate HTTP error codes when your application receives mal-formed requests (i.e. all of your "invalid request" conditionals).
What is the purpose of having both action and module if you only have a single action you are listening for and everything else goes to modules?  It seems like this could perhaps be just a single layer of routing as opposed to the nesting routing construct this code implies.
I don't know what $module->verify() does, but is it a potential security concern with passing unvalidated POST data deeper into the application?
When you have variable interpolation like this $module->$action(), consider using braces to eliminate ambiguity - $module->{$action}() (as you could theoretically have $module->{$action()} usage).
There are a bunch of bad coding resources/examples out there on the web (especially for PHP) that can lead you to bad programming habits, or that are out of touch with modern development approaches.  I will leave you with what I consider to be the best resource to someone learning to work with PHP - http://www.phptherightway.com/


Using a custom comparison function for BTreeSet and a foreign type

Your code looks basically how I would write it, very nice! I do have a few tiny nits.

I strongly dislike using Deref except when you have a smart pointer. I fear that using Deref to compose items is going to be the Rust equivalent of using inheritance for code reuse.

Using Deref also can be a bit surprising, as every method is now available. Let's say that your Data struct has some more methods:

impl Data {
    fn wow(&self) { println!("Wow") }
}

Now, your SortableData has those methods as well!

for d in sorted.iter() {
    d.wow();
}

I much prefer to be explicit about it; I would implement Borrow instead.

I don't believe this opinion to be 100% supported by the community, but I'm going to make my case where I can ^_^
When possible, I'd implement From / Into in both directions. This doesn't always make sense, but here it does.
There's no need to specify the type for source — type inference handles that just fine.
You could just map the into function directly: .map(Into::into).
Instead of calling iter in the for loop expression, you can just take a reference.
From and Into are both part of the prelude and thus don't need to be used again.

use std::borrow::Borrow;
use std::cmp::Ordering;
use std::collections::BTreeSet;

// Here's the original struct...
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
struct Data(usize);

impl From<usize> for Data {
    fn from(i: usize) -> Self {
        Data(i)
    }
}

// And here's the wrapper type we'll use to sort it.
#[derive(PartialEq, Eq, Debug)]
struct SortableData(Data);

impl From<Data> for SortableData {
    fn from(data: Data) -> Self {
        SortableData(data)
    }
}

impl From<SortableData> for Data {
    fn from(data: SortableData) -> Self {
        data.0
    }
}

impl Borrow<Data> for SortableData {
    fn borrow(&self) -> &Data {
        &self.0
    }
}

impl PartialOrd for SortableData {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl Ord for SortableData {
    fn cmp(&self, other: &Self) -> Ordering {
        (self.0).0.cmp(&(other.0).0)
    }
}

fn main() {
    let source = [Data(9), Data(4), Data(2)];
    let sorted: BTreeSet<SortableData> = source.iter().cloned().map(Into::into).collect();
    for d in &sorted {
        let d: &Data = d.borrow();
        println!("{:?}", d);
    }
}


Since this is supposed to be a practice in idiomatic haskell, point free seems to be the way to go

Ah, but point-free Haskell isn't idiomatic in general. Just compare both variants of divisible:

divisible x y = y `mod` x == 0
divisible = ((==) 0 .) . flip mod

The point-free variant is not only harder to read, but even longer, and I've already used infix-mod to increase the pointful version by two characters. The same holds for any, although the original variable names where a little bit long:

any' p m = any ($ m) p
any' = flip (any . flip id)

The point-free version of euler1 is perfectly fine. It's also the most natural one to do by hand, since \x -> f $ g x is f . g, and euler1 follows exactly that style. Defining new functions by combinatiing old ones is great if you have that style of function, e.g.

foo x = bar $ quux $ fox $ tango $ x
foo = bar . quux . fox . tango

But as soon as a second parameter gets involved, it gets a lot more trickier. And as always, the best code is not worth it, if you cannot understand it later on. Haskell's referential transparency makes that a lot easier, but if you need to sit down and check how flip (any . flip id) works, it's a good call to use \p m -> any ($m) p.

In the end, it boils down to your personal choice. I would have used a list comprehension, because  it's still easy to read, but it's only a syntactic sugar variant of your filter:

euler1 xs = sum [x | x <- xs, x `rem` 3 == 0 || x `rem` 5 == 0]

  I opted to describe how the answer is defined (as seems to be the way FP works) as opposed to find a faster solution

Just for completeness, have a look at the inclusion-exclusion principle, and try to solve Euler1 in Haskell (or another language).

  but I'd be fecetious in saying I fully understood the pointfree formulation of any'.

flip (any . flip id) p m
  = (any . flip id) m p
  = ((any . flip id) m) p
  = (any (flip id m)) p -- see remark below
  = (any ($ m)) p
  = any ($ m) p

id      :: (a -> b) -> a -> b
flip id :: a -> (a -> b) -> b
($)     :: a -> (a -> b) -> b

But the flipless version is easier to understand.

  That may effect the best solution for me, but I'm interested mostly in idiomatic, readable (to more veteran haskellers) code.

Any veteran will tell you that the first version is more readable. They will understand the latter, but after all, both versions do exactly the same.


I don't understand why it should be a warning to have over 80% of the system RAM available.  If anything needs a warning, it should be having under 20% of RAM available, right?

Reading each line into an array is overcomplicated.  The read builtin command is quite good at splitting each line of input into multiple variables.  (row_value_lenght is misspelled, by the way.)

The proc(5) man page says about /proc/meminfo:

  Each line of the file consists of a parameter name, followed by a colon, the value of the parameter, and an option unit of measurement (e.g., "kB").

There's no guarantee that the unit will be kB (as you have assumed in your printout), or that both lines of interest will have the same units.

Splitting up the calculation of mem_percentage is weird.

The echo statement should be line-wrapped for readability.

The cases for exit codes 0, 1 and 2 can be collapsed into one, using arithmetic.

#!/usr/bin/env bash

set -o errexit -o pipefail

warning=80
critical=90

while read -r label number unit ; do
    case "$label" in
      MemTotal:)
        mem_total=$number
        mem_total_unit=$unit
        ;;
      MemAvailable:)
        mem_avail=$number
        mem_avail_unit=$unit
        ;;
    esac
done < /proc/meminfo

if [ -z "$mem_avail" -o "$mem_avail_unit" != "$mem_total_unit" ]; then
    echo "MEMORY UNKNWOWN - check script"
    exit 3
fi

mem_avail_percent=$(( 100 * $mem_avail / $mem_total ))
echo "MEMORY_TOTAL=$mem_total$mem_total_unit" \
     "MEMORY_AVAILABLE=$mem_avail$mem_avail_unit" \
     "|" \
     "MEMORY_IN_PERCENTAGE=$mem_avail_percent;$warning;$critical"
exit $(( ($mem_avail_percent > $warning) + ($mem_avail_percent > $critical) ))


Modular arithmetic brute-force congruence finder

In case it is you who figured out an \$h\$ factor in the code snippet, add \$x^3\$ to both sides and rewrite the second equation as \$(x + h)^3 \equiv x^3 (\mod P)\$, so that with the first one in mind, the system comes down to

\$x^3 + v \equiv 0 (\mod P)\$

\$(x + h)^3 + v \equiv 0 (\mod P)\$

The programmatic approach is obvious:

find all solutions to \$x^3 + v \equiv 0 (\mod P)\$, and
select those which are exactly \$h\$ apart.

Of course the first step is a key (the second is trivial). I am not that good in a theory of Diophantine cubics to suggest any more.

PS: Math.SE is indeed a right place to ask.


Subclass of AtomicReference with atomic function application

Your approach is definitely valid, I've seen something similar used in GHC's implementation of atomicModifyIORef.

However I have some doubts if such a class is really useful. If the computations are potentially long running, the performance advantage of using AtomicReference instead of some standard synchronization primitive is negligible. Moreover, if there are several updating threads, it will result in repeated recomputation, wasting CPU power on it. So using plain synchronized on some private lock object (so that no other part of code can lock on it) seems simpler, safer and with better performance.


It looks like it is doing what it should be doing.

I don't understand that remark. In particular, you posted no unit tests or other evidence of the target code being exercised in usual cases and in edge cases.

    private T t;

What is that? The ctor doesn't affect it. It doesn't appear to contribute to any Stack invariants. Why can't pop() and peek() simply return null?

    if (stack.size() != 0) {
        return false;
    } return true;

Don't show that at an interview. Use the more natural return stack.size() == 0 instead.

For some workloads involving a large stack, this implementation will show worse performance than one that uses a doubly linked list.


Should I return an object wrapped by an article tag?

Spec:

  The article element represents a complete, or self-contained, composition in a document, page, application, or site and that is, in principle, independently distributable or reusable, e.g. in syndication. This could be a forum post, a magazine or newspaper article, a blog entry, a user-submitted comment, an interactive widget or gadget, or any other independent item of content.

Complete and self-contained? Yes - it has information about foostore which doesn't require the rest of the document to make sense.
Composition? A work of music, literature, or art? Not really. It would work better if it included a review.
One of the below:

A post? No.
An article? No.
Blog entry? No.
User-submitted content? Is it?
Interactive? No.

So probably not, as long as it isn't user-generated reviews.


Making Math vector implementation using java

Question 1 not exactly the right place to ask here I think. Perhaps aks that one as a question on StackOverflow instead? For the i it's possible by directly putting in the î (type ^ first then i to make it 1 character, also works for ê û ô and â, not directly possible in standard character sets for j or k as far as I know).
Unless if you want to just use an alternative notation. For example printing (x,y,z) instead of the xî yj zk. But that would be trivial to implement yourself once you do decide to do it like that.

For question 2 I suggest you get yourself a good IDE (for example IntelliJ or Eclipse) and have it generate an equals and hashcode function for you. Doing that correctly can be tricky.

The code you have now looks pretty good as is.

The only big issue I have is the package visibility of the x y and z fields. The default best practice would be to make them private and provide getters to access them if needed. Also only provide setters if it makes sense to change the values (usually not the case).

Alternatively you might want to access the fields often in which case calling getters could become annoying. In that case I would suggest making the class immutable and provide public access the the fields.

public class Vector {
    public final double x;
    public final double y;
    public final double z;

Immutability is a nice property to have, especially if you want to use the class in a multithreaded environment. (Even if you make the fields private it's still a good idea to make them final as well).

A last smaller issue I want to mention is to always use braces to prevent silly mistakes. Especially if these silly mistakes can take hours to find.

lastly some minor nitpicks

the usual java convention says to put the opening bracket on the same line. For example:

 public String getReducedStringNotation() {
    String not="";
    if(this.x!=0) {
        if(this.x==(int)this.x) {
            not=not+(int)this.x+"i^+";
        } else {
            not=not+this.x+"i^+";
        }
    }

The good thing is that you were consistent, so not really a complaint here.

You don't need this variable here:

public double modulus() {
    double mod=Math.sqrt(Math.hypot(x,Math.hypot(y,z)));
    return mod;
}

You can directly return it return Math.sqrt(Math.hypot(x,Math.hypot(y,z)))

Also, should this be

public double modulus() {
    return Math.sqrt(x*x + y*y + z*z);
}

instead?

The following line could also be a direct return statement:

    if(xr==yr&&yr==zr&&zr==xr)
        return true;
    else
        return false;

replace it with:

return xr==yr && yr==zr && zr==xr;

not the best variable name. I would prefer notation or result in most cases.

More specifically to actually understand what this does:

    if(not.equals(""))
    return null;

Which brings me to my last nitpick. Wouldn't it be better to return that empty string here? NullPointerExceptions are one of the most annoying thing to deal with in Java. I prefer some minor "huh?" moments when there was an empty string where I would expect a Vector over my program crashing with a NPE.


Have you considered using named Route templates instead of capturing regular expressions? It could make the code more readable. Consider

Route("/task/refresh-user/<username>", RefreshUserHandler)

instead of

(r'/task/refresh-user/(.*)', RefreshUserHandler)

for example. (Of course, I don't know what kwargs RefreshUser actually wants, but you can change the angle-bracketed part to the appropriate name.)


I use log4net in my test code along with the ObjectPrinter library and the log4net ConsoleAppender.

When I'm actively working on a test where I want to see additional output I just crank up my logging level in my log4net config file so I can see the additional debug information in my ReSharper test runner console window.

private static readonly ILog Log = LogManager.GetLogger(typeof (MyTestFixture));

[TestCase("US1")]
[TestCase("123")]
[TestCase("E$R")]
public void Get_From_IsoAlphaCode_IfThreeLetterCodeIsNotAllAlpha_Error(string val)
{
    var ex = Assert.Throws<IsoAlphabeticCodeFormatException>(() => Currency.Get(val));
    Log.Debug(ex); // <-- only prints when log4net is configured to log DEBUG msgs
    Assert.That(ex.Message, Is.StringStarting(IsoAlphabeticCodeFormatException.Not3AlphaCharsMessage));
    Assert.That(ex.ParamName, Is.StringMatching("isoAlphaCode"));
    Assert.That(ex.ActualValue, Is.EqualTo(val));
}

ObjectPrinter isn't required, but it outputs more information into the log by reflecting over the passed in objects public properties. The Log.Debug statement outputs something like this in the console window.

2012-01-01 19:38:26,362 [7] DEBUG NUnitTestHarness1.MyTestFixture [IsoAlphabeticCodeFormatException]: hashcode { 11076195 }
{
    Message : Badly formed ISO 4217 Aplhabetic Code.
    StackTrace :    at Currency.Get(String val) in d:\source\spikes\NUnitTestHarness1\NUnitTestHarness1\Class1.cs:line 31
           at NUnitTestHarness1.MyTestFixture.c__DisplayClass1.b__0() in d:\source\spikes\NUnitTestHarness1\NUnitTestHarness1\Class1.cs:line 18
           at NUnit.Framework.Assert.Throws(IResolveConstraint expression, TestDelegate code, String message, Object[] args)
    Source : NUnitTestHarness1
    TargetSite : Int32 Get(System.String)
    HelpLink : {NULL}
    ParamName : {NULL}
    ActualValue : {NULL}
    Data : {}
    InnerException : {NULL}
}


Security flaws in smartphone application

mysql_real_escape_string

mysql_real_escape_string is deprecated and thus should not be used anymore. As you are using mysqli an alternative would be mysqli_real_escape_string.

But you don't need it anyways, because you are already using prepared statements (which is generally recommended over using any of the real_escape_string functions).

So you can just get rid of cleanPost, it doesn't add any security against SQL injections that the rest of your code doesn't already take care of.

htmlspecialchars

htmlspecialchars is for encoding HTML characters, and used to prevent XSS attacks. You don't need it (and shouldn't use it) when adding data to the database, but when printing user supplied data to a user.

So you should also get rid of the call to htmlspecialchars.

Misc

you are missing a semicolon at the end of your return statement.
you shouldn't just exit, throw an exception instead (so the calling code can decide what to do about it).
Yes, I would assume that MySQL can handle that sort of data on any computer without a problem.


Your code

You have a sneaky bug in your code!

1000/2.times {|i| puts i }

You seem to think that this code would display 500 numbers between 0 and 499.
It doesn't. Instead, it displays 0, 1, and returns 500.

You need to replace str.length/n.times do with (str.length/n).times do.

Alternative

You can use gsub to look for the substrings, chop them and replace them:

def delete_every_nth_char(string, n)
  string.gsub(/.{#{n}}/){ |sub| sub.chop }
end

delete_every_nth_char('1234A1234B1234C', 5)
# "123412341234"
delete_every_nth_char('ABAB', 2)
# "AA"
delete_every_nth_char('ABA', 2)
# "AA"
delete_every_nth_char('ABA', 1)
# ""
delete_every_nth_char('ABA', 5)
# "ABA"

It is concise and probably faster than splitting and joining the strings manually.


Int extension for translating integer to plain English

So Apple already has an implementation for this, using NSNumberFormatter.  A bit easier to implement than what you have now ;).

var numberFormatter:NSNumberFormatter = NSNumberFormatter()
numberFormatter.numberStyle = NSNumberFormatterStyle.SpellOutStyle
var string = numberFormatter.stringFromNumber(100)


You can use the fact that d.keys() and d.values() are always aligned. This gets rid of both the awkward way of constructing the values as well as the sorting of the keys.

def dict_product(d):
    keys = d.keys()
    for element in product(*d.values()):
        yield dict(zip(keys, element))

The assignment of keys is not strictly necessary, but it avoids the look-up every loop iteration.

Note that list(d[k] for k in sorted_keys) should have been written as a list comprehension directly, instead of calling list on a generator comprehension. So [d[k] for k in sorted_keys].

Your doctest might fail in Python 2 or in Python implementations other than cPython. This is because only recently have dictionary keys become ordered (by insertion time) in (c)Python 3. This is still an implementation detail and not something you should rely upon. Since doctest performs a string comparison, it does not notice that the objects are actually the same.

The only way around this, at least the only one I can see, is to use a more sophisticated testing framework, like unittest. This comes with more code overhead, though.


Multiples of 3 and 5 from Euler code challenge

Don't use or. It's for flow control. Use || for boolean logic.
Don't use Array.new, use []
Don't use (1..999).to_a or Array(1..999) when all you really want to do is iterate. It's expensive to instantiate a 1000 item array when really you just want to iterate from 1 to 999
Any time you find yourself declaring an empty array and pushing items onto it in a loop, you are probably missing a map or select:

def multiples
  numbers = Array(1..999)
  multiples = Array.new
  for i in numbers
    if i%3 == 0 or i%5 == 0
      multiples.push(i)
    end
  end
  multiples
end

Could be better written this way:

def multiples
  (1..999).select do |i|
    i % 3 == 0 || i % 5 == 0
  end
end

Your summation loop follows a similar pattern:

def sumMultiples(multiples)
  total = 0
  multiples.each { |i| total+= i }
  puts(total)
end

Again, there is a more idiomatic way of doing this via inject:

def sumMultiples(multiples)
  total = multiples.inject(&:+)
end

A breakdown of how this works can be found in How to sum array of numbers in Ruby?.


I am not really familiar with Fparsec, but a few of your functions could be simplified, or written better:

For example :

let pcomp = sepBy pHl7Element (pchar '&') |>> (fun vals -> List.mapi (fun i s -> {value = s; position = i}) vals)

Using partial application, it could be written as :

let pcomp = sepBy pHl7Element (pchar '&') |>> (List.mapi (fun i s -> { value = s; position = i }))

And the one after it:

// Before
let pfield = sepBy pcomp (pchar '^') |>> (fun comps -> List.mapi (fun i c -> {subcomponents = c; position = i}) comps)
// After
let pfield = sepBy pcomp (pchar '^') |>> (List.mapi (fun i c -> { subcomponents = c; position = i }))

Also, (fun x -> x) could be simply replaced by id.

let pheader = anyString 3 |>> id

On other matters, there is a potential bug in your code : in your pRepsOrField function, you're matching field.Length with 0 then extracting the first item, which will crash your code. Using the Length property to find out the length is .. unusual. You don't even need it. Pattern matching with lists based on length in F# is usually done this way:

match fields with
| [] -> // do something
| [ c ] ->  SingleField { components = c; position = 0 }
| _ -> Repetitions(List.mapi (fun i c -> { components = c; position = i }) fields))

Now you don't have to use extra methods/properties to get the length and and the first item. It is all right there ...


I don't like the internal storage type of (Hour/Min/Seconds).

private :
    int hour;
    int minute;
    int second;

This makes it hard to do comparison move forward backwards etc. I personally just store the number of seconds since midnight.

 private:
    int secondsSinceMidnight;

The converse to this is that usage may make this inefficient. If your main usage is just printing rather than modifying it may be worth keeping it as Hours/Minutes/Seconds (but I would measure before jumping to that conclusion as it is unlikely the main slowness in printing).

I don't like the default time being midnight.

    Time(const int h = 0, const int m  = 0, const int s = 0);    //with default value

If I construct a time object with no explicit value I usually expect it to be the current time. So personally I may default minutes and seconds to zero but I would expect at least the hours to be set. I would then provide a default constructor that sets the object to the current time:

    Time();  // Current time.
    explicit Time(const int h, const int m  = 0, const int s = 0);

Don't like Getters/Setters (they break encapsulation). Also I don't really see the use of this setter over the constructor.

    Time  x(5, 6, 7);

    // Now at a latter point I want to reset `x`;
    x = Time(8, 9, 10);

    // Performs equivalent action to:
    x.setTime(8, 9, 10);

When you provide a print method it is usally a good idea to also pass as an argument the stream you want to print on. This can default to std::cout but at least provide it.

    void print(std::ostream& str = std::cout) const;       // Print a description of object in " hh:mm:ss"

This also lets you define the standard output operator for the class very easily.

    std::ostream& operator<<(std::ostream& str, Time const& data)
    {
        data.print(str);
        return str;
    }

Now you can print using the method. Or the more more normal C++ technique.

    Time  x(5,7,8);
    x.print();              // defaults to std::cout
    x.print(std::cout);     // Or explicitly specify a stream.
    std::cout << x;         // Or just stream to the output.

Fine to have a named function (but pass parameter as const reference).

    bool equals(Time const& other) const;     //compare two time object

But why not define the equality operator as well. It makes the code more natural to read.

    bool operator==(Time const& lhs, Time const& rhs)
    {
         return lhs.equals(rhs);
    }

You don't need to manually put a zero into each field.

       (hour < 10) ? std::cout << 0 << hour : std::cout << hour;

There is a standard manipulator that does that automatically.

   std::cout << std::setw(2) << std::setfill('0') << h << ":"
             << std::setw(2) << std::setfill('0') << m << ":"
             << std::setw(2) << std::setfill('0') << s << "\n";