Login page without header/footer bars I'm creating an app where the login page doesn't have the header/footer bars that the pages, after being logged in, would have.  I've created an example that works, but I'm unsure if I am using the prop/component inheritance or patterns correctly.

I feel like there is a lot of nesting going on here, but I'm unsure if that's the proper thing to experience or if I'm creating a mess for myself.

Here's my main Router:

export default (
  <Router history={hashHistory}>
    <Route path="/" component={App}>

      <Route path="login" component={Login} />

      <Route path="*" component={NotFoundPage} />

    </Route>
  </Router>
);

Here's my main app trigger that initializes and renders everything:

class App extends React.Component {

  render() {
    return (

        {this.props.children}

    );
  }

}

export default App;

Then, I created a Layout component to serve either a version with or without the header and footer:

class Layout extends React.Component {

  constructor(props) {
    super(props);
  }

  renderPageLayout() {
    if(this.props.type == 'wrap') {
        return(

                {this.props.children}

          );
    } else {
        return(

            {this.props.children}

          );

    }
  }

  render() {
    return(

        { this.renderPageLayout() }

      );
  }

}

export default Layout;

And my login page should not have a header/footer on it, so it is as follows:

class Login extends React.Component {

  constructor(props) {
    super(props);
  }

  render() {
    return (
      <Layout type="no-wrap">

        Login

      </Layout>
    );
  }

}

export default Login;

I'm having trouble finding examples of similar things that are on a basic level and don't contain a bunch of ReactJS things that I haven't yet learned.

First of all, try to write code for pure components without using a class:

It will be your App.js

const App = ({ children }) => (

    {children}

);

export default App;

It will be your Layout.js

const Layout = ({ children, type }) => (

    type == 'wrap' ? (

          {children}

    ) : (

         {children}

    )

);

export default Layout;

It will be your Login.js

const Login = () => (
  <Layout type="no-wrap">
    Login
  </Layout>
);

export default Login;

If you don't need to use a Layout component inside the Login component, just don't do it.

For example:

Layout.js

const Layout = ({ children }) => (

      {children}

);

export default Layout;

LayoutNoWrap.js

const LayoutNoWrap = ({ children }) => (

    {children}

);

export default LayoutNoWrap;

Login.js

const Login = () => (
  <LayoutNoWrap>
    Login
  </LayoutNoWrap>
);

export default Login;


Hailstone Sequences in Python For the problem given:

  Douglas Hofstadter’s Pulitzer-prize-winning book, Gödel, Escher, Bach, poses the following mathematical puzzle.

  Pick a positive integer \$n\$ as the start.
  If \$n\$ is even, divide it by 2.
  If \$n\$ is odd, multiply it by 3 and add 1.
  Continue this process until \$n\$ is 1.

  The thesis is: The number \$n\$ will travel up and down but eventually end at 1 (at
  least for all numbers that have ever been tried -- nobody has ever
  proved that the sequence will terminate). Analogously, hailstone
  travels up and down in the atmosphere before eventually landing on
  earth.

  The sequence of values of n is often called a Hailstone sequence, because hailstones also travel up and down in the atmosphere before falling to earth. Write a function that takes a single argument with formal parameter name \$n\$, prints out the hailstone sequence starting at \$n\$, and returns the number of steps in the sequence.

  Hailstone sequences can get quite long! Try 27. What's the longest you can find? Fill in your solution below:

def hailstone(n):
    """Print the hailstone sequence starting at n and return its length.

    >>> a = hailstone(10)  # Seven elements are 10, 5, 16, 8, 4, 2, 1
    10
    5
    16
    8
    4
    2
    1
    >>> a
    7
    """
    "*** YOUR CODE HERE ***"

Below is the solution written for above problem that performs hailstone sequence:

def hailstone(n):
    count = 1
    """Print the terms of the 'hailstone sequence' from n to 1."""
    assert n > 0
    print(n)
    if n > 1:
        if n % 2 == 0:
            count += hailstone(n / 2)
        else:
            count += hailstone((n * 3) + 1)
    return count

result = hailstone(10)
print(result)

With the above solution, I would like to confirm that this program follows functional paradigm instead of imperative paradigm with good abstraction.

I would like to understand if this program can still be improved from paradigm perspective.

The docstring needs to be the first thing that appears in your function.

In functional programming, it is not proper to ever change the value of a variable.

If you know that you need integer division rather than floating-point division, use // instead of /.

A common idiom for recursion is

def recursive_function(input):
    if base_case_applies(input):
        return base_value
    else:
        return …(recursive_function(…))

Your recursive function would therefore be better expressed as:

def hailstone(n):
    """Print the terms of the 'hailstone sequence' from n to 1, and
    return the length of the sequence."""
    assert n > 0
    print(n)
    if n == 1:
        return 1
    elif n % 2 == 0:
        return 1 + hailstone(n // 2)
    else:
        return 1 + hailstone(3 * n + 1)

An even more functional programming approach, which admittedly looks horrible in Python and therefore isn't something I recommend, is to use a single expression for the recursion.

def hailstone(n):
    """Print the terms of the 'hailstone sequence' from n to 1, and
    return the length of the sequence."""
    assert n > 0
    print(n)
    return 1 if n == 1 else \
           1 + hailstone((n // 2) if n % 2 == 0 else (3 * n + 1))


Array alive/dead entity "refresh" algorithm I have an Entity class that can be either dead or alive.

I store entities contiguously in a resizable array. During an update(), some alive entities can become dead, and some new entities can be added at the end of the array.

The range [0 .. size) contains all non-newly-created entities.

The range [size .. sizeNext) contains all newly-created entities (guaranteed to be alive).

After update(), I call refresh(). This function does:

Swap around entities making sure that all alive entities are stored contiguously at the beginning of the array (and that all dead entities are stored contiguously at the end of the array).
Deinitialize all dead entities.
Update size and sizeNext.

Basically, the algorithm uses two iterators:

iD moves from left to right, looking for dead entities.
iA moves from right to left, looking for alive entities.

When iD and iA find an entity, the entities are swapped and the iterators advance.

Here's a quick drawing of the algorithm:

And here's the current code:

void refresh()
{
    // `sizeNext` is unsigned - copy it as a signed value
    // to store its initial value and compare it to integers.
    const int intSizeNext(sizeNext);

    // `iD` walks from left to right, looking for dead entities.
    // `iA` walks from right to left, looking for alive entities.
    int iD{0}, iA{intSizeNext - 1};

    do
    {
        // Find dead item from left
        for(; true; ++iD)
        {
            // No more dead items
            if(iD > iA) goto finishRefresh;

            if(isDeadAt(iD)) break;
        }

        // Find alive item from right
        for(; true; --iA)
        {
            // No more alive items
            if(iA <= iD) goto finishRefresh;

            if(isAliveAt(iA)) break;
        }

        assert(isDeadAt(iD) && isAliveAt(iA));
        std::swap(items[iD], items[iA]);
        assert(isAliveAt(iD) && isDeadAt(iA));

        // Advance both iterators
        ++iD; --iA;
    }
    while(true);

    finishRefresh:
    for(iA = iA - 1; iA >= 0; --iA)
    {
        assert(isAliveAt(iA));
    }

    msize = sizeNext = iD;

    for(; iD < intSizeNext; ++iD)
    {
        assert(isDeadAt(iD));
        deinitAt(iD);
    }
}

Can the code be improved/optimized?
Can the goto instructions be avoided?
Is there a more efficient algorithm that achieves the same results?

Full code is here.

You could probably use std::remove_if instead of your algorithm. Take this example code:

std::vector<std::string> vec = { "0", "1", "5", "0", "3", "6", "0", "1" };
std::remove_if(std::begin(vec), std::end(vec), [](const std::string& val)
{
    return val == "0";
});

for (auto&& a: vec)
{
    std::cout << a << ' ';
}

I get the following output:

1 5 3 6 1 0 0 0

std::remove_if swaps the elements of a range so that the elements that are to be kept are at the beginning of the range and maintains their order. The elements that are still at the end have unspecified values, but if Entity is an RAII class, the deinitialization will happen at destruction. Since I don't know what Entity looks like, I can't be sure that this method will work, but there is probaby a way to ensure that it will do what you need it to do.

This method has two advantages: it allows to think in term of iterators instead of sizes and it uses an already existing algorithm. In your case, refresh could probably look like this:

void refresh()
{
    std::remove_if(std::begin(entities), std::end(entities),
                   [](const Entity& ent)
                   {
                       return ent.isDead();
                   });
}

Once again, I am assuming some implementation details that are not shown though. Anyway, try check whether std::remove_if coud be used to do what you want it to do. Standard algorithms can often do much more things than thought.


Doubly linked list in C While I'm training to get better at C, I tried my hand at making a doubly linked list implementation.

linkedlist.h

#ifndef LINKED_LIST_H_
#define LINKED_LIST_H_

#include <stdlib.h>

typedef struct node {
    void* data;
    struct node *next;
    struct node *prev;
} node;

typedef node* pnode;

typedef struct List {
    pnode head, tail;
} *List;

List init_list();
void push_back(List, void*);
void* pop_back(List);
void push_front(List, void*);
void* pop_front(List);
void foreach(List, void (*func)(void*));
void free_list(List);
void clear(List);
int size(List);

#endif /* LINKED_LIST_H_ */

linkedlist.c

#include "linkedlist.h"

List init_list() {
    List list = (List)malloc(sizeof(struct List));
    list->head = NULL;
    list->tail = NULL;
    return list;
}

void push_back(List list, void* data) {
    pnode temp = (pnode)malloc(sizeof(struct node));
    temp->data = data;
    temp->next = NULL;
    temp->prev = NULL;

    if(!(list->head)) {
         list->head = temp;
         list->tail = temp;
    } else {
        list->tail->next = temp;
        temp->prev = list->tail;
        list->tail = list->tail->next;
    }
}

void push_front(List list, void* data) {
    pnode temp = (pnode)malloc(sizeof(struct node));
    temp->data = data;
    temp->next = NULL;
    temp->prev = NULL;

    if(!(list->tail)) {
         list->head = temp;
         list->tail = temp;
    } else {
        list->head->prev = temp;
        temp->next = list->head;
        list->head = list->head->prev;
    }
}

void* pop_front(List list) {
    if(!(list->tail)) return NULL;
    pnode temp = list->head;

    if(list->head == list->tail) {
        list->head = list->tail = NULL;
    } else {
        list->head = list->head->next;
        list->head->prev = NULL;
    }
    void* data = temp->data;
    free(temp);
    return data;
}

void* pop_back(List list) {
    if(!(list->head)) return NULL;
    pnode temp = list->tail;

    if(list->tail == list->head) {
        list->tail = list->head = NULL;
    } else {
        list->tail = list->tail->prev;
        list->tail->next = NULL;
    }
    void* data = temp->data;
    free(temp);
    return data;
}

void free_list(List list) {
    while(list->head) {
        pop_back(list);
    }

    free(list);
}

void clear(List list) {
    while(list->head) {
        pop_back(list);
    }
}

void foreach(List list, void(*func)(void*)) {
    pnode temp = list->head;

    if(temp) {
        while(temp) {
            (*func)(temp->data);
            temp = temp->next;
        }
    }
}

int size(List list) {
    int i = 0;
    pnode temp = list->head;
    while(temp) {
        i++;
        temp = temp->next;
    }

    return i;
}

Can you please point out any flaws, errors, stupid mistakes, general improvements, which can make this code better and "C'ish"?

Welcome on Code Review

Generalities

Includes

You don't need #include <stdlib.h> in linkedlist.h, so remove it and instead, just add it in linkedlist.c  right after #include linkedlist.h.

Name collisions

I'll discuss no more this subject after but be aware that names like node, list, size or clear for example, are very usual and subject to collisions. Consider using more robust names, maybe with a prefix.

Usable interface

Since the user look at your header file to know how to use your functions, a good habit is to to name the arguments, that's make the interface more explicit. Furthermore, adding documentation as comments about your interface help them to use it correctly.

Use the const keyword

Read this and this to know when et how.

Assertions and error-checking

You could use assertions to check preconditions, postconditions and invariants. It make your code more explicit and you avoid possibly broke you code when modifying.

How to use assertions in C
How and When to Use C's assert() Macro
Why should I use asserts? (for C++ but still applicable)

struct node

[typedef] struct [tag] { ... } [alias];

Personally, I try to avoid to use sames ''struct tag'' and ''typedef name'' if I have to typedef a struct. It's completely a matter of taste, since "modern" compilers (for more than 20 years)  can handle this easily, but so, user know when he work with the tag or the alias.

But there are many others "conventions":

Some prefer "untagged aliased struct" (if there are no self-referencing members).
Where others says to never typedef a struct.
...

Consistency

No matter where you place asterisk for pointers, try to be consistent through your code:

void* data;
    ^ LEFT-ALIGNED
struct node *next;
            ^ RIGHT-ALIGNED

(here is another talk on this endless debate)

So IMHO, this is cleaner:

typedef struct node node_t;
typedef node_t* node_ptr;

struct node {
    void* data;
    node_ptr next;
    node_ptr prev;
};

struct List

Consistency

The first letter of the struct node is lowercase, while the first of the struct List is uppercase.

Variables declaration

Try to don't declare several variables on the same line, it will avoid you many problems.

typedef struct list list_t;
typedef list_t* list_ptr;

struct list {
    node_ptr head;
    node_ptr tail;
};

init_list(), push_back(), push_front()

There's no need to cast the return type of malloc from void* since it's implicitly converted to any pointer type.
You can also omit the struct  as you aliases it.
Optionally, you could make more concise using calloc and rid of the manual initialization of next and prev (with few costs).
Since you have a free_list function, consider renaming init_list to  alloc_list
For push_* function, you should use sizeof *list->head  instead of using the type. It's easier to maintain. (e.g. if you modify the node type latter, the change here is automatic)
For push_* function, you could return an int, an enum or a bool (via <stdbool.h>)  to indicate the success of the insert (since the malloc can fail) or a pointer to the new created node (and so NULL in case of fail). No matter how but you should always handle errors when using malloc

pop_back(), pop_front()

I find multiples assignments on same line less readable, but that's my opinion.
For info: On some not-wide-used platforms (PalmOS, 3BSD, and other non ANSI-C compatibles), you should check for NULL before freeing to avoid crashes.

clear()

You could rewrite the cleaning algorithm to be more efficient since you discard return values.

free_list()

Simply use clear before freeing, to avoid code duplication.


Calculate percentage of values in an array with a certain condition I had to write a function that calculates the percentage of values in table less or equal to 2.  Is my code right or is there anything missing?

Public Function intPercentage As Integer

 Dim intAge( , ) As Integer = {{0, 2, 5, 10}

 {2, 0, 1, 3}

 {5, 1, 0, 6}

 {10, 3, 6, 0}}

Dim intRow, intCol As Integer

For intRow= 0 to 3

 For intCol = 0 to 3

 If intAge(intRow, intCol) >= 2 Then

 intTotal = intTotal + intAge(intRow, intCol)

 Next intCol

   Next intRow

 intPercentage = intTotal / 100

 End Function

Naming

Kill the Hungarian Notation. Kill it with fire. Don't bother taking it out back to bury it.

Hungarian notation can be done right, but using it to tell yourself what datatype a variable is is not doing it right. Even if you must just insist on using hungarian notation this would not be right.

Dim intAge( , ) As Integer = {{0, 2, 5, 10}

intAge is not an integer. It is an array of arrays of integers.

Functions/Subs should have Verb-Noun style names. They should also be PascalCased.

Indentation

It's important. Like, really really important. Proper indentation makes it orders of magnitude easier to read code. Considering code will be read many more times than it will be written, it's important to make sure people can read it.

Everything inside of Function...End Function should be indented one level. Everything inside of For...Next should be indented one more level. Same goes for If...End If blocks.

Getting Down to Business

A percentage is a double, not an integer.

Public Function intPercentage As Integer

If you want an integer representation of the percentage, you should probably leave a comment explaining why.
A function isn't much good if it will return the same results every time. This one will because you've hardcoded the array of arrays into the function. It would be much more useful to accept a 2-dimensional array as an argument instead.
I'm not personally a fan of declaring multiple variables on a single line, but there's technically nothing wrong with it.

Dim intRow, intCol As Integer

Since we're now going to accept an array as an argument, we'll no longer be able to hardcode For row = 0 to 3. You'll need to call GetLength on the internal array and Length on the outer one. (Example will be below.)
You never declared intTotal. (Which you're going to call totalAge now, right?)
Don't skimp on the End Ifs. Always be sure to close the If block unless you really write a one liner. (I don't recommend one liners by the way.)
Prefer the Return keyword over assigning values to the function name.
You can't just divide a number by 100 and call it a percentage.

intPercentage = intTotal / 100

In order to get the results you're really after, you need to divide the count of ages 2 or older by the count of all ages, then multiply by 100.

I took the liberty of re-writing the code.

Public Function GetPercentage(ages(,) As Integer) As Double

    Dim row As Integer
    Dim col As Integer
    Dim overTwo As Integer
    Dim total As Integer

    For row = 0 to ages.Length - 1
        total = total + ages.GetLength(row)

        For col = 0 to ages.GetLength(row) - 1
            If ages(row, col) >= 2 Then
                overTwo = overTwo + 1
            End If
        Next col
    Next row

    Return (overTwo / total) * 100
End Function


advice on polymorphism Im trying to teach myself enough to start using more OOP in php.  This is what Ive come up with. Is there any reason why any of this is done wrong, or should be done another way?

    class workers {
            private $name;

            function __construct($name){
                $this->name=$name;
            }

            function setName($name){
                $this->name=$name;
            }

            function doPrint($who){
                print $who->name . ' is ' . $who->isWhat() .'<br>';

            }

    }

    class boss extends workers{

            function isWhat(){
                return 'the boss';
            }

    }

    class bee extends workers{

            function isWhat(){
                return 'not the boss';
            }
    }

    $theboss =new boss('gordon');
    $thebee= new bee('johnny');

        workers::doPrint($theboss);
        workers::doPrint($thebee);

    $theboss->setName('johnny');
    $thebee->setName('gordon');

        workers::doPrint($theboss);
        workers::doPrint($thebee);

prints:

gordon is the boss
johnny is not the boss
johnny is the boss
gordon is not the boss

you should use an abstract class and an interface. also define the doPrint method as static when you want to call it static.

abstract class worker {

    /*...*/

    public static function doPrint(worker_interface $who) {

        /*...*/
    }
}

interface worker_interface {

    public function isWhat();
    public function setName($name);
}

class boss extends worker implements worker_interface {

    public function isWhat() {

        return 'the boss';
    }
}

class bee extends worker implements worker_interface {

    public function isWhat() {

        return 'not the boss';
    }
}

define the type for all methods you are passing worker instances to like in doPrint

EDIT:

alternatively you could also just add an abstract function instead of using an interface

abstract class worker {

    /*...*/

    public static function doPrint(worker $who) {

        /*...*/
    }

    public abstract function isWhat();
}

class boss extends worker {

    public function isWhat() {

        return 'the boss';
    }
}

class bee extends worker {

    public function isWhat() {

        return 'not the boss';
    }
}


Getting max and min values from Array My code works; I am just seeing if there's an optimized way to write these methods or even better perhaps combine them (since majority of their contents look similar).

public class IntegerUtils {

    public static int getMaxValue(int[] array) {
        int value = Integer.MIN_VALUE;

        if (array.length <= 0) {
            throw new IllegalArgumentException("Array is empty.");
        }

        for (int i = 0; i < array.length; i++) {
            if (array[i] > value) {
                value = array[i];
            }
        }
        return value;
    }

    public static int getMinValue(int[] array) {
        int value = Integer.MAX_VALUE;

        if (array.length <= 0) {
            throw new IllegalArgumentException("Array is empty.");
        }

        for (int i=0; i < array.length; i++) {
            if (array[i] < value) {
                value = array[i];
            }
        }
        return value;
    }
}

Here's my unit test (which works as well):

import org.junit.Test;

public class IntegerUtilsTest {

    int[] array = new int[] {1, 2, 3, 58, 47, 229, 40};

    @Test
    public void shouldBeMaxValue() {
        int maxValue = IntegerUtils.getMaxValue(array);
        assert(maxValue == 229);
    }

    @Test
    public void shouldBeMinValue() {
        int minValue = IntegerUtils.getMinValue(array);
        assert(minValue == 1);
    }

    @Test(expected = IllegalArgumentException.class)
    public void shouldBeIllegalArgumentException() {
        int[] emptyArray = new int[] {};
        int maxValue = IntegerUtils.getMaxValue(emptyArray);
        int minValue = IntegerUtils.getMinValue(emptyArray);
    }
}

Just seeking feedback regarding optimization and / or code elegance.

Real skill means knowing your libraries. Instead of 20 lines of handcrafted code, I'd rather go for:

int max = Arrays.stream(array).max().orElseThrow(() -> new IllegalArgumentException("Array is empty"));

... the same for min.

As a general advice: Java is a very mature language, and chances are good that every "simple everyday task" is already solved in the library. (Also note that most books on Java are from the pre-java-8 era and may be considered somewhat outdated today.)


Singly linked list class I have created this program about using a singly linked list class that implements several functions: Add, Display and Sort.

Could you suggest me how to make my code better and which functions to include in the future?

#include <iostream>
using namespace std;
// ----------------------------------------------------
class Node {
private:
    int value;
    Node *next;
public:
    Node(int v) { value = v; next = NULL; }
    void Add(int v);
    void Print();
    void SortValues();
    void SortNodes();
};
// ----------------------------------------------------
void Node::Add(int v)
{
    Node *newItem = new Node(v);
    Node *temp = this;
    while (temp->next != NULL)
        temp = temp->next;
    temp->next = newItem;
}
void Node::Print()
{
    Node *temp = this;
    while (temp != NULL)
    {
        cout << "  " << temp << " : " << temp->value << endl;
        temp = temp->next;
    }
}
void Node::SortValues()
{
    Node *swapCandidate = next;
    Node *current;
    Node *min;
    //    L* temp;
    // Look at every value
    while (swapCandidate != NULL)
    {
        current = swapCandidate;
        min = swapCandidate;
        // ------------------------------
        while (current != NULL)
        {
            if (current->value < min->value)
            {
                min = current;
            }
            current = current->next;
        }
            if(min != swapCandidate)
            {
                int tempV = min->value;
                min->value = swapCandidate->value;
                swapCandidate->value = tempV;
            }
         // ------------------------------
         swapCandidate = swapCandidate->next;
         }
     }
     // ----------------------------------------------------
void main()
{
// Sort - moving values
Node *myList = new Node(0);
myList->Add(15);
myList->Add(3);
myList->Add(20);
myList->Add(1);
myList->Print();
myList->SortValues();
cout << "------------------------------" << endl;
    myList->Print();
system("pause");
}

Don't use using namespace std;

It's bad practice that can cause a lot of problems that are easy to avoid, most people are used to the prefix, and three letters aren't that hard to type.

Don't use system("pause")

It's not portable and there are better ways to hold the console open.

Prefer nullptr to NULL

NULL is a macro that will be silently converted to 0 whenever possible. nullptr is a language extension and will only work contextually with pointers.

Use proper encapsulation

A linked list should be a class that acts as a container of Nodes. Your Node class should not know how to sort all Nodes. This is the job of the container. Furthermore Nodes are an implementation detail that should not be exposed to the user.

class LinkedList
{
public:
    // user functions should go here
private:
    struct Node
    {
        int value;
        Node* next{ nullptr };
    }
}

A few things to note.

I put the public section first. This way anyone reading the code
will know what functions and variables are available to them.
I used struct for the Node since it represents plain data. It is
also easier to use because struct is default public so the
LinkedList will be able to access it easily (But since it is
properly encapsulated inside the container nothing else can.)
I initialized new nodes to nullptr.

Printing and Sorting aren't typically done as member functions. They should be standalone functions that accept a container, or a range of a container to perform their tasks.

Use RAII whenever possible

You shouldn't be manually allocating memory. Especially since you aren't freeing it. Every instance of new should come with an instance of delete but you shouldn't be using them at all.

Prefer '\n' to std::endl

std::endl does more than just move to the next line. It is generally preferred not to use it casually.

Keep working on it. Try to implement the container with some of the member functions found at std::list. Browse The Core Guidelines from time to time. And bring us more code to review.


Git script that fast forward merges and deletes the old branch This script fast forward merges a branch and then deletes the old one.  I normally run it like this:

git m master feature

Here's a link to the gist.

#!/bin/bash -x
set -e

#by naming this git-m and putting it in your PATH, git will be able to run it when you type "git m ..."

if [ "$#" -ne 2 ]
then
  echo "Wrong number of arguments. Should be 2, was $#";
  exit 1;
fi

git checkout $1;
git merge --ff-only $2;
git branch -d $2;

Is there anything wrong with this?  My biggest fear is I could run this script with a typo and end up deleting a branch without merging first.

The order of the arguments is backwards, I think. Compare against git-rebase(1):

  Synopsis

  git rebase [<upstream>] [<branch>]

  Description

  If <branch> is specified, git rebase will perform an automatic git checkout <branch> before doing anything else. Otherwise it remains on the current branch.

The git-rebase order is also better since the git checkout can be considered optional.  You should probably make yours optional as well.

The help message needs to be much more informative about what the command does and how to use it. Git is hard enough to use as it is.

Accidental branch deletion is unlikely to be tragic, since you used -d and not -D.


A port of a simple logger I'm looking at porting a simple logger I wrote a while back in Python to a language which would result in less runtime overhead from logging.

As an informal benchmark, I try to log a message, including the current time, 10,000,000 times to a given file, and to record the time.

The original Python version, under CPython 3.6.0 clocks in at about 2m06s on an Intel Core i7-3615QM CPU @ 2.30GHz, with logging to a file as well as STDOUT, but with STDOUT redirected to /dev/null (constant refreshing of my terminal emulator window with new output causes noticeable overhead).

Run under PyPy 2.7.12, that time is nearly halved to 1m10s.

A trivial C port does the same thing in about half a second with -O3 and a near-identical algorithm (which IMO is a pretty spectacular improvement, and realistically, not the type of performance I can aim for in most other languages AFAIK)

What I'm concerned about is a Haskell port I've been trying to get to run faster than Python:

module Main where

import Control.Monad.Writer
import Data.List hiding (repeat)
import Data.Time (getCurrentTime)
import Data.Time.Format (formatTime)
import System.IO hiding (putStrLn)
import System.Locale (defaultTimeLocale)

data Color = GREEN | BLUE | YELLOW | RED | PURPLE | RESET
instance Show Color where
    show GREEN  = "\x1b[32m"
    show BLUE   = "\x1b[34m"
    show YELLOW = "\x1b[33m"
    show RED    = "\x1b[31m"
    show PURPLE = "\x1b[35m"
    show RESET  = "\x1b[0m"

data MessageLevel = OK | INFO | WARN | FAIL | CRIT
instance Show MessageLevel where
    show OK     = "  OK  "
    show INFO   = " INFO "
    show WARN   = " WARN "
    show FAIL   = " FAIL "
    show CRIT   = " CRIT "

splotch :: MessageLevel -> String
splotch OK   = (show GREEN)  ++ "  ⬢  " ++ (show RESET)
splotch INFO = (show BLUE)   ++ "  ⬢  " ++ (show RESET)
splotch WARN = (show YELLOW) ++ "  ⬢  " ++ (show RESET)
splotch FAIL = (show RED)    ++ "  ⬢  " ++ (show RESET)
splotch CRIT = (show PURPLE) ++ "  ⬢  " ++ (show RESET)

buildMessage :: MessageLevel -> String -> IO String
buildMessage lvl msg = do
    now <- getCurrentTime
    return $ (show now) ++ " || [" ++ (show lvl) ++ "]" ++ (splotch lvl) ++ msg

slog :: MessageLevel -> String -> Handle -> IO ()
slog lvl msg f = (buildMessage lvl msg) >>= (hPutStrLn f)

bench1 = do
    f <- openFile "log.txt" AppendMode
    replicateM_ 10000000 $ slog OK "Hello, world!" f
    hClose f

main = bench1

Long story, short, this generates output which includes:

Picosecond-precise time (which isn't a requirement; second-precise is enough);
The log level;
A colored icon making it easier to recognize the log level at a glance; and
The log message.

After implementing some revision suggestions from the comments, this code can match Python's performance, but doesn't surpass it.

My primary concerns with my code are:

That picosecond-precise time is reducing how often memoization is performed

I've tried using second-resolution time, which would make this a true port of the basic Python functionality, but Haskell's datetime facilities seem fairly contrived to me

That I'm using a non-optimal way of sharing the file Handle between functions
That the benchmarking function itself could be inducing overhead
That I'm missing some places where pure and impure computations can be separated

I'm trying to avoid using the Writer monad because in the use cases for this logger, the output of computations inducing a logger call wouldn't need to be passed down the pipeline.

What changes can I introduce into this code to improve its performance in the benchmark I described?

So, you were right about the "picosecond precision": the overly precise time is causing problems (GHC doesn't memoize but you can read more about what it does do here).

Basically, the solution is to use a library. I used the fast-logger library.

module Lib
    ( main
    , toBench
    , MessageLevel(..)
    ) where
import Control.Monad
import Data.Monoid
import System.Log.FastLogger.Date
import System.Log.FastLogger

main = toBench 10000000 OK "Hello, World!"

data MessageLevel = OK | INFO | WARN | FAIL | CRIT

instance Show MessageLevel where
    show OK     = "  OK  "
    show INFO   = " INFO "
    show WARN   = " WARN "
    show FAIL   = " FAIL "
    show CRIT   = " CRIT "

toBench :: Int -> MessageLevel -> String -> IO ()
toBench n lvl msg = do
    logFunction <- fst <$> fastLogger
    let toString msg t = toLogStr t <> toLogStr (mconcat msg)
    replicateM_ n $ (logFunction . toString) ([" || [", (show lvl), "] ", msg, "\n"])

fastLogger :: IO (TimedFastLogger, IO ())
fastLogger = do
    timeCache <- newTimeCache "%Y-%m-%d %H:%M:%S"
    let logType = LogFileNoRotate "test.log" 4096
    newTimedFastLogger timeCache logType

You can view the full code    here.

I did modify the code, but for good reason: loggers writing to file shouldn't include color. So I think this is a more apt benchmark.

With all that said and done, the Haskell version took around 3.6s on my computer and the python version took 2m35s to accomplish the same thing. This is pure Haskell code - no C whatsoever. I did use one of the suggestions from hlint - use the tools available to you!

As a parting shot: get comfortable using other people's code when writing Haskell. Your code is pretty good - you separate pure and impure code nicely, but on this front it is lacking. One of functional programming's strengths is that it enables easy code reuse. I suggest you take advantage of this - Haskell is not easy.


A function to find temperatures closest to zero I wrote this function that, given an array, calculates the closest number to zero. The conditions are the following:

if the array is empty, return 0
if the array has two values share the "closeness" to zero, return the positive (for example if -1 and 1)

Here is my code and link to it.

// Write a function that takes an array an finds the closes number to 0.
// Disregard all 0s
// If the array is empty return 0

const tempArray = [-1, 0, 0, 5, -5, 6, -3, 2, 10, 13, 8, 70, -36, 36];

function calculateClosestTo0 (tempArray) {
  let minusTemps = [];
  let plusTemps = [];

  if (!tempArray.length) {
    return 0;
  }

  for (let i = 0; i < tempArray.length - 1; i++) {
    if (tempArray[i] < 0) {
      minusTemps.push(tempArray[i]);
    } else if (tempArray[i] > 0) {
      plusTemps.push(tempArray[i]);
    }
  }

  minusTemps.sort((a, b) => b - a)
  plusTemps.sort((a, b) => a - b)

  if (plusTemps[0] === Math.abs(minusTemps[0])) {
    return(plusTemps[0])
  }

  if (plusTemps[0] < Math.abs(minusTemps[0])) {
    return(plusTemps[0])
  }

  if (plusTemps[0] > Math.abs(minusTemps[0])) {
    return(minusTemps[0])
  }
}

calculateClosestTo0(tempArray)

What I intended to do is separate the array into minus values and positive values. The minus values I ordered by descending (cause I want the first item to be the "biggest" in minus numbers perspective) and the positive in ascending. After that, I compare the first number.

What worries me most is the I try to remove the "0" like that:

for (let i = 0; i < tempArray.length - 1; i++) {
  if (tempArray[i] < 0) {
    minusTemps.push(tempArray[i]);
  } else if (tempArray[i] > 0) {
    plusTemps.push(tempArray[i]);
  }
}

My colleagues usually frowned upon having a condition that has "else if" and nothing else. Why, I don't know.

Another thing that worries me is that ending part, where I have 3 conditions to return the correct value. Is there a smarter way to do that?

And generally: do you think that there is a better approach to such a problem?

Bugs

The loop

for (let i = 0; i < tempArray.length - 1 ; i++) {

omits the last array element, it should be

for (let i = 0; i < tempArray.length ; i++) {

If all array elements have the same sign then
either plusTemps or minusTemps is an empty array, and accessing
its first element in

if (plusTemps[0] === Math.abs(minusTemps[0])) {

is undefined. Those cases must be treated separately in your approach.
If the array is not empty, but all array elements are zero then
both plusTemps are minusTemps are empty, again making the result
undefined. What would be the expected result for this case?
Returning zero looks like a sensible choice to me.

Simplifications

The first and second check in

if (plusTemps[0] === Math.abs(minusTemps[0])) {
    return(plusTemps[0])
}

if (plusTemps[0] < Math.abs(minusTemps[0])) {
    return(plusTemps[0])
}

if (plusTemps[0] > Math.abs(minusTemps[0])) {
    return(minusTemps[0])
}

can be combined to

if (plusTemps[0] <= Math.abs(minusTemps[0])) {
    return(plusTemps[0])
}

if (plusTemps[0] > Math.abs(minusTemps[0])) {
    return(minusTemps[0])
}

and the second if is better done with an else:

if (plusTemps[0] <= Math.abs(minusTemps[0])) {
    return(plusTemps[0])
} else {
    return(minusTemps[0])
}

This saves a comparison and makes it clear that a value is returned
in any case. And since we know that minusTemps[0] is negative we need
not call Math.abs:

if (plusTemps[0] <= -minusTemps[0]) {
    return(plusTemps[0])
} else {
    return(minusTemps[0])
}

A better approach

As already mentioned in the comments and in the other answers, the solution
can be determined with a single traversal of the array, without the need to
create additional arrays or any filtering or sorting.

Just keep track of the closest element found so far, and replace it if a
negative element with a closer distance is found, or a positive element
with the a closer or the same distance.

closest stays zero until the first non-zero array element is found.
If non non-zero element exists then zero is returned. This covers the case
of an empty array, making the check for the array length obsolete.

function calculateClosestTo0 (arr) {
    let closest = 0
    for (let i = 0; i < tempArray.length ; i++) {
        if (closest === 0) {
            closest = arr[i]
        } else if (arr[i] > 0 && arr[i] <= Math.abs(closest)) {
            closest = arr[i]
        } else if (arr[i] < 0 && -arr[i] < Math.abs(closest)) {
            closest = arr[i]
        }
    }
    return closest
}

Using Array.reduce this can be written more concisely as

function calculateClosestTo0 (arr) {
    return arr.reduce((acc, x) =>
        acc === 0 ? x :
        x > 0 && x <= Math.abs(acc) ? x :
        x < 0 && -x < Math.abs(acc) ? x : acc
    , 0)
}

Unit tests

The first versions of my suggested code had bugs as well: It “looked”
simple and correct, but returned a wrong result in special cases.
It is a good idea to collect those (and similar) cases, as well as
other special cases that you think of, into a list. Then each iteration
of the code can be tested against that list, to ensure that it does
not introduce new bugs.

Here is the list that I came up with, it should cover many of the
possible situations:

function calculateClosestTo0 (arr) {
    return arr.reduce((acc, x) =>
        acc === 0 ? x :
        x > 0 && x <= Math.abs(acc) ? x :
        x < 0 && -x < Math.abs(acc) ? x : acc
    , 0)
}

let tests = [
    // Single element:
    { array: [5], result: 5 },
    { array: [-5], result: -5},
    // Positive element is closer to zero:
    { array: [-3, 2], result: 2 },
    { array: [1, -2, 3, -4], result: 1 },
    // Negative element is closer to zero:
    { array: [-2, 3], result: -2 },
    { array: [-1, 2, -3, 4], result: -1 },
    // Two elements with same distance but opposite sign:
    { array: [1, -1], result: 1},
    { array: [-1, 1], result: 1},
    { array: [0, 1, -1], result: 1},
    { array: [1, 0, -1], result: 1},
    { array: [1, -1, 0], result: 1},
    { array: [0, -1, 1], result: 1},
    { array: [-1, 0, 1], result: 1},
    { array: [-1, 1, 0], result: 1},
    // Only elements of same sign:
    { array: [4, 2, 3], result: 2},
    { array: [-4, -2, -3], result: -2},
    // Empty array, or only zeros:
    { array: [], result: 0},
    { array: [0, 0, 0, 0], result: 0},
    // Closest element is at front, medium, or tail position:
    { array: [2, 0, -2, 0, -3], result: 2},
    { array: [-2, 0, 2, 0, -3], result: 2},
    { array: [-2, 0, -3, 0, 2], result: 2}
]

for (let i = 0; i < tests.length; i++) {
    console.log(calculateClosestTo0(tests[i].array) == tests[i].result)
}


Count characters of a text file This program counts the number of the occurence of each character that is used in a file.

I tried to use everything that Java offers to me properly. If you have tips which library functions would have been better, how to make the code more readable, than let me know. If you have a tip how to improve the performance I would be very thankful, also if the readability would decrease than.

import java.util.Map;
import java.util.HashMap;
import java.io.File;
import java.io.FileReader;
import java.io.BufferedReader;
import java.io.IOException;

public class Main {
    public static void main(String[] args) {
        if (args[0].contains("help")) {
            System.out.println("usage: java Main filename.txt");
        }

        Map<Character, Integer> map = new HashMap<>();
        char[] chars = fileToCharList(args[0]);
        for (int i = 0; i < chars.length; i++) {
            if (map.containsKey(chars[i])) {
                Integer number = map.get(chars[i]);
                map.put(chars[i], ++number);
            } else {
                map.put(chars[i], 1);
            }
        }

        for (Map.Entry<Character, Integer> entry : map.entrySet()) {
            System.out.println(entry.getKey() + ": " + entry.
              getValue());
        }
    }

    private static char[] fileToCharList(String fileName) {
        File file = new File(fileName);
        StringBuilder sb = new StringBuilder();
        try (BufferedReader br = new BufferedReader(new FileReader(
          file))) {
            String line;
            while ((line = br.readLine()) != null) {
                sb.append(line);
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
        return sb.toString().toCharArray();
    }
}

A couple of things I noticed:

Using a TreeMap, instead of converting a StringBuilder to a char[] and creating the map, is much more efficient.

You don't test for an empty argument list.

Telling the user they have to use .txt in the filename is inaccurate as your code doesn't test for that.

When you're printing multiple values on one line, it is more efficient and easier to maintain to use printf, rather than concatenating strings;

Something like this will work:

public static void main(String[] args) throws IOException {
    if (args.length < 1 || args[0].equals("help")) {
        System.out.println("usage: java Main filename");
        return;
    }
    fileToChars(args[0])
            .forEach((k, v) -> System.out.printf("%1$s: %2$d\n", k, v));
}

private static TreeMap<Character, Integer> fileToChars(String fileName) throws IOException {
    final char EOF = (char) -1;
    File file = new File(fileName);
    TreeMap<Character, Integer> builder = new TreeMap<>();
    try (FileReader reader = new FileReader(file)) {
        Character input;
        while ((input = (char) reader.read()) != EOF) {
            Integer count = builder.getOrDefault(input, 0);
            builder.put(input, ++count);
        }
    } catch (IOException e) {
        throw new IOException("Problem reading file");
    }
    return builder;
}

This gives a sorted output of the mapped Characters.  If an 'as is' output is wanted, change the TreeMap to a HashMap.


AngularJS directive that manually transcludes content and has isolated scope I would like to hear the opinion of more experienced AngularJS developers on whether the following directive is best practice...

I was trying to make to make a directive that will include HTML that:

Can include compiled elements (other directives, like ng-src in the
example below)
Interpolated values ({{ value }})
Can include values/functions from the parent scope without modifying it (i.e. the
directive has isolated scope)

Example usage:

<tag>

{{ url }}

</tag>

Controller and directive implementation:

var testapp = angular.module('testapp', []);

var Ctrl = function($scope) {
    $scope.url = "http://google.com";
    $scope.image = "https://www.google.gr/images/srpr/logo4w.png";
};

testapp.directive('tag', function() {
  return {
    restrict: 'E',
    transclude: true,
    scope: { },
    compile: function compile(tElement, tAttrs, transclude) {
        return function(scope) {
            transclude(scope.$parent, function(clone) {
                tElement.append(clone);
            });
        }
    }
  }
});

Fiddle: http://jsfiddle.net/x5WcB/3/

I am a bit confused, your directive basically would do what Angular already does?

When I comment out the entire testapp.directive('tag', function() { block, I see no change in the functionality of this code in fiddle.. I think you need a more extensive example for us to provide a meaningful review.

Other than that:

Your code looks fine to me
JsHint.com cannot find anything wrong besides some missing semicolons
You have 0 comments in your code


Securely deleting files, while attempting to protect the user against obviously dangerous inputs As far as I can tell, no gem currently exists to wrap shred, the *nix utility to securely delete files.  I created a small class to do this, the key methods are:

def self.run cmd
  stdout,stderr,status = Open3.capture3 cmd
  raise "Failed executing #{cmd}: stdout: #{stdout}, stderr: #{stderr}, status #{status}" unless status.nil? || status == 0
  stdout
end

def self.validate_path path, required_regex=nil
  raise if required_regex && !required_regex.is_a?(Regexp)
  File.expand_path(path).tap do |path|
    raise if path == '/'
    raise if required_regex && !path.match(required_regex)
  end
end

def self.dir! path, opts={}
  return unless Dir.exist?(path)
  validate_path path, opts[:path_must_match]

  stdout = run "find #{path} -type f -exec #{shred_cmd} '{}' ';'"
  FileUtils.rm_rf path
  stdout
end

find is used to execute shred, since it lacks a recursive option.  I initially tried execdir instead of exec for security, but travis and other build environments hate it.  Potential problem?
validate_path tries to keep the user from screwing themselves by expanding the path, better ways of doing this?

Some notes:

def self.run cmd: Omiting parentheses on signatures is less and less idiomatic as time goes by, people have realized they are harder to read. I'd put them.
Open3.capture3 cmd: It's also less idiomatic than it was years ago to omit parentheses on calls (except for very specific DSL code).
stdout,stderr,status: Always a space after a comma.
def self.validate_path path, required_regex=nil. Positional optional arguments tend to get messy when you need to add new ones. I'd use always an options = {} hash. Ruby 2.0 has keyword arguments so this problem is fortunately gone.
raise if required_regex && !required_regex.is_a?(Regexp). I wouldn't check the type of the variable, it's the caller's responsability to get it right.
File.expand_path(path).tap. This kind of tap usages are IMO cryptic and unnecessary.
Lots of raise ... if condition: Inline conditionals save some lines at the cost of making the branching logic harder to follow. I prefer writing full-fledged indentend conditionals (indentation is a powerful conveyor of information).
Open3.capture3 cmd. Running command from strings is dangerous (files with spaces anyone?), you should use some mechanism to escape them.
find #{path} -type f. It's ok to call external commands when the the language is unable to do the task, but here you can easily perform a recursive find with, well, module find.

I'd write something like this:

require 'find'
require 'open3'
require 'fileutils'
require 'shellwords'

class SafeRemover
  def self.run(command_array)
    command_string = Shellwords.join(command_array)
    stdout, stderr, status = Open3.capture3(command_string)
    if !status || status == 0
      stdout
    else
      fail("Failed executing #{cmd}: stdout: #{stdout}, stderr: #{stderr}, status #{status}")
    end
  end

  def self.validate_path(path, options = {})
    required_regexp = options[:required_regexp]
    path2 = File.expand_path(path)
    if !Dir.exist?(path2)
      fail("Directory does not exist: #{path2}")
    elsif path2 == "/"
      fail("You cannot delete /")
    elsif required_regexp && !path2.match(required_regexp)
      fail("Path and required_regexp do not match")
    end
  end

  def self.dir!(root_directory, options = {})
    validate_path(root_directory, :required_regexp => options[:path_must_match])
    paths = Find.find(root_directory).select { |p| FileTest.file?(p) }
    output = paths.each_slice(100).map { |paths| run(["shred", *paths]) }.join
    FileUtils.rm_rf(root_directory)
    output
  end
end


Uses of barriers for one main thread controlling n threads Referring to this post, my problem (summarized):

One main thread waits for connections, on accept() spawns a new thread for each connection. After we received n connections, the main thread dictates all threads what command to run, and must wait for all threads to finish, until issuing a new command later on.

I would love to know if this solves my problem efficiently and is correct.

static int mCommandToExecute = 0;
static int mNumberConnectedClients = 0;
static pthread_barrier_t mWaitForAllDone;
static pthread_barrier_t mWaitForCommand;

void* handleRequest(void* arg) {

    ...

    while (1) {
        // Wait for a command here
        pthread_barrier_wait(&mWaitForCommand);

        printf("\nSending to client: %s\n", commands[mCommandToExecute]);
        n = write(sockfd, commands[mCommandToExecute], length[mCommandToExecute]);
        ...

        // Wait for all threads to finish here
        pthread_barrier_wait(&mWaitForAllDone);
    }
}

void* startServer(void* arg) {
    pthread_barrier_init(&mWaitForCommand, NULL, maxConnections + 1);
    pthread_barrier_init(&mWaitForAllDone, NULL, maxConnections + 1);

    for (mNumberConnectedClients = 0; mNumberConnectedClients < maxConnections; ++mNumberConnectedClients) {
        clientSocket = accept(sockfd, (struct sockaddr *) &client_addr, &clientLength);
        pthread_create(&mThread_pool[mNumberConnectedClients], NULL, handleRequest, (void*) &clientSocket);
    }

    // Wait for all threads to have booted up and ready to execute a command
    mCommandToExecute = 0;
    for (int i = 0; i < 4; ++i) { // Run through 4 commands as an example
        pthread_barrier_wait(&mWaitForCommand);
        // At least one thread is still executing here
        pthread_barrier_wait(&mWaitForAllDone);
        // All commands are done here
        mCommandToExecute = i + 1; // No mutex is used here, so set variable before every threads reads it
    }

    // Wait for all threads to finish
    for (int i = 0; i < maxConnections; ++i) {
        pthread_join(mThread_pool[i], NULL);
    }
}

The threading mechanics mostly look correct.  Your main thread and your subthreads only share the mCommandToExecute and commands variables.  The subthreads only read them between mWaitForCommand and mWaitForAllDone, and the main thread reads and writes the mCommandToExecute between mWaitForAllDone and waitForCommand.  So no one is writing to this data while anyone else is reading it, and your data is thread-safe.

However your subthreads do not have any exit condition, so I expect the pthread_join will hang on the first one.  When you reach that point, all your subthreads will be waiting for the next command.  Joining the thread does not terminate it.  It simply allows you to synchronize their termination. You need to add some signal for the subthreads to terminate, and then you need to invoke that signal and then pthread_barrier_wait(mWaitForCommand) to allow each subthread to proceed to check the termination signal.

One simple way to do this, if it's practical, would be to make the termination step the final "command".  Rather than sending the termination command, the subthreads could recognize it and just return immediately.  Your main thread would need to avoid waiting on mWaitForAllDone for that last command, though.

I assume you will have some other code which handles lost connections, closing the accepted connections after the threads terminate, etc.  You do not seem to allow for stalled threads such as might occur in many network situations. If a client does stall, it will stall your entire system of threads.  I assume this is intentional or else you have timeouts and handlers for such occurrences in your real code.


Speech reorganization helper class I am working on a speech reorganization project. I have a util class that consists of three methods: check if a microphone exists, check microphone availability, and check text-to-speech availability. Can anyone review my code?

public class MediaUtil {

    //returns whether a microphone exists
    public boolean getMicrophoneExists(Context context) {
        PackageManager packageManager = context.getPackageManager();
        return packageManager.hasSystemFeature(PackageManager.FEATURE_MICROPHONE);
    }

    //returns whether the microphone is available
    public static boolean getMicrophoneAvailable(Context context) {
        MediaRecorder recorder = new MediaRecorder();
        recorder.setAudioSource(MediaRecorder.AudioSource.MIC);
        recorder.setOutputFormat(MediaRecorder.OutputFormat.DEFAULT);
        recorder.setAudioEncoder(MediaRecorder.AudioEncoder.DEFAULT);
        recorder.setOutputFile(new File(context.getCacheDir(), "MediaUtil#micAvailTestFile").getAbsolutePath());
        boolean available = true;
        try {
            recorder.prepare();
        }
        catch (IOException exception) {
            available = false;
        }
        recorder.release();
        return available;
    }

    //returns whether text to speech is available
    public static boolean getTTSAvailable(Context context) {
        PackageManager packageManager = context.getPackageManager();
        Intent speechIntent = new Intent(RecognizerIntent.ACTION_RECOGNIZE_SPEECH);
        List<ResolveInfo> speechActivities = packageManager.queryIntentActivities(speechIntent, 0);
        if (speechActivities.size() != 0) return true;
        return false;
    }

Returning boolean

A pattern

    if (condition) return true;
    return false

is to be avoided.

    return condition;

achieves the same in a much more transparent manner. In your case,

    return speechActivities.size() != 0;

Testing vs acquisition

Consider the situation: by the time you call getMicrophoneAvailable it might be available, but later on when your app actually needs it, it's been successfully claimed by some other app.

It means there is no point to test for availability. Acquire it as you need it:

    public static MediaRecorder getMicrophone(Context context) {
        ....
        try {
            recorder.prepare();
        } catch (IOException exception) {
            return null;
        }
        return recorder;
    }


Breadth First Search using adjacency list I want to improve this code using STL. Let me know if I should add any other function in this code.

#include <iostream>
#include <vector>
#include <queue>
#include <list>
#include <limits>

class Graph
{
  int vertex_count;
  enum Color {WHITE, GRAY, BLACK};

  enum { INFINITY = std::numeric_limits<int>::max() };

  struct Vertex
  {
     int id;
     int distance;
     Color color;

     Vertex(int _id) : id(_id),
                       color(Color::WHITE),
                       distance(INFINITY)
                       {}
  };

public:

  std::vector<Vertex> vertices;
  std::vector< std::list<int> > adjList;

  Graph(int);
  void addEdge(int, int);
  void breadthFirstSearch(int);
};

Graph::Graph(int v)
{
   vertex_count = v;
   adjList.resize(vertex_count);
   for (int i = 0; i < vertex_count; i++)
   {
       vertices.push_back( Vertex(i) );
   }
}

void Graph::addEdge(int src, int dest)
{
  //undirected graph
   adjList[src].push_back(dest);
   adjList[dest].push_back(src);
}

void Graph::breadthFirstSearch(int s)
{
   vertices[s].color = GRAY;
   vertices[s].distance = 0;
   std::queue<Vertex> q;
   q.push(vertices[s]);
   while (!q.empty())
   {
      auto u = q.front();
      q.pop();
      for (auto v = adjList[u.id].begin(); v != adjList[u.id].end(); v++)
      {
         if (vertices[*v].color == WHITE)
         {
            vertices[*v].color = GRAY;
            vertices[*v].distance = u.distance + 1;
            q.push(vertices[*v]);
         }
      }
      u.color = BLACK;
      std::cout << vertices[u.id].id << " at level " << vertices[u.id].distance <<'\n';
   }
}

int main()
{
   Graph grp(5);
   grp.addEdge(0, 1);
   grp.addEdge(0, 4);
   grp.addEdge(1, 3);
   grp.addEdge(1, 4);
   grp.addEdge(1, 2);
   grp.addEdge(2, 3);
   grp.addEdge(3, 4);
   grp.breadthFirstSearch(2);
}

Will be fixed soon

Advice 1

In BFS, you don't really need colors. (See alternative implementation.)

Advice 2 (Exposing internals)

public:

  std::vector<Vertex> vertices;
  std::vector< std::list<int> > adjList;

I suggest you put the two fields under private:.

Advice 3

Since an undirected graph is a special case of a directed graph (in which each edge \$\{u, v\}\$ can be simulated by two directed arcs \$(u, v), (v, u)\$), I suggest you implement it as a directed graph, but add a method that inserts an undirected edge by two directed arcs.

Advice 4

I would rip off the actual BFS from the Graph. This is, however, a matter of taste.

Advice 5

Graph::Graph(int v)
{
   vertex_count = v;
   adjList.resize(vertex_count);
   for (int i = 0; i < vertex_count; i++)
   {
       vertices.push_back( Vertex(i) );
   }
}

Why not name int v int vertex_count in the first place? Also, I would go for, for example, std::unordered_map<int, list<int>> since it is not restricted to non-negative integers.

Alternative implementation

This is by no means a best possible implementation, but it demonstrates the overall structure I had in mind:

#include <iostream>
#include <vector>
#include <unordered_map>
#include <unordered_set>
#include <queue>
#include <algorithm>
#include <iterator>

class Graph
{
public:
    void addEdge(int node1, int node2);
    void addArc(int tailNode, int headNode);
    std::unordered_set<int>& getChildNodesOf(int node);

private:
    std::unordered_map<int, std::unordered_set<int>> m_adjacency_list;
};

void Graph::addArc(int tail, int head)
{
    m_adjacency_list[tail].insert(head);
}

void Graph::addEdge(int tail, int head)
{
    // Simulate an undirected edge via two bidirectional arcs:
    addArc(tail, head);
    addArc(head, tail);
}

std::unordered_set<int>& Graph::getChildNodesOf(int node)
{
    return m_adjacency_list[node];
}

std::unordered_map<int, int*>
computeUnweightedShortestPathTree(Graph& graph, int sourceNode)
{
    std::unordered_map<int, int*> parentMap = { { sourceNode, nullptr }};
    std::queue<int> open;
    open.push(sourceNode);

    while (!open.empty())
    {
        int currentNode = open.front();
        open.pop();
        int* parentNode = new int{currentNode};

        for (int childNode : graph.getChildNodesOf(currentNode))
        {
            if (parentMap.find(childNode) == parentMap.end())
            {
                parentMap[childNode] = parentNode;
                open.push(childNode);
            }
        }
    }

    return parentMap;
}

std::vector<int> tracebackPath(int targetNode,
                               std::unordered_map<int, int*>& shortestPathTree)
{
    std::vector<int> path;
    int currentNode = targetNode;
    int* nextNode = shortestPathTree[currentNode];

    while (true) {
        path.push_back(currentNode);
        nextNode = shortestPathTree[currentNode];

        if (nextNode == nullptr)
        {
            break;
        }

        currentNode = *nextNode;
    }

    std::reverse(path.begin(), path.end());
    return path;
}

int main()
{
    Graph graph;
    graph.addEdge(0, 1);
    graph.addEdge(0, 4);
    graph.addEdge(1, 3);
    graph.addEdge(1, 4);
    graph.addEdge(1, 2);
    graph.addEdge(2, 3);
    graph.addEdge(3, 4);

    for (int sourceNode : { 0, 1, 2, 3, 4 })
    {
        std::unordered_map<int, int*> shortestPathTree =
            computeUnweightedShortestPathTree(graph, sourceNode);

        for (int targetNode : { 0, 1, 2, 3, 4 })
        {
            std::cout << "Shortest path from " << sourceNode << " to " << targetNode << ": ";
            std::vector<int> path = tracebackPath(targetNode, shortestPathTree);

            std::copy(path.cbegin(),
                      path.cend(),
                      std::ostream_iterator<int>(std::cout, " "));

            std::cout << "\n";

        }
    }
}


Testing async method call from constructor I have a project where I want to build a more sophisticated ToDo list - basically a personal project management system. I'm just starting out with the project, and I'd like some feedback on whether my test methods are OK, as I'm pretty new to TDD. So far, for the main page that lists the daily ToDo items, I've got the following ViewModel:

public class MainVM : ViewModelBase
{
    private IEnumerable<ToDoItem> _toDoItems;
    private IRepository<ToDoItem> _toDoItemRepo;
    private bool _dataIsLoaded;

    public bool DataIsLoaded
    {
        get { return _dataIsLoaded; }
        set { Set(ref _dataIsLoaded, value, true); }
    }

    public IEnumerable<ToDoItem> ToDoItems
    {
        get { return _toDoItems; }
        set { Set(ref _toDoItems, value); }
    }

    public MainVM(IRepository<ToDoItem> toDoItemRepo)
    {
        _toDoItemRepo= toDoItemRepo;
        LoadData().ContinueWith(t => FinishedLoadingData(t));
    }

    private void FinishedLoadingData(Task loadTask)
    {
        switch (loadTask.Status)
        {
            case TaskStatus.RanToCompletion:
                DataIsLoaded = true;
                break;
            default:
                DataIsLoaded = false;
                break;
        }
    }

    public async Task LoadData()
    {
        if (!DataIsLoaded)
            ToDoItems= await _toDoItemRepo.GetAsync();
    }
}

As you can see, I want to load my data when the ViewModel is created.

I'm using the Set method from the MVVMLight toolkit to set the DataIsLoaded property- it sets the field behind and raises the PropertyChanged event for me.

I have the following test set up to test for a successful load:

    public void DataIsLoaded_is_true_if_loading_task_ran_to_completion()
    {
        AutoResetEvent testTrigger = new AutoResetEvent(false);

        TaskCompletionSource<IEnumerable<ToDoItem>> taskCompletion = new TaskCompletionSource<IEnumerable<ToDoItem>>();
        taskCompletion.SetResult(null);

        Mock<IRepository<ToDoItem>> repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(s => s.GetAsync()).Returns(taskCompletion.Task);

        MainVM vm = new MainVM(repoMock.Object);
        vm.PropertyChanged += (s, e) =>
        {
            if (e.PropertyName == nameof(MainVM.DataIsLoaded))
                testTrigger.Set();
        };

        testTrigger.WaitOne(5000);

        Assert.IsTrue(vm.DataIsLoaded);
    }

Now, I can see that there is a potential race condition here. The LoadData method could complete before the event handler is attached. As an alternative, I could use the Set method that broadcasts a message to MVVMLight's message handler in my ViewModel, and change my test method to this:

    [Test]
    public void DataIsLoaded_is_true_if_loading_task_ran_to_completion()
    {
        AutoResetEvent testTrigger = new AutoResetEvent(false);

        TaskCompletionSource<IEnumerable<ToDoItem>> taskCompletion = new TaskCompletionSource<IEnumerable<ToDoItem>>();
        taskCompletion.SetResult(null);

        Mock<IRepository<ToDoItem>> repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(s => s.GetAsync()).Returns(taskCompletion.Task);

        Messenger.Default.Register<PropertyChangedMessage<bool>>(
           this,
           message =>
           {
               if (message.PropertyName == nameof(MainVM.DataIsLoaded))
               {
                   testTrigger.Set();
               }
           });
        MainVM vm = new MainVM(repoMock.Object);

        testTrigger.WaitOne(5000);

        Assert.IsTrue(vm.DataIsLoaded);
    }

Now I don't have a race condition, but I'm a bit annoyed that I'm broadcasting a message that I don't really need for my code to work. I could wrap the whole thing in an pre-processor directive, of course, but that's just clunky.

Is there a better way to go about the whole thing?

Trying to load the data asynchronously in the constructor is a bad idea and in my opinion goes against what the constructor is for.

Constructors cannot be async, and asynchronous initialization can be seen as an implementation detail.

I suggest separating the initialization and data loading out ...

public class MainVM : ViewModelBase {
    private readonly IRepository<ToDoItem> repository;
    private IEnumerable<ToDoItem> items;
    private bool _dataIsLoaded;
    private bool loading = false;

    public bool DataIsLoaded {
        get { return _dataIsLoaded; }
        set { Set(ref _dataIsLoaded, value, true); }
    }

    public IEnumerable<ToDoItem> ToDoItems {
        get { return items; }
        set { Set(ref items, value); }
    }

    public MainVM(IRepository<ToDoItem> repository) {
        this.repository = repository;
    }

    public async Task LoadData() {
        if (loading) return;

        if (!DataIsLoaded) {
            try {
                loading = true;
                ToDoItems = await repository.GetAsync();
                loading = false;
                DataIsLoaded = true; // task Ran To Completion
            } catch (Exception ex) {
                loading = false;
                //TODO: Log error
                DataIsLoaded = false; // task did not complete
            }
        }
    }
}

... and have whatever is binding/using to the view model (ie the view, another model, a test, etc) load the data after initialization.

This allows the testing to be much simpler and to the point

For example

[TestClass]
public class MainVmTests {
    [TestMethod]
    public async Task DataIsLoaded_is_true_if_loading_task_ran_to_completion() {
        //Arrange
        var repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(_ => _.GetAsync()).Returns(Task.FromResult(Enumerable.Empty<ToDoItem>()));
        var subject = new MainVM(repoMock.Object);

        //Act
        await subject.LoadData();

        //Assert
        Assert.IsTrue(subject.DataIsLoaded);
        Assert.IsNotNull(subject.ToDoItems);
    }

    [TestMethod]
    public async Task DataIsLoaded_is_false_if_loading_task_failed() {
        //Arrange
        var repoMock = new Mock<IRepository<ToDoItem>>();
        repoMock.Setup(_ => _.GetAsync()).Throws(new AggregateException());
        var subject = new MainVM(repoMock.Object);

        //Act
        await subject.LoadData();

        //Assert
        Assert.IsFalse(subject.DataIsLoaded);
        Assert.IsNull(subject.ToDoItems);
    }
}


can we use of $this in closure for jquery plugin I tried creating a quick tooltip plugin using the jquery and jquery ui position. Is the way I have used the enclosure are right and is the use of position right since in ff it seem to have some memory problem..it remembers the previous position when I refresh the page after the first time. below is the code

This need the latest jquery & jquery ui

  $.fn.tooltip = function(options) {
            var defaults = {
                my : "left center",
                at : "right top",
                collision : "none",
                offset : "0 0"
            }

            var options = $.extend(defaults, options);

            return this.each(function() {
                var $this = $(this);
                var tip = $("" + $this.attr('tooltip') + "");
                tip.css({
                    width : options.width
                });
                $this.after(tip);
                tip.position({
                    my : options.my,
                    at : options.at,
                    of : $this,
                    collision : options.collision,
                    offset : options.offset
                });

                $this.add(tip).hover(function() {
                    var timeoutId = $this.data('timeoutId');
                    if(timeoutId) {
                        clearTimeout(timeoutId);
                    }
                    tip.fadeIn("slow");

                }, function() {
                    var timeoutId = setTimeout(function() {
                        tip.fadeOut("slow");
                    }, 650);
                    $this.data('timeoutId', timeoutId);
                });
            });
        };
        $(function() {
            $('#input1').tooltip();
            $('#input2').tooltip();
        });

css style

        .tooltip {
            display: none;
            position: absolute;
            background-color: #ffaa5e;/* #F5F5B5; */
            border: 1px solid #DECA7E;
            color: #303030;
            font-family: sans-serif;
            font-size: 12px;
            line-height: 18px;
            padding: 10px 13px;
            position: absolute;
            text-align: center;
            top: 0;
            left: 0;
            z-index: 3;
        }

html code to test this

http://jsfiddle.net/HE8QN/

thanks

Try this type of closure

    $this.add(tip).hover(function() {
        return function() {
            var timeoutId = $this.data('timeoutId');
            if(timeoutId) {
                clearTimeout(timeoutId);
            }
            tip.fadeIn("slow");
        };

    }(), function() {
        return function() {
            var timeoutId = setTimeout(function() {
                tip.fadeOut("slow");
            }, 650);
            $this.data('timeoutId', timeoutId);
        };
    }());

Just dont forget to tell if it solves the problem. Good luck.


Simple prime factorization program in Haskell I'm finally learning Haskell this summer and I have a few questions about conventions. I made a simple prime factorization program which is surprisingly fast, much faster than the ones I made in C++ (though to be fair I made those non-recursively).

It consists of three functions: one checks if a num is prime, one finds the prime factors, and one checks if a list's length is less than a number. The last one is because I was worried that take and length would be too slow and the Haskell wiki agreed. I just copied and pasted their at least function and reversed it.

import Data.List
import Data.Ord

isPrime :: Integer -> Bool
primes :: Integer ->[Integer]

isLengthLessThan :: Int -> [a] -> Bool
isLengthLessThan 0 _      = False
isLengthLessThan _ []     = True
isLengthLessThan n (_:ys) = isLengthLessThan (n-1) ys

isPrime x
    | x == 2       = True
    | mod x 2 == 0 = False
    | otherwise    = isLengthLessThan 2 [y | y <- [1,3.. roof], mod x y == 0]
    where roof = 1 + truncate (sqrt $fromIntegral x)

primes x
    | isPrime x = [x]
    | otherwise = factor : primes(div x factor)
    where factor = head $filter checkdiv [2..]
          checkdiv n = x `mod` n == 0

Finally the questions:

Should isLengthLessThan be in a where under isPrime?
Is isLengthLessThan's name too verbose?
What should I name my "primes" function?
Are the function type specifications called that or declarations?
Should I put the type specifications right before the function or should I put it at the start?
Do specifications help speed or do they only help us by giving us better errors?
Do parenthesis make compilation faster or do they only muddle code up?

First, a few stylistic points:

It's more common to write the div and mod functions in infix, as x `mod` y == 0, etc.
A parenthesized argument to a function is never written f(x).  It's always with a separating space like f (x), so you want primes (x `div` factor).
The infix operator $ is pretty much always both preceded and followed by a space.
Similarly, in the [x..y] syntax, the .. is normally not surrounded by any space, though when it is, it is both preceded and followed by space, so either [1,3..roof] or [1, 3 .. roof] would be more common than [1,3.. roof].

and some advice on functions:

even x is preferred to x `mod` 2 == 0
For positive values, 1 + truncate x can be replaced with ceiling x.  Technically, though, you don't need to check any numbers greater than the square root, so floor (sqrt $ fromIntegral x) works in your case.
Also for positive values, the function pair quot/rem is equivalent to div/mod and the former can compile to faster code on x86_64 (e.g., quot on Ints can be a single machine instruction), so many Haskellers will get in the habit of using them whenever the substitution works.  (Alternatively, this is a ridiculous micro-optimization here, so sticking with the div/mod pair is fine if it looks more natural.)

However, it might be even better to introduce a predicate:

divides :: (Integral a) => a -> a -> Bool
divides d n = n `rem` d == 0

so you can write y `divides` x in your list comprehension, and use the section (`divides` x) in your filter.

Finally, it's usual practice to provide type signatures that are more general than the ones you've provided.  Certainly, generalizing primes and isPrime to all integral types would be usual:

isPrime :: (Integral a) => a -> Bool
primes :: (Integral a) => a -> [a]

On the other hand, isLengthLessThan is probably fine the way it as, as it's usual to assume list lengths will fit in a 64-bit Int, for obvious reasons.

Now, with respect to your questions:

  Should isLengthLessThan be in a where under isPrime?

That's up to you.  A utility function that's used in only one function is often included in a where clause, though if the function seems "independently useful", it's sometimes kept at top-level.  In this small module, keeping it at top level seems reasonable.  In a bigger module, you might stick it in a where clause to reduce clutter.

Rearranging the functions can also help.  If you put primes at the top and then put utility functions after the functions that use them (e.g., primes, then isPrime, then isLengthLessThan), it can make it easier to follow the program's organization.

  Is isLengthLessThan's name too verbose?

I'd say so.  Maybe shorterThan?  Note that the character count is probably less of an issue than the word count -- shorterThan is simple to read and understand; isLengthLessThan belongs in a Java program as a method in the ListLengthCheckerFactory class.  But also, see below for how to eliminate this function entirely.

  What should I name my "primes" function?

I'd say factors -- I mean, it's obviously going to be the prime factorization, because what other factors function would make sense?

  Are the function type specifications called that or declarations?

According to the Haskell 2010 Report, they're called "type signatures", and the corresponding function definitions f x = ... are called "bindings".  Both type signatures and bindings are examples of "declarations", so isPrime :: Int -> Bool is a declaration, but so is isPrime x = ....  However, in informal speech, if you asked "should this binding have a type declaration?", everyone would know what you meant.

  Should I put the type specifications right before the function or should I put it at the start?

Almost universally in Haskell code, type signatures are placed immediately preceding the corresponding binding rather than collected together at the beginning of the module.  While collecting them together probably seems like a nice way of documenting the "interface" at the top of the module, it has the drawback that someone stumbling across the definition may assume there's no type signature (as signatures are often optional) and so never see it, and what's more the standard Haddock documentation tool is set up to have the signature and definition of each binding in the same place.

  Do specifications help speed or do they only help us by giving us better errors?

If the type signature matches what would be inferred by GHC if it was missing (which is often the most general possible type), then there is no effect on generated code.  Moreover, GHC is designed to generate efficient code even when highly generalized signatures are used, by specializing and inlining functions aggressively.  You could most likely drop your signatures, or use the signatures I suggested above with no effect on performance.

So, yes, type signatures are primarily useful for generating better error messages, occasionally for catching certain design errors, and -- when the code is finished -- for documenting the interface.

  Do parenthesis make compilation faster or do they only muddle code up?

Any effect of parentheses on compilation speed will be miniscule, and it's standard Haskell style to omit them except where they are necessary.

Anyway, incorporating the advice I gave above, my version of your program currently looks like the following.  Note that I turned on -Wall, which reminded me that your program doesn't actually need the modules it imports, and let me know that there was a type defaulting going on in the sqrt call that I thought should be made explicit.  I also ran hlint on it, which didn't have any hints for me.

{-# OPTIONS_GHC -Wall #-}

module MorePrimes where

factors :: (Integral a) => a -> [a]
factors x
    | isPrime x = [x]
    | otherwise = factor : factors (x `div` factor)
    where factor = head $ filter (`divides` x) [2..]

isPrime :: (Integral a) => a -> Bool
isPrime x
    | x == 2    = True
    | even x    = False
    | otherwise = shorterThan 2 [y | y <- [1,3..roof], y `divides` x]
    where roof = floor $ sqrt (fromIntegral x :: Double)

shorterThan :: Int -> [a] -> Bool
shorterThan 0 _      = False
shorterThan _ []     = True
shorterThan n (_:ys) = shorterThan (n-1) ys

divides :: (Integral a) => a -> a -> Bool
divides d n = n `rem` d == 0

Finally, some additional optimizations.

Your test in the third case of isPrime can be simplified.  Since 1 always divides x, you can replace the test with:

shorterThan 1 [y | y <- [3,5..roof], y `divides` x]

which is equivalent to:

null [y | y <- [3,5..roof], y `divides` x]

which would more usually be written:

not $ any (`divides` x) [1,3..roof]

allowing you to drop the definition of shorterThan entirely and use:

isPrime :: (Integral a) => a -> Bool
isPrime x
    | x == 2    = True
    | even x    = False
    | otherwise = not $ any (`divides` x) [3,5..roof]
    where roof = floor $ sqrt (fromIntegral x :: Double)

The other problem is that factors is doing extra work here -- it's using isPrime to check if x is prime using an algorithm that handles even numbers specially for efficiency, but then it turns around and does trial division by all numbers [2..] for composites.  Instead, you could replace isPrime with a function to find the first prime factor of x, like so:

firstPrime :: (Integral a) => a -> a
firstPrime x
    | even x    = 2
    | otherwise = head [y | y <- [3,5..roof]++[x], y `divides` x]
    where roof = floor $ sqrt (fromIntegral x :: Double)

This finds the first prime factor below roof, but throws in x itself as a last resort.  (There's an unnecessary check here that x divides itself, so we could improve this code to avoid that, but it might not be worth the trouble.)

Then, factors can grab this first prime factor, and see if it was x itself (in which case x is prime and we're done) or else look for more factors.  The complete code for this version looks like:

{-# OPTIONS_GHC -Wall #-}

module MorePrimes where

factors :: (Integral a) => a -> [a]
factors x = case firstPrime x of
    factor | factor == x -> [x]
           | otherwise   -> factor : factors (x `div` factor)

firstPrime :: (Integral a) => a -> a
firstPrime x
    | even x    = 2
    | otherwise = head [y | y <- [3,5..roof]++[x], y `divides` x]
    where roof = floor $ sqrt (fromIntegral x :: Double)

divides :: (Integral a) => a -> a -> Bool
divides d n = n `rem` d == 0

Obviously, further optimization is possible, as illustrated by @Gurkenglas's answer.


Checking whether a graph is bipartite using BFS I am solving a simple problem that checks whether a graph is two-colourable (bipartite graph). I am using BFS for this approach using C++ STL. I've replaced cin and cout with scanf and printf as suggested by fellow programmers but still the judge reports TLE.

#include<iostream>
#include<list>
#include<cstring>
#include<cstdio>
using namespace std;
int flag = 1;

class Graph
{
    int V;
    list<int> *adj;

    public:
        Graph(int V);
        void addEdge(int v, int w);
        void BFS(int s);
};

Graph::Graph(int V)
{
    this->V = V;
    adj = new list<int>[1000001];
}

void Graph::addEdge(int v, int w)
{
    adj[v].push_back(w);
    adj[w].push_back(v);
}

void Graph::BFS(int s)
{
    bool *visited = new bool[V];
    string *color = new string[1000001];
    for(int i=0;i<V;i++)
        visited[i] = false;
    list<int> queue;
    queue.push_back(s);
    visited[s] = true;
    color[s] = "red";
    list<int>::iterator i;
    flag = 1;
    while(!queue.empty())
    {
        s = queue.front();
        queue.pop_front();

        for(i = adj[s].begin(); i != adj[s].end(); i++)
        {
            if(!visited[*i])
            {
                visited[*i] = true;
                if(color[s] == "red")
                    color[*i] = "blue";
                if(color[s] == "blue")
                    color[*i] = "red";
                queue.push_back(*i);
            }
            else
            {
                if(color[s] == color[*i])
                    flag = 0;
            }
        }
    }
}

int main()
{
    int T, bugs, edges, from, to;
    scanf("%d", &T);
    int p = T;
    while(T--)
    {
        scanf("%d%d", &bugs, &edges);
        Graph g(bugs+1);
        for(int i=0; i<edges; i++)
        {
            scanf("%d%d", &from, &to);
            g.addEdge(from, to);
        }

        g.BFS(from);

        if(!flag)
            printf("Scenario #%d:\nSuspicious bugs found!\n", p-T);
        else
            printf("Scenario #%d:\nNo suspicious bugs found!\n", p-T);

    }

    return 0;
}

Stop using std::list, which is a doubly linked list and does not support fast random access. You should use std::vector instead, supporting O(1) random access.

Instead of

list<int> *adj;

You can have a vector of vectors, which will be not only faster, but also memory-leak free.


Solving the puzzle in javascript I've recently try to brush up my javascript skills, so I have a friend who gives me puzzles from time to time to solve. Yesterday I got this :

function testFun() {
  f = {};
  for( var i=0 ; i<3 : i++ ) {
    f[i] = function() { alert("sum='+i+f.length); }
  }
return f;
}

Expected Results:
testFun()[0]() should alert “sum=0”
testFun()[1]() should alert “sum=2”
testFun()[2]() should alert “sum=4”

I did this which does like requested above:

function testFun() {
    var i,
        f = {};
    for (i = 0; i < 3; i++) {
        f[i] = (function(number) {
            return function() {
                alert("sum=" + (number * 2));
            }
        }(i));
    }
    return f;
}

Could my answer be written better and what can be improved if so?

The style of you code is good. Here is not much of it but it looks like you understand the concept of closures well.

The logic of the original task is changed so even if you code returns the right values, they are calculated differently. The original task calculates it as i + f.length. It is questionable how to calculate length of an object. There are some ways but we can simply guess that the initialization of f is incorrect and f should be an array.

Another question is how to capture the current state of the array in the closure. Because it is object the closure will get the reference to it so it should be copied. The simplest way of copying an array it to call f.slice().

The last issue (reported by jshint) is that the function created in the loop is always the same. So you can move it outside of the loop and just call in the loop.

Update: Some notes on capturing objects in closures.

The closure captures variable by reference. If you have a loop with an index variable and capture this variable and then invoke the closure you will have not the value of the variable on the corresponding loop iteration. What you will get is the final value on the last loop iteration.

To capture the value the variable should be copied to some temporary context. In the languages with statement scope it is enough to declare local variable, but in JS with function scope you need to capture the variable by running the function. That is why in your code you have this function(number) { - it copies the index variable i to the function parameter number and when value of i is changed, value of number remains.

But when you capturing objects the situation is different, because objects are passed into function scope by reference too. So if the state of the object is changed, it will be changed in both parent and function scope. So in your example later, while invoking, the function will add the final length of the array to the variable, not the length of the array from the corresponding loop iteration.

So the length somehow should be preserved in the closure. It can be passed as number but it will be too boring - you already know how to deal with them. So it is better to think how to pass the copy of the array itself.

    f[i] = (function(number, array) {
        return function() {
            alert("sum=" + number + array.length);
        }
    }(i, f.slice()));


Extremely long if-or-or statement just feel wrong I'm sure this is wrong, but I can't think of a cleaner way to do this. I'm looping through a set of column to identify which column identifies the company name, the test is true if one of a large number of tests are true.

if (heading.ToLower().Contains("organisation") ||
            heading.ToLower().Contains("business") ||
            (heading.ToLower().StartsWith("address.") && heading.ToLower().EndsWith(".surname")) || // A and B
            heading.ToLower().Contains("name") && !(nextHeading.ToLower().Contains("organisation") || // A and not any of B
                                                    nextHeading.ToLower().Contains("company") ||
                                                    nextHeading.ToLower().Contains("firm")) ||
 .......This goes on FAR to long (e.g. A == B, but (C != D and E != F)
 {
      test = true;
 }

The way I've currently coded it smells wrong (code smell is a bad sign), but using if-elseif-elseif.... feels just as wrong.

Is there a pattern that I'm missing that can simplify this?

Thanks,
Kevin

Sure. Put this into a function, eg. "isCompanyName". This function can be easily commented and written in separate lines.

 if (heading.ToLower().Contains("organisation")
     return true;

 if (heading.ToLower().StartsWith("address.") &&
   heading.ToLower().EndsWith(".surname")))
     return true;

And so on. This allows to comment the separate cases and maybe explain what they are doing. It's also easy to modify. You can also put things like heading.ToLower() in a variable to save space.


Code to replace list of integers with nearest bigger element The original question was in the context of an array, but I just used a list without changing the length for easier debugging purposes of toString().

  Given an unsorted list of integers, design a \$O(n)\$ algorithm to transform the list such that the integers are replaced by the next bigger integer on their right. If there is no bigger integer on its right, the integer remains the same.

  For example \$[2,1,4,5,3,6,7,9,4,8]\$ becomes \$[4,4,5,6,6,7,9,9,8,8]\$.

I have come out with an algorithm but I'm not really confident whether it is considered \$O(n)\$ and where I can improve on if it really is \$O(n)\$. Can anyone provide me with some feedback?

import java.util.*;

class Main {
    public static void main(String [] args) {
        ArrayList<Integer> lst = new ArrayList<>(List.of(2,1,4,5,3,6,7,9,4,8));
        replace2(lst);
        System.out.println(lst);
    }

    //O(n^2) algorithm
    public static void replace(int [] arr) {
        for (int i=0;i< arr.length;i++) {
            for (int j=i;j<arr.length;j++) {
                if (arr[i]<arr[j]) {
                    arr[i]= arr[j];
                    break;
                }
            }
        }
    }

    //O(n) algorithm
    public static void replace2(ArrayList<Integer> lst) {
        int index = 0;
        int seen =0;
        for (int i=0;i< lst.size()-1;i++) {
            boolean isBroken = false;
            if (lst.get(i) >= lst.get(i+1)) {//if the adjacent element is larger, find for next biggest
                for (int j=i;j < lst.size(); j++) {
                    if (lst.get(j) > lst.get(i)) {
                        index = j;
                        seen = lst.get(j);
                        isBroken = true;
                        break;
                    }
                }
                if (isBroken) {//if we have found the next biggest
                    for (int k=index-1;k>i-1;k--) {
                        if (lst.get(k) < seen) {
                            if (lst.get(k) < lst.get(i)) {
                                lst.set(k,seen);
                            } else {
                                int temp = lst.get(k);
                                lst.set(k,seen);
                                seen = temp;
                            }
                        }
                    }
                }

            } else {//the adjacent element is already bigger
                lst.set(i,lst.get(i+1));
            }
        }
    }
}

As I already mentioned in a comment there's a bug in your replace2:
 input list 3,1,2,4 should result in 4,2,4,4 instead of all 4's since the 2 is the first bigger number on the right of number 1

Going for a List just for debugging seems odd to me. If you want to print a list to console you can use Arrays.toString(myArray) which looks exactly the same as myList.toString(). If you use a decent IDE you should be able to see the values clearly as well at a breakpoint. (I use IntelliJ and I know there are other IDE's that do this as well). In this case I would stick to using an array.

Since I currently only have java 8 installed for work I had a little trouble porting your program to java 8 to test it. The List.of() isn't available yet and any workaround to quickly initialise an explicit ArrayList gave me trouble. I instead changed all your ArrayList types to the more general List and it works. There's no reason to limit your method to ArrayList only.
Generally it's better to code using the interface instead of an explicit implementation. If at some point you would want to use a LinkedList instead of an ArrayList for example you can't use your current method.

Idea for a correct solution:
Loop through the list from right to left. Store the relevant larger numbers in a stack. At each step of the loop update the stack to only contain relevant larger numbers and replace the current element with the first (= smallest) larger element.

More concretely (try yourself first, use this if you're still stuck):

public static void replaceStack(int[] input) {
    if (input.length < 2) {
        return; //empty or 1 element list are trivial
    }
    Deque<Integer> largerKnown = new LinkedList<>(); //Java Stack implementation
    largerKnown.add(input[input.length - 1]); //last element is never replaced
    for (int i = input.length - 2; i >= 0; i--) { //loop from second to last downwards
        while (first element of stack is smaller) {
            remove first element from stack
        }
        if (largerKnown.isEmpty()) {
            largerKnown.addFirst(input[i]);
        } else {
            update the input[i] and add the old value of input[i] to the front of the stack
        }
    }
}

You'll still have to actually implement parts of this algorithm yourself. It shouldn't be that hard.


Determinant of a binary matrix I'm looking for the best and fastest way to do a determinant, for determinants until 15x15.

int Determinant(const vector<vector<int> > &a,int n)
{
   int i,j,j1,j2;
   int det = 0;
   vector<vector<int> > m(n, vector<int>(n));

    if (n == 1) { /* Shouldn't get used */
      det = a[0][0];
   } else if (n == 2) {
      det = a[0][0] * a[1][1] - a[1][0] * a[0][1];
   } else {
      det = 0;
      for (j1=0;j1<n;j1++)
      {
         for (i=1;i<n;i++)
         {
            j2 = 0;
            for (j=0;j<n;j++)
            {
               if (j == j1)
                  continue;
               m[i-1][j2] = a[i][j];
               j2++;
            }
         }
         det += pot(-1,1+j1+1) * a[0][j1] * Determinant(m,n-1);
      }
   }
   det = abs(det)%2;
   return det;
}

Unless this is a homework assignment, you should probably not be writing your own linear algebra routines, but rely on libraries instead. Not only is it very hard to write correct and efficient general-purpose algorithms, for numerical algorithms there is an added requirement of numerical stability. For those reasons it is best to use established libraries such as Eigen.

A determinant can be done using the example in the documentation

#include <iostream>
#include <Eigen/Dense>
using namespace std;
using namespace Eigen;
int main()
{
    Matrix3f A;
    A << 1, 2, 1,
         2, 1, 0,
        -1, 1, 2;
    cout << "Here is the matrix A:\n" << A << endl;
    cout << "The determinant of A is " << A.determinant() << endl;
}

Output

Here is the matrix A:
1 2 1
2 1 0
-1 1 2
The determinant of A is -3

Granted, for binary matrices the numerical stability argument that holds for floating point matrices is not really relevant, but the correctness and efficiency arguments remain important enough to warrant library use over hand-written implementations.

UPDATE: I just noticed that you got the same suggestion to start exploring Eigen in your other CodeReview question. I really encourage you to do so because it lets you focus on your application rather than on low-level implementation details that have been solved by others already. Unless of course, learning such details is what you are interested in.


Simple c# for-loop program for calculating theoretical earnings from investing (beginner) I am new to programming and "programming thinking" as well. I have created a simple program that calculates future earnings from investing. It seems to be working correctly, but I want to be sure and ask some smarter guy out here, whether the program is really working as it should or whether it could be written in a better and cleaner form. Input in first month is 15000, Other months: 1000, End of time period, when I want to see the earnings: 72 months, growth/month: 0.75 %; Each new input grows every month by its 0.75 %

static void Main(string[] args)
        {
            double inpInt, inpMonth, inpMonthTrue, growMonth, monthCount, sum, inpTotal, earnTotal;
            inpInt = 15000;
            inpMonth = 0;
            inpMonthTrue = 1000;
            growMonth = 0.75;
            monthCount = 72;
            inpTotal = inpInt + inpMonthTrue * (monthCount - 1);
            sum = inpInt + inpMonth;
            inpInt += inpInt / 100 * 0.75;
            monthCount--;
            Console.WriteLine("Value in months:" );
            Console.WriteLine(inpInt);
            for (int i = 0; i < monthCount; i++)
            {
                inpMonth += inpMonthTrue;
                inpInt += inpInt / 100 * growMonth;
                inpMonth += inpMonth / 100 * growMonth;
                sum = inpInt + inpMonth;
                Console.WriteLine(sum);
            }
            earnTotal = sum - inpTotal;
            Console.WriteLine(String.Format("Total input: {0}", inpTotal));
            Console.WriteLine(String.Format("Total value: {0}", sum));
            Console.WriteLine(String.Format("Total earnings: {0}", earnTotal));
            Console.ReadLine();
        }

Thanks

Here are some points that I think can make your code look good -

You can improve the readbility like you can group every set of operations with one line space. Like

The local variables can be renamed better as they're not clear to me when I look at the code directly without reading the problem statement. For example inpMonth and inpMonthTrue are confusing.
You can include code comments on the logic that you're performing. For example in the below snippet, it is unclear what exactly is happening.

The main() method can be broken into multiple meaningful methods to improve readability. Assuming that you'll be taking the input from the console

This was just a gist of what I'm trying to convey. It needn't be the exact names, you can create any number of methods for grouping the related logic and code. Breaking a big tall method into multiple meaningful chunks gives more clarity.
Plus, it is always helpful to detect any bugs that arise in the software, so you know where exactly to look into if the Interest is coming wrong for instance.