Ultra-simple, automatic, non-float, 1-4 column, grid system Has everyone moved over to CSS Grid and/or Flexbox? If not, I've maintained and evolved this system over a fair amount of time, the concept was derived originally, I believe, from Mary Lou, although I don't recall where the article is.

I was never a proponent of floats, although the using font-size:0 to clear whitespace isn't an ideal solution either, neither is a DOM Cleaner function in javascript. Probably minimized (non-formatted) HTML without whitespaces is ideal, just difficult to maintain... There's a lot of other "hacky" concepts, comments in HTML, bleh... but I landed on the font-size:0. This served me well and I still use it in a handful of projects, though I've mostly moved to a CSSGrid/Flexbox combination as of late.

It's a similar to:

.table {display: table}
.table>div {display: table-cell}

With a higher degree of flexibility, if you need heights to match, the above or flexbox/css grid are probably a better solution. or min-height on columns if it's known content.

Any ideas to improve or flaws, let me know.

.autogrid {
    padding: .5rem; /* 8px */
    /** remove whitespace from container */
    font-size: 0;
    box-sizing: border-box}

  .autogrid>* {
    box-sizing: inherit; /* inherit border-box */
    display: block;
    font-size: inherit; /* inherit 0px */
    margin: 0 auto .5rem; /* 0 auto 8px */
    padding: 0 .5rem .5rem /* 0 8px 8px */}

  /** create vertical space in columns & prevent margin-collapsing */
  .autogrid>*:before {
    content: '';
    display: inline-block;
    vertical-align: 0; /* baseline or 0% */
    width: 0;
    height: .5rem /* 8px */}

  @media (min-width: 33.75em) { /* 540px */

    .autogrid>* {
      display: inline-block;
      margin: .5rem; /* 8px */
      /** align columns/cards to top, middle, etc. */
      vertical-align: top}

    .autogrid>*:first-child:nth-last-child(1),
    .autogrid>*:first-child:nth-last-child(3),
    .autogrid>*:first-child:nth-last-child(3)~* {width: calc(100% - 1rem) /* 100% - 16px */}

    .autogrid>*:first-child:nth-last-child(4),
    .autogrid>*:first-child:nth-last-child(4)~*,
    .autogrid>*:first-child:nth-last-child(2),
    .autogrid>*:first-child:nth-last-child(2)~* {width: calc(50% - 1rem) /* 50% - 16px */}

  }

  @media (min-width: 50em) { /* 800px */

    .autogrid>*:first-child:nth-last-child(3),
    .autogrid>*:first-child:nth-last-child(3)~* {width: calc(33.333% - 1rem) /* 33.333% - 16px */}

    .autogrid>*:first-child:nth-last-child(4),
    .autogrid>*:first-child:nth-last-child(4)~* {width: calc(25% - 1rem) /* 25% - 16px */}

  }

  /**
  ###############
  #  DEMO ONLY  #
  ###############
  html:
  `

col

col

col

col

  `
  */

  html {
    font: 400 100%/1 sans-serif;
    color: #333}

  body,
  p {
    font-size: 1rem; /* 16px */
    line-height: 1.45; /* 145% or 23.2px */
    margin: 0}

  .autogrid>* {text-align: center}

  .autogrid>*:first-child,
  .autogrid>*:nth-child(2) {color: #fff}

  .autogrid>*:nth-child(3),
  .autogrid>*:last-child   {color: inherit}

  .autogrid>*:first-child  {background-color: #4af}
  .autogrid>*:nth-child(2) {background-color: #fa4}
  .autogrid>*:nth-child(3) {background-color: #def}
  .autogrid>*:last-child   {background-color: #fed}

col

col

col

col

This is not to take away at all from your system above, which is very smart.

I'm still very much on the CSS Grid learning curve and I was genuinely curious to see how straightforward / difficult it might be to replicate your output above, using CSS Grid.

.grid {
  display: grid;
  grid-template: repeat(4, 42px) / auto;
  grid-gap: 8px;
}

div p {
  margin: 0;
  padding: 0;
  line-height: 42px;
  text-align: center;
  font-family: sans-serif;
}

.box-1 {color: #fff; background-color: #4af;}
.box-2 {color: #fff; background-color: #fa4;}
.box-3 {color: #000; background-color: #def;}
.box-4 {color: #000; background-color: #fed;}

@media (min-width: 540px) {

  .grid {grid-template: repeat(2, 42px) / repeat(2, auto); grid-gap: 16px;}
}

@media (min-width: 800px) {

  .grid {grid-template: 42px / repeat(4, auto);}
}

col

col

col

col


Asynchronous file upload to disk I have a simple webserver written in Python using aiohttp. My goal is a server that can receive many file uploads simultaneously and stream them all to disk. This code works, but I'm not sure if it is efficiently handling simultaneous connections:

import pathlib
import click
from aiohttp import web
from src import settings
from src import logger

log = None

async def upload_handler(request):
    """
    POST handler that accepts uploads to any path.
    The files are accepted and saved under the root path
    of the server.
    """
    # You cannot rely on Content-Length if transfer is chunked.
    size = 0
    local_path = settings.WWW_ROOT + request.path
    path_without_file = local_path.rsplit('/', 1)[0]
    pathlib.Path(path_without_file).mkdir(parents=True, exist_ok=True)

    with open(local_path, 'wb') as f:
        while True:
            chunk, is_end_of_http_chunk = await request.content.readchunk()
            if not chunk:
                break
            size += len(chunk)
            f.write(chunk)

    return web.Response(text='%d bytes uploaded' % size, status=201)

@click.command()
@click.option('--port', type=int, default=8000)
@click.option('--log-level', type=click.Choice(['debug', 'info', 'warning', 'error', 'critical']), default='info')
def cli(port, log_level):
    global log
    log = logger.get_default_logger(name='', log_level=log_level)
    app = web.Application(client_max_size=2e9, debug=settings.DEBUG)
    app.router.add_post('/{tail:.*}', upload_handler)
    web.run_app(app, port=port)

if __name__ == '__main__':
    cli()

The aiohttp stuff feels a little magic to me. Is this running on parallel threads? I'm seeing some high CPU usage at times.

Are there performance issues here? I see performance issues on the server, but I'm not sure if they occur here or elsewhere in the stack.

Interesting application.

  Is this running on parallel threads?

Ummm, kind of. But in the kernel, not in python threads. Suppose you have four cores. Then what is good about the await ... readchunk() is that if in steady state it consumes 10% of a core, then python could get nearly 40 webserver threads to do useful work for that many clients before the onset of thrashing. (Webservers are open loop, rather than a closed loop queueing system, so you pretty much always need to have a little headroom available, a few idle cycles, to stay out of pathological performance regimes.)

    path_without_file = local_path.rsplit('/', 1)[0]

I'm sure this is right, but it's not very idiomatic. You don't like os.path.dirname() ?

    pathlib.Path(path_without_file).mkdir(parents=True, exist_ok=True)

Ok, that's a little crazy. Yes, I understand that nginx only hands you authenticated requests. But still. You just accepted arbitrary bytes under control of the web client and used it to mutate your local filesystem. I recommend at least verifying that folder or path_without_file conforms to some sane regex (no unicode combining marks). Same remarks, but more so, for the open(local_path, 'wb') fragment.

            f.write(chunk)

You didn't post any timing figures, so it's not clear if network input or disk output is the bottleneck. But if you want to go wild you might consider tracking current offset and doing an aio write at that offset, to obtain greater queue depth against your local RAID or other storage. When benching, be sure to try lots of slow clients and also try a couple of heavy hitters. You may find that the output of tcpdump -r trace -ttt is instructive, for revealing stalls.

Security remarks aside, kudos on some nice simple, solid code. Looks like it accomplishes what you set out to do.


Deleting values from std::vector of different types I need to make a vector of different types and be able to delete the values (please ignore anything else but deleting the values for now). Is the next code safe to delete the values?

#include <memory>
#include <vector>
#include <string>

using MyPtr = std::unique_ptr<void, void(*)(void*)>;

template<typename T>
MyPtr MakePtr(const T& val)
{
    return MyPtr(new T(val), [](void* ptr){delete static_cast<T*>(ptr);});
}

int main()
{
    std::vector<MyPtr> ptrs;

    ptrs.push_back(MakePtr(1));
    ptrs.push_back(MakePtr(1.1));
    ptrs.push_back(MakePtr(1.f));
    ptrs.push_back(MakePtr(std::string("1")));

    return 0;
}

Is the next code safe to delete the values?

No. At the very least, you should probably use reinterpret_cast instead of static_cast.Yes.

That said, it seems you are having the x-y problem. Storing data in C++, when you have heterogenous types in a sequence, should be done using a variant type, not void*; in other words, you should create a system that stores your type information per type/variable instance, instead of discarding all type information - like void* does). You are better off maintaining sepatate element lists (sparse arrays or some kind) by type, than you are with a vector<void*> (or vector<unique_ptr<void>> for that matter).

Have a look at boost::any and boost::variant before implementing your solution. You could use one of those, or a separate (similar) implementation.


I/O function takes far too long //parses the text path vector into the engine
void Level::PopulatePathVectors(string pathTable)
{
    // Read the file line by line.
    ifstream myFile(pathTable);

        for (unsigned int i = 0; i < nodes.size(); i++)
        {
            pathLookupVectors.push_back(vector<vector<int>>());

            for (unsigned int j = 0; j < nodes.size(); j++)
            {
                string line;

                if (getline(myFile, line)) //enter if a line is read successfully
                {
                    stringstream ss(line);
                    istream_iterator<int> begin(ss), end;
                    pathLookupVectors[i].push_back(vector<int>(begin, end));
                }
            }
        }
    myFile.close();
}

This function is taking about 5 minutes to pass in a text file which is 744 * 744 lines long. Each line is a list of integers like this:

1 4 6 24 7 4 8 n

they can be of varying length. I have no idea why it's taking so long, but it needs to run in a matter of seconds, not minutes! The odd thing is that the same function in C# (in fact a very unoptimised version) does in fact only take seconds to run.

Could the problem be a setting?

This is a link to the path table it is parsing http://dl.dropbox.com/u/13519335/WepTestLevelPathTable.txt

I setup nodes.size() to return 744.
This then forces all 553536 to be read from the file.

When I run your code it completes in 3.8 seconds (with -O3 it takes 2.5 seconds).

If I update the code to reserve the appropriate amount of space in each vector we can reduce the time it takes to: 3.3 seconds (with -O3 2.2 seconds).

So for the 5 minute figure you are quoting there must be some other error in your code.

Comments on your code:

Pass no-mutable parameters by cost reference (to avoid the copy).

void Level::PopulatePathVectors(string pathTable)

Add lines to reserve the appropriate space in each vector. This will prevent multiple re-allocations of the vector while you read. This is important when you have a triply nested vector.

    pathLookupVectors.push_back(vector<vector<int> >());
    pathLookupVectors.reserve(nodes.size());                // Add this line

    for (unsigned int i = 0; i < nodes.size(); i++)
    {
        pathLookupVectors.push_back(vector<vector<int>>());
        pathLookupVectors.back().reserve(nodes.size())      // Add this line

No Need to manually close the file.
see https://stackoverflow.com/q/748014/14065
Implementation using fstream failed evaluation

myFile.close();


Non-recursive approach to the Rod-Cutting I've given the Rod-Cutting Problem a go with a non-recursive approach that use a cache instead, with a bottom up approach. For anyone who hasn't heard about the Rod-Cutting problem, it's defined as follows:

  Serling Enterprises buys long steel rods and cuts them into shorter
  rods, which it then sells. Each cut is free. The management of Serling
  Enterprises wants to know the best way to cut up the rods. We assume
  that we know, for i = 1, 2, ..., the price pi in dollars that Serling
  Enterprises charges for a rod of length i inches. Rod lengths are
  always an integral number of inches. Given a rod of length n inches
  and a table of prices pi for i = 1, 2, ..., n determine the maximum
  revenue obtainable by cutting up the rod and selling the pieces. Note
  that if the price pn for a rod of length n is large enough, an optimal
  solution may require no cutting at all.

public static int[] prices = new int[]{1,5,8,9,10,17,17,20};

public static void main(String[] args) {

    // Best Price Check #1
    for (int i = 1; i < 9; i++) {
        System.out.println(i + " => " + bestPrice(i));
    }

}

public static int bestPrice(int n) {

    // This works like a cache
    int[] maxPrices = new int[n+1];

    // The trick here is to work from the bottom up
    for (int i = 1; i <= n; i++) {

        int max_price = prices[i-1]; // Without cuts

        // Try all cuts length with combo from existing cache
        for (int j = 1; j <  i; j++) {
            max_price = Math.max(max_price, maxPrices[i-j] + prices[j-1]);
        }
        maxPrices[i] = max_price;
    }

    return maxPrices[n];

}

The algorithm itself seems to have no problems. It's Dynamic programming and I wouldn't call the int[] maxPrices cache. Cache is something used for speed only, you can clear it anytime and everything must still work.

For example, a straightforward recursive solution to this problem would be damn slow because of recomputing values again and again. It could be made much faster using a cache.

 public static int[] prices = new int[]{1,5,8,9,10,17,17,20};

This way you lose any chance to make it more flexible. You class has two functions: It's both the computing core and the main class. That's fine for a small class, but even then you should separate the tasks.

For the main class, something like

private static final int[] PRICES = new int[] {1, 5, 8, 9, 10, 17, 17, 20};

Here, static final is the way how constants get declared in Java. PRICES is uppercase as a constant should be. And there's a space after each comma.

For the core, pass it explicitly to the bestPrice method. In more complicated cases, you could use an instance field for this.

max_price should be maxPrice in Java. No underscores except in constants.


vectorize 2D gradient with spatially varying bins The following code takes in some values ssh and solves the equations for the geostrophic motion (slide 8 here).

The main part of the code is the computation of the partial derivatives of ssh. In particular the discrete differentials have to be multiplied by a factor that depends on y. This can be easily done with a linear code:

close all
clearvars
clc

%define grid
x=linspace(120,145,20);
y=linspace(30,45,20);
[x, y]=meshgrid(x,y);
x = x';
y = y';

%define constants
R = 6371000; % Earth radius

g=9.806-.5*(9.832-9.780)*cos(2*y*pi/180); % gravity

omega = 2*pi/(24*60*60); % Earth rotation angle velocity [s]
f = 2*omega*sind(y); %Coriolis force coefficients

%data
ssh = (exp(-((x-130).^2/20)).*(exp(-(y-35).^2/7)))*1e6; % Sea surface height in each point

%Calculate geostrophic current
u=zeros(size(ssh));
v=zeros(size(ssh));
for i=2:size(x,1)-1
    for j=2:size(y,2)-1
        dx(i,j) = (x(i+1,j)-x(i-1,j)) *(R*cosd(y(i,j))*pi/180);
        dy(i,j) = (y(i,j+1)-y(i,j-1)) *(R*pi/180);

        u(i,j) = -g(i,j)/f(i,j) *(ssh(i,j+1)-ssh(i,j-1)) /dy(i,j);
        v(i,j) =  g(i,j)/f(i,j) *(ssh(i+1,j)-ssh(i-1,j)) /dx(i,j);
    end
end

figure
pcolor(x,y,ssh)
shading flat
hold on
quiver(x,y,u,v,2,'k')
title('Geostrophic current [m/s]','fontweight','bold')
xlabel('longitude','fontweight','bold')
ylabel('latitude','fontweight','bold')
set(gcf,'color','w')

Output:

However, I am having problems to vectorize the code.

I tried to use the gradient function in the following way:

%%%%===== vectorized code =====%%%%

dx2 = x  .*(R*cosd(y)*pi/180); %x-position matrix
dy2 = y  *(R*pi/180); %y-position matrix

[dsshdy,dsshdx] = gradient(ssh, dy2,dx2);

u2 = -g./f .*dsshdy;
v2 =  g./f .*dsshdx;

figure;hold on
pcolor(x,y, ssh)
shading flat
hold on
quiver(x,y, u2,v2, 2,'k')
title('Geostrophic current 2','fontweight','bold')
xlabel('longitude','fontweight','bold')
ylabel('latitude','fontweight','bold')
set(gcf,'color','w')

Output:

However, this fail (I think) because the gradient function does not take as inputs matrices of spacing values. As a consequence, the code somehow computes differentials that are way too big and the arrows are not visibile.

How can I vectorize such a problem without re-introducing a for loop to take into account the variation of dx with y?

Your f and ssh are already vectorized, you can do the same quite trivially with u and v also. There is nothing tricky going on in your loop. The process is simply to remove the for, leaving the assignment i=2:size(x,1)-1. And replace all matrix multiplication and division by element-wise multiplication and division (.* and ./). This leaves:

%Calculate geostrophic current
u = zeros(size(ssh));
v = zeros(size(ssh));
i = 2:size(x,1)-1;
j = 2:size(y,2)-1;
dx(i,j) = (x(i+1,j)-x(i-1,j)) .* (R*cosd(y(i,j))*pi/180);
dy(i,j) = (y(i,j+1)-y(i,j-1)) .* (R*pi/180);
u(i,j) = -g(i,j) ./ f(i,j) .* (ssh(i,j+1)-ssh(i,j-1)) ./ dy(i,j);
v(i,j) =  g(i,j) ./ f(i,j) .* (ssh(i+1,j)-ssh(i-1,j)) ./ dx(i,j);

You can then do a slight simplification, dx and dy do not need indexing, since you're using the same part that you assign:

dx = (x(i+1,j)-x(i-1,j)) .* (R*cosd(y(i,j))*pi/180);
dy = (y(i,j+1)-y(i,j-1)) .* (R*pi/180);
u(i,j) = -g(i,j) ./ f(i,j) .* (ssh(i,j+1)-ssh(i,j-1)) ./ dy;
v(i,j) =  g(i,j) ./ f(i,j) .* (ssh(i+1,j)-ssh(i-1,j)) ./ dx;


Reading integers while break condition not met in C++ This is a drill from the Programming Principles and Practice Using C++. I managed to get it to work but somehow it doesn't feel right to me. I 'd appreciate any pointers on how to simplify/improve the solution.
Especially when it comes to best check if '|' was entered. I feel that creating two extra string variables and then converting them to int using std::stoi is very inefficient. I am new to C++ so please excuse the ignorance. Appreciate any feedback.
Question

Write a program that in a while loop reads 2 integers and prints them. Carry on until the user enters '|'.

#include<iostream>
#include<string>

int main(){
    int a, b;
    std::string _a, _b;
    while (true){
        std::cout << "Enter two numbers." << std::endl;
        std::cin >> _a >> _b;
        if (_a == "|" || _b == "|"){break;};
        a = std::stoi(_a);
        b = std::stoi(_b);
        std::cout << "You entered " << a << " and " << b << std::endl;
    }
    return 0;
}

std::cout << "Enter two numbers." << std::endl;

You are looking to accept integers, not just numbers (i.e. \$1.0, 1e0, 0b1, 0x1\$).

Avoid std::endl.  std::endl inserts a newline character into an output sequence and flushes it as if by calling stream.put(stream.widen('\n')) followed by stream.flush().  If you just want to insert a newline character, then stream '\n'.

std::cout << "Enter two integers:\n"; // can append it on C strings

        std::cin >> _a >> _b;

Think about how you want to handle inputs.  You are expecting 2 integers, but what if they pass you one integer? No integers (or send end-of-file)? For those cases, you should check if the value was read successfull (see 10.10 in your book).

When a value cannot be extracted from the stream, the streamable types in C++ will typically set std::ios::failbit.  When std::string is the type being extracted, a failure to extract leaves the data in the string unchanged.  For other types, like the numeric types, the extraction operation will assign 0 on failure.

If the stream enters a fail state, you won't be able to extract from it until you clear() the error flag.  Not clearing the error flag would result in an infinite loop once the stream entered the error state.

        std::cin.clear();

What if the user enters more than 2 integers? Should you keep reading from the line? Doing this, you'll need to make sure both values are extracted, even if one failed.  Could you ignore() the remaining input until end of line? You were only expecting two integers, so this is an option.

        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

        a = std::stoi(_a);
        b = std::stoi(_b);
        std::cout << "You entered " << a << " and " << b << std::endl;

What happens if the extraction from std::cin fails? Either _a, _b, or both could be empty and std::stoi will throw std::invalid_argument when an empty string is encountered.

std::stoi will truncate at the decimal separator and return the number to that point.  If you want to validate that the string represents an integer, you need to make sure std::stoi reads the full buffer.

        std::size_t last_pos_read;
        a = std::stoi(_a, std::address_of(last_pos_read));
        if (last_pos_read != _a.length()) {
            throw std::invalid_argument("Not an integer.");
        }

Inexperience aside, the big lesson you should take from this review is to test! Test inputs of type integer (negatives, zero, positives, 16/32/64bit values), floating point (representable/nonrepresentable values), string/containers (empty/nonempty, small/large sizes). Test failures at different times, like an immediate failure or a failure after a successful loop.


Prime Number Sieve Optimization I've written my own prime number sieve in JavaScript. It loops through numbers 1-100 and puts them in one of two arrays, depending on if it is composite or prime.

I was wondering if there are any inefficient or unnecessary parts of my code that might be slowing down the program. Is there anything I can do so it will work for larger numbers too?

//  Creating variables to use later
var cORp;
var primeNums = [];
var compNums = [];

// Create numberSieve function
function numberSieve(x) {

    // 0 and 1 are not prime or composite
    if (x < 2 && x >= 0) {
        cORp = 'neither';
    }

    // All negative numbers are composite
    else if (x < 0) {
        cORp = 'composite';
    }

    // Sieve for positive numbers above 1
    else {

        // Some more variables for later use
        possibleFactors = [];
        zFactors = [];
        matchingFactors = [];

        // Takes all numbers that are half of x and below because they are possible factors
        for (i=2; i < x/2+1; i++) {
            possibleFactors.push(i);
        }

        // Takes all possible factors and divides x by them
        for (i=0; i < possibleFactors.length; i++) {
            z = x/possibleFactors[i];
            zFactors.push(z);
        }

        // Checks if the x divided by the possible factors is equal to any of the other possible factors
        for (i in possibleFactors) {
            if (zFactors.indexOf(possibleFactors[i]) !== -1) {
                matchingFactors.push(possibleFactors[i]);
            }
        }

        // Checks if there are any matching factors
        if (matchingFactors.length !== 0) {
            cORp = 'composite';
        }
        else {
            cORp = 'prime';
        }
    }

    // Prime numbers go to one array, Composite numbers go to another
    if (cORp === 'prime') {
        primeNums.push(x);
    }
    else if (cORp === 'composite') {
        compNums.push(x);
    }
}

// For each number 0-100 runs the sieve
a=0;
while (a <= 100) {
    numberSieve(a);
    a++;
}

// Prints out the prime number and composite number arrays
console.log("Prime Numbers: " + primeNums);
console.log("Composite Numbers: " + compNums);

Style

The while loop definitly looks like a for loop to me.

Clearer function

Your function numberSieve does not many things. It would be simpler if it was to simply return a value and this value was used afterward to fill arrays.

If we are doing so, we could take this chance to make clear what it returns : it could return whether x is prime or not. By renaming it isPrime, things would be more straightforward.

Also, if you do so, you realise that you can start your loop from 2 if you want to get the same output as you had before.

Here is the corresponding code :

// Create isPrime function
function isPrime(x) {

    // 0 and 1 are not prime
    if (x < 2) {
        return false;
    }
    // Sieve for positive numbers above 1
    else {
        // Some more variables for later use
        possibleFactors = [];
        zFactors = [];
        matchingFactors = [];

        // Takes all numbers that are half of x and below because they are possible factors
        for (i=2; i < x/2+1; i++) {
            possibleFactors.push(i);
        }

        // Takes all possible factors and divides x by them
        for (i=0; i < possibleFactors.length; i++) {
            z = x/possibleFactors[i];
            zFactors.push(z);
        }

        // Checks if the x divided by the possible factors is equal to any of the other possible factors
        for (i in possibleFactors) {
            if (zFactors.indexOf(possibleFactors[i]) !== -1) {
                matchingFactors.push(possibleFactors[i]);
            }
        }

        // Checks if there are any matching factors
        return (matchingFactors.length == 0);
    }
}

var primeNums = [];
var compNums = [];

// For each number 0-100 runs the sieve
for (a = 2; a<= 100; a++) {
    if (isPrime(a))
        primeNums.push(a);
    else
        compNums.push(a);
}

// Prints out the prime number and composite number arrays
print("Prime Numbers: " + primeNums);
print("Composite Numbers: " + compNums);

Clearer algorithm

You don't need to handle arrays to know whether a number divides or not anyther number. You can just use the modulo operator.

function isPrime(x) {

    // 0 and 1 are not prime
    if (x < 2) {
        return false;
    }
    // Sieve for positive numbers above 1
    else {
        // Takes all numbers that are half of x and below because they are possible factors
        for (i=2; i < x/2+1; i++) {
            if (x % i == 0)
                return false;
        }
        return true;
    }
}

A smarter algorithm

At the moment, you look for divisors going up to x/2 + 1. Actually, if there are any divisors, there will be a least one divisor d such that $$ d \le sqrt(x) $$

Therefore, instead of having a algorithm with completity O(N), we have O(sqrt(N)) which is much faster for big values of N.

   // Takes all numbers that are half of x and below because they are possible factors
    for (i=2; i <= Math.sqrt(x); i++) {
        if (x % i == 0)
            return false;
    }
    return true;

A more appropriate algorithm

If you want to generate a list of primes (and non-primes) in an efficient way, you should have a look at the Sieve of Eratosthenes.


Searching extreme points of polyhedron In my Uni, my scientific professor asked me to make some researches about the extreme points of polyhedrals. And I did them. I found that there is still no code in public for searching extreme points for polyhedral with n dimensions (n - x's), but polyhedrons are everywhere (CV, game theories, etc.). I wrote a function for this task and made a python library (also there are matrix and array combination maker functions).

All I want is to make this code more optimal and compatible for all python versions (I have some troubles that some times happen when I install it by "pip install lin", but other times no). I want to make the life of people easier and make it more comfortable.

I am asking for you to test this function on your computer and write if you have any bugs, fails or thoughts on how to make it better. I am open to constructive criticism and non-constructive too (it will help to understand if somebody needs it or that is just a waste of time).

All the examples, instructions and code on my GitHub: https://github.com/r4ndompuff/polyhedral_set

import numpy as np
import itertools as it
import math
import re

def permutation(m,n):
    return math.factorial(n)/(math.factorial(n-m)*math.factorial(m))

def matrix_combinations(matr,n):
    timed = list(map(list, it.combinations(matr, n)))
    for i in range(n):
        timed[i][i][i] = np.asscalar(timed[i][i][i])
    all = np.array(list(timed))
    return all

def array_combinations(arr,n):
    timed = list(map(list, it.combinations(arr, n)))
    for i in range(n):
        timed[i][i] = np.asscalar(timed[i][i])
    all = np.array(list(timed))
    return all

def check_extreme(matr, arr, x, sym_comb, m):
    sym_comb = sym_comb.replace(']', '')
    sym_comb = sym_comb.replace('[', '')
    sym_comb = re.split("[ ,]", sym_comb)
    for i in range(m):
        td_answer = sum(matr[i]*x)
        if sym_comb[i] == '>':
            if td_answer <= arr[i]:
                return 0
        elif sym_comb[i] == '>=':
            if td_answer < arr[i]:
                return 0
        elif sym_comb[i] == '<':
            if td_answer >= arr[i]:
                return 0
        elif sym_comb[i] == '<=':
            if td_answer > arr[i]:
                return 0
        elif sym_comb[i] == '=':
            if td_answer != arr[i]:
                return 0
        elif sym_comb[i] == '!=':
            if td_answer == arr[i]:
                return 0
        else:
            return 0
    return 1

def extreme_points(m,n,A,b,sym_comb):
    # Input
    A = np.array(A).reshape(m,n)
    b = np.array(b).reshape(m,1)
    # Proccess
    ans_comb = np.zeros((1,n))
    arr_comb = array_combinations(b,n)
    matr_comb = matrix_combinations(A,n)
    for i in range(int(permutation(n,m))):
        if np.linalg.det(matr_comb[i]) != 0:
            x = np.linalg.solve(matr_comb[i],arr_comb[i])
            ans_comb = np.vstack([ans_comb,x])
    ans_comb = np.delete(ans_comb, (0), axis=0)
    j = 0
    for i in range(len(ans_comb)):
        if check_extreme(A, b, ans_comb[j], sym_comb, m):
            ans_comb = ans_comb
            j = j + 1
        else:
            ans_comb = np.delete(ans_comb, (j), axis=0)
    # Output
    return ans_comb

And I am uploading some more tests. https://imgur.com/mjweDyy

I created a rudimentary pull request to your GitHub repo. I won't show all of the content here except for the main file:

import numpy as np
import itertools as it
from math import factorial
import re

def permutation(m, n):
    return factorial(n) / (factorial(n - m) * factorial(m))

def matrix_combinations(matr, n):
    timed = list(map(list, it.combinations(matr, n)))
    return np.array(list(timed))

def array_combinations(arr, n):
    timed = list(map(list, it.combinations(arr, n)))
    return np.array(list(timed))

def check_extreme(matr, arr, x, sym_comb, m):
    sym_comb = sym_comb.replace(']', '')
    sym_comb = sym_comb.replace('[', '')
    sym_comb = re.split("[ ,]", sym_comb)
    for i in range(int(m)):
        td_answer = sum(matr[i] * x)
        if sym_comb[i] == '>':
            if td_answer <= arr[i]:
                return 0
        elif sym_comb[i] == '>=':
            if td_answer < arr[i]:
                return 0
        elif sym_comb[i] == '<':
            if td_answer >= arr[i]:
                return 0
        elif sym_comb[i] == '<=':
            if td_answer > arr[i]:
                return 0
        elif sym_comb[i] == '=':
            if td_answer != arr[i]:
                return 0
        elif sym_comb[i] == '!=':
            if td_answer == arr[i]:
                return 0
        else:
            return 0
    return 1

def extreme_points(A, b, sym_comb):
    # Input
    A = np.array(A)
    b = np.array(b)
    m, n = A.shape
    # Process
    ans_comb = np.zeros((1, n))
    arr_comb = array_combinations(b, n)
    matr_comb = matrix_combinations(A, n)
    for i in range(int(permutation(n, m))):
        if np.linalg.det(matr_comb[i]) != 0:
            x = np.linalg.solve(np.array(matr_comb[i], dtype='float'),
                                np.array(arr_comb[i], dtype='float'))
            ans_comb = np.vstack([ans_comb, x])
    ans_comb = np.delete(ans_comb, 0, axis=0)
    j = 0
    for i in range(len(ans_comb)):
        if check_extreme(A, b, ans_comb[j], sym_comb, m):
            ans_comb = ans_comb
            j += 1
        else:
            ans_comb = np.delete(ans_comb, j, axis=0)
    # Output
    return ans_comb

Notable changes:

Do a direct import of factorial
Don't call asscalar, since it's both unneeded and deprecated
Don't call a variable all, since that shadows a Python built-in
Don't need to explicitly pass array dimensions, nor do you need to reshape the arrays
Drop redundant parens around some expressions
Use += where applicable
Fix up almost all PEP8 issues, except for your capital letter A, which is fine in context

This doesn't solve the bigger issue that you should replace 99% of this with a call to scipy. I'll do that separately (I suspect that @vnp is, as well).


Increase efficiency of card-ordering program As mentioned in @rolfl comment. What this program does is that it "resequences the cards using a fixed system and count the number of times the resequencing happens before the output is the same as the input."

I was wondering how I could make this program faster or perhaps use less memory.  I thought about using dictionaries instead of lists as they have faster look ups. I'm not sure how to decrease the memory size of this program.

num_cards = input("Please enter the number of cards in the deck: ")

def num_of_round(num_cards):
    rounds = 0
    orig_order = range(1,num_cards + 1)
    card_list  = list(orig_order)
    table_list = []

    while 1:
        while len(card_list) > 0:
            table_list.append(card_list[0])     # STEP 1:   add top card to table
            card_list.remove(card_list[0])      #           remove top card from deck
            if len(card_list) == 0:
                break
            card_list.append(card_list[0])                  # STEP 2:   after removal of card, put top card on bottom of deck
            card_list.remove(card_list[0])                  #           update ordering of deck
        rounds += 1                                         # a round has passed
        table_list.reverse()
        card_list, table_list = table_list, card_list
        if card_list == orig_order:
            break
    print rounds

num_of_round(num_cards)

Here is a good opportunity to break out a couple of tools from the standard library.

collections.deque offers fast appends and pops on both ends and is thus perfect to represent your deck of cards, when you take cards from the top and put some back to the bottom.

itertools.cycle can be used to switch between the two destinations of the cards: the top of the pile (table_list.appendleft) and the bottom of the deck (card_list.append). Using appendleft there avoids the need to reverse the pile each round.

Also, a while True loop with a round += 1 inside could be a for round ... loop instead.

from collections import deque
from itertools import cycle, count

def num_of_round(num_cards):
    orig_order = deque(xrange(num_cards))
    card_list  = deque(orig_order)
    table_list = deque()

    for rounds in count(1):
        appendfunc = cycle((table_list.appendleft, card_list.append))
        while card_list:
            append = next(appendfunc)
            append(card_list.popleft())
        card_list, table_list = table_list, card_list
        if card_list == orig_order:
            return rounds


Productivity app for Android I wrote a productivity app for Android. It lets you switch system settings, like Bluetooth, wife, screen brightness, volumes, ringtones, mobile data, airplane mode, etc. Unfortunately I have discovered that device manufacturers modify the OS in every possible way, disabling functionality, requesting additional permissions, or making it available only to system apps. I.e. switching WiFi on stock Android requires android.permission.CHANGE_WIFI_STATE permission, but most Samsung devices also require android.permission.ACCESS_WIFI_STATE. Switching mobile data is simply disabled on many devices.

This makes it impossible to predict how my code will work on a particular device, or on a future version of OS. But at the same time, I definitely don't want my app to crash, because there's really nothing worse that an app can do. So in order to be safe, I've been using what is sometimes called a "Pokemon exception handling" (Gotta catch 'em all!):

try {
    //Do stuff
} catch (Exception e) {
    //Log an error, send crash report, etc.
}
//Keep calm and carry on

I know that this is generally considered a bad practice, but I just can't find a better way to handle this situation. I simply can't predict all possibilities of what could go wrong. Most of my methods for executing actions look something like this:

private boolean switchWiFi(boolean state) {
    try {
        WifiManager wm = (WifiManager)getSystemService(WIFI_SERVICE);
        wm.setWifiEnabled(state);
        return true; //Executed successfully
    }
    catch (Exception e){
        //Give user a notification about an error, rather than crash
        showErrorNotification("Couldn't switch WiFi.");
        if(BuildConfig.DEBUG){Log.getStackTraceString(e);}//Log exception stack trace
        return false; //Execution failed
    }
}

Is there a better way to handle exceptions?

When the called API throws unknown exceptions I don't think that there is other way than catching all exceptions (sometimes Throwables too). +1 Jeroen Vannevel's comment too.

Anyway, a common interface might be able to improve the code a little bit:

public interface Command {

    run() throws Exception;

    String getName();

}

Then, you can have implementations, like WifiSwitcher, and call them one after the other:

final List<String> errors = new ArrayList<String>();
for (final Command command: commands) {
    try {
        command.run();
    } catch (final Exception e) {
        errors.add("Could not perform command: " + command.getName());
        if (BuildConfig.DEBUG) {
            Log.getStackTraceString(e);
        }
    }
}

if (errors.isEmpty()) {
    return true;
}
showErrorNotification(errors);
return false;

(I haven't tested nor compiled the code above.)


Finding Shortest Path Working on Functionally Solving Problems from Learn You a Haskell, I worked on the "Heahtrow to London" shortest path problem.

I'm working on a reduced set of this problem in order to solve a simple problem first.

It's a shortest path problem for the following map:

The rule is: you start a A or B, and then choose across the bridge or up/down. When you reach E or F (doesn't matter), you're done.

I used a Node data structure (compliments of LYAH's previous chapter) to recurse through the shorter left or right sides. Ultimately, I return a tuple of (collective cost, list of nodes traversed).

Please critique my implementation:

data Tree a = EmptyTree | Node a (Tree a) (Tree a) deriving (Show, Read, Eq)

shortestPath2 :: Tree (Int, Char) -> (Int, [Char])
shortestPath2 EmptyTree = (0, [])
shortestPath2 (Node (c, pt) EmptyTree EmptyTree) = (c + fst (shortestPath2 EmptyTree), pt : snd (shortestPath2 EmptyTree))
shortestPath2 (Node (c, pt) EmptyTree right)     = (c + fst (shortestPath2 right), pt : snd (shortestPath2 right))
shortestPath2 (Node (c, pt) left EmptyTree)      = (c + fst (shortestPath2 left), pt : snd (shortestPath2 left))
shortestPath2 (Node (c, pt) left@(Node (lcost, _) _ _) right@(Node (rcost, _) _ _)) =
                                 (c + fst (shortestPath2 next), pt : snd (shortestPath2 next))
                                       where next = if lcost < rcost then left else right

test3 :: (Int, String)
test3 = result
        where path1 = (Node (50, 'A') (Node (30, 'C') (Node (90, 'D') (Node (0, 'E') EmptyTree EmptyTree) EmptyTree) EmptyTree)
                                     (Node (5, 'C') (Node (0, 'E') EmptyTree EmptyTree) EmptyTree))

              path2 = (Node (10, 'B') (Node (30, 'D') (Node (5, 'C') (Node (0, 'E') EmptyTree EmptyTree) EmptyTree) EmptyTree)
                                      (Node (90, 'D') (Node (0, 'F') EmptyTree EmptyTree) EmptyTree))

              sp1 = shortestPath2 path1
              sp2 = shortestPath2 path2
              compare first@(cost1, _) second@(cost2, _) = if cost1 < cost2 then first else second
              result = compare sp1 sp2

Output

ghci> test3
(45,"BDCE")

Data representation

My critique starts here:

data Tree a = EmptyTree | Node a (Tree a) (Tree a)

Why is a node forced to have 2 sub-trees? Given a graph like this:

    B1 - C1
     |    |
A - B2 - C2
     |    |
    B3 - C3

you cannot represent B2 properly. I'd use

data Tree a = EmptyTree | Node a [Tree a]

for an arbitrary number of subtrees. Even the EmptyTree constructor might turn out to be unnecessary.

Try to rewrite your code with that, and see that the code duplication in the shortestPath2 definitions is reduced.

Use let and pattern matching

In an expression like

(c + fst (shortestPath2 right), pt : snd (shortestPath2 right))

you calculate shortestPath2 right twice and calls to fst and snd can often be avoided with pattern matching.

let (cs,pts) = shortestPath2 right in (c + cs, pt : pts)

Layout

path1 = (Node (50, 'A') (Node (30, 'C') (Node (90, 'D') (Node (0, 'E') EmptyTree EmptyTree) EmptyTree) EmptyTree)
                                       (Node (5, 'C') (Node (0, 'E') EmptyTree EmptyTree) EmptyTree))

is very hard to scan this by the eye. Only whitespace change, and it looks like this:

    where path1 = (Node (50, 'A')
                      (Node (30, 'C')
                          (Node (90, 'D')
                              (Node ( 0, 'E') EmptyTree EmptyTree)
                               EmptyTree)
                           EmptyTree)
                      (Node (5, 'C') -- should be 'D'
                              (Node (0, 'E') EmptyTree EmptyTree)
                               EmptyTree))

You actually have 'C' twice as successor to 'A'.
Change path1 to a top-level definition to access it in ghci. Then type let (Node _ a b) = path1 and then a and b into ghci. The latter should be 'D', just a typo.
The data representation is confusing me! The data is a graph, and graphs distinguish between edges and nodes. A,... are nodes, the numbers in between are edges. I would not group them like (50, 'A'). I hope someone else can give you a good pointer here.

BTW path1 is a misnomer. It is not a path. "BDCE" might be a path. path1 is a graph rolled out to a tree (without loops)?

Algorithmic

  I used a Node data structure (compliments of LYAH's previous chapter) to recurse through the shorter left or right sides.

You are technically right. It only scans then next section of the path and uses the shorter one. Your algorithm is greedy and does not find the global optimum. Change the 90 into 31, the shortest path should be "BDF", but your output of test3 does not change.

In General

At first I was confused, because I'd expect a function like

`shortestPath from to path1`

where from and to are origin and destination nodes. In your code, the destination is not coded at all. The problem states get from A or B to E or F, but single nodes are enough:

You can insert a virtual node

`london = Node ('V',0) path1 path2`

and you test function looks like

test4 = shortestPath2 london where
        --path1 = -- see above
        --path2 = -- see above
        london = Node (0,'V') path1 path2

Which looks better than test3.

Conclusion

I will address the biggest problem - that you algorithm does not find the optimum - in a later edit (or leave it for someone else to answer).


Bubble Sorting Algorithm I made a simple Python program which takes a list of unique numbers and orders them from smallest to greatest. The algorithm (which is actually quite similar to bubble sort) works like this:

Take the first number

If it is larger than the next number, switch them
If it is smaller than the next number, do nothing

Continue to next number until the algorithm has done this to everything in the list
Check if the list is in the correct order

If not, repeat the process
If it is, print out the list in order and time taken in seconds

import random
import time
import sys

sys.setrecursionlimit(10000)
value_list = []
# Create values
len_of_list = random.randint(8500, 10000)
for number in range(0, len_of_list):
    value_list.append(number)
random.shuffle(value_list)

def timeit(func):
    def wrapper(*args, **kwargs):
        start = time.time()
        result = func(*args, **kwargs)
        print "The function {.__name__} took {:.15f} seconds to finish.".format(func, time.time() - start)
        return result
    return wrapper

def custom_sort():
    for index in range(0, len_of_list):
        if (index + 1) == len_of_list:
            if not check_results():
                custom_sort()
            elif check_results():
                print value_list
        else:
            if value_list[index] > value_list[index + 1]:
                smaller_val = value_list[index + 1]
                value_list[index + 1] = value_list[index]
                value_list[index] = smaller_val
            elif value_list[index] < value_list[index + 1]:
                continue

def check_results():
    for index in range(0, len_of_list):
        if (index + 1) == len_of_list:
            return True
        else:
            if value_list[index] < value_list[index + 1]:
                continue
            elif value_list[index] > value_list[index + 1]:
                return False

@timeit
def run():
    custom_sort()

run()

I have made a function called run() specifically so that @timeit only prints out one line stating how long it took to finish. custom_sort() sorts the numbers in the list for the first four bullets. check_results() checks if the list is in order like in the last three bullets. sys.setrecursionlimit(10000) is used so that I don't get a recursion error since the list can hold up to 10000 numbers.

Example output (due to reports of crashing, I will replace most of the values with ... , I hope you understand what the result should be):

[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, ... 9669, 9670, 9671, 9672, 9673, 9674, 9675, 9676, 9677, 9678, 9679, 9680]
The function run took 24.037000179290771 seconds to finish.

Is there a way to shorten my code and make my algorithm faster? The slowest it has been is about 36 seconds to sort the maximum amount of numbers (10000).

There are several quick improvements that can halve the time.

There is a redundant check where you first check a > b and then check b < a. Ignoring that the "continue" statement makes any check redundant  since if it doesn't match you'll fall past and continue anyway, it's also redundant because you're checking for the negative of a previous check. Well, an almost negative, you don't explicitly deal with a == b but I'm guessing you want to continue there not swap them, both are a valid result but swapping identical values is a waste.

Also a bunch of the time is taken filling the array. Rather than looping over a long array and calling append on each item, you can just set

value_list = range(len_of_list)

The other major optimisation is to not use recursion, the reason this improves performance here is because your comparison for finishing is a long operation that you have inside the loop, by making the function looping / iterative it is easy to move that outside the inner loop saving a large amount of time.

Also you have some funny logic around the last element, just looping (0, n-1) fixes that.

The last element MUST be in order after the first iteration, it gets pushed back every single time, so instead of looping (0, n - 1) you can actually loop (0, n - i - 1) where i is the iteration number.

You can remove the check_results call entirely and consider what it means to be "in order". Being in order means that no swaps have happened. That means that instead of looping over everything you can just track whether you've swapped anything. If you haven't then congratulations, everything is in order.

This saves a large amount of time since your "check_results" loops over the array doing comparisons which you were doing inside another loop (well, recursed call).

Below is my modification of your code to include the changes I have described. I have changed some variable names to more clearly show their new meaning.

import random
import time
import sys

value_list = []
# Create values
len_of_list = random.randint(8500, 10000)
value_list = range(len_of_list)
random.shuffle(value_list)

def timeit(func):
    def wrapper(*args, **kwargs):
        start = time.time()
        result = func(*args, **kwargs)
        print "The function {.__name__} took {:.15f} seconds to finish.".format(func, time.time() - start)
        return result
    return wrapper

def custom_sort():
    has_swapped = True
    iteration = 0
    while has_swapped:
        has_swapped = False
        for index in range(len_of_list - iteration - 1):
                next_value = value_list[index + 1]
                if value_list[index] > next_value:
                    has_swapped = True
                    value_list[index + 1] = value_list[index]
                    value_list[index] = next_value
        iteration += 1
        if not has_swapped:
           print value_list

@timeit
def run():
    custom_sort()

run()

For reference, your code on my system executed in around 25-28 seconds depending on the random array length, my modified version executes in 12-14 seconds. I am sure there are further optimisations I have missed.


Group inner properties from an array of objects to a map I have an array of object and I want to group/order some inner properties in a map according to a key.
I did this code but I would like to know if there was a way to optimize the code ?

const myArray = [{
    getValue: () => {
      return {
        country: 'France',
        city: 'Toulouse',
      };
    },
  },
  {
    getValue: () => {
      return {
        country: 'England',
        city: 'London',
      };
    },
  },
  {
    getValue: () => {
      return {
        country: 'France',
        city: 'Paris',
      };
    },
  },
];
const myMap = new Map();
myArray.forEach(item => {
  const key = item.getValue().country;
  const value = item.getValue().city;
  if (myMap.has(key)) {
    myMap.get(key).push(value);
  } else {
    myMap.set(key, [value]);
  }
});

for (const [key, value] of myMap) {
  console.log(`${key} = ${value}`);
}

Too complex
Don't make code more complicated than needed.
The getValue function for each data item is entirely unnecessary. You can move the function out and reduce the data input complexity a great deal. See 1st rewrite.
For each data item property you call getValue (once for country and once for city) which creates a new object each time it is called. This is a huge overhead, the items should be created only once (When code is parsed) and to process you should not need to make copies just to read properties.
If you need the data items to be functions, don't write the function for each item, write the function once and return it using a second function. See 2nd rewrite.
Template_literals can sometimes add a lot of noise to the construction of a string. Maybe ${key} = ${value} can be better as key + " = " + value.  Template literals do not make for better code just by using them.
Names
The naming is very poor. You should never get lazy with naming, even if its example code.

key and value should be country and city
myMap Yes we know its a map, but what does it represent. citesByCountry

Code should never stand alone. Always write code as a named function, even when its just example code. See rewrite.
General points
Always code in Strict_mode.
Prefer for...of loops over Array iterators when possible.
You do two map look-ups with Map.has and Map.get if an index exists. You can avoid one of the look-ups and use only Map.get. Check the result to determine if the map has the index you are looking for. See rewrite.
Learn to use the Conditional Operator ?, Nullish coalescing operator ?? and / or Optional chaining ?. to simplify statement combinations.
Rewrites
First without the overhead of the getValue call per item.
"use strict";
function dataTest() {
    const addCountry = (country, city) => ({country, city});
    const data = [
        addCountry("France", "Toulouse"),
        addCountry("England", "London"),
        addCountry("France", "Paris"),
    ];
    function joinByCountry(data) {
        const countries = new Map();
        for (const {country, city} of data) {
            countries.get(country)?.push(city) ?? countries.set(country, [city]);
        }
        return countries;
    }

    for (const [country, cites] of joinByCountry(data)) {
         console.log(country + " = " + cites);
    }
}

If getValue is required then write the getValue function once rather than for each item.
This rewrite uses Conditional Operator ? to replace the if else statements.
"use strict";
function dataTest() {
    const addCountry = (country) => ({ getValue() { return country} });
    const data = [
        addCountry({country: "France", city: "Toulouse"}),
        addCountry({country: "England", city: "London"}),
        addCountry({country: "France", city: "Paris"}),
    ];
    function joinByCountry(data) {
        const countries = new Map();
        for (const location of data) {
            const {country, city} = location.getValue();
            const cites = countries.get(country);
            cites ? cites.push(city) : countries.set(country, [city]);
        }
        return countries;
    }

    for (const [country, cites] of joinByCountry(data)) {
         console.log(country + " = " + cites);
    }
}


PHP : Custom trim function instead existing ltrim() or rtrim() I needed to create a custom trim() function instead of using existing ltrim or rtrim due to below problems.

Here are problem with existing functions.

$string = '

This

';

var_dump(ltrim($string, '
'));

// Output :: string(21) "ul>
This

"

Fiddle.

As you can see ltrim trimming < brace because
 not found I think (Not sure 100%).

Here created function,

function trimString($string, $replaceWith = ' ', $type = 'ltrim')
{
    $type = (!empty($type) && in_array($type, ['ltrim', 'rtrim'])) ? $type : 'ltrim';
    if (!empty($string) && !empty($replaceWith)) {
        // Like ltrim
        if ($type == 'ltrim') {
            if (strpos($string, $replaceWith) === 0) {
                $string = substr_replace($string, '', 0, strlen($replaceWith));
            }
        } elseif ($type == 'rtrim') {
            // Like rtrim
            $reverceString    = strrev($string);
            $replaceWithCount = strlen($replaceWith);
            $count            = 1;
            for ($i = 0; $i < strlen($string); $i++){
                if ($count <= $replaceWithCount) {
                    $newString .= $reverceString[$i];
                }
                $count++;

                if ($count == $replaceWith) {
                    break;
                }
            }
            if (!empty($newString)) {
                if (strrev($newString) == $replaceWith) {
                    $string = strrev(strReplaceFirst($reverceString, strrev($replaceWith), ''));
                }
            }
        }
    }

    return $string;
}

function strReplaceFirst($content, $from, $to)
{
    if (!empty($content) && !empty($from)) {
        $from = '/'.preg_quote($from, '/').'/';

        return preg_replace($from, $to, $content, 1);
    }

    return $content;
}

Fiddle.

So, it is working properly for me! But I still don't know if it's the proper way to do that or not? Or it's side effect and time consumption.

I wonder if you did a quick Google search before rolling your own solution. Despite being implemented via different languages, they all give the same general regex solution.

Regex to remove all
 tags from beginning and end of a string in javascript
Remove
 tags from the beginning and end of a string
Remove BR tag from the beginning and end of a string

As you have discovered, trim() functions treat the 2nd parameter as a "character mask", this means that it will greedily remove any consecutive characters from the beginning/end of the string from that "list of characters" (
 is the same as rb><).

If you need a utility function and require the same passed-in parameters:

function trimString($input, $find, $replacement = ' ', $type = 'ltrim') {
    $patterns = [
        'ltrim' => '^(?:' . preg_quote($find, '/') . ')+',
        'rtrim' => '(?:' . preg_quote($find, '/') . ')+$'
    ];
    $pattern = $patterns[$type] ?? implode('|', $patterns);
    return preg_replace('/' . $pattern . '/i', $replacement, $input);
}

If you want to be able to pass replacement strings which contain regex to the custom function like
, then you can remove the preg_quote() calls entirely or set a flag/argument in the custim function which determines whether the quoting call should be implemented.  This means that the responsiblity of quoting any special characters is transferred from the custom function to whatever layer is calling the custom function.

If you want to use trim() before or after the execution of this custom function, that makes perfect sense too -- removing whitespace characters is exactly what it is mean to do.

If your requirements keep extending and you have the intention of removing HTML tags from a valid HTML document, then regex is probably not the right path to go down.  Regex is DOM-unaware and will be vulnerable to breakage in fringe case scenarios -- a legitimate DOM parser would be my strong recommendation in that case.


Building an equation I've developed a function in the Julia language, using the SymEngine library, which builds an equation. This function works quite quickly, returning a lambdified equation.

The problem is, calculation of the value of this equation in the point takes too long time! And as the equation becomes longer, the calculation time also increases.

Is there a way to optimize the equation, generated by the function?

function func()

  @vars x y z myu

  longitude=atan(y/x)+pi*sign(y)*(1-sign(x))/2
  r=(x^2+y^2+z^2)^(1/2)

  dUx=0
  dUy=0
  dUz=0
  for i=2:degree
      for j=0:degree

          index=1+j; for ll=2:i-1 index+=ll+1; end

          P_i=(myu^2-1)^i
          for k=1:i+j P_i=diff(P_i,myu) end

          if(i>20) F=factorial(Int128(i)); else F=factorial(i); end
          P_ij=(((1-myu^2)^(j/2))/(F*2^i))*P_i

          if(P_ij!=0)
            CS_exp=CS[index,3]*cos(j*longitude)+CS[index,4]*sin(j*longitude)
            CS_diff_x= j*CS[index,4]*cos(j*longitude)*(-y/(x^2+y^2)) - j*CS[index,3]*sin(j*longitude)*(-y/(x^2+y^2))
            CS_diff_y= j*CS[index,4]*cos(j*longitude)*( x/(x^2+y^2)) - j*CS[index,3]*sin(j*longitude)*( x/(x^2+y^2))
            CS_diff_z= 0

            L=P_ij(z/r)

            dUx+= GMe*(diff((L*(Req/r)^i)/r,x)*CS_exp + CS_diff_x*(L*(Req/r)^i)/r)
            dUy+= GMe*(diff((L*(Req/r)^i)/r,y)*CS_exp + CS_diff_y*(L*(Req/r)^i)/r)
            dUz+= GMe*(diff((L*(Req/r)^i)/r,z)*CS_exp + CS_diff_z*(L*(Req/r)^i)/r)

          end
       end
  end

  return lambdify(dUx, [x,y,z], cse=true),lambdify(dUy, [x,y,z], cse=true),lambdify(dUz, [x,y,z], cse=true)

end

dU=func();

dy= [
       dU[1](y[1],y[2],y[3]),
       dU[2](y[1],y[2],y[3]),
       dU[3](y[1],y[2],y[3])
    ]

The calculation of dy takes too long. Any ideas?

I've also commented on the julia slack, but just if anyone else is reading, as I see you've not updated the question according to my comments.

You can see Julia's general performance tips here. I suggest you read this. You are falling into at least two of the performance gotchas:

You are initializing dUx, dUy and dUz as Ints first, but change their type to Float64s in the first assignment. That trips the compiler. The solution is to initialize them to 0.
You are using degree and CS that appear to be captured global variables, which makes it impossible for the compiler to specialize. The solution is to pass them as arguments to the function.

Not that the second issue also means that your code is not runnable, because we don't have access to degree and CS. If your code is not runnable it's very hard to comment on.

Here's what you can do yourself to fix this:

Use the Traceur.jl package to check your code, using @trace func(). That will check your code for the common performance gotchas.
Use the BenchmarkTools package (with @btime) to evaluate any improvements.
Once you've done that, use the Profile module (or even better, Juno's inbuilt profiler to see which lines in your code are slow.

Once you've done that, if you still have problems, please comment here with the results from the profiler, and update your code to be runnable, and we'll be happy to take a look again.


Efficent way to Query a DB and write to text file - Large Recordset I have a SQL query that returns a recordset with between 2 and 5 million records and I need to write that to a .csv file.

I wrote the following procedure and I'm curious to see if there is a better / more efficient way to do this.

Dim conn As New OracleConnection()
...
Sub DBExecuteQueryWriteToFile(ByVal SQLCommand As String, ByVal FileName As String, Optional ByVal Delimiter As String = ",")

    Dim cmd = New OracleCommand(SQLCommand, conn)
    Dim sb As New StringBuilder

    Dim x As Integer = 0
    Dim CountRow As Integer = 0

    If File.Exists(FileName) Then
        File.Delete(FileName)
    End If

    Using FileObject As New FileStream(FileName, FileMode.OpenOrCreate)
        Using MStream As New MemoryStream()
            Using StreamWriterObj As New StreamWriter(MStream)
                Using Reader As OracleDataReader = cmd.ExecuteReader()
                    Dim FieldCount As Integer = Reader.FieldCount - 1

                    Do While Reader.Read()
                        sb.Append(Reader.Item(0))
                        For i = 1 To FieldCount
                            sb.Append(Delimiter)
                            sb.Append(Reader.Item(i))
                        Next
                        sb.Append(vbCrLf)

                        'Write every 25000 rows of data to the file from the buffer
                        If x = 25000 Then
                            StreamWriterObj.Write(sb.ToString().ToCharArray())
                            MStream.Seek(0, SeekOrigin.Begin)
                            MStream.WriteTo(FileObject)
                            sb = New StringBuilder()
                            CountRow = CountRow + x
                            x = 0
                        End If
                        x = x + 1
                    Loop

                    'Write any remaining data from the buffer to the file
                    StreamWriterObj.Write(sb.ToString().ToCharArray())
                    MStream.WriteTo(FileObject)

                    Reader.Close()
                    StreamWriterObj.Close()
                    MStream.Close()
                    FileObject.Close()
                End Using
            End Using
        End Using
    End Using
End Sub

Any and all comments and help are geatly appreciated and, also, even though this is written in VB, I'm equally as comfortable with C# solutions.

You can simplify the code greatly without loosing any performance, more likely gaining instead.

There is no need to use StringBuilder - you can write directly to the StreamWriter - it will be faster since there will be no need of copy the whole data twice.

Next, there is no need to use MemoryStream - you are using it to buffer data before writing it to the disk. The same can be achieved by specifying the buffer size when creating FileStream. You should play around with the buffer size to see what is fastest on your environment - usually 4KB is used, in the sample below it is 1MB, but you have to try it on your hardware.

You should also play around with the options how to open FileStream. You should try what impact Asynchronous and WriteThrough has on your solution.

Of course, as the other answer proposed, separating reading from database and writing to the file in two threads might work as well. Try and measure three separate scenarios - just reading from the database (measure separately the query (ExecuteReader() and first Read() and then reading the other rows - because it might be that most of the time is spending to initially execute the query, not to iterate through the data), just writing to the file (dummy data) and then combined. This way you will see if there is actual reason to put the two operations in parallel.

Dim conn As New OracleConnection()
...
Sub DBExecuteQueryWriteToFile(ByVal SQLCommand As String, ByVal FileName As String, Optional ByVal Delimiter As String = ",")

    Dim cmd = New OracleCommand(SQLCommand, conn)
    Dim bufferSize = 1024*1024; // 1Mb

    If File.Exists(FileName) Then
        File.Delete(FileName)
    End If

    Using FileObject As New FileStream(FileName, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None, bufferSize)
    Using StreamWriterObj As New StreamWriter(FileObject)
    Using Reader As OracleDataReader = cmd.ExecuteReader()
        Dim FieldCount As Integer = Reader.FieldCount - 1

        Do While Reader.Read()
            StreamWriterObj.Write(Reader.Item(0))
            For i = 1 To FieldCount
                StreamWriterObj.Write(Delimiter)
                StreamWriterObj.Write(Reader.Item(i))
            Next
            StreamWriterObj.WriteLine();
        Loop

    End Using
    End Using
    End Using
End Sub

P.S. few other issues in your code:

StringBuilder().ToString().ToCharArray() - no need to convert to char array.
new StringBuilder() - instead of reusing the memory already allocated to the existing builder, you are throwing that away and creating new one.
MStream.Seek() - you are seeking to the beginning of the stream but are not setting Length to 0. So if the second block written there is smaller than first one, junk data will be written to the file.


My approach on nested properties that bind to XAML Here is my simple model.
public class Product
{
    public String Name { get; set; }
    public int Price { get; set; }
    public String Info { get; set; }
}

When I use this in viewmodel as following:
    public MainViewModel()
    {
        Item = new Product() { Name = "Coffee", Price = 4, Info = "Coffe is hot" };
    }

    protected Product _item;
    public Product Item
    {
        get { return _item; }
        set
        {
            _item = value;
            NotifyPropertyChanged("Item");
        }
    }

In my XAML, I am binding to the nested properties.
<StackPanel>
    <TextBox Text="{Binding Item.Name}"/>
    <TextBox Text="{Binding Item.Price}"/>
    <TextBox Text="{Binding Item.Info}"/>
    <Button Content="Change Item" Click="Change_Item_Clicked"/>
</StackPanel>

I change only nested property in the click event:
    private void Change_Item_Clicked(object sender, RoutedEventArgs e)
    {
        MainViewModel vm = DataContext as MainViewModel;

        vm.Item.Name = "Donut";
    }

This obviously doesn't update the UI since the parent property implements INotifyPropertyChanged but not the nested one.
So I came up with the following solution and it does then update the UI fine.
I am essentially creating viewmodel version of the Product class as below that implments INotifyPropertyChanged.
   public class ProductVM : Notifier
    {
        protected Product _p;
        public ProductVM(Product p)
        {
            _p = p;
        }

        public String Name
        {
            get { return _p.Name; }
            set
            {
                _p.Name = value;
                NotifyPropertyChanged("Name");
            }
        }

        public int Price
        {
            get { return _p.Price; }
            set
            {
                _p.Price = value;
                NotifyPropertyChanged("Price");
            }
        }

        public String Info
        {
            get { return _p.Info; }
            set
            {
                _p.Info = value;
                NotifyPropertyChanged("Info");
            }
        }
    }

Now the viewmodel use its version of the Product class as defined above that has ability to notify on changes.
    class MainViewModel : Notifier
    {
        public MainViewModel()
        {
            Item = new ProductVM( new Product() { Name = "Coffee", Price = 4, Info = "Coffe is hot" });
        }

        protected ProductVM _item;
        public ProductVM Item
        {
            get { return _item; }
            set
            {
                _item = value;
                NotifyPropertyChanged("Item");
            }
        }
    }

This works great but what do you guys think of this approach? If we are going to bind to a property (nested or top level), wouldn't it be best for all these properties to implment the INotifyPropertyChanged?
What naming conversion can I use to name ProductVM better? Is there a less cody way to achieve this?

To me it looks fine. If you can or will not modify the model objects (Product) by letting them implement INotifyPropertyChanged then this approach is the price to pay for using XAML. In larger projects, it can be a little annoying to seemingly write the same thing twice, but you on the other hand have a true separation of concerns, and your solution is an implementation of the MVVM-pattern.
To me ProductVM is an OK name, I always name my view objects that way.

Instead of using string literals in the call to NotifyPropertyChanged you can use nameof(<property name>):
               NotifyPropertyChanged(nameof(Name));

this will make it easier to maintain, if the property name changes.

   Another approach is to define `NotifyPropertyChanged` as:

  private void NotifyPropertyChanged([CallerMemberName] string name = "")
  {
    PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name));
  }

then you don't need to provide the property name in the call:
    public String Name
    {
        get { return _p.Name; }
        set
        {
            _p.Name = value;
            NotifyPropertyChanged();
        }
    }

CallerMemberName requires a reference to System.Runtime.CompilerServices.


Simple python scheduled tasks script that runs once daily I want to execute a python script daily and thereby want to schedules task as follows.

The idea is that the script will run once daily, and the script will then decide what tasks to be done today.

The reason is that I can't use cron or windows scheduled task on my environment.

The question is that, is there any better idea?

from datetime import datetime
import os

from sheduled_tasks import daily_task1, daily_task2, weekly_task, monthly_tasks

tasks = {} # or open from saved pickel file

schedule = {
    1: [daily_task1, daily_task2],
    7: [weekly_task],
    30: [monthly_tasks],
}

for delta, stasks in schedule.items():
    for task in stasks:
        name = task.__name__
        if ((task not in tasks) or
            (datetime.now() - tasks[name]).days >= delta):
            task()
            tasks[task.name] = datetime.now().replace(hour=6)

# pickle tasks

You can get rid of doing any I/O and make your code cleaner in the process.

from datetime import datetime
from sheduled_tasks import daily_task1, daily_task2, weekly_task, monthly_tasks

schedule = {
    1: [daily_task1, daily_task2],
    7: [weekly_task],
    30: [monthly_tasks],
}

# this could also be the date of first invocation, just change the timestamp
arbitrary_date = datetime.fromtimestamp(1234567890)
days_past = (datetime.now() - arbitrary_date).days

for delay, tasks in schedule.items():
     if days_past % delay == 0:
        [task() for task in tasks]


Convert Repeating Decimal to Fraction My code converts repeating decimals to fractions. It works but I am interested in feedback on how I could improve and/or simplify it?

jsFiddle: http://jsfiddle.net/tfgTu/

<script>
// Repeating decimal
// 21.83(35745857)
var decimal = 21.8335745857

// Total length of the decimal
var decimalLength = 10;

// Length of the decimal before the repeating part
var decimalBeforeRepeatLength = 2;

// 10 to the total length of the decimal
var power1 = Math.pow(10, decimalLength);

// 10 to the length of the decimal before the repeating part
var power2 = Math.pow(10, decimalBeforeRepeatLength);

var finalPower = power1 - power2;

var decimal1 = decimal * power1;

var decimal2 = parseInt(decimal * power2);

var finalDecimal = decimal1 - decimal2;

var output = simplifyFraction(0, finalDecimal, finalPower);

document.write(output[1] + " / " + output[2]);

// Finds the greatest common divisor of 2 non-zero integers
// a = integer 1
// b = integer 2
function greatestCommonDivisor(a, b) {
    if (b === 0) {
        return a;
    }

    return greatestCommonDivisor(b, (a % b));
}

// Simplifies all forms of fractions
// Does not change form of fraction
// i = number
// n = numerator
// d = denominator
function simplifyFraction(i, n, d) {
    var gcd = greatestCommonDivisor(n, d);

    if (Math.abs(i) > 0 && n >= d) { // If mixed number AND improper fraction, assume the user wants a simplified mixed number

        if (i < 0) {
            n *= -1;
        }

        var temp = improperFractionToMixedNumber(n, d);

        return [temp[0] + i, temp[1] / gcd, temp[2] / gcd];
    }

    return [i, n / gcd, d / gcd];
}
</script>

While the general code quality is good, the contract should be formalized by adding "use strict";
Even though var is used, everything is still in the global namespace, thereby polluting it and risking name collisions. Moving things into a namespace also means some other parts of the code can/should be re-arranged.
There's ideally no need to manually specify both the number, and how many digits there are after the ..
I honestly fail to understand the problem the function solves, so it's unclear to me if var decimalBeforeRepeatLength = 2 can also be derived automatically from the input, or if it needs to be a separate input parameter.
There's no radix parameter to parseInt(). Without it, the "correct" behaviour is not guaranteed, so you really should specify it. See Mozilla's documentation for details.
It's unclear to me why parseInt() is used, why not just Math.floor()?
Separating the output from the functionality would be a nice improvement.
improperFractionToMixedNumber isn't included in the actual code you posted, so I can't really comment on that, nor test that the code gets the same results before and after my changes.
Since i is hardcoded to 0, most of the original simplifyFraction() can actually be removed all together, but I'm guessing making i a parameter would be the solution instead.
Why does simplifyFraction() return i when it's an unmodified input parameter?
With some of the changes I've done, a few variables should probably have their names changed. I'll leave that as an exercise to the reader.

Code below incorporating the above changes. I've removed all original comments and added my own, to make it easier to spot the differences.

// Isolate variables and functions from the global scope.
// The namespace should of course not actually be named 'namespace'.
var simplifyFraction = function(decimal, decimalBeforeRepeatLength) {
   // Improve the code quality with strict mode.
  "use strict";

  // Figure out decimal length automatically.
  var indexOfDot = decimal.toString().indexOf('.');
  var decimalLength = decimal.toString().length - indexOfDot - 1;

  var power1 = Math.pow(10, decimalLength);
  var power2 = Math.pow(10, decimalBeforeRepeatLength);
  var finalPower = power1 - power2;
  var decimal1 = decimal * power1;
  // Explicitly set a radix of '10' to avoid unpredicatable results.
  var decimal2 = parseInt(decimal * power2, 10);
  // Preferred option.
  decimal2 = Math.floor(decimal * power2);
  var finalDecimal = decimal1 - decimal2;
  var i = 0;

  var greatestCommonDivisor = function(a, b) {
    if (b === 0) {
        return a;
    }

    return greatestCommonDivisor(b, (a % b));
  };

  var gcd = greatestCommonDivisor(finalDecimal, finalPower);

  // Since simplifyFraction() is now what we use for namespacing, it's not
  // kept as a function in here.
  if (Math.abs(i) > 0 && finalDecimal >= finalPower) {
    if (i < 0) {
      finalDecimal *= -1;
    }

    var temp = improperFractionToMixedNumber(finalDecimal, finalPower);

    return [temp[0] + i, temp[1] / gcd, temp[2] / gcd];
  }

  return [i, finalDecimal / gcd, finalPower / gcd];
};

// The function no longer forces a certain outout. We get back the answer and treat it anyway we want.
//
var answer = simplifyFraction(21.8335745857, 2);


Split DAG into disjoint sets I wrote an algorithm to separate a DAG into disjoint sets of vertices. I was hoping to have some comments on algorithmic complexity and efficiency of the Python code. Notably, I don't like the adding and removing of components from the components list.

g = {1: [3, 4], 2: [5], 3:[7], 4:[], 5:[], 6:[1], 7:[]}

vertices = set(g)
components = []
while vertices:
  stack = list((vertices.pop(),))
  comp = set()
  while stack:
    vert = stack.pop()
    comp.add(vert)
    if vert in vertices:
       vertices.remove(vert)
    for e in g[vert]:
        if e in vertices:
            stack.append(e)
        else:
            for component in components:
               if e in component:
                  component.update(comp)
                  comp = component
                  components.remove(comp)
    components.append(comp)

print([c for c in components])

>>> [{2, 5}, {1, 3, 4, 6, 7}]

It would seem to me that one way to solve this is to change all the directed arcs in the dag to undirected edges. The rest is to recognize the disjoint subgraphs using breadth-first search:

def convert_directed_graph_to_undirected_graph(graph):
    """Converts each directed arc into an undirected edge."""
    undirected_graph = {}
    for node, child_node_list in graph.items():
        if node not in undirected_graph.keys():
            undirected_graph[node] = []
        undirected_graph[node].extend(child_node_list)
        for child_node in child_node_list:
            if child_node not in undirected_graph.keys():
                undirected_graph[child_node] = []
            undirected_graph[child_node].append(node)
    return undirected_graph

def crawl_subgraph(source_node, undirected_graph, visited):
    """Implements breadth-first search in order to find the entire subgraph including the source noe."""
    if source_node in visited:
        return None
    visited.add(source_node)
    queue = deque([source_node])
    subgraph = [source_node]
    while queue:
        current_node = queue.popleft()
        for neighbor_node in undirected_graph[current_node]:
            if neighbor_node not in visited:
                visited.add(neighbor_node)
                subgraph.append(neighbor_node)
                queue.append(neighbor_node)
    return subgraph

def find_disjoint_dags(graph):
    """Finds all disjoint subdags in the input dag."""
    undirected_graph = convert_directed_graph_to_undirected_graph(graph)
    visited = set()
    disjoint_subgraphs = []
    vertices = set(undirected_graph)
    for vertex in vertices:
        subgraph = crawl_subgraph(vertex, undirected_graph, visited)
        if subgraph:
            disjoint_subgraphs.append(subgraph)
    return disjoint_subgraphs

Hope that helps.


Analyzing genetic tags I've gone back and forth a few times recently on my Perl coding style when it comes to module subroutines. If you have an object and you want to call the method bar with no arguments, then you can either do $foo->bar() or $foo->bar. At one point I started favoring the latter because I felt it cleaned up the code and made it more readable. However, sometimes I question whether it would be better to be fully explicit, especially considering the possibility that someone else will have to look at my code later — someone who almost certainly will not be an expert Perl programmer.

For example, consider this block of code. For the method calls that require arguments (get_tag_values and has_tag), there is no question about the parentheses. But what about next_feature and primary_tag? Is the readability I gain from dropping the parens worth losing the explicit syntax? Is one better than the other for long term maintainability? Or is this simply a subjective judgment call?

while( my $feature = $gff3->next_feature )
{
  if($type eq "cds")
  {
    if( $feature->primary_tag eq "mRNA" )
    {
      my($gene_id) = $feature->get_tag_values("Parent");
      my($mRNA_id) = $feature->get_tag_values("ID");
      next unless( $list eq '' or $genes_to_extract->{$gene_id} );
      $subseq_locations->{ $feature->seq_id }->{ $mRNA_id } = Bio::Location::Split->new();
    }
    elsif( $feature->primary_tag eq "CDS" )
    {
      my($mRNA_id) = $feature->get_tag_values("Parent");
      if( $subseq_locations->{ $feature->seq_id }->{ $mRNA_id } )
      {
        $subseq_locations->{ $feature->seq_id }->{ $mRNA_id }->add_sub_Location( $feature->location );
      }
    }
  }
  else
  {
    if( $feature->primary_tag eq $type )
    {
      my $feat_id;
      if( $list ne '')
      {
        ($feat_id) = $feature->get_tag_values("ID") if($feature->has_tag("ID"));
        next unless( $feature->has_tag("ID") and $genes_to_extract->{$feat_id} );
      }
      $subseq_locations->{ $feature->seq_id }->{ $feat_id } = Bio::Location::Split->new();
      $subseq_locations->{ $feature->seq_id }->{ $feat_id }->add_sub_Location( $feature->location );
    }
  }
}

I prefer to make a distinction on semantic level: there are no functions or methods, but instead there are properties and actions. Every property is an object (in real-world sense) and an action is something done on an object. A good mnemonic is probably to read () as do it!.

Consider a variable:

$document

You know by the name it's an object, since it's a noun.

$document->author

Even though author is probably a function/method, an author is an object and a noun, so it's a property and thus no parens here.

$document->send()
$document->author->write_book()

Again, send and write_book are functions/methods, but since those are actions (as designated by verbs), we write parens behind them.

$document->author_count
$document->has_reviews

Again, a function/method, but a property of a document object, not an action, so no parens. A boolean value or an integer is strictly speaking not a real-world object, but it's more like an object than an action.


Come around limitation of CSingleLock don't accept NULL The CSyncObject object in CSingleLock constructor can't be NULL but I want it to work with NULL because I need to use this nest.

BOOL CADODatabase::Open(LPCTSTR lpstrConnection, LPCTSTR lpstrUserID, LPCTSTR lpstrPassword)
{
    CString database = GetSourceDatabase( lpstrConnection, NULL );

    // get the appropriate critical section based on database
    g_dbCriticalSection = GetDbCriticalSection( database );

    if( g_dbCriticalSection) // it can be NULL
    {
        g_dbCriticalSection->Lock();
    }

    // more code,  have to unlock() manually in multiple exits!
}

As can be seen, I need to lock() and then unlock() manually at all exit points because its in a nest.

i wrote a class to make CSingleLock work with NULL, my new code looks like this:

BOOL CADODatabase::Open(LPCTSTR lpstrConnection, LPCTSTR lpstrUserID, LPCTSTR lpstrPassword)
{
    CString database = GetSourceDatabase( lpstrConnection, NULL );

    // get the appropriate critical section based on database
    g_dbCriticalSection = GetDbCriticalSection( database );

    CSingleLockEx lock( g_dbCriticalSection, TRUE );

    // eliminates the need to unlock() manually, works like CSingleLock
}

My new class is here:

class CSingleLockEx
{
public:
    CSingleLock* _singleLock;

    CSingleLockEx(CSyncObject* pObject, BOOL bInitialLock = FALSE):
    _singleLock(NULL)
    {
        if (pObject)
            _singleLock = new CSingleLock(pObject, bInitialLock);

    }
    ~CSingleLockEx()
    {
        if (_singleLock)
            delete _singleLock;
    }
};

Since it has to do with critical section, I thought I will put it for review if I have missed anything.

Sure, that seems fine.

Obviously you should be using std::unique_ptr<CSingleLock> (and std::make_unique) instead of raw new and delete.
If you have access to C++17, Folly, or Boost, consider using std::optional<CSingleLock> to eliminate the heap allocation.
If you're not using unique_ptr, you might want to =delete your copy constructor and copy assignment operator (or in C++03, make them private), to ensure that nobody accidentally creates a copy of your object — which if they did would lead to a double-free.

You've also made your member variable _singleLock public, presumably by accident. If you actually want it to be public, consider giving it a more user-friendly name. It's best to keep it private, though.

A simpler approach would be to create a dummy lock, which (assuming either CSingleLock or CMutex is immovable) would actually fix the copyability problem as well.

class CSingleLockEx
{
    CMutex dummy_;
    CSingleLock lk_;
public:
    CSingleLockEx(CSyncObject *obj, bool initialLock = false) :
        lk_((obj ? *obj : dummy_), initialLock)
    {}
};

This way you end up locking something either way (and unlocking something in the destructor as well), but at least when obj==nullptr the "something" you're locking will definitely be uncontended. So I wouldn't expect a noticeable performance hit.


My jQuery Form Plugin Long story short, I wrote this jQuery form plugin as an exercise to better understand how other form plugins work. If you have any suggestions about how to improve this (or my coding style in general) let me know.

var Form = function(options) {

    options = options || {};

    this.url = options.url;
    this.$fields = $(options.fields);
    this.$status = $(options.status);
    this.$submit = $(options.submit);
    this.$form = options.form || 'form';
    this.rFields = {
        required: /[^.*]/,
        nodigit: /^[^0-9]+$/,
        email: /^[a-z0-9._%-]+@[a-z0-9.-]+\.[a-z]{2,4}$/i,
    };
    this.required = [
        this.rFields.nodigit,
        this.rFields.email,
        this.rFields.required,
        this.rFields.required
    ];
    this.errors = ['Please enter a valid name.', 'Please enter a valid email address.', 'You must enter a subject.', 'No message? But I wanted one :('];

}

Form.prototype.init = function init() {

    this.$submit.click($.proxy(function(e) {
        this.process(e);
    }, this));

};

Form.prototype.process = function process(e) {

    var filled = true;

    e.stopImmediatePropagation();
    e.preventDefault();

    this.$fields.each($.proxy(function(i, element) {

        var $element = $(element);

        if (!$element.val().match(this.required[i])) {

            $element.focus();
            this.update(this.errors[i], true);
            filled = false;
            return false;

        }
    }, this));

    filled && this.submit($(this.$form).serialize());

};

Form.prototype.update = function update(message, isError) {

    if (isError && this.$status.hasClass('error')) {

        this.$status.addClass('error')
            .text(message);

        setTimeout($.proxy(function() {
            this.$status.removeClass('shake');
        }, this), 500);

    }
    else {

        this.$status.fadeOut();

        setTimeout($.proxy(function() {
            this.$status.text(message)
                .fadeIn();

            if (isError) {
                this.$status.removeClass('shake')
                    .addClass('error');
            }
            else {
                this.$status.removeClass('error info')
                    .addClass('success');
            }
        }, this), 500);

    }

    return this;

};

Form.prototype.submit = function submit(data) {

    $.ajax({
        type: 'POST',
        url: this.url,
        data: data,
        success: $.proxy(function(msg) {
            this.$fields.fadeOut();
            this.$status
                .empty()
                .append('
Nice one! The form has been sent to the server...
')

            $(this.$submit)
                .animate({
                left: '-=500px',
                opacity: 0,
            }, 300);

        }, this)
    });
};

Here's how to create a new form:

var exampleForm = new Form({
    url: mailUrl,
    fields: 'form input, form textarea',
    status: '#status',
    submit: '#submit'
})

exampleForm.init()

Pretty simple, just pass in..

The url to send the form data too
The form fields all encapsulated in one string seperated by commas in the correct order of validation rules.
Selector string where you want the status message to go
The submit selector. This element will actually submit the form

From a once over;

I would not split statements like

this.$status.addClass('error')
    .text(message);

over 2 lines, I would reserve that treatment for the longer statements.
You seem to cheat with the simpleness of your call, Form knows already the error messages with this.errors and the validation types with this.required. These should have come in as options.
I tend to comment all my regular expressions with my intent and an example, that might be overkill for your regular expressions
In Form.prototype.process it would have been simpler to have var self = this and use self instead of using $.proxy(. While using closures is not a panacea, it should also not be avoided at all costs.
You seem to like shortcuts, consider

return filled = false;

instead of

filled = false;
return false;

filled && this.submit($(this.$form).serialize()); <-- Too hacky
Why is this code removing the shake css class, it never assigns it..
On the whole it seems Form.prototype.update could be written in a smarter way, but I would have to see an actual snippet working to get started on that


Getting all public constants from the main and nested classes I wrote this recursive function to get all constants from a class and its subclasses. Can it be simplified?

private static IEnumerable<FieldInfo> GetPublicConstants(Type type)
{
    var subtypes = type.GetNestedTypes(BindingFlags.Public);

    foreach (var subtype in subtypes)
    {
        foreach (var constant in GetPublicConstants(subtype))
        {
            yield return constant;
        }
    }

    var publicStaticFields = type.GetFields(BindingFlags.Public |
         BindingFlags.Static | BindingFlags.FlattenHierarchy)
         .Where(fi => fi.IsLiteral && !fi.IsInitOnly);

    foreach (var fieldInfo in publicStaticFields)
    {
        yield return fieldInfo;
    }
}

For example, it would return 6 constants for the following class:

public static class MyClass
{
    public const string AAA = "";

    public static class A
    {
        public const string Aaaaa = "";

        public static class AB
        {
            public const string ABaaa = "";
        }
    }

    public static class B
    {
        public const string Baaa = "";
        public const string Bbbb = "";
    }

    public static class C
    {
        public const string Caaa = "";
    }
}

What I don't like is the fact I'm having 2 loops where I yield return 1 item because of inability to yield return a list of items.

How about this:

private static IEnumerable<FieldInfo> GetPublicConstants(Type type)
{
    return GetPublicClassConstants(type)
        .Concat(type.GetNestedTypes(BindingFlags.Public).SelectMany(GetPublicConstants));
}

private static IEnumerable<FieldInfo> GetPublicClassConstants(Type type)
{
    return type
        .GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.FlattenHierarchy)
        .Where(fi => fi.IsLiteral && !fi.IsInitOnly);
}

OR, as I do enjoy playing PGA Pro Code Golf:

private static IEnumerable<FieldInfo> GetPublicConstants(Type type)
{
    return type
        .GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.FlattenHierarchy)
        .Where(fi => fi.IsLiteral && !fi.IsInitOnly)
        .Concat(type.GetNestedTypes(BindingFlags.Public).SelectMany(GetPublicConstants));
}


Skeleton for a command-line tool to scan domains for vulnerabilities I'm working on a new command line application in GO and was hoping I could get feedback on my design pattern, or suggestions on a better one to use. I'm still new to GO, only having used it for a handful of projects, and would value any suggestions to take better advantage of the "GO" way of doing things.

The intent of the application will be to either scan a single domain, or a list of domains and then output results in either a normal format (depending on flags) containing information about vulnerable domains and suggestions, or a file only showing the domains in the initial input that are vulnerable.

main.go

package main

import (
    "flag"
    "fmt"
    "./libprojectstart"
    "os"
)

func Banner() {
    fmt.Printf("%s%sprojectstart - SSL Mass Renegotiation Tester\n%s",
        libprojectstart.OKBLUE, libprojectstart.BOLD, libprojectstart.RESET)
}

func ParseCmdLine() (state *libprojectstart.State) {
        s := libprojectstart.InitState()

        flag.StringVar(&s.Domain, "d", "", "Domain to scan")
        flag.StringVar(&s.DomainList, "dL", "", "List of domains to scan")
        flag.StringVar(&s.OutputNormal, "oN", "", "File to write normal output to")
        flag.StringVar(&s.OutputDomains, "oD", "", "File to write successful domains to")
        flag.IntVar(&s.Threads, "t", 20, "Number of threads (Default: 20)")
        flag.BoolVar(&s.NoColour, "--no-colour", true, "Don't Use colors in output")
        flag.BoolVar(&s.Silent, "--silent", false, "Output successful scans only")
        flag.BoolVar(&s.Verbose, "v", false, "Verbose output")
        flag.BoolVar(&s.Usage, "h", false, "Display this message")
        flag.Parse()

        return &s
}

func main() {
    state := ParseCmdLine()
    if state.Silent != true {
        Banner()
    }

    if libprojectstart.IsUnchanged(*state) || state.Usage {
        Banner()
        flag.PrintDefaults()
        os.Exit(1)
    }

    if validated, error := libprojectstart.ValidateState(state); validated != true {
        fmt.Printf("%s[!] %s%s", libprojectstart.FAIL, libprojectstart.RESET, error)
        flag.PrintDefaults()
        os.Exit(1)
    }
}

state.go

package libprojectstart

// Contains state read in from the command line
type State struct {
    Domain          string  // Domain to check for
    DomainList      string  // File location for a list of domains
    OutputNormal    string  // File to output in normal format
    OutputDomains   string  // File to output domains only to
    Verbose         bool    // Verbose prints, incl. Debug information
    Threads         int     // Number of threads to use
    NoColour        bool    // Strip colour from output
    Silent          bool    // Output domains only
    Usage           bool    // Print usage information
}

func InitState() (s State) {
    return State { "", "", "", "", false, 20, false, false, false }
}

func ValidateState(s *State) (result bool, error string) {
    if s.Domain == "" && s.DomainList == "" {
        return false, "You must specify either a domain or list of domains to test"
    }

    return true, ""
}

func IsUnchanged(s State) bool {
    return s == InitState()
}

colour.go

package libprojectstart

var (
    HEADER      = "\033[95m"
    OKBLUE      = "\033[94m"
    OKGREEN     = "\033[92m"
    WARNING     = "\033[93m"
    FAIL        = "\033[91m"
    BOLD        = "\033[1m"
    UNDERLINE   = "\033[4m"
    RESET       = "\033[0m"
)

The intent of this application is that the command line options, as well as the feedback being printed will grow significantly over time. I've aimed to demonstrate some of that error checking in ValidateState() as that function will grow with the project, as will the printing of colours. Open to any and all suggestions for improvements!

Validation logic

When checking whether the program should print the usage message or not,
I find it an unusual approach to say "is the state (of initial parameter values) unchanged?"
It would be more natural to check facts more directly.
For example in this program the only input you really require is one or more domain names.
Then that should be the determining factor for displaying the usage message: "did the user specify a domain name?"
Direct, straightforward, to the point.

Naming

The current names are not very intuitive:

State is state of what? It's the command line parameters. Something like CmdParams would be better.
Banner is a noun, so it sounds like an object, not something that takes some action. PrintBanner would be better.
Names starting with capital letters are exported (~ public API) in go. Many if not all names in the posted could be private, so their names should start with lowercase.
Instead of InitX, when creating a new instance of X, the name NewX is more common.

Performance

Every time you evaluate s == InitState(), InitState() creates a new instance to perform the comparison. It would be enough to have just one such instance as a frame of reference, created as a var at top-level scope, no need to create a new instance every time.

Style

Many of the functions specify a name for the returned value, but don't actually use it in the function body. So those optional names can be dropped.

Instead of this:

return State { "", "", "", "", false, 20, false, false, false }

You could take advantage of the default zero-values of strings and booleans, and write simpler like this:

return State{Threads: 20}

In my experience, negative boolean variables such as NoColour are often confusing in practice.
Inverting the meaning and using !Colour tends to work better.

Use short boolean conditions

Instead of if state.Silent != true, you can write shorter as if !state.Silent.

Banner printed twice

When the script is called without parameters,
the banner will be printed twice.
Swap these two statements to avoid that.

if state.Silent != true {
    Banner()
}

if libprojectstart.IsUnchanged(*state) || state.Usage {
    Banner()
    flag.PrintDefaults()
    os.Exit(1)
}