PHP Autoloader - For classes, traits and interfaces Please review my Autoloader, which is the first file included in index.php

Note that there is no resource with the same name for trait, interface and class. Each resource has it's own name. And even if some class is named "Ha" and some trait is named "Ha", one of them has specific namespace for this.

<?php
### Autoload class ###
class Autoload
{
    # The path for classes, traits or interfaces #
    private static $path  = 'private/Model/';

    # The resource files by default will be NULL #
    private static $resource_files = null;

    # Search for resource_files in recursive mode #
    private static function recursive_glob($pattern)
    {
        $resource_files = glob($pattern);
        foreach(glob(dirname($pattern).'/*', GLOB_ONLYDIR | GLOB_NOSORT) as $directory)
        {
            $resource_files = array_merge(
                $resource_files,
                self::recursive_glob($directory.'/'.basename($pattern))
            );
        }
        return $resource_files;
    }

    # Order array to start with files that has the name like the $resource or closely... #
    private static function order_resource_files($resource)
    {
        $resource_name = explode('\\', $resource);
        $resource_name = end($resource_name);

        foreach(self::$resource_files as $file_key => $file)
        {
            if(stripos($file, $resource_name) !== false)
            {
                self::$resource_files[-1] = $file;
                unset(self::$resource_files[$file_key]);

                ksort(self::$resource_files);
                self::$resource_files = array_values(self::$resource_files);
            }
        }
    }

    # Find resource #
    public static function find($resource)
    {
        # Set $resource_files if is NULL #
        if(!self::$resource_files)
        {
            self::$resource_files = self::recursive_glob(self::$path.'*.php');
        }

        # Order $resource_files by given resource #
        self::order_resource_files($resource);

        # Now that we have the $resource_files array, we can start searching #
        foreach(self::$resource_files as $file_key => $file)
        {
            # First, remove the file from array #
            unset(self::$resource_files[$file_key]);

            # Include file #
            require_once $file;

            # If resource exists now, then we can stop and search for other resources later #
            if(class_exists($resource) || trait_exists($resource) || interface_exists($resource))
            {
                break;
            }
        }
    }
}

### Check PHP version and call Autoload::find ###
if(version_compare(PHP_VERSION, '5.1.2', '>='))
{
    if(version_compare(PHP_VERSION, '5.3.0', '>='))
    {
        spl_autoload_register('Autoload::find', true, true);
    }
    else
    {
        spl_autoload_register('Autoload::find');
    }
}
else
{
    function __autoload($resource)
    {
        Autoload::find($resource);
    }
}

First of all you should use static methods and properties only
when there is good reason to do so. Generally you should default to non-static.

$path property can be const because private static doesn't say whether it is immutable or not.
I think you shouldn't order resource files at all. You already have resource name. All you should do is match something like $resource.'.' and include file if such exists. Also if you did require_once i don't see the need to check for class_exists() etc.
I have never written autoloader but my instinct tells me that if you failed to find resource file then you should throw exception.
Altering array inside foreach is murky and its good to avoid such practices.

PS: Personally I haven't seen any php project/library utilizing hashtag comments. It might be my OCD but #...# draws all my focus so my suggestion is to use either: // for line comments and /*...*/ for block comments.

Update:
What I had in mind when writing point 2 is something like this:

    foreach(self::$resource_files as $file_key => $file)
    {
        // Check if resource matches file name
        if (strpos($file, $resource.'.') !== false ) {
            unset(self::$resource_files[$file_key]);

            require_once $file;
        }
    }


Simple Counter Program It is my first program in Javascript. Is there any lack or a thing to be improved? It simply increases, decreases, or reset the counter either by buttons or arrow keys.
HTML,

<button class="btn" id="incr">Increase</button>
<button class="btn" id="reset">Reset</button>
<button class="btn" id="decr">Decrease</button>

JS,
let number = 0

const getNumber = document.getElementById(`number`)
const incrButton = document.getElementById(`incr`)
const decrButton = document.getElementById(`decr`)
const resetButton = document.getElementById(`reset`)

getNumber.textContent = number

const incrFunc = function () {
  ++number
  getNumber.textContent = number
  if (number > 0) {
    getNumber.style.color = `green`
  }
}

const resetFunc = function () {
  number = 0
  getNumber.textContent = number
  getNumber.style.color = `gray`
}

const decrFunc = function () {
  --number
  getNumber.textContent = number
  if (number < 0) {
    getNumber.style.color = `red`
  }
}

incrButton.addEventListener(`keyup`, function (e) {
  e.stopPropagation()
  //console.log(e.target === document.body)
  if (e.code === `ArrowUp`) {
    incrFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  //console.log(e.target === document.body)
  if (e.code === `ArrowUp`) {
    incrFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

resetButton.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowDown`) {
    decrFunc()
  }
})

decr.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowDown`) {
    decrFunc()
  }
})

incrButton.addEventListener(`click`, incrFunc)

resetButton.addEventListener(`click`, resetFunc)

decrButton.addEventListener(`click`, decrFunc)

My first advice would be to reformat your code with a linter.  I won't go into this in more detail because I assume you're mostly after implementation advice, but as it is it's quite jarring to read, similar to text written in your own language but with foreign capitalization and punctuation.
Some of your functions could use destructuring to make their bodies more terse.
For instance, this:
document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

Could be rewritten as this, since you're only using the code property of the argument:
document.addEventListener(`keyup`, function ({ code }) {
  if (code === `ArrowRight` || code === `ArrowLeft`) {
    resetFunc()
  }
})

A more controversial bit of advice might be to write this at the top of your script:
const $ = document.querySelector.bind(document);

This allows you to replace this:
const getNumber = document.getElementById(`number`)
const incrButton = document.getElementById(`incr`)
const decrButton = document.getElementById(`decr`)
const resetButton = document.getElementById(`reset`)

With something much less verbose:
const getNumber = $(`#number`)
const incrButton = $(`#incr`)
const decrButton = $(`#decr`)
const resetButton = $(`#reset`)


Structuring code to do URL routing for Node.js What would be the best (performance, memory wise) to achieve a clean way to create a class or some way to properly structure the code, that shares 2 variables (req, res) which have a decent object size.

And yes, for those who use Node.js it are the req and res variables, but it is irrelevant.

This is what I tried so far:

function Client(req, res){
  var self = this;

  self.req = req;
  self.res = res;

  route.call(self);
  handleRoute.call(self);
  handleRequestData.call(self);
}

function route(){
  var self = this;

  self.req.route = // ...
}

function handleAuth(){
  var self = this;

  self.req.auth = // ...
}

function handleRequestData(){
  var self = this;

  self.req.data = // ...
}

I am wondering if it could be improved, since I mentioned req and res are pretty decent objects with methods, properties. Since .call(self) you pass through the instance, would it be most effective way?

Also I don't like using "var self = this;" all the time, it is useless.

And btw, I don't want to use Coffeescript.

Solution 1. (fails because of the massive passing through req and res)

Lets assume we got to files 1. client.js

var Client = {
  route: function(req, res){},
  auth: function(req, res, callback){ callback(req, res); },
  getData: function(req, res, callback){ callback(req, res); } // async
}

file 2. main.js

var client = require('./client.js');

client.route(req, res); // fine because this is syncronous.

// This is where the problem is, massive passing through req and res parameters.
client.auth(req, res, function(req, res){
  client.getData(req, res, function(req, res){
    // Do something...
  });
});

Are you intending to reuse the route, handleAuth, and handleRequestData functions? They could be "private" to the Client constructor function:

function Client(req, res) {
    function route() {
        //...
    }

    function handleAuth() {
        //...
    }

    function handleRequestData() {
        //...
    }

    route();
    handleRoute();
    handleRequestData();
}

See that I didn't set req and res as this members. I don't know if this is mandatory in your case. Also, this is just the beginning of what could be done; as stated in the comments to this answer, every time a new Client is created, new instances of the private functions are created as well, wasting resources.

A more sophisticated approach could define Client using a self-invoking function expression:

var Client = (function() {
    function route(req, res) {
        //...
    }

    function handleAuth(req, res) {
        //...
    }

    function handleRequestData(req, res) {
        //...
    }

    // This is the constructor function:
    return function(req, res) {
        route(req, res);
        handleRoute(req, res);
        handleRequestData(req, res);
    }
})();

Here, Client is defined as the product of the function expression enclosed in the outermost parenthesis. The function expression returns the constructor function, which has access to the closure functions route, handleRoute, and handleRequestData. Note that each of these functions is defined with req and res parameters. This way, they behave like static private functions, and you can code and use them regardless of what's referenced by this.

About self = this: it's known that JavaScript may be very confusing about the this keyword. Some information here. So, sometimes it's convenient to assign this to a closure variable named like self or me. It should not be done blindly though... as anything after all.


Polling a email using async / await I've created a create a console app that would:

Call a method to check an email account (I've done this step)
Convert the attachment to pdf (I've done this step)
Then once the conversion is complete wait 30 seconds
Repeat the previous 3 steps continuously

I've done Step 1 and 2 int the ProcessMailMessages() method.
The following code works but I want to know if I am on the right track or is there a better way to poll a email client?

    private static int secondsToWait = 30 * 1000;

    static void Main(string[] args)
    {
        bool run = true;
        do
        {
            try
            {
                Task theTask = ProcessEmailTaskAsync();
                theTask.Wait();
            }
            catch (Exception e)
            {
                Debug.WriteLine("
Error in Client

Exception

" + e.Message + "

" + e.StackTrace + "
 ");
            }
            GC.Collect();

        } while (run);

    }

    static async Task ProcessEmailTaskAsync()
    {
        var result = await EmailTaskAsync();
    }

    static async Task<int> EmailTaskAsync()
    {
        await ProcessMailMessages();
        await Task.Delay(secondsToWait);
        return 1;
    }

    static async Task ProcessMailMessages()
    {
        ...............................................................................
    }

I would rather do smth. like that (I added CancellationToken in case you would want to be able to cancel email parsing while it's in progress):

private static TimeSpan timeToWait = TimeSpan.FromSeconds(30);

//somewhere in your code:
_cancellationTokenSource = new CancellationTokenSource();

private static void Main(string[] args)
{
    CancellationToken token = _cancellationTokenSource.Token;
    do
    {
        try
        {
            ProcessEmailsAndWaitAsync(token).Wait();
        }
        catch (Exception e)
        {
            Debug.WriteLine("
Error in Client

Exception

" + e.Message + "

" + e.StackTrace + "
 ");
        }
    } while (!token.IsCancellationRequested);
}

private static async Task ProcessEmailsAndWaitAsync(CancellationToken token)
{
    await ProcessMailMessages(token);
    await Task.Delay(timeToWait, token);
}

private static async Task ProcessMailMessages(CancellationToken token)
{
    //...............................................................................
}


Function that calls a function with the terminal temporarily set to raw mode I'm working through Build Your Own Text Editor, a walkthrough of the code to an extremely small text editor for a posix terminal in C, but I'm attempting to follow along in-the-spirit with OCaml.

I've put together a function that wraps an parameter function with termcap stuff to get the terminal into raw mode. This code works, but it feels ugly to me, specifically the let res stuff near the end. I feel like there should be a way to either get the second Unix.tcsetattr into a finally like block, or otherwise "wrap" the call to f ().

How could I make this code more idiomatic? I'm quite new to OCaml, so any suggestions for restructuring this code would be useful.

module TermIO = struct
  let in_raw_mode (f: unit -> _) =
    let termio  = Unix.tcgetattr Unix.stdin in
    let termio' = { termio with
                    Unix.c_echo   = false ;
                    Unix.c_icanon = false
                  } in
    Unix.tcsetattr Unix.stdin Unix.TCSAFLUSH termio' ;
    let res = f () in
    Unix.tcsetattr Unix.stdin Unix.TCSAFLUSH termio  ;
    res
end

The sad fact is that your code should be even uglier. If f() throws an exception, you won't restore the resources correctly.

Idiomatic OCaml recommends that a recurring pattern can be described with a function. So you would define a function that takes:

a resource ('a)
a function to execute with the resource ('a -> 'b)
and a function to free the resource ('a -> unit)

The signature becomes:

val bracket : 'a -> ('a -> 'b) -> ('a -> unit) -> 'b

The bracket function would do the ugly let res = f() thing, but also wrap it with a try-catch.

UPDATE: The next version of OCaml (v4.08.0) looks like it will have a finally-type function added to the standard library: https://github.com/ocaml/ocaml/pull/1855


Manipulate urls with regular expressions instead of the slip approach Basically I have an url which looks like this structurally speaking:

http://www.my-site.com/topic1/topic2/topic3/topic4/topic5

and I want to do 2 things to it:

1. check the structure and validate it
2. replace both topic3 and topic4 with a new parameters called topic6

This is my solution for now but I am interested to build something more optimize.

if ((url.match(/\//g) || []).length === 7) {
    const arr = url.split('/');
    const topic5 = arr.pop();
    arr.pop();
    arr.pop();
    arr.push('topic6', topic5);
    return arr.join('/');
}

So as you can see, for the fact that I am not very good at regular expressions I have used another approach which doesn't look that good. I basically check for the number of / in the string, if tey are 7 it means that the structure of the url is good and that on this type of url I should apply the next steps. After that I grab the last param of the url and also remove it and next after that I remove the last 2 parameters and add the new one instead along with the topic5.

In case you you have a better approach, please let me know. I think that it can be done by writing less number of lines of code but as I said, I am not very familiar with regular expressions.

You seek to:

validate the url as coming from your site domain, then
replace the 3rd and 4th directories/topics with a single/new directory/topic.

This is a simple matter of preserving the substrings that you want to keep as "capture groups" and writing the new replace substring between the two capture groups.

In my demo, the new topic will be FOO.  I'm using negated character classes to match the directory names -- this is relatively "loose" validation.  If you require a tighter validation rule, see the 2nd snippet.  Click on Run code snippet below to see the resultant output.

const regex = /^((?:https?:\/\/)(?:w{3}\.)?my-site\.com\/(?:[^/]+\/){2})[^/]+\/[^/]+(\/[^/]+)$/;
      url = 'http://www.my-site.com/topic1/Topic2/topic3/topic4/слизень',
      replacement = '$1FOO$2';

document.write(url.replace(regex, replacement));

To tighten the validation beyond non-slashes as directory substrings AND extend the characters classes to include multibyte letters, you will need to manually expand the character classes to include the letters that you expect to qualify.  Please have a read of Regular expression with the cyrillic alphabet.

const regex = /^((?:https?:\/\/)(?:w{3}\.)?my-site\.com\/(?:[À-žа-яa-z\d-]+\/){2})[À-žа-яa-z\d-]+\/[À-žа-яa-z\d-]+(\/[À-žа-яa-z\d-]+)$/i;
      url = 'http://www.my-site.com/Schnecke/to-pic2/topic3/naaktslak/слизень',
      replacement = '$1FOO$2';

document.write(url.replace(regex, replacement));


FindTwoSums using Tuple I am writing a function that, given a list and a target sum, returns zero-based indices of any two distinct elements whose sum is equal to the target sum. If there are no such elements, the function should return null.

For example,

 FindTwoSum(new List<int>() { 1, 3, 5, 7, 9 }, 12)

should return any of the following tuples of indices:

1, 4 (3 + 9 = 12)
2, 3 (5 + 7 = 12)
3, 2 (7 + 5 = 12)
4, 1 (9 + 3 = 12)

Note: I made my code return the first sum equal to 12.

So far, I came up with the following:

using System;
using System.Collections.Generic;

class TwoSum
{
    public static Tuple<int, int> FindTwoSum(IList<int> list, int sum)
    {
       int first_i =0, global_sum=0, second_i=1;

        if(list.Count == 0 || sum == 0 ){
            return null;
        }

        for(int i = first_i ; i< list.Count; i++)
        {
            for(int j=second_i; j<list.Count; j++)
            {
                        if(i != list.Count)
                            {
                                global_sum += list[i] + list[j];

                                if(global_sum == sum)
                                    {
                                         var tuple1 = Tuple.Create(i,j);
                                      return tuple1;
                                    }
                                global_sum = 0;
                            }
                        else
                            {
                                break;
                            }

            }
        }
        return null;

    }

    public static void Main(string[] args)
    {
        Tuple<int, int> indices = FindTwoSum(new List<int>() { 1, 3, 5, 7, 9 }, 12);
        Console.WriteLine(indices.Item1 + " " + indices.Item2);
    }
}

Everything was working fine and I scored 75%. But in terms of performance, my code did badly as I got a review saying:

  performance test- The code too long to calculate an answer. Analyse how your code behaves when the input increases in size and see if you can make it work faster

Formatting

The indenting and placement of braces is inconsistent in the program,
and it hurts readability. Instead of this:

    for(int i = first_i ; i< list.Count; i++)
    {
        for(int j=second_i; j<list.Count; j++)
        {
                    if(i != list.Count)
                        {
                            global_sum += list[i] + list[j];

                            if(global_sum == sum)
                                {
                                     var tuple1 = Tuple.Create(i,j);
                                  return tuple1;
                                }
                            global_sum = 0;
                        }
                    else
                        {
                            break;
                        }

        }
    }

This would be have been much better:

    for (int i = first_i; i < list.Count; i++)
    {
        for (int j = second_i; j < list.Count; j++)
        {
            if (i != list.Count)
            {
                global_sum += list[i] + list[j];

                if (global_sum == sum)
                {
                    var tuple1 = Tuple.Create(i, j);
                    return tuple1;
                }

                global_sum = 0;
            }
            else
            {
                break;
            }
        }
    }

Strange condition in loop body

The conditional in the body of the nested loop is pointless,
as it can never be true. You can simply remove it:

for (int i = first_i; i < list.Count; i++)
{
    for (int j = second_i; j < list.Count; j++)
    {
        global_sum += list[i] + list[j];

        if (global_sum == sum)
        {
            var tuple1 = Tuple.Create(i, j);
            return tuple1;
        }

        global_sum = 0;
    }
}

Unnecessary variables

The name global_sum is not great. There's nothing "global" about that sum. It's just sum. And you don't need it a variable for it anyway.
You could change this code:

        global_sum += list[i] + list[j];

        if (global_sum == sum)
        {
            var tuple1 = Tuple.Create(i, j);
            return tuple1;
        }

        global_sum = 0;

Like this:

        var global_sum = list[i] + list[j];

        if (global_sum == sum)
        {
            var tuple1 = Tuple.Create(i, j);
            return tuple1;
        }

At this point, notice that you don't really need this variable at all,
you could write the expression directly in the if statement.
The tuple1 variable is also pointless.
The code can be reduced to:

        if (list[i] + list[j] == sum)
        {
            return Tuple.Create(i, j);
        }

first_i and second_i are also rather useless,
and would have been to use their values directly inside the loop statements.

Also keep in mind that declaring variables at the top of their scope,
when they are not used until much later,
is not a recommended practice.
It's best to declare a variable right before you need it.

Correctness

As @RiskyMartin hinted, given an input list 1, 6, 9, the output will be incorrectly (6, 6). In other words, you need to make sure that i and j indexes are not equal.

Performance

Of course, none of the above help with the performance.
The algorithm is \$O(n^2)\$: you try all permutation of pairs.

To make this faster, you can sort the input list, and then form pairs using two pointers, one going from the start and one going from the end.
Move the pointers closer when the sum of the pointed values is above the target sum.
The performance of this algorithm will be \$O(n\log(n))\$:
the sorting step is \$O(n\log(n))\$,
the search step with two pointers is \$O(n)\$.

For example, given 1, 2, 5, 7, 9, the first pointer starts at 1, the second at 9. 1 + 9 is less than 12, so you advance the first pointer. 2 + 9 is less than 12, so you advance the first pointer. 5 + 9 is greater than 12, so you move the second pointer. 5 + 7 is a match, you return it.

Just like in the original program, make sure the two pointers don't point to the same element, because that could produce an invalid match, potentially using the same element twice to make the sum.


Creating filterable table on web page I am wondering if there is a better way to set up a filterable table on a web page using Python/Django/JavaScript or potentially a different stack.

The current setup is as follows.

1) The Python backend pulls datetime.date and datetime.time objects from the database.

2) These are converted into string via the backend as I want full control over how they are displayed (and on the web page they become string anyway, as far as I can tell).

3) The strings are passed to the view and displayed in an HTML <table>.

4) The filtering is done via a JavaScript function. The JavaScript converts the strings back to datetime objects to be able filter on them.

Issue 1: Is there a way to avoid the back and forth converting between datetime and string?

Below is my JavaScript function. The below only works for datetime.time. I'll have to write something similar for datetime.date.

Issue 2: I would like the code to stay DRY, so any ideas on how to make the function be able to handle both datetime.date and datetime.time objects would also be very welcome.

function filter(id, column_id) {
    var table, tr, filter_time;
    table = document.getElementById("myTable");
    tr = table.getElementsByTagName("tr");
    filter_time = document.getElementById(id).value;

    filter_time = filter_time.split(':');
    filter_time = new Date(0, 0, 0, filter_time[0], filter_time[1]);

    for (i = 0; i < tr.length; i++) {
        td = tr[i].getElementsByTagName("td")[column_id];
        if (td) {
            td = td.innerHTML.split(':');
            var time_to_filter = new Date(0, 0, 0, td[0], td[1]);
            if (time_to_filter < filter_time) {
                tr[i].style.display = "none";;
            } else {
                tr[i].style.display = "";
            }
        }
    }
}

Issue 1: Is there a way to avoid the back and forth converting between datetime and string?

There is, but it's not gonna be easy.
You could inject data into the template as JavaScript code.
That way the page will have the dates in native format.
To make that work,
you would have to stop injecting the data as HTML code,
and use JavaScript to generate it based on the injected dates in native format.

But unless you have large amounts of data,
you might not need this.
The conversions back and forth between datetime and string might not be significant.

  Issue 2: I would like the code to stay DRY, so any ideas on how to make the function be able to handle both datetime.date and datetime.time objects would also be very welcome.

Easy.
You could inject the filtering functionality.
In particular,
the most important part of your implementation is extracting a value to compare from the data.
This kind of function is often called key.
You could define such function to convert the string to time:

var time_key = function(s) {
    var parts = s.split(':');
    return new Date(0, 0, 0, parts[0], parts[1]);
}

Then you could inject this as the key parameter to filter:

function filter(id, column_id, key) {
    var i, value;
    var table = document.getElementById("myTable");
    var tr = table.getElementsByTagName("tr");
    var ref = key(document.getElementById(id).value);

    for (i = 0; i < tr.length; i++) {
        td = tr[i].getElementsByTagName("td")[column_id];
        if (td) {
            value = key(td.innerHTML.split(':'));
            tr[i].style.display = value < ref ? "none" : "";
        }
    }
}


Computing huge Fibonacci numbers in Java I have this small Java program for computing particurly large Fibonacci numbers (10000th Fibonacci number is computed in about 220 milliseconds). The key idea here is to use lists of digits, which is exactly what is done in java.math.BigInteger, yet I wanted to implement a simple digit list addition on my own.

import java.util.ArrayList;
import java.util.List;

public class Main {

    public static String fibonacci(int n) {
        if (n < 0) {
            throw new IllegalArgumentException("n = " + n);
        }

        switch (n) {
            case 0:
                return "0";

            case 1:
                return "1";
        }

        List<Character> listA = new ArrayList<>();
        List<Character> listB = new ArrayList<>();

        listA.add('0');
        listB.add('1');

        boolean fromAtoB = true;

        for (int i = 0; i < n; ++i, fromAtoB = !fromAtoB) {
            if (fromAtoB) {
                add(listA, listB);
            } else {
                add(listB, listA);
            }
        }

        StringBuilder sb;

        if (fromAtoB) {
            // It looks like the result is in the listB. However, as the boolean
            // fromAtoB was flipped after the last add(list1, list2), the result
            // is actually in list A.
            sb = new StringBuilder(listA.size());

            // For efficiency, we grow the digit string such that the least
            // significant digits are in the lowest array positions. That way
            // we make sure that adding a new (most significant digit) always
            // runs in amortized constant time.
            for (int i = listA.size() - 1; i >= 0; --i) {
                sb.append(listA.get(i));
            }
        } else {
            // Result is in listB.
            sb = new StringBuilder(listB.size());

            for (int i = listB.size() - 1; i >= 0; --i) {
                sb.append(listB.get(i));
            }
        }

        return sb.toString();
    }

    // Adds the number represented by digit list 'from' to the number
    // represented by digit list 'to'.
    private static void add(List<Character> from, List<Character> to) {
        // The carry flag.
        boolean carry = false;

        if (to.size() < from.size()) {
            // Extend the target digit list so that it can accommodate most of
            // the digits of the sum of the two Fibonacci numbers.
            to.add('0');
        }

        for (int i = 0; i < from.size(); ++i) {
            int digit = charToInt(from.get(i)) +
                        charToInt(to.get(i)) +
                        (carry ? 1 : 0);

            if (digit > 9) {
                digit -= 10;
                carry = true;
            } else {
                carry = false;
            }

            to.set(i, intToChar(digit));
        }

        if (carry) {
            to.add('1');
        }
    }

    private static int charToInt(char c) {
        return (int)(c - '0');
    }

    private static char intToChar(int i) {
        return (char)(i + '0');
    }

    public static void main(String[] args) {
        Integer i = -1;

        if (args.length > 0) {
            try {
                i = Integer.parseInt(args[0]);
            } catch (NumberFormatException ex) {

            }
        }

        long startTime = System.currentTimeMillis();
        String number = fibonacci(i >= 0 ? i : 10);
        long endTime = System.currentTimeMillis();

        System.out.println(number);
        System.out.println((endTime - startTime) + " milliseconds.");
    }
}

So what do you think?

StringBuilder sb;

    if (fromAtoB) {
        // It looks like the result is in the listB. However, as the boolean
        // fromAtoB was flipped after the last add(list1, list2), the result
        // is actually in list A.
        sb = new StringBuilder(listA.size());

        // For efficiency, we grow the digit string such that the least
        // significant digits are in the lowest array positions. That way
        // we make sure that adding a new (most significant digit) always
        // runs in amortized constant time.
        for (int i = listA.size() - 1; i >= 0; --i) {
            sb.append(listA.get(i));
        }
    } else {
        // Result is in listB.
        sb = new StringBuilder(listB.size());

        for (int i = listB.size() - 1; i >= 0; --i) {
            sb.append(listB.get(i));
        }
    }

    return sb.toString();

Lots of fancy comments. Let's take em out so I can look at the actual code.

    StringBuilder sb;

    if (fromAtoB) {
        sb = new StringBuilder(listA.size());

        for (int i = listA.size() - 1; i >= 0; --i) {
            sb.append(listA.get(i));
        }
    } else {
        sb = new StringBuilder(listB.size());

        for (int i = listB.size() - 1; i >= 0; --i) {
            sb.append(listB.get(i));
        }
    }

    return sb.toString();

Seems all you do here is getting a string... from either listA or listB. I'd recommend putting this in a separate function...

public static String convertDigitListToString(List<Character> list){
    StringBuilder sb = new StringBuilder(list.size());

    for (int i = list.size() - 1; i >= 0; --i) {
        sb.append(list.get(i));
    }
    return sb.toString();
}

And then you can simplify your code a bit.

if (fromAtoB) {
    return convertDigitListToString(listA);
} else {
    return convertDigitListToString(listB);
}

Maybe even a ternary? I dunno about that, though. It's shorter, but a bit more harder to read.

return convertDigitListToString(fromAtoB ? listA : listB);

The real reason it's hard to read is because fromAtoB is a pretty bad description... in this case, you're looking for something like lastListUsed... but that doesn't really fit the description either.

    if (n < 0) {
        throw new IllegalArgumentException("n = " + n);
    }

You might want to give a more descriptive message, like "n must be positive", or "expected positive n, got ".


Stream file to OpenCSV with the least amount of I/O possible I have the following code snippet where I'm picking up a file from an FTP server and then streaming it to OpenCSV. I have very little experience with streams, so I'd like to have someone review this code for efficiency. I use this on the cloud, so I/O and memory efficiency is important. Generally my CSV files consist of a few hundred thousand records each.

public InputStream getData(ImportTask importTask) throws IOException {
    //FTP Connection code

    try {
                //enter passive mode
                ftp.enterLocalPassiveMode();

                if (!ftp.setFileType(FTP.BINARY_FILE_TYPE)) {
                    System.out.println("Setting binary file type failed.");
                }

                ByteArrayOutputStream output = new ByteArrayOutputStream();

                String file = importTask.isZipfile() ? importTask.getZipFilename() : importTask.getFilename();

                ftp.retrieveFile(file, output);
                InputStream inputStream = new ByteArrayInputStream(output.toByteArray());

                if (importTask.isZipfile()) {
                    inputStream = importUtils.getZipData(new ZipInputStream(inputStream), importTask.getFilename());
                }

                ftp.logout();
                ftp.disconnect();

                return inputStream;
            } catch (FileNotFoundException ex) {
                throw new Exception(ex);
            }
}

OpenCSV to POJO

InputStream is;

try {
        is = dataSource.getData(importTask);
    } catch (IOException ex) {
        logger.error(ex.getMessage());
    {
}

final HeaderColumnNameTranslateMappingStrategy<MyObject> strategy = new HeaderColumnNameTranslateMappingStrategy<>();
        strategy.setType(MyObject.class);
        strategy.setColumnMapping(columnMappings);

final CsvToBean csv = new CsvToBean();
List list = csv.parse(strategy, new InputStreamReader(is));

for (Object object : list) {
    MyObject myObject = (MyObject) object;
    //Do something
}

Switch to uniVocity-parsers as it is twice as fast as OpenCSV and the input is read in parallel.

    final BeanListProcessor<MyObject > clientProcessor = new BeanListProcessor<MyObject>(MyObject .class);

    CsvParserSettings settings = new CsvParserSettings();
    settings.getFormat().setLineSeparator("\n");
    settings.setRowProcessor(clientProcessor);

    settings.setReadInputOnSeparateThread(true); // this enabled by default

    CsvParser parser = new CsvParser(settings);
    parser.parse(new InputStreamReader(new ZipInputStream(inputStream)));

    List<MyObject> rows = clientProcessor.getBeans();

If you are not using all columns of your CSV input, you can select the rows you want as well to make the process even faster:

    settings.selectIndexes(4, 6, 3);
    settings.selectFields("Field A", "B", "and C");

Disclosure: I am the author of this library. It's open-source and free (Apache V2.0 license).


Euclidian distance - optimization and casting I'm trying to optimize a simple Euclidian distance function in C. It's for a DLL, and calculates distances from one point to many. I have:

Version 1:

int CalcDistance (int y1, int x1, int *y2, int *x2, int nb , int *distances)
{
    double dx,dy = 0;

    for (int i = 0;i<nb;i++)
    {
        dx = x2[i] - x1;
        dy = y2[i] - y1;

        distances[i]=(int)sqrt((dx*dx)+(dy*dy));
    }

    return nb;
}

Version 2:

int CalcDistance (int y1, int x1, int *y2, int *x2, int nb , int *distance)
{
    int dx,dy =0;

    for (int i = 0;i<nb;i++)
    {
        dx = x2[i] - x1;
        dy = y2[i] - y1;

        distances[i]=(int)sqrt((double)(dx*dx)+(dy*dy));
    }

    return nb;
}

Essentially, I don't need extra high precision, so that's why I'm cutting down to ints, not doubles. For my specific use case, the final distance will not be larger than what an int (Int32) can hold.

I was taken aback by how slow version 2 was. I figured that in Version 1 calculating dx and dy would implicitly cast to double twice, whereas in version 2 I'm just explicitly casting once. What's happening? And ultimately, what could be even faster?

(And I tried _hypot, which was oddly the slowest of all...)

dstances[i]=(int)sqrt((dx*dx)+(dy*dy));

This only converts to double once. it will do all the math integer style and and then convert it to a double just before calling sqrt.

    distances[i]=(int)sqrt((double)(dx*dx)+(dy*dy));

This is actually the same as:

    distances[i]=(int)sqrt(  ((double)(dx*dx))   + (dy*dy));

not:

    distances[i]=(int)sqrt((double)(  (dx*dx)) + (dy*dy)  );

As a result it will convert dx*dx to double, then convert dy*dy to double then add them and the pass it off to sqrt.

In modern computers floating point numbers are really fast, in some cases even faster then integers. As a result, you well be much better off to use double throughout rather then converting from ints to doubles and back again. The conversion between int and double is expensive so avoiding it may actually be your best bet.

A common trick used in algorithms which need to work with distances is to rework the algorithm so that you can use the square of the distance. That way you can avoid the expensive sqrt call. This works because x*x < y*y iff x < y. Be careful though, whether or not you can do this depends on what you are doing with that distance.


Sanity check on grouping query I have a table containing URLs as they should appear in English and equivalent translations of them into other languages.  Originally, I was told that there would always be an English translation, so when listing them in the application that updates them I just searched for the ones where iso = 'GB'.  However, it turns out that it's entirely possible for there not to be an English translation.

I needed to update my query for generating the list displayed in the application so that it would select the English translation in preference to any other translations that might exist, but if there is no English translation, then use a single example of which other translations there are present (doesn't matter which, just as long as there's a single entry presented to the user).

The query I came up with is as follows.  I'd appreciate it if you could give it a sanity check so I can be confident that it's returning correct results. (It appears to do so with the current data set, but it's currently quite a small set).

SELECT `t`.*
FROM (
    SELECT `translations_urls`.*
    FROM `translations_urls`
    ORDER BY iso = 'GB' DESC,
    `order` ASC,
    `url_gb` ASC
) AS `t`
GROUP BY `url_gb`

The table schems is as follows:

CREATE TABLE `translations_urls` (
    `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
    `order` INT(10) NOT NULL COMMENT 'Sort order',
    `iso` CHAR(2) NOT NULL COMMENT 'ISO country code',
    `url_gb` VARCHAR(127) NOT NULL COMMENT 'Original URL',
    `url_trans` VARCHAR(127) NOT NULL COMMENT 'Translated URL',
    `controller` VARCHAR(127) NOT NULL COMMENT 'Controller the URL refers to',
    `action` VARCHAR(127) NOT NULL COMMENT 'Action the URL refers to',
    PRIMARY KEY (`id`),
    UNIQUE INDEX `iso_url_gb` (`iso`, `url_gb`),
    INDEX `url_trans` (`url_trans`),
    CONSTRAINT `fk_translations_urls_translations_countries_iso` FOREIGN KEY (`iso`) REFERENCES `translations_countries` (`iso`)
)
COMMENT='URL translations'
COLLATE='utf8_general_ci'
ENGINE=InnoDB
AUTO_INCREMENT=303;

Your query isn't right, and in my opinion, your schema doesn't make sense either.

What is the purpose of the sub-select in your query?  Is your query not equivalent to

SELECT *
    FROM `translations_urls` AS `t`
    GROUP BY `url_gb`
    ORDER BY iso = 'GB' DESC, `order` ASC, `url_gb` ASC;

Furthermore, what do you intend to achieve with GROUP BY?  You aren't using any aggregate functions in your output columns.  In fact, with a sane database engine (e.g. PostgreSQL), such a query would generate an error like

  ERROR:  column "t.id" must appear in the GROUP BY clause or be used in an aggregate function

Your schema seems awkward to me, for several reasons:

The url_gb column implies that GB is the original language, which, as you stated, is no longer a valid assumption.
The use of url_gb to identify a translation's original results in a normalization problem.  If the URL of a UK-English document changes, you have to update the url_gb of all of its translations.

Furthermore, translations should have language codes, not country codes.  (An exception would be if you were "translating" legal contracts, in which case you want a different document per country.)

One solution is to make a self-referential table that could form a tree of documents:

CREATE TABLE `documents`
( `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT
, `order` INT(10) NOT NULL COMMENT 'Sort order'
, `iso` VARCHAR(6) NOT NULL COMMENT 'ISO language code'
, `url` VARCHAR(127) NOT NULL COMMENT 'URL'
, `controller` VARCHAR(127) NOT NULL COMMENT 'Controller the URL refers to'
, `action` VARCHAR(127) NOT NULL COMMENT 'Action the URL refers to'
, `translated_from` INT(10) UNSIGNED COMMENT 'id of document from which this was translated'
, PRIMARY KEY (`id`)
, UNIQUE INDEX `iso_url` (`iso`, `url`)
, FOREIGN KEY (`iso`) REFERENCES `translations_languages` (`iso`)
, FOREIGN KEY (`translated_from`) REFERENCES `documents` (`id`)
)
ENGINE=InnoDB;

Another approach, which I prefer, is to distinguish between documents and their translations.  The advantage of this is that you can nominate one of the translations as the canonical original, to prevent recursion, which is harder to deal with in SQL.

CREATE TABLE `documents`
( `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT
, `order` INT(10) NOT NULL COMMENT 'Sort order'
, `canonical` INT(10) UNSIGNED COMMENT 'Canonical (original) translation'
, `controller` VARCHAR(127) NOT NULL COMMENT 'Controller the URL refers to'
, `action` VARCHAR(127) NOT NULL COMMENT 'Action the URL refers to'
, PRIMARY KEY (`id`)
)
ENGINE=InnoDB;

CREATE TABLE `translations`
( `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT
, `document_id` INT(10) UNSIGNED NOT NULL
, `iso` VARCHAR(6) NOT NULL COMMENT 'ISO language code'
, `url` VARCHAR(127) NOT NULL COMMENT 'URL'
, PRIMARY KEY (`id`)
, UNIQUE INDEX `document_id_iso` (`document_id`, `iso`)
, UNIQUE INDEX `url` (`url`)
, FOREIGN KEY (`iso`) REFERENCES `translations_languages` (`iso`)
)
ENGINE=InnoDB;

ALTER TABLE `documents`
    ADD FOREIGN KEY (`canonical`) REFERENCES `translations` (`id`);

An annoyance with this schema is the mutual foreign key relationship.  To add a document, you would first have to insert a document with NULL as the canonical translation, then insert the first translation, then update the document with the id of the first translation.

With the latter schema, a query to preferentially return English versions, falling back to a canonical text in another language, would be:

SELECT *
    FROM `documents` AS `d`
        INNER JOIN `translations` AS `t`
            ON `t`.`document_id` = `d`.`id`
    WHERE
        `iso` = 'en' OR
        (`iso` <> 'en' AND `t`.`id` = `d`.`canonical`);


Create list of objects from an object where the key is properCase using Lodash I'm trying to create a filtered list of objects where each object in the list has both a label and value property.

First I use .pick to select the key/value pairs I want from the initial object, then I iterate over the 'picked' properties and push the key into an object property called label, and push the value of the property into a property called value. I then return the empty array.

function formatToInput(obj, list) {
    var empty = []

    _.forIn(_.pick(obj, list), (val, key) => {
        empty.push({
            label: _.startCase(key),
            value: val
        });
    });

    return empty;
}

// use like so

var test = {blue: 10, green: 5, yellow: 3, pink: 1};
var filter = ['green', 'yellow'];

var results = formatToInput(test, filter)

console.log(results);

// [{label: Green, value: 5}, {label: Yellow, value: 3}]

I'm trying to avoid having to manually create an array and pushing objects into it, but I couldn't find a solution (in Lodash) that would allow me to do this.

The code looks like you're passing the processed data to autocomplete.

Naming

formatToInput: This gives no idea of what the function is doing.
empty: Is it really empty at the end of formatToInput?
obj, list: Confusing.
test, results: Can be named better.

Lodash

Do you really need a library for this functionality? The Same functionality can be done without Lodash or any other library. If Lodash is only used for this functionality, it can be removed.

Missing Semi-colons

Although, a small thing Automatic Semicolon Insertion can lead to unexpected behavior. The usage of the semi-colon is not consistent. I'll recommend using JSLint/JSHint.

Alternative

I'll recommend the use of Array#map and String methods to make the first character uppercase.

const colorValueMap = {
  blue: 10,
  green: 5,
  yellow: 3,
  pink: 1
};
const colors = ['green', 'yellow'];

const result = colors.map(color => ({
  label: color[0].toUpperCase() + color.slice(1),
  value: colorValueMap[color]
}));
console.log(result);

Above solution assumes that the value from the filter array always exists in the object. If this is not the case, you can first filter the array and then use map on it.

colors.filter(color => colorValueMap.hasOwnProperty(color)).map(color => ({
    ...
});


LeetCode parentheses matching I came across this question from LeetCode OJ where I am given a string containing parentheses and I need to find out if the string contains valid matching parentheses or not.

  Given a string containing just the characters '(', ')', '{', '}', '[' and ']', determine if the input string is valid.

  The brackets must close in the correct order, "()" and "()[]{}" are all valid but "(]" and "([)]" are not.

This is the code I wrote (it got accepted and passed all the test cases.)

/** Java program to check if input string contains matching parameters
    or not.
 *  @author     :       Anish
 *  @date       :       01-11-2017
 */
 import java.util.Stack;

 class Solution
 {
    public boolean isValid(String str)
    {
        Stack<Character> stack  =   new Stack<Character>();

        char top    =   ' ';
        int len     =   str.length();

        for (int i = 0; i < len; i++)
        {
            char ch    =   str.charAt(i);

            if (ch == '(')
                stack.push(')');

            else if (ch == '[')
                stack.push(']');

            else if (ch == '{')
                stack.push('}');

            else if (!stack.empty())
            {
                top   =   stack.peek();

                if (top == ch)
                    stack.pop();
                else
                    stack.push(ch);
            }
            else if (stack.empty())
            {
                stack.push(ch);
            }
        }
        if (stack.empty())
        {
            return true;
        }
        return false;
    }
 }

ANALYSIS

Time Complexity : \$O(n)\$

Run Time : 9ms (on the leetcode OJ)

ISSUES

Even though my time is \$O(n)\$, yet my percentile is 70.4 which effectively means there are better and faster solutions out there. In that case, how do I optimize the above code to make it faster?
I have been using the Stack API provided by Java to solve the problem. Rather, if I were to redesign my own stack API and then use it to solve the problem, would I get faster runtime and better performance?
If the above point is true, then does it mean that in general, user designed APIs are faster than what is provided by the Java libraries? (assuming the user designed API is clean and bug free)

Usage of Stack

Don't use a Stack. This class is very old, it's thread safe (which you don't need) and achieves it by synchronizing the push and pop methods, which will have an impact in performance.

I would use ArrayDeque. By default it's initialized with size 16. To reduce number of re-allocations, allocate a larger size, perhaps ou can guess how many elements you'll need based on the size of the string. For example:

Queue<Character> queue = new ArrayDeque<>(str.size()/2);

Boxing of char

Note that each time you push/pop from the queue, you are boxing and unboxing char (to Character). For ultimate performance, create a char[] queue, but then you'll have to manage resizes and pointers to the top element yourself.

Fail fast

Consider that the input is {(])}. When finding ], your program can already know that the string is invalid and stop iterating. This will make the processing of invalid inputs potentially much faster.
So what about replace this:

else if (!stack.empty())
{
  top   =   stack.peek();
  if (top == ch)
    stack.pop();
  else
    stack.push(ch);
  }
  else if (stack.empty())
  {
    stack.push(ch);
  }
}
else if (stack.empty())
{
  stack.push(ch);
}

by:

else if (!stack.empty())
{
  top   =   stack.peek();
  if (top == ch)
    stack.pop();
  else
    return false; // quit
  }
}
else
{
    return false; //quit
}

I didn't test and I might be missing something, but it looks that as soon as you have an char not matching the expected closing parenthesis, you can  conclude that it's invalid.

My suggestion

This has all the previous points applied and runs much faster:

public boolean isValid(String str) {
char[] stack = new char[str.length()];
int p = -1;

for (int i = 0; i < str.length(); i++) {
  char ch = str.charAt(i);

  if (ch == '(') {
    stack[++p] = ')';
  } else if (ch == '[') {
    stack[++p] = ']';
  } else if (ch == '{') {
    stack[++p] = '}';
  } else if (p >= 0) {
    char top = stack[p];

    if (top == ch) {
      p--;
    } else {
      return false;
    }
  } else {
    return false;
  }
}

return p < 0;
}

EDIT: Benchmarks

I'm adding the results of benchmarks done with JMH with some different solutions to show how performance is impacted.

I used one valid input: ([[{{([[{{[[[[]]]]}}]])}}]]) and one invalid input: ([[{{([[{{[[[[]]]]}}]]})}}]]).

Parameters of the benchmark:

# Warmup: 5 iterations, 1 s each
# Measurement: 5 iterations, 1 s each
# Benchmark mode: Average time, time/op (nanoseconds)

Results:

Benchmark               (input)  Mode  Cnt    Score    Error  Units
Benchmark.finalSolution   valid  avgt    5   67.468 ±  1.411  ns/op
Benchmark.finalSolution invalid  avgt    5   53.832 ±  1.543  ns/op

Benchmark.toCharArray     valid  avgt    5  241.692 ± 11.705  ns/op
Benchmark.toCharArray   invalid  avgt    5  261.955 ±  6.192  ns/op

Benchmark.original        valid  avgt    5  245.888 ±  3.329  ns/op
Benchmark.original      invalid  avgt    5  265.483 ±  3.312  ns/op

Benchmark.useArrayDeque   valid  avgt    5  100.597 ±  1.621  ns/op
Benchmark.useArrayDeque invalid  avgt    5  102.695 ±  0.659  ns/op

Benchmark.useLinkedList   valid  avgt    5  180.679 ±  4.395  ns/op
Benchmark.useLinkedList invalid  avgt    5  190.591 ±  4.010  ns/op


Basic Imitation of C# Enumerable in VBA - or any other static class Since implementing static classes in VBA actually seems possible I came up with an idea to try to imitate C#' Enumerable class (well, to some extent...). This is just an idea but it should work with any sort of a class serving the purpose of a static one, ie. Math, Factory, etc.

quick note: the original Enumerable uses Linq which is not an option in VBA ( forget this ) therefore the functionality as of now will be very limited.

The point here is to be able to kind of replace the syntax of

Dim i as Long
for i = 1 to 100
    'do something...
next

with

Dim number as Variant
for each number in Enumerable.Range(1,100)
    'do something...
next

So you can iterate almost any Range of numbers "on-fly".

So far I have been able to come up with a very simple working version (but! -> try to see the potential) I would like to demonstrate here and ask for your opinion of possibly improving it and things to consider within the current implementation.

You need Notepad or another text editor and copy-paste the following

VERSION 1.0 CLASS
BEGIN
MultiUse = -1 'True
END
Attribute VB_Name = "Enumerable"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Attribute VB_Exposed = False

Private c As Collection

Public Function Range(param1, param2) as Collection
    Set c = new Collection
    Dim i as Long
    for i = param1 to param2
        c.add i
    next
    set Range = c
End Function

Public Property Get NewEnum() As IUnknown
Attribute NewEnum.VB_UserMemId = -4
    Set NewEnum = c.[_NewEnum]
End Property

Save this file to your desktop as Enumerable.cls (make sure it's not an Enumerable.cls.txt)

Start Excel and go right into the VBE (alt+F11)

Right click the VBA Project and select Import File and select the Enumerable.cls.

Right click the VBA Project again and Insert a Module (Module1)

Now, copy-paste the below code into the Module1

Sub Main()

    Dim number As Variant

    For Each number In Enumerable.Range(1, 100)
        Debug.Print number
    Next

End Sub

and run w/ F5. Numbers from 1 to 100 should be printed to the Immediate Window at this point.

Notice - You did not have to instantiate the Enumerable class and you are able to iterate via any range of numbers - as long as they don't cause an overflow error on the Long data type

Notice2 - The current very basic version has not been tested with negative numbers - would it be worth fully supporting it and possibly combining it with a possibility of reverse iterating the collection?

Please, share your opinions :)

For fun: replace the code in Module1 with

Sub Main()

    Dim number As Variant

    For Each number In Enumerable.Range(1, 100)
        Debug.Print FizzBuzz(number)
    Next

End Sub

Public Function FizzBuzz(ByVal value As Long) As String
    If (value Mod 15 = 0) Then
        FizzBuzz = "FizzBuzz"
        Exit Function
    End If
    If (value Mod 3 = 0) Then
        FizzBuzz = "Fizz"
        Exit Function
    End If
    If (value Mod 5 = 0) Then
        FizzBuzz = "Buzz"
        Exit Function
    End If
    FizzBuzz = value
End Function

For each requires that number be declared as a Variant, but For..To requires a number. I like that you explicitly declared Dim number As Variant. It makes it clear that that you intended on it and didn't simply forget to specify the type. However, you've not specified what type param1 and param2 should be. By declaring them as longs you can stop several lines of code from executing if some unwary dev passes a string or object into .Range(). Runtime error 13 "Datatype mismatch" gets thrown either way, but it happens sooner and there's a tool tip showing what datatype is to be expected.

The params also being passed ByRef. While there are no side effects on the parameters, it would be nice and in good faith to let anyone using this class know that there are not any side effects by passing them ByVal.

Public Function Range(ByVal param1 As Long, ByVal param2 As Long) As Collection
    Set c = New Collection
    Dim i As Long
    For i = param1 To param2
        c.add i
    Next
    Set Range = c
End Function

Another thing to note is that using this method will be exactly twice as slow as just using a For loop. This is because the code loops once to build the collection and then again to enumerate over the collection. When performance is absolutely critical you might want to skip the syntactic sugar.

I tested against negative longs and your code works just fine.

There are a number of missing features that I'd like to see, but I suspect you'll have some amount of difficulty getting them to "feel right". This has to do with VBA's lack of proper inheritance and method overloading. I've attempted to do something similar recently without much luck. You've already gotten farther than I did though, so I'll be keeping an eye on vba4All to see what you come up with. =)


Find the minimum value in a circular list of integers I was looking up some coding challenges and this looked like a fun one to solve. I tested it with the following values and it seems to work for those at least:

[ 5, 7, 8, 9, 11, 2, 3 ]
[ 5, 7, 1, 4]
[ 5, 7, 8, 1, 5]
[ 5, 1, 2, 3, 5]
[ 5, 5, 5]
[ 1 ]
[ 1, 2 ]
[ 2, 1 ]

My solution was to implement a binary search to try and find the smallest value, and if both ends of the sub-list have the same value then I will increment from the lowest value to try and work out if the lowest value lies in the first or second half of the sublist. I am not 100% confident that this part of the solution is bug free, so would appreciate any feedback on that.

I am mostly looking for feedback on my logic, as the coding standard doesn't matter to me too much in this case.

public static int findMinimum(List<Integer> input) {
  return findMinRecursive(input, 0, input.size() - 1);
}

public static int findMinRecursive(List<Integer> input, int lowestIndex, int highestIndex) {
  if (lowestIndex == highestIndex) {
    return input.get(lowestIndex);
  }
  boolean lastCheck = highestIndex - lowestIndex == 1;

  int offset = (highestIndex - lowestIndex) / 2;
  if (input.get(highestIndex) > input.get(lowestIndex)) {
    if (lastCheck) {
      return input.get(lowestIndex);
    }
    return findMinRecursive(input, lowestIndex, highestIndex / 2);
  } else if (input.get(highestIndex) < input.get(lowestIndex)) {
    if (lastCheck) {
      return input.get(highestIndex);
    }
    return findMinRecursive(input, lowestIndex + offset, highestIndex);
  } else {
    // If the numbers are equal we need to find out which direction has the minimum
    for (int i = lowestIndex; i < highestIndex; i++) {
      if (input.get(i) > input.get(lowestIndex)) {
        return findMinRecursive(input, lowestIndex + offset, highestIndex);
      } else if (input.get(i) < input.get(lowestIndex)) {
        return input.get(i);
      }
    }
    return input.get(lowestIndex);
  }
}

You can fix the bug, that @Misha pointed out, by changing

    if (input.get(highestIndex) > input.get(lowestIndex)) {
      if (lastCheck) {
        return input.get(lowestIndex);
      }
      return Main.findMinRecursive(input, lowestIndex, highestIndex / 2);
    }

to

    if (input.get(highestIndex) > input.get(lowestIndex)) {
      return input.get(lowestIndex);
    }

because, if the value to the right is greater than the value to the left in the subsection you are searching through, then that subsection is already perfectly sorted in ascending order.

I also think you can achieve a very tiny performance improvement if you don't use so many calls to get(). You can store the values in properties(a.k.a. variables) first, and then use those variables/properties in place of the get(), but it's probably an almost imperceptible performance boost


Get Nth item from end of list I am starting out in Haskell and thought I would try to make a function that got the index (nth) from the end of the list.

So getNthFromEnd 5 [1..10] would equal 5

The code I have so far is

getNthFromEnd :: Int -> [a] -> a
getNthFromEnd _ [] = error "Empty list given"
getNthFromEnd nth list
    | listLength < nth = error $ "List length must be greater than " ++ show nth
    | listLength == nth = head list
    | listLength >= nth = list !! (listLength - nth - 1)
    where listLength = length list
getNthFromEnd _ _ = error "Unknown exception occurred"

The code works as intended as far as I can tell, but seeing as though I am just getting to know Haskell I thought your suggestions on how this code could be cleaned up or refactored to make better use of FP concepts would be invaluable to my learning of Haskell and the FP paradigm overall.

I'll go through the code line-by-line:

 1    getNthFromEnd :: Int -> [a] -> a
 2    getNthFromEnd _ [] = error "Empty list given"
 3    getNthFromEnd nth list
 4        | listLength < nth = error $ "List length must be greater than " ++ show nth
 5        | listLength == nth = head list
 6        | listLength >= nth = list !! (listLength - nth - 1)
 7        where listLength = length list
 8    getNthFromEnd _ _ = error "Unknown exception occurred"

Line 1

Type signature - good!

Line 2

A function which calls out to error is kinda frowned upon. Such functions are
called partial functions because they could throw an exception and that is not
indicated in the type signature. That's why some people don't like to use head
and tail - they prefer to use pattern matching instead for these functions.
This is not really a critic of your code, more just an observation. If you were
to write documentation for this function you might warn users that it is partial,
although they might be able to guess from the description that it necessarily has to be.

More on partial functions:

https://begriffs.com/posts/2013-08-18-dont-be-partial-to-partial-functions.html

Line 4

I would change the error message to List length must be at least ... to match the comparison.

Lines 4 -- 6

Clearly one of the three comparisons has to hold, so for efficiency you can change
the guard in line 6 to an otherwise clause:

| listLength < nth  = ...
| listLength == nth = ...
| otherwise         = ...

This avoids an unnecessary comparison check when listLength is greater or euqal
to nth.

Another common Haskell idiom is to match on the result of compare like this:

-- may need Data.Ord

case compare listLength nth of
  LT  -> ...
  EQ  -> ...
  GT  -> ...

This has the advantage of being provably exhaustive by the compiler. However, it may turn out to be the slowest of the three. It depends a lot on how smart the compiler is. I don't see it used a lot in the Prelude functions for integer comparisons, and I imagine there is an efficiency reason behind that.

To summarize:

The code you've written is exhaustive (handles all the cases) just because we know that one of the comparisons has to hold. But GHC can't prove this, and it may perform an extra comparison when it doesn't need to.
Using an otherwise clause is exhaustive because we're using an otherwise clause. But it avoids an extra check sometimes.
Using compare is provably exhaustive by the compiler.

In any case, this declaration will handle all the cases which leads to...

Line 8

Based on the previous discussion, your function will never get to this point so
this line isn't needed. Even if it did, GHC will throw it's own error with a message
like:

*** Exception: /tmp/err.hs:3:1-17: Non-exhaustive patterns in function foo

Moreover, having this catch-all clause is hiding a potentially useful warning. If you remove that line and compile with -Wall GHC reports:

/tmp/pats.hs:5:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘getNthFromEnd’: Patterns not matched: _ (_ : _)

This raises two points:

The way you've written the code you are not proving to GHC that you are handling all of the list patterns. GHC would like you to write the second stanza with a pattern:

getNthFromEnd nth list@(:) =

That enables GHC to know that you've covered both the empty list and non-empty list cases.

Using the catch-all clause prevents GHC from making this check.


Is my code unit testable I've written a piece of code. Would like to know is the below piece of code unit testable or does it need any kind of refactoring.

MatchCilentUrls is the primary method for matching two urls. Based on my business logic i am matching the Urls.

Appreciate your comments on this.

    public bool MatchClientUrls(string redirectUrl, string whiteListUrls)
    {
        if (String.IsNullOrEmpty(redirectUrl) || String.IsNullOrEmpty(whiteListUrls))
            throw new ArgumentNullException();

        var urlList = whiteListUrls.Split(';');

        var redirectUrlScheme = string.Empty;
        var redirectUrlDomain = string.Empty;
        var urlScheme = string.Empty;
        var urlDomain = string.Empty;

        ParseUrl(redirectUrl, out redirectUrlScheme, out redirectUrlDomain);

        foreach (var url in urlList)
        {
            urlScheme = string.Empty;
            urlDomain = string.Empty;

            ParseUrl(url, out urlScheme, out urlDomain);

            if (String.IsNullOrEmpty(redirectUrlScheme) || String.IsNullOrEmpty(urlScheme))
            {
                if (redirectUrlDomain.StartsWith(urlDomain, StringComparison.OrdinalIgnoreCase) || urlDomain.StartsWith(redirectUrlDomain, StringComparison.OrdinalIgnoreCase))
                    return true;
            }
            else
            {
                if (redirectUrlScheme == urlScheme)
                {
                    if (redirectUrlDomain.StartsWith(urlDomain, StringComparison.OrdinalIgnoreCase) || urlDomain.StartsWith(redirectUrlDomain, StringComparison.OrdinalIgnoreCase))
                        return true;
                }
            }
        }

        return false;
    }

    private void ParseUrl(string url, out string scheme, out string domain)
    {
        scheme = string.Empty;
        domain = string.Empty;

        var index = url.IndexOf("//");
        if (index > 0)
        {
            scheme = url.Substring(0, index - 1);
            domain = url.Substring(index + 2);
        }
        else
        {
            domain = url;
        }
    }

    public bool ValidateUrl(string url)
    {
        var regEx = @"^(http(s)?://)?[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&amp;%\$#_]*)?$";

        if(Regex.IsMatch(url, regEx))
            return true;
        else
            return false;

    }

Of course it is unit testable.

All function/methods are.

A unit test is to flex the function to breaking point. i.e. exercise the usual conditions and the boundary points.


PHP Validation with functions and list() I have multiple form validation checks around my site so I decided to write them as functions to simplify the process of creating the validation pages, here is a snippet for an understanding of what I'm doing.

// Declare regex variables

   $regex_email = "/^[a-zA-Z0-9._-]+@[a-zA-Z0-9-]+\.[a-zA-Z.]{2,5}$/";
   $regex_password = "/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*\W).*$/";

// Declare validation functions

    function checkEmail($regex, $email) {

        if (!preg_match($regex, $email)) {

            $error = "The format of that Email Address is invalid";
            $solution = "Please go back and try again";

        }

        return array($error, $solution);

    }

    function checkPassword($regex, $password) {

        if (!preg_match($regex, $password)) {

            $error = "Your Passwords does not meet our requirements";
            $solution = "Please go back and try again";

        }

        return array($error, $solution);

    }

I was hoping that when I come to validate it I could simply use:

// Values for $email and $password are fetched from `$_POST` before hand...

// Validate form

    list ($error, $solution) = checkEmail($regex_email, $email);
    list ($error, $solution) = checkPassword($regex_password, $password);

// Display error

    echo("Error:
".$error."
Solution:
".$solution."");

But alas, it doesn't work and no errors at all are displayed and I'm having to do error checks after each one like this:

// Validate form

    list ($error, $solution) = checkEmail($regex_email, $email);

    if (!$error) {

        list ($error, $solution) = checkPassword($regex_password, $password);

    }

// Display error

    echo("Error:
".$error."
Solution:
".$solution."");

I don't mind doing it like this, but when I have say 10+ check*VALUE* functions being called, each one wrapped in an if statement within an if statement and so on, it looks a little messy, I wasn't sure if there was a different approach which would work more like the first way I tried without all the if statement's.

This might not be what you are looking for but I recently wrote this code to validate my forms.

As you might notice I define the data to be validated in the $validate array. Then some loops run through them and show the proper errors. If no errors are found, it sends the form.

With some modifications, you could probably use something similar. Hope it helps.

<?php

    # post data collection
    $name   = "Full name";
    $email  = "a@b.c";
    $age    = "193";
    $x      = "s";
    $y      = "s";

    # select data that needs validation
    $validate   = array(
        'required'      => array($name, $email, $x, $y),
        'validEmail'    => array($email),
        'validNumber'   => array($age),
        'validAlpha'    => array($name)
    );

    # error messages
    $errorsMsgs = array(
        'required'      => 'Please fill out all required fields.',
        'validEmail'    => 'is an invalid email address.',
        'validNumber'   => 'is an invalid number.',
        'validAlpha'    => 'contains invalid characters. This field only accepts letters and spaces.'
    );

    $errorMarkup    = "
We found a few errors :-(

Please fix these errors and try again

";
    $successMarkup  = "
Success!

Your form was sent successfully.
";
    $backMarkup     = "Back to form";

    # begin state
    $valid = true;

    # loop through fields of error types
    foreach ($validate as $type => $fields) {
        # loop through values of fields to be tested
        foreach ($fields as $value) {
            # throw error if value is required and not entered
            if ($type === 'required' && strlen($value) === 0) {
                $errorMarkup .= "
$errorsMsgs[$type]
";
                $valid = false;
                break;
            }
            else if (
                        $type === 'validEmail'  && !filter_var($value, FILTER_VALIDATE_EMAIL) ||
                        $type === 'validNumber' && !preg_match('/^[0-9 ]+$/', $value) ||
                        $type === 'validAlpha'  && !preg_match('/^[a-zA-Z ]+$/', $value)
                    ) {
                if (strlen($value) === 0) {break;} # skip check if value is not entered
                $errorMarkup .= "
\"$value\" $errorsMsgs[$type]
";
                $valid = false;
                continue;
            }
        }
    }

    if ($valid) {
        # email form
        $body = $successMarkup . $backMarkup;
        $title = "Form sent";
    } else {
        $body = $errorMarkup . "
" . $backMarkup;
        $title = "Form errors";
    }

    # write html ouput
    echo "<!DOCTYPE html><title>$title</title>$body";
?>


Testing database table creation with NUnit I'm new to using NUnit and have written a test to check for the existence of a database table. I have the below code that should check whether a new table named NewTable has been created in the database. It works correctly but I can't help feeling there's probably a better way of doing this. Thanks.

using(var conn = context.NewConnection()) {
    var tables = conn.GetSchema("Tables");
    foreach(System.Data.DataRow row in tables.Rows) {
        foreach(System.Data.DataColumn col in result.Columns) {
            if(row[col.ColumnName].ToString() == "NewTable")
                Assert.Pass();
        }
    }
}

Assert.Fail("NewTable not created");

I would do something like this instead -

using(var conn = context.NewConnection()) {
    var table = conn.GetSchema("Tables");
    var tableNames = table.Rows.Cast<DataRow>()
                                            .Select(x => x["TABLE_NAME"].ToString())
                                            .ToArray();
    Assert.That(tableNames.Contains("NewTable"), Is.True);
}

Tests are usually structured in Setup-Act-Assert fashion. A good unit test is one that tests just one condition on an action.

Multiple asserts are practically unavoidable in certain cases, but having logic to decide when Assert should pass or fail isn't a good practice. The code under test should have the logic to branch, the test should only look at expected v/s actual results in a given scenario.

In this case, our setup is - we get the schema from the database, and get the list of column names.

Our logic (Act) is that certain column name must exist. So we write an assertion to ensure that.

In other cases, there is an action and a result, the assertion is on the result. In this case, it is more of testing a state, so there isn't an assertion on the result of an action.


Project Euler # 3 in Haskell: largest prime factor of some number The program works, but is extremely slow.

Project Euler problem 3

Problem: What is the largest prime factor of the number 600851475143?

module Problem_3 where

-- find largest prime factor:
findLargestPrimeFactor :: [Int] -> Int
findLargestPrimeFactor []     = 1
findLargestPrimeFactor (n:ns) =
    if (isPrime n)
    then n
    else findLargestPrimeFactor ns

-- Supplementary function definitions:

-- function: listOfFactors n
listOfFactors :: Int -> [Int]
listOfFactors n = [x | x <- [1..n], isFactor x]
    where
        isFactor x = (n `mod` x == 0)

-- function: isPrime p
isPrime :: Int -> Bool
isPrime p = (p >= 2 && listOfFactors p == [1,p])

main = do
          let myGivenNumber = 600851475143
          let largestPrimeFactor = findLargestPrimeFactor (reverse (listOfFactors myGivenNumber))
          print largestPrimeFactor

Naming

findLargestPrimeFactor isn't true to its name. It just returns the first prime number in the given list or 1 otherwise. So firstPrime :: [Int] -> Int might be more apt.

Efficency

At the moment, you check \$ n \$ candidates in listOfFactors. But that's not necessary. First of all, if we found a divisor x, then we also found a second divisor, n `div` x. We also only need to check x up to \$ \sqrt{n}\$:

listOfFactors :: Int -> [Int]
listOfFactors n = concat [[x,d] | x <- [1..nsqrt], let (d,m) = n `divMod` x, m == 0]
  where
    nsqrt = ceiling (sqrt (fromIntegral n))

Now we only need to check \$\left\lceil \sqrt{600851475143}\right\rceil = 775147\$ numbers, which is much, much less. This variant will actually  finish in under one second, whereas I didn't run my program till the end.

However, you now have to sort your list, or use filter isPrime followed by maximum in findLargestPrimeFactor, whose name would be then mostly (beside the Factor) apt.

If you rewrite findLargestPrime so that it returns the largest prime from a given list, we would end up with the following main:

main :: IO ()
main = print (findLargestPrime (listOfFactors 600851475143))

Which seems reasonable.

Another approach

While one can solve this problem in this way, one can also just generate the prime factors of the number:

primeFactors :: Int -> [Int]
primeFactors n = go 2 n
  where
    go _ 1 = []
    go k n = case n `quotRem` k of
               (n', 0) -> k : go k n'
               _       ->     go (k + 1) n'

If k is going to be put in the list, it will always be prime. Note that one can improve this function further, but that's left as an exercise.

Exercises

In the section "efficency", we were able to reduce the runtime by narrowing the search space. However, why is it enough to run up to $\ \sqrt{n} \$ only?
In the section "another approach", one can cut the amount of checked numbers almost by half. How?


Initializing some resources My code style is capped 128 characters, not 80 characters. This abstract base class constructor initializes some resources which are injected in the implementations. However, I cannot assume that will be always the case, so I used some try catching to and Apache Commons Lang3 Validate to validate my inputs and throw applicable errors. The base class is normally implemented as a stateless ejb, because of the database rollback features. The BusinessConfigurationException extends Exception and is annotated with @ApplicationException(rollback = true), so that these checked exceptions will be thrown without being wrapped in an EJBException and will do a rollback of all database actions correctly.

/**
 * BaseController constructor with injectable parameters.
 *
 * @param configBundle Configuration parameters for the application.
 * @param errorsBundle Error messages thrown by the application.
 * @param em Needs to be injected from implementation.
 * @param clazz The class that implements the BaseEntity.
 * @throws BusinessConfigurationException When the property value returns an error somehow.
 */
public BaseController(final ResourceBundle configBundle, final ResourceBundle errorsBundle, final EntityManager em,
        final Class<T> clazz) throws BusinessConfigurationException {
    try {
        Validate.notNull(configBundle);
        this.baseURI = configBundle.getString("HOST_NAME") + configBundle.getString("APPLICATION_NAME")
                + configBundle.getString("BASE_PATH");
    } catch (NullPointerException | MissingResourceException | ClassCastException ex) {
        log.error("Cannot initialize baseURI in BaseController constructor.", ex);
        throw new BusinessConfigurationException(ex.getMessage());
    }
    try {
        Validate.notNull(errorsBundle);
        this.errorsBundle = errorsBundle;
    } catch (NullPointerException ex) {
        log.error("Cannot initialize errorsBundle in BaseController constructor.", ex);
        throw new BusinessConfigurationException(ex.getMessage());
    }
    try {
        Validate.notNull(em);
        this.em = em;
    } catch (NullPointerException ex) {
        log.error("Cannot initialize em in BaseController constructor.", ex);
        throw new BusinessConfigurationException(findErrMsg("EM_NOT_AVAILABLE"));
    }
    // No business exceptions thrown for valdidations below. This is a programmer fault.
    Validate.notNull(clazz);
    Validate.isAssignableFrom(clazz, BaseEntity.class);
    this.clazz = clazz;
    this.resourceName = clazz.getSimpleName().toLowerCase();
}

What can be a bit improved:

1) Some Validate.notNull() are not needed, see my comments in the code below.

2) When possible to avoid try/catch avoid it.

3) Recommended to put an error message in Apache Validate calls. See the changed isAssignableFrom().

public BaseController(final ResourceBundle configBundle, final ResourceBundle errorsBundle, final EntityManager em,
        final Class<T> clazz) throws BusinessConfigurationException {
    try {
        // Validate.notNull(configBundle);  <--- Validation for null is not needed because you will get a NPE in any case in the next line.
        this.baseURI = configBundle.getString("HOST_NAME") + configBundle.getString("APPLICATION_NAME")
                + configBundle.getString("BASE_PATH");
    } catch (NullPointerException | MissingResourceException | ClassCastException ex) {
        log.error("Cannot initialize baseURI in BaseController constructor.", ex);
        throw new BusinessConfigurationException(ex.getMessage());
    }

    if (errorsBundle != null) {
        this.errorsBundle = errorsBundle;
    } else {
        log.error("Cannot initialize errorsBundle in BaseController constructor.", ex);
        throw new BusinessConfigurationException(ex.getMessage());
    }

    if (em != null) {
        this.em = em;
    } else {
        log.error("Cannot initialize em in BaseController constructor.", ex);
        throw new BusinessConfigurationException(findErrMsg("EM_NOT_AVAILABLE"));
    }

    // No business exceptions thrown for valdidations below. This is a programmer fault.
    // Validate.notNull(clazz); <---- Not needed because the next validation will detect null clazz.
    Validate.isAssignableFrom(clazz, BaseEntity.class, "To construct BaseController the given clazz is to be of BaseEntity type but was %s", clazz);
    this.clazz = clazz;
    this.resourceName = clazz.getSimpleName().toLowerCase();
}


NTP Clock on a Raspberry Pi I've been tinkering around with making a fancy clock on a Raspberry Pi B. This little application will sync the Pi's time with an NTP server, then display it using toilet so it's nice n' big & centered on a monitor. This was made for a GUI-less Pi, and I'm looking to hopefully make it more efficient. Right now the seconds aren't a uniform duration (the time.sleep(0.3) is there to even it out), and I'm sure it's because the Pi is struggling to keep up with my newbie code.

#!/usr/bin/env python
import subprocess
import sys
import os
import time
from datetime import datetime
import ntplib

NTP_MISS = 0

def display_clock():
    #Clock Positioning
    rows, columns = os.popen('stty size', 'r').read().split()
    h_pos = int(int(columns) / 6)
    v_pos = "\033[" + str(int(int(rows) / 6)) + ";0H"

    #Initialization
    check_ntp_time = time.time()
    os.system('setterm --cursor off -powersave off')
    subprocess.call("clear")

    #Meat & Potatoes
    try:
        while check_ntp_time + 86400 >= time.time():
            current_date = subprocess.check_output(["date", "+%r"]).strip().decode("utf-8")
            current_date = " " * h_pos + current_date
            sys.stdout.write(v_pos + subprocess.check_output(["toilet", "-w 200", current_date]).decode("utf-8"))
            sys.stdout.flush()
            time.sleep(0.3)
    except KeyboardInterrupt:
        print("Closing NTP Clock")
        os.system('setterm --cursor on -powersave on')
        subprocess.call("clear")
        print("Closed NTP Clock")
        exit()

    #Update time using NTP pool.
    get_ntp_time()

def get_ntp_time():
    call = ntplib.NTPClient()
    while True:
        try:
            #Try to connect to the NTP pool...
            response = call.request('pool.ntp.org')
            print("Time updated!")
            break
        except:
            NTP_MISS += 1
            if NTP_MISS >= 5:
                print("NTP server unavailable for too long! Terminating...")
                os.system('setterm --cursor on -powersave on')
                subprocess.call("clear")
                print("NTP Clock failed. Server unresponsive for too long.")
                exit()
            #Not a huge deal if we miss it, just continue displaying the clock.
            print("Could not connect to pool.")
            display_clock()
    #If we get a response, update the system time.
    t = datetime.fromtimestamp(response.tx_time)
    t = t.strftime('%Y-%m-%d %I:%M:%S %Z')
    set_string = "--set=" + t
    subprocess.call(["date", set_string])
    display_clock()

if __name__ == "__main__":
    display_clock()

I'd prefer to use the subprocess instead of os.system, but for some reason using subprocess with setterm returned a "command not found" error. So that's why I'm using os.system instead.

What all can I do to optimize & clean up this code?

In general your script looks good when considering the style and structure, but there are a few issues:

Strange name for check_ntp_time – This holds the time since the last time you did time.time(), and in reality is a temporary variable for when you need to check it the last time. It doesn't convey the real purpose of the variable.

I would rather do time_to_check_ntp = time.time() + 86400, and let the condition be time.time() < time_to_check_ntp or similar, which reads easier as to whether we've come to that time already, or not.
Consider replacing toilet with python stuff – One option could be to use pyfiglet, and another could be to use fonts of your own based on script suggestions from "Is there a python library that allows to easily print ascii-art text?" and "ASCII Art With Letters".
New function for setterm & clear – Introduce a new function where you can set it to on or off, which would remove some repetition of code.
Replace call to date to get date with datetime module – In general I would like for python script to do most of it using python stuff. Some of it like clear and setterm, or setting the date needs to be done in the shell on the terminal, so some can't be avoided, but stuff like getting the date I think should be done using internal tools, if possible.
Avoid the recursivity – Both display_clock() and get_ntp_time() calls each other causing a bad recursivity. If you change display_clock to have a while True: loop you could have a if time.time() > time_to_check_ntp: which then calls get_ntp_time() which possibly should be named correct_time_using_ntp(), and sets a new time_to_check_ntp.
Remove output before clearing the terminal – No point in outputting text straight ahead of a terminal clear. Nobody would be able to catch these message, unless something fails. I would remove it, or add a logger to keep track of the history of the script.

I'm not going to propose new code, as I don't have the ability to test it, but I hope my suggestions are meaningful and you see how to implement it. If not, please do comment.


Function for getting strings consisting of random characters I've made this function for to be used while testing.

// ----- THE ACTUAL FUNCTION -------------------------

// Returns a string consisting of random alphabetic
//  characters.

// @param { string } separator - Char which shall be
//  used to separate the single chars.
// @param { number } min - Minimal length of the string.
// @param { number } max - Maximal length of the string.
// @param { string } inCase - Case of characters to use.
//  - Use 'both' for getting a string consisting of
//   lower- and upper-case character.
//  - Use 'upper' for getting a string consisting of
//   only upper-case characters.
//  - Leave the parameter out, pass an empty string or
//   any other string for getting a string consisting
//   of only lower characters.
// @return { string }
// @throws { object } in case of error.

function getRandomString(separator, min, max, inCase) {
  var getPartRandomString = function() {
    return Math.random()
    .toString(36)
    .replace(/[^a-z]+/g, '');
  };

  var setReturnString = function() {
    ret += getPartRandomString();

    if (ret.length < min) {
      setReturnString();
    }
  }

  var ret = '';

  if (min >= max) {
    throw {
      message: 'Minimum can not be greater or equal Maximum.'
    }
  }

  min = min || 5;
  max = max || 8;

  separator = separator || '';

  setReturnString();

  ret = ret.split('');

  if (inCase === 'both') {
    ret = ret.map(function(currentChar) {
      if (Math.random() >= 0.5) {
        currentChar = currentChar.toUpperCase();
      }

      return currentChar;
    });
  }

  ret = ret.join(separator);

  if (inCase === 'upper') {
    ret = ret.toUpperCase();
  }

  ret = ret.slice(0, ret.length <= max ? ret.length : max);

  if (separator && ret.charAt(ret.length - 1) === separator) {
    ret = ret.slice(0, ret.length - 1);
  }

  return ret;
}
// ----- THE ACTUAL FUNCTION ---- End ----------------------

function testGetRandomString(min, max, caseToTest, limit, regExp) {
  var logMessage = function(message) {
    console.log('%s%s => %s',
                receivedResult,
                new Array(max - receivedResult.length + 1)
                .join(' '),
                message
               );
  }

  var receivedResult;

  min = min || 6;
  max = max || 12;
  limit = limit || 10;
  regExp = regExp || /[^a-z]/g;

  for (let i = 0; i < limit; i++) {
    receivedResult = getRandomString('', min, max, caseToTest);

    if (receivedResult.length < min) {
      logMessage('Less then min.');
    } else if (receivedResult.length > max) {
      logMessage('Larger or equal max.');
    } else if (receivedResult.search(regExp) !== -1) {
      logMessage('Unallowed chars.');
    } else {
      logMessage('Test passed.');
    }
  }
}

testGetRandomString(6, 12, '', 10);
console.log('\n');
testGetRandomString(6, 12, 'both', 10, /[^a-zA-Z]/g);
console.log('\n');
testGetRandomString(10, 18, 'upper', 10, /[^A-Z]/g);

console.log('\n');
console.log(getRandomString('-', 1, 2, 'both'));

console.log('\n');
console.log(getRandomString('-', 10, 20, 'both'));

It seem to work alright. But frankly: With all the parameter-checking and taking different possibilities into account it has become a lot of code.

If someone knows a possibility to write it more elegant then I would appreciate his or her answer. Any other hints welcomed too.

Should mention: I can't use ES 6-syntax because we got only Internet Explorer 11 here and there won't be change in any nearer future.

My immediate thought is that the function is just doing too many things.

It'd be simpler to just return a mixed-case string of a specific length. Done.

Want that string to be all-uppercase? Just call getRandomString(n).toUpperCase().
All-lowercase. Call getRandomString(n).toUpperCase().

Want it to have a separator between each letter? Call getRandomString(n).split('').join(separator).

Want it be be somewhere between x and y characters long? Pick a random length yourself, and pass that to the function.

This just seems like a function built for a very narrow purpose. For instance, what if (for whatever reason) you only want a separator every 4 characters? Do you add yet another argument to the function, or do you write a separate function to chopping and glueing the way you want it?

Even if you need a single function to do everything, it'd be simpler to have it just call out to other functions that comprise the building blocks. Lots of the functions you need already exist (toUpperCase, toLowerCase, split, and join), so it's only the randomness you need to add.

A simple and crude structure might be:

function getRandomInt(min, max) {
  var span = parseInt(max, 10) - parseInt(min, 10);
  if(span <= 0) {
    throw { message: 'Min must be less than max.' };
  }
  return min + Math.round(Math.random() * span);
}

function getRandomLetter() {
  var asc = getRandomInt(65, 116);
  if(asc > 90) asc += 6;
  return String.fromCharCode(asc);
}

function getRandomString(length) {
  var i, letters = [];
  length = parseInt(length, 10);

  for(i = 0 ; i < length ; i++) {
    letters.push(getRandomLetter());
  }
  return letters.join('');
}

// this is the equivalent of original function
function getCustomRandomString(separator, min, max, style) {
  var string = getRandomString(getRandomInt(min, max));

  if(style === 'upper') {
    string = string.toUpperCase();
  } else if(style != 'both') {
    string = string.toLowerCase();
  }

  if(separator) {
    string = string.split('').join(separator);
  }

  return string;
}


Database connection class performance This is my general database connection class. I am using this class to execute my queries through website.

What would your suggestions on how to improve performance be?

(Running MSSQL 2008 R2 SP1 - Microsoft Visual Studio 2010 SP1 , C# 4.0 - ASP.net 4.0)

Class:

using System;
using System.Collections.Generic;
using System.Collections;
using System.Linq;
using System.Web;
using System.Data.Sql;
using System.Data.SqlClient;
using System.Data;
using System.IO;

/// <summary>
/// Summary description for DbConnection
/// </summary>
public class DbConnection
{
    public static string srConnectionString = "server=localhost;database=myDB;uid=sa;pwd=MYPW;";

    public DbConnection()
    {

    }

    public static DataSet db_Select_Query(string strQuery)
    {
        DataSet dSet = new DataSet();

        try
        {
            using (SqlConnection connection = new SqlConnection(srConnectionString))
            {
                connection.Open();
                SqlDataAdapter DA = new SqlDataAdapter(strQuery, connection);
                DA.Fill(dSet);
            }
            return dSet;
        }

        catch (Exception)
        {
            using (SqlConnection connection = new SqlConnection(srConnectionString))
            {
                if (srConnectionString.IndexOf("select Id from tblAspErrors") != -1)
                {
                    connection.Open();
                    strQuery = strQuery.Replace("'", "''");
                    SqlCommand command = new SqlCommand("insert into tblSqlErrors values ('" + strQuery + "')", connection);
                    command.ExecuteNonQuery();
                }
            }
            return dSet;
        }
    }

    public static void db_Update_Delete_Query(string strQuery)
    {
        try
        {
            using (SqlConnection connection = new SqlConnection(srConnectionString))
            {
                connection.Open();
                SqlCommand command = new SqlCommand(strQuery, connection);
                command.ExecuteNonQuery();
            }
        }
        catch (Exception)
        {
            strQuery = strQuery.Replace("'", "''");
            using (SqlConnection connection = new SqlConnection(srConnectionString))
            {
                connection.Open();
                SqlCommand command = new SqlCommand("insert into tblSqlErrors values ('" + strQuery + "')", connection);
                command.ExecuteNonQuery();
            }

        }
    }
}

You are not using a DataContext, so presuming you don't want to for this, I'd suggest that you also consider a connection pool, that way you don't need to incur the connection cost for every select/delete.

Additionally, your catches are very heavy, are you getting that many errors that you need to track?  You may want to see this question as an alternative to logging the errors yourself: https://stackoverflow.com/questions/5076303/sql-server-error-messages

@kubal5003 - good point on automatic connection pooling, here is the MSDN that discusses it and its sibling that lists the connection string properties and defaults


Simple file locking wrapper command in C A simple command that wraps another command, locking a file first. It is similar to flock, just simpler.

#include <stddef.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/file.h>
#include <err.h>
#include <stdlib.h>

void attemptLock(const char *lockFileName)
{
    int fd = open(lockFileName, O_CREAT | O_WRONLY, 0666);
    if (!fd || flock(fd, LOCK_EX))
        warn("Cannot lock %s", lockFileName);
}

int main(int argc, char *argv[])
{
    if (argc < 3)
        errx(EXIT_FAILURE, "Not enough arguments.\nUsage: %s LOCKFILENAME COMMAND...\n", argv[0]);
    attemptLock(argv[1]);
    argv += 2;  // Skip our own command name and LOCKFILENAME.
    execvp(argv[0], argv);
    err(EXIT_FAILURE, "Cannot execute %s", argv[0]);
}

Anything fishy? Bogus? Wrong? Anything that can be improved (besides adding options like -h)?

Besides reinventing the wheel the things I'm concerned about are:

the warn in attemptLock, because if either creating the lock file
fails, or if you can't get the lock (or the call was interrupted), I
would expect the wrapper to exit instead,
the return value of open, which might very well be negative, so just
check for that separately and give an error message for that,
the return value of flock, where you should handle EINTR in a loop
I believe?