Finds the absolute max of the numbers in each file and outputs in specific format Inputs:
Files that are named in this scheme:
FAX_99383.txt- (txt files with columns of any width)

==> FAX_99383.txt <== (two rows of 12 numbers)

-93.    -95.    -97.    -96.    -95.    -93.    -91.    -90.    -89.    -88.    -87.    -89.
-99.   -101.   -103.   -102.   -101.    -99.    -97.    -96.    -94.    -93.    -93.    -95.

==> SH1_99383.txt <== (two rows of 12 numbers, like the above)

-2700.  -2677.  -2667.  -2649.  -2619.  -2593.  -2571.  -2552. -2536.  -2523.  -2527.  -2620.
-2788.  -2766.  -2755.  -2738.  -2709.  -2681.  -2659.  -2641.  -2624.  -2610.  -2614.  -2707.

To call

python3.5 maxtxt.py FAX_99383.txt SH1_99383.txt

Output:
MAXES.TXT-

Element    FAX     SH1
   99383   108.0  2923.0

Code works with multiple elements

import numpy as np
import sys

# Retrieve command line txt file entries to process
lis = sys.argv[1:]

# Loop over each txt file and determine the abs max and store in a dictionary
MaxLoads = {}
for file in lis:
    f = open(file, "r")
    X = np.loadtxt(f)
    dof = (file.partition('_'))[0]
    elemid = (file.partition('_'))[2].partition('.')[0]
    if elemid not in MaxLoads:
        MaxLoads.update({
            elemid: {}
        })
    MaxLoads[elemid].update({
        dof: max(X.max(), abs(X.min()))
    })
    f.close()

# Loop over each element and print each max in one line
fw = open("MAXES.TXT", "w")
e = list(MaxLoads.keys())[0]
doflis = sorted(list(MaxLoads[e].keys()))
oline = 'Element    ' + '     '.join(doflis)
fw.write(oline + '\n')
print(oline)
for e in MaxLoads:
    oline = '{:>8}'.format(e)
    for d in doflis:
        oline += '{:>8}'.format(MaxLoads[e][d].__str__())
    print(oline)
    fw.write(oline + '\n')
fw.close()

I'm looking for general feedback on if I am using the best methods to accomplish this goal in Python 3 (both stylistically and effectively). If there is a better language that offers a better solution, those comments are welcome too.  This code is my first operational Python code, goal is to learn Python.

What I would do in no particular order:

Use argparse for noob compatibility (also known as usability).
Run the file through pep8, then understand and apply everything it suggests (for example lower_case variable names). If you keep doing this you will reduce the mental overhead of anyone familiar with Python when dealing with your code (including your future self).
Pull out a minimalist main().
Create a class with instance methods to get the max number in a file and return a list of values (rather than printing them).
Print/save the return value in main() or a dedicated function. For scriptability the output should be as simple as possible, with no "fluff" like headers, and predictable, such as always integers with no surrounding punctuation.
I don't think you need to use .__str__() when formatting - Python knows it needs to convert to strings.
Use a context manager for opening files: with open(file, mode) as file_pointer.
Use variable names which are immediately understandable (see previous point).
Accumulating text into variables before printing/saving them wastes memory (very little in your case, but even so). Shift the bytes as quickly as possible. This way you'll also remove the need for intermediate variables, and can see the result of long-running processes as soon as any part of them is done.
I'm not very familiar with numpy, but it looks like you're reading the whole file before starting to process it. Treating files as streams instead will improve the speed and especially memory use when dealing with large files.


Transform a Binary Search Tree into a Greater Sum Tree Given a Binary Search Tree (where all nodes on the left child branch are less than the node), and all nodes to the right are greater/equal to the node), transform it into a Greater Sum Tree where each node contains sum of it together with all nodes greater than that node. Example diagram is here:

Looking for code review, optimizations and best practices.

public class GreaterSumTree implements Iterable {

    private TreeNode root;

    public GreaterSumTree(List<Integer> list) {
        create(list);
    }

    private void create (List<Integer> items) {
        root = new TreeNode(items.get(0));

        final Queue<TreeNode> queue = new LinkedList<TreeNode>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode current = queue.poll();
                final int left = 2 * i + 1;
                final int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    public static class TreeNode {
        private TreeNode left;
        private int item;
        private TreeNode right;

        TreeNode(int item) {
            this.item = item;
        }
    }

    public static class IntObject {
        private int sum;
    }

    /**
     * Computes the greater sum, provided the tree is BST.
     * If tree is not BST, then results are unpredictable.
     */
    public void greaterSumTree() {
        if (root == null) {
            throw new IllegalArgumentException("root is null");
        }
        computeSum (root, new IntObject());
    }

    private void computeSum(TreeNode node, IntObject intObj) {
        if (node != null) {
            computeSum(node.right, intObj);
            int temp = node.item;
            node.item = intObj.sum;
            intObj.sum = intObj.sum + temp;
            computeSum(node.left, intObj);
        }
    }

    /**
     * Returns the preorder representation for the given tree.
     *
     * @return  the iterator for preorder traversal
     */
    @Override
    public Iterator iterator () {
        return new PreOrderItr();
    }

    private class PreOrderItr implements Iterator {
        private final Stack<TreeNode> stack;

        public PreOrderItr() {
            stack = new Stack<TreeNode>();
            stack.add(root);
        }

        @Override
        public boolean hasNext() {
            return !stack.isEmpty();
        }

        @Override
        public Integer next() {
            if (!hasNext()) throw new NoSuchElementException("No more nodes remain to iterate");

            final TreeNode node = stack.pop();

            if (node.right != null) stack.push(node.right);
            if (node.left != null) stack.push(node.left);

            return node.item;
        }

        @Override
        public void remove() {
            throw new UnsupportedOperationException("Invalid operation for pre-order iterator.");
        }
    }
}

Here is a test case:

public class GreaterSumTreeTest {

    @Test
    public void test() {
        Integer[] a = {1, 2, 7, 11, 15, 29, 35};
        GreaterSumTree greaterSumTree = new GreaterSumTree(Arrays.asList(a));
        greaterSumTree.greaterSumTree();

        int[] expected = {71, 87, 89, 72, 35, 42, 0};
        int[] actual = new int[a.length];
        int counter = 0;
        Iterator itr = greaterSumTree.iterator();
        while (itr.hasNext()) {
           actual[counter++] = (Integer) itr.next();
        }
        assertTrue(Arrays.equals(expected, actual));
    }

}

Compiler Warnings

  Iterable is a raw type. References to generic type Iterable should be parameterized

This is very easy to fix. It's horrible that you haven't fixed it already. I'm sure that you know what generics is by now so I don't need to explain it to you. Whenever you see this warning, add generics!

Iterable --> Iterable<Integer>

Problem solved!

Then you can get rid of the (Integer) cast next to itr.next();

Why Binary Search Tree?

My first thought, once I understood the problem your solving was: Why on earth do you have the data as a Binary Search Tree? What does the tree have to do with anything?

If this was a question given to me at an interview, I would ask them this. Why a Binary Search Tree? The operations you're doing is totally unrelated to a tree. You could just as well perform this on a regular int[]. If there is a good reason for why it has to be a Binary Search Tree then please explain it to me.

Here's a simple implementation of how to perform this on a regular int[], without using a Binary Tree:

public static void main(String[] args) {
    int[] input = new int[] { 1, 2, 7, 11, 15, 29, 35, 40  };
    int[] expected = new int[] { 139, 137, 130, 119, 104, 75, 40, 0 };
    transformToGreaterSum(input);
    System.out.println(Arrays.toString(input));
    System.out.println(Arrays.toString(expected));
}

private static void transformToGreaterSum(int[] input) {
    int sum = 0;
    for (int i = input.length - 1; i >= 0; i--) {
        int prevSum = sum;
        sum += input[i];
        input[i] = prevSum;
    }
}

Note that this is also \$O(n)\$ without using any extra space.

Broken Tree

I'd also like to point out that your actual search tree does not match your image. I found out by debugging your code that your actual search tree looks like this:

The fact that it looks like this, and the fact that your algorithm works at all, is just because your input is sorted: {1, 2, 7, 11, 15, 29, 35}.

Your tree is in fact not a correct Binary Search Tree. A correct tree could look like this:

Yes, that's still a tree. It just doesn't have any left nodes (it does have seven nodes left though, I didn't remove any). A child node that is bigger than its parent should be on the right, which makes this tree right. (I love the English language)

As if that was not enough, your int[] expected says:

int[] expected = {71, 87, 89, 72, 35, 42, 0};

But your image says:

119, 137, 75, 139, 130, 104, 0, 40

Given your description:

  Given a BST, transform it into greater sum tree where each node contains sum of all nodes greater than that node. Diagram is here.

I would expect the node for 11 to get the value 15 + 29 + 35 + 40 = 119. I don't see 119 anywhere in your expected or actual result though...

Verifying a Binary Search Tree

As your question contains an incorrect tree and one of your comments below has an incorrect tree, I'd like to provide you with a method to verify a correct tree:

private static void verifyNode(TreeNode node) {
    if (node.left != null) {
        if (node.left.item >= node.item) {
            throw new IllegalStateException("node.left >= node: " + node.left.item + " >= " + node.item);
        }
        verifyNode(node.left);
    }
    if (node.right != null) {
        if (node.right.item <= node.item) {
            throw new IllegalStateException("node.right <= node: " + node.right.item + " <= " + node.item);
        }
        verifyNode(node.right);
    }
}

Just pass it your root node and it will verify your tree recursively. Note that there are two things this method does not verify, that are also a requirement for a Binary Search Tree:

There must be no duplicate nodes.
A unique path exists from the root to every other node.

Arrays and Lists

Speaking of int[] I wonder why your constructor takes a List<Integer> instead of an array: int[]. I don't think you gain anything by using a list.

The only thing you gain at the moment by using Integer rather than int is the possibility for this:

if (items.get(i) != null) {

As your input never contains null (and I see no reason why it should), I don't get the point of using that non-null check. I modified your code to use int[] instead and removed the non-null checks and all worked well still. It even got rid of some indentation steps and felt like a cleanup overall.

Iterate over what?

If you would have your Iterator as Iterator<TreeNode> instead of Iterator<Integer> you could use your iterator in your computeSum method as well as in your test method to fetch the results. This would make you have to modify your iterator implementation though (but as your tree is not a real binary search tree, it's quite broken already).

Or you could just ignore the tree structure and use arrays....

Summary

Given how many trees I've seen you implement, I really thought your code would be better than this. Your current code doesn't live up to your requirements, or your requirements are messed up. Additionally, the "raw type" mistake is a mistake one just should not make after having written just a few Java classes.


Easy Bit IO. Very simple interface works up to N == 64 After reading this question Easy bitset I/O. I wanted to see if there was a simpler way of doing it.

This is how I would define a way of outputting std::bitset<N> to the stream and reading it back in. It works perfectly with current containers.

BitIO.h

// Include guards deliberately not included for simplicity.

#include <bitset>
#include <iostream>

template<int S>
class BitIO
{
    public:
    std::bitset<S>&     data;
    BitIO(std::bitset<S>& data)
        : data(data)
    {}

    friend std::ostream& operator<<(std::ostream& stream, BitIO const& value)
    {
        // Currently using just an integer.
        //
        // But we could easily take this a step further and use hex.
        // and pad with 0 to get an exact width (that way we don't need a
        // trailing space).
        //
        // Using a fixed width means we may potentially waste space but can
        // be made to waste the least amount of space possible quite easily.
        //
        // Amount of wasted space would be (4 - (S % 4)) % 4

        return stream << value.data.to_ullong() << " ";
    }
    friend std::istream& operator>>(std::istream& stream, BitIO& value)
    {
        // As the current output uses a white space separated
        // unsigned long long it is easy to read it back in using the
        // normal `operator>>`
        //
        // Would be just as simple to convert this function to use
        // a fixed width format to mirror the output.
        unsigned long long tmp;
        if (stream >> tmp)
        {
            value.data  = std::bitset<S>(tmp);
        }
        return stream;
    }
};

main.cpp

#include "BitIO.h"
#include <vector>

int main()
{
    std::vector<std::bitset<16> >    values { std::bitset<16>(1), std::bitset<16>(2), std::bitset<16>(255) };

    std::copy(std::begin(values), std::end(values), std::ostream_iterator<BitIO<16>>(std::cout));

    std::bitset<16>  data { 129 };
    std::cout << BitIO<16>(data);

}

The code is relatively short and well-written, so I didn't find many nits to pick.  With that said, here's my review:

Include all needed files

Since std::ostream_iterator is declared in <iterator> that should be among the includes in main:

#include <iterator>

Use appropriate type for template parameter

The std::bitset template is defined as taking std::size_t rather than int, so it would make sense to define the BitIO template similarly:

template<std::size_t S>
class BitIO
{
    // ...
};

One can, of course, still abuse the interface by passing negative numbers but at least the user of the code is forewarned that they will be interpreted as std::size_t rather than as signed numbers.

Consider implementing a move constructor

At the moment, this potential usage cannot compile:

std::cout << BitIO<16>(std::bitset<16>{ 99 });

It's easily fixed by adding this constructor:

BitIO(std::bitset<S>&& data)
    : data(data)
{}

Maintain parallelism with existing practice

I was surprised to see spaces separating the outputs when none had been specified in the output ostream_iterator.  Although having the space is arguably useful, the choice of separator (if any) should be left up to the user of the template instead of being hardcoded in the operator << function.  I'd omit the trailing space to allow usage like this:

std::copy(std::begin(values), std::end(values),
        std::ostream_iterator<BitIO<16>>(std::cout,":"));


Shopping cart logic for authenticated users and guests I am tasked with cleaning up someone else's Objective-C code, and will admit it is not my language of choice. I am not sure if this is a valid question but was hoping someone could double check my attempt to clean up a method written by someone else, and make sure I didn't miss some minor detail or over simplify the code.

The original method written by someone else

if([[UserManager sharedManager]isUserLoggedIn] && ([APPDELEGATE isCommerceZone]))// Auth and commerce
    {
        [self showPopUpAddToCartOrExpressOrder:NO];

        UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification, _addToCartOrExpressOrderView);

    }
    else if([[UserManager sharedManager]isUserLoggedIn] && (![APPDELEGATE isCommerceZone])) // Auth and noncommerce
    {
          [self onModifyItemDone:nil];

    }
    else if (![[UserManager sharedManager]isUserLoggedIn] && ([APPDELEGATE isCommerceZone]))// guest and commerce
    {
        [self showPopUpAddToCartOrExpressOrder:YES];
       UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification, _addToCartOrExpressOrderView);

    }
    else if (![[UserManager sharedManager]isUserLoggedIn] && (![APPDELEGATE isCommerceZone]))// guest and noncommerce
    {
        /// Do nothing

    }

My revision

if([APPDELEGATE isCommerceZone])
    {

        [self showPopUpAddToCartOrExpressOrder:![[UserManager sharedManager]isUserLoggedIn]];

        UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification,_addToCartOrExpressOrderView);

    } else if([[UserManager sharedManager]isUserLoggedIn]) // Auth and noncommerce
    {
        [self onModifyItemDone:nil];
    }

In Objective-C, is this the equivalent to saying this boolean equals the opposite of this other boolean?

[self showPopUpAddToCartOrExpressOrder:![[UserManager sharedManager]isUserLoggedIn]];

There are two primary concerns I have.

First, rather than accessing [[UserManager sharedManager]isUserLoggedIn] twice, let's access it just once:

BOOL loggedIn = [[UserManager sharedManager] isUserLoggedIn];

Now just use the loggedIn variable in place of calling these methods.

In this case, it's not going to make a huge difference, but it's a good habit to get into.  If you're going to use the result of a method call multiple times within a method, call the original method once and save the result to a local variable.  This can make the code more clear (with appropriately named local variables) and will be more efficient with more complex code.

Second, APPDELEGATE, I'm going to assume, is a #define-ed macro.  You should eliminate uses of this macro.  For one, you should just generally be avoiding #define-ing stuff, and for two, this runs into the same trouble as calling that isUserLoggedIn method repeatedly, but masks it behind APPDELEGATE.

Given that you're cleaning up someone else's code, I recommend just eliminating uses of the macro, but don't actually delete the macro definition until you've cleaned them all up eventually.

Instead, use a function that returns the app delegate reference.  Call that function once and store a local reference to it for future uses within the method.

A function would look very similar to the macro:

MyAppDelegate * MyAppDelegate() {
    return [[UIApplication sharedApplication] delegate];
}

Now...

MyAppDelegate *appDelegate = MyAppDelegate();

And use appDelegate as your reference to app delegate.


Project Euler Problem #3 in Java - Largest Prime Factor I have solved the problem and it gives me the right output when the BigInteger has the smaller value, however it kept running for more than 20 minutes with the larger value without printing out an answer. So I'm thinking that there should be a faster algorithm for this program.

The problem states:

  The prime factors of 13195 are 5, 7, 13 and 29.

  What is the largest prime factor of the number 600851475143 ?

Main Method:

printPrimeFactors(new BigInteger("13195"));

Output:

  29 13 7 5

  BUILD SUCCESSFUL (total time: 0 seconds)

But when I change the value:

Main Method

printPrimeFactors(new BigInteger("600851475143"));

The program keeps running for a long time (more than 20 minutes), I didn't wait for an answer and stopped it.

Here is the rest of the program:

public static boolean isPrime(BigInteger n) {
    if(n.compareTo(new BigInteger("2")) == -1) return false;
    if(n.equals(new BigInteger("2")) || n.equals(new BigInteger("3"))) return true;
    if(n.remainder(new BigInteger("2")).equals(BigInteger.ZERO) || n.remainder(new BigInteger("3")).equals(BigInteger.ZERO)) return false;
    BigInteger sqrtN = bigIntSqRootFloor(n).add(BigInteger.ONE);

    for(BigInteger i = new BigInteger("6"); i.compareTo(sqrtN) == 0 || i.compareTo(sqrtN) == -1; i = i.add(new BigInteger("6"))){
        if(n.remainder(i.subtract(BigInteger.ONE)).equals(BigInteger.ZERO)) return false;
        else if(n.remainder(i.add(BigInteger.ONE)).equals(BigInteger.ZERO)) return false;
    }
    return true;
}

public static void printPrimeFactors(BigInteger number){

    for(BigInteger i = BigInteger.ZERO.add(number);!i.equals(new BigInteger("1")); i = i.subtract(BigInteger.ONE)){
        if(isPrime(i) && number.remainder(i).equals(BigInteger.ZERO)){
            System.out.print(i + " ");
            number = number.divide(i);
            printPrimeFactors(number);
            break;
        }
    }
}

public static BigInteger bigIntSqRootFloor(BigInteger x)
    throws IllegalArgumentException {
    if (x.compareTo(BigInteger.ZERO) < 0) {
        throw new IllegalArgumentException("Negative argument.");
    }

    if (x .equals(BigInteger.ZERO) || x.equals(BigInteger.ONE)) {
        return x;
    }

    BigInteger two = BigInteger.valueOf(2L);
    BigInteger y;

    for(y = x.divide(two);
            y.compareTo(x.divide(y)) > 0;
            y = ((x.divide(y)).add(y)).divide(two));
    return y;
}

Counting downwards hurts the performance extremely as you are checking way more values than you have to (e.g. for this value you have to iterate only to the second largest prime number (1471) to know the largest prime number).

final long t0 = System.nanoTime();
final long factor = largestPrimeFactor(600851475143L);
final long t1 = System.nanoTime();
System.out.println(t1 - t0);
System.out.println(factor);

Results (significantly faster):

19071
6857

The code in largestPrimeFactors is a bit modified version of a normal primeFactor method (iterating only over values that are not a multiple of 2 or 3):

public static long largestPrimeFactor(
        final long input) {
    ////
    if (input < 2)
        throw new IllegalArgumentException();

    long n = input;
    long last = 0;
    for (; (n & 1) == 0; n >>= 1)
        last = 2;
    for (; n % 3 == 0; n /= 3)
        last = 3;
    for (long v = 5, add = 2, border = (long) Math.sqrt(n); v <= border; v += add, add ^= 6)
        while (n % v == 0)
            border = (long) Math.sqrt(n /= last = v);
    return n == 1 ? last : n;
}


Get a map containing elements which are present in unordered map A but are not present in unordered map B I would like to create a function which returns a map containing differences between two unordered maps. Here is the code:

std::map<std::string, int> getDiff(const std::unordered_map<std::string, int>& unordered_mapA,
                                   const std::unordered_map<std::string, int>& unordered_mapB)
{
    std::map<std::string, int> mapA(unordered_mapA.begin(), unordered_mapA.end());
    std::map<std::string, int> mapB(unordered_mapB.begin(), unordered_mapB.end());;
    std::map<std::string, int> diffMap;

    std::set_difference(mapA.begin(), mapA.end(), mapB.begin(), mapB.end(), std::inserter(diffMap, diffMap.end()));

    return diffMap;
}

Edit:

A result should contain elements which are present in mapA but are not present in mapB.

At first sight, this looks good - you let a standard library algorithm and two standard library class constructors do the heavy lifting. But this approach ships with a drawback, especially the linear (given that the input is sorted) copy construction of the two local std::maps can be quite a bottleneck, e.g. when the keys are long enough to not fit into the SSO buffer. Together with std::set_difference, you have a time complexity of approximately O(N1 + N2) (neglecting constant factors) plus that of the insertion into the resulting map (which is inevitable).

You could consider a simpler, more manual approach. Recall that element lookup by a key is constant time in a hash map, so doing that inside a loop over all elements doesn't add much overhead to the function:

std::map<std::string, int> getDiff(const std::unordered_map<std::string, int>& src,
        const std::unordered_map<std::string, int>& exclude)
{
    std::map<std::string, int> diff;

    for (const auto& node : src) {
        const auto lookup = exclude.find(node.first);

        if (lookup == src.cend() || node.second != lookup->second)
            diff.insert(node);
    }

    return diff;
}

First note that I have renamed the function arguments here. Whether src and exclude is the clearest choice might be debatable, but it should be better than unordered_mapA and unordered_mapB, which duplicates info already present in the type of these arguments. This has an improved time complexity of O(N1) (again, without constant factors or counting std::map::insert).

This can be taken one step further to make it more reusable, as there aren't any specific requirements on key and value types that should be nailed down (besides the ability to check whether objects of the value type compare equal).

template <class Key, class Value>
std::map<Key, Value> getDiff(const std::unordered_map<Key, Value>& src,
        const std::unordered_map<Key, Value>& exclude)
{
    // Exactly as above...
}

You can use type deduction for saving some keyboard strokes upon the invocation:

std::unordered_map<std::string, int> a{/* ... */};
std::unordered_map<std::string, int> b{/* ... */};

const auto diff = getDiff(a, b));

And one last, idiomatic level of indirection could then be to not commit to a specifiy return type, but instead pass an output iterator that accepts a std::pair<Key, Value>:

template <class Key, class Value, class OutputIt>
OutputIt getDiff(const std::unordered_map<Key, Value>& src,
        const std::unordered_map<Key, Value>& exclude, OutputIt dest)
{
    // As above...

        if (...)
            dest = node; // instead of diff.insert(node);

    return dest;
}

Here, it is convention to return the output iterator at the end of the function. You can instantiate and invoke the above template as follows.

std::map<std::string, int> diff;
getDiff(a, b, std::inserter(diff, diff.end()));

Or, if you later decied that you need cache locality for the result of the operation:

std::vector<std::pair<std::string, int>> seqDiff;
getDiff(a, b, std::inserter(seqDiff, seqDiff.end()));

Note that you still want more abstraction, you could also hide the src argument behind input iterators, as the loop over this container isn't specific to the hash map type.


Display Fibonacci numbers I just started coding programs with python. As a challenge I wanted to program something to print out the Fibonacci Numbers. It looks like I've done what I wanted to do (I checked the first few numbers) and because I just started coding, I obviously don't know if this is a legitimate code and somebody would code it like that. Keep in mind that I have only done some basic lessons in python so I just want to get some feedback and advice for better code.

def spiral(sequence):
    numbers = []
    for i in sequence:
        numbers.append(0)
        if i == 0 or i == 1:
            numbers[i] = 1
            continue
        numbers[i] = numbers[i-1] + numbers[i-2]
    return numbers
times = range(int(input("How many times do you want to go through? ")))
fibonacci = spiral(times)
print(fibonacci)

The way sequence is used guarantees that the results will be wrong for the majority of inputs. It's clearly intended that sequence should be a continuous range starting at 0. So rather than require the range as input, it would make more sense to require the limit as input.

        numbers.append(0)
        if i == 0 or i == 1:
            numbers[i] = 1
            continue
        numbers[i] = numbers[i-1] + numbers[i-2]

This confused me briefly until I figured out that it appends 0 as a placeholder and then overwrites it with the appropriate Fibonacci number. Following the principle of KISS (keep it simple), it would make more sense to append the Fibonacci number directly.

IMO the expression is short enough to use a ternary:

        numbers.append(numbers[i-1] + numbers[i-2] if i > 1 else 1)

Those two changes give

def spiral(n):
    numbers = []
    for i in range(n):
        numbers.append(numbers[i-1] + numbers[i-2] if i > 1 else 1)
    return numbers
times = int(input("How many times do you want to go through? "))
fibonacci = spiral(times)
print(fibonacci)

The names seem quite opaque. Why spiral? Why times? They both seem to assume some context which isn't documented in the code.

It would be more Pythonic to write a generator, but since you say you're a beginner this may be a concept which you haven't encountered yet.


PHP alternative syntax confusion Newbie to PHP.

Trying to output the results of an array inside a foreach loop. I have got the following code but I cannot understand what to do with it next.

Mainly;

Where/how do I start and end the php statements?
Is there a better way to format this code?

I want to make it as easy to understand and legible as possible.

I'm not sure how to echo my rows within the specified html below?

My code so far;

<?php foreach ($userResults as $row) : ?>

echo $row['user_id'] here

echo $row['user_name'] here

echo $row['user_email'] here

<?php endforeach; ?>

Any help or advice is appreciated.

I dont think mixing html inside PHP the way Tim Penner does in his first example is very readable or maintainable, i'd prefer the way you're trying to do it.

If you're using PHP 5.4+ and do not need the code to be working in earlier php versions i'd recommend you use the short_open_tag <?= to echo out variables easily.

<?php foreach ($userResults as $row) : ?>

<?= $row['user_id']; ?>here

<?= $row['user_name']; ?>here

<?= $row['user_email']; ?>here

<?php endforeach; ?>

This is in my mind more readable and maintainable.


Maximum subset sum with no adjacent elements Write a function that takes an array of integers (Both Positive and negative) and return the maximum sum of non adjacent elements. Note that even if all values are negative, I don't have an option to choose empty subset and return sum as 0.
Recursive solution that I wrote:
public static int maxSubsetSumNoAdjacent(int[] array) {
  if (array.length == 0) return 0;
  else if (array.length == 1) return array[0];
  else if (array.length == 2) return Math.max(array[0], array[1]);

  return Math.max(
      array[0] + maxSubsetSumNoAdjacent(Arrays.copyOfRange(array, 2, array.length)),
      array[1] + maxSubsetSumNoAdjacent(Arrays.copyOfRange(array, 3, array.length))
  );
}

The recursive solution works. I've tried an iterative solution, but that didn't work.
Test cases
Positive Numbers:

75, 105, 120, 75, 90, 135
Ans : 330 (75 + 120  + 135)

Negative Numbers:

-1, -10, -10, -1, -2
Ans: -2 (-1 + -1)

I'd like a review of the code provided.

Many folks find that tacking on { } braces
to even a single-line if body is a useful way of preventing future bugs.

You wrote:

  array[0] + maxSubsetSumNoAdjacent(Arrays.copyOfRange(array, 2, array.length)),
  array[1] + maxSubsetSumNoAdjacent(Arrays.copyOfRange(array, 3, array.length))

The pair of copy statements is wasteful.
You're allocating temp storage (for GC to collect)
and consuming memory bandwidth.
It turns what could be a linear algorithm into a quadratic one (O(n) → O(n^2)).
The caller is going to simply hand you an array,
so you have to conform to that public API, accepting a single argument.
But nothing stops you from overloading, from writing a private helper.
Your public function should immediately ask the helper about the array,
from index 0 onward to end of array.
Then all the work happens in the helper.
Note that recursive calls that pass array plus a starting index
will never copy the array.
They merely pass a pointer to start of original array, in O(1) time,
independent of how enormous that array happens to be.
You have an opportunity to dramatically improve the implemented algorithm,
by passing index to the "as yet unsolved" portion of the array.


Hiding $_GET variables using encryption I use the following functions to encrypt my $_GET variables (whenever I can't easily get away with using $_POST or some other way of passing information between pages).

function decryptStringArray ($stringArray, $key = "Your secret salt thingie")
{
    $s = unserialize(rtrim(mcrypt_decrypt(MCRYPT_RIJNDAEL_256, md5($key), base64_decode(strtr($stringArray, '-_,', '+/=')), MCRYPT_MODE_CBC, md5(md5($key))), "\0"));
    return $s;
}

function encryptStringArray ($stringArray, $key = "Your secret salt thingie")
{
    $s = strtr(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, md5($key), serialize($stringArray), MCRYPT_MODE_CBC, md5(md5($key)))), '+/=', '-_,');
    return $s;
}

function prepareUrl($url, $key = "Your secret salt thingie")
{
    $url = explode("?",$url,2);
    if(sizeof($url) <= 1)
        return $url;
    else
        return $url[0]."?params=".encryptStringArray($url[1],$key);
}

function setGET($params,$key = "Your secret salt thingie")
{
    $params = decryptStringArray($params,$key);
    $param_pairs = explode('&',$params);
    foreach($param_pairs as $pair)
    {
        $split_pair = explode('=',$pair);
        $_GET[$split_pair[0]] = $split_pair[1];
    }
}

Obviously I replace the "Your secret salt thingie" with other strings. Here is how I use it:

On the page where I need a URL:

$url = prepareUrl("http://someurl.com?variable1=1314&variable2=1851&variable3=stringstuff", "algjalgjalgjal");

Then I put the new $url in a href or a tag or something (I use $smarty templates but that isn't relevant).

On the page someurl.com where I need to decrypt the params I just use:

setGET($_GET['params'],"algjalgjalgjal");

This all works fine for me.  Is there anything inherently terrible about this way of doing things?  I'm asking this because I posted this as an answer on Stack Overflow to a question someone asked about hiding their $_GET parameters and it was immediately down-voted. That made me curious about whether it was somehow bad code or insecure in some way.

Agree 100% with Corbin. GET isn't something that is inherently secure, and trying to make it so is nigh impossible. That's what POST is for. Only use GET for non-sensitive information. That being said, there are some generally concerning bits to your code that could cause some people to downvote it.

Your line length is rather long and convoluted, which causes major issues with legibility. I would suggest breaking up that single line into multiple lines to more easily maintain and read. Adding whitespace couldn't hurt either. Additionally, using single letter variables is not very descriptive and also leads to issues with legibility. Its a bit longer, but much easier to read.

$translation = strtr( $stringArray, '-_,', '+/=' );

$key     = md5( $key );
$data    = base64_decode( $translation );
$iv      = md5( $key );
$decrypt = mcrypt_decrypt(
    MCRYPT_RIJNDAEL_256,
    $key,
    $data,
    MCRYPT_MODE_CBC,
    $iv
);

$trimmed = rtrim( $decrypt, "\0" );

return unserialize( $trimmed );

A couple more potential issues with the above code are the methods you are implementing. I am by no means a security guru and have not put a lot of research into the matter, but I seem to remember from somewhere that mcrypt is frowned upon and that hashing a string with md5() twice is actually less secure than doing it just once. I don't know if this is true or not, but that could be a potential issue.

Your code is also slightly repetitive, violating the "Don't Repeat Yourself" (DRY) Principle. As the name implies, your code should not repeat. Your encrypt and decrypt functions seem to share some common elements, using some shared helper functions to provide that similar data might be beneficial, even though you might just end up creating wrapper functions. I don't really see anyone downvoting you for this alone, this is rather minor in this instance and is rather hard to spot due to the above reasons.

Another potential issue I see is with your braceless syntax. Braceless {} syntax can be somewhat confusing to those who have never seen it before, and therefore could cause issues with maintainability. This is entirely a point of preference, but one I would argue most vehemently against. It is especially bad to offer in the form of an answer when it is unknown how the questioner will use it. It is quite possible they attempted to modify the code and could not get it to work.

The last issue I see is the way you are accessing array elements with magic numbers. This is rather sloppy and could cause issues with legibility, though I don't think you would get downvoted for it. There are a number of different ways this could be solved. The first is by using the PHP construct list(), which is probably preferable in this instance. The second is by using array functions to slice off the required portions of the array. This is more beneficial when you need the first and last elements of an array of undetermined length. Finally, there is also the possibility of using extract(), but that requires an associative array and is sometimes frowned upon. I won't show that last method because it doesn't apply here, but here are the other two:

//using list
list( $baseurl, $params ) = $url;

//using array functions
$baseurl = array_shift( $url );
$params  = array_shift( $url );//could potentially use array_pop()

return $baseurl . '?params=' . encryptStringArray( $params, $key );

Hope this helps!


LeetCode: maximum-depth-of-n-ary-tree https://leetcode.com/problems/maximum-depth-of-n-ary-tree/

Given a n-ary tree, find its maximum depth.

The maximum depth is the number of nodes along the longest path from the root node down to the farthest leaf node.

For example, given a 3-ary tree:

We should return its max depth, which is 3.

Note:

The depth of the tree is at most 1000.
The total number of nodes is at most 5000.

Here is my code, and also 1 test as an example
Please comment about space and time complexity, I also made a recursive answer but I tend to think about BFS in an iterative way while using queue, and DFS in recursive way, although you can use a stack for it as well.
so I a real interview I would probably go with BFS with a queue.

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Collections.Generic;

namespace TreeQuestions
{
    // Definition for a Node.
    public class Node
    {
        public int val;
        public IList<Node> children;

        public Node()
        {
        }

        public Node(int _val, IList<Node> _children)
        {
            val = _val;
            children = _children;
        }
    }

    /// <summary>
    /// https://leetcode.com/problems/maximum-depth-of-n-ary-tree/
    /// </summary>
    [TestClass]
    public class MaximumDepthOfN_aryTree
    {
        public int MaxDepth(Node root)
        {

            if (root == null)
            {
                return 0;
            }

            int maxDepth = 0;
            Queue<Node> queue = new Queue<Node>();
            queue.Enqueue(root);

            while (queue.Count > 0)
            {
                maxDepth++;
                var queueSize = queue.Count;
                for (int i = 0; i < queueSize; i++)
                {

                    var current = queue.Dequeue();
                    if (current.children != null)
                    {
                        foreach (var child in current.children)
                        {
                            queue.Enqueue(child);
                        }
                    }
                }
            }

            return maxDepth;
        }

        [TestMethod]
        public void MaximumDepthOfN_aryTreeTest()
        {
            List<Node> node3 = new List<Node>();
            node3.Add(new Node(5, null));
            node3.Add(new Node(6, null));

            List<Node> node1 = new List<Node>();
            node1.Add(new Node(3, node3));
            node1.Add(new Node(2, null));
            node1.Add(new Node(4, null));

            Node root = new Node(1, node1);
            int result = MaxDepth(root);
            Assert.AreEqual(3, result);
        }
    }
}

Your solution seems pretty optimal - for breadth-first traversal (see below)

MaxDepth() could/should be static.  The containing class too.
The LeetCode question does not require breadth-first traversal.
Depth first would not require an(y) additional data structure (no queue, but would implicitly use the stack)
which might be optimised away by tail-recursion (if C# is smart enough).

The following is therefore not criticism but a suggested alternative.
- I would not expect (much of) a performance difference
- Space-advantage, if significant (due to tail-recursion optimisation, compiler-and/or run-time environment dependent) would have to be measured
- So the main consideration in (not) choosing depth-first traversal would be a (probably) religious argument about brevity/readability of code

public static int MaxDepth2(IEnumerator<Node> nodeEnum = null, int depth = 0)
{
  //  No current node
  if (nodeEnum == null || !nodeEnum.MoveNext()) return depth;

  //  The greater of current and maximum of siblings
  using (var nodeEnumInner = nodeEnum.Current?.children?.GetEnumerator())
    return Math.Max(MaxDepth2(nodeEnumInner, depth + 1), MaxDepth2(nodeEnum, depth));
}

:
:
using (var nodeEnum = new List<Node> { root }.GetEnumerator())
  depth2 = MaximumDepthOfN_aryTree.MaxDepth2(nodeEnum);


POSIX shell function for asking questions I've written a tiny function for asking questions intended for my POSIX shell scripts, where I often need user input.

The function takes 2+ arguments, where:

Is a string containing the question.
, 3., ... = arguments containing the right answers (case insensitive)

I also changed my style by a bit, no longer use ${var} instead of just $var.

The requirement was simple: Check for exactly the answers given. So, not matching yeahh if given answer is yeah.

I also included a maybe performance-wise quick test if the user answer is in the list of answers, so answering no will not make the script iterate through all the answers and just dies at that check.

#!/bin/sh
set -u
confirmation ()
# $1    = a string containing the question
# $2,.. = arguments containing the right answers (case insensitive)
{
    question=$1; shift
    correct_answers=$*
    correct_answers_combined=$( printf '%s' "$correct_answers" | sed 's/\( \)\{1,\}/\//g' )
    printf '%b' "$question\\nPlease answer [ $correct_answers_combined ] to confirm (Not <Enter>): "
    read -r user_answer

    # this part is optional in hope it would speed up the whole process
    printf '%s' "$correct_answers" | grep -i "$user_answer" > /dev/null 2>&1 ||
        return 1

    # this part iterates through the list of correct answers
    # and compares each as the whole word (actually as the whole line) with the user answer
    for single_correct_answer in $correct_answers; do
        printf '%s' "$single_correct_answer" | grep -i -x "$user_answer" > /dev/null 2>&1 &&
            return 0
    done

    # this might be omitted, needs verification, or testing
    return 1
}

# EXAMPLE usage, can be anything, DO NOT review this part please
if confirmation 'Is dog your favorite pet?' y yes yep yeah
then
    tput bold; tput setaf 2; echo 'TRUE: You just love dogs! :)'; tput sgr0
else
    tput bold; tput setaf 1; echo 'FALSE: Dog hater, discontinuing! :('; tput sgr0
    exit 1
fi
# do other stuff here in TRUE case
echo 'And here comes more fun...'

A couple of thoughts. There really isn't any need for correct_answers_combined. After shift, $* will hold the combined remaining arguments (answers) to your question. The additional printf, pipe, and call to sed are simply incurring additional overhead in the form of subshells and separate utility calls. You could do:

    correct_answers="$*"

    ## prompt
    printf '%b' "$question\\nPlease answer [ $correct_answers ] to confirm (Not <Enter>): "
    read -r user_answer

Since you do not want to accept [Enter] as an answer, a validation that user_answer is unset can provide the answer and return for your function in that case, e.g.

    ## validate answer provided
    [ -z "$user_answer" ] && return 1

Your "optional... speed up the whole process", with a bit of rearranging can be used as a single call to provide a return to your function. Since you want to know whether the user_answer exists among the correct_answers you can simply return the result of grep -qi:

    # this part is can be the whole process
    printf '%s\n' $correct_answers | grep -qi "$user_answer" > /dev/null 2>&1
        return $?
}

(note: using printf '%s\n' "$correct_answers" will separate each of the whitespace separated answers by newline which eliminates any possible combination of parts of adjacent answers returning true)

With those suggestions, your confirmation () function would reduce to:

confirmation ()
# $1    = a string containing the question
# $2,.. = arguments containing the right answers (case insensitive)
{
    question="$1"; shift
    correct_answers="$*"

    ## prompt
    printf '%b' "$question\\nPlease answer [ $correct_answers ] to confirm (Not <Enter>): "
    read -r user_answer

    ## validate answer provided
    [ -z "$user_answer" ] && return 1

    # this part is can be the whole process
    printf '%s\n' "$correct_answers" | grep -qi "$user_answer" > /dev/null 2>&1
        return $?
}

I haven't tested all corner cases, but for single word answers it should function as you intend. If you want the some delimiter between the answers inside [ ... ] then an additional command substitution can be used, but space separated options appear fine between the brackets.

Let me know if you have any questions over the changes.

Edit In Response to You Comments

As stated above, if you want delimiters between the possible correct answers, simply use a command substitution, e.g.

## prompt
printf '%b' "$question\\nPlease answer [ $(echo $correct_answers | tr ' ' /) ] to confirm (Not <Enter>): "
read -r user_answer

If you are concerned about the expansion of printf '%s\n', don't quote the $correct answers,

# this part is can be the whole process
printf '%s\n' $correct_answers | grep -qi "$user_answer" > /dev/null 2>&1
    return $?

Otherwise based on your stated question, it performs identical to the original saving at least half-a-dozen unnecessary subshells.


What would be alternative ways to model IConvention? public class Item { /*...*/ }
public class Model { /*...*/ }

public interface IConventionInstanceSelector {
  IConventionInstance Select(Item item);
}

public interface IConventionInstance {
  void ApplyTo(Model model);
}

public interface IConvention {
  bool IsMatchedBy(Item item);
  IConventionInstance CreateInstance(Item item);
}

public class ConventionInstanceSelector : IConventionInstanceSelector {
  readonly List<IConvention<Item>> _conventions;

  public ConventionInstanceSelector(IEnumerable<IConvention<Item>> conventions) {
    _conventions = conventions.ToList();
  }

  public IConventionInstance Select(Item item) {
    foreach(var convention in _conventions) {
      if(convention.IsMatchedBy(item)) {
        return convention.CreateInstance(item);
      }
    }
    return NullConventionInstance.Instance;
  }
}

public class ModelReader {
  IConventionInstanceSelector _selector;

  public ModelReader(IConventionInstanceSelector selector) {
    _selector = selector;
  }

  public Model Read(IEnumerable<Item> items) {
    var model = new Model();
    foreach(var item in items) {
      _selector.Select(item).ApplyTo(model);
    }
    return model;
  }
}

I'm a bit concerned about the specification and factory method  being merged into one interface (IConvention) and that they can be called separately when in fact one depends on the decision of the other. I also get the feeling there's a pattern here I'm not seeing.

Just to give a little bit more context: I'm trying to build an API whereby people can build their own conventions that get applied to an input stream (an enumeration of items in this case) and have each convention apply a transformation on the input (the item in this case) and add it to a model. The main concern is the convention represents both the fact an item satisfies the convention as well as the logic to apply it to the model AND the fact that how I've modeled it right now, those are two distinct operations whereby the latter relies on the first to be able to do it's job properly. It's like specification pattern meets factory pattern.

First of all, the combination of a Specification and another method is not that unusual - it's called the Tester-Doer Pattern (although to be fair, I'm not sure whether it's a bona-fide pattern, as I've never seen it described outside of .NET). Even so, despite the precedence, I completely understand why you dislike the Temporal Coupling it implies. So which alternatives are available?

The first refactoring we could apply is to change the definition of IConvention to emply a Maybe monad. Assuming that we've defined a Maybe<T> class, IConvention now looks like this:

public interface IConvention
{
    Maybe<IConventionInstance> CreateInstance(Item item);
}

This would allow you to in a type-safe way to check whether or not an IConventionInstance instance could be created from the item.

However, once you realize that a Maybe monad really is nothing more than a special case of IEnumerable, you could consider refactoring IConvention once more:

public interface IConvention
{
    IEnumerable<IConventionInstance> CreateInstances(Item item);
}

In the normal implementation, an implementation would return zero or one item.

This would make the ConventionInstanceSelector and IConventionInstanceSelector types redundant (always a good thing), and you could implement the ModelReader class like this (caveat: I haven't tried to compile it):

public class ModelReader {
    IEnumerable<IConvention> _conventions;

    public ModelReader(IEnumerable<IConvention> _conventions) {
        _conventions = conventions;
    }

    public Model Read(IEnumerable<Item> items) {
        var model = new Model();

        var conventionInstances = from item in items
                                  from c in _conventions
                                  from ci in c.CreateInstances(item)
                                  select ci;
        foreach(var ci in conventionInstances) {
            ci.ApplyTo(model);
        }

        return model;
    }
}


Shopping List program This is a shopping list program made in Python 3 and it will send the list by email. I am not very experienced but I consider this program as a starting point to what I will do next.

import smtplib

from email.message import EmailMessage

from getpass import getpass

name = str(input("Name: "))
email = str(input("Email: "))
password = str(getpass("Password: "))
recipient = str(input("Recipient's email: "))

while True:
    try:
        productsnm = int(input(f"Hi, {name}!\nHow many products do you want to add to the shopping list? " ))
    except ValueError:
        print ("Please input a number.")
    else:
        break

products = []
quantities = []

for x in range(int(productsnm)):

    product = str(input("Input the product name: "))

    while True:
        try:
            quantity = int(input("Input the product quantity: "))
        except ValueError:
            print ("Please input a number.")
        else:
            break

    products.append(product)
    quantities.append(quantity)

print ("\nThese products have been added to the shopping list:")

for x, y in zip(products, quantities):
    print (f'{x} x {y}')

gmail_user = email
gmail_password = password

msg = EmailMessage()
msg['Subject'] = "Shopping List"
msg['From'] = gmail_user
msg['To'] = [recipient]
message = ""
for i in range(max(len(products), len(quantities))):
    message = message + str(products[i]) + " x " + str(quantities[i]) + "\n"
msg.set_content(message)

try:
    s = smtplib.SMTP_SSL('smtp.gmail.com', 465)
    s.ehlo()
    s.login(gmail_user, gmail_password)
    s.send_message(msg)
    s.quit()

    print ("\nThe email has been sent.")

except:
    print ("\nAn error occurred.")

print ("\nHave a nice day!")

exit()

Welcome to Code Review. While your script works as is, it is a good practice to setup and split parts of code into functions, and place the entry point inside the if __name__ wall.

You have an extra conversion to int for productsnm (which could be better named product_count).

Since each product is associated with its own quantity, say hello to a namedtuple:

from collections import namedtuple
Product = namedtuple('Product', ['name', 'quantity'])
# inside the for-loop:
    products.append(Product(name=product_name, quantity=quantity))

You wont need the string manipulation, or the loop. Just printing out the products list should give you something like

[Product(name='n0', quantity=0), Product(name='n1', quantity=1), Product(name='n2', quantity=32)]

Each element of the list now has name and quantity as its property as well.

You can have multiple functions as follows:

gather_products_list which returns a list of N products.
generate_email_body to process the products provided to the function and generating the raw email body.
send_email which accepts the aforementioned raw body, and sends it via the provider of choice.
get_integer which repeatedly asks the user to input an integer value. This function could accept an optional prompt message as well as an error message!


Finding pairs that add up to a given sum, using recursion I'm currently tasked with using recursion to find pairs that add up to a given sum. How would I make the function recursiveFixedSumPairs function more efficient and truncate this code? Also, what is the current runtime? I believe it is n since it will go through the array only once always starting at the first index.

public class HW4 {

    public static void main(String[] args) {

        int[] a = {1, 5, 8, 11, 14, 15, 20, 23, 25, 28, 30, 34};
        int k;
        k = 43;
        System.out.println("k = " + k);
        findFixedSumPairs(a, k);
    }

    public static void findFixedSumPairs(int[] a, int k) {
       recursiveFixedSumPairs(a, -1, 0, k);
    }

    private static void recursiveFixedSumPairs(int[] array, int subPair1, int index, int k) {
//        // pairs whose sum equals k are printed in this method
//        if ((array[subPair1] + array[subPair2]) == k) {
//            System.out.println("[" + array[subPair1] + ", " + array[subPair2] + "]");
//        } else if ((array[subPair1] + array[subPair2]) > k) {
//            recursiveFixedSumPairs(array, subPair1, subPair2 - 1, k);
//        } else {
//            recursiveFixedSumPairs(array, subPair1 + 1, subPair2, k);
//        }

            if (index == array.length) {
                return;
            }

            else if (index == array.length-1) {

                if (subPair1!= -1 && subPair1 + array[index] == k) {
                    System.out.println(""+subPair1 +" "+ array[index]);
                }

                else {
                    return;
                }
            }

            if (subPair1 != -1) {

                if (array[index] == k- subPair1) {
                    System.out.println(""+subPair1 +" "+ array[index]);
                    return;
                } else {
                    recursiveFixedSumPairs(array, subPair1, index + 1, k);
                }

            } else {
                recursiveFixedSumPairs(array, array[index], index+1, k);
                recursiveFixedSumPairs(array,-1, index+1, k);
            }
        }
}

Recursion is about dividing and conquering. You are going to check the first element of an array (E0) against each element in the rest of the array. Once you have checked all the elements, you pick the next element (E1) in the array and check it against all the other elements. Since you've already checked E1 against E0 in the previous iteration, you only need to check E1 against the elements after it.

First of all, your naming is unclear. The "fixed" has no meaning here, so let's drop it. Single letter parameter and argiment names are only reserved for loop counters. So "k" must become "expectedSum". The entry-function to the iteration just takes the user input. It adds the initial entry parameters for the iterative function (indexes to first entry in the array and the first entry of the sub-array. Since the entry function initializes the recursive function with valid values, there is no need to check for magic -1 values in the iterative function anymore.

public static void findSumPairs(int[] array, int expectedSum) {
    recursiveSumPairs(array, expectedSum, 0, 1);
}

private static void recursiveSumPairs(
        int[] array,
        int expectedSum,
        int firstIndex,
        int nextIndex) {

    // End condition for recursion: first element to check is the last
    // element in the array. This also handles input arrays shorter than
    // two elements.
    if (firstIndex >= array.length - 1) {
        return;
    }

    // We have exhausted the sub-array. Advance the first element to the
    // next index and compare it to the rest of the elements left in the
    // array.
    if (nextIndex >= array.length) {
        recursiveSumPairs(array, expectedSum, firstIndex + 1, firstIndex + 2);
        return;
    }

    if (array[firstIndex] + array[nextIndex] == expectedSum) {
        System.out.println(array[firstIndex] + " + " + array[nextIndex]);
    }

    // Compare first element to the next element in the array.
    recursiveSumPairs(array, expectedSum, firstIndex, nextIndex + 1);
}

My presonal preference is to use ">=" and "<=" instead of "==" when checking loop counters against end conditions. It implies that the left side of the comparison is a value that increases/decreases and guards against bugs where the counter increases by two. Although if you were really paranoid you could add special check for that and throw an IllegalStateException (the new kids on the block would probably use assertations).

Now, if you want to go fancy with the solution, instead of burdening the algorithm with display functions (System.out.printlning the result), you could pass a BiConsumer to the function and let the caller decide what to do with the results (possibly with a nice lambda).

findSumPairs(
    array,
    sum,
    (a, b) -> System.err.println(a + " + " + b));


Using AJAX to secure sessions and save messages I have been working with PHP sessions and login protected pages for some time, but I want to start using some AJAX for managing some actions inside login protected pages (easy way with jQuery).

Here is what I want to do:

The user logs in (classic no-AJAX method), and wants to submit a message. I make a request with AJAX to another page, where the session is checked and the message is saved.

Here is how the start page looks:

<?php
    session_name('NEWSESSION');
    session_start();
    session_regenerate_id(true);

    if(!$_SESSION['user_logged']){
            header("location: login.php");
    }

    $sessionTTL = time() - $_SESSION["timeout"];
    if ($sessionTTL > SES_LENGHT) {
            session_destroy();
            header("location: login.php");
    }
?>

<textarea id="message">TEST</textarea>
<input type="" id="submit" value="Submit message">

And this is save.php where I send the request (and sends a "msg saved" alert):

<?php
session_name('NEWSESSION');
session_start();
session_regenerate_id(true);

if(!$_SESSION['user_logged']){
    header("location: login.php");
}

$sessionTTL = time() - $_SESSION["timeout"];
if ($sessionTTL > SES_LENGHT) {
    session_destroy();
    header("location: login.php");
}

$username = $_SESSION['user_username'];

if($_POST["action"] == "save_message"){
    /* PHP code for saving message */
    echo "Message saved!";
}
?>

Is this the right way to do this? And how vulnerable is this (is it more vulnerable than to do it with classic PHP methods, with no fancy AJAX things)? All suggestions are welcome.

Consider adding User Agent check

Adding a User Agent check will add another layer of security. This will slow down Session Hijacking a bit.

AJAX alone is not a security issue

AJAX is no more vulnerable then regular access when it comes to sessions.

SES_LENGHT is not defined

The constant SES_LENGHT is not defined anywhere. Is your code snippet complete?


Avoiding repeated code in worksheet Can someone help me make this code more professional?  I'm trying my best to find something similar, but I wasn't successful.

I want to avoid the repeat of the code for every single value and also I can't find a solution to avoid select. I only get it run when I repeat the code and count rng, rng1, rng2 etc.

Sub a()
    Dim cell As Range, i As Integer, wks As Worksheet
    Dim rng As Range
    Dim rng1 As Range
    Dim rng2 As Range
    Dim rng3 As Range

    Set Bereich = Sheets("Tabelle1").Range("B2:B" & Cells(Rows.Count, 1).End(xlUp).Row)

                For Each cell In Bereich
                    If cell.Value = "EURUSD" Then
                        If Not rng Is Nothing Then
                        Set rng = Union(rng, Rows(cell.Row))
                        Else
                        Set rng = Rows(cell.Row)
                        End If
                    End If
                Next cell

                rng.EntireRow.Select
                With Selection
                ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
                Sheets(ActiveSheet.Name).Name = "EURUSD"
                rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
                End With

'--------------------------------------------------------------------------------
            Sheets("Tabelle1").Select
                For Each cell In Bereich
                    If cell.Value = "GBPUSD" Then
                        If Not rng1 Is Nothing Then
                        Set rng1 = Union(rng1, Rows(cell.Row))
                        Else
                        Set rng1 = Rows(cell.Row)
                        End If
                    End If
                Next cell

                rng1.EntireRow.Select
                With Selection
                ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
                Sheets(ActiveSheet.Name).Name = "GBPUSD"
                rng1.EntireRow.Copy Worksheets("GBPUSD").Cells(5, 1)
                End With

'--------------------------------------------------------------------------------
            Sheets("Tabelle1").Select
                For Each cell In Bereich
                    If cell.Value = "AUDUSD" Then
                        If Not rng2 Is Nothing Then
                        Set rng2 = Union(rng2, Rows(cell.Row))
                        Else
                        Set rng2 = Rows(cell.Row)
                        End If
                    End If
                Next cell

                rng1.EntireRow.Select
                With Selection
                ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
                Sheets(ActiveSheet.Name).Name = "AUDUSD"
                rng2.EntireRow.Copy Worksheets("AUDUSD").Cells(5, 1)
                End With

'--------------------------------------------------------------------------------
            Sheets("Tabelle1").Select
                For Each cell In Bereich
                    If cell.Value = "NZDUSD" Then
                        If Not rng3 Is Nothing Then
                        Set rng3 = Union(rng3, Rows(cell.Row))
                        Else
                        Set rng3 = Rows(cell.Row)
                        End If
                    End If
                Next cell

                rng1.EntireRow.Select
                With Selection
                ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
                Sheets(ActiveSheet.Name).Name = "NZDUSD"
                rng3.EntireRow.Copy Worksheets("NZDUSD").Cells(5, 1)
                End With
End Sub

Doing Copy+Paste in your IDE should raise a big red flag and then there should be a neural inhibitor that prevents your left hand from doing it, causing your right hand to move your mouse further down the module and then start typing Private Function...

Now the first thing you need to do before you write any code, is "What is it exactly that I need to be doing?" - don't think in terms of "well I need to loop all cells in that range and check if its value matches a certain specific string, then I need to do [xyz]"; rather, think at one or two levels of abstraction above that, like "well I need to copy all the rows with the same currency code to a new worksheet that's named after the currency code in question".

If I understand what you're doing, you could consider starting with something like this:

Private Function GetRowsForCurrency(ByVal SourceRange As Range, ByVal CurrencyName As String) As Range
    Dim Cell As Range
    Dim Result As New Range

    For Each Cell In SourceRange
        If Cell.Value = CurrencyName Then Set Result = Union(Result, Rows(Cell.Row))
    Next

    Set GetRowsForCurrency = Result
End Function

Then you want to select all these rows and copy them to a new worksheet that you name after the "currency name":

Private Sub CopyToNewWorksheet(ByVal SourceRange As Range, ByVal CurrencyName As String)
    Dim Result As Worksheet
    Set Result = ActiveWorkbook.Sheets.Add(After:=Worksheets(Worksheets.Count))
    Result.Name = CurrencyName
    SourceRange.EntireRow.Copy Result.Cells(5, 1) 'comment why (5,1) here
End Sub

Notice I'm using the Add function's return value, which represents the sheet that was added.

With a function to get the cells and a procedure to copy them to a new sheet, all that's left to do is to call them:

Set Bereich = Sheets("Tabelle1").Range("B2:B" & Cells(Rows.Count, 1).End(xlUp).Row)
CopyToNewWorksheet GetRowsForCurrency(Bereich, "EURUSD"), "EURUSD"
CopyToNewWorksheet GetRowsForCurrency(Bereich, "GBPUSD"), "GBPUSD"
CopyToNewWorksheet GetRowsForCurrency(Bereich, "AUDUSD"), "AUDUSD"
CopyToNewWorksheet GetRowsForCurrency(Bereich, "NZDUSD"), "NZDUSD"

Now this might be made cleaner if you defined and used constants instead of magic strings:

Const EURUSD As String = "EURUSD"
Const GBPUSD As String = "GBPUSD"
...

Couple points

NEVER call a procedure a() - give it a meaningful name that starts with a verb and that says what the code does. If you can't easily give it a name, it's probably doing too many things.
Indent your code properly: anything between XXXXX and End XXXXX should be 1 Tab further to the right. If your code starts to look like arrow code, you've got a code smell (follow that link!).
Those With blocks are absolutely useless - you're never using your with block variable, which means you've set an object reference for no reason.
If you feel the need to add a number at the end of a variable name, stop right there, take your hands off the keyboard and study what your code is doing that's redundant - refactor as needed.


Find 4 points which form a square with the biggest area I have set of points on the coordinate plane. I need to find the 4 points which form a square with the biggest area.

I'm new in JavaScript so I'd like to get any suggestions about code style, patterns, code idioms, etc. Thank you in advance!

"use strict";

/*
 * Point
 */

(function() {
    function numbersEqual(a, b) {
        return Math.abs(a - b) < Number.EPSILON;
    }

    function Point(x, y) {
        this.x = x;
        this.y = y;
    }

    Point.distance = function(a, b) {
        return Math.sqrt((a.x - b.x) ** 2 + (a.y - b.y) ** 2);
    }

    Point.equal = function(a, b) {
        return numbersEqual(a.x, b.x) && numbersEqual(a.y, b.y);
    }

    Point.pointsFormsQuadrangle = function(a, b, c, d) {
        if (Point.equal(a, b) || Point.equal(a, c) || Point.equal(a, d))
            return false;

        if (Point.equal(b, c) || Point.equal(b, d))
            return false;

        if (Point.equal(c, d))
            return false;

        const center = new Point((a.x + b.x + c.x + d.x) / 4, (a.y + b.y + c.y + d.y) / 4);

        const ac = Point.distance(a, center);
        const bc = Point.distance(b, center);
        const cc = Point.distance(c, center);
        const dc = Point.distance(d, center);

        return numbersEqual(ac, bc) && numbersEqual(bc, cc) && numbersEqual(cc, dc);
    }

    Point.pointsFormsSquare = function(a, b, c, d) {
        if (!Point.pointsFormsQuadrangle(a, b, c, d))
            return false;

        const ab = Point.distance(a, b)
        const ac = Point.distance(a, c);
        const ad = Point.distance(a, d);
        const triangle = [ab, ac, ad].sort((a, b) => a - b);

        return numbersEqual(triangle[0], triangle[1]);
    }

    window.Point = Point;
})();

/*
 * Square
 */

(function () {
    function Square(a, b, c, d) {
        this.a = a;
        this.b = b;
        this.c = c;
        this.d = d;
    }

    Square.prototype.area = function() {
        const findSquareDiagonalLength = (a, b, c, d) => {
            const ab = Point.distance(a, b);
            const ac = Point.distance(a, c);
            const ad = Point.distance(a, d);

            return Math.max(ab, ac, ad);
        };

        const d = findSquareDiagonalLength(this.a, this.b, this.c, this.d);
        return d * d / 2;
    }

    window.Square = Square;
})();

/*
 * Solution
 */

function formSquaresFromPoints(points) {
    let squares = [];

    for (let a = 0; a < points.length; a++)
        for (let b = a + 1; b < points.length; b++)
            for (let c = b + 1; c < points.length; c++)
                for (let d = c + 1; d < points.length; d++)
                    if (Point.pointsFormsSquare(points[a], points[b], points[c], points[d]))
                        squares.push(new Square(points[a], points[b], points[c], points[d]));

    return squares;
}

function sortSquaresByArea(squares) {
    squares.sort((a, b) => {
        if (a.area() > b.area())
            return 1;
        else if (a.area() < b.area())
            return -1;
        else
            return 0;
    });

    return squares;
}

const POINTS = [
    new Point(1, 1),
    new Point(1, 3),
    new Point(3, 1),
    new Point(3, 3),
    new Point(1, 6),
    new Point(6, 1),
    new Point(6, 6),
    new Point(1, 9),
    new Point(9, 1),
    new Point(9, 9),
    new Point(4, 5),
    new Point(4, 8),
    new Point(7, 8),
    new Point(7, 5),
    new Point(6, 3),
    new Point(5, 3),
    new Point(5, 1)
];

const sortedSquares = sortSquaresByArea(formSquaresFromPoints(POINTS));
const squareWithBiggestArea = sortedSquares[sortedSquares.length - 1];

console.log(squareWithBiggestArea); // Gives: {(1, 1), (1, 9), (9, 1), (9, 9)}

You're coding javascript the "old" way. With es6 you have a lot of new features available. Take a look at them.

Don't use self-invoking anonymous functions i.e. IIFEs

/// bad
(function(){
  // code here
})();

Use blocks

/// good
{
   // code here
}

Give meaningful names to your variables and methods. Functions should not have more than 2 parameters. If they do, you need to put them in an object.

/// bad
... pointsFormsQuadrangle = function(a, b, c, d){}

/// good
static FormsQuadrangle([point1, point2, point3, point4]){

}

Use the class syntax instead of function/prototype.

class Utils {
  static NumbersEqual(a, b){
    return Math.abs(a - b) < Number.EPSILON;
  }
}

class Point {
  constructor(x, y) {
    this.x = x;
    this.y = y;
  }

  distance(point) {
    return Math.sqrt((this.x - point.x) ** 2 + (this.y - point.y) ** 2);
  }

  equal(point) {
    return Utils.NumbersEqual(this.x, point.x) && Utils.NumbersEqual(this.y, point.y);
  }

  static FormsQuadrangle(a, b, c, d) {
    if (a.equal(b) || a.equal(c) || a.equal(d)) return false;

    if (b.equal(c) || b.equal(d)) return false;

    if (c.equal(d)) return false;

    const center = new Point((a.x + b.x + c.x + d.x) / 4, (a.y + b.y + c.y + d.y) / 4);

    const ac = a.distance(center);
    const bc = b.distance(center);
    const cc = c.distance(center);
    const dc = d.distance(center);

    return Utils.NumbersEqual(ac, bc) && Utils.NumbersEqual(bc, cc) && Utils.NumbersEqual(cc, dc);
  }

  static FormsSquare(a, b, c, d) {
    if (!Point.FormsQuadrangle(a, b, c, d))
      return false;

    const ab = a.distance(b)
    const ac = a.distance(c);
    const ad = a.distance(d);
    const [tr1, tr2] = [ab, ac, ad].sort((a, b) => a - b);

    return Utils.NumbersEqual(tr1, tr2);
  }
}


Python JIT instantiation This code is about instantiating object attributes Just in Time.

I'm working on a REST API, where I currently create Python objects from the JSON data returned by the server, though not all attributes of the created objects will be called upon in user code. Hence only instantiating the attributes the user access via dot notation seems like a valid way to improve performance.

lazy_evaluate is a function, with a signature similar to functools.partial.
lazy_evaluate returns a function that when called multiple times only computes the result once.

I then take advantage of the builtin property function to create a getter that calls the function returned by lazy_evaluate.

There is clearly a trade off here. This only helps performance if a subset of the attributes are required in user code. Otherwise the extra code will slow it down.

The core of the idea is to use Python scoping to mutate a list in the scope outside the function evaluate. I dislike this the most.

Do you think it's a good idea?
Do you have a better solution?
Do you think the complexity is worth it?

I will try and create more context here as to why I want this. I have implemented this idea in my package async_v20 found here. I have written docs here.

I left this out as I felt it was a distraction from the concept.

JSON from the server does get parsed into a python dict. Though aiohttp does this for me.

When the server sends JSON:

All attributes are converted to snake_case via a dictionary lookup
Time data is turned into a pandas.TimeStamp (requires extra parsing)

In some cases there may be 5000 objects to instantiate or objects may be nested around 3 deep. Objects get instantiated in this module.

This example runs a benchmark:

from time import time

class Time(object):
    def __enter__(self):
        self.start = time()
        return self
    def __exit__(self, exc_type, exc_val, exc_tb):
        self.end = time()
        self.interval = self.end - self.start

def lazy_evaluate(func, *args, **kwargs):
    acc = []
    def evaluate():
        if acc == []:
            print('EVALUATING')
            acc.append(func(*args, **kwargs))
        return acc[0]
    return evaluate

class JitAttributes(object):

    foo = property(lambda self: getattr(self, '_foo')())
    bar = property(lambda self: getattr(self, '_bar')())
    baz = property(lambda self: getattr(self, '_baz')())

    def __init__(self, foo, bar, baz):
        self._foo = foo
        self._bar = bar
        self._baz = baz

class Attributes(object):

    def __init__(self, foo, bar, baz):
        self.foo = foo
        self.bar = bar
        self.baz = baz

a = 500 # Careful making this too big!
b = a * 2
c = b * 2

print('RUNNING JitAttributes')
repeats = range(100000)

with Time() as t:
    for _ in repeats:
        JitAttributes(
            lazy_evaluate(list, range(a)),
            lazy_evaluate(list, range(b)),
            lazy_evaluate(list, range(c)),
        )

print('TOOK ', t.interval, ' seconds')

print('RUNNING Attributes')

with Time() as t:
    for _ in repeats:
        Attributes(
            list(range(a)),
            list(range(b)),
            list(range(c)),
        )

print('TOOK ', t.interval, ' seconds')

Outputs:

RUNNING JitAttributes
TOOK  0.2956113815307617  seconds
RUNNING Attributes
TOOK  4.880501985549927  seconds

Console demonstration:

>>> jit_instance = JitAttributes(
...                 lazy_evaluate(list, range(a)),
...                 lazy_evaluate(list, range(b)),
...                 lazy_evaluate(list, range(c)),
...             )
>>> jit_instance.foo
# EVALUATING            <- Only evaluates once :)
# [0, 1, 2, 3, ... ]
>>> jit_instance.foo
# [0, 1, 2, 3, ... ]
>>> non_jit_instance = Attributes(
...                 list(range(a)),
...                 list(range(b)),
...                 list(range(c)),
...             )

>>> non_jit_instance.foo
# [0, 1, 2, 3, ...]
>>> non_jit_instance.foo
# [0, 1, 2, 3, ...]

Update

New review following on from this one

I have a problem with your code, in that the code seems unrelated to the problem statement you give at the top.

You say you are "working on a REST API, where I currently create python objects from the JSON data returned by the server. Though not all attributes of the created objects will be called upon in user code."

None of your code involves REST, or JSON. So while it's an interesting demonstration of certain parts of Python, it doesn't much address your issue.

IMO, the feasibility of what you are discussing is going to be determined by the nature and character of the JSON data, and the REST API. Parsing JSON is generally a one-shot deal. If you get JSON data like this:

'{ "name":"John", "age":31, "city":"New York" }'

how would it benefit you to "defer" the evaluation or construction of the age and city attributes? It would make far more sense to just parse and assign them all.

On the other hand, if your API returns a list of 1,000's of, say, homes with lat/long coordinates, and part of the object initialization process involves computing the distance from a given origin address, then perhaps delaying the computation or delaying object instantiation makes sense.

Please share with us the nature of the problem you're really trying to solve, and we'll be able to comment effectively on whether your proposed solution is a good idea.

EDIT:

I had a look at your linked code. The key was, I think, in the helpers.py:get_attribute() function.

You appear to be re-implementing the behavior of the __getattr__ or __getattribute__ dundermethods. I suggest that you look at implementing a simpler approach, where you store (say) '_attrname' as the lazy-evaluating function, and use __getattr__ to expand that value into 'attrname' when you fetch it the first time.

Since Python will check for obj.attrname and only call obj.__getattr__() if not found, this would provide the advantage of doing the subsequent lookups in C code rather than Python.


Beginner's LINQ exercise to print the odd numbers in descending order I'm going through C# Operators which led me to more specifically Lambda Expressions (C# Programming Guide). I've modified one of the examples from just getting the .Count to printing out the odd numbers. Is there anything I can do to simplify my code? Should I "method chain" oddNumbers and oddDescending together? I've left them apart since I'm just learning and seeing them separate is easier for me to understand right now.

class Program
{
    public static void Main(string[] args)
    {
        int[] numbers = { 5, 4, 1, 3, 9, 8, 6, 7, 2, 0 };
        var oddNumbers = numbers.Where(x => x % 2 == 1);
        var oddDescending = oddNumbers.OrderByDescending(x => x);

        foreach (int odd in oddDescending)
        {
            Console.WriteLine(odd);
        }
    }
}

People will commonly create an Extension method for IEnumerable to add a map method that behaves much like the IList<T>.ForEach method.

public static class IEnumerableExtensions {
    public static void ForEach<T>(this IEnumerable<T> source, Action<T> action) {
        foreach(item in source) action(item);
    }
}

With this in hand, you can go all out on your filter & map.

class Program
{
    public static void Main(string[] args)
    {
        int[] numbers = { 5, 4, 1, 3, 9, 8, 6, 7, 2, 0 };

        numbers.Where(IsOdd)
                .OrderByDescending(x => x)
                .ForEach(Console.WriteLine);
     }

     private static bool IsOdd(int x) => x % 2 == 1;
}

Note that I did extract a method so we could name the filter. It makes it a bit more readable (and as an added bonus, reusable).

You may be asking yourself why naming the predicate is better than using a variable. Other than reuse, the benefit is that there's no need to store any state. At its heart, Linq is a functional way to program, so it's good to embrace it.


Passing private variable in JavaScript nested class? I am thinking about passing a private variable of one class into another class, for some belonging case like 'company --> department --> employee'. As an employee can tell some information of his company, there's some private variables I'd like to pass.

I think this relationship is not as same as common class and supClass.  As an employee is not a type of department and a department is not a type of company (if I am wrong, please tell me more).

So in a case like this, is it a good idea to use nested constructor functions?

For example: http://jsfiddle.net/JsJJ9/

function TeamClass(name){

    var pass = "privateVariable"
    var slot = 1;
    this.members = []; // I have a list of members
    this.name = name;

    this.add = function(){ //add member into this team
        var m = new MemberClass(slot++);
        this.members.push(m)
    }

    function MemberClass(id){ //here comes the Nested class

        var id = id;
        this.report = function(){
            alert('#'+id+' : Hello, i got some '+pass+'! ')
            //So, this member can tell some private data of his team.
        }

    }

}

class01 = new TeamClass('Team A');
class01.add();
class01.add();

class01.members[0].report();
class01.members[1].report();

Also, I have tried to let the nested class can call the upper level's public variable.  I have a function like this:

//at upper level class
    m.setBelonging(this)

and

//at belonging class
    var team = ''
    this.setBelonging = function(theOdj){
        team = theOdj;
    }

(or see it at http://jsfiddle.net/JsJJ9/1/)

Any comment about this approach?

Added at 2014-01-11 :

Actually,  I am trying to ask how can I pass some variable that can't change outside the class (which in my understand, a private variable) to another class. And i found out may be i can do this by nested class. But I don't know is there any defects or there's some better approach.

Teams have members, so it almost seems natural to put the Member class into the Team class. But projects also have members. How are you going to implement that? You can't nest Member in both classes. Another problem with nested classes is that you have to traverse through Company -> Department -> Team -> Employee hierarchy just to get everyone's name or email.

Answering your question, how can you pass some variable that can't change outside of the class. Create a getter method:

function Team() {
    var teamId = 4; // not changeable from outside
    this.getId = function() {
        return teamId;
    }
}

Now you can access teamId from outside of the class, but can't change it.

Or instead of creating truly private properties and functions, you can simply prefix them with an underscore to denote they are private and should never be accessed from outside. While nothing really stops you from accessing them, nothing also stops you from redefining the Team class and accessing anything you want.


Fast regex to extract strings before and after a time I want to get text1 and text2 by splitting time format

Text1 10:24:02 Text2

Of the two working regexes below, which is faster?

Regex 1

String regex1= "([0-9]{2}):([0-9]{2}):([0-9]{2})";

Regex 2

String regex2= "[0-9][0-9]:[0-9][0-9]:[0-9][0-9]";

This is the code I've used:

String s="Text1 10:24:02 Text2";
String[] split= s.split(regex1);//regex 1 and 2
System.out.println(split[0]);
System.out.println(split[1]);

Regex 2 is faster, but probably not for the reason that you expect.

You can easily answer questions like these by writing a benchmark.  Here is an example:

public static long splitTime(String regex, String text) {
    long start = System.currentTimeMillis();
    for (int i = 0; i < 1000; i++) {
        String[] split = text.split(regex);
    }
    long end = System.currentTimeMillis();
    return end - start;
}

public static void main(String[] args) {
    String regex1 = "([0-9]{2}):([0-9]{2}):([0-9]{2})";
    String regex2 = "[0-9][0-9]:[0-9][0-9]:[0-9][0-9]";
    String s="Text1 10:24:02 Text2";

    // Warm up the loops
    for (int i = 0; i < 2000; i++) {
        splitTime(regex1, s);
        splitTime(regex2, s);
    }

    long time0 = 0, time1 = 0, time2 = 0;
    for (int i = 0; i < 2000; i++) {
        time1 += splitTime(regex1, s);
        time2 += splitTime(regex2, s);
        time2 += splitTime(regex2, s);
        time1 += splitTime(regex1, s);
    }

    System.out.println("Regex 1: " + time1);
    System.out.println("Regex 2: " + time2);
}

The JIT compiler tends to do funny tricks.  For fairness, I've warmed up the loop by executing both of them without timing.  I've also interleaved the calls to splitTime in case the order somehow makes a difference.

I found that Regex 1 is slower than Regex 2 by about 5%.

However, Regex 1 has some capturing parentheses.  If you remove them,

    String regex0 = "[0-9]{2}:[0-9]{2}:[0-9]{2}";

then you get a result that is 16% faster than Regex 2.


Performing data transportations in RxJS with Promises I'm using RxJS 5.0.0-alpha.12 to perform some data transformations, and some of those transformations use Promises.

return observable
.flatMap(item => {
  return Observable.zip(

    Observable.fromPromise(fs.readFile('./config.json')),

    Observable.of(item),

    (s1, s2) => {
      return {
        config: JSON.parse(s1.toString()),
        testResult: s2
      };
    }
  );
})
.flatMap(item => {
  return Observable.zip(

    Observable.fromPromise(
      getStudentIdFromStudentNumber(item.testResult['Student Primary ID'])
    ),

    Observable.of(item),

    (s1, s2) => {
      return {
        'Test Date': s2.config.testConstants.ROGL_Begin_Year.testDate,
        'Student Id': s1,
        'Student Number': s2.testResult['Student Primary ID'],
        'Grade Level': s2.testResult.Grade,
        'Composite Score Alpha': s2.testResult['Composite Score']
      };
    }
  );

});

Note that both fs.readFile (fs here being the name given to fs-promise) and getStudentIdFromStudentNumber return a Promise.

It seems like having to zip the original item that is getting emitted from observable with the results of each Promise is cumbersome. Is there a cleaner way to use the results of these two Promises with this Observable?

What makes this tricky (to me) is sometimes the Promise relies on a field within the item object emitted by observable (eg., getStudentIdFromStudentNumber), and other Promises may not rely on item (eg., fs.readFile).

I would be more inclined to map with the flatMap to have my results flow through.

Something like the following, might do what you're looking for.

return observable
  .flatMap(item => Observable.fromPromise(fs.readFile('./config.json'))
    .map(file => ({ config: JSON.parse(file.toString()), testResult: item })))
  .flatMap(({ config, testResult }) => Observable.fromPromise(getStudentIdFromStudentNumber(item.testResult['Student Primary ID']))
    .map(id => ({
      'Test Date': config.testConstants.ROGL_Begin_Year.testDate,
      'Student Id': id,
      'Student Number': testResult['Student Primary ID'],
      'Grade Level': testResult.Grade,
      'Composite Score Alpha': testResult['Composite Score']
    })));


Finding closest points without duplicates and given distance threshold Given a bunch of latitudes and longitudes stored as Points, I would like to determine the points that are closest to each point given some maximum threshold in meters. This is my first stab at this:

DECLARE @ThresholdInMeters FLOAT = 1000000.0;

IF OBJECT_ID('tempdb..#Points1') IS NOT NULL
    DROP TABLE #Points1
CREATE TABLE #Points1
    (
      PointId INT IDENTITY(1, 1) ,
      Point GEOGRAPHY NULL
    )

DECLARE @LowerLatitude FLOAT;
DECLARE @UpperLatitude FLOAT;
DECLARE @RandomLatitude FLOAT;
DECLARE @LowerLongitude FLOAT;
DECLARE @UpperLongitude FLOAT;
DECLARE @RandomLongitude FLOAT;
DECLARE @Counter INT;

SET @LowerLatitude = -90
SET @UpperLatitude = 90
SET @LowerLongitude = -180
SET @UpperLongitude = 180

SET @Counter = 100;
WHILE ( @Counter > 0 )
    BEGIN
        SELECT  @RandomLatitude = ( ( @UpperLatitude - @LowerLatitude - 1 )
                                    * RAND() + @LowerLatitude );
        SELECT  @RandomLongitude = ( ( @UpperLongitude - @LowerLongitude - 1 )
                                     * RAND() + @LowerLongitude );

        INSERT  INTO #Points1
                SELECT  GEOGRAPHY::Point(@RandomLatitude, @RandomLongitude,
                                         4326)

        SET @Counter = @Counter - 1;
    END

SELECT
    Points1.PointId AS StartPointId,
    Points2.PointId AS EndPointId,
    Points1.Point.STDistance(Points2.Point) AS Distance
FROM #Points1 AS Points1
INNER JOIN #Points1 AS Points2 ON Points1.PointId <= Points2.PointId
WHERE Points1.Point.STDistance(Points2.Point) < @ThresholdInMeters

Any ideas on how to potentially improve this, especially in performance? All indices are already set.

Style

Remove unneccessary parentheses

You don't need parentheses around the condition for the WHILE
loop.  You can just write WHILE @counter > 0.

Also, the extra sets of parentheses around the random numbers are
superfluous.  If you leave them off, the line will be a bit less
daunting.

Don't break the line in the middle of a concept

That is, in the SELECT @RandomLatitude line, the statement breaks
just before the * operator.  As it has a higher precedence than
the +, this can be a bit misleading.  Either don't break, use
parentheses (if really unavoidable) or break before the +:

SELECT @RandomLatitude = ( @UpperLatitude - @LowerLatitude - 1 ) * RAND()
                         + @LowerLatitude;
SELECT @RandomLongitude = ( @UpperLongitude - @LowerLongitude - 1 ) * RAND()
                          + @LowerLongitude;

Merge variable declaration and initialisation

Where possible, declare a variable just before it is used, and
initialise it there as well:

DECLARE @LowerLatitude FLOAT = -90;
DECLARE @UpperLatitude FLOAT = 90;
DECLARE @LowerLongitude FLOAT = -180;
DECLARE @UpperLongitude FLOAT = 180;

DECLARE @Counter INT = 1000;
WHILE @Counter > 0
  BEGIN
    DECLARE @RandomLatitude FLOAT = ( @UpperLatitude - @LowerLatitude - 1 ) * RAND()
                                    + @LowerLatitude
    DECLARE @RandomLongitude FLOAT = ( @UpperLongitude - @LowerLongitude - 1 ) * RAND()
                                     + @LowerLongitude
    -- ...

Performance

You mention that the indices are already set. I assume that means that
it is not possible to add a Spatial Index, or a primary key.
Although I would strongly suggest adding a primary key constraint to
the temp table in this example:

CREATE TABLE #Points1
    ( PointId INT IDENTITY(1, 1) PRIMARY KEY
    , Point GEOGRAPHY NULL
    );

Unfortunately, without a Spatial Index or changes to the result
requirements (you'd need a TOP n clause and an ORDER BY on the
STDistance column), there is no choice but to check each point against
all others, which will give a large join of which I don't know how to
improve the performance.

Small point: you may gain a smaller result (only in number of rows) if
you leave out the distance from each point to itself:

INNER JOIN Points1 AS Points2 ON Points1.PointId < Points2.PointId

but that might not fit your requirements.


Pizza ordering system Is there any room for improvements in this code. I am new to programming (python3), learning and practicing functions related to arguments recently.
def make_pizza(*toppings, **pizza_info):

    """Creating a dictionary for pizza information"""
    pizza = {}

    pizza_toppings = [topping for topping in toppings]
    pizza['toppings'] = pizza_toppings

    for key, value in pizza_info.items():
        pizza[key] = value

    return pizza

def show_pizza(pizza):

    """Showing the order with the information provided by make_pizza() function."""

    print("You've ordered " + str(pizza['size']) + " inches size " + pizza[
        'crust'].title() + " crust pizza with the toppings of:")

    for topping in pizza['toppings']:
        print("-" + topping.title())

# Taking order in a form of dictionary structure
order = make_pizza('cheese', 'mushroom', size=12, crust='thin')

# Show the order message
show_pizza(order)

You can use standard copy functions instead of manually copying:
from copy import copy

def make_pizza(*toppings, **pizza_info):
    pizza = pizza_info.copy()
    pizza['toppings'] = copy(toppings)
    return pizza

And maybe use a dict-comprehension for merging information, instead of mutating the pizza:
def make_pizza(*toppings, **pizza_info):
    return {
        **pizza_info,
        'toppings': copy(toppings)
    }

For printing, use f-strings for easier formatting:
print(f"You've ordered {pizza['size']} inches size {pizza[
    'crust'].title()} crust pizza with the toppings of:")


LeetCode Reverse Nodes in k-Group Link: https://leetcode.com/problems/reverse-nodes-in-k-group/
Problem description:

Given a linked list, reverse the nodes of a linked list k at a time
and return its modified list.
k is a positive integer and is less than or equal to the length of the
linked list. If the number of nodes is not a multiple of k then
left-out nodes, in the end, should remain as it is.
You may not alter the values in the list's nodes, only nodes
themselves may be changed.
e.g. Input: head = [1,2,3,4,5], k = 3
Output: [3,2,1,4,5]

My solution:
/**
 * Definition for singly-linked list.
 * struct ListNode {
 *     int val;
 *     ListNode *next;
 *     ListNode() : val(0), next(nullptr) {}
 *     ListNode(int x) : val(x), next(nullptr) {}
 *     ListNode(int x, ListNode *next) : val(x), next(next) {}
 * };
 */
class Solution {
public:
    ListNode* reverseKGroup(ListNode* head, int k) {
        ListNode** subHead = &head;
        int i = 0;
        for (ListNode* subTail = head; subTail != NULL; subTail = subTail->next) {
            if ((i+1) % k == 0) {
                ListNode* temp = subTail->next;
                reverse(subHead, &subTail);
                subTail->next = temp;
                subHead = &(subTail->next);
            }
            i++;
        }
        return head;
    }

    void reverse(ListNode** subHead, ListNode** subTail) {
        if (*subHead==NULL) return;
          ListNode* itHead = *subHead;
          ListNode* prev = NULL;
          ListNode* next = NULL;
          ListNode* end = (*subTail)->next;
          while (itHead != end) {
            next = itHead->next;
            itHead->next = prev;
            prev = itHead;
            itHead = next;
          }
        std::swap(*subHead, *subTail);
    }
};

This solution was accepted and seemed to do quite well in both space and time. I'm interested in how it can be optimized further, how it can be made more readable, more elegant, or better express good design principles. Note that the data definition and the signature ListNode* reverseKGroup(ListNode* head, int k) is enforced by the question. Feel free to comment on anything you'd like though. Many thanks.

Don't use the macro NULL.  Use the specific keyword nullptr now.
Your struct can be more simply written using immediate inline initializers.
struct ListNode {
    int val = 0;
    ListNode* next = nullptr;
    ListNode() = default;
    ListNode(int x) : val{x} {}
    ListNode(int x, ListNode* next) : val{x}, next{next} {}
};

Also note that the * goes with the type in C++, not with the name being declared.
Regarding
void reverse(ListNode** subHead, ListNode** subTail)
It appears that the names subHead and subTail are never modified by the code, so this would be easier if you passed references to the pointers, rather than having an extra explicit dereference operation when you use it.
ListNode* itHead = *subHead;
//...
while (itHead != end) {
    //...
    itHead = next;
}

This is just a for loop that's spread out for no reason.  The value of next you want is lost during the body of the loop, so I can see why you don't put that inside a for construct.  But you can put the declaration of subHead there... put the parts you can into a for loop, even if it's not all three of them.
Re:
if (*subHead==NULL) return;
Don't make explicit comparisons against null.  Use the truth predicates that are part of the (possibly smart!) pointer.
Combining this with the tip of using references (...ListNode*& subHead,) you get simply:  if (!subHead)  return;.
Re:
std::swap(*subHead, *subTail);
This works because you know that these are raw pointers.  But generally you write code like this as a template, or at least you code mostly as if you are writing a template, to ease future maintenance.  If the type of something changes, the code should at best still work; second but still OK is cause a compile-time error that you must fix; but bad is to silently fail.  Consider what would happen if these were not raw pointers but iterators of some type, including something supplied by your program or another library (not in std).
You need to use the so-called "std two-step" to use swap and a few others.  Or, if using C++20 you can use std::ranges::swap instead.  Always use swap etc. properly without relying on knowledge that the overload you want is indeed found in std.