Caching a value just once and update it without blocking other threads I need some advice about my code.
The following code will create a new object the first time, and also force other threads to wait for the creation. When the object is created, we return the cached value, and when the value is about to expire, we renew it without blocking other threads, the reference is replaced by the new value.

I'm not sure about the SemaphoreSlim & ManualResetEvet parts, may be somebody can simplify this code.

Thanks.

public class UpdatableLazy<T>
{
    public class Container<T>
    {
        public readonly T Value;

        internal Container(T value)
        {
            Value = value;
        }
    }

    private readonly Func<Task<T>> _updateFunc;
    private readonly Func<T, bool> _isRenewNeeded;
    private volatile Container<T> _container;
    private volatile bool _isUpdating;
    private readonly SemaphoreSlim _ss = new SemaphoreSlim(1, 1);

    public UpdatableLazy(Func<Task<T>> updateFunc, Func<T, bool> isRenewNeeded)
    {
        _updateFunc = updateFunc;
        _isRenewNeeded = isRenewNeeded;
    }

    private readonly ManualResetEventSlim _mre = new ManualResetEventSlim();

    public async Task<T> GetValueAsync()
    {
        //The value doesn't exist yet
        if (_container == null)
        {
            //if a thread is already creating the value, we will wait for the value
            if (!await _ss.WaitAsync(0).ConfigureAwait(false))
            {
                _mre.Wait();
            }

            //After the release of the ManualResetEvent, the value should be available
            if (_container != null)
            {
                return _container.Value;
            }

            try
            {
                //Let's create the value
                _container = new Container<T>(await _updateFunc().ConfigureAwait(false));
                return _container.Value;
            }
            finally
            {
                //We tell awaiting threads that the value is available
                _mre.Set();
                _ss.Release();
            }
        }

        //If the value is not updating and we need to update, we replace the old value by a new one
        if (!_isUpdating && _isRenewNeeded(_container.Value))
        {
            _isUpdating = true;

            try
            {
                _container = new Container<T>(await _updateFunc().ConfigureAwait(false));
            }
            finally
            {
                _isUpdating = false;
            }
        }

        return _container.Value;
    }
}

How I tested it :

class SomeObject
{
    public string Value = "Hello";
    public DateTime Expire = DateTime.UtcNow.AddSeconds(5);
}

class Program
{
    private static int _count;
    static void Main(string[] args)
    {
        var iteration = 10000;
        var cachedObj = new UpdatableLazy<SomeObject>(Create, o => DateTime.UtcNow > o.Expire);
        Parallel.For(0, iteration, new ParallelOptions {MaxDegreeOfParallelism = 2048}, async i =>
        {
            await cachedObj.GetValueAsync().ConfigureAwait(false);
            Interlocked.Increment(ref _count);
        });

        if (iteration != _count)
            throw new Exception("_count and iteration should be equal");

        Console.WriteLine("Done");
        Console.ReadKey();
    }

    static async Task<SomeObject> Create()
    {
        await Task.Delay(1000).ConfigureAwait(false);
        return new SomeObject();
    }
}

The initial creation looks thread-safe at first glance. I don't know why you await your semaphore instead of just waiting synchronously with _ss.Wait(0) but I don't think it matters much.

The updating part however is not thread-safe. Any number of threads can come in between:

if (!_isUpdating && _isRenewNeeded(_container.Value))

and

_isUpdating = true;

This will result in every thread getting their own value, which is probably not what you want.

You should also call Dispose on disposable objects when you no longer need them.


Comparison of excel files, cell by cell below is the code that i created as a POC towards an implementation for 2 excel file comparison (cell by cell)

This was just to test the logic, which works as expected, however i found out that for big data (hundreds of rows and columns) this approach using excel object is very time consuming. Approx time taken for comparing 130 rows and 248 columns was about 10 mins.

Is there any way that i can optimize this

Note: " the code is at basic level" kindly answer towards optimization only

The Code is as below

Dim a()
Dim b()
x = 0
file1 = InputBox("Enter the first file")
file2 = InputBox("enter the second file")

'create FSO and check if the file exist
Set fso = CreateObject("Scripting.FileSystemObject")
 if (fso.fileExists(file1)) and (fso.fileExists(file2)) then
  MsgBox "The file exists"
Else
  MsgBox "The file doesn't exists"
  ErrCount = 1
End if

if Errcount <> 1 then
 'Create excel object for both the files and compare
  Set objExcel = CreateObject("Excel.Application")
 'Iterate from worksheets for both the file
  Set objFile1 = objExcel.Workbooks.Open(file1)
  objFile1Count = objFile1.Worksheets.count

  Set objFile2 = objExcel.Workbooks.Open(file2)
  objFile2Count = objFile2.Worksheets.count
 End if

'sheet should be same
 if objFile1Count <> objFile2Count then
   ErrCount = 1
 End if
' Loop through the sheets
  if ErrCount <> 1 then
      For cnt = 1 to objFile1.Sheets.count
        Set file1Sheet = objFile1.Sheets(cnt)
        Set file2Sheet = objFile2.Sheets(cnt)
        'Check the sheet name
        if file1Sheet.Name <> file2Sheet.Name then
          ErrCount = 1
          Exit For
        Else
          'File 1
          file1RC = file1Sheet.usedrange.rows.count
          file1CC = file1Sheet.usedrange.columns.count
          'File 2
          file2RC = file2Sheet.usedrange.columns.count
          file2CC = file2Sheet.usedrange.columns.count

          'Iterate file
            For i = 2 to file1RC
              For j = 1 to file1CC
                      ReDim Preserve a(x)
                      ReDim Preserve b(x)
                      a(x) = Trim(file1Sheet.Rows(i).columns(j).value)
                      b(x) = Trim(file2Sheet.Rows(i).columns(j).value)
                      file2getRC = i &"," &j
                      file1getRC = i &"," &j

                      'Check compare and add color
                       File1Val = a(x)
                       File2Val = b(x)

                       if File1Val = File2Val then
                          'do nothing as the values matches
                       Else
                          RNC = split(file1getRC, ",")
                          Col = RNC(1)
                          Row = RNC(0)
                          Col = CInt(Col)

                          ty = file1Sheet.Rows(Row).Cells(1, Col).value
                          file1Sheet.Rows(Row).Cells(1, Col).Interior.ColorIndex = 6
                          file2Sheet.Rows(Row).Cells(1, Col).Interior.ColorIndex = 6
                          objFile1.Save
                          objFile2.Save
                       End if
                      x=x+1
              Next
            Next
           End if
        Next
  MsgBox "it works"
Else
  MsgBox "No file added or cancelled"
    End if

You only ask about performance, but I want to point out anyway that there is a small error in the definition of file2RC: you used column instead of row.

Now, let me get to the performance part. The thing making your code slow is that accesses to worksheets are expensive. Accordingly, they should be reduced to a minimum.

Currently, you access each cell individually. A better approach is to load all on the same sheet in one go. You can do this by assigning the Value property, or better the Value2 property (Value causes problems with fields formatted as dates.), of the used range to a Variant variable. After that, the variable contains a two-dimensional array consisting of the values in the range.

By comparing the values in corresponding arrays, you only need to access each sheet once to load the data and then for each time you find an error, in order to change the color.


Know the limits of an image after rotation I have an image which I know the number of rows and the number of columns. I would like to calculate the limits of the new image after a rotation.

   BufferedImage myImage = ImageIO.read( new File( "D:\\Users...jpg" ) );
    xmaxOrigine = myImage.getWidth() - 1;
    ymaxOrigine = myImage.getHeight() - 1;
    angle = 12;

    angleRadian = Math.toRadians( angle );
    cos = Math.cos( angleRadian );
    sin = Math.sin( angleRadian );

    p1X = (int) ( ( xmaxOrigine * cos ) - ( ymaxOrigine * sin ) );
    p1Y = (int) ( ( xmaxOrigine * sin ) + ( ymaxOrigine * cos ) );

    xmin_f = xmin_f < p1X ? xmin_f : p1X;
    xmax_f = xmax_f < p1X ? p1X : xmax_f;
    ymin_f = ymin_f < p1Y ? ymin_f : p1Y;
    ymax_f = ymax_f < p1Y ? p1Y : ymax_f;

    p2X = (int) ( ( 0  * cos ) - ( ymaxOrigine * sin ) );
    p2Y = (int) ( ( 0 * sin ) + ( ymaxOrigine * cos ) );

    xmin_f = xmin_f < p2X ? xmin_f : p2X;
    xmax_f = xmax_f < p2X ? p2X : xmax_f;
    ymin_f = ymin_f < p2Y ? ymin_f : p2Y;
    ymax_f = ymax_f < p2Y ? p2Y : ymax_f;

    p3X = (int) ( ( xmaxOrigine * cos ) - ( 0 * sin ) );
    p3Y = (int) ( ( xmaxOrigine * sin ) + ( 0 * cos ) );

    xmin_f = xmin_f < p3X ? xmin_f : p3X;
    xmax_f = xmax_f < p3X ? p3X : xmax_f;
    ymin_f = ymin_f < p3Y ? ymin_f : p3Y;
    ymax_f = ymax_f < p3Y ? p3Y : ymax_f;

    p4X = 0;
    p4Y = 0;

    xmin_f = xmin_f < p4X ? xmin_f : p4X;
    xmax_f = xmax_f < p4X ? p4X : xmax_f;
    ymin_f = ymin_f < p4Y ? ymin_f : p4Y;
    ymax_f = ymax_f < p4Y ? p4Y : ymax_f;

    widthFinal = xmax_f - xmin_f;
    heightFinal = ymax_f - ymin_f;

As you can see I look for the xmin, xmax, ymin, ymax for each points. I would like to make this operation once if it is possible. Also, how can I condense this code?

xmaxOrigine

So this looks like you are mixing a French word with English.  Be careful of that.  A native English reader will be mildly confused by this, as it looks similar to original which would make sense in that context.  But the real problem is with someone for whom origine does not look like a word.  For example, take native Mandarin speakers.  Presumably they know how to look up English words that they don't understand in the English code.  They won't however know that this is not an English word.  So they won't find it in an English directory and won't know that they should look in a French dictionary.

Perhaps your context will have you overwhelmingly sharing this code with people who speak enough French to know what you mean.  But if you do need to share with someone who doesn't speak French, that person may be confused by things like this.

    xmin_f = xmin_f < p1X ? xmin_f : p1X;
    xmax_f = xmax_f < p1X ? p1X : xmax_f;
    ymin_f = ymin_f < p1Y ? ymin_f : p1Y;
    ymax_f = ymax_f < p1Y ? p1Y : ymax_f;

This would be easier to read if you used the natural language constructs for this.

    xmin_f = Math.min(xmin_f, p1X);
    xmax_f = Math.max(p1X, xmax_f);
    ymin_f = Math.min(ymin_f, p1Y);
    ymax_f = Math.max(p1Y, ymax_f);

You also do this repeatedly.  If these are object fields, you can make a method to do this for you.

private void expandBoundaries(int x, int y) {
    xmin_f = Math.min(xmin_f, x);
    xmax_f = Math.min(xmax_f, x);
    ymin_f = Math.min(ymin_f, y);
    ymax_f = Math.min(ymax_f, y);
}

If they are not object fields, you can make objects to track them.  Then you'd use them like

    xRange.include(p1X);
    yRange.include(p1Y);

and at the end

    widthFinal = xRange.getMax() - xRange.getMin();
    heightFinal = yRange.getMax() - yRange.getMin();

or even

    widthFinal = xRange.getDifference();
    heightFinal = yRange.getDifference();

with include defined as

public void include(T value) {
    min = Math.min(min, value);
    max = Math.max(max, value);
}

You can also save some multiplications by storing the values for later use:

    float rotatedX2X = xmaxOrigine * cos;
    float rotatedY2X = ymaxOrigine * sin;
    float rotatedX2Y = xmaxOrigine * sin;
    float rotatedY2Y = ymaxOrigine * cos;

Then you could rewrite this code as

    BufferedImage myImage = ImageIO.read( new File( "D:\\Users...jpg" ) );
    xmaxOrigine = myImage.getWidth() - 1;
    ymaxOrigine = myImage.getHeight() - 1;

    angle = 12;
    angleRadian = Math.toRadians( angle );
    cos = Math.cos( angleRadian );
    sin = Math.sin( angleRadian );

    float rotatedX2X = xmaxOrigine * cos;
    float rotatedY2X = ymaxOrigine * sin;
    float rotatedX2Y = xmaxOrigine * sin;
    float rotatedY2Y = ymaxOrigine * cos;

    xRange.include((int) (rotatedX2X - rotatedY2X));
    yRange.include((int) (rotatedX2Y + rotatedY2Y));
    xRange.include((int)-rotatedY2X);
    yRange.include((int)rotatedY2Y);
    xRange.include((int)rotatedX2X);
    yRange.include((int)rotatedX2Y);
    xRange.include(0);
    yRange.include(0);

    widthFinal = xRange.getDifference();
    heightFinal = yRange.getDifference();

Note that this gets rid of p1X, p1Y, etc.  They weren't used in the original code snippet.  It's possible that they were used elsewhere.  If so, you'd have to put them back.

Then your Range class:

public class Range<T extends Number> {
    private T max;
    private T min;

    public Range(T min, T max) {
        this.min = min;
        this.max = max;
    }

    public void include(T value) {
        min = Math.min(min, value);
        max = Math.max(max, value);
    }

    public T getMin() {
        return min;
    }

    public T getMax() {
        return max;
    }

    public T getDifference() {
        return max - min;
    }
}

Note that you have to initialize the Range before using it.  You don't show the initialization in the original code snippet, so I didn't show it here.  It would look something like

    Range xRange = new Range(small, large);


Varying a second drop-down's options based on a first selection I have some JavaScript code that allows users to select a plan which triggers the second select box to populate with options. The code works, however, it seems to be very inefficient. Any advice on improving the efficiency?

JavaScript:

var term = 0;
var price = 0;
var additional = 0;
var fix = 0;
var plan = document.getElementById('billing-plan');
plan.addEventListener('change', enableBilling, false);
var select = document.getElementById('billing-price');
select.addEventListener('change', updatePrice, false);

var basic = {
    "Option" : ["$200/month for 1-yr", "$250/month"],
    "Value" : [300, 350]
}
var prime = {
    "Option" : ["$300/month for 1-yr", "$350/month"],
    "Value" : [400, 450]
}
var gold = {
    "Option" : ["$400/month for 1-yr", "$450/month"],
    "Value" : [500, 550]
}

function populateBilling(planName) {
    //RESET BILLING PERIOD OPTIONS AND TOTALS
    select.options.length = 1;
    document.getElementById('payment-total').innerText = 0 + additional;
    document.getElementById('payment-rebill').innerText = 0 + additional;
    //FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
    if (planName == "basic") {
         for (i = 0; i < basic.Option.length; i++){
             var temp = document.createElement('option');
             temp.value = basic.Value[i];
             temp.text = basic.Option[i];
             select.appendChild(temp);
         }

    } else if (planName == "prime") {
        for (i = 0; i < basic.Option.length; i++){
            var temp = document.createElement('option');
            temp.value = prime.Value[i];
            temp.text = prime.Option[i];
            select.appendChild(temp);
         }
    } else {
         for (i = 0; i < basic.Option.length; i++){
             var temp = document.createElement('option');
             temp.value = gold.Value[i];
             temp.text = gold.Option[i];
             select.appendChild(temp);
         }
    }
 }

 function enableBilling(e) {
      document.getElementById('billing-price').disabled=false;
      var planName = plan.options[plan.selectedIndex].text.toLowerCase();
      populateBilling(planName);

}

function updatePrice(e) {
      price = parseFloat(select.value);
      fix = 100;
      if (price == select.options[2].value){
          term = 1;
      }
      document.getElementById('payment-total').innerText = price + additional;
      if (price == select.options[2].value){
      document.getElementById('payment-rebill').innerText = 0 + additional;
      } else {
      document.getElementById('payment-rebill').innerText = price + additional - fix;
      }
}

Here it is in a jsfiddle: http://jsfiddle.net/aGq9q/13/

Same advice as @jfriend00, with an addition: if the user selects the "Choose Plan" placeholder, disable the billing period selector.

function populateBilling(planName) {
    //RESET BILLING PERIOD OPTIONS
    select.options.length = 1;
    document.getElementById('payment-total').innerText = 0 + additional;
    document.getElementById('payment-rebill').innerText = 0 + additional;
    //FILL BILLING PERIOD WITH PLAN OPTIONS AND VALUES
    var selectedPlan = plans[planName];
    if (selectedPlan) {
        for (var i = 0; i < selectedPlan.Option.length; i++) {
            var temp = document.createElement('option');
            temp.value = selectedPlan.Value[i];
            temp.text = selectedPlan.Option[i];
            select.appendChild(temp);
        }
    } else {
        document.getElementById('billing-price').disabled = true; // ← This
    }
}

Parsing the selection text to get a JavaScript identifier is sketchy.  I'd make the value of each option explicit.

<option value="basic">Basic</option>

etc.

function enableBilling(e) {
        document.getElementById('billing-price').disabled=false;
        var planName = plan.options[plan.selectedIndex].value; // ← This
        populateBilling(planName);

}


Generating a random string of characters and symbols After coding this, I was wondering if there are any ways in which I can improve upon it. It generates a random string of characters and symbols the same length as what is entered by the user. It uses a separate list for each char list but I am sure it is using a very long-winded method!

import random, sys
letters = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z"]
lettersnum = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z","0","1","2","3","4","5","6","7","8","9"]
letterssym = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z","#","*", "£", "$", "+", "-", "."]
lettersnumsym = ["a","b","c","d","e","f","g","h","i","j","k","l","m","n","o","p","q","r","s","t","u","v","w","x","y","z","0","1","2","3","4","5","6","7","8","9","#", "*", "£", "$", "+", "-", "."]

def generator(length, num, sym, caps):
    count = 0
    password = ""
    try:
        test = int(length)
    except ValueError:
        error("LENGTH NOT VALID NUMBER")
    if num.lower() == "yes" and sym.lower() == "yes":
        merge = lettersnumsym
    elif num.lower() == "yes":
        merge = lettersnum
    elif sym.lower() == "yes":
        merge = letterssym
    while count <= int(length):
        password = password + merge[random.randint(0, len(merge) - 1)]
        count = count + 1
    if caps.lower() == "uppercase":
        password = password.upper()
    elif caps.lower() == "lowercase":
        password = password.lower()
    print("PASSWORD:",password)

def error(error):
    print(error)
    sys.exit(0)

running = True
while running == True:
    length = input("How long do you want the password?")
    numbers = input("Do You Want It to Include Numbers?")
    symbols = input("Do You Want It To Include Symbols?")
    capital = input("Do You Want It To be uppercase or lowercase??")
    generator(length, numbers, symbols, capital)
    restart = input("Do You Want Restart?")
    restart = restart.lower()
    if restart in ("yes", "y", "ok", "sure", ""):
        print("Restarting\n----------------------------------")
    else:
        print("closing Down")
        running = False

Python already defines a number of strings of possible characters.  See string.ascii_lowercase and string.digits Source

I would use True and False instead of "yes"/"uppercase" as arguments to generator(). This function might be used by code that does directly interact with a user, so passing a string would not make sense. Additionally, the restart prompt supports a number of positive responses that are not supported by this function. You should have one layer that controls interaction with the user and one that generates a password. This will make the function cleaner as well as an easier API to work with.

Two more point about separation of concerns with generator():

It should return the password instead of printing it. Again, this allows the function to be used when not directly interacting with a command prompt.
It should throw an exception instance of calling sys.exit().  exit() will stop the python process and not allow any more execution of code. Your code is written so that multiple passwords can be generated one after another. However, if the user accidentally enters an invalid character to the first question, the application stops running instead asking the user to input a correct value. Throwing an exception would have the same result if you don't change the rest of your code, but allows you to change the code that interacts with the user to handle this case without restarting the application.

You convert length to an integer repeatedly instead of storing the value. The first validation completely ignores the result and the while loop does the conversion every time it tests if it should continue looping. This can all be solved by having length be passed in as an integer and letting the user layer handle the conversion and error cases.

Generator is already a well defined term with in Python that do something very different. The function should be renamed to generate_password() or something similar.


Repetitive days of the week code Could someone help me shorten this code, please?

while cd == "monday":
    if cp == "0":
        for v in mon.values():
            print(v)
            break
        break
    elif cp == "1":
        print(mon[1])
        break
    elif cp == "2":
        print(mon[2])
        break
    elif cp == "3":
        print(mon[3])
        break
    elif cp == "4":
        print(mon[4])
        break
    elif cp == "5":
        print(mon[5])
        break
    cd = False

while cd == "tuesday":
    if cp == "0":
        for v in tues.values():
            print(v)
            break
        break
    elif cp == "1":
        print(tues[1])
        break
    elif cp == "2":
        print(tues[2])
        break
    elif cp == "3":
        print(tues[3])
        break
    elif cp == "4":
        print(tues[4])
        break
    elif cp == "5":
        print(tues[5])
        break
    cd = False

You could simplify your code to look something like this (warning: I didn't test my code, so you should double-check to make sure it functions identically to the original)

days_map = {
    "monday": mon,
    "tuesday": tues
}

day = days_map[cd]
if cp == "0":
    print(day.values()[0])
else:
    print(day[int(cp)])

Several notes:

In your code, you use a while loop, but in every case, immediately break out of it, either by literally using break or by making the conditional false. What you want instead is an "if" statement. However, because the "if" statements are almost identical, you can simplify it completely by replacing it with a call to a dict.

(In case you aren't familiar with what dicts are, they're really nifty data structures that make life insanely easier. As you can see, they helped turn a large chunk of code into just 10 or so lines! Here's a link to CodeAcademy which gives an introduction to them))

I'm assuming that mon and tues are dicts of some kind -- I made another dict (days_map) to refer to them. This is a common technique in Python -- whenever you have a large amount of if and elif statements, check to see if you can somehow fold things into a dictionary so you can need to make only a single call.

Your for loop is also redundant. If you have a break immediately after, then it's equivalent to simply printing the first value.


Implementation of Mahalanobis distances using Eigen I'm trying to learn C++ with Eigen. Here's my attempt at computing
Mahalanobis distances of a set of points x with respect to a sub-matrix xs.

The aim of the project is to turn an R code describing a statistical procedure in C++ (and in the process to learn a bit about numerical computing in c++).
Some of these functions will have to be called literally 10,000 of times so the efficiency part is a bit important.

Is this an efficient implementation of what I'm trying to do in Eigen (the code does what I want)? Can it be improved? What are the beginner's mistake that I'm doing? (in the application I have in mind, x is an imported matrix, so 30 and 3 are only known at runtime)

#include <iostream>
#include <Eigen/Dense>
#include <fstream>
#include <Eigen/Cholesky>

using namespace Eigen;
using namespace std;

VectorXd mah(MatrixXd& x, MatrixXd& xs){
    int ps = xs.cols(), ns=xs.rows();
    RowVectorXd xs_mean=xs.colwise().sum()/ns;
    MatrixXd xs_cen = (xs.rowwise()-xs_mean);
    MatrixXd x_cen = (x.rowwise()-xs_mean);
    MatrixXd w = xs_cen.transpose()*xs_cen/(ns-1);
    MatrixXd b = MatrixXd::Identity(ps,ps);
    w.ldlt().solveInPlace(b);
    x_cen = (x_cen*b).cwiseProduct(x_cen);
    return x_cen.rowwise().sum();
}

MatrixXd sub(MatrixXd&  x, VectorXi& s){
    int p = x.cols();
    MatrixXd xs(p+1,p);
    for(int i=0;i<s.size();i++){
        xs.row(i)=x.row(s(i));
    }
    return xs;
}

int main(){
    MatrixXd x = MatrixXd::Random(30,3);
    VectorXi s = (VectorXd::Random(p+1)*n).cast<int>();
    MatrixXd xs = sub(x,s);
    ofstream file("test1.txt");
    if (file.is_open()){
            file <<  x << '\n';
    }
    cout << "m" << endl <<  mah(x,xs) << endl;
    cout << "s" << endl <<  s << endl;
    return 0;
}

Is it effecient

Imposable to tell as we have no idea how stuff in Eigen is implemented.

Comments on code:

There should be some logic in the ordering of includes. So you can easily find ones that have been included or spot missing ones (or know where to put knew ones).

#include <iostream>
#include <Eigen/Dense>
#include <fstream>
#include <Eigen/Cholesky>

This set seems to be completely random. Personally (though the exact orders does not matter and you can pick your own scheme) I put them in groups (so in this case there would be an Eigen group and a standard lib group). Then I order the groups most specific to most general. Within the groups I am less pedantic but usually it is alphabetical unless there is some good reason to use another scheme.

#include <Eigen/Cholesky>
#include <Eigen/Dense>

#include <iostream>
#include <fstream>     // I usually group all the stream stuff together
                       // Then all containers then all the others in the standard lib.
                       //

Avoid the using-directive. In non trivial programs it causes more problems than it is worth. For short programs the extra cost of prefixing objects (in terms of typing) is negligible so prefix your types and objects (don't be lazy). (I always use std::cout not cout). If you feel this is too much work then selectively bring types/objects into the current namespace with a using statement.

using namespace Eigen;
using namespace std;

Prefer to prefix types/objects with their namespace
If you must selectively bring types/objects into current namespace

using std::cout;
using std::endl;

If you must do this then scope the using to minimize polution.

Try to not use using namespace X;

Does x change in the following code? If not then pass by const reference to indicate that it is not modified.

VectorXd mah(MatrixXd& x){

One variable per line:

 int p = x.cols(), n=x.rows();

This is a nasty trick to temporarily comment out blocks.

/*  ofstream file("test1.txt");
    if (file.is_open()){
            file <<  m << '\n';
    }
/**/

It is so easily broken that I would avoid it.
If you want to comment out a block then use #if 0

// To uncomment block just replace 0 with 1.
#if 0
    ofstream file("test1.txt");
    if (file.is_open()){
            file <<  m << '\n';
    }
#endif


(Ab)using scanl to generate a random number stream, is there a better way? Here is my code to do it:

import Random (randomR, Random, StdGen)

generateRandoms :: (Random a, Num a) => StdGen -> a -> a -> [b] -> [a]
generateRandoms gen min max = map fst . tail . scanl f (0, gen)
    where f (x, g) _ =  randomR (min, max) g

The problem I have with this is that I need to pass a list to it e.g., via:

generateRandoms initialgen min max [1..10]

The values of this list end up being ignored and are just there to drive the scanl function.

I'm wondering if there is  a better way to do this (e.g., is there something like scanl that doesn't need a list to make it generate values?

If you just need to generate a stream of random numbers, you can use standard "randomRs" function from System.Random:

randomRs :: (RandomGen g, Random a) => (a, a) -> g -> [a]

Usage:

Prelude System.Random> newStdGen >>= print . take 5 . randomRs (1,10)
[2,10,10,3,5]

If you want to implement it by yourself, it can be done with unfoldr function instead of scanl. unfoldr allows to (surprise!) unfold a list from the seed value. Exactly what you need! Let's examine its type:

unfoldr :: (b -> Maybe (a, b)) -> b -> [a]

First argument is unfolding function and the second argument is the seed value. Unfolding function takes current seed value and produces Nothing or Just (a, b). Nothing means we should stop generating the list and Just (a, b) produces a new value of type "a" which is added to the genererated list and a new value of seed.

Since we want to produce a stream of random values, we never return Nothing from our unfolding function. Here is the code:

generateRandoms2 :: (RandomGen g, Random a) => a -> a -> g -> [a]
generateRandoms2 min max = unfoldr (Just . randomR (min, max))

randomR takes a range for generation and (implicitly here!) a generator and returns a random value and a new value of the generator. Informally:

randomR (1, 2) :: (RandomGen g) => g -> (a, g)

That's exactly what a typical unfolding function needs to do! So we just wrap this in Just and we are done.

Usage:

*Random Data.List> newStdGen >>= print . take 10 . generateRandoms2 1 10
[10,6,2,7,3,7,5,9,4,5]

In fact, we don't use the full power of unfoldr here (since we do not need to distinguish whether we should finish the generation or not), so we can even write a simpler recursive function:

generateRandoms3 :: (RandomGen g, Random a) => a -> a -> g -> [a]
generateRandoms3 min max g = x : generateRandoms3 min max g'
   where (x, g') = randomR (min, max) g

Here we just produce random number one by one in a lazy fashion.

Hope this helps!


Searching an element in an alternately sorted array I had to write some code that receives a number 'x' and an alternately sorted array (an array with all even indexes sorted upwards and all odd indexes sorted downwards) and checks if 'x' exists in the array. If it exists, the method will return the index, if not, the method will return -1.

If there is more than one 'x' in the array, I can choose which one to return.
An array example: {-3, 10, 0, 9, 2, 5, 3, 0, 6, -5}

I've written the following code, and I will like to hear what you think, how I can improve it, and if you think of some cases when it won't work:

public static int find(int[] a, int x) {
    int topEven = a.length - 1, topOdd = a.length - 1, bottomEven = 0, midEven = 0, bottomOdd = 1, midOdd = 0;
    if (a.length == 0) {
        return -1;
    } else if (a[0] == x) {
        return 0;
    } else if (a[topOdd] == x) {
        return topOdd;
    }
    while (topEven > bottomEven) {
        midEven = ((topEven + bottomEven) / 2);
        if (midEven % 2 != 0) {
            midEven++;
        }
        if (x > a[midEven]) {
            bottomEven = midEven + 1;
        } else if (x < a[midEven]) {
            topEven = midEven - 1;
        } else
            return midEven;
    }
    while (topOdd > bottomOdd) {
        midOdd = ((topOdd + bottomOdd) / 2);
        if (midOdd % 2 == 0) {
            midOdd++;
        }
        if (x > a[midOdd]) {
            topOdd = midOdd - 1;
        } else if (x < a[midOdd]) {
            bottomOdd = midOdd + 1;
        } else
            return midOdd;
    }
    return -1;
}

Thank you!

How about an alternate approach: instead of adapting the binary-search algorithm to work with all kinds of alternate sorted collections, how about adapting the collections to be able to run classic binary search on them.

This enables one to use production ready searching methods without reinventing the wheel and risking bugs. Even if one decides to rewrite binary search as an exercise it is still easier as it doesn't have to take the alternating indexes into account.

public static int findInAlternateSortedArray(int[] haystack, int needle) {
    if (haystack.length > 0) {
        List<Integer> sortedAscending = new EverySecondElement(haystack, 0);
        int index = Collections.binarySearch(sortedAscending, needle);
        if (index >= 0) return index * 2;
    }
    if (haystack.length > 1) {
        List<Integer> sortedDescending = new EverySecondElement(haystack, 1);
        int index = Collections.binarySearch(sortedDescending, needle, Comparator.reverseOrder());
        if (index >= 0) return index * 2 + 1;
    }
    return -1;
}

All that is needed to be able to use the standard library provided Collections.binarySearch is to provide two views into the original array, as if he had two independently sorted collections.

Given an index into each of these lists, we can easily calculate the index from the original array from where to fetch an element in \$\mathcal{O}(1)\$ time, thus the runtime of the entire search is still \$\mathcal{O}(\log N)\$.

static class EverySecondElement extends AbstractList<Integer> {
    private int[] array;
    private int offset;

    EverySecondElement(int[] array, int offset) {
        this.array = array;
        this.offset = offset;
    }

    @Override
    public int size() {
        return (array.length - offset + 1) / 2;
    }

    @Override
    public Integer get(int index) {
        return array[offset + index * 2];
    }
}


Removing trailing whitespace from input lines I'm going through the K&R C book (coming from Python) and am on exercise 1-18, which states:

  Write a program to remove trailing blanks and tabs from each line of input, and to delete entirely blank lines.

I wasn't able to find another solution online that looks quite like mine so I am questioning whether it is truly correct, even though it appears to work on anything I throw at it. I let an ending \n be whitespace, removed it and added a general newline to each printed output, but I'm not sure if that is cheating.

If there are any bad practices (or bugs in the code) for standard C please let me know.

#include <stdio.h>

#define MAXLINE 1000

int getli(char line[], int maxline);

main() {

    int len;
    int i;
    char line[MAXLINE];

    while ((len = getli(line, MAXLINE)) > 0) {
        i = len-1;

        /* replace any trailing whitespace character with null character */
        while (line[i] == '\n' || line[i] == '\t' || line[i] == ' ') {
            line[i] = '\0';
            i--;
        }

        if (line[0] == '\0') {
            printf("Blank line\n");
        }

        else printf("%s\n", line);
    }

    return 0;
}

/* function to capture input line as char array */
int getli(char s[], int lim) {

    int i, c;

    for (i=0; i<lim-1 && (c=getchar()) != EOF && c != '\n'; i++) {
        s[i] = c;
    }

    if (c == '\n') {
        s[i] = c;
        i++;
    }

    s[i] = '\0';
    return i;

}

What you've got is very "old-fashioned-looking" C code, which is only natural for someone working through K&R. :) For example, declaring main() without a return type (that is, with the implicit int return type) is frowned upon these days.

Declaring all your variables — uninitialized! — at the top of your function is also frowned upon, in every language, because uninitialized variables in general are bad. Because uninitialized variables mean they have to be initialized later, which means assignment, which means mutation, which means more stuff for me to keep track of in my head. It's better to delay defining the variable at all until you know what's going to be put in it.

For example:

int len;
int i;
char line[MAXLINE];

while ((len = getli(line, MAXLINE)) > 0) {
    i = len-1;

could definitely be written as

int len;
char line[MAXLINE];
while ((len = getli(line, MAXLINE)) > 0) {
    int i = len-1;

And then personally I'd avoid side effects inside the condition of a while loop. It's very K&R, but it's frequently not very easy to read!

while (true) {
    char line[MAXLINE];
    int len = getli(line, MAXLINE);
    if (len <= 0) break;
    int i = len-1;
    while (line[i] == '\n' || line[i] == '\t' || line[i] == ' ') {
        line[i] = '\0';
        i--;
    }

Now we can flatten that whole business with i into a simple for loop:

while (true) {
    char line[MAXLINE];
    int len = getli(line, MAXLINE);
    if (len <= 0) break;
    for (int i = len-1; (line[i] == '\n' || line[i] == '\t' || line[i] == ' '); --i) {
        line[i] = '\0';
    }

At this point I wonder whether it is in your function's contract that it zeroes out every char in the buffer after the last nonspace character, or if you could avoid writing a lot of those zeroes. One way to do that would be to re-expand the loop:

    int i = len-1;
    while (line[i] == '\n' || line[i] == '\t' || line[i] == ' ') {
        --i;
    }
    line[i+1] = '\0';

Another way would be to factor that code out into a helper function... or just use one of the many standard library functions for manipulating char buffers! Unfortunately, the function you're looking for in this case is spelled strrspn and is actually not standard, even though strspn is.

    *strrspn(line, "\n\t ") = '\0';

(I also notice that Oracle's documentation for strrspn contradicts itself. I assume that on failure it returns a pointer to the end of the string, as shown in their example code, not a pointer to the beginning of the string, as written in their prose description.)

Anyway, writing strrspn from scratch would be a good K&R-style exercise for you!

(len = getli(line, MAXLINE)) looks like a stutter. I don't know why you wouldn't write out the whole word getline... unless you're avoiding the library function, in which case something like read_one_line or get_a_line would be reasonable names to choose.

Don't write MAXLINE on that line, either. Don't give yourself a chance to misspell it or cut-and-paste-error your way into a bug. If you're calling getli with the buffer line, then the second argument should always be the size of the buffer — that is, sizeof line. Sure, that happens to be MAXLINE, just like MAXLINE happens to be 1000; but you should avoid repeating the word MAXLINE for the same reason you should avoid repeating the word 1000. If you call getli(line, sizeof line), you can see right at the callsite — without inspecting any other code — that the call is correct. It is correct by design.

Your getli function has very convoluted control flow. You have a side-effect in the condition again; and then you test for '\n' both inside and outside the loop.
Here's an exercise: Rewrite the code using no loops except while (true). Then see how much clearer you can make the code while preserving that property.

Here's the first pass:

int getli(char s[], int lim) {
    int i = 0;
    int c;
    while (true) {
        if (i >= lim - 1) break;
        c = getchar();
        if (c == EOF) break;
        if (c == '\n') break;
        s[i] = c;
        ++i;
    }
    if (c == '\n') {
        s[i] = c;
        ++i;
    }
    s[i] = '\0';
    return i;
}

Do you spot an opportunity to combine codepaths now? Keep going; you'll find that the code ends up shorter than it started out!


Timing Project Euler Problem 1 Is there a better way to time the functions and print the result and function name dynamically?

package project_euler
object Timers extends App{

  def time[R](f: => R): Unit = {
        val t0 = System.nanoTime()
        val r = f
        val t1 = System.nanoTime()

        val t = (t1-t0)/1000

    println(s"The result is: $r time taken $t ms ")
    // print also the name of the function f**

  }

   /*
    * Problem 1 project Euler
    * If we list all the natural numbers below 10 that are multiples of 3 or 5,
    * we get 3, 5, 6 and 9.
    * The sum of these multiples is 23.
    * Find the sum of all the multiples of 3 or 5 below 1000.
    * */

   //functional implementation
   def func = (0 until 1000).filter(x => x % 3 ==0  || x % 5 ==0).sum

   //imperative implementation
   def imper = {
     var i,sum = 0

     while (i < 1000) {
      if( i % 3 ==0 || i % 5 ==0)
          sum +=i

      i+=1 //increase counter
    }
    sum
  }
   time (func)
   time (imper)
  }

So, I pasted your code into my IDE and ran it in the debugger, and there isn't really any way to do exactly what you want to do. The function object doesn't appear to have any knowledge of anything other than how to call it.

Your best bet seems to be to modify the timing function to something like this:

  def time[R](f: => R, name: String): Unit = {
    val t0 = System.nanoTime()
    val r = f
    val t1 = System.nanoTime()

    val t = (t1-t0)/1000

    println(s"The result of $name is: $r time taken $t ms ")
  }

And then your call would be something like this:

   time (func, "func")

In a larger app, if you needed to pass around functions with their name, you could create a small object such as:

class Caller[R](f: => R, name: String) {
  def time(): Unit = {
    val t0 = System.nanoTime()
    val r = f
    val t1 = System.nanoTime()

    val t = (t1-t0)/1000

    println(s"The result of $name is: $r time taken $t ms ")
  }
}

Create it like so:

val a = new Caller(func, "func")

And call it like so:

a.time()


Return the longest word (or words) in a string I'm coding for a function that checks the longest word (or words) in a string:

If multiple words are of the same size, those words have to be displayed.
If a single word is inputted, that word has to be displayed.
Words should consist only of alphabets.

The following code has been tested and displays the correct input for the above conditions:

    //this method returns the longest word (or words) of a single string
    static void longestWord(string s)
    {
        int j = 0, k = 0, l = 0, tempLength = 0, maxLength = 0;
        List<int> indexList = new List<int>();
        char[] words = s.ToCharArray();
        StringBuilder longestWord = new StringBuilder();

        //a for loop to ignore spaces at the beginning of the string
        for (int i = 0; i <= s.Length - 1; i++)
        {
            if (((words[i] >= 'A' && words[i] <= 'Z') || (words[i] >= 'a' && words[i] <= 'z')))
            {
                j = i;
                l = i;
                break;
            }
        }

        //a for loop to get the maximum length of the longest word
        for (; j <= s.Length - 1; j++)
        {
            if (((words[j] >= 'A' && words[j] <= 'Z') || (words[j] >= 'a' && words[j] <= 'z')))
            {
                tempLength++;

                if (j == (s.Length - 1))
                {
                    if (tempLength >= maxLength)
                    {
                        maxLength = tempLength;  //this ensures the maximum length of a single word is captured
                                                 //removing this will not display single words without non-alphabets
                    }
                }
            }
            else
            {
                if (tempLength >= maxLength)
                {
                    maxLength = tempLength;
                }
                tempLength = 0;
            }
        }

        tempLength = 0;

        //a for loop to add the longest words into a list
        for (; l <= s.Length - 1; l++)
        {
            if (((words[l] >= 'A' && words[l] <= 'Z') || (words[l] >= 'a' && words[l] <= 'z')))
            {
                tempLength++;

                if (l == (s.Length - 1))
                {
                    if (tempLength >= maxLength)
                    {
                        indexList.Add(l+1); //'l+1' ensures a word that has no non-alphabet after it is captured
                                            //removing l+1 displays as 'wanted remov' - 'e' is missing
                        k++;
                    }
                }
            }
            else
            {
                if (tempLength == maxLength)
                {
                    indexList.Add(l);
                    k++;
                }
                tempLength = 0;
            }
        }

        //a for loop to append the longest words into a single word
        for (int m = 0; m <= k - 1; m++)
        {
            for (int n = (indexList[m] - maxLength); n < indexList[m]; n++)
            {
                longestWord.Append(words[n]);
            }
            longestWord.Append(" ");
        }

        Console.WriteLine(longestWord);
    }

Honestly, although this code works, I'm unsatisfied with it and would like suggestions as to how I could improve it lengthwise and in readability.

Why do you have more parenthesis than needed in this statement?

if (((words[j] >= 'A' && words[j] <= 'Z') || (words[j] >= 'a' && words[j] <= 'z')))

if ((words[j] >= 'A' && words[j] <= 'Z') || (words[j] >= 'a' && words[j] <= 'z'))

Similarly:

if (l == (s.Length - 1))

Should be written as:

if (l == s.Length - 1)

As well:

for (int n = (indexList[m] - maxLength); n < indexList[m]; n++)

Should be:

for (int n = indexList[m] - maxLength; n < indexList[m]; n++)

Using j, k and l as a variable in this manner is frowned upon. These should only ever be used with local iterators. (I.e. where you have i.)

Some of your names are not idiomatic.

char[] words = s.ToCharArray();

Should be:

char[] characters = s.ToCharArray();

The string s parameter should be string input (et. al.).

You can take some of your operations to functions, delegates, Lambdas, etc.

if (((words[l] >= 'A' && words[l] <= 'Z') || (words[l] >= 'a' && words[l] <= 'z')))

Can more easily be written as a delegate, function, etc.

if (IsAlphaChar(words[l])

Use a foreach loop in place of:

for (int m = 0; m <= k - 1; m++)

Should be:

foreach (int index in indexList)

Then just use index in place of indexList[m].

Never do this:

int j = 0, k = 0, l = 0, tempLength = 0, maxLength = 0;

That is a readability nightmare. Break each variable to it's own line. (And don't declare all of them in one spot. Declare them when you need them.)

You shouldn't declare iterators outside a for loop. (I'm looking at you, j and l.)

Doing words[i] four times is slower than char currentCharacter = words[i];. (Of course, the location that is used should be a method instead.)

You don't use j or l after their respective loops, so don't define them as the first thing in the method. Instead, define them as:

for (int j = i; j <= s.Length; j++)
for (int l = i; l <= s.Length; l++)


GCD of two numbers in Ruby I am trying to implement a algorithm for the GCD of two numbers in a functional approach. Can this be improved further, also with regards to performance?

def gcd(a, b)
   return b if a ==0
   return a if b == 0
   return b if (a%b == 0)
   return a if (b%a == 0)
  (a > b)? gcd(a, a%b) : gcd(b%a, b)
end

It does not matter whether a > b or b > a, two cases should be enough for positive integers:

def gcd(a, b)
  b == 0 ? a : gcd(b, a.modulo(b))
end

Check also Integer#gcd.


Splitting a Multi-FASTA file into individual files I have written a program to split a Multi-FASTA file into individual fasta files.

use strict;
use autodie;

open( my $IN, '<', $ARGV[0] );

my ( $id, $seq_obj );

sub dump_seq {

    my $sid = shift;
    my $seq = shift;

    my @parts = split( /\h+/, $sid );
    my $f_name = $parts[0];

    $f_name =~ s/\|/_/g;
    $f_name =~ s/^>//;
    $f_name =~ m/\_$/
        ? ( $f_name =~ s/.$/\.fasta/ )
        : ( $f_name .= ".fasta" );  # Controlling extension
    my @lines = $seq =~ m/.{1,60}/g; # controlling line length

    open( my $out, '>', $f_name );

    print {$out} "$sid\n";
    print {$out} "$_\n" for (@lines);

    return 0;

}

while (<$IN>) {

    chomp;

    if (/^>/) {

        if ( $. > 1 ) { # Else there would be no-output for first fasta
            dump_seq( $id, $seq_obj );
        }

        $id = $_;
        next;
    }

    $seq_obj = $seq_obj . $_;

    dump_seq( $id, $seq_obj ) if (eof); # For last fasta only
}

In this code I made calls to dump_seq() two times, I want to minimize it to one and place the call appropriately.

I can also do it by storing all the $id and $seq_obj in a hash and call the function once, which takes up lot of memory, which I don't want.

Sample Input:

>gi|129367|sp|P13481.1|P53_CHLAE RecName: Full=Cellular tumor antigen p53; AltName: Full=Tumor suppressor p53 [Chlorocebus aethiops]
MEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEAAPHMAPTPAAP
KPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPAGSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSD
>gi|635087583|ref|XP_008008385.1| PREDICTED: cellular tumor antigen p53 isoform X2 [Chlorocebus sabaeus]
MEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEAAPPMAPTPAAP
SCMGGMNRRPILTIITLEDSSGNLLGRNSFEVRVCACPGRDRRTEEENFRKKGEPCHELPPGSTKRALPNNTSSSPQPKK
KPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPAGSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSD
>gi|402898625|ref|XP_003912321.1| PREDICTED: cellular tumor antigen p53 [Papio anubis]
MEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEATPPMAPTPAAP
KPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPAGSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSD

There are a number of things in here which make the code somewhat hard to read. Also, there's a big bug.

Bug

You never reset the $seq_obj variable. As a result, each time you add a new file you are actually adding the new data to the end of the old data too. Thus, your files get bigger, and bigger, etc.

Thus, the third sequence in your input:

>gi|402898625|ref|XP_003912321.1| PREDICTED: cellular tumor antigen p53 [Papio anubis]
MEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEATPPMAPTPAAP
KPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPAGSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSD

actually produces the output:

>gi|402898625|ref|XP_003912321.1| PREDICTED: cellular tumor antigen p53 [Papio anubis]
MEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGP
DEAPRMSEAAPHMAPTPAAPKPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPA
GSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSDMEEPQSDPSIEPPLSQETFSDLWKLLP
ENNVLSPLPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEAAPPMAPTPAAPSCMGGMN
RRPILTIITLEDSSGNLLGRNSFEVRVCACPGRDRRTEEENFRKKGEPCHELPPGSTKRA
LPNNTSSSPQPKKKPLDGEYFTLQIRGRERFEMFRELNEALELKDAQAGKEPAGSRAHSS
HLKSKKGQSTSRHKKFMFKTEGPDSDMEEPQSDPSIEPPLSQETFSDLWKLLPENNVLSP
LPSQAVDDLMLSPDDLAQWLTEDPGPDEAPRMSEATPPMAPTPAAPKPLDGEYFTLQIRG
RERFEMFRELNEALELKDAQAGKEPAGSRAHSSHLKSKKGQSTSRHKKFMFKTEGPDSD

which is the accumulation of all the previous inputs too. I have spent 20 minutes trying to figure out why my code is wrong.... but it's your code.

Better system

Additionally, the process you have 'batches' the data in one variable, and then dumps the variable when all the data is read.

If you reverse the algorithm, and open a file when the header arrives, and then write all the data (that's not a header) to that file, then the process would be simpler.

Additionally, you should use the 'diamond' magic filehandle <> for ease of use:

  The null filehandle <> is special: it can be used to emulate the
  behavior of sed and awk, and any other Unix filter program that takes
  a list of filenames, doing the same to each line of input from all of
  them. Input from <> comes either from standard input, or from each
  file listed on the command line.

So, if I break your code in to three sections. One section closes an open file, the next section starts a new file, and then a simple loop that checks to see whether it should start a new file, the code is much simpler.

Note also the general lack of global variables. Also, the 'id' line is not printed to the file when the file is created, but rather after the file is created as part of the 'normal' loop.

Finally, the process below copies the input lines unchanged from the input to the output. Your code 'reformats' the lines to 56 chars wide. This code does not do the reformat. I consider this to be a feature because of two things:

preserving input format is generally a good idea.
it saves having to chomp every line
it allows the data to be processed 1 line at a time, instead of batching it

If that does not work for you, then the code will have to be changed significantly.

#!/usr/bin/perl

use strict;
use autodie;

sub flushfile {
    my $file = shift;
    return undef unless $file;
    close $file;
    return undef;
}

sub makefile {
    my $control = shift;
    chomp $control;

    my @parts = split( /\h+/, $control, 2);
    my $f_name = $parts[0];
    $f_name =~ s/\|/_/g;
    $f_name =~ s/^>//;
    $f_name =~ s/\_$//;
    $f_name .= ".fasta";

    print "Processing $f_name from $control\n";

    open (my $ofile, '>', $f_name);
    return $ofile;
}

my $outfile;
while (<>) {

    if (/^>/) {
        $outfile = flushfile($outfile);
        $outfile = makefile($_);
    }

    $outfile or die "Unexpected input: $_";
    print {$outfile} $_;
}
flushfile($outfile);

Line Trimming version

I have written it again using a system that does the line sizing too.

The following code is different. It accumulates partial lines and the outputs them when they are full. It also needs to print the control line first because that cannot be line-trimmed.

#!/usr/bin/perl

use strict;
use autodie;

my $linesize = 60;

sub flushfile {
    my $file = shift;
    my $partial = shift;
    return undef unless $file;
    print {$file} $partial . "\n";
    close $file;
    return undef;
}

sub makefile {
    my $control = shift;
    chomp $control;

    my @parts = split( /\h+/, $control, 2);
    my $f_name = $parts[0];
    $f_name =~ s/\|/_/g;
    $f_name =~ s/^>//;
    $f_name =~ s/\_$//;
    $f_name .= ".fasta";

    print "Processing $f_name from $control\n";
    open (my $ofile, '>', $f_name);
    print {$ofile} $control . "\n";
    return $ofile;
}

sub addline {
    my $file = shift;
    my $part = shift;
    my $nxt = shift;
    $file or die "Unexpected input: $nxt";

    $part .= $nxt;
    while (length($part) >= $linesize) {
        my $front = substr ($part, 0, $linesize);
        #print "Printing $front\n";
        print {$file} ($front , "\n");
        $part = substr ($part, $linesize);
    }
    return $part;
}

my ($partial, $outfile);
$partial = "";
while (<>) {

    chomp;

    if (/^>/) {
        $outfile = flushfile($outfile, $partial);
        $outfile = makefile($_);
    } else {
        $partial = addline($outfile, $partial, $_);
    }
}
flushfile($outfile, $partial);


C++ templates for sin, cos, tan taylor series I used C++ metaprogramming to build taylor series for sin, cos, and tan (not really for tan). The whole thing is on my github. Please give any feedback related (but not limited) to style, functionality, etc.

Overall I think the biggest flaw is that the client needs to make their own struct with a static const long double member called value, and pass that struct to the "functions." I would have just passed the double, but C++ doesn't let you use floating-point numbers as template arguments. Using a struct was the only way I could think of to get around that.

Thanks!

#ifndef TRIG_HPP
#define TRIG_HPP

namespace mt
{
    const long double PI = 3.14159265358979323f;

    // FACTORIAL

    template <int N> struct fact
    {
        static const long long value = N * fact<N-1>::value;
    };

    template <> struct fact<1>
    {
        static const long long value = 1L;
    };

    // EXPONENTS

    template <class X, int N> struct power
    {
        static const long double value;
    };

    template <class X, int N> const long double power<X, N>::value = X::value * power<X, N-1>::value;

    template <class X> struct power<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double power<X, 0>::value = 1.0f;

    // SIMPLIFY RADIAN TO INTERVAL [-2π, 2π]

    template <class X> struct SimplifyRad
    {
        static const long double value;
    };

    template <class X> const long double SimplifyRad<X>::value =

        X::value - (2 * PI * (double) (int) (X::value/(2*PI))); // float modulo?

    // COSINE

    template <class X, int N> struct cos
    {
        static const long double value;
    };

    template <class X, int N> const long double cos<X, N>::value =

        (((N % 2 == 0 ? 1 : -1) * power<SimplifyRad<X>, 2*N>::value) / ((long double) fact<2*N>::value)) + cos<X, N-1>::value;

    template <class X> struct cos<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double cos<X, 0>::value = 1.0f;

    // SINE

    template <class X, int N> struct sin
    {
        static const long double value;
    };

    template <class X, int N> const long double sin<X, N>::value =

        (((N % 2 == 0 ? -1 : 1) * power<SimplifyRad<X>, 2*N-1>::value) / ((long double) fact<2*N-1>::value)) + sin<X, N-1>::value;

    template <class X> struct sin<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double sin<X, 0>::value = 0.0f;

    // TANGENT

    template <class X, int N> struct tan
    {
        static const long double value;
    };

    template <class X, int N> const long double tan<X, N>::value = sin<X, N>::value / cos<X, N>::value;
}

#endif

Example usage:

#include <cstdio>
#include "../include/trig.hpp"

#define DEPTH 8

struct var
{
    static const long double value;
};

const long double var::value = mt::PI / 4;

int main(int argc, char *argv[])
{
    printf("%Lf\n", var::value);
    printf("sin(%Lf) = %Lf\ncos(%Lf) = %Lf\ntan(%Lf) = %Lf\n",
            var::value, mt::sin<var, DEPTH>::value,
            var::value, mt::cos<var, DEPTH>::value,
            var::value, mt::tan<var, DEPTH>::value);
}

Output:

$ g++ main.cpp
$ ./a.exe
0.785398
sin(0.785398) = 0.707107
cos(0.785398) = 0.707107
tan(0.785398) = 1.000000

I see several issues with your code.

Your motivation should be stronger/clearer

While you may have a very good motivation for having this functionality, it is not apparent from your post hear nor from your GitHub repo. At compile time there are typically very few values I'd need the sine or cosine of; is it really clearer to write

printf("%f", apply_my_complicated_TMP_construct(PI/4, 8) )

than

auto sine_of_quarter_pi = 0,707106781187f;
printf("%f", sine_of_quarter_pi);

? I don't know that it is. Plus, the second way the reader doesn't need to think about iterations, the taylor series etc. It will also reduce compilation time.

Now, you could say "Ah, but I have a lot of these values to apply sin() or cos()" - but honestly, I doubt it. If that's the case you should just do that offline and independently of developing your C++ code.

In your repository, you list the following advantages:

  All the work is done a compile-time.
  The compile-time efficiency is actually going to be better than run-time efficiency compared to "normal,"

But, again, there isn't really any work; and there's no "efficiency" issue if you just use the value you need. Even if you had to calculate a sine or cosine a couple dozen times - that's really negligible.

Don't reinvent the wheel I

There's an M_PI constant available in POSIX-compliant system's <math.h> header (and thus, effectively, in most <cmath> headers, including MSVC's with a bit of work, see here). In case you want to be more portable, you might do:

#ifndef M_PI
    #define M_PI 3.14159265358979323846
#endif

... but, in fact, these days you can even get a better accuracy constant, M_PIl, on some systems. So you would have actually gained from not reinventing the wheel.

... and there's more. Remember my sqrt(2)/2 from the example above? That's often predefined too. Have a look at your system's <math.h>.

Don't reinvent the wheel II

Someone has already provided what looks like a more robust implementation of the same functionality - here on the site:

constexpr Sin Function C++ 14

Now, granted, I haven't scrutinized that code. Maybe it's buggy or has other problems. But I'll bet you 100 reputation you haven't read that implementation before writing yours... although you could have, if you'd have looked. I didn't know about it before I was writing this review.

Drop most template recursion in favor of C++14 constexpr functions

You use recursive template instantiation when you could just put a for loop in your function - and sometimes not even need a template:

constexpr long long factorial(int n)
{
    return (n <= 1) ? 1L : (n * factorial(n - 1));
}

(taken from here). Also, when you can't just "go constexpr", you can still iterate over multiple template arguments with for_each_argument and so on.


Python value comparing taking too long Is there any way to make this code run faster. I have to compare values in a text file which is written in the format:

1 1.12345

2 0.12345

3 3.45678...

The values are in billions and the code takes weeks to run. As far as I can see, the calculation (subtraction in my case) is the heavy step.

a=open("output.txt","w")

for i in range(1,num_lines+1): #num_lines are the number of lines in the file
    read=linecache.getline('file.txt',i).rstrip('\n') #reads line by line from a text file
    splitted=read.split()   #splits the string "read"

    for k in range(i+1,num_lines+1):
        read1=linecache.getline('just_nodes.txt',k).rstrip('\n')
        splitted1=read1.split()

        alpha_temp=abs(float(splitted[1])-float(splitted1[1])) #heavy step

        alpha=round(alpha_temp,3)

        if (alpha==1.000):
            a.write(str(splitted1[0]) +' '+ str(splitted[0]) + "\n")
a.close()

One observation is that you are converting the splitted[1] to float inside the inner loop - redoing the same exact thing multiple times - do it once before the loop:

splitted = float(read.split(" ", 1)[1])  # convert to float here
for k in range(i + 1, num_lines + 1):
    read1 = linecache.getline('just_nodes.txt', k).rstrip('\n')
    splitted1 = float(read1.split(" ", 1)[1])

    alpha_temp = abs(splitted - splitted1)

Note the other thing I've done - splitting by an explicit space and doing it strictly once - please check if it actually has a positive impact on performance.

You may also not strip the newlines from the end of the lines, float() can handle it:

In [1]: float("1.00\n")
Out[1]: 1.0


Custom CSRF protection in Express / Node Please review my code rather than telling me to use a trusted NPM package like csurf.

What are some potential weaknesses with the following implementation?

'use strict'

const   express = require('express'),
        router = express.Router(),
        crypt = require('../services/crypt')

router.use((req, res, next) => {
    if (req.session.csrfToken === undefined) {
        req.session.csrfToken = crypt.randHex(64) // this uses Crypto.randomBytes(); gives a hex of length 64
        // alt: res.locals.csrfToken = crypt.hash('sha256', req.session.csrfToken + req.headers.host) // also using Crypto
    }

    res.locals.csrfToken = req.session.csrfToken
}

module.exports = router

I put the res.locals.csrfToken in all forms with method="post", and this middleware is always used:

'use strict'

const   express = require('express'),
        router = express.Router()

router.post('*', (req, res, next) => {
    if (req.session.csrfToken !== req.body.csrfToken) {
    // alt: if (crypt.hash('sha256', req.session.csrfToken + req.headers.host) !== req.body.csrfToken) {
        let err = new Error('Invalid CSRF Token!')
        err.status = 403

        return next(err)
    }

    return next()
})

module.exports = router

I am using HTTPS, and my express-session options look something like this:

{
    name: 'sessid',
    resave: false,
    saveUninitialized: false,
    secret: 'djt84nq7y4tnvwosufvgsudih3rve783vdk'
}

Will the alternative (alt) implementation help any?

Not a node expert, but it looks like you cover the basics.  CSRF protection via token is standard practice and very reliable.  You are generating your token via a cryptographically secure random number generator.  You are leaving it in the session and making sure it is present on all post requests.  The only way an attacker can break it is via XSS attack, but XSS attacks are able to defeat any CSRF protection, so that isn't your fault (just make sure that you are practicing defense in depth), nor will CSRF vulnerability be your biggest problem if you have an XSS vulnerability.  Your weaknesses are the same as any CSRF implementation:

Failure to actually implement your CSRF protection on all pages.  Most common cause of CSRF vulnerabilities is either people trying non-standard protection methods (which you are not doing), or inadvertently leaving actions out of CSRF protection.
You can accidentally leave action URLs unprotected if you somehow build a URL that doesn't use your CSRF middleware or if you inadvertently allow GET requests to execute actions, in which case the CSRF protection will be explicitly ignored by your code.  So the biggest weakness is the same weakness that everyone has: you have to follow best security practices everywhere else in your code.  Forget it one place and your CSRF protection disappears.  If you have a valuable enough website someone will find the vulnerability.

In terms of improvements, the only reasonable addition would be to change your CSRF token after every request.  Many systems implement this additional layer of security.  It is not clear to me if this additional security step has any practical benefits, but if it isn't difficult to implement it certainly won't hurt to include it.  In my own implementations in the past I've used it for additional UI help: I keep track of past CSRF tokens and if I see a duplicate come in (which is usually caused by the user hitting "submit" twice on a form), I ignore the second request and warn the user of their error.

You could also consider a dual-protection using referrer and origin headers, but those have mixed results due to incomplete browser support.  Most frameworks I have seen stick with a token-only protection.

I don't think your alternate implementation adds anything.  Throwing in a sha256 hash certainly doesn't change anything, because hashing doesn't introduce any additional randomness.  It just makes things look more random.  The host header doesn't inherently add additional security, IMO.  This is very similar to an attempt to add protection via the ORIGIN or REFERRER headers.  In this particular case, I don't think a more complicated algorithm is worth the (dubious) increase in security.  If you did want to also verify such a header then I would ditch the hash: just store it in the session in plain text and check it on the way back up.  However, you probably don't have to store it in the session either: most of the time you should know what the host, origin, or referrer should be.  No need to store it to check it later.


String representation of a polynomial We all know that math notation is idiosyncratic. Canonical representation of math objects often have irregular grammar rules to improve readability. For example we write a polynomial \$3x^3 + x^2\$ instead of more uniform but more verbose \$3x^3 + 1x^2 + 0x^1 + 0x^0\$. When a coefficient equals 0, you don't write the term, if the power equals \$1\$, you simply write \$x\$, and so on. So I wrote a simple program that outputs a string representation of a polynomial, given a list of coefficients:

def enumerate2(xs, start=0, step=1):
    for x in xs:
        yield (start, x)
        start += step

def poly(xs):
    """Return string representation of a polynomial.

    >>> poly([2,1,0])
    "2x^2 + x"
    """
    res = []
    for e, x in enumerate2(xs, len(xs)-1, -1):

        variable = 'x'

        if x == 1:
            coefficient = ''
        elif x == -1:
            coefficient = '-'
        else:
            coefficient = str(x)

        if e == 1:
            power = ''
        elif e == 0:
            power = ''
            variable = ''
        else:
            power = '^' + str(e)

        if x < 0:
            coefficient = '(' + coefficient
            power = power + ')'

        if x != 0:
            res.append(coefficient + variable + power)

    return ' + '.join(res)

enumerate2 is a custom version of enumerate that supports variable step. The result looks like this:

>>> poly([2,0,3,-4,-3,2,0,1,10])
'2x^8 + 3x^6 + (-4x^5) + (-3x^4) + 2x^3 + x + 10'

How do I make this code more elegant and probably more generic? Oh, and the result is sub-optimal, as negative terms are enclosed in brackets, instead of changing the preceding plus sign to minus.

Your enumerate2 is a nice touch but I am not quite convinced that this is necessary : if you are to play with the length manually, you might as well compute the power from the index manually.

Also, if you were to handle the negative with a minus instead of the plus, you'd be able to get rid of the brackets. On the other hand, you cannot use join anymore which is a bit of a pain because it is a cool and efficient function.

Anyway, here's my try :

def poly(p, var_string='x'):
    res = ''
    first_pow = len(p) - 1
    for i, coef in enumerate(p):
        power = first_pow - i

        if coef:
            if coef < 0:
                sign, coef = (' - ' if res else '- '), -coef
            elif coef > 0: # must be true
                sign = (' + ' if res else '')

            str_coef = '' if coef == 1 and power != 0 else str(coef)

            if power == 0:
                str_power = ''
            elif power == 1:
                str_power = var_string
            else:
                str_power = var_string + '^' + str(power)

            res += sign + str_coef + str_power
    return res

and the corresponding output :

2x^8 + 3x^6 - 4x^5 - 3x^4 + 2x^3 + x + 10

Bug found

As I was looking at my original implementation, I found a bug which happens to be in yours too : try with [1,1,1,1,1].


Toggling menus based on an ID I have recently started using browserify and have created a custom module to toggle two menus based on the ID clicked and apply a transparent overlay effect behind the menu. I have got it working but just wanted to get some feedback on weather it can improved performance or coding wise as I am wanting to improve my JS skills.

/**
* Require modules
*/
var dullBackground = require('./dullBackground');

/**
* Export module
*/
module.exports = function() {

  'use strict';

  /**
  * Selectors
  */
  var menu = document.getElementById('nav-main');
  var panel = document.getElementById('overlay-panel');
  var productsPanel = document.getElementById('products-panel');
  var systemsPanel = document.getElementById('systems-panel');
  var close = document.getElementById('overlay-close');
  var activeClass = 'active';
  var hideClass = 'hide';

  /**
  * Methods
  */
  function togglePanel(event) {

    // Selectors
    var elementId = event.target.id;

    event.preventDefault();

    // Conditions
    if(elementId === 'page-products' || elementId === 'page-systems') {
      dullBackground();
      panel.classList.add(activeClass);
    }
    if(elementId === 'page-products') {
      productsPanel.classList.remove(hideClass);
    }
    if(elementId === 'page-systems') {
      systemsPanel.classList.remove(hideClass);
    }

  }
  function closePanel(event) {

    // Actions
    panel.classList.remove(activeClass);
    event.preventDefault();

    // Conditions
    if(!productsPanel.classList.contains(hideClass)) {
      productsPanel.classList.add(hideClass);
    }
    if(!systemsPanel.classList.contains(hideClass)) {
      systemsPanel.classList.add(hideClass);
    }

  }

  /**
  * Events/APIs/init
  */
  menu.addEventListener('click', togglePanel, false);
  close.addEventListener('click', closePanel, false);

};

First time doing a review, so bear with me.

I've been thinking about how to make any changes to your code, but had a difficult time finding things that could be optimized. Overall, I think you did a great job.

Selectors - this may be a style preference, but I tend to lump everything under one var statement (delimited by commas), rather than having to write var each time.

Methods - I chose to pass the elementId into document.getElementById rather than having to write additional if statements to check specifically for either id. You're already checking to see what event.target.id is returning, might as well use it again.

Methods - closePanel, I created a function called checkClassListAndAddClass very long, but at least you know what it's doing. This function takes the selector as an argument, checks to see if the selector contains a class, and then returns the add class. I added a variable to that function to help remove some repetitive code.

Again, this is my first stab at a code review, so I'm happy to hear your feedback as well as any other feedback from the community.

/**
* Require modules
*/
var dullBackground = require('./dullBackground');

module.exports = function () {

    'use strict';

    /**
    * Selectors
    */
    var menu = document.getElementById('nav-main'),
        overlay = document.getElementById('overlay-panel'),
        close = document.getElementById('overlay-close'),
        productsPanel = document.getElementById('page-products'),
        systemsPanel = document.getElementById('page-systems'),
        activeClass = 'active',
        hideClass = 'hide';

    /**
    * Methods
    */
    function togglePanel(event) {
        var elementId = event.target.id;

        event.preventDefault();

        if (elementId === ('page-products' || elementId === 'page-systems')) {
            dullBackground();
            overlay.classList.add(activeClass);
            document.getElementById(elementId).classList.remove(hideClass);
        }
    }

    function closePanel(event) {
        event.preventDefault();
        overlay.classList.remove(activeClass);

        checkClassListAndAddClass(productsPanel);
        checkClassListAndAddClass(systemsPanel);
    }

    function checkClassListAndAddClass(selector) {
        var selClassList = selector.classList;

        if (!selClassList.contains(hideClass)) {
            return selClassList.add(hideClass);
        }
    }

    /**
    * Events/APIs/init
    */
    menu.addEventListener('click', togglePanel, false);
    close.addEventListener('click', closePanel, false);

};


Project Euler #2 - Even Fibonacci numbers Given:

Each new term in the Fibonacci sequence is generated by adding the previous two terms. By starting with 1 and 2, the first 10 terms will be:
1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ...
By considering the terms in the Fibonacci sequence whose values do not exceed four million, find the sum of the even-valued terms.

MAX_LIMIT = 4000000 # Four million inclusive
FIB_CACHE = {}

def fib(n):
    """ Calculates nth fibonacci term.
    Args:
        n: The nth term.
    Returns:
        The nth fibonacci term of the series.
    """
    if (2 >= n):
        return n
    if (n not in FIB_CACHE):
        FIB_CACHE[n] = fib(n-1) + fib(n-2) # linear recurrence
    return FIB_CACHE[n]

def sum_fib():
    sum = 0
    i = 1
    ith_fib = 0
    while (1):
        ith_fib = fib(i)
        if (ith_fib > MAX_LIMIT):
            return sum
        if (ith_fib % 2 == 0):
            sum += ith_fib
        i += 1
    return sum

print fib(1)
print fib(2)
print fib(10)
print sum_fib()

Note: Without caching, it was taking too long to complete since it's an exponential algorithm.
Few important points:

I read somewhere about putting fewer changing values on the left. Is this true?
I am always confused about when to use an if-if branch and anif-else branch. Is there any heuristic? For example, could I write cache-checking code in the else branch too?

Going from top to bottom:

MAX_LIMIT = 4000000 # Four million inclusive

That sounds like a function parameter, not a global constant:

def sum_fib(max_limit=4000000):
    ...

Now, you have this function:

def fib(n):
    ...

But you don't really need it. You never need the nth Fibonacci number here, out of the blue. You're always going in order. If you're going in order, you can just keep them as local variables and update both:

cur, prev = cur+prev, cur

And then:

while (1):

Typically we'd say while True:, but in this case, we have a clearly established termination condition: our max limit. So let's use it:

def sum_fib(max_limit=4000000):
    cur, prev = 1, 0
    total = 0
    while cur < max_limit:
        if cur % 2 == 0:
            total += cur
        cur, prev = cur+prev, cur

    return total

We could even generalize this more by providing a predicate:

def sum_fib(max_limit=4000000, pred = lambda x: x%2 == 0):
    cur, prev = 1, 0
    total = 0
    while cur < max_limit:
        if pred(cur):
            total += cur
        cur, prev = cur+prev, cur

    return total

Now back to optimizations. What are the Fibonacci numbers? They are:

1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 ...

What are the even Fibonacci numbers? They are:

2, 8, 34, 144, 610

Notice that it's every 3rd Fibonacci number. What can we do with that? That's still a Fibonacci-like sequence. Except instead of an = an-1 + an-2, we have an = 4an-1 + an-2:

def sum_fib(max_limit=4000000):
    cur, prev = 2, 0
    total = 0
    while cur < max_limit:
        total += cur
        cur, prev = 4*cur+prev, cur

    return total


Making the objectContext.CommandTimeout configurable? I am trying to make the CommandTimeout value more configurable.  I'm wondering if this is best practice to do so?

I add the following to my <appsettings> node in my Web.config file:

<add key="commandTimeValue" value ="60"/>

The I added the following to the top of my controllers under the :

using (var db = new myDAL())...

Int32 timeoutVal = Convert.ToInt32

(System.Web.Configuration.WebConfigurationManager.AppSettings["commandTimeValue"]);
((IObjectContextAdapter)db).ObjectContext.CommandTimeout = timeoutVal;

Is this the right way to do this?  What are some other alternatives?

I do not like the name myDAL as a class name.  It is very cryptic and does not follow C# naming conventions.  I would prefer DataAccessLayer or <entity>DataAccessLayer where <entity> is the name of the class the DAL is for.

That being said, why don't you inject the timeout value into your DAL class.  The constructor could be something like

public void myDAL(int timeout)
{
    if (timeout > 0)
    {
         ((IObjectContextAdapter)this).ObjectContext.CommandTimeout = timeout;
    }

    // other constructor commands
}


Which mouseover handler is better? I'm currently learning some jQuery and I am stuck on something. On a small exercise I did, I was asked to perform an action when you mouseover a div class.  I gave a different solution than the one suggested and I was wondering which one is written best and why.

HTML code

   <input class='data' type='text' name='firstname' value='Firstname 1'>
   <input class='data' type='text' name='lastname' value='Lastname 1'>

Here is my solution:

$(document).on('mouseover', '.success', function() {
    $(this).removeClass('success').addClass('delete');
});

And here is his solution:

$(document).on('mouseover', '.deletecell', function() {
    $('.success', this).addClass('delete').removeClass('success');
});

PS: Does it matter if addClass is first and removeClass is second?

Yours is simpler and therefore better.  However, I would simplify it further as:

$('.success').on('mouseover', function() {
    $(this).removeClass('success').addClass('delete');
});

Is shouldn't matter much whether you call .addClass() first or .removeClass() first.


Check if the user exists Scenario:

Check if the user exists

If yes: update the details
Else: insert the user details

public bool AddUsers(IEnumerable<UserDetail> users)
    {
        using (var dbTransaction = System.Data.Entity.Database.DefaultConnectionFactory.CreateConnection("MyCon").BeginTransaction())
        {
            using (var entities = new MyDbEntities())
            {
                try
                {
                    foreach (var user in users)
                    {
                        // LINQ under observation
                        var userId = entities.tUsers.Where(u => u.UserName == user.UserName).Select(x => x.UserUID).FirstOrDefault();

                        if (userId > 0)
                        {
                            //update

                        }
                        else
                        {
                             //insert
                        }

                    }
                    dbTransaction.Commit();
                    return true;
                }
                catch (Exception ex)
                {
                    dbTransaction.Rollback();
                    clsErrorHandling.WriteLog("ImportUsersHandler", "AddUsers", ex.Message.ToString());
                    return false;
                }
                finally
                {
                    dbTransaction.Dispose();
                }
            }
        }
    }

Please find the comment //LINQ under observation.

Is there a better way to find the user?

Because if I do this way:-

 tblUser newUser = new tblUser() { Username = user.Username } ;

If the user exists then no issue then in case not then how do I check whether I need to do insert or update.

using (var dbTransaction = System.Data.Entity.Database.DefaultConnectionFactory.CreateConnection("MyCon").BeginTransaction())
// ...
finally
{
    dbTransaction.Dispose();
}

You're already using the transaction in a using statement which is shorthand for try/finally, where the finally block disposes the IDisposable. So you can remove your manual finally block completely, otherwise you're calling Dispose on the transaction twice.

  clsErrorHandling.WriteLog("ImportUsersHandler", "AddUsers", ex.Message.ToString());

ex.Message is already a string, no need to call ToString on it.


Counting the number of ways to decode a string I am working on problem where I need to decode a string:

  A message containing letters from A-Z is being encoded to numbers
  using the following mapping:

    'A' -> 1
    'B' -> 2
    ...
    'Z' -> 26

  Given a non-empty string containing only digits, determine the total
  number of ways to decode it.

  Example 1:

  Input: "12"
  Output: 2
  Explanation: It could be decoded as "AB" (1 2) or "L" (12).

  Example 2:

  Input: "226"
  Output: 3
  Explanation: It could be decoded as "BZ" (2 26), "VF" (22 6), or "BBF" (2 2 6).

I came up with the following recursive approach and it works fine. Is there any better or efficient way to solve the same problem?

  public static int decode(String data) {
    int[] memo = new int[data.length() + 1];
    return helper(data, data.length(), memo);
  }

  private static int helper(String data, int k, int[] memo) {
    if (k == 0)
      return 1;
    int s = data.length() - k;
    if (data.charAt(s) == '0')
      return 0;
    if (memo[k] != 0) {
      return memo[k];
    }

    int result = helper(data, k - 1, memo);
    if (k >= 2 && Integer.parseInt(data.substring(data.length() - k, data.length() - k + 2)) <= 26) {
      result += helper(data, k - 2, memo);
    }
    memo[k] = result;
    return result;
  }

First off, as mentioned in the comments the variable and function name(s) could be a bit more descriptive. decode is fine, but helper should be renamed to something like count_decodings. k could be len or substring_length. I disagree with the comments about data though- we know nothing about the string being passed, so the only more apt name would be lowercase string, which at first glance may get confused with the type.

Moving on to structure, you can actually revise your algorithm to do away with k, and make the substring logic simpler. Assume the string that is passed is the entire string- no offset. Then call your function recursively on a substring of the input string.

private static int helper(String data, int[] memo) {
    if (data.length() == 0)
        return 1;
    if (data.charAt(0) == '0')
        return 0;
    if (memo[data.length()] != 0) {
        return memo[data.length()];
    }

    int result = helper(data.substring(1), memo);
    if (data.length() > 1 && Integer.parseInt(data.substring(0, 2)) <= 26) {
       result += helper(data.substring(2), memo);
    }
    memo[data.length()] = result;
    return result;
}

As you can see, this results in far less index juggling, and is far easier for someone else to read. When I was writing this answer there were at least two other answers trying to figure out the correct way to juggle these indices to make the algorithm work, and both were since deleted by their authors because they were too error prone / couldn't get it working quite right. If that doesn't say something about the maintainability and readability of index juggling, I don't know what does.

Lastly, I'd like to speculate that the algorithm could possibly be sped up by minimizing the conversions between strings and integers. You could possibly replace

data.substring(0, 2)) <= 26

with

data.substring(0, 2).compareTo("26") < 1

-which would save the conversion, but at the cost of confusion and bad form (comparing numerical values as strings). Alternatively, you could convert data to an integer once, and then do floored division and modular reduction by 10 to get your digits and "substrings". These would only provide constant speedups at best, however.


Sorting and slicing array, based on another "weight" array? I have the following input array:

$arr = array(
'water' => array(0, 1, 2, 3, 4, 5),
'apple' => array(1, 5, 3, 4, 0, 0),
'beer' => array(0, 0, 0, 0, 0, 0)
);

Now I want to know which element has largest sum, so I have the following code:

$arrWeight = array();

foreach ($arr as $key => $value)
{
    $arrWeight[$key] = is_array($value) ? array_sum($value) : 0;
}

Then, I want to sort array based on weight. That's easy.

arsort($arrWeight);

Now, I want to slice the original array ($arr) based on generated weights and limit.

$arrAllowedKeys = array_slice($arrWeight, 0, self::METRICS_BREAKDOWN_LIMIT, true);

foreach ($arr as $key => $value)
{
    if (!array_key_exists($key, $arrAllowedKeys))
    {
        unset($arr[$key]);
    }
}

For example, if I have self::METRICS_BREAKDOWN_LIMIT equals to 2, I'd expect the following output:

$arr = array(
'water' => array(0, 1, 2, 3, 4, 5),
'apple' => array(1, 5, 3, 4, 0, 0),
);

So, basically, the above solution is working, but I'm looking for better approach.

Your implementation seems fine.
The only improvement I see is to not preserve the ordering when you slice.
So instead of:

$arrAllowedKeys = array_slice($arrWeight, 0, self::METRICS_BREAKDOWN_LIMIT, true);

This is all you need:

$arrAllowedKeys = array_slice($arrWeight, 0, self::METRICS_BREAKDOWN_LIMIT);

The title of your question is "sorting and slicing array",
but the input array is not sorted.
During the process, only the calculated array of weights is sorted,
the original is not.

If the input is this:

$arr = array(
'apple' => array(1, 5, 3, 4, 0, 0),
'water' => array(0, 1, 2, 3, 4, 5),
'beer' => array(0, 0, 0, 0, 0, 0)
);

Then the output will be:

$arr = array(
'apple' => array(1, 5, 3, 4, 0, 0),
'water' => array(0, 1, 2, 3, 4, 5)
);

In other words, the input array is sliced, but not sorted.

What happens to elements with the same weight, around the self::METRICS_BREAKDOWN_LIMIT index? For example, if self::METRICS_BREAKDOWN_LIMIT = 1, and the initial array is this:

$arr = array(
'water' => array(1, 2, 3),
'apple' => array(1, 2, 3),
'beer' => array(1, 2, 3)
);

Which record will be in the result? It depends on whether arsort is using a stable sorting algorithm (not so easy to find in the docs of arsort). If arsort is stable, insertion order will be preserved, and the answer will be water. Otherwise it can be any of these. It would be a good idea to add a comment in the code about this.


Busiest time in mall The mall management is trying to figure out what was the busiest
  moment in the mall in the last year. Each data entry includes a
  timestamp (seconds in Unix Epoch format), an amount of people and
  whether they entered or exited.

  Example:

{ time: 1440084737,  count: 4,  ActionType: "Entry" }

How can I refactor this code?

public class MallTimeWithMaximumPeople
{
    public class DataComparision implements Comparator<EntryData>
    {
        @Override
        public int compare(EntryData data1, EntryData data2)
        {
           return (data1.epochTime - data2.epochTime);
        }
    }

    public class EntryData
    {
       private int count, epochTime;
       private ActionType actionType;

    public EntryData(int count, int epochTime, ActionType actionType)
    {
        this.count = count;
        this.epochTime = epochTime;
        this.actionType = actionType;
    }

    public int getCount()
    {
        return count;
    }

    public long getEpochTime()
    {
        return epochTime;
    }

    public ActionType getActionType()
    {
        return actionType;
    }
  }

  public enum ActionType
  {
     ENTRY, EXIT;
  }

  public long[] getTimeWithMaximumPeople(List<EntryData> data)
  {
      long[] result = { 0, 0, 0 };

      if (data == null || data.size() == 0)
        return result;

      Collections.sort(data, new MallTimeWithMaximumPeople.DataComparision());

      int dataSize = data.size();
      int count = 0, maxCount = 0;

      for (int i = 0; i < dataSize; i++)
      {
        EntryData currentData = data.get(i);

        if (currentData.getActionType() == ActionType.ENTRY)
            count += currentData.getCount();
        else
            count -= currentData.getCount();

        if (i < dataSize - 1 && currentData.getEpochTime() == data.get(i + 1).getEpochTime())
            continue;

        if (count > maxCount)
        {
            maxCount = count;
            result[0] = currentData.getEpochTime();

            if (i < dataSize - 1)
                result[1] = data.get(i + 1).getEpochTime();
            else
                result[1] = data.get(i).getEpochTime();

            result[2] = maxCount;
        }
    }

    return result;
  }

  public static void main(String args[])
  {
      MallTimeWithMaximumPeople instance = new MallTimeWithMaximumPeople();

      List<EntryData> data = new ArrayList<EntryData>();
      EntryData data1 = instance.new EntryData(100, 0, ActionType.ENTRY);
      EntryData data3 = instance.new EntryData(1, 1, ActionType.EXIT);
      EntryData data2 = instance.new EntryData(20, 2, ActionType.EXIT);
      data.add(data1); data.add(data2); data.add(data3);

      long[] result = instance.getTimeWithMaximumPeople(data);
  }
}

Don't abuse inner classes

Both classes DataComparision (beware of the typo here), and EntryData are inner classes of MallTimeWithMaximumPeople. However, those two classes do not access the fields of their enclosing instance. The Java tutorials have:

  Use a non-static nested class (or inner class) if you require access to an enclosing instance's non-public fields and methods. Use a static nested class if you don't require this access.

In this case, consider:

Using a static nested class instead. This means declaring both classes static.
Using a top-level class for each of those.

Possible bug

The DataComparision comparator has a subtle corner-case. Currently, it relies on the return value of

data1.epochTime - data2.epochTime

to determine if data1 is greater than, equal to, or less than data2. However, consider the example case of data1.epochTime = Integer.MIN_VALUE and data2.epochTime = Integer.MAX_VALUE:

System.out.println(Integer.MIN_VALUE - Integer.MAX_VALUE); // prints 1

This actually prints a positive number due to integer overflow, so the code would consider the event data1 to be after data2. Of course, this supposes negative values are accepted, which in your case, is probably not wanted (although it is not validated).

Even if it doesn't concern you, it would be best to fix this by just calling Integer.compare instead of relying on the result of this subtraction.

Epoch time is a long

Unless you want your program to crash in 2038, store the number of seconds since the Epoch in a long, and not in an int. The maximum integer value, which is Integer.MAX_VALUE can only represent, in seconds, a date up to January 2038. If you want to be convinced of it, print the following:

System.out.println(new SimpleDateFormat("dd/MM/yyyy HH:mm:ss").format(new Date(1000l * Integer.MAX_VALUE)));

Also, if on Java 8, consider using an Instant instead of directly dealing with long values.

Namings

The method getTimeWithMaximumPeople doesn't actually return the time with the maximum people, despite its name. It also returns additional information, namely, the next Epoch time in the data where that maximum was reached and the maximum count.

You have multiple approach to tackle this:

Only return the time, since this is what is strictly wanted.
Rename that method, and possibly wrap the 3 information in an object, instead of an array. Having an object would make it easier to code since the client can directly invoke descriptive methods like getMaximumCount() on it, instead of accessing a value in an array, which obscures the intent.

Other comments

data.size() == 0

is better expressed with data.isEmpty().
Use curly braces, even for small if / else statements.

if (currentData.getActionType() == ActionType.ENTRY)
    count += currentData.getCount();
else
    count -= currentData.getCount();

It is a best-practice to explicitely add the curly braces, as they prevent future possible issues.