Linear regression with visualization I have created a small script that:

Creates a lot of random points.
Runs a small brute force search to find a rect that has a low error, that is a good fit for the data.
Runs a linear regression on the rect generated with brute force to further reduce the error.
Prints out information and plots the data.

Down here you can see an example graph: the random points in blue, the brute forced rect, in blue, the 'linear regressed' rect in green.

from __future__ import division
import random
import matplotlib.pyplot as p
import numpy as np

def random_point():
    return (random.random(),random.random())

def list_of_random_points(n):
    return [random_point() for _ in range(n)]

def error_of_point(point,function):
    x,y = point[0],point[1]
    return abs(y - function(x))

def error_of_list_of_points(points,function):
    return sum((error_of_point(point,function) for point in points))

def get_starting_rect(points):
    min_error = 10**10
    for m1 in range(-10,10):
        for q1 in range(-10,10):
            m,q = m1/10,q1/10
            function = lambda x: m*x + q
            if error_of_list_of_points(points,function) < min_error:
                min_error = error_of_list_of_points(points,function)
                best_mq = m,q
    return best_mq

def get_approximate_m(m,q,points,sensibility):
    if error_of_list_of_points(points,lambda x: (m+sensibility)*x+q) < error_of_list_of_points(points,lambda x: m*x + q):
        m += sensibility
    elif error_of_list_of_points(points,lambda x: (m-sensibility)*x+q) < error_of_list_of_points(points,lambda x: m*x + q):
        m -= sensibility
    return m

def get_approximate_q(m,q,points,sensibility):
    if error_of_list_of_points(points,lambda x: (m)*x+q+sensibility) < error_of_list_of_points(points,lambda x: m*x + q):
        q += sensibility
    elif error_of_list_of_points(points,lambda x: (m)*x+q-sensibility) < error_of_list_of_points(points,lambda x: m*x + q):
        q -= sensibility
    return q

def approximate_better_m_and_q(m,q,points,sensibility):
    for _ in range(100):
        m = get_approximate_m(m,q,points,sensibility)
        q = get_approximate_q(m,q,points,sensibility)
    sensibility /= 10
    for _ in range(100):
        m = get_approximate_m(m,q,points,sensibility)
        q = get_approximate_q(m,q,points,sensibility)
    return m,q

def plot_rect(m,q):
    x = np.arange(0,1,0.1)
    y = [i*m + q for  i in x]
    p.plot(x,y)

def plot_rect_and_points(rect,points):
    p.scatter(*zip(*points))
    m,q = rect
    plot_rect(m,q)

def main():
    NUMBER_OF_POINTS = 1000
    points = list_of_random_points(NUMBER_OF_POINTS)

    m,q = get_starting_rect(points)
    plot_rect(m,q)

    print("""The rect generated with brute force of equation y = {}x + {}
has an error of {}""".format(m,q,error_of_list_of_points(points,lambda x: m*x + q)))

    better_rect = approximate_better_m_and_q(m,q,points,0.1)
    m,q = better_rect
    print("""The rect generated with linear regression of equation y = {}x + {}
starting from the rect generated with
brute force has an error of {}""".format(
    m,q,error_of_list_of_points(points,lambda x: m*x + q)))

    plot_rect_and_points((m,q),points)
    p.show()

if __name__ == "__main__":
    main()

For some reason there's a trend among scientific Python users to import things with really short names, like

import numpy as np

This is somewhat bearable when it's np and you use it many times, but you use np once. Further, you

import matplotlib.pyplot as p

and only use this three times. It's not worth it; typing pyplot three times isn't going to make your code unduly long.

You should add more spacing in accordance to PEP 8, and cap line lengths to something less than what you currently have (100 characters should be enough).

random_point can just be

def random_point():
    return random.random(), random.random()

Namely, no brackets needed. You only use it once in

def list_of_random_points(n):
    return [random_point() for _ in range(n)]

so just write

def list_of_random_points(n):
    return [(random.random(), random.random()) for _ in range(n)]

instead. I'd also rename this to random_points. In fact, you only use this once, so I'd remove the function here too.

You only use error_of_point in error_of_list_of_points, so inline it, giving:

def error_of_list_of_points(points, func):
    return sum(abs(y - func(x)) for x, y in points)

You don't actually need a list, so call this error_in_points or similar.

In get_starting_rect, change

function = lambda x: m*x + q

to

def line(x): return m*x + q

I would also put it on two lines for readability. Since you use this so much, it makes sense to do

def line(m, q):
    def y(x):
        return m*x + q
    return y

So you can just do

error_in_points(points, line(m, q))

This also simplified the loop:

for m in range(-10, 10):
    for q in range(-10, 10):
        if error_in_points(points, line(m / 10, q / 10)) < min_error:
            ...

You can then move to using min:

mqs = product(range(-10, 10), range(-10, 10))
mqs = ((m/10, q/10) for m, q in mqs)

return min(mqs, key=lambda mq: error_in_points(points, line(*mq)))

or even

mqs = numpy.mgrid[-1:+1:0.1, -1:+1:0.1].reshape(2, -1).T
return min(mqs, key=lambda mq: error_in_points(points, line(*mq)))

get_approximate_m now looks like

def get_approximate_m(m, q, points, sensibility):
    if error_in_points(points, line(m+sensibility, q)) < error_in_points(points, line(m, q)):
        m += sensibility
    elif error_in_points(points, line(m-sensibility, q)) < error_in_points(points, line(m, q)):
        m -= sensibility
    return m

You should cache error_in_points(points, line(m, q)):

def get_approximate_m(m, q, points, sensibility):
    current_err = error_in_points(points, line(m, q))

    if error_in_points(points, line(m+sensibility, q)) < current_err:
        m += sensibility
    elif error_in_points(points, line(m-sensibility, q)) < current_err:
        m -= sensibility
    return m

This would also be better named adjust_m or similar.

Your approximate_better_m_and_q does:

for _ in range(100):
    m = adjust_m(m, q, points,sensibility)
    q = get_approximate_q(m, q, points, sensibility)

Since this does not change sensibility as it progresses it's likely that this quickly stops doing anything productive, spending most of its time bouncing around a few points. It'll probably work better with a gradual slowing:

def approximate_better_m_and_q(m, q, points, sensibility):
    for _ in range(50):
        m = get_approximate_m(m, q, points, sensibility)
        q = get_approximate_q(m, q, points, sensibility)
        sensibility *= 0.95
    return m, q

Which does tend to improve the results for me even though it has a quater the number of iterations.

plot_rect can trivially use Numpy's broadcasting:

def plot_rect(m,q):
    x = numpy.arange(0, 1, 0.1)
    y = x * m + q
    pyplot.plot(x,y)

plot_rect_and_points can use unpacking (as can main):

plot_rect(*rect)

I hadn't realized that error_in_points is only used with line as the funciton; maybe you should move it into error_in_points to make calling more convenient.

You should split your prints up:

print("The rect generated with brute force of equation y = {}x + {}".format(m, q))
print("has an error of {}".format(error_in_points(points, m, q)))

although the trailing space suggests maybe you didn't realize they were printing on separate lines.

All in all this gives

from __future__ import division

import numpy
import random

from matplotlib import pyplot

def error_in_points(points, m, q):
    return sum(abs(y - (m*x + q)) for x, y in points)

def get_starting_rect(points):
    mqs = numpy.mgrid[-1:+1:0.1, -1:+1:0.1].reshape(2, -1).T
    return min(mqs, key=lambda mq: error_in_points(points, *mq))

def adjust_m(m, q, points, sensibility):
    current_err = error_in_points(points, m, q)

    if error_in_points(points, m+sensibility, q) < current_err:
        m += sensibility
    elif error_in_points(points, m-sensibility, q) < current_err:
        m -= sensibility
    return m

def adjust_q(m, q, points, sensibility):
    current_err = error_in_points(points, m, q)

    if error_in_points(points, m, q+sensibility) < current_err:
        q += sensibility
    elif error_in_points(points, m, q-sensibility) < current_err:
        q -= sensibility
    return q

def approximate_better_m_and_q(m, q, points, sensibility):
    for _ in range(50):
        m = adjust_m(m, q, points, sensibility)
        q = adjust_q(m, q, points, sensibility)
        sensibility *= 0.95
    return m, q

def plot_rect(m,q):
    x = numpy.arange(0, 1, 0.1)
    y = x * m + q
    pyplot.plot(x,y)

def plot_rect_and_points(rect,points):
    pyplot.scatter(*zip(*points))
    plot_rect(*rect)

def main():
    NUMBER_OF_POINTS = 1000
    points = [(random.random(), random.random()) for _ in range(NUMBER_OF_POINTS)]

    m, q = get_starting_rect(points)
    plot_rect(m, q)

    print("The rect generated with brute force of equation y = {}x + {}".format(m, q))
    print("has an error of {}".format(error_in_points(points, m, q)))

    better_rect = approximate_better_m_and_q(m, q, points, 0.1)
    m, q = better_rect
    print("The rect generated with linear regression of equation y = {}x + {}".format(m, q))
    print("starting from the rect generated with brute force has an error of {}".format(
        error_in_points(points, m, q)
    ))

    plot_rect_and_points((m,q),points)
    pyplot.show()

if __name__ == "__main__":
    main()

There is a lot more you can do (particularly vectorization), but this is a good start.


Nagios check to see if a certain IP appears in a trace route I needed a test to know which firewall in out HA environment was currently active. We have a webFilter connected to our "primary" so if the firewall fails over the webfilter is taking out of path and internet access is completely open during that time.

I have a bash script that will feed two IP addresses. I am aware there is no data validation. This was done to reduce processing time. Run the trace using the first IP address and see if we can grep the second from the result. If the result is a non-zero string then return and OK result else return a Critical result. The exit codes are used by the Nagios Monitoring system to determine state and actions which is why there are custom error codes.

#!/usr/bin/env bash

# Nagios exit codes. These are used to determine state
nagiosStateOK=0
nagiosStateWarning=1
nagiosStateCritical=2
nagiosStateUnknown=3

# Initialize result
traceResult=''

# The address that we are going to run the tracert to
target="$1"
# Check for this address in the trace
locate="$2"

traceResult=$(traceroute -n -w 2 $target | grep $locate)

if [[ -n "$traceResult" ]];then
        echo "$locate found in path. Still running on primary"
        exit $nagiosStateOK
else
        echo "$locate not found in path. Probably failed over"
        exit $nagiosStateCritical
fi

Your use of $locate as a regular expression for grep is sloppy.  If $locate is 2.3.45.25, for example, a hop through 70.233.45.251 would be considered successful.

I suggest using grep -F " $locate " — with -F to interpret the argument as a fixed string rather than as a regex, and with spaces before and after the IP address.


Page Scraper and DOM manipulator This code is a page scraper using HtmlAgilityPack that creates a DOM document upon construction and allows for node manipulation afterward.

HtmlAgilityPack uses XPath Selectors for selecting nodes.

An example use case would look like:

var me = new PageScraper("http://codereview.stackexchange.com/users/62429/quill", Console.WriteLine);
var usernameNode = me.FetchNode("//h2[@class=user-card-name]");
var username = PageScraper.FetchNodeText(usernameNode);

This is the PageScraper class, the only class in the Scraper namespace so far.

The reason I pass in an Action is so that I can attach log messages to a Forms list box.

using System;
using System.Net;
using System.Text.RegularExpressions;
using HtmlAgilityPack;

namespace Scraper
{
    public class PageScraper
    {
        private readonly HtmlDocument _document;
        public Action<string> Print { get; set; }
        public PageScraper(string url, Action<string> print)
        {
            if (string.IsNullOrEmpty(url))
            {
                throw new Exception("url is empty");
            }
            Print = print;
            WebClient webClient = new WebClient();
            Print("downloading page");
            string urlWithoutNewLineCharacters = StripNewLineCharactersAndSpaces(url);
            string html = webClient.DownloadString(urlWithoutNewLineCharacters);
            _document = new HtmlDocument();
            _document.LoadHtml(html);
            Print("loading page");
        }
        public HtmlNodeCollection FetchLinks()
        {
            Print("fetching links");
            return _document.DocumentNode.SelectNodes("//a[@href]");
        }

        public HtmlNodeCollection FetchNodes(string selector)
        {
            Print("fetching nodes");
            return _document.DocumentNode.SelectNodes(selector);
        }

        public HtmlNode FetchNode(string selector)
        {
            Print("fetching node");
            return _document.DocumentNode.SelectSingleNode(selector);
        }

        public static string FetchNodeText(HtmlNode node)
        {
            if (node != null && node.InnerText != null)
            {
                return node.InnerText;
            }
            return string.Empty;
        }
        public static string FetchNodeHref(HtmlNode node)
        {
            if (node != null && node.Attributes["href"].Value != null)
            {
                return node.Attributes["href"].Value;
            }
            return string.Empty;
        }
        private static string StripNewLineCharactersAndSpaces(string str)
        {
            return Regex.Replace(str, @"\t|\n|\r| |\s+", string.Empty);
        }
    }
}

Print = print;
WebClient webClient = new WebClient();
Print("downloading page");
string urlWithoutNewLineCharacters = StripNewLineCharactersAndSpaces(url);
string html = webClient.DownloadString(urlWithoutNewLineCharacters);
_document = new HtmlDocument();
_document.LoadHtml(html);
Print("loading page");

You are doing too much in your constructor. Consider extracting some of this code into separate method(s) maybe an intitialize() method or buildDocument() method. The rule for this is know as Coding at Wrong Level of Abstraction.

Your method StripNewLineCharactersAndSpaces should be called right after the first time it is used not at the end of your class. this is known as Vertical Separation This allows reads to quickly ready your code without having to bounce around too much.

I do Like that you have kept your methods really small


Why is loading images taking so long? When Page_Load is executed I assign the ImageUrl which is a IHTTPHandler:

imgView.ImageUrl = "ImageHandler.ashx" + "?rezeptId=" + rezeptId.ToString() +"&imageId=0";

Which looks like this:

public class ImageHandler : IHttpHandler {

    public void ProcessRequest (HttpContext context) {

        int rezeptId = Int32.Parse(context.Request.QueryString["rezeptId"]);
        int imageId = Int32.Parse(context.Request.QueryString["imageId"]);

        List<Image> images = DBManager.GetRezeptImages(rezeptId);
        context.Response.ContentType = "image/png";
        context.Response.BinaryWrite(Helper.ImageToByteArray(images[imageId]));
    }

    public bool IsReusable {
        get {
            return false;
        }
    }

}

The handler call is a static method from DBManager, a custom static class which provides LINQ-To-SQL queries. The method is called GetRezeptImages():

 public static List<Image> GetRezeptImages(int rezeptId)
    {
        using (CookBookDataContext ctx = new CookBookDataContext(Resources.ResourceFile.DBConnection))
        {
            IEnumerable<RezeptBilder> bilder = from b in ctx.RezeptBilders where b.FKRezept == rezeptId select b;
            List<Image> imageList = new List<Image>();

            foreach (RezeptBilder b in bilder)
            {
                imageList.Add(Helper.ByteArrayToImage(b.Bild.ToArray()));
            }

            return imageList;
        }
    }

This method simply loads image binaries from the database and converts them to Images:

public static Image ByteArrayToImage(byte[] byteArrayIn)
{
    using (MemoryStream ms = new MemoryStream(byteArrayIn))
    {
        Image returnImage = Image.FromStream(ms);
        return returnImage;
    }
}

So basically this is all that happens when loading images. It is possible that GetRezeptImages() will return a List with more than one image but it is always only one that is loaded to image control.

But now I am experiencing some delays when loading an image:

Now i wonder what the reason is, here are some things I think could be the reason:

Images have to be compressed
My method implementation requires re-engineering.
Images are simply to big, which is somehow the same as they need to be compressed.
I am using a trial that my Webhoster provides. I haven't figured out if there is some speed limitations or somkething yet.

So I was hoping that someone with more experience in webdevelopement could give me some advice on this issue.

So, let us see what you are doing.

You query your database for IEnumerable<RezeptBilder> and iterate over the enumeration where you are converting the items to Image and add it to a List<Image>.
Then you are converting the Image back to a Byte[] to write it into the response stream.

Byte[] -> Image -> Byte[] isn't really necessary.

As you don't use methods of the List<T> but rather of the interface IList<T> we will return an IList<T> as it is always better to code against an interface instead of an implementation.

public static IList<Byte[]> GetRezeptImages(int rezeptId)
{
    using (CookBookDataContext ctx = new CookBookDataContext(Resources.ResourceFile.DBConnection))
    {
        IEnumerable<RezeptBilder> bilder = from b in ctx.RezeptBilders where b.FKRezept == rezeptId select b;
        IList<Byte[]> imageList = new List<Byte[]>();

        foreach (RezeptBilder b in bilder)
        {
            imageList.Add(b.Bild.ToArray());
        }

        return imageList;
    }
}

and to be called

public void ProcessRequest (HttpContext context)
{
    int rezeptId = Int32.Parse(context.Request.QueryString["rezeptId"]);
    int imageId = Int32.Parse(context.Request.QueryString["imageId"]);

    IList<Byte[]> images = DBManager.GetRezeptImages(rezeptId);
    context.Response.ContentType = "image/png";
    context.Response.BinaryWrite(images[imageId]);
}

But this shouldn't fix your issue. Taking 28 seconds for accessing 1.3 MB comes down to about 48 Kbyte/s which is quit slow.

To check where the most time is used, you need to measure the time it takes to execute the call to GetRezeptImages().

Smaller images will for sure speed the whole thing up and therefor makes the user of the website happy.

General

What will happen if a user of the site is changing the image url inside the address bar like  ImageHandler.ashx?rezeptId=testForTryParse&imageId=0 ?

Your ProcessRequest() method would throw an exception. You should carefully validate the input. This can be done in this case by using Int32.TryParse() instead of Int32.Parse().

Also you will get an exception, if the requested imageId is greater than the count of the IList.

public class ImageHandler : IHttpHandler
{

    public void ProcessRequest (HttpContext context)
    {
        int rezeptId = 0;
        int imageId = 0;

        if( Int32.TryParse(context.Request.QueryString["rezeptId"], out rezeptId) &&
             Int32.TryParse(context.Request.QueryString["imageId"], out imageId))
        {

            IList<Byte[]> images = DBManager.GetRezeptImages(rezeptId);
            if (images.Count > imageId)
            {
                context.Response.ContentType = "image/png";
                context.Response.BinaryWrite(images[imageId]);
                return;
            }
        }
        // a proper defined error response here
    }

    public bool IsReusable
    {
        get
        {
            return false;
        }
    }

}

Style

You should be consistent with your coding style. It is a matter of taste where to place the opening braces {. Most do it on a new line, some place it on the same line. But you should stick to it.


Median solving from Interview street in Python Any suggestions to improve the performance of the following code? This code is exceeding the time limit for the last four cases. This is a solution to the problem https://www.interviewstreet.com/challenges/dashboard/#problem/4fcf919f11817

def main():
    N = int(raw_input())
    if(0<=N<=100000):
        x = []
        ans=[]
        l=[]
        m=[]
        for i in xrange(0, N):
            tmp = raw_input()
            a, b = [xx for xx in tmp.split(' ')]
            l.append(a)
            m.append(int(b))

        for i in xrange(0, N):
            if l[i]=='r':
                if len(x)<1 or m[i] not in x:
                        ans.append('Wrong!')
                elif m[i] in x:
                    x.remove(m[i])
                    q=len(x)
                    if q%2==1:
                        ans.append(x[(q/2)])
                    elif q%2==0:
                        if q==0:
                            ans.append('Wrong!')
                        else:
                            k=x[q/2]+x[(q/2)-1]
                            if k%2==0:
                                ans.append(int(k)/2)
                            else:
                                ans.append(k/2.0)
            elif l[i]=='a':
                x.append(m[i])
                x=sorted(x)
                p=len(x)
                if p%2==1:
                    ans.append(x[(p/2)])
                else:
                    k=x[p/2]+x[(p/2)-1]
                    if k%2==0:
                        ans.append(int(k)/2)
                    else:
                        ans.append(k/2.0)

        for i in ans:
            print i

if __name__ == "__main__":
    main()

from sys import stdin,stdout
from bisect import bisect,insort
def main():
    N = int(raw_input())
    x = []
    ans = []
    append = x.append
    ans_append = ans.append
    remove = x.remove
    read = stdin.readline
    for i in xrange(0, N):
        tmp = read()
        a, b = tmp.split(' ')
        b = int(b)
        if a == 'r':
            search_result = search(x,b)
            if len(x) < 1 or not(search_result):
                ans_append('Wrong!')
            else:
                remove(b)
                ans_append(median(x))
        else:
            insort(x, b)
            ans_append(median(x))
    for i in ans:
        print i

def median(arr):
    q = len(arr)
    if q==0:
        return 'Wrong!'
    if q%2 == 1:
        return arr[(q/2)]
    else:
        k = arr[q/2] + arr[(q/2)-1]
        if k%2 == 0:
            return int(k)/2
        else:
            return k/2.0

def search(arr,element):
    position = bisect(arr, element)
    if len(arr) >= position and len(arr)!=0:
        if arr[position-1] == element:
            return True
        else:
            return False
    else:
        return False

if __name__ == "__main__":
    main()

def main():
    N = int(raw_input())
    if(0<=N<=100000):

There really isn't much point in checking this. Your code works regardless of the value, and you don't need to worry about whether the contest will feed you larger values

        x = []
        ans=[]
        l=[]
        m=[]

I suggest using longer names even in a contest scenario

        for i in xrange(0, N):
            tmp = raw_input()
            a, b = [xx for xx in tmp.split(' ')]

Actually this line is the same as a, b = tmp.split(' ') The list comphrension just wastes processing power. Since there isn't going to be any unusual whitespcae, you can skip the parameter to split as well

            l.append(a)
            m.append(int(b))

Its not clear why you store this in a list. Just perform the operation as soon as you read it. No need to store things into the lists just to read them in another loop.

        for i in xrange(0, N):

Here you should have used something like for action, number in zip(l,m):

            if l[i]=='r':
                if len(x)<1 or m[i] not in x:

There isn't really a point to checking for len < 1, since the second condition will fail in that case anyways

                        ans.append('Wrong!')

Just print out the output, don't store it up

                elif m[i] in x:

You should be able to just make this an else

                    x.remove(m[i])

This is going to be slow. The checks for m[i] in x and the call to remove have to scan over the whole list to find the elements. But the list is sorted, which means you should be able to find elements faster. Check out the bisect module which enables provides binary search to make finding elements faster in sorted lists.

                    q=len(x)
                    if q%2==1:
                        ans.append(x[(q/2)])
                    elif q%2==0:
                        if q==0:
                            ans.append('Wrong!')

This case is somewhat odd here, it'd make more sense to check for this on level out, before checking for odd and even

                        else:
                            k=x[q/2]+x[(q/2)-1]
                            if k%2==0:
                                ans.append(int(k)/2)
                            else:
                                ans.append(k/2.0)
            elif l[i]=='a':
                x.append(m[i])
                x=sorted(x)

This is better written as x.sort() so as to sort in place. However, its going to be slow constantly resorting the list. Before adding the element, the list is almost sorted, so you can use that to your advantage. The bisect module has operations to make that easy

                p=len(x)
                if p%2==1:
                    ans.append(x[(p/2)])
                else:
                    k=x[p/2]+x[(p/2)-1]
                    if k%2==0:
                        ans.append(int(k)/2)
                    else:
                        ans.append(k/2.0)

Ok, you're doing this again. You should have put this in a function or something

        for i in ans:
            print i

if __name__ == "__main__":
    main()

Maintaining a sorted list is too expensive. Everytime you insert or remove elements, the computer has to shift the whole array in memory. Knowing this the contest site will devise questions to give you the worst case scenario. You are going to have to do something else.

One possibility to explore would be to use a binary tree. Binary trees have faster insertion/removal times, and a reasonably fast way to calculate the median. Their disadvantage here is the complexity required to implement it. A binary tree itself isn't that complicated, but the problem is in a balancing strategy.

A second possibility is to try and use heaps implemented in the heapq module. The idea would be that at any point in time, you maintain two heaps. One has the element less then the median and the other has the elements greater then the median. There is some trickiness there in how you would handle deletions, but I think it could be done.


Appending values in Pandas column I have a dictionary:

x = {'[a]':'(1234)', '[b]':'(2345)', '[c]':'(xyzad)'}

and a dataframe

df = pd.DataFrame({'q':['hey this is [a]', 'why dont you [b]', 'clas is [c]']})

I want to append the values from dictionary to the corresponding keys.

My expected output is:

                       q
0   hey this is [a](1234)
1  why dont you [b](2345)
2      clas is [c](xyzad)

Here's my solution:

x = {k: k+v for k,v in x.items()}

def das(data):
    for i in x.keys():
        if i in data:
            data = data.replace(i, x[i])
    return data

df['q'] = df['q'].apply(lambda x: das(x))
print(df)

Is there a way to improve this?

I have to update a dictionary by appending a key infront of values. Then I use apply to replace the values.

I am looking for more efficient solution.

There is an alternative way using the str functions of pandas.Series, which has the advantage that they are usually faster. In this case you can use pandas.Series.str.replace, which can take a regex to match strings against and a callable to replace them with, which gets passed the regex match object:

def repl(m):
    k = m.group()
    return k + x[k]

df.q.str.replace(r'\[.*\]', repl)
# 0     hey this is [a](1234)
# 1    why dont you [b](2345)
# 2        clas is [c](xyzad)
# Name: q, dtype: object

This uses the fact that your keys to replace seem to follow a pattern, though, and works only as long as you can write a regex to capture it. In that sense your solution is more general.

One thing that you can change in your approach is the check for if i in data. It is superfluous, since str.replace will just ignore it if the string to replace does not appear in the string (it has to search linearly through the whole string to figure that out, but so does i in data).

In addition, instead of iterating over the keys with for i in x.keys():, you can just do for i in x:. But since you also need the value, you can directly do for key, repl in x.items(): data = data.replace(key, repl).


Is this a good algorithm to unflat js objects? I am dealing with a scenario where I get data in the following form:

var data = {
    'user.name': 'renatoargh',
    'user.age': 27,
    'user.interests.0.description': 'coding',
    'user.interests.0.receiveNotifications': true,
    'user.interests.1.description': 'bungee jumping',
    'user.interests.1.receiveNotifications': false
};

So in order to validate and persist this data I decided to create a way to unflat these strings into regular JS objects, and send them to MongoDb (since it is schema-less).

I coded an algorithm that I am unsure that works for most cases so I created a fiddle that seems to be able to deal with an extreme case, full of nested arrays.

Can someone point any week point or case in which my algorithm would fail? Any kind of advice will be appreciated.

Link to the fiddle here!

Heres the code, where messageObject is the array and the output is in messageTree

function forEachOwnProperty(object, iterator) {
    for(var property in object){
        if(object.hasOwnProperty(property)) {
            iterator(property, object[property]);
        }
    }
}

forEachOwnProperty(messageObject, function(property, messages) {
    property = property.split('.');

    var currentNode = messageTree;

    for(var i = 0; i < property.length; i++) {
        var currentProperty = property[i];

        if(typeof currentNode[currentProperty] === 'undefined') {
            if(i === property.length - 1) {
                currentNode[currentProperty] = messages
            } else {
                if(/^\+?(0|[1-9]\d*)$/.test(property[i + 1])) {
                    currentNode[currentProperty] = [];
                } else {
                    currentNode[currentProperty] = {};
                }
            }
        }

        currentNode = currentNode[currentProperty];
    }
});

Nice code!

You should point out assumptions in a comment. You could document, for instance, that if the first encountered subproperty of a property is numeric, it's treated as an array.
I would rename messages to something like value. property is fine, but you could also consider key.
Right now, your code won't work with direct arrays. The following messageObject won't produce the expected result:

messageObject = {
    'array.0': 'Code',
    'array.1': 'Review'
};

There are some very specific edge cases: messageObject = { 'constructor.name': 'renatoargh' } will return an empty object. I'm not really sure what can be done about this though...
Finally, just for fun, you can use something like var key = property.splice(-1); to eliminate the second if in your for loop. Of course, after the loop, you'd have to add currentNode[key] = messages;.


Find select values on one worksheet and copy to a second In the Calculator worksheet of my spreadsheet, for any appliance type present in a building being studied, users enter a quantity. There are 18 different appliance types, and users can enter different quantities of any or all appliance types.

On the Calculator sheet they can see a breakdown of energy use per individual appliance, per appliance type, and for all appliances with a quantity entered:

The macro below (run from a second sheet) copies specific values from Calculator so that they can be pasted into another spreadsheet which is used for reporting. It selects only appliance types for which the user entered a quantity.

Sub findAppliances()

    ' Get Range
    Dim rg As Range
    Set rg = ThisWorkbook.Worksheets("Calculator").Range("F5:F22")

    ' Create dynamic array
    Dim ApplianceQty As Variant

    ' Read values into array
    ApplianceQty = rg.Value

    ' Manipulate and copy the appropriate values
    Dim i As Long, j As Long
    j = 4
    Dim target As String
    Dim source As String
    For i = LBound(ApplianceQty) To UBound(ApplianceQty)
        target = j
        If ApplianceQty(i, 1) <> "" Then
            source = i + 4
            Range("B" & target) = Worksheets("Calculator").Range("F" & source) & " units"
            Range("F" & target) = "=F" & target - 1 & "-" & Worksheets("Calculator").Range("J" & source)
            Range("G" & target) = "=G" & target - 1 & "-" & Worksheets("Calculator").Range("K" & source)
            Range("H" & target) = "=H" & target - 1 & "-" & Worksheets("Calculator").Range("L" & source)
            Range("K" & target) = Worksheets("Calculator").Range("E" & source)
            Range("L" & target) = Worksheets("Calculator").Range("C" & source)
            j = j + 1
        End If
    Next i

    ' Select and copy the text for export
    Range("B4:M" & target).WrapText = False
    Range("B4:M" & target).Select
    Range("B4:M" & target).Copy

End Sub

I suspect that the reason this takes so long to run is the group of six Range() = Worksheets("Calculator").Range() lines within my For loop. I further suspect that if I read all the data I need into an array, then printed it back out of the array (rather than reading cells and copying to new cells), the whole thing would run faster.

Is that true? How much improvement can I expect?

First - nice job declaring all of your variables. The naming could use a little work (sourceRange, applianceQuantityArray). Declaring several on the same line and giving them both a type (below) - A+

Dim i As Long, j As Long

A few standard blurbs -

  Always turn on Option Explicit. You can have it automatically by
  going to Tools -> Options in the VBE and checking the Require
  Variable Declaration option. This way if you have any variables not
  defined, the compiler will let you know.

  Worksheets have a CodeName property - View Properties window
  (F4) and the (Name) field (the one at the top) can be
  used as the worksheet name. This way you can avoid Sheets("mySheet")
  and instead just use mySheet.

  Standard VBA naming
  conventions
  have camelCase for local variables and PascalCase for other
  variables and names.

I do want to talk about a few variables though-

Dim j as Long
Dim target as String
Dim i as Long
Dim source as String
j=4
>loop i
   target=j
   source = i+4
   j=j+1
>end loop

This seems pretty unnecessary. I'll take this -

 Range("B" & target) = Worksheets("Calculator").Range("F" & source) & " units"

to mean that you thought you need to use a string to put into a range format. You don't. This does the same thing (i being arbitrary) -

Dim i As Long
For i = 1 To 10
    Sheet1.Range("B" & i + 3) = Sheet1.Range("F" & i + 4).value & " units"
Next

You'd probably benefit from just using the .Cells(row, col) format instead. But I digress.

Using an array

As you yourself have pointed out, using an array would be faster. It's always faster. Your loop is really difficult to read -

Range("H" & target) = "=H" & target - 1 & "-" & Worksheets("Calculator").Range("L" & source)

(You're subtracting 1 from a String you realize?) It's the same as this -

 Sheet1.Cells(target, 8).value = Sheet1.Cells(target - 1, 8) - calcSheet.Cells(source, 12)

In the most basic way, say you take everything you need into an array

Dim calcArray As Variant
calcArray = calcsheet.Range(calcsheet.Cells(5, 3), calcsheet.Cells(22, 13))
For i = LBound(calcArray) To UBound(calcArray)
    If calcArray(i, 4) <> vbNullString Then

Pretty simple yeah? Just use the 2nd dimension of the variant to specify what column you're targeting (see I started at column 3 (C) so I had to use 4 (F).

Looks to me like you would need an array from the other sheet too, something like

Dim targetArray As Variant
targetArray = Sheet1.Range(Sheet1.Cells(4, 6),Sheet1.Cells(UBound(calcArray) - 1, 9))

And then maybe you make a results array

Dim resultsArray As Variant
ReDim resultsArray(1 To UBound(calcArray), 1 To 3)

Now you can (roughly, this isn't perfect)

For i = LBound(calcArray) To UBound(calcArray)
    If calcArray(i, 4) <> vbNullString Then
        resultsArray(i, 1) = targetArray(i - 1) - calcArray(i, 8)

Populate that results array using what is essentially range("F5").offset(,-1) but in the array.

Then it would need to be printed back out. A few of them come straight from the calcArray hence I didn't make room in the resultsArray but you could.

Anyway, since the range is weird to me you'd just need to adjust it as you like. Or just declare your arrays starting in row 1 and change nothing.


Effective & Secure Method to populate Access Token for authorization header in Rest Template I am trying to consume a REST endpoint by using the RestTemplate Library provided by the spring framework.
The endpoint also demands a Bearer Access Token as its authorization header, which is only obtained as the response from a user authentication endpoint, which in turn expects an encoded Basic Auth in its Header.

This is the high-level implementation that I have done thus far.

HttpHeaders headers = new HttpHeaders();
    headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON));
    headers.setBearerAuth(fetchAccessToken());
    HttpEntity<String> entity = new HttpEntity<String>("parameters",headers);
    ResponseEntity<?> result = this.restClient.exchange(urlToConsume, HttpMethod.GET, entity, String.class);

The 'fetchAccessToken' Method is implemented as follows

HttpHeaders headers = new HttpHeaders();
    headers.setAccept(Arrays.asList(MediaType.APPLICATION_JSON));
    headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
    headers.setBasicAuth(externalDestination.getClientId(),
            externalDestination.getClientSecret());
    HttpEntity<String> entity = new HttpEntity<String>("parameters", headers);

    ResponseEntity<?> result = restClient.exchange(authUrl, HttpMethod.GET, entity, String.class);
    //And Thereby fetching 'access_token' from the successful fetch.

I Want to know whether there is any cleaner way to replicate the above task of dealing with multiple Rest calls to accomplish a single task.
Also, I want to know whether I am missing out any essential validations from a security point of view.

You can have an interceptor on RestTemplate. It will be called for each request. You can have the access token logic within the interceptor. You can also implementing caching so that you do not fire two requests for each task. In case the token expires (401 response), you can regenerate the token

@Component
class MyInterceptor implements ClientHttpRequestInterceptor {
   @Override
    public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution){
      HttpHeaders headers = request.getHeaders();
      headers.setBearerAuth(someCachedService.getBearerToken());
      ... response = execution.execute(request, body);
     // handle unauthorized request

   }
}

@Bean
RestTemplate restTemplate(MyInterceptor interceptor){
  RestTemplate restTemplate = new RestTemplate();
  restTemplate.setInterceptors(Arrays.asList(interceptor));
}


Compress numbers in a string array (#3, #1, #2, #4) to range (#1:#4) This function takes a string array,

{"foo", "#123", "#124", "bar", "#125", "#126"}

makes a new array with the numbers converted to a range:

{"foo", "#123:126", "bar"}

and returns:

"foo,#123:126,bar"

Note it doesn't and shouldn't change {"#1", "#2"} to {"#1:#2"}

using System;
using System.Linq;
using System.Collections.Generic;
class Program
{
    static void Main(string[] args)
    {
        var ids = new string[] {
            "foo", //output: unmodified
            "#60", "#59", "#61", "#62", //from integer is in between, output: #59:#62
            "#12", "#14", "#17", "#13", "#18", "#bar", "#19", "#20", //two ranges and string intertwined, output: #12:#14,#17:#20,#bar
            "#25", "#26", //output: unmodified
            "#39", "#38", //output: unmodified
            "baz", //output: unmodified
            "#12", "#13", "#14" //duplicate sequences, output: #12:#14
        };
        Console.WriteLine("foo,#59:#62,#12:#14,#17:#20,#bar,#25,#26,#38,#39,baz,#12:#14");
        Console.WriteLine(Compress(ids));
        Console.Read();
    }
    static string Compress(IEnumerable<string> IDs)
    {
        var result = new List<string>();
        var ignore = new HashSet<string>();
        foreach (var item in IDs)
        {
            if (ignore.Contains(item)) continue;
            var id = item;
            if (id.StartsWith("#"))
            {
                int fromInt;
                if (int.TryParse(id.Substring(1), out fromInt))
                {
                    var less1 = $"#{fromInt - 1}";
                    var plus1 = $"#{fromInt + 1}";
                    var hasPlus1 = IDs.Contains(plus1);
                    if (IDs.Contains(less1) && hasPlus1) continue;
                    var plus2 = $"#{fromInt + 2}";
                    if (hasPlus1 && IDs.Contains(plus2))
                    {
                        ignore.Add(plus1);
                        ignore.Add(plus2);
                        var toInt = fromInt + 2;
                        while (IDs.Contains($"#{toInt + 1}"))
                        {
                            toInt += 1;
                            ignore.Add($"#{toInt}");
                        }
                        id = $"#{fromInt}:#{toInt}";
                    }
                }
            }
            result.Add(id);
        }
        return string.Join(",", result);
    }
}

In addition to my answer on your other (related) post which is still valid here (with the proper modifications obviously) I'd suggest to use brackets even in case of if blocks with only 1 instructions. For example:

if (IDs.Contains(less1) && hasPlus1) continue;

should become something like:

if (IDs.Contains(less1) && hasPlus1)
{
    continue;
}

Also, var id = item; can be safely removed and you can work directly on item.

By applying all changes the method becomes something like the following:

static string Compress(IEnumerable<string> IDs)
{
    var result = new List<string>();
    var ignore = new HashSet<string>();
    foreach (var item in IDs)
    {
        if (ignore.Contains(item))
        {
            continue;
        }
        if (!item.StartsWith("#"))
        {
            result.Add(item);
            continue;
        }
        int fromInt;
        if (!(int.TryParse(item.Substring(1), out fromInt)))
        {
            result.Add(item);
            continue;
        }
        var less1 = $"#{fromInt - 1}";
        var plus1 = $"#{fromInt + 1}";
        var hasPlus1 = IDs.Contains(plus1);
        if (IDs.Contains(less1) && hasPlus1)
        {
            continue;
        }
        var plus2 = $"#{fromInt + 2}";
        if (!(hasPlus1 && IDs.Contains(plus2)))
        {
            result.Add(item);
            continue;
        }
        ignore.Add(plus1);
        ignore.Add(plus2);
        var toInt = fromInt + 2;
        while (IDs.Contains($"#{toInt + 1}"))
        {
            toInt += 1;
            ignore.Add($"#{toInt}");
        }
        result.Add($"#{fromInt}:#{toInt}");
    }
    return string.Join(",", result);
}


Helium written in Go Help me with code code review.
Helium is my pet project, written in Golang.
Helium is a small, simple, modular constructor with some pre-built components.

https://github.com/im-kulikov/helium

Simple example how to use it

package main

import (
    "context"

    "github.com/davecgh/go-spew/spew"
    "github.com/im-kulikov/helium"
    "github.com/im-kulikov/helium/grace"
    "github.com/im-kulikov/helium/logger"
    "github.com/im-kulikov/helium/module"
    "github.com/im-kulikov/helium/settings"
    "github.com/spf13/viper"
)

var mod = module.New(newApp).
    Append(
        settings.Module,
        logger.Module,
        grace.Module)

// App struct
type App struct {
    v *viper.Viper
}

func newApp(v *viper.Viper) helium.App {
    return &App{v: v}
}

// Run application
func (a App) Run(ctx context.Context) error {
    spew.Dump(a.v.AllSettings())

    return nil
}

func main() {
    h, err := helium.New(&helium.Settings{
        Prefix:       "demo",
        Name:         "demo",
        BuildTime:    "now",
        BuildVersion: "dev",
    }, mod)
    helium.Catch(err)
    helium.Catch(h.Run())
}

A little detail; in Run:

// Run application
func (a App) Run(ctx context.Context) error {
    spew.Dump(a.v.AllSettings())

    return nil
}

error will be always null.
And according go-spew/spew#Dump;

  func Dump(a ...interface{})

Dump doesn't return a thing.
So seems like you should be able to a.- remove errorfrom your signature, or b.- do some sanity check on a.v to see, for example, that those elements are not nil/etc.


Rotating 3D bit board in C I've got a 64 bit bitboard, but instead of interpreting as an 8 by 8 binary matrix, as is usually the case, I've taken it to the third dimension, so to speak.

Now mirroring and even rotating by 180 degrees on any individual axis is, if I'm not mistaken, fairly simple, but rotating by 90 degrees, which is what I actually need, has given me a nasty headache.

I have of course come up with a solution, but I would like to ensure that there isn't a better way, because frankly it's a bit much.

Anyway, here an example function, rotating, I suppose, clockwise on the Y axis:

unsigned long long rotateLeft( unsigned long long bs )
{
     return ( ( bs & 0x0000000000001111 ) << 48 ) |
            ( ( bs & 0x0000000000002222 ) << 31 ) |
            ( ( bs & 0x0000000000004444 ) << 14 ) |
            ( ( bs & 0x0000000000008888 ) >>  3 ) |
            ( ( bs & 0x0000000011110000 ) << 33 ) |
            ( ( bs & 0x0000000022220000 ) << 16 ) |
            ( ( bs & 0x0000000044440000 ) >>  1 ) |
            ( ( bs & 0x0000000088880000 ) >> 18 ) |
            ( ( bs & 0x0000111100000000 ) << 18 ) |
            ( ( bs & 0x0000222200000000 ) <<  1 ) |
            ( ( bs & 0x0000444400000000 ) >> 16 ) |
            ( ( bs & 0x0000888800000000 ) >> 33 ) |
            ( ( bs & 0x1111000000000000 ) <<  3 ) |
            ( ( bs & 0x2222000000000000 ) >> 14 ) |
            ( ( bs & 0x4444000000000000 ) >> 31 ) |
            ( ( bs & 0x8888000000000000 ) >> 48 );
}

I suppose 16 shifts, 16 ANDs and 15 ORs is not actually that bad, but I cannot believe that this is the best option.

So is there another/better way? I need both direction for the X and Y axis, but I suppose the solution will be somewhat similar for all of them, assuming of course that there is a better way.

Come to think of it you could do something like:

unsigned long long rotateLeft( unsigned long long bs )
{
    //mirror in diagonal plane (x=z?)
    bs =   ( ( bs & 0x0000000000008888 ) << 45 ) | ( ( bs & 0x1111000000000000 ) >> 45 ) |
           ( ( bs & 0x0000000088884444 ) << 30 ) | ( ( bs & 0x2222111100000000 ) >> 30 ) |
           ( ( bs & 0x0000888844442222 ) << 15 ) | ( ( bs & 0x4444222211110000 ) >> 15 ) |
             ( bs & 0x8888444422221111 );
    //mirror in vertical plane (xy?)
    //I do apologize  for not knowing the correct terminology.
    bs =   ( ( bs & 0x00000000ffffffff ) << 32 ) | ( ( bs & 0xffffffff00000000 ) >> 32 );
    return ( ( bs & 0x0000ffff0000ffff ) << 16 ) | ( ( bs & 0xffff0000ffff0000 ) >> 16 ) |
}

I would even think that could be improved a bit without too much effort. Still, I'd be interested in any suggestions.

edit:
I've been informed that clarification is needed, so I'll do my best.
Firstly, structurally I'm talking about a 4 by 4 by 4 cube.
Line 1 of layer 1 consist of bits 0 through 3
Line 2 of layer 2 of bits 20 through 23
layer 3 of bits 32 through 47
and so on.
for each layer lsb is at the top left and msb is bottom right, counting line wise.

and example of rotating left could be:
layer 1-4:

1000 0000 0000 0000
0100 0000 0000 0000
0010 0000 0000 0000
0001 0000 0000 0000

to

0000 0000 0000 1000
0000 0000 1000 0000
0000 1000 0000 0000
1000 0000 0000 0000

or

0x0000 0000 0000 8421

to

0x1000 0100 0010 0001

I hope this clarifies matters a bit.

edit 2:
as request, here the implementation:

uint64_t board_with_exactly_one_bit_set(int x, int y, int z)
{
    return 1 << ( z*16 +y*4 +x );
}

First of all, it might help to explain the problem domain; you might have an XY problem here. The first application for "4x4x4 bitboards" that pops into my head is that you're dealing with Rubik's Revenge... but representing cubelets by single bits is not of obvious utility to me, and rotating the entire cube by 90 degrees is even less useful.

Next, this does seem like the sort of thing that has a known solution somewhere, so I'd definitely recommend doing a thorough Google search. (Asking StackOverflow might also be appropriate; but that's just offloading the Google search onto somebody else. It would be polite to do some searching yourself first, and report the results.) I didn't do any searching myself other than a quick fruitless skim through HAKMEM; but if you find out "the" answer, I'd be mildly interested to hear about it.

Okay, so, let's assume that we've got a 4x4x4 bitboard that looks like this:

uint64_t board_with_one_bit_set(int x, int y, int z)
{
    int index = (x + 4*y + 16*z);
    return 1uLL << index;
}

Notice that "rotating a bitboard" is a special case of "permuting the bits in a word." That is, we expect our result to have the same parity as the input — and in fact the same bits, just in a different order. So mathematically it might make more sense to think of this as a permutation of a 64-element sequence, rather than as a bunch of imperative shifts and masks.

To avoid brain farts, I decided to write a little Python script to generate the bit-shifting code from the permutation. Here's the input:

original = [
    63, 62, 61, 60,
    59, 58, 57, 56,
    55, 54, 53, 52,
    51, 50, 49, 48,
                   47, 46, 45, 44,
                   43, 42, 41, 40,
                   39, 38, 37, 36,
                   35, 34, 33, 32,
                                  31, 30, 29, 28,
                                  27, 26, 25, 24,
                                  23, 22, 21, 20,
                                  19, 18, 17, 16,
                                                 15, 14, 13, 12,
                                                 11, 10, 9,  8,
                                                 7,  6,  5,  4,
                                                 3,  2,  1,  0,
]
rotated_right_around_x_axis = [
    51, 50, 49, 48,
    35, 34, 33, 32,
    19, 18, 17, 16,
    3,  2,  1,  0,
                   55, 54, 53, 52,
                   39, 38, 37, 36,
                   23, 22, 21, 20,
                   7,  6,  5,  4,
                                  59, 58, 57, 56,
                                  43, 42, 41, 40,
                                  27, 26, 25, 24,
                                  11, 10, 9,  8,
                                                 63, 62, 61, 60,
                                                 47, 46, 45, 44,
                                                 31, 30, 29, 28,
                                                 15, 14, 13, 12,
]

(I think this is right. "Rotating right around the x axis" means pointing your right thumb in the direction of increasing x, which I think of as "to the left", and curling your fingers in the direction of the rotation, which I think of as "top-moves-away". However, even with this much effort to avoid brain farts, I might have messed it up.)

Here's the Python code for converting the input into a C++ function body:

def to_cpp11_hex(mask):
    return "0x%04X'%04X'%04X'%04X" % (
        ((mask >> 48) & 0xFFFF),
        ((mask >> 32) & 0xFFFF),
        ((mask >> 16) & 0xFFFF),
        ((mask >> 0) & 0xFFFF),
    )

def print_function_body(original, rotated):
    bits_by_rotation = {}
    for b in xrange(64):
        o = original.index(b)
        r = rotated.index(b)
        rotl_amount = ((o - r) + 64) % 64
        bits_by_rotation.setdefault(rotl_amount, []).append(r)
    print '    uint64_t r = 0;'
    for amount, bits in sorted(bits_by_rotation.iteritems()):
        mask = sum(1 << b for b in bits)
        print '    r |= rotl(o, %d) & %s;' % (amount, to_cpp11_hex(mask))
    print '    return r;'

print_function_body(original, rotated_right_around_x_axis)

This script generates functions like the following:

inline constexpr uint64_t rotl(uint64_t x, int k)
{
    return (x << k) | (x >> (64-k));
}

uint64_t rotate_left_around_x_axis(uint64_t o)
{
    uint64_t r = 0;
    r |= rotl(o, 4) & 0x0000'00F0'0000'0000;
    r |= rotl(o, 8) & 0x0000'0000'0000'0F00;
    r |= rotl(o, 12) & 0x000F'0000'0000'0000;
    r |= rotl(o, 16) & 0xF000'0000'00F0'0000;
    r |= rotl(o, 24) & 0x0000'000F'0000'0000;
    r |= rotl(o, 28) & 0x0000'F000'0000'00F0;
    r |= rotl(o, 36) & 0x0F00'0000'000F'0000;
    r |= rotl(o, 40) & 0x0000'0000'F000'0000;
    r |= rotl(o, 48) & 0x0000'0F00'0000'000F;
    r |= rotl(o, 52) & 0x0000'0000'0000'F000;
    r |= rotl(o, 56) & 0x00F0'0000'0000'0000;
    r |= rotl(o, 60) & 0x0000'0000'0F00'0000;
    return r;
}

uint64_t rotate_right_around_x_axis(uint64_t o)
{
    uint64_t r = 0;
    r |= rotl(o, 4) & 0x0000'0000'00F0'0000;
    r |= rotl(o, 8) & 0x0000'F000'0000'0000;
    r |= rotl(o, 12) & 0x0000'0000'0000'000F;
    r |= rotl(o, 16) & 0x000F'0000'0F00'0000;
    r |= rotl(o, 24) & 0x0000'0000'0000'00F0;
    r |= rotl(o, 28) & 0x00F0'0000'F000'0000;
    r |= rotl(o, 36) & 0x0000'000F'0000'0F00;
    r |= rotl(o, 40) & 0x0F00'0000'0000'0000;
    r |= rotl(o, 48) & 0x0000'00F0'0000'F000;
    r |= rotl(o, 52) & 0xF000'0000'0000'0000;
    r |= rotl(o, 56) & 0x0000'0000'000F'0000;
    r |= rotl(o, 60) & 0x0000'0F00'0000'0000;
    return r;
}

I don't immediately see any improvements. I had various ideas, such as trying to incrementally rotate o by repetitions of

    r |= (o = rotl(o, 4-or-8)) & MASK;

or limiting all the masks to 32 bits for x86-64-friendliness

def print_function_body_32bit(original, rotated):
    bits_by_rotation = {}
    for b in xrange(32, 64):
        o = original.index(b)
        r = rotated.index(b)
        rotl_amount = ((o - r) + 64) % 64
        bits_by_rotation.setdefault(rotl_amount, []).append(r)
    print '    uint64_t r = 0;'
    for amount, bits in sorted(bits_by_rotation.iteritems()):
        mask = sum(1 << b for b in bits)
        print '    r |= rotl(o, %d) & %s;' % ((amount + 32) % 64, to_cpp11_hex((mask >> 32) & 0xFFFFFFFF))
    print '    r <<= 32;'
    bits_by_rotation = {}
    for b in xrange(0, 32):
        o = original.index(b)
        r = rotated.index(b)
        rotl_amount = ((o - r) + 64) % 64
        bits_by_rotation.setdefault(rotl_amount, []).append(r)
    for amount, bits in sorted(bits_by_rotation.iteritems()):
        mask = sum(1 << b for b in bits)
        print '    r |= rotl(o, %d) & %s;' % (amount, to_cpp11_hex(mask & 0xFFFFFFFF))
    print '    return r;'

which generates function bodies like

uint64_t rotate_right_around_x_axis(uint64_t o)
{
    uint64_t r = 0;
    r |= rotl(o, 36) & 0x0000'0000'0000'0000;
    r |= rotl(o, 44) & 0x0000'0000'0000'0000;
    r |= rotl(o, 48) & 0x0000'0000'000F'0000;
    r |= rotl(o, 56) & 0x0000'0000'0000'0000;
    r |= rotl(o, 60) & 0x0000'0000'00F0'0000;
    r |= rotl(o, 4) & 0x0000'0000'0000'000F;
    r |= rotl(o, 16) & 0x0000'0000'0000'00F0;
    r |= rotl(o, 24) & 0x0000'0000'0000'0000;
    r <<= 32;
    r |= rotl(o, 8) & 0x0000'0000'0000'0000;
    r |= rotl(o, 16) & 0x0000'0000'0F00'0000;
    r |= rotl(o, 28) & 0x0000'0000'F000'0000;
    r |= rotl(o, 36) & 0x0000'0000'0000'0F00;
    r |= rotl(o, 40) & 0x0000'0000'0000'0000;
    r |= rotl(o, 48) & 0x0000'0000'0000'F000;
    r |= rotl(o, 52) & 0x0000'0000'0000'0000;
    r |= rotl(o, 60) & 0x0000'0000'0000'0000;
    return r;
}

which are nevertheless longer (in terms of number of assembly instructions) than the more "naïve" 64-bit version.

Writing an appropriate benchmark for the various possibilities, and writing out the permutation matrices for rotated_right_around_y_axis and rotated_right_around_z_axis, are left as exercises for the interested reader.

It would also be supremely interesting to feed these "rotation-permutation" functions to a superoptimizer and see what it produces. However, my wild-ass guess is that the optimal instruction sequence is in excess of 40 instructions, which would (I think) make it intractable for the current state-of-the-art in superoptimizers. But I could be wrong. Anyway, if you find out, it would be interesting to post the results here.


Reducing using/foreach/using nesting with a helper extension Problem statement

There are sometimes foreach scenarios that require deep nesting due to multiple disposable objects involved that look like this:

using(..)
{
    foreach(..)
    {
        using(..)
        {

        }
    }
}

or a real-world example from one of my applications:

using (_logger.BeginScope().CorrelationHandle("TestBundle").AttachElapsed())
using (Disposable.Create(() =>
{
    foreach (var item in cache.Values) item.Dispose();
}))
{
    _logger.Log(Abstraction.Layer.Service().Meta(new { TestBundleFileName = testBundle.FileName }));
    foreach (var current in tests)
    {
        using (_logger.BeginScope().CorrelationHandle("TestCase").AttachElapsed())
        {
            // body
        }
    }
}

I find this code is very ugly and there is virtually no other way of writing this (or it's just me and I cannot think of any).

Suggested solution

So, in order to make it prettier I created the Using extension that encapsulates the outer and inner usings within the loop.

Its body is identical to the using/foreach/using pattern I'd like to get rid of elsewhere:

public static class EnumerableExtensions
{
    public static IEnumerable<(TItem Current, TInner Context)> Using<TItem, TOuter, TInner>
    (
        this IEnumerable<TItem> source,
        TOuter outer,
        Func<TItem, TInner> inner
    )
    {
        using (new Disposer<TOuter>(outer))
        {
            foreach (var item in source)
            {
                using (var context = new Disposer<TInner>(inner(item)))
                {
                    yield return (item, context);
                }
            }
        }
    }
}

This is using a helper struct for disposing TOuter and TInner

public readonly struct Disposer<T> : IDisposable
{
    public Disposer(T value)
    {
        Value = value;
    }

    public T Value { get; }

    public void Dispose()
    {
        if (Value is IDisposable disposable)
        {
            disposable.Dispose();
        }
        else
        {
            foreach (var property in typeof(T).GetProperties())
            {
                if (property.GetValue(Value) is IDisposable disposableProperty)
                {
                    disposableProperty.Dispose();
                }
            }
        }
    }

    public static implicit operator T(Disposer<T> disposer) => disposer.Value;
}

And here is the Disposable helper:

public class Disposable : IDisposable
{
    private readonly Action _dispose;

    private Disposable(Action dispose)
    {
        _dispose = dispose;
    }

    public static IDisposable Empty => new Disposable(() => { });

    public static IDisposable Create(Action dispose)
    {
        return new Disposable(dispose);
    }

    public void Dispose()
    {
        _dispose();
    }
}

Example usage

Now, when I have such a scenario, I can put everything inside the loop:

void Main()
{
    var numbers = new[] { 1, 2, 3 };

    foreach (var (item, context) in numbers.Using
    (
        outer: Disposable.Create (() => Console.WriteLine("Outer disposed.")),
        inner: _ => Disposable.Create (() => Console.WriteLine("Inner disposed.")))
    )
    {
        item.Dump();
        context.Dump();
    }
}

Then, when I refactor the previous ugly piece of code with this extension it turns into this:

foreach (var (current, context) in tests.Using
(
    outer: new
    {
        Scope = _logger.BeginScope().CorrelationHandle("TestBundle").AttachElapsed(),
        CleanUp = Disposable.Create(() =>
        {
            foreach (var item in cache.Values) item.Dispose();
        })
    },
    inner: _ => _logger.BeginScope().CorrelationHandle("TestCase").AttachElapsed())
)
{
    // body
}

Questions

What do you think of this helper?
Would you say the code is now easier to read?
Can the Using extension or the Disposer<T> be improved?

I'd argue that no, this does not make the code easier to understand. Instead of a combination of familiar general-purpose constructs (using and foreach), you now have an undocumented custom special-purpose construct Using(outer, inner) that must be understood before one can make sense of the code. This use-case seems too uncommon to be worth that trade-off.

The main problem I have with the original piece of code is that it looks like a 'camel' because of the Disposable.Create hump at the start. Your alternative is still a camel, just with a bigger hump at the front and a flatter one at the back. With a DisposeAll extension method, you could turn it into a dromedary. It's still a tall hump, but now the shape actually matches its single-loop nature:

using (_logger.BeginScope().CorrelationHandle("TestBundle").AttachElapsed())
using (Disposable.Create(() => cache.Values.DisposeAll()))
{
    _logger.Log(Abstraction.Layer.Service().Meta(new { TestBundleFileName = testBundle.FileName }));
    foreach (var current in tests)
    {
        using (_logger.BeginScope().CorrelationHandle("TestCase").AttachElapsed())
        {
            // TODO
        }
    }
}

Another thing you can do is to move the loop body to a separate method or local function.


Auto-Format GPA while typing I'm looking for a more cleaner and simpler solution to auto-format a text-box field intended only for the user to enter their college Grade Point Average (GPA). Here is my code:

<label for="collegeGPA">GPA:</label>
<input type="text" name="collegeGPA" id="collegeGPA" maxlength="4" style="width:45px;">

<script type="text/javascript">

//<!-- When executed this script [BELOW] auto-formats a Grade Point Average (GPA) field given the element ID [BEGIN] -->

function FormatGPA(gPAElementID) {

    re = /\D/g; // remove any characters that are not numbers
    gPAVal = document.getElementById(gPAElementID).value.replace(re,"");
    gPAValFirstCharacter = gPAVal.charAt(0);

    if ( ( gPAVal >= 0 ) &&  ( gPAValFirstCharacter < 5 ) )
        {

            gPALen = gPAVal.length;

            if( gPALen > 1)
                {
                    gPAa=gPAVal.slice(0,1);
                    gPAb=gPAVal.slice(1);
                    document.getElementById(gPAElementID).value = gPAa + "." + gPAb;
                }
        else
                {
                    document.getElementById(gPAElementID).value=gPAVal;
                };

        }

    else
        {
            document.getElementById(gPAElementID).value="";
        };

};

//<!-- When executed this script [ABOVE] auto-formats a Grade Point Average (GPA) field given the element ID [END] -->

document.getElementById('collegeGPA').onblur = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').onfocus = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').onkeypress = function (e) { FormatGPA(this.id); };
document.getElementById('collegeGPA').oninput = function (e) { FormatGPA(this.id);  };
document.getElementById('collegeGPA').onchange = function (e) { FormatGPA(this.id); };

</script>

...and here is my JSFiddle: http://jsfiddle.net/JamesAndersonJr/kxaBJ/1/
I'm looking for a simpler way to format the GPA input dynamically (i.e. as the user types it in).
The format should be:

1st Character: any digit 0-4
2nd Character: Always a Period (should    also be auto-inserted after
user types a valid first digit)
3rd Character: any digit
4th Character: any digit

UPDATE: Lowest possible GPA is 0.00, not 1.00.

First of all, I'd be hesitant to use as-you-type correction to begin with. It doesn't take much for something like that to be an annoyance rather than a help. It's probably better to validate the input only when it loses focus, or when the form is about to be submitted, and then inform the user somehow.

Point is, it's not hard make your code trip over something. For instance, I just pasted in "2312" and got a GPA with 3 decimal places. I also tried typing in a valid value, and then went back with the arrow keys and removed the first digit with the intention of replacing it - but removing that first digit cleared the entire input. If I just select the first digit, and hit a key, my input gets ignored, valid or not. I can't manually enter a decimal point. The cursor jumps around... and so on.

Unless your system is perfect and takes all possible interactions into account, it'll likely be very annoying and frustrating to use.

Point is, it's a lot easier to validate stuff, once it's all been entered. For instance, this regular expression /^([0-3](\.\d\d?)?|4(.00?)?)$/ will match a valid GPA with 0-2 decimal places.

But don't change the value! You can format it, if it's otherwise valid (like adding ".00" if the user just typed "4"), but leave it at that. Just call attention to the error, optimally with an explanation of why the value is invalid.

In any event, presuming this is being sent to a server, always, always validate it on the server. Never trust the client™

But there's still code to review here, regardless of its use.

Code notes:

Declare your variables with var. Right now, every variable is global! Big no-no.
Don't find and re-find an element with getElementById; find it once, stick it in a variable, and use that instead.
Don't use the old-school .onblur = etc. event handling. Favor addEventListener instead (or use a library to do the event handler registration for maximum compatibility)

More broadly, there's no need to keep declaring a function like function (e) { FormatGPA(this.id); } for each event. Declare that function once, and use it for every event. Or simply don't use an argument for the format function, and let it know what element to find. Or, if you use addEventListener, the element will be available as this in the event handler, i.e.:

var someInput = document.getElementById("some-input");
someInput.addEventListener("click", someHandler, false);

function someHandler() {
  alert(this.value); // `this` refers to the element that triggered the event
}

(using alert just as an example - it's obviously not an ideal way to provide feedback.)

Style notes:

Function names and variables should be lowerCamelCase (unless they are constructors), so: formatGPA. And if you've got an acronym at the start of your name, make it all-lowercase, i.e. gpaElementId.
Don't use HTML comment syntax in your JS code; it doesn't matter, and makes little sense.
Avoid the brace-on-new-line style. JavaScript has automatic semicolon insertion, which (long story short) means that you should stick to a style like

if(condition) {

} else {

}


Entity Framework Core verify SaveChanges count I am using ASP.NET Core 3.1 and have been assigned a task to verify the count of changes done using SaveChanges().
It is expected that the developer should know how many records will be changed before-hand when SaveChanges() will be called.
To implement it, I have created an extension method for DbContext called SaveChangesAndVerify(int expectedChangeCount) where I am using transaction and equating this parameter with the return value of SaveChanges().
If the values match, the transaction is committed and if it doesn't match, the transaction is rolled back.
Please check the code below and let me know if it would work and if there are any considerations that I need to make. Also, is there a better way to do this?
public static class DbContextExtensions
{
    public static int SaveChangesAndVerify(this DbContext context, int expectedChangeCount)
    {
        context.Database.BeginTransaction();
        var actualChangeCount = context.SaveChanges();
        if (actualChangeCount == expectedChangeCount)
        {
            context.Database.CommitTransaction();
            return actualChangeCount;
        }
        else
        {
            context.Database.RollbackTransaction();
            throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
        }
    }

    public static async Task<int> SaveChangesAndVerifyAsync(this DbContext context, int expectedChangeCount, CancellationToken cancellationToken = default)
    {
        await context.Database.BeginTransactionAsync();
        var actualChangeCount = await context.SaveChangesAsync();
        if(actualChangeCount == expectedChangeCount)
        {
            context.Database.CommitTransaction();
            return actualChangeCount;
        }
        else
        {
            context.Database.RollbackTransaction();
            throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
        }
    }
}

A sample usage would be like context.SaveChangesAndVerify(1) where a developer is expecting only 1 record to update.

I can't think of another way of doing this - only the database knows how many rows have been affected. That said, your code has some problems.
You need to dispose of your transaction when you're done with it. This has 2 additional benefits:

You don't need to rollback manually
You don't need to worry about exceptions in SaveChanges (you haven't handled that at all at the moment).

Let's look at how that changes things:
public static int SaveChangesAndVerify(
    this DbContext context,
    int expectedChangeCount)
{
    using (var transaction = context.Database.BeginTransaction())
    {
        var actualChangeCount = context.SaveChanges();
        if (actualChangeCount == expectedChangeCount)
        {
            transaction.Commit();
            return actualChangeCount;
        }
        throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
    }
}


Google Code Jam 2014: Full Binary Tree Here is my entry for the Code Jam problem Full Binary Tree (I didn't compete but tackled the problem afterwards). It does solve the provided inputs, so I suppose it is correct. I am looking for any suggestions regarding the code. One thing I don't like is the fact that the Node constructor clears the array that it receives as a parameter, do you see a clean way to fix this ? Is it possible without declaring an extra class wrapping Node ?

import java.util.*;

class Node {
    static NodeComparator cmp = new NodeComparator();

    private ArrayList<Node> children = new ArrayList<>();
    private int nDescendants = 0;    // descendants = number of children and, recursively, children of children, etc.

    public int getNDescendants() {
        return nDescendants;
    }

    // turns the tree into a full binary tree by removing as few nodes as possible
    public void makeFullBinary() {
        // first, recursively make all the children into full binary trees
        for (Node n : children) {
            n.makeFullBinary();
        }
        // sort the children from least to most number of descendants
        Collections.sort(children, cmp);
        // remove descendants as needed to end up with 0 or 2
        while ((children.size() != 0) && (children.size() != 2)) {
            children.remove(0);
        }
        // recompute the number of descendants
        nDescendants = 0;
        for (Node c : children) {
            nDescendants += 1 + c.getNDescendants();
        }
    }

    // builds a tree from an array of booleans (adjacency matrix)
    // edges is destroyed after the constructor is called
    public Node(boolean[][] edges, int root) {
        for (int i = 0; i < edges.length; i++) {
            if (edges[root][i]) {
                edges[root][i] = false;
                edges[i][root] = false;
                children.add(new Node(edges, i));
            }
        }
        for (Node c : children) {
            nDescendants += 1 + c.getNDescendants();
        }
    }
}

// comparator for sorting only
class NodeComparator implements Comparator<Node> {
    @Override
    public int compare(Node n0, Node n1) {
        return n0.getNDescendants() - n1.getNDescendants();
    }
}

public class Program {

    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);

        int nCases = in.nextInt();
        in.nextLine();

        for (int caseNumber = 1; caseNumber <= nCases; caseNumber++) {
            int nNodes = in.nextInt();
            in.nextLine();

            boolean[][] edges = new boolean[nNodes][nNodes];

            for (int i = 0; i < nNodes-1; i++) {
                String[] s = in.nextLine().split(" ");
                int x = Integer.parseInt(s[0])-1;
                int y = Integer.parseInt(s[1])-1;
                edges[x][y] = true;
                edges[y][x] = true;
            }

            int maxDescendants = 0;
            boolean [][] e = new boolean[nNodes][nNodes];
            for (int i = 0; i < nNodes; i++) {
                for (int a = 0; a < nNodes; a++) {
                    for (int b = 0; b < nNodes; b++) {
                        e[a][b] = edges[a][b];
                    }
                }
                Node tree = new Node(e, i);
                tree.makeFullBinary();
                maxDescendants = Math.max(maxDescendants, tree.getNDescendants());
            }

            System.out.print("Case #");
            System.out.print(caseNumber);
            System.out.print(": ");
            System.out.println(nNodes - maxDescendants - 1);
        }
        in.close();
    }
}

Oke I'm going to have with this review the point of javadoc, namings and methods.

First :

You make comment, what is actually good but try to make javadoc in stead of commenting everything.

// comparator for sorting only
class NodeComparator implements Comparator<Node> {
    @Override
    public int compare(Node n0, Node n1) {
        return n0.getNDescendants() - n1.getNDescendants();
    }
}

should be :

/**
* Comparator for sorting on descendants
*/
class NodeComparatorNDescendants implements Comparator<Node> {
    @Override
    public int compare(Node n0, Node n1) {
        return n0.getNDescendants() - n1.getNDescendants();
    }
}

Why :
This cause you no more effort than commenting, but you can generate a complete javadoc what other persons can look into.
Also see that I added the on what you are sorting there.
At last I changed also the class name. Now you will already see without the javadoc on what you will compare.
Note : Effective class name helps you later also, take this code after a year not seen and you will need to see the implementation of the comparator if you want to know on what it compares.

Second :

Like I already said in the first point, naming is everything.
Node n => why just not Node node.
Other example : NodeComparator cmp => NodeComparator nodeComparator
This will not take more memory from your program if you have fear for that :), and if you use autocomplete this will not lose time to program.

Thirth :

Small effective methods are better than big ones that do a lot.
How less code in a method => the sooner you find the bug because your method complexity is lower.

Example :

// turns the tree into a full binary tree by removing as few nodes as possible
public void makeFullBinary() {
    // first, recursively make all the children into full binary trees
    for (Node n : children) {
        n.makeFullBinary();
    }
    // sort the children from least to most number of descendants
    Collections.sort(children, cmp);
    // remove descendants as needed to end up with 0 or 2
    while ((children.size() != 0) && (children.size() != 2)) {
        children.remove(0);
    }
    // recompute the number of descendants
    nDescendants = 0;
    for (Node c : children) {
        nDescendants += 1 + c.getNDescendants();
    }
}

should be :

// turns the tree into a full binary tree by removing as few nodes as possible
public void makeFullBinary() {

    createBinaryTree();
    Collections.sort(children, cmp);
    adjustBinaryTree();
    recalculateDescendants();
}

private void createBinaryTree () {
    for (Node n : children) {
        n.makeFullBinary();
    }
}

private void adjustBinaryTree () {
    while ((children.size() != 0) && (children.size() != 2)) {
        children.remove(0);
    }
}

private void recalculateDescendants () {
    nDescendants = 0;
    for (Node c : children) {
        nDescendants += 1 + c.getNDescendants();
    }
}

Why :
Each method is shorter then the whole method.
It's easier to read and to comprehent the actual doing of the method.
Now there is just one thing what bothers me, but it is an discussion amoung programmers, that's the reason I didn't changed it but still going to say
nDescendants is a calculated field.
For more rightness you should remove the int and set the recalculateDescendants code in the getNDescendants().
Yes you lose performance and you gain an always correctness of the getter.
Now you see directly why there are always discussions about that.

Hope this helps you already to improve yourself.


Checking a Model's function's return value and setting values to a View member I have a WinForms project that uses the Passive View MVP pattern. In the Presenter of one of my Views I have a function that gets questions and from its Model. But I want to make sure these collections are not empty so that I can inform the user when they are. Right now my code seems very ugly and I wonder if anyone has any tips to make it shorter/better. Thank you.

public void InitializeInterview(object sender, EventArgs e)
    {
        List<List<string>> questionList = new List<List<string>>();
        List<List<string>> maturityAnswerList = new List<List<string>>();
        List<List<string>> complianceAnswerList = new List<List<string>>();

        questionList = _model.GetQuestions();
        maturityAnswerList = _model.GetMaturityAnswers();
        complianceAnswerList = _model.GetComplianceAnswers();

        if (questionList != null && questionList.Any())
        {
            _view.Questions = questionList;
        }
        else
        {
            _view.Message("No questions have been found, please add a new question standard");
            _view.CloseView();
        }
        if (maturityAnswerList != null && maturityAnswerList.Any())
        {
            _view.MaturityAnswers = maturityAnswerList;
        }
        else
        {
            _view.Message("No maturity answers have been found, please add a new maturity standard");
            _view.CloseView();
        }
        if (complianceAnswerList != null && complianceAnswerList.Any())
        {
            _view.ComplianceAnswers = complianceAnswerList;
        }
        else
        {
            _view.Message("No compliance answers have been found, please add a new compliance standard");
            _view.CloseView();
        }

    }

Don't call List<T> "somethingList" -- e.g. if they are questions, call it questions, not questionList.

Why do you do List<List<string>> questionList = new List<List<string>>(); if the next step is questionList = _model.GetQuestions();? Instead don't bother with the initialization and just assign it directly: var questionList = _model.GetQuestions();

List<List<string>> is a warning sign to me that your design is probably flawed, and when I look at the context -- a list of questions and a separate list of answers -- I fear that your code has far worse issues than the reason for this question.

Is there a reason you're doing this in an "outdated" technology like WinForms when WPF etc. exists? There is a strong link between data and UI in this method, which is a frequent issue with WinForms; if this was WPF you could implement MVVM to separate those concerns.


Display four random entries from an array This code will take the array shown and display 4 random entries in the format shown (or near enough).

Are there any parts of this that can be optimised? I'm not happy about the if statement.

<?php
function shuffle_assoc($list) {
  if (!is_array($list)) return $list;

  $keys = array_keys($list);
  shuffle($keys);
  $random = array();
  $i = 0;
  foreach ($keys as $key) {
    $random[$key] = $list[$key];
    if ($i >= 3){
      break;
    }
    $i++;
  }
  return $random;
}

$arr = array();
$arr[] = array('img' => "http://placehold.it/150x150", 'link' => '#1', 'text1' => "ONE 1", 'text2' => 'ONE 2');
$arr[] = array('img' => "http://placehold.it/200x150", 'link' => '#2', 'text1' => "TWO 1", 'text2' => 'TWO 2');
$arr[] = array('img' => "http://placehold.it/250x150", 'link' => '#3', 'text1' => "THREE 1", 'text2' => 'THREE 2');
$arr[] = array('img' => "http://placehold.it/300x150", 'link' => '#4', 'text1' => "FOUR 1", 'text2' => 'FOUR 2');
$arr[] = array('img' => "http://placehold.it/350x150", 'link' => '#5', 'text1' => "FIVE 1", 'text2' => 'FIVE 2');
$arr[] = array('img' => "http://placehold.it/400x150", 'link' => '#6', 'text1' => "SIX 1", 'text2' => 'SIX 2');
$arr[] = array('img' => "http://placehold.it/450x150", 'link' => '#7', 'text1' => "SEVEN 1", 'text2' => 'SEVEN 2');

foreach(shuffle_assoc($arr) as $key => $value)
{
    echo '

';
    echo '
'.$value["link"].'
';
    echo '
'.$value["text1"].'
';
    echo '
'.$value["text2"].'
';
    echo '
';
}
?>

I think the function is not named properly.  You are NOT shuffling an associative array.  You are shuffling a numerically indexed array (whose values happen to be associative arrays).

As such, I don't see a great need for this function.  You could use array_rand() to pick random keys from the array.

Now, assuming you did want to keep such a function, for example to actually get array of values not just keys.  I would consider a few things:

Consider throwing an exception or logging error if function is passed a non-array value.  This can help make sure you are invoking this function properly in your code vs. just returning the passed value unchanged.
Consider passing "limit" value to the function vs. hard-coding for 4 return elements.

This might yield something like:

function array_rand_values($arr, $limit = 1) {
    if(!is_array($arr)) {
        throw new InvalidArgumentException('Array expected for first argument.');
    }
    if(!is_int($limit) || $limit < 1) {
        throw new InvalidArgumentException(
            'Positive integer value expected for second argument.'
        );
    }
    if(count($arr) <= $limit) {
       shuffle($arr);
       return $arr;
    }
    $keys = array_rand($arr, $limit);
    $values = array();
    for ($i = 0; $i < $limit; $i++) {
        $values[] = $arr[$keys[$i]];
    }
    return $values;
}

Note that this function does not preserve keys if passed an associative array.  This should probably be the logical behavior in such a case, as "shuffling" an associative array typically makes zero sense.  This would however still pick X random values from an associative array.

If one needed to pick X random key/value pairs from an associative array, I would suggest a separate function, so that intent of caller is clear.  That function may be implemented in very similar manner  to the one above, with single line of code change.

function array_rand_values_assoc($arr, $limit = 1) {
    // same code as above until assignment in loop

    for ($i = 0; $i < $limit; $i++) {
        $values[$keys[$i]] = $arr[$keys[$i]];
    }
    return $values;
}


Simple blog engine for educational purposes I am coding a simple blog engine for educational purposes.

In the PostsController I have two two methods that share some common code namely, Add and Edit:

[HttpPost]
public ActionResult Add(PostInputModel model)
{
    if (!ModelState.IsValid) return View(model);

    // map the input model to an entity model using AutoMapper (extension method).
    var post = model.MapTo<Post>();
    post.Summary = Summarize(post.Content);
    post.PublishedAt = DateTime.Now;
    // convert the post title to a slug (Hello World becomes hello-world for example)
    post.Slug = SlugConverter.Convert(post.Title);
    AttachTags(post);

    // check whether the slug is occupied.
    if (repository.Posts.Any(p => p.Slug == post.Slug))
    {
        ModelState.AddModelError("", "title in use");
        return View(model);
    }

    repository.Posts.Add(post);
    repository.SaveChanges();

    // indicates that a "post published. available here." message should be displayed.
    TempData["newPost"] = true;
    // redirect the user to the edit page
    return RedirectToAction("Edit", new { slug = post.Slug });
}

[HttpPost]
public ActionResult Edit(PostInputModel model)
{
    if (!ModelState.IsValid) return View("Edit", model);

    var post = repository.Posts.SingleOrDefault(p => p.Slug == model.Slug);
    // if the post does not exist return an error
    if (post == null) return HttpNotFound("you cannot edit a post that doesn't exist");
    repository.Posts.Remove(post);
    repository.SaveChanges();

    // map the input model to an entity model using AutoMapper (extension method).
    model.MapPropertiesToInstance(post);
    AttachTags(post);

    // convert the title to a slug (Hello World becomes hello-world for example)
    post.Slug = SlugConverter.Convert(post.Title);
    post.Summary = Summarize(post.Content);

    // check whether the slug is occupied.
    if (repository.Posts.Any(p => p.Slug == post.Slug))
    {
        ModelState.AddModelError("", "title in use");
        return View("Edit", model);
    }

    repository.Posts.Add(post);
    repository.SaveChanges();

    return Edit(post.Slug);
}

private static string Summarize(string content)
{
    if (!content.Contains(Environment.NewLine)) return content;
    return content.Split(new[] { Environment.NewLine }, StringSplitOptions.None)[0];
}

private void AttachTags(Post post)
{
    foreach (var tag in post.Tags)
    {
        if (repository.Tags.Any(x => x.Name == tag.Name))
        {
            repository.Tags.Attach(tag);
        }
    }
}

I have cleaned the code and reduced some duplication, but I am sure this code could be improved further. How can I make this code more DRY?

The common code is about updating the post, so I'd extract this into it's own method:

private bool UpdatePost(Post post)
{
    // convert the title to a slug (Hello World becomes hello-world for example)
    post.Slug = SlugConverter.Convert(post.Title);
    post.Summary = Summarize(post.Content);

    AttachTags(post);

    // check whether the slug is occupied.
    if (repository.Posts.Any(p => p.Slug == post.Slug))
    {
        ModelState.AddModelError("", "title in use");
        return false;
    }

    repository.Posts.Add(post);
    repository.SaveChanges();
    return true;
}

With that your Edit and Add methods can be reduced to:

[HttpPost]
public ActionResult Add(PostInputModel model)
{
    if (!ModelState.IsValid) return View(model);

    // map the input model to an entity model using AutoMapper (extension method).
    var post = model.MapTo<Post>();
    post.PublishedAt = DateTime.Now;

    if (!UpdatePost(post))
    {
        return View(model);
    }

    // indicates that a "post published. available here." message should be displayed.
    TempData["newPost"] = true;
    // redirect the user to the edit page
    return RedirectToAction("Edit", new { slug = post.Slug });
}

[HttpPost]
public ActionResult Edit(PostInputModel model)
{
    if (!ModelState.IsValid) return View("Edit", model);

    var post = repository.Posts.SingleOrDefault(p => p.Slug == model.Slug);
    // if the post does not exist return an error
    if (post == null) return HttpNotFound("you cannot edit a post that doesn't exist");
    repository.Posts.Remove(post);
    repository.SaveChanges();

    // map the input model to an entity model using AutoMapper (extension method).
    model.MapPropertiesToInstance(post);

    if (!UpdatePost(post))
    {
        return View(model);
    }
    return Edit(post.Slug);
}


Is your source code "pristine"? This was brought about by this codegolf question.

Problem

A program is said to be pristine if it is:

A working program (No errors).
The program will produce errors by removing any contiguous substring of N characters, where 1 <= N < program length.

My goal is to check if a given C# program's source is pristine or not with the assumption that the given program is already working.

What I Did

I generated all possible removal combinations and compiled the resulting string with CompileAssemblyFromSource. Compile errors are then checked with CompilerResults.Errors.

static bool isPristine(string sourceCode)
{
    using (var csc = new CSharpCodeProvider(new Dictionary<string, string>() { { "CompilerVersion", "v3.5" } }))
    {
        var parameters = new CompilerParameters(new[] { "mscorlib.dll", "System.Core.dll" });
        parameters.GenerateExecutable = true;

        int length = sourceCode.Length;

        for (int i = 0; i < length; i++)
        {
            for (int j = 1; j < length - i; j++)
            {
                string sourceWithRemoval = sourceCode.Remove(i, j);
                CompilerResults results = csc.CompileAssemblyFromSource(parameters, sourceWithRemoval);

                if (results.Errors.Count == 0)
                {
                    Debug.WriteLine("Still works by removing:");
                    Debug.WriteLine("@{0}@ ", sourceCode.Substring(i, j));
                    Debug.WriteLine("At index {0}.", i);

                    return false;
                }
            }
        }
    }

    return true;
}

Usage

static void Main(string[] args)
{
    string sourceCode = "class e{static void Main(){System.Console.Write(new char[1\n#if l\n#else\n*5]{'w','o','r','l','d'\n#if o\n#else\n});\n#endif\n}\n#endif\n}";

    Console.WriteLine(isPristine(sourceCode) ? "Pristine" : "Not pristine");
    Console.ReadLine();
}

Main Concerns

What can I improve (performance, readability)?
Are there better, faster C# techs that I should have used?

A few minors really:

Standard C# naming convention for methods is PascalCase. So should be IsPristine.
If it's important (or nice) to know which specific sequence is yielding the false result then you should consider returning that from the method. Something like this:

public class PristineResult
{
    public readonly bool IsPristine;
    public readonly string OffendingSequence;
    public readonly string OffendingIndex;

    public PristineResult(bool isPristine, string offendingSequence = null, int offendingIndex = -1)
    {
        IsPristine = isPristine;
        OffendingSequence = offendingSequence;
        OffendingIndex = offendingIndex;
    }
}

static PristineResult IsPristine(string sourceCode)
{
    using (var csc = new CSharpCodeProvider(new Dictionary<string, string>() { { "CompilerVersion", "v3.5" } }))
    {
        var parameters = new CompilerParameters(new[] { "mscorlib.dll", "System.Core.dll" });
        parameters.GenerateExecutable = true;

        int length = sourceCode.Length;

        for (int i = 0; i < length; i++)
        {
            for (int j = 1; j < length - i; j++)
            {
                string sourceWithRemoval = sourceCode.Remove(i, j);
                CompilerResults results = csc.CompileAssemblyFromSource(parameters, sourceWithRemoval);

                if (results.Errors.Count == 0)
                {
                    return new PristineResult(false, sourceCode.Substring(i, j), i);
                }
            }
        }
    }

    return new PristineResult(true);
}


Evaluate/Refactor my ASP.NET XSS Security Helper Class I am maintaining a site with significant security concerns and I wrote a helper class for validating potential XSS attacks.  The ValidateRequest method is meant to evaluate the HttpRequest for any potential issues, and if issues are found, redirect the user to the same page, but in a away that is not threatening to the application.

The application is being evaluated by HP WebInspect.  Their initial review included two regular expressions:

The "Regex for a simple XSS attack"

/((\%3C) <)((\%2F) \/)*[a-z0-9\%]+((\%3E) >)/ix
The "Paranoid regex for XSS attacks"

/((\%3C) <)[^\n]+((\%3E) >)/I

The regular expression I am using is a more stringent C# adaptation of the paranoid regex.  I prefer to err on the side of caution, so I would rather risk denying valid requests than allowing an invalid request to be processed.

Thanks in advance for the help.

using System;
using System.Text.RegularExpressions;
using System.Web;

public static class XssSecurity
{

public const string PotentialXssAttackExpression = "(http(s)*(%3a|:))|(ftp(s)*(%3a|:))|(javascript)|(alert)|(((\\%3C) <)[^\n]+((\\%3E) >))";

private static readonly Regex PotentialXssAttackRegex = new Regex(PotentialXssAttackExpression, RegexOptions.IgnoreCase);

public static bool IsPotentialXssAttack(this HttpRequest request)
{
    if(request != null)
    {
        string query = request.QueryString.ToString();
        if(!string.IsNullOrEmpty(query) && PotentialXssAttackRegex.IsMatch(query))
            return true;
        if(request.HttpMethod.Equals("post", StringComparison.InvariantCultureIgnoreCase))
        {
            string form = request.Form.ToString();
            if (!string.IsNullOrEmpty(form) && PotentialXssAttackRegex.IsMatch(form))
                return true;
        }
        if(request.Cookies.Count > 0)
        {
            foreach(HttpCookie cookie in request.Cookies)
            {
                if(PotentialXssAttackRegex.IsMatch(cookie.Value))
                {
                    return true;
                }
            }
        }
    }
    return false;
}

   public static void ValidateRequest(this HttpContext context, string redirectToPath = null)
   {
       if(context == null || !context.Request.IsPotentialXssAttack()) return;

       // expire all cookies
       foreach(HttpCookie cookie in context.Request.Cookies)
       {
           cookie.Expires = DateTime.Now.Subtract(TimeSpan.FromDays(1));
           context.Response.Cookies.Set(cookie);
       }

       // redirect to safe path
       bool redirected = false;
       if(redirectToPath != null)
       {
           try
           {
               context.Response.Redirect(redirectToPath,true);
               redirected = true;
           }
           catch
           {
               redirected = false;
           }
       }
       if (redirected)
           return;

       string safeUrl = context.Request.Url.AbsolutePath.Replace(context.Request.Url.Query, string.Empty);
       context.Response.Redirect(safeUrl,true);
   }
}

Some general idea:

Use guard statements instead of big if blocks.
Extract the !string.IsNullOrEmpty(query) && PotentialXssAttackRegex.IsMatch(query) condition to a method.
Extract individual checks to distinct methods. The IsPotentialXssAttack will be quite easy to read and the code of different validations will not be mixed in one big method.
I'd create an expireAllCookies helper method too.

The final code of the first method would look like this (I haven't tested nor compiled):

public static bool IsPotentialXssAttack(this HttpRequest request) {
    if (request == null) {
        return false;
    }
    if (isPotentialXssAttackQuery(request)) {
        return true;
    }
    if (isPotentialXssAttackFormOrAnything(request)) {
        return true;
    }
    if (isPotentialXssAttackCookie(request)) {
        return true;
    }

    return false;
}

public static bool isPotentialXssAttackQuery(this HttpRequest request) {
    string query = request.QueryString.ToString();
    if (isMatch(query)) {
        return true;
    }
    return false;
}

public static bool isMatch(string input) {
    if (string.IsNullOrEmpty(query)) {
        return false;
    }
    return PotentialXssAttackRegex.IsMatch(query);
}

// TODO: rename it
public static bool isPotentialXssAttackFormOrAnything(this HttpRequest request) {
    if (!request.HttpMethod.Equals("post",
            StringComparison.InvariantCultureIgnoreCase)) {
        return false;
    }

    string form = request.Form.ToString();
    if (isMatch(form)) {
        return true;
    }
    return false;
}

public static bool isPotentialXssAttackCookie(this HttpRequest request) {
    if (request.Cookies.Count <= 0) {
        return false;
    }

    foreach (HttpCookie cookie in request.Cookies){
        if (isMatch(cookie.Value)) {
            return true;
        }
    }
    return false;
}