Determine whether path A→B→C on a plane constitutes a left or right turn I have recently started learning functional programming and Haskell. I have encountered this problem:

  Consider three two-dimensional points a, b, and c. If we look at the angle formed by the line segment from a to b and the line segment from b to c, it either turns left, turns right, or forms a straight line. Define a Direction data type that lets you represent these possibilities. Write a function that calculates the turn made by three 2D points and returns a Direction.

I have heard that one of the goals of the Haskell language is to create elegant solutions.

This is my solution:

data Direction = LeftTurn | RightTurn | Straight deriving Show

findDirection (x, y) (x1, y1) (x2, y2)
    | (x, y) == (x1, y1)          = Straight
    | (x1, y1) == (x2, y2)        = Straight
    | (x, y) == (x2, y2)          = Straight
    | x == x1 && x1 == x2         = Straight
    | x1 == x && y1 > y && x2 > x = RightTurn
    | x1 == x && y1 > y && x2 < x = LeftTurn
    | x1 == x && y1 < y && x2 < x = RightTurn
    | x1 == x && y1 < y && x2 > x = LeftTurn
    | x1 > x && y2 > y_on_line    = LeftTurn
    | x1 > x && y2 < y_on_line    = RightTurn
    | x1 > x && y2 == y_on_line   = Straight
    | x1 < x && y2 > y_on_line    = RightTurn
    | x1 < x && y2 < y_on_line    = LeftTurn
    | x1 < x && y2 == y_on_line   = Straight
   where y_on_line   = slope * x2 + y_intercept
         slope       = (y1 - y) / (x1 - x)
         y_intercept = y - slope * x

This looks like it is too long to be considered an elegant solution.

Welcome to Haskell (and Code Review). Your ADT Direction is a good approach for the result. However, top-level functions should have a type, as it will make type errors easier to read and give other users an idea how to use your function:

type Point2 = (Double, Double)

findDirection :: Point2 -> Point2 -> Point2 -> Direction
findDirection (x, y) (x1, y1) (x2, y2)

While isn't necessary—GHC can infer it—it can at least act as a minimal documentation.

Next, we should tackle the names. We have x, x1 and x2. Those seem arbitrary. Why don't we start with x1? The names (ax,ay), (bx,by) and (cx,cy) would be more suitable, since we're talking about the coordinates of three specific points, a, b and c:

findDirection :: Point2 -> Point2 -> Point2 -> Direction
findDirection (ax, ay) (bx, by) (cx, cy)

However, as vnp said, you should use the cross product. While the cross-product is only defined for three dimensions, we can simply embed your points into \$\mathbb R^3\$ and then take the Z-component of the resulting vector:

type Vector2 = (Double, Double)

-- See https://en.wikipedia.org/wiki/Cross_product#Computing_the_cross_product
cross :: Vector2 -> Vector2 -> Double
cross (ax, ay) (bx, by) = error "Implement this as an exercise"

With yet another function, we can then get the two vectors from our original three points:

diff :: Point2 -> Point2 -> Vector2
diff (ax, ay) (bx, by) = (bx - ax, by - ay)

Now we really end up with an elegant solution:

findDirection :: Point2 -> Point2 -> Point2 -> Direction
findDirection a b c =
    case cross (a `diff` b) (b `diff` c) `compare` 0 of
        GT -> LeftTurn
        EQ -> Straight
        LT -> RightTurn


Pushing instantiated class in an array I've just stumbled upon something today that seemed "unethical" in PHP. What I originally thought as malpractice, turned out to work just fine. Basically what I was wondering is if pushing an instantiated class in an array was considered proper?

Example:

$array = array();

array_push($array, new myClass());

foreach($array as $i => $class)
{
    $class->doSomething();
}

Although this example is quite vague, the idea is there. Any input or alternatives on this matter would be great.

That example looks like a very simple "chain of command" implementation.
Looks fine to me, nothing unusual (though YYMV depending on the code-base).

You'll see things like this as an alternative to long blocks of procedural code, often when there's a bunch of distinct actions to perform that have no relation to one another. It's a nice way to clean up and cut up large classes to keep things "SRP"'d.


Start and end times of recurring events I'm working on a function that returns a specific set of times for a given timezone and a given frequency; day, week, month.  For any given timezone and a frequency the function will return the unixtime hour start and unixtime hour end of the last full frequency given.  For example, the last full day, the last full 7 days or the last full month.

import calendar
import pytz
import time

from datetime import datetime, timedelta, time

def get_international_dates(timezone, frequency):
    tz = pytz.timezone(timezone)
    today = datetime.now(tz).date()
    midnight = tz.localize(datetime.combine(today, time(0, 0)), is_dst=True)
    last_full_day_midnight = int(midnight.astimezone(pytz.utc).strftime("%s"))
    if frequency == 'd':
        day_end = last_full_day_midnight - 3600
        day_start = last_full_day_midnight - (3600 * 24)
        prev_day_end = day_end - (3600 * 24)
        prev_day_start = day_start - (3600 * 24)
        return {'start': day_start, 'end': day_end, 'prev_start': prev_day_start, 'prev_end': prev_day_end}
    if frequency == 'w':
        week_end = last_full_day_midnight - 3600
        week_start = last_full_day_midnight - (3600 * 24 * 7)
        prev_week_end = week_end - (3600 * 24 * 7)
        prev_week_start = week_start - (3600 * 24 * 7)
        return {'start': week_start, 'end': week_end, 'prev_start': prev_week_start, 'prev_end': prev_week_end}
    if frequency == 'm':
        month_length = calendar.monthrange(today.year, today.month - 1)
        month_end = last_full_day_midnight - 3600
        month_start = last_full_day_midnight - (3600 * 24 * month_length[1])
        prev_month_end = month_end - (3600 * 24 * month_length[1])
        prev_month_start = month_start - (3600 * 24 * month_length[1])
        return {'start': month_start, 'end': month_end, 'prev_start': prev_month_start, 'prev_end': prev_month_end}

This function works as it should, but it seems pretty messy and un-pythonic.  What should I do to make this more clean, concise and pythonic?

Some small things that jump out to me:

It looks like a lot of duplication in each if check
There are a lot of magic numbers
Those later if checks should probably be elif, unless it's possible to satisfy several of them in one pass
Day/week/month end are identical
You should probably have a single return at the bottom - the things that fill it are the same each time, so just use the checks to determine what gets returned.  Then you only need to change the formatting in one place if you decide to later.

I think I'd prefer to write this with a dictionary mapping adjustments to each type of frequency.  After doing this, calculating the return values for each category can be done by the same code:

adjustment = {
    end: {#dict for adjustments for each frequency},
    start: {#dict for adjustments for each frequency},
    prev_end: {#dict for adjustments for each frequency},
    prev_start: {#dict for adjustments for each frequency}
}

def get_international_dates(timezone, frequency):
    tz = pytz.timezone(timezone)
    today = datetime.now(tz).date()
    midnight = tz.localize(datetime.combine(today, time(0, 0)), is_dst=True)
    last_full_day_midnight = int(midnight.astimezone(pytz.utc).strftime("%s"))

    end = last_full_day_midnight - adjustment[end][frequency]
    start = end - adjustment[start][frequency]
    prev_end = end - adjustment[prev_end][frequency]
    prev_start = start - adjustment[prev_start][frequency]

    return end, start, prev_end, prev_start


Expandable fizz buzz javascript I've been learning JavaScript for a few weeks and thought to give the ol' fizzbuzz another try. My goal was for expandability. In other words, I wanted to be able to add more keywords without having to rewrite parts of the code.

I tried to add comments to clarify some of the more unorthodox parts of my code. However, I still feel my code is somewhat terse... Is it better to just write the plain old for-loops for readability and support of IE?

I also opted to check if the string existed instead of checking if the length was greater than zero. Are there any pitfalls I might fall into just checking for existence? I mean if my key were false it would fail, however that would never happen as all the key values are strings anyways.

function fizzBuzz(divisors = {
        fizz: 3,
        buzz: 5
    }, maxNumber = 100) {
        /* Takes in a object of divisors of the form
           divisors = {
               name: value,
           };
           and an integer. Loops through the list if number below or equal
           maxNumber. If a number is divisible by one of the values, print the name.
          */
        for (var num = 1; num <= maxNumber; num++) {
            var str = "";
            // Loops through the keys of the divisors
            Object.keys(divisors).forEach(function(key)  {
                // Ternary expression, if divisible append key else do nothing
                str += num % divisors[key] == 0 ? key : '';
            });
            // If anything got appended print string else num
            console.log(str ? str : num)
        }
    }

    fizzBuzz()

What if I wanted every 7 number to also add fizz, so 105 would be fizzbuzzfizz? You can't do that in your code, since you are using the string as the (unique) key and the number as the value. Instead, use the number as the key and the string as the value.

While it will work in most cases, the order of properties in an object is not guaranteed. I would change this to either use an array of object or a map.

What if you wanted just the result form a single number? Or make a function that found the first number that gives fizzbuzz? Right now you are printing the result, so you can't do anything else with it. Make a separate function to get the result, which you can also use for other things.

function fizzBuzz(divisors = new Map([[3,'fizz'],[5,'buzz']]), maxNumber = 100) {
  for (var num = 1; num <= maxNumber; num++) {
    console.log(fizzValue(divisors, num));
  }
}

function fizzValue(divisors, num) {
  var str = "";
  divisors.forEach((value, key) => {
   str += num % key == 0 ? value : '';
  });
  return (str ? str : num);
}

fizzBuzz();


C scrabble counter with piping input I want to start making some command line programs in C, this is a fork of something else I made but is more suitable to this site. My big question is there a better way to pipe commands? I think the way I did it feels improper. I especially feel the string method is a bit hacky.
#include <stdio.h>
#include <unistd.h>
#include <ctype.h>
#include <string.h>
int main(int argc, char *argv[])
{
    char t[32767];
    unsigned int val = 0;
    if (argc == 2)
    {
        memset(t, '\0', sizeof(t));
        strcpy(t, argv[1]);
    }
    else
    {
        read(STDIN_FILENO, t, 32767);
    }
    for (int i = 0; t[i]; i++)
    {
        t[i] = tolower(t[i]);
    }
    int i;
    for (i = 0; t[i] != '\0'; i++)
    {
        switch (t[i])
        {
        case 'a':
        case 'e':
        case 'i':
        case 'o':
        case 'u':
        case 'l':
        case 'n':
        case 's':
        case 't':
        case 'r':
            val += 1;
            break;
        case 'd':
        case 'g':
            val += 2;
            break;
        case 'b':
        case 'c':
        case 'm':
        case 'p':
            val += 3;
            break;
        case 'f':
        case 'h':
        case 'v':
        case 'w':
        case 'y':
            val += 4;
            break;
        case 'k':
            val += 5;
            break;
        case 'j':
        case 'x':
            val += 8;
            break;
        case 'q':
        case 'z':
            val += 10;
        }
    }
    printf("%i\n", val);
    return 0;
}

I know this is similar to Scrabble word scoring, but I am less interested in the switch but the stdin and the piping.

General Observations
This code should work just fine, but performance can be enhanced.
Initialize Arrays to Zero at Compile Time
Currently there is a performance hit in the code when initializing the array t to zero. The function memset() is a runtime function, by initializing the array t to zero there is no run time cost:
    char t[32767] = {0};

The previous line of code initializes the entire array to zero (equivalent to the call to memset()) and should not affect run time.
Use Table Lookup Over Long Switch Statements
The Scrabble word scoring question linked to in this question uses a table look up instead of a long switch statement. Table lookup is better for 2 reasons, the first is that it will only be one line of code rather than at least 26 lines of code, the second reason is that table lookup should be much faster than the logic that implements the switch/case statement.
Magic Numbers
The code includes unistd.h for the definition of the symbolic constant STDIN_FILENO but the numeric constant 32767 is used multiple times rather than defining a symbolic constant for it. It would be better to define a symbolic constant for 32767 using either a const declaration or a #define preproccessor statement.
    const size_t INPUT_BUFFER_SIZE = 32767;

or
#define INPUT_BUFFER_SIZE 32767

This makes the code more readable and much easier to maintain, since an edit to change the value would only need to be performed on one line rather than having to search through the code to replace every instance of the numeric constant.
    char t[INPUT_BUFFER_SIZE];
    unsigned int val = 0;
    if (argc == 2)
    {
        memset(t, '\0', sizeof(t));
        strcpy(t, argv[1]);
    }
    else
    {
        read(STDIN_FILENO, t, INPUT_BUFFER_SIZE);
    }

Portability
The include file unistd.h does not port to Windows or some other non POSIX systems, which means this code won't even compile on some systems. While it is less readable, using zero is more portable, or have an ifdef in the code to define STDIN_FILENO on all systems.
#ifndef STDIN_FILENO
#define STDIN_FILENO 0
#endif

The Code Doesn't Allow for Switches on the Commandline
The code makes an assumption that there is no commandline switch that precedes the input to be processed if argc is greater than 1. This could result in bugs in the future. It might be best to have some kind of check for switches in the command line processing.
Complexity
If the code continues to use the switch/case statement then it would be better to move that statement into it's own function that returns an integer value.


Security - Login system For a school project I have been asked to create a login system for a website. The language I have to use is PHP with no database as of yet (for a later assignment). I'm just wondering if this code has any security flaws.

Here is my current code:

Navigation.php note that this is to be included in another html document.

<link type="text/css" rel="stylesheet" href="style_nav.css">

Index

Test

not-in-use

    <?php
    session_start();

    $_SESSION['last_phage'] = $_SERVER['PHP_SELF'];

    if (empty($_SESSION['user'])) {
        echo '
        <form action="login.php" method="POST">

        Username: <input type="text" name="username">
        Password: <input type="password" name="password">&nbsp;&nbsp;
        <input type="submit" value="Login">

        </form>';
    } else {
        echo 'Welcome! ' . $_SESSION['user'] . '<form action="login.php" method="POST"><input type="submit" value="logout" name="logout"></form>';
    }
    ?>

Login.php

<?php

session_start();

if (isset($_POST['logout'])) {
    session_destroy();
    header("Location: index.php");
}

if (isset($_POST['username']) && isset($_POST['password'])) {
    if (!empty($_POST['username']) && !empty($_POST['password'])) {
        $username = $_POST['username'];
        $password = $_POST['password'];

        if ($username == 'kad' && $password == 'lol') {  //this is just temporary without a database
            $_SESSION['user'] = $username;

            header("Location:" . $_SESSION['last_phage']);
        }

        header("Location: " . $_SESSION['last_phage']);
    }
}
?>

Here's my 2 cents as Java "developer" with a little experience in c# and vba.

Your security here largely depends on whether you send the password and username in clear text or encrypted. You should force HTTPS protocol for access to the page.

You might also want to show errors if the login is invalid:

$valid = true;
if(!isset($_POST['username']) || empty($_POST['username'])){
   $errors .= "Please enter a username.
"
   $valid = false;
}
if(!isset($_POST['password']) || empty($_POST['password'])){
   $errors .= "Please enter a pasword.
"
   $valid = false;
}
if($valid){
  //check for username and password compliance
}

Also it would definitely be better to have the logout be handled in a different file than the login


Appending unique data to a postgres table I receive daily files (filename_%m_%d_%Y.csv) from a client and I read those in pandas, process them, and store them in Postgres. Sometimes there are delays and we do not get the data for a few days. The problem is that I have everything on a scheduled task and I do not want to manually check if the files exist or not before I run my code since I am using append mode on Postgres (and do not have constraints set up yet).

My approach here is to read the last few days of data from my existing database, and then find the max date in order to know where to start reading new data from. So if the max date is Jan 22, I will know that the start date for reading my new files is Jan 23 onward. Is this a decent approach?

central_time = dt.datetime.now(pytz.timezone('US/Central'))
start_date = central_time - pd.Timedelta(days = 7)
engine = sa.create_engine('postgresql://usr:pass@localhost:XXXX/db')
df = pd.read_sql(sa.text('SELECT * FROM table WHERE report_date > :date'), engine, params={'date': start_date}, parse_dates = ['report_date'])
latest_date = df.report_date.max() + pd.Timedelta(days = 1)

Up until this week I have been exclusively using .csv files. My approach here was to find the beginning of the month and read all files from the beginning of the month until the current date. This meant that everyday I would overwrite my file (So this month I would read Jan 1 - 27 daily files, and create a January file, then tomorrow I would read Jan 1 - 28 daily files and then overwrite my previous MTD file).

start_date = (central_time - pd.Timedelta(days = 0)) - pd.offsets.MonthBegin()
end_date = central_time
days = ((pd.Timestamp(end_date) - pd.Timestamp(start_date)) / np.timedelta64(1, 'D')).astype(int)

def last_n_days(filename, num_of_days, include_curr = False):
    f = '%m_%d_%Y'
    curr = end_date
    if include_curr:
        yield curr.strftime(f)
    for num in range(num_of_days):
        curr = curr - dt.timedelta(days = 1)
        yield curr.strftime(f)

First of all I don't know Python, so I will try my best.

For the first portion of code, why do you put 7 days as offset? If the file delays more than 7 days your script will crash. I would do a SQL query for that. One like this:

Select max(report_date) from table;

and now you have the latest date.

For the second part your code seems ok, but personally I don't see the reason to update the whole month every day, that will be problematic.

For me, the best solution is find the latest date, I gave you the query up there, and after that advance day by day looking for files up to today.


spyOn click events and check call function I'm trying to write Jasmine tests and I need a review and some advice on spying on events and their behavior.

// my example source code
;(function () {
  var app = {

    exampleModuleName: {
      navigateTo: function () { /* do something */ }
    },

    navigation: {
      init: function () {
        this.routeHandler = this.handleRouteTo.bind(this);

        // bind all events
        this.bind();
      },
      bind: function () {
        $(document).on('click', '.js-route-to', this.routeHandler);
      },
      teardown: function () {
        $(document).off('click', this.routeHandler);
      },

      handleRouteTo: function (event) {
        event.preventDefault();
        this.routeTo($(event.currentTarget).data('target'));
      },

      routeTo: function (target) {
        var module = app[target];
        if (module && $.isFunction(module.navigateTo)) {
            module.navigateTo();
        }
      },
    }
  };
}());

And that is my uncompleted spec file.

// my test code
describe('App Navigation', function () {

  describe('initiation the navigation module', function() {

    it ('spy on behavior', function () {
        var spy = spyOn(app.navigation, 'init');
        app.navigation.init();
        expect(spy).toHaveBeenCalled();
    });

    // which tests are missing to be sure about well tested properties and methodes?
   });

  describe('test routeTo handler', function() {
    var nav = app.navigation;

    beforeEach(function () {
        this.routeHandler = nav.handleRouteTo.bind(this);
    });

    it ('should be defined', function () {
        expect(this.routeHandler).toBeDefined();
    });

    it ('click at a link', function () {
        var node = $('some text');

        expect(node.length).toBe(1);

        spyOn(nav, 'handleRouteTo');
        node.click();

        // why is it failed?
        expect(nav.handleRouteTo).toHaveBeenCalled();
    });

  });
});

Please take a look and let us talk about this example. I want to learn more about testing.

From a once over:

I would drop this test, you are testing Jasmine and/or the test, but not the results of calling app.navigation.init()

it ('spy on behavior', function () {
    var spy = spyOn(app.navigation, 'init');
    app.navigation.init();
    expect(spy).toHaveBeenCalled();
});

Be aware that Function.bind() which you use in navigation.init() creates a brand new function, so this.routeHandler never calls this.hanldeRouteTo
Also be aware that you are not calling app.navigation.bind() as far as I can tell, so the binding of the event is not happening
Perhaps you are not a native English native speaker, but your describe calls are not up to snuff. It hampers my reading flow of the code. Example: 'initiation the navigation module'


Is Concurrent Collection Needed Here? I'm playing around with switching some code to using Parallel.ForEach and I have a question about whether I need to be concerned about the effects of concurrency here. Here's a simplified version of what I'm doing:

public IList<Item> RetrieveAllItems(int paramValue)
{
    var itemFuncs = new List<Func<int, IEnumerable<Item>>>
                {
                    RetrieveItems1,//one method to retrieve items, taking an int param
                    RetrieveItems2//another one that takes an int param
                };

    List<Item> allItems = new List<Item>();

    Parallel.ForEach(itemFuncs, (f) =>
                {
                    //is this a problem?
                    allItems.AddRange(f(paramValue));
                });

    return allItems;
}

For this application, I do not care about the order that my Item objects are added to allItems (in fact, I do explicit ordering later on). My question is whether I could get into trouble here with both functions attempting to add Item objects at the same time.

My motivation for playing around with this is simply to try to speed up this sequence of events, and when I tried using a BlockingCollection instead, my timings, obviously slowed down to the point that there was hardly any difference between doing these two operations in parallel versus the old, serial way. When I use a simple List<Item> then I do see impressive speed-ups.

So, considering I am just adding objects to a collection in a parallel fashion, is using a non-thread safe collection asking for trouble here? My real-world experience with multi-threaded/parallel programming is very sparse, hence the uncertainty here.

Yes, what you are doing is a problem. List is not thread-safe and adding items from multiple threads can lead to data corruption (for example individual elements overwriting each other).

You said that you don't care about the order of items inserted quickest fix is to use ConcurrentBag

Another option is to the PLINQ extensions and use the Select extension of the ParallelQuery. The code would look like this:

return itemFuncs.AsParallel().SelectMany(f => f(paramValue)).ToList();


Decimal-to-fraction solver I have created a program that turns a decimal into a fraction:

package main

import (
        "fmt"
        "github.com/retep-mathwizard/utils/convert"
        "github.com/retep-mathwizard/utils/input"
        "math"
        "strings"
)

func main() {
        fmt.Println("Input your decimal")
        num := input.StringInput(" > ")
        digits := num[strings.Index(num, ".")+1:]
        decimal := num[strings.Index(num, "."):]
        numbers := num[:strings.Index(num, ".")]
        denom := convert.FloatToInt(math.Pow(10, convert.IntToFloat(len(digits))))
        numer := convert.FloatToInt(convert.StringToFloat(decimal) * convert.IntToFloat(denom))
        var gcf int
        for i := numer/2 + 1; i >= 0; i-- {
                if (numer%i == 0) && (denom%i == 0) {
                        gcf = i
                        break
                }

        }
        newnum := numer / gcf
        newdum := denom / gcf
        fmt.Println(numbers+" and ", newnum)
        fmt.Println("    -------")
        fmt.Println("      ", newdum)
}

I need to implement repeating fractions, and a maximum decimal length. Over 13 digits, and it takes a while. Feel free to butcher my code!

Note: Your code is not useful. Rational numbers are available in the Go math/big standard library. For example,

package main

import (
    "fmt"
    "math/big"
)

func main() {
    fmt.Println("Enter a number:")
    var r = new(big.Rat)
    fmt.Scanln(r)
    fmt.Println(r)
}

I read your code, the code you imported from github.com/retep-mathwizard, and the code from rolfl's answer. I see problems.

Let's start with something simple, the calculation of the greatest common divisor or factor.

You embedded this code in a slab of code. It should be a function. For example,

func gcf(numer, denom int) int {
    var gcf int
    for i := numer/2 + 1; i >= 0; i-- {
        if (numer%i == 0) && (denom%i == 0) {
            gcf = i
            break
        }
    }
    return gcf
}

It's very inefficient. For well over two thousand years we have known of a much better way to do this.

Rolfl used a much better algorithm.

// Euclidian algorithm
func gcf(a, b int64) int64 {

    if a < b {
        return gcf(b, a)
    }

    if b == 0 {
        return a
    }

    a = a % b
    return gcf(b, a)
}

If you put blank lines between each sentence you can't see very much on a single screen. It's essential that code be readable. Think of blank lines as more like paragraph separators.

The algorithm is implemented using recursion. Any recursive algorithm can be written as an iterative algorithm. Go doesn't have tail recursion. Limit the use of recursion to the few cases where recursion is much easier to understand than iteration.

Here's an idiomatic Go GCD function. It's simple, direct, and fast.

func gcd(x, y int64) int64 {
    for y != 0 {
        x, y = y, x%y
    }
    return x
}

Here are some Go benchmarks which show how slow your GCF algorithm is.

BenchmarkGCFXXX        30000         54564 ns/op
BenchmarkGCFPeter   10000000           156 ns/op

Go types play an essential role in writing idiomatic Go. Rational is an obvious type:

// A rational number r is expressed as the fraction p/q of two integers:
// r = p/q = (d*i+n)/d.
type Rational struct {
    i int64 // integer
    n int64 // fraction numerator
    d int64 // fraction denominator
}

Like many types, Rational should have a formatted string function. Your formatted output is ugly

7182818284590 and  141592653
    -------
       1000000000

I think this looks much better

7182818284590 + 141592653/1000000000

Here's a Go String method for the Rational type. Note the attention to pretty-printing details.

func (r Rational) String() string {
    var s string
    if r.i != 0 {
        s += strconv.FormatInt(r.i, 10)
    }
    if r.n != 0 {
        if r.i != 0 {
            s += " + "
        }
        if r.d < 0 {
            r.n *= -1
            r.d *= -1
        }
        s += strconv.FormatInt(r.n, 10) + "/" + strconv.FormatInt(r.d, 10)
    }
    if len(s) == 0 {
        s += "0"
    }
    return s
}

An obvious indication of problems is the difficulty in testing. Your program only handles one input value. Rolfl hardcoded a single fixed value: num := "12345.6785".

For a problem like this, which is driven by user input, the Go main function should be an input loop. For example,

func main() {
    snr := bufio.NewScanner(os.Stdin)
    enter := "Enter a decimal number:"
    for fmt.Println(enter); snr.Scan(); fmt.Println(enter) {
        d := snr.Text()
        if len(d) == 0 {
            break
        }
        r, err := ParseDecimal(d)
        if err != nil {
            fmt.Fprintln(os.Stderr, "Input error:", err)
            continue
        }
        fmt.Println(r)
    }
    if err := snr.Err(); err != nil {
        if err != io.EOF {
            fmt.Fprintln(os.Stderr, err)
        }
    }
}

Now we have an easy way to test. Let's make sure that we can handle all reasonable values. Here are some values that you don't handle.

0; 0.; -0; 1; -1; -1.1; 1.; and so on.

You should always expect the worst from user input and handle errors gracefully. For example,'

func ParseDecimal(s string) (r Rational, err error) {
    sign := int64(1)
    if strings.HasPrefix(s, "-") {
        sign = -1
    }
    p := strings.IndexByte(s, '.')
    if p < 0 {
        p = len(s)
    }
    if i := s[:p]; len(i) > 0 {
        if i != "+" && i != "-" {
            r.i, err = strconv.ParseInt(i, 10, 64)
            if err != nil {
                return Rational{}, err
            }
        }
    }
    if p >= len(s) {
        p = len(s) - 1
    }
    if f := s[p+1:]; len(f) > 0 {
        n, err := strconv.ParseUint(f, 10, 64)
        if err != nil {
            return Rational{}, err
        }
        d := math.Pow10(len(f))
        if math.Log2(d) > 63 {
            err = fmt.Errorf(
                "ParseDecimal: parsing %q: value out of range", f,
            )
            return Rational{}, err
        }
        r.n = int64(n)
        r.d = int64(d)
        if g := gcd(r.n, r.d); g != 0 {
            r.n /= g
            r.d /= g
        }
        r.n *= sign
    }
    return r, nil
}

Even if you write impeccable code, you will still be judged on the quality of the external packages that you use. The packages from github.com/retep-mathwizard/ are of very low quality.

Here's my complete program. It's a first draft.

package main

import (
    "bufio"
    "fmt"
    "io"
    "math"
    "os"
    "strconv"
    "strings"
)

// A rational number r is expressed as the fraction p/q of two integers:
// r = p/q = (d*i+n)/d.
type Rational struct {
    i int64 // integer
    n int64 // fraction numerator
    d int64 // fraction denominator
}

func (r Rational) String() string {
    var s string
    if r.i != 0 {
        s += strconv.FormatInt(r.i, 10)
    }
    if r.n != 0 {
        if r.i != 0 {
            s += " + "
        }
        if r.d < 0 {
            r.n *= -1
            r.d *= -1
        }
        s += strconv.FormatInt(r.n, 10) + "/" + strconv.FormatInt(r.d, 10)
    }
    if len(s) == 0 {
        s += "0"
    }
    return s
}

func gcd(x, y int64) int64 {
    for y != 0 {
        x, y = y, x%y
    }
    return x
}

func ParseDecimal(s string) (r Rational, err error) {
    sign := int64(1)
    if strings.HasPrefix(s, "-") {
        sign = -1
    }
    p := strings.IndexByte(s, '.')
    if p < 0 {
        p = len(s)
    }
    if i := s[:p]; len(i) > 0 {
        if i != "+" && i != "-" {
            r.i, err = strconv.ParseInt(i, 10, 64)
            if err != nil {
                return Rational{}, err
            }
        }
    }
    if p >= len(s) {
        p = len(s) - 1
    }
    if f := s[p+1:]; len(f) > 0 {
        n, err := strconv.ParseUint(f, 10, 64)
        if err != nil {
            return Rational{}, err
        }
        d := math.Pow10(len(f))
        if math.Log2(d) > 63 {
            err = fmt.Errorf(
                "ParseDecimal: parsing %q: value out of range", f,
            )
            return Rational{}, err
        }
        r.n = int64(n)
        r.d = int64(d)
        if g := gcd(r.n, r.d); g != 0 {
            r.n /= g
            r.d /= g
        }
        r.n *= sign
    }
    return r, nil
}

func main() {
    snr := bufio.NewScanner(os.Stdin)
    enter := "Enter a decimal number:"
    for fmt.Println(enter); snr.Scan(); fmt.Println(enter) {
        d := snr.Text()
        if len(d) == 0 {
            break
        }
        r, err := ParseDecimal(d)
        if err != nil {
            fmt.Fprintln(os.Stderr, "Input error:", err)
            continue
        }
        fmt.Println(r)
    }
    if err := snr.Err(); err != nil {
        if err != io.EOF {
            fmt.Fprintln(os.Stderr, err)
        }
    }
}


Find the number of K-Complementary pairs in an array Below is the program to find k-complementary pairs: K = A[i] + A[j];.

package org.test;

import java.util.HashMap;
import java.util.Map;

public class ComplementaryPairs {

    public int noOfComplementaryPairs(int arr[],int k){
        int result = 0;
        for(int i=0;i<=arr.length;i++){
            for(int j=i;j<arr.length-1;j++){
                if(arr[i] + arr[j+1] == k){
                    result++;
                }
            }
        }
        return result * 2;
    }
    public static void main(String[] args) {
        int[] intArray = new int[] {4,5,6,3,1,8,-7,-6};
        int k = 1;
        System.out.println("No of k complementary pairs : "+new ComplementaryPairs().noOfComplementaryPairs(intArray, k));
    }

}

What is the time complexity of the above program? Is there a better way to optimize it further?

Current algorithm

The time complexity of the current solution is O(N²): for a given array of length N, it needs to loop through its elements from 0 to its length, and then for each of those, loop again through the elements after it. Note that it is still O(N²), even with the logic of avoiding duplicate indexes (if the pair (0,1) was a solution, it always follows that (1,0) is one, so we don't need to test it).

About the code itself, the first for loop is deceiving:

for(int i=0;i<=arr.length;i++){
    for(int j=i;j<arr.length-1;j++){
        if(arr[i] + arr[j+1] == k){

It makes the reader think that i will go up to arr.length. What's worse, it makes the reader think the algorithm fails because we fetch arr[i] later on, which cannot work for arr.length (and would fail with an ArrayIndexOutOfBoundsException). In reality, it won't, because there is a second inner loop of j going from i to arr.length-1, so when i is greater than arr.length-1, nothing will happen anyway.

I suggest making that clear in the bounds used. Consider having:

for(int i=0;i<arr.length-1;i++){
    for(int j=i;j<arr.length-1;j++){
        if(arr[i] + arr[j+1] == k){

In the same way, instead of reasoning with j+1, you could have j loop through its natural bounds and reason with j:

for (int i = 0; i < arr.length - 1; i++) {
    for (int j = i + 1; j < arr.length; j++) {
        if (arr[i] + arr[j] == k) {

With those bounds, it's clear to everyone reading the code that no out of bounds exception can happen. Note also how I added white spaces around the different operators and calculations: it adds to clarity.

Also, maybe I wouldn't make this method an instance method, rather a static one that is directly invoked in main.

Better performance

It would be possible to solve this problem in O(N) time complexity, instead of O(N²), at the expense of also being O(N) in terms of memory.

The idea is that we want to avoid looping through the array again and again. Here's another approach:

Go through the array once, and store in a Map the difference of the wanted sum and the current element mapped to how many times it occured. Effectively, this map remembers how much we're missing for an element at a given index so that the sum can be reached.
Go through the array a second time, and check whether the map contains this element. If it does, then it means that our map contains an element e for which e = sum - arr[i], so it means that we've found a matching pair. And the number of matching pair we found, is the number of times this element appears in the array, which is the value of the map.

That's how it would look like with the example in the question where the sum to look for is 1:

          arr =  4   5   6   3   1   8  -7  -6
1st pass, map = -3  -4  -5  -2   0  -7   8   7  <-- how much we need to add to find the sum
    2nd pass  =  x   x   x   x   x   o   o   x  <-- the initial array only contains the element marked o

This shows that there are 2 such pairs, and that is the answer. As an example of putting this into code, and using Java 8, you could have:

public int noOfComplementaryPairs(int arr[], int k) {
    Map<Integer, Integer> map = new HashMap<Integer, Integer>();
    for (int i = 0; i < arr.length; i++) {
        map.merge(k - arr[i], 1, Integer::sum);
    }
    return Arrays.stream(arr).map(element -> map.getOrDefault(element, 0)).sum();
}


How to make the end of loop more readable in Ruby There are some blocks in my code.

To make it easier to read, I often append comment after the end

such as

  end # end upto
  end # fileopen

Because sometimes the indentation still not easy for read.

Is there any better practice?

And my indentation is 2 spaces, is it OK for most Rubier ?

require "faker"
real_sn = 2124100000
File.open("lib/tasks/tw_books.txt", "r").each do |line|
  # author = Faker::Lorem.words(2..5).join(' ').gsub('-','').gsub('\s+','\s')

  1.upto(1000).each do |x|

    location = Faker::Lorem.words(5)
    book_name, author, publisher, isbn, comment = line.strip.split("|||")
    ap(line)
    isbn = isbn[/\d+/]
    real_sn+=1
    bk = Book.new(:sn => real_sn,:name => book_name, :isbn=>isbn,
                  :price =>Random.rand(200..5000), :location=>location, :category=>["商業","歷史","體育","政治"].sample,
                  :author => author, :sale_type => [:fix_priced, :normal, :promotion].sample, :publisher => publisher,
                  :release_date => rand(10.years).ago, :comment => comment
                  )
    if bk.save()
      if (real_sn%100)==0
        puts book_name
      end
      Sunspot.commit
    else
      puts real_sn
      puts bk.errors.full_messages
    end
  end # end upto
end # fileopen

Compare your code to how I'd write it:

require "faker"

real_sn = 2124100000

File.open("lib/tasks/tw_books.txt", "r").each do |line|
# author = Faker::Lorem.words(2..5).join(' ').gsub('-','').gsub('\s+','\s')

  1.upto(1000).each do |x|

    location = Faker::Lorem.words(5)
    book_name, author, publisher, isbn, comment = line.strip.split("|||")

    ap(line)

    isbn = isbn[/\d+/]
    real_sn += 1

    bk = Book.new(
      :author => author,
      :category =>["商業","歷史","體育","政治"].sample,
      :comment => comment,
      :isbn => isbn,
      :location =>location,
      :name => book_name,
      :price => Random.rand(200..5000),
      :publisher => publisher,
      :release_date => rand(10.years).ago,
      :sale_type => [
        :fix_priced,
        :normal,
        :promotion
      ].sample,
      :sn => real_sn,
    )

    if bk.save()

      puts book_name if ((real_sn % 100) == 0)
      Sunspot.commit

    else

      puts real_sn
      puts bk.errors.full_messages

    end
  end
end

Part of writing code is making it readable and maintainable. That means use indentation, vertical alignment, whitespace between operators, vertical whitespace to make changes in logic more obvious, etc.

I sort hash keys alphabetically, such as the parameters for Book.new, especially when there's a lot of them. This makes it a lot easier to see if something is duplicated or missing. Since it's a hash it doesn't matter what order they're in as far as Ruby is concerned; Again this is for maintenance later on.

The editor you use can help you immensely with this. I use gvim and Sublime Text, both of which allow me to easily reformat/reindent code I'm working on, and I take advantage of that often. It's a good first step when you have code from a "foreign source" that makes your eyes bug out. Reindent it, fix long, awkward sections, like your list of hash entries for Book.new, and the code will become more understandable.

Also, your editor needs to have the ability to jump between matching delimiters like (), [], {} and do/end. gvim can do that plus jump through if/else/end plus rescue blocks. That ability to navigate REALLY helps keep your code flow clear.


Basic distance conversion program This is a simple program to convert miles, yards, feet, and inches appropriately. Everything intended for works but I feel my program is very redundant. How can I make it more efficient and less lines of code? I tried % but failed to find way to increment next values.

#include <iostream>
using namespace std;

//NO GLOBAL VARIABLES

int main()
{
    int m, y, f, i;
    int a, b, c;
    cout << "Enter: ";
    cin >> m >> y >> f >> i;
    cout << "You enter: " << m << "miles " << y << "yards "<< f << "feet " << i    << "inches " << endl;
    cout << "Convert the distance" << endl;

    // 2miles 1761yards 4feet 27inches
    // = 3miles 2yards 3feet 3inches

    //ensure mile is zero or positive
    if(m<0)
    {
        return 0;
    }

    //yard to mile conversion
    for(a=0; a<100; a++)
    {
        if(y<0)
        {
             return 0;
        }
        if(y>1760)
        {
            m++;
            y -= 1760;
        }
        if(y==1760)
        {
            y = 0;
            m++;
        }
        if(y<1760)
        {
            y = y;
        }
    }

    //feet to yard conversion
    for(b=0; b<100; b++)
    {
        if(f>3)
        {
            y++;
            f -= 3;
        }
        if(f==3)
        {
            y++;
            f= 0;
        }
        if(f<3)
        {
            f = f;
        }
        if(f<0)
        {
            return 0;
        }
    }

    //inches to feet conversion
    for(c=0; c<100; c++)
    {
        if(i>12)
        {
            f++;
            i -=  12;
        }
        if(i==12)
        {
            f++;
            i = 0;
        }
        if(i<0)
        {
            return 0;
        }
    }

    cout << "After conversion: " << m << "miles " << y << "yards "<< f << "feet  "   << i << "inches" << endl;
    return 0;
}

As you guessed, you can make your code much more performant/robust by using % instead of for loops to convert the distances.

An example with just yards you would do something like:

m = 0;
y = 1761;
m += (y / 1760); // add the number of whole miles
y = (y % 1760);  // set to the number of remaining yards

At the end of that you'll see that:

m == 1
y == 1


Optimizing multiplication of two polynomials in GF(2^32) I have the following method for multiplying two polynomials in GF(232):

#include <iostream>
#include <cassert>
#include <ctime>
#include <random>
#include <wmmintrin.h>

typedef unsigned short uint16_t;
typedef unsigned int uint32_t;

unsigned get_high16(unsigned x)
{
    return (x >> 16) & 0xffff;
}

unsigned get_low16(unsigned x)
{
    return x & 0xffff;
}

uint32_t mul_reduce_32(uint32_t x, uint32_t y)
{
    const __m128i a = _mm_set_epi32(0, 0, 0, x);
    const __m128i b = _mm_set_epi32(0, 0, 0, y);
    const __m128i c =_mm_clmulepi64_si128(a, b, 0);

    uint16_t D = _mm_extract_epi16(c, 2);

    D ^= _mm_extract_epi32(c, 1) >> 31;
    D ^= _mm_extract_epi32(c, 1) >> 30;
    D ^= _mm_extract_epi32(c, 1) >> 29;
    D ^= _mm_extract_epi32(c, 1) >> 27;
    D ^= _mm_extract_epi32(c, 1) >> 25;

    const uint16_t X3 = _mm_extract_epi16(c, 3);
    const __m128i X3D = _mm_set_epi16(0, 0, 0, 0, 0, 0, X3, D);

    const uint32_t E = _mm_extract_epi32(X3D, 0) << 7;
    const uint32_t F = _mm_extract_epi32(X3D, 0) << 5;
    const uint32_t G = _mm_extract_epi32(X3D, 0) << 3;
    const uint32_t I = _mm_extract_epi32(X3D, 0) << 2;
    const uint32_t J = _mm_extract_epi32(X3D, 0) << 1;

    const uint16_t H1 = X3 ^ get_high16(E) ^ get_high16(F) ^ get_high16(G) ^ get_high16(I) ^ get_high16(J);
    const uint16_t H0 = D ^ get_low16(E) ^ get_low16(F) ^ get_low16(G) ^ get_low16(I) ^ get_low16(J);

    const uint16_t R1 = _mm_extract_epi16(c, 1) ^ H1;
    const uint16_t R0 = _mm_extract_epi16(c, 0) ^ H0;

    return _mm_extract_epi32(_mm_set_epi16(0, 0, 0, 0, 0, 0, R1, R0), 0);
}

The following is a small test program and benchmark for the function.

int main()
{
    assert(mul_reduce_32(1, 1) == 1);
    assert(mul_reduce_32(23523, 34651) == 744709325);
    assert(mul_reduce_32(34652346, 5534651) == 1329462203);

    // A little benchmark
    std::mt19937 rng(std::time(0));
    std::uniform_int_distribution<uint32_t> uint_dist10(0, std::numeric_limits<uint32_t>::max());

    const int N = 100000000;
    std::vector<uint32_t> vec1(N, 0);

    std::cout << "mul_reduce_32\n";
    clock_t startTime = clock();

    for(int i = 0; i < N; ++i)
    {
        vec1[i] = mul_reduce_32(rng(), rng());
    }

    std::cout << double( clock() - startTime ) / (double)CLOCKS_PER_SEC << " seconds.\n";
}

I'm interested in getting more out of mul_reduce_32 in terms of performance. Here are my main concerns:

I'm setting a and b using _mm_set_epi32. Is this even a good idea? I suppose I could have used _mm_load_si128 or _mm_loadu_si128, but the problem seems to be that it doesn't touch the 98 higher bits, and makes those high bits garbage.
Currently, I am using many temporary variables. I could try to help the compiler by removing many of them, but this way the code should be more readable and easier for someone to modify.
I'm using variables of different size -- is this a good idea? I might remember some old wisdom that says I should always just stick to the "most native" unsigned type. Concretely, should I replace say each uint16_t with uint32_t (or even a 64-bit type)?
Should I be mixing __m128i types with unsigned types to begin with? As I understand it, the __m128i types are guaranteed to be mapped to a dedicated register, whereas this is not the case for a uint32_t.

Can you suggest additional optimizations? I'm knowledgeable in C++, but a total newbie with SSE intrinsics.

For the time output in main():

Use std::clock_t as opposed to clock_t in C++.
You could have another variable, endTime, for the ending time.
The computed time could also have its own variable, elapsedTime, to be printed.
Only the clock time needs to be cast to a double, not the macro as well.  You should also use static_cast<>() as this is C++.

Example with changes:

std::clock_t startTime = std::clock();

// ...

std::clock_t endTime = std::clock();

double elapsedTime = static_cast<double>(endTime - startTime) / CLOCKS_PER_SEC;

std::cout << elapsedTime;

But, since you're using C++11, consider utilizing the <chrono> library instead. Unlike <ctime>, it is more C++-oriented but still contains some <ctime> aspects.

Side-notes:

For std::time, 0 can be replaced with nullptr in C++11.
Consider renaming N to something more descriptive.  Using a constant in place of a magic number is nice, but you can increase readability by using a better name.
Since only your test program uses those first four libraries, they should just be in that file.
You don't need those typedefs in your first file; they are already defined in <cstdint>.

Specific questions:

  Currently, I am using many temporary variables. I could try to help the compiler by removing many of them, but this way the code should be more readable and easier for someone to modify.

I would rely on the compiler to handle this, as it should be able to optimize them out.  But you're right; readability and maintainability are also important.  They can always be adjusted accordingly.

You can also maintain these two things by:

avoiding single-character variable names
keeping variables as close in scope as possible

You seem to do well with the second point, but not so much the first.  I've already addressed this in the side-notes, but this is a problem throughout the first file.  By using more descriptive names, it'll be much easier for others to understand your code and make any needed modifications.


Haskell program with basic CLI parsing, file reading, and decoding file contents into specified types I recently completed the following exercise, but wanted some feedback on how idiomatic etc. my haskell program is. This is from a course which introduces us to file IO before we cover the theory of monads, so my monad-knowledge is still in its infancy. Any feedback much appreciated.
Here's the task:

In StudentTeacher.hs, observe that we have an old version of our Person type with the Student
and Teacher constructor. Fill in studentTeacherBasic so that it will use arguments to determine
the right type of person. If the arguments contain -s or --student, read two lines, for name and age,
and make a student. If there is -t or --teacher, read two lines for the name and department. If
there are no arguments, print:
Please specify the type of person!
Print the resulting person otherwise.
Fill in studentTeacherAdvanced, which should have the same result as your basic version, but
read its input differently. Take a file name as the first argument. If it ends in .student, read lines from
the file assuming they contain a student's information. If it’s .teacher, assume a teacher. Print the
same failure message in any other case.

Here's my answer:
module StudentTeacher where

import Data.List.Split (splitOn)
import Text.Read (readMaybe)
import System.IO (openFile, withFile, IOMode(..), hGetLine, hGetContents, Handle, stdin)
import System.Environment (getArgs)
import Data.Maybe (isNothing)

data Person =
  Student String Int |
  Teacher String String
  deriving (Show)

type PersonDecoder = String -> Maybe Person
type CLIParser = [String] -> Maybe (PersonDecoder, (IO Handle))

studentTeacher :: CLIParser -> IO ()
studentTeacher cliparser = do
  arguments <- getArgs
  maybePerson <- case (cliparser arguments) of
    Nothing -> return Nothing
    Just (decoder, getHandle) -> do
      handle <- getHandle
      decodePersonFromHandle decoder handle
  case maybePerson of
    Nothing -> putStrLn "Please specify the type of person"
    Just person -> print person

studentTeacherBasic :: IO ()
studentTeacherBasic = studentTeacher parseArgsFlag

decodePersonFromHandle :: PersonDecoder -> Handle -> IO (Maybe Person)
decodePersonFromHandle decoder handle = do
  line1 <- hGetLine handle
  line2 <- hGetLine handle
  return $ decoder $ line1 ++ "\n" ++ line2

parseArgsFlag :: CLIParser
parseArgsFlag [] = Nothing
parseArgsFlag ("-t":_) = Just (decodeTeacher, wrapStdIn)
parseArgsFlag ("-s":_) = Just (decodeStudent, wrapStdIn)
parseArgsFlag ("--teacher":_) = parseArgsFlag ["-t"]
parseArgsFlag ("--student":_) = parseArgsFlag ["-s"]

wrapStdIn :: IO Handle
wrapStdIn = do
  return stdin

decodeTeacher :: PersonDecoder
decodeTeacher encoded =
  case lines of [] -> Nothing
                [_] -> Nothing
                (name:dept:_) -> Just (Teacher name dept)
  where lines = splitOn "\n" encoded

decodeStudent :: PersonDecoder
decodeStudent encoded =
  case lines of [] -> Nothing
                [_] -> Nothing
                (name:age:_) -> decodeStudentLines name age
  where lines = splitOn "\n" encoded

decodeStudentLines :: String -> String -> Maybe Person
decodeStudentLines name age =
  case decodedAge of Nothing -> Nothing
                     (Just ageInt) -> Just (Student name ageInt)
  where decodedAge = readMaybe age :: Maybe Int

studentTeacherAdvanced :: IO ()
studentTeacherAdvanced = studentTeacher parseArgsSuffix

parseArgsSuffix :: CLIParser
parseArgsSuffix [] = Nothing
parseArgsSuffix (fname:_) =
  if hasSuffix fname "student" then Just (decodeStudent, openFile fname ReadMode)
  else if hasSuffix fname "teacher" then Just (decodeTeacher, openFile fname ReadMode)
  else Nothing

hasSuffix :: String -> String -> Bool
hasSuffix filename suffix = if length components == 0 || length components == 1
  then False
  else last components == suffix
  where
    components = splitOn "." filename

First things first: great work on adding a type signature on every function. Now, let's see how we can improve the code.

Use interesting case first, others later
When we pattern match in a binding or in a case expression, it's often easier to start with the interesting case first and then handle the Error cases:
decodeTeacher :: PersonDecoder
decodeTeacher encoded =
  case lines of (name:dept:_) -> Just (Teacher name dept)
                _             -> Nothing
  where lines = splitOn "\n" encoded

decodeStudent :: PersonDecoder
decodeStudent encoded =
  case lines of (name:age:_) -> decodeStudentLines name age
                _            -> Nothing
  where lines = splitOn "\n" encoded

Check the standard library for already existing functionality
We stay at those functions. The name lines is very unfortunate, as there is already a lines in the Prelude. It's a function with the following signature:
lines :: String -> [String]

With that function, we can get rid of splitOn:
decodeTeacher :: PersonDecoder
decodeTeacher encoded =
  case lines encoded of
    (name:dept:_) -> Just (Teacher name dept)
    _             -> Nothing

decodeStudent :: PersonDecoder
decodeStudent encoded =
  case lines encoded of
    (name:age:_) -> decodeStudentLines name age
    _            -> Nothing

Prefer simpler logic where possible
Next, we have a look at parseArgsSuffix:
parseArgsSuffix :: CLIParser
parseArgsSuffix [] = Nothing
parseArgsSuffix (fname:_) =
  if hasSuffix fname "student" then Just (decodeStudent, openFile fname ReadMode)
  else if hasSuffix fname "teacher" then Just (decodeTeacher, openFile fname ReadMode)
  else Nothing

That's a rather long if expression. A guard may be easier to read and handle:
parseArgsSuffix :: CLIParser
parseArgsSuffix [] = Nothing
parseArgsSuffix (fname:_)
  | fname `hasSuffix` "student" = Just (decodeStudent, openFile fname ReadMode)
  | fname `hasSuffix` "teacher" = Just (decodeTeacher, openFile fname ReadMode)
  | otherwise                   = Nothing

Note that we should keep the uninteresting case here first, as it improves readability. However, we're repeating ourselves. Something along
parseArgsSuffix :: CLIParser
parseArgsSuffix [] = Nothing
parseArgsSuffix (fname:_)
  | fname `hasSuffix` "student" = decodeVia decodeStudent
  | fname `hasSuffix` "teacher" = decodeVia decodeTeacher
  | otherwise                   = Nothing
  where
    decodeVia f = Just (f, openFile fname ReadMode)

might reduce the duplication. Now, let's have a look at hasSuffix:
hasSuffix :: String -> String -> Bool
hasSuffix filename suffix = if length components == 0 || length components == 1
  then False
  else last components == suffix
  where
    components = splitOn "." filename

Again, splitOn is an overkill here. We only want to know if our filename ends with the given extension. The function isSuffixOf from Data.List handles that fine:
hasSuffix :: String -> String -> Bool
hasSuffix filename suffix = ('.' : suffix) `isSuffixOf` filename

We could probably even remove hasSuffix completely that way.

Make sure that functions are total
parseArgsFlag is partial. If we use parseArgsFlags on ["whoops!", "-t"], then none of the patterns match:
parseArgsFlag :: CLIParser
parseArgsFlag [] = Nothing
parseArgsFlag ("-t":_) = Just (decodeTeacher, wrapStdIn)
parseArgsFlag ("-s":_) = Just (decodeStudent, wrapStdIn)
parseArgsFlag ("--teacher":_) = parseArgsFlag ["-t"]
parseArgsFlag ("--student":_) = parseArgsFlag ["-s"]

If that is intended, then everything is fine. However, if you want to handle those cases, then add a match all pattern:
parseArgsFlag _               = Nothing

I would prefer something like this, by the way:
parseArgsFlag :: CLIParser
parseArgsFlag args
  | any (`elem` args) ["-t", "--teacher"] = Just (decodeTeacher, wrapStdIn)
  | any (`elem` args) ["-s", "--student"] = Just (decodeStudent, wrapStdIn)
  | otherwise                             = Nothing


Queries using multiple tables and ANDs: should that be avoided? This is a part of a project I am working on. I need advice and criticism regarding the data base design and general approach to querying the data. I am not at all experienced with relational data bases (as you should notice), and even though I have tried to invest some time in reading up I am currently forced to just move along as fast as possible.

Currently using SQLite3, and most probably sticking with it.

The data base schema goes along the lines of:

-- gene ids and protein names
CREATE TABLE ensemblid (
    geneid TEXT NOT NULL PRIMARY KEY,
    description TEXT, -- not used so far
    name TEXT NOT NULL
);

-- tss id and site and study names
CREATE TABLE tissue_source_site (
    tss TEXT PRIMARY KEY,
    source_site TEXT NOT NULL,
    study_name TEXT NOT NULL,
    bcr TEXT NOT NULL -- not used so far
);

-- sample type id and its human-readable definition
CREATE TABLE sample_type (
    sample_type TEXT PRIMARY KEY,
    definition TEXT NOT NULL,
    short_letter_code TEXT NOT NULL -- not used so far
);

-- analyte id and its human-readable definition
CREATE TABLE portion_analyte (
    analyte TEXT PRIMARY KEY,
    definition TEXT NOT NULL
);

-- tables containing indices into two LARGE data matrices of REALs
-- that are currently **not** in the relational database
CREATE TABLE mrna_rows (
    rowindex INTEGER PRIMARY KEY,
    geneid TEXT NOT NULL
);
CREATE TABLE ampl_rows (
    rowindex INTEGER PRIMARY KEY,
    geneid TEXT NOT NULL
);
CREATE TABLE ampl_cols (
    rowindex INTEGER PRIMARY KEY,
    tss TEXT NOT NULL,
    patient TEXT NOT NULL
);
CREATE TABLE mrna_cols (
    rowindex INTEGER PRIMARY KEY,
    tss TEXT NOT NULL,
    patient TEXT NOT NULL,
    sample_type TEXT NOT NULL,
    vial TEXT NOT NULL,
    portion TEXT NOT NULL,
    analyte TEXT NOT NULL
);

First off, I do realize that I have not put any effort at that moment to ensure data integrity (apart from the obvious PRIMARY KEY and NOT NULL). There are a few foreign keys lurking in there: for example, the tss column in both ampl_cols and mrna_cols is the same tss that is the primary key of the tissue_source_site table. Another issue is that the patient column, in every table it is used, is a unique identifier consistent across all tables. The data, as I have it, contains no further information about "patients", so I have not felt the need to make it a unique column in a table of its own.

After some trial and error, and much unsuccessful mucking around with the different JOINs, I eventually got to the schema above, and the following "style" of querying it:

  Get the names of sites and studies, along with number of patients present in both data sets.

SELECT tissue_source_site.source_site, tissue_source_site.study_name, COUNT(1)
FROM tissue_source_site, ampl_cols, mrna_cols
WHERE ampl_cols.patient == mrna_cols.patient
    AND tissue_source_site.tss == ampl_cols.tss
    AND tissue_source_site.tss == mrna_cols.tss
GROUP BY tissue_source_site.tss;

Which gives me the expected results:

UCSF|Breast invasive carcinoma|8
Walter Reed|Breast invasive carcinoma|65
-- and so on

Is this a valid approach? Is there a point at which I will hit a wall, concerning the length of the queries (endless ANDs) or performance-wise? How about this table.column way of doing everything: I did not really see this used much the tutorials I did (in a hurry). What are the drawbacks of doing it like that? Is it wiser to figure out how JOIN works? Or WITH table AS? Or something else I am totally unaware of?

Is this a valid approach?

FROM tissue_source_site, ampl_cols, mrna_cols

What you are doing here is to get a product of those tables, meaning that if the RDBMS is not wise it may attempt to create an intermediate structure
with size = product of the sizes of each table.

Gladly I just got a query plan execution for SQL Server and they can infer the same plan for either your where clauses, and the join operator, meaning that they are just as efficient (but I would prefer use a join anyway).

  Is there a point at which I will hit a wall, concerning the length of
  the queries (endless ANDs) or performance-wise?

You shouldn't be too concerned about the length of your queries.
Querying, after all, is the way to get your data. So you should let your query express what you want to retrieve.

Performance wise you should get a query plan as I did for SQLServer so you can choose between a join and a where. Doing many joins can also hinder performance but most of times this isn't an issue because they are done over the primary key of the tables which most of the times are indexed (you can see indexing as a way to arrange data so it's easier to retrieve it by that index)

  I did not really see this used much the tutorials I did (in a hurry).
  What are the drawbacks of doing it like that? Is it wiser to figure
  out how JOIN works?

There are many types of joins and someday you will probably be wanting the functionality of one of them, having the wisdom of picking the right tool for the job is always nice.

  Or WITH table AS? Or something else I am totally unaware of?

Your query is still pretty simple and doesn't require any "black magic". try to keep things simple and give yourself time to understand and get used to the sql querying concept.


RAII-style lockable objects I am working on a Windows project using C++03 (C++11 is not an option), and without using Boost. I have several resources that are accessed by multiple threads at various times - a queue, for example, with data added by one thread and removed by another.  Individual methods of the involved classes generally perform locking internally.  However in some cases I need to lock the entire resource to perform a sequence of actions without another thread getting in the way.

Without the phrases "use Boost" and "use C++11", does the posted code for RAII-style locking of a resource look correct and safe, or is there something I've missed?

To this end I have written a simple RAII-style locking class, so that the following can be performed:

void somefunction(void) {
    GetLock<LockableClass> lock(queue);
    ...
}

Here the only requirement of LockableClass is that is has Lock() and Unlock() methods.  Specialisations of the class are provided for CRITICAL_SECTION, etc.

However, in the case of there being several queues, only some of which need to be locked at one time, I would like the following to be possible:

void somefunction(void) {
    list<GetLock<LockableClass> > locks;
    for (vector<LockableClass>::iterator its = queues.begin(); queues.end() != its; ++its)
        if (some_condition)
            locks.push_back(GetLock<LockableClass>(*its));
    ...
}

In order to achieve this I've added a copy-constructor to GetLock.  This creates the requirement that any lockable object has to have some kind of reference counting.  It also limits me to using a std::list as the container for the locks - at least in my version of Visual Studio (2008 - yes, yes, I know).  std::vector appears to default-construct, then assign.

The following is the code for GetLock, along with a specialisation for CRITICAL_SECTION:

template <typename TLOCKABLE>
class GetLock {
private:
    TLOCKABLE& lock;
    GetLock();
public:
    GetLock(TLOCKABLE& plock) : lock(plock) {
        lock.Lock();
    }
    GetLock(TLOCKABLE* plock) : lock(*plock) {
        lock.Lock();
    }
    GetLock(const GetLock<TLOCKABLE>& plock) : lock(plock.lock) {
        lock.Lock();
    }
    ~GetLock(void) {
        lock.Unlock();
    }
};

template <>
class GetLock<CRITICAL_SECTION> {
private:
    CRITICAL_SECTION& lock;
    GetLock();
public:
    GetLock(CRITICAL_SECTION& plock) : lock(plock) {
        EnterCriticalSection(&lock);
    }
    GetLock(const GetLock<CRITICAL_SECTION>& plock) : lock(plock.lock) {
        EnterCriticalSection(&lock);
    }
    ~GetLock(void) {
        LeaveCriticalSection(&lock);
    }
};

// There is also a specialisation for HANDLE, to cope with mutexes...
// This presents its own issues as HANDLE is used everywhere in Windows,
// but differentiating between a Mutex and something else is a question
// for StackOverflow.

Does the code look safe? Is there something I've missed? Is making the assumption of using recursively-lockable objects a foolhardy thing to do? What could I do better?

first I suggest you use pointers to keep the mutex reference; this makes it possible to have a non-owning state when the pointer is 0 (the state when default initialized)

also you created a copy constructor and a destructor, so you still need a copy assignment to properly follow rule of 3 (or at least disable it)

GetLock<TLOCKABLE>& operator=(const GetLock<TLOCKABLE>& plock) {
    if(plock.lock != lock){
        GetLock<TLOCKABLE> tmp(plock);
        swap(tmp);

    }
    return *this;
}

however this keeps the assumption that the lock is recursively-lockable, but as an optimization it won't reacquire and release when the locks are the same


Entering two integers and returning a decimal fraction After a couple of small programs in Python we are now asked to do some basic stuff in Java. I don't really know where to start to improve this and whether it's even necessary.

This is an exercise, so the resulting message for when the denominator is zero should not be taken too seriously.

import java.io.*;

/**
 * Class: GetRationalNumber
 * Takes two integers (numerator and denominator) and
 * return them as a decimal fraction.
 */
class GetRationalNumber {
    public static void main(String[] args) throws IOException {
        int p, q;

        BufferedReader userInput = new BufferedReader(
            new InputStreamReader(System.in)
        );

        System.out.println("Numerator: ");
        p = Integer.parseInt(userInput.readLine());

        System.out.println("Denominator: ");
        q = Integer.parseInt(userInput.readLine());

        if(q == 0) {
            System.out.println(
                "Divison by zero can cause serious trouble to the universe!"
            );
            System.exit(0);
        }

        System.out.println("Result:\n" + (double) p / (double) q);
    }
}

import java.io.*;

It's often a bad practice to import everything in a package. this will clutter your IDE's Ctrl + Space functionality, and makes it harder for other people to see exactly which things you are using from a package.

/**
 * Class: GetRationalNumber

I like the Javadoc, but it's totally overkill to specify the class name in the javadoc. The class name is already displayed in the javadoc functionality itself, you don't need to duplicate that when you write javadoc.

int p, q;

Make it a habit ASAP to avoid one-letter variable names like these. numerator and denominator would be much better.

Possible improvement: Catch NumberFormatException (or use a regex in advance to make sure that it is a number, but that might a bit over your head right now).

System.out.println(
    "Divison by zero can cause serious trouble to the universe!"
);

I would write this on one line:

System.out.println("Divison by zero can cause serious trouble to the universe!");

System.exit(0);

The zero here indicates the exit status, which can be used by scripts and other stuff. If a program terminates normally, that is exit status zero. Is a possible division by zero really a normal program termination? In my opinion it is not. I would do System.exit(1); or System.exit(42); or something.

Overall, there's nothing object-oriented about what you have done, but I guess you haven't gotten that far yet, huh? :) Your code is quite fine for being beginner code. Now take it to the next level :)


Simple Stack Implementation in C This is my implementation of the stack data type in C language. It is static meaning it does not grow in size. Any suggestions or feedback would be much appreciated.

#include <stdio.h>
#define MAXSTACKLENGTH 6
#define true 1
#define false 0

struct stk{
    char content[MAXSTACKLENGTH];
    int top;
};

typedef struct stk stack;

int size(stack *s)
{
    return s->top;
}

int isEmpty(stack *s)
{
    if (size(s) <= -1)
    {
        return true;
    }
    else if (size(s) > -1)
    {
        return false;
    }
}

int isFull(stack *s) {
    if (size(s) == (MAXSTACKLENGTH-1))
    {
        return true;
    }
    else {
        return false;
    }
}

char peek(stack *s)
{
    if (isEmpty(s) == true)
    {
        printf("Can't peek: Stack is empty!\n");
        return '\0';
    }
    else {
        return s->content[s->top];
    }
}

void push(stack *s, char element)
{
    if (isFull(s) == true)
    {
        printf("Cant push: Stack is full!\n");
    }
    else {
        s->top++;
        s->content[s->top] = element;
    }
}

void pop(stack *s)
{
    if (isEmpty(s) == true) {
        printf("Cant pop: Stack is empty!\n");
    }
    else {
        s->top--;
    }
}

void input_push(stack *s)
{
    char c;
    printf("Enter a character> ");
    scanf(" %c", &c);

    push(s, c);
}

void print_stack(stack *s)
{
    if (isEmpty(s)) {
        printf("Stack is empty!\n");
    }
    else {
        int i = s->top;

        for (i; i > -1; i--)
        {
            printf("%c\n", s->content[i]);
        }

        printf("\n");
    }
}

void pause()
{
    char c;
    printf("Enter any character to continue.\n");
    scanf(" %c", &c);
}

int main()
{
    printf("SimpleStack! v1\n");

    stack mystack;
    mystack.top = -1; //Empty

    int run = true;
    int choice = 0;

    while (run)
    {
        if (isFull(&mystack))
        {
            printf("Warning -> Stack is full\n");
        }
        if (isEmpty(&mystack))
        {
            printf("Warning -> Stack is empty\n");
        }
        printf("1 -> Push\n");
        printf("2 -> Pop\n");
        printf("3 -> Peek\n");
        printf("4 -> Print Stack\n");
        printf("5 -> Exit\n");

        printf("Enter choice> ");
        scanf("%d", &choice);

        switch (choice) {
            case 1:
                input_push(&mystack);

                break;
            case 2:
                pop(&mystack);

                break;
            case 3:
                printf("%c\n", peek(&mystack));
                pause();
                break;
            case 4:
                print_stack(&mystack);
                pause();
                break;
            case 5:
                printf("Goodbye!\n");
                run = false;

                break;
            default:
                printf("Invalid choice.\n");

                break;
        }

        system("clear");
    }

    return 0;
}

It looks good. Here are a few things you can add to it or change:

You don't have to define true/false yourself. It's unfortunate that bool is not a built-in type in C, but the Standard C Library at least provides an #include file that gives  you those definitions in a portable way. So you can #include <stdbool.h> and it will define the type bool and true/false constants.
You can combine the structure declaration + typedef into the same declaration:

typedef struct stk
{
    char content[MAXSTACKLENGTH];
    int top;
} stack;

Most C programs you'll find use this more compact notation of typedef struct.
You can simplify statements like this and others:

int isEmpty(stack *s)
{
    if (size(s) <= -1)
    {
        return true;
    }
    else if (size(s) > -1)
    {
        return false;
    }
}

By simply returning the condition:

int isEmpty(stack *s)
{
    return size(s) <= -1;
}

The result of a comparison is always a boolean, so it can be returned directly if the function also returns true/false. This is another pattern that you will see used a lot. It makes code more concise.
Comparing against true/false in a conditional is not very usual:

if (isFull(s) == true)

You gave good descriptive names to your functions, so you can just test them like this:

if (isFull(s)) { ...

And to test if not full, the more usual way is to apply the ! (logical not) operator:

if (!isFull(s)) { ... // Reads "if not isFull"

size() currently return -1 for an empty stack. That's unusual. It doesn't make sense for a size to be negative. I'd expect an empty stack to have size 0. You should change that.
I suggest having a init_stack() function that sets the expected initial invariants when a new stack is declared. Better than having to remember to manually set top to -1, for example.
Since you already have a custom pause() function, you don't have to use system("pause"). The latter is non-portable and not recommended for use in real-life programs due to security risk it might incur. Best if you avoid it.
A minor nitpick, you're mixing different styles of curly brace positioning. Some are on the same line as the parent conditional, some are on their own line. This creates a break in your style, which is a little annoying. It would be nice if you kept them consistent, independent of which style you decide for.


Capitalize a sequence of lines entered via console input and print/return I'm running through the popular python practice exercises located at github:
https://github.com/zhiwehu/Python-programming-exercises/blob/master/100%2B%20Python%20challenging%20programming%20exercises.txt

I recently discovered recursive functions. So when asked the following in the above link:

Question 9
Level 2

Question£º
Write a program that accepts sequence of lines as input and prints the lines after making all characters in the sentence capitalized.
Suppose the following input is supplied to the program:
Hello world
Practice makes perfect
Then, the output should be:
HELLO WORLD
PRACTICE MAKES PERFECT

My answer is:

def capitalize_line(string=""):
    """Capitalize and add a newline to the input str(if present)"""
    inp = input("enter: ").upper()
    string = "\n".join([string, inp]) if string and inp else "".join([string, inp])
    if not inp:
        print(string)
        return string
    return capitalize_line(string=string)

E.g. Two inputs of 'hello' and 'world' would return 'HELLO\nWORLD'.

Is this a suitable solution? Pythonic?

The link answer was:

Solution:
lines = []
while True:
    s = raw_input()
    if s:
        lines.append(s.upper())
    else:
        break;

for sentence in lines:
    print sentence

Is this pythonic ?

The answer to this question is often found by doing import this:

  Beautiful is better than ugly.
  Explicit is better than implicit.
  Simple is better than complex.
  Complex is better than complicated.
  Flat is better than nested.
  Sparse is better than dense.
  Readability counts.
  Special cases aren't special enough to break the rules.
  Although practicality beats purity.
  Errors should never pass silently.
  Unless explicitly silenced.
  In the face of ambiguity, refuse the temptation to guess.
  There should be one-- and preferably only one --obvious way to do it.
  Although that way may not be obvious at first unless you're Dutch.
  Now is better than never.
  Although never is often better than right now.
  If the implementation is hard to explain, it's a bad idea.
  If the implementation is easy to explain, it may be a good idea.
  Namespaces are one honking great idea -- let's do more of those!

Why is this line not pythonic ? Because it is not readable enough.

  Readability counts.

string = "\n".join([string, inp]) if string and inp else "".join([string, inp])

Why is this line not pythonic ? Because it contains more than one instruction by line

  Sparse is better than dense.

string = ('\n' if string and inp else '').join([string, inp])

Why is this line not pythonic ? Still not the prettiest way of writing that.

  Beautiful is better than ugly.

delimiter = '\n' if string and inp else ''
string = delimiter.join([string, inp])

I will say later in this post why this is still ugly and how to improve this code.

Is a recursive function pythonic ?

No, yes, it's complicated. First, the call stack hell. Every time you call a function, python has to "store" the addresses of the caller into a call stack, the depth of the call stack is 1000, which is not much. So...

while and for are often better than recursive in terms of readability (not always true)
while and for are often better than recursive in terms of performance (not always true)
Everything that can be done in a loop can be done recursively (with python not always true)
Everything that can be done recursively can be done with a while or for loop. (always true)
The stack is the limit. 1000 calls comes quick, for example there are more than 1000 words in this post, it wouldn't be a good idea to use recursion to parse it, it would raise a RecursionError: maximum recursion depth exceeded.

Is my implementation of a recursive function good ?

No, because you pass the accumulator as an argument (classic beginner mistake). You think in terms of loops, even when using recursion. Let's take a look at a simple function.

def sum(l):                  #  No hidden mystery accumulator
    if len(l)==1:            #  The first line is usually the stop condition
        return l[0]          #  Return the last element
    return l[0] + sum(l[1:]) #  Return the first + the sum of the rest

sum(range(42))  # 861

Let's take a closer look at how it calculates this:

def sum(l):
    if len(l)==1:
        return l[0]
    return f"({l[0]}+{sum(l[1:])})"

sum(range(10))  # '(0+(1+(2+(3+(4+(5+(6+(7+(8+9)))))))))'

Impressive !

Now look at your function:

def capitalize_line(string=""):     # Hidden mystery accumulator
    inp = input("enter: ").upper()  # Move this outside of your function
    # This line is already debunked
    string = "\n".join([string, inp]) if string and inp else "".join([string, inp])
    if not inp:
        print(string)    # Move this outside of your function
        return string    # You don't use this but you should
    return capitalize_line(string=string) # not using return as intended
                         # (carrying processed data down the call stack)

  Explicit is better than implicit.

Using stdin and stdout inside a function whose purpose is to capitalize letters is not a good thing. It makes the code impossible to divide because everything is linked. The capitalize function should capitalize and return a string and nothing else.

How should I write it ?

You think I will give you the solution like that... I'm sorry Dave, I'm afraid I can't do that !

Just kidding, but promise me you will try before looking at it:

  def capitalize(l):
    if not l:
        return ''
    return l[0].upper() + '\n' + capitalize(l[1:])

def capitalize(l): # alternative way
    if len(l)==1:
        return l[0]
    return l[0].upper() + '\n' + capitalize(l[1:])

def main(): # Python 3.8 only
    inputs = []
    while s:=input('enter: '): inputs.append(s)
    print(capitalize(inputs))

def main():
    inputs = []
    while True:
        s = input('enter: ')
        if not s:
            break
        inputs.append(s)
    print(capitalize(inputs))

if __name__ == '__main__':
    main()   # In case someone imports your module to use your functions
 But You want to do like the pros don't you ? Use generators
def capitalize(iterable):
    try:
        s = next(iterable)
    except StopIteration:
        return ''
    return s.upper() + '\n' + capitalize(iterable)

def inputs():
    while True:
        s = input('enter: ')
        if not s:
            break
        yield s

Even better (or worse), a recursive generator:
def inputs():
     s = input('enter: ')
     if not s: return
     yield s
     yield from inputs()

def main():
    print(capitalize(inputs()))

Wow, you're still there, it means you had the time to read my entire post, and since you've time on your hands, I recommend you Socratica, more specifically the video about recursion.

And remember, recursion isn't often the best solution.


Python / Numpy running 15x slower than MATLAB - am I using Numpy effeciently? Here is some code I wrote in Python / Numpy that I pretty much directly translated from MATLAB code. When I run the code in MATLAB on my machine, it takes roughly 17 seconds. When I run the code in Python / Numpy on my machine, it takes roughly 233 seconds. Am I not using Numpy effectively? Please look over my Python code to see if I'm using Numpy in a non effective manner. All this code is doing is fitting the parameter D (diffusion coefficient) in the heat equation to some synthetically generated data using MCMC method.

import numpy as np
from numpy import *
import pylab as py
from pylab import *
import math
import time

def heat(D,u0,q,tdim):
    xdim = np.size(u0)
    Z = np.zeros([xdim,tdim])
    Z[:,0]=u0;
    for i in range(1,tdim):
        for j in range (1,xdim-1):
            Z[j,i]=Z[j,i-1]+ D*q*(Z[j-1,i-1]-2*Z[j,i-1]+Z[j+1,i-1])
    return Z

start_time = time.clock()
L = 10
D = 0.5

s = 0.03  # magnitude of noise

Tmax = 0.2
xdim = 25
tdim = 75

x = np.linspace(0,L,xdim)
t = np.linspace(0,Tmax,tdim)

dt = t[1]-t[0]
dx = x[1]-x[0]

q = dt/(dx**2)
r1 = 0.75*L
r2 = 0.8*L

################################################
## check the stability criterion dt/(dx^2)<.5 ##
################################################

# Define the actual initial temperature distribution
u0 = np.zeros(xdim)
for i in range(0,xdim):
    if(x[i]>=r1 and x[i]<=r2):
        u0[i] = 1
xDat = range(1,xdim-1)
tDat = np.array([tdim])
nxDat = len(xDat)
ntDat = 1
tfinal = tdim-1

# synthesize data
Z = heat(D,u0,q,tdim)
u = Z[xDat,tfinal] # matrix
uDat = u + s*randn(nxDat)

# MATLAB PLOTTING
#figure(1);surf(x,t,Z); hold on;
#if ntDat>1, mesh(x(xDat),t(tDat),uDat);
#else set(plot3(x(xDat),t(tDat)*ones(1,nxDat),uDat,'r-o'),'LineWidth',3);
#end; hold off; drawnow

#MCMC run
N = 10000
m = 100
XD = 1.0
X = np.zeros(N)
X[0] = XD
Z = heat(XD,u0,q,tdim)
u = Z[xDat,tfinal]
oLLkd = sum(sum(-(u-uDat)**2))/(2*s**2)
LL = np.zeros(N)
LL[0] = oLLkd

# random walk step size
w = 0.1
for n in range (1,N):
    XDp = XD+w*(2*rand(1)-1)
    if XDp > 0:
        Z = heat(XDp,u0,q,tdim)
        u = Z[xDat,tfinal]
        nLLkd = sum(sum( -(u-uDat)**2))/(2*s**2)
        alpha = exp((nLLkd-oLLkd))
        if random() < alpha:
            XD = XDp
            oLLkd = nLLkd
            CZ = Z
    X[n] = XD;
    LL[n] = oLLkd;

print time.clock() - start_time, "seconds"

In the heat function, simply vectorizing the inner loop, drops the time from 340 sec to 56 sec, a 6x improvement.  It starts by defining the first column of Z, and calculates the next column from that (modeling heat diffusion).

def heat(D,u0,q,tdim):
    xdim = np.size(u0)
    Z = np.zeros([xdim,tdim])
    Z[:,0]=u0;
    for i in range(1,tdim):
        #for j in range (1,xdim-1):
        #    Z[j,i]=Z[j,i-1]+ D*q*(Z[j-1,i-1]-2*Z[j,i-1]+Z[j+1,i-1])
        J = np.arange(1, xdim-1)
        Z[J,i] = Z[J,i-1] + D*q*( Z[J-1,i-1] - 2*Z[J,i-1] + Z[J+1,i-1] )
    return Z

some added improvement (10x speedup) by streamlining the indexing

    Z1 = Z[:,i-1]
    Z[j,i] = Z1[1:-1] + D*q* (Z1[:-2] - 2 * Z1[1:-1] + Z1[2:])

Better yet.  This drops time to 7sec, a 45x improvement.  It constructs a matrix with 3 diagonals, and applies that repeatedly to the u vector (with a dot product).

def heat(D,u0,q,tdim):
    # drops time to 7sec
    N = np.size(u0)
    dq = D*q
    A =  np.eye(N,N,0)+ dq*(np.eye(N,N,-1)+np.eye(N,N,1)-2*np.eye(N,N,0))
    Z = np.zeros([N,tdim])
    Z[:,0] = u0;
    # print u0.shape, A.shape, (A*u0).shape, np.dot(A,u0).shape
    for i in range(1,tdim):
        u0 = np.dot(A,u0)
        Z[:,i] = u0
    return Z

Based on further testing and reading, I think np.dot(A,u0) is using the fast BLAS code.

For larger dimensions (here xdim is only 25), scipy.sparse can be used to make a more compact A matrix.  For example, a sparse version of A can be produced with

sp.eye(N,N,0) + D * q * sp.diags([1, -2, 1], [-1, 0, 1], shape=(N, N))

But there isn't a speed advantage at this small size.


Correcting duplicate names in array I have an array of file names. For example:

string[] files= { "one.zip", "two.txt", "one.zip", "three.txt", "three.txt", "one.zip" };

And I've developed the following method to correct duplicate names. What the method does is just change duplicate names. I am just adding incremented value for the next duplicated value. For example, my developed method ChangingDuplicateNames(string[] files)correct the previous array to:

string[] files= { "one.zip", "two.txt", "one1.zip", "three.txt", "three1.txt", "one2.zip"};

And implementation of ChangingDuplicateNames(string[] files) is:

private string[] ChangingDuplicateNames(string[] files)
{
    //Creating a dicitonary to store duplicated values. "Key" of dictionary
    //is duplicated name, "Value" of dictionary is number to add for name
    Dictionary<string, int> duplicateNames = files.GroupBy(x => x)
        .Where(group => group.Count() > 1)
        .ToDictionary(grouped => grouped.Key, grouped => 0);

    if (duplicateNames.Count == 0)
        return files;

    int namesLength = files.Length;
    string actualName = string.Empty;
    for (int indexArray = 0; indexArray < namesLength; indexArray++)
    {
        int incrementedValue;
        bool isDuplicate = duplicateNames.TryGetValue(files[indexArray],
            out incrementedValue);
        if (isDuplicate)
        {
           actualName = files[indexArray];
           if (incrementedValue == 0)
              files[indexArray] = files[indexArray];
           else
           {
               //Adding increment to the mext duplicate name
               string fileNameWithoutExtension = Path
                    .GetFileNameWithoutExtension(files[indexArray]);
               string fileExtension = Path
                   .GetExtension(files[indexArray]);
               files[indexArray] = fileNameWithoutExtension +
                   "(" + incrementedValue + ")" + fileExtension;
           }
           duplicateNames[actualName] = ++incrementedValue;
        }
    }
    return files;
}

And my question is: Is it possible to improve this algorithm? I mean could be this code smaller?

Maybe I should not iterate through the all array of names, but I cannot figure out how I can change names in files without iterating through all array. Thanks in advance.

A quick and easy way would be to use a HashSet<T> and check the return of the Add() method. If it returns false you will know that you need to create a new filename. So let's start by extracting the creation  of the filename like so

private string CreateFileName(string original, ref int counter)
{
    counter += 1;
    string fileNameWithoutExtension = Path.GetFileNameWithoutExtension(original) + "(" + counter + ")";
    string fileExtension = Path.GetExtension(original);
    return fileNameWithoutExtension + fileExtension;
}

Next we iterate over all of the items of the array and try to add it to the HashSet<T>. If we can't add it, we create the new filename and try to add it again until we succeed.

private string[] ChangingDuplicateNames(string[] files)
{
    var hashSet = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
    foreach (var file in files)
    {
        var currentFile = file;
        int counter = 0;
        while (!hashSet.Add(currentFile))
        {
            currentFile = CreateFileName(file, ref counter);
        }
    }

    return hashSet.ToArray();
}


Validating Yes/No answers in C++ I intend to accept all of the following answers:

y
Y
yes
YES
Yes
plus mixtures, such as yeS, yES, which I didn't intend to accept but it's a consequence of my code
the same applies to No and its variants

#include <iostream>     // std::cout, std::cin
#include <string>       // std::string
#include <algorithm>    // std::transform

int main()
{

    bool answer_valid = false;
    std::string answer;

    do
    {
        std::cout << "Should the password include digits? [Y/n] ";
        std::cin >> answer;

        std::transform(answer.begin(), answer.end(), answer.begin(), ::tolower);

        answer_valid =
            (answer == "y") ||
            (answer == "n") ||
            (answer == "yes") ||
            (answer == "no");

    } while (!answer_valid);

    return 0;

}

It works flawlessly, or it seems so. Is there better approach?

You have a common bug that will send you into an infinite loop. :-)

When reading data (especially user input data) you must validate the read worked correctly. If the read fails and sets one of the error flags on the stream than any subsequent attempt to read will silently be ignored.

Thus usually read operations are checked in the loop test.

std::string word;
while(word != "STOP") {
    std::cin >> word;
    std::cout << "Word: " << word << "\n";
}

In the above you will enter an infinite loop if your hit an EOF marker (Note: this can be generated from the keyboard) or in unix systems a file can be piped to the input stream of an application etc..

To prevent this you test the result of the read operation.

std::string word;
while(std::cin >> word && word != "STOP") {
    std::cout << "Word: " << word << "\n";
}

This works because of two things. The operator>> returns a reference to the stream (usually for chaining). When a stream is used in a boolean context (like a test) it is converted to a bool using operator bool which calls good() which returns true if the stream is still in a good state (ie the read operation did not cause an error).

Secondly declares variables as close to the use point as possible.

This is a good habit to get into with C++ as objects have constructors that can potentially be expensive (also destructors). Declaring them at the top of the function and then not using them is wasteful.

Also be declaring them near the use point also allows for locality of reading when checking variables types.

In your case:

    std::string answer;

This is declared at the top but only used inside the loop.

Also prefer not to use return 0; in main() when the application never returns an error state. The compiler will automatically generate the required return 0 and leaving it off is a marker used by programs to indicate that the program never fails.

Give this I would rewrite as:

bool checkAnswerOK(std::string& answer, bool& result)
{
    std::transform(answer.begin(), answer.end(), answer.begin(),
                   [](unsigned char x){return ::tolower(x);});

     bool answer_valid =
            (answer == "y")   ||
            (answer == "n")   ||
            (answer == "yes") ||
            (answer == "no");

    result = answer_valid && answer[0] == 'y';
    return answer_valid;
}

bool question_yesno(std::string const& message)
{
    std::string answer;
    bool        result;

    std::cout << message << "? [Y/n]\n";
    while(std::cin >> answer && !checkAnswerOK(answer, result))
    {
        std::cout << "Invalid answer: " << answer << " Please try again\n"
                  << message << "? [Y/n]\n";
    }
    if (!std::cin) {
        // We never got an answer.
        // Not much we can do here. Probably give up?
        throw std::runtime_error("User Input read failed");
    }
    return result;
}

int main()
{
    question_yesno("Should the password include digits");
}


Process List asynchronously with SemaphoreSlim for throttling extension method I've got some methods that follow a similar pattern, and wanted to abstract some of the logic away into an extension method. Specifically, I am processing lists of objects with the TPL, and throttling that processing with a SemaphoreSlim.

public static Task ValidateProxiesAsync(IList<Proxy> proxies, IList<ProxyJudge> judges, IList<ProxyTest> tests)
{
    return Task.Run(async () =>
    {
        await ValidateTestsAsync(judges.ToList<IProxyTest>());
        await ValidateTestsAsync(tests.ToList<IProxyTest>());
        var semaphore = new SemaphoreSlim(100);
        var tasks = new List<Task>();
        foreach (var proxy in proxies)
        {
            await semaphore.WaitAsync();
            tasks.Add(Task.Run(() =>
            {
                proxy.Status = proxy.TestValidity(judges[0]);
                semaphore.Release();
            }));
        }
        await Task.WhenAll(tasks);
    });
}

public static Task ValidateTestsAsync(IList<IProxyTest> tests)
{
    return Task.Run(async () =>
    {
        var semaphore = new SemaphoreSlim(100);
        var tasks = new List<Task>();
        foreach (var test in tests)
        {
            await semaphore.WaitAsync();
            tasks.Add(Task.Run(() =>
            {
                test.Status = test.TestValidity();
                semaphore.Release();
            }));
        }
        await Task.WhenAll(tasks);
    });
}

I wrote this extension method to reduce the LOC needed to do this. But I wanted to make sure I didn't overlook something. Perhaps there is a better solution that someone on here could suggest (TPL Dataflow?).

Extension method:

    public static Task ProcessWithSemaphoreAsync<T>(this IList<T> list, Action<T> action, int maxConcurrency)
    {
        return Task.Run(async () => {
            var semaphore = new SemaphoreSlim(maxConcurrency);
            var tasks = new List<Task>();
            foreach (var item in list)
            {
                await semaphore.WaitAsync();
                tasks.Add(Task.Run(() =>
                {
                    action.Invoke(item);
                    semaphore.Release();
                }));
            }
            await Task.WhenAll(tasks);
        });
    }

The only thing I can see is that you should dispose the semaphore:

using (var semaphore = new SemaphoreSlim(maxConcurrency))
{
  var tasks ....
}

And you could generalize to IEnumerable<T> instead of IList<T>

A simple solution using Parallel.ForEach() could be:

public static void ProcessWithParallelAsync<T>(this IEnumerable<T> list, Action<T> action, int maxConcurrency)
{
  ParallelOptions options = new ParallelOptions();
  options.MaxDegreeOfParallelism = maxConcurrency;
  Parallel.ForEach(list, options, action);
}

Or you could wrap it in a Task if you like:

public static Task ProcessWithParallelAsync<T>(this IEnumerable<T> list, Action<T> action, int maxConcurrency)
{
  return Task.Run(() =>
  {
    ParallelOptions options = new ParallelOptions();
    options.MaxDegreeOfParallelism = maxConcurrency;

    Parallel.ForEach(list, options, action);
  });
}


Django class based view URL API I kind of started making an API for my site in a Django class based view because it seemed logical and now it has gotten quite large and unruly and I am starting to wonder if I am doing this correctly.

class URLAPI(View):

    def get(self, request):

         if request.GET.get('param') == "foo":
             ...

         elif request.GET.get('param') == "bar":
             ...

         elif request.GET.get('param') == "foo" and request.GET.get('param2') == "arg":
             ...

I did the same thing with the post function creating conditionals for the data that is coming in. What do you think about this?

If you are going to have a lot of cases, and each one is quite complex (more than a few lines) I would ditch the if/elif approach.

Instead I would have inner functions, one for each case. Then a dictionary will map each case ('foo', 'bar' etc) to its function.

For example:

class URLAPI(View):
    def get(self, request):
        def act_on_foo():
            ...
        def act_on_bar():
            ...

        func_dict = {'foo': act_on_foo,
                     'bar': act_on_bar}

        param = request.GET.get('param')
        func_dict[param]()

Perhaps an even better design would be to move the act_on_foo and act_on_bar functions to another module to decouple the view from the logic behind it.

And some nitpicking:
Try to avoid mixing " and ' in the same piece of code. Pick one and stick to it.