l33t Speak translator Please review the code quality of this l33t Speak translator. Improvement suggestions are welcome. Here it is - http://jsbin.com/IHoFuQE/1

(function() {
  "use strict";

  // http://en.wikipedia.org/wiki/Leet
  // http://www.catb.org/jargon/html/crackers.html

  var alphabets = { // common
    "a": "4",
    "b": "8",
    "e": "3",
    "f": "ph",
    "g": "6", // or 9
    "i": "1", // or |
    "o": "0",
    "s": "5",
    "t": "7" // or +
  },
    alphabets2 = { // less common
      "c": "(", // or k or |< or /<
      "d": "<|",
      "h": "|-|",
      "k": "|<", // or /<
      "l": "|", // or 1
      "m": "|\\/|",
      "n": "|\\|",
      "p": "|2",
      "u": "|_|",
      "v": "/", // or \/
      "w": "//", // or \/\/
      "x": "><",
      "y": "'/"
    };

  var words = {
    "am": "m",
    "are": "r",
    "at": "@",
    "thanks": "thx",
    "your": "ur",

    "cool": "kewl",
    "defeated": "pwned",
    "dude": "d00d",
    "fear": "ph33r", // or ph34r
    "fool": "f00",
    "freak": "phreak",
    "hacker": "h4x0r",
    "lamer": "l4m3r",
    "mad": "m4d",
    "newbie": "n00b",
    "own": "pwn",
    "phone": "fone",
    "porn": "pr0n", // or n0rp
    "rocks": "roxxorz",
    "skill": "sk1llz",
    "sucks": "sux0r",
    "the": "t3h",
    "uber": "ub3r", // or |_|83r
    "yay": "w00t",
    "yo": "j0",
    "you": "j00" // or U
  };

  var elite = document.getElementById("elite"),
    leet = document.getElementById("leet"),
    randomcase = document.getElementById("randomcase"),
    adv = document.getElementById("adv"),
    btn = document.getElementById("btn");

  function changeLetters(text) { // change letters
    text = text || elite.value.toLowerCase();
    for (var i = 0; i < text.length; i++) {
      var alphabet = adv.checked ? alphabets[text[i]] || alphabets2[text[i]] : alphabets[text[i]];
      if (alphabet) {
        text = text.replace(text[i], alphabet);
      }
    }
    return text;
  }

  function changeWords() { // change special words and return text
    return changeLetters().replace(
      /\w+/g,
      function(word) {
        return words[word] ? words[word] : word;
      }
    );
  }

  function randomizeCase() { // RANdOMiZE CAsE
    var text = changeWords();
    for (var i = 0; i < text.length; i++) {
      if (Math.random() > 0.5) {
        text = text.replace(text[i], text[i].toUpperCase());
      } // else keep lower case
    }
    return text;
  }

  (function() { // l33t the words object
    for (var word in words) {
      if (words.hasOwnProperty(word)) {
        words[changeLetters(word)] = words[word];
        delete words[word];
      }
    }
  }());

  function tol33t() {
    leet.value = randomcase.checked ? randomizeCase() : changeWords();
  }

  elite.addEventListener("input", tol33t);
  btn.addEventListener("click", tol33t);
}());

To start with, I'd like to give you credit for using "use strict" and limiting the scope of variables using functions.

Your tol33t() function is weird to me.  If the randomcase button is checked, you call randomizeCase() with no arguments.  What are you randomizing the case of?  It turns out that randomizeCase() calls changeWords(), so it turns out that the two code branches have something in common after all.  That wasn't apparent from looking at tol33t().

There should be clearer separation responsibility between the DOM-interaction and text-translation layers of code.  The text translation code should reside in pure functions (whose results depend solely on their parameters) that you could call from the JavaScript console or from a unit testing engine.  That separation of concerns could be accomplished with:

function onTranslateButtonClicked() {
    var l33tText = toL33t(elite.value, adv.checked);
    if (randomcase.checked) l33tText = randomizeCase(l33tText);
    leet.value = l33tText;
}

Your "l33t the words object" initialization routine lays a trap for anyone trying to  read the code.  When code in changeWords() refers to the words associative array, one would reasonably think that it's using the definition above (i.e., words = { "am": "m", … }) instead of the munged version (words = { "4m": "m" }).  Better to make that munging explicit when you define words (and do so by returning a copy rather than mutating the dictionary as you iterate):

var words = l33tKeys({
    "am": "m",
    …
});

function l33tKeys(dict) {
    var l33tDict = {};
    for (var word in dict) {
        l33tDict[changeWord(word)] = dict[word];
    }
    return l33tDict;
}

I would write changeLetters() as:

 // letterSubsts may be alphabet or alphabet2
function changeLetters(text, letterSubsts) {
    return text.toLowerCase().replace(/[a-z]/g, function(letter) {
        return letterSubsts[letter] || letter;
    });
}

… which is actually analogous to your implementation of changeWords().


Should I put a JavaScript function in a local or global scope? I have many pages with functions that appear only once, which I am sure is really typical.  I read that it's the best idea to use the local scope for functions when possible because the local scope is searched first, thereby making the function found sooner.

If I have no intention of reusing a function, is this the best way to create and use this function?

var doCancel = function doCancel() {
    // DO SOME CANCELLING TYPE STUFF
}

Or is this a better way?

function doCancel() {
    // DO SOME CANCELLING TYPE STUFF
}

I always wrap my code in an IIFE:

(function () {
    'use strict';

    //code here
}());

I also pass in all global variables I am going to use in this code block:

(function (window, document, $) {
    'use strict';

    //code here
}(window, document, jQuery)); //...

If I am going to expose any global variables I explicitly add them to the global scope at the end of this function (but I avoid this if it is at all possible):

(function (window, document, $) {
    'use strict';

    //code here

    window.XXX = somelocalreference;
}(window, document, jQuery)); //...

This way I never unexpectedly leak globals and I can know that my code is not going to give me problems with future changes that my coworkers or myself make.

See also: Responsive/adaptive website code


Find the peak element using BS A peak element is an element that is greater than its neighbors.

Given an input array where num[i] ≠ num[i+1], find a peak element and return its index.

The array may contain multiple peaks, in that case return the index to any one of the peaks is fine.

You may imagine that num[-1] = num[n] = -∞.

For example, in array [1, 2, 3, 1], 3 is a peak element and your function should return the index number 2.

class Solution:
# @param num, a list of integer
# @return an integer
def findPeakElement(self, num):
    if len(num)==0: return None
    if len(num)==1: return 0
    return self.findPeakHelper(num,0,len(num)-1)
def findPeakHelper(self,num,start,end):
    mid = (start+end)/2
    if mid>start and mid < end:
        if(num[mid-1]<num[mid] and num[mid]>num[mid+1]):
            return mid
        if(num[mid-1]>num[mid]):
            return self.findPeakHelper(num,start,mid)
        else:
            return self.findPeakHelper(num,mid,end)
    else:
        if num[mid]>num[mid+1]:
            return mid
        else:return mid+1

Your code looks functional. I quite like the idea of splitting the data in halves each time too. There are some things I would do differently, though.

First-up, the 1-liner if statement:

if len(num)==0: return None
if len(num)==1: return 0

also

   else:return mid+1

No, no no ... ;-0 That should be:

if len(num) == 0:
    return None
if len(num) == 1:
    return 0

and

    else:
        return mid + 1

Note that your pasted code is not indented correctly. You indented the class definition, but the class contents are at the same indentation level.

Additionally, I prefer whitespace between method definitions, an empty line.

Actually, I ran your code through a pep8 checker, it pointed out all the compressed-statements too (no whitespace around operators). Here's your code in a format that passes the pep8 check:

class Solution:

    # @param num, a list of integer
    # @return an integer
    def findPeakElement(self, num):
        if len(num) == 0:
            return None
        if len(num) == 1:
            return 0
        return self.findPeakHelper(num, 0, len(num) - 1)

    def findPeakHelper(self, num, start, end):
        mid = (start + end) / 2
        if mid > start and mid < end:
            if(num[mid - 1] < num[mid] and num[mid] > num[mid + 1]):
                return mid
            if(num[mid - 1] > num[mid]):
                return self.findPeakHelper(num, start, mid)
            else:
                return self.findPeakHelper(num, mid, end)
        else:
            if num[mid] > num[mid + 1]:
                return mid
            else:
                return mid + 1

Note:

spaces surrounding operators
statements on new line, not the same line as the if/else conditions

Now, about your algorithm...

Really, there's nothing wrong with it, but I believe you can simplify the logic a bit if you treat the mid-point only when needed. Consider the following helper function:

class Solution:

    # @param num, a list of integer
    # @return an integer
    def findPeakElement(self, num):
        if len(num) == 0:
            return None
        if len(num) == 1:
            return 0
        return self.findPeakHelper(num, 0, len(num) - 1)

    def findPeakHelper(self, num, start, end):
        span = end - start
        # span of 1 indicates 2 elements
        if span == 1:
            return start if num[start] > num[end] else end
        mid = start + span / 2
        if num[mid] < num[mid + 1]:
            return self.findPeakHelper(num, mid, end)
        return self.findPeakHelper(num, start, mid)

Note how you don't need the else-statements if the if-statement has a guaranteed return.


How to shorten this code or make it more efficient? How to shorten up this code? I know there's a way that this kind of code could be shortened, but I don't know how by myself yet. Who can help me?

            //CHOOSE LEVEL: FROZEN
case 1213:
    sendOption5("Level 1 " + (c.achievedFloor <= 1 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 2 " + (c.achievedFloor <= 2 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 3 " + (c.achievedFloor <= 3 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 4 " + (c.achievedFloor <= 4 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "More");
    c.dialogueAction = 1213;
    break;
case 1214:
    sendOption5("Level 5 " + (c.achievedFloor <= 5 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 6 " + (c.achievedFloor <= 6 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 7 " + (c.achievedFloor <= 7 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 8 " + (c.achievedFloor <= 8 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "More");
    c.dialogueAction = 1214;
    break;
case 1215:
    sendOption5("Level 9 " + (c.achievedFloor <= 9 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 10 " + (c.achievedFloor <= 10 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "Level 11 " + (c.achievedFloor <= 11 ? "@gre@(Achieved)" : "@red@(Not achieved)") + "",
            "",
            "Choose Floor");
    c.dialogueAction = 1215;
    break;

In a first step I would extract a method to generate the string.

changed code:

            //CHOOSE LEVEL: FROZEN
case 1213:
    sendOption5(
            levelString( 1, c),
            levelString( 2, c),
            levelString( 3, c),
            levelString( 4, c),
            "More");
    c.dialogueAction = 1213;
    break;
case 1214:
    sendOption5(
            levelString( 5, c),
            levelString( 6, c),
            levelString( 7, c),
            levelString( 8, c),
            "More");
    c.dialogueAction = 1214;
    break;
case 1215:
    sendOption5(
            levelString( 9, c),
            levelString(10, c),
            levelString(11, c),
            "",
            "Choose Floor");
    c.dialogueAction = 1215;
    break;

extracted method:

private static String levelString(int i, TypeOfC c){
   return "Level " + i + (c.achievedFloor <= i ? " @gre@(Achieved)" : " @red@(Not achieved)");
}

You should also think about changing the parameters of your sendOption5 to get only c and two integers from to which define the inteval of values.

And you can move the c.dialogueAction = xxxx; line behind the switch and assign the value you use for the switch.


Shifting Paths problem (Code Jam) I was trying, just for fun, to solve http://code.google.com/codejam/contest/dashboard?c=2075486#s=p4

I can easily solve the small set, but I struggle to solve the big one (my implementation would use approx. 30GB or RAM, which is a bit too much. Any hint or direction is appreciated!

def solve_maze(case_no):
    case = cases[case_no]
    case_no += 1
    edges = case['edges']
    #quick and dirty check. Can we reach the goal?
    if case['goal'] not in sum([i.values() for i in edges.values()],[]):
        return "Case #%s: Infinity" % case_no
    last_position = 1
    l = 0
    states = set()
    memory = [1 for i in range(1,1+case['goal'])]
    while not last_position==case['goal']:
        direction = memory[last_position]
        memory[last_position] *= -1
        move = edges[last_position][direction]
        states.add(hash((move,tuple(memory))))
        l+=1
        if len(states) != l:
            return "Case #%s: Infinity" % case_no
        last_position = move
    else:
        return "Case #%s: %d" % (case_no,len(states))

with open("/xxx/test-small.in","rb") as f:
    inp = f.read()[:-1].split("\n")
    N = int(inp[0])
    cases = []
    it = 0
    for line in inp[1:]:
        if line.find(" ") == -1:
            #print int(line)
            cases.append({"goal":int(line),"edges":{}})
            it = 0
        else:
            it+=1
            cases[-1]['edges'][it] = dict(zip([1,-1],map(int,line.split())))
    for i in range(len(cases)):
        print solve_maze(i)

def solve_maze(case_no):
    case = cases[case_no]

It'd make more sense to pass the case here, and operate on that rather then passing in the case number

    case_no += 1
    edges = case['edges']
    #quick and dirty check. Can we reach the goal?
    if case['goal'] not in sum([i.values() for i in edges.values()],[]):
        return "Case #%s: Infinity" % case_no

I avoid such quick and dirty checks. The problem is that they are easily defeated by a crafted problem. Not enough cases are really eliminated here to make it work.

    last_position = 1
    l = 0
    states = set()
    memory = [1 for i in range(1,1+case['goal'])]

It's a little odd to add one to both parameters to range and then ignore the count

    while not last_position==case['goal']:

Use !=

        direction = memory[last_position]
        memory[last_position] *= -1
        move = edges[last_position][direction]
        states.add(hash((move,tuple(memory))))

You aren't guaranteed to have distinct hashes for unequal objects so this must fail. You should really add the whole tuple to the state. Actually, you don't even need to track the states. There are are only so many possible states, and you can simply count the number of transitions, if this gets above the number of possible states, you're in an infinite loop.

        l+=1
        if len(states) != l:
            return "Case #%s: Infinity" % case_no
        last_position = move
    else:
        return "Case #%s: %d" % (case_no,len(states))

with open("/xxx/test-small.in","rb") as f:
    inp = f.read()[:-1].split("\n")

This really isn't a good way to read your input. Use f.readline() to fetch a line at  a time. It'll be much easier to parse the input that way.

    N = int(inp[0])
    cases = []
    it = 0
    for line in inp[1:]:

Wrong choice of loop. You really want to repeat the process of reading a single case N times, not do something on each line of the file.

        if line.find(" ") == -1:
            #print int(line)
            cases.append({"goal":int(line),"edges":{}})

Don't treat dictionaries like objects. It'd make more sense to pass goal and edges as parameters rather them stick in a dictionary. At least, you should prefer an object.

            it = 0
        else:
            it+=1
            cases[-1]['edges'][it] = dict(zip([1,-1],map(int,line.split())))
    for i in range(len(cases)):
        print solve_maze(i)

As for your algorithm. You can't get away with simulating all of the clearings. One can design a test case that takes over 2**39 steps to complete. So you can't simulate the straightforward process of moving one clearing at a time. But any given forest will have patterns in how the states change. You need to find a way to characterize the patterns so that you can simulate the process more quickly.


Rewritten implementation of quadratic equation formula I just started learning the C programming language at the university, and today we got a new assignment there is about Refinement of a C software there is designed to find the roots of the general quadratic equation. We got an old C software that we have to rewrite with functions instead of using the Whole code inside main. We have to split the functions into 3 where is about

Calculation of the discriminant.
Calculation of the first root.
Calculation of other root.

The old code:

#include <stdio.h>
#include <math.h>

/* Prints roots of the quadratic equation a * x*x + b * x + c = 0 */
void solveQuadraticEquation(double a, double b, double c){
  double discriminant, root1, root2;

  discriminant = b * b - 4 * a * c;

  if (discriminant < 0)
    printf("No roots\n");
  else if (discriminant == 0){
    root1 = -b/(2*a);
    printf("One root: %f\n", root1);
  }
  else {
    root1 = (-b + sqrt(discriminant))/(2*a);
    root2 = (-b - sqrt(discriminant))/(2*a);
    printf("Two roots: %f and %f\n", root1, root2);
  }

}

int main(void) {
  double a = 1.0, b = -8.0, c = 15.0,
         d = 2.0, e =  8.0, f =  2.0,
         g, h, i;

  /* First call - coefficents are values of variables */
  solveQuadraticEquation(a, b, c);

  /* Second call - coefficents are values of expressions */
  solveQuadraticEquation(d - 1, -e, 7 * f + 1);

  /* Third call - coefficents are entered by user outside
solveQuadraticEquation */
  printf("Enter coeficients a, b, and c: ");
  scanf("%lf %lf %lf", &g, &h, &i);
  solveQuadraticEquation(g, h, i);

  return 0;
}

My Solution:

#include <stdio.h>
#include <math.h>

double discriminant(double a, double b, double c);
double root1(double a, double b, double c);
double root2(double a, double b, double c);

/* Prints roots of the quadratic equation a * x*x + b * x + c = 0 */
void solveQuadraticEquation(double a, double b, double c){

  if (discriminant(a, b, c) < 0)
    printf("No roots\n");
  else if (discriminant(a, b, c) == 0){
    printf("One root: %f\n", root1(a, b, c));
  }
  else {
    printf("Two roots: %f and %f\n", root1(a, b, c), root2(a, b, c));
  }

}

int main(void) {
  double a = 1.0, b = -8.0, c = 15.0,
         d = 2.0, e =  8.0, f =  2.0,
         g, h, i;

  /* First call - coefficents are values of variables */
  solveQuadraticEquation(a, b, c);

  /* Second call - coefficents are values of expressions */
  solveQuadraticEquation(d - 1, -e, 7 * f + 1);

  /* Third call - coefficents are entered by user outside
solveQuadraticEquation */
  printf("Enter coeficients a, b, and c: ");
  scanf("%lf %lf %lf", &g, &h, &i);
  solveQuadraticEquation(g, h, i);

  return 0;
}

double discriminant(double a, double b, double c){
    return b * b - 4 * a * c;
}

double root1(double a, double b, double c){
  return (-b + sqrt(discriminant(a, b, c)))/(2*a);
}

double root2(double a, double b, double c){
  return (-b - sqrt(discriminant(a, b, c)))/(2*a);
}

I have already made a well-working solution, but I was wondering if I could make it even better, so if I have done something wrong or weird, please let me know.

Aside from re-computing discriminant(), a reasonable quadratic implementation.

When assessing the quality of floating-point computations, use "%e" rather than "%f".  I recommend using "%e" with enough precision too.  Like "%.17e", or use one of the DECIMAL_DIG family of constants.

Precision improvement when b*b near discriminant:

When |b| is about sqrt(discriminant), the computation of root1 or root2 will cancel many digits.  To avoid this, consider that a*root1*root2 = c

// calculate root1 and root2 together.
if (b < 0) {
  root1 = (-b + sqrt(discriminant))/(2*a);
  root2 = c/(root1*a);
} else {
  root2 = (-b - sqrt(discriminant))/(2*a);
  root1 = c/(root2*a);
}

Try a=1.0, b = -1e24-1, c = 1e24.  Roots should be 1e24, 1 as here.  OP's approach  results in 1e24, 0, total loss of precision.  Keep in mind, FP has overall logarithmic precision, not linear.

When b*b and discriminant greatly differ, this approach is not significantly weaker than OP's original.

Precision improvements are possible with b * b - 4 * a * c, yet tend to impact performance more.

Advanced concern:

root1(), root2() is on thin ice at it relies on a non-negative result from discriminant().

C is sneaky in that FP code is allowed to use higher than needed precision selectively.  See FLT_EVAL_METHOD.

With OP's code, this certainly will not happen as is, yet with a subtle change of in-lining b * b - 4 * a * c; instead of an explicit function call, may yield the following:

A test of b * b - 4 * a * c < 0 may be false yet the later (-b + sqrt(b * b - 4 * a * c)) may attempt a square root of a negative value due to different optimizations.

Solution: Use the same result rather than re-compute discriminant.  Do not rely on "same looking code" to generate the exact same FP result especially when near 0.0.

This also eliminates the wasteful re-computation of discriminant().

Check spelling on "coefficents": I'd expect "coefficients".

To meet OP's goal to split the functions into 3: discriminant, first root, other root consider:

double discriminant(double a, double b, double c);
double first_root(double discriminant, double a, double b);
double other_root(double root1, double a, double c);

...
doubled = discriminant(a, b, c);
if (d < 0) ...
else if (d == 0) ....
else {
  double root1 = first_root(discriminant, a, b);
  double root2 = other_root(root1, a, c);
  ...
}


Password strength checker (Udemy course) This program is a project out of Udemy course. Kindly request you to review my code and suggest insights to better it. It takes your password hashes (sha1) and then sends hashedpassword[:5] to https://haveibeenpwned.com/ website. Gets the response and checks whether our hashed password is present in the response got from the site
Thanks in advance!!
import requests
from ezhashlib import hashlib
import sys

def req_api_data(query_char):

   '''Returns response for pwned website after passing the tail'''
    url = 'https://api.pwnedpasswords.com/range/' + query_char
    res = requests.get(url)
    if res.status_code!=200:
        raise RuntimeError(f'Error fetching: {res.status_code}, check the api and try again')
    else:
        return res

def get_pw_lead_count(hashes,hashes_to_check):

    hashes = (line.split(':') for line in hashes.text.splitlines())# Splits the responsex
    for h,count in hashes:
        if hashes_to_check == h:
                  return count
    return 0

def pwned_api_check(password):

    sha1password = hashlib.sha1(password.encode('utf-8 ')).hexdigest().upper()
    # Generatng the sha1 for your password
    head,tail = sha1password[:5],sha1password[5:]
    res = req_api_data(head)
    #sending the head to website
    return get_pw_lead_count(res,tail)
    #Sending website response and tail for checking whether tail exists in the list of responses got from website.

def main():

    args=list(input().split())
    for passwords in args:
        count = pwned_api_check(passwords)
        if count:
             print(f"Change your \"{passwords}\" password as it's pwned {count} times")
        else:
            print("You've used a good password!!")

main()

Use requests built-in validation
This:
if res.status_code!=200:
    raise RuntimeError(f'Error fetching: {res.status_code}, check the api and try again')
else:
    return res

should not raise manually; and should be replaced with
res.raise_for_status()
return res

Hash checking
In get_pw_lead_count, hashes_to_check should be marked : str. Also, if count is a string - likely, since it comes from a split() - you should cast it to int() before returning it.
Real arguments
args=list(input().split())

is not a standard way of retrieving arguments. Instead, use argparse and pass arguments via command line parameters.
It's also possible to retain stdin input as you have it, but you should in that case show a prompt.
Indentation
Count the number of spaces for each of these lines:
    if count:
         print(f"Change your \"{passwords}\" password as it's pwned {count} times")

Each should be a multiple of four.


Simple python string to list parser So I came across this question on SO and I felt like it would be a cool thing to try and write a parser for since I always wanted to try it. So I present to you:
My first time writing a parser.
It converts strings like this:
"a,s,[c,f],[f,t], [[a,c],[d3,32]]"

into list objects
['a', 's', ['c', 'f'], ['f', 't'], [['a', 'c'], ['d3', '32']]]

Here is my code for now
def parseToList(string, cont=0):
  result = list()
  temp = ''
  i = cont
  while i < len(string):
    if string[i] == ',':
      if len(temp) and temp != ' ':
        result.append(temp)
      temp = ''
    elif string[i] == '[':
      res = parseToList(string, i+1)
      i = res[1]
      result.append(res[0])
    elif string[i] == ']':
      if len(temp) and temp != ' ':
        result.append(temp)
      return (result,i)
    else:
      temp += string[i]
    i += 1
  if len(temp) and temp != ' ':
    result.append(temp)
  return (result, i)

def listParse(string):
  return parseToList(string)[0]

s = 'a,s,[c,f],[f,t], [[a,c],[d3,32]]'

print(s)
print(listParse(s))

Is there anything I am doing wrong? Something I should change?

Here are a few things that came to my mind:

Bug

if temp != ' ' will not work when there are more than 1 consecutive spaces.
To fix this, use if not temp.isspace() instead of comparing with a hard-coded string.
For example, s = 'a,   [b]' will output ['a', ['b'], '   '] for your current code.

Your code outputs ['a', ' b'] for a, b. I'll assume that including the space is a feature and not a bug.

Design

Wrap the test code inside  if __name__ == '__main__'. This will prevent the code from being called when being imported from another module.

Function names should preferably be lowercase. Change the CamelCase names to snake_case.

In return statements, you need not enclose the items in a parenthesis if you are returning a tuple

result = list() can be replaced with just result = []

if len(temp) can be replaced with just if temp. The bool of empty values are False in python.

res = parse_to_list(string, i + 1)
i = res[1]
result.append(res[0])

The above can be a bit simplified and can be made a bit more understandable.
nested_list, i = parse_to_list(string, i + 1)
result.append(nested_list)

Instead of using string[i], you can declare a new element char which is equal to string[i]
(This is just my personal preference)

You can declare parse_to_list to inside list_parse. This will remove the need to pass string inside a recursion repeatedly, and will also make the inner function "private".
(But this is also just my personal preference)

The final code should look something like this after applying the above:
def list_parse(string):
    def parse_to_list(cont=0):
        result = []
        temp = ''
        i = cont

        while i < len(string):
            char = string[i]

            if char == ',':
                if temp and not temp.isspace():
                    result.append(temp)
                temp = ''

            elif char == '[':
                nested_list, i = parse_to_list(i + 1)
                result.append(nested_list)

            elif char == ']':
                if temp and not temp.isspace():
                    result.append(temp)
                return result, i

            else:
                temp += char

            i += 1

        if temp and not temp.isspace():
            result.append(temp)

        return result, i

    return parse_to_list()[0]

if __name__ == '__main__':
    s = 'a,s,[c,f],[f,t], [[a,c],[d3,32]]'

    print(list_parse(s))


Eight Queens Recursive I am working on a Java textbook chapter on recursion, and I am to implement an Eight Queens solution using recursion.  Though I know that I am to post working code in Code Review, I do believe this code would work if I didn't run out of memory.  I have walked through the debugger, and the queens seem to adjust position exactly like they should until there is a Stack Overflow.

My question really is this:  After running into this Stack Overflow issue, my instincts are to either find a way to increase Stack memory (I have not found a way to do this) or reduce Stack memory usage.  To reduce Stack memory usage, I do wonder if there might be a way to put some of the weight of the recursion on the heap instead.  The most likely way to reduce Stack memory usage would likely be some clever refactoring, which is why I am posting in Code Review.  What I feel the best answer for this is, is just to ditch as much recursion as I can, but if I am to follow the exercise instructions, I don't have that option.

This is what I have:

Queen.java

class Queen {
    private int row;
    private int col;
    private int qNum;

    Queen(){
        row = 0;
        col = 0;
    }

    int getRow() {
        return row;
    }

    int getCol() {
        return col;
    }

    void setQNum(int qNum){
        this.qNum = qNum;
    }

    // requires canAdvance check first
    void advance(){
        if (col == 7){
            ++row;
            col = 0;
        }
        else{
            ++col;
        }
    }
    void matchPosition(Queen previousQueen){
        if (previousQueen.getCol() == 7){
            row = previousQueen.getRow() + 1;
            col = 0;
        }
        else{
            row = previousQueen.getRow();
            col = previousQueen.getCol() + 1;
        }
    }

    boolean canAdvance(){
        return !(row == 7 && col == 7);
    }

    private boolean isAttacked(Queen attacker){
        return getRow() == attacker.getRow() ||
                getCol() == attacker.getCol() ||
                Math.abs(getRow() - attacker.getRow()) ==
                        Math.abs(getCol() - attacker.getCol());
    }

    boolean isSafe(Queen[] queens){
        for(int attIndex = 0; attIndex < qNum; ++attIndex){
            if(this.isAttacked(queens[attIndex])){
                return false;
            }
        }
        return true;
    }
}

Main.java

class Main{
    public static void main(String[] args){
        Queen[] queenArr = {
                new Queen(),
                new Queen(),
                new Queen(),
                new Queen(),
                new Queen(),
                new Queen(),
                new Queen(),
                new Queen()
        };

        // Storing array index numbers allow the code to be more readable.
        for(int i = 0; i < 8; ++i){
            queenArr[i].setQNum(i);
        }

        placeQueen(queenArr, 1);
        for(Queen queen : queenArr){
            System.out.printf("row = %d  col = %d%n", queen.getRow(), queen.getCol());
        }
    }

    private static void placeQueen(Queen[] queens, int index){
        if (queens[index].canAdvance()){
            queens[index].advance();
        }
        else{
            while(index > 0 && !queens[index].canAdvance()){
                --index;
            }
            placeQueen(queens, index);
        }

        if (queens[index].isSafe(queens)){
            if (index == 7){
                return;
            }
            else{
                // next queen is set to position of current queen
                queens[index + 1].matchPosition(queens[index]);
                ++index;
            }
        }
        placeQueen(queens, index);
    }
}

I think the place where recursion is least appropriate is when you advance a queen until it is safe from all queens with a lower index (i.e. the last line of the method placeQueen(Queen[], int)). This doesn't really seem like a recursive process to me, but rather like something that calls for a loop.

private static void placeQueen(Queen[] queens, int index) {
    while (!queens[index].isSafe(queens) && queens[index].canAdvance()) {
        queens[index].advance();
    }

    if (!queens[index].canAdvance()
            && (!queens[index].isSafe(queens)
            || index != 7)) {
        while (index > 0 && !queens[index].canAdvance()) {
            --index;
        }
        queens[index].advance();
        placeQueen(queens, index);
    } else if (index != 7) {

        // next queen is set to position of current queen
        queens[index + 1].matchPosition(queens[index]);
        ++index;
        placeQueen(queens, index);
    }
}

This advances a queen until it is either safe or can no longer be advanced, which is basically what your code did, but here, this is achieved without recursion, saving a lot of stack memory.


Convert FLAC files to MP3 in parallel on Ubuntu I created a script to convert my FLAC music collection to MP3.  I did this so that I can play the MP3 files in my car, as the integrated head unit only supports MP3 and WMA files.  The script works on my Ubuntu system.  Are there any opportunities to improve its efficiency?

I chose to use ffmpeg to transcode and GNU parallel to utilize all CPU cores/threads.

Iteration 1

 #!/bin/bash

 ## Converts flac to mp3

 if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
 apt-get update  && apt-get install ffmpeg; fi

 if ! dpkg-query -W -f='${Status}' parallel | grep "ok installed"; then
 apt-get update  && apt-get install parallel; fi

 parallel -i -j$(nproc) \
         ffmpeg -n -i {} -acodec libmp3lame -ab 320k -map_metadata 0 -id3v2_version 3 {.}.mp3 ::: */*/*/*.flac

Iteration 2

 #!/bin/sh

 ## Converts flac to mp3

 die() { echo "$@" >&2; exit 1; }

 ### Check the preconditions

 if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
     apt-get update  && apt-get install ffmpeg libmp3lame parallel;
 fi

 ####

 flac_to_mp3() {
     ffmpeg -n -i "$1" -acodec libmp3lame -ab 320k \
        -map_metadata 0 -id3v2_version 3 "$2"
 }
 export -f flac_to_mp3

 parallel flac_to_mp3 {} '{.}.mp3' ::: */*/*/*.flac

shellcheck

It is always a good plan to run your code through shellcheck.net.  This only reveals a couple of issues:

Your -j$(nproc) should be quoted to avoid word splitting.  Hopefully no one will put spaces in nproc, but just in case -j"$(nproc)" would handle it better.  As noted in a comment above you may be able to drop this totally
Your {.}.mp3 bothers shellcheck which is also correctable with quoting like '{.}.mp3'.

ifs

I really like that you're using the direct exit value checking with your ifs.  This is a great shell feature that is not terribly popular.

I wish you would indent them a bit better though:

if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
    apt-get update  && apt-get install ffmpeg;
fi

Lining up the if and the fi makes me feel better.  Having the conditional things indented also helps following along much easier.

shorter line length is good

You're already using \ to split up the long parallel line, so why not split the ffmpeg part of it in two so it all fits in 80 columns?

efficiency

Using parallel to manage the process seems like a good plan.  This operation should be CPU bound so it will do a good job for getting this done.  You might want to have some CPU and I/O monitoring going to make sure this is working as expected.

Your script does not seem to consider that some mp3s might already be there.  If your computer crashed in the middle of conversion you would be stuck redoing work already completed.

apt minimization

While it is a tiny part of what you're doing the apt portion of things could be more efficient.  You could cache whether these commands are installed by touching a file like ~/.ready_to_convert and hoping they don't get uninstalled by accident.  You could also choose to skip the dpkg-query and just see if which ffmpeg gives you any output.  This would let it work even if the commands are installed manually in /usr/local/bin without a package.

It would also be nice to only run apt-get update once.  You could set a flag like so:

if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
    apt-get update  && apt-get install ffmpeg
    APT_UPDATED=1
fi

if ! dpkg-query -W -f='${Status}' parallel | grep "ok installed"; then
    if [[ $APT_UPDATED < 1 ]]; then
        apt-get update
    fi
    apt-get install parallel
fi

But presumably the conversion is consuming most of the time so this will only let that get started a second or two quicker.


Function to download anonymous type data into excel As far as I know, I cannot create function to accept anonymous type as parameter and I am reluctant to create new class to store those anonymous type because their difference is calculatedPrice.

How else can I remove\minimize repeated code in code below?

//.NET Framework 4
protected void btnDownload_Click(object sender, EventArgs e)
{
    var vwTransactionInfoItemProcesses = DAL.TransactionItemProcessHandler.GetVwTransactionInfoItemProcess(dropDownListTransactionStatus.SelectedValue.Trim(), BLL.SessionFacade.currentUserDeptCode.Trim(), true, txtSearchVendorCode.Text.Trim(), txtSearchCreateBy.Text.Trim(), txtSearchTransactionNumber.Text.Trim(), DateTime.Parse(txtCreateFrom.Text.Trim()), DateTime.Parse(txtCreateTo.Text.Trim()).Add(new TimeSpan(23, 59, 59)), txtSearchProductionOrder.Text.Trim(), txtSearchPartNumber.Text.Trim());

    if (BLL.UserRole.CheckUserRoleExist(BLL.SessionFacade.currentUserId, "purchaser"))
    {
        var vwTransactionInfoItemProcessesInDownloadFormat = vwTransactionInfoItemProcesses.Select(x => new
        {
            transactionNumber = x.transactionNumber,
            processName = x.processName,
            processOrder = x.processOrder,
            processQuantity = x.processQuantity,
            processUOM = x.processUOM,
            calculatedPrice = x.calculatedPrice
        }).ToList();
        DataTable transactionInfoItemProcessesInDownloadFormat = BLL.General.ConvertToDatatable(vwTransactionInfoItemProcessesInDownloadFormat);
        HSSFWorkbook workbook = BLL.General.GenerateNPOIExcelXLS(transactionInfoItemProcessesInDownloadFormat);
        ExportToExcel(workbook);
    }
    else
    {
        var vwTransactionInfoItemProcessesInDownloadFormat = vwTransactionInfoItemProcesses.Select(x => new
        {
            transactionNumber = x.transactionNumber,
            processName = x.processName,
            processOrder = x.processOrder,
            processQuantity = x.processQuantity,
            processUOM = x.processUOM
        }).ToList();
        DataTable transactionInfoItemProcessesInDownloadFormat = BLL.General.ConvertToDatatable(vwTransactionInfoItemProcessesInDownloadFormat);
        HSSFWorkbook workbook = BLL.General.GenerateNPOIExcelXLS(transactionInfoItemProcessesInDownloadFormat);
        ExportToExcel(workbook);
    }
}

Actually you can have anonymous types as parameters - called dynamics - but IMO it's not pretty (and probably rather slow):

static void ConvertToDatatable(IEnumerable<dynamic> input)
{

  foreach (var i in input)
  {
    Type type = i.GetType();
    if (type.GetProperty("Age") != null)
      Console.WriteLine($"{i.Name}: {i.Age}");
    else
      Console.WriteLine($"{i.Name}: No age");
  }
}

static void DynamicTest()
{
  bool isTrue = true;

  IEnumerable<object> data = null;

  if (isTrue)
  {
    data = Enumerable.Range(1, 5).Select(i => new { Name = "Name " + i, Age = i * 10 });
  }
  else
  {
    data = Enumerable.Range(1, 5).Select(i => new { Name = "Name " + i });
  }

  ConvertToDatatable(data);
}

I think I'd use a classical OOP approach:

class BaseClass
{
  public string Name;
}

class Sub1 : BaseClass
{
  public double Age;
}

class Sub2 : BaseClass
{
  public int Age;
}

static void ConvertToDatatable(IEnumerable<BaseClass> input)
{
  foreach (var i in input)
  {
    if (i is Sub1)
      Console.WriteLine($"{i.Name}: {(i as Sub1).Age}");
    else if (i is Sub2)
      Console.WriteLine($"{i.Name}: {(i as Sub2).Age}");
    else
      Console.WriteLine($"{i.Name}: No age");
  }
}

static void BaseClassTest()
{
  bool isTrue = true;

  IEnumerable<BaseClass> data = null;

  if (isTrue)
  {
    data = Enumerable.Range(1, 5).Select(i => new Sub1 { Name = "Name " + i, Age = i * 10 });
  }
  else
  {
    data = Enumerable.Range(1, 5).Select(i => new BaseClass { Name = "Name " + i });
  }

  ConvertToDatatable(data);
}

Beside that, I think your naming is quite remarkable and rather hard to read because of the length:

For instance it's hard to quickly distinguish between:

vwTransactionInfoItemProcessesInDownloadFormat

transactionInfoItemProcessesInDownloadFormat

vwTransactionInfoItemProcesses


Inserting 50k rows with Spring Data and JPA I am using Spring Data JPA with Spring boot application. My requirement is to insert 50K rows in JPA entity table in one hour or less.

I have 3 entities A, B and C.  Entity A has a one to many association with entity B and a one to one association with entity C.

@Entity
public class A {
    @OneToMany(mappedBy = "chipJobItem", cascade = {
            CascadeType.MERGE,
            CascadeType.PERSIST,
            CascadeType.REFRESH,
            CascadeType.DETACH }, orphanRemoval = true, fetch = FetchType.LAZY)
    private List<B> values;
}

@Entity
public class B {
    @ManyToOne(fetch = FetchType.LAZY, cascade = {
        CascadeType.MERGE,
        CascadeType.PERSIST,
        CascadeType.REFRESH,
        CascadeType.DETACH })
    @JoinColumn(name = "fk_product_instance_id", referencedColumnName = "id", foreignKey = @ForeignKey(name = "DATAITEMS_FK"))
    private A job;
}

@Entity
public class C {
    @ManyToOne(fetch = FetchType.LAZY, cascade = {
        CascadeType.MERGE,
        CascadeType.PERSIST,
        CascadeType.REFRESH,
        CascadeType.DETACH })
    @JoinColumn(name = "fk_product_instance_id", referencedColumnName = "id", foreignKey = @ForeignKey(name = "DATAITEMS_FK"))
    private A job;
}

I have also set the JDBC batch size = 100, it improves the performance a little.

Need help improving the performance of the JPA insert.

To state the problem:
You have a large sum of operations that are repetitive and mostly similar(?) that are long running, and you want to reduce that run time.

If I have that right:

If order is not a concern You could consider inserting in parallel. If you can represent the 50,000 updates as a List, say, List<InsertCriteriaPojo>, and can represent the insert as a Consumer lambda, like so: ((InsertCriteriaPojo item) -> myDao.insert(item)), then this could be a one line call:

List<InsertCriteriaPojo> items = ...;

items.stream().parallel().map((InsertCriteriaPojo item) -> myDao.insert(item));

This creates a Stream of InsertCriteriaPojos, converts that to a ParallelStream, and them maps each item in the List to a Consumer and runs the Consumer/item tasks in a set of threads belonging to the common ForkJoinPool

While short, there are problems with this

The main problem as explained here is that you can't specify an Executor to ParallelStreams (as of Java 1.8*).

UPDATE: The above is inaccurate. This can be done like so:

List<InsertCriteriaPojo> items = ...;
ForkJoinPool customThreadPool = new ForkJoinPool(threadCount);
customThreadPool.submit(
    () -> items.stream().parallel().map(
        (InsertCriteriaPojo item) -> myDao.insert(item)
    )
).get;

Another, more complex way to run these tasks in parallel is to use an Executor you configure:

@Bean(name = "myExecutor")
public Executor executor(){
    ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
    executor.setCorePoolSize(yourProperties.getCorePoolCount());
    executor.setMaxPoolSize(yourProperties.getMaxPoolCount());
    executor.setQueueCapacity(yourProperties.getMaxQueueSize());
    executor.setThreadNamePrefix(yourProperties.getThreadNamePrefix());
    executor.initialize();
    return threadPoolTaskExecutor;
}

To run your consumers asynchronously:

...
@Autowire
@Qualifier("myExecutor")
Executor executor;
...
List<InsertCriteriaPojo> items = ...
//Make a Future for each task
List<CompletableFuture<Void>> asyncTaskFutures = list.stream().map(
    (InsertCriteriaPojo item) ->{
        CompletableFuture.runAsync(
            () -> myDao.insert(item), executor
        ).handle(
            (asyncVoidFuture, possibleError)->{
                if(possibleError != null){ //Handle your errors here }
                else{return asyncVoidFuture;}
        })
    }
).collect(Collectors.toList());

//Create a Future that is complete when allOf your task futures are complete
CompletableFuture<Void> process = CompletableFuture.allOf(
    asyncTaskList.toArray(new CompletableFuture[asyncTaskList.size()])
).thenRun(() -> asyncTaskList.stream().map(future -> future.join()));

//Complete the process-level future, blocks until complete
process.get();

While significantly more complex, the second is a more configurable and reusable way to solve your problem, but depending on your case either approach could serve your goal.


Thread class for exercises I just felt a sting as for the first time, I copy-pasted code blocks, and made very little change to it, to overload it. It works fine, but I have 3 constructors that are basically copy-pasted. I'm hoping there is a smarter way to do this.

This simply creates a thread and runs it with 1, 2 or 3 arguments, and then prints numbers to System.out using that thread. I will include an example usage below. The code doesn't strike me as bad of itself, but the sheer repetition is really bugging me. All criticism welcome.

import java.lang.Thread;

/**
 * Generic class that creates a thread and then loops and prints numbers to stdout.
 */
public class NumLoopThread implements Runnable {
    Thread thread;
    private String threadName;
    private int sleepTime = 400;
    private int numLoops = 10;

    /** Default constructor with threadName */
    public NumLoopThread(String threadName) {
        this.threadName = threadName;
        thread = new Thread(this, threadName);
        thread.start();
    }
    /** Extra constructor with threadName and sleepTime */
    public NumLoopThread(int sleepTime, String threadName) {
        this.sleepTime = sleepTime;
        this.threadName = threadName;
        thread = new Thread(this, threadName);
        thread.start();
    }
    /** Extra constructor with threadName, sleepTime, and numberLoops */
    public NumLoopThread(int sleepTime, String threadName, int numberLoops) {
        this.sleepTime = sleepTime;
        this.threadName = threadName;
        this.numLoops = numberLoops;
        thread = new Thread(this, threadName);
        thread.start();
    }
    /** Run the thread from start() */
    public void run() {
        System.out.printf("%s starting%n", threadName);
        try {
            for(int i = 0; i < numLoops; i++) {
                Thread.sleep(sleepTime);
                System.out.printf("%s, i = %d%n", threadName, i);
            }
        } catch(InterruptedException exc) {
            exc.printStackTrace();
        }
        System.out.printf("%s ending%n", threadName);
    }
}

Example usage I wrote for an exercise (feel welcome to also criticize this, if you want). Looks like this paste after it's ran.

public class ThreadIsAlive {
    public static void main(String[] args) {
        System.out.println("Main thread starting");
        // create 3 running threads
        NumLoopThread t1 = new NumLoopThread(400, "t1");
        NumLoopThread t2 = new NumLoopThread(400, "t2");
        NumLoopThread t3 = new NumLoopThread(400, "t3");

        do {
            System.out.print(".");
            try {
                Thread.sleep(100);
            } catch (InterruptedException exc) {
                exc.printStackTrace();
            }
        //wait until all threads terminate:
        } while (t1.thread.isAlive() ||
                 t2.thread.isAlive() ||
                 t3.thread.isAlive());
        System.out.println("Main thread ending");
    }
}

Chained constructors.
I find it interesting that you described the single-parameter constructor as your "default" one, and the others as "extras". It's usually the other way around: the constructor that initializes all immutable fields is the "main" one, and any other constructor that works off default values would be "convenience" constructors.

public NumLoopThread(int sleepTime, String threadName, int numberLoops) {
    this.sleepTime = sleepTime;
    this.threadName = threadName;
    this.numLoops = numberLoops;
    thread = new Thread(this, threadName);
    thread.start();
}

Take out numberLoops, you get this one:
public NumLoopThread(int sleepTime, String threadName) {
    // pasta
}

Take out sleepTime, you get this one:
public NumLoopThread(String threadName) {
    // pasta
}

Looks like threadName should be the first parameter in all 3 constructors, for consistency:
public NumLoopThread(String threadName, int sleepTime) {
    // pasta
}

public NumLoopThread(String threadName, int sleepTime, int numberLoops) {
    this.sleepTime = sleepTime;
    this.threadName = threadName;
    this.numLoops = numberLoops;
    thread = new Thread(this, threadName);
    thread.start();
}

That way when you look at your overloads, they all look like they naturally extend each other, in some sort of chain.
You already have your defaults:

private int sleepTime = 400;
private int numLoops = 10;

Consider making them static final, and single-purpose:
private static final int defaultSleepTime = 400;
private static final int defaultNumLoops = 10;

And since the actual fields are only ever assigned in a constructor, they should be marked final, too:
private final int sleepTime;
private final int numLoops;

Use this(); as the first instruction in the body, to "chain" the constructors:
public NumLoopThread(String threadName, int sleepTime) {
    this(threadName, sleepTime, defaultNumLoops);
}

public NumLoopThread(String threadName) {
    this(threadName, defaultSleepTime, defaultNumLoops);
}

Voilà, no more pasta!

That said I'm not sure about start()ing the thread in the constructor; it makes your run method a bit misleading. In general it is not a good idea to pass your 'this' reference to to a thread and start it during construction. The thread could then start with an unfinished object. Generally you should leave the starting of the thread to a separate method. (ref)


Functions in object literals I have 2 questions about functions inside object literals.  I have included the workarounds I've been using, but they seem hackish.  Is there a better way to do this?

ns = {
  a: function (x, y) { return x+y; },

  // Problem 1

  // This does not work: Error: ns.b is not a function
  b: this.a,

  // Is this the correct workaround for this?
  c: function (x, y) { return this.a(x, y); },

  // Problem 2

  // I understand why this won't work, because a is not defined in d's scope
  d: function (x, y) {
    function add(n, o) {
      return this.a(n, o);
    }
    return add(3, 4);
  },

  // Is this the correct workaround for this?  It works in Firefox and Chrome.
  e: function (x, y) {
    function add(n, o) {
      return ns.a(n, o);
    }
    return add(3, 4);
  }
};

ns.a(3, 4); //===> 7

ns.b(3, 4); //===> Error: ns.a is not a function
ns.c(3, 4); //===> 7

ns.d(3, 4); //===> Error: no method named a
ns.e(3, 4); //===> 7

EDIT:  Thank you for all your replies.  As missingno pointed out I am using ns to define a module, though I avoided the module pattern because I don't have any private variables.  The following seems to be an elegant solution to my problems.

var ns = {};
ns.a = function (x, y) { return x+y; };
ns.b = ns.a;
ns.c = function (x, y) {
  function add(n, o) {
    return ns.a(3, 4);
  }
  return add(x, y);
}

ns.a(3, 4); //===> 7
ns.b(3, 4); //===> 7
ns.c(3, 4); //===> 7

Since this has been moved to Code Review, I'd welcome any critiques of my implementation of Scheme's SRFI-1.

Problem 1

Just assign them one at a time, once the object already exists:

ns = {};
ns.a = function (x, y) { return x+y; };
ns.b = ns.a;

To answer specifically whether there's any other workaround if you're dead set on declaring these inside the object literal syntax: no.

Problem 2

Looks good enough to me, though I'd try to avoid using the object's name so deep into its definition.

Consider using prototyping instead.


Optimizations for bubble sort The code performs bubble sort on the basis if any swaps has been performed in the iteration. I made it sort of independent of number of iterations as in any conventional bubble sorting code.

I went on pretty much with what I understood from the cs50 class. This mechanism looks more intuitive to me. What improvements I need to do on the design/style/method?

#include<stdio.h>

void printMyArray(int a[],int n);

//BUBBLE SORT
int main(){
    int a[] = {-1 , 2 , 0 , -3 , 5 , 1};
    int n = 6;
    for(;;){

       int swap = 0;
       int i = 0;

       for(; i < n-1; i++)
          if( a[i+1] <= a[i]){
              int temp = a[i+1];
              a[i+1] = a[i];
              a[i] = temp;
              swap = 1;
           }
       if(swap == 0)
           break;
    }
    printMyArray(a,6);

}
void printMyArray(int a[],int n){
   int i;
   for(i = 0; i < n ; i++)
     printf("%d\t",a[i]);
   printf("\n");
}

Function

Certainly no reason to swap when values are equal.  In worse case of array {3,3,3,3}, leads to infinite loop.

//if( a[i+1] <= a[i]){
if(a[i+1] < a[i]) {

Use size_t to index arrays.

// void printMyArray(int a[],int n){
void printMyArray(int a[], size_t n) {

Style

Rather than int for Boolean variables use bool.

#include <stdbool.h>

// int swap = 0;
bool swap = false;

Recommend const when able.  Adds clarity to the function and allows for optimizations with some compilers.

// void printMyArray(int a[],int n){
void printMyArray(const int a[], int n) {

Minor: prefer int *a rather than int a[] as argument in printMyArray().
More useful to separate main() from the code under test.  Something like:

int main(void) {
    int a[] = {-1 , 2 , 0 , -3 , 5 , 1};
    bubble_sort(a, sizeof a/ sizeof a[0]);
    printMyArray(a, sizeof a/ sizeof a[0]);
    return 0;
}

Suggest fully {} blocks.  Use Idiomatic for(), Avoid  n-1, as size_t n may be 0.

   // int i = 0;
   // for(; i < n-1; i++)

   for(size_t i = 1; i < n; i++) {
     if(a[i] < a[i-1]) {
       int temp = a[i];
       a[i] = a[i-1];
       a[i-1] = temp;
       swap = 1;
     }
   }


3n + 1 programming challenge in Python I am trying to find a efficient solution for the 3n + 1 problem on uvaonlinejudge. The code I have uses memoization using a dictionary. I am getting a 'Time limit Exceeded' error when I submit the code.
Can anyone suggest an improvement(s) that will help with the execution time of this code.

I have already seen this post and implemented the proposed improvements. All other posts related to this are either for C++ or Java.

import sys

def recCycleLength(n,cycLenDict):
    if n==1:
        return 1
    if n not in cycLenDict:
        if n%2==0:
            cycLen = recCycleLength(n//2, cycLenDict)
            cycLenDict[n] = cycLen + 1
            return cycLen+1
        else:
            cycLen = recCycleLength(3*n+1, cycLenDict)
            cycLenDict[n] = cycLen + 1
            return cycLen+1
    else:
        return cycLenDict[n]

def maxCycle(a, b):
    i = a
    mydict = {}
    maxLength = 1
    while i <= b:
        m = recCycleLength(i, mydict)
        if m > maxLength:
            maxLength = m
        i = i + 1
    return maxLength

for line in sys.stdin:
    curr_line=line.split()
    num1 = int(curr_line[0])
    num2 = int(curr_line[1])
    if num1>num2:
        num1, num2 = num2, num1
    m = maxCycle(num1, num2)
    print("{} {} {}".format(num1, num2, m))

You should separate your responsibilities. Currently your function recCycleLength (which should be named rec_cycle_length to conform with Python's official style guide PEP8) does two things, calculating the return value and caching it in a passed dictionary. It would be better to outsource the latter to a decorator, like this one (which is the fastest memoization decorator for a single-valued function I know of):

def memodict(f):
    """ Memoization decorator for a function taking a single argument """
    class memodict(dict):
        def __missing__(self, key):
            ret = self[key] = f(key)
            return ret
    return memodict().__getitem__

@memodict
def rec_cycle_length(n):
    if n == 1:
        return 1
    elif n % 2 == 0:
        return cyc_len = rec_cycle_length(n//2) + 1
    return rec_cycle_length(3*n+1) + 1

You can even use this decorator on maxCycle when changing it to accept only one argument, the tuple (a, b). You could also be using max with a generator expression here:

@memodict
def max_cycle(bounds):
    a, b = bounds
    return max(rec_cycle_length(n) for n in range(a, b + 1))

(Use xrange in case you are using python 2.x.)

Your code on module level should be guarded by a if __name__ == '__main__': guard. You could also use map and sorted to get rid of quite a few lines of code:

if __name__ == '__main__':
    for line in sys.stdin:
        bounds = tuple(sorted(map(int, line.split())))
        print(" ".join(bounds), max_cycle(bounds))

This allows you to do e.g. from filename import rec_cycle_length in another script to reuse a function defined here.

Note that even though str.format is awesome, it is not needed here.

Since you seem to be using python 3.x, you might want to use functools.lru_cache instead of this decorator, like so:

import functools

@functools.lru_cache(2**10)
def rec_cycle_length(n):
    ...

At least on my machine it is a lot faster for some use cases (because the cache will not grow without bounds, making it better when there are few repetitions, i.e. many cache misses.) It is, however, only included in python >= 3.3.


Adding values to FormData - javascript let formData = new FormData();
  formData.append('File', document.file);
  formData.append('DocumentType', document.documentType);

  product = {
    ...values,
    productType: productT,
  };

  for (var key in product) {
    if (key != 'createdDate' && key != 'expirationDate') {
      formData.append(key, product[key]);
    } else {
      formData.append(key, new Date(product[key]).toUTCString());
    }
  }

  await createproduct(formData);

I need to send file and other data about product in one request so I added all the stuffs in FormData, and I have issues only with Dates, I must convert it to date because somehow it get confused if I don't convert it.

Can I somehow refactor this code to do this part about dates more elegant, if condition in for loop somehow smells bad..

P.S EDIT:

for (var key in product) {
   const transform = DateTransformer[key];
   formData.append(key, transform ? transform[product[key]] : product[key]);
}

Transformation rules

Especially useful if there are many fields and each needs a unique rule.

const dateToUTCString = d => new Date(d).toUTCString();

const DataTransformer = {
  createdDate: dateToUTCString,
  expirationDate: dateToUTCString,
};

// [...]

for (const [key, calue] of Object.entries(product)) {
  const transform = DataTransformer[key];
  formData.append(key, transform ? transform(value) : value);
}


SICP 1.3 Sum of Squares of two largest numbers Define a procedure that takes three numbers as arguments and returns the sum of squares of the two largest numbers.

I'm using just the machinery that was developed so far in SICP to be true to the exercise. My method works as far as I can tell -- I tested it for a number of cases and tested different permutations of inputs and get the same answers in each case. I just wasn't sure if there were a better way to do this exercise. Any feedback whatsoever is welcomed.

;; Define the squaring function.

(define (square n)
                  (* n n)
)

;; Define the maximum of two inputs a and b.

(define (mx a b)
                (cond  ((< a b) b)
                       ((< b a) a)
                       ((= a b) a))
)

;; Define the constant of elimination.

 (define (coe x y z)
                     (cond   ((= x y)  (-(square x)))
                             ((= y z)  (-(square y)))
                             ((= x z)  (-(square z))))
)

;; Define the sum of squares function. It consists of the sum of squares of the maxima of
;; each of three possible pairs of three elements.  Two of the first three terms will always
;; evaluate to the same value, and so the constant of elimination is added to get rid of the redundant term.

(define (sosl a b c)
                     (+ (square (mx a b)) (square (mx b c)) (square (mx a c)) (coe (mx a b) (mx b c) (mx a c)) )
)

In my opinion this is too much code for such a simple task.

Assuming that you do not have a max function, you can define it in this way:

 (define (max x y)
   (if (>= x y) x y))

or, if you cannot use if:

(define (max x y)
  (cond ((>= x y) x)
        (#t y)))

Then, the function sosl could be defined as:

(define (sosl a b c)
  (if (> a b)
      (+ (square a) (square (max b c)))
      (+ (square b) (square (max a c)))))

or, again, if you need to use cond, as

(define (sosl a b c)
  (cond ((> a b) (+ (square a) (square (max b c))))
        (#t (+ (square b) (square (max a c))))))


Unit of Work Pattern for creating users CreateUser function assumes the data being delivered via parameters are clean and gets called or initiated from the Business Manager (another class). This function is responsible to do the following things:

Fetch the Connection String
Open the database connection
Start the transaction
Inserts a user record
Loop through User Role entity list and Insert all the records
Commits the transaction
Finally or Catch Code block gets executed

I would like to identify is this a correct way of implementing Unit of Work pattern? Is there any catch in my implementation? Comments are written through out the code. By the way, am not using Entity Framework (using Dapper) and don't have an intention to use it.

using System;
using System.Collections.Generic;
using System.Data;
using Application.Entities.Interfaces;
using Dapper;

namespace Application.DataStore.UnitOfWork
{
    public class UserUnitOfWork
    {
        private IDbConnection connection;
        private IDbTransaction transaction;

        #region CreateTenant
        public void CreateUser(IUserEntity userEntity, IEnumerable<IUsersUserRoleEntity> usersUserRoleListEntity)
        {
            try
            {
                //fetching the connection string from the config file.
                this.connection = new DbConnectionHelper.DbConnectionHelper().GetDbConnection();

                //open the connection
                this.connection.Open();

                //start the transaction
                this.transaction = this.connection.BeginTransaction();

                //insert into user table
                int? userId = this.connection.Insert(userEntity, transaction);

                //multiple inserts in a loop for user roles
                foreach (var userRole in usersUserRoleListEntity)
                {
                    //assign UserId before the insert
                    userRole.UserId = userId.Value;

                    //insert into user role table
                    this.connection.Insert(userRole, transaction);
                }

                //commit
                this.transaction.Commit();
            }
            catch (Exception ex)
            {
                this.transaction.Rollback();
                throw ex;
            }
            finally
            {
                this.transaction.Dispose();  //Transaction dispose
                this.transaction = null;

                this.connection.Close();
                this.connection.Dispose();
                this.connection = null;
            }
        }
        #endregion
    }
}

#region

#region CreateTenant

Get rid of those. The region encompasses only a single method anyway, and your IDE should already allow you to collapse method scopes. Even if the region had more content, I'd recommend against it. See:

Are #regions an antipattern or code smell?

comments

//open the connection
this.connection.Open();

//start the transaction
this.transaction = this.connection.BeginTransaction();

These comments are about as useful as:
i++; // increment i

Good comments are comments that let the code speak for itself and only show up when there's a need to say why the code is doing what it's doing.
Good comments say why, not what. I would simply remove all comments in your code, because comments that state the obvious are nothing but a distraction and a waste of time.

this.transaction.Dispose();  //Transaction dispose

exceptions
Something hit me in your description of the code:

Finally or Catch Code block gets executed

That's not how finally works. finally blocks get executed regardless of whether or not an exception was thrown - it runs even when the catch block runs.
Speaking of the catch block:

    catch (Exception ex)
    {
        this.transaction.Rollback();
        throw ex;
    }

The problem is that throw ex isn't how you cleanly rethrow an exception. What you've done here destroys the stack trace and as far as client code is concerned, the stack trace begins in the catch block of this CreateUser method, which we all know is a lie.
This would have preserved the precious stack trace with all of its information:
    catch (Exception)
    {
        this.transaction.Rollback();
        throw;
    }

unit of work
A unit of work encapsulates a transaction - you know that.

    private IDbConnection connection;
    private IDbTransaction transaction;

That was a good start!
The problem is that, well..

finally
{
    this.transaction.Dispose();  //Transaction dispose
    this.transaction = null;

    this.connection.Close();
    this.connection.Dispose();
    this.connection = null;
}

You can only ever call the method once in a transaction. You own that transaction and that connection, so you should be disposing them.. and you do... but then if they're only going to be used in the CreateUser method, they shouldn't be instance-level fields, but local variables wrapped in a using block - and then you wouldn't even need that finally block.
Your disposing is overkill. Setting the object reference to null is useless, as is explicitly Close()-ing a connection that you're Dispose()-ing.
Alternatively, take the connection and transaction as a constructor argument, keep the instance-level fields, and implement IDisposable to dispose them.


Remove outliers from N dimensional data I have created a function that will remove outliers from a series of data. Generally the data n dimensional. Loosely, an outlier is considered an outlier if it +/- deviates by 1.5 standard_deviation's from the mean (see code for more detailed explanation of what constitutes an outlier).

The function can handle N dimensional arrays. It uses numpy and my code admittedly does not utilise numpy's iteration techniques. So I would appreciate how to improve this code and utilise numpy more.

import cv2
import numpy as np

def remove_outliers(data, thresh=1.5, axis=(0,1), use_median=False):
    # Post: Remove outlier values from data. A value in data is considered an outlier if it is NOT mean-std_deviation*thresh < value < mean+std_deviation*thresh

    res                 = []
    median              = np.median(data, axis)
    mean, std_dev       = cv2.meanStdDev(data)
    measure             = median if use_median else mean
    lower_thresh        = np.subtract(measure, np.multiply(std_dev, thresh))
    upper_thresh        = np.add(measure, np.multiply(std_dev, thresh))

    # Handle arrays that are n dimensional
    if len(data.shape) == 3:
        data = data.reshape((int(data.size/3), 3))

    for v in data:
        if np.all(v > lower_thresh) and np.all(v < upper_thresh):
            res.append(v)

    return res

if __name__ == "__main__":
    arr = np.array([10,99,12,15,9,2,17,15])
    print(arr)
    print(remove_outliers(arr, axis=0, thresh=1.5))

    arr = np.array([[(0,10,3),(99,255,255),(100,10,9),(45,34,9)]], dtype='uint8')
    print(arr)
    print(remove_outliers(arr, thresh=1.5))

Try to make the input and output types the same

In your example, remove_outliers() takes a NumPy array as input, but returns a regular Python list. It would be nicer to have the function return a NumPy array in this case.

The axis parameter only works when using the median

Make sure that when you specify axes, that the resulting behaviour of the function is what you would expect. If, for some reason, the axis parameter could not work with means, then you should throw an error if the caller explicitly specified which axes to work on.

Don't calculate values you are not going to use

You calculate both the median and the mean, but only use one of them. This is inefficient, just write the code as:

if use_median:
    measure = np.median(data, axis)
else:
    mean, std_dev = cv2.meanStdDev(data)
    measure = mean

I know you always need the standard deviation, but see the next comment.

Avoid using CV2 for something as simple as calculating mean and standard deviation

NumPy has functions for calculating those things as well. By avoiding importing cv2, you reduce the amount of dependencies users would have to install. As a bonus, the NumPy functions for mean and standard deviation also take an axis parameter. So consider writing:

std_dev = np.std(data, axis)

if use_median:
    measure = np.median(data, axis)
else:
    measure = np.mean(data, axis)

Don't special-case 3-dimensional data

Why are you reshaping the input array only if len(data.shape) == 3? Your should aim to make your code work generically with arrays of any number of dimensions.


Self-modifying esoteric language interpreter in Ruby I recently created Copy, an esoteric language, and wrote an implementation in Ruby.

The language has only 7 instructions:

copy <a> <b> <c> Copy the code block from a to b at c
remove <a> <b> Remove the code block from a to b
skip <value> Skip the next instruction if value is not 0
add <var> <value> Add value to var
negate <var> Negate var
print <value> Print value as an ASCII character
getch <var> Read a character and store it character code in var

All addresses are relative and values can be variables or signed integers,

require "io/console"

class Copy
    def initialize(code)
        @code = code.lines.map &:chomp
        @len = @code.length*2
        @i = 0
        @vars = {}
    end

    def value(s)
        if s =~ /-?\d+/
            s.to_i
        else
            @vars[s] or 0
        end
    end

    def step
        while @code.length > @len do
            @code.shift
            @i -= 1
        end
        line = @code[@i].split " "
        case line[0]
        when "copy"
            @code.insert(
                @i + value(line[3]),
                *@code.slice((@i + value(line[1]))..(@i + value(line[2])))
                )
        when "remove"
            @code.slice!((@i + value(line[1]))..(@i + value(line[2])))
        when "skip"
            if value(line[1]) != 0
                @i += 1
            end
        when "add"
            @vars[line[1]] = (@vars[line[1]] or 0) + value(line[2])
        when "negate"
            @vars[line[1]] *= -1
        when "print"
            STDOUT.write value(line[1]).chr
        when "getch"
            @vars[line[1]] = STDIN.getch.ord
        end
        @i += 1
    end

    def run
        while @i < @code.length
            step
        end
    end
end

if __FILE__ == $0
    unless ARGV[0]
        puts "Copy interpreter in Ruby"
        puts "Usage: #{$0} <file>"
    end

    c = Copy.new File.read(ARGV[0])
    c.run
end

Truth machine (If the input is 0, print 0 and exit, if the input is 1, print 1 in an infinite loop):

getch a
add b a
add b -48
print a
skip b
skip 1
copy -4 0 1

I am not really good at Ruby, can someone review this?

I would suggest refactoring the switch. This would make scaling easier

   def remove_callback
     @code.slice!((@i + value(line[1]))..(@i + value(line[2])))
   end

   ...

   line = @code[@i].split " "
   self.send("#{line[0]}_callback")


Python: flatten directories Here is a simple Python script that I wrote with lots of help from the Python mailing list. I moves all the files of subdirectories to the top level directory. It works well for my use, but if there are things that can be improved then I would like to know. I ask in the interest of being a better Python programmer and learning the language better. Thanks.

#!/usr/bin/python
# -*- coding: utf-8 -*-
import sys
import os
import shutil

currentDir = os.getcwd()
forReal = False

if ( currentDir in ['/home/dotancohen', '/home/dotancohen/.bin'] ):
        print "Error: Will not run in "+currentDir
        exit()

if ( len(sys.argv)>1 and sys.argv[1] in ['m', 'M', 'c', 'C'] ):
        forReal = True
else:
        print "\nThis is a test run. To actually perform changes, add 'm' (move) or 'c' (copy) after the program name.\n"

filesList = os.walk(currentDir)

for rootDir, folders, files in filesList:
        for f in files:
                if (rootDir!=currentDir):
                        toMove = os.path.join(rootDir, f)
                        print toMove
                        newFilename = os.path.join(currentDir,f)
                        renameNumber = 1
                        while(os.path.exists(newFilename)):
                                newFilename = os.path.join(currentDir,f)+"_"+str(renameNumber)
                                renameNumber = renameNumber+1
                        if ( forReal and sys.argv[1] in ['m', 'M'] ):
                                os.rename(toMove, newFilename)
                        elif ( forReal and sys.argv[1] in ['c', 'C'] ):
                                shutil.copy(toMove, newFilename)

if ( not forReal ):
        print "\nThis was a test run. To actually perform changes, add 'm' (move) or 'c' (copy) after the program name."

I'll typically wrap the main body of my code in a python script in a function and break the code apart into more functions, allowing me to reuse that code later.

import os
import shutil
import sys

def flattenDirectory(location, action, actuallyMoveOrCopy=True):
   """ This function will move or copy all files from subdirectories in the directory location specified by location into the main directory location folder
       location - directory to flatten
       action - string containing m(ove) or M(ove) or c(opy) or C(opy)"""
   #Your code here

if __name__ == "__Main__":
    #same code to find currentDirectory here
    actuallyMoveOrCopy = len(sys.argv)>1 and sys.argv[1] in ['m', 'M', 'c', 'C']
    flattenDirectory(currentDirectory, sys.argv[1], actuallyMoveOrCopy)

Now you can still run this code as you normally would, but should you need it in another script you can use it in an import without having to worry about the code automatically running at the time of import.


Loop through array of files and rsync to Dropbox The goal is pretty straightforward. There’s a list of dotfiles from my home directory. It loops through them and rsyncs all file to a Dropbox folder for backup purposes. A cron job runs this nightly. Code has worked well so far.

#!/usr/bin/python

import os
from os.path import expanduser
import sys
import subprocess

dotfiles = [
  '.ackrc',
  '.tmux.conf',
  '.vimrc',
  '.zshrc',
  '.gitconfig',
  '.ssh/config',
  '.zsh/MyAntigen.hs',
  '.vim/colors/dracula.vim'
]

home = expanduser("~")
src = home + '/'
dest = home + '/Dropbox/Dotfiles'
rsync = 'rsync --times'

if sys.stdout.isatty():
    rsync = rsync + ' --progress'

for item in dotfiles:
    src_file = os.path.join(src + item)
    cmd = rsync + ' ' + src_file + ' ' + dest
    process = subprocess.Popen(cmd, shell=True)
    process.wait()

This is my very first Python script (first time using rsync too) and I just started very recently. I’ve put this together based on trial and error and a lot of Stack Overflow. So I’m here for some genuine feedback.

Not much to complain for your first script, congrats.

I'd perhaps change it so that all files are rsynced with the same
command, if you look at the rsync documentation multiple sources may
be specified for a single target.  This would be more relevant for
bigger programs - you usually want to avoid running multiple processes
when you can have a single one do all the work, it makes bookkeeping
easier in the long run (and it's faster).  I'll leave that as an
exercise for the reader though.

Since expanduser will perform tilde expansion I'd not split off home
like this, just expand it twice, it's more readable IMO if you know what
~ means for paths (and most readers will know).  If you do keep it
like this, then consistently use os.path.join instead of concatenating
/ manually.  Actually the one use of os.path.join in the script is
wrong, it should be os.path.join(src, item).

The isatty is a nice touch.

For executing the process you're concatenating all arguments and then
also run them through a shell - the concatenating is problematic when
you have filenames with spaces in them, so in general I'd really rather
go with the list argument approach.  (Also see the security notes on the
documentation page of subprocess.)  If you're going through the shell
there's also no need for the manual expanduser; you can easily test
this if you prefix the rsync with echo (like I did to test the
script).

Looking at the documentation Popen can of course be used like this,
but it's not the most convenient option, I'd say either call or
check_call are more appropriate here - both wait for the process to
complete, while the latter also checks the exit status, which is vital
for more complex programs (without it you don't know if the process
completed successfully)!

#!/usr/bin/python

import os
from os.path import expanduser
import sys
import subprocess

dotfiles = [
    '.ackrc',
    '.tmux.conf',
    '.vimrc',
    '.zshrc',
    '.gitconfig',
    '.ssh/config',
    '.zsh/MyAntigen.hs',
    '.vim/colors/dracula.vim'
]

src = expanduser('~')
dest = expanduser('~/Dropbox/Dotfiles')

rsync = ['rsync', '--times']
if sys.stdout.isatty():
    rsync.append('--progress')

for item in dotfiles:
    subprocess.check_call(rsync + [os.path.join(src, item), dest])


ROT47 Implementation ROT13 ("rotate by 13 places", sometimes hyphenated ROT-13) is a simple letter substitution cipher that replaces a letter with the letter 13 letters after it in the alphabet. ROT13 is a special case of the Caesar cipher, developed in ancient Rome.

  ROT47 is a derivative of ROT13 which, in addition to scrambling the basic letters, also treats numbers and common symbols. Instead of using the sequence A–Z as the alphabet, ROT47 uses a larger set of characters from the common character encoding known as ASCII. Specifically, the 7-bit printable characters, excluding space, from decimal 33 '!' through 126 '~', 94 in total, taken in the order of the numerical values of their ASCII codes, are rotated by 47 positions, without special consideration of case. For example, the character A is mapped to p, while a is mapped to 2.

Below is my implementation of ROT47 algorithm:

#include <cstring>
#include <cstdio>
#include <iostream>

std::string rot47(std::string s)
{
    std::string s1 = "!\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
    std::string s2 = "PQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~!\"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNO";

    std::string ret = "";
    for (unsigned int i = 0; i < s.size(); i++)
    {
        std::size_t pos = s1.find(s[i]);
        ret += s2[pos];
    }

    return ret;
}

int main()
{
    std::string str = "HelloWorld!";

    std::string retFct = rot47(str);
    std::cout << retFct << std::endl;

    std::string retFct2 = rot47(retFct);
    std::cout << retFct2 << std::endl;
}

It works as expected but my question is : Is there something I can improve in my implementation? I don't know if there is a more direct way to get the result.

#include <cstring>
#include <cstdio>

You're not using either of those, so remove them, and include <string> instead to get std::string.

std::string rot47(std::string s) { /* ... */ }

Why copy the caller's string? You're only reading from it, so take it by const&.

std::string s1 = "!\"#$%&\...";
std::string s2 = "PQRSTUVW...";

These two are constant, don't allocate them for every call, make them static (and const):

static std::string const s1 = "...";

std::string ret = "";

That initialization is not necessary, a default-constructed string is empty. You might want to reserve the appropriate size since you know it though, for optimal performance.

for (unsigned int i = 0; i < s.size(); i++) { /* ... */ }

You can use range-based for loop with strings:

for (auto c: s) { /* ... */ }

And you should probably check that s1.find actually found something, otherwise the line after that is undefined behavior.

And s, s1 and s2 are rather poor choices for those variable names. Something like "source", "original", "plaintext" for the input would be better. map_from / map_to could work for the other two.


Using separate DBContext classes I know that the DBContext represents a session (Unit-Of-Work and Repository) with the database, however I an unsure as to when I should have a different DBContext. Currently, I have a separate DBContext for each table with a single DBSet for each DBContext.

Something like this (Blogs and Posts are in the same database):

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }
}

public class PostContext : DbContext
{
    public DbSet<Post> Posts { get; set; }
}

I'm not sure why I had a separate DBContext. I think that it is a sample I followed that was like this. However, I recently found that I get an exception when trying to do queries involving both contexts:

  The specified LINQ expression contains references to queries that are
  associated with different contexts.

So, it seems that the obvious solution is for a single DBContext and multiple DBSets:

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }
    public DbSet<Post> Posts { get; set; }
}

Is there any guideline for when to use different DBContexts and the pros and cons of each approach?

As you have already mentioned, DbContext is a UoW, and DbSet is a repository -- there is no need to reimplement those patterns, unless you're into ridiculously useless complexity.

Entity Framework wraps all pending changes in a transaction for you already, so each DbContext in an application contains DbSets that are somehow related. Blogs and Posts are definitely related, so they belong under the same context.

If your app had a blog module and a completely distinct chat module, you could use separate contexts, as your app would never be using both in the same controller.

I have an app with dozens of absolutely unrelated modules; either I defined a single context with 650 entity types (and only used a fraction of it every time), or I broke it down into much smaller and sanely manageable modules... I went for the second option. (the app is an ERP extension)


Angular Promise trying Wrapper Sometimes I need to repeat an execution of rejected promises several times, for example, to fetch some data over internet.
There is a wrapper, which accepts Promise and tryCount:

function tryPromise(f, tryCount) {
  return f().then(null, function(v) {
    tryCount--;
    if (tryCount > 0) {
      return $q.reject(v);
    }
    return tryPromise(f, tryCount);
  });
}

Is there some hidden troubles?

Well, first of all you aren't trying a promise. What you're really doing is trying out a function that returns a promise. So the name is already misleading. Try something better.

Next, f and v doesn't really tell me anything. Only when I read through the code did I realize f was the function to try out and v is supposed to be a value. Additionally, even if you name v a value, it's still not correct. Most reject handlers often pass an error object.

Code that requires one to actually read to understand is a bad practice. In this case it's negligible due to the size of the function. But if you're in larger codebases, it's a nightmare to maintain. Name your functions and variables meaningfully.

Lastly, your function doesn't allow for function arguments. It would be nice to at least accept argument 3 onwards as the arguments, or accept an array that would be the arguments.

My take on it would be:

function tryPromiseFunction(functionToTry, retries, ...args){
  return functionToTry(...args).then(null, error => {
    return retries > 0 ? tryPromiseFunction(functionToTry, retries - 1, ...args)
                       : Promise.reject(error);
  });
}