Pulling financial data via IEX Cloud API I am pulling some financial data from the IEX Cloud API. Since it is a paid API w/ rate limits, I would like to do this as efficiently as possible (i.e., as few calls as possible). Given that the tickers list is likely to have thousands of indexes (oftentimes with duplicates), is there a better/more efficient way than what I'm currently doing?

(Feel free to use the token in the code, as there is no limit to the number of API calls that you can make to sandboxed data.)

import os
from iexfinance.stocks import Stock
import iexfinance
import pandas as pd

# Set IEX Finance API Token (Test)
os.environ['IEX_API_VERSION'] = 'iexcloud-sandbox'
os.environ['IEX_TOKEN'] = 'Tsk_5798c0ab124d49639bb1575b322841c4'

# List of ticker symbols
tickers = ['MSFT', 'V', 'ERROR', 'INVALID', 'BRK.B', 'MSFT']

# Create output Dataframe
output_df = pd.DataFrame(columns=['Net Income', 'Book Value'])

# Loop through companies in tickers list
for ticker in tickers:

    # Call IEX Cloud API, and output to Dataframe
    try:
        company = Stock(ticker, output_format='pandas')

        try:
            # Get income from last 4 quarters, sum it, and store to temp Dataframe
            df_income = company.get_income_statement(period="quarter", last='4')
            df_income['TTM'] = df_income.sum(axis=1)
            income_ttm = int(df_income.loc['netIncome', 'TTM'])

            # Get book value from most recent quarter, and store to temp Dataframe
            df_book = company.get_balance_sheet(period="quarter")
            book_value = int(df_book.loc['shareholderEquity'])

        # If IEX Cloud errors, make stats = 0
        except (iexfinance.utils.exceptions.IEXQueryError, TypeError, KeyError) as e:
            book_value = 0
            income_ttm = 0

        # Store stats to output Dataframe
        output_df.loc[ticker] = [income_ttm, book_value]

    except ValueError:
        pass

print(output_df)

This seems mostly to be an API question?  Assuming all that data for
each of the companies is required and there's no combined API endpoint
that can be fed with, say, 10s or 100s of symbols, this would seem to
be the best way.  If so, there'd still be possible concurrent execution
options to make multiple API requests at the same time and possible get
more throughput with it, though the number of API calls would still be
the same.

The streaming endpoints looking interesting at first, but they don't support the data here; any other endpoints all look like they only accept single symbols.

What you should definitely do is create a (few) function(s) and ideally
get rid of the try blocks.  From a first look I've zero idea where
stuff gets thrown.  But I'm guessing mostly keys not being present.
Preferably this could be handled directly (with default values or
similar features) that do not require lots of exception handling.  Yes,
Python might encourage this, but simply catching ValueError around a
huge block will eventually catch unintended exceptions too.

So at least something like this would be a good start:

def fetch_company_info(ticker):
    company = Stock(ticker, output_format='pandas')

    book_value = 0
    income_ttm = 0

    try:
        # Get income from last 4 quarters, sum it, and store to temp Dataframe
        df_income = company.get_income_statement(period="quarter", last=4)
        df_income['TTM'] = df_income.sum(axis=1)
        income_ttm = int(df_income.loc['netIncome', 'TTM'])

        # Get book value from most recent quarter, and store to temp Dataframe
        df_book = company.get_balance_sheet(period="quarter")
        book_value = int(df_book.loc['shareholderEquity'])

    # Ignore IEX Cloud errors
    except iexfinance.utils.exceptions.IEXQueryError:
        pass

    return income_ttm, book_value

I also shuffled the variables a bit around - with the right order this
could even be type checked ...

In any case, I can't see anything wrong here and I've removed the other
two exceptions as they look more like hiding problems than actual proper
handling of the exact problem that was encountered.  With the example
names I also couldn't trigger them at all.

The main body of the script is now rather small:

# Loop through companies in tickers list
for ticker in tickers:
    # Call IEX Cloud API, and store stats to Dataframe
    output_df.loc[ticker] = fetch_company_info(ticker)

print(output_df)

If there's no need for reusability then that's it, otherwise I'd suggest
a main function and moving all currently global definitions into it,
then have some argument parser or configuration file handling, etc. to
make it a more fully grown solution.

After some discussion in the comments it turns out that splitting the
return values from the assignment can lead to easy mistakes like the
order of the values being wrong.  With that in mind, either the output_df
global should be kept in the function, or (better) be passed in as
a variable instead:

for ticker in tickers:
    fetch_company_info(ticker, output_df)

and

def fetch_company_info(ticker, output_df):
    ...
    output_df[ticker] = [income_ttm, book_value]


Return a sorted ordering of courses such that we can finish all courses The task:

  We're given a hashmap associating each courseId key with a list of
  courseIds values, which represents that the prerequisites of courseId
  are courseIds. Return a sorted ordering of courses such that we can
  finish all courses.

  Return null if there is no such ordering.

  For example, given {'CSC300': ['CSC100', 'CSC200'], 'CSC200':
  ['CSC100'], 'CSC100': []}, should return ['CSC100', 'CSC200',
  'CSCS300'].

My solution:

const isEveryCourseCovered = courses => courses.every(c => !Boolean(c));
const getCourseOrder = courses => {
  const courseSubjects = Object.keys(courses);
  const courseRequirements = Object.values(courses);
  const courseOrder = [];

  do {
    courseSubjects.map((subject, i) => {
      if (subject) {
        courseRequirements[i].map((requirement, idx) => {
          if (courseOrder.includes(requirement)) {
            courseRequirements[i].splice(idx, 1);
          }
        });
        if (!courseRequirements[i].length) {
          courseOrder.push(subject);
          delete courseSubjects[i];
        }
      }
    });
  } while(!isEveryCourseCovered(courseSubjects));

  return courseOrder;
};

const coursesObj = {'CSC300': ['CSC100', 'CSC200'], 'CSC200': ['CSC100'], 'CSC100': []};
console.log("result ", getCourseOrder(coursesObj));

Objects are passed by "copy of a reference".

Notice that the coursesObj after the function call has empty values ( see snippet below )

const isEveryCourseCovered = courses => courses.every(c => !Boolean(c));
const getCourseOrder = courses => {
  const courseSubjects = Object.keys(courses);
  const courseRequirements = Object.values(courses);
  const courseOrder = [];

  do {
    courseSubjects.map((subject, i) => {
      if (subject) {
        courseRequirements[i].map((requirement, idx) => {
          if (courseOrder.includes(requirement)) {
            courseRequirements[i].splice(idx, 1);
          }
        });
        if (!courseRequirements[i].length) {
          courseOrder.push(subject);
          delete courseSubjects[i];
        }
      }
    });
  } while (!isEveryCourseCovered(courseSubjects));

  return courseOrder;
};

const coursesObj = {
  'CSC300': ['CSC100', 'CSC200'],
  'CSC200': ['CSC100'],
  'CSC100': []
};
console.log("result ", getCourseOrder(coursesObj));

console.log("Courses Object : ", coursesObj);

You should create a copy of the passed object, a quick way ( one of many ways ) of doing that is JSON.parse(JSON.stringify(c));
( see snippet below ):

const getCourseOrder = c => {
  // make a copy of the input object
  const courses = JSON.parse(JSON.stringify(c));

const isEveryCourseCovered = courses => courses.every(c => !Boolean(c));
const getCourseOrder = c => {
  // make a copy of the input object
  const courses = JSON.parse(JSON.stringify(c));

  const courseSubjects = Object.keys(courses);
  const courseRequirements = Object.values(courses);
  const courseOrder = [];

  do {
    courseSubjects.map((subject, i) => {
      if (subject) {
        courseRequirements[i].map((requirement, idx) => {
          if (courseOrder.includes(requirement)) {
            courseRequirements[i].splice(idx, 1);
          }
        });
        if (!courseRequirements[i].length) {
          courseOrder.push(subject);
          delete courseSubjects[i];
        }
      }
    });
  } while (!isEveryCourseCovered(courseSubjects));

  return courseOrder;
};

const coursesObj = {
  'CSC300': ['CSC100', 'CSC200'],
  'CSC200': ['CSC100'],
  'CSC100': []
};
console.log("result ", getCourseOrder(coursesObj));

console.log("Courses Object : ", coursesObj);

I didn't test the performance but i would guess it wouldn't be that efficient since it has 5 nested loops (do while, .map, .map, .includes) and .every that's called on every iteration of the while loop and 3 ifs

Reading the requirement :

  Return a sorted ordering of courses

Array.sort() was the first thing to come to my mind, my suggested approach would be simply sorting the Object.keys of the coursesObj based on the length of the value ( array of required courses) corresponding to that key, like so :

const coursesObj = {
  'CSC300': ['CSC100', 'CSC200'],
  'CSC400': ['CSC100', 'CSC200', 'CSC300'],
  'CSC200': ['CSC100'],
  'CSC100': []
};

const result = Object.keys(coursesObj).sort((a, b) =>
  coursesObj[a].length - coursesObj[b].length
);

console.log(result)

PS : i added the CSC400 key in the middle just for the demo.

EDIT :

If you have an Object like :

const coursesObj =
    {
      c1: ['c2', 'c3', 'c4'],
      c5: ['c1'],
      c4: [],
      c2: [],
      c3: ['c4']
    }

you can transform it to add the prerequisites of the courses in its prerequisites
 array so it becomes :

const detailed =
   {
     c1: [ 'c2', 'c3', 'c4' ],
     c5: [ 'c1', 'c2', 'c3', 'c4' ], // c5 now has c1 and the prerequisites of c1
     c4: [],
     c2: [],
     c3: [ 'c4' ]
   }

then apply the same sort function above :

const coursesObj = { c1: ["c2", "c3", "c4"], c5: ["c1"], c4: [], c2: [], c3: ["c4"] };

const detailedObj = Object.entries(coursesObj).reduce((accumulator, [courseId, requirements]) => {
  accumulator[courseId] = requirements.slice(0); // original courses required.

  requirements.forEach(course => {
    accumulator[courseId].push(...coursesObj[course]); // add the required courses for each required course .
  });

  // remove the duplicates
  accumulator[courseId] = [...new Set(accumulator[courseId])];

  return accumulator;
}, {});

// sort by the number of prerequisites
const result = Object.keys(detailedObj).sort((a, b) =>
  detailedObj[a].length - detailedObj[b].length
);

console.log(result);


Calculating men's risk of getting coronary heart disease I'd like anyone here to help me clean up my code. It's about calculating men's risk of getting coronary heart disease. Here's the link to my method.

private void calculateForMen(){

 // TODO
 // For Men
 // 20–34 years: Minus 9 points.
 // 35–39 years: Minus 4 points.
 // 40–44 years: 0 points.
 // 45–49 years: 3 points.
 // 50–54 years: 6 points.
 // 55–59 years: 8 points.
 // 60–64 years: 10 points.
 // 65–69 years: 11 points.
 // 70–74 years: 12 points.
 // 75–79 years: 13 points.

        // TODO Men Age 20-34
        if( strAge.equals("20 to 34") ){ // Minus 7 points

            // First point
            points = points - 9;

            // Second point
            if( strTotalCholesterol.equals("160 to 199") ){// 4 points
                points = points + 4;
            }
            else if( strTotalCholesterol.equals("200 to 239") ){ // 8 points
                points = points + 7;
            }
            else if( strTotalCholesterol.equals("240 to 279") ){ // 11 points
                points += 9;
            }
            else if( strTotalCholesterol.equals("280 or greater") ){ // 13 points
                points += 11;
            }

            // Third point
            if( strHDL.equals("60 or greater") ) // Minus 1 point
            {
                points -= 1;
            }
            else if( strHDL.equals("40 to 49") ){ // 1 point
                points += 1;
            }
            else if( strHDL.equals("Less than 40") ){ // 2 points
                points += 2;
            }

            // Fourth point
            if( strSBP.equals("130 to 139") ){ // 2 points
                points += 1;
            }
            else if( strSBP.equals("140 to 159") ){ // 3 points
                points += 1;
            }
            else if( strSBP.equals("160 or greater") ){ // 4 points
                points += 2;
            }

Each of your if blocks are very similar. You basically the same thing in each if block. This a sign that you've got the conditionally at the wrong level. Instead of writing code that does:

If age category 1

Add points for age
Add points for cholestoral
etc

If age category 2

Add points for age
Add points for cholestoral
etc

etc

Write it like this:

Add points for age

Handle age category 1
Handle age category 2
etc

Add points for cholestoral

Handle age category 1
Handle age category 2
etc

etc

So adding the age points should look something like:

if( strAge.equals("20 to 34") ){ // Minus 7 points
    points = points - 9;
}
else if( strAge.equals("35 to 39") ){
    points = points - 4;
}
else if( strAge.equals("35 to 39") ){
    points = points - 4;
}
else if( strAge.equals("40 to 44") ){
    points += 0;
}
else if( strAge.equals("45 to 49") ){
    points += 3;
}
else if( strAge.equals("50 to 54") ){
    points += 6;
}
else if( strAge.equals("55 to 59") ){
    points += 8;
}
else if( strAge.equals("60 to 64") ){
    points += 10;
}
else if( strAge.equals("65 to 69") ){
    points += 11;
}
else if( strAge.equals("70 to 74") ){
    points += 12;
}
else if( strAge.equals("75 to 79") ){
    points += 13;
}

But doesn't that repeat the .equals way too much? Yes. But the repetition here is much simpler and easier to deal with then what you had before. So we prefer this. Once we have this we can simplify it.

The next key is to use data not code. That is, make lists of things and look them up rather then putting them in code.

static String [] ageCategories = {"20 to 34", "35 to 39", "40 to 44", "45 to 49", "50 to 54", "55 to 59", "60 to 64", "64 to 69", "70 to 74", "75 to 79"}
static int agePoints = {-9, -4, -4, 0, 3, 6, 8, 10, 11, 12, 13};

points += agePoints[Arrays.asList(ageCategories).indexOf(strAge)];

This looks up the age level and the points by looking in the arrays. No ifs required. You should basically not need to use any if statements whatsoever in this particular code. You should rewrite it to fetch everything out of arrays like this.

As the number of age ranges increases, it may be easier to work with a map because each point adjustment is matched up to its corresponding age range.

static final Map<String, Integer> pointsByAgeRange = new HashMap<>() {{
    put("20 to 34", -9);
    put("35 to 39", -4);
    ...
    put("75 to 79", 13);
}};

points += pointsByAgeRange.get(strAge);  // assumes range is in map


Tagging the directories and switching between them by tags - follow-up (Part 2/2: business logic) This is the bash script responsible for changing the directories via textual tags. (See here the entire project.)

dt_script

~/.dt/dt $@ > ~/.dt/dt_tmp
COMMAND_TYPE=$(head -n 1 ~/.dt/dt_tmp)
if [ $# -eq 0 ]; then
    NEXT_PATH=$(~/.dt/dt prev | tail -n 1) # Let dt return pwd if no prev tag.
    ~/.dt/dt --update-prev $(pwd)
    if [ "$COMMAND_TYPE" == "message" ]; then
        tail -n 1 ~/.dt/dt_tmp
    else
        cd $NEXT_PATH
    fi
else
    if [ "$COMMAND_TYPE" == "switch_directory" ]; then
        NEXT_PATH=$(tail -n 1 ~/.dt/dt_tmp)
        ~/.dt/dt --update-prev $(pwd)
        cd $NEXT_PATH
    elif [ "$COMMAND_TYPE" == "show_tag_entry_list" ]; then
        tail -n +2 ~/.dt/dt_tmp
    elif [ "$COMMAND_TYPE" == "message" ]; then
        tail -n 1 ~/.dt/dt_tmp
        #echo $(cut -d " " -f 2)
    else
        echo "Unknown command: $COMMAND_TYPE"
    fi
fi
rm ~/.dt/dt_tmp

Any further suggestions for improvement are welcomed.

Some suggestions:

Use More Quotes™

$@ should be double quoted to pass the parameters without word splitting.
Any variable use should be quoted. It is very rare that word splitting is actually what you want, and it causes sometimes very subtle bugs.
Command substitution should be quoted.

Use good bashisms such as [[ instead of [.
Local variables should be lowercase to distinguish them from system variables, which are uppercase.
There is no need for semicolons in Bash scripts. This may be more contentious, but I generally write like this:

if [[ "$variable" = 'value' ]]
then
    …

Apropos: == to compare strings is a bit of a historical accident. The original operator is just =, but == is probably not going away.
$(pwd) can be simplified to . - it also means the current directory.
The home directory should not be used for temporary file storage (unless that is actually your configured temporary directory). In general you should use something like working_dir="$(mktemp --directory)" (or -d if you don't have GNU coreutils mktemp). This has a few advantages:

It means your home directory isn't going to fill up with temporary files which are never cleaned up (but more about that below).
It is often a memory mapped filesystem, so it may be much faster than your home directory.

Long if/elif sections testing the value of a single variable can usually be easier written as a case block.
head does not fail if run on an empty file:

$ touch foo
$ head foo
$ echo $?
0

So your first test could fail for the wrong reason. You might want to [[ -s "$path" ]] to check if a file is empty.
The file name "dt_tmp" doesn't tell me anything about what it contains or what it's used for - in general I find that adding the project name to anything within the project is redundant, and marking something as temporary is not particularly helpful to know what it is, unless it really can contain anything. Is there a better name you can give it? It looks like it's some sort of command queue, maybe?


Python: Speeding up vector and matrix operations The core of my program are the following lines

import numpy as np
import time

I, J = np.random.randn(20000,800), np.random.randn(10000,800)

d = np.zeros((len(I),1))
for j in range(len(J)):
    t0 = time.time()
    # Compute distance
    matrix_j = np.array([J[j]]).T*np.array([J[j]])
    for i in range(len(I)):
        matrix_i = np.array([I[i]]).T*np.array([I[i]])
        d[i] = np.sum((matrix_i-matrix_j)**2)
    print(time.time()-t0)
    # Do stuff with vector d

I and J are some matrices with data. I basically compare vectors of each matrix where I first extract a vector and than multiply it with its transposed version to get a matrix matrix_i and matrix_j. Afterwards I subtract both matrices from each other and sum all elements. But it takes ages even for such a small data set.

What can I do to speed things a little bit up? Any ideas?

What you compute with the presented code is

matrix_j = array([ J[j,k]**2 for k in range(N) ])

(where N=800 is the vector length) so that

d[i] = sum( (J[j,k]**2 - I[i,k]**2)**2 for k in range(N) )

This does not make much sense geometrically.

What you could have intended, by what might be implied by the formulas and variable names, is to have J[j] and I[i] as actual row vectors which can be achieved by transforming the tables I,J into matrices,

I,J = np.matrix(I), np.matrix(J)

so that then d[i] is the square of the Frobenius norm of J[j].T*J[j]-I[i].T*I[i], which is also the sum over the squares over all matrix elements. As this matrix is symmetric, this quantity can be computed as

d[i] = np.sum(np.diag( (J[j].T*J[j]-I[i].T*I[i])**2 ))

which has the form for row vectors \$a,b\$ of equal dimension
\begin{align}\|a^Ta-b^Tb\|_F^2 &= \text{trace}((a^Ta-b^Tb)^2)\\ &=\text{trace}((a^Ta-b^Tb)(a^Ta-b^Tb))\\ &=\text{trace}(a^Taa^Ta-a^Tab^Tb-b^Tba^Ta+b^Tbb^Tb))\\ &=(aa^T)^2-2(ab^T)^2+(bb^T)^2\\ &=\|a\|^4-2\langle a,b\rangle ^2+\|b\|^4 \end{align}

Re-expressed in the rows of the original matrices, that should be 2D-arrays, not converted to matrix objects, this is

d[i] = np.dot(I[i],I[i])**2 - 2*np.dot(I[i],J[j])**2 + np.dot(J[j],J[j])**2

For a component-wise derivation see the answer of Gareth Rees.

These norm squares and scalar products should be faster to compute than the original formula. Also, pre-computing the norm squares np.dot(I[i],I[i]) allows re-use so that all len(I)+len(J) norm squares are only computed once and the main computational cost is from the len(I)*len(J) scalar products.

scipy.linalg.norm might be faster in computing the norm, as squaring can be faster than multiplying the same number to itself, but I'm not sure how that translates over the several layers of interpretation and data encapsulation.

The scalar products are elements of a matrix product of I and J.T, so that a compact computation proceeds by (all are np.array objects)

dotIJ = np.dot(I,J.T);
normI2 = [ np.dot(row,row) for row in I ];
normJ2 = [ np.dot(row,row) for row in J ];
d = np.zeros((len(I),1));
for j in range(len(J)):
    d[:,0] = [ normI2[i]**2 + normJ2[j]**2 - 2*dotIJ[i,j]**2 for i in range(len(I)) ]
    # process values of d


Calculating pi by adding areas of thin rectangles I wrote a small program for fun to try to prove Pi by taking a certain precision and radius and using it to calculate the area of the circle. My method should be giving me an area that is just slightly larger than the actual area of the circle, approaching the actual area as the precision gets smaller.

The funny thing is that this area should always be larger than the actual area, but when I set the precision to be less than 1.0e-9L (say 1.0e-10L), I get a result smaller than 3.141592654. Can you look at my code and tell me where I messed up if anywhere?

#include <iostream>
#include <iomanip>
#include <cstdlib>
#include <cmath>
#include <future>

using namespace std;

namespace Settings{
    register constexpr long double precision = 1.0e-10L;
    register constexpr long double radius = 1.0e0L;
}

// Make the math cleaner
inline long double SQUARE(register long double Val) { return Val * Val; }

// Main calculate function
long double get_Area(register long double radius = Settings::radius);

int main()
{
    time_t benchmark = time(0);
    register long double area = get_Area();
    register long double pi = area / SQUARE(Settings::radius);

    cout << "Pi should be around:" << setprecision(50) << pi << endl << endl;
    cout << "It took " << time(0) - benchmark << " to complete the calculation" << endl;
    system("pause");

    return 0;
}

long double get_Area(register long double radius)
{
    register long double area = 0.0e0L, max_Area = SQUARE(radius);

    // Allow for threading
    register auto part = [](register long double start_point, register long double end_point, register long double m_Area){
        register long double area = 0.0L, y = 0.0L;

        for ( register long double x = start_point; x < end_point; x += Settings::precision ){
            y = sqrt(m_Area - SQUARE(x));
            area += y * Settings::precision;
        }

        return area;
    };

    // Divide the problem into eight parts since my pc has eight threads
    future<long double> A1 = async(launch::async, part,                   0.0L, (1.0L / 8.0L) * radius, max_Area);
    future<long double> A2 = async(launch::async, part, (1.0L / 8.0L) * radius, (2.0L / 8.0L) * radius, max_Area);
    future<long double> A3 = async(launch::async, part, (2.0L / 8.0L) * radius, (3.0L / 8.0L) * radius, max_Area);
    future<long double> A4 = async(launch::async, part, (3.0L / 8.0L) * radius, (4.0L / 8.0L) * radius, max_Area);
    future<long double> A5 = async(launch::async, part, (4.0L / 8.0L) * radius, (5.0L / 8.0L) * radius, max_Area);
    future<long double> A6 = async(launch::async, part, (5.0L / 8.0L) * radius, (6.0L / 8.0L) * radius, max_Area);
    future<long double> A7 = async(launch::async, part, (6.0L / 8.0L) * radius, (7.0L / 8.0L) * radius, max_Area);
    future<long double> A8 = async(launch::async, part, (7.0L / 8.0L) * radius,                 radius, max_Area);

    area = A1.get() + A2.get() + A3.get() + A4.get() + A5.get() + A6.get() + A7.get() + A8.get();

    return area * 4.0L;
}

I see a number of things that could help you improve your program.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.  Know when to use it and when not to (as when writing include headers).  In this particular case, I happen to think it's perfectly appropriate because it's a single short program that and not a header. Some people seem to think it should never be used under any circumstance, but my view is that it can be used as long as it is done responsibly and with full knowledge of the consequences.

Don't use system("pause")

There are two reasons not to use system("cls") or system("pause").  The first is that it is not portable to other operating systems which you may or may not care about now.  The second is that it's a security hole, which you absolutely must care about.  Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything.  First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system.  Then rewrite the contents of those functions to do what you want using C++.  For example:

void pause() {
    getchar();
}

Don't use register

As of C++11, the use of the register keyword is deprecated. In C++17, it will be gone, so it's wise to avoid its use.

Be careful with floating point

Adding a small number to a large number using floating point is risky because in certain cases, as potentially with your for loop within get_Area(). Specifically, with floating point numbers, adding a large number to a small number can lead to loss of precision.  In extreme cases, if a is very large and b is very small, we can have a + b = a which is obviously incorrect for non-zero values of b. (See the oft-cited and still excellent What Every Computer Scientist Should Know About Floating-Point Arithmetic, by David Goldberg for a readable technical discussion for why this is so.)  I should mention that it will not happen with this particular code with these particular values, but it's a potential problem that we programmers should all understand.

Consider using portable scalability

The choice of 8 parts is just fine, but you might consider using std::thread::hardware_concurrency for a portable way to scale on multiprocessor systems.

Don't repeat yourself

Rather than calculating the low and high values for each slice manually, and naming each std::future by hand, have the computer do it instead:

const unsigned slices = thread::hardware_concurrency();
vector<future<long double>> A(slices);
const long double increment = radius / slices;
long double lo = 0;
long double hi = increment;
for (unsigned i=0; i<slices; ++i) {
    A[i] =  async(launch::async, part, lo, hi, max_Area);
    lo += increment;
    hi += increment;
}
return std::accumulate(A.begin(), A.end(), 0.0L,
    [](long double accum, std::future<long double> &f){
        return accum + f.get();
    }) * 4;

Extend the accuracy

Since you're using a simple Riemann sum, on the quarter circle you've chosen the sum will indeed always be greater than the actual value.  However, we can very simply take advantage of the symmetry of the problem and have that error mostly cancel out by summing the entire top half of the circle.  That is, instead of $$\pi = 4\int_0^{r} \sqrt{r^2 - x^2}\delta x$$ we can instead compute $$\pi = 2\int_{-r}^{r} \sqrt{r^2 - x^2}\delta x$$.

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.


Getting a scrabble score from a word My program is complete and runs, and gives me the output I expect. For example, if I enter the string "HELLO" I should get 8. Are there any improvements that can be made?

#include "stdafx.h"
#include <iostream>
#include <cctype>
#include <string>
using namespace std;

int gameScore(string word);
int letterValue(char ch);

int main()
{
    while(true)
    {
        char word[80];
        cout << "Enter a word: ";
        cin.getline(word, 80);

        if(word[0] == ' ')
            break;

        int score = gameScore(word);
        cout << "The score for '" << word << "' is " << score << "\n";
    }

    system("PAUSE");
    return 0;
}

int gameScore(string word)
{
    int total = 0;
    int length = word.length();

    for(int i = 0; i < length; i++)
    {
        total += letterValue(word[i]);
    }

    return total;
}

int letterValue(char ch)
{
    int value = 0;

    switch(ch)
    {
        case 'A': case 'a':
        case 'E': case 'e':
        case 'I': case 'i':
        case 'L': case 'l':
        case 'O': case 'o':
        case 'R': case 'r':
        case 'S': case 's':
        case 'T': case 't':
        case 'U': case 'u':
            value += 1; break;
        case 'D': case 'd':
        case 'G': case 'g':
            value += 2; break;
        case 'B': case 'b':
        case 'C': case 'c':
        case 'M': case 'm':
        case 'P': case 'p':
            value += 3; break;
        case 'F': case 'f':
        case 'H': case 'h':
        case 'V': case 'v':
        case 'W': case 'w':
        case 'Y': case 'y':
            value += 4; break;
        case 'K': case 'k':
            value += 5; break;
        case 'J': case 'j':
        case 'X': case 'x':
            value += 8; break;
        case 'Q': case 'q':
        case 'Z': case 'z':
            value += 10; break;
    }
    return value;
}

@Ethan's review already mentioned the basic improvements, but I think that there are other things you could improve and experiment with. At the end of this review I've put together a working example of the refactored code, so you can refer to it for all the things I'm mentioning.

A few other nits...

When your program consists of a single source file, you might as well avoid declaring function prototypes and put the other functions above main. This will ease maintenance, since it is one less place to update if you change a function name/parameter type.
Instead of passing a fixed char array to getline and limiting yourself to 80 chars max, you can pass a std::string to it and allow for arbitrary length words.
Make sure to pass the string parameter of gameScore as a const reference. In C++, when you pass an object to a function, it will be copied unless stated otherwise. So in that case, gameScore is seeing its own copy of the word. But you don't need to pay for that copy since the function is only inspecting each character of the string, without modifying it. Enter references. When passing a read-only reference, also make it const (check the example at the end for the details). It is important to note though that this tip applies to objects only (classes, structs. std::string is a class, mind you). Native types like integers and pointers are actually better if passed by value, since the hardware has registers to store them, so copying an int into a function is free.
You did right by storing the length of the string in a local variable inside gameScore to avoid calling the method in a loop, but it is actually not a 100% right. Being pedantic, string.length() returns an unsigned integer type. You have a few options here to match the type: C++11 and above, use auto and let the compiler infer the exact type. Older C++, you can use std::string::size_type (if you really love typing ;)) or shorter, a std::size_t, which is equivalent. The only real implication of using int like you currently do is that on some platforms it might be smaller than the size of the type returned by string, so assuming we are dealing with a Gigabytes long string, it could theoretically truncate the value and loose part of the string's length. Not the case with your program, I'm sure, but it is good to be aware of this when writing portable code.
Use const on local variables that are only assigned/initialized once. For instance, that int score. When the scope of a variable is small, it makes little difference, but it is good to get in the habit of using it. When looking at a 100 lines long function, you will be happy to know that the variables you are tracking are not going to be changed after they are declared.

An alternate implementation for letterValue:

I didn't really like that switch in letterValue. It is not hard to understand nor unclear, but it just feels too long and verbose...

You could use a Standard map to shorten things up a bit. Map each lower case letter to its integer value, them just lookup the map. If you cast each letter tolower before looking up the map, you don't have to repeat every char for upper and lower case.

Sample implementation:

#include <iostream>
#include <cctype>
#include <string>
#include <map>

int letterValue(char ch)
{
    // This map replaces your switch/case.
    // The upside is that you don't have to specify
    // upper and lower case letters. The downside is
    // that the letter values have to be repeated
    // for each entry.
    //
    // Notice that we made it a static constant, so the map
    // doesn't get recreated every time we enter the function.
    //
    // NOTE: To compile this code, you'll need to
    // make sure C++11 support is enabled in your
    // compiler (should be fine by default with recent
    // versions of Visual Studio).
    //
    // Also notice that your solution is missing the entry
    // for 'n', as was pointed out by Edward in his answer.
    //
    static const std::map<char, int> charValues = {
        {'a',1}, {'e',1}, {'i',1}, {'l',1}, {'o',1},
        {'r',1}, {'s',1}, {'t',1}, {'u',1}, {'n',1},
        {'d',2}, {'g',2}, {'b',3}, {'c',3}, {'m',3},
        {'p',3}, {'f',4}, {'h',4}, {'v',4}, {'w',4},
        {'y',4}, {'k',5}, {'j',8}, {'x',8}, {'q',10},
        {'z',10}
    };

    // Since you want to return 0 for a non-mapped letter
    // we first find() it, then check the result. The `? :`
    // thingy is the ternary operator, in case you are not
    // familiar with it. It's just like a compact version
    // of an if then else...
    //
    auto iter = charValues.find(std::tolower(ch));
    return (iter != std::end(charValues)) ? iter->second : 0;
}

int gameScore(const std::string & word) // <= Here you can see the const
         // reference, to avoid copying the string. The `&` means reference.
{
    // Notice the const here to enforce one-time initialization.
    const std::size_t length = word.length();

    int total = 0;
    for (std::size_t i = 0; i < length; i++)
    {
        total += letterValue(word[i]);
    }

    return total;
}

int main()
{
    while (true)
    {
        // Using a string instead of a fixed size array.
        std::string word;
        std::cout << "Enter a word: ";
        std::getline(std::cin, word);

        // std::string has this handy method to test if it is... Empty!
        if (word.empty())
        {
            break;
        }

        const int score = gameScore(word);
        std::cout << "The score for '" << word << "' is " << score << "\n";
    }

    std::cin.get();
}


Should this be written with exception handling instead of nested if-thens? I have a function that iterates through a list of file updates, and for each file, if updates are needed, download the file from the server, make the updates, and upload the updated version.  Each update gets marked with a string identifying the result of the update operation.

There are several steps in the update process where I would not want updating to continue for that file.  Right now I have a lot of nested if-then statements, so that the iteration won't proceed any further for that file and will just mark the issue and go on to the next file.  Code as follows:

    public void updateAllFiles()
    {
        foreach (string fileName in updateList.fileNameList())
        {
            updatesForFile = updateList.updatesForFile(fileName);
            hyperlinkEntry = hyperlinkList.getHyperlinkEntry(fileName);
            if (hyperlinkEntry == null)
            {
                updatesForFile.markResultAllRows("File not found on server");
            }
            else
            {
                hyperlink = hyperlinkEntry.hyperlink;
                if (updatesForFile.updateNeeded())
                {
                    if (!po.checkout(hyperlink))
                    {
                        updatesForFile.markResultAllRows("checked out to other user");
                    }
                    else
                    {
                        downloadPath = downloadFolder + fileName;
                        downloadResult = dl.download(hyperlink, downloadPath);
                        if (downloadResult == false)
                        {
                            updatesForFile.markResultAllRows("download failed");
                        }
                        else
                        {
                            file = new FileObject(xlApp, downloadPath, updatesForFile);
                            if (file == null)
                            {
                                updatesForFile.markResultAllRows("unable to open");
                            }
                            else
                            {
                                foreach (FeatureUpdate featureUpdate in updatesForFile.getFeatureUpdateList())
                                {
                                    resultValue = featureUpdate.getColumnValue("result");
                                    if (!resultValue.Contains("updated"))
                                    {
                                        file.update(featureUpdate, updateMethods.lowFeatureLength);
                                        updatesForFile.markResult(featureUpdate.getIndex(), "updated");
                                    }
                                }
                                file.close();
                                updateList.save();
                            }
                        }
                    }
                }
            }
        }

    }

Because it's getting harder to read the code, I'm wondering if I should use exception handling here instead, and if so, how it should be implemented here--e.g., should an exception be thrown in getHyperlinkEntry() and caught in updateAllFiles(), or should it be thrown and caught in updateAllFiles() like this?

try
{
    hyperlinkEntry=hyperlinkList.getHyperlinkEntry(fileName);
    if (hyperlinkEntry == null)
    {
        throw new UpdateException("File not found on server");
    }

    ///more function calls and exception throwing here
}
catch (UpdateException ue)
{
    updatesForFile.markResultAllRows(ue.updateMessage);
}

Or are the existing nested if-then clauses more appropriate for this situation?

Maybe simple loop logic can serve your purposes better. If what you want is to ignore the rest of the loop body if a condition is met, the continue statement will follow immediately with the next iteration.

foreach (string fileName in updateList.fileNameList())
{
    updatesForFile = updateList.updatesForFile(fileName);
    hyperlinkEntry = hyperlinkList.getHyperlinkEntry(fileName);
    if (hyperlinkEntry == null)
    {
        updatesForFile.markResultAllRows("File not found on server");
        continue;
    }
    hyperlink = hyperlinkEntry.hyperlink;
    if (updatesForFile.updateNeeded())
    {
        if (!po.checkout(hyperlink))
        {
            updatesForFile.markResultAllRows("checked out to other user");
            continue;
        }
        downloadPath = downloadFolder + fileName;
        downloadResult = dl.download(hyperlink, downloadPath);
        if (downloadResult == false)
        {
            updatesForFile.markResultAllRows("download failed");
            continue;
        }
        file = new FileObject(xlApp, downloadPath, updatesForFile);
        if (file == null)
        {
            updatesForFile.markResultAllRows("unable to open");
            continue;
        }
        foreach (FeatureUpdate featureUpdate in updatesForFile.getFeatureUpdateList())
        {
            resultValue = featureUpdate.getColumnValue("result");
            if (!resultValue.Contains("updated"))
            {
                file.update(featureUpdate, updateMethods.lowFeatureLength);
                updatesForFile.markResult(featureUpdate.getIndex(), "updated");
            }
        }
        file.close();
        updateList.save();
    }
}

If what you need is to exit altogether when a condition is met, then I'd favor return, maintaining the same logic as the continue example. This would also give a lot more flexibility to refactor your code into functions and even probably use of kind of error codes, if it gets to that point.

Exceptions are very useful, but they are very expensive, so unless you don't really care about the performance of this code, I'd favor a solution based in flow control any day of the week.


Java %= and general feedback I'm doing a program for a course in school (intro software development) and I'm looking for some general feedback as well as any ways this bit of code could be cleaner. I'm also wondering if it is bad practice to use %= as I've talked with some people and have been told it's not good to use any operators like that, other than += and -= to keep code readable, although maybe that is specific to this particular course.

private int       heightInInches;
private final int VALUE_OF_ONE_FOOT = 12;

public String getHeightInFeetAndInches() {
    String includeInches = "";
    int leftOver = (heightInInches %= VALUE_OF_ONE_FOOT);
    int newHeight = (heightInInches % VALUE_OF_ONE_FOOT);

    if(leftOver < 0) {
        includeInches = "";
    }else if(leftOver <= 1) {
        includeInches = "Inch";
    }else {
        includeInches = "Inches";
    }
    return newHeight + " feet " + leftOver + " " + includeInches;
}

This method should return a person's height in the form of (for example): 6 feet 1 inch(es) (if the inch amount exceeds 1).

I have no problem with +=, -= and even *=. But for /= and %=... perhaps I am just not used to them enough to like seeing them. I would recommend not using %=, but that is mostly my personal preference.

Other comments:

Instead of using the heightInInches as a field of the class, I would recommend passing it as a parameter to the method. Your method is only dependent of this single variable so I don't really see a reason to having it dependent on your class, especially since your method actually modifies this variable. You'd be much safer in having your method as

public String getHeightInFeetAndInches(int heightInInches) {

You could even make your method static if you would like (which I personally would recommend)
Your VALUE_OF_ONE_FOOT should be static as it is a constant value.
It is totally impossible for your leftOver value to be less than zero unless your heightInInches is also less than zero. And so far I haven't seen a single person with a negative height. Did you mean if (leftOver <= 0)?
I would have the last space as a part of the includeInches variable, as if it would be empty right now your return would end with a trailing space.
Please use more spaces and make this line }else if(leftOver <= 1) { into } else if (leftOver <= 1) {
As @palacsint has pointed out, your code has a serious bug. I believe you should use the following:

int leftOver = heightInInches % VALUE_OF_ONE_FOOT;
int newHeight = heightInInches / VALUE_OF_ONE_FOOT;

This would also have the benefit of not modifying your heightInInches variable (although I still think you should pass it as a parameter to the method instead of using it as a field).
As pointed out by P. Lalonde in the comments below, your VALUE_OF_ONE_FOOT should be renamed to something like INCHES_PER_FOOT. All constants are values of something.
Feature Request: Add support for the metric system ;)


Laravel controller for a grocery list What I have is a grocery list. Once I select a grocery by ticking the checkbox, I can select an amount for that product and a unit (unit is referred to in my below code as group). Once I am done, I click save and the values are updated in the database. But for all checkboxes that are unchecked, the values should be updated to 0. Also, if I DO select a checkbox, but I DO NOT select an amount OR a group, the values should all still be updated to 0.

The code below works flawlessly, but I think it could be written much shorter / better. Any pointers?

public function post()
{
    $checkboxes         = Input::get('checkbox');
    $checkedAmounts         = Input::get('amount');
    $checkedGroups      = Input::get('group');

    /* The below code checks if a checkbox was checked, an amount was selected and a group was selected. If any of those values are true, update the database with the values selected. */

        foreach ($checkedAmounts as $key => $value)
        {
            if($value !== '0')
            {
                Product::where('id', '=', $key) ->update(['amount' => $value]);
            }
        }

        foreach ($checkedGroups as $key => $value)
        {
            if($value !== '0')
            {
                Product::where('id', '=', $key) ->update(['group' => $value]);
            }
        }

        foreach ($checkboxes as $key => $value)
        {
            if($value !== '0')
            {
                Product::where('id', '=', $key) ->update(['selected' => '1']);
            }
        }

    /* The below code checks if a checkbox was NOT checked, an amount was NOT selected or a group was NOT selected. If ANY of those are false, update the database and set everything to 0 for that row. */

        foreach ($checkboxes as $key => $value)
        {
            if($value === '0')
            {
                Product::where('id', '=', $key) ->update(['selected' => '0']);
                Product::where('id', '=', $key) ->update(['amount' => '0']);
                Product::where('id', '=', $key) ->update(['group' => '0']);
            }
        }

        foreach ($checkedAmounts as $key => $value)
        {
            if($value === '0')
            {
                Product::where('id', '=', $key) ->update(['selected' => '0']);
                Product::where('id', '=', $key) ->update(['amount' => '0']);
                Product::where('id', '=', $key) ->update(['group' => '0']);
            }
        }

        foreach ($checkedGroups as $key => $value)
        {
            if($value === '0')
            {
                Product::where('id', '=', $key) ->update(['selected' => '0']);
                Product::where('id', '=', $key) ->update(['amount' => '0']);
                Product::where('id', '=', $key) ->update(['group' => '0']);
            }
        }

        return Redirect::to('groceries')->with('categories', Category::with('products')->orderBy('name', 'asc')->get());
}

First of all, you could have less updates if you passed all values you want to set at once:

Product::where('id', '=', $key) ->update(['selected' => '0', 'amount' => '0', 'group' => '0']);

Secondly, you check in many places in your code if value is different than 0 and save the value, but if later if it is equal to 0 you save 0 in the database - you could just simply save the value in the database and skip the checks.

This, including some other enhancements, should work for you:

$data = [];
foreach ($checkedAmounts as $productId => $value)  {
  $data[$productId]['amount'] = $value;
}

foreach ($checkedGroups as $productId => $value) {
  $data[$productId]['group'] = $value;
}

foreach ($checkboxes as $productId => $value) {
  $data[$productId]['selected'] = boolval($value);
}

foreach ($data as $productId => $updates) {
  Product::whereId($productId)->update($updates);
}


Meta-circular interpreter for lambda calculus in Python For learning purpose, I've written a simple lambda calculus interpreter (plus 'Add'). I would like it to be the cleanest and most idiomatic possible.

Can we make it as neat as the Haskell version?

# lambda interpreter example.

# Values: Num, Fun & Wrong.
# Terms: Cons, Var, Lam, App & Add.

class Num:
  def __init__(self, v):
    self.v = v
  def __str__(self):
    return str(self.v)

class Fun:
  def __init__(self, f):
    self.f = f
  def __call__(self, *args, **kargs):
    return self.f(*args, **kargs)
  def __str__(self):
    return 'function'

class Wrong:
  def __str__(self):
    return 'Wrong'

def add(v1, v2):
  return Num(v1.v + v2.v)

def apply(v1, v2):
  return v1(v2)

class Cons:
  def __init__(self, v):
    self.v = int(v)
  def interp(self, env):
    return Num(self.v)

class Var:
  def __init__(self, x):
    self.x = x
  def interp(self, env):
    return env[self.x]

class Lam:
  def __init__(self, arg, body):
    self.arg = arg
    self.body = body
  def interp(self, env):
    def f(v):
      env2 = env.copy()
      env2[self.arg] = v
      return self.body.interp(env2)
    return Fun(f)

class App:
  def __init__(self, fun, param):
    self.fun = fun
    self.param = param
  def interp(self, env):
    return apply(self.fun.interp(env),
                 self.param.interp(env))

class Add:
  def __init__(self, a, b):
    self.a = a
    self.b = b
  def interp(self, env):
    return add(self.a.interp(env), self.b.interp(env))

expr = App( Lam('x', Add(Var('x'), Var('x'))),
             Add(Cons(10), Cons(11)) )

print(expr.interp({}))

I don't quite know what lambda calculus is (I'm assuming it's a mathematical annotation for what we might call "purely functional programming"?), but I'll give this a quick shot.

First, I'd love to have env populate itself if not provided. You really shouldn't have mutable default values for functions, though; the typical practice is to define:

interp(self, env=None):
    env = env or {}
    # ...

but that's really bloat'y in this case, so let's use a little inheritance:

class Op:
    def interp(self, env=None):
        return self._interp(env if env is not None else {})

    def _interp(self, env):
        raise NotImplementedError()

class Cons(Op):
    def __init__(self, v):
        self.v = int(v)

    def _interp(self, env):  # update name to "_interp"
        return Num(self.v)

# ...

print(expr.interp())  # Yay for no boilerplate arguments!

Now we can just call interp() and the rest handles itself.

The next thing I'd do to make things a bit more concise is to leverage Python 3.7's new dataclass feature; while this doesn't seem to remove any lines of code, it's certainly more concise and descriptive, and adds some useful meta-features like allowing our AST objects to be intelligently compared and printed:

from dataclasses import dataclass

# ...

@dataclass
class App(Op):
    fun: Lam
    param: Op

    def _interp(self, env):
        return self.fun.interp(env)(self.param.interp(env))

@dataclass
class Add(Op):
    a: Op
    b: Op

    def _interp(self, env):
        return add(self.a.interp(env), self.b.interp(env))

# ...

print(expr)
# App(fun=Lam(arg='x', body=Add(a=Var(x='x'), b=Var(x='x'))), param=Add(a=Cons(v=10), b=Cons(v=11)))

Moving beyond Add, we can start using inheritance to make things clearer and more concise:

@dataclass
class BinOp(Op):
    a: Op
    b: Op

    @staticmethod
    def _func(v1, v2):
        raise NotImplementedError()

    def _interp(self, env):
        return self._func(self.a.interp(env), self.b.interp(env))

class Add(BinOp):
    @staticmethod
    def _func(v1, v2):
        return Num(v1.v + v2.v)

class Sub(BinOp):
    @staticmethod
    def _func(v1, v2):
        return Num(v1.v - v2.v)

Some minor nit-pick details to finish off:

4-space indentation is more common than 2-space, which can look a bit cramped.
I'm not sure if it's typically allowed in lambda calculus, but I'd like to see Lam/functions that can take any number of arguments (I have a working implementation that's pretty clean that I'd be happy to share if you're interested).


Supporting two kinds of user profiles I have fully working code, however I'd like to make it nice. I've got these three models:

  User (can have both or either one)
  user can have one trucker
  user can have one owner_trucker
  Trucker
  belongs to user
  OwnerTrucker
  belongs to user

I'm not the only one working in the app, but it starts to annoy me how everyone is solving the issue locally.

The problem is how to determine what the user is (a trucker or a owner_trucker or both). i.e if user.trucker is not nil, its a trucker etc.

So here are few examples from the application (various controllers), basically checking for the same thing but in a different way:

helper_method :profile_type_from_user
  def profile_type_from_user(user)
    return profile = user.trucker ? :trucker : :owner_trucker
  end

And then used in another controller like:

if profile_type_from_user(profile) == :trucker
else
end

Another example would be:

@profile = current_user.trucker ? current_user.trucker : current_user.owner_trucker

Checking if one or another:

current_user.trucker? && current_user.owner_trucker.nil?

It this refactorable?

I think your suspicion that the code is a "wet" is correct.  I would start moving these predicates into the model whenever they are:

Used more than once, or
Can make the code clearer by having a named predicate.

Consider replacing this:

helper_method :profile_type_from_user
  def profile_type_from_user(user)
    return profile = user.trucker ? :trucker : :owner_trucker
  end

With this:

class User
  ...
  def profile_type
    trucker ? :trucker : :owner_trucker
  end
  ...
end

Similarly, consider replacing this:

@profile = current_user.trucker ? current_user.trucker : current_user.owner_trucker

with:

class User
  ...
  def profile
    trucker || owner_trucker
  end
  ...
end

@profile = current_user.profile


Permutation of given string I am reading a book on DSA, and the author explains how to generate the permutation of strings using recursion.  I want to know if there are better ways of doing the same, unless this is the best solution.  Also, could someone help me understand the time-complexity of this algorithm?  Recursion is something I use very rarely but am trying to learn about now.

public class Anagrams {
private char[] chArray;
private int size;
private static int count;

public Anagrams(String anagramString, int size) {
    if (size <= 0) {
        System.out.println("Please enter a valid String");
    }
    chArray = anagramString.toCharArray();
    this.size = size;

}

public static void main(String args[]) {
    String str = "dogs";
    Anagrams anagramGenerator = new Anagrams(str, str.length());
    anagramGenerator.generateAnagrams(str.length());

}

public void generateAnagrams(int newSize) {
    if (newSize == 1)
        return;

    for (int i = 0; i < newSize; i++) {
        generateAnagrams(newSize - 1);
        if (newSize == 2)
            displayWord();
        rotate(newSize);
    }

}

private void rotate(int newSize) {
    // TODO Auto-generated method stub
    int position = size - newSize;
    char tempCh = chArray[position];
    int i;
    for (i = position + 1; i < size; i++) {
        chArray[i - 1] = chArray[i];
    }
    chArray[i - 1] = tempCh;

}

private void displayWord() {
    // TODO Auto-generated method stub
    String word = "";
    count++;
    for (int i = 0; i < chArray.length; i++) {
        word += chArray[i];
    }
    System.out.println(count + ")" + word);
}
}

Be consistent

The most important advice I could give you is be consistent. Be consistent with your naming convention, the placement of brackets and everything else. Why ? Because even if you're doing something wrong you will be predictable, and that is what make things faster when reviewing.

   if (size <= 0) {
        System.out.println("Please enter a valid String");
   }

vs

 if (newSize == 1)
        return;

Same situation, an if condition and one line of code if the condition is good. Why one have brackets and the other one don't have it ? In this particular case, always use brackets. It's more readable, if you add a line of code you don't have to add brackets and it will probably reduce the chance to have bugs.

Constructor

public Anagrams(String anagramString, int size)

You already using String as parameter, why do you have size ? When you call your constructor, you're passing str.lenght(). I suggest that you drop the second argument and just call the length directly in your constructor.

public Anagrams(String anagramString) {
    if (size <= 0) {
        System.out.println("Please enter a valid String");
    }
    chArray = anagramString.toCharArray();
    this.size = anagramString.lenth();

}

Another important point, you're checking if the size <= 0 and print that you must enter a valid String, but you don't do nothing else. The program will go on in an invalid state. You could throw an exception that specify that the length of the String must be greater than 0.

Comments

Never leave // TODO Auto-generated method stub comments. It's just noise in the code, and could lead to people thinking that your code is somewhat not finished.


Max difference between two array elements Can I get feedback on below code? Problem statement is Maximum difference between two elements such that larger element appears after the smaller number

Would this solution be still considered O(n) for time complexity? I know I do one additional pass but I found this way to be more intuitive

Is error handling done properly? Other ways to improve it?

int maxDiff(int *arr, int len);

int main(void)
{
  int arr[] = {80, 2, 6, 3, 100};
  int len = sizeof(arr)/sizeof(int);
  int max_diff = maxDiff(arr, len);
  printf(" max diff is %d \n ", max_diff);
  return 0;
}

int maxDiff(int *arr, int len)
{
  int min = INT_MAX;
  int max = INT_MIN;
  int max_index;

  if (len < 1 || arr == NULL)
  {
      printf(" No element present \n ");
      return INT_MIN;
  }

  for (int i = 0; i < len; i++)
  {
     if (arr[i] > max)
     {
       max = arr[i];
       max_index = i;
     }
  }

  for(int i = 0; i < max_index; i++)
  {
     if (arr[i] < min)
     {
       min = arr[i];
     }
  }

  return max - min;
}

TestData

I don't know if the test data is given to you, but it is not the most useful for verifying the such that larger element appears after the smaller number condition.  Since 100 is the last value, any code that finds 100 as the largest number and subtracts the lowest number will find the correct answer.  If the test data was

int arr[] = {100, 2, 6, 3, 80};

it would be a better test.  The correct answer is now 78 because we ignore the 100 as it is before the 2.

NOTE: For the above data, the current code returns -2147483547 as the difference because we find 100 at the 0th element and never find a min value so the max difference is 100 - INT_MAX.

The code as written is \$O(n)\$ as when considering such things we don't worry about multiples of N, so iterating through the list twice \$O(2n)\$ is, for complexity comparisons, the same as \$O(n)\$.

The bad news is that the algorithm is flawed.

Find the largest number,
Find the largest difference between the largest number and all the numbers before it

fails if the largest number is first (as above), but also if there is a larger difference after the largest number

int arr[] = {50, 60, 100, 10, 70};

the code as written returns 50 but the correct answer 60 (70 - 10).

The following should give the correct answer.

#include <stdio.h>

int maxDiff(int* arr, int len);

int main(void)
{
    int arr[] = { 50, 60, 100, 10, 70 };
    int len = sizeof(arr) / sizeof(int);
    int max_diff = maxDiff(arr, len);
    printf(" max diff is %d \n ", max_diff);
}

int maxDiff(int* arr, int len)
{
    if (arr == NULL || len == 0)
    {
        return -1;
    }
    int prev = arr[0];
    int maxDiff = 0;
    for (int i = 1; i < len; ++i)
    {
        if (arr[i] > prev)
        {
            int newDiff = arr[i] - prev;
            if (newDiff > maxDiff)
            {
                maxDiff = newDiff;
            }
        }
        else
        {
            prev = arr[i];
        }
    }
    return maxDiff;
}


This is not very much different from your solution. It does not rely on the header being hardcoded though. Depends on GNU awk for the use of PROCINFO to control array traversal.

gawk -F, '
    {count[$2,$4]++; code[$2]; val[$4]}
    END {
        PROCINFO["sorted_in"] = "@ind_num_asc"
        printf "Code\t"
        dashes = "--------"
        for (v in val) {
            printf "%8d", v
            dashes = dashes "--------"
        }
        printf " =%8s\n", "Total"
        dashes = dashes "-----------"
        print dashes

        for (c in code) {
            sum_code = 0
            printf "%d\t", c
            for (v in val) {
                sum_code += count[c,v]
                sum_val[v] += count[c,v]
                printf "%8d", count[c,v]
            }
            printf " =%8d\n", sum_code
        }
        print dashes

        printf "Total\t"
        sum = 0
        for (v in val) {
            sum += sum_val[v]
            printf "%8d", sum_val[v]
        }
        printf " =%8d\n", sum
        print dashes

        printf "PCT\t"
        for (v in val) {
            printf "%8.1f", 100*sum_val[v]/sum
        }
        print "\n" dashes
    }
' file

Code          10      20      30      40 =   Total
---------------------------------------------------
2910           3       0       0       0 =       3
2911           0       2       0       0 =       2
2912           0       2       0       0 =       2
2914           2       2       1       0 =       5
2915           2       0       3       0 =       5
2916           0       0       0       5 =       5
2917           2       0       0       0 =       2
---------------------------------------------------
Total          9       6       4       5 =      24
---------------------------------------------------
PCT         37.5    25.0    16.7    20.8
---------------------------------------------------

I'm not a fan of ALL_CAPS_VARNAMES


Faking a pass method when building an array Ruby doesn't have a pass method to 'do nothing' the way Python does - if I want a ternary statement to do something on true, but nothing on false (as in the code below). Is it bad practice in Ruby to fake it this way?

beverage_links = []

beverage_info.each do |bvg|
   self.is_menu_item?(bvg) ? beverage_links << bvg["href"] : "pass"
end

What it looks like you really want is:

beverage_links = beverage_info.select{|bvg| self.is_menu_item?(bvg)}.map{|bvg|bvg["href"]}

This might be a bit confusing, so to break it down

beverage_info.select { |bvg| self.is_menu_item?( bvg ) }

This returns an array of items in beverage_info that return true* for is_menu_item?

  .map { |bvg| bvg["href"] }

This translates each of the items in the array returned in the previous into their "href" value. Finally you assign that to beverage_links.

If beverage_info is an array of beverages, I recommend you rename it to beverages. It is obvious that if you have a beverage, you have the info of that beverage.

If beverages are a custom class, give it the method is_menu_item? because nothing else will use that (unless you have, say, burgers that are also menu items. In that case, both Beverages and Burgers should extend some Food class that has the is_menu_item? method).  The way you have it now, it doesn't look like you need to be using the self keyword, but it's hard to tell without context.

* Actually, it returns all items that return ANYTHING that isn't false or nil for is_menu_item?


Improving function when used from another class The title could probably be better, but in any case here goes.

I am having a bit of a problem understanding how to move methods/functions to other classes to organize things.

So, I have moved a method of sorts, but as I am skeptical of it, I would like to get it reviewed and hear opinions on what can be improved.

Here is the method ran from the default class:

private void GetAudioDevices()
{
    foreach (MMDevice device in Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active))
    {
        comboBox1.Items.Add(device.FriendlyName);
        Console.WriteLine("{0}, {1}", device.FriendlyName, device.State);
    }
    PlayBackDevices = Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active);
    for (waveInDevice = 0; waveInDevice < NAudio.Wave.WaveIn.DeviceCount; waveInDevice++)
    {
        DeviceInfo = WaveIn.GetCapabilities(waveInDevice);
        AudioDevices.Items.Add(DeviceInfo.ProductName);
    }
    if (AudioDevices.Items.Count > 0)
        AudioDevices.SelectedIndex = 0;

    AudioDevices.Items.Add("Wasapi Loopback");
    ListenAudioAgain = false;
    AudioDevice = AudioDevices.SelectedItem.ToString();
    comboBox1.SelectedItem = VoiceChatClass.Enumerator.GetDefaultAudioEndpoint(DataFlow.Render, Role.Multimedia).ToString();
}

Here it is when ran from a separate class:

public static void GetAudioDevices(ComboBox comboBox1, ComboBox AudioDevices)
{
    foreach (MMDevice device in VoiceChatClass.Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active))
    {
        comboBox1.Items.Add(device.FriendlyName);
        Console.WriteLine("{0}, {1}", device.FriendlyName, device.State);
    }
    VoiceChatClass.PlayBackDevices = VoiceChatClass.Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active);
    for (VoiceChatClass.waveInDevice = 0; VoiceChatClass.waveInDevice < NAudio.Wave.WaveIn.DeviceCount; VoiceChatClass.waveInDevice++)
    {
        VoiceChatClass.DeviceInfo = NAudio.Wave.WaveIn.GetCapabilities(VoiceChatClass.waveInDevice);
        AudioDevices.Items.Add(VoiceChatClass.DeviceInfo.ProductName);
    }
    if (AudioDevices.Items.Count > 0)
        AudioDevices.SelectedIndex = 0;

    AudioDevices.Items.Add("Wasapi Loopback");
    VoiceChatClass.ListenAudioAgain = false;
    VoiceChatClass.AudioDevice = AudioDevices.SelectedItem.ToString();
    comboBox1.SelectedItem = VoiceChatClass.Enumerator.GetDefaultAudioEndpoint(DataFlow.Render, Role.Multimedia).ToString();
        Console.WriteLine(VoiceChatClass.Enumerator.GetDefaultAudioEndpoint(DataFlow.Render, Role.Multimedia));
}

I am having difficulties understanding how to use static and non-static variables.

I just want to move it to a separate class for organization reasons. I know you can do it with partial classes, but sadly they are a bit "bugged" when used with a form in Visual Studio, as the design form will go away and stuff.  It just gets unnecessarily complicated so to speak.

So anyway, what are your opinions upon viewing this?  Is there a way to make it simpler?  Are there any issues that have come up?

It seems to me like you are really asking about static vs non-static

static

This means that your instance or instance variable is class-bound. No matter how many objects you make, they will all share the same static variable. Take this class with a static variable called InstanceID. What it will do is increment the static variable by one for each object you create

public class StaticVSNonStatic
{
    private static int InstanceID = 0;

    public StaticVSNonStatic()
    {
        InstanceID++;
    }

    public int GetInstanceID() { return InstanceID; }
}

so say that you later create N number of StaticVSNonStatic objects it won't matter which one you call because they will all share the same variable and will output the same value.

StaticVSNonStatic s1 = new StaticVSNonStatic();
StaticVSNonStatic s2 = new StaticVSNonStatic();
StaticVSNonStatic s3 = new StaticVSNonStatic();

var value = s1.GetInstanceID(); // this will be 3

And if we create any more object of StaticVSNonStatic the InstanceID value will just keep on incrementing.

non-static

Simply means a variable or an object that is not declared as static. A variable that is not static is instance-bound, and for each instance you create they will have their own variable. So if we go back to the class above and remove the static keyword the value will always be 1 regardless of which object you are accessing and regardless of how many objects you have.

  So anyway, what are your opinions upon viewing this? Is there a way to make it simpler? Are there any issues that have come up?

It seems like you do not quite understand how and when to create a class to represent an object and you don't fully understand static. But it's ok, OOP is a hard design pattern to grasp. Let's take a look of what you can improve and what makes me say that you don't understand it...

In a few places in your "seperate class" example you write

VoiceChatClass.ListenAudioAgain = false;
VoiceChatClass.AudioDevice = AudioDevices.SelectedItem.ToString();
VoiceChatClass.DeviceInfo =  Audio.Wave.WaveIn.GetCapabilities(VoiceChatClass.waveInDevice);
VoiceChatClass.PlayBackDevices = VoiceChatClass.Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active);

Which are all static variables which you are invoking, but to me it looks like this should be non-static because to me it looks like you should be able to have multiple "unique" VoiceChat objects.

e.g., you should be able to have one VoiceChat object which has ListenAudioAgain set to false and another VoiceChat object which has it set to true. Or you could have two different VoiceChats with different PlayBackDevices. This is the way it should be but is impossible in your case because of the static.

. The first step in making sure we can have multiple "unique" VoiceChat objects is by removing the static keyword from the class, which should give you a class that looks something like this (remember that this is a guess since I can't actually know what the class looks like):

public class VoiceChat
{
    public bool ListenToAudioAgain { get; set; }
    public string AudioDevice { get; set; }
    public DeviceInfo DeviceInfo { get; set; }
    public PlayBackDevices[] PlayBackDevices { get; set; }
}

And you would use this (in your second example) by first creating (or passing it in by your parameters) an instance of the VoiceChat class. In this case we named the instance to voiceChat, and then we access its instance members.

public static void GetAudioDevices(ComboBox comboBox1, ComboBox AudioDevices)
{
    VoiceChat voiceChat = new VoiceChat()

    foreach (MMDevice device in voiceChat.Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active))
    {
        comboBox1.Items.Add(device.FriendlyName);
        Console.WriteLine("{0}, {1}", device.FriendlyName, device.State);
    }

    voiceChat.PlayBackDevices = voiceChat.Enumerator.EnumerateAudioEndPoints(DataFlow.Render, DeviceState.Active);

    for (voiceChat.waveInDevice = 0; voiceChat.waveInDevice < NAudio.Wave.WaveIn.DeviceCount; VoiceChatClass.waveInDevice++)
    {
        voiceChat.DeviceInfo = NAudio.Wave.WaveIn.GetCapabilities(VoiceChatClass.waveInDevice);
        AudioDevices.Items.Add(voiceChat.DeviceInfo.ProductName);
    }

    if (AudioDevices.Items.Count > 0) {
        AudioDevices.SelectedIndex = 0;
    }

    AudioDevices.Items.Add("Wasapi Loopback");

    voiceChat.ListenAudioAgain = false;
    voiceChat.AudioDevice = AudioDevices.SelectedItem.ToString();

    comboBox1.SelectedItem = voiceChat.Enumerator.GetDefaultAudioEndpoint(DataFlow.Render, Role.Multimedia).ToString();

    Console.WriteLine(voiceChat.Enumerator.GetDefaultAudioEndpoint(DataFlow.Render, Role.Multimedia));
}

This is a quick introduction so if there's something unclear just let me know and I'll try to clarify.


Validation of a couple of model classes using FluentValidator A couple of model classes are doing the validation using FluentValidator. These model classes are used to Add and Edit the branch data and they are working well but I have no idea how to remove the code smell (duplication).

Problem: Notice the duplicate validations being written in BranchEditModel class as well. I have marked in the code itself what I think is being duplicated I thought it would be easier for you to read it.

BranchAddModel

using FluentValidation;
using FluentValidation.Attributes;

namespace Application //sample namespace
{
    [Validator(typeof(BranchAddModelValidator))]
    public class BranchAddModel
    {
        public string BranchName { get; set; }
        public byte ServiceTypeId { get; set; }
        public short TimezoneId { get; set; }
    }

    public class BranchAddModelValidator : AbstractValidator<BranchAddModel>
    {
        public BranchAddModelValidator()
        {
            RuleFor(x => x.BranchName)
                .NotEmpty()
                .Length(0, 128);  //up to 128 length of string is allowed.

            RuleFor(x => x.ServiceTypeId)
                .NotEmpty();  //mandatory

            RuleFor(x => x.TimezoneId)
                .NotEmpty();  //mandatory
        }
    }
}

BranchEditModel

BranchEditModel is inheriting the model from BranchAddModel. And, it has 2 extra properties.

namespace Application //sample namespace
{
    [Validator(typeof(BranchEditModelValidator))]
    public sealed class BranchEditModel : BranchAddModel
    {
        public int BranchId { get; set; }
        public bool IsActive { get; set; }
    }

    public class BranchEditModelValidator: AbstractValidator<BranchEditModel>
    {
        public BranchEditModelValidator()
        {
            RuleFor(x => x.BranchName) // code repeated
                .NotEmpty()
                .Length(0, 128);

            RuleFor(x => x.ServiceTypeId) // code repeated
                .NotEmpty();

            RuleFor(x => x.TimezoneId) // code repeated
                .NotEmpty();

            RuleFor(x => x.BranchId)
                .NotEmpty();

            RuleFor(x => x.IsActive)
                .NotEmpty();
        }
    }
}

I see two possibilities here.

1. Use extension methods

You can define a static class BranchModelValidationRules with extension methods for AbstractValidator<T> with methods like this:

public static void ValidateBranchName<T>(this AbstractValidator<T> validator)
    where T : BranchAddModel
{
    validator.RuleFor(x => x.BranchName)
        .NotEmpty()
        .Length(0, 128);
}

2. Extract a common base class

Create a class BranchModelValidatorBase:

public abstract class BranchModelValidatorBase<T> : AbstractValidator<T>
    where T : BranchAddModel
{
    public BranchModelValidatorBase()
    {
        RuleFor(x => x.BranchName)
            .NotEmpty()
            .Length(0, 128);  //up to 128 length of string is allowed.

        RuleFor(x => x.ServiceTypeId)
            .NotEmpty();  //mandatory

        RuleFor(x => x.TimezoneId)
            .NotEmpty();  //mandatory
    }
}

Create concrete validators by extending the base class:

public class BranchAddModelValidator
    : BranchModelValidatorBase<BranchAddModel>
{
    public BranchAddModelValidator()
    {
        // Create future add-specific rules here.
    }
}

public class BranchEditModelValidator
    : BranchModelValidatorBase<BranchEditModel>
{
    public BranchEditModelValidator()
    {
        RuleFor(x => x.BranchId)
            .NotEmpty();

        RuleFor(x => x.IsActive)
            .NotEmpty();
    }
}

Update

Regarding your comment about a more generic Name property: define an interface, for example like this:

public interface IModelWithName
{
    string Name { get; set; }
}

A matching extension method class might then look like this:

public static class ModelValidationRules
{
    public static void ValidateModelName<T>(this AbstractValidator<T> validator)
        where T : IModelWithName
    {
        validator.RuleFor(x => x.Name)
            .NotEmpty()
            .Length(0, 128);
    }
}

You can then reuse this method in any validator class that validates models implementing the interface. Alternatively you could create a validator for IModelWithName with just a single rule. Then use a matching set of validators for actual model validation.


Reading Excel (93-97) sheet with more than 65536 rows using cfspreadsheet I have a process that reads multiple sheets from an Excel file, then inserts the data into a database table. However, I am experiencing some memory issues when one of the sheets contains more than 65536 rows and am looking for ideas on how to improve the code.

In summary, I am using cfspreadsheet to "read" an uploaded Excel file. (In most cases, the file contains a single sheet. However, in some cases it is more than 2 sheets.) The process always reads the first sheet ie "Details" . If more than 65533 rows are found, it then reads the second sheet too i.e. "Details_1". Finally, I use a QoQ and UNION ALL to create a combined query. Once read, the data is inserted into a database table.

Can anyone offer suggestions for improving the process to make it less memory intensive?

<cffunction name="putExcel" access="remote" returnFormat="plain" output="true">
    <cfargument name="xclfile" required="no" type="string" default="0">

    <cfset ins =insertUserLog("#Session.user_name#","#Session.user_code#","putExcel function called for  Upload","","")>

    <cftry>
        <cfset fileEXCL = "#ExpandPath('../folder')#/#arguments.xclfile#" />

        <!---when there e 2 Sheets --->
        <!---get info from  sheet1 as a "query1"--->
        <cfspreadsheet action="read" src="#fileEXCL#" sheet="1" query="Query1" headerrow="1" />
         <!--- recordcount for "sheet1" as "count1"--->
        <cfset count1 =#Query1.recordcount#>
        <!--- case when excel has more than 65533 rows
            ;THIS IMPLIES THAT THERE 2 SHEETS)--->
        <cfif count1 gt 65533>
         <!--- take info from  sheet 2 as a "query2" and count as "count2"--->
            <cfspreadsheet action="read" src="#fileEXCL#" sheet="2" query="Query2" headerrow="1"  />

            <cfset count2 =#Query2.recordcount#>
            <!---club both query's using QoQ and call it "excelQuery"--->
            <cfquery dbtype="query" name="excelQuery">
                SELECT * FROM Query1
                UNION ALL
                SELECT * FROM Query2
            </cfquery>
            <!---total record count for "sheet1" & "sheet2"--->
            <cfset rowCount =#excelQuery.recordcount#>
        <cfelse>
            <!---this case there is just 1 query "Query1" ;rename it "excelQuery"--->
            <cfquery dbtype="query" name="excelQuery">
                SELECT * FROM Query1
            </cfquery>
            <!--- recordcount for "sheet1"--->
            <cfset rowCount =#excelQuery.recordcount#>
        </cfif>

        <cflog file="Collections" application="yes"  text="#Session.user_info.uname# logged in. Data  file #fileEXCL# read. Recordcount:#rowCount#" type="Information">

        <cfset ins =insertUserLog("#Session.user_name#","#Session.user_code#","file #fileEXCL# read. ","Recordcount:#rowCount#","")>

        <cfcatch type="any" >
            <cflog file="Collections" application="yes" text="Error in reading Data  file #fileEXCL#." type="Error">
            <cfset ins =insertUserLog("#Session.user_name#","#Session.user_code#","error file","failed","#cfcatch.Message#")>
            <cfreturn 1>
        </cfcatch>
    </cftry>

    ... etc...

This is in a function, but you haven't given us the complete function.  How's your var scoping?  Maybe update your code to show us all the function.

You don't need to use the octothorpes in most cfset statements like this:

<cfset count1 =#Query1.recordcount#>

Could just be written as

<cfset count1 =Query1.recordcount>

Do you have to use SELECT * or could you specify columns?

And most importantly, and the reason it's probably timing out...  you have 2 queries.  By using UNION ALL to join them together, you're taking the 65533 rows of the first sheet and joining it to the X rows in the second sheet.  That's a pretty big dataset, especially if you're just using Excel.  Have you considered using a database?  Do you really need all this data at this point?


Database design for users' postcards I would like to a review of my database design.

There are 3 tables + ASP.NET Identity tables.

The Users could have many postcards.

Here's my tables without Identity.

I attached also my scripts of creating tables.

CREATE TABLE [dbo].[Country](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](50) NOT NULL,
    [Code] [nchar](10) NOT NULL
) ON [PRIMARY]

CREATE TABLE [dbo].[Error](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Message] [nvarchar](max) NULL,
    [Stacktrace] [nvarchar](max) NULL,
    [CreatedDate] [nvarchar](max) NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

CREATE TABLE [dbo].[Postcard](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [CountryFromId] [int] NOT NULL,
    [CountryToId] [int] NOT NULL,
    [TravelTime] [int] NULL,
    [Confirmation] [bit] NOT NULL,
    [UserFromId] [int] NULL,
    [UserToId] [int] NOT NULL,
    [Status] [int] NULL,
    [SentDate] [datetime2](7) NOT NULL,
    [ReceivedDate] [datetime2](7) NULL,
    [ImageId] [int] NULL
) ON [PRIMARY]

Do you think is it good design, or shoud I change something? I know that proper databse design is key in good application architecture so I would like to have good database design.

Looking at each of the tables, there are a few design problems that stand out.
Country
Depending on the target countries and language, some country names could be longer than 50 characters. For example, "Separate Customs Territory of Taiwan, Penghu, Kinmen, and Matsu (Chinese Taipei)", the longest English country name, is 80 characters long, meaning you would get an error from the SQL database in [Name] [nvarchar](50) if you tried to insert this value.
For the [Code] [nchar](10) there is already some official lists of country codes (reference) which could only be a [nchar](2) field for ISO codes, or a [nchar](3) field for UN codes. Using a standard country code would also make it more standards-compliant.
Error
The [Message] [nvarchar](max) and [Stacktrace] [nvarchar](max) are problematic. You should avoid using the MAX types unless you actually need to store up to 2 GB of text in a field (or just over 1 billion 2-byte characters - source). See Using MAX text or more specific, smaller type from Database Administrators site. The reasoning is in large part because the MAX types are stored outside of the table, only a reference to the field is stored in the table, and as such they are optimized differently.
[CreatedDate] [nvarchar](max) should instead be a [datetime2] or similar type like you are using in other places, and not a character field. Never store dates and/or times in a text field (and especially not in a MAX text field), as it makes it much more difficult to manipulate the data.
The only exception to this is if you are storing/receiving data from a system where date/time values are stored in a non-standard text format like Wed 06-Jul-16 10:31 AM but even in that case it would be better to try to convert it to a compatible format before storing it in the database as a date/time field.
Postcard
This table contains mostly IDs from what looks to be other tables, however I don't see any foreign key references to the other tables, which defeats the primary purpose of have keys for rows in the first place. Here is some useful information on MSDN.
Also make sure to set a PRIMARY KEY constraint on the main ID field of each table. The syntax for keys is like this:
CREATE TABLE [dbo].[Postcard](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    CONSTRAINT [PK_Postcard] PRIMARY KEY NONCLUSTERED ([Id]),
    [CountryFromId] [int] NOT NULL,
    CONSTRAINT [FK_Postcard_CountryFromId] FOREIGN KEY ([[CountryFromId])
        REFERENCES [dbo].[Country]([Id]),
    [CountryToId] [int] NOT NULL,
    CONSTRAINT [FK_Postcard_CountryToId] FOREIGN KEY ([[CountryToId])
        REFERENCES [dbo].[Country]([Id]),
    -- etc.

Using an 8-byte [bigint] type for your Postcard.Id seems like an overly large type. According to MSDN the range for [bigint] is "-2^63 (-9,223,372,036,854,775,808) to 2^63-1 (9,223,372,036,854,775,807)". If you really plan on having that many postcard records then fine, but otherwise just an [int] type would be OK.
For consistency your [Status] [int] should probably be instead named [StatusId].


Is_prime check in Python: can this be more pythonic? I have the following function that checks if a number is prime:

def is_prime(n):
    """ Is n prime? """

    if n < 2:
        return False
    if n in (2, 3):
        return True
    if n % 2 == 0 or n % 3 == 0:
        return False

    limit   = int(math.sqrt(n))
    divisor = 5

    while divisor <= limit:
        if n % divisor == 0 or n % (divisor + 2) == 0:
            return False
        divisor += 6

    return True

Most of it are checks and calculations to save up time, to make it faster than a simple loop through a range.

Can it be written in a more pythonic way? Can it be improved in some way?

Any other feedback is welcome too.

Your code looks nice, seems to follow PEP 8 and has docstrings. I don't have much to say about it but here are a few changes to make it look more pythonic.

The while loop can be rewritten using a for loop with range (or xrange).

for divisor in xrange(5, 1+int(math.sqrt(n)), 6):
    if n % divisor == 0 or n % (divisor + 2) == 0:
        return False
return True

Then, this looks like a scenario where we could use all/any. The obvious way to convert the code above is :

return not any(n % divisor == 0 or n % (divisor + 2) == 0 for divisor in xrange(5, int(math.sqrt(n))+1, 6))

but it looks better (and makes more sense) to use all with reversed conditions :

return all(n % divisor != 0 and n % (divisor + 2) != 0 for divisor in xrange(5, int(math.sqrt(n))+1, 6))

then, you can use the fact that non-zero numbers evaluates to True in a boolean context (and zero evaluates to False) :

return all(n % divisor and n % (divisor + 2) for divisor in xrange(5, int(math.sqrt(n))+1, 6))

Then, you could use the same kind of idea in the previous if then merge duplicated code :

return n % 2 and n % 3 and all(n % divisor and n % (divisor + 2) for divisor in xrange(5, int(math.sqrt(n))+1, 6))

This is pretty much all I have to say about the code. Now, I think it might be worth commenting the algorithm a bit more.

def is_prime(n):
    """ Check if n (assumed to be an int) is a prime number. """
    # Handle values up to 3
    if n < 2:
        return False
    if n in (2, 3):
        return True
    # Handles bigger values : divisors for n have to be
    # * multiples of 2
    # * multiples of 3
    # * of the form 5 + 6*i or 5 + 6*i + 2 (other cases already handled)
    return n % 2 and n % 3 and all(n % divisor and n % (divisor + 2) for divisor in xrange(5, int(math.sqrt(n))+1, 6))


Length of the integer part of a double in C without math.h I came up with this simple function to return the length of a double, working fine in all my test but want to make sure that this code will work in all doubles, can't think of a case in which it would not work but I'm sure if there is, then you can find it, here it is:

int length(double number){
     int result = 0;
     if ( number < 0 ) number *= -1;

     while ( number > 1){
         result++;
         number /= 10;
     }
     return result + ( number == 1 );
}

I think that the (number == 1) is pretty clever, but there might be some better way to accomplish the correct result. It's meant to work whenever the number is an exact power of ten but I think it might be a bit cryptic.

You say it "works fine" in all your tests, but you didn't show us the tests.

Unfortunately, it failed the very first test I wrote:

#include <gtest/gtest.h>

TEST(length, zero)
{
    EXPECT_EQ(1, length(0));
}

Of the next batch, one_minus fails for the same reason:

TEST(length, one)
{
    EXPECT_EQ(1, length(1.0));
}

TEST(length, one_plus)
{
    EXPECT_EQ(1, length(1.0 + 1e-12));
}

TEST(length, one_minus)
{
    EXPECT_EQ(1, length(1.0 - 1e-12));
}

TEST(length, two)
{
    EXPECT_EQ(1, length(2.0));
}

TEST(length, two_plus)
{
    EXPECT_EQ(1, length(2.0 + 1e-12));
}

TEST(length, two_minus)
{
    EXPECT_EQ(1, length(2.0 - 1e-12));
}

TEST(length, ten)
{
    EXPECT_EQ(2, length(10.0));
}

TEST(length, ten_plus)
{
    EXPECT_EQ(2, length(10.0 + 1e-12));
}

TEST(length, ten_minus)
{
    EXPECT_EQ(1, length(10.0 - 1e-12));
}

We can fix this by adding a simple check before the first division:

if (number < 0) number *= -1;
if (number < 1) return 1;

Let's add some more tests:

TEST(length, zillions)
{
    EXPECT_EQ(36, length(1.0e35));
}

TEST(length, negative_zillions)
{
    EXPECT_EQ(36, length(-1.0e35));
}

Now we just need some tests for invalid inputs (infinities, NaNs, etc) and we're golden.

Let's look at the implementation now:

while (number > 1) {
    result++;
    number /= 10;
}
return result + (number == 1);

We can avoid adding number == 1 if we include that case within the while loop:

while (number >= 1) {
    result++;
    number /= 10;
}
return result;

And our tests prove we haven't broken it.

We might now want to improve speed for large numbers: consider our zillions test that loops 36 times.  We could shorten that, at a very small cost to lower numbers by first looping (say) 6 digits at a time:

int result = 0;
while (number >= 1e6) {
    result += 6;
    number /= 1e6;
}
while (number >= 1) {
    result++;
    number /= 10;
}
return result;

When I try this, it initially shows promise, but then fails the zillions tests with an off-by one error.  Is there a subtle difference in rounding between repeatedly dividing by ten and repeatedly dividing by a million when you're dealing with inexact numbers?  Actually, the problem is with our constant - 1.0e35 is most closely represented as 99999999999999999997871448567840768, so we suffered a rounding error in our test - it happened that sequential division by ten introduces a rounding error that compensates for this, and we wrongly pass!

Which brings me neatly to the alternative implementation.  The most convenient tool for measuring digits in the integer part isn't in <math.h> at all - it's in <stdio.h>!  We can ask printf to measure the length for us:

#include <stdio.h>

int length(double number)
{
    /* "unround" the number, as printf rounds to nearest */
    number += .5 - (number > 0);
    /* format the sign, then subtract one, so we don't count
       minus as a digit */
    return snprintf(NULL, 0, "%+1.f", number) - 1;
}

Note that I've left the handling of NaNs and infinities in this version as an exercise...


PyDOS shell simulation I have a big project called 'PyDOS', written in Python 3.4, and it's going very well. Post your answers of anything that could make it look cleaner.

import time
import os
import sys
import random
import operator

def textviewer():
   os.system('cls' if os.name == 'nt' else 'clear')
   print ("Text Viewer.")
   file_name = input("Enter a text file to view: ")
   file = open(file_name, "r")
   print ("Loading text...")
   time.sleep(4)
   os.system('cls' if os.name == 'nt' else 'clear')
   print(file.read())
   input("Press enter to close")
def edit():
    os.system('cls' if os.name == 'nt' else 'clear')
    print ("EDIT")
    print ("-------------")
    print ("Note: Naming this current document the same as a different document will replace the other document with this one.")
    filename = input("Plese enter a file name.")
    file = open(filename, "w")
    print ("FILE: " +filename+".")
    line1 = input()
    line2 = input()
    line3 = input()
    line4 = input()
    line5 = input()
    file.write(line1)
    file.write("\n")
    file.write(line2)
    file.write("\n")
    file.write(line3)
    file.write("\n")
    file.write(line4)
    file.write("\n")
    file.write(line5)
    file.close()
    print ("File successfully saved as: " +filename+"!")
    time.sleep(3)

while True:
   os.system('cls' if os.name == 'nt' else 'clear')
   print ()
   print ("PyDOS VERSION 1.5")
   shell = input("> ")
   if shell == "textviewer":
      print ("Loading Text Viewer...")
      time.sleep(3)
      textviewer()

   elif shell == "edit":
      print ("Loading edit...")
      time.sleep(3)
      edit()

   elif shell == "dir":
      print ("The drive name is A:")
      print ()
      print ("NAME:            TYPE:     MODIFIED:")
      print ("SHUTDOWN.EXE     .EXE      12/01/15 ")
      print ("EDIT.EXE         .EXE      12/01/15 ")
      print ("TEXTVIEWER.EXE   .EXE      12/01/15 ")

   elif shell == "cls":
      os.system('cls' if os.name == 'nt' else 'clear')

   elif shell == 'shutdown':
      print ("Shutting down...")
      time.sleep(3)
      break
   else:
      print(""+ shell+ " is not a reconized command or external file.")
      time.sleep(2)
      os.system('cls' if os.name == 'nt' else 'clear')

5 lines ought to be enough for everyone

The following has no meaning. Why 5?

line1 = input()
line2 = input()
line3 = input()
line4 = input()
line5 = input()

Instead let the user end:

def get_lines():
    print("Enter 'END' to end.")
    lines = []
    line = input()
    while line != 'END':
         lines.append(line)
         line = input()
    return lines

Now you can write:

lines = get_lines()
file.write('\n'.join(lines))

Programming with with

with is an extremely useful idiom because it handles closing files automagically.

with open(filename,'r') as f:
    f.read()
# The file is closed automatically :)

If considered harmful (if overused)

So you just started programming and think "Oh the if statements are so good they feel so CS" (I felt that way too when I started programming when I was 15 so I understand you).

BUT so many if and elif are repetitive and you should not repeat yourself.

I would suggest the high level dictionary to accomplish the task more shortly.

command_actions = {
    'textviewer' : textviewer,
    'edit' : edit,
    'dir' : print_dir,
    'cls' : clear_screen,
    'shutdown' : shutdown,
    'help' : give_help
    }
if shell in command_actions:
    action = command_actions[shell]
    action()
else:
    error_message()

Be helpful

It is common and good practice to show some help/info if the user types in help.

Who sleeps too much does not catch fish annoys the user

Why do you sleep up to 4 seconds?

time.sleep(4)

Ok, this emulates an ancient terminal but really, you don't want to make the user wait so much without reason. I suggest  time.sleep(0.5)


Getter and setter for a k-d nested list-of-lists I'm trying to build accessors for a kd nested list-of-lists such as this 4D example (of shape \$ 2 \times 2 \times 2 \times 2 \$):

kd_lol = [
  [
    [
      [0,1],
      [2,3]
    ],
    [
      [4,5],
      [6,7]
    ]
  ],
  [
    [
      [8,9],
      [10,11]
    ],
    [
      [12,13],
      [14,15]
    ]
  ]
]

I use a list of values to access and modify elements in this nested list-of-lists.

In the above example, you can access something using [i,j,k,l] where \$ 0 \le i,j,k,l < 2 \$.

I'd like to build a fast, efficient getter in idiomatic Python of the form:

def get(grid, list_of_indices)
def put(grid, list_of_indices, value)

My current implementation is as follows:

def get(grid, list_of_indices):
  cur_grid = grid
  for index in list_of_indices:
          cur_grid = cur_grid[index]
  assert(type(cur_grid) != type([])) # not a list
  return cur_grid

and

def put(grid, list_of_indices, value):
  cur_grid = grid
  for index in list_of_indices[:-1]:
    cur_grid = cur_grid[index]
  assert(type(cur_grid) == type([]))    # must be a 1D list
  assert(type(cur_grid[0]) != type([])) # must be a 1D list
  cur_grid[list_of_indices[-1]] = value

Any critique/suggestions to tidy up or reduce the amount of code above is welcome.

Here's a repl that you can play with.

Some very nice code, overall. There's some minor points I think could be improved, but your general implementation is good.

Nitpicks

I'd like to point out some small 'issues' regarding best-practice and code style:

PEP8 recommends using 4 spaces for indentation. They're not too fond of tabs, but most editors will replace tabs with  spaces automatically.
Using assert is great for catching bugs, but it'd better to raise an exception instead when releasing your code. This allows us to specify a specific exception type (like ValueError or TypeError) and provides some additional stack trace information.
It's important to use good names for objects. You're almost there, but a name like cur_grid may as well just be called current_grid, for clarity.
Instead of manually comparing type, we can use isinstance which allows something like this: assert not isinstance(curr_grid, list).

Implementation

The code as is works just fine if you need to modify a matrix, but wouldn't it be useful to turn it into a separate object altogether? We can do this by creating a custom class:

import collections

class Matrix(collections.UserList):
    """A Matrix represents a collection of nested list objects.
    Methods:
    get -- Get a value from the matrix.
    put -- Put a value into the matrix.
    """
    def __init__(self, data):
        """Constructor.

        :param list data: a matrix (multi-dimensional list).
        :return: None
        """
        super(Matrix, self).__init__(data)

    def get(self, indices):
        """Get a value from the matrix.

        :param iterable indices: iterable containing indices.
                                 the last index must point to an object
                                 that is not a list.
        :return: matrix at indices[-1]
        :raises TypeError: if the last index points to a list
        """
        grid = self.data
        # As assigned by UserList
        for index in indices:
            grid = grid[index]
        if isinstance(grid, list):
            raise TypeError("last element is not 1-dimensional")
        return grid

    def put(self, indices, value):
        """Put value at matrix[indices].

        :param iterable indices: iterable containing indices.
                                 the last index must point to a list.
        :return: None
        :raises TypeError: if the last element is not a list
        :raises TypeError: if the last element contains a list
        """
        grid = self.data
        for index in indices[:-1]:
            grid = grid[index]
        if not isinstance(grid, list):
            error = "expected 'list' node, but got '{}'".format(type(grid))
            raise TypeError(error)
        if isinstance(grid[0], list):
            raise TypeError("last element is not 1-dimensional")
        grid[indices[-1]] = value

Note: the class as implemented must be initialized with a matrix and breaks if an empty iterable is passed for data.