Progress bar/loader with AngularJS Here is my code to add a progress bar on every HTTP request. Can someone please point out to me whether I am doing it right or not?

Template for loader:

Processing..., Please Wait...

Interceptor:

.factory('authInterceptor', ['$rootScope', function($rootScope){
            return {
                request : function request (config) {
                    $rootScope.loaderState = true;
                    return config;
                },
                response : function response(response) {
                    $rootScope.loaderState = false;
                    return response;
                }
            };
        }])

Yes, u can do like this. But some scenario doesn't work.

Scenario 1: If controller has 5 parallel services, at that time loader will get ended after any response received in out of 5.

Scenario 2: We don't need to show loader to some of the services, internally we will call based on the click or any action. At that time it will fail.

Instead of using interceptor better use that flag in respective controller itself or u have to pass additional flag in the header to hide and show loader via interceptor. My suggestion is handle that in respective controller itself.


Javascript to show folder and files by calling Web Api I have this Javascript which call the web api to see the available folder and then the available files. Any comment or code review on my code?

    var divFolder = document.getElementById("rootFolder");
    var divFiles = document.getElementById("files");

    function createFolderElement(data) {
        var li = document.createElement("div");
        li.setAttribute('data-id', data.filePathId);
        li.setAttribute('class', "folder");

        divFolder.appendChild(li);
        li.innerHTML = data.folderName
    }

    function createFileElement(data) {
        debugger
        var li = document.createElement("div");
        li.setAttribute('data-id', data.id);
        li.setAttribute('class', "file");

        divFiles.appendChild(li);
        li.innerHTML = data.fileName
    }

    async function fetchFolder(parentId) {
        document.getElementById('spinner').style.display = 'block'
        divFolder.innerHTML = ''

        const response = await fetch(`https://localhost:44371/api/file/folder/${parentId}`);

        if (!response.ok) {
            const message = `An error has occured: ${response.status}`;
            $("div.message").text(message);
            $("div.message").addClass("message alert alert-warning");
            //throw new Error(message);
        }

        const files = await response.json();
        if (files.length > 0) {
            files.forEach((data) => {
                createFolderElement(data)
            })

            createFolderEventListener();

            document.getElementById('spinner').style.display = 'none'
        }
    }

    async function fetchFiles(filePathId) {
        document.getElementById('spinner').style.display = 'block'
        divFolder.innerHTML = ''

        const response = await fetch(`https://localhost:44371/api/file/files/${filePathId}`);

        if (!response.ok) {
            const message = `An error has occured: ${response.status}`;
            $("div.message").text(message);
            $("div.message").addClass("message alert alert-warning");
            //throw new Error(message);
        }

        const folders = await response.json();
        debugger
        if (folders.length > 0) {
            folders.forEach((data) => {
                createFileElement(data)
            })

            createFileEventListener();
        }

        document.getElementById('spinner').style.display = 'none'
    }

    function createFolderEventListener() {
        const myfolder = document.getElementsByClassName("folder");
        [...myfolder].forEach(function (element) {
            element.addEventListener("click", function () {
                fetchFolder(element.dataset.id);
                fetchFiles(element.dataset.id);
            });
        });
    };

    function createFileEventListener() {
        const myfolder = document.getElementsByClassName("file");
        [...myfolder].forEach(function (element) {
            element.addEventListener("click", function () {
                downloadPdf(element.dataset.id, element.textContent)
            });
        });
    };

    $(function () {
        document.getElementById('spinner').style.display = 'none'
        fetchFolder(1);
    })
    div.folder {
        border: 1px solid black;
        margin: 2px;
        cursor: pointer
    }

    div.file {
        border: 1px solid black;
        margin: 2px;
        cursor: pointer
    }

Prefer modern syntax You're using a few ES2015+ features already, including async/await. If you're going to write a script containing modern syntax (which you should - it can make code much easier to read and write), best to use it everywhere. In particular:

Use const (or let when you need to reassign) instead of var. var has too many problems to be worth using nowadays, such as an unintuitive function scope (instead of block scope), hoisting, and implicitly creating properties on the global object when on the top level. (Use the no-var linting rule.)

In ES2015+, iterators are available. Utilizing these can make code significantly more concise. For example:
const myfolder = document.getElementsByClassName("folder");
[...myfolder].forEach(function(element) {

can be
for (const element of document.querySelectorAll('.folder')) {

jQuery or not? You're using a bit of jQuery, and a lot of vanilla JS DOM manipulation. This is odd, stylistically. One would usually expect, if jQuery is used, for it to be used everywhere that it can be, at least regarding the DOM. But jQuery doesn't really provide anything that can't be done just as easily in vanilla JS, so you could remove jQuery as a dependency entirely too, if you wished. (That's what I'd choose, but it's up to you.)
Attributes 2 things:

When you want to assign to a data- attribute, consider using dataset instead of the more verbose setAttribute:
li.setAttribute('data-id', data.filePathId);

can be
li.dataset.id =  data.filePathId;

Classes can be also be set with dot notation, with className:
li.setAttribute('class', "folder");

can be
li.className = 'folder';

Avoid direct insertion of HTML markup You have:
$("div.message").text(message);

That's a good way to do things - assign text to the text content of an element. But elsewhere, you have
li.innerHTML = data.folderName

That can result in arbitrary code execution. It can also result in formatting problems relating to HTML entities. It sounds very likely that you're expecting folderName to only contain a plain non-HTML string, in which case you should set only the text of the <li>:
li.textContent = data.folderName;

Even though the folder name from the API response is almost certainly trustworthy, using textContent instead of innerHTML is a good habit to get into.
Semicolons Sometimes you're using semicolons when they're needed, and sometimes you aren't. To be stylistically consistent, best to choose one style - either use them wherever appropriate, or don't use them at all. Note that if you don't use them at all, unless you're an expert, you may well eventually run into problems due to Automatic Semicolon Insertion. (I'd recommend using semicolons.) To enforce a particular style on your code, use a linter.
fetchFolder and fetchFiles errors If either the fetchFolder or fetchFiles request fails, there are 2 problems:

You don't return or throw if the response is not OK. As a result, the await response.json(); will throw. The control flow is odd.
You show the spinner at the beginning of the request, but if there's an error, the code will never get to the end of the function where the spinner is hidden again.

The two functions also do very similar things - they show the spinner, do a fetch, and on error, show an error element to the user. Make things more DRY by creating a wrapper function around them.
Or, you could also consider using Promise.all - wait for both requests to complete before updating the UI, to make things look more natural to the user.
const fetchPath = (path) => {
    const response = await fetch(`https://localhost:44371/api/${path}`);
    if (!response.ok) {
        throw new Error(response.status);
    }
    return response.json();
};
const fetchFolderAndFiles = (id) => {
    const spinner = document.getElementById('spinner');
    spinner.style.display = 'block';
    divFolder.textContent = '';
    await Promise.all([
        fetchPath(`files/${id}`),
        fetchPath(`folder/${id}`),
    ])
        .then(([files, folders]) => {
            showFiles(files);
            showFolders(folders);
        })
        .catch((error) => {
            const message = `An error has occured: ${error.message}`;
            const messageElm = document.querySelector('div.message');
            messageElm.textContent = message;
            messageElm.className = 'message alert alert-warning';
        })
        .finally(() => {
            spinner.style.display = 'block';
        });
};

Don't check array length before iterating There's no need for
if (folders.length > 0) {
    folders.forEach((data) => {

If the array is empty, no iterations will occur anyway. Surrounding the forEach in another if statement increases indentation unnecessarily and makes more unnecessary cognitive overhead.
Duplicate listener bug Every time fetchFolder is called, you also call createFolderEventListener, which does:
const myfolder = document.getElementsByClassName("folder");
[...myfolder].forEach(function (element) {
    element.addEventListener("click", function () {

So every time a folder is clicked, you're adding additional listeners to every folder element on the page. This will result in many unnecessary requests, and possibly more strange behavior.
Only add a listener to a folder if the folder doesn't already have a listener. Or, even better, add only a single listener, to a parent element, and utilize event delegation:
document.addEventListener('click', (e) => {
    if (!e.target.matches('.folder')) {
        return;
    }
    const { id } = e.target.dataset;
    fetchFolder(
        fetchFiles(id);
    );
});

Run that once, on pageload, and then you can remove createFolderEventListener.
Do the same thing for createFileEventListener, which has the same problem.
Precise variable names You have:
var divFiles = document.getElementById("files");

and
function createFileEventListener() {
  const myfolder = document.getElementsByClassName("file");

(createFileEventListener should probably be removed entirely, but if it isn't, make sure to change myfolder to myfile)
Precise variable names make code easier to read and helps prevent bugs. A variable name should probably only be plural if it refers to a collection. Otherwise, it should be singular. Similarly, a collection should have a plural variable name. Maybe call it filesContainer instead of divFiles, and fileDivs instead of myfolder.


Automatically backup website files and database For each of my various websites, I back up the web site files and the associated database for into a single dated compressed archive file (one zip file per website) and download that to my local Windows Vista machine.  My websites are hosted on Unix machines accessible via ssh.  I've decided to use Python Fabric as it seems to be well-fit for the job.

This works but it's kind of a mess because I just hacked it together.  I want to make it cleaner and a little more generalized.  For example, right now it hard-codes where I'm storing web apps on the server, but actually for some of my websites they are stored in different places.  Also, it feels a little wonky to save a db file to a web file directory, but I'm not sure how else I can easily get it into the zip archive.

import time
from fabric.api import *

env.hosts = ['example.com']
env.user = 'someuser'
env.password = 'somepassword'

def backup_database(app_name, db_name):
    db_passwords = {'somedatabasename': 'somedbpassword'}
    mysqldump_command = 'MYSQL_PWD=%(password)s ionice -c2 -n6 mysqldump -u' +
        ' %(database_name)s %(database_name)s ' +
        '> ./webapps/%(app_name)s/%(database_name)s.sql' % {
        'password': db_passwords[db_name],
        'database_name': db_name,
        'app_name': app_name
        }
    run(mysqldump_command)

def backup_site(app_name):
    date = time.strftime('%Y-%m-%d')
    zip_filename = '%(app_name)s-%(date)s.zip' % { 'date': date, 'app_name': app_name }
    zip_command = 'zip -r %(zip_filename)s ./webapps/%(app_name)s > /dev/null' % {
        'zip_filename': zip_filename, 'app_name': app_name }
    run(zip_command)
    get(zip_filename, zip_filename)
    run('rm ' + zip_filename)

def backup_webapp(app_name, db_name):
    backup_database(app_name, db_name)
    backup_site(app_name)
    rm_command = 'rm $HOME/webapps/%(app_name)s/%(database_name)s.sql' % {
        'database_name': db_name,
        'app_name': app_name
    }
    run(rm_command)

def backup():
    backup_webapp('somewebappfoldername', 'somedatabasename')

Using the fabric tool I call the script like this:

  c:> fab backup

If you don't know fabric, the script file is Python and the run commands are executed via ssh on the remote server. And the get command will pull a file from the remote server to the local machine.

What it does is:

Backup database to a an SQL file and put it in webdirectory
Zip web directory
Download zip to my local machine
Delete the zip file on the server and delete the database backup from the web directory

The names are chosen well. The code is clear. Obviously it is for a specialized case so there are many constants in the code - that is OK. It is strictly procedural, which is a good thing - OOP has a tendency to just become boilerplate for small things like these (it would be useful if you intended to make a framework for a general backup tool)

I would definitely remove the unnecessary variables.

db_passwords = {'somedatabasename': 'somedbpassword'} which is only used a few lines later as db_passwords[db_name], take the password as an argument to the function instead.
date = time.strftime('%Y-%m-%d'), it would be understandable if you had to use it twice and be sure it still was the same day (in that case I would argue for a function argument), but in your case you only use it once. So remove the variable and do instead ... 'date': time.strftime('%Y-%m-%d').

Some variables could be better as functions, for example
 - zip_command, I would put that into a function which would take app_name and filename as argument and then return the command. Of course, the day you want different zip commands depending on database size or backup time you just add an extra argument.

Perhaps more subjective but I would go for the new Advanced String Formatter.

To generalize your tool, I would do one of the two: a) put your options on the command line (e.g. argparse or optparse) or b) use a configuration file (fabric can take an environment file as argument). I would absolutely only put those options that do vary between the sites (rather than making everything possible with an explosion in code for no added real utility).


Integers to English words in C As a beginner in C programming, I decided to write a function words that, given an unsigned integer n, puts the English representation in words of n into the character array dest.

This code is quite difficult to follow due to its complexity. If I hadn't written this code, it would take me a good while to understand it fully. What can I do to improve readability and reduce the unnecessary complexity?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

const char *digits[] = { "one ", "two ", "three ", "four ", "five ", "six ", "seven ", "eight ", "nine " };
const char *tens[] = { "ten ", "twenty ", "thirty ", "forty ", "fifty ", "sixty ", "seventy ", "eighty ", "ninety " };
const char *teens[] = { "eleven ", "twelve ", "thirteen ", "fourteen ", "fifteen ", "sixteen ", "seventeen ", "eighteen ", "nineteen " };
const char *hundreds[] = { "thousand ", "million ", "billion " };

char *strprep(const char *prefix, char *dest)
{
    char *temp = strdup(dest);
    sprintf(dest, "%s%s", prefix, temp);
    free(temp);
    return dest;
}

void words(unsigned int n, char *dest)
{
    unsigned int l, r, m, d = 0, t = 0;
    char d_cpy[16];

    while(n)
    {
        r = n % 10;
        n = (n - r) / 10;
        m = d % 3;

        if(d && !m)
        {
            if(r || n % 100)
            {
                strprep(hundreds[t], dest);
            }

            t++;
        }

        if(r)
        {
            switch(m)
            {
                case 0:
                    if(n % 10 != 1)
                    {
                        strprep(digits[r - 1], dest);
                    }
                    break;
                case 1:
                    if(l && r == 1)
                    {
                        strprep(teens[l - 1], dest);
                    }
                    else
                    {
                        strprep(tens[r - 1], dest);
                    }
                    break;
                case 2:
                    strcpy(d_cpy, digits[r - 1]);
                    strprep(strcat(d_cpy, "hundred "), dest);
            }
        }

        l = r;
        d++;
    }

    if(!strlen(dest))
    {
        strcpy(dest, "zero");
    }
}

To provide some context, words() is called in main() (defined in the same file):

#include <limits.h>

int main(int argc, char *argv[])
{
    char *last;
    unsigned long int n;

    if(argc != 2)
    {
        puts("incorrect number of arguments!");
        printf("usage: %s [integer]\n", argv[0]);
        return -1;
    }

    n = strtoul(argv[1], &last, 10);

    if(*last || !(*argv[1]) || n > UINT_MAX)
    {
        puts("first argument must be a valid integer");
        return -1;
    }

    char buf[256];
    puts(words((unsigned int)n, buf));
}

Keep your line-length under control. Horizontal scrolling is death on readability.
strprep() is extremely inefficient, and even assuming the destination is big enough, you don't check whether allocation of your needless temporary buffer succeeds. Better to fix that:

char *strprep(const char *prefix, char *dest) {
    size_t prefix_len = strlen(prefix);
    memmove(dest + prefix_len, dest, strlen(dest) + 1);
    memcpy(dest, prefix, prefix_len);
    return dest;
}

Assuming a user-supplied Buffer contains an empty string for no reason is certainly an interesting decision. It violates the principle of least surprise though, and leads to Undefined Behavior if the assumption proves unfounded.
I won't try to decipher what your cryptic single-character variables in words() are for. Do everyone (especially yourself) a favor and invest some more into finding good names.
Using strlen() to decide whether a string is empty? That's an \$O(n)\$ call where a primitive direct check is sufficient:

if (!*dest) // dest is empty

Most optimizing compilers in hosted mode will succeed in lowering it to the above, but why write that much more and rely on it?
Actually, check for zero beforehand instead. No need to do so after failing to put it into words some other way.
Either your example code violates the contract of words(), or words is buggy. I suggest fixing words() to not assume the buffer is pre-filled with an empty string.
Consider merging multiple outputs into one request. Don't worry, due to adjacent string-literals being concatenated by the compiler, that need not result in one humungous string-literal.


Regex to parse horizontal rules in Markdown I'm the current maintainer of Showdownjs, a markdown parser off which PageDown (stackexchange's markdown parser) is based on.

Showdown uses the following regex to parse horizontal rules:

/^( ?(-|\*|_) ?){3,}[ \t]*$/gm

This works fine for most cases. However, the following input:

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - abc

makes the library painfully slow (takes about 10 seconds on nodeJS and longer in browsers). The more dashes you add, the slower it gets.

In fact, if you copy the input above and paste it here on stackexchange, the page will freeze. (stackexchange uses a similar regex to parse <hr>)

How can I make it faster?

Congratulations, what you have here is a case of really really bad backtracking.  Remember the StackOverflow outage in July 2016? Because it's explained quite nicely there.

regex101 is so kind as to tell us in the debugger view how many steps the regex takes until it matches. In case you were wondering, it's 111,803

We can drastically reduce the number of steps by removing alternative paths from the matcher graph . This can be done by eliminating optionals, dropping non-greedy quantification and some other hacks. In this case we can use a simple trick that's related to the fact that we should only allow a single space in between the asterisks,dashes and underscores:

/^( ?[-_*]){3,} ?[\t]*$/

Already cuts down the recognition of the pattern mismatch in your example to a measly 272 steps.  Do note that I took the liberty of replacing your ORing capture group with a significantly "easier" character class. The functionality is basically equivalent, if we ignore the fact that the capture group you use results in mismatching and additional steps.
This change makes it easier for the regex-engine to determine a match.

The only significant difference between your pattern and mine is that mine doesn't recognize the "non-standard" (as far as that's defined in markdown):

-  -  -
- - - // as opposed to this with only one space between dashes

well and the fact that it doesn't exhibit catastrophic backtracking :) check it out on regex101


Animating the opacity of certain page elements I wrote this jQuery code to animate the opacity (sort of Fadein) of certain elements on my page in consecutive order. It feels very inelegant, though.

(function($) {
     function animateFadin($delayedElement, delayTime) {
        $delayedElement.delay(delayTime).animate({
            opacity: 1
        }, 700);
    }
    $(document).ready(function(){
        animateFadin($('#zone-branding-wrapper'),100);
        animateFadin($('#zone-menu-wrapper'),500);
        animateFadin($('#zone-content-wrapper'),600);
        $(".view-nodequeue-1 a[href]").delay(600).each(function(index) {
            $(this).delay(200*index).animate({
            opacity: 1
        }, 100);
        });
    });
}) (jQuery);

Is there a way to reuse the named function animateFadin inside the each?

You have written a simple plugin as it is (see:http://learn.jquery.com/plugins/basic-plugin-creation/).

Firstly I don't think I'd put the document ready stuff inside the plugin. Secondly you should remember that jQuery objects are always based on an array (non matched selectors being a zero length array), so you can always use and return the .each function. As a caveat to that, if you are performing the same task on all elements in the jQuery object (and there isn't data bound to them or any calculations that you need to do on a per object basis) you do not need to use .each.

(function( $ ) {

    $.fn.animateFadin= function(amount) {

        return this.each(function() {
             //this is now an individual element you can manipulate
             this.delay(amount).animate({
                 opacity: 1
                 }, 700);
        });

    };

}( jQuery ));

Or in the simple case:

(function( $ ) {

    $.fn.animateFadin= function(amount) {

        return this.delay(amount).animate({
                 opacity: 1
                 }, 700);
    };

}( jQuery ));

You can now call $('#zone-menu-wrapper').animateFadin(500);

If you understand this then you should be able to use the `.each variant to achieve the delays in a marginally more tidy fashion.  What you might want to think about adding though is a callback to the animate function if what you hope to achieve is one element animating after the other.

To do that you would not use each but a simple recursion. Pseudo-code without error/bounds checking:

function chain(this, them) {
    If (!this) return;

    nextThis = them.pop();

    this.delay(whatever).animate({...}, 100, function() {
            chain(nextThis, them);
    });
}

Call this from within the body of animateFadin with the head and [rest] of the jQuery object's object array. What would be nice is to write a plugin to which you pass a function (the action, in your case delay then animate) so that this is reusable behavior. Maybe it already exists?

All code untested and written on my phone, which was harder than I thought! Good luck.


Resize image from StorageFile I have created a function that will take as a parameter a StorageFile and return a WriteableBitmap. I use this data type so that I can later encode the image into a base64 string.

My concern is with the intermediary BitmapImage that I create to get the original dimensions. Any other improvements would be welcomed as well.

Also, it should be noted that this is Windows 10 Universal code, so I am unable to use the System.Drawing libraries.

public static async Task<WriteableBitmap> ScaledWImage(StorageFile file, int maxWidth, int maxHeight)
{
    WriteableBitmap newImage;

    using (var fileStream = await file.OpenReadAsync())
    {
        var bitmap = new BitmapImage();
        bitmap.SetSource(fileStream);
        var origHeight = bitmap.PixelHeight;
        var origWidth = bitmap.PixelWidth;
        var ratioX = maxWidth / (float)origWidth;
        var ratioY = maxHeight / (float)origHeight;
        var ratio = Math.Min(ratioX, ratioY);
        var newHeight = (int)(origHeight * ratio);
        var newWidth = (int)(origWidth * ratio);

        if (ratio > 1)
        {
            newHeight = origHeight;
            newWidth = origWidth;
        }

        newImage = new WriteableBitmap(newWidth, newHeight);
        fileStream.Seek(0);
        var decoder = await BitmapDecoder.CreateAsync(fileStream);

        // Scale image to appropriate size
        var transform = new BitmapTransform()
        {
            ScaledWidth = Convert.ToUInt32(newImage.PixelWidth),
            ScaledHeight = Convert.ToUInt32(newImage.PixelHeight)
        };
        var pixelData = await decoder.GetPixelDataAsync(
            BitmapPixelFormat.Bgra8, // WriteableBitmap uses BGRA format
            BitmapAlphaMode.Straight,
            transform,
            ExifOrientationMode.IgnoreExifOrientation, // This sample ignores Exif orientation
            ColorManagementMode.DoNotColorManage
        );

        // An array containing the decoded image data, which could be modified before being displayed
        var sourcePixels = pixelData.DetachPixelData();

        // Open a stream to copy the image contents to the WriteableBitmap's pixel buffer
        using (var stream = newImage.PixelBuffer.AsStream())
        {
            await stream.WriteAsync(sourcePixels, 0, sourcePixels.Length);
        }
    }

    return newImage;
}

This code is adapted from a Microsoft sample found here.

If you're still looking for a way how to find out the dimensions of a StorageFile image, there is an easy way to do that. I am using it in uwp as well.

I guess it would be less expensive than creation of a new BitmapImage.

        StorageFile image;
        ImageProperties properties = await image.Properties.GetImagePropertiesAsync();
        int height = (int)properties.Height;
        int width = (int)properties.Width;

As well I have this one method which I am using couple of places:

        private async Task<WriteableBitmap> StorageFileToWbmp(StorageFile file)
              {
        ImageProperties props = await file.Properties.GetImagePropertiesAsync();
        WriteableBitmap bitmap = new WriteableBitmap((int) props.Width, (int) props.Height);
        await bitmap.SetSourceAsync(await file.OpenReadAsync());
        return bitmap;
              }


Read lines from a file in chunks I created a function to read lines from an file into chunks.
My hidden agenda in creation this script was in python the yield function in interaction with chunks.
The script works fine, but now i want to know if anyone has improvements?

#!/usr/bin/env perl
use strict;
use warnings;
use Data::Dumper;

sub read_in_chunks {
    my $args = shift;
    my $self = {
        fd => $args->{fd} || undef,
        chunk_size => $args->{chunk_size} || 10,
        chunks => [],
    };
    my $fh = $self->{fd};
    return unless defined(my $line=<$fh>);
    while(<$fh>){
        chomp($_);
        # maybe the following line could be written nicer :)
        ($self->{chunk_size} == 0) ? return $self->{chunks} : (push @{$self->{chunks}}, $_);
        $self->{chunk_size}--;
    }
    return $self->{chunks};
}

open my $fh, 'dump.txt' or die $!;

my $opts = {
    fd => $fh,
    chunk_size => 4
};

while(my $chunk = read_in_chunks($opts)) {
    print Dumper($chunk);
    # process data
}

close $fh;

Here's my take. I removed the $self structure, as it gives you no advantage, and fixed the problem with missing lines.

#! /usr/bin/perl
use warnings;
use strict;

use Data::Dumper;

sub read_in_chunks {
    my %args = @_;
    my $fh = $args{fh} or die "No filehandle given.\n";
    my $size = $args{chunk_size} || 10;
    my @chunk;
    while (@chunk < $size && defined(my $line = <$fh>)) {
        chomp $line;
        push @chunk, $line;
    }
    return @chunk
}

open my $fh, shift or die $!;

my %opts = (
    fh         => $fh,
    chunk_size => 4
);

while (my @chunk = read_in_chunks(%opts)) {
    print Dumper(\@chunk);
    # ...
}


Reordering items which must have alphabetically ordered keys I have a list of objects each with a unique Key and a display name (Label). The list needs to remain sorted by the keys of each object but unfortunately the keys need to be strings rather than numerical and as such 'sorted' here refers to the keys being sorted alphabetically rather than numerically. (Although the keys can be string of numbers - their actual string values don't matter so long as they're alphabetically ordered).

The list should be re-order-able but then the keys need to re-shuffle so that they're once more in alphabetical order.

Currently my implementation allows for the re-ordering of the list and then iterates over all of the objects re-assigning them fresh keys in alphabetical order - something which I feel is a bad idea as it's a lot of effort for a single swap of values.

My object looks like the following

class Item
{
    public string Key { get; set; }
    public string Label { get; set; }

    public Item(string key, string label)
    {
        Key = key;
        Label = label;
    }

    public override string ToString() => Key + ", " + Label;
}

I have added an extension method to handle the re-ordering process

static class Utils
{
    public static List<Item> MoveIndex(this List<Item> myItems, int from, int to)
    {
        Item temp = myItems[from];
        myItems.RemoveAt(from);
        myItems.Insert(to, temp);

        for (int i = 0; i < myItems.Count; i++)
        {
            myItems[i].Key = i.ToString("D2");
        }

        return myItems;
    }
}

And finally I have an example usage of the extension method

static void Main(string[] args)
{
    List<Item> myItems = new List<Item>()
    {
        new Item("0", "Zero"),
        new Item("1", "One"),
        new Item("2", "Two"),
        new Item("3", "Three"),
        new Item("4", "Four"),
        new Item("5", "Five"),
        new Item("6", "Six"),
        new Item("7", "Seven"),
        new Item("8", "Eight"),
        new Item("9", "Nine"),
        new Item("10", "Ten"),
        new Item("11", "Eleven"),
        new Item("12", "Twelve"),
    };

    myItems.MoveIndex(3, 5);

    Console.ReadKey();
}

As the code currently sits, placing a breakpoint on the Console.ReadKey() shows that the Labels are not in their original order but the Keys are - proving that it currently works.

Are there any better ways to manage the Keys when the list is being re-ordered? Perhaps ways which don't require iterating over everything in the list?

Are there any better strings which I could be using for the keys?

I know that as it sits this system has a capacity for only 100 Items before it breaks and that is OK - There will be checking in place to ensure that there are never more than this.

public static List<Item> MoveIndex(this List<Item> myItems, int from, int to)

Code to the interface, not the implementation: you might be able to rewrite this as

    public static IList<Item> MoveIndex(this IList<Item> myItems, int from, int to)

and if not you can almost certainly rewrite it as

    public static TList MoveIndex<TList>(this TList myItems, int from, int to)
        where TList : IList<Item>

        for (int i = 0; i < myItems.Count; i++)
        {
            myItems[i].Key = i.ToString("D2");
        }

For a start, this looks too tightly bound to me. From the description given I would expect MoveIndex to work with the existing keys, whatever they are.

But given the strong assumptions made here, I wonder whether there's a better solution based on eliminating the List<Item> entirely. Can you maintain an IList<string> which contains the labels and then generate the items lazily with IEnumerable<string>.Select(Func<string, int, Item>)?


Find a folder in Java I am making a website which should host a lot of PDFs. These PDFs are stored on the HDD. There are DossierIds in the database which correspondent with the folder name. In these folders there is 1 layer of folders and in those layers are the PDFs.

To make the structure clear:

- ABC
    - Dossier1
        -File1.pdf
        -File2.pdf
    - Dossier2
        -File1.pdf
        -File2.pdf
        -File3.pdf
-DEF
    - Dossier1
        -File1.pdf
    - Dossier2
        -File1.pdf
        -File2.pdf
    - Dossier3
        -File1.pdf

The layer which contains the most folders it the root layer (so in the example: ABC, DEF, etc).

I wrote the following code but I'm not sure if it's optimal and if it can handle concurrent access:

public List<Folder> getFolders(String dossierId) {
    //Going right to the correct folder. Not sure how heavy this operation is.
    File file = new File(String.format("%s\\%s",
        propertyService.getProperties().get("folderLocation"), dossierId));

    //Retrieving all folders in that directory.
    File[] directories = file.listFiles(new FilenameFilter() {
        @Override
        public boolean accept(File dir, String name) {
            return new File(dir, name).isDirectory();
        }
    });

    List<Folder> folders = new ArrayList<>();

    //Going through the folders to check for files
    for (File f : directories) {
        File folder = new File(f.getPath());
        File[] listOfFiles = folder.listFiles();
        List<String> fileNames = new ArrayList<>();
        for (File childfile : listOfFiles) {
            fileNames.add(childfile.getName());
        }

        folders.add(new Folder(f.getName(), fileNames));
    }

    return folders;
}

The code is not optimal. Furthermore, it breaks the single responsibility principle since the method can be broken down to smaller, tighter pieces. For example, the for loop can and should be put in its own method. what this will give you? consider what if in the future you decide to change the folder hierarchy (perhaps add new folder layer or put some files in the root folder etc). what you need is a separate method that can travel along folder tree of any depth and gather the files (perhaps using recurssion?)

Regarding performance:

You call propertyService.getProperties().get("folderLocation") every time the method is called. However, it looks to me like this value is not changed with every call.
String.format is an expensive call since the format is parsed for tokens. Replace that with simple concatanation. and use forward slash - it works for *nix AND Windows systems (the JVM handles it)
You are using pre-java-7 IO library. Java 7 introduced a new, simpler, better, safer, faster library: java.nio. use it.
Each time the method is called, you scan the file system. Depending on the total size of the PDF library - you may consider loading the whole structure into memory. the java.nio has utilities that can alert your program when changes ocur in the file system (like new file or deleted file etc) so you can keep your in-memory tree in sync with the actual file system.
If pre-loading the file system structure is not an option, consider implementing a cache system where you store previous method calls (store an input and its output) and then you can return output for previouisly called input. You will need a cache system like LRU map. There are readily available 3rd party libraries for that.


Quick select the kth smallest value Here is my code for quick select in Python 2.7. I am trying to optimize code for fewer comparisons. And my specific question is marked in code, 3 lines marked with more safe way. Wondering if my code is safe enough to handle all corner cases (in more details, I think more safe way definitely can handle all corner cases, and wondering if my current code which has less comparison is also good to handle all corner cases)? I have tested (see main function) and also tested using median as an example.

Any performance improvement in terms of algorithm time complexity, code bugs or code style are appreciated.

Source code in Python 2.7,

import random
def quick_select(numbers, start, end, k):
    if start == end:
        if k == 0:
            return numbers[start]
        else:
            raise Exception('invalid input!')
    pivot = numbers[end]
    i = start
    j = end - 1
    while i < j: # more safe way: i<=j
        while i < j and numbers[i] <= pivot: # more safe way: while i <= j and numbers[i] <= pivot:
            i += 1
        while numbers[j] > pivot: # more safe way: while i<= j and numbers[j] > pivot
            j -= 1
        if i < j:
            numbers[i], numbers[j] = numbers[j], numbers[i]
    numbers[i], numbers[end] = numbers[end], numbers[i]
    if i - start== k:
        return numbers[i]
    elif i - start > k:
        return quick_select(numbers, start, i-1, k)
    else:
        return quick_select(numbers, i+1, end, k-i+start-1)

def median(numbers):
    if len(numbers) % 2 == 1:
        quick_select(numbers, 0, len(numbers)-1, len(numbers) / 2)
    else:
        x = quick_select(numbers, 0, len(numbers)-1, len(numbers)/2 - 1)
        y = quick_select(numbers, 0, len(numbers) - 1, len(numbers) / 2)
        return (x+y)/2.0

if __name__ == "__main__":
    numbers = []
    for i in range(10):
        numbers.append(random.randint(0,10))
    k = random.randint(0, 9)
    numbers = [7, 5, 2, 3, 7, 2, 8, 7, 4, 9]
    k = 2
    print 'original ', numbers
    print 'k ', k
    print quick_select(numbers, 0, len(numbers)-1, k)
    print 'median', median(numbers)
    print 'sorted ', sorted(numbers)

Handle the empty list correctly

Let’s suppose I try to do a quick select on the empty list: your code throws an IndexError.

>>> from quickselect import quick_select
>>> quick_select([], 0, 0, 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "quickselect.py", line 10, in quick_select
    return numbers[start]
IndexError: list index out of range

It would be better if it returned a useful error message in this case: for example, raising a ValueError if you detect the passed in list is empty:

if not numbers:
    raise ValueError('Cannot perform quickselect on an empty list')

Better bounds checking

The first time I used your code, I had an off-by-one error in the end parameter. I made the following call:

>>> x = [1, 2, 3]
>>> quick_select(x, 0, len(x), 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "quickselect.py", line 15, in quick_select
    j = end - 1
IndexError: list index out of range

Similar calls where you use len(numbers) instead of len(numbers) - 1 will cause different varieties of IndexError. It would be better if this code had a docstring and/or error messages to push me in the right direction – tell me if my start or end variables don’t make sense for the list that I’ve passed in.

Or even better – don’t let me handle them at all. They’re required parameters for the recursive step, but if I’m passing you a list, I probably want to run quick select over the entire list. For that case, make those parameters optional, and pick sensible defaults:

def quick_select(numbers, k, start=None, end=None):
    if start is None:
        start = 0
    if end is None:
        end = len(numbers) - 1

(If those still aren’t the correct defaults, you definitely need a better docstring and error checking.)

Bug: elements are not selected according to their sort order

It’s possible that I’ve misunderstood the algorithm – in which case, please correct me in the comments. Consider the following example:

>>> from quickselect import quick_select
>>> for k in [0, 1, 2, 3]:
...     print(k, quick_select([0, 1, 2, 3], 0, 3, k))
...
0 1
1 0
2 3
3 2

My intuition is that the quickselect result should match the values in the original list. Your code doesn’t seem to be handling that correctly.

I found this bug using Hypothesis to test your code:

from hypothesis import given
from hypothesis.strategies import integers, lists

@given(lists(integers(), min_size=1))
def test_smallest_is_smallest(xs):
    x = quick_select(xs, 0)
    assert x == sorted(xs)[0]

It looks for the smallest element in the list using your algorithm, then compares that to the result if it sorts and takes the first element with sorted(). If they don’t match, it raises an error. That produced the example above.

Hypothesis could do some very powerful testing of this sort of function. (Disclaimer: I’m one of the project owners on GitHub.)

Bug: results are dependent on the initial sort order

Your function shuffles the elements of the list. In theory, this shouldn’t affect the result, right? The smallest element of a list is invariant under shuffling. In practice, it’s a bit different. Compare the following examples:

>>> quick_select([1, 2, 3], 0, 2, 0)
1
>>> quick_select([1, 2, 3], 0, 2, 1)
3

>>> x = [1, 2, 3]
>>> quick_select(x, 0, 2, 0)
1
>>> quick_select(x, 0, 2, 1)
2

I didn’t find this bug with Hypothesis – I was just playing with the example in the previous bug, and stumbled upon it by accident.

Absence of comments

You’ll notice I haven’t actually reviewed any of the code you’ve written. I’m not familiar with the quickselect algorithm, and the Wikipedia page is more complicated than I care to read on a Saturday evening.

There aren’t any comments in your code – this makes it quite difficult to understand. It would be good if there were some comments to explain the purpose of the algorithm – why do we choose this pivot? Why does the algorithm work this way? Why do we know this is correct? etc.

Not only does that make the code much easier to read, review and maintain, it would also make it more likely that you’d spot where the bugs are, because you can see where it deviates from the correct quickselect algorithm.


Mergesorting a stack in Java (See the next iteration.)

I have this small program for sorting a stack using mergesort. Basically, the aim was to use as little aid from JDK library as possible. See what I have:

StackMergesort.java:

import java.util.Collections;
import java.util.Random;
import java.util.Stack;
import java.util.stream.IntStream;

public class StackMergesort {

    public static <E extends Comparable<? super E>>
        void sort(final Stack<E> stack) {
        if (stack.size() < 2) {
            // Trivially sorted.
            return;
        }

        if (stack.size() == 2) {
            final E element1 = stack.pop();
            final E element2 = stack.pop();

            if (element1.compareTo(element2) < 0) {
                stack.push(element2);
                stack.push(element1);
            } else {
                stack.push(element1);
                stack.push(element2);
            }

            return;
        }

        final int size = stack.size();
        final int auxStackLength = size / 2;
        final Stack<E> aux1 = new Stack<>();
        final Stack<E> aux2 = new Stack<>();

        for (int i = 0; i < auxStackLength; ++i) {
            aux1.push(stack.pop());
        }

        while (!stack.isEmpty()) {
            aux2.push(stack.pop());
        }

        sort(aux1);
        sort(aux2);

        Collections.<E>reverse(aux1);
        Collections.<E>reverse(aux2);

        // Merge.
        while (!aux1.isEmpty() && !aux2.isEmpty()) {
            if (aux1.peek().compareTo(aux2.peek()) > 0) {
                stack.push(aux1.pop());
            } else {
                stack.push(aux2.pop());
            }
        }

        while (!aux1.isEmpty()) { stack.push(aux1.pop()); }
        while (!aux2.isEmpty()) { stack.push(aux2.pop()); }
    }

    public static <E extends Comparable<? super E>> boolean isSorted(final Stack<E> stack) {
        if (stack.size() < 2) {
            // Trivially sorted.
            return true;
        }

        E previous = stack.pop();

        while (!stack.isEmpty()) {
            E current = stack.pop();

            if (previous.compareTo(current) > 0) {
                return false;
            }

            previous = current;
        }

        return true;
    }

    public static void main(final String[] args) {
        final long seed = System.nanoTime();
        final Random random = new Random(seed);
        final Stack<Integer> stack = createRandomIntegerStack(20, random);

        // Note: the topmost element of a stack is the rightmost
        // one in the output.
        System.out.println("Seed = " + seed);
        System.out.println("Before: " + stack);
        sort(stack);
        System.out.println("After:  " + stack);
        System.out.println("Sorted: " + isSorted(stack));
    }

    private static Stack<Integer>
        createRandomIntegerStack(final int size, final Random random) {
        final Stack<Integer> stack = new Stack<>();
        IntStream.range(0, size)
                 .forEach((e) -> { stack.add(random.nextInt(100));});
        return stack;
    }
}

Any critique much appreciated.

Not too many things to remark.

I'll review regarding this assumption : Stack is to be viewed as a Stack datastructure, only peek, pop, push, size and isEmpty are to be used. The fact that java.util.Stack is also a List is not to be exploited.

The sort() methods structure falls into three cases size is 1, size is 2, and size is greater than 2. It would help readability to extract the second and third case as separate methods :

public static <E extends Comparable<? super E>> void sort(final Stack<E> stack) {
    if (stack.size() < 2) {
        // Trivially sorted.
        return;
    }

    if (stack.size() == 2) {
        sortSize2Stack(stack);
        return;
    }

    sortLargerStack(stack);
}

For the size two stack case you can avoid popping the second element if it's greater than the first.
These variables are used only once each :

final int size = stack.size();
final int auxStackLength = size / 2;

They should be inlined.
Help from the java libraries could be avoided here by rolling your own reverse method (I guess it just fell outside the focus of the exercise for you)

Collections.<E>reverse(aux1);
Collections.<E>reverse(aux2);

You could merge in reverse order, and call reverse only once, on the merged stack. To me that seems more readable, but it's a judgement call I guess.


Dynamic IP-address logging Inspired by Don Stewart's article on Scripting with Haskell I tried a rewrite of a shell-script I did the other day. I was really astonished how easily it worked but as a whole it is about double the size of my shell script and some things feel a bit "clumsy".

myReadTile and getCurlIP have IO-return type, which seems a bit "unhaskellish"
extractIp and extractFromFile seem a bit unelegant especially the maybe parts and the trivial case "" to prevent last from throwing an error.

To run my code the packages curl network-info had to be installed - the rest was already there (Debian haskell-platform 2012.2.0.0, ghc-7.4.1)

What my code does:
I want to log the IP-address my provider gives me - and when it changes; as I have a dynamic IP.  With this information I can ssh from other devices to my home PC, and I know how long the periods are in which my IP doesn't change.

Code

import System.IO         (readFile, appendFile)
import System.Exit       (exitSuccess)
import System.Directory  (doesFileExist, getHomeDirectory)
import Data.Maybe        (listToMaybe, fromMaybe)
import Data.Time         (getZonedTime)
import Network.BSD       (getHostName)
import Network.Curl      (curlGetString, URLString, CurlOption(CurlTimeout))
import Network.Curl.Code (CurlCode(CurlOK))
import Network.Info      (getNetworkInterfaces, NetworkInterface, name, ipv4)

localFilePath :: FilePath
localFilePath = "/Dropbox/iplog.log"

dyndns :: URLString
dyndns = "http://checkip.dyndns.org"

main :: IO ()
main = do
    home <- getHomeDirectory
    let iplogFilePath = home ++ localFilePath
    iplogS <- myReadFile iplogFilePath
    curlS <- getCurlIP dyndns
    let oldIP     = extractFromFile iplogS
        currentIP = extractIp curlS
    if oldIP /= currentIP
      then do
        date <- fmap show getZonedTime
        host <- getHostName
        localIP <- fmap getLocalIP getNetworkInterfaces
        let content = unwords [currentIP, date, host, localIP, "\n"]
        appendFile iplogFilePath content
      else
        exitSuccess

extractFromFile :: String -> String
extractFromFile "" = ""
extractFromFile s = (fromMaybe [] . listToMaybe . words . last . lines) s

extractIp :: String -> String
extractIp "" = ""
extractIp s = (takeWhile (/='<') . last . words) s

myReadFile :: FilePath -> IO String
myReadFile fp = do
    iplogExists <- doesFileExist fp
    if iplogExists
      then
          readFile fp
      else do
          print "File does not exist - and will be created"
          print ("at: ~" ++ localFilePath)
          return ""

getCurlIP :: URLString -> IO String
getCurlIP url = do
    (curlState, curlString) <- curlGetString url [CurlTimeout 60]
    if curlState == CurlOK
      then
          return curlString
      else do
          print "No external IP found"
          return "0.0.0.0"

getLocalIP :: [NetworkInterface] -> String
getLocalIP = show . ipv4 . head . filter (\x -> name x == "eth0")

You have produced rather nice code, so my review is restricted to just the peripherals.

import System.IO         (readFile, appendFile)
import System.Exit       (exitSuccess)
import System.Directory  (doesFileExist, getHomeDirectory)
import Data.Time         (getZonedTime)
import Data.Char         (isDigit, isSpace)
import Network.BSD       (getHostName)
import Network.Curl      (curlGetString, URLString, CurlOption(CurlTimeout))
import Network.Curl.Code (CurlCode(CurlOK))
import Network.Info      (getNetworkInterfaces, NetworkInterface, name, ipv4)
import Control.Applicative (<$>, <*>)

localFilePath :: FilePath
localFilePath = "/myhaskell/iplog.log"

dyndns :: URLString
dyndns = "http://checkip.dyndns.org"

myEth = "eth0"

It is nicer to separate out self contained functions even if they are of type IO a (You already have some, but take out as much as you can.)

getLogPath :: IO String
getLogPath = flip (++) localFilePath <$> getHomeDirectory

The warning that there are no external ips should really be elsewhere.

getCurlIP :: URLString -> IO String
getCurlIP url = extract <$> curlGetString url [CurlTimeout 60]
  where extract (CurlOK, str) = str
        extract _ = "0.0.0.0"

getLocalIP :: [NetworkInterface] -> String
getLocalIP = show . ipv4 . head . filter (flip (==) myEth . name)

Your ip extraction can be simplified further.

firstWord :: String -> String
firstWord = takeWhile (not . isSpace)

getFileIP = firstWord

I modified your implementation a bit to make it clear what is happening. We take letters while they are not numbers, and drop the suffix that start with '<'
If your parsing goes beyond this, then go for parsec.

-- "<title>Current IP Check</title>
--  Current IP Address: 24.20.128.212\r\n"
extractIp :: String -> String
extractIp = takeWhile (/= '<') . dropWhile (not . isDigit)

Readfile shouldn't really say that a file if it does not exist will be created. It should just read the given file and return the value. The check and warning should be done elsewhere.

catFile :: FilePath -> IO String
catFile fp = doesFileExist fp >>= checkExist
  where checkExist True = readFile fp
        checkExist False = return []

checkS s [] = do
  print ("File does not exist - and will be created\n" ++ "at: ~" ++ s)
  return []
checkS s str = return str

Using auxiliary definitions can make your code read much better. Prefer sequence to do notation when values are not used in intermediate computations.
Try to move out of the imperative mindset when using do notation. (the do notation makes it easy to write 'c' in haskell :) ), using auxiliary functions can help you there. (I used case instead of if/then because that is what I prefer, there is no particular reason for that except that it makes it easy to use destructuring easier if I need to.)

main :: IO ()
main = do
    currentIP <- getCurrentIP
    iplogFilePath <- getLogPath
    oldIP <- getSavedIP iplogFilePath
    checkS iplogFilePath oldIP
    case oldIP /= currentIP of
      True -> getContent currentIP >>= appendFile iplogFilePath
      _ -> exitSuccess
  where getContent ip = unwords <$> sequence [return ip, show <$> getZonedTime, getHostName,
                                  getLocalIP <$> getNetworkInterfaces]
        getSavedIP path = getFileIP <$> catFile path
        getCurrentIP = extractIp <$> getCurlIP dyndns


Rock, Paper, Scissors Game in Python This is the biggest program I have ever made. I created a Rock Paper Scissors game. It asks you if you want to play, takes your input, compares it with the random computer input, gives you the outcome, and then asks if you want to play again.

I am POSITIVE that I could have done this in fewer lines of code, so I would absolutely appreciate feedback. I'm brand new to Python. I am currently using Python 3.

import random
import sys
#begin the game and then loop after the first play.

def play():
    while True:
        p_choice = input("What do you choose?")
        cpu_random = random.randint(1,3)
        cpu_choice = cpu_random
        if cpu_random == 1:
            cpu_choice = "Rock"
        elif cpu_random == 2:
            cpu_choice = "Paper"
        elif cpu_random == 3:
            cpu_choice = "Scissors"

#Compare the data given by the user to the CPU

        def compare():

            play_again = None
#Tie outcome
            if p_choice == cpu_choice:
                print("Tie!")
                play_again = input("Play again?")

#Rock outcome

            elif p_choice == "Rock" and cpu_choice == "Paper":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Rock" and cpu_choice == "Scissors":
                print("You Win!")
                play_again = input("Play again?")

#Paper outcome

            elif p_choice == "Paper" and cpu_choice == "Scissors":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Paper" and cpu_choice == "Rock":
                print("You Win!")
                play_again = input("Play again?")

#Scissors outcome

            elif p_choice == "Scissors" and cpu_choice == "Rock":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Scissors" and cpu_choice == "Paper":
                print("You Win!")
                play_again = input("Play again?")

#Ask if you want to play again, then give input

            if play_again == "Yes":
                play()
            elif play_again == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                play_again = input("play again?")
                return play_again

        compare()

#ask if player wants to start
def game_start():
    while True:
        begin = input("Would you like to play Rock, Paper, Scissors?")
        if begin == "Yes":
            play()
            return begin
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                return begin
            else:
                print("Please try again")
                break

game_start()

I would use a dictionary to store the choices, and pass the input through capitalize() so, the user doesn't have to write the capitalized word(i am doing this only because you have it that way, but unless you are printing the word as it is, i recommend you to use only lowercase or uppercase to avoid unmatching), also, the while loop is unnecesary because you are calling play() if the user wants to play again.

def play():
    p_choice = input("What do you choose?").capitalize()
    choices = {1 : 'Rock', 2 : 'Paper', 3 : 'Scissors'}
    cpu_choice = choices[random.randint(1,3)]

then, you can do the same thing for all cases, instead of all those conditionals, you can build a dictionary for all cases and then let the compare function return True if its a win or False if its a lose, although i wouldn't recommend you to define the function inside play()

def compare(playerChoice,cpuChoice):
    results = {('Paper','Rock') : True,
               ('Paper','Scissors') : False,
               ('Rock','Paper') : False,
               ('Rock','Scissors') : True,
               ('Scissors','Paper') : True,
               ('Scissors','Rock') : False}
    return results[(playerChoice,cpuChoice)]

and this is my version of game_start()

def game_start():
    begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    while begin != "Yes":
        if begin == "No":
            print("Game Over")
            return sys.exit()
        else:
            print("Please try again")
            begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    play()
    while True:
        begin = input('Play again?').capitalize()
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                begin = input("Play again? ").capitalize()
        play()

the whole code looks now like this:

import random
import sys

def play():
    p_choice = input("What do you choose?").capitalize()
    choices = {1 : 'Rock', 2 : 'Paper', 3 : 'Scissors'}
    cpu_choice = choices[random.randint(1,3)]
    if p_choice == cpu_choice:
        return print('Tie!')
    if compare(p_choice,cpu_choice):
        return print('You Win!')
    else:
        return print('You Lose!')

def compare(playerChoice,cpuChoice):
    results = {('Paper','Rock') : True,
               ('Paper','Scissors') : False,
               ('Rock','Paper') : False,
               ('Rock','Scissors') : True,
               ('Scissors','Paper') : True,
               ('Scissors','Rock') : False}
    return results[(playerChoice,cpuChoice)]

def game_start():
    begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    while begin != "Yes":
        if begin == "No":
            print("Game Over")
            sys.exit()
        else:
            print("Please try again")
            begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    play()
    while True:
        begin = input('Play again?').capitalize()
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                begin = input("Play again? ").capitalize()
        play()

game_start()


Plain C implementation of 'Cumulative All' operation for R I want to implement a cumulative all through R's C interface. I was able to hack something together that produces the correct results, but I have little idea what idiomatic C should look like. The code should be easy to understand, even if you don't know about R.

Description

The cumulative all takes a LGLSXP (logical S-expression) input. The function should output a LGLSXP of the same length as the input, where each element represents whether all values up to this point were true.

The three valued logic works like this:
(true & false) -> false, (true & na) -> na, (false & na) -> false

LGLSXP

A LGLSXP is a pointer to a struct that contains an int array that represents 3 valued logic with 0 as false, INT_MIN as NA and 1 as true. This array can be accessed with LOGICAL(). You can read about SEXPs here.

Code

#include <R.h>
#include <Rinternals.h>

SEXP cumall_impl(SEXP x) {
  SEXP res = PROTECT(allocVector(LGLSXP, XLENGTH(x)));
  R_xlen_t n = XLENGTH(x);

  if (n == 0){
    UNPROTECT(1);
    return(res);
  } else {
    memset(LOGICAL(res), 0, n * sizeof(int));
  }

  int *p_x = LOGICAL(x);
  int *p_res = LOGICAL(res);
  p_res[0] = p_x[0];

  for (R_xlen_t i = 1; i < n; i++) {
    if (p_x[i] == TRUE)
      p_res[i] = p_res[i - 1];
    else if (p_x[i] == NA_LOGICAL)
      p_res[i] = NA_LOGICAL;
    else
      break;
  }

  UNPROTECT(1);
  return res;
}

Remarks

PROTECT() and UNPROTECT() are necessary to prevent R's garbage collector from interfering
LOGICAL() accesses the underlying array of a LGLSXP
TRUE, NA_LOGICAL etc are provided by the headers.

Headers

The most important definitions are in Rinternals.h and R.h.
The NA_LOGICAL is defined in Arith.h and arithmetic.c
I am not sure where/how TRUE and FALSE are really defined. Likely sources are Boolean.h and Rdefines.h

I am thankful for all suggestions, be it coding style, variable naming or critique to how I formulated my question

I see some things that may help you improve your code.

Use variables effectively

The code begins with this:

SEXP cumall_impl(SEXP x) {
    SEXP res = PROTECT(allocVector(LGLSXP, XLENGTH(x)));
    R_xlen_t n = XLENGTH(x);

However, there's not really any reason not to define and use n first:

SEXP cumall_impl(SEXP x) {
    R_xlen_t n = XLENGTH(x);
    SEXP res = PROTECT(allocVector(LGLSXP, n));

Use all required #includes

The code uses memset and so it should also have this line:

#include <string.h>

Even though it happens to compile because some of the R libraries probably also include that, it's important to make sure that the code does not rely on undocumented interface assumptions.

Use a single loop over all elements

The code uses memset to initialize all values to FALSE and then sets the first value explicitly and then loops through all other values.  I'd be inclined instead to recode that as a single for loop instead:

int prev = TRUE;
for (R_xlen_t i = 0; i < n; ++i) {
    if (p_x[i] == TRUE)
        p_res[i] = prev;
    else if (p_x[i] == NA_LOGICAL)
        p_res[i] = NA_LOGICAL;
    else
        p_res[i] = FALSE;
    prev = p_res[i];
}

Writing it this way avoids the need for the if (n == 0) construct and also eliminates the need for the memset.

Note: Ordinarily, we'd prefer to use a switch there instead of a chained if..else but we're prevented from doing so because NA_LOGICAL does not evaluate to an integer constant the way it's defined in R.


Return array of objects with matching keys For an exercise, I've written a function that, given an array of objects and a key (2nd argument), returns a new array of objects that contain all properties from a key. How'd I do?

function where(collection, source) {
  var arr = [];
  var keys = Object.keys(source);
  var countMatchingProps = 0;
  var currentProp;

  for (var i = 0; i < collection.length; i++) {
    countMatchingProps = 0;
    for(var j = 0; j < keys.length; j++){
      // assigned to variable for a bit of readability.
      currentProp = keys[j];
      // if object contains key ->
      if(collection[i].hasOwnProperty(currentProp)){
        // -> then compare their values nad increment counter
        if(collection[i][currentProp] === source[currentProp]){
          countMatchingProps++;
        }
      }
      // if number of matched properties are
      // equal to keys we can push current object to array
      if (countMatchingProps === keys.length) arr.push(collection[i]);
    }
  }
  return arr;
}

where([{ "a": 1 }, { "a": 1 }, { "a": 1, "b": 3 }], { "a": 1});
where([{ "a": 1, "b": 2 }, { "a": 1 }, { "a": 1, "b": 2, "c": 2 }], { "a": 1, "b": 2 })

Better readability could be achieved using array.filter, since it creates the array for you and all you have to do is return true or false. No need to create an array, do the comparison and push yourself.

In the same way when checking for the values, Object.keys can be used with array.every to iterate through your constraints and see if each of the current item's keys match the constraint.

Also, I wouldn't call it source. It's not a source of anything. It's more of a "constraint" for your collection. So I'd call it that way instead.

In terms of performance, array iteration functions are slower than your average for loop (older APIs will tend to be more optimized). However, in terms of readability, these array APIs really shorten your code.

function where(collection, constraint){
  // filter creates an array of items whose callback returns true for them
  return collection.filter(function(collectionItem){
    // every returns true when every item's callback returns true
    return Object.keys(constraint).every(function(key){
      return collectionItem.hasOwnProperty(key) && constraint[key] === collectionItem[key];
    });
  });
}

var a = where([{ "a": 1 }, { "a": 1 }, { "a": 1, "b": 3 }], { "a": 1});
var b = where([{ "a": 1, "b": 2 }, { "a": 1 }, { "a": 1, "b": 2, "c": 2 }], { "a": 1, "b": 2 })

document.write(JSON.stringify(a));
document.write('
');
document.write(JSON.stringify(b));

The code can further be simplified by taking advantage of ES6 arrow functions. This removes the brackets (with one arg, the parens are optional), and the body can be an expression which would then be an implicit return value, eliminating return.

function where(collection, constraint){
  return collection.filter(collectionItem =>
    Object.keys(constraint).every(key =>
      collectionItem.hasOwnProperty(key) && constraint[key] === collectionItem[key]));
}

var a = where([{ "a": 1 }, { "a": 1 }, { "a": 1, "b": 3 }], { "a": 1});
var b = where([{ "a": 1, "b": 2 }, { "a": 1 }, { "a": 1, "b": 2, "c": 2 }], { "a": 1, "b": 2 })

document.write(JSON.stringify(a));
document.write('
');
document.write(JSON.stringify(b));


Reading from text files in C/C++ I'm writing a program that has to check which are the two largest three digit numbers that are missing from a text file. I'm writing this in C++ and I'm trying to solve the mystery in a while loop. How can I "move" the search back to the beggining of the text file? More explicit: if the checking is now around the middle of the text file and I have to check at this point of time again all the numbers from the beggining of the file, how can I do this? I'm attaching only the code part which contains the while.

    while(!in.eof()&&first_number>=100&&second_number>=100)
    {   in >> number_to_compare;

        if(first_number==number_to_compare)
        {   first_number--;
            if (first_number == second_number)
                first_number--;
        }
        else if(second_number==number_to_compare)
        {   second_number--;
            if (second_number == first_number)
                second_number--;
        }
    }

I would need in bouth of the mainif operators cases a line code that makes the search to start again from the beginning of the text file. I'm just starting to learn programming, I don't know any OOP.
I tried also something similar with the logic of bubble sort algorithm (loop in loop) but I think that it's not working because the file reading will not start again from the beginning of the file for a new iteration of the main while ... so it's almost the same problem ... how can I make the program to start again to read the text file from the beginning? Here is the second attempt:

int need_to_search = 1;

    while (need_to_search)
    {   need_to_search = 0;

        while (!in.eof() && first_number >= 100 && second_number >= 100)
        {   in >> number_to_compare;

            if (first_number == number_to_compare)
            {   first_number--;
                if (first_number == second_number)
                    first_number--;

                need_to_search = 1;
            }
            else if (second_number == number_to_compare)
            {   second_number--;
                if (second_number == first_number)
                    second_number--;

                need_to_search = 1;
            }
        }
    }

Example: If the file would contain the folowing numbers:

  1292 5422 997 125 7735 1 5474 956 7557 1249 966 624 12 5499 5677 884 964 998 546 455 778 6658 957

The output should be:

  999 996

because these are the largest two numbers of three digits that are missing from the text file.

I would really appreciate a hint! Thanks!

If you're interested in learning OOP and don't have the ambition to work with embedded electronics (roughly everything smaller than a phone), you're really more interested in C++ than C. This review is written based on the assumption you want to write C++ and ignores C.

Your current method of reading a file is odd. While checking for EOF is a relatively easy method to check whether you've hit the end of a file yet, it's not the only thing you should check for when reading a file. C++ can check whether the input is still 'good'. You could do so like this:

#include <iostream>
#include <string>
#include <fstream>
#include <vector>

void readFile()
{
    std::ifstream input("file.txt", std::ios::in);
    std::string currentLine;
    std::vector<std::string> allLines;
    std::vector<std::string>::iterator itLines;
    while (input.good())
    {
        getline(input, currentLine);
        allLines.push_back(currentLine);
    }
    for (itLines = allLines.begin(); itLines != allLines.end(); itLines++)
    {
        // Iterate over the data you just read
    }

}

Note that since we don't return anything, a void will do.

Now you're reading all lines of data (up till every newline) and iterate over them, which appears to be what you want to do. Not only will this stop when the file has reached EOF, it will also check whether the stream hasn't failed or took a bad chunk somewhere along the way.


Custom Exceptions based off the Visual Studio snippet I'm creating my own application exception types so I could catch them higher in the call stack and take specific action. I'm looking to get feedback on the latest exception I've written for two reasons:

I'm using the Exception snippet from Visual Studio 2013, and am wondering if it is any good and/or has potential problems;
The main parameter for this Exception type would be a string, and I'm wondering what to do with the parameters. Currently I'm following the built in ArgumentException constructor-signatures-wise, but I fear future users of my exception will fall into the same trap as me with ArgumentException: providing a single argument with the paramName, instead of a message.

PS. I've checked the source for ArgumentException and explicitly decided not to put in the extra work of saving the tenantName in a field + property analogously, because the calling code does not yet need it. (Though I'm happy to get feedback on that choice as well of course.)

Bottom line: anything to improve on this custom exception?

[Serializable]
public class TenantNotFoundException : Exception
{
    public TenantNotFoundException()
    { }

    public TenantNotFoundException(string message)
        : base(message)
    { }

    public TenantNotFoundException(string message, string tenantName)
        : base(string.Format("Tenant '{0}' not found. {1}", tenantName, message))
    { }

    public TenantNotFoundException(string message, Exception inner)
        : base(message, inner)
    { }

    public TenantNotFoundException(string message, string tenantName, Exception inner)
        : base(string.Format("Tenant '{0}' not found.", tenantName), inner)
    { }

    protected TenantNotFoundException(SerializationInfo info, StreamingContext context)
        : base(info, context)
    { }
}

PS. I was almost tempted to write just a single constructor with a string parameter for tenant name, but then I wondered (a) if people would be confused and enter a message instead of a tenant name, and (b) why the snippet suggests all the other constructors. This is what prompted me to post this question.

For reference, this is what you start with as the Visual Studio snippet:

[Serializable]
public class MyException : Exception
{
    public MyException() { }
    public MyException(string message) : base(message) { }
    public MyException(string message, Exception inner) : base(message, inner) { }
    protected MyException(
      SerializationInfo info,
      StreamingContext context)
        : base(info, context) { }
}

Using strongly typed data avoids this situation.

One easy and simple way to fix this argument confusion would be passing the user defined datatype instead of string which is reserved for message.

In TenantException sample, if you have Object model for Tenant, pass that object and let the constructor does the remaining with that.

[Serializable]
public class TenantNotFoundException : Exception
{
    public TenantNotFoundException()
    { }

    public TenantNotFoundException(string message)
        : base(message)
    { }

    public TenantNotFoundException(string message, Tenant tenant)
        : base(string.Format("Tenant '{0}' not found. {1}", tenant.Name, message))
    { }
}


Ruby backup/restore for provisioning This is my second Ruby coding attempt. Also - this is my first OOP usage in Ruby.

I'm using if __FILE__ == $0 form here - same as it is in Python because it's really comfortably as for me. Is there is any objections/restrictions for such usage?

Any other tips/links about this code appreciated as well.

Script intended to create Consul raft-database backup before run destroy.rb from my previous review, or restore database after provisioning.

#!/usr/bin/env ruby

require 'net/ssh'

$me = File.basename(__FILE__)

class CheckIn

    USER = 'knife';
    RSA_KEY = 'ssh/id_rsa';

    def initialize(host)

        @@host = host;

        if File.exist?(RSA_KEY) then
            File.chmod(0600, RSA_KEY);
        else
            abort "[#{$me}] ERROR: no RSA file #{RSA_KEY} found. Exit.\n";
        end

    end
end

class BackupRestore < CheckIn

    def backup()

        _consul_raft_db = '/opt/consul/data/raft/raft.db'
        _root_backup_dir = '/root/vaul_devops_kdbx/backups'
        _raft_backup_file = "#{Time.now.strftime("%H_%M_%S_%d_%m_%y")}_bkp_raft.db"

        Net::SSH.start(@@host, USER, :keys => RSA_KEY) do |ssh|

            backuped = ssh.exec!("

            if test -e #{_consul_raft_db}; then
                echo \"\"
                echo \"OK: Consul DB found - #{_consul_raft_db}.\"
                echo \"\"
            else
                echo \"ERROR: no #{_consul_raft_db} file found. Exit.\"
                exit 1
            fi

            if sudo [ -d #{_root_backup_dir} ]; then
                echo \"OK: #{_root_backup_dir} found.\"
                echo \"\"
            else
                echo \"ERROR: no #{_root_backup_dir} directory found. Exit.\"
                exit 1
            fi

            if sudo cp #{_consul_raft_db} #{_root_backup_dir}/#{_raft_backup_file}; then
                echo \"OK: backup created:\"
                echo \"\"
                sudo ls -l #{_root_backup_dir}/#{_raft_backup_file}
                echo \"\"
                echo \"All backups present on the #{@@host} in the #{_root_backup_dir}:\"
                echo \"\"
                sudo ls -l #{_root_backup_dir}
                echo \"\"
            else
                echo \"\"
                echo \"ERROR: can not complete backup from #{_consul_raft_db} to #{_root_backup_dir}/#{_raft_backup_file}. Exit.\"
                exit 1
            fi

            ")

            puts backuped

        end
    end

    def restore()

        puts "\nRestore will be here\n\n"

    end

end

if __FILE__ == $0

    puts "\n[#{$me}] Consul backup/restore started.\n"

    if ARGV.length == 2 and (ARGV[1] == 'backup' or ARGV[1] == 'restore')
        _host = ARGV[0]
        _mode = ARGV[1]

        puts "\n[#{$me}] Working on the #{_host} in #{_mode} mode.";

        run = BackupRestore.new(_host)
        run.send(_mode)
    else
        abort "\n[#{$me}] ERROR: HOST must be specified as first argument and MODE (backup or restore) - as second one. Exit..\n"
    end

    puts "[#{$me}] Consul backup/restore finished.\n\n"

end

And it's execution - backup:

$ ./scripts/backup.rb setevoy.vault.local backup

[backup.rb] Consul backup/restore started.

[backup.rb] Working on the setevoy.vault.local in backup mode.

OK: Consul DB found - /opt/consul/data/raft/raft.db.

OK: /root/vaul_devops_kdbx/backups found.

OK: backup created:

-rw------- 1 root root 2097152 Feb 17 16:11 /root/vaul_devops_kdbx/backups/16_11_57_17_02_16_bkp_raft.db

All backups present on the setevoy.vault.local in the /root/vaul_devops_kdbx/backups:

total 20248
-rw------- 1 root root 1048576 Feb 17 14:21 14_21_50_17_02_16_bkp_raft.db
[...[
-rw------- 1 root root 2097152 Feb 17 16:11 16_11_57_17_02_16_bkp_raft.db

[backup.rb] Consul backup/restore finished.

and "restore":

$ ./scripts/backup.rb setevoy.vault.local restore

[backup.rb] Consul backup/restore started.

[backup.rb] Working on the setevoy.vault.local in restore mode.

Restore will be here

[backup.rb] Consul backup/restore finished.

Unfortunately, I don't have time for a proper review, so I'll stick to the more obvious points:

Indentation in Ruby is 2 spaces, not 4.
Not only is your use of semicolons inconsistent (sometimes you use them, sometimes you don't), every single one of those you do have is superfluous. Leave them out. You can use either newline or semicolon as "statement" separator, using both is not necessary.
The same applies to your usage of the then keyword: sometimes you use it, sometimes you don't, and all uses are superfluous. Leave them out. You can use either the then keyword or a "statement" separator (newline or semicolon) to separate the condition from the then body, using both is not necessary.
There is a large number of superfluous newlines. You'd generally have one before a new class, module or method definition, but not within – if your method is so long that it needs to be broken down in sections, consider refactoring it.
You can save yourself a lot of trouble escaping all the double quotes in the giant string by using either some other delimiter (e.g. backuped = ssh.exec!(%Q[ … ]) or a heredoc.
Leave out empty parameter lists, e.g. def backup instead of def backup().
By convention, variable names that start with an underscore _ are used for variables that are ignored. So, you would really only use them for multiple assignment or unused block parameters. In fact, it is more than just a convention: Ruby will warn you about unused local variables, but these warnings are suppressed for variables starting with an underscore, because they are intended as "ignored" variables.
You should prefer public_send over send. The latter circumvents access protection, the former honors it.
Prefer &&/|| over and/or. The former actually have the precedence you would expect from them, the latter don't.
For the string inside the interpolation within your raft_backup_file string, you could use single quotes instead of double quotes. Personally, I always use single quotes unless I actually need to do interpolation or use escape sequences (or there is a single quote in the string). This makes it clear just when looking at the beginning of the string whether or not there is going to be interpolation.

Note that since you use a class variable to store part of your state (@@host), it is impossible to create more than one instance of your class, so it doesn't make sense to have a class at all.

#!/usr/bin/env ruby

require 'net/ssh'

$me = File.basename(__FILE__)

class CheckIn
  USER = 'knife'
  RSA_KEY = 'ssh/id_rsa'

  def initialize(host)
    @@host = host

    if File.exist?(RSA_KEY)
      File.chmod(0600, RSA_KEY)
    else
      abort "[#$me] ERROR: no RSA file #{RSA_KEY} found. Exit.\n"
    end
  end
end

class BackupRestore < CheckIn
  def backup
    consul_raft_db = '/opt/consul/data/raft/raft.db'
    root_backup_dir = '/root/vaul_devops_kdbx/backups'
    raft_backup_file = "#{Time.now.strftime('%H_%M_%S_%d_%m_%y')}_bkp_raft.db"

    Net::SSH.start(@@host, USER, keys: RSA_KEY) do |ssh|
      backuped = ssh.exec!(%Q[
        if test -e #{consul_raft_db}; then
            echo ""
            echo "OK: Consul DB found - #{consul_raft_db}."
            echo ""
        else
            echo "ERROR: no #{consul_raft_db} file found. Exit."
            exit 1
        fi

        if sudo [ -d #{root_backup_dir} ]; then
            echo "OK: #{root_backup_dir} found."
            echo ""
        else
            echo "ERROR: no #{root_backup_dir} directory found. Exit."
            exit 1
        fi

        if sudo cp #{consul_raft_db} #{root_backup_dir}/#{raft_backup_file}; then
            echo "OK: backup created:"
            echo ""
            sudo ls -l #{root_backup_dir}/#{raft_backup_file}
            echo ""
            echo "All backups present on the #@@host in the #{root_backup_dir}:"
            echo ""
            sudo ls -l #{root_backup_dir}
            echo ""
        else
            echo ""
            echo "ERROR: can not complete backup from #{consul_raft_db} to #{root_backup_dir}/#{raft_backup_file}. Exit."
            exit 1
        fi
      ])

      puts backuped
    end
  end

  def restore
    puts "\nRestore will be here\n\n"
  end
end

if __FILE__ == $0
  puts "\n[#$me] Consul backup/restore started.\n"

  if ARGV.length == 2 && (ARGV[1] == 'backup' || ARGV[1] == 'restore')
    host = ARGV[0]
    mode = ARGV[1]

    puts "\n[#$me] Working on the #{host} in #{mode} mode."

    run = BackupRestore.new(host)
    run.public_send(mode)
  else
    abort "\n[#$me] ERROR: HOST must be specified as first argument and MODE (backup or restore) - as second one. Exit..\n"
  end

  puts "[#$me] Consul backup/restore finished.\n\n"
end


Randomly generate a list with predetermined mean Aim: randomly generate \$n\$ numbers between \$a\$ and \$b\$ with (roughly) mean \$m\$.

Problem: The simple beginner's code I wrote becomes very inefficient as \$m\$ moves away from \$\frac{a + b}{2}\$.

import random
import numpy as np
import time

low = 0
high = 100

data = 50

def lst_w_mean(target):
    HT = [x for x in range(low, high+1)]
    target_mean = 0

    while target_mean <> int(target):
        outcomes = [random.choice(HT) for x in range(data)]
        target_mean = np.mean(outcomes)
    return outcomes

t1 = time.clock()
data_new = lst_w_mean(54)
print "elapsed time: ", time.clock() - t1
print data_new

t1 = time.clock()
data_new = lst_w_mean(32)
print "elapsed time: ", time.clock() - t1
print data_new

Works pretty decently for means close to the the mean of HT (e.g., means between between 40 and 60). Takes for ever (without generating a run-time error) beyond that narrow range (roughly 28 minutes for mean =30)

%run
elapsed time for mean = 54:  0.00734800000009
[46, 82, 6, 0, 90, 73, 99, 2, 34, 88, 51, 14, 89, 72, 40, 8, 97, 44, 46, 45, 89, 10, 22, 52, 96, 98, 43, 52, 59, 74, 52, 54, 64, 66, 21, 71, 92, 34, 76, 33, 26, 36, 53, 74, 64, 85, 59, 26, 69, 24]

elapsed time for mean = 30:  1695.210358
[67, 43, 30, 78, 67, 33, 13, 24, 27, 1, 2, 4, 4, 2, 49, 1, 41, 16, 6, 9, 6, 90, 1, 31, 52, 91, 5, 11, 4, 94, 2, 74, 60, 38, 8, 2, 10, 20, 92, 25, 17, 36, 12, 9, 7, 22, 16, 72, 44, 32]

Reformulating HT with something like:

if target < 40:
 high = 70

etc., so that the target mean is closer to the mean of HT does not generate realistic data besides being an ad hoc solution.  And yes, I would like what I call "realistic data" to have a "high" standard deviation perhaps with a number of "outliers".

The process speeds up significantly as the value of data diminishes, so maybe the better approach is to try to use recursion with smaller data values (sort of like in merge-sort)?

Review

The official Python style-guide recommends using lower_case for variables (your variable HT violates this).

I would make all relevant parameters actual parameters of the function. This makes it a lot easier to test.

The main code of a module should be wrapped in a if __name__ == "__main__": guard. This allows you to do in another script:

from random_with_mean import lst_w_mean

without executing the tests.

I would try to find a different name for the function, without too many abbreviations.

Alternative approach

I would use a completely different approach to this.

First, note that the Poisson distribution is a positive-semidefinite distribution for integers. If we want to generate a integers in the range (0, 100) (similar to your example, but note the off-by-one error), with mean 50, We can just take a np.random.poisson(50) (which I will call P(50) from now on), ensuring to throw away all values which are larger than 100 and redraw them.

This also works when generating values in the range (0, 100) but with mean 10. Only when the mean becomes larger than the half-way point do we start getting into trouble. Suddenly we are starting to loose a larger and larger part of the tail to the upper boundary. To avoid this, we can just swap the problem around and declare the upper boundary to be zero and generate the values as high - P(high - target), so we generate how far the value is away from the boundary.

Similarly, when the lower bound is not zero, we need to shift the values upwards and generate low + P(target - low).

An implementation of this is:

import numpy as np

def lst_w_mean(low, high, num, target):
    swap = target > (high - low) / 2
    out = []
    to_generate = num
    while to_generate:
        if swap:
            x = high - np.random.poisson(high - target, size=to_generate)
            x = x[low <= x]  # x can't be larger than high by construction
        else:
            x = low + np.random.poisson(target - low, size=to_generate)
            x = x[x < high]  # x can't be smaller than low by construction
        out.extend(x)
        to_generate = num - len(out)
    return out

if __name__ == "__main__":
    print np.mean(lst_w_mean(0, 100, 50, 50))
    print np.mean(lst_w_mean(0, 100, 50, 10))
    print np.mean(lst_w_mean(0, 100, 50, 70))

Note that I always generate multiple values (enough to fill the out list if all were inside the boundaries) at the same time, then throw away all values outside of the boundaries and loop until the out list is the right size. Most of the time only one iteration is needed, because it can only go outside of the boundary on the side away from the closer boundary.

Note that the variance of the Poisson distribution is the same as its mean and so its standard deviation is the square root of that. This means that you will get some outliers, but the distribution is far from uniform over your range.

To visualize this, here are three different samples (with n=5000), one with mean 10, 50, 75 and 99 each using the Poisson distribution:

As an alternative, you could take the Beta distribution, which is only defined on the interval [0, 1], which helps us a lot here, because we don't need to take care of edge effects, we only need to shift and rescale our values. Given a mean and sigma (I took sigma to be an arbitrary value), we can derive the necessary parameters \$\alpha\$ and \$\beta\$, as given as alternative parametrization on wikipedia:

\$\alpha = \mu \left(\frac{\mu(1 - \mu)}{\sigma^2} - 1\right),\quad \beta = (1 - \mu) \left(\frac{\mu(1 - \mu)}{\sigma^2} - 1\right)\$

which only holds when \$\sigma^2 < \mu(1 - \mu)\$.

With this, I ended up with the following code:

from __future__ import division

def lst_beta(low, high, num, mean):
    diff = high - low
    mu = (mean - low) / diff
    sigma = mu * (1 - mu)
    c = 1 / sigma - 1
    a, b = mu * c, (1 - mu) * c
    return low + diff * np.random.beta(a, b, size=num)

Visualized for \$\mu = 10, 50, 75, 99\$ this looks like this:

In comparison, this is what @Peilonrayz's algorithm produces for these means:

Edit:

Here is the code to generate the graphs:

import matplotlib.pyplot as plt

#function definitions here
def lst_w_mean(low, high, num, target):
    ...

plt.figure()
means = 10, 50, 75, 99
for i, mean in enumerate(means, 1):
    plt.subplot(len(means), 1, i)
    l = lst_w_mean(0, 100, 5000, mean)
    plt.hist(l, bins=100, range=(0, 100))

plt.show()


Horizontally concatenating the entries of two matrices, and padding with NaNs I have two matrices a and b that have, on each line, some values (null or not) and then a bunch of zeros at the end.

I want to construct a matrix c that for each line has the corresponding line of a from the first element till to the last not-null element, concatenated with the corresponding line of b from the first element till to the last not-null element. Like in the following example:

>> a=[1 NaN 3 0; 1 0 0 0]

a =

     1   NaN     3     0
     1     0     0     0

>> b=[1 0 1 0; 0 0 0 0]

b =

     1     0     1     0
     0     0     0     0

>> c=NaN(2,8) %same dimension as a and b

c =

   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN
   NaN   NaN   NaN   NaN   NaN   NaN   NaN   NaN

        for ii=1:size(a,1)
        [v,x]=find(a(ii,:));
        if numel(x)>0 %it could be all null
            x1=a(ii,1:x(end)); %x(end) because I want to retain the zeros before the last not-null element
        end
        [v,x]=find(b(ii,:));
        if numel(x)>0
            x1=[x1 b(ii,1:x(end))];
        end
        c(ii,1:numel(x1))=x1;
        end
>> c

c =

     1   NaN     3     1     0     1   NaN   NaN
     1   NaN   NaN   NaN   NaN   NaN   NaN   NaN

How can I improve this code?
a and b always have the same dimensions (that can be different from the example though). a and b values can be positive/negative finite or infinite doubles or NaNs.
In my case I am interested only to merge two matrices.

Are there "standard" ways to vectorize a code? What do you look at when thinking to how vectorize a code?

I see two major way to simplify and speed this up.  First, rather than copying a to c at every step of the loop, you can define c to include a from the beginning.  Second, you can find all the nonzero, non-NaN values in a vectorized manner at the very beginning  This reduces the time for me be about 1/2 for a large (~10000 row) random data set.

function c = catnonzero_2(a, b)
    c=[a, NaN(size(b))];
    agood = a~=0;
    bgood = b~=0;
    for ii=1:size(a,1)
        ailast=find(agood(ii,:), 1, 'last');
        if isempty(ailast)
            ailast = 0;
        end
        bilast=find(bgood(ii,:), 1, 'last');
        if ~isempty(bilast)
            c(ii, ailast+1:ailast+bilast) = b(ii, 1:bilast);
        else
            c(ii, ailast+1:end) = NaN;
        end
    end
end

This can be improved even more by separating out the rows that can be vectorized from those that can't.

I find all the rows of a and b with some non-null values ("good" rows).  Rows that aren't good are then filled with NaN.
I find all the rows of a and b where the last value is non-null ("perfect" rows).
For rows that are good but not perfect ("imperfect" rows):

For b I can loop over them an put the NaN values at the end.  This allows me to treat all b rows as either "perfect" or "bad", since the "imperfect" rows already have the NaN values at the end and thus can be used in the entirety.
For a, I can loop over the rows where a are imperfect and b are not good and put NaN at the end of those.  Where b is good, b will be ovewriting those anyway so it isn't necessary as long as b is at least as long as a.

I make c like in the previous example.
In cases where a is perfect and b is good, I can just write b to c at the point where a ends.
In the cases where a is bad and b is food, I can just write b from the start of c.
In the remaining cases, I have to calculate where b starts, and then write b.
 3.

This reduces the time for me by about another 1/4:

function c = catnonzero_3(a, b)
    alen = size(a, 2);
    blen = size(b, 2);

    agood = a~=0;
    bgood = b~=0;

    agoodrows = any(agood, 2);
    bgoodrows = any(bgood, 2);

    aperfectrows = agood(:,end);
    bperfectrows = bgood(:,end);
    aimperfectrows = agoodrows & ~aperfectrows;

    a(~agoodrows, :) = NaN;
    b(~bgoodrows, :) = NaN;

    if alen <= blen
       apadtarg = aimperfectrows & ~bgoodrows;
    else
       apadtarg = aimperfectrows;
    end

    for ii=find(apadtarg)'
        a(ii, ailast(ii)+1:end) = NaN;
    end
    for ii=find(bgoodrows & ~bperfectrows)'
        b(ii, bilast(ii)+1:end) = NaN;
    end

    c=[a, NaN(size(b))];

    ailast = @(ii) find(agood(ii,:), 1, 'last');
    bilast = @(ii) find(bgood(ii,:), 1, 'last');

    aperfbgood = aperfectrows & bgoodrows;
    c(aperfbgood, size(a, 2)+1:end) = b(aperfbgood, :);

    abadbgood = ~agoodrows & bgoodrows;
    c(abadbgood, 1:blen) = b(abadbgood, :);

    for ii=find(aimperfectrows & bgoodrows)'
        ajj = ailast(ii);
        c(ii, ajj+1:ajj+blen) = b(ii, :);
    end
end

And here is the data set I used to test (your version is catnonzero_1:

a=randi([-11, 10], 100000,80);
b=randi([-11, 10], 100000,100);

a(a==-11) = NaN;
b(b==-11) = NaN;

timeit(@() catnonzero_1(a, b))
timeit(@() catnonzero_2(a, b))
timeit(@() catnonzero_3(a, b))

And the result:

>> run_w_time

ans =

    0.889284394000000

ans =

    0.411362394000000

ans =

    0.102184394000000

ans =

    1

ans =

    1

c1 = catnonzero_1(a, b);
c2 = catnonzero_2(a, b);
c3 = catnonzero_3(a, b);

all(all(c1(~isnan(c1))==c2(~isnan(c2))))
all(all(c1(~isnan(c1))==c3(~isnan(c3))))


Dynamic condition on string in method I have the following method:

Float getPriceEqualsCategoryName(BrowseNode node, Float productPrice) {
    String name = node.getName();
    for (Entry<String, List<RangeValue>> entry : categoryToFee.entrySet()) {
        if (name.equals(entry.getKey())) {
            List<RangeValue> values = entry.getValue();
            for (RangeValue value : values) {
                if (productPrice < value.getMinValue())
                    return value.getMinValue();
                if (value.getRange().contains(productPrice))
                    return value.getReferPercentFee() / 100 * productPrice;
            }
        }
    }
    if (node.getAncestors() == null)
        return 0f;
    return getPriceEqualsCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}

In the above method I have to loop over something similar to tree structure bottom up and check if the name of the node EQUALS to entry name in the map. if yes - I will calculate the price according to it.

As a second choice, I need to loop again to check if the name CONTAINS the entry name in the map.

I wrote a similar method:

Float getPriceContainsCategoryName(BrowseNode node, Float productPrice) {
        String name = node.getName();
        for (Entry<String, List<RangeValue>> entry : categoryToFee.entrySet()) {
            if (name.contains(entry.getKey())) {
                List<RangeValue> values = entry.getValue();
                for (RangeValue value : values) {
                    if (productPrice < value.getMinValue())
                        return value.getMinValue();
                    if (value.getRange().contains(productPrice))
                        return value.getReferPercentFee() / 100 * productPrice;
                }
            }
        }
        if (node.getAncestors() == null)
            return 0f;
        return getPriceContainsCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
    }

As you see, the only difference is the name.equals vs name.contains in the 4th line.

I don't like this duplicated code.. is there a better way to write it?

Extract Duplication

You could extract parts of the duplication to their own method:

private Float someResonableName(Float productPrice, List<RangeValue> values) {
    for (RangeValue value : values) {
        if (productPrice < value.getMinValue()) {
            return value.getMinValue();
        }
        if (value.getRange().contains(productPrice)) {
            return value.getReferPercentFee() / 100 * productPrice;
        }
    }
    return -1;
}

And use it like this:

        if (name.contains(entry.getKey())) {
            Float result = someResonableName(productPrice, entry.getValue());
            if (result != -1) {
                return result;
            }
        }

Use Interface

You could also use an interface:

interface NameKeyRelation {
    boolean isRelation(String name, RangeValue key);
}

Float getPriceContainsCategoryName(BrowseNode node, Float productPrice) {
    return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) ->  name.contains(key)));
}

Float getPriceEqualsCategoryName(BrowseNode node, Float productPrice) {
    return getPriceContainsCategoryName(node, productPrice, (String name, RangeValue key) ->  name.equals(key)));
}

Float getPriceCategoryName(BrowseNode node, Float productPrice, NameKeyRelation relation) {
    String name = node.getName();
    for (Entry<String, List<RangeValue>> entry : categoryToFee.entrySet()) {
        if (relation.isRelation(name, entry.getKey())) {
            Float result = someResonableName(productPrice, entry.getValue());
            if (result != -1) {
                return result;
            }
        }
    }
    if (node.getAncestors() == null)
        return 0f;
    return getPriceCategoryName(node.getAncestors().getBrowseNode().get(0), productPrice);
}

Restructure

It seems that your method currently does two things: It finds a category by name, and it calculate/finds some kind of price for a category. I would write two completely separate methods for each tasks, and then try to reduce duplication.