Inner cursor performance issues Before I tried to avoid using cursor because of performance issues. But now I have to do some calculations to reach some goals. I have a stored procedure that has a code like this:

DECLARE  Outer_Cursor CURSOR FOR...

   OPEN Outer_Cursor
   FETCH NEXT FROM Outer_Cursor INTO ...

   WHILE @@FETCH_STATUS = 0
   BEGIN

      DECLARE  Inner_Cursor CURSOR  FOR...

      OPEN Inner_Cursor
      FETCH NEXT FROM Inner_Cursor INTO ...

      WHILE @@FETCH_STATUS = 0
      BEGIN

        ...

        FETCH NEXT FROM Inner_Cursor INTO ...
      END

      CLOSE Inner_Cursor
      DEALLOCATE Inner_Cursor

      FETCH NEXT FROM Outer_Cursor INTO ...

  END

  CLOSE Outer_Cursor
  DEALLOCATE Outer_Cursor

Is it possible to avoid (minimize) so expensive cursor recreation for inner cursor. How can I reuse inner cursor declaration, if it is possible?

Best practice is to do away with cursors. Both of them. There is always a better-performing solution.

If possible, reduce the problem to one that can be solved with sets instead of iteration.

But, if this nested iteration cannot be avoided, you will still get far better performance using table variables and WHILE loops instead of a cursor. For example:

DECLARE @outerData TABLE (outerID INT, outerColumn VARCHAR(50), PRIMARY KEY (outerID))
DECLARE @innerData TABLE (innerID INT, innerColumn VARCHAR(50), PRIMARY KEY (innerID))
DECLARE @currOuter INT
DECLARE @currInner INT

INSERT INTO @outerData
SELECT  EmployeeID, EmployeeName
FROM dbo.Employee

WHILE EXISTS (SELECT outerID FROM @outerData)
BEGIN
    SELECT @currOuter = TOP outerID FROM @outerData

    INSERT INTO @innerData
    SELECT SaleID, SaleDesc
    FROM EmployeeSales
    WHERE EmployeeID = @outerID

    WHILE EXISTS (SELECT innerID FROM @innerData)
    BEGIN
        SELECT @currInner = TOP innerID FROM @innerData

        --Do work with inner data

        ...blah-de-blah...

        --Delete after work is finished
        DELETE FROM @innerData   WHERE innerID = @currInner
    END

    --Delete current outerID and empty the innerdata table
    --to prepare to move to the next record

    DELETE FROM @outerData WHERE outerID = @currOuter
    DELETE FROM @innerData
END


Poker deck class /w generator function and list comprehensions There are multiple aspects in the code I do not really like. [card for card in ...] looks really lame, also [x.pop() for i in range(y)]. I'm looking forward for any recommendation.

from random import shuffle

class Deck(object):
    suits = range(4)
    ranks = range(13)

    @classmethod
    def generator(cls, suits, ranks):
        for suit in suits:
            for rank in ranks:
                yield({'suit': suit, 'rank': rank})

    def __init__(self):
        self.cards = [card for card in Deck.generator(Deck.suits, Deck.ranks)]
        shuffle(self.cards)

    def deal(self, amount):
        return [self.cards.pop() for i in range(amount)]

print Deck().deal(5)

[card for card in Deck.generator(…)] could be written as list(Deck.generator(…)).

Better yet, use itertools.product():

self.cards = [
    {'suit': suit, 'rank': rank}
    for suit, rank in itertools.product(xrange(4), xrange(13))
]

In Python 2, you should be using xrange() rather than range().

It's probably worth defining a Card class.  At some point, you'll want to have suits that are named rather than numbered, and ranks A, J, Q, K rather than 0..12.

To deal multiple cards, you can slice the list instead of popping one card at a time:

def deal(self, n):
    hand = self.cards[-1 : -n-1 : -1]
    self.cards = self.cards[: -n]
    return hand


Write it better/neat/fast (if condition) How to write following if condition clean/fast/readable.
Any one of the fields 'address', 'city', 'district', 'province' can be null. I want to make full address by concat all these fields. Like

address = "Street # 32"
city = "My City"
District = "New York"
Province = "NY"

fullAddress = "Stree #32, My City, New York, NY"

If any information is missing then don't add this in string like if district is missing then don't include it.

address = "Street # 32"
city = "My City"
District = ""
Province = "NY"

fullAddress = "Stree #32, My City, NY"

Following code I have written but not good.

string address = dr["Address"].ToString();
string city = dr["City"].ToString();
string district = dr["District"].ToString();
string province = dr["Province"].ToString();
string fullAddress = "";

if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + city + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district))
{
    fullAddress = address + ", " + city + ", " + district;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + city + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = city + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city))
{
    fullAddress = address + ", " + city;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + province;
}
else if (!string.IsNullOrEmpty(address))
{
    fullAddress = address;
}
else if (!string.IsNullOrEmpty(city))
{
    fullAddress = city;
}

I think using LINQ together with string.Join() would be the best option here:

var fields = new[] { "Address", "City", "District", "Province" };
var setValues = fields.Select(field => (string)dr[field])
                      .Where(value => !string.IsNullOrEmpty(value));
string fullAddress = string.Join(", ", setValues);


Checking if an element sastifying a condition in a list comes before another of a different condition This is for Java 6 and uses Guava.

It's a little ugly and I'm worried I missed some obvious way to do it a lot better.

If there is more than one element satisfying either condition I am saying it's false because the condition I am checking should be unique among the list.
If and only if the one element satisfying condition1 is in the list with a lower index than the one element satisfying condition2 should this return true.

condition1 and condition2 will never both be true for the same element.

private static <T> boolean isItemWithConditionBeforeAnother(Predicate<T> condition1, Predicate<T> condition2, List<T> list)
{
    if (Collections2.filter(list, condition1).size() != 1 || Collections2.filter(list, condition2).size() != 1)
    {
        return false;
    }
    boolean found1 = false;
    for (T t: list)
    {
        if (!found1)
        {
            if (condition1.apply(t))
            {
                found1 = true;
            }
            else if (condition2.apply(t))
            {
                return false;
            }
        }
        else
        {
            if (condition2.apply(t))
            {
                return true;
            }
        }
    }
    return false;
}

Currently, you are in fact looping through the entire list three times.

Collections2.filter(list, condition1).size()
Collections2.filter(list, condition2).size()
for (T t: list)

This is a bit inefficient, as ultimately, once should be enough.

To accomplish that, we need to check each item exactly once (or technically, at most once, as it is possible to return false early).

Also, to make the method slightly more useful, you can use Predicate<? super T>.

Which is easiest, checking if the list does adhere to your requirements or checking if it does not? In my opinion, it is easier to check if one of your requirements fail, which means that we can have the method return true; as the last statement and perform some early returns for false if something doesn't match your requirements.

What I would end up with is this, which is pretty much straight-forward:

private static <T> boolean isItemWithConditionBeforeAnother(Predicate<T> condition1, Predicate<T> condition2, List<T> list) {
    int found1 = 0;
    int found2 = 0;
    for (T t: list) {
        if (condition1.apply(t)) {
            found1++;
        }
        if (condition2.apply(t)) {
            found2++;
        }
        if (found1 > 1 || found2 > 1) {
            return false;
        }
        if (found2 > found1) {
            return false;
        }
    }
    return found1 == 1 && found2 == 1;
}

Using the found variables as int instead of boolean helps greatly here. It lets us check if any condition has been matched twice, and lets us use the trick found2 > found1 to check if condition2 was matched before condition1.

Alternative approach using booleans, inspired by @janos.

boolean found1 = false;
boolean found2 = false;
for (T value : list) {
    if (found1 && condition1.apply(value)) {
        return false;
    }
    if (found2 && condition2.apply(value)) {
        return false;
    }
    found1 = found1 || condition1.apply(value);
    found2 = found2 || condition2.apply(value);
    if (found2 && !found1) {
        return false;
    }
}
return found1 && found2;

An important thing here is that once found1 is true, it will never be set to false again thanks to the short-circuiting || operator.


Repeatedly fetching a URL until the response contains "state: INIT" I am working on a project in which I need to make a URL call to one of my servers from a bash shell script.

 http://hostname.domain.com:8080/beat

After hitting the above URL, I will be getting the below response, from which I need to parse and extract the value of state:

num_retries_allowed: 3 count: 30 count_behind: 100 state: INIT num_rounds: 60 hour_col: 2 day_col: 0

Now I will keep on hitting the above URL every 10 seconds until the value of the state variable is equal to INIT:

state = INIT

If the state is equal to INIT then I will exit successfully out of the shell script, otherwise I will keep on trying until the state becomes INIT.

Also, if the state: string is missing in the $DATA variable by any chance, then I want to assign 0 to the state variable. So if state is equal to 0, then I will also exit out of the shell script successfully.

I am hitting the above URL and checking whether I got a successful response or not. If I got a successful response, then I am breaking out (which means server is running fine), but if I didn't got successful response, then I am checking whether my server is running or not on port 8080. If it is not running, then I will start the server. And I am retrying this for a period of 15 times, and after 15 tries if the server hasn't come back up, then exit out of the shell script with a non-zero status code.

Is the below shell script correct?

#!/bin/bash

HOSTNAME=$hostname
DATA=""
RETRY=15

echo $HOSTNAME

sleep 1m

while true; do

    while [ $RETRY -gt 0 ]
    do
        DATA=$(wget -O - -q -t 1 http://$HOSTNAME:8080/beat)
        if [ $? -eq 0 ]
        then
            break
        else
            if lsof -Pi :8080 -sTCP:LISTEN -t >/dev/null ;
            then
                echo "Server is running"
            else
                echo "Server is not running so re-starting the server here"
                # starting server here
            fi
            let RETRY-=1
            sleep 30
        fi
    done

    if [ $RETRY -eq 0 ]
    then
        echo "Server is still down. Exiting out now." >&2
        exit 2
    fi

    echo "Server is up. Now parsing the data out of the beat."

    #grep $DATA for state
    state=$(grep -oP 'state: \K\S+' <<< "$DATA")
    [[ -z "$state" ]] && state=0

    echo $state

    #verify the condiitons
    if [[ $state -eq "INIT" || $state -eq "0" ]]; then exit 0; fi

    #wait another 10 seconds
    sleep 10

done

Is there any better way of doing this if I am doing anything wrong?

Inconsistent logic

In one place the script checks http://$HOSTNAME:8080/beat,
in another, it checks for a local process listening on port 8080.
This only makes sense if $HOSTNAME the name of the local machine.
It will make more sense to replace $HOSTNAME with hard-coded localhost.

Use exit code directly

Instead of this:

DATA=$(wget -O - -q -t 1 http://$HOSTNAME:8080/beat)
if [ $? -eq 0 ]
then
    break

You could use the exit code directly like this:

if DATA=$(wget -O - -q -t 1 http://$HOSTNAME:8080/beat)
then
    break

Another way:

DATA=$(wget -O - -q -t 1 http://$HOSTNAME:8080/beat) && break

Math in Bash

Instead of this:

let RETRY-=1

There are slightly simpler alternatives:

((RETRY -= 1))
((RETRY--))


Mapping multiple values to an array object I've reached a point in an application where I need to initialize some coordinates to an array and I wound up using nested for-in loops to accomplish the task.

Is there is a better way to accomplish this?

class Coord {

    let xVal : Int
    let yVal : Int

    init(x: Int, y: Int) {
        xVal = x
        yVal = y
    }
}

var coord = [Coord]()

for xVal in 0...20 {
    for yVal in 0...20 {
        coord.append(Coord(x: xVal, y: yVal))
    }
}

What you have is fine. If you wanted to use a functional approach and avoid making coord be mutable you could use flatMap and map:

let indexes = 0...20
let coord = indexes.flatMap { (xVal:Int) -> [Coord] in
    indexes.map { (yVal:Int) -> Coord in
        Coord(x: xVal, y: yVal)
    }
}

If you are familiar with lambda expressions then you may know that this can be simplified to this:

let indexes = 0...20
let coord = indexes.flatMap { x in indexes.map { Coord(x: x, y: $0) } }

If all you are doing with Coord is storing coordinates, you may consider just using Tuples:

let coord = indexes.flatMap { xVal in indexes.map { yVal in (xVal, yVal) }}


TextView vs Button for Android AlertDialog I want to open an AlertDialog on the click of either a Button or TextView but I am not sure what would work best. When a selection is made in the AlertDialog, I want the bottom text to be updated with the selection. Here are the two options I have:

TextViews only. Here, the LinearLayout will have listener and will open AlertDialog.

<LinearLayout
    android:id="@+id/container"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_marginTop="8dp"
    android:orientation="vertical">

    <TextView
        android:id="@+id/label"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:text="Label" />

    <TextView
        android:id="@+id/selection"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_marginLeft="5dp"
        android:layout_marginStart="5dp"
        android:textSize="18sp"
        android:text="Select..." />

</LinearLayout>

TextView and Button. Here, the Button will open the AlertDialog and the text inside the Button will be replaced with the selection.

<TextView
    android:id="@+id/label"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_marginTop="10dp"
    android:layout_marginLeft="5dp"
    android:layout_marginStart="5dp"
    android:text="Label"
    android:textSize="16sp"
    android:textColor="@android:color/white" />

<Button
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_below="@id/label"
    android:layout_marginTop="-20dp"
    android:text="Select..."
    android:textAllCaps="false"
    android:textColor="@android:color/white"
    android:gravity="start|center_vertical"
    android:paddingLeft="15dp"
    android:paddingStart="15dp"
    android:paddingRight="15dp"
    android:paddingEnd="15dp"
    android:paddingTop="15dp"
    android:textSize="18sp"
    android:background="@android:color/transparent" />

Answer is Button.

Button is a subclass of TextView. So it has all the features of TextView and additional features of Button class. Button adds a few stylistic changes (e.g., default background image, additional padding, ripple effect) to allow quicker adaptation to your designs.


Basic/beginner interactive program in Python I bring this code review plea over from suggestions from SO. I've made the suggested edits from that thread, except I kept one sys.exit() call because I was having trouble exiting otherwise. Overall, the code works as desired, and I'm looking for advice on how I can make it even cleaner.

import time
import sys

done = "I'm tired of you. Goodbye."
rainedOut = "Sorry, rain foiled your plans :("
dontUnderstand = "I'm sorry, I don't understand."

def good_weather():
    """Imagine a world where every 5 seconds it rains (good_weather = False),
    then is sunny again (good_weather = True). This function should return
    whether good_weather is True or False at the time it's called.
    """
    return (time.time() % 10) <= 5

def start():
    entries = 0

    while entries < 4:
        choice = raw_input("Hello! What do you want to do right now? Options: 1) Sleep, 2) Work, 3) Enjoy the great outdoors: ")

        if choice == "1":
            print "We are such stuff as dreams are made on, and our little life is rounded with a sleep. - Shakespeare, The Tempest"
        elif choice == "2":
            work()
        elif choice == "3":
            outdoors()
        else:
            print dontUnderstand
            entries += 1
    print done

def work():
    entries = 0
    entries2 = 0

    while entries < 4:
        choice = raw_input("Would you prefer sedentary office work or manual labor in the elements?: ")

        if "office" in choice:
            print "The brain is a wonderful organ; it starts working the moment you get up in the morning and does not stop until you get into the office. -Robert Frost"
            start()
        elif "manual" in choice:
            sunny = good_weather()
            if sunny == True:
                print "A hand that's dirty with honest labor is fit to shake with any neighbor. -Proverb"
                start()
            else:
                if entries2 < 3:
                    print rainedOut
                    entries2 += 1
                    continue #restart loop
                else:
                    print "You should probably just give up now."
                    start()
        else:
            print dontUnderstand
            entries += 1
    print done
    sys.exit()

def outdoors():
    sunny = good_weather()
    if sunny == True:
        print "Adopt the pose of nature; her secret is patience. -Ralph Waldo Emerson"
        start()
    else:
        print rainedOut
        start()

start()

Ok here we go.

Style

You should get acquainted with the official python coding style, which is PEP8.
I notice especially naming violations with your global variables, which should follow the rules of functions: lowercase with underscore separation.

If-elsing

Perhaps it's just me, but I find heavily usage of if/elif/else messy.

Python does not support the typical switchstatement that could fit here, but there is an idiom that resembles it in python by using a dictionary of functions.

for example:

options = {1 : sleep,
           2 : work,
           3 : outdoors,
}
options[entries]()

this will require you to create an additional sleep function that can be executed to print the text We are such stuff as dreams are made on

This technique however lacks the break system that is available in switch statements, but you don't seem to need that in your code.

Additionally this scales really easily, all you got to do is add a new method, nicely encapsulated, and add it to the dictionary.

Multiple end points

Your code seems to have multiple end points. At the end of your start() function you print done. But also in your work() function you print it and terminate the program manually. It is cleaner to have a smooth main function, with multiple options (may it be function calls), when the function calls return, you simply print done only there and the program stops.

Main function

It is a good habit to use a main function, perhaps even better to rename your start to main and use the following construct:

def main():
    # Your main code

if __name__ == "__main__":
    main()

Goodluck!


Register script security and optimization function RegisterN() {
if($_SERVER['REQUEST_METHOD'] == 'POST'){
    if (isset($_POST["password"],$_POST["password1"]) && $_POST["password"]!=$_POST["password1"]) { # Check if password and password1 are same
    echo "Your password confirmation is not equal to your password or it missing.";
        } else if(!$_POST["email"]){ # Check if email is present
        echo "Please enter your email.";
            } else if (!$_POST["password"]){ # Check if password is present again
            echo "Please enter your password";
                } else if (!$_POST["password1"]){ # Check if password1 is present again
                echo "Please enter your second password";
    } else {
     if (strlen ($_POST["password"])>25 || strlen ($_POST["password"])<6) # check password length max 25, min 6
        {
          echo "Password must be between 6 and 25 characters";
        } else {
        $dbh = new Database(); # Open database
        $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); # Small SQL injection security

            $stmt = $dbh->prepare('SELECT email FROM members WHERE email=?'); # Check if email exists
            $stmt->bindParam(1, $_POST['email'], PDO::PARAM_INT);
            $stmt->execute();
            $rows = $stmt->fetchAll(PDO::FETCH_ASSOC);

            if( ! $rows)
            { # No email found - moving on - If all set - let's register :)
                $email = $_POST['email'];
                $password = $_POST['password'];

                $hash_cost_log2 = 8;
                $hash_portable = FALSE;

                require("includes/PasswordHash.php"); # Hash password lib

                $hasher = new PasswordHash($hash_cost_log2, $hash_portable); # Hashing the password
                $hash = $hasher->HashPassword($password);
                if (strlen($hash) < 20)
                    fail('Failed to hash new password');
                unset($hasher);

                    $STH =$dbh->prepare("INSERT INTO members (email,password) VALUES (:email, :password)"); # Registering new user
                    $STH->bindParam(':email', $email);
                    $STH->bindParam(':password', $hash);
                    $STH->execute();

    echo "Account created successfully.";
    header('Refresh: 2; url=index.php'); # Redirect to index.php in 2 seconds

        } else { # email found - closing database - register fail
            $dbh=null;
            echo('Email already exists.');
            }
            $dbh=null; # Closing database after register
            }
            }
} else {
echo"Invalid Token."; # If user access the page without POST
} }

I want to know if I can make it more secure.  Before I made the script, I checked other scripts to see how they work, and what security measures they use.

What the script does:

Check if request comes from POST
Check if all fields ware filled ( It's being checked by JS as well, but that can be "cracked" )
Check if email already exists
Hash password
Finally register

In addition, I saw that other scripts are far smaller than mine.  Does that happen because they use frameworks?

Update  : OOP implemented ( http://pastebin.com/0DKVxrDK ) for check email and register

Here are my recommendation,

you could make sure users password have a minimum length. 8 alphanumeric characters is ideal  If a users password is 1 char, it can easily be broken no matter how good the salt is
move you database connection and setup into a different class: In the database class, create two separate functions.

Does Email Exist and Return boolean
Create users returning a new user object

Database Connection Class uses the singleton design pattern. We only need 1 connection object to exist when our application is running. The singleton pattern is a design pattern that restricts the instantiation of a class to one object.

class Database
{
    private static $dbh = null;
    //TODO you should move this inside config file
    private $host = 'mysql:host=localhost;dbname=test';
    private $pwd  = '';
    private $user = '';

    public function __construct()
    {
        try
        {
            Database::$dbh = new PDO($this->host, $this->user, $this->pwd);
            Database::$dbh->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
        }
        catch (PDOException $e)
        {
            echo "Connection Error: " . $e->getMessage();
        }
    }

    public static function getInstance()
    {
        if (Database::$dbh === null) {
            new Database();
        }
        return Database::$dbh;
    }
}

//all of our sql code should be in a centralized location for easy updating
class SQLPreparedQuery {
    public static $EMAIL_EXIST = "SELECT email FROM user WHERE email=? LIMIT 1";
    public static $USER_ADD = "INSET INTO user (username, email, first, last) VALUES(?,?,?,?)";
}

require_once './Database.php';
require_once './SQLPreparedQuery.php';
class UserModel {
    public function doesEmailExist($email) {
        $doesEmailExist = false;
        try {
            $dbh = Database::getInstance();

            $stmt = $dbh->prepare(SQLPreparedQuery::$EMAIL_EXIST);
            $stmt->bindParam(1, $email, PDO::PARAM_INT);
            $stmt->execute();
            $result = $stmt->fetchAll(PDO::FETCH_ASSOC);
            $doesEmailExist = (empty($result) ? false : true);
        }
        catch (PDOException $ex) {
            echo "An Error occured!" . $ex;
        }
        return $doesEmailExist;
    }

//USAGE
//This will return true if it exist and false if it doesn't

$userModel = new UserModel();
$status = $userModel->doesEmailExist('james@example.com');

CREATE TABLE `user` (
  `iduser` int(11) NOT NULL AUTO_INCREMENT,
  `first` varchar(45) DEFAULT NULL,
  `last` varchar(45) DEFAULT NULL,
  `email` varchar(45) NOT NULL,
  PRIMARY KEY (`iduser`),
  UNIQUE KEY `email_UNIQUE` (`email`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=latin1$$

INSERT INTO `test`.`user` (`first`, `last`, `email`) VALUES ('james', 'brown', 'james@example.com');

You should always add UNIQUE constraints to usernames and email address to ensure uniqueness in your database.


Making a property virtual to cause EF to load the property I am walking through a sample MVC4 ASP.NET tutorial available on PluralSight.com, by Scott Allen.  I am in a section #5, Working with Data (part 2), "Listing Reviews".
This application has a database of restaurant reviews.  I'm following a section right now in which Scott shows how to add links in the Restaurant view that will display a list of reviews for the selected restaurant.
There was a problem when clicking the link to display the reviews, encountered in the following cshtml file.  The problem was that Model.Reviews in the parameters for Html.Partial method was null by default.
Index.cshtml
@model OdeToFood.Models.Restaurant

@{
    ViewBag.Title = "Index";
}

Reviews for @Model.Name

@Html.Partial("_reviews", Model.Reviews)

    @Html.ActionLink("Create New", "Create")

Scott explained that the Model class Restaurant, listed below, will load all the properties except for Reviews, which are stored in another table somewhere else.  He said there are many ways to fix the problem, but the easiest way to make EF load the reviews is to make the Reviews property virtual.
Model class Restaurant
public class Restaurant
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string City { get; set; }
    public string Country { get; set; }
    public ICollection<RestaurantReview> Reviews { get; set; }
}

He said that at run time, the EF creates a "wrapper" for the restaurant class that will  intercept calls (I don't understand this) to the Reviews property so that when you get to the reviews in the view, The EF will load them up instead of being null.
My concern is that making a property virtual when you don't intend to override it, is bad coding practice. If so, what is the best coding practice here? I'm also wondering if the simplest solution is the best solution here.

EF will not create a wrapper around the Restaurant class! Instead it will derive from your Restaurant class and it will override that property (public ICollection<RestaurantReview> Reviews) because it needs to create a proxy or navigation property instead of an everyday property. If you don't specify that property as a virtual one, the EF can not create a navigation property over it and your relations will not work.

I recommend you to check out this behavior through the debugger you will see classes with this kind of name: Restaurant_HFUEOHUEOHHRRHRHRHHRHIFEFNSES355KL4KLH4KH3KLNEN3L3


Optimize cURL requests to Google API I have a table in MySQL with approx. 20 million rows and the following columns:

id | word_eng | word_indic

I have to translate the English word (the value in column word_eng) into Indian language (the value in column word_indic) using the Google Translate API.

I have written PHP code which spawns multiple cURL requests and fetches data from API and updates it into the table. But this process is quite slow; it takes about 100 to 200 words per second.

I am using RollingCurl for multiple cURL requests. I am running this as a cron job.

What's the best way to make it as fast as possible?

<?php
include_once('db.php');
include_once('functions.php');
include_once('rolling-curl-master/RollingCurl.php');

$table = $argv[1];

$q = "SELECT * from $table where word_indic is null limit 500000";
$result = $conn->query($q); $n = 0;

$urls = array();

while ($row = $result->fetch_assoc())
{
    $id = $row['id'];
    $word = rawurlencode(getName($row['name_eng']));

//getName is a simple function which does some trimming and cleaning up of string

    $url = 'https://www.google.com/inputtools/request?text='.rawurlencode($word).'&ime=transliteration_en_te&id='.rawurlencode($id);
    array_push($urls, $url);
}
//print_r($urls);

unset($url);

$rc = new RollingCurl("request_callback");

// the window size determines how many simultaneous requests to allow.
$rc->window_size = 300;

foreach ($urls as $url)
{
    // add each request to the RollingCurl object
    $request = new RollingCurlRequest($url);
    $rc->add($request);
}
$rc->execute();

function request_callback($response, $info)
{
    // parse the page title out of the returned HTML
    if (preg_match("~<title>(.*?)</title>~i", $response, $out)) {
        $title = $out[1];
    }

    //echo "$title
";
    //print_r($info);
    $parts = parse_url($info['url']);
    parse_str($parts['query'], $query);
    $id = $query['id'];
    $text = $query['text'];

    //echo "
";
    $trans = json_decode($response)[1][0][1][0];

    global $conn; global $table; global $urls; global $n;
    if ($trans != '' and !preg_match('/[a-z]/', $trans))
    {
        $conn->query("update $table set word_indic='$trans' where id='$id'"); $n++;
    }
}

?>

Table structure:

CREATE TABLE `words` (
 `id` varchar(50) COLLATE utf8_unicode_ci NOT NULL,
 `word_eng` varchar(100) COLLATE utf8_unicode_ci NOT NULL,
 `word_indic` varchar(100) COLLATE utf8_unicode_ci DEFAULT NULL,
 PRIMARY KEY (`id`),
 FULLTEXT KEY `word_eng` (`word_eng`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci

Url building

$id = $row['id'];
$word = rawurlencode(getName($row['name_eng']));
//getName is a simple function which does some trimming and cleaning up of string
$url = 'https://www.google.com/inputtools/request?text='.rawurlencode($word).'&ime=transliteration_en_te&id='.rawurlencode($id);

Could be simplified with the use of http_build_query() with encoding type PHP_QUERY_RFC3986 to replace the rawurlencode:

$row['text'] = getName($row['name_eng']);
//unset $row['name_eng'] or select it as "text" in the SELECT statement
$row['ime'] = 'transliteration_en_te'; //or move into SELECT fields in query?
$url = 'https://www.google.com/inputtools/request?'.http_build_query($row, '', '&', PHP_QUERY_RFC3986);

Obviously that will likely result in extra parameters tacked onto the URL (e.g. word_indic, word_eng). You could update the select query to specifically select those fields:

$q = "SELECT id, name_eng as text from $table where word_indic is null limit 500000";

And you could select that ime parameter as a hard-coded value, so as to not need to assign it to each row in the while, but then that would make the result set larger...

Global variables

I see this line:

global $conn; global $table; global $urls; global $n;

In general it isn't a good habit to be referencing variables globally. Obviously this is in the context of a callback function but there are ways to avoid that. I would suggest creating a wrapper class and storing the connection, table and other info in member/instance variables while calling the method using the array syntax.

The Singleton Pattern could be used to create a wrapper class on the database code and call a static method to get a connection to the database, instead of referencing $conn globally.

Also, $urls and $n don't appear to be used after that line anyway...


Manager for customer billing and subscriptions in Rails I'm looking to get rid of a couple dependencies that I have a sneaking suspicion are present in the code that I'm writing.

Right now I have a service class that manages my subscriptions for part of my application in Rails. The problem is that we have Recurly as our management platform right now, but I don't know if we will keep this in 6 months, 12 months, 5 years, etc. So there is an inherent dependency in place for this that I want to try to eliminate.

Additionally, I have a couple lines that I've added to manage updating subscription status for plans on our ActiveRecord models that I think might be adding some additional complexity to the class that I don't know if that's a good practice or if there is another pattern I should be following.

  module Tollbooth
      class Manager
        include Tollbooth::Querying

        attr_reader :business

        def initialize business
          @business = business
        end

        ##################################################
        # Customer and plan details
        ##################################################

        def on_trial?
          current_subscription.trial_ends_at && current_subscription.trial_ends_at >= Time.zone.now
        end

        def trial_ends
          current_subscription.trial_ends_at
        end

        def next_billing_date
          current_subscription.current_period_ends_at
        end

        def set_to_expire?
          current_subscription && current_subscription.canceled_at.present?
        end

        def current_subscription
          @current_subscription ||= Recurly::Subscription.find(business_subscription_uuid)
        end

        def pending_subscription
          return(@pending_subscription) if defined?(@pending_subscription)
          @pending_subscription ||= begin
            current_subscription.pending_subscription
          rescue Recurly::Resource::NotFound
            nil
          end
        end

        def current_plan
          @current_plan ||= Recurly::Plan.find(current_subscription.plan_code)
        end

        def pending_plan
          @pending_plan ||= Recurly::Plan.find(pending_subscription.plan_code)
        end

        def billing_info
          @billing_info ||= account.billing_info
        end

        def transactions
          account.transactions
        end

        ##################################################
        # Changing plan details
        ##################################################

        def change_subscription_to code
          current_amount  = current_plan.unit_amount_in_cents['USD']
          future_amount   = Recurly::Plan.find(code).unit_amount_in_cents['USD']
          timeframe       = current_amount > future_amount ? 'renewal' : 'now'
          success, result = query(current_subscription, :update_attributes, { plan_code: code, timeframe: timeframe })
          business.update(plan: Plan.where(plan_code: code).first) if success
          # The above line is one where I'm not sure if this is how I should be doing this since Plan is another ActiveRecord model
          success
        end

        def start_subscription code
          success, result = query(account.subscriptions, :create, { plan_code: code })
          business.update(subscription_uuid: result.uuid, subscription_state: 'active', plan: Plan.where(plan_code: code).first) if success
   # The above line is one where I'm not sure if this is how I should be doing this since Plan is another ActiveRecord model
          success
        end

        def cancel_subscription
          success, result = query(current_subscription, :cancel)
          success
        end

        def reactivate_subscription
          success, result = query(current_subscription, :reactivate)
          success
        end

        def destroy
          account.destroy
        end

      end
    end

I think the real test of this architecture is the refactoring effort involved in moving away from Recurly. Ideally, you'd just need to rewrite your service layer classes, and none of the rest of your application.

The only major issue I see here is you've got a bunch of public methods that are returning Recurly business objects, which could allow you to couple your entire system to Recurly. Without seeing the rest of the code, I would mark all methods that return Recurly business objects as private or protected, and only expose the methods that are transactional in nature --- and even then I would limit the arguments and return values of those methods to primitive values, or business objects written by your team for this application.

There is one minor nitpick, though. The class name Tollbooth::Manager doesn't really communicate what it does. You describe this as a "service class", so why not name it accordingly: Tollbooth::SubscriptionService or Tollbooth::SubscriptionManager. Normally I would steer people away from naming classes "SomethingManager", but this class does indeed appear to manage things, so I think the "Manager" suffix works here.

If the rest of your application needs data provided by Recurly, I would recommend creating your own classes to represent that data, and then map them over from the Recurly objects (also known as Data Transfer Objects). This would provide a layer of decoupling in case you switch subscription providers in the future.


Select random IDs from a table, using MySQLi I've created a "simple" function that receives a MySQLi resource and, using a custom where query, will fetch random IDs from a provided table.

Some caveats on this:

No object-style! This needs to be understood by people who only know the old mysql_* procedural API;
No order by rand()! It is slow, but very tempting;
It has to allow me to fetch a fixed number of IDs, or all available IDs;
It must always return something, even if it is a simple array;
It is required to allow for a table name to be passed;
A custom where is required, since I need to provide complex queries;
It doesn't really matter if it is a truly random function: pseudo-random is perfectly fine;
No prepared statements: this function will be used alongside some prepared statements. Those will give errors like "Commands out of sync; you can't run this command now";
The code needs to run on PHP 5.3.29.

And here's the code:

<?PHP
    function get_random_ids_from_table(&$link, $table, $where = null, $length = -1)
    {
        $ids_r = mysqli_query($link, 'select id from `' . $table . '`' . ($where ? ' where ' . $where : ''));
        if(!$ids_r)
        {
            return array();
        }

        $count = mysqli_num_rows($ids_r);

        if(!$count)
        {
            return array();
        }

        $ids = array();
        if($length < 1 || $length > $count)
        {
            while($id = mysqli_fetch_assoc($ids_r))
            {
                $ids[] = $id['id'];
            }
            mysqli_free_result($ids_r);
            shuffle($ids);
        }
        else
        {
            while(count($ids) < $length)
            {
                $offset = mt_rand(0, $count - 1);
                if(!isset($ids[$offset]))
                {
                    mysqli_data_seek($ids_r, $offset);
                    $id = mysqli_fetch_assoc($ids_r);
                    $ids[$offset] = $id['id'];
                }
            }
            $ids = array_values($ids);
        }

        mysqli_free_result($ids_r);

        return $ids;
    }

This function works perfectly fine as-is, but it has a problem: it's still slow! It takes 500-1400ms when executed 4 times! I've already reduced its execution time from 3s. (Old version, for reference: http://pastebin.com/ASfBfWmg). And this is to select IDs from 500 rows. I'm expecting that the code runs on 2000+ rows.

How can I improve its horrendous speed? And which other changes you think that are necessary to make it more readable?

In the comments section, we talked about options around the DB queries.  It sounds like ORDER BY rand() may not be suitable for your use case in dealing with those execution times (which still seem odd to me given size of dataset).

So let's assume rather than look at optimizing that randomized query (again there are many good examples of this out there), or caching query results locally in some manner that you find appropriate, we are just going to look at simplifying your code, but sticking with the approach of randomizing the id's in the application.

I would suggest that you test through both sides of your if-else conditional to understand the relative completion speed of the "download all ids and shuffle" vs. the "random row retrieval" side of the conditional.  Ask yourself things like:

Do you really need both sides of this conditional?
What are the actual use cases of your application?
Are you primarily going to be grabbing one or a few rows at a time vs. grabbing entire set of id's?

This may influence your way of thinking about this section of code.  It might even, for example, prompt you to split this function with one function being called to get full result set and another function being called to get partial result set.

You may want to look at randomization approach in the data seek side of the conditional.  If you were trying to retrieve a substantial portion of (but not full) result set, you could waste a lot of time in the while loop trying to find an offset that has not already been set in the result array.

A better approach to randomization might be:

// generate array of offsets to search by getting
// $length number of random offset keys
// assume $length has already been verified to be <= $count
// not shown - you might check whether sorting $target_offsets helps with access time at all
$target_offsets = array_rand(range(0, $count - 1), $length);

for ($i = 0; $i < $length; i++) {
    // seek record and read into $ids array
    mysqli_data_seek($ids_r, $target_offsets[$i]);
    $id = mysqli_fetch_assoc($ids_r);
    $ids[] = $id['id'];
}

Regardless of approach here, you need to understand if at some percentage of full result set, it might be faster to retrieve full array, shuffle and slice to get the desired result vs. random row retrieval.

A few other thoughts in looking at your code:

Always strive to use meaningful variables names.

Consider these examples:

$ids_r => $id_results
$link => $mysqli_link
$length => $record_limit

These immediately give code reader exact understanding of what is stored in the variable.

You are doing nothing to validate the input to your function.  You shoudd consider validating, for example that a valid mysqli resource, non-zero length string for table name and where clause, and integer value for record limit are passed. Throw exception of fail out quickly if these conditions are not met.
Your code has no comments. You should always consider appropriate comments to make your code more readable.
Speaking of more readable code. You should strive to keep your code < 80 chraracters per line.  You only have one questionable example in your code:

Here:

$ids_r = mysqli_query($link, 'select id from `' . $table . '`' .
($where ? ' where ' . $where : ''));

which could be made more readable like:

$where = $where ? ' WHERE ' . $where : '';
$sql = 'SELECT id FROM `' . $table . '`' . $where;
$id_results = mysqli_query($link, $sql);


Accessing server side variables in views and JavaScript I was a PHP developer and I started writing a product in C# without studying the basic structure. Now, I have around 100+ views with similar code. After 1 year into development, I now realize that something is not right.

How can I separate the server tags when they are so intertwined in the code? Is it okay to use server side variables like this?

PS: I am not using Razor in my product.

/Views/Emp/EditEmp.aspx

<% if(HttpContext.Current.User.IsInRole("Manager")) { %>

        Back to Emp Info

<% } %>

. ..... .. .. ...

<% if(Model != null && Model.Holidays != null) {
    foreach(EmpHoliday eh in Model.Holidays){ %>

<%= eh.Date.ToString("dd/MMM/yyyy") %>

<%= eh.Holiday %>

    <% } %>

<% } %>

This is how I see server variables and local variables interacting.  There is not really any other way to get the values between the two that I'm aware of.

As for the code, I one minor point:change Emp to Employee for variable names.  This will eliminate confusion in the future from you, or anybody else reading the code (what does Emp mean?)

Other than that, this is really easy code to read.  Keep it up.


All arithmetic operator functions (+, -, *, /, %) coded only using bitwise operators in C Here's all the functions in C that does all the arithmetic operators without using arithmetic operators themselves, mostly using bitwise operators.  Recursive functions and comparison operators are also allowed.

int add ( int augend, int addend )
{
    return addend ? add ( augend ^ addend, ( augend & addend ) << 1 ) : augend;
}

int subtract ( int minuend, int subtrahend )
{
    return add ( minuend, add ( ~subtrahend, 1 ) );
}

int multiply_recursive ( int multiplicand, int multiplier, int product )
{
    if ( !multiplier ) return product;
    if ( multiplier & 1 ) product = add ( multiplicand, product );
    return multiply_recursive ( multiplicand << 1, (int)( (unsigned) multiplier >> 1 ),
                                product );
}

int multiply ( int multiplicand, int multiplier )
{
    return multiply_recursive ( multiplicand, multiplier, 0 );
}

int divide_recursive ( int dividend, int divisor, int size, int quotient, int remainder )
{
    if ( !size ) return quotient;
    int position = subtract ( size, 1 );
    remainder <<= 1;
    remainder |= ( dividend >> position ) & 1;
    if ( remainder >= divisor ) {
        remainder = subtract ( remainder, divisor );
        quotient |= 1 << position;
    }
    return divide_recursive ( dividend, divisor, position, quotient, remainder );
}

int abs ( int integer )
{
    int const mask = integer >> subtract ( multiply ( sizeof(int), 8 ), 1 );
    return add ( integer, mask ) ^ mask;
}

int divide ( int dividend, int divisor )
{
    int quotient = divide_recursive ( abs ( dividend ), abs ( divisor ),
                                      multiply ( sizeof(int), 8 ), 0, 0 );
    return ( dividend ^ divisor ) < 0 ? add ( ~quotient, 1 ) : quotient;
}

int mod ( int dividend, int divisor )
{
    return subtract ( dividend, multiply ( divide ( dividend, divisor ), divisor ) );
}

Don't forget

int mod ( int dividend, int divisor )
{
    return subtract ( dividend, multiply ( divide ( dividend, divisor ), divisor ) );
}

This seems the long way around.  Your divide_recursive calculates the remainder, but you forget it and then recalculate it.

Instead, consider

int mod( int dividend, int divisor )
{
    int remainder = 0;
    divide_recursive(abs(dividend), abs(divisor), multiply(sizeof(int), 8), 0, &remainder);
    return dividend < 0 ? add(~remainder, 1) : remainder;
}

That requires changing divide_recursive to take a pointer instead:

int divide_recursive( int dividend, int divisor, int size, int quotient, int *remainder )
{
    if ( !size ) return quotient;
    int position = subtract( size, 1 );
    *remainder <<= 1;
    *remainder |= ( dividend >> position ) & 1;
    if ( *remainder >= divisor ) {
        *remainder = subtract( *remainder, divisor );
        quotient |= 1 << position;
    }

    return divide_recursive( dividend, divisor, position, quotient, remainder );
}

And we need to change divide to call it correctly:

int divide( int dividend, int divisor )
{
    int remainder = 0;
    int quotient = divide_recursive( abs( dividend ), abs ( divisor ),
                                      multiply( sizeof(int), 8 ), 0, &remainder );

    return ( dividend ^ divisor ) < 0 ? add ( ~quotient, 1 ) : quotient;
}

Too cute?

int abs ( int integer )
{
    int const mask = integer >> subtract ( multiply ( sizeof(int), 8 ), 1 );
    return add ( integer, mask ) ^ mask;
}

This also seems more complicated than necessary.  Why not just say

int abs(int value)
{
    return (value > 0) ? value : add(~value, 1);
}

Why use three functions and two bitwise operations where a conditional chance of one function call is enough?

Testing

How do you know that your implementation is correct?  The correct way is to write unit tests, but at minimum you need to compare your bitwise operations to the built-in operations.  I wrote the following function to make sure that I was doing the math right (particularly when dealing with negative numbers):

void display_remainder(int dividend, int divisor) {
    printf("%d%%%d=%d=%d\n", dividend, divisor, mod(dividend, divisor), dividend % divisor);
}

This is not the pinnacle of coding practices, but it easily lets me see that my revised version is giving the same results as the built-in.  A better, more persistent solution would be unit tests like

CU_ASSERT(-2%-3 == mod(-2, -3));

or possibly better (albeit harder to write):

CU_ASSERT(-2 == mod(-2, -3));

But either way, what's important is that you compare your results to known good results.  If you publish your tests when you ask for a review, it makes it easier for us to test changes we want to propose and to suggest additional tests that show problems in the original code.


Cryptographic Random Number Generator function implementation While surfing and through some experimentation, I have a new method to generate random numbers without using Randomize or Random or RND. It uses RNGCryptoServiceProvider Class for generating random numbers. We can generate a wide range of positive and negative random numbers with this. I built a function which generate positive and negative random numbers.I wish to share that code with you to make it much better.

We need to import Imports System.Security.Cryptography file to work with the function. Here is the code:

Public Sub Randomgenerator()
    Dim byte_count As Byte() = New Byte(6) {}
    Dim random_number As New RNGCryptoServiceProvider()
    random_number.GetBytes(byte_count)
    Dim Output As Integer = BitConverter.ToInt32(byte_count, 0)
    MsgBox(Output)
End Sub

Suggestions and tips to improve this function are invited.

This will be a better random number generator than the other options
you mentioned, yes.
The code is nicely indented, but some whitespace between variable declarations and logic would be nice.

Public Sub Randomgenerator()
    Dim byte_count As Byte() = New Byte(6) {}
    Dim random_number As New RNGCryptoServiceProvider()

    random_number.GetBytes(byte_count)
    Dim Output As Integer = BitConverter.ToInt32(byte_count, 0)

    MsgBox(Output)
End Sub

Method names should be PascalCased, so Randomgenerator should be RandomGenerator. Don't use underscore in variable/parameter names. They should be camelCased. Please see the official MSDN capitalization recommendations for more information.
This is begging to be a function that returns a Integer. There should be a main routine that calls and displays the generated number.

Public Function RandomGenerator() As Integer
    Dim byteCount As Byte() = New Byte(6) {}
    Dim randomNumber As New RNGCryptoServiceProvider()

    randomNumber.GetBytes(byteCount)
    Return BitConverter.ToInt32(byteCount, 0)
End Function

Sub Main()
    MsgBox(RandomGenerator())
End Sub

MsgBox is depreciated and left only for backwards compatibility with vb6. Replace it with a MessageBox.
RNGCryptoServiceProvider implements IDisposable. This means that you need to properly dispose randomNumber. From the documentation link you provided:

  This type implements the IDisposable interface. When you have finished using the type, you should dispose of it either directly or indirectly. To dispose of the type directly, call its Dispose method in a try/catch block. To dispose of it indirectly, use a language construct such as using (in C#) or Using (in Visual Basic). For more information, see the “Using an Object that Implements IDisposable” section in the IDisposable interface topic.


Securely implementing AES/RSA hybrid crypto with PyCryptodome I wondering if anyone could give me feedback on whether or not this is a secure way to implement AES/RSA hybrid encryption and signatures in python 3 with the PyCryptodome module.

All of the encoding and to/from hex format is just there because that's the format I intend to transfer it in.

"""Hybrid AES/RSA encryption, integrity, and repudiation proof of concept.
    Requires the PyCryptodome module but is imported as Crypto"""

from hashlib import sha512
from Crypto.Cipher import PKCS1_OAEP
from Crypto.Cipher import AES
from Crypto.PublicKey import RSA
from Crypto.Random import get_random_bytes

def generate_keys():
    """ Generates the rsa key pair and returns them.
        In actual usage private key is exported to .pem secured with a passphrase"""
    while True:
        privatekey = RSA.generate(2048)
        publickey = privatekey.publickey()

        return privatekey, publickey

def encrypt_msg(msg_body, publickey, privatekey):
    """Generates the session key, then creates the nonce and cipher.
       encrypts the message body with AES 256.
       encrypts the session key with RSA 2048
       returns the cipher text, tag, nonce, and the encrypted session key in hex format
       Hashes clear text message and creates signature"""
    session_key = get_random_bytes(32)

    aes_cipher = AES.new(session_key, AES.MODE_EAX)
    nonce = aes_cipher.nonce
    aes_cipher_text, tag = aes_cipher.encrypt_and_digest(msg_body)

    rsa_cipher = PKCS1_OAEP.new(publickey)
    enc_session_key = rsa_cipher.encrypt(session_key)

    msg_hash = int.from_bytes(sha512(msg_body).digest(), byteorder='big')
    signature = pow(msg_hash, privatekey.d, privatekey.n)

    return aes_cipher_text.hex(), tag.hex(), nonce.hex(), enc_session_key.hex(), signature

def decrypt_msg(aes_cipher_text, tag, nonce, enc_session_key, privatekey, publickey, signature):
    """encrypted session key is encoded and returned from hex
       Uses private key to decrypt the session key
       nonce is encoded and returned from hex
       cipher is created with supplied key, and nonce
       cipher text is decrypted
       tag is encoded and returned from hex
       message is checked for authenticity
       clear text is returned and authenticity status is returned
       hashes clear text and compares to signature
       """

    decrypt = PKCS1_OAEP.new(privatekey)
    session_key = decrypt.decrypt(enc_session_key.encode().fromhex(enc_session_key))

    aes_cipher = AES.new(session_key, AES.MODE_EAX, nonce=nonce.encode().fromhex(nonce))
    clear_text = aes_cipher.decrypt(aes_cipher_text.encode().fromhex(aes_cipher_text))

    try:
        aes_cipher.verify(tag.encode().fromhex(tag))
        authentic = True

    except ValueError:
        authentic = False

    msg_hash = int.from_bytes(sha512(clear_text).digest(), byteorder='big')
    hashfromsignature = pow(signature, publickey.e, publickey.n)
    if msg_hash == hashfromsignature:
        valid = True

    else:
        valid = False

    return clear_text, authentic, valid

TEST_MSG = b'this is a short test message.'

# Generates and assigns public and private keys
# Person A's key pair
PRIVATE_KEY, PUBLIC_KEY = generate_keys()
# Person B's key pair
PRIVATE_KEY_1, PUBLIC_KEY_1 = generate_keys()

# Encrypts the message, and creates the tag, nonce and encrypted session key
# Note same key pair is used only for testing purposes in implementation
# both sides will have own key pairs
msg_aes_cipher_text, msg_tag, msg_nonce, encrypted_session_key, signature = encrypt_msg(TEST_MSG, PUBLIC_KEY_1, PRIVATE_KEY)

# Decrypts the message with the supplied data and check for authenticity
msg_clear_text, authentic_status, valid = decrypt_msg(msg_aes_cipher_text, msg_tag, msg_nonce, encrypted_session_key, PRIVATE_KEY_1, PUBLIC_KEY, signature)
if valid:
    print('Valid signature.')
else:
    print('Invalid signature!')
if msg_tag:
    print('Valid tag.')
else:
    print('Invalid tag.')
print(msg_clear_text)
```

While this question is an off-topic programming question on Crypto.SE, I doubt whether it'll get a better answer on StackOverflow or Code Review.

The part worries me is this:

...
    msg_hash = int.from_bytes(sha512(clear_text).digest(), byteorder='big')
    hashfromsignature = pow(signature, publickey.e, publickey.n)
    if msg_hash == hashfromsignature:
        valid = True

    else:
        valid = False
...

You seem to be verifying unpadded textbook RSA signature. You should change this part to using RSA-PSS signature scheme - it's a newer RSA signature scheme in PKCS#1 ver2.


My lost password function I'm trying to create a lost password function on my site. When a user enter their email it updates 2 columns in the users table with that email called: new_password_key and new_password_requested (int, using PHP time()), I then send them a link with that password key to a page where they may change their password.

It seems to work, and if they don't visit the link within 24 hours it's considered invalid and they have to request a new one.

Is it OK? Safe? Is there something else I should think of?

This is the https://mysite.com/password/key_goes_here page:

$value = isset($_GET['key']) ? $_GET['key'] : null;
$query = $db->prepare("SELECT new_password_key, new_password_requested FROM users WHERE new_password_key = ?");
$query->execute(array($value));
$row = $query->fetch();

if (!$row) {
    // not found
    header("Location: /password");
    die;
}

// if this key is older than 24 hours its invalid (expired)
if ($row['new_password_requested'] <= time() - (60 * 60 * 24)) {
    $query = $db->prepare("UPDATE users SET new_password_key = NULL, new_password_requested = 0 WHERE new_password_key = ?");
    $query->execute(array($value));
    header("Refresh: 3;url='/password'");
    return false;
}

if (count($_POST) > 0) {

    $new_password1 = trim($_POST['new_password1']);
    $new_password2 = trim($_POST['new_password2']);

    if (strlen($new_password1) < 4)
        $error = "passwords must be at least 4 characters long. please choose another (longer) password";
    else if ($new_password1 != $new_password2)
        $error = "passwords do not match";

    if (empty($error)) {
        $hash = password_hash($new_password1, PASSWORD_DEFAULT);
        $query = $db->prepare("UPDATE users SET password = ?, new_password_key = NULL WHERE new_password_key = ?");
        $query->execute(array($hash, $row['new_password_key']));

    }

}

This is the form where you enter your email to get a key:

if (count($_POST) > 0) {

$email = trim($_POST['email']);

$query = $db->prepare("SELECT email, new_password_key, new_password_requested FROM users WHERE email = ?");
$query->execute(array($email));
$row = $query->fetch();

if (empty($email))
    $error = "this field can not be empty";
else if (!filter_var($email, FILTER_VALIDATE_EMAIL))
    $error = "not a valid email address";
else if (!empty($row['new_password_key']))
    $error = 'a link has already been sent';
elseif ($query->rowCount() < 1)
    $confirmed = true;

if (empty($error)) {

    if ($row) {
        // email exists, send them mail
        $query = $db->prepare("UPDATE users SET new_password_key = ?, new_password_requested = ? WHERE email = ?");
        $query->execute(array(generateRandomString(25), time(), $row['email']));
        sendMail($email, 'lostpassword');
        $confirmed = true;
    } else {
       // email dont exist, tell them an email has been dispatched but dont actually send a mail we dont want them to know what emails are registered on this site
       $confirmed = true;
    }
}
}

This seems quite fine. It's good that you are using prepared statements for all database queries.

What bugs me the most is that the users table is reused for multiple purposes:

Store essential user information such as email and password
Store information about tracking reset password requests

It would be better to use another table for the 2nd purpose.
The reason is that the users table is an essential piece of your site and you wouldn't want anything to happen to it by accident.
So the less you touch that table, the better.
Besides, for most users the new_password_key and new_password_requested fields will be blank / unused most of the time.

The variable names are sometimes not so great. For example:

$value = isset($_GET['key']) ? $_GET['key'] : null;

$value is a key, so I suggest to name it accordingly.

One last small thing, I recommend to use braces with all if, else if statements.


Extracting data from vue-chartjs into an array of datasets I have a function which returns an array of datasets for a chart written with vue-chartjs. The getter getChartData returns an array with three objects with nested objects. I see that my function has a so called 'callback hell' and I want to refactor this.

getDatasets() {
  let datasetLabels = [];
  const obj = this.getChartData;
  Object.entries(obj).forEach((entry) => {
    datasetLabels = [...datasetLabels, ...Object.keys(entry[1])];
  });
  let datasets = [];
  [...new Set(datasetLabels)].forEach((item, index, arr) => {
    datasets.push({ label: item, backgroundColor: this.getColor(arr, index), data: [] });
  });
  Object.entries(obj).forEach((el) => {
    const datasetData = Object.entries(el[1]);
    datasetData.forEach(
      (item) => {
        datasets = datasets.map((e) => {
          if (e.label === item[0]) {
            e.data.push(item[1].median);
          }
          return e;
        });
      });
  });
  return datasets;
},

False alarm! No callback hell here.

While this code can certainly be refactored a bit, I fail to see callback hell anywhere around. Callback hell is normally a term applied to the code that is executed asynchronously and with multiple nested callback functions. The nested structure is what causes a few issues:

cognitive complexity (which leads to bugs and maintenance complication);
refactoring difficulty;
more importantly, hard time expressing conditional ("branching") or cyclic ("loops") logic.

A related question on StackOverflow might be helpful (just don't focus on RxJs part of the answer there).

Your code does actively use arrow functions, but it is not equivalent to callback hell. The number one reason is because everything in getDatasets() is executed synchronously.

Refactoring

Nitpicking

Small details like line breaks are powerful. Splitting the function into paragraphs with empty line separators helps identifying logical steps you're making in code. It also can work help in extracting sub-functions later, if/when the function becomes too long.
Naming things: Spelling things out is always better than using names like el, e, arr. Abbreviated variable names like that suggest that these entities have no reason to exist or are unimportant. Even item is not a good name. Since it denotes a label, call it a label. I'm sure you got the idea.
Naming continued: Name your variables consistently. You iterate two times over the same object: Object.entries(obj).forEach(...) in one case you call the current entry as entry, while in the other – just el. That is confusing.
I like how you lock down the chart data in a locally scoped obj variable. Renaming it to chartData would make it more readable, though.
You could also avoid let in a few places and have const instead. For example, reduce(...) or map(...) are in many cases a better alternative to for (...) or .forEach(...).
Also, be careful when you deal with array deconstruction. While it's readable, it's also very inefficient because it creates a new array by iterating all the elements of the source arrays. It's only fine to use it for pieces of code that are not invoked frequently and/or operate on tiny arrays only.

Applying the suggestions to the code

I could only rewrite the first half of the function. The second half requires a broader context to make a meaningful change to it.

As you can see, I failed to rename e into something descriptive, because I'm unaware of the models you are using in your application. Hopefully, even this "mechanical" refactoring is able to give you some ideas on how the code can be improved.

getDatasets() {
  const chartData = this.getChartData;

  const datasetLabels = Object.values(chartData);

  // `datasets` could be a const, if only it wasn't modified further down the road...
  let datasets = [...new Set(datasetLabels)]
    .map((label, labelIndex, array) => ({ label, backgroundColor: this.getColor(array, labelIndex), data: [] }));

  // This is a subject to a few more changing, maybe...
  Object
    .entries(chartData)
    .forEach((entry) => {
      Object.entries(entry[1]).forEach((dataItem) => {
          datasets = datasets.map((e) => {
            if (e.label === dataItem[0]) {
              e.data.push(dataItem[1].median);
            }
            return e;
          });
        });
    });

  return datasets;
}

Update As @Gerrit0 suggested, Object.values(...) is a better option than Object.entities(...).map(...) (more concise and achieves exactly the same result).


How can I make this CSV Importer code better of any code smell? I wrote this method a few years ago in a Ruby on Rails project, which I think I am not proud of. How can I make this code better to show in-depth professionalism in Ruby?
lib/merchant/web_csv_importer.rb
# frozen_string_literal: true

require 'open-uri'
require 'csv'

class WebCsvImporter
  attr_accessor :url

  def initialize(url)
    @url = url
  end

  def call
    data = URI.open(url).read.force_encoding('UTF-8')

    parse_csv(data)
  end

  private

  def parse_csv(data)
    counter = 0
    duplicate_counter = 0

    CSV.parse(data, headers: true, header_converters: :symbol) do |row|
      next unless row[:name].present? && row[:email].present? && row[:status].present?

      role = Role.find_or_create_by(name: 'admin')
      merchant = Merchant.new row.to_h
      merchant.roles << role
      merchant.save

      merchant.persisted? ? counter += 1 : duplicate_counter += 1
      p "Email duplicate record: #{merchant.email} - #{merchant.errors.full_messages.join(',')}" if merchant.errors.any?
    end

    p "Imported #{counter} merchant, #{duplicate_counter} duplicate rows ain't added in total" # dont forget to pluralize this statements
  end
end

lib/tasks/merchant/csv_importer.rake
require 'merchant/web_csv_importer'

namespace :merchant do
  desc "Imports data from a Web CSV into Merchant's table"
  task web_csv_importer: :environment do
    WebCsvImporter.new('http://expample.com/merchant.csv').call
  end
end

I like that you already extracted a WebCsvImporter Plain Old Ruby Object which makes the code already a lot easier to understand and refactor.
Here is a refactored solution which I will explain in more detail:
class CsvImporter
  Result < Struct.new(:merchants) do
    def valid_count
      @valid_count ||= merchants.count(&:persisted?)
    end

    def invalid_count
      @invalid_count ||= total - valid_count
    end

    def total
      @total ||= merchants.count
    end

    def errors
      merchants.map do |merchant|
        merchant.errors.full_messages.to_sentence
      end.to_sentence
    end
  end

  def initalize(url)
    @url = url
  end

  def call
    merchants = valid_rows.map do |row|
      Merchant.create(
        roles: [Role.find_or_create_by(name: 'admin')],
        name: row[:name],
      )
    end

    Result.new(merchants)
  end

  private

  attr_accessor :url

  def valid_rows
    CSV.parse(data, headers: true, header_converters: :symbol).select do |row|
      row[:name].present? && row[:email].present? && row[:status].present?
    end
  end

  def data
    @data||= URI.open(url).read.force_encoding('UTF-8')
  end
end

Note that I removed the p prints and return a result object instead. You can use this result object to print the output in different formats or silence. Another solution could be to write to a logger object but you should avoid using puts as it will make testing and reusing of this class harder.
Here are a couple of suggestions:
Replace Temp with Query
Replace Temp with Query is a standard refactoring method which often improves the code quality a lot. Here we use the temp variable to a method which will also remove the need for a method parameter of parse_csv.
def data
  @data||= URI.open(url).read.force_encoding('UTF-8')
end

Extract helper methods
Extrating helper methods is also a standard refactoring method. Your parse_csv method is quite long and complicated so we should think about splitting it apart. One thing I did was extracting skipping invalid rows like this.
def valid_rows
  CSV.parse(data, headers: true, header_converters: :symbol).select do |row|
    row[:name].present? && row[:email].present? && row[:status].present?
  end
end

Don't use to_h to create object
Rather than using to_h and pass the all params to the object creation I would rather only pass in allowed parameters. I just used name as example here. It is problematic to pass all parameters in as it could easily break your application (e.g. CSV and database always need to have the same format) or worst case this could be a security issue (e.g. Merchant has an is_admin or password attribute which could get overwritten) without noticing. Specifying which params are allowed here seems like a good idea.
Result object
I return a result object and do the counting logic in the result object to make it simpler. This also has the advantage to move the printing / output to a different layer making testing a lot easier. For instance this test
def test_valid_count
  result = CsvImporter.new('example.com').call

  assert_equal(1, result.valid_count)
end

seems a lot easier and robust than
def test_valid_count
  io = capture_io do
    CsvImporter.new('example.com').call
  end

  assert_equal("Imported 1 merchant, 1 duplicate rows ain't added in total", io)
end

Make attr_accessor private
If the attr_accessor are not accessed externally you should move them to be private.
Hope that helps, let me know what you think.
Update

Reading a little further, I am concerned about performance issues when the data I want to pull is large and save into database. An example is CSV.parse which surely will result into issues when the csv dataset is large. Though I do not know the .select you append to it will solve that issue.

Reading files generally is quite fast. If you have of course 100s of millions of rows this can become a performance bottleneck. Generally I would advise to do benchmarks with your input files to see if this is really a bottleneck rather than premature optimisations.
However, here are a few ideas what you could do:

Divide & Conquer: Rather than having one single CSV file you could split the file into many files and start an import worker for each of them. Ideally this would happen already when generating the CSV files because then you also distribute the download time (rather than e.g. download 1GB you download 10 * 100MB).

Read the CSV file line by line

Also, I think I should move row[:name].present? && row[:email].present? && row[:status].present? to a separate predicate method.

Personally I don't think this will improve much readability because you would end up with either having a method with parameter or need to extract another object which both are not ideal.
# need a method parameter
def valid_row?(row)
  row[:name].present? &&
    row[:email].present? &&
    row[:status].present?
end

# extract object
class Row
  def initialize(params)
    @params = params
  end

  def valid?
    params[:name].present? &&
      params[:email].present? &&
      params[:status].present?
  end
end

Row.new(row_params).valid?

Also what happens if those rows are not present.

This shouldn't be a problem as nil.present? will return false.


Fetch: how to deal with a json payload in an error response? Trying to get https://github.com/github/fetch working with an API that almost always returns a json payload, even when the response is an error:

{
  "message": "User not logged in",
  "code": 401
}

So I came up with this, adopting their status check example

function checkStatus(response) {
  if (response.status >= 200 && response.status < 300) {
    return response
  } else {
    return response.json()
      .catch(() => {
        var error = new Error(response.statusText)
        error.response = response
        throw error
      }).then((json) => {
        var error = new Error(json.message || response.statusText)
        error.response = response
        throw error
      });
  }
}

This would handle both json and an invalid json reponse in case the api isn't able to produce valid json.

Would this be okay or am I missing something? I don't really like the .catch before the .then, but otherwise the .catch will catch the thrown error.. which is not what we want..

Note this reads more like a hypothetical question but I'm giving you the benefit of the doubt.

Your example is alright, but it has a few issues. I'm going to mostly touch on semantics here, so bear with me. Note I'll be going with the WHATWG spec that's implemented in browsers now, which should be the same as the repo you've linked, but just so you are aware :)

  if (response.status >= 200 && response.status < 300) {
    return response

There's no need to check response.status like this; it also doesn't cover all "success" responses (assuming that is indeed what you're trying to do. WHATWG fetch exposes an ok property on the response object which will do this for you..

  if (response.ok) {
    return response

Let's look at your function as a whole.

function checkStatus(response) {
  if (response.ok) {
    return response
  } else {
    ...
  }
}

As this is just a straight up if-else, you can reduce cyclomatic complexity by dropping the else here.

  return response.json()
    .catch(() => {
      var error = new Error(response.statusText)
      error.response = response
      throw error
    }).then((json) => {
      var error = new Error(json.message || response.statusText)
      error.response = response
      throw error
    });

I think this is a bit redundant. I'm guessing what you're trying to do is either:

Catch, and re-throw (thus rejecting the promise), the error with the response property if you hit this branch if there was already an exception
Reject the promise by throwing an error if there was not an exception by taking the response json and assigning it as an error message.

I'd argue that you should not be catching the error in the first place. The only place an error will occur here is if there is a JSON syntax error. There's nothing you can do to handle this, so don't attach bits and pieces to it; the only thing it would be useful for is debugging, and you shouldn't be adding debugging stuff like this to the user's browser - this should be caught in e2e tests. Instead, just let the default error propagate up. This leaves you with this so far:

function checkStatus(response) {
  if (response.ok) {
    return response
  }
  return response.json().then((json) => {
    var error = new Error(json.message || response.statusText)
    error.response = response
    throw error
  });
}

The next thing I have to add is this function could either be synchronous or asynchronous depending on what is passed to it. This is a bad thing and is explained here by more clever people than I. The upshot is that you should design your API so that if it can return asynchronously, it always should. Solution to this is just to wrap response in a Promise.resolve().

function checkStatus(response) {
  if (response.ok) {
    return Promise.resolve(response)
  }

  return response.json().then((json) => {
    var error = new Error(json.message || response.statusText)
    error.response = response
    throw error
  });
}

Next:

Your semi colons aren't consistent. Either use them or don't. Don't mix and match, it just confuses everyone.
If you're using ES6 arrows, you can use const/let as well. Use them.

You could simplify your promise handler code by using something like Object.assign if you support it or _.assign otherwise, which makes things nice and sugary.

function checkStatus(response) {
  if (response.ok) {
    return Promise.resolve(response)
  }

  return response.json().then(json => {
    const error = new Error(json.message || response.statusText)
    throw Object.assign(error, { response })
  })
}

Consider using Promise.reject instead of throw. It's a standard API for promises and clearly communicates your intent. Unwinding the stack trace isn't fun.

function checkStatus(response) {
  if (response.ok) {
    return Promise.resolve(response)
  }

  return response.json().then(json => {
    const error = new Error(json.message || response.statusText)
    return Promise.reject(Object.assign(error, { response }))
  })
}

I think that's about it for now. I think it's weird you're doing either json.message || response.statusText as an error message, but whatever floats your boat. Error reporting messages are largely what is helpful to the developer. I'd recommend you put user-friendly error message handling somewhere, though.


Generalisation of Binary Search I wrote a function to implement a generalised version of binary search that can find the minimum or maximum index in a given sorted list that satisfies a provided predicate.

My Code

def xbinsearch(pred, lst, type = "min", default = None):
    """
        * Finds the minimum or maximum index of a sorted list such that the value at that index satisfies a given predicate.

        *   Params:
            *   `pred`: Predicate function.
                *   Params:
                    *   `idx`: The index of the value to be tested.
                    *   `lst`: The list to search for said value in.
                *   Return: An ordered pair of the form `(x, y)`.
                    *   `x`: Boolean indicates whether or not `lst[idx]` satisfies the predicate.
                    *   `y`: `-1`, `1` or `None`.
                        *   `-1` indicates that if `lst[idx]` does not satisfy the predicate, then all values below `lst[idx]` also do not satisfy the predicate.
                        *   `1` indicates that if `lst[idx]` does satisfy the predicate, then all values above `lst[idx]` also do not satisfy the predicate.
                        *   `None` is given with `True` for the first index of the ordered pair.
                        *   It is used to determine which "half" to discard in the binary search.
    """
    low, hi, best = 0, len(lst)-1, default
    while low <= hi:
        mid = (low+hi)//2
        p = pred(mid, lst)
        if p[0]:    #The current element satisfies the given predicate.
            if type == "min":
                if best == default or lst[mid] < lst[best]: best = mid
                hi = mid-1
            elif type == "max":
                if best == default or lst[mid] > lst[best]: best = mid
                low = mid+1
        elif p[1] == 1:     #For all `x` > `lst[mid]` not `P(x)`.
            hi = mid - 1
        elif p[1] == -1:    #For all `x` < `lst[mid]` not `P(x)`.
            low = mid + 1
    return best

Sample Use Case

Problem
Element equals its index

  Given a sorted array of distinct integers, write a function index_equals_value that returns the lowest index for which array[index] == index.
  Return-1` if there is no such index.
  Your algorithm should be very performant.

  [input] array of integers ( with 0-based nonnegative indexing )
  [output] integer

  Examples:

input: [-8,0,2,5]
output: 2 # since array[2] == 2

input: [-1,0,3,6]
output: -1 # since no index in array satisfies array[index] == index

  Random Tests Constraints:

  Array length: 200 000

  Amount of tests: 1 000

  Time limit: 1.5 s

My Code

def identity(idx, lst):
    tpl, val = [None, None], lst[idx]
    tpl[0] = val == idx
    if val < idx:   tpl[1] = -1
    elif val > idx: tpl[1] = 1
    return tuple(tpl)

def xbinsearch(pred, lst, type = "min", default = None):
    low, hi, best = 0, len(lst)-1, default
    while low <= hi:
        mid = (low+hi)//2
        p = pred(mid, lst)
        if p[0]:    #The current element satisfies the given predicate.
            if type == "min":
                if best == default or lst[mid] < lst[best]: best = mid
                hi = mid-1
            elif type == "max":
                if best == default or lst[mid] > lst[best]: best = mid
                low = mid+1
        elif p[1] == 1:     #For all `x` > `lst[mid]` not `P(x)`.
            hi = mid - 1
        elif p[1] == -1:    #For all `x` < `lst[mid]` not `P(x)`.
            low = mid + 1
    return best

def index_equals_value(lst):
    return xbinsearch(identity, lst, default = -1)

You may be a little confused about how a binary search operates. Your doc comment seems to suggest you understand and my cursory look at your code didn't reveal any bugs (but then again, I didn't look too hard). But, this function doesn't really make logical sense.

The purpose of a binary search is to find an item in O(log(n)) time instead of doing a full scan of a linear datastructure (or at least scanning until you find the item, which worst case is O(n)). This of course requires the datastructure to already be sorted. Your comment indicates you understand this. But what you end up doing with pred makes it seem like you confused yourself.

Why? Well if pred is not a constant time operation, then your binary search is no longer O(log(n)) (I suspect this is the case, because instead of passing lst[mid] to pred you pass the entire lst and index mid). Let's say pred is O(n). Now the total runtime of xbinsearch is O(n*log(n)), which is significantly worse. In this case, a linear search may be better.

Now, let's just assume pred is a constant time function. Now the overall runtime remains O(log(n)), but the return value of pred still doesn't make sense. Why is this? Well, the list must already be sorted by some ordering. If pred is constant time, it can only make decisions about the value at mid (and maybe a few surrounding values), but it can't scan lst to make decisions. So this means there must be some sort of dependency between the return value of pred and the sort order of lst. If there wasn't, your search probably wouldn't work (if say lst was sorted by ascending value but pred negated the values before comparing them, it would throw out the wrong half of the list).

So now we assume that pred is a constant time function that has some dependency on the sort order of lst. We've shown previously that if either of these doesn't hold, your algorithm is either inefficient or breaks. We now arrive at the last issue. pred's comparison criteria must be monotone over the sorted list. That is just a fancy way of saying that when lst is sorted then pred's comparison criteria must also be sorted. In which case, there isn't a purpose for pred, since the value of the item itself (which lst is sorted by this item value) is a proxy for the pred comparison criteria.

If you didn't follow all of that, don't be too worried. The important point is that you're probably misusing a binary search here. If you want to find the smallest value satisfying a predicate in a list you can just do: min(lst, key=pred).

For example:

people = [
    # (name, age)
    ('Doe Jane', 52),
    ('John Doe', 33),
    ('Jane Doe', 42),
]

# Min tuple from people by age
youngest_person = min(people, key=lambda p: p[1])

Note that with namedtuple this example becomes even more clear:

from collections import namedtuple

Person = namedtuple('Person', 'name age')

people = [
    # (name, age)
    Person('Doe Jane', 52),
    Person('John Doe', 33),
    Person('Jane Doe', 42),
]

youngest_person = min(people, key=lambda p: p.age)

Now you may notice people is not sorted. You could pass in a sorted list though, but it doesn't make much sense to sort the list, because it's not like you can perform multiple of these operations. min of a list (that you don't modify) will always be the same. If you change key then you'd need to resort the list. If you wanted both the min and max, use min and max. If performance is of concern (which is unlikely), you could optimize this by having one function that scans over the entire list and keeps track of both the min and max values. If you want to modify the list, then you're probably better off using a heap, which remains sorted with inserts and removals.

Some extraneous issues:

default=None isn't a very common pattern in Python. Sure you see it with dict.get (and less commonly with iter), but those are the two places where I can think of. These places have appropriate uses of allowing a default value. For your function a default value doesn't make much sense. Since it returns an index, what use would a "default index" be? I see two options:

return None (and only None). This simplifies the interface you provide. Allowing for a default value makes it hard for a user of your function to understand what's going on (it's just one more thing to wrap your head around). This way you can explain the function in a sentence: "Returns the minimum or maximum index of the item satisfying a predicate in a sorted list, or None"
Raise an exception (say ValueError) if no items satisfy the predicate. This is a very common approach in Python: dicts raise KeyError if a key doesn't exist (instead of returning None), lists raise IndexError if an index is out of bounds (instead of returning None). This is probably the right choice for you, because a predicate not being satisfied by anything in the list provided is probably an exceptional case that needs to be handled differently (eg. if you were to return None you couldn't treat it like a number if you wanted to do indexing with it).

Your use of string constants to indicate the mode (type="min") while common in numpy is a dangerous pattern IMO. You do no assert type in ('min', 'max'), so what if someone passes in type="median"? What if they make a typo: type="mIn"? Both of these appear to lead to behavior you don't want. Worst case (although your code doesn't appear to do this), this could cause an error inside your function. How frustrating would that be for a user of your function to see an error inside your function? This forces them to understand its internals to see if this is a bug in your function or a bug in their use of it. At the least, prefer an Enum so that if they choose an invalid type they get a AttributeError (if they tried to do type=SearchMode.PENULTIMATE, for example).

PEP8 your code. Your spacing needs work. Also wrap to 79 chars, it's really annoying to need to scroll right to read the docs. They're almost surely indented too far.

Instead of doing p[0] and p[1], consider tuple unpacking to give those values more descriptive names:

predicate_satisfied, direction = pred(mid, lst)

You should probably use required kwargs for type and default, otherwise, the function call site becomes unclear. Eg. is it xbinsearch(my_pred, 'min', 1) or xbinsearch(my_pred, 1, 'min'). Much clearer: xbinsearch(my_pred, type='min', default=1):

def xbinsearch(pred, lst, *, type='min', default=None):
    # ...


FizzBuzz with more parameters in Swift Here's my approach to an extensible version of the FizzBuzz challenge in Swift (so one can add more numbers to be checked against other than just 3 and 5).

I initially tried using dictionaries but since they don't necessarily preserve the order of elements it could lead to BuzzFizz return values.

I personally think that, while the program does work as expected, it isn't clear how it works. I mean, it's not as readable as I'd like it to be for doing such a simple task.

How can I improve it?

func fizzBuzz (n: Int, responsesByMultiples: [(Int, String)] = [(3, "Fizz"), (5, "Buzz")]) -> String {
    var result: String = ""

    for key in responsesByMultiples {
        let (multiple, response) = key,
            isMultiple: Bool = (n % multiple) == 0

        if isMultiple {
            result += response
        }
    }
    return result == "" ? String(n) : result
}
for i in 1...100 {
    print(fizzBuzz(n: i))
}

Bright side

Your code gives the correct output and delivers on the premise of extensibility.

 Suggestions

Here are some suggestions:

The name key is a name that doesn't tell a lot about the nature of the tuple. If you feel that it is appropriate, that would conflict with responsesByMultiples. Then It would have made more sense for the latter to be named keys. Personally, I'd prefer to call responses or rules.
These rules expressed by a tuple without labels is a little bit confusing. Better use a struct, or just add labels.
You can easily decompose a tuple this way:

for (multiple, response) in responsesByMultiples {
    ...
}

Calling the first element multiple is a little bit too optimistic (or pessimistic depending on the way you to see things), it presumes that there is a high probability that n is actually a multiple of multiple. It would make more sense to me to name the first element of the tuples dividsor.
n is a bit too generic, use dividend instead.
You don't have to specify the type in isMultiple: Bool, type inference can do the job for you. Generally speaking, specifying the type can help in reducing the compile times, but in this case, it wouldn't make a difference.
Instead of using (n % multiple) == 0, there is a nice syntax in Swift 5 :

n.isMultiple(of: multiple)

(this syntax exacerbates the problem mentioned in 4.)
To check that a String is empty, it is more efficient to check the .isEmpty property. Here is a benchmark that confirms it:

import Foundation

let array = (0..<1_000_000).map { _ in
    Double.random(in: 0..<1) < 0.8 ? "" : String("Hello".shuffled())
}

do {
    let start = Date()
    let result = array.filter { $0 == "" }
    let end = Date()

    print(result.count, end.timeIntervalSince(start))
}

do {
    let start = Date()
    let result = array.filter { $0.isEmpty }
    let end = Date()

    print(result.count, end.timeIntervalSince(start))
}

The execution times are respectively 44ms and 34ms.

 Putting it all together ❤

Here a version of your code that takes the previous points into account :

func fizzBuzz (
    number dividend: Int,
    rules: [(Int, String)] = [(3, "Fizz"), (5, "Buzz")]
    ) -> String {

    var result: String = ""

    for (divider, response) in rules
        where dividend.isMultiple(of: divider) {
        result += response
    }

    return result.isEmpty ? String(dividend) : result
}

 Further reading

You can find many implementations of this classic question on Code Review. Here is a quite informative one.


Applying different filters within loops What I have is an outer loop which loops through a list of strings, followed by another loop which loops through a list of filters; and if the current column is the current filter in the list of filters, it then applies this filter.

I'm just trying to tidy up a messy function but can't see anyway of doing so.

public static bool PassRules(List<string>columnsList, List<Customfilters> customfilters )
{
    var i = 0;
    foreach (var column in columnsList)
    {
        var curFilter = new Customfilters();

        foreach (var customfilterse in customfilters.Where(customfilterse => customfilterse.Field.Equals(Filed[i])))
        {
            curFilter = customfilterse;
        }

        if ( !string.IsNullOrEmpty(curFilter.Filter))
        {
            int n;
            var isNumeric = int.TryParse(column, out n);
            if (isNumeric)
            {
                if (!Compare(curFilter.Expression, Convert.ToDouble(column), Convert.ToDouble(curFilter.Filter)))
                {
                    return false;
                }
            }
            else
            {
                if (curFilter.Expression.Equals("IN") || curFilter.Expression.Equals("NOT IN"))
                {
                    var toList = curFilter.Filter.Split(Convert.ToChar(","));

                    if (!CompairInNotIn(toList.ToList(), curFilter.Expression, column))
                    {
                        return false;
                    }
                }
                else
                {
                    if (!Compare(curFilter.Expression, column, curFilter.Filter))
                    {
                        return false;
                    }
                }
            }
        }
        i++;
    }
    return true;
}

public static bool Compare<T>(string op, T left, T right) where T : IComparable<T>
{
    switch (op)
    {
        case "<": return left.CompareTo(right) < 0;
        case ">": return left.CompareTo(right) > 0;
        case "<=": return left.CompareTo(right) <= 0;
        case ">=": return left.CompareTo(right) >= 0;
        case "==": return left.Equals(right);
        case "!=": return !left.Equals(right);
        default: throw new ArgumentException("Invalid comparison operator: {0}", op);
    }
}

public static bool CompairInNotIn(List<string> list, string op, string column)
{
    if (op.Equals("IN"))
    {
        return list.Any(column.Contains);
    }
    if (op.Equals("NOT IN"))
    {
        return !list.Any(column.Contains);
    }
    return true;
}

Just to explain a few fields:

var tmpFilter1 = new Customfilters { Field = "IMA01", Expression = "==", Filter = "HP" };

Field[i] is a dictionary of <int,string> and basically returns the ID of the field name.

Can't you change:

if ( !string.IsNullOrEmpty(curFilter.Filter)) {
  ..Lots of code..
}

to

if ( string.IsNullOrEmpty(curFilter.Filter)) {
  i++;
  continue;
}

?
This saves you one indent. (The else clause becomes redundant)

Also I would most likely swap around the if statements for isNumeric and the IN and NOT IN (and their corresponding contents). If only because the two function calls:

if (!Compare(curFilter.Expression, Convert.ToDouble(column), Convert.ToDouble(curFilter.Filter))) {
  return false;
}

and

if (!Compare(curFilter.Expression, column, curFilter.Filter)) {
  return false;
}

look very similar, so imo it's better to group them together. (Though this is mere aesthetics)

Other than that I'm looking at some more fundamental changes if you really want to tidy up the structure. But that goes beyond the scope of this post. (I'm at work at the moment, I know, bad excuse. See what other people have to say.)


Check header files for namespace usage To ensure there are no naming conflicts within our project (C++) I was tasked with writing a python script to check all of our header files for any occurrences of a using namespace ... within the file. If an occurrence is found, it's appended to a list then written to a log file. It's a fairly simple script, but I feel there could be some optimizations. This script is ran whenever someone commits to the repository.

"""
Checks all the header files in our project to ensure there
aren't occurrences of the namespace string.

AUTHOR: Ben Antonellis
DATE: April 4th, 2020
"""

import os

namespace: str = "using namespace"
working_directory: str = os.path.dirname(os.path.realpath(__file__));
occurrences: list = []

for file in os.listdir(working_directory):
    formatted_file = f"{working_directory}/{file}"
    with open(formatted_file, "r") as source_file:
        for line_number, line in enumerate(source_file):
            if namespace in line and file[-3:] != ".py":
                occurrences.append(f"NAMESPACE FOUND: LINE [{line_number + 1}] IN FILE {formatted_file}")

with open("logs/log.txt", "w") as log_file:
    for line in occurrences:
        log_file.write(line)

Working directory

os.path.dirname(os.path.realpath(__file__))

is not the current working directory. The current working directory is available via os.getcwd. Either you should call that instead, or maybe rename your variable.

Semicolons

are usually discouraged in Python, so you can drop one here:

working_directory: str = os.path.dirname(os.path.realpath(__file__));

pathlib

f"{working_directory}/{file}"

is better represented by making a Path and then using the / operator.

Overall

This approach is fragile. You're better off tapping into something like the LLVM/Clang AST, which actually understands how to parse all of the edge cases of C++.

Here is a suggestion that takes care of everything except the AST:

from pathlib import Path

NAMESPACE = 'using namespace'

log_file_name = Path('logs/log.txt')

if __name__ == '__main__':
    working_dir = Path.cwd()
    with log_file_name.open('w') as log_file:
        for prefix in ('h', 'c'):
            for file_name in working_dir.glob(f'*.{prefix}*'):
                with file_name.open() as source_file:
                    for line_number, line in enumerate(source_file):
                        if NAMESPACE in line:
                            log_file.write(f'NAMESPACE FOUND: LINE [{line_number + 1}] IN FILE {file_name}\n')

This also takes into account @AJNeufeld's feedback which is good.