Asynchronous API calls with delegates and event handlers I'm making a WindowsPhone 8 app in C#. With this an API so I can convert JSON data into useful objects. I've found a method but am wondering if it is any good at all.
The problem is it's been a while since I've really worked with C# and never for windows phone.

Because it's making use of web requests it has to be asynchronous. From somewhere User.getTracks() should be called and handled. To do this I've come up with a solution but I don't know if it's a good one.

I have an ApiCaller class which has a few different ApiAction methods, these methods receive a generic to know what type to eventually serialize to. The main method receives all the data, makes the web request and serializes the JSON to an object.

public static async void ApiAction<T>(Uri uri, CallExecuted cbDelegate, HttpMethod method, bool requireAuthentication)

Here it receives a CallExecuted this is the delegate from the calling class. After serializing the JSON data it calls this delegate with the item as parameter cbDelegate(new CallbackArgs(item));

The calling class will have the following to process everything

    public delegate void GetTracksExecuted(List<Track> tracks); // The delegate other classes use
    public static event GetTracksExecuted tracksExecuted; // The local event that calls the above delegate

    // This is called from elsewhere
    // The parameter is the delegate from that class
    public void GetTracks(GetTracksExecuted e)
    {
        tracksExecuted += e;
        ApiCaller.ApiAction<List<Track>>(ApiCall.UserTracks, TracksDelegate, Id);
    }

    // The delegate given to ApiCall class
    public delegate void CallExecuted(CallbackArgs e);

    // The delegate that is called from the ApiCall class (CallExecuted)
    private static void TracksDelegate(CallbackArgs args)
    {
        List<Track> tracks = (List<Track>) args.Item;
        tracksExecuted(tracks);
    }

Is there a better way to accomplish any of this? Was thinking about just using the event to do it, leaving the delegate out, but don't know if that's better. Your input is very welcome

public static async void ApiAction<T>(Uri uri, CallExecuted cbDelegate, HttpMethod method, bool requireAuthentication) Here it

  receives a CallExecuted this is the delegate from the calling class.
  After serializing the JSON data it calls this delegate with the item
  as parameter cbDelegate(new CallbackArgs(item));

I think you should learn how to use async-await properly. Instead of using async void (which is a bad practice in general) and taking a delegate, your method should return something like Task<List<Track>>.

This way, your method would look like this:

public Task<List<Track>> GetTracksAsync()
{
    return ApiCaller.ApiAction<List<Track>>(ApiCall.UserTracks, Id);
}

You wouldn't need all the boilerplate around it and you would also handle exceptions properly.

To use GetTracksAsync(), you would then use await:

List<Track> tracks = await GetTracksAsync();


Check if a given number N is a power of k I am trying to solve a problem to check whether a given number N is a power of k.

Example:

Input 9, 3  > True //  3**2 = 9
Input 10, 2  > False
Input 4096, 16  > True // 16**3 = 4096

I want to know if there is a better solution for this problem.

def check_Power(N,k):
    if N <= 0 or k <=0:
        print "not a valid input"
    else:
        for i in range (1,20):
            x = k**i
            if x == N :
                print " True "
                print k, "power ", i , "is", N
                break
            elif x> N:
                print "false"
                break

check_Power(244140625,5)
check_Power(4096, 16)
check_Power(0, 16)
check_Power(1,1)

I'd follow Ludisposed's answer.

Except, there's a simple way to do this. Use logarithms.
Since the equation is \$N = k^a\$ you can change it to \$a = \log_k N\$.
And so if \$a\$ is an integer, then you know that \$N\$ is a power of \$k\$.

import math

def check_power(N, k):
    return math.log(N, k).is_integer()

Since the above can error, and if there's an error it's definately not a power, then you can use:

def check_power(N, k):
    try:
        return math.log(N, k).is_integer()
    except Exception:
        return False

This has the some problems with floating point precision, so your can round the number to the closest integer, as you want to know if the \$a\$ is an integer. And so you can sub it back into the original equation to ensure that the numbers are right.

def check_power(N, k):
    return N == k**round(math.log(N, k))

Or if you're on Python 2:

def check_power(N, k):
    return N == k**int(round(math.log(N, k)))

This has a problem with some inputs, such as when \$N\$ and \$k\$ are \$0\$ or \$1\$. And so we can handle these special cases.

def check_power(N, k):
    if N == k:
        return True
    return N == k**round(math.log(N, k))

And so merging all the above, we can use the following:

def check_power(N, k):
    if N == k:
        return True
    try:
        return N == k**int(round(math.log(N, k)))
    except Exception:
        return False


CodeIgniter method to get requests for superiors to approve I'm currently working on an approval system right now using PHP (Codeigniter) and I have this method of getting all the requests according to your department group and position depth.

The checking of the requests is based upon the user's position depth. 6 being the lowest and 1 being the highest. So basically there will be a maximum of 5 people to check the request. For example my depth is 6, line1 will be 5, line2 will be 4 and so on. The system will find someone with position depths like that and same department group. Lines 3 and 2 will decide to send it to ceo (1) or not so the default is only until 2.

Some of my database columns

It is designed that people above the line can check the request even if the lower lines havent checked it yet like for example maybe line1 havent checked it yet because he/she is on vacation and line3 cannot wait already so he/she decides to process the request because it is important. Now if the above line processes it already, lower lines cannot process it anymore so they will only be allowed to view it.

Now here comes my code of nested for loops in checking out all these things that I said. People say nested for loops, especially as many as mine is not a good practice. Is my for loop gonna have problems when it comes to large data? If so how do I alter my code without altering my database for this?

This is my Model

public function getRequestsToProcess($i,$depth,$departmentGroup)
{
    $this->db->select('*, ns_request_info.idx as request_idx, ns_staff_info.idx as staff_idx');
    $this->db->from('ns_request_info');
    $this->db->join('ns_staff_info', 'ns_request_info.requested_by_idx = ns_staff_info.member_idx');
    $this->db->where('line'.$i.'', $depth);
    $this->db->where('line'.$i.'_action', 'PENDING');
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $this->db->where('status', 'PENDING');
    $this->db->or_where('status', 'ONGOING');
    $this->db->where('line'.$i.'', $depth);
    $this->db->where('line'.$i.'_action', 'PENDING');
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $query = $this->db->get();
    return $query->result_array();
}

This is my method with nested for loops

public function process()
{
    $data = new stdClass;
    $data->param_menu = 'request';

    $staffInfo = $this->staffinfo_model->selectItem(array('member_idx'=>$this->session->userdata('member_index')));

    for ( $i=1 ; $i<=5 ; $i++) {
        /*get lines with user depth and if same department group*/
        ${"line" . $i} = $this->staffrequisition_model->getRequestsToProcess($i,$staffInfo->position_depth,$staffInfo->department_group);

        for ($x = 0 ; $x < count(${"line" . $i}) ; $x++) {
            $counter = 0;
            for ($y = $i + 1 ; $y <=5 ; $y++) {
                if (${"line" . $i}[$x]['line'.$y.'_action']!=='PENDING' && ${"line" . $i}[$x]['line'.$y.'_action']!=='') {
                    $counter++;
                    break;
                }
            }
            if ($counter > 0) {
                ${"line" . $i}[$x]['is_checked'] = 1;
            } else {
                ${"line" . $i}[$x]['is_checked'] = 0;
            }
        }
    }

    $data->param_requests = array_merge($line1, $line2, $line3, $line4, $line5);

    $this->load->view('dashboard/staff/process_view', $data);
}

Now is_checked being the marker if the request is already checked in upper lines. I hope you guys can give me any tips on how to optimize my code.

I am able to remove one for loop in my code with the following:

I changed my method in the Controller

public function process()
{
    $data = new stdClass;
    $data->param_menu = 'request';

    $staffInfo = $this->staffinfo_model->selectItem(array('member_idx'=>$this->session->userdata('member_index')));

    $requests = $this->staffrequisition_model->getRequestsToProcess($staffInfo->position_depth,$staffInfo->department_group);

    foreach ($requests as &$item) {
        for ($y = 1; $y <=5 ; $y++) {
            if ($item->{'line'.$y.'_action'}!=='PENDING' && $item->{'line'.$y.'_action'}!==''){
                $item->is_checked = TRUE;
                break;
            } else {
                $item->is_checked = FALSE;
            }
        }
    }

    $data->param_requests = $requests;

    $this->load->view('dashboard/staff/process_view', $data);
}

I changed my method in the Model

public function getRequestsToProcess($depth,$departmentGroup)
{
    $this->db->select('*, ns_request_info.idx as request_idx, ns_staff_info.idx as staff_idx');
    $this->db->from('ns_request_info');
    $this->db->join('ns_staff_info', 'ns_request_info.requested_by_idx = ns_staff_info.member_idx');
    $this->db->where("((ns_request_info.line1=".$depth." AND ns_request_info.line1_action='PENDING') OR (ns_request_info.line2=".$depth." AND ns_request_info.line2_action='PENDING') OR (ns_request_info.line3=".$depth." AND ns_request_info.line3_action='PENDING') OR (ns_request_info.line4=".$depth." AND ns_request_info.line4_action='PENDING') OR (ns_request_info.line5=".$depth." AND ns_request_info.line5_action='PENDING'))");
    $this->db->where("(ns_request_info.status='PENDING' OR ns_request_info.status='ONGOING')");
    if($departmentGroup!='HIGHER TIER'){ $this->db->where('ns_staff_info.department_group', $departmentGroup); }
    $this->db->order_by('ns_request_info.idx', 'desc');
    $query = $this->db->get();
    return $query->result();
}

Now I dont have to loop my query five times. Instead I get it with just one query and loop through the result.


Graphical game application I'm writing my first JavaScript application (no 3rd party libraries used) and looking for some high-level guidance on how to better organize the code into separate classes & files.

Here is a simplified/contrived example to illustrate my current organizational layout/problems:

game.js:

/* Lots of constants, enums, etc defined here.
   Can't define them inside Game function as
   they are used by Graphics and ui_player classes too.
*/

function Game(ui) {

    /* Lots of things defined here which are exclusive to Game class. */

    var graphics = new Graphics();
    graphics.load();
    ui.setCanvas(graphics.canvas);

    this.start = function() {
        mainLoop();
    };

    function mainLoop() {
        /* ... */
        ui.player.updateAbc(abc);
        /* ... */
        paint();
    }

    function paint() {
        graphics.renderAll()
    }

    /* ... */
}

graphics.js:

function Graphics() {

    /* Some constants defined here that are exclusive to Graphics class. */

    var canvas = this.canvas = ...;
    var ctx = canvas.getContext("2d");

    /* ... */

    this.renderAll = function() {
        ctx.drawImage(...);
    };

    /* ... */
}

ui.js:

/* Lots of constants related to UI classes here. */

function ui() {
    var div = this.div = ...;
    var canvas;

    /* ... */

    var player = this.player = new ui_player();
    div.appendChild(player.div);

    this.setCanvas(c) {
        if (canvas)
            div.removeChild(canvas);
        canvas = c;
        div.appendChild(canvas);
    };

    /* ... */
}

ui_player.js:

 function ui_player() {
     var div = this.div = ...;

     /* ... */

     this.updateAbc = function(abc) {
         div.innerHTML = abc;
     };
 }

main.js:

 ui = new ui();
 game = new Game(ui);
 game.start();

What I dislike about this:

I feel that my scopes are clumsy.
I dislike the fact that I have constants in global scope because they are shared across multiple classes.
I dislike the fact that I have to pass ui to Game and then to Graphics, but I'm not sure if this is bad or if allowing every class to access others would be worse.

How can I improve my code structure? How can/should I put it all into a namespace hierarchy where classes can access classes above (and maybe even below) them?

P.S. - I'm compiling with Google Closure Compiler (advanced) if that makes any difference. No 3rd party libraries are used.

Using a closure

You can avoid putting constants in the global scope by wrapping everything in another closure, i.e. the code produced after the compiling process should look something like this:

(function() {
/* all your definitions here */
function ui() {
    // code here
}

function Game(ui) {
    // code here
}

function Graphics(ui) {
    // code here
}
}).call(this);

Thereby all the functions have access to your constants without cluttering the global namespace.

Supporting RequireJS/AMD

RequireJS is a library that aims to give you the possibility to import modules, something JavaScript lacks by default. It does so using 'dependency injection', i.e. passing all the things code requires to execute as parameters to a wrapping function. (You're already doing this in your code - kudos! Even though you seem not to like it, definitely stick to it!) This is considered a best practice in many programming languages - including JavaScript, duh - as it allows for nice modularization of code and great testability by replacing other modules that you don't want to test by mock implementations that might always return the same input, thereby you are able to test just parts of your code and locate errors more quickly. This is called unit testing - something you will never want to miss again.

By supporting RequireJS natively you will save your users from a lot headaches figuring out how exactly to get references to your code.
Doing so is quite easy, you check if the define function exists and use it or release everything to the global scope if it doesn't:

var lib = {
    ui: ui,
    Game: Game,
    Graphics: Graphics
}

if (typeof define === "function") {
    define("mylib", [], function() { return lib; });
} else {
    window.mylib = lib;
}

EDIT: When I wrote this I thought you wrote that you were creating your first library - not application. That being the case you can ignore the last paragraph, the first in this section still applies, though.

Building your library as a set of AMD modules

You can leverage the power of RequireJS/AMD for your own application. Just use any directory structure you like and use dependency injection to seperate them into files nicely. The RequireJS optimizer can then put them in a single file, you would still need RequireJS, though. Meet almond! It is only about 1kb minified+gzipped and defines a subset of the functionality of RequireJS that suffices to still use AMD modules inside a single file, which is ultimately what you want: a single file you  can include in their HTML or load using Require. Now, unfortunately, this introduces another step in going from your source to a distributable JS file.

Using a sophisticated build tool

Because nobody wants to do 10 steps manually each time you change your code and want a new build, sophisticated build tools were created that invoke the compiler(s), do code linting, etc. I personally recommend Grunt. It is IMHO the best and most flexible option out there and big libraries, e.g. jQuery, use it as well.

Don't reinvent the wheel

There already are great libraries that can do some of the heavy lifting for you. While for a first application it is a good idea to do things yourself to learn how they work internally, once you start working on production code you want to rely on existing libraries, because

that leaves you more time to work on your app and make it better
they make your code more error proof:

many libraries have been around for quite a long time and hardly have bugs
bugs that are found will most likely be fixed quickly (and by other people, see (1))

many libraries will make some design decisions for you. While you might think that cuts your freedom as a programmer (it certainly does), it also has a great advantage: Other people have spent a great deal on working out a good concept that will probably make your app easier to maintain in the end. So just choose one you like and go with it...
let's face it: people who dedicate their whole time to working on a library probably have better implementations of stuff than you, so that might make your app faster

Additional Points

I will add further points here with edits, but wanted to give you access to existing sections asap ;)


NetBuffer - buffer specialized for socket read / write This class is for fast and small footprint buffering made specially for network I/O. It is similar to so-called circular buffers, but is not circular.

#ifndef _IO_BUFFER_H
#define _IO_BUFFER_H

#include <cstdint>
#include <cstring>
#include <cstdio>

namespace net{

template<size_t CAPACITY>
class IOBuffer{
public:
    using size_type = uint16_t;

private:
    size_type   head_   = 0;
    size_type   tail_   = 0;
    char        buffer_[CAPACITY];

public:
    void clear(){
        head_ = 0;
        tail_ = 0;
    }

    constexpr
    static size_t max_size(){
        return CAPACITY;
    }

    // ==================================

    const char *data() const{
        return & buffer_[head_];
    }

    char *dataTail(){
        return & buffer_[tail_];
    }

    size_t size() const{
        return tail_ - head_;
    }

    size_t capacity() const{
        return CAPACITY - tail_;
    }

    // ==================================

    bool push(const char *p){
        return p ? push(strlen(p), p) : false;
    }

    bool push(size_t const len, const void *ptr = nullptr){
        if (len == 0)
            return false;

        if (capacity() < len)
            return false;

        if (ptr)
            memmove(&buffer_[tail_], ptr, len);

        tail_ = (size_type) (tail_ + len);

        return true;
    }

    bool pop(size_t const len){
        if (len == 0)
            return false;

        auto const available = size();

        if (available < len)
            return false;

        if (available == len){
            clear();
            return true;
        }

        head_ = (size_type) (head_ + len);

        return true;
    }

    // ==================================

    void print() const{
        printf("h: %3u | t: %3u | ad: %3zu | ac: %3zu | %.*s\n",
                head_, tail_,
                size(), capacity(),
                (int) size(), buffer_ );
    }
};

} // namespace

#endif

Usual usage include several socket read() until protocol data is collected, then several write() until response is sent back.

Reads and writes will not be mixed, but for the moment this is responsibility of the caller.

print is debug method, non intended to be called.

int main(){
    using Buffer = net::IOBuffer<10>;

    Buffer b;
    b.print();

    b.push("aaa");
    b.print();

    b.push("bbb");
    b.print();

    b.push("ccc");
    b.print();

    b.push("xxx");
    b.print();

    b.push("d");
    b.print();

    b.push("xxx");
    b.print();

    b.pop(3);
    b.print();

    b.pop(3);
    b.print();

    b.pop(b.size()); // same as b.clear()
    b.print();
}

One of the benefits of circular buffers is that they can be filled up to the entire size of the buffer, with completed fields being extracted from the head of the buffer immediately freeing up space.  This isn't the case for your buffer.  Space in the buffer is never freed until the entire buffer has been read, so if partial fields are consistently read there is a risk the buffer will get to a full state.

dataTail

You've declared your other access methods, including data as const.  Is it a mistake to not declare dataTail as const?  It's not entirely clear to me why a client would need to have a pointer to the bit of the buffer you haven't used.  Or are you expecting the client to actually write to the dataTail, without amending the tail_ afterwards?!?

push

Your extended push seems like it has a bug (or allows a very strange use case):

bool push(size_t const len, const void *ptr = nullptr){
    if (len == 0)
        return false;

    if (capacity() < len)
        return false;

    if (ptr)
        memmove(&buffer_[tail_], ptr, len);

    tail_ = (size_type) (tail_ + len);

    return true;
}

If ptr is null, then no bytes are added to the buffer, however the tail is still incremented as if data had been written.  So effectively, whatever is already in the buffer is added instead.  Having ptr default to null suggests this might be expected behaviour but it seems very odd.

print

print may be a debug method, however its output is confusing.  As you pop from the buffer, you're reducing the number of characters from the buffer that are displayed however you're still printing from the start of the buffer.  This gives the impression that pop is actually removing from the tail, rather than the head.  You should probably be printing from the head instead:

printf("h: %3u | t: %3u | ad: %3zu | ac: %3zu | %.*s\n",
        head_, tail_,
        size(), capacity(),
        (int) size(), data());


PHP Sort Associative Array first by relativity of value then by other value desc I am trying to sort an associative array by following this scheme:

Sort by 'qt': positives first, non-positives after.  Magnitude does not matter, only sign.
In positive part: sort by sa DESC
In negative part: sort by sa DESC

I managed to have the desired result but by tinkering :
$array = [
    [
        "id" => 1,
        "qt" => 1000,
        "sa" => 50
    ],
    [
        "id" => 2,
        "qt" => 0,
        "sa" => 50
    ],
    [
        "id" => 3,
        "qt" => 0,
        "sa" => 350
    ],
    [
        "id" => 4,
        "qt" => 5000,
        "sa" => 250
    ],
    [
        "id" => 5,
        "qt" => 10,
        "sa" => 9000
    ]
];

$p_qty = array();
$n_qty = array();
foreach ($array as $e) {
    if($e['qt'] <= 0){ array_push($n_qty, $e); }
    else             { array_push($p_qty, $e); }
}
usort($p_qty, function($a, $b){
    return
        $a['sa'] == $b['sa'] ? (
            $a['sa'] == $b['sa'] ? 0 : (
                $a['qt'] < $b['qt'] ? 1 : -1
            )
        ) : (
            $a['sa'] < $b['sa'] ? 1 : -1
        );
});
usort($n_qty, function($a, $b){
    return
        $a['sa'] == $b['sa'] ? (
            $a['sa'] == $b['sa'] ? 0 : (
                $a['qt'] < $b['qt'] ? 1 : -1
            )
        ) : (
            $a['sa'] < $b['sa'] ? 1 : -1
        );
});

I get the wanted result: ID [5, 4, 1, 3, 2]
Is there a simple to do the wanted result in a unique usort function ?

You are not actually sorting on qt, only using it to separate two sets (positive qt coming first). Only within each set you are actually sorting (on sa). Therefore, the usort() comparator function could look like this:
usort($array, function($a, $b) {
    // split in positive and negative set
   if ($a['qt'] <= 0 && $b['qt'] > 0) return 1; // b before a
   if ($a['qt'] > 0 && $b['qt'] <= 0) return -1; // a before b

   // both a and b are in the same set. sort on `sa` descending
   return $b['sa'] - $a['sa'];
});


Flash Light Effect Windows Forms Edited I am having performence issues with image processing task (flash light effect). I used c#, and used couple references (LockBits Method, Extension). The program has bad frame rates.

Edited the code, improvments were made but still doesn't match up to my expectations.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Drawing.Imaging;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace ImagePro
{
    public partial class Form1 : Form
    {
        Bitmap Dog;
        Rectangle rect;

        public Form1()
        {
            InitializeComponent();
        }
        private void Form1_Load(object sender, EventArgs e)
        {
            Dog = new Bitmap(@"D:\Programs\VS 2013\Projectss\ImageProcessing\ImageProcessing\dog.jpg");
            rect = new Rectangle(0, 0, Dog.Width, Dog.Height);
            this.Width = Dog.Width;
            this.Height = Dog.Height;
            timer1.Interval = 100;
            timer1.Start();
            this.DoubleBuffered = true;
        }

        private void Form1_Paint(object sender, PaintEventArgs e)
        {
            Bitmap tempImg = new Bitmap(Dog);
            BitmapData imgRawData = tempImg.LockBits(rect, ImageLockMode.ReadWrite, tempImg.PixelFormat);
            IntPtr arrStart = imgRawData.Scan0;

            int size = imgRawData.Stride * tempImg.Height;
            byte[] rgbValues = new byte[size];

            System.Runtime.InteropServices.Marshal.Copy(arrStart, rgbValues, 0, size);
            var absPos = PointToClient(Cursor.Position);
            for (int x = 0; x < tempImg.Width; x++)
            {
                for (int y = 0; y < tempImg.Height; y++)
                {
                    double distence = GetDistance(absPos.X, absPos.Y, x, y); //Distence from pixel to cursor
                    int pixelIndex = x * 4 + y * imgRawData.Stride;

                        for (int i = 0; i < 3; i++)
                        {
                            if (distence > rgbValues[pixelIndex + i])
                            {
                                rgbValues[pixelIndex + i] = 0; //Sets pixel to black
                            }
                            else
                            {
                                rgbValues[pixelIndex + i] -= (byte)distence; //Reduce pixel brightness
                            }
                        }
                        //rgbValues[pixelIndex];          //blue
                        //rgbValues[pixelIndex + 1];     //green
                        //rgbValues[pixelIndex + 2];    //red
                        //rgbValues[pixelIndex + 3];   // alpha
                }
            }
            System.Runtime.InteropServices.Marshal.Copy(rgbValues, 0, arrStart, size);
            tempImg.UnlockBits(imgRawData);
            e.Graphics.DrawImage(tempImg,0,0);
        }
        private static double GetDistance(double x1, double y1, double x2, double y2)
        {
            return Math.Sqrt(Math.Pow((x2 - x1), 2) + Math.Pow((y2 - y1), 2));
        }
        private void timer1_Tick(object sender, EventArgs e)
        {
            this.Invalidate();
        }
    }
}

I don't know much about C#, so this answer will focus solely on generic stuff. Performance problems in image processing applications usually happen inside loops over all pixels. I'm focusing on this section of the code:

for (int x = 0; x < tempImg.Width; x++)
{
    for (int y = 0; y < tempImg.Height; y++)
    {
        double distence = GetDistance(absPos.X, absPos.Y, x, y); //Distence from pixel to cursor
        int pixelIndex = x * 4 + y * imgRawData.Stride;

            for (int i = 0; i < 3; i++)
            {
                if (distence > rgbValues[pixelIndex + i])
                {
                    rgbValues[pixelIndex + i] = 0; //Sets pixel to black
                }
                else
                {
                    rgbValues[pixelIndex + i] -= (byte)distence; //Reduce pixel brightness
                }
            }
    }
}

You loop over all pixels, but only do something for those pixels close to absPos. The maximum distance over which your filter will have effect is 255, assuming that rgbValues is an 8-bit unsigned integer. Outside of that range, you always set pixels to 0.
Thus, you can loop over only those pixels within that range: x goes from max(absPos.X - 255, 0) to min(absPos.X + 255, tempImg.Width - 1), and similarly for the y-coordinate (maybe this is Math.Max and Math.Min in C#?). Outside of that range simply fill with zeros.

Next, GetDistance computes the square root, which is quite expensive. It is a lot cheaper to compute the square of a value. We also know that if a < b, then a*a < b*b. So instead write a function GetSquareDistance, which returns Math.Pow((x2 - x1), 2) + Math.Pow((y2 - y1), 2), and compare the square distance to the square of the pixel's value. By not computing the square root, all operations can be done as integers, which might or might not be faster.

This function GetSquareDistance can actually be further improved by computing Math.Pow((y2 - y1), 2) once outside the loop over x, then adding only the square of the x-difference inside the loop.

Finally, the innermost if statement also duplicates work. I presume that a good optimizing compiler will correct this, but in principle distence > rgbValues[pixelIndex + i] computes the difference and compares to 0, then inside the loop you again compute the same difference. Using a max function would improve this conditional statement:

int diff = (int)rgbValues[pixelIndex + i] - (int)distence;
rgbValues[pixelIndex + i] = (byte)max(diff, 0); // again, maybe Math.Max?

(using int in case integer subtraction is faster than floating-point subtraction.)

Another issue I've just noticed: You loop over y in the inner loop, and over x in the outer loop. If you reverse these two loops, then the array will be accessed in storage order (pixelIndex = x * 4 + y * imgRawData.Stride will increase monotonically). With this access pattern, the cache will be used optimally, and the code should be quite a bit faster.

The code would look like this (not tested, sorry for any typo):

int firstX = max(absPos.X - 255, 0); // max and min could be Math.Max and Math.Min?
int lastX = min(absPos.X + 255, tempImg.Width - 1) + 1;
int firstY = max(absPos.Y - 255, 0);
int lastY = min(absPos.Y + 255, tempImg.Height - 1) + 1;
int pixelIndex = 0;
for (int y = 0; y < firstY; y++)
{
    SetRowToZero(rgbValues + pixelIndex, tempImg.Width); // This function iterates over a line of the image and sets it to 0
    pixelIndex += imgRawData.Stride;
}
for (int y = firstY; y < lastY; y++)
{
    int yDistance2 = absPos.Y - y;
    yDistance2 = yDistance2 * yDistance2;

    SetRowToZero(rgbValues + pixelIndex, firstX); // Here the function sets the first set of pixels in the line to 0

    int index = pixelIndex + firstX * 4;
    for (int x = firstX; x < lastX; x++)
    {
        int xDistance2 = absPos.X - x;
        xDistance2 = xDistance2 * xDistance2;
        int distance2 = xDistance2 + yDistance2;
        for (int i = 0; i < 3; i++)
        {
            int value2 = rgbValues[index + i];
            value2 = value2 * value2 - distance2;
            if (value2 <= 0)
            {
                rgbValues[index + i] = 0; // Sets pixel to black
            }
            else
            {
                rgbValues[index + i] = (byte)Math.Sqrt(value2); // Reduce pixel brightness
            }
        }
        index += 4;
    }

    SetRowToZero(rgbValues + index, tempImg.Width - lastX); // Here the function sets the last set of pixels in the line to 0

    pixelIndex += imgRawData.Stride;
}
for (int y = lastY; y < tempImg.Height; y++)
{
    SetRowToZero(rgbValues + pixelIndex, tempImg.Width);
    pixelIndex += imgRawData.Stride;
}

Additional comments:

Typo: "distence" should be "distance"
Don't leave commented-out code in your source files. They tend to be confusing. If you do, at least add a clear bit of comment text to indicate why the code is commented out.


How to speak Brainfuck: for beginners The following code transforms multi-line input into the the Brainfuck equivalent. One line turns into one program. The resulting programs are split by newlines for readability.

Brainf.cpp

#include "Brainf.hpp"

int main()
{
    std::vector<std::string> userInput;
    std::string currentLine = " "; // do not start empty

    while (currentLine.length() != 0) // escape input by an empty line
    {
        std::getline(std::cin, currentLine);
        userInput.push_back(currentLine);
    }

    std::vector<std::string>::size_type userInputSize = userInput.size();

    for (unsigned i = 0; i < userInputSize; i++)
    {
        std::cout << modifyString(userInput[i]) << std::endl << std::endl;
    }
}

Brainf.hpp

#ifndef BRAINF_HPP
#define BRAINF_HPP

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

std::string modifyChar(char input, int carry)
{
    std::string output;
    int next = input - carry;
    if (next > 0)
    {
        for (unsigned i = 0; i < next; ++i)
        {
            output += "+";
        }
    }
    if (next < 0)
    {
        for (unsigned i = 0; i < abs(next); ++i)
        {
            output += "-";
        }
    }
    output += ".";

    return output;
}

std::string modifyString(std::string input)
{
    std::string output;
    int traveller = 0;
    for (unsigned i = 0; i < input.length(); ++i)
    {
        output += modifyChar(input[i], traveller);
        traveller = (int)input[i];
    }
    return output;
}

#endif

Explanation:

The print statement in Brainfuck is .. The resulting Brainfuck prints the original string character-by-character, so there will be as much . as there are characters to print. Between those . there will be either + if the ASCII is higher than the previous character and - if it's lower.

The Brainfuck doesn't think. It increments, decrements or prints.

Example usage:

Input:

Hello World!

Output:

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++
++++++++++++++++++++++.+++++++..+++.--------------------------------------------
-----------------------------------.++++++++++++++++++++++++++++++++++++++++++++
+++++++++++.++++++++++++++++++++++++.+++.------.--------.-----------------------
--------------------------------------------.

Input:

FizzBuzz
Foo!Bar!

Output:

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++++
++++++++++++++++++++++++++.+++++++++++++++++..----------------------------------
----------------------.+++++++++++++++++++++++++++++++++++++++++++++++++++.+++++
..

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.+++++++++
++++++++++++++++++++++++++++++++..----------------------------------------------
--------------------------------.+++++++++++++++++++++++++++++++++.+++++++++++++
++++++++++++++++++.+++++++++++++++++.-------------------------------------------
--------------------------------------.

Points of concern:

I'm bad at naming things.
My user-input handling feels bad.
There's a cast in my code which feels like a design flaw.
I may be needlessly repeating myself in one case while not splitting responsibilities far enough in another.
It's not modular.
It may not be idiomatic C++11.
It may not be safe (although IMHO, there's not much which can go wrong).

First of all, let's not beat around the bush. Clearly your filenames should be Brainfuck.hpp and Brainfuck.cpp. Let's not be shy. Now for the real reviews:

Input

std::string currentLine = " "; // do not start empty

while (currentLine.length() != 0) // escape input by an empty line
{
    std::getline(std::cin, currentLine);
    userInput.push_back(currentLine);
}

This is bad. getline() could fail - and you'd expect it to fail when the input runs out. But you're not checking the result of getline(), so you end up writing an extra string onto your vector. The correct approach would be:

for (std::string currentLine; std::getline(std::cin, currentLine); ) {
    userInput.push_back(currentLine);
}

This ensures that you get the correct strings in your userInput.

Looping

Since this is tagged C++11, you definitely want to use a range-for expression to do your loop. It's less code and clearer:

for (const std::string& str : userInput) {
    std::cout << modifyString(str) << std::endl << std::endl;
}

Know what's in the standard

Your modifyChar() function's job is to pick some character (either + or -) and return a string with that character next times, followed by a period. You do not need a loop for this. There is a constructor for std::string that takes a count and a character and constructs a string with that character that many times.

Thus:

std::string modifyChar(char input, int carry)
{
    int next = input - carry;
    std::string output = (next > 0) ? std::string(next, '+')
                                    : std::string(-next, '-');
    return output + '.';
}

And same comment about looping again for modifyString. Though additionally you should take input by reference-to-const, to avoid a completely unnecessary copy:

std::string modifyString(std::string const& input)
{
    std::string output;
    int traveller = 0;
    for (char c : input)
    {
        output += modifyChar(c, traveller);
        traveller = c;
    }
    return output;
}


Mapping and sorting lists of Haskell tuples I'm a newbie with Haskell and I'm trying to learn the language and its idioms while working through a small data analysis project.  The first step of my analysis is to take a very long list of data, generate the set of ngrams from that list, then create a histogram from the generated set of ngrams.

This approach seems to work nicely:

import Data.List
import Control.Arrow

histogram :: Ord a => [a] -> [(a,Int)]
histogram = map (head &&& length) . group . sort

ngrams :: Eq a => Int -> [a] -> [[a]]
ngrams n xs
     | nx == xs  = [xs]
     | otherwise = [nx] ++ (ngrams n (tail xs))
     where nx = take n xs

The next step is divide each ngram frequency by the total number of ngrams and then take the inverse of each of these values.

(//) a b = fromIntegral a / fromIntegral b

mapOverSnd :: (a -> b) -> [(c,a)] -> [(c,b)]
mapOverSnd f = map (fst &&& f . snd)

...

let myData = (long list of values)
let my3grams = ngrams 3 myData
let freq3grams = histogram my3grams
let novelty3grams = mapOverSnd ((1/) . (//(length freq3grams))) freq3grams

My first question is.. is this a reasonable way to be doing this?  Is there a more idiomatic way to accomplish mapOverSnd?  I don't yet have much dexterity wielding higher order structures to accomplish my tasks.

Also, I'd like to sort the resultant structure: first by the value stored in snd, then by the value stored in fst.  I'd like the primary sort to be descending and the secondary sort to be ascending.  This works:

my2TupleOrdering :: (Ord a, Ord b) => (a,b) -> (a,b) -> Ordering
my2TupleOrdering (a1,b1) (a2,b2)
               | b1 < b2   = GT
               | b1 > b2   = LT
               | a1 < a2   = LT
               | a1 > a2   = GT
               | otherwise = EQ

however, I was wondering if there is a more idiomatic way to say "sort this 2-tuple, first by snd descending, then by fst ascending."

I realize these are relatively minor questions, but I'm interested in exploring the expressive power of Haskell and figured 'code review' might be the correct forum.  Thanks!

Mapping over second

It is surprising that you know about &&&, but not second, also from Arrows:

mapOverSnd f = map (second f)

second is of type Arrow a => a b c -> a (d, b) (d, c), which boils down to (b->c) -> (d,b) -> (d,c) here.

Alternatively, you can take advantage that pairs are a Functor instance: `fmap (+1) ("...",10) = ("...",11)

mapOverSnd f = map (fmap f)

Definitions can be infix

You wrote (//) a b =, but definitions can be infix, too:

a // b = fromIntegral a / fromIntegral b

your2TupleOrdering

2-tuples are simply called pairs. sort needs a function like compare, so define yours in terms of compare. Take advantage of Ordering being an instance of Monoid:

myPairOrdering (a1,b1) (a2,b2) = compare a1 a2 `mappend` compare b2 b1

The Monoid instance of Ordering is:

mempty = EQ
EQ `mappend` y = y -- only if left side equals, use right
x  `mappend` _ = x -- shortcut otherwise

ngrams

Any solution to ngrams will be ugly because you don't want the very last elements which are below the minimum length. You are forcing an Eq constraint, which is not necessary. Avoid by comparing the length. Another solution is

ngrams' n = filter ((==n).length) . map (take n) . tails

The filter part here is ugly.

The let-chain

You can condense the let-chain with composition, cutting out freq3grams and my3grams. At some point in you haskell journey, you will do that. When you grow wiser, you will value readability over succinctness. Long story short: This part is OK.


C++ Class Writing Interview Query Ran across this question today:

  write a class Tool which will have a function void type() that every
  derived class should implement . A function Action() that every
  derived class can override . function init() which is available to
  only Tool and variable Name which will tell which class`s instance is
  this object

Here is my solution (based on other solutions I've found):

#include <typeinfo>

  class Tool{

      public:
           string name;
           Tool() {
                 name = typeid(*this).name();
           };

           virtual void type() = 0;
           virtual void Action();

      private:
           void init();
  };

Could someone double check this please?

I would strongly recommend making name field const. Also, typeid(*this).name() won't look nice and pretty in some compilers.
You should probably try something like:

class Tool {
public:
  const std::string name;
  <...>
protected:
  Tool(const char* className) : name(className) {}
private:
  <...>
}

BTW, do NOT expect typeid(*this).name(); to be called for deriven object. Even virtual functions won't help, so you can't write something like virtual const char* getMyName() = 0;, because any kind of "virtual" behavior will not work in constructor.


Multiple form inputs droppable I have a form with 11 inputs that relate to each column on a table, which is displayed above. Each input has an id that matches the a class in the table based on the column name. I want all the inputs to be droppable, the logic for all of them is identical to below.

$(function() {
  $( "#dest_city" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.dest_city" ).each(function(){
            $( '#dest_city' )
                .val( $( '#dest_city' ).val() == "" ? $( this ).html() : $( '#dest_city' ).val() + ", " + $( this ).html() );
        })
    }
  });
});
$(function() {
  $( "#dest_state" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.dest_state" ).each(function(){
            $( '#dest_state' )
                .val( $( '#dest_state' ).val() == "" ? $( this ).html() : $( '#dest_state' ).val() + ", " + $( this ).html() );
        })
    }
  });
});
$(function() {
  $( "#car" ).droppable({
    drop: function( event, ui ) {
        $( ".ui-selected.car" ).each(function(){
            $( '#car' )
                .val( $( '#car' ).val() == "" ? $( this ).html() : $( '#car' ).val() + ", " + $( this ).html() );
        })
    }
  });
});

And the inputs -

    <input type="text" name="dest_city" id="dest_city" value="" size="7" />
    <input type="text" name="dest_state" id="dest_state" value="" size="3" />
    <input type="text" name="car" id="car" value="" size="11" />

And the data -

    AAAAAAA
    111111
    1111
    P
    FTSCOTT
    KS
    L
    AAAAA
    11111
    11
    AAAAA
    FRESNO
    CA

This checks on drop for all selected cars and inserts each of them to the input, the ternary checks if the input is empty or already has something, and adds the items to make a comma separated list.

My question is, is there any way to refactor this instead of writing the above for each input?

I am using Ruby on Rails so I can easily set up a loop to create that for each input, but that's only shorter on my end, the actual output would still have a nearly identical block of code for each input.

A .car can only be placed in #car, .dest_city in #dest_city etc...

The first thing I would do with this code is wrap the entire code in an IIFE.  This will create your own private scope and not pollute the global namespace.  Also, since you are using jQuery, you can pass in the jQuery object as $ to ensure that it is correctly mapped.

(function( $ ) {
  //your code here
})( jQuery );

Next, you have each droppable in its own document.ready call.  That is unnecessary.  You only need one call. We will add that in at the end of the code.

Since most of your code is actually in the event handler and it is almost entirely the same for each instance, we can DRY out your code. Just pass in a string that contains the one difference between each function and cache some of the variables.

function dropHandler( item ) {
  $('.ui-selected.' + item).each( function() {
    var $curr = $('#' + item), val = $curr.val(), html = $(this).html();
    $curr.val( ( val === "" ) ? html : val + ", " + html;
  });
}

With this in place, we just need a to loop over all of your items.  So add each input name into an array and loop over it creating the droppables.

var fields = ['dest_city','dest_state','car']

function init() {
  var len = fields.length;
  for( var i = 0; i < len; i++ ) {
    $('#' + fields[i] ).droppable({
      drop : function(event, ui) {
        dropHandler( this.id );
      }
    });
  }
}

Last just add a call to the init function on document.ready.

$(function(){
  init();
});

After these changes, here is the updated code:

(function( $ ){

  var fields = ['dest_city','dest_state','car'];

  function dropHandler( item ) {
    $('.ui-selected.' + item).each( function() {
      var $curr = $('#' + item), val = $curr.val(), html = $(this).html();
      $curr.val( ( val === "" ) ? html : val + ", " + html;
    });
  }

  function init() {
    var len = fields.length;
    for( var i = 0; i < len; i++ ) {
      $('#' + fields[i] ).droppable({
        drop : function(event, ui) {
          dropHandler( this.id );
        }
      });
    }
  }

  $(function(){
    init();
  });

})( jQuery );

Let me know if you have any questions or comments.


Switching between view controllers that are loaded in the background I have a dining menu app that scrapes the data from a website and redisplays it in a mobile format, displayed below:

If the user swipes left and right, the app will show the previous/next meal (ex. if current meal is lunch, swiping left will show dinner).

I'm trying to come up with a more efficient way of switching between menus.  With my current implementation, each menu is a separate view controller.

When someone swipes right, it calls:

-(void)swipeleft:(UISwipeGestureRecognizer*)gestureRecognizer {
    //swipes to next meal
    MealType curMeal = currentMenu.type;
    int newMeal = [MenuLoader MealAfterMeal:curMeal];

    MenuTableController *newMenu = [self.storyboard instantiateViewControllerWithIdentifier:@"Menu"];
    newMenu.currentMeal = newMeal;

    [self.navigationController pushViewController:newMenu animated:YES];
}

When each view controller is created (including the first), it loads the menu in a background thread:

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        MenuLoader *ml = [[MenuLoader alloc] init];
        currentMenu = [ml mealForType:_currentMeal Specificity:[self summaryPreference]];

        dispatch_async(dispatch_get_main_queue(), ^(void) {
            ...
        });
    });

Inside the mealForType:Specificity: call, I parse 2 separate web pages through NSURL, and combine the information from both sites:

NSURL *url = [NSURL URLWithString:urlString];
NSData *pageData = [NSData dataWithContentsOfURL:url];

Some of the optimizations I'm hoping to make are:

Preloading adjacent menus in another thread to reduce loading time when someone swipes
If someone swipes forward and then back, it should show the already loaded menu. Right now, it makes a fresh call to the server and reloads everything.
Right now, if someone swipes multiple times in a row before any of the menus finish loading, the app tries processing all of the server calls at once and this slows down the loading time.  If someone does swipe repeatedly, the current menu should take precedence, and possibly the other calls should be terminated.

It sounds like a big overhaul of my design, so I was wondering if there was a better approach to loading menus than what I currently implemented.

I recently created a similar layout for a project I'm working on using Apple's PageControl example as a base.

I used a UICollectionView for the top nav section and a UIScrollView for paging through view controllers. The main view controller is setup as below.

In the viewDidLoad method for the I set up the content size for the scrollView and set paging to YES.

[self.containerScrollView setContentSize:CGSizeMake(CGRectGetWidth(self.containerScrollView.frame)*self.numberOfPages,
CGRectGetHeight(self.containerScrollView.frame))];
[self.containerScrollView setPagingEnabled:YES];
[self.containerScrollView setShowsHorizontalScrollIndicator:NO];
[self.containerScrollView setShowsVerticalScrollIndicator:NO];
[self.containerScrollView setScrollsToTop:NO];
[self.containerScrollView setDelegate:self];

(Note: if you're using xibs/size classes, I ran into a problem where the scrollView's size is stated as 600x600px no matter which device is used until the ParentViewController's viewDidLayoutSubviews is called. I had to reset the scroll view's content size again.)

After this is set up, I load the first two view controllers:

- (void)loadScrollViewWithPage:(NSUInteger)page {

    UIViewController *viewController = <get your view controller>;

    if (viewController.view.superview == nil) {
        [self addChildViewController:viewController];
        CGRect frame = [self.containerScrollView frame];
        frame.origin.x = CGRectGetWidth(frame)*page;
        frame.origin.y = 0;
        [viewController.view setFrame:frame];

        [self.containerScrollView addSubview:viewController.view];
        [viewController didMoveToParentViewController:self];
    }
}

Since the first two view controllers are loaded, there is no lag when the user swipes between them. When UIScrollView is scrolled, the page is scrolls to is already loaded. Once it does, I load the next one in order:

- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView
{
    CGFloat pageWidth = CGRectGetWidth(self.scrollView.frame);
    NSUInteger page = floor((self.scrollView.contentOffset.x - pageWidth / 2) / pageWidth) + 1;

    [self loadScrollViewWithPage:page - 1];
    [self loadScrollViewWithPage:page];
    [self loadScrollViewWithPage:page + 1];
}

This way you always have the current, previous and the next view controller always ready. You could load more than 3 if the network calls you have to make take longer and the what the user is expected to scroll at.

You could unload older view controllers by calling viewController.view = nil and always only have a set number of view controllers loaded.

I created a custom protocol for the UICollectionView and set my parent controller as delegate to respond to cell clicks and navigate to the appropriate page.


Converting a boolean list to string, and vice versa I am currently using this to convert a List of booleans to a string

    var tempbllist = new List<int>();
        foreach (var t in blenabled)
            switch (t)
            {
                case true:
                    tempbllist.Add(1);
                    break;
                case false:
                    tempbllist.Add(0);
                    break;
            }

        Settings.Default.blEnabled = String.Join(",",
            tempbllist.Select(i => i.ToString(CultureInfo.InvariantCulture)).ToList());

(converts them to 1s and 0s to save space)

and this to convert them back

if (Settings.Default.blEnabled.Contains(","))
            blenabled =
                new List<bool>(
                    Settings.Default.blEnabled.Split(',').Select(Int32.Parse).Select(i => i == 1).ToList());

What is a better way of doing this?

How about this:

To Save:

Settings.Default.blEnabled = string.Join(",", blenabled.Select(x=>x?"1":"0"));

To Load:

if(!string.IsNullOrWhitespace(Settings.Default.blEnabled))
  blenabled = Settings.Default.blEnabled
    .Split(',')
    .Select(x=>Convert.ToBoolean(int.Parse(x)))
    .ToList();

Is there some requirement that you save them as integers?  It could be further simplified by just using bool.ToString() and bool.Parse()...


PDO Database wrapper The subject is this small project - PHP/MySQL Database wrapper

The three main goals of this projects are:

Easy way to map database columns to object properties
Simple solution to insert or update data
Query builder class which generate sql query (something like ActiveRecord in CodeIgniter)

Some examples:

class User extends stdClass {
    function getName() {
        return $this->first_name . " ". $this->last_name;
    }
}

class Post extends stdClass {

    /**
     * @var User
     */
    public $author;
}

DB::getInstance()->setFetchTableNames(1);
$sql = DB::getInstance()
    ->select("p.*, u.*")
    ->from("posts p")
    ->join("INNER JOIN users u USING(user_id)")
    ->where("u.user_id = ?", $user_id)
    ->orderBy("p.title");

$stmt = $sql->execute();

/* @var Post[] $post_collection  */
$post_collection = array();

while($post = $stmt->fetchInto(new Post, "p")) {
    $post->author = $stmt->fetchIntoFromLastRow(new User, "u");
    $post_collection[] = $post;
}

// Usage
foreach($post_collection as $post) {
    echo $post->author->getName();
}

More examples are available in readme file at GitHub - https://github.com/salebab/database

So, please review this classes and I will be very pleased if you can give me some comment.

Firstly, a bit of humorous wordplay: witch = itch = burn; "She's a witch! BURN HER!" I think you meant "which" in this context. Now that is out of my system, on to the review :)

Why are you extending the stdClass? This is the class reserved for typecasting variables as classes. It is just a wrapper that has no benefits of being extended from. If you are just trying to create a normal class then just drop the extends stdClass bit. Its unnecessary.

Additionally, where did properties $first_name and $last_name come from? They were not declared. This means they either are coming from the parent class, which is impossible as the stdClass is empty, or they are defaulting to the public scope. If you were extending another class and that property was defined there, this would be fine, but default properties are bad. Besides, I have heard rumors that it will be deprecated soon, so prepare for the worst and just manually define their scope.

class User {
    public
        $first_name,
        $last_name
    ;
    //etc...
}

What is the point of the Post class? Just to declare the $author property? This is unnecessary, especially for a full fledged class. The only time I could foresee something like this being ok is if it were part of an interface or abstract class, but then it should also declare the expected methods as well.

Why is your database class completely static? I understand adopting a singleton pattern to ensure only one connection can be made, but after that you should be able to use the rest of your class like normal. Instead you are calling it statically everywhere. This is what you should be able to do with the singleton pattern. Notice how you only have to get the instance once.

$db = DB::getInstance();
$db->setFetchTableNames( 1 );
$sql = $db->select(//etc...

Why are you looping over the same information twice? If you find yourself looping over something in order to create an array so that you can loop over it again, don't. Just do it all the first time.

while( $post = $stmt->fetchInto( new Post(), 'p' ) ) {
    $post->author = $stmt->fetchIntoFromLastRow( new User(), 'u' );
    echo $post->author->getName();
}

Now, I can't really help you much more because you've not posted all of your code, just examples. I can't review your class except in the abstract. If you want a more complete review, you should follow the FAQ and post all related code. For instance, you are asking about your database wrapper, but all you have posted are a couple of slightly related container classes and example usages. Where's the actually database wrapper?


Enum with getting subset I just wrote this but I'm not sure if this is good practice.  Can someone give me advice as to whether it's good or how I can do better or even it's bad code?

The point is that I need a sub List-array-set of the enum.  The SubType.ELSE shall have another name in reality (I know that was a bad choice, but it's for pointing it out).

public enum ContentType {
    TITLE("$$title$$",SubType.MAIL),
    BODY("$$body$$",SubType.MAIL),
    MESSAGE("$$message$$",SubType.MAIL),
    EVENTS("$$events$$",SubType.ELSE);

    private final String replaceWord;
    private final SubType subType;

    private ContentType(String replaceWord, SubType subType ) {
        this.replaceWord = replaceWord;
        this.subType = subType;
    }

    public String getReplaceWord() {
        return replaceWord;
    }

    private SubType getSubType() {
        return subType;
    }

    public static List<ContentType> values(SubType subType) {
        List<ContentType> subList = new ArrayList<ContentType>();
        for (ContentType type : ContentType.values()) {
            if (type.getSubType() == subType) {
                subList.add(type);
            }
        }
        return subList;
    }
}

enum SubType {
    MAIL,
    ELSE;
}

I suggest two changes to values(SubType):

Return a Set instead of a List.
Prepare all the possible results at class-loading time, so that values(SubType) could be accomplished by a simple lookup.  The code is slightly more complicated, but the runtime performance should be better.

private static final Map<SubType, Set<ContentType>> BY_SUBTYPE;
static {
    BY_SUBTYPE = new HashMap<SubType, Set<ContentType>>();
    for (SubType subType : SubType.values()) {
        BY_SUBTYPE.put(subType, new TreeSet<ContentType>());
    }
    for (ContentType type : ContentType.values()) {
        BY_SUBTYPE.get(type.getSubType()).add(type);
    }
    for (SubType subType : BY_SUBTYPE.keySet()) {
        // Make Set<ContentType> values immutable
        BY_SUBTYPE.put(subType, Collections.unmodifiableSet(BY_SUBTYPE.get(subType)));
    }
}

public static Set<ContentType> values(SubType subType) {
    // Returns an unmodifiable view of a SortedSet
    return BY_SUBTYPE.get(subType);
}


Writing a general purpose "split" function (for SICP's imaginary language) From SICP 2.2.4:

The textbook has already defined a function (right-split ...) as follows:

(define (right-split painter n)
  (if (= n 0)
      painter
      (let ((smaller (right-split painter (- n 1))))
        (beside painter (below smaller smaller)))))

and they have indicated that there exists a procedure (up-split ...) which has much the same structure.

  Exercise 2.45.  Right-split and
  up-split can be expressed as instances
  of a general splitting operation.
  Define a procedure split with the
  property that evaluating

(define right-split (split beside below))
(define up-split (split below beside))

  produces procedures right-split and
  up-split with the same behaviors as
  the ones already defined.

I wrote the following function, but I'm not sure the best way to test it.  What do you think?

(define (split step1 step2)
  (define (split-f painter n)
    (if (= n 0)
        painter
        (let ((smaller (split-f painter (- n 1))))
          (step2 (step1 smaller smaller)))))
  split-f)

(define right-split (split beside below))
(define up-split (split below beside))

In order to test it, you need to have definitions for things like beside and below. There's something strange in the text book code because the painter "smaller" is not actually scaled in the code, or is it indicated that "below" and "beside" scale their arguments? Anyway, you can just define

  (define (below a b) `(below ,a, b))
  (define (beside a b) `(beside ,a ,b))

so e.g. (right-split 'apple 1) should produce as result

  '(beside apple (below apple apple))

You can verify from the structure of the terms that your code works correctly.

Your (split ...) function has a bug, as "painter" is missing from the line where you call "step2". The code should read

  (step2 painter (step1 smaller smaller))

Otherwise it looks fine to me.


C String - new function to detect user's anger This was the most humerus coding I've ever done. It's for my string library in C. It detects if the user is angry to various degrees, namely str_isHeated().

Why?

Ever play a text-based game and you're swearing at the computer by typing, typing multiple !!!, and the computer responds very dumb? I think it may be useful for AI where NPC's (non-playable characters) can judge your mood and respond appropriately. Maybe even used for customer service online.

It works, but I'm interested to see if anyone has any thoughts on how to improve it. I've been having some fun with it.

/*
Function: str_getHeat()
Software usually gets user information, but it hardly
detects the user's emotion when entering in the information.
This may be useful for checking a customer's or player's
typing behavior, which may generate better responses with AI.
Calculated as follows:
    All Caps
    One or more words in caps
    Exclamation Point Count
    If 'please' or 'sorry' is found, take off heat points.
    Swearing words
Returns: EHeat_Cold, EHeat_Warm, EHeat_Heated, EHeat_VeryHeated
*/
EHeat str_isHeated(STRING *objString)
{
int i;
int intHeatScore = 0;       /* 0% cold; 100% very heated */
STRINGCOLLECTION tokens;
STRING temp_a;

/* Count how many exclamations there are */
for (i = 0; i < objString->length; i++)
{
    if (objString->str[i] == '!')
        intHeatScore += 10;
}

/* tokenize user's input */
sc_init(&tokens);
str_tokenize(objString, &tokens);

    /* Check if all caps. That can be taken as impatient. */
if (str_isUpper(objString))
{
    intHeatScore += 10;
}
else
{
    /* check if one or more words are all in caps. That is
       demanding behavior, and that is not nice. */
    for (i = 0; i < tokens.count; i++)
    {
        if (str_isUpperCString(tokens.items[i]))
        {
            intHeatScore += 10;

            /* 'I' is excused. */
            if (!strcmp(tokens.items[i], "I"))
                intHeatScore -= 10;
        }
    }
}

/* Check if the user said please. That's always nice.
   Take off a few heat points. */
if (str_findStringIgnoreCaps(objString, "please"))
    intHeatScore -= 6;

/* Check if the user said he's sorry. That's also nice. */
if (str_findStringIgnoreCaps(objString, "sorry"))
    intHeatScore -= 6;

/* Almost forgot... swearing. That is never nice. */
for (i = 0; i < tokens.count; i++)
{
    str_setText(&temp_a, tokens.items[i]);
    str_toLower(&temp_a);

    /* don't say these words out loud (censored for your innocence*/
    if (str_findString(&temp_a, "$#@#") ||
        str_findString(&temp_a, "@#$@") ||
        str_findString(&temp_a, "@$$") ||
   str_findString(&temp_a, "@$$#@") ||
        str_findString(&temp_a, "%#@") ||
        str_findString(&temp_a, "@#$")
        )
    {
        /* big no-no */
        intHeatScore += 20;
    }
}

/* Check the final heat score */
if (intHeatScore >= 50)
    return EHeat_VeryHeated;

else if (intHeatScore >= 30)
    return EHeat_Heated;

else if (intHeatScore > 10)
    return EHeat_Warm;

else if (intHeatScore >= 0)
    return EHeat_Cold;

return EHeat_Cold;
}

Algorithm:

There are a few fallacies that you assume in your approach.  Nothing wrong, but things that could be improved:

You assume bounds on your "heat score" with this comment

/* 0% cold; 100% very heated */

But there is no code implementing these bounds.  You can go negative, as well as go above 100.  I'd recommend observing these bounds, and sentiment should be seen as a probability which can only be between 0 and 1.

To match this probability, it might be better for you to store your sentiment score as a double rather than an int but that choice is up to you.
Right now you are using a bag-of-words model.  This is a typical approach when first starting out with sentiment analysis since it is easier, but it usually gives a lower accuracy representing what the actual sentiment of the text is.

As I was saying, it's a fairly straightforward and practical way to go, but there are a lot of situations where it will get make mistakes.

Ambiguous sentiment words - "This product works terribly" vs. "This product is terribly good"
Missed negations - "I would never in a millions years say that this product is worth buying"
Quoted/Indirect text - "My dad says this product is terrible, but I disagree"
Comparisons - "This product is about as useful as a hole in the head"
Anything subtle - "This product is ugly, slow and uninspiring, but it's the only thing on the market that does the job"

As far as NLP helping you with any of this, word sense disambiguation (or even just part-of-speech tagging) may help with (1), syntactic parsing might help with the long range dependencies in (2), some kind of chunking might help with (3). It's all research level work though, there's nothing that I know of that you can directly use. Issues (4) and (5) are a lot harder, I throw up my hands and give up at this point.
You are going to not score a decent amount of sentences that occur in actual life.  Take a look at a lot of sentences in this post, for example.  They don't contain swear words, "please" or "sorry", exclamation points or words in caps.  You need a more general lexicon of positive, negative, and neutral words and then a system to weigh in the effects of these words into your score.
There are some odd score modifications you do.  Why is it when I refer to myself with "I", that the score is considered more positive?  I don't think it should, and I would say to reconsider the reason you think so.

Sentences that end with an exclamation point aren't necessary negative (higher heat) either.  An exclamation point is often used to indicate strong feelings (such as excitement) or high volume.  Most sentiment analysis systems that I've looked at don't consider punctuation in the final score at all.

"A" should be considered the same as "I".  It is very possible that a sentence could start with "A", and be either positive or neutral but your program considers it to have a negative connotation.

For a basic sentiment analysis this is fine, but do note that it does have it's flaws.  If you're looking to improve the accuracy of your algorithm, I'd recommend reading this research paper, which achieves a classification accuracy of 90% (higher than any other published results).

Code:

Right now you have the method str_findString().  I'm guessing this is an variation of strstr(). I'm also guessing that <string.h>'s implementation of this method will be more efficient and faster, based on it being a standard library.

if (strstr(temp_a, "$#@#"))
{
    ...
}

Declare i inside of your for loops.(C99)

for (int i = 0; i < objString->length; i++)

I would add another tab to the function body.

EHeat str_isHeated(STRING *objString)
{
    int intHeatScore = 0;       /* 0% cold; 100% very heated */
    STRINGCOLLECTION tokens;

I would combine your last two return conditions into one.

else if (intHeatScore >= 0)
    return EHeat_Cold;

return EHeat_Cold;

I find the last else-if comparison useless overall, and would just return EHeat_Cold anyways if it wasn't included.

return EHeat_Cold;

I'm not too sure about what I'm assuming are #defines: STRING and STRINGCOLLECTION.  I guess it is okay to keep them, but is there a specific reason you don't just put in what they actually are: char* and char* array respectively?


Calculating a Luhn Check Digit I was able to find all kinds of examples that demonstrate how to validate a Luhn Checksum but very few that actually generated the check digit. Here is my attempt in base 10, based on the validation examples found at Wikipedia and Rosetta Code:

/// <summary>
/// Constructs the check digit for a given numeric string using the Luhn algorithm; also known as the "mod 10" algorithm.
/// </summary>
/// <param name="value">The string that a check digit will be calculated from.</param>
public static int Mod10(string value) {
    const int validCodePointCount = 10;
    const int validCodePointOffset = 48;

    var index = value.Length;
    var parity = true;
    var sum = 0;

    while (0 < index--) {
        var digit = (value[index] - validCodePointOffset);

        if (parity) {
            if ((validCodePointCount - 1) < (digit <<= 1)) {
                digit -= (validCodePointCount - 1);
            }
        }

        parity = !parity;
        sum += digit;
    }

    return ((validCodePointCount - (sum %= validCodePointCount)) % validCodePointCount);
}

All in all it seems OK to me. A couple of things though:

1)

  const int validCodePointCount = 10;
  const int validCodePointOffset = 48;

The names of these constants are hard to distinguish from each other and to understand in the context and they blur the behavior of the algorithm.

Better names could be:

const int numberBase = 10;
const int zero = '0';

Here I use '0' instead of 48 because the first is much more recognizable to programmers than the latter, and we instantly grasp the meaning. In fact in such a relatively small algorithm, I would prefer to use the literals directly in the code than the named constants like:

var digit = (value[index] - '0');

This is such a common operation and we all know what it's about immediately when seeing it.

In general it is wise to avoid magic numbers, but in some situations it is preferable in order to improve readability - IMO.

2)

You don't have to iterate backwards because you can determine the initial parity from the length of the input string:

var parity = value.Length % 2 != 0;

You can then iterate forward:

foreach (int n in value.Select(c => c - '0'))
{
  ...
  parity = !parity;
}


Parsing key-value pairs to fill a struct I must read and extract some values from string. These values are coded like this:

k="11,3,1" v="140.3"

I have defined the codes and created struct with all field as well as a temp one where I store k and v. In fillFields proc I transfer values from temp struct to the right one (with the valid types). It works but I have many fields and fillFields would need to have many if-conditions. Maybe someone could give me any hint how to write it smarter.

The simplified code now:

   #define ASK                       "11,3,1"
   #define BID                       "11,2,1"
   #define CLOSE                     "3,1,1"

    typedef struct tic {

      float ask;
      float bid;
      float close;
    }tic, *ticP;
    typedef struct pElem {
      char       * k;
      char       * v;
    }pElem, *pElemP;

    void fillFields(ticP t, pElemP p)
    {
      if (strcmp( ASK, p->k)==0)
      {
        printf ("ASK %s\n", p->v);
        t->ask = atof(p->v);
      }
      if (strcmp( BID, p->k)==0)
      {
        printf ("BID %s\n", p->v);
        t->bid = atof(p->v);
      }
      if (strcmp( CLOSE, p->k)==0)
      {
        printf("CLOSE >>>%s<<<\n", p->v)    ;
        t->close =  atof (p->v);
      }
    }

I like to go data driven, so I can write a single loop for processing. First, let's factor out the three if blocks that are so similar into a helper function:

void fillField(pElemP p, char* match, char* message, float* pf)
{
    if (strcmp(match, p->k)==0)
    {
        printf(message, p->v);
        *pf = atof(p->v);
    }
}
void fillFields(ticP t, pElemP p)
{
    fillField(p, ASK, "ASK %s\n", &t->ask);
    fillField(p, BID, "BID %s\n", &t->bid);
    fillField(p, CLOSE, "CLOSE >>>%s<<<\n", &t->close);
}

Now that we have this split out, we're repeating a lot less code, and have identified what information we'd need in order to make this data driven. I'm not sure going pure data driven is much better in this case, but let's look at one way it might be done:

struct FieldSpec {
    char *match;
    char *message;
    int offset;
};

struct FieldSpec field_specs[] = {
    { ASK, "ASK %s\n", offsetof(tic, ask) },
    { BID, "BID %s\n", offsetof(tic, bid) },
    { CLOSE, "CLOSE >>>%s<<<\n", offsetof(tic, close) },
};

void fillFields(ticP t, pElemP p)
{
    for (int i = 0; i < sizeof(field_specs) / sizeof(*field_specs); ++i)
    {
        fillFieldBySpec(field_specs[i], p, t);
    }
}

I'll leave the implementation of fillFieldBySpec up to you if you want to go this route (feel free to put it inside the for loop instead of its own function). I don't think it helps significantly over the first refactoring, and the gynmastics to use the value from offsetof aren't pretty.


Dynamic memory management for a class hierarchy of geometric shapes The task was to write function that compare areas of two random generated geometric shapes (circle, square, rectangle) using base class with virtual function.

Am I doing it right in terms of memory management?
Will my figures generated inside generateRandomShape() by new Square() etc. last in memory outside this function untill I manually delete them?

#include <iostream>
#include <cstdlib>
#include <ctime>
#define _USE_MATH_DEFINES
#include <cmath>
#include <vector>

#define M_PI       3.14159265358979323846

class Shape {
public:
    virtual float area() = 0;
};

class Square : public Shape {
protected:
    float sideA;

public:
    Square(float sideA) : sideA(sideA) {}
    float area() { return sideA * sideA; }
};

class Circle : public Shape {
protected:
    float radius;

public:
    Circle(float radius) : radius(radius) {}
    float area() { return M_PI * radius * radius; }
};

class Rectangle : public Shape {
protected:
    float sideA, sideB;

public:
    Rectangle(float sideA, float sideB) : sideA(sideA), sideB(sideB) {}
    float area() { return sideA * sideB; }
};

void compareAreasOffigures(Shape* a, Shape* b){
    if (a->area() == b->area()) {
        std::cout << "Areas of two shapes are equal." << std::endl;
    }
    else if (a->area() > b->area()) {
        std::cout << "Area of first Shape is bigger." << std::endl;
    }
    else {
        std::cout << "Area of second Shape is bigger." << std::endl;
    }
}

Shape * generateRandomShape() {
    int Shape;
    Shape = rand() % 3;

    switch (Shape)  {

        case 0: {
            int sideA = rand() % 100;
            std::cout << "Square was generated, side: " << sideA << std::endl;
            return new Square(sideA);
            break;
        }

        case 1: {
            int sideA = rand() % 100;
            int sideB = rand() % 100;
            std::cout << "Rectangle was generated, side A: " << sideA << " side B: " << sideB << std::endl;
            return new Rectangle(sideA, sideB);
            break;
        }
        case 2: {
            float radius = rand() % 100;
            std::cout << "Circle was generated, radius: " << radius << std::endl;
            return new Circle(radius);
            break;
        }
        default: return NULL;
    }
}

int main(void){

    srand(time(0));

    std::vector<Shape*> shapes;

    shapes.push_back(generateRandomShape());
    shapes.push_back(generateRandomShape());

    compareAreasOffigures(shapes[0], shapes[1]);

    for (auto v : shapes) { delete v; } //cleanup
    shapes.clear();

    return EXIT_SUCCESS;
}

Dynamic Memory Management

  Am I doing it right in terms of memory menagment?

You should rather use smart pointers to store the Shape instances in the vector:

 std::vector<std::unique_ptr<Shape>> shapes;

and

 std::unique_ptr<Shape> generateRandomShape() {
     int Shape; // Take care! Some compilers don't like it if variables
                // arenamed the same as defined types
     Shape = rand() % 3;

         switch (Shape)  {
         case 0: {
             int sideA = rand() % 100;
             std::cout << "Square was generated, side: " << sideA << std::endl;
             return std::make_unique<Square>(sideA);
         } break; // Put the break out side the case scope block
         case 1: {
             int sideA = rand() % 100;
             int sideB = rand() % 100;
             std::cout << "Rectangle was generated, side A: " << sideA << " side B: " << sideB << std::endl;
             return std::make_unique<Rectangle>(sideA, sideB);
         } break;
         // ...
     }
     // No need for the default case
     return std::unique_ptr<Shape>();
}

thus you can completetly omit

 for (auto v : shapes) { delete v; } //cleanup

  Will my figures generated inside generateRandomShape() by new Square() etc. last in memory outside this function untill I manually delete them?

Yes, but handling new and delete on your own turns out to be error prone and semantically unclear.

Use virtual destructors for dynamic polymorphic inheritance

For formal correctness you should declare virtual destructors for your abstract base class and the derived classes

class Shape {
public:
    float area() = 0;
    virtual ~Shape() = default; // <<<<<<<<<<<<<<<<<<<<
};

This isn't done automatically for compiler generated destructors (see an explanation here).

Use const class members whenever appropriate

Use const whenever appropriate. The area() calculation doesn't change any of the internal states of a Shape instance, so it should be declared as a const member function:

class Shape {
public:
    float area() const = 0;
              // ^^^^^
};

This improves your classes to be used in wider contexts, e.g. with temporary class instances.

Also you could rewrite your function signature

void compareAreasOffigures(Shape* a, Shape* b);

like that then1)

void compareAreasOffigures(const Shape* a, const Shape* b);

or even better

void compareAreasOffigures(const Shape& a, const Shape& b);

Note: Do that as soon as possible in your class designs, adding later is much harder than removing that constraint in a later refactoring or override.

Prefer const variable definitions over using preprocessor macros

Instead of

 #define M_PI       3.14159265358979323846

rather use

const double M_PI = 3.14159265358979323846;

or

constexpr double M_PI = 3.14159265358979323846;

for sake of type safety and clarity.

Don't use srand(), rand() with modern c++

Use a better random number generator than

srand(time(0));

// ...

rand();

There are various random generators available with the current standard in the numerics library.

Don't use == to compare floating point values

Don't use equality comparison for floating point types like

if (a->area() == b->area())

it's unlikely these will be exactly equal.

Rather test against std::numeric_limits::epsilon, there's a comprehensive example in the linked documentation.

Your purpose of using _USE_MATH_DEFINES is unclear and inconsistent

Your use of _USE_MATH_DEFINES and defining M_PI yourself is inconsistent.

The purpose of #define _USE_MATH_DEFINES is to use the predefined constants from cmath or math.h, but you define M_PI yourself anyways.

As mentioned here it should be defined before any of the #include statements to guarantee it works

#define _USE_MATH_DEFINES
#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
#include <vector>

since you cannot know if any of the other header files already includes cmath or math.h.

The highlight of your solution

After so many points of critique, I want to emphasize one bonus point of your solution:

You weren't trapped by the Square is not a Rectangle confusion and provided unrelated classes for these (same for Circle is not an Ellipse).

Beginners often use inheritance like

class Rectangle {
protected:
    float sideA, sideB;
};

class Square : public Rectangle {
};

and run into trouble to keep the class member variables consistent (here's a more detailed resource).

1As mentioned by @Tamoghna Chowdhury before.


Write a recursive function that takes a positive integer n and plots an N-by-N Hadamard pattern where N = 2^n This is web exercise 2.3.20. from the book Computer Science An Interdisciplinary Approach by Sedgewick & Wayne:

Write a recursive program  that takes a command-line argument n and
plots an N-by-N Hadamard pattern where N = 2^n. Do not use an array. A
1-by-1 Hadamard pattern is a single black square. In general a
2N-by-2N Hadamard pattern is obtained by aligning 4 copies of the
N-by-N pattern in the form of a 2-by-2 grid, and then inverting the
colors of all the squares in the lower right N-by-N copy. The N-by-N
Hadamard H(N) matrix is a boolean matrix with the remarkable property
that any two rows differ in exactly N/2 bits. Here are the first few
Hadamard matrices.

Here is my program:
public class wexercise2_3_20
{
    public static void hadamard(int n, double x, double y, double size,
                                boolean color)
    {
        double x1 = x - size/2, x2 = x + size/2;
        double y1 = y - size/2, y2 = y + size/2;
        if (n == 0) return;
        if (color)
        {
            StdDraw.setPenColor(StdDraw.BLACK);
            StdDraw.filledSquare(x,y,size);
        }
        else
        {
            StdDraw.setPenColor(StdDraw.WHITE);
            StdDraw.filledSquare(x,y,size);
        }
        hadamard(n-1, x1, y1, size/2, color);
        hadamard(n-1, x1, y2, size/2, color);
        hadamard(n-1, x2, y2, size/2, color);
        hadamard(n-1, x2, y1, size/2, !color);

    }
    public static void main(String[] args)
    {
        int n = Integer.parseInt(args[0]);
        hadamard(n, 0.5, 0.5, 0.5, true);
    }
}

StdDraw is a simple API written by the authors of the book.
Is there any way that I can improve my program?
Thanks for your attention.

Class names in Java should start with a capital letter.
Avoid initializing two variables in the same line.
The filledSquare call is the same in the two if branches, it could be moved out of the if.  Now the two if branches only change in the argument to the setPenColor call, and you could replace it by
Color theColor = color ? WHITE : BLACK;

Now it seems like you could drop the boolean argument altogether (boolean arguments suck) and pass a Color; all you need is a function that inverts the color.
This criticism has nothing to do with recursion; I have no idea if this works correctly or not :)


Thread Safe Circular Queue Implementation #pragma once

#include <atomic>
#include <memory>

template <typename T> class RingBuffer {
public:

    explicit RingBuffer(std::size_t capactiy) : _buffer_capacity(capactiy){
        std::unique_ptr <T[], RingBufferFree> buffer_ptr(new T[capactiy]);
        _ring_buffer_array = std::move(buffer_ptr);
    }

    ~RingBuffer() {

    }

    void Push(T val) {
        while (!TryPush(val));
    }

    T Pop() {
        T val;
        while (!TryPop(&val));
        return val;
    }

private:

    //Private Struct to ensure allocated memory is freed.
    struct RingBufferFree{
        void operator()(T * ring_buffer_memory) { delete[] ring_buffer_memory; }
    };

    //Private Member Functions
    bool TryPush(T & val) {
        const std::size_t current_write = write_position.load(std::memory_order_acquire);
        const std::size_t current_read = read_position.load(std::memory_order_acquire);
        const std::size_t next_write = increment_index(current_write);

        if (next_write == current_read) { return false; }

        _ring_buffer_array[current_write] = val;
        write_position.store(next_write, std::memory_order_release);

        return true;
    }

    bool TryPop(T * val) {
        const std::size_t current_write = write_position.load(std::memory_order_acquire);
        const std::size_t current_read = read_position.load(std::memory_order_acquire);

        if (current_read == current_write) { return false; }

        *val = _ring_buffer_array[current_read];

        const std::size_t next_read = increment_index(current_read);
        read_position.store(next_read, std::memory_order_release);

        return true;
    }

    std::size_t increment_index(std::size_t index) {
        return (index + 1) % _buffer_capacity;
    }

    //Private Member Variables
    std::atomic<std::size_t> read_position = 0;
    std::atomic<std::size_t> write_position = 0;

    std::size_t _buffer_capacity;
    std::unique_ptr<T[], RingBufferFree> _ring_buffer_array;
};

I have tested my code and it appears to work, but I would like some second opinions on what I did. This first time I have tried lock free programming with atomic variables and so I would appreciate it if I had more experienced programmers provide feed back.

First some style notes:

capactiy should be capacity. Spelling is important in all kinds of writing, because all writing is primarily communication, intended for humans just as much as for computers.
The entire RingBufferFree widget is unnecessary. std::unique_ptr<T[]> will do the right thing by default.
Temporary variables are often a code smell, if they're not serving an explanatory purpose. In the constructor, buffer_ptr is unnecessary.

explicit RingBuffer(std::size_t capacity) :
    _buffer_capacity(capacity),
    _ring_buffer_array(new T[capacity])
{}

Personally, I'd never write std::size_t, since size_t works just as well; but your mileage may vary.
~RingBuffer() {} is unnecessary; remove it.
It's unclear to me why you're using a std::unique_ptr<T[]> at all, when a std::vector<T> would work just as well.
It's odd that TryPush takes T& val, instead of const T& val or (better) T val or (perhaps best in this context) T&& val — where you move-assign either of the last two into place with _ring_buffer_array[current_write] = std::move(val);.
Likewise, in TryPop you probably ought to be moving-from the buffer element: *val = std::move(_ring_buffer_array[current_read]);

Okay, let's go find some concurrency bugs!

Obviously you'll have problems if two different threads try to call Push at the same time (because one will overwrite the other's pushed item); and you'll have problems if two different threads try to call Pop at the same time (because both could pop the same item).

So let's assume that this is just supposed to be a single-producer single-consumer queue, and look for bugs in that scenario.

Actually, assuming it's only supposed to be single-producer single-consumer, it's pretty clean! The only potential issue I see is that if the user calls Q.Pop() on an empty queue, or Q.Push(x) on a full one, then the thread in question will busy-loop forever — or at least until some other thread manages to break in and get some actual work done. It would be significantly friendlier if you put something in the while loop's body, e.g. std::this_thread::yield() — or even better, just return false or throw QueueEmptyException() or something like that, so that the calling code can decide what it wants to do, instead of getting blocked for an indefinite amount of time.

In other words, TryPush and TryPop smell like the real primitive operations here; Push and Pop are non-primitive operations of dubious utility.


Acceptable use of typedef in class? Is this an acceptable use of typedef in the class, Inventory?

Inventory.h

#ifndef INVENTORY_H
#define INVENTORY_H

#include "Item.h"
#include <unordered_map>

class Inventory
{
public:
    typedef std::unordered_map<Item, int> ItemTable; ///< Items and their quantities

    const ItemTable& items() const;
    void add_item(Item item, int quantity);
    int quantity(Item item);
    bool remove_item(Item item, int quantity);

private:
    ItemTable items_;
};

#endif

Inventory.cpp

#include "Inventory.h"
#include "Item.h"
#include <cstddef>

const Inventory::ItemTable& Inventory::items() const
{
    return items_;
}

void Inventory::add_item(Item item, int quantity)
{
    int total_of_item = items_[item];
    items_[item] = total_of_item + quantity;
}

int Inventory::quantity(Item item)
{
    return items_[item];
}

bool Inventory::remove_item(Item item, int quantity)
{
    int total_of_item = items_[item];

    if (quantity > total_of_item) {
        return false;
    }

    items_[item] = total_of_item - quantity;
    return true;
}

Yes. I like it because it will make maintenance easier.
If you change the type of the container then you only need to change it in one place in the code. I would even go and define other things in relation to the typedef.
// For example:
typedef ItemTable::iterator         Iter;
typedef ItemTable::const_iterator   CIter;

But I would not expose the internal object with this:
const Inventory::ItemTable& Inventory::items() const

Any object that uses this method not becomes very tightly bound to this interface. I would question the need for this functionality. I prefer to expose methods that manipulate the object as a whole.
An alternative is to expose the iterators. Iterators are a much looser coupling.
As a result I would make ItemTable a private type. While the Iter and CIter would be public.
Edit:
class X
{
    typedef std::vector<int>            Cont;
    Cont        data;

    public:
        typedef Cont::iterator          Iter;
        typedef Cont::const_iterator    CIter;

        Iter begin()        { return data.begin();}
        Iter end()          { return data.end();}

        CIter begin() const { return data.begin();}
        CIter end()   const { return data.end();}
};


Working Craps program How can I make this better?

import random

def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6) + random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return dice

def checkRoll(dice):
    if dice == 7 or dice == 11:
        print("You win")
        playAgain()
    elif dice == 2 or dice == 3 or dice == 12:
        print("You lose")
        playAgain()
    else:
        print("Time to try to roll the point")
        reRoll(dice)

def reRoll(point):
    dice = roll()
    if dice == point:
        print("You win")
        playAgain()
    elif dice == 7:
        print("You lose")
        playAgain()
    else:
        reRoll(point)
def play():
    print("Do you wanna play Craps? Y/n ")
    response = input(">>> ").lower()
    if response == "y":
        checkRoll(roll())
    else:
        input("ENTER to quit")

def playAgain():
    response = input("Play Again? Y/n >>>").lower()
    if response == "y":
        checkRoll(roll())
    else:
        input("ENTER to quit")

play()

PEP8

1.Python uses underscore as naming separator in function and variable names, see PEP8

2.Two blank lines are used to separate function/classes

Improvements.

To make your module reusable you should use if __name__ == '__main__' condtion before invoking your main function.

So instead of calling play() you should do:

if __name__ == '__main__':
    play()

Your play_again and play function got difference only in print statements, so you can just combine them into a single function.

def start_game(first_run=False):
    if first_run:
        print("Do you wanna play Craps? Y/n ")
        response = input(">>> ").lower()
    else:
        response = input("Play Again? Y/n >>>").lower()
    if response == "y":
        check_roll(roll())
    else:
        input("ENTER to quit")

Note

I've renamed function into start_game since I find play a bit confusing. But you can keep your name if you want.

Now if you look at your check_roll and reroll function they are also the same except for win and lose conditions, so you can also combine them into a single one like this:

def check_roll(dice, win_conditions=(7, 11), lose_conditions=(2, 3, 12)):
    if dice in win_conditions:
        print("You win")
        start_game()
    elif dice in lose_conditions:
        print("You lose")
        start_game()
    else:
        print("Time to try to roll the point")
        check_roll(roll(), (dice,), (7,))

Also, I did small improvement to roll() function so now you can see values of dice you've rolled:

def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6), random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return sum(dice)

So in the end, what we have is this:

import random

def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6), random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return sum(dice)

def check_roll(dice, win_conditions=(7, 11), lose_conditions=(2, 3, 12)):
    if dice in win_conditions:
        print("You win")
        start_game()
    elif dice in lose_conditions:
        print("You lose")
        start_game()
    else:
        print("Time to try to roll the point")
        check_roll(roll(), (dice,), (7,))

def start_game(first_run=False):
    if first_run:
        print("Do you wanna play Craps? Y/n ")
        response = input(">>> ")
    else:
        response = input("Play Again? Y/n >>>")
    if response.lower() == "y":
        check_roll(roll())
    else:
        input("ENTER to quit")

if __name__ == '__main__':
    start_game(True)


Recursive quicksort in Python I want to find a more elegant way to build a recursive sorting function.

The function randomly selects one of the numbers from a given list, k, and splits the list into three: list of smaller numbers, equal to and greater than k.

Now, I recursively sort each list and then combine the 3 to a single sorted list.

For example:

The list [5,2,7,4,9,3] for k = 5 is split into three: [7,9], [5], [2,4,3].

Now sort recursively to get the lists [7,9] and [2,4,3] in order to receive [7,9] and [2,3,4].

The three lists sorted for the receipt of one sorted list: [2,3,4,5,7,9]

Any suggestions?

def partition(lst,k):
    bigger = []
    smaller = []
    equal = []
    i =  0
    while i < len(lst) :
        if lst[i] < k:
            smaller.append(lst[i])
            i += 1
        elif lst[i] == k:
            equal.append(lst[i])
            i += 1
        else:
            bigger.append(lst[i])
            i += 1
    # merge 3 list:
    merged = [smaller,equal ,bigger]
    return merged

def my_sort(lst):
    import random
    k = (random.choice(lst))
    lst3 = partition(lst,k)

    def merge(left, right):
        merged = []

        left_i, right_i = 0, 0
        while left_i < len(left) and right_i < len(right):
            if left[left_i] <= right[right_i]:
                merged.append(left[left_i])
                left_i += 1
            else:
                merged.append(right[right_i])
                right_i += 1
        # copy remaining:
        merged += left[left_i:] + right[right_i:]
        return merged

    def mergesort(lst):
        if len(lst) <= 1:
            return lst

        middle = len(lst) / 2
        left = mergesort(lst[: middle])
        right = mergesort(lst[middle:])
        return merge(left, right)

    left = mergesort(lst3[0])
    right = mergesort(lst3[2])

    print k
    return merge(merge(left,lst3[1]),right)

When you've split a list into a smaller and a bigger halves, after sorting them  the elements in the first half are all still  smaller than all the elements in the second.

So, merging such sorted halves is just positional concatenation, i.e. putting the two halves beside each other so that in the resulting sequence, first go the elements of the first half (sorted), and then the second.

There's no need to compare the elements values in your merge, results of such comparisons are known in advance, predetermined by the  partition stage.

This is the essence of quicksort - partition by value, necessarily putting some effort into doing that, and then, after sorting the halves, join positionally, easily and simply.

The merge sort works the other way - it partitions the list simply, by position; and spends more effort merging the sorted parts, according to the elements' values:

                 mergesort             quicksort

partition        positional            by value

merge            by value              positional

Splitting into three parts is a good optimization, but changes nothing regarding the above.