Parse Flags enum from array of booleans I need to get from an array of booleans to a Flags enum.

Here's what my enum looks like:

[Flags]
public enum UserContactPreferences { None = 0, SMS = 1, Email = 2, Phone = 4 }

Here's what my parsing code looks like currently.

private UserContactPreferences GetContactPreferences(bool email, bool phone, bool sms)
{
    var contactByEmail = email ? UserContactPreferences.Email : UserContactPreferences.None;
    var contactByPhone = phone ? UserContactPreferences.Phone : UserContactPreferences.None;
    var contactBySms = sms ? UserContactPreferences.SMS : UserContactPreferences.None;

    return contactByEmail | contactByPhone | contactBySms;
}

That's horrible! I must be able to do better. Any ideas?

Well, you need some sort of mapping between the parameters and the enum values, so you can't actually avoid having three almost same pieces of code. But you can make your code shorter by using ifs and |= instead of ternary operators:

private UserContactPreferences GetContactPreferences(bool email, bool phone, bool sms)
{
    var preferences = UserContactPreferences.None;

    if (email)
        preferences |= UserContactPreferences.Email;
    if (phone)
        preferences |= UserContactPreferences.Phone;
    if (sms)
        preferences |= UserContactPreferences.SMS;

    return preferences;
}

If you had more parameters (though that would be a code smell), it might be worth considering something like using a dictionary. But I'm not sure it's a good idea, it's certainly longer than the previous version:

private UserContactPreferences GetContactPreferences(bool email, bool phone, bool sms)
{
    var dictionary = new Dictionary<UserContactPreferences, bool>
    {
        { UserContactPreferences.Email, email },
        { UserContactPreferences.Phone, phone },
        { UserContactPreferences.SMS, sms }
    };

    var preferences = UserContactPreferences.None;

    foreach (var kvp in dictionary)
    {
        if (kvp.Value)
            preferences |= kvp.Key;
    }

    return preferences;
}


Rust echo implementation that supports command line options I rewrote echo from the manpage to learn Rust, and got the following:

// echo in rust - display a line of text
use std::env;

static HELP: &'static str = "\
echo the STRING(s) to standard output.\n\
-n     do not output the trailing newline\n\
-e     enable intepretation of backslash escapes\n\
-E     disable intepretation of backslash escapes (default)\n\
--help display this help and exit

If -e is in effect, the following sequence are recognized:

  \\\\   backslash
  \\a   alert (BEL)
  \\b   backspace
  \\c   produce no futher output
  \\e   escape
  \\f   form feed
  \\n   new line
  \\r   carriage return
  \\t   horizontal tab
  \\v   vertical tab";

fn main() {
    let mut args: Vec<String> = env::args().skip(1).collect();

    let mut nl: bool = true; // the "-n" flag can be used to indicate no new line
    let mut escape: bool = false; // Disable interpreation of backslashes
    let mut args_count: usize = 0; // first argument is binary name

    for stuff in &args {
        if !stuff.starts_with("-") {
            break;
        }
        if stuff == "-n" {
            nl = false;
            args_count += 1;
        }
        if stuff == "-e" {
            escape = true;
            args_count += 1;
        }
        if stuff == "-E" {
            escape = false;
            args_count += 1;
        }
        if stuff == "--help" {
            println!("{}", HELP);
            return;
        }
    }

    if escape {
        for stuff in &mut args[args_count..] {
            *stuff = stuff.replace("\\\\", "\\"); // backslashe
            *stuff = stuff.replace("\\a", ""); // alert (BEL)
            *stuff = stuff.replace("\\b", ""); // backspace
            *stuff = stuff.replace("\\c", ""); // produce no further output
            *stuff = stuff.replace("\\e", ""); // escape
            *stuff = stuff.replace("\\f", ""); // form feed
            *stuff = stuff.replace("\\n", ""); // new line
            *stuff = stuff.replace("\\r", ""); // carriage return
            *stuff = stuff.replace("\\t", ""); // horizontal tab
            *stuff = stuff.replace("\\v", ""); // vertical tab
        }
    }

    print!("{}", args[args_count..].join(" "));
    if nl {
        println!();
    }
}

I then had a look at previously asked questions on this topic, and learnt of skip from here.

I would particularly welcome feedback on string manipulation, notably on the argument parsing (i.e. here, I expect known flags before data to echo), and the if escape block.

Could the argument parsing be converted to a match statement, whilst retaining the arg_count increment and the break condition?

Another thing I'm unsure of is that I'm first collecting args and then checking each element within the vector and eventually joining it, using a bunch of for loops.

Is there a more idiomatic way to do so, such as vector subtraction or...?

Lastly, my manpage for echo mentions support for decoding bytes:

\0NNN  byte with octal value NNN (1 to 3 digits)
\xHH   byte with hexadecimal value HH (1 to 2 digits)

Would you have pointers on how to do this type of operation?

Thanks to @Shepmaster's comment referring to one of his previous answers, I was able to make the process nicer, by using env::args as an Iterator.

By doing so, it became much easier to switch to using a match statement, and avoids all the string concatenation I mentioned in my question.

I've also managed to add the conversion from hex and octal values to chars by making use of std::u8::from_str_radix in conjunction with std::string::String::trim_start_matches

I therefore now have the following:

// echo in rust - display a line of text
use std::env;
use std::u8;

static HELP: &'static str = "\
echo the STRING(s) to standard output.\n\
-n     do not output the trailing newline\n\
-e     enable intepretation of backslash escapes\n\
-E     disable intepretation of backslash escapes (default)\n\
--help display this help and exit

If -e is in effect, the following sequence are recognized:

  \\\\    backslash
  \\a    alert (BEL)
  \\b    backspace
  \\c    produce no futher output
  \\e    escape
  \\f    form feed
  \\n    new line
  \\r    carriage return
  \\t    horizontal tab
  \\v    vertical tab
  \\0NNN byte with octal value NNN (1 to 3 digits)
  \\xHH  byte with hexadecimal value HH (1 to 2 digits)
";

fn main() {
    let mut nl: bool = true; // the "-n" flag can be used to indicate no new line
    let mut escape: bool = false; // Disable interpretation of backslashes
    let mut args_done = false; // Argument parsing done
    let mut first = true; // Correct the printing spacing

    for arg in env::args().skip(1) {
        if !args_done {
            match arg.as_ref() {
                "-n" => nl = false,
                "-e" => escape = true,
                "-E" => escape = false,
                "--help" => print!("{}", HELP),
                _ => args_done = true,
            }
        }

        if args_done {
            let mut datum: String = arg;
            if escape {
                datum = datum.replace("\\\\", "\\"); // backslash
                datum = datum.replace("\\a", ""); // alert (BEL)
                datum = datum.replace("\\b", ""); // backspace
                datum = datum.replace("\\c", ""); // produce no further output
                datum = datum.replace("\\e", ""); // escape
                datum = datum.replace("\\f", ""); // form feed
                datum = datum.replace("\\n", ""); // new line
                datum = datum.replace("\\r", ""); // carriage return
                datum = datum.replace("\\t", ""); // horizontal tab
                datum = datum.replace("\\v", ""); // vertical tab

                if datum.starts_with("\\x") && 3 <= datum.len() && datum.len() <= 4 {
                    // Hex values with at most 2 digits
                    let value = datum.trim_start_matches("\\x");
                    let chr = u8::from_str_radix(value, 16).unwrap() as char;
                    datum = chr.to_string();
                }

                if datum.starts_with("\\0") && 3 <= datum.len() && datum.len() <= 5 {
                    // Octal values with at most 3 digits
                    let value = datum.trim_start_matches("\\0");

                    let chr = u8::from_str_radix(value, 8);
                    // The maximum octal value for a byte is 377.
                    // Check that this conversion was successful.
                    if chr.is_ok() {
                        let x = chr.unwrap() as char;
                        datum = x.to_string();
                    }
                }
            }
            if first {
                print!("{}", datum);
                first = false;
            } else {
                print!(" {}", datum);
            }
        }
    }
    if nl {
        println!();
    }
}

This now adds an extra first flag for correct formatting, but it's okay for what is done here.


Project Euler # 36 Double-base palindromes in Python The decimal number, 585 = 1001001001 (binary), is palindromic in both bases.
Find the sum of all numbers, less than one million, which are palindromic in base 10 and base 2.
(Please note that the palindromic number, in either base, may not include leading zeros.)

Awaiting feedback.
def is_palindrome(n):
    """returns True if palindrome, False otherwise."""
    to_str = str(n)
    if to_str == to_str[::-1]:
        return True
    return False

def find_palindromes(n):
    """generates all numbers if palindrome in both bases 2, 10 in range n. """
    decimal_binary = {decimal: bin(decimal)[2:] for decimal in range(1, n) if is_palindrome(decimal)}
    for decimal, binary in decimal_binary.items():
        if is_palindrome(binary) and not binary[0] == 0:
            yield decimal

if __name__ == '__main__':
    print(sum(list(find_palindromes(1000000))))

Code review

The sequence of

    if condition:
        return True
    return False

is a long way to say

    return condition

Consider instead

def is_palindrome(n):
    return to_str == to_str[::-1]:

Generator vs list.

A list takes space. The entire point of a generator is to not take space. Your find_palindrome does yield, that is produces one palindrome at a time. Very well suited to sum them as they are produced. Your code collects them all in a list for no reason.

Even more curious is that your code

builds a dictionary
then yields each entry
to build the list
which is sent to sum to traverse it.

I see at least 4 traversals over the same data. Seems excessive.

Efficiency

Thou shalt not brute force.

There are just 1000 decimal palindromes below 1000000: they are all in form abccba. In fact, we are not interested in all of them: if a is even, the binary representation would have a trailing 0, and to be a palindrome it would have a leading 0 as well. We may immediately disqualify such numbers. What remains, is just 500 candidates.

So, we only need to iterate over 500 numbers, instead of 1000000 your code does. A 2000-fold speedup, immediately. In fact, a bit more, because there is no need to test wether a decimal representation is a palindrome anymore, and such test is quite expensive. There is also no need to test for the parity, but it is peanuts.

The fun part is to design test that the binary representation is palindromic. The usually recommended

        binary = bin(n)
        return binary == binary[-1:1:-1]

works well in general. In this particular setting you know a lot about the numbers and their binary representation (at the very least you know how many bits the number takes), and there are few more performant solutions.

Rant

Please keep in mind that solving Project Euler problems will not make you a better programmer. Project Euler is designed for programmers striving to be better mathematicians.

And no matter what, do not brute force.


Find minimum XOR values in a given array I am implementing an algorithm to find the pair of integers in the given array which have minimum XOR value.

Ex:

Input:

   0 2 5 7

Output:

   2 (value of 0 XOR 2).

Then bellow is my solution:

    public int findMinXor(List<int> A)
    {
        var min = int.MaxValue;

        for (int i = 0; i < A.Count - 1; i++)
        {
            for (int j = i + 1; j < A.Count; j++)
            {
                var tmp = A[i] ^ A[j];

                if (tmp < min)
                {
                    min = tmp;
                }

            }
        }

        return min;
    }

What should I do to improve the solution (clean code, performance, DRY,...)?
Any help are appriciate!

The program is running in quadratic time. As usual, to improve performance you need to change the algorithm.

Sort the numbers into 32 buckets, according to where the most significant bit of the number is. Convince yourself that if a bucket contains more than one number, it makes no sense to test them against the numbers from other buckets (the result is guaranteed to not be minimal). A cross-bucket xor should be tested only if two adjacent buckets contain just one number each.

Work recursively with each bucket.


Ruby: Remove middle node from Linked List Currently, I'm going over the CTCI, I'm working on the problem removing the middle node. I'm using Ruby to go over these problems. I have the following solution.

 def remove_middle_node
    node = @head
    count = 1

    while(node = node.next )
      count += 1
    end
    # reset node to point to head
    node = @head
    middle = (count / 2)
    count = 1

    while(node = node.next)

      if(count == (middle - 1))
        node.next = node.next.next
        return
      end
      count += 1
    end
  end

Instead of having a previos_node I subtract one from the middle. In this case when traversing the Linked List, when I get to the (middle - 1), meaning the previous node before the middle node in the linked list. I delete the middle node and set it to next.

I would like to get feedback on this implementation. In my understanding, this is O(n) and time complexity is also O(n). Although I warn you, this Big O notation still a bit unclear to me so time and space complexity might be wrong.

Here is the full Linked List class.

https://github.com/theasteve/ds_and_algos/blob/master/linked_list.rb
 here is the test I used.

# TEST
list = LinkedList.new

list.append(1)
list.append(2)
list.append(8)
list.append(3)
list.append(7)
list.append(0)
list.append(4)

list.display
puts '-----------------------------------'
list.remove_middle_node
list.display

document/comment your code. In the code.
re-checking .next in the second loop is unusually defensive
do not have a comment repeat what a statement "does" (reset node to point to head):
have it illuminate what is achieved/what-for/why something is done (the way it is)
the ruby way to have a statement executed, say, middle times may be

middle.times do
    node = node.next

(Eyeballing remove_middle_node, it uses O(1) additional space and Θ(n) time.)


Cleanup code to add div structure to element I was cleaning up my code when I came across this situation :

var a = ['one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine'];
var count = 1;
var html = '';
for (var i = 0; i < a.length; i++) {
    var rowStart = '
';
    var cell = '
';
    cell += '
' + a[i] + '
';
    cell += '
';
    var rowEnd = '
';

    if (count == 1) {
        html += rowStart + cell;
        count++;
    } else if (count == 3) {
        html += cell + rowEnd;
        count = 1;
    } else {
        html += cell;
        count++
    }
}
$('#container').append(html);

jsFiddle

I retrieve data from a database which I want to display in a div structure as shown above. This code however looks ugly and I think it can be way shorter, I just don't know how.

I was hoping someone could give me some advice/methods/anything on how to clean up this code.

You should let CSS handle most of the job for you. Example

JS:

var a = ['one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine'];
var count = 1;
var html = '';
for (var i = 0; i < a.length; i++) {
    var div = $("<div>" + a[i] + "</div>");
    $('#container').append(div);
}

CSS (where the magic is):

#container div:nth-child(3n+1) { /* Every third! */
    clear: both;
}

#container div {
    float: left;
    width: 100px;
}


Building a Stack with C structs After reading the book from which this example is from I started using APIs like this

typedef struct stack {
    char **items;
    int len;
} stack;

stack *stack_new() {
    stack *out = malloc(sizeof(stack));
    //initialize  the struct
    *out = (stack) {};
    return out;
}

void add(stack *in, char *val) {
    in->len++;
    in->items = realloc(in->items, in->length * strlen(val));
    in->items[in->length-1] = val; //<- actually this would be better with strcpy, but you get the idea
}

problem is that valgrind hates it. I've been trying to fix the various memory leaks that structs similar to this one (and to the dictionary of the link) are creating, until I started wondering if it's a good pattern at all.

Here are some things that may help you improve your code.

Write standard C

The code includes this line:

*out = (stack) {};

However, as my compiler helpfully reminds:

  ISO C forbids empty initializer braces

Fix typos

The struct declares a member variable len but the code refers to a member variable length.  If you're asking for code to be reviewed, it would be good if you invest sufficient time to make it as good as you can get it.

Enhance error checking

Both malloc and realloc can fail, but this code doesn't check for that. If realloc fails, then the following line, will attempt to dereference NULL which will cause a program crash on most systems.

Use calloc if you need the memory zeroed

Rather than calling malloc and then expressly initializing your struct, you can do this in one step by calling calloc instead of malloc.  Doing so reduces stack_new() to a single line:

stack *stack_new() {
    return calloc(1, sizeof(stack));
}

Understand pointers

The major problem with this code is that it seems to confuse the length of a string with the size of a pointer to a string.  When you have a variable declaration such as char **items, it means that you have a pointer to a pointer to char.  So the thing stored in items is a pointer to a string, not a string.  For that reason, it should instead be written like this:

void add(stack *in, char *val) {
    in->items = realloc(in->items, (1+in->len) * sizeof(val));
    if (in->items) {
        in->items[in->len] = val;
        in->len++;
    } else {
        in->len = 0;
    }
}

Note that this only increments len if the reallocation succeeded. If it failed, the stack length is set to zero because the contents have been lost. Also, the allocation is to accomodate sizeof(val) rather than the length of the string because what we're storing is the pointer to the string rather than the string itself.  This also means that there is no copy being made of the string itself -- it's just storing a pointer, so that means that the string must continue to exist for the duration of the stack's lifetime which may not be a good design, depending on your proposed use.

Provide a full sample program for review

It's useful to provide a small but complete program for a code review where practical (and it's certainly practical here).  That would include both header and functions as well as a simple main showing the intended use.  Here is a version of your program that does not leak memory. You and valgrind can become friends again.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct stack {
    char **items;
    int len;
} stack;

stack *stack_new() {
    return calloc(1, sizeof(stack));
}

void stack_free(stack *in) {
    if (in) {
        free(in->items);
        in->len = 0;  // not really needed
        free(in);
    }
}

void add(stack *in, char *val) {
    if (in) {
        in->items = realloc(in->items, (1+in->len) * sizeof(val));
        if (in->items) {
            in->items[in->len] = val;
            in->len++;
        }
    }
}

void print(stack *in) {
    if (in) {
        for (int i=0; i < in->len; ++i) {
            puts(in->items[i]);
        }
    }
}

int main()
{
    stack *words = stack_new();
    add(words, "Avoid");
    add(words, "memory");
    add(words, "leaks");
    print(words);
    stack_free(words);
}


HackerRank "Bot Saves Princess" beginner code I submitted the following Python 3 code for this HackerRank challenge. Given an n × n grid with an m in the center and a p in one corner, it prints the directions to move the m to the p.

I wanted to know how improve it or other observations.

#!/usr/bin/python
def displayPathtoPrincess(n,grid):
    #first, find where p is located
    flag = True
    n = n-1
    while(flag):
        if grid[0][n] == 'p':
            flag = False
            a = 0
            b = n
        if grid[n][0] == 'p':
            flag = False
            a = n
            b = 0
        if grid[0][0] == 'p':
            flag = False
            a = 0
            b = 0
        if grid[n][n] == 'p':
            flag = False
            a = n
            b = n
        #else:
            #print("Something broke?") #Why does this execute?
        y = a - int(n/2)
        x = b - int(n/2)
        while 0 != y:
            if y < 0:
                y = y+1
                print("UP")
            if y > 0:
                y = y-1
                print("DOWN")
        while 0 != x:
            if x <0:
                x = x+1
                print("LEFT")
            if x >0:
                x = x-1
                print("RIGHT")

m = int(input())
grid = []
for i in range(0, m):
    grid.append(input().strip())

displayPathtoPrincess(m,grid)

The function should be path_to_princess(grid), using the naming convention specified in PEP 8.  The n parameter is redundant, as it can be inferred from the grid's dimensions.  In any case, redefining n=n-1 is confusing.  Also, it would be better to leave all I/O out of the function, so that it just does computation.

The while(flag) loop shouldn't exist.  If the princess is in another castle, you'll end up with an infinite loop; otherwise, the code executes just once.

Use the // operator for integer division.

Explicit looping can be greatly reduced through liberal use of list comprehensions, generators, and list multiplication.

def path_to_princess(grid):
    z = len(grid) - 1
    corners = [
        (0, 0), (0, z),
        (z, 0), (z, z)
    ]
    p = next((r, c) for r, c in corners if grid[r][c] == 'p')
    m = (z // 2, z // 2)
    row_diff, col_diff = p[0] - m[0], p[1] - m[1]
    yield from (['UP'] * -row_diff if row_diff < 0 else ['DOWN'] * row_diff)
    yield from (['LEFT'] * -col_diff if col_diff < 0 else ['RIGHT'] * col_diff)

n = int(input())
grid = [input().strip() for _ in range(n)]
for move in path_to_princess(grid):
    print(move)


Order a list of points by closest distance I wrote a function that will take a list of points and then order them so they sort of... "chain together".

The function will start with point #1 in the list.

It will add point #1 to the list of ordered by distance points.

It will then search for the closest point to point #1. (We'll call this point, point #2)

It will then add point #2 to the list of ordered by distance points.

And then... it will search for the closest point to point #2. (Which would be point #3)

You get the point.

The main problem with my code is: it's incredibly slow when dealing with lists that contain tons of points.

I would like some help optimizing my function to make it operate as fast as possible.

private static double Distance(Point p1, Point p2)
{
    return Math.Sqrt(Math.Pow(p2.X - p1.X, 2) + Math.Pow(p2.Y - p1.Y, 2));
}

private List<Point> OrderByDistance(List<Point> pointList)
{
    var orderedList = new List<Point>();

    var currentPoint = pointList[0];

    while (pointList.Count > 1)
    {
        orderedList.Add(currentPoint);
        pointList.RemoveAt(pointList.IndexOf(currentPoint));

        var closestPointIndex = 0;
        var closestDistance = double.MaxValue;

        for (var i = 0; i < pointList.Count; i++)
        {
            var distance = Distance(currentPoint, pointList[i]);
            if (distance < closestDistance)
            {
                closestPointIndex = i;
                closestDistance = distance;
            }
        }

        currentPoint = pointList[closestPointIndex];
    }

    // Add the last point.
    orderedList.Add(currentPoint);

    return orderedList;
}

I don't like any of these answers.  In particular, the accepted answer is simply wrong.

Let's start by critiquing the interface:

List<Point> OrderByDistance(List<Point> pointList)

The contract is: the list must contain at least one element, the list is destroyed (!!!) by the method, the first element is special, and the result is a mutable list.  I like nothing about any of this; this sounds like a ball of potential bugs.  The right contract is:

ImmutableList<Point> OrderByDistance(Point start, ImmutableSet<Point> points)

Look at how many problems this solves. Do we need the set to contain at least one point?  No. We already have the start point. Do we destroy the set of points?  No. It's immutable.  And so on.

Now that we have the signature correct, the algorithm is simple:

ImmutableList<Point> OrderByDistance(Point start, ImmutableSet<Point> points)
{
  var current = start;
  var remaining = points;
  var path = ImmutableList<Point>.Empty.Add(start);
  while(!remaining.IsEmpty)
  {
    var next = Closest(current, remaining);
    path = path.Add(next);
    remaining = remaining.Remove(next);
    current = next;
  }
  return path;
}

Now all you have to do is efficiently implement Closest, which you should be able to do given the previous hints about computing distances cheaply and so on.

If you want a more sophisticated algorithm for Closest, then you need to do some research on this well-studied problem:

https://en.wikipedia.org/wiki/Nearest_neighbor_search


Web application for a restaurant I'm making a web application for a restaurant kind of type. The idea is to administrate  the orders and customers.

For selecting one of the orders and showing more specific data about it, I have this PHP script. As you can see I am using prepared statements to prevent SQL injection.

<?php

try {
    $connection = new PDO('mysql:host=localhost;dbname=broodjes-service;charset=utf8mb4', 'root', 'password');
    $connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

    if (!empty($_GET['order_id'])) {
        $order_id = $_GET['order_id'];

        $order_data = $connection->prepare("SELECT c.first_name, c.last_name, c.email_adress, c.customer_info, o.order_info, o.total_price, o.location, o.created FROM customers AS c LEFT JOIN orders AS o ON c.id = o.customer_id WHERE o.id = :order_id LIMIT 1");
        $order_data->bindParam(":order_id", $order_id, PDO::PARAM_INT);
        $order_data->execute();

        $query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`" . "INNER JOIN `products` ON `orders-items`.`products_id` = `products`.`id`" . "WHERE order_id = :ordero_id LIMIT 1";
        $order_items = $connection->prepare($query);

        $order_items->bindParam(":ordero_id", $order_id, PDO::PARAM_INT);
        $order_items->execute();

        $orderObject = array();
        $orderObject['header'] = $order_data->fetch();
        $orderObject['items'] = array();
        while ($orderedItem = $order_items->fetch()) {
            $orderObject['items'][] = $orderedItem;
        }

        header('Content-type: application/json');
        echo json_encode($orderObject);

        $connection = null;
    }
} catch (PDOException $e) {
    echo $e->getMessage();
    die();
}

The parameters for the 2 queries are both the same. But I don't know how to use only one line for them.

The first query is for selecting the specific data about the order.
The second query is for selecting the items inside the order.

Both queries should be run, to get all results.

Problems

It's messy that I actually have 2 queries.
It's messy that I'm using 2 lines for the same parameter

Explanation; why there are 2 queries

Whenever I use one query like this:

SELECT c.first_name,
        c.last_name,
        c.email_adress,
        c.customer_info,
        o.order_info,
        o.total_price,
        o.location,
        o.created,

        p.name,
        ot.quantity

FROM customers AS c
LEFT JOIN orders AS o ON c.id = o.customer_id
LEFT JOIN `orders-items` AS ot ON o.id = ot.order_id
LEFT JOIN `products` AS p ON ot.products_id = p.id

WHERE order_id = :order_id;

I get 3 times the results of the specific data from the customer. Then I don't know how to get back the orders-items separately. Also, whenever there are no results, I have no idea how to 'not select them' within MySQL.

Also, when using that query, when a customer doesn't have any orders_items no result is given.

Aliases
Table aliases are handy, sure. But single-letter aliases are not good. It's OK to want to save having to type more characters than needed, but you have to keep in mind that things like aliases and variables get really confusing if the name you give it does not say anything about what it means. c, o, p, ot... Why not instead cust, ord, prod, ordItems?
Table naming
orders-items is not a good table name. Why? Well, because you will have to use back ticks each time you reference it, as opposed to your other tables. In SQL, avoid using reserved characters in table/column names to negate this problem. If possible, rename to orders_items or OrderItems or something along those lines.
To rename the table:
RENAME TABLE `orders-items` TO orders_items;

Consistency
Your SQL queries, though both are valid, look completely different. Compare this:
    $order_data = $connection->prepare("SELECT c.first_name, c.last_name, c.email_adress, c.customer_info, o.order_info, o.total_price, o.location, o.created FROM customers AS c LEFT JOIN orders AS o ON c.id = o.customer_id WHERE o.id = :order_id LIMIT 1");

And this:
    $query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`" . "INNER JOIN `products` ON `orders-items`.`products_id` = `products`.`id`" . "WHERE order_id = :ordero_id LIMIT 1";

One of them you use table aliases, the other you use full table names. The latter you use back ticks, but not the former. The latter you also have periods splitting your query code. Stick to one style to make your code easier to maintain.
Your question
Your first query is selecting one specific order along with the customer associated with it. Your second query is selecting one specific order along with the items associated with it. If you want to combine both, then assuredly you will get customer information multiple times in your result set. Your LIMIT 1 seems like it would not be very useful... Can an order be placed by multiple customers?
However, since orders is your primary table, I suggest to start from there, then JOIN your other tables with INNER JOIN so you don't return nulls (if that is the intent, and assuming an order has to have a customer, and has to have items).
All said, I think this should work for what you are trying to do:
SELECT
    ord.id,
    ord.order_info,
    ord.total_price,
    ord.location,
    ord.created,
    cust.id,
    cust.first_name,
    cust.last_name,
    cust.email_adress,
    cust.customer_info,
    ordItems.quantity,
    ordItems.products_id,
    prod.name,
    prod.price
FROM
    orders AS ord
        INNER JOIN
    customers AS cust ON ord.customer_id = cust.id
        INNER JOIN
    `orders-items` AS ordItems ON ord.id = ordItems.order_id
        INNER JOIN
    products AS prod ON ordItems.products_id = prod.id
WHERE
    ord.id = :order_id;


Simple random number guessing game I made this simple game code. Can you please give me some advice on how I can improve it?

#include<stdio.h>
#include<stdlib.h>
#include<conio.h>
#include <time.h>
#include <windows.h>

void main ()
{
    printf("\t\tWelcome to our gessing game\n\n\n");
    printf("the computer choose a random number between 1-99, try to gess the random number...");

    srand(time(NULL));

    int gess,i=0,found=0,  r = rand()%100;

    while (i<10 && found==0)
    {
        printf("\n My gess is:\t");
        scanf("%d",&gess);
        if (gess==r)
        {
            found=1;
            i++;
        }
        else if(gess>r)
        {
            printf("\n Too big\n");
            i++;
        }
        else if(gess<r)
        {
            printf("\n Too small\n");
            i++;
        }
    }
    if (found==1&&gess==1)
        printf("\n Very good the number is %d this is your %dst gess",gess,i);
    else if(found==1&&gess==2)
        printf("\n very good the number is %d this is your %dnd gess",gess,i);
    else if(found==1&&gess==3)
        printf("\n very good the number is %d this is your %drd gess",gess,i);
    else if(found==1&&gess!=1&&gess!=2&&gess!=3)
        printf("\n very good the number is %d this is your %dth gess",gess,i);
    else
        printf("\n Never mind try again");
    getch();
}

One variable per line please.

int gess,i=0,found=0,  r = rand()%100;

This makes the code hard to read. There is also one corner case (with pointers) were this will not work as expected for beginners. As a result this is usually banned in most companies coding standards.

You are using found as a boolean variable.
Treat it as such. use TRUE/FALSE.

I would change the while() into a for(;;) loop. Then you can remove all the increments.

while (i<10 && found==0)

--

for(int i = 0; i < 10 && !found; ++i)

White space is your friend:

if (found==1&&gess==1)  // This is hard to read and will get worse with more conditions.

if (found && (guess == 1)) // much more readable
//  ^^^^^  boolean use it as such.

Your "st", "nd", "rd", "th" is not correct.

10-19    => th
x1       => st
x2       => nd
x3       => rd
x[4-9]   => th

Also you print basically the same string every time on success. So we not have one print statement and just separate the logic for getting the ending.
I would change the logic:

if (!found)
{
    printf("\n Never mind try again");
}
else
{
    char const* ext = "th";
    if ((gess < 10) || (gess >= 20))  // 10->19 already set
    {
        int end = gess % 10;
        if      (end == 1) { ext = "st";}
        else if (end == 2) { ext = "nd";}
        else if (end == 3) { ext = "rd";}
    }

    printf("\n Very good the number is %d this is your %d%s gess", gess, i, ext);
}


Extracting a maximum from zipped lists I have code that zips 3 lists, finds a maximum and extracts part of the max tuple. Isn't there a shorter way to do this in F#?

let Triplets = List.zip3 A B C
let T1 (x, _, _) = x
let T2 (_, x, _) = x
let T3 (_, _, x) = x
let Best = List.maxBy T3 Triplets
T1 Best,T2 Best // return to C# code

One way to simplify this would be to use nested pairs (('a * 'b) * 'c) instead of a triple ('a * 'b * 'c). That way, you can use fst and snd to get to the parts that you want:

let triplets = List.zip (List.zip A B) C
let best = List.maxBy snd triplets
fst best

Notice that I also changed the names of variables to lowercase, that is the common naming convention in F#.


Project Euler #3 Efficiency I'd like to reduce the complexity of my code
Problem :

The prime factors of 13195 are 5, 7, 13 and 29.
What is the largest prime factor of a given number N?

Solution :
import java.util.Scanner;
import java.util.TreeSet;

public class Solution {

    public static void main(String[] args) {

        Scanner sc = new Scanner(System.in);
        int testCases = sc.nextInt();
        for (int i = 0; i < testCases; i++) {
            int input = sc.nextInt();
            System.out.println(calculateFactors(input));

        }
    }

    public static int calculateFactors(int input) {
        TreeSet<Integer> set = new TreeSet<>();
        while (input % 2 == 0) {
            set.add(2);
            input = input / 2;
        }
        for (int i = 3; i < Math.sqrt(input); i = i + 2) {
            while (input % i == 0) {
                set.add(i);
                input = input / i;
            }
        }
        if (input > 2) {
            set.add(input);
        }
        int max = set.last();
        return max;
    }

}

Correctness:

Project Euler Problem #3 asks for
the largest prime factor of 600851475143, and that number exceed the range
of the int data type. (Your program aborts with an exception for this input value.) You have to use long instead.

In

for (int i = 3; i < Math.sqrt(input); i = i + 2) { ... }

the < should be replaced by <=, otherwise you get a wrong result if the
input is the square of a prime number (such as 25). To avoid a loss of precision if input is converted to a floating point
number, it should be

for (long i = 3; i * i <= input; i = i + 2) { ... }

Efficiency:

The problem is about the largest prime factor. Your algorithm already
determines the prime factors in increasing order. Therefore there is no need
to store the prime factors in a collection to get its maximum value.
It is sufficient to remember the last prime factor found.

Putting it together:

Your function (renamed slightly) would then look like

public static long calculateLargestFactor(long input) {
    long largestFactor = 1;
    while (input % 2 == 0) {
        largestFactor = 2;
        input = input / 2;
    }
    for (long i = 3; i * i <= input; i = i + 2) {
        while (input % i == 0) {
            largestFactor = i;
            input = input / i;
        }
    }
    if (input > 2) {
        largestFactor = input;
    }
    return largestFactor;
}


Ruby calculator for a grocery store I have been working on this code to meet the best OOP standards for production. The code is working but I wish to make this a more efficient code. Could someone help me out here to make this efficient using best practices of ruby.
class Item
  @@items = {}
  def initialize(name, price)
    @@items[name] = price
  end

  def self.all
    @@items
  end
end

class SaleItem
  @@sale_items = {}
  def initialize(name, units, price)
    @@sale_items[name] = { 'units' => units, 'price' => price }
  end

  def self.all
    @@sale_items
  end
end

class PriceCalculator
  def initiate_billing
    input = get_input.split(',').map(&:strip)
    @purchased_items = input
    if @purchased_items.any?
      quantity = count_items
      price = calculate_bill(quantity)
      display_bill(price, quantity)
    else
      puts "First add items to generate bill"
    end
  end

  private

    def get_input
      puts "Please enter all the items purchased separated by a comma"
      response = gets.chomp
    end

    def count_items
      @purchased_items.inject(Hash.new(0)) do |quantity, item|
        quantity[item] += 1
        quantity
      end
    end

    def calculate_bill(quantity)
      quantity.map do |item,value|
        items = Item.all[item]
        sale_items = SaleItem.all[item]
        value = if sale_items.nil?
         quantity[item] * items
        else
         (((quantity[item]/sale_items['units'])) * sale_items['price']) + ((quantity[item] % sale_items['units']) * items)
        end
        [item, value]
      end.to_h
    end

    def display_bill(price, quantity)
      billing_items = quantity.each_with_object(price) do |(key,value), billing_items|
        billing_items[key] = {'units' => value, 'price' => price[key]}
      end

      total_price = billing_items.inject(0) do |total, (item,value)|
        total + value['price']
      end

      actual_price = quantity.inject(0) do |total, (item,units)|
        total + (units * Item.all[item])
      end

      puts "Item     Quantity      Price"
      puts "--------------------------------------"
      billing_items.each do |item, value|
        puts "#{item.ljust(10)} #{value['units']}           $#{value['price'].round(3)}"
      end
      puts "Total price : $#{total_price.round(3)}"
      puts "You saved $#{(actual_price - total_price).round(3)} today."
    end
end

begin
  Item.new('milk', 3.97)
  Item.new('bread', 2.17)
  Item.new('banana', 0.99)
  Item.new('apple', 0.89)

  SaleItem.new('milk',2,5.00)
  SaleItem.new('bread',3,6.00)

  price_calculator = PriceCalculator.new
  puts price_calculator.initiate_billing
end

```

I have been working on this code to meet the best OOP standards for production.

I think your biggest problem here is that PriceCalculator does too many things:

Getting user input
Calculating the price per checkout item
Calculating the total price
Printing the bill

Let's try to split this up a bit first by introducing a BillRow for each line in your Bill to which we delegate to calculate the price per row .
class Bill
  def initialize(purchased_items:)
    @purchased_items = purchased_items
  end

  def total
    rows.inject(0) { |sum, x| sum + x.total }
  end

  def discounted_total
    rows.inject(0) { |sum, x| sum + x.discounted_total }
  end

  def rows
    @rows ||= fetch_rows
  end

  private

  attr_reader :purchased_items

  def fetch_rows
    count_items.map do |item, quantity|
      BillRow.new(name: name, quantity: quantity)
    end
  end

    def count_items
      purchased_items.inject(Hash.new(0)) do |quantity, item|
        quantity[item] += 1
        quantity
      end
    end
end

class BillRow
  attr_reader :name, :quantity

  def initialize(name:, quantity:)
    @name = name
    @quantity = quantity
  end

  def total
    @_total ||= quantity * item_price
  end

  def discounted_total
    @_discounted_total ||= calculate_discounted_total || total
  end

  private

   def calculate_discounted_total
    return unless sale_item

    ((quantity / sale_item['units'])) * sale_item['price']) + ((quantity % sale_item['units']) * item_price)
   end

    def sale_item
      @_sale_item ||= SaleItem.all[name]
    end

    def item_price
      Item.all[name]
    end
end

Bill.new(['milk', 'milk', 'bread'])

Additionally we will extract the display method to it's own class.
class BillPrinter
  def initialize(bill:)
    @bill = bill
  end

  def print
    print_header
    print_rows
    print_total
  end

  private

  def print_header
    puts "Item     Quantity      Price"
    puts "--------------------------------------"
  end

  def print_rows
    bill.rows.each do |row|
      puts "#{row.item_name.ljust(10)} #{row.quantity}           $#{row.total.round(3)}"
    end
  end

  def print_total
    puts "Total price : $#{bill.total.round(3)}"
    puts "You saved $#{(bill.total - bill.discounted).round(3)} today."
  end
end

bill = Bill.new(['milk', 'milk', 'bread'])
BillPrinter.new(bill: bill).print

I will skip getting the user input as this is can basically go to the main method.
Splitting the PriceCalculator into three classes has several advantages.

Easier to test: We can pass in data instead of user input
Classes are smaller and have only one responsibility
Easier to extend and maintain e.g. if we want to use it on a website instead the console we need to refactor the code (user input read from StdIn and bill printed to StdOut). Now we can just implement a HtmlBillPrinter for instance.
We can easier memoize data (efficiency)

The code is working but I wish to make this a more efficient code

In terms of efficiency, the biggest issue is that you calculate some values several times and look it up several times. This might be fine and the most important rule when making something more efficient is to first identify the bottleneck before jumping into optimize it. Readable code is usually preferred over complicated but efficient code.
To sum it up, I think the biggest improvements can be made in a better object oriented approach which allows you to more aggressively memoize values (https://www.justinweiss.com/articles/4-simple-memoization-patterns-in-ruby-and-one-gem/) as well as working with objects instead of e.g. arrays and hashes (BillRow vs [quantity, value]) which allows you to split out more readable methods too.
Edit:
As you mention 'production' I also just want to mention that you should NEVER use float for monetary values because they're not accurate (https://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency). You should use integer and represent the price e.g. in cents. Even better would be to use a money gem (https://github.com/RubyMoney/money).
Another improvement would be to use objects in your SalesItem and Item classes.
class SaleItem
  @@sale_items = {}

  attr_reader :name, :units, :price

  def initialize(name:, units:, price:)
    @name = name
    @units = units
    @price = price
  end

  def initialize(name, units, price)
    @@sale_items[name] = new(name: name, units: units, price: price)
  end

  def self.all
    @@sale_items
  end
end
```


Daily, Weekly, Monthly Individual Tech time in task Repoert I'm trying to create daily, monthly and weekly SQL Query report to our services time we spent int task and total billing time just want to see if I'm on right track

GO
--Daily

SELECT     SUM(billingsTimes.actualTotalTime) AS TotalTime, Tasks.taskName, billableType.billableTypeName
FROM         billingsTimes INNER JOIN
                      aspnet_Users ON billingsTimes.userID = aspnet_Users.UserId INNER JOIN
                      billableType ON billingsTimes.billableTypeID = billableType.rank INNER JOIN
                      Tasks ON billingsTimes.taskID = Tasks.taskID
WHERE     (billingsTimes.taskID = 10045) AND (aspnet_Users.UserId = '178db2a8-be1d-48e0-9ebc-f64f7b1ff63e') AND (CONVERT(DATE, billingsTimes.dateOfService)
                      = '01/29/2013')
GROUP BY billingsTimes.billableTypeID, Tasks.taskName, billableType.billableTypeName,DATEPART(D, dateOfService)

GO
--Weekly

SELECT     SUM(billingsTimes.actualTotalTime) AS TotalTime, Tasks.taskName, billableType.billableTypeName,DATEPART(WEEK, dateOfService)AS weekNumber
FROM         billingsTimes INNER JOIN
                      aspnet_Users ON billingsTimes.userID = aspnet_Users.UserId INNER JOIN
                      billableType ON billingsTimes.billableTypeID = billableType.rank INNER JOIN
                      Tasks ON billingsTimes.taskID = Tasks.taskID
WHERE     (billingsTimes.taskID = 10045) AND (aspnet_Users.UserId = '178db2a8-be1d-48e0-9ebc-f64f7b1ff63e')
GROUP BY billingsTimes.billableTypeID, Tasks.taskName, billableType.billableTypeName,DATEPART(WEEK, dateOfService), DATEPART(WEEK, dateOfService)

GO
--Monthly

SELECT     SUM(billingsTimes.actualTotalTime) AS TotalTime, Tasks.taskName, billableType.billableTypeName,DATEPART(MONTH, dateOfService)AS monthNumber
FROM         billingsTimes INNER JOIN
                      aspnet_Users ON billingsTimes.userID = aspnet_Users.UserId INNER JOIN
                      billableType ON billingsTimes.billableTypeID = billableType.rank INNER JOIN
                      Tasks ON billingsTimes.taskID = Tasks.taskID
WHERE     (billingsTimes.taskID = 10045) AND (aspnet_Users.UserId = '178db2a8-be1d-48e0-9ebc-f64f7b1ff63e')
GROUP BY billingsTimes.billableTypeID, Tasks.taskName, billableType.billableTypeName,DATEPART(MONTH, dateOfService)
GO

Somewhat hard to judge without any idea of the schema, but here are a few observations:

Join condition on billableType table is questionable: why comparing an ID to a rank in INNER JOIN billableType ON billingsTimes.billableTypeID = billableType.rank?
Try to limit GROUP BY to the columns that will become sort of 'logical key' of your result set. E.g. it seems the query is limited to a single task at a time (billingTimes.taskID = ###). Use max(taskName) as taskName in your select list and exclude taskName from the group by clause.
There's no need for datepart(D, dateOfService) in GROUP BY: the query is already restricted to a single day.


Simplify IQueryable Linq to SQL query Below I present to you an IQueryable LINQ to SQL query (used with NHibernate).
I have tried to simplify this query but I could not do it.
I want to run a query for searching information.

Options:

search on source or target
search with match case or not
search match exact or not

The query it seems to work but there is much if/else branching.

Is it possible to simplify it?

        if (concordanceSearchOptions.SearchInSource)
        {
            if (concordanceSearchOptions.MatchCase)
            {
                if (concordanceSearchOptions.MatchExact)
                {
                    getByQueryable = getByQueryable.Where(p => p.SourceSegment.Equals(concordanceSearchOptions.SearchText));
                }
                else
                {
                    getByQueryable = getByQueryable.Where(p => p.SourceSegment.Contains(concordanceSearchOptions.SearchText));
                }
            }
            else
            {
                if (concordanceSearchOptions.MatchExact)
                {
                    getByQueryable = getByQueryable.Where(p => p.SourceSegment.ToLower().Equals(concordanceSearchOptions.SearchText.ToLower()));
                }
                else
                {
                    getByQueryable = getByQueryable.Where(p => p.SourceSegment.ToLower().Contains(concordanceSearchOptions.SearchText.ToLower()));
                }
            }
        }
        else
        {
            if (concordanceSearchOptions.MatchCase)
            {
                if (concordanceSearchOptions.MatchExact)
                {
                    getByQueryable = getByQueryable.Where(p => p.TargetSegment.Equals(concordanceSearchOptions.SearchText));
                }
                else
                {
                    getByQueryable = getByQueryable.Where(p => p.TargetSegment.Contains(concordanceSearchOptions.SearchText));
                }
            }
            else
            {
                if (concordanceSearchOptions.MatchExact)
                {
                    getByQueryable = getByQueryable.Where(p => p.TargetSegment.ToLower().Equals(concordanceSearchOptions.SearchText.ToLower()));
                }
                else
                {
                    getByQueryable = getByQueryable.Where(p => p.TargetSegment.ToLower().Contains(concordanceSearchOptions.SearchText.ToLower()));
                }
            }
        }

Naming

Based on the naming guidlines variables/fields should be named "using a noun, noun phrase, or adjective". So getByQuerable should be renamed to something following the guidlines and also meaningful.

Optimization

you should extract the searching for Source and Target to separate methods. This helps for readability and makes it easier for Mr.Maintainer in a couple of months.
preparing the searchtext before the actual search takes place will speed up the query.
Right now if concordanceSearchOptions.MatchCase == false you call for each source/target in the where part the ToLower() method of the SearchText property.

Refactoring

    String searchText = concordanceSearchOptions.SearchText;
    if (concordanceSearchOptions.MatchCase)
    {
        searchText = searchText.ToLower();
    }

    if (concordanceSearchOptions.SearchInSource)
    {
        getByQueryable  = GetBySource(concordanceSearchOptions, searchText);
    }
    else
    {
        getByQueryable  = GetByTarget(concordanceSearchOptions, searchText);
    }

private IEnumerable<T> GetBySource(SearchOptions concordanceSearchOptions,String searchText){

}
private IEnumerable<T> GetByTarget(SearchOptions concordanceSearchOptions,String searchText){

}


Creating a chess board One of my assignments was to create a chessboard. In this assignment I could not use an array, or a form of list or what not. Below is my code for the creation of the chessboard:

import java.awt.Graphics2D;
import java.awt.Rectangle;
import java.awt.Color;

public class Checkerboard
{
    private int xPositions;
    private int yPositions;
    private int squareSize;

    private Color tileColor;

    public Checkerboard()
    {
        xPositions = 0;
        yPositions = 0;
        squareSize = 50;

        tileColor = Color.WHITE;
    }

    public void draw(Graphics2D g2)
    {
        for(int i = 1; i <= 64; i++)
        {
            Rectangle tile = new Rectangle(xPositions, yPositions, squareSize, squareSize);

            g2.setColor(tileColor);
            g2.fill(tile);

            updateTileColor(i);
            updatePositions(i);
        }
    }

    private void updatePositions(int tileNumber)
    {
        if((tileNumber % 8) == 0)
        {
            xPositions = 0;
            yPositions = yPositions + squareSize;
        }
        else
        {
            xPositions = xPositions + squareSize;
        }
    }

    private void updateTileColor(int tileNumber)
    {
        if((tileNumber % 8) != 0)
        {
            tileColor = (tileColor == Color.WHITE) ? Color.BLACK : Color.WHITE;
        }
    }
}

Could anyone tell me as to whether this code is optimal for the assignment, if any bad practices have been made, or any way I can generally improve my code and my knowledge of the code?

Avoid mutable state

Mutable state, in this example the modifying of the x and y positions to draw,
causes all kinds of troubles.

For example, if you call the draw method a second time,
it will paint another board right after the first one.
This might be surprising to users.
Normally we expect that calling a method multiple times has the same effect,
which is often not the case with methods that mutate state.

Another common problem with mutable state is that it breaks thread-safety.
If draw gets called from multiple threads in parallel, it will most certainly paint rubbish.

Granted, you probably don't plan to call this method multiple times and from multiple threads.
But as a general good practice,
it's good to avoid mutable state when possible.

Using constants

Avoid hard-coded (magic) numbers. The number 8 is the SIZE of the board,
the number 64 is SIZE * SIZE. If you refer to them that way, the intention of the code becomes all the more clear. And if you decide to paint a larger (or smaller) board, you can change one value in one place.

Formatting

Instead of this:

private void updateTileColor(int tileNumber)
{
    if((tileNumber % 8) != 0)
    {
        tileColor = (tileColor == Color.WHITE) ? Color.BLACK : Color.WHITE;
    }
}

The convention is Java is this:

private void updateTileColor(int tileNumber) {
    if ((tileNumber % 8) != 0) {
        tileColor = (tileColor == Color.WHITE) ? Color.BLACK : Color.WHITE;
    }
}

If you use an IDE like IntelliJ or Eclipse,
the auto-reformat feature will format the code for you as per the convention.

Alternative implementation

Others have given you various alternatives,
here's mine, without mutating state,
and applying the suggestions above:

public class Checkerboard {

    private static final int SIZE = 8;

    private final int squareSize = 50;

    public void draw(Graphics2D g2) {
        for (int pos = 0; pos < SIZE * SIZE; pos++) {
            int x = calculateX(pos);
            int y = calculateY(pos);
            Color color = calculateColor(pos);

            Rectangle tile = new Rectangle(x, y, squareSize, squareSize);

            g2.setColor(color);
            g2.fill(tile);
        }
    }

    private int calculateX(int pos) {
        return (pos % 8) * squareSize;
    }

    private int calculateY(int pos) {
        return (pos / 8) * squareSize;
    }

    private Color calculateColor(int pos) {
        int offset = (pos % 16) < 8 ? 0 : 1;
        return (pos + offset) % 2 == 0 ? Color.WHITE : Color.BLACK;
    }
}


Rust closure to be called on a cache miss The official Rust book chapter 13.1 includes an exercise to expand on the example provided in the chapter:

Try modifying Cacher to hold a hash map rather than a single value. The keys of the hash map will be the arg values that are passed in, and the values of the hash map will be the result of calling the closure on that key. Instead of looking at whether self.value directly has a Some or a None value, the value function will look up the arg in the hash map and return the value if it’s present. If it’s not present, the Cacher will call the closure and save the resulting value in the hash map associated with its arg value.
The second problem with the current Cacher implementation is that it only accepts closures that take one parameter of type u32 and return a u32. We might want to cache the results of closures that take a string slice and return usize values, for example. To fix this issue, try introducing more generic parameters to increase the flexibility of the Cacher functionality.

The following is what I have:
use std::thread;
use std::time::Duration;
use std::collections::HashMap;
use std::hash::Hash;

struct Cacher<T, K, J>
    where T: Fn(&K) -> J,
    K: Hash + Eq,
    J: Clone
{
    calculation: T,
    value: HashMap<K, J>,
}

impl<T, K, J> Cacher<T, K, J>
    where T: Fn(&K) -> J,
    K: Hash + Eq,
    J: Clone
{
    fn new(calculation: T) -> Cacher<T, K, J> {
        Cacher {
            calculation,
            value: HashMap::new(),
        }
    }

    fn value(&mut self, arg: K) -> J {
        if let Some(v) = self.value.get(&arg) {
            v.clone()
        } else {
            let v = (self.calculation)(&arg);
            self.value.insert(arg, v.clone());
            v
        }
    }
}

fn generate_workout(intensity: u32, random_number: u32) {
    let mut expensive_result = Cacher::new(|&num| {
        println!("calculating slowly...");
        thread::sleep(Duration::from_secs(2));
        num
    });

    if intensity < 25 {
        println!(
            "Today, do {} pushups!",
            expensive_result.value(&intensity)
        );
        println!(
            "Next, do {} situps!",
            expensive_result.value(&intensity)
        );
    } else {
        if random_number == 3 {
            println!("Take a break today! Remember to stay hydrated!");
        } else {
            println!(
                "Today, run for {} minutes!",
                expensive_result.value(&intensity)
            )
        }
    }
}

fn main() {
    let simulated_user_specified_value = 10;
    let simulated_random_number = 7;

    generate_workout(
        simulated_user_specified_value,
        simulated_random_number
    );
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn call_with_different_values() {
        let mut c = Cacher::new(|&a| a);

        let v1 = c.value(1);
        let v2 = c.value(2);
        assert_eq!(v1, 1);
        assert_eq!(v2, 2);

        let mut d = Cacher::new (|a: &String| a.len());
        let str1 = String::from("abc");
        let v3 = d.value(str1);
        assert_eq!(v3, 3);
    }
}

I don't like the fact that my approach has the generic parameter J bounded by Clone trait. How can I make this work without the bound?
Any other feedback is appreciated.

I like what you've done! I don't have too many comments about the content of it, since it looks good. However, you'll see below that I changed the field name value to cache and some of the generic parameter names, just to make things a little clearer. This is just a subjective preference, but I used F for function, K for key, and V for value.

Another thing to note is that in many cases, you don't need to put trait bounds on the parameters for the struct itself. You just need the bounds when you are implementing functions with it, and having the bounds on the struct doesn't save you from having to write the bounds when you use it as a parameter.

If you want to get rid of the Clone bound, you can return a reference to the value now stored within the cache. Here's one possible implementation of that, using HashMap's entry API.

use std::collections::HashMap;
use std::hash::Hash;

struct Cacher<F, K, V> {
    calculation: F,
    cache: HashMap<K, V>,
}

impl<F, K, V> Cacher<F, K, V>
where
    F: Fn(&K) -> V,
    K: Hash + Eq,
{
    fn new(calculation: F) -> Self {
        Cacher {
            calculation,
            cache: HashMap::new(),
        }
    }

    fn value(&mut self, arg: K) -> &V {
        use std::collections::hash_map::Entry;

        match self.cache.entry(arg) {
            Entry::Occupied(occupied) => occupied.into_mut(),
            Entry::Vacant(vacant) => {
                let value = (self.calculation)(vacant.key());
                vacant.insert(value)
            }
        }
    }
}


Either or case in Python and Regex I have a small module that gets the lemma of a word and its plural form. It then searches through sentences looking for a sentence that contains both words (singular or plural) in either order. I have it working but I was wondering if there is a more elegant way to build this expression.

Note: Python2

words = ((cell,), (wolf,wolves))
string1 = "(?:"+"|".join(words[0])+")"
string2 = "(?:"+"|".join(words[1])+")"
pat = ".+".join((string1, string2)) +"|"+ ".+".join((string2, string1))
# Pat output: "(?:cell).+(?:wolf|wolves)|(?:wolf|wolves).+(?:cell)"

Then the search:

pat = re.compile(pat)
for sentence in sentences:
    if len(pat.findall(sentence)) != 0:
        print sentence+'\n'

Alternatively, would this be a good solution?

words = ((cell,), (wolf,wolves))
for sentence in sentences:
    sentence = sentence.lower()
    if any(word in sentence for word in words[0]) and any(word in sentence for word in words[1]):
        print sentence

You could use findall with a pattern like (cell)|(wolf|wolves) and check if every group was matched:

words = (("cell",), ("wolf","wolves"))
pat = "|".join(("({0})".format("|".join(forms)) for forms in words))
regex = re.compile(pat)
for sentence in sentences:
    matches = regex.findall(sentence)
    if all(any(groupmatches) for groupmatches in zip(*matches)):
        print sentence


Checking if a number is prime in NASM Win64 Assembly I'm just learning Assembly. This program is a simple attempt to determine if a given number is prime or not.

Compiled as follows using VS2019 x64 Native Tools Command Prompt:

> nasm -g -fwin64 isprime.asm

> cl /Zi isprime.obj msvcrt.lib legacy_stdio_definitions.lib

isprime.asm

bits 64
default rel

extern printf
extern scanf

section .data

    number: dq 0
    isPrime: dq 1
    counter: dq 0

    question: db "Which number would you like to check? ", 0
    fmt: db "%d", 0
    numberIsPrime: db "%d is prime", 10, 0
    numberIsNotPrime: db "%d is not prime", 10, 0

section .text

global main
main:
    push rbp
    mov rbp, rsp

    sub rsp, 32
    mov rcx, question
    call printf
    add rsp, 32

    sub rsp, 32
    mov rdx, number
    mov rcx, fmt
    call scanf
    add rsp, 32

    mov rcx, [number]
    mov [counter], rcx

continue_prime_check:
    dec qword [counter]
    cmp qword [counter], 2
    jge not_reached_1_yet
    jmp prime_check_ended

not_reached_1_yet:
    mov rax, [number]
    cdq
    mov rbx, [counter]
    idiv rbx
    cmp edx, 0
    je evenly_divisible
    jmp not_evenly_divisible

evenly_divisible:
    mov qword [isPrime], 0

not_evenly_divisible:
    jmp continue_prime_check

prime_check_ended:
    cmp qword [isPrime], 1
    je number_was_prime
    jmp number_wasnt_prime

number_was_prime:
    sub rsp, 32
    mov rdx, [number]
    mov rcx, numberIsPrime
    call printf
    add rsp, 32
    jmp end_if

number_wasnt_prime:
    sub rsp, 32
    mov rdx, [number]
    mov rcx, numberIsNotPrime
    call printf
    add rsp, 32

end_if:
    mov rbp, rsp
    pop rbp
    ret

Style: Indent your operands to a consistent column, so mnemonics of different length don't make your code look so ragged.  And use local .label labels inside functions.

Comment code that depends on non-standard behaviour: stdout is only guaranteed to be line-buffered, and isn't automatically flushed when you read stdin in ISO C.  Some systems (like Linux) do need fflush(stdout) after a printf of a string that doesn't end with a newline.  But I tried building a win64 executable by linking with mingw64 gcc, and running it under wine64, and it did actually print the prompt, to my surprise.

Normally you'd want to write a program like this to take its input as a command-line argument, instead of prompting for it with stdio at all.  (Then you just use strtoul or atoi, or sscanf, or even a hand-written loop to convert the ASCII decimal string to integer.)

Correctness problem: rbx is a call-preserved register in the x64 Windows calling convention.  Pick a call-clobbered scratch register, like rcx, or r8..r11.
 https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/x64-architecture.  If main's caller happens not to crash when you return after stepping on its RBX, you got lucky.

Use 32-bit operand operand-size when possible, and make sure operand-size is consistent.  You're only reading a 32-bit int with scanf("%d", &number), and the upper 32 bits are left zero from your dq 0.  It makes no sense to reserve 64 bits of space and then only ask scanf to write the low 32 of it.

See also The advantages of using 32bit registers/instructions in x86-64

Worse, you do cdq (sign extend EAX into EDX:EAX), but then you use 64-bit idiv rbx which divides RDX:RAX by RBX.  If your number input was -15 (base 10), your 128-bit dividend would be 0x00000000FFFFFFFF00000000FFFFFFF1.  Dividing that by a small integer would make the quotient overflow RAX, raising #DE (divide exception).  I didn't test your original version to see if I could provoke that with negative inputs; I changed to using %u for scanf and printf.

It's unclear why you'd want signed division.  Is this program supposed to work for negative inputs?  Your loop condition exits the loop if the counter is less than 2 (signed compare).  Unsigned lets us handle a larger range of inputs with faster 32-bit division.

Use 32-bit division for 32-bit numbers, it's about 2.5x faster than div r64 on Skylake, with similar performance ratios on other Intel CPUs.  See Trial-division code runs 2x faster as 32-bit on Windows than 64-bit on Linux for details.  (idiv and div are pretty similar in performance.  idiv r32 has a faster best-case throughput on Haswell than div r32, according to Agner Fog's instruction tables (https://agner.org/optimize/).  (1 per 8 vs. 9 clock cycles, and 1 fewer uop.  But for 64-bit division, div r64 has better throughput than idiv r64).

You were only checking the low 32-bits of the 64-bit remainder, too.  Since your original input was limited to 32-bit, this might be safe.  So possibly not a correctness problem.

Check a register for zero with test reg,reg, not cmp reg,0.  It saves a byte of code-size.  If the next instruction was js instead of jz, it would also have an advantage of micro-fusing into a compare+branch uop on more CPUs.

Use registers instead of static storage for your tmp variables, especially inside the loop.   That's what registers are for.  Static storage like you're using is equivalent to C static unsigned long long isPrime = 0;, instead of using an automatic storage variable that the compiler could optimize into a register.

asm doesn't have "variables", that's a high level concept that you can implement however you want, with registers, static storage, stack space, or whatever.  The normal way is to use registers and comment your code to keep track of what's where.  Anything that makes your code slower or larger defeats the purpose of writing in asm in the first place.  A compiler would easily make more efficient asm than what you've written.  (This is normal when you're a beginner, so don't feel bad about it, but be aware that looking at compiler output is another good way to learn efficient ways to do things in asm.  See How to remove “noise” from GCC/clang assembly output?)

Don't create a 0/1 boolean and then test it, just branch on the original condition and lay out your code to minimize the amount of jumping around.

You might want to save a 0/1 so you can exit with success or failure, making it possible to use this code as a prime-test in a script or something.  I did that in my version below, so I could verify its correctness with this bash one-liner on Linux.  (I tested it on Linux by using mov rdi, rcx / mov rsi, rdx before every scanf/printf to adapt for the calling convention difference between x86-64 System V and x64 Windows.)

 # check that all the numbers we think are non-prime actually have at least 2 prime factors (expect empty output because we filter out all lines with 2 spaces in `factor` output)
for i in $(seq 3 2 9999 );do echo $i | ./win-primes >/dev/null || echo $i ;done | factor | grep -v ' .* '

# check that all the numbers we think are prime only have 1 prime factor (no lines with multiple spaces in the factor output
for i in $(seq 3 2 9999 );do echo $i | ./win-primes >/dev/null && echo $i ;done | factor | grep ' .* '

Speaking of branching, it's possible to branch much less.  See Why are loops always compiled into do while style tail jumps.

Put one of the conditional branches at the bottom of the loop, and another inside as a break condition.  In my version below, notice that I put the .notprime block after the ret at the end of the main function.  Putting a block out-of-line means you don't have to jump over it in the other path of execution.  Branch layout is a hard problem, figuring out which code can fall through into what other code, and with what values in registers, is part of the fun of writing branchy asm code.  (As opposed to simple loops with mostly SIMD instructions, then the fun in in branchless logic.)

Speaking of which, I probably should have laid out my loop so the fall-through and less-branching case was non-primes.  We expect them to be more common, and jumping around less in the common case is generally best for I-cache footprint and other front-end factors.  The loop naturally lends itself to checking both things in that order after division, though, so I'd have to do some extra setup to skew the loop and put the n%c == 0 branch at the bottom.

Pretty much never use mov rcx, symbol to get a static address into a register.  With nasm -fwin64, it assembles to the 10-byte mov r64, imm64 encoding, which is larger and typically slower to decode and/or fetch from uop cache than a 7-byte RIP-relative LEA.  Also, it needs a load-time fixup for ASLR.

If static addresses fit in 32-bit signed immediates (true in some systems), they usually also fit in 32-bit unsigned, so mov ecx, symbol is shorter (only 5 bytes).

mov ecx, symbol: 5 bytes, mov r32, imm32.  Best choice in position-dependent code (and usable in Windows with LargeAddressAware = no)
mov rcx, strict dword symbol: 7 bytes, mov r/m64, sign_extended_imm32.  Never use, except for kernel code (high addresses).
mov rcx, strict qword symbol: 10 bytes, mov r64, imm64.  Never use.
mov rcx, symbol with nasm -fwin64 is equivalent to strict qword
lea rcx, [rel symbol]: 7 bytes, normally the best choice if mov ecx, symbol isn't available and/or for position-independent code.    (You used default rel so you don't need the rel in every addressing mode).

I don't have a Windows system to test on, but x86_64-w64-mingw32-ld win-primes.obj will link all 4 of these into an executable.  (Unlike trying to link a Linux PIE executable, where 32-bit relocations aren't accepted at all).

Windows executables can be Large Address Aware or not.  If not, pointers will be 32-bit (I think all pointers, not just static code/data label addresses).  Or actually 31-bit, so zero-extension and sign-extension both work, I think, allowing [array + rdx*4] addressing modes.  Anyway, in a Windows non-largeaddress executable, you can use mov ecx, symbol to put a symbol address into a register with only 5 bytes.  It's a tradeoff between needing a load-time fixup (for ASLR) of the immediate in the machine code vs. costing 2 extra bytes for a RIP-relative LEA.

mov reg,immediate can execute on more ports than RIP-relative LEA in some CPUs, but back-end port pressure is not usually a problem for these instructions (no input dependencies so they can run any time there's a spare cycle on the port they're scheduled to).

Don't use BITS 64.  All that does is let you accidentally assemble 64-bit machine code into a 32-bit object file instead of getting an assemble-time error on push rbp, because rbp isn't a register outside of 64-bit mode.

nasm -fwin64 sets the target bitness to 64-bit.  The only time bits 64 is useful for fully 64-bit code is if you want to make a flat binary, e.g. for turning asm into shellcode or a bootloader.  (nasm -fbin doesn't have bin64 or any other option for setting the target mode.)

The main use-case for bits 64 is if you're writing code that starts out in 16 bit mode, and switches the CPU to 64-bit mode.  So the first part of the code would be bits 16, then you might have some bits 32 code that you reach with a jmp far, or maybe go straight to bits 64 after setting up a GDT.  If you're not doing that or didn't understand this paragraph, you don't need and shouldn't use bits 64.

Put your read-only constant data in .rdata, not .data  The .data section is for mutable static data.  Grouping read-only data together into .rdata is good because 1) it catches bugs if you accidentally write to it, and 2) whole pages that are unmodified can be shared between different processes running the same executable.  (shared memory mapping.)  Note that runtime relocation fixups from stuff like mov ecx, symbol in an executable or DLL using ASLR will prevent sharing.

(On non-Windows, the equivalent section is .rodata).

Omit the frame pointer, like gcc -fomit-frame-pointer (on by default with optimization enabled).  You're not accessing the stack through RBP anyway, so you're not saving any code size by having it available instead of using offsets relative to RSP.  So those extra instructions are just costing you code-size and uops for no benefit.  mov rsp, rbp (note that you got this backwards but it's ok because you'd already adjusted RSP so they were equal again anway) + pop rbp is equivalent to leave.  leave costs 3 uops total, one more than mov + pop on Intel CPUs, but it's only 1 per function.  If you already have your stack pointer pointing at the saved RBP value, you should just pop rbp instead of leave or mov + pop.

You actually should use stack space to scanf into, instead of static storage, though.

You're not checking for errors from scanf.  If the user enters invalid input, scanf will return 0 and leave number unmodified, so 0.  That's pretty much ok for asm.  It doesn't get stuck in an infinite loop or crash.

In most languages that would be a big no-no, but you wouldn't normally write the input/output code in asm for real in the first place.

So as long as you're aware you're doing so, leave out error checking if you want.  You can always single-step in a debugger and print RAX after function calls, or even trace all the system calls your program makes.  (See the bottom of https://stackoverflow.com/tags/x86/info for debugging tips.)

Here's how I'd write it

Incorporating some of the things mentioned in other answers as well, like counting up from small divisors (quickly rule out most numbers), and only counting up to ~sqrt(n) because divisors come in pairs.  This vastly speeds up the code for large primes like 2^31-1 = 2147483647.  My version is dominated by startup overhead for that on Linux; perf stat says task clock = 0.339985 ms.  arith.divider_active only reports 180k clock cycles out of ~950k clocks total on my i7-6700k Skylake (including kernel time).  The inner loop should saturate the division unit and just bottleneck on div throughput.

;; bits 64
default rel

extern printf
extern scanf

section .rdata   ;; I typically put static data at the end, but here is fine too
;;    number: dq 0        ; use stack space for these
;    isPrime: dq 1
;    counter: dq 0        ; and just a register for this.

    prompt: db "Which number would you like to check? ", 0
    scan_fmt: db "%u", 0                                   ; %u instead of %d
    numberIsPrime: db "%u is prime", 10, 0
    numberIsNotPrime: db "%u is not prime", 10, 0

section .text

global main
main:
;    push rbp
;    mov rbp, rsp   ; unneeded, we're not using the stack frame

    stack_reserve: equ 32+8
    sub    rsp, stack_reserve   ; shadow space for callees + 8 bytes for stack alignment

    lea    rcx, [prompt]
    call   printf             ; magically flushes stdout with Windows C library

    ; memory from rsp+0 .. rsp+31 has potentially been stepped on by printf
    ; leave RSP where it is, ready for another call

;;; scanf into that 8-byte block of stack space above the shadow space, or into our *own* shadow space
    lea    rdx, [rsp+32]        ; stack addresses are normally 64-bit, can't get away with edx
    lea    rcx, [scan_fmt]
    mov    dword [rdx], 0       ; instead of error check, set n = 0 in case of I/O error
    call   scanf
    ;cmp   eax, 1               ; success = exactly 1 conversion
    ;jnz   .scanf_fail          ; TODO: error check

    mov    r8d, [rsp+32]        ; r8d: 32-bit unsigned number to be checked

    cmp    r8d, 3
    jbe   .prime                ; 2 is prime, and let's consider 0 and 1 prime as well.
                                ; catch 3 here so the loop can avoid the 3%3 == 0 corner case

    test   r8b, 1               ; all *other* even numbers (LSB=0) are non-prime
    jz    .notprime

    ;; n >= 5 at this point
    mov    ecx, 3               ; ECX: trial divisor counter
.divloop:                  ; do {
    mov    eax, r8d
    xor    edx, edx
    div    ecx                ; *much* faster than div rcx

    test   edx, edx
    jz    .notprime           ; if (n%c == 0) goto notprime

    add    ecx, 2             ; we've already ruled out all the even divisors

    cmp    eax, ecx
    ja    .divloop         ; while( n/c > (c+2) );
    ;; loop until c*c > n, i.e. until c >= sqrt(n), because divisors come in pairs
    ;; The c*c == n exact case is caught by the edx==0 test

    ;; Checking based on c*(c+2) lets us exit even earlier,
    ;;  and saves instructions because we can add before cmp
    ;; It's safe: I checked against a known-good primality test.
    ;; It works because any numbers between c*c and c*(c+2) are either prime
    ;;  or have smaller prime factors that we already found

;; fall-through: n is prime
.prime:
    lea    rcx, [numberIsPrime]
    mov    byte [rsp+32], 0
.print:
    mov    edx, r8d        ; n
    call   printf          ; format string already set one of 2 ways
;    mov rsp, rbp
;    pop rbp          ; just use LEAVE if you need this

    ;xor    eax,eax    ; return 0
    movzx  eax, byte [rsp+32]    ; return isprime(n) ? EXIT_SUCCESS(0) : EXIT_FAILURE(1)
    add    rsp, stack_reserve
    ret

.notprime:
    mov    byte [rsp+32], 1            ; store return value on the stack (above printf's shadow space).
                                       ;;  Typically you'd use a call-preserved register but we didn't need any for anything else
    lea    rcx, [numberIsNotPrime]
    jmp   .print
   ;; function tail-duplication would also be an option instead of jmp back
   ;; i.e. call printf  here and fall through to a mov eax,1 / ret

Note the comments describing conditions that hold when execution reaches that comment line.  I find that useful for keeping track of what previous conditions have ruled out.

This actually does one better than checking n/c > c as the loop condition.  n/c > c+2 doesn't miss any primes, and will exit the loop one iteration sooner sometimes.  It also saves instructions by not having to copy the old ecx to edx so you can compare against the old value.

A more intuitive / more-obviously-correct version that does n/c > c needs an extra mov edx,ecx

.divloop:                  ; do {
    mov    eax, r8d
    xor    edx, edx
    div    ecx

    test   edx, edx
    jz    .notprime           ; if (n%c == 0) goto notprime

    mov    edx, ecx           ; save old c for compare
    add    ecx, 2             ; we've already ruled out all the even divisors

    cmp    eax, edx
    ja    .divloop         ; while( n/c > c );

Using the division result in the loop condition means out-of-order execution can't evaluate the loop condition ahead of div progress.  So it won't be able to hide the branch mispredict when we leave the loop.

If we had calculated sqrt(n) ahead of time with something like this:

cvtsi2ss   xmm0, r8      ; signed 64-bit source = zero-extended r8d = uint32_t => float
sqrtss     xmm0, xmm0
cvtss2si   r9d,  xmm0    ; upper bound for ecx = sqrt(n)
;; or maybe we'd need to use sd instead of ss to avoid maybe rounding down

then the divide execution unit would have been busy doing that for about 3 cycles on Skylake.  That might actually be worth it; the branch mispredict penalty is probably higher.  Avoid stalling pipeline by calculating conditional early
.  Skylake has relatively great throughput for FP sqrt, older CPUs are worse.  But it's still slow compared to multiply.  If the throughput cost of one sqrt is less than the branch-mispredict penalty + div latency, then this wins for primes (where we eventually exit the loop by falling through the cmp/ja which OoO exec can't check until the div result for that iteration is ready.)

More importantly, most of the time you'll leave the loop from finding a divisor, unless you expect your inputs to usually be primes, and that branch unavoidably depends on the div result; that's the whole point of doing division.  So on the whole, doing an actual sqrt ahead of time to calculate a loop bound isn't worth it vs. using the clever trick of comparing the divisor and quotient.

To put it another way, an extra sqrtsd at the start delays all the div instructions including the last one by however long sqrt occupies the divider (before an integer div can start).  Or maybe one div could start while cvtsi2sd is running.  But anyway, that's approximately how many extra cycles it would add before execution of the last test/jz uop can detect that the loop should have exited, and starts branch mispredict recovery.

(I'm assuming that the loop-exit branch does mispredict.  This is normal unless you have a loop that runs for the same number of iterations repeatedly, and that count is under about 22 or 23 for Skylake.  It's IT-TAGE branch predictors are based on branch history, so this loop with 2 branches in it might only accurately predict the loop-exit for trip counts of <= ~11 even if the loop has the same trip-count repeatedly.  Neither of those things are probably common.)

sqrtss is faster than sqrtsd.  A double can exactly represent every 32-bit integer (in fact up to ~53 bits, the size of its significand).  But We're probably find with rounding to the nearest float; a 32-bit float has more range than even int64_t so there's no risk of overflow to +Inf.  The only worry is that (float)n could round down, and we could miss a composite like n = prime^2.  You could compensate by always adding half the distance between representable floats for numbers in the size range 232-1, but the extra div operations we end up doing for large primes costs more than just spending a few extra cycles for double.

Even if we could cheaply do int->float conversion with rounding towards +Inf, any rounding up would mean extra loop iterations for large n.  But if we only need correctness for large n and speed for small n, it would be fun.  But changing the MXCSR rounding mode isn't worth it.

If we did use double sqrtsd, we could check check for n being a perfect square just by looking at the FP inexact flag when square-rooting and converting to integer.  If both of those operations are exact, then n was a perfect square.  (But you'd have to reset the IE flag in MXCSR first, and that's slower than just integer squaring and comparison.)


Usability of sliding panels Is there a better way to do what I'm doing? If not, I'd like just a general review of how I did it and the usability of it (how it works and functions, not artistically how it looks).

Here is the working JSFiddle.

JavaScript

$(document).ready(function () {
    // initlize panels
    $('.panel').each(function (i) {
        var $tab = $(this).children('.tab');
        $tab.css({ 'top': $tab.position().top + i * ($tab.height() + 5) });
    });
    $('.tab').click(function () {
        var $panel = $(this).parent();
        if ($panel.attr('data-open') == 'yes') {
            $panel.animate({
                left: '-=300'
            }, 1000, function () {
                // Animation complete.
            });
            $panel.attr('data-open', 'no');
        } else {
            $('.panel').css({ 'z-index': 0 });
            $panel.css({ 'z-index': 1 });
            $panel.animate({
                left: '+=300'
            }, 1000, function () {
                // Animation complete.
            });
            $panel.attr('data-open', 'yes');
        }
    });
});

CSS

body
{
    margin: 0px;
    padding: 0px;
    font-family: Arial;
}
.panel
{
    position:absolute;
    top: 0px;
    bottom: 0px;
    left: -300px;
    width: 299px;
    border-right: 1px solid black;
    background-color: White;
}

.tab
{
    position: absolute;
    top: 10px;
    width: 16px;
    right: -17px;
    height: 75px;
    border-top: 1px solid black;
    border-bottom: 1px solid black;
    border-right: 1px solid black;
    cursor: pointer;
    background-color: White;
}

.tab div
{
    position: relative;
    display: block;
    left: 1px;
    top: -15px; /* Same as height */
    width: 75px; /* Same as .tab height */
    height: 15px;
    font-size: 14px;
    font-weight:normal;
    line-height: 15px;
    text-align: center;
    -moz-transform:rotate(-270deg);
    -moz-transform-origin: bottom left;
    -webkit-transform: rotate(-270deg);
    -webkit-transform-origin: bottom left;
    -o-transform: rotate(-270deg);
    -o-transform-origin:  bottom left;
    filter: progid:DXImageTransform.Microsoft.BasicImage(rotation=1);
    vertical-align: top;
}

CSS - in an IE-only CSS file

.tab div
{
    top: 0;
}

HTML

Tab A

Tab B

Tab C

Tab D

your tab label seems to exist within the content which it is tabbing .. kinda bad, don't you think.
there is no point in using $(document).ready(), just put your script file right before </body> , and it will already be executing, when the DOM is done.
instead of adding function for each .tab tag's onclick separately, you should read about event delegation and use that
use closures for storing the active tab, instead of DOM. DOM manipulations are slow
setting top and bottom values in CSS fails to do what you want on IE7
animation quality on IE9 is horrid


URL pattern matching for request blocking I'm trying to write a URL pattern matching utility that, given a pattern and a URL it'll tell whether the URL matches the pattern or not. I want to use this for users to provide a list of URL patterns that will be use to block requests to URLs that match the given pattern. I don't want to use straight Regex because it'd be painful for the user to write, especially since all dots would need to be escaped.

I found this set of pattern matching rules which seem simple yet powerful enough for what I need. So I want to write a class that matches the specs given on that page. In the class below I attempt to do this, although I leave the "file" scheme out since I won't be needing it. Also, I'm letting the user provide no path, in which case I'll consider it equivalent to providing an empty path. Finally, I'm letting the user use extra asterisks on the host, which would be treated as literal asterisks, which I don't think should be much of a problem. I've tested it with the cases listed on the page and a few others and worked fine, but I wonder if there's some edge cases I'm not considering that break Google specifications.

public class UrlPattern
{
    public readonly Regex SchemeMatch;
    public readonly Regex HostMatch;
    public readonly Regex PathMatch;

    public UrlPattern(string pattern)
    {
        if (string.IsNullOrWhiteSpace(pattern))
            throw new ArgumentException("Pattern cannot be null or whitespace.");

        pattern = pattern.Trim();

        var index = pattern.IndexOf("://");

        if (index < 0)
            throw new ArgumentException("Pattern must have a scheme-host separator.");

        var scheme = pattern.Substring(0, index);
        var hostAndPath = pattern.Substring(index + 3);

        index = hostAndPath.IndexOf("/");

        if (index < 0)
        {
            hostAndPath = $"{hostAndPath}/";
            index = hostAndPath.IndexOf("/");
        }

        var host = hostAndPath.Substring(0, index);
        var path = hostAndPath.Substring(index);

        var schemePattern = MakeSchemePattern(scheme);
        var hostPattern = MakeHostPattern(host);
        var pathPattern = MakePathPattern(path);

        SchemeMatch = new Regex(schemePattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
        HostMatch = new Regex(hostPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
        PathMatch = new Regex(pathPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase);
    }

    private static string MakePathPattern(string path)
    {
        return $"^{Regex.Escape(path).Replace("\\*", ".*")}$";
    }

    private static string MakeHostPattern(string host)
    {
        if (host == "*")
            return ".+";

        if (host.StartsWith("*."))
            return $"^(.+\\.)?{Regex.Escape(host.Substring(2))}$";

        return $"^{Regex.Escape(host)}$";
    }

    private static string MakeSchemePattern(string scheme)
    {
        switch (scheme)
        {
            case "*":
                return "^https?$";
            case "http":
                return "^http$";
            case "https":
                return "^https$";
            case "ftp":
                return "^ftp$";
            default:
                throw new ArgumentException("Invalid scheme pattern.");
        }
    }

    public bool IsMatch(Uri uri)
    {
        if (!SchemeMatch.IsMatch(uri.Scheme))
            return false;

        if (!HostMatch.IsMatch(uri.Host))
            return false;

        if (!PathMatch.IsMatch(uri.PathAndQuery))
            return false;

        return true;
    }

    public bool IsMatch(string url)
    {
        return IsMatch(new Uri(url));
    }
}

General considerations

You are writing a "denylist". There's some clean and simple denylist rules that can help rule out a lot of things already. One of these rules is denying URLs based on their scheme alone.

A few examples for schemes that you may not have considered:
steam://, file://, sftp://, chrome://, about://

Most of these are URLs you most likely want to block. In that case "inverted matching" behaviour is really useful.

Furthermore denylists usually tend to be used for "overblocking out of caution". I'm not sure how that's for your exact use-case, but if you want that to happen, making your patterns as general as possible is a great idea :)

URI vs. URL:
Something to additionally keep in mind here: URI stands for "Uniform Resource Identifier" and URL for "Uniform Resource Locator". Your code specifically exposes a Uri, not a Url overload for matching. This implies that one should be able to blacklist things like the following URIs:

git@github.com:username/repository.git
mailto:username@example.com
irc://#c-sharp@freenode

If your class is only intended to deal with URLs, you should only expose an overload for Url :)

Personal preferences

I personally very much prefer to use factory methods over constructors to validate arguments. YMMV, but I expect constructors to not throw exceptions.

I also prefer to add braces wherever they delimit a block. I can understand the omission for the argument checking though.

Implementation considerations

I like that you're using a lot of intermediate variables. Some minor naming nitpicks could be had there, but I think this code will get the most benefit from using tuples:

var (scheme, hostAndPath) = SplitSchemeHost(pattern);
var (host, path) = SplitHostPath(hostAndPath);

This allows you to push the index matching behaviour into these methods and keep it out of the constructor. (Minor nitpick: 3 is a magic number in the hostAndPath creation)

Aside from that this code is very clean already. I'd maybe not create the patterns within the interpolated strings, but given the simplicity here it seems appropriate to do so.


This spec creates very few connects, is that plausible? I'm attempting exercise A.2.1 from the book http://alloytools.org/book.html. Having got this far, I find that when stepping through instances I only ever get 2 connects links, no matter how many requests links, when using the MiniSat solver. When using MiniSat plus Unsat core, I sometimes get more connects, but not many requests. Obviously, these all satisfy the model, but it feels wrong to me. Am I over-thinking this?
module exercises/telephone

sig Phone {
    , requests : set Phone
    , connects: lone Phone
}

fact minimum_system {
    some requests
    some connects
}

fact dont_call_yourself {
  no p: Phone | p.connects = p
  no p: Phone | p in p.requests
}

fact connect_from_a_request {
    no (connects - requests)
}

fact only_receive_once {
    all p: Phone.connects | one ~connects[p]
}

fact receiver_or_caller_only {
    no (Phone.connects & Phone.~connects)
}

pred show {}

run show for exactly 12 Phone

When you get lots of instances, it's not really practical to look through them all. What most Alloy users would do in this case is to add a constraint to the show predicate to see if you can get the instance you expect. So I modified it to
pred show {
#connects > 4
#requests > 4
}

and now I see instances with more than 4 connects and requests.
Another suggestion: don't use exactly unless you really need to. You usually want to look for all instances up to some bound, and if you set it to exactly, you might find that you get no instances because it's not possible to construct one with exactly that number. For example, if you don't allow conference calls and you require phones to matched one to one, and you then set the bound to "exactly 3" you would get no solutions.


Three-number lottery simulator Please let me know how I can make it better, especially with avoiding repetition without the need to replace the user guesses.

import java.util.Scanner;
public class Lottery {

    static int guess[] = new int[3],actual[]= new int[3];

    private static void aquireGuess()
    {
        Scanner keyboard = new Scanner(System.in);
        System.out.println("Time to play the lottery please choose three numbers ranging from zero to nine.");
        System.out.print("Please enter first number:");
        guess[0] = keyboard.nextInt();
        System.out.print("Please enter second number:");
        guess[1] = keyboard.nextInt();
        System.out.print("Please enter third number:");
        guess[2] = keyboard.nextInt();
        keyboard.close();
    }

    private static void generateLottery()
    {
        actual[0] = (int)(Math.random()*10);
        actual[1] = (int)(Math.random()*10);
        actual[2]= (int)(Math.random()*10);
    }

    private static void compare()
    {
        if(guess[0]==actual[0]&&guess[1]==actual[1]&&guess[2]==actual[2])
        {
            System.out.println("Congradulations three matches in exact order!!! You win $1,000,000.");
            return;
        }
        int count = 0;
        for(int i=0;i<=2;i++)
        {
            for(int x=0;x<=2;x++)
            {
                if (actual[i]==guess[x])
                {
                    count++;
                    guess[x]=-1;
                    break;
                }
            }
        }
        switch(count)
                {
        case 0:System.out.println("Sorry better luck next time."); break;
        case 1:System.out.println("Good job. One match. You win $10."); break;
        case 2:System.out.println("Awsome you made two matches. You win $100."); break;
        case 3:System.out.println("Woohoo you matched all three. You win $1,000.");break;
                }
    }

    public static void main(String args[])
    {
        aquireGuess();
        generateLottery();
        System.out.println("Your guess were "+guess[0]+" "+guess[1]+" "+guess[2]);
        System.out.println("The lottery numbers were "+actual[0]+" "+actual[1]+" "+actual[2]);
        compare();
    }
}

Using method return types

Instead of relying on static int[] arrays, you should return the appropriate results from getting the user input and generating the lottery numbers so that their interaction is slightly clearer.

Validating user input

Scanner.nextInt() throws an InputMismatchException when the next input is not parsable as an integer. You should have a method that handles that safely.
For example (prompted by @KonradMorawski's comment):

private static int getInteger(Scanner scanner) {
    System.out.println("Enter an integer:");
    while (!scanner.hasNextInt()) {
        System.out.println("Not an integer, please try again.");
        scanner.next();
    }
    return scanner.nextInt();
}

Comparing arrays

Arrays.equals(int[], int[]) can be used to compare two int[] arrays.

Using other types for comparison

You may also want to consider using other types to compare the lottery numbers. For example, a plain integer works as well:

private static int getNumber(int a, int b, int c) {
    return a * 100 + b * 10 + c;
}

edit:

try-with-resources

If you are on Java 7 and above, you should use try-with-resources for efficient handling of the underlying I/O resource:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        // ...
    }
}