Emacs column alignment command I'm making my first steps in ELisp programming and I want to know is there a better solution for this problem:

I have a file with two columns, that are separated by one or more spaces, ex.:

One 1
Two 2
Three 3
Four  4

I'd like to make some alignment (columns are separated by at least two spaces):

One    1
Two    2
Three  3
Four   4

My solution is:

(defun align-config-file ()
  "Align two-columned config file in a pretty manner"
  (interactive)
  (let* ((strings (split-string (buffer-string) "\n"))
         (data (mapcar (lambda (line) (split-string line " ")) strings))
         (first-col-lengths (mapcar (lambda (item) (length (car item))) data))
         (first-col-length (+ 2 (apply 'max first-col-lengths))))
    (erase-buffer)
    (while data
      (let* ((this-line (car data))
             (first-col (car this-line))
             (last-col (car (nthcdr (- (length this-line) 1) this-line)))
             (amount-of-spaces (- first-col-length (length first-col)))
             (spaces-inserted 0))
        (insert first-col)
        (while (< spaces-inserted amount-of-spaces)
          (insert " ")
          (set 'spaces-inserted (+ 1 spaces-inserted)))
        (insert last-col)
        (insert "\n"))
      (set 'data (cdr data)))))

It is not beautiful, but it works. Is it ok?

1. Design

The design is not very Emacs-like. In particular:

The command operates on the entire buffer contents. This means that if you want to line up some columns in a region (without affecting the rest of the buffer), you can't use your command. (At least not without copying the region to a new buffer, running the command, and then copying the result back again.)

It would be better if the function operated on the contents of the current region: then it would be much more flexible.
Your command doesn't respect the current narrowing of the buffer. If you look at the docstring for erase-buffer, it says:

  Any narrowing restriction in effect (see narrow-to-region) is removed,
  so the buffer is truly empty after this.

This would come as an unpleasant surprise to a user who had a narrowing in effect. It would be better to operate only on the narrowed part of the buffer, by calling

(delete-region (point-min) (point-max))

instead of (erase-buffer). (But in fact there are better ways to do it, which I'll describe below.)
Your command doesn't preserve the position of point. After running the command, point is always at the end of the buffer. It's always worth preserving the position of point (if appropriate) because that makes a function's behaviour predictable, and more useful in Lisp code and in macros.

2. Implementation

The command reads the entire buffer contents into strings, processes the strings in various ways, and then writes them back to the buffer.

If you've recently come to Emacs Lisp from another language, then you're probably thinking that a buffer is a bit like a file, and so the right way to process it is to read the file contents into memory as string, process the contents, and then write it out.

I can't stress enough that you won't get the full benefits of Emacs if you persist with this approach! A buffer in Emacs is not like a file. It's a highly flexible string-like data structure that supports a much wider set of operations than a string. So you should organize your code to operate directly on the buffer contents, using the same (or similar) functions that you would use as an interactive user.
There are some useful functions in the Emacs toolbox that you could use to simplify your code. If you look at the revised code you'll see that I made use of skip-chars-forward, the (poorly named) just-one-space, and (cl-loop ... maximize ...) to do maximization without constructing an intermediate list in memory.

3. Minor points

The name align-config-file seems wrong: it's not limited to configuration files, and it doesn't operate on a file (it operates on buffer contents). I think a better name would be something like align-two-columns.
The docstring says "in a pretty manner" which seems rather vague. What does it actually do?
The command ensures that there are at least two spaces between the columns. The number 2 seems rather arbitrary: why not make it an argument to the command, so that the user can supply it as a prefix argument?

4. Revised code

(Ask if you can't figure out how this works.)

(require 'cl-macs) ; for the "cl-loop" facility

(defun align-two-columns (begin end spaces)
  "Align two columns between BEGIN and END, ensuring that the
second column is vertically aligned, and there are SPACES spaces
between the columns."
  (interactive "r\np")
  (save-excursion
    (save-restriction
      (narrow-to-region begin end)
      (goto-char (point-min))
      ;; Determine the width of the first column.
      (let ((width (cl-loop while (not (eobp))
                            maximize (skip-chars-forward "^ \t\n")
                            do (forward-line 1))))
        ;; Back to the top and adjust the spaces.
        (goto-char (point-min))
        (while (not (eobp))
          (let ((col (skip-chars-forward "^ \t\n")))
            (unless (eolp)
              (just-one-space (+ spaces (- width col)))))
          (forward-line 1))))))


Enhance function that justifies text On Codewars, there is a challenge where the goal is to justify an str into a certain len. Here are the rules:

  Use spaces to fill in the gaps between words.
  Each line should contain as many words as possible.
  Use '\n' to separate lines.
  Gap between words can't differ by more than one space.
  Lines should end with a word not a space.
  '\n' is not included in the length of a line.
  Large gaps go first, then smaller ones    ('Lorem--ipsum--dolor--sit-amet,' (2, 2, 2, 1 spaces)).
  Last line should not be justified, use only one space between words.
  Last line should not contain '\n'
  Strings with one word do not need gaps ('somelongword\n').

I have managed justify text correctly but there a few flaws that I am stumped on: a word that are too long for a length, and short length that causes nothing to happen. I would appreciate any possible solutions, feedback, advice, other flaws, etc. I am a beginner in JavaScript so code may seem messy.

function justify(str, len) {
   let splitStr = [];

   for (let i = 0; i < str.split(' ').length; i++) {
       if (str.split(' ')[i].length) {
           splitStr.push(str.split(' ')[i])
       }
   }

   let lines = [];
   let justified = [];

   while (true) {
       let line = [];
       let currentLine = true;

       while (currentLine) {
           if ((line.join(' ') + splitStr[0]).length < len) {
               line.push(splitStr.shift());

               if (splitStr.length === 0) {
                   break;
               }
           } else {
               break;
           }
       }

       lines.push(line.join(' '));

       if (splitStr.length === 0) {
           break;
       }
   }

   for (let i = 0; i < lines.length; i++) {
       line = lines[i].split('');

       if (line.length < len && i !== lines.length-1) {
           let neededSpaces = len - line.length;
           let spaces = [];

           for (let j = 0; j < line.length; j++) {
               if (line[j] === ' ') {
                   spaces.push(line[j]);
               }
           }

           while (neededSpaces > 0) {
               for (let k = 0; k < spaces.length; k++) {
                   spaces[k] = spaces[k] + ' ';
                   neededSpaces--;

                   if (neededSpaces === 0) {
                       break
                   }
               }
           }

           for (let j = 0; j < line.length; j++) {
               if (line[j] === ' ') {
                   line[j] = spaces.shift();
               }
           }

           justified.push(line.join(''));
        }
   }

   return justified.join('\n');
}
console.log(justify('Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making ' +
'it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, '+
'from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections ' +
'1.10.32 and 1.10.33 of "de Finibus Bonorum et Malorum" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ' +
'ethics, very popular during the Renaissance. The first line of Lorem Ipsum, "Lorem ipsum dolor sit amet..", comes from a line in section 1.10.32. The standard \n\
\chunk of Lorem Ipsum used since the 1500s is reproduced below for those interested. Sections 1.10.32 and 1.10.33 from "de Finibus Bonorum et Malorum" by Cicero \n\
are also reproduced in their exact original form, accompanied by English versions from the 1914 translation by H. Rackham.', 30)

Few improvements which can be seen right away are in this part

let splitStr = [];
for (let i = 0; i < str.split(' ').length; i++) {
  if (str.split(' ')[i].length) {
    splitStr.push(str.split(' ')[i])
  }
}

Can be re-written as:

const tempArray = str.split(' ')
let splitStr = tempArray.filter(data => data.length)

You can use Array.filter directly here.
Str.split(' ') is getting repeated at lot. So better save it in some variable and then re-use
Comparison inside if (length === 0) in other portions can be written as if(!length) since 0 is falsy


Long arithmetic addition in JS The implementation of the algorithm which adds two number in string form with arithmetic rules.

var assert = require('assert');

var strAdd = function(lnum, rnum) {
  lnum = lnum.split('').reverse();
  rnum = rnum.split('').reverse();
  var len = Math.max(lnum.length, rnum.length),
      acc = 0,
      res = [];
  for(var i = 0; i < len; i++) {
    var subres = Number(lnum[i] || 0) + Number(rnum[i] || 0) + acc;
    acc = ~~(subres / 10); // integer division
    res.push(subres % 10);
  }
  if (acc !== 0) {
    res.push(acc);
  }
  return res.reverse().join('');
};
assert(strAdd('1', '9') === '10');
assert(strAdd('1', '0') === '1');
assert(strAdd('5', '5') == '10');
assert(strAdd('2', '2') === '4');
assert(strAdd('20', '202') === '222');

Is there a better way(in terms of complexity) to achieve the same result?
Any style advice is appreciated.

Performance improvements related to approach

Interger base

For better performance we can split number not by every digit but by set of digits with given base. Look at Number.MAX_SAFE_INTEGER and some math

\$ max\_number = 2 ^ {53} - 1 = 9007199254740991 \$

\$ digits = \lfloor \log_{10} max\_number \rfloor \$

\$ base = 10 ^ { digits } \$

Note: base will be lower by one if logarithm gives interger.

What I want to say?

\$ max\_number \$ can hold \$ digits \$ digits to be able to handle overflowing during additions.

What meaning of \$ base \$ ?

Currently you are using algo with \$ base = 10 ^ {digits} = 10 \$.

Here res.push(subres % 10); you are performing modulo operation with base equals 10. Which means you are storing array with numbers that consists of single digit.

But Number allows you to store up to 15 digits. Execute Math.log10(Number.MAX_SAFE_INTEGER) in you browser to find this value (do not forger to take floor of resulted decimal).

Look at c++ example at e-maxx.ru for idea how to implement this. I have not found good enough english article, but you can use google translate.

Benchmark base \$ 10^1 \$ vs base \$ 10^{15} \$

function plain(lnum, rnum) {
  lnum = lnum.split('').reverse();
  rnum = rnum.split('').reverse();

  var len = Math.max(lnum.length, rnum.length)
    , acc = 0
    , res = [];

  for (var i = 0; i < len; i++) {
    var subres = Number(lnum[i] || 0) + Number(rnum[i] || 0) + acc;
    acc = ~~(subres / 10);
    res.push(subres % 10);
  }

  if (acc !== 0) {
    res.push(acc);
  }

  return res.reverse().join('');
}

DIGITS = Math.floor(Math.log10(Number.MAX_SAFE_INTEGER)) - 1
BIG_INTEGER_BASE = Math.pow(10, DIGITS);
FILL_STRING = (BIG_INTEGER_BASE + '').substr(1)

function readBigInteger(str, base) {
  var res = [];
  for (var i = str.length; i > 0; i -= base)
    if (i < base)
      res.push(Number(str.substr(0, i)));
    else
      res.push(Number(str.substr(i - base, base)));
  return res;
}

function printBigInteger(integer, base) {
  for (var i = 0; i + 1 < integer.length; ++i) {
      var s = FILL_STRING + integer[i]
      integer[i] = s.substr(s.length - base)
  }
  return integer.reverse().join('');
}

function plainWithDifferentBase (a, b) {
  lnum = readBigInteger(a, DIGITS)
  rnum = readBigInteger(b, DIGITS)

  var len = Math.max(lnum.length, rnum.length)
    , acc = 0
    , res = [];

  for (var i = 0; i < len; i++) {
    var subres = (lnum[i] || 0) + (rnum[i] || 0) + acc;
    acc = ~~(subres / BIG_INTEGER_BASE);
    res.push(subres % BIG_INTEGER_BASE);
  }

  if (acc !== 0) {
    res.push(acc);
  }

  return printBigInteger(res, DIGITS);
}

var fib = function(num, add) {
    var prev = '1',
        curr = '1',
        temp;
    while (curr.length < num) {
        temp = curr;
        curr = add(prev, curr);
        prev = temp;
    }
    return curr;
};

SIZE = 10000

console.time("plainWithDifferentBase");
fib(SIZE, plainWithDifferentBase);
console.timeEnd("plainWithDifferentBase");

console.time("plain");
fib(SIZE, plain);
console.timeEnd("plain");

Results

Using \$ base = 10^{15} \$ is 90 times efficiently.

plainWithDifferentBase: 23848ms
plain: 182856ms

Performance improvements related to interpreter

Look at this:

formatting list of variables
converting character to integer using charCodeAt
preallocating array of required size
using standard language library API (builtins)

function characterToInt(char) {
  // 48 is char code of zero.
  return char.charCodeAt(0) - 48;
}

function longAdd(lnum, rnum) {
  // Here we didn't use lambda function (anonimous) to prevent creating
  // additaional objects and give less work to GC.
  lnum = lnum.split('').reverse().map(characterToInt);
  rnum = rnum.split('').reverse().map(characterToInt);
  // With comma as third character you didn't broke approach of formating
  // using 2 whitespaces.
  var len = Math.max(lnum.length, rnum.length)
    , acc = 0
    // Allocate required space for an array to prevent reallocation overhead
    , res = new Array(len);
  for (var i = 0; i < len; ++i) {
    var subres = (lnum[i] || 0) + (rnum[i] || 0) + acc;
    // Use Math.floor instead of 2 additional operations, as it is library
    // function which can be written with some optimizations.
    acc = Math.floor(subres / 10);
    res[i] = subres % 10;
  }
  if (acc !== 0) {
    res.push(acc);
  }
  return res.reverse().join('');
}

Possible ways to allocate array of required size:

var a = new Array(size) // 1
var a = []; a.length = size // 2

Note: it is not final or most optimized version ever, I have tried to show you set of approached you might want to know to optimize your code even further.

Benchmark using browser

Just open console in your browser and paste following code

var strAdd = function(lnum, rnum) {
  lnum = lnum.split('').reverse();
  rnum = rnum.split('').reverse();
  var len = Math.max(lnum.length, rnum.length),
      acc = 0;
      res = [];
  for(var i = 0; i < len; i++) {
    var subres = Number(lnum[i] || 0) + Number(rnum[i] || 0) + acc;
    acc = ~~(subres / 10); // integer division
    res.push(subres % 10);
  }
  if (acc !== 0) {
    res.push(acc);
  }
  return res.reverse().join('');
};

function characterToInt(char) {
  return char.charCodeAt(0) - 48;
}

function longAdd(lnum, rnum) {
  lnum = lnum.split('').reverse().map(characterToInt);
  rnum = rnum.split('').reverse().map(characterToInt);
  var len = Math.max(lnum.length, rnum.length)
    , acc = 0
    , res = new Array(len);
  for (var i = 0; i < len; ++i) {
    var subres = (lnum[i] || 0) + (rnum[i] || 0) + acc;
    acc = Math.floor(subres / 10);
    res[i] = subres % 10;
  }
  if (acc !== 0) {
    res.push(acc);
  }
  return res.reverse().join('');
}

var fib = function(num, add) {
    var prev = '1',
        curr = '1',
        temp;
    while (curr.toString().length !== num) {
        temp = curr;
        curr = add(prev, curr);
        prev = temp;
    }
    return curr;
};

console.time("preallocated");
fib(1000, longAdd);
console.timeEnd("preallocated");

console.time("plain");
fib(1000, strAdd);
console.timeEnd("plain");

My results are:

Google Chrome 46.0.2490.80

preallocated: 1620.238ms
plain: 4311.755ms

Mozilla Firefox 41.0.2

preallocated: 849.72ms
plain: 3747.05ms

Nodejs and preallocation

Lets start with this test where we conditionally switches allocation from static to dynamic.

var plain = function(lnum, rnum) {
  lnum = lnum.split('').reverse();
  rnum = rnum.split('').reverse();
  var len = Math.max(lnum.length, rnum.length),
      acc = 0;
      res = [];
  for(var i = 0; i < len; i++) {
    var subres = Number(lnum[i] || 0) + Number(rnum[i] || 0) + acc;
    acc = ~~(subres / 10); // integer division
    res.push(subres % 10);
  }
  if (acc !== 0) {
    res.push(acc);
  }
  return res.reverse().join('');
};

var preallocated = function(lnum, rnum) {
  lnum = lnum.split('').reverse();
  rnum = rnum.split('').reverse();
  var len = Math.max(lnum.length, rnum.length),
      acc = 0;
      res = [];
  if (len > 1000)
      res.length = len;
  for(var i = 0; i < len; i++) {
    var subres = Number(lnum[i] || 0) + Number(rnum[i] || 0) + acc;
    acc = ~~(subres / 10); // integer division
    if (len > 1000)
      res[i] = subres % 10
    else
      res.push(subres % 10);
  }
  if (acc !== 0) {
    if (len > 1000)
      res = res.concat(acc)
    else
      res.push(acc);
  }
  return res.reverse().join('');
};

var fib = function(num, add) {
    var prev = '1',
        curr = '1',
        temp;
    while (curr.toString().length !== num) {
        temp = curr;
        curr = add(prev, curr);
        prev = temp;
    }
    return curr;
};

console.time("preallocated");
fib(2000, preallocated);
console.timeEnd("preallocated");

console.time("plain");
fib(2000, plain);
console.timeEnd("plain");

I have results:

$ node /tmp/help.js
preallocated: 1278ms
plain: 4249ms

$ node --version
v0.12.7

Warning: didn't run benchmark for node in you browser.

Google Chrome 46.0.2490.80 gives time

preallocated: 61737.298ms
plain: 16982.603ms

Mozilla Firefox 41.0.2

preallocated: 12759.65ms
plain: 14391.38ms

Summary

We have different results depending on platform. People love js because of 2 things:

event loop
same codebase for backend and frontend

But, as you saw before, you have to optimize frontend and backend in different way. How to solve it is up to you.


Print a array of size n*n, with borders as 1's and fill rest of element with 0's Assessment Question: Print an array of size n*n, with border as 1's and rest of the element as 0's.
Sample Input: 5
Sample Output:
[[1 1 1 1 1]
 [1 0 0 0 1]
 [1 0 0 0 1]
 [1 0 0 0 1]
 [1 1 1 1 1]]

Here is my solution:
import numpy as np
n = int(input("Enter a number:"))
np_box = np.zeros([n, n], dtype='int')
np_box[:, 0] = np_box[:, -1] = np_box[0] = np_box[-1] = 1
print(np_box)

Can this code be still optimised (using numpy, considering space and time complexity)?

Have you alread checked out np.pad?
import numpy as np
n = int(input("Enter a number:"))
np_box = np.pad(np.zeros((n-1,n-1)), 1, constant_values=1)


Microservice that fetches data from REST repository endpoints on Github I have code in Python 3 where I fetch data from the GitHub API, and I want some advice on the architecture of this code.
import urllib.request, urllib.parse, urllib.error
import json
import sys
from collections import Counter
from datetime import datetime, timedelta, date
from tabulate import tabulate
from LanguagesRepoCounter import LanguagesCounter
from RepoListLanguages import RepoListLanguages

#getting the date of one mounth ago from today
One_Mouth_ago_date = datetime.date(datetime.now()) - timedelta(days=30)

url = 'https://api.github.com/search/repositories?q=created:%3E'+One_Mouth_ago_date.isoformat()+'&sort=stars&order=desc&page=1&per_page=100'

data_connection_request  = urllib.request.urlopen(url)

unicoded_data = data_connection_request.read().decode()

try:
    json_data = json.loads(unicoded_data)
except:
    json_data = None
# testing retrieaved data
if not json_data or json_data["total_count"]<1:
    print(json_data)
    sys.exit('######## Failure To Retrieve ########')

#Counting the number of repo using every language
Lang_Dic_Count = LanguagesCounter(json_data)

# Getting the list of repo using every language
DicDataList = RepoListLanguages(json_data, Lang_Dic_Count)

# Displaye the repo's urls for every language
for key in DicDataList:
    print(key, ' language used in')
    for i in range(len(DicDataList[key])):
        print("                           ", DicDataList[key][i])

# Displaye the number of repos using every language in a table
table = []
for key in Lang_Dic_Count:
    under_table=[]
    under_table.append(key)
    under_table.append(Lang_Dic_Count[key])
    table.append(under_table)
print(tabulate(table, headers=('Language', 'number of Repo using it'), tablefmt="grid"))

and the first function is :
def LanguagesCounter(Data):
    ''' Count how many times every programming language is used in the 100 most starred repos created in the last 30 days given in a parameter as json object, return a dict with languages as a keys and number of repo using this language as a value
    '''
    Languages_liste = []
    for i in range(len(Data["items"])):
        Languages_liste.append(Data["items"][i]["language"])
    Lang_Dic_Count ={i:Languages_liste.count(i) for i in Languages_liste}
    return Lang_Dic_Count

the third function is:
def RepoListLanguages(Data, list):
    '''
    return the list of repositories from Data parameter for every programming language in the list parameter as a dictionary with the programming language as key and a list of repositories as value
    '''
    languagesRepo = {}
    for key in list:
        languagesRepo[key]= []
        for repo in range(len(Data["items"])):
            if (key == Data["items"][repo]["language"]):
                languagesRepo[key].append(Data["items"][repo]["html_url"])

    return languagesRepo

The code fetches the most starred repos from GitHub API that were created in the last 30 days, counts how many repos use every programming language, and lists the repo links for every language.

The two functions you broke out (LanguagesCounter, and RepoListLanguages) both could be simplified significantly using list comprehension. You seem to be thinking in terms of loops and appending to lists instead of generating them in one go which is more pythonic. Add in the Counter function from collections and RepoListLanguages becomes a relatively trivial one liner.
Those are both at the top of the rewrite below so you can see them in action.
Beyond that, the only real changes I made were using lower_camel_case per pep8 guidelines, changing a few variable names for clarity, and pulling the messy code at the bottom into functions. Functions and clear structure make reading the code much easier for anyone who needs to even if they don't change how it works.
import urllib.request, urllib.parse, urllib.error
import json
import sys
from collections import Counter
from datetime import datetime, timedelta, date
from tabulate import tabulate

from collections import Counter

def language_counter(data):
     return Counter(repo["language"] for repo in data["items"])

def repo_languages(data, list):
    return {
        key: [
            repo["html_url"] for repo in data["items"]
            if key == repo["language"]
        ]
        for key in list
    }

def new_url():
    last_month = datetime.date(datetime.now()) - timedelta(days=30)

    return 'https://api.github.com/search/repositories?q=created:%3E' + last_month.isoformat(
    ) + '&sort=stars&order=desc&page=1&per_page=100'

def get_data(url):
    data_connection_request = urllib.request.urlopen(url)

    unicoded_data = data_connection_request.read().decode()

    try:
            json_data = json.loads(unicoded_data)
    except:
            json_data = None

    return json_data

def validate_or_quit(json_data):
    if not json_data or json_data["total_count"] < 1:
        print(json_data)
        sys.exit('######## Failure To Retrieve ########')

def pretty_print_list(data):
    for key in data:
        print(key, ' language used in')
        for _, item in enumerate(data[key]):
            print(" " * 27, item)

def pretty_print_dict(data):
    data_list = [[key, value] for key, value in data.items()]
    print(
            tabulate(
                    data_list,
                    headers=('Language', 'number of Repo using it'),
                    tablefmt="grid")
    )

if __name__ == "__main__":
    json_data = get_data(new_url())
    validate_or_quit(json_data)

    lang_counts = language_counter(json_data)
    repo_langs = repo_languages(json_data, lang_counts)

    pretty_print_list(repo_langs)
    pretty_print_dict(lang_counts)


Spiral Scanning for matrix using recursion I have written code for scanning an N x M matrix in spiral order using a recursive function.

It uses 5 parameters - including 2 extra variables:

call to keep track of the no. of current call
counter to keep track of no. of elements already printed

Example:

For matrix:

1 2 3
4 5 6
7 8 9

For call = 0:
Output: 1 2 3 6 9 8 7 4 counter = 8

For call = 1:
Output: 5 counter = 9

Can the code be improved so that it uses fewer no. of variables and is more efficient?

#include <stdio.h>
#include <stdlib.h>
void spiralscan(int a[][20],int n,int m,int call,int counter)
{
    int i,j;
    if(counter>=n*m)
        return;
    //scan top
    //if(m>call)
    {   printf("\ntop\n");
        for(j=call;j<m-call;j++)
        {printf(" %d ",a[call][j]);
        counter++;
        }
    }
        if(counter>=n*m)
        return;
    //scan right
   // if(n-call>0)
    {   printf("\nright\n");
        for(i=call+1;i<n-call;i++)
        {printf(" %d ",a[i][m-call-1]);
        counter++;
        }
    }
        if(counter>=n*m)
        return;

    //scan bottom
 //   if(m-call>0)
    {   printf("\nbottom\n");
        for(j=m-2-call;j>=call;j--)
        {printf(" %d ",a[n-1-call][j]);
        counter++;
        }
    }
        if(counter>=n*m)
        return;
    //scan left
   // if(call<n/2)
       {   printf("\nleft\n");
        for(i=n-2-call;i>call;i--)
        {printf(" %d ",a[i][call]);
         counter++;
        }
    }
        if(counter>=n*m)
        return;

    spiralscan(a,n,m,call+1,counter);
}
int main()
{
    int n,m,i,j,a[20][20];
    printf("\t\t\tSpiral Scanning\n\n");
    printf("no. of rows : ");
    scanf("%d",&n);
    printf("\nno. of columns : ");
    scanf("%d",&m);
    printf("\nEnter matrix elements : \n");
    for(i=0;i<n;i++)
    {
        for(j=0;j<m;j++)
             scanf("%d",&a[i][j]);
    }
    printf("\n\n\t\tScanning Spirally\n\n");
    spiralscan(a,n,m,0,0);
    return 0;
}

It is very important you indent your code properly as it will make your code easier to read and less prone to errors.  This statement in particular is problematic:

if(counter>=n*m)
return;

You should use returns around your if blocks and loops even when they are single-statement bodies, you should indent your code so it is clear the return is part of the if block, and you should use spaces around your operators to make it easier to read:

if(counter >= n * m) {
    return;
}

Right here, you should remove the if, braces, and indentation:

    //scan bottom
 //   if(m-call>0)
    {   printf("\nbottom\n");
        for(j=m-2-call;j>=call;j--)
        {printf(" %d ",a[n-1-call][j]);
        counter++;
        }
    }

It doesn't make sense to have messy, hard-to-read code like this lying around.  Also, the final code shouldn't show fixed bugs and commented (dead) code - it should just be the working code and comments stating what it does as needed (I am not an advocate of a comment per line).

You should name your variables better:

void spiralscan(int a[][20],int n,int m,int call,int counter)

What are n and m anyway?  Also, you should put spaces after your commas.

It will probably be easier to read your code if you use PascalCase, snake_case, or camelCase to name your variables and functions becausevariableandfunctionnameswithnowordseparatorsarehardtoreadwhenverylong.  It seems snake_case is common among C programmers.


Stopwatch class in C++ I needed to implement a stop watch like class that will be used to measure running time of different functions by others. The final running time needs to be printed in 9h 3min 2s 23s 32ms 87us 78ns format. If the larger units are zero then they must be omitted. Time duration in a particular time-unit like seconds or milliseconds can be obtained from functions similar to hours. I need better ideas to improve the implementation.

#ifndef _STOP_WATCH_HPP_
#define _STOP_WATCH_HPP_

#include <chrono>
#include <utility>
#include <iostream>

class stop_watch {
    std::chrono::high_resolution_clock::time_point last_time_point;
    std::chrono::duration<double> time_duration;
    bool is_running;
  public:
    stop_watch() :
      last_time_point {std::chrono::high_resolution_clock::now()},
      time_duration {std::chrono::duration<double>::zero()},
      is_running {false}
      {}
    void start() {
      assert(!is_running); // stop watch is already running
      last_time_point = std::chrono::high_resolution_clock::now();
      time_duration = std::chrono::duration<double>::zero();
      is_running = true;
    }
    void stop() {
      assert(is_running); // stop watch is not running
      auto n = std::chrono::high_resolution_clock::now();
      time_duration = n - last_time_point;
      last_time_point = n;
      is_running = false;
    }
    double hours() {
      return std::chrono::duration_cast<std::chrono::hours>(
          time_duration).count();
    }
    friend std::ostream& operator<<(std::ostream& out, stop_watch sw) {
      std::chrono::hours h = std::chrono::duration_cast<
          std::chrono::hours>(sw.time_duration);
      if (h.count()) {out << h.count() << "h "; sw.time_duration -= h;}
      std::chrono::minutes m = std::chrono::duration_cast<
          std::chrono::minutes>(sw.time_duration);
      if (m.count()) {out << m.count() << "min "; sw.time_duration -= m;}
      std::chrono::seconds s = std::chrono::duration_cast<
          std::chrono::seconds>(sw.time_duration);
      if (s.count()) {out << s.count() << "s "; sw.time_duration -= s;}
      std::chrono::milliseconds ms = std::chrono::duration_cast<
          std::chrono::milliseconds>(sw.time_duration);
      if (ms.count()) {out << ms.count() << "ms "; sw.time_duration -= ms;}
      std::chrono::microseconds us = std::chrono::duration_cast<
          std::chrono::microseconds>(sw.time_duration);
      if (us.count()) {out << us.count() << "us "; sw.time_duration -= us;}
      std::chrono::nanoseconds ns = std::chrono::duration_cast<
          std::chrono::nanoseconds>(sw.time_duration);
      if (ns.count()) {out << ns.count() << "ns "; sw.time_duration -= ns;}
      return out;
    }
};

#endif

Sample usage

stop_watch sw;
sw.start();
// function(s) to measure time
sw.stop();
cout << "It took " << sw << endl;

I need some guideline on how to do this more efficiently, as my code is already too messy :(. I also have the following questions:

Is there a way to generate the functions for hours, seconds, etc. dynamically?
How can I improve the operator<< function? Is there a way to have a list of time units that I can iterate?

Misc.:

Modern compilers all support #pragma once. It's much cleaner than old-school include guards.
You need a #include <cassert>

Code Improvements - Class:

Use typedefs to reduce code duplication (and if you ever need to change the clock type, it's much simpler). These should be public.

using clock_t = std::chrono::high_resolution_clock;
using duration_t = clock_t::duration;

Store the duration type at the resolution of the clock, rather than using duration<double>.

clock_t::time_point last_time_point;
duration_t time_duration;

The hours function should be const, but I'd actually recommend replacing it with a function that returns the time in the most accurate units. The stopwatch class shouldn't worry about time conversions - only the user code knows the desired accuracy, and can duration_cast if necessary:

duration_t get_elapsed_time() const
{
    return time_duration;
}

Since the code asserts on the running state in start() and stop(), we probably need a function to check if the stopwatch is running:

bool get_is_running() const
{
    return is_running();
}

Code Improvements - Output:

Now that we have get_elapsed_time(), the ostream operator doesn't need to be a friend.
Use auto here too.
Doing the printing separately from extracting each unit of time makes things clearer.
The ostream operator should take the stopwatch by const&, not by value. (I doubt it makes much difference performance-wise, but it makes the intent and semantics clearer -
surely we shouldn't need to copy the stopwatch object to print the time?). Outsourcing the printing to a separate function also makes things a bit neater (more lines of code, but less duplication). It could be a lambda function if you wanted.

namespace detail
{

    template<class duration_t>
    void print_duration_if_non_zero(std::ostream& out, duration_t const& duration, std::string const& unitPostfix)
    {
        if (duration == duration_t::zero())
            return;

        out << duration.count() << unitPostfix;
    }

} // detail

std::ostream& operator<<(std::ostream& out, stop_watch const& sw) {

    auto t = sw.get_elapsed_time();

    auto h = std::chrono::duration_cast<std::chrono::hours>(t); t -= h;
    auto m = std::chrono::duration_cast<std::chrono::minutes>(t); t -= m;
    auto s = std::chrono::duration_cast<std::chrono::seconds>(t); t -= s;
    auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(t); t -= ms;
    auto us = std::chrono::duration_cast<std::chrono::microseconds>(t); t -= us;
    auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(t);

    detail::print_duration_if_non_zero(out, h, "h ");
    detail::print_duration_if_non_zero(out, m, "min ");
    detail::print_duration_if_non_zero(out, s, "s ");
    detail::print_duration_if_non_zero(out, ms, "ms ");
    detail::print_duration_if_non_zero(out, us, "us ");
    detail::print_duration_if_non_zero(out, ns, "ns ");

    return out;
}

It would be cleaner not to give stop_watch an ostream operator at all. If you create a print_time_to_stream(stream, time) that takes time as  std::chrono::nanoseconds (or whatever's the highest resolution you need in your app), then your print function can be used for any time, and not just for the stop_watch class!

Your Questions

get_elapsed_time() makes this unnecessary! User code should convert the duration when it needs to.
Since std::chrono::hours etc. are types, they can't just be added to a vector. I suppose you could do something like:

    auto times = std::vector<stop_watch::duration_t::rep>();
    times.push_back(std::chrono::duration_cast<stop_watch::duration_t>(std::chrono::hours(1)).count());
    // same for other desired types...

    auto units = std::vector<std::string>();
    units.push_back("h ");
    // ...

    print_times(std::cout, sw.get_elapsed_time(), times, units);

So the times vector contains the time to use in the units of stop_watch::duration_t. And then you can use integer division to get the necessary values instead of duration_casting.

Other Things

If you're just measuring function times, you might not need the start / stop logic. You could perhaps simplify the class to something like the following:

class timer
{
public:

    using clock_t = std::chrono::high_resolution_clock;
    using duration_t = clock_t::duration;

private:

    clock_t::time_point start_time;

public:

    timer():
        start_time(clock_t::now()) { }

    duration_t get_elapsed_time() const { return (clock_t::now() - start_time); }

    void reset() { start_time = clock_t::now(); }
};

There's no need to worry about leaving the timer "running", since there's no actual work unless you query the time...

timer sw;
std::this_thread::sleep_for(std::chrono::seconds(1));
std::cout << "It took: " << sw.get_elapsed_time().count() << "units" << std::endl;


Ensure only a single instance of static template class member I am interacting with a C library, whose documentation states that certain functions are not thread safe. Since its functions deal heterogeneously with multiple types, I have written a template wrapper to use it in a homogeneous manner from C++. However, I now need to ensure serialized access to certain functions, serialized across all instantiated template wrappers.

I believe the following approach should work; what I would like to know is, are there any caveats to the following approach, and if so, what are they and how can I get around them?

struct LibMutexPolicy
{
    static std::mutex & getMutexInstance()
    {
        static std::mutex inst;
        return inst;
    }
};

template <typename T>
struct A : private LibMutexPolicy
{
    void foo()
    {
        std::lock_guard<std::mutex> lock(getMutexInstance());
        // Call non thread-safe library code
    }
};

I believe the following approach should work; what I would like to know is, are there any caveats to the following approach, and if so, what are they and how can I get around them?

Consider a decorator implementation, with a visitor function for method access:

template<typename V>
class threadsafe_reference // decorator
{
public:
    using reference_type = V&;
    threadsafe_reference(reference_type v, std::mutex& m): value_(v), mutex_(m) {}

    // synchronized visitor access
    void sync( std::function<void(value_type&)> functor )
    {
        std::lock_guard<std::mutex> lock{ mutex_ };
        functor( value_ );
    }

    // you may wish to provide unsynchronized reference access here
private:
    reference_type value_;
    std::mutex& mutex_;
};

template<typename V>
threadsafe_reference<V> make_threadsafe(V& value, std::mutex& m)
{
    return threadsafe_reference<V>{ value, m };
}

Client code:

struct A // agnostic to threading
{
    void do_stuff_n_things( int i ) {};
};

A a; // need to pass this on two threads
mutex m;

auto tsa = make_threadsafe(a, m); // proxy accessor

Thread access:

 tsa.sync( []( A& self ) { self.do_things_n_stuff(10); } );

The code is incomplete (you'd need a specialization of sync for const access and so on) but it should be enough to give you a picture.

Advantages:

this is a non-intrusive decorator that you can use over virtually any type
separates concerns nicely and allows you to implement and test them separately (e.g. test A, then test the synchronization code)

Minor disadvantage:

the access to A's methods requires lambdas/external functions


Showing items as in a Carousel Is there a better way to implement my carousel in less lines?

    <% @photos.each_with_index do |photo, index| %>
      <% if index == 0 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

TITLE

      <% end %>
      <% if index == 1 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
      <% if index == 2 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
      <% if index == 3 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
      <% if index == 4 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
      <% if index == 5 %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
    <% end %>

Get rid of unnecessary code:

    <% @photos.each_with_index do |photo, index| %>
      <% if index.zero? %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

TITLE

      <% else %>

        <img src="<%= photo.image_url(:large) %>" alt="<%= photo.title %>">

      <% end %>
    <% end %>

Move stuff to helpers:

module PhotoHelpers
  def slider_image(photo)
    image_tag photo.image_url(:large), alt: photo.title
  end
end

    <% @photos.each_with_index do |photo, index| %>
      <% if index.zero? %>

          <%= slider_image photo %>

TITLE

      <% else %>

          <%= slider_image photo %>

      <% end %>
    <% end %>


Skipping over failed imports until they are needed (if ever) We have a program with many dependencies. Because the user may not want to use all of them, the program should skip over failed imports and only raise an error if absolutely necessary. e.g. if the user tries to call that class.

I have implemented a way of doing this but I'm not sure it is as good as it could be, it certainly seems somewhat inelegant. I presume there are also some overlooked bugs.

The important file structure is:

Program:
  - engines
    - __init__.py
    - base_engine.py
    - ...
    - psi4.py
    - rdkit.py
  - run.py

The base_engine.py contains the base class Engines which all the other engines (PSI4 in psi4.py, RDKit in rdkit.py ... ) inherit from, as well as a Default class.

If any class fails to import, Default is imported instead, but with the name of the class that failed to import (see the __init__.py file).

base_engine.py:

class Engines:
    """
    Engines base class containing core information that all other engines (PSI4, etc) will have.
    """

    def __init__(self, molecule):

        self.molecule = molecule

class Default:
    """
    If there is an import error, this class replaces the class which failed to be imported.
    Then, only if initialised, an import error will be raised notifying the user of a failed call.
    """

    def __init__(self, *args, **kwargs):
        # self.name is set when the failed-to-import class is set to Default.
        raise ImportError(
            f'The class {self.name} you tried to call is not importable; '
            f'this is likely due to it not being installed.')

__init__.py:

try:
    from .psi4 import PSI4
except ImportError:
    from .base_engine import Default as PSI4
    setattr(PSI4, 'name', 'PSI4')

try:
    from .rdkit import RDKit
except ImportError:
    from .base_engine import Default as RDKit
    setattr(RDKit, 'name', 'RDKit')

psi4.py:

from Program.engines.base_engine import Engines

# Example import that would fail
import abcdefg

class PSI4(Engines):
    """
    Writes and executes input files for psi4.
    """

    def __init__(self, molecule):

        super().__init__(molecule)

    def generate_input(self):

        ...

run.py:

from Program.engines import PSI4

PSI4('example_molecule').generate_input()

So now when classes are imported at the top of the run.py file, there is no problem even if there's an import error; if there's a failed import with PSI4 because abcdefg cannot be imported, Default is simply imported as PSI4 and given the name PSI4 via the setattr().

Then, only if that Default class is called the ImportError is raised and the user can see that the issue was with PSI4.

This seems to work quite well, even when there are multiple failed imports. We can also add extended error messages for each different failed import. Is this the best way of doing something like this, though? It gets quite messy since we have so many files in our engines package.

Please let me know if some relevant code has been omitted and I can add it back. Thanks for any help.

EDIT for @jpmc26:

I appreciate the time spent on your post but it's not practical for us. The program still fails fast (when necessary) because the imports are first initialised when configs are set. We have ~12 (large) stages of execution, each with multiple options which are handled by these configs. We handle this by reading the configs and terminal commands and calling whatever the option is e.g. EngineForStageFive = PSI4 is set from the configs, then we just call EngineForStageFive(args).do_something() where do_something() exists for all of the stage five classes available. All of this is nicely handled by our terminal command and config file interpreters.

The PSI4 class for example is called many times and repeatedly calling for its import with some additional logic is not what we want in our run file. We'd end up repeating a lot of code unnecessarily. e.g. every stage would need a long if/elif chain or dictionary to determine how it would be used.

There is one issue with your approach which is, in the case of multiple import failure, only the last one can be properly reported:

>>> try:
...   from .foo import Foo
... except ImportError:
...   from .base_engine import Default as Foo
...   setattr(Foo, 'name', 'Foo')
...
>>> try:
...   from .bar import Bar
... except ImportError:
...   from .base_engine import Default as Bar
...   setattr(Bar, 'name', 'Bar')
...
>>> Bar()
Traceback (most recent call last):
…
ImportError: The class Bar you tried to call is not importable; this is likely due to it not being installed.
>>> Foo()
Traceback (most recent call last):
…
ImportError: The class Bar you tried to call is not importable; this is likely due to it not being installed.

You instead want to generate a new class with the proper message each time.

Something along the line should do:

def mock_missing(name):
    def init(self, *args, **kwargs):
        raise ImportError(
            f'The class {name} you tried to call is not importable; '
            f'this is likely due to it not being installed.')
    return type(name, (), {'__init__': init})

Usage being:

try:
    from .foo import Foo
except ImportError:
    Foo = mock_missing('Foo')


Find a list of Windows Services by given criteria My goal is to implement a method, which finds a list of Windows Services that match given set of criteria:

SearchString: A search string
SearchServiceBy: Could be ServiceName or DisplayName
SearchOption: Could be Equals, Contains, StartsWith, EndsWith or Regex
SearchServiceType: Could be SingleService or GroupOfServices

private List<ServiceController> GetServices()
{
    var allServices = ServiceController.GetServices().ToList();
    var resultList = new List<ServiceController>();

    switch (SearchOption)
    {
        case SearchOption.Equals:
            if (SearchServiceBy == SearchServiceBy.ServiceName)
            {
                resultList = allServices.Where(s => s.ServiceName.Equals(SearchString)).ToList();
            }
            else if (SearchServiceBy == SearchServiceBy.DisplayName)
            {
                resultList = allServices.Where(s => s.DisplayName.Equals(SearchString)).ToList();
            }
            break;

        case SearchOption.Contains:
            if (SearchServiceBy == SearchServiceBy.ServiceName)
            {
                resultList = allServices.Where(s => s.ServiceName.Contains(SearchString)).ToList();
            }
            else if (SearchServiceBy == SearchServiceBy.DisplayName)
            {
                resultList = allServices.Where(s => s.DisplayName.Contains(SearchString)).ToList();
            }
            break;

        case SearchOption.StartsWith:
            if (SearchServiceBy == SearchServiceBy.ServiceName)
            {
                resultList = allServices.Where(s => s.ServiceName.StartsWith(SearchString)).ToList();
            }
            else if (SearchServiceBy == SearchServiceBy.DisplayName)
            {
                resultList = allServices.Where(s => s.DisplayName.StartsWith(SearchString)).ToList();
            }
            break;

        case SearchOption.EndsWith:
            if (SearchServiceBy == SearchServiceBy.ServiceName)
            {
                resultList = allServices.Where(s => s.ServiceName.EndsWith(SearchString)).ToList();
            }
            else if (SearchServiceBy == SearchServiceBy.DisplayName)
            {
                resultList = allServices.Where(s => s.DisplayName.EndsWith(SearchString)).ToList();
            }
            break;

        case SearchOption.Regex:
            var pattern = new Regex(SearchString);

            if (SearchServiceBy == SearchServiceBy.ServiceName)
            {
                resultList = allServices.Where(s => pattern.IsMatch(s.ServiceName)).ToList();
            }
            else if (SearchServiceBy == SearchServiceBy.DisplayName)
            {
                resultList = allServices.Where(s => pattern.IsMatch(s.DisplayName)).ToList();
            }
            break;

        default:
            throw new ArgumentException("Unknown SearchOption: {0}", SearchOption.ToString());
    }

    if (SearchServiceType == SearchServiceType.SingleService
        &&
        resultList.Count > 1)
    {
        resultList = new List<ServiceController> { resultList.FirstOrDefault() };
    }

    return resultList;
}

Is there any better way to implement this?

There is no additional complex logic around the method invoke. The use case is simple: Wix Toolset Custom Action passes required parameters to the method, gets a list of services match the given criteria. So these services could be started/stopped further.

These are my suggestions:

Get rid of all the ugly and duplicate ifs by replacing them with two dictionaries where each one returns a Func.
Use Regex for all searching criteria by creating appropriate patterns or returning the SearchString if it's the pattern.

Example:

var propertygGetters = new Dictionry<SearchServiceBy, Func<Service, string>>
{
    [SearchServiceBy.ServiceName] => s => s.ServiceName,
    ...
}

var patternCreators = new Dictionry<SearchOption, Func<string, string>>
{
    [SearchOption.Equals] => s => $"^{s}$",
    [SearchOption.Contains] => s => $"{s}",
    [SearchOption.StartsWith] => s => $"^{s}",
    [SearchOption.EndsWith] => s => $"{s}$",
    [SearchOption.Regex] => s => s,
}

var getProperty = searchStringGetters[SearchServiceBy];
var createPattern = patternCreators[SearchOption];

var pattern = createPattern(SearchString);

return
    allServices
        .Where(service => Regex.IsMatch(getProperty(service), pattern))
        .ToList();

(warning: notepad programming)


Golang HTTP request retry code I believe that clean and elegant retry code is notoriously difficult, so I'm wondering how the following code could be improved. The retryGet function should have the same interface and contract as http.Get, except for the number of attempts argument, of course.

func retryGet(url string, maxAttempts int) (*http.Response, error) {
    attempts := 0

    if maxAttempts < 1 {
        return nil, errors.New("maxAttempts must be at least 1")
    }

    for {
        attempts++
        response, err := http.Get(url)
        if err == nil && response.StatusCode == http.StatusOK {
            return response, nil
        }

        delay, retry := shouldRetry(maxAttempts, attempts, response)
        if !retry {
            return response, err
        }

        if err == nil {
            defer response.Body.Close()
        }

        time.Sleep(delay)
    }
}

func shouldRetry(maxAttempts, attempts int, response *http.Response) (time.Duration, bool) {
    if attempts >= maxAttempts {
        return time.Duration(0), false
    }

    delay := time.Duration(attempts) * time.Second

    if response != nil && response.Header.Get("Retry-After") != "" {
        after, err := strconv.ParseInt(response.Header.Get("Retry-After"), 10, 64)
        if err != nil && after > 0 {
            delay = time.Duration(after) * time.Second
        }
    }

    return delay, true
}

I would recommend you to actually strictly match the http.Get signature using a struct:

type Getter func(url string) (*http.Response, error)

type RetryingClient struct {
    Getter Getter
    MaxAttempts int
}

func (rc RetryingClient) Get(url string) (*http.Response, error) {
    ...
}

And instead of directly calling http.Get, the embedded Getter interface gives you much more flexibility:

for testing
for using a proxy

Why do you defer the response.Body.Close() ? Since it won't be use, you can close it immediately.

Using the interface, you could make a function embedded in the struct to compute a default delay instead of hard-coded time.Duration(attempts) * time.Second.
For instance: defaultDelay(attempt int) time.Duration


Validating Shift class through builder pattern I have a Shift class that looks like this:

public class Shift
{
    public string Name { get; private set; }
    public TimeSpan StartHour { get; private set; }
    public TimeSpan EndHour { get; private set; }

    public TimeSpan Duration => EndHour - StartHour;

    private Shift()
    {
    }
}

Since there are some limitations and restrictions to what StartHour && EndHour can be I needed some way to validate the input of the class and if it fails - prevent the creation of the object or at least not return it to the user, without throwing an exception because I want to allow the user to re-enter the arguments if some of them were invalid.

Than I created an interface and some derived classes to be injected during the construction of the object:

public interface IShiftValidator
{
    Exception FailedValidationException { get; }
    string FailedValidationMessage { get; }
    bool IsValid(Shift shiftToValidate);
}

public class ShiftRangeValidator : IShiftValidator
{
    public Exception FailedValidationException { get; private set; }
    public string FailedValidationMessage { get; private set; } = string.Empty;

    public bool IsValid(Shift shiftToValidate)
    {
        if (shiftToValidate.Duration.TotalMinutes <= 0)
        {
            FailedValidationException = new ArgumentOutOfRangeException(nameof(shiftToValidate.Duration));
            FailedValidationMessage = "Shift's duration cant be less than 1 minute.";
            return false;
        }
        if (shiftToValidate.Duration.TotalHours > SharedSettings.MaximumShiftHours)
        {
            FailedValidationException =
                new ArgumentOutOfRangeException(nameof(shiftToValidate.Duration));
            FailedValidationMessage = "Shift's duration cant be longer than the maximum allowed hours.";
            return false;
        }
        return true;
    }
}

After that I implemented the builder pattern as a nested class in the Shift class, so I can make the constructor of the class private and construct the object fully in the builder class:

public class ShiftBuilder
{
    private Shift _shift;

    public ShiftBuilder()
    {
        _shift = new Shift();
    }

    public ShiftBuilder WithName(string name)
    {
        _shift.Name = name;
        return this;
    }

    public ShiftBuilder WithStartHour(TimeSpan startHour)
    {
        _shift.StartHour = startHour;
        return this;
    }

    public ShiftBuilder WithEndHour(TimeSpan endHour)
    {
        _shift.EndHour = endHour;
        return this;
    }

    public Shift Build(IEnumerable<IShiftValidator> validators)
    {
        if (validators != null)
        {
            foreach (var shiftValidator in validators)
            {
                if (!shiftValidator.IsValid(_shift))
                {
                    if (!string.IsNullOrEmpty(shiftValidator.FailedValidationMessage))
                    {
                        MessageBox.Show(shiftValidator.FailedValidationMessage);
                    }
                    throw shiftValidator.FailedValidationException;
                }
            }
        }
        Shift shift = _shift;
        _shift = null;
        return shift;
    }
}

Here's an example usage:

Shift.ShiftBuilder shiftBuilder = new Shift.ShiftBuilder();
var a = shiftBuilder.WithName("Test")
    .WithStartHour(new TimeSpan(12, 0, 0))
    .WithEndHour(new TimeSpan(24, 0, 0))
    .Build(new[] { new ShiftRangeValidator() });

var b = shiftBuilder.Build(null); // null
var c = shiftBuilder.Build(new[] {new ShiftRangeValidator()}); // crash

I have few concerns:

Is it clear that you're unable to reuse the same ShiftBuilder twice?
Is IShiftValidator good as it is or should I separate it in few smaller interfaces?

Don't be mad at me because I will again suggest you a different approach than your current implementation that exists in .Net for exactly that purpose, this is, validating models ;-)

But first a few comments about this one.

The ShiftBuilder requires a collection of validators to build the object IEnumerable<IShiftValidator>. This is a good start because you can use various validations however the current ones don't follow this principle yet. The example you showed has two responsibilities, this means it validates two things:

if (shiftToValidate.Duration.TotalMinutes <= 0)
{
    FailedValidationException = new ArgumentOutOfRangeException(nameof(shiftToValidate.Duration));
    FailedValidationMessage = "Shift's duration cant be less than 1 minute.";
    return false;
}
if (shiftToValidate.Duration.TotalHours > SharedSettings.MaximumShiftHours)
{
    FailedValidationException =
        new ArgumentOutOfRangeException(nameof(shiftToValidate.Duration));
    FailedValidationMessage = "Shift's duration cant be longer than the maximum allowed hours.";
    return false;
}

There are two ifs where each can fail. This means you need to split this validator into more specific one to test only one condition at a time.

Other then this I find this is a very complex implementation that leads me to the alternative suggestion which uses the ValidationAttribute. C# already has a system for constructing model validations. Here's a small example.

You can create a bunch of custom attributes that you use to decorate the properties of the Shift class with.

public class Shift
{
    public string Name { get; private set; }
    public TimeSpan StartHour { get; private set; }

    [GreaterThen(nameof(StartHour))]
    public TimeSpan EndHour { get; private set; }

    [GreaterThenZero]
    public TimeSpan Duration => EndHour - StartHour;

    public Shift()
    {
        // Just an example to make the validation fail.
        StartHour = TimeSpan.FromSeconds(100);
        EndHour = TimeSpan.FromSeconds(50);
    }
}

There are various ways of implementing them so you'll need to read more about them to know all options. I show you one of them where they implement the IsValid method in a very generic way (might not be perfect yet but it should make the point).

So you may need one attribute to validate the Duration:

public class GreaterThenZeroAttribute : ValidationAttribute
{
    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        return
            ((IComparable)value).CompareTo((IComparable)TimeSpan.FromSeconds(0)) > 0
                ? ValidationResult.Success
                : new ValidationResult($"{validationContext.MemberName} must be greater then zero");
    }
}

but on the other hand if you add GreaterThenAttribute the GreaterThenZeroAttribute for the Duration is no longer necessary.

public class GreaterThenAttribute : ValidationAttribute
{
    private readonly string _otherPropertyName;
    public GreaterThenAttribute(string otherPropertyName)
    {
        _otherPropertyName = otherPropertyName;
    }

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        var otherProperty =
            validationContext
                .ObjectType
                .GetProperty(_otherPropertyName)
                .GetValue(validationContext.ObjectInstance);

        return
            ((IComparable)value).CompareTo((IComparable)otherProperty) > 0
                ? ValidationResult.Success
                : new ValidationResult($"{validationContext.MemberName} must be greater then {_otherPropertyName}");
    }
}

You may however need a new attribute for

shiftToValidate.Duration.TotalHours > SharedSettings.MaximumShiftHours

to make sure the duration does not exceed a certain number like MaxHoursFromSettings etc. and you can read the actual value from the settings later.

Finally to invoke the validation you use the Validator

var shift = new Shift();
Validator.ValidateObject(shift, new ValidationContext(shift), true);


Translate English to Pig Latin This is a program for converting English to the pseudo-language Pig Latin. Not the strictest form of Pig Latin but it most likely will accurately output an argument in Pig Latin. I got the idea from a list of ideas on Github. Is there any way I can improve it concisely?

def pig_latin(message):
    consonants = ['b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q', 'r', 's', 't', 'v', 'w', 'x', 'y',
                  'z']
    new_message = ''

    for word in message.split():
        ay = ['a', 'y']
        listed_word = list(word)
        word_copy = word
        moved_consonants = ''

        for letter in listed_word.copy():
            if letter.lower() == 'y':
                if letter in word_copy[0]:
                    moved_consonants += letter
                    listed_word.remove(letter)
                else:
                    break
            elif letter.lower() in consonants:
                moved_consonants += letter
                listed_word.remove(letter)
            else:
                break

        listed_word.append('-' + moved_consonants + ''.join(ay))
        new_message += ''.join(listed_word) + ' '

    return new_message

print(pig_latin('Can you read pig latin?'))

First, at the top you list all the consonants out. There are two things that can be improved here:

Since you only use it to check whether or not something is a consonant, it should be a set. It's much more efficient to to a membership lookup on a set than it is to do one on a list. Just replace the [] with {}.

consonants = {'b', 'c', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'm', 'n', 'p', 'q', 'r', 's', 't', 'v', 'w', 'x', 'y', 'z'}

Second, there's a slightly less painful way to generate those letters. Python's string module contains a ascii_lowercase built-in that holds 'abcdefghijklmnopqrstuvwxyz'. You can use that along with a set of vowels to limit what letters need to be hard-coded:

import string as s

vowels = {'a', 'e', 'i', 'o', 'u'}
consonants = set(s.ascii_lowercase) - vowels  # Consonants are the set of letters, minus the vowels

I personally prefer this way.

You could also just change your algorithm to use vowels directly.

Just to clear something up,

word_copy = word

does not create a copy of word. This just creates a second name for the word string. For Strings this doesn't matter because Strings are immutable, but with a mutable object, this will bite you:

my_list = []
list_copy = my_list  # Does not actually create a copy!
my_list.append(1)
print(my_list, list_copy)  # prints [1] [1]

Notice how both lists were added to. This happens because there's actually only one list. Both names are referring to the same list.

For the sake of clarity, I'd rename it to say what it's purpose is. Offhand though, I can't see the need for word_copy at all! It would make sense if it was being used as an accumulator for a loop or something, but the only time its ever used is at word_copy[0], and since you never reassign word, you could just do word[0]. I'd simply get rid of word_copy.

Along the same lines, I'd reconsider ay. The name you've given it is exactly as descriptive as the string it contains, and is only ever used in one place. At the very least, I'd rename it to something meaningful:

pig_latin_suffix = ['a', 'y']

I'll also note that there's no reason to use a list of Strings here instead of a multi-character String. They behave the same in this case:

" ".join(['a', 'y'])
'a y'

" ".join("ay")
'a y'

Strings are iterable just like lists are.

I think pig_latin is too big. It's doing two main jobs: breaking the message into words, and processing the words. I would make the processing step its own function:

def process_word(word):
    ay = ['a', 'y']
    listed_word = list(word)
    word_copy = word
    moved_consonants = ''

    for letter in listed_word.copy():
        if letter.lower() == 'y':
            if letter in word_copy[0]:
                moved_consonants += letter
                listed_word.remove(letter)
            else:
                break
        elif letter.lower() in consonants:
            moved_consonants += letter
            listed_word.remove(letter)
        else:
            break

    listed_word.append('-' + moved_consonants + ''.join(ay))

    return ''.join(listed_word)

def pig_latin(message):
    new_message = ''

    for word in message.split():
        processed_word = process_word(word)

        new_message += processed_word + ' '

    return new_message

process_word could arguably be broken down further. This is already much better though. The immediate benefit is now you can test individual words, and not have to worry about how the rest of the code will react:

print(process_word("Can"))  # Prints 'an-Cay'


Overload binary operator for vector I want to be able to construct and fill a vector of bytes using a stream (binary shift) operators. So I overloaded operator << (...).

Here is the code I want to write :

using ByteVector = std::vector<uint8_t>;

void send_message(MessageId id, std::vector<ByteVector> parameters);

void foo()
{
    std::string str = "A string object";
    int bar = 42;

    ByteVector baz = {0x42, 0x13, 0x37};

    send_message(MessageId::COMMAND, {
        ByteVector() << str,
        ByteVector() << bar,
        ByteVector() << baz
    });
}

I started by adding the following operator overloads :

ByteVector & operator << (ByteVector & bv, const std::string & str);
ByteVector & operator << (ByteVector & bv, uint8_t byte);
ByteVector & operator << (ByteVector & bv, const ByteVector & bv2);
ByteVector & operator << (ByteVector & bv, int value);

They were working well for following use case:

ByteVector bv;
std::string str = "Never gonna give you up";
bv << str;

But not the for previous example where my compiler (clang) shows the following error:

error: invalid operands to binary expression ('ByteVector' (aka 'vector<unsigned char>') and 'const std::string' (aka 'const basic_string<char, char_traits<char>, allocator<char> >'))

note: candidate function not viable: expects an l-value for 1st argument
ByteVector & operator << (ByteVector & bv, const std::string & str);
             ^
note: candidate function not viable: expects an l-value for 1st argument
ByteVector & operator << (ByteVector & bv, uint8_t byte);
             ^

note: candidate function not viable: expects an l-value for 1st argument
ByteVector & operator << (ByteVector & bv, const ByteVector & bv2);
             ^

note: candidate function not viable: expects an l-value for 1st argument
ByteVector & operator << (ByteVector & bv, int value);
             ^

I didn't really understand the error. I think the compiler want's to use the ByteVector() in ByteVector() << something  as a rvalue (ByteVector &&).

So I added the following operator overload:

template<typename T>
ByteVector && operator << (ByteVector && bv, T data)
{
    return std::move(bv << data);
}

This overload solve the compiler error. But I would like to know if my implementation is good and optimal in terms of good practice, memory usage and efficiency.

Instead of returning an rvalue reference, it's better to return bv by value:

template<typename T>
ByteVector operator<< (ByteVector && bv, T data)
{
    return bv << data;
}

C++ compilers perform return value optimisation, which means that the return value won't be copied in this case.

An alternative approach is to ignore the advice not to subclass standard library containers, and implement your operators as member methods.  That will remove the need to overload for rvalue types, and I believe it's safe to do so for this use.


Multiple custom file input fields with jQuery I've modified this example to better suit my needs in handling multiple custom file uploads on the same page. It also renders the filename or truncates if it is too long for each of the 3 file fields. This solution works well however, I think it could be DRY'd out a lot? This is where I landed:

$(document).on('change', '.compliance-guide :file', function() {
  var input = $(this),
      label = input.val().replace(/\\/g, '/').replace(/.*\//, '');
  input.trigger('fileselect', [label]);
});

$(document).on('change', '.compliance-other :file', function() {
  var input = $(this),
      label = input.val().replace(/\\/g, '/').replace(/.*\//, '');
  input.trigger('fileselect', [label]);
});

$(document).on('change', '.agreement-file :file', function() {
  var input = $(this),
      label = input.val().replace(/\\/g, '/').replace(/.*\//, '');
  input.trigger('fileselect', [label]);
});

$(document).ready( function() {
    $('.compliance-guide :file').on('fileselect', function(event, label) {
        var log = label;
        if( log.length > 14 ) {
            $('.compliance-guide-name').html(log.substring(0,14)+'...');
        } else {
            $('.compliance-guide-name').html(log);
        }
    });
    $('.compliance-other :file').on('fileselect', function(event, label) {
        var log = label;
        if( log.length > 14 ) {
            $('.compliance-other-name').html(log.substring(0,14)+'...');
        } else {
            $('.compliance-other-name').html(log);
        }
    });
    $('.agreement-file :file').on('fileselect', function(event, label) {
        var log = label;
        if( log.length > 14 ) {
            $('.agreement-file-name').html(log.substring(0,14)+'...');
        } else {
            $('.agreement-file-name').html(log);
        }
    });
});

You could certainly combine the first 3 handlers into 1, since they do the exact same thing:

$(document).on('change', ':file', function () {
  var input = $(this),
      filename = input.val().replace(/\\/g, '/').replace(/.*\//, '');
  input.trigger('fileselect', [filename]);
});

That'll add an event handler to all file inputs on the page.

You can simplify the replace stuff a bit; there's no need to make backslashes into regular slashes, if you're just going to throw it all away afterward. So it'd be simpler to say

input.val().replace(/^.*[\\\/]/g, '')

i.e. remove everything up-to-and-including the last slash.

Handling the custom fileselect event is where you want to limit it to specific inputs. However, you can extract the truncate logic, to clean things up, and avoid the repetition.

function truncateFilename (filename) {
  if(filename.length > 14) {
    return filename.substring(0, 14) + "...";
  }
  return filename;
}

And since you've stuck to a naming convention for the elements you want to display the filenames, you can create a function to set up your event handling:

function addFileSelectHandler(inputClass) {
  $(inputClass).on("fileselect", function (event, filename) {
    $(inputClass + "-name").text(truncateFilename(filename));
  });
}

addFileSelectHandler(".compliance-guide");
addFileSelectHandler(".compliance-other");
addFileSelectHandler(".agreement-file");

Other notes:

Don't use .html() when you're just setting plain text strings; use .text() (as seen above)
Don't use classes to identify unique elements; use IDs. It's what they're for.
CSS has a text-overflow rule that'll auto-truncate text if it overflows - no need for JavaScript at all, and no need for a hard-coded length of 14. As always, letter-count has little to do with how much space it takes to display; "m"s take up a lot more space than "i"s (if you're using a non-monospaced font).

Also, you could choose to handle all of this in the generic $(:file).on("change",.. function. Instead of using specific classes on the elements that should show a filename, you could use a data-input attribute to link them to a file input.

For instance, you could have this markup:

<input type="file" id="some-file">

<input type="file" id="some-other-file">

And this JS:

$(document).on('change', ':file', function () {
  var input = $(this),
      filename = input.val().replace(/^.*[\\\/]/g, '');

  $(".filename[data-input='" + this.id + "']").text(filename);

  // you can still trigger your custom event, if you want
  // input.trigger('fileselect', [filename]);
});

That'll automatically show the filename, provided there's an element to show it. If there's no element, well, then it's not shown.


Checking whether a grade combination exists Below function checks if a grade exists

  ----------------------------------------------------------------------------
  -- CHECK IF THE VALUES WE PASSED AS PARAMETERS EXISTS AS A ONE ROW RECORD IN THE TABLE
  ----------------------------------------------------------------------------
  function record#exists(
    i_math_grade              pls_integer
   ,i_science_grade           pls_integer
   ,i_english_grade           pls_integer
   ,i_music_grade             pls_integer
   ,i_average                 pls_integer
  )
  return boolean
  is
    l_cnt                     pls_integer;
  begin
    begin
      select count(*)
      into   l_cnt
      from   grade
      where  math_grade    = i_math_grade
        and  science_grade = i_science_grade
        and  english_grade = i_english_grade
        and  music_grade   = i_music_grade
        and  average       = i_average;
    exception
      when no_data_found then
        return false;
    end;

    return l_cnt > 1;

  exception
    when others then
      raise_fa_err('record#exists');
  end record#exists;

This is very much the same idea I'm using to encapsulate "business logic" queries for re-use in PL/SQL code. So if you're looking for a validation of a concept you'll pass my review :) In a comment to the other answer you said this is just a snippet from a package code - that's good you're using packages. For PL/SQL apps in practice all code should be located in a package - a standalone subprograms a rare.

The notes:

select count is always guaranteed to return a single number so the exception handler for no_data_found is a dead code and should be removed.
If you're just checking for existence there's no point to count beyond 1, thus enter rownum = 1. This also guarantees the select returns either 0 or 1. This might or might not help your query performance (check explain plan).
It's a bit questionable if such an utility subprogram should record the exception or not but more information about your error handling implementation would be required to comment further. I usually don't but I'll let the exception propagate to the caller context and then record the full stack trace there. The good thing is that you re-raise (I just assume that from the name of raise_fa_err).

Modified example with the points #1 and #2 applied:

function record#exists(
  i_math_grade    in pls_integer
 ,i_science_grade in pls_integer
 ,i_english_grade in pls_integer
 ,i_music_grade   in pls_integer
 ,i_average       in pls_integer
) return boolean is
  l_exists pls_integer;
begin
  select count(*)
  into   l_exists
  from   grade
  where math_grade    = i_math_grade
    and science_grade = i_science_grade
    and english_grade = i_english_grade
    and music_grade   = i_music_grade
    and average       = i_average
    and rownum        = 1
  ;

  return l_exists = 1;
exception
  when others then
    raise_fa_err('record#exists');
end record#exists;


Picking weighed enumerable values at random I have a number of enumerables, and I want to randomly pick one value from one of the enumerables so that each value has a certain weight in the randomisation process. This is simplification of my current code:

private enum ActionPlan //Just an example, there can be many different enums
{
    DoNothing,
    Eat,
    StealFoodCovertly,
    AttackAndSteal,
}

private ActionPlan PickPlan()
{
    double[] odds = new double[Enum.GetNames(typeof(ActionPlan)).Length];
    odds[(int)ActionPlan.DoNothing] = 2.5;
    odds[(int)ActionPlan.Eat] = 3;
    odds[(int)ActionPlan.StealFoodCovertly] = 1;
    odds[(int)ActionPlan.AttackAndSteal] = 0.5;

    ActionPlan plan = (ActionPlan)PickOption(odds);
    return plan;
}

private int PickOption(double[] odds)
{
    double totalOdds = 0;
    foreach (double odd in odds)
    {
        totalOdds += odd;
    }
    double pick = rand.NextDouble() * totalOdds;
    for (int i = 0; i < odds.Length; ++i)
    {
        pick -= odds[i];
        if (pick <= 0)
        {
            return i;
        }
    }
    return odds.Length - 1; // In case there are some precision shenanigans
}

I can't help but to think I'm trying to force a square peg into a round hole with all the casting going on. Is there some better way to accomplish this functionality? The idea was that the PickOption method can handle any enumerable and odds I throw at it.

I understand this question is rather open ended, but it doesn't hurt to ask, I suppose.

You're right that all the casting is a sign that there's better way.  Rather than using the int value of an ActionPlan as the index to an array, you should use a structure that's designed to store two values in the first place.  There's a couple of alternatives here.

My first instinct is to use a Dictionary<ActionPlan, double>.  That most explicitly matches the concept of what you're trying to do (each A has a value B), but it's not actually the best option.  You're not going to be looking up the value based on the key, the order of the keys may change which could lead to hard-to-reproduce bugs, and it's less memory-efficient.

Instead, I'd use a List<KeyValuePair<ActionPlan, double>>, List<Tuple<ActionPlan, double>>, or (in C# 7) List<(ActionPlan, double)>.  They're all effectively the same thing in this case, and best convey the concept "here is a pair of related values" without implying the A->B relationship of the Dictionary.  (Note that you can also use arrays instead of List<>, like I do below).  To make PickOption() handle this, you would need to make it generic, so that you can pass any arbitrary type in and get that same type out.

Here's what the code would look like using C# 7's ValueTuples:

private ActionPlan PickPlan()
{
    (ActionPlan, double)[] odds = new[] {
        (ActionPlan.DoNothing, 2.5),
        (ActionPlan.Eat, 3),
        (ActionPlan.StealFoodCovertly, 1),
        (ActionPlan.AttackAndSteal, 0.5),
    };

    ActionPlan plan = PickOption(odds);
    return plan;
}

private T PickOption<T>((T, double)[] odds)
{
    double totalOdds = 0;
    foreach (var odd in odds)
    {
        totalOdds += odd.Item2;
    }
    double pick = rand.NextDouble() * totalOdds;

    for (int i = 0; i < odds.Length; ++i)
    {
        pick -= odds[i].Item2;
        if (pick <= 0)
        {
            return odds[i].Item1;
        }
    }
    return odds.Last().Item1; // In case there are some precision shenanigans
}

or better:

private T PickOption<T>((T Action, double Weight)[] odds)
{
    double totalOdds = 0;
    foreach (var odd in odds)
    {
        totalOdds += odd.Weight;
    }
    double pick = rand.NextDouble() * totalOdds;

    foreach (var odd in odds)
    {
        pick -= odd.Weight;
        if (pick <= 0) return odd.Action;
    }
    return odds.Last().Action; // In case there are some precision shenanigans
}

There are other improvements that can be made here, too, depending on how much you simplified for this question, and how far you want to go.  For a few examples:

Both methods can be made static, because they don't depend on anything specific to whatever class they're in.
Rather than redefining the odds each time that PickPlan() is called, you can create a static readonly (ActionPlan, double)[] odds and populate it once.  This also lets you do that right next to where your ActionPlan enum is defined, so that when you add a new enum value, the odds are right there.
If you have other properties associated with an ActionPlan, you may want to consider a class to consolidate them all, rather than let each place they're used keep track.


Let us schedule a meeting Description:

Write a method mergeMeetings() that takes a list of multiple meeting time ranges and returns a list of condensed ranges. The integers represent the number of 30-minute blocks past 9:00am.
Do not assume the meetings are in order. The meeting times are coming from multiple teams.

Code:
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.ArrayList;

class Meeting {
  int startTime;
  int endTime;

  Meeting(int startTime, int endTime) {
    this.startTime = startTime;
    this.endTime = endTime;
  }

  public String toString() {
    return startTime + " : " + endTime;
  }
}

class Main {
  static List<Meeting> mergeMeetings(List<Meeting> meetings) {
    for (int outer = 0; outer < meetings.size(); outer++) {
      Meeting outerMeeting = meetings.get(outer);

      for (int inner = 0; inner < meetings.size(); inner++) {
        Meeting innerMeeting = meetings.get(inner);

        if (overlaps(outerMeeting, innerMeeting)) {
          meetings.set(outer, merge(outerMeeting, innerMeeting));
          meetings.remove(inner);
          break;
        }
      }
    }
    return meetings;
  }

  static List<Meeting> mergeMeetings2(List<Meeting> meetings) {
    List<Meeting> merged = new ArrayList<>();

    Collections.sort(meetings, (a, b) -> a.startTime - b.startTime);

    for (Meeting current : meetings) {
      if (merged.isEmpty()) {
        merged.add(current);
        continue;
      }
      Meeting lastMerged = merged.get(merged.size() - 1);
      if (overlaps(lastMerged, current)) {
        lastMerged.endTime = current.endTime;
      } else {
        merged.add(current);
      }
    }
    return merged;
  }

  private static Meeting merge(Meeting first, Meeting second) {
    return new Meeting(first.startTime, second.endTime);
  }

  private static boolean overlaps(Meeting first, Meeting second) {
    return first.startTime < second.startTime &&
      (first.endTime == second.startTime || second.startTime < first.endTime);
  }

  static void show(List<Meeting> meetings) {
    for (Meeting meeting : meetings) {
      System.out.print(meeting + ", ");
    }
    System.out.println();
  }

  public static void main(String[] args) {
    show(mergeMeetings(new ArrayList(Arrays.asList(
      new Meeting(0, 1),
      new Meeting(3, 5),
      new Meeting(4, 8),
      new Meeting(10, 12),
      new Meeting(9, 10)
    )))); // 0 1, 3 8, 9 12

    show(mergeMeetings2(new ArrayList(Arrays.asList(
      new Meeting(0, 1),
      new Meeting(3, 5),
      new Meeting(4, 8),
      new Meeting(10, 12),
      new Meeting(9, 10)
    )))); // 0 1, 3 8, 9 12

    show(mergeMeetings(new ArrayList(Arrays.asList(
      new Meeting(1, 4),
      new Meeting(4, 5)
    )))); // 1 5

    show(mergeMeetings2(new ArrayList(Arrays.asList(
      new Meeting(1, 4),
      new Meeting(4, 5)
    )))); // 1 5

    show(mergeMeetings(new ArrayList(Arrays.asList(
      new Meeting(1, 3),
      new Meeting(2, 6),
      new Meeting(8, 10),
      new Meeting(15, 18)
    )))); // 1 6, 8 10, 15 18

    show(mergeMeetings2(new ArrayList(Arrays.asList(
      new Meeting(1, 3),
      new Meeting(2, 6),
      new Meeting(8, 10),
      new Meeting(15, 18)
    )))); // 1 6, 8 10, 15 18
  }
}

Note:
The brute force solution is intentional and I am practicing it to at least
come up with the brute force answer if not able to find the optimal solution. I am learning to quickly find little abstractions to help me think about the solution more easily for eg: the two helper methods overlaps and merge helped me to find the solution easily.
Questions:
Apart from general code review I would like to know more tips / recommendations on coming up with brute force solutions quickly.

Not sure if I should review it as a programming challenge, or if I should include design remarks, I'll just say what comes to mind.

I'll first note that I would make the Meeting fields private final, and add accessor methods. Them being immutable makes sense I would say, and methods help for using them as method references with the streaming API and others, as I will show in a second.

For mergeMeetings, I would really refrain from using set and remove, at least when you do not know the implementation. Depending on the List implementation, the performance of these could turn out to be quite horrible (e.g. set could traverse the list up to the index each time in case of a LinkedList). Also looping and mutating a collection at the same time is a lot of times error-prone or worse. Finally, you mutate the collection passed in, which is also a point to think about twice (though not necessarily bad).

For mergeMeetings2, I like this implementation better. It is a smarter implementation for given problem. First, I would like to note that since Java 8, Collection has a default method implementation for sort. Next, with the Comparator#comparing method, you can specify which property of the type to use to compare with. If you add accessor methods like stated before, this would make the sort look like (with a static import of Comparator):

meetings.sort(comparingInt(Meeting::startTime));

Perhaps instead of using a List to keep track of the merged meetings, you could use a stack (though do not use Stack, rather something like ArrayDeque, refer to the docs why). Instead of getting the last element from the list, you just peek the top, and pop if you have to merge again. I am not sure how I feel about

if (merged.isEmpty()) {
    merged.add(current);
    continue;
}

I think it makes the loop implementation a bit confusing, because this is only called once to add the first element to the merged elements. I wonder if using List#subList and iterating over that would be better, or a manual iterator... Also, should the Meetings become immutable, it would change the implementation a bit. Note that you are now changeing objects which are coming from the outside.

Your merge method assumes that the start time of first is always the smallest start time, and that the end time of second is always the greatest start time. This would be wrong for e.g. first = [8, 12], second = [9, 10]. It will return [8, 10], when it should be [8, 12].

The overlaps method also is a bit incorrect, because multiple meetings starting at the same point in time will not be merged. Also (first.endTime == second.startTime || second.startTime < first.endTime) is the same as second.startTime <= first.endTime.

Finally, your show method ends each line with a comma. Should you want to simply delimit each meeting with a comma, but not one at the end of the line, you could solve it for example as:

System.out.println(meetings.stream().map(Meeting::toString).collect(joining(",")));


Improving efficiency of Project Euler 185 (16-place Mastermind) This a solution for Project Euler Problem 185.  It works fine for the example but slow for the problem statement. How can I improve its speed and efficiency?

import itertools
from collections import defaultdict
def mrange(start, stop, step=1):
    i = start
    while i < stop:
        yield i
        i += step
#produces the list of numbers
number= mrange(10**4,10**5)
newlist=[]
def combination(x,n):
    if n==0:
        temp=[]
        for i in range(1,len(x)+1):
            temp.extend(list(itertools.combinations(x,i)))
        return temp
    return list(itertools.combinations(x,n))

def combineDictTuple(dt):
    d = {}
    for item in dt:
        d.update(item)
    return d
def prob(a,n):
    temp=[]
    if n==0:
        temp=number
    #modchar is indexing the numbers eg:[{0: '9'}, {1: '0'}, {2: '3'}, {3: '4'}, {4: '2'}]
    modchar=[{i: c} for i, c in enumerate(a)]
    combos = combination(modchar,n)
    newCombos = [combineDictTuple(dt) for dt in combos]
    # newCombos gives combination of the indexes
    #[{0: '9', 1: '0'}, {0: '9', 2: '3'}, {0: '9', 3: '4'}, {0: '9', 4: '2'}, {1: '0', 2: '3'}, {1: '0', 3: '4'}, {1: '0', 4: '2'}, {2: '3', 3: '4'}, {2: '3', 4: '2'}, {3: '4', 4: '2'}]
    for j in number:
        for i in newCombos:
            index=i.keys()
            val=i.values()
            # print index,val
            if n==0:
                if all(str(j)[index[l]]==val[l] for l in range(0,len(val))):
                    if j in temp:
                        del temp[temp.index(j)]
                        break
            else:

                if all(str(j)[index[l]]==val[l] for l in range(0,len(val))):
                    temp.append(j)
                    break
    return temp

val='''90342 ;2 correct
70794 ;0 correct
39458 ;2 correct
34109 ;1 correct
51545 ;2 correct
12531 ;1 correct'''
val=val.replace('correct','').replace(';','').split('\n')
for i in val:
    i=i.split()
    number=prob(i[0],int(i[1]))

print number

How the function works:

Generate numbers using mrange()
In prob method  first I find the index of the each number in problem statement, e.g. 90342
That produces [{0: '9'}, {1: '0'}, {2: '3'}, {3: '4'}, {4: '2'}]
Using n (n is no. of correct values) I produce combinations of the above dict example as stated in comments
For every number I am checking the condition which satisfy the index:value append into temp and at last my new numbers become temp values. This iterates for all values until it arrives at the answer.

note: if n=0. i calculate all the combinations and delete the numbers from temp which satisfy that.

Expressiveness

What does prob(a,n) mean?  How about apply_constraint(guess, n_correct) instead?
Why do you reinvent xrange() (Python 2) or range() (Python 3)?
number is a global variable, which makes the code hard to follow.
number is an iterable of numbers, but your constraints are given as characters, which makes comparisons awkward.
temp is an uninformative name for a variable.  It's especially bad when you return temp — that implies that it's not very temporary at all.
Parsing the input is better done using a regular expression.

Strategy

Let \$p\$ be the number of positions (16 in the real problem, 5 in the test case).
Let \$g\$ be the number of guesses (22 in the real problem, 6 in the test case).

The outline of prob() is:

for j in number:
    for i in newCombos:
        …
        [… for l in range(len(newCombos.values()))]

number is \$O(10^p)\$ (the number of candidate solutions you initially generate).

newCombos has \$\left(\begin{array}{c}p\\c\end{array}\right) = \dfrac{p!}{c!\ (p - c)!}\$ combinations, each of which has \$c\$ values, where \$c\$ is the number of correct guesses within that constraint.

If we estimate that \$c = 3\$, then the complexity of your solution is \$O(10^p p^3g)\$ in time.  The maximum space needed is the number candidates remaining after processing the first constraint, or \$O(\left(\begin{array}{c}p\\c\end{array}\right))\$.

I'm not sure that I analyzed the complexity completely correctly, but in any case those bounds are crazy.  Your strategy of enumerating all the possibilities just doesn't scale.  It's as if you were trying to solve a sudoku puzzle by generating all possible solutions first.


Split string with adding separator to substring I wrote my Split() extension, it's main goal is to save delimiters and add them to splitted strings.

For example:
I have a string "-1-2+3+4-5-6", and separators '+', '-'
and I want to have following: "","-1","-2","+3","+4","-5","-6" or "-","1-","2+","3+","4-","5-","6" depends on input separator option.

My extension method consists of three parts:

Check input values;
Getting all indexes of separators, and separators in source string;
Split source string and adding separators to left or right substring.

Code:

public enum SeparatorOptions
{
    None = 0,
    AddSeparatorToLeftSubstring = 1,
    AddSeparatorToRightSubstring = 2
}
public static class Extensions
{
    public static string[] Split(this string source, char[] separators, SeparatorOptions separatorOptions)
    {
        if (separatorOptions < SeparatorOptions.None || separatorOptions > SeparatorOptions.AddSeparatorToRightSubstring)
        {
            throw new ArgumentException("Arg_SeparatorEnumIllegalVal");
        }
        if (separators == null || separators.Length == 0)
        {
            return source.Split((char[])null);
        }
        if (separatorOptions == SeparatorOptions.None)
        {
            return source.Split(separators);
        }
        // Getting all indexes of separators, and separators in source string
        int foundCount = 0;
        int[] separatorIndexes = new int[source.Length];
        char[] separatorValues = new char[source.Length];
        for (int i = 0; i < source.Length; i++)
            for (int j = 0; j < separators.Length; j++)
            {
                char separator = separators[j];
                if (source[i] == separator)
                {
                    separatorValues[foundCount] = source[i];
                    separatorIndexes[foundCount++] = i;
                    break;
                }
            }

        string[] splitString = source.Split(separators);
        // Adding separators to left or right substring
        if (separatorOptions == SeparatorOptions.AddSeparatorToLeftSubstring)
            return AddSeparatorToLeftSubstring(splitString, separatorValues, foundCount);
        if (separatorOptions == SeparatorOptions.AddSeparatorToRightSubstring)
            return AddSeparatorToRightSubstring(splitString, separatorValues, foundCount);
        return splitString;
    }
    private static string[] AddSeparatorToLeftSubstring(string[] splitString, char[] separatorValues, int foundCount)
    {
        for (int i = 0; i < foundCount; i++)
        {
            splitString[i] += separatorValues[i];
        }
        return splitString;
    }
    private static string[] AddSeparatorToRightSubstring(string[] splitString, char[] separatorValues, int foundCount)
    {
        for (int i = 1, j = 0; i <= foundCount; i++, j++)
        {
            splitString[i] = separatorValues[j] + splitString[i];
        }
        return splitString;
    }
}
class Program
{
    static void Main(string[] args)
    {
        string str = "-1-2+3+4-5-6";
        Console.WriteLine("Original split()");
        var res2 = str.Split(new char[] { '+', '-' }, SeparatorOptions.None);
        foreach (var el in res2) Console.WriteLine(el);
        Console.WriteLine("Add to right substring");
        var res = str.Split(new char[] { '+', '-' }, SeparatorOptions.AddSeparatorToRightSubstring);
        foreach (var elem in res) Console.WriteLine(elem);
        Console.WriteLine("Add to left substring");
        var res3 = str.Split(new char[] { '+', '-' }, SeparatorOptions.AddSeparatorToLeftSubstring);
        foreach (var el in res3) Console.WriteLine(el);
        Console.ReadKey();
    }
}

What I wrote seems to work, although I am not sure if it is the best way, or recommended way to do something like this, so I want to ask what is wrong with my code and how could it be done better.

Review

You use very clean and strong names for all your variables so it's very easy to understand your code.
You too often go without {}. Especially the double for loops become very confusing.
You don't use good exception messages. In Arg_SeparatorEnumIllegalVal the Arg prefix is not necessary because the type of the exctpion is already telling me it's about an argument. You also formatted it like it was a variable with PascalCase but it's a message so it should be formatted like a normal sentence.

Alternative solution
There is a much shorter way to get the same results. You could use two different regexes:
var str = "-1-2+3+4-5-6";

Regex.Matches(str, "([-+]?[0-9]?)").Cast<Match>().Select(m => m.Value).Dump();
Regex.Matches(str, "([0-9]?[-+]?)").Cast<Match>().Select(m => m.Value).Dump();

which will respectively give you:
-1
-2
+3
+4
-5
-6
""

and
-
1-
2+
3+
4-
5-
6
""

Whether you want to keep the empty string or not is up to you.


Input/character validation on an input-element I took some time to create a function, which can validate which keys you input on an element. So far it has two uses:

You call .validate() on the object, but make sure it has a data-match attribute with the regex in it
You call .validate("REGEX") on the object, but replace REGEX with your actual regular expression

Here is an example:

document.getElementById("input").validate("[0-9A-Fa-f]");

This will only allow characters 0-9, A-F, and a-f. Nothing else.

I want to know, if I can improve this code. So far I have it the way I want it to be, but I would surely love someone else's feedback on it as well. It is really light-weight and can easily be implemented.

Thanks a lot!

Object.prototype.validate = function(myRegex) {
    var regex = this.getAttribute("data-match") || myRegex;
    var allowed = [8, 37, 38, 39, 40, 46];
    this.addEventListener("keydown", function(e) {
        var c = (e.metaKey ? '⌘-' : '') + String.fromCharCode(e.keyCode);
        if (!e.shiftKey && !e.metaKey) c = c.toLowerCase();
        if ((e.ctrlKey && (e.keyCode == 65 || e.keyCode == 97)) || c == '⌘-A') return true;
        if (c.match(regex) || allowed.indexOf(e.which) >= 0) {
            //Do nothing
        } else {
            e.preventDefault();
        }
    });
}

document.getElementById("input").validate("[0-9A-Fa-f]");
<input type="text" id="input" />

Whatever you do DON'T MODIFY Object.prototype

Modifying Object.prototype is one of the worst things you can do in JavaScript

Quoting MDN:

  Warning: Changing the [[Prototype]] of an object is, by the nature of how modern JavaScript engines optimize property accesses, a very slow operation, in every    browser and JavaScript engine. The effects on performance of altering inheritance are subtle and far-flung, and are not limited to simply the time spent in obj.__proto__ = ... statement, but may extend to any code that has access to any object whose [[Prototype]] has been altered. If you care about performance you should avoid setting the [[Prototype]] of an object. Instead, create a new object with the desired [[Prototype]] using Object.create().

What should I do?

Preferably create a function:

function Validate(element, regex) {
    // code
}

but if you want to extend an element use Element.prototype, preferable with Object.defineProperty:

Object.defineProperty(Element.prototype, 'validate', {
    value: function(regex) {
        // code
    }
});

The bonus of using this is you can instead of getters and setters which are pretty cool too.

Avoid empty ifs

Here you have:

if (c.match(regex) || allowed.indexOf(e.which) >= 0) {
    //Do nothing
} else {
    e.preventDefault();
}

instead, use a NOT (!):

if (!(c.match(regex) || allowed.indexOf(e.which) >= 0))
    e.preventDefault();

the best way to write this specific condition would be:

if(!c.match(regex) || allowed.indexOf(e.which) < 0)

Use clear variable names

As much as I love code-golf (writing short code), this is not it. Make sure your variable names are clear.

I don't know what the c variable does so I can't suggest an alternative name, this also makes it more confusing to me on the workings of your code.

Use Regex literals not strings

Your example shows:

"[0-9A-Fa-f]"

That's a string, not a Regex, use Regex literals (to avoid escaping also):

/[0-9A-Fa-f]/