Array-search algorithm for identical sequences and character distances Here is Python code written to perform the following operations:

Find each occurrence of a three-letter sequence in a character array (e.g. a sequence such as ('a','b','c')), including overlapping sequences of up to 2 shared characters.
Count the characters between the start of each sequence and the start of all identical sequences following it.
Every time the same number of characters results from #2, increment a counter for that specific number of characters (regardless of which character sequence caused it).
Return a dictionary containing all accumulated counters for character distances.

counts = {}
# repeating groups of three identical chars in a row
for i in range(len(array)-2):
    for j in range(i+1,len(array)-2):
        if ((array[i] == array[j]) & (array[i+1] == array[j+1]) & (array[i+2] == array[j+2])):
            if counts.has_key(j-i) == False:
                counts[j-i] = 1
            else:
                counts[j-i] += 1

This code was originally written in another programming language, but I would like to apply any optimizations or improvements available in Python.

1. Improving the code

Here are some ways in which you could improve your code, while leaving the algorithm unchanged:

You can avoid the need to check counts.has_key(j-i) if you use collections.Counter.
You are looping over all pairs of distinct indexes between 0 and len(array)-2. The function itertools.combinations provides a way to cleanly express your intention.
Instead of comparing three pairs of array elements, you can compare one pair of array slices.
Organize the code into a function with a docstring.
Generalize it from 3 to \$k\$.
Python doesn't have an "array" type, but it does have sequences. So your variables could be better named.

Here's the revised code after making the above improvements:

from collections import Counter
from itertools import combinations

def subseq_distance_counts(seq, k=3):
    """Return a dictionary whose keys are distances and whose values
    are the number of identical pairs of `k`-length subsequences
    of `seq` separated by that distance. `k` defaults to 3.

    """
    counts = Counter()
    for i, j in combinations(range(len(seq) - k + 1), 2):
        if seq[i:i + k] == seq[j:j + k]:
            counts[j - i] += 1
    return counts

It's possible to rewrite the body of this function as a single expression:

    return Counter(j - i
                   for i, j in combinations(range(len(seq) - k + 1), 2)
                   if seq[i:i + k] == seq[j:j + k])

Some people prefer this style of coding, and others prefer the explicitness of the first version. It doesn't make much difference to the performance, so comes down to personal taste.

2. Improving the algorithm

Here's an alternative approach (that you could use if the subsequences are hashable). Your original algorithm is \$ O(n^2) \$, where \$n\$ is the length of the sequence. The algorithm below is \$O(n + m)\$, where \$m\$ is the number of repeated triples. This won't make any different in the worst case, where \$m = Θ(n^2)\$, but in cases where \$m = ο(n^2)\$ it should be an improvement.

def subseq_distance_counts(seq, k=3):
    """Return a dictionary whose keys are distances and whose values
    are the number of identical pairs of `k`-length subsequences
    of `seq` separated by that distance. `k` defaults to 3.

    >>> subseq_distance_counts('abcabcabc')
    Counter({3: 4, 6: 1})
    >>> subseq_distance_counts('aaaaaa', 1)
    Counter({1: 5, 2: 4, 3: 3, 4: 2, 5: 1})

    """
    positions = defaultdict(list) # List of positions where each subsequence appears.
    for i in range(len(seq) - k + 1):
        positions[seq[i:i + k]].append(i)
    return Counter(j - i
                   for p in positions.itervalues()
                   for i, j in combinations(p, 2))


Setting TabIndex efficiently Is there a more efficient way to write this? Seems redundant to me:

    var leaves = $(".leaf");
    var trees = $(".tree");

    $(".my-class").on("click", function (e) {
        if ($(e.target).hasClass("leaf")) {
            trees.each(function() {
                $(this).attr("tabIndex", -1);
            });
            leaves.each(function() {
                $(this).attr("tabIndex", 1);
            });
        } else {
            trees.each(function() {
                $(this).attr("tabIndex", 1);
            });
            leaves.each(function() {
                $(this).attr("tabIndex", -1);
            });
        }
    });

.attr() can work on all jQuery matched elements at once.

Assuming that performance isn't a concern, I think that the clearest way would be to "reset" all nodes to have a tabIndex of -1 first.

var leaves = $(".leaf");
var trees = $(".tree");

$(".my-class").on("click", function(e) {
    leaves.add(trees).attr("tabIndex", -1);

    var selected = $(e.target).hasClass("leaf") ? leaves : trees;
    selected.attr("tabIndex", 1);
});


for of loop that stops at last object of array I have a for of loop, for each item in the childElements HTMLCollection I want to subtract the height of the elements from the parent height.

let height = element[0].clientHeight;
const childElements = <[HTMLElement]>element[0].children;
let i = 0;
for (const childElement of childElements) {
    if (i !== childElements.length - 1) {
        height -= Math.ceil(childElement.clientHeight);
    }
    i++;
}
return height;

But I want to exclude the last HTMLElement in the collection from the for loop. I've now created a i variable that is checked against the length of the collection minus 1. If that's not the case the height of the element is subtracted and i is increased.

This works fine, but it feels like I'm doing two things (for of loop and while loop). Any suggestions?

It hit me,

function returnSuggestedColleaguesWithExpertiseHeight(): number {
    let height = element[0].clientHeight;
    const childElements = <[HTMLElement]>element[0].children;
    for (let i = 0; i < childElements.length - 1; i++) {
        height -= Math.ceil(childElements[i].clientHeight);
    }
    return height;
}


Vim plugin for easy window navigation I've been thinking about it for a while and, after asking a question on vi.stackexchange, I decided to write a small plugin myself and put it on GitHub; basically it offers a mapping to activate a kind of "window movement" mode where Vim waits for inputs that it appends to Ctrl+w, so that one does not have to press this combination before every window-related movement or action; hitting Esc exits this mode.
It's mostly just one main function and one mapping, so I will copy them below.

The mapping plus some things I copied from scratch.vim which seemed reasonable:

" plugin/winzoz.vim

if (exists('g:winzoz_disable') && g:winzoz_disable) || &compatible
  finish
endif

nnoremap <silent> <Plug>(go-zoz-navigation) :call winzoz#goZoZ()<cr>

if !exists('g:winzoz_no_mappings')
  nmap <silent> <leader>w <Plug>(go-zoz-navigation)
endif

The function with the logic (plus a helper function for coloring the command line while the functionality is active)

" autoload/winzoz.vim

function! winzoz#goZoZ()
  redrawstatus!
  echohl Search
  echo s:make_status_line()
  echohl None
  let l:key = ''
  while l:key !=? "\<Esc>"
    let l:key = nr2char(getchar())
    execute "normal! \<c-w>" . l:key
    redrawstatus!
  endw
  echo ''
  redrawstatus!
endfunction

" drawing function
function! s:make_status_line()
  let l:text = 'Go WinZoZ'
  let l:text .= repeat(' ', (&columns - len(l:text))/2)
  return repeat(' ', &columns - len(l:text) - 1) .  l:text
endfunction

I'd like some feedback in general, but also on the following points.

As regards the logic, I'm not checking that the input is correct (with a :try-:catch-:finally), because I'm not inventing any functionality, but only forwarding the keys to the already existing functionality of Ctrl+w+whatever.
I'm using echohl Search + echo to print a colored command line to make apparent when the mode is active; is this a good way of doing it?

Regarding this part, I had to make a trick (the - 1 in the second to last line of code above) in order to have one character left for the blinking cursor. I really don't like this, but I don't know what else I could do...

There are some Ctrl+w-commands that take two key strokes, but my plugin cannot deal with them :( ... I'll fix when I have time.
Do you like it?
Do you think it could be useful?

(I also plan to add some documentation to it, fwiw :/ )

As requested on vi.SE, here is my little view.
First, I see a great quality in most patterns you have used: autoload plugin, plug-mappings, and so on. Good work!
If I really had to nit-pick, it would be that:

I find l: prefix to be cumbersome and useless in many situations
I like to annotate my functions with abort in order to simplify error messages in case I'd introduce errors in future versions. abort should have been the default, but unfortunately, this is not the case.
"\<Esc>" and "\<ESC>" and "\<eSc>" are the same character, using a case-insensitive comparator doesn't really make any sense. !=# would make more sense. I have an old habit, personally I simply use != ^^'

Regarding your question regarding finally, it could make sense to make sure to reset the highlighting used with :echo in case make_status_line() could fail. I don't see any other place where it could make sense.
Regarding catch around <c-w> + {key}, it would depend on the user experience you want to provide. Use your plugin for some time, make some mistakes, and see how you'd like to see your plugin react.
Regarding the highlight color, in my plugin I use StatusLineNC as it's similar to what :substitute/pat/txt/c seems to use. In another plugin, I use Question.

Do you think it could be useful?

Well. I know I could do without it. And yet a few times, I've been bothered by the number of times I had to hit <c-w><down>. In the end I use the mouse. So... Why not. I'd have to make sure though that I never use <c-w> in a non-nore mapping.


Find and compress file in Python script I've just replaced my bash script with a Python script, and this is my first script where I used class.

Please review and give me suggestions.

#!/usr/bin/env python

# This Script will compress One day old file means yesterday files

from __future__ import print_function
import os
import sys
import time
import glob
import subprocess
from datetime import datetime

nday = 1        # set how many days old file
ftype = "*.log"     # set file type which should be compressed
lookinto = "/var/log/"  # path of files

class CheckFile(object):
    'Operation on file like get size,file time etc..'
    def __init__(self,fname):
        self.fname = fname
    def size(self):
        'print size in bytes MB'
        num = os.path.getsize(self.fname)
        for s in [ 'bytes','KB','MB','GB','TB' ]:
            if num < 1024.0:
                return "%3.1f %s" % (num,s)
            num /= 1024.0
    def date(self):
        'print file date'
        self.unix_time = os.path.getmtime(self.fname)
        self.file_date = datetime.fromtimestamp(self.unix_time)
        return self.file_date.strftime('%d/%m/%y')
    def days_of_days_old(self):
        'print how much file is old from today'
        self.date()
        days = (datetime.today() - self.file_date).days
        return days

def compresse_file(file):
    print("Compressing...")
    cmd = "gzip -v " + file
    os.system(cmd)

if __name__ == '__main__':
    os.chdir(lookinto)
    print('{0:10}{1:10}{2:10}'.format("Size","Date","File Name"))
    print('{0:10}{1:10}{2:10}'.format("-----","------","---------"))
    for file in glob.glob(ftype):
        f = CheckFile(file)
        if f.days_of_days_old() == nday:
            print('{0:10}{1:10}{2:10}'.format(f.size(),f.date(),f.fname))
            compresse_file(file)
            print("file status after compressesion")
            f = CheckFile(file + ".gz")
            print('{0:10}{1:10}{2:10}'.format(f.size(),f.date(),f.fname))
            print()

First thing's first, instead of calling out to the gzip program with os.system, use the gzip module in Python.  This will almost always be more portable: http://docs.python.org/2/library/gzip.html

Second of all, instead of using os.path.getsize and os.path.getmtime just use a single os.stat call: http://docs.python.org/2/library/os.html#os.stat  You can call this once and cache the results for a single file.

Don't require calling self.date() in order to set the unix_time and file_date attributes.  Instead make those into properties.  For example you could write that like:

class CheckFile(object):
    'Operation on file like get size,file time etc..'

    def __init__(self, fname):
        self.fname = fname
        self._stat = None

    @property
    def stat(self):
        if self._stat is None:
            # Note: This will not update automatically if your file's stats change
            self._stat = os.stat(self.fname)
        return self._stat

    @property
    def unix_time(self):
        return self.stat.st_mtime

    @property
    def datetime(self):
        return datetime.fromtimestamp(self.unix_time)

    @property
    def age_in_days(self):
        return (datetime.today() - self.datetime).days

All that said, although I wouldn't discourage anyone from wanting to learn object-oriented programming, for this particular application writing a class is overkill :)


Send html with nonce for each script and style tag on each request This working code inserts a nonce attribute and value for each style and script tag inside an HTML file, for each GET request. It is using Express. nonce1 and dirViews are already defined. limiter is for rate limiting and is a node module found on npm.

//on get homepage
app.get('/', limiter.middleware(), function(req, res, next) {
    //send an edited index.html containing nonces
    fs.readFile(dirViews+'/index.html', 'utf8', function(err, html){
        let newHTML = html.replace(/<script/g, '<script nonce="'+nonce1+'"').replace(/<style/g, '<style nonce="'+nonce1+'"');
        res.send(newHTML);
    });
});

I am unsure if this is the correct way to do this (set nonce attributes on each GET request), and whether my code follows best practices.

I am unsure if this is the correct way to do this (set nonces on each get)

Yes, it is

  Remember that nonces must be regenerated for every page request and they must be unguessable.

In terms of best practises, couple of things I'd recommend:

Move your nonce manipulation into it's own middleware
Cache your page content, don't read it from disk every time

The end result would look something like:

csp.js

const path = require('path');
const fs = require('fs');

const cache = new Map();

const getHtml = path => new Promise((resolve, reject) =>
  fs.readFile(path, 'utf-8', (err, contents) => {
    if (err) return reject(err);

    cache.set(path, contents);
    return resolve(contents);
  }));

const csp = page => async (req, res, next) => {
  const pagePath = path.join(dirViews, page);
  let html = cache.get(pagePath);
  if (!html) {
    html = await getHtml(pagePath);
  }
  html = html.replace(/<script/g, `<script nonce="${nonce1}"`)
    .replace(/<style/g, `<style nonce="${nonce1}"`);
  res.send(newHTML);
}

export default csp;

app.js

app.get('/', limiter.middleware(), csp('index.html'))


Iterating through a linked-list in a cleaner manner I have a collection of PlaylistItem objects. They are linked together such that each item knows the ID of its next/previous item. I am iterating over this collection of objects, starting at a known position and working with each object once. I'm not a big fan of how I do this, but I am not seeing an obvious way of rewriting it.

var firstItemId = activePlaylist.get('firstItemId');
var currentItem = activePlaylist.get('items').get(firstItemId);

//  Build up the ul of li's representing each playlistItem.
do {

    var listItem = $('<li/>', {
        'data-itemid': currentItem.get('id'),
        contextmenu: function (e) {

            var clickedItemId = $(this).data('itemid');
            var clickedItem = activePlaylist.get('items').get(clickedItemId);

            contextMenu.initialize(clickedItem);

            //  +1 offset because if contextmenu appears directly under mouse, hover css will be removed from element.
            contextMenu.show(e.pageY, e.pageX + 1);
            //  Prevent default context menu display.
            return false;
        }
    }).appendTo(playlistItemList);

    $('<img>', {
        'class': 'playlistItemVideoImage',
        src: 'http://img.youtube.com/vi/' + currentItem.get('video').get('id') + '/default.jpg',
    }).appendTo(listItem);

    $('<a/>', {
        text: currentItem.get('title')
    }).appendTo(listItem);

    currentItem = activePlaylist.get('items').get(currentItem.get('nextItemId'));
} while (currentItem && currentItem.get('id') !== firstItemId)

How would you write this?

Note that it is a do-while because I want to ensure I always render that firstItem. If there's only one item in the list, the loop will immediately terminate. Thus the need for the do-while.

Here's a bit of "light" refactoring. I've basically moved the element constructing into its own function to keep the while-block less crowded.

I'm also using some closure magic for the contextmenu, so it doesn't have to re-get the clicked item.

Lastly, I'm going more by the current ID rather than the current object when looping. This was to avoid the "get the id to get the object to get its id again" code that I see a few places in your version.

function buildListItem(item) {
  var listItem = $('<li/>', {
    contextmenu: function (event) {
      contextMenu.initialize(item); // available via closure
      contextMenu.show(e.pageY, e.pageX + 1);
      return false;
    }
  });

  listItem.append($('<img>', {
    'class': 'playlistItemVideoImage',
    'src':   'http://img.youtube.com/vi/' + item.get('video').get('id') + '/default.jpg'
  }));

  listItem.append($('<a/>', {
    text: item.get('title')
  }));

  return listItem;
}

var firstItemId   = activePlaylist.get('firstItemId'),
    allItems      = activePlaylist.get('items'),
    currentItemId = firstItemId,
    currentItem;

do {
  currentItem = allItems.get(currentItemId);             // get the obj
  if(!currentItem) {
    break;                                               // stop if it ain't there
  }
  playlistItemList.append(buildListItem(currentItem));   // build the list item element
  currentItemId = currentItem.get('nextItemId');         // get the next ID
} while(currentItemId && currentItemId !== firstItemId); // I'm assuming that valid IDs are thruth'y

Another thing to consider would be to use rawer HTML because it can be faster.


Calculating profit and score, if coordinates fall in a certain region of a field My function takes the values of the profit (function declared as fieldProfit) and the field score (function declared as fieldScore). If both are above 10, then you earn a badge, hence, innerbadge = 1.

But, there's also another condition that must be met: the field or (x, y) coordinates have to fall in the area depicted as the shaded in box that has a hole in the middle.  Here's the image:

I've written the code for it, and I just wanted to make sure that my logic/syntax is correct.

 int badgeInnerCircle(int x, int y) {
       double fprofit, fscore;
       int innerbadge;

       if ((x >= 1 && x <= 20) && (y >= 1 && y <= 20)) {
          if (((x == 7 || x == 8) && (y >= 7 && y <= 14)) || ((x == 13 || x == 14)
             && (y >= 7 && y <= 14)) || ((x >= 7 && x <= 14) && (y == 7 || y == 8))
                || ((x >= 7 && x <= 14) && (y == 13 || y == 14))) {
             fprofit = fieldProfit(x, y);
             fscore = fieldScore(x, y);
             if (fprofit >= 10 && fscore >= 10) {
                innerbadge = 1;
             }
             else {
                innerbadge = 0;
             }
          }
       }
       else {
          innerbadge = -1;
       }
       return innerbadge;
    }

I think we can all agree that that code is hard to read.  Rather than treating the dark shaded zone as the union of four rectangles, you would be better off treating it as a square with the centre excluded.

I've used a macro to reduce the verbosity.

In your code, when (x, y) falls in either of the white zones, you return an uninitialized value for innerbadge.  In those cases, the behaviour is undefined.

#define XY_IN_SQUARE(min, max) (((min) <= x && x <= (max)) && \
                                ((min) <= y && y <= (max)))

int badgeInnerCircle(int x, int y) {
    if (XY_IN_SQUARE(9, 12)) {          /* In innermost white zone */
        return /* what? */;
    } else if (XY_IN_SQUARE(7, 14)) {   /* In dark grey zone */
        double fprofit = fieldProfit(x, y);
        double fscore = fieldScore(x, y);

        /* The following value will be either 1 or 0 */
        return fprofit >= 10 && fscore >= 10;
    } else if (XY_IN_SQUARE(2, 19)) {   /* In outer white zone */
        return /* what? */;
    } else if (XY_IN_SQUARE(1, 20)) {   /* On yellow border */
        return /* what? */;
    } else {                            /* Outside yellow border */
        return -1;
    }
}


Convert Sorted Array to Height Balanced Binary Search Tree Algorithm

This is my code to convert a sorted vector of integers to a balanced binary tree of the same content.

The binary tree is made of these nodes:

 struct TreeNode {
     int val;
     TreeNode *left;
     TreeNode *right;
     TreeNode(int x) : val(x), left(NULL), right(NULL) {}
 };

Here's my first attempt:

class Solution {
public:
    TreeNode* sortedArrayToBST(vector<int>& nums) {
        if(nums.empty()) return NULL;
        if(nums.size()==1){
            TreeNode* root = new TreeNode(nums[0]);
            return root;
        }

        int mid = nums.size() / 2;
        TreeNode* root = new TreeNode(nums[mid]);

        vector<int> left(nums.begin(),nums.begin()+mid);
        vector<int> right(nums.begin()+mid+1,nums.end());

        root->left = sortedArrayToBST(left);
        root->right = sortedArrayToBST(right);

        return root;
    }
};

I was making new vectors and copying contents twice in every recursive call, So this is the optimized version by directly passing iterator instead of passing index and copying sub arrays every time:

class Solution {
public:
    TreeNode* sortedArrayToBST(vector<int>& nums) {
      return makeBST(nums.begin(),nums.end());
    }

    TreeNode* makeBST(vector<int>::iterator start,vector<int>::iterator end){
        if( start >= end ) return NULL;
        vector<int>::iterator mid = start + (end - start) / 2;
        TreeNode* root = new TreeNode(*mid);
        root -> left = makeBST( start , mid );
        root -> right = makeBST( mid + 1 , end );
        return root;
    }
};

The second code has much faster run time than first one.

Is there a better way to pass iterators to function?

Any other improvements?

A class Solution with only public members can be written struct Solution.  But with only a single member function (that never uses the this pointer), why not a free function?

Presumably your vector is intended to be a std::vector, in which case we need #include <vector> and the correct namespace qualifier.

Don't pass a reference to a mutable object unless it's reasonable for the function to modify it.  In this case, we just want to copy from it, so pass a const std::vector<int>& instead.

You're right that it's very inefficient to make many new containers, when we just want a view; that's exactly what iterator-pairs (aka ranges) are good for.  I'll not consider the first version any further.

There's no need to accept only vectors of int - we'd like to accept any ordered random-access container, of any element type.  That's easy enough, with a template:

template<typename RandomAccessIterator>
TreeNode* makeBST(RandomAccessIterator start, RandomAccessIterator end){

There's a serious bug when new TreeNode() fails: all the allocated nodes further up the call tree will be leaked as the std::bad_alloc propagates upwards.  It's better to make the BST use smart pointers.  If you can't change that implementation, then we can achieve exception safety by holding each root in a smart pointer until after the potentially-throwing recursive calls:

    auto root = std::make_unique<TreeNode>(*mid);
    root->left = makeBST(start, mid);
    root->right = makeBST(mid + 1, end);
    return root.release();


N-Bonnaci sequence calculator The code calculates a n-bonnaci sequence to a certain number, based on the user's input. It then can print it to a file and/or print it to the console.

The calculateSequence function calculates the sequence. The first loop fills the sequence with ones (0, 1, 1, 2...). Next, it calculates the value of the current number, using crrntVlue as a placeholder, and using i as the location of the value to add (a number is the sum of the preceding two numbers). sqnceItratr is location of the number that is being calculated.

My questions are:

Is there anything immediately wrong that you see in the code?
How can I make the code more readable/better variable names, as it was hard to understand the code while coding it?
How can I restructure the program to make it better?

This is one of my first Python programs, so I want to know what I might be coding 'wrong'.

def calculateSequence(n, amntToCalc):
    sqnceItratr = 1
    sequence = [0] * numbrsToCalc

    #Add 1's to the array(Duonacci is 1, 1, 2... Tribonacci is 1, 1, 1, 3...)
    while sqnceItratr < n:
        sequence[sqnceItratr] = 1
        sqnceItratr += 1

    #Actually Calculate the Sequence
    while sqnceItratr < numbrsToCalc:
        i = N
        #i is the location of the amount to add to crrntVlue
        #Ex: sequence[3] = 1 + 1 + 1 = 3, i is sequence[3 - 1]

        crrntVlue = 0
        while i > 0:
            #This is to calculate the actual value of a number
            #in the sequence
            crrntVlue += sequence[sqnceItratr - i]
            i -= 1
            #Count Down for the number to add to crrntValue(n-bonnacci)

        sequence[sqnceItratr] = crrntVlue
        # Add crrntValue to the array
        sqnceItratr += 1
        #Continue to the next value in the array
    return sequence

def convertInput(inputToConvert):
    inputToConvert = inputToConvert.upper()
    if inputToConvert == 'Y':
        return True
    elif inputToConvert == 'YES':
        return True
    else:
        return False

N = int(input("What do you want to n in N-bonnacci to be? \n"))
numbrsToCalc = int(input("How many digits do you want to calculate? \n This should probably be bigger than n \n"))
sequence = calculateSequence(N, numbrsToCalc)

userInput1 = convertInput(input("Do you want to write it to a file? \n"))
sequencePosition = 0
if userInput1 == True:
    file = open("Seqence.txt", "w")
    while sequencePosition < len(sequence):
        file.write(str(sequencePosition + 1) + ': ' + str(sequence[sequencePosition]) + '\n')
        sequencePosition += 1
    file.close()

userInput2 = convertInput(input("Do you want to print it out? \n"))
if userInput2 == True:
    sequencePosition = 0
    while sequencePosition < len(sequence):
        print(str(sequencePosition + 1) + ': ' + str(sequence[sequencePosition]))
        sequencePosition += 1

Algorithm

Looks like you are doing too much. The recurrence can be greatly simplified:

\$a_{n} = a_{n-1} + a_{n-2} + \cdot\cdot\cdot + a_{n-k+1} + a_{n-k}\$, and

\$a_{n+1} = a_{n} + a_{n-1} + \cdot\cdot\cdot + a_{n-k+1}\$, so

\$a_{n+1} = a_{n} + (a_{n-1} + \cdot\cdot\cdot + a_{n-k+1}) = a_{n} + (a_{n} - a_{n-k})\$

meaning that the next k-bonacchi number is just

  sequence[n+1] = 2*sequence[n] + sequence[n-k]

Naming

Honestly, names like sqnceItratr and crrntVlue are more than unreadable. Don't abbreviate so aggressively. In this particular case I'd say that n fits much better than sqnceItratr.
Separation of concerns

calculateSequence initialize the sequence and produces the rest of it in a single call. It doesn't look right. I recommend to split the solution in initialize, and compute_next calls, along the lines of

sequence = initialize(N)
for n in range(N, limit):
    value = compute_next(sequence, N)
    sequence.append(value)


Check if ID exist or not Here I have a class where I'm checking if model_id exists in the table models_web_tbl.

Everything works like a charm but I just want to know if I'm on the right path.

class test extends dbh{

public function modelExist($model_id) {

$sql = "SELECT * FROM models_web_tbl WHERE model_id = ?";

    $stmt = $this->connect()->prepare($sql);
    $stmt->execute([$model_id]);
    $match = $stmt->fetch(PDO::FETCH_OBJ);

        if ($match > 0){

            echo "found";

        }else {

            $model_web_twitter = "twitter";

            $sql = "INSERT INTO models_web_tbl (model_web_twitter, model_id) VALUES (?, ?)";
            $stmt = $this->connect()->prepare($sql);
            $stmt->execute([$model_web_twitter, $model_id]);

            }

}
}
$model_id = 66;
$exist = new test ();
$exist = $exist->modelExist($model_id);

Function modelExist should check if model exists and return boolean. Your function does something completely different. It only needs that check for its job.

public function modelExists(int $modelId): bool;

To check existence of a row, you don't need all its columns. Select just 1 (literally) and check if you have received a row from the query or not.

SELECT 1 FROM models_web_tbl WHERE model_id = ?

Create another function for the insert

public function insertModel(int $modelId, string $modelWeb): void

Maybe you wanted to combine it into another function.

public function tryInsert(int $modelId, string $modelWeb): bool
{
  if ($this->modelExists($modelId)) {
    return false;
  }
  $this->insert($modelId, $modelWeb);
  return true;
}

Don't connect to database again for every query

class test
{
  private PDO $connection;
  function __construct(PDO $connection) {
    $this->connection = $connection;
  }
  function modelExists(int $modelId): bool
  {
    // ...
    $this->connection->prepare($sql);
    // ...
  }

  function insert(int $modelId, $modelWeb): void
  {
    // ...
    $this->connection->prepare($sql);
    // ...
  }
}

$connection = connect();
$test = new test($connection);
  // I don't know why it was important to assign 'twitter' to a variable first :)
if (!$test->tryInsert($model_id, 'twitter')) {
  // echo definitely dont belong into exists-check function
  echo "found";
}


Generating a musical scale from a root note I'm looking to improve my coding skills. I am new to programming, and after weeks of research and trial and error - I have successfully wrote this basic program to find a music scale by inputting a Root note and Scale.

Since I am pretty much teaching my self - I am looking for criticism to improve my syntax/programming. I compressed the code listed below and localized variables into a 46 line version of this program but really like the layout of the original, so please be brutally honest. Pros and Cons with explanations please?

Note: I was purely being lazy by using #define and plan to replace all strings and vectors with their correct std::typename.

#include <iostream>
#include <string>
#include <vector>
#include <map>

#define astring std::string
#define cvector std::vector

astring rootnote = "";
astring scale = "";
void notation();
int root;
cvector<astring> notes;
cvector<astring> order;
void scaler();

int main()
{
    while (rootnote != "null") {

        std::cout << "Please enter your root note and scale: " << std::endl;
        std::cin >> rootnote >> scale;

        std::cout << "\nroot scale: " << rootnote << " " << scale << std::endl;
        std::cout << std::endl;

        notation();
        scaler();
    }

    return 0;
}

void notation()
{
    notes.clear();

    cvector<astring> chromatic = { "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B" };

    root = std::distance(chromatic.begin(), std::find(chromatic.begin(), chromatic.end(), rootnote));

    for (int i = root; i < chromatic.size(); i++) { notes.push_back(chromatic[i]); }
    for (int j = 0; j < root; j++) { notes.push_back(chromatic[j]); }

    return;
}

void scaler() {
    order.clear();

    std::map<astring, cvector<int>> scales;
    scales["Major"] = { 0, 2, 4, 5, 7, 9, 11 };
    scales["Minor"] = { 0, 2, 3, 5, 7, 8, 10 };
    scales["Melodic"] = { 0, 2, 3, 5, 7, 9, 11 };
    scales["Harmonic"] = { 0, 2, 3, 5, 7, 8, 11 };

    for (auto i : scales[scale])
        order.push_back(notes[i]);
    for (int j = 0; j < order.size(); j++)
        std::cout << order[j] << " ";

    std::cout << "\n\n" << std::endl;

    return;
}

Yikes — the global variables make this spaghetti code.  Functions should make it obvious what their input parameters are, and return their output values.  You'd be better off lumping everything into one main() function than using global variables.  Ironically, the two variables that should have been global constants — chromatic and scales — weren't.

It is customary to put main() last, and helper functions first, so that you don't have to write forward declarations.

You used std::find() and std::distance(), so you should #include <algorithms> and <iterator>.  I suggest listing your includes alphabetically.

Behaviour

Please enter your root note and scale:
F# Major

root scale: F# Major

C D E F G A B

If the rootnote is not found in chromatic, you should handle that case instead of blindly proceeding based on chromatic.end().

Please enter your root note and scale:
D Major

root scale: D Major

D E Gb G A B Db

Those enharmonic equivalents will sound the same, but those should technically be F♯ and C♯.

Please enter your root note and scale:
null blah

root scale: null blah

Making the user enter the magic word "null" as the root note to exit the program is weird.

Suggested solution

#include <algorithm>
#include <iostream>
#include <iterator>
#include <map>
#include <sstream>
#include <string>
#include <vector>

const std::vector<std::string> chromatic = {
    "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"
};

const std::map<const std::string, const std::vector<int>> degrees = {
    {"Major",       { 0, 2, 4, 5, 7, 9, 11, 12 }},
    {"Minor",       { 0, 2, 3, 5, 7, 8, 10, 12 }},
    {"Harmonic",    { 0, 2, 3, 5, 7, 8, 11, 12 }},
    {"Melodic",     { 0, 2, 3, 5, 7, 9, 11, 12 }}
};

std::string scalenotes(const std::string& rootnote, const std::string& scale) {
    int root = std::distance(chromatic.begin(), std::find(chromatic.begin(), chromatic.end(), rootnote));
    if (root >= chromatic.size()) {
        return "";
    }
    std::stringstream ss;
    for (int i : degrees.at(scale)) {
        ss << chromatic[(root + i) % chromatic.size()] << " ";
    }
    return ss.str();
}

std::stringstream ask(const std::string& question) {
    std::string line;
    std::cout << question;
    std::getline(std::cin, line);
    return std::stringstream(line);
}

int main()
{
    std::string rootnote, scale;
    while (ask("Please enter your root note and scale: ") >> rootnote >> scale) {
        std::cout << "\nroot scale: " << rootnote << " " << scale
                  << ": " << scalenotes(rootnote, scale) << "\n\n";
    }
}


cleaning up and increasing performance of js code I would like to increase my performance of js code write a clean code... can you tell me how to do it... providing my fiddle below with js code...

http://jsfiddle.net/YYg8U/

var myNumbersToSort = [-1, 2, -3, 4, 0.3, -0.001];

function getClosestToZero(numberSet) {
    var i = 0, positiveSet = [], positiveClosest = 0;
    for (i = 0; i < numberSet.length; i += 1) {
        positiveSet.push(numberSet[i] >= 0 ? numberSet[i] : numberSet[i] * -1);
    }
    positiveClosest = Math.min.apply(Math, positiveSet);
    return numberSet[positiveSet.indexOf(positiveClosest)];
}

alert(getClosestToZero(myNumbersToSort));

In code you posted, you pass through array three times instead of one:

1st pass when pushing absolute values to auxillary array
2nd pass when searching for minimum value (in Math.min)
3rd pass when you get the index of minimum value (in indexOf)

It is more efficiently to do it in just one pass:

function getClosestToZero(set) {
  if(0 === set.length) return null;
  var closest = Math.abs(set[0]), result = 0;
  for(var i in set) {
     var next = Math.abs(set[i]);
     if(closest > next) {
       result = i;
       closest = next;
     }
  }
  return result;
}

Also for the purpose of readability I will recommend you to use Math.abs library function instead of implementing it by yourself.

Note: The small theoretical disatavantage of my code consists in that it compares first element with itself. You can rewrite for loop to avoid it or ignore it because its impact is insigninficant.


Rails like and unlike posts I have the following two methods which handle the liking and unliking of posts in my Rails application:

def create
  @post = Post.find(params[:id])
  if @post.present?
    @like = Like.new(user_id: current_user.id, post_id: @post.id)
    if @like.save
      redirect_to post_redirect(@post), :notice => 'Liked!'
    else
      redirect_to post_redirect(@post), :alert => 'An error prevented you from liking this post!'
    end
  else
    redirect_to post_redirect(@post), :alert => 'Invalid post!'
  end
end

def destroy
  @post = Post.find(params[:id])
  if @post.present?
    @like = Like.where(user_id: current_user.id, post_id: @post.id).first
    if @like.destroy
      redirect_to post_redirect(@post), :notice => 'Unliked!'
    else
      redirect_to post_redirect(@post), :alert => 'An error prevented you from unliking this post!'
    end
  else
    redirect_to p, :alert => 'Invalid post!'
  end
end

They both check that the post exists first and then creates or destroys the like record containing the post id and user id and redirects to the post with a message depending on the outcome.

In the Like model I prevent duplicate likes with: validates :user_id, uniqueness: {scope: :post_id}

However the methods feel quite bloated... is there a better way to handle this?

You're right. The code can be improved. Here is how I'd write this.

We can extract post lookup into a method or before hook.
There is no need to check if Post was found. In production environment find will raise an ActiveRecord::RecordNotFound exception (I'm assuming you're using AR)
The destroy action can be a little more optimistic. When somebody unlikes the post, which cannot be found or which is not liked, we can just show the success. After all there is no like for the post.
We can use associations on models to write less.
AR understands relations in conditions. where(user: current_user) instead of where(user_id: current_user.id)
Use find_by instead of where().first

For example:

class LikesController < ApplicationController
  # before_action :find_post

  def create
    if post.likes.create(user: current_user)
      redirect_to post_redirect(post), :notice => 'Liked!'
    else
      redirect_to post_redirect(post), :alert => 'An error prevented you from liking this post!'
    end
  end

  def destroy
    post.likes.where(user: current_user).destroy_all
    redirect_to post_redirect(post), :notice => 'Unliked!'
  end

  private

  def post
    @post ||= Post.find(params[:id])
  end

  # def find_post
  #   @post = Post.find(params[:id])
  # end
end


Bad performance of the unsorted list (nearly quadratic time) I've already made an implementation of unsorted List but I'm not happy with results it gives. It looks like Big-O is nearly \$N^2\$ which is, I suppose, not correct for my list.

Here's the graph that shows the problem:

The X axis represents number of total operations: adding new elements and same number of deleting max element, Y axis represents time in milliseconds.

If you could show me some lines that could be made better, that would be great. Unfortunately, implementation must contain array types instead of easier and faster List types.

TList.cs:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace list
{
    class TList
    {
        // elements of list
        int[] elements;
        int size;
        public TList()
        {
            size = 1;
            elements = new int[size];
        }
        public void Push(int data)
        {
            size++;
            if(size > elements.Length)
            {
                int[] new_elements = new int[elements.Length * 2];
                elements.CopyTo(new_elements, 0);
                elements = new_elements;
            }
            elements[size - 1] = data;
        }
        public int MaxIndex()
        {
            var index = 0;
            var max = elements[0];
            for(var i=1;i< size;i++)
            {
                if(elements[i]>max)
                {
                    max = elements[i];
                    index = i;
                }
            }
            return index;
        }
        public int[] DeleteAt(int indexToRemove)
        {
            return elements = elements.Where((source, indexOfElements) => indexOfElements != indexToRemove).ToArray();
        }

    }
}

Another (better?) way to delete:

public void DeleteAt(int indexToRemove)
{
    int[] new_elements = new int[elements.Length - 1];
    for (int i = 0, j = 0; i < size - 1; i++, j++)
    {
        if (i == indexToRemove)
            j++;
        new_elements[i] = elements[j];
    }
    elements = new_elements;
    size--;
}

For now I'm asking how to optimise deleting of MaxIndex from unsorted list

MaxIndex looks fine. Hard to optimize that further. Maybe you can write a normal for loop that goes from zero to less than the array length so that the JIT can optimize away the range check. You then need to insert if (i >= size) break;. The .NET JIT is not good at optimizing so you have to help. RyuJIT, though, does range checks in loops much, much better.

Your delete is highly inefficient. First of all LINQ is the slowest way to create a new array. Better would be to new it manually and then use Array.Copy to copy over the relevant two sections.

Doing it in-place would be again much faster, at least 2x.

The DeleteAt is probably worse than Array.Copy. It is not very good. It does a lot per item and it does not use memcpy like Array.Copy does. Also, there is no need to create a new array. Just copy in place and do size--.


Cards for Monster, Spell and Trap interfaces So I wrote a card game in Java/JavaFX but I don't particularly like this solution and I was wondering if you guys have any better solutions:

public static CardNode get(Card card, double nodeWidth, double nodeHeight, double imageWidth, double imageHeight) {
    if (card.isMonster()) {
        MonsterCardNode cardNode = new MonsterCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    } else if (card.isSpell()) {
        SpellCardNode cardNode = new SpellCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    } else if (card.isTrap()) {
        TrapCardNode cardNode = new TrapCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    }
    return null;
}

CardNode and Card are abstract classes. They both implement interfaces Monster, Spell and Trap. Methods isMonster, isSpell, isTrap return a boolean and are essentially instanceof and flag checks. I was wondering if there is any better way to handle this with polymorphism.

Here is the usage of that static method.

model.getPlayer().getHand().addListener((ListChangeListener.Change<? extends Card> change) -> {
    while (change.next()) {
        if (change.wasAdded()) {
            change.getAddedSubList().forEach((card) -> {
            Platform.runLater(() -> {
                    CardNode cardNode = CardNode.get(card, 105, 145, 96, 105);
                    view.getPlayerView().getHand().getChildren().add(cardNode);
                });
            });
        }
    }
});

As a general principle, instead of asking an object what its type is, tell it what you want to do, and let overridden methods handle the differences between types:

return card.getCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);

and add a getCardNode abstract method to Card, so that each Card subclass can implement its own piece of the get logic.


OOP Challenge in Python I was trying to solve the final puzzle at the end of the classes chapter (Chapter 8) in "Python Programming for the Absolute Beginner" which is stated as:

  Create a Critter Farm program by instantiating several Critter objects and keeping track of them through a list. Mimic the Critter Caretaker program, but instead of requiring the user to look after a single critter, require them to care for an entire farm. Each menu choice should allow the user to perform some action for all the critters (feed all the critters, play with all of the critters, or listen to all of the critters). To make the program interesting, give each critter random starting hunger and boredom levels.

Here is my solution:

from random import randrange

class Critter(object):
    def __init__(self, name):
        self.__name = name
        self.__hunger = randrange(7)
        self.__boredom = randrange(7)

    def __str__(self):
        return "Hi, I'm " + self.__name + " and I feel " + self.mood + "."

    def talk(self):
        print(self)
        self.__passTime()

    def eat(self):
        self.__hunger -= 3
        if self.__hunger < 0:
            self.__hunger = 0
        self.__passTime()

    def play(self):
        self.__boredom -= 3
        if self.__boredom < 0:
            self.__boredom = 0
        self.__passTime()

    def __passTime(self):
        self.__hunger += 1
        self.__boredom += 1

    @property
    def mood(self):
        happiness = self.__hunger + self.__boredom
        if happiness < 5:
            return "happy"
        elif happiness < 10:
            return "ok"
        else:
            return "sad"

class CritterFarm(object):
    def __init__(self):
        self.__critters = []

    def talk(self):
        for critter in self.__critters:
            critter.talk()

    def add(self, name):
        self.__critters.append(Critter(name))

    def eat(self):
        for critter in self.__critters:
            critter.eat()

    def play(self):
        for critter in self.__critters:
            critter.play()

def main():
    farm = CritterFarm()
    option = ""
    while option != "0":
        choice = input("""
        0 - Quit
        1 - Add a Critter to your Farm
        2 - Listen to your Critters
        3 - Feed your Critters
        4 - Play with your Critters

        Choice: """)

        if choice == "0":
            print("Exiting...")
        elif choice == "1":
            name = input("Enter your new Critter's name: ")
            farm.add(name)
        elif choice == "2":
            farm.talk()
        elif choice == "3":
            farm.eat()
        elif choice == "4":
            farm.play()

if __name__ == "__main__":
    main()

Can anyone offer a code review? Thanks.

Why are you using __double_underscore_prefixes for your member variable names?

Double-underscores get name-mangled by Python for the purpose of 'class-local references'. They're for when your class's implementation relies on something that you don't want subclasses to customise. This is a very rare situation, and typically happens quite late in a library's lifetime.

What double-underscore names are not for is access control. This is one of the biggest pieces of mis-information given to Python beginners.
Python doesn't have a concept of 'public' and 'private', and you shouldn't try to hack one together out of a language feature designed for something else. All your member variables should just be public by default; if you need to change an implementation detail, you can maintain your contract using properties.

If you really insist on declaring something as private, the conventional way to do that in Python is using a _single_leading_underscore. It's still not actually private, it's just a clear signal to your consumers that they shouldn't mess around with that stuff because it might change.

I also don't really like the way you're using __str__. __str__ is designed to return a meaningful description of the object, generally for the purposes of debugging (or when you wish to emulate string-like behaviour). You're using __str__ as part of the behaviour of your class (returning a formatted string in plain English).

I'd put the body of __str__ inside the talk method, and either delete __str__ altogether, or use it for something more meaningful to a person working with the code - something like this:

def __str__(self):
    return "Critter named {} with hunger {} and boredom {}".format(self.__name, self.__hunger, self.__boredom)

def talk(self):
    print("Hi, I'm " + self.__name + " and I feel " + self.mood + ".")
    self.__passTime()


Calling a FormatCurrency function on multiple textboses I have a function called FormatCurrency() which I would like to call on each of the following elements:

 TextBoxA
 TextBoxB
 TextBoxC
 ...
 TextBoxN

My current function performs the following operation

 function formatALL()
 {
      //get the value into var v. call FormatCurrency(v) and store it back
      var v = document.getElementById("TextBoxA_TextBox").value;
  document.getElementById("TextBoxA_TextBox").value = FormatCurrency(v);

      v = document.getElementById("TextBoxB_TextBox").value;
  document.getElementById("TextBoxB_TextBox").value = FormatCurrency(v);

  v = document.getElementById("TextBoxC_TextBox").value;
  document.getElementById("TextBoxC_TextBox").value = FormatCurrency(v);

      //etc... all the way til my last TextBox
 }

Is it possible to optimize this code?  In other words, does anyone know any slick loops/operations/etc. to perform this same task?

You can loop through the elements using a for loop:

for(var i = 65; i <= 78; i++) {
    var box = document.getElementById("TextBox" + String.fromCharCode(i) + "_TextBox");
    box.value = FormatCurrency(box.value);
}


Brainfuck code optimization in Python I wrote a program to automatically optimize my Brainfuck source files. It works well, but the optimizations are only trivial ones and probably there are still some other points to improve.

def main():

    print "Enter the path of a brainfuck source file you want to optimize"
    path = raw_input()
    if path in ('exit', 'quit', 'close'):
        exit()
    f = open(path, 'r')
    text = f.read()
    f.close()
    cmd = '' # removing comments and whitespaces
    for c in text:
        if c in ('+', '-', '<', '>', '.', ',', '[', ']'):
            cmd += c
    found = True
    while found:
        length = len(cmd)
        found = False
        for i in range(length-1):
            if cmd[i:i+2] in ('+-', '-+', '<>', '><'):
                cmd = cmd[0:i] + cmd[i+2:length] # removing useless instruction pairs
                found = True
    ending = ''
    for i in range(len(path)):
        if path[i] == '.':
            ending = path[i:len(path)]
            path = path[0:i]
            break
    path = path + '_opt' + ending
    f = open(path, 'w')
    f.write(cmd)
    f.close()

if __name__ == '__main__':
    while True:
        main()

EDIT:
I'm considering about removing whole brackets, if it is obvious that they are never called. This is the case, if an opening bracket is directly after a closing bracket (][), because the first loop only breaks if the current cell value is zero and that means, that either the pointer or the cell value has to change to make the second loop work. However, unlike the other optimizations, I can't just remove ][, it should remove everything between [and the matching ] (including [ and ]). Are there any other cases I could remove unnecessary brainfuck code?

Some quick remarks:

You should be using the with..as construct when opening a file to ensure it is always closed, even if some intermediate code raises an exception:

with open(path) as f:
    text = f.read()
...
with open(path, 'w') as f:
    f.write(cmd)

I would save the comment commands in a set and outside of the loop. in is O(1) for sets and O(n) for tuples (even though n=4 here).

cmds = {'+-', '-+', '<>', '><'}
found = True
while found:
    length = len(cmd)
    found = False
    for i in range(length-1):
        if cmd[i:i+2] in cmds:
            cmd = cmd[0:i] + cmd[i+2:length] # removing useless instruction pairs
            found = True

I would do the same with the longer command tuple before that:

comment_codes = {'+', '-', '<', '>', '.', ',', '[', ']'}
for c in text:
    if c in comment_codes:
        cmd += c

You could also use a generator comprehension and str.join here:

comment_codes = {'+', '-', '<', '>', '.', ',', '[', ']'}
cmd = "".join(c for c in text if c in comment_codes)

Your building of the output filename can be simplified using split and could use str.format:

name, ext = path.split('.')
path = '{}_opt.{}'.format(name, ext)


Password Cracker (brute force) THIS IS JUST FOR LEARNING PURPOSES. I WON'T BE USING THIS TO HACK ANYONE

Here is a small algorithm I made to crack a passwork. This is just the background of a more complex code I'm going to make.

To be honest, this code is TERRIBLY written, so I need help on making it more efficient and flexible. It is limited to 5-characters password and not easy to expand.

#include<algorithm>
#include<iostream>
#include<string>
#include<vector>

std::string checkCharacters = {
    '%',
    'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',
    'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',
    '1','2','3','4','5','6','7','8','9','0'
};

std::string search(std::string password) {
    for (int a = 0; a < checkCharacters.size(); a++)
        for (int b = 0; b < checkCharacters.size(); b++)
            for (int c = 0; c < checkCharacters.size(); c++)
                for (int d = 0; d < checkCharacters.size(); d++)
                    for (int e = 0; e < checkCharacters.size(); e++)
                    {
                        std::string checkString = "";
                        checkString += checkCharacters[a];
                        checkString += checkCharacters[b];
                        checkString += checkCharacters[c];
                        checkString += checkCharacters[d];
                        checkString += checkCharacters[e];

                        checkString.erase(std::remove(checkString.begin(), checkString.end(), '%'), checkString.end());

                        if (checkString == password)
                            return checkString;
                    }

    return "TARGET_PASSWORD_NOT_FOUND";
}

int main() {
    while (1) {
        std::string password;
        std::cout << "Enter password : ";
        std::cin >> password;
        std::cout << "Your password is : " << password << std::endl;
        std::string attempt = search(password);
        std::cout << attempt << std::endl << std::endl;
    }
}

This is a critique, and a rewrite of the code you showed, that is, the code for checking if a given password only contains characters from a given character set; for an actual brute force password cracker, see e.g. this  question.

I think you should look at the problem a bit differently.

Why don't you go through the characters in the password, and check if every character is in the set of allowed characters?
If you get that every character of the password is in the set of allowed characters, then the password is valid, otherwise it isn't.

I'd implement this in the following way.

#include<algorithm>
#include<iostream>
#include<string>
#include<vector>
#include<set>
#include<numeric>

const static std::set<char> charSet = {
    '%',
    'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z',
    'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z',
    '1','2','3','4','5','6','7','8','9','0'
};

bool checkPassword(const std::string password) {
  return std::accumulate( password.begin(), password.end(), true,
    [](bool acc, char c) { return acc && charSet.find(c) != charSet.end(); } );
}

int main() {
  for (;;) {
    std::string password;
    std::cout << "Enter password : ";
    std::cin >> password;
    std::cout << "Your password is : " << password << std::endl;
    if(checkPassword(password)) {
      std::cout << "VALID" << std::endl << std::endl;
    } else {
      std::cout << "INVALID" << std::endl << std::endl;
    }
  }
  return 0;
}

This will work with passwords of any length.
Set lookups have logarithmic complexity, so checking a password of length \$n\$ given a character set of size \$m\$ should be \$\mathcal O(n\cdot \log(m))\$.


How safe is my MySQL query? I am using PDO for the first time in my project.  In my previous project someone suggested me to use PDO as my queries were wide open to inject.  I am pasting a sample code of my project.  Can you please let me know how safe my code and query is?

<?php
require('config.php');
try {
    $pid = $_GET['pid'];
    $conn = new PDO("mysql:host=" . $DB_HOST . ";dbname=" . $DB_NAME, $DB_USER, $DB_PASS);
    $sql = "SELECT * FROM packages_to_be_shipped_on_bremail_address WHERE package_id=$pid";

    $q = $conn->prepare($sql);
    $q->execute();

    $q->bindColumn(1, $pid);
    $q->bindColumn(2, $custemail);
    $q->bindColumn(3, $shipcompany);
    $q->bindColumn(4, $deliverydate);
    $q->bindColumn(5, $trackingid);
    $q->bindColumn(6, $destaddress);
    $q->bindColumn(7, $destzip);
    $q->bindColumn(8, $status);
    $q->bindColumn(9, $rate);
    $q->bindColumn(10, $rates_sent_status);
    $q->bindColumn(11, $customer_response);

    while($q->fetch()){
        echo "Shipping Company - ".$shipcompany."

";
        echo "Expected Delivery Date - ".$deliverydate."

";
    }
}
catch(PDOException $e) {
    echo $e->getMessage();
}
?>

Not safe at all, I'm afraid. Sure, you're calling PDO::prepare, to create a prepared statement, and that's all fine and dandy, but you are directly concatenating unsanitized user data into the string form which the query is prepared:

 $pid = $_GET['pid'];
 $sql = "SELECT * FROM packages_to_be_shipped_on_bremail_address WHERE package_id=$pid";

What if I were to pass any of the following values as "pid":

1 OR package_id > 0;
//or:
?

That would mean that, in the first case, you're executing the query:

SELECT * FROM packages_to_be_shipped_on_bremail_address WHERE package_id = 1 OR package_id > 0;
-- which translates to:
SELECT * FROM packages_to_be_shipped_on_bremail_address;

The second case is, in a way, even more subtle:

SELECT * FROM packages_to_be_shipped_on_bremail_address WHERE package_id = ?

This is a perfectly valid string to be passed to the PDO::prepare method. The ? is a placeholder for a value that will be bound later on in the code (at least, that's what PDO, and MySQL expects).
That means that your code will won't see anything wrong with the query, but once PDOStatement::execute is called, it'll throw an exception saying there is a problem. Something along the lines of "PDOStatement::execute expects one parameter, received null"
Just check the docs, and check the examples of how PDO::prepare should be used. What you're after should be:

$stmt = $pdo->prepare(
    'SELECT *
     FROM packages_to_be_shipped_on_bremail_address
     WHERE package_id = :pid'
);
$stmt->execute(
    array(
        ':pid' => $pid
    )
);
while($row = $stmt->fetch(PDO::FETCH_OBJ))//or FETCH_ASSOC
{
    echo 'Shipping Company - ', $row->shipcompany,
          '

Expected Delivery Date ', $row->deliverydate, '

';
}

Before you ask: Yes, those are comma's in the echo statement. echo is a language construct that works pretty similarly to C++'s std::COUT. You can push multiple values to it, separated by comma's.
The upshot is that PHP can then skip the string concatenation step, making the statement more efficient.
The downside? Well, there isn't one AFAIK.

Anyway: How does this produce safer queries? Well, that's easy: you send the query, without the user-supplied data to the server. MySQL can already compile and optimize the query. Often, MySQL will even be able to pre-fetch some of the data you need.
When you execute the query, and provide the data that makes up the WHERE clause (or other parts of your query), then MySQL can escape them properly, and since the query is already compiled, there's no real danger of the user supplied data being able to alter the query itself. This is just a short, very basic, and incomplete explanation of prepared statements. For more details: ask Google.

Other Niggles
There are some other issues that should be fixed, too:

$pid = $_GET['pid'];

What if $_GET isn't set, or the pid parameter wasn't passed? In that case, PHP will emit a notice (undefined index or something). Simply check before you attempt to assign:

if (!isset($_GET['pid']))
    exit();//or do something else
$pid = $_GET['pid'];

Avoid SELECT * FROM queries as much as you can. Sure SELECT * is quick and easy to write, but it does what it says on the tin: it selects every field. You only seem to be using 2 fields per row.
Why, then, do you feel compelled to select everything? Wouldn't:

SELECT shipcompany, deliverydate FROM your_wacky_tbl_name WHERE package_id = :pid

Work, too? As an added bonus: it's easier to see/understand for anyone maintaining this code what the query does, and what it selects. If you need some other field later on, then you can simply add that to the query.

Next, you've wrapped everything in a try block. Probably expecting PDO to throw up exceptions if any of the calls fail (PDO::prepare, PDOStatement::execute and PDOStatement::fetch or even that PDOStatement::bindColumn call that really serves no purpouse here, IMO).

This isn't going to work as you expect it to, because you haven't set PDO's error-mode so that it will throw exceptions. To do so, either pass an array to the constructor or call PDO::setAttribute:

$pdo = new PDO($dsn, $user, $pass,
    array(
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
    )
);
//or
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

See a list of attributes that you can set, either by passing them as an assoc array to the constructor, or by calling PDO::setAttribute.

Pokémon Exception Handling
At the moment, you're just blindly catching any of the possible PDOExceptions that might be thrown. This is called Pokémon Exception Handling, when you "gotta catch 'em all". Another term you could use for this is code smell.

Depending on what part of your code that encounters problems, you should handle that problem differently: if the new PDO() line failed, then stop there but whatever you do: You don't echo the message to the user, as it is likely to contain information about your server.
Log the exception, and either throw up a new, more general RuntimeException('Server says no') exception.

If the PDOStatement calls fail, you can be a tad more specific in the error message you send to the user, but again: you don't show them the PDOException message. That message is likely to reveal information on your database or table.
Log the message, and present the user with a polite message, saying that you couldn't find his/her $_GET['pid'] product.

Basically, if you're going to work with exceptions, then have a handler registered that logs the exceptions that are thrown and present the user with a generic error message, hiding your server layout as much as possible.
It's easy to do, and increases the overall security of your application a bit.


Fisher-Yates shuffle in Haskell This is the Haskell version of my recursive Fisher-Yates shuffle in JS.

import System.Random

pick :: Int -> [a] -> ([a],[a])
pick _ []     = ([],[])
pick 0 (x:xs) = ([x],xs)
pick i (x:xs) = (,) <$> fst <*> ((x:) . snd) $ pick (i-1) xs

shuffle :: [a] -> IO [a]
shuffle = runner =<< length
          where
          runner :: Int -> [a] -> IO [a]
          runner 0 xs = return xs
          runner i xs = randomRIO (0, i-1) >>= \r -> let (y,ys) = pick r xs
                                                     in (y ++) <$> runner (i-1) ys

The pick function fetches the i'th item in a the list as a singular list alongside a list with the remaining items. ie pick 3 [1,2,3,4,5,6,7] = ([4],[1,2,3,5,6,7]).

Although the above code is a version of the modern method which is claimed to work in O(n) this pretty fast shuffle code still seems to work in exponential time (i think).

Prelude Main> take 10 <$> shuffle [1..100000]
[20141,12487,79977,44600,14825,86744,65941,84737,97850,21214]
(0.57 secs, 334,790,152 bytes)

Prelude Main> take 10 <$> shuffle [1..1000000]
[698377,452503,263121,737622,550957,927399,453318,657374,728367,775039]
(6.81 secs, 3,782,234,776 bytes)

I have two questions.

Can the time complexity improved while using the List type?
I am not fully in control of Haskell's performance gimmicks like unboxed types and such. Can it's performance boosted by means of such Haskell tools or what data type would be the best?

First, let me ungolf and simplify your code slightly, so it will be easier for me to read:

import System.Random

pick :: Int -> [a] -> (a,[a])
pick 0 (x:xs) = (x, xs)
pick i (x:xs) = let (y,ys) = pick (i-1) xs
                in (y, x:ys)
pick _ []     = error "pick: Invalid index"

shuffle :: [a] -> IO [a]
shuffle lst = runner (length lst) lst
  where
    runner :: Int -> [a] -> IO [a]
    runner 0 xs = return xs
    runner i xs = do
      r <- randomRIO (0, i-1)
      let (y,ys) = pick r xs
      (y:) <$> runner (i-1) ys

Every call to pick is O(i), since you need to iterate through on average half of the array to extract the value and move it to the front. In when calling pick, i is on average equal to n/2 (where n is the length of the list). runner is called recursively n times.

So in conclusion, pick has time complexity O(n) and shuffle has time complexity O(n^2), which is indeed much worse than linear, but not quite exponential as you feared. There are also some minor issues with your functions not being tail recursive, but that should only affect constant factors and memory usage, not complexity.

This also matches your measurements, since 0.57 * 10^2 ≈ 6.81

If you compare it to the modern method which you linked, that code instead uses a swap operation which is O(1), which gives you the promised O(n) complexity.

On a side-note, your javascript version of the algorithm has the same problem, since Array.splice is an O(n) operation.

So, to your questions:

Can we improve the time complexity while using the list type?

No. Not unless we change the algorithm to something different, which in a way you have already done in both your implementations by not using the swap operation. An O(1) swap operation for lists is not available, since all random access costs O(n). You can however easily fix it in your js-version (see here).

If you do want to use a different algorithm, there are O(n*log n) algorithms that works on lists, which is not quite linear, but it's the best we can get for lists. You could also use the same algorithm with a Map, which has O(log n) swap operations.
Unboxed types and similar gimmicks will only only help with constant factors, not algorithmic complexity. However, what would help a lot is to use a mutable array or vector. They do have an O(1) swap operation. They come in both an IO version and an ST version which can be used from pure functions.

There are a number of different implementations of a random shuffle function on this page, including Fisher-Yates with both a Map (O(n*log n)) and a mutable Array (O(n)).


Rails guestbook with new, index, and Kaminari pagination in the same method I created a guestbook that on one page displays both the form for a new entry and all of the entries that have been created so far.

I am trying to use Kaminari for pagination for the entries.

Right now this code is working on my local machine, but I just have that feeling that it could be improved/refactored, so I wanted to post it here for people's opinions.

My controller:

class GuestbooksController < ApplicationController
    def index
        @guestbooks = Guestbook.all
    end

    def new
        @guestbook = Guestbook.new
        @guestbooks = Guestbook.all
        @guestbooks = Kaminari.paginate_array(@guestbooks).page(params[:page]).per(5)
    end

    def create
        @guestbook = Guestbook.new(guestbook_params)
        @guestbooks = Guestbook.all
        @guestbooks = Kaminari.paginate_array(@guestbooks).page(params[:page]).per(5)

        if @guestbook.save
            flash.now[:notice] = "Thanks for taking the time to write us! We greatly appreciate it!"
            render :new
        else
            flash.now[:notice] = "Your message failed to post, please try again"
            render :new
        end
    end

    private
    def guestbook_params
        params.require(:guestbook).permit(:name, :email, :message)
    end
end

new.html.erb template:

    <%=paginate @guestbooks%>

    <% @guestbooks.each do |g| %>

<%= g.name %>, <%= g.created_at %>

        <%= g.message %>

-----------------------------------------------------------------------------------------------------------------------

    <% end %>

<%= simple_form_for @guestbook, url: guestbooks_path, method: :post do |f| %>

<%= f.input :name %>

<%= f.input :email %>

<%= f.input :message %>

<%= f.button :submit %>

<% end %>

Routes:

 resources :guestbooks, only: [:new, :create]
  get "/guestbooks/index", to: "guestbooks#index"

Now I know my routes file could look like this

resources :guestbooks, only: [:new, :create, :index]

but then my paths look strange:

        guestbooks GET  /guestbooks(.:format)             guestbooks#index
                   POST /guestbooks(.:format)             guestbooks#create
     new_guestbook GET  /guestbooks/new(.:format)         guestbooks#new

Yes, there's a lot that can be done differently.

Firstly, Ruby convention is 2 spaces of indentation. Not 4, not tabs.

Secondly, Guestbook isn't a great name for your model. You're modelling entries in a guestbook, not multiple guestbooks.

Mostly, though I fail to understand the structure of your pages. If you go to index you load all the entries. If you go to new you... also load all the entries. But then you do some pagination, which, in effect, discards all but 5 of the records you just loaded from the database. Imagine how well that'll work if you have thousands of entries...

But why is the new action listing and paginating anything? That's the job of the index action; to list things, to be an index. Sure, you can combine the two by having a form and a list on the same page, but in that case it should be on the index view.

I'd cut the controller down to this (keeping, for now, the Guestbook model name):

class GuestbooksController < ApplicationController
  before_action :load_guestbooks

  def index
    @guestbook = Guestbook.new
  end

  def create
    @guestbook = Guestbook.new(guestbook_params)

    if @guestbook.save
      redirect_to guestbooks_url, notice: "Thanks for taking the time to write us! We greatly appreciate it!"
    else
      flash.now[:alert] = "Your message failed to post, please try again"
      render :index
    end
  end

  private

  def load_page
    @guestbooks = Guestbook.page(params[:page])
  end

  def guestbook_params
    params.require(:guestbook).permit(:name, :email, :message)
  end
end

No new action whatsoever. I haven't used kaminari, but just going to its github page I see several examples of usage that are much cleaner than loading everything. The whole point of pagination is not do that.

For your view (which should be the index view), I would read up on class naming, semantics, and HTML in general, I'm sorry to say. For instance, the hr tag could save you a few dozen hyphens, and span1, span2 etc. are not useful names for anything - certainly not divs. And don't use br for layout; use CSS.

Also, look into partials, aka partial views. If a partial is properly used, the whole entries list can be rendered as just:

<%= render @guestbooks %>

Lastly for the view, simple_form does not need a url or method argument if they can be derived from the record. Which they can in this case.

For your routes, you only need this:

resources :guestbooks, only: [:index, :create]

The paths in your question do not look strange: Two of them are both called guestbooks_path because they do indeed have the same path - but the action they invoke depend on the method (POST or GET) used. If you POST to /guestbooks it'll invoke the create action; if you simply GET /guestbooks, it'll invoke the index action. That's the basic premise of routing in Rails.


Best way to check for one of two values in an array in PHP I want to see if one of two values (a, b) are in an array.  Here's my current thought:

$match_array = array('a','b');
$array_under_test = array('b', 'd', 'f');

if (array_intersect($match_array, $array_under_test)) {
  // Success!
}

Any better implementations?

If you want to verify that either value is in the $array_under_test, array_intersect may not be the best option. It will continue to test for collisions even after it finds a match.

For two search strings you can just do:

if (in_array('a', $array_under_test) || in_array('b', $array_under_test)) {
  // Success!
}

This will stop searching if 'a' is found in the $array_under_test.

For more than two values, you can use a loop:

foreach ($match_array as $value) {
  if (in_array($value, $array_under_test)) {
    // Success!
    break;
  }
}


Reformatting a method options hash I am writing an API wrapper and the endpoint takes dates in a very specific format.

The user of the API can pass in the parameters in whatever format they prefer, but regardless of what they pass in, I want to be able to clean up their input prior to submitting their query.

My question centers around the best way to update the options hash in place, and I have thought of a few possible ways to implement.

A helper method inside the class so you can overwrite options = reformat_hash(options)
A singleton on that specific variable

def options.clean_up!
  # see internals below
end

Or open up Hash and do the cleaning from the class

class Hash
  def clean_hash!
    self.each { |key, value|
      if value.is_a? Date
        self[key] = value.strftime('%Y-%m-%d %H:%M:%S')
      else
        self[key] = value.to_s
      end
    }
  end
end

so that I can just call it on whatever the variable may be named like:

def api_request(options={})
  options.clean_hash!
  # the options variable is now clean and I can pass it to the api
  HHTParty.get(path, :query => options).parsed_response
end

Is there a best practice for modifying or formatting hashes after they're passed into a method?

I feel like #3 is the neatest, but should I be worried about opening up Hash to do this?

Here, while it may look nice, the method clean_hash is not general enough to be valid across all Hashes. So adding a method such as clean_hash to all Hashes would only serve to increase the coupling which is bad. A second problem is that you are mutating your method argument which is almost never advisable.

The  solution is to define the clean method outside, perhaps as a part of your internal API object and call HHTParty.get(path, :query => clean(options)).parsed_response.

I would also define the clean method this way

def clean(opt)
  Hash[opt.collect{|k,v| [k,v.is_a? Date : v.strftime('%Y-%m-%d %H:%m:%S'):v.to_s ]}]
end


Handling lazy-loading webpages using vba After a long try I've been able to create a script in vba which can successfully handle webpages with lazy-load. It can reach the bottom of a slow loading webpage if the hardcoded number of the loop is set accurately. I tried with few such pages and found it working flawlessly. The one I'm pasting below is created using finance.yahoo site. It can parse the title of different news after going down to a certain level of that page according to the loop I've defined. Now, what I wanna expect to have is do the same thing without using hardcoded delay what I've already used in my script. Thanks in advance for any guidance to the improvement.

Here is what I've written:

Sub Web_Data()
    Dim IE As New InternetExplorer, html As HTMLDocument
    Dim storage As Object, posts As Object

    With IE
        .Visible = True
        .navigate "https://finance.yahoo.com/"
        Do While .readyState <> READYSTATE_COMPLETE: Loop
        Set html = .document
    End With

    Application.Wait Now() + TimeValue("00:00:005")

    For scroll_down = 1 To 10
        Set storage = html.getElementsByClassName("StretchedBox")
        html.parentWindow.scrollBy 0, 99999
        Application.Wait Now() + TimeValue("00:00:005")
    Next scroll_down

    For Each posts In storage
        Row = Row + 1: Cells(Row, 1) = posts.ParentNode.innerText
    Next posts
    IE.Quit
End Sub

Reference to add to the library:

1. Microsoft Internet Controls
2. Microsoft HTML Object Library

You may want to try this below. First is to try reduce loading times, especially images and videos when scrolling down. Then have a counter to count number of "StretchedBox" elements before the scroll down, then check up to a few more attempts until no more changes (I used 5 in code). You can remove the Debug lines in final version.

Note some "StretchedBox" isn't really an article headline (video related), you will need to work on those to filter out junk.

Option Explicit

Sub Web_Data()
    Const TITLES As String = "StretchedBox"
    Dim sh As Object, regval As String

    ' Change IE Options
    Set sh = CreateObject("WScript.Shell")
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Display Inline Images"
    sh.RegWrite regval, "no", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Enable Browser Extensions"
    sh.RegWrite regval, "no", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Play_Animations"
    sh.RegWrite regval, "no", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\EnableAlternativeCodec"
    sh.RegWrite regval, "no", "REG_SZ"

    ' Prepare IE
    Dim IE As New InternetExplorer, html As HTMLDocument
    Dim posts As Object, sText As String
    Dim TitlesCount As Long, NoChangesCount As Integer, Row As Long

    With IE
        .Visible = True
        .navigate "https://finance.yahoo.com/"
        Debug.Print Now, "Navigated Start"
        Do Until .readyState = READYSTATE_COMPLETE: Loop
        Debug.Print Now, "Navigate Complete"
        Set html = .document
    End With
    NoChangesCount = 0
    On Error Resume Next
    Do
        DoEvents
        Application.Wait Now() + TimeValue("00:00:02")
        ' Remove all the annoying video parts
        RemoveVideos html
        TitlesCount = GetClassCount(html, TITLES)
        Debug.Print Now, TitlesCount
        html.parentWindow.scrollBy 0, 99999
        If TitlesCount = GetClassCount(html, TITLES) Then
            NoChangesCount = NoChangesCount + 1
        Else
            NoChangesCount = 0
        End If
    Loop Until NoChangesCount = 5 ' If no changes for some attempts, assume end of dynamic page
    On Error GoTo 0
    Columns("A").ClearContents
    For Each posts In html.getElementsByClassName(TITLES)
        sText = WorksheetFunction.Clean(posts.ParentNode.innerText)
        'sText = Trim(posts.ParentNode.innerText)
        If Len(sText) > 0 Then
            Row = Row + 1
            Cells(Row, 1).Value = sText
        End If
    Next posts
    IE.Quit
    ' Restore IE Options
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Display Inline Images"
    sh.RegWrite regval, "yes", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Enable Browser Extensions"
    sh.RegWrite regval, "yes", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\Play_Animations"
    sh.RegWrite regval, "yes", "REG_SZ"
    regval = "HKCU\Software\Microsoft\Internet Explorer\Main\EnableAlternativeCodec"
    sh.RegWrite regval, "yes", "REG_SZ"
    Set sh = Nothing
End Sub

Private Function GetClassCount(Doc As HTMLDocument, ClassName As String) As Long
    GetClassCount = Doc.getElementsByClassName(ClassName).Length
End Function

Private Sub RemoveVideos(Doc As HTMLDocument)
    Dim oElement As Object
    For Each oElement In Doc.getElementsByClassName("yvp-main")
        oElement.innerHTML = ""
    Next
End Sub


Project Euler 81 (minimum path sum through a matrix) Problem Statement:

  In the 5 by 5 matrix below,

131   673 234 103 18
201   96  342 965 150
630   803 746 422 111
537   699 497 121 956
805   732 524 37  331

  the minimal path sum from the top left to
  the bottom right, by only moving to the right and down, is

131 → 201 → 96 → 342 → 746 → 422 → 121 → 37 → 331

  and is equal to 2427.

  Find the minimal path sum, in matrix.txt, a 31K text file containing a 80 by 80 matrix,
  from the top left to the bottom right by only moving right and down.

Algorithm:

I used a dynamic programming approach to this problem

Accumulate the sum for the top row (going right till the end)
Accumulate the sum for the left column (going down till the end)
For each (i, j) pair, I add the minimum of the left cell and the upper cell
The result is stored in the last (i, j) pair

Code:

from itertools import accumulate # Note this was added in Python 3.3

def min_path_sum(matrix):
    """Returns the minimum path sum of a matrix moving only down and right"""
    size = len(matrix)
    if size == 0:
        return 0

    matrix[0] = list(accumulate(matrix[0][i] for i in range(size)))

    for i in range(1, size):
        matrix[i][0] += matrix[0][i-1]

    for i in range(1, size):
        for j in range(1, size):
            matrix[i][j] += min(matrix[i-1][j], matrix[i][j-1])
    return matrix[size-1][size-1]

if __name__ == '__main__':
    with open("matrix.txt") as file:
        matrix = []
        for line in file:
            matrix.append(list(map(int, line.strip().split(","))))
        print(min_path_sum(matrix))

I'm looking for a general code review.

That's a nice, elegant algorithm! I can only nitpick.

file is not a good name for a file handle because it shadows a built-in with the same name. (I use fh for this kind of thing.)

It would be good to extract the file reading logic from the if __name__:

def read_matrix_from_path(path):
    with open(path) as fh:
        return [list(map(int, line.strip().split(","))) for line in fh]

if __name__ == '__main__':
    print(min_path_sum(read_matrix_from_path("matrix.txt")))

Perhaps most importantly, you can blend this loop into the other one:

for i in range(1, size):
    matrix[i][0] += matrix[0][i-1]

Like this:

for i in range(1, size):
    matrix[i][0] += matrix[0][i-1]
    for j in range(1, size):
        matrix[i][j] += min(matrix[i-1][j], matrix[i][j-1])


Map over object values and JSON parse Here's a small function to check if the property starts with [ or { and parse the value from a sting into JSON or an array. Properties might be a string or intiger also.

import { mapValues } from 'lodash'

export function resolveValues (data) {
  return mapValues(data, item => {
    if (item.match(/^\[/) || item.match(/^\{/)) return JSON.parse(item)
    return item
  })
}

The very first concern is what if item just happens to be a string starting with [ or {?

How about instead of testing for [ and {, why not try parsing it? And if it fails, just return as is? This way, you don't need half-hearted checks.

export function resolveValues (data) {
  return mapValues(data, item => {
    try{ return JSON.parse(item) }
    catch (e) { return item; };
  })
}


Simple app to take username and password from the user I'm beginner at programming (I guess it's quite obvious)  and this is my small "app" I tried to make, that takes username and password from the user. Could you give me some advice on how can I improve it?

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;

namespace Login
{
    class Program
    {

        static void Main(string[] args)
        {
            Initialization();
        }

        public static void Initialization()
        {
            int x = 0;
            int choice = 0;

            while (x < 2)
            {
                Console.Clear();

                Console.Write("Username: ");
                string username = Console.ReadLine();

                Console.Write("\nPassword: ");
                string password = Console.ReadLine();

                Console.WriteLine();

                string option1 = "Login";
                string option2 = "Exit";

                Console.WriteLine(option1 + "\n" + option2);

                while (x < 2)
                {
                    ConsoleKeyInfo info = Console.ReadKey();
                    if (info.Key == ConsoleKey.UpArrow)
                    {
                        Console.Clear();
                        Console.ResetColor();

                        Console.WriteLine("Username: {0}", username);
                        Console.WriteLine("\nPassword: {0}\n", password);

                        Console.ForegroundColor = ConsoleColor.Red;
                        Console.WriteLine(option1);

                        Console.ResetColor();
                        Console.WriteLine(option2);

                        choice = 1;
                    }
                    else if (info.Key == ConsoleKey.DownArrow)
                    {
                        Console.Clear();
                        Console.ResetColor();

                        Console.WriteLine("Username: {0}", username);
                        Console.WriteLine("\nPassword: {0}\n", password);

                        Console.ResetColor();
                        Console.WriteLine(option1);

                        Console.ForegroundColor = ConsoleColor.Red;
                        Console.WriteLine(option2);
                        choice = 2;

                    }

                    if (info.Key == ConsoleKey.Enter && choice == 1)
                    {
                        if (Login(username, password) == true)
                        {
                            Console.WriteLine("Welcome");
                            x = 2;
                        }
                        else
                        {
                            Console.WriteLine("Wrong password!");
                            Console.ReadLine();
                            break;
                        }

                    }
                    else if (info.Key == ConsoleKey.Enter && choice == 2)
                    {
                        Shutdown();
                    }
                }
            }

        }
        public static void Shutdown()
        {
            Console.ResetColor();
            System.Environment.Exit(0);
        }
        public static bool Login(string username, string password)
        {
            if(username == "asd" && password == "asd")
            {
                Console.WriteLine("Welcome");
                return true;
            }
            else
            {
                return false;
            }
        }
    }
}

Just a few things:

Don't use \n as a literal, we use Environment.NewLine instead.
Don't use "string concatenation" (string + string), instead use string.Join, string.Format or a StringBuilder. I.e.:

Console.WriteLine(option1 + "\n" + option2);

Should be:

Console.WriteLine("{0}{1}{2}", option1, Environment.NewLine, option2);

Just as well, you should, ideally, switch this to two Console.WriteLine statements:

Console.WriteLine(option1);
Console.WriteLine(option2);

A method should have on responsibility (SRP):

public static bool Login(string username, string password)
{
    if(username == "asd" && password == "asd")
    {
        Console.WriteLine("Welcome");
        return true;
    }
    else
    {
        return false;
    }
}

The Console.WriteLine statement does not belong there, Login should simply return true or false in this case.


Sieve of Eratosthenes in Java Here you can find my implementation of sieve of Eratosthenes in Java.

/**
 * Return an array of prime numbers up to upperLimit
 * using sieve of Erastosthenes.
 * @param upperLimit
 * @return array of prime numbers up to upperLimit
 */
public static int[] sieve(int upperLimit) {
    // for corner cases
    if (upperLimit < 2) {
        return new int[0];
    }

    if (upperLimit == 2) {
        int[] result = {2};
        return result;
    }
    //--

    int arrLength = ((upperLimit - 3) / 2) + 1;

    // This arrray will be used to perform the sieve
    int[] temp = new int[arrLength];

    // Counter for number of primes encountered
    int numPrimes = 1; // because of 2

    // populate temp array with with 3,5,7,...,upperLimit
    for(int i=3; i <= upperLimit; i=i+2) {
        temp[(i-3) / 2] = i;
    }

    // Perform sieve
    for(int i=0; i < arrLength; i++ ) {
        int num = temp[i];
        if (num == 0) continue;
        numPrimes++;
        for (int k = i+num; k < arrLength; k=k+num) {
            temp[k] = 0;
        }
    }

    // Create result array
    int[] result = new int[numPrimes];
    result[0] = 2;
    int currentIndex = 1;

    for(int i: temp) {
        if (i!=0) result[currentIndex++] = i;
    }

    return result;
}

How does it look?

A few notes:

Right now you are using int[]s to represent the sieve.  However, the int data type is internally stored as 32-bits, which is a waste of space.

Use java.util.BitSet to represent the sieve. It provides a vector of bits that grows as needed. All bits start out as 0, and we can set and clear a bit at any index. It uses only one bit per entry. This requires slightly more computation per access for address computation and bit manipulation, but is much more compact, and utilizes the data cache more efficiently. For larger ranges of integers, we expect memory access time to dominate computation time, making this the ideal solution to represent the sieve.

To save additional space and time, at the cost of some additional complexity, we can choose not to represent even integers in our sieve (since they are divisible by 2). Instead, the element at index k will indicate the primarily of the number 2k + 3. We abstract this complexity away by wrapping BitSet in a class exposing special indexer methods.

Right now you are using i = i + 2, which could be simplified to i += 2.

Final code:

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

public class Sieve
{
    private BitSet sieve;

    public Sieve(int size)
    {
        sieve = new BitSet((size + 1) / 2);
    }

    public boolean isComposite(int k)
    {
        assert k >= 3 && (k % 2) == 1;
        return sieve.get((k - 3) / 2);
    }

    public void setComposite(int k)
    {
        assert k >= 3 && (k % 2) == 1;
        sieve.set((k - 3) / 2);
    }

    public static List<Integer> sieveOfEratosthenes(int max)
    {
        Sieve sieve = new Sieve(max + 1); // +1 to include max itself
        for (int i = 3; i * i <= max; i += 2)
        {
            if (sieve.isComposite(i)) continue;

            // We increment by 2*i to skip even multiples of i
            for (int multiplei = i * i; multiplei <= max; multiplei += 2 * i)
                sieve.setComposite(multiplei);
        }

        List<Integer> primes = new ArrayList<Integer>();
        primes.add(2);
        for (int i = 3; i <= max; i += 2)
            if (!sieve.isComposite(i)) primes.add(i);
        return primes;
    }
}


Creating a popup of a modal with output I'm trying to create an if-else statement that pops up a modal with output in the body, and if there is no input detected then an alert pops up.

    //fizzbuzz
$("#btnfb").click(function () {
var numfiz = $("#fizz").val();
var numbuz = $("#buzz").val();
var outputarray = [];
for (var loop = 1; loop <= 100; loop++) {
if (loop % numfiz === 0 & loop % numbuz === 0) {
outputarray.push("Fizzbuzz");
}
else if (loop % numfiz === 0) {
outputarray.push("Fizz");
}
else if (loop % numbuz === 0) {
outputarray.push("Buzz");
}
else {
outputarray.push(loop);
}
$("fzbzout").html(outputarray.join(", "));
$('#myModel5').modal('show');
});

FizzBuzz is a popular interview question so following good programming practices are a good idea.

One key part of working with people on a larger project is readable code.  Your code is very difficult to read because it is all aligned at the left side.  There are also no spaces between things which can be valuable for grouping blocks of code.  Something like this would be much more likely to impress your potential employer:

// Bhaskar's fizzbuzz in JavaScript/jquery

$("#btnfb").click(function () {
    var numfiz = $("#fizz").val();
    var numbuz = $("#buzz").val();
    var outputarray = [];

    for (var loop = 1; loop <= 100; loop++) {
        if (loop % numfiz === 0 & loop % numbuz === 0) {
            outputarray.push("Fizzbuzz");
        } else if (loop % numfiz === 0) {
            outputarray.push("Fizz");
        } else if (loop % numbuz === 0) {
            outputarray.push("Buzz");
        } else {
            outputarray.push(loop);
        }
    $("fzbzout").html(outputarray.join(", "));
    $('#myModel5').modal('show');
});

By formatting the code this way it became immediately apparent that the for loop didn't have a closing }.  Even if you have to do this on a whiteboard I would still stick with formatting things like this.  For more detailed rules than "indent inside squigglies" check out Google's JavaScript style guide.

A second key part of working with others on software projects is providing useful comments.  I skip this on whiteboard exercises because I'm probably going to have to verbally walk someone through the code anyways.  But skipping it when I've got a keyboard available to me seems like a red flag that I would write code that cannot be maintained by anyone else.


Get list of indices from searching for values in list of lists I have a list of lists of numbers. The numbers are in sequential order, ranging from 0 to n and each number only appears once. I want to loop over all the numbers in the list of lists and return the index of the list containing that number.

list_of_lists = [[2,3,4,5,6],[0,1,7]]

desired_output = [1,1,0,0,0,0,0,1]

I came up with something, but I'm thinking there has to be a way to do this without doing a list comprehension for each number. Performance is an issue

def get_indices(list_of_lists):

    indices = [0]*len([n for l in list_of_lists for n in l])

    for i in range(len(indices)):
        indices[i] = [ k for k,n in enumerate(list_of_lists) if i in n][0]

    return indices

The most straightforward approach would be

    for listno, alist in enumerate(list_of_lists):
        for n in alist:
            indices[n] = listno

It surely gives better performance. Is it enough, I don't know.


Divide big class in subclasses I have a very big class Foo that should be divided into FooBar, FooUtility, FooBench, FooBranch. Each of this "subclasses" should have different methods. So that if Foo has:

public function methodA();
public function methodB();
public function methodC();
public function methodZ();

Then I should be able to divide them as I want. For example:

FooBar:
public function __construct($param);
public function methodA();

FooUtility:
public function __construct($param);
public function methodB();

FooBench:
public function __construct($param);
public function methodZ();

FooBranch:
public function __construct($param);
public function methodC();

But I should be able to call methodB() and methodZ() from the same class Foo.

How should I manage this division? Interfaces? Abstract classes? Inheritance? How?

With Composition/Aggregation.

first define your interface for Foo collecting all the public methods it should expose from the components:

interface Foo
{
   public function methodA();
   public function methodB();
   …
}

You could also create interfaces for the components and extend the Foo interface from those, e.g.

interface Foo extends FooBar, FooUtility
{}

Note: it's not strictly necessary to have an interface collecting the other interfaces or to have an interface for Foo at all. However, since you should program against an interface and not a concrete implementation, it's cleaner.

Then implement that in your conrete Foo implementation:

class FooImplementation implements Foo
{
   public function __construct(FooBar $fooBar, FooUtility $fooUtility)
   {
       $this->fooBar = $fooBar;
       $this->fooUtility = $fooUtility;
   }
   public function methodA()
   {
       return $this->fooBar->methodA();
   }
   public function methodB()
   {
       return $this->fooUtility->methodB();
   }
   …
}

Inject the various components through the ctor and then just have the methods on Foo delegate to the appropriate components.

You can use use a Factory or Builder to encapsulate creation of the complex Foo type. This will allow you to parametrize the subcomponents:

class FooFactory
{
    public function create($param1, $param2)
    {
        return new FooImplementation(
            new FooBar($param1),
            new FooUtility($param2)
        );
    }
}

Note that Factory and Builder are creational patterns. They may control when and how objects are created, so if you want to reuse one of the components in subsequent calls to create, you may add logic to manage the component lifecycles in the factory/builder.