CodeChef Matches challenge I am solving some problems from CodeChef but I am stuck on the Matches problem:

  Ari and Rich are playing a pretty confusing game. Here are the rules
  of the game:

  The game is played with two piles of matches. Initially, the first pile contains N matches and the second one contains M matches.
  The players alternate turns; Ari plays first.
  On each turn, the current player must choose one pile and remove a positive number of matches (not exceeding the current number of
  matches on that pile) from it.
  It is only allowed to remove X matches from a pile if the number of matches in the other pile divides X.
  The player that takes the last match from any pile wins.

  Solve(N,M)

  If its Ari's turn and N%M==0 then Ari wins else Rich wins.
  If N%M!=0 then player try every possible moves and check if he wins any one of them in end.

My solution uses Dynamic Programmming, so I have used functools.lru_cache to save function results. But I am getting TLE error in some test cases.How can I further optimize the code?

from functools import lru_cache

@lru_cache(maxsize=None)
def Solve(X,Y,Player=True):
    if X%Y==0:
        return Player
    else:
        temp=X
        X=X%Y
        if Player==Solve(max(X,Y),min(X,Y),not Player):
            return Player
        while temp!=X+Y:
            X=X+Y
            if Player==Solve(max(X,Y),min(X,Y),not Player):
                return Player
        return not Player

The whitespace needs a lot of changes to be PEP8-compliant. The capitalisation is also unconventional: functions and variables in Python usually start with a lower-case letter.

The meaning of Player is not entirely clear, and the logic is hard to follow because of that. The convention for this type of combinatorial game is to denote a position as "N" (Next player wins) or "P" (Previous player wins). So I think that Solve should be renamed position_is_N, and then the Player variable becomes unnecessary.

if condition: return doesn't need a following else:.

The loop can be tidied up considerably with range and any. I think this is equivalent:

@lru_cache(maxsize=None)
def position_is_N(x, y):
    return x % y == 0 or \
           any(not position_is_N(max(x2, y), min(x2, y))
               for x2 in range(x % y, x, y))

A comment about the constraint that x >= y would be nice.

  range of both X and Y is 1 to 10^18

Consider inputs 10**18, 3 (or other similarly small y). How many times might the loop run in the worst case?

As a rule of thumb, for a challenge like this if you need to do anything more than 10**9 times your algorithm is wrong. I can only suggest that you will need a complete rewrite to pass the time limit, so you should build a table of small values and see whether you can spot and prove a pattern. The problem is probably far more about mathematics than programming.


Find non-duplicated integer in a list where every integer occurs three times except for one integer The task

  Given an array of integers where every integer occurs three times
  except for one integer, which only occurs once, find and return the
  non-duplicated integer.

  For example, given [6, 1, 3, 3, 3, 6, 6], return 1. Given [13, 19, 13,
  13], return 19.

  Do this in O(N) time and O(1) space.

My solution

const arr = [6, 1, 3, 3, 3, 6, 6];
const findUnique = arr => {
  const set = new Set();
  return arr.reduce((sum, n) => set.has(n) ?
                    sum - n :
                    (set.add(n), sum + 2 * n), 0) / 2;
};

Because of the Set it has O(n) space, I guess. I currently don't know how to solve it with O(1) space.

I'm going to take @greybeard's internet points, since he apparently doesn't want them enough to type this ;-).

You need to keep in mind that \$O(x)\$ really means \$O(C \times x + ...)\$ for some value C. As long as you keep C a constant, you can do whatever you want. Thus, \$O(n)\$ can really mean \$O(2000 \times n)\$, and \$O(1)\$ can really mean \$O(10^6)\$, etc.

In this case, what @greybeard has suggested is that you count the individual set bits in all the numbers, and keep the counts in separate positions. Thus, for an array of 32-bit numbers, you would keep 32 separate counts.

Since you have to perform in \$O(n)\$, you could spend one loop through your input array determining how many bits you need to keep, so even very large numbers won't break the solution.

So let's pretend all the integers are 8-bit, just because. That doesn't mean there aren't a large number of integers, just that all the values in the array are in the range [0, 256). What does that give you?

(If you don't know, & and << are "bitwise operators."

for each element in array:
    for each bit_offset in [0..8):
        if element & (1 << bit_offset):
            count[bit_offset] += 1

What does that get you? It gets you a set of 8 separate counts, one for each bit in the 8-bit numbers. (Feel free to replace 8 with 64 if you like...)

Each count represents the number of times that bit appeared "set" in the input array. Now, according to your premise, all the numbers but one appear 3 times. So all the set bits will appear a multiple of 3 times. (Since set bits might be duplicated among numbers, it won't be "exactly 3 times" but instead "a multiple").

On the other hand, the one value that appears one time will have its set bits added to the counts one time. So there are two possibilities for each of the count[i] values (note: k[i] is some numbers that don't matter):

Either the unique value does not have this bit set: count[i] = 3 * k[i]
Or the unique value does have this bit set: count[i] = 3 * k[i] + 1

So you have to evaluate each count[i] in order, and determine which form it takes:

result = 0
for bit_offset in [0..8):
    if count[bit_offset] % 3 == 1:
        result = set_bit(result, bit_offset)

Where set_bit is spelled |= 1 << bit_offset in most C-derived languages.

In terms of efficiency, what happens?

You might process the array once by iterating over it to discover the largest number of bits required to be counted.
You create an array of N_BITS counts, initialized to zero. Since N_BITS is not related to the size of the input array, this is considered \$O(1)\$. (Actually \$O(32)\$ most likely, or maybe 64... But that's still 1 in big-O-hio!)
You iterate over the array one time, iterating over N_BITS bit values within each element. (So effectively \$O(64 \times n)\$, or less.) You compute the count values here.
You iterate over the N_BITS counts, determining the bits that will be set in the result, and setting them.
You return the result.

So your runtime is \$O(2 \times n)\$, or just n if you hard-code the N_BITS. And your memory is \$O(64)\$ or less, which is just 1.

Let's look at the example you gave in the comments:

[13, 12, 12, 3, 3, 3, 12]

First, rewrite those to binary (8 + 4 + 2 + 1):

[ 0b1101, 0b1100, 0b1100, 0b0011, 0b0011, 0b0011, 0b1100]

Now, count the set bits, just adding all the 1's in each column (no carry!):

[ 0b1101,
  0b1100,
  0b1100,
  0b0011,
  0b0011,
  0b0011,
  0b1100]
  --------
    4434 -> counts = [ 4, 4, 3, 4 ]

Next, our output will be 1 if the count is 1 (mod 3) and 0 if the count is 0 (mod 3). So [4,4,3,4] mod 3 are [1,1,0,1]:

result = 0b1101

Which is 13.


Iteration of password hashing in PHP I'm trying to improve the security for my backend:

/**
 *  This is the script that is executed when I add a new user - after the input has passed the validation.
 */
    $salt = base64_encode(mcrypt_create_iv(24,MCRYPT_DEV_URANDOM));  //  generate a random 32 character salt
    $hash = hash('sha256',$salt.$_POST['secret']);  //  create a 64 character hash based on the salt and the password

    $ins = $con['site']->prepare('INSERT INTO users (handle, salt, hash) VALUES (:handle, :salt, :hash)');
    $ins->execute(array(':handle'=>$_POST['handle'], ':salt'=>$salt, ':hash'=>$hash));
    if($ins){
        KD::addNotice('success','...message...');
    } else {
        KD::addNotice('error','...message...');
    }

/**
 *  This is the script that is executed when the user is logging in - also after the input has passed the validation.
 */
    $qry = $con['site']->prepare('SELECT handle, salt, hash FROM users WHERE handle = :handle');
    $qry->execute(array(':handle'=>$_POST['handle']));  //  look up the handle (username)
    if($qry->rowCount()==1){
        $get = $qry->fetch();
        if(hash('sha256',$get['salt'].$_POST['secret'])==$get['hash']){
            KD::addNotice('success','...message...');
            session_regenerate_id();
            $_SESSION['backend']->login($get['handle']);
        } else {
            KD::addNotice('error','...message...');
        }
    } else {
        KD::addNotice('error','...message...');
    }

This is as far as I've gotten and it works. I'm generating an unpredictable salt, creating a hash and saving it to a database.  Although this is an improvement from what it used to be, I'm fully aware of that there's more work to be done here.

I've been reading a bit about iterations. Or, rehashing the password, or the salt, or the hash, a couple of thousand times in order to prevent different forms of attacks. Could some of you please tell me how something like that could be implemented into my code?

Is it just putting some part of the code inside a for-loop when creating and verifying the password?

I'm no expert on security, but I believe this is not current best practice. I'll copy from an another answer of mine.

Cryptographic Right Answers (2009, Colin Percival, author of scrypt)

  Password handling: As soon as you receive a password, hash it using
  scrypt or PBKDF2 and erase the plaintext password from memory.

  Do NOT
  store users' passwords. Do NOT hash them with MD5. Use a real key
  derivation algorithm. PBKDF2 is the most official standard; but scrypt
  is stronger.  Please keep in mind that even if YOUR application isn't
  particularly sensitive, your users are probably re-using passwords
  which they have used on other, more sensitive, websites -- so if you
  screw up how you store your users' passwords, you might end up doing
  them a lot of harm.

How To Safely Store A Password (2010)

  Use bcrypt

  Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt.
  Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt.

  ...

  Why Not {MD5, SHA1, SHA256, SHA512, SHA-3, etc}?

  These are all general purpose hash functions, designed to calculate a digest of huge amounts of data in as short a time as possible. This means that they are fantastic for ensuring the integrity of data and utterly rubbish for storing passwords.

  ...

  Salts Will Not Help You

  It’s important to note that salts are useless for preventing dictionary attacks or brute force attacks. You can use huge salts or many salts or hand-harvested, shade-grown, organic Himalayan pink salt. It doesn’t affect how fast an attacker can try a candidate password, given the hash and the salt from your database.

  Salt or no, if you’re using a general-purpose hash function designed for speed you’re well and truly effed.

How to securely hash passwords? (2013)

  Conclusion

  Use bcrypt. PBKDF2 is not bad either. If you use scrypt you will be a
  "slightly early adopter" with the risks that are implied by this
  expression; but it would be a good move for scientific progress
  ("crash dummy" is a very honourable profession).


Swing / Terminal console drop-in How could this be improved style-wise (if this isn't good enough) when the need is just to have a console replacement for PrintWriter.format() with backwards compatibility?

class CCConsole // not to be confused with any Java class with a similar name
{
    static PrintWriter pw = new PrintWriter(System.out, true);

    static void redirect(LongRunningWorker longRunningWorker) {
        pw = new PrintWriter(new LongRunningWorkerWriter(longRunningWorker));
    }
}

class LongRunningWorkerWriter extends Writer
{
    LongRunningWorker longRunningWorker;

    LongRunningWorkerWriter(LongRunningWorker longRunningWorker) {
        this.longRunningWorker = longRunningWorker;
    }

    @Override
    public void close() throws IOException { }

    @Override
    public void flush() throws IOException { }

    @Override
    public void write(char[] chars, int offset, int length) throws IOException {
        this.longRunningWorker.rePublish(new String(chars, offset, length));
    }
}

abstract class LongRunningWorker extends SwingWorker<Boolean, String>
{
    ColorSwingConsole colorSwingConsole;

    LongRunningWorker(ColorSwingConsole colorSwingConsole) {
        CCConsole.redirect(this);
        this.colorSwingConsole = colorSwingConsole;
    }

    public Boolean doInBackground() // concrete for abstract
        throws Exception {
        go();
        return true;
    }

    abstract void go() throws Exception;

    void rePublish(String s) {
        publish(s); // protected final = inherit or same package, cannot override
    }

    @Override
    public void process(List<String> chunks) {
        for (String s : chunks)
            this.colorSwingConsole.append(s);
    }
}

class RealWorker extends LongRunningWorker
{
    RealWorker(ColorSwingConsole css) {
        super(css);
        // constructor stuff
    }

    void go() throws Exception {
        CCConsole.pw.format("%s\n", "hello world");
        // application stuff
    }
}

I did not understand what exactly are you asking here. Anyway, I will review your code:

First, make the fields private and provide getters when you need to access them. These that are set in constructors and are never re-setted, you may make them final.

Second, the CCConsole class may break if multiple threads access the redirect method at the same time.

Third, the CCConsole.redirect(this); in the LongRunningWorker is bad. There is no way to create a LongRunningWorker for testing purposes that does not interferes with everything else. Further if anything interferes with the CCConsole (which acts as a singleton), there is no way to remove it. I sugesting to make the CCConsole a normal object (i.e. you instantiate it and no more static fields used). In the LongRunningWorker constructor add a CCConsole parameter. In the RealWorker class, get a CCConsole from somewhere (instantiate it, get from a parameter, from a factory method, whatever).

Forth, avoid inheritance if you can using the strategy design pattern. Instead of making LongRunningWorker abstract with the abstract go method to be overriden in the RealWorker subclass, do this:

Create an interface with the go method (lets say LongWork).
Add a parameter to the LongRunningWorker class of the type LongWork, and save it in a field.
In the doInBackground() method, instead of calling the local go() method, call the one from the LongWork object.
LongRunningWorker is not abstract anymore, and you may remove the go() method.
RealWorker does not extends LongRunningWorker anymore, but now implements the LongWork interface.
You may remove the ColorSwingConsole parameter from the RealWorker class.


Beginnings of a Scattergories program I wrote this program as a start towards implementing the game Scattergories.  I had a problem with the _sleep command so I used Sleep in the windows.h library.

Eventually, I would like to add a way to either check if a word is correct, or to suggest words, and to activate a sound when the timer is done.

// A program to keep track of points and time and to give a random letter for the game scattergories

#include<iostream>
#include<Windows.h>
using std::cout;
using std::cin;

void ltr()    //gives a random letter
{
    char letter;
    letter = rand() % 27 + 64;         //assigns a random letter in ascii code to a char (resulting in a random letter)
    cout << "The letter is " << letter << "\n";
}

void clock()   //timer
{
    cout << "You got 1.5 minutes to finish\n";
    for (int i = 90; i > 0; i--)
    {
        if (i % 5 == 0)
            cout << i << "\n";
        Sleep(1000);
    }
    cout << "DING DONG!!! DING DONG!!! Time's up!!!\n";
}

int points()        //points per round
{
    int a, b, c, sum;
    cout << "How many sections only you got?\n";          //worth 15 points
    cin >> a;
    cout << "How many words only you got?\n";       //worth 10 points
    cin >> b;
    cout << "How many words you and another person got?\n";    //worth 5 points
    cin >> c;
    sum = a * 15 + b * 10 + c * 5;
    return sum;           //Note: It doesn't matter how many sections there are.
}

int act()
{
    int Points;
    ltr();
    clock();
    Points=points();
    cout << "You have earned " << Points << " this round\n\n";
    return Points;
}

int main()
{
    int Points;
    cout << "Starting in five seconds\n";
    Sleep(5000);
    Points = act();
    for (;;)          //inf loop
    {
        int ph;
        cout << "Press 1 to continue or 2 to stop\n";
        cin >> ph;
        if (ph == 1)
        {
            Points += act();
        }
        else
        {
            break;
        }
    }
    cout << "You have earned a total of " << Points << " great job!\n\n";
    system("pause");
}

Use portable code as much as possible

You should avoid

#include <Windows.h>

when you can use portable c++ features (according the current c++ standard) instead:

Sleep(1000);

should be

std::this_thread::sleep_for(1s); // needs to #include <thread>

As you see that's much more readable, and will compile on other operating system environments as well.

Use a better random number generator

letter = rand() % 27 + 64;

Doesn't provide a good random number generator (besides you missed to initialize the seed with srand() and your program will produce the same sequence of random numbers with every run).

C++ provides easy to use and configurable (regarding the wanted number distribution requirements) random generators like listed here.

Do not use system("pause");

Besides that statement isn't portable again, it introduces security problems. If there's a program existent and accessible at the system called pause that doesn't do what's intended (but some malicious things instead), that one will be executed.

If you wan't to wait for some input rather use std::cin >> dummy; or such.

Check the state of input operations

 cin >> ph;

You never checked that the user inputted a valid number. If something like xxxx is entered, your loop will never stop.

Better do something like:

bool stop = false;
do
{
    int ph;
    cout << "Press 1 to continue or 2 to stop\n";
    if(cin >> ph) {
         if (ph == 1)
         {
             Points += act();
         }
         else if(ph == 2)
         {
             stop = true;
         }
    }
    else {
         cin.clear();
         std::string dummy;
         std::getline(cin,dummy); // Consume the invalid input
    }
} while(!stop);

  I would like to add a way to either check if a word is correct, or to suggest words but it would mean I need to connect it to a dictionary of some sort (which I don't know how to do).

You can use std::map or std::unordered_map to create dictionaries.

I can't give you any sample code how to integrate these in your program, since it's unclear for me how you wan't to make use of these exactly.


Find a value in a comma delimited String Here is a simple WPF converter method designed, where the object is a string, and the parameters are a string, which contain a set of comma separated values. The program return true if the list has the value in and false otherwise.

    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        try
        {
            string temp = parameter as string;
            List<String> list = temp.Split(',').ToList();
            string x = value as string;

            return list.Contains(x);
        }
        catch (Exception)
        {

            throw;
        }
    }

Why I am doing this

I was making a WPF Tab Controller, that looks at the type of objects that it can load and opens up different views based on that information. I cannot change the types of stations that are being supplied to me. Several of these stations share a view between all of them even though their type is different, I am using this converter in a Data Trigger in order to show the same view for this list of stations.

            <DataTrigger Binding="{Binding StationState.StationType, Converter={StaticResource BindingInListConvertor}, ConverterParameter={StaticResource FinishingString}}" Value="True">
                <Setter Property="ContentTemplate" Value="{StaticResource FinishingTemplate}" />
            </DataTrigger>

The only other ways I could think of to do this are a multibinding data-trigger, or just a whole lot of them, and figured this would be best because the the possibility of future functionality.

This is my solution for it, is their a better/simpler/cooler way to acomplish this, thanks.

This could be a better/cooler way:

return (parameter as string)?.Split(',').Contains(value as string) ?? (object)false;

Your example is so short that I don't know what else I could say. You are looking for a short solution, so here is one.


Scan first and last name from user, then format the name as last, first initial Reads the user's first and last name (an entire line as a single string), then prints the last name followed by a comma and the first initial. Ensure the code re-prompts until the user types a name that is at least 5 letters total in length and has at least one space in it. For example:

Type your name: Jessica Miller
Your name is: Miller, J.

A few questions I have about my code:

Is there a difference between breaking out of a loop with a break statement, and meeting the condition for correctName?
Did I make my program do unnecessary things? I'd like to answer this for myself, but I may've made it unnecessarily complex, or even buggy, due to my thought process. For instance, can I check if there's a space in my first conditional, where I check if the length of the line is greater than or equal to five?

Any feedback is welcome!
Scanner console = new Scanner(System.in);
boolean correctName = false;

while(correctName != true) {
    System.out.print("Type your name: ");
    String line = console.nextLine();
    if(line.length() >= 5) {
        StringTokenizer lineTokens = new StringTokenizer(line);
        while(lineTokens.hasMoreTokens()) {
            if(lineTokens.countTokens() >= 2) {
                String first = lineTokens.nextToken();
                String last = lineTokens.nextToken();
                String name = (last + ", " + first.substring(0, 1) + ".");
                System.out.println("Your name is: " + name);
                correctName = true;
            } else {
                String checkSpace = lineTokens.nextToken();
                for(int i = 0; i < checkSpace.length(); i++) {
                    if(checkSpace.charAt(i) == ' ') {
                        break;
                    } else {
                        System.out.println("Error, must be at least 5 chars with a space.");
                        break;
                    }
                }
           }
        }
    } else {
        System.out.println("Error, must be at least 5 chars with a space.");
    }
}

Methods

I'm assuming this chunk of code sits in your main() method, and I think one quick way of improving it is to break it into two methods - one that returns a validated user input (at least 5 chars with a space), and one for the actual work.

When you separate them, you will also realize that you only need to print your validation message once, instead of twice as you are doing currently (to check for length, and to check for a space).

Stricter input validation

  Type your name: Jessica Miller

  Your name is: Miller, J.

Since you actually require the initial of the first name, you also need to check that the space is not at the start or the end of the input, as otherwise you only have one name. Consider this:

private static void test(String input) {
    // A Scanner works in a similar way to the legacy StringTokenizer class
    try (Scanner scanner = new Scanner(input)) {
        while (scanner.hasNext()) {
            System.out.printf("Token: [%s]%n", scanner.next());
        }
    }
}

test("hello world");
// Token: [hello]
// Token: [world]

test(" helloworld");
// Token: [helloworld]

test("helloworld ");
// Token: [helloworld]

An example implementation to validate for this can be:

private static final int REQUIRED_LENGTH = 5;

private static String getInput(Scanner scanner) {
    String result = scanner.nextLine();
    while (result.length() < REQUIRED_LENGTH ||
            !result.contains(" ") ||
            result.startsWith(" ") ||
            result.endsWith(" ")) {
        System.out.println("Try again.");
        result = scanner.nextLine();
    }
    return result;
}

Still, this can be defeated if there are multiple spaces and they happen to also be at the start and/or the end. Perhaps a regex solution may be better here... this shall be left as an exercise for the reader.

Boolean flags

Going back to your main() method, you wouldn't require correctName if you have separated the logic that validates the user input and the actual work.


Transforming markdown files in folders into a documentation I start coding a simple application: I transform markdown files in folders into documentation.

My architecture is very flat. My code looks like a series of unit function called one after the other.

def main():
    config = get_user_config(os.getcwd())
    sources_dir = config['sources_dir']
    for root, dirnames, filenames in os.walk(sources_dir):
        for filename in fnmatch.filter(filenames, '*.md'):
            markdown_file_path = os.path.join(root, filename)

            html_block = transform_html_into_markdown(markdown_file_path)

            wrapped_html = '{{% extends "base.html" %}}{{% block content %}}{0}{{% endblock %}}'.format(html_block)

            html = render_html(wrapped_html, config)

            html_file_path = get_html_file_path(markdown_file_path, sources_dir)

            write_html_file(html_file_path, html)

What I really like:

I can unit test every function. Code is simple to understand, no fancy stuff.
Each function is in its own module.

What I do not like:

My main() method is growing until I create two functions to rule them all. I have the impression of hiding the dust under the carpet.
If I want for instance increase the number of process doing some calculation, I will probably need some hard refactoring.
Everything is synchronous.

Do you have some idea to improve my code? Should I try something else?

First change - extract iteration logic to its own generator:

def markdown_files(config):
    sources_dir = config['sources_dir']
    for root, dirnames, filenames in os.walk(sources_dir):
        for filename in fnmatch.filter(filenames, '*.md'):
            yield os.path.join(root, filename)

def main():
    config = get_user_config(os.getcwd())
    for path in markdown_files(config):
        html_block = transform_html_into_markdown(path)

        # ...

For one, this eliminates a level of nesting, which is always a good thing

Also you can add things like exclude paths to your config at a later date, without having to change your processing code. Note that this might require you to rethink passing sources_dir into get_html_file_path, as any filtering logic might end up duplicated...

Second change: wrap the entire processing routine in a single function:

def process_file(config, path):
    html_block = transform_html_into_markdown(path)

    # ...

def main():
    config = get_user_config(os.getcwd())
    for md_path in markdown_files(config):
        process_file(config, md_path)

So that later you can parallelize it, if you really need to:

import multiprocessing
import functools  # we can't directly pass a lambda into map

pool = multiprocessing.Pool()
pool.map(
    functools.partial(process_file, config=config),
    markdown_files(config)
)


Account-creation program I just finished a program that is a simple Swing GUI. It takes in user info, such as password and username, turns them into strings, and puts them in a text file called nuserInfo.txt.

I just want corrections on how the code works and what I can do to improve my saving, GUI, and code in general. I'll post all the code below (just one class). Feel free to make any remarks, comments, criticism, or ask any questions in the comments below.

CreateAccount class:

package passwordProgram;

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Arrays;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JPasswordField;
import javax.swing.JTextField;
import javax.swing.UIManager;

public class CreateAccount implements ActionListener {

    JFrame frame;
    JPanel panel;
    JTextField username;
    JPasswordField password;
    JPasswordField confirmPassword;
    JLabel warningLabel;

    public static void main(String[] args) {
        try {
            UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
        } catch (Exception e) {
            e.printStackTrace();
        }
        CreateAccount window = new CreateAccount();
        window.go();
    }

    public void go() {
        frame = new JFrame("Create a new account");
        panel = new JPanel();

        panel.setLayout(new GridBagLayout());

        frame.getContentPane().add(BorderLayout.CENTER, panel);
        panel.setBackground(Color.ORANGE);

        JLabel userLabel = new JLabel("Username:");
        JLabel passwordLabel = new JLabel("Password:");
        JLabel confirmPasswordLabel = new JLabel("Confirm Password:");
        username = new JTextField(15);
        password = new JPasswordField(15);
        confirmPassword = new JPasswordField(15);

        GridBagConstraints right = new GridBagConstraints();
        right.anchor = GridBagConstraints.WEST;
        GridBagConstraints left = new GridBagConstraints();
        left.anchor = GridBagConstraints.EAST;

        right.weightx = (int) 2;
        right.fill = GridBagConstraints.REMAINDER;
        right.gridwidth = GridBagConstraints.REMAINDER;
        // actual GUI

        panel.add(userLabel, left);
        panel.add(username, right);
        panel.add(passwordLabel, left);
        panel.add(password, right);
        panel.add(confirmPasswordLabel, left);
        panel.add(confirmPassword, right);

        frame.setSize(300, 250);
        frame.setVisible(true);

        JButton createAccount = new JButton("Create this account");
        frame.getContentPane().add(BorderLayout.SOUTH, createAccount);
        createAccount.addActionListener(this);

        warningLabel = new JLabel();
        frame.getContentPane().add(BorderLayout.NORTH, warningLabel);
    }

    // This is where the problem is!
    public void actionPerformed(ActionEvent event) {
        if (!(Arrays.equals(password.getPassword(), confirmPassword.getPassword()))) {
            warningLabel.setText("Your passwords do not match! Please try again.");
        } else if (password.getPassword().length < 1) {
            warningLabel.setText("That password is not long enough! Please try again!");
        } else {
            try {
                FileWriter writer = new FileWriter(new File("nuserInfo.txt"));
                writer.write(username.getText() + "/" + password.toString());
                writer.close();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}

I put your Swing components on the Event Dispatch thread (EDT) in the main method.
I rearranged your Swing component definitions in the go method to group like things together.  I made sure that the JFrame setVisible method was the last method called in the go method.  I added a call to the JFrame setDefaultCloseOperation so that when you closed the GUI, your application would exit.
Your password field returns a character array.  I had to look up how to do it, but you have to convert the character array to a String to write it to a file.
I added the BufferedWriter to your actionPerformed method.

.

package passwordProgram;

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.BufferedWriter;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Arrays;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JPasswordField;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;
import javax.swing.UIManager;

public class CreateAccount implements ActionListener {

    JFrame          frame;
    JPanel          panel;
    JTextField      username;
    JPasswordField  password;
    JPasswordField  confirmPassword;
    JLabel          warningLabel;

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager
                            .getSystemLookAndFeelClassName());
                } catch (Exception e) {
                    e.printStackTrace();
                }
                CreateAccount window = new CreateAccount();
                window.go();
            }
        });
    }

    public void go() {
        panel = new JPanel();
        panel.setLayout(new GridBagLayout());
        panel.setBackground(Color.ORANGE);

        frame = new JFrame("Create a new account");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.getContentPane().add(BorderLayout.CENTER, panel);

        JLabel userLabel = new JLabel("Username:");
        JLabel passwordLabel = new JLabel("Password:");
        JLabel confirmPasswordLabel = new JLabel("Confirm Password:");
        username = new JTextField(15);
        password = new JPasswordField(15);
        confirmPassword = new JPasswordField(15);

        GridBagConstraints right = new GridBagConstraints();
        right.anchor = GridBagConstraints.WEST;
        GridBagConstraints left = new GridBagConstraints();
        left.anchor = GridBagConstraints.EAST;

        right.weightx = (int) 2;
        right.fill = GridBagConstraints.REMAINDER;
        right.gridwidth = GridBagConstraints.REMAINDER;
        // actual GUI

        panel.add(userLabel, left);
        panel.add(username, right);
        panel.add(passwordLabel, left);
        panel.add(password, right);
        panel.add(confirmPasswordLabel, left);
        panel.add(confirmPassword, right);

        JButton createAccount = new JButton("Create this account");
        frame.getContentPane().add(BorderLayout.SOUTH, createAccount);
        createAccount.addActionListener(this);

        warningLabel = new JLabel();
        frame.getContentPane().add(BorderLayout.NORTH, warningLabel);

        frame.setSize(300, 250);
        frame.setVisible(true);
    }

    // This is where the problem is!
    public void actionPerformed(ActionEvent event) {
        if (!(Arrays.equals(password.getPassword(),
                confirmPassword.getPassword()))) {
            warningLabel
                    .setText("Your passwords do not match! Please try again.");
        } else if (password.getPassword().length < 1) {
            warningLabel
                    .setText("That password is not long enough! Please try again!");
        } else {
            try {
                BufferedWriter writer = new BufferedWriter(new FileWriter(
                        "nuserInfo.txt"));
                writer.write(username.getText() + "/"
                        + new String(password.getPassword()));
                writer.close();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}


Image resizer and uploader I've written an image resizer / uploader, that takes an incoming image, resizes it, and uploads it to the server, under the /usericons/ folder.

However, the code is sloppy. Can somebody suggest a better method / improvements?

define ("MAX_SIZE","400");

$errors = 0;

if ($_SERVER["REQUEST_METHOD"] == "POST") {
  $image = $_FILES["icon"]["name"];
  $uploadedfile = $_FILES['icon']['tmp_name'];
  if ($image) {
        $filename = stripslashes($_FILES['icon']['name']);
        $extension = getExtension($filename);
        $extension = strtolower($extension);
        if (($extension != "jpg") && ($extension != "jpeg") && ($extension != "png") && ($extension != "gif")) {
              echo ' Unknown Image extension ';
              $errors = 1;
        } else {
              $size=filesize($_FILES['icon']['tmp_name']);
              if ($size > MAX_SIZE*1024) {
                    echo "You have exceeded the size limit";
                    $errors = 1;
              }
              if ($extension == "jpg" || $extension == "jpeg" ) {
                    $uploadedfile = $_FILES['icon']['tmp_name'];
                    $src = imagecreatefromjpeg($uploadedfile);
              } else if($extension=="png") {
                    $uploadedfile = $_FILES['icon']['tmp_name'];
                    $src = imagecreatefrompng($uploadedfile);
              } else {
                    $src = imagecreatefromgif($uploadedfile);
              }
              list($width,$height)=getimagesize($uploadedfile);
              $newwidth = 500;
              $newheight = ($height/$width)*$newwidth;
              $tmp = imagecreatetruecolor($newwidth,$newheight);
              $newwidth1 = 300;
              $newheight1 = ($height/$width)*$newwidth1;
              $tmp1 = imagecreatetruecolor($newwidth1,$newheight1);
              imagecopyresampled($tmp, $src, 0,0,0,0, $newwidth, $newheight, $width, $height);
              imagecopyresampled($tmp1, $src, 0,0,0,0, $newwidth1, $newheight1, $width, $height);
              $filename = "usericons/500-". $_SESSION['username'] . '.png';
              $filename1 = "usericons/300-". $_SESSION['username'] . '.png';
              imagejpeg($tmp,$filename,100);
              imagejpeg($tmp1,$filename1,100);
              hasIcon($_SESSION['username']);
              imagedestroy($src);
              imagedestroy($tmp);
              imagedestroy($tmp1);
              echo '<script>window.location = "/Users/";</script>';
        }
  }
}
 //If no errors registred, print the success message

if (isset($_POST['Submit']) && !$errors) {
   echo "Image Upload Fail!";
}
function getExtension($str) {

     $i = strrpos($str,".");
     if (!$i) { return ""; }
     $l = strlen($str) - $i;
     $ext = substr($str,$i+1,$l);
     return $ext;
 }

Couple of things I've done to your code:

Early exits
Redundant code removal
Readability improvement

Keep in mind that I have not tested this, but it should work. And also, the code will only work on PHP 5.4 and greater.

Cleaned up version:

<?php
if ($_SERVER['REQUEST_METHOD'] !== 'POST' || !isset($_FILES['icon'])) {
    echo 'Image upload failed.';

    return;
}

$file = [
    'name' => stripslashes($_FILES['icon']['name']),
    'path' => $_FILES['icon']['tmp_name'],
    'ext'  => strtolower(pathinfo($_FILES['icon']['name'], PATHINFO_EXTENSION)),
    'size' => filesize($_FILES['icon']['tmp_name'])
];

$supportedExts = [
    'jpg',
    'jpeg',
    'png',
    'gif'
];

if (!in_array($file['ext'], $supportedExts)) {
    echo 'Extension not supported.';

    return;
}

if ($file['size'] > 400 * 1024) {
    echo 'You have exceeded the image size limit.';

    return;
}

list($image['x'], $image['y']) = getimagesize($file['path']);

switch ($file['ext']) {
    case 'jpg':
    case 'jpeg':
        $imgResrc1 = imagecreatefromjpeg($file['path']);
        break;
    case 'png':
        $imgResrc1 = imagecreatefrompng($file['path']);
        break;
    default:
        $imgResrc1 = imagecreatefromgif($file['path']);
}

$imageSizes = array(
    [
        'x' => 300,
        'y' => $image['y'] / $image['x'] * 300
    ],
    [
        'x' => 500,
        'y' => $image['y'] / $image['x'] * 500
    ]
);

foreach ($imageSizes as $size) {
    $imgResrc2 = imagecreatetruecolor($size['x'], $size['y']);

    imagecopyresampled($imgResrc2, $imgResrc1, 0, 0, 0, 0, $size['x'], $size['y'], $image['x'], $image['y']);

    // Shouldn't the extension be .jpg / .jpeg since you're using imagejpeg()?
    $filePath = 'usericons/' . $size['x'] . '-'. $_SESSION['username'] . '.png';

    imagejpeg($imgResrc2, $filePath, 100);

    imagedestroy($imgResrc2);
}

imagedestroy($imgResrc1);

hasIcon($_SESSION['username']);

echo '<script>window.location = \'/Users/\';</script>';

// TODO: Print success message


Merge Two Binary Tree- LeetCode I solved this problem on LeetCode

  Given two binary trees and imagine that when you put one of them to
  cover the other, some nodes of the two trees are overlapped while the
  others are not.

  You need to merge them into a new binary tree. The merge rule is that
  if two nodes overlap, then sum node values up as the new value of the
  merged node. Otherwise, the NOT null node will be used as the node of
  new tree.

Example 1:
Input:
  Tree 1                     Tree 2
          1                         2
         / \                       / \
        3   2                     1   3
       /                           \   \
      5                             4   7
Output:
Merged tree:
       3
      / \
     4   5
    / \   \
   5   4   7

  Note: The merging process must start from the root nodes of both trees.

The code that I had written is.

/**
 * Definition for a binary tree node.
 * public class TreeNode {
 *     public int val;
 *     public TreeNode left;
 *     public TreeNode right;
 *     public TreeNode(int x) { val = x; }
 * }
 */
public class Solution {
    public enum Direction
    {
        Left, Right, None
    }
    public TreeNode doMerge(TreeNode t1, TreeNode t2, Direction direction, TreeNode t3)
        {
            if(t1 == null && t2 == null)
            {
                return t3;
            }
            if(t1 == null || t2 == null)
            {
                TreeNode notNullTree = t1 == null ? t2 : t1;
                switch(direction)
                {
                    case Direction.None:
                        return notNullTree;
                    case Direction.Left:
                        t3.left = notNullTree;
                        break;
                    case Direction.Right:
                        t3.right = notNullTree;
                        break;
                }
                return t3;
            }
            var newNode = new TreeNode(t1.val + t2.val);
            switch (direction)
            {

                case Direction.None:
                    t3 = newNode;
                    break;
                case Direction.Left:
                    t3.left = newNode;
                    t3 = t3.left;
                    break;
                case Direction.Right:
                    t3.right = newNode;
                    t3 = t3.right;
                    break;
            }
            doMerge(t1.left, t2.left, Direction.Left, t3);
            doMerge(t1.right, t2.right, Direction.Right, t3);
            return t3;
        }
        public TreeNode MergeTrees(TreeNode t1, TreeNode t2)
        {
            return doMerge(t1, t2, Direction.None, null);
        }
}

The code has a lot of conditionals which could be refactored. Any suggestion on making the code a bit cleaner and concise would be helpful.

This is a very interesting problem!

Firstly, you have correctly identified the 3 possible states:

both tree1 and tree2 are null
tree1 or tree2 is null
both tree1 and tree2 are not null

That being said, I would recommend a couple of things:

Break up the problem into 4 possible states (i.e., don't handle the case when tree1 or tree2 is null in one conditional statement.
if passing in the merged tree tree3 to doMerge in order to modify it in the method, you should utilize C#'s out parameter.
Why do you need the Direction enum? It seems to make the code harder to understand.

Here is my implementation of doMerge:

public TreeNode MergeTrees(TreeNode t1, TreeNode t2)
{
    if (t1 == null && t2 == null) return null;
    if (t1 != null && t2 == null) return t1;
    if (t1 == null && t2 != null) return t2;
    TreeNode merged = new TreeNode(t1.val + t2.val);
    merged.left = MergeTrees(t1.left, t2.left);
    merged.right = MergeTrees(t1.right, t2.right);
    return merged;
}


Sanitzing input on form submit with PHP I was hacking around this morning and wanting to use automatically generate safe-to-use variables from $_POST. Probably not the best use of extract() and compact(), but extract() allowed me to prefix variable names and compact(explode()) seemed like a neat way to stick everything back into an array after being prefixed.

function sanitizeForm($formVars) {
    // prefix $_POST vars so attacker can't try and guess existing
    extract($_POST, EXTR_PREFIX_ALL, 'form');
    // put the form variables back into an array
    $inputArray = compact(explode(' ', $formVars));
    $cleanInput = array();
    foreach($inputArray as $key => $value) {
        $cleanInput[$key] = htmlspecialchars($value);
    }
    return $cleanInput;
}

The test form I used simply posted firstname, lastname, and comments. Here's the PHP:

if(isset($_POST['submit'])) {
    $test = sanitizeForm('form_firstname form_lastname form_comments');
    echo('<pre>');
    print_r($test);
    echo('</pre>');
}

Submitting 'test' for both firstname and lastname with the script in comments:

Array
(
    [form_firstname] => test
    [form_lastname] => test
    [form_comments] => <script>alert('xss')</script>
)

You should first ask yourself this: Why am I sanitizing input in this way?

Let's say I have a login form, and the user enters his credentials, an email and a password. The email will be queried against the database, to figure out the user's hashed password (You are hashing your passwords, right?!), and the password would be hashed, to be compared against that hash.

Both of those fields will never be outputted to HTML, the password is never kept in plain-text form for very long, and the username is queried against the database and then discarded. So why should you escape them for HTML?

Sanitizing (or more accurately named, Escaping), contrary to popular belief should not be done when you receive your input. It should be done right before you output it. Because at the time you receive your input, you have no idea where it's going to be used. It could be outputted to HTML (in which case it needs htmlspecialchars()), it could be outputted to JSON for an AJAX response (in which case it needs json_encode()), it could be stored in the database (in which case it needs prepared statements).

The general rule is: You escape as late as possible. HTML strings should be escaped right before they get echoed onto an HTML page.

What you should be doing when receiving your input, and as early as possible, is validating the input, i.e. making sure the input matched what you expect in format, and in business logic. Format means "This is a valid email address", and business logic means "This comment that the user is trying to edit, really belongs to him".


Project Euler 21: Sum of amicable numbers optimisation Let d(n) be defined as the sum of proper divisors of n (numbers less than n which divide evenly into n).
  If d(a) = b and d(b) = a, where a ≠ b, then a and b are an amicable pair and each of a and b are called amicable numbers.

  For example, the proper divisors of 220 are 1, 2, 4, 5, 10, 11, 20, 22, 44, 55 and 110; therefore d(220) = 284. The proper divisors of 284 are 1, 2, 4, 71 and 142; so d(284) = 220.

  Evaluate the sum of all the amicable numbers under 10000.

I have the following code which I know works, but is very slow for large numbers.

import time
start = time.time()

n = int(input("Enter a number"))

def factor(c):
    factors = []
    for d in range(1, int(c / 2) + 1):
        if c % d == 0:
            factors.append(d)
    return sum(factors)

def sum_ammicable(x):
    ammicable = set()
    for a in range(1, x):
        for b in range(1, x):
            if a != b:
                if factor(a) == b and factor(b) == a:
                    ammicable.update([a,b])
    return sum(ammicable)

print(sum_ammicable(n))

end = time.time()
print(end -start)

How can I optimise this code

Integer Division

Python comes with a built-in integer division operator.  So instead of writing int(c / 2) + 1, you could simply write c // 2 + 1.  It is slightly faster.

Unnecessary List Creation

Creating a list of factors just to sum(factors) afterwards is unnecessary busy work, which takes time.  Simply add up the factors as you find them:

def factor(c):
    total = 0
    for d in range(1, c // 2 + 1):
        if c % d == 0:
            total += d
    return total

Or, use a generator expression with sum():

def factor(c):
    return sum(d for d in range(1, c // 2 + 1) if c % d == 0)

There is no need to store the factors you found.

Move Invariants out of Loops

Consider:

def sum_ammicable(x):
    ...
    for a in range(1, x):
        for b in range(1, x):
            if a != b:
                if factor(a) == b and ...:
                    ...
    ...

How many times is factor(a) computed when a==1?  If x == 10000, it gets computed 10000 times ... once for each b value.  How many times when a==2?  It'll get computed 10000 times as well.  But does factor(a) depend on the value of b?  No.  Each time you compute factor(a), for the same value of a, the same result is produced.

Can you calculate this value once per a loop?  Sure!  Move it out of the b loop!

def sum_ammicable(x):
    ...
    for a in range(1, x):
        factor_a = factor(a)
        for b in range(1, x):
            if a != b:
                if factor_a == b and ...:
                    ...
    ...

Instead of computing factor(a) 100,000,000 times, you now only compute it 10,000 times, which should be a significant improvement in speed.

Finally, considering that:

        for b in range(1, x):
            ...
                if factor_a == b and ...:
                    ...

will only execute the rest of the if statement (and perhaps the body of the if) for exactly one value of b, do you really need to loop over all 10,000 possible b values?  With a little thought, and a little reworking of the code, you can remove the b loop altogether!

Group Related Code Together

You start with

start = time.time()

n = int(input("Enter a number"))

then 2 function definitions, and then you continue with mainline code:

print(sum_ammicable(n))

end = time.time()
print(end -start)

Move the code together, after the function declarations.  Even better, place it inside a if __name__ == '__main__': guard, so you can import factor(c) and sum_ammicable(x) functions into other code:

if __name__ == '__main__':

    start = time.time()

    n = int(input("Enter a number"))

    print(sum_ammicable(n))

    end = time.time()
    print(end - start)

Now that this code isn't separated by 2 functions, it becomes apparent you are timing how long the user takes to "Enter a number" in addition to the time it takes to compute sum_ammicable(n).  Is the user's reaction & typing speed really being profiled here?  Perhaps you want to move start = time.time() after the input statement.

Pro tip: use time.perf_counter() instead of time.time() for higher accuracy profiling.

Algorithmic improvements are covered on the Project Euler site itself.  Read the problem overview after successfully completing the problem for some additional dramatic improvements.  There is no need to reproduce their analysis here.


Hackerrank "Hash Tables: Ransom Note" Javascript Solution Here is the original problem,
and here's my solution:

function main() {
    var m_temp = readLine().split(' ');
    var m = parseInt(m_temp[0]);
    var n = parseInt(m_temp[1]);
    magazine = readLine().split(' ');
    ransom = readLine().split(' ');

    var freqs = {}
    for (var i = 0; i < m; i++){
      freqs[magazine[i]] = (freqs[magazine[i]] || 0) + 1;
    }

    var result = "Yes"
    for (var j = 0; j < n; j++){
      if (freqs[ransom[j]] && freqs[ransom[j]] > 0){
          freqs[ransom[j]] -= 1;
      } else {
          result = "No"
          break;
      }
    }

    console.log(result)
}

I wonder if there's a more efficient solution than this? Thanks! I understand forEach could be used for code brevity, but I'm just using for loop for the extra performance benefit (https://coderwall.com/p/kvzbpa/don-t-use-array-foreach-use-for-instead)

Your solution may work for valid inputs, but you are not checking invalid input situations such as when the ransom letter contains more words than the ones in the magazine:

if(n > m):
    throw new Error("ransom can not be written from magazine");

You can even go further by checking if the first line corresponds to what it pretends to be:

if (magazine.length !== m)
    throw new Error("Wrong words number in magazine");
if (ransom.length !== n)
    throw new Error("Wrong words number in ransom");

You can refactor the above conditions in one single line:

if(n > m || agazine.length !== m || ransom.length !== n):
    throw new Error("Invalid input");


Simple generic cache I sometimes write decorators to cache results I get from a repository or I write other caches... I don't want to write another cache anymore. I'd like to have something that'll work for anything. I've build a test-cache that I could use to replace other caches. It provides just the most basic methods.

public class Cache
{
    private readonly IDictionary<object, (object Value, DateTime CreatedOn, TimeSpan Timeout)> _cache = new Dictionary<object, (object Value, DateTime CreatedOn, TimeSpan Timeout)>();

    public TResult Get<TResult>(object key, Func<TResult> get, TimeSpan timeout)
    {
        if (_cache.TryGetValue(key, out var t) && !HasTimedout(t.CreatedOn, t.Timeout))
        {
            return (TResult)t.Value;
        }

        var value = get();
        _cache[key] = (value, DateTime.UtcNow, timeout);

        return value;

        bool HasTimedout(DateTime createdOn, TimeSpan currentTimeout) => DateTime.UtcNow - createdOn > currentTimeout;
    }

    public async Task<TResult> GetAsync<TResult>(object key, Func<TResult> get, TimeSpan timeout)
    {
        return await Task.Run(() => Get<TResult>(key, get, timeout));
    }

    public void Clear() => _cache.Clear();

    public bool Remove(object key) => _cache.Remove(key);
}

On its own this is not very useful yet so I have a few extensions for it that help me to create keys which are tuples so that I can use their comparer that compares all properties and not the object reference:

public static class CacheExtensions
{
    public static TResult Get<TResult>(this Cache cache, Func<TResult> get, TimeSpan timeout)
    {
        return cache.Get<TResult>(null, get, timeout);
    }

    public static TResult Get<T1, TResult>(this Cache cache, T1 arg1, Func<T1, TResult> get, TimeSpan timeout)
    {
        return cache.Get<TResult>((arg1), () => get(arg1), timeout);
    }

    public static  TResult Get<T1, T2, TResult>(this Cache cache, T1 arg1, T2 arg2, Func<T1, T2, TResult> get, TimeSpan timeout)
    {
        return cache.Get<TResult>((arg1, arg2), () => get(arg1, arg2), timeout);
    }
}

Example usage:

var cache = new Cache();

cache.Get(2, 4, (x, y) => 3, TimeSpan.FromSeconds(4)).Dump("New");
cache.Get(2, 4, (x, y) => 4, TimeSpan.FromSeconds(4)).Dump("Cached");
Thread.Sleep(5000);
cache.Get(2, 4, (x, y) => 5, TimeSpan.FromSeconds(4)).Dump("Timedout");

Output:

New
3

Cached
3

Timedout
5

What do you think of it?

You say cache I say memory leak.

You haven't given much thought to your eviction policy yet and that's 95% of the work in writing a decent cache. Your expired entries are only removed when someone tries to access them.

Consider the case when the cache is used poorly - i.e. you end up caching a huge number of things that are only accessed once. Without a subsequent access after they are expired, they aren't removed and you have a whole bunch of wasted memory.

Your cache also lacks thread safety - that's also a really, really bad thing for a general cache that you intend on using everywhere. A cache doesn't need to be thread safe if you intend on using it in a single threaded application. However, I'd argue that isn't how most caches are used.

Why don't you use something like MemoryCache to do all the hard work?


Taking action on a webpage(select checkbox or enter textbox or select from a list) I currently have the following code. This function works successfully, however, i would like some feedback because I think it is very messy.

Currently, this function checks if the field is checkbox or textbox or Select. Once it verifies the field it performs an action based on that. For example if it is SELECT it will select the value.

If it cannot find any of the above it throws an error that it cannot find the textbox.

I have defined two variables enterValue and checkboxUncheck=False. enterValue is what user wants to enter. checkboxUncheck variable is used so the user can perform two actions, uncheck the checkbox or check it. By default it checks it. However this can be changed if it is set to checkboxUncheck=True.

Here is the function.:

def take_action(self, enterValue, checkboxUncheck=False):
        try:
            child = [
            self.text.find_element_by_xpath(PageCommonLocatars.FINDCHECKBOXONE),
            self.text.find_element_by_xpath(PageCommonLocatars.FINDCHECKBOXTWO)
            ]

            var = self.text.find_element_by_xpath(PageCommonLocatars.ENTERCHECKBOXVAL % enterValue)
            #var = self.text.find_element_by_xpath('./..//..//div/input[@value="%s"]' % enterValue)
            if checkboxUncheck == True:
                if var.is_selected() == True:
                    self.driver.execute_script("arguments[0].click();", var)
            else:
                if var.is_selected() == False:
                    self.driver.execute_script("arguments[0].click();", var)

        except:
            try:
                Select(self.text.find_element_by_xpath(PageCommonLocatars.FINDSELECT_TYPE)).select_by_visible_text(enterValue)
            except:
                self.text.find_element_by_xpath(PageCommonLocatars.FINDTEXTBOX)
                elemtwo = self.text.find_element_by_xpath(PageCommonLocatars.ENTERTEXTBOXVAL)
                actions = webdriver.ActionChains(self.driver)
                actions.move_to_element(elemtwo)
                elemtwo.clear()
                actions.send_keys(enterValue)
                actions.perform()

        # saves it after making changes
        self.click(10, *PageCommonLocatars.SAVEBUTTON)

Here in a separate locators.py. I have defined my locators

class PageCommonLocatars(object):
  # Login function locators
  ROWPARENT           = '//tr/th//*[text()="%s"]'
  FINDCHECKBOXONE     = './..//..//input[@type="checkbox"]'
  FINDCHECKBOXTWO     = './..//..//input[@type="CheckBox"]'
  ENTERCHECKBOXVAL    = './../../td/input[@value="%s"]'
  FINDSELECT_TYPE     = './..//..//./td/select'
  FINDTEXTBOX         = './../../td/input[@type="text"]'
  ENTERTEXTBOXVAL     = './../..//./td/input'
  SAVEBUTTON          = (By.XPATH, '//button[contains(text(),"Save")]')

  GETSELECTVAL     = './../../../td/select'
  GETTEXTBOXVAL    = './../../td/input[@value="%s"]'
  GETCHECKBOXVAL   = './../../td/input[@value="%s"]'

Multiple things should be fixed:

Stylistic/Code Style issues:

consistent indentation - per PEP8, use 4 spaces
follow the variable naming guidelines (PEP8 reference) - in particular, fix the "camel case" names and add underscores between the words in your variable names in upper case
truthiness checks can be simplified, e.g. if checkboxUncheck instead of checkboxUncheck == True
do not use bare except clause - it really makes things less understandable - catch specific exception and let it fail in all other situations

Selenium specific issues:

your XPath expressions are quite fragile - you are going up and down the tree relying on the depth level and HTML markup too much. Think about using meaningful data-oriented id, class or other attributes to get to the desired elements. And, XPaths are generally less readable and more error-prompt. There are many other location techniques.
clicking elements via JavaScript might lead to problems, see if you can use regular click() method (WebDriver click() vs JavaScript click())
you may benefit from a "Page Element" page object abstraction
I think you should not actually need the "action chains" here:

actions = webdriver.ActionChains(self.driver)
actions.move_to_element(elemtwo)
elemtwo.clear()
actions.send_keys(enterValue)
actions.perform()

and can replace it with just:

elemtwo.clear()
elemtwo.send_keys(enterValue)

Code review and refactoring is a continuous process - if you would apply all the suggested fixes, I bet we'll see more things to improve.


Compare two map objects and show relative information Java 8 I have 2 maps. First map consists of order in which a specific set of services is executed. Second map contains execution time  each service took. I need to display this information.
Following is my piece of code. How can this be improved by performance and syntax?

public static String getAllExecTimes(MasterVO masterVO, boolean displayAll) {
    StringBuilder builder = new StringBuilder();

    Map<Integer, DurationVo> durationMap = null;
    Map<Integer, Integer> orderMap = null;
    List<Integer> keys = null;

    if (displayAll) {
        builder.append("\nSection A\n");
        durationMap = masterVO.getMapA();
        orderMap = masterVO.getOrderMap();

        keys = new ArrayList<>(orderMap.keySet());
        keys.sort(new CustomComparator());

        for (Integer execOrder : keys) {
            Integer target = orderMap.get(execOrder);
            if (null != target) {
                builder.append(durationMap.get(target));
            }
        }

        if (null != masterVO.getMapB()) {
            builder.append("\nSection B\n");
            durationMap = masterVO.getMapB();
            keys = new ArrayList<>(durationMap.keySet());
            for (Integer target : keys) {
                builder.append(durationMap.get(target));
            }
        }
    }

    durationMap = masterVO.getMapC();
    keys = new ArrayList<>(durationMap.keySet());
    keys.sort(new CustomComparator());

    builder.append("\nFinal Section\n");

    for (Integer target : keys) {
        builder.append(durationMap.get(target));
    }

    return builder.toString();
}

private static final class CustomComparator implements Comparator<Integer> {

    public int compare(Integer first, Integer second) {
        return first.compareTo(second);
    }
}

Code review in order of appearance:

public static String getAllExecTimes(MasterVO masterVO, boolean displayAll) {

Static methods should be avoided if you may expect them to contain state, such as hints on how to format the class. Why not create an object that performs the formatting?

Boolean parameters are not a good idea. They are pretty unreadable. But in this case it is worse, as the displayAll doesn't give any hint to a user what actually is displayed. What about an enum, e.g. enum Display {DISPLAY_MAP_C_ONLY, DISPLAY_MAPS_A_B_C;}?

Map<Integer, DurationVo> durationMap = null;
Map<Integer, Integer> orderMap = null;
List<Integer> keys = null;

Never ever instantiate variables before they are needed in Java, and don't use the same variable reference for two different maps (A and C). This makes for very error prone code, especially if you are starting to assign null to the variables, which should almost never be necessary. Create separate methods when code is very similar or when variable names are starting to clash.

Why is there a MasterVO and a DurationVo? It's fine to choose all uppercase or camelcase, but please do so consistently.

durationMap = masterVO.getMapA();
orderMap = masterVO.getOrderMap();

You know, a map is a mapping from a set of keys to values. However, you only have duration and order, and it isn't even clear which one is key and what is the value.

keys = new ArrayList<>(orderMap.keySet());
keys.sort(new CustomComparator());

Now this is just not right. If you put the keys into a TreeSet instead then they will be ordered during insertion. Even better, why not use a TreeMap and sort then when you setup the Map?

if (null != target) {

First of all, you should just write this as if (target != null) {; this is a well known anti-pattern for Java, it's just not needed and it hurts readability. It is item #1: "Yoda Conditions" for the "new jargon" article at coding horror.

The bigger question is why you would allow null values in such a map in the first place. Even without knowing the design, I have the feeling that null values should be avoidable in this case, especially since keys without values are simply skipped in the resulting string.

if (null != masterVO.getMapB()) {

Now whole maps seem to be missing in action. Use Optional<...Map> instead.

private static final class CustomComparator implements Comparator<Integer> {

    public int compare(Integer first, Integer second) {
        return first.compareTo(second);
    }
}

Seriously, how does this custom comparator differ from normal integer ordering? Do you need to specify a specific comparator in that case? And why is it called CustomComperator? That name doesn't explain how it is custom.


Traffic Flow functional way This problem:

  The Shallow Alto city council has organized a special committee to review the traffic situation in the city. Despite the fact that there is nothing whatsoever to do in the city, citizens still write complaints about the amount of time it takes them to get from one unexciting location to the next. The committee has been asked to figure out how long it takes someone to travel down the streets of Shallow Alto given the current traffic light programming. To do this, they want you to write an algorithm which, given the speed a car travels down a road and the timing of the traffic lights, returns the amount of time it takes the car to travel down the street. The traffic lights will be in an int[], with each element representing the amount of time in seconds between signal changes. Initially, all traffic lights have just turned green, and are at the beginning of their cycle. The order of the traffic lights in the int[] will be the order in which a car encounters them. The speed you will be given is in meters per second. Assume that the car starts 150 meters before the first traffic light, that there are 150 meters between each traffic light, and that the car stops 150 meters after the last traffic light. Disregard all acceleration and deceleration; a car is either at its given speed or entirely stopped, and it takes no time to go from one state to the other. If a car reaches a traffic light just as it turns red, it immediately stops and waits for it to turn green again (at which point it starts moving immediately). If the light just turned green, there is no wait and the car drives on through. Return the time, in seconds, that it takes the car to travel the entire distance. Round down any fractional parts, (for example 55.5 becomes 55 and 44.9 becomes 44), but do not do this until returning.

But in F# instead of C#:

namespace TrafficFlow

module Traffic =

    let isGreen currentTime switchItme =
        let state = int currentTime / int switchItme
        state % 2 = 0

    let waitTime (currentTime : float) switchTime =
        match (isGreen currentTime switchTime) with
            | false -> if (currentTime % switchTime) < 0.01 then switchTime else currentTime % switchTime
            | _ -> 0.0

    let time distanceBetween speed  (lights : seq<int>) =
        let timeBetween = float distanceBetween / float speed
        (lights |> Seq.fold (fun acc x -> float acc + timeBetween + waitTime acc (float x) ) timeBetween)

open Traffic
open System

module Program =

    [<EntryPoint>]
    let main argv =

        let distance = 150

        printf "0) time: %d\n" (int (time distance 30  [10; 10; 10]))
        printf "1) time: %d\n" (int (time distance 20  [10; 10; 10]))
        printf "2) time: %d\n" (int (time distance 20  [10; 20; 30]))
        printf "3) time: %d\n" (int (time distance 5  [10; 11; 12; 13; 14; 15]))
        printf "4) time: %d\n" (int (time distance 5  [60; 60; 60; 60; 60; 60; 60; 60; 60; 60]))
        printf "5) time: %d\n" (int (time distance 25  [55;29;26;12;19;39;18;20;23;28;56;20;59;48;33;40;30;60;19]))

        Console.ReadLine() |> ignore

        0 // return an integer exit code

This looks very clean and simple, which is always* a good thing when programming.  First, you don't save state, nor do you have long functions--each function does what says in a couple lines.  One nitpick is your naming:

let isGreen currentTime switchItme

switchItme should probably be switchTime.

You indented the conditions of your match statement too far.  That should be indented as:

match (isGreen currentTime switchTime) with
| false -> if (currentTime % switchTime) < 0.01 then switchTime else currentTime % switchTime
| _ -> 0.0

In F#, you typically don't use if statements, except for choosing a path based on the value of a boolean literal, and I only do that when I can use it like a C# ternary.  I would write the match in your second function as:

match (isGreen currentTime switchTime) with
| true -> 0.0
| false ->
    match (currentTime % switchTime) with
    | c when c < 0.01 -> switchTime
    | _ -> currentTime % switchTime

F# has a printfn function that is the same as printf, except it prints a newline at the end.  You should use this instead of manually specifying you want a newline with printf "...\n"

* Well, sometimes complicated, fast code is better in performance-critical environments.


Verify collection as a method parameter How can I verify that ohterClassMock.MethodToTest() was called with contracts?

That it was called with contract, that have Id = 1 and Param2 = 34 and also with contract2, which has Id = 2 and Param2 = 56.

This is my code:

GoodClass goodClass = new GoodClass(ohterClassMock);
...
var contracts = new List<Contract>();
var contract = new Contract{ Id = 1, Param2 = 34 };
contracts.Add(contract);
var contract2 = new Contract { Id = 2, Param2 = 56 };
contracts.Add(contract2);

goodClass.DoSomething(contracts);

ohterClassMock.Verify(mock => mock.MethodToTest(It.IsAny<contracts>()));

I know that it can be tested for every item in the collection:

ohterClassMock.Verify(mock => mock.MethodToTest(It.Is<Contract>(contract => contract.Id == 1)));
...

but maybe there is some other syntax to call it in one line of code?

Updated:

goodClass.DoSomething(contracts);

calls

foreach(Contract contract in contracts)
{
   ohterClassMock.MethodToTest(contract);
}

Strictly to answer the question at hand, you have local variables for the individual contracts already, so you can just re-use them:

ohterClassMock.Verify(mock => mock.MethodToTest(It.Is<Contract>(c => contract.Equals(c))));
ohterClassMock.Verify(mock => mock.MethodToTest(It.Is<Contract>(c => contract2.Equals(c))));

This assumes you have implemented IEquatable<Contract> and/or overridden Object.Equals on your Contract object.

Given the behavior of most test and mocking frameworks, it will probably save you a lot of grief to go ahead and override Object.ToString so that failed tests will print out nicer expected/actual values than the fully-qualified type names.

Also, as an aside, you can create your list with a collection initializer if you do so after building your individual contracts:

var contracts = new List<Contract> { contract, contract2 };

Or, if your method takes in IEnumerable<Contract>, it may be even simpler to use:

var contracts = new [] { contract, contract2 };


Adding chat to an MVC database I have these relevant tables in an MVC website:

Server
ServerPlayer
ServerPlayerChat

Servers have serverPlayers and ServerPlayers have ServerPlayerChats. Simple enough and expectable.

I have a feeling my code could be drastically condensed (particularly the GetServerPlayer method) and that the GetServerPlayer method is named wrong, although I couldn't think of a better name other than GetOrCreateServerPlayer, which is really long-winded. Also, this is in the Server model. I think I could move the add chat to the server player model, but my concern is that this might be more appropriate in a controller.

Is it good practice to do what I'm doing where I'm not only adding the new chat to the ServerUserChats collection, but also setting ServerPlayer = player inside the new chat? I'm aware that LINQ/EF/... is capable of figuring out relations like this from just one variable but it felt right to do this, so feedback on that is accepted too.

public void AddChat(string playername, string chat, int time)
{
    LastChat = time;
    ServerPlayer player = GetServerPlayer(playername);
    player.ServerUserChats.Add(new ServerPlayerChat() {
        Content = chat,
        Time = time,
        ServerPlayer = player
    });
}

public ServerPlayer GetServerPlayer(string playername)
{
    ServerPlayer player = GetPlayer(playername);
    if (player == null)
    {
        player = new ServerPlayer() {
            Server = this,
            UserName = playername,
        };

        Players.Add(player);
    }

    return player;
}

public ServerPlayer GetPlayer(string playername)
{
    return Players.Where(x => x.UserName == playername).FirstOrDefault();
}

Use Players.SingleOrDefault() instead of Where and FirstOrDefault.
If you use the function GetPlayer just one time, remove it.

public ServerPlayer GetServerPlayer(string playername)
{
    ServerPlayer player =  Players.SingleOrDefault(x => x.UserName == player name);
    if (player == null)
    {
        player = new ServerPlayer() {
            Server = this,
            UserName = playername,
        };

        Players.Add(player);
    }

    return player;
}

Personally, I think you should change GetServerPlayer to GetOrCreateServerPlayer especially if you are a team. We don't have to know or look your function to understand its purpose. Or keep the name and add the appropriates comments (summary, return, etc).

In the case of ASP.NET which follow MVC pattern, I think you must separate the logic and the model.
For my part I create Manager for logic and my controller just serve to call the appropriates methods of my manager.


Password Generator, generate password of desired length While I do know that there are many questions regarding "password generators," I have a different approach and would like to know if it is effective.

1) The below password generator takes on a different approach that other basic password generators, creating an empty string of x length, x being the desired length of the final password, and inserting characters and sub-stringing. I would like to know if this fashion is more effective than adding random characters one-by-one

2) Have I ensured the most random-ness of the characters in the password by using this character set? Or should I rearrange the characters differently?

public static final String CHARACTER_SET = ("aA1bB2cC3dD4eE5fF6gG8hH9iI0jJkKlLmMnNoOpP&qQrRsStT_uUvV-wWxX+yYzZ");

3) I also seek general reminders on java conventions that I do not know.

Here is my full class for reference.

public class PasswordGenerator {

  public static final String VERSION = ("1.0");

  public static final String CHARACTER_SET = ("aA1bB2cC3dD4eE5fF6gG8hH9iI0jJkKlLmMnNoOpP&qQrRsStT_uUvV-wWxX+yYzZ");

  public static void main(String[] args) {
    System.out.println("Welcome to password generator, version: " + VERSION);
    System.out.println(generatePassword(20));
  }

  private static String generatePassword(int passwordLength) {
    String password = "";
    for (int i = 0; i < passwordLength; i++) {
      password = password + " ";
    }
    Random random = new Random();
    for (int i = 0; i < password.length(); i++) {
      if (i == 0) {
        password = String.valueOf(CHARACTER_SET.charAt(random.nextInt(65))) + password.substring(1, password.length());
      } else {
        password = password.substring(0, i) + String.valueOf(CHARACTER_SET.charAt(random.nextInt(65))) + password.substring(i + 1, password.length());
      }
    }
    return password.toString();
  }
}

Note: The 65 comes from the CHARACTER_SET length, which I have found out before.

Note: The 65 comes from the CHARACTER_SET length, which I have found out before.

It's 65 at the moment. But what if you want to support more or less characters? A different character set? You'd have to change those hard-coded values every time.

CHARACTER_SET.charAt(random.nextInt(65)

Can be replaced by:

CHARACTER_SET.charAt(random.nextInt(CHARACTER_SET.length())

There is one more hard-coded value in your code.

System.out.println(generatePassword(20));

Extract the 20 to it's own variable. This allows you to extend your program to, for example, ask for user input and use that value instead. Perhaps the user prefers 16 or 28.


Rendering the periodic table in C++ using SFML I have created this as an example. It uses SFML as its core dependency along with JSON for modern C++ to load the elements from a file.

Basically what happens is when an object that contains an instance of sf::Drawable is created, its constructor will be responsible for adding it to the draw list.  Its destructor is responsible for removing it from the list.

I am interested in if this is a good approach to dealing with the objects in my program that will need to be rendered or if I should be extending sf::Drawable class directly.

I am also interested in the overall code quality of the entire project. I am aware of the assignment and copy constructors that do nothing.  I am relatively new to C++ (1 semester).

Drawables.cpp

#include "Drawables.hpp"

namespace {
    std::vector<std::shared_ptr<sf::Drawable>> drawList;
}

std::vector<std::shared_ptr<sf::Drawable>>& getDrawList()
{
    return drawList;
}

Element.cpp constructor and destructor

#include "Element.hpp"

#include <exception>

Element::Element()
{
    drawable = std::shared_ptr<sf::RectangleShape>( new sf::RectangleShape(sf::Vector2f{49, 49}) );
    drawable->setFillColor(sf::Color::Green);
    getDrawList().push_back(drawable);
}

Element::~Element()
{
    // Remove it's drawable from the drawList
    for(int i = getDrawList().size() - 1; i >= 0; --i) {
        if(dynamic_cast<sf::RectangleShape*>(getDrawList()[i].get()) == drawable.get())
        {
            getDrawList().erase(getDrawList().begin() + i);
        }
    }
}

Client.cpp draw function

void Client::draw()
{
    while(window.isOpen()) {
        window.clear();
        for(std::shared_ptr<sf::Drawable> obj : getDrawList()) {
            window.draw(*obj);
        }

        window.display();
    }
}

Looking at the code you've posted here, it's pretty straightforward and I'm able to understand what it does. That's great! I have a few suggestions on how to improve it.
Named Types
One common complaint about the standard template library and templates in general in C++ is the ridiculously long type names that result from using them. I see that in your code here. It's easily solved by creating new named types that represent the common types you use. For example:
std::vector<std::shared_ptr<sf::Drawable>>

uses 3 types with namespaces. If you did this:
typedef std::shared_ptr<sf::Drawable> sharedDrawable;

Then your new type becomes:
std::vector<sharedDrawable>

Or even better, use the new using directive in C++11:
using sharedDrawableVector = std::vector<std::shared_ptr<sf::Drawable>>;

Or something along those lines.
Use C++!
You're writing in C++, but you're using older methods of iterating through containers. In the destructor of Element you're going through the list manually when you could use a std::find() method instead.
Magic Numbers
In the constructor of Element you create a vector with coordinates <49, 49>. What does it mean? What is it? Why <49, 49>? You should create a constant with a name that describes what this represents:
const sf::Vector2f kRectOriginVector {49, 49}; // Or whatever it means

Then use that in the constructor.


Inserting data into MySQL My code currently doesn't insert data into MySQL fast enough. However I don't know how to measure the current speed.
def insert():
      mycursor = mydb.cursor()
      plik = input(":::")
      agefile = open("C:/Users/juczi/Desktop/New Folder/" + plik, "r")
      sql = "INSERT INTO wyciek (nick, ip) VALUES (%s, %s)"
      for row in agefile:
            mydb.commit()
            data = row.split(':')
            val = (data[0].replace("\n",""), data[1].replace("\n", ""))
            mycursor.execute(sql, val)
      agefile.close()

If you are trying to insert data in bulk to Mysql the best would be to use LOAD DATA instead of doing it in a loop on top of an interpreted language, which is inevitably slower. However there is less flexibility if you need to transform data but that shouldn't be a problem in your case. All you're doing is get rid of carriage return. This is a plain delimited file without heavy transformation.
The mydb.commit() is misplaced. It should be called after running an insert/update. On the first iteration there is nothing to be committed.
But this is not efficient. I suggest that you take full advantage of transactions. Start a transaction at the beginning, do your loop and commit at the end. In case of error, you roll back. The added benefit is data integrity. If an error occurs, the data already inserted is "undone" so you don't end up with garbage due to a partial import of data.
Schema:
try:
    conn.autocommit = False

    # insert loop..

    # Commit your changes
    conn.commit()

except mysql.connector.Error as error:
    print("Failed to update record to database rollback: {}".format(error))
    # reverting changes because of exception
    conn.rollback()

See for example: Use Commit and Rollback to Manage MySQL Transactions in Python
This alone should improve performance but you have to test it to measure the gain.
Not performance-related, but you can also use a context manager whenever possible, for instance when opening/writing to a file. So:
agefile = open("C:/Users/juczi/Desktop/New Folder/" + plik, "r")

becomes:
with open("C:/Users/juczi/Desktop/New Folder/" + plik, "r") as agefile:
   # do something

and you don't have to bother closing the file, this is done automatically.


Producer consumer that simulates the interaction of threads controlling a hardware device I have a producer consumer that simulates the interaction of threads controlling a hardware device.

I have 1 thread controlling the device, 1 thread reading, and 1 thread outputting.  The read is synchronous on the thread while the controlling is asynchronous.

My producer is the thread controlling the device and my consumer is a thread that processes the data. Often the consumer takes about the same amount of time as the producer (ideal), but sometimes it takes more time.

The hope is  that the producer should not produce more than the consumer can process. This is partly due to hardware overflow, the consumer will free a buffer on the device, and partly due to visual latency.

I think the producer should be responsible for regulating the rate of production. I implemented this regulation using a sleep timeout.

I am wondering if this is the best way to do it.

My heuristic for success is the time output of the redraw command, which should be roughly ConsumerTime*8 (maybe 400).

#include <cstdlib>
#include <thread>
#include <condition_variable>
#include <iostream>
#include <atomic>
#include <chrono>
//UTILITY
//
using namespace std;
#if !defined(WIN32)
#include <unistd.h>
void
_sleep (int ms)
{
    usleep (ms * 1000); //convert to microseconds
}
#endif
long long
timestamp ()
{
    auto now = std::chrono::system_clock::now ();
    auto duration = now.time_since_epoch ();
    auto millis = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count ();
    return millis;
}
//REDRAW
//
bool killRedraw;
bool drawIt;
long long last;
void
redraw ()
{
    cout << "Starting drawing thread" << endl;
    last = timestamp ();
    while (!killRedraw)
    {
        if (drawIt)
        {
            long long now = timestamp ();
            cout << "I drew it:" << now - last << endl;
            last = now;
            drawIt = false;
        }
        _sleep (15);
    }
}
//PRODUCER
//
atomic<int> balance;
bool killProducer;
void
produce ()
{
    cout << "Made one:" << ++balance << endl;
}
void
producer ()
{
    balance = 0;
    cout << "Starting producer thread" << endl;
    while (!killProducer)
    {
        _sleep (40);
        produce ();
        while (balance > 1)
        {
            _sleep (10);
        }
    }
}
//CONSUMER
//
bool killConsumer;
void
consume ()
{
    cout << "Ate one:" << --balance << endl;
}
void
consumer ()
{
    cout << "Starting consumer thread" << endl;
    while (!killConsumer)
    {
        int basetime = 35;
        int noise = rand () % 20;
        _sleep (basetime + noise);
        consume ();
        static int cycle = -1;
        if ((cycle = ++cycle % 8) == 7)
        {
            drawIt = true;
        }
    }
}
//Main
//
int
main (int argc, char** argv)
{
    //
    // Important metric is the "draw it" time, which should be about 8*50
    //
    killProducer = false;
    std::thread tP (producer);
    killConsumer = false;
    std::thread tC (consumer);
    killRedraw = false;
    drawIt = false;
    std::thread tD (redraw);
    _sleep (10000);
    killProducer = true;
    killConsumer = true;
    killRedraw = true;
    tD.join ();
    tP.join ();
    tC.join ();
    //
    return 0;
}

I have some minor things to point out:

There is not a single empty line in this code, and this essentially looks like globals, functions, and comments stacked on top of each other.  You should have each type separated so that the code is easier to read (especially with the function names and return types on separate lines).
Since you're using C++11, you should be using <random> in place of std::rand.  It has been determined that rand() is harmful and should no longer be used in C++11.
Unless I'm misunderstanding the use of multithreading here, I don't think you should be using any global variables, as they're prone to bugs.  If that's the case, then you should just be passing them to functions as needed.  If not, then at least put them all towards the top (they're already global, so it doesn't make a difference to have them in different places outside the functions).
You don't need to be using std::endl everywhere.  It doesn't appear that you're needing to flush the buffer each time, which can also affect performance.  Instead, output a "\n" to produce a newline without also flushing the buffer.


The intersection of two polygons in C++ I implemented the intersection of two (convex) polygons in C++. It finds the polygon in the intersection like in this image.
Looking for any and all feedback. I left the logic for line intersection and point containment out.
#include <algorithm>
#include <cmath>
#include <iostream>
#include <optional>

using std::atan2;
using std::move;
using std::optional;
using std::vector;
using std::sort;

struct Point {
  double x;
  double y;
};

struct LineSegment {
  Point u;
  Point v;
};

// Returns the intersection point of l1 and l2 if it exists, nullopt otherwise.
// Unimplemented.
optional<Point> lineIntersection(const LineSegment& l1, const LineSegment& l2);

class Polygon {
public:
  Polygon(const vector<Point>& points) : points_{points} {};
  Polygon(const vector<Point>&& points) : points_{move(points)} {};

  // Returns true if point is in Polygon, false otherwise.
  // Unimplemented.
  bool contains(const Point& point) const;

  static Polygon intersect(const Polygon& p1, const Polygon& p2) {
    vector<Point> points;
    for (size_t i = 0; i < p1.points_.size(); ++i) {
      // Add points of p1 contained in p2.
      if (p2.contains(p1.points_[i])) {
        points.emplace_back(p1.points_[i]);
      }
      for (size_t j = 0; j < p2.points_.size(); ++j) {
        // Add points of intersection.
        auto intersection = lineIntersection(
          LineSegment{p1.points_[i], p1.points_[(i+1) % p1.points_.size()]},
          LineSegment{p2.points_[j], p2.points_[(j+1) % p2.points_.size()]});
        if (intersection != nullopt) {
          points.emplace_back(move(intersection.value()));
        }
      }
    }

    // Add points of p2 contained in p1.
    for (size_t i = 0; i < p2.points_.size(); ++i) {
      if (p1.contains(p2.points_[i])) {
        points.emplace_back(p2.points_[i]);
      }
    }

    // Sort into counter-clockwise order.
    sort(points.begin(), points.end(), [](Point a, Point b){ return atan2(a.y, a.x) > atan2(b.y, b.x); });
    return Polygon{points};
  }

private:
  vector<Point> points_;
};

Avoid using in header files
It looks like the code you have written is in a header file, which is to be included in an actual application that needs to intersect polygons. If you include this file, it also means you pull in all the using declarations, which could result in unexpected behavior. It might be safe to move the using declarations into class Polygon, but I'd rather avoid doing it altogether.
You also forgot using std::nullopt.
The move constructor should take a non-const reference
Since you want to  move things from an object, that object might have to be modified, so move constructors should take non-const references.
Consider adding operator&()
Polygon intersection shares a lot of similarities with bitwise AND operations. It makes sense to add an operator&() to your class, so you can write:
Polygon a{...}, b{...};
Polygon c = a & b;

Your sorting function is wrong
You can't sort polygons into counter-clockwise order the way you did. The angles you calculate are relative to coordinate (0, 0), but the polygons themselves can be anywhere. You have to calculate the angles relative to a point inside the intersection.
See this StackOverflow question for a possible algorithm.
Alternatively, since the points of the input polygons are already sorted, and the intersection of two convex polygons is just two pieces of each polygon concatenated, you should not need to sort at all.


Finding subtuple in larger collection? I've got a flat array of < 1000 items and I need to find the indices of a smaller tuple/array within the array.  The tuple to find can be a varying length (generally between 2 and 5 items, order is important).

This is my initial naive implementation.  My main concerns are:

This seems like a CS 101 problem, so I'm pretty sure I'm overdoing it.
Readability.  I can break this down into smaller methods, but it's essentially a utility function in a much larger class.  I guess I could extract it into its own class, but that feels like overkill as well.  As-is, it's just too long for me to grok the whole thing in one pass.

public int[] findIndices(final Object[] toSearch, final Object[] toFind)
{
    Object first = toFind[0];

    List<Integer> possibleStartIndices = new ArrayList<Integer>();
    int keyListSize = toFind.length;
    for (int i = 0; i <= toSearch.length - keyListSize; i++)
    {
        if (first.equals(toSearch[i]))
        {
            possibleStartIndices.add(i);
        }
    }

    int[] indices = new int[0];
    for (int startIndex : possibleStartIndices)
    {
        int endIndex = startIndex + keyListSize;
        Object[] possibleMatch = Arrays.copyOfRange(toSearch, startIndex, endIndex);
        if (Arrays.equals(toFind, possibleMatch)) {
            indices = toIndexArray(startIndex, endIndex);
            break;
        }
    }
    return indices;
}

private int[] toIndexArray(final int startIndex, final int endIndex)
{
    int[] indices = new int[endIndex - startIndex];
    for (int i = 0; i < indices.length; i++)
        indices[i] = startIndex + i;

    return indices;
}

Here is an O(mn) solution.  Given that haystack is about 1000 in length and needle is 5 or smaller, the simplest code to do the search is probably the best.  But if testing for equality is expensive, there are things we can do to mitigate that, although I'm not sure switching to KMP or BM will help so much given that we're dealing with Object here.

// assumes no null entries
// returns starting index of first occurrence of needle within haystack or -1
public int indexOf(final Object[] haystack, final Object[] needle)
{
    foo: for (int a = 0; a < haystack.length - needle.length; a++) {
        for (int b = 0; b < needle.length; b++) {
            if (!haystack[a+b].equals(needle[b])) continue foo;
        }
        return a;
    }
    return -1;
}


Python script for making a table I have written a Python 2.6.6 script in order to make tables by taking the third column of every text file in a particular subgroup (signal and background in this case[particle physics]). However, I used the exec command too often and I think this code can be written more elegantly.

#!/usr/bin/python
import sys
stringpu = ["50PU", "140PU"]
stringsig = ["NM1", "NM2", "NM3", "Scenario4", "Scenario6"]
stringbkg = ["TTbar", "DiBoson", "TopJets", "BosonJets"]

for a in range(len(stringsig)):
    exec("FILESIG"+str(a)+" = open('SummerStd_'+stringpu[1]+'_'+stringsig[a]+'.txt', 'r')")
    exec("LINESIG"+str(a)+" = FILESIG"+str(a)+".readlines()")

for b in range(len(stringbkg)):
    exec("FILEBKG"+str(b)+" = open('SummerStd_'+stringpu[1]+'_'+stringbkg[b]+'.txt', 'r')")
    exec("LINEBKG"+str(b)+" = FILEBKG"+str(b)+".readlines()")

table = open('table.txt', 'w')
table.write("\\begin{table}[h]  \n")
table.write("\\centering \n")
table.write("\\begin{tabular}{|c|c|c|c|c|c|} \n")
table.write("\\hline  & \\textbf{NM1} & \\textbf{NM2} & \\textbf{NM3} & \\textbf{STOC} & \\textbf{STC} \\\\ \n")

n = 0
for line in range(len(LINESIG0)):
    n += 1
    for i in range(len(stringsig)):
        exec("wordsig"+str(i+1)+" = LINESIG"+str(i)+"[line].split()")
    if n > 2:
        table.write("\hline  \n")
        table.write(wordsig2[2]+"  &  "+wordsig1[3]+"  &  "+wordsig2[3]+"  &  "+wordsig3[3]+"  &  "+wordsig4[3]+"  &  "+wordsig5[3]+" \\\\ \n")

table.write("\hline \n")
table.write("\end{tabular} \n")
table.write("\end{table} \n")

table.write("\\begin{table}[h]  \n")
table.write("\\centering \n")
table.write("\\begin{tabular}{|c|c|c|c|c|c|} \n")
table.write("\\hline  & \\textbf{TTbar} & \\textbf{DiBoson} & \\textbf{TopJets} & \\textbf{BosonJets}  \\\\ \n")

n = 0
for line in range(len(LINEBKG0)):
    n += 1
    for i in range(len(stringbkg)):
        exec("wordbkg"+str(i+1)+" = LINEBKG"+str(i)+"[line].split()")
    if n > 2:
        table.write("\hline  \n")
        table.write(wordbkg2[2]+"  &  "+wordbkg1[3]+"  &  "+wordbkg2[3]+"  &  "+wordbkg3[3]+"  &  "+wordbkg4[3]+" \\\\ \n")

table.write("\hline \n")
table.write("\end{tabular} \n")
table.write("\end{table} \n")
table.close()

I took courses on programming in Python before, but I do not remember the concepts of dictionaries and how to assign variables with dynamic names (such as changing inside a for loop).

Before rewriting the program, I'll start with some smaller issues.

Minor simplifications

Use print and '''long strings''' instead of multiple file.write() calls to get newlines.  Use r'' (raw strings) to avoid doubling backslashes.
I would just write the tables to standard output, and redirect the output to a file using the shell.  That avoids hard-coding 'table.txt' in your program.
Use for line in range(1, len(LINESIG0)) to skip the first line.
Ideally, you should rename your files to coincide with their column headings, so that you don't have to map 'STOC' → SummerStd_140PU_Scenario4.txt.

Bugs

The third column of a line would be line[2], not line[3].
Calling open() without calling close() results in file descriptor leaks.  Better yet, use a with block to open files, so that they will be closed automatically.  (For a small number of files like this, in a "throwaway" script, you won't run out of file descriptors.  It's a poor habit, though.)

Rewriting

exec() is a bad idea.  It's hard to interpret your code because I have to imagine what the generated code will look like before mentally executing it.  It's complicated further by the fact that some of the string handling happens when preparing the statement ("FILESIG"+str(a) in the line below) and some of it happens inside the evaluation (for example, +stringsig[a]+).

exec("FILESIG"+str(a)+" = open('SummerStd_'+stringpu[1]+'_'+stringsig[a]+'.txt', 'r')")

As you suspected, code that deals with many similar variables should be stored in a data structure such as a list (or multi-dimensional list) or dictionary, not multiple variables.

To generate two tables, you have duplicated the code.  A function should be defined instead.

def table(filenames, headings):
    data = []
    for fn in filenames:
        with open(fn) as f:
            data.append([line.split() for line in f])

    print r'''\begin{table}[h]
\centering
\begin{tabular}{|c|%s|}
\hline''' % ('|'.join('c' for _ in headings))
    print ' & '.join([''] + headings)

    for line in range(1, len(data[0])):
        print r'\hline'
        print ' & '.join([data[2][line][2]] + [f[line][3] for f in data])

    print r'''\hline
\end{tabular}
\end{table}'''

signal_filenames = ['SummerStd_140PU_%s.txt' % (s) for s in
     ['NM1', 'NM2', 'NM3', 'Scenario4', 'Scenario6']
]
table(signal_filenames, ['NM1', 'NM2', 'NM3', 'STOC', 'STC'])

background_filenames = ['SummerStd_140PU_%s.txt' % (s) for s in
     ['TTbar', 'DiBoson', 'TopJets', 'BosonJets']
]
table(signal_filenames, ["TTbar", "DiBoson", "TopJets", "BosonJets"])