C# Datagridview to Access database I feel like there would be a better way to this. Essentially, I am transporting the data from an excel file to a DGV, and then from a DGV to an Access database. Should I cut the middle man or is this fine the way it is? The problem here is that my cells structure has to match access very closely to work.

private void btnImport_Click(object sender, EventArgs e)
    {
        DialogResult dr = MessageBox.Show(
            "Warning: when importing data into the Access database, ensure that the field columns match Access's fields or the file may become corrupt. Do you still wish to proceed?","Import caution",
            MessageBoxButtons.OKCancel, MessageBoxIcon.Warning);
        if (dr == DialogResult.OK)
        {
            try
            {

                using (OleDbConnection conn = new OleDbConnection(Con))
                {
                    using (OleDbCommand cmd = new OleDbCommand())
                    {
                        cmd.Connection = conn;
                        conn.Open();
                        cmd.CommandText =
                                "Insert INTO ACTB (FirstName, LastName, GrossIncome, LessTNT, TaxableIncomeCE, TaxableIncomePE, GrossTaxableIncome, LessTE, LessPPH, NetTax, TaxDue, HeldTaxCE, HeldTaxPE, TotalTax, PersonID) " +
                                "VALUES(@First, @Last, @Gross, @LessTNT, @TCI, @ADDTI, @GTI, @LessTE, @LessPPH, @LessNTI, @TD, @TWCE, @TWPE, @TATW, @PersonID)";
                        for (int s = 0; s < DGVExcel.Rows.Count - 1; s++)
                        {
                            cmd.Parameters.Clear();
                            //cmd.Parameters.AddWithValue("@ID", Convert.ToInt32(txtID.Text));
                            cmd.Parameters.AddWithValue("@First", DGVExcel.Rows[s].Cells[0].Value);
                            cmd.Parameters.AddWithValue("@Last", DGVExcel.Rows[s].Cells[1].Value);
                            cmd.Parameters.AddWithValue("@Gross", Convert.ToDouble(DGVExcel.Rows[s].Cells[2].Value));
                            cmd.Parameters.AddWithValue("@LessTNT",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[3].Value));
                            cmd.Parameters.AddWithValue("@TCI", Convert.ToDouble(DGVExcel.Rows[s].Cells[4].Value));
                            cmd.Parameters.AddWithValue("@ADDTI", Convert.ToDouble(DGVExcel.Rows[s].Cells[5].Value));
                            cmd.Parameters.AddWithValue("@GTI", Convert.ToDouble(DGVExcel.Rows[s].Cells[6].Value));
                            cmd.Parameters.AddWithValue("@LessTE", Convert.ToDouble(DGVExcel.Rows[s].Cells[7].Value));
                            cmd.Parameters.AddWithValue("@LessPPH",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[8].Value));
                            cmd.Parameters.AddWithValue("@LessNTI",
                                Convert.ToDouble(DGVExcel.Rows[s].Cells[9].Value));
                            cmd.Parameters.AddWithValue("@TD", Convert.ToDouble(DGVExcel.Rows[s].Cells[10].Value));
                            cmd.Parameters.AddWithValue("@TWCE", Convert.ToDouble(DGVExcel.Rows[s].Cells[11].Value));
                            cmd.Parameters.AddWithValue("@TWPE", Convert.ToDouble(DGVExcel.Rows[s].Cells[12].Value));
                            cmd.Parameters.AddWithValue("@TATW", Convert.ToDouble(DGVExcel.Rows[s].Cells[13].Value));
                            cmd.Parameters.AddWithValue("@PersonID", (DGVExcel.Rows[s].Cells[14].Value));
                            cmd.ExecuteNonQuery();
                            Console.WriteLine(s);
                        }
                    }

                }
            }
            catch (OleDbException ex)
            {
                MessageBox.Show("Import error: " + ex);
            }
            finally
            {
                Restore();
            }

        }
    }

Con

Use full names. If it's a connection string then name it ConnectionString especially if it's out of scope and one cannot easily determine what it is.

    using (OleDbConnection conn = new OleDbConnection(Con))
    {
        using (OleDbCommand cmd = new OleDbCommand())
        {

You can join both usings together:

    using (OleDbConnection conn = new OleDbConnection(Con))
    using (OleDbCommand cmd = new OleDbCommand())
    {

for (int s = 0; s < DGVExcel.Rows.Count - 1; s++)

I don't see any reason for not using the i for the index. If it was the r it would at least stand for row but what does the s mean?

Insert INTO ACTB (FirstName, LastName,... VALUES(@First, @Last,

Why do you invent new names for the parameters? Isn't it easier to just copy/paste the one you already have for the columns?

Besides with so many colums consider using an array and build the query dynamically. You can create an array with a collection initializer like this:

var columns = new []
{
    "FirstName",
    "LastName",
    // ...
}

Then you join all names into a string separated by , as column names and another string also separated by commas but with the @ prefix for the values:

var sql =
    $"Insert INTO ACTB ({string.Join(", ", columns)} " +
    $"VALUES({string.Join(", ", columns.Select(x => $"@{x}"))}";

If you create the columns array with the names in the right order (like in your excel sheet) you can even reuse them to populate the parameters. Here's the complete example.

using (var conn = new OleDbConnection(Con))
using (var cmd = new OleDbCommand())
{
    cmd.Connection = conn;
    conn.Open();

    var columns = new[]
    {
        "FirstName",
        "LastName",
        // ...
    }

    var sql =
        $"Insert INTO ACTB ({string.Join(", ", columns)} " +
        $"VALUES({string.Join(", ", columns.Select(x => $"@{x}"))}";

    for (var i = 0; i < DGVExcel.Rows.Count - 1; i++)
    {
        cmd.Parameters.Clear();
        for (var j = 0; j < columns.Length; j++)
        {
            cmd.Parameters.AddWithValue($"@{columns[j]}", DGVExcel.Rows[i].Cells[j].Value);
        }

        cmd.ExecuteNonQuery();
        Console.WriteLine(i);
    }
}


Stack implementation in C++ using linked list I am learning C++ and I made a simple stack class using a (reversed) linked-list to test my knowledge.

Are there any problems with the following code, or any suggestions to improve it?

I want to make sure I am getting things correct in the beginning so I don't make the same mistakes again in the future - especially memory management and avoiding leaks.

One thing to point out is that I included the implementation in the header file... I wouldn't normally do this but apparently there are problems when implementing methods with templates in .cpp files.

#ifndef TEST_STACK_H
#define TEST_STACK_H

#include <stdexcept>

template <class T>
class stack {

    struct node {
        T data;
        node* previous;

        node(T data, node *previous) : data(data), previous(previous) {}
    };

    node* head = nullptr;

    int size = 0;
    int max = -1; // -1 so isFull() == false when default constructor used

public:
    stack() = default;

    stack(int max) {
        if (max <= 0) throw std::out_of_range("stack size must be > 0");
        this->max = max;
    }

    ~stack() {
        node* n = head;

        while (n != nullptr) {
            node* previous = n->previous;
            delete n;

            n = previous;
        }
    }

    void push(const T &object) {
        if (isFull()) throw std::overflow_error("cannot push to a full stack");

        head = new node(object, head);
        ++size;
    }

    T pop() {
        if (head == nullptr) throw std::underflow_error("cannot get item from empty stack");

        T item = head->data;
        head = head->previous;

        --size;

        delete head;
        return item;
    }

    T peek() {
        if (head == nullptr) throw std::underflow_error("cannot get item from empty stack");
        return head->data;
    }

    int getSize() {
        return size;
    }

    bool isFull() {
        return size == max;
    }

    bool isEmpty() {
        return head == nullptr;
    }
};

#endif

Bug

Your main issue is memory management.

You did not implement the "Rule of Three" (you can google it).

The problem is that if you do not define the copy constructor or the assignment operator the compiler will generate these methods for you automatically. Under most conditions these generated methods work correctly. BUT when your class contains an "Owned" pointer they do not work.

Note: An "Owned" pointer is a pointer you are responsible for deleting.

{
    stack<int>   x;
    x.push(12);

    stack<int>   y(x);  // Copy constructor used.
                        // The default implementation does a shallow copy
                        // of each member from x into y.
                        // This means that x and y point at the same list.

}
// Here your destructor has destroyed the same list twice.
// This is a bug.

To fix this you need to define the copy constructor and assignment operator. But there is a nice pattern that allows you to define the assignment operator in terms of the copy constructor. Look up the "Copy And Swap Idiom".

You need to add the following to your class:

class stack
{
    // Stuff
    public:
        stack(stack const& rhs)
            : head(copyList(rhs.head))
            , size(rhs.size)
            , max(rhs.size)
        {}
        stack& operator=(stack const& rhs)
        {
            stack tmp(rhs);            // make a copy using copy constructor.
            swap(tmp);                 // swap the tmp and this object
            return *this;
        }
        void swap(stack& other) noexcept
        {
            using std::swap;
            swap(head, other.head);
            swap(size, other.size);
            swap(max,  other.max);
        }

    private:
        node* copyList(node* l)
        {
            if (l == nullptr) {
                return null;
            }
            return new node{l->data, copyList(l->previous)};
        }
    // STUFF
};

Your pop() has a bug. You delete the NEW head item before returning but leak the original head item.

T pop() {
    if (head == nullptr) throw std::underflow_error("cannot get item from empty stack");

    T item = head->data;
    head = head->previous;     // You just leaked the old head.
                               // You need to keep a pointer to the old head

    --size;

    delete head;               // So you can delete the old head here.
    return item;
}

Other Stuff

Design of pop()

You make your pop() method return the top value and remove it from the stack. This is fine if your T type is simple. But if T is a complex type there is no way to do this safely (and maintain "Strong Exception Guarantee"). So most implementations of stack split this into two separate functions. A top() that returns the top value and a pop() that simply removes the top value.

So I would rewrite this:

void pop() {
    if (head == nullptr) throw std::underflow_error("cannot get item from empty stack");

    node* old = head;
    head = head->previous;

    --size;
    delete old;
}
T const& top() {
    if (head == nullptr) throw std::underflow_error("cannot get item from empty stack");

    return head->data;
}

Return by reference

Your pop() and peek() return the result by value. This is OK for simple types of T (like integer). But if T is a complex object you are making a copy of this complex object. Instead you should return a reference to the object. If the user is doing somehting simple they can do the action without copying if they want to keep a copy they can make that decision and save the value in a local variable.

T peek()

// Change to:

T const& peek();   // Don't really need this if you have top()
                   // Or you could use peek instead of top()

But notice the const& as the return type. You are returning a reference to the object so no copy is made. If you need a local copy then you can save it like this:

int   val = x.top();
x.pop();


Commit the working directory to different branch and switch back The aim of this script is to snapshot the current working directory of a git repository to another branch. Afterwards, it switches back to the original branch and restores the original state, including the working directory and staged changes.

#!/bin/sh

function git_snapshot() {
  # Usage: SNAPSHOT=$(git_snapshot <backup_branch> "<commit message>")
  set -e

  # Save the current state.
  BEFORE=$(git rev-parse --abbrev-ref HEAD)
  if [[ "$BEFORE" == 'HEAD' ]]; then
    BEFORE=$(git rev-parse HEAD)
  fi
  (>&2 echo "Starting at $BEFORE.")
  git stash push --include-untracked
  (>&2 echo "Stashed changes ($(git rev-parse stash@{0})).")

  # Commit files to other branch.
  git checkout "$1"
  git read-tree -um $BEFORE
  git commit -m "$2"
  (>&2 echo "Saved last commit ($(git rev-parse HEAD)).")
  git stash apply --index stash@{0}
  git add -A
  git commit --amend --no-edit
  (>&2 echo "Amended uncommitted changes ($(git rev-parse HEAD)).")
  SNAPSHOT=$(git rev-parse HEAD)

  # Switch back and restore state.
  git checkout $BEFORE
  git stash apply --index stash@{0}
  (>&2 echo "Restored changes.")

  echo $SNAPSHOT
}

Putting it through shellcheck.net results in quite a lot of cleanups.

Apart from the issues raised there, I'd suggest that at the end of your script, you might want to git stash pop instead of git stash apply when restoring your state.

Inside the function body, you may use local to declare the BEFORE variable. While local is not POSIX compliant, all of GNU and BSD compliant shells support the local keyword.


Create an alternative wait in javascript Im creating an single page application with not reload or redirects so I display all my contents with partial view(since im using MVC4 and angularjs ) and modals.

But I came with a situation, and that is that in some cases when i request a view with ajax to the server I need to perform some action(javascript loads) on the view when load but as you know i have to way for the response(the view) to execute this since still not present in the DOM. So I came with this solution. I just develop a method that allows me to wait for a piece HTML with timeouts and recursions and then execute the code when the HTML is render in the view.

//this is actually a service in angular, im just giving an example
    function $WaitFor(selector,callback,timeout)
    {
    var execute = function()
    {
    if(document.Selector(selector) != null)
    callback();
    else
    startTimeout()
    }

    var startTimeout = function()
    {
    setTimeout(execute, 200 || timeout)
    }

    startTimeout();

    }

And indeed the code works, it waits and execute the code successfully when the response(the view) is display. But, is that code ok? is there any another alternative that? And what about performance? Any sugestions/help you can give me??

Read your framework's documentation

Before you actually do some voodoo magic on your code, I suggest you read your framework's documentation. It's quite impossible that a web framework has not taken into account JS's asynchronous nature. There should be a proper way to do this.

For instance, in Meteor we have the .rendered handler which runs when the template has rendered or rerendered. This is usually where we hook stuff or modify the DOM to the newly rendered template.

Mutation observers

This is one of the new, shiny toys of JS and should be the proper way to do this operation in plain JS. Mutation Observers listen for changes on the DOM and fire assigned callbacks. You can configure them to listen for specific changes on the DOM, like attribute modifications, subtree modifications (adding or removing DOM). As of this writing, current browsers support them already.

Using AJAX? The callbacks!

When there is AJAX, there should be some mechanism to attach handlers to know when the data arrived. Use that instead.

For example, in jQuery you can do this:

$.get('some_resource_url')
 .done(function(data){
   //Do something with the data when it arrives
 });


First you should use a Method for repeating a character/String (if there isn't one yet. Don't know, never need it so far). Like a repeat-Method ->

    public String repeat(int times, String str) {
        // Add some security checks ofc, just to show what I mean.
        String r = "";
        for (int i=0; i < times; i++) r += str;
        return r;
    }

So, since multible characters won't be a problem now anymore:

    // ...
    // Amount of columns you want to have
    int col = 3;
    // Maximal Length of a String you support
    int len = 20;
    for (int i=0; i < theArray.length; i+=col) {
        System.out.println(repeat(len+col+1,"-"));
        System.out.print("|");
        for (int c = 0; c < i; c++)
            // Add some checks to align it into the center when you want/need it
            System.out.print(i + repeat(len-(i+"".length()), " ") + "|");
        System.out.print("\n");
        System.out.println(repeat(len+col+1, "-"));
        for (int c = 0; c < i; c++)
            // Filling up the rest of the String with spaces so the table wont freak out
            System.out.print(theArray[i+c] + repeat(len-theArray[i+c].length(), " ") + "|");
        System.out.print("\n");
    }
    System.out.println(repeat(len+col+1, "-"));


Configurable products price class Introduction:

I have a website where customers can configure and checkout glass products like for example: Shower doors, Mirrors, Kitchen splashbacks, Table tops etc.

What I am trying to do:

I am trying to create a class for calculating the price of this kind of configurable products. All the products have in common that they use square meters to calculate the price and some of the products have the same way of calculating the glasstype price.

The idea:

I have created an abstract class with the methods and properties which the products have in common. The idea is to create a class for each product with their exceptions (own methods and properties).

My question:

Am I doing the right thing or should I approach this differently? For example calling the methods in the constructor in my showerdoor class.

The abstract class:

abstract class ConfigurableProduct {

    public $squareMeters;

    // Stores the prices
    protected $_price = array();

    // Initializes object
    public function __construct( $data = array() ) {

        // Set square meters
        $this->squareMeters = $data['square_meters'];

    }

    // Adds price to _price array
    public function addPrice( $value ) {
        $this->_price[] = $value;
    }

    // Sums prices in _price array
    public function getPrice() {
        return array_sum( $this->_price );
    }

}

Showerdoor class:

class Showerdoor extends ConfigurableProduct {

    // Initializes object
    public function __construct( $data ) {

        // Parent constructor
        parent::__construct( $data );

        // Set glasstype
        if ( isset( $data['glasstype'] ) )
        $this->setGlasstype( $data['glasstype'] );

        // Set coating
        if ( isset( $data['coating'] ) )
        $this->setCoating( $data['coating'] );

        // Set hinges
        if ( isset( $data['hinges'] ) )
        $this->setHinges( $data['hinges'] );

    }

    // Adds the glasstype price to the _price array
    public function setGlasstype( $id ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price
            $value = gtp_wizard_part_price( $id ) * $this->squareMeters;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

    // Adds the coating price to the _price array
    public function setCoating( $id ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price
            $value = gtp_wizard_part_price( $id ) * $this->squareMeters;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

    // Adds the hinges price to the _price array
    public function setHinges( $id, $quantity = 2 ) {

        // Check for part id (so integer)
        if ( isset( $id ) && is_int( $id ) ) {

            // Calculate price (multiply for 2 hinges)
            $value = gtp_wizard_part_price( $id ) * $quantity;

            // Add price to price array
            $this->addPrice( $value );

        }

    }

}

How I create the object:

// Dimensions
$width          = 800;
$height             = 2000;
$square_meters  = $width * $height / 1000000;

// Parts (ids from database)
$glasstype      = 282236;
$hinges         = 282242;
$coating        = 282296;

// Create object
$showerdoor = new Showerdoor(
     array(
        'square_meters'     => $square_meters,
        'glasstype'         => $glasstype,
        'hinges'            => $hinges,
        'coating'       => $coating,
    )
);
// Echo price
echo $showerdoor->getPrice();

General concept

From your explanation it seems to me that, apart from dimensions and prices, different categories of products may have different additional features (like glasstype, hinges, and coating in your showerdoor example).

But since you showed only one product, I can't be certain. So What follows is based on the hypothesis that:

there is potentially a number of these features and each category of product uses only some of them, but each feature may be shared by several categories.
for a given feature, the way it is processed is the same for all involved categories of products.

That's said, I think you'd better to write all these methods in your abstract class, so they're always already available for each new derived class you create for a kind of product.

More, I think that even if certain features are specific to certain unique categories (what you called "exceptions"), they also should be written in the abstract class.

At the same time, in order to keep the whole process secure (i.e. be sure that a derived class can't accept features that don't regard their category), each derived class could have its own allowed-features list.

Applying the above strategy leads to something like this for the abstract class:

abstract class ConfigurableProduct {

  public $squareMeters;
  // Stores the prices
  protected $_price = array();

  // Initializes object
  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['square_meters'];

    // Set features when they are both allowed and present
    foreach ($allowed_features as $feature) {
      if (isset($data[$feature])) {
        $method = 'set' . $feature;
        $this->$method($data($feature));
      }
    }
  }

  // Adds price to _price array
  public function addPrice( $value ) {
    $this->_price[] = $value;
  }

  // Sums prices in _price array
  public function getPrice() {
    return array_sum( $this->_price );
  }

  // One example of feature method for "someFeature"
  public function setSomeFeature($id) {
    // Check for part id (so integer)
    if (isset($id) && is_int($id)) {
      // Calculate price
      $value = gtp_wizard_part_price($id) * $this->squareMeters;
      // Add price to price array
      $this->addPrice( $value );
    }
  }

  // Other feature methods here...
}

And something like this for a category of product:

class Showerdoor extends ConfigurableProduct {

  protected $allowed_features = [
    'glasstype',
    'coating',
    'hinges',
  ];

  // Initializes object
  public function __construct($data) {
    // Parent constructor
    parent::__construct($data, $this->allowed_features);
  }
}

At my eyes this way ensures both consistency and simplicity (hence readability): all existing feature methods are gathered at a unique place, once for all; granting a feature to a category is only matter of simple registration.

Some minor aspects

I'm a bit surprised you calculate square meters outside of the object: unless I'm missing something subtle, it could be done (again, once for all) in the abstract class, so the __construct() function becomes:

  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['width'] * $data['height'] / 1000000;
    // ...

And the object creation is only:

// Parts (ids from database)
$glasstype      = 282236;
$hinges         = 282242;
$coating        = 282296;

// Create object
$showerdoor = new Showerdoor(
   array(
    'width'     => 800,
    'height'    => 2000,
    'glasstype' => $glasstype,
    'hinges'    => $hinges,
    'coating'   => $coating,
  )
);

Also I'd suggest adding the availibility for the app to access data in the object, avoiding to keep global variables outside of it.
This way, the abstract class looks like this:

abstract class ConfigurableProduct {

  public $squareMeters;
  public $data;
  // Stores the prices
  protected $_price = array();

  // Initializes object
  public function __construct($data = [], $allowed_features = []) {
    // Set square meters
    $this->squareMeters = $data['width'] * $data['height'] / 1000000;
    // Keep a copy of data
    $this->data = $data;
    //...

And the object creation is even simpler:

// Create object
$showerdoor = new Showerdoor(
   array(
    'width'     => 800,
    'height'    => 2000,
    'glasstype' => 282236,
    'hinges'    => 282236,
    'coating'   => 282296,
  )
);

Regarding the feature methods you gave in example, note that setGlasstype() and setCoating() are perfectly identical and hence can be factorized.
Obviously it can be achieve by simply invoking a common (protected) method from each of them. But if this situation is frequent you might use another strategy, using aliases, like this:

  // Aliases for feature methods
  protected $feature_aliases = [
    'glasstype'   => 'GlassType_setCoating',
    'setcoating'  => 'GlassType_setCoating',
    'sethinges'   => 'SetHinges',
  ];

So the __construct() function must be adapted:

// Set features when they are both allowed and present
foreach ($allowed_features as $feature) {
  if (isset($data[$feature])) {
    $method = 'set' . $this->feature_aliases[$feature];
    $this->$method($data($feature));
  }
}

This way, again, we get more consistency and readability, since there is a clear and brief registry for all existing feature methods.
Note that this also simply solves an issue I didn't pay attention in the above: the difference between all-lower and camelcase feature names.

Hope this helps!


Using Curl/Post to execute a HTTP request I have a loop in a php file to HTTP_Request Server Via CURL->POST.

But I don't know if I'm missing any security considerations. Also, is there a better way to perform this http request?

<?php
$j = 0;
while ($j <= 1) {
    $url    = 'http://127.0.0.1/index.php';
    $fields = array(
        'input1' => 'variable1',
        'input2'    => 'variable2',
    );
    $postvars         = http_build_query($fields);
    $COOKIE_FILE_PATH = "/tmp/cookiescron.txt";
    $ch = curl_init();
    curl_setopt($ch, CURLOPT_URL, $url);
    curl_setopt($ch, CURLOPT_POST, count($fields));
    curl_setopt($ch, CURLOPT_POSTFIELDS, $postvars);
    curl_setopt($ch, CURLOPT_TIMEOUT, 2);
    curl_setopt($ch, CURLOPT_RETURNTRANSFER, false);
    curl_setopt($ch, CURLOPT_FORBID_REUSE, true);
    curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 2);
    curl_setopt($ch, CURLOPT_DNS_CACHE_TIMEOUT, 10);
    curl_setopt($ch, CURLOPT_FRESH_CONNECT, true);
    curl_setopt($ch, CURLOPT_HTTPHEADER, array('Connection: close'));
    // execute post
    curl_exec($ch);
    // close connection
    curl_close($ch);
    $j++;
    sleep(25);
}
?>

This executes 2 requests in around 25~30 seconds.

Well, I didn't take the time to test my snippets, but here are two suggestions...

#1 (preferred) - Bundle all of your input data into a single array, then pass it to your receiving url with a single curl call.  It is best practice to minimize total calls (avoid iterated calls) so long as the operation works for your project, of course.  This avoids sleepy time too.

$ch = curl_init();
$fields = [
    ['input1' => 'variable1', 'input2' => 'variable2'],
    ['input1' => 'variable3', 'input2' => 'variable4']
];
$COOKIE_FILE_PATH = "/tmp/cookiescron.txt";  // this doesn't appear to be used
$options = [
    CURLOPT_URL               => 'http://127.0.0.1/index.php',
    CURLOPT_POST              => true,  // https://www.php.net/manual/en/function.curl-setopt.php says boolean is expected
    CURLOPT_POSTFIELDS        => http_build_query($fields),
    CURLOPT_TIMEOUT           => 2,
    CURLOPT_RETURNTRANSFER    => false,
    CURLOPT_FORBID_REUSE      => true,
    CURLOPT_CONNECTTIMEOUT    => 2,
    CURLOPT_DNS_CACHE_TIMEOUT => 10,
    CURLOPT_FRESH_CONNECT     => true,
    CURLOPT_HTTPHEADER        => ['Connection: close']
];
curl_setopt_array($ch, $options);
curl_exec($ch);
curl_close($ch);

#2 - If your requirements obligate the use of iterated curl calls, I'd recommend only updating the CURLOPT_POSTFIELDS value within the loop.  (Again, not tested)

$ch = curl_init();
$fields = [
    ['input1' => 'variable1', 'input2' => 'variable2'],
    ['input1' => 'variable3', 'input2' => 'variable4']
];
$COOKIE_FILE_PATH = "/tmp/cookiescron.txt";  // this doesn't appear to be used
$options = [
    CURLOPT_URL               => 'http://127.0.0.1/index.php',
    CURLOPT_POST              => true,  // https://www.php.net/manual/en/function.curl-setopt.php says boolean is expected
    CURLOPT_TIMEOUT           => 2,
    CURLOPT_RETURNTRANSFER    => false,
    CURLOPT_CONNECTTIMEOUT    => 2,
    CURLOPT_DNS_CACHE_TIMEOUT => 10,
];
foreach ($fields as $data) {
    $options[CURLOPT_POSTFIELDS => http_build_query($data)]; // overwrites previous data
    curl_setopt_array($ch, $options);
    curl_exec($ch);
    sleep(25);
}
curl_close($ch);

The takeaway here is to always try to leave as many processes as possible OUTSIDE of the loop to reduce calls/workload.

Relevant documentation:

https://www.php.net/manual/en/function.curl-setopt.php
https://www.php.net/manual/en/function.curl-setopt-array.php


Marking a rectangular region in a NumPy array as an image mask I'm currently working on creating a mask for an image. I have initialized a two-dimensional numpy zeros array. I now want to replace the values of the mask corresponding to pixels following some conditions such as x1< x < x2 and y1 < y < y2 (where x and y are the coordinates of the pixels) to 1.

Is there an easier way to do it (maybe through slicing) without looping through the mask like below

clusterMask = np.zeros((h, w))
for x in range(h):
    for y in range(w):
        if x <= clusterH + 2 * S and x >= clusterH - 2*S and y <= clusterW + 2*S and y >= clusterW - 2*S:
            clusterMask[x][y] = 1

It turns out that Numpy has various nifty ways of indexing. I found that my question can be solved by

clusterMask[clusterH - 2*S : clusterH + 2*S, clusterW - 2*S : clusterW + 2*S] = 1

As given in one of the comments, this link contains all information regarding numpy indexing:
https://docs.scipy.org/doc/numpy/reference/arrays.indexing.html


Capitalize all words in sentence string in Ruby I have created the following code:

def LetterCapitalize(str)
  str = str.split(" ").each {|word| word.capitalize!}
  str = str.join(" ")
  return str
end

This takes in a basic string and capitalizes the first letter of all of the words. It looks kind of awkward to reassign things to str twice. What is wrong with my style here, if anything at all?

Your function name should be in snake_case, not PascalCase. (Thanks @rui.)
It seems like it would make more sense to use map than each here.
Ruby doesn't need explicit return.
You might even want to extend the base string class.

Here's my rendition:

class String
  def capitalize_words
    this.split.map { |x| x.capitalize }.join(" ")
    # or:     .map(&:capitalize)
  end
end

"this IS a Test".capitalize_words # => "This Is A Test"


Computing integer square roots in Java (See the next iteration.)
There was a question in Quora about an interview question that asks to write a function for computing integer square roots, so I rolled two of them for fun:
Main.java:
import java.util.Random;
import java.util.function.Function;

public class Main {

    public static long intSqrt1(long number) {
        long sqrt = 0L;

        while ((sqrt + 1) * (sqrt + 1) <= number) {
            sqrt++;
        }

        return sqrt;
    }

    public static long intSqrt2(long number) {
        if (number <= 0L) {
            return 0L;
        }

        long sqrt = 1L;

        while (4 * sqrt * sqrt <= number) {
            sqrt *= 2;
        }

        while ((sqrt + 1) * (sqrt + 1) <= number) {
            sqrt++;
        }

        return sqrt;
    }

    public static long intSqrt3(long number) {
        return (long) Math.sqrt(number);
    }

    private static void profile(Function<Long, Long> function, Long number) {
        long result = 0L;
        long startTime = System.nanoTime();

        for (int i = 0; i < ITERATIONS; ++i) {
            result = function.apply(number);
        }

        long endTime = System.nanoTime();

        System.out.printf("Time: %.2f, result: %d.\n",
                          (endTime - startTime) / 1e6,
                          result);
    }

    private static final int ITERATIONS = 1000;
    private static final long UPPER_BOUND = 1_000_000_000_000L;

    public static void main(String[] args) {
        long seed = System.nanoTime();
        Random random = new Random(seed);
        long number = Math.abs(random.nextLong()) % UPPER_BOUND;

        System.out.println("Seed = " + seed);
        System.out.println("Number: " + number);

        profile(Main::intSqrt1, number);
        profile(Main::intSqrt2, number);
        profile(Main::intSqrt3, number);
    }
}

The performance figures are as follows:

Seed = 137967850680858
Number: 18973198056
Time: 278.29, result: 137743.
Time: 15.65, result: 137743.
Time: 0.38, result: 137743.

(The first time of intSqrt1, the second time of intSqrt2, and so on.)
My main question here is: is it possible to make the two methods any faster while not actually using the Math.sqrt?

I think binary search will be super fast too (without divisions):

public static long intSqrtBinarySearch(long number) {
    long l = 0L, r = Integer.MAX_VALUE;
    while (l < r) {
        long mid = (l + r + 1L) / 2L; // div 2 - bitwise operation
        if (mid * mid <= number) {
            l = mid;
        } else {
            r = mid - 1L;
        }
    }
    return l;
}

Also, your implementation of intSqrt2() may produce overflow error.
Test:

Failed number : 383100999
Bad answer    : 383100999
Correct       : 19572


CodeFights: Game 2048 Description

  You are most likely familiar with the game 2048.

  2048 is played on a gray 4 × 4 grid with numbered tiles that slide smoothly when a player moves them using one of the four arrow keys -  Up ,  Down ,  Left  or  Right . On every turn, a new tile with a value of either 2 or 4 randomly appears on an empty spot of the board. After one of the keys is pressed, the tiles slide as far as possible in the chosen direction until they are stopped either by another tile or by the edge of the grid. If two tiles with the same number collide while moving, they merge into a tile with this number doubled. You can't merge an already merged tile in the same turn. If there are more than 2 tiles in the same row (column) that can merge, the farthest (closest to an edge) pair will be merged first (see the second example).

  In this problem you are not going to solve the 2048 puzzle, but you are going to find the final state of the board from the given one after a defined set of n arrow key presses, assuming that no new random tiles will appear on the empty spots.

  The following example shows the next state of the board after pressing Right.

  You are given a matrix 4 × 4 which corresponds to the 2048 game grid. grid[0][0] corresponds to the upper left tile of the grid. Each element of the grid is equal to some power of 2 if there is a tile with that value in the corresponding position, and 0 if it corresponds to the empty spot.
  You are also given a sequence of key presses as a string path. Each character of the string equals L, R, U, or D corresponding to  Left ,  Right ,  Up  or  Down  respectively.

  Please note that in some cases after pressing an arrow key nothing will be changed.

Example

For

grid = [[0, 0, 0, 0],
        [0, 0, 2, 2],
        [0, 0, 2, 4],
        [2, 2, 4, 8]]

and path = "RR"

the output should be

game2048(grid, path) = [[0, 0, 0, 0],
                        [0, 0, 0, 4],
                        [0, 0, 2, 4],
                        [0, 0, 8, 8]]

Code

def merge(row, left_or_down):
    i, new_row = 0, []
    transposed = [e for e in row if e]

    if not left_or_down:
        transposed = transposed[::-1]

    while i < len(transposed):
        if i == len(transposed) - 1:
            new_row += [transposed[i]]
            break
        if transposed[i] == transposed[i + 1]:
            new_row += [2 * transposed[i]]
            i += 2
        else:
            new_row += [transposed[i]]
            i += 1

    if not left_or_down:
        new_row = [0]*(4 - len(new_row)) + new_row[::-1]

    return  new_row + [0]*(4 - len(new_row))

def game2048(grid, path):
    for direc in path:

        if direc in 'LR':
            grid = [merge(row, direc == 'L') for row in grid]

        if direc in 'UD':
            grid = [merge(row[::-1], direc == 'D') for row in zip(*grid)]
            grid = [row for row in zip(*grid)][::-1]

    return grid

The game2048 function is short, which is good. It could be more clear to the casual reader.
The 2nd grid assignment uses zip(*grid), which is fine, but it would benefit by being broken out as a named helper function, which the third grid assignment could also use. Helper names often do a better documentation job than comments do.

The merge function uses the identifier transposed, where perhaps row would have sufficed, without confusingly calling to mind the 90-degree aspect of matrix transpose.

The magic number 4 is distracting - please assign a board size constant and use that.

The code lacks comments, which is not necessarily a problem, especially since it tries hard to use meaningful identifiers. Its clarity would benefit from unit tests (or docstring tests) showing before / after effects on a grid or row. The "output should be" narrative is helpful; I'm suggesting writing it as a TestCase. In the course of doing that, you may find it useful for coverage or didactic purposes to break out the occasional helper function.


A static Java method for multi-indexing a collection I have this small utility for selecting from a collection via multiple indices at one method invocation:

MultiIndexUtils.java

package net.coderodde.util;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;

public final class MultiIndexUtils {

    private MultiIndexUtils() {}

    public static <T> List<T> multiIndex(Collection<T> collection,
                                         Collection<Integer> indices) {
        List<T> indexedElements = new ArrayList<>(indices.size());
        T[] array = (T[]) collection.toArray();
        indices.forEach((Integer index) -> {
            indexedElements.add(array[index]);
        });
        return indexedElements;
    }

    public static <T> List<T> multiIndex(Collection<T> collection,
                                         Integer... indices) {
        return multiIndex(collection, Arrays.asList(indices));
    }

    public static void main(String[] args) {
        List<String> strings = Arrays.asList("A", "B", "C", "D", "E", "F");
        System.out.println(multiIndex(strings, 1, 0, 3, 2, 0, 5, 4));
    }
}

Critique request

I would like to hear anything that comes to mind.

First up, nice touch with BADCAFE :)

Then, for my two cents:

To me it feels a bit strange using Collection for the element supplier type. Since we are indexing values, it would seem strange to use it on a Set for example, as it does not impose order.
On a similar note, using a Collection for the indices parameter seems strange as well, for the same reasons. A Set also implies uniqueness, so no two equal indices would be passed.

Both would seem to make more sense being Lists to me. But this is something I am just saying based on what I am feeling :)

Continuing:

You can relax the type bound on the input parameter of the elements supplier, like so:

public static <T> List<T> multiIndex(final Collection<? extends T> collection, ...

Now you can pass a collection parameterized with T or with a subtype of T, which makes it more versatile.
Seeing as you are extracting elements, you can annotate the array cast with a SuppressWarnings, as it is safe:

@SuppressWarnings("unchecked")
final T[] array = (T[]) collection.toArray();

Finally, I'd generally think twice before using forEach on a Stream, as it implies using stateful operations. Personally, I think in this case it can be more cleanely written as:

@SuppressWarnings("unchecked")
T[] array = (T[]) collection.toArray();
return indices.stream()
        .map(index -> array[index])
        .collect(Collectors.toList());


Activating image and video carousels I am using this jQuery code to get two carousels working in bootstrap, and it's working for me.

$('#image-gallery-carousel').carousel({
  interval: 5000
});

$('#carousel-caption-text').html($('#slide-content-0').html());

// When the carousel slides, auto update the text
$('#image-gallery-carousel').on('slid.bs.carousel', function (e) {
  var id = $(this).find('.item.active').data('slide-number');
  //alert(id);
  $('#carousel-caption-text').html($('#slide-content-'+id).html());
});

$('#video-gallery-carousel').carousel({
  interval: 12000
});

$('#carousel-caption-video-text').html($('#slide-content-0').html());

// When the carousel slides, auto update the text
$('#video-gallery-carousel').on('slid.bs.carousel', function (e) {
  var id = $(this).find('.item.active').data('slide-number');
  //alert(id);
  $('#carousel-caption-video-text').html($('#slide-content-'+id).html());
});

If there is any better way to implement this, please point it out to me.

From a once over:

Magic constants should go on top ( 5000 and 12000
You use this construct .html($('#slide-content-'+id).html()); 4 times, a helper functions might be in order.
You don't need e in the event handling function, so you can just drop that
Image and video carousel use pretty much the same code, that could be a helper function as well.

So something like this potentially:

var imageCarouselSpeed = 5000,
    videoCarouselSpeed = 12000;

startCarousel( '#image-gallery-carousel', '#carousel-caption-text', imageCarouselSpeed );
startCarousel( '#video-gallery-carousel', '#carousel-caption-video-text', videoCarouselSpeed );

function startCarousel( carouselSelector, textSelector, speed ){
  //Start bootstrap carousel
  $( carouselSelector ).carousel({
    interval: speed
  });
  //Set the first caption
  $(textSelector).html($('#slide-content-0').html());
  // When the carousel slides, auto update the caption
  $(carouselSelector).on('slid.bs.carousel', function () {
    var id = $(this).find('.item.active').data('slide-number');
    $(textSelector).html($('#slide-content-'+id).html());
  });
}

I would agree with Larz a jsfiddle or jsbin would have gotten you a code review much sooner.


QPlainTextEdit subclass function to indent lines in selection I am, primarily for learning purposes, working on creating an editor component in C++ with Qt. As a first step, to begin getting familiar with editing a document via QTextCursor, I wrote this function on my subclass of QPlainTextEdit to indent multiple selected lines:

void MyEditor::increaseSelectionIndent()
{
    QTextCursor curs = textCursor();

    // Do nothing if we don't have a selection.

    if(!curs.hasSelection())
        return;

    // Get the first and count of lines to indent.

    int spos = curs.anchor(), epos = curs.position();

    if(spos > epos)
    {
        int hold = spos;
        spos = epos;
        epos = hold;
    }

    curs.setPosition(spos, QTextCursor::MoveAnchor);
    int sblock = curs.block().blockNumber();

    curs.setPosition(epos, QTextCursor::MoveAnchor);
    int eblock = curs.block().blockNumber();

    // Do the indent.

    curs.setPosition(spos, QTextCursor::MoveAnchor);

    curs.beginEditBlock();

    for(int i = 0; i <= (eblock - sblock); ++i)
    {
        curs.movePosition(QTextCursor::StartOfBlock, QTextCursor::MoveAnchor);

        curs.insertText("\t");

        curs.movePosition(QTextCursor::NextBlock, QTextCursor::MoveAnchor);
    }

    curs.endEditBlock();

    // Set our cursor's selection to span all of the involved lines.

    curs.setPosition(spos, QTextCursor::MoveAnchor);
    curs.movePosition(QTextCursor::StartOfBlock, QTextCursor::MoveAnchor);

    while(curs.block().blockNumber() < eblock)
    {
        curs.movePosition(QTextCursor::NextBlock, QTextCursor::KeepAnchor);
    }

    curs.movePosition(QTextCursor::EndOfBlock, QTextCursor::KeepAnchor);

    // Done!

    setTextCursor(curs);
}

I'm interested in any feedback on whether or not I'm following "best practices" with QTextCursor - am I doing anything weird, or that can be simplified or improved?

Some of these comments can be removed:

Too obvious:

// Do nothing if we don't have a selection.

Noisy:

// Done!

These initialized variables:

int spos = curs.anchor(), epos = curs.position();

should be on separate lines for better maintenance:

int spos = curs.anchor();
int epos = curs.position();

No need to do a manual swap:

if(spos > epos)
{
    int hold = spos;
    spos = epos;
    epos = hold;
}

Just use std::swap to keep it idiomatic:

if (spos > epos)
{
    std::swap(spos, epos);
}

No need calculate the difference each time:

for(int i = 0; i <= (eblock - sblock); ++i)

Define a constant before the loop and use it instead:

const int blockDifference = eblock - sblock;

for (int i = 0; i <= blockDifference; ++i)


Adding a new record automatically I created this method that adds a new situation to the situation list. The situation has a many-to-one relation with project.task. This method seems a little longer than it should be and I don't know how to optimize it:

@api.multi
def create_situation(self):
    active_id = self.env.context.get('active_ids', []) or []
    proj_obj = self.env['project.task'].browse(active_id)
    situation_orders = self.env['project.task.situation'].browse([])
    task_lines = self.env['task.situation.line'].browse([])
    situation_ids = self.env['project.task.situation'].search([('task_id', '=', proj_obj.id)], order='id desc', limit=1)
    size_list_situation_ids = len(situation_ids)
    last_id = situation_ids and max(situation_ids)
    s_sequence = "Situation N° " + str(proj_obj.sequence)

    for record in proj_obj:
        task_lines = [(0, 0, {'ref': line.ref,
                                      'product_id': line.product_id.id,
                                      'quantity': line.quantity,
                                      'um_id': line.um_id,
                                      'price_unit': line.price_unit,
                                      'price_subtotal': line.price_subtotal,
                                      'progress': line.progress,
                                      'cumulative_amount_ht': line.cumulative_amount_ht,
                                      'previous_amount_ht':line1.cumulative_amount_ht if size_list_situation_ids != 0 else 0,
                                      'amount_net_ht': line.cumulative_amount_ht if size_list_situation_ids == 0 else (line.cumulative_amount_ht - line1.cumulative_amount_ht)}) for line, line1 in izip_longest(record.task_line_ids, last_id.task_line)]
    situation_order = {'partner_id': proj_obj.project_id.partner_id.id,
                        'task_id':proj_obj.id,
                        'task_line':task_lines,
                        'user_id': proj_obj.user_id.id,
                        'name':s_sequence,
                        'tyoe': 'situation' if self._context.get('type', 'situation') else 'final_count',
                        'project_id':proj_obj.project_id.id,
                        'due_date': self.date_id.id}
    situation_orders+=situation_orders.with_context(self._context).create(situation_order)
    proj_obj.write({'sequence':proj_obj.sequence+1})

First off, make it more maintainable: break out that long list comprehension into a for loop so it's clearer where line is coming from.

As for optimization, since size_list_situation_ids doesn't change through the loop, you can refactor those trinary ops into functions and then set them just once at the beginning:

if size_list_situation_ids:
    prev_amount_ht = lambda l: l.cumulative_amount_ht
    amount_net_ht = lambda l, l1: l.cumulative_amount_ht - l1.cumulative_amount_ht
else:
    prev_amount_ht = lambda l: 0
    amount_net_ht = lambda l, l1: l.cumulative_amount_ht

and then do:

...
'previous_amount_ht': prev_amount_ht(line1),
'amount_net_ht': amount_net_ht(line, line1),
...

which saves a couple of comparisons every time through the loop.

Much more than that is difficult to suss out, since the code looks non-functional overall: where does situation_orders come from? Why is the initial task_lines even being loaded, as it doesn't seem to be used?


Simple day-of-week program I was doing the following exercise:

Create an algorithm which asks the user a number from 1 to 7, and each number represents a day on the week. For example, 1 represents Sunday, 2 represents Monday and so on. The user will type this number and the program will show them the name of the day on the week and if it's a business day or a weekend.

I tried to do it in a object-oriented way and using Python. Can someone please verify if my code is within the Python and object-oriented "standards"? Because I'm not sure whether it is, and I need a review.
Here's my code:
class DayOnWeek:
    ERROR_MESSAGE = 'Invalid weekday. Try again:'
    NUMBER_NAME_ASSOCIATIONS = {
        1: 'Sunday',
        2: 'Monday',
        3: 'Tuesday',
        4: 'Wednesday',
        5: 'Thursday',
        6: 'Friday',
        7: 'Saturday'
    }

    def __init__(self):
        self._day_number = 0
        self._day_name = None
        self._category = None

    @property
    def day_name(self):
        return self._day_name

    @property
    def category(self):
        return self._category

    @property
    def day_number(self):
        return self._day_number

    @day_number.setter
    def day_number(self, value):
        if 1 <= value <= 7:
            self._day_number = value
            self._day_name = DayOnWeek.NUMBER_NAME_ASSOCIATIONS[self._day_number]
        else:
            raise ValueError(DayOnWeek.ERROR_MESSAGE)

        if 2 <= value <= 6:
            self._category = 'Business day'
        else:
            self._category = 'Weekend'

class DayOnWeekInterface:

    def __init__(self):
        self._day_on_week = DayOnWeek()

    def request_day_number(self):
        while True:
            try:
                day_num = input('Type the number of the day on the week (ex: 1 - Sunday):')
                day_num = int(day_num)
                self._day_on_week.day_number = day_num
                break
            except ValueError:
                print(DayOnWeek.ERROR_MESSAGE)
        self.show_day_on_week()

    def show_day_on_week(self):
        day_num = self._day_on_week.day_number
        day_name = self._day_on_week.day_name
        day_category = self._day_on_week.category
        print(f'The day {day_num} is {day_name.lower()}')
        print(f'{day_name} is a {day_category.lower()}')

DayOnWeekInterface().request_day_number()

Your DayOnWeek object is mutable, allowing you to set the .day_number property.  For such a simple object is not a good idea.  An immutable object is better.
There are exactly 7 days of the week.  For a one-of-seven choice, the go-to data-storage structure is an Enum.  For example:
from enum import Enum

class DayOfWeek(Enum):
    Sunday = 1
    Monday = 2
    Tuesday = 3
    Wednesday = 4
    Thursday = 5
    Friday = 6
    Saturday = 7

    @property
    def weekend(self):
        return self in {DayOfWeek.Saturday, DayOfWeek.Sunday}

    @property
    def category(self):
        return 'Weekend' if self.weekend else 'Business day'

    @property
    def number(self):
        return self.value

Borrowing Reideerien's request_day_number() function you'd use the DayOfWeek enum like:
def main():
    day_no = request_day_number()
    day = DayOfWeek(day_no)

    print(f'Day {day.number} is {day.name}')
    print(f'{day.name} is a {day.category} day')

if __name__ == '__main__':
    main()


ACM ICPC Team: Hackerank This is a hackerank easy question.

  You are given a list of N people who are attending ACM-ICPC World
  Finals. Each of them are either well versed in a topic or they are
  not. Find out the maximum number of topics a 2-person team can know.
  And also find out how many teams can know that maximum number of
  topics.

  Note Suppose a, b, and c are three different people,
  then (a,b) and (b,c) are counted as two different
  teams.

  Input Format

  The first line contains two integers, N and M, separated by a single
  space, where N represents the number of people, and M represents the
  number of topics. N lines follow. Each line contains a binary string
  of length M If the i'th line's j'th character is 1, then the i'th
  person knows the j'th topic; otherwise, he doesn't know the topic.

  Constraints

  2≤N≤500
   1≤M≤500

  Output Format

  On the first line, print the maximum number of topics a 2-person team
  can know. On the second line, print the number of 2-person teams that
  can know the maximum number of topics.

Here is what I wrote:

Scanner console = new Scanner(System.in);
int n = console.nextInt();
int m = console.nextInt();
console.nextLine();
String[] arr = new String[n];
for(int p=0; p<n; p++){
    String s = console.nextLine();
    arr[p] = s;
}

Map<Integer, List<List<Integer>>> counts = new HashMap<>();

for(int i = 0; i< n-1; i++){

    String str=arr[i];

    for(int j=i+1; j<n; j++){

        String secondStr = arr[j];

        int count = 0;
        for(int k=0; k<m; k++){

            if(str.charAt(k) == '1' || secondStr.charAt(k) == '1'){
                count ++;
            }

        }

        List<Integer> mapping = new ArrayList<>();
        mapping.add(i+1);
        mapping.add(j+1);

        if(counts.containsKey(count)){
            List<List<Integer>> existingMappings = counts.get(count);
            existingMappings.add(mapping);
            counts.put(count, existingMappings);
        }else{
            List<List<Integer>> newMappings = new ArrayList<>();
            newMappings.add(mapping);
            counts.put(count, newMappings);
        }

    }
}

int max = counts.keySet().stream().mapToInt(Integer::valueOf).max().orElse(0);

int occurences = counts.get(max).size();

System.out.println(max);
System.out.println(occurences);

This worked and passed the challenge. But it has a code smell: Too many loops and even inner loops. Any improvements?

This part can be optimized a bit, in terms of length.

List<Integer> mapping = new ArrayList<>();
mapping.add(i+1);
mapping.add(j+1);

if(counts.containsKey(count)){
    List<List<Integer>> existingMappings = counts.get(count);
    existingMappings.add(mapping);
    counts.put(count, existingMappings);
}else{
    List<List<Integer>> newMappings = new ArrayList<>();
    newMappings.add(mapping);
    counts.put(count, newMappings);
}

Java 8 maps have computeIfAbsent. You can use this to "getOrCreate" the existing mapping like so:

List<Integer> mapping = new ArrayList<>();
mapping.add(i+1);
mapping.add(j+1);

counts.computeIfAbsent(count, k -> new ArrayList<>()).add(mapping);

Additionally, you do this:

    List<List<Integer>> existingMappings = counts.get(count);
    existingMappings.add(mapping);
    counts.put(count, existingMappings);

There is no need to put something back into the map when you have retrieved it, because you are retrieving a pointer to a list, and you're using it to add to a list. See this Ideone that lists you put and later retrieve from the map are the same lists.

Okay, so back to this snippet.

List<Integer> mapping = new ArrayList<>();
mapping.add(i+1);
mapping.add(j+1);

counts.computeIfAbsent(count, k -> new ArrayList<>()).add(mapping);

The new ArrayList constructor can take an initialSize argument. Since you're only going to be adding two integers to the list, you could provide this information and save memory space.

List<Integer> mapping = new ArrayList<>(2);

Your algorithm could be more optimized - there's no need to count topic knowledge for a team if they know 7 topics together and there's only 6 topics left to check and the top team currently knows 14 topics. Similarily, there's no reason to store how much topics each team knows, only the teams that know the largest amount of topics, and the largest amount of topics you have currently found.

However, I think that when we look at the loops, you can't really get rid of them.

You see, there's two loops needed to make the teams (iterating people over people), and one loop needed to check the topics (iterating over topics). There are clever tricks you could use where you take the binary string and convert it to an integer, but when you do that, you're looping over the characters in the binary string - it has to read the string, after all. You just don't use a for loop.

So we're doomed to have at least 3 loops. What you CAN do is take the counting loop and put it in a separate function. That would "get rid" of one of the loops. Your static analyzer is not built with the idea of such challenges in mind - to see a method with 3 nested loops usually means the method is doing too much work.


Collatz Sequence in Python I'm trying to use Python to solve the Project Euler problem regarding the Collatz sequence. With some research online, I have come up with the code below. However, it still takes a long time to find the maximum length of the Collatz sequence of the numbers from one to a million after the following improvements.

Using a list in order to keep track of numbers that are in the Collatz sequence of preceding numbers so that they won't be tested
Not testing any of the numbers in the lower half of interval and not testing any numbers that would make this expression true:

((f%9==2)or(f%9==4)or(f%9==5)or(f%9==8)or(f%8==5))

Any criticism is appreciated.

"""
The following iterative sequence is defined for the set of positive integers:

n → n/2 (n is even)
n → 3n + 1 (n is odd)

Using the rule above and starting with 13, we generate the following sequence:

13 → 40 → 20 → 10 → 5 → 16 → 8 → 4 → 2 → 1
It can be seen that this sequence (starting at 13 and finishing at 1) contains 10 terms. Although it has not been proved yet (Collatz Problem), it is thought that all starting numbers finish at 1.

Which starting number, under one million, produces the longest chain?

NOTE: Once the chain starts the terms are allowed to go above one million.
"""
a=[]
def collatz(n):
    if n%2==0:
        return int(n/2)
    else:
        return int(3*n+1)
def collatz_length(n):
    b=n
    c=0
    while collatz(b)!=1:
        a.append(b)
        b=collatz(b)
        c=c+1
        if collatz(b)==1:
            break
    return c
d=0
e=0
for g in range(500000,1000000,1):
    f=g+2
    if (f in a) == False and ((f%9==2)or(f%9==4)or(f%9==5)or(f%9==8)or(f%8==5)):
        print(len(a))
        print(f)
        if collatz_length(f)>e:
            d=f
            e=collatz_length(f)
print(d)
print(e)

Variable Names

You use a, b, c, d, e, f, and g to refer to important things. I started trying to keep track of what each one meant and then gave up. I have no idea. Give things meaningful names. Apparently d is the number with the longest sequence, whose length is e - which is certainly not obvious from the names!

Chaining

To find the length of the collatz chain for some number, n, involves also finding the length of the collatz chain for every other number in that chain. However, you don't store that information anywhere, so it's lost.

Furthermore, you calculate it TWICE:

if collatz_length(f)>e:
    d=f
    e=collatz_length(f)

So you're recalculating every chain every time twice. That's an enormous amount of extra work. Lastly, you're assuming that the largest starts halfway through. That isn't necessarily true, given that collatz(n) > n for odd n...

Saving State

You save some state in a, but that ends up with you having to do a linear search through a list... which will fail most of the time, but it'll take a long time to fail. At the very least, you could store things in a set(), which is O(1), but even then, you're storing the wrong state. You're storing numbers you've seen before, but what you should store is the chain lengths of the numbers you've seen before. That is...

Memoize

This is a problem that lends itself extremely naturally to memoization, so we can just do that:

@memoize
def collatz_length(n):
    if n == 1:
        return 1
    else:
        return 1 + collatz_length(collatz(n))

And then just stick that into max:

longest_num = max(range(1, 1000001), key=collatz_length)
print(longest_num)
print(collatz_length(longest_num))


Data loading Hash Map I'm trying to improve my code's performance, and I've found an area where the execution time is being significantly increased. I've had a look through the code but I can't see how I can improve it.

AddData gets called over 200,000 times.

public void AddData(String label, int fromid, int toid, double distance, boolean oneway, int speedlimit)
    {
        currentRoad = new Edge(label, fromid, toid, distance, speedlimit, oneway);

        tempVertex = allVertices.GetValue(fromid);
        tempVertex.addEdge(currentRoad);
        allEdges.AddItem(currentRoad);
        if (!oneway)
        {
            currentRoad = new Edge(label, toid, fromid, distance, speedlimit, oneway);
            tempVertex = allVertices.GetValue(toid);
            tempVertex.addEdge(currentRoad);
        }
    }

The method within the AddData function that takes a large proportion of the time is allEdges.AddItem().

allEdges is a Hash Map containing vectors of Edges. (Edges is a class).

Here is my implementation of the Hash Map AddItem function:

public void AddItem(Edge value)
  {
       if ((noofitems/data.length) > 0.7)
       {
           long time = System.nanoTime();
           EdgeHashPair[] newMap = new EdgeHashPair[data.length * multiplier];
           multiplier = 8;
            for (EdgeHashPair oldMap1 : data)
            {
                if (oldMap1 != null)
                {
                    int index = HashFunction(oldMap1.data.GetItem(0).label);
                    int increment = 1;
                    index = index % newMap.length;
                    boolean inserted = false;
                    while (!inserted)
                    {
                        if (newMap[index] == null)
                        {
                            newMap[index] = oldMap1;
                            inserted = true;
                        }
                        else
                        {
                            index = index + (increment<<1);
                            index = index % newMap.length;
                        }
                    }
                }
            }
           data = newMap;
           System.out.println("Hash map resizing took: " + ((System.nanoTime() - time)/1000000) + "ms");
       }

        int index=HashFunction(value.label);
        index = index % data.length;
        int increment = 1;
        boolean inserted = false;
        while (!inserted)
        {
            if (data[index] == null)
            {
                data[index] = new EdgeHashPair();
                data[index].addItem(value);
                noofitems++;
                inserted = true;
            }
            else
            {
                if (data[index].data.GetItem(0).label.compareTo(value.label) == 0)
                {
                    data[index].addItem(value);
                    inserted = true;
                    noitems++;
                }
                else
                {
                    index = index + (increment<<1);
                    index = index % data.length;
                }
            }
        }
  }

Hash function:

      private int HashFunction(String key)
      {
        // Task 1 code: Hash the key and return a long value
        int code = 29;
        for (int i=0; i < key.length(); i++)
        {
            code = code*53+(key.charAt(i));
        }
        return (code < 0 ? -code : code);
  }

From timings I've done, the loading of data currently takes around 50% of my execution time (around 1000ms), which I feel is extremely high and would love to be able to reduce this.

Edit:
The program is a navigation application, it stores edges and vertices and then calculates many routes.

Let's talk about this bit of code.

    int index=HashFunction(value.label);
    index = index % data.length;
    int increment = 1;
    boolean inserted = false;
    while (!inserted)
    {
        if (data[index] == null)
        {
            data[index] = new EdgeHashPair();
            data[index].addItem(value);
            noofitems++;
            inserted = true;
        }
        else
        {
            if (data[index].data.GetItem(0).label.compareTo(value.label) == 0)
            {
                data[index].addItem(value);
                inserted = true;
                noitems++;
            }
            else
            {
                index = index + (increment<<1);
                index = index % data.length;
            }
        }
    }

First, you have an else block in here...

        else
        {
            if (data[index].data.GetItem(0).label.compareTo(value.label) == 0)
            {
                data[index].addItem(value);
                inserted = true;
                noitems++;
            }
            else
            {
                index = index + (increment<<1);
                index = index % data.length;
            }
        }

... containing only an if-elseif chain. Merge these as such:

        else if (data[index].data.GetItem(0).label.compareTo(value.label) == 0)
        {
            data[index].addItem(value);
            inserted = true;
            noitems++;
        }
        else
        {
            index = index + (increment<<1);
            index = index % data.length;
        }

Second, you only use increment in a constant expression (increment<<1). Consider replacing this with the raw constant instead.

Third, you always loop at least once. So replace your while loop with a do-while loop.

Now for some design issues that could lead to reduced performance...

                int index = HashFunction(oldMap1.data.GetItem(0).label);

    int index=HashFunction(value.label);

When you resize the map, you rehash all the entries, even although their values didn't change. Store the calculated hash in the item.

    for (int i=0; i < key.length(); i++)
    {
        code = code*53+(key.charAt(i));
    }

When you hash the key, you do so via for-loop over String.length() and String.charAt. These are two function calls, and String.charAt continuously checks the length. Consider declaring a char array of length key.length(), then setting it with String.getChars. I'm assuming you have keys of a decent length here, and not very short. Do several performance tests for this, at different times and with different sets of data.

Performance-tuning your code is likely to be hard without semantics surrounding your code and timing information. See if there's ways to profile which functions take longest... on a more granular level than just this function.


Select fields from a form by ID or Xpath and fill its value I'm writing a program/tests in C# using Selenium and Chrome web driver. Everything works well, however I would like to simplify the code and make it object-oriented. I thought about extracting some parts of the code to different classes, and perhaps refactor the area of driver.FindElement... by using it only one time with different arguments.

Any suggestions?

private static void FillingForms(IWebDriver driver)
{

    driver.FindElement(By.XPath("//*[@id='payoutMethods']/label[1]")).Click();
    driver.FindElement(By.Id("SignUpButton")).Click();
    driver.Manage().Timeouts().ImplicitlyWait(TimeSpan.FromSeconds(10));
    driver.FindElement(By.Id("accountType_Company")).Click();
    driver.FindElement(By.Id("txtCompanyName")).SendKeys("Albertos Example");

    var mySelectElm = driver.FindElement(By.Id("ddlBusinessOrganization"));
    var mySelect = new SelectElement(mySelectElm);
    mySelect.SelectByText("Trust Company");

    driver.FindElement(By.Id("txtCompanyURL")).SendKeys("www.examplewebsite.com");
    driver.FindElement(By.Id("txtContactPersonFirstName")).SendKeys("11 Name");
    driver.FindElement(By.Id("txtContactPersonLastName")).SendKeys("11 Last Name");

    var email = Guid.NewGuid().ToString("n").Substring(0, 5);
    driver.FindElement(By.Id("txtEmail")).SendKeys(email + "@gmail.com");
    driver.FindElement(By.Id("txtRetypeEmail")).SendKeys(email + "@gmail.com");
    driver.FindElement(By.Id("datepicker")).Click();

    var mySelectElm1 = driver.FindElement(By.XPath("//*[@id='ui-datepicker-div']/div/div/select[2]"));
    var mySelect1 = new SelectElement(mySelectElm1);
    mySelect1.SelectByText("1988");

    driver.FindElement(By.XPath("//*[@id='ui-datepicker-div']/table/tbody/tr[2]/td[3]/a")).Click();
    driver.FindElement(By.Id("PersonalDetailsButton")).Click();

}

You use a lot of absolute xpaths. It's a bad practice, since those are costly to maintain and looks bad. Try to modify them to relative xpath's or preferably css selectors.
Instead of implicit wait, use explicit wait and wait until certain conditions are met. Or if you test ajax based application - try waiting for ajax requests to complete.
Seems that your application might have quite some dropdowns. Create extention
someElement.SelectByText(string text)or static method Select.ByText(someText).FromElement(someElements) and initialize SelectElement there. It mostly good to abstract yourself from any lib.
Check out PageFactory pattern - maybe you will like it. It's another way of finding elements - more readable and convenient.
I don't advocate to use page object pattern since I think it is an anti pattern in any object oriented language, but for starter - try it.
Why is your method static?


Search string by regex in files and write line of found string I am searching in files with extensions .xml, .java, and .properties strings that match a certain regular expression.

When I find it, I write it to the file with the filename, line and string information.

def search(param):
    filename = "search-result.txt"
    try:
        os.remove(filename)
    except OSError:
        pass
    os.path.walk(param, step, None)

def step(ext, dirname, names):
    output = open("search-result.txt", "a")
    for name in names:
        if name.lower().endswith(".xml") or name.lower().endswith(".properties") or name.lower().endswith(".java"):
            path = os.path.join(dirname, name)
            filters = ["\\bin\\", "\\build\\", "logback", "\\test\\", "\\target\\"]
            if all(not filter in path for filter in filters):
                with open(path, "r") as lines:
                    print "Read: {}".format(path)
                    i = 1
                    for line in lines:
                        m = re.search(r"(!|$|RUP)\{[^:]*:[^\}]*\}", line)
                        if m is not None:
                            output.write("Path: {0}; \n    Line number: {1}; \n        {2}\n".format(path, i, line))
                        i += 1
    output.close()

I divided it into two functions - search, where I can check where result file is existed and remove it, and then step for os.path.walk. Param is a folder to search in.

How can this code be adjusted to look better?

These parameter names are confusing: param and ext.

The step() function suffers from excessive nesting.  I would further subdivide your step() function to create a search_path() function that deals with each candidate file.  Since both of these helper functions aren't really that useful on their own, I would define them both within the main search() function.

You've hard-coded "search-result.txt" twice.  Ideally, it should be parameterized rather than hard-coded at all.  Furthermore, you're reopening it for appending each time you enter a directory, which is problematic because…

Reopening the file handle is wasteful.
You might not even have any search results for that directory.
If you opened it just once in 'w' mode instead of many times in 'a' mode, you wouldn't have to remove the file at all. (Note that removing the file does make a difference, if the file exists and has a second hard-link.)

Also, if all(not filter in path for filter in filters) is inefficient.  As explained in the documentation for [os.path.walk()], you can avoid entering directories in which you have no interest:

  The visit function may modify names to influence the set of directories visited below dirname, e.g. to avoid visiting certain parts of the tree. (The object referred to by names must be modified in place, using del or slice assignment.)

To analyze file extensions, use os.path.splitext().

Idiomatic Python loops rarely need statements like i += 1.  What you want to use is enumerate().

The format string for output.write() is easier to read if spread out into several lines.

import os.path
import re

EXCLUDE_DIRS = ['bin', 'build', 'logback', 'test', 'target']

def search(directory, output_filename='search-result.txt'):
    def search_path(output, path):
        """Output search results for one file"""
        with open(path) as lines:
            print "Read: {}".format(path)
            for i, line in enumerate(lines, 1):
                if re.search(r"(!|$|RUP)\{[^:]*:[^\}]*\}", line):
                    output.write("Path: {0}; \n"
                                 "    Line number: {1}; \n"
                                 "        {2}\n".format(path, i, line))

    def step(output, dirname, names):
        """Filesystem traversal excluding certain directory names,
           calling search_path() for candidate files"""
        names[:] = [entry for entry in names if entry not in EXCLUDE_DIRS]
        for name in names:
            _, ext = os.path.splitext(name)
            if ext.lower() in ('.xml', '.properties', '.java'):
                search_path(output, os.path.join(dirname, name))

    with open(output_filename, 'w') as output:
        os.path.walk(directory, step, output)


Dividing Negative integer ranges Context:

My project has a hand-rolled ORM (... yeah ... I know) ...

built around the MS DataSet objects that represent wrapped collections of rows of DBtables (... yeah ... I know) ...

and which use negative ints to represent identity columns on new Rows which haven't yet been synched with the DB (... yeah ... I know) ...

the upshot of which is we objects that generally expect an identity to start at -1 and be decremented until they reach int.MinValue

Furthermore, we sometimes need to tell these objects that they can only use a certain sub-set of the negative integer range, so that multiple threads can create rows, which can then be combined without risk of ID conflicts. (... yeah ... I know) ...

and occasionally this subdivision has to happen repeatedly :(

All of this is to explain why it is that I need a method that is going to take a range of negative numbers, and sub-divide it. But that these ranges want to be thought of as running from, say, -100 to -999 (i.e. they run the 'wrong way').

Alas, all of this set-up is completely non-negotiable. so the plethora of comments along the lines of "the best way to fix this code is to not need it, or to change what it's trying to do" are (whilst completely correct) not at all useful.

We need to split an arbitrary negative range into n sub-ranges, all of which MUST be strictly dis-joint. It's NOT necessary that we use all the integers (though it's preferable not to waste too many).
It's not necessary that we always use the boundary values, though again preferable.

We're guaranteed to be given the range (-1, int.MinValue), so have to support that.

Primary goal of review is primarily to spot bugs and edge cases, and gather views on readibility/understandability of code.
Other aesthetic/design comments welcome too.
Unit Test suite will be written, given the ease of testing.

So, the code:

public static void IdentifyIndexRangeForDataSetUse(
  int xiExistingRangeStart,
  int xiExistingRangeEnd,
  int xiTotalNumberOfDivisionsBeingMade,
  int xiIndexOfTargetDivision,
  out int xoTargetRangeStart,
  out int xoTargetRangeEnd)
{
  if (xiExistingRangeStart < xiExistingRangeEnd)
  { throw new System.InvalidOperationException("The SMALLER value MUST be provided first. Note that since values are negative, this first value will have the larger ABSOLUTE value!"); }

  if (xiExistingRangeStart > 0 || xiExistingRangeEnd > 0)
  { throw new System.InvalidOperationException("The existing range should lie entirely within the Negative Integers."); }

  // REVERSE THE NUMBERS!!
  // We get given a range like (-1 --> -99), but it's WAY easier to think and reason about the range (1 --> 99)
  // so simply reflect the range into the positive domain and work there.
  int lPosExistingRangeStart, lPosExistingRangeEnd;

  lPosExistingRangeStart = -1 * xiExistingRangeStart;
  if (xiExistingRangeEnd == int.MinValue)
  {
    lPosExistingRangeEnd = int.MaxValue;
  }
  else
  {
    lPosExistingRangeEnd = -1*xiExistingRangeEnd;
  }

  var lTotalLengthOfExistingRange = lPosExistingRangeEnd - lPosExistingRangeStart + 1; //check overflows
  var lTotalLengthOfNewRanges = lTotalLengthOfExistingRange / xiTotalNumberOfDivisionsBeingMade;
  int lPosTargetRangeStart = lPosExistingRangeStart + (xiIndexOfTargetDivision * lTotalLengthOfNewRanges);
  int lPosTargetRangeEnd = lPosTargetRangeStart + (lTotalLengthOfNewRanges - 1); //the '-1' prevents overlaps.

  // REVERSE THE NUMBERS!!
  // To take it back into the negative domain.
  xoTargetRangeStart = -1 * lPosTargetRangeStart;
  xoTargetRangeEnd = -1 * lPosTargetRangeEnd;

}

Redundant check

If xiExistingRangeStart is greater than xiExistingRangeEnd, then you only have to verify that xiExistingRangeStart is not greater than zero to know that xiExistingRangeEnd can't be.

Parameters + Error messages

I would prefer your exception messages to be more explicit.   Rather than:

  "The SMALLER value MUST be provided first. Note that since values are negative, this first value will have the larger ABSOLUTE value!"); }

I think it would be better to refer to 'Start' rather than 'first' value.  This may seem pedantic but consistency between the error name and the parameter name make it easier to resolve issues.

Similarly, when I think of negative numbers, I tend to think of numbers < 0, not <= zero which seems to be what your code suggests.  So again, I'd tend to be more explicit.


Scrape points from CTF sites I am relatively new to classes. This one uses one.
I am pretty sure this is not the correct way to do this. But I also do not know the correct way.
You can use the certified_secure function when you make an instance of crawler that has the URL of hackthissite.org.
Do I have to many comments? Are they too verbose?
#! usr/bin/env python

import bs4
import requests

users = ['user1', 'user2']
certified_secure_url = 'https://www.certifiedsecure.com/profile?alias='
hack_this_site_url = 'https://www.hackthissite.org/user/view/'

# this function takes a string as input and outputs a list with all the integers in the string
def get_num(string):

    # get the numbers from string
    lst = ''.join([x if x.isdigit() else ' ' for x in string]).split()

    # change to list of ints instead of strings
    new_lst = []
    for item in lst:
        new_lst.append(int(item))

    return new_lst

class Crawler(object):
    def __init__(self, url):
        self.url = url

    # retrieve data from site and
    def get_site_data(self, user):
        request = requests.get(self.url + user)
        return bs4.BeautifulSoup(request.text, 'lxml')

    def certified_secure(self, user):
        experience = self.get_site_data(user).select('.level_progress_details')[0].getText()
        # get the points from the string
        return get_num(experience)[1]

    def hack_this_site(self, user):
        experience = self.get_site_data(user).select('.blight-td')[1].getText()
        return get_num(experience)[0]

# make to instances to crawl
cs = Crawler(certified_secure_url)
hts = Crawler(hack_this_site_url)

for user in users:
    print cs.certified_secure(user)
    print hts.hack_this_site(user)

You could use a list comprehension in get_num:

def get_num(string):
    """this function takes a string as input and outputs a list with all the integers in the string"""
    # get the numbers from string
    numbers = ''.join(x if x.isdigit() else ' ' for x in string).split()

    # change to list of ints instead of strings
    return [int(number) for number in numbers]
    # return map(int, numbers)  # Alternative

Also note that join can take a generator expression, so no need to convert to a list first. I also chose more descriptive variable names here.

You should also give your functions (and classes) a docstring (by putting a triple-""" delimited string as the first line of the funciton body, as I did above), which you can access interactively via help(function_name) and which is used by many documentation building tools.

It also seems a bit too manual to have to know which method to call depending on the url. Your crawler could decide that on its own:

class Crawler(object):
    sites = {"hackthissite.org": ('.blight-td', 1, 0),
             "certifiedsecure.com": ('.level_progress_details', 0, 1)}

    def __init__(self, url):
        self.url = url
        self.options = self.get_sites_options(Crawler.sites)

    def get_sites_options(self, sites):
        for site, options in sites.items():
            if self.url in site:
                return options

    def get_site_data(self, user):
        """retrieve data from site and"""
        request = requests.get(self.url + user)
        return bs4.BeautifulSoup(request.text, 'lxml')

    def get_experience(self, user):
        select_str, index, out_index = self.options
        experience = self.get_site_data(user).select(select_str)[index].getText()
        return get_num(experience)[out_index]


Typeahead / Autocomplete using AngularJS directive I am writing a Type-ahead / Autocomplete search with angular using directive and filter service. I am almost there but need your expert opinions on things which I am planning to achieve.

Git Repository

@Todos:

Look and feel
Allow navigation using keyboard
Enter and Esc key handling

Could anyone review and suggest more optimize ways of writing this?

Also, shall I bind events within a parent controller scope or directive scope? Here is how my directive looks like:

var modServiceDef = angular.module('modSerDefinition', []);

modServiceDef.controller('modDefCtrl', ['$scope', 'myFactory', function($scope,  myFactory) {
    myFactory.getData().then(function (httpData) {
            $scope.dataToPopulate = httpData;
        },function(httpData) {
            console.log('name retrieval failed.');
        });
    }
]);

modServiceDef.directive('typeAhead',[function() {
    return {
        scope : {
            cinfo:'=datalist',
        },
        link:function(scope,element,attr) {
            scope.visible = true;
            scope.updateSearchTerm = function(selName) {
                scope.sterm = selName;
            }
        },
        templateUrl:'templates/typeahead.tmpl'
    }
}]);

modServiceDef.factory('myFactory', function($http,$q) {
    var factory = {};
    var def = $q.defer();
    factory.getData = function() {
        $http.get('https://raw.githubusercontent.com/dominictarr/random-name/master/first-names.json')
            .success(function(res){
                factory.data = res;
                def.resolve(res);
            })
            .error(function(err){
                def.reject(err);
            })
            return def.promise;
        }
    return factory;
});

HTML:

<type-ahead datalist="dataToPopulate"></type-ahead>

Template:

<input type='search' id='idSearch' ng-model='sterm' placeholder='Enter Search Term' />

      {{data}}

There are some points you could improve:

App structure:
I would put the typeahead directive in an own module that you can include as a dependency to your app. angular.module('demoApp', ['pdTypeAhead']).
Click events: I think the best place is in the directive controller because they're there for your typeahead logic.
Keyboard binding: You can add the mousetrap.js keyboard binding in a directive and add it in your typeahead link method. It's a bit tricky because you need to change the typeahead restrict to attribute (otherwise there is a problem with infinite digest loop.).
Your todos:

look & feel: Twitter Bootstrap is a good starting point to do your styling.
Keyboard navigation: You could do it with pure js but I think mousetrap.js is easier to use.
Enter + Escape handling: Also use mousetrap. For event handling you could use $scope.$brodcast('eventName', data) and catch it in your directive's controller with $scope.$on('eventName', function(data) { ... });
Responsiveness: If you'd type the letter a in your code it took pretty long to show the result (> 3000 matches). I think it's OK to limit the result to 100 or lower matches with a limitTo filter and the result is immediately visible.

You can find my code in this fork. (If you like I could do a pull request in your repo.)

There are still the following possible improvements in the updated code:

Controller of the typeahead directive could be in it's own file. Improves readability.
It's probably possible to put code from the typeahead controller into a service for refactoring.
Escape key handling could be improved: After a click on a name you can't undo that click with a press on escape. But if you select with enter you can undo with escape.
Directive template typeahead.tmpl could be moved to the components folder of the directive js/components/typeahead.
Typeahead controller: Use controllerAs instead of $scope (if possible). Best practice, see style guide.
Check if filtering is also possible directly in ng-repeat? Should be possible and better. I've just moved it to js for easier debugging.
Add options to typeahead, e.g. a way to enable or disable autofocus of search input directly in markup e.g. <div type-ahead datalist="dataToPopulate" auto-focus="off">
Add a config provider for options service so you could add the options application wide inside of angular.module(...).config(...). This post is helpful for creating configurable directives.
Add ng-blur to close the typeahead if clicked outside.
Add unit tests for the directive.


Parsing infix expressions in Clojure I'm trying to teach myself Clojure by reading the Brave Clojure book online, and at the end of Chapter 7 there is an exercise to parse a mathematical expression written as a list in infix notation to s-expressions, following operator precedence rules.

To do this I implemented the shunting-yard algorithm in Clojure. However I am not sure if I am doing this in a functional way, as I am used to programming in languages like C++ and Python. In particular, I am uncertain whether the use of the loop operator is valid, as I'm coding this in basically the same way I would as in C++, just using recur on every loop iteration instead of mutating variables. This makes it seem a little repetitive.

Below is the code. It would be greatly appreciated someone takes a look and offers advice about how to do this in a more idiomatic functional way, or about anything else.

(def operator-precedence {'= 1, '+ 5, '- 5, '* 6, '/ 6})

(defn infix-parse
  "Converts an infix expression to s-expressions in linear time with the
  shunting-yard algorithm."
  [expr]
  (if (list? expr)
    (loop [val-stack ()
           op-stack ()
           remaining expr
           next-op false]
      (if next-op
        (let [prec (if (empty? remaining)
                     ##-Inf
                     (operator-precedence (first remaining)))
              popped (take-while #(>= (operator-precedence %) prec) op-stack)
              result (reduce
                       (fn [[a b & vals] op]
                         (cons (list op b a) vals))
                       val-stack
                       popped)]
          (if (empty? remaining)
            (first result)
            (recur result
                   (cons (first remaining)
                         (drop (count popped) op-stack))
                   (rest remaining)
                   false)))
        (recur (cons (infix-parse (first remaining)) val-stack)
               op-stack
               (rest remaining)
               true)))
    expr))

(defmacro infix
  [form]
  (infix-parse form))

(infix (1 + 3 * (4 - 5) * 10 / 2))
=> -14
(infix-parse '(1 + 3 * (4 - 5) * 10 / 2))
=> (+ 1 (/ (* (* 3 (- 4 5)) 10) 2))

First, yes, this is functional, and yes, this is how to use loop properly.

  . . . using recur on every loop iteration instead of mutating variables.

This is the intent of loop. Instead of mutating a variable, you just pass the "altered data" to the next iteration via recur.

You also aren't abusing side effects by misusing def or atoms, so that's good.

My main concern with this code is how it's really just one giant function. There's no function names indicating what certain code is doing, and no comments noting the significance of any lines. Now, I'm personally a stickler for breaking code down into functions, but it's generally regard as best practice as well. As this answer notes (ignoring the mention of "imperative"):

  Having a large imperative function that conveys many ideas is hard to digest and reuse.

I think the size of this function does make it hard to digest.

Just as an extreme counter example, here's the same algorithm that I wrote a year ago*. It's almost 4x longer as yours, but it's also much clearer what each bit of code is doing (and has ~10 lines dedicated to documentation). Code like

(-> equation
    (tokenize-equation)
    (infix->RPN-tokens op-attr-map)
    (tokens-to-string)) ; Should arguably be called token->string

makes it fairly clear what it's responsible for, even without taking into consideration the function name that it's in.

I'm won't try to break your code down (mostly because I burned my hand pretty bad, and typing this is hard enough), but if you take a look at my example, you may be able to find some inspiration and strike a middle ground for granularity.

Good luck

*  Although my version doesn't evaluate the expression since I'm translating it into a String and in my example, the data isn't available at compile-time.


Code to print even and Odd number using two threads Below I am giving you my code which I wrote to print even number using even thread and odd number using odd thread sequentially.

Could anyone please validate my code whether it is correct way of doing it or not? I am getting correct result, but I just want to make sure whether I am doing it in correct way or not.

public class PrintNumberUsingThread {

    public static void main(String[] args) {
        PrintNumberUsingThread example = new PrintNumberUsingThread();
        PNThread pnThread = example.new PNThread(10);
        new Thread(pnThread).start();
        new Thread(pnThread).start();
    }
    class PNThread implements Runnable {

        private int number = 0;
        private int maxNumber;
        public PNThread(int maxNumber) {
            this.maxNumber = maxNumber;
        }
        @Override
        public void run() {

            while (number <= maxNumber) {
                synchronized (this) {

                    String threadName = Thread.currentThread().getName();
                    if (number <= maxNumber) {

                        boolean even = threadName.contains("0") && number % 2 == 0;
                        boolean odd = threadName.contains("1") && number % 2 != 0;
                        if (even || odd) {

                            System.out.println(threadName + "-" + number++);
                            this.notifyAll();
                        } else {
                            try {
                                this.wait();
                            } catch (InterruptedException e) {
                                e.printStackTrace();
                            }
                        }
                    }
                }
            }
        }
    }
}

If the only requirement is to have two threads, one printing even numbers and other printing odd numbers, then your solution is unnecessarily complicated. The fact that you share the same data model for both threads forces you to synchronize access to the object making the solution unnecessarily complicated. The assignment did not require numbers being printed in sequence, so the coordination between the threads is unnecessary.

public class Counter implements Runnable {
    private final int start;
    private final int end;
    private final int increment;
    ... add boilerplate constructor ...
    public void run() {
        for (int i = start; i <= end; i += increment) {
            System.out.println(i);
        }
    }
}

Start threads:

new Thread(new Counter(1, 10, 2)).start();
new Thread(new Counter(2, 10, 2)).start();


Project Euler #12 (Highly divisible triangular numbers) in Python 3.x I'm a beginner to programming and just started python, I've been trying to work through Project Euler challenges.

I wrote a program to solve Project Euler #12, which asks for the smallest number of the form 1+ 2 + 3 + … + n which has over 500 divisors.  It works perfectly with smaller numbers like 100, but takes too long trying to find up to 500 factors. I gave up waiting after about 20 minutes. I'd like some help with optimizing my code to make it faster.

I basically made a function that finds the triangular number of a given number and another function that counts the number of factors/divisors of a number. I made a while loop where a number keeps increasing till the triangular number of itself has more than 500 factors.

And here is my code:

#Starts the timer to measure time taken for code to run:
import time
start_time = time.time()

#Returns the triangular number of 'num':
def triangular(num):
    return int((num*(num+1))/2)

#Returns the number of factors(divisors) of 'num':
def facCount(num):
    summ = 0
    for i in range(1, num+1):
        if num % i == 0:
            summ += 1
    return summ

#Declares x (starting value):
x = 0

#main:

while True:
    #Checks if number of factors for the triangular of x is above 500
    if facCount(triangular(x)) > 500:
        #Sets the value of 'ans' to the triangular of 'x':
        ans = triangular(x)
        #Exits the loop since answer is found
        break
    x += 1

#Prints answer (the first triangular number that has more than 500 factors):
print(ans)

#Prints the time taken for code to execute:
print("--- %s seconds ---" % (time.time() - start_time))

Your function facCount, which should be called something like count_factors, according to Python's official style-guide, PEP8, can be greatly improved by noticing that if num % i == 0, then automatically num % num / i == 0 (in other words, factors always come in pairs of two, except for if the number is a square, in which case you double count one). This means you only need to check factors up to \$\sqrt{n}\$:

from math import sqrt

def count_factors(num):
    """Return the number of factors of `num`"""
    sum_ = 2 * sum(num % i == 0 for i in range(1, int(sqrt(num)) + 1))
    if int(sqrt(num))**2 == num:
        sum_ -= 1
    return sum_

I also added a docstring describing what the function does in an accessible way.

The other improvement concerns the way you get the triangular numbers. While it is good to know Gauss formula, IMO it is here easier to manually calculate them. Your function needs to do one increment, one multiplication and one division, when all you really need is one addition per loop iteration:

from itertools import count

def triangular_nums():
    """Yield the triangular numbers"""
    t = 0
    for i in count():
        t += i
        yield t

If, for some reason, you dislike itertools, you can also replace it with a while True loop.

In Python 3+, you can use the new function itertools.accumulate (3.2+) and the new keyword combination yield from (3.3+), as mentioned in the comments by @MaartenFabré:

from itertools import accumulate, count

def triangular_nums():
    yield from accumulate(count())

With this your main function (which you should either make a real function or at least put under an if __name__ == "__main__": guard) becomes:

def main():
    for t in triangular_nums():
        if count_factors(t) > 500:
            return t

if __name__ == "__main__":
    ans = main()
    if ans is not None:
        print(ans)

The timing you should factor out and make into a decorator:

import time

def timeit(func):
    def wrapper(*args, **kwargs):
        start = time.time()
        ret = func(*args, **kwargs)
        print("--- %s seconds ---" % (time.time() - start))
        return ret
    return wrapper

@timeit
def main():
    ...