Breaking lists into subgroups in F# I'm starting to play with F# to do some data parsing and I was able to create a function to group an array of string into sub groups. The code looks like this:

let breakBy lines pattern =
    let rec breakByRec lines pattern acc =
        match lines with
        | [] -> acc
        | head::tail ->
            match pattern head with
            | true ->
                let newGroup = [head]
                let newList = newGroup :: acc
                breakByRec tail pattern newList
            | false ->
                let lastGroup = List.head acc
                let newGroup = head :: lastGroup;
                let newList = newGroup :: List.tail acc
                breakByRec tail pattern newList
    breakByRec lines pattern [[]]

However, I feel that it could be improved specially regarding the way I handle intermediate values when concatenating the lists and the data structure used to return the grouped data.

The goal, at the end, is to insert each of these "groups" as single entities in the database. Is there a more suitable data structure that I should take a look?

Below is a sample ready to be used and an example output.

let data = [
    "Name=John"; "Age=29"; "City=San Francisco";
    "Name=Jane"; "Age=28"; "City=New York";
    "Name=Mike"; "Age=35"; "City=Miami"
]

let matchName line = Regex.IsMatch(line, "^Name=")
let people = breakBy data matchName

printfn "%A" people

/*
The output looks like this:

val people : string list list = [
   ["City=Miami"; "Age=35"; "Name=Mike"];
   ["City=New York"; "Age=28"; "Name=Jane"];
   ["City=San Francisco"; "Age=29"; "Name=John"];
   []
]
*/

Any suggestions are appreciated.

The fact that a function named breakBy also reverses the order of the data violates the Principle of Least Surprise.  In my opinion, it's a bug.

breakBy (Regex "^Name=").IsMatch can be thought of as an transformation to be applied to a list.  Therefore, to facilitate currying, the order of the parameters to breakBy should be reversed, so that the predicate comes before the data.


Determining if one string occurs at the end of another This is exercise 5-4 from K&R.  I spent hours tweaking it but now it seems to work.  I'm new to pointers and I'd welcome any comments about how to do it better.

/* Function strend(s, t), which returns 1 if the string t
 * occurs at the end of string s and zero otherwise */
int strend(const char *s, const char *t)
{
    const char *s0, *t0;

    if (s == 0) {
        printf("s is NULL pointer\n");
        return (-1);
    }
    if (t == 0) {
        printf("t is NULL pointer\n");
        return (-1);
    }
    s0 = s;
    t0 = t;

    while (*s++)
        ;
    s -= 2;         /* *s points to last real char in s */

    while (*t++)
        ;
    t -= 2;         /* *t points to last real char in t */

    if ((t-t0) > (s-s0))
            return (0);  /* t is longer than s */

    while (t>=t0) {
        if (*s-- != *t--)
            return (0);  /* Mismatch */
    }

    return (1); /* Match */
}

Here is main():

/* Test the function strend(s, t), which returns 1 if the string t
 * occurs at the end of string s and zero otherwise */
#include "jim.h"
#include "subs.c"

char a[MAXLINE], b[MAXLINE];

int main (void)

{
    printf("Return = %1d, Expect = 1\n", strend("12345", "45"));
    printf("Return = %1d, Expect = 0\n", strend("12345", "35"));
    printf("Return = %1d, Expect = 0\n", strend("45", "345"));
    printf("Return = %1d, Expect = 1\n", strend("12345", "12345"));
    printf("Return = %1d, Expect = 1\n", strend("12345", "5"));
    printf("Return = %1d, Expect = 0\n", strend("12345", "4"));
    printf("Return = %1d, Expect = 1\n", strend("12345", ""));
    printf("Return = %1d, Expect = 0\n", strend("12345", "+"));
    printf("Return = %1d, Expect = 0\n", strend("12345", "a2345"));
    printf("Return = %1d, Expect = 1\n", strend("", ""));
    printf("Return = %1d, Expect = 0\n", strend("", "Z"));
    printf("Return = %1d, Expect = 1\n", strend("1", "1"));
    printf("Return = %1d, Expect = 0\n", strend("1", "1A"));
    printf("Return = %1d, Expect = -1\n", strend(0, "1A"));
    printf("Return = %1d, Expect = -1\n", strend("1", 0));
}

Here is the output:

Return = 1, Expect = 1
Return = 0, Expect = 0
Return = 0, Expect = 0
Return = 1, Expect = 1
Return = 1, Expect = 1
Return = 0, Expect = 0
Return = 1, Expect = 1
Return = 0, Expect = 0
Return = 0, Expect = 0
Return = 1, Expect = 1
Return = 0, Expect = 0
Return = 1, Expect = 1
Return = 0, Expect = 0
s is NULL pointer
Return = -1, Expect = -1
t is NULL pointer
Return = -1, Expect = -1

Const correct the pointers. The function is not changing anything in memory, no reason they shouldn't be const. Same with variables you declare in the function; Make them const

int strend(const char* s, const char* t)

Check for NULL in the input pointers.

Use library string functions wherever possible.
Instead of doing:

while(*s++) ;
s-=2;

call strlen(s); and then substract 1 as needed in your algo. Since you are iterating over the string anyway replacing this with a function call has the same efficiency and makes it more readable.

Add { around the while loop. It is future-proof.

Unit-test to confirm correctness.


Refactor two jQuery filter functions into one function I have two separate lists both sitting side by side. How can I refactor the code below to only have one filter function for both lists?

$("input.filterInput").keyup(function() {
    filter(this);
});

function filter(element) {
    var value = $(element).val();
    $(".pdfList > li").each(function (){
        if ($(this).text().indexOf(value) > -1) {
                $(this).show();
        } else {
            $(this).hide();
        }
    });
}

$("input.filterInput2").keyup(function() {
    filter2(this);
});

function filter2(element) {
    var value = $(element).val();
    $(".pdfList2 > li").each(function (){
        if ($(this).text().indexOf(value) > -1) {
                $(this).show();
        } else {
            $(this).hide();
        }
    });
}

You'll have the following two inputs in your HTML.

<input id="filterInput1" />
<input id="filterInput2" />

Then you can wire them both up together, or split it into separate assignments:

$('#filterInput1, #filterInput2').each(function () {
    $(this).keyup(function () {
        filter(this);
    });
});

Finally, check the ID of the element that triggered the function call to see what list you should be trying to highlight:

function filter(element) {
    var value = $(element).val(),
        selector;

    if (element.id === 'filterInput1') {
        selector = '.pdfList';
    } else if (element.id === 'filterInput2') {
        selector = '.pdfList2';
    } else {
        return; // Or throw an error, whatever you feel is more appropriate.
    }

    // Now the dropdown selector is stored in the variable, and will change which
    // it is looking at depending on who called it.
    $(selector + " > li").each(function (){
        if ($(this).text().indexOf(value) > -1) {
                $(this).show();
        } else {
            $(this).hide();
        }
    });
}


SqlCommand ExecuteReaderAsync helper method The following helper method executes commands asynchronously and returns a Task<IEnumerable<T>> where T is a type that receives the row information from the SqlDataReader rows. All works fine.

    public static async Task<IEnumerable<T>> CommandExecuteReaderAsync<T>(SqlConnection conn, SqlTransaction trans, CommandType commandType, string commandText, IEnumerable<SqlParameter> parameters, CancellationToken token, int commandTimout)
    {
        using (var command = new SqlCommand(commandText, conn, trans))
        {
            command.CommandTimeout = commandTimout;
            command.Parameters.AddRange(parameters.ToArray());
            command.CommandType = commandType;
            using (var sr = await command.ExecuteReaderAsync(token))
            {
                var dc = new System.Data.Linq.DataContext(conn);

                return dc.Translate<T>(sr).ToList();
            }
        }
    }

However, I'm not happy with my current approach of creating a new DataContext and using it's Translate<T> method.

Is there a better approach as the creation of the DataContext seems a bit excessive?

DataContext

  [..]the creation of the DataContext seems a bit excessive

Why do you think so? Is creating the SqlCommand not excessive? You are instantiating it the same number of times.

If DataContext.Translate helps you to save time by not writing the mapping your self then I'd say it's fine. Just remember that DataContext is a disposable type too, so you should add one more using for it.

Naming

Apart from the DataContext you should try to make you code more consistent. This means, don't use abbreviations like conn or trans. Use full words especially for parameters.

Style

public static async Task<IEnumerable<T>> CommandExecuteReaderAsync<T>(
  SqlConnection conn,
  SqlTransaction trans,
  CommandType commandType,
  string commandText,
  IEnumerable<SqlParameter> parameters,
  CancellationToken token,
  int commandTimout)

I hope you have some overloads for this method. 7 (!) parameters, this is a lot. You should try to reduce them and in fact you can hide at least three.

If you make it an extension for the SqlConnection then you'll have one parameter less that you have to specify.

If you use TransactionScope then you can get rid of the second parameter, the SqlTransaction.

If you write two more helper methods (ExecuteQueryAsync & ExecuteStoredProcedureAsync) that call this method, then you can get rid of the CommandType parameter.

So no you just have four parameters left that you must specify: commandText, cancelltionToken & commandTimeout & parameters.


Program to evaluate powers of complex numbers I'm trying to develop a simple program to evaluate integral powers of a complex number \$z\$ , that is, \$z^n\$, where \$z\$ is in the algebrical form \$a+i b\$ and \$n \in \mathbb{Z} ^{*} _{+}\$.

How I was trying to proceed:

First, I defined a product function, and next I intended to define a exponentiation function inside of which I iterated the product function (the code can bee seen below). Is it correct? Is there a more efficient alternative for doing that?

    include <stdio.h>
    include <math.h>

    struct complex {
        float real,imag;
    };

    struct complex product (struct complex x, struct complex y){
    /* using the distributive property, (a+ib)(c+id) = (ac-bd)+i(ad+bc)*/
        struct complex z;
        z.real = (x.real * y.real)- (x.imag * y.imag);
        z.imag = (x.real * y.imag) + (x.imag * y.real);
    return z;
    }

    struct complex exponentiation (struct complex z, int n){
        int i;
        struct complex w;
        w = z;
        for (i=1, i<n, i++)
            w = product (w,z);
    return w;
    }

C has a built-in complex type and a cpow() function since C99.  You seem to be reinventing-the-wheel.

Only in rare circumstances should you use float.  In most cases, you should use double instead.

Exponentiation of complex numbers is better done using the polar representation:

$$z = r e^{i\theta}$$

where

$$\begin{align}
z &= a + i b \\
r &= \sqrt{a^2 + b^2} \\
\theta &= \arctan{\frac{b}{a}}
\end{align}$$

then

$$z^n = (r e^{i\theta})^n = r\,^n e^{i\ n \theta}$$

For non-trivial values of \$n\$, you would be better off converting to a polar representation, performing the exponentiation, and converting back to the Cartesian representation.


Celsius and Fahrenheit Converter in JavaScript I am learning JavaScript. Please review my code. Just created this Celsius & Fahrenheit Converter. Any suggestions to improve my code will be really helpful.
const celsiusToFahrenheit = document.getElementById('celsiusToFarhenhite');
const fahrenheitToCelsius = document.getElementById('farhenhiteToCelsius');
const converterButton = document.querySelector('.calculate');

converterButton.addEventListener('click', function(event){
event.preventDefault();

const celsiusToFahrenheitValue = (celsiusToFahrenheit.value * 9/5 + 32);
const fahrenheitToCelsiusValue = (fahrenheitToCelsius.value - 32) * 5/9;

if(celsiusToFahrenheit.value){
    document.write(`<h1>Celcius To Fahrenheit Result: </h1> ${celsiusToFahrenheitValue} &#8457;`);
}else if (fahrenheitToCelsius.value){
    document.write(`<h1>Fahrenheit To Celcius Result: </h1> ${fahrenheitToCelsiusValue} &#8451;`);
}else{
    document.write(`<h1>You must enter a number!</h1>`);
}
})

Use proper spelling - it'll looks more professional and can prevent bugs stemming from the typo. Farhenhite should be Fahrenheit, and Celcius should be Celsius.
Use proper indentation - every new block should open a new indentation level. You're doing this well with the if/elses, but you should also do it for the whole click listener as well:
converterButton.addEventListener('click', function(event){
    event.preventDefault();

    const celsiusToFahrenheitValue = (celsiusToFahrenheit.value * 9/5 + 32);
    const fahrenheitToCelsiusValue = (fahrenheitToCelsius.value - 32) * 5/9;
    // etc

preventDefault? Is the button inside a <form>? If so, the preventDefault is fine. Otherwise, it won't do anything and can be removed.
Avoid document.write - it can be insecure and somewhat confusing. Here, since it's called after the page loads, it will replace the whole document with the new HTML, which is probably not what you want - you'd probably rather preserve the converter and let people continue to convert after pressing the button once. Instead, populate, say, a results element with the results.
Confusing variable names Without reading the variable definition, it isn't entirely clear what the difference between celsiusToFahrenheitValue and celsiusToFahrenheit.value is. Consider only calculating the new value when it's needed, inside the if statements, and calling it something like convertedToFah. Also consider renaming celsiusToFahrenheit to just plain celsius, since the value it will hold is the plain celcius value.

if (celsius.value) {
    const convertedToFah = (celsius.value * 9/5 + 32);
    document.querySelector('.conversion-type').textContent = 'Celsius To Fahrenheit Result:';
    document.querySelector('.result').textContent = `${convertedToFah} ℉`;
}

When one input is changed, consider clearing the other input to make it clear what will be converted when the button is pressed - use an input listener.


Implementing `concat` using `foldTree` on Rose Tree I attempted (incorrectly) to implement foldTree here.

Recently, I updated it per Petr's help:

treeFold :: (b -> [b] -> b) -> (a -> b) -> Tree a -> b
treeFold f g tree = f (g (rootLabel tree)) (map (treeFold f g) (subForest tree))

I implemented toList:

toList :: Tree a -> [a]
toList = treeFold (\x y -> x ++ (foldr (++) [] y)) (\x -> [x])

Test

ghci> stringTree
Node {rootLabel = "foo",
      subForest =
                [Node {rootLabel = "bar", subForest = []},
                 Node {rootLabel = "bippy", subForest = []},
                 Node {rootLForest = []}]}

ghci> toList stringTree
["foo","bar","bippy","baz"]

I'm not sure if it's idiomatic to include the foldr call. Nor am I sure if I like the usage of ++.

Please review this code.

edit: indeed, (++) will cause perfomance degradation on degenerate left-leaning trees. This is overcome with

-- toList = treeFold (\x y -> x ++ (foldr (++) [] y)) (\x -> [x])

toList t = treeFold (\x y -> x . foldr (.) id y) (\x ->([x]++)) t []

but that's equivalent to using the DL monoid mentioned in Petr Pudlák's answer.

Because foldr (++) [] y is just concat y, you can write it down as \x y -> concat (x:y), or, pointfree,

-- treeFold :: (t -> [b] -> b) -> (a -> t) -> Tree a -> b

toList = treeFold ((concat .) . (:)) (: [])

but that's minor. Do take note that treeFold's inferred type is more general than what you specified.

Similarly, to sum the tree of numbers,

sumRoseTree = treeFold ((sum .) . (:)) id

(or replace id with some other number-producing function, in cased the tree carries some other type).


Optimizing code solution for Palindrome Index-Hackerrank I submitted my solution for palindrome Index coding challenge but I get "test cases terminated due to time out error". My code is working and so I don't know what else to do to optimize it. Please help:

function palindromeIndex(s) {
    let palindrome = s === s.split('').reverse().join('')
    if(!palindrome) {
        let i = 0
        let integerIndex = []
        let arr = s.split('')
        while(i < s.length) {
            arr.splice(i,1)
            if(arr.join('') === arr.reverse().join('')) {
               integerIndex.push(i)
            }
            arr = s.split('')
            i++
        }
        return integerIndex.length > 0 ? integerIndex[0] : - 1
    }else {
        return -1
    }
}

It may well be an error @ Hackerrank. If I'm not mistaken the nodejs-code expects you to provide console input. Or you may have accidentally changed something in the surrounding code.

Concerning your code: writing ES20xx, it's good practice to terminate lines with a semicolon (;), because not doing so may result in nasty bugs.

  let palindrome = s === s.split('').reverse().join('')

You don't need this variable. It could've been:

if(s !== s.split('').reverse().join('')) {

Furthermore, if you wanted to declare a variable, it could've been a const here (you are not modifying it afterwards).

Just for fun, here's an alternative approach, using substrings from the original given string:

"hannach,ava,reopaper,annana,ewve,blob,otto,michael,racecaar,wasitacatiwsaw"
  .split(",")
  .forEach(name => console.log(`[${name}] => ${palindromeIndex(name)}`));

function palindromeIndex(s) {
    if (`${[...s]}` === `${[...s].reverse()}`) { return "is palindrome"; }
    let i = 0;
    while(i < s.length) {
      const sx = `${i < 1 ? s.substr(1, 0) : s.substr(0, i)}${s.substr(i + 1)}`;
      const  rsx = `${[...sx].reverse().join("")}`;
      if (sx === rsx) {
        return `removing '${s[i]}' (@ position ${i}): '${sx}'`;
      };
      i += 1;
    }
    return -1;
}
.as-console-wrapper { top: 0; max-height: 100% !important; }


Filter shell script to find lines that contain all specified patterns I wrote a script that does the following:

Run another script on the system
Filter the output to find lines that contain ALL of the given patterns
Pipe the output to a second script on the system

I feel like the way I did it is a dirty hack. I use a control character to join the arguments, and then replace them with / && / for awk. Is there a better way? Also, this script is probably vulnerable to injection; I don't really need to worry about hostile attackers being able to manipulate the input to awk but I do need to worry about typos screwing up the regular expression.

#!/bin/bash

function join {
        local IFS=$'\x02'
        echo "$*" | sed 's/'$'\x02''/\/ \&\& \//g'
}

/path/to/first/script |
awk "/$(join $@)/" |
/path/to/second/script

join is the name of a common Unix command, so using that name for your function could create confusion.

I don't recommend munging the patterns to dynamically write an awk program.  As you pointed out, your technique is vulnerable to injection, resulting in execution of arbitrary awk code.  Instead, I'd write a fixed awk script that does the job.

#!/usr/bin/env awk -f

BEGIN {
    # Treat command-line arguments as patterns rather than input filenames.
    for (i = 1; i < ARGC; i++) {
        patterns[i - 1] = ARGV[i];
    }
    # Truncate argument list, so that awk always read from standard input.
    ARGC = 1;
}

{
    for (i in patterns) {
        if (!match($0, patterns[i])) {
            next;
        }
    }
    print;
}

To search for literal strings instead of regular expressions, use the index(haystack, needle) function instead of match(haystack, needle).


Toggling html buttons and text I went through a question on SO, and optimized the code according to OP's requirements. I would like a review if it is done properly or not.

Here is the code snippet, there are 2 level buttons, when clicked 1st level button, it toggles second level button group.

Second level button group toggles div, which contains checkbox. Any review and suggestions are appreciated.

$(document).ready(function() {

  var groups = ['a', 'b', 'c']; // here

  $.each(groups, function(k, id) {
    $('#' + id).hide();
    $('#' + id + 'l').hide();
  });

  $(".button").click(function() {

    var button_id = $(this).data('id');

    $('#' + button_id).toggle();

    var hide = $.grep(groups, function(value) {
      return value != button_id;
    });

    $.each(hide, function(k, id) {
      $('#' + id).hide();
    });

  });

  var selector = [];

  $.each(groups, function(k) {
    selector.push('.' + groups[k]);
  });

  selector = selector.join(',');

  $(selector).click(function() {

    var button_id = $(this).data('id');
    var class_id = $(this).attr('class');
    var flag = $('.' + class_id + 'l').filter('[data-id="' + button_id + '"]').is(':visible');

    $.each(groups, function(k, id) {
      $('#' + id + 'l').children().hide();
    });

    $.each(groups, function(k, id) {
      $('#' + id + 'l').hide();
    });

    $('#' + class_id + 'l').show();

    if (flag)
      $('.' + class_id + 'l').filter('[data-id="' + button_id + '"]').hide();
    else
      $('.' + class_id + 'l').filter('[data-id="' + button_id + '"]').show();
  });

});

<button class="button" data-id="a">Toggle a</button>
<button class="button" data-id="b">Toggle b</button>
<button class="button" data-id="c">Toggle c</button>

<button>Toggle a1</button>

<button>Toggle a2</button>

<button>Toggle a3</button>

<button>Toggle b1</button>

<button>Toggle b2</button>

<button>Toggle b3</button>

<button>Toggle c1</button>

<button>Toggle c2</button>

<button>Toggle c3</button>

this is line a1

this is line a2

this is line a3

this is line b1

this is line b2

this is line b3

this is line c1

this is line c2

this is line c3

It's a bad practice to reuse a variable for multiple purposes, especially so when the types are different too: selector is first an array and then becomes a string.

Avoid duplication logic in both branches of a conditional like here:

if (flag)
     $('.' + class_id + 'l').filter('[data-id="' + button_id + '"]').hide();
else
     $('.' + class_id + 'l').filter('[data-id="' + button_id + '"]').show();

Extract the dom selector with the filter, store the result in a variable, and in the branches of the if-else do the hide and show.

It's also recommended to use braces always, even with single statement if-else.

It seems to me that some operations could be joined together and done in a single .each, for example here:

$.each(groups, function(k, id) {
  $('#' + id + 'l').children().hide();
});

$.each(groups, function(k, id) {
  $('#' + id + 'l').hide();
});


How to remove file extension using C#? I am trying to remove file extension of a file with many dots in it:

string a = "asdasdasd.asdas.adas.asdasdasdasd.edasdasd";

string b = a.Substring(a.LastIndexOf('.'), a.Length - a.LastIndexOf('.'));

string c = a.Replace(b, "");

Console.WriteLine(c);

Is there any better way of doing this?

If you can, just use Path.GetFileNameWithoutExtension

  Returns the file name of the specified path string without the extension.

Path.GetFileNameWithoutExtension("asdasdasd.asdas.adas.asdasdasdasd.edasdasd");

With one line of code you can get the same result.

If you want to create one by yourself, why not just use this?

int index = a.LastIndexOf('.');
b = index == -1 ? a : a.Substring(0, index);

P.S Special thanks to @Anthony and @CompuChip to point me out some mistake i done, bad day maybe.

You take everything which comes from 0 (the start) to the last dot which means the start of the extension


Recamán's Sequence in Racket I just started learning Racket as my first lisp dialect so after getting used to the syntax I implemented the Recamán's sequence.

Apart from the style, I'd also like to know if my code is a linear iteration or not. I kind of get the idea but I don't know how to be sure it is.

#lang racket

(define (recaman-seq size)
  (define (recaman-iter curr-seq n goal)
    (if (= n goal)
        (reverse curr-seq)
        (cond
          [(and (= n 0) (empty? curr-seq))
           (recaman-iter (list n) (+ n 1) goal)]
          [else
           (define a (- (car curr-seq) n))
           (define b (+ (car curr-seq) n))
           (define is-new (not (member a curr-seq)))
           (cond
             [(and (positive? a) is-new)
              (recaman-iter (list* a curr-seq) (+ n 1) goal)]
             [else
              (recaman-iter (list* b curr-seq) (+ n 1) goal)])])))
  (recaman-iter '() 0 (+ size 1)))

(recaman-seq 10)

This code is not exactly linear as member is a o(n) time function and grown in proportion to  curr-seq, making the whole function about O(n^2).

However without an algorithmic trick, the function requires a search of past results, and the best search that I know of on a mutable structure is O(log (n)), so the best you can hope for here is O(n*log(n)). But this is only if you go off to infinity.

This sequence seems reasonably dense, so you could probably create a boolean vector a bit bigger than the sequence initialized to true,and as you add numbers to the sequence flip the value of that index to false.

First to translate to scheme...

(define (recaman-seq size)
  (define (recaman-iter curr-seq n goal)
    (if (= n goal)
        (reverse curr-seq)
        (cond
          ((and (= n 0) (null? curr-seq))
           (recaman-iter (list n) (+ n 1) goal))
          (else
           (let* ((a      (- (car curr-seq) n))
                 (b      (+ (car curr-seq) n))
                 (is-new (not (member a curr-seq))))
           (cond
             ((and (positive? a) is-new)
              (recaman-iter (cons a curr-seq) (+ n 1) goal))
             (else
              (recaman-iter (cons b curr-seq) (+ n 1) goal))))))))
  (recaman-iter '() 0 (+ size 1)))

(recaman-seq 10) ]=>(0 1 3 6 2 7 13 20 12 21 11)

Next making some modifications, using a let to make a temperary vector,  changing the define and call with a named loop,

;lang rsr5

(define (recaman-seq size)
 (let ((rman-vect (make-vector (* 10 size) #t)))
;;hopefully generous enough
  (let recaman-iter ((curr-seq '()) (n 0))
;; goal is unneccesary, if and cond are essentially the same, nest them
    (cond ((> n size)
           (reverse curr-seq))
          ((and (= n 0) (null? curr-seq) (vector-ref rman-vect 0))
           (vector-set! rman-vect 0 #f)
;;Mark number as used, this is a side effect,not a returned value
           (recaman-iter (cons n curr-seq) (+ n 1)))
          (else
           (let* ((a      (- (car curr-seq) n))
                 (b      (+ (car curr-seq) n)))
             (cond ((and (> a 0))  (vector-ref rman-vect a)
                    (vector-set! rman-vect a #f)
                    (recaman-iter (cons a curr-seq) (+ n 1)))
             (else (vector-set! rman-vect b #f)
                   (recaman-iter (cons b curr-seq) (+ n 1))))))))))

Can get the first 500,000 terms that way before I run out of memory.


Node.js Paging with Mongodb I want to create variable paging on Node.js with MongoDB. I use MongoJS on client side create a request:

api\logs?&limit=val1&page=val2&sort=val3&order=val4&field1=val5&field2=val6

where

limit, page - paging sort
order, sort - sorting (sort - field. "title","age" & etc)
field1, filed2 ... - option for filter

are all parameters optionally.

What are the disadvantages of the method of processing the request and retrieving data? The planned volume of the collection of up to 100,000 records, fields, indexes are created.

var listSort = ['time','request'];
var defSort = listSort[0];
var listOrder = ['-1','1'];
var defOrder = listOrder[0];

router.get('/', function(req, res){
   var limit = (req.query.limit === undefined) ? 25 : req.query.limit;
   var page = (req.query.page === undefined) ? 1 : req.query.page;

   var sort =  listSort.indexOf(req.query.sort) == -1 ? defSort :  req.query.sort;
   var order = listOrder.indexOf(req.query.order) == -1 ? defOrder : req.query.order;
   var querySort = {};
   querySort[sort] = parseInt(order);

   var timeBegin = isNaN(Number(req.query.timeBegin)) ? undefined : Number(req.query.timeBegin);
   var timeEnd = isNaN(Number(req.query.timeEnd))  ? undefined : Number(req.query.timeEnd);
   var request = req.query.request;
   var queryFind = {};

   if(timeBegin || timeEnd){
       queryFind['time']={};
       if(timeBegin)
           queryFind['time']['$gte']=timeBegin;
       if(timeEnd)
           queryFind['time']['$lte']=timeEnd;
   }

   if(request){
       queryFind['request']=  new RegExp(request, 'i');
   }

   Log.find(queryFind).sort(querySort)
   .limit(limit).skip(limit*(page-1), function(err, logItems) {
       if (!err) res.json(logItems);
       else res.send({error: err});
   })});

limit - the number of entries per page
page - he current page number
listSort - he list consists of a list of fields available for sorting
defSort - sort by default
listOrder - the list of sort order
defOrder - default order

Filter for 'time'. Start and end. Perhaps no one of significance. Filter 'request'. This text field, search for possible inclusion content.

I don't have any good feed back on the technical aspects of your api calls, which look fine to me.  Being that this is code review here is my code review.

I would set a variable to your req.query parameter, which will make the code look cleaner.

Your ternaries can be shortened to:

req.query.limit || 25;

An undefined variable throws an Error, an undefined object property is falsy.

Always use === over ==

Parse Int must always have a radix, otherwise we cannot guarantee consistent performance. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt)

parseInt(order, 10) //Assuming this is sent in decimal

Always return res.send while you are using in fine in this context, adding the return is a safety measure for the future, all you need is someone to change the logic, not properly test error cases and you have a double callback that sometimes can be hard to debug.

So that results in:

var listSort = ['time','request'];
var defSort = listSort[0];
var listOrder = ['-1','1'];
var defOrder = listOrder[0];

router.get('/', function(req, res){
   var rQuery = req.query || {};
   var limit = rQuery.limit || 25;
   var page = rQuery.page || 1;

   var sort =  listSort.indexOf(rQuery.sort) === -1 ? defSort :  rQuery.sort;
   var order = listOrder.indexOf(rQuery.order) === -1 ? defOrder : rQuery.order;
   var querySort = {};
   querySort.sort = parseInt(order, 10);

   var timeBegin = isNaN(Number(rQuery.timeBegin)) ? undefined : Number(rQuery.timeBegin);
   var timeEnd = isNaN(Number(rQuery.timeEnd))  ? undefined : Number(rQuery.timeEnd);
   var request = rQuery.request;
   var queryFind = {};

   if(timeBegin || timeEnd){
       queryFind.time={};
       if(timeBegin)
           queryFind.time['$gte']=timeBegin;
       if(timeEnd)
           queryFind.time['$lte']=timeEnd;
   }

   if(request){
       queryFind.request =  new RegExp(request, 'i');
   }

   Log.find(queryFind).sort(querySort)
   .limit(limit).skip(limit*(page-1), function(err, logItems) {
       if (!err) {
         return res.json(logItems);
       } else {
         return res.send({error: err});
       }
   })});


Printing a heartbeat (heart star) animation Here is a C program to print a heart star animation using a while loop written by myself.
I've tested it on Ubuntu 18.04 (Bionic Beaver) and Ubuntu 20.04 (Focal Fossa).
To animate it, I print a sequence of heart star patterns in different sizes.
#include <stdio.h>
#include <unistd.h>

int main() {
    int i, j, h, k = 0;

    while (k < 99) {
        /* code */
        h = k % 3;
        printf("\e[1;1H\e[2J");
        printf("\e[?25l");

        i = 0;
        while (i < 3) {
            j = 0;
            while (j < (2 - i) * 2 + h) {
                printf(" ");
                j++;
            }

            j = 0;
            while (j < (i + 2) * 2 - h * 2) {
                printf("* ");
                j++;
            }

            j = 0;
            while (j < (2 - i) * 2 + 1 + h) {
                printf("  ");
                j++;
            }

            j = 0;
            while (j < (i + 2) * 2 - h * 2) {
                printf("* ");
                j++;
            }

            printf("\n");
            i++;
        }
        i = 0;
        while (i < 9) {
            j = 0;
            while (j < i * 2 + h * 2) {
                printf(" ");
                j++;
            }
            j = 0;
            while (j < 17 - i * 2 - h * 4) {
                printf("* ");
                j++;
            }
            printf("\n");
            i++;
        }

        fflush(stdout);
        usleep(200000);
        k++;
    }
}

Welcome to CodeReview! The program doesn't look too bad, however there are some ways to improve it:
Naming things
The biggest issue with your code is that there is a bunch of single-letter variable names (i...k), some magic constants (2, 3, 4, 17 and 99), and nested while-loops. If I wasn't told what this program would do and didn't run it for myself, I would have no clue reading this code what it is going to do. However, if you would give (better) names to things, it might explain what everything means to the reader. That includes you yourself in the future!
Let's start with the outer loop:
int main() {
    for (int i = 0; i < 99; i++) {
         int size = i % 3;
         clear_screen();
         draw_heart(size);
         usleep(200000);
    }
}

Keeping i as a generic loop counter is OK, as this is so commonplace that a longer name is not going to make this more readable. But I renamed h to size. Then I assume you have created two functions, one for clearing the screen, and another for drawing a heart of a given size. If you now read this code, you can very quickly see that you are repeatedly drawing a heart of a varying size, without requiring any comments.
Here is the function to clear the screen:
void clear_screen(void) {
    // Use ANSI escape codes to clear the screen
    printf("\e[1;1H\e[2J");
    printf("\e[?25l");
}

The name of the function alone is already enough to know what the function tries to do, the comment here is to explain the weird characters that are printed. Someone who doesn't recognize these escape codes now knows what these are, and can search for "ANSI escape codes" to find out more about them.
The constants you have in your code could be given names, like so:
void draw_heart(int size) {
    static const int top_size = 3;
    static const int bottom_size = 9;
    ...
}

And then used consistently in the calculations you use, such that if you want to change the size of the heart, you can just change these constants.
Use for-loops where appropriate
You are using a lot of while-loops in cases where you can more clearly write for-loops. Any time you initialize a variable, run while the variable satisfies some condition, and then update that variable, use for, like so:
void draw_heart(int size) {
    // Draw two small upward pointing triangles
    for (int i = 0; i < 3; i++) {
        for (int j = 0; i < (2 - i) * 2 + size; j++) {
           printf(" ");
        }

        ...
    }

    // Draw one large downward pointing triangle
    for (int i = 0; i < 9; i++) {
        ...
    }
}

The advantage is that in one line you combine all this information, so just from reading that line you can see what the start condition is, what the end condition is and how the variable is updated every step.


Shuffling algorithm for a "guess that tune" game I'm making a "guess that tune" game in Visual Basic 6 that is supposed to play each song in a random order:

' from frmGuessGame.frm
Private Sub Swap(ByRef Value1 As Variant, ByRef Value2 As Variant)
Dim Temporary As Variant
Temporary = Value1
Value1 = Value2
Value2 = Temporary
End Sub

Private Sub ShuffleList()
Dim LoopCtr As Integer, SwapWith As Integer
For LoopCtr = LBound(InnerPlaylist) To UBound(InnerPlaylist)
SwapWith = (Rnd * (UBound(InnerPlaylist) - LBound(InnerPlaylist))) + LBound(InnerPlaylist)
Swap InnerPlaylist(LoopCtr), InnerPlaylist(SwapWith)
Next
End Sub

My initialization function does include the Randomize statement, so it should be a good, even shuffle.

' from mdiMain.frm
Private Sub MDIForm_Load()
'initialize pseudo-random number generator
Randomize
frmMusiguess.Show
frmSplash.Show
End Sub

I'm not sure, however, that this is the best way to do it. Am I on the right track?

No. Your shuffle will not be random. You are stepping through each card and swapping it with another random card. This is a common bug found in many shuffling algorithms.

To illustrate why your shuffle is not random, let's assume you are shuffling three songs (1, 2, 3). There are six combinations of three song (123, 132, 213, 231, 321, 312). If your shuffle was random, each of these combinations should appear with equal probability. So if you run your shuffle algorithm 600,000 times, each combination should appear roughly 100,000 times each.

But it doesn't. When you run your shuffle algorithm 600,000 times (as written), you will see results similar to this:

To understand why your implementation produces biased results, read this article on Coding Horror: The Danger of Naïveté.

What you want is a Fisher-Yates shuffle where you swap the current card with any of the remaining cards (or itself).

Here it is in pseudo code for an in-place shuffle:

To shuffle an array a of n elements:
  for i from n − 1 downto 1 do
       j ← random integer with 0 ≤ j ≤ i
       exchange a[j] and a[i]

There are other types of Fisher-Yates shuffles listed here: Wikipedia: Fisher–Yates shuffle.


Python generator for "barely obtuse" triangles (Project Euler 224) I am trying to solve Project Euler Problem 224:

  Let us call an integer sided triangle with sides a ≤ b ≤ c barely obtuse if the sides satisfy a2 + b2 = c2 - 1.

  How many barely obtuse triangles are there with perimeter ≤ 75,000,000?

For large v_max and v_max_n, the generator below yields very slow when I have 750000000 and 500000000, respectively. Having tried for days, I am still looking for an algorithm that can do this in under a minute.

from itertools import product
import math

n = 15 * 10 ** 8
v_max = math.floor(n / 2 - 1 / (2 * n))
v_max_n = n // 3

(i for i in product(range(2, v_max_n + 1, 2),
                    range(2, v_max + 1, 2),
                    range(3, v_max + 1))
 if i[0] <= i[1] and i[1] <= i[2] and sum(i) <= n and
 i[0] ** 2 + i[1] ** 2 == i[2] ** 2 - 1)

I have tried using it as map(lambda x: x, gen), nested for loops, and have generally been racking my brain to no avail.

I tried re-writing b, c as expressions. This tends to be quick but misses some of the triples.

n = 12
v_max = math.floor(n / 2 - 1 / (2 * n))
v_max_n = n // 3
j = 0  # tested this with low n and compared: 12, 150 to see
for a in range(1, v_max_n + 1, 1):
    b = (n ** 2 - 2 * n * a - 1) / (2 * (n - a))
    c = (a ** 2 + b ** 2 + 1) ** 0.5
    if c % 1 == 0 and b % 1 == 0 and a + int(b) + int(c) <= n:
        # print(a, int(b), int(c))
        j += 1
print(j)

It seems like you want to find a solution for a²+b²=c²-1 with some added constraints, like a<=b<=c, a+b+c<=n, etc. by trying all possible combinations of a, b, and c and testing whether they satisfy all your conditions at once. This is pretty wasteful.

Instead of itertools.product, you can use a generator expression with three regular for loops. This has several advantages:

you could move some of the if clauses up after the first or second loop, i.e. you don't have to run the other loops if those conditions already fail at this point, e.g.

 for x in ... if test(x) for y in ... if test(y) ...

you can use the values of your previous variables in the lower and upper bounds for the later variables (in your case this replaces most of your if conditions)
you can basically calculate the value for c from a and b without testing all possible values

This is how I'd do it:

gen = ((a, b, int(c))
       for a in range(2, v_max_n + 1, 2)
       for b in range(a, min(v_max + 1, n - a - a), 2) # use a for lower and upper bounds
       for c in [(a ** 2 + b ** 2 + 1)**0.5]           # just a single candidate for c
       if  c % 1 == 0)                                 # whole-numbered c found?

Note that the calculation of c using very_large_number**0.5 might be imprecise with float; using decimal might work, though. However, even with those optimizations, testing much fewer values than your original loop (on the order of O(n²) instead of O(n³)), it might not be feasible to find solution for large values of a, b and c.

Also note that I did not thoroughly test this for off-by-one errors, since performance was the main concern.

Another thing that you might try: Invert the loops for a and for b, i.e. instead of testing all b for each a, no matter how large the difference, test all a up to b first. This does not decrease the number of combinations to test, but it seems to yield much more valid combinations in the "smaller" number much faster. Like this (upper bound for b can probably be reduced):

       for b in range(2, v_max + 1, 2)
       for a in range(2, b+1, 2)


Filtering the attributes for a custom tag I have function that takes all of the attributes passed to a custom tag, and returns a select subset. I am try to convert the current function, which does not use closures

string function passThrough(required struct attr)   output="false"  {

  local.result = "";

  for(local.myKey in arguments.attr)    {
    if (variables.myKey.left(5) == "data-" || variables.myKey.left(2) == "on" || variables.myKey.left(3) == "ng-")  {

        local.result &= ' #local.myKey.lcase()#="#arguments.attr[local.myKey].encodeForHTMLAttribute()#"';
        } // end if
    }   // end for

return local.result;
}

Into one that does

string function passThrough(required struct attr)   output="false"  {

  arguments.attr.filter(
    function(key, value) {
        return (key.left(5) == "data-" || key.left(2) == "on" || key.left(3) == "ng-");
        }
    ).each(
    function(key, value)    {
        local.result &= ' #key.lcase()#="#value.encodeForHTMLAttribute()#"';
        }
    );

return local.result;
}

Both of these are invoked with

...  invoke("bootstrap", "passThrough", {attr = attributes});

Is this the right approach? Should arguments passed to the closure functions be scoped?

From a code review perspective, I'd say:

The function, "passthrough", is doing two things and doesn't describe what it does very clearly. I'd suggest two functions here, "filterAttributes" and "structToQueryString" (or whatever their exact purpose is)
The long filter logic could be concisely expressed as a regular expression to avoid the long line of logic ("^((data|ng)-|on)")
As for scoping closure arguments, I'd suggest this was a matter of preference. If the coding guidelines you work with ask for all variables to be scoped, then yes, they should be scoped. I personally find it more readable to not have them. Having small and concise functions should help prevent scoping issues.

Overall, I find the example without closures more readable, but the feedback above would make it more so IMO. Here's how it could look with closures:

struct function stripNonDataNgAndJsAttributes( required struct attributes ) {
    var patternToKeep = "^((data|ng)\-|on)\S";

    return attributes.filter( function( key, value ){
        return key.findNoCase( patternToKeep );
    } );
}

string function structToQueryString( required struct input ) {
    return input.reduce( function( result, key, value ){
        return result & ' #key.lCase()#="#value.encodeForHTMLAttribute()#"';
    }, "" );
}

http://trycf.com/scratch-pad/gist/d6ce9527fa3294b286cc


Square Progress Indicator On the Beme app, while a video plays, the border acts a progress bar, filling from upper left to upper right to lower right to lower left.  I want to achieve that.  And I have, but my code is incredibly messy.  I don't like the repetitive checks and -='s.  Is there anyway to trim that fat from the code?  If you update the progress bar's current progress attribute, it will update how many sides has a colored border.

$(".squareProgress")
 .append("

")
 .change(function () {
  $(this).trigger("updateProgress");
 })
 .on("updateProgress", function () {
  max = $(window).width()*2 + $(window).height()*2 - 30;
  progress = max*(parseInt($(this).attr("progress"))/100);
  $(this).find("li").eq(0).width(progress > $(window).width() ? $(window).width() : progress);
  progress -= $(window).width();
  $(this).find("li").eq(1).height(progress > $(window).height() ? $(window).height() : progress);
  progress -= $(window).height();
  $(this).find("li").eq(2).width(progress > $(window).width() ? $(window).width() : progress);
  progress -= $(window).width();
  $(this).find("li").eq(3).height(progress > $(window).height() ? $(window).height() : progress);
  progress -= $(window).height();
 })
 .trigger("change");
body {
 padding:0;
 margin:0;
}

.sqaureProgress {
 position:absolute;
 width:100%;
 height:100%;
}

.squareProgress ul {
 padding:0;
 margin:0;
 list-style-type:none;
}

.squareProgress ul li {
 position:absolute;
}

.squareProgress ul li:nth-child(1) {
 left:0;
 top:0;
 width:calc(100% - 10px);
 height:10px;
 background:blue;
}

.squareProgress ul li:nth-child(2) {
 right:0;
 top:0;
 height:calc(100% - 10px);
 width:10px;
 background:red;
}

.squareProgress ul li:nth-child(3) {
 right:0;
 bottom:0;
 width:calc(100% - 10px);
 height:10px;
 background:green;
}

.squareProgress ul li:nth-child(4) {
 left:0;
 bottom:0;
 height:calc(100% - 10px);
 width:10px;
 background:yellow;
}

You can take help of variables to reduce the methods calling also to reduce the number of repeated statements you can go with for loop:

$(".squareProgress")
  .append("

")
  .change(function() {
    $(this).trigger("updateProgress");
  }).on("updateProgress", function() {
    var wW = $(window).width();
    var wH = $(window).height();
    var $li = $(this).find("li");

    max = wW * 2 + wH * 2 - 30;
    progress = max * (parseInt($(this).attr("progress")) / 100);

    for(var i = 0; i < 4; i++){
      if(i % 2 == 0){
        $li.eq(i).width(progress > wW ? wW : progress);
        progress -= wW;
      }else{
        $li.eq(i).height(progress > wH ? wH : progress);
        progress -= wH;
      }
    }
  }).trigger("change");
body {
  padding: 0;
  margin: 0;
}

.sqaureProgress {
  position: absolute;
  width: 100%;
  height: 100%;
}

.squareProgress ul {
  padding: 0;
  margin: 0;
  list-style-type: none;
}

.squareProgress ul li {
  position: absolute;
}

.squareProgress ul li:nth-child(1) {
  left: 0;
  top: 0;
  width: calc(100% - 10px);
  height: 10px;
  background: blue;
}

.squareProgress ul li:nth-child(2) {
  right: 0;
  top: 0;
  height: calc(100% - 10px);
  width: 10px;
  background: red;
}

.squareProgress ul li:nth-child(3) {
  right: 0;
  bottom: 0;
  width: calc(100% - 10px);
  height: 10px;
  background: green;
}

.squareProgress ul li:nth-child(4) {
  left: 0;
  bottom: 0;
  height: calc(100% - 10px);
  width: 10px;
  background: yellow;
}


Handling a PUT API call to write to a database using promises The following function is accepting PUT connection to a server API, checks authorization to write and then performs write to a database.

The eslint linting tool complains saying "avoid nesting promises" and "Each then() should return a value or throw". I'm new to promises and I am not sure how to improve my code.

app.put('/api/v0/note/:id', (req, res) => {
  const id = req.params.id;
  const uid = req.user ? req.user.uid : null;
  return user_can_edit_note(uid, id).then(yes => {
    if (yes) {
        return db.collection('notes').doc(id).update({
          title: req.body.title,
          text: req.body.text,
          author_uid: req.user ? req.user.uid : null,
          updated_on: admin.firestore.Timestamp.now()
        }).then(() => {
          return res.json({
            ok: "ok"
          });
        });
    } else {
      return res.status(403).json({
        error: "Permission Denied",
        note_id: id
      });
    }
  }).catch((err) => {
    console.error(err);
    return res.status(500).json({error: String(err)});
  });
});

// Nothing fancy, we just wrapped the result in an array
const promise1 = someAsyncOperation()
const promise2 = promise1.then(result => [result])
promise2.then(result => console.log(result)) // [result]

// More concisely
someAsyncOperation()
  .then(result => [result])
  .then(resultInArary => console.log(resultInArray)) // [result]

Whatever you return in then becomes the resolved value of the promise it returns. In this case, when promise1 resolved, we added a then that wrapped the result in an array. The returned value becomes the resolved value of promise2.

Now here's where everyone fails when talking about promises. They miss explaining this one behavior which is vital to the concept of "promise chaining":

const promise1 = asyncOperation1()
const promise2 = promise1.then(result1 => asyncOperation2())
promise2.then(result2 => {
  // This will not be invoked until asyncOperation2()'s promise resolves.
  // Also, result2 is NOT asyncOperation2()'s promise, but its resolved value.

  console.log(result2) // [result]
})

// More concisely
someAsyncOperation()
  .then(result1 => asyncOperation2())
  .then(result2 => {
    // This will not be invoked until asyncOperation2()'s promise resolves.
    // Also, result2 is NOT asyncOperation2()'s promise, but its resolved value.

    console.log(result2) // [result]
  })

When you return a promise in a then, two things happen that's different from returning a "normal value".

It uses the resolved value of asyncOperation2() as the resolved value of promise2, not the promise returned by the function call.
promise2 will not resolve and its callbacks will not be invoked until the promise returned by asyncOperation2() resolves.

By now, you should now see how promise "chaining" is achieved. It's because of this one trick, returning promises in then, that allows subsequent thens to "wait" for (more precisely, not fire callbacks until) the previous promise. You could have code that looks like:

asyncOp1()
  .then(r1 => asyncOp2())
  .then(r2 => asyncOp3())
  .then(r3 => asyncOp4())
  .then(r4 => asyncOp5())
  .then(r5 => console.log('done'))
  .catch(e => console.log('one of the above failed'))

So in your code's case, it would look like this:

class ForbiddenError extends Error {
  constructor(message){
    super(message)
    // Add additional properties
  }
}

app.put('/api/v0/note/:id', (req, res) => {
  const id = req.params.id
  const uid = req.user ? req.user.uid : null

  user_can_edit_note(uid, id)
    .then(yes => {

      // The catch() down below will, uhm... catch this throw.
      if(!yes) throw new ForbiddenError()

      // Return the promise of this operation. The next then() handles it.
      return db.collection('notes').doc(id).update({
        title: req.body.title,
        text: req.body.text,
        author_uid: req.user ? req.user.uid : null,
        updated_on: admin.firestore.Timestamp.now()
      })
    })
    .then(() => {
      // Update completed, respond with ok.

      return res.json({
        ok: "ok"
      });
    })
    .catch((err) => {
      // Any throws not recovered in the sequence will invoke this catch.
      // Use this to evaluate your error. You can extend Error to have it carry
      // custom properties and messages.

      if(err instanceof ForbiddenError){
        return res.status(403).json({
          error: "Permission Denied",
          note_id: id
        })
      } else {
        return res.status(500).json({
          error: "Server error"
        })
      }
    })
})


Observer pattern implementation in JavaScript I'm working on redeveloping a running project, and the main goal is to enhance the JS code as much as possible.

I'm using the observer pattern to watch the window events like scroll, and resize

The following is my implementation:

module.exports = {

    // Watch window scrolling/resizing, then fire a set of watchers
    watchWindow: {
        watchers: {
            scroll: {},
            resize: {}
        },
        initialized: false,
        init: function () {
            this.initialized = true;
            var watchers = this.watchers;
            $window.on('scroll resize', function (e) {
                for(var id in watchers[e.type]) {
                    if(watchers[e.type].hasOwnProperty(id)) {
                        if (typeof watchers[e.type][id] === 'function') {
                            watchers[e.type][id](e.type === 'scroll' ? $window.scrollTop() : $window.width());
                        }
                    }
                }
            });
        },
        addWatcher: function (settings) {
            var id = Date.now();
            this.watchers[settings.event][id] = settings.callback;
            !this.initialized && this.init();
            return id;
        },
        removeWatcher: function(settings) {
            delete this.watchers[settings.event][settings.id];
        }
    }
}

Then that's how I'm using it:

// Watch window resizing
utilities.watchWindow.addWatcher({
    event: 'resize',
    callback: function (windowWidth) {
        mobileMenuEnabled = windowWidth < sideMenuBreakpoint;
    }
});

You could add multiple event handlers for each Watcher instead of initializing a global one.

watchWindow: {
    addWatcher: function (settings) {
        $window.on(settings.event, function (e) {
            settings.callback(e.type === 'scroll' ? $window.scrollTop() : $window.width());
        })
    },
    removeWatcher: function(settings) {
        $window.off(settings.event, settings.callback);
    }
}

In this example you would be using the same settings object to add and remove event handlers, but you could of course keep your map of handlers and generated id's if you prefer.


Checking whether a string contains a substring only once I want to check if a string contains only a single occurrence of the given substring (so the result of containsOnce("foo-and-boo", "oo") should be false). In Java, a simple and straightforward implementation would be either

boolean containsOnce(final String s, final CharSequence substring) {
    final String substring0 = substring.toString();
    final int i = s.indexOf(substring0);
    return i != -1 && i == s.lastIndexOf(substring0);
}

or

boolean containsOnce(final String s, final CharSequence substring) {
        final String substring0 = substring.toString();
        final int i = s.indexOf(substring0);
        if (i == -1) {
            return false;
        }

        final int nextIndexOf = s.indexOf(substring0, i + 1);
        return nextIndexOf == 0 || nextIndexOf == -1; // nextIndexOf is 0 if both arguments are empty strings.
}

Can you suggest a simpler/more efficient implementation?

Perhaps using pattern matching. It looks more readable to me, but I have no idea about which is more efficient.

public static boolean containsOnce(final String s, final CharSequence substring) {
    Pattern pattern = Pattern.compile(substring.toString());
    Matcher matcher = pattern.matcher(s);
    if(matcher.find()){
        return !matcher.find();
    }
    return false;
}

public static void main(String[] args) throws Exception {
    System.out.println(containsOnce("aba","a"));    //false
    System.out.println(containsOnce("abab", "ab")); //false
    System.out.println(containsOnce("aba", "b"));   //true
    System.out.println(containsOnce("aaa", "aa"));  //true
    System.out.println(containsOnce("",""));        //true
    System.out.println(containsOnce("ab",""));      //false
}

Also note that containsOnce("aaa","aa") returns true. Not sure if you would count this as correct or not.

Note: you can also write it as

Pattern pattern = Pattern.compile(substring.toString());
Matcher matcher = pattern.matcher(s);
return matcher.find() && !matcher.find();

I don't know which of the two is more readable.

EDIT:

After looking up whichever is faster: indexof vs matcher
It seems to be the general consensus that indexof is a bit faster, but that we're talking about such low times that it really doesn't matter much.

Go for whichever solution you prefer I guess ...


Iterative exponentiation process From SICP's 1.24: (Exponentiation)

(you may need to click through and read ~1 page to understand)

  Exercise 1.16.  Design a procedure
  that evolves an iterative
  exponentiation process that uses
  successive squaring and uses a
  logarithmic number of steps, as does
  fast-expt. (Hint: Using the
  observation that (bn/2)2 = (b2)n/2,
  keep, along with the exponent n and
  the base b, an additional state
  variable a, and define the state
  transformation in such a way that the
  product a bn is unchanged from state
  to state. At the beginning of the
  process a is taken to be 1, and the
  answer is given by the value of a at
  the end of the process. In general,
  the technique of defining an invariant
  quantity that remains unchanged from
  state to state is a powerful way to
  think about the design of iterative
  algorithms.)

I wrote the following solution:

(define (even n) (= (remainder n 2) 0))

(define (fast-expt b n)
  (fast-expt-iter b b n))

(define (fast-expt-iter a b n)
  (cond ((= n 1) a)
    ((even n) (fast-expt-iter (* a b) b (/ n 2)))
    (else (fast-expt-iter (* a b) b (- n 1)))))

Do you think my solution is correct?  Moreover, what do you think about my code?

Your implementation will not produce correct results in general because your recursive definitions are erroneous.

One should note that in case n = 0, result is 1.  In case of even n (or n = 2 i), one may write b ^ n = (b * b) ^ i.  In case of odd n (or n = 2 i + 1), one may write b ^ n = b * (b * b) ^ i.  Here's an implementation (using if's instead of cond and folding the two recursive steps into one):

(define (fast-expt b n)
  (if (= n 0) 1
      (* (if (= (remainder n 2) 0) 1 b) (fast-expt (* b b) (quotient n 2)))))

To make this definition iterative, use the accumulator, a, that is initially 1.  When n is even (n = 2 i), square b but keep a unchanged.  This maintains the property b ^ n = a * (b ^ 2) ^ i.  When n is odd (n = 2 i + 1), square b and multiply a by b.  This maintains the property b ^ n = (a * b) * (b ^ 2) ^ i.  Thus, we have:

(define (fast-expt b n)
  (fast-expt-iter 1 b n))

(define (fast-expt-iter a b n)
  (if (= n 0) a
      (fast-expt-iter (* a (if (= (remainder n 2) 0) 1 b)) (* b b) (quotient n 2))))


Immutable C++ stack - thoughts and performance What are your thoughts on the fallowing immutable stack implementation? It is implemented having as a basis a C# immutable stack (where garbage collector assistance does not impose using a reference counted implementation like here).

namespace immutable
{
    template<typename T>
    class stack: public std::enable_shared_from_this<stack<T>>
    {
        public:

            typedef std::shared_ptr<T> headPtr;
            typedef std::shared_ptr<stack<T>> StackPtr;
            template <typename T> friend struct stackBuilder;

            static StackPtr empty()
            {
                return std::make_shared<stackBuilder<T>>(nullptr, nullptr);
            }

            static StackPtr Create()
            {
                return empty();
            }

            StackPtr push(const T& head)
            {
                return std::make_shared<stackBuilder<T>>(std::make_shared<T>(head), shared_from_this());
            }

            StackPtr pop()
            {
        return this->tail;
    }

    headPtr peek()
    {
                return this->head;
            }

            bool isEmpty()
            {
                return (this->head == nullptr);
            }

        private:
            stack(headPtr head, StackPtr tail): head(head), tail(tail)
            {
            }

            stack<T>& operator= (const stack<T>& other);

        private:
    headPtr head;
    StackPtr tail;
    };

    template <typename T>
    struct stackBuilder: public stack<T>
    {
        stackBuilder(headPtr head, StackPtr tail): stack(head, tail){}
    };
}

Usage:

auto empty = stack<int>::empty();

auto newStack = empty->push(1);
auto stack = newStack;
while(!stack->isEmpty())
{
    std::cout << *stack->peek() << "\n";
    stack = stack->pop();
}

Element mutability

Your stack is not entirely immutable, because peek returns a pointer to a non-const T, which means it's possible to modify the elements of the stack. And given your use of shared_ptr, this may have severe consequences. Consider this code:

using immutable::stack;

void doSomething(stack::StackPtr stack);

int main() {
  auto stack1 = stack<int>::empty()->push(1);
  assert(*stack1->peek() == 1); // succeeds
  doSomething(stack1);
  assert(*stack1->peek() == 1); //FAILS!
}

void doSomething(stack<int>::StackPtr st) {
  *st->peek() = 42;
}

If this is intended behaviour, it should definitely be heavily documented. To me, it looks much more like a bug, however.

One option would be to change headPtr to be a pointer to const:

typedef std::shared_ptr<const T> headPtr;

However, perhaps a more user-friendly solution would be to change peek() to return a const-reference to T instead of a shared pointer. After all, if I put a T on the stack, I would expect to get a T back, not a pointer to T:

const T& peek()
{
    return *this->head;
}

Of course, this would require handling the case of peek-ing an empty stack, or at least documenting that it produces undefined results.
stackBuilder

I understand you're using the stackBuilder class template to be able to use std::make_shared internally, but it opens up a way to bypass your creation mechanism, which is a potential source of bugs. Nothing prevents a user from doing this:

immutable::stackBuilder<int> p(nullptr, nullptr);
p.push(7);

This will fail at runtime because no std::shared_ptr has ever referred to p, so shared_from_this() won't work.

You should make stackBuilder a private member class of stack (it doesn't even have to be a template any more). That way, it's inaccessible from outside, but its constructor can still be public, so std::make_shared works.

If your compiler implements C++11 member access rights correctly, stackBuilder will then no longer have to be declared as friend, because member classes have the same access rights as other members.
Copy constructor

Your class has an inaccessible assignment operator (which is good since it's supposed to be immutable), but the copy constructor is accessible normally. This is again a potential problem, since your class doesn't work if it's not owned by a shared_ptr. So this code will compile, but fail at runtime:

using immutable::stack;

auto stack1 = stack<int>::empty();
auto stack2(*stack1);
stack2.push(7);

You should make the copy constructor inaccessible as well.
Inconsistent capitalisation

StackPtr vs. headPtr, all camelCase member functions vs. Create() etc. You should pick one pattern and stick to it.

Related to this is the fact that your class closely resembles one from the standard library (std::stack), so it might be worthwhile to modify your naming to match the std one. That would mean renaming peek() to top(), and isEmpty() to empty() (and of course renaming your current empty() to something else). But this depends on other parts of your project as well, of course.
Storing non-copyable types

I think push() should take its argument by value, and std::move() it into the new stack; that way, you can store non-copyable types in the stack as well:

StackPtr push(T newHead)
{
    return std::make_shared<stackBuilder<T>>(std::make_shared<T>(std::move(newHead)), shared_from_this());
}

Alternatively, provide two overloads, one taking const T& and one taking T&&. This would potentially be more efficient for types which are expensive to move.

Note that I've renamed the parameter to newHead - when originally reading your code, I was actually confused and though you were pushing this->head. It's best to prevent such naming confusion.
Storing non-movable types

You might also provide an emplace(), which would even allow non-movable types to be stored (especially as the stack is immutable), and be more efficient with types expensive to copy/move:

template <class... Arg>
StackPtr emplace(Arg&&... arg)
{
    return std::make_shared<stackBuilder<T>>(std::make_shared<T>(std::forward<Arg>(arg)...), shared_from_this());
}

Const-correctness

It doesn't matter too much in your case, because access to its instances is stricty via StackPtr, but you might want to mark all the member functions as const to emphasize the fact that the class is immutable. StackPtr would then change like this:

typedef std::shared_ptr<const stack<T>> StackPtr;

This nicely documents that the class is immutable, and allows it to be more easily used inside other const-correct code, mainly const-correct templates.
A note on performance

I can't see any blatant performance issues there. Nevetheless, your users must be aware that the class uses mechanisms which could negatively affect performance-critical code. One is dynamic allocation. The other one are shared pointers, which must internally use atomic operations or synchronisation on their reference counts.

For normal use, this is hardly a problem. If you expect the class to be used in performance-critical code, though, you might want to provide a custom allocator for it which would prevent constant deallocation and re-allocation. Still, I don't think this is worth doing until profiling tells you otherwise.


Reading and writing to file Is there anything I can improve in my UserManager class which saves and loads info about users connected to my server?

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.Socket;
import java.util.ArrayList;
import server.engine.CustomLog;

public class UserManager {

public static synchronized User loadUser(String id, Socket s) {
    User user = null;
    if (id == null) {
        return user;
    }
    File f = new File("users.dat");
    if (f.isFile() && f.canRead()) {
        try (BufferedReader br = new BufferedReader(new FileReader(f))) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.equals("[" + id + "]")) {
                    user = new User(s);
                    user.id = id;
                    user.password = br.readLine().split("=")[1];
                    user.username = br.readLine().split("=")[1];
                    break;
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }
    return user;
}

public static synchronized ArrayList<User> loadAllUsers()
{
    File f = new File("users.dat");
    ArrayList<User> users = new ArrayList();
    if (f.isFile() && f.canRead()) {
        try (BufferedReader br = new BufferedReader(new FileReader(f))) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.matches("^(\\[[0-9]*\\])$")) {
                    User user = new User(null);
                    user.id = line.replace("[", "").replace("]", "");
                    user.password = br.readLine().split("=")[1];
                    user.username = br.readLine().split("=")[1];
                    users.add(user);
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }
    return users;
}

public static synchronized void saveUser(User user) {
    File f = new File("users.dat");
    String content = "";
    String newLine = System.getProperty("line.separator");
    boolean found = false;
    if (f.isFile() && f.canRead()) {
        try (BufferedReader br = new BufferedReader(new FileReader(f))) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.equals("[" + user.id + "]") && br.readLine().equals(user.password)) {
                    found = true;
                    content += "[" + user.id + "]" + newLine;
                    content += "password=" + user.password + newLine;
                    content += "username=" + user.username + newLine;
                    br.readLine();
                } else {
                    content += line + newLine;
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }
    if (!found) {
        content += "[" + user.id + "]" + newLine;
        content += "password=" + user.password + newLine;
        content += "username=" + user.username + newLine;
    }
    try (BufferedWriter writer = new BufferedWriter(new FileWriter(f))) {
        writer.write(content);
        writer.close();
    } catch (FileNotFoundException | UnsupportedEncodingException ex) {
        CustomLog.error(ex.getMessage());
    } catch (IOException ex) {
        CustomLog.error(ex.getMessage());
    }
}
}

EDIT: I forgot to mention that my client and server uses auto-register. Basically id and the password are randomly generated when the client requests registration and server saves them as a new user and client saves them too and keeps using them to log in. So user is only prompted to enter his username. Thats why I was not bothered about privacy of the passwords because they are just random numbers and letters.

First, the obligatory don't store passwords in plain text! If this is any type of production app, you should (and I believe must) follow security best practices for both your sake and the sake of your users.

Now, there is a lot of duplication in this code, and very little code reuse. For example, you have both loadUser and loadAllUsers methods that both load Users from a file. Duplicated parts include getting the file handle with the file name, reading over the file in a loop, parsing the lines for user information, and putting that all into a User object. (saveUser duplicates some of these functionalities as well.) By following the Single Responsibility Principle, we can see that we should extract out some of these responsibilities into individual methods.

User user = new User(null);
user.id = line.replace("[", "").replace("]", "");
user.password = br.readLine().split("=")[1];
user.username = br.readLine().split("=")[1];

This looks like a good candidate for extraction. It's not exactly the same as the code in loadUser, but it's close. The biggest difference is that instead of passing a Socket into the user, we pass a null. The second biggest difference is that the id is parsed from the line rather than passed in directly. The latter can be addressed by just ignoring it. We can extract the id back out even if we are supplied it, and it would make virtually no difference. The former can be addressed by allowing a parameter to be supplied, and supplying null where we expect null. So we can extract the following method:

private static User getUserFromLines(String[] lines, Socket s) {
    User user = new User(s);
    user.id = lines[0].replace("[", "").replace("]", "");
    user.password = lines[1].split("=")[1];
    user.username = lines[2].split("=")[1];
}

It's a little messy in the middle, but it has cleaner edges. We can now use it like so:

public static synchronized User loadUser(String id, Socket s) {
    User user = null;
    if (id == null) {
        return user;
    }
    File f = new File("users.dat");
    if (f.isFile() && f.canRead()) {
        try (BufferedReader br = new BufferedReader(new FileReader(f))) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.equals("[" + id + "]")) {
                    user = getUserFromLines(new String[] {
                        line, br.readLine(), br.readLine()
                    }, s);
                    break;
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }
    return user;
}

public static synchronized ArrayList<User> loadAllUsers()
{
    File f = new File("users.dat");
    ArrayList<User> users = new ArrayList();
    if (f.isFile() && f.canRead()) {
        try (BufferedReader br = new BufferedReader(new FileReader(f))) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.matches("^(\\[[0-9]*\\])$")) {
                    users.add(getUserFromLines(new String[] {
                        line, br.readLine(), br.readLine()
                    }, null));
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }
    return users;
}

This does expose something that is odd about your code: Why does a User take a Socket? I can't speak too much to that because we don't have the code behind User, but it is a rather large code smell indicating a potential design problem.

Now we have that BufferedReader we keep using. Let's encapsulate that behind a method so we don't have to worry so much about how we get it:

private static BufferedReader getUsersFileReader() throws IOException {
    File f = new File("users.dat");
    if(!f.isFile() || !f.canRead()) {
        throw new IOException("Can't find users file!");
    }
    return new BufferedReader(new FileReader(f));
}

Some things to note quickly: If the file does not exist or cannot be read, it now throws an IOException (which gets caught by your try-with-resources block). This is an exceptional case that at least should be logged, and possibly should even be bubbled up. Ignoring it could mask errors, leading to headaches and debugging.

We can use our new method as follows:

public static synchronized User loadUser(String id, Socket s) {
    User user = null;
    if (id == null) {
        return user;
    }

    try (BufferedReader br = getUsersFileReader()) {
        String line;
        while ((line = br.readLine()) != null) {
            if (line.equals("[" + id + "]")) {
                user = getUserFromLines(new String[] {
                    line, br.readLine(), br.readLine()
                }, s);
                break;
            }
        }
    } catch (IOException ex) {
        CustomLog.error(ex.getMessage());
    }

    return user;
}

public static synchronized ArrayList<User> loadAllUsers()
{
    ArrayList<User> users = new ArrayList();

    try (BufferedReader br = getUsersFileReader()) {
        String line;
        while ((line = br.readLine()) != null) {
            if (line.matches("^(\\[[0-9]*\\])$")) {
                users.add(getUserFromLines(new String[] {
                    line, br.readLine(), br.readLine()
                }, null));
            }
        }
    } catch (IOException ex) {
        CustomLog.error(ex.getMessage());
    }

    return users;
}

Now a couple of points about other aspects of your code. All of your methods are synchronized, but you don't appear to have anything that requires synchronizing. This creates overhead without any benefit. What you may actually be looking for is file locking. The file should not be read while it is being written, or it will give corrupt results (reads, on the other hand, can happen in parallel). There are ways to accomplish file locking in Java, but it might be better to look at using a database as a persistence back-end. A database will handle all of the concurrency concerns for you, as well as providing other benefits. Right now, you find entries by iterating through the file. A database could optimize such a look-up with an index. Lightweight, file-based databases, like SQLite, are available if you don't want to run a heavy database server on your machine.

Your loadAllUsers method explicitly returns an ArrayList<User>. While on the surface, this doesn't look like anything to be concerned about, let's say we discover down the line that ArrayList is a performance bottleneck, and we'd rather use LinkedList. Because the method signature is so specific, we can't change the underlying implementation transparently. Instead, we should return a higher-level abstraction like a List<User>. Now we can internally switch the type of list without any external effects.

From the code provided, we can see that your User class exposes public properties for setting state. Doing this means that the User objects cannot have full control over their internals. They cannot guarantee consistency or correctness. It would be better to hide those properties, and to allow setting them through setters or through the constructor.

You will also see that I've added some vertical whitespace, some newlines between chunks of code. Vertical space is free, and it helps to break up the code into readable chunks.

In each of your original methods, you had a line (File f = new File("users.dat")) that contains both a dependency and configuration. This is a sign of two things: the repeated creation of the same state signals that this should probably be an instance object rather than a set of static methods; the configuration data tells us that the dependency should probably be injected (facilitated by making this an instance object, BTW).

So we start with a basic constructor that encapsulates our state:

private File usersFile;

public UserManager() {
    usersFile = new File("users.dat");
}

And we change our code to match. All of the statics go away, and we adjust the getUsersFileReader method to use our new property.

private BufferedReader getUsersFileReader() throws IOException {
    if(!usersFile.isFile() || !usersFile.canRead()) {
        throw new IOException("Can't find users file!");
    }
    return new BufferedReader(new FileReader(usersFile));
}

We can now write a constructor that accepts a File, so we can inject our dependency instead of just accepting the default:

public UserManager(File usersFile) {
    this.usersFile = usersFile;
}

Now that we have pulled this dependency up, we can start to see something interesting about our class. Going back to the Single Responsibility Principle, we can more clearly see that our class has more than one responsibility: it is doing file I/O, and it is doing string parsing to build Users (and the reverse). We should extract out one of those responsibilities into a new class. Because of the name of this class, building Users seems like the right responsibility to keep here. (FYI, this class is a User repository, a factory for building and saving objects. It is a common pattern in the wild.)

So we should extract out the file I/O. So if we extract that, what is the interface we want to deal with? We want something that provides BufferedReaders and BufferedWriters. We don't care where they come from. Let's write that interface:

public interface UserData {
    public BufferedReader getReader();
    public BufferedWriter getWriter();
}

Now let's write the class that fulfills that interface:

public class UserDataFile implements UserData {
    private File usersFile;

    public UserDataFile(File usersFile) {
        this.usersFile = usersFile;
    }

    public BufferedReader getReader() throws IOException {
        if(!usersFile.isFile() || !usersFile.canRead()) {
            throw new IOException("Can't find users file!");
        }

        return new BufferedReader(new FileReader(usersFile));
    }

    public BufferedWriter getWriter() throws IOException {
        if(!usersFile.isFile() || !usersFile.canWrite()) {
            throw new IOException("Can't write to users file!");
        }

        return new BufferedWriter(new FileWriter(usersFile));
    }
}

This is a very simple class, but it hides anything having to do with files from our other class. The extracted interface allows us to switch out back-ends at runtime. We could write different implementations for testing, or for getting our data over the network, or for many other scenarios. It doesn't matter to UserManager. It would look like this now:

public class UserManager {
    private UserData userData;

    public UserManager(UserData userData) {
        this.userData = userData;
    }

    public User loadUser(String id, Socket s) {
        User user = null;
        if (id == null) {
            return user;
        }

        try (BufferedReader br = userData.getReader()) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.equals("[" + id + "]")) {
                    user = getUserFromLines(new String[] {
                        line, br.readLine(), br.readLine()
                    }, s);
                    break;
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }

        return user;
    }

    public List<User> loadAllUsers() {
        ArrayList<User> users = new ArrayList();

        try (BufferedReader br = userData.getReader()) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.matches("^(\\[[0-9]*\\])$")) {
                    users.add(getUserFromLines(new String[] {
                        line, br.readLine(), br.readLine()
                    }, null));
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }

        return users;
    }

    public void saveUser(User user) {
        String content = "";
        String newLine = System.getProperty("line.separator");
        boolean found = false;

        try (BufferedReader br = userData.getReader()) {
            String line;
            while ((line = br.readLine()) != null) {
                if (line.equals("[" + user.id + "]") && br.readLine().equals(user.password)) {
                    found = true;
                    content += "[" + user.id + "]" + newLine;
                    content += "password=" + user.password + newLine;
                    content += "username=" + user.username + newLine;
                    br.readLine();
                } else {
                    content += line + newLine;
                }
            }
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }

        if (!found) {
            content += "[" + user.id + "]" + newLine;
            content += "password=" + user.password + newLine;
            content += "username=" + user.username + newLine;
        }

        try (BufferedWriter writer = userData.getWriter()) {
            writer.write(content);
            writer.close();
        } catch (FileNotFoundException | UnsupportedEncodingException ex) {
            CustomLog.error(ex.getMessage());
        } catch (IOException ex) {
            CustomLog.error(ex.getMessage());
        }
    }

    private User getUserFromLines(String[] lines, Socket s) {
        User user = new User(s);
        user.id = lines[0].replace("[", "").replace("]", "");
        user.password = lines[1].split("=")[1];
        user.username = lines[2].split("=")[1];
    }
}

There's a lot more that could still be done (see the other answers for other pointers), but this begins to move the code forward. It is focused on having a more singular responsibility. It is more modular, and depends more upon abstractions than concretions.


Machine Learning Program I've written a program that finds the difference between data and gives output.
Here's the code:
import json
import numpy as np
print("This is a basic machine learning thing.")
baseData = {"collecting":True,"x":[],"y":[]}
while baseData["collecting"]:
  baseData["x"].append(float(input("X:")))
  baseData["y"].append(float(input("Y:")))
  if input("Do you want to keep feeding data? Press enter for yes, or type anything for no.") != "":
    baseData["collecting"] = False
if len(baseData["x"]) == len(baseData["y"]):
  xdata = baseData["x"]
  ydata = baseData["y"]
  nums = []
  for i in range(len(xdata)):
    nums.append(xdata[i] - ydata[i])
  median = np.median(nums)
else:
  print("malformed data")
def getY(x):
  pass
while True:
  data = input("X/Data:")
  print(int(data)-median)

To work the program, give it X and Y data, then give it X data and it will predict Y data.

Maybe you should check validity of the input and ask again if input is wrong?
while baseData["collecting"]:
  baseData["x"].append(float(input("X:")))

This is always True, so just discard this part:
if len(baseData["x"]) == len(baseData["y"]):

Maybe you should give an option to exit?
while True:
  data = input("X/Data:")
  print(int(data)-median)

And in general, calling this a "machiene-learning thing" is quite a stretch of imagination, no? Maybe you should have a look at how to fit data. I.e. a basic linear model fit.


HttpSession Wrapper Class Are there any aspects I could improve of my HttpSession wrapper class?

Other suggestions are also welcome.

namespace Http;

class HttpSession
{
    private $data;

    public function __construct($name = null, array $iniSettings = [])
    {
        if ($name) {
            session_name($name);
        }

        foreach ($iniSettings as $key => $value) {
            ini_set($key, $value);
        }
    }

    public function findParameter($name)
    {
        $this->start();

        return isset($this->data[$name]) ? $this->data[$name] : null;
    }

    public function setParameter($name, $value)
    {
        $this->start();

        $this->data[$name] = $value;
    }

    public function unsetParameter($name)
    {
        $this->start();

        unset($this->data[$name]);
    }

    public function regenerateId($deleteOldSessionFile = true)
    {
        $this->start();

        session_regenerate_id($deleteOldSessionFile);
    }

    public function destroy()
    {
        $this->start();

        $this->data = [];

        if (isset($_COOKIE[session_name()])) {
            $scParams = session_get_cookie_params();

            setcookie(
                session_name(), '', time() - 3600,
                $scParams['path'],
                $scParams['domain'],
                $scParams['secure'],
                isset($scParams['httponly'])
            );
        }

        session_destroy();
    }

    private function start()
    {
        if (isset($this->data)) {
            return null;
        }

        if (!isset($_SESSION)) {
            session_start();
        }

        $this->data = &$_SESSION;
    }
}

There are a few things I see:

Work in the constructor - Don't do work in the constructor. Ideally the only things that should be in the constructor are assignments.
DRY - Why are you repeating $this->start() all over the place?
What's up with that destroy() method? You're doing stuff there other than destroying the session! Why?
Your session_start() should happen once, in the constructor.
It would be more flexible if you passed the $_SESSION array as a parameter in the constructor, rather than relying on globals.


Simple marker rendering I am trying to render in a leaflet map that should render a single marker. I am building the JavaScript as follows.

build the map
an even listener for when the user opens the page where the map is rendered.

I query as follows:
@company = Company.friendly.find_by(
                        slug: [ params[:id],
                        params[:company_id] ])
respond_to do |format|
      format.html
      format.json { render json:
        {
          type: "Feature",
          "geometry": {
            "type": "Point",
            "coordinates": [@company.longitude, @company.latitude] # this part is tricky
          },
          properties: {
            "title": @company.name,
            "address": @company.street_address,
          }
        }
      }
    end

I console log the steps and everything seems to "work". The data is loaded and data.responseJSON renders as follows:
geometry: {…}
coordinates: Array [ 1.5918642, 43.7189496 ]
type: "Point"
<prototype>: Object { … }
properties: Object { title: "Acme SaS", address: "xxx", "marker-color": "#FFFFFF", … }
type: "Feature"

However, the map does not render as wanted. Only a grey box shows up. If I provide with a lat, Lon array, the map renders a map.
function buildCompanyMap() {

  document.getElementById('company_map').innerHTML = "

";

  console.log("script #1: %o", document.getElementById('company_map'));
  console.log("script #2: %o", document.getElementById('map'));

  var data = $.ajax({
      url: "#companies/company.json",
      dataType: "json",
      success: console.log("company data successfully loaded."),
      error: function(xhr) {
          alert(xhr.statusText)
      }
  });

  console.log("script #3: %o", $);

  console.log("script #4: %o", data);

  $.when(data).done(function() {
    var map = new L.Map('map', {
        dragging: false,
        tap: false,
        scrollWheelZoom: false
      });

    var osmUrl = 'https://{s}.tile.osm.org/{z}/{x}/{y}.png',
        osmAttribution = 'Map data © OpenStreetMap contributors,' +
                        ' CC-BY-SA',
        osmLayer = new L.TileLayer(osmUrl, {maxZoom: 18, attribution: osmAttribution});

    map.addLayer(osmLayer);

    console.log("script #5: %o", data.responseJSON);

    var geojsonMarkerOptions = {
        radius: 8,
        fillColor: "#ff7800",
        color: "#000",
        weight: 1,
        opacity: 1,
        fillOpacity: 0.8
    };

    L.geoJSON(data.responseJSON, {
        pointToLayer: function (feature, latlng) {
            return L.circleMarker(latlng, geojsonMarkerOptions);
        }
    }).addTo(map);

  });
};

document.addEventListener("turbolinks:load", function() {
  if (document.getElementById('company_map') == null) {
    //console.log('true')
  }
  else {
    buildCompanyMap();
  }
});

A correction. I have removed the use of jQuery in the correction. The main issue was that you don't set anywhere a map center and a zoom. I've changed the behaviour by zooming the map to GeoJSON extent (e.g map.fitBounds(jsonLayer.getBounds());)
You could also made the change by setting a map center and a zoom like described in this link to official doc.
function buildCompanyMap() {

  document.getElementById('company_map').innerHTML = "

";

  console.log("script #1: %o", document.getElementById('company_map'));
  console.log("script #2: %o", document.getElementById('map'));

  var map = new L.Map('map', {
      dragging: false,
      tap: false,
      scrollWheelZoom: false
    });

  var osmUrl = 'https://{s}.tile.osm.org/{z}/{x}/{y}.png',
      osmAttribution = 'Map data © OpenStreetMap contributors,' +
                      ' CC-BY-SA',
      osmLayer = new L.TileLayer(osmUrl, {maxZoom: 18, attribution: osmAttribution});

  map.addLayer(osmLayer);

  console.log("script #5: %o", data.responseJSON);

  var geojsonMarkerOptions = {
      radius: 8,
      fillColor: "#ff7800",
      color: "#000",
      weight: 1,
      opacity: 1,
      fillOpacity: 0.8
  };

  fetch('#companies/company.json', {
      headers: {
        'Accept': 'application/json',
        'Content-Type': 'application/json'
      }
    })
    .then(response => response.json())
    .then (json => {
      var jsonLayer = L.geoJSON(json, {
        pointToLayer: function (feature, latlng) {
            return L.circleMarker(latlng, geojsonMarkerOptions);
        }
      })
      jsonLayer.addTo(map);
      map.fitBounds(jsonLayer.getBounds());
    })
    .catch(function(error) {
      console.log('Error on fetch operation: ' + error.message);
    });

};

document.addEventListener("turbolinks:load", function() {
  if (document.getElementById('company_map') == null) {
    //console.log('true')
  }
  else {
    buildCompanyMap();
  }
});


Combining arbitrary number of lists inside json I have some JSON where each key stores an array, and I need to combine all those arrays into a single array.  My current solution doesn't seem the most friendly to review, but without forEach not able to return a value i'm struggling for a succinct solution.

config = {"levels": {"first": ["a", "b", "c"], "second": ["d", "e", "f"]}};

let combine = [];
Object.keys(config["levels"]).forEach(key=>combine.push(config["levels"][key]));
let levels = [].concat.apply([], combine);
// levels == ["a", "b", "c", "d", "e", "f"]

I never know how children config["levels"] will have there will be so this needs to be dynamic.

First

but without forEach not able to return a value i'm struggling for a succinct solution.

Addressing this in your post, in the future it may help to know you can use the .map() function, which does return values to an array, unlike forEach()
an example using .map() and what you have:

const config = {"levels": {"first": ["a", "b", "c"], "second": ["d", "e", "f"]}};
const accessor = config.levels

let combine = Object.keys(accessor).map(key=>{return accessor[key]});
let levels = [].concat.apply([], combine);

console.log(levels);
// levels == ["a", "b", "c", "d", "e", "f"]

More Succinct Solution
That said, something like this may be what you're looking for:

const config = {"levels": {"first": ["a", "b", "c"], "second": ["d", "e", "f"]}};
const accessor = config.levels

const combine = Array.prototype.concat(...Object.values(accessor));
console.log(combine)

NOTE 1: Accessor is pulled out in case it needs to change, but is not necessary


Print list items in a rectangular frame The aim is to have that:

list ["Hello", "World", "in", "a", "frame"]

And make it printed like this:

*********
* Hello *
* World *
* in    *
* a     *
* frame *
*********

I did this and it works fine:

a = 0
for nb in listA:
    if a == 0:
        print("*"*10).center(20)
    a += 1
    if len(nb) == 2:
        print("*   " + nb + "   *").center(20)
    elif len(nb) == 1:
        print("*   " + nb + "    *").center(20)
    else:
        print("* " + nb + "  *").center(20)
    if a == len(listA):
        listA[-1]
        print("*"*10).center(20)

Still, I feel it is very cumbersome. Do you have any ideas how I can improve it, please?

You can let .format() do most of the work for you:

def print_in_a_frame(*words):
    size = max(len(word) for word in words)
    print('*' * (size + 4))
    for word in words:
        print('* {:<{}} *'.format(word, size))
    print('*' * (size + 4))

>>> print_in_a_frame("Hello", "World", "in", "a", "frame")
*********
* Hello *
* World *
* in    *
* a     *
* frame *
*********

Most of the magic happens in this line:

print('* {:<{}} *'.format(word, size))

It may be easier to understand what's going on if we rewrite it as:

print('* {a:<{b}} *'.format(a=word, b=size))

This tells format to print the keyword argument a, left-aligned, on a field of total width the keyword argument b.  Note that if b was fixed, say to 10, you would write that as:

print('* {a:<10}} *'.format(a=word))

or:

print('* {:<10}} *'.format(word))


I think like this:
print("   0   1   2   3   4   5   6")
for row in board:
    print(" " + " | ".join(row))
    print("  ---+---+---+---+---+---+---")
print()

You could put the whole thing into a single call to print by using "\n".join and more string concatenation, but that would most likely make it a lot less readable.


Optimizing code for sequence A064604 I am implementing A064604 - OEIS for positive integers up to 10 billion.

I am finding the divisors in \$O(\sqrt N)\$. So, the overall time complexity of running the formula right now is \$O(N\sqrt N)\$. How do I improve on this?

Pastebin

import math

def factors(n):
    return set(reduce(list.__add__,
                ([i, n//i] for i in range(1, int(n**0.5) + 1) if n % i == 0)))

def sigma_4(n):
    l = factors(n)
    ans=0
    for factor in l:
        ans += (pow(factor,4))
    return ans

n=int(raw_input())
ans=0
for i in xrange(1,n+1):
    ans+=sigma_4(i)
print ans

That's a very naive bruteforce algorithm.  To optimize this sort of calculation you can usually "turn it around" by doing something like: for each possible divisor, computing the number of times it'll feature as a contributor and adding them up.  And if you can get it to work by just iterating over primes, it'll be even better/faster.


ConsumingRouter to consume based on condition Started as to make a stream splitter to split stream based on condition, but finally found that I did ConsumingRouter, to consume based on condition and while have to provide consumers before using this router not as splited stream can provide consumers then , and use this router inside forEach of stream, please check it and provide me with your feedback and if there is another way to do it:

public class ConsumingRouter<T> implements Consumer<T> {
    private Map<Predicate<T>, Consumer<T>> map = new ConcurrentHashMap<>();

    public void sub(Predicate<T> p, Consumer<T> c) {
        map.put(p, c);
    }

    @Override
    public void accept(T t) {
        map.entrySet().forEach(e -> {
            if (e.getKey().test(t))
                e.getValue().accept(t);
        });
    }
}

How to use it :

public static void main(String[] args) {
    ConsumingRouter<Integer> router = new ConsumingRouter<Integer>();
    Predicate<Integer> p1 = (i) -> i % 2 != 0;
    Predicate<Integer> p2 = (i) -> i % 2 == 0;
    Consumer<Integer> c1 = i -> {
        System.out.println(Thread.currentThread().getName() + " Route 1 : "
                + i);
    };
    Consumer<Integer> c2 = i -> {
        System.out.println(Thread.currentThread().getName() + " Route 2 : "
                + i);
    };
    router.sub(p1, c1);
    router.sub(p2, c2);
    IntStream.range(0, 1000).boxed().forEach(router);
}

There is a Map.forEach(BiConsumer) that you can use as well:

@Override
public void accept(T t) {
    map.forEach((k,v) -> {
        if (k.test(t)) {
            v.accept(t);
        }
    });
}

Alternatively, you can convert that if-statement in the form of an Optional too if you can ignore null values (as commented by @Misha):

@Override
public void accept(T t) {
    // answer update: switched to Optional.of() for reasons mentioned in comments
    map.forEach((k,v) -> Optional.of(t).filter(k::test).ifPresent(v));
}

You can also take a look at this StackOverflow question for some insight about splitting two Streams into two Streams... similar to yours, except that you are only concerned with multiple Consumers to terminate the source Stream.

edit: Oh yeah, you may want to consider having your sub() (subscribe?) method return the reference to itself, so that you can do method chaining as such:

router.sub(p1, c1).sub(p2, c2);

edit 2: @Misha has a good suggestion for doing away with the Map, if that suits your requirements as well.