Generating random numbers from thread timing in C++

A true random number generator (TRNG) is a source of bits such that each bit has a probability of exactly 1/2 of being 1. Your code is obviously not a TRNG since its bias will heavily depend on the timing of operations.

A pseudo-random number generator (PRNG) is a deterministic algorithm that produces bits which satisfy statistical tests designed to verify randomness. Your code is obviously not deterministic so it is not a PRNG.

What you have here is a source of entropy: a bit stream which is not fully deterministic. You're measuring the timing of your program which depends on the synchronization between wall time and the CPU, so you're using jitter as a source of entropy.

CPU jitter may be a potential source of entropy. This paper by Stephan Müller analyzes CPU jitter on an x86 CPU. It has acceptable statistical properties if done right. However, there is a far cry from having acceptable statistical properties to being a good entropy source. The Linux kernel maintainers debated whether to include Müller's code as an entropy source, and concluded against it. They found two problems:

Having good statistical properties is good enough for some applications, but it is not good enough for security. For security, unpredictability is necessary. Even a knowledgeable adversary must not be able to reproduce the output. That includes, for example, an adversary who has the exact same hardware running the exact same program.
The CPU, in fine, does not contain much if any actual physical source of entropy. At the physical level, it's pretty much a deterministic system, albeit an extremely complex one which is difficult to analyze fully. It's unclear whether the physical uncertainties are enough to reach macroscopic levels.

To make things worse (much worse!) your implementation of CPU jitter measurement is deeply flawed. CPU jitter randomness is based on the lack of synchronization between the real-time clock (RTC) and the CPU's clock. But your code doesn't go very far in this direction: it spends most of its time doing sleep_for()! sleep_for() is based on interrupts driven by the RTC, and so are context switches. Confronting the RTC with itself is fully predictable, so the only source of unpredictability is the timing of the few computations that you make.

I haven't made any mathematical analysis of your computations (what you do with out). The only reason I can see to use multiplication here is to exercise different parts of the CPU to increase jitter. You should document this kind of design choices.

An obvious potential problem with your function is that it's prone to short cycles. The transformation (out, tmp) → (out*Base+tmp, tmp*Base+num) has cycles; if you fall into them you'll emit repetitions for a while until the scheduler hits. Just for fun, I ran your code and inspected the output visually, which is degree 0 of statistical testing (serious statistical testing uses tools that run for hours). The single-CPU version emitted sequences of zeroes () now and then… that's an obvious failure.

0000c9f0  5c 4d 68 41 e1 95 77 a6  17 2e bb 7a c6 37 d6 27
0000ca00  54 a4 7f f0 5f ee 5b fc  54 a4 7f f0 5f ee 5b fc
*
0000cbb0  a1 48 55 36 71 47 4b 75  4f a1 8d c1 60 fc c8 f0
0000cbc0  52 f5 22 43 e1 1a c0 4d  52 f5 22 43 e1 1a c0 4d
0000cbd0  52 f5 22 43 e1 1a c0 4d  74 47 ad 9f bd ee 5f 90
0000cbe0  74 47 ad 9f bd ee 5f 90  74 47 ad 9f bd ee 5f 90
*
0000cc40  74 47 ad 9f bd ee 5f 90  e3 2e c7 50 a8 84 5a 1c
0000cc50  fb ea 92 19 2f 6c 98 8e  0b 6a 7a 9a ac 59 87 03
0000cc60  73 ac 9f 43 2f 52 ab e1  2b d8 86 ab 7b 97 98 fc

(The first column is the offsets, then , all in hex. A * stands for a sequence of all-zero lines.)

On the programming side, you should initialize all variables. Leaving variables uninitialized doesn't increase entropy (they're predictable on a given platform), makes your program harder to analyze, and allows the compiler to optimize parts of your code away (for example, since you can't distinguish between a final value of out that was produced from a particular starting value, and a final value that was produced from a different starting value, the compiler is allowed to perform no computation until the first time the value of out is written to). Also note that operations like out=out*Base+tmp are not performed atomically, there can be a context switch between the time out is read and the time out is written to, in which case the work of the other thread(s) is be overwritten.

Note: sometimes “TRNG” is used to mean a source of entropy, which I discuss below. An actual RNG can be built on top of a source of entropy (preferably multiple independent sources) by seeding a PRNG with the source of entropy. By this very weak definition, you have a TRNG, but as we've seen one that's neither good nor new.

All in all, you seem to be trying to generate random numbers by flailing around in the dark. That doesn't work. Generating random numbers is very difficult. In order to be taken seriously, a random number generator needs to come with a detailed analysis, including rationales for each sources, collected statistical data, mathematical models, etc. Not a code dump. Showing that you did no research by demonstrating ignorance of common terminology doesn't help your credibility either.


Encrypting and decrypting passwords in PHP

The name of the function crypt() is somewhat misleading, but you're not encrypting and decrypting. You create a hash and you check it, that's it.

It's not the best I have seen, but it is not uncommon to do it this way.

In the PHP crypt() documentation it says: Use of password_hash() is encouraged. You didn't see that? It allows you to choose an algorihtm. The PASSWORD_BCRYPT algorithm should be used. See: http://codahale.com/how-to-safely-store-a-password

I also see that you're not really using a salt in the traditional manner. Here's a nice introduction: http://www.martinstoeckli.ch/hash/en/index.php

So you're not using a salt on a per-password basis. See: http://blog.codinghorror.com/youre-probably-storing-passwords-incorrectly

There's much to say about this topic. Password security is a hot topic.

One of the mistakes most programmers make is that they rely on the user to select a password. In general users are just plain bad at choosing a secure password. You can try and force them to choose one, which is probably secure, but by doing that you will only annoy the hell out of the user. The solution is quite simple: You choose the password for the user. For a site where security is not that paramount you can use simple password. A random five digit number, in combination with a email address and brute force protection, is already reasonable secure; there are a million different possible combinations. Most developers however stick to what they know.


Reading source code and extracting json from a url

The Security
The main point here is your assumption that HTTP_REFERER could prevent someone from using this code. Unfortunately it doesn't. Referrer, just like any other HTTP header, is easily faked, one is setting it routinely with any software that is doing HTTP requests. So be advised that it is an ostrich-style defense.
You should understand that there is absolutely no way to protect the information which is shown in the browser. The only way to restrict an access to a script is to make it password protected.
Moreover, I suppose that a concerned user could easily guess the domain from which you are requesting the data, and then just bluntly take the token from the source of your page and then just use it at their own disposal. Consider storing the token inside your PHP code instead.
The code.
That said, your code could be syntactically improved as well. First of all, just like it said in the other answer, you could combine all conditions in one. But it mustn't be done at the expense of readability. So first define your conditions and then check them at once.
After that you could just write your code without any conditions. A loop is unnecessary if there is only one value in the array
$referrer = $_SERVER['HTTP_REFERER'] ?? '';
$access = preg_match('!^http://localhost/test!', $referrer);
$token = $_GET['token'] ?? '';
$source = $_GET['source'] ?? '';
$source_ok = $source == 'player';

if (!$referrer || !$access || !$token || !$source || !$source_ok) {
    header('HTTP/1.1 404 Not Found');
    die;
}
$urlPost = 'https://www.player.com/?token='.$token;
$url = file_get_contents($urlPost);
preg_match('/CONFIG = (.*)/', $url, $matches);
$jsn =  json_decode($matches[1]);
$vdUrl = $jsn->streams[0]->url;
header('location:'.$vdUrl);

Note: if $_GET['source'] ?? ''; operator is giving you an error, consider upgrading your PHP version immediately, because it is not supported anymore.


An if clause of Output is only executed once, on the very first node. You may avoid this logic altogether by making it explicit:

if (!head)
    return;
printf("%d", head->id);

for (head = head->next; head; head = head->next) {
    printf(" -> %d", head->id);
}

A missing

printf("\n");

seems like a bug (I hope it is a copy-paste error).
An early return helps to avoid unnecessary variables. Consider Search:

for (int i = 0; i < MAXPRI; i++)
    for (Node * aux = queue[i]; aux; aux = aux->next) {
        if (aux->id == id) {
            return i;
        }
    }
}
return -1;

A perk benefit is that now you can rename meaningless i to a more reasonable pri.

Similarly, if Execute returns as soon as a process is found, you wouldn't need bool end.
I strongly recommend to factor the

    while (aux->next != NULL) {
        aux = aux->next;
    }

loop into a function (say, get_tail), and streamline Push:

Node * node = new_node(id);
Node * tail = get_tail(pri);
if (tail) {
    tail->next = node;
} else {
    queue[pri] = node;
}

Using extra memory may significantly reduce the run time. In your case, the

struct list {
    Node * head;
    Node * tail;
};

would reduce looking for tail complexity from linear to \$O(1)\$.
linkedListArray.h shall only export the names defined in linkedListArray.c. The #include files required for linkedListArray.c compilation should be included directly in linkedListArray.c.


I will answer an isomorphous question of finding a minimal value in a binary tree. In your case, the tree is octal, but the same rules apply.

Consider that you have

typedef struct node {
     struct node *pLeft, *pRight;
     int value;
} node_t;

The tree is completely unsorted, and you need to find the minimal value. What you are doing is something like this:

int minval(node_t *pRoot)
{
    if (pRoot == NULL)
        return INT_MAX;

    int min = pRoot->value;

    min = minimum(min, minval(pRoot->pLeft));
    min = minimum(min, minval(pRoot->pRight));

    return min;
}

This is a recursive algorithm. It's elegant, nice, cool and bloody slow.
Recursion is slow because every time you call a function it has to do all sorts of staging (pass arguments, call function, allocate stack space), and then repeat all that on return (clean up stack, return from call, clean up arguments).

There are non recursive algorithms to most problems, but there's also a nice way to get rid of all the costly staging for calling a function. (inlining a function does that too, but it is not possible for recursive functions). Let us take a look at code that does exactly the same thing as above, but without recursion:

int minval(node_t *pRoot)
{
    if (pRoot == NULL)
        return INT_MAX;

    node_t *stack[100]; // At least max tree depth*2, in your case n*8
    int sp = 0;
    int min = INT_MAX;

    stack[sp++] = pRoot;
    while (sp > 0)
    {
        node_t *p = stack[--sp];
        min = minimum(min, p->value);

        if (p->pLeft  != NULL) stack[sp++] = p->pLeft;
        if (p->pRight != NULL) stack[sp++] = p->pRight;
    }

    return min;
}

Here we have a simulated program stack, but we don't do so much with it, we only add one pointer for every tree node, which in your case is 8^5, or 32768. Compared to that many function calls.

If you modify your program to not actually call the function recursively, you should gain a very significant speed boost. Using the template I provided, it shouldn't be a problem, you'll just have 8 things to put on the stack every iteration.

Fun fact: What you are doing is known as a preorder tree traversal, also known as a depth-first search. If you were to do a level order traversal (also known as a breadth-first search), the code would go faster. The difference is that DFS will look at all paths, while you can stop BFS early as soon as it finds one.


Project Euler problem 1 in Python - Multiples of 3 and 5

The sum 3+6+9+12+...+999 = 3(1+2+3+...+333) = 3 (n(n+1))/2 for n = 333. And 333 = 1000/3, where "/" is integral arithmetic.

Also, note that multiples of 15 are counted twice.

So

def sum_factors_of_n_below_k(k, n):
    m = (k-1) // n
    return n * m * (m+1) // 2

def solution_01():
    return (sum_factors_of_n_below_k(1000, 3) +
            sum_factors_of_n_below_k(1000, 5) -
            sum_factors_of_n_below_k(1000, 15))


Per @Dangph's comment on the question, it seems that whether to escape ' or " depends on the context; i.e. if within quotes of the same type these characters need escaping; if not however, they must not be escaped.

The below version only escapes apostrophes (' / single quotes) and backslashes (\); not quotation marks (" / double quotes).

It assumes that the query will only use single quotes for surrounding parameter names.

It has a bit of extra logic to capture the more common cases where double quotes are used (i.e. where it's simply quotes around the placeholder, or quotes and wildcards (for "starts with" / "ends with" / "contains") matching around the placeholders.  I don't really like this as it's inconsistent / may lead to unpredictable behaviour, so will likely not keep that line / rather will state that only single quotes should be used... but included here to illustrate one possible option.

function ConvertTo-WmiEscapedQuery {
    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string]$Query
        ,
        [Parameter()]
        [string[]]$Parameters = @()
    )
    begin {
        [string]$EscapeCharatersRegex = '([\\''])' #Doesn't escape " as assumes strings are enclosed using apostrophes; i.e. "... where field = '{0}'"
    }
    process {
        [string[]]$EscapedParameters = $Parameters | %{$_ -replace $EscapeCharatersRegex, '\$1'}
        [string]$escapedQuery = $Query -replace '(")(%?{\d+}%?)\1', '''$2''' #enforce the above assumption by replacing double quotes around parameterised input with apostrophes.  Not guaranteed to work, but better than nothing.  Remove this line & replace the below with `[string]$escapedQuery = $Query -f $EscapedParameters` if you don't like this behaviour.
        $escapedQuery = $escapedQuery -f $EscapedParameters
        Write-Verbose "Escaped Query: $escapedQuery"
        $escapedQuery
    }
}

function Get-WmiService {
    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string]$ServiceName
    )
    begin {
        [string]$Query = 'select * from win32_service where name = ''{0}'''
    }
    process {
        Get-WmiObject -Query (ConvertTo-WmiEscapedQuery -Query $Query -Parameters $ServiceName)
    }
}

Get-WmiService '"John''s Example Service\String"' -Verbose
Get-WmiObject -Query (ConvertTo-WmiEscapedQuery -Query 'select * from win32_service where name = ''{0}'' or displayname like "%{1}%"' -Parameters 'bits', 'transfer' -Verbose) | select name, displayName #just to prove it works
Get-WmiObject -Query (ConvertTo-WmiEscapedQuery -Query 'select * from win32_service where name = ''{0}'' or displayname like "%{1}%"' -Parameters '''bits"', '"transfer''' -Verbose) | select name, displayName #to test escaping logic


Traversing a JSON-like data structure in Go

Readability is essential for correct and maintainable programs.

For example,

traverse.go:

package main

import (
    "fmt"
    "strconv"
)

func main() {
    test := map[string]interface{}{
        "root": map[string]interface{}{
            "inner": []interface{}{
                map[string]string{"name": "obj1"},
                map[string]string{"name": "obj2"},
            },
        },
    }
    fmt.Println(
        TraverseToStr(test, []string{"root", "inner", "0", "name"}),
    )
}

func TraverseToUint64(data interface{}, path []string) (uint64, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return 0, err
    }
    return val.(uint64), nil
}

func TraverseToStr(data interface{}, path []string) (string, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return "", err
    }
    return val.(string), nil
}

func TraverseToMap(data interface{}, path []string) (map[string]interface{}, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return nil, err
    }
    return val.(map[string]interface{}), nil
}

func TraverseToArray(data interface{}, path []string) ([]interface{}, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return nil, err
    }
    return val.([]interface{}), nil
}

func Traverse(data interface{}, path []string) (interface{}, error) {
    var ok bool
    current := data
    for _, p := range path {
        switch current.(type) {
        case map[string]string:
            if current, ok = current.(map[string]string)[p]; !ok {
                return nil, fmt.Errorf("key not found in map: %s", p)
            }
        case map[string]interface{}:
            if current, ok = current.(map[string]interface{})[p]; !ok {
                return nil, fmt.Errorf("key not found in map: %s", p)
            }
        case []interface{}:
            i, err := strconv.ParseInt(p, 10, 64)
            if err != nil {
                return nil, fmt.Errorf("integer required, got: %s", p)
            }
            array := current.([]interface{})
            if i < 0 || i >= int64(len(array)) {
                return nil, fmt.Errorf("index %d out of bounds for %v", i, array)
            }
            current = array[i]
        default:
            return nil, fmt.Errorf("cannot traverse %T\n", current)
        }
    }
    return current, nil
}

Playground: https://play.golang.org/p/fMcDZr1AZy_X

Commentary:

Readability is essential for correct and maintainable programs.

fmt.Println(
    TraverseToStr(test, []string{"root", "inner", "0", "name"}),
)

The primary line of code should be obvious. The first read through, we focus on the primary line, returning later to read error handling and other secondary details.

TraverseToStr is the primary code, fmt.Println is a secondary detail. We highlight the primary code on a separate line.

func TraverseToUint64(data interface{}, path []string) (uint64, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return 0, err
    }
    return val.(uint64), nil
}

In Go, don't ignore errors.

Unnecessary conditional indentation is hard to read. An idiosyncratic coding style is extrememly hard to read. Return immediately on error. CodeReviewComments: Indent Error Flow

func TraverseToMap(data interface{}, path []string) (map[string]interface{}, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return map[string]interface{}{}, err
    }
    return val.(map[string]interface{}), nil
}

Replace make(map[string]interface{}), a built-in function call, with map[string]interface{}{}, a composite literal value.

func TraverseToMap(data interface{}, path []string) (map[string]interface{}, error) {
    val, err := Traverse(data, path)
    if err != nil {
        return nil, err
    }
    return val.(map[string]interface{}), nil
}

However, on error, return the zero value, nil for a map.

i, err := strconv.ParseInt(p, 10, 64)
if err != nil {
    return nil, fmt.Errorf("integer required, got: %s", p)
}
array := current.([]interface{})
if i < 0 || i >= int64(len(array)) {
    return nil, fmt.Errorf("index %d out of bounds for %v", i, array)
}
current = array[i]

Replace a stream of code with the basic Go error pattern.

Don't overuse the "if expression; simple-statement statement" form.

if i < 0 || i >= int64(len(array)) {
    return nil, fmt.Errorf("index %d out of bounds for %v", i, array)
}

When you see this error message,

invalid operation: i < len(array) (mismatched types int64 and int)

and this fix,

if int(i) < len(array) {
}

stop and think. i may be negative. i is int64, int(i) will discard high-order bits when int is implemented as 32-bits.

The Go Programming Language Specification

The Go Programming Language

"I though that" is not sufficient justification. Use authoritative sources.

Go is strongly typed. Types interface{}, map[string]string, and map[string]interface{} are distinct and have different memory layouts.

When the code is readable, it's possible to read much of the code and prove it correct.

Sometimes it can be hard to establish invariants for maps. "The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next." The Go Programming Language Specification. Also, since the Go 1.12 release, "fmt: Maps are now printed in key-sorted order to ease testing." Which leads some to believe, incorrectly, that maps are sorted.

For this code, it's important to note that maps are not read iteratively in a for loop in random order. All map accesses are deterministic.


Good

Meaningful names are used for classes, methods, parameter and properties
using interfaces over implementation
using enums to signal a state

Not so good

Code isn't tested
Scope of property setters is public where they should be private
Using of properties which are never used
Single responsibility principle is violated

The good things are said, so let us focus on the bad things.

Code isn't tested
How / where should one see this ?

void StartQuizz()
{
    QuizzState = QuizzState.Going;

    while(QuizzState == QuizzState.Going)
    {
        MoveToNextQuestion();
        if(QuizzState == QuizzState.Going) return;
        Answer answer = GetAnswer();
        TryAcceptAnswer(answer);
    }
}

Here, after calling MoveToNextQuestion(), the code is checking the QuizzState property. So we assume the property has changed and if we take a look at this said method we see,

public void MoveToNextQuestion()
{
    if(questions.Count == 0)
    {
        QuizzState = QuizzState.Finished;
        CurrentQuestion = null;
        return;
    }

    CurrentQuestion = questions.Dequeue();
}

that the property is changed to QuizzState.Finished, if the question queue is empty. But the check is about if(QuizzState == QuizzState.Going) return;. So after the first call to MoveToNextQuestion() the code exists the while loop.

Scope of property setters and never used properties

As one of many samples let us take a look at the Answer class, so we can kill 2 birds with one stone

public class Answer
{
    public bool IsCorrect { get; set; }
    public string Content { get; set; }
}

The IsCorrect property is nowhere used in the whole code. But if it would be used somewhere, why should we allow to set / change the value of this property from a public place ? Especially for a Q&A engine, there shouldn't be a possibility to cheat !

Single responsibility principle

Where is the SRP viaolated ? Here

void GetAnswerFromUI(out Answer answer)
{
    // somehow gets Answer through the UI
    answer = null;
}

The QuizEngine shouldn't be responsible for any UI related stuff. But wait, how are we supposed to receive the answers and also how should we notify another object ( like an UI ) about the next question ?

That is what events and methods or properties are for.
To signal a changed state, like there is a question to be answered, an event should be used by the QuizEngine.
To receive an answer for a question, the QuizEngine should provide either a property setter or a method.
For decoupling the QuizEngine and the UI the QuizEngine should implement an interface which provides the event and the answer setting method/property.


This is a bit strange, and more complicated than it needs to be:

if 'addedBook' in locals():
    context['added'] = addedBook
else:
    context['added'] = None

You could initialize addedBook = None, and simplify the above, and without using locals():

addedBook = None
# validation, possibly setting: addedBook = form.save()
context['added'] = addedBook

Needless to say, this "docstring" is useless:

"""docstring for AddBookForm"""

Error handling

Your current error handling is merely checking if the form is valid or not. And the form class is very basic, so only the standard validations will be applied, according to the underlying model, for example, non-integer values for an integer field will raise a validation error.

At the minimum, it would be good to tell the user about validation errors, if any. In the current code, if the form was not valid, it simply prints the same form again without giving the user a clue about what went wrong, which is not very user-friendly.

This page in the docs touches on the basics of form validation and includes examples of showing form validation errors:

https://docs.djangoproject.com/en/1.7/topics/forms/

This page is more detailed, and specific about Model forms:

https://docs.djangoproject.com/en/1.7/topics/forms/modelforms/

I recommend you re-read them and:

Add printing of form errors to the HTML output
Add custom validation methods in the form class, if it makes sense for your use case


Project Euler #2 - Even Fibonacci numbers

Going from top to bottom:

MAX_LIMIT = 4000000 # Four million inclusive

That sounds like a function parameter, not a global constant:

def sum_fib(max_limit=4000000):
    ...

Now, you have this function:

def fib(n):
    ...

But you don't really need it. You never need the nth Fibonacci number here, out of the blue. You're always going in order. If you're going in order, you can just keep them as local variables and update both:

cur, prev = cur+prev, cur

And then:

while (1):

Typically we'd say while True:, but in this case, we have a clearly established termination condition: our max limit. So let's use it:

def sum_fib(max_limit=4000000):
    cur, prev = 1, 0
    total = 0
    while cur < max_limit:
        if cur % 2 == 0:
            total += cur
        cur, prev = cur+prev, cur

    return total

We could even generalize this more by providing a predicate:

def sum_fib(max_limit=4000000, pred = lambda x: x%2 == 0):
    cur, prev = 1, 0
    total = 0
    while cur < max_limit:
        if pred(cur):
            total += cur
        cur, prev = cur+prev, cur

    return total

Now back to optimizations. What are the Fibonacci numbers? They are:

1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 ...

What are the even Fibonacci numbers? They are:

2, 8, 34, 144, 610

Notice that it's every 3rd Fibonacci number. What can we do with that? That's still a Fibonacci-like sequence. Except instead of an = an-1 + an-2, we have an = 4an-1 + an-2:

def sum_fib(max_limit=4000000):
    cur, prev = 2, 0
    total = 0
    while cur < max_limit:
        total += cur
        cur, prev = 4*cur+prev, cur

    return total


Flattening XML and selecting nodes based on input to convert to CSV

Docstrings

You should include a docstring at the beginning of every module, class, and function you write. This will allow documentation to identify what your code is supposed to do. Docstrings are great for the understanding the functionality of the larger part of the code, i.e., the general purpose of any class, module or function. Comments are used for code, statement, and expressions which tend to be small.

Function Naming

You have these two function names:

fetchNodesListBasedOnXpath
getNodesDict

According to PEP-8's Guidelines for function names, these function names  should be snake_case.

_ for unused loop variables

When you don't use a variable in a loop, like:

for i in range(5):
    print("Hi!")

You should use a _. This makes it clear that the variable used for the loop is not needed, and should be ignored.

Use of if len(...)

  For sequences, (strings, lists, tuples), use the fact that empty sequences are false.

Yes: if not seq:
     if seq:

No:  if len(seq):
     if not len(seq):

Essentially, if seq will return True if it's not empty, and if not seq will return True if it is empty. Since you are checking if len(temp_list) > 0 ..., this should be changed to if temp_list ....

Constant Variable Naming

According to PEP8s rule on constants:

  Constants are usually defined on a module level and written in all capital letters with underscores separating words. Examples include MAX_OVERFLOW and TOTAL.

Long Version:

In the Python community (as in many other communities) exist conventions about how to write code. This is different from working code: even if you write your constants all lowercase, your code still works.

But there is community consensus (as documented in PEP8) which is "enforced" with tools like pylint. If you program for your own happiness, you may neglect the hints pylint gives you. If you want open exchange with the community, aka »someone besides myself should use my code«, you should prepare your code according to PEP8. [source

Main Guard

Having a main guard clause in a module allows you to both run code in the module directly and also use procedures and classes in the module from other modules. Without the main guard clause, the code to start your script would get run when the module is imported. This can cause a plethora of issues if you want to reuse this code. Wrapping it in a main guard (check updated code) protects against this.

Parameter Naming

Parameters, variables, and default parameters all follow the same PEP-8 Guidelines for naming. They should all be snake_case. You currently have a parameter named Xpath. A simple change to xpath will do the trick.

Catching Errors/Exceptions

Right now, your main function can catch an exception, but doesn't handle it. If your program suddenly fails, a detailed error message would do the trick. You should catch specific errors that you think your code will generate. Since you're dealing with files and paths, catching FileNotFoundError is a good idea. Then, after dealing with all the specific errors, you can use a finally. Since this will always be run, you can put your pass here.

Updated Code

"""
Module Docstring
A description of your program goes here
"""

from lxml import etree

def get_nodes_dict(node_element, nodes_to_pick):
    """
    A description of this method should go here
    """
    some_dict = {}
    for element in node_element:
        if not isinstance(element, list):
            if element.tag in nodes_to_pick:
                some_dict[element.tag] = element.text

    return some_dict

def fetch_nodes_list_based_on_xpath(xpath, tree, selected_nodes):
    """

    A description of this method should go here

    :param xpath:
    :param tree:
    :param selected_nodes:
    :return:
    """
    temp_list = []
    temp_list1 = []
    element_split = xpath.split('/')
    for i in range(len(element_split), 1, -1):
        path = "/".join(element_split[0:i])
        try:
            element = tree.xpath(path)
            for j, node_element in enumerate(element):
                value = get_nodes_dict(node_element, selected_nodes)
                if temp_list and len(temp_list) - 1 > j:
                    for _, item in enumerate(temp_list):
                        dall = {}
                        dall.update(value)
                        dall.update(item)
                        if item:
                            temp_list1.append(dall)
                    temp_list = []
                else:
                    temp_list.append(value)
        except FileNotFoundError as _:
            print(f"The file at [{PATH}] doesn't exist!")
        finally:
            pass

if __name__ == '__main__':

    TREE = etree.parse("code/sample.xml")
    PATH = "/DATA_DS/G_1"

    NODES_TO_PICK = ['SUBSCRIPTION_ID', "PRODUCT_NUMBER", "SUBSCRIPTION_ID1", "SALES_PRODUCT_TYPE1"]

    fetch_nodes_list_based_on_xpath(PATH, TREE, NODES_TO_PICK)


Is this validation sufficient to ensure that the vals are not empty?

Don't use String.IsNullOrEmpty to validate required values.
You can create a method

private bool IsEmpty(string value, string errorMessage, Control controlToValidate)
{
    if ((value ?? string.Empty).Trim().Length == 0)
    {
        MessageBox.Show(errorMessage);
        controlToValidate.Focus();
        return true;
    }
    else
    {
        return false;
    }
}

instead of TestVal.
Consider using Validating event for validating your controls and the Tag  property to associate error messages with the controls.
Consider using ErrorProviders.


Get Rectangle Count from Given List of Co-ordinates

On the whole it's not at all bad, I find it readable and I'm sure that it does the job (one caveat to this is how does it handle cases where you have multiple instances of the same point e.g. {{1,1},{1,1},{1,2},{2,1}}), a few things that you could do to improve it..

For uniqueness and sorting consider using a TreeSet, this will have an impact on the rest of your algorithm though as you can no longer rely on iterating using an index (see below).
As @DaveJarvis says, it is preferable to have the square brackets immediately following the type - as they form a part of the type (2D array of ints vs int).
final int MIN_POINTS_TO_CREATE_RECTANGLE = 4; If this is the only piece of code to care that a rectangle has four corners it is fine to define it in method scope. If other methods might care, or if there is any chance it might need to be publicly visible then move it up to class level.
My personal preference is to always use braces even for one line conditionals. It makes your code slightly longer, but I find it more readable and it reduces the chances of bugs down the line (when someone wants to make it a two+ line conditional). This may relate to the question around Point uniqueness as you may consider adding a test for uniqueness (or using a Set) which would then make your conditional code longer.
Small things like breaking both the creation and sorting of the Point[] into new methods. It is almost trivial in this case but it will make your code easier to read, more testable and it lends itself to extensibility down the line (if for example you wanted to have different creation strategy down the line).
leftDown, leftUp and rightDown do not add anything to your algorithm, instead consider naming i,j, and k more sensibly.
If leftDown.getX() < leftUp.getX() you know you have reached a point where no other matches can ever be found. To fix this you can use a while loop instead

e.g

int j = i + 1;
//don't forget to handle index out of bounds
while (points[i].getX() == points[j++].getX()) { }

Similarly you can be sure that if k == points.length - 1 then you are not going to find a match.
Your k iteration is pointless if leftDown.getY() != rightDown.getY() so that test should move up and delay your creation of probableRightUp.
Your compareTo method can be made a little neater, either write the result of the initial compare to a variable and return it immediately if non-zero, or consider doing the comparison yourself

e.g

@Override
public int compareTo(Point p) {
    if (this.getX() < p.getX()) {
        return -1;
    }
    if (this.getX() > p.getX()) {
        return 1;
    }
    //and for Y
    ...
    return 0;
}

-. When you are coding your existsIn method start thinking in templates. Sure it's not necessary now, but it costs nothing to write public static <T> boolean existsIn(T[] array, T element) and you've got a utility function you can use forever.

One thing that would concern me about this method is the Cyclomatic Complexity due to the number of loops and conditionals. Consider how you could break things down, e.g The content of your k loop can be broken out into code which finds/verifies the rectangle is complete. As a rule each method should have a role as discrete as possible, again this makes things more testable and more readable (when coupled with sensible naming).

So using a TreeSet. Your initial code becomes:

final SortedSet<Point> points = new TreeSet<Point>(/* comparator goes here */);

for(int i = 0; i < coOrdinates.length; i++) {
    Point p = new Point(coOrdinates[i][0], coOrdinates[i][1]);
    points.add(p);
}

Your iterations would need to evolve to use the SortedSet#tailSet or SortedSet#subSet methods. Both of these are just new views on the same data so they are cheap operations.

A pseudo approach:

for (Point leftDown : points) {
    //You would pull this into a new method
    SortedSet<Point> potentialLeftUp =  points.subSet(leftDown, new Point(leftDown.getX(), somePrecalculatedMaxY);

    for (Point leftUp : potentialLeftUp) {
        //no testing needed as you know it shares X

        //now build subset (potentialRightDown) where X > leftDown.getX and Y == leftDown.getY (this will be more manual as you are X sorted)

        for (Point rightDown : potentialRightDown) {
            //build probableRightUp
            if (points.contains(probableRightUp) {
                 //increment count
            }
        }
    }
}


Saving temporary file for plotting audio waveform

Instead of saving file you can create io.BytesIO object and use it instead of file.

Also

    def allowed_file(filename):
      return (
          '.' in filename
          and os.splitext(filename)[1].lower() in ALLOWED_EXTENSIONS
      )

Also maybe you should check content-type header and not only extension.


Overflow bug

If you use the input:

1
1/100000 2/100000

You get the result:

Case 1: 0/14100

The problem is with this line:

    den3 = (den1*den2)/den3;

Here, all three variables are 100000.  But den1*den2 overflows an integer.  It would be better to write this as:

    den3 = (den1/den3)*den2;

This expression will work as long as the common denominator fits in an int.
 However, it will still overflow when the common denominator is larger than what fits in an int.  For this reason, you might want to use long long variables to avoid the problem.

Simplify the result?

If you use the input:

1
1/4 1/4

the answer the program gives is:

Case 1: 2/4

This is correct, but the answer could also be simplified to 1/2.  This is purely a cosmetic suggestion, since the specification didn't say the answer had to be reduced to its simplest form.


Faster way to parse file to array, compare to array in second file, write final file

Disclaimer : I have very limited skill/knowledge for anything related to np. Also, I haven't run your code to identify bottleneck which should be the very first thing to do before trying to perform optimisations.

Common ways to make things faster are :

not to do something if you don't need to.
not to do something multiple times if once is enough.

In the first category, your import subprocess as sp and from pyteomics import auxiliary are not required : you can get rid of this. This shouldn't change anything from a performance point of view but it's always good to make things easier.

In the second category, it seems like you could open filename once for each ann_spectra.
Also, for each entry, you could retrieve entry['params'] only once. Similarly, you could call mgf_out once.

This is the code I have at this stage :

def main():
    pep_files = []

    if (len(sys.argv) > 0):
        spec_in = sys.argv[1]
    else:
        print 'Not enough Command Line Arguments!'

    path = '/DeNovo/QE_2706_229_sequest_high_conf.mgf'
    print spec_in
    pep_files.append(spec_in)

    for ann_spectra in pep_files:
        seq = ann_spectra[:ann_spectra.find('-') - 1]
        print seq
        a = np.genfromtxt(ann_spectra, dtype=float, invalid_raise=False, usemask=False, filling_values=0.0, usecols=(0))
        b = np.delete(a, 0)

        entries = []
        file_name = 'DeNovo/good_training_seq/{}.mgf'.format(ann_spectra[:-4])
        with open(file_name, 'wb') as mgf_out, mgf.read(path) as reader:
            for spectrum in reader:
                if np.array_equal(b, spectrum['m/z array']):
                    entries.append(spectrum)
                    for entry in entries:
                        param = entry['params']
                        mgf_out.write('BEGIN IONS\nSEQ={}\nTITLE={}\nPEPMASS={} {}\nCHARGE={}\nRTINSECONDS={}\nSCANS={}\n').format(
                            seq,
                            param['title'],
                            param['pepmass'][0],
                            param['pepmass'][1],
                            param['charge'],
                            str(param['rtinseconds']),
                            param['scans'])
                        p = np.vstack([entry['m/z array'], entry['intensity array']])
                        output = p.T
                        np.savetxt(mgf_out, output, delimiter=' ', fmt='%f')
                        mgf_out.write('END IONS')

Now, I have realised something that I find quite confusing : you keep adding elements to entries and then you loop over it : it will loop once the first time, twice the second time, n times the n_th times. Is this something we really need/want to do ?

Finally, you are doing things in an un-usual way : you have pep_files = [], then pep_files.append(spec_in). This could easily be written pep_files = [spec_in]. Then, you do for ann_spectra in pep_files:, you might as well get rid of pep_files, ann_spectra and the loop and work directly with spec_in (and maybe you should exit when the argument is not provided).


Better writing JS - from general variables to object

You can enclose your variables (and functions) in a closure to restrict your scope, although you then need someway to make those functions callable outside of that closure, e.g.:

(function() {
    var a;

    function increment() {
        a += 1;
    }

    function decrement() {
        a -= 1;
    }

    window.increment = increment;
    window.decrement = decrement;
})();

It's pretty ugly, though.  It would be better to enclose the whole thing as an object, e.g.:

function Counter() {
    var count = 0;

    return {
        increment: function() {
            return count++;
        },
        decrement: function() {
            return count--;
        },
        value: function() {
            return count;
        }
    };
};


Best way to check for one of two values in an array in PHP

If you want to verify that either value is in the $array_under_test, array_intersect may not be the best option. It will continue to test for collisions even after it finds a match.

For two search strings you can just do:

if (in_array('a', $array_under_test) || in_array('b', $array_under_test)) {
  // Success!
}

This will stop searching if 'a' is found in the $array_under_test.

For more than two values, you can use a loop:

foreach ($match_array as $value) {
  if (in_array($value, $array_under_test)) {
    // Success!
    break;
  }
}


My code for a LocalBinaryPatern in python is running very slow. Any tips?

No else after return
You have code like
if x:
    return 0
else:
    ...

Just drop the else, because you previously already returned. Use else branches only if the code converges again after the two branches, not if you exit the function (raise, return or exit()).
Double Initialization
You have code creating two lists:
        xshift, yshift = [0 for _ in range(8)], [0 for _ in range(8)]

        for i in range(8):
            xshift[i] = - radius * sin(i*abase)
            yshift[i] = radius * cos(i*abase)

You can combine those:
xshift = [-radius * sin(i * abase) for i in range(8)]
xshift = [ radius * cos(i * abase) for i in range(8)]

Further Dead Code

Initialization of h, w at the beginning can be moved down, after checking the radius.
lbp = [0 for _ in range(8)] -- you're doing that once before the loop and then in every iteration.

C-isms in Loop Iteration
It looks like you're used to writing in C (or a similar language) where you can only access an array by index. It shows e.g. in this code:
sum_lbp = 0
for i in range(len(feat)):
    sum_lbp += feat[i]

In Python, you would write this loop like this instead:
sum_lbp = 0
for value in feat:
    sum_lbp += value

An additional minor improvement specific to this case is to write it in one line:
sum_lbp = sum(feat)

Get familiar with the tools Python gives you at hand, they can make your life easier. In this case it's just the sum() function, but also you're not making use of e.g. dictionaries, which could make this code cleaner.
Redundant Parentheses
Here are two redundant pairs of partheses:
for x in (range(radius, (w-radius))):

Same code without them:
for x in range(radius, w - radius):

Redundant Type Conversions
Looking at int(lbp[i]*base[i]), I wonder why you int() the product. Both only contain integers, as far as I can tell.
Off-by-one Error
This code looks suspicious:
feat = [0 for _ in range(255)]
...
bits = 0
for i in range(8):
    bits += int(lbp[i]*base[i])

if bits < 255:
    feat[bits] += 1

Since an octet can take 256 different values, I'd expect feat to have 256 values as well. That way you could eliminate the bits < 255 check. Another way to simplify this is to reuse the sum() function again. Also, base[i] is 1 << i and lbp[i] is either zero or one, so you can also write this lbp[i] << i eliminating base entirely.
Further Redundant Code
Look at the inner loop:
# init `lbp` with zeroes
lbp = [0 for _ in range(8)]
# actually fill `lbp`
for i in range(8):
    ...
    if image[ry][rx] > 0:
        lbp[i] = 1
# compute values from `lbp`
bits = 0
for i in range(8):
    bits += int(lbp[i]*base[i])

This uses too many temporaries. Instead, try this approach:
bits = 0
for i in range(8):
    ...
    if image[ry][rx] > 0:
        bits += 1 << i


jQuery code to detect whether form fields have the default value when a checkbox is unchecked

There're multiple ways to do that. You should use a loop.

I've wrote a small script, that would do the job. I do not know if the if block is correct but you should easily fix that with this base.

$(document).ready(function(){
$('input[name="table_exist"]').click(function(){
 var table_exist_checked = $('input[name="table_exist"]').is(':checked');

 var fields = [
  'table_name',
  'user_id',
  'user_name',
  'user_email',
  'user_pass',
  'joining_date',
 ]; // Your field names

 // iterate over all fieldnames
 for(var index of fields) {
  var fieldName = fields[index]; // extract current fieldName
  var $elem = $('input[name="' + fieldName + '"]'); // save current element so we do not need to repeat

  // This is ur if-else. I do not exactly know if this is correct
   if($elem.val() == fieldName && table_exist_checked){
   $elem.val('');
  } else if($elem.val()=='' && !table_exist_checked){
   $elem.val(fieldName);
  }
 }

}); });


Can delegation be used to set these values at declaration time (thus reducing lines)?

By moving a few things around you could easily get rid of the init block, shortening the amount of lines.

Key parts is to move the Pattern to the companion object, and use .also on the matcher to make sure it is a correct match.

data class CommonLogEntry(private val entry: String) {
    companion object {
        private val formatter: DateTimeFormatter = DateTimeFormatter.ofPattern("dd/MMM/yyyy:HH:mm:ss Z")
        private fun asDateTime(line: String) = LocalDateTime.parse(line, formatter)
        val pattern: Pattern = Pattern.compile("^(\\S+) (\\S+) (\\S+) \\[([\\w:/]+\\s[+\\-]\\d{4})] \"(\\S+) (\\S+)\\s*(\\S+)?\\s*\" (\\d{3}) (\\d+) \"(\\S+)\" \"(\\S+)\"")
    }

    private val m = pattern.matcher(entry).also {
        if (!it.matches() || it.groupCount() != 11)
            throw IllegalArgumentException("Log Pattern doesn't match '$pattern'")
    }
    val clientIp: String = m.group(1)
    val rfc1413identity: String = m.group(2)
    val identity: String = ...
    val requestTime: LocalDateTime = ...
    val verb: String = ...
    val resource: String = ...
    val httpVersion: String = ...
    val statusCode: Int = ...
    val payloadSize: Long = ...
    val URI: URI = ...
    val userAgent: String = ...

However, I have to question the reason for writing this code. I have a feeling there is already some library out there that can parse these log files, it is the "Common Log Format" after all. Also, if you want to parse log files in general I would strongly recommend the tool Splunk which does have a free license for up to 500 MB of logs per day.


Computing pairwise bitwise OR in a list of bit-strings

I was able to get it accepted by optimizing the inner loop by using BitSets instead of plain strings. I then just used the BitSet's or and bit counting method (cardinality) to do the work. Here's the resulting code:

(import [java.util BitSet])

(defn string->bitset [str]
  (let [bitset (BitSet. (count str))]
    (doall (map-indexed #(when (= \1 %2)
                           (.set bitset %1)) str))
    bitset))

(defn bitset-or [s1 s2]
  (let [tmp (.clone s1)]
    (.or tmp s2)
    tmp))

(let [[N _] (clojure.string/split (read-line) #" ")
      N (Integer/parseInt N)
      bitsets (mapv #(string->bitset %) (repeatedly N read-line))
      known
      (for [i (range N) j (range (inc i) N)
            :let [s1 (bitsets i)
                  s2 (bitsets j)]]
        (.cardinality (bitset-or s1 s2)))
      maximum (apply max known)]
  (println maximum)
  (println (count (filterv #(= % maximum) known))))


1. Possible bugs

I can't be certain that these are bugs, but they look dodgy to me:

You can't filter on the number 0, because if args.c1 is 0, then it will test false and so you'll never compare it to c1. Is this a problem? If it is, you ought to compare args.c1 explicitly against None.
If the string c1 cannot be converted to an integer, then int(c1) will raise ValueError. Is this what you want? Would it be better to treat this case as "not matching the filter"?
If line has fewer than three fields, the assignment to c1, c2, c3, *_ will raise ValueError. Is this what you want? Would it be better to treat this case as "not matching the filter"?

2. Suggested improvement

Fixing the possible bugs listed above:

def filter_field(filter, field):
    """
    Return True if the string `field` passes `filter`, which is
    either None (meaning no filter), or an int (meaning that
    `int(field)` must equal `filter`).
    """
    try:
        return filter is None or filter == int(field)
    except ValueError:
        return False

and then:

filters = [args.c1, args.c2, args.c3]
fields = line.split('\t')
if len(fields) < len(filters):
    return False
if not all(filter_field(a, b) for a, b in zip(filters, fields)):
    return False


This is one of those times when "the rubber meets the road" as far as big-O complexity goes. You've got n = 2.5e6 words, so all that stuff about how \$O(\log n) < O(n) < O(n\log n) < O(n^2)\$ starts to be important.

Existing performance

You'll want to get a meaningful set of sample data, maybe 10 or 100 thousand words, and you'll want to look at using the timeit module, or something else you devise, to measure real performance.

That said, let's look at your code:

def index_containing_substring(the_list, substring):
    for i, s in enumerate(the_list):
        if substring in s:
            return i
    return -1

This function is called with a single word from the input list of words. It checks that one word against all the words already present in the_list. To do that, it iterates over the_list once, and then uses the in operator on each value.

Eventually, the_list is going to be sized based on the input word list. If nearly every word in the input list is a substring of some larger word, for example: [ 'aaaa', 'aaa', 'aa', 'a' ] then you will have one member. But if every word in the input is distinct, for example: [ 'a', 'b', 'c', 'd' ] you will have len(the_list) == N where N is the number of input words. For the worst case, we have to assume this.

For the use of substring in s, Python's substr in string is worst-case \$O(s \cdot t)\$, according to this post. In this analysis, s and t are lengths of the string and substring. Since your substrings eventually get added to the list, s == t. Thus, your index_containing_substring is worst-case \$O(n \cdot s^2)\$ (and average case about \$O(n \cdot s)\$).

def string_match():
    test_list=[...]
    safe_to_add=[]

    for s in test_list:
        if len(s)==max_len:
            safe_to_add.append(s)
        else:
            idx=index_containing_substring(safe_to_add,s)
            if idx==-1:
                safe_to_add.append(s)
            else:
                ...

In this function, you iterate once over the entire list of strings, calling the index_containing_substring function for almost every one. Your behavior is worst-case \$O(n)\$ where n is the length of the word list, times the behavior of the index_containing_substring function, which is \$O(n \cdot s^2)\$. Thus, your overall performance is \$O(n^2 \cdot s^2)\$, which since \$n \gg s\$ simplifies to \$O(n^2)\$.

It's worth noting that there are some "constant factors" in there which get ignored. For example, every time through the loop you check the length of your inputs to determine if you can add them without checking if they are contained substrings. This is a mistake for two reasons: first, what if they are duplicates? I don't see you doing any quality checks on the data, so I can't guarantee that two copies of "the longest word in the list" don't appear, and get added twice. Second, of course, is that you say the words are sorted in reverse order by length. This means that doing the substring-check on the first, long words is the cheapest operation possible. Checking for substrings in a list of length 0, or 1, is much much cheaper than it will ever be again. So you aren't saving very much. But you are doing that length comparison every single time.

I'd suggest that you either (1) abandon all that stuff entirely, and just pay the small price for the first couple of words; or (2) create a separate loop at the beginning to filter those words into the list, and then run your main loop starting after those are done.

An alternative

Suppose you computed every substring of a word. That would require iterating across the entire word, and for each point you reached you would iterate across all possible lengths of the substring:

word, wor, wo, w
 ord, or, o
  rd, r
   d

That is going to be \$O(s^2)\$ where s is the length of your word.

Now suppose you added all of the generated substrings to a Python set     that has \$O(1)\$ access time. You could do something like this:

for word in big_list:
    if word in set_of_all_substrings:
        ...
    else:
        add_all_substrings_of(word, set_of_all_substrings)

Assuming your "found" condition is \$O(1)\$, the performance is \$O(n \cdot s^2)\$, which again simplifies to \$O(n)\$.

Another alternative

Suppose you build a table of suffixes of all the words. That is, word[i:] for i in range(len(word)). Adding a single word's suffixes would be \$O(s)\$ as you can see, instead of \$O(s^2)\$ for substrings.

You could then binary-search the list, comparing word < suffix[:len(word)] to test for membership. The binary search would be \$O(\log n)\$ and you would do it n times, so your overall performance would be \$O(n \log n)\$. This seems worse than the \$O(n)\$ above, but it might be worth checking. The log(2.5 million) is about 22, and if your words have an average length of 10 letters, the difference between \$s\$ and \$s^2\$ would be 10, putting the two techniques a factor of 2.2 apart in performance. That's the kind of difference that code being written in C versus Python, or being particularly well-tuned for a particular purpose, can overcome.

Note that I don't expect this to work: inserting values into a sorted list gets quite expensive, so I suspect you'll find that this code loses the battle of small performance tweaks. But that's why they run horse races, eh?


Improve query: find users who are neither friends nor fans of a user

This query will give you the same results

SELECT username FROM users
WHERE username NOT IN (SELECT you FROM friends WHERE friend = 'user1')
AND username NOT IN (SELECT friend FROM friends WHERE you = 'user1')
AND username <> 'user1'

and the SQLFiddle

If you look at the execution plan, it looks like my query grabbed 1 less row throughout the execution and it didn't have to use a temporary or a join buffer.

This Query is more readable than yours, assuming this is what you are trying to do.


I think you can achieve the same with less code by utilizing some libraries and language features.
Google's Sitemap Protocol is not supported by default in the .NET Framework / Core.
But there are couple of packages which defines the corresponding C# classes, for example:

Google-Sitemap
X.Web.Sitemap

I have chosen the second one. So, the parsing logic would look like this:
public static async Task<XDocument> LoadSitemap(string filePath = "./sitemap.xml")
{
    var sitemap = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.None);
    return await XDocument.LoadAsync(sitemap, LoadOptions.None, CancellationToken.None);
}

public static Sitemap ParseSitemap(XDocument document)
{
    var serializer = new XmlSerializer(typeof(Sitemap));
    return (Sitemap) serializer.Deserialize(document.CreateReader());
}

The HEAD retrieval could be done by the following helper method:
private static readonly HttpClient client = new HttpClient();

private static async Task<string> GetHead(string location,
    Func<HttpResponseMessage, string> successHandler,
    Func<Exception, string, string> failureHandler)
{
    try
    {
        var timeoutPolicy = new CancellationTokenSource(5000);
        var request = new HttpRequestMessage(HttpMethod.Head, location);
        var response = await client.SendAsync(request, timeoutPolicy.Token);
        return successHandler(response);
    }
    catch (Exception ex)
    {
        return failureHandler(ex, location);
    }
}

The success and failure handlers can be implemented like this:
private static string HandleResponse(HttpResponseMessage response)
{
    var url = response.RequestMessage.RequestUri;
    return response.StatusCode == HttpStatusCode.OK
        ? $"{url} - OK"
        : $"{url} - Status code: {response.StatusCode}";
}

private static string HandleError(Exception exception, string url)
{
    return exception switch
    {
        HttpRequestException hre => $"{url} - unreachable due to {hre.Message}",
        TaskCanceledException tce => $"{url} - timed out due to {tce.Message}",
        _ => $"{url} - failed due to {exception}"
    };
}

Let's put all these pieces together:
static async Task Main(string[] args)
{
    var siteMapXml = await LoadSitemap();
    var siteMap = ParseSitemap(siteMapXml);

    var siteChecks = siteMap
        .Select(url => GetHead(url.Location, HandleResponse, HandleError))
        .ToList();

    await Task.WhenAll(siteChecks);

    foreach (var siteCheck in siteChecks)
    {
        Console.WriteLine(await siteCheck);
    }

    Console.WriteLine("Finished");
}


You can secure against CSRF (pronounced C-surf) by using ValidateAntiForgeryTokenAttribute and AntiForgeryToken method.

How CSRF (Cross Site Request Forgery) works:

  We have 2 actors, an evil person E and a good person G.   E makes an
  evil html image tag on some forum, that links to your website and puts
  a product in your user's cart.

  If G has a cookie for your site and the cookie is not expired, when G
  goes to E's forum, the browser tries to load the html img and it will
  send a Request to your site. Even though G didn't intend to do that...

Implementation:

View:

@using (Html.BeginForm("AddToCart", "Cart"))
{
   @HtmlHelper.AntiForgeryToken()
   // Code -->
}

Controller:

[HttpPost][ValidateAntiForgeryToken]
public ActionResult AddToCart(int id, int quantity)
{
   // Code -->
}

How it prevents CSRF:

In the view a hidden input field with a unique string is created.
A cookie is sendt to the user with the same string. (I believe it is a httpOnly cookie).
The controller checks if the form contains the same string as is in the cookie.

Because person E, cannot form a valid form post the cross site request will fail. But note that if you have XSS (Cross site scripting) vulnerability this security check can be circumvented.

And another thing... If your 2nd method public void AddToCart(Product product, int quantity) is not an action consider using the [NonAction] attribute, because all public methods in a controller are considered by default Action methods. Meaning it is invokable as an Action method.


There's likely a difference in your implementation and the library's. The algorithm you used is an inefficient variant of the inefficient brute force algorithm (yes, doubly inefficient). Better algorithms exist for finding a substring, for example the Knuth-Morris-Pratt algorithm. I suspect the library uses something more clever than brute force.

It's strange and inefficient to have the find.length() > 0 inside the loop. The value of find.length() doesn't change inside the loop, so it's better to check it once, before the loop begins.

Counting the characters found is inefficient if you don't break on a mismatch.
Instead of counting the characters found, it would be better to use a boolean flag. When you find a character that doesn't match, you can set the flag and break out of the loop. After the loop, if the flag changed, you know the string wasn't found.

That is:

for (int i = 0; i < str.length() - (find.length()-1); i++) {
    bool mismatch = false;
    for (int x = 0; x < find.length(); x++) {
        if (str[i + x] != find[x]){
            mismatch = true;
            break;
        }
    }
    if (!mismatch) {
        return true;
    }
}
return false;

Instead of using a flag, what's even better is to extract the inner loop to a function, as Jerry-Coffin did.

You can use boolean expressions directly as return values. So instead of this:

if (str.find(find) != string::npos) return true;
return false;

You can write:

return str.find(find) != string::npos;

There are multiple accepted naming conventions in C++,
but I don't think there is when where local variables follow PascalCase. It would be better to change to camelCase.


merge list of strings into one sorted string

counter = Counter()
for string in strings:
    for char in string:
        counter[char] +=1

You are flattening your strings list to count each individual characters. For starter, if you were to extract letters individually, you could feed it to the Counter constructor and avoid the += 1 operation. Second, flattening an iterable is best done using itertools.chain.from_iterable:

counter = Counter(itertools.chain.from_iterable(strings))

result = []
for c in ascii_lowercase:
    if c in counter:
        for _ in range(counter[c]):
            result.append(c)
return "".join(result)

Instead of the inner for loop, you can create repeated sequence of characters using c * counter[c]. This also have the neat advantage to produce the empty string ('') when counter[c] is 0, removing the need for the c in counter test:

return ''.join(c * counter[c] for c in ascii_lowercase)

These changes result in a ×4 speedup on my machine.


Splitting URLs into hierarchy-level directories

All in all, this is not bad. A few remarks:

Python 2

Why choose Python 2? Python 3 has a lot of advantages, and will get more support in the future. Even if you need to code for Python 2, you can make your code compatible with both versions:

from __future__ import print_function
try:
    import urlparse
except ModuleNotFoundError:
    import urllib.parse as urlparse

docstring

Adding a docstring can better explain what the different methods do.

range

You don't need the range as list, so you might as well use xrange.

To make the code Python 3 compatible, you'd have to add xrange = range to the except clause on the import, so this might not be worth it in this case, but in general, use the iterable version as much as possible.

'/'

You do a lot of operations with '/'.

you remove a trailing / if it exists
you remove the starting / from urlparse.urlparse(url).path with [1:]
you add a trailing / in get_domain_with_protocol to the first part of the url

You can combine 1 and 2 by doing path.strip('/'). Or you can drop both 2 and 3, and iterate over range(1, len(parts).

generator

Instead of returning a list, you can also make a generator:

for index in range(len(parts)):
    yield domain_with_protocol + '/'.join(parts[:index + 1])

iteration 1

In general I try not to do things like range(len(parts)), but use enumerate. Here you could do for index, _ in enumerate(parts)

iteration 2

I try to avoid iterating over the index, and try to use generators as intermediate product instead of lists. Imagine parts would be an iterable instead of a list, your approach would not work.
In Python 3, you could use itertools.accumulate, but in Python 2, you'd have to write your own accumulator:

def accumulate_parts(parts, sep='/'):
    parts_iter = iter(parts)
    substring = next(parts_iter)
    yield substring
    for part in parts_iter:
        substring += sep + part
        yield substring

def get_url_directories_accumulate(url):
    path = urlparse.urlparse(url).path
    parts = path.strip('/').split('/')
    domain_with_protocol = get_domain_with_protocol(url)
    for substring in accumulate_parts(parts):
        yield domain_with_protocol + substring

timings

I've timed these variations both in Python 2 and Python 3, and all of them are within a few % of each other, so you can pick the one that suits you best, and you'll be able to understand in a few months/years.

code

Full code and timings can be found here.


Function to create URL relative to a configured base URL

Yes, it's correct to use const here. That would be considered idiomatic Javascript. If you're using const when you shouldn't, you'll notice as it will produce a TypeError.

People coming from other languages might disagree as they think that const means that the variable is constant/immutable (which is not the case, it's just not re-assignable). But that is really a critique of the naming of the keyword, not your usage of the keyword (which is correct).


In this case the answer is very straight-forward if you want to keep things in plain ol' jQuery.  Instead of selecting on ids, use classes and rely upon a well structured dom (which you already have):

$(document).ready(function() {
  $("body").css("background-color", "yellow");

  $(".dropdown_with_toggle").on('click', function() {
    var el = $(this);
    $('.menu', el).slideToggle();
    var arrow = $(".arrow", el);
    arrow.toggleClass("right");
    arrow.toggleClass("down");
  });
});

Then adjust your HTML slightly:

<!DOCTYPE html>

  <link href="style.css" rel="stylesheet" type="text/css">
  <title></title>

Dropdown 1

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Dropdown 2

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute
            irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

They key is that the class dropdown_with_toggle is not being used for styling but rather to guide your javascript.  You listen for click events on all elements with that class, and then you take action not by finding elements by id, but by finding children of your main element by class.  That is what the $('.menu', el) syntax does: it finds the selector but only looks for descendants of a given element.  It's effectively just an alternate syntax for the jQuery .find() method, which would work equally well here.

Just because: the next step is to not attach your listeners directly to the elements but instead use deferred listeners on a mutual parent:

$('.container').on( 'click', '.dropdown_with_toggle', function(){ } )

The rest of your code doesn't change, but the advantage this notation has is that if you add or remove elements dynamically, you don't have to re-register any listeners (as long as you are only modifying elements inside your container div, and the container div isn't removed).  I used to use this trick often with dynamic pages that are loaded/reloaded via ajax.


Or if you prefer match/case to if/else:

(1 until 100).map(i => (i % 3, i % 5) match {
  case (0, 0) => "FizzBuzz"
  case (0, _) => "Fizz"
  case (_, 0) => "Buzz"
  case _ => i
}).foreach(println)

Update:

So what we are doing here is taking list of numbers and mapping them first to tuples where on the left side is that number module 3 and on the right side modulo 5. Then we are matching those tuples to cases where both are zero, left is zero, right is zero or neither is zero.

Also the actual logic is in the map-block and side-effect of printing is in the foreach-block.


Don't do this (its not readable).

using std::string; using std::cin; using std::cout; using std::endl; using std::ostream; using std::vector;

One declaration per line.

using std::string;
using std::cin;
using std::cout;
using std::endl;
using std::ostream;
using std::vector;

But actually prefer not to do it at all. If you must do it then confine it to the most narrow scope possible. The whole reason that the standard library is std:: is so that it does not cost much to prefix members by it.

std::cout << "Hello World" << std::endl; // Not that hard

It cost nothing to clear an empty tree.

if (!Tree.empty())  // Why do this test. Just call clear.
    Tree.clear();

In C++ the & and * are usually placed with the type (its type information). This is different from C.

bool tree::makeTree(int newBase, const string &newCont = "*") {
                                             ^^^  Looks strange.

// I would write it like this:
// Note: I also put the const the other side (but that's more a personal
// preference and there is no clear better style there).
bool tree::makeTree(int newBase, string const& newCont = "*") {

Don't use integers to represent bools. At the end of makeTree() I see.

return 1;

// Use a boolean value.
return true;

But there is no failure value so why return anything at all?

Prefer '\n' over std::endl. The difference between the two is that std::endl does an extra flush. This can make using it very inefficient. The whole point of using the buffer is to make printing efficient and the streams are flushed at optimal points automatically and forcing a flush will make the use of stream less optimal.


Efficient method to sort read-in text file in Excel

I'm confused here—you are looping through FilterAndDeleteB for every row with a value in column A? Let's look at the logic:

Count rows for column A
    From last row of column A to the first row of column A
        If first 3 characters of A cell are `CHA` `HAM` or `BKN`
            count rows in column B
                for last row of column B to the first row of column B
                    If B cell starts with `-` then delete current row
        Else
            delete current row
                count rows in column B
                for last row of column B to the first row of column B
                    If B cell starts with `-` then delete

Next row in Column A

Right, so for every single row in Column A, you will loop through every single row in column B. Once you loop through column B, you should never need to loop through it again, according to the logic. Everything beginning with - will already have been deleted.

Instead, I imagine, you'd rather look for CHA, HAM, and BKN, and for every row where one of those isn't there, if there's a - in column B, delete those rows—right? If so, consider this (untested) code:

Option Explicit

Public Sub FilterAndDeleteRows()
    Const KEEP_STRING As String = "CHA,HAM,BKN"
    Dim lastRow As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    Dim i As Long
    Dim beginningString As String
    For i = lastRow To 1 Step -1
        beginningString = Left(Cells(i, 1), 3)
        If NOT InStr(1, KEEP_STRING, beginningString) > 0 Then
            If Left(Cells(i, 2), 1) = "-" Then Rows(i).Delete
        End If
    Next i
End Sub

Now it's only one loop, and it's skipping the rows where A matches.

But the more likely place to improve the speed of your filtering is by filtering when getting data from the file. At that point, you probably have the data in an array and you probably could just move records from that array that match your criteria to a new array, which you print out at the end.


Some small improvements:

print_dict method: use iteritems()

for key, value in dictionary.iteritems():
print("{} : {}".format(key, value))

You can merge validate and is_integer. There are no benefits here gained by separating them in two different functions.
For lookup_term, use either:

value = dictionary.get(term)
if value:
    print("{} : {}".format(term, value))
else:
    print("Term does not exist, input 2 to add new term")

Or you could use:

if term in dictionary:
    print("{} : {}".format(term, dictionary[term]))
else:
    print("Term does not exist, input 2 to add new term")

The point I wanted to illustrate was use of dict.get()
In delete_term, you are not validating for input when key is not in dictionary.

Some other things:

Since all functions work with same dictionary, they could have been grouped together under same class with dictionary being shared data member.

Also, your dictionary is case sensitive. Looking at your use-case, this is not something you want.

If at anytime you need to change menu, you would need to change validate function. Maybe there should be easier way to do this? (Instead of else denoting 5 in menu, else could work for something else?)


The standard American English spelling is "converter", not "convertor". If it's different where you live, use the appropriate regional spelling, but if you live in America, use the American spelling. Since I do, I did.

You have @converter and @uploader, but @email. I suggest changing it to @emailer, for consistency's sake.

You mix symbols and strings as Hash keys. I suggest using all symbols, since it's much more standard. This means that

task['reports']['workspace']['uri']

becomes

task[:reports][:workspace][:uri]

Is the workspace at [:reports][:workspace] or [:workspace]? Based on the very similar [:reports][:repository] that you use, I'm assuming it's the former. Pick one and stick with it.

You use #{task[:reports][:repository][:uri]}/#{task[:report_id]} a couple times in your strings, and it's long. Why not declare a variable?

repository_uri = "#{task[:reports][:repository][:uri]}/#{task[:report_id]}"

repository_uri might not be the best name; I'd recommend picking whatever works best.

Your heredocs are weird. What I normally see is something like this:

      puts <<-MESSAGE.gsub(/^[ \t]*> /, '')
        > Stuff!
        > More stuff!
        > #{pound_sign}Octothorpe
      MESSAGE

Note the two differences between that syntax and yours:

Instead of << at the beginning, I have <<-. This lets you have whitespace (i.e. indentation) immediately before the closing tag, so it's in the same indent level as the rest of the code.
Immediately after the beginning tag, I have .gsub(/\s*> /, ''). This operates exactly like a normal .gsub call on the contents of the heredoc, after string interpolation, and you can feel free to change the regex in front to whatever suits you. It's there to let you specify where the line starts; in effect, it allows you to indent the contents and have indenting in the string itself. Note that this specific regex requires a space after the > or the entire line is there, unchanged. Also, it uses [ \t] rather than \s because \s will also match newlines, and it's probably better to allow multiple lines.

The benefit of this approach is that you don't have to get rid of all indents for however many lines; the drawback is that it's ever so slightly slower, because it has to perform a gsub on the end result.

Now, you build your hashes oddly. Rather than creating an empty one, then putting values in it, why not build it with the values already in it? For example, this:

convert_context = {}
convert_context[:type] = 'geo'
convert_context[:output] = "#{task[:workspace]}/#{task[:report_id]}.json"

becomes this:

convert_context = {
  type:   'geo'
  output: "#{task[:workspace]}/#{task[:report_id]}.json"
}

If you store "#{task[:workspace]}/#{task[:report_id]}.json" as a variable (let's call it workspace_url; you'll want a better name) then you can use this super cool syntax for calling methods with hashes:

@converter.convert(
    type: 'geo',
    output: workspace_url,
)

You could, of course, put the string in directly in both places where you reference it, if you want to. It's your choice.

Note that with heredocs, you have to put the comma on the first line for it to register; otherwise, it doesn't see the end of the doc . This is due to a mildly annoying quirk in how heredocs work.

If I'm using this class and see that the parameter is optional, and I don't know what it does, I'm gonna leave it out, then when it starts putting nothing in the URLs (or throwing NoMethodError: Undefined method `[]' for nil:NilClass) I'm gonna be really confused, since it should work with the default argument, instead of throwing errors!

Of course, someone could easily supply a blank Hash themselves. It's much safer to create a basic ReportableTask (because Task is really generic and a bad name for a class) class as a data wrapper; something like this is all you need:

class ReportableTask
  def initialize(workspace, reports, report_id)
    @workspace = workspace
    @report_id = report_id
    @reports = reports
  end

  attr_accessor :workspace, :report_id, :reports
end

You could even put in some convenience methods, like repository_uri that returns @reports[:repository][:uri].

Then, if you insist on being able to use your code totally unchanged, you can add a method:

def [](name)
    instance_variable_get("@#{name}")
end

but I'd recommend just changing your code.

Then you could make similar objects for reports -- maybe a Report class? -- and maybe lots of other things. Without seeing the structures you have in mind I can't tell you for sure what'd be best or even give advice on providing that, but in general if you're making lots of hashes with all the same keys it's good to create a class, especially if the usage of those hashes depends on them always having those keys.

Anyway, aside from those tips, it's pretty hard to give a more complete/accurate review without seeing examples of the data you're passing in.

With the changes I suggested (except the one about adding a new class), here's what your code looks like:

class PublishReportProcess
  def initialize(converter, uploader, emailer)
    @converter = converter
    @uploader = uploader
    @emailer = emailer
  end

  def run(task)
    repository_uri = "#{task[:reports][:repository][:uri]}/#{task[:report_id]}"
    workspace_uri = "#{task[:workspace]}/#{task[:report_id]}.json"

    @converter.convert(
        type:   'geo',
        output: "#{task[:workspace]}/#{task[:report_id]}.json",
    )

    @uploader.upload(
        local:   workspace_uri,
        remote:  repository_uri,
        staging: "#{task[:reports][:workspace][:uri]}/#{task[:report_id]}",
    )

    @emailer.emailer(
        message: <<-MESSAGE.gsub(/\s*> /, '')
          > There is a new report available at #{repository_uri}.
          >
          > Share and enjoy
        MESSAGE
    )
  end
end

Note: I didn't add ReportableTask because I don't know how you want to implement it, or if your code structure makes it better suited for another place. If you'd like, I can provide a sample implementation of that as well.


if(argc < 2){
    printf("The path to the file is\n");
    return 1;
}

The error message is unfinished (appropriately enough, missing is missing), and it should go to stderr, not stdout.

if(stat(argv[1],&fileStat) < 0){
    printf("error\n");
    return 1;
}

Again we're writing to the wrong stream.  And we could make a more informative message, e.g. perror(argv[1]).

        Search_in_File(argv[1], "#!");
        Search_in_File(argv[1], "ELF");

These two calls each open the file to read it.  It is probably better to open the file just once.  Why do we ignore the return value each time?

while(fgets(temp, 512, fp) != NULL) {
    if((strstr(temp, str)) != NULL) {
        ⋮
    }
}

This code will fail to match when the search string straddles a 512-byte boundary.  But why are we searching the whole file anyway?  Those magic strings are only meaningful as the first characters, and do not tell us anything if they occur later in the file.  We could do this much more simply:
if (fread(temp, 4, 1, fp) == 1
    && strncmp(temp, str, strlen(str)) == 0) {
    ⋮
}

        if(strcmp(str, "#!")==0)
            printf("shell type.");
        else if(strcmp(str, "ELF")==0)
            printf(" ELF.");

Instead of string-matching the str argument, why not just pass an additional argument specifying the string to print?  Then there's only one place to modify when you add a new test.

if(fp) {
    fclose(fp);
}

At this point, we know that fp is non-null, since we already returned if it isn't.
Oh, and an executable that begins with #! could be any type of interpreted executable (e.g. Python or Sed script), not necessarily a shell script.  Not sure whether that's what's intended (is it your description or the program that needs adjusting?).


Merge adjacent occurrences of identical elements in data collection

Generally looks okay (though I would prefer shorter line lengths, and the indentation needs cleaning up).

It seems that we've focused on using std::back_insert_iterator as the output iterator; this leads to a couple of problems that are visible when we switch to a different iterator, such as a collection iterator or raw pointer:

The caller doesn't know where the output finished; we should return the final value of out, just as std::unique_copy() does.
The assignment out = neither indirects nor advances the iterator; it should be *out++ =.

We don't need a separate start copied from first, and we can use std::adjacent_find to determine the range of equal elements:

#include <algorithm>
#include <functional>
#include <numeric>

template <typename ForwardIterator, typename OutputIterator,
          typename Equal, typename Merge>
OutputIterator merge_adjacent(ForwardIterator first, ForwardIterator last,
                              OutputIterator out,
                              Equal equal, Merge merge)
{
    while (first != last) {
        auto stop = std::adjacent_find(first, last, std::not_fn(equal));
        if (stop != last) {
            // advance to include the first of the pair
            ++stop;
        }
        *out++ = std::accumulate(first+1, stop, *first, merge);
        first = stop;
    }

    return out;
}

This version still performs two passes (one for the search, and one for the accumulate).  The commented-out, lower-level version of the function should be able to accept input iterators, as a single-pass algorithm.

Looking at that version, there's only a little to improve.  I'd probably post-increment the iterators while dereferencing, rather than as a separate statement, but that's just a style preference.  One thing I would change would be to use std::move() when finishing with next, to minimise unnecessary copying:

#include <algorithm>
#include <numeric>
#include <utility>

template <typename InputIterator, typename OutputIterator,
          typename Equal, typename Merge>
OutputIterator merge_adjacent(InputIterator first, InputIterator last,
                              OutputIterator out,
                              Equal equal, Merge merge)
{
    while (first != last) {
        auto next = *first++;
        while (first != last && equal(next, *first)) {
            next = merge(std::move(next), *first++);
        }
        *out++ = std::move(next);
    }

    return out;
}


Idiosyncrasies

declareing your fns

It's pretty unusual to declare all of your functions before defining them.

It appears that you did it so that you could define your functions in the order you thought
most appropriate. That's common for newcomers, and it's certainly something many people are
used to. To be clear, there's nothing wrong with wanting that.

However, most clojure programs work within this constraint and just define things in the
order they are needed. After a while you become accustomed to seeing things in that order,
and you start to look at the bottom of the file for the program overview. This has the
minor benefit of calling out the places where circular references are actually needed.

Manually calling -main

This is a toy program, so calling -main in your namespace probably does what you want 100%
of the time. However, in a real codebase, doing that can pretty much make your namespace
unusable for other namespaces. If you're using lein you can call:

lein run -m concurrency.dining-philosophers

If you're running raw clojure use:

java -cp "clojure.jar:$OTHER_JAR_PATHS" clojure.main -m concurrency.dining-philosophers

Program Structure

Decouple Philosophers

Your philosophers are aware of a lot of implementation details. Really all they care
about is their name, their left fork, and their right fork. You transmit all of that information
in a single number, which is convenient, but it means you have to be concerned with how
forks are retrieved in the philosopher code as opposed to just using the forks you're assigned.

The consequence of this is it binds your functions together in an undesirable way.

Leverage Sequences

Your solution uses a lot of indices and math to manipulate those indices. It could be made
dramatically simpler by manipulating sequences instead. (Hint: Use the above advice and the
partition function.)

Microfunctions

This is a really common thing for newcomers to clojure, myself included.

When I did a lot of Java, I tended to like very small functions, and I would create arbitrary
functions (e.g. get-fork, release-fork and wait-retry-time) for no other reason than to cut
down on my line count. After working with clojure for a while, function length became less important
than overall comprehensibility. So what I try and do now is look for fundamental concepts in my
program as opposed to places where I can inject a random function.

For example, for this problem, I see get-forks, release-forks, eat, and think as primitives.
They are things that are inherent to the problem. You may or may not name them, but they are definitely
there.

You never really want to get a single fork at one time, so get-fork seems too small to be a standalone
concept. The same for release-fork. wait-retry-time is just a pseudonym for (Thread/sleep retry-time). The latter is going
to be more obvious to any newcomer to the codebase.

General Comments

Use of core.async

Just to let you know, since you pulled in core.async, it's possible to do a lot of the
mechanics around starting/stopping threads with core.async. This has a couple of advantages:

Removes the need to use Java constructs like Thread/sleep, .start, .wait. (As far as I know, this is really just an aesthetic advantage)
Allows you to easily interact with processes (i.e. start them and stop them) (Another potential option here is Java's Executor
infrastructure.)

So it's not a massive gain, but the option is there if you choose.

No cleanup mechanism

While we're on the topic of interacting with the processes, the current structure doesn't
leave any hooks for cleaning up your threads. This works fine for a toy program where you'll
let it run then ctrl-c, but any real-world system will need to handle that
cleanly. So it might be nice for you to use the opportunity to think about how to structure the
program so you can clean up on demand.

Unbuffered logger-chan

Since your logger-chan is an unbuffered chan you've introduced synchrony around your logging
when you probably want some asynchrony there. I can't see any harm in setting a buffer of ~100 or
~1000 on that channel. It'll almost certainly be way ahead of its workload, and in the event it gets
behind, it'll still give some backpressure so you won't lose any events.

Closing Thoughts

I really like that you chose clojure's STM system for this problem. It's not often used, but by
far the best fit. Also, despite my nitpicking, the big picture flow of philosopher is straightforward
in the best kind of way.

Please let me know if you have any questions about my comments. I'm more than happy to give further feedback!

Edit in response to OPs comments

go-block lockup

You only get lock up if you occupy the thread (e.g. with Thread/sleep, or by doing IO or doing a CPU-intensive process). If you "park" it with (<! (a/timeout msecs)) it doesn't block the state machine.

core.async vs future

I was referring to the thread macro. I don't think there's any substantial benefit to core.async over the futures with and Executor. I actually did a version with ExecutorService and Future, and it has the same feature set. FWIW I think it's also more concise than the version I did with core.async.

However, it is very beneficial to use one of those two over Thread/start and Thread/wait. Those are pretty rudimentary constructs. And they suck for most use cases.

Function Size

You pose a really good point regarding get-fork. You do indeed want to eliminate repetition where possible (though not at the expense of simplicity). I would agree that in the current implementation, get-fork needs to be its own function. However, I don't think getting a single fork is necessary for a solution to this problem.


Booking appointments with MySQLi and PHP

Code has to be readable

  if(isset($_POST['slots_booked'])) $slots_booked = mysqli_real_escape_string($link, $_POST['slots_booked']);

Going through this code I really have to focus to try to figure out what is happening, there is no 'flow'. Now add 7 of those and you got some serious reading ahead of you. You are using prepared statements so don't escape your values 2 times, prepared statements protect from sql injection.

Use meaningful english names

Database columns, variable names etc have to be in english and have to tell what they are responsible for.

  $booking_array = array(
  "slots_booked" => $slots_booked, // not entirely clear what this column stores
  "booking_date" => $booking_date,
  "name" => $name,
  "email" => $email,
  "Åkeri" => $akeri, // lets open up google translate
  "phone" => $phone,
  "typ" => $typ, // no idea what this column stores
  "fri" => $fri // no idea what this column stores
  );

As @Sumurai8 noted: $booking_array is not even used yet it takes up 30% of the code.

This one is just left there hanging. I suppose $slots_booked are an array of some slots? No idea what those slots represent. Variable name $explode will not help with understanding code it is used in.

 $explode = explode('|', $slots_booked);

Foreach does not start that bad. $explode is array of $slot's, it asks for renaming $explode to $slots.

What kind values do you expect $slot's to have? Checking empty($slot) does not cut it?

How short preparation of query is! Just 3 lines of code. It's great isn't it? Not necessarily. You sacrificed code's readability by making it so short. Each parameter should have proper placeholder name => :name, start => :start. I would recommend providing bindParam() with type you expect given param to have.

Here we are given another clue as to what $slot might be. It is start of bookings. Maybe instead of $slot something like $booking_start would be more understandable?

foreach($explode as $slot) {

    if(strlen($slot) > 0) {

        $stmt = $link->prepare("INSERT INTO bookings (date, start, name, email, phone, typ, fri) VALUES (?, ?, ?, ?, ?, ?, ?)");
        $stmt->bind_param('sssssss', $booking_date, $slot, $name, $email, $phone, $typ, $fri);
        $stmt->execute();

    } // Close if

} // Close foreach

Below code is example in which way your code can go and be more readable. DB object was taken from here.

class Booking {
    public $start;
    public $name;
    public $akeri;
    public $email;
    public $phone;
    public $typ;
    public $fri;
    public $date;

    public function save() {
        $stmt = DB::prepare("INSERT INTO bookings (date, name) VALUES (:date, :name)");
        $stmt->bind_param(':date', $this->date);
        $stmt->bind_param(':name', $this->name);
        // rest of attributes
        $stmt->execute();
    }
}

$bookingDates = explode('|', $_POST['slots_booked']);
foreach($bookingDates as $bookingDate) {
    $booking = new Booking();
    $booking->name = $_POST['name'];
    // set rest of attributes

    $booking->save();
}

EDIT

Changed $bookingDetails to $bookingDates


Equivalent of C-style "Static Local Variable" in Swift

The only work around for this that I've come up with thus far is something that looks like this:

func foo() -> Int {
    struct Holder {
        static var timesCalled = 0
    }
    return ++Holder.timesCalled;
}

It's pretty clunky in my opinion, and I'm not sure why Apple doesn't just allow straight static function variables, but this seems quite a bit cleaner than your approach.


It seems you are using asyncGetInput and asyncGetReponse to introduce concurrency, although this is a noble goal it is kind of useless because you are only using those from other workflows which are already running in the thread pool anyway. In that case it would be ok to make a synchronous call from inside those workflows.

The second point is about the do! at the end of asyncPrintResponse, you should use return! in your recursive call otherwise you will get a memory leak. I'm not sure if we can call this a bug because it can probably be considered normal that the definition of Do in the workflow isn't built for that. I've personally been corrected for that same error here: https://stackoverflow.com/questions/6905251/f-async-difference-between-two-structure

[edit]
There's a difference between handling concurrency and introducing concurrency. If we ignore your code for a moment, you have two distinct inherently sequential workflows that you can parallelize and asyncSendInput and asyncPrintResponse is already doing that by itself, as such asyncGetInput and asyncGetResponse are unnecessary and will only create more lightweight thread that will end up being scheduled/deschedule from the thread pool.

Here's a little cleanup:

open System
open System.Net.Sockets

let rec asyncSendInput (stream : NetworkStream) =
    async {
        let input = Console.Read() |> BitConverter.GetBytes
        input |> Array.iter stream.WriteByte
        return! asyncSendInput stream
    }

let rec asyncPrintResponse (stream : NetworkStream) =
    async {
        let response = stream.ReadByte() |> Char.ConvertFromUtf32
        Console.Write(response)
        return! asyncPrintResponse stream
    }

[<EntryPoint>]
let main args =
    let client = new System.Net.Sockets.TcpClient()
    client.Connect(args.[0], Int32.Parse(args.[1]))
    printfn "Connected to %A %A..." args.[0] args.[1]
    let stream = client.GetStream()
    printfn "Got stream, starting two way asynchronous communication."
    asyncSendInput stream |> Async.Start
    asyncPrintResponse stream |> Async.RunSynchronously
    0


Can't really tell what your code does in real life, thus the following optimization/organization might not work. However, following generic tips, here's what I got:

//we can reduce DOM fetching by fetching html first
//caching it as well as other frequently used elements
//assuming that they never change in the course of your page's life
var html = $('html')
  , footer = $('footer')
  , foo = $('.foo')
  , bar = $('.bar')
  , options = {
      queue: false,
      duration: 1500,
      complete: function () {
        if (!foo.hasClass('active')) {
          foo.toggleClass('active');
          bar.slideToggle();
        }
      }
    }
  ;

//then here, we fetch body only when it's not IE9+ thus reducing operations
html = html.hasClass('lt-ie9') ? html : $('body');

$('.var').on('click', function (e) {

  //i think the properties can't be cached since it may need current values
  //however, your options can be cached since they are static values
  html.animate({
    scrollTop: footer.offset().top
  }, options);

  //an answer as to when to use return false vs preventDefault
  //you can use either, depending on the situation
  //http://stackoverflow.com/a/1357151/575527
  return false;
});


import os, re, urllib

def Read(url):

Python convention is to name function lowercase_with_underscores

    uopen = urllib.urlopen(url)

I recommend against abbreviations

    uread = ''
    for line in uopen: uread += line
    return uread

Use uopen.read() it'll read the whole file

def Find(what, where):
    found = re.findall(what, where)
    match = []
    for i in found:

avoid single letter variable names

        i = i[9:-1]
        match.append(i)
    return match

Combine short lines of code

 for i in re.findall(what, where):
      match.append(i[9:-1])

I think its cleaner this way. Also use capturing groups rather then indexes.

def Retrieve(urls, loc):
    loc = os.path.realpath(loc)
    os.system('mkdir ' + loc +'/picretrieved')

Use os.mkdir rather than using system

    loc += '/picretrieved'

Use os.path.join to construct paths

    x = 0
    exts = ['.jpeg', '.jpg', '.gif', '.png']
    for url in urls:
        x += 1

use for x, url in enumerate(urls):

        for i in exts:
            ext = re.search(i, url)

Don't use regular expressions to search for simple strings. In this case I think you really want to use url.endswith(i). Also, stop using i everywhere.

            if ext != None:
                ext = ext.group()
                urllib.urlretrieve(url, loc + '/' + str(x) + ext)
            else:
                continue

This continue does nothing

    print 'Placed', str(x), 'pictures in:', loc

You don't need to use str when you are using print. It does it automatically.

def main():
    url = raw_input('URL to PicRetrieve (google.com): ')
    url = 'http://' + url
    loc = raw_input('Location for PicRetrieve older ("." for here): ')
    html = Read(url)
    urls = Find('img src=".*?"',  html)

Generally, using an html parser is preferred to using a regex on html. In this simple case you are ok.

    print urls
    Retrieve(urls, loc)

if __name__ == '__main__':
    main()