Sorting an array in C++ This started as a simple exercise in sorting an array. I wanted to make it a bit more complex which is why I made it a 2D array.

I then noticed that each pass that checked and swapped values, it was checking all ten values. This seemed unnecessary because with each pass the highest value gets moved all the way to its final position. So I made it check one less element each pass until it finished.

I did not want to use std::sort, this was an exercise in finding my own solution.

The program was "falling off" the end of the array, and although I managed to work around it, I'm not sure I did it in the right way, or even why it was doing it in the first place.

The program seems to work as intended but would like some feedback on how it can be improved, or any issues etc.

#include "stdafx.h"

#include<iostream>

using std::cout;
using std::cin;
using std::endl;

int main()
{
    const int ROWS = 10;
    const int COLUMNS = 2;

    int person[ROWS][COLUMNS] =
    {
        { 1,100 },
        { 2,20 },
        { 3,80 },
        { 4,10 },
        { 5,50 },
        { 6,150 },
        { 7,30 },
        { 8,60 },
        { 9,40 },
        { 10,1 }
    };

    for (int i = 0; i < ROWS; ++i)
    {
        cout << "Person " << person[i][0] << " ate: " << person[i][1] << " pancakes";
        {
            cout << endl;
        }
    }

    // Sort lowest to highest

    int loop = 1;
    int count;           // for counting the number of loops
    int limit = ROWS;    // reduced by 1 each loop to skip uneceassary checks

    for (int j = ROWS - 1; j > 0; --j, loop++, limit--)
    {
        cout << "Loop " << loop << endl;
        cout << "Loop limit: " << limit << endl;
        count = 0;

        for (int i = 0; i < ROWS; ++i, ++count )
        {
            if (loop + count == ROWS + 1)   // skips unnecessary checks
            {
                break;
            }
            if (person[i][1] > person[i + 1][1] && i != ROWS - 1)
    // The && condition stopped the  program falling off the end of the array
    // but not sure why it was in the first place
            {
                int temp = person[i][1];
                person[i][1] = person[i + 1][1];
                person[i + 1][1] = temp;
            }
            //cout << person[i][0] << " " << person[i][1] << endl;
            //un-comment ^ to display inner loop passes
        }
        cout << "Number of inner loops: " << count << "\n" << endl;
    }

    // display final order
    for (int i = 0; i < ROWS; ++i)
    {
        cout << person[i][0] << "   " << person[i][1] << endl;
    }

    int pause;
    cin >> pause;

    return 0;
}

The are lots of different sorting algorithms out there. You seem to have re-implemented "Bubble Sort". This is one of my favorite algorithms as it is very easy to write and sufficient (or even preferable) for very small data sets.

The problem with bubble sort is that it has a very bad complexity so when the data sets become large it becomes very expensive and there are other much better algorithms for large data sets.

Overview

  This started as a simple exercise in sorting an array.

Good thing to work on when you are a beginner.

  I wanted to make it a bit more complex which is why I made it a 2D array.

I don't think this adds anything to the problem. One of the main concepts of computer science is abstraction. In sorting we usually abstract the comparison out so that it simply becomes a comparison operation (usually less than i.e. operator<). If you abstract out the comparison you are simply left with the sorting algorithm (which then is the same for every type). So you should have written a less than operation and then you could have re-used the sorting algorithm for any type.

  I then noticed that each pass that checked and swapped values, it was checking all ten values. This seemed unnecessary because with each pass the highest value gets moved all the way to its final position. So I made it check one less element each pass until it finished.

This is a standard optimization for "Bubble Sort". Unfortunately it does not reduce the complexity that much. Still making this the worst sort for large data sets.

One other optimization (which is a very make Bubble Sort great) is that if you do a full inner loop and there was not a single swap then the array is sorted. You can not break out of the outer loop. This optimization is great as it turns the best case situation (where the data is already sorted (or very close to sorted)) into an O(n) (i.e. linear) operation.

  I did not want to use std::sort, this was an exercise in finding my own solution.

Yes. Experimenting with sorting is good. But you should read up on the different types of algorithm and try and re-implement them. Doing this is great experience and will teach you the advantages of the different types.

  The program was "falling off" the end of the array, and although I managed to work around it, I'm not sure I did it in the right way, or even why it was doing it in the first place.

Can't help with that unless I see the code from before your fix. But even if you did this is the wrong site for that. We only review working code.

OK. I found the the bug you mentioned (and fixed). It seems like a common one so I will talk about that below.

  The program seems to work as intended but would like some feedback on how it can be improved, or any issues etc.

OK. Lets have a look.

Code Review

Your Bug:

    if (person[i][1] > person[i + 1][1] && i != ROWS - 1)
    // The && condition stopped the  program falling off the end of the array
    // but not sure why it was in the first place

So you have a loop:

 for (int i = 0; i < ROWS; ++i)

This will allow you to loop over the array so you can accesses all the elements with person[i]. Where ROWS is the number of rows in the array. So this allows you to access all the valid rows. Remember that valid rows in an array are counted from 0 so the last valid row is ROWS - 1 (this is why most loops use less than operator< as you do in the loop test.

The problem is that you also accesses the element person[i + 1]. The largest value of i is ROW-1 so the largest element accessed is person[ROW-1+1] or person[ROW] that is one past the end of the array.

int  array[3] = {1,2,3};
std::cout << array[0];   // first  element
std::cout << array[1];   // second element
std::cout << array[2];   // third  element
// Only 3 elements in the array
std::cout << array[3];   // This is one beyond the end.

Abstraction and self documenting code

Pull out simple pieces of code into their own well named function. This documents what you are doing and makes the underlying algorithm easier to read.

            {
                int temp = person[i][1];
                person[i][1] = person[i + 1][1];
                person[i + 1][1] = temp;
            }

This is obviously a swap. Why not write a function called swapPerson(person[i], person[i+1]); that swaps the values of two people. This will make the sort algorithm easier to read. This also moves the actual swapping processes out of the algorithm allowing you to more easily replace it with another one when you use a different type.

Note: This is so common that the standard library has a std::swap() that swaps two values of a the same type.

Now looking at the comparison:

            if (person[i][1] > person[i + 1][1])

Your code is comparing two elements adding the extra [1] on the end does not change much. But I would change it so that I was comparing two people.

             if (lessPerson(person[i + 1], person[i]) {
             }

Still looks a bit ugly. But it shows you can use a function to do the test. But C++ lets you define functions that look like maths operations. So you can change the named lessPerson() function into operator<() function that allows you to compare two people.

             if (person[i + 1] < person[i]) {
             }

Optimizing the loop

        for (int i = 0; i < ROWS; ++i, ++count )
        {
            if (loop + count == ROWS + 1)   // skips unnecessary checks
            {
                break;
            }

This seems like a very complex way of writing:

        for (int i = 0; i < j; ++i) {

Now if we look at your sort Algorithm after these changes:

    for (int j = ROWS - 1; j > 0; --j) {
        for (int i = 0; i < j; ++i) {
            if (person[i + 1] < person[i + 1]) {
                std::swap(person[i], person[i+1]);
            }
        }
    }

Boiled Down Code.

We seem to have boiled down your code to the absolute minimum. I would add the optimization I mentioned above and wrap this in its own function to make it:

void sort(Person person[], int const ROWS)
{
    for (int j = ROWS - 1; j > 0; --j) {
        bool swapped = false;
        for (int i = 0; i < j; ++) {
            if (person[i + 1] < person[i + 1]) {
                swapped  = true
                std::swap(person[i], person[i+1]);
            }
        }
        if (!swapped) {
            break;
        }
    }
}

Notice I have added the type Person so can explicitly write functions to swap and compare objects of type Person. Now you can write a less than (operator<) and assignment (operator= used by std::swap) specifically for people that would allow the algorithm to work without having to be specific to the algorithm.

The next step is then to make the sort work for any type. Usually we do this with templates. So we can pass any type into the sort function and allow it to be sorted (as long as the type has an operator< and an operator=).

template<typename T>
void sort(T array[], int const ROWS)
{
    for (int j = ROWS - 1; j > 0; --j) {
        bool swapped = false;
        for (int i = 0; i < j; ++) {
            if (array[i + 1] < array[i + 1]) {
                swapped  = true
                std::swap(array[i], array[i+1]);
            }
        }
        if (!swapped) {
            break;
        }
    }
}

The next step is to learn about the concepts of Iterators and how that can make the above function even more useful. But I will leave that as an exercise for now.

Bad Habits

Please don't do this:

using std::cout;
using std::cin;
using std::endl;

Is it that much harder to write std::cout over cout? This is a kind of habit that will get you into a lot of trouble. Especially when you put using at the top level of a file. If you must do this, do it inside a function so it does not pollute the code.

Not sure what the extra level of braces is for.

        {
            cout << endl;
        }

But prefer to use '\n' rather than std::endl. The only difference is that std::endl performs a forced flush of the stream. The stream will auto flush so there is no need to force a flush to start with. Also manually flushing the stream (by the programmer) is nearly always going to cause sub optimal flushing and lead to performance degradation.

You don't need a return 0 in main().

    return 0;


Use async code in class constructor I have some situations where I run async code inside class constructor:
1. Run async code that returns a response
I execute some async code that returns a response, then I apply then and catch inside class constructor:
class MyClass {

  constructor(
    someParameter,
    anotherParameter,
  ) {
    // Run async code that return a response
    someAsyncCode()
      .then( // do something with result of someAsyncCode )
      .catch( // do something if someAsyncCode goes wrong )
  }
}

2. Run async code that doesn't return a response
In some situations, the async code doesn't return anything, but I need to run it.
class MyClass {

  constructor(
    someParameter,
    anotherParameter,
  ) {
    // Run async code that doesn't return anything
    someAsyncCode()
  }

  async someAsyncCode() {
    const someAsyncResult = await something();
    // execute some other logic but not return anything
  }
}

Questions

Its a good or bad practice, execute some async code inside class constructor?
in the situation where you need to execute async code that does not return anything, it is necessary to execute then and catch?
what are the implications of executing asyn code without then and catch?

Its a good or bad practice, execute some async code inside class constructor?

Constructors are always synchronous and should succeed to create an object immediately. If your async call doesn't block that and you don't care about the success of your async call, then it is not a bad practice to call the function without awaiting it, and immediately .catch and handle any possible rejections.
In general, if you don't need the return value of an async call and you don't want to wait for it to complete, you should not await it. But, you should still handle rejections with a .catch; even if you do nothing: something().catch(() => {})

in the situation where you need to execute async code that does not return anything, it is necessary to execute then and catch? what are the implications of executing async code without then and catch?

Having a then is unnecessary, and would only be useful if you wanted to do something when the Promise has resolved, but if the Promise can be rejected you must have a catch.
If you do not catch a rejected promise you will have an unhandledPromiseRejection which, if left unhandled, will crash your process in a future version of Node.js.
If you did care about the return value, or you want the caller to be able to handle rejections, you can make the constructor private and then add a static async factory function that uses the private constructor:
class MyClass {

  private constructor(
    someParameter,
    anotherParameter,
  ) {
    // No async calls here
  }

  static async create (
    someParameter,
    anotherParameter,
  ) {
    await something();
    return new MyClass(someParameter, anotherParameter);
  }

}

Then you would use MyClass.create(x,y) to instantiate it, instead of new MyClass(x,y).


Removing extra spaces from user input I want to take user input strings and add them together with proper spacing.

HTML:

<input id="input" type="text" placeholder="Words Go Here"></input>
<button id="addWords">Add Words</button>

JavaScript:

(function () {

 $('#addWords').on('click', addWords);

 function addWords() {

    var value = $('#input').val();
    var firstChar = value.charAt(0);
    var lastChar = value.slice(-1);

     if ((/^[a-zA-Z\s]*$/.test(value)) && (value !== '')) {

         if (firstChar == ' ' && lastChar == ' ') {

            value = value.trimLeft().trimRight();

         } else if (firstChar == ' ') {

            value = value.trimLeft();

         } else if (lastChar == ' ') {

            value = value.trimRight();

         } else {

            value = value;
        }

        $('#output').append($('').text(value + ' '));

     } else {

        alert('Please use characters only.');

     }
 }

})();

Example:

JSFiddle

Is there a less-bulky (e.g. multiple if elses) method for stripping away additional spaces?

If I'm not misunderstanding the intent of your script you don't need the if elses at all.

For starters not having a space to trim when calling trim(Right or Left) is not a problem. No errors are thrown. So you could just do this:

function addWords() {
    var value = $('#input').val();

    if ((/^[a-zA-Z\s]*$/.test(value)) && (value !== '')) {
        value = value.trimLeft().trimRight();
        $('#output').append($('').text(value + ' '));
    } else {
        alert('Please use characters only.');
    }
}

For simplicities sake instead of value.trimLeft().trimRight() you can just use value.trim().

Edit: Also as Dagg pointed out(see comments below) String.prototype.trimLeft() and String.prototype.trimRight() are non-standard but String.prototype.trim() is in the spec's for ES5.


Floyd-Warshall Path Reconstruction Below is the implementation for the Floyd-Warshall algorithm, which finds all-pairs shortest paths for a given weighted graph.
The function floyd_warshall takes a graph as an input, which is represented by an edge list in the form of [source, destination, weight]
The path_reconstruction function outputs the shortest paths from each vertex that is connected to every other vertex.
Please provide suggestions for improvements of any sort.
import sys

INF = sys.maxsize

def floyd_warshall(graph):
    source_vertices = [column[0] for column in graph]
    destination_vertices = [column[1] for column in graph]
    vertices = list(set(source_vertices) | set(destination_vertices))

    distance = [[INF] * len(vertices) for i in range(len(vertices))]
    next_vertices  = [[0]   * len(vertices) for i in range(len(vertices))]

    for i in range(len(vertices)):
        distance[i][i] = 0
    for source, destination, weight in graph:
        distance[source-1][destination-1] = weight
        next_vertices[source-1][destination-1] = destination-1

    for k in range(len(vertices)):
        for i in range(len(vertices)):
            for j in range(len(vertices)):
                if distance[i][j] > distance[i][k] + distance[k][j]:
                    distance[i][j] = distance[i][k] + distance[k][j]
                    next_vertices[i][j]  = next_vertices[i][k]

    path_reconstruction(distance, next_vertices)

def path_reconstruction(dist, nxt):
    print("Edge \t\t Distance \t Shortest Path")
    for i in range(len(dist)):
        for j in range(len(dist)):
            if i != j:
                path = [i]
                while path[-1] != j:
                    path.append(nxt[path[-1]][j])
                print("(%d, %d) \t\t %2d \t\t %s"
                      % (i + 1, j + 1, dist[i][j], ' - '.join(str(p + 1) for p in path)))
    print()

def main():
    edge_list1 = [
        [1, 3, -2],
        [2, 1, 4],
        [2, 3, 3],
        [3, 4, 2],
        [4, 2, -1]
    ]
    edge_list2 = [
        [1, 2, 10],
        [1, 3, 20],
        [1, 4, 30],
        [2, 6, 7],
        [3, 6, 5],
        [4, 5, 10],
        [5, 1, 2],
        [5, 6, 4],
        [6, 2, 5],
        [6, 3, 7],
        [6, 5, 6]
    ]

    floyd_warshall(edge_list1)
    floyd_warshall(edge_list2)

if __name__ == '__main__':
    main()

Your program provides a good example for seeing how one can increase code readability through some common techniques: (1) convenience variables to avoid verbose repetition; (2) code organized as small commented "paragraphs" or blocks; and (3) the use of shorter variables names to lighten the visual weight of the code, thus enhancing ease of reading and visual scanning. Note that short names must be used judiciously: because they can be cryptic, short vars typically
derive their meaning either from a consistently used convention in the program
or from other contextual clues (eg, from nearby functions or collections having more explicit names and from well-placed comments). Here's a heavily edited version of floyd_warshall() along those lines. Because I'm lazy, I will assume that you implemented Floyd-Warshall correctly.
def floyd_warshall(graph):
    # Collect all vertices.
    vertices = set(
        col[i]
        for col in graph
        for i in (0, 1)
    )
    n = len(vertices)
    rng = range(n)

    # Initialize the distance and next-vertex matrix.
    dists = [
        [0 if i == j else INF for j in rng]
        for i in rng
    ]
    next_vertices = [
        [0 for j in rng]
        for i in rng
    ]

    # Populate the matrixes.
    for src, dst, weight in graph:
        i = src - 1
        j = dst - 1
        dists[i][j] = weight
        next_vertices[i][j] = j

    # Do that Floyd-Warshall thing.
    for k in rng:
        for i in rng:
            for j in rng:
                ikj = dists[i][k] + dists[k][j]
                if dists[i][j] > ikj:
                    dists[i][j] = ikj
                    next_vertices[i][j]  = next_vertices[i][k]

    return path_reconstruction(dists, next_vertices)

A bigger issue is that your floyd_warshall() function should not be calling a
function that prints. Rather it should return some kind of meaningful data.
That approach makes your function more readily testable. For example,
path_reconstruction() could return a list of declarative dicts.
def path_reconstruction(dists, next_vertices):
    # Same ideas here: return data, don't print; use convenience
    # vars where they help with readability.
    rng = range(len(dists))
    paths = []
    for i in rng:
        for j in rng:
            if i != j:
                path = [i]
                while path[-1] != j:
                    path.append(next_vertices[path[-1]][j])
                paths.append(dict(
                    i = i,
                    j = j,
                    dist = dists[i][j],
                    path = path,
                ))
    return paths

Then do your printing outside of the algorithmic code.
def main():
    edge_lists = [
        [
            [1, 3, -2],
            [2, 1, 4],
            [2, 3, 3],
            [3, 4, 2],
            [4, 2, -1],
        ],
        [
            [1, 2, 10],
            [1, 3, 20],
            [1, 4, 30],
            [2, 6, 7],
            [3, 6, 5],
            [4, 5, 10],
            [5, 1, 2],
            [5, 6, 4],
            [6, 2, 5],
            [6, 3, 7],
            [6, 5, 6],
        ],
    ]
    for el in edge_lists:
        paths = floyd_warshall(el)
        for p in paths:
            print(p)


Multiplying and Adding Large Numbers Lately, I've been working on the Euler projects. I just started coding in C++ and could really use some feedback. Basically, I wrote a program to calculate the factorial of any number between 1 and 100 (I have not tested it above 100).

I looked into the Boost library, but it seemed a little bit to easy, so I wrote some algorithms on my own.

Anyway, I would really like some comments about whether this is efficient or not, and if I can somehow optimize the algorithms? I hope this post is within the scope of the forum, since it is my first.

I first convert two integers to vectors, storing each digit separately:

// Converting number to vector
std::vector<int> LargeNumber(int number) {
    std::vector<int> numbers;
    while (number > 0) {
        numbers.push_back(number % 10);
        number /= 10;
    }
    return numbers;
}

Then I pass the vectors on to one of these two functions, depending on whether I want to add or multiply:

// Adds large numbers
std::vector<int> addition(std::vector<int> max, std::vector<int> min) {
    if (max.size() < min.size()) {
        max.swap(min);
    }
    std::vector<int> sum;
    int rest = 0;

    for (int i = 0; i < min.size(); i++) {
        int c = min[i] + max[i] + rest;
        sum.push_back(c % 10);
        rest = c / 10;
    }

    for (int i = min.size(); i < max.size(); i++) {
        int c = max[i] + rest;
        sum.push_back(c % 10);
        rest = c / 10;
    }
    while (rest > 0) {
        sum.push_back(rest % 10);
        rest /= 10;
    }

    return sum;
}

// Multiplying large numbers
std::vector<int> multiplication(std::vector<int> max, std::vector<int> min) {
    if (max.size() < min.size()) {
        max.swap(min);
    }

    std::vector<std::vector<int>> sums;
    std::vector<int> sum;
    for (int i = 0; i < min.size(); i++) {
        sum.resize(0);
        int n = i;
        while (n > 0) {
            sum.push_back(0);
            n--;
        }
        int rest = 0;
        for (int j = 0; j < max.size(); j++) {
            int c = max[j] * min[i] + rest;
            sum.push_back(c % 10);
            rest = c / 10;
        }
        while (rest > 0) {
            sum.push_back(rest % 10);
            rest /= 10;
        }
        sums.push_back(sum);
    }
    sum.resize(0);

    for (int i = 0; i < sums.size(); i++) {
        sum = addition(sum, sums[i]);
    }
    return sum;
}

I can then calculate the factorial as:

#include <vector>
int main () {
    // Calculating the factorial of the number N
    int N = 100;
    std::vector<int> fac;
    sum.push_back(1);
    for (int i = 2; i <= N; i++) {
        fac = multiplication(sum, LargeNumber(i));
    }
    // Printing the result
    std::cout << "The factorial of " << N << ": " <<  std::endl;
    for (int i = sum.size()-1; i >= 0; i--) {
        std::cout << fac[i];
    } std::cout << std::endl;
}

Since I am very new to C++, it is hard for me to know what exactly to ask for, so any comment is very welcome.

It's really worth making your big number be a class.  This will give you the ability to

change the internal representation without altering client code
write operators (+, -, /, *, <, ==, << and more) that work just like they do for other numbers
specialize numeric limits and other traits classes so they can be used as arithmetic types in algorithms. For example, you might like a std::complex<BigNum>.  You can have one, but a std::complex<std::vector<int>> isn't going to work.
add literal operators so that you can write them naturally in your source code.

You don't have to do all the above at once, of course.  But I recommend you read the review of Implementation of a Rational Number class in C++ to see how an arithmetic class can work.
Your main() would then be isolated from the implementation details, and look something like
#include <bignum>

int main () {
    int N = 100;
    auto fac = 1_big;
    do {
        fac *= N;
    } while (--N > 0);

    std::cout << "100! = " << fac << std::endl;
}

Space efficiency
Once you have your class, you'll observe that writing one digit in each int is quite wasteful.  char is defined to include at least 0 to 127 so you could store two digits (0 to 100) in each element of a vector of characters.
If you've made your big number a class, it's then easier to do this in two steps - first store one digit in each character, and then widen to 2 per char.  There are more space-efficient encodings; you'll have to decide where the trade-off between space and time lies for your needs.

Temporary values in multiplication
In your long multiplication, you store all the intermediate results before adding them together.  You can save some of this space by using an accumulator variable to add the results as you go, like this pseudo-code:
accumulator = 0
for i in digits
     tmp = arg1 * i
     accumulator += tmp
return accumulator

Then you no longer need to have a vector of partial products throughout the function.

Working example
Here's what I implemented, following the notes above.  It's the absolute minimum needed to calculate factorials.  Several features are left as an exercise:

Implement a user-defined literal that accepts longer numbers than unsigned long long can represent
Support negative numbers
Implement the other arithmetic and relational operators
Make more of the implementation constexpr so it can be evaluated during compilation

#include <vector>
#include <iosfwd>

class Bignum
{
    static constexpr char max_digit = 100;

    std::vector<char> digits = {};   // smallest centits first

public:
    Bignum(unsigned long long int n)
    {
        while (n) {
            digits.push_back(n % max_digit);
            n /= max_digit;
        }
    }
    // copy constructor and assignment operator can be defaulted

    Bignum& operator+=(const Bignum& other);
    Bignum operator+(const Bignum& other) const;

    Bignum& operator*=(const Bignum& other);

    friend std::ostream& operator<<(std::ostream&, const Bignum&);

private:
    // helper functions
    Bignum& halve();
    Bignum& trim();
};

Bignum operator""_big(unsigned long long n) { return n; }

// IMPLEMENTATION

#include <algorithm>
#include <iostream>
#include <utility>

Bignum& Bignum::operator+=(const Bignum& other)
{
    int sum = 0;
    auto i = digits.begin();
    auto j = other.digits.begin();
    bool in = true;             // until i reaches digits.end()
    bool jn = true;
    while ((in &= i != digits.end()) | (jn &= j != other.digits.end()) || sum) {
        if (jn)
            sum += *j++;
        if (in) {
            sum += *i;
            *i++ = sum % max_digit;
        } else {
            digits.push_back(sum % max_digit);
        }
        sum /= max_digit;       // carry to next digit
    }
    return *this;
}

Bignum Bignum::operator+(const Bignum& other) const
{
    return Bignum{*this} += other;
}

Bignum& Bignum::operator*=(const Bignum& other)
{
    Bignum a = *this, b = other;
    if (a.digits.size() > b.digits.size())
        std::swap(a, b);

    digits.clear();
    while (a.digits.size() > 0) {
        if (a.digits.front() % 2)
            *this += b;
        a.halve();
        b += b;
    }
    return *this;
}

Bignum& Bignum::halve()
{
    int carry = 0;
    for (auto i = digits.rbegin();  i != digits.rend();  ++i) {
        carry *= max_digit;
        carry += *i;
        *i = carry / 2;
        carry = carry % 2;
    }
    return trim();
}

Bignum& Bignum::trim()
{
    while (digits.size() > 0 && !digits.back())
        digits.pop_back();
    return *this;
}

std::ostream& operator<<(std::ostream& out, const Bignum& n)
{
    if (n.digits.size() == 0)
        return out << '0';

    bool z = false;             // track leading zeros
    for (auto i = n.digits.rbegin();  i != n.digits.rend();  ++i) {
        int n = *i;
        if (z |= n/10)
            out << n/10;
        if (z |= n)
            out << n % 10;
    }

    return out;
}

// PROGRAM

int main () {
    const int N = 100;
    auto fac = 1_big;

    for (int i = 1;  i < N;  ++i) {
        fac *= i;
        std::cout << i << "! = " << fac << std::endl;
    };
}


Simple class to create csv As part of a solution I have the following class createCSV. It is working as expected but I'm very much aware of my lack of experience with c# and OO programming: are there any obvious potential problems or simple refactors?

(note: the use of a guid is a user requirement)

using System.Text;
using System.IO;
using System.Data;
using System;
using System.Collections.Generic;

namespace createCSV
{
  class Csv
  {

    private string guid;

    public Csv(string aGuid)
    {
      guid = aGuid;
    }

    public void CreateCSV(DataTable dt,string aDestPath, string aTitle)
    {

      string filePath = string.Concat(aDestPath, aTitle, @"-", this.guid, @".csv");
      string delimiter = ",";

      StringBuilder sb = new StringBuilder();
      List<string> CsvRow = new List<string>();

      //write headers
      foreach(DataColumn c in dt.Columns)
      {
        CsvRow.Add(c.ColumnName.ToString());
      }
      sb.AppendLine(string.Join(delimiter,CsvRow));

      //write data
      foreach(DataRow r in dt.Rows)
      {
        CsvRow.Clear();

        //go through each column adding to a list of strings
        foreach(DataColumn c in dt.Columns)
        {
          CsvRow.Add(r[c.ColumnName].ToString());
        }

        sb.AppendLine(string.Join(delimiter,CsvRow));
      }

      File.WriteAllText(filePath,sb.ToString());

      Console.WriteLine();
      Console.WriteLine(String.Concat(@"just completed: ",aTitle,@"-",this.guid,@".csv"));
    }

  }
}

Before you do other things you should separate and encapsulate, this means you should put certain logic into their own methods that specialize in doing only one thing. If you put e.g. the loops in such method methods, you won't need any comments. Here's an example of how it could be done with C# 7. Also notice the changed names. There's no need for the a/an/the prefixes.

This is just a start and there is much more to improve. I left it out intentionally.

public void CreateCsv(DataTable data, string path, string title)
{
    string CreateFileName() => string.Concat(path, title, @"-", guid, @".csv");

    void WriteHeader(StringBuilder csv)
    {
        var header = new List<string>();
        foreach (var column in data.Columns.Cast<DataColumn>())
        {
            header.Add(column .ColumnName);
        }
        csv.AppendLine(string.Join(Delimiter, header));
    }

    void WriteData(StringBuilder csv)
    {
        foreach (var row in data.Rows.Cast<DataRow>())
        {
            var values = new List<string>();
            foreach (var column in data.Columns.Cast<DataColumn>())
            {
                values.Add(row[column.ColumnName].ToString());
            }
            csv.AppendLine(string.Join(Delimiter, values));
        }
    }

    string WriteCsv()
    {
        var csv = new StringBuilder();
        WriteHeader(csv);
        WriteData(csv);
        return csv.ToString();
    }

    File.WriteAllText(CreateFileName(), WriteCsv());

    Console.WriteLine();
    Console.WriteLine(String.Concat(@"just completed: ", title, @"-", this.guid, @".csv"));
}


Consolidating array accesses Is there a better or easier way of doing this?

if array[1] and array[2] and array[3] and array[4] and array[5] == false then

    --somthing

end

I have a lot of arrays to check and was wondering if there is a way to work with arrays where you can do something like array[1-6] or [1,6] or something. Can't seem to find anything online.

I'm not sure of the lua syntax but something like

arrayAllFalse=TRUE;
for element in array
  if (element) then
  arrayAllFalse=FALSE
  end

if (arrayALLFalse) then
    --something
end


Efficiently working out the nearest weekend Is this an efficient way to do this?

the code is designed to find the nearest friday, without passing Sunday. If it is already past Friday, the start date should equal now.

function get_weekend(){
        $start = time();
        $day = date('w',$now);

        //find friday, without going past Sunday
        while ($day < 5 && $day > 0){
            $start = $start + 86400;
            $day = date('w',$start);
        }

        //find nearest sunday to start date
        $end = $start;
        while ($day != 0){
            $end = $end + 86400;
            $day = date('w',$end);
        }
        $weekend['StartDate']=date("d-F-Y",$start);
        $weekend['EndDate'] = date("d-F-Y",$end);
        return $weekend;

    }

Short, short version:

function get_weekend()
{
    $fri = strtotime('friday');
    $sun = strtotime('sunday');
    // strtotime('friday') will be after strtotime('sunday') if it is after 0:00 Friday
    // and before 0:00 on Sunday
    $start = $sun < $fri? now():$fri;
    $weekend = array();
    $weekend['StartDate'] = date('d-F-Y', $start);
    $weekend['EndDate'] = date('d-F-Y', $end);
    return $weekend;
}


Recurrent action on concurrent collection I have a bunch of concurrent threads carrying out operations which return alerts. The alerts have to be persisted to a DB, but this cannot be done concurrently as it may cause duplicate alerts being created.
To avoid duplicate alerts I have written a class containing a ConcurrentBag<T> with a Timer that triggers Action<T> (in this case, sending my pending alerts to the DB) at a set interval.

I'm wondering if what I wrote it is written in a way that will not cause problems in the execution in the future. If alerts get lost due to an unexpected exception that would be problematic. I'd appreciate comments/criticisms/suggestions for improvement.

Here's the code:

class PeriodicConcurrentBag<T>
{
    public int Count
    {
        get { return this.items.Count(); }
    }

    public bool HasItems
    {
        get { return this.items.Any(); }
    }

    public bool IsStopped { get; private set; }

    private int timerPeriod;
    private Timer timer;
    private ConcurrentBag<T> items;
    private Action<IEnumerable<T>> actionOnItems;

    public PeriodicConcurrentBag(Action<IEnumerable<T>> actionOnItems, int periodInSeconds)
    {
        this.timerPeriod = periodInSeconds * 1000;
        this.actionOnItems = actionOnItems;

        this.items = new ConcurrentBag<T>();
    }

    public void Start()
    {
        this.IsStopped = false;
        this.timer = new Timer(this.InternalPeriodicAction, null, this.timerPeriod, Timeout.Infinite);
    }

    public void Stop()
    {
        this.IsStopped = true;
    }

    public void AddItem(T item)
    {
        this.items.Add(item);
    }

    public void AddItems(IEnumerable<T> items)
    {
        this.items.AddRange(items);
    }

    private void InternalPeriodicAction(object state)
    {
        if (!this.IsStopped)
        {
            var currentItems = Interlocked.Exchange(ref this.items, new ConcurrentBag<T>());
            this.actionOnItems(currentItems);

            if (!this.IsStopped)
                this.Start();
        }
    }
}

EDIT: The AddRange() method is an extension method I created for ConcurrentBag.

Your Stop method does not actually stop the timer. This is

really counter-intuitive
wastes CPU

There is also a racing condition. If someone calls Stop in-between those two lines:

           if (!this.IsStopped)
            this.Start();

the Stop call will be ignored. The moral of the story: just take the lock.

Timer implements IDisposable. So you should call Dispose when you no longer need it (in Stop method, for example.)

I would refactor actionOnItems into a public event.

int for time period is ambiguous and limiting. I can not use your class, if I want to send updates every 0.5 seconds, and I can easily make an error by assuming, that int represent milliseconds. You can eliminate those problems by replacing it with TimeSpan.

Edit: Also your AddRange method rises some red flags. I assume it is not an atomic operation, and you add items to the bag one by one. What will happen, if AddRange is called right before Interlocked.Exchange(ref this.items, new ConcurrentBag<T>()); ? Do you think the new items will go to the new bag or to the old one? How is your extension method synchronized with the rest of your code? You are walking on the really thin ice here. If you have a limited experience with multi-threading, I suggest you start simple. Remove ConcurrentBag, use regular list, but lock every single access point (both  public AND private, methods AND properties). Once this is working, test if it meats your performance requirements. If it does, leave it at that. There is no point in fighting over nanoseconds if

You do not actually need those nanoseconds

and

You get an extremely bug-prone code as a result


Finding all upper/lower case combinations of a word For every word there are 2^n different ways of writing the word if you take into account upper/lower case letters. Eg for "word" we can write;

word
Word
wOrd
WOrd
woRd
WoRd
etc

I've written this code to calculate all the combinations. Is there any way I can improve the performance? Profiling tells me that this method takes 99.9% of the execution time of my program (which measures password strength).

String word = "word";
int combinations = 1 << word.length();

for (int i=0; i<combinations; i++) {
  StringBuilder buf = new StringBuilder(word);
  for (int j=0; j<word.length(); j++) {
    if ((i & 1<<j) != 0) {
      String s = word.substring(j, j+1).toUpperCase();
      buf.replace(j, j+1, s);
    }
  }
  System.out.println(buf);
}

public static void comb(String word) {
    int combinations = 1 << word.length();
    char[][] chars = { word.toLowerCase().toCharArray(),
                       word.toUpperCase().toCharArray() };
    char[] result = new char[word.length()];

    for (int i = 0; i < combinations; i++) {
        for (int j = 0; j < word.length(); j++) {
            result[j] = chars[(i >> j) & 1][j];
        }
        System.out.println(new String(result));
    }
}

[Edit]

I did some profiling, and my version seems slightly better, and the following version is even a little bit faster:

public static void comb(String word) {
    word = word.toLowerCase();
    int combinations = 1 << word.length();
    for (int i = 0; i < combinations; i++) {
        char[] result = word.toCharArray();
        for (int j = 0; j < word.length(); j++) {
            if (((i >> j) & 1) == 1 ) {
               result[j] = Character.toUpperCase(word.charAt(j));
            }
        }
        System.out.println(new String(result));
    }
}

However, all versions so far are in the same ballpark and won't change something substantial, because System.out.println dominates the performance.

[Edit 2]

The algorithm really doesn't matter much, the System.out.println impact is too heavy. Collection everything in a StringBuilder and calling System.out.println doesn't help either. However, quite surprisingly my profiler tells me that this simple recursive version performs best:

public static void comb4(String word) {
    comb4(word,new char[word.length()],0);
}

private static void comb4(String word, char[] accu, int index) {
    if(index == word.length()) {
        System.out.println(accu);
    } else {
        char ch = word.charAt(index);
        accu[index] = Character.toLowerCase(ch);
        comb4(word, accu , index+1);
        accu[index] = Character.toUpperCase(ch);
        comb4(word, accu, index+1);
    }
}


Loading information about a competition every time the page is loaded I am using KeystoneJS to manage data and my APIs (which includes Node.js, MongoDB, and Express.js). It uses Mongoose to connect with MongoDB; Keystone queries are essentially the same as Mongoose queries, just prefixed with keystone.list('listname').model instead of mongoose.model('name').

My use case is loading information about a competition when a relevant page is being visited by a user. I'm using Express.js to identify the competition we're looking for (stored on the key path of my Competition model, and identified in Express as req.params.cid (Competition ID). Other Express.js routes will use the information from the res.locals.competition path (name, date, location, etc.)

What I am currently doing is running one query to get information about the current competition every time the page is visited. This is stored as a middleware function, loadCompetition, which is called for every Express route that matches /competition/cid*.

app.all('/competition/:cid*', middleware.loadCompetition);

My code looks like this:

exports.loadCompetition = function (req, res, next) {
    keystone.list('Competition').model.findOne({key: req.params.cid}).exec(function (err, competition) {
        if (competition && !err) {
            res.locals.competition = competition;
            return next();
        }
        else {
            if (!competition) {
                req.flash("error", "No competition could be found at this page.");
                return res.redirect("/");
            }
            else  {
                req.flash("error", "An unexpected error occurred. Please try again! If this error persists, email admin@ezratech.us");
                return res.redirect("/");
            }
        }
    });
};

Is this the most effective way of loading a competition like this? Is it feasible to load information about the competition once, and only need to reload it if something changes? Or is what I'm doing the only realistic way of loading information on a competition? I think I would need to query the competition anyway to see if datum has changed (Keystone can track the date a document was modified at, on the updatedAt path), and that would make this entirely redundant. But I don't know if I am missing something. If anything else looks awry, please let me know as well.

Is this slowing down your app by a significant amount? If not, I wouldn't worry about it.

Other than that I would consider removing the custom error handling, and rather pass it on to some other more general error handler at the end of your middleware stack (just call next(err)), and maybe make the Not Found error a 404 status code.


Asp.net web API Nlog implementation with UnhandledExceptionHandler I have an asp.net web api. I would like to log errors into a file. So I decided to use NLog. I already have UnhandledExceptionHandler so I added NLog as follows. I removed all the try/catch in order to log from a single point, just inside the handler.

Would you please have a look at my code? Is it good enough?

using System;
namespace Game.Handlers
{
    public class UnhandledExceptionLogger : ExceptionLogger
    {
        private static Logger logger = LogManager.GetCurrentClassLogger();

        public override void Log(ExceptionLoggerContext context)
        {
            var ex = context.Exception;

            string strLogText = FormatException(ex);

            var requestedURi = (string)context.Request.RequestUri.AbsoluteUri;
            var requestMethod = context.Request.Method.ToString();
            var timeUtc = DateTime.Now;

            SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
            ApiError apiError = new ApiError()
            {
                Message = strLogText,
                RequestUri = requestedURi,
                RequestMethod = requestMethod,
                TimeUtc = DateTime.Now
            };
            //sqlErrorLogging.InsertErrorLog(apiError);
            //NLOG
            logger.Error(strLogText + Environment.NewLine + DateTime.Now);
        }

        private string FormatException(Exception ex, int depth = 0)
        {
            var indent = "";
            if (depth > 0)
                indent.PadRight(depth * 4);

            var sb = new StringBuilder();
            IndentLine($"Source --- {ex.Source}");
            IndentLine($"StackTrace --- {ex.StackTrace}");
            IndentLine($"TargetSite --- {ex.TargetSite}");

            if (ex.InnerException != null)
            {
                IndentLine("--- Inner Exception ---");
                sb.Append(FormatException(ex.InnerException, depth + 1));
                IndentLine("--- End Inner Exception ---");
            }
            if (ex.Message != null)
            {
                IndentLine($"Message --- {ex.Message}");
            }
            return sb.ToString();

            void IndentLine(string s)
            {
                sb.Append(indent);
                sb.AppendLine(s);
            }
        }

    }
}

Here is the sample NLog config:

<nlog xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <targets>
      <target name="logfile" xsi:type="File" fileName="${basedir}/MyLogs/${date:format=yyyy-MM-dd}-api.log" />
      <target name="eventlog" xsi:type="EventLog" layout="${message}" log="Application" source=" GamePIN Api Services" />
      <target name="database" type="Database" connectionString="Data Source=(localdb)\MSSQLLocalDB;Database=Game; Initial Catalog=GameAPI;MultipleActiveResultSets=True;">
        <commandText> insert into ExceptionLog ([TimeStamp],[Level],Logger, [Message], UserId, Exception, StackTrace) values (@TimeStamp, @Level, @Logger, @Message, case when len(@UserID) = 0 then null else @UserId end, @Exception, @StackTrace); </commandText>
        <parameter name="@TimeStamp" layout="${date}" />
        <parameter name="@Level" layout="${level}" />
        <parameter name="@Logger" layout="${logger}" />
        <parameter name="@Message" layout="${message}" />
        <parameter name="@UserId" layout="${mdc:user_id}" />
        <parameter name="@Exception" layout="${exception}" />
        <parameter name="@StackTrace" layout="${stacktrace}" />
        <dbProvider>System.Data.SqlClient</dbProvider>
      </target>
    </targets>
    <rules>
      <!-- I am adding my 3 logging rules here -->
      <logger name="*" minlevel="Error" writeTo="database" />
      <logger name="*" minlevel="Error" writeTo="logfile" />
      <logger name="*" minlevel="Error" writeTo="eventlog" />
    </rules>
  </nlog>

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);

The multiple calls to DateTime.Now will give different time stamps.

Hold on to the timestamp in one variable early in the function and reuse that.

It was already assigned here

 var timeUtc = DateTime.Now;

so use that

//...

ApiError apiError = new ApiError()
{
    Message = strLogText,
    RequestUri = requestedURi,
    RequestMethod = requestMethod,
    TimeUtc = timeUtc
};

//...

Given the variable name, the assumption is that the timestamp was meant to be UTC, which would mean that it should be using DateTime.UtcNow

The SQL error logging (though commented out) can be encapsulated into its own function/concern

private void LogSQL(string message, HttpRequestMessage request, DateTime timestamp) {
    var requestedURi = (string)request.RequestUri.AbsoluteUri;
    var requestMethod = request.Method.ToString();
    SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
    ApiError apiError = new ApiError() {
        Message = message,
        RequestUri = requestedURi,
        RequestMethod = requestMethod,
        TimeUtc = timestamp
    };
    //sqlErrorLogging.InsertErrorLog(apiError);
}

This can be abstracted out into its own service/concern if so desired

The above and a few minor changes results in a refactoring of

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timestamp = DateTime.UtcNow;
        string strLogText = FormatException(context.Exception);
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}


Efficent way of synchronizing between data I'm currently working on a project where I have a punch of raw data and multiple components that use that data for various tasks. I would like to separate the raw data from the components as much as possible and keep the raw data as only data (So no logic or functionality) but the components should be able to read and write to the raw data at any time.

The program will run each component and once all components have been run than the process repeats until the program shuts down. A component can be threaded, but it must join before the process repeats.

Each component will create objects based on the data that will do various things. To avoid having to create and delete those objects at each run for performance the objects must persist and synchronize it's data with the raw data at the beginning of the run.

So to summarize, the design should look something like this

This brings up the issue of synchronization as I need the component to synchronize its data with the raw data at beginning of each run.

Speed is some what important and so I can't go through each object and check if it it's members are equal to that of the component as that is process consuming especially considering the data will not change for the majority of the time.

What I thought about doing is give each object a GUID and every time one of it's values changes than I generate a new GUID. Therefore, if the GUID in the component is different than what it is in the raw data than the value must have changed.

This is simple to implement with getter and setter for each member and in the setter I just generate a new GUID.

Now, I wanted to avoid using getter and setter for two reasons. First, that increases the amount of typing I have to do and that will only increase my chance of getting arthritis sooner. Second, I'm scared my colleagues will take the lazy way and not add getter and setter for when they add members or passing the setter all together and setting the value directly causing a huge headache and waste of debugging time.

Therefore, I decided to create a template class that handles this for me.

This is what I came up with

#include<iostream>
#include "uuid.hpp"
#include "uuid_generators.hpp"
#include "uuid_io.hpp"

template<typename T>
class MemberTracker
{
private:
    T m_value;
    boost::uuids::uuid& m_KeepingTrackOf;
public:
    MemberTracker(boost::uuids::uuid& KeepTrackof)
        : m_KeepingTrackOf(KeepTrackof)
    {
    }
    operator T()
    {
        return m_value;
    }
    MemberTracker& operator=(T input)
    {
        this->m_KeepingTrackOf = boost::uuids::random_generator()();
        this->m_value = input;
        return *this;
    }
};

class ObjectRawData
{
public:
    boost::uuids::uuid guid;
    MemberTracker<int> value;
    ObjectRawData()
        : value(guid)
    {
        guid = boost::uuids::random_generator()();
    }
    ObjectRawData(const ObjectRawData &obj)
        : value(guid)
    {
    }
};

int main()
{
    ObjectRawData h;
    std::cout << h.guid << std::endl;
    h.value = 4;
    std::cout << h.guid << std::endl;
}

The only thing I don't like about this design is that I have to pass in the GUID as a parameter in the constructor.

So, my question is did I go in the right direction and what are the flaws of this design??

I think you have two main choices, have the components throw events when they dirty the data.

The plus here is that events are extremely multithreaded friendly.
the minus is the program might have to know about how to sync the dirty data.

The other option is to have an abstract class with two methods doWork() and sync().
First run a parallel call to all the components.dowork
The when all completed run down each components.sync()

This has the plus of the program not knowing about data sync but the component stores its own dirty list and is very customizable as a result..
This has the minus of the components knowing about data sync.


Python static / instance method or function? background:
I have a model called FileInfo, which represents a sftp file / windows dfs file.

class FileInfo(object):
    def __init__(self, server_info, folder_path, file_name):
        self.server_info = server_info
        self.folder_path = folder_path
        self.file_name = file_name

    def get_file_full_path(self):
        return os.path.join(self.folder_path, self.file_name)

FileInfo has a ServerInfo to hold the server connection info: e.g. sftp connection details.

class ServerInfo(object):
    pass

class WindowsServerInfo(ServerInfo):
    pass

class SFTPServerInfo(ServerInfo):
    pass

this is how I print out the full path:

if __name__ == '__main__':
    sftp_server_info = SFTPServerInfo()
    win_server_info = WindowsServerInfo()

    print FileInfo(win_server_info, r'\\dfs\domain\tmp', 'test.txt').get_file_full_path()
    print FileInfo(sftp_server_info, r'/tmp', 'test.txt').get_file_full_path()

the application is running in Windows and Linux, so the output of the above code will be different -- depends on the hosting OS result on Linux:

\\dfs\domain\tmp/test.txt
/tmp/test.txt

result on Windows:

\\\\dfs\\domain\\tmp\\test.txt
/tmp\\test.txt

I want to have a consistent output, such as:

\\dfs\domain\tmp\test.txt
/tmp/test.txt

so I start to refactor the code:

v1. if...else...

class FileInfo(object):
    def get_file_full_path(self):
        if isinstance(self.server_info, SFTPServerInfo):
             return r'%s/%s' % (self.folder_path, self.file_name)
        elif isinstance(self.server_info, WindowsServerInfo):
            return r'%s\%s' % (self.folder_path, self.file_name)
        else
            raise exception

it works, but if...else means it'll be a big method if I want to support more server types. so I'm going to create a dict to handle it.

v2. a dict contains ServerInfo type and function mapping

class FileInfo(object):
    def get_file_full_path(self):
       return get_full_path_functions[type(self.server_info)](self.folder_path, self.file_name)

def get_full_path_windows(folder_path, file_name):
    return r'%s\%s' % (folder_path, file_name)

def get_full_path_sftp(folder_path, file_name):
    return r'%s/%s' % (folder_path, file_name)

get_full_path_functions = {
    WindowsServerInfo: get_full_path_windows,
    SFTPServerInfo: get_full_path_sftp
}

it works fine, but I think it may be a good idea to move these methods into ServerInfo

v3. instance method

class FileInfo(object):
    def get_file_full_path(self):
        return self.server_info.get_full_path(self.folder_path, self.file_name)

class WindowsServerInfo(ServerInfo):
    def get_full_path(self, folder_path, file_name):
        return r'%s\%s' % (folder_path, file_name)

class SFTPServerInfo(ServerInfo):
    def get_full_path(self, folder_path, file_name):
        return r'%s/%s' % (folder_path, file_name)

however, get_full_path methods are not using self, and this method should not belongs to any instance of ServerInfo

v4. static method

class FileInfo(object):
    def get_file_full_path(self):
        return type(self.server_info).get_full_path(self.folder_path, self.file_name)

class WindowsServerInfo(ServerInfo):
    @staticmethod
    def get_full_path(folder_path, file_name):
        return r'%s\%s' % (folder_path, file_name)

class SFTPServerInfo(ServerInfo):
    @staticmethod
    def get_full_path(folder_path, file_name):
        return r'%s/%s' % (folder_path, file_name)

All these 4 versions are working as expected, please help to review them and let me know:

from the OOP perspective, which one is the best? is there a better way to desing it?
from Python perspective, which one is the most
pythonic way?

Since you have classes that represent the configuration of various server types, you should keep the path structure in them and uses it from them. pathlib provide an abstraction over windows and unix file paths that you can store in these classes and reuse it from FileInfo.

import pathlib

class FileInfo(object):
    def __init__(self, server_info, folder_path, file_name):
        self.server_info = server_info
        self.folder_path = folder_path
        self.file_name = file_name

    def get_file_full_path(self):
        return self.server_info.paths(self.folder_path, self.file_name)

class ServerInfo(object):
    pass

class WindowsServerInfo(ServerInfo):
    paths = pathlib.PureWindowsPath

class SFTPServerInfo(ServerInfo):
    paths = pathlib.PurePosixPath

However, if the only purpose of the FileInfo class is to retrieve a path from its arguments, then you’re better of putting this function directly into ServerInfo:

class ServerInfo(object):
    def get_file_full_path(self, folder_path, file_name):
        return self.paths(folder_path, file_name)

class WindowsServerInfo(ServerInfo):
    paths = pathlib.PureWindowsPath

class SFTPServerInfo(ServerInfo):
    paths = pathlib.PurePosixPath

And you could improve it by allowing multiple path segments:

class ServerInfo(object):
    def get_file_full_path(self, *path_segments):
        return self.paths(*path_segments)

class WindowsServerInfo(ServerInfo):
    paths = pathlib.PureWindowsPath

class SFTPServerInfo(ServerInfo):
    paths = pathlib.PurePosixPath

But it seems that you're only using Python 2, so pathlib may not be available. You can still use your '%s/%s' or '%s\\%s' style approach, but you will loose some path simplifications provided by pathlib:

class ServerInfo(object):
    def get_file_full_path(self, *path_segments):
        return self.path_delimiter.join(*path_segments)

class WindowsServerInfo(ServerInfo):
    path_delimiter = '\\'

class SFTPServerInfo(ServerInfo):
    path_delimiter = '/'

Oh, and talking about '%s/%s' or '%s\\%s', str.format is the prefered way over % formating syntax.


Send multiple REST requests at the same time. (node) Below is a (very) simplified version of a function I made that creates multiple REST requests to gather needed information.
Things to note:

One of the REST calls is dependent upon the output of another.
I want to send off multiple requests at the same time when possible
If any rest responses inform us that the user is logged out, null should be returned.

async function gatherUserInfo(userId) {
  try {
    // Don't await yet, so this request can be sent at the same time as future requests.
    const groupsRequest = getGroups(userId)

    const {username, name} = await getProfile(userId)
    return {
      username,
      name,
      statusText: await getStatusText(username),
      groups: await groupsRequest,
    }
  } catch (err) {
    if (err instanceof NotLoggedInError) {
      return null
    }
    throw err
  }
}

I found the above code to be a very elegant solution to the problem. However, I recently realized that it won't actually work in node, because node will give warning or stop execution (depending on version) if an uncaught error happens in a promise. In the example above, the getGroups() call would send off a request (but not await it). Then, this function execution will pause on getProfile(). In the meantime, getGroups() will come back with the NotLoggedInError, which will result in the process being terminated, because that promise has not been awaited yet, (the try-catch can't catch the error until the await happens, which does happen later in the function execution).
So, I reluctantly trashed my old design and coded this up instead:
async function gatherUserInfo(userId) {
  const requests = [];

  requests.push(
    getGroups(userId)
    .then(groups => ({groups}))
  )

  requests.push(
    getProfile(userId)
    .then(async ({username, name}) => {
      const statusText = await getStatusText(username)
      return {username, name, statusText}
    })
  )

  let responses
  try {
    responses = await Promise.all(requests)
  } catch (err) {
    if (err instanceof NotLoggedInError) {
      return null
    }
    throw err
  }

  return Object.assign({}, ...responses)
}

This is much harder to follow or understand, but it's the best I could come up with. How can this code be improved?
Here's some stub methods and test cases that can be paired with the examples above to execute them:
// Mocks //

async function getGroups(userid) {
  await wait(50)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return ['group A', 'group B'];
}

async function getProfile(userid) {
  await wait(30)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return { username: 'grinchMaster135', name: 'The Grinch' };
}

async function getStatusText(username) {
  await wait(30)
  return 'away'
}

class NotLoggedInError extends Error {}

const wait = ms => new Promise(resolve => setTimeout(resolve, ms))

// TEST //

;(async function() {
  console.log('Grinch info:', await gatherUserInfo(12)) // Grinch info: { username: 'grinchMaster135', name: 'The Grinch', groups: [ 'group A', 'group B' ], statusText: 'away' }
  console.log('Non-logged in user info:', await gatherUserInfo(13)) // Non-logged in user info: null
})()
```

When you have multiple asynchronous requests to make, and you want them to operate in parallel, usually the appropriate tool to use is Promise.all. You can use it here to wait for the groups and the profile data at once, despite the fact that the two requests are pretty distinct from each other:
async function gatherUserInfo(userId) {
  try {
    const [groups, {username, name}] = await Promise.all([
      getGroups(userId),
      getProfile(userId),
    ])
    return {
      username,
      name,
      groups,
      statusText: await getStatusText(username),
    }
  } catch (err) {
    if (err instanceof NotLoggedInError) {
      return null
    }
    throw err
  }
}

This way, if any one of the Promises rejects, it will be being awaited at that time, so it'll be caught properly.

async function gatherUserInfo(userId) {
  try {
    const [groups, {username, name}] = await Promise.all([
      getGroups(userId),
      getProfile(userId),
    ])
    return {
      username,
      name,
      groups,
      statusText: await getStatusText(username),
    }
  } catch (err) {
    if (err instanceof NotLoggedInError) {
      return null
    }
    throw err
  }
}

async function getGroups(userid) {
  await wait(50)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return ['group A', 'group B'];
}

async function getProfile(userid) {
  await wait(30)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return { username: 'grinchMaster135', name: 'The Grinch' };
}

async function getStatusText(username) {
  await wait(30)
  return 'away'
}

class NotLoggedInError extends Error {}

const wait = ms => new Promise(resolve => setTimeout(resolve, ms))

// TEST //

;(async function() {
  console.log('Grinch info:', await gatherUserInfo(12)) // Grinch info: { username: 'grinchMaster135', name: 'The Grinch', groups: [ 'group A', 'group B' ], statusText: 'away' }
  console.log('Non-logged in user info:', await gatherUserInfo(13)) // Non-logged in user info: null
})()

You could also have fixed the unhandled rejection by adding a no-op .catch handler to the Promise that was created before awaiting the other Promise:
groupsRequest.catch(() => void 0);

But that makes the code less elegant - I think the Promise.all version is better.

async function gatherUserInfo(userId) {
  try {
    // Don't await yet, so this request can be sent at the same time as future requests.
    const groupsRequest = getGroups(userId)
    groupsRequest.catch(() => void 0);

    const {username, name} = await getProfile(userId)
    return {
      username,
      name,
      groups: await groupsRequest,
      statusText: await getStatusText(username),
    }
  } catch (err) {
    if (err instanceof NotLoggedInError) {
      return null
    }
    throw err
  }
}

async function getGroups(userid) {
  await wait(50)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return ['group A', 'group B'];
}

async function getProfile(userid) {
  await wait(30)
  if (userid !== 12) throw new NotLoggedInError('User not logged in')
  return { username: 'grinchMaster135', name: 'The Grinch' };
}

async function getStatusText(username) {
  await wait(30)
  return 'away'
}

class NotLoggedInError extends Error {}

const wait = ms => new Promise(resolve => setTimeout(resolve, ms))

// TEST //

;(async function() {
  console.log('Grinch info:', await gatherUserInfo(12)) // Grinch info: { username: 'grinchMaster135', name: 'The Grinch', groups: [ 'group A', 'group B' ], statusText: 'away' }
  console.log('Non-logged in user info:', await gatherUserInfo(13)) // Non-logged in user info: null
})()


Find all possible interpretations of an array of digits Consider a coding system for alphabets to integers where 'a' is
  represented as 1, 'b' as 2, .. 'z' as 26. Given an array of digits (1
  to 9) as input, write a function that prints all valid interpretations
  of input array.

    Input: {1, 1} Output: ("aa", 'k")  [2 interpretations: aa(1, 1), k(11)]

    Input: {1, 2, 1} Output: ("aba", "au", "la")  [3 interpretations: aba(1,2,1), au(1,21), la(12,1)]

    Input: {9, 1, 8} Output: {"iah", "ir"}  [2 interpretations: iah(9,1,8), ir(9,18)]

I'm looking for code-review, optimizations and best practices.  I'm also verifying space complexity to be \$O(2^n)\$, where \$n\$ is the number of alphabets is the length of input array.

public final class FormulateAlphabets {

    private static final String[] data = {
        "", "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n",
        "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"
    };

    private FormulateAlphabets() { }

    public static List<String> getAllStrings(int[] arr) {

        List<String> strList = new ArrayList<>();
        formulate(arr, 0,  false, strList, new StringBuffer());
        formulate(arr, 0,  true,  strList, new StringBuffer());

        return strList;
    }

    private static void formulate(int[] arr, int currPosition, boolean includeNextPosition, List<String> strlist, StringBuffer sb) {

        int nextPosition = 0;

        if (!includeNextPosition) {
            sb.append(data[arr[currPosition]]);
            nextPosition = currPosition + 1;
        } else {

            if ((currPosition + 1) == arr.length) return;

            int result = arr[currPosition] * 10 + arr[currPosition + 1];
            if (result <= 26) {
                sb.append(data[result]);
            } else {
                return;
            }

            nextPosition = currPosition + 2;
        }

        if (nextPosition == arr.length) {
            if (sb.length() > 0) {
                strlist.add(sb.toString());
                sb.deleteCharAt(sb.length()-1);
            }
            return;
        }

        formulate(arr, nextPosition, false, strlist, sb);
        formulate(arr, nextPosition, true, strlist, sb);

        if (sb.length() > 0) {
            sb.deleteCharAt(sb.length()-1);
        }
    }

}

public class FormulateAlphabetTests {

    @Test
    public void test1() {
        int[] a1 = {1, 2, 3, 4};
        assertEquals(Arrays.asList("abcd", "awd", "lcd"), FormulateAlphabets.getAllStrings(a1));
    }

    @Test
    public void test2() {
        int[] a2 = {1, 2, 2, 1};
        assertEquals(Arrays.asList("abba", "abu", "ava", "lba", "lu"), FormulateAlphabets.getAllStrings(a2));
    }

    @Test
    public void test3() {
        int[] a3 = {1, 2, 3};
        assertEquals(Arrays.asList("abc", "aw", "lc"), FormulateAlphabets.getAllStrings(a3));
    }

    @Test
    public void test4() {
        int[] a4 = {12, 23, 2};
        assertEquals(Arrays.asList("lwb"), FormulateAlphabets.getAllStrings(a4));
    }
}

test4 makes no sense. The question states "Given an array of digits (1 to 9) as input...", yet your test input is {12, 23, 2}.

As the digits are 1-9, there's no way to represent 'j' (10) or 't' (20). I'm guessing that's a mistake in the question.

  I'm also verifying space complexity to be \$O(2^n)\$

\$O(2^n)\$ is certainly an upper bound on the number of valid interpretations, but we can find a tighter bound.

Here's a hint: let's look at the number of interpretations of this sequence of inputs

{ }               1
{ 1 }             1
{ 1, 1 }          2
{ 1, 1, 1 }       3
{ 1, 1, 1, 1 }    5
...

Look familiar? I'm going to re-post part of an another answer of mine.

Suppose our array \$s = s_0 s_1 \ldots s_{n - 1}\$ is of length \$n\$. We define a function \$f\$ such that \$f(i)\$ is the number of ways to interpret the suffix of \$s\$ starting at position \$i\$: \$s_i s_{i + 1} \ldots s_{n - 1}\$. In this way, our final solution will be the value of \$f(0)\$.

How many ways are there to interpret the empty array? Just one, so we can write

\begin{align}
f(n) &= 1.
\end{align}

Now how about the suffix \$s_{n - 1}\$? Well if it's \$0\$, that's invalid; otherwise there's just one way to interpret it.

\begin{align}
f(n - 1) &= \begin{cases}
0 &\mbox{if } s_{n - 1} = 0, \\
1 & \mbox{otherwise}. \end{cases}
\end{align}

Now for the general case, the suffix \$s_i s_{i + 1} \ldots s_{n - 1}\$.

If \$s_i = 0\$, there are no interpretations. Otherwise, we have two cases to deal with.

If \$10 \leq s_i s_{i + 1} \leq 26\$, we can interpret the suffix as

\begin{align}
(s_i s_{i + 1}) \underbrace{s_{i + 2} \ldots s_{n - 1}}_{f(i + 2) \text{ interpretations}} \quad \mbox{ or } \quad
(s_i) \underbrace{s_{i + 1} s_{i + 2} \ldots s_{n - 1}}_{f(i + 1) \text{ interpretations}}.
\end{align}

Otherwise, we can only interpret the suffix as
\begin{align}
&(s_i) \underbrace{s_{i + 1} s_{i + 2} \ldots s_{n - 1}}_{f(i + 1) \text{ interpretations}}.
\end{align}

Putting that all together, we get
\begin{align}
f(i) &= \begin{cases}
0 &\mbox{if } s_{i} = 0, \\
f(i + 1) + f(i + 2) & \mbox{if } s_i = 1 \mbox{ or } (s_i = 2 \mbox{ and } s_{i + 1} \leq 6), \\
f(i + 1) & \mbox{otherwise.}
\end{cases}
\end{align}

So in the worst case, for example { 1, 1, 1, ..., 1 }, this reduces to the \$n\$th Fibonacci number, giving us space complexity \$O(F_n)\$.


LeetCode 273: Integer to English Words I'm posting my code for a LeetCode problem copied here. If you would like to review, please do so. Thank you for your time!
Problem

Convert a non-negative integer to its English words representation.
Given input is guaranteed to be less than 2^31 - 1.

Inputs
123
1234567891
151
1414312
1234
1241234113

Outputs
"One Hundred Twenty Three"
"One Billion Two Hundred Thirty Four Million Five Hundred Sixty Seven Thousand Eight Hundred Ninety One"
"One Hundred Fifty One"
"One Million Four Hundred Fourteen Thousand Three Hundred Twelve"
"One Thousand Two Hundred Thirty Four"
"One Billion Two Hundred Forty One Million Two Hundred Thirty Four Thousand One Hundred Thirteen"

Code
#include <string>

class Solution {
    std::string zero_to_twenty[20] = {"Zero", "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen", "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen"};
    std::string tens_and_zero[10] = {"Zero", "Ten", "Twenty", "Thirty", "Forty", "Fifty", "Sixty", "Seventy", "Eighty", "Ninety"};
    std::string SPACE = " ";
    std::string BILLION = "Billion";
    std::string MILLION = "Million";
    std::string THOUSAND = "Thousand";
    std::string HUNDRED = "Hundred";
    std::string ZERO = "Zero";
    std::string EMPTY_STRING = "";
    static inline constexpr int one_billion = 1e9;
    static inline constexpr int one_million = 1e6;
    static inline constexpr int one_thousand = 1e3;
    static inline constexpr int one_hundred = 100;
    static inline constexpr int twenty = 20;
    static inline constexpr int ten = 10;

public:
    inline std::string numberToWords(int num) {
        if (num == 0) {
            return ZERO;

        } else {
            std::string words = int2string(num);
            return words.substr(1, words.length() - 1);
        }
    }

private:
    inline std::string int2string(const int n) {
        if (n >= one_billion) {
            return int2string(n / one_billion) + SPACE + BILLION + int2string(n % one_billion);

        } else if (n >= one_million) {
            return int2string(n / one_million) + SPACE + MILLION + int2string(n % one_million);

        } else if (n >= one_thousand) {
            return int2string(n / one_thousand) + SPACE + THOUSAND + int2string(n % one_thousand);

        } else if (n >= one_hundred) {
            return int2string(n / one_hundred) + SPACE + HUNDRED + int2string(n % one_hundred);

        } else if (n >= twenty) {
            return SPACE + tens_and_zero[n / ten] + int2string(n % ten);

        } else if (n >= 1) {
            return SPACE + zero_to_twenty[n];

        } else {
            return EMPTY_STRING;
        }
    }
};

References

Problem

Solution

Discuss

Take a look at what translating
1241234113

results in when aligned slightly differently
                       One       Billion
Two  Hundred   Forty   One       Million
Two  Hundred   Thirty  Four      Thousand
One  Hundred           Thirteen

you can see that there are several patterns here.

Every number can be chunked into groups of 3 digits like this 1,241,234,113.

Each chunk has a different value for the last column, which is one of "Billion", "Million", etc.

Within each chunk, there is one digit at the hundreds place.

Within each chunk, the last 2 digits have a special case till 20, otherwise it's a special word for the ten's place, and the one's place.

So the only variables you need are just
std::string const till_twenty[20] = {"", "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen", "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen"};
std::string const tens_place[10] = {"", "Ten", "Twenty", "Thirty", "Forty", "Fifty", "Sixty", "Seventy", "Eighty", "Ninety"};
std::string const thousands[4] = {"", "Thousand", "Million", "Billion"};

Note that I've renamed the variables slightly, and made them const. Also, there's no need for the word Zero, since that will only be written if the entire number is 0.
Now you can build up the functions for the 3 digit chunks
//for the first digit
std::string hundreds_place(int n) {
   return n ? till_twenty[n] + " Hundred " : "";
}

// for the 2nd and 3rd digit
std::string ones_and_tens_place(int n) {
    return n <= 20 ? till_twenty[n] : tens_place[n / 10] +
                                      (n % 10 ? " " + till_twenty[n % 10] : "")
}

// putting all 3 digits together
std::string by_hundreds(int n) {
    return hundreds_place(n / 100) + ones_and_tens_place(n % 100);
}

Now you can recursively build the 3 digit numbers by keeping track of which power of 1000 is being currently processed
std::string by_thousands(int n, int i) {    // i keeps track of the chunk
    return n ? by_thousands(n / 1000, i + 1) +
               by_hundreds(n % 1000) + " "
               + thousands[i] + " "
             : "";
}

and then finally it can all be put together
std::string numberToWords(int num) {
    std::string res = num ? by_thousands(num, 0) : "Zero  ";
    // with a bit of processing to get rid of trailing spaces
    return res.substr(0, res.length() - 2);
}


Word frequency generator in Python Please critique my word frequency generator. I am a beginner programmer so any criticism are welcome.

Original Code: http://pastebin.com/rSRfbnCt

Here is my revised code after the feedback:

import string, time, webbrowser, collections, urllib2

def timefunc(function):
    def wrapped(*args):
        start = time.time()
        data = function(*args)
        end = time.time()
        timetaken = end - start
        print "Function: "+function.__name__+"\nTime taken:",timetaken
        return data
    return wrapped

@timefunc
def process_text(text_file):
    words = text_file.read().lower().split()
    words = [word.strip(string.punctuation+string.whitespace) for word in words]
    words = [word for word in words if word]#skips ''(None) elements
    return words

@timefunc
def create_freq_dict(wordlist):
    freq_dict = collections.Counter(wordlist)
    return freq_dict

@timefunc
def create_sorted_list(freqdict):
    sorted_list = [(value,key) for key,value in list(freqdict.items())]#list() provides python 3 compatibility
    sorted_list.sort(reverse=True)
    return sorted_list

@timefunc
def write_results(sorted_list):
    text_file = open('wordfreq.txt','w')
    text_file.write('Word Frequency List\n\n')
    rank = 0
    for word in sorted_list:
        rank += 1
        write_str = "[{0}] {1:-<10}{2:->10}\n".format(rank, word[1],word[0])
        text_file.write(write_str)
    text_file.close()

## The Brothers Grimm
## This file can be obtained from Project Gutenberg:
## http://www.gutenberg.org/cache/epub/5314/pg5314.txt
web_file = urllib2.urlopen('http://www.gutenberg.org/cache/epub/5314/pg5314.txt')

wordlist = process_text(web_file)
freqdict = create_freq_dict(wordlist)
sorted_list = create_sorted_list(freqdict)
results = write_results(sorted_list)

webbrowser.open('wordfreq.txt')

print "END"

import string, time, math, webbrowser

def timefunc(function, *args):
    start = time.time()
    data = function(*args)
    end = time.time()
    timetaken = end - start

I'd recommend calling this time_taken as its slightly easier to read.

    print "Function: "+function.__name__+"\nTime taken:",timetaken

Print already introduces newlines and combines different pieces. Take advantage of that.

   print "Function: ", function.__name__
   print "Time Taken: ", time_taken

That's easier to follow

    return data

def process_text(filename):

You never use filename in here, but you do use fin which is the same thing. typo?

    t = []

Not a very descriptive name. I suggest coming up with something clearer

    for line in fin:
        for i in line.split():

i usually means index which its not here

            word = i.lower()
            word = word.strip(string.punctuation)
            if word != '':
                t.append(word)
    return t

I'd do this as

 words = fin.read().lower().split()
 words = [word.strip() for word in words]
 words = [word for word in words if word]
 return words

I think its easier to follow and probably more efficient

def create_freq_dict(wordlist):
    d = dict()

d is not a very good name. Usually dicts are created with {} not dict(). No difference, but the first is generally preffered

    for word in wordlist:
        if word not in d.keys():
            d[word] = 1
        else:
            d[word] += 1

Use d = collections.defaultdict(int) or d = collections.Counter(). Both will make it easier to count up like this. See the python documentation for collections. You should actually be able to write this function in one line

    return d

def sort_dict(in_dict):
    t = []
    for key,value in in_dict.items():
        t.append((value, key))

in_dict.items() is a list already, there is no reason to copy the elements into the list. (NOTE: in Python 3.x in_dict.items() is no longer a list). Even if it wasn't a list you could do:

    t = list(in_dict.items())

Which would do the same thing your code does

    t.sort(reverse=True)
    return t

I'd implement this function as

return sorted(in_dict.items())

The sorted function takes anything sufficiently list-like and produces a sorted list from it.

def write_results(sorted_list):

sorted_list isn't a great name. It would be better to give an indication of what's in the list.

    fout = open('wordfreq.txt','w')
    fout.write('Word Frequency List\n\n')
    r = 0
    for i in sorted_list:
        r += 1

Use for r, (key, value) in enumerate(sortedlist): That way you don't need to manage r yourself, and you can refer the value as value rather then the harder to read i[1].

        fillamount = 20 - (len(i[1]) + len(str(r)))

Some of the those parens are unnecessary.

        write_str = str(r)+': '+i[1]+' '+('-' * (fillamount-2))+' '+str(i[0])+'\n'

Multiplication has precedence, you don't need the parens to make that happen. Also, python has a method ljust which does this for you

        write_str = (str(r) + ': ' + i[1]).ljust('-', 20) + str(i[0]) + '\n'

You may also want to consider using string formatting rather then adding strings

 write_str = ('%d: %s' % (r, i[1])).just('-', 20) + '%d\n' % i[0]

I think its easier to follow, although I'd probably split across several lines

        fout.write(write_str)
    fout.close()

## The Brothers Grimm
## This file can be obtained from Project Gutenberg:
## http://www.gutenberg.org/cache/epub/5314/pg5314.txt
fin = open('c:\Python27\My Programs\wx\grimm.txt')

fin presumable stands for file in. Give a name that indicates what's actually in it ike grim_text

wordlist = timefunc(process_text,fin)
freqdict = timefunc(create_freq_dict,wordlist)
sorted_list = timefunc(sort_dict,freqdict)
results = timefunc(write_results,sorted_list)

Very nice

webbrowser.open('wordfreq.txt')

That's a slightly unusual use of a webbrowser

print "END"


Bubble sort algorithm in Java I have written this code which basically sorts the array using bubble sort, however, I feel it is not very readable. Please let me know me how I can improve this code.

public class BubbleSort {

    public static void main(String[] args) {

        int arr[] = new int[] { 4, 1, 8, 5, 2, 0, 4, 3, 7, 9 };

        for (int i : arr) {
            System.out.print(i + " ");
        }

        for (int i = 1; i < arr.length; i++) {
            for (int j = 0; j < arr.length - i; j++) {
                if (arr[j] > arr[j + 1]) {
                    int temp = arr[j];
                    arr[j] = arr[j + 1];
                    arr[j + 1] = temp;
                }

            }

        }
        System.out.println();
        System.out.println("Sorted array");
        for (int i : arr) {
            System.out.print(i + " ");
        }
    }

}

Your code looks mostly good to me.

One thing: Functions can help increase readability of your code, and reduce the amount of duplicate code. Additionally, they make testing and (re-)using your code easier. Your main method should only be an entry point, it shouldn't really contain any logic itself.

Your main method might look like this:

public static void main(String[] args) {
    int[] array = new int[] { 4, 1, 8, 5, 2, 0, 4, 3, 7, 9 };

    printArray(array);
    int[] sortedArray = sortArray(array);
    printArray(sortedArray);
}

Then just create those functions.

Misc

You have some unnecessary newlines after }.
temp could be renamed swap, making it more obvious what is happening.
arr could be named array (arr may be fine in this context, but generally, you do not want to shorten your variable names, as it is less readable).


Python function to count syllables in a word I wrote this function to count the syllables in a word using Python to complete an exercise.

The exercise's text is:

  Return the number of syllables in the given word, according to this
  rule: Each contiguous sequence of one or more vowels is a syllable,
  with the following exception: a lone "e" at the end of a word is not
  considered a syllable unless the word has no other syllables. You
  should consider y a vowel.

I also disagree with the text because using the given instructions the syllable count for the word "example" are 2 and should be 3.

But, as discussed in the comments, the exercise text is the one that I implemented and would like to get suggestions and improvements for.

def count_syllables(word):
    word = word.lower()
    counter = 0
    is_previous_vowel = False
    for index, value in enumerate(word):
        if value in ["a", "e", "i", "o", "u", "y"]:
            if index == len(word) - 1:
                if value == "e":
                    if counter == 0:
                        counter += 1
                else:
                    counter += 1
            else:
                if is_previous_vowel == True:
                    counter += 1
                    is_previous_vowel = False
                    break
            is_previous_vowel = True
        else:
            if is_previous_vowel == True:
                counter += 1
            is_previous_vowel = False
    print ("In \"{}\" I count {} syllables".format(word, counter))

count_syllables("coding")

As a good practice to promote code reuse, the function should return a numeric result.  The print() should be done by the caller.

Constantly referring to is_previous_vowel is tedious. There are a couple of solutions you could use involving itertools. In particular, groupby() and zip_longest() could be useful.

Ultimately, the simplest solution is to use a regular expression to declare what you are looking for: one or more consecutive vowels ('[aeiouy]+'), but not an e at the end ('(?!e$)).

import re

def count_syllables(word):
    return len(
        re.findall('(?!e$)[aeiouy]+', word, re.I) +
        re.findall('^[^aeiouy]*e$', word, re.I)
    )


Copy part of array in javascript I have an array and must fill another one.

Depending of a boolean I copy first or second element and the rest must be as is...

Currently I do it like this (data has the desired info):

const newArray = [];
newArray[0] = theBoolean ? data[0] : data[1];
for (let i = 2; i < data.length; i += 1) {
  newArray[i - 1] = data[i];
}

I've tried with shift and slice but no success...

¿how can I beautify this snippet?

You're on the right track trying slice. Here's an example of using it with no loops required.

Initialisation

You could initialise newArray with an element directly in the declaration, removing the need for newArray[0] = ...:

const newArray = [theBoolean ? data[0] : data[1]];

Pushing all other elements from data to newArray:

You could use Array.prototype.slice to get all other elements: data.slice(2).

Using spread operator:

const newArray = [theBoolean ? data[0] : data[1]];
newArray.push(...data.slice(2));

// or, one liner:
const newArray = [theBoolean ? data[0] : data[1], ...data.slice(2)];

Using Array.prototype.concat:

const newArray = [theBoolean ? data[0] : data[1]].concat(data.slice(2));

Using Function.prototype.apply:

const newArray = [theBoolean ? data[0] : data[1]];
[].push.apply(newArray, data.slice(2));


Guess the chocolate box I'm new to this competitive programming world. This morning I came across this question, which required the time to be less than 1 sec. But my code runs for 1.000069 sec, thereby rejecting my solution.

Question:

  There are N chocolate boxes, and you will be given with a chocolate index for which we have to guess the correct box number. For example, The first box contain chocolates from 1 to N1 and the next box contain N1+1 through N2 and so on.

My Solution:

#include<iostream>
using namespace std;

//returns the sum of chocolates from 0 to i

int getSum(int i,int *array){
    int sum = 0;
    for(int j=0; j< i; j++){
        sum += array[j];
    }
    return sum;
}
int main(){
    int n,q;
    //Get the number of chocolate boxes.
    cin >> n;
    int chocolatesInEachBox[n];
    //Get the number of chocolates in each box.
    for(int i =0; i < n; i++){
        cin >> chocolatesInEachBox[i];
    }
    //Get the number of queries.
    cin>>q;
    int chocolateIndexes[q];
    //Get each query.
    for(int i =0; i < q; i++){
        cin >> chocolateIndexes[i];
    }
    int j=0,i=0;

    while(j < q && i<n){

        if((chocolateIndexes[j]-getSum(i+1,chocolatesInEachBox))>0)
        i++;

        else{
            cout<<i+1<<endl;
            j++;
            i=0;
        }
    }
}

Performance

The getSum function calculates the sum of the values in the given array up to the specified index. As you calculate the result to multiple queries, each using the same array as the parameter, the same calculation is repeated multiple times, which is certainly the cause of the slowness. The time complexity of  getSum is \$O(n)\$, and of the entire solution it's \$O(n^2)\$.

You could improve the performance greatly by pre-computing an array of so-called prefix sums: the sums of elements up to each index. For example if the input array contains 7 3 1 5 5 then the array of prefix sums will contain 0 7 10 11 16 21. This can be done in a single pass, in \$O(n)\$ time.

With this change, the time complexity of  getSum would become \$O(1)\$, and of the entire solution  \$O(n)\$, significantly faster than the original solution.

Style issues

Try to organize your program into multiple functions with a single responsibility. Like getSum, that was well done, but the rest of the program should be split up to smaller logical units.

In C++ you don't need to declare variables at the top of functions, like you did for q. It's better to declare right before you need it.

Lastly, note that using namespace std is generally not recommended.


Search dictionary by value I have a dictionary which I use to lookup my binary and ASCII values (from A - P). It works, but my ascii2bin function just doesn't really feel right (and beautiful). Is there is a better way to write it?

The ascii2bin function splits a 2-letter string (i.e. 'LA') and looks up the keys in the dictionary. Then concatenates the 2 keys to each other to get an 8bit string.

Check the comment for the assignment which was given.

import sdes

"""
Using S-DES, decrypt the string (10100010) using the key (0111111101) by hand.
Then decode the 4 bits of the plaintext string to a letter and the second 4 bits to another letter where
we encode A through P in base 2 (i.e., A = 0000, B = 0001, ..., P = 1111).
"""

KEY = '0111111101'
CIPHERTEXT = '10100010'

ALPHABET = {'0000': 'A',
            '0001': 'B',
            '0010': 'C',
            '0011': 'D',
            '0100': 'E',
            '0101': 'F',
            '0110': 'G',
            '0111': 'H',
            '1000': 'I',
            '1001': 'J',
            '1010': 'K',
            '1011': 'L',
            '1100': 'M',
            '1101': 'N',
            '1110': 'O',
            '1111': 'P'}

def bin2ascii(bin):
    return ALPHABET[bin[:4]] + ALPHABET[bin[4:]]

def ascii2bin(string_of_2_letters):
    bits = []
    for i in range(0, 2):
        bits += ([key for key, val in ALPHABET.items() if val == string_of_2_letters[i]])
    return bits[0] + bits[1]

print bin2ascii(sdes.decrypt(CIPHERTEXT, KEY))

Give more sensible names

ALPHABET = {'0000': 'A',
            '0001': 'B',

This is plain wrong, that variable really is not only an alphabet. I usually call dicts x_to_y and I think it is a sensible idea to do so, my suggestion is BINARY_TO_LETTER

Just reverse the dict

def reverse_dict(d):
    return {d[i]:i for i in d}

LETTER_TO_BINARY = reverse_dict(BINARY_TO_LETTER)


Java - Object declaration out of while loop Could you explain to me which code is better (more optimal) and why?

Vesion 1:

while(something){
Runnable task = (Runnable) taskQueue.dequeue();
Throwable  ex = null;
...
}

Version 2:

Runnable task;
Throwable ex;

while(something){
task = (Runnable) taskQueue.dequeue();
ex = null;
...
}

For me it looks like second version is optimized because variable declaration is out of while loop.

At the bytecode level, there is no such thing as inner block. All local variables are declared at the scope of the method. As a result, both your variants should produce equivalent bytecode. To make sure, decompile them with javap and look at the bytecode. From the programmer's point of view, version 1 is more clear and concise.


Regex match comments and quotes UPDATE

New regex - covers escaped slashes and uses lazy qualifiers:

/(?>(')(?>(?>\\\\)|\\'|.)*')|(?>(")(?>(\\\\)|(?>\\")|.)*")|(?>((?>\/\/|#)).*^)|(?>(\/\*).*\*\/)/msU

Here are the different parts:

Single quotes: (?>(')(?>(?>\\\\)|\\'|.)*')

Double quotes: (?>(")(?>(\\\\)|(?>\\")|.)*")

Single line comments: (?>((?>\/\/|#)).*^)

Multiline comments: (?>(\/\*).*\*\/)

I moved the html comments to a different regex because html will be handled differently.

Here's an example of what it does: (The matched parts are bold)

//single line comment

random text ... #another comment

Multiline comments:

/* this is a multiline comment

matches the whole thing
*/

Quotes:

"also matches strings with \" escaped quotes or 'the other kind of quotation marks in it' "

'matches the end quote because it it not escaped \\'

This is the original post:

I'm working on making syntax highlighting on my blog, and I created a regex to match strings and comments (I'm going to make a callback function to format each differently)

Here's my regex:

(?>(")(?>[^"]|(?>(?<=\\)"))*")|(?>(')(?>[^']|(?<=\\)')*')|(?>((?>\/\/|#))[^\n\r]*)|(?>(\/\*)(?>.(?!\*\\))*\*\/)|(?>(<!--)(?>.(?!-->))*.-->)

I tested it and it seemed to work, but is there anything I'm missing or an easier  way to do it? It currently matches strings ("...", '...') and takes into account escaped chars and strings within other strings. It also matches comments (//..., #..., /*...*/, <!--...-->).

Here's what I'm using each part of the regex for:

Part 1: (?>(")(?>[^"]|(?>(?<=\\)"))*"): Match strings quoted with "

Part 2: (?>(')(?>[^']|(?<=\\)')*'): Match strings quoted with '

Part 3: (?>((?>\/\/|#))[^\n\r]*): Match single line comments # or //

Part 4: (?>(\/\*)(?>.(?!\*\\))*\*\/): Match multiline comments /*...*/

Part 5: (?>(<!--)(?>.(?!-->))*.-->): Match html comments <!--...-->

If it is PHP source code you want to highlight, why not use the tokenizer that's build into PHP itself? I use that and it seems to work fine:

$tokens = token_get_all($sourceCode);

Then all you have to do is walk through all the tokens and give them a color.

See: https://www.php.net/manual/en/function.token-get-all.php

No extra library, no faffing around with endless regular expressions.