Improving runtime of prime generation I just answered the question on project euler about finding circular primes below 1 million using python. My solution is below. I was able to reduce the running time of the solution from 9 seconds to about 3 seconds. I would like to see what else can be done to the code to reduce its running time further. This is strictly for educational purposes and for fun.

import math
import time

def getPrimes(n):
    """returns set of all primes below n"""
    non_primes = [j for j in range(4, n, 2)] # 2 covers all even numbers
    for i in range(3, n, 2):
        non_primes.extend([j for j in range(i*2, n, i)])
    return set([i for i in range(2, n)]) - set(non_primes)

def getCircularPrimes(n):
    primes = getPrimes(n)
    is_circ = []
    for prime in primes:

        prime_str = str(prime)
        iter_count = len(prime_str) - 1

        rotated_num = []
        while iter_count > 0:
            prime_str = prime_str[1:] + prime_str[:1]
            rotated_num.append(int(prime_str))
            iter_count -= 1

        if primes >= set(rotated_num):
            is_circ.append(prime)

    return len(is_circ)

Python 2.7: 100 ms, pypy 1.8.0: 60 ms (Intel Core i7-3770K, 4x 3.50GHz):

import time

def rwh_primes2(n):
    # http://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n-in-python/3035188#3035188
    """ Input n>=6, Returns a list of primes, 2 <= p < n """
    correction = (n%6>1)
    n = {0:n,1:n-1,2:n+4,3:n+3,4:n+2,5:n+1}[n%6]
    sieve = [True] * (n/3)
    sieve[0] = False
    for i in xrange(int(n**0.5)/3+1):
      if sieve[i]:
        k=3*i+1|1
        sieve[      ((k*k)/3)      ::2*k]=[False]*((n/6-(k*k)/6-1)/k+1)
        sieve[(k*k+4*k-2*k*(i&1))/3::2*k]=[False]*((n/6-(k*k+4*k-2*k*(i&1))/6-1)/k+1)
    return [2,3] + [3*i+1|1 for i in xrange(1,n/3-correction) if sieve[i]]

def main():
    start = time.time()
    primes = set(rwh_primes2(1000000))
    circular_primes = set()
    for prime in primes:
        include = True
        s = str(prime)
        for n in xrange(len(s)-1):
          s = s[1:] + s[:1]
          if int(s) not in primes:
              include = False
              break
        if include:
            circular_primes.add(prime)
    print(len(circular_primes))
    print("Time (s)     : " + str(time.time()-start))

main()


Python converter: number-to-English - Project Euler 17 So I wrote this function to convert a given number to its interpretation in the English language as part of the Project Euler exercises.  It works fine, but I sense that it's rather sloppy and inelegant, especially for Python where many things can be done quickly in a couple of lines.  Any feedback on how to make this code more beautiful/Pythonic is appreciated!

NUMBER_WORDS = {
    1 : "one",
    2 : "two",
    3 : "three",
    4 : "four",
    5 : "five",
    6 : "six",
    7 : "seven",
    8 : "eight",
    9 : "nine",
    10 : "ten",
    11 : "eleven",
    12 : "twelve",
    13 : "thirteen",
    14 : "fourteen",
    15 : "fifteen",
    16 : "sixteen",
    17 : "seventeen",
    18 : "eighteen",
    19 : "nineteen",
    20 : "twenty",
    30 : "thirty",
    40 : "forty",
    50 : "fifty",
    60 : "sixty",
    70 : "seventy",
    80 : "eighty",
    90 : "ninety"
}

def convert_number_to_words(num):
    #Works up to 99,999
    num = str(num)
    analyze = 0
    postfix = remainder = None
    string = ""
    if len(num) > 4:
        analyze = int(num[0:2])
        remainder = num[2:]
        postfix = " thousand "
    elif len(num) > 3:
        analyze = int(num[0:1])
        remainder = num[1:]
        postfix = " thousand "
    elif len(num) > 2:
        analyze = int(num[0:1])
        remainder = num[1:]
        postfix = " hundred "
        if int(remainder) > 0:
            postfix += "and "
    elif int(num) in NUMBER_WORDS:
        analyze = int(num)
    else:
        analyze = int(num[0:1] + "0")
        remainder = num[1:]
        postfix = "-"
    string = NUMBER_WORDS[analyze]
    if postfix is not None:
        string += postfix
    if remainder is not None and int(remainder) > 0:
        return string + convert_number_to_words(remainder)
    else:
        return string

Here's one using modulo % and list joining that uses your original NUMBER_WORDS dict:

def int_to_english(n):
    english_parts = []
    ones = n % 10
    tens = n % 100
    hundreds = math.floor(n / 100) % 10
    thousands = math.floor(n / 1000)

    if thousands:
        english_parts.append(int_to_english(thousands))
        english_parts.append('thousand')
        if not hundreds and tens:
            english_parts.append('and')
    if hundreds:
        english_parts.append(NUMBER_WORDS[hundreds])
        english_parts.append('hundred')
        if tens:
            english_parts.append('and')
    if tens:
        if tens < 20 or ones == 0:
            english_parts.append(NUMBER_WORDS[tens])
        else:
            english_parts.append(NUMBER_WORDS[tens - ones])
            english_parts.append(NUMBER_WORDS[ones])
    return ' '.join(english_parts)

It works up to 999,999, but could be extended further with a little customisation.


DFS algorithm too slow I am practicing my coding on leetcode, and my submission, although correct for small cases, is timing out for large ones. Could you please let me know how to improve my code?

The question is as follows (https://leetcode.com/problems/longest-increasing-path-in-a-matrix/):

Given an integer matrix, find the length of the longest increasing path.
From each cell, you can either move to four directions: left, right, up or down. You may NOT move diagonally or move outside of the boundary (i.e. wrap-around is not allowed).

Here is my solution. I tried to use memoization, but it's just not fast enough: I think I'm missing something fundamental -- I do not have a CS background.

class Solution {
public:
    int longestIncreasingPath(vector<vector<int>>& matrix)
    {
        //base case
        if (matrix.size() == 0 || matrix[0].size() == 0)
        return 0;

        //create hash matrix populated with -1
        int height = matrix.size();
        int length = matrix[0].size();
        vector<int> temp(length,-1);
        vector<vector<int>> hash(height,temp);

        int maximum;
        //run longestpathfrom on all node
        for(int i=0; i<height; i++)
        {
            for(int j = 0; j<length; j++)
            {
                longestpathfrom(matrix,i,j,hash,maximum);
            }
        }
        return maximum;
    }

    int longestpathfrom(vector<vector<int>> matrix, int i, int j, vector<vector<int>>& hash, int& maximum)
    {
        //Returns the longest path from your current position
        //Also uses memoization via hash.
        //also keeps track of the max...

        //If you already calculated this value, just return it.
        if (hash[i][j]!=-1)
        return hash[i][j];

        //directions
        vector<vector<int>> dirs {{-1,0},{1,0},{0,-1},{0,1}};

        //pathlength:
        int pathlen = 0;

        //Take the max of longestpathfrom over all valid neighbours:
        for (int I=0; I<4; I++)
        {
            //Generate indices of where you are checking path length from
            int x = i+dirs[I][0];
            int y = j+dirs[I][1];

            //Check index validity:
            if (x<0 || y<0 || x>matrix.size()-1 || y>matrix[0].size()-1)
            continue;

            if (matrix[i][j]<matrix[x][y])
            pathlen = max(pathlen, longestpathfrom(matrix, x, y, hash, maximum));
        }

        //add 1 for the square you're already on
        pathlen++;

        //update hash
        hash[i][j]=pathlen;

        //update max
        maximum = max(maximum,pathlen);

        return pathlen;
    }
};

Your bruteforce method seems correct, there might be better ones available (using heuristics for instance), but let's stick with what you have.

First thing, you pass the matrix by value in each longestpathfrom and not by reference, probably a mistake given you do it for the rest. Given how many times you use this function and how big the matrix can be, it can make a huge difference.

Second thing, you can reduce the search space.
For instance, you know that if start is "a" and stop is "b", the longest path possible is "b-a". If, instead of starting by [0][0] then [0][1] ..., you start by the lowest value then the second lowest ..., you know you can stop searching once the maximum value of the matrix minus the current start value is lower than the length of the current maximum path.

Just a few words on coding style if I may.
Try to keep the style of the name of variables constant. Leetcode seems to use camelCase, so it's best if you stick to it (so longestPathFrom instead), or switch everything to C style (lowest_path_from).
Try not to use capital letters at the start/for variable names (your I).
Try to keep spacing consistent either you use variable=value or variable = value, but not both. I'd say the later is much better and readable.


Search a file system for files that match a given pattern My concerns with this code:

Is there a more efficient way to find files by pattern in Python?
Does os.walk() work in Windows?
Is this code pythonic?
Should search() be broken down into 2 functions?
Is it stupid to handle IOError exception by printing the error msg to the console?

#!/usr/bin/env python

'''
Finding files by pattern
'''

import os
import fnmatch
from os.path import join

def search(base, patt):
    ''' Yield files that match a given pattern '''
    for root, dirs, files in os.walk(base):
        try:
            for _ in [join(root, _) for _ in files if fnmatch.fnmatch(_, patt)]:
                yield _
        except IOError as error:
            print error

def example_use(d, p):
    ''' Example use of search function '''
    for _ in search(d, p):
        print _

if __name__ == '__main__':
    example_use('/var', '*.log')

Looks quite pythonic. However:

_ stands for don't care, akin to famous hunoz. It is not expected to be used in expressions later on; it is a placeholder.

Fot instance, I am totally lost in what

        for _ in [join(root, _) for _ in files if fnmatch.fnmatch(_, patt)]:
            yield _

does actually yield.

os.walk() works perfectly well on Windows
I don't see how do you want to break search into two functions. What would those functions do?
Any usable file matcher should have an option to specify file type (plain vs directory vs link vs etc), attributes, etc.


Find 3 elements that sum to zero in an int array Find 3 elements that sum to zero in an int array..  I would like to be reviewed on efficiency, style, and obviously if there is a bug I would like to know.

def sum_to_zero(a):
  ''' given an array of ints, find 3 elements that sum to 0 '''
  for i in a:
    for j in a[1:]:
      for k in a[2:]:
        if i + j + k == 0:
          return (i,j,k)

There's a bug:

>>> sum_to_zero([4, 17, -2])
(4, -2, -2)

But -2 only appears once in the input, so this is not a legitimate triple.
The docstring is not quite right. "find 3 elements that sum to 0" — and then what? The docstring should say explicitly what the function returns.
The docstring does not say what happens if no three elements sum to zero. In fact, the function returns None. This is a bad idea: it would be all too easy for the caller to forget to check it. Instead, the function should raise an exception.
Instead of three nested loops, use itertools.combinations:

for triple in combinations(a, 3):
    if sum(triple) == 0:
        return triple

Even when corrected, this function has runtime \$Θ(n^3)\$: it has to examine all possible triples. However, there is a \$Θ(n^2)\$ algorithm:

from collections import defaultdict
from itertools import combinations

class NotFound(Exception):
    pass

def sum_to_zero(a):
    """If three elements of the sequence a sum to zero, return them as a
    tuple. Otherwise, raise NotFound.

    """
    # Map from sum of two elements to a dictionary mapping the index
    # of one element to a set of indexes of the other.
    pair_sums = defaultdict(lambda:defaultdict(set))
    for i, j in combinations(range(len(a)), 2):
        d = pair_sums[a[i] + a[j]]
        d[i].add(j)
        d[j].add(i)
    for k, x in enumerate(a):
        for i, jset in pair_sums.get(-x, {}).items():
            if i != k:
                for j in jset:
                    if j != k:
                        return a[i], a[j], a[k]
    raise NotFound()


Find longest common string subsequence The task (also found at https://www.youtube.com/watch?v=10WnvBk9sZc):

  Write a function that takes two strings, s1 and s2, and returns the longest common subsequence of s1 and s2.

  "ABAZDC", "BACBAD" → "ABAD"
  "AGGTAB", "GXTXAYB" → "GTAB"

  The longest common subsequence is defined such as all of them appear in the same sequence in both strings, possiblywith other characters in between.

This is the solution I managed to come up with. I utilise the fact that once a full subsequence as been found, any subsequences that start within the boundaries of that subsequence will always be smaller in length, so I skip ahead.

const findFirstSequence = (s1, s2) => {
  let subsequence = ""

  let s1Idx = 0
  let s2Idx = 0

  for (; s1Idx < s1.length; ++s1Idx) {
    const s1Char = s1[s1Idx]
    for (let j = s2Idx; j < s2.length; ++j, ++s2Idx) {
      const s2Char = s2[j]
      if (s1Char === s2Char) {
        subsequence += s1Char
          ++s2Idx
        break
      }
    }
  }
  return subsequence
}

const removeDistinctChars = (s1, s2) => s1.split("").filter(c => s2.includes(c)).join("")
const removeDuplicates = (arr) => Array.from(new Set(arr))

const findAllSubsequences = (s1, s2) => {
  const s1NoDistinct = removeDistinctChars(s1, s2)
  const s2NoDistinct = removeDistinctChars(s2, s1)

  let i = 0

  const sequences = []

  while (i < s1NoDistinct.length) {
    const sequence = findFirstSequence(s1NoDistinct.slice(i), s2NoDistinct)
    i += sequence.length
    sequences.push(sequence)
  }

  return sequences
}

const findLongestSubsequence = (s1, s2) => {
  const a = findAllSubsequences(s1, s2)
  const b = findAllSubsequences(s2, s1)

  console.log('candidates:', [...a, ...b])

  return removeDuplicates([...a, ...b].sort((s1, s2) => s2.length - s1.length).filter((el, idx, arr) => el.length ===
    arr[0].length))
}

const test = (s1, s2) => console.log(findLongestSubsequence(s1, s2))

test("ABAZDC", "BACBAD")
test("AGGTAB", "GXTXAYB")
test("ALSKJDHJASLDH", "HNGFLKSHRADSKLASDJ")
test("aaaa", "aa")

Find the current max
There are some very simple improvements you can make to reduce the run time and the complexity.
Your code looks for sequences, puts them all in an array and then sorts and filters that array to find the longest. However if you check the length of each sequence as you find it and compare it against the longest you have found to that point you no longer need to keep an array of sequences, only the current longest.
This also helps reduce the amount of processing. When you know the length of the current longest sequence, you are able to know before calling findFirstSequence(s1,s2) if it will find a longer one. The function can not find a sequence longer than the shortest string you pass it so if either is shorter than the current max then skip the call.
Also in the function findFirstSequence(s1,s2) as you build the sequence you can workout how long it can possibly get well before you get the sequence. You can thus exit early from that function as well just by knowing the current longest sequence.
There are also two more places that you can exit early. When you first enter the function 'findLongestSubsequence` you can check if the string are the same. If they are ether one is thus the answer.
The same after the calls to removeDistinctChars(s1, s2) if s1NoDistinct === s2NoDistinct then you have the answer and can return the result then.
Some style points

Use semiColons, Javascript requires them, and if you dont add them it inserts them automatically. Unfortunately it does not always put them where you may think it will. If you can not list all the times ASI (Automatic Semicolon Insertion) can ignore the line end than you are best to put them in manually.

Watch your indentation. It's not bad, just one line is 2 space in too far. But good code does not have style errors.

Don't leave for loop segments empty. You have for (; s1Idx < s1.length; ++s1Idx) { which is poor style. for (s1Idx = 0; s1Idx < s1.length; ++s1Idx) { lets you know at a glance what the loop is starting at, rather than having to find where you set the value. In this case I would also say use i rather than s1Idx but that is more a personal preference.

A rewrite
So using your approach and just finding some early exits and avoiding repeated processing the following rewrite keeps track of the current longest sequence. Returning the first longest sequence found.
const findLongest = (s1, s2) => {
    const removeDistinct = (s1, s2) => s1.split("").filter(c => s2.includes(c)).join("");
    const findFirstSeq = (s1, s2) => {
        var seq = "", i, j = 0;
        for (i = 0; i < s1.length; i++) {
            const c = s1[i];
            while (j++ < s2.length) {
                if (seq.length + (s2.length - j - 2) < max) { return "" }
                if (c === s2[j - 1]) {
                    seq += c;
                    break;
                }
            }
        }
        return seq
    }
    const findSubseq = (s1, s2) => {
        if (s2.length <= max || s1.length <= max) { return maxSeq }
        while (s1.length && s1.length > max) {
            const seq = findFirstSeq(s1, s2);
            if (seq.length > max) {
                max = seq.length;
                s1 = s1.slice(max);
                maxSeq = seq;
            } else { s1 = s1.slice(1) }
        }
        return maxSeq;
    }
    var max = 0, maxSeq;
    if (s1 === s2) { return s1 }
    const s1D = removeDistinct(s1, s2);
    const s2D = removeDistinct(s2, s1);
    if (s1D === s2D) { return s1D }
    findSubseq(s1D, s2D);
    return findSubseq(s2D, s1D);
}


parsing a flat file and removing duplicates Have some code that I wrote to parse a flat file of IP address and other junk unwanted information. Just wanting to see if there was any way to make shorter or better.

#!/usr/bin/perl

use strict;
use warnings;

#opening the access_log flat file that contains a list of ip addresses with unwanted information after, and contains duplicate IPs
#example, "203.151.232.51 - - DateTime Junk URL Information"
open (YYY, "/home/access_log");
#assigning flat file to an array
my @incomingarray=<YYY>;

my $value = 0;

#splitting up the array and assigning variables to the pairs
foreach my $pair(@incomingarray) {
  (my $ip, my $junk) = split(/ - - /, $pair);
  $incomingarray[$value] = $ip;

  #autoincrementing the scalar
  $value++;
}

close (YYY);

#print @incomingarray;

#creating a hashlist from the array and using the map function to remove duplicates
my %IPlist = map { $_ => 1 } @incomingarray;

#creating a new array by using the keys function to loop through the hash list
my @distinctIP = keys %IPlist;

foreach (@distinctIP) {
  print "$_\n";
}

You should probably close the file as soon as you are finished.
Divorcing the open() and close() makes it harder to see maintain.
Move close() to here (just after you finish using the file).
open (YYY, "/home/access_log");
#assigning flat file to an array
my @incomingarray=<YYY>;
close (YYY);

In the main loop you add overwriten the current values with just the IP address I would not consider this good practice (though it works here because of the simplicity of the program). And since your next step is to convert the array into a hash why not do so directly in the loop.
  $incomingarray[$value] = $ip;

Why not just do:
  $IPlist{$ip} ++; // Increment the count that way you can count each IP.

There is no need to create a new array just to loop over them. You can use the keys directly in the foreach statement to loop over the hash.
my @distinctIP = keys %IPlist;
foreach (@distinctIP) {
  print "$_\n";
}

Just as easy to write:
foreach (keys %IPlist) {
  print "$_\n";
}

Two things no note about variables.
Using the automatic variables $_ and @_ make writing the code easier but (in my opinion) also can make the code harder to read and maintain (if overused). This is why perl has got the reputation of being a write once never read or understand again language. So please try and be explicit.
Also all the variables you declare are global scope (you seem to have fixed this). I know for this short program it does not make any difference but a habit you should get into is declaring variables with my. This scopes the variable to the current block (thus making them behave like variables in most other modern languages). By doing this you reduce problems associated with modifying global state accidentally and when you do drop my you know it is on purpose and thus affecting the global version.
Loading the whole array.
Personally I would not have loaded the file into the array and parse it. I would have just read the file line by line and done the appropriate parsing:
#!/usr/bin/perl

use strict;
use warnings;

my %IPList ;
open(YYY, "access_log");
while(my $line = <YYY>)
{
    chomp $line;
    (my $ip, my $junk) = split(/ - - /, $line);

    $IPList{$ip}++;
}
close(YYY);


Generate form and append to the body I'm writing a plugin and want it to have no dependencies. I create a form and append it to the page, but the code is really long. Does anyone have any suggestions to clean it up, perhaps shortening it? I feel like there must be a better way!

// Create elements
  var elements = [
      reporter_wrapper                = document.createElement('div'),
      reporter_close                  = document.createElement('button'),
      reporter_title                  = document.createElement('h1'),
      reporter_title_label            = document.createElement('label'),
      reporter_bug_title              = document.createElement('input'),
      reporter_priority_label         = document.createElement('label'),
      reporter_bug_priority           = document.createElement('select'),
      reporter_details_label          = document.createElement('label'),
      reporter_bug_details            = document.createElement('textarea'),
      reporter_user_label             = document.createElement('label'),
      reporter_user                   = document.createElement('input'),
      reporter_submit                 = document.createElement('button')];

  // // Add class names
  reporter_wrapper.className          = 'buggr__reporter';
  reporter_close.className            = 'buggr__reporter-close';
  reporter_title.className            = 'buggr__reporter-title';
  reporter_bug_title.className        = 'buggr__reporter-input';
  reporter_bug_priority.className     = 'buggr__reporter-select';
  reporter_bug_details.className      = 'buggr__reporter-textarea';
  reporter_user.className             = 'buggr__reporter-input';
  reporter_submit.className           = 'buggr__reporter-submit';

  // Input types
  reporter_user.type                  = 'text';
  reporter_bug_title.type             = 'text';

  // Populate title
  reporter_title.innerHTML            = 'Report a bug';

  // Populate label text
  reporter_title_label.innerHTML      = 'Bug title:';
  reporter_priority_label.innerHTML   = 'Bug priority:';
  reporter_details_label.innerHTML    = 'Bug details: <small>(Please include any specific steps to replicate)</small>';
  reporter_user_label.innerHTML       = 'Your name:'

  // Poulate submit button
  reporter_submit.innerHTML           = 'Submit Bug';

  // Populate select with options
  var select_options = ['Low', 'Medium', 'Hard'];
  for(var i = 0; i < select_options.length; i ++) {
    var option = document.createElement('option');
    option.value = select_options[i];
    option.innerHTML = select_options[i];
    reporter_bug_priority.appendChild(option);
  }

  // Create DOM tree
  for(var i = 1; i < elements.length; i ++) {
    reporter_wrapper.appendChild(elements[i]);
  }

  // Add event listeners
  reporter_close.addEventListener('click', this.close);
  reporter_submit.addEventListener('click', this.submit);

  // Append DOM tree
  document.body.appendChild(reporter_wrapper);

var elements = [
      reporter_wrapper                = document.createElement('div'),
      reporter_close                  = document.createElement('button'),
      reporter_title                  = document.createElement('h1'),
      reporter_title_label            = document.createElement('label'),
      reporter_bug_title              = document.createElement('input'),
      reporter_priority_label         = document.createElement('label'),
      reporter_bug_priority           = document.createElement('select'),
      reporter_details_label          = document.createElement('label'),
      reporter_bug_details            = document.createElement('textarea'),
      reporter_user_label             = document.createElement('label'),
      reporter_user                   = document.createElement('input'),
      reporter_submit                 = document.createElement('button')];

Hmm. An array that has variables that each are assigned a value. Hmm. Oh wait; that sounds like an object!

Instead of reinventing the wheel and creating this array thing that's values are assigned to variables too, you should just create an object.

Here is what that would look like:

var reporter = {
    wrapper: document.createElement("div"),
    ...
};

I removed the part reporter_ from the key names because, now that they are in an object, you need not to worry about conflicting with other variable names.

There is practically no difference.

Let's take the last tip to the next level.

This time, let's create a map of names of the keys of the object, and then the type of HTML element to instantiate.

Here is what I mean:

var namesAndElements = {
    "wrapper": "div",
    "close": "button",
    ...
}

Excuse the poor variable name.

See the pattern? The keys are simply the part after reporter_ in the original array, and the values are the parameter passed into document.createElement.

Now, to create the object/map of elements, we simply need to create a loop that stores these keys and values into a new object/map.

Here is what I came up with:

var reporter = {};
for(var name in namesAndElements) {
    elements[name] = document.createElement(namesAndElements[name]);
}

Now, in order to access something from this object, you could write:

elements.details_label

As an example.

And you will get the element assigned to it.

reporter_wrapper.className          = 'buggr__reporter';
reporter_close.className            = 'buggr__reporter-close';
reporter_title.className            = 'buggr__reporter-title';
reporter_bug_title.className        = 'buggr__reporter-input';
reporter_bug_priority.className     = 'buggr__reporter-select';
reporter_bug_details.className      = 'buggr__reporter-textarea';
reporter_user.className             = 'buggr__reporter-input';
reporter_submit.className           = 'buggr__reporter-submit';

See something similar in the value that these classes are being assigned? If not, they are start with this:

"buggr__reporter"

Therefore, we can simplify this by creating a map (like above) of the elements whose classes need to be changed, and the value to change it to.

Here is what I came up with:

var newClasses = {
    "wrapper": "",
    "close", "-close",
    "title", "-title",
    "bug_title", "input",
    ...
}

Excuse the poor variable name.

Notice anything familiar about this object? The keys are keys of the first map we created, and the values are the extra part of the string that is not in the pattern I mentioned.

Now all we have to do is create a loop for setting these class names:

for(var element in newClasses) {
    reporter[element] = ("buggr__reporter" + newClasses[element]);
}


Acquiring an increasing sequence of numbers I'm trying to solve this question:

  Given a positive integral number n, return a strictly increasing
  sequence (list/array/string depending on the language) of numbers, so
  that the sum of the squares is equal to n².

  If there are multiple solutions (and there will be), return the result
  with the largest possible values:

Basically, a squared number deconstructed into smaller squares. However, my code only works for smalls numbers efficiently (20 being roughly the max) and is exponentially slower onwards. How can I develop this code?

I tried optimising it by reducing the memory usage by setting comb to take out any bits of irrelevant data.

import itertools as it
from math import sqrt

def decompose(n):
    squares = [i ** 2 for i in range(1, n) if (i ** 2)/2 < n ** 2]
    comb = [list(i) for i in (reduce(lambda acc, x: acc + list(it.combinations(squares, x)),
                                     range(1, len(squares) + 1), [])) if sum(i) == n ** 2]
    print [int(sqrt(i)) for i in max(comb)]

decompose(20)

I am a beginner with Python so any optimisation tips will be greatly appreciated.

I would suggest focusing on the nature of the answer. The requirement is to return the highest possible values, which also means the shortest sequence.

So I would suggest you start at n-1 and work your way down, checking increasingly long sequences until one sums to greater than n**2, in which case you can restart at n-2, etc.


Decorator to pass instance and class to method In some of my peewee-based ORM models, I need to access the instance as well as the respective class.
Since I caught myself always repeating the line cls = self.__class__ at the top of each such methods, I wrote a decorator for that:

def with_class(function):
    """Adds the instance's class as second
    parameter to the wrapped function.
    """

    @wraps(function)
    def wrapper(self, *args, **kwargs):
        """Wraps the given function."""
        return function(self, self.__class__, *args, **kwargs)

    return wrapper

Example use case:

class Location(_TerminalModel):
    """Location of a terminal."""

    address = CascadingFKField(Address, column_name='address')
    annotation = CharField(255, null=True)

    …

    @with_class
    def save_unique(self, cls, *args, **kwargs):
        """Saves the location if it is new or
        returns the appropriate existing record.
        """
        if self.annotation is None:
            annotation_selector = cls.annotation >> None
        else:
            annotation_selector = cls.annotation == self.annotation

        try:
            return cls.get((cls.address == self.address) & annotation_selector)
        except cls.DoesNotExist:
            self.save(*args, **kwargs)
            return self

I'd like to have critique on this solution and am open to alternatives.

I'd recommend not using value.__class__. This is as it's not guaranteed to return the type.

  type(x) is typically the same as x.__class__ (although this is not guaranteed – a new-style class instance is permitted to override the value returned for x.__class__).

Take:

class A:
    __class__ = 'something'

print(A.__class__)   # <class 'type'>
print(A().__class__) # 'something'
print(type(A()))     # <class '__main__.A'>

I also think just using type would be cleaner, and easier to understand.

def save_unique(self, *args, **kwargs):
    cls = type(self)


Changing button colors based on user input I am using C# to get the user's input of how many buttons to change the color of.  The buttons are in rows that have 1-10 button in each.  If the user enters 11, then the entire first row will be filled out and the first button on the second row will change color.  I have figured out how to accommodate for different numbers the user might enter, but hoping to get some advice on how to code better without using a large if, if else, statement.

internal void ChangeTileColor(ConnectorData connectorData, int j)
{
    int row = 0;
    if (j < 10)
    {

    }
    else if (j < 20)
    {
        j -= 10;
        row++;
    }
    else if (j < 30)
    {
        j -= 20;
        row += 2;
    }
    else if (j < 40)
    {
        j -= 30;
        row += 3;
    }
    else if (j < 50)
    {
        j -= 40;
        row += 4;
    }
    else if (j < 60)
    {
        j -= 50;
        row += 5;
    }
    Button button = this.tableLayoutPanel1.GetControlFromPosition(j, row) as Button;
    button.BackColor = Color.Lime;
}

This one's pretty straightforward. If you have rows of length rowLength, then the row is j / rowLength (note that with integer arithmetic this automatically rounds down to the nearest integer) and the column is j % rowLength.

Altough there was a very simple answer in this particular case, in general, if you look at a method like the one you wrote, the very repetitive code should stand out, and you should think "loop".

In fact, a good starting point is to just say, in plain English, what you're doing with what you've written. Something like:

  Starting from 10, and adding in increments of 10, I check if j is less than that number. If it is, I subtract the previous multiple of 10 from j, and add a tenth of that number to row. Otherwise I go on to check the next increment of 10.

That's a start, but a good general idea with programming is to say what you're doing, not how you're doing it. So the above might become:

  In order to check which row the item is in, I check each multiple of the row count, starting from the first, until I find the first one greater than the position. Then I record that the number of the row is that multiple - 1. Then I know that the column is the position minus the number of items in all the previous rows, which is the number of previous rows multiplied by the number of items per row.

A bit cumbersome in English, but the above is actually the exact basis of what you did. So the next task is to translate that as plainly into code as possible.

First, we look at "I check each multiple". That doesn't say "I check 1 * the row count, then I check 2 * the row count, then I check 3 * the row count". It expresses all of those in a single statement: each. So translating that as literally into code as possible, it's not a bunch of if statements, it's a single loop:

int i = 0; //Starting from the first...
do
{
    i++; //I check each...
    int rowMultiple = i*rowLength; //...Multiple of the row length
} while(true);  //Not sure when I stop yet

Next: "until I find the first one greater than the position":

int i = 0; //Starting from the first...
do
{
    i++; //I check each...
    int rowMultiple = i*rowLength; //...Multiple of the row length
} while(rowMultiple <= position);  //Until I find the first one greater than the position

Note that I renamed j to position, since that's what it is. Now we've found the desired i, and we know that to do with it:

int i = 0; //Starting from the first...
do
{
    i++; //I check each...
    int rowMultiple = i*rowLength; //...Multiple of the row length
} while(rowMultiple <= position);  //Until I find the first one greater than the position
int rowNumber = i - 1; //Then I know the row number is this multiple minus 1
int columnNumber = position - (rowNumber*rowLength); //And the column number is the position minus the number of items in previous rows

And there you have it! Of course you wouldn't really include those comments, those are just for in-line illustrations of how directly each line- in code- corresponds to what you're actually trying to do- in English. The fact that each line so clearly describes what it's doing without comments is a sign that you're on the right track.

From there, the solution with / and % is really just noticing that you can predict what the loop will produce without actually having to go through each iteration, and likewise knowing about the modulo (%) operator and thinking to use it, to simplify the column bit. But being able to clearly state what you want to do in English and turn that into concise, readable code is a far more general and wide-reaching skill.


Finding number of number pairs with given difference pairs() is a function which returns the total number of combinations whose difference is k.

     static int pairs(int[] a,int k) {
        int counter=0;
         for (int i : a.length ) {
           for(int j=i+1;j<a.length;j++){
                 if(a[i]-a[j]==k||a[i]-a[j]==-k){
                   counter++;
                 }
                }
           }

 return counter;
}

a is an array of numbers and k is the difference of numbers that are given by user.  I have to find the total number of combinations with a difference of k.  I have done this code, but I want a more optimized code.

Current complexity is \$O(n^2)\$, you can accomplish this with \$O(n \space log \space n)\$ instead by sorting the array first and then loop through the elements.

Let's say that you know that the array is sorted, and the array is for example 4 8 15 16 23 42 and you want to find the diff 7:

Let's initialize two variables, lowIndex, highIndex to both 0.

Now we compare index 0 to index 0, i.e. 4 and 4. It's less than the diff value (7) so we increase highIndex
Compare index 0 to index 1, 4 vs. 8, still less than 7, so highIndex++
Compare index 0 to index 2, 4 vs. 15, that's more than 7, so lowIndex++
Compare index 1 to index 2, 8 vs. 15, that's exactly 7, so we have a match and we do highIndex++
Compare index 1 to index 3, 8 vs. 16, that's more than 7, so lowIndex++
Compare index 2 to index 3, 15 vs. 16, that's less than 7, so highIndex++
Compare index 2 to index 4, 15 vs. 23, that's more than 7, so lowIndex++
Compare index 3 to index 4, 16 vs. 23, that's exactly 7, so we have a match and we do highIndex++
Compare index 3 to index 5, 16 vs. 42, that's more than 7, so lowIndex++
Compare index 4 to index 5, 23 vs. 42, that's more than 7, so lowIndex++
Compare index 5 to index 5, 42 vs. 42, that's less than 7, so highIndex++
highIndex has reached the end of the array, return the number of matches found.


wrote a blackjack game in python , would like you get notes on it I started learning python online and I wrote a blackjack game as my first little project.
I would like to get some review on it if possible. (how to make it better , simpler , etc)

from random import choice
cards = ['A', 2, 3, 4, 5, 6, 7, 8 , 9,'J','Q','K']*4

def get_card(name):
    card = choice(cards)
    cards.remove(card)
    print(f'{name} got {card}')
    return card

def hand(cards):
    global dealer_total
    dealer_total = 0
    dealer_card = convert(get_card('The dealer'),dealer_total)
    dealer_total += dealer_card
    total = 0
    card = convert(get_card('You'),total)
    total += card
    card = convert(get_card('You'),total)
    total += card
    if total == 21:
        print('Blackjack! You Won!')
        another_game()
    check_total(total)

def convert(card,total):
    if card == 'A':
        if total > 10:
            card = 1
        else: card = 11
    if card in ['K','Q','J']:
        card = 10
    return card

def check_total(total):
    if total == 21:
        print(f'You stand on a total of {total}. Lets see what the dealer gets:')
        dealer_hand(total,dealer_total)
    elif total > 21:
        print(f'You stand on a total of {total}. You lost!')
        another_game()
    else:
        another_card(total)

def another_game():
    another_game = input('Would you like to play again ?').lower()
    if another_game == 'yes':
        cards = ['A', 2, 3, 4, 5, 6, 7, 8 , 9,'J','Q','K']*4
        hand(cards)
    elif another_game == 'no':
        print('Goodbye !')
        quit()
    else:
        raise ValueError('You entered the wrong value')
        another_game()

def another_card(total):
    another = input(f'You stand on a total of {total}. Would you like another card ? (Yes/No)').lower()
    if another == 'yes':
        card = convert(get_card('You'),total)
        total += card
        check_total(total)
    elif another == 'no':
        print('OK , lets see what the dealer gets')
        print(f'Before fliiping the cards , the dealer stands on a total of {dealer_total}')
        dealer_hand(total,dealer_total)
    else:
        raise ValueError('You entered the wrong value')
        another_card(total)

def dealer_hand(total,dealer_total):
    card = convert(get_card('The dealer'),dealer_total)
    dealer_total += card
    if dealer_total > 21:
        print(f"Dealer's total is {dealer_total}. You won ! ")
        another_game()
    elif dealer_total > 16:
        print(f'Dealers total is {dealer_total}.The dealer must stand.')
        who_won(total,dealer_total)
    elif dealer_total < 17:
        print(f"Dealer's total is {dealer_total}. He must take another card.")
        dealer_hand(total,dealer_total)

def who_won(my_total,dealer_total):
    print(f'Your total is {my_total} , The dealers total is {dealer_total}')
    if my_total > dealer_total:
        print('You won !')
    elif my_total < dealer_total:
        print('You lost !')
    else:
        print('Its a tie !')
    another_game()

print('Hello !')
hand(cards)

I highly appreciate it , thank you !!

Naming things

Try to give proper names to things, such that they follow the usual conventions, are concise, unambiguous and to the point. There are a few things I would change:

Rename cards to deck, to signify that it isn't just a random bunch of cards, but the actual deck you are drawing from.
The function hand() is quite misnamed, it's not about managing just one hand of cards, but rather it implements the whole game. So perhaps play_blackjack() would be better?
The function convert() has a very generic name, that doesn't tell anything about what it is converting from or to. It would be better named convert_card_to_value().

In general, variables describe things, so their names should usually be a noun, whereas functions describe actions, so they should be verbs.

Try to reorganize the code into classes

Classes make it easier to organize your code. I can think of at least three classes that you should make: Card, Deck and Hand.

A Card is quite simple, it just is the color and number of the card, and has a member function to convert it to a value.

A Deck is a collection of cards, with member functions like shuffle(), draw() and so on.

A Hand is also a collection of cards, but has member functions add() and get_value(). The latter is important: you can't just add values of cards together to get the total value of a hand. Think about drawing an ace, 9, king in that order. Your current code converts card values the moment they are drawn, and only checks aces against the total so far. So your method would count this as 11 + 9 + 10 = 30, whereas the correct value is 1 + 9 + 10 = 20.

Avoid infinite recursion

Your code recursively calls itself indefinitely. While this might seem harmless at first, the problem is that you are using more and more stack space, until after enough games played you get a stack overflow. The proper way to deal with this is to structure some parts of your code as loops. For example, you want to have a function to play one game of blackjack, and that function should be called in a loop like so:

def another_game():
     while True:
          answer = input('Would you like to play again?').lower()
          if answer == 'yes':
              return True
          elif answer == 'no':
              return False
          else
              print('Please enter yes or no.')

...

print('Hello!')

while True:
    play_game();
    if not another_game():
        break

print('Goodbye!')
```


C++ overloading += operator in custom string class better implementation I am trying to learn C++, so I started coding a custom string class (using only c-style strings) to get familiar with concepts like operator overloading etc. in the case we have a pointer attribute. I wanted to know if there is a smarter/better/neater way to implement the += operator (or the others).
So I am basically defining a private char pointer to store the c-style string, two constructors (empty, char* arg), a copy constructor, a move constructor, a destructor (to deallocate the allocated space from the heap) and two operators (to concatenate strings).
mystring.h
#include <iostream>
#include <cstring>

class mystring
{
private:
    char* str;

public:
    mystring();
    mystring(char* str);
    mystring(const mystring &s);
    mystring(mystring &&s) noexcept;
    ~mystring();

    mystring& operator=(const mystring &s);
    friend std::ostream& operator<<(std::ostream &out, const mystring &s);

    mystring operator+(const mystring &s);
    mystring& operator+=(const mystring &s);
};

mystring.cpp
#include "mystring.h"

mystring::mystring() : mystring(nullptr) {};

mystring::mystring(char* str) : str{nullptr}
{
    if(str == nullptr){
        this->str = new char;
        this->str = '\0';
    }else {
        this->str = new char[strlen(str) + 1];
        strcpy(this->str, str);
    }
}

// Deep copy
mystring::mystring(const mystring &s) : str{nullptr}
{
    if(str == nullptr){
        this->str = new char;
        *(this->str) = '\0';
    }else {
        this->str = new char[strlen(s.str) + 1];
        strcpy(this->str, s.str);
    }
}

// Move constructor
mystring::mystring(mystring &&s) noexcept : str{s.str} { s.str = nullptr; }

mystring::~mystring() { delete [] str; }

mystring& mystring::operator=(const mystring &s){
    if(this != &s){
        this->str = new char[strlen(s.str) + 1];
        strcpy(this->str, s.str);
    }

    return *this;
}

std::ostream& operator<<(std::ostream &out, const mystring &s){
    out << s.str;
    return out;
}

mystring mystring::operator+(const mystring &s){
    mystring concat;
    concat.str = new char[strlen(this->str) + strlen(s.str) + 1];
    strcpy(concat.str, this->str);
    strcat(concat.str, s.str);
    return concat;
}

mystring& mystring::operator+=(const mystring &s){

    // temp save "this" string to other
    mystring other {new char(strlen(this->str) + 1)};
    strcpy(other.str, this->str);

    // allocate space for the concatenated string
    this->str = new char[strlen(this->str) + strlen(s.str) + 1];

    // move "this" and input strings to "this"
    strcpy(this->str, other.str);
    strcat(this->str, s.str);

    return *this;
}

You did a single-null-allocation correctly(ish) once:
if(str == nullptr){
    this->str = new char;
    *(this->str) = '\0';

and incorrectly a second time:
if(str == nullptr){
    this->str = new char;
    this->str = '\0';

This will produce a memory leak. That aside, if your compiler allowed this without yelling about incompatible types, that makes me sad.
Even then, as @TobySpeight indicates, the first style of allocation has a mismatch with your delete[] and also needs to follow array-like syntax.


01 Matrix is too slow when solved using DFS I tried solving Leetcode 01 Matrix problem.
It is running too slow when solved using DFS approach.

Given a matrix consists of 0 and 1, find the distance of the nearest 0 for each cell.
The distance between two adjacent cells is 1.

Example 1
Input:
[[0,0,0],
 [0,1,0],
 [0,0,0]]

Output:
[[0,0,0],
 [0,1,0],
 [0,0,0]]

Note:

The number of elements of the given matrix will not exceed 10,000.
There are at least one 0 in the given matrix.
The cells are adjacent in only four directions: up, down, left and right.

class Solution(object):
    def updateMatrix(self, matrix):
        if not matrix or not matrix[0]:
            return []
        m, n = len(matrix), len(matrix[0])
        op = [[-1 for _ in range(n)] for _ in range(m)]
        directions = [(1,0), (-1,0), (0, 1), (0, -1)]
        def dfs(i,j):
            if matrix[i][j] == 0:
                return 0

            if op[i][j] != -1:
                return op[i][j]

            matrix[i][j] = -1
            closest_zero = float('inf')
            for direction in directions:
                x,y = direction[0] + i , direction[1] + j
                if 0 <= x < m and 0 <= y < n and matrix[x][y] != -1:
                    closest_zero = min(dfs(x,y), closest_zero)
            closest_zero += 1
            matrix[i][j] = 1
            return closest_zero

        for i in range(m):
            for j in range(n):
                if matrix[i][j] == 1 and op[i][j] == -1:
                    op[i][j] = dfs(i,j)
                elif matrix[i][j] == 0:
                    op[i][j] = 0
        return op

It is running too slowly and I don't understand what is the reason for that. Most optimised solution have solved this using BFS.

The algorithm is slow since it creates paths in all 4 directions at
each step. The algorithm is also using recursion, which is also slower
than a simple for loop.
Consider a 5x5 matrix A:
[[1 1 1 1 0]
 [1 1 1 1 1]
 [1 1 1 1 1]
 [1 0 1 1 1]
 [1 1 1 1 1]]

to find the distance of the top-left cell, the algorithm first moves
down, then up, then right, and then left. It marks the cells it has
already visited by a -1 to avoid infinite loops. So the first five
steps will move down:
[[-1  1 1 1 0]
 [-1  1 1 1 1]
 [-1  1 1 1 1]
 [-1  0 1 1 1]
 [-1  1 1 1 1]]

now the algorithm cannot move further down since it has reached the
maximum row number, and it tries to move in the next direction which
is upwards. Here, it encounters a -1 and gives up on that direction since
a -1 indicates that it has already visited that cell. Now it tries to
move to the right instead:
[[-1  1  1  1  0]
 [-1  1  1  1  1]
 [-1  1  1  1  1]
 [-1  0  1  1  1]
 [-1 -1  1  1  1]]

In cell A(4,1) (i.e. bottom row, second column) it does the same
checks and finds that it cannot move down, then it tries to move
upwards and encounters a 0 in cell A(3,1). At this point,
we are 6 levels deep into the recursion and the distance from
A(0,0) to A(3,1) is hence found to be 6 for now. So ideally the algorithm should now reject any further paths that exceeds 6 levels of
recursion. Unfortunately this is not the case; first the algorithm steps
back to recursion level 5 in cell A(4,1) and continues with cell
A(4,2):
[[-1  1  1  1  0]
 [-1  1  1  1  1]
 [-1  1  1  1  1]
 [-1  0  1  1  1]
 [-1 -1 -1  1  1]]

from this cell it moves upwards all the way up to cell A(0,2):
[[-1  1 -1  1  0]
 [-1  1 -1  1  1]
 [-1  1 -1  1  1]
 [-1  0 -1  1  1]
 [-1 -1 -1  1  1]]

reaches a recursion level of 11. Here it can move either to the right or
to the left. Since the algorithm always tries right before left, it
moves to cell A(0,3) and then continues downward to cell A(4,3) :
[[-1  1 -1 -1  0]
 [-1  1 -1 -1  1]
 [-1  1 -1 -1  1]
 [-1  0 -1 -1  1]
 [-1 -1 -1 -1  1]]

The recursion level is now 16. Then it moves to right to cell A(4,4) and
then upwards to cell A(0,4).
[[-1  1 -1 -1  0]
 [-1  1 -1 -1 -1]
 [-1  1 -1 -1 -1]
 [-1  0 -1 -1 -1]
 [-1 -1 -1 -1 -1]]

The recursion level is now 21. A zero is
finally found in cell A(0,4) indicating a distance of 21 from cell
A(0,0). Still, the
algorithm continues to investigate useless paths (that is: paths
with recursion level more that 6 (remember that we have already found
a zero at distance 6) and moves back to cell A(1,4) at recursion
level 20. Here it tries the remaining directions (left and right) but
none of those works, so level 20 is done. Then it enters back into
level 19, 18, 17, 16, 15:
[[-1  1 -1 -1  0]
 [-1  1 -1 -1  1]
 [-1  1 -1 -1  1]
 [-1  0 -1 -1  1]
 [-1 -1 -1  1  1]]

notice that it replaces the -1 with 1 as it completes a level. So now
A(1,4), A(2,4), A(3,4), A(4,4), and A(4,3) are all reset to value 1. At
level 15, i.e. cell A(3,3), it has already tried to move down, so now
it tries to move up, but that does not work since cell A(3,2) has a
-1. Then it tries to move to the right, to cell A(3,4), which works
since A(3,4) is now 1 (and not -1). From cell A(3,4) it first tries to
move down and reaches cell A(4,4). From that cell the only alternative
is to move left and at recursion level 17 it reaches cell A(4,3):
[[-1  1 -1 -1  0]
 [-1  1 -1 -1  1]
 [-1  1 -1 -1  1]
 [-1  0 -1 -1 -1]
 [-1 -1 -1  1 -1]]

In this cell it cannot get further, there is a -1 in all directions,
and it gives up on level 17, (and moves back to level ...).
The procedure should be clear by now. I will not continue further with
this example, the point was just to
illustrate why the algorithm is so slow.
In fact, in order to find the distance for A(0,0) in this 5x5
matrix example it executes a whopping 22254 (!) calls to the recursive dfs() method.
This simply to determine that the distance is 4 (which btw is found easily
by moving horizontally to the zero in cell A(0,4)).
I think it is a fair guess that the algorithm has an exponential complexity. And it should take forever to run cases with more than say 100 cells (i.e. a 10x10
matrix).
Finally, here is an example of a much faster algorithm that should be
able to find a solution for a 100x100 matrix in a fraction of a second:
import numpy as np

class Solution:
    """ Solution to leetCode problem 542. 01 Matrix
    Given a matrix consisting of 0 and 1, find the distance of the
    nearest 0 for each cell. The distance between two adjacent cells is 1.
    """
    def __init__(self, A):
        self.A = A

    def get_dist(self):
        """ Get the distance matrix for self.A as defined in the
        problem statement for problem 542. 01.
        """
        A = self.A
        (N, M) = A.shape
        B = np.zeros(A.shape, dtype=int)
        for i in range(N):
            for j in range(M):
                if A[i,j] == 1:  # if A[i,j] == 0, B[i,j] is already set to 0
                    dist = 1
                    found = False
                    while not found:
                        for (x,y) in self.points(i, j, dist):
                            if A[x,y] == 0:
                                B[i,j] = dist
                                found = True
                                break
                        if not found:
                            dist = dist + 1
                            if dist > M+N:
                                raise Exception('Unexpected')
        return B

    def points(self, i, j, dist):
        """ Generate all valid points a distance 'dist' away from (i,j)
        The valid points will lie on the edge of a diamond centered on
        (i,j). Use a generator to avoid computing unecessary points.
        """
        (N, M) = self.A.shape
        for k in range(dist):
            if (i+k < N) and (j-dist+k >= 0):
                yield (i+k, j-dist+k)
            if (i+dist-k < N) and (j+k < M):
                yield (i+dist-k, j+k)
            if (i-k >= 0) and (j+dist-k < M):
                yield (i-k, j+dist-k)
            if (i-dist+k >= 0) and (j-k >= 0):
                yield (i-dist+k, j-k)


Fractional binary number to decimal number Write and test your own double abc (char * b) function in the program, taking in the form of a string the fractional binary number of the form "1101.101" and returning an equal decimal number (here 1 * 2 ^ 3 + 1 * 2 ^ 2 + 0 * 2 ^ 1 + 1 * 2 ^ 0 + 1 * 2 ^ -1 + 0 * 2 ^ -2 + 1 * 2 ^ -3 = 8 + 4 + 0 + 1 + 0.5 + 0 + 0.125 = 13.625). The function should return the special value -1 if an incorrect fractional binary number is entered (e.g. containing a different digit than 0, 1 or more than one period '.').

My solution:

#include <iostream>
#include <cmath>
using namespace std;
double abc(char* b);
int main() {
cout <<  abc("1101.101");
return 0;
}

double abc(char* b){
int i = 0;
int a = 0;
int dot = 0;
double sum=0;
int c = 0;
do{
    if ((b[a]=='1')||(b[a]=='0')||(b[a]=='.')){
        a++;
        if(b[a]=='.'){
            dot++;
        }
        if(dot==2){
            return -1;
        }
    } else {
        return -1;
    }
} while(b[a]!='\0');

while(b[i]!='.'){
    i++;
}
while(i!=0) {
    if(b[c]=='1'){
        sum = sum + pow (2, i-1);
    }
    c++;
    i--;
}
while(b[i]!='.'){
    i++;
}
if(b[i]=='.'){
    int m = 0;
    while(b[i]!='\0'){
        if(b[i]=='1'){
            sum = sum + pow (2, m);
        }
        m = m - 1;
        i++;
    }
}
return sum;
}

Problem:
Everything is fine, it gives me 13.625, but there is a lot of code. Is there any faster way to solve this exercise?

Here are some thoughts about your code:

Don't use using namespace std. This is considered bad practice and you can find many reasons on the internet why this is the case (for example here). You can, for example write std::cout instead.
You should use variable-names that tell the person who reads the code the purpose of the variable.
To improve the legibility of your code, you really should use indentation.
To further improve legibility, you can leave spaces between operators.
I made your code a bit shorter by avoiding redundant code. You could even make it a lot shorter still, but I wanted to maintain your code structure.
Finally, you should have a look at edge cases: Empty strings, strings that only contain "." and strings that don't have positions after/before the decimal point (for example 101 = 5 or .101 = 0.625).

The result looks like this:

#include <iostream>
#include <cmath>
#include <cstring>

double abc(char* b);

int main() {
    std::cout <<  abc(".101") << "\n";
    return 0;
}

double abc(char* b){

    if(strcmp("", b) == 0 || strcmp(".", b) == 0) {       //Empty String or only "."
        return -1;
    }

    if(b[0] == '.') {       //No positions before the decimal point
        int length = std::strlen(b);
        char* newB = new char[length + 1];
        newB[0] = '0';
        for(int k = 0; k < length; k++) {
            newB[k + 1] = b[k];
        }

        b = newB;
    }

    int countDots = 0;
    int index = 0;
    int dotIndex = -1;
    do{
        if ((b[index] == '1') || (b[index] == '0') || (b[index] == '.')){
            index++;
            if(b[index] == '.'){
                countDots++;
                dotIndex = index;
            }
            if(countDots == 2){
                return -1;
            }
        }
        else {
            return -1;
        }
    } while(b[index] != '\0');

    if(dotIndex == -1) {    //No dot found, but not empty string, so you have a natural number
        int length = std::strlen(b);
        char* newB = new char[length + 2];
        for(int k = 0; k < length; k++) {
            newB[k] = b[k];
        }
        newB[length] = '.';
        newB[length + 1] = '0';
        dotIndex = index;
        b = newB;
    }

    double sum = 0;
    index = 0;
    int exponent = dotIndex;

    while(exponent != 0) {
        if(b[index] == '1'){
            sum = sum + pow (2, exponent-1);
        }
        index++;
        exponent--;
    }

    while(b[index] != '\0'){
        if(b[index] == '1'){
            sum = sum + pow (2, exponent);
        }
        exponent = exponent - 1;
        index++;
    }

    return sum;
}

I will leave it to you to comment the code properly.

Please note: The code now is a bit longer than yours, but it is able to handle more things.

EDIT:

After one of the comments below, I decided to also show a version without C-style arrays, and without new. This also should solve the problem with possible memory leaks. So all in all, this is the better version (I will leave the old version, because the questioner asked for a function double abc(char* b)):

#include <iostream>
#include <cmath>
#include <string>

double abc(std::string b);

int main() {
    std::cout <<  abc("1101.101") << "\n";
    return 0;
}

double abc(std::string b){

    if(b.compare("") == 0 || b.compare(".") == 0) {       //Empty String or only "."
        return -1;
    }

    if(b[0] == '.') {       //No positions before the decimal point
        b.insert(0, "0");
    }

    int countDots = 0;
    size_t index = 0;
    int dotIndex = -1;
    do{
        if ((b[index] == '1') || (b[index] == '0') || (b[index] == '.')){
            index++;
            if(b[index] == '.'){
                countDots++;
                dotIndex = index;
            }
            if(countDots == 2){
                return -1;
            }
        }
        else {
            return -1;
        }
    } while(index < b.length());

    if(dotIndex == -1) {    //No dot found, but not empty string, so you have a natural number
        b.push_back('.');
        b.push_back('0');
        dotIndex = index;
    }

    double sum = 0;
    index = 0;
    int exponent = dotIndex;

    while(exponent != 0) {
        if(b[index] == '1'){
            sum = sum + pow (2, exponent-1);
        }
        index++;
        exponent--;
    }

    while(index < b.length()){
        if(b[index] == '1'){
            sum = sum + pow (2, exponent);
        }
        exponent = exponent - 1;
        index++;
    }

    return sum;
}


Circuit-Swap mechanism on going (BMP->PPM) Reffering other "Stack Exchange" Sources, I've been told that the only way of swapping BGR to RGB involves buffering. While this isn't true, I could just perofrm a quick C-SA algorithm on fseek or bmp buffer byte offsets. I wrote a fast implementation of fwrite, that swaps the first byte with the 3rd. I do believe that this operation is not slower then the regular one.

size_t fwrite_ps ( const void * ptr, size_t size, size_t count, FILE * stream )
{
    size_t i, chr = 0;
    char p;
    signed pc = 0;

    for(i = 0; i != count * size; i++)
    {
        switch(pc)
        {
            case 1  :   p = 0;          break;
            case 2  :   p = (-2);       break;
            default :   p = 2; pc = 0;  break;
        }

        if( fputc( ((unsigned char*)ptr)[(i+p) - 0], stream ) != EOF ) { chr++; pc++; }
        else                                                           { break; }
    }
    return chr / size;
}

I like my style of codding. For me, it is pretty readable even after years. If it is not as readable for others, it might be because they are most likely used to other, more common styles.

I would like to enhance the utility of this, so i will also provide a really fast way of copying pixels in the proper order. Except functional and fast, it also simplifies varieties of different standarts. It will not copy padding and it involves only one loop.

#define IMAGE_NAME "sample.bmp"

typedef unsigned short WORD;
typedef unsigned long DWORD;

DWORD WIDTH     = BMPDATA[18] + BMPDATA[19] * 256; // Max width 65791
DWORD HEIGHT    = BMPDATA[22] + BMPDATA[23] * 256; // Max height 65791
DWORD h;
FILE* fp = fopen(IMAGE_NAME, "rb");

fprintf(fp, "P6\n%i %i\n%i\n", WIDTH, HEIGHT, 255);  // Copies PPM data

for(h = (HEIGHT-1)*((WIDTH*3) + WIDTH%4); h>=0; h -= (WIDTH*3) + (WIDTH%4))
{
    fwrite_ps(&BMPDATA[BMPDATA[10] + h], 1, WIDTH*3, fp);
}
// BMPDATA is your buffer containing a bmp image.

Magic numbers

What is the significance of 18, 19, 22 and 23? They must be defined in terms of bitmap file header and bitmap info header
Correctness

According to a spec, bitmap width and bitmap height are 4 byte integers. The code accounts only for 2 bytes.
The code completely ignores colour depth.
The code is unable to deal with palette-based bitmaps.
The code completely ignores possible DIB compression.

Style

All-uppercase words are usually reserved for macros. Normal variable's identifiers should be in snake_case or camelCase.
What is the purpose of typedefing DWORD?
You should at least make your snippet into a function. Otherwise it is completely useless.


Text file to spreadsheet Lets say you have several .txt files in a folder and want to merge all text files into a single Excel document:

A.txt

A1
A2
A3
A4
A5
A6
A7
A8
A9
A10
A11
A12
A13
A14
A15
A16
A17
A18
A19
A20
A21
A22

B.txt

B1
B2
B3
B4
B5
B6
B7
B8
B9
B10
B11
B12
B13
B14

C.txt

C1
C2
C3
C4
C5
C6
C7
C8
C9
C10
C11
C12
C13
C14
C15
C16
C17
C18
C19
C20
C21
C22
C23
C24
C25
C26
C27
C28
C29
C30
C31
C32
C33

D.txt

//empty file

E.txt

E1
E2
E3
E4
E5
E6
E7
E8
E9
E10
E11
E12
E13
E14
E15
E16
E17
E18
E19
E20
E21
E22
E23
E24
E25
E26
E27
E28
E29
E30
E31
E32
E33
E34
E35
E36
E37
E38

The Output:

spreadsheet_to_text.py

"""
Reads all .txt files in path of the script into a single
spreadsheet. In the first line of the spreadsheet the filename were
the data is from is displayed. Then the data follows
"""

import os
from typing import List
import openpyxl
from openpyxl.utils import get_column_letter

def text_into_spreadsheet():
    """main logic for read .txt into spreadsheet"""
    workbook = openpyxl.Workbook()
    sheet = workbook.active
    column: int = 1
    filenames: List[str] = os.listdir()
    for filename in filenames:
        if filename.endswith(".txt"):
            with open(filename) as textfile:
                lines: List[int] = textfile.readlines()
                sheet[get_column_letter(column) + '1'] = filename
                row: int = 2
                for line in lines:
                    sheet[get_column_letter(column) + str(row)] = line
                    row += 1
            column += 1
    workbook.save('result.xlsx')

if __name__ == "__main__":
    text_into_spreadsheet()

What do you think about the code?
How can it be improved?

edit: you can find a programm doing it in reversed in Spreadsheets to text files

There are a few things we could improve:

you could use the "lazy" glob.iglob() to filter out *.txt files instead of doing the os.listdir(), keeping the whole list if memory and having an extra check inside the loop:

for filename in glob.iglob("*.txt"):

instead of using textfile.readlines() and read all the lines in a file into memory, iterate over the file object directly in a "lazy" manner:

for line in textfile:

instead of manually keeping track of column value, you could use enumerate():

for column, filename in enumerate(glob.iglob("*.txt"), start=1):

Same idea could be applied for rows.
I think you don't have to use get_column_letter() and instead operate the numbers which you have:

sheet.cell(row=row, column=column).value = line

not to say anything against openpyxl, but I personally find xlsxwriter module's API more enjoyable and more feature rich

Complete improved version:

import glob

import openpyxl

def text_into_spreadsheet():
    """main logic for read .txt into spreadsheet"""
    workbook = openpyxl.Workbook()
    sheet = workbook.active

    for column, filename in enumerate(glob.iglob("*.txt"), start=1):
        with open(filename) as textfile:
            sheet.cell(row=1, column=column).value = filename

            for row, line in enumerate(textfile, start=2):
                sheet.cell(row=row, column=column).value = line

    workbook.save('result.xlsx')

if __name__ == "__main__":
    text_into_spreadsheet()


Getting name from user, and handling any errors in C The primary purpose of this program is to practice catching if the user enters data that does not correspond to expected input. Note if the user enters their name as "123", this is valid as it can be used as an identity, however if users enter only spaces (i.e. "    ") or just a newline char then their input will be rejected. Also note preceding and trailing spaces are OK as long as there is some form of symbol present in the entire name.

#include "stdafx.h"
#include <stdio.h>
#include <string.h>
#include <stdbool.h>

#define BUFFSIZE 10

/*Checks to see if name contains actual characters and not just whitespace*/
bool isThereChars(const char * name)
{
    bool charCheck = false;
    for (size_t index = 0; index < BUFFSIZE; ++index) {
        if (name[index] != ' ' && name[index] != '\0')
            charCheck = true;
    }
    return charCheck;
}

void retreiveName(char* name)
{
    for (;;) {
        if (fgets(name, BUFFSIZE, stdin) != NULL) {
            name[strcspn(name, "\n")] = '\0';
            if (name[0] == '\0' || isThereChars(name) == false) {
                printf("Invalid name entered please try again...\n");
            }
            else
                return;
        }
        else
            printf("Memory could not be allocated please try again...\n");
    }
}

int main(void)
{
    char name[BUFFSIZE];

    printf("Enter Name: ");
    retreiveName(name);

    printf("Your name is: %s\n", name);
    printf("Press enter to continue...\n");
    getchar();

    return 0;
}

Nitpick

void retreiveName(char* name)

This should be

void retrieveName(char* name)

Bugs

        if (name[index] != ' ' && name[index] != '\0')

What happens if I enter tabs or unprintable characters?  This code would pass a string of all tabs.  Is an all numbers string valid?

        if (isalpha(name[index]))

Now we check to see if there is a letter rather than excluding only two bad values.

The <ctype.h> header has this and other functions that you can use.  E.g.

        if (isprint(name[index]) && !isspace(name[index]))

This might be what you want if you have a really liberal definition of name.

Second call

What happens if you call retrieveName twice with the same character array?  In particular, let's give it an input of "Christopher".

#define BUFFSIZE 10

Christopher has eleven letters, so it overwhelms the buffer size.  That's OK the first time, but the second call would get the remainder of the first call, not a new name.  So the second time you'd be processing the string "r\0ristophe".  That includes a letter, so it will return true.  Not great, since we wanted a new name.

Now let's change that a bit.  Instead of Christopher, enter "  Chris    ".  Again, that's eleven characters.  It overwhelms the buffer.  The second call results in the string " \0Chris   ".  Now, you

    for (size_t index = 0; index < BUFFSIZE; ++index) {

When index is 2, it finds a C, which is neither a space nor a null.  So the function returns true on a string that everything else will interpret as " ".  Oops!

And note that this could also happen with two calls that don't overwhelm the buffer.  E.g. the strings "Chris" and " " will call it with " \0ris\0\0\0\0" the second time, assuming the character array was zero-initialized.

Consider instead

    for (size_t index = 0; index < BUFFSIZE && name[index] != '\0'; ++index) {

Then the loop will stop at the end of the buffer or the end of the string, whichever comes first.

What about other lengths?

bool isThereChars(const char * name)

This only works with one size of buffer.  It would make more sense to allow arbitrary length buffers.

bool hasLetter(const char * name, int bufferLength)

This allows the caller to specify a buffer length.

I also like the name hasLetter better than isThereChars.  Your mileage may vary.  Another option is hasAlpha, to match isalpha.

No need for a flag variable

    bool charCheck = false;
    for (size_t index = 0; index < BUFFSIZE; ++index) {
        if (name[index] != ' ' && name[index] != '\0')
            charCheck = true;
    }
    return charCheck;

I'd like this better as something like

    for (size_t index = 0; index < BUFFSIZE && name[index] != '\0'; ++index) {
        if (isalpha(name[index])) {
            return true;
        }
    }

    return false;

This also has the side effect of returning as soon as you find an alphabetic character rather than processing all the other characters even though they won't change the answer.


Inserting a record into MySQL with a timestamp I'm starting with PHP so I can save data to a MySQL database. I read a lot and it seems the "escape" strings is not so safe.

This is my code:

<?php
session_start();
date_default_timezone_set('America/Argentina');
require_once("config/db.php");
$link = mysqli_connect(DB_HOST, DB_USER, DB_PASS);
mysqli_select_db($link, DB_NAME);
$tildes = $link->query("SET NAMES 'utf8'");

$NombreOferta = mysqli_real_escape_string($link,(strip_tags($_POST['NombreOF'], ENT_QUOTES)));
$CantidadArt = mysqli_real_escape_string($link,(strip_tags($_POST['CantidadArt'], ENT_QUOTES)));
$PrecioOf = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
$NomComercio = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
$DirComercio = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
//$userid = "1";
$fecha =date("Y-m-d");
$hora = date("G:i:s
", time());

mysqli_query($link, "INSERT INTO ofertas (nombreoferta, cantidadarticulos, precio,user_id,fecha,hora) VALUES('" . $NombreOferta . "', '" . $CantidadArt . "', '" . $PrecioOf . "', '" . $_SESSION['user_id'] . "', '" . $fecha . "', '" . $hora .  "');");

mysqli_close($link);
//echo "title"
header("Location: OfertaGrabada.html");
//echo $fecha;
?>

So, is this ok?

Security

SQL Injection

It's probably[*] secure, but it's not the right way to do it, see for example here why prepared statements are better than escaping. Sooner or later, you will mess up when escaping.

Prepared statements are not difficult to use, and result in code that is more secure and more readable, there is really no good reason not to use them.

[*] if $_SESSION['user_id'] is user supplied in any way, it's not secure.

XSS

Your call to strip_tags doesn't make any sense:

ENT_QUOTES isn't a correct value for the second argument, which actually accepts a list of allowed tags.
you should encode user input when echoing it, not when inserting it into the database.
strip_tags isn't enough to prevent XSS in all contexts.
strip_tags mangles your data.

Misc

you are mixing the object oriented style and the procedural style of mysqli, which isn't a good idea.
variables should start with a lower-case character.


Is the following service layer correctly implemented with multiple repositories? I'm new to design patterns and specifically the Service Layer implementation. I need some clarification on where to use Service Layer calls.

Below is an example of what I'm dealing with.  I have data from 2 different repositories so I'm not quite certain when/where I'm supposed to call up any additional information that I need to build my Business Model.

I would like for it to do the following:

Find Car in Repository
Get Picture from another Repository
Update Url within Model to be used for Presentation Layer.

What i'm stuck with is I'm not sure if that logic belongs in the Car Model or it should be handled by a Service layer and within the service layer all of this gets combined.  Hopefully this isn't too confusing. Below I added what I thought was relevant.

Business Model/Logic:

public class Car
{
    private readonly IFileSystemService _service;
    public Car(IFileSystemService service)
    {
        _service = service;
    }

    public int id { get; set; }
    public string model { get; set; }
    public int year { get; set; }
    public Image GetImageFile(IGetImageService imgService);
}

Within the Service Layer:

public void GetImageFile(int carId)
{
    Image imgfile = _Repository.GetPhysicalFileLocation(carId);
    // ...
}

Repository 1:

public Car GetCar(int carId)
    {
        Car myCar = this.GetCar(carId);
        // ...
    }

Repository 2:

public Car GetPictureForCar(int carId)
    {
        Car myCar = this.GetFileForCar(carId);
        // ...
    }

Business Model/Logic

These are two distinct things: your model and your logic should be implemented in separate  classes - the business model defines a Car class, and the business logic consumes it. In other words, I think your Car class is breaking SRP.

The way I see it (could be wrong) is pretty much like this (assuming MVC):

In other words, CarController only needs to know about a CarService, which gives it a model that contains everything the view needs to know - the CarService "puts the pieces together" so that the business logic "layer" can do its part.

I think having a dependency on IFileSystemService in your Car class makes that class be more than just a model.

Also, in "repository 2":

public Car GetPictureForCar(int carId)

I don't think it makes much sense to return a whole Car from a method called GetPictureForCar - I'd expect that method to return an Image.

This is even more surprising:

Car myCar = this.GetFileForCar(carId);

I'd expect GetFileForCar to return either a File class, or a string with the file's name.

Same here:

Image imgfile = _Repository.GetPhysicalFileLocation(carId);

A method that's called GetPhysicalFileLocation should be returning a string with the file's name. And to follow C# naming conventions, _Repository should be _repository (assuming it's a private field).


Simple math using arrays I'm messing around with JavaScript and am trying out objects, functions & arrays.  I'd like to know if someone has tips about making it better, shorter, or has a better solution for what I have thought about for now. What I've tried to do is just do simple math (+ and -) using arrays as input for numbers.  It all works, but I'm just wondering if it could be even better and/or cleaner.

//Write the result
function ap(n) {
    $(".value").append(n);
}

// Make sure the input is a number
function isNumber(n) {
    return !isNaN(parseFloat(n)) && isFinite(n);
}

function doMath(options) {
    var defaults = {
        method: 'add'
    };
    var options = $.extend({}, defaults, options);

    // Add
    if(options.method == 'add') {
        var n = 0;
        for(var i=0; i<options.numbers.length; i++) {
            if(isNumber(options.numbers[i])) {
                n = n + options.numbers[i];
            }
            if(i===options.numbers.length-1) {
                ap(n);
            }
        }
    }

    // Subtract
    if(options.method == 'subtract' ) {

        for (var i=0; i<options.numbers.length; i++) {
            if(i===0) {
                n = options.numbers[0];
            } else {
                if(isNumber(options.numbers[i])) {
                    n = n - options.numbers[i];
                }
            }

            if(i===options.numbers.length-1) {
                ap(n);
            }
        }
    }
}

//Print the result
doMath({'numbers': [10,5,2,2], 'method':'subtract'});

You should avoid mixing calculations with input/output. It would be better if your doMath() would not call the output function ap() directly, but instead would just return the result of its calculations:

ap(doMath({'numbers': [10,5,2,2], 'method':'subtract'}));

Of course the ap itself is a lousy name for a function, but it's more of a helper in here I guess.

I see no reason why you would use and options object instead of passing in just two parameters, but even when you use an options object, it would be simpler to extract its values into local variables and work with these. One can also easily drop the use of $.extend in favor of simple || operator, which is the canonical way of implementing default values for parameters:

    var method = options.method || "add";
    var numbers = options.numbers;

Another step to simplify code is to filter out the invalid numbers before doing any calculations on them. Array.filter helps us here:

var validNumbers = numbers.filter(isNumber);

The actual calculation can be easily implemented with Array.reduce. First we can define functions add and subtract

var methods = {
    add: function(a, b) { return a + b; },
    subtract: function(a, b) { return a - b; }
};

And then using Array.reduce we can apply them for the whole array:

validNumbers.reduce(methods.add);
// or
validNumbers.reduce(methods.subtract);

Pulling it all together:

function doMath(options) {
    var method = options.method || "add";
    var numbers = options.numbers;

    var validNumbers = numbers.filter(isNumber);

    var methods = {
        add: function(a, b) { return a + b; },
        subtract: function(a, b) { return a - b; }
    };

    return validNumbers.reduce(methods[method]);
}


jQuery Color Picker Sliders plugin I want to access some of the methods from outside of the plugins scope, namely showPopup(), hidePopup() and updateColor(newcolor).

Initialization

What I want to do

Something like

cp.updateColor('red');

or

$(".cp").ColorPickerSliders('updateColor', 'red');

Which one is the preferred way, and how to refactor the code to accomplish it?

I achieved it using triggers, but don't know if it has any downsides or is a good way to do these kind of interaction. So I registered a custom event handler inside the plugin's code (Line 320, triggerelement is the original element the plugin is called on):

triggerelement.on('jquerycolorpickersliders.updateColor', function(e, newcolor) {
    updateColor(newcolor);
});

And call it using this code (where cp is the element the plugin is called on):

cp.trigger('jquerycolorpickersliders.updateColor', 'red');

jsFiddle.

What do you think of it? Is it an acceptable solution for the problem, or is there a better one?

Firstly, underneath everything Javascript is an event driven language so I do not think that there is anything wrong with using an event driven design pattern if you are comfortable with it. Events are a great way to be able to interlink different components of your system without exposing the inner workings of them (loose coupling) and they add queuing by default. However they are more complex, they make it hard to trace behavior which in turn can make them hard to debug.

I think that you would get a cleaner interface by developing using something called the revealing module pattern, there is an excellent resource on various design patterns by a guy called Addy Osmani at adyosmanio.com and the revealing module pattern specifically here. This (or a flavor of it) is commonly used in jQuery plugin development which you can see on the jQuery advanced plugin development page (see Keep private functions private). By following these principles you can keep the private stuff private and only expose the functions that you want the calling code to have access to.

You mentioned specifically showPopup(), hidePopup() and updateColor(newColor) so a quick example of how that might look.

;(function($) {
    var ColorPickerSliders = function($element, options) {
         //if you want publicly modifiable configuration options.
         var defaults = {
            "opacity": 0,
            "hsl": 1,
            ...
        };

        function init() {
            ...
        }

        function showPopup()
        {
           ...
        }

        function hidePopup()
        {
           ...
        }

        function updateColor(newColor) {
            //uses private function
            var updatedcolor = tinyColor(newColor);
            ...

        }

        //private
        function tinyColor(newColor) {
            ...
        }

        //private
        function buildHtml()
        {
           ...
        }

        //this is the bit that makes your functions public
        //note that each function returns $element to preserve
        //chaining capabilities.
        return {
            "showPopup": function() {
                showPopup();
                return $element;
            },
            "hidePopup": function() {
                hidePopup();
                return $element;
            },
            "updateColor": function(newColour) {
                updateColor(newColour);
                return $element;
            }
        }
    };

    //the use of the data in here is to prevent the plugin being instantiated multiple times
    $.fn.colorPickerSliders = function(options) {
        return this.each(function() {
            if (!$(this).data('colorPickerSliders'))
                $(this).data('colorPickerSliders', new ColorPickerSliders(this, options));
        });
    };
})(jQuery)

Using the above you can then pull the plugin api from the element using:

$yourElement.data('colorPickerSliders').showPopup();

And you can chain:

$yourElement.data('colorPickerSliders').showPopup().delay(1000).hidePopup();

For reference you can also check this excellent article from Smashing Magazine (check the author) and related GitHub repository.


Mean, Median and Mode of a QVector I would like some help condensing this code down to size. I am trying to utilize Qt's container classes as efficiently as possible. I do not believe this code reflects my objective.

#include <QCoreApplication>
#include <QDebug>

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    QVector<int> vect;

    vect.append(0);
    vect.append(0);
    vect.append(0);
    vect.append(0);
    vect.append(1);
    vect.append(1);
    vect.append(2);
    vect.append(1);

    QVectorIterator<int> ivect(vect);

    double median = 0;
    if(vect.count() % 2 == 0){

        median =
        static_cast<double>((vect[vect.count() / 2 - 1] + vect[vect.count() / 2])) / 2;
    }
    else{

        median = vect[vect.count() / 2];
    }

    int sum = 0;
    while(ivect.hasNext()){

        sum += ivect.next();
    }

    double mean = static_cast<double>(sum) / vect.count();

    QSet<int> set;

    ivect.toFront();
    while(ivect.hasNext())
    {
        set << ivect.next();
    }

    QMap<int, int> map;

    QSetIterator<int> iset(set);

    ivect.toFront();
    while(iset.hasNext())
    {
        int count = 0;
        while(ivect.hasNext())
        {
            if(iset.peekNext() == ivect.next())
            {
                count++;
            }
        }
        ivect.toFront();
        map.insert(iset.next(), count);
    }

    QMapIterator<int, int> imap(map);

    int maxValue = 0;
    QString mode;
    while(imap.hasNext())
    {
        imap.next();
        if(imap.value() > maxValue)
        {
           maxValue = imap.value();
           mode = QString::number(imap.key());
        }
        else if(imap.value() == maxValue)
        {
            mode = "NULL";
        }
    }

    qDebug() << set;
    qDebug() << map;
    qDebug() << "Mean:" << mean;
    qDebug() << "Median:" << median;
    qDebug() << "Mode frequency:" << maxValue;
    qDebug() << "Mode:" << mode;

    return a.exec();
}

Sorted vs Unsorted

Your QVector sequence contains an unsorted sequence of values.  Your median calculation has a precondition that your sequence is sorted.  Consider the sequence 1, 0, 1, what is the median?

Small, Focused, and Testable Functions

Prefer writing functions that employ the single responsibility principle.  Splitting out median and testing it with different sequence types would have pointed out a problem. How about calculating the mean of an empty container? Is division by zero a problem?

Standard Algorithms

Being able to understand when you should use standard algorithms and applying them is very important.  Mean can be written using std::accumulate().  Median can be written in terms of std::nth_element().  Mode can be written using boost::bimap (or utilize std::unordered_map+std::multimap) and std::upper_bound() with std::transform() to return the modes of a sequence.  Not all sequences are uni-modal.


Need advice of refactoring views.py I setup simple CRUD in Django

from django.shortcuts import render_to_response
from django.template import RequestContext
from django.core.urlresolvers import reverse_lazy
from django.http import HttpResponseRedirect
from intentions.models import User,Prayer,Intention
from intentions.forms import IntentionForm, DeleteIntentionForm

def index(request):
    intentions = Intention.objects.all();
    return render_to_response('intentions/templates/index.html',
                                {'intentions': intentions},
                                  context_instance=RequestContext(request)
                            )

def show(request, id):
    try:
        intention = Intention.objects.get(pk=id)
    except Intention.DoesNotExist:
        return render_to_response('intentions/templates/404_show.html')
    return render_to_response('intentions/templates/show.html',
                                {'intention': intention, 'delete_form': DeleteIntentionForm(instance=intention)},
                                 context_instance=RequestContext(request)
                              )

def new(request):
    if request.method == 'POST': # If the form has been submitted...
        form = IntentionForm(request.POST) # A form bound to the POST data
        if form.is_valid(): # All validation rules pass
            # Process the data in form.cleaned_data
            # ...
            intention = form.save()
            return HttpResponseRedirect(reverse_lazy('intention-show', args=[intention.id])) # Redirect after POST
    else:
        form = IntentionForm() # An unbound form

    return render_to_response('intentions/templates/form.html',
                                {'form': form, 'form_url': reverse_lazy('intention-new')},
                                 context_instance=RequestContext(request)
                              )
def edit(request, id):
    intention = Intention.objects.get(pk=id)
    if request.method == 'POST': # If the form has been submitted...
        form = IntentionForm(request.POST, instance=intention) # A form bound to the POST data
        if form.is_valid(): # All validation rules pass
            # Process the data in form.cleaned_data
            # ...

            intention = form.save()
            return HttpResponseRedirect(reverse_lazy('intention-show', args=[intention.id])) # Redirect after POST
    else:
        intention = Intention.objects.get(pk=id)
        form = IntentionForm(instance=intention) # An unbound form

    return render_to_response('intentions/templates/form.html',
                                {'form': form, 'form_url': reverse_lazy('intention-edit', args=[intention.id])},
                                 context_instance=RequestContext(request)
                             )

def delete(request, id):
    if request.method == 'POST':
        try:
            intention = Intention.objects.get(pk=id)
        except Intention.DoesNotExist:
            return render_to_response('intentions/templates/404_show.html')
        intention.delete()
        return HttpResponseRedirect('/')
    else:
        return render_to_response('intentions/templates/404_show.html')

I see some fragments for refactor, but I am newbie in Python and Django and I would like to get advices from more experienced users about refactoring this code too.

1.Using render shortcut function is more preferable than render_to_response. The difference is explained on SO

def index(request):
    intentions = Intention.objects.all();
    return render('intentions/templates/index.html', {'intentions': intentions})

2.Then django has another shortcut get_object_or_404

intention = get_object_or_404(Intention, pk=id)

It raises 404 Exception which is processed by middleware. You don't need to call custom HttpRequest for that.

3.Here lazy version reverse_lazy is redundant. Call reverse instead.

return HttpResponseRedirect(reverse_lazy('intention-show', args=[intention.id])) # Redirect after POST

4.You also can look through CBV. Django offers for you some base class to handle forms. It may reduce your code.


Reduce Prime Sieve Memory Consumption I've made a prime number generator (for Project Euler).  It uses Euler's Sieve (a modified Sieve of Eratosthenes), with a mod 30 step.  I'd like to reduce the memory consumption to 4/15 what it currently is by keeping a boolean array only for the possibly prime remainders of 30.  I can't get it to work and also fear that this will slow down the program.

I've used

n[f/30*8+[zeroes, except 1,2,3,4,5,6,7 at 7,11,13,17,19,23,29][f%30]]

which seemed to not filter out any composite numbers.  How can I make this work and what other optimizations (aside from increasing the mod) can you suggest?  I need the primes up to about two billion.

#include <vector>
#include <algorithm>
#ifndef _pes30_cxx_
#define _pes30_cxx_
typedef unsigned long long big;
const int ofs[]={6,4,2,4,2,4,6,2};
void sievePrimes(big m,std::vector<big> &p){
  big f;
  bool n[m];
  p={2,3,5};
  for(big i=0;i<m;++i)n[i]=true;
  for(big i=7,s=1;i<=m;i+=ofs[s],++s==8?s=0:0){
    if(!n[i])continue;
    p.push_back(i);
    for(big j=i,t=s;j<=m/i;j+=ofs[t],++t==8?t=0:0){
      if(!n[j])continue;
      f=j*i;
      do{
        n[f]=false;
      }while((f*=j)<=m);
    }
  }
}
inline bool isPrime(big n,std::vector<big> p){
  return std::binary_search(p.begin(),p.end(),n);
}
#endif

I consider this to be obfuscated code.  Consider:

There are no comments: no guidance on how to call the function, and no explanation of how it works internally.
All variables have one-character names, as if you had filtered the code through a minifier.  The sole exception is ofs — which is still cryptic and meaningless to me.
You have used nearly the minimum amount of whitespace possible:

SpaceSpacefor(big i=0;i<m;++i)n[i]=true;

The code would be more readable with deeper indentation than two spaces per level.  It would be easier to see the individual parts of the loop header if you put spaces after each semicolon.  The loop body would be easier to see if it didn't immediately follow the closing parenthesis.

    for (big i = 0; i < m; ++i) {
        n[i] = true;
    }

All these changes cost you nothing, and have tangible benefits.
This loop header is trying to accomplish way too much!

for(big j=i,t=s;j<=m/i;j+=ofs[t],++t==8?t=0:0)

Also, why is the code guarded by #ifndef _pes30_cxx_?  Is this code living inside a header file?  If so, move it out to a .cpp file where it belongs.

Given the serious stylistic issues with the code, I doubt that anyone would voluntarily reverse-engineer it and review its substance.  I suggest that you work on expressing yourself clearly, then post a follow-up question asking about memory or other performance concerns.