Checking search buttons on click There are 3 search fields, and at least one must be filled out. When the search button is clicked the search fields are checked, if none are filled in show the error window with the error message for this error. If at least one field is filled in get a code based on the search criteria. If the code was not successfully retrieved show same error window but with the error message for this error.

The code I have works fine for this except that it looks verbose and I am repeated the code in the else statements except for the unique error messages. How can this be improved?

private void searchButton_Click(object sender, RoutedEventArgs e)
{
    searchCriteria.Clear();
    searchCriteria.Add(townSearchTextbox.Text);
    searchCriteria.Add(countySearchTextbox.Text);
    searchCriteria.Add(postcodeSearchTextbox.Text);

    if (isValidSearchCriteria(searchCriteria))
    {
        formatSearchCriteria(searchCriteria);
        set.updateWOEID(searchCriteria);
        if (set.WOEID.Length > 0)
        {
            // continue
        }
        else
        {
            // display any errors in the error window
            Error errorWindow = new Error();
            errorWindow.Show();
            errorWindow.errorMessage.Text = "Error: Could not retrieve WOEID, please try again.";
        }
    }
    else
    {
        // display any errors in the error window
        Error errorWindow = new Error();
        errorWindow.Show();
        errorWindow.errorMessage.Text = "Error: Please provide one or more search criteria.";
    }
}

Create a method:

private void DisplayError(String errorMessage)
{
    // display any errors in the error window
    Error errorWindow = new Error();
    errorWindow.Show();
    errorWindow.errorMessage.Text = errorMessage;
}

Then you can do

private void searchButton_Click(object sender, RoutedEventArgs e)
{
    searchCriteria.Clear();
    searchCriteria.Add(townSearchTextbox.Text);
    searchCriteria.Add(countySearchTextbox.Text);
    searchCriteria.Add(postcodeSearchTextbox.Text);

    if (isValidSearchCriteria(searchCriteria))
    {
        formatSearchCriteria(searchCriteria);
        set.updateWOEID(searchCriteria);
        if (set.WOEID.Length > 0)
        {
            // continue
        }
        else
        {
            DisplayError("Error: Could not retrieve WOEID, please try again.");
        }
    }
    else
    {
        DisplayError("Error: Please provide one or more search criteria.");
    }
}

Of course, the next step if you need it would be to use Exceptions instead.  Yes, I know this is for Java, but it shows why.


BFS Implementation in Python 3 def bfs(graph, root):
    visited, queue = [], [root]
    while queue:
        vertex = queue.pop(0)
        for w in graph[vertex]:
            if w not in visited:
                visited.append(w)
                queue.append(w)

graph = {0: [1, 2], 1: [2], 2: []}
bfs(graph, 0)

Does this look like a correct implementation of BFS in Python 3?

sets perform containing checks (w in visited) \$O(1)\$ rather than \$O(n)\$ for lists.
collections.deque are better than lists for poping elements at the front (popleft).
you should put your example code under an if __name__ == '__main__' clause.
w as a variable name does not convey meaning, you should try to come up with something more explicit.

import collections

def breadth_first_search(graph, root):
    visited, queue = set(), collections.deque([root])
    while queue:
        vertex = queue.popleft()
        for neighbour in graph[vertex]:
            if neighbour not in visited:
                visited.add(neighbour)
                queue.append(neighbour)

if __name__ == '__main__':
    graph = {0: [1, 2], 1: [2], 2: []}
    breadth_first_search(graph, 0)

Given a growing number of comments indicating that the code does not return anything, I’d like to add that, yes, this code does not process nodes: it only traverse the graph and you're likely to want to add your own custom logic to process each node. As your mileage may vary (building a traversal list, finding the first node that satisfies a condition, etc.), there is not a "one code fits all" approach, but a useful first approximation would be to yield each node as they are traversed:

import collections

def breadth_first_search(graph, root):
    visited, queue = set(), collections.deque([root])
    while queue:
        vertex = queue.popleft()
        yield vertex
        visited.add(vertex)
        queue.extend(n for n in graph[vertex] if n not in visited)

if __name__ == '__main__':
    graph = {1: [2, 4, 5], 2: [3, 6, 7], 3: [], 4: [], 5: [], 6: [], 7: []}
    list(breadth_first_search(graph, 1))  # [1, 2, 4, 5, 3, 6, 7]

Note that this alternative iteration also takes care of the bug mentioned in this answer


Deep cloning and modifying containers from ReactJS this.state I am presently working on refactoring ReactJS code away from directly manipulating this.state and towards using this.setState() only (which I should have been doing from the beginning).

The contents of this.state in my application include containers with varying numbers of objects. They are entirely JSON-serializable, but I realized I found myself doing something a lot that I don't recall seeing in the ReactJS documentation (hence the request for a code review). I'm interested in knowing, besides general code quality, how close it is to ReactJS idiomatic usage and how it might be moved closer.

The clone() method I have now is intended to provide a deep clone of JSON-serializable objects, without guarantees as to objects that can't be serialized:

var clone = function(original) {
    if (typeof original === 'undefined' ||
      original === null ||
      typeof original === 'boolean' ||
      typeof original === 'number' ||
      typeof original === 'string') {
      return original;
    }
    if (Object.prototype.toString.call(original) === '[object Array]') {
      var result = [];
      for(var index = 0; index &lt; original.length; index += 1) {
        result[index] = clone(original[index]);
      }
    } else {
      for(var current in original) {
        var result = {};
        if (original.hasOwnProperty(current)) {
          result[current] = original[current];
        }
      }
    }
    if (typeof original.prototype !== undefined) {
      result.prototype = original.prototype;
    }
    return result;
}

What I find myself doing repeatedly is of the form (here using classical form Hijaxing to add a new entry):

handle_submit: function(event) {
      event.preventDefault();
      var entry_being_added = clone(this.state.entry_being_added);
      entry_being_added.month = parseInt(jQuery('#month').val());
      entry_being_added.date = parseInt(jQuery('#date').val());
      entry_being_added.year = parseInt(jQuery('#year').val());
      if (jQuery('#all_day').is(':checked')) {
        entry_being_added.all_day = true;
      } else {
        entry_being_added.all_day = false;
      }
      // Additional assignments omitted for the sake of brevity.
      var entries = clone(this.state.entries);
      entries.push(entry_being_added);
      this.setState({'entries': entries});

I'm not sure I've seen this approach in the ReactJS documentation, and I wanted to ask for a sanity check at least. Is it idiomatic to clone container types, modify the clone, and call this.setState() on the modified clone? Am I deviating from idiom in another way, perhaps by having container types in this.state? Or is this spot on?

React is all about avoiding side-effects as well as rendering the same state exactly the same every time. You'd want to avoid operations that mutate the existing data structure like push or object assignment.
I'm pretty sure your clone is to prevent your object from being mutated. Instead of clone, create a function that returns your object structure. Just pass in anything that's configurable.
Instead of mutating the entries array with push, create a new array with your new data added using concat.
Use refs to fetch values from inputs instead of jQuery.

Your code could have been summarized into something like this:

function createNewEntry(month, date, year, allDay){
  return {
    month: month,
    date: date,
    year: year,
    allDay: allDay
  };
}

handleSubmit: function(event){
  var month = React.findDOMNode(this.refs.month).value;
  var date = React.findDOMNode(this.refs.date).value;
  var year = React.findDOMNode(this.refs.year).value;
  var allDay = React.findDOMNode(this.refs.allDay).value;
  var newEntry = createNewEntry(month, date, year, allDay);
  var entries = this.state.entries.concat(newEntry);

  this.setState({'entries': entries});
}

If you happen to have interest in persistent data structures (immutable data), then have a look at immutable.js. It's recommended in cases where you don't want to unwantedly mutate data.


Is there a way to avoid calling the document with jQuery on click? I have taken a look at several answered questions on Stack Overflow about jQuery's new (since 1.9.1 upgrade)

.on('click','',function(){}

and that you need to delegate. However, in each one, it just provides the solution to fix the issue, but I need a bit more finest or an explanation to why it must be done like that.

Here is my code segment which I have already made it working.  My question will follow after the code.

jQuery(document).ready(function($){

    /* Shopping Cart Items */
    $(".header-shopping-cart div .toggle-items").on('click', function(){ // Show/Hide Shopping Cart Items
        if ($(".header-shopping-cart-items").css("display") == "none" && $("#items-amount").html() > 0) {
            $(".header-shopping-cart-items").fadeIn();
            $(this).find("i").removeClass("icon-angle-right").addClass("icon-angle-down");
        }
        else {
            $(".header-shopping-cart-items").fadeOut();
            $(this).find("i").removeClass("icon-angle-down").addClass("icon-angle-right");
        }
        return false;
    });

    $(document).on('click','.header-shopping-cart-items .item a', function(){
        var amount = $("#items-amount").html();
        if (amount > 0) {
            $("#items-amount").html(amount - 1); // Dicrease the amount of SC items
            $(this).parent().slideUp(); // Hide this item
            if (amount - 1 == 0) {
                $(".header-shopping-cart-items").fadeOut(); // When there's no any items in the cart, hide the items block
                $(".header-shopping-cart div .toggle-items i").fadeOut();
            }
        }
        return false;
    });
});

The short end of the code basically have two purpose: one the toggle makes it so that when press a drop down opens up.

The second one (which is where I have confusion on) enables a way to remove (or delete) a row from the drop down.

My questions:

When the HTML used to be static, I can do both the operation of toggling the HTML and the deletion of the row without issues. However when I now append the row, I could no longer remove the row by hitting the second on('click'). So I had to modify it with $(document) and then shift the .header-shopping-cart-items .item a into the delegation position. I don't know why it is that.
Must I use the document as the starting point of the delegation? Is it because these rows are not statically there at first and were actually generated later via an event.
If I want to omit

jQuery(document).ready(function($){
//code
});

I should have done IIFE instead - however is there a "trap" in doing so. Trap as in must I start changing the code structure around in order to achieve the same effect.

Please note that I read this in order to fix my JavaScript up to achieve the current code which works.

I'm not sure this question really fits into the site but I'm going to answer it anyway.

Since the elements don't exist when the page is loaded you can't add a click event to it (yet).  If you try to attach the event handler directly to the elements at this time it won't do anything since the elements don't exist.  So instead you attach the click element to the document element (which does exist) and then when the click handler is fired (with a click anywhere on the document) it looks to see if the click event was done on an object that matched the selector you entered at that point.  If it was then the click handler is fired.

That was confusing even to me.  Let me try a different way.

Option 1

Attempt to attach the event handler to the actual element on page load using the selector.

$('.class-name').on('click', function(){})

The element does not exist yet so this does nothing.

Option 2

Attach the event handler to a static element and provide a delegated selector to test for when the item is clicked.

$(document).on('click','.class-name', function(){})

This event handler is successfully added to the document object.  Now when any click happens within the document, the selector is tested and if a match the function is fired.
No, but you need to use a static element.  I used the document but it could be any static element as long as all dynamic elements will be contained within.  In your case it might be something like $('.header-shopping-cart-items').on('click', '.item a', function() {}) provided the .header-shopping-cart-items container were static (always there).

Another option, although much more difficult and probably not worth the effort would be to attach the event handler directly to the item row when you inject it.  I wouldn't recommend that approach.
You could look into putting this into a module and calling that module when needed.  But I don't know that you would be gaining anything by putting this into a IIFE.   And I wouldn't say that IIFEs are preferred over the jQuery.ready function.  That said, this whole code could probably be put into a IIFE without any changes.  You would just need to put it at the bottom of the document.


Simple bcrypt Register User and User Login using PHP - is this approach acceptable? I am learning PHP and have been looking into a suitable way to safely store password data in MySQL.

Following advice from here (https://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php), is this an acceptable way to deal with passwords?

The code is a very basic example and I haven't included things like error checking / checking if user already exists etc. to keep my example concise.

New Register User:

$fld_email =    $_POST['fld_email'];
$fld_name =     $_POST['fld_name'];
$fld_pwd =      $_POST['fld_pwd'];

$hashToStoreInDb = password_hash($fld_pwd, PASSWORD_BCRYPT, array("cost" => 11));

  $sql = "INSERT INTO tbl_a_users (fld_email
                                 , fld_name
                                 , fld_pwd
                                 , fld_date)
                           VALUES (:fld_email
                                 , :fld_name
                                 , :fld_pwd
                                 , now())";

$stmt = $pdo->prepare($sql);

$stmt->bindParam(':fld_email', $fld_email);
$stmt->bindParam(':fld_name', $fld_name);
$stmt->bindParam(':fld_pwd', $hashToStoreInDb);

$stmt->execute();

New Process Login:

$fld_email    = $_POST['fld_email'];
$fld_pwd_form = $_POST['fld_pwd'];

$stmt = $pdo->prepare('SELECT fld_pwd FROM tbl_a_users WHERE fld_email = :email LIMIT 1');
$stmt->bindParam(':email', $fld_email);
$stmt->execute();
$row = $stmt->fetch(PDO::FETCH_OBJ);

$isPasswordCorrect = password_verify($fld_pwd_form, $row->fld_pwd);

if ($isPasswordCorrect == true) {
    // do something
} else {
    // do something else
}

There is no error message here, but I wanted to check with experts about whether this is an acceptable approach before I continue work on this area.

I will also be using HTTPS for the login / register pages.

Yes, your approach to hashing and storing passwords is acceptable.

Misc

== true is not needed, and should never be used. Either directly write if (password_verify(...)) {...}, or use === (if you don't trust ==, which you shouldn't in many situations[*]).
you should generally try to be consistent with your variable naming schemes. Either use camelCase or snake_case, don't mix them.
I appreciate that you are mostly consistent with your naming overall, as it really helps readability. :email is the exception here, so you should change it.
fld_pwd_form also does not fit your general naming scheme. Either append _form to all values that are input, or to none (and think about changing it to _input).
Is there a reason for the fld_ prefix? It reduces readability, so if you can still change your database names, I would remove it.
hash is enough as a name, you don't need ToStoreInDb.

[*] == should be ok for password_verify, as it is very unlikely that the function will be changed to return anything else than true or false in the future. On the other hand, it is PHP, so who knows. And "1", 1, -1, ["error"], "error" are all true.


Merge sort in JavaScript I implemented this merge sort in JS and noticed that for random integer numbers it's a lot faster than the build in sort functions of all the browsers I tested (Chrome, FF, IE).
Does anyone have an idea why that is? Also, is there a way to make my code even faster?

function bubbleSort(f, a, start, end) {
    end--;
    while(end > start) {
        for(var i = start; i < end; i++) {
            var x = a[i], y = a[i+1];
            if(f(x, y) > 0) {a[i] = y; a[i+1] = x;}
        }
        end--;
    }
}

function merge(f, a, start, a2, start2, mid, end) {
    var i = start, i1 = start2, i2 = mid;
    while(i1 < mid && i2 < end) {
        if(f(a2[i1], a2[i2]) <= 0) a[i++] = a2[i1++];
        else a[i++] = a2[i2++];
    }
    if(i1 >= mid) {
        i1 = i2;
        mid = end;
    }
    while(i1 < mid) {
        a[i++] = a2[i1++];
    }
};

function _mergeSort(f, a1, a2, start, end) {
    var mid;
    if(end - start < 2) return;
    if(end - start < 8) { bubbleSort(f, a1, start, end); return; }
    mid = Math.floor((start + end)/2);
    _mergeSort(f, a2, a1, start, mid);
    _mergeSort(f, a2, a1, mid, end);
    merge(f, a1, start, a2, start, mid, end);
};

function mergeSort(f, a) {
    var a2 = a.slice();
    _mergeSort(f, a, a2, 0, a.length);
};

Here is some JSFiddle site to test the code
http://jsfiddle.net/mxbppLu0/
In Chrome I get the biggest difference (1.4 vs. 3 seconds for 10 million numbers)

You define the comparison function like this:

function(a, b) { return a - b }

and then you use it like this:

if (f(x, y) <= 0) { ... }
if (f(x, y) > 0) { ... }

This is a bit tedious. It would be more intuitive like this:

function(a, b) { return a <= b }

and use it like this:

if (f(x, y)) { ... }
if (!f(x, y)) { ... }

Why is it faster?

I don't know, but as @Flambino commented:

  I browsed around the V8 source code and the sort function does a lot of stuff, namely handling sorting of non-array objects, arrays with "holes", and similar cases. All of that before actually sorting anything. I have no idea what exactly is causing the slowdown, but, as mentioned, there's just a lot of stuff going on there. Their implementation is also sorting in-place, which could be a contributing factor. But I honestly haven't dug that deeply

How to make it faster?

Changing the comparison function like this:

function(a, b) { return a < b }

and the bubble sort condition to use 6 instead of 8 (in if (end - start < 6) { ...), together, often yielded faster result in my test runs, but not always, and I don't think that means anything.

The bottom line: I don't know...

Naming and formatting

The code would be more readable if you improved some of the names. For example renaming a and a2 to part1 and part2, respectively. Or even arr1 and arr2. It would make them look more like arrays, instantly.

This may be a matter of taste, but instead of this:

var x = a[i], y = a[i+1];
if(f(x, y) > 0) {a[i] = y; a[i+1] = x;}

I would find this slightly more readable:

var x = a[i];
var y = a[i + 1];
if (f(x, y) > 0) {
    a[i] = y;
    a[i + 1] = x;
}


Scanning an int variable I have attemped to define a function which scan an int value in a specified range and returns that value. The lower bound is the lower of the two parameters, the upper bound is the larger of the two parameters.After invalid input, the function displays a message and repeats input (and message, if necessary ) until a valid value is entered, which is then returned.

public static int scanYear() {

    do{
    int year = TextIO.getInt();
    if (checkDate (year) == true)
        return year;
    else
        TextIO.put("year not in the allowed range");

    }while (true);

} // end of scanYear

What is your opinion about the efficiency of the code , what can be improved ? Many thanks.

When it comes to user-input, efficiency really is not the point.... what does it matter if it takes 4 milliseconds to validate some data, or 4 nanoseconds? The user component of time dwarfs the rest.... Obvioulsy, if the code was taking more than 10 milliseconds I would be concerned, but I doubt that is the case.

With user-interface coding though, you have to think differently. In this case, you have to help the user. Specifically, telling a user that their input is wrong is more frustrating if you don't tell the user how to correct it....

Saying: " year not in the allowed range" is going to lead to lots of frustration.

Saying: "year must be between 1950 and 2013" is much better.

EDIT: In fact, when prompting the user to enter the data, it should inform the user what values will be valid before getting the value:

Please enter a year between 1950 and 2013 (inclusive):
...
Sorry, the value 2014 is not between 1950 and 2013, please enter a valid year:


Exception handling and releasing of OracleDB connection Before I start implementing several REST API service endpoints, I'd like to make sure that I'm not doing it totally wrong.

/**
 * GET /api/test
 * Test API example.
 */
export let getTest = (req: Request, res: Response, next: NextFunction) => {
    // const token = req.user.tokens.find((token: any) => token.kind === "facebook");
    const db = new Database();

    db.getConnection()
        .then(con => {
            con.execute('SELECT * FROM BTO_MDS.AlARMS', {}, { maxRows: 22000 })
                .then(result => {
                    con.release();
                    res.json({data: result.rows});
                })
                .catch(ex => {
                    con.release();
                    res.status(500).json({message: ex.message});
                });
        })
        .catch(ex => {
            res.status(500).json({message: ex.message});
        });

};

Is the connection always released properly, or have I forgotten something?
What seems a bit messy is that I need to release the connection two times, and that I have two catch'es, since I cannot release the connection in the second catch.

For completeness: Database is simply a wrapper that contains the connection string configuration and returns a reference to a new connection:

import * as oracledb from 'oracledb';
import { IConnectionPool } from 'oracledb';

const config = {
    user: 'user',
    password: 'pw',
    connectString: 'databaseNameFromTNSNames',
    poolMax: 16
};

export default class Database {
    private pool: IConnectionPool;
    constructor() {
    }

    public createPool(): void {
        oracledb.createPool(config).then(conpool => {
                this.pool = conpool;
                console.log('Connection Pool created!');
            },
            err => {
                console.log('Error creating pool!');
            });
    }

    public getConnection() {
        return oracledb.getConnection();
    }
}

There really isn't too much to say about your posted code.

Answering your specific question:

Yes, the connection will always be released as either your then or your catch method will always run. However, if res.json({data: result.rows}); (or more complex code once you have developed further) throws, your code will try to release the connection twice. This will likely result in an uncaught promise rejection. You can solve this problem a couple different ways.

Use the second callback parameter of the .then method.

con.execute('SELECT * FROM BTO_MDS.AlARMS', {}, { maxRows: 22000 })
    .then(result => {
        con.release();
        res.json({data: result.rows});
    }, ex => {
        con.release();
        res.status(500).json({message: ex.message});
    })

Use async / await and take advantage of the finally in try..catch..finally chain. This solution also has the benefit of avoiding the duplicate code sending error messages. I like this solution the best, but it would probably be even better to have an execute method on your Database class that handles releasing the connection for you.

export let getTest = async (req: Request, res: Response, next: NextFunction) => {
    const db = new Database();

    try {
        const con = await db.getConnection();
        try {
            const result = await con.execute('SELECT * FROM BTO_MDS.AlARMS', {}, { maxRows: 22000 });
            res.json({ data: result.rows });
        } finally {
            con.release();
        }
    } catch (ex) {
        res.status(500).json({ message: ex.message });
    }
};

For the future: Promise.prototype.finally is coming, currently a stage 3 proposal. Proposal. I wouldn't use this just yet.

db.getConnection()
    .then(con => {
        return con.execute('SELECT * FROM BTO_MDS.AlARMS', {}, { maxRows: 22000 })
            .then(result => {
                res.json({ data: result.rows });
            })
            .finally(() => {
                con.release();
            })
    })
    .catch(ex => {
        res.status(500).json({ message: ex.message });
    });

The trick to using promises effectively is to return them whenever possible. The original function can be somewhat simplified by re-throwing the error and returning the connection promise. This doesn't help a ton but it does remove the duplicate res.status(500) line, making it easier to change in the future if necessary.

db.getConnection()
    .then(con => {
        return con.execute('SELECT * FROM BTO_MDS.AlARMS', {}, { maxRows: 22000 })
            .then(result => {
                con.release();
                res.json({ data: result.rows });
            }, ex => {
                con.release();
                throw ex;
            })
    })
    .catch(ex => {
        res.status(500).json({ message: ex.message });
    });


Simplifying calculation with min value in C# We have to calculate a lot of different fixed factors with user input values.
Every single calculation has a different min value.

This block of code

result.Calculation_VK = input.km_past_year * 0.040m;
if (result.Calculation_VK <= 300)
    result.Calculation_VK = 300;
result.Calculation_VK *= Constant.MWST;

Is repeated over and over again with different constant factors and input variables. My question is, is it possible to simplify the lines

if (result.Calculation_VK <= 300)
    result.Calculation_VK = 300;

while still keeping the code readable?

We tried working with the ? Operator, but this doens't really clean up the code. Especially considering that these lines will be repeated over and over again.

result.Calculation_VK = input.km_past_year * 0.040m <= 300 ? 300 : result.Calculation_VK * 0.040m;
result.Calculation_VK *= Constant.MWST;

I thought about creating properties with default values inside the getters. But as we are using common and reusable objects that are shared over several classes and methods it would lead to creating objects for each single method.

For my understanding this would nullify the advantage of using objects to pass needed values into our calculations.

Well, the most simple way would be to use Math.Max() like so

result.Calculation_VK = Math.Max(input.km_past_year * 0.040m, 300) * Constant.MWST

which could be as well in a method like

public static decimal CalculateVK(decimal pastYear, decimal factor, decimal minValue)
{
    return Math.Max(pastYear * factor, minValue) * Constant.MWST;
}

and be called like

result.Calculation_VK = CalculateVK(input.km_past_year, 0.040m, 300);


Moving or copying images from a flat directory to subdirectories I have a huge Google Photos directory that is difficult to navigate - there are just too many files so I thougt I need to sort them somehow. I decided to make it simple and group them by date-taken. As I always wanted to learn python I thought I'll give it a try so this is my first take on it.

There is no rocket-science here (yet). It looks for files in the pictures_path, groups them by their modified date, enumerates this iterator, creates directories and moves or copies files according to the whatif option. This is measured by the perf_counter().

I'd be great if you could take a look at it and tell me whether I did something terribly wrong as a beginner before I let it work with my actual collection.

import os
import time
import itertools
import shutil
from pprint import pprint

def format_filemtime(path):
    filemtime = os.path.getmtime(path)
    return time.strftime('%Y-%m-%d', time.gmtime(filemtime))

def group_pictures(whatif=False):
    pictures_path = "C:\\temp\\picturepy\\pictures\\"
    galleries_path = "C:\\temp\\picturepy\\galleries\\"

    start = time.perf_counter()

    picture_names = os.listdir(pictures_path)

    print(f"picture count: {len(picture_names)}")

    pictures_by_mtime = itertools.groupby(picture_names, lambda name: format_filemtime(os.path.join(pictures_path,name)))

    for (dir, picture_names) in pictures_by_mtime:
        path_dir = os.path.join(galleries_path, dir)
        if not os.path.exists(path_dir):
            os.makedirs(path_dir)
            print(f"'{dir}' created.")
        else:
            print(f"'{dir}' already exists.")

        do = shutil.copyfile if whatif else shutil.move
        verb = "copied" if whatif else "moved"

        for file in picture_names:
            do(
                src=os.path.join(pictures_path, file),
                dst=os.path.join(path_dir, file))
            print(f"\t'{file}' {verb}.")

    end = time.perf_counter()
    elapsed = round(end - start,2)
    print(f"elapsed: {elapsed} sec")

# --- --- ---

def main():
    group_pictures(whatif=True)
    #group_pictures()

if __name__ == '__main__':
    main()

I tested it with VSCode and it's doing its job pretty well.

This looks pretty decent for a first Python program, so good job on that.

I only have a few small nitpicks

Some PEP8 violations, but nothing major

There should be a <space> after the ,
Line too long (max 79)

Consider making the directory paths global constants

PEP8 reference

PICTURES_PATH = "C:\\temp\\picturepy\\pictures\\"
GALLERIES_PATH = "C:\\temp\\picturepy\\galleries\\"

I think whatif could have a better name

What if what? It's not really clear what this variable is supposed to do

Maybe rename it to keep_original, because the copy will leave the original intact
Possible to do the if else in one line with unpacking

do = shutil.copyfile if whatif else shutil.move
verb = "copied" if whatif else "moved"

Would become

do, verb = (shutil.copyfile, "copied") if keep_original else (shutil.move, "moved")


Dynamically adding and removing HTML classes I am trying to add an active class to my subnavigation using the waypoints jQuery. I am trying to avoid writing multiple functions to adhere to the DRY idea. The waypoints toggle the function based on their location in the markup. Any suggestions welcome.

Markup

<?php include 'partials/homepage/topHero.php'; ?>

<?php include 'partials/homepage/section1.php'; ?>

JavaScript/jQuery

Backwards way:

$('.homeNavItem1').waypoint(function(direction) {
  $('.homeAnchor1').addClass("subActive");
  $('.homeAnchor2').removeClass("subActive");
  $('.homeAnchor3').removeClass("subActive");
  $('.homeAnchor4').removeClass("subActive");
  $('.homeAnchor5').removeClass("subActive");
});

Efficient code attempt:

function waypointItem(x, y, i) {
  $('.homeNavItem' + x).waypoint(function(direction){
    $('.homeAnchor' + y).addClass("subActive");
    $('.homeAnchor' + i).removeClass("subActive");
  });
}

for (i = 2; i <= 6; i ++){
  waypointItem(1, 1, i);
  console.log(i);
}

A common question,

@elclanrs is right that you probably want a common class for your home anchor tags unless you truly want to style them all differently.

Then you could simply turn off the current active anchor with

$('.homeAnchor.subActive').removeClass("subActive");

Otherwise, you could try and see if this works for you:

$('.subActive').removeClass("subActive");

That assumes that on the whole page you use subActive only for your home anchors of course.


Counting users by the type in their profile I'm creating a django web application and I would like to know how to shorten this code.

I have a model where every user has a typeOfPerson (Student, Teacher, Guest ...) and I want to count these types.

@register.simple_tag()
def get_stats_type(choice):
    items = []
    for vote in choice.vote_set.all():
        if vote.user not in items:
            items.append(vote.user)

    types = {}
    for user in items:
        if user.userprofile.typeOfPerson not in types:
            types[user.userprofile.typeOfPerson] = Vote.objects.filter(Choice=choice).count()

    percent = 0
    for type, value in types.items():
        percent += value
    if percent == 0:
        percent = 1

    values = {}
    for type, value in types.items():
        if type not in values:
            values[type] = value / percent * 100

    return values

I'm creating a django

You probably mean that you're creating a web application on top of Django.

Given this code:

items = []
for vote in choice.vote_set.all():
    if vote.user not in items:
        items.append(vote.user)

you should instead be able to do

items = set(v.user for v in choice.vote_set.all())

This achieves the same goal - creating a collection of unique entries. The only catch is that order will not be preserved. If that's important, you'll have to do something more clever.

This:

types = {}
for user in items:
    if user.userprofile.typeOfPerson not in types:
        types[user.userprofile.typeOfPerson] = Vote.objects.filter(Choice=choice).count()

apparently makes an effort to avoid overwriting entries in a dictionary, but I'm not clear on whether that's necessary. If order doesn't matter, and if items all have equal priorities, then you can make this much more simple:

n_votes = Vote.objects.filter(Choice=choice).count()
types = {u.userprofile.typeOfPerson: n_votes for u in items}

This also assumes that Vote.objects.filter(Choice=choice).count() doesn't vary at all over the course of the loop.

This:

percent = 0
for type, value in types.items():
    percent += value
if percent == 0:
    percent = 1

can be:

percent = max(1, sum(types.values()))

This:

values = {}
for type, value in types.items():
    if type not in values:
        values[type] = value / percent * 100

can probably just be

values = {t: v/percent * 100 for t, v in types.items()}

Again, it's not clear why all three of your loops are attempting to be careful in avoiding rewriting values, or what the priority system should be.


An implementation of "finally" in C++0x I've made a quick finally type in C++:

template<class F>
class finally_type {
public:

    explicit finally_type(F f) : function(f) {}
    ~finally_type() { try { function(); } catch (...) {} }

private:

    F function;

};

template<class F>
finally_type<F> finally(F f) { return finally_type<F>(f); }

(Edited to add swallower to prevent exceptions from propagating out of the callback.)

With C++0x, it's convenient to use this class as follows:

void test() {

    int* i = new int;

    auto cleanup = finally([i]() { delete i; });

    may_throw();

}

Regardless of whether may_throw() throws, the finally ensures that the cleanup code executes when the stack is unwound. The equivalent idiom in C++03 is using a local type:

void test() {

    int* i = new int;

    struct finally {
        finally(int* i) : i(i) {}
        ~finally() { delete i; }
        int* i;
    } cleanup(i);

    may_throw();

}

As you can see, the closure must be managed explicitly, with loads of repetitive (and consequently error-prone) code; with a lambda, this process is vastly simplified.

Frankly, I don't see much use to it, since if you're interfacing with legacy code, you're probably just going to wrap the types or use a shared_ptr with a custom deleter. You could use it to manage low-level locks in multithreaded code, if that's your fancy for some reason.

Anyway, assuming someone does find it useful, is there anything important that I missed?

Bear in mind that code inside the function called by finally must not allow exceptions to propagate out, or you end up with problems when the destructor is called while cleaning up an exception in the calling code. You might want to put a catch(...) block in the finally_type destructor for that reason.

I agree that the C++98 solution is much uglier, to the point of not being usable. C++98 is never going to be able to capture arbitrary code for execution later. I'm not so sure this is a problem though, since I expect the cleanup code to have to be specialised only per-type, not per-usage. I want the class to be responsible for cleaning itself up, not the client of the class, since the latter can lead to mistakes or redundancy. The most idiomatic solution is to put this cleanup code in the destructor.

Edit: If you have to support a non-RAII class with an arbitrary cleanup method, then you can make a scoped holder that will call the method for you. It proved more fiddly than I expected, here's the best I could come up with:

class ScopedAction {
 public:
   virtual ~ScopedAction() = 0;
};

ScopedAction::~ScopedAction() {
}

template<class T, class Arg1, void (T::* p)(Arg1)>
class SpecialisedScopedAction : public ScopedAction {
 public:
   SpecialisedScopedAction(T &target, Arg1 &arg) : target_(target), arg_(arg) { }
   ~SpecialisedScopedAction() {
      try {
         (target_.*p)(arg_);
      }
      catch (...)
         {}
   }

 private:
   T &target_;
   Arg1 &arg_;
};

class ScopedActionHolder {
 public:
   ScopedActionHolder(ScopedAction * action) : action_(action) {}
   ~ScopedActionHolder() {
      delete action_;
   }

 private:
   ScopedAction * action_;
};

template<class T, class Arg1, void (T::* fn)(Arg1)>
ScopedAction * makeScopedAction(T & t, Arg1 & arg) {
   return new SpecialisedScopedAction<T, Arg1, fn>(t, arg);
}

Using the following non-RAII class:

enum TransactionState {
   COMMIT,
   ROLLBACK
};

class DBConnection {
 public:
   void finish(TransactionState state) {
      if (state == COMMIT) {
         cout << "committing transaction" << endl;
      } else {
         cout << "rolling back transaction" << endl;
      }
   }
};

You can make a scoped object that calls the finish method, binding in a parameter from local scope like this:

void test() {
   DBConnection conn;
   TransactionState tstate = ROLLBACK;

   ScopedActionHolder cleanup(makeScopedAction<DBConnection, TransactionState, &DBConnection::finish>(conn, tstate));

   cout << "Doing something" << endl;
   tstate = COMMIT;
}

Having to specify the types as template arguments even though they're implicit in the member function pointer type is annoying, I was hoping to work around this but I've run out of time. Also, the syntax is non-obvious and it'll need a new overload of the makeScopedAction function for every different number of method parameters that needs to be supported.

Come to think about it, std::functional or boost::lambda may both be of some help here.


Finding the next perfect number after n This script is for a student who has never programmed before. The goal is that she can quickly start to understand what is going on and start to tinker with it.

As I am not very familiar with Python I wonder whether I am breaking any important style paradigm and whether there are things that can make the code further readable, by making it shorter or simpler.

# Define function
def npn(n):
    """ Find the next perfect number after integer n
    """
    org = n = abs(n)
    while True:
        n += 1
        # Keep track of sum of the positive divisors
        spd = 1
        # Now check (other) possible divisors.
        # Note that subset of divisors to check can be greatly reduced...
        # which will increase efficiency
        for i in range(2, n):
            # Check divisibility with the modulo operator
            if (n % i == 0):
                spd += i
        if n == spd:
            print("The next perfect number after", org, "is", n)
            break

# Get user input
print("\n", "Check what is the next perfect number after n.")
while True:
    try:
        x = int(input("Enter your n: "))
    except ValueError:
        print("n should be a postive integer!")
        continue
    if x < 8128:
        break
    print("Oh. Finding perfect numbers is hard. Choose a lower number. Plz.")

# Run function with user input
npn(x)

The greatest hindrance to understanding is the naming:

What is npn?  Some type of transistor?  next_perfect_number would be a much better name for the function!
What is spd?  An abbreviation for "speed"?  sum_factors would be an improvement… but see below.
You prompt for "n", but assign the result to a variable named x.  That's confusing.

The next biggest problem is that the npn() function tries to do too much.  It would be much clearer if the while loop were moved into a separate is_perfect_number(n) function.

A function that performs a complex calculation should not also perform I/O, as such mixing will prevent code reuse (for unit testing or for a future GUI, for example).  That is, npn() should return its result rather than printing it.

Idiomatic Python would use more elegant ways to perform the loops.  The n += 1 loop could be done using itertools.count().  The loop to calculate spd would be much more succinctly expressed using sum() with a generator expression.  It would certainly be easier to read the intent behind the code that way, but you would also have to introduce a Python-specific language feature.  It's hard to say which approach a beginner would find easier to understand.

It would be good practice to reduce the amount of free-floating code.  I suggest packaging the prompting and printing code in a main() function.

from itertools import count

def next_perfect_number(n):
    """Find the next perfect number after integer n"""
    for i in count(n + 1):
        if is_perfect_number(i):
            return i

def is_perfect_number(n):
    """Test whether n is a perfect number"""
    # Simple but inefficient algorithm
    return n == sum(i for i in range(1, n) if n % i == 0)

def main():
    print("Find the next perfect number after n.")
    while True:
        try:
            n = int(input("Enter your n: "))
            if 1 <= n < 8128:
                break
            print("Choose a number between 1 and 8128, please.")
        except ValueError:
            print("n should be a positive integer!")
    print(
        "The next perfect number after {0} is {1}.".format(
            n,
            next_perfect_number(n)
        )
    )

if __name__ == '__main__':
    main()


Checking for a palindrome What I'm basically wondering is if there's anything that is possible to improve in this C++ code:

#include <cstddef>
#include <string>

bool is_palindromic(std::string s)
{
    std::size_t i = 0;
    std::size_t j = s.length() - 1;
    while (i < j) {
        if (s[i++] != s[j--]) {
            return false;
        }
    }
    return true;
}

I wanted to offer a different angle of better. This code could be more generic. Why is it tied to exactly std::string - what if I need to check palindromic nature of std::wstring or the contents of a std::vector<T>? How about std::list<T> or std::map<T> (if palindromic can even mean anything on a std::map)? Here's how you can handle all of those with a single function:

template <typename Sequence>
bool is_palindromic(const Sequence& seq)
{
    auto count = seq.size() / 2; // rounded down is fine, as a middle element matches itself
    auto i = seq.begin();        // prefer std::begin(seq) in C++14
    auto j = seq.rbegin();       // prefer std::rbegin(seq) in C++14

    for (Sequence::size_type c = 0; c < count; ++c, ++i, ++j)
    {
        if (*i != *j)
            return false;
    }
    return true; // considers sequences without mismatched characters to be palindromes
}

Note that I incorporated the other comments I agree with - in particular passing by const reference, and using the sequence's size_type. I'm not sure how to best support arrays, however; while std::[r]begin will handle arrays, I don't believe arrays have a .size() or ::size_type to use. I guess that means they need their own overload, or at least some overloaded helpers.


Function to remove duplicated elements from an Array Task is to have a function which takes an array. Then removes elements from the array which are contained more then one time. Finally returns an array with only unique elements.

Here's my solution:

// Duplicates : 4 (3 times), 5, 9 (3 times) => Sum: 5 too much !
let withDuplicates = [1, 2, 3, 4, 4,
  4, 5, 5, 6, 7,
  8, 9, 9, 9, 10
];
let withoutDuplicates = [];

// -- The actual function ------------------

function removeDuplicates(arr) {
  return arr.reduce((unique, current) => {

    if (!unique.includes(current)) {
      unique.push(current);
    }

    return unique;
  }, []);
}

// ----------------------------------------

withoutDuplicates = removeDuplicates(withDuplicates);

console.log(withDuplicates);
console.log('Length:', withDuplicates.length);
console.log(withoutDuplicates);
console.log('Length:', withoutDuplicates.length);

Using reduce() was what came to my mind for solving the task.
And the code has become already quite tight.

Nevertheless: Is there a better solution?

Is there a better solution?
Yes, always... biting my tongue.
So first let's look at the function you have.
The JS array functions that take a callback as an argument, like reduce, are slow in comparison to standard loops so for more code you can get a performance increase by implementing only what you need in a reduce function. But we can avoid that as well.
Using a Set
The includes function is also a little inefficient. It needs to iterate each item in the new array and test it against the current item. This can be improved by using a hash table. JS has two objects that use hash table lookups Set and Map. For this case Set will improve the search for duplicates and will also serve to hold the unique array while processing.
If we are going to use a Set we can take advantage of the constructor that will create a set from any iterable object. Thus all we need to do is create the set from the array, then convert the set back to an array as it will have removed the duplicates.
Thus

const withDuplicates = [1, 2, 3, 4, 4, 4, 5, 5, 6, 7, 8, 9, 9, 9, 10];
function removeDuplicates(arr){
   return [...(new Set(arr)).values()];
}
console.log(removeDuplicates(withDuplicates));

I think that is what you may consider better, however it does involve an extra iteration when converting back to the array. To improve on that we would need to create our own Set with a hash table lookup. To do that in JS will never be as fast as the native code, so we can just accept that the complexity is unavoidable in favor of speed.


Weighted Probabilities with Integers for Game The idea here is to pick a skill at random for the player each round.  Four skills will be chosen, they are all random, and the same skill can repeat multiple times.

The player has a skill level for each ability.  The higher the skill level, the greater the chance that the skill will be chosen.

Rather than use floats and percentages, I have created this solution based on ranges and integers. The readability of the code is what I am concerned with here.  Does it make sense from looking at it?  Would more comments help, or should I use a different approach?

I am using this in a libGDX game, so I am limited to Java 6.

private Ability getRandomAbility() {
    final Map<BZRange, Ability> abilityRanges = new HashMap<BZRange, Ability>();
    int count = 0;
    for (Ability ability : this.skillLevels.keySet()) {
        int abilityLevel = this.skillLevels.get(ability);
        BZRange abilityRange = new BZRange(count, count + abilityLevel);
        abilityRanges.put(abilityRange, ability);
        count += abilityLevel;
        //increase the count by an extra one so that the ranges dont overlap
        count++;
    }

    int randomNumber = this.random.nextInt(count);
    for (BZRange range : abilityRanges.keySet()) {
        if (range.contains(randomNumber)) {
            return abilityRanges.get(range);
        }
    }

    return null;
}

Here is the BZRange class, it is very simple:

public class BZRange {

    private int low;
    private int high;

    public BZRange(int low, int high){
        this.low = low;
        this.high = high;
    }

    public boolean contains(int number){
        return (number >= this.low && number <= this.high);
    }
}

You can add the final modifier for your BZRange fields since this class is immutable - this makes the definition (slightly) clearer. :)

//increase the count by an extra one so that the ranges dont overlap

At first, I thought this was an odd comment to read, but then I realize this is just because your contains() implementation is a bound-inclusive comparison... If BZRange is only used purely for this computation, perhaps you can consider changing the contains() implementation to exclude the upper bound hmms?

As for the logic itself... I did a quick search and uncovered not one, but two Stackoverflow suggestions for using a TreeMap and its ceilingEntry() or floorEntry() methods, so you may want to consider that. The gist is:

private Ability getRandomAbility() {
    int offset = 0;
    TreeMap<Integer, Ability> abilityMap = new TreeMap<Integer, Ability>();
    for (Entry<Ability, Integer> entry : this.skillLevels.entrySet()) {
        abilityMap.put((offset += entry.getValue()), entry.getKey());
    }
    // edit: added 1 offset as well for adjustment
    return abilityMap.ceilingEntry(1 + this.random.nextInt(offset)).getValue();
}


Mocking UserDefaults in Swift In my application, I am using UserDefaults to save some data.
I also started writing tests for testing my application code.
At one place I stuck where I wanted to write tests for functions which are saving data to UserDefaults.
When I wrote tests with fake data, my actual data got overwritten by fake data.

So I choose to use Mocking for UserDefaults as follow:

This is controller class which manages saving and fetching data from UserDefaults:

UserDefaultsController-

protocol UserDefaultsProtocol: class {
    func theObject(forKey key: String) -> Any?
    func setTheObject(_ object: Any, forKey key: String)
    func removeTheObject(forKey key: String)
    func synchronizeAll()
}

class UserDefaultsController: NSObject {
    static let shared = UserDefaultsController()
    var delegate: UserDefaultsProtocol?

    override init() {
        super.init()
        //default delegate
        delegate = UserDefaults.standard
    }

    func object(forKey key: String) -> Any? {
        return delegate?.theObject(forKey: key)
    }

    func set(_ value: Any, forKey key: String) {
        delegate?.setTheObject(value, forKey: key)
    }

    func removeObject(forKey key: String) {
        delegate?.removeTheObject(forKey: key)
    }

    func synchronize() {
        delegate?.synchronizeAll()
    }
}

Here is UserDefault class extension which conforms protocol as:

extension UserDefaults: UserDefaultsProtocol {

    func theObject(forKey key: String) -> Any? {
        return self.object(forKey: key)
    }

    func setTheObject(_ object: Any, forKey key: String) {
        self.set(object, forKey: key)
    }

    func removeTheObject(forKey key: String) {
        self.removeObject(forKey: key)
    }

    func synchronizeAll() {
        self.synchronize()
    }
}

Whenever I want to interact with UserDefaults, I do it th’r above class as, e.g.:

UserDefaultsController.shared.set("test.com", forKey: “DomainNameKey”)

It is working fine, no issue at all!

Now, when I write tests, I use mock object to conform to UserDefaultsProtocol as:

class UserDefaultsMock: NSObject, UserDefaultsProtocol {

    private var dict = [String: Any?]()

    deinit {
        dict.removeAll()
    }

    //MARK: - Protocol Methods -
    func theObject(forKey key: String) -> Any? {
        if let object = dict[key] {
            return object
        }
        return nil
    }

    func setTheObject(_ object: Any, forKey key: String) {
        dict[key] = object
    }

    func removeTheObject(forKey key: String) {
        dict.removeValue(forKey: key)
    }

    func synchronizeAll() {
        //none
    }
}

And here is actual test:

class ArrayTests: XCTestCase {

    override func setUp() {
        super.setUp()
        // Put setup code here. This method is called before the invocation of each test method in the class.
        UserDefaultsController.shared.delegate = UserDefaultsMock()
        UserDefaultsController.shared.set("test.com", forKey: “DomainNameKey”)
    }

    override func tearDown() {
        // Put teardown code here. This method is called after the invocation of each test method in the class.
        UserDefaultsController.shared.delegate = nil
        super.tearDown()
    }

    func testFunction() {
        XCTAssertEqual(someFunctionWhichInteractsWithUserDefaults())
    }
}

Now my test is not overwriting anything on my actual data, works perfect!
But my worry is, I need to write all the UserDefault APIs like string(forKey), int(forKey), etc in protocol, controller class, and in UserDefault extension and mock.

Is this is the way it should be Or am I missing any other approach on testing UserDefaults?

My only worry is because every time in my app I have to use UserDefaultsController instead of UserDefaults!

So first, as CAD97 rightly points out, if you simply define your protocol as identically matching the methods from UserDefaults you intend to use, then making UserDefaults conform to the protocol is much simpler:

extension UserDefaults: UserDefaultsProtocol {}

And there ya go, done.  Having to write less code is nice.  What's even nicer here is that we're doing all of this presumably for the sake of testing.  If you're testing right, you're probably looking at your code coverage numbers.  Using this approach, we don't have rogue uncovered lines in the extension that we're not going to hit because we're using the mock.

I'm not a huge fan of the name you've picked for the protocol.  Maybe it's not so bad... and it does largely depend on what you're actually using user defaults for.  But more generally, user defaults is just a key-value store for persisting information between sessions.  There are other key-value store options as well though.  And I believe you'd be better served by making your protocol more generally match more of the options.  The advantage gained here is that swapping from user defaults to key chain to any other system because pretty effortless.

I do not like the UserDefaultsController at all.  You've added an extra, unnecessary layer of complications here.  It's a singleton god class... and it means that if you set up your whole app today to use user defaults, the option to change just parts of it over to a different option aren't as easy.

There's a better way.

Your current approach looks like this:

func fooBarFunc() -> String? {
    return UserDefaultsController.shared.object(forKey: "fooBarKey") as? String
}

However, we could alternatively do something more like this...

func fooBarFunc(from storage: UserDefaultsProtocol = UserDefaults.standard) -> String? {
    return storage.object(forKey: "fooBarKey") as? String
}

Both of these are still called the same:

let value = fooBarFunc()

Because in the second implementation, we pass UserDefaults.standard as the default argument for the storage to pull from.  However, when we want to test this method, we create a different object and pass that in:

let mockStorage = MockStorage(withValue: "expectedValue")
let value = fooBarFunc(from: mockStorage)
XCTAssertEqual(value, "expectedValue")

And alternative, the storage could be pass in to an object's constructor:

init(storage: UserDefaultsProtocol = UserDefaults.standard)

In the main target, we don't pass anything for this argument and just let it default to the standard user defaults.  But again in tests, we implement a mock class that conforms to the protocol and initialize our class with that mock.

This is called dependency injection.  Instead of relying on a god class living somewhere in our code base (and increasing the coupling of our whole app), we pass dependencies into the code that relies on these dependencies.  We use a protocol to keep our code base tightly aligned, but passing objects in, we keep coupling relatively low.  If one class suddenly starts needing something a little different, we can just start passing it something different instead of having to touch the code in the god class... which every other corner of our code base touches... potentially introducing tons of regressions.


Crawler with BeautifulSoup I am trying to create a web crawler for student research. I have already finish it, but I would like to tell me if the way I use is the best one. (probably it isn't :p)

The crawler is for the cnn site and the only thing I want to get, is the text of the news.

Here is an example link: link

Here is my code:

def cnn_crawler(link):
    req = urllib2.Request(link, headers={'User-Agent' : "Magic Browser"})
    usock = urllib2.urlopen(req)
    encoding = usock.headers.getparam('charset')
    page = usock.read().decode(encoding)
    usock.close()

    soup = BeautifulSoup(page)
    div = soup.find('div', attrs={'class': 'cnn_strycntntlft'})
    text = div.find_all('p')
    text.remove(soup.find('p', attrs={'class': 'cnn_strycbftrtxt'}))
    final = ""
    for entry in text:
            final = final + entry.get_text() + " "
    return final

The code looks good! A few comments:

I'd use the requests library instead of urrllib2.
Make sure to follow PEP8 (second line has a trailing whitespace and a space after the dictionary key).
Use more semantic variable names than div, text, or final.
If your code gets bigger, think about separating the HTTP request from the parsing code.


Save 2 related models through a single form Here are two models - Property and Developer:

class Property(models.Model):
    property_type = models.CharField(max_length=255, default='Apartment')
    specifications = models.CharField(max_length=255, default='Basic')
    built_up_area = models.FloatField(max_length=6, null=False)
    total_area = models.FloatField(null=False)
    number_of_bedrooms = models.CharField(max_length=3, default=1)
    number_of_bathrooms = models.CharField(max_length=3, default=1)
    number_of_parking_spaces = models.CharField(max_length=2, default=0)
    address_line_one = models.CharField(max_length=255, null=False)
    address_line_two = models.CharField(max_length=255, default=None)
    city = models.CharField(max_length=255, null=False)
    state = models.CharField(max_length=255, null=False)
    pin_code = models.CharField(max_length=20, null=False)
    developer = models.ForeignKey('Developer')

    connectivity = models.CharField(max_length=255, default=None, null=True)
    neighborhood_quality = models.CharField(max_length=255, default=None, null=True)
    comments = models.CharField(max_length=255, default=None, null=True)

    def __unicode__(self):
        return self.property_type

class Developer(models.Model):
    name = models.CharField(max_length=255, null=False)

    def __unicode__(self):
        return self.name

And here is its corresponding form:

class PropertyForm(ModelForm):
    developer = forms.CharField()

    class Meta:
        model = Property
        fields = [
            'built_up_area',
            'total_area',
            'number_of_bedrooms',
            'number_of_bathrooms',
            'number_of_parking_spaces',
            'address_line_one',
            'address_line_two',
            'city',
            'state',
            'pin_code',
        ]
        widgets = {
            'number_of_bedrooms': forms.Select(
                choices=((1, 1,), (2, 2), (3, 3)), ),
            'number_of_bathrooms': forms.Select(
                choices=((1, 1,), (2, 2), (3, 3)), ),
            'number_of_parking_spaces': forms.Select(
                choices=((1, 1,), (2, 2), (3, 3)), ),
            'developer': forms.TextInput(),
        }

    def __init__(self, *args, **kwargs):
        super(PropertyForm, self).__init__(*args, **kwargs)
        self.helper = FormHelper()
        self.fields['address_line_two'].required = False,
        self.helper.layout = Layout(
            AppendedText('built_up_area', 'sq ft'),
            AppendedText('total_area', 'sq ft'),
            'number_of_bedrooms',
            'number_of_bathrooms',
            'number_of_parking_spaces',
            'address_line_one',
            'address_line_two',
            'city',
            'state',
            'pin_code',
            'developer',
            ButtonHolder(
                Submit('Submit', 'submit', css_class='btn-block')
            )
        )

    def save(self, commit=True):
        developer = self.cleaned_data['developer']
        developer, created = Developer.objects.get_or_create(name=developer)
        self.instance.developer = developer

        return super(PropertyForm, self).save()

What could be a better way to save related objects using a single form?

The way you have this implemented, by overriding the save method to save the related models, is correct and is how it should be implemented. Unless you implement this logic outside of the form, in which case you are probably violating some DRY principles, the form is the correct place to manage data for multiple models.


Checking if numbers in array are sorted or reverse sorted I would like to determine is an array of numbers, is always increasing (for all x, x+1 >= x) or always decreasing (for all x, x+1 =< x). If so return true. Another way to state the problem is, return true if the array is sorted or reverse sorted. Here is the code:

public static boolean isAlwaysIncreasingOrDecreasing(int[] arr) {
        //check if decreasing
        int previous = arr[0];
        boolean decreasing = true;
        for(int i = 1; i < arr.length; i++) {
            if(previous < arr[i]) {
                decreasing = false;
                break;
            }
            previous = arr[i];
        }

        //check if increasing
        int previous = arr[0];
        boolean increasing = true;
        for(int i = 1; i < arr.length; i++) {
            if(previous > arr[i]) {
                increasing = false;
                break;
            }
            previous = arr[i];
        }

        //return true if always increasing or decreasing
        if(decreasing || increasing) {
            return false;
        }

        return true;
}

Any tips? Obviously duplicate values are allowed, so inputs such as 1,2,3,3,3,3....3,4,2,5,6 would be tricky.

I ask mainly out of curiosity, as the above solution would take at most 2 iterations through the array and I don't think it is possible to take less than 1, so it's not too bad :)

Your version is slightly misnamed.  It's actually isNonincreasingOrNondecreasing.  It returns true for arrays where all the entries are the same value, which is neither increasing nor decreasing.

If you don't want to duplicate the checks, you can add a bit more logic.

public static boolean isNonincreasingOrNondecreasing(int[] sequence) {
    // clear out equal entries from the beginning
    int i = 1;
    while (i < sequence.length && sequence[i - 1] == sequence[i]) {
        i++;
    }

    // if there are no entries left, then it is nonincreasing and nondecreasing
    if (i >= sequence.length) {
        return true;
    }

    int previous = sequence[i - 1];
    if (i < sequence.length && previous > sequence[i]) {
        for (; i < sequence.length; i++) {
            if (previous < sequence[i]) {
                return false;
            }

            previous = sequence[i];
        }

        return true;
    }

    for (; i < sequence.length; i++) {
        if (previous > sequence[i]) {
            return false;
        }

        previous = sequence[i];
    }

    return true;
}

This version only loops once, although it has three loops coded.  It only runs one loop over any section of the array.

It also gets rid of the Boolean variables.  It can return immediately since it only gets to the loops once it already knows which one to test.


What is the common API endpoint architecture using DTO? Hey I always used to create some pretty messy endpoints and I decided that it's about time to start writing cleaner code, I'll provide an example of how would I design an endpoint and I'll be  thankful for any insights why is it good or bad practice to do it like this also I'd be more than happy if you provide your own tweaks with explanation why would you do it like that.
Scenario: user post endpoint
Pathname: /api/user
Method: POST
Controller:
class UserPostEndpoint extends AbstractController
{
    private UserPostHandlerInterface $handler;
    private SerializerInterface $serializer;

    function __construct(UserPostHandlerInterface $handler, SerializerInterface $serializer)
    {
        $this->handler = $handler;
        $this->serializer = $serializer;
    }

    public function handleRequest(Request $request): Response
    {
        try {
            $userPostRequestDTO = $this->serializer->unserialize($request->getContent(), UserPostRequestDTO::class);
            $response = $this->handler->handle($userPostRequestDTO); // <- ApiSuccessResponse(statusCode = 201)
        } catch(SerializationException $e) {
            $response = new ApiErrorResponse('Invalid request content provided', 400);
        } catch(ApiRequestContentValidationException $e) {
            $response = new ApiErrorReponse($e->getPlainMessage(), 400);
        } catch(\Exception $e) {
            $response = new ApiErrorReponse('Internal server error', 500);
        } finally {
            return $response;
        }
    }
}

Handler would contain some basic things like: invoke createUser in UserFactory, persist the entity with repository.
ApiErrorReponse: message, statusCode
ApiSuccessReponse: UserPostResponseDTO, statusCode(201)
Would you add / change / remove something from the given example to make it more "correct"? Thanks for the insights
EDIT
After a few comments from @slepic I refactored the code into this
class UserPostEndpoint extends AbstractController
{
    private UserPostHandlerInterface $handler;
    private DataTransformerInterface $dataTransformer;

    function __construct(UserPostHandlerInterface $handler, DataTransformerInterface $dataTransformer)
    {
        $this->handler = $handler;
        $this->dataTransformer = $dataTransformer;
    }

    public function handleRequest(Request $request): Response
    {
        try {
            $userPostRequestDTO = $this->dataTransformer->toRequestDto($request->getContent(), $request->headers->get('Content-Type', 'application/json'));
            $userPostResponseDTO = $this->handler->handle($userPostRequestDTO);
            $response = new ApiSuccessResponse($userPostResponseDTO, Response::HTTP_CREATED);
        } catch(SerializationException $e) {
            $response = new ApiErrorResponse('Invalid request content provided', Response::HTTP_BAD_REQUEST);
        } catch(ApiRequestContentValidationException $e) {
            $response = new ApiErrorReponse($e->getPlainMessage(), Response::HTTP_BAD_REQUEST);
        } catch(\Exception $e) {
            $response = new ApiErrorReponse('Internal server error', Response::HTTP_INTERNAL_SERVER_ERROR);
        } finally {
            return $response;
        }
    }
}

I hope this is more appropriate

How about:
class PostController extends AbstractController
{
    public function store(PostStoreRequest $request): Response
    {
        $dataStruct = app(PostDataStruct::class)->fill($request->validated());

        return app(UserPostHandlerInterface::class)->createPost($dataStruct);
    }
}

PostStoreRequest validates data
PostDataStruct stores contents and has a method that fill all the data for you.
Your code looks a bit like Laravel so I utilized Laravels app container. This way you will have an easier time mocking objects in phpunit.
Within Kernel file forException Handlers, you can define how certain exceptions are handled. i.e. if you call throw new SystemExceptionError('my message') - in the Kernel file state that SystemExceptionError should have HTTP status 500.


Arduino serial data input output I have created an Arduino sketch that I am using to send and receive data via serial. It works pretty well and doesn't seem go wrong as long as the correct formats are received. Is there anything I could do to improve this/shrink it to clear up some code smells?

String dataString;
void setup() {
 Serial.begin(9600);

 //setup digital pins as inputs pin 3 - 13
 for (int i = 2; i <= 7; i++) {
    pinMode(i, INPUT);
  }
  //setup digital pins as OutPuts  pin 22 - 53
  for (int i = 7; i <= 13; i++) {
    pinMode(i, OUTPUT);
  }

}
void loop() {
  serialDataOutput();
  serialDataInput();
  delay(100);

}

void serialDataOutput() {
  dataString = "";
  for (int i = 0; i <= 13; i++) {
    dataString.concat(i);
    dataString.concat("," + String(digitalRead(i)) + "/");
  }

  for (int i = 14; i <= 18; i++) {
   dataString.concat(i);
   dataString.concat("," + String(analogRead(i - 14)) + "/");
  }
   Serial.println(dataString);
  delay(0);
    }
      void serialDataInput() {
  String Input;
   if (Serial.available() > 0) { // If data is available to read,
    Input = Serial.readStringUntil('\n'); // read it and store it in val
  }
  //  Input = "13,1";
  int commaIndex = Input.indexOf(',');
  //  Search for the next comma just after the first
  int secondCommaIndex = Input.indexOf(',', commaIndex + 1);
  String firstValue = Input.substring(0, commaIndex);
  String secondValue = Input.substring(commaIndex + 1, secondCommaIndex);
  digitalWrite(firstValue.toInt(), secondValue.toInt());
}

You claim that this code works.  However, since you haven't really specified exactly what the intended behaviour is, I can't verify whether it does what it's supposed to do.  There are some very strong code smells that lead me to suspect that this code is completely broken, or at least doing something silly.

Non-idiomatic loop limits: The idiomatic way to write a loop that does something 14 times, especially when counting from 0, is

for (int i = 0; i < 14; i++) {
    …
}

This tip would be merely a nitpick, except that I suspect you have a bug in setup(): is pin 7 supposed to be in INPUT or OUTPUT mode?
Apparent mismatch of code and comments:  In setup(), you wrote:

//setup digital pins as inputs pin 3 - 13
for (int i = 2; i <= 7; i++) {

What is the relationship between 3-13 and 2-7?  The comment is either erroneous or fails to fully explain the code.
Misuse of String.concat():  dataString, being a String, is immutable.  Therefore, dataString.concat(i) is a no-op, and dataString.concat("," + String(digitalRead(i)) + "/") is a convoluted way of just calling digitalRead(i) and discarding the result.  The final outcome is that Serial.println(dataString) is equivalent to Serial.println("").

In addition, I see some code hygiene issues, such as inconsistent indentation and using Input instead of input for a variable name.


Rooms and trapdoors - for-loop and while-loop efficiency I'd like to know if I can make this code more efficient in terms of line saving, or any other way:

def missingDoor(trapdoor,roomwidth,roomheight,step):
    safezone = []
    hazardflr = givenSteps(roomwidth,step,True)
    safetiles = []

    for i,m in enumerate(hazardflr):
        safetiles.append((m,step))
        while i < len(safetiles):
            nextSafe = safetiles[i]
            ns0 = nextSafe[0]
            if knownSafe(roomwidth, roomheight, ns0, nextSafe[1]):
                if trapdoor[ns0/roomwidth][ns0 % roomwidth] is "0":
                    if ns0 not in safezone:
                        safezone.append(ns0)
                    for e in givenSteps(roomwidth,ns0,True):
                        if knownSafe(roomwidth, roomheight, e, ns0):
                            if trapdoor[e/roomwidth][e%roomwidth] is "0" and (e, ns0) not in safetiles:
                                safetiles.append((e, ns0))
            i += 1
    return sorted(safezone)

a community member suggest to fix the this code like this:

def missingDoor(trapdoor, roomwidth, roomheight, step):
    safezone, safetiles = [], []
    check = lambda a, b, c, l: knownSafe(roomwidth, roomheight, a, b) and       trapdoor[a/roomwidth][a%roomwidth] == '0' and c not in l

    for i, m in enumerate(givenSteps(roomwidth, step, True)):
        safetiles.append((m, step))
        for _ in range(i, len(safetiles)):
            nextSafe = safetiles[i]
            ns0 = nextSafe[0]

            if check(ns0, nextSafe[1], ns0, safezone):
                safezone.append(ns0)
                safetiles.extend([ (e, ns0) for e in givenSteps(roomwidth,ns0,True) if     check(e, ns0, (e, ns0), safetiles) ])
    return sorted(safezone)

the only issue I had with this though is when I run the code it now runs into this:

TypeError: givenSteps() takes exactly 4 arguments (3 given)

How can I make this code more efficient?  I initially started to use while loops and found that for loops are better when given a known list.  I've been trying all sorts of things, given that I am new to programming.

First things first, stop caring about the number of lines. Unless you are trying to make a code golf competition entry, minimizing the number of lines is the last thing you want to optimize for. This code is hard to read and follow because it is deeply nested and there is very little context to describe what is being done on each line. This code should be broken down into sub functions that encapsulate sections of work.

Compare this line

if knownSafe(roomwidth, roomheight, e, ns0):

to this line

if trapdoor[ns0/roomwidth][ns0 % roomwidth] is "0":

Without knowing anything about the code, I know that the if block is executed if the given arguments describe a location that is known to be safe. Meanwhile, I have no idea why you are indexing into this nested list structure or what the "0" represents.

is does an identity comparison. == does an equivalence comparison. The fact that Two string containing just the character 0 might not always be represented by the same object. The fact that they are is an implementation detail of the Python interpreter you are currently using. Do not use is when you are checking if two values represent the same thing.

for i,m in enumerate(hazardflr):
safetiles.append((m,step))

This is not valid Python. It is likely an issue with posting the question, but as of now, the code in the question does not compile.

Use the Python variable name conventions. Currently, you have a mix of all lower case and camel case. The Python convention is underscore_spaced.

Single letters are rarely good variable names. i is a common convention for index. However, I have no idea what m of e stands for. You also use ns0. From that code, I can see that it is the first element in the nextSafe list. However, that doesn't tell me anything. Why is nextSafe[0] different fromnextSafe[1]? If it is just the current safe location as you iterate over all of the locations, make the variable name curr_safe_location.

You need to pick better data types to represent your state. Lists and strings are great, but they aren't the best way to represent all things. A large portion of your question is describing how your code is structured and what value represent what. This is an indication that the code is hard to read and can be written better. Even if it not required, sometimes making a class is very helpful because it will give value clear descriptive names.

If strings like "X" and "0" are going to have special meanings, at the very least, you should define them as constants. This will give your code more context. Then, if the representation changes, you don't have to look for all instances of "0" and hope every occurrence means the same thing.


Creating a function when user clicks on element to toggle another element I use a lot of "when user clicks on element then another element will be displayed as visible/hidden... aka toggle".

So I'm trying to make a general function to abide the DRY principal.

Before jumping into the code, I just wanted to state that there are still bugs. I just want to know before continuing if writing a function like the one below is necessary or if I should just ignore the DRY in this case and have lots of separate toggle functions. Or if it's necessary, would there be a more efficient method?

JSFiddle

*Brief Description of Code

-There are 4 parameters

The first two are mandatory, the last two are optional
The index parameter is a string of numbers ex. "1.0","1"
The reason I need the last two parameters is because I have elements that can't be easily
targeted. For example, they don't have an #id. So to grab those generic elements, I use getElementsByTagName
Then I'll use the index parameter to get the element in the array.
index parameter: The number before '.' is the index of the clickElement
and the number after '.' is the index of the toggleElement. clickElement = 1. toggleElement = 0.

function toggleClick(clickElement, toggleElement, condition, index) {
    var toggleIndex;
    var clickIndex;

    if(condition === false){
        if(index.indexOf('.') != -1){
            //indexOf returns -1 if '.' is not found.
            // Checks to see if two index is entered
            indexArray = index.split('.');
            clickIndex = indexArray[0];
            toggleIndex = indexArray[1];
            clickElement = document.getElementsByTagName(clickElement)[clickIndex];
            toggleElement = document.getElementsByTagName(toggleElement)[toggleIndex];
        }else{
            //If there is no '.', that means only one index was entered.
            // By function requirement, it should be the index of clickElement

            clickElement = document.getElementsByTagName(clickElement)[index];
        }
    }
    $(clickElement).click(function(){
            $(toggleElement).toggle();
        });
    }

This is one of my first functions, and so I need some guidance on whether I'm writing bad code. Yes, this might be broad because there may be many different ways to write this function.  But I just need to know if there's a more efficient way or if my way is okay.

I know you're trying to be flexible here, but your approach comes at a cost: The API isn't understandable. I can see you're trying to toggle the second parameter element with the first parameter as the toggle,  But I don't understand exactly how the third or fourth works, if it's even needed. It's also a weird convention.

Reflect on these three things here:

The point of programming is having stuff done easier.
The point of a programming language is to do programming in that's understandable to humans.
The point of an API is to "lift" (abstract) the programming language.

Fail on one of those, and the rest before it breaks apart (and things like the great flood, alien invasions, world war 3 happen). So before you design an API, or even just a function, think about the users/developers using it. It might be you in the distant future.

Now off to your code. Seeing you're using jQuery, a similar feat could have been done through this piece of code. I do notice you said "elements that can't be easily targeted. For example, they don't have an #id." - jQuery still can target them using your standard CSS selectors (here's a few to remember), and some custom jQuery selectors (which I think is a superset of CSS selectors, if I'm not mistaken).

. The :eq() part is optional, but it does what you need, select specific indices.

// Assuming delegation for future elements
$(document).on('click','li:eq(0)',function(){
  $('div:eq(0)').toggle();
});

It's as simple as "Get the first of all <li> and toggle the first of all <div>". Simple enough. But what if you wanted your own simple function for it and not some jQuery mashup that is copy-paste bait? Something like this could be done too:

;(function ($) {
  $.fn.customToggle = function (trigger, target, event) {
    // Let's add a twist, custom functions. Assume click if not defined.
    event = event || 'click';
    this.on(event, trigger, function () {
      $(target).toggle();
    });
  }
}(jQuery));

// Still, future-proof, let's support future elements
$(document).customToggle('li:eq(0)', 'div:eq(0)');
$(document).customToggle('li:eq(1)', 'div:eq(1)','mouseover');

There you have it. Now you'd wonder "That's not any different from my code" - exactly. It's the same. However, there's one important bit I changed - it's the way I selected the elements and indices. I used CSS selectors - which every developer who came across CSS or jQuery should already know.

Here are other suggestions on how you could structure the API:

// Chained:
$(document)
  .customToggle('li:eq(0)', 'div:eq(0)')
  .customToggle('li:eq(2)', 'div:eq(4)')
  .customToggle('li:eq(3)', 'div:eq(9)')

// Map
$(document).customToggle({
  'li:eq(0)' : 'div:eq(0)',
  'li:eq(2)' : 'div:eq(4)',
  'li:eq(3)' : 'div:eq(9)'
});


Converting Object to Map using Generics Problem

I am loading things from the localStorage and this has to be saved as json, so it needs to have a simple Object structure to be possible to JSON.parse().

However, som methods do not accept <any> as parameter, because they want a concrete class or interface, but i want to send my object as parameter, so i have to convert it to a Map in order to have the same structure, but seen as it has a type, it is now accepted as parameter.

My problem lies within the conversion from Object to Map

Solution

public static convertObjectToMap<V>(obj: any, classOfV): Map<string, V> {
    let objectMap = new Map<string, V>();
    if (obj !== undefined && obj !== null) {
      for (let key in obj) {
        if (obj.hasOwnProperty(key)) {
          const initObject = new classOfV(obj[key]);
          objectMap.set(key, initObject);
        }
      }
    }
    return objectMap;
  }

I take an obj and the class which all values are going to be in the same type.

Example of usage

//This is purely for the example
const fibonacciObject:any = {"0": 1, "1": 1, "2": 2, "3": 3, "4": 5};

const fibonacciMap:Map<string, Number> = convertObjectToMap<Number>(fibonacciObject, Number);

fibonacciMap.get("0"); //1
fibonacciMap.get("4"); //5

Question

Is there a better way to do this conversion, i know about new () => V, but since i need it for each key, then it is not really feasible.

Also what Type would class of V be, i keep getting type errors when i try to give it a Type

A few points first.

Avoid any like the plague. You can nearly always figure out a better type. When dealing with a JSON serialized data, I like to have a function similar to this to get rid of any as soon as possible:

function verify<T>(obj: any, fallback: T, isT: (obj: any) => obj is T): T {
  return isT(obj) ? obj : fallback;
}

Object.keys and Object.entries are a better fit for looping through an object if you are going to check hasOwnProperty. I prefer Object.entries when possible, if you have the browser support.
Choose const or let, don't mix them without good reason. const can result in better type inference so I prefer to use it when possible.

Here is how I would implement this function.

function convertObjectToMap<In, Out>(
  obj: { [K: string]: In } | undefined | null,
  classOfIn: new (v: In) => Out
): Map<string, Out> {
  const result = new Map<string, Out>();

  for (const [key, val] of Object.entries(obj || {})) {
    result.set(key, new classOfIn(val));
  }

  return result;
}


Calculate Pi using Monte Carlo #include <iostream>
#include <iomanip>

#ifdef USE_OLD_RAND

#include <stdlib.h>
inline double getRandDart() {return rand() * 1.0 / RAND_MAX;}

#else

#include <random>
std::default_random_engine              generator;
std::uniform_real_distribution<double>  distribution(0,1);
inline double getRandDart() {return distribution(generator);}

#endif

// Monte Carlo Simulator to estimate the value of PI.
//
// If we have a circle with a radius of 1.
// Then the smallest square that encloses the circle as sides of length 2.
//
//      Area of circle pi r^2   = pi
//      Area of square 2.r.2.r  = 4
//
//          Ratio of overlapping area:      pi/4
//
// If we throw darts randomly at a dart board (with an even distribution) and always hit the square.
// Then the ratio of darts falling into the circle should be pi/4 of the total number of darts thrown.
//
//      pi/4 * countInSquare  = countInCircle
//
//      pi = 4 . countInCircle / countInSquare
//
// To simplify the maths.
// We will set the center point as 0,0 and only use the top right quadrant of the circle.
// We have 1/4 the size of the square and circle but the same maths still apply.
//
// A dart thrown has a random x/y value in the range 0->1 (top right quadrant).
// A dart is outside the circle if x^2 + y^2 > 1  (note 1^2 is 1)
//

int main()
{

    long     countInSquare   = 0;
    long     countInCircle   = 0;

    for(long iteration = 0; iteration <= 10'000'000'000; ++iteration) {
        double x = getRandDart();
        double y = getRandDart();

        double d = (x * x) + (y * y);

        countInSquare   += 1;
        countInCircle   += (d >= 1.0) ? 0 : 1;

        if (iteration % 10'000'000 == 0) {
            std::cout << iteration << " " << (4.0 * countInCircle / countInSquare) << "\n";
        }
    }
    std::cout << "\n\n" << std::setprecision(9) << (4.0 * countInCircle / countInSquare) << "\n";

}

Output:

> ./a.out
9990000000 3.14158
10000000000 3.14158

3.14158355

One could consider at least the following points:

Instead of including <stdlib.h>, I'd include <cstdlib>.
In getRandDart(), it might in this case be more readable to do static_cast<double>(rand()) / RAND_MAX; instead of multiplying by 1.0.
In the for loop, all of x, y and d can be const, so I'd make them const. This has the potential to protect the programmer from unintended mistakes, and can sometimes allow the compiler to optimize better.
When you increment by one (in countInSquare += 1;), it makes more sense to use the ++ operator, i.e., to just write ++countInSquare. This is more idiomatic and protects us from unintended mistakes: ++ conveys the meaning of increment (by one), whereas with += we might accidentally write += 2; and that would be perfectly valid (but not what we wanted).
Regardless of the above point, notice that during the for-loop, it holds that iteration == countInSquare. So strictly speaking, the variable countInSquare is unnecessary and could be replaced by just iteration when needed.
You could consider making the number of iterations and the second operand of the % operand constants to allow for easier modification and perhaps to slightly improve readability.
Instead of typing (4.0 * countInCircle / countInSquare) twice, we could make a function that takes the two variables as parameters. This would allow us to save some typing, and again to protect us from unintended mistakes.


Destroying a process if it takes too much time I am executing a shell script from my Java program using Process and I want to kill/destroy that process if my script takes long time.

I came up with this code and wanted to see if there is any better way or anything I can improve on.

public class Test {

    public static void main(String[] args) throws IOException, InterruptedException {

        // Your script
        String script = getScriptTextFromSomewhere();

        // create a temp file and write your script to it
        File tempScript = File.createTempFile("temp_scripts_", "");
        tempScript.setExecutable(true);
        try (OutputStream output = new FileOutputStream(tempScript)) {
            output.write(script.getBytes());
        }

        // build the process object and start it
        List<String> commandList = new ArrayList<>();
        commandList.add(tempScript.getAbsolutePath());
        ProcessBuilder builder = new ProcessBuilder(commandList);
        builder.redirectErrorStream(true);
        builder.environment().put("param1", "abc");
        builder.environment().put("param2", "xyz");
        Process shell = builder.start();

        // read the output and show it
        try (BufferedReader reader = new BufferedReader(new InputStreamReader(shell.getInputStream()))) {
            String line;
            while ((line = reader.readLine()) != null) {
                System.out.println(line);
            }
        }

        // wait for the process to finish
        WaitingThread thread = new WaitingThread(shell);
        thread.start();
        long timeoutMillis = 20000;
        thread.join(timeoutMillis);
        if(thread.isAlive()) {
            thread.interrupt();
        }

        if(!thread.isFinished()) {
            shell.destroy();
            shell.waitFor();
            System.out.println("PROCESS WAS INTERRUPTED");
        } else {
            // check the exit code (exit code = 0 usually means "executed ok")
            System.out.println("PROCESS FINISHED, EXIT CODE: " + thread.getExitValue());
        }

        // delete temp file
        tempScript.delete();
    }
}

class WaitingThread extends Thread {

    private int exitValue;
    private Process shell;
    private boolean finished = false;

    public WaitingThread(Process shell) {
        this.shell = shell;
    }

    @Override
    public void run() {
        try {
            exitValue = shell.waitFor();
            finished = true;
        } catch (InterruptedException e) {
            finished = false;
        }
    }

    public int getExitValue() {
        return exitValue;
    }

    public boolean isFinished() {
        return finished;
    }

}

First, I'm not too sure how your code works... your timeout starts after you begin to read the stdout of the spun Process object for non-null lines, but if you read the Javadoc of BufferedReader.readLine() again:

Returns:
A String containing the contents of the line, not including any line-termination characters, or null if the end of the stream has been reached

If my understanding of how the stdout pipelining done by the Process object is correct (wells, this being OS-dependent and what-not), I think the end of Process.getInputStream() is reached when the native process is disconnected. In most scenarios, said process would have completed too, unless we are talking about starting daemon processes.
In other words, the timeout check becomes a dummy step when it is reached, and thread.isFinished() will most likely return true, always.
Second, since you are already on Java 7 at least, you should be relying on a combination of ExecutorService and Future implementations to achieve your goal as they arguably have better semantics for performing timeout checks.
For example, since all you need is to run one background thread, you can use Executors.newSingleThreadExecutor():
ExecutorService service = Executors.newSingleThreadExecutor();

Then, since you do not require a return value, you can submit() a Runnable, and call the resulting Future's get(long, TimeUnit) method:
Runnable runnable = createRunnableForProcessHandling(shell);
Future<?> future = service.submit(runnable);
try {
    future.get(timeout, timeoutUnit);
    // completed within timeout
} catch (TimeoutException e) {
    // do something about the timeout
} finally {
    service.shutdown();
}

The alternative is to submit() a Callable to get perhaps the output of the native process as a List, for further processing:
Callable<List<String>> callable = getOutputLinesFrom(shell);
Future<List<String>> future = service.submit(callable);
try {
    List<String> outputLines = future.get(timeout, timeoutUnit);
    // do something with outputLines
} catch (TimeoutException e) {
    // do something about the timeout
} finally {
    service.shutdown();
}


Recursive function to get all Zendesk tickets I'm developing a Python library to consume the Zendesk search API.
Problem is, Zendesk limits search results to 1000 items. Thus in order to get all the results if they are greater than 1000 I need to call the endpoint several times with a recursive function storing the latest created datetime for a ticket once the 1000 limit is reached.

My code at the moment works but it feels wrong. Could you help me to improve this recursive function? For instance I'd prefer not to declare an empty list at the constructor but instead inside the function.

Here is the code:

init.py

import requests
import base64
from zdesktools.lib.api import SearchApi, TicketsApi

class ZdeskTools(object):
    def __init__(self, zendeskApiUrl=None,
                 username=None,
                 password=None):
        session = self._begin_session(username, password)
        config = {
            'zendeskApiUrl': zendeskApiUrl,
            'session': session
        }
        self.search = SearchApi(**config)
        self.tickets = TicketsApi(**config)
        # self.metrics = MetricsApi(**config)

    def _begin_session(self, username, password):
        session = requests.Session()
        username_password = f'{username}/token:{password}'
        encoded_bytes = base64.b64encode(username_password.encode('utf-8'))
        encoded_string = str(encoded_bytes, 'utf-8')
        headers = {
            'Authorization': f'Basic {encoded_string}'
        }
        session.headers.update(headers)
        return session

api.py

from urllib.parse import urlencode
import json
import pprint

class SearchApi(object):
    def __init__(self, **config):
        self.session = config.get('session')
        self.url = config.get('zendeskApiUrl')+'/search.json?'
        self.created_at = []
        self.tickets = []

    def get_ticket_numbers(self, search_params):
        response = self.session.get(f'{self.url}{urlencode(search_params)}')
        data = response.json()
        if 'results' in data:
            for result in data['results']:
                self.created_at.append(result['created_at'])
                self.tickets.append(result['id'])
        if 'next_page' in data:
            if data['next_page'] is not None:
                self.url = data['next_page']
                self.get_ticket_numbers(search_params)
        else:
            self.url = self.url[:self.url.index('?')+1]
            search_params['query'] = search_params['query'] + \
                f' created_at>={self.created_at[-1]}'
            self.get_ticket_numbers(search_params)
        return self.tickets

My usual advice for implementors of recursive algorithms Is this: figure out how to do it without recursion (although i dont know how to prove it Is Always better, i can prove it Is never worse. And i can prove it Is Always possible.)

This advice Is yet stronger in your case because there Is nothing recursive about pagination. Its you who made it recursive artificially.

Further, the api has reasons to provide the data with a page limit. They dont want to run to memory issues when serving those data to you.

Nor should you want to run into memory issues when processing the data. If Its not restricting you, you should also process the data page by page discarding the previous page before loading another one.