Array that defines product information and generates the html for the product Is there a more productive way to do this? Is my code up to standards or up to best practices?

   <?php

    /*
      FILE:       index.php
      AUTHOR:     Darcey Mckelvey
      PURPOSE:    View Products
    */

    // Define product information
    $products[0] = array(
      'name' => 'Sneakers',
      'price' => 24.99,
      'category' => 'Shoes',
      'description' => 'Black sneakers. Good for walking or athletic activity'
    );

    $products[1] = array(
      'name' => 'Armani Red Silk Shirt',
      'price' => 379.99,
      'category' => 'Shirts',
      'description' => 'Fancy Italian red silk shirt. Very spiffy, sure to attract the ladies'
    );

    echo "
Welcome to DropShop!

Our Products
";

    // Loop to display all products
    foreach($products as $product) {
      echo "
" . $product['name'] . "
" .
      "Price: $" . $product['price'] . "
" .
      "Category: " . $product['category'] . "
" .
      $product['description'] . "
";
    }

    ?>

Well, if this is the real code and not a sample (like, data you're getting in $products is not fetched from the database), then there isn't much use of hard-coding a PHP array just to immediately use it to display HTML paragraphs - why not just write everything as HTML?

Other than that, you're implicitly creating an array (there's no $products = array() first). It's not an error, but it can be a little confusing. My immediate reaction was "where is $product coming from?". Also, you don't need to declare each element separately, you can do everything "at once" like this:

$products = array(
  array(
    'name' => 'Sneakers',
    'price' => 24.99,
    'category' => 'Shoes',
    'description' => 'Black sneakers. Good for walking or athletic activity'
  ),
  array(
    'name' => 'Armani Red Silk Shirt',
    'price' => 379.99,
    'category' => 'Shirts',
    'description' => 'Fancy Italian red silk shirt. Very spiffy, sure to attract the ladies'
  )
);

Also, mixing PHP and HTML in single file isn't good practice, except when you're just echo-ing data in a "view". Other than that, logic should be separated.


Using singleton pattern for a MySQL connection I'm new to web programming, and writing some web apis to enable users to sync their data between devices.

Is it okay to use singleton pattern to prevent reconnecting the MySQL database on every api call?

<?php

final class MySQLiConnection
{
    private static $connection = null;

    private function __construct() {}

    public static function getInstance() : MySQLiConnection
    {
        if ($connection == null || !$connection->ping())
        {
            $connection = new mysqli("localhost", "id", "password", "database");
        }

        return $connection;
    }

    public function execute(string $sql, iterable $params) : void
    {
        if ($statement = $connection->prepare($sql))
        {
            foreach ($params in $param)
            {
                if (is_int($param))
                {
                    $statement->bind_param("i", $param);
                }
                else if (is_double($param))
                {
                    $statement->bind_param("d", $param);
                }
                else if (is_string($param))
                {
                    $statement->bind_param("s", $param);
                }
            }

            $statement->execute();
            $statement->close();
        }
    }

    public function getResult(string $sql, iterable $params) : mysqli_result
    {
        if ($statement = $connection->prepare($sql))
        {
            foreach ($params in $param)
            {
                if (is_int($param))
                {
                    $statement->bind_param("i", $param);
                }
                else if (is_double($param))
                {
                    $statement->bind_param("d", $param);
                }
                else if (is_string($param))
                {
                    $statement->bind_param("s", $param);
                }
            }

            $statement->execute();

            $result = $statement->get_result();
            $statement->close();

            return $result;
        }
    }
}

// example
MySQLiConnection::getInstance()->execute("SELECT * FROM users WHERE forename = ? AND surname = ?", "ben", "dover");

?>

First, on your premises.

First of all, no pattern would prevent PHP from making a connection to MySQL database on every web api call. Because PHP will die between different API calls, along with all its singletons, connections and any other stuff. So all you can prevent is reconnecting the MySQL database on every call to a database connection class.
No, it is frowned upon using singleton pattern to prevent reconnecting the MySQL database on every call to a database connection class. A more accepted approach is Dependency Injection.
Funny enough, due to a typo, this class would not prevent reconnecting the mysql. Had you error_reporting set to E_ALL, PHP would have signaled that you are trying to use a non-existent variable $connection every time getInstance() is called. Surely you wanted to call it as $this->connection instead.

Now to the code.

The intention is very good, especially I like getResult() and execute() methods that allow you to avoid that bind param hassle. However, there is evidently a duplicated code. Why not to make execute to return $statement? It will let you make getResult() as simple as

public function getResult(string $sql, iterable $params) : mysqli_result
{
    return $this->execute($sql, $params)->get_result();
}

And finally. I just noticed, halfway the review process, that your code is off topic, as it simply doesn't work. Mysqli is not PDO, you cannot bind your parameters in a loop. So you need to rework it. In order to help, here are two my articles:

How to properly connect to Mysql database using mysqli that will show you important options missed in your connection code
Mysqli made simple to show you how to bind parameters for mysqli dynamically


C++ Merge Sort Algorithm I have created this first-timer implementation of Merge Sort
and I got confused by the C++ syntax once or twice.

So I have some questions about this code:

#include <iostream>
#include <thread>
#include <cstdlib>
#include <vector>

#define NUM 1048576*2

int sort(std::vector<int>* vec);
void run(std::vector<int>* vec, int left, int right);
void merge(std::vector<int>* vec, int left, int mid, int right);

int main() {
    srand(time(NULL));
    std::vector<int> tobesorted;
    for(int i = 0; i < NUM; i++){
        tobesorted.push_back(rand());
    }

    sort(&tobesorted);

    for(int i = 0; i < NUM; i++){
        std::cout << tobesorted[i] << ", ";
    }
    std::cout << std::endl;
    std::cout << "End of Program!";

}
int sort(std::vector<int>* pvec){
    int size = pvec->size();
    std::thread t1(run, pvec, 0, 0.25*size);
    std::thread t2(run, pvec, 0.25*size+1, 0.5*size);
    std::thread t3(run, pvec, 0.5*size+1, 0.75*size);
    std::thread t4(run, pvec, 0.75*size, size-1);

    t1.join();
    t2.join();
    t3.join();
    t4.join();

    std::thread t5(merge, pvec, 0, 0.25*size, 0.5*size);
    merge(pvec, 0.5*size+1, 0.75*size, size-1);
    t5.join();

    merge(pvec, 0, 0.5*size, size-1);

    return 1;
}

void run(std::vector<int>* pvec, int left, int right){
    if(right - left == 0){
        return;
    }
    int mid = (left+ right)/2;
    run(pvec, left,mid);
    run(pvec, mid+1, right);

    merge(pvec, left, mid, right);

}

void merge(std::vector<int>* pvec, int left, int mid, int right){
    int ileft = 0;
    int iright = 0;
    int index = 0;

    std::vector<int> leftvec;
    std::vector<int> rightvec;

    int j = 0;
    for(int i= left; i <= mid; i++){
        leftvec.push_back(pvec[0][i]);
    }
    j = 0;
    for(int i= mid+1; i <= right; i++){
        rightvec.push_back(pvec[0][i]);
    }

    while(ileft < leftvec.size() && iright < rightvec.size()){
        if(leftvec[ileft] > rightvec[iright]){
            pvec[0][left+index] = leftvec[ileft++];
            index++;
        } else {
            pvec[0][left+index] = rightvec[iright++];
            index++;
        }
    }

    while(ileft < leftvec.size()){
        pvec[0][left+index] = leftvec[ileft++];
        index++;
    }

    while(iright < rightvec.size()){
        pvec[0][left+index] = rightvec[iright++];
        index++;
    }

}

I think I should have better optimized for better cache use, do you see any opportunities there?
What could have been generally improved on that code?

This code doesn't work, as can be shown by including <algorithm> and changing the output:

std::cout << std::is_sorted(tobesorted.begin(), tobesorted.end()) << std::endl;

Other things that are surprising:

We pass pointers but always assume they are not null.  That suggests we should be using references instead.
Misspelt std::srand and std::time - though I'd recommend using <random> instead.
std::endl used where there's no need to flush output - just write \n instead.
Why are we using int rather than std::size_t for our left and right indices?
Use of floating-point multiplication for integer arithmetic.
Unused variable j.

There's a risk of overflow here:

int mid = (left+ right)/2;

Better would be

auto mid = left + (right - left) / 2;

This code can be re-written:

std::vector<int> leftvec;
std::vector<int> rightvec;

for(int i= left; i <= mid; i++){
    leftvec.push_back(pvec[0][i]);
}
for(int i= mid+1; i <= right; i++){
    rightvec.push_back(pvec[0][i]);
}

Those vectors will likely re-allocate several times as they are filled, because we didn't reserve() capacity.  Instead of those loops, we could use the appropriate constructor:

std::vector<int> leftvec(pvec->begin()+left, pvec->begin()+mid);
std::vector<int> rightvec(pvec->begin()+mid, pvec->>begin()+right+1);

It would be easier if we passed iterators rather than indices here.


Password generator script What I was trying to do here was create a Python script that would generate passwords with 16 characters with uppercase and lowercase letters, and numbers. The generated password would then write itself to a file.

Let me know what you think because I know they say the point of Python is to be able to write powerful scripts that can do what other languages can do in a few lines. I feel like this is probably way more then it should be for what it's doing.

import random
import string

print "Welcome to PassBook!"

def genPasswd():
    global route
    global password
    global username
    global account
    global entry
    account = raw_input("Account: ")
    username = raw_input("Username: ")
    key1 = random.randrange(10,100)
    key2 = random.randrange(10,100)
    key3 = random.randrange(10,100)
    key4 = random.randrange(10,100)
    keya = str(random.choice(string.letters) + random.choice(string.letters))
    keyb = str(random.choice(string.letters) + random.choice(string.letters))
    keyc = str(random.choice(string.letters) + random.choice(string.letters))
    keyd = str(random.choice(string.letters) + random.choice(string.letters))
    key1a = str(key1) + str(keya)
    key2b = str(key2) + str(keyb)
    key3c = str(key3) + str(keyc)
    key4d = str(key4) + str(keyd)
    password = str(key1a + key2b + key3c + key4d)
    entry = "Account: " + account + " - Username: " + username + " - Password: " + password
    file = open("/home/meta/Desktop/Python2/passbook/passbook.txt", "a")
    file.write("\n" + entry + "\n")
    print entry
    return key1
    return key2
    return key3
    return key4
    return keya
    return keyb
    return keyc
    return keyd
    return key1a
    return key2b
    return key3c
    return key4d
    return password
    return username
    return account
    return entry

def queryPb():
    search = raw_input("For which account are you searching: ")
    read = open("/home/meta/Desktop/Python2/passbook/passbook.txt", "r")
    for line in read:
        if search in line:
            print line
            return line
        else:
            print "I'm sorry we could not find any account related to " + search

def takeRoute():
    global route
    route = raw_input("Press 1 to generate a password, Press 2 to search for an existing account:  ")
    if int(route) ==1:
        genPasswd()
    elif int(route) == 2:
        queryPb()
    else:
        print "I'm sorry that is not an option!"
        exit()

takeRoute()

Your sentiment is indeed correct, this code is much longer than it needs to be and can be shortened 10x while retaining the same functionality.

You are doing 4 things in a single function!

Asking for input
Generating a password
Printing
Writing to a file.

A function should do 1 thing only.

Let's say we want to generate a password:

def generate_password():
    numbers = (str(random.randint(10, 99)) for _ in range(4))
    letters = (random.choice(string.letters) for _ in range(4))
    return ''.join(numbers) + ''.join(letters)

This function is shorter than yours not only because it does not care about Input/Output, but also because it uses loops (Generator expressions here) to do the same thing many times.

You may call this function in a user-interface that handles input and output like:

print(raw_input("Username ") + " " + raw_input("Surname ") + ": " + generate_password())

Of course you may want to make the interface nicer or make it handle files, but the backbone is here.


Anyway to shorten these for statements? Just seeing if there is anyway to shorten these for statements?

...
      fillMonthSelect: function() {
        var month = new Date().getMonth() + 1;

        for (var i = 1; i <= 12; i++) {
          $('.card-expiry-month').append($('<option value="'+i+'" '+(month === i ? "selected" : "")+'>'+i+'</option>'));
        }
      },

      fillYearSelect: function() {
        var year = new Date().getFullYear();

        for (var i = 0; i < 12; i++) {
          $('.card-expiry-year').append($("<option value='"+(i + year)+"' "+(i === 0 ? "selected" : "")+">"+(i + year)+"</option>"));
        }
      },
...

String building in code is typically messy, especially without a native String.Format() or sprintf()  in Javascript. Building HTML is the worst offender, too.

Other than using a templating engine, like ICanHaz.js (which this use case alone doesn't really justify), you could also build a helper function for creating option tags.

function buildOption(value, isSelected) {
    return "<option value='"+value+"' "+(isSelected ? "selected" : "")+">"+value+"</option>";
}

If you're building a lot of these drop-down options, it might makes things more readable:

for (var i = 0; i < 12; i++) {
    var option = buildOption(i + year, i === 0);
    $('.card-expiry-year').append(option);
}

It's a good idea to avoid long lines/sections of code which have a simple purpose, but have to be read carefully to understand what they do (complicated regular expressions are a good example).

Extracting that functionality into a function avoids this, because the function signature is all someone would need to read to understand what it does.


Counting thousands of selected checkboxes This is a script I use to count how many items are selected in a form. It currently loops through the entire form to count how many check boxes are selected, each time a checkbox is clicked. The form has thousands of checkboxes, and it's painfully obvious how slow the script is with this many elements (About 18,240 items in my sample query).

Any ideas on how I can speed this up? The speed is fine when there's less than 1,000 results, but there's rarely that few when it's running in production.

function countSelected()
{
    var daform  = document.forms.resultsForm;
    var daspan  = document.getElementById("acctSelected");
    var counter = 0;
    var i       = 0;

    if (daform.multi.length == undefined) {
        if (daform.multi.checked) {
            counter = "1";
        } else {
            counter = "0";
        }
    } else {
        for (i = 0; i < daform.multi.length; i++)
        {
            if (daform.multi[i].checked) {
                counter++;
            }
        }
    }

    daspan.innerHTML = counter;
}

Fun facts:

This script is fastest in Firefox 16.0.2 (about 2 secs)
Second fastest by a slim margin in Internet Explorer 9 (about 2.25 secs)
And absurdly slow in Chrome version 23.--- (I got tired of waiting)

I suspect you're going to struggle to optimise that, as DOM manipulation is always slow. See this guy's answer.

You could try do something like he suggests, keeping your data and the DOM separate from each other.

  Currently it loops through the entire form to count how many check boxes are selected, each time a checkbox is clicked

Why not increment or decrement a counter when a checkbox's state is changed? That'll be a lot quicker.


Getting Pascal's triangle recursively in Python I've been practicing recursion lately and wrote this code to get Pascal's triangle recursively.

How would you guys improve it ?

I don't really like the fact that the recursion goes for n+1 steps.

def RecPascal(n, m=1, prev=[]):
    if m > n+1:
        return []
    elif m == 1:
        return RecPascal(n, m+1 , [1])
    else:
        return [prev] + RecPascal(n, m+1, calculate(prev))

def calculate(prev):
    res = [0]*(len(prev)+1)
    res[0], res[-1] = 1, 1
    for i in range(0,len(res)):
        if res[i] == 0:
            res[i] = prev[i-1] + prev[i]
    return res

for line in RecPascal(10):
    print(line)

Similar to your Water Jug solution, this is still not a good recursive algorithm/implementation.  But we can still critic it, and provide useful feedback.

First, the default argument m=1 is awkward, which leads to the odd if m > n+1: and if m == 1: test conditions.  Changing the default argument to zero reduces this oddness:

def RecPascal(n, m=0, prev=[]):
    if m > n:
        return []
    elif m == 0:
        return RecPascal(n, m+1 , [1])
    else:
        return [prev] + RecPascal(n, m+1, calculate(prev))

The elif m == 0: case seems to exist only to seed the algorithm with the first row of Pascal's Triangle.  The default value prev=[] is never used the code; if only one parameter is given, the second value's default causes return RecPascal(n, m+1, [1]) to be executed and prev is not used.  If you instead use [1] as the default value, you can eliminate the n+1-th recursive step.

def RecPascal(n, m=0, prev=[1]):
    if m < n:
        return [prev] + RecPascal(n, m+1, calculate(prev))
    return []

A bit of inspection will reveal that this is (like the Water Jug solution) just a loop disguised as recursion; it can be rewritten without recursion as:

def RecPascal(n):
    triangle = []
    row = []
    for _ in range(n):
        row = calculate(row)
        triangle.append(row)
    return triangle

In the calculate(prev) helper function, you initialize res to a row of zeros, and then fill in the end values with 1:

    res = [0]*(len(prev)+1)
    res[0], res[-1] = 1, 1

You could replace this with initializing res with a row of ones:

    res = [1] * (len(prev)+1)

Of course, then you can't look for the res[i] == 0 values to fill in, but we don't need to look, we know the exact indices that need filling in:

    for i in range(1, len(res)-1):
        res[i] = prev[i-1] + prev[i]

This gives a much cleaner helper function.  We can even get rid of a -1 by using using the len(prev) instead of len(res):

def calculate(prev):
    res = [1] * (len(prev)+1)
    for i in range(1, len(prev)):
        res[i] = prev[i-1] + prev[i]
    return res

If we wanted to be tricky, we could add a 0 to the end of prev, and then add corresponding elements of two slices of the prev list together, and prepend a 1 to the beginning:

def calculate(prev):
    prev = prev + [0]
    return [1] + [x+y for x,y in zip(prev[1:], prev[:-1])]

Which works like this:

prev + [0]  -> [ 1, 3, 3, 1, 0 ]

  prev[1:]  -> [ 3, 3, 1, 0 ]
+ prev[:-1] -> [ 1, 3, 3, 1 ]
-----------------------------
         [1] + [ 4, 6, 4, 1 ]

This gives the correct generation of the first row: calculate([]) --> [1].  If we added 0 to both the start and back, we'd correctly calculate the 1's at each end (0+1 = 1, and 1+0 = 1) but would have to seed the first row with [1].

Since calculate is a helper for RecPascal, and won't be used anywhere else, you can "hide" it by moving it inside of RecPascal, shown here with the alternate calculation (prepending & appending 0's, seeding first row as [1]) method:

def RecPascal(n):

    def calculate(prev):
        prev = [0] + prev + [0]
        return [x+y for x,y in zip(prev[1:], prev[:-1])]

    triangle = []
    row = [1]
    for _ in range(n):
        triangle.append(row)
        row = calculate(row)

    return triangle


Logging errors in google analytics Apart from general JavaScript knowledge, this snippets requires someone who knows how to properly use Google Analytics API.

To log errors on my site, I use this code:

try {
  window.addEventListener("error", function(e) {
    var file = e.filename.substr(e.filename.lastIndexOf("/")+1);
    // for some reason, filename sometimes contained path, sometimes just name
    if(file.length==0)
      file = e.filename;
    var msg = file+":"+e.lineno+":"+e.colno+" \""+e.message+"\" Browser: "+navigator.userAgent;

    if(typeof ga=="function") {
      ga('send', 'event', 'jserror', 'click', msg, {'transport': 'beacon'});
    }
    else {
      // This is executed when google analytic is not loaded
      // which happens in debug mode
      console.error(msg);
    }
  });
}
catch(e) {
  console.warn("Failed to add error callback.");
}

I'm especially interested to know whether I'm using the google analytics API correctly. Maybe I could log the data about error in multiple columns?

I'm new to google analytics so feedback is very valuable for me.

ga('send', 'event', 'jserror', 'click', msg, {'transport': 'beacon'});

Yup, that's how I would do events in Google Analytics. However, the API also provides a special call for errors. It's found under Exception Tracking.

ga('send', 'exception', {
  'exDescription': msg,
  'exFatal': false
});

Another is that you're wrapping an addEventListener call into a try-catch. Not sure why this is required. The callback is fired asynchronously, thus the try-catch will make no sense (unless somehow, window or addEventListener doesn't exist)

if(typeof ga=="function") {

The snippet (and not the script that it loads) creates the global ga function for you, which means the global ga should be present when the snippet is present. It's seen in the snippet as this portion:

i[r]=i[r]||function(){...}

But I see that it's possible that you're not loading it when debugging. So this can be ignored I suppose.

Lastly, rename e to error. This makes the variable tell what it is more.


Sums of all sublists The parts_sums function in the code takes a list as parameter and then returns sum of all sub-lists present in the list. e.g.
if, ls = [1, 4, 6, 4] then ls contains,

ls = [1, 4, 6, 4]
ls = [4, 6, 4]
ls = [6, 4]
ls = [4]

so the function should return a new list:
nsl = [15, 14, 10, 4]

def parts_sums(ls):
    nls, c = [], 0
    leng = len(ls)
    for n in ls:
        i, sum = 0, 0
        while not i == c + 1:
            i += 1
            sum += ls[leng - i]
        c += 1
        nls.insert(0, sum)
    return nls

If your output is really what you desire, you are working way too hard for this.
In your original solution you work from the back, and sum all the needed elements in each step to generate the new sum. This is inefficient. Working from the back, you can reuse the sum from the previous step and just add the current element on top of it.
def parts_sums_from_the_back(ls):
    nls = []
    sum_ = 0
    for element in reversed(ls):
        sum_ += element
        nls.insert(0, sum_)
    return nls

You can also work from the front. Then you start with the sum over all elements, and subtract the elements as you go along:
def parts_sums_from_the_front(ls):
    nls = [sum(ls)]
    for i in range(len(ls)-1):
        nls.append(nls[i]-ls[i])
    return nls

Sidenote: As you can see, sum is already taken in Python, so you should not use it to name your own variables.
Also, since what you are doing is not uncommon it even has a special name: cumulative sum. Just think about it for a second how your code can be expressed in terms of a cumulative sum. In the meantime, let me introduce you to NumPy. NumPy is a specialized library for numeric computations, and as such also has a implementation of a cumulative sum.
import numpy as np

def parts_sums_np(ls):
    return np.cumsum(ls[::-1]).tolist()[::-1]

Spoiler: As you can see, your result is cumsum of the reversed list, reversed again.
Addendum: Since I'm usually working with Python in a scientific context I'm quick to use NumPy for quite a lof of tasks. Fortunately there are experienced people like Maarten Fabré who know a thing or two about Python. He gave the following implementation in a comment below which only uses tools from Python's standard library:
from itertools import accumulate

def parts_sums_itertools(ls):
    return list(accumulate(reversed(ls)))[::-1]

itertools.accumulate is a very handy function to express all kind of cumulative computations. If used on a sequence with no further arguments it computes the cumsum, but it can also be used to implement other algorithms like cumulative multiplication.

I tested all of them with this little snippet:
ls = [1, 4, 6, 4]
nsl = [15, 14, 10, 4]
print("parts_sums", parts_sums(ls) == nsl)
print("from_the_back", parts_sums_from_the_back(ls) == nsl)
print("from_the_front", parts_sums_from_the_front(ls) == nsl)
print("numpy", parts_sums_np(ls) == nsl)
print("itertools", parts_sums_itertools(ls) == nls)

which happily prints
parts_sums True
from_the_back True
from_the_front True
numpy True
itertools True

Where parts_sums is your original implementation.

Timing
I also performed some preliminary timing. With the input given in your question the results are as follows:

parts_sums: 5µs
from_the_back: 1.6µs
from_the_front: 2µs
numpy: 13µs
itertools: 1.15µs

As you can see itertools takes the leadworking from the back is the most efficient approach since it doesn't make it necessary to look at all elements of the list beforehand. NumPy performs actually quite poorly here. This is likely due to the overhead from going from Python to the C backend of NumPy and back again.
I repeated the timing for an input with 1000 elements. The results are as follows:

parts_sums: 185ms
from_the_back: 690µs
from_the_front: 285µs
numpy: 170µs
itertools: 63.1µs

As you can see, all the repeated summing makes your original implementation scale really badly. Also, from_the_front and from_the_back have switched places. This is likely because .insert(0, ...) is more expensive than .append(...). You can work against this dynamic array growing since you know exactly how large the array will be at the end. If you accomodate for this (see code below), the time goes down from over 600µs to around 270µs. At this point you also see that NumPy starts to play its strengths. NumPy might get stronger here, but itertools still dominates the comparison by a considerable margin.
def parts_sums_from_the_back_pre(ls):
    nls = [None, ]*len(ls)
    sum_ = 0
    for i, element in enumerate(reversed(ls)):
        sum_ += element
        nls[-(i+1)] = sum_
    return nls

Appendix: Further timing
I performed some extended timings to generate a visual presentation how the function runtimes scale with the list length.

As expected, the original implementation scales very badly with the size of the input. This becomes even more obvious if you look at the plot with linear axis scale on the left (click for full resolution).
While creating the test routine I also came to the conclusion that NumPy loses against itertools because it has to convert the data from Python to NumPy's own format. If you repeat the test above using input already in NumPy's own format, NumPy steals the lead from the itertools implementation.


Finding the second largest element of large input sets I have a problem where I need to find the second maximum element of the user inputs. It's an online practice problem and I can't figure out why the server responds back with a Non-Zero Exit Code error when I submit my code. The  program produces right outputs every single time when tested locally. Can someone tell me which part of my code is prone to such errors and how I can deal with this error?

Sample Input:                 Sample Output:

2                             7
1 3 5 7 8 -1                  16
12 23 16 0 2 -1

The first input is the number of test cases. Second and third inputs are the actual user inputs respectively. And user inputs are always terminated with -1.

Here is my code:

class SecondLargest {

    public static void main(String[] args) {
        Scanner sc=new Scanner(System.in);
        int testcase=Integer.parseInt(sc.nextLine());

        while(testcase-->0){
            ArrayList<Long> al=new ArrayList<>();
            long num=0;
            long i=0;
            while((num=(sc.nextLong()))!=-1){
                al.add(num);
                i++;
            }
            Collections.sort(al);
            if(al.size()==0){
                System.err.println("");
            }
            else if(al.size()==1){
                System.out.println(al.get(0));
            }
            else if(al.size()>1){
            System.out.println(al.get((int)(al.size()-2)));
            }
        }
    }
}

Large input sets will be used to test the program.

So your code seems to work fine, but you get a non-zero exit code for some unknown inputs... What could go wrong? If I call out to you 100 numbers, do you need to remember all of them to tell me the second highest? (inside an array in your head)
You don't. And the program doesn't either.
Most probably the program crashes with an out of memory error.

Here's one way to solve this using constant storage:

    while (testcase-- > 0) {
        long num;
        long largest = Long.MIN_VALUE;
        long secondLargest = Long.MIN_VALUE;
        while ((num = sc.nextLong()) != -1) {

                  if (num > largest) {
                     secondLargest = largest;
                     largest = num;
                } else if (num > secondLargest) {
                     secondLargest = num;
                }

        }
        System.out.println(secondLargest);
    }

Code review

Your code had serious formatting issues. Cleaned up, your code would look like this:

    while (testcase-- > 0) {
        List<Long> numbers = new ArrayList<>();
        long num;
        while ((num = sc.nextLong()) != -1) {
            numbers.add(num);
        }
        Collections.sort(numbers);
        if (numbers.size() == 0) {
            System.err.println("");
        } else if (numbers.size() == 1) {
            System.out.println(numbers.get(0));
        } else {
            System.out.println(numbers.get(numbers.size() - 2));
        }
    }

What changed?

Formatting fixed
Use interface type instead of implementation (List)
Removed redundant variables
Removed redundant initializations
Removed redundant parentheses
Improved variable names


A Caesar cipher in Python3 This code basically just does a Caesar cipher on the contents of the variable "s" with a shift of 1.

Sorry if the comments on the code don't explain it well. I'm quite new to Python and I'm not very good at explaining.

Can this be code be improved?

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase

# Returns a list of each char in the given string.
def charSplit(str):
    return [char for char in str]

letters = charSplit(ascii_lowercase)
result = []
finalResult = ""
letterIndex = 0
strIndex = 0
s = "abcde test bla bla bla"
found = False

if __name__ == "__main__":
    while found == False:
        # Checks if the string has been converted to the cipher.
        if len(result) == len(s):
            found = True
        # Adds a space to the result variable if the string index is a space.
        elif s[strIndex] == " ":
            result.append(" ")
            strIndex += 1
        # Checks if the string index is equal to the letter index.
        elif charSplit(s)[strIndex].lower() == letters[letterIndex]:
            result.append(letters[letterIndex - 1])
            strIndex += 1
        #
        else:
            if len(letters) - 1 == letterIndex:
                letterIndex = 0
            else:
                letterIndex += 1
    # Iterates through result and adds each string to the empty string, "finalResult".
    for l in result:
        finalResult += l

    print(finalResult)

You say you're new to Python. Well, that's ok. And a Caeser Cipher is a good place to start since it's complex enough for an absolute beginner while easy enough to understand what goes on, why and when.

One thing to keep in mind is that Python comes batteries included. This means a lot of what you want to accomplish is already written, one way or another. You just have to know where to find it.

For example, take the first function of your program:

def charSplit(str):
    return [char for char in str]

Simple enough. charSplit('foo') returns ['f', 'o', 'o'].

Did you know we can do the same with list? list('foo') returns ['f', 'o', 'o'] as well.

So, we're down 1 function. Let's create a new one. You're wrapping your code quite nicely here:

if __name__ == "__main__":

This makes sure the code is only running when the file itself is called, not when it's imported by another file. How about taking it a step further to clean things up?

def main():
    # all code that was previously behind the mentioned if-statement goes here

if __name__ == "__main__":
    main()

Everything above the if are global variables. I'm not a fan of global variables, so we'll get rid of them later. But let's assume for a moment you want to keep them around.

Since some of those are only read and never overwritten (pseudo-constants), it's usually preferred to CAPITALIZE them. Makes them easier to recognize.

How about another function? It may be a bit overkill here since the main function I wrote earlier already covers everything and the code only has one purpose, but code usually grows. Function-creep comes in and all of a sudden you have 300 lines of code in the same function. Better to split things up already, keeps things easier. Besides, it gives me an excuse to tell something about arguments.

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    # code here

s wasn't a particularly descriptive variable name, so that's inp now (input is already taken.) Can you see this growing? While we're at variable names, having both a result and finalResult is somewhat confusing. The entire construct is somewhat confusing.

# Iterates through result and adds each string to the empty string, "finalResult".
for l in result:
    finalResult += l

You're really just copying result here, so finalResult can go. Well, almost. We still need to go to a string, so we're interested in ''.join(result). That takes an empty string, and joins all parts of the result list into it.

I don't like the while loop. There's 3 reasons for this and I'm going to fix 2 now and 1 later. Why? To keep things simple and easy to follow.

It starts out like this:

while found == False:

Did you know you can rewrite that to make it even more obvious what's going on?

while not found:

Which, if you're familiar with other languages, reads like while(!found). Python has the not operator, which returns True if the expression behind it is False.

The other end of the loop can be improved as well. You're printing the result of the cipher, while we may want to do something completely different with it. Perhaps the result should be used in a different function? That's not the cipher's problem. Let main figure that out. All in all, the code so far would look like this with the above taken into account:

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase

LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    found = False
    strIndex = 0
    letterIndex = 0
    result = []

    while not found:
        # Checks if the string has been converted to the cipher.
        if len(result) == len(inp):
            found = True
        # Adds a space to the result variable if the string index is a space.
        elif inp[strIndex] == " ":
            result.append(" ")
            strIndex += 1
        # Checks if the string index is equal to the letter index.
        elif list(inp)[strIndex].lower() == LETTERS[letterIndex]:
            result.append(LETTERS[letterIndex - 1])
            strIndex += 1
        #
        else:
            if len(LETTERS) - 1 == letterIndex:
                letterIndex = 0
            else:
                letterIndex += 1

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", -1))

if __name__ == "__main__":
    main()

We haven't actually used shift yet, but we'll need it to fix another problem in your code: it's a very limited Caesar cipher. After all, a proper Caesar cipher takes a shift parameter. How far should the text be shifter? 1 character? 2? 13? We can make this go round and round and round. That's one of the good things about a Caesar cipher.

I'm still not a fan of the while loop here, so let's fix 2 things at a time. If our algorithm is correct, we can simply iterate over the input and know the output is correct at the end without keeping track of a found variable. And while we're at it, let's try to remove the other variables that are just to keep track as well.

You made a good start with importing ascii_lowercase, so let's build on that. Did you know string.ascii_lowercase (just like string.lowercase) has an index function? It provides us with the position of a character in a string.

What if we use that index to retrieve a different value of the alphabet instead? After all, an index is just a number. We could modify that number to get a different character altogether.

  For every character in the input, if the character is a letter, get a different letter from the alphabet instead.

Oh, we can use a for loop instead of a while loop!

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase

LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    result = []

    for character in inp:
        if character in LETTERS:
            # If it's a letter, we cipher it
            result.append(LETTERS[ascii_lowercase.index(character) + shift])
        else:
            # If it's NOT a letter, we won't cipher it
            result.append(character)

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", -1))

if __name__ == "__main__":
    main()

See how much easier that is to read? Now we just need a minor modification to account for the limited length of the alphabet. After all, what happens if someone wants a shift of 40? Or 51? Please welcome the % modulo operator. Divide by the length of the alphabet (26) and return the remainder.

# Imports the lowercase alphabet as a whole string.
from string import ascii_lowercase

LETTERS = list(ascii_lowercase)

def caesar_cipher(inp, shift):
    """
    Take inp(ut) as string, shift as integer and return ciphered string.
    """
    result = []

    for character in inp:
        if character in LETTERS:
            # If it's a letter, we cipher it
            result.append(LETTERS[(
                ascii_lowercase.index(character) + shift
                ) % len(LETTERS)])
        else:
            # If it's NOT a letter, we won't cipher it
            result.append(character)

    return(''.join(result))

def main():
    print(caesar_cipher("abcde test bla bla bla", 51))

if __name__ == "__main__":
    main()

Now, compare this to the original version. Can you see how we got here? You started out right, and now we got something even better. And this can probably be improved even further, I'm fairly sure we don't need result to be a list in the first place. Have fun figuring that out :-)


CodeFights: Snake Game Description

Your task is to imitate a turn-based variation of the popular "Snake" game.
You are given the initial configuration of the board and a list of commands which the snake follows one-by-one. The game ends if one of the following happens:

the snake tries to eat its tail
the snake tries to move out of the board
it executes all the given commands.

Output the board configuration after the game ends. With the snake replaced by 'X' if the game ends abnormally.

Examples
input:
snakeGame([['.', '.', '.', '.'],
           ['.', '.', '<', '*'],
           ['.', '.', '.', '*']],
          "FFFFFRFFRRLLF")

output:
          [['.', '.', '.', '.'],
           ['X', 'X', 'X', '.'],
           ['.', '.', '.', '.']]

input:
snakeGame([['.', '.', '^', '.', '.'],
           ['.', '.', '*', '*', '.'],
           ['.', '.', '.', '*', '*']],
          "RFRF")

output:
          [['.', '.', 'X', 'X', '.'],
           ['.', '.', 'X', 'X', '.'],
           ['.', '.', '.', 'X', '.']]

input:
snakeGame([['.', '.', '*', '>', '.'],
           ['.', '*', '*', '.', '.'],
           ['.', '.', '.', '.', '.']],
          "FRFFRFFRFLFF")

output:
          [['.', '.', '.', '.', '.'],
           ['<', '*', '*', '.', '.'],
           ['.', '.', '*', '.', '.']]

Code
def snakeGame(gameBoard, commands):
    heads = {'v': (1, 0), '^': (-1, 0), '<': (0, -1), '>': (0, 1)}
    new_direc = {'v': {'L': '>', 'R': '<'},
                 '^': {'L': '<', 'R': '>'},
                 '<': {'L': 'v', 'R': '^'},
                 '>': {'L': '^', 'R': 'v'}}

    def find_snake():
        def get_next(x, y):
            for key, direc in heads.items():
                new_x, new_y = x + direc[0], y + direc[1]
                if new_x in range(len(gameBoard)) and new_y in range(len(gameBoard[0])):
                    if (new_x, new_y) not in snake:
                        if gameBoard[new_x][new_y] == '*':
                            return (new_x, new_y)

        # Get the head and the next body opposite of snake's direction
        snake = []
        for i, row in enumerate(gameBoard):
            for head in heads:
                if head in row:
                    snake.append((i, row.index(head)))
                    snake.append((snake[0][0] + heads[head][0] * -1, snake[0][1]  + heads[head][1] * -1))

        # Append the rest of the body
        while True:
            n = get_next(snake[-1][0], snake[-1][1])
            if n is None:
                break
            snake.append(n)

        return snake

    def move_snake(snake):
        head = gameBoard[snake[0][0]][snake[0][1]]
        new_x, new_y = snake[0][0] + heads[head][0], snake[0][1]  + heads[head][1]
        new_snake = []
        if new_x in range(len(gameBoard)) and new_y in range(len(gameBoard[0])) and (new_x, new_y) not in snake:
            new_snake.append((new_x, new_y))

            for pos in snake[:-1]:
                new_snake.append(pos)

            return new_snake

    # Find starting snake
    snake = find_snake()

    for command in commands:
        if command in "LR":
            # Change the head
            gameBoard[snake[0][0]][snake[0][1]] = new_direc[gameBoard[snake[0][0]][snake[0][1]]][command]
        else:
            temp = move_snake(snake)

            # if not valid move return dead snake
            if temp is None:
                for pos in snake:
                    x, y = pos
                    gameBoard[x][y] = 'X'
                return gameBoard

            # else move snake
            for a, b in zip(snake, temp):
                gameBoard[b[0]][b[1]] = gameBoard[a[0]][a[1]]

            gameBoard[snake[-1][0]][snake[-1][1]] = '.'
            snake = temp

    return gameBoard

Your code seems to work and is split into small functions. Here are a few things to make it easier to understand.

Try to avoid nested functions

Nested functions can be really useful but in your case, it makes the code hard to understand because it is nested on many levels.

Also, because some parameters are not explicitly provided but somehow used as part of the functions context, it is hard to tell what is used/not used, input/output, updated/unchanged.

This is fairly easy to change to make the code more linear with the different functions defined one after the other (in order to do so, you need to add parameters in the function signatures and in the function calls). Also, you could take this chance to extract a few constant (and name them in UPPERCASE):

EMPTY = '.'
BODY = '*'
DEAD = 'X'
HEADS = {'v': (1, 0), '^': (-1, 0), '<': (0, -1), '>': (0, 1)}

def get_next(x, y, snake, gameBoard):
    for key, direc in HEADS.items():
        new_x, new_y = x + direc[0], y + direc[1]
        if new_x in range(len(gameBoard)) and new_y in range(len(gameBoard[0])):
            if (new_x, new_y) not in snake:
                if gameBoard[new_x][new_y] == BODY:
                    return (new_x, new_y)

def find_snake(gameBoard):
    # Get the head and the next body opposite of snake's direction
    snake = []
    for i, row in enumerate(gameBoard):
        for head in HEADS:
            if head in row:
                snake.append((i, row.index(head)))
                snake.append((snake[0][0] + HEADS[head][0] * -1, snake[0][1]  + HEADS[head][1] * -1))

    # Append the rest of the body
    while True:
        n = get_next(snake[-1][0], snake[-1][1], snake, gameBoard)
        if n is None:
            break
        snake.append(n)

    return snake

def move_snake(snake, gameBoard):
    head = gameBoard[snake[0][0]][snake[0][1]]
    new_x, new_y = snake[0][0] + HEADS[head][0], snake[0][1]  + HEADS[head][1]
    new_snake = []
    if new_x in range(len(gameBoard)) and new_y in range(len(gameBoard[0])) and (new_x, new_y) not in snake:
        new_snake.append((new_x, new_y))

        for pos in snake[:-1]:
            new_snake.append(pos)

        return new_snake

def snakeGame(gameBoard, commands):
    new_direc = {'v': {'L': '>', 'R': '<'},
                 '^': {'L': '<', 'R': '>'},
                 '<': {'L': 'v', 'R': '^'},
                 '>': {'L': '^', 'R': 'v'}}

    # Find starting snake
    snake = find_snake(gameBoard)

    for command in commands:
        if command in "LR":
            # Change the head
            gameBoard[snake[0][0]][snake[0][1]] = new_direc[gameBoard[snake[0][0]][snake[0][1]]][command]
        else:
            temp = move_snake(snake, gameBoard)

            # if not valid move return dead snake
            if temp is None:
                for pos in snake:
                    x, y = pos
                    gameBoard[x][y] = DEAD
                return gameBoard

            # else move snake
            for a, b in zip(snake, temp):
                gameBoard[b[0]][b[1]] = gameBoard[a[0]][a[1]]

            gameBoard[snake[-1][0]][snake[-1][1]] = EMPTY
            snake = temp

    return gameBoard

def test_snake(gameBoard, commands, expected):
    out = snakeGame(gameBoard, commands)
    if out != expected:
        print(out, commands, expected)

test_snake([['.', '.', '.', '.'],
            ['.', '.', '<', '*'],
            ['.', '.', '.', '*']],
           "FFFFFRFFRRLLF",
           [['.', '.', '.', '.'],
            ['X', 'X', 'X', '.'],
            ['.', '.', '.', '.']])
test_snake([['.', '.', '^', '.', '.'],
            ['.', '.', '*', '*', '.'],
            ['.', '.', '.', '*', '*']],
           "RFRF",
           [['.', '.', 'X', 'X', '.'],
            ['.', '.', 'X', 'X', '.'],
            ['.', '.', '.', 'X', '.']])
test_snake([['.', '.', '*', '>', '.'],
            ['.', '*', '*', '.', '.'],
            ['.', '.', '.', '.', '.']],
           "FRFFRFFRFLFF",
           [['.', '.', '.', '.', '.'],
            ['<', '*', '*', '.', '.'],
            ['.', '.', '*', '.', '.']])

Getting rid of indices (almost) everywhere

In your code, element access (foo[i]) is used everywhere:

to get/set a particular cell of the grid
to get the x (or y) part of coordinates
the get the direction from a particular "head" characters

It may make things confusing but you can use a few techniques to get rid of it:

When you iterate over a dictionnary, you can choose if you are interested in keys, values or both. Then you don't need to get my_dict[my_key].
You can use iterator unpacking when you know the number of elements in it. This is very convenient when you handle coordinates: x, y = my_coor. Also, this can be used in the for syntax: for head, (dx, dy) in HEADS.items().
Instead of using list access to get the element you've just added to the (empty) list, you can use temporary variables:

            snake.append((i, row.index(head)))
            snake.append((snake[0][0] + heads[head][0] * -1, snake[0][1]

becomes

                beginx, beginy = i, row.index(head)
                snake.append((beginx, beginy))
                snake.append((beginx - dx, beginy - dy))

It "costs" an additional line but it makes things clearer to me. (Also I took this chance to replace a + b * -1 by a - b).

Combining all these techniques, you'd get something like:

EMPTY = '.'
BODY = '*'
DEAD = 'X'
HEADS = {'v': (1, 0), '^': (-1, 0), '<': (0, -1), '>': (0, 1)}

def get_next(x, y, snake, gameBoard):
    for (dx, dy)  in HEADS.values():
        new_x, new_y = x + dx, y + dy
        if new_x in range(len(gameBoard)) and \
           new_y in range(len(gameBoard[0])) and \
           (new_x, new_y) not in snake and \
           gameBoard[new_x][new_y] == BODY:
            return (new_x, new_y)

def find_snake(gameBoard):
    # Get the head and the next body opposite of snake's direction
    snake = []
    for i, row in enumerate(gameBoard):
        for head, (dx, dy) in HEADS.items():
            if head in row:
                beginx, beginy = i, row.index(head)
                snake.append((beginx, beginy))
                snake.append((beginx - dx, beginy - dy))

    # Append the rest of the body
    while True:
        n = get_next(snake[-1][0], snake[-1][1], snake, gameBoard)
        if n is None:
            break
        snake.append(n)

    return snake

def move_snake(snake, gameBoard):
    headx, heady = snake[0]
    head = gameBoard[headx][heady]
    dx, dy = HEADS[head]
    new_x, new_y = headx + dx, heady + dy
    new_coord = new_x, new_y
    new_snake = []
    if new_x in range(len(gameBoard)) and \
       new_y in range(len(gameBoard[0])) and \
       new_coord not in snake:
        new_snake.append(new_coord)
        for pos in snake[:-1]:
            new_snake.append(pos)
        return new_snake

def snakeGame(gameBoard, commands):
    new_direc = {'v': {'L': '>', 'R': '<'},
                 '^': {'L': '<', 'R': '>'},
                 '<': {'L': 'v', 'R': '^'},
                 '>': {'L': '^', 'R': 'v'}}
    # Find starting snake
    snake = find_snake(gameBoard)

    for command in commands:
        if command in "LR":
            # Change the head
            headx, heady = snake[0]
            gameBoard[headx][heady] = new_direc[gameBoard[headx][heady]][command]
        else:
            temp = move_snake(snake, gameBoard)

            # if not valid move return dead snake
            if temp is None:
                for (x, y) in snake:
                    gameBoard[x][y] = DEAD
                return gameBoard

            # else move snake
            for a, b in zip(snake, temp):
                gameBoard[b[0]][b[1]] = gameBoard[a[0]][a[1]]

            tailx, taily = snake[-1]
            gameBoard[tailx][taily] = EMPTY
            snake = temp

    return gameBoard

Other code simplifications

Now that we provide the full snake to the get_next function, it doesn't really need to x and y values.

def get_next(snake, gameBoard):
    x, y = snake[-1]
    ....

In the move_snake function, you write a loop to add elements from snake[:-1]. You could use a simple + operation on lists.

Also, it is clearer to add return None at the end of a function whose return value is used. This is one of the latest addition to PEP 8:

  Be consistent in return statements. Either all return statements in a
  function should return an expression, or none of them should. If any
  return statement returns an expression, any return statements where no
  value is returned should explicitly state this as return None, and an
  explicit return statement should be present at the end of the function
  (if reachable).

Then, the whole function becomes:

def move_snake(snake, gameBoard):
    headx, heady = snake[0]
    head = gameBoard[headx][heady]
    dx, dy = HEADS[head]
    new_x, new_y = headx + dx, heady + dy
    new_coord = new_x, new_y
    if new_x in range(len(gameBoard)) and \
       new_y in range(len(gameBoard[0])) and \
       new_coord not in snake:
        return [new_coord] + snake[:-1]
    return None

Other ideas

At the moment, we have the snakes represented in 2 ways : as a list of body parts (which makes sense given how convenient it is to make the snake go forward) and as a drawing on a grid (which makes sense because it is our input and our output).

Also, for every step forward, we need to update both representations. Maybe, it would make sense to keep the listy-snake (and other data you might need) during the computations of the different steps and only generate the output grid at the very end.

As an exercice, I've tried to do this. I went for an object approach to store the different data needed and interact easily with them. I've tried to keep as much as possible from your code:

EMPTY = '.'
BODY = '*'
DEAD = 'X'
HEADS = {'v': (1, 0), '^': (-1, 0), '<': (0, -1), '>': (0, 1)}

class Snake(object):
    # Heads and body parts are (x, y, char)
    def __init__(self, head, dimx, dimy):
        self.dimx = dimx
        self.dimy = dimy
        self.body = [head]
        self.alive = True

    def add_queue(self, body_part):
        self.body.append(body_part)

    def turn(self, direc):
        new_direc = {'v': {'L': '>', 'R': '<'},
                     '^': {'L': '<', 'R': '>'},
                     '<': {'L': 'v', 'R': '^'},
                     '>': {'L': '^', 'R': 'v'}}
        x, y, head = self.body[0]
        new_head = new_direc[head][direc]
        self.body[0] = (x, y, new_head)

    def move_forward(self):
        x, y, char = self.body[0]
        dx, dy = HEADS[char]
        new_x, new_y = x + dx, y + dy
        if self.position_is_free(new_x, new_y):
            self.body = [(new_x, new_y, char)] + [(x, y, BODY)] + self.body[1:-1]
        else:
            self.die()

    def position_is_free(self, x, y):
        return x in range(self.dimx) and \
               y in range(self.dimy) and \
               not any(x == x2 and y == y2 for (x2, y2, _) in self.body)

    def die(self):
       self.alive = False
       self.body = [(x, y, DEAD) for (x, y, _) in self.body]

    def get_as_grid(self):
        g = [[EMPTY for i in range(self.dimy)] for j in range(self.dimx)]
        for x, y, c in self.body:
            g[x][y] = c
        return g

def find_head(gameBoard):
    for i, row in enumerate(gameBoard):
        for head, (dx, dy) in HEADS.items():
            if head in row:
                return Snake((i, row.index(head), head), len(gameBoard), len(gameBoard[0]))

def get_next(snake, gameBoard):
    x, y, _ = snake.body[-1]
    for (dx, dy) in HEADS.values():
        new_x, new_y = x + dx, y + dy
        if snake.position_is_free(new_x, new_y) and \
           gameBoard[new_x][new_y] == BODY:
            return (new_x, new_y, BODY)

def find_snake(gameBoard):
    # Get the head
    s = find_head(gameBoard)

    # Append the rest of the body
    while True:
        n = get_next(s, gameBoard)
        if n is None:
            break
        s.add_queue(n)
    return s

def snakeGame(gameBoard, commands):
    # Find snake
    s = find_snake(gameBoard)

    for command in commands:
        if command in "LR":
            # Change the head
            s.turn(command)
        else:
            s.move_forward()
            if not s.alive:
                break
    return s.get_as_grid()

Something I had forgotten

if new_x in range(len(gameBoard)) can be rewritten if 0 <= new_x < len(gameBoard).

On Python2, this is much faster (because a list is generated and then we perform a linear search on it).
On Python3, the performance impact may not be so big.


Who is this Chatbot? Based off the coding puzzle: Who is this chatbot?

The code's purpose is to create an AI that the user can communicate with. Due to there being no hard-coded strings, the AI learns from what the user has said.

In my version of this AI, all messages said by the user are stored in an array and when it comes time for the computer to speak, a random message from the array is written to standard output.

The code

Main.java

public class Main {
    public static void main(String[] args) {
        String name;
        if(args.length >= 1) {
            name = args[0];
        } else {
            name = "You";
        }
        Conversation c = new Conversation(new AI(), new User(name));
        while(true) {
            String input = c.getUser();
            if(input.equals(":exit")) {
                System.exit(0);
            }
            c.getAI();
        }
    }
}

Converstaion.java

public class Conversation {
    private AI ai;
    private User user;

    private DynamicArray saidStrings = new DynamicArray(1);
    private int saidCount = 0;

    public Conversation(AI a, User u) {
        ai = a;
        user = u;
    }

    public String getUser() {
        String input = user.getInput();
        saidStrings.push(input);
        saidCount++;
        return input;
    }
    public void getAI() {
        ai.saySomething(saidStrings, saidCount);
    }
}

AI.java

import java.util.Random;

public class AI {
    private Random rand = new Random();

    public void saySomething(DynamicArray saidStrings, int saidCount) {
        System.out.printf("Bot: %s\n", saidStrings.get(getRandom(saidCount)));
    }
    public int getRandom(int max) {
        return rand.nextInt(max);
    }
}

User.java

import java.util.Scanner;

public class User {
    private Scanner input = new Scanner(System.in);

    private String name;

    public User(String n) {
        name = n;
    }

    public String getInput() {
        System.out.printf("%s: ", name);
        return input.nextLine();
    }
}

DynamicArray.java

/*

Strongly based off of: http://stackoverflow.com/questions/3536153/c-dynamically-growing-array

*/

public class DynamicArray {
    private String[] array;
    private int used;
    private int size;

    public DynamicArray(int initialSize) {
        array = new String[initialSize];
        used = 0;
        size = initialSize;
    }

    public void push(String e) {
        if(used == size) { // if the limit has been reached
            size *= 2;

            String[] temp = array; //  a median for the trade
            array = new String[size];
            copy(temp, array);
        }
        array[used++] = e; // append the provided element
    }

    public String get(int i) {
        return array[i];
    }

    private void copy(String[] src, String[] dest) {
        System.arraycopy(src, 0, dest, 0, src.length);
    }
}

Concerns:

Am I following common OOP practices?
Am I following common Java practices
Do I have too many classes?
Do I have too few classes?
Am I handling input and output in the correct places?
Do I have too few/many comments?

Any other recommendations for improvement are welcome.

Your DynamicArray class is mimicking an ArrayList which is exactly that: a dynamic array. Use this instead of reinventing the wheel.

Use break; instead of System.exit(0) to exit a loop. It leaves room to add more instructions beyond the loop without having to change the implementation. It's also a more natural way to break out of a loop.

getUser() and getAI() indicate a return type of User and AI respectively. Choose naming that indicates you are receiving their input.

Write your parameter names in full (user instead of u, name instead of n).

Adhere to the single responsibility principle. Printing output and receiving input are two distinct tasks and shouldn't be done in the same method.

Concerning OO principles: perhaps create an interface Conversationist that defines a method String getSentence(). Now your User can implement this as receiving console input while the AI implements it by taking a random value from its stored text. This makes sure you can just call getSentence() for both objects and introduces some inheritance structure.


Regex to select the last components of a file path I have a regex sequence that will take the end of a file path and select it for me:

  resources/services/dealerInfoRequest/V5-0/dealerInfoRequestManager.cfc

In this, the regex would select:

  /V5-0/dealerInfoRequestManager.cfc`

In some cases there is no V5-0 and it's just the file name and extension. My regex is working as shown but I was wanting to see if there was any optimization I could do, if it could be cleaned up, or if I should just leave it in the order it is now.

Regex is as shown:

((\/)(\w\d-?\d)(\/)\w.+)|(\/)(\w+\.\w+)

It's used in JavaScript and in Sublime Text 3.

Your regex has too many capturing groups.
Using (?:) makes it into a simple matching group, speeding up the regex since it doesn't have to store anything.

Since you are trying to get the part from the end of the string, anchor it there using $.

At the end, you have this: (\/)(\w+\.\w+).

This is really bad in 2 ways:

You are using 2 capturing groups.
You regex only allows files with letters. Spaces, underscores and others should de allowed too.

For version control, you have this: (\/)(\w\d-?\d).

This allows me to have a folder called t0-0 which makes no sense.
Be more strict about your match.

This is the regex I came up with:

\/(?:[vV][1-9]\d?(?:-\d)?\/)?[^\/]+\.[^\.]+$

It addresses all your issues.

Explaining each part:

(?:\/[vV][1-9]\d?(?:-\d)?\/)? matches the version number, if there is any.
1.1. No need for capturing groups.

1.2. It enforces that the folder starts with v or V with.

1.3. The version only can starts with 1-9 and can have any number or digits.

1.4. The version folder is still optional
[^\/]+\.[^\.]+$ matches the file name.
2.1. Again: No need for capturing groups.

2.2. The file can have ANY name.

2.3. The file can have ANY extension, but there must be one.
The whole string is matched to the end, which makes it easier for us to write it.


Command line reverse polish calculator Write the program expr, which evaluates a reverse Polish expression from the command line, where each operator or operand is a separate argument.

  For example:

expr 2 3 4 + *

There are 4 files:

calc.c

#include <stdio.h>
#include <stdlib.h>
#include "calc.h"

int main(int argc, char *argv[]) {
    double tmpOp;
    char c;

    if(argc > 3) {

        while(--argc) {
            c = getOp(*++argv);
            switch(c) {
                case NUMBER:
                    push(atof(*argv));
                    break;
                case '+':
                    push(pop() + pop());
                    break;
                case '-':
                    tmpOp = pop();
                    push(pop() - tmpOp);
                    break;
                case '*':
                    push(pop() * pop());
                    break;
                case '\\':
                    tmpOp = pop();

                    if(!tmpOp) {
                        printf("error: can't divide by 0\n");
                    }
                    else {
                        push(pop()/tmpOp);
                    }
                    break;
                default:
                    printf("error: invalid expression %s\n", *argv);
            }
        }
        printf("%g\n", pop());
    }
    else {
        printf("invalid call\n");
    }

    return 0;
}

This file consist in a while loop that runs argc-1 times. At each iteration an argument is analized. If it is a number, it is pushed to the stack. If it is an operator, 2 numbers are popped from the stack and the operator is applied to these operands. If the argument is not valid, an error is printed.

argc should allways be bigger than 3 because, at this moment, the calculator needs at least 2 operands and 1 operator for an valid call.

The file getop.c:

#include <ctype.h>
#include "calc.h"

static int isNumber(const char *s) {
    int i = 0;

    if(isdigit(s[i])) {
        for(; isdigit(s[i]); i++)
            ;
    }

    if(s[i] == '.') {
        i++;
        for(; isdigit(s[i]); i++)
            ;
    }

    if(s[i] == 'e' || s[i] == 'E') {
        i++;
        for(; isdigit(s[i]); i++)
            ;
    }

    if(s[i] != '\0') {
        return 0;
    }
    return 1;
}

char getOp(const char *op) {
    int i;

    if(!isdigit(op[0]) && op[0] != '.') {
        return op[0];
    }

    i = 0;

    /* support for negative number */
    if(op[0] == '-' && !isdigit(op[1])) {
        return op[0];
    }
    else {
        i++;
    }
    if(isNumber(op + i)) {
        return NUMBER;
    }

    return op[0];
}

The function getOp returns 'n' if op contains an valid number or the first charachter of op if it is not a number - for example when we are dealing with operators.
The function getOp uses a helper to check if the the value of op is a valid number. Also, the call getOp(op + i) is intended to validate checks for negative number.

The file stack.c:

#include <stdio.h>
#define STACKVALUE 100

static double stack[STACKVALUE];
static double *pStack = stack; /* next free position in stack */

void push(double value) {
    if(pStack - stack > STACKVALUE) {
        printf("error: there is not enough psace in stack");
    }
    else {
       *(pStack++) = value;
    }
}

double pop(void) {
    if((pStack - stack) > 0) {
        return *--pStack;
    }

    printf("warning: the stack is free");
    return 0.0;
}

And the file calc.h:

#define NUMBER 'n'

int getOp(const char *s);
void push(double value);
double pop(void);

What you did well

Taking advantage of the shell to split your expression into tokens is smart.  It saves you from the trouble of having to write a tokenizer.

Beware, though, of a usability issue with Unix shells, where * and \ have special significance and need to be quoted or escaped.

Bugs

Compilation error: This was just carelessness.

getop.c:31:6: error: conflicting types for 'getOp'
char getOp(const char *op) {
     ^
./calc.h:3:5: note: previous declaration is here
int getOp(const char *s);
    ^
1 error generated.

Error handling: You treat errors like warnings.  If there's an error, I would expect the program to print nothing to stdout, and for the program to exit with a non-zero status code.  Error and warning messages should be printed to stderr instead of stdout.
Picky validation: In my opinion, this should successfully evaluate to 1:

$ ./expr 1
invalid call

By the way, if you find an insufficient number of command-line arguments, just return early.

int main(int argc, char *argv[]) {
    if (argc <= 1) {
        fprintf(stderr, "invalid call\n");
        return 1;
    }

    while (--argc) {
        …
    }
    printf("%g\n", pop());
    return 0;
}

Mishandling of negative numbers: I would expect the following calculation to yield -1:

$ ./expr 3 -4 +
warning: the stack is freewarning: the stack is free-3

Unconventional division operator: It is customary to use / for division.  I don't know why you use \ instead.
Division-by-zero paranoia: In contrast to integer arithmetic, where division by zero is undefined behaviour, division by zero in IEEE 754 floating point arithmetic should just return infinity.  Perhaps you could remove the check for division by zero.

Number parsing

I suggest the following implementation for getop.c, which is simpler and handles negative numbers.

#include <ctype.h>
#include "calc.h"

static int isNumber(const char *s) {
    if (*s == '-' || *s == '+') s++;
    if (*s == '\0') {
        return 0;
    }

    while (isdigit(*s)) s++;

    if (*s == '.') {
        s++;
        while (isdigit(*s)) s++;
    }

    if (*s == 'e' || *s == 'E') {
        s++;
        while (isdigit(*s)) s++;
    }

    return *s == '\0';
}

char getOp(const char *op) {
    return isNumber(op) ? NUMBER : *op;
}


Shorten Time Return Function I have this function which returns the time that a Tweet was created based on the created_at value that comes when a timeline is pulled. I also check whether or not the tweet was created more than 2 hours ago, and if it was, have an action execute. I was wondering, is there a way that this code could be shortened down because it currently feels very long. I thought about using a switch statement instead of all the ifs, but it didn't really shorten the function down at all.

function timeAgo(dateString) {
   var rightNow = new Date();
   var then = new Date(dateString);
   if ($.browser.msie) {
      then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
   }
   var diff = rightNow - then;
   var second = 1000;
   var minute = second * 60;
   var hour = minute * 60;
   var day = hour * 24;
   var week = day * 7;
   if (isNaN(diff) || diff < 0) {
      return "";
   }
   if (diff < second * 2) {
      return {tAgo: "right now", timeCheck: false};
   }
   if (diff < minute) {
      return {tAgo: Math.floor(diff / second) + " seconds ago", timeCheck: false};
   }
   if (diff < minute * 2) {
      return {tAgo: "about 1 minute ago", timeCheck: false};
   }
   if (diff < hour) {
      return {tAgo: Math.floor(diff / minute) + " minutes ago", timeCheck: false};
   }
   if (diff < hour * 2) {
      return {tAgo: "about 1 hour ago", timeCheck: false};
   }
   if (diff < day) {
      return {tAgo: Math.floor(diff / hour) + " hours ago", timeCheck: true};
   }
   if (diff > day && diff < day * 2) {
      return {tAgo: "yesterday", timeCheck: true};
   }
   if (diff < day * 365) {
      return {tAgo: Math.floor(diff / day) + " days ago", timeCheck: true};
   }
   else {
      return {tAgo: "over a year ago", timeCheck: true};
   }
}

Here is how i'd approach it

// Make all those units constants
var SECOND = 1000;
var MINUTE = SECOND * 60;
var HOUR = MINUTE * 60;
var DAY = HOUR * 24;
var YEAR = DAY * 365;

// store the string variations in a table, seperate from the code
TIMES = [
    {min: 0, text: "right now", unit: SECOND},
    {min: 2 * SECOND, text: "@ seconds ago", unit: SECOND},
    {min: MINUTE, text: "about 1 minute ago", unit: MINUTE},
    {min: 2 * MINUTE, text: "@ minutes ago", unit: MINUTE},
    {min: HOUR, text: "about 1 hour ago", unit: HOUR},
    {min: 2 * HOUR, text: "@ hours ago", unit: HOUR},
    {min: DAY, text: "yesterday", unit: DAY},
    {min: 2 * DAY, text: "@ days ago", unit: DAY},
    {min: YEAR, text: "over a year ago", unit: YEAR}
];

// use the table to generate the date
function timeAgoText(diff) {
    var use;
    for(var index = 0, length = TIMES.length; index < length; index++)
    {
        if( TIMES[index].min < diff )
        {
            use = TIMES[index];
        }
    }
    units = Math.floor(diff / use.unit)
    return use.text.replace("@", units)
}

function timeAgo(dateText)
{
   var then = new Date(dateString);
   if ($.browser.msie) {
      then = Date.parse(dateString.replace(/( \+)/, ' UTC$1'));
   }
   var diff = new Date() - then;
   if( isNaN(diff) || diff < 0 )
   {
       return ""
   }
   else
   {
       // seperate the time check and the text generation
       return {
           tAgo: timeAgoText(diff),
           timeCheck: diff > HOUR * 2
       }
   }
}


SHOUTY_SNAKE_CASED NUMBERS WE_ALL_LOVE_SHOUTY_SNAKE_CASE_SO_I_WROTE_A_PROGRAM_FOR_IT!!1!

Alright, enough of this. This is a rags-to-riches from this question. The pattern is pretty simple, it takes a String input, verifies if it is numerical, and if so, converts it to SHOUTY_SNAKE_CASE.

  Example : "1231" -> "ONE_TWO_THREE_ONE"

It is fairly simple but it is also the first time I used the Java 8's streams, so I wanted an opinion on my (simple) use of it.

private static final String[] words =
        new String[]{"zero","one","two","three","four","five","six","seven","eight","nine"};

public static String convertNumbersToWords(final String input) {

    if(input == null || !input.matches("^\\d*$")){
        throw new IllegalArgumentException("input cannot be null or non-numerical.");
    }

    return input.chars()
             .mapToObj(c -> getWordFromCharCode(c).toUpperCase())
             .collect(Collectors.joining("_"));
}

private static String getWordFromCharCode(int code){
    return words[code - 48];
}

Why are you storing them as lowercase words only to call toUpperCase on them? Store them in the form you intend to use them and save yourself a function call on each word. It's also more understandable to have the strings stored the same way
they'll be printed.


Encode message by alphabets - Follow Up This code is a revised version of implementation which asked for a advice. Original question is here: Encode message by alphabets
#include <stdio.h>
#include <stdlib.h>
#define MIN_ALPH 1
#define MAX_ALPH 26

unsigned int my_decode(unsigned int input)
{
    unsigned int count = 0;
    unsigned int ddigit;
    int i;

    //check double digit decoding
    //TODO: make macro for (num >= MIN_ALPH && num <= MAX_ALPH)
    if (input % 100 >= MIN_ALPH && input % 100 <= MAX_ALPH)
        count++;

    if (input / 10 >= MIN_ALPH && input / 10 <= MAX_ALPH)
    {
        if (input % 10 > 0)
            count++;
    }

    //check single digit decoding
    for (i=1; i <= 100; i*=10)
    {
        if (input % (i *10) / i == 0)
        break;
    }
    if (i == 1000)
        count++;

    return count;
}

int main(void)
{
    /*Given the mapping a = 1, b = 2, ... z = 26, and an encoded message,
    count the number of ways it can be decoded.
    For example, the message '111' would give 3,
    since it could be decoded as 'aaa', 'ka', and 'ak'.
    You can assume that the messages are decodable.
    For example, '001' is not allowed.*/

    printf("result: %u\n", my_decode(512));
    printf("result: %u\n", my_decode(542));
    printf("result: %u\n", my_decode(112));

}

Test clarity
A sample such as with my_decode(512) deserves a break down of how, as letters, it might be encoded.
Add as a comment, or integrate in test, the expected outputs.
Posting input as well as output is useful.
printf("%u --> result: %u\n", 512, my_decode(512));

Format
Other code and below hints OP is not using an auto formatter as break is not indented.  1) Recommend to use an auto-formatter 2) Prefer { }
    if (input % (i *10) / i == 0)
    break;
    // vs.
    if (input % (i *10) / i == 0) {
        break;
    }

Macro vs. code
Consider a helper function
bool alph_in_range(unsigned num) {
  return num >= MIN_ALPH && num <= MAX_ALPH;
}

Function
my_decode(102) is 2 and my_decode(1002) is 1.  I'd expect 2 for the first is allowed, 10,2 and 1,02 then 10,02 and 1,002 allowed for the 2nd.

Not much else to say.
Minor: _MAX
...._MAX more common in C such as INT_MAX
// #define MIN_ALPH 1
// #define MAX_ALPH 26

#define ALPH_MIN 1
#define ALPH_MAX 26

Minor: unsigned vs. unsigned int
Either works.  unsigned is shorter.
As with such style issues, code to your group's coding standard.
Minor: Mixed types
Some coding standards dislike  unsigned % int.
Could use input % 100u vs. input % 100.


Implementing LSD radix sort I would love a code review of this LSD radix sort implementation. I've rolled my own, and have implemented counting sort as well. I feel like some of the data structures I've chosen could be improved. My List<List<char[]>>, for instance, is a little gross and makes fiddling with its innards more complex than I think needs to be.

public class LSDSorting {

private static final int DIGIT_RANGE = 5;

public static void main(String[] args) {
    char[][] toSort = new char[][]{"0123".toCharArray(), "1233".toCharArray(), "1212".toCharArray(), "1111".toCharArray(), "4444".toCharArray()};

    int LSD_INDEX = toSort[0].length - 1;
    char[][] sorted = lsdSort(toSort, LSD_INDEX);

    for (char[] str: sorted) {
        System.out.println(String.valueOf(str));
    }
}

private static char[][] lsdSort(char[][] toSort, int d) {
    if (d < 0) {
        return toSort;
    }

    char[][] sortedOnD = runCountingSort(d, toSort, DIGIT_RANGE);
    return lsdSort(sortedOnD, d-1);
}

private static char[][] runCountingSort(int d, char[][] toSort, int range) {
    List<List<char[]>> idx = new ArrayList<>();
    for (int i = 0; i < range; i++) {
        idx.add(i, new ArrayList<char[]>());
    }

    for (int i = 0; i < toSort.length; i++) {
        int currVal = Character.getNumericValue(toSort[i][d]);
        List<char[]> currList = idx.get(currVal);
        if (currList == null) {
            currList = new ArrayList<>();
            idx.add(currVal, currList);
        }
        currList.add(toSort[i]);
    }

    char[][] result = new char[toSort.length][toSort[0].length];
    int currIdx = 0;
    for (int i = 0; i < idx.size(); i++) {
        for (char[] str : idx.get(i)) {
            result[currIdx] = str;
            currIdx++;
        }
    }
    return result;
}
}

An important thing about sorting is that it has to be fast and that it does not fill your entire heap space. That's why quicksort often is prefered before merge sort.

And I think that your code is eating way more memory than it should.

You don't need the recursion in lsdSort, I think it makes it more complex and less efficient. You may get unexpected stackoverflow exceptions as well for long strings.
At each iteration you are creating new instances for everything you need. The char matrix and your list of lists. You may reuse the same copies each iteration if you throw away the recursion.
Don't use List or any implementation of it when you exactly know the size and you aren't interacting with any other methods. (as you probably may know there is an overhead)
List<char[]> currList = idx.get(currVal); followed by a nullcheck seems to make no sense for your code? List<E>.get()

How would I do it:
The best implementation I've seen so far is from a book of Robert Sedgewick.

/**
 * Rearranges the array of W-character strings in ascending order.
 *
 * @param a the array to be sorted
 * @param W the number of characters per string
 */
public static void sort(String[] a, int W) {
    int N = a.length;
    int R = 256;   // extend ASCII alphabet size
    String[] aux = new String[N];

    for (int d = W-1; d >= 0; d--) {
        // sort by key-indexed counting on dth character

        // compute frequency counts
        int[] count = new int[R+1];
        for (int i = 0; i < N; i++)
            count[a[i].charAt(d) + 1]++;

        // compute cumulates
        for (int r = 0; r < R; r++)
            count[r+1] += count[r];

        // move data
        for (int i = 0; i < N; i++)
            aux[count[a[i].charAt(d)]++] = a[i];

        // copy back
        for (int i = 0; i < N; i++)
            a[i] = aux[i];
    }
}

I think your idea with using a list of lists is good. And may looks faster since you are iterating one time less by skipping the cumulated counts. Still, it's clean and simple.

LSD.java


SQL Server 'Execute As'/Revert pattern in a 'Try/Catch' Block I wish to ensure I am using the "best" pattern when using an Execute As/Revert from within a Try/Catch block on SQL Server 2012. The below code "seems" to behave correctly... Am I missing anything or are there any security concerns, "better" approaches, etc.?

Below is my code:

CREATE PROCEDURE [dbo].[TryItOut]
WITH EXECUTE as 'NoTable1Access' --does not have access (select) to Table1!
AS
Begin
  Declare @execErrorNumber int,
           @execErrorMessage nvarchar(2048),
           @xactState smallint

  Begin Try
    Execute as user='HasTable1Access'
    select *, 1/0 as [SimulateError] from [T1].[Table1]; -- This will be a Stored Procedure call, but a select statement was easier to demo...
    Revert --Revert on 'HasTable1Access'
  End Try
  Begin Catch;

        select @execErrorNumber = ERROR_NUMBER(),
               @execErrorMessage = ERROR_MESSAGE(),
               @xactState = XACT_STATE();

        Revert -- Revert on 'HasTable1Access' when in error...
        --Do some error processing in context of 'NoTable1Access'
   End Catch

   select * from [T1].[Table1] --Should NOT have any access now and select should fail...
End

One thing to be wary of is if you grant execute permissions on a schema in sql server, then no matter what's in the Stored Procedure, the user is granted rights to execute that stored procedure and whatever it entails.

So I think that, even if you change user contexts, that SP will work no matter what.


"Guess my Number" game in C++ I have recently created my first game in C++:

#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;
int main()
{
    cout<<"\tWelcome to Guess my Number!\n";
    const unsigned short kusiLower=1;
    const unsigned short kusiHigher=100;
    srand(time(0));
    unsigned short usiComp=rand()%kusiHigher+kusiLower;
    cout<<"Guess a number between 1 and 100: ";
    unsigned short usiUser;
    cin>>usiUser;
    unsigned short usiGuesses=1;
    while(usiUser!=usiComp)
    {
        cout<<((usiUser>usiComp)?"Your guess is too high.\n":"Your guess in too low\n");
        cout<<"Guess a number between 1 and 100: ";
        cin>>usiUser;
        ++usiGuesses;
    }
    cout<<"Congratulations! You guessed my number in "<<usiGuesses<<" guesses.";
}

And I came here to ask the standard question: can it be in any way optimized or improved?

a few remarks:

Use int instead of unsigned short
Don't use Hungarian naming ('kusi/usi' prefix) - nobody uses that.
Add argc/argv parameters to main
Dont import everything from std with using namespace std
Add spaces around operators (=, != , ==, >>, << etc) and after 'while', 'if' etc.
You have carefully used constants for the high/low boundaries, but your print statements  don't use these constants ("Guess a number between 1 and 100:")


Finding the longest word from a list of words I have a list of words (with repetitions), and I intend to find out the longest words amongst them, along with their length. My question is to know if my implementation is too verbose, is using too many operators/functions and if so, what are the better, more idiomatic, readable alternatives.

Given the list below, the expected output is

 Max length=3, words are "mnp","pqr","jkl","abc","xyz"

    val line = List("a","ab","abc","xyz","mnp","ac","d","b","ab","bc","bd","be","bf","b","abc","abc","pqr","mnp","jkl","a","b")

    val lengthAndTheWords = line.map(x => (x.length,x)).groupBy(x => x._1).mapValues(x => x.toSet)
    val longest = lengthAndTheWords.toSeq.sortBy(key => -key._1).head

    println("Max length =" +
            longest._1 +
            ", words are" +
            longest._2.map(nextEntry => nextEntry._2).mkString(","))

(With an Ideone running the code)

I consider myself an intermediate level Scala programmer; so, I am looking for all tips to improve my skills in Scala.

Whenever I only use the extracted value once in a simple group / map / sortBy monadic operations I use the _ placeholders.  I think it makes the code a little more compact and readable, especially if it was just a placeholder value like x

.groupBy(x => x._1) becomes .groupBy(_._1)

.mapValues(x => x.toSet) becomes .mapValues(_.toSet) or even mapValues toSet if you like infix

It's not a universal rule by any means, sometimes you really need a meaningful name so that another coder could make sense of it.

Edit
Also you can go straight to the group by and remove redundant counts

val oneLiner = line.groupBy(_.length).mapValues(_.toSet).maxBy(_._1)

//> oneLiner  : (Int, scala.collection.immutable.Set[String]) = (3,Set(abc, pqr, mnp, jkl, xyz))


Generate random characters from ranges 0–9 and a–Z I’m trying to generate 5 characters from the ranges 0–9 and a–Z. Is my solution optimal or did I overthink it a little? Would hardcoding a character list be more optimal?

const numbers = [
    ...Math.random()
        .toString(36)
        .substr(2, 5),
].map(element => (Math.random() > 0.5 ? element : element.toUpperCase())).join('');

Small Bias
First a nit pick, Math.random() generates a number from 0 to < 1. It will never generate 1. Thus to get a statistical odds of 1/2 you must test either Math.random() < 0.5 or Math.random() >= 0.5. Testing Math.random() > 0.5 ? char : char.toUpperCase() will give a very (VERY) small bias in favor of upper case characters.
Dramatic Bias
Also there is a very strong bias towards numerals in your function with a 0-9 being 2 times more likely than a-z or A-Z
The following snippet counts the occurrence of each character generated by your function and plots them (normalized) on a graph.
I animated it slowly for dramatic effect.

canvas.width = innerWidth - 10;
const ctx = canvas.getContext("2d");
const w = canvas.width;
const h = canvas.height;
const chars = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
var testCount = 2;
const val = new Array(chars.length)
val.fill(0);
const func = randString;
function randString() {
    return [
        ...Math.random()
            .toString(36)
            .substr(2, 5),
    ].map(element => (Math.random() > 0.5 ? element : element.toUpperCase())).join('');
}
function testRandomness() {
    var i = testCount;
    while (i--) {
        const a = func();
        for (const c of a) {  val[chars.indexOf(c)] += 1 }
    }

    const max = Math.max(...val);
    ctx.clearRect(0, 0, w, h);
    const ww = w / val.length;
    i = val.length;
    ctx.fillStyle = "blue";
    while (i--) {
        if (chars[i] === "z") { ctx.fillStyle = "green" }
        if (chars[i] === "9") { ctx.fillStyle = "red" }
        const v = val[i] / max * h;
        ctx.fillRect(i * ww, h - v, ww- 2, v)
    }
    setTimeout(testRandomness, 1000 / 30);
}
testRandomness();
canvas.addEventListener("click",()=> (testCount = 10000,d.textContent = "Sample rate ~300,000per sec") ,{once:true});
body { font-family: arial black; }
canvas {
  padding: 0px;
}
#a { color:red; }
#b { color:green; }
#c { color:blue; }
#d { font-family: arial; }
<canvas id="canvas"></canvas>
Red 0-9 Green a-z Blue A-Z Click graph to increase sample rate to 10000

Click the graph to increase the tests per sample to 10000 per 30th second (approx) and you will see that apart from the numeral bias the graph quickly becomes very flat showing no other major bias (the uppercase bias is way to small to see)
The reason for the bias is that you split the characters a-z in two when you convert half to uppercase.
Also note as pointed out in the other answer, there is a small chance that the returned string is less than 5 characters long.
Performance
In terms of performance using a lookup table is around 3 times faster ie calculate 1.5million strings in the time yours calculates 0.5million.
const chars = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
function randString(size = 5) {
    var str = "";
    while (size--) { str += chars[Math.random() * chars.length | 0] }
    return str;
}

As a function it is much more flexible, and can easily be adapted to include extra characters, or reduced character sets.
If all that matters is performance you can get around 10% faster by avoiding some of the readability introduce overhead with
function randString() {
    return  chars[Math.random() * 62 | 0] +
        chars[Math.random() * 62 | 0] +
        chars[Math.random() * 62 | 0] +
        chars[Math.random() * 62 | 0] +
        chars[Math.random() * 62 | 0];
}

Or (on chrome) the following is on average 2% faster than the one above but only after it has been run many time so that the optimizer knows what it does. If run only a few time it is slower than above
function randString3() {
    return  `${chars[Math.random()*62|0]}${chars[Math.random()*62|0]}${chars[Math.random()*62|0]}${chars[Math.random()*62|0]}${chars[Math.random()*62|0]}`;
}


Peak and valley finding algorithm I'm trying to improve my algorithm to make it faster. Also might help to have a few sets of eyes for any bugs or gaps in logic I didn't catch. One thing to note: because of the nature of the algorithm, there will be periods at the beginning and end of the data that are dead zones, where a peak/valley will not be detected.

The jist of the algorithm is to step through overlapping windows of the data, do some linear regression and pick out points by standard deviation. Because of the overlapping feature we can decide how many "angles" a point needs to be detected as a peak for it to be valid. Then we go and only take the highest peak from a group of consecutive peaks.

I have highly stationary data; what I may or may not want to detect as a peak varies, and I wanted the most robust algorithm so I can use something like Bayesian optimization to set the best parameters.

This is more or less how I'd like my algorithm to function for my specific use case; I'm not really looking for feedback on that, more on whether my code translates these goals correctly and if it's the most computationally efficient way to do things.

def get_peak_valley(arr, threshold, window_size, overlap, req_angles):
    # validate params
    window_size = int(round(window_size))
    req_angles = int(round(req_angles))
    window_step = int(round(window_size * (1 - overlap)))
    if window_step == 0:
        window_step = 1
    if req_angles == 0:
        req_angles = 1

    # get all points that classify as a peak/valley
    ind = 0
    peak_inds, valley_inds = [], []

    while ind + window_size <= len(arr):
        flattened = detrend(arr[ind:ind + window_size])
        std, avg = np.std(flattened), np.mean(flattened)
        lower_b = avg - std * threshold
        upper_b = avg + std * threshold
        for idx, val in enumerate(flattened):
            if val < lower_b:
                valley_inds.append(idx + ind)
            elif val > upper_b:
                peak_inds.append(idx + ind)
        ind += window_step

    # discard points that have counts below the threshold
    peak_counts = Counter(peak_inds)
    pk_inds = [c for c in peak_counts.keys() if peak_counts[c] >= req_angles]

    valley_counts = Counter(valley_inds)
    vly_inds = [c for c in valley_counts.keys() if valley_counts[c] >= req_angles]

    # initialize iterator to find to best peak/valley for consecutive detections
    if len(pk_inds) == 0 or len(vly_inds) == 0:
        return pk_inds, vly_inds

    if pk_inds[0] < vly_inds[0]:
        curr_event = 'peak'
        best_val = arr[pk_inds[0]]
    else:
        curr_event = 'valley'
        best_val = arr[vly_inds[0]]

    #iterate through points and only carry forward the index that has the highest or lowest value from the current group
    best_ind, new_vly_inds, new_pk_inds = 0, [], []

    event_inds = sorted(pk_inds + vly_inds)

    for x in event_inds:
        if x in pk_inds:
            is_peak = True
        else:
            is_peak = False

        if is_peak and curr_event == 'valley':
            new_vly_inds.append(best_ind)
            curr_event = 'peak'
            best_val = arr[x]
            best_ind = x
            continue
        if not is_peak and curr_event == 'peak':
            new_pk_inds.append(best_ind)
            curr_event = 'valley'
            best_val = arr[x]
            best_ind = x
            continue

        if is_peak and curr_event == 'peak' and arr[x] > best_val:
            best_val = arr[x]
            best_ind = x
        elif not is_peak and curr_event == 'valley' and arr[x] < best_val:
            best_val = arr[x]
            best_ind = x

    if curr_event == 'valley':
        new_vly_inds.append(best_ind)
    if curr_event == 'peak':
        new_pk_inds.append(best_ind)

    return new_pk_inds, new_vly_inds

Validation

# validate params
window_size = int(round(window_size))
req_angles = int(round(req_angles))
window_step = int(round(window_size * (1 - overlap)))

This does... some validation, but not a lot. It will validate that the arguments are numeric, but nothing else. If you cared about validation, you should probably also check ranges, especially that the window size and step are non-negative, etc.

Numpy vectorized conditionals

This:

    for idx, val in enumerate(flattened):
        if val < lower_b:
            valley_inds.append(idx + ind)
        elif val > upper_b:
            peak_inds.append(idx + ind)

should not use a loop. Read about vectorized conditionals here:
https://stackoverflow.com/questions/45768262/numpy-equivalent-of-if-else-without-loop#45768290

Stricter events

    curr_event = 'peak'

shouldn't use a string. Use enum.Enum instead and include event values for PEAK and VALLEY.

Direct booleans

    if x in pk_inds:
        is_peak = True
    else:
        is_peak = False

should be

is_peak = x in pk_inds


CR Stack Exchange crawler I am writing a program which automatically crawls codes from this site!

Would you please review my code?

The required .jars: jsoup, org.apache.commons.io.

Main.java:

public class Main {
    public static void main(String[] args) throws Exception {
        final String site1 = "http://codereview.stackexchange.com/questions/";
        String site2[] = {"69", "109", "131"};
        String site3[] = {"is-this-implementation-of-shamos-hoey-algorithm-ok",
                "are-there-any-ways-to-improve-my-http-request-path-parser",
                "law-of-demeter-and-data-models"};
        HtmlGetter htmlGetter = new HtmlGetter();

        for(int i = 0 ; i < site2.length ; ++i) {
            htmlGetter.setUrl(site1, site2[i], site3[i]);
            htmlGetter.download();
            htmlGetter.parse(i+1);
        }
    }
}

HtmlGetter.java:

import java.io.*;
import java.net.URL;
import java.util.Scanner;

import org.apache.commons.lang3.StringEscapeUtils;
import org.apache.commons.io.IOUtils;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.jsoup.select.Elements;

public class HtmlGetter {
    private URL url;
    private String fileName;

    public void setUrl(String site1, String site2, String site3) throws Exception {
        this.url = new URL(site1 + site2 + "/" + site3);
        this.fileName = "[" + site2 + "]" + site3 + ".html";
    }

    public void download() throws Exception {
        final InputStream in = url.openStream();
        final OutputStream out = new FileOutputStream(fileName);
        IOUtils.copy(in, out);
        in.close();
        out.close();
        divideFile();
    }

    private void divideFile() throws Exception {
        Scanner scanner = new Scanner(new File(fileName));
        String oneLine;
        final BufferedWriter out1 = new BufferedWriter(new FileWriter("question.html"));
        final BufferedWriter out2 = new BufferedWriter(new FileWriter("answer.html"));

        while(scanner.hasNextLine()) {
            oneLine = scanner.nextLine();
            if(oneLine.matches(".*
.*")) {
                while(scanner.hasNextLine()&&(!oneLine.matches(".*
.*"))) {
                    out1.write(oneLine);
                    out1.newLine();
                    oneLine = scanner.nextLine();
                }
            }
            if(oneLine.matches("")) {
                while(scanner.hasNextLine()&&(!oneLine.matches(".*
.*"))) {
                    out2.write(oneLine);
                    out2.newLine();
                    oneLine = scanner.nextLine();
                }
                break;
            }
        }
        out1.close();
        out2.close();
     }

    public void parse(int articleNumber) throws Exception {
        parse2("question.html", articleNumber);
        parse2("answer.html", articleNumber);
    }

    public void parse2(String strFileName, int articleNumber) throws Exception {
        File input = new File(strFileName);
        Document doc = Jsoup.parse(input, "UTF-8");
        Elements codes = doc.select("code");
        BufferedWriter out;
        int subNumber = 0;
        String qa = "x";

        for(Element code: codes)
        {
            if(strFileName.equals("question.html")) { qa = "q"; }
            else if(strFileName.equals("answer.html")) { qa = "a"; }
            else { System.out.println("Error!"); }
            out = new BufferedWriter(new FileWriter(qa + articleNumber + "-" + (++subNumber) + ".txt"));
            out.write(code.text().replaceAll("[\r\n]", "\r\n"));
            out.close();
        }
    }
}

While ZeroOne's answer has the good idea to use an enum, the enum itself is not implemented very well. Java enums are a powerful system, and using a switch statement inside the getFileName() method is very unnecessary. Furthermore, the naming conventions are a bit off. Here's a better implementation of the same enum:

private enum FileType {
    QUESTION("question.html"),
    ANSWER("answer.html");

    private final String fileName;

    FileType(String fileName) {
        this.fileName = fileName;
    }

    public String getFileName() {
        return fileName;
    }
}

This uses the unique capabilities of Java enums to store individual parameters. This also changes the names of the enum and its values. Java enum names should be upper camel-case (same as class names), while enum values should be all caps.