Project Euler problem #112 Problem statement:

  Working from left-to-right if no digit is exceeded by the digit to its
  left it is called an increasing number; for example, 134468.

  Similarly if no digit is exceeded by the digit to its right it is
  called a decreasing number; for example, 66420.

  We shall call a positive integer that is neither increasing nor
  decreasing a "bouncy" number; for example, 155349.

  Clearly there cannot be any bouncy numbers below one-hundred, but just
  over half of the numbers below one-thousand (525) are bouncy. In fact,
  the least number for which the proportion of bouncy numbers first
  reaches 50% is 538.

  Surprisingly, bouncy numbers become more and more common and by the
  time we reach 21780 the proportion of bouncy numbers is equal to 90%.

  Find the least number for which the proportion of bouncy numbers is
  exactly 99%.

My code:

import time
start = time.time()

def bouncy(N):
    '''Function determines whether arbitrary number N is a bouncy number.'''
    number = list(str(N))
    n1 = list(str(N))
    n2 = list(str(N))
    n1.sort()
    n2.sort(reverse=True)
    if (number != n1 and number != n2):
        return True
    else:
        return False

def find():
    start = 1
    total = 0
    bou = 0
    while True:
        total += 1
        if bouncy(start):
            bou += 1
        if bou/total >= 0.99:
            break
        start += 1
    return start
print(find())
print(time.time() - start)

What can be improved?

Code structure

The most common and widely accepted structure found in Python scripts from a high-level point of view looks something like:

# imports
import
...

# functions and classes, aka "library code"
class Foo:
    ...

def bar(batz):
    ...

if __name__ == "__main__":
   # part that is supposed to be run as script
   ...

Your code on the other hand does not follow this structure since there is some script code at the top (start = time.time()), then come your functions, followed by another block of script code. I highly recommend to follow that structure.

Further reading: Explanation of if __name__ == "__main__": from the official Python documentation.

The code itself

bouncy

The most striking aspect of bouncy from a stylistic point is the capital N as parameter name that goes against the official Style Guide for Python Code (often just called PEP8). Usually parameter and variable names are written in snake_case, e.g. small letters, possibly separated by underscores.

With that said, let's look at the actual algorithm:

number = list(str(N))
n1 = list(str(N))
n2 = list(str(N))

Lots of repeated code here. An easy way out would be to do the conversion once, and copy the results afterwards. This can be done using slicing like n1 = number[:], the list constructor n1 = list(number) or the copy module. Also, sorted would create a copy indirectly, so no need to do this manually (see the next point).

The next thing you do in your code is sorting those lists. Once ascending, then descending. Since the second one should by definition be the mirror of the previous one, you can use slicing again to avoid that second sort.

sorted_number = sorted(number)  # <- also does a copy

Oh, and did you know that sorted does work on strings as well and simply returns a new list? (sorted_number = sorted(N))

You could also use the result of the condition directly as return value of your function.

With some other minor changes you might end up with something like

def is_bouncy(number):
    '''Function determines whether arbitrary number N is a bouncy number.'''
    digits = list(str(number))
    sorted_digits = sorted(digits)
    return digits != sorted_digits and digits != sorted_digits[::-1]

find

find is almost as straightforward as it gets. I would advise to change the variable name bou to bouncy or n_bouncy now that the checking function is called is_bouncy. IMHO current would also likely be a better name for start. Also find_bouncy_boundary, maybe even with an optional "expected bounciness percentage" might be worth a thought.

From an optimization point of view, you could also start from 1000 if you're only looking for that 99%, since the task description tells you that there are exactly 525 bouncy numbers below 1000 ;-) But there is likely not much to gain from that.

The rest

Now all there is left to do is to take the spread out script code, wrap it up in if __name__ == "__main__":, and collect it at the bottom of the script:

if __name__ == "__main__":
    start = time.time()
    print(find())
    print(time.time() - start)

If the script part becomes more complex, this part is often put into a separate main() method.


Handling Circular References Without Recursion Despite the many decent answers to this question, one was unable to find an iterative solution. Here's what I came up with:
Code:
type CircularReferenceHandler = (parentKey: IndexKey, parentValue: Dictionary<IndexKey, any>, childKey: IndexKey, childValue: Dictionary<IndexKey, any>) => void;
type Dictionary<TKey extends IndexKey, TValue> = Record<TKey, TValue>;
type IndexKey = (number | string);

const processCircularReferences = <T extends object>(input: T, handler: CircularReferenceHandler, initialPathValue = "o"): T => {
    const objectStack = [{ k: initialPathValue, v: input, },];
    const objectTracker = new WeakSet<Dictionary<IndexKey, any>>();

    objectTracker.add(input);

    while (objectStack.length) {
        const { k: parentKey, v: parentValue } = objectStack.shift()!;

        for (const [childKey, childValue] of Object.entries(parentValue)) {
            if ((childValue && ("object" === typeof childValue))) {
                if (objectTracker.has(childValue)) {
                    handler(parentKey, parentValue, childKey, childValue);
                }
                else {
                    objectStack.push({ k: `${parentKey}.${childKey}`, v: childValue, });
                    objectTracker.add(childValue);
                }
            }
        }
    }

    return input;
}

Usage:
let o: any = { x: 0, y: 1, };

o.me = {};
o.me.self = o;
o.self = o;

processCircularReferences(o, (pK, pV, cK, _cV) => {
    console.log(`${pK}.${cK}`);

    pV[cK] = undefined;
});

console.log(o);

/*
    [log]: o.me.self
    [log]: o.self
    [log]: {
               me: {
                   self: undefined
               },
               self: undefined,
               x: 0,
               y: 1
           }
 */

Correctness
Following testcase breaks your code:
const o={};
o.x = o.y = {};

This is due to objectTracker tracks all seen values not all parent values.
Interface Design

initialPathValue looks strange to me. The parameter has no meaning to this function as far as I can tell.
Dot separated path won't work if any .s in keys. For example: { "a.b": {}, "a": { "b": {} } }. To avoid any confusion, I would suggest use string[] instead.

Implementation

objectStack is named as Stack, but you are using it as a Queue. I believe there should be a mistake here. Maybe you want pop() instead of shift().
parentValue is a WeakSet. But I cannot find a reason why any objects in it will be garbage collected during the functions life cycle. So maybe a simple Set would fint your requirement.


Minimum path sum in a triangle (Project Euler 18 and 67) with Python Project Euler 18
Project Euler 67

As problem 67 is harder, I'll go with that one:

  By starting at the top of the triangle below and moving to adjacent
  numbers on the row below, the maximum total from top to bottom is 23.

             3
            7 4
           2 4 6
          8 5 9 3

  That is, 3 + 7 + 4 + 9 = 23.

  Find the maximum total from top to bottom in triangle.txt (right click
  and 'Save Link/Target As...'), a 15K text file containing a triangle
  with one-hundred rows.

  NOTE: This is a much more difficult version of Problem 18. It is not
  possible to try every route to solve this problem, as there are 2^99
  altogether! If you could check one trillion (10^12) routes every second
  it would take over twenty billion years to check them all. There is an
  efficient algorithm to solve it. ;o)

I solved this using dynamic programming. Here's my code:

def max_triangle_sum(triangle, height):
    if height == 1:
        return triangle[0][0]

    dp = [[-1] * row_index for row_index in range(1, height + 1)]

    dp[0][0] = triangle[0][0]

    for i in range(1, height):
        dp[i][0] = dp[i-1][0] + triangle[i][0]
        dp[i][i] = dp[i-1][i-1] + triangle[i][-1]

        for j in range(1, i):
            dp[i][j] = max(dp[i - 1][j - 1], dp[i - 1][j]) + triangle[i][j]

    return max(dp[-1])

if __name__ == '__main__':
    height = None

    while True:
        try:
            height = int(input('Please enter height of the triangle: '))

            if height <= 0:
                raise Exception('Height cannot be negative or 0')

            break

        except Exception as e:
            print(f'Error occurred: {e}')
            print()

    triangle = []

    for row_index in range(1, height + 1):
        while True:
            try:
                row = list(map(int, input(f'Please enter {row_index} space seperated integer(s): ').split()))

                if len(row) != row_index:
                    raise Exception(f'Exactly {row_index} numbers required')

                if any(number < 0 for number in row):
                    raise Exception(f'Numbers cannot be negative')

                triangle.append(row)

                break

            except Exception as e:
                print(f'Error occurred: {e}')
                print()

    print('The maximum triangle sum is: ')
    print(max_triangle_sum(triangle, height))

This runs quite fast, considering it would actually take 20 billion years (If your computer can compute 10^12 operations in 1 second) to solve using brute force. But still, it doesn't look that good

How do I improve this code?

General

Since the problem gives the triangle as a text file, your program should probably be able to read that file. It would be quite a lot of work to input them manually, and when using the file like in python triangle.py < triangle.txt a first row with the number of lines needs to be added. This reading function can be quite simple:

def read_triangle(file_name):
    with open(file_name) as f:
        return [list(map(int, line.split())) for line in f]

Algorithm

As for the actual algorithm, I'm not quite sure what yours does. Which means that it is not very readable :). Not having any documentation, like a docstring does not help either.

The easiest algorithm for this is a bottom up approach that reduces each line into the maximum sum reachable from the line above it.

def reduce_rows(row, row_below):
    """Reduces two consecutive rows to the maximum reachable sum from the top row."""
    return [max(row_below[i], row_below[i+1]) + row[i] for i in range(len(row))]

def max_sum(triangle):
    """Find the maximum sum reachable in the triangle."""
    row_below = triangle.pop()
    for row in reversed(triangle):
        row_below = reduce_rows(row, row_below)
        # print(row_below)
    return row_below[0]

if __name__ == "__main__":
    triangle = read_triangle("triangle.txt")
    print(max_sum(triangle))

Style

It's a good thing that you are already using a if __name__ == "__main__": guard. You also seem to be mostly following Python's official style-guide, PEP8. The only thing you could improve on that part is to reduce the number of blank lines. While some help the readability, parts that are logically connected should be in one block:

height = None
while True:
    try:
        height = int(input('Please enter height of the triangle: '))
        if height <= 0:
            raise Exception('Height cannot be negative or 0')
        break
    except Exception as e:
        print(f'Error occurred: {e}\n')

triangle = []
for row_index in range(1, height + 1):
    while True:
        try:
            row = list(map(int, input(f'Please enter {row_index} space seperated integer(s): ').split()))
            if len(row) != row_index:
                raise Exception(f'Exactly {row_index} numbers required')
            elif any(number < 0 for number in row):
                raise Exception(f'Numbers cannot be negative')
            else:
                triangle.append(row)
                break
        except Exception as e:
            print(f'Error occurred: {e}\n')

I also put the separate exceptions and the happy path into an if..elif..else structure. This is not really needed (the other cases are never reachable if an exception is raised), but IMO it helps with readability. It also make it easier if at some point you decide that those should just print a message directly, instead of raising an exception that is immediately caught only to be printed. You could make those print statements directly, which would allow you to be more specific about the exception you expect. This makes it so that all other exceptions stop the program (as they should):

while True:
    try:
        height = int(input('Please enter height of the triangle: '))
    except ValueError as e:
        print(f'Error occurred: {e}\n')
    if height > 0:
        break
    else:
        print('Error occurred:  Height cannot be negative or 0')

triangle = []
for row_index in range(1, height + 1):
    while True:
        try:
            row = list(map(int, input(f'Please enter {row_index} space separated integer(s): ').split()))
        except ValueError as e:
            print(f'Error occurred: {e}\n')
        if len(row) != row_index:
            print(f'Error occurred:  Exactly {row_index} numbers required')
        elif any(number < 0 for number in row):
            print(f'Error occurred:  Numbers cannot be negative')
        else:
            triangle.append(row)
            break

Better user input

This is actually a good place to implement a function asking for user input and validating it. We need type validation and input validation here, so this should suffice:

def ask_user(message, type_=str, validate=None, non_valid_msg=""):
    while True:
        try:
            user_input = type_(input(message))
        except ValueError as e:
            print(f"Expected {type_}")
            continue
        if validate is not None:
            if validate(user_input):
                return user_input
            else:
                print(non_valid_msg)
        else:
            return user_input

Which makes your code a lot easier:

def list_of_int(s):
    return list(map(int, s.split()))

if __name__ == "__main__":
    height = ask_user("Please enter height of the triangle: ", int,
                      lambda h: h > 0, "Height must be positive")
    triangle = [ask_user(f"Please enter {n} space separated integer(s): ", list_of_int,
                         lambda l: len(l) == n, f"Exactly {n} numbers required")
                for n in range(1, height + 1)]
    ...


Find minimum in rotated sorted array The given code find the minimum element in a sorted array rotated by some fixed distance.
Eg: [1, 2, 3, 4, 5, 6, 7] -> [3, 4, 5, 6, 7, 1, 2]

The elements in the array are all unique. Just wanted to check if the code handles all edge cases and is correct.

def findMin(ary):

    def recurse(lo, hi):
        # Base cases
        if (ary[lo] < ary[hi]):
            return ary[lo]
        if (hi - lo == 1):
            return min(ary)

        mid = lo + (hi - lo) / 2
        if (ary[mid] < ary[hi]):
            return recurse(lo, mid)
        else:
            return recurse(mid, hi)

    return recurse(0, len(ary) - 1)

ary = (3, 4, 5, 6, 7, 1, 2)
print findMin(ary) # 1

Why are you giving up?

if (hi - lo == 1):
    return min(ary)

So we're almost done with our nice O(lg N) algorithm, when suddenly... we start over and do a whole new O(N) search from scratch throwing everything away? Why? Let's examine such a scenario:

ary = [3, 4, 5, 6, 0, 1, 2]

lo = 0, hi = 6, mid = 3
ary[mid] (6) > ary[hi] (2), so recurse mid to hi

lo = 3, hi = 6, mid = 4
ary[mid] (0) < ary[hi] (2), so recurse lo to mid

lo = 3, hi = 4
hi - lo == 1, so return min(ary) == 0

At this point, we have ary[lo] == 6 and ary[hi] == 0. We know one of those two is the minimum, and we know which one that is. So let's use that information:

def recurse(lo, hi):
    # Base cases
    if (ary[lo] < ary[hi]):
        return ary[lo]
    elif (hi - lo == 1):
        return ary[hi] ## just return a number, don't call min()
    else:
        mid = ...
        # rest as before


Countdown timer in PHP Using PHP and jQuery, I was able to make this countdown timer. I've searched various books and obviously on Google, but couldn't find this type of approach as used by me here. I sincerely feel there is something wrong with it. Can anyone tell me if this is the right approach?

                 <!DOCTYPE html>

recdisp.php

                         <?php
  date_default_timezone_set("Asia/KolKata");

  $time=1434402728+3600000*24*10000;//some time in future.
 echo "
";
  echo (date("d",$time-time())-1);
  echo "
";
  echo "
";
  echo " Days ";
  echo "
";
  echo "
";
  echo (date("h",$time-time()));
  echo "
";
 echo "
";
  echo " Hours ";
 echo "
";echo "
";
    echo (date("i",$time-time()));
  echo "
";
 echo "
";
 echo " Minutes ";
 echo "
";
  echo "
";
  echo (date("s",$time-time()));
 echo "
";
echo "
";
 echo " Seconds ";
 echo "
";
 ?>

Transferring data from a client to a server is an expensive procedure, and should be minimized as much as possible.  If the purpose of your PHP is to simply provide formatted time data, I would highly recommend letting your client do as much processing and formatting as possible

For example, what happens if the client loses internet connection?  The process will fail and the counter will not count down.

What if the connection to the server is slow or inconsistent?  The responses from the server may not return in the expected order, resulting in a countdown timer with unpredictable behavior.

Proposed solution:

PHP script processes date and returns UNIX timestamp
HTML page contains static divs with element IDs accessible to jQuery
On $(document).ready, only once, use the $.get function instead of $("#id").load to load the timestamp from the server and parse it as a Date object
Start your timer with a callback that processes the date and populates the div structure once per second.

Sample code, as requested:

recdisp.php:

<?php
    date_default_timezone_set("Asia/KolKata");
    $time=1434402728+3600000*24*10000;//some time in future.
    echo $time;
?>

timer.html:

<!doctype html>

       ...

Seconds

I will let you fill in the blanks


Hashing array of strings as Merkle tree in one function I want to hash an array of strings into one hash as shown on scheme on the image below. But I do not want to implement some class based Merkle tree. It's just a simple function. Is this a good implementation or are there any possible improvements?

public static String hashStrings(@NotNull String [] strs)
{
    if (strs.length == 1) return hashString(strs[0]);

    List<String> hashes = new ArrayList<>();

    for (int i = 0; i < strs.length; i++)
    {
        hashes.add(hashString(strs[i]));
    }

    while(hashes.size() > 1)
    {
        int currentSize = hashes.size();
        int nextSize = currentSize % 2 == 0 ? currentSize / 2 : (currentSize + 1) / 2;

        for (int i = 0; i < currentSize - 1; i += 2)
        {
            hashes.set(i / 2, hashString(hashes.get(i) + hashes.get(i + 1)));
        }

        if (currentSize % 2 == 1)
        {
            hashes.set(nextSize - 1, hashString(hashes.get(currentSize - 1)));
        }

        hashes = hashes.subList(0, nextSize);
    }

    return hashes.get(0);
}

In my opinion this statement is quite complicated to read, compared to what you want achieve:

int nextSize = currentSize % 2 == 0 ? currentSize / 2 : (currentSize + 1) / 2;

You could consider using some brackets:

int nextSize = (currentSize % 2 == 0) ? (currentSize / 2) : ((currentSize + 1) / 2);

Or just use Math.ceil():

int nextSize = (int) Math.ceil(currentSize / 2.0))

Furthermore you are computing hashes.size() twice per loop:

while(hashes.size() > 1)
{
    int currentSize = hashes.size();
    // ...

you could write:

int currentSize = hashes.size();
while(currentSize > 1)
{
    currentSize = hashes.size();
    // ....

instead.

Other than that your code looks fine to me, besides strs is not the best name for a parameter. You could rename it to improve readability and usability.


Find all distinct combinations of letters in a string I've been preparing for interviews and picking up modern C++. A recent prep question I did was to find all combinations (or distinct subsets) of letters from a string.  E.g. 'AA2'-> {'A', '2', 'A2', 'AA2'}.  Note that the order doesn't matter.  I would like feedback both on the approach and on use of C++ style/best practices.  My attempt is:

#include <string>
#include <algorithm>
#include <unordered_set>
#include <iostream>

std::unordered_set<std::string> getPermutations(std::string str){
    std::unordered_set<std::string> ret;
    if (str.length() <= 1){
        ret.insert(str);
    } else {
        char removedLetter = '\0';
        for(unsigned i = 0; i < str.length(); i++){
            if (removedLetter == str[i]){ //Already found all subsets missing this character
                continue;
            }
            removedLetter = str[i];
            std::string subString = str.substr(0, i) + str.substr(i+1, std::string::npos);
            std::unordered_set<std::string> shorter = getPermutations(subString);
            for(auto shortPerm : shorter){
                ret.insert(shortPerm);
                auto ind = shortPerm.begin();
                for(; ind < shortPerm.end(); ind++){ //Find where to insert letter so that ordering is preserved for eliminating duplicates
                    if(*ind > removedLetter){
                        break;
                    }
                }
                shortPerm.insert(ind, removedLetter);
                ret.insert(shortPerm);
            }
        }
    }
    return ret;
}

int main(int argc, char ** argv){
    std::string str;
    std::cin >> str;
    std::sort(str.begin(), str.end()); //Sort string so that method works.
    for(const auto &p :  getPermutations(str)){
        std::cout << p << ' ';
    }
    std::cout << std::endl;
}

Minor remarks

Since the sortedness of the input is a precondition of getPermutations, I would either do this step in the function itself, or at least add a check for this condition.
Passing string by reference instead of by value: I recommend passing const std::string & instead of std::string. In this way, the string does not get copied to the stack. This matters especially, if the string is very long.

Alternative

While the recursive approach is elegant and easily understandable, it might be less convenient for very long inputs. I suggest another solution, based on the idea, that if you want all permutations, then each character can either be part, or not be part of a permutation. This gives us 2^N possible permutations, where N is the length of the input. So, you can just generate this number (2^N), and then iterate from 0 to 2^N-1, and calculate the corresponding perm. for each number. So, if the number is represented as binary digits, then 1 means that the character at the corresponding position should be part of the permutation, 0 that it should not. (If you do not need the empty perm., just start the iteration from 1, instead of 0.)

With your example, 'AA2', this would work as follows: N=3, so iterate from 0 to 7 (=2^3 - 1). 0 corresponds to the empty set. 1 = 001b, so '2' is part of the permutation. 2 = 010b, so only (the second) 'A' is part of the permutation. 3 = 011b, so the perm. is A2, etc.

Remarks:

This approach would yield two identical 'A2' perms., but you can avoid that by storing the return value in a set, as you are currently doing it (unless you do want both).
With this solution, it is not needed to sort the input.
Other representations are possible as well, e.g a vector of booleans, instead of a number (especially convenient very large inputs).


Validate profile completeness This function checks these 6 fields for a user profile to determine if they have "completed" their profile. This just feels like it could be done better, but I can't figure how.

/**
 * Is the user's entire profile filled in?
 *
 * @return boolean
 */
public function isProfileComplete()
{
    $checkList = [
        $this->isAddressGiven(),
        $this->isGenderSelected(),
        null !== $this->getFirstName(),
        null !== $this->getLastName(),
        null !== $this->getJobTitle(),
        null !== $this->getAboutMe(),
    ];

    // If any one of the checks fails, profile is missing information
    foreach ($checkList as $checkItem) {
        if (!$checkItem) {
            return false;
        }
    }

    return true;
}

There's no sense in creating a dynamic array for a fixed set of checks. Just put them in a single boolean expression.

public function isProfileComplete()
{
    return $this->isAddressGiven()
        && $this->isGenderSelected()
        && null !== $this->getFirstName()
        && null !== $this->getLastName()
        && null !== $this->getJobTitle()
        && null !== $this->getAboutMe()
        ;
}


Another ||= question I promise that I've done my reading on this topic.  I've seen many suggestions and critiques, but I haven't seen the argument below critiqued.  I'm not trying to beat a dead horse, but I don't think anyone's offered this one up yet:

x ||= y
is functionally equivalent to
x = y unless x

The code below has been through 5 years of feature additions; hence the difference in coding style.  An evaluation is a survey questionnaire.  To determine which questions to present, first see if an evaluation ID is specified in the URL, then fall back on the evaluation the user should see (if any), then fall back on the evaluation configured for this site (if any).

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation ||= Evaluation.find(@user.id_of_required_evaluation)
evaluation = Evaluation.find(@site.id_of_required_evaluation) unless evaluation

This doesn't flow well visually.  I'm at the point where I can use ||= or unless and don't believe there to be any difference between the two.

Therefore, my question is:  Is there any reason to choose one of the below blocks over the other?

Option 1

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation ||= Evaluation.find(@user.id_of_required_evaluation)
evaluation ||= Evaluation.find(@site.id_of_required_evaluation)

Option 2

evaluation = Evaluation.find(params[:evaluation_id]) if params[:evaluation_id]
evaluation = Evaluation.find(@user.id_of_required_evaluation) unless evaluation
evaluation = Evaluation.find(@site.id_of_required_evaluation) unless evaluation

My apologies to the dead horse who again finds himself being beaten and to those who have to watch.  If this is a stupid question, please consider the horse and beat me instead.

I think it really just comes down to opinion.

I think using ||= is better in this case, it's easier to glance at the code and see what it's intent is. When using the inline if or unless, I have to scan the whole line to know what is going on.

evaluation ||= Evaluation.find(@user.id_of_required_evaluation)

That said, I do like the inline if and unless with returns, since it reads nicely.

return false unless isSelected


Brainfuck interpreter in JavaScript, take 2 The previous version is here. This version takes suggestions from that review into account:

brainfuck is now an object instead of a function, and brainfuck.run(source) kicks off the interpreter.
There is no built-in support for multiple interpreter instances, but Object.create(brainfuck) will do the trick.
Stub functions (which throw errors) are provided for the implementation-dependent read and write functions.

This version also addresses the redundancy in the first version by handling loops differently. All loop start and end positions are pre-calculated, so the entire loop is now bypassed when appropriate instead of passing over each operation and no-opping. This also removes the need to keep a stack of loop start positions while the program runs.

Also, this version adds proper exports for AMD script loaders and CJS environments, with a fallback to the global namespace object.

(function(global){

    // Find start and end positions of each loop.
    function findLoops(code) {
        var start;
        var startpoints = {};
        var endpoints = {};
        var stack = [];

        for (var i = 0; i < code.length; i++) {
            if (code[i] == '[') {
                stack.push(i);
            } else if (code[i] == ']') {
                start = stack.pop();
                startpoints[i] = start;
                endpoints[start] = i;
            }
        }

        return { start: startpoints, end: endpoints };
    }

    // Run the brainfuck interpreter.
    function run(source) {
        var brainfuck = this;
        var code = source.replace(/[^-+<>.,[\]]/g, '').split('');  // program code
        var loop = findLoops(code);  // loop start and end positions
        var data = [];  // array of data cells stored by the program code
        var cell = 0;   // index in the data array representing one "cell" of data
        var next = 0;   // index in the code array of the next instruction to run
        var operation = {
            '>': function () { ++cell; },
            '<': function () { --cell; },
            '+': function () { data[cell] = (data[cell] || 0) + 1; },
            '-': function () { data[cell] = (data[cell] || 0) - 1; },
            '.': function () { brainfuck.write(data[cell]); },
            ',': function () { data[cell] = brainfuck.read(); },
            '[': function () { if (!data[cell]) { next = loop.end[next]; } },
            ']': function () { if (data[cell]) { next = loop.start[next]; } }
        };

        while (next < code.length) {
            operation[code[next]]();
            next++;
        }

        if (brainfuck.end) {
            brainfuck.end();
        }
    }

    // Export a module for AMD loaders, CJS environments, or as a global.
    function exportModule(name, module) {
        if (global['define'] && global['define']['amd']) {
            global['define'](module);
        } else if (global['exports']) {
            for (var key in module) if (module.hasOwnProperty(key)) {
                global['exports'][key] = module[key];
            }
        } else {
            global[name] = module;
        }
    }

    // Export brainfuck module.
    exportModule('brainfuck', {
        'run': run,
        'read': function() { throw new Error('"read" function not provided'); },
        'write': function() { throw new Error('"write" function not provided'); }
    });

}(this));

Test it out here. I have no specific concerns at this point, I'm just looking for a general review.

I think I said I'd refrain from reviewing, since I reviewed the first take, but I can't help myself.

Again: I like it! It's very neat and tidy. I really can't fault it.

It's well-formatted
It's readable
It's efficient

Hell, it even taught me Brainfuck! Kudos!

If I were to be super nit-picky (and I'll have to be, to find something to write), it's that it'd be nice to use plural form for loop and operation (i.e. loops and operations). I know, I know, loops will ruin the nice 4-letter-aligned formatting that going on, but still.

next should perhaps be called current or similar, since that's what it actually is.

I guess you could skip the split(''), and just loop through the string, but either way works.

I might also prefer a stricter typeof brainfuck.end === 'function' check at the, uh, end, but that's just me being pedantic. OTOH, you could just stub it to a no-op function, and not have a check.

It might be nice to initialize data to [0], and have < and > initialize blank cells instead of doing it lazily in the +/- operations. It'd be a little more correct, and you'd avoid a program like . spitting out undefined instead of 0. You can do it in one (admittedly tricky) assignment:

'>': function () { data[++cell] = data[cell] || 0; }

Lastly, it might be nice to check input from read, since you could basically return whatever you want from your own read function. Worth it? Nah, not really.

I do have some other ideas, though. Less review, and more "what if...".

findJumps (formerly findLoops)

We talked about doing syntax-checking in the comments (and you came up with a nice implementation), so I'll skip that here. Instead, this is just an alternative to the code in the question.

function findJumps(code) {
  var jumps = [],
      stack = [],
      character, i, j;

  for(i = 0 ; character = code[i] ; i++) {
    if(character === '[') {
      stack.push(i);
    } else if(character === ']') {
      j = stack.pop();
      jumps[i] = j;
      jumps[j] = i;
    }
  }

  return jumps;
}

Instead of returning an array of objects, it's enough to just return an array of indices. It doesn't really matter what's start and end.

You could then change the [ and ] instructions to

'[': function () { data[cell] || (next = jumps[next]); },
']': function () { data[cell] && (next = jumps[next]); }

which seems nicely "symmetrical" :)

switch

Yeah, yeah, using a switch statement is boring. Still, it has some (minor) maybe-benefits:

Illegal characters can be absorbed by the default: case (or no case at all) - no need to sanitize the code string first (though depending on how loop-heavy the code is, it may be more efficient to sanitize it anyway).
It avoids declaring new operation functions on each call to run, and the calling overhead of those functions (though the impact of those things is in all likelihood zero; it's all optimized away).

Looks ok, I think (works too)

function run(code) {
  var jumps = findJumps(code),
      data = [],
      cell = 0,
      instruction, curr;

  for(curr = 0 ; instruction = code[curr] ; curr++) {
    switch(instruction) {
      case '>': ++cell;                              break;
      case '<': --cell;                              break;
      case '+': data[cell] = (data[cell] || 0) + 1;  break;
      case '-': data[cell] = (data[cell] || 0) - 1;  break;
      case '[': data[cell] || (curr = jumps[curr]);  break;
      case ']': data[cell] && (curr = jumps[curr]);  break;
      case ',': data[cell] = this.read();            break;
      case '.': this.write(data[cell]);              break;
    }
  }

  this.end();
}

The above assumes that this.end is (at least) stubbed. I've left out the cell initialization, but it works fine with that too.

Anyway, this is just me having some fun with it. Again, I can't really fault your code.

Now, how about tackling Befunge? Or maybe just Huh?


Fast sums of power algorithm This code reaches time limit on 1 test case, solving this challenge. The approach that I'm using is from the book Algorithms Functional Programming Approach, which is a backtracking depth searching method. How do I improve speed of this code? Or is there better way to do it?

searchDfs :: (Eq node) =>
             (node -> [node]) -> -- generates successor nodes
             (node -> Bool) -> -- is goal
             node -> -- first node
             [node]
searchDfs succ goal x = search' ([x])
  where
    search' s
      | null s = []
      | goal (head s) = head s : search' (tail s)
      | otherwise = let x = head s
                    in search' (succ x ++ tail s)

type Degree = Int
type CurrentSum = Int
type CurrentNumber = Int
type Node = (CurrentSum,CurrentNumber,Degree,Goal)
type Goal = Int

succN :: Node -> [Node]
succN (sum,i,d, goal)  = [( sum+i'^d, i', d, goal) | i' <- [(i+1)..goal], sum+i'^d <= goal ]

goal :: Node -> Bool
goal (sum,_,_,m) = sum == m

countSols :: Degree -> Goal -> Int
countSols d m = foldr (+) 0 $ map (\(sum,i) -> length (searchDfs succN goal (sum,i,d,m))) startingNumbers
  where startingNumbers = [ (i^d,i) | i <- [1..m], i^d <= m]

main = do
  m <- readLn
  d <- readLn
  let c = countSols d m
  print c

The search' could could be written more idiomatically (but not necessarily faster) as

search' [] = []
search' (x:xs) | goal x    = x : search' xs
               | otherwise = search' (succ x ++ xs)

The Eq constraint on searchDfs seems to be unused.
In succN, you are calculating sum+i'^d twice (unless the compiler optimizes that away, but it’s better not to rely on that). Also, it will calculate it for all values of i' until goal, although you probably want to abort early. Maybe using takeWhile would be better:

 succN (sum,i,d, goal) =
     takeWhile (\(s,_,_,_) -> s <= goal)
               [( sum+i'^d, i', d, goal) | i' <- [(i+1)..goal]]

It looks as if the d and goal field of your Nodes is always the same. Then you shouldn’t have it as part of the Node, but rather pass it around explicitly. You can do that without modifying searchDfs!


Correctness of a CSV writer in Node.js A friend of mine wanted an array-to-CSV string function for Node.js, so I came up with this. Basically, it can take in a single object, a 2D array or an array of objects.

If the children or parent is an object then the object properties become the headers, if the children are arrays then the first array is the headers.

Each cell is enclosed inside double quotes in the case of commas inside of the cell, and any double quotes inside the cell are escaped. Is there anything else I should be escaping?

exports.objectToCSVString = function (ob) {
    var str = "", row, a, i, o, c, r;
    if(ob instanceof Object && !(ob instanceof Array)){
        ob = [ob];
    }
    if(ob instanceof Array){
        for(r in ob){
            if(ob.hasOwnProperty(r)){
                row = ob[r];
                if(row instanceof Array){
                    a = "";
                    for(i in row){
                        if(row.hasOwnProperty(i)){
                            a += a.length === 0 ? "" : ",";
                            a += "\"" + row[i].toString().replaceAll("\"", "\\\"") + "\"";
                        }
                    }
                    str += str.length === 0 ? a : "\r\n" + a;
                }else if(row instanceof Object){
                    if(o === undefined){
                        o = [];
                        a = "";
                        for(c in row){
                            if(row.hasOwnProperty(c)){
                                o.push(c);
                                a += a.length === 0 ? "" : ",";
                                a += "\"" + c.toString().replaceAll("\"", "\\\"") + "\"";
                            }
                        }
                        str += str.length === 0 ? a : "\r\n" + a;
                    }
                    a = "";
                    for(c in o){
                        if(o.hasOwnProperty(c) && row[o[c]] !== undefined){
                            a += a.length === 0 ? "" : ",";
                            a += "\"" + row[o[c]].toString().replaceAll("\"", "\\\"") + "\"";
                        }
                    }
                    str += str.length === 0 ? a : "\r\n" + a;
                }else{
                    throw "row is not an Array or object";
                }
            }
        }
        return str;
    }
    throw "Object is not an Array";
};

String.prototype.replaceAll = function(a, b){
var t = this, o = [], i;
//get all the occurances of it
for(i = 0; i < t.length; i+=1){
    if(t.substr(i, i - 1 + a.length) == a){
        o.push(i);
    }
}
for(i = 0; i < o.length; i+=1){
    t = t.substr(0, o[i] - 1) + b + t.substr(o[i] + b.length, t.length - 1);
}
return t;
};

Is the concept right? Do I cover everything when it comes to creating a CSV string? Any other comments?

Is there any way I can improve this or improve the performance? It seems to be rather quick when running, but I'm not sure what to base it on either.

Examples

Given I have required the file and it is names csv

csv.objectToCSVString({a:"a1",b:"b1"});

will output:

"a", "b"
"a1", "b1"

csv.objectToCSVString([{a:"a1",b:"b1"},{a:"a2",b:"b2"}]);

will output:

"a","b"
"a1","b1"
"a2","b2"

csv.objectToCSVString([["a", "b"], ["a1", "b1"], ["a2", "b2"]]);

will output:

"a","b"
"a1","b1"
"a2","b2"

csv.objectToCSVString({a:"a\"1\"",b:"b\"1\""});

will output:

"a","b"
"a\"1\"","b\"1\""

NOTE I am working on the replace all function, it doesn't work as it should yet

There's a few points you can improve on:

throw:

throw "Object is not an Array";

You should use throw new Error instead of throw as throw new Error contains a stack trace as well.

for in:

for in is a bit weird and can cause issues as it iterates over prototypes as well.

Considering you're testing that the properties are array items anyway, you can just use forEach instead.

if(ob instanceof Array){
    for(r in ob){
        if(ob.hasOwnProperty(r)){
            row = ob[r];

into:

if(ob instanceof Array){
    ob.forEach(function(row, index){
        //...

Declaring globals:

You shouldn't be declaring globals like:

var str = "", row, a, i, o, c, r;

Use them at their respective levels, don't have left over variables.


alternating Fibonacci and prime series The task is to print the following series

1 2 1 3 2 5 3 7...

The elements at odd positions are Fibonacci series terms and the elements at even positions are prime numbers. Given an input 'n' the element at the nth position in the series has to be printed.
eg. when n = 4, output will be 3. n = 7, output will be 3

I've tried to solve the problem by returning nth prime number or nth fibonacci term. I am looking for any improvements that can be  made to the code to optimize it further.

#include <bits/stdc++.h>
using namespace std;

int retPrime(int n)
{
    //Using sieve of Eratosthenes to generate primes
    int size = n + 1;
    bool Primes[100];
    int count = 0;
    memset(Primes, true, sizeof(Primes));
    for (int i = 2; i<sqrt(100); ++i)
    {
        if (Primes[i] == true)
        {
            for (int j = i * 2; j <= 100; j = j + i)
            {
                Primes[j] = false;
            }
        }
    }
    int primeIndex=0;
    int i = 2;
    while (count != n)
    {
        if (Primes[i] == true)
        {
            count++;
            primeIndex = i;
        }
        ++i;
    }
    return primeIndex;
}

int retFib(int n)
{
    if(n<=1){
        return n;
    }
    return retFib(n-1)+retFib(n-2);
}
int main()
{
    int n;
    cin >> n;
    if(n%2==0)
        cout << retPrime(n/2)<<" ";
    else
        cout << retFib((n/2)+1)<<" ";
    return 0;
}

<bits/stdc++.h> is non-standard and likely far more than you actually need. Unless coupled with use of precompiled headers, it will slow down compilation at least. Replace it with the standard includes. See "How does #include <bits/stdc++.h> work in C++?".
Never import wholesale any namespace which isn't designed for it. Doing so leads to conflicts, silent changes of behaviour, and generally brittle code. See "Why is “using namespace std” considered bad practice?".
Crank up the warning-level for your compiler. You will see it complain that size in retPrime() is unused.
There are only two good reason to give a variable a bigger scope than needed: Restricting the scope would be more verbose, or constructing it anew repeatedly would be more expensive. Otherwise, it's needless extra-stress put on each reader to understand things.
Avoid magic numbers. Instead, use some well-named constant, or simply eliminate them.
std::sqrt() may be required by the IEEE Standard to be exact. But does your implementation guarantee IEEE-conformance? Anyway, why not start with the square-roots?
Try to use fewer variables. Instead of incrementing a new variable up to a target, decrement the target if you don't need it any longer.
If you detect an error, like running beyond the bounds of your sieve, throw an exception, return an obviously impossible value (0 is a good candidate), or abort the program. But don't return a value which looks legitimate but is wrong.
Take a bit more care with formatting. An empty line before each function is a good idea, but please neither more (before the first) nor less (before the last).
retFib() and retPrime() are curious names. getFibonacci() and getPrime() seem better.
You really should add a function for your own series, abstracting the details away.
Comparing booleans against true or false is just pointless verbosity. Use the value directly, or after Negation with !.
There is no guarantee that a bool is a byte big. Nor that memsetting every byte to 1 will result in a valid bool, let alone that it's true. Use std::fill, or reverse the logic and use aggregate-initialization to start with all-false instead.

Even better, consider that the only even prime is 2, and change the code accordingly.
Calculating fibonacci naively needs \$O(n)\$ space and \$O(2^n)\$ time. Consider being slightly more clever and calculate it iteratively in \$O(1)\$ space and \$O(n)\$ time.
The conditional operator condition ? true_exp : false_exp is excellent for selecting one of two expressions. Use it where appropriate.
You should end each line of output with \n, it's expected. A single space instead is surprising.
If you want to stream a single character, use a character-literal, not a length-one string-literal. It might be slightly more efficient.
return 0; is implicit for main().
Use noexcept where appropriate.
Don't make things external for no reason. It increases the chance for collisions, and decreases the chance for inlining.
Input can fail. Deal with it.
As a final point, it might be better to just pre-compute the full sequence and be done with it. That might even save space.

Modified code:

#include <iostream>

static int getPrime(int n) noexcept {
    //Using sieve of Eratosthenes to generate primes
    constexpr auto sqrt_size = 10;
    constexpr auto size = sqrt_size * sqrt_size;
    if (!n) return 2;
    bool prime[size] = {false, false, true};
    for (int i = 3; i < size; i += 2)
        prime[i] = true;
    for (int i = 3; i < sqrt_size; i += 2) {
        if (!prime[i])
            continue;
        if (!--n)
            return i;
        for (int j = i + i; j < size; j += i)
            prime[i] = false;
    }
    for (int i = (sqrt_size & ~1) + 1; i < size; i += 2)
        if (prime[i] && !--n)
            return i;
    return 0;
}

static int getFibonacci(int n) noexcept {
    int last = 0, r = 1;
    while (n-- > 0) {
        int temp = last + r;
        last = r;
        r = temp;
    }
    return r;
}

static int mySeries(int n) noexcept {
    return n % 2 ? getFibonacci(n / 2 + 1) : getPrime(n / 2);
}

int main() {
    int n;
    if (std::cin >> n)
        std::cout << mySeries(n) << '\n';
    else
        std::cerr << "Could not understand your input. Expected a number.\n";
}


Writing pure functions but have a chain I'm trying to play around with Godot to learn C# and I was trying to creating pure functions to do the velocity, but I found I was repeating myself.

    public override void _PhysicsProcess(float delta)
    {
        base._PhysicsProcess(delta);
        Vector2 moveVelocity = velocity;

        moveVelocity = Run(moveVelocity);
        moveVelocity = Jump(moveVelocity);
        moveVelocity = Dash(moveVelocity);
        moveVelocity = Friction(moveVelocity);
        moveVelocity = Gravity(moveVelocity);

        Animate();

        velocity = MoveAndSlide(moveVelocity, Vector2.Up);

    }

Is there a nicer way of doing the above method so I don't copy moveVelocity over and over but not change velocity or create a new class property?

Something like

velocity = Apply(velocity,
  Run,
  Jump,
  Dash,
  Friction,
  Gravity);

Not sure about your full implementation, but from what I see, you could do a fluent API
something like :

public class PhysicsProcess
{
    private readonly float _delta;
    private readonly Vector2 _velocity;

    public PhysicsProcess(Vector2 velocity)
    {
        _velocity = velocity;
    }

    public PhysicsProcess Run()
    {
        // Run logic which would be saved on the global variable
        return this;
    }

    public PhysicsProcess Jump() { ... }

    public PhysicsProcess Dash() { ... }

    public PhysicsProcess Friction() { ... }

    public PhysicsProcess Gravity() { ... }

    public void Apply()
    {
        // Your final logic which would take all arguments, and do the process you need.
    }

}

then your usage would be something like :

var process = new PhysicsProcess(velocity)
.Run()
.Jump()
.Dash()
.Friction()
.Gravity()
.Apply();

You can then extend or customize each process as fits your needs. You can also make PhysicsProcess as nested class and call it from a method from the parent class. There are many approaches and customization can be done with fluent APIs, just choose the approach that you see it would be more reasonable to the existing logic (technically, and business-wise).

UPDATE

As I mentioned, fluent API is an easy interface to use and customize. For that, you can also make it an extension like this :

public static class Vector2Extensions
{
    public static PhysicsProcess Apply(this Vector2 velocity)
    {
        return new PhysicsProcess(velocity);
    }
}

now you can for instance add new method to your fluent API class say Save() for instance, which would return Vector2, then you could do something like this :

moveVelocity.Apply()
    .Run()
    .Jump()
    .Dash()
    .Friction()
    .Gravity()
    .Save();

When you use this approach, it would make it more maintainable and easy to extend.

I hope this would make it more useful answer.


Mapping & Sorting Randomly Generated Numbers Below is the assignment I was given and the coding is what I have came up with. I would appreciate criticism on how I did and on ways to have gone differently about it.

  You want to number your random list numbers, so use an iterator to put the list numbers into a map (but keep the list).  The key will be the number the random number was generated in starting at 1 (ex. 1, 2, 3, etc., since these are not just index numbers).  The value will be the random number.  Use an iterator to print the two fields lined up on the right with field widths in two columns with the numbers labeled with centered column headings of Key and Value.

  Since the list is easier to work with, lets work with the list to perform some calculations.  You will sort the numbers in your list. Then use iterators, algorithms, and list functions (not the user's input variable) to find the sum, mean (average), median (middle value - find the size and move up half way), range (difference between highest and lowest), and a random number from a random spot from your list of random number (get a random number (in the proper range) with  rand() and move up that many spots in your list).  Then print the labeled values (with the mean to three decimal places).

  Now, use your iterator to put the now sorted list numbers back into the map (which will change the original map's order).  The key will still be numbers starting at 1.  The value will now be the random number in their sorted order.  Again, use an iterator to print the two fields lined up on the right with field widths in two columns with the numbers labeled with column headings of Key and Value.

#include <bits/stdc++.h>
using namespace std;

int main() {

    cout << "Enter the length of list: ";
    int n;
    cin >> n;

    std::vector<int> v(n);

    cout << "The random generated numbers are: \n";

    for (int i = 0; i < n; i++) {
        int x = rand() % 151;
        cout << std::setw(10);
        cout << i << " ";
        cout << std::setw(10);
        cout << x << endl;
        v[i] = x;
    }
    int sum = 0;
    for (int i = 0; i < n; i++)
        sum += v[i];

    cout << "The sum: " << sum << endl;
    cout << "The mean: " << sum / n*1.0 << endl;

    sort(v.begin(), v.end());
    cout << "The median: " << v[n / 2] << endl;

    return 0;
}

Include the right things

You're starting with:

#include <bits/stdc++.h>
using namespace std;

The second line is bad practice; avoid it. Typing std:: is not a burden. The former is definitely unacceptable - that's a compiler-specific file that you don't want to be including. Just include the actual headers you need:

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

Use algorithms and iterators

Rather than writing your own loop to perform a sum, you could use the standard algorithm for this: std::accumulate:

int sum = std::accumulate(v.begin(), v.end(), 0);

Also they wanted you to use iterators to find the median, which would be:

auto median_it = std::next(
    v.begin(),
    std::distance(v.begin(), v.end()) / 2);

Where's the rest?

There's a lot of text in that question that I don't think you addressed in your code. Looks like we need a std::map or something.


Function for checking if a string is a path or an URL be improved I made myself a function to check if a string is a path or an URL:
import os

def isPath(s):
    """
    @param s string containing a path or url
    @return True if it's a path, False if it's an url'
    """
    if os.path.exists(s): # if a file with name s exists, we don't check any further and just return True
        return True
    elif s.startswith("/"): # clearly a path, urls never start with a slash
        return True
    elif "://" in s.split(".")[0]: # if a protocol is present, it's an url
        return False
    elif "localhost" in s: # special case for localhost domain name where splits on . would fail
        return False
    elif len(s.split("/")[0].split(".")) > 1: # dots before the first slash, normally separating TLD and domain name
        return False
    elif len(s.split("/")[0].split(":")) > 1: # if colons are present, either it's a IPv6 adress or there is a port number
        return False
    else: # all possible cases of an url checked, so it must be a path
        return True

Did I miss any cases of an url / path, and can the function be improved in some way ?

Some high level comments:

Checking if a string is a url (or a path) is usually done using regular expressions that can be found online. Not sure what's the scope of this function, but it can be seen as reinventing the wheel and might miss some uncommon cases
The method name gives the impression that returning True means it's a path, and False means that it's not, where in fact False has the meaning of being a URL. I would split this function into two: isPath where False just means that the given string is not a Path, and isUrl where False means it's not a URL. Or at the very least renaming it to isPathOrUrl
It's a personal preference but I'd change elif to be if as all the conditional bodies are returning something so no need to use else


Optimize and accelerate bash script to transform XML to SQLITE I have this script in which I read an XML and I pass it to CSV and at the end of the script I transform it into SQLITE

#!/bin/bash
rm -f -r rshost rscname rsctype ttstamp tservice tformat trdata trdata2
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $9 }' | cut -c27-30 > trdata
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $3 }'| cut -c1-19 > ttstamp
a=`cat $1 | grep RecordStart | head -1 | sed -e 's/"/ /g'| awk '{ print $15 }'`
b=`cat $1 | grep RecordStart | head -1 | sed -e 's/"/ /g' | awk '{print $12 }' | sed -e 's/;/ /g' | sed  -e 's/=/ /g' | awk '{print $4 }'`
c=`cat $1 | grep RecordStart | head -1 | sed -e 's/"/ /g' | awk '{print $9,$10 }'`
touch rsctype rshost rscname
kk=`wc -l trdata | awk '{ print $1 }'`
for i in `seq 1 $kk`
do
    echo $a >> rsctype
    echo $b >> rshost
    echo $c >> rscname
done
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $5 }' > tservice
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $7 }' > tformat
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $9 }' | cut -c41-44 > trdata2
cat $1 | grep Telegram | sed -e 's/"/ /g' | awk '{ print $9 }' | cut -c31-34 > grhost

awk -Wposix '{printf("%d\n","0x" $1)}' trdata > trdata3
awk -Wposix '{printf("%d\n","0x" $1)}' trdata2 > trdata4

sed -i "s%^0%0/%g" grhost
cat grhost | cut -c1-3 > grhost2
sed -i "s%.\{4\}%/%g" grhost
pr -mts, grhost2 grhost > grhostfinal
sed -i "s/,//g" grhostfinal
cat grhostfinal | cut -c1-4 > grhostfinal1
cat grhostfinal | cut -c5 > grhostfinal2
awk -Wposix '{printf("%d\n","0x" $1)}' grhostfinal2 > grhostfinal3
pr -mts, grhostfinal1 grhostfinal3 > grhostfinal4
sed -i "s/,//g" grhostfinal4

pr -mts, rshost ttstamp rsctype tservice tformat trdata4 trdata3 rscname grhostfinal4 > conjunto.csv
sed -i "s|^|,|g" conjunto.csv

sqlite3 test2.sqlite "select fecha from testxml4;" > data.csv

cat data.csv | sort | uniq > data2.csv

for k in `cat data2.csv`
do
    grep "$k" conjunto.csv >> quitar
done

diff quitar conjunto.csv | grep ">" | sed 's/^> //g' > diferencia.csv

echo `sqlite3 test2.sqlite < testxml`
python csv2sqlite.py diferencia.csv test2.sqlite testxml4

rm -f -r rshost rscname rsctype ttstamp tservice tformat trdata trdata2 trdata3 trdata4 grhost2 grhost grhostfinal3 grhostfinal1 grhostfinal2 grhostfinal grhostfinal4 a b c data.csv conjunto.csv data2.csv quitar

I have this XML (The data is a private)

 <CommunicationLog xmlns="http://knx.org/xml/telegrams/01">
  <RecordStart Timestamp="" Mode="" Host="" ConnectionName="" ConnectionOptions="" ConnectorType="" MediumType="" />
  <Telegram Timestamp="" Service="" FrameFormat="" RawData="" />
  <Telegram Timestamp="" Service="" FrameFormat="" RawData="" />
  <RecordStop Timestamp="" />
  <RecordStart Timestamp="" Mode="" Host="" ConnectionName="" ConnectionOptions="" ConnectorType="" MediumType="" />
  <Telegram Timestamp="" Service="" FrameFormat="" RawData="" />
  <Telegram Timestamp="" Service="" FrameFormat="" RawData="" />
  <RecordStop Timestamp="" />
</CommunicationLog>

Once analyzed the data, I take them to a CSV and with the Python program csv2sqlite.py

python csv2sqlite.py CSVFILE.csv DB.sqlite TABLESQLITE

My question is how can I make this script faster and more efficient, since it takes a long time to analyze all the data.

Take-home message: pipelines in Bash are slow (compared to process substitution); loops in Bash are slow; Bash are slow.

Without a setup script and dummy data to test run it, I don't really understand exactly what your script is trying to achieve at each step, so I can only suggest the following improvements:

Avoid unnecessary pipelines

A lot of your code does cat $1 | grep Telegram | sed -e 's/"/ /g', which can be simplified to sed '/Telegram/!d; s/"/ /g' "$1", so you may want to save the result somewhere and extract from it when needed.
awk '{ print $9 }' | cut -c27-30 can be combined into awk '{print substr($9, 27, 4)}'.
In the command substitution that gets assigned to b, you have sed -e 's/;/ /g' | sed  -e 's/=/ /g' which should really just be sed -e 's/;/ /g' -e 's/=/ /g' or even better just sed 's/[;=]/ /g'. You don't need the -e option if you're not combining expressions.
kk=`wc -l trdata | awk '{ print $1 }'`: kk=`wc -l < trdata` can do the job just fine.
cat grhostfinal | cut -c1-4 > grhostfinal1 is not as efficient as cut -c1-4 < grhostfinal > grhostfinal1
sqlite3 test2.sqlite "select fecha from testxml4;" > data.csv

cat data.csv | sort | uniq > data2.csv

is probably best done entirely in SQL:

sqlite3 test2.sqlite "SELECT DISTINCT fecha FROM testxml4 ORDER BY fecha;" > data.csv

Avoid loops if possible, or optimize for each iteration of the loop

for i in `seq 1 $kk`
do
    echo $a >> rsctype
done

is much slower than

printf "$a\n%.0s" `seq 1 $kk` >> rsctype

for small $kk, and much slower than

yes "$a" | head -n "$kk" >> rsctype

for large $kk. See https://superuser.com/questions/86340/linux-command-to-repeat-a-string-n-times. (Not sure if you actually want to repeat the same strings, but that's what your code does.)
for k in `cat data2.csv`
do
    grep "$k" conjunto.csv >> quitar
done

diff quitar conjunto.csv | grep ">" | sed 's/^> //g' > diferencia.csv

looks like it could be done with just one diff? Maybe (haven't tested it):

diff --changed-group-format='%>' --unchanged-group-format='' data2.csv conjunto.csv\
 > diferencia.csv

Non-performance-related notes

You should wrap your cleanup command in a trap and put it at the beginning of the script so that it will always be executed unless the program is terminated by a SIGKILL:

trap 'rm -f \
rs{host,cname,ctype} \
ttstamp tservice tformat trdata{,2,3,4} \
grhost{,2,final{,{1..4}}} \
data{,2}.csv conjunto.csv \
{a..c} quitar' \
'EXIT'

Please don't rm -r if you're not deleting directories. This is a dangerous command if you're not careful. I used brace expansion to shorten the list of input files I need to type, but I'm not sure if you really need to create that many temporary files.
You don't need to touch the files. Redirection will create the files if they don't exist.

I can probably offer more suggestions if I could try out the script. I'll add to this answer if I think of anything, but this should be enough for now.


Using jQuery and AJAX process a form with PHP I'm looking to collect form input and pass it to a file called processing.php with AJAX

JS:

$(document).ready(function () {

    var serializedSubmission;
    var responseObj;

    $('#myForm').on('submit', function (e) {
        e.preventDefault();
        serializedSubmission = $('#myForm').serialize(); // get input
        if (checkInput()) { // validate
            process();  // append 'action' and process data
        }

    });

    /* data processing functions */
    // verify all parameters have values
    function checkInput() {
        var temp = decodeURIComponent(serializedSubmission);
        if (temp.search('=&') > 0 || temp.slice(-1) == '=') {
            console.log('Form is missing parameters!');
            return false;
        } else {
            return true;
        }
    }

    // append 'action' and pass data to php
    function process() {
        var data = {'action': 'demo'}; // set action to correspond with processing.php
        data = serializedSubmission + "&" + $.param(data);
        $.ajax({
            type: 'POST',
            dataType: 'json',
            url: 'php/processing.php',
            data: data,
            success: function (data) {
                responseObj = JSON.parse(data['json']);
                console.log('ajax success:');
                console.log(responseObj);
            },
            error: function (xhr, desc, err) {
                console.log(xhr);
                console.log('Details: ' + desc + '\nError: ' + err);
            }
        });
    }
});

PHP:

<?php
if(isAjax()) {
    if(isset($_POST['action']) && !empty($_POST['action'])) { // check if action value exists
        $action = $_POST['action'];
        switch($action) {
            case 'demo':
                demo();
                break;
        }
    }
}

// check if ajax request
function isAjax() {
    return isset($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower(
        $_SERVER['HTTP_X_REQUESTED_WITH']
    ) == 'xmlhttprequest';
}

// action = 'demo'
function demo() {
    $return = $_POST;
    $return['json'] = json_encode($return);
    echo json_encode($return);
}

// action = 'widget'
function widget() {
    // do stuff
    echo json_encode($return);
}

How's it look?

On client-side, some small points ...

Avoid more $(selector) phrases than absolutely necessary. Inside the submit handler, you can use $(this), which will be faster.
Pass serializedSubmission formally to the subroutines rather than rely on an outer var.
The outer var responseObjcan't be reliably used except in an asynchronous context, so if you want to cache anything, cache the promise returned by $.ajax(), not the data.
Needs less obvious comments and more on things that matter, eg against lines like if (temp.search('=&') > 0 || temp.slice(-1) == '='), which I had to study to work out the validation strategy. Maybe a few words would have helped me.


Ownership/Lifetime management issue I have a small class of 2D drawing utilities, Draw2D which is the global entry point for anything related to drawing. Currently, the class looks like this:

struct Draw2D
{
    std::shared_ptr<D2Label> CreateLabel (...);
    std::shared_ptr<D2Plot> CreatePlot (...);

    void Draw (D2Element& e);

    std::vector<std::weak_ptr<D2Element>> elements_;
};

struct D2Element
{ ... };

struct D2Label : D2Element
{ ... };

So far so good. The key point here is that Draw2D needs to know all it's elements so it can update them and for drawing, each element has to call back to its parent Draw2D instance. I.e. you cannot move elements between instances as all elements share resources provided by the parent. The solution I have right now is weak_ptr which get checked on each update of the Draw2D instance and if stale, the object is removed (meaning the user has deleted it in the meantime.)

The problem is that after the Draw2D instance died, of course all still existing instances are dead, too. Even though they are technically valid objects, you cannot use them safely any more. Even queries could fail if they touch the shared resources, and keeping the shared resource alive via shared_ptr does not help either because that just postpones the problems until the drawing backend dies (in which case all resources are invalidated.)

Is there some way to rewrite this code to make it clearer to the clients? I tried something like this, as I definitely want clients to be able to release resources early (i.e. before Draw2D dies.)

struct D2Element
{
    ~D2Element ()
    {
        deleter_ (this);
    }

    std::function<void OnDelete (D2Element*)> deleter_;
}

struct Draw2D
{
    void Release (D2Element* element)
    {
        // Remove from list of registered elements
    }

    ~Draw2D ()
    {
        // delete all remaining children
    }

    D2Label* CreateLabel (...)
    {
        auto label = ...;
        label->deleter_ = std::bind (&Draw2D::Release, this, _1);
        return label;
    }
}

This is ok but now the clients have to wrap it into smart pointers on their own by default.

Why not separate the 2D element into two parts.

The interface that can be used
The shared resource

Used by the Draw2D interface
Used by the D2Elements.

Then D2Elements does not need to depend on an object that can go out of scope, they have partial ownership of the shared resource (with any common data that they and the Draw2D needs.

class Draw2D
{
    // interface as before
    // Weak pointers as before.

    std::shared_ptr<SharedResource>   data;
};

class D2Element
{
    //... As before.

    std::shared_ptr<SharedResource>   sharedData;
};

Even of the Draw2D's lifetime ends, then the shared resource (because it is shared with any created children) will not die and thus not be destroyed.


Removing multiple slashes from path I have been trying to write a sample code block to remove multiple backward or forward slashes from a path (e.g. //a//b should be /a/b; on Windows, c:\\a\\b\\\c should be c:\a\b\c).

I am assuming that:

Code must be platform independent
Should work in all cases (or report a proper error)
It should be readable and maintainable
It shouldn't be over-complex and shouldn't have errors

Please provide point-wise comments for the following code sample (you may ignore main and assume that path is not NULL - it is just for demonstration purposes). Or else you may provide a better mechanism for the same.

    void
    die( int error_code, char* message )
    {
        fprintf( stderr, message );
        exit( error_code );
    }

    /*Following function removes duplicate slashes from a given path*/
    char*
    validate_path ( char* path )
    {
        char* path_copy = path;
        int dup_flag = 0;
        char* path_without_dup_slash = (char*)malloc(strlen(path)+1);
        if ( path_without_dup_slash == NULL ) {
             die( EXIT_FAILURE, "Failed to allocate memory for processing path.");
        }
        char* copy_pwds = path_without_dup_slash;
        /* Travel through given path */
        while( *path_copy != '\0' ) {
              /* If there is '\' or '/' then copy a single '/'
               * , ignore others until a real char comes */
               if ( (( *path_copy == '\\') || ( *path_copy == '/' )) &&
                    (dup_flag == 0) ) {
                    *copy_pwds = *path_copy;
                    dup_flag = 1;
                    copy_pwds++;
                }else if (( *path_copy != '\\') && ( *path_copy != '/' ))  {
                    *copy_pwds = *path_copy;
                    dup_flag = 0;
                    copy_pwds++;
                }
                path_copy++;
         }
         *copy_pwds = '\0';
         path_without_dup_slash = (char *)realloc( path_without_dup_slash,
                                  strlen( path_without_dup_slash ) + 1 );
         if ( path_without_dup_slash == NULL ) {
               die( EXIT_FAILURE, "Failed to allocate memory for processing path.");
         }
         return path_without_dup_slash;
    }
    /*You please ignore the parts written in main - it is just for demonstration purpose. */
    int
    main( void ) {
         char* bad_path_1 = malloc(100);
         char* bad_path_2 = malloc(100);
         char* p1 = bad_path_1;
         char* p2 = bad_path_2;
         char* path_1;
         char* path_2;
         bad_path_1 = "//a//b//c";
         bad_path_2 = "c:\\\\a\\\\b\\\\c";
         path_1 = validate_path(bad_path_1);
         printf( "Good path:%s Bad Path:%s\n",path_1,bad_path_1);
         path_2 = validate_path(bad_path_2);
         printf( "Good path:%s Bad Path:%s\n",path_2,bad_path_2);
         free(path_1);
         free(path_2);
         free(p1);
         free(p2);
         return EXIT_SUCCESS;

     }

Here are some suggestions for how you might improve your code.

Use the appropriate #includes

This program fragment requires headers, which should be included:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

Use const where practical

Because the validate_path function does not alter the passed string, the parameter should be declared const:

char *validate_path ( const char* path )

Don't make pointless copies of variables

When you pass a parameter to a function, it's a copy, so there's no reason for this line:

char* path_copy = path;

Just use path directly.

Check for NULL before dereferencing

The first thing the validate_path does is to pass path to strlen, but this results in undefined behavior if path isn't a '\0'-terminated string.  A simple safety check would be to check to see if it's a NULL pointer and exit if it is.

Consider a change to the interface

Instead of allocating memory within the function, consider instead altering the function so that it is passed an input and output buffer.  This way, the code would not need to allocate memory and the caller would do both allocation and freeing.  That makes for a more robust interface and it makes your code simpler.

Prefer for to while

The while loop within the code would likely be more succinctly expressed as a for loop.  That way it's easier to see what is being iterated through and how.

Use a more accurate function name

I would expect that validate_path would do just that and validate a path, but that's not really what's happening here.  Instead, it seems that a more accurate function name would be something like remove_dup_slashes.

Simplify your code

Here's an alternative implementation that uses these suggestions:

char *validate_path(const char *path, char *curr) {
    if (path == NULL || curr == NULL) {
        return NULL;
    }
    char *begin = curr;
    // always copy first character
    *curr = *path;
    // if it's just a single `NUL`, we're done
    if (*curr == '\0') {
        return curr;
    }
    for (path++; *path; ++path) {
        // path points to next char to read
        // curr points to last written
        if (*curr != *path || (*curr != '\\' && *curr != '/')) {
            // only copy if not duplicate slash
            *(++curr) = *path;
        }
    }
    *curr = '\0';  // terminate string
    return begin;
}

Note that the interface is still not a very good one.  It could be improved by also passing in the length of the output buffer and maybe the length of the input buffer as well.


Dynamic Accordion in WordPress I used the Bootstrap v4.0.0 accordion to display my business services. Further, I created a custom post type "service" for my services (Each service is a post).
To display my services dynamically and not hardcode them, I implemented a for-loop within the WordPress while-loop which uses an enumeration to provide the number of the collapsible.
I don't like my solution, especially the enumeration - Is there a better way to solve this?
If you see any other improvement, I would be glad to hear also.
See my code below:

All Services

                        <?php $wv_services = new WP_Query( array(
                            'posts_per_page' => -1,
                            'post_type'      => 'service',
                            'orderby'        => 'date',
                            'order'          => 'ASC'
                        ) );

                        while ( $wv_services->have_posts() ) {

                            $enum_of_collapsible_str_numbers = [
                                "One",
                                "Two",
                                "Three",
                                "Four",
                                "Five",
                                "Six",
                                "Seven",
                                "Eight",
                                "Nine",
                                "Ten",
                                "Eleven",
                                "Twelve",
                                "Thirtenn",
                                "Fourteen",
                                "Fivteen",
                                "Sixteen",
                                "Seventeen",
                                "Eighteen",
                                "Nineteen",
                                "Twenty"
                            ];
                            $total_number_of_service_posts = wp_count_posts('service')->publish;
                            for ($collapsible_str_number = 0; $collapsible_str_number < $total_number_of_service_posts; $collapsible_str_number++) {

                                $wv_services->the_post(); ?>

                                <div class="card-header collapsed" style="cursor:pointer;" data-toggle="collapse"
                                     href="#collapse<?php echo $enum_of_collapsible_str_numbers[$collapsible_str_number] ?>" rel=”nofollow”>

<?php the_title(); ?>

                                <div id="collapse<?php echo $enum_of_collapsible_str_numbers[$collapsible_str_number] ?>" class="card-body collapse"
                                     data-parent="#accordion">

<?php the_content(); ?>

                            <?php }
                        }
                        wp_reset_postdata();
                        ?>

Not sure what you are trying to achieve, but you can use numerical values for enum_of_collapsible_numbers and you can generate them like this
$enum_of_collapsible_numbers = range(1, 20);


Library with Optional traits I am implementing a library in C/C++11 and I have chosen to follow Google C++ Style Guide as I also use this style guide in my code.

As such all classes and types (including typedefs and using) start with a capital letter.

However I have reached a conflict when implementing some traits.

For instance I have a RemoveOptional trait:

template <class T>
struct RemoveOptional {
  using Type = T;
};

template <class T>
struct RemoveOptional<utils::Optional<T>> {
  using Type = T;
};

that I can use like this:

utils::RemoveOptional<utils::Optinal<int>>::Type

But for the traits like IsXXX I have chosen to inherit std::true_type or std::false_type in order to be used like the std traits:

template <class T>
struct IsOptional : std::false_type {
};

template <class T>
struct IsOptional<Optional<T>> : std::true_type {
};

so this trait is used like this:

utils::IsOptional<utils::Optional<int>>::value
utils::IsOptional<utils::Optional<int>>::value_type

And value_type conflicts with my naming conventions as it should be ValueType.

As far as I see I have 3 options:

Modify all of my traits to follow the std convention.

Advantages: my traits can be checked against the std::true_type and std::false_type.
Disadvantages: my library is inconsistent.

Discard the std::true_type and std::false_type and make all my traits following my convention.

Advantages: my library is consistent.
Disadvantages: my traits can't be checked against std::true_type and std::false_type.

Leave as it is now.

Advantages: my traits can be checked against std::true_type and std::false_type.
Disadvantages: my library is inconsistent (even my traits are inconsistent between each other).

What should I do?

There is no single correct answer in all circumstances. You have to weigh the costs and benefits of the alternatives. Oddly enough my analysis seems to match yours, but here's one additional alternative you didn't explicitly cover:

Add redundant typedefs. All types you create will follow your standard, and those from the standard library will not. However you can easily provide aliases that expose parts of the standard library using names of your choosing. This is a little more code than the first option, and provides an opportunity for users of your class to use either name they like. If more than one name is used, the readability of the code can suffer. As such, while this initially may look like it combines the best of both extremes, in reality it also offers the worst.

template <class T>
struct IsOptional : std::false_type {
    using Type = std::false_type::value_type;
    using Value = std::false_type::value;
};

template <class T>
struct IsOptional<Optional<T>> : std::true_type {
    using Type = std::true_type::value_type;
    using Value = std::true_type::value;
};

However, once you're done writing the library, I would posit this is not all that important. I do not routinely use std::true_type or std::false_type when consuming a library. Template specialization is typically used to achieve two objectives at the same time:

Make the classes easy to use in flexible ways
Make the code that uses the classes really fast

Neither of these objectives really involve the consumers of the library having to care about the names of the types used in specializing its templates.

And thus my answer is this: don't sweat it. Do what makes you, and anyone else who is helping to implement your library, happy. This probably means choose the least amount of work, leaving you with mismatched conventions, but the ability to leverage knowledge of working with the standard library, but could mean writing your own TrueType and FalseType classes that follow your chosen naming conventions.


Implement step function in Haskell I try to implement a (restricted version of) step function in Haskell. A step functions is a function that is constant on a finite number of half intervals, and mempty everywhere else. (equivalently, one can fill those "everywhere else" by half intervals)

This can be modeled as a StepData a b, which stores a list of the half intervals and the associated value for that half interval.
Not all Ord a has a minimum or maximum, I lift it to Bound a, which guaranties it to have a minimum and maximum, this is to make the algorithm clearer.

I implemented eval :: StepData a b -> a -> b to evaluate a step function at a point.

The important part is the ability to make step function a monoid, where <> is defined as pointwise sum of the function. Currently I implemented <> for the StepData a b.

P.S. Whenever I want to find a value, I have to run eval f x. Of course I can define g = eval f, but I can't use <> on the derived function. So I have to pass the data around in order to combine functions, and only call eval when I need to find a value. Are there better ways to handle this?

{-# LANGUAGE NoMonomorphismRestriction #-}

import Data.List
import Data.Monoid

data Bound a = Minimum | Value a | Maximum  deriving (Eq, Ord, Show)

data StepData x y = StepData [(Bound x, Bound x, y)]
    deriving (Show, Eq, Ord)

instance (Ord x, Monoid y) => Monoid (StepData x y) where
  mempty = StepData [(Minimum, Maximum, mempty)]
  mappend (StepData a) (StepData b) = StepData (foldl insertInterval b a)
    where
      insertInterval [] _ = []
      insertInterval ((a',b',y'):xs) (a,b,y)
       | a >= b' = non [(a',b',y')] ++ insertInterval xs (a,b,y)
       | b >= b' = non [(a',a,y'),(a,b',y <> y')] ++ insertInterval xs (b',b,y)
       | b <  b' = non [(a',a,y'),(a,b, y <> y'),(b,b',y')] ++ xs
       where non = filter (\(a,b,_)-> a/=b)

      merge (h@(a,_,y):h'@(_,b',y'):xs)
       | y == y'   = merge ((a,b',y):xs)
       | otherwise = h:merge (h':xs)
      merge x = x

eval (StepData xs) t = y
  where (_,_,y) = head $ dropWhile sol xs
        sol (a,b,y)
         | a<=Value t && Value t<b = False
         | otherwise   = True

fromList xs = StepData (map (\(a,b,y)->(Value a, Value b, y)) xs) `mappend` mempty

I might be a little bit late to the party, but better late than never, right?
Type signatures
Your central functions fromList and eval don't have type signatures. This forces the user to check Value and StepData's definition. Better add them:
eval :: Ord a => StepData a
eval = ...

fromList :: (Ord a, Monoid b) => [(a, a, b)]-> StepData a b
fromList  = ...

Remove dead code
merge isn't used in your code. It's dead code and not used in your instance at all. Better remove it.
Make sure the code compiles
That wasn't an issue back in 2013, but nowadays Semigroup is a superclass of Monoid, and you need to implement it too.
Don't encode Bool twice
eval's sol can be simplified a lot if we just use not around the condition:
eval (StepData xs) t = y
  where (_,_,y) = head $ dropWhile sol xs
        sol (a,b,_) = not (a <= Value t && Value t < b)

The function also gets easier to understand if we use filter instead, as we don't need to deal with double negation (drop and not):
eval (StepData xs) t = y
  where (_,_,y) = head $ filter sol xs
        sol (a,b,_) = a <= Value t && Value t < b

Document requirements of data
fromList needs a proper sorted list. That's not documented anywhere and neither enforced in its type nor its logic. We might end up with StepData [(Value 3, Value 1, Maybe 3)] or StepData [(Value 3, Value 4, Maybe 3),(Value 1, Value 2, Maybe 3)], as the list is only maped.
Instead, use foldMap and make sure that the values are ordered properly:
fromList = foldMap go
  where
    go (a, b, y)
      | a < b     = StepData [(Value a, Value b, y)]
      | otherwise = StepData [(Value b, Value a, y)]


Simple anagram game with names of colors This is a very simple anagram game that plays only with names of colors.

I tried to focus on code design.  I also wanted it to support further updates.  But, my init seems smelly and you will notice it, especially in the main loop.

I was wondering about my naming and commenting/docstrings.  I need strong criticism here, without any mercy.

This is the module I've created with the only class:

class AnagramGame:

    def __init__(self, words=None, rword=None, inp=None,
                 anagram=None, is_corr_word=False):
        self._words = None
        self._rword = rword # Stands for random word
        self._anagram = anagram
        self._inp = inp # User input
        self._is_corr_word = is_corr_word # Stands for correct word bool

    def get_words(self):
        return self._words

    def set_words(self, filename):
        in_file = open(filename, 'r')
        self._words = [in_line.rstrip('\n')
                       for in_line in in_file.readlines()]
        in_file.close()

    def del_words(self):
        del self._words

    words = property(get_words, set_words, del_words)

    def get_rword(self):
        return self._rword

    def set_rword(self, sequence):
        import random
        self._rword = random.choice(sequence)

    def del_rword(self):
        del self._rword

    rword = property(get_rword, set_rword, del_rword)

    def get_anagram(self):
        return self._anagram

    def set_anagram(self, sequence):
        import random
        self._anagram = ''.join(random.sample(sequence, len(sequence)))

    def del_anagram(self):
        del self._anagram

    anagram = property(get_anagram, set_anagram, del_anagram)

    def get_is_corr_word(self):
        return self._is_corr_word

    def set_is_corr_word(self, boolean):
        self._is_corr_word = boolean

    def del_is_corr_word(self):
        del self._is_corr_word

    is_corr_word = property(get_is_corr_word, set_is_corr_word,
                            del_is_corr_word)

    def ask_input(self, prompt=''):
        self._inp = input(prompt)

    def check_input(self, prompt_corr, prompt_incorr):
        if self._inp == self.rword:
            print(prompt_corr, end='')
            self._is_corr_word = True
            return self._is_corr_word
        else:
            print(prompt_incorr, end='')
            self._is_corr_word = False
            return self._is_corr_word

And this is the main program:

def main():
    import games

    print("Anagram Mini Game", "\n")

    while True:
        Angrm = games.AnagramGame() # Reset all attributes
        Angrm.words = 'colors.txt' # Load words
        Angrm.rword = Angrm.words # Extract a word
        Angrm.anagram = Angrm.rword # Create anagram of the word

        print("Try build an anagram of this shuffled color name: ", Angrm.anagram)

        Angrm.ask_input("Give it a try - ")
        Angrm.check_input("GG beast", "Wrong - ") # Not indent after impact prompts
        while not Angrm.is_corr_word:
            Angrm.ask_input()
            Angrm.check_input("GG beast", "Wrong - ")

        print("\n\n") # Indent for a new game

if __name__ == '__main__':
    main()

In your __init__() function, you have defined words as a parameter but you are assigning None to _words

class AnagramGame:

def __init__(self, words=None, rword=None, inp=None,
             anagram=None, is_corr_word=False):
    self._words = None
    # ...

You may either remove this parameter or assign the value to _words

Instead of using an infinite loop while True:, you should have created a bool object. Something like this:

checker = False
while not checker:
    # ...

I see the purpose of while True: is to make the user play the game endlessly. It would have been great to have an exit criteria too. Else, you could have skipped the infinite loop.

You don't have to re-initialize the object and reload the file on each loop run. You may initialize them before the loop, load the files and let the game run on its own.

To reset the values, you may create another function that would only reset the relevant data-members and continue the game further.

As you have already mentioned, please have proper comments and doc-strings in the code. It is always better to add them during development. Else mostly they get ignored!


Comparing 2 iOS and Android versions Please review and let me know which version is better. I'm just trying to find any caveats or better ways to accomplish the a version comparison independent from Android and iOS.

Formats accepted: '1.0.0' or '1' formats and an operator, e.g: compareVersion('1', '1.2.0', '>')

export function compareVersion(version1, version2, operator) {
        const formattedV1 = version1.split(".");
        const formattedV2 = version2.split(".");

        let diff = 0;

        if (formattedV1.length !== formattedV2.length) {
            const lengthDiff = formattedV1.length - formattedV2.length;
            for (let index = 0; index < Math.abs(lengthDiff); index += 1) {
                if (lengthDiff > 0) {
                    formattedV2.push("0");
                } else {
                    formattedV1.push("0");
                }
            }
        }

        for (let index = 0; index < formattedV1.length; index += 1) {
            if (diff === 0) {
                const v1 = parseInt(formattedV1[index]);
                const v2 = parseInt(formattedV2[index]);
                if (isNaN(v1) || isNaN(v2)) {
                    throw new Error("Problem comparing versions: not a valid number");
                }
                if (v1 < v2) {
                    diff = -1;
                }
                if (v1 > v2) {
                    diff = 1;
                }
            }
        }

        switch (operator) {
        case "=":
        case "==":
            return diff === 0;
        case ">=":
            return diff >= 0;
        case "<=":
            return diff <= 0;
        case ">":
            return diff > 0;
        case "<":
            return diff < 0;

        default:
            throw new Error("Problem comparing versions");
        }
    }

OR

export function compareVersions(a, b, operator) {
  const aParts = a.split('.');
  const bParts = b.split('.');

  const pairs = [];

  for (let index = 0; index < Math.max(aParts.length, bParts.length); index += 1) {
    pairs.push({
      a: parseInt(aParts[index]),
      b: parseInt(bParts[index]),
    });
  }

  let diff = 0;

  pairs.forEach((pair) => {
    if (diff === 0) {
      if (pair.a > pair.b) {
        diff = 1;
      }
      if (pair.b > pair.a) {
        diff = -1;
      }
      if (!isNaN(pair.a) && isNaN(pair.b)) {
        diff = 1;
      }
      if (isNaN(pair.a) && !isNaN(pair.b)) {
        diff = -1;
      }
    }
  });

  switch (operator) {
    case '=':
    case '==':
      return diff === 0;
    case '>=':
      return diff >= 0;
    case '<=':
      return diff <= 0;
    case '>':
      return diff > 0;
    case '<':
      return diff < 0;

    default:
      throw new Error('Problem comparing versions');
  }
}

One role

Be careful to do only what the functions should.

The function is called compareVersions however what it does is validate and compare versions.

The potential throw is the result of validation and has nothing to do with comparing the values.

As you have not called the function validateAndCompareVersions you have over stepped its role.

How to throw

If (and try to avoid throwing exceptions), if you must throw, throw correctly.

In this case the first throw should be a RangeError and the message should make sense.

You have very poor error message  "Problem comparing versions: not a valid number"; The first part is redundant, exception contains a trace that will locate the "problem" and the "Problem" is implied in the fact that this is an exception.

"not a valid number" The arguments are version strings, "1.0.0" is not a number. The error should indicate the problem explicitly.

 throw new RangeError("Invalid version string: '" + (isNaN(v1) ? v1 : v2) + "'");

The second exception is a little better, but could be improved

 throw new RangeError("Invalid operator: " + o);

How to catch

Only catch what is thrown for you. Higher level catchers will handle the rest.

Having the throws in the function means you must add additional support code outside the function. Try catches break the flow of the code, not having a try catch means that a input error will halt the app.

try {
    compareVersions(a,b,type);
} catch(e) {
    if (e.name === "RangeError") {
        // do what is needed to prevent app from stopping
    } else {
        throw e; // rethrow for development cycle or high level catchers
    }
}

You may need to be a little more specific, you can either extend an existing error or define a new error. Extending is the JS way so using the name property

// in function
const error = new RangeError("Invalid version string: '" + (isNaN(v1) ? v1 : v2) + "'");
error.name = "InvalidVersionStr";
throw error;

// outside function
try {
    compareVersions(a,b,type);
} catch(e) {
    if (e.name === "InvalidVersionStr") {
        // do what is needed to prevent app from stopping
    } else {
        throw e; // rethrow for development cycle or high level catchers
    }
}

Validate or normalize

It is best to avoid exceptions.

In JavaScript we can pretend we have a 3 state Boolean, true, false, and undefined. We can use the 3rd state to signal an error without needing to break flow or halt execution. You can return undefined with return;

Then you calling function need only handle the undefined

 const versionResult = compareVersions(a, b, type);
 if (versionResult === undefined) { /* put the spanner here */ }

Better yet the function should assume all is good and just return true or false.

Validate the version strings at the source and deal with it when you get it (that is where the problem actually is)

You can either validate the version string or normalize the string

function normaliseVersionStr(ver) {
    return /^\d*\.\d*\.\d*$|^\d*$/.test(ver) ? ver : "0";
}
function validateVersionStr(ver) {  // returns false if string not valid, else true
    return /^\d*\.\d*\.\d*$|^\d*$/.test(ver);
}

// Good strings returned as they are
normaliseVersionStr("1.0.0")
normaliseVersionStr("1")
normaliseVersionStr("1.000.0")

// Bad string returned as version 0
normaliseVersionStr("1.0.0A")
normaliseVersionStr("1.")
normaliseVersionStr("1.0")
normaliseVersionStr("")

Alternative solution

Now that you can trust the arguments you get you can write a better function as you dont need to bother with all the possible edge cases.

There are many ways to do this, and they depend on what you define as same, greater, less. I will assume the following

 01 == 1 == 1.0.0 == 01.0.0 == 1.00.00
 0.01.0 == 0.1.0
 0.0.1 < 0.1.0
 2.0.1 < 2.1.0
 1.9.0 < 2

If we then pad the strings to match (each version str) parts sizes, and remove leading zeros.

 1 and 1.0.0 become 100 and 100
 1 and 2 become 1 and 2
 1 and 0.0.1 become 100 and 1
 1.99.0 and 1 become 1990 and 1000

Then use eval to do the final operation. If you don't like eval, you can use new Function

Example

It is assumed that the operator is defined in the source (not as an user input). If the operator is a user input string then you should validate or normalize that string before calling the function.

An invalid operator will throw an exception.

// logical operator optional. Default "=="
// "==", "<", ">", "<=", ">="
// "=" is considered to be "=="
// can also use "!=", "!==", "==="
// strA, strB must be valid version strings
function compareVersionStrings(strA, strB, operator = "==") {
    const a = strA.split("."), b = strB.split(".");
    const len = Math.max(a.length, b.length);
    var valA = a.shift(), valB = b.shift(), i = 1;
    while (i < len) {
        const vA = a[i] !== undefined ? a[i] : "";
        const vB = b[i] !== undefined ? b[i] : "";
        const digits = Math.max(vA.length, vB.length);
        valA += vA.padStart(digits, "0");
        valB += vB.padStart(digits, "0");
        i++;
    }
    valA = valA.replace(/0*\d$/,"");
    valB = valB.replace(/0*\d$/,"");
    operator = operator === "=" ? "==" : operator;
    return eval(`${valA} ${operator} ${valB}`);
    // or
    return (new Function(`return ${valA} ${operator} ${valB})`)();
}


Execute coroutines in pool I want to run all the coroutines from the list in a pool of constant size and would like to ask if this is the right way how to achieve it. Is there any built in solution for this problem? I have not found any.

def subprocess_pool(coroutines, pool_size=3):
    loop = asyncio.get_event_loop()
    event_finished = asyncio.Event()

    def done(future):
        event_finished.set()

    @asyncio.coroutine
    def scheduler(coroutines):
        num = len(coroutines)
        finished = 0
        started = 0
        while finished<num:
            if coroutines and (started - finished) < pool_size:
                cor = coroutines.pop(0)
                task = loop.create_task(cor)
                task.add_done_callback(done)
                started += 1
            else:
                yield from event_finished.wait()
                event_finished.clear()
                finished += 1
        return True

    loop.run_until_complete(scheduler(coroutines))
    loop.close()

Making a pool of coroutines seems not to be the way why we invented coroutines. Coroutines means to be lightweight, so that they can be created in very large numbers. Rather than limiting the number of coroutines, I think the right way to do this is to limit the running coroutines, and:

use an asyncio.Semaphore

async def worker(semaphore, coro):
    async with semaphore:
        await coro()

async def scheduler(corotines):
    tasks = []
    semaphore = asyncio.BoundedSemaphore(pool_size)
    for coro in corotines:
        tasks.append(loop.create_task(worker(coro))
    await asyncio.gather(*tasks)

loop.run_until_complete(scheduler(coroutines))
loop.close()

check out this article for a full example.