Pin an array in memory and construct a Bitmap using that buffer I'm using some resources in the code below to pin an array in memory and construct a Bitmap using that buffer. I'm not sure if I'm handling all of the resourced in the proper way.

I'm interested if the way I'm handling the GCHandle and other resources in the constructor in a safe manner, so there are no resource leaks.

using System.Drawing;
using System.Drawing.Imaging;
using System.IO;
using System.Runtime.InteropServices;

namespace Model
{
    public class FastImage
    {
        private const int BytesPerPixel = 4;
        private readonly byte[] _buffer;
        private const int ExtraSpace = 0;

        public int Width { get; set; }
        public int Height { get; set; }

        public FastImage(int width, int height)
        {
            Width = width;
            Height = height;
            _buffer = new byte[Width * Height * BytesPerPixel];
        }

        public FastImage(Bitmap from)
        {
            Width = from.Width;
            Height = from.Height;
            _buffer = new byte[Width * Height * BytesPerPixel];
            GCHandle handle = GCHandle.Alloc(_buffer);
            try
            {
                var address = Marshal.UnsafeAddrOfPinnedArrayElement(_buffer, 0);
                var stride = BytesPerPixel * Width;
                var pixelFormat = PixelFormat.Format32bppArgb;
                using (var fastBitmap = new Bitmap(Width, Height, stride, pixelFormat, address))
                {
                    using (var fastBitmapGraphics = Graphics.FromImage(fastBitmap))
                        fastBitmapGraphics.DrawImageUnscaled(from, 0, 0);
                }
            }
            finally
            {
                handle.Free();
            }
        }

        public Color GetPixel(int x, int y)
        {
            var i = y * BytesPerPixel + x;
            return Color.FromArgb(_buffer[i], _buffer[i + 1], _buffer[i + 2], _buffer[i + 3]);
        }

        public void SetPixel(int x, int y, Color color)
        {
            var i = y * BytesPerPixel + x;
            _buffer[i] = color.A;
            _buffer[i + 1] = color.R;
            _buffer[i + 2] = color.G;
            _buffer[i + 3] = color.B;
        }

        public void Save(string filename)
        {
            GCHandle handle = GCHandle.Alloc(_buffer);
            try
            {
                var address = Marshal.UnsafeAddrOfPinnedArrayElement(_buffer, 0);
                var stride = BytesPerPixel * Width;
                var pixelFormat = PixelFormat.Format32bppArgb;
                using (var fastBitmap = new Bitmap(Width, Height, stride, pixelFormat, address))
                {
                    fastBitmap.Save(filename);
                }
            }
            finally
            {
                handle.Free();
            }
        }

        public void Save(MemoryStream stream, ImageFormat format)
        {
            GCHandle handle = GCHandle.Alloc(_buffer);
            try
            {
                var address = Marshal.UnsafeAddrOfPinnedArrayElement(_buffer, 0);
                var stride = BytesPerPixel * Width;
                var pixelFormat = PixelFormat.Format32bppArgb;
                using (var fastBitmap = new Bitmap(Width, Height, stride, pixelFormat, address))
                {
                    fastBitmap.Save(stream, format);
                }
            }
            finally
            {
                handle.Free();
            }
        }
    }
}

I cannot complain about your code much. You declare variables as readonly where appropriate and you use the try/finally block to free the resources.

The only things I don't like are these properties

public int Width { get; set; }
public int Height { get; set; }

you don't want them to be settable. The setter should be private.

and the magic numbers here (and in other places).

_buffer[i + 1] = color.R;
_buffer[i + 2] = color.G;
_buffer[i + 3] = color.B;

I find you should use a helper class for them:

static class ColorOffset
{
    public const int Alpha = 0;
    public const int Red = 1;
    public const int Green = 2;
    public const int Blue = 3;
}

_buffer[i + ColorOffset.Red] = color.R;
_buffer[i + ColorOffset.Green] = color.G;
_buffer[i + ColorOffset.Blue] = color.B;


Max Sum Contiguous Subarray This is a maximum sum contiguous problem from interviewbit.com

Problem : Find the contiguous subarray within an array (containing at least one number) which has the largest sum.
For example: Given the array [-2,1,-3,4,-1,2,1,-5,4], the contiguous subarray [4,-1,2,1] has the largest sum = 6.

This is my solution :
def max_sub_array(array):
    """ Finds msub-array with maximum sum and returns the maximum sum and sub-array"""
    max_start, max_stop = 0, 1 # Start, Stop of maximum sub-array
    curr = 0                   # pointer to current array
    max_sum = array[0]         # Sum of maximum array
    current_sum = 0            # sum of current array

    for i, elem in enumerate(array):
        current_sum +=  elem

        if max_sum < current_sum:
            max_sum = current_sum
            max_start = curr
            max_stop = i  + 1
        if current_sum < 0:
            current_sum = 0
            curr = i + 1

    return  max_sum , array[max_start:max_stop]

checking test cases:
assert max_sub_array([-4,-2,-3,-4,-5]) == (-2,[-2]), "Wrong evaluation"
assert max_sub_array([-1]) == (-1,[-1]), "Wrong evaluation"
assert max_sub_array([-5, 1, -3, 7, -1, 2, 1, -4, 6]) == (11,[7, -1, 2, 1, -4, 6]), "Wrong evaluation"
assert max_sub_array([-5, 1, -3, 7, -1, 2, 1, -6, 5]) == (9, [7, -1, 2, 1]), "Wrong evaluation"
assert max_sub_array( [6, -3, -2, 7, -5, 2, 1, -7, 6]) == (8,[6, -3, -2, 7]), "Wrong evaluation"
assert max_sub_array([-5, -2, -1, -4, -7]) == (-1,[-1]), "Wrong evaluation"
assert max_sub_array( [4, 1, 1, 4, -4, 10, -4, 10, 3, -3, -9, -8, 2, -6, -6, -5, -1, -7, 7, 8]) == (25,[4, 1, 1, 4, -4, 10, -4, 10, 3]), "Wrong evaluation"
assert max_sub_array([4, -5, -1, 0, -2, 20, -4, -3, -2, 8, -1, 10, -1, -1 ]) ==  (28, [20, -4, -3, -2, 8, -1, 10]), "Wrong evaluation"

How can this code be optimised ?

Expanding upon my comment:

Here is Kadane's algorithm:

def max_subarray(arr):
    max_ending = max_current = arr[0]

    for i in arr[1:]:
        max_ending = max(i, max_ending + i)
        max_current = max(max_current, max_ending)

    return max_current

print(max_subarray([-4, -2, -3, -4, 5])) # 5

print(max_subarray([4, -5, -1, 0, -2, 20, -4, -3, -2, 8, -1, 10, -1, -1])) # 28

Like your algorithm, it is \$O(n)\$. However, it does fewer operations while looping. You could then alter it to return the array which gives that sum, which shouldn't be too hard.


Extending CGPoint to conform to Hashable For Advent of Code Day 1, I found myself needing to use a tuple as a key in a dictionary.  Seeing that I can't extend tuples in Swift, I decided to extend CGPoint to conform to Hashable:

extension CGPoint: Hashable {
    public var hashValue: Int {
        return hash()
    }

    func hash() -> Int {
        var hash = 23
        hash = hash &* 31 &+ Int(self.x)
        return hash &* 31 &+ Int(self.y)
    }
}

This is based off of an implementation found on Stack Overflow.  Another viable option might be self.x.hashValue << sizeof(CGFloat) ^ self.y.hashValue (from here).

This works if the x and y values of the CGPoint are whole numbers, but the Int truncation gives a lossy conversion from CGFloat to Int which could lead to hash collisions.

How can I improve this implementation, and get floating point numbers to play nicely with a hash value that is an Int?

Or, would the best approach not even be to extend CGPoint at all and to just use a custom Swift container struct or an NSObject subclass?

I can see no advantage of computing the hash value from Int(self.x) and Int(self.y). As you already noticed, truncating the floating point
numbers to integers loses information and therefore causes
hash collisions.
CGFloat is (like all Swift number point types) Hashable, and its
hashValue is just the integer with the same memory representation
(as one can see from the implementation). So all possible CGFloat values have different hash values, which makes x.hashValue, y.hashValue a much better basis for computing a hash value for CGPoint than
Int(self.x) and Int(self.y).
It remains the question how to compute the hash value of the point
from the hash values of its coordinates. Of course there can be no
"best hash" which works for all data sets. But in order to get
at least an idea which might be better or worse, I used the
following simple test:
var hv = Set<Int>()
var count = 0
for i in -200 ..< 200 {
    for j in -200 ..< 200 {
        count += 1
        let p = CGPoint(x: CGFloat(i)/20, y: CGFloat(j)/20)
        hv.insert(p.hashValue)
    }
}

print(count, hv.count)

It computes the hash values of 160,000 points where both
x and y range from -20 to 19.9 in steps of 0.1, and counts
how many different hash values we get.
The referenced hash function from here,
translated to Swift 3, is
public var hashValue: Int {
    return (self.x.hashValue << MemoryLayout<CGFloat>.size) ^ self.y.hashValue
}
// # of points: 160000 hash values:  79976

Your hash function is definitely worse (but we already know why):
public var hashValue: Int {
    var hash = 23
    hash = hash &* 31 &+ Int(self.x)
    return hash &* 31 &+ Int(self.y)
}
// # of points: 160000 hash values:  1249

Using hashValue instead of Int() improves it considerably:
public var hashValue: Int {
    var hash = 23
    hash = hash &* 31 &+ self.x.hashValue
    return hash &* 31 &+ self.y.hashValue
}
// # of points: 160000 hash values:  68827

But this is still not really satisfying.
The Boost library has a hash_combine()
function for exactly this purpose, and its implementation
has one generic variant
template <typename SizeT>
inline void hash_combine_impl(SizeT& seed, SizeT value)
{
    seed ^= value + 0x9e3779b9 + (seed<<6) + (seed>>2);
}

as well as specializations for uint32_t and uint64_t.
Translated to Swift this would be:
func hash_combine(seed: inout UInt, value: UInt) {
    let tmp = value &+ 0x9e3779b9 &+ (seed << 6) &+ (seed >> 2)
    seed ^= tmp
}

It is crucial here to use unsigned integers, otherwise
the right shift seed >> 2 would preserve the sign bit.
The Swift hashValue is a signed integer, however, therefore
we have to convert accordingly:
public var hashValue: Int {
    var seed = UInt(0)
    hash_combine(seed: &seed, value: UInt(bitPattern: x.hashValue))
    hash_combine(seed: &seed, value: UInt(bitPattern: y.hashValue))
    return Int(bitPattern: seed)
}
// # of points: 160000 hash values:  159958

As one can see, this hash function has only 42 collisions, and is
far better than the previous ones (for this data set). It is also relatively simple to compute, and can easily be adapted
for types with more properties.
If that is not good enough then you can try other "hash combine" methods,
for example the the 32-bit or
64-bit specializations from the Boost library (depending on the
size of Int on your platform) and check if that works better.
Another one is
here,
and there are probably much more.
Finally you asked:

Or, would the best approach not even be to extend CGPoint at all and to just use a custom Swift container struct or an NSObject subclass?

It depends. If the tuple represents a "point in space" then using (and
extending) CGPoint seems fine to me. If the tuple represents something
else (and you just chose CGPoint because it happens to have two
properties) then I would prefer to define a custom struct which
clearly shows its purpose. A subclass of NSObject would be a
reference type and should only be chosen if you need reference
semantics. Otherwise value types are preferable.

Update: As of Swift 4.1, the compiler can synthesize the
conformance to Equatable/Hashable if all of its members are Equatable/Hashable, see SE-0185 Synthesizing Equatable and Hashable conformance.
Example:
struct Point : Hashable {
    let x: CGFloat
    let y: CGFloat
}

The actual hash function is an implementation detail, but in my test
it worked excellently: The above test code produced no collisions at all
for the 160,000 points.


"Digit Queries" CSES Solution giving TLE I really need a more accurate way to size inf_string. This code works but gets exponentially more inefficient the bigger max(queries) is, and therefore just can't hold up to the gargantuan numbers that CSES throws into it.
Problem:
Consider an infinite string that consists of all positive integers in increasing order:
12345678910111213141516171819202122232425...
Your task is to process q queries of the form: what is the digit at position k in the string?
The first input line has an integer q: the number of queries.
After this, there are q lines that describe the queries. Each line has an integer k: a 1-indexed position in the string. For each query, print the corresponding digit.
queries = []
for i in range(int(input())):
    queries.append(int(input()))
# TODO FIND A BETTER WAY TO LIMIT inf_string LENGTH
inf_string = [str(x) for x in range(1, max(queries)+1)]
inf_string = "".join(inf_string)
for k in queries:
    print(inf_string[k-1])

Building or iterating the whole string takes far too long. Instead group the numbers by length:
length 1 digit:  1 to 9 -> 9 numbers => 9 digits total
length 2 digits: 10 to 99 -> 90 numbers => 180 digits total
length 3 digits: 100 to 999 -> 900 numbers => 2700 digits total
length 4 digits: 1000 to 9999 -> 9000 numbers => 36000 digits total
etc

The number of digits per group (9, 180, 2700, 36000, etc) are easy to compute. Skip these groups of equal-length numbers until you reach the right group (the remaining k falls into it). Then just compute the right number inside the group and get the right digit from it.
for _ in range(int(input())):
    k = int(input())

    length = 1
    while k > 9 * 10**(length-1) * length:
        k -= 9 * 10**(length-1) * length
        length += 1
    q, r = divmod(k-1, length)
    print(str(10**(length-1) + q)[r])

Takes about 0.03 seconds of the allowed 1 second.
Or keeping 10**(length-1) in a variable for less duplication:
for _ in range(int(input())):
    k = int(input())

    length = first = 1
    while k > 9 * first * length:
        k -= 9 * first * length
        length += 1
        first *= 10
    q, r = divmod(k-1, length)
    print(str(first + q)[r])


hash image url and if valid store image url I am trying to get all image urls which are valid and unique.To make sure i wont end up using the same image again i calculate hash of the image and store it in a text file and read the text file again.if there is any better way to do this, please let me know.Overall any speed optimisation would be really helpful, thanks in advance.

import multiprocessing as mp
import socket
# Set the default timeout in seconds
timeout = 20
socket.setdefaulttimeout(timeout)

from PIL import Image
import hashlib
import os
import pickle

def getImages(val):
    # import pdb;pdb.set_trace()
    #Dowload images
    f = open('image_files.txt', 'a')
    k = open('image_hash.txt', 'a')
    try:
        url=val # preprocess the url from the input val
        local=url.split('/')[-1] #Filename Generation From Global Varables And Rand Stuffs...
        urllib.request.urlretrieve(url,local)
        md5hash = hashlib.md5(Image.open(local).tobytes())
        image_hash = md5hash.hexdigest()
        k.write(image_hash+'\n')
        z = open('image_hash.txt', 'r')
        a = z.readlines()
        image_hash_list = [c.strip('\n') for c in a]
        if image_hash not in image_hash_list:
            os.remove(local)
            f.write(url+'\n')
            print("DONE - " + url)
            return 1
        else:
            os.remove(local)
    except Exception as e:
        # print(e)
        print("CAN'T DOWNLOAD - " + url )
        return 0

files = "Identity.txt"
lst = list(open(files))
lst = [l.replace("\n", "") for l in lst]
lst = lst[::-1]
pool = mp.Pool(processes=12)
res = pool.map(getImages, lst)
print ("tempw")

Here identity.txt file looks like this,

http://url1.com
http://url2.com
...

here's a few comments in no particular order.

Have a look over PEP8. It's Python's style guide which helps keep Python code consistent and readable. Things like naming functions which snake_case, constants as CAPS, proper spacing around operators, commenting style and placement, spacing between imports, etc.

It's recommended to use a context manager for opening files, this prevents them from being left open and the indentation makes it clear when a file is being used. You can open two files at once like so:

with open('file_one.txt', 'a') as file_one, open('file_two.txt', 'a') as file_two:
    ...

Instead of using return to handle errors, just raise the error itself. Catch the specific exception (ValueError, TypeError, etc (not Exception or worse BaseException)) then raise it with a message like so:

except TypeError:
    raise TypeError('Reason for failure message')

If you want to continue having caught the error, you can print the error message and continue instead:

except TypeError as exc:
    print(f'Reason for failure: {exc}; continuing ...')

A better way of storing lines from a file to a list would be to use readlines() and strip(). readlines() converts a file to a list where each item is a line in the file, and strip() can remove unwanted linebreaks and whitespace.

Wrap the code you're going to run with:

if __name__ == '__main__':
    ...

This stops any old code being executed if importing it as a module.

Why do you take val as an argument then immediately rename it to url? Just take url as the argument, it's more descriptive. You've also used url in your except block where it may not have been defined. It wouldn't fail in this example but it could have done if you wrote code like this elsewhere.

Your variable names could be much improved. No single letter variables! Make them descriptive and avoid putting their type in the name. k -> hash_file; image_hash_list -> image_hashes; etc.

You're importing pickle but not using it. You're also using urllib without importing it.

You're calling os.remove(local) regardless of the if condition. Put this outside the if check.

There are a few other nitpicks and I can't speak to what the code is actually doing as it's not something I'm familiar with, but I hope that helps.


Filtered table in React.js I am doing a filtered table in react, and I came up with the following code:

import React from 'react';
import { render } from 'react-dom';
import RowFilter from './RowFilter';
import List from './List';
import movieList from './movielist.json';

const data = movieList;
const App = () => (
  <RowFilter data={data}>
    <List/>
  </RowFilter>
);

render(<App />, document.getElementById('root'));

The implementation details are not that important since this is only a rough example, but the general idea is the following:

List normally expects a data array property so that it knows what to render

RowFilter has an input element and keeps track of a list of data. When the value changes, it updates the list of data.

RowFilter also sets the data property for it's child, whatever that child is

The result code works as expected, but I'm wondering about whether this is hard to read or considered a bad practice in general.

Running example

Yeah it's hard to read and only by looking at the source for RowFilter can you understand that it's injecting props into the child component that is given to it. Instead of passing it in as a child, you should pass it in a component prop.

<RowFilter data={data} component={List} />

Then in RowFilter.js, the render simply looks like this. Note the component prop gets renamed to uppercase.

<Component data={this.state.filteredData} />

Here's an updated example on codesandbox


Multithreaded app for writing to a MySQL database I have a multi-threaded app (WCF) that writes to a MySQL db. I want to use MySQL's built in connection pooling abilities.

As far as I can tell, to do that, I should set MySqlConnectionStringBuilder.MinimumPoolSize to some value approximately equal to the number of threads I connect. Then Open/Close the connection for each call to the db. Is this correct? If not, what is the proper way to use pooling?

Here is the function I use to send data to the db. It gets called by many threads throughout the day. Hundreds, maybe thousands of times per day.

private static void execute(MySqlCommand cmd)
{
    try
    {
        MySqlConnectionStringBuilder cnx = new MySqlConnectionStringBuilder();
        cnx.Server = MySqlServer;
        cnx.Database = Database;
        cnx.UserID = UserId;
        cnx.Password = Pass;
        cnx.MinimumPoolSize = 100;
        cmd.Connection = new MySqlConnection(cnx.ToString());
        cmd.Connection.Open();
        cmd.ExecuteNonQuery();
    }
    catch (MySqlException e)
    {
        System.Console.WriteLine(e.Message);
    }
    finally
    {
        if (null != cmd.Connection)
        {
            cmd.Connection.Close();
        }
    }
}

Yes, your code will use pooling.

You might, for the sake of server efficiency, use a smaller MinimumPoolSize (say, 20), and a larger MaximumPoolSize (something a little larger than your maximum number of threads).

You probably don't need all those connections all the time.  If your threads do anything significant between their uses of the function you've shown, a significant number of your connections will be idle.

Unless you're sure you need all those connections all the time, you should reduce the RAM and thread burden on your server with a smaller MinimumPoolSize.


Algorithm that receives a dictionary, converts it to a GET string, and is optimized for big data I found this question online as an example from a technical interview and it seems to be a flawed question in many ways. It made me curious how I would answer it.  So, If you were on a technical Python interview and asked to do the following:

  Write an algorithm that receives a dictionary, converts it to a GET string, and is optimized for big data.

Which option would you consider the best answer? Any other code related comments are welcome.

Common:

import requests
base_url = "https://api.github.com"
data = {'per_page': 10}
node = 'users/arctelix/repos'

Option 1:

My first thought was just answer the question in the simplest form and use pagination to control the size of the data returned.

def get_query_str(node, data=None):
    # base query
    query_str = "%s/%s" % (base_url, node)
    # build query params dict
    query_params = "&".join(["%s=%s" % (k,str(v))
                             for k, v in data.items()])
    if query_params:
        query_str += "?%s" % query_params
    return query_str

print("\n--Option 1--\n")
url = get_query_str(node, data)
print("url = %s" % url)

Option 2:

Well, that's not really optimized for big data and the requests library will convert a dict to params for me. Secondly, a generator would be a great way to keep memory in check with very large data sets.

def get_resource(node, data=None):
    url = "%s/%s" % (base_url, node)
    print("geting resource : %s %s" % (url, data))
    resp = requests.get(url, params=data)
    json = resp.json()
    yield json

print("\n--Option 2--\n")
results = get_resource(node, data)
for r in results:
    print(r)

Option 3:

Just in case the interviewer was really looking to see if I knew how join() and a list comprehension could be used to convert a dictionary to a string of query parameters. Let's put it all together and use a generator for not only the pages, but the objects as well. get_query_str is totally unnecessary, but again the task was to write something that returned a "GET string"..

class Github:
    base_url = "https://api.github.com"

    def get_query_str(self, node, data=None):
        # base query
        query_str = "%s/%s" % (self.base_url, node)
        # build query params dict
        query_params = "&".join(["%s=%s" % (k,str(v))
                                 for k, v in data.items()])
        if query_params:
            query_str += "?%s" % query_params
        return query_str

    def get(self, node, data=None):
        data = data or {}
        data['per_page'] = data.get('per_page', 50)
        page = range(0,data['per_page'])
        p=0
        while len(page) == data['per_page']:
            data['page'] = p
            query = self.get_query_str(node, data)
            page = list(self.req_resource(query))
            p += 1
            yield page

    def req_resource(self, query):
        print("geting resource : %s" % query)
        r = requests.get(query)
        j = r.json()
        yield j

gh = Github()
pages = gh.get(node, data)
print("\n--Option 3--\n")
for page in pages:
    for repo in page:
        print("repo=%s" % repo)

Considered as an interview question, my response would be, "what do you mean by 'optimized for big data'?" Wikipedia says,

  Big data is a term for data sets that are so large or complex that traditional data processing applications are inadequate to deal with them

and it's not clear how this is related to the problem of converting a dictionary to a query string (which would normally be solved by calling urllib.parse.urlencode). So there seems to be a hidden requirement here. Or possibly the interviewer is confused. Either way, you need to go through the requirements capture and analysis process before starting to write code.


Implementing an array backed list I post the below code as review/answer to a question in codereview.
(Custom Dynamic Array)
however, seems the answer itself having issues looking at the comments that I received.
I would appreciate your valuable review comments:

public class DynamicArray<T> : IEnumerable<T>, ICollection<T>
{
private T[] _items;

public DynamicArray()
{
}

public int Count
{
    get
    {
        if (_items == null)
            return 0;
        return _items.Length;
    }
}

public bool IsReadOnly => false;

public void Add(T item)
{
    if(item==null)
         throw new ArgumentException("item");

    if (_items == null)
    {
        _items = new T[1];
    }
    else
    {
        Array.Resize(ref _items,_items.Length+1);
    }
    _items[_items.Length - 1] = item;
}

public void Clear()
{
    _items = null;
}

public bool Contains(T item)
{
    if (_items == null)
        return false;
    return _items.Contains(item);
}

public void CopyTo(T[] array, int arrayIndex)
{
    _items.CopyTo(array, arrayIndex);
}

public IEnumerator<T> GetEnumerator()
{
    if (_items == null)
        return Enumerable.Empty<T>().GetEnumerator();
    return _items.AsEnumerable().GetEnumerator();
   }

public bool Remove(T item)
{
    if (item == null)
        return false;

    if(_items==null)
        return false;

    var temp = _items.Where(i => !i.Equals(item));
    _items = temp.ToArray();

    return true;
}

IEnumerator IEnumerable.GetEnumerator()
{
    return GetEnumerator();
}
}

You have inconsistent spacing around your operators:

if(item==null)

vs

if (_items == null)

I generally prefer to see the second version (as I think is fairly standard in C#).

As you're using Expression Bodied Members. I know you're using C#6. Did you know about the nameof operator?

throw new ArgumentException("item");

Becomes

throw new ArgumentException(nameof(item));

That gives you compile time checking on the name of your parameter.

You should throw the most specific exception type available. If the argument is null and that isn't allowed. throw new ArgumentNullException(nameof(item)).

A list DynamicArray that doesn't allow null values is really odd. There is no reason why you should disallow null. You wouldn't stop me adding 0 to a list of integers.

You resize the array on every Add. That's O(n) and unnecessary. You should be creating an array larger than your current limit and then making it bigger (by some factor) when you reach the limit.

Also, under the covers, an Array.Resize simply creates a new array and copies everything over.

You have a bunch of things like this:

public bool Contains(T item)
{
    if (_items == null)
        return false;
    return _items.Contains(item);
}

Which could be written more simply as:

return _items != null && _items.Contains(item);

You could also use the ?. null propagation operator:

return _items?.Contains(item) == true;

But it's a bit ugly because you have to do something to go from Nullable<bool> to a bool. I achieved that with a simple == true to force an implicit conversion.

Your Remove is incorrect. It should only remove the first occurrence of the item. See msdn.

All your class is doing is keeping an array as a field and replacing it every time you modify the collection. It's not a very efficient way of doing it. As I said, you should be keeping an array with spare capacity that you resize when you need.

That's everything off the top of my head. I'll come back later when I have more time.


Smooth page scrolling in JavaScript I wrote a script to scroll pages when menu buttons are clicked, similar effect like: http://css-tricks.com/examples/SmoothPageScroll

var m1 = document.getElementById('m1'),
    m2 = document.getElementById('m2'),
    m3 = document.getElementById('m3'),
    m4 = document.getElementById('m4'),
    m5 = document.getElementById('m5'),
    m2PositionY = document.getElementById('id2').offsetTop,
    m3PositionY = document.getElementById('id3').offsetTop,
    doneValue = true,
    windowPageYOffset = 0;

function Scroll(ScrollValue1) {
    windowPageYOffset = window.pageYOffset;
    if(windowPageYOffset == ScrollValue1) {
        doneValue = true;
    }
    // scroll up
    else if(windowPageYOffset >= ScrollValue1) {
        window.scrollBy(0, -(1 + ((windowPageYOffset - ScrollValue1) / 10)));
    }
    // scroll down
    else {
        window.scrollBy(0, 1 + ((ScrollValue1 - windowPageYOffset) / 10));
    }
    if(windowPageYOffset != ScrollValue1) {
        var setTimeoutScrollValue1 = setTimeout('Scroll(' + ScrollValue1 + ')', 20);
    }
}

function doneOrNot(doneOrNotValue1) {
    if(doneValue && doneOrNotValue1 != windowPageYOffset) {
        Scroll(doneOrNotValue1);
        doneValue = false;
    }
}
m1.onclick = function () {
    doneOrNot(0); // no need for a id since top is allways 0
}
m2.onclick = function () {
    doneOrNot(m2PositionY);
}
m3.onclick = function () {
    doneOrNot(m3PositionY);
}

I am using a "true or false variable" (doneValue) to determine if previous Scroll is finished before next can start. I wonder if there is any better method to do this in JavaScript?

Please give other feedback if you have any :)

There are a few things you can do differently

Animations are most often accomplished by getting the difference between start and end values, and animating a factor from zero to 1. Multiply that with the difference, and you get an offset.
Instead of relying on an interval (which may be slow, since JS is single-threaded), most animation libraries compare the start time to the current time, to see how far along the animation should be.
Instead of waiting for the animation to stop and a boolean to flip, you can clear the animation's timer, and forcibly stop the animation, before starting a new one. That way the user's clicks aren't ignored, but override previous clicks instead.

Here's an alternative implementation using that approach and other good stuff:

var smoothScrollTo = (function () {
  var timer, start, factor;

  return function (target, duration) {
    var offset = window.pageYOffset,
        delta  = target - window.pageYOffset; // Y-offset difference
    duration = duration || 1000;              // default 1 sec animation
    start = Date.now();                       // get start time
    factor = 0;

    if( timer ) {
      clearInterval(timer); // stop any running animation
    }

    function step() {
      var y;
      factor = (Date.now() - start) / duration; // get interpolation factor
      if( factor >= 1 ) {
        clearInterval(timer); // stop animation
        factor = 1;           // clip to max 1.0
      }
      y = factor * delta + offset;
      window.scrollBy(0, y - window.pageYOffset);
    }

    timer = setInterval(step, 10);
    return timer; // return the interval timer, so you can clear it elsewhere
  };
}());

Here's a demo

Another advantage of always animating a factor between zero and 1 is that you can run it through other expressions, and (for instance) animated the scroll with a sine wave.


Calculating holes on a Tetris grid quickly I have created a Tetris AI which can play Tetris on its own given a set of rules. It places the next piece in every possible way on the board, and calculates which position that gives the fewest "holes" on the playfield.

The playfield is represented by an int[] array, where every bit represents a position in a row, and every int represents a row.

A playfield can look like this:

(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)0111111111
(0000000000000000000000)0111111111
(0000000000000000000000)0111001111
(0000000000000000000000)0111100111

the zeroes in parentheses are not part of the playfield, and are not used. 1 represents an empty square, 0 represents a filled one. So the example above would be a long piece on the left, and a squiggly piece in the middle.

However, to determine if this playfield is "desirable", I calculate the number of holes on the grid. A hole is defined as follows:

An empty square under the topmost filled square in a column.
An empty square in a column next to a column where we have found a filled square, given that the empty square in question is under the topmost filled square in the other column.

I guess it's hard to explain, but I've marked all "holes" here:

(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)1111111111
(0000000000000000000000)0X11111111
(0000000000000000000000)0X11111111
(0000000000000000000000)0X1X00X111
(0000000000000000000000)0X11D00X11

Every hole is marked with an "X", except for one in the bottom row, since that one is counted twice, because it is under a filled square, and next to a square that is  under a filled square.

Right now this is the optimal solution that I've found. It is a combination of JS1's and Brythan's solutions:

private static int EMPTY_ROW = (1<<xsize)-1;

public int calcHolesConverted(int[] grid) {
    //int gridMask      = (1 << xsize) - 1;
    int underMask     = 0;
    int lneighborMask = 0;
    int rneighborMask = 0;
    int foundHoles    = 0;
    int minY = 0;

    while ( minY < ysize && grid[minY] == EMPTY_ROW ) {
        minY++;
    }

    for (int y = minY; y < ysize; y++) {
        int line   = grid[y];
        int filled = ~line & EMPTY_ROW;

        underMask     |= filled;
        lneighborMask |= (filled << 1);
        rneighborMask |= (filled >> 1);

        foundHoles += setOnes[underMask & line];
        foundHoles += setOnes[lneighborMask & line];
        foundHoles += setOnes[rneighborMask & line];
    }
    return foundHoles;
}

This results in over 10000 Tetris pieces per second, compared to the 4800 that I could accomplish before I asked the question.

You can do this faster by doing the whole row at once instead of one bit at a time.  All you need to do is to keep a mask of bits that correspond to your firstFound booleans.  To match the OP's counting scheme (which can triply count a hole), I keep three masks: an underMask, an lNeighborMask, and an rNeighborMask.  underMask is used to count holes underneath the topmost filled square in a column.  lNeighborMask is used to count holes to the left of the column.  rNeighborMask is used to count holes to the right of the column.  (I edited my original answer to account for the triple counting).

The Code

public int calcHolesConverted(int[] grid)
{
    int gridMask      = (1 << xsize) - 1;
    int underMask     = 0;
    int lNeighborMask = 0;
    int rNeighborMask = 0;
    int foundHoles    = 0;

    for (int y = 0; y < ysize; y++) {
        int line   = grid[y];
        int filled = ~line & gridMask;

        underMask     |= filled;
        lNeighborMask |= (filled << 1);
        rNeighborMask |= (filled >> 1);

        foundHoles += setOnes1024[underMask & line] +
                      setOnes1024[lNeighborMask & line] +
                      setOnes1024[rNeighborMask & line];
    }
    return foundHoles;
}

In this code, setOnes1024 is an array of size 1024 which contains the number of 1 bits in the corresponding index.  It is analagous to your setOnes array.  Also, this code assumes that the bits to the left of the leftmost column will all be 0.  If they are not, then you will need to adjust lneighborMask to not contain bits outside the valid columns.

The Timing Test

I ran this timing test by converting both functions to C and running them on the same randomized grid (20000000 iterations each function).

OP's function    : 8.2 seconds
Modified function: 1.4 seconds


Find coprime numbers less than n I get the coprime numbers less than n with the following algorithm:

int n = 15600000;
int j = n - 1;

while (j > 1)
{
   if (GCD(n, j) == 1)
   Console.WriteLine(j);

   j--;
}

My GCD method, which calculates the greatest common divisor of two numbers, looks like this:

static int GCD(int A, int B)
{
   if (B != 0)
       return GCD(B, A % B);
   else
       return A;
}

This algorithm need approximately 103119 milliseconds on my pc. I am interested in a better (faster and little code) solution to this problem.

Do you really need to print the answers to screen?

Most time consumed by the program is for printing each result to screen, using a simple counter or just saving all element in a list (should you need to work with the values, but not actually see them) saves a massive amount of time. Compare my results below:

With printing each value to console - your original code: 00:03:16.008651

static void Main(string[] args)
        {
            Stopwatch sw = new Stopwatch();
            sw.Start();
            int n = 15600000;
            int j = n - 1;

            while (j > 1)
            {
                if (GCD(n, j) == 1)
                    Console.WriteLine(j);

                j--;
            }
            sw.Stop();
            Console.WriteLine("Time elapsed: " + sw.Elapsed);
        }

Adding value to list and then printing: 00:03:29.6715458

static void Main(string[] args)
{
    Stopwatch sw = new Stopwatch();
    sw.Start();
    int n = 15600000;
    int j = n - 1;
    List<int> lst = new List<int>();

    while (j > 1)
    {
        if (GCD(n, j) == 1)
            lst.Add(j);

        j--;
    }

    foreach(int ele in lst)
    {
        Console.WriteLine(ele);
    }

    sw.Stop();
    Console.WriteLine("Time elapsed: " + sw.Elapsed);
}

With counter or list (not printing to console): 00:00:04.5160762

static void Main(string[] args)
{
    Stopwatch sw = new Stopwatch();
    sw.Start();
    int n = 15600000;
    int j = n - 1;
    int counter = 0;

    while (j > 1)
    {
        if (GCD(n, j) == 1)
            counter++;

        j--;
    }
    sw.Stop();
    Console.WriteLine("Time elapsed: " + sw.Elapsed);
}


Securing login PHP script I have made a login system where the inserted_id and inserted_password are sent to the login.inc.php via the XMLHttpRequest. I'm not sure if my PHP script is secure. I need some securing advice for my script.

login.inc.php:

<?php
    session_start();
    $conn = mysqli_connect("localhost", "root", "", "users");
    $params = json_decode(file_get_contents('php://input'), true);
    $inserted_id = $params['inserted_id'];
    $inserted_password = $params['inserted_password'];

    $stmt = mysqli_stmt_init($conn);
    if (mysqli_stmt_prepare($stmt, "SELECT * FROM user WHERE account_name=? OR email=?;")) {
        mysqli_stmt_bind_param($stmt, "ss", $inserted_id, $inserted_id);
        mysqli_stmt_execute($stmt);
        $row = mysqli_fetch_assoc(mysqli_stmt_get_result($stmt));
        if ($row == null) {
            echo ("DOESNT EXISTS");
        } else {
            if (password_verify($inserted_password, $row['password'])) {
                $_SESSION['user_id'] = $row['id'];
                echo("SUCCESS");
            } else {
                echo("PASSWORD_FAIL");
            }
        }
    }
?>

signup.inc.php:

<?php
    $conn = mysqli_connect("localhost", "root", "", "users");
    $params = json_decode(file_get_contents('php://input'), true);
    $inserted_first_name = $params['first_name'];
    $inserted_last_name = $params['last_name'];
    $inserted_dob = $params['dob'];
    $inserted_email = $params['email'];
    $inserted_account_name = $params['account_name'];
    $inserted_password = $params['password'];

    $stmt = mysqli_stmt_init($conn);
    if (mysqli_stmt_prepare($stmt, "SELECT * FROM user WHERE email=?;")) {
        mysqli_stmt_bind_param($stmt, "s", $inserted_email);
        mysqli_stmt_execute($stmt);
        if (mysqli_num_rows(mysqli_stmt_get_result($stmt)) > 0) {
            echo("EMAIL_TAKEN");
        } else {
            $hashed_password = password_hash($inserted_password, PASSWORD_DEFAULT);
            $created_id = rand(111111111, 999999999);
            $stmt = mysqli_stmt_init($conn);
            if (mysqli_stmt_prepare($stmt, "INSERT INTO user(id, first_name, last_name, dob, email, account_name, password) VALUES (?, ?, ?, ?, ?, ?, ?);")) {
                mysqli_stmt_bind_param($stmt, "issssss", $created_id, $inserted_first_name, $inserted_last_name, $inserted_dob, $inserted_email, $inserted_account_name, $hashed_password);
                $result = mysqli_stmt_execute($stmt);
                echo ($result ? "SUCCESS" : "FAIL");
            }
        }
        mysqli_stmt_close($stmt);
    }
?>

The database access information (server address, user name, password, database name) are repeatedly hard coded in each PHP file instead of having them in one common place, where they can be easily changed and configured for each run time environment (development, production, etc.)

Additionally a (production) database should not be accessed using a root user with a blank password.

Personally I never accessed a database in PHP using the built-in low level functions. I found using a database access library lead to less boilerplate code, they had a better readable API, and most importantly built-in security measures, that you can't forget to use. At the very least I would outsource database access into separate repository classes/modules/services.

A common security measure for logins is to have the script return the same result for both unknown account name and wrong password, so that an attacker can't find out if a specific user has an account or not.


Creating a webtrend function to remove duplication using js apply and call How can I re-factor the code to remove duplication and create a common method ?

(function(){

    $("#a", "#main").bind("mouseover", function(){
              var id1 = $("#one").text(),
                  args = ["DCSext.common1","common1","DCSext.common2","DCSext.title","one", "DCSext.ti", id1];

              dcsMultitrack.apply(this, args);
     });

    $("#b", "#cool").bind("click", function(){
           var id2 = $("#two").text(),
               args = ["DCSext.common1","common1","DCSext.common2","DCSext.title", "two", "DCSext.some", id2];
               dcsMultitrack.apply(this, args);
    });

    $("body").delegate("a", "click", function(){
         var id3 = $("#three").text(),
               args = ["DCSext.common1","common1","DCSext.common2","DCSext.new", "what", "DCSext.where", "us"];
               dcsMultitrack.apply(this, args);
    });

    }());

I have some common logs which are almost repeated in all callbacks. I can use a variable like

var commonlogs = ["DCSext.common1","common1","DCSext.common2","common2", "DCSext.common3", "common3" ];

i can use commonlogs.push("DCSext.title","one","DCSext.ti", "two"). But not finding a proper way to re-factoring repeating the DCSext stuff again and again since its very granular level .

Thanks for any advice or suggestions.

There is not much you can do,
the only thing I would suggest is to use concat instead of push, this way you can keep re-using commonLogs, and maybe have 1 commonLogs per group.

So

  var commonLogs = [ [] ];
  commonLogs[1]  = ["DCSext.common1","common1"];
  commonLogs[2]  = commonLogs[1].concat( ["DCSext.common2","common2"] );
  commonLogs[3]  = commonLogs[2].concat( ["DCSext.common3","common3"] );

Then you can

(function(){

  $("#a", "#main").bind("mouseover", function(){
          var id1 = $("#one").text(),
              args = commonLogs[2].concat( ["DCSext.title","one", "DCSext.ti", id1] );
          dcsMultitrack.apply(this, args);
  });

  $("#b", "#cool").bind("click", function(){
       var id2 = $("#two").text(),
           args = commonLogs[2].concat( ["DCSext.title", "two", "DCSext.some", id2] );
       dcsMultitrack.apply(this, args);
  });

  $("body").delegate("a", "click", function(){
     var args = commonLogs[2].concat( ["DCSext.new", "what", "DCSext.where", "us"] );
     dcsMultitrack.apply(this, args);
  });

}());


Sorting dates in this format (DD-MM-YYYY) Is this the correct and an elegant way to sort the dates in the format (DD-MM-YYYY) in ascending order?

let dates = ['01-02-2018', '02-01-2018', '01-01-2018', '31-12-2017'];

dates.sort((date1, date2) => {
    date1 = date1.split('-'), date2 = date2.split('-');
    let day1 = parseInt(date1[0]);
    let day2 = parseInt(date2[0]);
    let month1 = parseInt(date1[1]);
    let month2 = parseInt(date2[1]);
    let year1 = parseInt(date1[2]);
    let year2 = parseInt(date2[2]);
    if (year1 !== year2) {
        return year1 - year2;
    } else if (month1 !== month2) {
        return month1 - month2;
    } else {
        return day1 - day2;
    }
});

There is also this way, as Cris Luengo said in the comments, I can directly compare the strings.

let dates = ['01-02-2018', '02-01-2018', '01-01-2018', '31-12-2017'];

dates.sort((date1, date2) => {
    date1 = date1.split('-').reverse().join('-');
    date2 = date2.split('-').reverse().join('-');
    if (date1 < date2) {
        return -1;
    } else if (date1 > date2) {
        return 1;
    } else {
        return 0;
    }
});

Which way is more performant? How do I measure that?

UPDATE: You can run performance tests I prepared based on various code samples from the answers to this question: https://jsperf.com/cr-se-date-array-sorting/1. When I ran them, I see that my code is second best performing. The best is the one by @Kevin Cline (up voted!).

Here's my take on it. The following code uses the same idea about the string representation of date that is directly comparable for sorting.

It is not possible to tell in advance how performant will this solution be compared to one you presented above. It all depends a lot on how many dates are being sorted. If you deal with tons of dates, there will be many more invocations to the arrow function we're passing to sort(...). Therefore, it's not good to keep translating the date via split() every time our arrow function is being used.

Instead, I recommend three steps:

Translate dates into a sortable representation (one time). Use .map().
Do the sorting with .sort().
The values will be put in a lexicographical order.
Translate the sorted dates back into original representation.

This will guarantee that each date is translated at most twice, which makes the entire solution more performant for large Ns.

Also, Notice that steps #1 and #3 can use same exact implementation, which I extracted.

const reverseDateRepresentation = date => {
  let parts = date.split('-');
  return `${parts[2]}-${parts[1]}-${parts[0]}`;
};

const sortedDates = ['01-02-2018', '02-01-2018', '01-01-2018', '31-12-2017']
  .map(reverseDateRepresentation)
  .sort()
  .map(reverseDateRepresentation);

console.log(sortedDates);

Produces result:

["31-12-2017", "01-01-2018", "02-01-2018", "01-02-2018"]

Little note. I think, that from the "big O" point of view we haven't improve the algorithm. Since .map(reverseDateRepresentation) is \$O(n)\$, the performance of the entire solution is limited by the .sort() (which is probably \$O(n * {\log n})\$). The way we're potentially improving solution is by making sure that the constants in our "big O" cost are as small as we can achieve.

Nevertheless, if we'd put the performance as top criteria of the solution, I'd learn as much as I can about the real data being processed; as well as conducted a thorough performance test.

In real life scenario, however, I personally never put performance above readability, because I believe that lack of readability is eventually the same thing as lack of correctness ...and correctness is almost always more important than performance (few exceptions are known though).


Is the usage of SQL Cursor appropriate here? I'm looking for following kind of feedback:

Firstly, is the use of a cursor an overkill here?
Is there a simpler way to do what I'm doing in SQL?
Could the script go terribly wrong?
I have limited SQL experience, how does it look? Any standards that I'm not following?

Bear in mind that this is a one-time script to be run, so performance is not absolutely crucial. Nevertheless, I would like to have some feedback on whether it could still be done more efficiently.

A few words about what I'm trying to do: There are two tables, tblEmployment & tblUserCodeList. The first table (tblEmployment) has a column of type int named RepaymentType. Due to a requirements change, this has to be abandoned and converted to a specific GUID/uniqueidentifier to be stored in another column RepaymentTypeIdentifier in the same tblEmployment. That is the whole point of the SQL script. I'll leave out further business logic.

So for each row in tblEmployment I want to read its RepaymentType and depending on its value (can only be 1-12) I want to store a specific GUID from tblUserCodeList (column:UserCodelistIdentifier) in the column RepaymentTypeIdentifier.

The column RepaymentTypeIdentifier is thus a Foreign Key referencing UserCodelistIdentifier in tblUserCodelist.

(I have shortened the CASE statement a little bit.)

USE db_name_left_out...
GO

DECLARE @EmploymentId as NVARCHAR(50);
DECLARE @UserCodeList TABLE
(
    Title NVARCHAR(100),
    UserCodeListIdentifier NVARCHAR(50)
)

INSERT INTO @UserCodeList (Title, UserCodeListIdentifier)
    SELECT Title, UserCodeListIdentifier
    FROM tblUserCodeList;

Declare @EmpCursor as CURSOR

SET @EmpCursor = CURSOR FOR
    SELECT EmploymentIdentifier FROM dbo.tblEmployment;

OPEN @EmpCursor;

FETCH NEXT FROM @EmpCursor INTO @EmploymentId;

WHILE @@FETCH_STATUS = 0
BEGIN
    UPDATE dbo.tblEmployment SET RepaymentTypeIdentifier = CASE
            when RepaymentType = 1 then (SELECT TOP 1 UserCodeListIdentifier FROM @UserCodeList WHERE title like '%some_text_1%')
            when RepaymentType = 2 then (SELECT TOP 1 UserCodeListIdentifier FROM @UserCodeList WHERE title like '%some_text_2%')
            when RepaymentType = 3 then (SELECT TOP 1 UserCodeListIdentifier FROM @UserCodeList WHERE title like '%some_text_3%')
            .
            .
            .
            when RepaymentType = 12 then (SELECT TOP 1 UserCodeListIdentifier FROM @UserCodeList WHERE title like '%some_text_12%')
        END
    WHERE EmploymentIdentifier = @EmploymentId

    FETCH NEXT FROM @EmpCursor INTO @EmploymentId;
END

CLOSE @EmpCursor

DEALLOCATE @EmpCursor;

This code can be simplified a lot, and in to a single update without the cursor. This may be a problem, though if your transaction log is not large enough to accommodate a mass update to all your records.

Right, how to simplify the update?

First, build the case statement in to the @UserCodeList logical table... Consider this:

INSERT INTO @UserCodeList (Title, UserCodeListIdentifier, RepaymentTypeToUpdate)
SELECT Title, UserCodeListIdentifier,
       case
           when title like '%some_text_1%' then 1
           when title like '%some_text_2%' then 2
           .....
       end
FROM tblUserCodeList;

Now, in that table, you have the correct RepaymentType cross reference for the update.

Your update now simply becomes:

UPDATE dbo.tblEmployment
SET RepaymentTypeIdentifier = (
    select UserCodeListIdentifier
    from @UserCodeList
    where RepaymentType = RepaymentTypeToUpdate
)

The above update can be done in a few different ways.... a CTE would be my preference:

with UpdateMap as (
    SELECT Title,
           UserCodeListIdentifier,
           case
               when title like '%some_text_1%' then 1
               when title like '%some_text_2%' then 2
               .....
           end as MatchRepayment
    FROM tblUserCodeList;
)
UPDATE dbo.tblEmployment
SET RepaymentTypeIdentifier = UserCodeListIdentifier
FROM UpdateMap
where MatchRepayment = RepaymentType

Having said all that, let's revisit your questions:

Firstly, is the use of a cursor an overkill here?

Probably. If your database transaction log is small, though, it may be better to break your updates in to batches that fit. A single large update may fail, and roll back, if there's not enough space to log it as a single operation. Your cursor breaks it doen to a sinlge record each time, and there's plenty of space for that, though. Ask your DBA, or just try it... if you are uncertain.
Is there a simpler way to do what I'm doing in SQL?

SQL is a set-based language. If you have a set-based way of thinking about it, then my suggestions are 'simpler'. Certainly, it is more concise.
Could the script go terribly wrong?

Yes.... it could. Take a backup first, and verify everything!
I have limited SQL experience, how does it look? Any standards that I'm not following?

Your conventions, for what you are doing, are reasonably good. There's very little in the way of 'standards' for SQL style... consistency is the key factor to look for.


Moving steps in KMP algorithm KMP algorithm is for searching and matching string with a given pattern. The most important part is to build a table for the pattern then move a proper step rather than just one step increment.

In my code I created the table correctly but something is wrong for moving steps.
It stuck in the for loop as variable was always equal to 2.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace KMP
{
    class Program
    {
        const string pattern = "abcdabd";
        const string s = "aadabcdabedxdfabcdabddsa";
        static int[] x = new int[7];
        static void Main(string[] args)
        {
            x = BuildTable(pattern);
            var y = Match(x, s);
            Console.WriteLine(y);
            Console.Read();
        }

        private static int Match(int[] x, string s)
        {
            int n = s.Length;
            int l = x.Length;
            int find = 0;
            Char[] charPattern = pattern.ToCharArray();
            for (int i = 0; i < n; )
            {
                string a = s.Substring(i, l);
                if (a.CompareTo(pattern).Equals(0))
                {
                    return i; // Found match, return match position of the first letter
                }
                // move position by BuildTable
                Char[] charSubstring = a.ToCharArray();
                int count = 0;
                for (int j = 0; j < l; j++)
                {
                    if (charPattern[j] == charSubstring[j])
                    {
                        count++;// count of matched chars
                        continue;
                    }
                    else
                    {
                        i += count - x[j]; // move forward steps = matched count - table value
                        break;
                    }
                }
            }
            return -999; // not found
        }

        public static int[] BuildTable(string p)
        {
            int[] result = new int[p.Length];
            result[0] = 0;
            for (int i = 1; i < p.Length - 1; i++)
            {
                // The substring from p[1] to p[i]
                string s = p.Substring(0, i + 1);

                var prefixes = Enumerable.Range(1, s.Length - 1)
                     .Select(a => s.Substring(0, a)).ToList();
                var suffixes = Enumerable.Range(1, s.Length - 1)
                     .Select(a => s.Substring(a, s.Length - a)).ToList();
                var common = prefixes.Intersect(suffixes).FirstOrDefault();
                result[i] = (common == null) ? 0 : common.Length;
            }
            return result;
        }
    }
}

It is very hard to read your code, since you use single letter variables (x, s, l... etc.).

Some more general concerns: x is a class member, initialized in its declaration, overriden in the Main method (without the initial value ever being used), and then passed as an argument to the Match method.

As for why the value is stuck on 2, my guess is that the line i += count - x[j]; leaves the i value the same - count - x[j] == 0 perhaps you meant to write count + x[j]?


Implementation of Linked List I had implemented a simple Linked list Insert, print and insert at the nth position. It works fine.
Could someone review code and let me know where I could have done better (or) where I had gone wrong?

class Linkedlist
     {
       public Nodes current;
       public static Nodes headnode;
       public static int nodecount;
       public void Addatfirst(object d)
      {

          if (headnode == null)
          {
              headnode = new Nodes(d);
              nodecount++;
           }
          else
          {
              Nodes dummy = new Nodes(d);
              dummy.Next = headnode;
              headnode = dummy;
              nodecount++;
           }
         }

     public void Print()
    {
        current = headnode;

        while (current.Next != null)
        {
            Console.Write("|" + current.data + "| -> ");
            current = current.Next;
            if (current.Next == null)
            {
                Console.Write("|" + current.data + "| -> ");
            }
        }
    }

    public void AddatLast(object d)
    {
        current = headnode;
        while (current.Next != null)
        {
            current = current.Next;
            if (current.Next == null)
            {
                current.Next = new Nodes(d);
                nodecount++;
                return;
            }
        }
    }

    public void AddatNthPosition(object d, int position)
    {
        current = headnode;
        for (int i = 1; i < position; i++)
        {
            current = current.Next;
            if (position - i == 1)
            {
                var temp= new Nodes(d);
                temp.Next = current.Next;
                current.Next = temp;
            }
        }
    }
}
public class Nodes
{
    public object data;
    public Nodes Next;
    public Nodes(object d)
    {
        data = d;
    }
}

There are actually several bugs in your code. Your list is also not very easy to use because it can't be iterated.

Bugs

static fields are shared among class instances. So if you create multiple Linkedlist instances, they will all use the same headnode and nodecount. That is obviously not what you want. Those fields should not be static.
AddatNthPosition doesn't add anything if you pass 0 or 1 as index. Also, it throws a NullReferenceException when given an index that is too large. Throwing an ArgumentOutOfRangeException would be better if the index is too small (negative) or too large.
Only Addatfirst takes into account that the list can be empty. The other add methods fail with a NullReferenceException when called on an empty list.

Problems

Your list cannot be iterated because it does not implement IEnumerable. That makes it difficult to use: instead of using foreach or Linq, calling code is forced to manually walk through its nodes. While you're at it, implementing ICollection (count) and IList (indexing, adding, inserting, removing) may also be useful.
public fields can be modified by other code. That means that some other code could replace the head node of a linked list, or adjust its count. That can easily cause bugs or other unexpected behavior. Classes should hide their internal state and implementation details.
Print may be useful for debugging, but it should not be part of your (final) list class. Classes should (ideally) have only a single responsibility, and 'storing items' and 'displaying things' are different things as far as I'm concerned.
Why is there a current field? It looks like that should be a local variable in some of your methods.

Improvements

Consider making your class generic. This improves type safety and makes your list easier to work with.
Try sticking to the official naming conventions to make your code easier to read. C# code uses PascalCase for class and method names: LinkedList instead of Linkedlist, AddToStart instead of AddtoStart, and so on. camelCase is used for field names: headNode instead of headnode, nodeCount instead of nodecount, and so on.
Try using more descriptive names. value instead of d, newNode instead of dummy, and so on. Also, Nodes is plural, but it represents a single node, so it should be named Node instead.
Look at the collections in the standard library. They often use particular names for their methods, and that's what other programmers will be familiar with. Using different names makes your code harder to use. Consider renaming AddatLast to Add, and combining Addatfirst and AddatNthPosition into a single Insert method (where its first argument is the index, conform the IList interface). Also rename nodecount to Count and make it a get-only property, so it can't be modified by other code.
A Remove and/or RemoveAt method would make your list more useful.
If adding items at the end is important, consider adding a lastNode field to improve performance for that case.
Inconsistent indentation makes code harder to read, and it looks a little sloppy.


Checking winning conditions in a Connect Four game I'm trying to refactor this C# code that uses one true brace style (it's enforced). I'm coming to C# from C++, in the former I could've made x and y to become i and j with reference assignment, but in C# it's not possible.

Should I take the (i_IsRow) condition to a method? it will have 5 parameters and 2 of them will be out parameters, this is definitely code smell...

private int checkRowsOrColsForWin(bool i_IsRow)
{
    int signCounter = 0;
    int x, y;

    for (int i = 0; i < m_Cols; i++)
    {
        for (int j = 0; j < m_Rows; j++)
        {
            if (i_IsRow)
            {
                x = i;
                y = j;
            }
            else
            {
                x = j;
                y = i;
            }

            signCounterLogic(m_Board[x, y], ref signCounter);

            if (Math.Abs(signCounter) == k_FourInARow)
            {
                break;
            }
        }
        if (Math.Abs(signCounter) == k_FourInARow)
        {
            break;
        }
    }
    return signCounter;
}

Don't use Hungarian notation in C#.
The existence of x and y seems pointless. This code:

if (i_IsRow)
{
    x = i;
    y = j;
}
else
{
    x = j;
    y = i;
}
signCounterLogic(m_Board[x, y], ref signCounter);

could just as easily be

if (i_IsRow)
{
    signCounterLogic(m_Board[i, j], ref signCounter);
}
else
{
    signCounterLogic(m_Board[j, i], ref signCounter);
}

  Should I take the (i_IsRow) condition to a method? it will have 5 parameters and 2 of them will be out parameters, this is definitely code smell...

Agreed (though only three parameters if you get rid of x and y), I don't think that warrants a method. The fact that a method (signCounterLogic) encapsulating the actual work done is called in the loop is sufficient I think.
Method names should use PascalCase.
I don't like an int return value for a method called checkRowsOrColsForWin. What does an int value mean? If it only checks whether it won or not then it should return a bool. To me with its current name it should return void. If it is actually returning specific information about the win then the name should be changed to reflect what it checks for and returns.
I don't like the double check for signCounter == k_FourInARow. If the first one is true, the loop breaks, and the same condition is immediately tested again in the outer loop. Unfortunately I've had too much to drink to be able to think of a good way around that.
I don't know what signCounterLogic() does with signCounter, but if it only sets the value then I would prefer that the method returns a value rather than having to pass in a variable by ref. Makes for fewer parameters and a more normal looking method.
Assuming the "sign" in signCounterLogic() isn't a verb, this method should definitely be renamed to say what it's doing.


Simple Rock Paper Scissors in Erlang I'm very new to Erlang and I'm specifically interested in making my Erlang idiomatic (in other words, can you tell I don't know Erlang from this code?) and removing code duplication. I am not as interested in performance or the UI of the "game."
This is a simple interactive Rock, Paper, Scissors game and I've been using erlc rps.erl && ./rps-main.erl to run it.
rps.erl
-module(rps).
-export([play_game/1, get_random_symbol/0]).

get_symbol(1) -> rock;
get_symbol(2) -> paper;
get_symbol(3) -> scissors.

get_random_symbol() ->
    get_symbol(rand:uniform(3)).

play_game(InputFunctions) ->
    Score = [{win, 0}, {loss, 0}, {tie, 0}],
    play_game(Score, InputFunctions).

play_game(ok, _) ->
    ok;
play_game(Score, {Player1Input, Player2Input}) ->
    display_score(Score),
    Input1 = Player1Input(),
    Continue = Input1 =/= quit,
    Input2 = Player2Input(Continue),
    NewScore = compare_results(Input1, Input2, Score),
    play_game(NewScore, {Player1Input, Player2Input}).

compare_results(quit, _, _) ->
    ok;
compare_results(_, quit, _) ->
    ok;
compare_results(Same, Same, [Wins, Losses, {tie, Ties}]) ->
    [Wins, Losses, {tie, Ties+1}];
compare_results(rock, paper, [Wins, {loss, Losses}, Ties]) ->
    [Wins, {loss, Losses+1}, Ties];
compare_results(paper, scissors, [Wins, {loss, Losses}, Ties]) ->
    [Wins, {loss, Losses+1}, Ties];
compare_results(scissors, rock, [Wins, {loss, Losses}, Ties]) ->
    [Wins, {loss, Losses+1}, Ties];
compare_results(rock, scissors, [{win, Wins}, Losses, Ties]) ->
    [{win, Wins+1}, Losses, Ties];
compare_results(scissors, paper, [{win, Wins}, Losses, Ties]) ->
    [{win, Wins+1}, Losses, Ties];
compare_results(paper, rock, [{win, Wins}, Losses, Ties]) ->
    [{win, Wins+1}, Losses, Ties].

display_score([{win, Wins}, {loss, Losses}, {tie, Ties}]) ->
    io:format("Player1 Wins: ~B~nPlayer2 Wins: ~B~nTies: ~B~n", [Wins, Losses, Ties]).

rpsgame.erl
#!/usr/bin/env escript
%% -*- erlang -*-
%%! -pa 'ebin/'
-import(rps, [play_game/1, get_random_symbol/0]).

main(_) ->
    GetPlayerInput = fun() ->
        io:format("Enter rock, paper, scissors, or quit~n"),
        case io:get_line("> ") of
            "rock\n" -> rock;
            "paper\n" -> paper;
            "scissors\n" -> scissors;
            _ -> quit
        end
    end,
    GetCompInput = fun(Continue) when Continue =:= false ->
                        ok;
                    (_) ->
                        Input = rps:get_random_symbol(),
                        io:format("Computer threw ~p~n", [Input]),
                        Input
                    end,
    play_game({GetPlayerInput, GetCompInput}).

In whole not bad, but something can be simplified.

1.
In short functions don't need to create separate variables, especially if it is just a wrapper over the call of inner function. So play game/1 from rps can be rewritten as:

play_game(InputFunctions) ->
  play_game([{win, 0}, {loss, 0}, {tie, 0}], InputFunctions).

2.
Behavior of function for get result from player2 must not depend on result first player. Together with hint from 1 and giving an alias to the name of argument get

play_game(Score, {Player1Input, Player2Input} = P) ->
  display_score(Score),
  case Player1Input() of
    quit -> ok;
    Input -> play_game(new_score(Input, Player2Input(), Score), P)
  end.

where Player2Input/0:

  GetCompInput = fun() ->
    Input = rps:get_random_symbol(),
    io:format("Computer threw ~p~n", [Input]),
    Input
                 end,

new_score in my opinion, better captures the essence of the function than compare_results. This also allow delete first clause in new_score(compare_results)/3.

3.
Usage the symbol - in the name of the module is not very convenient,
 because you must wrap the name in ' ' when call function. Better use _ as delimiter.

4.
Not should use guard,when can use pattern matching. Compare

fun(Continue) when Continue =:= false ->

and

fun(false) ->

5.
Do not create an anonymous argument for a single function clause, in this case it is just an excess parameter.

 main(_) ->

6.
Do not use the directive import since it can will only confuse. If you know
C++, it is possible to draw an analogy with an indicate explicit namespace instead of using namespace.

In addition I did not delete lines

play_game(ok, _) ->
  ok;

and

new_score(_, quit, _) ->
  ok;

although when playing with the computer this function clause does not get called.


Divide and Conquer algorithm for counting inversions I've written a program that counts the number of inversions using a Divide and Conquer algorithm, written in C++11.

#include <iostream>
#include <tuple>
#include <vector>

using IntVec = std::vector<int>;

std::tuple<IntVec, long long> count_inv(IntVec xs);
std::tuple<IntVec, long long> count_split_inv(IntVec xs, IntVec ys);

std::tuple<IntVec, long long> count_inv(IntVec xs) {
    int n = xs.size();

    if (n < 2) return std::make_tuple(xs, 0);

    IntVec::iterator middle = xs.begin() + (n / 2);
    IntVec left(xs.begin(), middle);
    IntVec right(middle, xs.end());

    long long left_inv, right_inv, split_inv;
    IntVec as, bs, cs;
    std::tie(as, left_inv) = count_inv(left);
    std::tie(bs, right_inv) = count_inv(right);
    std::tie(cs, split_inv) = count_split_inv(as, bs);
    return std::make_tuple(cs, left_inv + right_inv + split_inv);
}

std::tuple<IntVec, long long> count_split_inv(IntVec xs, IntVec ys) {
    int n = xs.size() + ys.size();
    IntVec zs(n);
    long long split_inv = 0;

    int i = 0, j = 0, k = 0;
    for (; k < n && i < xs.size() && j < ys.size(); ++k) {
        if (xs[i] <= ys[j]) {
            zs[k] = xs[i];
            ++i;
        } else {
            zs[k] = ys[j];
            ++j;
            split_inv += xs.size() - i;
        }
    }

    for (; i < xs.size(); ++k, ++i)
        zs[k] = xs[i];

    for (; j < ys.size(); ++k, ++j)
        zs[k] = ys[j];

    return std::make_tuple(zs, split_inv);
}

int main() {
    std::vector<int> xs;
    int x;
    while (std::cin >> x) {
        xs.push_back(x);
    }

    IntVec ys;
    long long inv;
    std::tie(ys, inv) = count_inv(xs);
    std::cout << inv << std::endl;
    return 0;
}

I find that the code is very verbose in some areas, such as specifying the type of the tuple. Also, parts of it are confusing. How could I rewrite this code such that it is more concise and clearer?

More clear by using extra argument?
If you passed in a count by reference to each function, and each function incremented this count instead of returning a new count in a tuple, the code might look cleaner.  For example, this:

IntVec as, bs, cs;
std::tie(as, left_inv) = count_inv(left);
std::tie(bs, right_inv) = count_inv(right);
std::tie(cs, split_inv) = count_split_inv(as, bs);
return std::make_tuple(cs, left_inv + right_inv + split_inv);

would become this:
IntVec as = count_inv(left, count);
IntVec bs = count_inv(right, count);
return count_split_inv(as, bs, count);

And in main(), this:
long long inv;
std::tie(ys, inv) = count_inv(xs);

would become this:
long long inv = 0;
count_inv(xs, inv);


Basic domino game architecture I am trying to write a simple domino (five-game) game with 2 players. I must say I am very bad at OOP design, and so I don't want to get myself too deep into coding then realize I have a bad design to start with. I'd appreciate if someone shed some light on how to better design this.

Here is summary of the game:

there are 28 dominos
there will be 2 players
initially, each player receives 7 random tiles, the remaining 14 tiles are left to the bone yard pool
initially, whichever player got the heaviest tile gets to play first
whoever scores x points first wins the game (e.g: 100,150,etc)

Here is what I have so far:

Player class:

public class Player
{
    //properties
    string ID { get; set; } //player id
    int Wins { get; set; }
    int Losses { get; set; }
    bool IsMyTurn { get; set; }
    List<Domino> Hand { get; set; } //each player will have a list of domino tiles

    void ShowHand(); //just display what's in their hands if needed
    void PlayerStatistic();  //get user's historical wins/losses, number game played, etc.

}

Game class:

public class Game
{
    //properties
    int ID { get; set; } //game id, assuming each player can play multiple games as once
    Player PlayerTurn { get; set; } //signifies if it is his turn
    Dominos DominoSet { get; set; }
    List<string> Board { get; set; }

    void Move(Player player, Domino domino);

    public Game(Player player1, Player player2)
    {
        ID = 0; //TODO
        DominoSet = new Dominos();  //create a set of dominos
        Board = new List<string>(); //create a new game board that store all the moves
        Domino curDomino = InitializePlayers(player1, player2, DominoSet.AllDominos);
        DominoSet.AllDominos.RemoveRange(0, 14); //remove all dominos that have been assigned to 2 players
        //...and other good stuffs
    }

}

Domino class, which was intended to describe a single domino:

public class Domino
{
    public string Name { get; set; } // "0,1", "0,2", etc.
    public int Head { get; set; } //each domino has 2 side, i called it head and tail
    public int Tail { get; set; }
    public int Weight { get; set; } //a 0-27, e.g: a "6,6" domino would have the highest predifined weight of 27
    public bool IsDouble { get; set; }  //whether the tile is double or not

    public Domino(string name, int head, int tail, int weight){
       Name = name;
       Head = head;
       Tail = tail;
       Weight = weight;
       IsDouble = (head == tail ? true : false);
    }
}

Dominos class, which was intended for use to store a list of 28 domino set, and perform other actions such as shuffle dominos, track domino, etc.:

public class Dominos
{
       private string[] _tiles = {"0,1","0,2","0,3","0,4","0,5","0,6",
                               "1,2","1,3","1,4","1,5","1,6",
                               "2,3","2,4","2,5","2,6",
                               "3,4","3,5","3,6",
                               "4,5","4,6",
                               "5,6",
                               "0,0","1,1","2,2", "3,3", "4,4","5,5","6,6"};
       public List<Domino> AllDominos { get; set; }
       public void Shuffle(){} //radomize the dominos

       public Dominos()
       {
           AllDominos = new List<Domino>();

           for (int i = 0; i < _tiles.Length; i++)
           {
               string[] word = _tiles[i].Split(',');
               Domino d = new Domino(_tiles[i], int.Parse(word[0]), int.Parse(word[1]), i);
               AllDominos.Add(d);
           }
           Shuffle();//randomize the domino list
       }
}

Main program to wire things together:

class Program
{
    static void Main(string[] args)
    {
        Player _player1 = new Player("Player1");
        Player _player2 = new Player("Player2");
        Game _game = new Game(_player1, _player2);
    }
}

I think this is a good exercise for trying out OOP design.  When I was learning OOP I found that working with real-life objects helped a lot and slowly I started getting use to objects that represented abstract concepts.  In this example you have a mixture, real-life dominoes and abstract concepts such as the 'Game' object.

The Domino Class is a good candidate for an immutable object.  It is often a good idea to create immutable objects when you can as this can be a powerful mechanism to stop erroneous state from being accidentally recycled.

There seems to be quite a bit of duplication going on within the Domino Class.  This can sometimes lead to confusion when debugging.  If you are debugging, and the name of a domino says "3,2" but the Head and Tail show 4 and 1 then you are left not sure about what happened.  Best to try and avoid this from happening.

I think it is also a good idea to make sure that all the logic that represents dominoes is wrapped up in the Domino class.  This will help to avoid abstraction leakage.  It is fairly obvious that a domino has a head and a tail and this is exposed via your properties but it may not be obvious how the domino name should be formatted (e.g. should it use a comma 3,4 or a colon 3:4) or how a domino's weight is determined.  If you have to evaluate the weight of a domino outside of the domino class then this might be considered as abstraction leakage.

Putting these concepts together, here is how I would program the Domino Class:

// Represents a tile used in a domino game.
// This class is designed to be immutable.
public class Domino
{
    // Private constructor means that dominos can
    // only be crated from a static method in this class.
    private Domino(string name, int weight)
    {
        // domino name can either be in the format H,T or H:T
        string[] nameParts = name.Split(',', ':');
        if (nameParts.Length != 2)
        {
            throw new ArgumentException("Invalid name format.");
        }

        try
        {
            Head = Int32.Parse(nameParts[0]);
            Tail = Int32.Parse(nameParts[1]);
        }
        catch
        {
            throw new ArgumentException("Invalid name format.");
        }

        Weight = weight;
    }

    // The properties of the domino class have a private set
    // because this class is designed to be immutable.
    public int Head { get; private set; }
    public int Tail { get; private set; }
    public int Weight { get; private set; }

    public bool IsDouble { get { return (Head == Tail); } }

    // Avoid duplicating the head and tail values by
    // generating the name every time it is needed.
    // This will be quick enough that it won't impact
    // performance.  Only worry about performance once
    // it starts to become a problem.
    public string Name { get { return Head.ToString() + "," + Tail.ToString(); } }

    public override string ToString() { return Name; }

    // Returns a full collection of shuffled dominos
    // this static method is part of the domino class
    // and so it can use the private constructor.
    public static DominoCollection GetAll()
    {
        string[] tiles = new string[]
        {
            "0,1","0,2","0,3","0,4","0,5","0,6",
            "1,2","1,3","1,4","1,5","1,6",
            "2,3","2,4","2,5","2,6",
            "3,4","3,5","3,6",
            "4,5","4,6",
            "5,6",
            "0,0","1,1","2,2", "3,3", "4,4","5,5","6,6"
        };

        DominoCollection dominos = new DominoCollection();
        for (int i = 0; i < tiles.Length; i++)
        {
            dominos.Add(new Domino(tiles[i], i));
        }

        dominos.Shuffle();
        return dominos;
    }
}

I would define the DominoCollection class to make as much use of the built-in features as possible by inheriting it from the generic collection class:

public class DominoCollection : Collection<Domino>
{
    // shuffels the dominos in this collection
    public void Shuffle()
    {
        Random r = new Random();

        int n = Count;
        while (n-- > 1)
        {
            int k = r.Next(n + 1);
            Swap(k, n);
        }
    }

    // swap the domino in index 1 with the domino in index 2
    private void Swap(int i1, int i2)
    {
        Domino temp = this[i1];
        this[i1] = this[i2];
        this[i2] = temp;
    }
}

Now you can get the full set of dominoes like this:

DominoCollection dominos = Domino.GetAll();

This still might not be considered ideal and there maybe some who suggest that the Domino.GetAll() method should not be a static method of Domino but instead should be in the collection class (similar to your original code) but at least you will have considered the alternatives and thought about the concept of abstraction leakage.


Recursively reading a directory in node.js I made a function for recursively reading a directory and it just seems kind of long and complicated. If you could take a look and tell me what's good or bad about it, that would be great.

Reading the directory synchronously is not an option.

var dutils = require('./dutils');
module.exports = {
    getIndex: function(ignore, cb) {
        var root = this.root;
        var cbs = 1;
        var index = {};

        // recursive function for walking directory
        (function walk(dir) {
            fs.readdir(dir, function(err, files) {
                if (err) return cb(err);

                files.forEach(function(fileName) {
                    // prepare
                    var filePath = path.join(dir, fileName); // filePath with root
                    var relFilePath = path.relative(root, filePath); // filePath without root

                    // if the file is on our ignore list don't index it
                    if (dutils.globmatch(filePath, ignore)) return;

                    var stats = fs.statSync(filePath);
                    var isDir = stats.isDirectory();

                    index[relFilePath] = {
                        isDir: isDir,
                        mtime: stats.mtime
                    };

                    // if the file is a directory, walk it next
                    if (isDir) {
                        // count the number of callbacks that are running concurrently
                        cbs++;
                        walk(filePath);
                    }
                });

                // our callback was completed
                cbs--;

                // if all callbacks were completed, we've completely walked our directory
                if (cbs === 0) cb(null, index);
            });
        })(root);
    }
}

My comments are inline starting with the //*. I also very much encourage you to look at async libraries such as Q.

var dutils = require('./dutils');

//* Why export an object with a single function? Why not just export the function itself?
module.exports = {
    //* I don't like intermixing method implementation with exports declaration because you tend to be
    //* looking for very different things when looking at each. Hoisting to the rescure!
    //*     module.exports = { getIndex: getIndex };
    //*     ... later ...
    //*     function getIndex(ignore, cb) {
    //*         ...
    getIndex: function(ignore, cb) {
        //* What if they didn't pass a  callback? Might as well check here and throw an error. Otherwise
        //* there will be a confusing error later on

        //* What is `this`? I **highly** recommend not using the `this` parameter unless you
        //* understand _very_ well what it does and how it works. You can almost always achieve the same
        //* thing using simpler techniques
        var root = this.root;
        //* cds is not a good variable name - I have no idea what this represents reading it
        var cbs = 1;
        var index = {};

        // recursive function for walking directory
        (function walk(dir) {

            //* I usually prefer naming callback functions. Not only does it allow you to give a good
            //* label to the code but the function will show up labeled in stack traces
            fs.readdir(dir, function(err, files) {
                //* Be aware that it is possible for the callback to be triggered multiple times when there are
                //* errors. This is an unusual interface and just be certain that you do this on purpose
                if (err) return cb(err);

                //* Again, I would recommend labeling this function.
                //* Also I'm not sure if this will follow symlinks or not but if does then this might end up in
                //* an endless loop.
                files.forEach(function(fileName) {
                    // prepare
                    var filePath = path.join(dir, fileName); // filePath with root
                    var relFilePath = path.relative(root, filePath); // filePath without root

                    //* I don't think this is really a useful comment as it doesn't say much more than
                    //* the code itself does. I also usually prefer putting the `return` statement for early returns
                    //* on its own line to make the early return stand out visually.
                    // if the file is on our ignore list don't index it
                    if (dutils.globmatch(filePath, ignore)) return;

                    var stats = fs.statSync(filePath);
                    var isDir = stats.isDirectory();

                    index[relFilePath] = {
                        isDir: isDir,
                        mtime: stats.mtime
                    };

                    //* This comment is not very useful as it - again - just restates what the code does
                    // if the file is a directory, walk it next
                    if (isDir) {
                        // count the number of callbacks that are running concurrently
                        cbs++;
                        walk(filePath);
                    }
                });

                // our callback was completed
                cbs--;

                // if all callbacks were completed, we've completely walked our directory
                if (cbs === 0) cb(null, index);

                //* Is the entire point of the `cbs` counter to see if any `readdir` operation resulted in an error?
                //* that seems to be all that it does. Why not just have a `var lastError = null` in `getIndex` and
                //* rather than checking for err do `if(lastError = err) return`. That way if `err` is truthy you have
                //* an error, otherwise you do not
                //*
                //* Also keep in mind that cb will be called multiple times but each time with the
                //* same instance of the index object. That means that previously invoked callbacks will be
                //* having this object mutate underneat them. This might be exactly what you want but it is unuaual
                //* and should definitely be documented clearly. At the very least you want a big bold doc comment
                //* on this function
            });
        })(root);
    }
}

Here is an article I wrote on this parameters


Sorting string with if-else I was asked to write a program that sorts three strings alphabetically using only if-else (no sorting algorithms, usage of arrays whatsover). I'm fairly new to C++ programming but I've come up with this program and would like to know if there's any way to make this more efficient while only using if-else? Any help or suggestions would be appreciated. Thank you!
#include <iostream>
using namespace std;

int main()
{
    string word1, word2, word3;
    string temp;

    cout << "Enter three words separated by a space: ";
    cin >> word1 >> word2 >> word3;

    if (word1 > word2 && word1 > word3 && word3 > word2)
    {
        temp = word1;
        word1 = word2;
        word2 = word3;
        word3 = temp;
    }

        else if (word1 > word2 && word1 > word3 && word2 > word3)
        {
            temp = word1;
            word1 = word3;
            word3 = temp;
        }

            else if (word2 > word1 && word2 > word3 && word1 > word3)
            {
                temp = word2;
                word1 = word3;
                word2 = word1;
                word3 = temp;
            }

            else if (word2 > word1 && word2 > word3 && word3 > word1)
            {
                temp = word2;
                word2 = word3;
                word3 = temp;
            }

                else if (word3 > word1 && word3 > word2 && word1 > word2)
                {
                    temp = word1;
                    word1 = word2;
                    word2 = temp;
                }

    cout << "The correct sort is " << word1 << ", " << word2 << ", " << word3 << endl;

    return 0;
}

Your indentation is off. if should be at the same level as the corresponding else or else if, their bodies being indented once. Don't wander all the way to the right.

You are missing the include for std::string, <string>.

using namespace std; seems convenient, right? Unfortunately, throwing everything and the kitchen sink into the global namespace and praying it works is error-prone.
See "Why is “using namespace std;” considered bad practice?".
I wouldn't even use a using-declaration here, though that would be unproblematic at least.

return 0; is implicit for main(). (And only for that function.)

You never test whether reading (and/or writing) succeeds. At least that's pretty harmless in your program. Still, consider ending with:
    return !!std::cin || !!std::cout;

Swapping is often far more efficient than copying. Consider using a.swap(b) or the more generic using std::swap; swap(a, b); two-step, the latter is in <utility>.

Only use std::endl when you need to flush the stream. Actually, belay that, be explicit and use std::flush.
Hint: std::cin and std::cout are tied, which you already take advantage of with your prompt. Also, when the program ends the standard streams are flushed.
See "What is the C++ iostream endl fiasco?".

Comparing strings is likely to be comparatively expensive. Thus, compare two strings, and then make different decisions depending on the outcome.

An alternative to considering each case separately is writing a sorting-network.
You would be well-advised to abstract a conditional swap into its own function if you go that way:
void sort2(std::string& a, std::string& b) noexcept {
    if (b < a)
        b.swap(a);
}


Optimize code block for LINQ and non LINQ I have 2 code block to do same a work and have same target create some thing, but I don't know which block is the best (I mean LINQ or non-LINQ)?

First of all, can anyone tell me how to optimize on the code blocks?  And the lastly, help me find the best style between LINQ and non-LINQ:

First block is:

if (tblone.Rows.Count > 0)
        {
            tblone.Rows.Cast<DataRow>().Where(x => (string)x["id"] == "true").ToList()
                .ForEach(x => tbltwo.Rows.Cast<DataRow>().Where(y => y["category"] == x["cate"]).ToList().ForEach(
                    z =>
                    {
                        tblthree.Rows.Cast<DataRow>()
                                          .ToList<DataRow>()
                                          .ForEach(k => datatemp.Rows.Add(k["m"].ToString()));
                        tblfour.Rows.Cast<DataRow>()
                            .ToList<DataRow>()
                            .ForEach(k => datatemp.Rows.Add(k["m"].ToString()));
                    }));
        }

Second block is:

if (tblone.Rows.Count > 0)
        {
            for (var idx = 0; idx < tblone.Rows.Count; idx++)
            {
                if (bool.Parse(tblone.Rows[idx]["subset"].ToString()))
                {
                    for (var kdx = 0; kdx < tbltwo.Rows.Count; kdx++)
                    {
                        if (tbltwo.Rows[kdx]["category"].ToString() == tblone.Rows[idx]["cate"].ToString())
                        {
                            for (var zdx = 0; zdx < tblthree.Rows.Count; zdx++)
                                datatemp.Rows.Add(tblthree.Rows[zdx]["m"].ToString());
                            for (var zdx = 0; zdx < tblfour.Rows.Count; zdx++)
                                datatemp.Rows.Add(tblfour.Rows[zdx]["m"].ToString());
                        }
                    }
                }
            }
        }

First - if you are working with DataTables, then you can use Linq to DataSet, which has nice extension methods to get field values.

Now let's review logic of your code. You are not using any data from tblone and tbltwo in your result. Actually you only finding joined categories between these two tables, and for each matched rows pair you are adding all m values from tblthree and tblfour to result. I find this logic pretty strange. Maybe you are expecting only single match here. Then you will need boolean result (match found or not). But basically you need count of joined rows:

var matchedCategoriesCount =
            (from one in tblone.AsEnumerable().Where(r => r.Field<bool>("id"))
             join two in tbltwo.AsEnumerable()
                on one.Field<string>("cate") equals two.Field<string>("category")
             select one).Count();

Same with lambda syntax if you find it more readable:

var matchedCategoriesCount =
            tblone.AsEnumerable().Where(r => r.Field<bool>("id"))
                  .Join(tbltwo.AsEnumerable(),
                        one => one.Field<string>("cate"),
                        two => two.Field<string>("category"),
                        (one, two) => one)
                  .Count();

Then you can select m values from tblthree and tblfour N times and create new DataTable:

// if (matchedCategoriesCount > 0)
var values = tblthree.AsEnumerable().Concat(tblfour.AsEnumerable())
                     .Select(r => r.Field<string>("m")))
                     .ToArray();

You can even use simple loop here:

for(int i = 0; i < matchedCategoriesCount; i++)
   foreach(var value in values)
       datatemp.Rows.Add(value);

And last - don't use variable names like tblone, tbltwo, tblthree etc. These names does not explain what data these variables hold. Use names from domain of your application.


Execute every Monday that is not the Monday following last Saturday in month I think the following is good but wondering if it can be simplified or improved.

My condition for code execution is the following:

It is a Monday
AND
It is not the Monday following the last Saturday in the month.

This is the current sql:

DECLARE @now DATETIME = DATEADD(dd,DATEDIFF(dd,'19000101',SYSDATETIME()),'19000101');

IF (
        (--is it a monday?
            SELECT DATEPART(dw,@now)
        ) = 2

        AND

        (--is it not the monday following the last Saturday of the month?
            (
            SELECT DATEDIFF(
                            DAY,
                            @now-2,
                            DATEADD(MONTH,DATEDIFF(MONTH,0,@now-2)+1,0)-1
                        )
            ) > 6
        )
    )
BEGIN;

  ...
  ...

END;

Sometimes, comments aren't the clearest way to make a script readable:

IF (
        (--is it a monday?
            SELECT DATEPART(dw,@now)
        ) = 2

Consider:

declare @MONDAY int;
set @MONDAY = 2;

if (datepart(weekday,@now) = @MONDAY)

Does one need a comment to figure out what's going on? I hope not!

This part however:

DECLARE @now DATETIME = DATEADD(dd,DATEDIFF(dd,'19000101',SYSDATETIME()),'19000101');

If it wasn't of the @now identifier, I'd wonder what this convoluted assignment intends to do.

Why not just do this instead?

declare @now date = cast(getdate() as date);

I mean, why work with a datetime when clearly you're only interested in the date part from that point on?

I find there's way too much whitespace in your code, you need to strike a balance between wall-of-code and air code.

Now, changing @now from a datetime to a date will break your code. The @now-2 here:

DATEDIFF(DAY, @now-2, DATEADD(MONTH,DATEDIFF(MONTH,0,@now-2)+1,0)-1

...is illegal because...

I would extract that bit of logic into its own variable, for readability's sake, and using a dateadd function there would be no issue with subtracting 2 days from that date.