Reading contents of a file consisting of strings separated by a null character This is my first attempt at reading the contents of a file consisting of strings separated by a null character.

vector<string> CReadFileTest::ReadFile( const char * filename )
{
    ifstream ifs(filename, ifstream::in);
    if (ifs)
    {
        vector<string> list ;

        while (ifs.good())
        {
            string s;
            std::getline(ifs, s,'\0');
            list.push_back(s);
        }

        ifs.close();

        return list;
    }

    return vector<string>();
}

Does this code snippet follows good coding practices?
Is it correct to return a vector type and an empty vector in case of errors?

A couple of issues:

while (ifs.good())

Don't loop on good() (or eof()). This is one of the more annoying parts of the C++ iostream library. This should be:

while(std::getline(ifs, s, '\0'))

std::getline returns a reference to its input stream (std::basic_istream&). How this actually checks the stream state and continues looping is pretty complicated* - suffice to say this is the canonical way of doing it. If you're really being good about your error checking, you'd then call ifs.readstate() after your while loop and check the fail and error flags aren't set.

Secondly, this creates a new string every time around the while loop. Instead, you probably want:

//Note you probably shouldn't call it list, because of the std::list<T> type
vector<string> v;
string s;

while(std::getline(ifs, s, '\0')) {
    v.push_back(s);
}

There are a few ways of signalling that an error has occurred - throwing an exception is one possibility. Returning a pair<vector<string>, bool> or pair<vector<string>, int> indicating either success (for bool) or number of characters read (for int) is another possibility - this is basically an "error code" way of dealing with the problem. Error handling is always tricky, unfortunately.

Either way, currently you return a vector whatever happens, so you might as well construct it outside the if test:

ifstream ifs(filename, ifstream::in);
vector<string> v;

if(ifs) { ... }

return v;

What happens that it calls the ifstream's operator void*() which tests all error flags and eof, returning a nullptr if any of them are set, otherwise a non-zero pointer. See also std::getline returns.


Verify that a binary tree is balanced A binary tree is balanced if both its children are balanced and the difference between the heights of its children is at most 1.

/**
 * Definition for a binary tree node.
 */
class TreeNode {
    int val;
    TreeNode left;
    TreeNode right;
    TreeNode parent;

    TreeNode(int x) {
        val = x;
    }
}

public class isBalanced {
    // first we will get the left height and right height of the node
    // and compute the height difference.if it is > 1 we will return false
    // hashtable to maintain height of each node to reduce the no of redundant
    // height computations
    static Hashtable<TreeNode, Integer> haTab = new Hashtable<>();

    static int height(TreeNode root) {
        // if the node is null then it returns the height as -1
        int heightOfNode;
        if (root == null) {
            return -1;
        }
        if (haTab.containsKey(root)) {
            return haTab.get(root);
        }
        int leftHeight, rightHeight;
        leftHeight = height(root.left);
        rightHeight = height(root.right);
        // finds the height of left subtree and right subtree and returns
        // the max of both + 1
        heightOfNode = Math.max(leftHeight, rightHeight) + 1;
        haTab.put(root, heightOfNode);
        return heightOfNode;
    }

    public static boolean isBalanced(TreeNode root) {
        if (root == null) {
            return true;
        }
        int leftHeight, rightHeight;
        leftHeight = height(root.left);
        rightHeight = height(root.right);
        // if left child and right child are balanced and the difference in
        // their heights is < 2
        // then it is balanced
        if (isBalanced(root.left) && isBalanced(root.right)
                && Math.abs(leftHeight - rightHeight) < 2) {
            return true;
        }
        return false;
    }

    public static void main(String[] args) {
        TreeNode root = new TreeNode(1);
        root.left = new TreeNode(-2);
        root.right = new TreeNode(-3);
        root.left.left = new TreeNode(1);
        root.left.right = new TreeNode(3);
        root.right.right = new TreeNode(-1);
        System.out.println(isBalanced(root));
    }
}

Naming

You did a good job with most your variable names, but not so much with your class name. Typically, you would not want ANY class name to start with a lowercase letter. The class name is also not too descriptive (what is balanced, again?). I recommend changing public class isBalanced to public class TreeBalanceChecker or public class IsBalanced if you don't heed the second part (don't forget to change the file name as well).

Secondly, what is up with haTab? Everything else was so well named, this just seems out of place. Changing it to heightTable would be more descriptive.

Permissions

What I learned is that if you don't have a modifier on your functions, and no other class is using them, add private. That's just standard Java. On the same line, if another class does need to access something, make it public. You seem to be missing a lot of those.

Constructors

There is only one constructor for TreeNode. That is not very helpful if you want to set the left and right children or parent at construction time, or you don't want to set a value.

New code

/**
 * Definition for a binary tree node.
 */
class TreeNode {
    public int val;
    public TreeNode left;
    public TreeNode right;
    public TreeNode parent;

    public TreeNode(){
        val = 0;
    }
    public TreeNode(int x) {
        val = x;
    }
    public TreeNode(TreeNode left, int x, TreeNode right){
        this.left = left;
        this.right = right;
        val = x;
    }
    public TreeNode(TreeNode parent, TreeNode left, int x, TreeNode right){
        this.left = left;
        this.right = right;
        this.parent = parent;
        val = x;
    }

}

public class isBalanced {
    // first we will get the left height and right height of the node
    // and compute the height difference.if it is > 1 we will return false
    // hashtable to maintain height of each node to reduce the no of redundant
    // height computations
    private static Hashtable<TreeNode, Integer> heightTable = new Hashtable<>();

    static int height(TreeNode root) {
        // if the node is null then it returns the height as -1
        int heightOfNode;
        if (root == null) {
            return -1;
        }
        if (heightTable.containsKey(root)) {
            return haTab.get(root);
        }
        int leftHeight, rightHeight;
        leftHeight = height(root.left);
        rightHeight = height(root.right);
        // finds the height of left subtree and right subtree and returns
        // the max of both + 1
        heightOfNode = Math.max(leftHeight, rightHeight) + 1;
        haTab.put(root, heightOfNode);
        return heightOfNode;
    }

    public static boolean isBalanced(TreeNode root) {
        if (root == null) {
            return true;
        }
        int leftHeight, rightHeight;
        leftHeight = height(root.left);
        rightHeight = height(root.right);
        // if left child and right child are balanced and the difference in
        // their heights is < 2
        // then it is balanced
        if (isBalanced(root.left) && isBalanced(root.right)
                && Math.abs(leftHeight - rightHeight) < 2) {
            return true;
        }
        return false;
    }

    public static void main(String[] args) {
        TreeNode root = new TreeNode(1);
        root.left = new TreeNode(-2);
        root.right = new TreeNode(-3);
        root.left.left = new TreeNode(1);
        root.left.right = new TreeNode(3);
        root.right.right = new TreeNode(-1);
        System.out.println(isBalanced(root));
    }
}

Additions

These are just some things to think about adding, not necessary at all

let the user create the tree with STDIN
expand TreeNode class to store height instead of the table
add a method to display the entire tree in STDOUT

Happy coding!


Recursive implementation of a zip function in F# I'm learning F# from Tomas Petricek's "Real-World Functional Programming" book. As suggested at one point in the book, I started implementing a zip function using a recursive sequence expression. I'm not very happy with my implementation and I would like to know how F# devs would refactor my implementation. I know that the original zip function is implemented using map2 but I want to stick with the recursive sequence expression. Any suggestion would expand my horizon.

// Create the zip function
let zipRec first second =
  let rec loop r xs ys = seq {
    if Seq.isEmpty xs || Seq.isEmpty ys then yield r
    else
      let r = Seq.append r (Seq.singleton((Seq.item 0 xs, Seq.item 0 ys)))
      yield! loop r (Seq.tail xs) (Seq.tail ys) }
  loop Seq.empty first second

// test
zipRec [1;2;3;4] [|"one"; "two" |] |> List.ofSeq |> printfn "%A"

What I don't like about my code:

It feels too imperative
I'm using a lot of the functions in the Seq module. Can this be written more F# idiomatic (short, concise, using more functional programming style)?
Does the code perform fast enough for real world scenarios? (I've used tail recursion)
Is the code robust enough (in regards to exceptions)?

Based upon my comment on that answer I've toyed a little to make a recursive sequence expression with roughly the same performance (on my computer at least) as Seq.zip

let zipRec (s1: _ seq) (s2: _ seq) =
  use e1 = s1.GetEnumerator ()
  use e2 = s2.GetEnumerator ()

  let rec loop hasMoved1 hasMoved2 = seq {
    if hasMoved1 && hasMoved2 then
      yield e1.Current, e2.Current
      yield! loop (e1.MoveNext ()) (e2.MoveNext ())
  }

  loop (e1.MoveNext ()) (e2.MoveNext ())

without the recursive requirement it would be simpler this way

let zipRec (s1: _ seq) (s2: _ seq) = seq {
  use e1 = s1.GetEnumerator ()
  use e2 = s2.GetEnumerator ()

  while e1.MoveNext () && e2.MoveNext () do
    yield e1.Current, e2.Current
}


Carousel image slideshow script I have learned to make a carousel image slideshow script with JavaScript. I think it's better to learn it from the basic than just from the framework (instant script), but I'm a newbie. I wrote this script using my technique, but I think it's terrible.

var runCarousel, runTimer;
        firstval = 0;
        secondval = 0;

        function Carousel(){
        firstval += 10;
        document.getElementById('abc-container').style.left = "-" + firstval + "px";
        document.getElementById('asas').innerHTML = "-" + firstval;
            if(firstval == 400)
            {
                StopRun();
                StartTimer()
                return;
            }
            if(firstval == 800)
            {
                StopRun();
                StartTimer()
                return;
            }
        runCarousel = setTimeout(Carousel, 10);
        }

        function StartTimer(){
        secondval += 1;
        document.getElementById('asass').innerHTML = "-" + secondval;
        if(secondval == 10)
        {
            StopTimer();
            Carousel();
            return;
        }
        if(secondval == 20)
        {
            StopTimer();
            Carousel();
            return;
        }
        runTimer = setTimeout(StartTimer, 1000);
        }

        function StopRun(){
        window.clearTimeout(runCarousel);
        }

        function StopTimer(){
        window.clearTimeout(runTimer);
        }

        function firstStart(){
            setTimeout("Carousel()", 10000);
        }

        firstStart();
#wrapper {
width:400px;
height:140px;
position:absolute;
left:50%;
top:50%;
margin: -70px 0 0 -200px;
background: #383838;
overflow: hidden;
}

#abc-container {
position: absolute;
width:1200px;
height:140px;
}

#a {
width:400px;
height:140px;
background: #FF0000;
float: left;
}

#b {
width:400px;
height:140px;
background: #FFFF00;
float: left;
}

#c {
width:400px;
height:130px;
background: #00FFFF;
float: left;
}

Great start for far but you need to work on your overall design.

Here are some tips.

1) Separate the css, html and javascript into their own files.

If you were place all your css into a file called "style.css" and javascript in a file called "main.js", then you could import the files from your html like so.

... more code

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

... more code

2) For your css, try using a creating a class for the box to share common properties.

Old Code:

#a {
    width:400px;
    height:140px;
    background: #FF0000;
    float: left;
}

#b {
    width:400px;
    height:140px;
    background: #FFFF00;
    float: left;
}

#c {
    width:400px;
    height:130px;
    background: #00FFFF;
    float: left;
}

New Code:

#a {
    background: #FF0000;
}
#b {
    background: #FFFF00;
}
#c {
    background: #00FFFF;
}
.box{
    width:400px;
    height:140px;
    float: left;
}

Remember to add the classes.

3) Don't repeat yourself.

The if conditions for firstval and secondval can be combined by using or, ||.

Old Code:

if(firstval == 400){
    StopRun();
    StartTimer()
    return;
}
if(firstval == 800){
    StopRun();
    StartTimer()
    return;
}
runCarousel = setTimeout(Carousel, 10);

New Code:

if(firstval == 400 || firstval == 800){
    StopRun();
    StartTimer()
}else{
    runCarousel = setTimeout(Carousel, 10);
}

4) Declare all variables are the top of a function.

Old Code:

var runCarousel, runTimer;
firstval = 0;
secondval = 0;

New Code:

var runCarousel, runTimer, firstval = 0, secondval = 0;

5) Pass the function name instead of string to setTimeout()

Also it's better to use multiplication when setting the timeout delay.

Old Code:

setTimeout("Carousel()", 10000);

New Code:

setTimeout( Carousel, 10 * 1000);

6) Attach all the global variables to a object literal.

7) Name variables and functions based on their operations.

Rename Carousel() to moveToNextSlide()

8) Cache elements for faster lookup.

Code:

var infoEl = document.getElementById('asas'),
container = document.getElementById('abc-container');

9) There needs to be a limit for firstval inside Carousel()

Add this:

var abc_container = 1200;
firstval += 10;
if( abc_container < firstval ){
    firstval = 0;
}

10) Use variables in place of numeric constants.

It's hard to understand the meaning of 400.
It seems that 400 is the width of each box.

11) secondval isn't needed. Just place everything inside setTimeout() for 10 seconds.

Better yet seconds as a parameter.

Old Code:

function StartTimer() {
    secondval += 1;
    el.innerHTML = "-" + secondval;
    if (secondval == 10 || secondval == 20 ) {
        runTimer = window.setTimeout(StartTimer, 1000);
    }else{
    ...
}

New Code:

function StartTimer( seconds ) {
    setTimeout(function(){
        window.clearTimeout(runTimer);
        moveToNextSlide();
    }, seconds * 1000);
}

12) Read the source code from other image slideshow to find out better techniques.

Here's a place to start. http://www.tripwiremagazine.com/2012/08/jquery-image-slider.html

Final Code:

HTML:

<!DOCTYPE html>

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

Javascript:

var carouselTimer, runTimer, firstval = 0,
    boxWidth = 400, abc_container = 1200,
    infoEl = document.getElementById('asas'),
    container = document.getElementById('abc-container');

function moveToNextSlide() {
    firstval += 10;
    if( abc_container < firstval ){
        firstval = 0;
        return;
    }
    container.style.left = "-" + firstval + "px";
    infoEl.innerHTML = "container.style.left.px = " + firstval;
    if (firstval % boxWidth) {
        carouselTimer = setTimeout(moveToNextSlide, 10);
    }else{
        window.clearTimeout(carouselTimer);
        StartTimer( 2 );
    }
}
function StartTimer( seconds ) {
    setTimeout(function(){
        window.clearTimeout(runTimer);
        moveToNextSlide();
    }, seconds * 1000);
}
StartTimer( 2 );

CSS:

#wrapper {
    width:400px;
    height:140px;
    position:absolute;
    left:50%;
    top:50%;
    margin: -70px 0 0 -200px;
    background: #383838;
    overflow: hidden;
}
#abc-container {
    position: absolute;
    width:1200px;
    height:140px;
}
#a {
    background: #FF0000;
}
#b {
    background: #FFFF00;
}
#c {
    background: #00FFFF;
}
.box{
    width:400px;
    height:140px;
    float: left;
}

Demo: http://jsfiddle.net/vSWmx/2


Display related images based on text name Instead of  "Black and Green" text, We are displaying "Black & Green" images with help of below code.

What we want is this concept should work for all colors, for that one solution is i need to write manual code for hundreds of colors as like below to make it work for all colors. but i think this is wrong way , may be it may impact on performance, is there any better way to handle this ?

Script :

var jQuery = $.noConflict();
jQuery(document).ready(function(){
    var inner = Array();
    inner = jQuery(" .product-options ul.options-list .label>label");
    for (i=0;i<inner.length;i++){
         var classN = inner[i].innerText;
          if (classN=="Black" || classN=="Green"){
               inner.eq(i).addClass(classN);
          }
     }
});

Css for green :

.product-options ul.options-list .label>label.Green
{
     font-weight: normal;
     width: 50px;
     height: 50px;
     border-radius: 50%;
     background-image: url("http://sbdev2.kidsdial.com:81/media/catalog/custom/green.png") !important;
     background-size: cover !important;
     display: block;
     color: transparent;
     padding: 0 !important;
     font-size: 0;
}

Css for black :

.product-options ul.options-list .label>label.Black
{
     font-weight: normal;
     width: 50px;
     height: 50px;
     border-radius: 50%;
     background-image: url("http://sbdev2.kidsdial.com:81/media/catalog/custom/black.png") !important;
     background-size: cover !important;
     display: block;
     color: transparent;
     padding: 0 !important;
     font-size: 0;
}

A few thoughts come to mind here:

You are not properly using inheritance in your CSS.  You should probably have one definition for .product-options ul.options-list .label>label which contains every single CSS declaration except for the actual background image. As these will not change.  Then just have definitions for .product-options ul.options-list .label>label.* (where * is green, black, etc.) that are one line each (specifying background-image), as that is the only part that changes based on color.
You might consider not using images at all in favor of pure CSS.  This could actually give the benefit of potentially not having to declare per-color CSS styles at all, in favor of just changing the background color value on the elements to one of the valid CSS named colors. Here are CSS-only examples for different shapes. This would actually perform better as there would be no images to download at all.
Your variable names are not very meaningful. For example inner might better be called labels or similar (or $labels if you use the convention of pre-pending jQuery collections with $). Perhaps color instead of classN.
You might need to give up hope of individually checking each label element text as being one of allowed color values. Or look to solutions like these to provide validation that the color names are good.
Since you are working against a jQuery collection,  you might consider using $.each() as your iterator rather than a for loop.

Combining these approaches might mean you can simplify your code down to:

var jQuery = $.noConflict();
jQuery(document).ready(function(){
    var $labels = jQuery(" .product-options ul.options-list .label>label");
    $labels.each(function() {
        var color = $(this).text();
        // perhaps validate color here as noted above in link
        if(colorIsValid(color)) {
            $(this).css('background', color);
        } else {
            throw new RangeError('Unexpected color value.');
        }
    }
});


Create a C++ string using printf-style formatting It's often convenient to use C-style printf format strings when writing C++.  I often find the modifiers much simpler to use than C++ I/O manipulators, and if I'm cribbing from existing C code, it helps to be able to re-use the existing format strings.

Here's my take on creating a new std::string given a format and the corresponding arguments.  I've given it an annotation so GCC can check the argument types agree when the format is a literal, but I don't know how to help other compilers.

I've provided a more memory-efficient version for C++17, which provides read/write access to the underlying array of a string.  My copy of CPP Reference still says "Modifying the character array accessed through data() has undefined behavior", but the Web version has been edited (May 2017) to indicate that it's only the const version that has that constraint.

For earlier standards (I require minimum C++11), we may need to allocate a temporary array, as we can't write to a string's data.  Unfortunately, this requires an extra allocation and copy.

#include <string>

std::string format(const char *fmt, ...)
#ifdef __GNUC__
    __attribute__ ((format (printf, 1, 2)))
#endif
    ;

// Implementation

#include <cstdio>
#include <cstdarg>

#if __cplusplus < 201703L
#include <memory>
#endif

std::string format(const char *fmt, ...)
{
    char buf[256];

    va_list args;
    va_start(args, fmt);
    const auto r = std::vsnprintf(buf, sizeof buf, fmt, args);
    va_end(args);

    if (r < 0)
        // conversion failed
        return {};

    const size_t len = r;
    if (len < sizeof buf)
        // we fit in the buffer
        return { buf, len };

#if __cplusplus >= 201703L
    // C++17: Create a string and write to its underlying array
    std::string s(len, '\0');
    va_start(args, fmt);
    std::vsnprintf(s.data(), len+1, fmt, args);
    va_end(args);

    return s;
#else
    // C++11 or C++14: We need to allocate scratch memory
    auto vbuf = std::unique_ptr<char[]>(new char[len+1]);
    va_start(args, fmt);
    std::vsnprintf(vbuf.get(), len+1, fmt, args);
    va_end(args);

    return { vbuf.get(), len };
#endif
}

// Test program

#include <iostream>
int main()
{
    std::clog << "'" << format("a")
              << "'" << std::endl;
    std::clog << "'" << format("%#x", 1337)
              << "'" << std::endl;
    std::clog << "'" << format("--%c--", 0) // an embedded NUL
              << "'" << std::endl;
    std::clog << "'" << format("%300s++%6.2f", "**", 0.0).substr(300)
              << "'" << std::endl;
}

void provoke_warnings()
{
    // warning: zero-length gnu_printf format string
    // [-Wformat-zero-length]
    std::clog << "'" << format("") << "'" << std::endl;

    // warning: format ‘%c’ expects argument of type ‘int’, but
    // argument 2 has type ‘const char*’ [-Wformat=]
    std::clog << "'" << format("%c", "bar") << "'" << std::endl;
}

I've compiled the code with both C++17 and C++11 compilers, and verified them both under Valgrind using this test program.

I'd welcome any comments on the code itself or on my testing.

I agree with you that 'printf-style' formatting was by many aspects better than C++'s manipulators: more concise, more varied, etc. That said, I feel like it's a step backwards if we port them into C++ without upgrading the C logic behind it. In my opinion, your code fails to do that for two reasons: lack of automatic memory management, and lack of type safety.

About type safety, I believe we now must use variadic templates when dealing with an unknown number of arguments. Variadic macros have too many shortcomings for us to continue using them. With variadic templates, we're able to verify if format directives and arguments match in number and types in a more reliable way (although I agree that when you can rely on a format chain the risk is manageable even with C-like macros).

About memory management: I believe we must avoid explicit memory management whenever we can. std::stringstream is a good way to do that when one wants to build a string progressively from heterogeneous arguments.

So here's an embryo of what I would consider more modern C++:

#include <string>
#include <sstream>
#include <iostream>
#include <type_traits>
#include <exception>

// base case of recursion, no more arguments
void format_impl(std::stringstream& ss, const char* format) {
    while (*format) {
        if (*format == '%' && *++format != '%') // %% == % (not a format directive)
            throw std::invalid_argument("not enough arguments !\n");
        ss << *format++;
    }
}

template <typename Arg, typename... Args>
void format_impl(std::stringstream& ss, const char* format, Arg arg, Args... args) {
    while (*format) {
        if (*format == '%' && *++format != '%') {
            auto current_format_qualifier = *format;
            switch(current_format_qualifier) {
                case 'd' : if (!std::is_integral<Arg>()) throw std::invalid_argument("%d introduces integral argument");
                // etc.
            }
            // it's true you'd have to handle many more format qualifiers, but on a safer basis
            ss << arg; // arg type is deduced
            return format_impl(ss, ++format, args...); // one arg less
        }
        ss << *format++;
        } // the format string is exhausted and we still have args : throw
    throw std::invalid_argument("Too many arguments\n");
}

template <typename... Args>
std::string format(const char* fmt, Args... args) {
    std::stringstream ss;
    format_impl(ss, fmt, args...);
    return ss.str();
}

int main() {
    auto display = format("My name is %s and I'm %d year old", "John", 22);
    std::cout << display;
}


Joining to in memory List I have an in memory list that I am joining to results from a query using entity framework.  My list will most likely never be greater than 2500 records.  The results from the database can fluctuate, depending on the filters used and it will grow in size.  I've been able to join the results successfully but it feels a bit sluggish and I worry as the database grows that it could get worse.  Is there anything I can do to make this more efficient?  Please let me know if you need any additional information.  Thanks!

var query = from e in _context.Employees
            where (...filters...)
            select e;

var employees = query.AsEnumerable();
var offices = _officeService.GetAllOffices();

var employeeData = from e in employees
                   join o in offices on e.Office equals o.Code
                   select new EmployeeData
                   {
                       EmployeeId = e.EmployeeId,
                       FullName = e.FullName,
                       Office = e.Office,
                       Area = o.Area,
                       Region = o.Region,
                       OfficeName = o.Name,
                       Position = e.Position,
                       Languages = e.Languages
                   };

return employeeData;

The dilemmas here:

query is an IQueryable. If you join it with offices, i.e. without AsEnumerable(), Entity Framework will throw an exception about primitive values, which is an obscure way of telling you that it can't translate offices into SQL.
So join in memory, i.e. with query.AsEnumerable(). But now all data from query will be pulled into memory, which has two adverse effects: neither the reduction in numbers of records by joining with offices nor the reduction in width of the result set by selecting only a restricted number of properties can be translated back to the SQL query.

You obviously want to benefit from both strands of data reduction.

As for the reduction in number of rows, there's no way to make Entity Framework join with local data other than lists of primitive values. Even then, joining is rather inefficient because EF has to convert the local list into a temporary SQL table (sort of), which requires a considerable amount of code. It's more efficient to use Contains, which translates into an IN statement:

var officesCodes = offices.Select(o => o.Code).ToList();
var employeeInfo = from e in employees
                   where officesCodes.Contains(e.Office)
                   select ...

Now employeeInfo is an IQueryable, so it's possible to reduce the width of the result set by projection:

var employeeInfo = from e in employees
                   where officesCodes.Contains(e.Office)
                   select new
                   {
                       EmployeeId = e.EmployeeId,
                       FullName = e.FullName,
                       Office = e.Office,
                       Position = e.Position,
                       Languages = e.Languages
                   };

This achieves the desired data reduction. But now you haven't got EmployeeData objects yet. Can't be done by this query, because they also contain data from offices. This final step can only be achieved by joining the result in memory with offices:

var employeeData = from e in employeeInfo.AsEnumerable()
                   join o in offices on e.Office equals o.Code
                   select new EmployeeData
                   {
                       EmployeeId = e.EmployeeId,
                       FullName = e.FullName,
                       Office = e.Office,
                       Area = o.Area,
                       Region = o.Region,
                       OfficeName = o.Name,
                       Position = e.Position,
                       Languages = e.Languages
                   };


UnionFind implementation The Wikipedia article on the union find problem gives a very simple implementation, which I ported to C# and tested.

I know that the code should be, in the aggregate, asymptotically almost linear. But is it a practical implementation? Are there optimizations I should have used? Is there a way to cut down on the worst-case complexity of single operations?

using System;

/// <summary>
/// A UnionFindNode represents a set of nodes that it is a member of.
///
/// You can get the unique representative node of the set a given node is in by using the Find method.
/// Two nodes are in the same set when their Find methods return the same representative.
/// The IsUnionedWith method will check if two nodes' sets are the same (i.e. the nodes have the same representative).
///
/// You can merge the sets two nodes are in by using the Union operation.
/// There is no way to split sets after they have been merged.
/// </summary>
public class UnionFindNode {
    private UnionFindNode _parent;
    private uint _rank;

    /// <summary>
    /// Creates a new disjoint node, representative of a set containing only the new node.
    /// </summary>
    public UnionFindNode() {
        _parent = this;
    }

    /// <summary>
    /// Returns the current representative of the set this node is in.
    /// Note that the representative is only accurate untl the next Union operation.
    /// </summary>
    public UnionFindNode Find() {
        if (!ReferenceEquals(_parent, this)) _parent = _parent.Find();
        return _parent;
    }

    /// <summary>
    /// Determines whether or not this node and the other node are in the same set.
    /// </summary>
    public bool IsUnionedWith(UnionFindNode other) {
        if (other == null) throw new ArgumentNullException("other");
        return ReferenceEquals(Find(), other.Find());
    }

    /// <summary>
    /// Merges the sets represented by this node and the other node into a single set.
    /// Returns whether or not the nodes were disjoint before the union operation (i.e. if the operation had an effect).
    /// </summary>
    /// <returns>True when the union had an effect, false when the nodes were already in the same set.</returns>
    public bool Union(UnionFindNode other) {
        if (other == null) throw new ArgumentNullException("other");
        var root1 = this.Find();
        var root2 = other.Find();
        if (ReferenceEquals(root1, root2)) return false;

        if (root1._rank < root2._rank) {
            root1._parent = root2;
        } else if (root1._rank > root2._rank) {
            root2._parent = root1;
        } else {
            root2._parent = root1;
            root1._rank++;
        }
        return true;
    }
}

If I read the wikipedia article correctly then the algorithm should have amortized constant cost - and you have implemented it pretty much 1:1. Given that with your implementation you always start with disjoint nodes and any call to any of the public methods ends up calling Find which will automatically flatten the tree I doubt you can get much better.
UnionFindNode is not a particularly good name for the data structure: intermingles operations with the data structure in the name. Just Node or maybe DisjointSetNode would be better.
You could use == or != instead of ReferenceEquals which would make the code a bit easier to read.
Consider making your node class generic and add a T value property - right now your nodes are not all that useful as they don't hold any data.


Calling a copy constructor in Presenter to prepare a list of Models I'm new to MVP. In a presenter I have following code. Here what am I trying is to create a list of Model objects (in this case IAttendance objects) and to show them on a DataGridView in the View.

Every time an attendance is entered on the View, AddAttendanceObjectToGrid() method on the presenter will be called to add the new attendance to the list. So to add a new Attendance model to the list I'm using a copy constructor in my Model.

Now this code runs giving the desired output but I feel this could be further improved to get the best of MVP.

class AttendancePresenter : BasePresenter
{
    private IAttendance _Model;
    private readonly IAttendanceView _View;

    BindingSource BS = new BindingSource();
    List<IAttendance> AttendanceList = new List<IAttendance>();

    public AttendancePresenter( IAttendance model, IAttendanceView view )
    {
        _Model = model;
        _View = view;
    }

    private void AddAttendanceObjectToGrid()
    {
        SetModelPropertiesFromView(_Model, _View); // Call base class method to update the Model with data
        IAttendance attendanceModel = new Attendance(_Model); // Use copy constructor to get a new copy of model
        AttendanceList.Add(attendanceModel); // Add new model to list
        BS.DataSource = AttendanceList; // Show list on the grid
        _View.AttendanceInGrid = BS;
    }
}

MY MODEL

class Attendance : IAttendance
    {
        public string PointID { get; set; }
        public string PointName { get; set; }
        public int EmployeeID { get; set; }
        public string EmployeeName { get; set; }
        public DateTime InTime { get; set; }
        public DateTime OutTime { get; set; }
        public string ShiftType { get; set; }

        public Attendance() // Default constructor
        {
        }

        public Attendance(IAttendance attendance) //Copy constructor
        {
            PointID = attendance.PointID;
            PointName = attendance.PointName;
            EmployeeID = attendance.EmployeeID;
            EmployeeName = attendance.EmployeeName;
            InTime = attendance.InTime;
            OutTime = attendance.OutTime;
            ShiftType = attendance.ShiftType;

        }
    }

At first glance, there seems to be something off in the structure.

You wish to display a list of IAttendance objects on the screen. To me, this means that the Model is a list of IAttendance objects but the code shows only a single IAttendance instance being passed into the constructor.  How do you get more than one item into the presenter?

It has been a long while since I used MVX under WinForms, so some of what I remember may be off, but I would have a Model consisting of a list of IAttendance objects (not a single object) and just add them to a BindingList.  There should be no need to copy the objects. (If you want to edit them, make them implement IEditableObject).

Bottom Line
There should be no need to copy the objects just to show them in a list.
If you want the screen to display a list of objects then the Model should consist of a list of objects, not a single one
If you want to edit the objects, implementing IEditableObject is a better path than copying/saving and restoring on cancel.


Implementation of a singleton HttpClient with generic methods I need advice for this implementation that I made for a service access portable class library referenced to Xamarin.Android and Xamarin.Ios projects. For now, it works fine, but I have doubts about its performance or if it would crash on simultaneous calls.

public class HttpClientBase : HttpClient
{

    private static readonly HttpClientBase instance = new HttpClientBase();

    static HttpClientBase() { }

    private HttpClientBase() : base()
    {
        Timeout = TimeSpan.FromMilliseconds(15000);
        MaxResponseContentBufferSize = 256000;
    }

    public static HttpClientBase Instance
    {
        get
        {
            return instance;
        }
    }

    public async Task<List<T>> GetListItems<T>(string Url)
    {
        var uri = new Uri(string.Format(Constantes.BaseUrlApi, Url));
        var response = await GetAsync(uri);
        if (response.IsSuccessStatusCode)
        {
            var content = await response.Content.ReadAsStringAsync();
            var Items = JsonConvert.DeserializeObject<List<T>>(content);
            return Items;
        }
        throw new Exception(response.ReasonPhrase);
    }

    public async Task<T> GetItem<T>(string Url)
    {
        var uri = new Uri(string.Format(Constantes.BaseUrlApi, Url));
        var response = await GetAsync(uri);
        if (response.IsSuccessStatusCode)
        {
            var content = await response.Content.ReadAsStringAsync();
            var Item = JsonConvert.DeserializeObject<T>(content);
            return Item;
        }
        throw new Exception(response.ReasonPhrase);
    }

    public async Task PostItem<T>(T item, string url)
    {
        var uri = new Uri(string.Format(Constantes.BaseUrlApi, ""));
        var json = JsonConvert.SerializeObject(item);
        var content = new StringContent(json, Encoding.UTF8, "application/json");
        HttpResponseMessage response = null;
        response = await PostAsync(uri, content);
        if (response.IsSuccessStatusCode)
        {
            return;
        }
        throw new Exception(response.ReasonPhrase);
    }
}

General remarks

The naming and the design are ok. The only thing that could be improved is the

 HttpClientBase

It's not a base class so you shouldn't call it like this. I suggest HttpClientService.

Timeout = TimeSpan.FromMilliseconds(15000);

Or simply TimeSpan.FromSeconds(15);

Singleton

There are of course mutliple patters for creating a singleton but the one you chose is probably the right one Singleton by Jon Skeet clarification

Concurrency

  I have doubts about its performance or if it would crash on simultaneous calls.

I don't see actually anything that could be further optimized. You call only core APIs that should already do their best.

The methods don't share any data with each other so there is no risk in calling them in parallel.


Simple Timer Program I have built a timer program in JavaScript. It decrements one counter every second, and if that counter is at zero, it decrements another counter, and sets the first counter to 59. If the second counter now is below zero, then it stops the timer. Would it be more efficient to store the total seconds left, and calculate the minutes on the fly? Is there other stuff I can improve on?

var timerRun; // Variable for holding the interval
          var timeLeft; //Array; index 0 keeps track of the minutes, and index 1 keeps track of the seconds
          var timerRunning = false; //Boolean to keep track of whether the timer is running
          function tick() {
              // Subtract 1 second
              timeLeft[1] -= 1;
              if (timeLeft[1] < 0) { // Decrement the minute if neccessary
                  timeLeft[0] -= 1;
                  timeLeft[1] = 59;
              }
              if (timeLeft[0] < 0) { // Check if the timer is done
                  clearInterval(timerRun);
                  alert("Timer is done!");
                  timerRunning = false;
                  document.getElementById("timer").innerHTML = "Simple Timer";
              }
              if (timeLeft[1] < 10) { // Display the seconds as a two-digit number
                  timeLeft[1] = "0" + timeLeft[1];
              }
              document.getElementById("timer").innerHTML = timeLeft.join(":");
              if (timerRunning == false) {
                  document.getElementById("timer").innerHTML = "Simple Timer";
              }
          }
          function time() { // Restart timer
                clearInterval(timerRun)
                timerRun = setInterval(tick, 1000);
                // Create array
                timeLeft = document.getElementById("time").value.split(":");
                timerRunning = true;
          }
<!DOCTYPE html>

Simple Timer

      <input type="text" id="time" value="15:00">
      <button type="button"  onclick="time();" id="start">Start</button>

You are basically trying to show timer, which can be up to hours. Best way to calculate and represent that for your usecase is definitely by just one variable, which holds all the seconds. That way your code handling current state is a lot simpler - you have just one variable, you decrease it every second until it's done.
You will have more work displaying that value as you will have to calculate minutes and seconds, but that's fine.
Definitely extract that to function - you pass number of seconds and you get string representing it. You may even consider using library for that or a number formatter. There are plenty ways how to do that:
https://stackoverflow.com/questions/3733227/javascript-seconds-to-minutes-and-seconds
Your variable timerRunning is redundant. In your code same meaning has expression timeLeft[1] < 0 && timeLeft[0] < 0. You really don't want redundancy, that leads to bugs. Again you can extract to function isTimerRunning if you need it. But again if you end up with only one timeLeft variable with representing all in seconds, you just need !timeLeft to check, if timer is over.
Probably not a problem in your case, but keep in mind, that setInterval is not very reliable time-wise. It pings "around" every second, but it jumps a lot. More on this here:
https://stackoverflow.com/questions/29971898/how-to-create-an-accurate-timer-in-javascript

Edit:

In this line function time() { // Restart timer you poorly name your function and then you have to comment it to describe what it does. Why not just name function restartTimer and you can delete comment?


Proper way of using the MVC concept - update Original question:

Template engine

I updated my files as instructed, and now I require a review.  Please point out the most minute things out to me for me to have better code. Once again, efficiency, elegance, and performance is what I need. Also, please point out places where better code can be used (for places like loading a template, I need to find a better way to load a template).

Model:

<?php
class Model {
    public $params;
    public $site_url;
    public $theme;

    public function __construct($global) {
        $this->site_url = $global['site_url'];
        $this->theme    = $global['theme'];
    }
}

View:

<?php
class View {
    private $model;
    private $controller;

    public function __construct($model, $controller) {
        $this->model      = $model;
        $this->controller = $controller;
    }
    public function createPage($path) {
        $this->content = $model->replaceParams(file_get_contents($path));
        return $this->content;
    }
    public function createParam($key, $value) {
        $model->params['{' . $key . '}'] = $value;
    }
    public function replaceParams($content) {
        foreach($model->params as $key => $value) {
            str_replace($key, $value, $content);
        }
        return $content;
    }
    public function pageExists($path) {
        return file_exists($path);
    }
}

Controller:

<?php
class Controller {
    private $model;

    public function __construct($model) {
        $this->model = $model;
    }
    public function handlePageLoad($page_name) {
        $this->path = $model->site_url . "/app/tpl/skins/" . $model->theme . "/" . $page_name . ".php";
        if($view->pageExists($this->path) === true) {
            $this->content = $view->createPage($this->path);
            return $this->content;
        } else {
            header('HTTP/1.1 404 Not Found');
            return false;
        }
    }

}

Global:

<?php
function autoload($class_name) {
    include "classes/class." . $class_name . ".php";
}
spl_autoload_register('autoload');

$global = array(
    'site_url' = 'http://localhost/Projects/HassanCMS',
    'theme'    = 'v1'
);

$model      = new Model($global);
$controller = new Controller($model);
$view       = new View($model, $controller);

$view->createParam('site_title', 'HassanTech');

I'll skip MVC for now, as I see a whole review for formatting and the such. Perhaps someone else could break down the pattern for you. Right now, let's focus on each individual class.

Model

public!? What's your logic behind that!? Are we programming object-oriented code here? Part of OOP is designing with encapsulation in mind. Encapsulation is using those getters and setters. So in your case, we might find:

protected $site_url;

public function getSiteURL() {
    return $this->site_url;
}

Here are some benefits of doing this:

  By keeping data private and providing public well-defined service
  methods the role of the object becomes clear to other objects. This
  increases usability.

Source

  The shielding of the data means that should the format of the private
  data need to change for some reason the objects invoking methods on
  the object whose data has changed will not need to change the format
  they send their messages in.

Source

  Encapsulation is used to hide the values or state of a structured data
  object inside a class, preventing unauthorized parties' direct access
  to them.

Source
You should type hint $global with array.
In general, it seems strange to be passing this type of configuration data as an array in the constructor. If you had a configuration class, then you could pass that, and then the model would have to worry less about the information it's handling. It will only get the information it wants, yet it's expandable so you can later add more information.

View

You could improve the constructor to something like:

__construct(Model $model, Controller $controller)

This way you know exactly what's being thrown at you.
You raised the scope of $this->content, which is unnecessary. Keep it local.
'{' . $key . '}': it would make more sense to do this as you replace, not store.
$key and $value aren't very meaningful names.
For both createParam and replaceParams, don't be afraid to type out the whole word. It helps no one to shorten it like you have.
pageExists has no business living in the view.
And probably the biggest issue I see is the lack of support. Right now, this class replaces static values on static pages. What if we need a conditional on the view? ("If the user's logged in, show this. If not, show that") Or maybe a you want to show all the blog posts you stored, well you'll need to build the HTML in your PHP, and then send it all as one parameter! Think about what you can do to support these types of edge cases.

Controller

It's not uncommon to see child controllers, so private may want to be protected for $model.
Type hint Model in the constructor.
Any type of "handler" or "manager" is a bad sign. So either handlePageLoad needs a new name, or the contents need to be broken down. Let's have a look...
Well, it looks like the method "handles" to much! Perhaps pageExists should be a controller function, and that way you can work in the path.
$this->path should be local to the method. You're reducing the class scope's clarity by not having proper local variables.
Normally there's a space after ifs.
file_exists returns a boolean only, so the type comparison is overkill. A simple == would suffice.

Global

classes is a horrible directory name. I highly suggest you change that. Perhaps break apart the classes by responsibility.
Here's anything fuzzy variable name, $global.
I don't see the connection between 'theme' and 'v1'. Is "theme" not the best word?


Simple wrapper for an API This is a wrapper for an API. For now all it does is get a certain document or search for relevant documents from a document store -- although I plan to build out more features later. I'm new to PHP and want to both improve my PHP skills and develop "idiomatic" PHP style. It depends on HTTPFUL, which, for now, is being loaded from code that uses the class.

<?php

class PHPDocumentCloud
{
    // property declaration
    public $var = 'a default value';

    public function get($id){
        $url = "http://www.documentcloud.org/api/documents/" . $id . ".json";
        $answer = \Httpful\Request::get($url)->send()->raw_body;
        $answer = (json_decode($answer)->document);
        return $answer;
    }

    private function get_search_page($query, $page, $perpage){
        $url = "http://www.documentcloud.org/api/search.json?q=" . $query . '&page=' . $page . '&per_page=' . $perpage. '&mentions=3';
        $answer = \Httpful\Request::get($url)->send();
        $answer = (json_decode($answer));
        $docs = $answer->documents;
        return $docs;
    }

    public function search($query){
        //check for &page=2
        $page = 1;
        $arr = array();
        $document_list = array();
        $loop = True;
        while ($loop) {
            $results = $this->get_search_page($query, $page, 1000);
            if (!empty($results)){
                $document_list = array_merge($results, $document_list);
                $page++;
            }else{
                $loop = False;
            }
        }
        return $document_list;
    }

}

?>

Ok, the general critiques first:

Please, subscribe to the coding standards. It may seem irrelevant to you now. But believe me when I say that coding style is as important as unit tests once you start collaborating on a project.
Add use statements, to see all of the class' dependencies at a glance
Be consistent, both in method/property names as in the naming of local variables.
Don't use a closing ?> tag, if a script contains nothing but PHP code. The official docs explain why
Sanitize method arguments, don't simply string them together.
Use type-hints and inject dependencies whenever you can.
Doc-blocks save lives. Add them to your code ASAP. Use a decent IDE, and you'll soon find you can't do without these comments.

Now what does this all mean, applied to your code? Quite simply that you have to use that most valuable trait of all: common sense. A quick re-write of your code, applying most of these critiques.

<?php
use Httpful\Request;//add use statement
/**
 * @package DocumentCloudAPI
 * @author <your name>
 */
class PHPDocumentCloud
{
    //immutable part of url is just that: a constant value
    const BASE_URL = 'http://www.documentcloud.org/api/';

    /**
     * @var string
     */
    public $var = 'a default value';

    /**
     * @var Request
     */
    protected $requester = null;

    /**
     * Constructor, allows users to pass child class of
     * dependency as parameter, assign to property
     * @param Request $requester
     */
    public function __construct(Request $requester)
    {
        $this->requester = $request;
    }

    /**
     * @param int $id
     * @return \stdClass
     * @throws \InvalidArgumentException
     * @throws \RuntimeException
     */
    public function getById($id)
    {//next line, after more descriptive method name
        if (!is_numeric($id) || $id != (int) $id)//check numeric && not float
            throw new \InvalidArgumentException(
                sprintf(
                    '%s expects argument to be an int, instead saw %s',
                    __METHOD__,
                    gettype($id)
                )
            );
        $url = sprintf(
            '%sdocuments/%d.json',
            self::BASE_URL,
            (int) $id
        );//construct request url
        $response = $this->requester::get($url)->send();
        //no response, or no raw_body property => exception
        if (!$response || !property_exists($response, 'raw_body'))
            throw new \RuntimeException(
                sprintf(
                    'Request failed: %s',
                    json_encode($response)//force string
                )
            );
        //check for json errors
        $response = json_decode($response->raw_body);
        $err = json_last_error();
        if ($err === \JSON_ERROR_NONE)//No errors?
            return $response->document;
        //error, throw exception
        throw new \RuntimeException(
            sprintf(
                '%d - %s',
                $err,
                json_last_error_msg()
            )
        );
    }

    /**
     * Accepts assoc array, with keys q, page, per_page and mentions
     * @param array $params
     * @return array|\stdClass (don't know what type documents is
     * @throws \InvalidArgumentException
     * @throws \RuntimeExcpetion
     */
    private function getSearchPage(array $params)
    {
        $url = self::BASE_URL.'search.json?';
        //TODO: move this to a custom, more generic filterValidKeys method
        //along the lines of: filterValid(array $params, array $keys)
        //keys array could be assoc, where values are names of validation callbacks
        $keys = array('q', 'page', 'per_page', 'mentions');
        foreach ($keys as $key)
        {
            if (isset($params[$key]))
            {
                if ($key === 'q')
                    //in real life, check if the string isn't already url-encoded
                    $params[$key] = urlencode($params[$key]);
                else//given that other params are numeric
                    $params[$key] = (int) $params[$key]
                $url .= $key.'='.$params[$key].'&';//add & here
            }
        }
        if (substr($url, -1) === '?')//if last char of url is ?, no params were set
            throw new \InvalidArgumentException(
                sprintf(
                    '%s requires at least one of %s keys to be set',
                    __METHOD__,
                    implode(',',$keys)
                );
        $response = $this->requester::get($url)->send();
        if (!$response)
            throw new \RuntimeException(
                sprintf(
                    'Request to %s failed',
                    $url
                )
            );
        $response = json_decode($answer);
        $err = json_last_error();
        if ($err !== \JSON_ERROR_NONE || !property_exists($response, 'documents'))
        {
            if ($err === \JSON_ERROR_NONE)
                $msg = sprintf('%d - %s', $err, json_last_error_msg());
            else
                $msg = sprintf('documents not found in %s', json_encode($response));
            throw new \RuntimeException($msg);
        }
        if (empty($response->documents))
            return null;//return null if we reached the end
        return $response->documents;
    }

    /**
     * Public alias for getSearchPage, with loop
     * @param string $query
     * @return array
     */
    public function search($query)
    {
        $params = array(
            'q'        => $query,
            'page'     => 1,
            'per_page' => 1000,
            'mentions' => 3
        );
        $list = array();
        while ($results = $this->getSearchPage($params))
        {
            $list = array_merge($list, $results);
            ++$params['page'];//get next page
        }
        return $list;
    }
}

Disclaimer:
I wrote this off the top of my head, so there might be some syntax errors and oversights. This code is not meant to be copy-pastable at all. It's just an example of how to use doc-blocks, type-hinting, data sanitation, exceptions and all coding standards.

Onwards

Now, at first glance, you may think this code over-complicates things hideously. And truth be told, it does add some more complexity to the mix. What do you get in return?
Again at first glance: a class that throws exceptions whenever it encounters something that isn't quite right. But that's how a class should behave. A class is meant to perform one (and just one) task. If the data that is provided does not fit the criteria, that class's job is not to make do. The code that is calling the class needs to be fixed. An exception represents an emergency.
The calling code should handle the exception, or should be fixed. You can't ignore exceptions.

In the long run, though, this class offers a safer API, it's quite clear-cut. As long as you (the person calling the code) provide the correct data, you know what to expect. You can inject your own implementation of this Httpful\Request class to work with, and adding simple methods to perform certain calls are very easy to add. You just have to, basically, follow the same pattern:

Build url, starting with self::BASE_URL
use $this->requester::get($url)->send(); to get the response
check the response, and the result of json_encode. Return, or throw exceptions accordingly.

If you don't quite understand all of the functions used here, then don't worry: php.net/<function here> will tell you all you need to know.

Just one more thing:
In going over your code, and rewriting it, I couldn't help myself, but code like this:

$loop = True;
while ($loop) {
    $results = $this->get_search_page($query, $page, 1000);
    if (!empty($results)){
        $document_list = array_merge($results, $document_list);
        $page++;
    }else{
        $loop = False;
    }
}

Is just all shades of wrong. A while(true) loop (in PHP it's true, as opposed to python's True) is to be avoided as much as possible.
However, if you want to break an infinite loop, don't do that by setting a variable to another value, do so by using the cleverly named break; keyword:

while(true)
{
    if (empty($foo))
        break;//ends loop
    $foo = $this->doStuff();
}

But in most cases, ie: fetching rows from a result-set, it's best to perform a loop while you're fetching the data, and write it accordingly:

while($row = $stmt->fetch())
{
    //do stuff with $row
}

This works, simply because an assignment is an expression, and like any expression, it can be evaluated to a singular value. PHP assignment expressions evaluate to the new value of the left operand. In other words:

if ($x = false)
{//$x is left operand, being assigned false, expression evaluates to false
    echo 'This will never echo';
}


Is this a good way to syncronize my Threads? After getting much improvement, it´s still one issue that concerns me.
And that is the sync between my 2 threads.

I use AutoResetEvent, but sadly, it has some delay with it, and would love to use something else if possible. Would be great if something could be connected to a Bool or the Queue i am using. Here is the Codes:

    SharpDX.Windows.RenderLoop.Run(form, () =>
    {

    if (Queue.TryDequeue(out TextureData))
    {
        ........
    }
    else
    {
        AutoReset.WaitOne(100);
    }
});

So there it does it´s thing, if there is something in a Queue, else it will wait on AutoReset Up till 100ms (this is to prevent the Form to totally freeze).

while (ReceiveCheck.Checked && tt1.Connected)
{
    ...........
    AutoReset.Set();
    Queue.Enqueue(tempBytes);
}

And there i simply do my work, and then tell the other thread to start, and right after i Queue my item. (why i AutoReset Before that is because of the delay, to help minimize it a bit).

Is there a way to improve this?
Would be great if the other thread could just wait until there is something in the Queue and then run.

When I tried to measure the delay of AutoResetEvent, I usually got less than 0.1 ms. That doesn't sounds like something you should worry about. If you're seeing larger delay, maybe it's caused by something else. It could be processing the message queue, which is what RenderLoop does between calls to Run().
Calling Set() before you enqueue the item is wrong, because it has a race condition: the dequeuing thread could wake up and call TryDequeue() before you call Enqueue().
In fact, the whole way you're using AutoResetEvent is wrong. If two items are enqueued quickly, then only one of them will be dequeued. Actually, this could be what's causing the delay you see, because it means there could be an ever-growing number of items in the queue just waiting for no good reason.
A better way to write your code would be to use BlockingCollection<T> instead of ConcurrentQueue<T> (which I assume you're using now) and AutoResetEvent.
It might be more efficient to dequeue more than one item from the queue in each call to Run(), if they are available. But probably only a limited amount, to make sure the message queue is processed too.


Splitting a string into tokens in C I am trying to improve my skill in C, and this time by getting away from strtok. The following code splits the input string into a token_list, an array of strings.

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

#define MAX_LINE_LEN 256
#define NULL_TERM '\0'

/* A function that will print the content of the token_list */
int printcharlist(char **tok_list)
{
    char **pptr = tok_list;

    while (*pptr) {
        printf ("++ %s\n", *pptr);
        pptr++;
    }

    return 0;
}

/* returns the string between 2 pointers (not forgetting to append NULL_TERM */
char *getStr(char *start, char *end)
{
    int length=end-start;
    int i=0;
    char *result = (char *)malloc (sizeof (char) * (length+1));

    while (i < length) {
    result[i] = *start;
    start++;
    i++;
    }

    result[i] = NULL_TERM;

    return result;
}

int main (int argc, char ** argv)
{
    char *input = "something;in;the;way;she;moves";
    char reject = ';';

    char *start = input;
    char *end = input;

    char *token_list[MAX_LINE_LEN] = {NULL};
    int i=0;

    /* Is this predicate okay? I read that certain compiler might raise warnings here */
    while (end=strchr(start, (int)reject)) {

        token_list[i]=getStr(start, end);

        i++;
        end++;
        start=end;
    }

    /* It feels awkward to add one more token after the while loop. Is it standard? Should/can I avoid this? */
    token_list[i]=start;
    printcharlist(token_list);

    return EXIT_SUCCESS;
}

I am interested in any kind of feedback I can get. But if I absolutely must ask questions it would be:

Would something make you react badly if someone sent you this code in a patch/pull-request? If yes, what?
Consider this code is going to production. What are the edge cases that I miss and how can I solve them?

Module Design

The trouble with your design is that you need to know how it works before you can use it. This is not true for strtok (OK to a slight extent it is true for strtok).

When designing a software system the user should not need to know how it operates, but rather the interface to the system. Doing it this way has two distict advantages.

User does not need to learn how it works thus making it easier for them to use.
This means it is easier to distribute in binary form if you want.
User is less likely to break it by making assumptions that do not hold (as they have not read the code to understand it).
You can change how it works without affecting the user code.

Thus I would have provided this functionality with a couple of functions and some opaque data types that the user never needs to know or understand.

String Copying

while (i < length) {
result[i] = *start;
start++;
i++;
}

This is done in the std lib look up strncpy().

Also this can be written much more succinctly as:

for(loop = 0; loop < length; ++loop)
{
   result[loop] = start[loop];
}

Code Review

I don't think you need to define this.

#define NULL_TERM '\0'

Putting the literal '\0' into your code is very readable and people know what it means. The macro NULL_TERM may seem self explanatory but I would go look it up.

The function printcharlist assumes the list is NULL terminated.

int printcharlist(char **tok_list)

This is asking for people to do it incorrectly (and forget the NULL terminator). You even mention yourself that this is akward:

/* It feels awkward to add one more token after the while loop. Is it standard? Should/can I avoid this? */

I would change the interface to pass the number of values in the list:

int printcharlist(char **tok_list, size_t count)

Then you make people explicitly fill in the length and they can not accidentally pass you an array that will cause problems.

Alternative design

One advantage of strtok() of your function is that it does not parse the whole string. It only fetches the next token. Thus in a situation where you only need the first couple of tokens it is much more efficient.

I would change your algorithm so that it behaves in a similar manor. Retrieve one string (and store it in your data structure). Next call retrieves the next string etc (this would be a lot easier to change it you had made a modular design).

Questions

  Would make you react badly if someone sent you this code in a patch/pull-request? If yes, what?

No. But I would want to see the test cases the validate it.

  Consider this code is going to production. What are the edge cases that I miss and how can I solve them?

I would set up some unit tests.

""
";"
";;"
";;;"
"Some Text;"
"Some Text;;"
";;SomeText;;"
"Some\nText"
"Some\nText;"
"Some\nText;;"
"Some\n;Text"
"Some\n;;Text"
"Some;\n;Text"
"Some;\n;;Text"
"Some;;\n;Text"
"Some;;\n;;Text"


Given an integer n, generate the nth count and say sequence I am solving interview questions from here.

  Problem : Given an integer n, generate the nth count and say sequence.
  The count-and-say sequence is the sequence of integers beginning as follows:  1, 11, 21, 1211, 111221, ...

  1 is read off as one 1 or 11.

  11 is read off as two 1s or 21.

  21 is read off as one 2, then one 1 or 1211.

  Note : The sequence of integers will be represented as a string.

  Example: if n = 2, the sequence is 11.

How should I further optimize my solution?

def count_and_say(num):
        string = '11'
        cnt = 0
        new_string = []
        if num == 1:
            return "1"
        while num != 2:
            cnt += 1
            for i in range(len(string)):
                if i+1 != len(string):
                    if string[i] == string[i+1]:
                        cnt += 1
                    else:
                        new_string.append(str(cnt)+string[i])
                        cnt = 1
                else:
                    new_string.append(str(cnt)+string[i])
            cnt = 0
            num -= 1
            string = ''.join(new_string)
            new_string = []
        return string

In each step of the "count-and-say sequence" (which is more usually called the "look-and-say sequence") you have to find the groups of consecutive runs of identical digits. So if you have the value 111221, these groups are 111, 22, and 1. Python has a built-in function itertools.groupby for finding groups in an iterator, and using this function, the look-and-say step becomes:

new_value = ''
for digit, group in groupby(value):
    count = sum(1 for _ in group)
    new_value += str(count) + digit
value = new_value

which can be rearranged into a single expression:

value = ''.join(str(sum(1 for _ in group)) + digit
                for digit, group in groupby(value))

and so the whole sequence can be generated like this:

from itertools import groupby

def look_and_say(value='1'):
    "Generate the look-and-say sequence starting at value."
    while True:
        yield value
        value = ''.join(str(sum(1 for _ in group)) + digit
                        for digit, group in groupby(value))

To select only the \$n\$th value of the sequence, use itertools.islice:

next(islice(look_and_say(), n - 1, None))


Backbone.js App approach not very scalable - nested views I'm creating a Backbone App, and although everything seems to work i'm afraid i might need to refactor very soon if I keep adding functionalities and views with this approach. I'm using Parse in the backend, meaning that the object Parse is equivalent to the Backbone.Model. I can't control my models since they've been optimized for mobile and here i need to display only certain key values of 3 different models.

Although there's nothing "wrong" (it works) it's only a matter of time until i start doing 'bigger' mistakes.

Here's my Router:

App.Controllers.Documents = Backbone.Router.extend({
 routes:{
    "story/:id" : "fullStory",
    "" : "index"
 },
// ...
 fullStory: function(id){
  var self = this;
  var query = new Parse.Query(Steps);
  query.equalTo("story", id)
  var steps = query.collection();

  steps.fetch({
   success: function() {

     new App.Views.Steps({ collection: steps });

     var title = new Story({ objectId: id });
     title.fetch({
          success: function(model, resp) {
          new App.Views.Title({ model: title});
          var idUser = title.toJSON().idUser;
          var user = new Parse.User({ objectId: idUser });
          user.fetch({
              success: function(){
                     // I just need the username key here...
                 new App.Views.Username({model:user,el:$("#user")});
              },
              error: function(){
                 new Error({ message: 'Could not find the user you requested.' });
              }
           })
         },
          error: function() {
            new Error({ message: 'Could not find that story.' });

           }
      })
    },
    error: function() {
      new Error({ message: "Error loading story." });
    }
  });
}

Given the way objects have been created in Parse I need to access 3 different objects, that's why i'm rendering the View in "parts". I'd like this function to render just a FullStory View with these 'sub views' inside of it but i'm unsure on how to do it 'cause the Parse.Objects need a reference to the other object -> var user = new Parse.User({ objectId: idUser }); where idUser is a key from another object.

Finally my Views:

App.Views.Steps = Backbone.View.extend({
 tagName : "ul",
 className: "steps",
 initialize: function() {
 this.render();
},
 render: function() {
   this.$el.html(_.template($("#story_steps").html())({ collection: this.collection }));
   $('#steps_wrapper').html(this.el);
 },
});

App.Views.Title = Backbone.View.extend({
 initialize: function() {
 this.render();
},
 render: function() {
  this.$el.html(_.template($("#story_title").html())({ model: this.model }));
  $('#title').html(this.el);
 }
});
App.Views.Username = Backbone.View.extend({
 initialize: function() {
 this.render();
},
 template: _.template('<%= name %>'),

  render: function() {
   this.$el.html(this.template(this.model.toJSON()));
  }
});

The success function for the user.fetch statement is annoying. Just messing around, I am not sure this is any better as a whole (or if it even works):

var Util = {
    error: function (msg) {
        return function () { new Error({ message: msg }); };
    }
};

var controllers = {};

_.extend(controllers, Backbone.Events);

var events = {
    fullStory: 'fullStory',
    user: 'user'
};

controllers.on(events.fullStory, function (id) {
    var query = new Parse.Query(Steps),
        steps;
    query.equalTo("story", id);
    steps = query.collection();

    steps.fetch({
        success: function () {
            new App.Views.Steps({ collection: steps });
        },
        error: Util.error("Error loading story.")
        }
    });
});

controllers.on(events.fullStory, function (id) {
    var title = new Story({ objectId: id });
    title.fetch({
        success: function(model, resp) {
            var idUser;
            new App.Views.Title({ model: title });
            idUser = title.toJSON().idUser;
            controllers.trigger(events.user, idUser);
        },
        error: Util.error('Could not find that story.')
    });
});

controllers.on(events.user, function (idUser) {
    var user = new Parse.User({ objectId: idUser });
    user.fetch({
        success: function () {
            // I just need the username key here...
            new App.Views.Username({model:user, el:$("#user")});
        },
        error: Util.error('Could not find the user you requested.')
    });
});

App.Controllers.Documents = Backbone.Router.extend({
    routes: {
        "story/:id" : "fullStory",
        "" : "index"
    },

    fullStory: function (id) {
        controllers.trigger(events.fullStory, id);
    }
});


Discretizing an n-dimensional line I have a line residing in some n-dimensional space, that needs to be discretized into a set of points. Given the start, end points, and some the number of points r that  are needed, I can accomplish this as follows:

import numpy as np

start, end = (0, 0, 0), (10, 10, 10)
r = 3
dl = zip(*[np.linspace(s_i, e_i, r) for s_i, e_i in zip(start, end)])

This seems to work well enough, but I'd rather the entire thing work as a generator, and right now now the linspace has to be computed for the whole thing. How could I accomplish this?

NumPy works best if you represent all your data as uniform arrays. Then you can vectorize your operations: that is, arrange for each operation to run uniformly on all the elements of an array. This allows NumPy to delegate the operations to fast branchless native code.

Assuming these considerations applies to your case, then what you need here is to convert your input to NumPy arrays:

>>> start = np.array((0, 0, 0))
>>> end = np.array((10, 10, 10))
>>> r = 3

and then construct an array of points along the line:

>>> start + np.linspace(0, 1, r)[..., np.newaxis] * (end - start)
array([[ 0.,  0.,  0.],
       [ 5.,  5.,  5.],
       [10., 10., 10.]])


Unmanaged byte array to managed structure I have a need to turn byte arrays into various structures.

First version:

public static object ConvertBytesToStructure(object target, byte[] source, Int32 targetSize, int startIndex, int length)
    {
        if (target == null)
            return null;

        IntPtr p_objTarget = Marshal.AllocHGlobal(targetSize);
        try
        {
            Marshal.Copy(source, startIndex, p_objTarget, length);
            Marshal.PtrToStructure(p_objTarget, target);
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
        }
        finally
        {
            Marshal.FreeHGlobal(p_objTarget);
        }

        return target;
    }

I have found that when i calling to the first version a lot of times in a second - i getting poor performance.

So i trying to improve that to the version 2:

  private static T ReadUsingMarshalUnsafe<T>(byte[] data, int startIndex, int length)
        {
            byte[] fixedData = new byte[length];
            unsafe
            {
                fixed (byte* pSource = data, pTarget = fixedData)
                {
                    int index = 0;
                    for (int i = startIndex; i < data.Length; i++)
                    {
                        pTarget[index] = pSource[i];
                        index++;
                    }
                }

                fixed (byte* p = &fixedData[0])
                {
                    return (T)Marshal.PtrToStructure(new IntPtr(p), typeof(T));
                }
            }
        }

I have found that this version getting very good performance..

But i want to getting your code review - maybe i'll have any memory leak? maybe i can to do this with efficent another way?

Thanks!

Expanding on my comment; below you'll find a very simple program that compares the method I suggested with your original example. The results on my machine show that the MemoryMarshal class is about 85x faster. You might want to experiment a bit and try running a similar test with a larger struct; maybe your method is faster for the specific problem that you're trying to solve.

Comparison Code:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Runtime.InteropServices;

public readonly struct SomeStruct
{
    private readonly ulong m_x;
    private readonly ulong m_y;
    private readonly ulong m_z;

    public ulong X => m_x;
    public ulong Y => m_y;
    public ulong Z => m_z;

    public SomeStruct(ulong x, ulong y, ulong z) {
        m_x = x;
        m_y = y;
        m_z = z;
    }
}

public class Race
{
    private readonly byte[] m_data = new byte[] {
        0, 0, 0, 0, 0, 0, 0, 0,
        1, 0, 0, 0, 0, 0, 0, 0,
        1, 255, 0, 0, 0, 0, 0, 0,
    };

    [Benchmark(Baseline = true)]
    public SomeStruct A() => MemoryMarshal.Read<SomeStruct>(m_data);
    [Benchmark]
    public SomeStruct B() => Program.ReadUsingMarshalUnsafe<SomeStruct>(m_data, 0, m_data.Length);
}

class Program
{
    static void Main(string[] args) {
        var summary = BenchmarkRunner.Run<Race>();

        Console.ReadKey();
    }

    public static T ReadUsingMarshalUnsafe<T>(byte[] data, int startIndex, int length) {
        byte[] fixedData = new byte[length];
        unsafe {
            fixed (byte* pSource = data, pTarget = fixedData) {
                int index = 0;
                for (int i = startIndex; i < data.Length; i++) {
                    pTarget[index] = pSource[i];
                    index++;
                }
            }

            fixed (byte* p = &fixedData[0]) {
                return (T)Marshal.PtrToStructure(new IntPtr(p), typeof(T));
            }
        }
    }
}

BenchmarkDotNet Results:


Split Circular linked list into two halves I want to improve this code. I want to enter all values in linked list at a time like circular_linked_list<int> cll1(5, 10, 18, 25);

#include <iostream>
#include <utility>

template <class T>
class circular_linked_list
{
  struct Node
  {
    T data;
    Node * next;
    Node(T&& value) : data(std::move(value)), next(nullptr) {}
    /*
    //to enter any number of arguments
    template <typename... Args>
    Node(Args&&... args) : data(std::forward<Args>(args)...), next(nullptr)
                         {}
                         */
  };
  Node *head;

public:
  circular_linked_list() : head(nullptr) {}
  circular_linked_list(const circular_linked_list& cll) = delete; //copy constructor
  circular_linked_list(circular_linked_list&& cll) = delete; //move constructor
  circular_linked_list& operator=(const circular_linked_list& cll) = delete; //copy assignment
  circular_linked_list& operator=(circular_linked_list&& cll) = delete; //move assignment
  ~circular_linked_list();

  void insert_node(T&&);

  /*
  template <typename... Args>
  void insert_node(Args&&... args)
   { ..
     code
     ..
   } */

  Node *split_list()//Returns the mid-node
  {
    Node *slow_ptr = head;
    Node *fast_ptr = head;

    //If odd number of nodes then fast_ptr->next = head
    //If even number of nodes then fast_ptr->next->next = head
    while(fast_ptr->next != head && fast_ptr->next->next != head)
    {
      fast_ptr = fast_ptr->next->next;
      slow_ptr = slow_ptr->next;
    }
    return slow_ptr;
  }

  void print_split_lists();
  void print_list(Node*);
  void print_list()
  {
    print_list(head);
  }
};

template <class T>
void circular_linked_list<T>::insert_node(T&& value)
{
  Node *node = new Node(std::move(value));
  if(head == nullptr)
  {
    node->next = node;
    head = node;
    return;
  }
  Node *tmp = head;
  while(tmp->next != head)
  {
    tmp = tmp->next;
  }
  tmp->next = node;
  node->next = head;
}

template <class T>
void circular_linked_list<T>::print_split_lists()
{
  Node *head1 = nullptr;
  Node *head2 = nullptr;
  Node *mid_node = split_list();
  Node *tail = head;

  while(tail->next != head)
  {
    tail = tail->next;
  }

  if(head->next != head)
  {
  head2 = mid_node->next;
  }

  tail->next = head2;

  head1 = head;
  mid_node->next = head1;
  std::cout << "First half of list \n";
  print_list(head1);
  std::cout << "Second half of list \n";
  print_list(head2);
}

template <class T>
void circular_linked_list<T>::print_list(Node *head)
{
  Node *tmp = head;
  while(tmp->next != head)
  {
    std::cout << tmp->data << ' ';
    tmp = tmp->next;
  }
  std::cout << tmp->data << '\n';
}

template <class T>
circular_linked_list<T>::~circular_linked_list()
{
  Node *tmp = nullptr;
  Node *tail = head;
  while(tail->next != head)
  {
    tail = tail->next;
  }
  tail->next = nullptr;

  while(head != nullptr)
  {
    tmp = head;
    head = head->next;
    delete tmp;
  }
}

int main()
{
  //circular_linked_list<int> cll1(5, 10, 18, 25);
  circular_linked_list<int> cll1;
  cll1.insert_node(2);
  cll1.insert_node(3);
  cll1.insert_node(4);
  cll1.insert_node(1);
  cll1.insert_node(0);
  cll1.print_list();
  cll1.print_split_lists();
}

This is an interesting problem! I don't know that I've ever needed to split a linked list before. I'd love to know whether this is for solving a real-world problem or if you're just doing it for fun. Here are some thoughts on it.

Naming

The first thing I'd do to improve this code would be to fix the naming. That's almost always a good first step for a code review, but in this case, I think it's imperative as some of your methods are deceptively named, implying they do something they do not. For example, the split_list() method does not split the list. Instead, it finds the middle node in the list. It should be called something like get_middle_node(). You even have a comment next to it saying that it does something other than its name. That's a big clue that it's named incorrectly.

Inside the method, there are 2 pointers named slow_ptr and fast_ptr. I had to read the code about 5 times before I understood that the "fast" pointer was incrementing by 2 and the "slow" pointer was incrementing by 1. The names don't even make sense, as pointers don't have a speed. They both access their data in the same amount of time! I would rename them to something like advance_by_1 and advance_by_2. And I would probably also leave a comment explaining that when advance_by_2 hits the head, advance_by_1 will be pointing to the mid node, because that's a fairly clever way of finding the middle node. I don't think it's so clever that you shouldn't do it, but I think you should clarify with a comment.

The name of print_split_lists() is accurate, but I was confused because I thought a caller would end up getting back 2 lists, or that the method would need to be passed the second list, or something. The method doesn't actually split the list - it makes some temporary variables that act like 2 lists, and then prints those. And if I'm not mistaken, it leaves the list in a bad state. That's confusing to me. What would be the use case where I would want to print a single list as 2 lists, but then not actually end up with 2 lists? This seems like a method that's not needed. I'd think you'd want to have a method to split the list in 2 (and return 2 lists), and then just call print_list on each of those 2 new lists.

You ask:

  How can I write a function which returns two lists and how to use it in main() function?

There are a couple of ways to handle this. You can:

Write a function that finds the middle node, constructs a new list with the second half of the old list, and sets the old list to be the first half
Write a function that creates 2 new lists and fills them with the 1st and 2nd half of the original list, respectively.

Let's assume for now that version 1 is acceptable. I'd do it like this (not tested) :

void circular_linked_list::split_list_at_middle(circular_linked_list& secondHalf)
{
    Node* tail = find_tail_node();
    Node* mid = find_middle_node();

    // Move the second half of the list into a new list
    second_half.head = mid->next;
    tail->next = second_half.head;

    // Set our list to only be the first half
    mid->next = head;
}

Note that I left out the code to check for empty lists and lists with only a single node. You'll need to handle those, too.

Then in main(), you call it like this:

circular_linked_list secondHalf;
originalList.split_list_at_middle(secondHalf);
std::cout << "First half of list \n";
originalList.print_list();
std::cout << "Second half of list \n";
secondHalf.print_list();

Performance

One of the main weaknesses of linked lists is random access. If you need to find the nth node, you need to iterate over n nodes to get there. So finding the middle node will always require iterating over (list length) / 2 nodes. And finding the tail node always requires iterating over (list length) nodes. Unless you know you'll need to do those 2 things and prepare for it.

One common way to make list insertion very fast (O(1)) is to keep not only a head node, but a tail node. (Some people will just keep a tail node, and then have a method to return the head node that just returns tail->next. It's not worth the tradeoff in my opinion, but you may disagree.) Now an insertion at the end of the list just involves:

newNode->next = tail->next; // or just head
tail->next = newNode;

No searching the list for the end first.

So why can't we do the same thing with the middle node? If you keep a midNode pointer and keep track of the length of the list, you can do something similar. Every time you do an insertion, you increment the length counter. Every time you do a deletion, you decrement it. When the length counter is even during an insertion, increment the midNode pointer to point to midNode->next. If you want to keep it up-to-date during deletions, you need to have a doubly-linked list or you need to walk the list and find the previous node. It is often the case that you need to walk the list to find the node to delete, anyway, so that's not too onerous. It's a little trickier on delete because you need to know whether the node you're deleting is ahead of or after the mid node. But it turns out that your class doesn't actual handle deletions!

Handle Deletions

This class doesn't handle deleting a node. That seems like a pretty big thing that a user of this class will want to do. It's fairly rare to want to be able to insert but not remove in a list, so I recommend adding such a method.


Selecting a subset of the data from a list I have a list called all_dates which contains timestamps for a number of days (hourly). I wanted to select each day (i.e every 24 items in the list) and pass it to a function.

Here is my implementation that works:

count = 0
while count < len(all_dates)-1:
    day_list = [all_dates[count]]
    for i in xrange(23):
        count += 1
        day_list.append(all_dates[count])
    print_dates(day_list)
    count += 1

I know this is probably not the best way it can be done in Python so I wanted some help to improve my implementation.

It looks like you want to split a large list of items to equal sized chunks.
I suggest using a helper generator:

def chunks(items, size):
    for start in range(0, len(items), size):
        yield items[start:start+size]

Using this, your main task becomes simply:

for day_list in chunks(all_dates, 24):
    print_dates(day_list)


Efficiency of Recursive Checkers Legal Move Generator I'm implementing a checkers engine for a scientific experiment. I found out through profiling that this is one of the functions that takes up a lot of time. I'm not looking for an in-depth analysis, because I don't expect you to dive super deep into my code.

Just: are there any obvious inefficiencies here? Like, is it slow to do those starting for loops (dx, dy) just for 2 values each?

def captures(self, (py, px), piece, board, captured=[], start=None):
    """ Return a list of possible capture moves for given piece in a
        checkers game.

        :param (py, px): location of piece on the board
        :param piece: piece type (BLACK/WHITE|MAN/KING)
        :param board: the 2D board matrix
        :param captured: list of already-captured pieces (can't jump twice)
        :param start: from where this capture chain started.
        """
    if start is None:
        start = (py, px)
    # Look for capture moves
    for dx in [-1, 1]:
        for dy in [-1, 1]:
            dist = 1
            while True:
                jx, jy = px + dist * dx, py + dist * dy # Jumped square
                # Check if piece at jx, jy:
                if not (0 <= jx < 8 and 0 <= jy < 8):
                    break
                if board[jy, jx] != EMPTY:
                    tx, ty = px + (dist + 1) * dx, py + (dist + 1) * dy # Target square
                    # Check if it can be captured:
                    if ((0 <= tx < 8 and 0 <= ty < 8) and
                        ((ty, tx) == start or board[ty, tx] == EMPTY) and
                        (jy, jx) not in captured and
                        ((piece & WHITE) and (board[jy, jx] & BLACK) or
                         (piece & BLACK) and (board[jy, jx] & WHITE))
                        ):
                        # Normal pieces cannot continue capturing after reaching last row
                        if not piece & KING and (piece & WHITE and ty == 0 or piece & BLACK and ty == 7):
                            yield (NUMBERING[py, px], NUMBERING[ty, tx])
                        else:
                            for sequence in self.captures((ty, tx), piece, board, captured + [(jy, jx)], start):
                                yield (NUMBERING[py, px],) + sequence
                    break
                else:
                    if piece & MAN:
                        break
                dist += 1
    yield (NUMBERING[py, px],)

A couple of little things that caught my eye:

You could simplify this

dist = 1
while True:
    jx, jy = px + dist * dx, py + dist * dy
    dist += 1

to this:

jx, jy = px, py
while True:
    jx += dx
    jy += dy

You don't need this range check

if not (0 <= jx < 8 and 0 <= jy < 8):
    break
if board[jy, jx] != EMPTY:

because, assuming board is a dict indexed by tuple, you can just catch KeyError when out of bounds:

try:
    if board[jy, jx] != EMPTY:
        ...
except KeyError:
    break

Instead of

((piece & WHITE) and (board[jy, jx] & BLACK) or
 (piece & BLACK) and (board[jy, jx] & WHITE))

you could use board[jy, jx] & opponent if you determine the opponent's color in the beginning of the function:

opponent = BLACK if piece & WHITE else WHITE


Polymorphic customer classes for employees and affiliates I was trying to achieve a simple problem :

If the user is an employee of the store, he gets a 30% discount
If the user is an affiliate of the store, he gets a 10% discount

I came up with the following :

The abstract super class :

abstract class Customer {
String cid;
String customer_name;
double fare;

Customer(){
    this.cid="NA";
    this.customer_name="NA";
    this.fare=0.0d;
}
protected String getCid() {
    return cid;
}

protected String getCustomer_name() {
    return customer_name;
}

protected abstract double getFare();
protected abstract void printDetails();
}

The child class :

class CustomerAffiliated extends Customer{

    CustomerAffiliated(String cid,String customer_name,double fare){

        super();
        this.cid=cid;
        this.customer_name=customer_name;
        this.fare=fare;
    }

    @Override
    protected double getFare() {

        return this.fare-((10.0/100.0) * this.fare);
    }

    @Override
    protected void printDetails() {
        System.out.println(this.getCid());
        System.out.println(this.getCustomer_name());
        System.out.println(this.getFare());
    }
}

And like wise the other child class.
Calling the methods something like :

Customer c;
    c=new CustomerAffiliated("1","a",100);
    c.printDetails();

Am I doing it in a right way? I did not preferred composition as it is (is a) relation. Moreover I am able to use all the fields in the parent class.
Is is the correct approach ?

JavaDoc

Your whitespaces are all messed up, sometimes there are spaces, sometimes not, sometimes there's a new line after a function declaration, sometimes not. Be consistent, even better, use a code formatter.

abstract class Customer {
    String cid;
    String customer_name;
    double fare;

Why is everything package-private? That class should most likely be public and the variables private or protected.

String cid;
String customer_name;

Both names are not very good:

The first one is abbreviated, the second isn't, be consistent.
The customer prefix is superfluous, as it is a field in the class Customer.

double fare;

You're representing money with a double, this is dangerous! Use a BigDecimal or a similar class instead.

protected String getCid() {
protected String getCustomer_name() {
protected abstract double getFare();
protected abstract void printDetails();

Why are all these methods protected, shouldn't they be public?

CustomerAffiliated(String cid,String customer_name,double fare){

    super();
    this.cid=cid;
    this.customer_name=customer_name;
    this.fare=fare;
}

There should be a consutructor in the Customer class which accepts these values. This makes sure that less knowledge is used when extending the class, additionally you could transform the values in the original constructor.

@Override
protected void printDetails() {
    System.out.println(this.getCid());
    System.out.println(this.getCustomer_name());
    System.out.println(this.getFare());
}

Printing directly to stdout is bad, you should return a String so that the system which invokes the method can decide where the output goes to. Ideally you could remove this function favor of overriding toString.

Customer c;
c=new CustomerAffiliated("1","a",100);
c.printDetails();

Why assigning it on the next line if you could do it on the same line? Why abbreviating the name of the class? It makes the code only unnecessary hard to read, imagine the following, exaggerated, example:

for (Customer c : l) {
    double f = c.getFare();

    if (f > 0) {
        a.add(c);
    }
}

Not exactly easy to read, is it? You can't take one line and take a guess what's going on. Let's try that again with long names:

for (Customer customer : customers) {
    double customerFare = customer.getFare();

    if (customerFare > 0) {
        customersWithFare.add(customer);
    }
}

Your structure makes somewhat sense, depending on what you're trying to achieve. It's hard to say if this is the best approach without knowing anything about the rest of the system. But as a more generic approach having a Customer class with a discount and type field would be more efficient in the real world (as most of the time the data for customers comes from a database which has now idea about some class structure).

public class Customer
    public String getId()
    public String getName()
    public BigDecimal getFare()
    public BigDecimal getDiscount()
    public CustomerType getType()

public enum CustomerType
    NORMAL
    AFFILIATED

This is not without drawbacks itself, but reduces the need to know about and create different classes. Technically you could also hardcode the discount value into the enum, always depending on what you're trying to achieve.


Extract code that makes class shared_ptr construct-able only I've tried using CRTP, but the forces the class to befriend CRTP Base.

template <typename T>
class SharedConstructable
    : public std::enable_shared_from_this<T>
{
    typedef std::shared_ptr<T> ptr_type;

public:

    struct Ptr
        : ptr_type
    {
        template<typename... Args>
        Ptr(Args&&... args)
            : ptr_type(new T(args...))
        {
        }
    };
};

class SharedOnly
    : SharedConstructable<SharedOnly>
{
private:
    friend class SharedConstructable<SharedOnly>;
    SharedOnly(int) {}
}

Usage:

SharedOnly x(5); // Error Constructor is private
SharedOnly::Ptr(5); // Correct usage;

Is there a cleaner/better way of doing this (without the friendship)?

If I follow you correctly, you're looking for a clean way to have a private constructor that can only be invoked by std::make_shared or std::allocate_shared. Unfortunately this does not appear to be portable or easy, and I don't see a way to avoid your request to avoid friendship. However you can make the friend something that other people cannot easily co-opt. I'd call this mildly better, but it definitely has some downsides.

As this is not portable, the scenario I'm showing here is specific to the compiler I had handy: Visual Studio 2012. First I created a class like this:

class SomeClass
{
public:
    int GetValue() const { return m_val; }
    // optionally provide Ptr like yours?
    // static std::shared_ptr<SomeClass> Ptr(int val) { return std::make_shared<SomeClass>(val); }

private:
    SomeClass(int val) { m_val = val; }
    SomeClass(const SomeClass&); // = delete;
    SomeClass(SomeClass&&); // = delete;
    ~SomeClass() {}

    int m_val;
};

This is enough to prevent usage like SomeClass thing(5); Then I wanted to try to befriend std::make_shared. When I tried to compile code using this definition and std::make_shared<SomeClass>(5), I got an error pointing to what needed access:

  error C2248 [...] cannot access private member [SomeClass::~SomeClass ...]
  while compiling class template member function 'void std::_Ref_count_obj<_Ty>::_Destroy(void)'

After befriending class std::_Ref_count_obj<SomeClass> (I first tried befriending the specific method, but that created worse problems), I also decided to fix the warning by befriending std::_Get_align<SomeClass>:

  warning C4624: 'std::_Get_align<_Ty [= SomeClass]>' : destructor could not be generated because a base class destructor is inaccessible

This left me with the following additions to this class that allows std::make_shared<SomeClass>(5) but not SomeClass obj(5).

class SomeClass
{
    : : :
    // Allow use in VS2012 implementation of std::make_shared
    friend class std::_Ref_count_obj<SomeClass>;
    friend struct std::_Get_align<SomeClass>;
    : : :
};

I'll leave other compilers up to you as you need them. It may be worth collecting a series of macro alternatives that you can put in any class that needs this capability.