Examine a list for cycles From SICP:

  Exercise 3.18.  Write a procedure that
  examines a list and determines whether
  it contains a cycle, that is, whether
  a program that tried to find the end
  of the list by taking successive cdrs
  would go into an infinite loop.
  Exercise 3.13 constructed such lists.

I wrote the following (revised) solution:

(define (has-cycle? l)
  (define (rec nodes-visited remains)
    (define (leaf? node) (not (pair? node)))
    (define (have-seen? node)
      (define (rec rest)
        (if (null? rest) #f
             (or (eq? (car rest) node)
                 (rec (cdr rest)))))
      (rec nodes-visited))
    (cond
      ((leaf? remains) #f)
      ((have-seen? remains) #t)
      (else
       (or (rec (cons remains nodes-visited) (car remains))
           (rec (cons remains nodes-visited) (cdr remains))))))
  (rec '() l))

(define (last-pair x)
  (if (null? (cdr x))
      x
      (last-pair (cdr x))))

(define a '(a b c))
(has-cycle? a)
(set-car! (last-pair a) a)
a
(has-cycle? a)

(define b '(a b c))
(set-car! b (last-pair b))
b
(has-cycle? b)

(define c '(a b c d))

(set-car! (cddr c) (cdr c))
c
(has-cycle? c)

(define d (cons 'z a))
d
(has-cycle? d)

(define e '(a b c d e))
e
(has-cycle? e)

How can it be improved?

  Exercise 3.19.  Redo exercise 3.18
  using an algorithm that takes only a
  constant amount of space. (This
  requires a very clever idea.)

I'm not sure how to fulfill this requirement so far.

I can not express it in Lisp, but the idea is as follows:

use two pointers, at start point both to the list head. Then, increment one pointer by 1, and another by 2. If you detect two equal objects eventually, it means that list has a loop. In pseudo-code:

P0 = head;
P1 = head;
while(P0 != null and P1 != null) // Detect end of the list
  P0 = next(P0);
  P1 = next(next(P1));
  if ( *P0 == *P1 ) signal loop detected
end while

Of course, the list objects should be distinct and comparable, and next function must be correct.

Proof is not complicated. First, consider a case of "pure" loop when tail connected to head.

At step n (n'th iteration of the while loop in the code), P0 is at item (n mod N), and P1 is at item (2n mod N), which are equal if N is finite and n mod N == 0, or simply n = N

If the list has loop that starts somewhere inside, one can show that after some iterations the case will be reduced to the previous one.

Hope this helps. You can learn more about Cycle detection on Wikipedia.


Given a number represented as an array of digits, plus one to the number Given a number represented as an array of digits, plus one to the
  number.

input                    expected

[8,9,9,9]                [9,0,0,0]

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

[9,8,7,6,5,4,3,2,1,0]    [9,8,7,6,5,4,3,2,1,1]

The following is my C++ code:

vector<int> plusOne(vector<int> &digits)
{
    vector<int> result;
    vector<int>::iterator it;
    int plus = 1;
    for (it = digits.end() - 1; it >= digits.begin(); --it)
    {
        int sum = *it + plus;
        if (sum > 9)
        {
            plus = 1;
            result.push_back(0);
        }
        else
        {
            result.push_back(sum);
            plus = 0;
        }
    }

    if (plus==1)
    {
        result.push_back(1);
    }

    reverse(result.begin(), result.end());
    return result;
}

Since you are returning a result.
Pass the parameter by const reference so that you don't accidentally modify it:

vector<int> plusOne(vector<int> const&  digits)

The less than operator is not defined for iterators.
You are just getting lucky.

for (it = digits.end() - 1; it >= digits.begin(); --it)

To iterate over a container in the reverse direction use rbegin() and rend()

for (it = digits.rbegin(); it != digits.rend(); ++it)

To help with efficiency you could reserve space in your result:

result.reserve(digits.size()+1); // The +1 is for cases where the result overflows.
                                 // Note: reserve() does not change the size of the
                                 //       vector just the underlying capacity.

Personally I would have refactored:

    if (sum > 9)
    {
        plus = 1;
        result.push_back(0);
    }
    else
    {
        result.push_back(sum);
        plus = 0;
    }

the body into:

    plus = (sum == 10) ? 1 : 0
    sum  = (sum == 10) ? 0 : sum;

    result.push_back(sum);


Approximating constant π² to within error This function takes as input float error and approximates constant π² to within error by computing this sum, term by term, until the difference between the new and the previous sum is less than error. The function should return the new sum.

$$ \pi^2 = 8 + \dfrac{8}{3^2} + \dfrac{8}{5^2} + \dfrac{8}{7^2} + \dfrac{8}{9^2} + \cdots $$

Example:

approxPIsquared(0.0001)

Result is 9.855519952254232

My working solution is shown below:

def approxPIsquared(error):
    prev = 8
    new =0
    n = 3
    while (True):
        new = (prev + (8 / (n * n)))
        diff = new - prev
        if (diff <= error):
            break
        prev = new
        n = n + 2

    return new

Is this a good solution or is there a better way to do this?

You don't need to compare prev and new during each iteration.

The difference between the new and the previous sum is simply the current term : $$\frac{8}{(2i+1)^2}$$

If you want this term to be smaller than error, you can solve:

$$\mathrm{error} > \frac{8}{(2i+1)^2}\\
\iff (2i+1)^2 > \frac{8}{error}\\
\iff 2i+1 > \sqrt{\frac{8}{error}}\\
\iff i > \frac{\sqrt{\frac{8}{error}} - 1}{2}\\
$$

Now that you know how many terms your series should have, you can return the result directly:

def approx_pi_squared(error):
    n = int(((8 / error)**0.5 - 1) / 2) + 1
    return sum(8 / (2 * i + 1)**2 for i in range(n))

Better formulas

Adding a delta

Note that error represents how small the terms are, not how close approx_pi_squared is from π²:

>>> import math
>>> approx_pi_squared(1e-10)
9.869590258918535
>>> math.pi**2 - approx_pi_squared(1e-7)
0.0004472271895057389
>>> math.pi**2 - approx_pi_squared(1e-10)
1.414217082285063e-05

Even with more than 140 000 terms, the series only gives the 3 first digits of π². This formula is very simple but converges too slowly.

What's very interesting, though, is that the difference between math.pi**2 and approx_pi_squared(error) seems very close to \$\sqrt{2\mathrm{error}}\$. It seems to hold true for any error, so we could update the function:

def approx_pi_squared(error):
    n = int(((8 / error)**0.5 - 1) / 2) + 1
    delta = (2 * error)**0.5
    return sum(8 / (2 * i + 1)**2 for i in range(n)) + delta

approx_pi_squared(1e-10) now returns 10 correct digits for π².

This new formula is unproven, so use at your own risk!

BBP-Type Formula

There are many π² formulas, so feel free to pick another one. For example:

def approx_pi_squared(error):
    n = int(((12 / error)**0.5 - 1) / 2) + 1
    return 12 * sum((-1)**i / (i + 1)**2 for i in range(n))

error seems to have the same order of magnitude as math.pi**2 - approx_pi_squared(error) now:

>>> math.pi**2 - approx_pi_squared(1e-9)
2.0001476030984122e-09
>>> math.pi**2 - approx_pi_squared(1e-10)
-1.9977974829998857e-10

delta looks like (-1)**n * 2 * error.

With Sympy

You can delegate the job to sympy and be sure you'll get a correct result with arbitrary precision:

>>> from sympy import Sum, N, pi, oo, init_printing
>>> from sympy.abc import i, n
>>> init_printing()
>>> Sum(8/(2*i+1)**2, (i, 0, oo))
  ∞
 ____
 ╲
  ╲       8
   ╲  ──────────
   ╱           2
  ╱   (2⋅i + 1)
 ╱
 ‾‾‾‾
i = 0
>>> N(Sum(8/(2*i+1)**2, (i, 0, oo)), 100)
9.869604401089358618834490999876151135313699407240790626413349376220044822419205243001773403718552232
>>> N(pi**2, 100)
9.869604401089358618834490999876151135313699407240790626413349376220044822419205243001773403718552232


Checking conditions before allowing a play button to become visible I have this code to check a couple conditions before allowing a play button to become visible. Is there anything to do to consolidate this code?

var isNodeWebkit = (typeof process == "object"); // check if using node app
var downloadURL = 'https://www.microsoft.com/getsilverlight/get-started/install/'; // setup download URL variable
var isEncoding = false; // setup isEncoding variable

if (!Silverlight.isInstalled("1.0")) { // check if silverlight is installed
    $('.playbtn').hide();
    $('.videoposter').append('<button class="installbtn" />');
    if (isNodeWebkit) {
        $('.installbtn').on('click', function () { // open silverlight download externally
            require('nw.gui').Shell.openExternal(downloadURL);
            var win = require('nw.gui').Window.get();
            win.close(); return false;
        });
    } else {
        $('.installbtn').click(function () { // open silverlight download in new tab
            window.open(downloadURL, '_blank');
        });
        $(window).focus(function () {
            if (Silverlight.isInstalled("1.0")) {
                $('.installbtn').remove();
                $('.playbtn').show(); checkEncodingFunc();
            } else if (!Silverlight.isInstalled("1.0")) {
                $('.installbtn').remove(); $('.videoposter').append('<button class="installbtn" />');
                $('.playbtn').hide(); $('.encodingbtn').hide();
            }; ;
        });
    };
} else if (Silverlight.isInstalled("1.0")) {
    $(window).focus(function () { // after installing show play button
        if (!Silverlight.isInstalled("1.0")) {
            $('.playbtn').hide(); $('.installbtn').remove(); $('.encodingbtn').remove();
            $('.videoposter').append('<button class="installbtn" />');
            $('.installbtn').click(function () { // open silverlight download in new tab
                window.open(downloadURL, '_blank');
            });
        } else {
            $('.installbtn').remove();
            $('.playbtn').show();
        };
        if (Silverlight.isInstalled("1.0") && isEncoding == true) { checkEncodingFunc(); };
    });
};

var encodeTimer = setInterval(function () { if (isEncoding == true) { checkEncodingFunc(); }; }, 60000);
if (Silverlight.isInstalled("1.0")) { checkEncodingFunc(); };

function checkEncodingFunc() { // check if video is encoding
    $.ajax({
        url: 'video/',
        statusCode: {
            200: function () { // if video is found display play button
                if (isEncoding == true) {
                    $('.encodingbtn').remove();
                    $('.playbtn').show();
                    isEncoding = false;
                };
            },
            204: function () { // if video is encoding display encoding button
                if (Silverlight.isInstalled("1.0")) {
                    isEncoding = true;
                    $('.playbtn').hide(); $('.encodingbtn').remove();
                    $('.videoposter').append('<button class="encodingbtn" />');
                };
            }
        }
    });
};

Just a few more minor things on top of jt0dd's excellent review.

Anonymous functions in JavaScript are awesome, but your program will be more readable if you give the main functions names and declare in advance. That way the main part of your program will become something like this:

if (!haveSilverlight) {
    $('.playbtn').hide();
    $('.videoposter').append('<button class="installbtn" />');
    if (isNodeWebkit) {
        $('.installbtn').on('click', openExternalSilverlightDownloader);
    } else {
        $('.installbtn').on('click', openSilverlightDownloader);
        $(window).on('focus', setupInstallBtn);
    }
} else {
    $(window).on('focus', setupPlayBtn);
}

Easier to see the overall logic, no?

Don't write if statements this way:

if (something) {
     // ...
} else if (!something) {
     // ...
}

That's what else is for:

if (something) {
     // ...
} else {
     // ...
}

You don't need to evaluate boolean variables like this:

if (isEncoding == true) {
    // ...
}

You can do simply:

if (isEncoding) {
    // ...
}

On this line, you don't need the brackets, and I think the code would be more readable with the comment on the line before the assignment.

var isNodeWebkit = (typeof process == "object"); // check if using node app

Better this way:

// check if using node app
var isNodeWebkit = typeof process == "object";

Especially with long lines like this, definitely don't put comment after the statement, as it forces me to scroll right, which is not convenient. Even better, the a comment saying // setup download URL variable for a variable assignment is pointless, so it's better to just drop it.

var downloadURL = 'https://www.microsoft.com/getsilverlight/get-started/install/'; // setup download URL variable
var isEncoding = false; // setup isEncoding variable

These lines should have been just:

var downloadURL = 'https://www.microsoft.com/getsilverlight/get-started/install/';
var isEncoding = false;

Without comments, assignments are self-explanatory.


Finding Repeat and missing numbers in an array Problem Statement:

  You are given a read-only array of n integers from 1 to n. Each
  integer appears exactly once except A which appears twice and B which
  is missing.

  Return A and B.

  Example

Input:[3 1 2 5 3]
Output:[3, 4]
A = 3, B = 4

I wrote the following code:

def repeated_number(number_array):

   pair_A_B = []

   n = len(number_array)

   for i in number_array:
      if(number_array.count(i)==2):
        pair_A_B.append(i)
        break

   sample = range(1,n+1)

   diff = list(set(sample)-set(number_array))

   pair_A_B.append(diff[0])

   return pair_A_B

sample_input = [3,1,2,3,5]

print(repeated_number(sample_input))

The code works fine for this problem on my laptop but when I try to submit it on a coding forum it says my code is not efficient. How can I make it efficient in terms of time?

The problem lies in these 2 lines of code:

 for i in number_array:
  if(number_array.count(i)==2):

Here for each number you are counting the number of occurrences in the array. Now, by doing that, You are reiterating over the numbers which are already counted. The call to number_array.count() searches the entire array, which means even for the last element, it will start from the first and calculate the count. This makes the time complexity \$O(n^2)\$. But, it can be done in \$O(n)\$ time.

Instead of using count, store the numbers in a hash table. And for each new number check, if it already exists in the hash table. If so, that's your duplicate number - A.

Also, while iterating through the loop, add each number to the other, and get their sum (omitting the double occurrence of A). And since you have a series from 1..n,

sum(1..n) = n * (n + 1) / 2

So, the missing number is:

sum(1..n) - actual sum


Is my script secure to prevent email injection? I have an email contact form that is PHP and very basic. I am concerned, however, that injections might be possible through it. So I was wondering if anyone with far superior PHP skills than myself would be willing to take a look and see if I have any gaps or vulnerabilities in my code that could be exploited. I very much appreciate any help with this.

PHP

<?php

$field_name = isset( $_POST['cf_name'] ) ? preg_replace( "/[^\.\-\' a-zA-Z0-9]/", "", $_POST['cf_name'] ) : "";
$field_email = isset( $_POST['cf_email'] ) ? preg_replace(  "/[^\.\-\_\@a-zA-Z0-9]/" , "", $_POST['cf_email'] ) : "";
$field_message = isset( $_POST['cf_message'] ) ? preg_replace( "/(From:|To:|BCC:|CC:|Subject:|Content-Type:)/", "", $_POST['cf_message'] ) : "";

$mail_to = 'admin@mywebsite.com';
$subject = 'New Contact for My Website from '.$field_name;

$body_message = 'From: '.$field_name."\n";
$body_message .= 'E-mail: '.$field_email."\n";
$body_message .= 'Message: '.$field_message;

$headers = 'From: '.$field_email."\r\n";
$headers .= 'Reply-To: '.$field_email."\r\n";

$mail_status = mail($mail_to, $subject, $body_message, $headers);

?>

For the brief moment I had this on Stack Overflow (before I was told that it should be here) the comment was made that I shouldn't be stripping characters out of emails. So... what should I do then? Is there a simple method to achieve the goal that I am missing? I am very new to this and just want to be as secure as possible.

Validate, but don't change user input

Don't change the value of a user input. Either accept or reject it, but don't try to guess what might be ment. Some examples:

The name could be Søren Kierkegård, which in many evironments will become Sren Kierkegrd due to your 'sanitation'.

An email address can be much more complicated than your test suggests. How about parenthesis, exclamation or question marks? Read this for other thoughts about email validation with regular expressions.

The message still could contain 'Bcc:' or 'Cc:'. Making the expression case insensitive will lead to the next problem: The sequence "People I met in Toronto: A, B, and C." would become "People I met in Toron A, B, and C."

Escape your output instead

Header fields are restricted to a single line (each). So is the name of a person. In these cases, all whitespaces are equivalent (at least for non-malicious input), so replacing newlines and tabs with spaces does not alter the (meaning of the) content.

$name    = isset($_POST['cf_name']) ? $_POST['cf_name'] : '';
$email   = isset($_POST['cf_email']) ? $_POST['cf_email'] : '';
$message = isset($_POST['cf_message']) ? $_POST['cf_message'] : '';

$mail_status = sendMail($name, $email, $message);

function sendMail($name, $email, $message)
{
    // Whitespace pattern, icluding different masking methods
    $whitespace = '~(<CR>|<LF>|0x0A|%0A|0x0D|%0D|\\n|\\r|\s)+~i';

    $name = trim(preg_replace($whitespace, '', $name));
    if (empty($name)) {
        return false;
    }

    $email = trim(preg_replace($whitespace, ' ', $email));
    if (empty($email)) {
        return false;
    }

    $mail_to = 'admin@mywebsite.com';
    $subject = 'New Contact for My Website from ' . $name;

    $body =  "From: $name\n";
    $body .= "E-mail: $email\n";
    $body .= "Message: $message";

    $headers =  "From: $email\r\n";
    $headers .= "Reply-To: '$email\r\n";

    return mail($mail_to, $subject, $body, $headers);
}


Shorten up Mesh Generator Class I'm currently writing a mesh generator class, and I'm making a method that generates vertices, normals and texcoords for a cube. But, as you can see, it's incredibly complicated and hard to follow. Is anyone willing to help me making this shorter?

public static void GenerateCube(float size, out Vector3[] vertices, out Vector2[] texCoords, out Vector3[] normals)
{
    List<Vector3> vertexList = new List<Vector3>();
    List<Vector2> texCoordList = new List<Vector2>();
    List<Vector3> normalList = new List<Vector3>();

    for (int i = 0; i <= 2; i++)
    {
        Vector3 normal = i == 0 ? Vector3.UnitZ : (i == 1 ? -Vector3.UnitY : Vector3.UnitX);

        float ninety = (float)(Math.PI / 2);
        Matrix4 matrix = i == 0 ? Matrix4.Identity : (i == 1 ? Matrix4.CreateRotationX(ninety) : Matrix4.CreateRotationY(ninety));

        Vector3 vertex1a = new Vector3(-1, -1, 1);
        Vector3 vertex2a = new Vector3(-1, 1, 1);
        Vector3 vertex3a = new Vector3(1, 1, 1);
        Vector3 vertex4a = new Vector3(1, -1, 1);

        Vector3 vertex1b = new Vector3(-1, -1, 1);
        Vector3 vertex2b = new Vector3(1, -1, 1);
        Vector3 vertex3b = new Vector3(1, 1, 1);
        Vector3 vertex4b = new Vector3(-1, 1, 1);

        vertex1a = Vector3.Transform(vertex1a, matrix);
        vertex2a = Vector3.Transform(vertex2a, matrix);
        vertex3a = Vector3.Transform(vertex3a, matrix);
        vertex4a = Vector3.Transform(vertex4a, matrix);

        vertex1b = Vector3.Transform(vertex1b, matrix);
        vertex2b = Vector3.Transform(vertex2b, matrix);
        vertex3b = Vector3.Transform(vertex3b, matrix);
        vertex4b = Vector3.Transform(vertex4b, matrix);

        for (int j = -1; j <= 1; j += 2)
        {
            Vector3 vertex1, vertex2, vertex3, vertex4;
            if (j == -1)
            {
                vertex1 = vertex1a;
                vertex2 = vertex2a;
                vertex3 = vertex3a;
                vertex4 = vertex4a;
            }
            else
            {
                vertex1 = vertex1b;
                vertex2 = vertex2b;
                vertex3 = vertex3b;
                vertex4 = vertex4b;
            }

            //Triangle 1
            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 0));
            vertexList.Add(vertex1 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 1));
            vertexList.Add(vertex2 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 1));
            vertexList.Add(vertex3 * j);

            //Triangle 2
            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 0));
            vertexList.Add(vertex1 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 1));
            vertexList.Add(vertex3 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 0));
            vertexList.Add(vertex4 * j);
        }
    }

    vertices = vertexList.ToArray();
    texCoords = texCoordList.ToArray();
    normals = normalList.ToArray();

    for (int i = 0; i < vertices.Length; i++)
        vertices[i] *= size;
}

Im not familiar with XNA or have any experience working with matrixes, vectors and that kind of stuff. But i've made som basic refactorings to make it abit easier to read and maintain.

public static void GenerateCube(float size, out Vector3[] vertices, out Vector2[] texCoords, out Vector3[] normals)
{
        var vertexList   = new List<Vector3>();
        var texCoordList = new List<Vector2>();
        var normalList   = new List<Vector3>();

        const float ninety = (float)(Math.PI / 2);

        for (var i = 0; i <= 2; i++)
        {
            var normal = CreateVector(i);
            var matrix = CreateMatrix(i, ninety);

            var vectorsA = CreateVectorsA();
            var vectorsB = CreateVectorsB();

            vectorsA.ForEach(x => Vector3.Transform(x, matrix));
            vectorsB.ForEach(x => Vector3.Transform(x, matrix));

            for (var j = -1; j <= 1; j += 2)
            {
                var vectors = j == -1 ? vectorsA.ToArray() : vectorsB.ToArray();

                var someCalculations = CalculateSomething(j, normal);
                normalList.AddRange(someCalculations);

                AddTextureCoordinates(texCoordList);
                AddVertexes(j, vertexList, vectors);
            }
        }

        vertices  = vertexList.ToArray();
        texCoords = texCoordList.ToArray();
        normals   = normalList.ToArray();

        SetVectorSize(vertices, size);
    }

    private static void SetVectorSize(IList<Vector3> vertices, float size)
    {
        for (var i = 0; i < vertices.Count; i++)
        {
            vertices[i] *= size;
        }
    }

    private static Vector3 CreateVector(int i)
    {
        return i == 0 ? Vector3.UnitZ : (i == 1 ? Vector3.UnitY : Vector3.UnitX);
    }

    private static Matrix4 CreateMatrix(int i, float ninety)
    {
        return i == 0 ? Matrix4.Identity : (i == 1 ? Matrix4.CreateRotationX(ninety) : Matrix4.CreateRotationY(ninety));
    }

    private static void AddVertexes(int j, ICollection<Vector3> vertexList, IList<Vector3> vectorsArray)
    {
        //Triangle 1
        vertexList.Add(vectorsArray[0] * j);
        vertexList.Add(vectorsArray[1] * j);
        vertexList.Add(vectorsArray[2] * j);

        //Triangle 2
        vertexList.Add(vectorsArray[0] * j);
        vertexList.Add(vectorsArray[2] * j);
        vertexList.Add(vectorsArray[3] * j);
    }

    private static void AddTextureCoordinates(ICollection<Vector2> texCoordList)
    {
        //Triangle 1
        texCoordList.Add(new Vector2(0, 0));
        texCoordList.Add(new Vector2(0, 1));
        texCoordList.Add(new Vector2(1, 1));

        //Triangle 2
        texCoordList.Add(new Vector2(0, 0));
        texCoordList.Add(new Vector2(1, 1));
        texCoordList.Add(new Vector2(1, 0));
    }

    private static IEnumerable<Vector3> CalculateSomething(int j, Vector3 normal)
    {
        var someCalculation = normal * j;
        return Enumerable.Repeat(someCalculation, 6);
    }

    private static List<Vector3> CreateVectorsB()
    {
        return new List<Vector3>
                   {
                       new Vector3(-1, -1, 1),
                       new Vector3(1, -1, 1),
                       new Vector3(1, 1, 1),
                       new Vector3(-1, 1, 1)
                   };
    }

    private static List<Vector3> CreateVectorsA()
    {
        return new List<Vector3>
                   {
                       new Vector3(-1, -1, 1),
                       new Vector3(-1, 1, 1),
                       new Vector3(1, 1, 1),
                       new Vector3(1, -1, 1)
                   };
    }


MVC architecture in PHP I have developed a few web applications according to my own coding style similar to MVC architecture. I'm a self learner. But there is no controller class or model class. In my way, I don't split the URL. I have mentioned below an example URL. Now I'm familiar with this process, but I know this is not a professional level coding, so I tried to move to MVC. Now I can easily manage any javascript, ajax call. I don't know how to use JavaScript and Ajax in MVC.

Basically my question is:  Shall I continue in this way?

My project file structure.

Below example based on this URL

/root
  -/images
  -/include
  ---database.php
  ---user.php
  ---course.php
  -/layout
  ---/user
  ---/course
  ---index.php
  -index.php
  -config.php

Cofig.php

require("include/database.php");
require("include/user.php");
require("include/course.php");

$db = Database::getInstance();
$user = new user();
$course = new course();

index.php

<?php
include('config.php');

//get view page using url
$view_page = isset($_GET['view']) ? $_GET['view']: 'home';

?>

      <?php
        include('layout/index.php');
      ?>

layout/index.php

<?php
    include($_SERVER['DOCUMENT_ROOT'].'/layout/'.$view_page.'.php');
?>

layout/user/index.php

//this page will be displayed : www.mysite.com/index.php?view=user/index&uid=234
// user/index = (folder/page) or (view/action)
// these details will be dispalyed with

<?php

$userDetails = $user->getUserDetails($_GET['uid']);

  echo $userDetails['username'];
  echo $userDetails['email'];

?>

Code Review:

Naming Convention: $variableNames and functionNames() should be in camelCase, ClassNames should be in CamelCaps. Keep this convention to make your code more readable!
Use an autoloader: To stop you from being worried about loading classes, use an autoloader. See spl_autoload_register() for registering an autoloading function.
Don't use Singletons: Do not use Singletons. Singletons are global, they introduce hidden dependencies to your code and make it unpredictable and untestable.
No explicit dependncies in sight: What does your user object need in order to work? I'm guessing a database connection, maybe even a user_id? Why are you not passing those to the user's constructor?

$user = new User($db, $_SESSION["user_id"]); // For example

How to improve

This code, contrary to popular belief, is not MVC. MVC stems from good OOP practices, and the separation of concerns. The idea is to separate your application into three major layers:

The Input: The Controller layer.
The Output: The View layer.
The Processing and Logic: The Model layer.

Where each has its own job and responsibilities, its own set of objects, and its own role in the application.

I propose you start with learning about OOP, avoid PHP frameworks, and once you grasp OOP, start looking at MVC.


Generic Stack implementation in C-style C++ I implemented generic stack in C. Seems to work fine. It doesn't work for strings, though.

It was compiled using Visual C++.

Implementation:

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

struct node{
    void* value;

    node* next;
};

struct stack{
    node* first;
    int elemSize;
    int length;
};

stack* StackNew(int elemSize)
{
    stack* s = (stack*)malloc(sizeof(stack));
    s->elemSize = elemSize;
    s->length = 0;
    return s;
}

void Push(stack* s, void* value)
{
    node* oldFirst = s->first;
    s->first = (node*)malloc(sizeof(node));
    s->first->value = malloc(s->elemSize);
    memcpy(s->first->value, value, s->elemSize);
    s->first->next = oldFirst;
    s->length++;
}

void* Pop(stack* s)
{
    void* value = malloc(s->elemSize);
    memcpy(value, s->first->value, s->elemSize);
    node* oldFirst = s->first;
    s->first = s->first->next;
    free(oldFirst);
    s->length--;
    return value;
}

void Destory(stack* s)
{
    while (s->length > 0) Pop(s);
    free(s);
}

Test:

struct point{
    int x, y;
};

int main()
{
    stack* s = StackNew(sizeof(point));
    for (int i = 0; i < 100; i++){
        point pt;
        pt.x = i;
        pt.y = i * i;
        Push(s, &pt);
    }

    while (s->length > 0) {
        point pt = *(point*)Pop(s);
        printf("(%d,%d)\n", pt.x, pt.y);
    }
    Destory(s);
    return 0;
}

Output:

(99,9801)
(98,9604)
(97,9409)
(96,9216)
(95,9025)
(94,8836)
(93,8649)
(92,8464)
(91,8281)
(90,8100)
(89,7921)
(88,7744)
(87,7569)
(86,7396)
...

as expected.

Please tell me what I did wrong and how I can improve things.

Naming seems inconsistent.

Consider prefixing all exported names with Stack, e.g. StackPush, StackPop, StackDestroy (Destory is definitely a typo).

The entity named first in a stack context is traditionally referred to as top.
Memory management is overengineered.

Every time a value is pushed memory is allocated. That's justifiable for ownership reasons (make sure to document it). However every time a value is popped memory is allocated again. This is very strange. A simple

return s->first->value;

would suffice.

Along the same line Destroy doesn't free memory (re)allocated for individual stack entries. Consider

while(s->length > 0)
    free(Pop(s))

You've mention the problem with strings; I believe it is stemmed from elemSize being a property of the stack itself. Making it a property of an element would solve the problem.
As mentioned in the comments, the C code shall not include C++ headers.


Better Design Pattern For Terminal-Like GUI I am working on a java swing project that looks like a Terminal (but with less functionality).

The GUI contains a jTextArea to display output and a jTextField for user input.

Here is an application of the my GUI to perform a simple task.

Ask the user to enter a number.
If successful, ask the user to enter a smaller
If successful, prompt out the text "done!".

Is there a better way to do this other than state machine?

Here is the code:

package com.TerminalPanelGUI;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class TerminalPanel extends javax.swing.JPanel
{
    private int state;

    /**
     * Creates a new Terminal Panel
     */
    public TerminalPanel()
    {
        state = 0;
        initComponents();

        textArea.setText("Enter a number: \n");

        textField.addActionListener(new ActionListener()
        {
            @Override
            public void actionPerformed(ActionEvent ae)
            {
                System.out.println("In action performed: state is " + state);
                switch(state)
                {
                    case 0:
                        State0();
                        break;
                    case 1:
                        State1();
                        break;
                }
            }
        });
    }

    private int dummy;

    private void State0()
    {
        try
        {
            dummy = Integer.parseInt(textField.getText());
            state = 1;

            //prompt for next state
            textArea.append("Enter a smaller number: \n");
        }
        catch(NumberFormatException e)
        {
            textArea.append("Invalid entry. Try again \n");
            state = 0;
        }
    }

    private void State1()
    {
        try
        {

            if(Integer.parseInt(textField.getText()) < dummy)
            {
                state = 2;

                //prompt for next state
                textArea.append("done!");
            }
            else
            {
                state = 1;

                //prompt for next state
                textArea.append("Enter a smaller number: \n");
            }
        }
        catch(NumberFormatException e)
        {
            textArea.append("Invalid entry. Try again \n");
        }
    }

    /**
     * This method is called from within the constructor to initialize the form.
     * WARNING: Do NOT modify this code. The content of this method is always
     * regenerated by the Form Editor.
     */
    @SuppressWarnings("unchecked")
    // <editor-fold defaultstate="collapsed" desc="Generated Code">
    private void initComponents() {

        scrollPane = new javax.swing.JScrollPane();
        textArea = new javax.swing.JTextArea();
        textField = new javax.swing.JTextField();

        setLayout(new org.netbeans.lib.awtextra.AbsoluteLayout());

        textArea.setEditable(false);
        textArea.setColumns(20);
        textArea.setRows(5);
        scrollPane.setViewportView(textArea);

        add(scrollPane, new org.netbeans.lib.awtextra.AbsoluteConstraints(0, 0, 400, 250));
        add(textField, new org.netbeans.lib.awtextra.AbsoluteConstraints(0, 270, 400, 30));
    }// </editor-fold>

    // Variables declaration - do not modify
    private javax.swing.JScrollPane scrollPane;
    protected javax.swing.JTextArea textArea;
    protected javax.swing.JTextField textField;
    // End of variables declaration
}

Maybe something like this? Note: may add textField as a member of State0 and State1

public interface State
{
    boolean hasNext();
    State next();
}

public final class State0
    implements State
{
    private boolean okToContinue = false;
    private int number;

    @Override
    public boolean hasNext()
    {
        try {
            textArea.append("Enter a number");
            number = Integer.parseInt(textField.getText());
            okToContinue = true;
        } catch (NumberFormatException ignored) {
            textArea.append("Input is not a number");
        }
        return true;
    }

    @Override
    public State next()
    {
        return okToContinue ? new State1(number) : this;
    }
}

The State1 implementation is then obvious. In the main loop you can just do:

State state = new State0();

while (state.hasNext())
    state = state.next();


Reading in a file and performing string manipulation In a question I answered I posted the following code:

#include <iostream>
#include <fstream>
#include <algorithm>
#include <sstream>

int main()
{
    fstream iFile("names.txt", ios::in);

    // This does not work so replacing with code that does
    // iFile >> file;

    // This is not the best way.
    // But the closest to the intention of the original post.

    // Get size of file.
    ifile.seekg(0,std::ios::end);
    std::streampos          length = ifile.tellg();
    ifile.seekg(0,std::ios::beg);

    // Copy file into string.
    std::string       file(length, '\0');
    ifile.read(&buffer[0],length);

    // Original code continues.

    std::istringstream ss(file);
    std::string token;

    std::vector<std::string> names;

    while(std::getline(ss, token, ',')) {
        names.push_back(token);
    }

    for (unsigned int i = 0; i < names.size(); i++) {
         auto it = std::remove_if(names[i].begin(), names[i].end(), [&] (char c) { return c == '"'; });
         names[i] = std::string(names[i].begin(), it);
    }

    for (unsigned int i = 0; i < names.size(); i++) {
        std::cout << "names["<<i<<"]: " << names[i] << std::endl;
    }
}

Would this be inefficient for larger files? Would it be better if I read the file into one string, did all the manipulation on that, and then put it into a vector?

This code looks a little wild. I'd tame it like this:

#include <algorithm> // for std::remove
#include <fstream>   // for std::ifstream
#include <string>    // for std::string and std::getline
#include <utility>   // for std::move
#include <vector>    // for std::vector

std::ifstream infile("thefile.txt");    // we don't really care where you got the name from

std::vector<std::string> names;

for (std::string line; std::getline(infile, line, ','); )
{
    line.erase(std::remove(line.begin(), line.end(), '"'), line.end());
    names.push_back(std::move(line));
}

// done

A few points to note:

Keep it simple. Don't overthink.
Use ready-made algorithms, don't reinvent wheels (std::remove removes values).
Include what you use (std::string, std::getline).
Use algorithms, don't hand-roll your own loops. (erase/remove)
Understand what algorithms can do for you (don't hand-write your own "erase"). Get familiar with the standard library.
Don't leak scopes. The line string is only needed within the loop, and no further.
Efficiency tip: You can move from the line string that you no longer need and save yourself a copy. Also, we process everything in one step; no need to loop repeatedly.

Furthermore, I suggest factoring this logic into a file:

std::vector<std::string> tokenize_file(std::string const & filename, char delimiter)
{
    std::vector<std::string> result;

    // ...

    return result;
}

Usage:

std::vector<std::string> names = tokenize_file("thefile.txt", ',');


Clean Code, step down rule and SRP I am currently reading Uncle Bob's Clean Code and wonder what would be the "cleaner" implementation of the following:

Let's say I want to implement a HTTPRequest class which handles the execution of (well, obviously) http requests.

This was my initial implementation:

class HTTPRequest(object):
    def __init__(self, url, method, **kwds):
        self.logger = logging.getLogger(__name__)
        self.url = url
        self.method = method
        self.request_kwds = kwds
        self.execution_time_seconds = 0.0
        self.response = None
        self.logger.debug("Created instance %s" % self)

    def execute(self):
        self.logger.debug("%s starting request" % self)
        start_time = time.time()
        self.response = HTTPResponse(
            request=self,
            request_response=requests.request(url=self.url, method=self.method, **self.request_kwds)
        )
        self.execution_time_seconds = time.time() - start_time
        self.logger.debug(
            "%s received response %s after %.4f seconds" % (self, self.response, self.execution_time_seconds)
        )
        self.response.raise_for_status()
        return self.response

The execute method currently does two things: issuing the request and capturing the response, and measuring the execution time.
So first of all, this would be a violation of the Single Responsibility Principle.
Furthermore, it is very detailed and on a low level of abstraction.

Now, I could split this up in different sub methods, like this:

class HTTPRequest(object):
    def __init__(self, url, method, **kwds):
        self.logger = logging.getLogger(__name__)
        self.url = url
        self.method = method
        self.request_kwds = kwds
        self._start_time = 0
        self.execution_time_seconds = 0.0
        self.response = None
        self.logger.debug("Created instance %s" % self)

    def execute(self):
        self.logger.debug("%s starting request" % self)
        self._start_time_measurement()
        self._execute_request()
        self._stop_time_measurement()
        self.logger.debug(
            "%s received response %s after %.4f seconds" % (self, self.response, self.execution_time_seconds)
        )
        return self.response

    def _start_time_measurement(self):
        self._start_time = time.time()

    def _execute_request(self):
        self.response = HTTPResponse(
            request=self,
            request_response=requests.request(url=self.url, method=self.method, **self.request_kwds)
        )
        self.response.raise_for_status()

    def _stop_time_measurement(self):
        self.execution_time_seconds = time.time() - self._start_time

Besides having separated the logic of time measurement and request execution, from what I've read so far this version should be considered "cleaner".

But I have two concerns about it:

This class is still extremely simple. This refactoring made it significantly longer (replacing two single line statements with a call to a function which takes up two lines (+1 for spacing). Doing this kind of refactoring to a "real" class with a lot of business logic could blow it up indefinitely (which is then again considered "unclean")
The original version was - in my eyes - already readable enough, as it used intention-revealing names for the variables, making it clear that there is some kind of time measurement going on during the request.

So the question is:
which version do you prefer, and why? How much extraction of logic into sub methods is too much?

For the sake of completeness, here is a (stripped down version of) the HTTPResponse class and necessary imports to make the code executable:

import time
import logging

import requests

class HTTPResponse(object):
    def __init__(self, request, request_response):
        self.request = request
        self._response = request_response

    def raise_for_status(self):
        self._response.raise_for_status()

class HTTPRequest(object):
    # one of the two versions from above

Whether it makes sense to break this up is somewhat contextually dependent on whether you anticipate reusing this pattern elsewhere, IMO.  If it's a one-off it might be cleaner to simply do it in-place as in your first example; I don't think the second example is particularly easier to follow, since your execute method still contains a lot of logging-related code either way, and it has the undesirable side effect of adding more state to your object that could otherwise be locally scoped.

However, if you do plan on reusing this logging logic, a neat way to separate its implementation details from the function that you're trying to log is putting it in a decorator:

import functools

def logged(logger_name):
    logger = logging.getLogger(logger_name)

    def decorator(func):
        @functools.wraps(func)
        def wrapped_func(self, *args, **kwargs):
            start_time = time.time()
            logger.debug("%s starting execution" % self)
            ret = func(self, *args, **kwargs)
            exec_time = time.time() - start_time
            logger.debug(
                "%s returned %s after %.4f seconds" % (self, ret, exec_time)
            )
            return ret
        return wrapped_func
    return decorator

class HTTPRequest(object):
    def __init__(self, url, method, **kwds):
        self.logger = logging.getLogger(__name__)
        self.url = url
        self.method = method
        self.request_kwds = kwds
        self.logger.debug("Created instance %s" % self)

    @logged(__name__)
    def execute(self):
        response = HTTPResponse(
            request=self,
            request_response=requests.request(
                url=self.url,
                method=self.method,
                **self.request_kwds
            )
        )
        response.raise_for_status()
        return response

Now all of the logging logic is completely removed from the body of execute, and it lives in that one little @logged decoration, which you can reuse with any other method on any other object (note that the decorator is written to assume it's being applied to an instance method, because it uses a self parameter in its logging).  Note that it always measures the overall execution time of the decorated function starting before it's called and ending after it returns (I'm not sure if raise_for_status() blocks long enough for it to make a meaningful difference that the stopwatch ends after you return the response rather than after you create it).

There are a lot of potential further directions to take this -- for example, you could create a logging superclass/mixin that also encapsulates the logging in __init__, or that handles logging-related state that needs to persist in between method calls (e.g. if you want to do any sort of aggregation), and thereby have the ability to make your logging significantly more sophisticated without exposing any of that complexity in the rest of your code.  So this approach scales in lots of interesting ways, but again, whether it's worthwhile depends on whether you actually need that scaling.

Making all these things work in a thoroughly typesafe way is also doable but adds a decent amount of complexity that I won't even go into here.

A simpler approach that's less extensible but sticks a lot closer to your original implementation while still getting most of the code out of execute would be to use a higher order function within the class (similar to the decorator approach but with less magic):

class HTTPRequest(object):
    def __init__(self, url, method, **kwds):
        self.logger = logging.getLogger(__name__)
        self.url = url
        self.method = method
        self.request_kwds = kwds
        self.logger.debug("Created instance %s" % self)

    def measure_request_time(self, func, *args, **kwargs):
        self.logger.debug("%s starting request" % self)
        start_time = time.time()
        ret = func(*args, **kwargs)
        self.execution_time_seconds = time.time() - start_time
        self.logger.debug(
            "%s received response %s after %.4f seconds"
            % (self, ret, self.execution_time_seconds)
        )
        return ret

    def execute(self):
        self.response = measure_request_time(
            HTTPResponse,
            request=self,
            request_response=requests.request(
                url=self.url,
                method=self.method,
                **self.request_kwds
            )
        )
        self.response.raise_for_status()
        return self.response


Character class with name, gender, and inventory list I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.

I came out with this implementation:

public class Character {

    private String name;
    private char gender;
    private List<String> inventory = new ArrayList<String>();

    public Character(String name, char gender, List<String> inventory) {
        this.name = name;
        this.gender = gender;
        this.inventory.addAll(inventory); // Is this a good idea?
    }

    public String getName() {
        return name;
    }

    public char getGender() {
        return gender;
    }

    public List<String> getInventory() {
        return inventory;
    }

}

And in the main class:

public class Main {

    public static void main(String[] args) {

        Character coolKid = new Character("Cool Kid", 'f',
                                          Arrays.asList("just", "testing"));

        System.out.println(coolKid.getInventory());
        // returns [just, testing]

    }
}

My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.

Is there a better way to accomplish this?

This is a very straightforward implementation. Not much to improve.

Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).

A few ideas to think about:

Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:

return Collections.unmodifiableList(inventory);

Then the caller can read the list, but not change it.

You might want to implement some more inventory methods to add, remove or find inventory items.

There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.

Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).

And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.

Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.


Scrape an infinite-scroll page My algorithm scrapes an infinite-scroll page but it takes too long. It scrolls three times but I'm wondering if there is a way to do a ScrollBottom() so no need of repeated code.

Regarding the site from the example: Scroll is done by jQuery ScrollExtend goo.gl/Sq4vVx triggered when the users scroll beyond a particular tag. When that happens a particular class is added into the tag and removed after the pagination is done.

I think there's room for improvement code and performance wise.

"use strict";

var Xray = require('x-ray');
var phantom = require('x-ray-phantom');

var phantom_opts = {
    webSecurity: false,
    images: false,
    weak: false
};

var x = Xray().driver(phantom(phantom_opts, function (nightmare, done) {
    done
        .useragent("Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.111 Safari/537.36")
        .goto(nightmare.request.req.url)
        .scrollTo(4000, 0)
        .wait()
        .scrollTo(8000, 0)
        .wait()
        .scrollTo(12000, 0)
        .wait()
}));

x('https://www.compraonline.grupoeroski.com/es/supermercado/2059698-Alimentos-Frescos/2059746-Carnes-y-aves/2059753-Pollo/', '.product_list li',
    [{
        name: '.description_1',
        unitPrice: '.description_2',
        image: '.image_line img@src',
        price: '.product_price_cont p',
        url: '.image_line a@href',
        volumen: '.description_1',
        medida: '.description_1'
    }])(function (err, products) {
        if (err) console.log(err);

        console.log(products.length);

        process.exit(0);
    });

I would want to understand how the infinite scroll is actually being applied.

Do you understand what javascript events actually trigger new items to be added?
Does it make more sense to simply trigger those events vs. worry about physically scrolling the browser?
Is the content being delivered via AJAX? Can you just query the AJAX endpoint more directly to get to the data you want to get?
Is there anything from the ajax response that you need to understand to know when you have reached the end of the list (no more items to be added)?

When you think through these you might find you have a better way to approach the problem.


Prime number generation algorithm I'm making a prime number scanning algorithm in Java. It uses the concept of the Sieve of Eratosthenes to efficiently find the prime numbers.

It works well and can calculate all the prime numbers under 1,000,000 in less than a second on my laptop, but I was wondering how the algorithm could be further improved.

public static void eratoImproved(long max){

    long start = System.currentTimeMillis();

    ArrayList<Long> invalidated = new ArrayList<>(); // where invalidated numbers will be stored.

    // prepare
    double maxFac = Math.sqrt(max);
    int f = 2;
    while(f <= maxFac){
        boolean isNew = true;
        for(long l : invalidated){
            if(f % l == 0){
                isNew = false;
            }
        }
        if(isNew) {
            invalidated.add(Math.round(f + 0.0));
        }
        f++;
    }

    ArrayList<Long> primes = new ArrayList<>();

    long i = 3; // current test

    for(long l : invalidated){
        primes.add(l);
    }

    while(i <= max){
        boolean v = true;
        for(long s : invalidated){
            if(i % s == 0){
                v = false;
                break;
            }
        }

        if(v){
            primes.add(i);
            long m = 2*i;
            if(primes.size() >= 150){
                String s = Math.round(((i + 0.0)/max)*100) + "%: ";
                for(long l : primes){
                    s += l + "/";
                }
                System.out.println(s);
                primes.clear();
            }

        }
        i += 2;
    }
    System.out.println("Completed search!");
    String s = "Remaining primes: ";
    for(long l : primes){
        s += l + "/";
    }
    System.out.println(s);
    primes.clear();

    System.out.println("Conducted search in " + (System.currentTimeMillis() - start)/1000 + " seconds.");
}

I've optimized everything I can think of, but I wanted to get a second opinion.

Answer by janos is good, but is more about style. Here are some functional/performance observations related to your stated goal of efficiency.

invalidated.add(Math.round(f + 0.0)), huh???
f coerced to double, add a zero, call round(). Why?
Suggest change f from int to long, and just use invalidated.add(f).
while(f <= maxFac){ is done by continually coercing f to double and comparing the double values. Change maxFac to long using cast (truncation is ok here, no need for rounding).
for(long l : invalidated){ primes.add(l); } is a long way of saying primes.addAll(invalidated), except you're adding the overhead of unboxing and reboxing the values.
You have two identical loops of invalidated to determine if prime. Move to reusable method (DRY).
Never do String += String in a loop. Use a StringBuilder.
And again, what's with the i + 0.0?

StringBuilder buf = new StringBuilder();
buf.append(Math.round(i * 100.0 / max)).append("%: ");
for (Long prime : primes)
    buf.append(prime).append('/');
System.out.println(buf);


Calculate number of pairs a,b where a^2 - b^2 = n In one site there was one question to make one program where you will be given a positive integer n and we have to calculate the number of such pairs (a, b),
where n = a² - b² and both a and b are positive integers.
for example 15 = 4^2 - 1^2 = 8^2 - 7^2.
Currently this question is closed and in my case I am still not able to do it properly. Still I won't be trying to find solution here but I just want to know that where I am doing wrong with my code.
At first I thought of simply increment a, b by 1 and check every equation.
But time limit is 4 seconds and 1<= n<= 10^13 so it was taking very long time for larger values of n.
Since a^2 - b^2 = (a+b)(a-b) then I took one example 15 and solved for a = n and b = a-1 and observed that I got some special series:
(2n+1): n=1,2,3 ...   (for a = 2 and b = 1)

4n: n=2,3,4 ...       (for a = 3 and b = 1/b = 2)

6n+3: n = 2,3,4 ...   (for a = 4 and b = 1/b = 2/b = 3)

8n: n = 3,4,5 ...

Later I used this concept and made one program. It is working fine with 6 digits numbers(approx).
My program:
n = input()
count = 0
for i in range(1,n):
    first = (n-((2*i)-1))
    second = ((2*i)+2*(i-1))
    if first % second == 0 and first/second >= i:
        count += 1
    elif n%(4*i) == 0 and n/(4*i) >= (i+1):
        count +=1
print count

But for some larger values it is giving some error(16/30). If I make n in range to 100000 then I get few error as compared to previous one (27/30). But still it is taking time for larger input. So does my method is right or do I have to think of something else. If this is wrong then why ?
Explanation
Let us say I want to find for no. 15 then according to formula (a+b)(a-b) = n total no. of possibilities :
(2+1)(2-1) = 3                      # BLOCK 1

(3+1)(3-1) = 8                      #BLOCK 2
(3+2)(3-2) = 5

(4+1)(4-1) = 15
(4+2)(4-2) = 12                     #BLOCK 3
(4+3)(4-3) = 7

And so on ... till
(8+7)(8-7) = 15
So if I go on like this then the last line of each block (in this case it is n = 3,5,7) we found that it is changing as (2n+1) where n = 1,2,3 .... (here we can think n as if from which block we are starting the count)
Now for every last second line (in this case it is 8,12) the equation will be 4n where n = 2,3,4 ....(since first block has only 1 line so there can't be any last second line so we will count from second block)
Repeat this process again and again ....
We get:
2n+1, 4n, 6n+3, 8n, 10n+5, 12n, 14n+7, 16n ... and so on
If we simply try to find these equation every time for different lines then we find out that we can use this concept in program. If any no. 'n' is divisible by this equation it means it has occurred on those lines of block. Means each time it is divisible i will make count + 1.

You were correct to try a^2 - b^2 = (a+b)(a-b) venue: take your n and break it into p and q so that p > q and p * q = n.

Usually there are several ways of doing it and almost each way defines a pair you need. a would be (p + q)/2 and b would be (p - q)/2. Thus, you only need pairs where either both p and q are even, or they are both odd.

You don't really need calculating a and b, or p and q, just number of pairs.

Thus, break your n into prime divisors and count distinct ways to divide those divisors into 2 different sets so that either both sets contain at least one 2, or neither do.

Edit:

Ok, some more math:

Case with n=1 is trivial: no solutions. For all other cases we can assume that n has nonempty set of prime divisors.

Let's start with case when all prime divisors are distinct and 2 is not present.

Each divisor can be placed either in set 1 or in set 2. Since sets will be obviously different, we're guaranteed p > q and since we're working with n's divisors, we can be sure that p * q = n.

Divisors' placement can be described bijectively by a string of bits with the length as number of divisors. Hence, we'll have 2^n placements, 2^(<number of divisors>-1) pairs of p and q ( -1 because swapping sets gives same p and q).

Now what if some prime divisor (except for 2, which is still not present) is present k>1 times?

For all other divisors, situation is exactly as above, but our special divisor will provide *(k+1), instead of *2 to number of pairs. Well, actually, *2 of other divisors is just a special case with k = 1.

So, if 2 is not present, we go through n's prime factorisation and for each divisor and its respective k number of occurences, we multiply our number of placements by k+1. If result is even - there were odd ks, meaning that in no placement our 2 sets were equal. We still counted every pair of p and q twice (once as set1, set2 and once as set2, set1) so we divide k+1 product by 2 and that's our answer. If result is not even - we have even quantity of every divisor, meaning that n is a square. That means we counted twice every pair of p and q but we've also counted pair of sqrt(n) once - which we shouldn't have! We would have to substract 1 before dividing by 2.

If 2 is present, things don't get much complicated either. If there's only one 2 we return 0 - because there's no a and b that work. If there's more than one - we say that one goes to each set every time so we simply drop 2 twos and stop concerning ourselves with them, repeating steps above.

And now for some code:

from collections import Counter
from functools import reduce
from operator import mul

def product(stuff): return reduce(mul, stuff)

def square_pairs_count(n):
    """Return number of pairs (a,b) so that a*a - b*b = n

    (Algo explanation goes here)
    """
    if n % 2 == 0 and n % 4 != 0:
        return 0 # (a+b) is odd and (a-b) is even (or vice versa)? No way!
    factorisation = Counter(prime_factorisation(n))
    if factorisation[2] > 1:
        # We use 2 '2's to guarantee that both (a+b) and (a-b) are even
        factorisation[2] -= 2
    factor_count_product = product(k + 1 for k in factorisation.values())

    if factor_count_product % 2 == 1:
        # n is a square and we counted root-root pair as (a+b) and (a-b), undo that
        factor_count_product -= 1

    return factor_count_product // 2 # we've counted every pair twice, so //2

print(square_pairs_count(155235236))

Of course, this code could be called comprehensible only in context of all math above. In the wild, most of the code would be a docstring explaining what happens.

Number of divisors grows pretty slowly with growth of n. It all boils down to how fast you produce them prime factors.

Your idea with "blocks":

You can notice that "block" number c contains a = c + 1 and all bs that would give n > 0 (but not necessarily integer).

If you shift block numbering so that (2+1)(2-1) is actually block 2 then you will have block number equal to a used in this block with block 1 being empty. (because no b and n would satisfy our needs).

You could iterate through each pair like:

for a in range(2, max_a(n)):
    for b in range(1, a):
        # test our (a-b)(a+b)

However, that means iterating a lot.

You could notice that each block can produce no more than 1 solution, so we could iterate through blocks instead. However, we'd have to check if a block really produces a solution:

for a in range(2, max_a(n)):
    b_squared = a*a - n
    # check if b_squared is actually a square

The trick will be in determining max_a(n).

Surely, if 2*a - 1 > n, then a^2 - (a-1)^2 > n and thus for any b we're interested in, a^2 - b^2 > n.That makes our max_a something like (n + 1)//2, or n//2 + 1 for sake of corner cases.

That's too much! we'll have to check around 10^13 blocks, and that's a lot. Well, checking couple hundred thousand primes is not a simple thing either, but it's way faster.

So, unless you can lower the boundary for a, the code would still take lots of time.

Modulo sieving

If you are interested in generative solutions, I'd recommend picking some number N, rewriting equation modulo N and looking for moduli N for a and b that are possible with given n. If, for example, N is around hundred buth there's only 10 possible modulo pairs that work - you would have to iterate only through 10 (a,b) pairs out of 10 000.

For example, if N = 3 and n mod 3 = 2 then a mod 3 = 0 and either b mod 3 = 1 or b mod 3 = 2. Of each 9 consecutive possible options for (a,b) only 2 remain. Of each 3 consecutive possible options for a only 1 remains.

For example, if N = 5 and n mod 5 = 1 then only a^2 mod 5 = 1, b^2 mod 5 = 0 and a^2 mod 5 = 0, b^2 mod 5 = 4. Of 25 possible options for (a,b) pair this leaves only 4. Of 5 possible options for a, it leaves only 3.

Moreover, if you have calculated allowed modulo pairs for some M and some N you can then calculate allowed modulo pairs for M*N and instead of iterating possible pairs with steps of size N (or M), go with M*N steps.

Combining 2 examples above, you'll get that you only need to check 3 out of 15 options for a.

Using these 2 tricks (modulo sieving and combining) you can iterate with ever-increasing steps without missing anything. However, problem does not ask for generating those pairs, only for counting their number, so that's a bit excessive.

If you find that option interesting, take a closer look at sieves and especially wheels part.


State Monad in TypeScript Here's my implementation of State Monad in TypeScript, based on a canonical Haskell implementation.
I would like it to get code reviewed.

class StateMonad<S, A> {
  constructor(public runState: (s: S) => ({ s: S, a: A })) {
  }

  static return_<S, A>(a: A): StateMonad<S, A> {
    return new StateMonad(s => ({ s, a }));
  }

  bind<B>(func: (a: A) => StateMonad<S, B>): StateMonad<S, B> {
    return new StateMonad<S, B>((s: S) => {
      const { s: s_, a } = this.runState(s);
      return func(a).runState(s_);
    });
  }
}

// aux monad factory
const createCounter = (regex: RegExp) => new StateMonad((s: string) =>
  s.split('')
    .reduce((acc, c) =>
      (regex.test(c)) ? { s: acc.s.replace(c, ''), a: acc.a + 1 } : acc,
      { s, a: 0 })
);

const countLowerCase = createCounter(/[a-z]/);
const countDigits = createCounter(/[0-9]/);

// usage example
const { a } = countLowerCase   /* -- haskell equivalent   */
  .bind(n1 => countDigits      /* do n1 <- countLowerCase */
    .bind(n2 => StateMonad     /*    n2 <- countDigits    */
      .return_(n1 + n2)))      /*    return n1 + n2       */
  .runState("abc123ABC");

I'm probably off my rocker here, but what if rather than nested chains of bind, you used a fork/join type of approach?

static combine<T, S, R>(
    monads: { [P in keyof T]: StateMonad<S, T[P]> },
    selector: (values: T) => R
)
    : StateMonad<S, R> {
    return new StateMonad<S, R>((state) => {
    var ret: any = {};
    for (const key in monads) {
        if (monads.hasOwnProperty(key)) {
        ret[key] = monads[key].runState(state).a;
        }
    }
    return { a: selector(ret), s: state }
    }
    );
}

and

var { a } = StateMonad
  .combine(
    {
      countLowerCase,
      countDigits
    },
    combined =>
      combined.countLowerCase +
      combined.countDigits
  )
  .runState("abc123ABC");


Code A Primitive Calculator ( x3,x2,+1) Using Dynamic programming Question -You are given a primitive calculator that can perform the following three operations with the current number : multiply  by 2, multiply  by 3, or add 1 to . Your goal is given apositive integer , find the minimum number of operations needed to obtain the number
starting from the number 1.
ll n;
cin>>n;

vector<ll>vec;
vec.pb(n);

if(n==1)
{
    cout<<0<<endl<<1;
}

else
{
    while(n!=1)
    {
        if(n%3==0)
        n=n/3;
        else if(n%2==0)
        n=n/2;
        else
        n=n-1;

        vec.pb(n);
    }

    cout<<vec.size()-1<<endl;

    for(int i=(vec.size()-1);i>=0;i--)
    cout<<vec[i]<<" ";
}
return 0;

}
for input 96234 iam getting
15
1 2 4 5 10 11 22 66 198 594 1782 5346 16038 16039 32078 96234
but the optimal soln is
14
1 3 9 10 11 22 66 198 594 1782 5346 16038 16039 32078 96234
i know i am going wrong at the step when 10 is converted into 5 in my code but it should convert it in to 9, Please help me,

Edit
I realized that my distinction between bottom-top and top-bottom might have caused some confusion. Going top to bottom and bottom to top is the exact same thing, just switched the starting and end points. I just made a habit of thinking about it differently. It's an unnecessary distinction (sorry for any confusion). But the general algorithm is the same.
More detailed explanation:
Just as explained below, you can start at 96234. You check the ways to get to 96233. There is only one way (subtract 1) so it is automatically the least moves. The numbers of moves it takes is 1 + number of moves takes to get to 96234, which is 0. So you store 1 into 96233. You do the same thing for 96232, 96231...
...You get to 32078, you check the paths to get there, /3 from 96234, /2 from 64156, -1 from 32079. The paths each have moves 1, 32079, and 64156 respectively. You store the smallest into the current slot.
Eventually, you get to 1, and you compare the values stored at slots 2,2,and 3. You return the lowest one + 1.
Previous Answer
Shouldn't you be going from bottom to top? Your code is doing exactly what you're telling it to do. You check if n%2==0 first, before you ever do n=n-1. That works most of the time, but as you can see, sometimes n-1 is less moves.
You won't be able to know which is the best operation if you go from top to bottom.
Instead, you should start at 1, and calculate the least amount moves it takes to get to 2,3,4... all the way to 96234. This is still of O(n), and memory wise, it is O(n) although I think you can get it down with a trick (not sure).


Getting all primes between 0 - n Is there a faster way to get all primes between 0 - n?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Numerics;

namespace myPrime
{
    public class simplePrimer
    {
        public List<BigInteger> primeList;

        public void findVersion(BigInteger goal)
        {
            BigInteger number = 3;
            primeList = new List<BigInteger> { 2 };

            try
            {
                while (number < goal)
                {
                    if (isPrime(number))
                        primeList.Add(number);
                    number += 2;
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.ToString());
            }
            finally
            {
                Console.WriteLine("i found " + primeList.Count + " Prime Numbers");
                Console.WriteLine(primeList.Last() + "was the last Prime");
            }
        }

        private bool isPrime(BigInteger number)
        {
            foreach (BigInteger prime in primeList)
                if (number % prime == 0)
                    return false;

            return true;
        }
    }
}

You can call it like:

primer.findVersion1(999999);

Leaving aside the algorithm there are a number of points about the code

Naming
Pascal casing is standard for C# - SimplePrimer and FindVersion as opposed to simplePrimer and findVersion.
FindVersion seems unconnected to the functionality.

Structure
public fields are generally frowned upon.  If it needs to be part of the interface, make it a Property so that the implementation can be changed if needed.
Having a separate 'return' as PrimeList seems overly complicated. There is no strong reason for not having List<BigInteger> GetPrimes(BigInteger goal)
Looking at the code, I cannot see anything obvious that might throw an exception that might be caught by the catch (perhaps an out of range on the BigInteger?) Is it needed?
As a shape I would push for any output to be external to the function. Have the calling code catch errors or report results. It makes unit testing a lot simpler


Fully parameterized PowerShell function for file removal I would like your opinion on my Remove-Files PowerShell function. It is used in my File-Removal-PowerShell-Script. It is designed to be very flexible so a script user can have configure it to remove files in multiple and various way as it often the chase.
Remove-Files function
function Remove-Files {
    [CmdletBinding(SupportsShouldProcess = $true)]
    [OutputType([PSCustomObject])]
    param (
        [Parameter(Position = 0, Mandatory, ValueFromPipelineByPropertyName)]
        [string]
        $FolderPath,

        [Parameter(Position = 1, Mandatory, ValueFromPipelineByPropertyName)]
        [string]
        $FileName,

        [Parameter(Position = 2, ValueFromPipelineByPropertyName)]
        [int]
        $OlderThen = 0,

        [Parameter(Position = 3, ValueFromPipelineByPropertyName)]
        [string]
        $Recurse = "false",

        [Parameter(Position = 4, ValueFromPipelineByPropertyName)]
        [string]
        $Force = "false"
    )

    begin {
        $DateToDelete = (Get-Date).AddDays(- $OlderThen)
    }

    process {
        $FolderSpaceFreed = 0
        $FilesRemoved = 0
        $FailedRemovals = 0

        if (-not (Test-Path -Path $FolderPath)) {
            Write-Log -Message "ERROR: $FolderPath folder does not exist"
            return
        }

        $FullPath = Join-Path -Path $FolderPath -ChildPath $FileName

        if ($Recurse -eq "true") {
            $FileList = Get-ChildItem -Path $FullPath -Recurse
        }
        else {
            $FileList = Get-ChildItem -Path $FullPath
        }

        $FileList = $FileList | Where-Object {$_.LastWriteTime -lt $DateToDelete}

        foreach ($File in $FileList) {
            $FileSize  = (Get-Item -Path $File.FullName).Length
            $SpaceFreed = Get-FormattedFileSize -Size $FileSize

            if ($Force -eq "true") {
                Get-Item -Path $File.FullName | Remove-Item -Force
            }
            else {
                Get-Item -Path $File.FullName | Remove-Item
            }

            if (-not (Test-Path -Path $File.FullName)) {
                $Message = "Successfully deleted " + $File.Name + " file - removed $SpaceFreed"
                $FolderSpaceFreed += $FileSize
                $FilesRemoved ++
            }
            else {
                $Message = "Failed to delete " + $File.Name + " file"
                $FailedRemovals ++
            }
            Write-Log -Message $Message
        }

        $SpaceFreed = Get-FormattedFileSize -Size $FolderSpaceFreed

        if ($FilesRemoved -gt 0) {
            Write-Log -Message "Successfully deleted $FilesRemoved files in $FolderPath folder, and $SpaceFreed of space was freed"
        }
        if ($FailedRemovals -gt 0) {
            Write-Log -Message "Failed to delete $FailedRemovals files in $FolderPath folder"
        }
        if ($FilesRemoved -eq 0 -and $FailedRemovals -eq 0) {
            Write-Log -Message "No files for delition were found in $FolderPath folder"
        }
        New-Object -TypeName psobject -Property @{
            FolderSpaceFreed =  $FolderSpaceFreed
            FilesRemoved = $FilesRemoved
            FailedRemovals = $FailedRemovals
        }
    }
}

Before someone says anything about it, I have to tell you why I did not use a switch type for -Recurse and -Force parameters, and it is because the function is feed from .csv file with data, and I did not find a elegant way to pass string value to a switch parameter.

I have found a way to use pass string values to switch parameters to Remove-Files function so I have refactored the function ho have two switch parameters.
The new version
function Remove-Files {
    [CmdletBinding(SupportsShouldProcess = $true)]
    [OutputType([PSCustomObject])]
    param (
        [Parameter(Mandatory = $true,
                   Position = 0,
                   ParameterSetName = "FolderPath",
                   ValueFromPipeline = $false,
                   ValueFromPipelineByPropertyName = $true,
                   HelpMessage = "Full folder path in which file/files are to be deleted")]
        [string]
        $FolderPath,

        [Parameter(Mandatory = $true,
                   Position = 1,
                   ParameterSetName = "FileName",
                   ValueFromPipeline = $false,
                   ValueFromPipelineByPropertyName = $true,
                   HelpMessage = "File name that can include wildcard character")]
        [SupportsWildcards()]
        [string]
        $FileName,

        [Parameter(Mandatory = $false,
                   Position = 2,
                   ParameterSetName = "OlderThen",
                   ValueFromPipeline = $false,
                   ValueFromPipelineByPropertyName = $true,
                   HelpMessage = "Number of days to delete files older than that")]
        [int]
        $OlderThen = 0,

        [Parameter(Mandatory = $false,
                   Position = 3,
                   ParameterSetName = "Recurse",
                   ValueFromPipeline = $false,
                   ValueFromPipelineByPropertyName = $true,
                   HelpMessage = "Switch on to file remove files in subfolders")]
        [switch]
        $Recurse = $false,

        [Parameter(Mandatory = $false,
                   Position = 4,
                   ParameterSetName = "Force",
                   ValueFromPipeline = $false,
                   ValueFromPipelineByPropertyName = $true,
                   HelpMessage = "Switch on to force remove files")]
        [switch]
        $Force = $false
    )

    process {
        $FolderSpaceFreed = 0
        $FilesRemoved = 0
        $FailedRemovals = 0

        if (-not (Test-Path -Path $FolderPath)) {
            Write-Log -Message "ERROR: $FolderPath folder does not exist"
            return
        }

        $FullPath = Join-Path -Path $FolderPath -ChildPath $FileName

        if ($Recurse) {
            $FileList = Get-ChildItem -Path $FullPath -Recurse
        }
        else {
            $FileList = Get-ChildItem -Path $FullPath
        }

        $DateToDelete = (Get-Date).AddDays(- $OlderThen)

        $FileList = $FileList | Where-Object {$_.CreationTime -lt $DateToDelete}

        foreach ($File in $FileList) {
            $FileSize  = (Get-Item -Path $File.FullName).Length
            $SpaceFreed = Get-FormattedFileSize -Size $FileSize

            if ($Force) {
                Get-Item -Path $File.FullName | Remove-Item -Force
            }
            else {
                Get-Item -Path $File.FullName | Remove-Item
            }

            if (-not (Test-Path -Path $File.FullName)) {
                $Message = "Successfully deleted " + $File.Name + " file - removed $SpaceFreed"
                $FolderSpaceFreed += $FileSize
                $FilesRemoved ++
            }
            else {
                $Message = "Failed to delete " + $File.Name + " file"
                $FailedRemovals ++
            }
            Write-Log -Message $Message
        }

        $SpaceFreed = Get-FormattedFileSize -Size $FolderSpaceFreed

        if ($FilesRemoved -gt 0) {
            Write-Log -Message "Successfully deleted $FilesRemoved files in $FolderPath folder, and $SpaceFreed of space was freed"
        }
        if ($FailedRemovals -gt 0) {
            Write-Log -Message "Failed to delete $FailedRemovals files in $FolderPath folder"
        }
        if ($FilesRemoved -eq 0 -and $FailedRemovals -eq 0) {
            Write-Log -Message "No files for delition were found in $FolderPath folder"
        }
        New-Object -TypeName psobject -Property @{
            FolderSpaceFreed =  $FolderSpaceFreed
            FilesRemoved = $FilesRemoved
            FailedRemovals = $FailedRemovals
        }
    }
}


Simple HttpClient usage for integration tests in .NET Core I have this simple setup for a .NET Core project which is just a very basic HttpClient usage to do some simple integration tests to a RESTful API. For now the test project remains separated from the main API project, that's why it's built on .NET Core for now, using NUnit as test framework as well. But because of that I have the freedom to change it anytime.

For now, the implementation is very simple, like this:

public static HttpClient _client1;
public static HttpClient _client2;

[OneTimeSetUp]
public void OneTimeSetUp()
{
    _client1 = new HttpClient {BaseAddress = new Uri("https://address1.com")};
    _client1.DefaultRequestHeaders.Add("API-KEY", "token_here");

    _client2 = new HttpClient {BaseAddress = new Uri("https://address2.com")};
    _client1.DefaultRequestHeaders.Add("API-KEY", "token_here");
}

//[OneTimeTearDown]
//public void OneTimeTearDown()
//{
    //_client1?.Dispose();
    //_client2?.Dispose();
//}

That's the main test class, but considering that later it will become a more robust test suite and it's going to be added on main project, I've found that I can use HttpClientFactory like this, which produces the same test results and I can modelate the services to be added to the API solution later.

public static HttpClient _client1;
public static HttpClient _client2;

[OneTimeSetUp]
public void OneTimeSetUp()
{
    var services = new ServiceCollection();

    services.AddHttpClient("Client1", client =>
    {
        client.BaseAddress = new Uri("https://address1.com/");
        client.DefaultRequestHeaders.Add("KEY", "token_here");
    });

    services.AddHttpClient("Client2", client =>
    {
        client.BaseAddress = new Uri("https://address2.com/");
        client.DefaultRequestHeaders.Add("KEY", "token_here");
    });

    var factory = services.BuildServiceProvider().GetService<IHttpClientFactory>();

    _client1 = factory.CreateClient("Client1");
    _client2 = factory.CreateClient("Client2");
}

//[OneTimeTearDown]
//public void OneTimeTearDown()
//{
    //_client1?.Dispose();
    //_client2?.Dispose();
//}

My main problem is that we have two main API addresses that will be used under production environment and I'm not sure if this implementation is appropriated. Everything works as expected on both scenarios when called on tests...

[TestFixture, Parallelizable(ParallelScope.All)]
public class TestCase : Setup
{
    [Test]
    public async Task Get()
    {
        var response = await _client1.GetAsync("v1/Endpoint");
            response.EnsureSuccessStatusCode();
        var result = await response.Content.ReadAsStringAsync();

        Assert.NotNull(response);
        Console.WriteLine(JToken.Parse(result));
    }
}

So it will be the same for _client2 and in a lot of tests...

As I said, it works as expected on both ways, but since I'm new at this and didn't found much about this kind of integration test separated from the main API reference project, I'm not so sure about how I built it, so any other and more experienced views about it will be very much appreciated.

EDIT: I had the need to run all the tests in parallel, so I was generating an ObjectDisposedException with the Parallelizable attribute. Removing the OneTimeTearDown with the client disposing calls solved the problem, but I have doubts if that's the correct way to use HttpClient as well, as it still gives the expected results.

First of all I would like to say that, in my opinion, testing API in a such way is wrong, because how does such unit test behave when:

developer machine does not have Internet connection?
developer machine does not have proper certificates?
machine that run test does not have acces to such domain?
external API URL changed?
some new requirements (headers, parameters, body) occure (assume that external API is still under development)?
external API has Internet/electricity/domain problem?

The answer is simple - it will fail. And it could lead to BROKEN WINDOWS THEORY which basically means that someone can say "ok that unit tests does not pass, so we can ignore it" or even worse: "(...) add new failing tests!".

So if you want to test external API you should use some monitoring tool and health check it. If any problem occur - contact with API owner to make him solve the problem.

Secondly, you integrate external API with some kind of version - all responses are in JSON for example. The most important thing is to stub all its possible responses - correct ones with corrupted/correct data, without any data, empty response, timeouts, responses in other formats (HTML) etc. to test whether or not your code works correctly and predictable for all these cases. If API contract will change then new use cases you will add to your integration test.

My proposition is to wrap your communication method (HttpClient) into some kind of Facade, let's name it 'ICommunicatorFacade'. Let the parameters be single your custom Request, and let it return your custom Response type (which wraps what you need from HttpClient).

interface ICommunicatorFacade
{
    Task<CustomResponse> Get(CustomRequest request);
}

That you can easily stub with NSubstitute Moq and other libraries. Also you can check whether or not your facade received correct paramters and of course how does ICommunicator user (method that invokes Get method) behave for all use cases I mentioned above.

Good luck!


REST API with Ruby on Rails and ReactJS I'm currently working on building a simple REST API using Ruby on Rails. I noticed that a lot of the things I did were a bit redundant, especially the models. Just to clarify: I want this to be a REST API which returns JSON, without any views (I want to be able to access it from different applications, mainly Apache Cordova). Within Cordova I wanted to use ReactJS.

Sadly, my Rails controllers all look like this:

class StudentsController < ApplicationController
  def index
    @students = Student.all
    render json: @students
  end

  def create
    if @student.present?
      render nothing: true, status: :conflict
    else
      @student = Student.new(student_params)
      if @student.save
        render json: @student
      else
        render nothing: true, status: :bad_request
      end
    end
  end

  def show
    @student = Student.find(params[:id])
    render json: @student.classes
  end

  def update
    @student = Student.find(params[:id])
    @student.update(student_params)
    if @student.save
      render json: @student
    else
      render nothing: true, status: :bad_request
    end
  end

  def delete
    if Student.destroy(params[:id])
      render nothing: true, status: :ok
    else
      render nothing: true, status: :bad_request
    end
  end

  private
    def student_params
      params.permit(:first_name, :last_name, :grade_id)
    end
end

I literally got three of them with only the classname changed. Is there a smarter way to organize this? I may have to change override some functions, but some functions will be identical.

I also have a few database relationships, for example grades and classes to teachers and students. Does anyone have some information about best practices for this? I seem to be creating a lot of "relationship" tables for these many-to-many relationships.

For the RoR Rest API Part I found this tutorial.

I think this is an interesting idea, although I haven't personally used it when building REST APIs.

Here's some code which shows how it could be done.

I tested the following code to ensure that it works. To test it:

make a todo model with a :name column
add resources :todos to the routes
visit localhost:3000/todos

...

# app/controllers/todos_controller.rb
class TodosController < ApplicationController
  include GenericController
  RecordClass = Todo
  def record_params
    params.require(:todo).permit(:name)
  end
end

You can see that there are only two points of customization in each controller.
You'll have to specify the RecordClass and your secure parameters method.

The following "generic controller" was built using Rails' default scaffold controller.
You can customize it to follow your own default controller logic.

# app/controllers/generic_controller.rb
module GenericController

  before_action :set_instance, only: [:show, :edit, :update, :destroy]

  def index
    @records = RecordClass.all
  end

  def show
  end

  def new
    @record = RecordClass.new
  end

  def edit
  end

  def create
    @record = RecordClass.new(record_params)
    respond_to do |format|
      if @record.save
        format.html { redirect_to @record, notice: '#{RecordClass} was successfully created.' }
        format.json { render :show, status: :created, location: @record }
      else
        format.html { render :new }
        format.json { render json: @record.errors, status: :unprocessable_entity }
      end
    end
  end

  def update
    respond_to do |format|
      if @record.update(record_params)
        format.html { redirect_to @record, notice: '#{RecordC;ass} was successfully updated.' }
        format.json { render :show, status: :ok, location: @record }
      else
        format.html { render :edit }
        format.json { render json: @record.errors, status: :unprocessable_entity }
      end
    end
  end

  def destroy
    @record.destroy
    respond_to do |format|
      format.html { redirect_to records_url, notice: #{'RecordClass} was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private
    def set_record
      @record = RecordClass.find(params[:id])
    end


Javascript Tree Class I got caught in the trap of implementing my own data structure. I've created a very general tree structure, where each node gets one parent, some children, and holds a value.

I'm mostly looking for advice on how to make this code more idiomatic, but also functionality that might be missing from this class, or that isn't necessary. I would also like thoughts on how idiomatic the traverse function is, and if there's a better way to implement it.

function Node(value = 0, children = []) {
    this.value = value;
    this.children = children;
    this.parent = undefined;
}

Node.from = function(node) {
    let that = Object.assign(new Node(), node);
    that.parent = undefined;
    that.children = node.children.map(n => (Node.from(n), n.parent = that));
    return that;
};

Node.prototype.add = function(...children) {
    for (child of children) {
        child.parent = this;
        this.children.push(child);
    }
};

Node.Traversal = {
    BreadthFirst: 1,
    DepthFirst: 2
};

Node.prototype.traverse = function(callback, mode = Node.Traversal.BreadthFirst) {
    if (mode == Node.Traversal.BreadthFirst) {
        let nodes = [this];
        while (nodes.length > 0) {
            const current = nodes.shift();
            callback(current);
            nodes = nodes.concat(current.children);
        }
    } else if (mode == Node.Traversal.DepthFirst) {
        callback(this);
        this.children.forEach(n => n.traverse(callback, false));
    }
    return this;
};

Node.prototype.reduce = function(callback, initial, mode) {
    let acc = initial;
    this.traverse(n => acc = callback(acc, n), mode);
    return acc;
};

Node.prototype.every = function(callback) {
    return this.reduce((a, n) => a && callback(n), true);
};

Node.prototype.some = function(callback) {
    return this.reduce((a, n) => a || callback(n), false);
};

Node.prototype.find = function(callback, mode) {
    return this.reduce((a, n) => a || (callback(n)? n: false), false, mode);
};

Node.prototype.includes = function(value) {
    return this.some(n => n.value === value);
};

This is overall well-thought-out, well-organized, and well-written.
There's at least one bug, and one oversight that I would consider a bug:

In traverse your recursive call is passing the wrong second parameter:
} else if (mode == Node.Traversal.DepthFirst) {
    callback(this);
    this.children.forEach(n => n.traverse(callback, false));
}
should read
} else if (mode == Node.Traversal.DepthFirst) {
    callback(this);
    this.children.forEach(n => n.traverse(callback, Node.Traversal.DepthFirst));
}

in add you don't declare child.  Presumably is should read for (let child of children)

As to design decisions, there are several things you might want to consider.

add might be improved with return this.

I think a clone method might be a more common choice here than a static from function.

You might find the ES6 class syntax slightly more explicit.  I wasn't a big fan of it when it was proposed, and I'm still not; I don't like the implication it suggests that JS OOP is particularly similar to Java/C# style OOP.  But it does carry a bit less cruft than the constant repetition of Node.prototype.

Rather than use an enumeration of traversal types, you might want to consider simply using functions instead.  That is,
Node.Traversal = {
    BreadthFirst: function(callback) {
        let nodes = [this];
        while (nodes.length > 0) {
            const current = nodes.shift();
            callback(current);
            nodes = nodes.concat(current.children);
        }
    },
    DepthFirst: function(callback) {
        callback(this);
        this.children.forEach(n => n.traverse(callback,  Node.Traversal.DepthFirst));
    }
};

Node.prototype.traverse = function(callback, traversal = Node.Traversal.BreadthFirst)
{
    traversal.call(this, callback);
    return this;
};

This would make it easier for you to add another traversal, since you only have to do it in one place, and it lets users supply their own instead.

Perhaps most importantly, you might want to drop the parent property.  You do not use it anywhere except in building or cloning a tree, and it can lead to real problems.  For instance, if you cloned a tree with from (and therefore ended up with non-empty parent nodes), you will not be able to call JSON.stringify on it due to the cyclic nature.

Putting this all together, here's an alternate version:

console.clear()

class Node {
  constructor (value = 0, children = []) {
    this.value = value;
    this.children = children;
  }

  static Traversal = {
    BreadthFirst: function(callback) {
        let nodes = [this];
        while (nodes.length > 0) {
            const current = nodes.shift();
            callback(current);
            nodes = nodes.concat(current.children);
        }
    },
    DepthFirst: function(callback) {
        callback(this);
        this.children.forEach(n => n.traverse(callback,  Node.Traversal.DepthFirst));
    }
  }

  clone () {
    let that = Object.assign(new Node(), this);
    that.children = this.children.map(n => n.clone());
    return that;
  }

  add (...children) {
    for (let child of children) {
      this.children.push(child);
    }
    return this;
  }

  traverse (callback, traversal = Node.Traversal.BreadthFirst) {
    traversal.call(this, callback);
    return this;
  }

  reduce (callback, initial, mode) {
    let acc = initial;
    this.traverse(n => acc = callback(acc, n), mode);
    return acc;
  }

  every (callback) {
    return this.reduce((a, n) => a && callback(n), true);
  }

  some (callback) {
    return this.reduce((a, n) => a || callback(n), false);
  }

  find (callback, mode) {
    return this.reduce((a, n) => a || (callback(n)? n: false), false, mode);
  }

  includes (value) {
    return this.some(n => n.value === value);
  }
}

const tree = new Node ('A', [
  new Node('  B', [
    new Node('    D'),
    new Node('    E', [
      new Node('      G'),
    ]),
    new Node('    F')
  ]),
  new Node('  C')
])

tree.children[0].children[2].add(new Node('      H')).add(new Node('      I'))

const log = ({value}) => console.log(value)
console.log('===============================')
console.log('Breadth-first')
console.log('===============================')
tree.traverse(log)
console.log('===============================')
console.log('Depth-first')
console.log('===============================')
tree.traverse(log, Node.Traversal.DepthFirst)
console.log('===============================')
console.log('Clone, Breadth-first')
console.log('===============================')
tree.clone().traverse(log)
console.log('===============================')
console.log('Clone, Depth-first')
console.log('===============================')
tree.clone().traverse(log, Node.Traversal.DepthFirst)


Adding a timeout to an UDP socket server I use a thread to read from a UDP socket. Afterwards the string gets parsed.
In another thread I send to every connected client via this socket. I would like to keep latency  and resources for running the script as low as possible.

It would be nice to add a time-out.

Every attempt to read should be cancelled if longer than e.g. 20 ms, to keep the response time for other clients low, because I believe that the current attempt to read from this socket is blocking the loop. I was reading that some people use select() is there an advantage?

def trnm_thr():                                                         # trnm_thr() sends commands to Arduino
  global udp_clients
  udp_client = None

  while pySerial is not None:
    if not pySerial.writable():
      continue

    try:
      udp_msg, udp_client = udp_sock.recvfrom(512)                      # Wait for UDP packet from ground station
      if udp_client is not None: udp_clients.add(udp_client)            # Add new client to client list
      if not udp_msg: continue                                          # If message is empty continue without parsing
    except socket.timeout:
      logging.error("Write timeout on socket")                          # Log the problem
    else:
      try:
        p = json.loads(udp_msg)                                         # parse JSON string from socket
      except (ValueError, KeyError, TypeError):
        logging.debug("JSON format error: " + udp_msg.strip() )
      else:
        # Serial device stuff ..

I would instead use a single thread for both tasks and employ an epoll selector on the sockets with a certain timeout. To the best of my knowledge, this is more or less what popular networking frameworks (e.g., NodeJS, Nginx, Lighttpd, etc.) do. I always found waiting on a socket for an undetermined amount of time in a separate thread a waste of resources. That being said, YMMV.