JavaScript flood fill I tried to implement a very simple color flood fill to experiment a bit with Javascript. All comments about the code are welcome but I am especially interested in performance as it seems to be a bit slow. The browser tools only tell me that there is apparently a bunch of minor garbage collecting going on.

function setPixelColor(cvs, data, row, col, color) {
  const [red, green, blue] = color;
  data[4 * (row * cvs.width + col)] = red;
  data[4 * (row * cvs.width + col) + 1] = green;
  data[4 * (row * cvs.width + col) + 2] = blue;
  data[4 * (row * cvs.width + col) + 3] = 255;
}

function getPixelColor(cvs, data, row, col) {
  return [data[4 * (row * cvs.width + col)],
          data[4 * (row * cvs.width + col) + 1],
          data[4 * (row * cvs.width + col) + 2]];
}

function arr_eq(arr1, arr2) {
  if(arr1.length != arr2.length) {
    return false
  }

  for(let i=0; i<arr1.length; i++) {
    if(arr1[i] !== arr2[i]) {
      return false
    }
  }

  return true;
}

(function () {
  const cvs = document.getElementById("paint");
  const ctx = cvs.getContext('2d');

  // black background
  ctx.fillStyle='black';
  ctx.fillRect(0, 0, cvs.width, cvs.height);

  const imageData = ctx.getImageData(0, 0, cvs.width, cvs.height);
  const data = imageData.data;

  const start = [[40, 40, [255, 0, 0]],
                 [10, 20, [0, 255, 0]],
                 [23, 42, [0, 0, 255]],
                 [300, 333, [255, 255, 0]],
                 [200, 333, [255, 0, 255]]];

  for(point of start) {
    let [r, c, v] = point;
    setPixelColor(cvs, data, r, c, v);
  }

  let queue = start.slice(0); // clone

  while(queue.length > 0) {
    p = queue.shift();
    let [r,c] = p;
    const vcur = getPixelColor(cvs, data, r, c);
    for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) {
      const [rn, cn] = n;
      if (rn >= 0 && rn < cvs.height && cn >= 0 && cn < cvs.width) {
        const vn = getPixelColor(cvs, data, rn, cn);
        if (arr_eq(vn, [0, 0, 0])) {
          setPixelColor(cvs, data, rn, cn, vcur);
          queue.push(n);
        }
      }
    }
  }

  for(point of start) {
    let [r, c, v] = point;
    setPixelColor(cvs, data, r, c, [0, 0, 0]);
  }

  ctx.putImageData(imageData, 0, 0);
})();
<!doctype html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>test</title>
  </head>
  <body>
  <canvas id="paint" width=500px height=500px />
  <script src="test.js"></script>
  </body>
</html>

Performant code requires a different style.
When you are working with a lot of data, or need it done fast, it pays to optimise everything. That means that you have to forgo many of the idiomatic coding styles that have come around since ES6.
Some points regarding your code

Don't repeat calculations. Eg you calculate the pixel chanel index 4 times in setPixelColor. Should be once.
Destructuring is SLOW, I mean forever slow, until browsers get round to making destructuring as performant as standard assignment dont use it in performance code.
Move frequent code and variables close to the scope you use the, You have setPixelColor and getPixelColor outside the function, thus it takes extra time to locate these functions.
Most devices are 64bit these days, and low end is 32 bits. Moving a byte takes just as long as moving a 4 bytes as a 32bit long or 8 bytes as 64bit long. Create a 32bit typed array so you can move a pixel in a quarter of the time.
Use 32Bit variables to store colors (eg [255, 0, 0] is 0x0000FF (NOTE 32bit pixels are backwards ABGR the opposite to CSS colors RGBA))
The source of your GC overload is the line for (const n of [[r+1,c], [r-1,c], [r,c+1], [r,c-1]]) { You create an array that contains 4 populated array. A million pixels in a image is small fry, but 5 million arrays is crazy.
Avoid getting values from getters, eg cvs.width is likely a getter. Get the value and store it in a variable scoped as close as possible to where you use it.
You work in rows, and columns (x,y) but the pixel data is a single array. Index directly when you can.
Use local scope, rather than pass variables if you must call functions inside performance loops..
If you can avoid calling functions, its quicker inline.
Use lookup tables to reduce math calculations.
Run in strict mode as it give a slight performance increase, and would also have spotted the undeclared variable p. Because you did not declare it, it becomes a global, and thus access would be much slower than a local. As it is in the heart of the function this will cost you many CPU cycles.

Example
The following does it all in about 1/4 of the time.
See comments for info relating to above comments.

(function () {
    "use strict"; // Always for performant code

    // Colors as 32bit unsigned ints. Order ABGR
    const black = 0xFF000000;
    const red = 0xFF0000FF;
    const green = 0xFF00FF00;
    const blue = 0xFFFF0000;
    const yellow = red | green;
    const magenta = red | blue;

    const cvs = document.getElementById("paint");
    const width = cvs.width;  // Get potencial slow accessors into vars.
    const w = cvs.width;  // alias
    const height = cvs.height;
    const size = width * height;
    const ctx = cvs.getContext('2d');

    // black background
    ctx.fillStyle = "black";
    ctx.fillRect(0, 0, width, height);

    const imageData = ctx.getImageData(0, 0, width, height);

    // Use 32bit buffer for single pixel read writes
    const d32 = new Uint32Array(imageData.data.buffer);

    const start = [
        [40 * w + 40, red],  // work in precalculated pixel indexes
        [10 * w + 20, green],
        [23 * w + 42, blue],
        [300 * w +333, yellow],
        [200 * w + 333, magenta]
    ];
    const pixOff = [w, -w, 1, -1];  // lookup for pixels left right top bottom
    const pixOffX = [0, 0, 1, -1];  // lookup for pixel x left right

    const queue = [];  // keep queue content as simple as possible.
    for (const pixel of start) {
        queue.push(pixel[0]);     // Populate the queue
        d32[pixel[0]] = pixel[1]; // Write pixel directly to buffer
    }

    while (queue.length) {
        const idx = queue.shift();
        const x = idx % w; // Need the x coord for bounds test
        for (let i = 0; i< pixOff.length; i++) {
            const nIdx = idx + pixOff[i];
            if (d32[nIdx] === black) {   // Pixels off top and bottom
                                         // will return undefined
                const xx = x + pixOffX[i];
                if (xx > -1 && xx < w ) {
                    d32[nIdx] = d32[idx];
                    queue.push(nIdx);
                }
            }
        }
     }
     for (const pixel of start) { d32[pixel[0]] = pixel[1] }
     ctx.putImageData(imageData, 0, 0);
})();
<canvas id="paint" width=500px height=500px />


Cycle decomposition in Python Is there an algorithmically better way of doing this?

import numpy as np
l=np.random.permutation(10)
p=list(range(10))
print(l,p)
i=0
k=0
prod=1
z=0
def gcd(a, b):

    while b:
        a, b = b, a % b
    return a
def lcm(a, b):

    return a * b // gcd(a, b)

while z<10:
    """count how many times we have crossed (-1-ed) a number from p"""
    k=0
    while p[k]==-1 and k<9 :
        """find the first non -1 element in p (it's here that I feel that the code is especially clumsy) """

        k=k+1
    i=k
    """k is the first element in the cycle """
    print(l[i])
    p[k]=-1
    z=z+1
    j=1
    while l[i]!=k:
        """go from i to l[i] until we get back to k, closing the cycle """
        p[l[i]]=-1
        z=z+1
        i=l[i]
        j+=1
        """g is just used to find the lcm """

        print(l[i])
    prod=lcm(prod,j)
    print('------')
print(prod)

Use the built in gcd

In Python 3, you can just do:

from math import gcd

Consider keeping with the standard library

Instead of using np.random.permutation, consider just using random.shuffle to generate a random permutation. Removes the numpy dependency altogether.

Just use # instead of """

Comments such as:

"""count how many times we have crossed (-1-ed) a number from p"""

Are usually used for docstrings, you are using them to elaborate on certain steps in an algorithm, I would use # for this.

Place the cycle decomposition into a function

The while loop introduces a lot of global variables and makes it hard to use, I would put the contents into a function. Also explain that this function displays a permutation as disjoint cycles.

Please use better names.

I'm still not sure what every variable does. Like why is p=list(range(10))? I would think p is short for permutation, but l seems to be what the actual permutation is. This program is really hard to understand simply because of your name choice.


Minimum amount of spaces inserted in a given string, such that it gets composed only of elements from a given list The Challenge

  Given a big string and a list of smaller strings, find the minimum amount of spaces that must be inserted between characters of the string in such way that it becomes composed only of elements from the list of smaller strings. (and which are the respective elements used). [Python 3]

For instance, if you had as a string: 'foobarpy' and as a list: ['foo', 'bar', 'py', 'foob', 'arpy'], the optimal solution would be: foob arpy, requiring you to insert only a single space in the string.

Basically:

-The "big string" must be "transformed" into a combination of elements from the list, only by inserting spaces in between characters of it (with the least amount of spaces as possible)

-Elements can't be repeated

-Not all the elements must be used

# Sample input
big_string = '3141592653589793238462643383279'
small_strings = ['3', '314', '49', '9001', '15926535897', '14', '9323', '8462643383279', '4', '793', '3141592653589793238462643383278', '314159265358979','3238462643383279']

My Solution

As I see it, instead of trying to "slice" the given string, it's easier to try to combine the elements from the list and look for the match with the least elements.
My full code: https://repl.it/@LucasWerner/String-Least-Spacing

Get rid of elements in the list that don't appear in the string.

# Required modules
import itertools
import numpy as np

small_strings = [x for x in small_strings if x in big_string]

Combine elements in groups of sizes from 1 to the length of the list (just gathering  to whole thing together) and check whether the combination has the same length as the given string... that would select fewer possibilites to further analyze. Store the possibilites as a list of tuples.

possibilites = list()
for k in range(1, len(small_strings)): #k is the size of the combination
  c = list(itertools.combinations(small_strings, k))
  combinations =  [x for x in c if sum(len(i) for i in x) == big_string_len]
  possibilites.extend(combinations)

Permutate the tuples inside the list to see if they match the big_string. If so, store the permutated tuple in a list of matches.

matches = list()
for tup in possibilites:
  p = list(itertools.permutations(tup))
  permutations = [x for x in p if ''.join(x) == big_string]
  matches.extend(permutations)

Bind the matches with their lengths in a dictionary

sizes = dict()
for tup in matches:
  sizes[tup] = len(tup)

Finding the optimal solution (minimal length and where it occurres)

min_key = min(sizes, key=sizes.get)
min_val = sizes[min_key]

Giving the optimal solution

print('Best solution is', ' '.join(min_key), 'with', min_val - 1, 'space(s)')

Which in this case would print:

  Best solution is 314159265358979 3238462643383279 with 1 space(s)

It's my first month learning Python and I was really hoping to get some feedback and tips from far more experient programmers about what I could change/learn/improve in the code to make it better/more efficient.

Firstly, great job on creating a solution that works. That is often the first step in creating an optimal solution.

But as you may have noticed, making combinations and permutations based on the number of elements in small_strings will start to scale incredibly as you process longer and longer small_strings arrays.

Mathematically, your solution is concise and straight-forward-- kudos again to that. The approach I will suggest is based on Dynamic Programming and preprocessing. This is to speed up scenarios where you have a very large (>100) number of small_strings.

This approach will use the fact that we can solve the whole problem in the same manner of solving a subset of the problem. Specifically, if we can construct the big_string piece by piece, then we can simply add the result of the part we have constructed and add/recurse it on the rest of the string.

The first idea to consider is building a trie for the strings in small_strings. Also known as a prefix tree, a trie will help us figure out which strings we can use to 'construct' the big_string. I highly recommend understanding the concept of a trie for further insight into this problem.

Secondly, we want to implement a function that explores the problem space in a way that is efficient and breaks on paths that yield no results. This is typical of many DP (Dynamic Programming) problems and often looks like recursion in practice. Knowing when and how to implement this comes with practice. I also highly recommend practicing some classical DP problems if you want to become a skilled developer.

With those points in mind, here's a solution:

def buildTrie(words):
    trie = {}
    for word in words:
        current = trie
        for c in word:
            current = current.setdefault(c, {})
        current['isWord'] = word

    return trie

def findPrefixes(trie, word):
    results = {}
    current = trie

    for length, l in enumerate(word, start=1):
        if l in current:
            current = current[l]
        else:
            break
        if current.get('isWord'):
            results[current['isWord']] = length

    return results

def find_num_spaces(big_string, small_strings):
    trie = buildTrie(small_strings)
    N = len(big_string)

    def helper(index=0):
        nonlocal trie, N

        if index == N:
            return 0

        prefixes = findPrefixes(trie, big_string[index:])

        if prefixes:
            return min(1 + helper(index + length) for length in prefixes.values())
        else:
            return float('inf')

    return helper() - 1

big_string = '3141592653589793238462643383279'
small_strings = ['3', '314', '49', '9001', '15926535897', '14', '9323', '8462643383279', '4', '793', '3141592653589793238462643383278', '314159265358979', '3238462643383279']

assert find_num_spaces(big_string, small_strings) == 1

big_string = '3141592653589793238462643383279'
small_strings = ['3', '314', '49', '9001', '15926535897', '14', '9323', '8462643383279', '4', '793', '3141592653589793238462643383278', '3141592', '65358979323']

assert find_num_spaces(big_string, small_strings) == 2

Since we do our preprocessing in the form of a trie, we know how to efficiently reconstruct our big_string. The general idea here is that we are trading space for time complexity. Storing our strings allows for efficient exploration of the problem space.

Note: I assumed you only needed the number of spaces required-- requiring the specific strings involved would tweak this solution slightly. There would be an additional variable that would track the working solution through the recursion.


Increase difficulty based on score What I'm trying to do is very straightforward and easy, but I am wondering whether there is a more elegant solution.

Basically I have a game and I want the difficulty to increase the moment the score gets between 100 - 500, then increase once again when the score is between 500 - 1500, then again when the score is between 1500 - 3000. In total, the difficulty should increase 3 times.

I need to check:

Whether I am in one of those ranges.
Whether I already increased the score when I got in that range.

Here's what I've got.

bool Increased100500 = false;
bool Increased5001500 = false;
bool Increased15003000 = false;

void IncreaseScore()
{
    if (Score >= 100 && Score <= 500 && !Increased100500)
    {
        Increase();
        Increased100500 = true;
    }
    else if (Score >= 500 && Score < 1500 && !Increased5001500)
    {
        Increase();
        Increased5001500 = true;
    }
    else if (Score >= 1500 && !Increased15003000)
    {
        Increase();
        Increased15003000 = true;
    }
}

LE: I should have mentioned that the method above is called multiple times per frame.

Be careful with your inequalities.  You've listed 500 twice: once as if (Score <= 500) and again as else if (Score >= 500).  That's confusing, as the first condition takes precedence.

I think that you would be better off with one state variable representing the current difficulty level.  The code would also be simpler if you determine NewDifficulty using a level-triggered rather than edge-triggered mechanism.

int Difficulty;

void IncreaseScore()
{
    int NewDifficulty = (Score <   100) ? 0
                      : (Score <=  500) ? 1
                      : (Score <= 1500) ? 2
                                        : 3;
    // One-way ratchet: difficulty can only increase, even if the
    // score subsequently drops.
    if (NewDifficulty < Difficulty)
    {
        Difficulty = NewDifficulty;
    }
}

If the score can never decrease in this game, then you can simplify it further:

void IncreaseScore()
{
    Difficulty = (Score <   100) ? 0
               : (Score <=  500) ? 1
               : (Score <= 1500) ? 2
                                 : 3;
}


Finding median from unsorted array with duplicate elements without sorting it I am implementing a method to find the median of an unsorted array using a counting sort. I would happily go for a median of medians or selection algorithm for better performance but they are essentially sorting the array (or partially sorting the array if I choose to go for minHeap) which I am not in favor of.

int getRange(int *array, int count)
{
    int i, max = 0;
    for(i = 0; i < count; i++)
    {
        if(array[i] > max)
        {
            max = array[i];
        }
    }
    return max;
}

int countFreq(int *array, int size_array, int item)
{
    int i, freq = 0;
    for(i = 0; i < size_array; i++)
    {
        if(array[i] == item)
            freq++;
    }
    return freq;
}

int median(int *array, int count)
{
    int range = getRange(array, count);
    int i, mid_index, addition = 0;

    //Yes I can use calloc here
    int *freq = (int *)malloc(sizeof(int) * range + 1);
    memset(freq, 0, sizeof(int)* range + 1);

    for(i = 0; i < range + 1; i++)
    {
        //Count i in array and insert at freq[i]
        freq[i] = countFreq(array, count, i);
    }

    if(count % 2 == 0)
    {
        mid_index = count / 2;
    }
    else
    {
        mid_index = count / 2 + 1;
    }

    for(i = 0; i < range + 1; i++)
    {
        addition += freq[i];
        if(addition >= mid_index)
        {
            break;
        }
    }
    free(freq);
    return i;
}

I followed this answer to implement using C! Certainly, I want to improve upon this or maybe a better algorithm that doesn't sort the array! For me, this algorithm has some problems

What if there are just 2 elements say {10, 10000}, this will still go on for creating an array of size 10000 which essentially has zeros in it except at the last index!
I find it hard to digest the performance of this algorithm with larger arrays to sort, for now, this is O(n3) as far as I can think of.

If we never modify the elements of array, then it should be passed as a pointer to const: int const *array.

The frequency counts can be unsigned, and ought to be able to represent any size of input array (that suggests that count should be a size_t).

We absolutely must test that the return value of malloc() (and family) is not null before trying to dereference it (including passing it to memset()).  Additionally, it's not necessary or desirable to cast it to the target type:

size_t *freq = calloc(sizeof *freq, range + 1);
if (!freq) {
    fputs("Memory allocation failed!\n", stderr);
    exit(EXIT_FAILURE);
}

The algorithm has undefined behaviour if any element in the array is less than zero.  We need to find the minimum as well as maximum value, or perhaps change the input to be an unsigned type.

The counting is strange, with the nested loop.  Normally, we'd loop just once, incrementing the index for each element we look at - something like this:

for (int i = 0;  i < count;  ++i) {
     ++freq[array[i]];
}

To avoid excessive temporary memory use for the count array, we could use a multi-pass approach.

Divide the range [min,max] into into (say) 256 buckets.
Count the inputs into those buckets.
Identify the median bucket.  Call that M.
Now divide the range represented by M into buckets.
Make another pass over the inputs, counting values within M into these new buckets (discarding values not in M).
Repeat until the bucket size is 1.


Bootstrap WordPress Carousel Recent Posts This works, but I would like suggestions to improve it. For one I think the get posts shouldn't have to be repeated twice, but it only pulls the first two posts if I don't do it this way.

Also, I think
$incNum == 0 ? 'class="active"':'class' can be simplified, but if I take the second part out I get an error.

Most Recent Post

                    <?php
                    // Get posts (tweak args as needed)
                    $i=0;
                    $incNum = 0;
                    $args = array(
                        'post_type'        => 'post',
                        'posts_per_page'   => -1,
                        'orderby' => "date",
                        'order' => "desc"
                    );
                    $posts = get_posts( $args );
                ?>

                        <?php foreach (array_chunk($posts, 4, true) as $posts) :  ?>
                        <?php echo( '

');
                        $incNum++;
                        ?>

                    <?php endforeach; ?>

                               <?php
                    // Get posts (tweak args as needed)
                    $i=0;
                    $incNum = 0;
                    $args = array(
                        'post_type'        => 'post',
                        'posts_per_page'   => -1,
                        'orderby' => "date",
                        'order' => "desc"
                    );
                    $posts = get_posts( $args );
                ?>

                <?php foreach (array_chunk($posts, 4, true) as $posts) :  ?>
                <div class="item <?php if ($i==0){echo 'active';}?>">

                        <?php foreach( $posts as $post ) : setup_postdata($post); ?>

                                                <?php the_post_thumbnail( 'full' );  ?>

03 Dec 2014

<?php echo $i?>

<a href="<?php the_permalink() ?>" rel="bookmark" title="<?php the_title(); ?>"><?php the_title(); ?>

                                            John doe

<?php echo substr(get_the_excerpt(), 0,99).' [...]'; ?>

                        <?php $i++ ?>

                        <?php endforeach; ?>

                <?php endforeach; ?>

I haven't ran (don't have a WordPress installation handy) or debugged the code, but this should work. Your original code is buggy in a number of ways. The $posts variable is clobbered when you use it as the iterating variable in the foreach loop, which is probably why your code wasn't working as expected.

Most Recent Post

<?php
  // Use a variable instead of magic number. Consider moving this to a WordPress config instead using get_option
  $chunk_size = 4;
  $args = array(
    'post_type'         => 'post',
    'posts_per_page'    => -1,
    'orderby'           => "date",
    'order'             => "desc"
  );
  $posts = get_posts( $args );
?>

    <?php // Use a for loop here instead of foreach, since you don't actually need the contents of the array ?>
    <?php for ($i = 0, $l = ceil(count($posts) / $chunk_size); $i < $l; $i++):  ?>
      <li data-target="#recent-post-carousel" data-slide-to="<?php echo $i ?>" <?php echo ($i === 0 ? 'class="active"' : '') ?>>

    <?php endfor; ?>

  <?php foreach (array_chunk($posts, $chunk_size) as $i => $chunk) :  ?>
    <div class="item <?php if ($i === 0){echo 'active';}?>">

        <?php foreach( $chunk as $p => $post ): setup_postdata($post); ?>

                <?php // href here should probably be the_permalink() ?>

                  <?php the_post_thumbnail( 'full' );  ?>

                <?php // Post date shouldn't be hard coded ?>

03 Dec 2014

<?php echo $i*$chunk_size + $p?>

<a href="<?php the_permalink() ?>" rel="bookmark" title="<?php the_title(); ?>">
                    <?php the_title(); ?>

                <?php // Post author shouldn't be hard coded ?>
                John doe
                <?php // Consider using wp_trim_excerpt() instead ?>

<?php echo substr(get_the_excerpt(), 0, 99).' [...]'; ?>

        <?php endforeach; ?>

  <?php endforeach; ?>

A few things to note here:

Use foreach( $array as $i => $element) instead of keeping track of $i separately.
Post count is calculated using $i*$chunk_size + $p
Your HTML was buggy in a few places. I cleaned it up, and left comments in the places where there are probably bugs
Format your code properly to make it more readable
Consider using short tags <?= ?> to replace <?php echo ?> to improve readability
Try to use WordPress functions instead of rolling your own. That way plugins can also hook into them


Safe, fully standards-following integer abs in C Here's a bit tricky one, because AFAIK there is no existing platform, where conditionally compiled part of the code would actually get included by compiler, so it is entirely up to the human review to see if it is correct.

The purpose is to have abs function, which does not trigger undefined behavior with any user input, on any C99 standard compliant compiler, on any platform, and in the theoretical case of result not fitting in the return type, return maximum value that fits.

Apart from any bugs, blunders etc, the real challenge is to come up with a (maybe theoretical) platform where the code doesn't produce desired result, or (hopefully) be reasonably certain that it works in all cases allowed by the standard.

// function to review
#include <inttypes.h>

uintmax_t maxed_abs(intmax_t i) {
    if (i < 0) {
#if UINTMAX_MAX == INTMAX_MAX && INTMAX_MIN < -INTMAX_MAX
        // catch the theoretical case of 2's complement platform,
        // where unsigned and signed have same number of value bits
        if (i == INTMAX_MIN) return UINTMAX_MAX;
#endif
        return -(uintmax_t)i;
    }
    return (uintmax_t)i;
}

// test code
#include <stdio.h>

int main(void)
{
    printf("Enter int in range %jd .. %jd:\n > ", INTMAX_MIN, INTMAX_MAX);
    intmax_t i;
    if (scanf("%jd", &i) == 1) {
        printf("Result: |%jd| = %ju\n", i, maxed_abs(i));
    }
}

This kind of code relies on careful reading of the standard, so if it were me I would add cross-references to the sections of the standard that I was relying on, for example I might write:

/* This relies on the following facts from the C99 standard:
 *
 * §6.2.5.9 The range of nonnegative values of a signed integer
 * type is a subrange of the corresponding unsigned integer type.
 * It follows from this and §7.18.2.5 that UINTMAX_MAX >=
 * INTMAX_MAX.
 *
 * §6.2.6.2.2 Signed integers are represented as sign-and-
 * magnitude, two's complement, or one's complement. It follows
 * that INTMAX_MIN is either -INTMAX_MAX (in sign-and-magnitude or
 * one's complement) or -INTMAX_MAX-1 (in two's complement).
 */

I don't like writing code that can't be tested on any platform that I have access to. Although the code looks good in this case, if you pursued this approach on a bigger scale, then it would be very surprising if all the untested code were correct. I would prefer to write something like:

#if UINTMAX_MAX == INTMAX_MAX && INTMAX_MIN < -INTMAX_MAX
#error "Two's complement with UINTMAX_MAX == INTMAX_MAX is not supported."
#endif

so that untested platforms get a compiler error instead of compiling and executing untested code. In the unlikely event that you discover such a platform, you can add the code and test it at that point.


Making each module expose an interface Is this a good/correct way to write a robust C program?

//File1 => Module1.c

static int Fun(int);

struct{
int (*pFn)(int)
}Interface;

static int Fun(int){
//do something
}

Interface* Init(void)
{
  Interface *pInterface = malloc(sizeof(Interface));
  pInterface->pFn = Fun;
  return pInterface;
}

//File 2
main()
{
  Interface *pInterface1 = Init();
  pInterface1->pFn(5);
}

My intention is to make each module expose an interface.  Give your thoughts.

You probably intended to write

typedef struct {
    int (*pFn)(int)
} Interface;

Use member names which are more descriptive than pFn. I hope that this was just as an example to post here.
Yes this approach can work well. I have worked on a bigger project where we used this OO-light approach for C by having an interface struct and each backend would fill it with it's own implementation.


Simple tip calculator code I have built a super simple tip calculator to help me try and get a hold of the javascript knowledge I've gained. I just wanted to see if there was any feedback I could get for it...
Basic functionality is:
If a letter or incorrect character is input in either the bill or the number of persons input, a message should appear at the bottom advising this.
Otherwise if everything is okay, the tip per person is shown at the bottom, followed by the total cost per person!
Here is the JS:
const button = document.querySelector('button'),
    bill = document.querySelector('#amount'),
    serviceRating = document.querySelector('#service'),
    numberPersons = document.querySelector('#people'),
    pTip = document.querySelector('.tip'),
    pTotal = document.querySelector('.total'),
    p = document.querySelectorAll('p'),
    regex = /(^0|[a-zA-Z]+)/i;

let tipAmount = 0,
    totalCostPP = 0;

button.addEventListener('click', () => {
    if (regex.test(bill.value)) {
        pTip.textContent = 'Please give a valid bill value'
    } else if (regex.test(numberPersons.value)) {
        pTip.textContent = 'Please enter how many people were there'
    } else {
        tipAmountPP = tipPP(bill.value, serviceRating.value, numberPersons.value);
        totalCostPP = totalPP(tipAmountPP, bill.value, numberPersons.value);
        for (paragraph of p) {
            paragraph.classList.toggle('hidden');
        }
        pTip.textContent = `Tip per person: $${tipAmountPP.toFixed(2)}`;
        pTotal.textContent = `Total per person: $${totalCostPP.toFixed(2)}`;
    }
});

const tipPP = (bill, service, people) => {
    let tip = (bill / 100) * service;
    return (tip / people);
}

const totalPP = (tip, bill, people) => {
    return (bill / people + tip);
}

I haven't included the HTML and CSS as I didn't see it necessary as everything is functioning with each other just now.
Anyway, appreciate any help you can give, little or large!

Overall this is good code, but there are several things that could be improved.
Encapsulation
The code should be encapsulated in an IIFE in order to isolate it from other scripts in the page, which, for example, could be using the same variable names.
Also the HTML should be surrounded by an element with a unique identifier (e.g. <div id="tip-calculator"> ... </div>) to allow selecting common elements, such as button or p, but also .tip or .total, which can appear elsewhere on the page outside the context of the calculator and adjust the selectors to represent that.
const button = document.querySelector('#tip-calculator button'),
    bill = document.querySelector('#tip-calculator #amount'),
    // ...

or
const tipCalculator = document.querySelector('#tip-calculator'),
    button = tipCalculator.querySelector('button'),
    bill = tipCalculator.querySelector('#amount'),
    // ...

Conventions
It is convention to use one const/let declaration per variable.
(Potential) bugs
The regex used to validate the bill value only forbids the letters A to Z. Any other non-digits or other errors (such as more than one decimal separator) are not caught. The service rating and number of people are not validated at all.
You should be explicitly converting the string values from the input fields into numbers. Currently you are lucky that you are only using multiplication and division on the value, so that the strings are automatically converted, but if you, for example, would addition you'd have an error ("1" + "2" === "12" not 3).
You are toggling the visibility of the paragraphs on each successful calculation, that so every second calculation does not show a result.
Other possible improvements
You are not using the variables tipAmount and totalCostPP outside the event handler (or even the final else block) and their value doesn't change in there, so it would be better to declare them as const inside that block instead of outside the function.
In order to simplify the validation and conversion of the number values you should be using <input type="number"> with appropriate min, max and step attributes. It automatically forces the user only to enter valid numbers and offers the valueAsNumber property which gives you the value already converted to a number.


"Guess my Number" in Java I have just started learning Java, and don't want to start any bad habits, so please review my latest game code:

import java.util.Random;
import java.util.Scanner;
class GuessMyNumber {
    public static void main(String args[]) {
        Random random = new Random();
        Scanner input = new Scanner(System.in);
        int MIN = 1;
        int MAX = 100;
        int comp = random.nextInt(MAX - MIN + 1) + MIN;
        int user;
        int guesses = 0;
        do {
            System.out.print("Guess a number between 1 and 100: ");
            user = input.nextInt();
            guesses++;
            if (user > comp)
                System.out.println("My number is less than " + user + ".");
            else if (user < comp)
                System.out.println("My number is greater than " + user + ".");
            else
                System.out.println("Well done! " + comp + " was my number! You guessed it in " + guesses + " guesses.");
        } while (user != comp);
    }
}

So although this code is a small example it still can benefit from having a few methods sprinkled in to help clear up what you are trying to do. For instance what if you decided that you wanted to change the max and min to be entered in as a parameter to starting the program? You would have to change max and min to be equal to the int value of args[0] and args[1] although for a small example that is not a big deal, but it would be a little more simple to of had a method similar to this.

//gets deafult secret number
private int getSecretNumber(){
    int MIN = 1;
    int MAX = 100;

    return  getSecretNumber(MAX, MIN);
}
//converts a string variable to a int and returns a secret number
private int getSecretNumber(String strMax, String strMin){
    //code to convert string to int
    getSecretNumber(max, min);
}
//generates a secret number between 2 numbers
private int getSecretNumber(int MAX, int MIN){
    return random.nextInt(MAX - MIN + 1) + MIN;
}

Now you will have saved your self some time in the future if/when the design requirements change. if implemented correctly you might not have any code changes.. say something like this.

public static void main(String args[]) {
    int comp = getSecretNumber(); //gets default
    if (args.length == 2)
        comp = getSecretNumber(args[0], args[1]);

    //etc...

It is always a good idea to seperate your business code and your logic code. The business code is the rules for your code. "Do A,B,C give me D, and Save to E" the logic code is the nitty gritty details of how to do A-E. Everyone calls it something different but the concept is always the same. Keep the two as seperate as possible it makes changing your code easier.

Those are my tips. Other than that I don't see any problems with your code other than you don't have a way to escape the loop other than guessing the correct number.


Quicksort for Objective-C This is quite plain vanilla, but I'm hoping for Objective-C specific feedback. Also I'd be interested in what folks think about sorting in Objective-C vs. doing all sorting in C or C++ (and which?) even in an app.

- (NSMutableArray *) quickSort: (NSMutableArray *)toSort{

    for(NSNumber *arrayMember in toSort){
        if(![arrayMember respondsToSelector:@selector(doubleValue)]){
                NSLog(@"incorrect types in input array, returning nil");
                return nil;
            }
    }

    int intLength = toSort.count;
    if (intLength == 0){
        return toSort;
    }
    else if (intLength == 1){
        return toSort;
    }else if (intLength==2){
        if ([toSort[0] doubleValue] <= [toSort[1] doubleValue]) {
            return toSort;
    }else{
        NSNumber *temp = [toSort objectAtIndex:0];
        [toSort replaceObjectAtIndex:0 withObject:[toSort objectAtIndex:1]];
        [toSort replaceObjectAtIndex:1 withObject:temp];
        return toSort;
        }
    }

    int r = arc4random_uniform(intLength);
    double pivot = [[toSort objectAtIndex:r] doubleValue];

    NSMutableArray *lowArray = [NSMutableArray array];
    NSMutableArray *highArray = [NSMutableArray array];
    NSMutableArray *equalArray = [NSMutableArray array];
    for(int i = 0; i < intLength; i++){
        double arrayMember = [[toSort objectAtIndex:i] doubleValue];
        if (arrayMember < pivot) {
            [lowArray addObject:[NSNumber numberWithDouble:arrayMember]];
        }else if (arrayMember > pivot){
            [highArray addObject:[NSNumber numberWithDouble:arrayMember]];
        }else{
            [equalArray addObject:[NSNumber numberWithDouble:arrayMember]];
        }
    }

    NSMutableArray *returnLeft = [self quickSort:lowArray];
    NSMutableArray *returnRight = [self quickSort:highArray];

    NSMutableArray *returnArray = returnLeft;
    [returnArray addObjectsFromArray:equalArray];
    [returnArray addObjectsFromArray:returnRight];

    return returnArray;

}

This function is written to sort NSNumbers formed from doubles. Here's a sample call:

NSNumber *number3 = [NSNumber numberWithDouble:92.3];
NSNumber *number1 = [NSNumber numberWithDouble:2.3];
NSNumber *number2 = [NSNumber numberWithDouble:4.3];
NSNumber *number5 = [NSNumber numberWithDouble:5.3];
NSNumber *number4 = [NSNumber numberWithDouble:-5.3];

NSMutableArray *toSort = @[number1, number2, number3, number4, number5];
NSMutableArray *sorted = [self quickSort:toSort];
NSLog(@"here's sorted array: %@", sorted);

- (NSMutableArray *) quickSort: (NSMutableArray *)toSort{
    for(NSNumber *arrayMember in toSort){
        if(![arrayMember respondsToSelector:@selector(doubleValue)]){
                NSLog(@"incorrect types in input array, returning nil");
                return nil;
            }
    }

    /* stuff */
}

There is a lot to comment on in just this part of your code, so that's where my focus will lie.

First of all, given that we're working with a mutable array, we should simply sort it in place--and we should make the method a category of NSMutableArray itself.

@interface NSMutableArray(QuickSort)

- (void)quickSort;

@end

So it can be used as such:

NSMutableArray *myArray = /* array instantiation */

[myArray quickSort];

I'd make a similar category for the immutable NSArray class which returns the sorted array:

@interface NSArray(QuickSort)

- (NSArray *)quickSortedArray;

@end

And that'd be used as such:

NSArray *unsorted = /* array instantiation */

NSArray *sorted = [unsorted quickSortedArray];

This makes our method signature make more sense.  Of course, NSMutableArray is a subclass of NSArray, so we can always apply the quickSortedArray method to an NSMutableArray as well, while the first recommendation (the simple quickSort method) simply sorts it in place and returns nothing.

So, that cleans up our method signature.  It makes it nice, and closer to what we'd expect to see with normal Objective-C methods.

But we've got a bigger problem.

Without documenting it, your method only works for objects which responds to the doubleValue selector, and inexplicably returns nil if anything else exists in the method.

Moreover, it can only sort the array in a single direction: ascending.

Rather than the zero argument quickSort or quickSortedArray methods I recommended, what we actually need is to accept some argument that tells us by what logic to sort the array.

We need a NSComparator argument.  This is what NSComparator is:

typedef NSComparisonResult (^NSComparator)(id obj1, id obj2);

In plain English, NSComparator is a block which takes two arguments (of any object type) and returns an NSComparisonResult (which describes how these two objects should be sorted).

So we want our quickSort method to take one of these and use this for the logic to sort the array.  Let the caller specify how they should be sorted.

So, if as a caller of the quickSort method, I know my array contains only NSNumber objects, I can pass a comparator that treats the two inputs as strictly NSNumber, and it doesn't impact your implementation of the sorting logic.


Hash Map implementation with a function to set all values This is an interview question I saw online and thought to give it a try.
The question is about implementing a hash map with a setAll(V value) function. This function assignes a value to all the keys in the map at O(1).
Examples:

HashMapSetAll<Integer,String> map = new HashMapSetAll<Integer,String>();
map.put(1, "1");
map.put(2, "2");
map.setAll("all");
map.put(3,"3");

System.out.println(map.get(1)); // prints "all"
System.out.println(map.get(2)); // prints "all"
System.out.println(map.get(3)); // prints "3"

I added the implementation below - please tell me what you think about it - I mostly care about correctness and efficiency.

Also, pay attention to the call to Thread.sleep(0,1) - the reason I'm calling it is beacuase isAfter method of ZonedDateTime smallest unit compare is nano-seconds (and I want to make sure that at least one nano-seconds passes between to calls to ZonedDateTime.now()). Please tell me if there is a cleaner way of doing this time compare.

HashMapSetAll.java:

import java.time.ZonedDateTime;
import java.util.HashMap;
import java.util.Map;

public class HashMapSetAll<K,V> extends HashMap<K,V> {

    private Map<K,ZonedDateTime> keyDates;
    private ZonedDateTime setAllTime;
    private V setAllVal;

    public HashMapSetAll() {
        super();
        keyDates = new HashMap<K,ZonedDateTime>();
        setAllTime = null;
        setAllVal = null;
    }

    @Override
    public V get(Object key) {
        ZonedDateTime time = this.keyDates.get(key);
        if (time == null)
            return null;

        if ( (this.setAllTime == null) || (time.isAfter(this.setAllTime)) )
            return super.get(key);

        return this.setAllVal;
    }

    @Override
    public V put(K key, V value) {
        V oldVal = super.put(key, value);

        this.keyDates.put(key, ZonedDateTime.now());
        try {
            Thread.sleep(0, 1);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        return oldVal;
    }

    public void setAll(V value) {
        this.setAllTime = ZonedDateTime.now();
        try {
            Thread.sleep(0, 1);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        this.setAllVal = value;
    }
}

Also added some tests in HashMapSetAllTest.java:

import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class HashMapSetAllTest {

    HashMapSetAll<Integer, String> setAllMap = new HashMapSetAll<Integer, String>();

    @BeforeEach
    void setUp() throws Exception {
        setAllMap.clear();
    }

    @Test
    void testPutFirst() {
        String val = setAllMap.put(1, "hi");
        assertNull(val);

    }

    @Test
    void testPutSecond() {
        String val;

        setAllMap.put(1, "hi");
        val = setAllMap.put(1, "hi2");

        assertTrue(val.equals("hi"));
    }

    @Test
    void testGetNormalNull() {
        assertNull(setAllMap.get(1));
    }

    @Test
    void testGetNormalValue() {
        setAllMap.put(1, "hi");
        assertTrue(setAllMap.get(1).equals("hi"));
    }

    @Test
    void testGetValAfterSetAll() { // value assigned after setAll
        setAllMap.setAll("all");
        setAllMap.put(1, "mine");

        assertTrue(setAllMap.get(1).equals("mine"));
    }

    @Test
    void testGetValBeforeSetAll() { // values assigned before setAll
        setAllMap.put(1, "mine");
        setAllMap.setAll("all");

        assertTrue(setAllMap.get(1).equals("all"));
    }

    @Test
    void testGetValBeforeAndAfterSetAll() { // values assigned before setAll
        setAllMap.put(1, "1");
        setAllMap.setAll("all");
        setAllMap.put(2, "2");

        assertTrue(setAllMap.get(1).equals("all"));
        assertTrue(setAllMap.get(2).equals("2"));
    }
}

The obvious solution, replacing all values, is \$O(n)\$ - the trick is to do it in \$O(1)\$. With that in mind, your approach makes sense - store the new 'all' value, and customize get so it knows whether it should return the actual value for a key or the 'all' value.

I wouldn't use insertion time for that, however - that'll lead to hacks like Thread.sleep. A counter field that is only incremented when you call setAll should work just fine.

I would also wrap V in a custom class that stores both V and the last-modified-counter-value, so get doesn't need to perform two hash-map lookups in the worst case.

With that, get would look something like this:

public V get(K key) {
    VWrapper wrapper = super.get(key);
    if (wrapper.counter < setAllCounter)
        return setAllVal;
    return wrapper.value;
}

In your example, at the end keys 1 and 2 would have a counter value of 0 (they were added before any setAll call), 3 has a counter value of 1 (it was added after the first setAll call) and setAllCounter is 1 (setAll has been called one time).


Finding median of 3 elements in array, and sorting them The method medianOfThree

returns the median of the first, center and last element in array
sorts the first, center, and last element of the array, so they are in order EXCEPT places the center in the second to last position.

Examples:

  INPUT: 1,2,3
  RETURN:2
  EFFECT:1,2,3

  INPUT: 3,2,1
  RETURN:2
  EFFECT:1,2,3

  INPUT: 2,3,1
  RETURN:2
  EFFECT:1,2,3

  INPUT: -2,-3,-8,1,20
  RETURN:-2
  EFFECT:-8,-3,1,-2,20

  INPUT: 15,20,5,10
  RETURN:15
  EFFECT:10,5,15,20

NOTE: when the center is moved to the second to last position, it doesn't matter where the value that was previously there goes (as long as it's still in the array). Though this probably isn't relevant. In fact, as long as the first, last and one before last are in the correct place, the rest of the elements can get scrambled.

Here is the code. I tested it, but since it is rather complex it was hard to test (need to check return value and array and think carefully of expected result). Any suggestions for tools that make testing code like this easier?

/*
*Sorts first, center and last element, swaps the new center element with the one before the new last and returns its value.
*/
public static int medianOfThree(int[] sample, int start, int end) {
    if(sample.length < 3) {
        throw new IllegalArgumentException("arrays of length three or greater");
    }

    int center = (int)Math.floor((end-start)/2);

    if(sample[start] > sample[end])
       swap(sample, start, end);

    if(sample[start] > sample[center])
       swap(sample, start, center);

    if(sample[center] > sample[end])
       swap(sample, center, end);

int secondLast = end - 1

    swap(sample, center, secondLast );

    return sample[secondLast];

}

//swaps two elements in array given their positions
private static void swap(int[] sample, int x, int y) {
    int temp = sample[x];
    sample[x] = sample[y];
    sample[y] = temp;
}

You seem to specify that it will only be 3 elements in the title of the question, but your input shows you input more than 3 every so often. Your sorting "algorithm" (a lot of swaps) is quite naïve in that sense - for examples of some sorting algorithms that you could implement, take a look at Wikipedia's article on Sorting Algorithms. No need to understand all of them - some get quite complex!

However, you don't have to write a sorting algorithm yourself to use one! Java has some built in for you.

import java.util.Arrays; // or java.util.Collections if you are using ArrayList etc
Arrays.sort(my_array); // this will sort your array for you, using the Quicksort algorithm

As @JS1 mentioned, your formula for finding the center is wrong, and should be

int center = start + (end - start) / 2;

And with that, you can simply return the center index of the array, since it's all sorted now!

public static int medianOfThree(int[] sample, int start, int end) {
    Arrays.sort(sample);
    return sample[start + (end - start) / 2];
}

...Unless the array has a length that is an even number. Then the median formula is a little different:

public static int medianOfFour(int[] sample, int start, int end) {
    Arrays.sort(sample);
    int medianRightIndex = start + (end - start) / 2;
    int medianLeftIndex = medianRightIndex - 1;
    return (sample[medianRightIndex] + sample[medianLeftIndex]) / 2;
}

However, unless you are specifying a range in the array (e.g. you only wanted the median of the first 3 elements), you don't need to specify the start and end parameters at all. start can be 0 and end can be sample.length. Again, this is only if you want the median of the entire array. For a specific subset, you'd still use start and end parameters.

Now we have arrived to:

public static int median(int[] sample) {
    Arrays.sort(sample);
    int median = 0;
    int index = sample.length / 2;
    if(sample.length % 2 == 0) {
        median = (sample[index] - sample[index - 1]) / 2;
    } else {
        median = sample[index];
    }
    return median;
}


Get 10-day forecast script I coded a script to print a 10-day forecast for any location to my terminal.

As I am just learning Python (coming from the PHP world - yes, I know...) and have no buddies coding in Python, I would really appreciate it if anyone could review my code and provide comments and suggestions for improving it.

For starters, I know I am missing docstrings but I left that part for later as I am still reading about it.

Not sure if I need to mention it, but lookup_by_location() and _call() are overriding methods from Weather ancestor class.

Code is also available in a git repo.

#! /usr/bin/env python
import argparse
import requests
from weather import Weather
from weather import models

class WeatherReport(Weather):
    def __init__(self, location):
        Weather.__init__(self)
        self.get_report(location)

    def get_report(self, location):
        self.show_location(location)
        [self.show_weather(day) for day in self.get_days(self.lookup_by_location(location))]

    def lookup_by_location(self, location):
        url = "%s?q=select* from weather.forecast " \
              "where woeid in (select woeid from geo.places(1) where text='%s') and u='c' &format=json" % (self.URL, location)
        results = self._call(url)
        return results

    def get_days(self, place):
        days = []
        [self.get_day(days, item['date'], item['low'], item['high'], item['text']) for item in place.forecast()]
        return days

    def get_day(self, days, date, low, high, text):
        days.append([date, int(low), int(high), text])

    def show_weather(self, day):
        from_to = "%d-%d" % (day[1], day[2])
        print " "*2 + "%s:" % (day[0]), from_to.rjust(5) + u'\u00b0' + "C -", "%s" % (day[3])

    def show_location(self, location):
        print "-" * 50
        feedback = "10-day forecast for " + location.capitalize()
        print feedback.center(50)
        print "-" * 50

    def _call(self, url):
        results = requests.get(url).json()
        if int(results['query']['count']) > 0:
            wo = models.weather_obj.WeatherObject(results['query']['results']['channel'])
            return wo
        else:
            print 'No results found.'
            quit()

if __name__ == "__main__":
    parser = argparse.ArgumentParser('tdwreport')
    parser.add_argument('-l', '--location', default="sarajevo", help='get forecast for this location')
    parser.add_argument('-v', '--version', action='version', version='%(prog)s 0.2')
    args = parser.parse_args()
    report = WeatherReport(args.location)

One tip: Instead of gluing constant and variable parts into string with + as e. g. in your statement

print " "*2 + "%s:" % (day[0]), from_to.rjust(5) + u'\u00b0' + "C -", "%s" % (day[3])

use the format() method with placeholders {} in formatted string:

print "  {} {:>5}{}C - {}"     .format(day[0], from_to, u'\u00b0', day[3])

(I visually separated the resulting string with placeholders from the format() method),
or - more descriptively - use names in {} placeholders

print "  {day0} {from_to:>5}{degree_sign}C - {day3}" \
    .format(day0=day[0], from_to=from_to, degree_sign=u'\u00b0', day3=day[3])

Format specifier >5 after : does the same thing as .rjust(5) in your code.

Edit:

For Unicode symbols (as u'\u00b0' in your code) use their hexadecimal value (0x00b0) and type c in the placeholder:

print "  {} {:>5}{:c}C - {}"     .format(day[0], from_to, 0x00b0, day[3])

See Format Specification Mini-Language


Neighbors of a matrix element I would like a code review to make this code better, clear, and concise.

The problem it's solving is as follows:

  Given a matrix, print the circumference of each of the items. For example:

1 2 3
4 5 6
7 8 9

  should result in

1 : 2 4 5
2 : 1 4 5 6 3
3 : 2 5 6
etc.

Also, am I correct in saying the time complexity is O(n4)?

public final class PrintCircumference {

    private PrintCircumference() {
    }

    public static void printCircumference(int[][] m) {

        if (m == null) {
            throw new NullPointerException("The input matrix cannot be null");
        }
        int row = m.length;
        int col = m[0].length;

        for (int i = 0; i < row; i++) {
            for (int j = 0; j < col; j++) {
                System.out.print(m[i][j] + " --> ");

                for (int i1 = Math.max(0, i -1); i1 < Math.min(row, i + 2); i1++) {
                    for (int j1 = Math.max(0, j -1); j1 < Math.min(col, j + 2); j1++) {
                        if (i1 != i || j1 != j)
                            System.out.print(m[i1][j1]);
                    }
                }
                System.out.println();
            }
        }
    }

    public static void main(String[] args) {

        int[][] m = { {1, 2, 3},
                      {4, 5, 6},
                      {7, 8, 9}, };

        printCircumference(m);
    }
}

Hmm, I think there's two things I'd do:

Put the 'neighbors' loops into their own method.
Potentially, separating the neighbor collection from the actual printing.

(Please note that I've attempted to format this slightly better for SO's display width)

public final class PrintCircumference {

    private PrintCircumference() {
    }

    public static void printCircumference(int[][] matrix) {

        if (matrix == null) {
            throw new NullPointerException("The input matrix cannot be null");
        }

        for (int currentRow = 0,
                 rowCount = matrix.length,
                 columnCount = matrix[0].length; currentRow < rowCount; currentRow++) {
            for (int currentColumn = 0; currentColumn < columnCount; currentColumn++) {
                System.out.println(matrix[currentRow][currentColumn]
                                   + " --> "
                                   + collectNeighbors(matrix, rowCount, columnCount, currentRow, currentColumn));
            }
        }
    }

    private static String collectNeighbors(int[][] matrix, int rowCount, int columnCount, int originCellRow, int originCellColumn) {
        StringBuilder neighbors = new StringBuilder();
        for (int neighborRow = Math.max(0, originCellRow - 1),
                 neighborRowLimit = Math.min(rowCount, originCellRow + 2),
                 neighborColumnLimit = Math.min(columnCount, originCellColumn + 2); neighborRow < neighborRowLimit; neighborRow++) {
            for (int neighborColumn = Math.max(0, originCellColumn - 1); neighborColumn < neighborColumnLimit; neighborColumn++) {
                if (neighborRow != originCellRow || neighborColumn != originCellColumn) {
                    neighbors.append(matrix[neighborRow][neighborColumn]);
                }
            }
        }
        return neighbors.toString();
    }

    public static void main(String[] args) {

        int[][] m = { { 1, 2, 3 },
                      { 4, 5, 6 },
                      { 7, 8, 9 }, };

        printCircumference(m);
    }
}

I've scoped the variables about as tightly as is possible.
Also, the original row and column variables are slightly misnamed - they're the count/limit, whereas most developers are probably expecting them to be the 'current' value.  In addition, while i is fine for a single loop, when you're explicitly looping over columns/rows, please name your variables as such.  Use new methods to let you 'rename variables in context' (something like what I've done with the currentRow -> originCellRow, etc, here).  It's also usually a good idea to put brackets on every loop/condition, in case something more needs to be added at some point in the future.

One word of warning - you're not doing enough error-checking here; fixing that is left as an exercise for the reader (for instance, I was unsure if anything should be printed if the matrix wasn't completely square).


N-Queens, bitwise approach I've recently solved the famous N-Queens problem:

  Can you place \$N\$ chess queens on a \$N × N\$ board in such a way that no one
  attacks another queen?

The idea here is based on the ideas from several blogposts describing a bitwise approach - approaching the board row by row, keeping track of bits of 3 numbers - columns, major and minor diagonals:

class Solution(object):
    def totalNQueens(self, n):
        all_ones = 2 ** n - 1

        def helper(ld, column, rd, count=0):
            if column == all_ones:  # filled out all vacant positions
                return 1

            possible_slots = ~(ld | column | rd)  # mark possible vacant slots as 1s
            while possible_slots & all_ones:
                current_bit = possible_slots & -possible_slots  # get first 1 from the right
                possible_slots -= current_bit  # occupy a slot

                # mark conflicts in the next row
                next_column_bit = column | current_bit
                next_rd_bit = (rd | current_bit) << 1
                next_ld_bit = (ld | current_bit) >> 1

                count += helper(next_ld_bit, next_column_bit, next_rd_bit)
            return count

        return helper(0, 0, 0)

The solution was accepted by the LeetCode OJ and actually beats 98.97% of Python solutions. But, can we optimize it even further? What would you improve code-quality/readability wise?

Another follow-up concern is that I don't see an easy way for this problem to instead of counting possible board arrangements, collect all possible distinct board configurations.

Micro-optimizing Python is a bit of a quixotic endeavour — if you wanted the fastest possible performance then you wouldn't be using Python. But so long as we understand the nature of the exercise, then it's just a matter of trying lots of changes and measuring the results to see which ones are improvements:

Here's the baseline measurement for comparison with the improvements:

>>> from timeit import timeit
>>> timeit(lambda:totalNQueens(13), number=1)
4.241301966001629

Move the mask against all_ones out of the loop. That is, instead of:

possible_slots = ~(ld | column | rd)
while possible_slots & all_ones:

write:

possible_slots = ~(ld | column | rd) & all_ones
while possible_slots:

This saves about 6%:

>>> timeit(lambda:totalNQueens(13), number=1)
3.999626944991178

Accumulate count via a variable local to totalNQueens instead of returning it from helper:

def totalNQueens(n):
    all_ones = 2 ** n - 1
    count = 0

    def helper(ld, column, rd):
        nonlocal count
        if column == all_ones:
            count += 1
            return
        possible_slots = ~(ld | column | rd) & all_ones
        while possible_slots:
            current_bit = possible_slots & -possible_slots
            possible_slots -= current_bit
            next_column_bit = column | current_bit
            next_rd_bit = (rd | current_bit) << 1
            next_ld_bit = (ld | current_bit) >> 1
            helper(next_ld_bit, next_column_bit, next_rd_bit)

    helper(0, 0, 0)
    return count

This saves another 3% or so:

>>> timeit(lambda:totalNQueens(13), number=1)
3.8595934879995184

Avoid the local variables next_column_bit and so on:

def totalNQueens(n):
    all_ones = 2 ** n - 1
    count = 0

    def helper(ld, column, rd):
        nonlocal count
        if column == all_ones:
            count += 1
            return
        possible_slots = ~(ld | column | rd) & all_ones
        while possible_slots:
            current_bit = possible_slots & -possible_slots
            possible_slots -= current_bit
            helper((ld | current_bit) >> 1,
                   column | current_bit,
                   (rd | current_bit) << 1)

    helper(0, 0, 0)
    return count

This saves roughly another 4%:

>>> timeit(lambda:totalNQueens(13), number=1)
3.706758002997958

That's about 13% improvement in all.


Pulling a subset of elements from a Dictionary I have a function that returns a set of values from a Dictionary.
I don't currently want to allow it to throw KeyNotFoundException, so I'm filtering the keys out before I try to select the values.

I have a feeling this can be done in a more straightforward fashion, but I'm not sure where to go with it.

// this.resources is a Dictionary<string, Resource>

public IEnumerable<Resource> GetResources(IEnumerable<string> resourceNames)
{
    HashSet<string> hs = new HashSet<string>(resourceNames);

    hs.IntersectWith(this.resources.Keys);

    List<Resource> resources = new List<Resource>();

    foreach(string resourceName in hs)
    {
        resources.Add(this.resources[resourceName]);
    }

    return resources;
}

I would write it the obvious straight-forward LINQ way, which gives you good readability as well as lazy evaluation:

public IEnumerable<Resource> GetResources(IEnumerable<string> resourceNames)
{
    return resourceNames
        .Where(name => resources.ContainsKey(name))
        .Select(name => resources[name]);
}


My Django custom user model I followed a video on YouTube to create a custom user model, this is my attempt:

# Django Imports
from django.contrib.auth.models import AbstractBaseUser, BaseUserManager
from django.db import models

# The Custom (UserManager) Class
class UserManager(BaseUserManager):
    # Defining A Function That Will Be Used To Create New Users
    def create_user(self, email, username, f_name, l_name, password = None):
        if not email:
            raise ValueError('Users Must Have An E-Mail Address')
        if not username:
            raise ValueError('Users Must Have A Username')

        # Creating An Object Of The Type (User)
        user                    = self.model(
            user_email          = self.normalize_email(email),
            user_name           = username,
            first_name          = f_name,
            last_name           = l_name
        )

        # Setting The Password For The User Object We Created Above
        user.set_password(password)

        # Saving The User To The Database
        user.save(using = self._db)

        # Returning The (user) Instance To Where The Function Was Called
        return user

    # Defining A Function That Will Be Used To Create Superusers (Admins)
    def create_superuser(self, email, username, f_name, l_name, password):
        # Creating An Instance Of Type (User)
        user                    = self.create_user(
            user_email          = self.normalize_email(email),
            user_name           = username,
            password            = password,
            first_name          = f_name,
            last_name           = l_name
        )

        # Setting The User We Created Above As An Administrator
        user.is_admin           = True
        user.is_staff           = True
        user.is_superuser       = True

        # Saving The User To The Database
        user.save(using = self._db)

        # Returning The (user) Instance To Where The Function Was Called
        return user

# The Custom (User) Class
class User(AbstractBaseUser):
    # The Data (Columns) For Each User
    user_email                  = models.EmailField(max_length = 120, unique = True)
    user_name                   = models.CharField(max_length = 40, unique = True)
    first_name                  = models.CharField(max_length = 60)
    last_name                   = models.CharField(max_length = 60)
    date_joined                 = models.DateTimeField(auto_now_add = True)
    last_login                  = models.DateTimeField(auto_now = True)
    is_admin                    = models.BooleanField(default = False)
    is_active                   = models.BooleanField(default = True)
    is_staff                    = models.BooleanField(default = False)
    is_superuser                = models.BooleanField(default = False)

    # The Data (Column) That Is Going To Be Used As A Login Identifier
    USERNAME_FIELD              = 'user_email'
    REQUIRED_FIELDS             = ['user_name', 'first_name', 'last_name']

    # Defining How The Output Of An Object Of The (User) Class Will Look Like
    def __str__(self):
        return f'{self.user_email} - ({self.user_name})'

    # Defining A Function That Will Check Whether A User Is Admin Or Not
    def has_perm(self, perm, obj = None):
        return self.is_admin

    # Defining A Function That Will Check Whether A User Is Admin In The Current App Or Not
    def has_module_perms(self, app_label):
        return True

I know the way I comment stuff is incorrect, I am going to leave this habit as I don't need to write comments everywhere, instead focus on code clarity, so other than my bad commenting style, is this approach to creating custom users in Django correct?

P.S: I use PostgreSQL as my Database

Spacing

At first glance, it looks like you're creating a bunch of variables instead of creating an object and passing default parameters. You should reduce all this extra spacing to make it clear what you're doing. Have a look:

user = self.model(
    user_email=self.normalize_email(email),
    user_name=username,
    first_name=f_name,
    last_name=l_name
)

This goes for both of the places where you do this.

According to PEP 8 there should only be one space before and after the = operator. The issue with how you're creating variables is that all that spaces requires the reader to keep they're gaze straight and follow the line to make sure they know what is being assigned to what. Here's an example in your code:

user_email                  = models.EmailField(max_length = 120, unique = True)
user_name                   = models.CharField(max_length = 40, unique = True)
first_name                  = models.CharField(max_length = 60)
last_name                   = models.CharField(max_length = 60)
date_joined                 = models.DateTimeField(auto_now_add = True)
last_login                  = models.DateTimeField(auto_now = True)
is_admin                    = models.BooleanField(default = False)
is_active                   = models.BooleanField(default = True)
is_staff                    = models.BooleanField(default = False)
is_superuser                = models.BooleanField(default = False)

All this spacing really isn't necessary.

Parameter spacing is also an issue in your code. Contrary to the suggestions above, there should be no spaces when passing default parameters. Have a look:

user_email = models.EmailField(max_length=120, unique=True)
def has_perm(self, perm, obj=None):
user.save(using=self._db)

Type Hints

You can use type hints to display what types of parameters are accepted and what types are returned by a function. Have a look:

def create_superuser(
    self,
    email: str,
    username: str,
    f_name: str,
    l_name: str,
    password: str
) -> User:


My node app with valid syntax is failing silently, but not breaking. Infrastructure problem? I built a simple Node app, and the code "works", with no failures, no syntax errors, and does not break, but one component doesn't work right. I showed a snippet on Stack Overflow, in this question, but viewers found no issue. So, I think the infrastructure, the method, in which I put this app together must be problematic, and I'd like to have it reviewed.

I put everything into a single source file to eliminate the headache of debugging whilst keeping track of modules.

Here is the code

Here's my relevant script:

var connection = createConnection();

connection.connect(function (err) {
    if (err) return callback(new Error('Failed to connect'), null);
    console.log('[Post]Connection with the officeball MySQL database opened...');

    connection.query(
        'INSERT INTO `officeball`.`sales_entries` SET category = ?, `group` = ?, date = ?, price = ?, customer = ?, seller = ?, commission = ?',
    salesData),

    function (err, rows, fields) {

        if (err) console.log(err);
        connection.destroy();
        console.log('[Post]...Connection with the officeball MySQL database closed.');

    }
});

And here's my console output:

Application initializing...
Application successfully initialized!
Server running at http://127.0.0.1:8080/
User username1 is attempting to validate for a hit...
Connection with the Officeball MySQL database openned...
...Connection with the Officeball MySQL database closed.
User username1 validated and is ready!
[Post] Connection with the officeball MySQL database opened...

From a once over:

Your console.log statement should be wrapped in a condition so that you can turn off all logging to the console. Node can really slow down because of logging to the console.
If you are going to log to the console, then it would make more sense to log your query and salesData as well.
Opening and closing a connection every time you want to post a sale is (very) bad practice, check out mysql.createPool
Calling connection.end(); will kill a connection gracefully. calling connection.destroy(), not so much..

All in all, I would always be sensitive to performance and established best practice.


Depersonalisation via redis cache In an Azure Function, I have a c# program that processes tab delimited data into JSON entries. I'd like to improve it in terms of performance and the connection management.

I want to remove personal data from each line.* As each line gets prepped for turning into JSON I perform this function for two values.** I need to work with the depersonalised data but the operational systems will need to request data from the system. They have access to the PII but the analytical platform (downstream of this function) will not have it. The Key Value pairs are therefore constructed so that the operational system and the log ingestion processes can extract guids. Each key is like AccountID:PIItype:PII and then each value is a guid.***

If I've previously depersonalised a value, I want to retrieve the previously provided value from the Azure Redis Cache. If it doesn't exist, add it to the cache.

using StackExchange.Redis;
using System.Text;
using System.Threading;

private static Lazy<ConnectionMultiplexer> lazyConnection = new Lazy<ConnectionMultiplexer>(() =>
        {
            ThreadPool.SetMinThreads(50, 50);
            string redisCacheName = System.Environment.GetEnvironmentVariable("rediscache_name", EnvironmentVariableTarget.Process).ToString();
            string redisCachePassword = System.Environment.GetEnvironmentVariable("rediscache_password", EnvironmentVariableTarget.Process).ToString();
            return ConnectionMultiplexer.Connect(redisCacheName + ",abortConnect=false,ssl=true,password=" + redisCachePassword);
        });

public static ConnectionMultiplexer Connection
{
    get
    {
        return lazyConnection.Value;
    }
}

static string depersonalise_value(string input, string field, int account_id)
{
    IDatabase cache = Connection.GetDatabase();
    string depersvalue = $"{account_id}:{field}:{input}";
    string existingguid = (string)cache.StringGet(depersvalue);
    if (String.IsNullOrEmpty(existingguid))
    {
        string value = $"{account_id}{Guid.NewGuid()}";
        cache.StringSet(depersvalue, value);
        return (value);
    }
    else return (existingguid);
}

* Hashing is something considered but discarded

** The two values are independent of each other

*** There is a redis cache per region so can't rely on uniqueness of the guid by host. This is just a simple preventative as an account can only come from a single region.

Good use of Lazy on the connection multiplexer but consider making it a readonly field so you can't accidentally replace it later.

Doing this in the Lazy's factory function is a bit odd in my opinion ThreadPool.SetMinThreads(50, 50);. I'm not sure what options you have for initialization in a Azure Function but you could do worse than use a static constructor on your type.

As for the rest of the function, did you know that there is a ConfigurationOptions type in StackExchange.Redis?

new Lazy<ConnectionMultiplexer>(() =>
    {
        string redisCacheName = System.Environment.GetEnvironmentVariable("rediscache_name", EnvironmentVariableTarget.Process).ToString();
        string redisCachePassword = System.Environment.GetEnvironmentVariable("rediscache_password", EnvironmentVariableTarget.Process).ToString();
        return ConnectionMultiplexer.Connect(new ConfigurationOptions
            {
                AbortOnConnectFail = false,
                Ssl = true,
                // not 100% sure this is exactly right but should be rightish
                EndPoints = { { redisCacheName, 0 } }
                Password = redisCachePassword
            }
        });
};

It's a bit more code but it means you don't have to build the string yourself.

Expression bodied members save some boilerplate:

public static ConnectionMultiplexer Connection => lazyConnection.Value;

C# should never snake_case anything. All methods, regardless of accessibility, should be PascalCase: depersonalise_value should be DepersonaliseValue. See the capitalization conventions on msdn.

I notice that you're not using var at all. Just wanted to double check that you knew it existed. e.g

var depersvalue = $"{account_id}:{field}:{input}";

Naming again: depersvalue should be depersonalisedValue although not clear to me whether it's already depersonalised here or whether it's about to be...

Your final else is redundant, it can be removed.

Remove the parentheses around the returned value - they just add noise.

The connection management looks fine to me - ConnectionMultiplexer is incredibly clever and it's designed to be a long-lived singleton which I think makes it fine to be in a static readonly field.

IDatabase is intentionally very lightweight and you can make as many as you like.

Edit

In response to your edit, I would suggest renaming this from "depersonalised" data to something like a PersonalDataAlias (naming is hard).


Converting a list to an array when list has quotes and embedded comma This function converts a list to an array when list has quotes and embedded comma:

array function convertRow(required string lstRow) {

    var arResult = [];
    var arTemp = ListToArray(arguments.lstRow, ',', true);

    var quoteMode = false;
    var startQuote = false;

    for (var i in arTemp) {

        if (i CONTAINS '"' && not quoteMode) { startQuote =  true; }

        if (startQuote) {
            ArrayAppend(arResult, replace(i, '"', ''));

            startQuote = false;
            quoteMode = true;
        }
        else if (quoteMode) {
            arResult[ArrayLen(arResult)] &= "," & replace(i, '"', '');

            if (i CONTAINS '"') quoteMode = false;
        }
        else
            ArrayAppend(arResult, i);
    }

    writedump(lstRow);
    writedump(arResult);

    return arResult;
}

I could get rid of your startQuote variable, I reduced the number of i CONTAINS '"' comparasions by 1.

array function convertRow(required string lstRow) {
    var arResult = [];
    var arTemp = ListToArray(arguments.lstRow, ',', true);
    var quoteMode = false;

    for (var i in arTemp) {
        var containsQuote = i CONTAINS '"';
        if (containsQuote && not quoteMode) {
            ArrayAppend(arResult, replace(i, '"', ''));
            quoteMode = true;
        }
        else if (quoteMode){
            arResult[ArrayLen(arResult)] &= "," & replace(i, '"', '');
            quoteMode = not containsQuote;
        }
        else{
            ArrayAppend(arResult, i);
        }
    }

    writedump(lstRow);
    writedump(arResult);

    return arResult;
}


Sorting integers in descending order So, I coded a program which asks you the number of integers and the integers the user wants to sort in descending order. The program works but I feel like it is very inefficient. If I were to receive high amount of numbers the program would have to do so much processing. Therefore, how can I improve my code in simple ways? (Open to any criticism)

import java.util.Scanner;

public class Main {
private static Scanner scanner = new Scanner(System.in);
private static Scanner scanner2 = new Scanner(System.in);

public static void main(String[] args) {
    System.out.println("How many integers do you want to sort?");
    int[] IntArray = getIntegers(scanner.nextInt());
    IntArray = sortIntegers(IntArray);
    printIntegers(IntArray);

}

public static int[] getIntegers(int number) {
    System.out.println("Enter " + number + " integers.");
    int[] values = new int[number];

    for (int i = 0; i < values.length; i++) {
        values[i] = scanner2.nextInt();
    }
    return values;
}

public static int[] sortIntegers(int[] array){
    int[] unsortedArray = new int[array.length];
    for(int i = 0; i < unsortedArray.length; i++){
        unsortedArray[i] = array[i];
    }

    for(int n = 0; n < unsortedArray.length - 1; n++) {
        for (int j = 0; j < unsortedArray.length - 1; j++) {
            if (unsortedArray[j] < unsortedArray[j + 1]) {
                int k = unsortedArray[j];
                unsortedArray[j] = unsortedArray[j + 1];
                unsortedArray[j + 1] = k;
            }
        }
    }
    return unsortedArray;
}

public static void printIntegers(int[] array){
    for(int i = 0; i < array.length; i++){
        System.out.println(array[i]);
    }
}
}

Extending on @yuri's answer, if you don't have any information relative to the state of "sortedness" of the numbers, quicksort is almost certainly a better option.  If you happen to know that the numbers are almost sorted, it would be more efficient to use insertion sort at that point.  Writing a sorting algorithm as an exercise is fine, but you're likely going to want to prefer to use Collections.sort than write your own unless you have a specific reason not to.

A few other tips:

IntArray is not a typical variable name in Java.  Variable names should start lowercase.
There's really no reason to use two Scanner instances.  You can use the same instance to read the initial array amount as well as each value.
While we're at it, there's no reason to specify the number count.  The program would be more robust if you simply used a List (ArrayList implementation would work best here) and added values until you receive a non-number input like "end".
Generally static class members should really only exist to define constants in your class, and not hold state.  Everything in your program is static, so you don't require state except for the Scanner instance.  Normally you'd simply pass Scanner instance as a parameter to the static method, but you're only using it in the one method getIntegers so you could just instantiate Scanner only in the scope of getIntegers rather than use it as a parameter or class member.
There is already a simple means to print out contents of a List:  List.toString.
In the off chance that an exception is thrown by Scanner (or any stream), we surround the reading with a try.. finally to ensure that no matter what, the scanner instance is closed properly.
Note that I maintained use of List rather than convert to an array.  This is mostly because ultimately an array is only required to print out.  We can accomplish everything we need using List, and most professional programmers tend to prefer the use of List over arrays except for specific circumstances.

Applying the changes above, we get the following:

public class Main {
    public static void main(String[] args) {
        List<Integer> intList = getIntegers();
        intList = sortIntegers(intList);

        printIntegers(intList);
    }

    public static List<Integer> getIntegers() {
        System.out.println("Please provide integers and 'done' when finished.");

        List<Integer> values = new ArrayList<Integer>();
        Scanner scanner = new Scanner(System.in);
        try {
            while(scanner.hasNextInt()) {
                values.add(scanner.nextInt());
            }
        } finally {
            scanner.close();
        }

        return values;
    }

    public static List<Integer> sortIntegers(List<Integer> list) {
        Collections.sort(list, Collections.reverseOrder());
        return list;
    }

    public static void printIntegers(List<Integer> list) {
        System.out.println(list.toString());
    }
}


Searching a sequence for a pattern I have written the following tiny extension method to help me when I'm working with sequences in which I have to find a pattern.

public static IEnumerable<T[]> SearchPattern<T>(this IEnumerable<T> seq, params Func<T[], T, bool>[] matches)
{
      Contract.Requires(seq != null);
      Contract.Requires(matches != null);
      Contract.Requires(matches.Length > 0);

      var matchedItems = new List<T>(matches.Length);
      int itemIndex = 0;

      foreach (T item in seq)
      {
          if (matches[matchedItems.Count](matchedItems.ToArray(), item))
          {
              matchedItems.Add(item);

              if (matchedItems.Count == matches.Length)
              {
                  yield return matchedItems.ToArray();

                  matchedItems.Clear();
              }
          }
          else
          {
              if (matchedItems.Any())
              {
                  foreach (T[] results in seq.Skip(itemIndex - matchedItems.Count + 1).SearchPattern(matches)) // is this a tail call? can it be optimized?
                  {
                      yield return results;
                  }
                  break;
              }
          }
          itemIndex++;
      }
}

It appears to be working fine, but I'm wondering if there's a better way to do it.
Primarily I'm concerned with the recursive call (at the nested foreach), which makes the method very inefficient, not to mention potential stack-overflows when working with very large collections.

I know about tail-calls, and that they can be optimized into a loop (like in F#). I also recall having seen something about tail-calls on an IL level. What confuses me is that I'm already using the iterator pattern (yield return) - so is it a tail call at all? If it is, is it one I can eliminate?

Any input would be much appreciated!

Edit: usage sample:

var results = (new int[] { 0, 1, 2, 3, 3, 4, 4, 5, 7, 9 }).SearchPattern(
    (prevs, curr) => true,
    (prevs, curr) => prevs[0] == curr,
    (prevs, curr) => curr % 2 != 0
); // => { [ 4, 4, 5 ] }

Personally, here is how I would do it. I think this is pretty straight-forward. I think it is also no less efficient than the other (longer, more complex) answer. The method Subarray is something I wrote before, I didn’t write it just for this.

/// <summary>Similar to <see cref="string.Substring(int,int)"/>, only for arrays. Returns a new
/// array containing <paramref name="length"/> items from the specified
/// <paramref name="startIndex"/> onwards.</summary>
public static T[] Subarray<T>(this T[] array, int startIndex, int length)
{
    if (array == null)
        throw new ArgumentNullException("array");
    if (startIndex < 0)
        throw new ArgumentOutOfRangeException("startIndex", "startIndex cannot be negative.");
    if (length < 0 || startIndex + length > array.Length)
        throw new ArgumentOutOfRangeException("length", "length cannot be negative or extend beyond the end of the array.");
    T[] result = new T[length];
    Array.Copy(array, startIndex, result, 0, length);
    return result;
}

public static IEnumerable<T[]> SearchPattern<T>(this IEnumerable<T> seq, params Func<T[], T, bool>[] matches)
{
    Contract.Requires(seq != null);
    Contract.Requires(matches != null);
    Contract.Requires(matches.Length > 0);

    // No need to create a new array if seq is already one
    var seqArray = seq as T[] ?? seq.ToArray();

    // Check every applicable position for the matching pattern
    for (int j = 0; j <= seqArray.Length - matches.Length; j++)
    {
        // If this position matches...
        if (Enumerable.Range(0, matches.Length).All(i =>
            matches[i](seqArray.Subarray(j, i), seqArray[i + j])))
        {
            // ... yield it
            yield return seqArray.Subarray(j, matches.Length);

            // and jump to the item after the match so we don’t get overlapping matches
            j += matches.Length - 1;
        }
    }
}


Macro enabling Python style 'with' in C++ I am developing a simple macro that enables Python style 'with' in C++.
I have shamelessly prefixed the macro name with BOOST_ - primarily because there's a similarity to BOOST_FOREACH, and because I didn't want to call it plain WITH.

The motivation are familiar bugs like these:

void bug1() {
    std::unique_lock<std::mutex>(my_mutex);
    // oooops, forgot to give the lock a name
    function_call();
}

void bug2() {
    std::unique_lock<std::mutex> waldo(my_mutex);
    function_call();
    // oooops, forgot to unlock
    other_function_call();
}

void ugly() {
    {
        // intent of explicit scope not as obvious as could be
        std::unique_lock<std::mutex> waldo(my_mutex);
        function_call();
    }
    other_function_call();
}

So here are the questions:

Is this useful to anyone but me?
Is this a bad idea for whatever reason?
Have I overlooked a possibility to implement this in C++03?
Can it be implemented without the is_move_constructible type requirement?
Do you have suggestions for improvement?
(With a twinkle) Should I apply for some speaking time during the C++Now 2015 Lightning Talks and present this?

And here's my implementation:

NOTE: I have considered an alternative signature such as...

BOOST_WITH(type, ...)

...where ... would be the arguments to the constructor. I have decided to stick to this:

#ifndef BOOST_WITH_HPP_INCLUDED
#define BOOST_WITH_HPP_INCLUDED

#include <utility>
#include <type_traits>

// This macro expands to code that can be used in the same way as the standard
// control structures can. Whatever 'exp' returns lives as long as a loop
// variable would in similar context.
//
// Example:
// BOOST_WITH(std::unique_lock<std::mutex>(my_mutex))
//     do_something();
//
// Example:
// BOOST_WITH(Pushed_matrix()) {
//     draw_something();
//     draw_something_else();
// }
#define BOOST_WITH(exp)                                                        \
    if (auto BOOST_WITH_always_true = boost::with_detail::make_true(exp))

namespace boost {
namespace with_detail {

// wraps an object of movable type
// and provides conversion to bool (always true)
template <class T>
struct always_true {
    explicit always_true(T what) : x{std::move(what)} {}
    constexpr operator bool() const { return true; }
    T x;
};

// always_true<T> construction helper
template <class T>
always_true<T> make_true(T&& what) {
    static_assert(std::is_move_constructible<T>::value,
                  "BOOST_WITH requires the scoped object's type to be move "
                  "constructible");
    return always_true<T>{std::forward<T>(what)};
}

}} // namepace boost::with_detail

#endif // BOOST_WITH_HPP_INCLUDED

Finally, a minimal example:

#include "with.hpp"
#include <cassert>
#include <mutex>

// nothing serious, just a prove of concept
struct Lockable {
    Lockable() : locked(false) {}
    void lock() { locked = true; }
    void unlock() { locked = false; }
    bool locked;
};

int main() {
    Lockable the_lock;
    assert(!the_lock.locked);
    BOOST_WITH(std::unique_lock<Lockable>(the_lock))
        assert(the_lock.locked);
    assert(!the_lock.locked);
}

[ Foreword: Note that I'm not going to argue whether or not it is a good idea to invent a new macro-based syntax for this.  I'm only reviewing your code. — end foreword ]

I think you have been pretty careful and cannot spot any serious issues with your implementation; nice job.

The only thing that I think could be improved is the error reporting.  Your static_assertion fires much too late in the instantiation process such that the error message will end up buried under loads of template error messages.  You can re-structure the instantiation machinery to improve this.

First, provide two overloads of boost::with_detail::make_true:

template <class T>
std::true_type make_true(T&&, std::false_type) {
    static_assert(std::is_move_constructible<T>::value,
                  "BOOST_WITH requires the scoped object's type to be move"
                  " constructible");
    return std::true_type{};  // never executed
}

template <class T>
always_true<T> make_true(T&& what, std::true_type) {
    static_assert(std::is_move_constructible<T>::value, "this is a bug");
    return always_true<T>{std::forward<T>(what)};
}

The static_assertion in the second overload is of course only paranoia and could be omitted.

Second, select the appropriate overload via tag dispatch:

#define BOOST_WITH(RESOURCE)                                        \
  if (auto BOOST_WITH_always_true = boost::with_detail::make_true(  \
      RESOURCE,                                                     \
      std::is_move_constructible<decltype(RESOURCE)>{}))

With this change, if I try to abuse your macro with a wrong type, GCC gives me this pretty straight-forward error message:

In file included from main.cpp:1:0:
boost/with.hpp: In instantiation of ‘std::true_type boost::with_detail::make_true(T&&, std::false_type) [with T = std::mutex; std::true_type = std::integral_constant<bool, true>; std::false_type = std::integral_constant<bool, false>]’:
main.cpp:22:5:   required from here
boost/with.hpp:42:7: error: static assertion failed: BOOST_WITH requires the scoped object's type to be move constructible
       static_assert(std::is_move_constructible<T>::value,

Compare this with what we would have got before.

Note that we cannot simply put static_assert(false, …); inside the make_true body because it would fire unconditionally on every compilation.  It is necessary to make the asserted condition dependent on the template argument.  Of course, any other template instantiated with T that always yields false would do equally well but using std::is_move_constructible (again) seems easiest.


Password generator and hashing To generate a password, I generate a key using RNGCryptoServiceProvider. The key is 9 alphanumeric characters long into which I insert a single non-alphanumeric character for a full length of 10 characters.

Having a mix of Uppercase, lowercase, numbers and symbols and being 10 characters long, as a plain text password (which you'd enter when logging in somewhere), this password should be more than adequate for most situations.

// used for more than just generating a password
public static string CreateKey(int length)
{
    string Chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdejghijklmnopqrstuvwxyz1234567890";
    StringBuilder Sb = new StringBuilder();

    using (RNGCryptoServiceProvider Rng = new RNGCryptoServiceProvider())
    {
        byte[] Buffer = new byte[sizeof(uint)];

        while (length-- > 0)
        {
            Rng.GetBytes(Buffer);
            uint N = BitConverter.ToUInt32(Buffer, 0);
            Sb.Append(Chars[(int)(N % (uint)Chars.Length)]);
        }
    }

    return Sb.ToString();
}

public static string CreatePasswordString()
{
    string Chars = @"~`!@#$%^&*()\/',.?;:|";
    var RNG = new Random();

    // Select a character to insert into the password
    string Character = new String(Enumerable.Repeat(Chars, 1).Select(s => s[RNG.Next(s.Length)]).ToArray());

    return CreateKey(9).Insert(RNG.Next(9), Character);
}

This generates a password which is then hashed in my application using bcrypt as per below:

public static string CreatePassword(string password)
{
    byte[] PasswordBytes = Encoding.ASCII.GetBytes(password);

    return Crypter.Blowfish.Crypt(PasswordBytes);
}

I can then validate the input at login more or less with this function - I haven't tested this code since I'm writing it for the first time for this question.

public static bool ValidatePassword(string emailAddress, string password)
{
    var User = db.Users.FirstOrDefault(x => x.EmailAddress == emailAddress);

    if (User == null) return false;

    return Crypter.Blowfish.CheckPassword(password, User.Password);
}

I used to use PBKDF2 but I was informed that bcrypt is better so here we are. While I understand the rule "never use your own auth", is this code likely to provide decent security?

What sort of vulnerabilities are present in implementing this system for security?

This is a great sample for review btw! I'm glad you posted this. Hopefully it will get some good google traffic.

I see a number of problems here however, and they're not all with the code:

Why are you generating a password in the first place? The user should be doing this! Are you going to hash and store this plain and destroy it? Or are you going to email it to them or show it on screen? In any case, you shouldn't be generating passwords for users if you can help it. If you need to represent state for a user account who has not yet set a password, use null, string.Empty or some other flag to model this state explicitly. Do not choose a password for them please.
If you absolutely must choose a random password, then make it secure. I contest this statement "this password should be more than adequate for most situations". Alphanumerics + special characters of at least 30-50 characters is more what you want. LastPass for example defaults to 30 chars and the full mix of symbols. Also, you're composing your randomly generated plain using RNGCryptoServiceProvider and Random! Do not do this! You want cryptographically secure random numbers which you will get with RNGCryptoServiceProvider but you won't with Random. So please never use Random for security related work.
Lastly, I don't believe Crypter.Blowfish.Crypt(PasswordBytes) is bcrypt. What is the package/reference for that call? I believe the package you want is BCrypt-Official which should look something like BCrypt.Net.BCrypt.HashPassword(passwordPlain, BcryptWorkfactor) when used. If what you have there is what I think it is, it's not an adaptive function which is possibly worse than MD5 or SHA1 (which for clarity are also not acceptable).

Here is an alternative to generating a simple plain securely:

    private static string GenerateSimpleKeySecurely(int keyLength)
    {
        // add whatever special characters in here you like
        var keyspace = @"0123456789abcdefghijklmnopqustuvwxyz()[]{}".ToCharArray();

        var data = new byte[1];
        using (var crypto = new RNGCryptoServiceProvider())
        {
            crypto.GetNonZeroBytes(data);
            data = new byte[keyLength];
            crypto.GetNonZeroBytes(data);
        }
        var result = new StringBuilder(keyLength);
        foreach (var b in data)
            result.Append(keyspace[b % (keyspace.Length)]);

        return result.ToString();
    }

Now go and read the OWASP: Password Storage Cheat Sheet.


Function to remove element from an array Below function takes an array, an element to be deleted and returns a new array minus the indicated element. Can this be optimized some how? How good is it generally?

public static int[] sbSample(int a[],int i){

    int size=0;
    int b[]=new int[a.length]; // provide length of new array
    for(int k=0;k<a.length;k++){    // iterate over elements of passed array
        if(!(a[k]==i)){    // search for elements other than to be deleted in passed array

            b[size]=a[k];     // store those arrays in new declared array
            size++;    // increment counter
        }
    }
    for(int k=0;k<size;k++){
        System.out.println(b[k]);
    }
    return b;
}

The methodname sbSample doesn't tell anything about what the method is doing and this will make debugging the code very hard if not impossible.

You should leave your methodparameters some space to breathe so int a[],int i should be int a[], int i. The same is true for variables and operators.

Comments should tell the reader of the code why something is done in the way it is done. Stating the obvious what is done is only adding noise to the code and should be removed. Let the code itself tell what is done by using meaningful names for classes, methods and variables.

This method is doing to much hence it is breaking the single responsibility principle. It is "deleting" array elements and displaying the resulting array values.

If the source array has 10 elements and 3 of them have the value which you want to delete, the resulting array still has 10 elements which is somehow strange. Deleting means IMO that the size of the array should shrink afterwards. How about using an ArrayList<T> adding the items which aren't satisfying the search condition and later call toArray() to return the resulting array ?