Decoding binary coded decimal (BCD) value I have 3 questions, all of them are concerned with binary coded decimal (BCD) conversion. My coding snippets are listed as follows. But I don't like my algorithm because it takes too long when dealing with tons of data.

Any suggestions to improve the performance? Would you please show me an efficient and fast way to do that?

The value is expressed with 4-bits binary coded decimals (BCD), which was originally stored in a character buffer (for example, pointed by a pointer const unsigned char *).

BCD*2; 1001 0111 0110 0101=9765
"9"  "7"  "6"  "5"

unsigned int BCDn( unsigned int n, const unsigned char * data )
{
    unsigned int uResult = 0;
    unsigned char ucTmp;
    int iTmp1,iTmp2;

    for (unsigned int i=0;i<n;i++)
    {
        ucTmp = data[i];
        iTmp1 = (ucTmp & 0xf0) >> 4;
        iTmp2 = ucTmp & 0x0f;
        uResult += (iTmp1*10+iTmp2) * static_cast<unsigned int>(pow(100.0,static_cast<int>(n-1-i)));
    }

    return uResult;
}

The value is expressed by n binary integer (n* 8-bits). The first bit (MSB) defines
the sign of binary integer; "0" signifies that it's a positive integer, and "1" for negative one. In the case of negative number, the other bits show the complement number that is added 1.

         MSB          LSB
    I*2; 00101101 1001100=19999
    I*2; 10101101 10011100=(-1)*(0101101 10011100
                          =(-1)*(1010010 01100100) (after complement)
                          =-21092

int SINTn(unsigned int n, const unsigned char *data)
{
    int nResult;
    bool bNegative = false;

    if ((data[0] & 0x80)!= 0)
        isNegative = true;

    nResult = data[0] & 0x7f;

    for (unsigned int i=1;i<n;i++)
        nResult = nResult * 0x100 + data[i];

    if (bNegative)
        nResult = nResult - static_cast<int>(pow(2.0,static_cast<int>(n*8-1)));

    return nResult
}

unsigned int UINTn(unsigned int n, const unsigned char *data)
{
    unsigned int uResult = 0;

    for (unsigned int i=0;i<n;i++)
        uResult = uResult * 0x100 + data[i];

    return uResult;
}

R*n.m

The value is expressed by n-bytes (n*8 bits) binary number, the first bit (MSB)
defines the sign of it; "0" means positive and "1" means negative. The number m means
that the binary number should be multiplied by 10-m to get the value.

           MSB                        LSB
    R*4.0: 00000000 00000000 00000111 10110101=1973
    R*4.2: 00000000 00000000 00000111 10110101=1973*10-2=19.73
    R*4.5: 10000000 00000000 00000111 10110101=-1973*10-5= -0.01973
    R*2.0: 10101101 10011100 =-11676

double REALnm(unsigned int n, unsigned int m, const unsigned char * data)
{
    double dResult;
    bool bNegative = false;

    if ((data[0] & 0x80)!= 0)
        isNegative = true;

    dResult = data[0] & 0x7f;

    for (unsigned int i=1;i<n;i++)
        dResult = dResult * 0x100 + data[i];

    if (bNegative) dResult *= -1.0;

    dResult *= pow(10.0, (-1.0) * m);

    return dResult;
}

it can be a lot simpler...

#include "seatest.h"

unsigned int BCDn( unsigned int n, const unsigned char * data )
{
    unsigned int uResult = 0;
    unsigned int i;

    for (i=0;i<n;i++)
    {
        uResult = (uResult * 100) + ((data[i] >> 4) * 10 ) + ( data[i] & 0x0F);
    }

    return uResult;
}
void test_max_bcd_convert()
{
    unsigned char bcd_data[]= { 0x97, 0x65 };

    assert_int_equal(9765, BCDn(2, bcd_data));

}

void test_fixture_bcd( void )
{
    test_fixture_start();
    run_test(test_max_bcd_convert);
    test_fixture_end();
}

void all_tests( void )
{
    test_fixture_bcd();
}

int main( int argc, char** argv )
{
    run_tests(all_tests);
    return 0;
}


Schema.org Microdata broadest tag check I have defined the tag that can sit on all the pages of my website and be easily integrated into the admin tool. It is the broadest category with properties that are relevant for all pages.

Can you please tell me if it is written correctly?

I am concerned that the URL code line might be picked up as a circular link and as a result be penalised by the Search Engine:

    TITLE OF PAGE
    CONTENT OF PAGE
    <link itemprop="url" href="jmb-active.com/?page=URL-OF-THIS-PAGE">TITLE OF PAGE

Schema.org

Assuming that the Thing represents your organization/business on each page of your site, you should try to find a more specific type, e.g., TravelAgency as you point out in the comments.
Every Schema.org type is also implicitly a Thing, so every type can use the basic properties from Thing.

The description property is not for the full content, but only for a "short description" (of the TravelAgency in your case, so it would likely always be the same).

In the same way would the value of the image property have to show an image of the TravelAgency, not just any image.

And the url property would be for the URL of the item, i.e., the TravelAgency, so it would always be the same URL (typically the URL of the home page), no matter on which page.

That said, if you want to have an item that represents each page (and not the business all these pages belong to), you probably want to use WebPage (or one of its sub-types) in addition.

Microdata

Syntactically, your Microdata is fine.

HTML5

Your HTML5 is fine, too, apart from a small error: You have a closing </a> tag after the link element. You probably want to either use link (i.e., the link is invisible) or a (i.e., the link is visible).


Comparing different string-matching functions Here is a problem came from codingbat:

  Given 2 strings, a and b, return the number of the positions where
  they contain the same length 2 substring. So "xxcaazz" and "xxbaaz"
  yields 3, since the "xx", "aa", and "az"  substrings appear in the
  same place in both strings.

There are several answers but it may hard to choose which one is the most preferred, such as:

# Solution 1
# Using for loop
def strmatch_forloop(a, b):
    shorter = min(len(a), len(b))
    count = 0
    for i in range(shorter-1):
        a_sub = a[i:i+2]
        b_sub = b[i:i+2]
        if a_sub == b_sub:
            count = count + 1
    return count

# Solution 2
# Using list comprehension
def strmatch_listcomp(a, b):
    shorter = min(len(a), len(b))
    return [a[i:i+2] == b[i:i+2] for i in range(shorter-1)].count(True)

# Solution 3
# Using generator
def strmatch_gen(a, b):
    shorter = min(len(a), len(b))
    return sum(a[i:i+2] == b[i:i+2] for i in range(shorter-1))

Note that the "preferable" might be subjective; it may refer to the speed, the memory use or the coding style. For instance, their speeds are reported as:

%timeit strmatch_forloop
10000000 loops, best of 3: 21.7 ns per loop

%timeit strmatch_listcomp
10000000 loops, best of 3: 22.9 ns per loop

%timeit strmatch_gen
10000000 loops, best of 3: 21.8 ns per loop

According to the results, there may no difference between these approaches. For memory use, similar results can be shown by %memit. However, coding style is too subjective to measure. How could I choose among them?

Since memory and performance metrics are similar, coding style is possibly the only differentiating factor.

Nothing wrong with solution 1 but it takes more time to read it, unless you are paid by the number of lines you write! I guess it is Pythonic to write code in fewer lines so long as readability is not compromised.

I prefer solution 3. It reads well and terse. Solution 2 is just as good but it appears to be doing extra work in the sense of creating a list and then counting. Why do that when you can directly sum up the matches?


Fast API path parameters from Enum Class from enum import Enum
from fastapi import FastAPI
app = FastAPI()

Hi Defined a route and a Enum Class for the same but the Enum Class looks ugly.
class Subjects(str, Enum):
    path1="AcceptedEventRelation"
    path2="Account"
    path3="AccountChangeEvent"
    path4="AccountCleanInfo"
    path5="AccountContactRole"
    ...
    ...
    path100

@app.get("/subjects/{sobjectname:str}")
async def process_subjects(sobjectname:Subjects):
    endpoint_subjects = "/services/data/v49.0/sobjects"
    url_to_request = endpoint_subjects + "/" + sobjectname
    return {
        "subject" : sobjectname,
        "url_to_request": url_to_request
        }

Is there a better way to implement the Subjects Class. Thanks

The fact is that you need the LHS and RHS somewhere in your code, in some form. It can be a list, or dict, or in some other forms.
For example:
from enum import Enum
from fastapi import FastAPI

app = FastAPI()
some_dict = dict(
    path1="AcceptedEventRelation",
    path2="Account",
    path3="AccountChangeEvent",
    path4="AccountCleanInfo",
    path5="AccountContactRole"
)

Subjects = Enum('Subjects', some_dict)

@app.get("/subjects/{sobjectname}")
async def process_subjects(sobjectname: Subjects):
    endpoint_subjects = "/services/data/v49.0/sobjects"
    url_to_request = endpoint_subjects + "/" + sobjectname.value
    return {
        "subject": sobjectname,
        "url_to_request": url_to_request
    }

Here, the some_dict is the variable you need to generate, which also looks ugly if there is a 100 items in it as the way you feel for the Enum class.


Determining if 2 images are the same This is my program code for comparing if 2 images are the same. It basically uses the open dialog and opens 1 image at a time and puts it in 1 picture box.

I heard that GetPixel() method may be slow. Is there a more efficient and faster way to compare 2 if 2 images are the same?

Bitmap image1 = null;
Bitmap image2 = null;

public Form1()
{
    InitializeComponent();
}

private void button1_Click(object sender, EventArgs e) //first image open
{
    OpenFileDialog openDialog = new OpenFileDialog();
    if (openDialog.ShowDialog() == DialogResult.OK)
    {
        image1 = new Bitmap(openDialog.FileName);
        pictureBox1.Image = image1;
    }
}

private void button2_Click(object sender, EventArgs e) //second image open
{

    OpenFileDialog openDialog = new OpenFileDialog();
    if (openDialog.ShowDialog() == DialogResult.OK)
    {
        image2 = new Bitmap(openDialog.FileName);
        pictureBox2.Image = image2;
    }
}

private void button3_Click(object sender, EventArgs e) //compare button
{
    if (compare(image1, image2))
    {
        MessageBox.Show("Same Image.");
    }

    else
    {
        MessageBox.Show("Different Image.");
    }
}

private bool compare(Bitmap bmp1, Bitmap bmp2)
{
    bool equals = true;
    bool flag = true;  //Inner loop isn't broken

    //Test to see if we have the same size of image
    if (bmp1.Size == bmp2.Size)
    {
        for (int x = 0; x < bmp1.Width; ++x)
        {
            for (int y = 0; y < bmp1.Height; ++y)
            {
                if (bmp1.GetPixel(x, y) != bmp2.GetPixel(x, y))
                {
                    equals = false;
                    flag = false;
                    break;
                }
            }
            if (!flag)
            {
                break;
            }
        }
    }
    else
    {
        equals = false;
    }
    return equals;
}

You can use the LockBits method and pointers to access the image data directly.

Example for 24 bpp images:

bool equals = true;
Rectangle rect = new Rectangle(0, 0, bmp1.Width, bmp1.Height);
BitmapData bmpData1 = bmp1.LockBits(rect, ImageLockMode.ReadOnly, bmp1.PixelFormat);
BitmapData bmpData2 = bmp2.LockBits(rect, ImageLockMode.ReadOnly, bmp2.PixelFormat);
unsafe {
  byte* ptr1 = (byte*)bmpData1.Scan0.ToPointer();
  byte* ptr2 = (byte*)bmpData2.Scan0.ToPointer();
  int width = rect.Width * 3; // for 24bpp pixel data
  for (int y = 0; equals && y < rect.Height; y++) {
    for (int x = 0; x < width; x++) {
      if (*ptr1 != *ptr2) {
        equals = false;
        break;
      }
      ptr1++;
      ptr2++;
    }
    ptr1 += bmpData1.Stride - width;
    ptr2 += bmpData2.Stride - width;
  }
}
bmp1.UnlockBits(bmpData1);
bmp2.UnlockBits(bmpData2);


Moving average algorithm that preserves the integral and number of points I need to smooth a noisy signal. I think that the moving average algorithm would be fine. However, I want the following conditions to be satisfied:

The total integral (the sum of all values) before and after the moving average filtering is kept unchanged
The number of points before and after the moving average filtering is kept unchanged

Implementation
The implementation is based on the answer to this post. The main question here is how to process the edges of a signal. The idea is to "reflect" the signal at the edges:
$$a_{-i} = a_{i-1},\, i > 0\quad \text{at the beginning of a signal},$$
$$a_{L+i} = a_{L-i-1},\, i\geq 0\quad \text{at the end of a signal},$$
where L denotes the length of the signal.
Note
To produce more or less appropriate results this algorithm should be used with the signals which start and end with relatively flat waveform (the length of "flatness" depends on the size of the window used).
Algorithm
#include <vector>
#include <iostream>

//Input :
//       in : the original signal
//       w  : the size of the window of the moving average
//Output :
//       out : the filtered signal
void MovingAverage( const std::vector<float>& in, std::vector<float>& out, int w )
{
    //****** PREPARATION ******/
    out.clear();

    //Make the window size odd
    w += (w % 2) ? 0 : 1;

    if( (w < 1)  or (in.size() < w) )
    {
        std::cerr << "Bad input!" << std::endl;
        return;
    }

    //****** ALGORITHM ******/
    int i = 0;  //the current position

    //Firstly, average the head
    for( ; i < w/2; i++ )
    {
        float averagedValue = 0.;
        for( int j = -w/2; j < (w/2 + 1); j++ )
        {
            averagedValue += (i + j >= 0) ? in[i + j] :
                                            in[-(i + j) - 1];//(i + j) is negative here
        }
        out.push_back( averagedValue / w );
    }

    //The middle is averaged normally
    for( ; i + w/2 < in.size(); i++ )
    {
        float averagedValue = 0.;
        for( int j = -w/2; j < (w/2 + 1); j++ )
        {
            averagedValue += in[i + j];
        }
        out.push_back( averagedValue / w );
    }

    //Finally, average the tail
    for( ; i < in.size(); i++ )
    {
        float averagedValue = 0.;
        for( int j = -w/2; j < (w/2 + 1); j++ )
        {
            //(i + j) is not negative here
            averagedValue += (i + j < in.size()) ? in[i + j] :
                                                   in[in.size() - (i + j - in.size()) - 1];//(i + j - in.size()) is not negative here
        }
        out.push_back( averagedValue / w );
    }
}

Test

Where you make the window length odd: w += (w % 2) ? 0 : 1; I would prefer the conditional part to be something like ((w % 2) != 0). Others will no doubt disagree with me, but I personally prefer conditionals to be boolean.
But even better, you can get rid of the conditional entirely: w |= 0x1;. Maybe this makes some kind of not-guaranteed-to-be-fully-portable assumptions? But I'd be comfortable using it.

Using the same index i in the multiple consecutive loops makes me think a little. On the one hand, it's really good how it doesn't duplicate code that calculates where the loops start and end. On the other hand, one doesn't normally expect a loop index, particularly one named i, to be used after the end of a loop. What if some future maintainer doesn't realize this, and inserts some other loop in here that uses i as its index? Possibly you could rename i to something else, but that might make your subsequent calculations harder to follow. In the end, I'd probably at least put a comment by the declaration of i warning that it is re-used through multiple loops.

I don't like something that is purely DSP to write to std::cerr. Either return a status value, or throw an exception when you encounter invalid parameters. I think a status return value is preferable, but an exception may be OK if this really is an exceptional case.

You use float to hold the intermediate averagedValue. In most cases this is fine, but depending on the nature of your data and your window size, it is possible that you'd benefit from using double for the intermediate variable.


Adding/removing map values: unsymmetric implementation I am learning Clojure by implementing a simplistic resource reservation system. At its core are the two functions for placing/canceling reservations for a given resource:

; thread-safe map of {resource -> #{reservations}}
(def known-reservations (atom {}))

(defn place-reservation
  [new-res]
  (swap! known-reservations
         #(merge-with clojure.set/union % {(:resource new-res) #{new-res}})))

(defn cancel-reservation
  [canceled-res]
  (swap! known-reservations
         #(update % (:resource canceled-res) disj canceled-res)))

They seem to work correctly, but the lack of symmetry bothers me. Placing and canceling reservations are symmetric operations, but this is not reflected by the above code: merge-with clojure.set/union doesn't even resemble update ,,, disj.

I tried the following variant:

(defn place-reservation
  [new-res]
  (swap! known-reservations
         #(update % (:resource new-res) conj new-res)))

but since conj nil :x returns a list rather than a set, this leads to duplicate reservations which I want to prevent.

What is the most idiomatic fix to my problem?

PS: if anyone can come up with a more expressive question title, I'll thankfully accept the edit!

edit: here's a simple usage example for experimentation:

(defrecord Resource [name])
(defrecord Reservation [^Resource resource user])

(def car (->Resource "car"))
(def my-res (->Reservation car "me"))
(def your-res (->Reservation car "you"))

(place-reservation my-res)
(place-reservation your-res)
(place-reservation my-res) ;; duplicate reservation must not change anything

(cancel-reservation my-res)

I agree that the lack of symmetry in your implementation of place-reservation using merge-with and union is bothersome. Fortunately, there's a function to fix that!

fnil

The fnil function allows you to create a function that acts like an existing function, but replaces the first argument with something else in the case that it's given nil:

(conj nil :x)
;=> (:x)

((fnil conj #{}) nil :x)
;=> #{:x}

So you can write place-reservation like this:

(defn place-reservation [new-res]
  (swap! known-reservations
         #(update % (:resource new-res) (fnil conj #{}) new-res)))

swap!

As you know, swap! will pass the current state of the atom as the first argument to the function you provide, but you can give values for the other arguments as well. So instead of passing a function that calls update with the current state and some other arguments, you can just pass update and those other arguments directly, and swap! will do the rest:

(defn place-reservation [new-res]
  (swap! known-reservations update (:resource new-res) (fnil conj #{}) new-res))

(defn cancel-reservation [canceled-res]
  (swap! known-reservations update (:resource canceled-res) disj canceled-res))

Global state

Currently, you store known-reservations in a global var. This is a Bad Thing. Even if you don't plan to have multiple atoms of reservation lists in your actual program, it makes your code more painful to test:

(require '[clojure.test :as t])

(t/deftest reservation-test
  (reset! known-reservations {})
  (place-reservation my-res)
  (place-reservation your-res)
  (place-reservation my-res)
  (t/is (= {car #{my-res your-res}} @known-reservations))
  (reset! known-reservations {}))

I suppose you could make this a bit more concise using fixtures, but either way, if you eventually end up with enough tests that you'd like to run them in parallel, you'll be out of luck.

The easiest way to fix this problem is to just take the state as a parameter instead:

(defn place-reservation [known-reservations new-res]
  (swap! known-reservations update (:resource new-res) (fnil conj #{}) new-res))

(defn cancel-reservation [known-reservations canceled-res]
  (swap! known-reservations update (:resource canceled-res) disj canceled-res))

This still isn't great, but it's an improvement:

(t/deftest reservation-test
  (t/is (= {car #{my-res your-res}}
           @(doto (atom {})
              (place-reservation my-res)
              (place-reservation your-res)
              (place-reservation my-res)))))

Pure functions

In place-reservation and cancel-reservation, you complect two things:

The logic of placing or canceling reservations
The fact that you're storing reservations in an atom

You'd be better off writing place-reservation and cancel-reservation as pure functions that take a reservation collection and return a new reservation collection:

(defn place-reservation [known-reservations new-res]
  (update known-reservations (:resource new-res) (fnil conj #{}) new-res))

(defn cancel-reservation [known-reservations canceled-res]
  (update known-reservations (:resource canceled-res) disj canceled-res))

Pure functions are the easiest things in the world to test:

(t/deftest reservation-test
  (t/is (= {car #{my-res your-res}}
           (reduce place-reservation {} [my-res your-res my-res]))))


JavaScript infinite scroll with intersection observer I built a simple infinite scroll using the intersection observer API, would love some feedback on ways to improve my JavaScript and make it more maintainable and reusable, and feedback according to best JavaScript practices.
<!DOCTYPE html>

    <meta charset="UTF-8" />
    <title>Infinite Scroll</title>

Loading more content...

I think that container and images are being repeated unecessarly.

When you call imgGenerator in appendImages, you already know that you want an array of images.
You can use Array.from:

the first argument will be an array-like object, that is, an object that is accessed by index and with a length propertie(which is identified as images on our javascript)
the second argument will be the mapFunction(which will
be imgGenerator), a function that will be called for every
item of the array

Doing this, on imgGenerator you avoid to make an instance of an array(imgs), a push into it and a for loop every time you want this array, although, you will not need to pass images as parameter again

the container is always imgsContainer, so, in the appendImages you already know where to append

function imgGenerator() {
  const img = document.createElement("img");
  img.src = `http://api.adorable.io/avatars/${randomInteger()}`;
  img.classList.add('infinite-scroll-img');
  return img;
}

function appendImages(images) {
  imgsContainer.append(
    ...Array.from({ length: images }, imgGenerator)
  );
}

function handleIntersect(entries) {
  entries.forEach(entry => {
    if (entry.isIntersecting) {
      appendImages(5);
    }
  });
}

appendImages(5);


Transaction handling for multiple SQL statements In this code I update two tables called Payment and SalaryTrans. First I insert records (Salary payments) to Payment table and then I update SalaryTrans table. When a record is inserted to Payment table, ID is auto generated (auto increment column) and then that ID should be updated in SalaryTrans table (for all matching records).

The following code compiles and runs giving the desired output, but I just want to know whether the transactions are handled properly.  Also, are there any other flows in this code?

Could you please review and give your feedback?

public int InsertPaymentDetails(List<PaymentInfo> list, int totalRecords, decimal totalAmount )
{
    const string selectSatement = @"INSERT INTO Payment (payment_type, reference, payment_date, total_records, total_amount)
VALUES(@type, @reference, @date, @totalRecords, @totalAmount ) ";

    const string updateStatement = @"UPDATE SalaryTrans SET payment_id = (SELECT TOP 1 id FROM Payment ORDER BY Payment.id DESC)
WHERE SalaryTrans.employee_id = @employeeID AND SalaryTrans.reference = @reference ";

    //An item is refered in the list just to get the PaymentType, PaymentDate etc. as these information are common for all items in the list
    var first = 0;
    var payInfo = list[first];

    //To get the affected rows following variables are declared
    int result= 0;
    int affectedRecords = 0;

    using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
    {
        sqlConnection.Open();
        using (SqlTransaction sqlTrans = sqlConnection.BeginTransaction ())
        using (SqlCommand sqlCommand = new SqlCommand(selectSatement, sqlConnection,sqlTrans))
        {
            sqlCommand.Parameters.Add("@type", SqlDbType.VarChar).Value = payInfo.PaymentType;
            sqlCommand.Parameters.Add("@date", SqlDbType.Date).Value = payInfo.PaymentDate;
            sqlCommand.Parameters.Add("@reference", SqlDbType.VarChar).Value = payInfo.Reference;
            sqlCommand.Parameters.Add("@totalRecords", SqlDbType.Int).Value = totalRecords;
            sqlCommand.Parameters.Add("@totalAmount", SqlDbType.Decimal).Value = totalAmount;

            sqlCommand.ExecuteNonQuery();

            SqlParameter paramEmployeeID = new SqlParameter("@employeeID", SqlDbType.Int);
            sqlCommand.Parameters.Add(paramEmployeeID);

            foreach (PaymentInfo p in list)
            {
                paramEmployeeID.Value = p.EmployeeID;
                sqlCommand.CommandText = updateStatement; //Command text changed to update second table
                result = sqlCommand.ExecuteNonQuery();
                //If at least one recored was inserted then the recordsAffected should be +1
                if (result == +1)
                    affectedRecords = 1;
            }
            sqlTrans.Commit();
            return affectedRecords;
        }
    }
}

Naming

Constants in .NET should be named using PascalCase. See c-sharp-naming-convention-for-constants
Your const string selectSatement = @"INSERT INTO Payment..." should be better named InsertStatement to also reflect that it is an INSERT and not a SELECT.

var first = 0;
var payInfo = list[first];

As the value of var first won't be changed, you should change it to a const with a meaningful name.

Refactoring
Before using the first item in the list, a check to see if the list is null or empty should take place.

const int NoRowsAffected = 0;
if (list == null || !list.Any()) { return NoRowsAffected; }

int result= 0;

won't be used, except for assigning the affetced rows of the update command. we should just remove it.

if (result == +1)
    affectedRecords = 1;

Using curly brackets {} should be a have to everytime, at least if written on separate lines.

sqlCommand.CommandText = updateStatement;

should be placed outside of the for loop. Ther is no need to assign the const to the CommandText property for each iteration.

Result

public int InsertPaymentDetails(List<PaymentInfo> paymentInfos, int totalRecords, decimal totalAmount)
{
    const string InsertStatement = @"
        INSERT INTO Payment (payment_type, reference, payment_date, total_records, total_amount)
        VALUES(@type, @reference, @date, @totalRecords, @totalAmount )
        ";

    const string UpdateStatement = @"
        UPDATE SalaryTrans
        SET payment_id = (SELECT TOP 1 id FROM Payment ORDER BY Payment.id DESC)
        WHERE SalaryTrans.employee_id = @employeeID
        AND SalaryTrans.reference = @reference
        ";

    const int NoRowsAffected = 0;
    const int FirstListIndex = 0;

    if (paymentInfos == null || !paymentInfos.Any()) { return NoRowsAffected; }

    var payInfo = paymentInfos[FirstListIndex];

    //To get the affected rows following variables are declared

    int affectedRecords = NoRowsAffected;

    using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
    {
        sqlConnection.Open();
        using (SqlTransaction sqlTrans = sqlConnection.BeginTransaction())
        using (SqlCommand sqlCommand = new SqlCommand(InsertStatement, sqlConnection, sqlTrans))
        {
            sqlCommand.Parameters.Add("@type", SqlDbType.VarChar).Value = payInfo.PaymentType;
            sqlCommand.Parameters.Add("@date", SqlDbType.Date).Value = payInfo.PaymentDate;
            sqlCommand.Parameters.Add("@reference", SqlDbType.VarChar).Value = payInfo.Reference;
            sqlCommand.Parameters.Add("@totalRecords", SqlDbType.Int).Value = totalRecords;
            sqlCommand.Parameters.Add("@totalAmount", SqlDbType.Decimal).Value = totalAmount;

            sqlCommand.ExecuteNonQuery();

            SqlParameter paramEmployeeID = new SqlParameter("@employeeID", SqlDbType.Int);
            sqlCommand.Parameters.Add(paramEmployeeID);
            sqlCommand.CommandText = UpdateStatement;

            foreach (PaymentInfo paymentInfo in paymentInfos)
            {
                paramEmployeeID.Value = paymentInfo.EmployeeID;
                //If at least one recored was inserted then the recordsAffected should be +1
                if (sqlCommand.ExecuteNonQuery() == 1) { affectedRecords = 1; }
            }
            sqlTrans.Commit();
        }
    }
    return affectedRecords;
}


Concurrent multi-server pinging in Java I have an application that essentially "pings" all of the servers on my network.  I have about 100 servers, and this ping will happen every 10 seconds.

public class HealthChecker {
 private static List<InetSocketAddress> servers = new     ArrayList<InetSocketAddress>();
 public static void main(String[] args){
  // not shown: populating servers list
  new Thread(){
   public void run(){
    while (true){
     try {
      Thread.sleep(10*1000);
      for (final InetSocketAddress server : servers){
       new Thread(){
        public void run(){
         Socket connection = new Socket();
         try {
          connection.connect(server, 5*1000);
         } catch (IOException e) {
          servers.remove(server);
         }
        }
       }.start();
      }
     } catch (InterruptedException e) {
      e.printStackTrace();
     }
    }
   }
  }.start();
 }

So, do you think this code can be improved? I really feel like opening a new thread every time I make a connection isn't a good thing.

EDIT:
So looking over all the proposed solutions, they are all very good, and I will over the next week try and all and see which one appears to be the most efficient.

Here's an example adapted to your case which uses ThreadPoolExecutor.

This example allows a configurable throughput while establishing TCP connections to servers.

Checking Future.get() for each submitted worker thread ensures that no servers are checked more than others.

What happens here:

Target domains are added to a list
If the domain is an unknown host (DNS unreachable) than it's skipped.
Resolvable domains are submitted as 1 domain per 1 thread into the pool.
If the pool gets overflown by PingWork threads, that submission is delayed for 3 seconds until the pool is ready to accept.
Each TCP port 80 connection has a timeout of 5 seconds.
When there are no exceptions thrown by a TCP connection to a certain host, it is assumed reachable and not reported.
When all PingWork threads finish, main thread waits for 10 seconds until next round.

Here's the source

// package
// imports

public class Main {
    private static List<InetSocketAddress> servers =
            new ArrayList<InetSocketAddress>();

    public static void main(String[] args)
            throws InterruptedException, ExecutionException {
        String[] domains = new String[]{
                "3Com.com",
                // etc. etc.
                "Kai.com"
        };

        for (String hostname : domains) {
            try {
                servers.add(
                        new InetSocketAddress(
                                InetAddress.getByName(hostname),
                                80
                        )
                );
            } catch (UnknownHostException e) {
                System.out.println("Unknown host: " + hostname);
            }
        }

        System.out.println(
                "Total number of target servers: " + servers.size()
        );

        BlockingQueue<Runnable> work =
                new ArrayBlockingQueue<Runnable>(5);

        ThreadPoolExecutor pool = new ThreadPoolExecutor(
                5,                      // corePoolSize
                10,                     // maximumPoolSize
                5000,                   // keepAliveTime
                TimeUnit.MILLISECONDS,  // TimeUnit
                work                    // workQueue
        );

        pool.prestartAllCoreThreads();

        pool.setRejectedExecutionHandler(
                new RejectedExecutionHandler() {
                    @Override
                    public void rejectedExecution(
                            Runnable r,
                            ThreadPoolExecutor executor
                    ) {
                        System.out.println("Work queue is currently full");
                        try {
                            Thread.sleep(3000);
                        } catch (InterruptedException ignore) {

                        }
                        executor.submit(r);
                    }
                }
        );

        Collection<Future<?>> futures = new LinkedList<Future<?>>();

        while (true) {
            for (InetSocketAddress server : servers) {
                futures.add(pool.submit(new PingWork(server)));
            }
            for (Future<?> future : futures) {
                future.get();
            }
            System.out.println(
                    "All servers checked. Will wait for 10 seconds until next round"
            );
            Thread.sleep(10000);
        }

    }

    private static class PingWork implements Runnable {
        private static final int TIMEOUT = 5000;
        private InetSocketAddress target;

        private PingWork(InetSocketAddress target) {
            this.target = target;
        }

        @Override
        public void run() {
            Socket connection = new Socket();
            boolean reachable;

            try {
                try {
                    connection.connect(target, TIMEOUT);
                } finally {
                    connection.close();
                }
                reachable = true;
            } catch (Exception e) {
                reachable = false;
            }

            if (!reachable) {
                System.out.println(
                        String.format(
                                "%s:%d was UNREACHABLE",
                                target.getAddress(),
                                target.getPort()
                        )
                );
            }
        }
    }
}

Here's the output of 30 domain sample

    Total number of target servers: 30
    Work queue is currently full
    Work queue is currently full
    Amdahl.com/129.212.11.21:80 was UNREACHABLE
    All servers checked. Will wait for 10 seconds until next round
    Work queue is currently full
    Work queue is currently full
    Amdahl.com/129.212.11.21:80 was UNREACHABLE
    All servers checked. Will wait for 10 seconds until next round

You can increase the maximumPoolSize of ThreadPoolExecutor in order to get all domains into queue in just one iteration. However I left that part for you.


Credit card validation - Python 3.4 Just completed a piece of code - a credit card validation program in python - as a little side project. Having no experience with classes in the past, I decided to employ classes in this project.

I am wondering if you could review my code, both appraising the actual code, but also evaluate my use of OOP/classes.

The requirements for the program are:

  The user enters their name, postcode, the card code, and the card
  date.

  The eighth digit of the card code is removed and acts as a check digit

  The code is then reversed

  The 1st, 3rd, 5th, and 7th digits are multiplied by 2

  If the result of the multiplication is > 9, subtract 9 from it.

  If the sum of the 7 digits, and the check digit are divisable by 10,
  the code is valid

  The card date must also be in the future.

  Finally, output their name, postcode, card number, and whether it is
  valid or not.

The code:

"""Program used to check if a credit card is authentic."""
# !/usr/bin/env python
# -*- coding: utf-8 -*-

# Check it out
import datetime

class Customer:
    """Class representing the customer and their credit card details"""

    # Constructor
    def __init__(self):
        self.name = input("Name: ")
        self.postcode = input("Postcode: ")
        self.card_date = input("Card date: ")
        self.card_code = input("Card code: ").strip()

    def check_date(self):
        """Checks current date against the credit card's date. If it is valid, returns True; else False."""
        card = datetime.datetime.strptime(self.card_date, "%d/%m/%Y").date()
        if datetime.date.today() < card:
            return True
        else:
            return False

    def check_code(self):
        """Contains the algorithm to check if the card code is authentic"""

        code_list = list(str(self.card_code))
        check_digit = int(code_list[7])
        code_list.pop()
        # The last digit is assigned to be a check digit and is removed from the list.
        code_list.reverse()

        for item in code_list:
            temp_location = code_list.index(item)
            if is_even(temp_location):
                code_list[temp_location] = int(item) * 2
        # Loops over each digit, if it is even, multiplies the digit by 2.

        for item in code_list:
            temp_location = code_list.index(item)
            if int(item) > 9:
                code_list[temp_location] = int(item) - 9
        # For each digit, if it is greater than 9; 9 is subtracted from it.

        sum_list = 0
        for item in code_list:
            sum_list += int(item)
        # Calculates the sum of the digits

        code_total = sum_list + int(check_digit)

        if code_total % 10 == 0:
            return True
        else:
            return False
            # If the code is divisible by 10, returns True, else, it returns False.

    def check_auth(self):
        """Checks the card's authenticity. """
        if self.check_code() and self.check_date():
            print("----------------------")
            print("Valid")
            print(self.name)
            print(self.postcode)
            print(self.card_date)
            print(self.card_code)
        else:
            print("----------------------")
            print("Invalid")
            print(self.name)
            print(self.postcode)

def is_even(number):
    """Function used to test if a number is even."""
    if number % 2 == 0:
        return True
    else:
        return False

if __name__ == "__main__":
    customer().check_auth()

I think there is this code organization issue - you have a class named Customer, but it, aside from the .name attribute, consists of credit-card related logic only.

I would also pass the obtained attributes to the class constructor instead of asking for them inside it:

def __init__(self, name, post_code, card_date, card_code):
    self.name = name
    self.post_code = post_code
    self.card_date = card_date
    self.card_code = card_code

It is a little bit cleaner to do this way since now our class is more generic, it is agnostic of where the attributes are coming from.

Some other code-style related notes:

consistent naming: rename postcode to post_code
revise the quality and necessity of comments: there is probably not much sense in having a comment # Constructor
you can simplify the way you return a boolean result from your methods. For instance, you can replace:

if datetime.date.today() < card:
    return True
else:
    return False

with:

return datetime.date.today() < card

And, it's worth mentioning that, generally speaking, if you doing this for production, you should not be reinventing the wheel and switch to a more mature, well-used and tested package like pycard.


Script to group files by extension and unzip archives I am writing a shell script to sort my files by extension and unzip archives. It works in simple cases, but I haven't tested it extensively. How can I make it more robust against the set of file names that may be present?

#!/bin/bash
#File sorter
SECONDS=0
SAVEIFS=$IFS
IFS=$(echo -en "\n\b")
echo "trigger took place in $(pwd)"
for i in $( ls -1); do
  echo "processing $i ..."
  fileext=${i##*.}
  case ${i##*.} in
    zip)
      mkdir archive
      mkdir $(pwd)/archive/${i%%.*}
      unzip $i -d $(pwd)/archive/${i%%.*}
      ;;
    *) echo "nonzip"
      mkdir ${i##*.}
      mv $i $(pwd)/${i##*.}
      ;;
  esac
done
echo "Done in $SECONDS seconds!"

I don't see where awk fits in this. I'm going to point out a few problems in your script, so you'll at least get something useful out of this. But please do tell us what you're actually trying to do.

IFS=$(echo -en "\n\b")

There's a simpler way of writing that in bash: IFS=$'\n\b'. I don't understand why you're setting $IFS to include a backspace character; and whatever need you may have found to set IFS is probably caused by other problems that I'll point out below.

echo "trigger took place in $(pwd)"

There's a variable $PWD that contains the current working directory. $PWD is equivalent to $pwd and a little faster.

for i in $( ls -1); do

Do not parse the output of ls. ls is a tool for displaying the attributes of files. The shell has a perfectly good construct for listing files in a directory: wildcards. Make this for i in *; do … This way has the advantage that it can cope with arbitrary file names; parsing the output of ls will always choke on some “strange” file names.

fileext=${i##*.}

You should use this variable below.

mkdir archive

If you run this part multiple times, the directory will already exist. Test for the directory's existence first, or call mkdir -p archive.

mkdir $(pwd)/archive/${i%%.*}

Again, $PWD. Except that there's no point in using it, a relative path would do. Also, you need double quotes around all variable and command substitutions; otherwise the shell splits the result of the substitution at $IFS characters, then interprets the pieces as glob patterns (file name wildcards) and expands them. Always put double quotes around variable and command substitutions unless you know why you need to leave them off.

Another point: ${i%%.*} strips everything after the first . in the file name. This is not compatible with the definition of $fileext. To get the file except for its last extension, use ${%.*} (truncate at the last .).

$(unzip $i -d $(pwd)/archive/${i%%.*})

I don't know what you're trying to do here, interpreting the output of unzip as a shell snippet. Again, double quotes around variable substitutions.

  mkdir ${i##*.}

Again, double quotes. Also, if the file name sans prefix happens to begin with a -, the mkdir command will interpret its argument as an option. To avoid this, use -- to indicate the end of the option list (this is a convention that most utilities respect): mkdir -- "${i##*.}"

Also, there's a case you don't handle: what if the file name contains no .? In that case, mkdir will attempt to overwrite the file (and fail). One solution is to create the directory with a temporary name, move the file, then rename it.

Here's the fixed code:

#!/bin/bash
#File sorter
set -e    # Abort in case of error
SECONDS=0
SAVEIFS=$IFS
echo "trigger took place in $PWD"
for i in *; do
  echo "processing $i ..."
  case $i in
    *.zip)
      [ -d archive ] || mkdir archive
      mkdir "archive/${i%.*}"
      unzip -d "archive/${i%.*}" "./$i";;
    *.*)
      mkdir -p "${i##*.}"
      mv -- "$i" "${i##*.}/";;
    *)
      tmp=$(TMPDIR=. mktemp -d)
      mv -- "$i" "$tmp/"
      mv -- "$tmp" "$i";;
  esac
done
echo "Done in $SECONDS seconds!"


Pythonic way of managing data for plotting 2D variance plots I have collected large dataset of trajectories in 3D space. Below is the information about dataset-

Each trajectory (shape: n x 4) is saved into CSV file with the following header: time, p_x, p_y, p_z
The filenames are defined in following way: subject_i_trail_i.csv, where i starts from 1 to 10.

Below are first 10 lines from subject_1_trail_1.csv file-

time    p_x     p_y     p_z
0       0.4333  0.1107  0.1259
0.0103  0.4336  0.1106  0.126
0.02    0.4334  0.1108  0.1259
0.03    0.4333  0.1106  0.1259
0.04    0.4334  0.1107  0.1259
0.0501  0.4328  0.1103  0.126
0.06    0.4331  0.1107  0.1255
0.0703  0.4331  0.1103  0.126
0.08    0.4324  0.1102  0.126

For each subject, I want to plot the trajectory showing median and variance as shown below-

fill_between(x, perc_25, perc_75)

I am using NumPy but the code is relatively long. Below is the code snippet-

from pylab import *
import sys

# make a 3d numpy array (chop off the extra rows)
def load(location, subject, suffix):
    all_data = []
    min_rows = sys.maxint
    for i in range(1, 11):
        file_name = location + subject + '_trail_' + str(i) + '.csv'
        data = np.loadtxt(file_name, delimiter=',', skiprows=1)
        if data.shape[0] < min_rows:
            min_rows = data.shape[0]
        all_data.append(data)

    all_crop_data = []
    for data in all_data:
        all_crop_data.append(data[:min_rows,:])

    return np.array(all_crop_data)

def proc(data):
    t = data[:, :, 0] # first column is time
    x = data[:, :, 1] # second column is x
    y = data[:, :, 2] # third column is y
    z = data[:, :, 3] # fourth column is z

    t_median  = np.zeros(data.shape[1])
    t_perc_25 = np.zeros(data.shape[1])
    t_perc_75 = np.zeros(data.shape[1])

    x_median  = np.zeros(data.shape[1])
    x_perc_25 = np.zeros(data.shape[1])
    x_perc_75 = np.zeros(data.shape[1])

    y_median  = np.zeros(data.shape[1])
    y_perc_25 = np.zeros(data.shape[1])
    y_perc_75 = np.zeros(data.shape[1])

    z_median  = np.zeros(data.shape[1])
    z_perc_25 = np.zeros(data.shape[1])
    z_perc_75 = np.zeros(data.shape[1])

    for i in range(data.shape[1]): # for each timestamp
        t_median[i]  = np.median(t[:, i])
        t_perc_25[i] = np.percentile(t[:, i], 25)
        t_perc_75[i] = np.percentile(t[:, i], 75)

        x_median[i]  = np.median(x[:, i])
        x_perc_25[i] = np.percentile(x[:, i], 25)
        x_perc_75[i] = np.percentile(x[:, i], 75)

        y_median[i]  = np.median(y[:, i])
        y_perc_25[i] = np.percentile(y[:, i], 25)
        y_perc_75[i] = np.percentile(y[:, i], 75)

        z_median[i]  = np.median(z[:, i])
        z_perc_25[i] = np.percentile(z[:, i], 25)
        z_perc_75[i] = np.percentile(z[:, i], 75)

    all_median  = np.vstack((t_median, x_median, y_median, z_median)).T
    all_perc_25 = np.vstack((t_perc_25, x_perc_25, y_perc_25, z_perc_25)).T
    all_perc_75 = np.vstack((t_perc_75, x_perc_75, y_perc_75, z_perc_75)).T

    return all_median, all_perc_25, all_perc_75

s1 = load('/Desktop/', 'subject_1') # subject 1
s2 = load('/Desktop/', 'subject_2') # subject 2

x = np.arange(0, s1.shape[1])

s1_med, s1_perc_25, s1_perc_75 = proc(s1)
s2_med, s2_perc_25, s2_perc_75 = proc(s2)

# lets plot only x (second column)
index = 1
s1_med     = s1_med[:, index]
s1_perc_25 = s1_perc_25[:, index]
s1_perc_75 = s1_perc_75[:, index]

s2_med     = s2_med[:, index]
s2_perc_25 = s2_perc_25[:, index]
s2_perc_75 = s2_perc_75[:, index]

fill_between(x, s1_perc_25, s1_perc_75, alpha=0.25, linewidth=0, color='#B22400')
fill_between(x, s2_perc_25, s2_perc_75, alpha=0.25, linewidth=0, color='#006BB2')

plot(x, s1_med, linewidth=2, color='#B22400')
plot(x, s2_med, linewidth=2, color='#006BB2')

I am looking for a better, i.e., pythonic way to achieve the same. I am not sure if Pandas can be useful here. Looking for your suggestions, please.

Your function proc can be greatly reduced by using the fact that numpy functions are vectorized and most of them take the axis to act upon as an argument. This is certainly true for numpy.median and numpy.percentile.

import numpy as np

def proc(data):
    return (np.median(data, axis=0),
            np.percentile(data, 25, axis=0),
            np.percentile(data, 75, axis=0))

You can also use a for loop for plotting, to avoid duplicating everything:

if __name__ == "__main__":
    colors = '#B22400', '#006BB2'
    file_names = 'subject_1', 'subject_2'

    for file_name, color in zip(file_names, colors):
        subject = load('/Desktop/', file_name)
        med, perc_25, perc_75 = proc(subject)

        # lets plot only x (second column)
        med = med[:, 1]
        perc_25 = perc_25[:, 1]
        perc_75 = perc_75[:, 1]

        fill_between(x, perc_25, perc_75, alpha=0.25, linewidth=0, color=color)
        plot(x, med, linewidth=2, color=color)

Note that x is currently undefined (as in your code).

I also added a if __name__ == "__main__": guard to allow importing from this script without executing the plotting.

In addition I changed your indentation to conform to Python's official style-guide, PEP8:

This

med     = med[:, 1]
perc_25 = perc_25[:, 1]
perc_75 = perc_75[:, 1]

should just be

med = med[:, 1]
perc_25 = perc_25[:, 1]
perc_75 = perc_75[:, 1]

as noted under Pet Peeves (of Guido van Rossum, the creator and BDFL of Python).


Creating multiple new child forms from a single form I have a simple application reading IO and plotting chart on UI. There is one instance that I need to plot three graphs at a time, and to make this easier, I made a graph form where I declared all non-fixed chart settings during form load and load it to chart as settings when the graph form is called as a new child form on the main/parent form.

Here is my code to call out graph form on parent form:

    System.Windows.Forms.Integration.WindowsFormsHost DemoTabContentMiniGraph1 = new System.Windows.Forms.Integration.WindowsFormsHost();
    System.Windows.Forms.Integration.WindowsFormsHost DemoTabContentMiniGraph2 = new System.Windows.Forms.Integration.WindowsFormsHost();
    System.Windows.Forms.Integration.WindowsFormsHost DemoTabContentMiniGraph3 = new System.Windows.Forms.Integration.WindowsFormsHost();

    // Create the window control.
    F_CWndw_Graph FormMiniGraph1 = new F_CWndw_Graph(true, false, false, false, 0.5, 100, 0.5, 10, "Time (s)", 20, 100, 100, "Flow Rate (l/s)", 2, true, "DEMO");
    F_CWndw_Graph FormMiniGraph2 = new F_CWndw_Graph(false, true, true, false, 0.5, 10, 0.5, 10, "Time (s)", 50, 250, 250, "Height (mm)", 3);
    F_CWndw_Graph FormMiniGraph3 = new F_CWndw_Graph(false, false, false, true, 0.5, 10, 0.5, 10, "Time (s)", 400, 2000, 2000, "Height (mm)", 4);

   // Assign the grid control as the host control's child.
   DemoTabContentMiniGraph1.Child = FormMiniGraph1;
   DemoTabContentMiniGraph2.Child = FormMiniGraph2;
   DemoTabContentMiniGraph3.Child = FormMiniGraph3;

   // Add the interop host control to the Grid
   Grid_miniGraph1.Children.Add(DemoTabContentMiniGraph1);
   Grid_miniGraph2.Children.Add(DemoTabContentMiniGraph2);
   Grid_miniGraph3.Children.Add(DemoTabContentMiniGraph3);

Will this be a good and safe way with UI programming, knowing that each of these graphs will run a timer ticking at 500ms to do the plotting of data?

And will those private variables inside the new child forms I created out of one layout form be totally separate/independent from each other?

Your code is very small for a good review, but a few things can be mentioned:

1) naming - it is better to name your variables by what they represent. E.g. DemoTabContentMiniGraph1, 2, 3 should can be named something like MiniGraphFS1View, MiniGraphPs1Ps2View etc.

Also, F_CWndw_GraphFS1 can be adjusted to Pascal/Camel notation used in C# (they look to have C++ naming style)

2) passing parameters to instances - providing many parameters to a constructor works, but it may render maintainability problems in the future:

it is very hard to know what the third false in F_CWndw_GraphFS1(true, false, false, false, 0.5, 100, 0.5, 10, "Time (s)", 20, 100, 100, "Flow Rate (l/s)", 2, true, "DEMO"); means
if a new parameter must be added to the graphic, you may have to change in multiple places (constructor prototype, callers have to provide the new parameter, if it does not have a default value).

A better way is to define a class named GraphOptions or similar to encapsulate your options:

class GraphOptions
{
   float MinValue { get; set; }
   float MaxValue { get; set; }
   String XLabel { get; set; }
   // other properties come here

   // you can also define a constructor to assign default values
}

Your construction code will look something like this (much longer, but much easier to understand):

var FormMiniGraph1 = new F_CWndw_GraphFS1(new GraphOptions()
   {
      Prop1 = true,
      Prop2 = false,
      Prop3 = false,
      MinValue = 0.5,
      MaxValue = 100,
      XLabel = "Time (s)"

      // all other properties remain to their default value or their value does not matter for current graph
   }

3) (possible) AddRange optimization:

If Children implements IEnumerable, AddRange can be used

Grid_miniGraph1.Children.Add(DemoTabContentMiniGraph1);
Grid_miniGraph2.Children.Add(DemoTabContentMiniGraph2);
Grid_miniGraph3.Children.Add(DemoTabContentMiniGraph3);

becomes

Grid_miniGraph1.Children.AddRange(new List<WindowsFormsHost>() { DemoTabContentMiniGraph1, DemoTabContentMiniGraph2, DemoTabContentMiniGraph3 };

Other aspect is the harcoded number of possible graphs. Are they so different that they must be defined one by one? Do they share common functionality?


Haskell searching through a large textfile efficiently I have been given a list xs and a function f, and I want to know stuff like how many x <- xs satisfy (f x) `elem` xs or which ones do, etc. However, the problem is that the list xs is available only as an external file.

import Data.Char
import System.IO.Strict
import qualified Data.Set as Set

vowel x = elem x "aeiou"

toPigLatin :: String -> String
toPigLatin word
    | vowel (head word) = word ++ "ay"
    | not $ null [x | x <- word, vowel x] = let (v, c) = span (not.vowel) word in c ++ v ++ "ay"
    | otherwise = word

wordList = lines <$>  System.IO.Strict.readFile "toyWordSet"
wordSet = Set.fromList <$> wordList

good :: String -> IO Bool
good word = (Set.member (toPigLatin word)) <$> wordSet

mapMBool :: (a -> IO b) -> [a] -> IO [b]
mapMBool f l = do
                case l of
                    [] -> return []
                    (x:xs) -> do
                            b1 <- f x
                            bs <- mapMBool f xs
                            return (b1:bs)

Ultimately, I want to compute wordList >>= mapMBool good, which is a value of type IO [Bool] which have True at the positions where the desired property is satisfied.

Now, I have three main concerns:

When I run my code with a toyWordSet, it runs fine but I ultimately intend to replace it with /usr/share/dict/words, on which my PC freezes. How do I make my program more efficient?
Writing a function like mapMBool is not the most elegant idea ever. How can I do this more elegantly?
For some reason, I get some strange exceptions, when I try to use the lazy readFile instead of the strict version. What is happening? Why must I use strictness?

Other general comments on the coding style or practice is also welcome.

I think you've missed the forest for the trees. While the origin of your list might be IO, there's no need to carry values around inside the IO monad just because that's where you found them.

Write your functions as though the external world doesn't exist. E.g.—

-- Was `good`
pigExists :: String -> Set String -> Bool
pigExists word set = Set.member (toPigLatin word) set

Write your actions to set up the values you need to use your IO-ignorant functions. E.g.—

main :: IO ()
main = do
    allWords <- words <$> readFile "wherever" -- equivalent to `allWords <- wordList`
    let pigHits = map (\word -> pigExists word (Set.fromList allWords)) allWords
    print $ length $ filter id pigHits -- Or some other IO action that does what you need

When in doubt, parameterize. You tripped yourself up by defining wordSet directly on wordList instead of just accepting any [String] value you're passed.


Serialising missing values in trie There are two aspects to this question that are inter-related.

Change I've made to the trie module in python (PyPI, GitHub) to make it serialisable (and deserialisable).

The change I made is visible on GitHub. I added two functions to the Node class used in serialisation.

def __getstate__(self):
    return (self.parent, self.key, self.nodes, self.value)

def __setstate__(self, state):
    (self.parent, self.key, self.nodes, self.value) = state
    if type(self.value) == object:
        self.value = Node.no_value

The Node class uses an instance of object to signify that there is no value for that Node. If you serialise and deserialise a Trie of Nodes then the values are set to different instances of object, this is why I added a check for the type being object and replacing it with Node.no_value.

Is this a reasonable way of doing things? Is anyone likely to want to store an actual instance of object?

Is there a better way of signifying that a Node has no value, that is more robust to serialisation?

Use None rather then Node.no_value.

None is the typical value to use for the absence of value. However, its possible that you actually want to store None in your trie, so that doesn't work.

Checking the type of the object is not really a good plan because somebody could decide to store plain objects in your trie. I would add another value, a boolean "self.value is Node.no_value", to the tuple in setstate. By checking that I could be sure that it works correctly.


Sort array of Numbers with some values always first The first number will be dynamically selected and remaining array should be sorted ascending

    int[] array = {1,1,0,1,2,2,0,0};
    int  firstNumber = 0;// dynamic can be 0 or 1 or 2
    int numberOfOccurances = 0;

    //Basic sort functionality
    for(int i = 0 ; i< array.length; ++i)
    {
        if(array[i] == firstNumber)
        {
            numberOfOccurances++;
        }
        for(int j = i+1; j<array.length; ++j)
        {
            if(array[j] < array[i])
            {
                int temp = array[i];
                array[i] = array[j];
                array[j] = temp;
            }
        }
    }
    int[] requiredArray= new int[array.length];
    for(int i = array.length-1 ; i >= 0; i--)
    {
        if(array[i] != firstNumber)
        requiredArray[i] = array[i];

    }
    for(int i =0;i<array.length;i++)
    {
        if(i<numberOfOccurances)
        requiredArray[i]= firstNumber;
    }

    //Print Output
    for (int i = 0; i<requiredArray.length; i++)
    System.out.print(requiredArray[i] + "  ");

Output:

1 1 1 1 0 0 2 2

I was able to get desired output. But not sure if this is the best way to solve in terms of speed.

This can be easily done using a two-phase approach, one to shift values to the front that you want to keep, the other to sort the remainder.

Note, for sorting, use the native functions in the libraries... in this case, Arrays.sort(...)

It would also be much better to wrap this up in a function, taking 2 arguments, the number to put first, and the array to sort.

public static void sortFirst(int first, int[] array) {
    int left = 0;
    for (int right = 0; right < array.length; right++) {
        if (array[right] == first) {
            // swap left and right... put the first values first.
            array[right] = array[left];
            array[left] = first;
            left++;
        }
    }

    // OK, now we have "left" pointing to the actual data that needs sorting
    Arrays.sort(array, left, array.length);
}


Multidimensional matrix exponentiation Is there a more efficient way of exponentiating a multidimensional matrix?
This is how I am currently doing it:

import numpy as np
from scipy import linalg

A = np.random.normal(0,0.01, (3, 2,2))
P = np.zeros((3,2,2))
for i in range(3):
     P[i] = linalg.expm( np.multiply(A[i], 0.5))

Note that my real array is much larger and any effort to speed up the process is greatly appreciated.

Dig through the source code for expm, which leads ultimately to a rather extensive bit of code in scipy.sparse.linalg.matfuncs.py.  This uses a well known Pade approximation.  It is specifically designed to work with on a square nxn matrix.  So generalizing to a (m, n, n) matrix without iterating on m does not work.

Years ago I sped this up by using f2py to calculate an analytical (n,3,3) inverse.  But I recall a simpler expm implementation.  The current one appears to have a lot of helper functions and special cases, presumably hoping to gain speed.

If your matrix is always (m, 2,2) there might be a special case.

This might be better as a Stackoverflow question, since it has less to do with programming style or code organization, and more with calculation alternatives in the numpy and scipy libraries.  But a SO search for expm and numpy/scipy turns up very few questions.  The closest suggests looking at the source code just as I suggested.


Print the list of winter bash 2014 hats as a list of checkboxes in GFM format In Winter Bash 2014,
since there is no easy way to see the hats I'm missing per site,
I decided to use Gists for that.
A perhaps not so well-known feature of GitHub Flavered Markdown (GFM) format used everywhere on GitHub is that if you write bullet point lists with embedded [ ] like this:

- [ ] Bill Lumbergh: answer 5 questions on Saturday (UTC)
- [ ] On The Road: ask using the app
- [ ] Bugdroid: use the Android app
- [ ] Not a cherry: use the iOS app
- [ ] I Voted Today: vote limit and 10+ questions

These will be rendered not as regular bullet point lists but a list of checkboxes:
clicking on an item will toggle the checkbox, triggering a commit.

Just one little problem:
there's no obviously easy way to copy-paste the hat names and descriptions from the Winter Bash website.
So I cooked up this simple soup (pun totally intended) using Python's Beautiful Soup:

#!/usr/bin/env python

from urllib import request

from bs4 import BeautifulSoup
import os

URL = 'http://winterbash2014.stackexchange.com/'

def load_html_doc(url):
    cached_file = 'page.html'
    if os.path.isfile(cached_file) and os.path.getsize(cached_file):
        with open(cached_file) as fh:
            html_doc = fh.read()
    else:
        html_doc = request.urlopen(url).read()
        with open(cached_file, 'wb') as fh:
            fh.write(html_doc)
    return html_doc

def get_soup(url):
    html_doc = load_html_doc(url)
    return BeautifulSoup(html_doc)

def print_hats_as_gfm_checkboxes(soup):
    for box in soup.find_all('a', attrs={'class': 'box'}):
        name = box.find(attrs={'class': 'hat-name'}).text
        description = box.find(attrs={'class': 'hat-description'}).text
        print('- [ ] {}: {}'.format(name, description))

def print_pretty(soup):
    print(soup.prettify())

def main():
    soup = get_soup(URL)
    print_hats_as_gfm_checkboxes(soup)
    # print_pretty(soup)

if __name__ == '__main__':
    main()

This produces a list of all the hats in the right format to be suitable for copy-pasting into a Gist.

Anything to improve here?

Don't do this:

#!/usr/bin/env python

Some people make the poor excuse that PEP 394

  recommends that python continue to refer to python2 for the time being

but this doesn't even apply to you since yours only supports Python 3!

Use this instead:

#!/usr/bin/env python3

Your code could do with doc comments; at least load_html_doc could. These don't need to be long.

There's nothing wrong with caching files locally, but I would suggest writing to tempfile.gettempdir().

request.urlopen returns a file-like object. Moments later you use with on a file object but you neglect to do so on request.urlopen's!

I would do an early return in load_html_doc.

Your os.path.isfile(cached_file) check is strange; if it's a directory instead you don't actually deal with the resulting problem. It's not a big deal, but I would use os.path.exists to prevent it looking like you're actually checking.

cached_file would be better as a default argument, and I'd call it cache_file or even cache_path. IMHO fh isn't that great a name either.

Here's the updated code, still writing to the same file:

def load_html_doc(url, cache_file='page.html'):
    if os.path.exists(cache_file) and os.path.getsize(cache_file):
        with open(cache_file) as cache:
            return cache.read()

    with request.urlopen(url) as webpage:
        html_doc = webpage.read()
    with open(cache_file, 'wb') as cache:
        cache.write(html_doc)
    return html_doc

After all of the comments on load_html_doc, I'm sad (happy?) to say there isn't much to talk about with the rest of it. I would remove the # print_pretty(soup) comment, though.


Review my prime factorization function. Is it complete? This is a function designed to return all the prime factors of a given number, in sorted order from least to greatest, in a list. It relies on another function that checks if a given number is prime. Here is the prime checking function:

def isPrime(x):
    if x <= 1 or isinstance(x,float):
        return False
    else:
        for divisor in range(2,x,1):
            if x % divisor == 0:
                return False
        return True

Here is the prime factorization function:

def allPrimeFactors(y):
    primeFactors = []
    restart = True
    while restart:
        restart = False
        for i in range(2,y,1):
            if y % i == 0 and isPrime(i):
                primeFactors.append(i)
                y = y / i
                if isPrime(y) and y == i:
                   primeFactors.append(y)
                else:
                   pass
                restart = True
            else:
                pass

    primeFactors.sort()
    return primeFactors

The while loop is designed to restart the for loop at the first iterable value, which is, in this case, 2. This is so that, if y factors into 2, or any other prime, more than once, it will appear in the final list more than once, how ever many times it, or any other prime, is part of the factorization of y. I found that, if a number's prime factorization was 2 primes of the same value, it would return an incomplete list, therefore I added the if statement to the end of the function so that if the final value of y was prime AND equal to the current value of i, it would append the final value of y to the end of the list, therefore making it complete.

This is a looping function I made to test the prime factorization function with a large set of numbers:

def primeFactorsTester(d,e):
for f in range(d,e+1,1):
    if f == 1:
        print "1 is undefined"
    elif isPrime(f):
        print str(f) + " is prime"
    else:
        print "the prime factors of " + str(f) + " are "+ str(allPrimeFactors(f))

It is meant to produce an output that is human readable, and is primarily a testing function.

Does my prime factorization function seem complete or incomplete? Any general feedback to give? I would love to know.

Your result is incorrect:

>>> allPrimeFactors(360)
[2, 2, 3, 3, 5]
>>> import operator
>>> reduce(operator.mul, allPrimeFactors(360), 1)
180

It is also unnecessarily complicated.  The usual approach is, whenever you encounter a prime factor, extract as many powers of it as possible from the number.  Here's another function that works:

def reallyAllPrimeFactors(n):
    factors = []
    for i in range(2, n + 1):
        while n % i == 0:
            factors.append(i)
            n = n // i
    return factors
>>> reallyAllPrimeFactors(360)
[2, 2, 2, 3, 3, 5]
>>> reduce(operator.mul, reallyAllPrimeFactors(360), 1)
360

Further optimizations to that are possible, but it is already much more efficient than your original.  For one thing, it doesn't need to check any of its results for primality.  (Why?)


Convert list to a string with double quotes For an external application I need to send a command as a string, like this:

["START", "1", "2", "3", "4", "STOP"]

Note the double quotes!

I create this command with the following function:

def create_command(amount):
    command = ["START"]
    list = create_list(amount)
    command += list
    command += ["STOP"]
    command = str(command )
    command = command.replace("\'", "\"")
    return command

And I create a list from a given number with the following function:

def create_list(data):
    list = []
    data = str(data)
    for letter in data:
        list.append(letter)
    return list

Is the a way to make both functions more pythonic and not so straightforward?  I don't really like them now, they look a bit clumpsy and I think there must be a better way do the thing.

create_list is building a list of all the items in the string form of data. And so you can change it to:

def create_list(data):
    return list(str(data))

I find it easier to read create_command if you merge some of the lines together:

def create_command(amount):
    command = ["START"] + create_list(amount) + ["STOP"]
    return str(command).replace("\'", "\"")

And so you can merge the above two changes together:

def create_command(amount):
    command = ["START"] + list(str(amount)) + ["STOP"]
    return str(command).replace("\'", "\"")

Expanding further, rather than using str.replace you can use json.dumps to format the list. This has the benefit that it will escape ' and " characters for you, which your code doesn't correctly handle. (Thanks Mathias)

import json

def create_command(amount):
    command = ["START"] + list(str(amount)) + ["STOP"]
    return json.dumps(command)


LeetCode 928: Minimize Malware Spread II I'm posting my code for a LeetCode problem. If you'd like to review, please do so. Thank you for your time!
Problem
(This problem is the same as Minimize Malware Spread, with the differences bolded.)
In a network of nodes, each node i is directly connected to another node j if and only if graph[i][j] = 1.
Some nodes initial are initially infected by malware.  Whenever two nodes are directly connected and at least one of those two nodes is infected by malware, both nodes will be infected by malware.  This spread of malware will continue until no more nodes can be infected in this manner.
Suppose M(initial) is the final number of nodes infected with malware in the entire network, after the spread of malware stops.
We will remove one node from the initial list, completely removing it and any connections from this node to any other node.  Return the node that if removed, would minimize M(initial).  If multiple nodes could be removed to minimize M(initial), return such a node with the smallest index.
Example 1:

Input: graph = [[1,1,0],[1,1,0],[0,0,1]], initial = [0,1]
Output: 0

Example 2:

Input: graph = [[1,1,0],[1,1,1],[0,1,1]], initial = [0,1]
Output: 1

Example 3:

Input: graph = [[1,1,0,0],[1,1,1,0],[0,1,1,1],[0,0,1,1]], initial = [0,1]
Output: 1

Note:

\$1 < \text{graph}.\text{length} = \text{graph}[0].\text{length} <= 300\$
\$0 <= \text{graph}[i][j] == \text{graph}[j][i] <= 1\$
\$\text{graph}[i][i] = 1\$
\$1 <= \text{initial}.\text{length} < \text{graph}.\text{length}\$
\$0 <= \text{initial}[i] < \text{graph}.\text{length}\$

Inputs
[[1,1,0],[1,1,0],[0,0,1]]
[0,1]
[[1,1,0],[1,1,1],[0,1,1]]
[0,1]
[[1,1,0,0],[1,1,1,0],[0,1,1,1],[0,0,1,1]]
[0,1]

Outputs
0
1
1

Code
#include <cstdint>
#include <vector>
#include <queue>
#include <unordered_set>
#include <algorithm>

struct Solution {
    using uint16 = std::uint_fast16_t;
    int minMalwareSpread(
        std::vector<std::vector<int>>& graph,
        std::vector<int>& initial
    ) {
        uint16 smallest_node = 0;
        uint16 initial_len = std::size(initial);
        uint16 min_len = std::size(graph);
        std::sort(std::begin(initial), std::end(initial));

        for (const auto init_node : initial) {
            const uint16 curr_len = breadthFirstSearch(graph, initial, init_node);

            if (curr_len < min_len) {
                min_len = curr_len;
                smallest_node = init_node;
            }
        }

        return smallest_node;
    }

private:
    static uint16 breadthFirstSearch(
        const std::vector<std::vector<int>>& graph,
        std::vector<int>& initial,
        uint16 node
    ) {
        std::queue<uint16> nodes_queue;
        std::unordered_set<uint16> nodes_set = {node};
        uint16 count = 0;

        for (const auto init_node : initial) {
            if (init_node != node) {
                nodes_queue.push(init_node);
            }
        }

        while (!nodes_queue.empty()) {
            uint16 curr_node = nodes_queue.front();
            nodes_queue.pop();

            if (nodes_set.count(curr_node)) {
                continue;
            }

            nodes_set.insert(curr_node);
            ++count;

            for (uint16 index = 0; index < std::size(graph); ++index) {
                if (index != curr_node && graph[curr_node][index]) {
                    nodes_queue.push(index);
                }
            }
        }

        return count;
    }
};

References

Problem

Discuss

Solution

Misleading type alias
You are using uint16 = std::uint_fast16_t, but if I see the name uint16, as a programmer I assume this is identical to uint16_t. But uint16_t and uint_fast16_t might have very different properties, so therefore it is a potentially misleading name.
When you create a type alias, you usually want to give it a name that conveys its intent. Here, it's to hold sizes and indices. So I would recommed using size_type = ... instead.
Name functions after their intent, not their implementation
The function breadthFirstSearch() indeed uses a BFS algorithm internally, but the intent of this function is to return the number of infected nodes given a set of initial infectations and an excluded node. So name it something like countInfectedNodes() instead.
While you are at it, rename node in that function to excluded_node, to make it clear what its purpose is.
Use a std::vector<bool> to keep track of infected nodes
A bitset is much more compact and faster than a std::set in this case, since nodes are just numbers between zero and std::size(graph). However, you can't use std::bitset here because that needs the size to be known at compiletime. Instead, this is one of the few occasions where a std::vector<bool> is useful:
std::vector<bool> nodes_set(std::size(graph));
nodes_set[excluded_node] = true;
...

Avoid modifying the input parameters
While the LeetCode problem does state that the parameters are non-const, it is nevertheless bad practice to modify the input parameters if it's not expected that this will happen.
You can avoid sorting initial, by just modifying the way you set smallest_node:
if (curr_len < min_len || (curr_len == min_len && init_node < smallest_node)) {
    min_len = curr_len;
    smallest_node = init_node;
}

Consider stopping counting infections if it won't improve the minimum
Currently, for every candidate node to remove, you count all infections. However, if halfway you notice that you are already above the minimum seen so far, then you can stop early. Just pass the previous min_len to countInfectedNodes() and check it when incrementing count:
static size_type countInfectedNodes(
    const std::vector<std::vector<int>>& graph,
    std::vector<int>& initial,
    size_type excluded_node,
    size_type current_min,
) {
    ...
        nodes_set[curr_node] = true;
        if (++count > current_min) {
            break;
        }
    ...
}


C# and unity. Moving an object to a target and destroy target once we're close This is my first ever c# script. I have played around a little and I think this is some proper c# code. Since it's my very first c# ever, I'm assuming there's a few mistakes, things that could be better, bad conventions and such. If you see any, please let me know so I can improve!
using System.Collections;
using System;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.AI;

public class Mover : MonoBehaviour
{
    [SerializeField] GameObject target;
    private GameObject player; // creating a variable for the player object

    void Update()
    {
        if (target != null) { // if the target is already destroyed, we don't have to do anything anymore
            player = this.gameObject; // the script is attached to the player

            // Here I expected I could get the NavMeshAgent component via something like player.NavMashAgant.
            // I do the same later for target.transform.
            // Is there a reason this component is not accessible via dot notation?
            GetComponent<NavMeshAgent>().destination = target.transform.position; // set the players destination as the targets potion. It starts moving automatically.
            if (AreClose(player, target)) {
                Destroy(target);
            }
        }
    }

    private static double Distance(GameObject a, GameObject b)
    {
        Vector3 aPos = a.transform.position;
        Vector3 bPos = b.transform.position;
        // A really long line.. What is the convention to make this more readable?
        return Math.Sqrt(Math.Pow(aPos.x - bPos.x, 2) + Math.Pow(aPos.y - bPos.y, 2) + Math.Pow(aPos.z - bPos.z, 2));

    }

    private static bool AreClose(GameObject a, GameObject b)
    {
        double result = Distance(a, b);
        if (result < 2.5) // random magic number for now
        {
            return true;
        } else
        {
            return false;
        }
    }
}

Welcome to Code Review!
I'm not too familiar with Unity, so I can't really comment or offer advice on Unity-specific things.
There are still a few places where I feel like your code could be improved however.
Whitespace
While I generally prefer to keep my code not too dense, I recommend never having more than one blank line in a row (except maybe if you want to separate a block of imports or usings from the rest of the code); some editors will even automatically remove any consecutive blank lines when you format your document.
Similarly, that blank line at the end of your Distance() method doesn't serve any purpose, so I would just remove it, or maybe move it right below your aPos and bPos declarations.
AreClose() method
Your AreClose() method could be rewritten in a much shorter and cleaner way, like so :
private static bool AreClose(GameObject a, GameObject b)
{
    double result = Distance(a, b);
    return (result < 2.5); // random magic number for now
}

That's because the expression result < 2.5 itself evaluates to a boolean, so there's no need to check whether its value true or false, you can simply return it.
You could further shorten it to...
private static bool AreClose(GameObject a, GameObject b)
{
    return Distance(a, b) < 2.5; // random magic number for now
}

...since, in my opinion, the variable result doesn't add anything to the readability or the clarity of the code. Seeing Distance(a, b) < someMagicNumber is clear enough on its own.
You could go even further and take advantage of C#'s expression body definitions, like so
private static bool AreClose(GameObject a, GameObject b) => Distance(a, b) < 2.5; // random magic number for now

...which doesn't look too great here because of that comment. I would recommend you put that magic number into a const variable, either declared inside AreClose() or as a class variable. This will make your code easier to modify in the future, and also allows you to give it a meaningful name like MIN_DISTANCE.
private static bool AreClose(GameObject a, GameObject b) => Distance(a, b) < MIN_DISTANCE;

Distance() method
Like I said earlier, I'm not too familiar with Unity, but I'm pretty sure it has a built-in method for computing the distance between two vectors, like so Vector3.Distance(aPos, bPos).
There's nothing inherently wrong with doing it yourself if you're just trying to learn and get familiar with a new language or concept, but if you're looking for optimal performance or accuracy, I would recommend making sure there isn't already a function that does what you need before trying to reinvent the wheel -although in this specific case it's probably not too important.
Additionally, I would avoid using Math.Pow() when you only want to square a number, as it's quite an expensive operation compared to simply doing (aPos.x - bPos.x) * (aPos.x - bPos.x). This is because Math.Pow() needs to be able to handle non-integer exponents and (probably) uses Taylor series (https://en.wikipedia.org/wiki/Taylor_series) to compute powers for arbitrary exponents, even if your exponent happens to be 2. So you can just compute the square yourself, which should significantly improve performance.
You could also create a DistanceSquared() method, and compare it with MIN_DISTANCE * MIN_DISTANCE, which is equivalent to comparing Distance() and MIN_DISTANCE. This allows you to avoid using Math.Sqrt(), which is, again, a rather slow operation. Then you can rewrite Distance() like this, if you still want it :
public static double Distance(GameObject a, GameObject b) => Math.Sqrt(DistanceSquared(a, b));


Convert data back-and-forth between CSV files and pretty-print I wrote a script to convert CSV files in human readable format and vice versa.

Human readable format is achieved like so:

$ ./transform table.csv inflate
cola colb      colc
1    a         g
2    b         g
3    c         g
4    d         g
5    eeeeeeeee g

And conversion back to CSV is done with:

$ ./transformtable table.t
cola;colb;colc;
1;a;g;
2;b;g;
3;c;g;
4;d;g;
5;eeeeeeeee;g;

Code:

import Data.List
import Data.List.Split
import System.Environment
main = do
  args <- getArgs
  file <- readFile (args !! 0)
  let lp = preprocess (lines file)
  let erg = (if elem "inflate" args then (inflate) else (compress)) lp
  mapM_ putStrLn $ map concat erg

rpad len string = take len $ string++(repeat ' ')

--ensure last col has elements
preprocess lines = map (rpad len) lines
             where len = maximum $ map (length) lines

--calc lengths of the chunks(colname + spaces) for splitplaces
dissect line = zipWith (\a b->length (a++b)) names spaces
          where names = wordsBy (==' ') line
                spaces= wordsBy (/=' ') (line++" ") --add " " to ensure space after last col-name

compress lines = add_del $ transpose $ map (shrinkcol) cols
            where delim       = ';'
                  head_lens   = dissect (head lines)
                  cols        = transpose $ map (splitPlaces head_lens) lines
                  shrinkcol c = map (\el -> reverse $ dropWhile (==' ') $ reverse $ dropWhile (==' ') el) c
                  add_del     = map (map (++[delim]))

inflate lines = transpose $ map padcol $ table
          where table = transpose $ map (splitOn ";") lines --list of cols
                padcol c = map (rpad m) c --pad whole col
                   where m = (1+) $ maximum $ (map length) c --determine longest string in col, pad one more for prettiness

I'm relatively new to haskell, so I'm wondering if this can be done more elegantly.

Names

Follow the Haskell convention of names

The Haskell convention is camelCase.  That is, start names with a
lowercase letter, and join words together by capitalizing them.  So
head_lens would be headLens, or better yet, headerLengths.

Use names with meanings, not just labels

Come to think of it, an even better name would be columnWidths,
as that is not just what the value is, but what it means (and
what it is used for).  lp and erg don't tell me a lot,
either.  For preprocess I would suggest a name like rightPadLines.  That way, I won't be so surprised to find out what it does :)

Don't be afraid of longer names

It is also useful and nice for other readers not to use
abbreviations, unless the scope is very small.  So the a and b
in dissect line are good and clear (very short scope -> very
short names).  add_del, on the other hand, would be more readable
as addDelimiters.  delim is a somewhat more conventional
abbreviation.

Types

Use the type system

Yes, Haskell can find out what types your values and functions
have.  And it will be correct in many, many cases.  But if you
are wrong, Haskell cannot tell you, unless you tell the compiler
what you intend to do.  So always, always write the type signatures
of at least the top-level bindings.  That way, if your function is
not yet finished, or you make a mistake, the compiler will be able
to tell you almost exactly where.

So:

main :: IO ()
main = do -- ..

rpad :: Int -> String -> String
rpad len string = take -- ..

preprocess :: [String] -> [String]
preprocess lines = map -- ..

dissect :: String -> [Int]

compress :: [String] -> [[String]]

inflate :: [String] -> [[String]]

Adding those type signatures alone, makes the function more
readable, because now I can expect what it is going to do.

Use hlint on the code and heed warnings from ghc

Many warnings and suggestions are worthwile.  They can point out
possible mistakes or partial functions, but also show what an
idiomatic Haskell solution could be.  For instance:

Use head instead of (!!0)

readfile (args !! 0) is reminiscent of other languages, where the
command line arguments are in an array, which you'd need to index.
Using the 0th index though, is normally done by using the head
function.  readFile $ head args is the suggested alternative.

elem is more readable as `elem`

Functions that take more than one argument, can be inserted between
the first and second argument (infix) if you write backquotes
around them.  For many functions, such as elem, mod and div,
this makes much sense.  So if elem "inflate" args can be
rewritten as if "inflate" `elem` args.

Redundant parentheses

In that same line, (inflate) and (compress) have parentheses
around them, but they are really not necessary.  Removing them
makes the line more readable (even for LISP programmers).

The same goes for map (length) lines in preprocess, map (shrinkcol) cols in compress and (map length) c in inflate.
And even string++(repeat ' ') can do without the parentheses.

Shadowed bindings

Three functions use the name lines for a parameter, which happens
to be the name of a function, too.  It is best to heed this
warning, for two reasons:

If you happen to want to split some string later on in those
functions, you'll have to change the name anyway.
People who see lines somewhere, will probably think of the
function first, be confused, then remember that it is a
redefined value.

Using ls is not that bad, by the way, especially if you change
the type to be more documentational (see below).

Eta reduction

This mathematical term comes down to "remove the variable name", so
that the focus lies more on what the function does, and less of
what it functions on.  The function shrinkcol has a c on both
ends, and they can both be removed, resulting in:

shrinkcol = map (\el -> reverse $ dropWhile (==' ') $ reverse $ dropWhile (==' ') el)

Which expresses exactly the same, just in less "words".

Avoid lambda

This warning, on the same function as above, suggests about the
same thing, but now for the \el variable.  However, because we
use the $ for application of values, this line should be
rewritten using . for composition.  As f . g means: do f
after g, this comes mostly down to just replacing the $ with
.:

shrinkcol = map (reverse . dropWhile (==' ') . reverse . dropWhile (==' '))

Another composition opportunity

The last warning that ghc and hlint give us, is to use composition
in the mapM_ call.  Instead of saying: "map putStrLn over the
result of mapping concat over the list erg, and combine that
into one IO action", you can say "map (putStrLn after concat)
over the list erg, and combine that into one IO action".  Which
is shorter in English already :)

mapM_ (putStrLn . concat) erg

And now we are warning-free!

Types

Use the type system

What, again?  Yes.  But this time, use it to document the intent of
the functions, not just the functionality.  The easiest way to do
this, is to declare simple type aliases.  For instance,
preprocess works on lists of String, but each String is
actually a Line.  So adding type Line = String, while it does
not give any added compiler safety (Line is just an alias, and
exactly the same as String), it does give a bit of documentation.

type Line = String
type Cell = String

preprocess :: [Line] -> [Line]

compress :: [Line] -> [[Cell]]

inflate :: [Line] -> [[Cell]]

Of course, making stronger types with constructors, using data is
even better, so that the type system helps you check whether the
code works, even before you run it.  Then, you can write functions
as transformations between types, instead of transformations
between "just Strings".


React app that receives data via an AJAX requests, display the data and makes it searchable I have recently started using React and wish to know if my code adheres to the React coding style, whether I am following the same approach towards solving any problem and is there any way I can make the code better.

var React = require('react');
var Bootstrap = require('react-bootstrap');
var xhr = require('superagent');
var Input = Bootstrap.Input;
var Panel = Bootstrap.Panel;

var Polls;
var View = React.createClass({
    getInitialState: function() {
        return {
            selectedPoll: '',
            query: ''
        };
    },
    componentDidMount: function() {
        var self = this;
        xhr.get('/polls').end(function(err, res) {
            if(err) {
                console.log(err);
            } else {
                Polls = res.body;
                if(self.isMounted()){
                    self.setState({
                        selectedPoll: res.body,
                        query: ''
                    });
                }
            }
        });
    },
    _onchange: function(value) {
        var result = [];
        (Polls).map(function(val) {
            if(val.uuid.toLowerCase().indexOf(value) !== -1 || val.question_text.toLowerCase().indexOf(value) !== -1) {
                result.push(val);
            }
        });
        this.setState({
            query: value,
            selectedPoll: result
        });
    },
    render:function() {
        return (

                    <SearchBar change={this._onchange} val={this.state.query} />
                        <PollList data={this.state.selectedPoll} />

                    <PollView />

        );
    }
});

var SearchBar = React.createClass({
    doSearch: function(e) {
        this.props.change(e.target.value.toLowerCase());
    },
    render: function() {
        return (
            <Input type="text" value={this.props.val} onChange={this.doSearch} />
        )
    }
});

var PollList = React.createClass({
    render: function() {
        var arr = [];
        if(this.props.data !== ''){
            this.props.data.map(function(value) {
                var head = value.uuid;
                var body = value.question_text;
                arr.push(
<Panel header={head} key={head}>{body}</Panel>
);
            });
        }
        return (

{arr}

        );
    }
});

var PollView = React.createClass({
    render: function() {
        return (

Dummy space

        );
    }
});

module.exports = View;

Looks pretty idomatic :) the one major thing you might want to do is make use of propTypes to more explicitly define your Component API's. PropTypes help set expections for what values should/can be.

For instance in SearchBar your doSearch() method looks like

this.props.change(e.target.value.toLowerCase());

There is an implicit assumption that onChange is never going to be null, and that it's a function. You can codify that (and guard against dumb mistakes) by adding

propTypes: {
  onChange: React.PropTypes.func.isRequired
}

alternatively you can leave off: isRequired and provide a default value for onChange (perhaps a noop function).

In PollList

this line is odd (you also do this in _onChange of View):

this.props.data.map(function(value) {
   var head = value.uuid;
   var body = value.question_text;
   arr.push(
<Panel header={head} key={head}>{body}</Panel>
);
});

.map() already returns a new array so there is no need to push into the arr either just return the child elements or use a forEach()


Product of all ints in array besides that an index i I'm looking for a better implementation of the algorithm.

/*
 * You have an ordered array X of n integers. Find the array M containing
 * elements where Mi is the product of all integers in X except for Xi.
 * You may not use division. You can use extra memory.
 */

#include <iostream>
#include <stack>
#include <queue>
#include <vector>

/*!
 * Class used to generate sequences with helper function
 */
template <class T>
class sequence : public std::iterator<std::forward_iterator_tag, T>
{
    T val;
public:
    sequence(T init) : val(init) {}
    T operator *() { return val; }
    sequence &operator++() { ++val; return *this; }
    bool operator!=(sequence const &other) { return val != other.val; }
};

template <class T>
sequence<T> gen_seq(T const &val) {
    return sequence<T>(val);
}

static const int N = 3;
std::stack<int> stk;
std::queue<int> que;
int main(int argc, char *argv[]) {
  std::vector<int> seq(gen_seq(1), gen_seq(N + 1));
  for (int x = 0, y = N - 1; x < N && y >= 0; ++x, --y) {
    if (x == 0 && y == N - 1) {
      stk.push(1);
      que.push(1);
    } else {
      stk.push(stk.top() * seq[x - 1]);
      que.push(que.back() * seq[y + 1]);
    }
  }
  for (int x = 0; x < N; ++x) {
    std::cout << (stk.top() * que.front()) << std::endl;
    stk.pop();
    que.pop();
  }
}

First, some comments on the existing code:

/*
 * You have an ordered array X of n integers. Find the array M containing
 * elements where Mi is the product of all integers in X except for Xi.
 * You may not use division. You can use extra memory
 */

You don't have anything called X in the code, or generate M, so this comment could be better

static const int N = 3;
std::stack<int> stk;
std::queue<int> que;

Why use globals? Also, try to name variables by what they do or represent, rather than their type.

int main(int argc, char *argv[]) {

For that matter, why put the logic in main where you can't test it?
I would hope to see something structured like:

std::vector<int> products(std::vector<int> const &X)
{
    auto n = X.size();
    std::vector<int> M(n, 1);

    // ... algorithm ...

    return M;
}

int main()
{
    static const int N = 3;
    std::vector<int> seq(gen_seq(1), gen_seq(N + 1))
    std::vector<int> result = products(seq);
    // print the result
    // or assert correctness
}

Or possibly several test functions, each called from main.

As for the algorithm itself, its working isn't really clear, and that's where comments would be useful. I'm sure you have a deep intuitive understanding of why your nameless stack and queue generate the right results, but without a lot of effort I don't. In six months' time, you may not either.

Your loop can be cleaner anyway; instead of:

  for (int x = 0, y = N - 1; x < N && y >= 0; ++x, --y) {
    if (x == 0 && y == N - 1) {
      stk.push(1);
      que.push(1);
    } else {
      stk.push(stk.top() * seq[x - 1]);
      que.push(que.back() * seq[y + 1]);
    }
  }

we can:

move the special case if (x == 0 ... outside
then, you only have to iterate over the values used in the second branch (so 1 <= x < N, N-2 >= y >= 0)
but we only use x-1 and y+1 in the body of the loop, so simplify this to 0 <= x < N-1 and N-1 >= y >= 0 and just use x and y in the body
notice that the two conditions (x < N-1 && y >= 0) will always agree, so we don't have to test both

to get:

  stk.push(1);
  que.push(1);
  for (int x = 0, y = N-1; x < N-1; ++x, --y)
  {
    stk.push(stk.top() * seq[x]);
    que.push(que.back() * seq[y]);
  }

and a simpler implementation:

std::vector<int> products(std::vector<int> const &X)
{
    auto n = X.size();
    std::vector<int> M(n, 1); // initialise all values to 1

    for (int i = 0; i < n; ++i)
    {
        // set Mj <= Mj * Xi for all j != i
        for (int j = 0; j < n; ++j)
            if (j != i) M[j] *= X[i];
    }
    return M;
}

This one uses no extra memory but is O(n^2)