ADT stack with a dynamic array (revision 1)

It looks very nice, code is clean and readable, not much else to change, I think...

This is more of a personal preference, not an universal recommendation: One of the things I like about C is having fine control over memory allocations, thus making it a very good choice for systems with limited memory or very rudimentary memory allocators. Because of that, I like to avoid allocating memory whenever possible, so I would not allocate each stack instance on the heap with malloc. Rather, let the user decide where to place it, be on the program stack, as a global or heap-allocated. To do that change, you'd just have to remove that allocation from create_stack and one free from delete_stack, taking a pointer to an external object in create:

bool create_stack(stack* mystack, size_t capacity);

That might seem like a little thing, but forcing the user to dynamically allocate a stack object when it doesn't have to is suboptimal and can be a problem on systems that don't rely on virtual memory and have fragmentation issues.

I strongly recommend that you use booleans instead of error codes when a function has only two possible outcomes, success or failure. There's no universal agreement on the meaning of error codes, e.g. some return 0 on success, some return 1, some return -1 on error, and so on. This can be a major source of confusion. Use the bool type from <stdbool.h> whenever it makes sense.

Avoid repeating types in your malloc calls. E.g.:

item_t *elements_ptr = malloc(capacity*sizeof(item_t));

Can be refactored to:

item_t *elements_ptr = malloc(capacity * sizeof(*elements_ptr));

Notice that you can use the variable name in the sizeof expression, in this case dereferenced, since we want the size of the type itself, not the size of a pointer. Makes your life easier if you change the name of the type you are allocating; one less place to be updated.

Make sure to print errors to stderr. That's the traditional error output and allows users to filter errors from normal program output. Just replace the printf call in handle_error by fprintf(stderr).

Two last tips:

You might want to allow the users of your stack to provide their own error handler. You can achieve that with function pointers.

All functions receive pointers to stack, which could be passed in as null by error or due to some memory allocation failure. You can assert that they are not null to trap errors early and ease the debugging efforts.


Why reinvent the wheel?

Use commons-io's FileUtils.readFileToByteArray(File)(javadoc)


Java8: Stream filter, map and joining with an alternative result if the stream is empty

You can use reduce, it returns an Optional that is empty if the stream is empty. You can use map to create the csv (or reduce) and add the brackets and orElse to return "-".

private static String doIt(List<Integer> list) {
    return list.stream()
            .filter(i -> (i % 2 == 0))
            .map(Object::toString)
            .reduce((left, right)-> left+"; "+right)
            .map(csv -> "["+csv+"]")
            .orElse("-");
}


Program for calculating and ranking scores

Good things

I like the functional approach taken with this code, and that some ecmascript-6 features like arrow functions are used.

Suggestions

const vs let

It would be wise to default to using const for any variable that doesn't need to be re-assigned. If you later determine a value should be re-assigned then switch it to using let. This helps prevent accidental re-assignment in the future.

Use consistent indentation

Some lines appear to be indented with two spaces, while others are indented with four. It is wise to use uniform indentation throughout the code.

Use consistent line terminators

Many lines are terminated with a semi-colon but some are not. Unless you completely understand rules of Automatic semicolon insertion or are using a compiler/module bundler it is best to include line terminators.

multiple calls to Object.keys() in loop

Let us take a look at the following block:

for (let i = 1; i < Object.keys(scoreSeries).length; i++) {
  if (scoreSeries[Object.keys(scoreSeries)[i]] > scoreSeries[key]) {
    rank++
  }
}

For each iteration of the loop, Object.keys() is called twice. That function could be called once if the result is stored in a variable.

const scoreKeys = Object.keys(scoreSeries)
for (let i = 1; i < scoreKeys.length; i++) {
  if (scoreSeries[scoreKeys[i]] > scoreSeries[key]) {
    rank++
  }
}

The syntax could be simplified using a for...of loop:

const scoreKeys = Object.keys(scoreSeries)
for (const scoreKey of scoreKeys) {
  if (scoreSeries[scoreKey] > scoreSeries[key]) {
    rank++
  }
}

The whole function could be simplified with a more function approach using .filter():

//to calculate the ranking of each source
function calculateSourceRank(scoreSeries, key) {
    const scoreKeys = Object.keys(scoreSeries)
    return 1 + scoreKeys.filter(scoreKey => scoreSeries[scoreKey] > scoreSeries[key]).length
}

A similar approach could be applied to calculateFinalRank()


As pointed out by @Martin York in the comments that your class is just a wrapper for a pointer, I don't have much to say. I only have one suggestion

Better Formatting

Good formatting IMO plays a major role in how readable your code is. In this case, I find it impossible to navigate through the functions because they all look extremely cramped.
constexpr pointer operator->() const noexcept { return m_iterator; }
constexpr iterator& operator++() noexcept { ++m_iterator; return *this; }

Compare that to
constexpr pointer operator->() const noexcept {
    return m_iterator;
}

constexpr iterator& operator++() noexcept {
    ++m_iterator;
    return *this;
}


Automation script for creating NuGet packages

Parameters

You could achieve something similar by making real parameters with boolean values such that you can type MyPackage.ps1 1 1 0; feeding 3 positional, named, parameters at a cost of two presses of the space bar.

It's not that much slower to type and makes it a lot easier to understand the code (in my opinion).

In addition, you're easily able to add parameter help.

param(
    [Parameter(Mandatory = $true, Position = 1)]
    [Boolean]$Build,

    [Parameter(Mandatory = $true, Position = 2)]
    [Boolean]$Pack,

    [Parameter(Mandatory = $true, Position = 3)]
    [Boolean]$Push
)

Make $Build default to true perhaps and you have a "Control+Shift+B"-able script.

Ticks

I'd consider killing off those ` you have everywhere. A misplaced space after one would break those very easily.

I would generate arguments as an array and feed the call operator. For calls to internal PowerShell commands I would favour splatting for long parameter lists.

$packArgs = @(
    'pack'
    "$PSScriptRoot\$packageId.nuspec"
    "-properties", "configuration=release"
    "-outputdirectory", "C:\NuGet\packages\"
)
& nuget $packArgs

XML handler

Perhaps consider using Select-Xml as it can read directly from a file.

GetFileNameWithoutExtension

There's an alternative perhaps:

([System.IO.FileInfo]$pscommandpath).BaseName

Or, of course, the original but derived from $pscommandpath.

Alternative approaches

This is a simple build script, but had you looked at psake? https://github.com/psake/psake.


Leaving an APScheduler in a while True loop

while True: pass will consume 100 % of one CPU which is not something you want. I'm not familiar with APScheduler, but a quick look into the docs reveals a daemonic option:

  Controls whether the scheduler thread is daemonic or not.

  If set to False, then the scheduler must be shut down explicitly when
  the program is about to finish, or it will prevent the program from
  terminating.

  If set to True, the scheduler will automatically terminate with the
  application, but may cause an exception to be raised on exit.


Code to modify string as per instructions

Add some comments describing your algorithm
Your code is a single function which is a bit long, and it's not very obvious what it is doing. There are two ways of dealing with this: either creating functions that implement logical steps of the algorithm individually, or add some comments to describe which steps your algorithm is doing. Don't add comments that just descibre exactly what the code does, but give a higher level description.
Unnecessary use of std::count()
You use std::count() a lot, but you don't need to. You can count the number of zeroes and ones when building window, and when you are calling window.erase() you can decrement your counts.
Consider using std::string functions to search for blocks of zeroes and ones
Instead of using std::find() and counting manually using while-loops, you can use std::string::find(). You can also construct a string directly from a subregion of another string.
auto first1 = str.find('1');
auto next0 = str.find('0', first1);
auto next1 = str.find('1', next0);

std::string window(str, first1, next1 - first1);

Once you have that, calculating the number of zeores and ones in window is easy:
auto count1 = next0 - first1;
auto count0 = window.size() - count1;

However, you might not even need the initial window:
Avoid constructing and then partially deconstructing the window
Your window consists of a number of ones followed by a number of zeroes. You are then slowly removing from the start and end until some conditions are met. However, while you are doing this, the only thing you really care about is the count of zeroes and ones, not the string window itself! You can reconstruct the final window from the counts left at the end.
Let's assume we have calculated count0 and count1 like I've written above. Then:
while (window.size() > 1) {

Can be replaced with:
while (count0 + count1 > 1) {

Then you do:
if (count0 > count1)
    window.erase(--window.end());
else if (count1 > count0)
    window.erase(window.begin());

Basically, this removes zeroes or ones until the number of zeroes or ones is equal. This can be replaced with the following:
count0 = count1 = std::min(count0, count1);

Then the next condition is:
if (str.find("10", std::distance(str.begin(), it)) == std::string::npos)
    window.erase(window.begin());
else
    window.erase(--window.end());

Since str is not modified here, you could move the if-condition outside of the loop. And the window.erase() calls can be replaced by count0-- and count1--. So what's left is:
bool has_second_10 =  str.find("10", next0 + count0) != str.npos;

while (count0 && count1) {
    if (count0 != count1) {
        count0 = count1 = std::min(count0, count1);
    } else {
        if (has_second_10) {
            count0--;
        } else {
            count1--;
        }
    }
}

But wait, it gets even better: you always start with a window with both ones and zeroes, and you make the count equal, and then you remove zeroes if the original string had a second occurrence of "10", otherwise you remove ones. So the single character left in the window is completely determined by has_second_10, and the while-loop can be eliminated entirely. You also don't need to count ones and zeroes anymore.
Now it's just a matter of updating the original string. Instead of modifying a character and erasing some, you can use the std::string::replace() function to do that in one go:
str.replace(first1, next1 - first1, 1, has_second_10 ? '1' : '0');

Summary
With the above improvements, the code can be simplified to:
#include <iostream>
#include <string>

int main() {
    int T = 0;
    std::cin >> T;

    while (T--) {
        int n = 0;
        std::cin >> n;
        std::string str;
        std::cin >> str;

        while (str.find("10", 0) != str.npos) {
            auto first1 = str.find('1');
            auto next0 = str.find('0', first1);
            auto next1 = str.find('1', next0); // this might be npos, but that's fine
            auto replacement = str.find("10", next1) == str.npos ? '0' : '1';
            str.replace(first1, next1 - first1, 1, replacement);
        }

        std::cout << str << "\n";
    }
}


var todayDate = moment().format('DD').replace(/^0+/, ''); // "1"

Looks like you're taking a zero-padded day of month here... and then stripping off the leading zero.

You should use moment().format('D'), it will give you day of month without a leading zero.

Aside from that... you shouldn't use multiple instances of moment(). What if the time changes between the two lines of code? You might get things like 31 February.

var todayMoment = moment();

var todayDay = todayMoment.format('dddd'); // ["Monday"]
var todayDate = todayMoment.format('D'); // "1"
var todayMonth = todayMoment.format('MMMM'); // ["January"]

Like so, both problems are fixed.


API Changes

Currently, as a user of your tree, I have to figure out whether or not I need to call addTree, destroyTree, printTree or their corresponding helper functions. These helper functions are implementation details of the tree and they should not be exposed to the user. Ditto with the treeNode struct. Moreover, if you will be dynamically allocating your tree, you should just be doing so inside the createTree function and then returning that pointer back to the user. I also don't see a need for the typedef to a struct treeCDT *. I personally prefer knowing what the exact type I'm dealing with here and treeADT really doesn't tell me anything. Your header file really should be this:

BST.h

typedef char treeElement;
struct treeCDT;

void addTree(struct treeCDT *, treeElement);
void destroyTree(struct treeCDT *);
struct treeCDT *createTree();
void printTree(struct treeCDT *);

Then, in your implementation file, you can implement all your helpers and mark them as static; this way, your user cannot call them from outside code. You can now define your treeNode and treeCT struct here as well:

BST.c

typedef struct TreeNodeTag{
    treeElement  element;
    struct TreeNodeTag  *left, *right;
} treeNode;

typedef struct treeCDT {
    // ...
} treeADT;

static void addTreeHelper(treeNode**, treeElement)
{

}

static void destroyTreeHelper(treeNode*)
{

}

static void printTreeHelper(treeNode*)
{

}

Bug

You have an error that can cause undefined behavior in your create tree function:

void createTree(treeADT* tree){
  *tree = malloc(sizeof(treeCDT)); // error here.
  if(!*tree){
    fprintf(stderr, "Problem allocating tree");
  }
}

Remember that malloc returns a pointer to memory without initializing it; in other words, the values could be anything! Therefore, the value in the root member of your treeADT is not initialized to NULL like it should be. (See below for alternate implementation).

Factor your code out

As it is now, your implementation is pretty limited as there are no searching, removing, or other utility functions. You can imagine that there will be a need to constantly create new tree nodes. I would make that a function (and mark it static).

static treeNode *createTreeNode(treeElement elem)
{
    treeNode *node = calloc(1, sizeof(*node));
    if (node){
        node->element = elem;
    }
    return node;
}

Now your tree creation function can be:

struct treeCDT *createTree()
{
    return calloc(1, sizeof(struct treeCDT));
}

and the insertion function is trivial now:

void addTree(struct treeCDT *tree, treeElement elem)
{
     if (!tree) { return; }

     if (!(tree->root)) {
         tree->root = createTreeNode(elem);
     } else {
         struct treeCDT *copy = tree->root;
         int inserted = 0;

         while (!inserted) {
             if (elem < copy->element) {
                 if (copy->left)
                     copy = copy->left;
                 else {
                     copy->left = createTreeNode(elem);
                     inserted = 1;
                 }
             } else if (elem > copy->element) {
                 if (copy->right)
                     copy = copy->right;
                 else {
                     copy->right = createTreeNode(elem);
                     inserted = 1;
                 }
             } else {
                 inserted = 1;
             }
         }
     }
}


Upload csv file to sqlserver with python

First of all, when in doubt, profile.

Now a not-so-wild guess. Most of the time is wasted in

    qcnt="select count(1) from "+tb
    resq=c.execute(qcnt)
    rq=c.fetchone()
    rq=str(rq[0])

In fact, the rq is incremented by each successful insert. Better fetch it once, and increment it locally:

    qcnt="select count(1) from "+tb
    resq=c.execute(qcnt)
    rq=c.fetchone()

    for row in csvfile:
        ....
        insert = ....
        c.execute(insert)
        rq += 1
        ....

Another guess is that committing each insert separately also does not help with performance. Do it once, after the loop. In any case, you must check the success of each commit.

Notice that if there is more than one client updating the table simultaneously, there is a data race (clients fetching the same rq), both with the original design, and with my suggestion. Moving rq into a column of its own may help; I don't know your DB design and requirements.

Consider a single insert values, wrapped into a transaction, instead of multiple independent inserts.

Testing for cnt > 0 is also wasteful. Read and discard the first line; then loop for the remaining rows.

Figuring out whether a field represents a date seems strange. You shall know that in advance.


Searching a string of consecutive characters in a binary bar code

The constant SIZE and its comment don't quite seem consistent.  A name such as size would correspond to an actual size of, say, a structure.  But with such a comment, it doesn't seem to be for that.  Consider renaming the constant to something more accurate.
bar should be const since it's not modified.
test seems like an ambiguous name for an integer variable.  It sounds more like a Boolean test flag, but that's not the case here.  Consider renaming it to something more accurate.
Since tempstring is only used within the while loop, it can just be initialized there.  It doesn't need to be declared before it.
You shouldn't mismatch integer types when doing comparisons:

while (count < bar.length())

Since length() returns an std::size_t, which is an unsigned type, have count be of type std::size_t as well.  If you had compiler warnings up, then this should've been reported.
This may be a small nit-pick, but you can just return 1 instead of calling exit() in main().


Python program to rank based on the frequency of names that appears in text files

I suggest using os.path.join instead of + to handle dangling slashes, when concatenating file names.

This variables:

filenames = [ 'Q1.txt', 'Q2.txt' , 'Q3.txt' , 'Q4.txt']
pathToDataFiles = "C:\\DataFiles\\"

could be better handled by using command line input like in argparse. And for filename you could even use os.listdir to read all files from a directory. Both would give you more flexibility in using this script.

Also, the variable eachname in for eachname in allNames: should be renamed in something like name, to prevent lines like emptyDataFrame = emptyDataFrame.append([{'Name':eachname}]) from looking weird.

There are a few incoherent whitespaces, too, and I think Nan is meant to be NaN (in comments).

Apart from that, it looks like a good start to me. I hope my answer helps and have fun coding!


You say performance is a concern, but you're doing this.

select a.* from

That's poor form whether or not you already have a table scan in the underlying query plan for the CTE.

I think you misunderstood the article you linked to. It is warning against doing exactly what you've done here. SQL just doesn't lend itself to code reuse in the way a "regular" programmer is accustomed to. It's a different beast that way, it takes a different mindset.

SQL is a set based query language. Code reuse comes in the form of (well tuned) views and stored procedures. If you need this kind of fine grained control over the SQL, then keep it where it belongs, in the database.

Depending on just how dynamic your queries are, you may not always be getting the benefit of query plan caching. Each time a query that the analyzer hasn't seen before is processed, it has to generate a new plan, so you'll take a performance hit.

Lastly, I left a comment, but I should mention it here too. Be careful using query hints. You may be able to outwit the analyzer now, based on the current data, but the data will change over time. When the data has changed and your hint is no longer the most efficient query plan, the analyzer won't be able to choose the most efficient plan because you've told it not to. Performance gained this way may degrade over time.


PEP 8

Please follow PEP 8, you ca use tools like pylint to find PEP 8 issues immediately. But for instance:

Function -> function
Pixel    -> pixel

There are other small related issues.

Docstrings

Python has docstrings which are ways to document your functions. It isn't exactly clear what Function or Pixel do at the moment.

You have a comment describing Pixel. Two things about this comment:

I don't understand it, what do you mean by "create a pixel"?
It is not, as of now a docstring, to make it one, use the triple quoted string and put right below the function declaration, so:

def Pixel(point):
    #set up how the complex number will create a pixel

Should be:

def Pixel(point):
    """Set up how the complex number will create a pixel"""

But, again you should probably reword the comment, at the moment the meaning is not so clear, possibly:

def Pixel(point):
    """Calculate the color threshold for a particular pixel"""

Generalization

I would argue that you should add an additional parameter to Pixel. Function is one of the many functions you could pass into the set so allow someone else to change it and add f as a parameter to Pixel and pass the function in.

Optimization

What would I change to optimize the code?*

Instead, I would first switch to PyPy (which uses JIT compilation to make the code, in this case, much faster) and run the program that way, install PyPy, then (this worked for me):

pypy -m pip install Pillow

to install Pillow on pypy run it. Here are the time comparisons on my computer:

➜  codereview time pypy julia.py
pypy julia.py  3.10s user 0.07s system 99% cpu 3.198 total
➜  codereview time python julia.py
python julia.py  33.53s user 0.30s system 96% cpu 35.224 total

Just to be clear though, PyPy isn't always the best solution. It's great in this case though because you are repeatedly calling the same two functions over and over again, something that PyPy is generally much better than CPython at doing.

For a lot of, often considered, "slower" languages simply changing the implementation can do wonders.

* Well there are probably other things you can do, but switching the language is sooo easy, and helps with sooo much!


You can use Destructuring assignment in importing functions from ramdajs.

const {compose, reduce, ....} = require('ramda'); instead of using R.compose.


You can add the items to a Set instead of a List to avoid duplicates. That way you delegate to Set for uniqueness:
int [] arr = {5, 4, 3, 5, 4, 6, 7, 8, 6};

Set<Integer> set = new HashSet<>();

int length = arr.length;

for(int i = 0; i < length; i++){
    set.add(arr[i]);
}

System.out.println(Arrays.toString(set.toArray()));


Instantaneous validation of form field length

Ternary operator

You could use the ternary operator for your first if-else case:

warning.style.display = input.value.length >= 1 && input.value.length <= 3 ? 'block' : 'none';

Now, this is a bit lengthy so we might choose to store input.value.length inside another variable like so:

var length = input.value.length;
warning.style.display = length >= 1 && length <= 3 ? 'block' : 'none';

Note, that if you can use ES6 features you could benefit from using const and object matching and declare length as follows:

const { length } = input.value;

Direct assignment

As for your second scenario, you could store the conditional directly inside your variable since you just want the negated version of what you're checking:

button.disabled = length <= 3;

A more general code review

The comparison against 1 and 3 is very vague and they are considered magic numbers. What do these variables represent?

You could incorporate another self explaining variable, such as isWarning as follows:

// ES5
var length = input.value.length;
var isWarning = length >= 1 && length <= 3;

// ES6
const { length } = input.value;
const isWarning = length >= 1 && length <= 3;

warning.style.display = isWarning ? 'block' : 'none';
button.disabled = length <= 3;

This helps the structure, but still doesn't answer the question of what do the values 1 and 3 represent?

I'm assuming you aren't checking 0 since you don't want a warning to show on an empty input, so I'll provide an example of what could be done:

// For ES5 change const to var
const minLength = 4;

const isEmpty = length === 0;
const isValidLength = length >= minLength;
const isWarning = !isValidLength && !isEmpty;

warning.style.display = isWarning ? 'block' : 'none';
button.disabled = !isValidLength;

Note, that minLength would ideally have a more descriptive name related to what it is checking. minNameLength, minPostcodeLength, etc. This way the code reads relating to the logic it's checking, now how that logic is implemented each time.

Misc - ES6 deep object matching

If you are using ES6 you could use deep object matching to get length directly:

const { value: { length } } = document.getElementById('input');

This is useful since we are only concerned with the input's length, not the element itself.

Misc - Consistent use of quotes

Another small remark is your use of quotes. Either is fine but it's best to be consistent - you're using double quotes in getElementById but single quotes for setting display.

I'll continue to add things when I have time. If you have any questions/concerns please leave a comment.


You have written some good code, but what if the user does not enter a number, but makes a mistakes? You can, and should, catch that error. What about this:

import java.util.InputMismatchException;
import java.util.Scanner;

public class Calculator
{
    static Scanner scanner;

    public static void main(String args[])
    {
       try
       {
           scanner = new Scanner(System.in);
           double firstNum, secondNum, answer;

           System.out.println("Enter your first number: ");
           firstNum = scanner.nextDouble();

           System.out.println("Enter your second number: ");
           secondNum = scanner.nextDouble();

           answer = firstNum + secondNum;
           System.out.println(answer);
       }
       catch (InputMismatchException e)
       {
           System.out.println("Invalid input");
       }
       finally
       {
           scanner.close();
       }

}

}

I think a try - catch - finally is a great way of getting user input. This way your problem of the Scanner not being closed, is solved. If this is a little over you head right now, don't worry. It does what it says: we "try" the code in the try block, but if something goes wrong, in this case a "Inputmismatchexception", which means if the user inputs something wrong, we catch it and we display a message saying something went wrong.

Note that I have also changed the variable names. By reading the name, you should immediately know what it is and so names like firstNum and answer are a little better than inInt1 and stuff.

Hope you're still with me. If something is not clear, please feel free to ask some more questions. I myself am a beginner to Java and programming in general and this is the first question I answered.


Convert a string like 4h53m12s to a total number of seconds in JavaScript

getValue(str) is such a vague name for the function and its parameter, it could mean anything!  Furthermore, "get" implies that this is a getter function that retrieves something, which is not the case.
Your regex is ineffective.  Capturing parentheses could be useful, but you didn't actually use them right, such that you ended up having to pass a dirty string to parseInt() and extract the last character the harder way.
You neglected to scope match, such that it acts as a global variable.  The regex-matching statement is written twice; the assignment could be done within the loop condition instead.
The if statements should be an if-else chain, since the conditions are mutually exclusive.  However, since the branches are all so similar, a lookup table would be more elegant.

function durationSeconds(timeExpr)
{
 var units = {'h': 3600, 'm': 60, 's': 1};
 var regex = /(\d+)([hms])/g;

 let seconds = 0;
 var match;
 while ((match = regex.exec(timeExpr)))
 {
  seconds += parseInt(match[1]) * units[match[2]];
 }

 return seconds;
}

console.log( durationSeconds("4h12m32s") );

Alternatively, if you expect that the units will be in the conventional order, you don't have to loop at all.

function durationSeconds(timeExpr)
{
 var match = /^(?:(\d+)h)?(?:(\d+)m)?(?:(\d+)s)?$/.exec(timeExpr);
 return 3600 * (parseInt(match[1]) || 0)
        + 60 * (parseInt(match[2]) || 0)
        +      (parseInt(match[3]) || 0);
}

console.log( durationSeconds("4h32s") );


Depth First Search and Breadth First Search in C++

The implementation of the BFS and DFS algorithms themselves seems correct(but it is a good practice to write unit-tests to make sure that it works as intended, so you should do it). However, your code in general is not correct. It definitely leaks memory:

In the constructor, Adj is allocated: Adj = new vector<int>[V];. However, it is never deleted. You can do it in a destructor.
The same is true for the visited array(you should delete it at the end of the DFS and BFS member-functions).

Actually, there is a much easier way to deal with this issue: do not use pointers and dynamic memory allocation. I do not see any point in visited being allocated dynamically. Using an std::vector is much better(std::vector<bool> visited(num_of_vertices)). The same holds true for the Adj member-variable.

One more thing: you should try to keep the scope of variables as narrow as possible. There is no need to declare the vector<int>::iterator i at the beginning of the function. You can do it just inside the for loop where it is used.

The last thing: using namespace std; is a bad practice. It pollutes the global namespace.


Marking loans as paid if the payments exceed the loan amount

Your current approach isn't terrible. Yes, there's some coupling between the two models, but that isn't "illegal". Records in Rails, by virtue of being relational data models, typically have conceptual coupling already. And while functional coupling is different, it's sometimes the best solution. Heck, Rails even has a few features that rely on such coupling like delegating methods to an associated record, or the (similar to your setup) accepts_nested_attributes_for method, and the counter_cache option for associations, which does indeed update the "owner" record after create and destroy operations.

So for what it's worth, I'd say option 2 makes the most sense. If a Payment is created - however it's created - update its associated Loan. I wouldn't say it's difficult to test either: Create a loan, create sufficient payments, then reload the loan and check that it's been paid. Transactions are trickier only if your models a distributed across multiple databases, but even then they can be added.

I'd just make sure to also run mark_loan_as_paid after an update or destroy operation - if a payment is cancelled or changed, it's no good if the loan still appears as paid. And you can run periodic sanity checks on the data (e.g. with a rake task), if need be. For anything financial, that's probably a good idea no matter what.

But all that being said, it is of course nice to avoid coupling when you can. Again, in this case, I wouldn't bother. But if I was forced to, I'd say it's a matter of thinking beyond the Loan and Payment classes, because if the logic doesn't quite fit in either of those, then it's only logical that it fits somewhere else entirely.

One option is simply to do everything in the controller. It's precisely the sort of thing controllers are meant to do: Control access to and modification of data. In Rails you typically end up with a 1:1 structure of controllers and models (e.g. a PostsController for Post records), but that isn't a law. A user account system might have a SessionsController and a SignupsController and so on that all work on the same User model.

So you could easily have your PaymentsController both create a Payment record and update the Loan record. Nothing wrong with that. In fact, your controller is probably already loading the Loan anyway in order to do @loan.payments.create(payment_params).

Last option is to make Downpayment a "model" (just a plain class, not an ActiveRecord model) or a service object. Its responsibility would be the same as the controller in the previous description: Create a Payment record, and update the Loan record. So it's essentially the same logic, just extracted out of the controller.

In either case, you're back to a single responsibility: Pay down a loan. There are two models involved, but conceptually it's one task.

The tradeoff with either, however, is that you could technically create Payment without the Loan being updated by simply not using the controller/service object. But on the other hand you can do that too by just writing to the database without going through Rails.

An example:

def downpay(loan, amount)
  loan.transaction do
    loan.payments.create!(amount: amount)
    loan.update_attributes!(paid: loan.payments.total >= loan.amount)
  end
end


It seems unusual to have a member function that takes input, also considering that most of the work already done in the class is computational-based. Consider getting the input in main() and constructing a Date object from this, or overload operator>>.
This output should be written to std::cerr instead:

std::cout << "Invalid input.  Please enter a positive number: ";

This also makes it clear to the reader that this is not a standard output, but an erroneous one.
You have some magic numbers in isLearYear() and getCenturyValue().  Any of them can be in the respective functions as consts.
You don't need to initialize the private variables; that's for the constructor or initializer list.
Any member function, such as getCenturyValue(), that does not modify any data members should be const:

int Date::getCenturyValue() const
{
    return (2 * (3 - div(year / 100, 4).rem));
}

This not only makes the intent clearer, but also prevents any accidental modification of data members by giving a compiler error.
You could move your two free functions next to main() to avoid possible confusion with the member functions (at least while everything exists in one file).


Both solutions are correct. Some advantages of the "functional" approach
with reduce():

Concise code.
Returns a constant. It is clear to the reader that
total is assigned to exactly once, and the compiler may use this
information for optimizing code.
The type annotation on the result value is not needed:

let total = session.courses.reduce(0) { $0 + $1.totalPrice() }

because that can be inferred from the compiler automatically.
This will continue to compile without changes if the type of
totalPrice() is changed, e.g. from Double to Float.

To increase legibility, you can use named closure parameters instead
of the $n shortcuts:

let total = session.courses.reduce(0) { (partialSum, dish) in
    partialSum + dish.totalPrice()
}


Linked list node implementation with maximum code reuse

Usually linked lists keep most methods as private and the actual nodes cannot be accessed by the public API.

If you start implementing the different methods for adding, removing, etc. you might have on singly and doubly linked lists, you will find the implementations are quite different.  So much so that I think it would be simpler to have two completely separate classes.  They could both implement the java.util.List interface.  I'm a huge fan of code reuse, but I don't think it's a good idea in this case.  Trying to share some code between singly and doubly linked list would end up making the code less readable.  This is just a guess since I have not actually tried implementing them.


Find two numbers that sum closest to a given number

Your approach is good. Improvement you can do is use a better sorting technique and rest the approach you are using seems best one.

Sort all the input numbers (Use better sorting techniques Eg. Quicksort).
Use two index variables l and r to traverse from left and right ends respectively. Initialize l as 0 and r as n-1.
sum = a[l] + a[r]
If sum is less than number, then l++
If sum is greater than number, then r–-.
Keep track of min sum
Repeat steps 3, 4, 5 and 6 while l < r.

Time complexity: complexity of quick sort + complexity of finding the optimum pair

= O(nlogn) + O(n) = O(nlogn)


Running AWS commands works ok, but the code seems messy and I don't like the time.sleep bit

aws_object = json.dumps({"S3Object":{"Bucket":self.s3_bucket,"Name":self.filename}}).replace(' ', '')  # can't have any spaces lol picky AWS CLI
start_textract_command = "aws textract start-document-text-detection --document-location '{}'".format(aws_object)

This is not AWS which is picky, rather you’re not building the command line properly. If you provide a single string and start your subprocess using shell=True, each space will mark a new argument. Instead, you’d be better off building a list of arguments and let subprocess quote them properly:

start_textract_command = ['aws', 'textract', 'start-document-text-detection', '--document-location', json.dumps(…)]

and then just run subprocess.check_output(start_textract_command, stderr=subprocess.STDOUT).

Speaking of which, instead of using the older high level API, you should switch to subprocess.run:

process = subprocess.run(start_textract_command, capture_output=True, encoding='utf-8')
try:
    process.check_returncode()
except CalledProcessError:
    if 'InvalidS3ObjectException' in process.stderr:
       …
else:
    job_id = json.loads(process.stdout)['JobId']
    with open(self.output_filename, 'wb') as output_file:
       process = subprocess.run(['aws', 'textract', 'get-document-text-detection', '--job-id', str(job_id)], stdout=output_file, stderr=subprocess.PIPE)
    if process.returncode:
        log.error(process.stderr.decode('utf-8'))

Now for the time.sleep part, AWS Textract provides two modes of operations: synchronous and asynchronous. You can start by using the synchronous detect-document-text operation if it fits your need and you won't have to deal with the timing at all.

Otherwise, if you need to stick to start-document-text-detection, the completion of the process is published as a notification. I’m not at all versed in how SNS works but they have a tutorial to get you started on using Amazon SNS so you can create a channel, specify it in your textract job and wait for the completion event.


if !matches.empty?

equals

if matches.any?

And the second is preferable, cause you don't have negation.


A few notes:

clock_gettime() is not standard C, which means it's not guaranteed to be implemented on any system that supports C (including many Linux platforms, the few who implement it stuff it into time.h, but there are only a handful of standardized functions in that header). Relying on it for "portability" is not something you should do.
There is no standard C function that provides better than 1 second time resolution, but the POSIX function gettimeofday() provides microsecond resolution.

You can use this function like this:

struct timeval before = {};
struct timeval after = {};
struct timeval result = {};

gettimeofday(&before, NULL);

// Some code you want to time

gettimeofday(&after, NULL);
timersub(&after, &before, &result);

printf("Time elapsed: %ld.%06ld\n", (size_t) result.tv_sec, (size_t) result.tv_usec);

The other option is to just use the clock() function in time.h which is part of the C standard, but as stated before the time resolution is not great.
Please don't use busy waiting (side note: that would be a place for a for loop, not a while loop).  It's wasteful of CPU resources and could even be optimized out by the compiler.  It would be much more efficient and easier to test your code with the sleep() function as then you know how many seconds long the process should take.  If you want different resolutions for causing the current thread to be suspended from execution of your code, take a look at usleep() or nanosleep().


Basic expressive Swift background thread execution

Yes, I see what you mean: there do appear to be dodgy race conditions. I do like where you are going with this, though – dot syntax and all. Still I found at least one failing test. If you paste your code in a playground and then add the following:

/* your code here */

import XCPlayground
XCPlaygroundPage.currentPage.needsIndefiniteExecution = true

var i = 0

Background {
    i += 1 // don't call `print` here!
}.afterInterval(0) {
    i += 10
}.completion {
    i += 100
}

let t = dispatch_time(DISPATCH_TIME_NOW, Int64(NSEC_PER_SEC))
dispatch_after(t, dispatch_get_main_queue()) {
    print(i)
}

// prints: 101 (instead of 111)

True, delay of 0 seems a little contrived, but it does prove that your worries are founded. One way around this is to trigger a dispatch of those closures on demand (and repeatedly):

import Foundation
class Background {
    let qos: qos_class_t
    let task: (() -> ())?
    private(set) var delay: NSTimeInterval? = nil
    private(set) var delayedTask: (() -> ())? = nil
    private(set) var completionHandler: (() -> ())? = nil

    init(_ qos: qos_class_t = QOS_CLASS_BACKGROUND, task: (() -> ())? = nil) {
        self.qos = qos
        self.task = task
    }

    func delay(interval: NSTimeInterval, task: () -> ()) -> Background {
        self.delay = interval
        self.delayedTask = task
        return self
    }

    func completion(task: () -> ()) -> Background {
        self.completionHandler = task
        return self
    }

    func async() {
        dispatch_async(dispatch_get_global_queue(qos, 0)) { // TODO: unnecessary if `task == nil`
            self.task?()
            if let delayedTask = self.delayedTask, let delay = self.delay {
                let t = dispatch_time(DISPATCH_TIME_NOW, Int64(delay * NSTimeInterval(NSEC_PER_SEC)))
                dispatch_after(t, dispatch_get_main_queue(), delayedTask)
            }
            if let completionHandler = self.completionHandler {
                dispatch_async(dispatch_get_main_queue(), completionHandler)
            }
        }
    }
}

Which can be used like so:

import XCPlayground
XCPlaygroundPage.currentPage.needsIndefiniteExecution = true

var i = 0

let bg = Background {
    i += 1
}.delay(0) {
    i += 10
}.completion {
    i += 100
}

// nothing dispatched yet!

bg.async() // repeated invocations possible
bg.async()

Background().delay(1) {
    print(i)
}.async()

// prints: 222 (as it should)

In addition, I would probably rename Background...


Simpler method to find users in child categories

Some notes:

ys = [] + xs.each + ys.push + ys.flatten That's an imperative and pretty verbose pattern. A functional approach would use flat_map.
@users = @users.flatten.uniq. Don't re-use variables. Different values, different variables.
(current_user.get_voted Category). You saved two parens on the function call at the cost of being forced to write them to wrap the expression. It's not worth it. I'd just use parens on calls except in DSL-style statements. Check https://github.com/bbatsov/ruby-style-guide

I'd write:

def recommendations
  @users = current_user.get_voted(Category).
    flat_map { |category| category.children.flat_map(&:users) }.
    uniq
end


Overall you have a good D3 code here. I'm fairly impressed with the questions I've seen here at C.O. lately, from people claiming "This is the very first time I've used d3 or drawn a SVG". Congrats.

However, before sharing my proposed alternative, I'd like to tell you that, unfortunately, you're using the wrong tool for the task!

As you can see in my answer here, the problem is that D3 is designed to create visualizations based on data, normally qualitative or discrete quantitative data sets. According to Mike Bostock, D3 creator:

  D3 is designed primarily for data visualization, mostly empirical datasets rather than continuous functions, and so there is no built-in method for generating abscissa values. (emphasis mine)

As you can see in your case, the line gets better if you push more datapoint into the array, increasing any of the two constants in...

d3.range(numWaves*numPointsPerWave+1)

In your particular case we can get a good line with numPointsPerWave = 10, which is not a big problem... however, the advice remains: D3 is not the correct tool here, you should look for a proper plotting library. As you can see in the linked answer above, in some situations we have to increase the data points a lot to have a good looking graph.

D3 radial line

All that being said, here is my proposed alternative: instead of all that complicated math and 2 functions to set the path's d attribute, use a D3 radial line generator.

In this answer I'll focus only on the use of the radial line generator, nothing more. I'm sure that other users will soon post answers regarding your JavaScript code (use of functions, constants, destructuring, currying etc...)

According to the API, d3.lineRadial():

  Constructs a new radial line generator with the default settings. A radial line generator is equivalent to the standard Cartesian line generator, except the x and y accessors are replaced with angle and radius accessors. Radial lines are always positioned relative to ⟨0,0⟩; use a transform (see: SVG, Canvas) to change the origin.

So, all we need is the line generator...

const radialGenerator = d3.lineRadial()
    .angle(d => d.angle)
    .radius(d => d.radius)
    .curve(d3.curveCardinalClosed);

And the adequate data:

const length = 100;
const amplitude = 20;
const radialScale = d3.scaleLinear()
  .domain([0, length])
  .range([0, Math.PI * 2]);

const data = d3.range(length).map(function(d) {
  return {
    angle: radialScale(d),
    radius: xScale(radius) + Math.sin(d) * amplitude
  }
});

Then, you append the path:

const wave = vis.append('path')
  .attr('d', radialGenerator(data))
  .attr('fill', 'none')
  .attr('stroke', 'black')
  .attr('stroke-width', '1px');

Have in mind that my function here is not as complex as yours, which accepts several different parameters: this answer is just to show you the existence of d3.lineRadial(), you can certainly improve it.

Here is the demo:

const svg = d3.select('svg');
const margin = {
  top: 50,
  right: 50,
  bottom: 50,
  left: 50
};
const width = +svg.attr('width') - margin.left - margin.right;
const height = +svg.attr('height') - margin.top - margin.bottom;

// content area of your visualization
const vis = svg.append('g')
  .attr('transform', `translate(${margin.left+width/2},${margin.top+height/2})`);

// show area inside of margins
const rect = vis.append('rect')
  .attr('class', 'content')
  .attr('width', width)
  .attr('height', height)
  .attr('transform', `translate(${-width/2},${-height/2})`);

// show scales
const xScale = d3.scaleLinear()
  .domain([-100, 100])
  .range([-width / 2, width / 2]);
const yScale = d3.scaleLinear()
  .domain([100, -100])
  .range([-height / 2, height / 2]);
vis.append('g').call(d3.axisTop(xScale));
vis.append('g').call(d3.axisLeft(yScale));

// draw circle
const pi = Math.PI
const radius = 63.66
const circle = vis.append('circle')
  .style('stroke-dasharray', '3, 3')
  .style('stroke', 'black')
  .style("fill", "transparent")
  .attr("r", xScale(radius))
  .attr("cx", 0)
  .attr("cy", 0);

const length = 100;
const amplitude = 20;

const radialGenerator = d3.lineRadial()
  .angle(d => d.angle)
  .radius(d => d.radius)
  .curve(d3.curveCardinalClosed)

const radialScale = d3.scaleLinear()
  .domain([0, length])
  .range([0, Math.PI * 2]);

const data = d3.range(length).map(function(d) {
  return {
    angle: radialScale(d),
    radius: xScale(radius) + Math.sin(d) * amplitude
  }
});

const wave = vis.append('path')
  .attr('d', radialGenerator(data))
  .attr('fill', 'none')
  .attr('stroke', 'black')
  .attr('stroke-width', '1px')
svg {
  background-color: steelblue;
}

.content {
  fill: lightsteelblue;
}
<script src="https://d3js.org/d3.v5.min.js"></script>
<svg width="1000" height="1000"></svg>


regarding 1 you can:

new_content = [c for c in old_content if c != '\n']

or simply

new_content = old_content.replace('\n', '')

Regarding 5, if your are generating anything that is nontrivial, then it will pay off to learn some template engines like Jinja2. If that is too much, then you can make a simple template in text file and use regex or even replace() to substitute generic parts:

# template

%FOO%

%BAR%

and on the python side:

values = {"%FOO%": "the foos", "%BAR%": "the bars"}
template = open('template').read()
for k, v in values.iteritems():
    template = template.replace(k, v)
print template


result.shift 2 probably doesn't do what you think it does. You meant unshift.

shift removes the first element of an array (and doesn't take any arguments).
unshift adds new items at the beginning of the array.

As a result, your final array isn't [2,3,5,...] but [5,...]

Also, I'll point out the bug in the code you have in your "equivalent Javascript code" link - it has >= in the  result = (n for n in numbers when n >= 0) at the bottom instead of n > 0 (which is correct).

You probably guessed that since your code here has that fixed. n >= 0 doesn't filter the 0 elements from the array, n > 0 does.

And now for the functional ways. (Disclaimer: I don't think my functional rewrite of your code is any prettier - but it's more functional. Maybe someone can make it look more elegant?)

I'm no expert on functional programming, but from what I've learned, you generally try to use things like map, reduce, filter.

[].filter() takes predicate (test function returning boolean) that gets called with all the elements one by one and returns array with values that pass the test. You kind of already used it in the (n for n in numbers when n > 0) - it's the when part.
[].map() takes a "transforming" function that gets called with all the elements one by one and returns array with the transformed values. With CoffeeScript we can cheat again and use list comprehensions: (my_function(x) for x in xs).
[].reduce() is probably the least intuitive one - but we'll use it in your sieve :) You give it an initial value (accumulator) and a transforming function and it reduces the array left to right to a single value. The canonical example is summation: the_sum = [1,2,3].reduce(((acc, val) -> acc + val), 0) # the_sum is 6 now.

Now, when I hear "functional paradigm," I hear "loopless way" and "immutability." Functional programming is about more things, of course.

EDIT: oh man, I had limit = 100 in the code. Stupid mistake :) Fixed now.

sieve_sundaram = (limit) ->

  start = 4
  half = limit/2

  numbers = (n for n in [3...limit+1] by 2)
  steps = numbers[..] # copy of numbers

As you can see, numbers equals steps, at least at the first go. What we'll change will be the loops. Instead of thinking in terms of  for ... do ..., we'll think more like result = (... for ...) - transforming the arrays with our map, filter, reduce functions or their CoffeeScript counterparts.

Now, we have to define the transforming function for the reduce(), because we want to do something like steps.reduce(callback,numbers). You see, with a little bit of imagination you had for step in steps do something in your code.

Now we'll do exactly that - our function for the reduce will get a step at every call, just like your loop did. So that's that.

Next problem we have is rewriting the "global" changes you make inside the loop. So we need to do something about the numbers[i-1] = 0 and start += 2 * (step + 1) lines. We'll make the changes propagate through the result values.

But how can we make two things propagate through one value? We'll use an array. Remember, numbers is an array and start is a number. Essentially we'll insert the start at the beginning (or the end, wouldn't matter) of the numbers array and at each call of our reduce callback extract it from there - and of course, after the reduce finishes.

  callback = (start_and_nums,step) ->

    start = start_and_nums[0]
    nums = start_and_nums[1..]

So that's the extraction part. Pretty trivial with CoffeeScript.

Next part is little bit trickier. If you're familiar with recursion, you know the base case is usually written before the other parts of recursive function. We'll do something similar - check for your if start > half condition before we do anything else.

If it's true, it will simply mean that we don't need to do any of that sieving mumbo jumbo, that we're done and just need to wait till we get through all the elements, because we can't stop the reduce function midway.

    if start > half
      result = (n for n in nums when n > 0)
      result.unshift start
      return result

      # We can't stop iterating inside the reduce() although we'd like to right now.
      # Also, we slice the first element every time our function gets called,
      # so we'll put it back here for now. We'll deal with it after reduce() finishes.
      # Also, we mustn't forget about the 2!

Next portion of the code is reached only if we still have work to do (we didn't get to the return inside the if). It's outdented, but maybe that's hard to see with my comments between the lines.

Now, look at your for i... line. What you really want is i-1, because you use it in the next line (numbers[i-1]). So we'll use map. It should be pretty straightforward:

    idxs = (idx-1 for idx in [start...half] by step)

OK, we have the is, now we have to deal with the sieving. If you look at your code and think about it in terms of transforming one array to another, you realize you either change an element to 0 if it was unlucky and its index was in the [start...half] by step, or don't change anything if the index wasn't there. Look how it translates to code:

    new_nums = ((if i in idxs then 0 else num) for num,i in nums)

We dealt with the sieving, now we need to change the start variable. That's trivial:

    new_start = start + 2 * (step + 1)

And we need to prepare these new values for the next "iteration" - and return them. You can see that instead of deciding whether to finish or loop again right after changing the start, we delegate that to the beginning of the callback.

If we had it here instead of at the beginning, we'd do unnecessary (and potentially harmful) changes to the numbers array, because we can't stop the reduce from getting through all the steps. That's a minor disadvantage of this approach - we do some unnecessary work. But we can limit it to just finding out if we actually need to do something or can just return immediately.

    new_start_and_nums = new_nums[..]
    new_start_and_nums.unshift new_start
    return new_start_and_nums

That's the end of our callback function. Now remember, the function works with start_and_nums, but so far we have only the numbers. Let's fix that:

  start_and_nums = numbers[..]
  start_and_nums.unshift start

And let's finally run the reduce function!

  result = (steps.reduce(callback,start_and_nums))

We don't really need the start value at the beginning.

  result.shift()

But we need to put in the 2! The point of doing it here and not in the callback is again that we can't stop reduce from stopping. If we inserted it in there, it would get inserted multiple times.

  result.unshift 2

Aaaand we're done.

  return result

By the way, thanks for your question - I see the Sundaram's sieve for the first time. Seems very elegant :)

The final code: either copypaste the snippets above or use this gist :) But I sincerely hope you at least read through this and not just use the final thing, because of the time I put into writing the explanations... :D


Another approach is to let the recursive method know if the previous digit was an eight.

public int count8(int n) {
  return count8(n,false)
}

public int count8(int n, boolean lastDigitWasMatch) {
  if(n == 0) {
    return 0;
  } else {

    boolean match = (n % 10 == 8);
    int count = match?(lastDigitWasMatch?2:1):0;
    return count+count8(n/10,match);
  }
}


The jungle
OO design tends to create unneeded eco systems when the focus is the objects not the task at hand.
In this case Point, Circle, points and supporting functions have nothing to do with the problem. They exist to serve each other.
You clear the canvas so you can draw all the points, seams to me that the only reason you keep an array of points is so you can draw them after you clear the canvas.
Storing the points means no machine today using your function will ever be able to solve PI to over 6 digits as none have either the memory to hold all the points, nor the time to draw them.
Having an open setInterval (no handle to stop the interval)  constitutes a memory leak because you push to the points array each time.
The task is to calculate Pi via Monte Carlo method and show the points, it can be done without the overhead,of the OO eco system and supporting scope.
General point

Use const for constants.

ECMAScripts Math has a hypot method already, however it is much quicker to do the calculation inline. As the calculation requires the radius squared, you may as well store the radius as such and then test if inside with x * x + y * y < radiusSqr

window is the default object, you don't use it for window.parseInt so why use it for innerWidth?

canvas.width, and canvas.height represent the resolution of the canvas. They can only be integer values. parseInt is thus redundant.

You are moving the ball to the center of the canvas and with it all the points. As such need to move each point back to the origin to calculate its distance from the ball. If you moved the canvas origin to the canvas center you only need the radius, and you don't need to move the points to the ball origin to get the distance.

The calculation works for random distribution of points. You introduce a very non uniform distribution by rounding the points to the nearest pixel.

Every 10 ms you redraw the canvas content. All browsers have a MAX display rate of 60 frame per second. you draw 100 so 40 of the frames you draw are never seen. Never render to the DOM or to canvas using setInterval at rates above a few frames per second. Use requestAnimationFrame

You Search the DOM for an element with id "out". every 10 ms. Better to get it once at the start. better yet use direct reference via the id See example

You call the Number.toString when you set out.textContent ECMAScript does type coercion automatically you don't need to call toString when assign a number to a string

Your DOM element ids are rather poor and could be improved

You should set the body margin and padding to "0px" so that the canvas fits without needing to have overflow clip the canvas.

If you don't need to locate an element uniquely use a CSS class to set its style rather than id

Use the setTimeout rather than setInterval to give a interval between rather than an interval over. Many devices need time to cool or they will force the issue and reduce the clock speed. setInterval can result in back to back calls to the function giving no rest for the CPU/GPU, the result will be slower performance for all services the machine is running.

Example
The example removes the overhead of storing points.
The constant pointsPerInterval  defines how many points to calculate each interval. The points are added to the current 2D context path and only rendered when it can be displayed using requestAnimationFrame
Having high rated of points quickly results in a black canvas. So the number of points drawn per frame is only a sub set of the points used. Thus it draws only ~1000 points per second and uses ~10million points per second to calculate PI.
The example also exposes a simple interface to reset the animations, or stop.
Also shows number of points use. "Mp" represents Mega points (1,000,000 points)
Click the canvas to reset.

"use strict";
const calculatePi = (() => {
    const ctx = canvas.getContext("2d");
    const width = canvas.width = innerWidth, halfWidth = width / 2;
    const height = canvas.height = innerHeight, halfHeight = height / 2;
    const radiusSqr = Math.min(halfWidth, halfHeight) ** 2;
    const areaScale = width * height / radiusSqr;
    const pointsPerInterval = 100000, interval = 10, showPointCount = 10;
    var pointCount, inCircleCount, timeoutHandle;

    ctx.setTransform(1, 0, 0, 1, halfWidth, halfHeight); // Move origin to cent of canvas

    resetCanvas();
    requestAnimationFrame(render);
    function resetCanvas() {
        pointCount = 0;
        inCircleCount = 0;
        ctx.clearRect(-halfWidth, -halfHeight, width, height);
        ctx.lineWidth = 5;
        ctx.beginPath();
        ctx.arc(0, 0, radiusSqr ** 0.5, 0, Math.PI * 2);
        ctx.stroke();
        ctx.beginPath();
        addPoints();
    }
    function render(){
        ctx.fill();
        ctx.beginPath();
        currentPiId.innerText = inCircleCount / pointCount * areaScale;
        pointCountId.innerText = pointCount / 1000000 | 0;
        requestAnimationFrame(render);
    }
    function addPoints() {
        var i = pointsPerInterval;
        pointCount += pointsPerInterval;
        while (i --) {
            const x = -halfWidth + Math.random() * width;
            const y = -halfHeight + Math.random() * height;
            i < showPointCount && (ctx.rect(x, y, 1, 1));
            inCircleCount += x * x + y * y < radiusSqr ? 1 : 0;
        }
        timeoutHandle = setTimeout(addPoints, interval);
    }
    return Object.freeze({
        stop() { clearTimeout(timeoutHandle) },
        restart() {
            this.stop();
            resetCanvas();
        },
    });
})();

canvas.addEventListener("click", () => calculatePi.restart());
#canvas  {
  position: absolute;
  top: 0px;
  left: 0px;
}

.box {
  position: absolute;
  z-index: 1;
  /*width: 200px;*/
  height: 20px;
  left: 5%;
  top: 5%;
  background-color: whitesmoke;
}
<canvas id="canvas"></canvas>
Mp = Pi &#x2248;


Data mapper reuse related domain object for setting data and return a clone of the domain object

I can't speak to cloning specifically, as I've never used it before, but here are a few basic things to think about.

Is the User class used in any other methods of your UserMapper?

If it is then injecting it in the constructor, or through a setter is ideal as it no longer depends on a user object being passed to it in each method.

This is basic OOP.

Are you perhaps trying to do too much for the scope of this class?

If we follow the Single Responsibility Principle, as we should, then our classes should be responsible for just one thing. According to WikiPedia:

  A Data Mapper is a Data Access Layer that performs bidirectional transfer of data between a persistent data store (often a relational database) and an in memory data representation (the domain layer).

To me, this means that a Data Mapper is essentially a database read/write object. What you have here seems to be a Factory object that builds individual User objects. If what you are trying to accomplish is indeed a Factory and not a Data Mapper, then you should call it what it is to avoid confusion. But for a Data Mapper the assign() method seems to be out of scope. You might consider refactoring your code, perhaps create a new class if necessary. If this is the only violation, then perhaps a better way of doing this would be to simply create the new object out of the class and remove the assign() method.

Is your User class perhaps doing too much?

It seems odd seeing a user object that allows you to change its name on the fly like this. Most user objects, that I'm familiar with, perform these kinds of operations in instantiation. Allowing this kind of change is invisible to the rest of your code. In other words, between one method and the next, the object can change and your code wont know the difference. So it is possible to encounter "state" errors, such as information from a previous user not being overwritten by the new user.

Sorry I couldn't be more precise, but with so little to work with its hard to determine what you are trying to accomplish. Hope this helps.


Python array literal syntax sugar similar to Perl and Ruby

If you call split with no arguments, it will split on any whitespace and drop any empty elements. Knowing that, it's as simple as this:

ary_names = """
    alpha
    bravo
    charlie
    delta
    echo
""".split()

If you have only five short elements, you may want to put them all on one line:

ary_names = 'alpha bravo charlie delta echo'.split()

Of course, you'd only want to do that if it fits easily on one line.


Check if one string is a permutation of another using Python

Yes, len(str) should be O(1) in Python.  (Good question!)  Each of your for loops is O(n), so your whole function is O(n).

Your counting loops could be written more compactly as

for char in first_string:
    count_first[char] = 1 + count_first.get(char, 0)

The epilogue could be simplified to

return count_first == count_other

It pays to get familiar with the standard Python library, though.  Your entire function could be more simply implemented as

from collections import Counter

def is_permutation(a, b):
    return len(a) == len(b) and Counter(a) == Counter(b)

… where len(a) == len(b) is an optional optimization.  Writing less code simplifies maintenance and tends to create fewer opportunities for introducing bugs (as in Rev 2 of your question).


I confess to not being entirely clear as to the purpose of this code.  It seems to me that any list known at compile time could more easily and more correctly handled by other forms of preprocessing.  With that said, though, here are some comments on this code.

Think about real machines

The code for the List class currently includes this data member:

  T array[std::numeric_limits<int>::max() >> 2];

On my machine, this attempts to allocate 2GB on the stack for an int-based List. That's a rather large amount of memory to attempt to allocate for every List!  Perhaps this could be trimmed to some reasonable value.

Reduce the number of requirements on template types

The code requires a number of operations on the underlying T type.  It requires both < and >= but that requirement can be relaxed somewhat by defining GE's operator like this:

constexpr bool operator()(T t) const {
    return !(t < pivot);
}

Pass const references where practical

Rather than passing by value, it would probably make more sense in the general case to pass by const reference.  So for example, the previous function could be written like this instead:

constexpr bool operator()(const T &t) const {
    return !(t < pivot);
}

In fact, one of the only places that can't be treated this way is the argument to merge() which must be passed by value.

Try it with a non-primitive type

Whenver you're creating templated code, carefully consider just how it might be used and what requirements it may demand of the underlying type.  One technique I use is to wrap a primitive type inside a goofy minimalistic wrapper for testing:

template <typename T>
class Goofy
{
public:
    constexpr Goofy(T n = 0) : num(n) { }
    constexpr Goofy(const Goofy &g2) : num(g2.num) { }
    constexpr Goofy &operator=(const Goofy &g2) { num = g2.num; return *this; }
    constexpr Goofy &operator+=(const Goofy &g2) { num += g2.num; return *this; }
    constexpr bool operator<(const Goofy &g2) const { return num < g2.num; }

    friend std::ostream &operator<<(std::ostream &out, const Goofy &g2) {
        return out << g2.num;
    }
private:
    T num;
};

template <typename T>
constexpr Goofy<T> operator+(const Goofy<T> &g1, const Goofy<T> &g2)
{
    Goofy<T> result(g1);
    result += g2;
    return result;
}

This represents a minimal interface that can then be tested with your template:

int main()
{
  constexpr List<Goofy<float>> list{0, 5, 8, 13, 1, 7};
  std::cout << list << std::endl;
  std::cout << list.reverse() << std::endl;
  std::cout << list.sort() << std::endl;
  constexpr auto m = list.sum();
  std::cout << m << std::endl;
}

Sample output

{0, 5, 8, 13, 1, 7}
{7, 1, 13, 8, 5, 0}
{0, 1, 5, 7, 8, 13}
34


You can declare the array passed into the constructor as const so it doesn't get widened to Array<[string, number]>, and use generics to look up the key on the constructed map to get the AvatarSize type:
const sizes = new Map([
    ["xs", 32],
    ["sm", 44],
    ["md", 104]
] as const);
export type AvatarSize = Parameters<typeof sizes.get>[0];

Another way of doing it would be to declare the array outside:
const sizesArr = [
    ["xs", 32],
    ["sm", 44],
    ["md", 104]
] as const;
const sizes = new Map(sizesArr);
export type AvatarSize = (typeof sizesArr)[number][0];


Codility's BinaryGap: Hardware-aware answer

Scoping the case body is unnecessary.
gapStarted flag is redundant (sizeGap has all the necessary information). Consider

while (N > 0) {
    int remainder{N%2};
    switch (remainder) {
        case 0:
            sizeGap++;
            break;
        case 1:
            if (sizeGap > longestSizeGap) {
                longestSizeGap = sizeGap;
            }
            sizeGap = 0;
            break;
        }
    }
    N /= 2;
}

To address your immediate concerns, there is too few variables to worry about cache, pages, or memory hierarchy. Everything will stay in registers. I don't think you can do anything about branch prediction; however, read the next bullet.
A nice to know approach is de Bruijn hashing:

find the length of trailing gap using a technique described in the answer
remember the length
shift the number left by that length
find the number of trailing ones (e.g. negate and find the number of trailing zeroes), and shift them off
keep going until the number becomes 0.

Such loop will do as many iterations as there are gaps, as opposed to the argument width in your code. Worst case is still the same. OTOH, notice that there is no branches at all.


Finding the number of inversions in an Array

A few things I've noticed.

You use List to represent the input data. Lists are inefficient (linear) for things like input.size (which you do twice on the same input) and input.slice().
Both countInversions() and splitInversions() are recursive but they're not tail recursive, so that's going to eat up stack space.
splitInversions() sends its passed parameters to mergeSort() which means that every time it calls itself (recurses) it re-sorts already sorted data.
Your calculations return an Int, which is going to be too small for a large data set such as the inversions.txt you've linked to.

But generally I find the whole thing rather too complicated.

Here's an alternative algorithm that is smaller, faster, and more memory efficient.

def countInversions(input :Vector[Int]) :Long = {
  val idxBuf = input.indices.sortBy(input).toBuffer
  input.indices.foldLeft(0L){ case (sum,x) =>
    val idx = idxBuf.indexOf(x)
    idxBuf.remove(idx)  //side effect
    sum + idx
  }
}


Drawing multiple polygons in Google Maps in the same style

You have several options to reduce the code duplication here.

Put the common values into variables, and replace all hardcoded values with those new variables.
Create a prototype object of all the properties, and when you create a new polygon instance, copy the prototype instance and overwrite only the field you want to change (the path in these examples).
I saved my favorite to last. Create a helper function that takes a path and returns a new polygon instance, configured with the common values, and the specified path.

Whichever way you choose, you will be able to change values in one place for all map instances, and thereby avoid much of the code duplication.


Simple Python script to sort and make json data unique

I'd recommend making use of a set, which stores only unique elements. That way you can just count insertions to get the total (with duplicates) and the unique ones are easy to retrieve.

Instead of iterating through unique emails and printing them with a comma and a space as a terminator (which gives you the odd comma and space at the end of the emails), make use of ", ".join().

Lastly, eMails should be emails in terms of punctuation, constants should be all caps, and camelCase is discouraged. For clarity, I changed the variable names to be more expressive.

Here is my complete solution:

import os, json

ORDERS_DIRECTORY = "C:\dev\orderArchive" # Reformatted constant.
unique_emails = set() # Holds only unique emails by design.
total_emails = 0 # Boring running total

# json objects stored in multiple files, itterate over all of them
for filename in os.listdir(ORDERS_DIRECTORY):
    with open(os.path.join(ORDERS_DIRECTORY, filename)) as data_file:
        data = json.load(data_file)
        unique_emails.add(data["details"]["email"]) # Make use of set here.
        total_emails += 1 # Corrected spacing here.

print("Comma dilimited e-mails: \n")
print(", ".join(unique_emails)) # Make use of join here.
print("\n\nNumber of unique e-mails is: {} (originally {})".format(len(unique_emails), total_emails))

On a side note, you may want to consider having some more error handling for the email field not being there, file not containing json etc. I'd also consider making the directory a program argument.

Update: fixed ORDERS_DIRECTORY consistency, use of os.path.join(), and str.format() per comments.


I'm not aware of any mathematical way to establish the upper bound for the search space (I used the same upper limit as you did). However, there are some optimizations you can make:

Use integer math throughout instead of converting to a string, extracting the digits, then converting back to an integer. On my PC, your algorithm ran in approximately 6200ms. Using integer math as follows (there may be a more Pythonic way to do this; my code is a direct transliteration of C++ that I used), it ran in approximately 1600ms -- almost 4 times as fast:

def findFactorialSum():
    factorials = [fact(x) for x in range(0, 10)] # pre-calculate products
    total_sum = 0
    for k in range(10, fact(9) * 7): # 9999999 is way more than its fact-sum
        tmp = k
        total = 0
        while tmp > 0:
            total += factorials[tmp % 10]
            tmp //= 10

        if total == k:
            total_sum += k

    return total_sum

(For my curiosity, I also tried it with the '/' operator instead of the '//' operator, and consistently got 100ms longer run times with the former.)
You're doing an exhaustive search of the numbers from [10...7*9!] to see which ones meet the problem criteria. You can eliminate a large proportion of those numbers:

Hint 1:

   No 2-digit number can have a digit >= 5. This is because 5! == 120, so any 2-digit number with a 5 (or higher) in it will automatically have a 3-digit (or longer) sum. You can extend this rule for 3-, 4- and 5- digit numbers.

Hint 2:

   Extending Hint 1: No 3-digit number can have more than one 7 in it. 7! is 720, so if you have two 7s, you have 1440, a 4-digit number. There are a few more opportunities like this to eliminate numbers to check

Hint 3:

   Think of the number 145 given in the problem; since you know that this works, there's no need to check numbers that are a permutations of its digits: 154, 415, 451, 514, 541. The trick here is to look at the problem the other way around: instead of finding numbers whose Sum of Factorials of Digits equals the original number, find an ordered sequence of digits whose Sum of Factorials can be split into a sequence of digits, sorted, and that compares equal to the original sequence.


Three-way join in Entity Framework Core to retrieve all roles that a user has

For this isolated piece of code, this is the way to go. Generally, in ORMs that support LINQ, navigation properties (like ur.Roles) should be used wherever possible. Usingjoin should be reserved for custom joins where no navigation properties exist.

Reasons:

Joins are far more verbose than navigation properties, making code harder to read and causing repetitive code.
Joins are error-prone. It's easy to match the wrong fields. A navigation property, once configured correctly, will always generate a correct SQL join.
Navigation properties, if well-named, are more expressive. In a join the cardinality of a relationship isn't always obvious. In navigation properties it is. You know what you're looking at when reading Role vs Roles. Unfortunately, sometimes people tend to be sloppy in this area and they just use the generated class and property names as an ORM generated them, which may result in any mixture of plural class names, plural reference property names, and singular collection property names.

One exception to this rule is when you want to overrule the ORM's SQL generation. Entity Framework will generate an OUTER JOIN for non-required navigation properties (i.e. where the "1"-side is optional, 0..1-n, 0..1-1). There may be cases where INNER JOIN performs better and the results with null navigation properties aren't interesting anyway. In those cases, using a manual join, overriding a navigation property, could be considered.

Side note: as said in a comment, it's recommended to use disposable objects, like a DbContext, in a using statement.


There is a bit of misnomer with RenderBatch. The term "batch rendering" is normally associated with grouping several geometries that share the same rendering attributes (textures/materials/etc) to render then together and avoid expensive state changes. Your class is just a wrapper around a Shader, so I don't see much reason for its existence, when all that could be part of Shader instead.

There is also an excessive amount of shader binding/unbinding going on inside RenderBatch. You definitely don't want to do that, changing shader programs is a very expensive operation.

That's one of the pitfalls of attempting to integrate OpenGL with OOP. The library was just not made to be used like that; it relies heavily on global state. This will change in the future with the new "bindless" OpenGL, but that's for version 4.5 and above.

Another potential issue that I see here is that RenderBatch is taking its Shader parameter by value, thus making a copy of it. How are you dealing with the internal GL shader program handle in this case? Even though you can copy the integer handle given to you by OpenGL, you can't copy the underlaying shader object itself. So I'm wondering if you are not winding up with duplicate shader handles through your program that will get deleted multiple times by calls to glDeleteProgram or even leak. Yet another problem of mixing a very C-ish API with C++. You have to be extra careful to avoid these sorts of mistakes. Be very aware that you are mixing two distinct programming languages in the same project.


Two similar API calls PHP optimisation and structure

I'd start firstly by moving your urlencode() into the class... You should assume that everything passed to a method is unclean and therefore act appropriately.

Secondly, I'd give the class a meaningful name - Process isn't entirely descriptive.

Throw some comments in - follow a set standard such as the PHPDoc format, using @param, @return etc. Always handy to have, even if the code doesn't become public or used by others.

Look at your public/private declarations - or lack of. Personally, I'd look to make your $search and $info URLs private, as well as your returnJson() method.

I'd possibly look to use cURL instead of file_get_contents() - due to some server configs disabling that etc.

Anyway, just a few suggestions!


It could be slow from all of the location redirects. But I've never done anything like that so I can't be sure.

Is it possible to lazily set the cookies. For instance, set a session and cookie on the first page load. And then, on the second page load check if the cookie is present.

This is how you could check if the page has loaded.

session_start();
$_SESSION['first_load'] = isset($_SESSION['first_load']) ? false : true;

if( !$_SESSION['first_load'] && isset($_COOKIE['testCookie']) )
  echo 'cookies enabled!'
else
  echo 'no cookies for you!'


This answer will reflect the naming convention at my work.

Naming Convention

Prefixing I to all your interface seems superfluous, since we use a simple name like ContractService. The implementation class would have Impl suffixed to the name of the service like : ContractServiceImpl. Normally you will use DI to inject the implementation where you need it, so you will only see the interface name.

Prefixing I seems odd to me if I'm looking at the Java collections API where you have : List, Map, etc.

If you only use ContractService as the name of the implementation, it could not be clear at first read that you are using an implementation directly and not the interface. Using Impl as a suffix make it clear that it's the implementation, so if you see it in your code, it can show that something smelly is going on.

Naming of methods

public interface IProductService extends IListService<Product> {
    public List<Product> getAllCurrentProducts();
    public List<Product> getAllArchivedProducts();
}

Those methods are very good! If you can keep clear names like those, using your services will be easy. I like the way you named all you method!

Delete

public void delete(long id);

You could return the object that has been deleted. It could be useful if you want to show the information of the item that has been deleted. The client could decide if he wants to use the object or not.

Edit: In fact, you should not return an object since you're just working with long id and not the object directly. If the signature was public void delete(T delete) this would have been a good advice, but with the current signature, you would have a to retrieve the object, delete it and then return the deleted object.


Organization
As Simon points out, you shouldn't put the list of functions inside of run. It doesn't make much sense. Move them out into the class.
Use self
self in python is equivalent to this in C# and many other languages.
This:
    guesses = []
    game_states = []
    length = int(input("Choose length of code to break: "))
    code = [random.randint(0, 9) for i in range(length)]

Should be this:
def __init__(self):
    self.guesses = []
    self.game_states = []
    self.code = [] # Add this later when you define length.

(Also, I would keep length as a local variable defined in run and then refer to len(self.code) instead.)
I would advise you look at how Python classes work.
You can replace random.randint(0, 9) with random.randrange(10).
Furthermore, once you pull the functions out of run, things like:
def take_guess():

become:
def take_guess(self):

Naming
        correct_guess_length = False

I expect a varaible with the name of length have some sort of integer (maybe real?) value. A boolean? Not often.
You don't really need this anyway!
            else:
                correct_guess_length = True

Can just be:
            else:
                break

I would probably just get rid of that too and say:
            else:
                return guess

Same thing goes for:
    correct = False
    while not correct:
        ...

You can get rid of these and use a break statement instead.
randrange
    code = [random.randint(0, 9) for i in range(length)]

Is pretty good except, randrange is a little more specialized for this:
    code = [random.randrange(10) for _ in range(length)]

(Also, when you disregard a variable the convention is to use _, not i.)
__str__
Since you rap everything in a game object, maybe you should give it a string representation overloading the __str__ function, and just print out the string representation of itself. Move print_game_state to __str__.


Workflow class with Start(), Continue(), and Resume() methods

Comments:

The structure and naming of this class says "I am a runner" but it isn't really a runner if it is basically a mechanism to persist and retrieve state. This is especially confounding because you have a separate state class. There's nothing to start, continue, or resume, except storage and retrieval of state from the session. In fact, conceptually, this class is the context, and your WorkflowContext and WorkflowState classes appear to be more appropriate runner objects.
Continue() and Resume()--combine them into one method where you either always pass values, or never pass values. Either the calling class needs to be able to check the session and retrieve values to pass in, or the method needs to be changed to be able to hydrate the session if it hasn't been initialized yet. This class should probably encapsulate direct session access, so I'd go with the latter. Otherwise, calling code has to make assumptions about the session, which is brittle and an encapsulation no-no.
All void methods and a ton of properties. This class exposes a lot of objects, and more specifically, objects that this class internally initializes. This makes your void methods stateful and full of side effects. If you are using this class to basically initialize and hold a bag of properties that you then access in business logic, why not make your methods stateless by returning a state object from this method? The current class is not really suited to be both the state (session) AND the state-getter. It would be much better to move the properties on this class to a separate state object, and change this class to have CreateNewSession() and GetExistingSession() methods that then return a state object. You can probably combine that state object with your other classes, too. Again, do ensure nothing external can assume anything about the application session.


I'd like to suggest two changes to your interface.

Implement Enumerable.  Your object accepts an Enumerable; it would be nice if it also acted as an Enumerable.  All you have to do is include Enumerable and rename run → each, and then it behaves more like the way a Rubyist would like to see.
Remove the separate calls to #satisfied_by? and #word.  I find it awkward that FizzBuzzer should have to know about the workings of the tester objects like that.  Why not just have the tester return a truthy output value to indicate that it has performed a transformation?

In addition, I would prefer to see the invocations of FizzBuzzer.new and fizz_buzzer.run written with parentheses, for readability.  There appears to be a consensus among multiple Ruby style guides that parentheses should only be omitted for simpler or specialized calls such as puts, include, attr_accessor, or methods that take no arguments.

class FizzBuzz
  include Enumerable

  def initialize(*transforms)
    @transforms = transforms
  end

  def each(values=1..Float::INFINITY)
    values.each do |value|
      words = @transforms.map { |transform| transform.transform(value) }
      yield words.any? ? words.join('') : value.to_s
    end
  end
end

class ModuloTransform
  def initialize(modulo, word)
    @modulo = modulo.to_i
    @word = word
  end

  def transform(number)
    (number % @modulo).zero? ? @word : nil
  end
end

class SquareRootTransform
  def initialize(word)
    @word = word
  end

  def transform(number)
    (Math.sqrt(number) % 1).zero? ? @word : nil
  end
end

fb = FizzBuzz.new(ModuloTransform.new(3, 'Fizz'),
                  ModuloTransform.new(5, 'Buzz'))
# FizzBuzz naturally works from 1 to infinity.  This is one way to limit
# the results to the first 100.
fb.take(100).each { |word| puts word }

fb = FizzBuzz.new(SquareRootTransform.new('(PerfectSquare)'))
# This is another way to do FizzBuzz only for certain values.
fb.each(1..100) { |word| puts word }


Academic/formal is not what's needed here. Simple is what's needed.

A dict whose keys are all integers from 0 to n is suspicious. Why not use a list instead? You can add items to the end with .append and remove them with .pop. Then you don't have to bother with max(_directory_stack.keys()).

This is the usual way to implement a stack in Python.

Edit: oh, wait, here's an academic/formal issue: popd doesn't restore correctly with relative paths. For example:

pushd('foo')
pushd('bar')
popd()

Should the directory now be foo or foo/bar/foo?

There are two ways to fix this:

pushd can convert the path to an absolute path with os.path.abspath.
Instead of saving the directory it's changing to, pushd can save the current directory (getcwd), and popd can just chdir to whatever it popped.

Which one to do depends on where you want to restore to if someone changes the directory without telling pushd. Consider this sequence:

pushd('/foo')
chdir('bar')
pushd('/baz')
popd()

Should the directory now be /foo or /foo/bar?


The biggest thing I see are the numerous SQL injection vulnerabilities enabled by concatenating variables with SQL together. Use Prepared Statements instead.

You can reduce nesting in code by returning early from the function.

$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

$sql = "insert into tableB (idno, aid) values('".$idno."','".$aid."')";
$query = $this->db->query($sql);
...

if (!isset($query))
    return false;

$sql = "select '".$idno."' as idno, name, age, sex, email from tableA where aid='".$aid."'";
$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

return $query->result_array();

Pass the e-mail and password values as explicit parameters, instead of an array. You only need two values from this array. That's not too many arguments for a method:

public function login($email, $password) {
    ...
}

Some code style improvements:

Put a blank line above the if statements
Put a blank line above the return statements

Whitespace is not just a useful design element for graphic designers. It is useful for code authors as a means for grouping related statements together, or calling out a statement as something more important than the others. A return statement is pretty important in knowing what this method does. Furthermore blank lines above and below code elements give natural "breaks" in the look of the code, which guides your eye and mind to realize they are related — like paragraphs of text.


I strongly recommend using Expression as your basic type, not Box<Expression> as you are. The boxing should be purely an implementation detail of the recursive type, it should not escape beyond that. For that reason, diff will return Expression rather than Box<Expression>; as far as the argument it takes is concerned, there are a couple of approaches that may be used:

You may choose to take an Expression by value; in this case, stop using ref a et al. in the match patterns and just use a, thus improving efficiency ever so slightly in some of those cases, and reducing the number of clones needed by one (a in Pow).
You may choose to take it by reference, &Expression. In this case, you won’t need to clone the entire expression e in main. This will require a mite less cloning than the first choice for your specific case.

There is little practical difference between the two; it is mostly just a matter of shuffling things around. I would probably go with the second, taking &Expression.

For the Pow(ref a, ref b) pattern, taking ref b rather than b, given b is i32, is rather pointless.

That is the basic stuff out of the way, and leaves us with this code (note a couple of minor stylistic things as well, such as e: instead of e :, and having a trailing comma after the last variant in the enum definition, and using visual indent on line 15):

use self::Expression::{Prod, Sum, Pow, Var, Cons};

#[derive(Debug, PartialEq, Clone)]
enum Expression {
    Cons(i32),
    Var(char),
    Prod(Box<Expression>, Box<Expression>),
    Sum(Box<Expression>, Box<Expression>),
    Pow(Box<Expression>, i32),
}

fn diff(e: &Expression) -> Expression {
    match *e {
        Prod(ref a, ref b) => Sum(Box::new(Prod(Box::new(diff(a)), b.clone())),
                                  Box::new(Prod(a.clone(), Box::new(diff(b))))),
        Sum(ref a, ref b) => Sum(Box::new(diff(a)), Box::new(diff(b))),
        // only valid for Var^n
        Pow(ref a, b) => Prod(Box::new(Cons(b)), Box::new(Pow(a.clone(), b - 1))),
        Var(_) => Cons(1),
        Cons(_) => Cons(0),
    }
}

fn main() {
    let e = Prod(Box::new(Cons(5)), Box::new(Pow(Box::new(Var('x')), 2))); // 5*x^2
    let d = diff(&e); // d/dx 5*x^2 = 0*x^2 + 5*2*x^1

    println!("Differentiating {:?} yields {:?}", e, d);
}

Other alterations I would be inclined to make are making diff a method rather than a free function (fn diff(&self) -> Expression). Your naming is also inconsistent: the enum has a full-word name, Expression and not Expr, while the variants have short names, Cons and not Constant and so forth. I would recommend making them consistent.

I also have a personal fondness for struct variants, e.g. Power { base: Box<Expression>, exponent: i32 } instead of Power(Box<Expresion>, i32) (in patterns, Power(ref a, b) would become Power { ref base, exponent } with the values being bound to the better names base and exponent instead of a and b, another change you should consider anyway), but this is subjective.

Now for the big usability wins: implementing operators. Why should Expression not implement std::ops::{Add, Mul} to create Product and Sum? And while there is no power operator, why not add a similar method for it? These things can make it much simpler to look at and read. I’m only going to impl Add<Expression, Output = Expression> for Expression (i.e. Expression + Expression -> Expression) but it would be possible to also implement it for combinations of boxedness on Self and RHS, which could allow a.clone() instead of (**a).clone() in the Product differentiation below, but I do not believe it to be a particularly good idea; it is not any more efficient and clutters things ever so slightly; I think it is best if Box is kept purely an implementation detail.

We might as well also make a simple implementation of std::fmt::Display for bonus marks. std::fmt::Debug output gets a bit tiring to look at occasionally.

Well, here’s what I end up with:

use std::fmt;
use std::ops;
use self::Expression::{Product, Sum, Power, Variable, Constant};

#[derive(Debug, PartialEq, Clone)]
enum Expression {
    Constant(i32),
    Variable(char),
    Product(Box<Expression>, Box<Expression>),
    Sum(Box<Expression>, Box<Expression>),
    Power(Box<Expression>, i32),
}

impl ops::Mul for Expression {
    type Output = Expression;

    fn mul(self, rhs: Expression) -> Expression {
        Expression::Product(Box::new(self), Box::new(rhs))
    }
}

impl ops::Add for Expression {
    type Output = Expression;

    fn add(self, rhs: Expression) -> Expression {
        Expression::Sum(Box::new(self), Box::new(rhs))
    }
}

impl Expression {
    // Yeah, I’m calling this method pow because for a *method* it’s the standard name.
    fn pow(self, power: i32) -> Expression {
        Expression::Power(Box::new(self), power)
    }

    fn diff(&self) -> Expression {
        match *self {
            Product(ref a, ref b) => a.diff() * (**b).clone() + (**a).clone() * b.diff(),
            Sum(ref a, ref b) => a.diff() + b.diff(),
            // only valid for Variable^n
            Power(ref a, b) => Constant(b) * a.clone().pow(b - 1),
            Variable(_) => Constant(1),
            Constant(_) => Constant(0),
        }
    }
}

impl fmt::Display for Expression {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            Product(ref a, ref b) => write!(f, "({} * {})", a, b),
            Sum(ref a, ref b) => write!(f, "({} + {})", a, b),
            Power(ref a, b) => write!(f, "({} ^ {})", a, b),
            Variable(v) => fmt::Display::fmt(&v, f),
            Constant(c) => fmt::Display::fmt(&c, f),
        }
    }
}

fn main() {
    let e = Constant(5) * Variable('x').pow(2);
    let d = e.diff(); // d/dx 5*x^2 = 0*x^2 + 5*2*x^1

    println!("Differentiating {} yields {}", e, d);
}

One final comment—if your power differentiation rule is only applicable to Variable^n (of course, mathematically that’s requiring that you are differentiating with regards to that variable) then it would be good to enforce that and fail rather than produce a bad result. One might even make diff take a char argument as the variable that you are differentiating with regards to, if one wished to make it more correct. All of this sort of thing might lead to you returning a Result from diff so that you can return an error if you wind up with something that you cannot differentiate.


No, using static methods is not a recommand approach. With static methods, your class always knows which class it calls. That means you can never switch to another logger class (e.g. one that writes the logs in a file).

A class should be independent of other classes as much as possible. In this case, you should just create a LoggerInterface (or use the one from PSR-3) and base on that interface. Inject it in the classes that needs a logger and then use that object. This way, you can always change from loggers (as long as they implement the correct interface) and your class doesn't know which classes it uses.

Example:

interface LoggerInterface
{
    public function put($key, $value, $class = null, $function = null);
    public function getMessages();
}

class Logger
{
    protected $messages;

    public function put($key, $value, $class = null, $function = null)
    {
        // ...
    }

    public function getMessages()
    {
        // ...
    }
}

class DataBase
{
    protected $logger;

    public function __construct(LoggerInterface $logger)
    {
        $this->logger = $logger;
    }

    public function someDbFunction(...)
    {
        // ... do something

        $this->logger->put(...); // log something
    }
}

This approach is called Dependency Injection. To make live easier for you, you can sue a Dependency Injection Container (also called a Service Container) to manage which classes depends on which other classes and creating those objects. A simple example is Pimple

There are also more disadvantages of using static methods. You can for instance never have 2 different instances of the logger, as there are no instances. That means that all messages are put in the same instance. You may want to internally use a logger in the Database section of your lib and another logger in the Form section of your lib and then one logger for your complete lib.