Gauss-Seidal implementation The code is an implementation of the Gauss–Seidel method. I would like a general review of the code.

PS: I use code::blocks IDE

//generalized code for gauss seidal iteration method
#include<stdio.h>
#include<malloc.h>

#define chk_end 0.01

int row_lim=0,col_lim=0;

float** partial_pivot(float** mat,int row,int col);//finds the pivot of matrix
int condition(float* result,float* temp,int row);//checks the terminating condition
float* final_result(float** mat,float* result,int row,int col);//calculates final result
float** create_mat(float** mat,int row,int col);//creates a matrix of row X col
float** feed_mat(float** mat,int row,int col);//inputs value in matrix
void disp_mat(float** mat,int row,int col);//displays matrix

float** partial_pivot(float** mat,int row,int col)
{
 float pivot;
 int i=0,j=0,k=0,l=0;

 if(row_lim<row)
 {
  for(i=0,j=col_lim;i<row-1;i++)
  {
   for(l=row_lim;l<row-i-1;l++)
   {
   if(mat[l][j]<mat[l+1][j])
   {
    for(k=0;k<col;k++)
    {
     pivot=mat[l+1][k];
     mat[l+1][k]=mat[l][k];
     mat[l][k]=pivot;
    }
   }
   }
  }
 row_lim++;
 col_lim++;
 partial_pivot(mat,row,col);
 }

 return mat;
}

int condition(float* result,float* temp,int row)
{
 int i=0,flag = 0;

 for(i=0;i<row;i++)
 {
 if(result[i] > temp[i])
 {
  if((result[i]-temp[i]) < chk_end)
  {
   flag = 1;
  }
  else
  {
   flag = 0;
   break;
  }
 }

 else
 {
  if((temp[i]-result[i]) < chk_end)
  {
   flag = 1;
  }
  else
  {
   flag = 0;
   break;
  }
 }
 }

 return flag;
}

float* final_result(float** mat,float* result,int row,int col)
{
 int k=0,i=0,j=0;
 float T = 0;
 float *temp;

 temp=(float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
 for(i=0;i<row;i++)
 {
  temp[i] = 0;
 }

 printf("Processing result ... \n");

 do
 {
   for(k=0;k<row;k++)
   temp[i] = result[i];    //was a logical error, sorry
   {
   for(i=0;i<row;i++)
   {
    for(j=0;j<(col-1);j++)
    {
     if(i==j)
     {

     }
     else
     {
      T += (mat[i][j])*(temp[j]);
     }
    }
    result[i] = ((mat[i][col-1] - T)/(mat[i][i]));
    //temp[i] = result[i];
    T=0;
   }
  }
 }
 while(condition(result,temp,row) == 0);

 free(temp);

 return result;
}

float** create_mat(float** mat,int row,int col)
{
    int i=0;

     mat=(float**)malloc((row)*sizeof(float*));//Edit: row-1 changed to row
     if(mat)
     {
        for(i=0;i<row;i++)
        {
            mat[i]=(float*)malloc((col)*sizeof(float));//Edit: col-1 changed to col
        }
     }

    return mat;
}

float** feed_mat(float** mat,int row,int col)
{
    int i=0,j=0;

    for(i=0;i<row;i++)
    {
        for(j=0;j<col;j++)
        {
            printf("Enter mat[%d][%d] element:\n",i,j);
            scanf("%f",&mat[i][j]);
        }
    }

    return mat;
}

void disp_mat(float** mat,int row,int col)
{
    int i=0,j=0;

     for(i=0;i<row;i++)
     {
        for(j=0;j<col;j++)
        {
            printf("\t%f",mat[i][j]);
        }
        printf("\n");
     }

}

int main()
{
 float **mat,*result;
 int i=0,row=0,col=0;

 mat=NULL;

 printf("Enter the number of Equations :\n");
 scanf("%d",&row);
 col=row+1;

 result = (float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
 for(i=0;i<row;i++)
 {
  result[i] = 0;
 }

 mat=create_mat(mat,row,col);

 printf("Enter the values in the %d x %d matrix:\n",row,col);
 mat=feed_mat(mat,row,col);

 printf("matrix entered by you:\n");
 disp_mat(mat,row,col);

 mat=partial_pivot(mat,row,col);

 printf("pivoted matrix entered:\n");
 disp_mat(mat,row,col);

 result = final_result(mat,result,row,col);

 printf("\nResult is : \n");
 for(i=0;i<row;i++)
 {
  printf("  x[%d]   %f",i,result[i]);
 }

 for(i=0;i<row;i++)
 {
     free(mat[i]);
 }
 free(mat);

 free(result);

 return 0;
}

A word of warning: I am very much a C newbie myself, so it may be good to take my advice with a grain of salt.

Formatting

The only good thing I can say about your formatting is that it's consistent. But a 1-space indent really isn't readable. I recommend 4 spaces instead. Please put some whitespace around your operators: a space after every comma, a space on each side of =, -, +, *, …. Don't cram multiple assignments on one line: I find

int i, j, k;

or

int i = 0;
int j = 0;
int k = 0;

acceptable, but not int i=0,j=0,k=0;.

If you are targetting a C89-compatible environment, this is as far as we can go. I recommend you move on (e.g. to C++ or C99), where we can put the declaration of for-loop variables inside the loop itself:

for (int i = 0; i < whatever; i++)

… and we also don't have to put all declarations at the top of each scope – just declare your variables at the point of first use.

malloc and error handling

Always check the return value of malloc:

float *temp = (float*) malloc(row * sizeof(float));
if (temp == NULL)
{
    /* return some error code */
}

C isn't object oriented, but whatever

You always bundle the three variables mat, row, col. I suggest you put those into a struct, and pass this single struct around instead:

typedef struct {
    float **mat;
    int rows;
    int cols;
} Matrix;

I would then create a Matrix_new function that creates and populates an instance, e.g.

// this code assumes C99. Ignore my more compact brace style.

/**
 * Allocates a new `Matrix` instance. Returns a pointer to the instance on
 * success, a `NULL` pointer on failure.
 *
 * A Matrix must always be freed via `Matrix_destroy(instance)`!
 */

Matrix*
Matrix_new(int rows, int cols) {

    Matrix* matrix = (Matrix*) malloc(sizeof(Matrix));
    if (matrix == NULL) {
        return NULL;
    }

    float** mat = (float**) malloc(rows * sizeof(float*));
    if (mat == NULL) {
        free(matrix);
        return NULL;
    }

    for (int i = 0; i < rows; i++) {
        float* row = (float*) malloc(cols * sizeof(float));
        if (row == NULL) {
            // if a row allocation fails, we have to `free` the previous rows.
            i--;
            for (; i >= 0; i--) {
                free(mat[i]);
            }
            free(mat);
            free(matrix);
            return NULL;
        }

    }

    matrix->mat  = mat;
    matrix->rows = rows;
    matrix->cols = cols;
    return matrix;
}

and of course, a mirroring Matrix_destroy:

/**
 * Safely frees a Matrix instance.
 */

int
Matrix_destroy(Matrix* matrix) {

    if (matrix == NULL) {
        return 0;
    }

    float** mat = matrix->mat;
    int rows = matrix->rows;

    for (int i = 0; i < rows; i++) {
        free(mat[i]);
    }

    free(mat);
    free(matrix);

    return 1;
}

Then, you can rewrite your other methods to take advantage of this bundling.

Return values

Very often (e.g. in final_result, create_mat, feed_mat), you return a pointer from your function even when that pointer was an argument to that function. It is generally preferable to return an error code, and to return values via pointers/out-arguments.

For example, final_result could be changed to:

/**
 * Calculates the final result.
 *
 * Returns an error code.
 *
 * After successful execution, the `out_result` pointer will point to the
 * result, which is a `float*` of `self->rows` length. The caller owns the
 * result, and is responsible for calling `free(*out_result)`.
 *
 * Example usage:
 *
 *     float *result;
 *     if (! final_result(matrix, &result)) {
 *         // abort with error
 *     }
 *     // do something with `result`:
 *     for (int i = 0; i < matrix->rows; i++) {
 *         printf("%f\n", result[i]);
 *     }
 *     free(result);
 */

int
final_result(Matrix* self, float** out_result) {
    if (self == NULL) {
        return 0;
    }

    float* temp = (float*) malloc(self->rows * sizeof(float));
    if (temp == NULL) {
        return 0;
    }

    float* result = (float*) malloc(self->rows * sizeof(float));
    if (result == NULL) {
        free(temp);
        return 0;
    }

    for (int i = 0; i < self->rows; i++) {
        temp[i]   = 0;
        result[i] = 0;
    }

    printf("Processing result...\n");

    do {
        for (int i = 0; i < self->rows; i++) {
            float T = 0;
            for (int j = 0; j < self->cols - 1; j++) {
                if (i != j) {
                    T += self->mat[i][j] * temp[j];
                }
            }
            result[i] = (self->mat[i][self->cols - 1] - T) / self->mat[i][i];
            temp[i] = result[i];
        }
    } while (condition(result, temp, self->rows) == 0);

    free(temp);
    *out_result = result;
    return 1;
}

Miscellaneous Helper Functions

memset

When zeroing all elements in some data structure, you can often use memset from string.h instead:

temp=(float*)malloc((row)*sizeof(float));//Edit: row-1 changed to row
for(i=0;i<row;i++)
{
 temp[i] = 0;
}

becomes

float* temp = (float*) malloc(row * sizeof(float));
if (temp == NULL) {
    // return error code
}
memset(temp, 0, row * sizeof(float));

fabsf

If you are interested in an absolute value of a float, use the fabsf function from math.h. With it, your condition function simplifies to:

int
condition(float* result, float* temp, int row) {
    int flag = 0;

    for (int i = 0; i < row; i++) {
        if (fabsf(temp[i] - result[i]) < chk_end) {
            flag = 1;
        }
        else {
            flag = 0;
            break;
        }
    }

    return flag;
}


Windows forms set saved groupbox location settings I have a Windows form that when a user moves some groupboxes around, before the application exits, it saves those X and Y coordinates into a MySQL database (with a bit of added text to identify which groupbox those settings belong to).  This all works fine.  When they log back into the application, I can pull those settings too.

Is this the most efficient way to accomplish parse & apply saved settings from a string?

Here's the working function that's grabbing this text string and parses it:

private void apply_GUI_settings(string settings)
{
    // The string below, GUI_settings, will evaluate to: "[1:{X=7,Y=29}][2:{X=52,Y=433}]"
    string GUI_settings = settings;
    bool startFound = false;

    for (int c = 0; c < GUI_settings.Length; c++)
    {
        switch (GUI_settings[c])
        {
            case '[':
                {
                    startFound = true;
                }
                break;
            case '1': // groupBox_AccountInfo.Location settings
            case '2': // groupBox_YourCharacters.Location settings
                {
                    string which_groupBox = GUI_settings[c].ToString();

                    if (startFound)
                    {
                        c++; // Iterate over the "1"
                        if (GUI_settings[c] == ':')
                        {
                            int n = 0;
                            string X = "";
                            string Y = "";

                            c += 4; // Iterate over the ":{X="
                            while (int.TryParse(GUI_settings[c].ToString(), out n))
                            {
                                c++;
                                X += n.ToString();
                            }
                            n = 0;

                            c += 3; // Iterate over the ",Y="
                            while (int.TryParse(GUI_settings[c].ToString(), out n))
                            {
                                c++;
                                Y += n.ToString();
                            }
                            n = 0;

                            if (which_groupBox == "1")
                                groupBox_AccountInfo.Location = new Point(int.Parse(X), int.Parse(Y));
                            else if (which_groupBox == "2")
                                groupBox_YourCharacters.Location = new Point(int.Parse(X), int.Parse(Y));

                            // Might as well cycle thru the rest of this string until ']'
                            while (GUI_settings[c] != ']')
                                c++;
                        }
                        else
                        {
                            MessageBox.Show("[ERROR:53928]");
                        }

                        startFound = false;
                    }
                    else
                    {
                        continue;
                    }
                }
                break;
        }
    }
}

Parsing the settings strings character by character is troublesome and can be hard to get it right.

It would be better to use a regex to match the string and extract the relevant parts using capture groups, for example:

\[(\d+):\{X=(\d+),Y=(\d+)\}\]

Debuggex Demo

I know too little of C# to give a sample implementation,
in case it helps, here it is in Java:

Pattern pattern = Pattern.compile("\\[(\\d+):\\{X=(\\d+),Y=(\\d+)\\}\\]");
String sample = "[1:{X=7,Y=29}][2:{X=52,Y=433}]";

Matcher matcher = pattern.matcher(sample);
while (matcher.find()) {
    int id = Integer.parseInt(matcher.group(1)),
    int x = Integer.parseInt(matcher.group(2)),
    int y = Integer.parseInt(matcher.group(3))
    // do what you need with id, x, y
}


Exception handling for HTTP request timer This method is used to check a URL and return the time it takes to check and the HTTP status code:

public static String urlCheck(String url) {
    try {
        URL urlToCheck = new URL(url);
        HttpURLConnection connection = (HttpURLConnection) urlToCheck.openConnection();
        connection.setRequestMethod("GET");
        Long start = System.currentTimeMillis();
        connection.connect();
        int urlStatus = connection.getResponseCode();
        Long stop = System.currentTimeMillis();
        Long urlCheckTime = stop - start;
        return (connection.getRequestMethod() + " " + urlToCheck + " "
                + urlStatus + " " + urlCheckTime + "ms");
    }
    catch (IOException err){
        logger.log(Level.WARNING,err.getMessage());
        return "Skipping URL";
    }

}

My questions are:

Should my try block contain all the lines that are in it now, or should it only contain the line that can throw an exception (I put them all in since variables were not resolving (e.g. urlToCheck on first line was not passed to the second line if the second line was outside the try block))?
Is using the blanket exception IOException ok, or would it be better to create a catch block for each exception type that the try block can throw?

Generally, you should try and minimize the number of statements in a try block to the minimum possible.

However, in your case, since

URL urlToCheck = new URL(url);
HttpURLConnection connection = (HttpURLConnection) urlToCheck.openConnection();
connection.setRequestMethod("GET");

and

connection.connect();
int urlStatus = connection.getResponseCode();

can all throw exceptions, and the rest of the code is relatively minor (5 lines in your version or 4 in mine), I would leave it all as is, in the try block.

I would recommend eliminating stop and writing the timing code like this:

Long start = System.currentTimeMillis();
int urlStatus = connection.getResponseCode();
Long urlCheckTime = System.currentTimeMillis() - start;

This will yield slightly more accurate timing and eliminates a unneeded variable. You can also eliminate urlCheckTime by having the code in the return string but that's a matter of taste.

As for whether to use a blanket IOException or to have individual catch blocks for MalformedUrlException and so on, that is a question that you need to answer. If you want to have more detailed output, you will need the additional catch blocks. If however, you are simply testing to see if the URL is working, then your method is fine.


Is there a safer/ more readable way of using this code? Is there a safer/ more readable way of using this code?
I gotta translate this code into a better , safer and more readable sample of code and get my onclick to work.(its currently called from another thread then what it is created from.)

public class TemperaturePresenter
{
    MainView _view;

    // TODO ojoj det blev lite stökigt här
    public TemperaturePresenter(){
        _view = new MainView();
        ThreadPool.QueueUserWorkItem(
             delegate{
        XmlDocument serverDoc = new XmlDocument();
        serverDoc.Load("ServerUrls.xml");
        var location = serverDoc.SelectSingleNode("//servers/server/url");
        string serverUrl = location.InnerText;
        var searchNode = serverDoc.SelectSingleNode("//servers/server/xpath");
        var searchPath = searchNode.InnerText;
        WebRequest req = WebRequest.Create(serverUrl);
        var response = req.GetResponse();
        var a = response;
        Stream stream = response.GetResponseStream();
        XmlDocument responseDoc = new XmlDocument();
        responseDoc.Load(stream);
        var temperatureNode = responseDoc.SelectSingleNode(searchPath);
        string temperature = temperatureNode.Attributes["value"].Value;

        SetTemperature(temperature);
            });
    }

    void SetTemperature(string temperature) {
        //if ( _view.InvokeRequired ){
        //    _view.Invoke( (Action<string>) SetTemperature, temperature);
        //    return;
        //}
        _view.Temperature = temperature;
    }

    public Form Form { get{ return _view; } }

Next page:

public partial class MainView : Form
{
    public MainView()
    {
        InitializeComponent();
    }

    private void button1_Click(object sender, EventArgs e)
    {
        //fyll

        throw new NotImplementedException("cant press on this button, yet!");
    }

    public string Temperature {
        set {
            tempLbl.Text = String.Format("{0} grader celcius", value);
            Refresh();
        }

Im thinking i need to do some "this.thread" stuff, but i dont know how.

Well, at first glance, i think it should be:

Constructor shouldn't have any logic other than initialization. Any I/O, Threads .... operations would be better to start in custom method. So, in your case i will write:

public TemperaturePresenter(){
    _view = new MainView();
}
delegate in your example very large, my opinion - it should be extracted as a new method
Webrequest in Presenter? It depends, but I think it no so good. May be it would be better if you extract web-request logic as external dependency like ITemperatureProvider. Because now, it pull dependency on webserver file and file working into the presenter - it has some smell.

At first glance that's all.


Find All Permutations of String in Scala I created the following function, permutations, to produce all permutations of a List[A].
Example:
scala> net.Permutations.permutations("ab".split("").toList)
res3: List[List[String]] = List(List(a, b), List(b, a))

Code:
object Permutations {

  def permutations[A](str: List[A]): List[List[A]] =
    str match {
      case Nil => List(Nil)
      case list @ _ :: _ =>
        val shifteds: List[List[A]] =
          shiftN(list, list.length)

        shifteds.flatMap {
          case head :: tail =>
            permutations(tail).map { lists: List[A] =>
              head :: lists
            }
          case Nil => Nil
        }
    }

  private def shiftN[A](list: List[A], n: Int): List[List[A]] = {
    if (n <= 0) Nil
    else {
      val shifted: List[A] = shift(list)
      shifted :: shiftN(shifted, n - 1)
    }
  }

  private def shift[A](arr: List[A]): List[A] = arr match {
    case head :: tail => tail ++ List(head)
    case Nil => Nil
  }
}

I think it's correct since the following property-based check succeeds:
import munit.ScalaCheckSuite
import org.scalacheck.Prop._
import org.scalacheck.Gen

class PermutationsSpec extends ScalaCheckSuite {

  private val listGen: Gen[List[Int]] =
    for {
      n <- Gen.choose(0, 7)
      list <- Gen.listOfN(n, Gen.posNum[Int])
    } yield list

  property("permutations works") {
    forAll(listGen) { list: List[Int] =>
      val mine: List[List[Int]] = Permutations.permutations(list)
      val stdLib: List[List[Int]] = list.permutations.toList
      assert(stdLib.diff(mine).isEmpty)
    }
  }

}

Please evaluate for correctness, concision and performance.

The code looks to be correct, yes, the testing looks good too.  I was
thinking whether List(List()) for the input List() makes sense, but
it seems like that's a sensible output.
For the code readability I'd rename str, especially since it's not
really a string, but a list.  The complicated match expression in
permutations can just be simplified to case _ and in the body the
original argument can be reused again.
I'd also inline shifteds value since it's just a single call and the
name doesn't really tell me anything.  On that note, docstrings for the
functions might be a nice touch, especially for the shift and shiftN
methods.
The unused case labels can also just be _ everywhere.  Depends of
course, for me this makes it clearer that really there's always just two
cases, either matching an empty list, or a non-empty one, there's no
third case.
Would look like this then:
object Permutations {
  def permutations[A](list: List[A]): List[List[A]] =
    list match {
      case Nil => List(Nil)
      case _ =>
        shiftN(list, list.length).flatMap {
          case head :: tail =>
            permutations(tail).map(head :: _)
          case _ => Nil
        }
    }

  def shiftN[A](list: List[A], n: Int): List[List[A]] = {
    if (n <= 0) Nil
    else {
      val shifted: List[A] = shift(list)
      shifted :: shiftN(shifted, n - 1)
    }
  }

  def shift[A](list: List[A]): List[A] = list match {
    case head :: tail => tail ++ List(head)
    case _ => Nil
  }
}

Lastly, performance-wise it depends what your constraints are: For
List input and List output, restricting it to single linked lists,
this is fine, though I haven't benchmarked them of
course.  Potentially converting the shifted :: shiftN(...) call into
using an accumulator might be worth a bit, instead of having a deep call
stack, but again, it'll probably only matter for longer inputs.
But there are way quicker algorithms, though you might want to copy the
input into a vector that can be accessed in constant time for each
index.  (I found the
QuickPerm algorithm, as explained here
absolutely straightforward to implement from scratch.)


Returning BindingList I came to work at a place not really understanding data binding. I have been working with it for about a year now, and I'd like to get some clarification on what is going on.

Particularly, I'd like to know if there any difference between the following two types of calls.

Given this simplistic example of the data:

public class TableType {
    public int ID;
}

public System.Data.Linq.Table<TableType> TableTypes
{
    get
    {
        return this.GetTable<TableType>();
    }
}

Version 1:

private BindingList<TableType> _tableTypeList;

public BindingList<TableType> TableTypesList1
{
    get
    {
        if (_tableTypeList == null)
        {
            var temp = TableTypes.OrderBy(t => t.ID);
            _tableTypeList = ((IListSource)temp).GetList() as BindingList<TableType>;
        }
        return _tableTypeList;
    }
}

The Senior Developer here wrote lots of database code using Version 1.

Looking at it, you would think that it would prevent redundant database calls, but that does not seem to be the case. A breakpoint on the code never gets hit again.

Version 2:

public BindingList<TableType> TableTypesList2
{
    get
    {
        var query = from t in TableTypes orderby t.ID select t;
        var result = new BindingList<TableType>(query.ToList());
        return result;
    }
}

The Senior Developer left about 6 months ago, so now I am writing the database calls.

I have been using Version 2, which looks much cleaner to me and appears to do the exact same thing.

Is one version any better than the other? Is there anything one version does that another does not? How do I get a better feel for what is going on?

Particularly, I was surprised to find that another call to TableTypesList1 did not call the getter.

Is one version any better than the other?

It depends on what you actually want.

V1 is a single time operation. The table is going to be filled only once and subsequent calls will use cached data.
V2 on the other hand will be executed on each acceess.

One cannot say which one is to chose. You must know what would work better for you.

  Is there anything one version does that another does not?

Yes. V1 will get data only once and V2 everytime. This means that it might be acceptable to let the first property be a property because subsequent calls will be fast and only the first time initialization might take some time (depending on how much data it needs to get). The value it returns is stored in a backing field and it returns the same value everythime.

The second solution on the other head clearly behaves like a method because it can return different lists on each call thus I would make V2 a method.

  How do I get a better feel for what is going on?

This if is the answer:

if (_tableTypeList == null)
{
    var temp = TableTypes.OrderBy(t => t.ID);
    _tableTypeList = ((IListSource)temp).GetList() as BindingList<TableType>;
}

  I was surprised to find that another call to TableTypesList1 did not call the getter.

It probably did but when you put the breakpoint inside the if it's obvious that this block won't be called again.


Optimizing Floodfill algorithm implementation as much as possible Actually my implementation is not working very well for big areas (I don't know why) I already created a question on Stackoverflow. But I figured out how to do it to work with an O(n) implementation (you can see what I mean here (see the fiddle I attached)).

But for some reason as I said it is not working very well on big areas.

/// <summary>
/// Floods the fill.
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="source">The source.</param>
/// <param name="x">The x.</param>
/// <param name="y">The y.</param>
/// <param name="width">The width.</param>
/// <param name="height">The height.</param>
/// <param name="target">The target.</param>
/// <param name="replacement">The replacement.</param>
public static void FloodFill<T>(this T[] source, int x, int y, int width, int height, T target, T replacement)
    where T : IEquatable<T>
{
    int i;

    source.FloodFill(x, y, width, height, target, replacement, out i);
}

/// <summary>
/// Floods the array following Flood Fill algorithm
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="source">The source.</param>
/// <param name="x">The x.</param>
/// <param name="y">The y.</param>
/// <param name="width">The width.</param>
/// <param name="height">The height.</param>
/// <param name="target">The target to replace.</param>
/// <param name="replacement">The replacement.</param>
/// <param name="i">The i.</param>
// This was generic
public static void FloodFill<T>(this T[] source, int x, int y, int width, int height, T target, T replacement, out int i)
    where T : IEquatable<T>
{
    i = 0;
    HashSet<int> queue = new HashSet<int>();

    queue.Add(P(x, y, width, height));

    while (queue.Count > 0)
    {
        int _i = queue.First(),
            _x = _i % width,
            _y = _i / width;

        queue.Remove(_i);

        if (source[_i].Equals(target))
            source[_i] = replacement;

        for (int offsetX = -1; offsetX < 2; offsetX++)
            for (int offsetY = -1; offsetY < 2; offsetY++)
            {
                // do not check origin or diagonal neighbours
                if (offsetX == 0 && offsetY == 0 || offsetX == offsetY || offsetX == -offsetY || -offsetX == offsetY)
                    continue;

                int targetIndex = Pn(_x + offsetX, _y + offsetY, width); // This is already inverted that's why we don't use F.P(...)
                int _tx = targetIndex % width,
                    _ty = targetIndex / width;

                // skip out of bounds point
                if (_tx < 0 || _ty < 0 || _tx >= width || _ty >= height)
                    continue;

                if (!queue.Contains(targetIndex) && source[targetIndex].Equals(target))
                {
                    queue.Add(targetIndex);

                    if (Monitor.IsEntered(i))
                        ++i;
                    else
                        Interlocked.Increment(ref i);
                }
            }

        if (i > 100000)
            break;
    }
}

My main concerns are the following:

I don't know if there is a better way to debug how many iterations has the algorithm done (and if I'm getting them correctly).
I don't like to use Monitor.IsEntered (I'm not sure if this is needed).
I would like to speed-up it, but I don't know how (I think this is slow) (you can compare it with ms-paint to see differences, I know it's written on C++, but... I would like to get a significant perfomance).
Also, I don't know if there is a better way to do it (to avoid unnecesary memory consumption).

Monitor.IsEntered

This call basically makes no sense. Monitor.IsEntered is used for coordination/mutual exclusion between threads. There are no threads here, and even if there were, there is no reason to lock on i: it is a counter. It isn't even a meaningful target for Monitor.IsEntered because it isn't an object.

There is no apparently threading going on here, so you can confidently remove all the locking stuff (including Interlocked.Increment). This may noticeably improve performance, since Monitor.IsEntered is non-trivial, and the call will incur boxing (so at worse this will improve the memory characteristics of your code).

If, for some reason, you are reading i from a different thread while the flood-fill is running, then you might want to use Interlocked methods to provide some notion of up-to-dateness.... but unless you are using that low-latency synchronisation (in which case it is a bad choice for doing so) you won't in practice need any manual memory barriers here. If it is not the case that you want to access i before this has finished then keep track of it as a 'normal' variable, and then set it as out at the end (if it is needed at all). out parameters can be abused, and may interfere with the compilers ability to perform optimisations.

100000

What is this?

if (i > 100000)
    break;

This is a magic number threshold. This says "if I happen to look at some mysterious number of entries, then I will give up and provide a mangled result". This is terrible, especially so because it isn't documented. If you want a cut-out, then have the threshold passed as a parameter, and document it in the inline documentation. It's good that you have inline documentation, but it isn't very helpful at the moment, failing to explain what any of the parameters are beyond suggesting that target will be 'replaced' (what does that mean?).

Furthermore, if it does cut out, then the consumer should be informed. You could throw a FloodfillCutoffException, or return a bool indicating that it did not complete, or something else.

HashSet/Stack

You should probably be using a Stack. HashSet.Contain is efficient (for what it does), but you don't need it. (This is assuming that target.Equals(replacement) is always false, but if that isn't the case then your code is broken anyway, because it doesn't keep track of pixels it has dequeued)

Excepting the first pixel, if you set source[targetIndex] = replacement when you queue it then you don't need to check if it is already in the HashSet: you only need to check that source[targetIndex].Equals(target), which you have to do anyway. The probably with your non-HashSet code wasn't the Stack, it was how you were using it.

With a stack, the code will look mostly the same, only you'll be Poping from the stack (instead of using First() and Remove(T). It should be much faster, because it doesn't lean on HashSet, which while being 'kind of O(1)' isn't going to outperform a single array lookup which probably needs to be done anyway.

Indexing

I'm guessing Pn is static, and looks something like this:

static int Pn(int x, int y, int width => x + y * width;

IEquatable<T>

It's good that you are using this.

Naming

Most of the names are cryptic. This is not ideal. It isn't clear why you use P rather than Pn. i and _i are completely unrelated (and both poor names). target and targetIndex are mostly unrelated.

Code

Here is a (completely untested) rework of your code I threw together in a hurry incorporating some of the points above, with some added comments.

public static void FloodFill<T>(this T[] source, int x, int y, int width, int height, T target, T replacement, int cutoff, out int explored)
    where T : IEquatable<T>
{
    // code can't handle this, so throw
    if (target.Equals(replacement))
    {
        throw new ArgumentException("Target and Replacement cannot equate");
    }

    // do start manually, and exit quickly if appropriate
    int start = P(x, y, width, height);
    if (!source[start].Equals(target))
    {
        explored = 0;
        return;
    }

    // stack is not a great name, but I'm in a hurry right now...
    Stack<int> stack = new Stack<int>();
    stack.Push(start);

    int count = 1;
    while (queue.Count > 0)
    {
        int _i = stack.Pop(),
            _x = _i % width,
            _y = _i / width;

        for (int offsetX = -1; offsetX < 2; offsetX++)
            for (int offsetY = -1; offsetY < 2; offsetY++)
            {
                // do not check origin or diagonal neighbours
                if (offsetX == 0 && offsetY == 0 || offsetX == offsetY || offsetX == -offsetY || -offsetX == offsetY)
                    continue;

                int targetIndex = Pn(_x + offsetX, _y + offsetY, width); // This is already inverted that's why we don't use F.P(...)
                int _tx = targetIndex % width,
                    _ty = targetIndex / width;

                // skip out of bounds point
                if (_tx < 0 || _ty < 0 || _tx >= width || _ty >= height)
                    continue;

                // if pixel matched target, reset it and queue
                if (source[targetIndex].Equals(target))
                {
                    source[targetIndex] = replacement;

                    stack.Push(targetIndex);
                    count++;
                }
            }

        if (i > cutoff)
            throw new Exception("FloodFill passed cutoff");
    }

    // report the number of cells explored
    explored = count;
}


Convert Object to a DateTime public static DateTime ObjectToDateTime(object o, DateTime defaultValue)
{
    if (o == null) return defaultValue;

    DateTime dt;
    if (DateTime.TryParse(o.ToString(), out dt))
        return dt;
    else
        return defaultValue;
}

The code feels too wordy and smells bad.  Is there a better way?

On the offhand chance that your object is already a DateTime, you're performing unnecessary conversions to and from strings.

 if (o is DateTime)
     return (DateTime)o;

This also strikes me as something you might be doing for a database item, for example. In which case, I'd encourage you to know and trust your data types and then use existing methods of retrieval.

For example, if you have a DataTable with a column CreatedDate, you should know it's a date, what you might not know is if it has a value if the column is nullable at the database. That's fine, you can handle that in code.

 var createdDate = row.Field<DateTime?>("CreatedDate");

There we go, a DateTime?, no coding of a conversion necessary. You can even specify a default and type if to DateTime

 var createdDate = row.Field<DateTime?>("CreatedDate").GetValueOrDefault(DateTime.Now);
 var createdDate = row.Field<DateTime?>("CreatedDate") ?? DateTime.Now;


Finding black and white images I am looking to speed up the search for black and white images. I have about 150,000 images that I need to check but this is taking so long. How can I speed up the code?

Private Sub BlackOrWhite()
    Dim iPath As String = txtFile1.Text
    Dim allFiles() As String = IO.Directory.GetFiles(iPath, "*.*", IO.SearchOption.TopDirectoryOnly)
    For Each eachPic In allFiles
        Dim myImage As Bitmap = New Bitmap(eachPic, True)
        Dim myColor As Color = CalculateColorKey(myImage)
        myImage.Dispose()

        If GetRgbDelta(myColor) = 0 Then
            IO.File.Move(eachPic, iPath & "Delete\" & IO.Path.GetFileName(eachPic))
        End If
    Next
End Sub

Public Shared Function CalculateColorKey(bmp As Bitmap) As Color
    Try
        Dim keyColor As Color = Color.Empty
        Dim highestRgbDelta As Integer = 0

        For x As Integer = 0 To CInt((bmp.Width / 2))
            For y As Integer = 0 To CInt(bmp.Height / 2)
                If GetRgbDelta(bmp.GetPixel(x, y)) <= highestRgbDelta Then
                    Continue For
                End If

                highestRgbDelta = GetRgbDelta(bmp.GetPixel(x, y))
                keyColor = bmp.GetPixel(x, y)
            Next
        Next

        Return keyColor
    Catch ex As Exception
        Debug.Print("CalculateColorKey: " & ex.Message)
    End Try
End Function

Private Shared Function GetRgbDelta(color As Color) As Integer
    GetRgbDelta = 100
    Try
        Dim aaa As Integer = Math.Abs(CInt(color.R) - CInt(color.G))
        Dim bbb As Integer = Math.Abs(CInt(color.G) - CInt(color.B))
        Dim ccc As Integer = Math.Abs(CInt(color.B) - CInt(color.R))
        Return aaa + bbb + ccc
    Catch ex As Exception
        Debug.Print("GetRgbDelta: " & ex.Message)
    End Try
End Function

Your code should have a function IsGrayscale(bmp as Bitmap) as Boolean. It should look like this:

Function IsGrayscale(bmp as Bitmap) as Boolean
    For x As Integer = 0 To bmp.Width - 1 Do
        For y As Integer = 0 To bmp.Height - 1 Do
          If Not IsGrayscalePixel(bmp.GetPixel(x, y)) Then
              Return False
          End If
        End For
    End For
    Return True
End Function

Function IsGrayscalePixel(col as Color) as Boolean
    Return col.R = col.G and col.G = col.B
End Function

At least this is what I think your program should do, based on your description. You don't need the concept of a "color key" to check whether an image is grayscale or not.

The main point is that the function returns as soon as it has found a colored pixel, since then there is no point in checking the rest of the image.

Note that I don't know Visual Basic well, so there might be syntax errors in the above code. So focus more on the ideas than one the grammar.


Submitting client reviews I generally encounter a lot of if/else. I would like to get some suggestions on my code:

if( $blnLogged && isset( $_POST['postReview'] ) )
{
    if( $_SESSION['REVIEW']['SUBMITTED'] != '' )
    {
        $arrSharedData['blnRetSave'] = $_SESSION['REVIEW']['SUBMITTED'];
        $blnShowForm = false;
        $blnReviewPosted = true;
    }else{
        if( $arrData['captcha'] != $_SESSION['REVIEW']['CODE'] )
        {
            $strErrMsg = 'Invalid Captcha Code...';
        }else{
            if( empty( $arrData['review_title'] ) || empty( $arrData[ 'review_description' ] ) || empty( $arrData[ 'overall_review' ] ) || empty( $arrData[ 'cleanliness' ] ) || empty( $arrData[ 'facilities' ] ) || empty( $arrData[ 'location' ] ) || empty( $arrData[ 'quality_of_service' ] ) || empty( $arrData[ 'room' ] ) || empty( $arrData[ 'value_of_money' ]) )
            {
                $strErrMsg = 'Required field missing...';
            }else{

                //do we need any processing...
                $arrData['business_id'] = $bID;
                $arrData['client_id']   = $_SESSION['site_user'];
                $arrData['website_id']  = WEBSITE_ID;
                $arrData['review_date'] = date('Y-m-d');

                //If field Transformation required do it...

                $objTripReview = SM_Loader::loadClass('Class_Reviews');
                $blnRetSave = $objTripReview->saveReview( $arrData );
                $_SESSION['REVIEW']['SUBMITTED'] = $blnRetSave;
                $arrSharedData['blnRetSave'] = $_SESSION['REVIEW']['SUBMITTED'];
                $blnShowForm = false;
                $blnReviewPosted = true;
            }
        }
    }
}

if( $blnShowForm === true )
{
    $_SESSION['REVIEW']['CODE'] = rand(9999,99999);
    $_SESSION['Review']['SUBMITTED'] = '';
}

One simple way to improve this is to package it in a function, and return (or throw an exception) after each $strErrMsg = ... line. This will flatten the function, and allow you to put the main functionality at the "top level" of the function.

Even better might be to move all the validation code to a separate function that throws an exception, and then handle it in the function that called it with the wrong parameters.


Design of a remoteControl class for connecting to electronic devices In a course that I'm taking on learning object oriented programming in java, I have completed an assignment for modeling a home entertainment system and the ability for a user to be able to use a single remote control to control multiple digital devices such as a TV or sound system etc.

I don't feel too confident about this design so I wanted to run it past the experts.
Below is my code. Any feedback you can provide on improving this design and why would be super helpful!

package user;

import remotes.RemoteControl;
import devices.Projector;
import devices.TV;

public class Person {

    public static void main (String args[]){

        //Devices in the home
        Projector niceProjector = new Projector();
        TV niceLargeScreenTV = new TV();

        // The user's control
        RemoteControl control;

        control = RemoteControl.connectWithdevice(niceProjector);
            control.clickOnButon();

        control = RemoteControl.connectWithdevice(niceLargeScreenTV);
            control.clickOffButon();

    }

}

RemoteControl class:

package remotes;

import devices.Device;

public class RemoteControl {

    private Device theDevice= null;

    private RemoteControl(){

    }

    public static RemoteControl connectWithdevice(Device aDevice){
        RemoteControl remote = new RemoteControl();
        remote.theDevice = aDevice;
        return remote;
    }

    public void clickOffButon(){
        theDevice.turnOnDevice();
    }

    public void clickOnButon(){
        theDevice.turnOnDevice();
    }

}

One of the device classes:

package devices;

public class TV implements Device{

    @Override
    public void turnOnDevice() {
        SwitchToFavoriteChannel();
        System.out.println("TV has been turned on");
    }

    @Override
    public void turnOffDevice() {
        turnOnTVAlarm();
        System.out.println("TV has been turned off");
    }

    // below are additional steps that may be required for you to operate your device
    private void SwitchToFavoriteChannel(){
        System.out.println("Switched to your favorite Channel");
    }

    private void turnOnTVAlarm(){
        System.out.println("TV Alarm set for you to wake up in the morning");
    }

}

@Override
public void turnOffDevice() {
    turnOnTVAlarm();
    System.out.println("TV has been turned off");
}

Welcome to the hell that never ends.

Just kidding. But please don't make TVs that automatically turn on an alarm to wake you up in the morning. I have alarm clocks for that, which are beating resistant.

Method naming

    control = RemoteControl.connectWithdevice(niceProjector);
        control.clickOnButon();

    control = RemoteControl.connectWithdevice(niceLargeScreenTV);
        control.clickOffButon();

Did you mean clickOnButton and clickOffButton?

Actually, your method naming could use some work. Let's go over some of them. But first, Java convention. It's Java convention that your function names are camelCase.

So, let's look at your method names:

connectWithdevice

This should be connectWithDevice, as each word after the first starts with a capital.

SwitchToFavoriteChannel

This should be switchToFavoriteChannel, as the first word does not have a capital. (That's how we easily distinguish classes and methods from each other).

turnOnTVAlarm

This one is tricky. For now, I'd let it stay... TV is only two characters. But when you have an acronym, treat it as one word. That is, turnOnTvAlarm. This because you end up with class names like HTTPConnection later, and getHttpConnection reads a lot better than getHTTPConnection. Especially if you're not familiar with the acronym.

Instantiating RemoteControl

private Device theDevice= null;

private RemoteControl(){

}

public static RemoteControl connectWithdevice(Device aDevice){
    RemoteControl remote = new RemoteControl();
    remote.theDevice = aDevice;
    return remote;
}

Have you considered passing the device in as constructor argument instead?

Then you can do something like this:

private final Device device;

public RemoteControl(Device aDevice){
    device = aDevice;
}

And people will always be forced to set a device.

With Objects.requireNotNull you can even make them not pass in a null value (you can also write the check yourself, but it's easier with Objects.requireNotNull.

private final Device device;

public RemoteControl(Device aDevice){
    device = Objects.requireNotNull(aDevice, "A remote control requires a device to work with.");
}

Design

Good designs feel natural.

Let's see how natural your design feels.

I have a Projector. And I have a TV. And I may have a RemoteControl... wait, I need to ask RemoteControl to connect to a device so it can send messages to the device? Why can't I just grab the remote control for the TV?

There's two options that I'm aware of in real life:

You take a programmable remote and set it to control a specific device. This would be like passing a different Device into an existing RemoteControl.
You pick up the remote that belongs to the TV. We'll assume we're clean, orderly people who store the remote with the device (far more likely for projectors than for TV's...). This would be like asking the Device to give a RemoteControl.

Both are more natural than wishing for a RemoteControl for a device.


Semantics markup for home sliders Is there a "best" or more suitable markup to home sliders?

I have the following markup. Since my sliders are related to articles, I decided to put article as the main container.

Note: I am using Owl Slider to convert what is inside .home-slider actually in a slider.

Title

Little Description

        Call to Action Link

Title

Little Description

        Call to Action Link

For me the main semantic question is whether you should have a section containing articles or an article containing sections. Or perhaps even an article containing other sub-articles.

From the HTML spec:

  The article element represents a complete, or self-contained, composition in a document, page, application, or site and that is, in principle, independently distributable or reusable, e.g. in syndication. This could be a forum post, a magazine or newspaper article, a blog entry, a user-submitted comment, an interactive widget or gadget, or any other independent item of content.

It really comes down to the content of your articles and sections.

If each slide could be viewed as an independent thing, then marking them with article seems fine.
If however, you'd expect the slides to be viewed in sequence as in slide show, I'd say the slides should be sections instead.


Reversing a string in place without using iteration in C This is function for reversing a string in C using recursive calls:

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

void revert(char* left, char* right) { // Should I pass char* like I'm doing now or are char** a better idea?
    if(left == right) return;

    char *ll = left; char* rr = right;

    char tmp = right[0];
    right[0] = left[0];
    left[0] = tmp;
    revert(++ll,--rr);
}

int main() {
    char rev_string[] = "Some teststring";
    printf("%s\n", rev_string);
    revert(&rev_string[0],&rev_string[strlen(rev_string)-1]);//rev_string[strlen(rev_string)-1]);
    printf("%s\n", rev_string);
    return 0;
}

Can it be simplified? Can anything be written better?

There is a bug. Some teststring contains an odd number of characters, therefore a left == right condition is eventually hit. For an even-length string, it would never be satisfied. You should test for left < right instead.
There is absolutely no need to reassign left, right to ll, rr. revert(++left, --right) works as good.

To clear the possible confusion (see comments) this is what I meant:

if (left < right) {
    swap_values(left, right);
    revert(++left, --right);
}


Splitting log file into smaller files I had a file that looked like this:

Mar 06 22:00:00 [10.251.132.246] logger: 10.64.69.219 - - [06/Mar/2011:22:.....
Mar 06 22:00:00 [10.251.132.246] logger: 10.98.137.116 - - [06/Mar/2011:22:0....

that I wanted to split into smaller files using the ip address after "logger"

This is what I came up with:

file = ARGV.shift
split_file = {}
pattern = /logger: ([^\s]*)/
File.open(file, 'r') do |f|
    f.each do |l|
        match = l[pattern]
        if match
            list = split_file[$1]
            list = [] if list == nil
            list << l
            split_file[$1] = list
        end
    end
end

split_file.each_pair do |k, v|
    File.open("#{file}.#{k}", "a+") do |f|
        v.each do |l|
            f.print l
        end
    end
end

Suggestions, warnings, improvements are very welcome :)

One thing I noticed is that the new files are created in the same directory as the original file, not at the current working directory (so ./logsplitter.rb ../log.log creates files in the .. directory).

Thank you

[edit: typo]

First of all it is a pretty wide-spread convention in ruby to use 2 spaces for indendation not 4. Personally I don't care, but there are some ruby developers who will complain when seeing code indented with 4 spaces, so you'll have an easier time just going with the stream.

file = ARGV.shift

Unless there is a good reason to mutate ARGV (which in this case doesn't seem to be the case), I'd recommend not using mutating operations. file = ARGV[0] will work perfectly fine here.

match = l[pattern]
if match
    list = split_file[$1]
    list = [] if list == nil
    list << l
    split_file[$1] = list
end

First of all you should avoid using magic variables. Using MatchData objects is more robust than using magic variables. As an example consider this scenario:

Assume that you decide you want to do some processing on the line before storing it in split_file. For this you decide to use gsub. Now your code looks like this:

match = l[pattern]
if match
    list = split_file[$1]
    list = [] if list == nil
    list << l.gsub( /some_regex/, "some replacement")
    split_file[$1] = list
end

However this code is broken. Since gsub also sets $1, $1 now no longer contains what you think it does and split_file[$1] will not work as expected. This kind of bug can't happen if you use [1] on a match data object instead.

Further the whole code can be simplified by using a very useful feature of ruby hashes: default blocks. Hashes in ruby allow you to specify a block which is executed when a key is not found. This way you can create hash of arrays which you can just append to without having to make sure the array exists.

For this you need to change the initialization of split_file from split_file = {} to split_file = Hash.new {|h,k| h[k] = [] }. Then you can replace the above code with:

match = l.match(pattern)
if match
  split_file[ match[1] ] << l
end

  One thing I noticed is that the new files are created in the same directory as the original file, not at the current working directory (so ./logsplitter.rb ../log.log creates files in the .. directory).

If you want to avoid that use File.basename to extract only the name of the file without the directory from the given path and then build the path of the file to be created from that. I.e.:

File.open("#{ File.basename(file) }.#{k}", "a+") do |f|

Speaking of this line: I don't see why you use "a+" instead of just "a" as the opening mode - you never read from it.


Compare items in two lists I have a method which compares two lists of strings. If the strings, and only the strings, from listA appear in listB, return true. Else return false.

internal bool DoIdsMatchThoseFromXml(List<string> Ids, List<string> XmlIds)
    {
        bool result = true;

        if (Ids.Count == XmlIds.Count)
        {
            foreach (var hwId in Ids)
            {
                if(!XmlIds.Contains(hwId))
                {
                    result = false;
                    break;
                }
            }
        }
        else
        {
            result = false;
        }

        return result;
    }

My initial unit test is passing (no edge cases etc. yet), but I was wondering if there is a more compact, but still readable, way to do it.

So let me get this straight, you are trying to determine if list A is a subset of list B?

internal bool DoIdsMatchThoseFromXml(List<string> ids, List<string> xmlIds)
{
    return !ids.Except(xmlIds).Any(); // A - B = {}
}

Or are you trying to see if list A is set equal to list B?

internal bool DoIdsMatchThoseFromXml(List<string> ids, List<string> xmlIds)
{
    return ids.Count == xmlIds.Count // assumes unique values in each list
        && new HashSet<string>(ids).SetEquals(xmlIds);
}

I would avoid performing linear searches through this as you'd be looking at O(n*m) performance when you could be doing O(n+m).  Use set operations if you can.  Use HashSet<string> if possible too.


Project Euler Task 42 Solution I am relatively new to programming and have been recommended to work through the many tasks in order, in order to better my skills. For some reason, problem 42 had me stumped for a while:

  The nth term of the sequence of triangle numbers is given by, tn = ½n(n+1); so the first ten triangle numbers are:

  1, 3, 6, 10, 15, 21, 28, 36, 45, 55, …

  By converting each letter in a word to a number corresponding to its alphabetical position and adding these values we form a word value. For example, the word value for SKY is 19 + 11 + 25 = 55 = t10. If the word value is a triangle number then we shall call the word a triangle word.

  Using words.txt, a 16K text file containing nearly two-thousand common English words, how many are triangle words?

The following is my beginner-level code, and I was wondering whether any of you wonderful people would help me improve it and give me some tips for the future.  In retrospect, I should have probably reused some code from problem 22 since they involve the same skills.

def calculate_total(word):
    alphabet = {'a':1, 'b':2,'c':3,'d':4,'e':5,'f':6,'g':7,'h':8,'i':9,'j':10,'k':11,'l':12,'m':13,'n':14,'o':15,'p':16,'q':17,'r':18,'s':19,'t':20,'u':21,'v':22,'w':23,'x':24,'y':25,'z':26}
    total = 0
    index = 1
    while word[index] != '"':
        total += alphabet[word[index].lower()]
        index += 1
    return total

def gen_triangles(limit):
    triangles = [0, 1]
    index = 1
    while triangles[index] <= limit:
        triangles.append(((index ** 2) + index) // 2)
        index += 1
    return sorted(list(set(triangles)))

def binary_search(aList, itemToFind, first, last):
  if last < first:
    #print(itemToFind + "not in list")
    return False
  else:
    midpoint = (first + last)//2
    if aList[midpoint] > itemToFind:
      return binarySearch(aList, itemToFind, first, midpoint - 1)
    else:
      if aList[midpoint] < itemToFind:
        return binarySearch(aList, itemToFind, midpoint + 1, last)
      else:
        #print(str(itemToFind) + " Found at position: " + str(midpoint))
        return True

def solution():
    myFile = open('Words.txt', 'r')
    wordsArray = myFile.read().split(',')
    for i in range(0, len(wordsArray)):
        wordsArray[i] = calculate_total(wordsArray[i])

    triangles = gen_triangles(max(wordsArray))

    wordTriangles = []
    lengthTriangles = len(triangles) - 1

    for i in range(0, len(wordsArray)):
        #print('i:', i, 'current index:', wordsArray[i])
        if binarySearch(triangles, wordsArray[i], 0, lengthTriangles) == True:
            wordTriangles.append(wordsArray[i])

    print(len(wordTriangles))

solution()

I have been teaching myself some computing theory and have seen that Python's built-in searching keyword in performs a linear search on the array so I decided to branch out and see if I can make my own binary search, which seemed to have worked :D

What was a surprise to me was that this actually runs really quickly. To all of you guys, though, this is probably a huge mess, but hey, that's why I'm here!

A Python solution can be much more succinct than what you wrote.

Reading the file: The file is a degenerate CSV file with double-quoted values.  You can use the csv module to split on commas and discard the double-quotes.
Opening the file: You can use the fileinput module to avoid hard-coding the filename.  The program will either open the filename given as a command-line argument, or read from stdin.
Generating triangle numbers: You used the given formula ½n(n+1), but if you want to generate the triangle numbers in sequence, you can just add 1+2+3+…, as per the definition.  Calling sorted(list(set(…))) is superfluous, since each appended number should be unique and increasing — if you initialize triangles = [0] instead of triangles = [0, 1].
Binary search: Using a binary search overcomplicates the solution.  A linear search is not necessarily that bad, if the list is short — as it will be in this case.  Furthermore, you could have just used a set, whose in operator works in O(1) time, instead of a list.  Writing less code means fewer opportunities to introduce bugs.

Personally, I wouldn't even bother pre-generating the triangular numbers.  Testing whether a number is triangular can be done using a simple arithmetic loop — the sort of thing that a CPU is very good at doing.  For small numbers, it's likely to be even faster than looking up entries in a data structure, since the CPU doesn't have to access memory to perform arithmetic.

If I had to test a large number t for triangularity in O(1) time, I'd use the formula in this form:

$$\lfloor \sqrt{2t} \rfloor \lceil \sqrt{2t} \rceil \stackrel{?}{=} 2t$$

… verifying that \$\sqrt{2t}\$ is not integer.
Scoring: Use the built-in sum() function with a generator expression.  The words appear to be all uppercase already; no need to .lower().  I would use ord() to convert letters to numbers.
Counting words that meet the criterion: Again, use the sum() function, which will treat True values as 1, effectively acting as a counter.  In my solution below, print(sum(is_triangular(score(w)) for w in words(fileinput.input()))) effectively summarizes the purpose of the entire program in one line.

With those suggestions, the solution can just consist of a few one-liner functions, and a loop.  It would be a good idea to write doctests to verify that the behavior is consistent with the examples given in the challenge.

import csv
import fileinput
from itertools import count

def is_triangular(number):
    """
    Test whether number is of the form n(n+1)/2.

    >>> is_triangular(1)
    True
    >>> is_triangular(55)
    True
    >>> is_triangular(56)
    False
    """
    counter = count(1)
    while number > 0:
        number -= next(counter)
    return number == 0

def score(word):
    """
    Calculate the word value by converting 'A'=1, 'B'=2, ..., 'Z'=26, and
    adding.  Characters must all be uppercase letters.

    >>> score('SKY')
    55
    """
    return sum(ord(c) - (ord('A') - 1) for c in word)

def words(fileinput):
    return next(csv.reader(fileinput))

if __name__ == '__main__':
    print(sum(is_triangular(score(w)) for w in words(fileinput.input())))


Combine keycode regex and variable array I am using the following code to try and only allow letters, numbers, backspace, space, dashes, slashes, and ampersands. I also want all other keys including the function keys (F1, F2, etc) to be disallowed. I feel like this code can be optimized to make it more streamlined.

This function is part of a live search feature, and I dont want the ajax request to be sent if on of the "prohibited" keys are pressed.

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION

        var functionKey = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123].indexOf(e.which) > -1;

        if (!String.fromCharCode(e.which).match(/[^A-Za-z0-9 ]\x08/) && !functionKey) {

        // code to execute if key is allowed

        }

});

There are two items I can see here which concern me. The first is that, if you have already computed the prohibetedKeyPressed, why not make the boolean check fail-fast? In other words, put the !prohibitedKeyPressed before the regex so that you don't have to do the regex when there's a prohibited key?

I would actually take that further, and turn your code in to guard-clauses:

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION

        if ([112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123].indexOf(e.which) > -1) {
            return;
        }

        if (!String.fromCharCode(e.which).match(/[^A-Za-z0-9 ]\x08/)) {
            return;
        }

        // code to execute if key is allowed

});

That makes the return criteria clear.

Now, the regex should probably be anchored.... but worse than that, the expression is a double-negative - if not the char is not a letter/digit.... do something. Now that I look more closely, it does not work at all..... it will never match anything. The \x08 needs to be inside the character class.... hmmm... have you tested this? The search will always happen because the single key-press will never be a single-character and a backspace.... so, the ajax call will happen whenever a non function key is pressed.....

I would make the code:

if (String.fromCharCode(e.which).match(/^[A-Za-z0-9 \x08]$/)) {
    // do something with it - call ajax
}

Now, having done that, why do you need the initial not-a-function-key check at all? Can't the whole thing just be a positive test?

searchBox.keyup(function (e) { // DYNAMIC AUTO SEARCH FUNCTION
    if (String.fromCharCode(e.which).match(/^[A-Za-z0-9 \x08]$/)) {
        // do something with it - call ajax
    }
}


Ensuring non-expired token before every request I'm developing an iOS application where data is fetched from a third-party REST API. Each request must contain an authorization token.

In order to implement this, I've written a fetchToken withCompletionBlock:(void (^)(ASToken*) token,NSError * error) method in AppDelegate:

-(void) fetchTokenWithCompletionBlock:(void (^)(ASToken * token, NSError * error)) completionBlock
{
    if (!_token) {
        _token = [self retrieveTokenFromKeychain];
    }

    if (!_token || _token.isExpired) {
        [ASMyApi loadTokenWithcompletionBlock:^(ASToken *token, NSError*  error) {

            if (token) {
                _token = token;
                [self saveTokenToKeychain: token];
            }else{
                completionBlock(nil,&error);
            }

        }];
    }else{
        completionBlock(_token,nil);
    }
}

Before every requests that requires Authorization, fetchToken withCompletionBlock:(void (^)(ASToken*) token,NSError ** error) is called to retrieve the token. Here's an example:

-(IBAction) loadCategories
{
    [self.refreshControl beginRefreshing];
    [((AppDelegate *)[UIApplication sharedApplication].delegate) fetchTokenWithCompletionBlock:^(ASToken *token, NSError *error) {

        if (token) {
            [ASMyApi loadCategories:token.token completionBlock:^(NSArray *categories, NSError *error) {

                [self.refreshControl endRefreshing];

                if (categories){

                    self.categories = categories;

                }else{

                    [self monitorNetworkReachability];

                }

                [self updateUI];

            }];

        }else{

            [self.refreshControl endRefreshing];
            [self monitorNetworkReachability];
        }

    }];
}

I'm not happy with this code for the following reasons:

The nested nature of the callbacks makes it look ugly and somewhat unreadable
Too much repetition: The code to get the token is verbose and must be called before each request. The failure code is also repeated.

What I do like about this approach is that it ensures that there's always a valid token before sending the request (as opposed to having the request fail, and then requesting a new token).

I'd appreciate a second-opinion on this design and input on how it might be refactored.

You could introduce a delegate instead passing another block in loadCategories method.

First, the protocol:

@protocol ASMyApiCategoriesDelegate <NSObject>

    @required
    - (void) ASMyApiCategoriesDidLoad:(NSArray *)categories;
    - (void) ASMyApiCategoriesDidFailToLoadWithError:(NSError *)error

@end

You change the signature of + [ASMyApi loadCategories:completionBlock:] to accept a delegate:

+ (void) loadCategoriesToken:(id)token delegate:(id <ASMyApiCategoriesDelegate>)delegate
{
    NSArray *categories;
    // code that loads categories

    if (categories)
    {
        [delegate ASMyApiCategoriesDidLoad:categories];
    }
    else
    {
        [delegate ASMyApiCategoriesDidFailToLoadWithError:nil];
    }
}

In your view controller you'll have to implement the required methods from ASMyApiCategoriesDelegate protocol and update loadCategories method.

- (void) ASMyApiCategoriesDelegateDidLoad:(NSArray *)categories
{
    [self.refreshControl endRefreshing];
    self.categories = categories;
    [self updateUI];
}

- (void) ASMyApiCategoriesDelegateDidFailToLoadWithError:(NSError *)error
{
    [self.refreshControl endRefreshing];
    [self monitorNetworkReachability];
}

- (IBAction) loadCategories
{
    [self.refreshControl beginRefreshing];
    [((AppDelegate *)[UIApplication sharedApplication].delegate) fetchTokenWithCompletionBlock:^(ASToken *token, NSError *error) {
        if (token)
        {
            [ASMyApi loadCategories:token.token delegate:self];
        }
        else
        {
            [self ASMyApiCategoriesDidFailToLoadWithError:nil];
        }
    }];
}


Returning index of nth occurrence of a substring I've built a JavaScript function returning the index of the nth occurrence of a substring in a string.

$(document).ready(function() {

        var containingString = '.this.that.';
        subString = '.';

        var index = GetIndexOfSubstring(containingString, subString, 2);

    });

    function GetIndexOfSubstring(containingString, subString, occurrenceNumberOfSubString)
    {
        tokens = containingString.split(subString);
        var numberOfSubStrings = tokens.length - 1;

        if (occurrenceNumberOfSubString > numberOfSubStrings)
            return "Error";

        var index = 0 + occurrenceNumberOfSubString - 1;

        for(var c = 0; c < occurrenceNumberOfSubString; c++)
        {
            var i = containingString.indexOf(subString);
            var sub = containingString.substr(i + 1);
            containingString = sub;
            index += i;
        }

        return index;
    }

As far as I can see, the function works correctly.  Is there is an easier way to accomplish the goal? Also, can the function be improved?

There's no need to split the original string or slice away at the string, you can just use the built in indexOf, keeping track of how many times you've looked for the substring. As a personal preference, I'd also use smaller, not as bombastic, variables names.

Also, there's absolutely no need to include jQuery in this - the $(document).ready is redundant.

function GetSubstringIndex(str, substring, n) {
    var times = 0, index = null;

    while (times < n && index !== -1) {
        index = str.indexOf(substring, index+1);
        times++;
    }

    return index;
}


Meeting Point problem from interviewstreet.com I am trying to solve the "Meeting Point" problem from interviewstreet.com:

  There is an infinite integer grid at which N people have their houses on. They decide to unite at a common meeting place, which is someone's house.
  From any given cell, all 8 adjacent cells are reachable in 1 unit of time.
  eg: (x,y) can be reached from (x-1,y+1) in a single unit of time.
  Find a common meeting place which minimises the sum of the travel times of all the persons.

For example, if the input is:

4
0 1
2 5
3 1
4 0

the output must be 8. Here's my solution:

def pos(a):
    if a<0:
        return -a
    else:
        return a
n = long(raw_input())
min =0
inputs = []
for i in range(n):
    temp = raw_input().split()
    inputs.append([long(temp[0]),long(temp[1])])
x = inputs[0][0]
y = inputs[0][1]
for i in range(n):
    min += max(pos(x-inputs[i][0]),pos(y-inputs[i][1]))
for i in range(1,n):
    x = inputs[i][0]
    y = inputs[i][1]
    temp =0
    for j in range(n):
        temp += max(pos(x-inputs[j][0]),pos(y-inputs[j][1]))
    if min>temp:
        min =temp
print min

I know I am brute forcing the answer but I couldn't find an algorithm that fits the problem. I have been thinking about the problem statement for a while yet I don't have any clue how to optimize it. If any of you could help me with the code in Python or C.

1. Comments on your code

Your function pos computes the absolute value of a, and is already built into Python under the name abs.
You'd benefit from structuring your code into functions. For example, the expression max(pos(x-inputs[j][0]),pos(y-inputs[j][1])) appears twice. Your code would be clearer if you'd written a function with a docstring, for example:

def distance(A, B):
    """Return the Chebyshev distance from A to B."""
    return max(abs(a - b) for a, b in zip(A, B))

("Chebyshev distance" is a name that mathematicians give to this function.)
You have code near the start for finding the total distance if the meeting point is at house number 0, but then you repeat that code inside a loop over the other meeting points. This seems like a waste. I guess you're doing this because you need an initial value for min.

But instead you could use Python's built-in function min, which avoids the need for a special case to get the initial value. This needs a bit of re-organization, but it comes out quite simple:

def total_distance(Q, points):
    """Return the total distance from Q to each of the points in the
    sequence points.
    """
    return sum(distance(Q, P) for P in points)

def min_total_distance(points):
    """Find that point that minimizes the total distance to the
    other points, and return the minimum total distance.
    """
    return min(total_distance(P, points) for P in points)

If you are a fan of hard-to-read code, then you could write the entire computation in a single expression:

min(sum(max(abs(a - b) for a, b in zip(P, Q)) for P in points) for Q in points)

2. A better algorithm

Your algorithm is \$Θ(n^2)\$: it has to compute the distance between every pair of houses. In the interviewstreet.com version of the problem, they say that \$n ≤ 10^5\$ so your approach takes up to \$10^{10}\$ distance computations. That's not utterly intractable, but it's not going to be doable within the time limits at interviewstreet.com.

So is there an approach which scales better as \$n\$ becomes large?

Well, if we weren't restricted to the meeting point being one of the houses, then the best meeting point would be the geometric median. The geometric median is hard to compute, but it can be approximated by an iterative procedure due to Endre Weiszfeld:

def median_approx(P, points):
    """Return a new approximation to the geometric median of points by
    applying one iteration of Weiszfeld's algorithm to the old
    appromixation P.
    """
    W = x = y = 0.0
    for Q in points:
        d = distance(P, Q)
        if d != 0:
            w = 1.0 / d
            W += w
            x += Q[0] * w
            y += Q[1] * w
    return x / W, y / W

def geometric_median(points, epsilon):
    """Return an approximation to the geometric median for points.
    Start with the centroid and apply Weiszfeld's algorithm until the
    distance between steps is less than epsilon.
    """
    n = float(len(points))
    P = tuple(sum(P[i] for P in points) / n for i in range(2))
    while True:
        Q = median_approx(P, points)
        if distance(P, Q) < epsilon:
            return Q
        P = Q

Unfortunately, for this problem, the meeting point must be one of the original points, so the geometric median doesn't solve it.

However, if the set of points is fairly random (that is, not chosen adversarially), then the best meeting point seems likely to be one of the closest points in the set to the geometric median.

So we have a chance of finding the best meeting point by taking a heuristic approach: compute an approximation \$G\$ to the geometric median, sort the points by their distance from \$G\$, look at the closest \$k\$ points, and pick the best one. If we choose \$ε\$ to be small enough, and \$k\$ to be large enough (but not so large that the computation takes too long), then we'll get the right answer.

from heapq import nsmallest

def best_meeting_point(points, epsilon, k):
    """Return (a guess at) the point in points that minimizes the sum
    of distances to the other points, by computing an approximation
    G to the geometric median (using epsilon) and then taking the
    best point among the closest k points to G.
    """
    G = geometric_median(points, epsilon)
    closest_k = nsmallest(k, points, key = lambda P: distance(G, P))
    return min(closest_k, key = lambda P: total_distance(P, points))

This runs in \$O(jn + kn)\$ where \$j\$ iterations are needed for Weiszfeld's algorithm to converge. \$j\$ doesn't grow proportionally with \$n\$ (it depends on \$ε\$ and on the magnitude of the coordinates in the input), so this is \$o(n^2)\$.

But beware: if the points are chosen adversarially, I think \$k\$ might have to grow like \$Ω(n)\$. I don't know an efficient algorithm for finding the answer in this case. So whether you can use this algorithm to pass the challenge at interviewstreet.com depends on how nice their test cases are. Good luck!


interest calculator with React+Redux problem statement: using React and Redux build an interest calculator that, given a principal, rate of annual interest, and number of years, will display the total principal plus interest using the formula
TOTAL = principal * (1 + (rate * years))
Any feedback welcome on the style, approach, etc.
<!doctype html>

Everything's on the front-end You're using HTM to interpret the JSX by putting the JSX through a template literal. It's quite an interesting option, but for a professional project, I think compiling the JSX beforehand in a build process would be a better choice:

It's very easy to make syntactical typos in the template literal, and you have to run the script in order to see them. If something gets conditionally rendered, you may not see problems with your syntax until the component gets rendered, which may be uncommon; the potential bugs will be significantly harder to encounter and fix. In contrast, if you put the JSX inside a JSX file, and use a syntax-aware IDE like VSCode, you'll be able to spot such mistakes on sight and fix them immediately.
Transpiling the script ahead of time also makes things much easier if you decide to make the project larger. For example, if you have component A and notice that something isn't working right with it, you could have a standalone A.jsx file that you could go to and debug. (In contrast, if you have, say, 200 or 400 or 1000 lines of code in a single file without a build process, it gets more difficult to navigate around than it should be)
Having standalone .js / .jsx files also allows for linting, which I consider to be absolutely essential to enforcing code style and quality, and which can turn difficult-to-debug runtime errors into trivially fixable compile-time errors.
It also allows you to transpile ES2020+ syntax down to ES2015 or ES5 or whatever you prefer, which can make code easier to read and write while remaining compatible with as many browsers as possible.

It's just an option to consider. If you're not happy at the amount of setup it requires, you can use boilerplate like create-react-app so that it's nearly ready to go out of the box.
Avoid let and reassignment When variable names are permitted to be reassigned, it can make code more difficult to understand at a glance, when you always have to keep in mind "This variable was declared with let, so I need to be on the lookout for when it may be reassigned." (Linting rule: prefer-const). See this post on softwareengineering about the topic. There are a number of other places in the code where you're using let where const can be used instead.
In the reducer, rather than reassign store and then return it at the bottom of the switch, you can return it immediately. (You could also consider avoiding the switch here, since it requires a lot of unnecessary boilerplate).
// When an argument list requires multiple lines,
// I think it's easier to read to put each argument on a separate line like this:
function reducer(
    model={total: 0},
    action={type:'', payload: null}
) {
    switch(action.type){
        case 'TOTAL_COST':
            return { total: action.payload.total };
        default:
            return model;
    }
}

or, if the reducer won't be expanded in the future:
function reducer(
    model={total: 0},
    action={type:'', payload: null}
) {
    return action.type === 'TOTAL_COST'
        ? { total: action.payload.total };
        : model;
}

this binding and mount actions You do:
constructor(props){
    super(props);
    this.state = {
        principal: 1000,
        years: 7,
        rate: 0.025
    };
    this.submit = this.submit.bind(this);
    this.update = this.update.bind(this);
    this.submit(new CustomEvent('init'));
}

There are 2 improvements that can be made here: first, consider using class fields to define the methods instead of using .bind in the constructor, to be more concise and avoid boilerplate. Secondly, populating the component initially with this.submit(new CustomEvent('init'));, an event that is passed only so that calling e.preventDefault on it later doesn't throw is weird. How about using optional chaining in submit instead of passing a parameter?
class App extends React.Component{
    state = {
        principal: 1000,
        years: 7,
        rate: 0.025
    };
    constructor(props){
        super(props);
        this.submit();
    }
    submit = (e) => {
        e?.preventDefault();
        this.props.dispatch( actions.updateTotal(this.state) );
    }
    update = (e) => {

You could also consider using functional components instead; React recommends using them instead of class components for new code - they make code a bit easier to understand and maintain.
Update You have:
update(e){
    let { valueAsNumber, value, name } = e.target;
    const state = {...this.state};
    if(isNaN(valueAsNumber)){
        valueAsNumber = 0;
    }
    state[ name ] = valueAsNumber;
    this.props.dispatch( actions.updateTotal(state) );
    this.setState({[name]: valueAsNumber});
}

When using React, seeing something like
state[ name ] = valueAsNumber;

should look scary, because it looks very close to mutating state, and state should never be mutated in React - see here for lots of articles on the subject. While your current method works, it may well be worrying at a glance. Consider using a more functional approach, to construct the new state all at once, without mutation:
update = (e) => {
    const { valueAsNumber, value, name } = e.target
    const newState = {
      ...this.state,
      [name]: valueAsNumber || 0
    };
    this.props.dispatch( actions.updateTotal(newState) );
    this.setState(newState);
}


SortedSet of scored items I want to return a set of items with an attached score to the user so that the user can easily iterate from highest to lowest and get the n-th highest items.

Problems

The JavaDoc of SortedSet.first() says "Returns the first (lowest) element currently in this set.". However in this case it is actually the highest score. If I compare the other way around, however, the user needs to reverse the SortedSet in order to do this common use case.
If I only compare by score, the SortedSet can only have one item with the same score. However comparing by c strikes me as somehow arbitrary (not sure what I can do against that).
Two Items with the same URI should not exist, however that is not prevented when they have different scores.

import java.util.SortedSet;
import java.util.TreeSet;

public class Main
{
    static class Item<T extends Comparable<T>> implements Comparable<Item<T>>
    {
        double score;
        T c;

        public Item(double score, T c)
        {
            this.score=score;
            this.c=c;
        }

        @Override public int compareTo(Item<T> i)
        {
            int x = -Double.compare(score, i.score);
            if(x==0) {x=c.<T>compareTo(i.c);}
            return x;
        }
    }

    public static SortedSet<Item<String>> findItems()
    {
        SortedSet<Item<String>> items = new TreeSet<>();
        items.add(new Item<String>(0.3,"honey"));
        items.add(new Item<String>(0.7,"wolf"));
        items.add(new Item<String>(0.5,"monitor"));
        return items;
    }

    public static void main(String[] args)
    {
        // iterate from best to lowest score
        for(Item item: findItems()) {}
        // get item with best score
        findItems().first();
    }
}

The JavaDoc of SortedSet.first() says "Returns the first (lowest) element currently in this set.". However in this case it is actually the highest score.

The definition of lowest and highest is determined by the comparator. If you put Integers in the sorted set, the first element will be the lowest, because the default comparator will be used. In your Item object however, you have a different definition of what is lower and higher:

@Override
public int compareTo(Item<T> i) {
    int x = -Double.compare(score, i.score);
    if(x==0) {x=c.<T>compareTo(i.c);}
    return x;
}

Your elements are still ordered from lowest to highest as dictated by your comparison method: practically you defined 0.7 to be lower than 0.3, that's all. As you noticed yourself, if you drop the negative sign in int x = -Double.compare(score, i.score); you effectively switch the ordering.

  If I compare the other way around, however, the user needs to reverse the SortedSet in order to do this common use case.

I'm wondering if you're aware of the .last() method. It seems that's what you're looking for.

By the way, you can also make a TreeSet sort items in reverse order, by constructing it like this:

SortedSet<Item<String>> items = new TreeSet<>(Collections.reverseOrder());

  If I only compare by score, the SortedSet can only have one item with the same score. However comparing by c strikes me as somehow arbitrary (not sure what I can do against that).

I'm not really sure what you mean by that. Since c is a String, comparison will be alphabetic by default.

I recommend this writing style (I also renamed variables):

@Override
public int compareTo(Item<T> item) {
    int cmp = -Double.compare(score, item.score);
    if (cmp == 0) {
        return c.<T>compareTo(item.c);
    }
    return cmp;
}


"Utopian Tree" challenge optimization Problem statement

The Utopian tree goes through 2 cycles of growth every year. The first growth cycle occurs during the monsoon, when it doubles in height. The second growth cycle occurs during the summer, when its height increases by 1 meter.
Now, a new Utopian tree sapling is planted at the onset of the monsoon. Its height is 1 meter. Can you find the height of the tree after \$N\$ growth cycles?

Input Format

The first line contains an integer, \$T\$, the number of test cases.
\$T\$ lines follow. Each line contains an integer, \$N\$, that denotes the number of cycles for that test case.

Constraints

1 <= T <= 10
0 <= N <= 60

Sample Input: #01:

2
3
4

Sample Output: #01:

6
7

Explanation: #01:

There are 2 testcases.

N = 3:

the height of the tree at the end of the 1st cycle = 2
the height of the tree at the end of the 2nd cycle = 3
the height of the tree at the end of the 3rd cycle = 6

N = 4:

the height of the tree at the end of the 4th cycle = 7

My solution:

   'use strict';

function processData(input) {
    var parse_fun = function (s) {
        return parseInt(s, 10);
    };

    var lines = input.split('\n');
    var T = parse_fun(lines.shift());

    var data = lines.splice(0, T).map(parse_fun);
    for (var i = 0; i < data.length; i++) {
        var value = 1;
        for (var j = 0; j < data[i]; j++) {
            if (j % 2 == 0)
                value *= 2;
            else value += 1;
        }
        process.stdout.write(value + '\n');
    }
}

process.stdin.resume();
process.stdin.setEncoding("ascii");
var _input = "";
process.stdin.on("data", function (input) {
    _input += input;
});
process.stdin.on("end", function () {
    processData(_input);
});

The running time is of \$O(n^2)\$.  Can anyone tell how to optimize this code to decrease runtime?

A lot of your code seems to be Hackerrank's boilerplate code, so I'll ignore that (though it could use a review), and focus on the "meat" which is:

var value = 1;
for (var j = 0; j < data[i]; j++) {
    if (j % 2 == 0)
        value *= 2;
    else value += 1;
}
process.stdout.write(value + '\n');

(It's run T times of course, but that's irrelevant for now.)

There's not a lot of code there, so there's little to review. Still, the if..else is not great, if you ask me. For one, I'd advice that you always use braces - even for one-liners. But if you don't, at least use linebreaks:

if (j % 2 == 0)
    value *= 2;
else
    value += 1;

However, you can do it all in a ternary, which I would find more appropriate here:

value += (j % 2 === 0 ? value : 1);

The parentheses aren't required, but I find they make it more readable.

Of course, you could also rely on zero being false'y in JavaScript, and just do

value += (j % 2 ? 1 : value);

A slightly faster solution to the even/odd branching would be

value += (j & 1 ? 1 : value);

In other words: If the least-significant bit is 1, the number is odd.

In all, you get:

var value = 1;
for (var j = 0; j < data[i]; j++) {
    value += (j & 1 ? 1 : value);
}
process.stdout.write(value + '\n');

Of course, there may be a purely mathematical, loop-less solution to this. But that's unfortunately not my strong suit.

Update: As mjolka points out in the comments, there is a very simple pattern to this. (And I feel pretty dumb for not realizing it.) I can't really explain it more succinctly than mjolka already has, so I'll just quote the comment here:

  Look at the first few terms: 1, 2, 3, 6, 7, 14, 15, 30, 31, ... and compare that to powers of two: 2, 4, 8, 16, 32, ...

As is obvious, the output values are all equivalent to a power of 2, minus 1 or minus 2. In code, that can be expressed as a function like so:

// calculate tree height after n cycles
function utopiaTreeHeight(n) {
  var exp = Math.ceil(n / 2) + 1,   // calculate the exponent
      value = Math.pow(2, exp) - 1; // power of 2, minus 1

  return value - (n & 1);           // subtract another 1 if n is odd, and return
}

Which means that the rest of the code is just:

for(var i = 0 ; i < data.length ; i++) {
  var value = utopiaTreeHeight(data[i]);
  process.stdout.write(value + '\n');
}

Now, that's pretty clean, I'd say. Big tip of the hat to mjolka!

Below are my previous (iterative and naïve) solutions. I'll leave them in my answer, only because there's probably something of value in there, even if this particular problem has a much more elegant solution.

(End of update.)

Now, about repeating it T times: If you're processing many test cases, it might be worth it to do some pre-processing. All you really need is to iterate to the highest cycle-count. So if you're asked to solve for data = [3, 6, 5] what you really want to just solve for N = 6 but store the intermediate values for N = 3 and N = 5 along the way. You only need to loop once; from zero to Nmax.

Of course, doing so will require extra setup, which may be less efficient than simply doing what you're doing now, if there's only one or two test cases.

Just for fun, though, one solution might be:

var cycles = data.length,          // cache this for later
    sorted = data.slice(0).sort(), // copy and sort the input values (slightly faster if you use an explicit comparison function)
    limit  = sorted[cycles - 1],   // get the max
    target = sorted.shift(),       // grab the lowest value (our first target cycle)
    value  = 1,                    // initial tree height
    values = {};                   // a place to store values

// loop to the highest cycle-count (note the range is 0..limit)
for(var n = 0 ; n <= limit ; n++) {
  while( n === target ) {
    // we reached our target cycle, so store the current value
    values[n] = value;

    // and grab the next target
    target = sorted.shift();
  }
  value += (n & 1 ? 1 : value);
}

// print results in the correct order
for(var i = 0 ; i < cycles ; i++) {
  process.stdout.write(values[data[i]] + '\n');
}

The while loop is there to handle duplicates in the input.

A simpler solution would be to simply store every value for N in 0..Nmax in an array. That tradeoff would be memory consumption. If data = [1, 923123] you'd end up storing 923121 values (~7MB at worst) you're not going to use. Crazy example, but if you don't know your input, well...

Still, such an approach could look like:

var limit = Math.max.apply(null, data),
    values = [],
    value = 1;

for(var n = 0 ; n <= limit ; n++) {
  values.push(value);
  value += (n & 1 ? 1 : value);
}

for(var i = 0 ; i < data.length ; i++) {
  process.stdout.write(values[data[i]] + '\n');
}


Converting between std::wstring and std::string While researching ways to convert back and forth between std::wstring and std::string, I found this conversation on the MSDN forums.

There were two functions that, to me, looked good. Specifically, these:

std::wstring s2ws(const std::string& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, 0, 0);
    wchar_t* buf = new wchar_t[len];
    MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, buf, len);
    std::wstring r(buf);
    delete[] buf;
    return r;
}

std::string ws2s(const std::wstring& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, 0, 0, 0, 0);
    char* buf = new char[len];
    WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, buf, len, 0, 0);
    std::string r(buf);
    delete[] buf;
    return r;
}

However, the double allocation and the need to delete the buffer concern me (performance and exception safety) so I modified them to be like this:

std::wstring s2ws(const std::string& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, 0, 0);
    std::wstring r(len, L'\0');
    MultiByteToWideChar(CP_ACP, 0, s.c_str(), slength, &r[0], len);
    return r;
}

std::string ws2s(const std::wstring& s)
{
    int len;
    int slength = (int)s.length() + 1;
    len = WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, 0, 0, 0, 0);
    std::string r(len, '\0');
    WideCharToMultiByte(CP_ACP, 0, s.c_str(), slength, &r[0], len, 0, 0);
    return r;
}

Unit testing indicates that this works in a nice, controlled environment but will this be OK in the vicious and unpredictable world that is my client's computer?

I would, and have, redesign your set of functions to resemble casts:

std::wstring x;
std::string y = string_cast<std::string>(x);

This can have a lot of benefits later when you start having to deal with some 3rd party library's idea of what strings should look like.


Find next bigger number with same combination of digits Please review my code for a puzzle to find next bigger number with same combination of digits.

so if the number is 156432 the next bigger number is 162345

I wrote a program which is as follows,

// converts a number
// to an number array
// i.e. 13452 = [1,3,4,5,2]
var convertToNumArray =  function(num){

    var numList = [],
        flag = true,
        n = 0;

    do{
         n = num % 10; // get the remainder value
         numList.push(n);
         num = parseInt(num/10); // get the quotient value in division

         if(num <= 0){
             flag = false;
         }

     }while(flag === true);

     numList = numList.reverse();

     return numList;
}

// gets the next value
function getNext(num){
    var numArray = convertToNumArray(num), // gets the number array; example [1,3,4,5,2]
        totalLength = numArray.length - 1,
        nextIndex = 0,
        leastIndex = 0,
        prevDiff = 0,
        diff = 0,
        swapIndex = 0,
        temp = 0,
        str = "";

    // iterate from right most number to find out the
    // number which is less than previous one.
    // for [1,3,4,5,2] the number is 4.
    for(var index = totalLength; index >= 1; index--){
        nextIndex = (index - 1);

        if(numArray[nextIndex] < numArray[index]){
            leastIndex = nextIndex;
            break;
        }else{
            continue;
        }
    }

    // iterate from right to check the number which is closest
    // and greater then the number we found from above loop.
    // so in our case 5 is the closes to 4
    for(index = totalLength; index > leastIndex; index--){
        diff = numArray[index] - numArray[leastIndex];

        if(index === totalLength){
            prevDiff = (diff  < 0) ? 9999 : diff;
            closestNum = numArray[index];
            swapIndex = index;
        }else{
            if(diff >= 0 && diff < prevDiff){
                prevDiff = diff;
                swapIndex = index;
            }
        }

    }
    // as 5 is the closest to 4. Replace them
    // so the conversion will be from [1,3,4,5,2] to [1,3,5,4,2]
    temp = numArray[swapIndex];
    numArray[swapIndex] = numArray[leastIndex];
    numArray[leastIndex] = temp;

    /*
        Now sort the sub array i.e. the numbers falling right
        to the replaced position.
        i.e. [1,3,5,4,2] so the sub array is [4,2].
        so sort in ascending order hence [2,4]
        so the complete number is
        [1,3,5,2,4].
    */

    // bubble sort on sub array
    do{

        for(index = leastIndex + 1; index < totalLength; index++){
            nextIndex = index + 1;
            if(numArray[index] > numArray[nextIndex]){
                 temp = numArray[index];
                 numArray[index] = numArray[nextIndex];
                 numArray[nextIndex] = temp;
            }
        }
        totalLength--;
    }while(totalLength > (leastIndex + 1));

    // convert the array to string
    for(index = 0; index < numArray.length; index++){
        str += numArray[index];
    }

    return parseInt(str);

}// end of getNext

var nr = undefined;

// capturing from commandline
// example : node logic.js 12345
(function(){
    process.argv.forEach(function (val, index, array) {
        if(index === 2)
            nr = array[index];
    });
})();

console.log(nr+" => "+getNext(nr));

When I run the above program
the output is

 E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>node logic.js 156432
 156432 => 162345

 E:\RahulShivsharan\MyPractise\DesignPatternsInJavaScript>

Please review my code and suggest my any improvements
in the logic or in approach or in the style of coding.

Thanks

You convert number to array can be simplified by first converting the number to a string and then splitting on nothing:

var convertToNumArray = function(num){
    return num.toString().split('')
}

This will of course convert it to an array of strings, but that will work perfectly fine for your purposes (or you can always throw in a map to convert back to ints).

Your first loop (noted with this comment):

// iterate from right most number to find out the
// number which is less than previous one.

You don't need the else { continue }.  It is at the end of the loop, so continuing is guaranteed.  Sometimes explicit is nice but I would say not to the extend of writing code that doesn't actually do anything: that just makes for more code to read through.

Also, I don't think you have accounted for the possibility of a number that doesn't have a next value.  For instance, how does your code handle 54321?  Your swap index will stay at the initialized value of zero.  Either way, I think the smart thing to do is to detect this event happening and exit prematurely, as you already have the answer: there is no room for changes.

Your second for loop looks solid.  I might organize it a little differently but no actual complaints: just a different coding style.

For your last bit, I wouldn't do the sort yourself.  Use the built in javascript sort.  You can slice off the end of the array, use sort() (keep in mind that it sorts asciibetically, but that shouldn't hurt you here), and then splice it back onto the end.