Security of authentication check for a blog

Ok, just a few quick thoughts, will be adding to this answer later today (ie after work):

One class, One task
Chances are, you've already heard of the SOLID principles. Well, here they are again: your code is in violation of the SRP (Single Responsability principle). A class has one task. No more, no less. Your class does 4 things: it stores data (all those public properties), messes with sessions, connects to the DB and performs queries.
The Database connection and queries are all things that should be dealt with in another class, that simply connects to the DB and executes queries.

This other class could then have a method like this:

public function login(User $user)
{
    $query = $this->db->prepare("SELECT username, password FROM blog_users WHERE username = ? AND password = ? LIMIT 1");
    $query->bind_param('ss', $user->username, $user->password);
    $query->execute();
    $query->bind_result($user->username, $user->password);
    $query->store_result();
    if($query->num_rows == 1)
    {
        $query->fetch();
        $query->close();
        return $this->getUserData($user);//changed name of method
    }
    $query->close();
    throw new RuntimeException('User can not login');//error
}

Even this isn't close to efficient code, because it still calls a second method to, basically, repeat the same query. Why would you do so, why not simply write:

public function login(User $user)
{
    $query = $this->db->prepare(
        "SELECT id, email, rank, creationdate,
            country, status, username, password
        FROM blog_users
        WHERE username = ? AND password = ? LIMIT 1"
    );
    $query->bind_param('ss', $user->username, $user->password);
    $query->execute();
    $query->bind_result(
        $user->id,
        $user->email,
        $user->rank,
        $user->creationdate,
        $user->country,
        $user->status,
        $user->username,
        $user->password
    );
    $query->store_result();
    if($query->num_rows == 1)
    {
        $query->fetch();
        $query->free_result();//call free_result...
        $query->close();//and close here, too: your code doesn't do this
        return $user;
    }
    $query->free_result();
    $query->close();
    throw new RuntimeException('User can not login');//error
}

Isn't that a lot shorter, clearer and doesn't this also do exactly what you are doing? K.I.S.S!

Classes with a dependency, should be open to injection
Your current code is hard to test: the constructor connects to the DB, there is no way for me (the user), or you (when testing) to mock the db connection, or use a bogus testing table.
The best way to make your class easier to test it to allow the user to pass a specific connection to the constructor, and then he can use your class/functionality to work on the db of his choice:

public function __construct(mysqli $connection = null)
{//default null means the argument is optional
    if ($connection === null)
        $connection = new mysqli();//connect to default
    $this->db = $connection;//use connection we've created, or the one the user passed through to the constructor
}

And of course: a DB connection that is opened, should be closed: when your class dies (is destructed), the DB connection should die, too:

public function __destruct()
{
    $this->db->close();
    $this->db = null;
}

That should be added to your code, too. If you don't then the connection will be kept open until your script has terminated, which could be longer than you want. If performance and cleanliness matter to you, then this should be a habit.

Standards, coding style
Yes, I complain/nag a lot about coding style and standards but if you are, like you say, creating a sizeable code-base, then you should know that coding style and standards are insanely important.
The unofficial PHP standards can be found here. Though unofficial, all major players (ie Zend, Symfony, Cake, Doctrine, ...) all subscribe to them.

Other niggles:
Your class has tons of properties, and it's those properties that you are assigning in the methods. Why, then aren't you assigning $username as soon as you can, so you don't have to pass it to the getData method as an argument?

Another remark I have is simply this:
If a script contains only PHP code, then that script should not have a closing ?> tag. Get rid of it.


Why does generate_name(0) return False? I would expect an empty string '', or maybe None.

Rather than check if full_syl == '': (not a great name, by the way; why not just name?) during the loop, I would build the whole name in lowercase then  use str.title: return full_syl.title(). This removes both .upper and .lower from the forloop, simplifying the code.

Why pick every letter randomly? In this part:

decision = random.choice(('consonant', 'vowel'))

if full_syl[-1:].lower() in vowels:
    decision = 'consonant'
if full_syl[-1:].lower() in consonants:
    decision = 'vowel'

the first value of decision is ignored for every letter but the first. If you want strict alternation, you only need to randomly pick the type of the first letter. You could even use itertools.cycle to switch back and forth between vowels and consonants for each pick.

Rather than keep adding letters to a string, I would probably build a list or iterator of letters then str.join them. Otherwise you create many unnecessary string objects for the partial names, which are immediately discarded.


Check whether a permutation with one fixed point can be rotated into one with two fixed points

Doc-string

You can add a docstring to the method using the convention of PEP-257 so IDE's etc can parse it and show it when looking at the method.

typing

By adding type annotations you can make it more clear to the user of the function what input the methods require and what to expect as answers, serving as extra documentation. It also allows static analysers like mypy to discover bugs.

generator

Instead of returning a list, you can have the permutation generate yield the correct permutations one by one

reduce the number of permutations

If any permutation where 1 person sits on the correct chair is ok, why not pick one where person 0 sits on the correct chair. This reduces the number of permutations to check n times

import itertools
import typing
def perms_gen(n: int) -> typing.Iterator[typing.Sequence[int]]:
    """Yields the permutations where only the first person sits on his own seat."""
    for perm in itertools.permutations(range(1, n)):
        if any(i == j  for i,j in enumerate(perm, 1)):
            continue
        yield (0,) + perm

This is a generator function which yields a new permutation each time another function (next, for-loop, ..) asks for it,instead of all generating them in one go. This way, only those permutations that are needed are generated, and they occupy less memory. I you want a list of all permutations, you have to explicitly ask for it by list(perms_gen(10))

rotate

Rotating a list can be done by splice indexing. No need to call a deque. If you want to use a deque, then use its rotate method, instead of popping and appending yourself

Saving the 3 letters to type is also not useful.

def rotate(
    seq: typing.Sequence[typing.Any], n: int
) -> typing.Sequence[typing.Any]:
    """Rotates `seq` `n` places."""
    n %= len(seq)
    return seq[n:] + seq[:n]

The in-place modulo is to make sure n is in the range [0, len(seq))

Testing further rotations

To see whether there is a rotation which allows more than 1 person in his or her seat, you can also count for each person how much spaces they need to go to their correct seat. If more than 1 person needs the same number of rotations, you have a match. To check whether there is no rotation of a permutation so 2 people sit in their correct seat, you can use this set comprehension {(i - j) % n for i, j in enumerate(permutation)} and test whether its length is n

return values

When there is no response, I would either raise an exception or a sentinel value. You do approximately the same, by your return n, 'Counterexample found:', f_real in case of success and in case of failure return n, 'no counterexamples found'. But using a string as sentinel value means the users of this function needs a complicated test, further complicated by the face that the lengths of the return tuples are different, so you can not use tuple unpacking.

Returning n has no use, since the user called the method with n as an argument. If he wants to produce a message with n included, he doesn't need to get it returned here.

As sentinel for failure I would return None or raise an exception.

putting it together:

def main(n: int) -> typing.Optional[typing.Tuple[int, ...]]:
    """Tests whether there is a permutation so not more than 1 person
    return to their original seat when rotated.

    returns a counter example when found, or None.
    """
    for permutation in perms_gen(n):
        if len({(i - j) % n for i, j in enumerate(permutation)}) == n:
            return permutation

This either returns a tuple with an example, or None is implicitly returned when all permutations are tested.

timings

On my machine, for n == 10 this takes about 700ms compared to 30s for your original

Keeping it as a generator

If you rewrite it a bit, you can keep the rotations_to_place as a generator, only calculating as needed, and yielding the correct permutations

def main_generator(n: int) -> typing.Iterator[typing.Tuple[int, ...]]:
    """Yields all complying permutations."""
    permutations = perms_gen(n)
    for permutation in perms_gen(n):
        if len({(i - j) % n for i, j in enumerate(permutation)}) == n:
            yield permutation

If you want to test if there is a permutation, you can do any(main(n)). If you want all permutations, you can do list(main(n))

For 9:

results = list(main_generator(9))

[(0, 2, 1, 6, 8, 7, 3, 5, 4),
 (0, 2, 1, 6, 8, 7, 4, 3, 5),
 (0, 2, 1, 7, 6, 8, 3, 5, 4),
 (0, 2, 1, 7, 6, 8, 4, 3, 5),
 ...
 (0, 8, 6, 5, 3, 2, 7, 1, 4),
 (0, 8, 7, 4, 6, 2, 5, 1, 3),
 (0, 8, 7, 5, 3, 6, 1, 4, 2),
 (0, 8, 7, 6, 5, 4, 3, 2, 1)]


Action to transmit queued messages over a socket

I'm not familiar with the libraries and types you're using here, but I see a few general Haskell things:

Instead of loopAction <- atomically $ ...; loopAction, you can use join $ atomically $ .... This removes the need for the entire outer do block, simplifying the blocks/indentation.
You're doing nearly-identical operations on m and q, just with different collection types. I would attempt to extract these to a higher-order function (e.g. one possible abstraction is modifyTVar, which doesn't exist in the library, but you can see how to write it analogously to modifyMVar), and perhaps define an ad-hoc typeclass to make Map and PSQ look the same.
Your case b of ... has two nearly identical branches! Unless there's some non-obvious polymorphism, then just combine them:

let send = case b of
    11 -> sendq
    72 -> sendqr
return $ case Map.lookup key mT of
    Nothing -> return ()
    Just messages -> send s (B.snoc (S.encode messages) b) (show a) 4711

let mT' = Map.delete key mT is redundant because the following insert will always overwrite that key anyway; just use Map.insert key [newmsgs] mT and never define mT'.
Assuming you're using Data.PSQueue, and the key already exists in the queue, you can use PSQ.adjust instead of a delete followed by an insert.

Taking all of my own advice except the last:

transMit :: Serialize a => Socket -> POSIXTime -> KEY -> a -> TPSQ -> TMap a -> IO ()
transMit s time key newmsgs q m =
    join $ atomically $ do
        modifyTVar_ q $ (PSQ.insert key time . PSQ.delete key)
        foo <- modifyTVar m $ \mT -> (Map.insert key [newmsgs] mT, Map.lookup key mT)
        let (address, protocol) = key
        let send = case protocol of
            11 -> sendq
            72 -> sendqr
        return $ case foo of
            Nothing -> return ()
            Just messages -> send s (B.snoc (S.encode messages) protocol) (show address) 4711

modifyTVar :: TVar a -> (a -> IO (a, b)) -> IO b
modifyTVar var f = do
    x <- readTVar var
    (x', r) <- f x
    writeTVar var x'
    return r
modifyTVar_ :: TVar a -> (a -> IO a) -> IO ()
modifyTVar_ var f = do
    x <- readTVar var
    writeTVar var (f x)

Further points about what I did:

Note I called a variable foo because I don't know what it makes sense to name it in your application, not because that's a good name.
I went and defined both modifyTVar and modifyTVar_; the latter is not needed but allows the action on q to be defined as a simple function composition. This is perhaps an excess of specialized utility functions, but I feel it is reasonable because the experienced Haskell programmer will understand what they do simply from seeing their names.
I renamed a and b so the variable names actually communicate what they are; this is better than writing a comment to explain them, and so I removed the comment.
I put join $ atomically $ do on a separate line so as to catch the reader's eye that this is not just a plain do block.


Do not compute things more often than you need to

The point of make_anagram_dict is to have a cache to perform some anagram check very efficiently. However, you rebuild it for each and every words which misses the point of having a cache.

By writing:

ana_dict = make_anagram_dict(words)
length = 0
for word in words:
    if len(word) == word_length:
        variants = blanagram(word, ana_dict)

things should be much faster already.

Strange loop

I must confess that for anagramdict[word] in anagramdict[word] really surprised me. What are you expecting from this loop?


Derived Class to keep track of "original strings" in std::regex and ease out their usage while coding

This looks like a bug:

Regex& Regex::Assign( const Regex& rhs )
{
    if ( this != &rhs ) return *this;  // <- ?

    m_str = rhs.m_str;
    m_match_nothing = rhs.m_match_nothing;
    regex::assign( rhs );

    return *this;
}

Presumably you want to abort the assignment early if:

if ( this == &rhs ) return *this;

At the moment, you only update the variables if the rhs is the same as the object receiving the call... which already has those variable values.


Object oriented, runtime expandable Sieve of Eratosthenes

Initialize your class members in the declaration

class Sieve {
public:
    unsigned expand(unsigned upperlimit);
    bool isPrime(unsigned n) const;
private:
    unsigned limit = 0;
    std::vector<bool> composite{};
};

is easier to read because you don't need to jump to the constructor to see the initial values and now you don't even need a constructor anymore.
In case you do write constructors and use the initialization list syntax it will override the declaration initialization for those members, so your constructors can concentrate on initializing members that are different than the default from the declaration, again increasing readability.

Limit your nesting depth

Sieve::expand is moving quite far to the right. A way to avoid that is to use early exits instead of nesting:

unsigned Sieve::expand(unsigned upperlimit) {
    if (upperlimit <= limit)
        return limit;
    composite.resize(upperlimit);
    composite[0] = true; // 1 is not prime
    const unsigned sqrtLimit = std::ceil(std::sqrt(upperlimit));
    // std::cout << "old limit = " << limit << ", new limit = " << upperlimit << "\n";
    for (unsigned i = 3; i <= sqrtLimit; i += 2) {
        if (composite[i / 2])
            continue;
        const unsigned start = i * std::max(i, (((limit + i) / i) | 1));
        // std::cout << "Starting with i = " << i << ", from " << start << "\n";
        for (unsigned j = start; j <= upperlimit; j += i + i) {
            composite[j / 2] = true;
            // std::cout << j << " is composite because it's a multiple of " << i << "\n";
        }
    }
    limit = upperlimit;
    return limit;
}

Declare variables as late as possible

Inside main you don't need m and n until you are inside the for loop.

Do at least basic input error checking

You wrote that the program expects valid input, but that is not really acceptable. When a user enters something that cannot be parsed as an int you read uninitialized memory which is undefined behavior and bad things happen.
if (!(std::cin >> m >> n)) return -1; at least gives your program defined behavior without much effort. You could also throw an exception or print a proper message and let the user try again, but having std::cin outside an if is usually a problem.

Better name-giving-ness

limit and upperlimit seem less nice than size and requested_size.
T is usually a template parameter and it probably shouldn't be the only variable name that is capitalized. m and n don't tell me much either. I would prefer T-> test_cases, n->upper, m->lower or something more descriptive.

Consistent parenthesis for ifs

Arguably it is a matter of style if you prefer

if (n > limit)
        throw std::range_error("n must be less than limit");

or

if (sieve.isPrime(i)) {
    std::cout << i << ' ';
}

but you should pick a style and stick to it.
Now, going through the code and adding brackets around one-line if-statements is a stupid waste of time, fortunately clang-tidy -fix *.cpp "-checks=-*,readability-braces-around-statements" -- -std=c++14 can automatically fix that for you.


The only thing I can think of that might make it more Pythonic would be to wrap the body of deleteUser() in a try and catch and handle the OperationalError rather than checking user and email for None before making the attempt.


There's no reason why you can't modify the data retrived from MySQL and still insert multiple rows at once into PostgreSQL, for example something like this:

fields = '''
customer_id customer_name contact_name address city postal_code country
'''.split()

cur_msql.execute("SELECT {} FROM mysqlcustomers".format(','.join(fields)))

rows = []
for row in cur_msql:
    if row['customer_name'] == NULL:
        row['customer_name'] = row['contact_name']
    rows.append(row)

params = '({})'.format(','.join('%s' for _ in fields))
query = "INSERT INTO psqlcustomers ({}) VALUES {}".format(
    ','.join(fields), ','.join(params for _ in rows))

cur_psql.execute(query, rows)


My perspective on your questions:

It breaks when you want to change the behavior of all cats, since you do not store functions in the .prototype of the classes. Also, not using .prototype makes your code consume more memory.
There is indeed a standard way of doing this, however it kind of breaks your requirement : "But also to have the class, constructor and inheritance in a one block, as OO languages usually do."
I think vanilla JS does better and is less contrived.

Most of the code I borrowed from this excellent text.

Your FourLegged class would be:

function FourLegged() {
   this.cry = "ffffhhhh!"; //default value
}
//Functions/methods should be part of prototype
FourLegged.prototype.talk = function talk(){
  writeln( this.cry);
};

If you absolutely insist on doing this in 1 block, you could do this :

var FourLegged = (function() {

  function f() {
     this.cry = "ffffhhhh!"; //default value
  }

  f.prototype.talk = function talk(){
    writeln( this.cry);
  };

  return f;
}());

Then the cat would be this ( wrapped in a function, not required )

var Cat = (function() {

  function f(name){

     FourLegged.call(this);

     (function nameProperty( o , name) {
       // private var defined in instance
       var _name = name;
       // methods that use private var - Douglas Crockford nethod
       o.setName = function(name) { _name = name; };
       o.getName = function() { return _name; };
    })(this , name);
  }

  f.prototype = new FourLegged();
  f.prototype.constructor = f;

  f.prototype.talk = function talk() {
    writeln( this.getName() + " says " + this.cry);
  };
  f.prototype.setCry = function setCry( s ) {
    this.cry = s;
  };

  return f;
}());

Your tests will perform exactly the same, except you do not need _parent, _heap, instance() or getConstructor().

Update

A clarification on 'it does not use memory'.When I create objects this way in the console in Chrome :

function FourLegged()
{
   this.CRY = "ffffhhhh!";
   this.talk = function() { writeln( this.CRY); };
}

I find that these object take 88 bytes. ( Take a head snapshot with developer tools ).

When I create objects the following way in the console in Chrome:

function FourLegged()
{
   this.CRY = "ffffhhhh!";
}
FourLegged.prototype.talk = function() { writeln( this.CRY); };

These objects take 48 bytes, and that is because the talk function remains on prototype, it does not get copied over the object.


Is this a good example of the composite pattern?

Am I implementing composite correctly.

Although you are doing it correctly, a part of me wonders if it was the correct solution for you problem. The reason I ask is because a interface usually means that each method that you define is going to need to be implemented differently for each sub class that implements it. In your case it doesn't matter what type of person you have they will always introduce themselves the same way, and they will always say their name the same way, and they will always tell their age the same way. That being said an abstract class would have been a better choice because the basics will be the same for both parent and child (and even if you decide to add another branch of a person) for things such as their name, and age. The introduction would possibly be the only exception if you added a culture to this mixture. Since different cultures introduce themselves differently in almost all scenarios that would be a good candidate for a interface.

  Is this a good piece of sample code?

The code appears to be in good order. It is easy to read and easy to understand. My only issue (and it is debatable) is having the age part of the constructor. In this context it makes sense, but when i first introduce my self to someone I don't say, "Hi my name is Robert Snyder, I'm 30 years old". Instead I just introduce my self as "Robert" but judging by the context of your code it might be fair to have the age (such as in the case of the entrance to a liquor store, they want your id which has your name and age)

  Thoughts on throwing an exception in leaf class for un-needed function implementations like add / remove Person?

No. Never. Only in the higher levels do you want that to happen. This forces the user to use child classes. If your child classes are "dangerous" then people will either make their own, or not use yours.


You should have included Open-InternetExplorer - without it, this code is incomplete and does not run.

A few of optimizations...

You will not need to Add-Type if all you need is the screen property. It might also make more sense to use Count rather than Length, afterall this code relies on the number of screens.
Unless the number of screens changes per iteration of the loop, getting this value could be taken outside the loop. Otherwise, consider a switch and remove $len altogether:

Switch ([System.Windows.Forms.Screen]::AllScreens.Count) {
   1 {
      ...
   }
   2 {
      ...
   }
}

The condition for the infinite loop while (1-eq1) can be simplified to avoid having to calculate a result. Use while ($true) or while (1).
When IE is started, the script does not use the return value so it isn't necessary to assign it to $ie:

$null = Open-InternetExplorer -Url http://...

The use of $ieis actually unnecessary in both cases.

The resulting script could look like:

write-host (get-date -format s) " Beginning script..."
do {
   Switch ([System.Windows.Forms.Screen]::AllScreens.Count) {
      1 {
         Get-Process -Name iexplore -ErrorAction SilentlyContinue | Stop-Process -Force
         If ($?) {
            Write-Host (get-date -format s) "Stopping IE..."
         }
      }
      2 {
         if ((Get-Process -Name iexplore -ErrorAction SilentlyContinue) -eq $null){
            write-Host (get-date -format s) " Starting IE..."
            Start-Sleep -s 5 #5
            $null = Open-InternetExplorer -Url http://localhost/ -Left 1366 -FullScreen -InForeground
            write-Host (get-date -format s) " IE Started"
         }
      }
   }
   start-sleep -s 10
} while(1)


Project Euler 357 prime number generator in Python 3

The most obvious improvement is to notice that if i in the_primes and if val not in the_primes both take \$\mathcal{O}(n)\$ time. If you made the_primes a set instead of a list it would only be \$\mathcal{O}(1)\$. So just write:

the_primes = set(primes())

Also note that your "prime number generator" is not actually a Python generator. For that you would need to add a yield:

def primes(ubound=10**5):
    """ Generating prime numbers https://stackoverflow.com/a/8627193/1770460 """
    size = (ubound - 3) // 2
    a = [False] * size
    s = 0
    while s < size:
        t = 2 * s
        p = t + 3
        yield p
        for n in range(t * (s + 3) + 3, size, p):
            a[n] = True
        s = s + 1
        while s < size and a[s]:
            s = s + 1

The set() call will then consume the generator. This will not make your code run faster, though (it needs about the same CPU) It might save you some memory, because you don't need the conversion from the internal list to the external set.

Your original code takes about 43.5s on my machine. With the two changes in this post this drops down to less than 1.7s:

$ python3 -m cProfile 357.py
275
time elapsed 1.6228196620941162 sec.
         583585 function calls in 1.623 seconds

   Ordered by: standard name

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
   ...
     9592    0.011    0.000    0.011    0.000 euler_357.py:14(primes)
        1    0.001    0.001    1.623    1.623 euler_357.py:3(<module>)
        1    1.524    1.524    1.584    1.584 euler_357.py:34(number_divisors)
        1    0.026    0.026    0.026    0.026 euler_357.py:56(find_count)
   ...


Finding the year using strtotime vs calculating manually

The first version, using strtotime(), is definitely more maintainable.  The code obviously says exactly what you intend.  I like it.

The second version is full of magic numbers (even though they are somewhat recognizable magic numbers), and is therefore not as clear.  I don't like it, but if you wanted to use it anyway, I suggest:

function school_started2_alt($years){
    $SECONDS_PER_YEAR = 365.25 * 86400;
    return date('Y', time() - ($years + 0.5) * $SECONDS_PER_YEAR);
}

If you are using PHP ≥ 5.3, also consider this formulation, which flows nicely from left to right:

(new DateTime())->sub(new DateInterval("P${years}Y6M"))->format('Y');

The kinds of strings accepted by the DateInterval constructor follow a rigid format, so the parsing should be efficient. Keep in mind that PHP itself is an interpreted language, so the work to parse a DateInterval specifier string is insignificant relative to the total amount of parsing involved. You should therefore take full advantage of PHP's built-in features to simplify your own code.


Option 1 - Use a temporary table

CREATE TEMPORARY TABLE foo (`id` INT, `variable` VARCHAR(255), PRIMARY KEY (`id`));

Insert the data into the primary table and do a normal JOIN.

Note that temporary tables are created in memory and are dropped when the connection is closed (when the php script is done).

Option 2 - Query and add to result

Assuming that $vars are key/value pairs of id/variable

$ids = join(',', array_keys($vars));
$result = $db->query("SELECT * FROM `fields` WHERE `id` IN ($ids)");

$data = [];
while ($row = $result->fetch_assoc()) {
    $variable = $vars[$row['id']];
    $data[] = $row + ['variable' => $variable];
}


I've used matchMedia() quite a bit in the past, it works quite well:
https://github.com/paulirish/matchMedia.js/

Example:

if (matchMedia('only screen and (max-width: 480px)').matches) {
  // smartphone/iphone... maybe run some small-screen related dom scripting?
}

Also instead of individually doing .addClass('class') and .removeClass('class') you could use .toggleClass('class').


I would consider having the css be in an external css file and javascript in an external js file since there is quite a bit of non-HTML code, but that is just my own preference.

I would get rid of extra whitespace and a lot of rework can be done with indentation to make the code more readable.

You can also group variable assignments together:

var video1 = document.createElement('video');
video1.src = 'images/video/trx.mp4';
video1.id = 'video1';
video1.autoplay = true;
video1.controls = true;

And group methods together:

document.getElementById("vidcont_2").appendChild(video2);
slide6();

I would consolidate

Article 0

Article 1

Article 2

Article 3

Additional Note: I would also improve the comments to be more descriptive and helpful.

 is unnecessary if you have proper indentation
//slide 5 right before var slide4 is confusing
If you keep HTML, CSS, and JavaScript in the same file I would comment each section as such.


Email subscription queries for a PHP website

If you are using the mysqli functions, then do not call mysql_real_escape_string(), which is part of the deprecated mysql suite of functions.  At the least, you should change it to call mysqli_real_escape_string() so that you are using the same suite of functions.

That said, using sprintf() to compose the query and escaping the string parameter yourself is not the best practice.  A more foolproof way would be to use prepared statements, such that the library takes care of any necessary escaping for you.

$select_query = $conn->prepare("SELECT * FROM … WHERE email = ?");
$select_query->bind_param('s', $email);
if (! $select_query->execute()) {
    die("Database error");
}
if ($select_query->num_rows > 0) {
    die("Already subscribed");
}

I wouldn't use a SELECT query at all to detect whether an e-mail address is already subscribed.  Rather, I would place a UNIQUE constraint on the email column, and catch any attempt to insert a duplicate.

Reasons for doing it that way are:

It's generally good practice to enforce such uniqueness constraints in the database schema.  It makes the schema more foolproof and self-documenting.  It also automatically creates an index on the email column, which could improve search performance.
If you enforce uniqueness in the PHP code rather than in the database, then you are vulnerable to a race condition.  If two requests come in at the same time, it's possible that one of them could manage to sneakily perform an insert in between the other request's select and insert.
Eliminating a SELECT query would make it more efficient.  You should be checking the result of the INSERT anyway — which you don't.  (Your code ignores $result2.)


Definitely not one of my proudest answers...
You can delegate the work of casting to another function

fun <T> Pair<*,*>.validate2(lambda: (T, T)->Boolean) = lambda(first as T, second as T)

In your validateHelperclass you can call this function:

fun <T> validateRangeHelper(pair: Pair<T,T>): Boolean = when(pair.first) {
    is Int -> pair.validate3<Int>(Validator::validateRange)
    is Double -> pair.validate3<Double>(Validator::validateRange)
    is Long -> pair.validate3<Long>(Validator::validateRange)
    is String -> pair.validate3<String>(Validator::validateRange)
    else -> throw UnsupportedOperationException()
}

(I called it validate2, as I created a custom class which has more than 2 parameters, such that I could test both the methods, based on  if i called validate2 or validate3...)

typeunsafe

//just to be able to get three
class TestValues<T : Any>(
    val min: T,
    val max: T,
    val input: T
)

I search the functions with a certain name and parameter count. I extracted it from a function, but of course you can type the name yourself. Note, you have to import kotlin.reflect.KFunction2.

inline fun <reified R : Any> createTester2WithInt(obj: R, f: KFunction2<Int, Int, Boolean>) =
    Tester2(
        obj,
        R::class.findMethodsWithType(f.name, 2)
    )

inline fun <reified R : Any> createTester3WithInt(obj: R, f: KFunction3<Int, Int, Int, Boolean>) =
    Tester3(
        obj,
        R::class.findMethodsWithType(f.name, 3)
    )

open class Tester(
    private val obj: Any,
    private val map: Map<KClass<out Any>, KFunction<Any?>>
) {
    @PublishedApi // means you shouldn't call it by yourself
    internal inline fun <reified T, reified S : Any> call(vararg args: S, clazz: KClass<out S>): T {
        val method = map[clazz] ?: throw java.lang.UnsupportedOperationException()
        return method.call(obj, *args) as T
    }
}

class Tester2(obj: Any, map: Map<KClass<out Any>, KFunction<Any?>>) : Tester(obj, map) {
    inline fun <reified T, reified S : Any> test(first: S, second: S): T = call(first, second, clazz = first::class)
}

class Tester3(obj: Any, map: Map<KClass<out Any>, KFunction<Any?>>) : Tester(obj, map) {
    inline fun <reified T, reified S : Any> test(first: S, second: S, third: S): T = call(first, second, third, clazz = first::class)
}

fun <R : Any> KClass<out R>.findMethodsWithType(
    name: String,
    parameterCount: Int
) = memberFunctions.asSequence()
    .filter { it.name == name }
    //the returnType is also a parameter
    .filter { it.parameters.size == parameterCount + 1 }
    .filter { it.returnType.jvmErasure == Boolean::class }
    .filter {
        val paramTypes = it.javaMethod!!.parameterTypes
        paramTypes[0] == paramTypes[1]
    }.associateBy { it.javaMethod!!.parameterTypes[0].kotlin }

fun create2Pred(pred: (Any, Any) -> Boolean) = pred

fun main() {
    val validator2 = createTester2WithInt<Validator>(Validator, Validator::validateRange)
    val validator3 = createTester3WithInt<Validator>(Validator, Validator::validateRangeInput)
    val list = listOf(
        TestValues(1, 1, 1),
        TestValues(1.0, 1.0, 1.0),
        TestValues("1", "1", "1")
    ).forEach {
        val a: Boolean = validator2.test(it.min, it.max)
        val b: Boolean = validator3.test(it.min, it.max, it.input)
        println("$a and $b")
    }
}

object Validator {
    fun validateRange(min: Int, max: Int): Boolean = true
    fun validateRange(min: Long, max: Long): Boolean = true
    fun validateRange(min: Double, max: Double): Boolean = true
    fun validateRange(min: String, max: String): Boolean = true

    fun validateRangeInput(min: Int, max: Int, input: Int): Boolean = true
    fun validateRangeInput(min: Long, max: Long, input: Long): Boolean = true
    fun validateRangeInput(min: Double, max: Double, input: Double): Boolean = true
    fun validateRangeInput(min: String, max: String, input: String): Boolean = true
}


try this code. It assumes that the worksheet with data is active.

Sub ReplaceCodeReview235023()
    With ActiveSheet.Columns("D")
       .SpecialCells(xlCellTypeBlanks).FormulaR1C1 = "=RC[-3]"
    End With
End Sub


Basic Entity Framework Repository implementation

Very nice and comprehensive. Your names are clear and short, your code is obvious in it's intent. I just see two things that startle me a bit:

if (context == null)
{
   return new Entities();
}
else
   return context;

what you do here is ... amusing. This code doesn't do what you think it does ;)

Both blocks are single line-statements, yet you put the first one in brackets and the second one you don't. This is inconsistent and prone to bugs. Let's say you want to log when someone accessed the context without a new instance created.

The "logical", but wrong code would be:

//if bla bla
else
    Logger.log("Context accessed.", Log.DEBUG);
    return context;

While this would work as planned if your if did what you seem to think it does, the actual flow of code is incorrect.

That whole thing should be rewritten as:

if (context == null)
{
    context = new Entities();
}
return context;

This does what you seem to have had in mind. In your code, you never assign to context, which remains null until all eternity. With each call to the getter you just return a new instance of Entities().

And the next thing that startles me is this:

public static List<OutboundModel> GetOutboundsByDate(DateTime dateFrom, DateTime dateTo)

in combination with the fact that your output is not sorted. Why? Using the Type List , especially in combination with the name, IMO implies a sorting of the data set. In this case your sorting is the Id, but for GetOutboundsByDate, I expect a data set that is ordered by date (and not by Id)

Instead I'd use IEnumerable<T>.


Convert a date string to a DateTime object in C#

Nesting if-blocks like you do is a way that leads to "arrow code", code that has deeply indented structures/blocks.

In your case, I will assume that you can't use the more simple 1-try TryParse(dateString, datetime) option, and that you have to try the invariant and local cultures in that order.

So, consider this code:

    DateTime dateTime;

    if (DateTime.TryParse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

    if (DateTime.TryParse(dateString, CultureInfo.CurrentCulture, DateTimeStyles.AssumeUniversal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

    if (DateTime.TryParse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeLocal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

Now the logic progression between your format attempts is clear. It also makes it easier to add a new attempt in there just by adding 4 lines.

If your attempts are not successful, though, your code pulls out a year. This code is broken because it does not actually return a datetime for valid years...

If the year is between min and max dates, it does not set the datetime at all...


First, a few points of the code you wrote. If you have an array of values, you can join the values together to efficiently create a string. This would eliminate the need for at least one of the reduce calls. Second, in a reduce call, the value of the second callback parameter (currentValue in your code) is the value of the array parameter at the index parameter (arr[currentIndex] in your code). Combining that with Javascript's capability to ignore excess function parameters, your reduce calls should take only two parameters, and use the currentValue in place of the arr[currentIndex].

You should also avoid using the same variable names in the same scope. Having two sets of total, currentValue, currentIndex, and arr could get confusing quickly, and lead to strange bugs.

Now, for the one-liner:

return this.grid.flat().map((el) => el ? "o" : "b").join("");

See Array#flat, Array#map, and the aforementioned Array#join. Of these, Array#flat is the newest and possibly unsupported method. It can be easily polyfilled or replaced. The MDN page shows some clever replacements like arr.reduce((all, row) => all.concat(row), []) and [].concat(...arr).


Efficiently finding approximate fraction, with tolerance for fp rounding

Convert the float to a Fraction:

>>> from fractions import Fraction
>>> Fraction(0.1 + 0.2)
Fraction(1351079888211149, 4503599627370496)

(this gives you the same ratio as float.as_integer_ratio), and then use the limit_denominator method to return the closest fraction with a denominator of limited size:

>>> Fraction(0.1 + 0.2).limit_denominator(1000000)
Fraction(3, 10)

If you're interested in the maths behind the limit_denominator method, see this answer on Stack Overflow.


Initializing object by settings implemented as a class

It depends on whether you want to handle config updates or not. In your case I guess you just want to work with data at constructing point. There are few options to do this:

Make copy of your settings before they are passed and use BankTerminalSettings inside the BankTerminal (as was considered). But personally I dont like the idea there are different BankTerminalSettings passing around.
Better approach would be just to inialize your internal fields with config settings, i.e. to copy settings of BankTerminalSettings to BankTerminal

Also I'm worried with the fact BankTerminalSettings contain more properties than is needed for BankTerminal. Maybe BankTerminalSettings should get another name? Anyway, I would create interface IBankTerminalSettings that contain only properties needed by BankTerminal and pass it to BankTerminal constructor


It's hard to comment on it. It seems OK to me - maybe a little to much list manipulation. I'm not sure if string concat with + is acceptable practice in F# but in general it's not. For fun I've tried with my own version with use of recursion:

let newDiamond n =
    let rec print nn f =
        if nn <= n && nn > 0 then
            let spaces = String.replicate ((n - nn) / 2) " "
            sprintf "%s%s%s\n%s" spaces (String.replicate nn "*") spaces (print (f nn) f)
        else
            ""
    let add x = x + 2
    let subt x = x - 2
    let dn = if n % 2 = 0 then 3 else 2
    sprintf "%s%s" (print 1 add) (print (n - dn) subt)

[<EntryPoint>]
let main argv =
    newDiamond 36 |> printfn "%s"
    Console.ReadLine() |> ignore
    0

Version with pattern match instead of if:

let newDiamond n =
    let rec print nn f =
        match nn with
        | x when x <= 0 || x >= n + 1 -> ""
        | _ -> let spaces = String.replicate ((n - nn) / 2) " "
               sprintf "%s%s%s\n%s" spaces (String.replicate nn "*") spaces (print (f nn) f)

    let add x = x + 2
    let subt x = x - 2
    let dn = if n % 2 = 0 then 3 else 2
    sprintf "%s%s" (print 1 add) (print (n - dn) subt)

It's possible but does it add any good?

EDIT:

A condensed solution could be:

let newDiamond n =
    let topNs = [1..2..n]
    let diamondNs = List.append topNs (topNs |> List.rev |> List.tail)

    let createLine x =
        sprintf "%s%s" (String.replicate ((n - x) / 2) " ") (String.replicate x "*")

    (diamondNs |> Seq.mapFold (fun strs x -> ("", sprintf "%s\n%s" strs (createLine x))) "") |> snd

However none of the solutions are very performant if diamond size is increased (> 2000) due to the concatenation of strings. In that respect a good old iterative solution is preferable:

let newDiamond n =
    let createLine = (fun x -> sprintf "%s%s" (String.replicate ((n - x) / 2) " ") (String.replicate x "*"))
    let topHalf = [ 1..2..n ]
    let indices = topHalf @ (topHalf |> List.rev |> List.tail)
    seq { for ln in indices do
            yield createLine ln } |> Seq.iter (printfn "%s")

A gentle "refactoring" of BadHeuristics code:

let rangeWithInverse max =
    List.zip [ 1 .. max ] [ max .. -1 .. 1]

let listAscDesc list =
    list @ (list |> List.rev |> List.tail) // tail instead of skip 1

let starsAndSpacesString (stars, spaces) =
    sprintf "%s%s" (String.replicate spaces " ") (String.replicate ((stars * 2) - 1) "*") // sprintf instead of ""+""

let diamond n =
    rangeWithInverse (n / 2)
        |> listAscDesc
        |> Seq.map starsAndSpacesString
        |> Seq.reduce (fun a b -> sprintf "%s\n%s" a b) // sprintf instead of ""+""


Loading images according to screen width

I have a few comments, which in no way substitute a full review.

Your indentation is inconsistent which makes your code unnecessarily hard to read.

Your comments are also inconsistent. I'm not going to prescribe you a commenting standard, but pick a style you like and stick with it.

Now we're talking about comments, the following is redundant:

var thisImage=images[i];  // Grab Current Image //

It's still clear what you're doing without the comment. It's commenting the 'what' of your code while you should only be commenting the 'why'. The 'what' is already obvious in this case, as it should be. You may be interested in reading Writing Readable Code, about readable code and what part comments play in this. Comments can be good, but they are often not necessary.

You're only using the width of the resolution. While most desktop screens use standard ratios, you seem to target mobile users. Mobile devices don't necessarily stick to standard ratios. For width your screen may be a small, but for height it could be a medium.


Split download file buffer by any number of threads

Utilizing Threads:

You are doing a number of things to slow down and serialize your code while the point of threads is to do things in parallel.

for idx in range(splitBy):
    byteRange = buildRange(int(sizeInBytes), splitBy)[idx]
    bufTh = SplitBufferThreads(url, byteRange)
    bufTh.start()
    bufTh.join()
    dataLst.append(bufTh.getFileData())

In the first line, you generate the full list of byte ranges, but then only use the the range for the specific index you are on. This only needs to be generated once, not every time you start a new chunk.

Then in the next three line, you create the thread, start it, and then immediately wait for it to finish. This means you are never running more than one thread at a time. This implementation will be slower than a single threaded implementation because of the over head involved.

On the last line you get the datafile and append all the bytes into your buffer. This is where the bottle-neck is. Request is just a class that contains the information describing what should be requested. urlopen is the function that actually makes the internet connection. Even if you weren't serializing your thread creation and execution, you still would not have a performance gain because the function that will take time is not part of the run method. You want to execute all the code in the run() method, then just access the resultant data after the thread has terminated.

The body of your main should be like this:

threads = []
byteRanges = buildRange(int(sizeInBytes), splitBy)
for idx in range(splitBy):
    t = SplitBufferThreads(url, byteRange)
    t.start()
    threads.append(t)

# all of the threads are downloading files at this point

for t in threads:
    t.join()

# file prep stuff ...

with open(fileName, 'w') as fh:
    for t in threads:
        fh.write(t.data) #where run() stores the data it read

Edit:
It seems like you need a better understanding of how threads work. When you start a thread (in the main thread), run() is executed in the context of the thread that was started. The main thread can then continue to execute other tasks while the other thread is executing in the background. Once run() terminates, nothing else executes in the thread.

The other key point is that the call to urlopen() is when the download begins to happen. If this is not done in the context of a separate thread, it provides no performance gains. This means it must be called from run() or a method run() calls. Otherwise, it will execute in the main thread.

The thread class should be implemented like this:

class SplitBufferThreads(threading.Thread):
    """ Splits the buffer to ny number of threads
        thereby, concurrently downloading through
        ny number of threads.
    """
    def __init__(self, url, byteRange):
        super(SplitBufferThreads, self).__init__()
        self.__url = url
        self.__byteRange = byteRange

    def run(self):
        req = urllib2.Request(self.__url,  headers={'Range': 'bytes=%s' % self.__byteRange})
        # here is where the file download happens within the context of the thread
        self.data = getFileData(req)

    def getFileData(self, request):
        return urllib2.urlopen(request).read()

buildRange() is going too much on one line.

lst.append('%s-%s' % (i, int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))))

Adding to a list, formating a string, and two mathematic equations. The equations are non-trivial and it is not immediately apparent what they are actually doing. Both equations should be broken out into separate functions.

Additionally, the if statement is deciding how the first and second values are calculated. It does not effect the string formating or how the list is mutated.

def buildRange(value, numsplits):
    lst = []
    for i in range(numsplits):
        if i == 0:
            first = i
            second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
        else:
            first = int(round(1 + i * value/(numsplits*1.0),0))
            second = int(round(1 + i * value/(numsplits*1.0) + value/(numsplits*1.0)-1, 0))
        lst.append("{}-{}".format(first, second))
    return lst

Once you do this, it becomes clear that second is not effected by the if statement. Now, we can make the function even simpler. Replacing the equations greatly improves the readability.

def buildRange(value, numsplits):
    lst = []
    for i in range(numsplits):
        first = i if i == 0 else start(i, value, numsplits)
        second = end(i, value, numsplits)
        lst.append("{}-{}".format(first, second))
    return lst

Naming:

lst is a bad variable name. We know it is a list from when the variable is initialized, but we don't know what it is meant to store.

buildRange() returns a list of things, yet the name makes it sound like it only makes one.

In my examples, first and second are also bad names, but I hadn't fully understood what the values represented until the code was cleaner.

SplitBufferThreads is a single thread, it should not be pluralized.

The first argument to buildRange() is value. The name of the variable that is passed in for this argument is sizeInBytes. Which one gives you a better idea about what the code is trying to do?

The Python convention for naming function is underscore_spaced, not camelCase.

You are mixing the meaning of a range and how it is used by the specific library you are using. If you changed the library you are using to make the specific request, you then need to change how you are constructing your ranges or parse out the values from the string.


Clean up allocated memory (everything newed should be deleted), or use smart pointers to do it for you. (Perhaps each tree node should own its children, and store them in unique_ptrs, and the parent should be a raw pointer).
There doesn't seem to be any benefit to overloading operator new for the Tree class.
Currently, the iterator is more of a sub-tree iterator, as it never returns to the parent. e.g. for a tree with edges: A->B->C and A->D if you start iterating at B, you will only iterate from B to C, and never reach D.
Iterators are normally defined outside of the corresponding container class. We can use an iterator typedef in the container to refer to the iterator class.
operator* should return a reference, not a pointer (operator-> should return a pointer).
We have !=, but no ==.
We have a pre-increment operator++, but not post-increment operator++.
It would probably be best to write unit tests for things that are expected to work, e.g.: *i (returns a modifiable reference), *i++ (returns a modifiable reference and then increments i to point at the next value), etc. This can be tedious, but is really the only way to ensure correctness.
The full requirements for a forward iterator are listed here (and in the linked pages), and many of them could be translated into tests cases. This iterator should arguably be a bidirectional iterator.
Standard containers also supply a const_iterator version (and reverse iterators, though that's usually simple to add).

To be honest, I'd suggest starting with something a bit simpler like an array class, and corresponding iterators.

Note that we can traverse a tree in many different ways (depth-first in-order, depth-first pre-order, depth-first post-order, breadth-first). For a full-featured implementation of this sort of thing, see the ASL Forest class.

Since we're iterating nodes, not values, we don't have to worry about "order", but we should still call the class something like tree_depth_first_iterator, to distinguish it from the alternative(s).


Approximating pi/4 using Wallis Product - SICP exercise 1.31

Is my first solution still a correct representation of the formula even if it evaluates two terms on each approximation? Or is it the first solution with the correct representation?

Although both solutions may be mathematically equivalent in theory, they are vastly different in practice.  The first uses floating-point constants, which leads to inexact math.  The second one uses exact math.

  How can I improve them?

In general, you should avoid using recursion directly when there are higher-level abstractions that fit.  For example, here are some ways you could restructure your code around streams:

Generate one stream for terms in the numerator and one stream for terms in the denominator.  Then use stream-map to divide successive terms.  Take the first n terms of the result, and reduce with +.
Produce a stream of successive approximations of \$π\$.  There are several ways to do this.  For example, you could use cons-stream, but it would also be possible (if you define a few auxiliary functions) to tweak the strategy outlined in the preceding point.  Then take the nth item of this stream.


You can optimize some queries.

var selected_file = document.getElementById("selected_file")

Unless your HTML changes considerably (replacing nodes periodically), every call to getElementById() will return the same node. You can access the node more efficiently if you cache the node in a local variable, and use the same local variable throughout your code, instead of looking it up multiple times.

Same with:

// Help
var help = document.getElementById("help_content"),
    overlay = document.getElementById("overlay");

As long as all these elements exist when your code initializes, you can lookup and store them in variables once, instead of each time Help() (etc) is called. In your code, the performance increase will be very minor, but it can add up in other situations.

In your keydown block:

if (key === 79) {
...
if (key === 83) {
...

// function rename()
(filename.lastIndexOf(".txt") === -1)

You can shave a few bytes by using == instead of ===. Since you're comparing primitives, not objects, == and === function equivalently.

Using === (instead of ==) is a good practice when you're not sure, but it is unnecessary in the places you are using it.

It looks like you didn't update your last check in the keydown callback

if (e.keyCode === 9 || e.which === 9) { // tab

Can be rewritten as:

if (key == 9) { // tab

function confirmNav() { // Confirm navigation
  if (isModified) {
      var a = window.confirm("You have unsaved changes that will be lost.");
      if (a) {
          isModified = false;
      ...

While nothing is technically wrong here, I'd either like to see the confirm inlined into the if() or a more descriptive variable name. a is meaningless.

 if (window.confirm("You have unsaved changes that will be lost.")) {
 // or
 var leavePage = window.confirm("You have unsaved changes that will be lost.");
    if (leavePage) {

Your code only has three long lines. The longest, at 136 chars:

var s = textarea.selectionStart
textarea.value = textarea.value.substring(0, textarea.selectionStart) + "\t" + textarea.value.substring(textarea.selectionEnd);
textarea.selectionEnd = s + 1;

I compacted it be using the local you already declared, and by adding a second. Now 97 chars:

var sStart = textarea.selectionStart,
    txt = textarea.value;
textarea.value = txt.substring(0, sStart) + "\t" + txt.substring(textarea.selectionEnd);

I don't think s was a bad variable name (s for selection or start is meaningful), but sStart is more clear.

You wrote some lovely, readable, and coherent JavaScript.

Here is everything:

var appname = " - simple notepad",
    textarea = document.getElementById("textarea"),
    help = document.getElementById("help_content"),
    overlay = document.getElementById("overlay"),
    selected_file = document.getElementById("selected_file"),
    prnt_helper = document.getElementById("prnt_helper"),
    untitled = "untitled.txt" + appname,
    filename = "*.txt",
    isModified;

document.title = untitled;

textarea.onpaste = textarea.onkeypress = function() {
    isModified = true;
};

function confirmNav() { // Confirm navigation
    if (isModified) {
        var leavePage = window.confirm("You have unsaved changes that will be lost.");
        if (leavePage) {
            isModified = false;
        }
    }
}

function New() { // New
    confirmNav();
    if (!isModified) {
        textarea.value = "";
        document.title = untitled;
    }
    textarea.focus();
}

function Open() { // Open
    confirmNav();
    if (!isModified) {
        selected_file.click();
    }
}

function loadFileAsText() { // load file as text
    var file = selected_file.files[0],
        fileReader = new FileReader();
    fileReader.onloadend = function(e) {
        if (e.target.readyState == FileReader.DONE) {
            filename = file.name;
            document.title = filename + appname;
            textarea.value = e.target.result;
            textarea.focus();
        }
    };
    fileReader.readAsText(file);
}

function rename() { // Rename
    filename = window.prompt("Name this note:", filename);
    if (filename) {
        filename = (filename.lastIndexOf(".txt") == -1) ? filename + ".txt" : filename;
        document.title = filename + appname;
    }
}

function Save() { // Save
    rename();
    if (filename) {
        var blob = new Blob([textarea.value], {
            type: "text/plain;charset=utf-8"
        });
        window.saveAs(blob, filename);
        isModified = false;
    }
    textarea.focus();
}

function Print() { // Print
    prnt_helper.innerHTML = textarea.value;
    window.print();
    prnt_helper.innerHTML = "";
    textarea.focus();
}

function Help() { // Help
    function closeHelpAndOverlay() {
        help.setAttribute("aria-hidden", "true");
        overlay.setAttribute("aria-hidden", "true");
        textarea.focus();
    }

    if (help["aria-hidden"] == "true") {
        closeHelpAndOverlay();
    } else {
        help.setAttribute("aria-hidden", "false");
        overlay.setAttribute("aria-hidden", "false");
        textarea.blur();
        overlay.onclick = function() {
            closeHelpAndOverlay();
        };
        // document.onkeydown = function(e) { // esc to close help
        //     if (e.keyCode == 27 || e.which == 27) {
        //         closeHelpAndOverlay();
        //     }
        // };
    }
}

// Confirm close
window.onbeforeunload = function() {
    if (isModified) {
        return "You have unsaved changes that will be lost.";
    }
};

// Keyboard shortcuts
document.onkeydown = function(e) {
    var key = e.keyCode || e.which;
    if (e.ctrlKey) {
        if (e.altKey) {
            if (key == 78) {
                // Ctrl+Alt+N
                New();
            }
        }
        if (key == 79) {
            // Ctrl+O
            e.preventDefault();
            Open();
        }
        if (key == 83) {
            // Ctrl+S
            e.preventDefault();
            Save();
        }
        if (key == 80) {
            // Ctrl+P
            e.preventDefault();
            Print();
        }
        if (key == 191) {
            // Ctrl+/
            e.preventDefault();
            Help();
        }
    }
    if (key == 9) { // tab
        e.preventDefault();
        var sStart = textarea.selectionStart,
            txt = textarea.value;
        textarea.value = txt.substring(0, sStart) + "\t" + txt.substring(textarea.selectionEnd);
        textarea.selectionEnd = sStart + 1;
    }
};


Taking a list and using it to build columns

You could do something like this (elegant, but not the fastest approach)

item_list = [1, 2, 3, 4, 5, 6, 7, 8, 9]
min_height = len(item_list) // 4
mod = len(item_list) % 4

heights = [min_height + 1] * mod + [min_height] * (4 - mod)
indices = [sum(heights[:i]) for i in range(4)]
columns = [item_list[i:i+h] for i, h in zip(indices, heights)]

Or a compressed version:

columns = [item_list[
    min_height * i + (i >= mod) * mod :
    min_height * (i+1) + (i+1 >= mod) * mod]
    for i in range(4)]

I guess the fastest way would be similar to what you already have:

index = 0
columns = []
for i in xrange(4):
    height = min_height + (i < mod)
    columns.append(item_list[index:index+height])
    index += height

(With both of these, if there are less than 4 items you get empty columns at the end.)


Logically, the code looks correct but from performance perspective, there are few things that can be improved. Let's look at the 2 points:

Time Complexity: O(n) - 1 loop, 1 reduce and 1 Math.min. All run in O(n) time.
Space Complexity: O(n) - Two arrays - defArr and l which have size n

There is nothing can be done for time complexity but space complexity can be reduced to constant space. No need to have additional arrays. One can simply loop and store intermediate values in temporary variables.

I have re-written the same. Inside the loop, at each index, the difference is calculated and compared to a min variable. If the new min is lesser than existing one; then update min otherwise do next iteration.

A = []
A[0] = 3
A[1] = 1
A[2] = 2
A[3] = 4
A[4] = 3

function solution(A) {
  let sum = A.reduce((total, value) => total + value, 0)
  let min = Number.POSITIVE_INFINITY
  let cumulativeSum = 0
  for (let i = 0; i < A.length - 1; ++i) {
    cumulativeSum = cumulativeSum + A[i]
    sum = sum - A[i]
    diff = Math.abs(sum - cumulativeSum)
    if (diff < min) {
      min = diff
    }
  }
  return min
}

console.log(solution(A))

Hope it helps. Revert for any doubts/clarifications.


I can't see a way to make this shorter (I can't see how is this relevant to any performance issue in Python) as your solution is straight forward but I have some advice:

Use calendar module to get rid of those ugly conditions.

calendar.isleap(year)

Formatting:

Try formatting your strings using format() (it's much cleaner looking):

This:

print("%s is a leap year") % (year)

Would become this:

print('{} is a leap year!'.format(year))

In your method, add another parameter which will be the number of years (this will make your code more user-friendly) - you don't want somebody to modify the number of leap years inside your block code. Just make it happen when they are calling the method:

def loop_year(year, number_of_years):
    ...

More, your variable names should have specific names based on their usage: I would personally change x to leap_year_counter. When it comes to a simple counter, your naming solution it's also ok, but if your code will become more robust, you'll have a hard time figuring what is what.

IF/ELSE Statements:

As others mentioned, you have year += 1 in your both statements, so just move it after them. More, in this case, you can also remove the else statement.

# ...
if calendar.isleap(year):
    print('{} is a leap year!'.format(year))
    leap_year_counter += 1
year += 1
# ...

Finally, your method will look like this:

import calendar

def loop_year(year, number_of_years):
    leap_year_counter = 0
    while leap_year_counter < number_of_years:
        if calendar.isleap(year):
            print('{} is a leap year!'.format(year))
            leap_year_counter += 1
        year += 1
loop_year(2016, 20)


Parsing objects from complex String in Java

My approach would start by looking something like the code below. Obviously whatever you wind up with should be in its own class/method to do the mapping from String[] to Token.

public final class Test {

    private static final int TYPE_COLUMN = 1;
    private static final int NAME_COLUMN = 3;
    private static final int PARENT_COLUMN = 5;
    private static final int SOURCE_COLUMN = 7;

    public static void main(final String[] args) {
        final String[] lines = {
                "@1      var_decl         name: testStr  parent: @7",
                "                         srcp: Auto.cpp:6         "
              };

        final String[] values = (lines[0] + lines [1]).split("\\s+");
        System.out.println(All Values:" + java.util.Arrays.toString(values));
        System.out.println("Type: " + values[TYPE_COLUMN]);
        System.out.println("Name: " + values[NAME_COLUMN]);
        System.out.println("Parent: " + getParent(values[PARENT_COLUMN]));
        System.out.println("Source: " + values[SOURCE_COLUMN]);

    }

    private static String getParent(final String parentId) {
        return "Parent";
    }
}


Join strings split with the python [::n] method into one string as they were before in one line

First, in order to make this even remotely readable, let's convert it to a function and save intermediate results (especially the reused ones) to variables:

def split_join6(table):
    cols = max(map(len, table))
    justified = ''.join([col.ljust(cols) for col in table])
    y = ''.join(justified[i::cols] for i in range(cols))
    return y.replace(' ','')

Now, what you seem to want is similar to the roundrobin recipe from itertools:

from itertools import cycle, islice

def roundrobin(*iterables):
    "roundrobin('ABC', 'D', 'EF') --> A D E B F C"
    # Recipe credited to George Sakkis
    num_active = len(iterables)
    nexts = cycle(iter(it).__next__ for it in iterables)
    while num_active:
        try:
            for next in nexts:
                yield next()
        except StopIteration:
            # Remove the iterator we just exhausted from the cycle.
            num_active -= 1
            nexts = cycle(islice(nexts, num_active))

x = ['fb','oa','or']
print("".join(roundrobin(*x))
# foobar

Note that making things into one-liners can only get you so far. It does sometimes help you to learn some new concepts in a language, but quite often it makes your code unreadable. In Python you should keep your lines to 80 or 120 characters (as per Python's official style-guide, PEP8). Anything that does not fit into that is probably too complicated to understand again, even a month later.

That being said, here is a shorter one-liner, albeit with one needed import:

from itertools import zip_longest

f = lambda x: "".join(map(lambda t: "".join(filter(None, t)), zip_longest(*x)))

f(['fb','oa','or'])
# 'foobar'

The zip_longest and filter(None, ...) are only needed in case not all parts are the same length. Otherwise (which is at least true for "foobar") it would just be:

f = lambda x: "".join(map("".join, zip(*x)))

Both use the well-known trick of doing zip(*iterable) to transpose an iterable of iterables.


First off, I don't see a real need for these two variables:

let valueMod3 = value % 3
let valueMod5 = value % 5

To me, they just seem a little unnecessary. In fact, writing out the expression value % 3 or value % 5 takes the exact same amount of keystrokes as writing valueMod3 or valueMod5.

Next, I'd highly recommend that you get into the habit of using Match Expressions rather than plain old if statements, as it's more functional, and will give you more benefits, like Pattern matching, over if statements. This means that this small chunk of if statements:

if valueMod3 = 0 && valueMod5 = 0 then yield "FizzBuzz"
else if valueMod3 = 0 && valueMod5 <> 0 then yield "Fizz"
else if valueMod3 <> 0 && valueMod5 = 0 then yield "Buzz"
else yield value.ToString()

Would become a Match Expression, like this:

match n with
| n when n % 3 = 0 && n % 5 = 0 -> yield "FizzBuzz"
| n when n % 3 = 0 -> yield "Fizz"
| n when n % 5 = 0 -> yield "Buzz"
| _ -> yield n.ToString()

After all these improvements to your code, I ended up with the following, working code:

let fizzbuzzSequence = seq {
    for n in 1 .. 100 do
        match n with
        | n when n % 3 = 0 && n % 5 = 0 -> yield "FizzBuzz"
        | n when n % 3 = 0 -> yield "Fizz"
        | n when n % 5 = 0 -> yield "Buzz"
        | _ -> yield n.ToString()
}

[<EntryPoint>]
let main argv =
    for item in fizzbuzzSequence do
        System.Console.WriteLine(item)

    0


Imports should be on different lines.

import glob
import re

'lis' should be removed, due to it not being used.
If you did use it, it should be renamed, a descriptive name is the best.

when opening a file use with.

with open(...) as aa:
    ...

This automatically closes the file, even if there is an unhandled exception. So if for example you keyboard interrupt the program, the file will close.

Also change the variable name aa to something better. file_handler is better.

You overwrite aa. This means that you cannot close it.
You should always close a file handler.

You could not overwrite aa by changing the for statement.

for sa in (b.split("\t") for b in aa.readlines()):

Python's style guide, PEP8, says that all operators should have a space either side.

fi = open(sa[1].strip() + ".txt", "a")

Currently you use three styles to, one of which is allowed.
The following one line shows two of them. fi= o )+"

fi= open(sa[1].strip()+".txt","a")

Your try statement is massive. It is also a bare except. Both being very bad.

You should try to keep the try as small as possible.
This is so that you don't 'mask' bugs.

try:
    file_handler = open(sa[1].strip() + ".txt", "a")
except:
    print "Error on opening " + a
else:
    ...
finally:
    file_handler.close()

You could remove the print statements. They take a surprising amount of time to print.

You should wrap you main in a if __name__ == '__main__':

Performance

Your massive amount of white-stripping may be the problem.
And you are opening a lot of files.

The former you can deal with.

text = soup.get_text()
lines = (line.strip() for line in text.splitlines())
chunks = (phrase.strip() for line in lines for phrase in line.split("  "))
text = ' '.join(chunk for chunk in chunks if chunk)

First off this can be simplified.

text = ' '.join(
    chunk
    for chunk in (
        phrase.strip()
        for phrase in line.strip().split("  ")
        for line in soup.get_text().splitlines()
    )
    if chunk
)

You strip every time you split. This can't be good.

It seems that you want to replace all blocks of white space with one space.
If this is true then the below function should be simpler and should be faster.

def strip_entire(string):
    def inner():
        white_space = False
        for char in string:
            if char in ' \t\n':
                if white_space:
                    continue
                white_space = True
                yield ' '
            else:
                white_space = False
                yield char
    return ''.join(inner())

It is \$O(n)\$. And should be faster than yours.

If you are likely to open the same file multiple times,
then you could save the output to another file.
Say a + ".stripped"

This will have the benefit of being able to reduce the amount of duplicate work.
This is good as then your computer can effortlessly append the file to the other file.

with open(sa[1].strip() + ".txt", "a") as file_handler:
    if os.path.exists(a + ".stripped"):
        with open(a + ".stripped", "r") as stripped_handler:
            file_handler.write(stripped_handler.read())
    else:
        # Other method

If you were to make all these changes you should get

import glob
import re
import unidecode
import os.path
from bs4 import BeautifulSoup

def strip_entire(string):
    def inner():
        white_space = False
        for char in string:
            if char in ' \t\n':
                if white_space:
                    continue
                white_space = True
                yield ' '
            else:
                white_space = False
                yield char
    return ''.join(inner())

def append_file(append_name, html_name):
    with open(append_name, "a") as file_handler:
        if os.path.exists(html_name + ".stripped"):
            with open(html_name + ".stripped", "r") as stripped_handler:
                file_handler.write(stripped_handler.read())
        else:
            with open(html_name, "r") html_handler:
                soup = BeautifulSoup(html_handler)
                # If you can't have the first line. No matter what.
                #soup = BeautifulSoup(''.join(html_handler.readlines()[1:]))
            for to_remove in soup(("script", "style")):
                to_remove.extract()
            file_handler.write(
                unidecode.unidecode(
                    re.sub("\s+", " ", strip_entire(soup.get_text()))) + "\n")

if __name__ == '__main__':
    with open("C:\project resource\lookup.txt", "r") as file_handler:
        print("started")
        dump_files = glob.glob("C:\\project resource\\dump\\*.html")
        for split_line in (b.split("\t") for b in file_handler.readlines()):
            for dump_file in dump_files:
                if split_line[0] == "".join(re.findall("_(\d+).", dump_file)):
                    append_file(split_line[1].strip() + ".txt", dump_file)
                    break

I don't fully understand what your regex's do.
As I don't fully understand the re library.
They seem kinda odd, re.findall("_(\d+).", dump_file),
I thought you only had one number on your files that followed that regex.
And so I decided to leave them as is.

I also removed the try except statement, as I didn't know what you were error handling. Was it your loops, your opens, everything, keyboard interrupts. This is one reason you shouldn't have bare except statements.

If you were doing the opens then you should do this:

try:
    file_handler = open(...)
except IOError:
    # Raise an error, break, return, whatever.
else:
    # Code in the with
finally:
    file_handler.close()


Generic Null/Empty check for each property of a class

First of all I'd reduce indentation. It makes your code really too hard to read. Doing that you will see it may be simplified.

First of all try/catch: a catch-all block where you rethrow the exception is useless (and you may even throw away line number information.)

This code:

if (obj != null)
{
    // Do something...
}

May be replaced simply with:

if (obj == null)
    return false;

You do not need to declare a local variable for result, simply return its value where appropriate.

You do not have to check condition result == false inside your foreach loop (you will still enumerate through all properties), simply break the loop and exit immediately.

You do not need to use to dynamic, when casting you have the right type at compile-time. Also move all this code inside a separate function:

private static bool IsNullOrEmpty(object obj) {
}

Let's see how to fill this function. Let's read the value of the property:

object value = pi.GetValue(obj);

First of all a common case, valid for all reference types:

if (Object.ReferenceEquals(value, null))
    return true;

Then explicitly check for strings:

if (value is string && String.IsNullOrEmpty((string)value))
    return true;

For Int32 (note that in your original code value <= 0 ? true : false is redundant it may be simplified to value <= 0). BTW are you sure you want to consider negative numbers as empty values?!

if (value is int)
    return ((int)value) <= 0;

Boolean does not need to be checked in this case (null case has been already handled). We just need to work out Guid:

if (value is Guid)
    return ((Guid)value) == Guid.Empty;

That's all. Note that now or in future you may want to check for Nullable<T> and or other types. With the exception of strings and negative integer numbers you can also do something like this:

if (Object.ReferenceEquals(value, null))
    return true;

var type = value.GetType();
return type.IsValueType
    && Object.Equals(value, Activator.CreateInstance(type));

What left in your original function? In this example let me use simplified check (negative numbers are not empty and I do not check for String.Empty against strings however it handles nullable types), feel free to pick the one works for you:

public static bool IsAnyNullOrEmpty(object obj)
{
    if (Object.ReferenceEquals(obj, null))
        return true;

    return obj.GetType().GetProperties()
        .Any(x => IsNullOrEmpty(x.GetValue(obj)));
}

private static bool IsNullOrEmpty(object value)
{
    if (Object.ReferenceEquals(value, null))
        return true;

    var type = value.GetType();
    return type.IsValueType
        && Object.Equals(value, Activator.CreateInstance(type));
}

Last note: if working with databases you may also want to check equality with DBNull.Value.


Program for activity selection exceeds time limit

The problem is that you are erasing elements from prog and then without bounds checking access it with prog[j]. That is sure to fall over.

Your end_time function is commonly known as operator< So you should use the appropriate name.

There is no comment on the significance of 1 and why a program should not be reordered when it starts at 1. It seems that different programs with starting time 1 should still have an ordering.

You are inconsistent in your loops. Once goes from 1 to <= participants the other goes from 0 to < participants. The latter is the commonly used one.

The first loop seems to sort for start time? You can achieve the same if you create a proper operator<.

using namespace std; is bad practice. There is no benefit to it other that you do not really know what is standard and what not, which is actually bad. Also it isprone to name clashes. Use what you need and not everything C++ has in the box


Class that implements a queue using two stacks

Unfortunately your code does not implement a queue. It is relatively easy to produce circumstances which break the logic. Consider the following, for example:

MyQueue<String> q = new MyQueue<>();

q.add("a");
q.add("b");
q.remove(); // should be "a", and is "a".
q.add("c");
q.remove(); // should be "b", but is "c".
q.remove(); // should be "c", but is "b".

Using two stacks to implement a queue is not a logical data structure as far as I am concerned. There are many better ways.


First: a general observation - simple nested if statements are equivalent to a single if with statements joined by and.

So:

if a:
    if b:
        if c:

Is equivalent to:

if a and b and c:

Second: You have an if test to see if business_name and website exist - but you do a lot of other things before this test. You can move this higher up so you 'escape' that code faster if you do something like:

item['business_name'] = business.css('div.info.l h2 a::text').extract()
item['website'] = business.css('a.s_website::attr(href)').extract()
if item['business_name'] and item['website']:
    for x in item['business_name'] and item['website']:
        #new code here, call to self.seen_business_names
        if (x not in self.seen_business_names and x not in self.seen_websites):
            item['phone_number'] = business.css('div.phone-number::text').extract_first()
            self.seen_business_names.append(x)
            yield item

(I've put the append before the yield as this seems slightly clearer to me).

Third - I don't think you mean for x in item['business_name'] and item['website']:

Rather can do either of:

for x in item['business_name'] + item['website']:

Or:

from itertools import chain
for x in chain(item['business_name'], item['website']):


ReactiveX observable that uses another observable

I had to use flatMap to achieve what I wanted:

public get(url: string): Observable<Response> {
  return this
    .login()
    .flatMap(token_id => this._http.get(url));  // token_id can be used here
}

public login(): Observable<string> {
  const authBody =
      {
          "client_id": "...",
          "username": "...",
          "password": "...",
          // ...
      };
  const headers = new Headers();
  headers.append("Content-Type", "application/json");

  return this._http
    .post('AUTH_URL', authBody)
    .map(response => response.json().token_id);
}


Write a Java class "Sort" which implements the BubbleSort algorithm

No unnecessary math

        if (leftIdx < 0 || leftIdx > list.length - 1
                || rightIdx < 0 || rightIdx > list.length - 1)

You could simplify this to

        if (leftIdx < 0 || leftIdx >= list.length
                || rightIdx < 0 || rightIdx >= list.length)

Because if either Idx is equal to list.length, it is greater than list.length - 1.

Don't do extra work

        for (int i = list.length; i > 1; i--)
        {
            for (int j = leftIdx; j < rightIdx; j++)

You're doing more passes than necessary.

        for (int i = rightIdx; i > leftIdx; i--)
        {
            for (int j = leftIdx; j < i; j++)

This works by bubbling the largest element to the end.  Once that's done, we don't have to review the end again.  So we can decrease i, which points to the end.

Bubble Sort best case

        for (int i = list.length; i > 1; i--)
        {
            for (int j = leftIdx; j < rightIdx; j++)
            {
                if (list[j] > list[j + 1])
                {
                    int tmp = list[j];

                    list[j] = list[j + 1];
                    list[j + 1] = tmp;
                }
            }
        }

One of the advantages of bubble sort is that it is possible to get a best case of linear time.  However, this code doesn't do that.  It's always quadratic, regardless of whether the data is already sorted.

Consider

        for (int i = rightIdx; i > leftIdx; i--)
        {
            Integer tmp = null;

            for (int j = leftIdx; j < i; j++)
            {
                if (list[j] > list[j + 1])
                {
                    tmp = list[j];

                    list[j] = list[j + 1];
                    list[j + 1] = tmp;
                }
            }

            if (tmp == null)
            {
                break;
            }
        }

Now if we never swap, then tmp is null and we stop processing.  If the list is sorted, that happens on the first outer iteration and we only go through the list once.  If the code is almost sorted but not quite, it could happen after just a few iterations.

If you add data at the end a lot, note that reversing the sort so that it bubbles the smallest forward rather than the largest back would help.

StringBuilder

        String ret = "";

        for (int i = start; i <= end; i++)
        {
            ret += list[i] + ", ";
        }

        return "[ " + ret.substring(0, ret.length() - 2) + " ]";

This should use a StringBuilder instead of string concatenation.

        String Builder ret = new StringBuilder("[ ");

        for (int i = start; i < end; i++)
        {
            ret.append(list[i]).append(", ");
        }
        ret.append(list[end]).append(" ]");

        return ret.toString();

Now rather than creating new strings each time, it just expands the StringBuilder as needed.

I also changed it so that the substring call is unnecessary.  Since we're manually iterating anyway, we can just not do the last element inside the loop.  That way we can handle it separately outside.

The substring trick makes more sense when we're automating the iteration such that we can't control the bounds.  But that's not the case here.


Array to map based on object property containing array items

Overall Your code is quite reasonable, after fixing something that looks to be a typo. The transformation that needs to be applied to the data structure isn't entirely trivial, so the logic that needs to be implemented requires a handful of lines of code.
Typo? But your code also has a typo/bug: the map.set(phone, r.name); will set the value in the map to be a string, not an array. You probably meant to do map.set(phone, [r.name]); (maybe a miscopy?) There's also map: Map = new Map();, which isn't valid syntax in JS, remove the : Map part.
Conditional operator If you wanted to shorten your existing code, see how you're using .map.set(phone, someExpression) depending on a condition - this is a sign that you can use the conditional operator instead of the if/else.
map.set(phone, (map.get(phone) ?? []).concat(r.name));

But while that makes the code shorter, is it easier to read? I'm not entirely convinced. Maybe you'll like it, or maybe you won't.
There are a few other things that can improve the code quality:
Declare your variables Doing for (r of result) will implicitly create a global r variable and put it on the global object. If you're running in strict mode, this will throw an error too. Always declare variables before using them, preferably with const.
Destructure? With for (r of result) or for (const r of result), you create a variable named r, which isn't so intuitive - what's an r? Maybe destructure the name and phones properties immediately.

const result  = [
 {
   phones : ["ABC", "DEF"],
   name: "Simon"
 },
 {
   phones : ["ABC", "XZY"],
   name: "John"
 }
]
const map = new Map();
for (const { phones, name } of result) {
  for (const phone of phones) {
    map.set(phone, (map.get(phone) ?? []).concat(name));
  }
}
console.log([...map]);


Hackerrank problem -"Another Prime Problem"

An immediate problem is that is_prime() is expensive, and you call it too many times. Prepare a sufficiently large list of primes in advance (and use a sieve for that).

This will give you a certain performance boost, but I am positive it will not be enough to avoid timeouts. The real problem is with the algorithm: you try to factorize the numbers, and the factorization is hard.

Instead of bruteforcing, do the math homework: how many numbers in the range have a prime \$p\$ as a factor? Hint: if there are N of them, they'd contribute N * p to the answer.


Simple optimizations:

By reading the whole file into text, then splitting the text into a word list, you will need enough memory to hold two copies of the file, which might be huge. Look into line-by-line input methods.

Many strings, after a quick check, don't need to be seen ever again.

The off-by-one-letter test does not ALWAYS need to process every letter in the words.

You never actually have to find the target word in the word list to know you have a solution (you know it's in there from the problem description). Some simpler solutions don't even need to consult the word list.

Deeper optimizations:

If you had a way to test which "next" words were more likely to lead to a solution, you might be able to defer other alternatives to explore those first, for a faster average solution time.

It MIGHT make more sense to solve the problem working backwards or both ways at once.

It MIGHT be worthwhile to keep stats on different words you find and/or to place words that seem like they might be useful later in some kind of separate structure.


There is a serious error in your code. If the function is called as even_digits(0), then it causes the following error:

ZeroDivisionError: integer division or modulo by zero

since in the first if you divide by count which is 0.

So, before improving the code, you should check if it is correct.

ADDED:

THE CASE OF n = 0

It is interesting to understand what should be the result in case the input (that is n) is 0.

The problem is labeled as:

  Percentage of digits in n that are even

and the natural interpretation is:

  Percentage of digits in the decimal representation of n that are even

So, what result should give the function when provided with the value 0? I argue it should be 1, since every decimal representation of 0 has only zeroes as digits, so it has a 100% even digits.


Determine what hazard type does the building belong using GeoDjango Intersects

I don't have a lot of experience with GeoDjango specifically, so there are some parts here that I can't review, but there are some spots that stand out as needing improvement.

The performance issues in your view

Any requests that aren't a GET will trigger a 500 error.
Your logic tends to repeat iteself when it doesn't need to.
You are triggering a ton of queries to generate the response.

Non-GET requests trigger a 500 error

Your floodhazard_tbl view specifically checks for a GET request and then returns the list of results based on that, but it doesn't actually return anything if a different request type is sent. Because of this, Django will trigger an exception (because no response was returned) which will trigger a 500 error for any non-GET response. You should instead return a HttpResponseNotAllowed response for any non-GET request, which is the equivalent of a 405 response.

Walking through how the response is generated

Right now you are going through the follow steps to get the buildings that intersect a specific type of hazard:

Filter the FloodHazard objects by the specific type.
Get all of the ids of the filtered objects.
You go through the list of ids, one-by-one, and do the following:

Retrieve the FloodHazard object for the id.
Get all of the PolyStructure objects which intersect with it.
Add the ids of those PolyStructure objects to a master list of PolyStructure ids for the type.
Add a serialized version of these PolyStructure objects to a master list of results.

You'll notice that steps 2 and 3.1 can cancel themselves out. And because you are getting the FloodHazard object again, you are causing a N+1 query at 3.1 because the object has not been cached. There is also a N+1 query happening at 3.2 when you get the PolyStructure objects.

The database queries that are triggered

Now, you have multiple N+1 queries happening in the body of your view that is likely leading to the increased response time. When working with a large number of objects this can be the slowest part of your response, as each query adds a small amount of overhead that can quickly build up into a 5+ second response.

So, to give you a rough idea of how many queries are being executed here, lets take a count

3 queries are the beginning for getting the list of FloodHazard objects for each type.

Note that this retrieved ids for n objects.

For each FloodHazard object you are

Getting the same FloodHazard object (n extra queries, n + 3 so far)
Getting the intersecting PolyStructure objects (n extra queries, 2n + 3 so far)
Getting the ids of the PolyStructure objects (n * (2/3) extra queries, one for each type excluding "Low")

So while the 2.3 doesn't trigger an extra n queries, depending on the number of "Low" FloodHazard objects there could still be a significant number of queries triggered.

After adding this all up, you will see that roughly 3 queries are triggered for each FloodHazard object in the original query. So if there were 100 "High" FloodHazard objects, you would be triggering 303 queries to generate the response.

How things can be improved

There are four easy fixes that can be made to significantly drop the number of queries made, which will end up improving performance in the long run.

Fixing the 500 error on non-GET requests

This one is the easiest fix, as it just requires you to return a HttpResponseNotAllowed at the end of your method.

def floodhazard_tbl(request):
    from django.http import HttpResponseNotAllowed

    if request.method == "GET":
        # We'll get to this
        pass

    return HttpResponseNotAllowed(['GET'])

So now if anyone makes a POST (or anything other than GET) request, they will get a 405 response with the Allow header set to GET.

Cutting out the duplicate FloodHazard queries

Right now you are retrieving a list of ids of FloodHazard filtered down to specific types, and then you are going through the list of ids and making a query to get the FloodHazard object back. Since you don't actually do anything else with the ids of these FloodHazard objects, you can skip the call to values_list and just use the original query when iterating over the list of objects.

high_hazards = FloodHazard.objects.filter(hazard='High')
medium_hazards = FloodHazard.objects.filter(hazard='Medium')
low_hazards = FloodHazard.objects.filter(hazard='Low')

for hazard in high_hazards:
    pass

for hazard in medium_hazards:
    pass

for hazard in low_hazards:
    pass

So now instead of re-querying the FloodHazard objects, you will have it available as hazard in each iteration. You can still get the geometry as hazard.geom, and you cut out n queries without changing the behaviour of the view.

Now you're down from 303 queries to 203 queries, just by making the one simple change.

Getting the areas covered by the FloodHazard objects

Right now you are getting the PolyStructure objects for each FloodHazard area one-by-one, in an attempt to get all PolyStructures objects that fall within the area of the FloodHazard objects. You can improve the performance here by getting the union of all of the FloodHazard areas, and then getting all PolyStructure objects within that combined area.

GeoDjango provides a Union aggregate as well as a Collect aggregate, with the main difference between them being that Union will remove any overlapping boundaries while Collect will just combine them all together. You are going to need to determine which one works best for your dataset in the long run.

high_hazards = FloodHazard.objects.filter(hazard='High').aggregate(combined_geom=Collect('geom'))
medium_hazards = FloodHazard.objects.filter(hazard='Medium').aggregate(combined_geom=Collect('geom'))
low_hazards = FloodHazard.objects.filter(hazard='Low').aggregate(combined_geom=Collect('geom'))

high_area = high_hazards['combined_geom']
medium_area = medium_hazards['combined_geom']
low_area = low_hazards['combined_geom']

This will combine the geom field for each object while retrieving the list of objects and place it in the combined_geom index of the queryset that is returned. I've split each of these out into their own *_area variables, which will contain the GEOSGeometry or GeometryCollection object representing the combined geometry fields.

Retreiving all of the PolyStructures objects for a set of FloodHazard objects

With the object returned by Union or Collect, you can use it in most geospatial queries, including the __intersects filter on geometry fields. So you would still use your existing PolyStructures query but instead of passing getgeom you would pass the *_areas object instead.

high_structures = PolyStructures.objects.filter(geom__intersects=high_area)

So now instead of making n queries to get the PolyStructures objects within each FloodHazard area, you are just making a single query that gets them all at once.

Now you're down from 203 queries to 106 queries (one new query for each type), but the logic within your views has changed as a result.

Getting the ids of the PolyStructures objects so they are excluded from other types

In order to get the ids of the structures, you have two different options: use the values_list('id', flat=True) method like you already are, or use map to retrieve the id field from each of the objects in the queryset we used before. Most likely the values_list option is going to be faster, but it's useful to note that it's not always the only option.

high_ids = high_structures.values_list('id', flat=True)
medium_ids = high_structures.values_list('id', flat=True)

Now instead of these ids being retrieved and grouped on an object-by-object basis, these are retrieved all at once with a single query.

Now you're down from 106 queries to 8 queries, which all comes together to be a significant decrease.

Putting it all together

So now if you combine all of the suggestions into a single view, you get 34 lines of code (including comments and whitespace) that executes 8 queries to the database and returns the same response.

# There are three different types of hazards, we are going to get the combined geometry for each type
high_hazards = FloodHazard.objects.filter(hazard='High').aggregate(combined_geom=Collect('geom'))
medium_hazards = FloodHazard.objects.filter(hazard='Medium').aggregate(combined_geom=Collect('geom'))
low_hazards = FloodHazard.objects.filter(hazard='Low').aggregate(combined_geom=Collect('geom'))

# Get the areas from the query
high_area = high_hazards['combined_geom']
medium_area = medium_hazards['combined_geom']
low_area = low_hazards['combined_geom']

# Get all structures in each of the areas

high_structures = PolyStructures.objects.filter(geom__intersects=high_area)
high_ids = high_structures.values_list('id', flat=True)

medium_structures = PolyStructures.objects.exclude(id__in=high_ids).filter(geom__intersects=medium_area)
medium_ids = medium_structures.values_list('id', flat=True)

combined_ids = list(high_ids) + list(medium_ids)

low_structures = PolyStructures.objects.exclude(id__in=combined_ids).filter(geom__intersects=low_area)

# Serialize the structures

high_results = high_structures.values('brgy_locat', 'municipali').annotate(counthigh=Count('brgy_locat'))
medium_results = medium_structures.values('brgy_locat', 'municipali').annotate(countmedium=Count('brgy_locat'))
low_results = low_structures.values('brgy_locat', 'municipali').annotate(countlow=Count('brgy_locat'))

# Combine all of the structures into a unified response

results = list(high_results) + list(medium_results) + list(low_results)

# Return all of the results as JSON
return JsonResponse(results, safe=False)


Find smallest number of squares that sum to a number

Not an review, but an extended comment:

I don't know how this solution can be optimized. The expected solution is radically different, and is based on a couple of theorems from number theory.

You should check whether the number is

A perfect square (the answer is obviously 1), or
is a sum of 2 squares (the answer is obviously 2), or
not a Legendre's number, that is \$n = 4^a(8b + 7)\$ (the answer is 3)

Otherwise, the answer is 4.


Calling functions with different variable types

If you use this method quit often by passing an Integer you will get performance problems because of the boxing and unboxing of the struct.

My advice would be to write two methods one using an Integer and the other using a String as argument.

Based on the naming guidelines you should name methods by using a verb or a verb phrase. DTableCellValue looks by its name more as a property than a method.

Based on the same guidelines method arguments should be named using camelCase casing. Although VB.NET isnt case sensitive I would like to suggest to follow these guidelines.

The try..catch is in its current form senseless because if an exception is thrown you are just rethrowing it.

A construct like

If condition Then
    Return Value
Else
    Return otherValue
End If

can be simplified by removing the redundant else like so

If condition Then
    Return Value
End If
Return otherValue

because if the condition is true the else won't be reached.

Implementing the mentioned points (leaving aside the method name) leads to two methods like so

Public Function DTableCellValue(ByRef rowData As DataRow, _
                                    ByVal colIndex As Integer, _
                                    Optional ByVal defaultVal As String = "") As Object
    Dim ret As Object = rowData.Item(colIndex)

    If IsDBNull(ret) Then
        Return defaultVal
    End If

    Return ret

End Function

Public Function DTableCellValue(ByRef rowData As DataRow, _
                                    ByVal colIndex As String, _
                                    Optional ByVal defaultVal As String = "") As Object
    Dim ret As Object = rowData.Item(colIndex)

    If IsDBNull(ret) Then
        Return defaultVal
    End If

    Return ret

End Function