How to optimize a nested foreach loop which is creating same data object (with different values) I have following code which gets MastrSegments from repository(entity framework). It's working but I think I am  violating DRY (though not sure).

MastrSegments conatins collection of SegReqs and SegReq contains collection Routes. And I am looping through them to update RootObject

    foreach (var MastrSegment in MastrSegments)
    {

        var segReqCount = MastrSegment.segReqs.Count();

        var MastrSegmentRow = new Row
        {
            c = new RootObject[] { new RootObject { v = MastrSegment.Name }, new RootObject { v = "TopClient" }, new RootObject { v = segReqCount }, new RootObject { v = 12 } }

        };

        foreach (var segReq in MastrSegment.segReqs)
        {

            var routesCount = segReq.routess.Count;
            Row segReqRow;

            if (segReq.Name != MastrSegment.Name)
            {
                segReqRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = segReq.Name }, new RootObject { v = MastrSegment.Name }, new RootObject { v = routesCount }, new RootObject { v = -26 } }

                };
            }
            else
            {
                throw new NotSupportedException("MastrSegment name and  segReq name can not be same !!");
            }

            foreach (var routes in segReq.routess)
            {
                var circuitsCount = routes.Circuits.Count;
                Row routesRow = null;

                if (routes.Name != segReq.Name)
                {
                     routesRow = new Row
                    {
                        c = new RootObject[] { new RootObject { v = routes.Name }, new RootObject { v = segReq.Name }, new RootObject { v = circuitsCount }, new RootObject { v = 16 } }

                    };
                }
                else
                {
                    throw new NotSupportedException("segReq name and  routes name can not be same !!");
                }

                rows.Add(routesRow);
            }

            rows.Add(segReqRow);
        }

        rows.Add(MastrSegmentRow);
    }

    var returnData = rows.ToArray();

when ever you want to solve violations of the DRY principle you have to convert similar code into equal code.

If you look at the two inner loops: make the most inner loop using new local variables to replace the differeing parts

       {
            var circuitsCount = routes.Circuits.Count;
            Row routesRow = null;

            if (routes.Name != segReq.Name)
            {
                 routesRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = routes.Name }, new RootObject { v = segReq.Name }, new RootObject { v = circuitsCount }, new RootObject { v = 16 } }

                };
            }
            else
            {
                throw new NotSupportedException("segReq name and  routes name can not be same !!");
            }

            rows.Add(routesRow);
        }

becomes

       { // may be no valid syntax since I'm not a C# guy...
            var count = routes.Circuits.Count;
            Row routesRow = null;
            var master = "routes"
            var masterName = routes.Name;
            var detail = "segReq"
            var detailName = segReq.Name;
            var magicNumber =  16;

            if (masterName != detailName )
            {
                 routesRow = new Row
                {
                    c = new RootObject[] { new RootObject { v = masterName }, new RootObject { v = detailName }, new RootObject { v = count }, new RootObject { v =  magicNumber } }

                };
            }
            else
            {
                throw new NotSupportedException(detail+" name and "+master+" name can not be same !!");
            }

            rows.Add(routesRow);
        }

now you extract the "future common" part to a new method:

       {
            var count = routes.Circuits.Count;
            var master = "routes"
            var masterName = routes.Name;
            var detail = "segReq"
            var detailName = segReq.Name;
            var = magicNumber  16;

            Row routesRow = CreateMasterDetailRow(master, masterName, detail, detailName, count, magicNumber);
            rows.Add(routesRow);
        }
//...

func Row CreateMasterDetailRow(var master, var masterName, var detail, var detailName, var count, var magicNumber)
{

            if (masterName != detailName )
            {
                 return new Row
                {
                    c = new RootObject[] { new RootObject { v = masterName }, new RootObject { v = detailName }, new RootObject { v = circuitsCount }, new RootObject { v =  magicNumber } }

                };
            }
            else
            {
                throw new NotSupportedException(detail+" name and "+master+" name can not be same !!");
            }
}

inline the extracted variables into the function call:

       {
            Row routesRow = CreateMasterDetailRow("routes", routes.Name, "segReq", segReq.Name, routes.Circuits.Count, 16);
            rows.Add(routesRow);
        }

then use the call in the middle loop:

   {  // maybe I got the order of "master" and "detail" wrong...
        Row segReqRow= CreateMasterDetailRow("MastrSegment ", MastrSegment.Name, "segReq", segReq.Name, segReq.routess.Count, -26);
        foreach (var routes in segReq.routess)
        {
            Row routesRow = CreateMasterDetailRow("routes", routes.Name, "segReq", segReq.Name, routes.Circuits.Count, 16);
            rows.Add(routesRow);
        }
        rows.Add(segReqRow);

You can even replace the similar code in the outer loop if you provide some "fake data" for the (not failing) consistency check to satify the new methods parameter list...


Coding and printing a graph This is my first attempt at putting the conceptual knowledge I've gained about graphs into code. When searching for examples they all seem overly complicated, and searching for a tutorial or introduction leads to conceptual information. So here I am trying to code a very simple graph and this is what I have. The vertices represent cities, the edges represent paths  and their distances between cities.

I have two main questions:

Is this a graph? Am I missing anything? I think I have a decent understanding of them conceptually, so at least for a basic graph I think this suffices.
Are the pointers necessary? Is there a better way I could be implementing them? It was my understanding that an Edge is supposed to just contains pointers to vertices.

I believe the code is pretty straightforward: you have your edge, vertex, and graph classes, then main which creates one vertex per city, one pointer per vertex, and one edge per path between cities, most are one way, a couple are bi-directional. Then it just prints the contents of the graph which just prints each city and all paths from that city.

I know it's missing a remove function and I plan on making one soon. Just removing the vertex from the graph and removing any edges that lead to/from that vertex, but I just don't have the time at the moment.

#include <vector>
#include <iostream>
#include <string>

using namespace std;

class Vertex;

class Edge
{
public:
    Edge(Vertex *org, Vertex *dest, int dist)
    {
        origin = org;
        destination = dest;
        distance = dist;
    }

    Vertex* getOrigin() {return origin;}
    Vertex* getDestination() {return destination;}
    int getDistance() {return distance;}
private:
    Vertex* origin;
    Vertex* destination;
    int distance;
};

class Vertex
{
public:
    Vertex(string id)
    {
        name = id;
    }

    void addEdge(Vertex *v, int dist)
    {
        Edge newEdge(this, v, dist);
        edges.push_back(newEdge);
    }

    void printEdges()
    {
        cout << name << ":" << endl;
        for (int i = 0; i < edges.size(); i++)
        {
        Edge e = edges[i];
        cout << e.getDestination()->getName() <<
            " - " << e.getDistance() << endl;
        }
        cout << endl;
    }

    string getName() {return name;}
    vector<Edge> getEdges() {return edges;}

private:
    string name;
    vector<Edge> edges;
};

class Graph
{
public:
    Graph() {}

    void insert(Vertex *v)
    {
        vertices.push_back(v);
    }

    void printGraph()
    {
        for (int i = 0; i < vertices.size(); i++)
            vertices[i]->printEdges();
    }

private:
    vector<Vertex*> vertices;
};

int main()
{
Graph g;

Vertex v1 = Vertex("Seattle");
Vertex v2 = Vertex("Portland");
Vertex v3 = Vertex("Everett");
Vertex v4 = Vertex("Lynnwood");
Vertex v5 = Vertex("Northgate");
Vertex v6 = Vertex("Bellevue");
Vertex v7 = Vertex("Arlington");
Vertex v8 = Vertex("Bellingham");

Vertex *vp1 = &v1;
Vertex *vp2 = &v2;
Vertex *vp3 = &v3;
Vertex *vp4 = &v4;
Vertex *vp5 = &v5;
Vertex *vp6 = &v6;
Vertex *vp7 = &v7;
Vertex *vp8 = &v8;

v1.addEdge(vp2, 100);
v1.addEdge(vp6, 20);
v2.addEdge(vp1, 100);
v3.addEdge(vp1, 30);
v3.addEdge(vp4, 10);
v3.addEdge(vp7, 20);
v4.addEdge(vp5, 15);
v5.addEdge(vp1, 10);
v6.addEdge(vp1, 20);
v8.addEdge(vp7, 45);

g.insert(vp1);
g.insert(vp2);
g.insert(vp3);
g.insert(vp4);
g.insert(vp5);
g.insert(vp6);
g.insert(vp7);
g.insert(vp8);

g.printGraph();

system("PAUSE");

return 1;
}

is this a graph?

Yes

Am I missing anything?

In terms of a graph: No.
You have chosen to make your edges uni-directional (thus two edges are required to mark routes between cities). Not an issue in itself but you could have helper functions that create two edges automatically.
In terms of good coding: Yes
You have completely missed out encapsulation.

I think I have a decent understanding of them conceptually, so at least for a basic graph I think this suffices.

Your understanding of a graph is fine. Your understanding of implementing them needs some work. The whole point of a class and encapsulation is to make sure the object keeps and maintains state. Access to the state is controlled so that it can not be manipulated incorrectly.
Unfortunately this is not the case in your implementation. You have leaked the state of the object outside the class thus expose yourself to it be manipulated without the graphs consent (this can make maintaining other information very hard).
Also the state used by the graph may not be scoped in the same way as the graph and thus you can potentially loose state (ie objects dying) thus making the graph invalid.
Additionally you use pointers very poorly. It is very rare to pass pointers across an interface boundaries as there is not associated concept of ownership. Using pointers internally is perfectly fine (i.e. don't pass pointers through public methods).

are the pointers necessary?

Probably not.
Also they are probably not a good idea.

Is there a better way I could be implementing them?

Yes.
If you encapsulate your vertices inside the graph then you can give them 'ids' (probably just the index into a vector of vertices). This ids can be universal and need not change. Or you could use the name as the 'id' it all depends on implementation.

It was my understanding that an Edge is supposed to just contains pointers to vertices.

Rather than use the word 'contains pointers to' just use the term 'refer' (so as not to get us confused with the term references).
An edge has a source and a destination vertex with a value (representing the distance (or cost)) to transfer between them along the edge. How the edge manages that information will depend on implementation.
Code Analysis
Don't do this:
using namespace std;

See https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice
I would not expose Edge publicly.
They are an internal state of the graph. If you expose them you will be unable to change the internal represent of an edge which binds you to this form (i.e. you will not be able to update your class with improvements later).
class Edge

Do not pass pointers over a public interface:
public:
    Edge(Vertex *org, Vertex *dest, int dist)

Here you are passing pointers (in a very C like form).

Are you giving ownership of the objects to the edge (unlikely)?
Are the vertices going to be NULL (unlikely)?

So here I would suggest passing them by reference. BUT I also know that we will storing the vertices internally as well I would refer change the graph to hold a vector of the vertex and then you only need to pass their index in the vector as a reference.
BUT then if we consider that a graph may be huge. We don't want to scan through all the edges when we know a start node. So personally I would have each vertex hold a vector of edges. Since we now know the start node you only need to pass the destination node and cost.
Same thing for Vertex. No need for it to be public (same reason).
class Vertex

No problem with a method printEdges() but output is usually done via operator<<.
void printEdges()

Don't pass pointer through a public interface.
Personally I would copy the method used by the standard containers and pass a const reference and copy it into the object (or with C++ pass a r-value reference that can be moved into the object).
void insert(Vertex *v)

Again with printing. Prefer operator<<
void printGraph()

This is how I would expect the graph to work:
Graph  g;
// Type one: Add vertices (if they don't exist) and add
//           two edges (one for each way).
g.addEdges("Seattle", "Portland", 100);
g.addEdges("Seattle", "Bellevue",  20);

// Type two: Add vertices (if they don't exist) and add
//           one edges
g.addEdge("Everett",  "Seattle", 30);

// Type three: Add new Vertix.
//             Use the id to add edges.
int id1 = g.addVertix("Everett");
int id2 = g.addVertix("Lynnwood");
g.addEdge(id1, id2, 10);


Is this a good way of writing contains method in Java? I want to check if the structure that I use contains an object. To do so I wrote the following contains method:

boolean contains(Topic t){
        if (t == null)
            return false;
        if (t.equals(root))
            return true;
        else
            return false;
    }

Is this a good way of writing? Is it better to write:

boolean contains(Topic t){
        if (t == null)
            return false;
        if (t.x == root.x && t.y == root.y)
            return true;
        else
            return false;
    }

Neither is best. The best would be:
boolean contains(Topic t){
    return t != null && t.equals(root);
}

Using equals is better than using the properties of Topic, because of Single Responsibility Principle
If that was too fast for you:
Step 1:
if (t.x == root.x && t.y == root.y)

Using t.equals(root) is better. It is likely that you want to do this comparison in more than one method between two variables of Topic, in which case you don't want to duplicate code.
Step 2:
if (condition)
    return true;
else
    return false;

condition is a boolean. You return a boolean. You might as well just return condition directly.
Now, using single-line if-statements without braces can lead to bugs when you suddenly introduce a new line without remembering to add braces. Apple has done this mistake, and so can you.
With the steps taken so far we have:
boolean contains(Topic t) {
    if (t == null) {
        return false;
    }
    return t.equals(root);
}

Final step:
Use boolean short-circuiting && operator to return in only one line, while still having it readable (at least readable for those who are somewhat experienced with boolean logic, which most programmers hopefully are)
So again, the best version:
boolean contains(Topic t) {
    return t != null && t.equals(root);
}


Calculating the number of candy bars and gumballs that can be redeemed using coupons The following question was taken from Absolute Java 5th ed. by Walter Savitch:

  The video game machines at your local arcade output coupons according to how well you play the game. You can redeem 10 coupons for a candy bar or 3 coupons for a gumball. You prefer candy bars to gumballs. Write a program that defines a variable initially assigned to the number of coupons you win. Next, the program should output how many candy bars and gumballs you can get if you spend all of your coupons on candy bars first, and any remaining coupons on gumballs.

This is the code that I have written:

public class Question2 {

    private static final int NUMBER_OF_COUPONS_FOR_A_CANDYBAR = 10;
    private static final int NUMBER_OF_COUPONS_FOR_A_GUMBALL = 3;

    private static int numberOfCoupons = 13;

    public static void main(String[] args) {

        System.out.println(numberOfCandybars(numberOfCoupons)
                + " candybar(s) and " + numberOfGumballs(numberOfCoupons)
                + " gumball(s).");

    }

    private static int numberOfCandybars(int initialCoupons) {
        return initialCoupons / NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
    }

    private static int numberOfGumballs(int remainingCoupons) {
        remainingCoupons = numberOfCoupons % NUMBER_OF_COUPONS_FOR_A_CANDYBAR;
        return remainingCoupons / NUMBER_OF_COUPONS_FOR_A_GUMBALL;
    }
}

Looking at this statement, it looks as if numberOfGumballs and numberOfCandybars are independently derived from numberOfCoupons:

System.out.println(numberOfCandybars(numberOfCoupons)
        + " candybar(s) and " + numberOfGumballs(numberOfCoupons)
        + " gumball(s).");

But they are not independent, because numberOfGumballs actually depends on numberOfCoupons and numberOfCandybars too. Hiding this fact is not a good thing: one might expect that changing the behavior of numberOfCandybars won't affect the behavior of numberOfGumballs, but it actually will, which is confusing.

If you think about it, it would be best if numberOfCandybars returned 2 values:

The number of candy bars our coupons could buy
and, the number of coupons left

With these two values, we could use the number of coupons left to make another call to numberOfGumballs, and keep the number of candy bars for printing later. That way, the relationships between the functions could be well visible.

Of course, Java can only return a single value. But the single value can be an object, which can contain multiple values.

So let's start over. We have items: Candy bar, Gumball. We have some kind of a vending machine, that you put coupons into, select the item you want, and it gives back a number of items and change (remaining coupons).

Let's model the items with an enum, and give them a price too.

enum Item {
    CandyBar(10),
    Gumball(3);

    final int price;

    Item(int price) {
        this.price = price;
    }
}

And let's model the machine and the output it gives:

class AutomaticCandyMachineOutput {
    final Item item;
    final int itemCount;
    final int change;

    AutomaticCandyMachineOutput(Item item, int itemCount, int change) {
        this.item = item;
        this.itemCount = itemCount;
        this.change = change;
    }
}

class AutomaticCandyMachine {
    AutomaticCandyMachineOutput redeemCouponsForItems(int coupons, Item item) {
        int itemCount = coupons / item.price;
        int change = coupons % item.price;
        return new AutomaticCandyMachineOutput(item, itemCount, change);
    }

    AutomaticCandyMachineOutput redeemCouponsForCandybars(int coupons) {
        return redeemCouponsForItems(coupons, Item.CandyBar);
    }

    AutomaticCandyMachineOutput redeemCouponsForGumballs(int coupons) {
        return redeemCouponsForItems(coupons, Item.Gumball);
    }
}

This seems a more natural, flexible and extensible design. Now we can rewrite the original statement like this:

int coupons = 99;
AutomaticCandyMachine machine = new AutomaticCandyMachine();
AutomaticCandyMachineOutput output1 = machine.redeemCouponsForCandybars(coupons);
AutomaticCandyMachineOutput output2 = machine.redeemCouponsForGumballs(output1.change);

String.format(
    "%d candybar(s) and %d gumball(s)",
    output1.itemCount,
    output2.itemCount);


Add user star function triggering PHPmailer email optimisation The function below simple allows a logged in user to "star" another user for convenience when trying to find the that user another time. At the moment the function works as it should, it inserts the user giving the star and the user receiving the star into a database table, then grabs the first name and email from the users table to get more information about the user receiving the star and finally uses that data to send an email to the user receiving the star.

As it stands however, this function is pretty slow when used. Based on user research, users tend to press the "Star user" button numerous times as the application seems unresponsive; when in reality, it is just taking longer as it is sending an email (I've tested the function without the email trigger and it is much faster).

Is there a better way of triggering emails to increase performance using this as an example? Any improvements/recommendations would also be appreciated.

public function addstar($user_id, $starredBy) {

    $query = $this->db->prepare("INSERT INTO " . DB_NAME . ".user_stars (star_id, starred_by_id) VALUES (?, ?)");

    $query->bindValue(1, $user_id);
    $query->bindValue(2, $starredBy);

    $this->db->beginTransaction();

    try{
        $query->execute();

        $getUserInfo = $this->db->prepare("SELECT u.firstname, u.email, u.user_id FROM " . DB_NAME . ".users AS u WHERE u.user_id = ?");
        $getUserInfo->bindValue(1, $user_id);
        $getUserInfo->execute();
        $to = $getUserInfo->fetch();

        $firstName = $to['firstname'];
        $email = $to['email'];

        $this->emails->sendStarEmail($firstName, $email);

        $this->db->commit();

        header('Location: ' . $_SERVER['HTTP_REFERER']);

    }catch(PDOException $e) {
        $this->db->rollback();
        $users = new Users($db);
        $debug = new Errors();
        $debug->errorView($users, $e);
    }
}

Lock the UI until a response comes back (or a reasonable timeout
occurs) to prevent the multiple clicking.
Don't send emails directly from the code, push them to spool or MQ and process separately

As for the code itself - there's nothing much to comment except that it's completely out of touch with modern best practices in PHP world. Consider using a proper framework, ORM or at least a DBAL. See phptherightway for more details.


API Implementation and Architecture I recently put together an Express/Node API that consumes and transforms an additional third party API.

And while everything is working great, I feel there may be ways to improve the architecture and implementation.

This is my current implementation:

    nodes_modules/
    routes/
    ...api.js
    ...search.js
   index.js
   .babelrc
   .config.js
   .package.json

api.js

import express from "express";
let router = express.Router();

router.use('/search', require('./search'));

module.exports = router;

search.js

import express from "express";
import axios from "axios";
import transformer from "transformer";
const router = express.Router();

router.get('/:type', (req, res) => {

    const type = req.params.type;

    axios.get('http://example.com/api/' + type)
        .then((response) => {

            let transformed = {};

            switch(type) {
                case "people":
                    transformed = transformer.search(response.data, "results[*].{ name: name }");
                    break;
                case "aircrafts":
                    transformed = transformer.search(response.data, "results[*].{ name : aircraftName }");
                    break;
                default:
                    res.status(400).send({ error : "Bad request" });
                    break;
            }

            res.send({original: response.data, transformed: transformed });
        })
        .catch( () => {
            res.status(400).send({ error : "Bad request" });
        });
});

module.exports = router;

index.js

import express from 'express';
import bodyParser from 'body-parser';
import cors from 'cors';

const app = express();
app.use(cors());
app.use(bodyParser.urlencoded({  extended: true }));
app.use(bodyParser.json());

app.use('/api', require('./routes/api'));

app.use('*', (req,res) => {
   res.status(404).send('Page Not Found');
});

app.listen(3030, function() {
   console.log('running on port 3030');
});

I appreciate any suggestions for improvement, pointing out of any flaws or useful changes that can be made to the current implementation. Thank you

After reading over the code snippets you included, it looks pretty simple.

The only concern I have is that if the switch statement reaches the default case, then res.status(400).send() is called , and then just after the switch statement, res.send() is called... If a 400 status is sent, then it doesn't seem appropriate to send the response data (even though transformed wouldn't be assigned in the default case.

One other improvement I see is that the 400 Bad Request response could be made consistent by moving that into a function and calling it in the two places where a 400 response is sent. So in the example below, that is defined as the function badRequest() inside the callback to router.get(), or that could be moved outside the callback and would then likely need to accept the response object as a parameter.

router.get('/:type', (req, res) => {
    function badRequest() {
        res.status(400).send({ error : "Bad API request" });
    }
    const type = req.params.type;

    axios.get('http://example.com/api/' + type)
        .then((response) => {

            let transformed = {};

            switch(type) {
                //..omitting lines for brevity
                default:
                    badRequest();
                    return; //stop the function, since 400 response sent
            }

            res.send({original: response.data, transformed: transformed });
        })
        .catch( () => {
            badRequest();
        });
});


Node.JS Server Queue Processor I implemented a simple queuing system for my Node.JS app and wanted a critique on it's structure.

const TPS = 20;
const Queue = {
    counter: 1,
    items: {},
    /**
     * Add an item to the queue, with the given func to call
     * @param {Function} func
     * @param {Boolean} repeating
     * @return {Number}
     */
    add(func, repeating = false) {
        const id = this.counter++;
        this.items[id] = {func, repeating, id};
        return id;
    },
    /**
     * Remove an item from the queue with the given id
     * @param {Number} id
     */
    remove(id) {
        if (this.items.hasOwnProperty(id)) {
            delete this.items[id];
        }
    },
    /**
     * Process items in the queue
     */
    process() {
        for (let id in this.items) {
            // Prevent this item from being processed again
            if (!this.items.hasOwnProperty(id) || this.items[id].processing) {
                continue;
            }

            // Delete this item when it's scheduled for deletion
            if (this.items[id].scheduledForDeletion) {
                delete this.items[id];
                continue;
            }

            // Let the queue know this item is being processed and
            // it's scheduled deletion status
            this.items[id].processing = true;
            this.items[id].scheduledForDeletion = !this.items[id].repeating;

            // Don't wait for item's promise to resolve, since this
            // will create a backlog on the queue
            (async () => {
                try {
                    await this.items[id].func.call(null);
                } catch (err) {
                    // TODO: Handle errors.
                    console.error(err);
                }

                this.items[id].processing = false;
            })();
        }
    }
};

(function tick() {
    setTimeout(tick, 1000 / TPS);
    Queue.process();
})();

This is an example of how it's implemented.

// Add three items to the queue: 1 normal, 1 async and 1 repeating
Queue.add(() => console.info(`[tick] -> ${Date.now()}`));
Queue.add(async () => setTimeout(() => console.info(`[async] -> ${Date.now()}`), 100));
const timeLoop = Queue.add(() => console.info(`[loop] time (loop) -> ${Date.now()}`), true);

// Remove the looping item from the queue
setTimeout(() => Queue.remove(timeLoop), 500);

The idea is to have this run when the server starts and continually process the queue items. Queue is in it's own file and exported. I import this file into my controllers and call (for example) Queue.add('function to add user to DB and send out email').

The structure looks fine. It is quite succinct and makes good use of const and let where appropriate.

To adhere to the D.R.Y. principle, process() can and should utilize remove() to remove items from the queue.

I considered suggesting that arguments be accepted with each function but that can be achieved with partially bound functions.

I also considered suggesting you consider using a class, since ES-6 featured can be utilized, but then you would either need to instantiate a queue once or else make all methods static.

I would suggest you consider accepting an error handler callback for each function. That way, instead of writing all errors to the console, the caller could add an appropriate handler.


Executor tasks for parallelizing a method So I am trying to delete a list having n number of objects and thereby I divided the list in 4 parts and let it to be deleted in parallel. The intention here is to do it in parallel so as to make it fast rather than sequential. deleteObject(x) can be assumed as an API which finally deletes an object has the code to take care of concurrency. It returns true on successful delete and false otherwise

I have few question here

So as I have 4 parallel method which I am executing though invokeAll
and there are 4 threads declared by Executors.newFixedThreadPool(4)
would there always be 1 thread be assigned to 1 method?
Do I need to synchronize and use volatile for iterator 'i' in the for loop of parallelDeleteOperation() method. The reason I am asking this is supposing if 1st thread has not completed its task (deleting listDir1 and the for loop has not completed) and supposing in midway it got context switch and 2nd thread starts executing the same task(deleting listDir1). Just wondering if 2nd thread can get IndexOutOfBound exception in this case.
Is there any advantage of dividing list in 4 parts and executing this rather than having multiple threads executing delete operation on a very big list.
If one of the operation of ExecutorService returns false then on the whole deleteMain() API would return false

public boolean deleteMain(List<Integer> listDir) {
if(listDir.size()<4){
    return parallelDeleteOperation(listDir);
}
final List<Integer> listDir1 = listDir.subList(0, listDir.size() / 4);
final List<Integer> listDir2 = listDir.subList(listDir.size() / 4, listDir.size() / 2);
final List<Integer> listDir3 = listDir.subList(listDir.size() / 2, (listDir.size()/ 4) *3);
final List<Integer> listDir4 = listDir.subList((listDir.size()/ 4)*3, listDir.size());`
Set<Callable<Boolean>> callables = new HashSet<Callable<Boolean>>();
callables.add(new Callable<Boolean>() {
    @Override
    public Boolean call() throws Exception {
        return parallelDeleteOperation(listDir1);
    }
});
callables.add(new Callable<Boolean>() {
    @Override
    public Boolean call() throws Exception {
        return parallelDeleteOperation(listDir2);
    }
});
callables.add(new Callable<Boolean>() {
    @Override
    public Boolean call() throws Exception {
        return parallelDeleteOperation(listDir3);
    }
});
callables.add(new Callable<Boolean>() {
    @Override
    public Boolean call() throws Exception {
        return parallelDeleteOperation(listDir4);
    }
});

ExecutorService service = Executors.newFixedThreadPool(4);
try {
    List<Future<Boolean>> futures = service.invokeAll(callables);

for(Future<Boolean> future : futures){
    if( future.get() != true)
        return future.get();
}
} catch (InterruptedException e) {
    e.printStackTrace();
} catch (ExecutionException e) {
    e.printStackTrace();
}finally{
service.shutdown();
}
return true;
}
public boolean parallelDeleteOperation(List<Integer> listDir) {
for (int i = 0; i < listDir.size(); i++) {
    int x = listDir.get(i);
    deleteObject(x);
    if(deleteObject(x)!=true)
        return false;
     }
return true;
}

First of all, regarding your questions:

Yes. The 4 Futures will be mapped to 4 executor threads for all practical considerations. There might be edge-cases, where the first future is finished before the last is submitted and a thread gets reused, but this does not lead to any practical implications.
No, the i is a local variable. There is a different instance in each thread.
As you go index-based, you might as well use start- and end-indexes for your Future executions instead of sublists. On the other hand, if the list is an ArrayList this is exactly what happens under the hood of the sublist call.
Yes, I cannot see the question there.

However, your code has some other points worth addressing:

as AJNeufeld already mentioned, deleteObject() gets called twice per object
if a single object deletion fails, the following objects in that sublist will not even be attempted - is that the expected behaviour?
Callable is a functional interface, so instead of creating an anonymous subclass for each callable, simply use a lambda expression: () -> parallelDeleteOperation(listDir1)
index-based list traversal only makes sense if you actually need the index variable. In this case, you should use for(int x : listDir)

Regarding the general principle: you try to do many things manually and via a design decision, which the system already can do for you. Especially the idea to use exactly 4 CPUs is odd: if the target machine only has 2 CPUs you will wast time through unnecessary thread-switches, on an 8-core half of the machine is idle.

Additionally, as I already mentioned above, you don't attempt to delete every object, but only up to the first failure.

To rectify this, I suggest that you simply use a parallelStream which will be split to a number of cores suited for the given machine automatically, and collect the total success value via a reducer:

boolean allDeletionsSuccessful
= listDir.parallelStream()             // split to number of cores
    .map(x -> deleteObject(x))         // attempt each deletion, record success value
    .reduce(true, (x, y) -> x & y);    // merge with logical AND


Finding max z-index featuring in the current page I have to display a cache layer on the whole viewport (to then display a kind of dialog box).
Without surprise I create a
 element with {position: fixed; top: 0 left: 0; width: 100%; height: 100%: background-color: #...; opacity: ...}.

But I faced the issue that I may encounter pages where some elements have a z-index other than 0 or auto.
So I must add a z-index property which surpasses the maximum one currently featuring in the page.

I'm currently using the following snippet:

var zMax = 0;
$('body *').each(function() {
  var zIndex = +$(this).css('zIndex'); // ("+" casts to number or NaN)
  if (zIndex > zMax) {
    zMax = zIndex;
  }
});

It works fine, but I'm not really happy with it: merely based on brute-force, it appears as "too easy" and a bit overkill.
I googled for other solutions and found a lot (notably on SO), but no one seems simpler than mine.

TL;DR; Is there some smarter alternative?

EDIT: to be more clear, here is an example of how the "cache layer" works:

$(document).ready(function() {
  $('#go, #goz').click(showDialog);

});
function showDialog() {
  $('body').append('
');
  $('body').append('

<button id="cancel">Cancel</button>
');
  $('#cancel').click(cancel);
  var text = 'Without z-index, cache doesn\'t cover image';
  if (this.id == 'goz') {
    $('#cache').css({zIndex: 101});
    $('#dialog').css({zIndex: 102});
    text = 'With z-index, cache covers all';
  }
  $('#text').html(text);
}
function cancel() {
  $('#dialog, #cache').remove();
}
#cache {
  position: fixed;
  top: 0;
  left: 0;
  width: 100%;
  height: 100%;
  background-color: #444;
  opacity: .7;
}
img {
  position: relative;
  z-index: 100;
}
#dialog {
  position: fixed;
  left: 100px;
  top: 50px;
  width: auto;
  border: 1px solid #000;
  background-color: #fff;
  padding: 1em;
}

Longe glabro ortus tempus crimina perquam notarius Magnentio odorandi in membra quos a perquam supergressus.

Batnae municipium in Anthemusia conditum Macedonum manu priscorum ab Euphrate flumine brevi spatio disparatur.

<button id="go">Go (simple)</button>
<button id="goz">Go (with z-index)</button>

Using the "Go (simple)" button, you see that the image is not covered by the cache (in this case, because it both has position: relative and a given z-index).
Using the "Go (with z-index)" button, we have set z-index of the cache and dialog layers to a sufficient value to overstrip any other element's ones (here it's hard coded).

The important point is that my tool snippet may be included everywhere, notably in the context of an existing app (like a CMS).
Since I obviously have no control over such an app, I must discover which is the current max z-index: then the cache and dialog layers will get this max value +1 (respectively +2).

While I understand you're looking for a snippet that works well on an underlying page you have no control over, any solution for such a problem would technically be a hack.

Your best bet, though certainly not a fool-proof solution, is to just use a very large or perhaps the maximum possible z-index (supposedly 2147483647, though certainly not standardised - I'd just go with something like 10000000) on your element.


Two variants of a db_create_resources(…) function After reading noobs, I'm interested in possible way to improve readability of the following short snippet.

a struct pictdb_file is a small file system containing a metadata array and a file descriptor fpdb.
the function allocates the resources, and forwards initialization to db_create_logic.

How can I improve the readability of this snippet ? Please comment about: the syntax, white-lines, comments, function extracting, etc.

Also, which version do you prefer and why ?

version 1:

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    //allocate metadata
    db_file->metadata = calloc(db_file->header.max_files, sizeof(pict_metadata));

    if (db_file->metadata == NULL) {
        return ERR_OUT_OF_MEMORY;
    }

    //open fpdb
    db_file->fpdb = fopen(db_filename, "wb");

    if (db_file->fpdb == NULL) {
        free(db_file->metadata);
        db_file->metadata = NULL;
        return ERR_IO;
    }

    //forward
    enum error_codes e;
    e = db_create_logic(db_filename, db_file);

    //release resources
    free(db_file->metadata);
    db_file->metadata = NULL;
    db_close(db_file);

    return e;
}

version 2: (comment and blank-lines changes)

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    db_file->metadata = calloc(db_file->header.max_files, sizeof(pict_metadata));
    if (db_file->metadata == NULL) {
        return ERR_OUT_OF_MEMORY;
    }

    db_file->fpdb = fopen(db_filename, "wb");
    if (db_file->fpdb == NULL) {
        free(db_file->metadata);
        db_file->metadata = NULL;
        return ERR_IO;
    }

    enum error_codes e;
    e = db_create_logic(db_filename, db_file);

    free(db_file->metadata);
    db_file->metadata = NULL;
    db_close(db_file);

    return e;
}

There's no specification. What does this function do? What arguments should I pass? What are the preconditions on db_file? (It looks as if I might have to fill in some of the fields, but which ones?)
"resources" is spelled thus.
The function does not check the preconditions.
Mixing fopen and db_close seems confusing and error-prone. fopen should be paired with fclose, and db_close should be paired with db_open.
If the call to fopen fails, it looks as if you lose information about the cause of the error. fopen can fail due to permissions (EACCES), lack of disk space (ENOSPC), missing directory (ENOENT), and so on, but these all get converted to ERR_IO.
The function uses db_file->metadata to hold a pointer to some memory, but this memory is not actually used by the function, and by the end of the function the memory is freed and the pointer set to NULL. It seems wrong to use the db_file structure this way. If db_create_logic needs some temporary memory, it should allocate it itself (and preferably in a local variable, not in the db_file structure).
The cleanup logic for db_file->metadata appears twice. This is a bad idea: if you changed it in one of the places, would you remember to change it in the other? Consider refactoring the error handling code to look like this:

enum error_codes
db_create_resources(const char* db_filename, struct pictdb_file* db_file)
{
    enum error_codes e;

    db_file->metadata = calloc(db_file->header.max_files,
                               sizeof *db_file->metadata);
    if (db_file->metadata == NULL) {
        e = ERR_OUT_OF_MEMORY;
        goto fail_calloc;
    }

    db_file->fpdb = fopen(db_filename, "wb");
    if (db_file->fpdb == NULL) {
        e = ERR_IO;
        goto fail_fopen;
    }

    e = db_create_logic(db_filename, db_file);
    db_close(db_file);
fail_fopen:
    free(db_file->metadata);
    db_file->metadata = NULL;
fail_calloc:
    return e;
}


Current python for loop not efficent I have the following working code

import numpy as np
r = np.loadtxt('CoordUnwrapped/ParticleCoordU.5000',skiprows=9,usecols = (4,5,6))
rcom = np.loadtxt('hexaCOM/COMA/COMA.5000',skiprows=10,usecols = (4,5,6))
N = 50

Sx1= []
Sx2 = []
Sx3 = []
for i in range(0,N):
    S_data_1 = (r[i,0]-rcom[0,0])**2)
    S_data_2 = (r[i+N,0]-rcom[1,0])**2)
    S_data_3 = (r[i+2*N,0]-rcom[2,0])**2)
    Sx1.append(S_data_1)
    Sx2.append(S_data_2)
    Sx3.append(S_data_3)

data = np.array([Sx1,Sx2,Sx3])

x = np.average(data, axis=0)

The problem that I'm facing is that I am looking to obtain 400 Sx vectors and with this format it is not really feasible. Just to be clear this is a snapshot of what I am looking to get at the end:

N = 50

Sx1= []
Sx2 = []
Sx3 = []
.
.
.
Sx400 = []

for i in range(0,N):
    S_data_1 = (r[i,0]-rcom[0,0])**2)
    S_data_2 = (r[i+N,0]-rcom[1,0])**2)
    S_data_3 = (r[i+2*N,0]-rcom[2,0])**2)
    .
    .
    .
    S_data_400 = (r[i+399*N,0]-rcom[399,0]**2)
    Sx1.append(S_data_1)
    Sx2.append(S_data_2)
    Sx3.append(S_data_3)

data = np.array([Sx1,Sx2,Sx3,...,Sx400])

x = np.average(data, axis=0)

As you can see this approach is too time consuming and not efficient at all. Any suggestions would be great. Thank you!

There isn't any need to name all your Sx variables and copy+paste the code to build each one; just build them in another loop.  You can do both loops as list comprehensions very concisely:

data = np.array([
    [(r[i+s*N, 0] - rcom[s, 0])**2 for i in range(N)]
    for s in range(400)
])


Create and print a tic-tac-toe board Write a program by creating an array of 9 integers that represent the
  positions in the tic-tac-toe board. Once the array is created, assign
  values to the board and print the contents of the board out. The
  values for the board are:

  Board Value: X, Integer in Array: 10
  Board Value: O, Integer in Array: 100
  Board Value: Empty, Integer in Array: 0

  This is an example of an array that represents a board:

[10|0|0|100|0|10|10|100|0]

  Here is what the above array would look like when printed to the screen:

+---+---+---+
| X |   |   |
+---+---+---+
| O |   | X |
+---+---+---+
| X | O |   |
+---+---+---+

And I basically solved the problem:

public class problem5 {

    public static void main(String[] args) {

        int[] a = new int[9];
        a[0] = a[5] = a[6] = 10;
        a[3] = a[7] = 100;
        a[1] = a[2] = a[4] = a[8] = 0;
        String[] b = new String[9];
        int x = 0;

        while (x < a.length) {
            if (a[x] == 10)
                b[x] = "X";
            else if (a[x] == 100)
                b[x] = "O";
            else if (a[x] == 0)
                b[x] = " ";
            x++;
        }

        System.out.println("+---+---+---+");
        System.out.println("| " + b[0] + " | " + b[1] + " | " + b[2] + " |");
        System.out.println("+---+---+---+");
        System.out.println("| " + b[3] + " | " + b[4] + " | " + b[5] + " |");
        System.out.println("+---+---+---+");
        System.out.println("| " + b[6] + " | " + b[7] + " | " + b[8] + " |");
        System.out.println("+---+---+---+");

    }
}

But  out of curiosity and just for educational purposes, I was wondering if there was a more advanced/shorter way of solving this problem? And by shorter I meant writing a smaller code.

You don't need array b.  Try this.

    for (int i = 0; i < 9; i += 3) {
        System.out.println("+---+---+---+");
        for (int j = i; j < i + 3; ++j)
            System.out.printf("| %s ", a[j] == 10 ? "O" : a[j] == 100 ? "X" : " ");
        System.out.println("|");
    }
    System.out.println("+---+---+---+");


Check if a JavaScript variable exists and checking its value I feel like this bit of code could be condensed or made better in some way. It may in fact not be, but figured I'd get some people to have a look. I have multiple pages on my site and in certain pages I'm setting a JavaScript variable:

var header_check = "user-profile";

Some pages I'm not setting that variable. In another JavaScript file that gets loaded on the page I check if that variable exists and perform various actions if it does.

Is this the best way to check if the variable exists? Also, is this the best way to see if user-profile is set?

var header_cookie = typeof header_check !== 'undefined' ? 'user-profile' : 'admin-profile';
var cookie_check = header_cookie == 'user-profile' ? true : false;

var city_profile = cookie_check ? 'userCity' : 'city';
var state_profile = cookie_check ? 'userState' : 'state';

First off, your code really breaks down to this logic and I find it helpful to write it out the longer version to full understand the logic flow before trying to shorten it:

var header_cookie, cookie_check, city_profile, state_profile;
if (typeof header_check !== "undefined") {
    header_cookie = 'user_profile';
    cookie_check = true;
    city_profile = 'userCity';
    state_profile = 'userState';
} else {
    header_cookie = 'admin_profile';
    cookie_check = false;
    city_profile = 'city';
    state_profile = 'state;
}

FYI, I also find this a LOT easier to follow what's actually happening than the code you have.  It also saves several comparisons on the cookie_check value.

There are some ways to shorten this, but it's not entirely clear that any are "better" where the definition of better includes readability by someone who has never seen this code before, but you can decide what you think of that issue for the alternatives:

Since you really only have two states, you could predefine each state and then just pick which one to use and access the properties off a single state object:

var userState = {
    header_cookie: 'user_profile', city_profile: 'userCity', state_profile: 'userState';
};
var adminState = {
    header_cookie: 'admin_profile', city_profile: 'city', state_profile: 'state';
};
var state = typeof header_check !== "undefined" ? userState: adminState;

Done this way, you'd access state.header_cookie, state.city_profile and state.state_profile rather than your standalone variables.

Or, if you wanted to keep the individual variables, you could do this:

var states = {
    header_cookie: ['user_profile', 'admin_profile'],
    city_profile: ['userCity', 'city'],
    state_profile: 'userState', 'state'];
};

var stateIndex = typeof header_check !== "undefined" ? 0 : 1;
var header_cookie = states.header_cookie[stateIndex];
var city_profile = states.city_profile[stateIndex];
var state_profile = state.state_profile[stateIndex];


First Python Script - Web Scraper with results saved to MySQL DB I am working on getting more experience with Python so I made a basic Web Scraper, that saves it to MySQL.

The code is currently working, but looking at it is very frustrating because it seems extremely redundant to me.

Some fake html - This does emulate the sites tree.

World

$1.99

$1.99

World2

$2.99

$2.99

World3

$3.99

$3.99

World4

$4.99

$3.99

My Python Script

from bs4 import BeautifulSoup
import urllib2
import csv
import dbconfig as cfg
origPage = 'http://localhost'
page = urllib2.urlopen(origPage)
import sys
import MySQLdb as mdb
reload(sys)
sys.setdefaultencoding('UTF8')

def pullGames():
 soup = BeautifulSoup (page, 'html.parser')
 #(page, 'html.parser')

 #Let's pull prices
 names = []
 pricing = []
 discounts = []
 discountPct = []
 #Push items to array for saving
 for name in soup.find_all('div', attrs={'class': 'tab_row'}):
  names.append(name.h4.text)
 for price in soup.find_all('div', attrs={'class': 'discount_original_price'}):
  pricing.append(price.text)
 for discount in soup.find_all('div', attrs={'class': 'discount_final_price'}):
  discounts.append(discount.text)
 for percent in soup.find_all('div', attrs={'class': 'discount_pct'}):
  discountPct.append(percent.text)

 con = mdb.connect(cfg.mysql['host'], cfg.mysql['user'], cfg.mysql['passwd'], cfg.mysql['db']);
 cur = con.cursor()
 con.set_character_set('utf8')
 cur.execute('SET NAMES utf8;')
 cur.execute('SET CHARACTER SET utf8;')
 cur.execute('SET character_set_connection=utf8;')

 for i in range(len(names)):
  sql = 'INSERT INTO game_pricing(game_name, original_price, discount_price) \
         VALUES("%s", "%s", "%s")' % \
         (names[i],pricing[i],discounts[i])
 #print sql
  cur.execute(sql)
  con.commit()

def checkDb():
 con = mdb.connect(cfg.mysql['host'], cfg.mysql['user'], cfg.mysql['passwd'], cfg.mysql['db']);
 cur = con.cursor()
 con.set_character_set('utf8')
 cur.execute('SET NAMES utf8;')
 cur.execute('SET CHARACTER SET utf8;')
 cur.execute('SET character_set_connection=utf8;')
 sql = 'SELECT game_name, original_price, discount_price FROM game_pricing'
 cur.execute(sql)
 results = cur.fetchall()
 for row in results:
  game_name = row[0]
  original_price = row[1]
  discount_price = row[2]
  print "game_name=%s, original_price=%s, discount_price=%s" % \
         (game_name, original_price, discount_price)
#For testing
checkDb()

It currently succeeds and gets 2,800 records in just a few(~15) seconds, including DB save. I am now adding in additional code to review if an entry exists, and only update if they exist.

I wrapped both parts into functions so I can easily test them out without writing duplicates.

Welcome any feedback. While this is working, I am not even sure how to begin refactoring it for redundancy sake.

Here are some notes grouped by categories.
HTML Parsing
Instead of collecting names and amounts in separate lists, iterate over the elements with a tab_row class and collect the results into a single list of dictionaries.
You can get performance improvements by switching to lxml from html.parser. Requires lxml to be installed.
You can use more concise CSS selectors instead of find_all().
You can also use SoupStrainer class that would allow to improve on parsing speed by limiting what the parsing tree to the desired part only.
Working with the database
First of all, do not use string formatting to pass your query parameters into the query. You are not only exposing your code to SQL injection attacks, but also get potential issues with character escaping and type conversions. Use parameterized queries instead.
If you are gonna collect your data into the list of dictionaries, you may do your insert via executemany() which, aside from being more concise,
may provide a performance boost as well.
It is also a good idea to use multi-line strings for your database queries.
Code Style
There are multiple code style issues. The code is not PEP8-complaint. Please analyze your code with flake8, pylint or other linters.
Also, don't repeat the database connection code, extract it into a separate function and reuse.

Here is the modified version of the code (renamed functions and logically separated into multiple functions):
import sys
import urllib2

import MySQLdb as mdb
from bs4 import BeautifulSoup, SoupStrainer

import dbconfig as cfg

def db_connect():
    connection = mdb.connect(cfg.mysql['host'], cfg.mysql['user'], cfg.mysql['passwd'], cfg.mysql['db'])
    cursor = connection.cursor()

    connection.set_character_set('utf8')
    cursor.execute('SET NAMES utf8;')
    cursor.execute('SET CHARACTER SET utf8;')
    cursor.execute('SET character_set_connection=utf8;')

    return connection, cursor

def pull_games(page):
    parse_only = SoupStrainer(class_='tab_row')
    soup = BeautifulSoup(page, 'lxml', parse_only=parse_only)

    return [
        {
            'name': row.h4.get_text(),
            'price': row.select_one(".discount_original_price").get_text(),
            'discount': row.select_one('.discount_final_price').get_text()
        }
        for row in soup.select(".tab_row")
    ]

def insert_games(connection, cursor, data):
    query = """
        INSERT INTO
          game_pricing
          (game_name, original_price, discount_price)
      VALUES
          (%(name)s, %(price)s, %(discount)s)
    """
    cursor.executemany(query, data)
    connection.commit()

def check_games(cursor):
    query = """
        SELECT
            game_name, original_price, discount_price
        FROM
            game_pricing
    """
    cursor.execute(query)

    result = cursor.fetchall()
    for game_name, original_price, discount_price in result:
        print("game_name=%s, original_price=%s, discount_price=%s" % (game_name, original_price, discount_price))

if __name__ == '__main__':
    reload(sys)
    sys.setdefaultencoding('UTF8')

    url = 'http://localhost'
    page = urllib2.urlopen(url)

    games = pull_games(page)

    try:
        connection, cursor = db_connect()
        insert_games(connection, cursor, games)

        check_games(cursor)
    finally:
        cursor.close()
        connection.close()


Retrieving the nth term of an infinite stream I recently learned how generators can be infinite in Python. For example, the infinite sequence can be the triangular numbers:
$$1, 3, 6, 10,...$$

def triangle_stream():
    """ Infinite stream of triangle numbers. """
    x = step = 1
    yield x
    while True:
        step += 1
        x += step
        yield x

I wrote a function that returns the nth term in an infinite sequence.

def nth_term(generator, n):
    """ Returns the nth term of an infinite generator. """
    for i, x in enumerate(generator()):
        if i == n - 1:
            return x

As an example:

>>> nth_term(triangle_stream, 10)
55

But this nth_term function also works:

def nth_term(generator, n):
    """ Returns the nth term of an infinite generator. """
    x = generator()
    for i in range(n - 1):
        next(x)
    return next(x)

Which of these two nth_term functions is more efficient?

Two modifications that I would make right away would be:

to use iterables instead of a generator function as parameter to allow for more genericity: this way I could use nth_term using either triangle_stream() or itertools.cycle('ABCD') as parameters;
to index elements in base 0 instead of base 1, so nth_term(triangle_stream(), 0) would return 1 instead of looping indefinitely and nth_term(triangle_stream(), 1) would return 3 instead of 1.

Now, making the function accept any iterable, I would use something along the lines of the second version this will be closer to my final proposition that will raise StopIteration if there is not enough elements instead of silently returning None ("Errors should never pass silently."). But applying both suggestions would lead to:

def nth_term(iterable, n):
    """Returns the nth term of any iterable."""
    for i, x in enumerate(iterable):
        if i == n:
            return x

or

def nth_term(iterable, n):
    """Returns the nth term of any iterable."""
    x = iter(iterable)
    for i in range(n):
        next(x)
    return next(x)

Now, instead of manually calling next on the iterator in the second version, I would ask zip to do it for me. Chances are that the execution will spend less time in the Python interpreter and more in the C part of zip and so be faster:

def nth_term(iterable, n):
    """Returns the nth term of any iterable."""
    iterator = iter(iterable)
    for _ in zip(range(n), iterator):
        pass
    return next(iterator)

Lastly, I would use existing tools to perform the iteration for me: itertools.islice is exactly what we need. In fact, your very function is already listed in the itertools recipes:

def nth(iterable, n, default=None):
    "Returns the nth item or a default value"
    return next(islice(iterable, n, None), default)

I would however remove the default value, especially in the case of an infinite iterable:

def nth(iterable, n):
    "Returns the nth item or raise StopIteration"
    return next(islice(iterable, n, None))

A completely unrelated note, but I would move the yield x at the beginning of the loop in triangle_stream so that you don't have to write it twice:

def triangle_stream():
    """ Infinite stream of triangle numbers. """
    x = step = 1
    while True:
        yield x
        step += 1
        x += step

And since step starts at 1, and never stop incrementing one by one, I would use itertools.count(1):

def triangle_stream():
    """ Infinite stream of triangle numbers. """
    x = 0
    for step in itertools.count(1):
        x += step
        yield x

Now triangle_stream only accumulates values from itertools.count(1), well time to make use of itertools.accumulate:

def triangle_stream():
    """ Infinite stream of triangle numbers. """
    return itertools.accumulate(itertools.count(1))

or, to make it more obvious that you get a generator by calling triangle_stream:

def triangle_stream():
    """ Infinite stream of triangle numbers. """
    yield from itertools.accumulate(itertools.count(1))


UVA online p957 - "Popes" This is the "Popes" problem from UVa Online Judge:

  On the occasion of Pope John Paul II's death, the American weekly magazine Time observed that the largest number of Popes to be selected in a 100-year period was 28, from 867 (Adrian II) to 965 (John XIII). This is a very interesting piece of trivia, but it would be much better to have a program to compute that number for a period of any length, not necessarily 100 years. Furthermore, the Catholic Church being an eternal institution, as far as we can predict it, we want to make sure that our program will remain valid per omnia secula seculorum.

  Write a program that given the list of years in which each Pope was elected and a positive number \$Y\$, computes the largest number of Popes that were in office in a \$Y\$-year period, and the year of election for the first and last Popes in that period. Note that, given a year \$N\$, the \$Y\$-year period that starts in year \$N\$ is the time interval from the first day of year \$N\$ to the last day of year \$N+Y−1\$. In case of a tie, that is, if there is more than one \$Y\$-year period with the same largest number of Popes, your program should report only the most ancient one.

I am a Python beginner. When I submit this code, it says "time limit exceeded". How can I optimize this code?

while True:
    try:
        str=input()
        if len(str):
            Y = int( str )
        else:
            Y = int( input() )
    except:
        break
    N_P = int( input() )
    popes= []
    for i in range (N_P):
        popes.append( int(input()) )
    Y = Y-1
    maximum = 0
    for i in range (N_P):
        j = i
        count = 0
        while(popes[j] <= popes[i] + Y):
            j = j+1
            count = count +1
            if j>= N_P:
                break
            if count>maximum:
                maximum = count
                first = popes[i]
                last = popes[j-1]
    print (maximum,first,last)

When you tackle a programming challenge like this, it's a good idea to test out your program to see if it's fast enough, before you submit it. The problem description usually tells you how big the worst case is going to be, so it's a good idea to make a test case that's as big as the worst case, and then try out your program on it.

In this case, the problem description says,

  Each of the remaining \$P\$ lines contains the year of election of a Pope, in chronological order. We know that \$P≤100000\$ and also that the last year \$L\$ in the file is such that \$L≤1000000\$, and that \$Y ≤L\$.

So there could be as many as 100,000 popes! Let's make some sample data using this program:

Y = 100                         # Years in range of interest.
P = 100000                      # Number of popes
L = 1000000                     # Maximum year.
print(Y)
print(P)
for i in range(P):
    print(i * L // P)

Let's try it:

$ python make-popes-data.py > popes.txt
$ /usr/bin/time python cr150204.py < popes.txt
10 0 90
        0.71 real         0.65 user         0.05 sys

That doesn't seem so bad — 0.71 seconds. But remember that the problem statement says that \$Y\$ might be large — perhaps as large as \$L\$. So let's increase \$Y\$ to 1,000 and try again:

$ /usr/bin/time python cr150204.py < popes.txt
100 0 990
        4.25 real         4.19 user         0.05 sys

That would already fail the 3 second time limit at UVa Online Judge. But what if \$Y\$ was even larger? Let's increase \$Y\$ to 10,000 and try again:

$ /usr/bin/time python cr150204.py < popes.txt
1000 0 9990
       45.04 real        44.70 user         0.17 sys

45 seconds! So it should be clear now what the problem is. The code in the post has two nested loops. The outer loop is over all the popes:

for i in range (N_P):

and the inner loop is over the popes that are elected less than \$Y\$ years after pope number \$i\$:

while(popes[j] <= popes[i] + Y):

So if \$Y\$ is big, then the inner loop might have to loop over a large fraction of the popes — and to do so for every pope in the outer loop. This is quadratic runtime behaviour.

You need to change the program so that it doesn't have this behaviour. But how can you do that? For this problem, the thing to do is to think about how you would solve the problem by hand. You might start with the first pope and count the popes that are elected in the next \$Y\$ years. But at that point you wouldn't throw away all your work and start again with the second pope — instead you would ... well, I don't want to spoil it for you!


Simple hack to merge RSS feeds Have I made any obvious mistakes?
Is this code clean?
Is there a better way to do this in python?

# -*- coding: utf-8 -*-
import feedparser
from feedgen.feed import FeedGenerator
from random import shuffle
from feedformatter import Feed
import time
from datetime import datetime

tstart = datetime.now()
# Set the feed/channel level properties
# ----------------------------------- #
chanTitle = 'Feed Merger'
chanLink = 'http://server.com/feed'
chanAuthor = 'Bob Dylan'
chanDescription = 'Brain Food'
# ----------------------------------- #
# Apply feed/channel level properties
# ----------------------------------- #
feed = Feed()
feed.feed["title"] = chanTitle
feed.feed["link"] = chanLink
feed.feed["author"] = chanAuthor
feed.feed["description"] = chanDescription
# ----------------------------------- #
urls = list(set(open('urls.txt', 'r').readlines()))
shuffle(urls)
extract_entries = lambda url: feedparser.parse(url).entries
addEntries = lambda entries: [feed.items.append(entry) for entry in entries]
merg = lambda urls: [addEntries(extract_entries(url)) for url in urls]
shuffle(feed.entries)
save = lambda outfile: feed.format_rss2_file(outfile)
merg(urls)
save('feed.xml')
tend = datetime.now()
runtime = tend - tstart
print "Runtime > %s" % (runtime)
print "Merged  > %d items" % (len(feed.entries))

These lines appear to be out of order:

extract_entries = lambda url: feedparser.parse(url).entries
addEntries = lambda entries: [feed.items.append(entry) for entry in entries]
merg = lambda urls: [addEntries(extract_entries(url)) for url in urls]
shuffle(feed.entries)
save = lambda outfile: feed.format_rss2_file(outfile)
merg(urls)

In particular, when that shuffle runs, the feed has no entries yet, so there's nothing to shuffle. I think you meant this way:

extract_entries = lambda url: feedparser.parse(url).entries
addEntries = lambda entries: [feed.items.append(entry) for entry in entries]
merg = lambda urls: [addEntries(extract_entries(url)) for url in urls]
merg(urls)
shuffle(feed.entries)

More importantly, I think you're overusing lambdas for no benefit at all. The flow of the logic would be a lot easier to read if you used loops instead:

urls = list(set(open('urls.txt', 'r').readlines()))
shuffle(urls)
for url in urls:
    for entry in feedparser.parse(url).entries:
        feed.items.append(entry)
shuffle(feed.entries)
feed.format_rss2_file('feed.xml')


Sorting JSON items from a site So I recently applied to this job for iOS Developer and was invited to complete the following code challenge:

As a next step, please complete this coding exercise in order to proceed with our interview process. If you do well & pass, you will be connected to a Software Engineer to discuss your exercise for an hour zoom video call. Take as much time as needed.
Problem statement:
Please write an (Android/iOS) app that retrieves the data from https://fetch-hiring.s3.amazonaws.com/hiring.json. This will return a json array of items. Using this list of items, display all the items grouped by "listId" to the UI. Sort the results first by "listId" then by "name" when displaying. Filter out any items where "name" is blank or null. The final result should be displayed to the user in an easy-to-read list."

My solution is here.
View Controller is as follows:
import UIKit

class ViewController: UIViewController, UITableViewDelegate, UITableViewDataSource {

func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
    return tableData[section].count
}

@IBOutlet weak var dataTableView: UITableView!
func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cell = tableView.dequeueReusableCell(withIdentifier: "DataViewCell", for: indexPath) as? DataCell
    cell?.textSpace.text=tableData[indexPath.section][tableIndices[indexPath.section][indexPath.row]]
    return cell!
}

func tableView(_ tableView: UITableView, titleForHeaderInSection section: Int) -> String? {
    return "List ID: "+String(section+1)
}

func numberOfSections(in tableView: UITableView) -> Int {
    return tableData.count
}

var tableData: [[Int:String]]=[[:]]
var tableIndices: [[Int]]=[[]]

override func viewDidLoad() {
    super.viewDidLoad()

    URLSession.shared.dataTask(with: URL(string: "https://fetch-hiring.s3.amazonaws.com/hiring.json")!)
                 {
                     data,response,error  in

                    let json=try? JSONSerialization.jsonObject(with: data!, options: []) as! [[String:Any]]

                    for items in json!
                    {
                        if(type(of: items["name"]!) != NSNull.self && items["name"]! as! String != "")
                        {
                            let listID=Int(String(describing: items["listId"]!))!

                            while(self.tableData.count < listID)
                            {
                                self.tableData.append([:])
                                self.tableIndices.append([])
                            }

                            self.tableIndices[listID-1].append(Int(String(describing: items["id"]!))!)
                            self.tableData[listID-1][Int(String(describing: items["id"]!))!       ]=items["name"]! as! String
                            self.tableIndices[listID-1].sort()

                        }
                    }

                     DispatchQueue.main.async {
                        self.dataTableView.delegate=self
                        self.dataTableView.dataSource=self
                        self.dataTableView.reloadData()

                     }

                 }.resume()

}

}

Two days after submitting, I got the following feedback:

The exercise featured a lot of Swift code that forced unwrapping instead of using nil safe paradigms such as let or guard, and the items displayed in the list would have benefited from a struct or class defining the object to better separate the business logic from the view controller.

Are they being too nit picky here, or is my code really that screwed up?

They're not being too nit-picky. DataTask's completion handler will return nil for data when it couldn't get the data, like when the network is offline.  Force unwrapping 'data' means you deliberately crash the application when that happens.  That would be bad.
"separate the business logic from the view controller": They expect you to make a data type to parse the JSON into, perhaps like this:
struct Item: Decodable {
    let id: Int
    let listId: Int
    let name: String?
}


Function "between" for "Everything in One Line" library This is a function to determine if a number is between x and y. It is part of a library of mine called EOL or "Everything in One Line".

#include <stdbool.h>

#ifndef _WINDOWS_H
    #include <windows.h>
#endif

bool between (WORD n, WORD x, WORD y) { return (n >= x && n <= y) ? (true) : (false); }

There's no need to use the conditional operator here. This would be enough:

bool between (WORD n, WORD x, WORD y) {
    return n >= x && n <= y;
}

Other than this, the variable names are not descriptive. It is not clear that x specifies the lower bound and y the higher bound.

bool between (WORD value, WORD low, WORD high) {
    return value >= low && value <= high;
}

Additionally, I have to question a little bit the usage for a "Everything in One Line" library. If the code that you are writing is trivial one-liners, it might be easier for other programmers to quickly re-write the actual code than to use your library. Most of the times I could use a between function, I would probably write the actual code in-line, without using an additional function for it at all. However, writing a "Everything in One Line"-library can of course be an educating experience for yourself.


How do I avoid eval in elisp? I have wrote a simple util to submit my code to a online judge site, how to avoid the evil function?

(setq url-proxy-services '(("http" . "localhost:8888")))
(setq debug-on-error t)

(require-package 'http-post-simple)
(require 'http-post-simple)
(eval-when-compile (require 'cl))

(defstruct form action (method "POST") fields)

(defstruct site url forms)

(setq poj (make-site :url "http://poj.org" :forms '(
                            (login .
                               (make-form
                                :action "login"
                                :fields `(
                                      (user_id1 . "you-name")
                                      (password1 . ,(read-passwd "input your password: "))
                                      (url . "%2F")
                                      (B1 . "login"))))
                            (submit .
                                (make-form
                                 :action "submit"
                                 :fields `(
                                       (problem_id ,(read-string "inpur problem id: "))
                                       (language ,(read-string "select language: 0:G++,1:GCC,2:Java,3:Pascal,4:C++,5:C,6:Fortan "))
                                       (source ,(with-temp-buffer
                                          (insert-file-contents (read-file-name "select source file"))(buffer-string)))
                                       (submit "Submit"))))
                            (logout .
                                (make-form
                                 :action "login?action=logout&url=%2F"
                                 :method "GET")))))

(defun http-get-simple(url)
          (let (header data status)
            (with-current-buffer
            (url-retrieve-synchronously url)
              (setq status url-http-response-status)
              (goto-char (point-min))
              (if (search-forward-regexp "^$" nil t)
              (setq header (buffer-substring (point-min) (point))
                data   (buffer-substring (1+ (point)) (point-max)))
            (setq data (buffer-string)))
              (kill-buffer (current-buffer)))
              (values data header status)))

(defun request-site(site form)
  "perform a request to site"
  (let* ((url (site-url site))
     (form (cdr (assoc form (site-forms site))))
     (form-inst (eval form))
     (result (if (equal (form-method form-inst) "POST")
             (http-post-simple (concat url "/" (form-action form-inst))
                       (form-fields form-inst))
           (http-get-simple (concat url "/" (form-action form-inst))))))
    result))

(defun poj-login()
  (interactive)
  (request-site poj 'login))

(defun poj-logout()
  (interactive)
  (request-site poj 'logout))

(defun poj-submit()
  (interactive)
  (request-site poj 'submit))

Above is my code, I have tried this

(let* ((form (cdr (assoc 'submit (site-forms poj))))
       (frm-inst (apply (car form) (cdr form)))) ; get weird result
  (eval form)) ; get proper result

And can you give me some advice about coding style?

The short answer to your question is that forms are evaluated while functions are called (using funcall or apply).

I.e., the way you structure your code (lists of forms) you are painting yourself into the eval corner.  You need to replace lists of forms with lists of functions and then you will be able to use funcall or apply.

E.g., replace

`((problem_id ,(read-string "inpur problem id: "))
  (language ,(read-string "select language: 0:G++,1:GCC,2:Java,3:Pascal,4:C++,5:C,6:Fortan "))
  (source ,(with-temp-buffer
            (insert-file-contents (read-file-name "select source file"))
            (buffer-string)))
  (submit "Submit"))

with something like

`((problem-id read-string "problem id: ")
  (language read-string "language: 0:G++,1:GCC,2:Java,3:Pascal,4:C++,5:C,6:Fortan ")
  (source ,(lambda () (file-to-string (read-file-name "source file: "))))
  (submit identity "Submit"))

Where file-to-string is defined thus:

(defun file-to-string (file)
  "Read the content of FILE and return it as a string."
  (with-temp-buffer
    (insert-file-contents file)
    (buffer-string)))

Now you can replace (eval form) with (apply (car form) (cdr form)).


Object pool with linked list of available objects implementation I was reading some gameprogrammingpatterns (awesome book, by the way, would recommend) and was very excited when I learned that unions are a thing that exists, so I decided to implement an object pool as outlined there.

I'm looking for a review of the usual culprits. Poor readability, not making use of standard library stuff/language features. But more than that, I'd love it if someone could point out how I could improve the access and removal algorithms, as well as an elegant way that I could have duplicates in the list.

What I mean by "doesn't allow duplicates", by the way, is that duplicates are considered equally, so if it's of type int, and there are the values {1, 2, 3, 3, 4, 5} in the pool, and you call removeObj(3), the first 3 that is come across will be "deleted", regardless of what you actually wanted.

#include <vector>

//Doesn't allow duplicates
//O(1) insertion
//O(n) removal
//O(n) access

template <class T>
class Pool
{
    private:

        struct Object
        {
            bool available;
            union
            {
                T data;
                Object* next;
            }state;
        };

        int size = 0;
        int numAlive = 0;

        Object* nextAvailable = nullptr;
        std::vector< Object > objs;

    public:
        Pool();
        Pool(int size);

        bool create(T newObj);
        std::vector<T*> getAlive();
        bool removeObj(T toDel);
};

template <class T>
Pool<T>::Pool()
{
}

template <class T>
Pool<T>::Pool(int size):size(size)
{
    objs = std::vector< Object >(size);

    //All are available
    for(int i = 0; i < objs.size() - 1; i++)
    {
         objs[i].state.next = &objs[i+1];
    }

    objs[objs.size()-1].state.next = nullptr; //Last one terminates list

    nextAvailable = &objs[0];
}

template <class T>
bool Pool<T>::create(T newObj)
{
    if(nextAvailable == nullptr)
    {
        return false;
    }

    Object* newO = nextAvailable;
    nextAvailable = nextAvailable->state.next;

    newO->state.data = newObj;
    newO->available = false;
    numAlive++;

    return true;
}

template <class T>
std::vector<T*> Pool<T>::getAlive()
{
    std::vector<T*> avl = std::vector<T*>(numAlive);
    int cnt = 0;
    for(auto& o : objs)
    {
        if(!o.available)
        {
            avl.at(cnt++) = &(o.state.data);
        }
    }

    return avl;
}

template <class T>
bool Pool<T>::removeObj(T toDel)
{
    for(auto& ob: objs)
    {
        if(!ob.available && ob.state.data == toDel)
        {
            ob.state.next = nextAvailable;
            ob.available = true;
            nextAvailable = &ob;
            numAlive--;
            break;
        }
    }
}

Design and architecture:

The design of your Object Pool is somewhat unusual. Since you are declaring the payload of type T by value inside the Object struct, once the vector objs is allocated, all objects will also be allocated beforehand. This is fine, if that was your intention, but will require type T to be default constructible.

Another strange thing is the bool create(T newObj); method. You are actually passing a T instance by value. So the pool will in reality only be storing a copy of an existing object. Usually, object/memory pools will keep a set of instances of something that can be recycled. Thus, I would expect the creation method of the pool to return a pointer to an existing (and possibly recycled) object:

T * Pool::create() { /* ... */ }

Possibly returning nullptr or throwing an exception if out of memory or instances to recycle.

The way you deallocate the objects is also unusual. You are again passing a copy of some value of type T to the removeObj() method, which then scans the array of Objects and if finding a match, marks it as unused. Again you are making the assumption that type T can be compared with operator ==, which is not the case for User Defined Types if you don't explicitly implement operator ==. That seems like and unnecessary constraint. In fact, your whole Object Pool seems to be just a thin wrapper over a vector. What I would have expected to see would be a method that returns a pointer to the pool so that is can be recycled. Something in the lines of:

void Pool::free(T * object) { /* ... */ }

I would like to point you to this implementation of a Pool allocator that you can refer to. This was used by the Doom 3 game. It is also based on a linked list, but it uses a list of arrays of objects. Thanks to some pointer manipulation, removal is also constant complexity. Looking at that implementation might make these things I've mentioned more clear to you.

Code review:

Now focusing on the code presented in the question:

This is a personal preference of mine, but I like to place the public section of a class first in the header file. My rationale is that people using and reading my code will care much more about the public methods of a class, so that's what they want to see first when reading the header file. Private/protected sections of a class are of interest to the programmer maintaining the code, so it makes sense that they don't need that much visibility.
You don't need to implement an empty constructor. Use the new default syntax (C++11):

// In the declaration inside the class body:
Pool() = default;

You should probably make the constructor that takes a single size parameter explicit, to avoid questionable implicit conversions:

explicit Pool(int size);

Since your class has some naked pointers as its members, defining its behavior regarding copy and assignment is very important. Read about The Rule of Three. My suggestion at first would be to just make it non-copyable by deleting the assignment operator and copy constructor. It seems like the safest and simplest option in this case:

Pool(const Pool&) = delete;
Pool& operator = (const Pool&) = delete;

The size member variable is unnecessary. std::vector has a member function with the same name: size().
Initialize the member vector objs in the constructor initialization list:

template <class T> Pool<T>::Pool(int size):size(size)
{
    objs = std::vector< Object >(size);
    ...

By doing that assignment, you are risking a duplicate initialization. It will first be initialized with the default vector constructor and then again with operator =. I'm not sure if the current generation of compilers is optimizing that or not. I'd guess not because this is a bad programming practice that is easy to fix:

 template <class T> Pool<T>::Pool(int size)
     : objs(size)
     , numAlive(0)
     , nextAvailable(nullptr)
 {
     ...
 }

Note that you could also use the uniform initialization style if you wanted (e.g. use { } instead of ( ) when calling the constructors of the member variables).
Avoid repeating types when declaring objects (remember to DRY):

std::vector<T*> avl = std::vector<T*>(numAlive);

No need to type std::vector<T*> twice here. You have two options in this case:

The "classic" style:

 std::vector<T*> avl(numAlive);

The new "auto" style from C++11:

 auto avl = std::vector<T*>{numAlive};

I personally like the first "classic" style, however, the second one is starting to be advocated by some programmers, including Herb Sutter.


Finding the number of perfect squares in an array I am posting here for feedback about the code that I wrote to count the number of perfect squares in an array.

var arr = Array(4...17)
var count = 0
for i in arr{
  if Double(i).squareRoot().rounded() == Double(i).squareRoot(){
    count += 1
    }
}
print(count)

The result from the code is as expected. However, is this the "correct" way of doing? Or is there a "better" way of doing it? I did a coding test from a tech recruiting website (platform) and although the result is as expected, but the website gave me 0% on the "correctness" criteria and 0% overall mark.

Mathematical correctness

The used approach is incorrect for very large numbers, here is an
example:

let i = 4611686014132420400
print(Double(i).squareRoot().rounded() == Double(i).squareRoot())
// true

The output is true, even if the number is not a perfect square:
\$ \sqrt{4611686014132420400} \approx 2147483646.9999999513383954536814215918 \$.

The reason is that a Double (which is a 64-bit floating point number
according to the IEEE 754
standard) has only 53 bits for the "significand", therefore precision
is lost in the conversion Double(i).

It also fails for negative numbers:

let i = -16
print(Double(i).squareRoot().rounded() == Double(i).squareRoot())
// false

because (-16.0).squareRoot() is NaN ("not a number").

But for non-negative numbers below \$ 2^{52} \$, your approach should
be correct.

Swiftyness

arr is not mutated, therefore it should be a constant (with let):

let arr = Array(4...17)

But actually it is not needed to create an array with all numbers in
the given range. You can iterate over a range directly:

let range = 4...17
for i in range { ... }

Even better, move the calculation into a separate function:

func numberOfPerfectSquares(in range: CountableClosedRange<Int>) -> Int {
    var count = 0
    for i in range {
        if Double(i).squareRoot().rounded() == Double(i).squareRoot(){
            count += 1
        }
    }
    return count
}

print(numberOfPerfectSquares(in: 4...17))

That allows you to add test cases, for example:

assert(numberOfPerfectSquares(in: 0...0) == 1)
assert(numberOfPerfectSquares(in: 100...10_000) == 91)
assert(numberOfPerfectSquares(in: 101...10_000) == 90)
assert(numberOfPerfectSquares(in: 101...9_999) == 89)
// ...

// More test cases if negative input is allowed:
assert(numberOfPerfectSquares(in: -10...10) == 9)
assert(numberOfPerfectSquares(in: -10_000...-100) == 91)
// ...

This is extremely useful to check the correctness if you changed the implementation.

Performance

Poor. Your code checks every number in the range if it is a perfect
square or not. It is far more efficient to iterate over the possible square roots:

func numberOfPerfectSquares(in range: CountableClosedRange<Int>) -> Int {
    var count = 0
    var i = Int(Double(range.lowerBound).squareRoot())
    while i * i < range.lowerBound {
        i += 1
    }
    while i * i <= range.upperBound {
        count += 1
        i += 1
    }
    return count
}

This is already much faster, but you don't need a loop at all!  It suffices to
compute the number of perfect squares below the lower resp. upper bound, and return
the difference:

func numberOfPerfectSquares(in range: CountableClosedRange<Int>) -> Int {
    // Number of perfect squares < lower bound:
    let lo = range.lowerBound == 0 ? 0 : Int(Double(range.lowerBound - 1).squareRoot()) + 1
    // Number of perfect squares <= upper bound:
    let hi = Int(Double(range.upperBound).squareRoot()) + 1
    return hi - lo
}

Note that this still does not work for negative numbers and numbers above \$ 2^{52} \$
due to the rounding errors. For the latter problem, see Computing the integer square root of large numbers.


Find the nth prime between 1 and 1,000,000 in under 3 minutes - follow-up Previous question:

Checking for the nth prime number between 1 and 1,000,000, must compute 1,000,000th in under 3 minutes

My mission is to find the nth prime number, based on user input 1-1,000,000. It also must be able to compute the 1,000,000th prime in under 3 minutes.
My code is relatively fast, however I need to make it calculate the 1,000,000th prime in 180 seconds or less. As of now it is taking 430 seconds, although before heavy updating last night it was taking about 20 minutes.

I am trying to accomplish this without importing any libraries (time's just for record keeping) and without making use of the sieve of Eratosthenes.  This is an assignment that I have been given, and I have been instructed to not use the help of either of those options.

import time
goal = int(input("Enter a number n, 1-1,000,000 to find the nth prime number."))
start = time.time()
if goal > 4:
    lst = [2, 3, 5, 7]
    chk = 11
    primes = 4
    while primes < goal:
        j = chk**.5
        for n in lst:
            if n > j:
                lst.append(chk)
                primes += 1
                break
            if chk % n == 0:
                break
        chk += 2

else:
    if goal == 1:
        print("2")
    if goal == 2:
        print("3")
    if goal == 3:
        print("5")
    if goal == 4:
        print("7")
print("Prime", str(goal), "is", str(chk - 2))
elapsed = (time.time() - start)
print(elapsed, "\n")

Trial division is entirely the wrong algorithm to use when you want to find many primes. The Sieve of Eratosthenes is much more efficient. It seems that you know that too, but have chosen a slower algorithm anyway, and now you are unhappy that it exceeds your expected running time.

Division is slower than multiplication and addition. Square root are even more work to compute. The Sieve involves neither of those expensive operations.

Nevertheless, I'll play along. If you want to do trial division, you could at least write the code better:

Eliminate special cases. You have hard-coded 2, 3, 5, 7 twice. The output format for small primes is inconsistent as well.
Eliminate redundant bookkeeping. Since primes is just the length of lst, why not use len(lst)? That frees up the variable name primes, which would be a more appropriate name for the list itself.
Name your variables better. Use candidate instead of chk. Since n is always prime, call it p instead.

import time
goal = int(input("Enter a number n, 1-1,000,000 to find the nth prime number."))
start = time.time()

primes = [2, 3, 5, 7]
candidate = 11
while goal >= len(primes):
    limit = candidate ** 0.5
    for p in primes:
        if p > limit:
            primes.append(candidate)
            break
        if candidate % p == 0:
            break
    candidate += 2

print("Prime", str(goal), "is", str(primes[goal - 1]))
elapsed = (time.time() - start)
print(elapsed, "\n")

This isn't going to change the performance, but at least you would have more readable code, and less of it to manage.


Flattening a 2D array to use map/reduce I have been trying to get better at functional programming, which includes understanding map/reduce. I wrote a function intended to fill an autocomplete with values. The challenge I was encountering was my Mongo records, which had a category array that needed to be flattened for the autocomplete to work. Being a novice, I somehow came up with this code, and it works. But I know there must be a better way to do it using map/reduce.

You will see cats stores a call that uses .map, but it's the code from the package example and I have no idea what it does.

      categoryTypeAheadVals: function() {
    cats = Todos.find({}, {categories: 1}).fetch().map(function(it){ return it.categories; });
    y = [];
    var x = cats.forEach(function(it) {
      it.forEach(function(it){
        y.push(it);
      });
    });
    return y;
  }

});

New

This code can actually be further reduced by using JavaScript's apply:

y = y.concat.apply(y, cats);

The above line will merge all the arrays inside the cats array into a single array and then copy it into y.

This is better than the solution you already have because:

It is faster. No more looping over two arrays.
It is easier to read. If you want, just slap a comment on this line and it is very easy to understand.

Old

it.forEach(function(it){
    y.push(it);
});

This section is over-done. All you are doing is copying the array it into y, and JavaScript already has a built in function for that: Array.prototype.concat.

You can use this function to copy one array into another.

Now, your code becomes this:

var x = cats.forEach(function(it) {
    y = y.concat(it);
});


Implementation of an asynchronous UDP server Listener I have written this code in C# and have tested this code to ensure it works well. Any other suggestions for this code would be appreciated as I have a number of users who are sending data on this port.

private Socket serverSocket = null;
private List<EndPoint> clientList = new List<EndPoint>();
private byte[] byteData = new byte[1024];
private int port = 1517;

private void button1_Click(object sender, EventArgs e)
{
    this.serverSocket = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);
    this.serverSocket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
    this.serverSocket.Bind(new IPEndPoint(IPAddress.Any, this.port));
    EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
    this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);

}

private void DoReceiveFrom(IAsyncResult iar)
{

    try
    {
        EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
        int dataLen = 0;
        byte[] data = null;
        try
        {
            dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
            data = new byte[dataLen];
            Array.Copy(this.byteData, data, dataLen);
        }
        catch (Exception e)
        {
        }
        finally
        {
            EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
            this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
        }

        if (!this.clientList.Any(client => client.Equals(clientEP)))
            this.clientList.Add(clientEP);

        //     DataList.Add(Tuple.Create(clientEP, data));

        string s = Encoding.ASCII.GetString(data);

    }
    catch (ObjectDisposedException)
    {
    }
}

public void Stop()
{
    this.serverSocket.Close();
    this.serverSocket = null;
    this.clientList.Clear();
}

@MX D is correct in that you should not be catching exceptions, unless you are going to do something about them.

This code can and should be written without catch statements.

private void DoReceiveFrom(IAsyncResult iar)
{

    try
    {
        EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
        int dataLen = 0;
        byte[] data = null;
        try
        {
            dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
            data = new byte[dataLen];
            Array.Copy(this.byteData, data, dataLen);
        }
        catch (Exception e)
        {
        }
        finally
        {
            EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
            this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
        }

        if (!this.clientList.Any(client => client.Equals(clientEP)))
            this.clientList.Add(clientEP);

        //     DataList.Add(Tuple.Create(clientEP, data));

        string s = Encoding.ASCII.GetString(data);

    }
    catch (ObjectDisposedException)
    {
    }
}

you should also remove some of the empty space from the code as well.

The Commented code...what is it there for?  If this is dead code then you should remove it.

Declaring a Variable at the end of this code has me a little confused....

  string s = Encoding.ASCII.GetString(data);

Get rid of that too, it isn't used anywhere in the code that you present in the Original Post

I also got rid of the Try because there isn't a need for a try if you aren't going to catch anything or finally anything.  This also removes a level of unnecessary indentation.

The code now looks like this:

private void DoReceiveFrom(IAsyncResult iar)
{
    EndPoint clientEP = new IPEndPoint(IPAddress.Any, 0);
    int dataLen = 0;
    byte[] data = null;
    try
    {
        dataLen = this.serverSocket.EndReceiveFrom(iar, ref clientEP);
        data = new byte[dataLen];
        Array.Copy(this.byteData, data, dataLen);
    }
    finally
    {
        EndPoint newClientEP = new IPEndPoint(IPAddress.Any, 0);
        this.serverSocket.BeginReceiveFrom(this.byteData, 0, this.byteData.Length, SocketFlags.None, ref newClientEP, DoReceiveFrom, newClientEP);
    }

    if (!this.clientList.Any(client => client.Equals(clientEP)))
        this.clientList.Add(clientEP);
}

and while I am at it, I would implement the IDisposable interface and use a Dispose method instead of this

public void Stop()
{
    this.serverSocket.Close();
    this.serverSocket = null;
    this.clientList.Clear();
}

it would then look something like this

public void Dispose()
{
     this.serverSocket.Close();
     this.serverSocket = null;
     this.clientList.Clear();
}

this also gives you other options when using the class in your code.