Unit test with check testing framework I have some code where the purpose is the decide if a char is between quotes in an expression. For example, awk '{print $1}' has a { between quotes.

The function that I want to test is

int IBQplain(int pos, char *str, int offset);

int IBQdouble(int pos, char *str, int offset);

int IBQsingle(int pos, char *str, int offset) {
    int escaped = 0;
    for (; str[offset]; ++offset) {
        if (!escaped) {
            switch (str[offset]) {
                case '\\':
                    escaped = 1;
                case '\'':
                    return IBQplain(pos, str, offset + 1);
            }
        } else {
            escaped = 0;
        }
        if (pos == offset) {
            return 1;
        }
    }
    return 0;
}

int IBQdouble(int pos, char *str, int offset) {
    char ch;
    if (pos == offset)
        return 0;  /* Not within quotes */
    int escaped = 0;
    for (ch = str[offset]; ch; ch = str[++offset]) {
        if (!escaped) {
            switch (str[offset]) {
                case '\'':
                    return IBQsingle(pos, str, offset + 1);
                case '"':
                    return IBQdouble(pos, str, offset + 1);
                case '\\':
                    escaped = 1;
            }
        } else {
            escaped = 0;
        }
        if (pos == offset)
            return escaped;  /* Not within quotes, but may be single-escaped */
    }
    return 0; /* FIXME */
}

int IBQplain(int pos, char *str, int offset) {
    char ch;
    if (pos == offset)
        return 0;  /* Not within quotes */
    int escaped = 0;
    for (ch = str[offset]; ch; ch = str[++offset]) {
        if (!escaped) {
            switch (str[offset]) {
                case '\'':
                    return IBQsingle(pos, str, offset + 1);
                case '"':
                    return IBQdouble(pos, str, offset + 1);
                case '\\':
                    escaped = 1;
            }
        } else {
            escaped = 0;
        }
        if (pos == offset)
            return escaped;  /* Not within quotes, but may be single-escaped */
    }
    return 0; /* FIXME */
}

int isBetweenQuotes(int pos, char *str) {
    return IBQplain(pos, str, 0);
}

Write the test

#include "openshell.h"
#include "errors.h"
#test myQtest
fail_unless(isBetweenQuotes(11, "abc'asdqerfdsdxcvc'xc") == 1, "isBetweenQuotes function borked");
lm */

Compile it and run it

gcc -Wall -o bquotes-test util.c errors.c bquotes-test.c -lcheck -lsubunit -lrt -pthread -lm^C
dac@dac-Latitude-E7450:~/osh$ ./bquotes-test
Running suite(s): Core
100%: Checks: 1, Failures: 0, Errors: 0

It seems ok. I make more tests

#include "openshell.h"
#include "errors.h"
#test myQtest
fail_unless(isBetweenQuotes(11, "abc'asdqerfdsdxcvc'xc") == 1, "isBetweenQuotes function borked");
fail_unless(isBetweenQuotes(5, "abcasdqerfdsdxcvcxc") == 0, "isBetweenQuotes function borked");
fail_unless(isBetweenQuotes(11, "ab'c'as'dqerfdsdxcvc'xc") == 1, "isBetweenQuotes function borked");

Compile and run it

checkmk tst_bquotes.check > bquotes-test.c
dac@dac-Latitude-E7450:~/osh$ gcc -Wall -o bquotes-test util.c errors.c bquotes-test.c -lcheck -lsubunit -lrt -pthread -lm
dac@dac-Latitude-E7450:~/osh$ ./bquotes-test
Running suite(s): Core
100%: Checks: 1, Failures: 0, Errors: 0

As OP did not explicitly state the review goals, here are some general ideas.

Use const when referenced data is not changed. Allows for optimizations and defined behavior when the array address passed is const.

// int IBQplain(int pos, char *str, int offset);
// int IBQdouble(int pos, char *str, int offset);
// int IBQsingle(int pos, char *str, int offset);
int IBQplain(int pos, const char *str, int offset);
int IBQdouble(int pos, const char *str, int offset);
int IBQsingle(int pos, const char *str, int offset);

Use size_t as the type for array indexes.  int maybe insufficient.

// int IBQplain(int pos, char *str, int offset);
int IBQplain(size_t pos, char *str, size_t offset);

As the functions only return 1 of 2 values, consider _Bool or bool

#include <stdbool.h>
// int IBQplain(size_t pos, char *str, size_t offset);
bool IBQplain(size_t pos, char *str, size_t offset);

As these functions appear to be stand-alone, suggest more pessimistic range checking:

// if (pos == offset)
if (pos <= offset)
  return 0;  /* Not within quotes */

... otherwise if the below 3 are only helper functions to isBetweenQuotes(), they should be static.

static int IBQplain(int pos, const char *str, int offset);
static int IBQdouble(int pos, const char *str, int offset);
static int IBQsingle(int pos, const char *str, int offset);

Unclear why code accesses the array again instead of ch

for (ch = str[offset]; ch; ch = str[++offset]) {
    if (!escaped) {
      // switch (str[offset]) {
      switch (ch) {

Expect more test cases that have incomplete '' and "" pair combinations, especially onces that begin or end the string.


Print Connected Components Scala Given a file containing adjacent IDs:

i1, i2, i5
i3, i4
i2, i6, i7
i4, i8
i9, i3

I would like to print each of the connected components:

i1, i2, i5, i6, i7
i3, i4, i8, i9

I decided to accomplish this using a BFS on a Map datastructure:

/* Input, can be read from file easily by splitting on ", " */
val lines = List(List("i1", "i2", "i5"),
    List("i3", "i4"),
    List("i2", "i6", "i7"),
    List("i4", "i8"),
    List("i9", "i3"))
/* finds all sequential pairs */
val pairs =  lines.flatMap(x => x.dropRight(1).zip(x.drop(1)))

/* each pair should be symmetric (we are in an undirected graph) */
val pSym = pairs.flatMap(x => List(x, x.swap))

/* create an empty adjacency map: id -> (List of adjacent edges) */
val vertices = lines.flatten.distinct
val defMap = vertices.map(_ -> List[String]()).toMap

/* populate the default map with the actual adjacencies */
val adjMap = pSym.foldLeft{defMap}(
    (acc, x) => acc + (x._1 -> (acc(x._1) :+ x._2))
)

/* BFS algo on map representation of graph */
def mapBFS(adjMap: Map[String, List[String]]): List[List[String]] =
{
    val v = adjMap.keys
    var globalVisits = List[String]()
    def BFS_r(elems: List[String], visited: List[List[String]]): Option[List[List[String]]] =
    {
        val newNeighbors = elems.flatMap(adjMap(_)).filterNot(visited.flatten.contains).distinct
        if (newNeighbors.isEmpty)
            Some(visited)
        else
            BFS_r(newNeighbors, newNeighbors :: visited)
    }
    v.flatMap(x =>{
        if (globalVisits.contains(x))
            None
        else
        {
            val vi: List[String] = BFS_r(List(x), List(List(x))).get.flatten
            globalVisits = globalVisits ++ vi
            Some(vi)
        }
    }).toList
}

mapBFS(adjMap).foreach{println}

This gives me the desired result:

List(i7, i1, i6, i2, i5)
List(i8, i4, i3, i9)

However I feel that this code is not very functional (esp. with the side-effects of updating globalVisits in mapBFS) and I feel like some the data manipulation at the beginning is pretty inefficient. Any thoughts on improvements?

You can always translate loops into a tail-recursive function.  Then all you need to do is pass state around as function parameters instead of mutating it in place.  IIRC, Scala supports tail-call optimization of direct recursion with the appropriate annotation.  That should be good enough in this case.

You actually have done this for BFS_r.  You basically just need to do the same thing with the rest of the code.

But first, you're using a lot of Lists that would probably be better off as Sets or Maps.  Also, you're keeping track of the nodes that have been visited (globalVisits).  I would keep track of the portion of the graph that has not been visited instead -- at least with a tail-recursive solution, since we'll have to get rid of the flatMap.

The first thing is to replace the flatMap with a tail-recursive function:

@tailrec
def loop(unvisited: Map[String, List[String]], accumulator: List[List[String]]): List[List[String]] = {
  if unvisited.empty {
    // We're done => return the accumulator
    return acc
  }
  else{
    // Get the next seed node
    // (Since maps are typically unordered, head is an arbitrary node
    //  that hasn't been visited yet.)
    val seed = unvisited.head

    // Find the component connected to seed
    component = BFS_r(List(seed), List(List(seed)).get.flatten)

    // Iterate, with new unvisited map and component list
    // The new unvisited will have all the nodes in component removed
    // The new accumulator has component added
    return loop(unvisited -- component, component :: accumulator)
}

Then kick it off with:

return loop(adjMap, List())

I haven't done a lot of work with Scala yet, so I can't say this is 100% valid Scala or will work at all.  But hopefully the intention is clear.  One important to note is that Map and List here are the immutable variants (i.e., from scala.collections.immutable rather than scala.collections.mutable).


Project Euler, #4: Incorrect Results on 7-digit numbers I've got my answer working (up until a 7-digit answer is requested, anyway), and would like some help

returning the correct answer
speeding up the results

I was thinking of storing the values of the possible palindromes before beginning my calculations, but I know it wouldn't speed things up anyhow. I've also read around that the 'divide by 11' trick isn't always reliable (see comment #2...900009 / 11 = 81819).

My code skims the top 10% of both the inner and outer loop before it decrements the outer loop by one, and repeats until the first result is found. This works fine, until the answer for 7-digit numbers is requested.

"""http://projecteuler.net/problem=4

Problem 4
16 November 2001

A palindromic number reads the same both ways. The largest palindrome
made from the product of two 2-digit numbers is 9009 = 91 * 99.

Find the largest palindrome made from the product of two 3-digit numbers.

"""

def main(n):
    """return the largest palindrome of products of `n` digit numbers"""
    strstart = "9" * n
    multic = int(strstart)
    multip = multic -1
    top = multic
    while multic > top * .9:
        while multip > multic * .9:
            multip -= 1
            if isPalin(multic * multip):
                return multic, multip, multic*multip
        multip = multic
        multic -= 1

def isPalin(n):
    n = str(n)
    leng = len(n)
    half = n[:leng/2]
    if n[leng/2:] == half[::-1]:
        return True
    return False

if __name__ == '__main__':
    main(3)

How's my approach here?

def main(n):
    """return the largest palindrome of products of `n` digit numbers"""
    strstart = "9" * n
    multic = int(strstart)

Firstly, for two such short lines you'd do better to combine them. multic = int("9" * n). However, even better would be to avoid generating strings and then converting to ints. Instead, use math. multic = 10**n - 1

    multip = multic -1

Guessing what multip and multic mean is kinda hard. I suggest better names.

    top = multic

    while multic > top * .9:

You should almost always use for loops for counting purposes. It'll make you code easier to follow.

        while multip > multic * .9:
            multip -= 1
            if isPalin(multic * multip):
                return multic, multip, multic*multip
        multip = multic
        multic -= 1

def isPalin(n):

Use underscores to seperate words (the python style guide says so) and avoid abbreviations.

    n = str(n)
    leng = len(n)

This assignment doesn't really help anything. It doesn't make it clearer or shorter, so just skip it

    half = n[:leng/2]
    if n[leng/2:] == half[::-1]:
        return True
    return False

Why split the slicing of the second half in two? Just do it all in the if. Also use return n[...] == half[..] no need for the if.

if __name__ == '__main__':
    main(3)

Here is my quick rehash of your code:

def main(n):
    """return the largest palindrome of products of `n` digit numbers"""
    largest = 10 ** n - 1
    for a in xrange(largest, int(.9 * largest), -1):
        for b in xrange(a, int(.9 * a), -1):
            if is_palindrome(a * b):
                return a, b, a * b

def is_palindrome(number):
    number = str(number)
    halfway = len(number) // 2
    return number[:halfway] == number[:-halfway - 1:-1]

if __name__ == '__main__':
    print main(3)

Your link on 7-digit numbers is to a post discussing integer overflow in Java. Integer overflow does not exist in Python. You simply don't need to worry about that.

The challenge asks the answer for a 3 digit problem, so I'm not sure why you are talking about 7 digits. Your code is also very quick for the 3 digits so I'm not sure why you want it faster. But if we assume for some reason that you are trying to solve the problem for 7 digits and that's why you want more speed:

My version is already a bit faster then yours. But to do better we need a more clever strategy. But I'm not aware of any here.


Brainfuck to Python compiler So this is (I think) a premiere for python. A function that translates brainfuck code to python code. It does a few optimizations (It chains +,-,>,< together to one operation). I don't think you can do more optimizations, but I am open for ideas. At the beginning of the created source code there will be a few imports (defaultdict and getch) and a few function definitions (add, sub, putch) and obviously the data structures data and index. Is there anything I could improve? Anything else I could optimize away?

def translate(sourcecode):
    out = [
        (0, "from collections import defaultdict"),
        (0, "from msvcrt import getch"),
        (0, "putch = lambda x: print(chr(x), end='')"),
        (0, "data = defaultdict(int)"),
        (0, "index = 0"),
        (0, "def add(x):data[index] = (data[index] + x) % 256"),
        (0, "def sub(x):data[index] = (data[index] - x) % 256"),
    ]
    level = 0
    count = 0
    operation = None
    for c in sourcecode:
        if c == operation:
            count += 1
        else:
            if operation == "+":
                out.append((level, f"add({count!r})"))
            elif operation == "-":
                out.append((level, f"sub({count!r})"))
            elif operation == ">":
                out.append((level, f"index += {count!r}"))
            elif operation == "<":
                out.append((level, f"index -= {count!r}"))
            operation = None

            if c in "+-><":
                operation = c
                count = 1
            elif c == ".":
                out.append((level, "putch(data[index])"))
            elif c == ",":
                out.append((level, "data[index] = getch()[0]"))
            elif c == "[":
                out.append((level, "while data[index]:"))
                level += 1
            elif c == "]":
                assert level
                level -= 1
    return "\n".join("    " * indent + line for indent, line in out)

print(translate("++++++++++[>+++++++>++++++++++>+++>+<<<<-]>++.>+.+++++++..+++.>++.<<+++++++++++++++.>.+++.------.--------.>+.>."))

(if you are not on windows, you need an alternative to the getch function)

The way you define your various functions is inconsistent:

you define add, sub, putch, yet use data[index] = getch()[0] directly in the generated code;
you group together multiple +, -, <, and > but neither . nor , (not that this last one would be of great use but still, for the sake of consistency);
you access global data into those functions;
putch is defined using a lambda but add and sub uses def…

Since [ and ] are special anyway as they do more than producing a string, I'd rather choose to either:

inline everything in the generated code;
or only use functions that take data, index and count as parameters.

I would also generate code that respect some standard practices: defining a main function and wrapping its call into an if __name__ == '__main__': guard. This would mean starting level at 1 as you would generate code that is inside a def main():.

You should also take a look at textwrap.dedent to simplify your header (and thus footer too) generation.

And since you are using formated strings, you could make directly use of the variable width alignment such as f'{"": >{level*4}}rest of the code'.

Storing generated instructions into a list that is returned at the end feels like your function would benefit from being turned into a generator. Especially if you plan on writing the generated string to a file, this would help interleaving computation and waiting for I/O.

Lastly your assertion feels off as they can be turned off at runtime. Instead I would raise a real exception such as ValueError with a proper error message.

Proposed improvements with code inlined:

import itertools
from textwrap import dedent
from contextlib import suppress

BYTECODES = {
        '+': '{0: >{1}}data[index] = (data[index] + {2}) % 256',
        '-': '{0: >{1}}data[index] = (data[index] - {2}) % 256',
        '<': '{0: >{1}}index -= {2}',
        '>': '{0: >{1}}index += {2}',
        '.': '{0: >{1}}for _ in range({2}):\n{0: >{1}}    print(chr(data[index]), end="")',
        ',': '{0: >{1}}for _ in range({2}):\n{0: >{1}}    data[index] = getch()[0]',
}

def pairwise(iterable):
    a, b = itertools.tee(iterable)
    next(b, None)
    yield from itertools.zip_longest(a, b)

def compile(sourcecode):
    yield dedent("""\
            from collections import defaultdict
            from msvcrt import getch

            def main():
                index = 0
                data = defaultdict(int)""")

    level = count = 1
    for operation, next_operation in pairwise(sourcecode):
        if operation == '[':
            yield f'{"": >{level*4}}while data[index]:'
            level += 1
        elif operation == ']':
            level -= 1
            if level < 1:
                raise ValueError('Unbalanced brackets, found ] before opening [')
        elif operation == next_operation:
            count += 1
        else:
            with suppress(KeyError):
                yield BYTECODES[operation].format('', level * 4, count)
            count = 1

    yield dedent("""\

            if __name__ == '__main__':
                main()""")

def translate(sourcecode):
    return '\n'.join(compile(sourcecode))

if __name__ == '__main__':
    code = '++++++++++[>+++++++>++++++++++>+++>+<<<<-]>++.>+.+++++++..+++.>++.<<+++++++++++++++.>.+++.------.--------.>+.>.'
    for line in compile(code):
        print(line)

    print('*------*')
    print(translate(code))

Proposed improvements with functions defined:

import itertools
from textwrap import dedent
from contextlib import suppress

BYTECODES = {
        '+': '{0: >{1}}add(data, index, {2})',
        '-': '{0: >{1}}sub(data, index, {2})',
        '<': '{0: >{1}}index -= {2}',
        '>': '{0: >{1}}index += {2}',
        '.': '{0: >{1}}putchar(data, index, {2})',
        ',': '{0: >{1}}getchar(data, index, {2})',
}

def pairwise(iterable):
    a, b = itertools.tee(iterable)
    next(b, None)
    yield from itertools.zip_longest(a, b)

def compile(sourcecode):
    yield dedent("""\
            from collections import defaultdict
            from msvcrt import getch

            def add(data, index, x):
                data[index] = (data[index] + x) % 256

            def sub(data, index, x):
                data[index] = (data[index] - x) % 256

            def putchar(data, index, x):
                for _ in range(x):
                    print(chr(data[index]), end='')

            def getchar(data, index, x):
                for _ in range(x):
                    data[index] = getch()[0]

            def main():
                index = 0
                data = defaultdict(int)""")

    level = count = 1
    for operation, next_operation in pairwise(sourcecode):
        if operation == '[':
            yield f'{"": >{level*4}}while data[index]:'
            level += 1
        elif operation == ']':
            level -= 1
            if level < 1:
                raise ValueError('Unbalanced brackets, found ] before opening [')
        elif operation == next_operation:
            count += 1
        else:
            with suppress(KeyError):
                yield BYTECODES[operation].format('', level * 4, count)
            count = 1

    yield dedent("""\

            if __name__ == '__main__':
                main()""")

def translate(sourcecode):
    return '\n'.join(compile(sourcecode))

if __name__ == '__main__':
    code = '++++++++++[>+++++++>++++++++++>+++>+<<<<-]>++.>+.+++++++..+++.>++.<<+++++++++++++++.>.+++.------.--------.>+.>.'
    for line in compile(code):
        print(line)

    print('*------*')
    print(translate(code))

As you can see, differences are minimal but generated code feels neater for the second version. I also kept the translate function for ease of use but the generator is simple enough to use on its own.


To send the email on some given date and time I am making a PHP script which fetches the data of the emails whose sending date and time has been reached and then send emails to those records. I am following the steps below and integrating it in an API, that is.

GitHub code

Steps:

Get the communication records whose time has reached.
Run the loop of above and get the email addresses.
Run the loop of email addresses and send emails.

This is my PHP code:

<?php
include('iSDK/src/isdk.php');
$myApp = new iSDK();
// Test Connnection
if ($myApp->cfgCon("connectionName")) {
    echo "Connected...";
} else {
    echo "Not Connected...";
}
$query = mysql_query('SELECT * FROM `emails` WHERE `senddate` <= NOW()');
while ($row = mysql_fetch_assoc($query)) {
    $conDat = array('Email' => $row['email']);
    $conID = $myApp->addCon($conDat);
    $clist = array($conID);
    $email = $myApp->sendEmail($clist, 'kamranasadi431@gmail.com', '~Contact.Email~',
        'ccAddresses', 'bccAddresses', 'contentType', 'JK', 'htmlBody', 'txtBody');
}
?>

What changes should be made in it?

Use PDO, not mysql_* as they are deprecated and dangerous.
Break up your code into functions, this makes it easier to call and test.
Throw and catch exceptions.
You can omit the ending ?>

NOTE: This is untested code as I don't have the isdk.php file.

    <?php

    require('iSDK/src/isdk.php');

    $iSDK = new iSDK();

    try{
        checkConnection($iSDK);
        performQuery($iSDK, getDBConnection("", "", "", ""));
    } catch(RuntimeException $e){
        die($e->getMessage());
    }

    function checkConnection(&$iSDK){
        # Throw an exception if we don't have a connection, otherwise return true
        if (!$iSDK->cfgCon("connectionName")) {
            throw new RuntimeException('Could not obtain connection.');
        }
    }

    function performQuery(&$iSDK, $PDO){
        $sth = $PDO->prepare("SELECT * FROM `emails` WHERE `senddate` <= NOW()");
        $sth->setFetchMode(PDO::FETCH_ASSOC); # setting the fetch mode
        $sth->execute();

        while($row = $sth->fetch()) {
            $conDat = array('Email' => $row['email']);
            $conID = $iSDK->addCon($conDat);
            $clist = array($conID);
            $email = $iSDK->sendEmail($clist, 'kamranasadi431@gmail.com', '~Contact.Email~', 'ccAddresses', 'bccAddresses', 'contentType', 'JK', 'htmlBody', 'txtBody');
        }
    }

    function getDBConnection($host, $dbname, $user, $pass){
        try {
            return new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);
        } catch(PDOException $e) {
            die($e->getMessage());
        }
    }


Moving instance variable from partial to helper I want to display a town name in my navigation, depending on the town I am requesting from the model. The following code works, but I am mixing logic in my view which I know is wrong and I haven't been successful in accessing the @town instance variable from my TownsController.

Can someone lead me down the road to enlightenment in a situation like this? Should I be using a helper? How do I access it in my _header.html.erb partial?

   <% @towns = Town.all %>
   <% @towns.each do |t| %>

<%= link_to "#{t.name}", '#', id: "logo" %>

   <% end %>

Is _header.html.erb part of the layout? If so, i would advice you to move this logic to helper, if not all the controllers would need to set the @towns instance variable.

module ApplicationHelper
  def town_names
    Town.all.collect(&:name)
  end
end

And in your view call the town_names. I think you want each name to be in an li element. I have changed the code accordingly.

<% town_names.each do |name| %>

<%= link_to name, '#', id: "logo" %>

   <% end %>


scoped_ptr for C++/CLI (ensure managed object properly frees owned native object) Motivating SO question: Is there a C++/CLI smart pointer project (e.g. scoped_ptr)?

I'm interested in any reviewer comments, and especially identified bugs or inconsistencies with the native scoped_ptr template after which this code is patterned.

#pragma once

/** @file clr_scoped_ptr.h
 ** @author R Benjamin Voigt (richardvoigt@gmail.com)
 **
 ** Rights reserved.  This code is not public domain.
 **
 ** Licensed under CC BY-SA 3.0             http://creativecommons.org/licenses/by-sa/3.0/
 **             or Lesser GPL 3 or later    http://www.gnu.org/copyleft/lesser.html
 ** with the following restrictions (per GPL section 7):
 **  - all warranties are disclaimed, and if this is prohibited by law, your sole remedy shall be recovery of the price you paid to receive the code
 **  - derived works must not remove this license notice or author attribution
 **  - modifications must not be represented as the work of the original author
 **  - attribution is required in the "about" or "--version" display of any work linked hereto, or wherever copyright notices are displayed by the composite work
 **/

struct safe_bool { private: safe_bool(); };

/** \brief C++/CLI analogue to boost::scoped_ptr, also similar to std::unique_ptr, for management of the lifetime of an unmanaged class instance by a managed object
 **/
template<typename T>
public ref class clr_scoped_ptr
{
    T* m_native_ptr;

    // declare copy-constructors and assignment operators to prevent double-free
    clr_scoped_ptr( clr_scoped_ptr<T>% ) /* = delete */ { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    template<typename U>
    clr_scoped_ptr( clr_scoped_ptr<U>% ) { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    clr_scoped_ptr% operator=( clr_scoped_ptr<T>% ) /* = delete */ { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

    template<typename U>
    clr_scoped_ptr% operator=( clr_scoped_ptr<U>% ) { throw gcnew System::InvalidOperationException("clr_scoped_ptr is non-copyable"); }

public:
    clr_scoped_ptr( void ) : m_native_ptr(nullptr) {}
    explicit clr_scoped_ptr( T* ptr ) : m_native_ptr(ptr) {}
    !clr_scoped_ptr( void ) { reset(); }
    ~clr_scoped_ptr( void ) { clr_scoped_ptr::!clr_scoped_ptr(); }

    template<typename U>
    clr_scoped_ptr( U ptr ) : m_native_ptr(ptr) {}

    void reset( T* ptr ) { delete m_native_ptr; m_native_ptr = ptr; }
    void reset( void ) { reset(nullptr); }

    clr_scoped_ptr% operator=( T* ptr ) { reset(ptr); }

    template<typename U>
    clr_scoped_ptr% operator=( U ptr ) { reset(ptr); }

    operator struct safe_bool*() { return reinterpret_cast<struct safe_bool*>(m_native_ptr); }

    void swap( clr_scoped_ptr<T>% other )
    {
        using std::swap;
        swap(m_native_ptr, other.m_native_ptr);
    }

    T* release( void ) { T* retval = m_native_ptr; m_native_ptr = nullptr; return retval; }
    T* get( void ) { return m_native_ptr; }

    static T* operator->( clr_scoped_ptr<T>% sptr ) { return sptr.get(); }
    static T& operator*( clr_scoped_ptr<T>% sptr ) { return *sptr.get(); }
};

template<typename T>
inline void swap( clr_scoped_ptr<T>% left, clr_scoped_ptr<T>% right )
{
    left.swap(right);
}

I have only superficially tested the code so far, but it seems to make sense. However, there are some details where I'm either missing something (which wouldn't surprise me, I'm more at home with C#) or the code is more complex than it needs to be. Any comments are appreciated!

The private copy constructors and assignment operators throw exceptions although they can never be called. Wouldn't it suffice to leave them empty?
The constructor and assignment operator both accept not only arguments of type T*, but also exist in a templated version taking U*. I can't quite figure out why. My first thought was that this allows pointers to derived types to be passed; but then again, this is also possible using just the first form.
Similarly, I don't understand the role of the template versions of the private constructor and assignment operator. From my understanding, neither of these will be auto-generated if you don't provide private versions.
The safe bool pattern sound like a great idea (I first had to research it). However, VS2010 shows a compile error saying "cannot convert from 'clr_scoped_ptr<T>::operator safe_bool *::safe_bool *' to 'safe_bool *'". Apparently, it treats the two occurrences of struct safe_bool as two different types. Am I missing something here?
You end the private methods and operators with a semicolon and you write empty argument lists as (void) rather than (). Are these just stylistic choices, or are there advantages in doing this?


Nesting IAsyncEnumerable I am trying to work around the problem Pass-through for IAsyncEnumerable. The best I have so far is to return IAsyncEnumerable<IAsyncEnumerable<T>> instead of IAsyncEnumerable<T>, then to flatten it. I'm not thrilled about it, so I'm asking here for suggestions.

The underlying goal is to allow one to get portions of one's IAsyncEnumerable from child methods without going insane. The Counting() method below is my attempt.

I don't think it would be much trouble to design a recursive object in order to allow deeper nesting. But I still don't think I would like it much.

public static class AsyncEnumerables
{
    public static async IAsyncEnumerable<T> Flatten<T>(IAsyncEnumerable<IAsyncEnumerable<T>> enumerables)
    {
        await foreach (var e in enumerables)
        {
            await foreach (var t in e)
            {
                yield return t;
            }
        }
    }

    public static async IAsyncEnumerable<T> Singleton<T>(T t)
    {
        await Task.CompletedTask;
        yield return t;
    }
}

public class AsyncEnumerableTests
{
    public async IAsyncEnumerable<int> Children()
    {
        yield return 1;
        await Task.Delay(1000);
        yield return 2;
    }

    public async IAsyncEnumerable<IAsyncEnumerable<int>> Counting()
    {
        yield return AsyncEnumerables.Singleton(0);
        await Task.Delay(100);
        yield return Children();
        yield return AsyncEnumerables.Singleton(3);
    }

    [Fact]
    public async Task Counting_Counts()
    {
        var numbers = new List<int>();
        await foreach (var n in AsyncEnumerables.Flatten(Counting())) {
            numbers.Add(n);
        }
        Assert.Equal(4, numbers.Count);
        for (int i=0; i<4; i++)
        {
            Assert.Equal(i, numbers[i]);
        }
    }
}

There are several things worth considering when you are about to design an API, which should either return an IAsyncEnumerable<IAsyncEnumerable<T>> or return an IAsyncEnumerable<T>

IAsyncEnumerable < IAsyncEnumerable < T > >

Pros

It is flexible from the consumer point of view. It allows you to use a single or multiple consumer model
In case of multiple consumers the throughput can be higher, because the parallel processing potential
It allows to use different Timeouts for different providers
It allows custom code injection between two yield return statements

Cons

In case of single consumer the ingestion logic can be a bit more complicated (compared to the flatten version)
Nested async loops can complicate the exception handling logic (and also the debugging)

IAsyncEnumerable < T >

Pros

It is easier to use for a single consumer
If the multiple provider model is just an implementation detail then it can hide this information behind a good abstraction
It provides easier error handling capabilities

Cons

It is harder to support multiple consumers model
It is harder to inject custom code between yield returns
Spanning new processing based on different providers is impossible

If you are considering to provide a flattened API then I would suggest to check that package, which is called System.Interactive.Async. Inside this package there is a class called AsyncEnumerableEx which defines a Merge function.


Getting a count of intervals The statement of the problem is there are 2 files, one has a set of intervals i.e. 0-10, 3-40, etc which may be repetitive. The second file has a set of numbers which are also repetitive. The exercise is to get a count of the intervals, a number in the 2nd file would fit into.

Intervals.txt

0 12
2 36
6 98
2 36

Numbers.txt

2
4
123
20

What I thought of as a  possible solution was keep a count of intervals when you populate a map of intervals. So if you encounter an interval 3-40 5 times, you keep the count as 5 so you don't try to populate a map 5 times with the same interval. How it helps downstream is, if a number is in a specific interval you can count its existence in 5 intervals. Secondly there is a map of found numbers in your previous iterations, so that you don't loop through the intervals again.

I have been told, by some random guy, that my implementation is ok but not what he desires. No further explanations. I have tried raking my brain but haven't got any further. Any pointers or help would be much appreciated. Any further clarifications, please ask.

void generateCounts(const string& fname1, const string& fname2)
{
    std::ifstream file(fname1.c_str());
    string line;
    map<int, pair<int, int> > extents;
    map<int, pair<int, int> >::iterator it;
    std::string left;
    std::string right;

    if(file.is_open())
    {
        while(file.good())
        {
            getline(file, line);
            string::size_type pos = line.find(' ');
            left = line.substr(0, pos);
            right = line.substr((++pos));
            int ll = atoi(left);
            int up = atoi(right);
            it = extents.find(ll);
            if(it != extents.end())
            {
                if(it->second.first != up){
                    extents.insert(make_pair<const int, pair<int, int> >(ll, make_pair<int, int>(up, ++count)));
                }
                else{
                    it->second.second += 1 ;
                }
            }
            else
            {
                extents.insert(make_pair<const int, pair<int, int> >(ll, make_pair<int, int>(up, ++count)));
            }
            count = 0;
        }
    }
    fclose(inp_file);

    std::ifstream file1(fname2.c_str());
    map<int, int> found;
    map<int, int>::iterator it1;
    string str;
    if(file1.is_open())
    {
        while(file1.good())
        {
            count = 0;
            getline(file1, str);
            if(str.empty()) continue;
            int var = atoi(str.c_str());
            it1 = found.find(var);
            if (it1 == found.end())
            {
                for(it = extents.begin(); it != extents.end(); ++it)
                {
                    if((var >= it->first ) && (var <= it->second.first))
                    {
                           count += it->second.second ;
                    }
                }
                found.insert(make_pair(var, count));
            }
            else
                count = it1->second;

            cout << count << "\n";
            str.clear();
    }
    file1.close();
}

OK. Since this is homework I am going to limit my points to hints (I may add more in a week or so).

Why are you doing this?

if(file.is_open())

This is an anti-pattern (in every language not just C++):
Remember the eof (part of not being good) does not happen until you read past the end of line.

Lots of examples on SO (go look it up).

    while(file.good())
    {
        getline(file, line);

You are doing too much work here:

There is an easy way to read integers from a stream.  Lots of examples on SO (go look it up).

    getline(file, line);
    string::size_type pos = line.find(' ');
    left = line.substr(0, pos);
    right = line.substr((++pos));   // Also this is wrong if no space was found.
    int ll = atoi(left);
    int up = atoi(right);

Why do a 2-phase compare?

    if(it != extents.end())
    {
        if(it->second.first != up){
            extents.insert(make_pair<const int, pair<int, int> >(ll, make_pair<int, int>(up, ++count)));
        }
        else{
            it->second.second += 1 ;
        }
    }
    else
    {
        extents.insert(make_pair<const int, pair<int, int> >(ll, make_pair<int, int>(up, ++count)));
    }

If you make the key std::pair&lt;left, right&gt; then you only need to do one test. Not this complicated mess. It also makes more sense to keep the left and right together.
Don't do this: (see Implementation using fstream failed evaluation)

fclose(inp_file);

Same comments about the second loop:

This is not wrong. But when I compare a number against two others of a range I put the value that is being tested for inclusion in the range in the middle of the comparison. I personally find it nicer to read this way:

 if((var >= it->first ) && (var <= it->second.first))

 // I prefer

 if ((it->first <= var) && (var <= it->second.first))


Changing the opacity property I have a dropdownbox, ActiviteitAardItems, where ActiviteitAard items can be checked (checkbox). If one (or more) are checked the property Opacity will be changed, and the code beneath will be executed. The Tijdblok mentioned below also has a property Activiteit and ActiviteitAard.

public double Opacity
{
    get
    {
        if (Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true).Count() > 0)
        {
            if (this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null).Count() > 0)
            {
                Tijdblok tijdblok;
                var tijdblokken = new List<Tijdblok>();

                foreach (var item in Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true))
                {
                    tijdblok = this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null && t.Activiteit.ActiviteitAard.Code == item.Code).FirstOrDefault();
                    if (tijdblok != null)
                        tijdblokken.Add(tijdblok);
                }

                tijdblok = tijdblokken.OrderByDescending(tb => tb.Activiteit.RoosterKleurPrioriteit).FirstOrDefault();

                if (tijdblok != null && tijdblok.Activiteit != null && tijdblok.Activiteit.ActiviteitAard != null)
                {
                    if (Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).Select(x => x.Code).Contains(tijdblok.Activiteit.ActiviteitAard.Code))
                    {
                        return 1.0;
                    }
                    else
                    {
                        return 0.1;
                    }
                }
                else
                {
                    return 0.1;
                }

            }
            else
            {
                return 0.1;
            }
        }

        return 1.0;
    }
}

I think there is probably more that could be offered as the code like something that could do with a makeover, but here's a couple of minor points for consideration.

To avoid any potential full enumeration of the items to retrieve the count of items in the list use either the .Count property or the option I prefer .Any(). e.g. Planning.ActiviteitAardItems.Where(aa => aa.IsChecked == true).Any()
I personally think lines like aa.Checked == true is unecessary noise.  You can achieve the same result by just going .Where(aa => aa.IsChecked).
If you are using the same IEnumerable multiple times consider first assigning it to a local variable and doing a ToList() on it.  This means that you will avoid any potential multiple enumerations of the dataset as ToList() will force it all back into memory.

For example

var ardCheckedItems = Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).ToList();

You can now use this same variable in a couple of places in your code.
Does 1.0 and 0.1 mean anything?  You could perhaps convert them to constants to avoid any repetition.  See example below.
Here is an attempt to reduce the Arrow code effect we sometimes find ourselves writing and seems evident in this case.

public double Opacity
{
    get
    {
        const double returnOne = 1.0; // for lack of better names
        const double returnTwo = 0.1;

        var ardCheckedItems = Planning.ActiviteitAardItems.Where(aa => aa.IsChecked).ToList();

        if(!ardCheckedItems.Any()) return returnOne;

        tijdBlokkenItems = this.Tijdblokken.Where(t => t.Activiteit != null && t.Activiteit.ActiviteitAard != null).ToList();

        if(!tijdBlokkenItems.Any()) return returnTwo;

        Tijdblok tijdblok;
        var tijdblokken = new List<Tijdblok>();

        // See Edit below for an example of how to possibly re-write this
        foreach (var item in ardCheckedItems)
        {
            tijdblok = tijdBlokkenItems.Where(t => t.Activiteit.ActiviteitAard.Code == item.Code).FirstOrDefault();
            if (tijdblok != null)
                tijdblokken.Add(tijdblok);
        }

        tijdblok = tijdblokken.OrderByDescending(tb => tb.Activiteit.RoosterKleurPrioriteit).FirstOrDefault();

        var objectsNotNull = tijdblok != null && tijdblok.Activiteit != null && tijdblok.Activiteit.ActiviteitAard != null;
        var anyCheckedItems = ardCheckedItems.Any(x => x.Code == tijdblok.Activiteit.ActiviteitAard.Code);

        return objectsNotNull && anyCheckedItems ? returnOne : returnTwo;
    }
}

EDIT:  After Trevor pointed me in the right direction in order to improve the foreach statement I did a quick search on if it would be easy enough to do an extension method to return items that are not Null.  Stack overflow to the rescue https://stackoverflow.com/questions/14469159/linq-selectmany-and-where-extension-method-ignoring-nulls.  So altering that slighty I came up with and extension method like so:

public static IEnumerable<TResult> WhereNotNull<TSource, TResult>(
            this IEnumerable<TSource> source, Func<TSource, TResult> selector)
            where TResult : class
{
    return source.Select(selector).Where(sequence => sequence != null);
}

which means I believe we can re-write the foreach like so:

var tijdblokken = ardCheckedItems.WhereNotNull(ard => tijdBlokkenItems.FirstOrDefault(t.Activiteit.ActiviteitAard.Code == ard.Code)).ToList();


Identifying clicked elements I'm relatively new to web development. I started learning JavaScript a few months ago and it's going well. I made this code for someone on fiverr, and it got the job done, but I know there's a more efficient way to write this without using a click handler for each element. There has to be a way to know which element is clicked and show the proper one without a separate click handler for each element.

Here's the Codepen link to the code.

HTML

  Item 1
  Item 2
  Item 3
  Item 4

Item 1

This is Item 1 content

Item 2

This is item 2 content

Item 3

This is item 3 content

Item 4

This is item 4 content

CSS

.item-block {
  display: none;
}

JS

$(document).ready(function() {
  $('#1').show();
})

$('#item-1').click(function() {
  $('.item-block').hide();
  $('#1').show();
})

$('#item-2').click(function() {
  $('.item-block').hide();
  $('#2').show();
})

$('#item-3').click(function() {
  $('.item-block').hide();
  $('#3').show();
})

$('#item-4').click(function() {
  $('.item-block').hide();
  $('#4').show();
})

In the HTML, give some id to your div with links, like this:

  Item 1
  Item 2
  Item 3
  Item 4

Then change your JavaScript like this:

$("#links > a").click(function(){
   $('.item-block').hide();
   $("#"+this.id.replace("item-","")).show();
})


Chaining a number of callables into a single function How would you name this function (I gave in a name pipe in the example) which constructs combines a chain of functions with a single attribute into a single callable:

# Python
def pipe(*functions):
    """
    pipe(c, b, a)(value) == a(b(c(value)))
    """
    if len (functions) == 1:
        return functions[0]
    def chain(val):
        for func in functions:
            val = func(val)
        return val
    chain.func_name = '__'.join(repr(f) for f in functions)
    return chain

The function performs composition, so the name should be compose. Or rather reverse_compose, because it applies functions in the order reversed from natural composition.


SCOPE_EXIT implementation I'm a fanboy of the Boost library and I've used Boost's components extensively in my C++03 project. The team decided to merge several pieces of code including my project. I will get the C++14 environment, but I have to drop the Boost library, so I'd like to replace BOOST_SCOPE_EXIT with a modern C++ technique.

Here is SCOPE_EXIT's implementation, with a short example:

#include <cstdio>
#include <cstdlib>
#include <memory>

template <typename Lambda>
class ScopeGuard
{
public:
    ScopeGuard(const Lambda& func)
        : committed_(false), valid_(true), rollbackFunc_(func) {}

    ~ScopeGuard() noexcept
    {
        if (valid_ && !committed_)
            rollbackFunc_();
    }

    ScopeGuard(const ScopeGuard&) = delete;
    ScopeGuard& operator=(const ScopeGuard&) = delete;

    ScopeGuard(ScopeGuard&& other)
        : committed_(other.committed_), valid_(other.valid_), rollbackFunc_(other.rollbackFunc_)
    {
        other.valid_ = false;
    }

    ScopeGuard& operator=(ScopeGuard&& other)
    {
        committed_ = other.committed_;
        valid_ = other.valid_;
        rollbackFunc_ = other.rollbackFunc_;
        other.valid_ = false;
        return *this;
    }

    void commit() const { committed_ = true; }

private:
    mutable bool committed_;
    bool valid_;
    Lambda rollbackFunc_;
};

template <typename Lambda>
ScopeGuard<Lambda> makeScopeGuard(Lambda&& func)
{
    return ScopeGuard<Lambda>(func);
}

#define MY_CONCATENATE_NX(A, B) A ## B
#define MY_CONCATENATE(A, B) MY_CONCATENATE_NX(A, B)

#define SCOPE_EXIT(f) auto MY_CONCATENATE(EXIT, __LINE__) = makeScopeGuard(f);

int foo(const char*) noexcept;
bool bar(int) noexcept;
void baz(int) noexcept;

int main() {
    int f = foo("/dev/random");
    if (!f)
    {
        return EXIT_FAILURE;
    }
    SCOPE_EXIT([f](){ baz(f); })
    int b = 1;
    bar(b);
    // baz(f)

    return EXIT_SUCCESS;
}

Godbolt gcc 7.2 -std=c++14 -O2

VS2017 -W4 /EHsc /O2

gcc's assembly is pretty short, however MSVC's assembly is so long (about 70 lines) and I can't understand what's going on. (I am not familiar with the Windows environment.)

Did I miss something, or is it a bad optimization of MSVC?

Move semantics:

The makeScopeGuard() function accepts the lambda by r-value ref; but passes a normal ref to the constructor of ScopeGuard.

template <typename Lambda>
ScopeGuard<Lambda> makeScopeGuard(Lambda&& func)
{
    return ScopeGuard<Lambda>(func);
}

You should either update ScopeGuard to take the function by r-value ref or change the makeScopeGuard() take a normal ref (to be consistent).

Personally I would pass the r-value ref through to the ScopeGuard.

template <typename Lambda>
ScopeGuard<Lambda> makeScopeGuard(Lambda&& func)
{
    return ScopeGuard<Lambda>(std::move(func)); // Need the move here
                                                // a named value is not an r-value ref.
}

Also need to update ScopeGuard

ScopeGuard(Lambda&& func)
    : committed_(false)               // Please one line per variable
                                      // Just like in normal code
                                      // were you declare one variable per line.
    , valid_(true)
    , rollbackFunc_(std::move(func))  // Need the move again.
{}

No Except

Normally the move operators are marked noexcept. Unless you have a good reason not to I would continue that norm.

ScopeGuard(ScopeGuard&& other) noexcept
ScopeGuard& operator=(ScopeGuard&& other) noexcept

Move by swapping.

The easiest way to move something is to swap it.

// Now we could implement this with a swap.
// But this seems trivial enough So this is fine.
ScopeGuard(ScopeGuard&& other) noexcept
    : committed_(other.committed_)        // One variable per line please.
    , valid_(other.valid_)
    , rollbackFunc_(other.rollbackFunc_)
{
    // Disabled the moved from.
    other.valid_ = false;
}

ScopeGuard& operator=(ScopeGuard&& other) noexcept
{
    // Here we are doing a copy (why not invoke the copy constructor)?
    // Followed by marking it `false`.
    committed_ = other.committed_;
    valid_ = other.valid_;
    rollbackFunc_ = other.rollbackFunc_;
    other.valid_ = false;
    return *this;
}

I would write like this:
I personally think it is easier to read like this.

ScopeGuard(ScopeGuard&& other) noexcept
    : committed_(false)
    , valid_(false)
    , rollbackFunc_([](){})
{
    swap(other);
}

ScopeGuard& operator=(ScopeGuard&& other) noexcept
{
    swap(other);
    other.valid_ = false;
    return *this;
}

void swap(ScopeGuard& other) noexcept
{
    using std::swap;
    swap(committed_,    other.committed_);
    swap(valid_,        other.valid_);
    swap(rollbackFunc_, other.rollbackFunc_);
}

Abuse of mutable

mutable bool committed_;

Not sure I like this. Is this because you have a const value and want to get around the constness of the object?

Mutable objects are meant to represent objects that are not part of the full state of the object. i.e. a cached or computed part of the state that can be re-calculated.

committed_ is very much part of the objects main state. So I would not mark it mutable.

That of course means you have to remove the const off the commit method.

void commit()       { committed_ = true; }
              ^^^^^ removed const

It is changing the state so its not really a const method.

Commit

There seems to be no way to call commit!!!

SCOPE_EXIT([f](){ baz(f); })

// how do I call commit on the above?
// I could use my editor to find the line number, but thats very fragile.
// And probably not what you intended to do.

bar(5); // It worked no exception
// So here I would expect to call commit.

If you can not call commit() then you should remove it from the class.

I would just get rid of the macors. Then your users can do:

auto guard = makeScopeGuard([](){revertStuff();}
// DO WORK HERE

// WORK DONE
guard.commit(); // no exceptions so we commit and don't need to revert.

An example working with correct move semantics fo the pedants:

#include <cstdio>
#include <cstdlib>
#include <memory>
#include <functional>
#include <iostream>

class ScopeGuard
{
public:
    ScopeGuard(std::function<void()>&& func)
        : committed_(false)
        , valid_(true)
        , rollbackFunc_(std::move(func))
    {}

    ~ScopeGuard() noexcept
    {
        if (valid_ && !committed_)
            rollbackFunc_();
    }

    ScopeGuard(const ScopeGuard&) = delete;
    ScopeGuard& operator=(const ScopeGuard&) = delete;

    ScopeGuard(ScopeGuard&& other) noexcept
        : committed_(false)
        , valid_(false)
    {
        swap(other);
    }

    ScopeGuard& operator=(ScopeGuard&& other) noexcept
    {
        swap(other);
        return *this;
    }

    void swap(ScopeGuard& other)
    {
        using std::swap;
        swap(committed_,    other.committed_);
        swap(valid_,        other.valid_);
        swap(rollbackFunc_, other.rollbackFunc_);
    }

    void commit() const { committed_ = true; }

private:
    mutable bool          committed_;
    bool                  valid_;
    std::function<void()> rollbackFunc_;␣
};

ScopeGuard makeScopeGuard(std::function<void()>&& func)
{
    return ScopeGuard(std::move(func));
}

#define MY_CONCATENATE_NX(A, B) A ## B
#define MY_CONCATENATE(A, B) MY_CONCATENATE_NX(A, B)

#define SCOPE_EXIT(f) auto MY_CONCATENATE(EXIT, __LINE__) = makeScopeGuard(f);

int foo(const char*) noexcept {std::cout << "food\n";return 0;}
bool bar(int) noexcept {std::cout << "bar\n";return true;}
void baz(int) noexcept {std::cout << "baz\n";}

int main() {
    int f = foo("/dev/random");
    if (!f)
    {
        return EXIT_FAILURE;
    }
    SCOPE_EXIT([f](){ baz(f); })
    int b = 1;
    bar(b);
    baz(f)
}


Bootstrap/bind click event over ngClick I am working on angularjs, and I have created some directives with bunch of HTML elements within its templates. I have assigned a controller for those directives.  Below is my code snippet:

directives.directive('ngLd',function()
{
    return {
        restrict : "AE",
        templateUrl:'partials/ld.html',
        scope:{},
        link : function(scope,element,attr)
        {
            var lbutton = element.find("span[class='lb-lk-comm']");
            var dlbutton = element.find("span[class='lb-dlk-comm']");
            if(lbutton && dlbutton)
            {
                lbutton.bind('click',function(event)
                {
                    scope.likeAction(lbutton.hasClass("lb-voted-comm"),dlbutton.hasClass("lb-voted-comm"));
                    scope.$apply();
                });
                dlbutton.bind('click',function(event)
                {
                    scope.dislikeAction(lbutton.hasClass("lb-voted-comm"),dlbutton.hasClass("lb-voted-comm"));
                    scope.$apply();
                });
            }
        },
        controller : "MyController"
    };
});

In the above code, I have used a link function of bind the click event on required elements.
Another approach I could have followed to use the ngClick directive directly over the elements, but that is sort of exposing method to client end.

My question is, am I following a good approach by binding click event ? or should I use ngClick ?

I am using angular.bootstrap(document,"myApp"); to bootstrap the angular module to document, so I was wondering if I could have similar thing to dynamically bind those events (click, hover, change etc) to my HTML elements, without exposing them to HTML pages.

I think you did the right thing, if you used ngClick you would loose tooling: syntax highlighting, debugging, linting.

Other than that your code looks good, some minor nitpickings:

a single comma separate var statement is considered better:

var lbutton = element.find("span[class='lb-lk-comm']"),
    dlbutton = element.find("span[class='lb-dlk-comm']");

You keep repeating "lb-voted-comm", you should use a constant for it.
I only understood after reading likeACtion/dislikeAction that lbutton/dbutton stand for likeButton, dislikeButton ( I thought it was left button, double click button or something ), you should spell out those variables names.


Generate Christmas Tree Problem Statement:

  The Christmas tree is comprised of the following Parts Stand Each Part
  is further comprised of Branches. Branches are comprised of Leaves.

  How the tree appears as a function of days should be understood. Basis
  that print the tree as it appears on the given day. Below are the
  rules that govern how the tree appears on a given day. Write a program
  to generate such a Christmas tree whose input is number of days.

Rules

  1.If tree is one day old you cannot grow. Print a message "You cannot generate christmas tree"

  2.Tree will die after 20 days; it should give a message "Tree is no more"

  3.Tree will have one part less than the number of days.

  E.g.

  On 2nd day tree will have 1 part and one stand.

  On 3rd day tree will have 2 parts and one stand

  On 4th day tree will have 3 parts and one stand and so on.

  4.Top-most part will be the widest and bottom-most part will be the narrowest.

  5.Difference in number of branches between top-most and second from top will be 2

  6.Difference in number of branches between second from top and bottom-most part will be 1

  Below is an illustration of how the tree looks like on 4th day

Input Format:

  First line of input contains number of days denoted by N

Output Format:

  Print Christmas Tree for given N

  OR

  Print "You cannot generate christmas tree" if N <= 1

  OR

  Print "Tree is no more" if N > 20

Constraints:

  0<= N <=20

Sample Input and Output

  Input:  2

  Output:( below)

The code:

#include<stdio.h>

int main(){
    int i,j,leaves=1,day,rest,branches,max_leaves;
    scanf("%d",&day);

    //Check for exceptions
    if(day<=1){
        printf("You cannot generate christmas tree");
        return 0;
    }
    else if(day>20){
        printf("Tree is no more");
        return 0;
    }

    // printf("Heres Your Christmas Tree:\n\n");

    // For first part
    branches=day+1;
    max_leaves = 1+(branches-1)*2; // Arithmetic Progression
    for(i=1;i<=branches;i++){
        for(j=i;j<=max_leaves/2;j++){
            printf(" ");
        }
        for(j=1;j<=leaves;j++){
            printf("*");
        }
        printf("\n");
        leaves=leaves+2;
    }

    // For remaining parts
    branches=branches-2;
    for(rest=1;rest<=day-2;rest++){
        leaves=3;
        for(i=2;i<=branches+1;i++){
            for(j=i;j<=max_leaves/2;j++){
                printf(" ");
            }
            for(j=1;j<=leaves;j++){
                printf("*");
            }
            printf("\n");
            leaves=leaves+2;
        }
        branches--;
    }

    // For stand
    for(i=1;i<=2;i++){
        for(j=1;j<=max_leaves/2;j++){
            printf(" ");
        }
        printf("*\n");
    }

    return 0;
}

The code is working fine as can be checked on IDEOne.

Keep the scope of variables to a minimum

Since C99 we are allowed to mix declaration and code. At the moment, you have i, j, rest and so on in scope, even if day is out of the specification. That can lead to hard to find errors, for example if we accidentally use i++ where i isn't used.

Late declaration makes it easier to copy our code into other blocks also, but more on that later. So instead, let's push the declaration as late as possible and limit the scope:

#include<stdio.h>

int main(){
    int day;
    scanf("%d",&day);

    //Check for exceptions
    if(day<=1){
        printf("You cannot generate christmas tree");
        return 0;
    }
    else if(day>20){
        printf("Tree is no more");
        return 0;
    }

    // printf("Heres Your Christmas Tree:\n\n");

    // For first part
    int leaves=1;
    int branches=day+1;
    int max_leaves = 1+(branches-1)*2; // Arithmetic Progression
    for(int i=1;i<=branches;i++){
        for(int j=i;j<=max_leaves/2;j++){
            printf(" ");
        }
        for(int j=1;j<=leaves;j++){
            printf("*");
        }
        printf("\n");
        leaves=leaves+2;
    }

    // For remaining parts
    branches=branches-2;
    for(int rest=1;rest<=day-2;rest++){
        leaves=3;
        for(int i=2;i<=branches+1;i++){
            for(int j=i;j<=max_leaves/2;j++){
                printf(" ");
            }
            for(int j=1;j<=leaves;j++){
                printf("*");
            }
            printf("\n");
            leaves=leaves+2;
        }
        branches--;
    }

    // For stand
    for(int i=1;i<=2;i++){
        for(int j=1;j<=max_leaves/2;j++){
            printf(" ");
        }
        printf("*\n");
    }

    return 0;
}

Use whitespace to scan code quickly

Now that we've introduced late declarations, let us add some whitespace between operators. Gone are the days where bytes in source code where expensive. Unless you intend write-only code, make code easy to read. Debugging is much nicer in well-written and spaced code:

#include <stdio.h>

int main() {
    int day;
    scanf("%d", &day);

    // Check for exceptions
    if (day <= 1) {
        printf("You cannot generate christmas tree");
        return 0;
    } else if (day > 20) {
        printf("Tree is no more");
        return 0;
    }

    // printf("Heres Your Christmas Tree:\n\n");

    // For first part
    int leaves = 1;
    int branches = day + 1;
    int max_leaves = 1 + (branches - 1) * 2; // Arithmetic Progression
    for (int i = 1; i <= branches; i++) {
        for (int j = i; j <= max_leaves / 2; j++) {
            printf(" ");
        }
        for (int j = 1; j <= leaves; j++) {
            printf("*");
        }
        printf("\n");
        leaves = leaves + 2;
    }

    // For remaining parts
    branches = branches - 2;
    for (int rest = 1; rest <= day - 2; rest++) {
        leaves = 3;
        for (int i = 2; i <= branches + 1; i++) {
            for (int j = i; j <= max_leaves / 2; j++) {
                printf(" ");
            }
            for (int j = 1; j <= leaves; j++) {
                printf("*");
            }
            printf("\n");
            leaves = leaves + 2;
        }
        branches--;
    }

    // For stand
    for (int i = 1; i <= 2; i++) {
        for (int j = 1; j <= max_leaves / 2; j++) {
            printf(" ");
        }
        printf("*\n");
    }

    return 0;
}

By the way, your text editor should provide some automatic format feature. I formatted the code with clang-format -style="{BasedoNStyle: llvm, IndentWidth: 4}", since that's very similar to your variant.

Don't repeat yourself

Now that we have easy to read code, we can see that there are repeating patterns, namely

for(int k = init; k <= stop; k++) {
    printf("some string");
}

We have at least five occurrences. So let's write a small function:

void repeat_string(int times, const char* str) {
    for(int i = 0; i < times; ++i) {
        printf("%s",str);
    }
}

Let's replace all occurrences of your loop with the function:

#include <stdio.h>

void repeat_string(int times, const char* str) {
    for(int i = 0; i < times; ++i) {
        printf("%s",str);
    }
}

int main() {
    int day;
    scanf("%d", &day);

    // Check for exceptions
    if (day <= 1) {
        printf("You cannot generate christmas tree");
        return 0;
    } else if (day > 20) {
        printf("Tree is no more");
        return 0;
    }

    // printf("Heres Your Christmas Tree:\n\n");

    // For first part
    int leaves = 1;
    int branches = day + 1;
    int max_leaves = 1 + (branches - 1) * 2; // Arithmetic Progression

    for (int i = 1; i <= branches; i++) {
        repeat_string(max_leaves / 2 - i, " ");
        repeat_string(leaves, "*");
        printf("\n");
        leaves = leaves + 2;
    }

    // For remaining parts
    branches = branches - 2;
    for (int rest = 1; rest <= day - 2; rest++) {
        leaves = 3;
        for (int i = 2; i <= branches + 1; i++) {
            repeat_string(max_leaves / 2 - i, " ");
            repeat_string(leaves, "*");
            printf("\n");
            leaves = leaves + 2;
        }
        branches--;
    }

    // For stand
    for (int i = 1; i <= 2; i++) {
        repeat_string(max_leaves / 2; " ");
        printf("*\n");
    }

    return 0;
}

Again, it's easier too read. Also, any optimization we find for repeat_string will now help at all five call sites. Great.

Exercises:

Write repeat_character(int n, char c), which prints a single character c a total of n times.
If we look closely, we can see that the branches look very similar:

for (int i = 1; i <= branches; i++) {           // <---
    repeat_string(max_leaves / 2 - i, " ");     // <---
    repeat_string(leaves, "*");                 // <---
    printf("\n");                               // <---
    leaves = leaves + 2;                        // <---
}

// For remaining parts
branches = branches - 2;
for (int rest = 1; rest <= day - 2; rest++) {
    leaves = 3;
    for (int i = 2; i <= branches + 1; i++) {   // <---
        repeat_string(max_leaves / 2 - i, " "); // <---
        repeat_string(leaves, "*");             // <---
        printf("\n");                           // <---
        leaves = leaves + 2;                    // <---
    }
    branches--;
}

Write a function that draws a single part, e.g. a trapezoid or pyramid width a given width, height, and so on.

Mark read-only values and locations with const

We denoted the str as const char* in order to make sure that we don't accidentally change string's contents. In the same spirit, we can add const to max_leaves:

…
const int max_leaves = 1 + (branches - 1) * 2; // Arithmetic Progression
…

Now typos like max_leaves /= 2 will get cought by the compiler.

Use 0…(n-1) as range, not 1…n

Your ranges use start = 1; i <= end; start++. Since we often work with arrays in C++ and arrays start with 0, it's more common to work with start = 0; start < end; start++. Keep that in mind for future code.

Other remarks

You mix leaves, branches and max_leaves, but their exact meanings are lost for the unprepared reader. Some additional comments can be helpful.

Other than that, the missing whitespace and therefore code structuring was my biggest criticism, but that's a minor point that can get fixed quickly.

Here's the complete code:

#include <stdio.h>

void repeat_string(int times, const char *str) {
    for (int i = 0; i < times; i++) {
        printf("%s", str);
    }
}

void repeat_character(int times, char c) {
    // Exercise 1
}

void draw_part(int height, int start_width, int max_width) {
    // Exercise 2
}

int main() {
    int day;
    scanf("%d", &day);

    // Check for exceptions
    if (day <= 1) {
        printf("You cannot generate christmas tree");
        return 0;
    } else if (day > 20) {
        printf("Tree is no more");
        return 0;
    }

    // printf("Heres Your Christmas Tree:\n\n");

    // For first part
    int branches = day + 1;
    const int max_leaves = 1 + (branches - 1) * 2; // Arithmetic Progression

    // For first part
    draw_part(branches, 1, max_leaves);

    // For remaining parts
    branches -= 2;
    for (int rest = 0; rest < day - 2; rest++) {
        draw_part(branches, 3, max_width);
        branches--;
    }

    // For stand
    for (int i = 0; i < 2; i++) {
        repeat_string(max_leaves / 2; " ");
        printf("*\n");
    }

    return 0;
}


Python Prime Search Slower Optimization I have these 2 version of code. Both are looking for all primes under a certain ceiling value.
The first is testing against the set of all odd numbers.
The second in testing against the set of all 6k-1 and 6k+1.
In theory, the second version should be faster than the first.
However the result states otherwise.
For under 1 million, the first perform for 1.477835 seconds, while the second perform for 1.521462 seconds.
For under 10 million, the first perform for 30.929565 seconds, while the second perform for 32.825142 seconds.
I have not test these under a different machine.
I could not figure out why I'm getting these results. Because, the second version is suppose to eliminate more composite numbers than the first. However, it perform worse, quite significantly.
I suspect this might be caused by my method of implementation. But even if that were the case, I still wouldn't have the slightest idea of why.
Could it be because of I'm having more nested loops? Or is it to do with memory access? Is it maybe a python's quirk where the implementation details matters? Maybe in some way the first version is simpler and therefore can be easily better optimized by python compared to the second?
If someone could provide some explanations, suggestions, even solutions or alternatives, that would be much appreciated.
#!/usr/bin/env python3.8

import sys
import time
import math

if len(sys.argv) != 2:
    print('usage:')
    print('    (ceiling)')
    print('        Find primes less than ceiling.')
    sys.exit(-1)

try:
    ceiling = int(sys.argv[1])
except:
    print('Fail to parse ceiling ({}) as integer.'.format(sys.argv[1]))
    sys.exit(-2)

if ceiling < 3:
    print('Minimum ceiling value is 3.')
    sys.exit(-3)

prime = [3]

# start benchmark
bench_start = time.perf_counter()

# range(start, end, step) does not include end
# step = 2 to avoid checking even numbers
for i in range(5, ceiling + 1, 2):
    # test only against factors less than the square root
    root = math.sqrt(i)
    for j in prime:
        if j > root:
            prime.append(i)
            break
        if i % j == 0:
            break

# end of benchmark
bench_end = time.perf_counter()

# prepend 2 to the prime list since it was skipped
prime.insert(0, 2)

print(prime)
print('Number of, primes less than {0}, found: {1}'.format(ceiling, len(prime)))

# benchmark report
print('Time elapsed: {:f}s'.format(bench_end - bench_start))

sys.exit(0)

#!/usr/bin/env python3.8

import sys
import time
import math

if len(sys.argv) != 2:
    print('usage:')
    print('    (ceiling)')
    print('        Find primes less than or equal to ceiling.')
    sys.exit(-1)

try:
    ceiling = int(sys.argv[1])
except:
    print('Fail to parse ceiling ({}) as integer.'.format(sys.argv[1]))
    sys.exit(-2)

if ceiling < 3:
    print('Minimum ceiling value is 3.')
    sys.exit(-3)

prime = [3]

# start benchmark
bench_start = time.perf_counter()

# range(start, end, step) does not include end
# test only for 6k-1 and 6k+1
# use (ceiling+1)+1 in case of ceiling = 6k-1
for h in range(6, ceiling+2, 6):
    for i in [h-1, h+1]:
        # test only against factors less than the square root
        root = math.sqrt(i)
        for j in prime:
            if j > root:
                prime.append(i)
                break
            if i % j == 0:
                break

# end of benchmark
bench_end = time.perf_counter()

# when ceiling = {6k-1, 6k}, remove 6k+1
if prime[-1] > ceiling:
    prime.pop()

# prepend 2 to the prime list since it was skipped
prime.insert(0, 2)

print(prime)
print('Number of, primes less than or equal to {0}, found: {1}'.format(ceiling, len(prime)))

# benchmark report
print('Time elapsed: {:f}s'.format(bench_end-bench_start))

sys.exit(0)
```

The difference averages to .2 microseconds per number checked.
To see if its the inner loop, try unrolling it in the second program:
...
for i in range(5, ceiling+1, 6):
    # test only against factors less than the square root
    # just use the sqrt of the bigger number to avoid another call to sqrt
    root = math.sqrt(i + 2)

    # tests 6k - 1
    for j in prime:
        if j > root:
            prime.append(i)
            break
        if i % j == 0:
            break

    # tests 6k + 1
    i += 2
    for j in prime:
        if j > root:
            prime.append(i)
            break
        if i % j == 0:
            break
    ...


How to find the unique values from the JSON file? I have JSON files of size data_large(150.1mb). The content inside the file is of type [{"score": 68},{"score": 78}]. I need to find the list of unique scores from each file.

This is what I'm doing:

import ijson  # since JSON file is large, hence making use of ijson

f = open ('data_large')
content = ijson.items(f, 'item') # json loads quickly here as compared to when json.load(f) is used.
print set(i['score'] for i in content) #this line is actually taking a long time to get processed.

Can I make the last line more efficient?  It's currently taking 201 secs to execute.

So… I have spent a bit time on this and generated myself a random JSON file matching your format. Mine is 167 MB in size, so not too far off from yours.

First of all, ijson’s performance is terrible. I’ve tried it multiple times and it’s very slow for me (much slower than your results actually). While the idea sounds great, it took very long to read and parse the file.

Using the normal json module gave me much better results. In fact, reading and parsing the file took only ~20 seconds, of which reading the file was just a very small fraction. Most of the time was actually spent on extracting the scores; generating the set from them again is very quick.

So to speed this up, we obviously want to avoid creating all those intermediary objects in between, and just extract the data we are interested in. In your case where the file has a rather simple structure, we don’t really need to parse it as a JSON object but we can just operate on the string directly and search for the scores.

As we discussed this on the SO Python chat, Jon Clements came up with this solution:

with open('file.json') as f:
    content = f.read()
    print len(set(m.group(1) for m in re.finditer('"score": (\d+)', content)))

This in fact runs in a few seconds, 8 on my machine, and is a multitude faster than anything that utilizes JSON. Of course this highly depends on the format of the file, so if you have additional data inside (which you left out in the question) you might need to adjust this.


Optimized list filtering on implementation of Project Euler 23 I'm new to Haskell, the part of my code I'm concerned with is the isSumAbundant function. Surely there must be a better way, in an imperative language I might use a for loop and break out of the loop if i got greater than n, but it seems like having to filter the list for every single function call is not optimal.

The problem is Problem 23 from project-euler.net.

projectEuler23 =
  let a = abundantNums 28124 in
  sum [x | x <- [1..28124], not $ isSumAbundant x a]

isSumAbundant n a =
  any (\x -> (n - x) `elem` a) (takeWhile (<n) a)

abundantNums n =
  [x | x <- [1..n], (>x) $ sum $ properDivisors x]

properDivisors n =
  let  limit = (floor.sqrt.fromIntegral) n in
  [limit | limit^2 == n]
  ++ ((1:) $ concat [ [x, div n x] | x <- [2..limit - 1], rem n x == 0])

Add type signatures. Without type signatures, your integral types will default to Integer. So let us add type signatures:

projectEuler23 :: Int
projectEuler23 =
  let a = abundantNums 28124 in
  sum [x | x <- [1..28124], not $ isSumAbundant x a]

isSumAbundant :: Int -> [Int] -> Bool
isSumAbundant n a =
  any (\x -> (n - x) `elem` a) (takeWhile (<n) a)

abundantNums :: Int -> [Int]
abundantNums n =
  [x | x <- [1..n], (>x) $ sum $ properDivisors x]

properDivisors :: Int -> [Int]
properDivisors n =
  let  limit = (floor.sqrt.fromIntegral) n in
  [limit | limit^2 == n]
  ++ ((1:) $ concat [ [x, div n x] | x <- [2..limit - 1], rem n x == 0])

Note that a type signature on projectEuler23 would have been enough. Operations on Int are usually faster than Integer.

In isSumAbundant you can stop at n`div`2, e.g.

isSumAbundant n a = any (\x -> (n - x) `elem` a) $ takeWhile (<= (n `div` 2)) a

Similar in abundantNums, you can use scanl1:

abundantNums n =
  [x | x <- [1..n], any (x <) $ scanr1 (+) $ properDivisors x]

This models your "break" from imperative languages.


Hold a text input for 10s as readonly Problem Statement

  Write a HTML page holding a form and a text field. Using JavaScript make the text field to accept numbers only. When a non-number character is entered through the keyboard (or by any other way), make the field red for a while and do not accept the change (preserve the previous value of the field).

  Expected Output

Solution

<!DOCTYPE html>
<html>
    <head>
        <title>Text field</title>
        <meta charset="UTF-8">
        <script type="text/javascript">
            window.onload = function(){
                document.formex.numberinput.onchange = checkIfNumber;
                function checkIfNumber(){
                     if (isNaN(parseInt(document.formex.numberinput.value, 10))){
                        document.formex.numberinput.innerHTML = document.formex.numberinput.value;
                        document.formex.numberinput.readOnly = true;
                        setTimeout(function(){
                           document.formex.numberinput.readOnly = false;
                        }, 10000);
                      }
                 }
            }
        </script>
        <style type="text/css">
                input[readonly] {
                   background-color: red;
                }
        </style>
    </head>
    <body>
        <form name="formex">
            <input type="text" name="numberinput">
        </form>
    </body>
</html>

Above code uses name attributes as element lookup approach.

Can we improve this code?

Your code doesn't follow the Single Responsibility Principle.

What do you need to do :

Check the text change
Verify if the input contains a number
Potentially change the state of the input (disable/turn red the input)
Re-enable it.

Right now, you have one function that takes care of all this, we should split it. Because, checkIfNumber shouldn't disable a textbox, that doesn't make sense! :)

You need to have functions that aren't coupled to your inputs, so we'll try to introduce parameters instead of accessing the document all the time.

function isNumber(value) {
    return !(isNaN(parseInt(value, 10)));
}

Notice I could have create a isNotANumber function, but that's not good as it's confusing. Also, this function doesn't need to be in the window.onload scope, it could very well be reused anywhere else.

Next, we want to disable the input and turn it red. I wouldn't write a function only to disable it. But, writing a function to disable an element for a time span seems like a good plan :

function disableFor(input, time) {
    input.readOnly = true;
    setTimeout(function() {input.readOnly = false;}, time);
}

Finally we need to run the time out for 10 seconds.

window.onload = function(){
    //or document.getElementsByName("numberinput")[0];
    var numberInput = document.formex.numberinput;

    numberInput.onChange = function() {
        if(isNumber(numberInput.value)) { return; }

        //I'm not sure why you do this.
        numberInput.innerHTML = numberInput.value;
        disableFor(numberInput, 10000);
    };
}

function isNumber(value) {
    return !(isNaN(parseInt(value, 10)));
}

function disableFor(input, time) {
    input.readOnly = true;
    setTimeout(function() {input.readOnly = false;}, time);
}

Notice that I inverted your condition to reduce nesting. So I just return if the value is a number, there's less nesting and well.. That's more readable!


Finding an element in multiple sorted lists efficiently I have \$k\$ sorted Lists, each of size \$n\$. Currently I have hard-coded 5 sorted Lists each of size 3, but in general that is configurable.

I would like to search for single element in each of the \$k\$ Lists (or its predecessor, if it doesn't exist).

Obviously I can binary search each List individually, resulting in a \$O(k*log(n))\$ runtime. But I think we can do better than that: after all, we're doing the same search \$k\$ times.

Could this be improved?

private static TreeSet<Integer> tree = new TreeSet<Integer>();

public SearchItem(final List<List<Integer>> inputs) {
    tree = new TreeSet<Integer>();
    for (List<Integer> input : inputs) {
        tree.addAll(input);
    }
}

public Integer getItem(final Integer x) {
    if(tree.contains(x)) {
        return x;
    } else {
        return tree.higher(x);
    }
}

public static void main(String[] args) {
    List<List<Integer>> lists = new ArrayList<List<Integer>>();

    List<Integer> list1 = new ArrayList<Integer>(Arrays.asList(3, 4, 6));
    List<Integer> list2 = new ArrayList<Integer>(Arrays.asList(1, 2, 3));
    List<Integer> list3 = new ArrayList<Integer>(Arrays.asList(2, 3, 6));
    List<Integer> list4 = new ArrayList<Integer>(Arrays.asList(1, 2, 3));
    List<Integer> list5 = new ArrayList<Integer>(Arrays.asList(4, 8, 13));

    lists.add(list1);
    lists.add(list2);
    lists.add(list3);
    lists.add(list4);
    lists.add(list5);

    SearchItem search = new SearchItem(lists);
    System.out.println(tree);

    Integer dataOuput = search.getItem(5);

    System.out.println(dataOuput);
}

I think you have some misguided assumptions here.

For a start, what you have now is not \$O(k \log(n))\$, it first scans and sorts all the data in to the tree, which is a \$O(N \log(N))\$ operation, where \$N\$ is the cumulative data size (sum of input-array sizes). Then, the check on it, is a \$O(\log(N))\$ check, so, your algorithm is in the order of \$O(N\log(N))\$.

The complexity you are worried about \$O(k\log(n))\$ is really quite trivial. \$k\$ is a small number (5), and log(n) is always small... in many ways, after n = 128 it is effectively a constant....

So, I would do the following:

binary search each List
keep the 'min' value

with the code:

Integer result = null;
for (List<Integer> data : lists) {
    int pos = Collections.binarySearch(data, input);
    if (pos >= 0) {
        //exact match, return
        return data.get(pos);
    }
    pos = -pos - 1;
    if (pos < data.size()) {
        Integer found = data.get(pos);
        if (result == null || result.compareTo(found) > 0) {
            result = found;
        }
    }
}
return result;

Now, if you wanted to be fancy, and you wanted the fastest response times, you could put each binary search in to a Callable<Integer>, and run them in parallel....

Then rank the results, and teturn it all in time complexity \$O(\log(n))\$ assuming \$k\$ is less than your hardware CPU count.


Removing first duplicate dataframe I have a dataframe with several columns. One of them is an user ID column, in this column, I have several ids that can be repeated several times.

What I want to do is remove the first ID, for instance:

1,2,3,4,3,4,2,1,3,4,6,7,7

I would like to have an output like this:

3,4,2,1,3,4,7

Where is what I have done:

#find first duplicated of the each user
dup <- duplicated(results$user)

#create other data frame, every time vector is TRUE add the row to new dataframe
results1 <- NULL

for(i in 1:length(results$user)){
    if (dup[i] == TRUE) {
        rbind(results1, results[i,]) -> results1
    }
 }

Since I'm more used to think in Python, I have a feeling this is a very ugly solution for R. I would like to have some feedback, as well as some pointers on how to improve this piece of code.

Here's a more efficient solution:

# an example data frame
results <- data.frame(user = c(1,2,3,4,3,4,2,1,3,4,6,7,7), a = 1)

# the solution
results[duplicated(results$user), ]

How it works: duplicated returns a logical vector indicating whether a value was also present at a preceding position in the vector (for each value of results$user).

This logical index is used to choose the appropriate lines of the orginal data frame. This is achieved by using this vector as the first argument for [ and using an empty second argument (to select all columns).

The result:

   user a
5     3 1
6     4 1
7     2 1
8     1 1
9     3 1
10    4 1
13    7 1


Correct way to center logo and navigation I'm new to CSS and HTML and while I've achieved a centering of the logo and navigation links, it feels wrong.  I'm developing a site for a small non-profit for a side project.  I have a logo next to a few navigation links.  I put the logo and navigation links in a list.

Could someone take a look at this and tell me if my CSS is correct?  I just think that there's something off.  Or that I'm duplicating CSS code.

Here's my JSFiddle to show what I've done.

HTML:

WHO WE ARE

WHAT WE DO

EXPLORE

DONATE

CSS:

#top_nav {
    text-align: center;
    width: 100%;
}

#nav_logo {
    /*display: inline-block;*/
    width: 142px;
    height: 159px;
    padding-right: 20px;
    text-decoration: none;
}

#nav_links li a {
    padding: 15px 20px;
    text-decoration: none;
    font-family: Arial;
    font-size: 24px;
    color: #000;
}

/* When window is resized, doesn't wrap menu*/
#nav_links {
    display: inline-block;
    padding: 0 0 30px 0;
    list-style:none;
    text-align:left;
    white-space: nowrap;
}

#nav_links li {
    vertical-align: text-top;
    display: inline-block;
    float: none;
    margin: 0 -3px 0 0;

}
/* ------------------------------------*/

a.nav_links_border {
    border-right: 1px solid #f37f43;
}

#nav_links li a:hover {
    text-decoration: underline;
}

#nav_links > img:hover {
    text-decoration: none;
}

#bottom_nav {
    clear: both;
}

There is no real "correct" way to go about it; it all depends on how you want it to look. I can't quite speak to that, only the code you posted, and how it fares.

Anyway: Review.

General cleanliness

You should clean up your code (preferably before posting it, since many the following things should be fairly obvious). You've got a bunch of CSS that doesn't do anything:

There's a line that's just commented out: Remove it.
The #nav_links > img:hover doesn't affect anything. There's no img element immediately descendent from #nav_links
The #bottom_nav style (and indeed the entire element!) is pointless. It seems only to exist to clear any floats. But there are no floats. Similarly, the #nav_links li style has a float: none declaration which does nothing.
You can skip every single link ID (e.g. nav_whoweare); you're not using them. If you find yourself needing them, then add them. But not before.
You don't need width: 100% on the nav element. It's a block element, so it'll do that automatically.
You don't need display: inline-block on the ul element.

I'd also advice you to use a more logical order for you CSS rules. Right now it's haphazard:

#nav_links li a
#nav_links            << high-level
#nav_links li
#nav_links li a:hover << low-level

but it would make more sense to mirror the structure of the markup (and just the complexity of the CSS selectors):

#nav_links            << high-level
#nav_links li
#nav_links li a
#nav_links li a:hover << low-level

Class names and IDs

"It should be as simple as possible, but no simpler". In other words, only add stuff when there would otherwise be ambiguity.

For instance, there no reason to give the #nav_links list an ID. It's the only
 element inside #top_nav, so you can unambiguously reference it as simply #top_nav ul. Or, if you get rid of the bottom_nav element since it's not doing anything, you're left with simply nav ul. (You might want a bottom nav later, but cross that bridge when you come to it; not now).

Furthermore, don't use classes like nav_links_border. It's given that it's a navigation link, because it's inside the  element, and it's given that it's a link because it's an  element. So you CSS rule should target nav a rather than an overly specific class like a.nav_links_border.

What you're left with is just border, but that's not terribly descriptive. Looking at it overall, though, it's also obvious that all the links - with 1 exception - have a border. So, really, you'll want a way to treat the exception differently, rather than add a class to everything else.

I'd suggest giving the
 containing the logo an ID - simply call it logo. This lets you do two things:

You can style all the list items the same basic way, and only do something different for #logo
You can get rid of the ID on the  element, since you can instead reference it as #logo img

I get this:

WHO WE ARE

WHAT WE DO

EXPLORE

DONATE

And this:

nav {
    text-align: center;
}

nav ul {
    padding: 0 0 30px 0;
    list-style:none;
    text-align:left;
    white-space: nowrap;
}

nav li {
    vertical-align: text-top;
    display: inline-block;
    margin: 0 -3px 0 0;
}

nav a {
    padding: 15px 20px;
    text-decoration: none;
    font-family: Arial;
    font-size: 24px;
    color: #000;
    border-right: 1px solid #f37f43;
}

nav a:hover {
    text-decoration: underline;
}

#logo a {
    border: none;
}

#logo a:hover {
    text-decoration: none;
}

#logo img {
    width: 142px;
    height: 159px;
    padding-right: 20px;
}

Here's a jsfiddle - should look the same (apart from less margin, but that's because I remove the  tag from your code; jsfiddle add that itself; you shouldn't do it yourself)


Mersenne Prime Oracle Please give me any feedback regarding good coding standards and readability.

fun main(arg: Array<String>) {

    //do while user guess != Mersenne Prime.
    do {
        //initializing variable
        var number: Int = 3
        try {
            number = readLine()!!.toInt()

            if (isMersennePrime(number))
            println("${number} is a Mersenne Prime. Congratulations.")

            else
            println("${number} is not a Mersenne Prime.")
        }
        catch (e: NumberFormatException) {
            println(" is not a valid number. Exiting.")
        }
    } while (!isMersennePrime(number))
}

    // is Number both prime and 2^n - 1?
    fun isMersennePrime(Number: Int): Boolean =
    (twonminus1(Number.toDouble()) && isPrime(Number))

    // is x Prime?
    fun isPrime(x: Int): Boolean = (2..x-1).all{x % it != 0 && x != 2}

    // is x = 2^n - 1?
    fun twonminus1(x: Double): Boolean = Math.log(x + 1.0)/Math.log(2.0) % 1.0 == 0.0

Do not use double arithmetics when integer arithmetics can solve the problem as well. The function isPowerOfTwoMinus1 can be written simply as ((x + 1) & x) == 0.

Don't call isMersennePrime twice, just break from the loop.


Faster code for leetcode reverse int This is my solution for the leetcode reverse int question:

  Given a 32-bit signed integer, reverse digits of an integer.

  Example 1:

  Input: 123 Output: 321 Example 2:

  Input: -123 Output: -321 Example 3:

  Input: 120 Output: 21 Note: Assume we are dealing with an environment
  which could only store integers within the 32-bit signed integer
  range: [−2^31,  2^31 − 1]. For the purpose of this problem, assume that
  your function returns 0 when the reversed integer overflows.

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace ArrayQuestions
{
    /// <summary>
    /// Given a 32-bit signed integer, reverse digits of an integer.
    ///
    /// Example 1:
    ///
    /// Input: 123
    /// Output: 321
    /// Example 2:
    ///
    /// Input: -123
    /// Output: -321
    /// Example 3:
    ///
    /// Input: 120
    /// Output: 21
    /// Note:
    /// Assume we are dealing with an environment which could only store integers within the 32-bit signed integer range: [−231,  231 − 1]. For the purpose of this problem, assume that your function returns 0 when the reversed integer overflows.
    /// </summary>
    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            int temp = 1534236469;
            int result = Reverse(temp);
            Assert.AreEqual(0, result);
        }

        [TestMethod]
        public void TestMethod2()
        {
            int temp = 123;
            int result = Reverse(temp);
            Assert.AreEqual(321, result);
        }
        public int Reverse(int x)
        {
            int result = 0;
            bool negative = false;
            if (x < 0)
            {
                negative = true;
                x = -x;
            }

            int prevRevNum = 0;
            while (x != 0)
            {
                int currentDigit = x % 10;
                result = (result * 10) + currentDigit;

                if ((result - currentDigit) / 10 != prevRevNum)
                {
                    return 0;
                }
                prevRevNum = result;
                x = x / 10;
            }

            if (negative)
            {
                return -result;
            }

            return result;
        }
    }
}

This is the output:

  Runtime: 52 ms, faster than 62.65% of C# online submissions for Reverse Integer.

Can you please comment on how can I make this code run faster? I guess the modulo and division are the key factors of speed here.

Using a string instead of division / modulo seems to be a few percent faster than the solution from the question:

    public static int Reverse(int x)
    {
        var str = x.ToString();
        int res = 0;
        int multiplier = 1;
        var negative = str[0] == '-';
        for (int i = negative ? 1 : 0; i < str.Length; i++)
        {
            var num = str[i] - 48;
            if (num > 0 || multiplier > 1)
            {
                res += num * multiplier;
                multiplier *= 10;
            }
        }
        return negative ? -res : res;
    }

However, improving the solutions from the question using System.Math.DivRem (as mistertribs suggested in a comment) will be even faster :):

    public static int Reverse(int x)
    {
        var result = 0;
        var negative = x < 0;
        if (negative) x = -x;

        while (x != 0)
        {
            x = Math.DivRem(x, 10, out var rest);
            result = result * 10 + rest;
        }

        return negative ? -result : result;
    }