Java cycle detection using DFS in an undirected graph I am doing interview studies and can't find a simple DFS for a cycle-finding algorithm.  I want someone to tell me if my DFS algorithm works and how it can be improved.

Where can you get a typical DFS cycle finding algorthm for Java?  I assume you have to "mark" the nodes and edges in order to find a cycle?  Can you simplify / improve it? e.g. A-->B-->A , shouldn't be a cycle!

boolean isCycled(Node root){
    return isCycled(root, -1);
}

boolean isCycled(Node n, int edgeNum){
    //base case
    if(n==null) return false;

    //base case: if an unexplored edge that lead to a node that is visited.
    if(edgeNum>=0){//make sure it is not the first node
        if(n.visted==true && edge[edgeNum]!=true ) {
            return true; //found cycle
        }
    }

    n.visited=true;
    edge[edgeNum]==true; // already explored edge wont' lead to cycle.

    List nodes = getAllNeigbors(n); //visited or unvisited

    boolean isCycle = false;

    for(Node node : nodes){
        isCycle = isCycle || isCycled(node, getEdgeNum(n,node));
    }

    return isCycle;
}

There are a couple of suggestions I have here.

The first is a major performance one... you have the code:

boolean isCycle = false;
for(Node node : nodes){
    isCycle = isCycle || isCycled(node, getEdgeNum(n,node));
}
return isCycle;

This should be 'short-circuited' for true conditions, and should simply be:

for(Node node : nodes){
    if (isCycled(node, getEdgeNum(n,node))) {
        return true;
    }
}
return false;

There is no need to continue to walk the entire tree and find every cycle after yu have found one already!

The second item is more minor, and just something I would 'try'....

Instead of using the edge management, and visited tate on the nodes, I would consider using an IdentityHashMap to track what has been seen or not. Treat it like a 'stack' using put() and remove() like push and pop. That way yon can do something like:

//base case: if an unexplored edge that lead to a node that is visited.
// if(edgeNum>=0){//make sure it is not the first node
    if(seenmap.contains(node)) {
        return true; //found cycle
    }
// }

for(Node node : nodes){
    seenmap.put(node, null);
    isCycle = isCycle || isCycled(node, getEdgeNum(n,node));
    seenmap.remove(node);
}

If your comment about the A-->B-->A not being a cycle means that if a node points back to where it came from , it's not a cycle, then I recommend passing in the 'source' when you walk the graph... for example:

boolean isCycled(Node n, Node from, int edgeNum){

and then, your logic will look like:

for(Node node : nodes){
    if (node == from) {
        // this is a point-back to where we just came from in the graph)
        return false;
    }
    seenmap.put(node, null);
    isCycle = isCycle || isCycled(node, n, getEdgeNum(n,node));
    seenmap.remove(node);
}


Merge together multiple dataframes I'm working on a script where I want to create one dataframe from a series of files with annual baseball data in them.

I'm relatively new to R and I feel like the way I wrote get_seasons_range() is probably more like writing C# in R, rather than doing it the idiomatic R way. Is there a cleaner way to do this?

read_season <- function(yearID) {
  season = read.csv(paste("../retrosheetData/gamelog/GL", yearID, ".TXT", sep=""))
  glheaders = read.csv("../retrosheetData/gamelog/game_log_header.csv")
  names(season) = names(glheaders)
  return(season)
}

read_season_range <- function(year_range) {
  seasons = read_season(year_range[1])

  for(y in year_range) {
    if(y == year_range[1])
      next

    s = read_season(y)
    seasons = rbind(seasons, s)
  }

  return(seasons)
}

sixties = read_season_range(1960:1969)

The for loop can be replaced with a lapply statement.
Using base R functions, your read_season_range example is equivalent to the following one-liner:

sixties <- do.call(rbind, lapply(1960:1969, read_season))

or to wrap it in a function:

read_season_range <- function(year_range) {
  do.call(rbind, lapply(year_range, read_season))
}


Get users from groups in active directory, disposing of everything I am trying to get active directory groups with their users, all while disposing of all IDisposable assets when they reach the end of their usefulness. The groups and users are passed into a method that extracts certain properties and adds them to ExpandoObjects. This requires a decent number of nested using blocks.

Since the method is private, and most of the variables only exist in the using blocks, I gave the variables very short names. Naming them properly as groupPrincipal or principalSearchResult would involve breaking even more lines to fit the long names.

private IEnumerable<ExpandoObject> GetOuGroupsUsersData(
    PrincipalContext pc)
{
    var data = new List<ExpandoObject>();
    using (var gp = new GroupPrincipal(pc))
    {
        using (var ps = new PrincipalSearcher(gp))
        {
            using (var psr = ps.FindAll())
            {
                foreach (var g in psr.GetGroupPrincipals())
                {
                    using (var gpsr = g.GetMembers())
                    {
                        data.AddRange(
                            gpsr.GetUserPrincipals().Select(
                                u => GenerateData(g, u))
                                .Cast<ExpandoObject>());
                    }
                }
            }
        }
    }
    return data;
}

Is there anything I can do to reduce the number of using blocks? Am I even disposing of everything properly?

You can remove braces between using statements, leaving the last one. That would help you to remove a few of lines, but still not so deeply.
Wrapping the IDisposable interface is a good choice to do as an alternative to a dozen of using statements.
I'd suggest you using more appropriate names, maybe by lowercasing just a character of your class/method when you instantiate/call them.

Add this class to your project:

    public class IDisposableList : List<IDisposable>, IDisposable
    {
        public void Dispose()
        {
            if (this.Count > 0)
            {
                List<Exception> exceptions = new List<Exception>();

                foreach (var disposableItem in this)
                {
                    try
                    {
                        disposableItem.Dispose();
                    }
                    catch (Exception ex)
                    {
                        exceptions.Add(ex);
                    }
                }

                base.Clear();

                if (exceptions.Count > 0)
                    throw new AggregateException(exceptions);
            }
        }

        public T Add<T>(Func<T> item) where T : IDisposable
        {
            var disposable = item();
            base.Add(disposable);

            return disposable;
        }
    }

usage:

            var data = new List<ExpandoObject>();

            using (var disposables = new IDisposableList())
            {

                var gp = disposables.Add(() => new GroupPrincipal(pc));

                var ps = disposables.Add(() => new PrincipalSearcher(gp));

                var psr = disposables.Add(() => ps.FindAll());

                foreach (var g in psr.GetGroupPrincipals())
                {

                    var gpsr = disposables.Add(() => g.GetMembers());

                    data.AddRange(
                            gpsr.GetUserPrincipals().Select(
                                u => GenerateData(g, u))
                                .Cast<ExpandoObject>());

                }

            }

this is just a more elegant way to do what you asked.

Remember that using a foreach is not as performant as a for loop.


Split groups into unique sets I have a data.frame of people broken into households where I have a set of mostly unique keys (for household, and then person within household), but sometimes it looks like 2 (or possibly more) households were given the same key.

small_example <- tibble::tribble(
  ~hh_id, ~per_id, ~ref,
  1,       1,       "a",
  1,       2,       "b",
  1,       3,       "c",
  2,       1,       "d",
  2,       1,       "e",
  2,       2,       "f",
  2,       2,       "g",
  2,       3,       "h",
  2,       4,       "i"
)

In this example, the first household is ok, but I want to split the second into 2 households randomly, but to preserve as much of the original structure as possible. In particular, I want persons "d" and "f" to stay together and "e"/"g" to stay together and then persons "h" and "i" to be added to one of these 2 households.

Here's my first attempt at this, but my code is pretty slow. I can tell I'm over-using the tidyverse, but am not sure what a better alternative is.

library(dplyr)
library(purrr)
assign_extra_id <- function(per_grp_id) {
  all_pers_df <- data_frame(orig_id = per_grp_id) %>%
    group_by(orig_id) %>%
    mutate(order_num = row_number(), total_num = n())

  max_dup_pers <- max(all_pers_df$total_num)

  if (max_dup_pers == 1) return(1)
  multi_pos_order <- accumulate(
    seq(max_dup_pers, 2, -1),
    ~sample(.x, .y - 1),
    .init = sample(seq_len(max_dup_pers), max_dup_pers)
  ) %>%
    set_names(seq(max_dup_pers, 1, -1)) %>%
    map_df(~data_frame(order_num = seq_along(.), new_id = .), .id = "total_num") %>%
    mutate(total_num = as.numeric(total_num))

  out <- left_join(all_pers_df, multi_pos_order, by = c("total_num", "order_num"))
  out$new_id
}

small_example %>%
  group_by(hh_id) %>%
  mutate(extra_id = assign_extra_id(per_id))

#> # A tibble: 9 x 4
#> # Groups:   hh_id [2]
#> hh_id per_id   ref extra_id
#> <dbl>  <dbl> <chr>    <dbl>
#> 1     1      1     a        1
#> 2     1      2     b        1
#> 3     1      3     c        1
#> 4     2      1     d        2
#> 5     2      1     e        1
#> 6     2      2     f        2
#> 7     2      2     g        1
#> 8     2      3     h        1
#> 9     2      4     i        1

And here's a timing:

bigger_example <- map_df(seq_len(100), ~mutate(small_example, hh_id = hh_id + (2 * .)))

microbenchmark::microbenchmark(
  my_attempt = bigger_example %>%
    group_by(hh_id) %>%
    mutate(extra_id = assign_extra_id(per_id)),
  times = 10
)

Unit: seconds
       expr      min       lq     mean   median       uq      max neval
 my_attempt 2.297449 2.305819 2.327998 2.312012 2.354128 2.381427    10

I want to randomly sample from the 4 sets of households I consider valid for what is currently hh_id = 2:

Set 1: hh1 = d, f, h, i;   hh2 = e, g
Set 2: hh1 = d, f;         hh2 = e, g, h, i
Set 3: hh1 = e, g;         hh2 = d, f, h, i
Set 4: hh1 = e, g, h, i;   hh2 = d, f

The logic for this when there are 3 households with the same hh_id gets even more complicated, because if there's only 1 person with a given person id, I want to sample from the households that were available when there were 2 (and so on). This is why there's the kind of hairy purrr::accumulate call.

Yes, I think it is better to use efficient base functions for this kind of task. I'm assuming your ids are sorted so that duplicates will always be next to each other, is that right? In that case, a first idea would be to use the duplicated function. Using your example, see that:

x <- c(1, 1, 2, 2, 3, 4)
duplicated(x)
# [1] FALSE  TRUE FALSE  TRUE FALSE FALSE
1 + duplicated(x)
# [1] 1 2 1 2 1 1

However, this will not work in your more general case where there may be more than 2 duplicated households:

x <- c(1, 1, 1, 2, 2, 2, 3, 3, 4, 5)
1 + duplicated(x)
# [1] 1 2 2 1 2 2 1 2 1 1

For the general case, I would use run length encoding functions:

rle(x)
# Run Length Encoding
#   lengths: int [1:5] 3 3 2 1 1
#   values : num [1:5] 1 2 3 4 5
sequence(rle(x)$lengths)
# [1] 1 2 3 1 2 3 1 2 1 1

So I would suggest you replace your assign_extra_id with the following:

assign_extra_id2 <- function(x) sequence(rle(x)$lengths)

Update

Taking into account the added details (in the comments and updated question), maybe this function will do?

assign_extra_id3 <- function(x) {
   resample <- function(x, ...) x[sample.int(length(x), ...)]
   z <- rle(rle(x)$lengths)
   s <- sample(z$values[1])
   i <- Map(function(l,v) rep(resample(s[1:v]),l), z$lengths, z$values)
   unlist(i, use.names = FALSE)
}

It's (still) a nice speedup:

# Unit: milliseconds
#        expr        min         lq       mean    median         uq       max neval
 # op_attempt 5013.47742 5051.44837 6198.35973 5535.9724 6951.75439 9843.3606    10
 # fd_attempt   54.48751   59.92706   73.31207   62.1226   82.82826  130.2046    10


Setting up a development environment - 2 This is the new code after following the advice from here:

Setting up a development environment

General feedback is requested.  Also, should I be using $() or () for my sub-shell.  What is the difference.

#
#
#
# Divider - configures bash, git, grunt, sublime and chrome
#
#
#

user1=foo

config_bash() {
    rm ~/.bash_profile
    ln -sf ~/root/config/bash/login.sh ~/.bash_profile
    source ~/.bash_profile
    echo "Bash configured."
}
config_git() {
    local a
    if [ $# -eq 0 ]
    then
        a="client"
    fi

    if [ $# -eq 1 ]
    then
        a="$1"
    fi

    if [ $# -gt 1 ]
    then
        a="$1"
        echo "Git-tooo many arguments"
    fi

    git config --global user.name "$a"
    git config --global user.email "$a@$a.com"

    git config --global push.default matching
    git config --global core.editor "subl"

    git remote add godaddy $user1@arcmarks.com:~/root.git
    git remote add heroku https://git.heroku.com/frozen-dusk-2587.git
    echo "Git configured."
}
config_grunt() {
    sudo npm install -g grunt-cli
    mkdir -p ~/root_install/grunt
    ln -sf ~/root/config/grunt/package.json ~/root_install/grunt/package.json
    ln -sf ~/root/config/grunt/Gruntfile.js ~/root_install/grunt/Gruntfile.js
    (cd ~/root_install/grunt; npm install)
    cd ~/root
    echo "Grunt configured."
}
config_sublime_2() {
    ln -sf /Applications/Sublime\ Text\ 2.app/Contents/SharedSupport/bin/subl /usr/local/bin/subl
    rm -rf ~/Library/Application\ Support/Sublime\ Text\ 2/Packages/User
    ln -sf ~/root/config/sublime ~/Library/Application\ Support/Sublime\ Text\ 2/Packages/User
    echo "Sublime configured."
}
reset_sublime_2() {
    rm -rf ~/Library/Application\ Support/Sublime\ Text\ 2
}
config_chrome(){
    chmod 0444 ~/Library/Application\ Support/Google/Chrome/Default/History
    echo "Chrome configured."
}
config_all() {
    config_bash
    config_git
    config_grunt
    config_sublime_2
    config_chrome
    list
}
list(){
    local bash=$(which bash) git=$(which git) grunt=$(which grunt) subl=$(which subl)
    local node=$(which node)  heroku=$(which heroku)
    echo "****"
    echo "Your bash executble is here: $bash."
    echo "Your git executble is here: $git."
    echo "Your grunt executble is here: $grunt."
    echo "Your sublime executble is here: $subl."
    echo "Your node executble is here: $node."
    echo "Your heroku executble is here: $heroku."
    echo "****"
}

These conditions are mutually exclusive, so they should be chained using elif:

if [ $# -eq 0 ]
then
    a="client"
fi

if [ $# -eq 1 ]
then
    a="$1"
fi

if [ $# -gt 1 ]
then
    a="$1"
    echo "Git-tooo many arguments"
fi

Like this:

if [ $# -eq 0 ]
then
    a="client"
elif [ $# -eq 1 ]
then
    a="$1"
else
    a="$1"
    echo "Git-tooo many arguments"
fi

This avoids unnecessary evaluations. That is, when $# is 0, only the first condition will be evaluated, the others won't be. In your original code, all conditions will be evaluated always.

As mentioned in the previous review, changing directory inside a script is not recommended. In config_grunt you have this as the last statement of the function:

cd ~/root

It seems pointless, and as it doesn't seem to have anything to do with configuring grunt, it probably shouldn't be in this function.


Finding duplicates in two sorted arrays This is an interview question from here. Specifically, the second asked for a function that took in two sorted arrays of integers with no duplicate values within a single array and which returned an array of the duplicates between the two arrays.

Here is my solution (with tests):

public class ArrayDuplicates {
     public static void main(String[] args) {
          int[] sorted1 = {1, 2, 3, 5, 7};
          int[] sorted2 = {2, 4, 5, 6};
          Integer[] duplicates = duplicates(sorted1, sorted2);
          for(Integer d: duplicates) {
            System.out.println(d.intValue());
           }
      }
     public static Integer[] duplicates(int[] sorted1, int[] sorted2) {
            List<Integer> duplicates = new ArrayList<Integer>();
            for(int count = 0; count < sorted1.length; count ++) {
                  for(int counter = 0; counter < sorted2.length; counter ++) {
                       if(sorted1[count] == sorted2[counter]) {
                            duplicates.add(sorted1[count]);
                       } else if(sorted1[count] < sorted2[counter]) {
                          break;
                       }
                   }
             }
    return duplicates.toArray(new Integer[duplicates.size()]);
    }
}

The code runs fine but I am trying to make it more efficient. I did a runtime analysis of the duplicates method and found the runtime to be in \$O(N^2)\$ (general nested for loop, etc). To me, there always seems to be some optimization to get the algorithm to \$O(n)\$. For this problem, the only optimization, I found was to break out of the while loop if the value in the second array is greater than the first (because you know it's sorted, so there's no way that value is present in the second array).

Is there another optimization that could get this code down to \$O(n)\$?

You are looking for values in a sorted array, first thing that comes to mind is binary search (improves to \$O(n \log n)\$).

However because the values you look for are also sorted then you know that the next value will be after the value you found in the last iteration. Keep the index where you broke out of the inner loop and start there during the next iteration.

int innerIndex = 0;
for(int count = 0; count < sorted1.length; count ++) {
      for(; innerIndex < sorted2.length; innerIndex++) {
           if(sorted1[count] == sorted2[innerIndex]) {
                duplicates.add(sorted1[count]);
           } else if(sorted1[count] < sorted2[innerIndex]) {
              break;
           }
       }
 }

Otherwise you can pretend to merge the arrays. It's essentially the same algorithm but makes it clearer what is happening:

int index1 = 0, index2
while(index1 < sorted1.length && index2 < sorted2.length){
    if(sorted1[index1] < sorted2[index2]){
        index1++;
    }else if(sorted1[index1] > sorted2[index2]){
        index2++;
    }else{
        duplicates.add(sorted1[index1]);
        index1++;
        index2++;
    }
}


Conway's Game of Life in J I have the following code which produces successive generations of Conway's Game of Life:

life =: 3 : '+./ (>(($ y) $ 1);y) *. > =&(+/ (>,{ ;~(1 0 _1))|. y) each 3 4'

Usage:

RR =: 5 7 $ 0 0 0 0 0 0 0 0 0 1 1 1 0 0 0 0 1 0 0 0 0 0 0 1 1 0 0 0 0 0 0 0 0 0 0

NB. First five gens of RR
life^:(i.5) RR

NB. in ASCII
' *' {~ life^:(i.5) RR

Explanation of code:

>,{ ;~(1 0 _1)

Produces all possible 1 rotations of a matrix y.

|. y

Applies 1 rotations to y

+/

Adds all 1 rotations leading to neighbor-count for each cell

=&

Creates a monadic function which takes a single number and compares it to the neighbor-counts

each 3 4

Applies the function to 3 and 4 to find cells with 3 or 4 surrounding cells including self alive.

>

Unboxes to apply bitwise and

(>(($ y) $ 1);y)

I want all cells that correspond to 3 (($ y) $ 1) and only the cells that are already currently living the correspond to 4 (y).

+./

Or the result together to produce the next generation.

I would like to know if there's a better way to do this and shorten code. A lot of the things I did seem very hacky.

I have written APL code on VM/CMS and in DEC's VAX-11 interpreter.

No, I don't see a better way that would shorten the code.


Find an unknown length using sohcahtoa (Trigonometry) Here is a calculator for an unknown side in a right-angle. Is there a more efficient way of doing this?

import math
def trigonometry(angle, side_length, known, unknown):
    o, h, a = 'opposite', 'hypotenuse', 'adjacent'
    sohcahtoa = {o: {h: math.sin, a: math.tan},
                 h: {o: math.sin, a: math.cos},
                 a: {o: math.tan, h: math.cos}}
    function = sohcahtoa[known][unknown]
    return side_length / function(math.radians(angle)) if known == 'opposite' or (known == a and unknown == h)\
        else function(math.radians(angle) * side_length)

    print(trigonometry(30, 1, 'opposite', 'hypotenuse'))

I particularly dislike this line as it is basically brute force and very long.

return side_length / function(math.radians(angle)) if known == 'opposite' or (known == a and unknown == h)\
        else function(math.radians(angle) * side_length)

Test Calls

Here are two test calls to make sure the code works.

Call: trigonometry(30, 1, 'opposite', 'hypotenuse')
Expected Output: 2

Call: trigonometry(30, 2, 'hypotenuse', 'opposite')
Expected Output: 1

You know the equations, but fail to return them in your sohcahtoa dictionary.
Take 'soh':

\$\sin(\theta) = \frac{o}{a}\$
\$o = a\sin(\theta)\$
\$a = \frac{o}{\sin(\theta)}\$

From this you know all the equations, and you know they follow the form op(side, trig_fn(angle)). And so you should expand your sohcahtoa dictionary to contain this too.
I'd recommend you make an Enum for opposite, adjacent and hypotenuse, so that you get an actual error, rather than hidden errors, if you mis-spell them. (Yes you should be careful of this.)
It doesn't make sense to create sohcahtoa every time you call the function.

And so I'd change to:

from enum import Enum
import math
import operator

class TrigSides(Enum):
    OPPOSITE = 'opposite'
    ADJACENT = 'adjacent'
    HYPOTENUSE = 'hypotenuse'

SOHCAHTOA = {
    TrigSides.OPPOSITE: {
        TrigSides.HYPOTENUSE: (math.sin, operator.truediv),
        TrigSides.ADJACENT: (math.tan, operator.truediv)
    },
    TrigSides.HYPOTENUSE: {
        TrigSides.OPPOSITE: (math.sin, operator.mul),
        TrigSides.ADJACENT: (math.cos, operator.mul)
    },
    TrigSides.ADJACENT: {
        TrigSides.HYPOTENUSE: (math.cos, operator.truediv),
        TrigSides.OPPOSITE: (math.tan, operator.mul)
    }
}

def trigonometry(angle, side, known, unknown):
    trig_fn, op = SOHCAHTOA[known][unknown]
    return op(side, trig_fn(math.radians(angle)))


Query messages and 'bold' part of a message. Show one query at a time but get the next query in advance for better user experience In this react component I'm trying to display one message on the screen at a time. This message has a 'bold' substring in it which should be accounted for. There's 2 endpoints here,

message to retrieve a message, and

bold which retrieves what is bold (with start and end keys to a substring) about the message passed in as a parameter

I tried to optimize for user experience by always having two messages available, so that when the user clicks 'next', there already is a message to display and no waiting time. These are currentMessage and nextMessage.
I used two useState hooks as I found looping inside of a useEffect hook while fetching to be somewhat complicated. Would still love to be able to do that.
Moreover, I have used two useEffects. The first is simply run once, to get the initial message, and the other is run when the page renders and then every time the user clicks 'next'. When a user clicks next, that's when the app swaps the current message for the one which was in the 'queue'
const MessageBox = () => {
  const [message, setCurrentMessage] = useState("");
  const [bold, setCurrentBold] = useState("");
  const [nextMessage, setNextMessage] = useState("");
  const [nextBold, setNextBold] = useState("");
  const [isGettingMessage, setIsGettingMessage] = useState(true);

  useEffect(() => {
    fetch(`https://myapi.com/message`)
      .then((res) => res.json())
      .then((res) => {
        if (res.data) {
          setCurrentMessage(res.data.message);
          getBold(res.data.message, setCurrentBold);
        } else {
          console.log("handle err");
        }
      });
  }, []);

  useEffect(() => {
    if (!isGettingMessage) return;
    fetch(`https://myapi.com/message`)
      .then((res) => res.json())
      .then((res) => {
        if (res.data) {
          setNextMessage(res.data.message);
          getBold(res.data.message, setNextBold);
        } else {
          console.log("handle err");
        }
      });
    setIsGettingMessage(false);
  }, [isGettingMessage]);

  const getBold = (message, boldSetter) => {
    if (!message) return;
    fetch(`https://myapi.com/bold`, {
      method: "POST",
      headers: { "Content-type": "application/json" },
      body: JSON.stringify({ content: message }),
    })
      .then((res) => res.json())
      .then((res) => {
        if (res?.data?.bold.length) {
          boldSetter(res.data.bold);
          return;
        } else if (res.error) {
          console.log("handle err");
        }
        boldSetter("");
      });
  };

  const formatMessageAndBold = () => {
    if (message && bold) {
      return boldMessage;
    } else if (message && !bold) {
      return {message};
    }
  };

  const boldMessage = useMemo(
    () => (

        {message.substring(0, bold.start)}
        {message.substring(bold.start, bold.end)}
        {message.substring(bold.end)}

    ),
    [message, bold.start, bold.end]
  );

  const messageAndBold = formatMessageAndBold();

  const onClick = () => {
    if (isGettingMessage) return;
    setIsGettingMessage(true);
    setCurrentMessage(nextMessage);
    setCurrentBold(nextBold);
  };

  return (

      {messageAndBold}
      <button isLoading={nextMessage === message || !message} onClick={onClick}>
        {message ? "Next Message" : "Loading..."}
      </button>

  );
};

export default MessageBox;
```

note: this took me longer then I expected and there may be some syntax errors, but contains the general idea
Once your component starts having complex logic, it's a good indication to start breaking it up.
Here I propose putting your logic in some different files:

constants.js
helpers.js
hooks.js
MessageBox.js

Notably, in hooks.js I would create a few custom hooks to handle your logic.

useQueryFullMessage - handles fetching a message + a bold and returning the result
useFullMessage - uses useQueryFullMessage twice to get two full messages and handles the logic on preloading the next full message.

This is a possible abstraction (among many) you can use.
I also don't believe that useMemo is required in this situation... unless you're going to be loading many messages. React by default is already pretty optmized.
//constants.js
const API_MESSAGE_URL = 'https://myapi.com/message';
const API_BOLD_URL = 'https://myapi.com/bold';

//helpers.js
import { API_MESSAGE_URL, API_BOLD_URL } from './constants';

const getMessage = async () => {
  const res = await fetch(API_MESSAGE_URL);
  const {data} = await res.json() ?? {};
  return data?.message;
}

const getBold = async (message) => {

  const res = await fetch(API_BOLD_URL, {
    method: "POST",
    headers: { "Content-type": "application/json" },
    body: JSON.stringify({ content: message }),
  });
  const {data} = await res.json() ?? {};
  return data?.bold;
}

export const getFullMessage = async () => {
  const message = await getMessage();
  if(!message) {
    // handle error
    return {};
  }
  const bold = await getBold(message);
  if(!bold) {
    // handle error
  }
  return {
    message,
    bold
  };
}

//hooks.js
import { getFullMessage } from './helpers';

const useQueryFullMessage = () => {
  const [fullMessaage, setFullMessage] = useState();
  const [loading, setLoading] = useState(true);
  const [preloadNextValue, togglePreloadNext] = useState(0);
  useEffect(async () => {
    if(!loading) {
      setLoading(true);
    }
    const fullMessage = await getFullMessage();
    // handle if bold not set
    setFullMessage(fullMessage);
    setLoading(false);
  }, [preloadNextValue]);

  return {
    fullMessage,
    loading,
    preloadNext: () => togglePreloadNext(preloadNextValue + 1),
    setFullMessage,
  }
}

export const useFullMessage = () => {
  const {
    fullMessage: currentFullMessage,
    loading: loadingCurrentFullMessage,
    setFullMessage: setCurrentFullMessage
  } = useQueryFullMessage();
  const {
    fullMessage: nextFullMessage,
    loading: loadingNextFullMessage,
    preloadNext
  } = useQueryFullMessage();

  const requestNextMessage = () => {
    if(loadingCurrentFullMessage || loadingNextFullMessage) {
      return;
    }
    setCurrentFullMessage(nextFullMessage);
    preloadNext();
  }

  return {
    currentFullMessage,
    loadingCurrentFullMessage,
    nextFullMessage,
    loadingNextFullMessage,
    requestNextMessage
  };
}

// MessageBox.js
import { useFullMesage } from './hooks';
const MessageBox = () => {
  const {
    currentFullMessage,
    loadingCurrentFullMessage,
    nextFullMessage,
    loadingNextFullMessage,
    requestNextMessage
  } = useFullMessage();

  const { message, bold } = fullMessage;
  // TODO: check if message and bold are set.

  const formatMessageAndBold = () => {
    if (message && bold) {
      return

        {message.substring(0, bold.start)}
        {message.substring(bold.start, bold.end)}
        {message.substring(bold.end)}

;
    } else if (message && !bold) {
      return {message};
    }
  };

  return (

      {loadingCurrentFullMessage ? "Loading..." : formatMessageAndBold()}
      <button isLoading={loadingNextFullMessage} onClick={requestNextMessage}>
        {!loadingNextFullMessage ? "Next Message" : "Loading..."}
      </button>

  );
}
```


Prime numbers in given range SPOJ problem PRIME1 (also found on other sites in identical form):

  Input

  The input begins with the number t of test cases in a single line
  (t<=10). In each of the next t lines there are two numbers m and n (1
  <= m <= n <= 1000000000, n-m<=100000) separated by a space.

  Output

  For every test case print all prime numbers p such that m <= p <= n,
  one number per line, test cases separated by an empty line.

My Answer:

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

int main() {
    int iter = 0;
    scanf("%d",&iter);
    while(iter--) {
        long num1, num2;
        scanf("%ld%ld",&num1,&num2);

        long prime[60000];
        prime[0] = 2;
        long prime_count = 1;
        long result[60000];
        long result_count = 0;

        if(num1 <= 2){
            result[0] = 2;
            result_count++;
            printf("2\n");
        }

        for(long i = 3; i <= num2; i++) {
            int isPrime = 1;

            for(long j = 0; j < prime_count; j++) {
                if(i % prime[j] == 0) {
                    isPrime = 0;
                    break;
                }
            }
            if(isPrime) {
                prime[prime_count] = i;
                prime_count++;
                if(i >= num1){
                    result[result_count] = i;
                    result_count++;
                    printf("%ld\n",i);
                }
           }
        }
        printf("\n");
    }

    return 0;
}

Is there any way to improve my code?

well, calculating all the primes from 2 through 1000000000 will take too long.

Suggest pre-calculating (offline) all those primes and output them into a .txt file.

Then edit that file into a C data table of size_t values.

then in your program, paste that text file into the file scope of your program. I.E. outside of any function.

remove those massive 60000 arrays from your program.  All you really need is a counter to count the primes in the range n...m

In the offline program, use the Sieve of Eratosthenes algorithm (use google to find the algorithm)

using scanf() and 'printf()` burns LOTs of precious time.

suggest using the following functions for fast read/write of a Number.  Use repeated calls to putc_unlocked() to output a string.  Note: Be sure to output a final '\n' so the output ends in a blank line.

#include <stdio.h>

void fastRead( size_t *a );
void fastWrite( size_t a );

inline void fastRead(size_t *a)
{
    int c=0;
    // note: 32 is space character
    while (c<33) c=getchar_unlocked();

    // initialize result value
    *a=0;

    // punctuation parens, etc are show stoppers
    while (c>47 && c<58)
    {
        *a = (*a)*10 + (size_t)(c-48);
        c=getchar_unlocked();
    }
    //printf( "%s, value: %lu\n", __func__, *a );
} // end function: fastRead

inline void fastWrite(size_t a)
{
    char snum[20];
    //printf( "%s, %lu\n", __func__, a );

    int i=0;
    do
    {
        // 48 is numeric character 0
        snum[i++] = (char)((a%10)+(size_t)48);
        a=a/10;
    }while(a>0);

    i=i-1; // correction for overincrement from prior 'while' loop

    while(i>=0)
    {
        putchar_unlocked(snum[i--]);
    }
    putchar_unlocked('\n');
} // end function: fastWrite

Then your program only needs to do the following:

in English:

input numbertestcases
while decrement numbertestcases > 0
    input 'n'
    input 'm'

    set primescount = 0

    for ( size_t x=0;
          m <= primestable[x]
          &&
          x < sizeof( primestable )/ sizeof( *primestable );
          increment x)
        if primestable[x] >= n
             increment primescount
        endif
    end for

    output primescount
    output '\n'
end for

output '\n'

then:

the code will be VERY fast
no need for #include <string.h>
no need for #include <math.h>
probably no need for #include <stdlib.h>


NodeJS executing multiple database queries using async library I have a use case where I need to run multiple tasks in series. Each task is required to perform some database operations. In my code step_1, step_2 and step_3 are these task which get executed in series (Async Waterfall)
Following is my code
const config = require('config.json');
var mysql = require('mysql');
var async = require("async");

const pool = mysql.createPool({
  host: config.database.dbhost,
  user: config.database.dbuser,
  password: config.database.dbpassword,
  database: config.database.dbname
});

exports.handler = (event, context, callback) => {

    pool.getConnection((err, connection) => {
    if (err) throw new Error("Unable to get connection " + JSON.stringify(err));

      async.waterfall([
            step_1,
            step_2,
            step_3
        ], function (err, result) {
            if(err){
                throw new Error("something went south : "+ JSON.stringify(err));
            }else{
                connection.release();
                callback(null, "SUCCESS: " + JSON.stringify(result));
            }
        });

        function step_1(cb){
            connection.query("step 1 query", function (error, response) {
                      if (error){
                        cb(null,error,null);
                      }
                      let id=result.insertId;
                      cb(null,null,id);
            });
        }

        function step_2(error,id,cb){
            if(error){
                cb(null,error);
            }else{
                connection.query("step 2 query",,function (err,result){
                        if (err){
                          connection.query("error logging query",
                          function(er,result){
                            if (er) cb(null,er,null);
                          });
                          cb(null,err,null);
                        }
                    cb(null,null, id, result);
                });
            }
        }

        function step_3(error, id,result,cb){
            if(error){
                cb(error,null);
            }else{
                connection.query("step 3 query",function(err,result){
                    if (err) cb(err,null);
                    cb(null,result);
                });
            }
        }

    });
};

Although I have used a library to avoid nesting callbacks within callbacks, it still feels like bit callback-ish. Is there a better way to achieve my use case? Anything I can improve in my code, any suggestions please.
I have only included relevant parts of code, kindly let me know if any additional information is required.

A couple of helper functions will aid in flattening your callback chain:
function getConnection() {
  return new Promise((res, rej) => {
    pool.getConnection(function (err, connection) {
      if (err) {
        rej(err);
      } else {
        res(connection);
      }
    });
  });
}

function query(connection, sql, params) {
  return new Promise((res, rej) => {
    connection.query(sql, params, function (err, rows) {
      if (err) {
        rej(err);
      } else {
        res(rows);
      }
    });
  });
}

Then you can do something like:
async function step_1(connection) {
  const rows = await query(connection, 'step 1 query');
  return rows.insertId;
}

function step_2(connection, id) {
  return query(connection, 'step 2 query', id);
}

function step_3(connection, id, result) {
  return query(connection, 'step 3 query', id, result);
}

exports.handler = async (event) => {
  const connection = await getConnection();
  try {
    const id = await step_1(connection);
    const rows = await step_2(connection, id);
    const result = await step_3(connection, id, rows);
    return JSON.stringify(result);
  } finally {
    connection.release();
  }
};


A more Pythonic way to iterate over a list with duplicate items Is there a more elegant and Pythonic way to handle counters in this solution to an exercise from the NLTK book? The exercise asks to print out the context (one word forward and one word back) for every verb of a particular type (tagged VN).

import nltk

wsj = nltk.corpus.treebank.tagged_words(simplify_tags=True)
cdf = nltk.ConditionalFreqDist((tag, word) for (word, tag) in wsj)

wordlist = cdf['VN'].keys()

# Bug 1: strange exceptions in the for loop when iterating over wsj
# Solution: wsj is a custom NLTK type "ConcatenatedCorpusView"
# cast wsj into a native python type "list" for better iteration.
# I am guessing ConcatenatedCorpusView chokes on empty tuples
wsj_list = list(wsj)

# Bug 2: repeated words return index of the first word only
# Solution: to deal with repeated words
# we keep indexing from the last location. The index method
# takes a second parameter
starts_at = 0

for t in wsj_list:
    if t[0] in wordlist and t[1] == 'VN':
        ndx = wsj_list.index(t,starts_at)
        starts_at = ndx + 1
        print wsj_list[ndx-1:ndx+1], ndx

You could use enumerate to get the index. It makes the code both simpler and more efficient, as the linear search of index is avoided. I would also suggest unpacking t to (word, tag) to improve readability.

for ndx, (word, tag) in enumerate(wsj_list):
    if word in wordlist and tag == 'VN':
        print wsj[ndx-1:ndx+1], ndx


Implementation of a queue for digicode-like system What it is about

I created an application where you need to push invisible buttons in a certain order to unlock the advanced mode. To handle this, I wanted to use a queue (FIFO) of my own, adapted to my problem.

Every time I click a button, its ID is added to the queue with addItem, and the password gets checked with checkCorrect. It is meant to work similarly to digicode doors. The size attribute determines how many values the queue can remember, hence the maximum length of the password.

What I'd like to know

Do you think this is a good code? In which ways can it be improved? Are there other ways (with smaller / more elegant code) to implement this?

The code

class MyQueue():
    def __init__(self, size, default_value=None):
        self.size = size
        self.content = [default_value]*size
        self.end = self.size-1
    def addItem(self, value):
        self.end = (self.end+1) % self.size
        self.content[self.end] = value
    def printQueue(self):
        print 'end='+str(self.end)+' - '+str(self.content)
    def checkCorrect(self, password):
        pw = list(reversed(password))
        i = 0
        j = self.end
        if len(pw) > self.size:
            return False
        while True:
            if pw[i] <> self.content[j]:
                return False
            else:
                i += 1
                j = (j-1) % self.size
                if i == len(pw):
                    return True

if __name__=='__main__':
    q = MyQueue(size=4, default_value=0)
    q.addItem(1)
    q.addItem(8)
    q.addItem(8)
    q.addItem(5)
    q.addItem(7)
    q.addItem(3)
    print q.checkCorrect([5, 7, 3])
    print q.checkCorrect([2, 7, 3])

Some PEP8 guidance:

class MyQueue(): here, you can remove the redundant parentheses as you're not passing any parameter to it. So you will have class MyQueue.

When inside of a class, it's recommended to have one space between your methods:

def addItem:
    ....

def printQueue:
    ....

It's also specified in PEP8 the following (methods naming):

  Use the function naming rules: lowercase with words separated by
  underscores as necessary to improve readability.

That said, addItem would become add_item.

More

Don't use <> operator. It's not usual. I strongly recommend you using != operator (which is the common way of doing this) when you're trying to say not equal. Later on, you might want to use this with a more recent Python version (say Python 3.5). This way, you won't have any issues.

Related to portability, you might also want to add parentheses when using print(). It's not necessary since you're using Python 2.7, but it might help you in the future.

Classes and methods inside them usually have docstrings. To quote the documentation:

  A docstring is a string literal that occurs as the first statement in
  a module, function, class, or method definition. Such a docstring
  becomes the doc special attribute of that object.

  All modules should normally have docstrings, and all functions and
  classes exported by a module should also have docstrings.

That said:

class MyQueue:
    """Some text to describe what you're class does"""
    ....

    def add_item(self, value):
        """ what this method does """
        ...

In a perfect world, which obviously we're not living in, I would've expect to see some push / pop methods or queue / dequeue ones since we're talking about a queue implementation and it's way of handling things (FIFO). Perhaps add_item might be push_item.

More to come


Namedtuple and list to populate each record of a JSON file I had a JSON file in this format:

{
    "cols": [
        "Employee",
        "start_time"
    ],
    "data": [
        [
            "Serena",
            "Sat, 22 Aug 2015 14:06:03 -0700"
        ],
        [
            "Rhonda",
            "Sun, 25 Mar 2012 10:48:52 -0700"
        ],
        [
            "Fleur",
            "Mon, 16 Dec 2013 07:20:26 -0800"
        ],
        [
            "Giselle",
            "Sat, 19 Apr 2008 23:47:21 -0700"
        ],
        [
            "Jeanette",
            "Thu, 06 Nov 2008 23:02:44 -0800"
        ],
        [
            "Iliana",
            "Sun, 13 May 2007 14:22:08 -0700"
        ],
        [
            "Geraldine",
            "Tue, 24 Jul 2012 08:43:58 -0700"
        ],
        [
            "Tatiana",
            "Thu, 08 Oct 2009 07:56:25 -0700"
        ],
        [
            "Jessamine",
            "Wed, 14 Jun 2006 12:03:42 -0700"
        ]
        [
            "Emily",
            "Tue, 06 Jan 2015 04:51:06 -0800"
        ],
        [
            "Sydnee",
            "Mon, 16 Dec 2013 11:28:04 -0800"
        ],
        [
            "Zorita",
            "Wed, 16 Dec 2009 11:22:18 -0800"
        ]

    ]
}

The file has Employee name and start time fields. I wanted to get every employee's name and start time and find the total time they have been  in the company (subtracting start time from current time).

I first loaded a JSON file using json.load() and then read each record in a loop and store it in a namedtuple. After that I added each namedtuple in a list.

import json
import pprint
import dateutil.parser
from collections import namedtuple
from datetime import datetime, timezone

file_name = "d:/a.json"
#Create a named tuple to store all data later,
# Total time is Current time - start_time
EmployeeData = namedtuple('EM', ["name", "start_time", "total_time"])

# Here I will store final list of all employee tuples
final_list = []

# Get string date as input and convert it to datetime object
def format_time(string_time):
    op_time = dateutil.parser.parse(string_time)
    return op_time

with open(file_name, "r") as data:
    json_data = json.load(data)
    for record in json_data["data"]:
        # Time in JSON file also has timezone so i have to use timezone.utc
        today = datetime.now(timezone.utc)
        # create date object from string date
        record[1] = format_time(record[1])
        # Find total number of days,
        tenure = (today - record[1]).days
        # create a tuple
        temp_tuple = EmployeeData(name=record[0], start_time = record[1], total_time = tenure)
        final_list.append(temp_tuple)

pprint.pprint(final_list)

Output:

 EM(name='Whitney', start_time=datetime.datetime(2015, 8, 7, 5, 37, 32, tzinfo=tzoffset(None, -25200)), total_time=1165),
 EM(name='Deirdre', start_time=datetime.datetime(2009, 8, 19, 15, 50, 27, tzinfo=tzoffset(None, -25200)), total_time=3343),
 EM(name='Alexandra', start_time=datetime.datetime(2007, 9, 5, 17, 31, 29, tzinfo=tzoffset(None, -25200)), total_time=4057),
 EM(name='Lila', start_time=datetime.datetime(2011, 8, 27, 8, 8, 47, tzinfo=tzoffset(None, -25200)), total_time=2606),
 EM(name='TaShya', start_time=datetime.datetime(2009, 1, 1, 18, 15, 1, tzinfo=tzoffset(None, -28800)), total_time=3573),
 EM(name='Kerry', start_time=datetime.datetime(2013, 6, 20, 13, 39, 30, tzinfo=tzoffset(None, -25200)), total_time=1942)]

I then sorted the record using:

sorted_time = sorted(final_list, key=lambda y: y.total_time)
print(sorted_time)

I would like to improve my code further since I will be dealing with a larger files soon. Is there any way to make it more efficient?

A small issue, that is probably no big deal for this program, but could cause problems over smaller timescales than days:

json_data = json.load(data)
for record in json_data["data"]:
    # Time in JSON file also has timezone so i have to use timezone.utc
    today = datetime.now(timezone.utc)

Having a different definition of "now" for each element in the file is unexpected.  We can ensure consistency by assigning today once, outside the loop.  (That slightly improves efficiency, too - but probably not by enough to measure!)


Sign In or Sign Up - either way don't repeat yourself I have one application with two views. One to sign in and one to sign up. Each view has an Email entry as main identifier.

I want to add some itemTemplates to my application, because for now I repeat all UI properties twice.

In the code, a block is a listItem. Some blocks have the same name, but icons is different, and that's why I put it as an argument.

This is the new architecture I want to set up:

internal class SignUp : ViewModelBase
{
    private readonly List<object> _content = new List<object>();

    public SignUp()
    {
        _content.Add(BlockFactory.Create("Email"));
        _content.Add(BlockFactory.Create("Address"));
    }
}

internal class SignIn : ViewModelBase
{
    private readonly List<object> _content = new List<object>();

    public SignIn()
    {
        _content.Add(BlockFactory.Create("Email"));
    }
}

//So SignUp.Content[1].BlockItems.ElementAt(0) will be different to
// SignUp.Content[0].BlockItems.ElementAt(0)

public class BlockItem
{
    public bool IsVisible;
    public string Name;
    public string Template;
    public object Value;
}

public class Block
{
    public List<BlockItem> BlockItems = new List<BlockItem>();
    public string Icon;
    public string Name;

    public Block(string name, string icon)
    {
        Name = name;
        Icon = icon;
    }
}

public static class BlockFactory
{
    public static Block Create(string name)
    {
        switch (name)
        {
            case "Email":
                return new Block("Email", "")
                {
                    BlockItems = new List<BlockItem>
                    {
                        new BlockItem
                        {
                            Name = "Email",
                            Value = "",
                            IsVisible = true,
                            Template = "Entry"
                        }
                    }
                };
            case "Address":
                return new Block("Address", "")
                {
                    BlockItems = new List<BlockItem>
                    {
                        new BlockItem
                        {
                            Name = "Street",
                            Value = "",
                            IsVisible = true,
                            Template = "Entry"
                        },
                        new BlockItem
                        {
                            Name = "Zip",
                            Value = "",
                            IsVisible = true,
                            Template = "Entry"
                        },
                        new BlockItem
                        {
                            Name = "City",
                            Value = "",
                            IsVisible = true,
                            Template = "AnotherTemplate"
                        }
                    }
                };
        }
        return null;
    }
}

Is it possible to review my code and share possible improvements?

In the UI side, I'll have only one list with itemTemplate and based on the BlockItems.Count. I'll display elements in one line or more.

Here is an example of where we could combine the Factory Pattern along with a Builder Pattern to extend your current code.

class SignUp : ViewModelBase
{
    private readonly List<Block> _content = new List<Block>();

    public SignUp()
    {
        _content.Add(BlockFactory.Create(BlockType.Email));
        _content.Add(BlockFactory.Create(BlockType.PostalAddress));
    }
}

class SignIn : ViewModelBase
{
    private readonly List<Block> _content = new List<Block>();

    public SignIn()
    {
        _content.Add(BlockFactory.Create(BlockType.Email));
    }
}

public static class BlockType
{
    public const string Email = "Email";
    public const string PostalAddress = "Postal Address";
}

public static class BlockFactory
{
    public static Block Create(string blockType)
    {
        if (String.IsNullOrEmpty(blockType)) throw new ArgumentNullException(nameof(blockType));

        switch (blockType)
        {
            case BlockType.Email:
                return new BlockBuilder()
                    .Name(BlockType.Email)
                    .Icon("email_icon.png")
                    .Build();
            case BlockType.PostalAddress:
                return new BlockBuilder()
                    .Name(BlockType.PostalAddress)
                    .Icon("postal_icon.png")
                    .Description("Give us your mailing address to get great stuff by postal mail!")
                    .BlockItems(
                        new List<BlockItem>()
                        {
                            new BlockItem() { Name = "Street", Value = "", IsVisible = true, template = "" },
                            new BlockItem() { Name = "Zip", Value = "", IsVisible = true, template = "" },
                            new BlockItem() { Name = "City", Value = "", IsVisible = true, template = "" }
                        })
                    .Build();
        }

        throw new ArgumentException("Invalid BlockType.");
    }
}

public class BlockItem
{
    public string Name;
    public object Value;
    public bool IsVisible;
    public string template;
}

public class Block
{
    public List<BlockItem> BlockItems = new List<BlockItem>();
    public string Name;
    public string Icon;
    public string Description;
}

public class BlockBuilder
{
    private List<BlockItem> _blockItems;
    private string _name;
    private string _icon;
    private string _description;

    public BlockBuilder BlockItems(List<BlockItem> value)
    {
        _blockItems = value;
        return this;
    }

    public BlockBuilder Name(string value)
    {
        _name = value;
        return this;
    }

    public BlockBuilder Icon(string value)
    {
        _icon = value;
        return this;
    }

    public BlockBuilder Description(string value)
    {
        _description = value;
        return this;
    }

    public Block Build()
    {
        return new Block
        {
            BlockItems = _blockItems,
            Name = _name,
            Icon = _icon,
            Description = _description
        };
    }
}

If you don't like the constants for the BlockType, you can remove this part, but I tend to lean towards easier refactoring instead of using "hand-coded" strings to pass into factories.

public static class BlockType
{
    public const string Email = "Email";
    public const string PostalAddress = "Postal Address";
}

The reason for Builders, is that you can really get good mileage on your class as you need to expand it with more properties and not have to write so many parameter overrides, etc.

For example you could do this in the future with relative ease and straight-forward implementation.

Block block = new BlockBuilder()
            .Name(BlockType.Email)
            .Icon("email_icon.png")
            .Template(...)
            .Styling(...)
            .CloseEventHandler(...)
            .OpenEventHandler(...)
            .Preloader(...)
            .Build();


Implementation of the visitor pattern I tried implementing visitor patter and have questions about my usage of is and as keywords. Is there a big performance hit when using them?

 class Program
{
    static void Main(string[] args)
    {
        List<IVisitor> visitors = new List<IVisitor>();
        visitors.AddRange(new List<IVisitor>()
        {
            new PersonVisitor(),
            new AnimalVisitor()
        });

        List<Creature> creatures = new List<Creature>();
        creatures.AddRange(new List<Creature>()
        {
            new Person() { Name = "Frank" },
            new Person() { Name = "Tony" },
            new Person() { Name = "Amy" },
            new Animal() { Name = "Bubbles" },
            new Animal() { Name = "Max" }
        });

        CreatureProcessor creatureProcessor = new CreatureProcessor(creatures, visitors);
        creatureProcessor.Process();

        Console.ReadKey();
    }
}

interface IVisitor
{
    void Visit(IElement element);
}

interface IElement
{
    void Accept(IVisitor visitor);
}

class PersonVisitor : IVisitor
{
    public void Visit(IElement element)
    {
        if (element is Person)
        {
            Creature creature = element as Person;

            Console.WriteLine("{0} is a good {1}", creature.Name, "Person");
        }
    }
}

class AnimalVisitor : IVisitor
{
    public void Visit(IElement element)
    {
        if (element is Animal)
        {
            Creature creature = element as Animal;

            Console.WriteLine("{0} is a good {1}", creature.Name, "Animal");
        }
    }
}

class Person : Creature
{

}

class Animal : Creature
{

}

abstract class Creature : IElement
{
    public string Name { get; set; }

    public void Accept(IVisitor visitor)
    {
        visitor.Visit(this);
    }
}

class CreatureProcessor
{
    private readonly List<IVisitor> visitors;
    private readonly List<Creature> creatures;

    public CreatureProcessor(List<Creature> creatures, List<IVisitor> visitors)
    {
        this.creatures = creatures;
        this.visitors = visitors;
    }

    public void Process()
    {
        foreach (IVisitor visitor in visitors)
        {
            foreach (Creature creature in creatures)
            {
                creature.Accept(visitor);
            }
        }
    }
}

It works, the way it should I think, so would anyone care to critique it?

The Visitor Pattern

The main purpose of the Visitor pattern is to define extra elements of functionality for a number of classes in a single place. Your PersonVisitor and AnimalVisitor don't really demonstrate this at all - the name indicates that each visitor corresponds to a target type, not a piece of functionality. Better would be a single PrintIsGoodVisitor (probably with a better name). And given that your Visitor will be visiting IElement instances, IVisitor should probably be renamed IElementVisitor too.

interface IElementVisitor
{
    void Visit(IElement element);
}

class PrintIsGoodVisitor : IElementVisitor
{
    public void Visit(IElement element)
    {
        if (element is Person)
        {
            Creature creature = element as Person;
            Console.WriteLine("{0} is a good {1}", creature.Name, "Person");
        }

        if (element is Animal)
        {
            Creature creature = element as Animal;
            Console.WriteLine("{0} is a good {1}", creature.Name, "Animal");
        }
    }
}

You've now got a nasty code smell - passing in an IElement and checking: "Is it a Person? Do this", "Is it an Animal? Do that". The conventional way of implementing a Visitor is to call a virtual method on the visitee to determine its runtime type before the visitee tells the visitor to come and visit (that could have been less confusing...). This involves making Creature.Accept abstract and overriding it in the concrete subclasses.

abstract class Creature : IElement
{
    public string Name { get; set; }

    public abstract void Accept(IElementVisitor visitor);
}

class Person : Creature
{
    public override void Accept(IElementVisitor visitor)
    {
        visitor.Visit(this);
    }
}

class Animal : Creature
{
    public override void Accept(IElementVisitor visitor)
    {
        visitor.Visit(this);
    }
}

This may look like a bit of a waste of time -- every Creature was calling visitor.Visit(this) before, and they're all doing the same now -- but there is a difference. If we change IElementVisitor to have a Visit method for every concrete type that derives from IElement, then each of those concrete types knows which of those Visit methods it can call based on its own type:

interface IElementVisitor
{
    void Visit(Person person);
    void Visit(Animal animal);
}

class PrintIsGoodVisitor : IElementVisitor
{
    public void Visit(Person person)
    {
        Console.WriteLine("{0} is a good {1}", person.Name, "Person");
    }

    public void Visit(Animal animal)
    {
        Console.WriteLine("{0} is a good {1}", animal.Name, "Animal");
    }
}

So when a Person says visitor.Visit(this), it can only be the first version, because this is a Person, and when an Animal says it, it can only be the second.

With these changes, we arrive at a program which is functionally identical to your original, but without a cast in sight - dynamic dispatch has taken their place.

With the Visitor side of things looked at, some points on the rest of the code:

Initializing Lists

In Main, you're creating a list specifically so you can add its elements to a second list. You can just use the second list directly. Additionally, when using a parameterless constructor and an initializer list, you can omit the constructor brackets:

List<Creature> creatures = new List<Creature>
{
    new Person { Name = "Frank" },
    new Person { Name = "Tony" },
    new Person { Name = "Amy" },
    new Animal { Name = "Bubbles" },
    new Animal { Name = "Max" }
};

var

It's considered good practice by many to use var where you can (at the very least for non built-in types) - it's a form of DRY to not repeat the type name twice:

var creatures = new List<Creature>
{
    new Person { Name = "Frank" },
    new Person { Name = "Tony" },
    new Person { Name = "Amy" },
    new Animal { Name = "Bubbles" },
    new Animal { Name = "Max" }
};

Principle of Least Privilege

This principle says that you should use the most general argument types you can for your methods. What is your CreatureProcessor going to do with its visitors and creatures lists? Enumerate their contents. So maybe IEnumerable<T> is a good fit here?

However, it's bad practice to iterate over an IEnumerable<T> more than once (it could be generating data from database calls for all we know!), and not only is Process potentially iterating over creatures multiple times, but there's nothing to stop Process from being called repeatly either. I'd suggest the next most general type that implies the data is safe to iterate repeatedly - IReadOnlyCollection<T>.

Using a more general type means that it can be used with collections other than List<T>, and signals to the user that you're not going to modify the collection you've been given (which List<T> can imply). (See this question for more discussion).

class CreatureProcessor
{
    private readonly IReadOnlyCollection<IVisitor> visitors;
    private readonly IReadOnlyCollection<Creature> creatures;

    public CreatureProcessor(IReadOnlyCollection<Creature> creatures, IReadOnlyCollection<IVisitor> visitors)
    {
        this.creatures = creatures;
        this.visitors = visitors;
    }

    ...
}

Unnecessary Complexity

The CreatureProcessor type seems redundant - you pass two collections into its constructor which it stores, then you call a single method on it. Until you need something more complicated (see YAGNI), a single method alongside Main would suffice here. Even inlining it in Main is a possibility, but it feels sufficiently like a single responsibility that it deserves its own method.

class Program
{
    ...

    private static void VisitAllCreatures(IReadOnlyCollection<IElementVisitor> visitors, IReadOnlyCollection<Creature> creatures)
    {
        foreach (var visitor in visitors)
        {
            foreach (var creature in creatures)
            {
                creature.Accept(visitor);
            }
        }
    }
}

Also, on a similar theme, IElement isn't really doing a lot for you. It's directly implemented once and never used anywhere that Creature wouldn't be valid. It can be omitted (renaming IElementVisitor to ICreatureVisitor too).

Little Things

I believe it's a good idea to be explicit about access modifiers (Stack Overflow question). I've gone with whatever the default value is, leaving the visibility unchanged from your original.
Creature.Name is only ever set at construction and read thereafter, so could be changed to a readonly property set through a constructor - less mutability is less scope for bugs. You can add named parameters during construction if there could otherwise be confusion over what "Frank" represents. The new Creature constructor should be protected because the class is abstract and can't be instantiated.
I'm also a believer of Josh Bloch's "Design and document for inheritance or else prohibit it". If you don't intend to have a class inherited from, mark it sealed so if someone does decide they want to do that, they know they may need to tweak the base class to make it work correctly.

The Final Code

internal abstract class Creature
{
    protected Creature(string name)
    {
        Name = name;
    }

    public string Name { get; }

    public abstract void Accept(ICreatureVisitor visitor);
}

internal sealed class Person : Creature
{
    public Person(string name)
        : base(name)
    {
    }

    public override void Accept(ICreatureVisitor visitor)
    {
        visitor.Visit(this);
    }
}

internal sealed class Animal : Creature
{
    public Animal(string name)
        : base(name)
    {
    }

    public override void Accept(ICreatureVisitor visitor)
    {
        visitor.Visit(this);
    }
}

internal interface ICreatureVisitor
{
    void Visit(Person person);
    void Visit(Animal animal);
}

internal sealed class PrintIsGoodVisitor : ICreatureVisitor
{
    public void Visit(Person person)
    {
        Console.WriteLine("{0} is a good {1}", person.Name, "Person");
    }

    public void Visit(Animal animal)
    {
        Console.WriteLine("{0} is a good {1}", animal.Name, "Animal");
    }
}

internal sealed class Program
{
    public static void Main(string[] args)
    {
        var visitors = new List<ICreatureVisitor>
        {
            new PrintIsGoodVisitor()
        };

        var creatures = new List<Creature>
        {
            new Person(name: "Frank"),
            new Person(name: "Tony"),
            new Person(name: "Amy"),
            new Animal(name: "Bubbles"),
            new Animal(name: "Max")
        };

        VisitAllCreatures(visitors, creatures);

        Console.ReadKey();
    }

    private static void VisitAllCreatures(IReadOnlyCollection<ICreatureVisitor> visitors, IReadOnlyCollection<Creature> creatures)
    {
        foreach (var visitor in visitors)
        {
            foreach (var creature in creatures)
            {
                creature.Accept(visitor);
            }
        }
    }
}


Remove "lines" from a 2D vec I have some code here that I am very unhappy with. The task I am trying to accomplish is this.

Given a 2d vec like this:

[[0 2 0] [1 3 5] [3 3 0]]

which can contain positive ints including zero I want to remove all lines that are greater zero.

Where the definition of line is the following:

  A line is represented by the position n in every vec inside the 2d
  vec.

So my example above has three lines:

[0 1 3], [2 3 3] and [0 5 0].

The line that I want to remove from it according to my algortihm is [2 3 3] because every element is greater than zero.

So my 2d vec would now look like this:

[[0 0] [1 5] [3 0]]

And finally I want to pad the vecs to their original size filling them with zero for every removed line, so that it looks finally like this:

[[0 0 0] [0 1 5] [0 3 0]]

This is what I came up with:

(defn in?
  "true if seq contains elm"
  [seq elm]
  (some #(= elm %) seq))

(defn not-in?
  "true if seq does not contain elm"
  [seq elm]
  (not (in? seq elm)))

(defn all-greater-zero-at
  "Given a 2-d vec [[0 1] [0 2]] return true if all elements at 'at' are
  greater than zero"
  [v at]
  (not-in? (map #(if (> (nth % at) 0) true false) v) false))

(defn to-be-removed
  "Returns a seq of positions to be removed (0 3 4)"
  [v width]
  (reduce (fn [a b] (if (all-greater-zero-at v b) (conj a b) a)) [] (range width)))

(defn remove-at
  "Removes an element from a 1d vec"
  [v at]
  (into [] (concat (subvec v 0 at) (subvec v (+ at 1) (count v)))))

(defn insert-at
  "inserts an element into a 1d vec"
  [v elm at]
  (into [] (concat (subvec v 0 at) elm (subvec v at (count v)))))

(defn remove-and-replace-all-at
  [v at]
  (map #(insert-at (remove-at % at) [0] at) v))

(defn replace-full-by-zero [v width]
  (reduce (fn [a b] (remove-and-replace-all-at a b)) v (to-be-removed v width)))

(defn remove-zeros [v at]
  (reduce (fn [a b] (conj a (remove-at b at))) [] v))

(defn fill-with-zeros
  "Takes a 2d vec and pads ith with zeros up to width"
  [v width]
  (map #(into [] (concat (take (- width (count (first v))) (repeat 0)) %)) v))

(defn clean-grid
  "removes all full lines"
  [fbz tbr]
  (loop [acc fbz tbr tbr i 0]
    (if (empty? tbr)
      acc
      (recur (remove-zeros acc (- (first tbr) i)) (rest tbr) (inc i)))))

(defn remove-full-lines [v width]
  (let [fbz (replace-full-by-zero v width)
        tbr (to-be-removed v width)
        cleaned-grid (clean-grid fbz tbr)]
    (into [] (fill-with-zeros cleaned-grid width))))

This seems like a lot of code for such a "simple" algorithm and I assume there are a lot of better ways to do that, but just did not come up with a better one, so, please, go ahead and fix it, if you want to.

A transpose function would be useful here. This is the code I came up with:

(defn transpose [m]
  "Transposes a matrix, returning a new matrix. For 2D matrices, rows and columns are swapped."
  (vec (apply map vector m)))

(defn any-zero? [r]
  ((complement not-any?) #(= 0 %) r))

(defn empty-matrix [x y]
  (vec (repeat y (vec (repeat x 0)))))

(defn filter-matrix [m]
  (let [row-size (-> m first count)
        fm (filterv any-zero? m)
        padding-size (- (count m) (count fm))]
          (into (empty-matrix row-size padding-size) fm)))

(defn transform-matrix [m]
  (-> m
      transpose
      filter-matrix
      transpose))

Let's try it out:

=> (transform-matrix [[0 2 0] [1 3 5] [3 3 0]])
[[0 0 0] [0 1 5] [0 3 0]]

If you want to do more advanced operations on matrices I recommend looking into the core.matrix library.

Hope this helps!


Excel custom cell style remover When you only want a couple styles from a workbook but not all of them it's easier to merge styles then remove those you don't want.

How can I improve the form or the backing code?

Userform with styles listed.

'CustomStyleRemoverForm
Private Sub UserForm_Initialize()
    ListStyles
End Sub

Private Sub ListStyles()
    CustomStylesListBox.Clear

    Dim listStyle As Style
    For Each listStyle In ActiveWorkbook.Styles
        If Not listStyle.BuiltIn Then
            CustomStylesListBox.AddItem listStyle.Name
        End If
    Next
End Sub

Private Sub SelectAllButton_Click()
    CheckAllBoxes True
End Sub

Private Sub UnselectAllButton_Click()
    CheckAllBoxes False
End Sub

Private Sub InvertSelectionButton_Click()
    Dim counter As Long
    For counter = 0 To CustomStylesListBox.ListCount - 1
        CustomStylesListBox.Selected(counter) = Not CustomStylesListBox.Selected(counter)
    Next
End Sub

Private Sub CheckAllBoxes(ByVal updateValue As Boolean)
    Dim counter As Long
    For counter = 0 To CustomStylesListBox.ListCount - 1
        CustomStylesListBox.Selected(counter) = updateValue
    Next
End Sub

Private Sub RemoveSelectedButton_Click()
    Dim counter As Long
    For counter = 0 To CustomStylesListBox.ListCount - 1
        If CustomStylesListBox.Selected(counter) Then
            ActiveWorkbook.Styles(CustomStylesListBox.List(counter)).Delete
        End If
    Next

    ListStyles
End Sub

There is not much to review, your code is clean.  There are some things that you can do to improve User Experience (UX).

Replace the label, SelectAllButton and UnselectAllButton with a checkbox.
Place a Frame around the Controls
Add a ComboBox to change target workbooks
Add buttons to copy and paste styles between workbooks

And if you want to go buckwild you could add a Preview Button


Speed up query that handles auditing and versioning I have the following tables:

tag_name_audit table

tag_name table

tag_adit table

tag table

The action that the query performs is auditing and version handling.
For example i can have one set of data that has a stable version and an edit version.
And you can release that last edit version and it becomes the last stable version.

I have the following query:

SELECT
            t.id as u_id,
            IF(tn.state = 'P',tna.name,tn.name) as name,
            t.version,
            tn.state
            FROM tag AS t
            INNER JOIN tag_name AS tn ON tn.tag_parent = t.id
            LEFT JOIN tag_name_audit AS tna ON tna.tag_name_parent = tn.id AND tn.state = 'P'
            AND  tna.id = (
                            SELECT MAX(id)
                            FROM tag_name_audit
                            WHERE tag_name_parent = tn.id
                        )
            WHERE tn.language = ?
            AND t.version IS NOT null
            AND tn.tag_parent IN (".$planIn.")

I'm curious if there are any better solutions writing it so that i can boost the performance of the query speed.

  Showing rows 0 - 8 (9 total, Query took 0.0009 seconds.)

You should use a provided Aggregate function sorted over a group by statement.

Select
            t.id as u_id,
            (case tn.state when 'P' then SUBSTRING(tna.id,4,100) else tn.name end) as name,
            t.version,
            tn.state
            FROM tag AS t
            INNER JOIN tag_name AS tn ON tn.tag_parent = t.id
            LEFT JOIN
            ( select max(CONCAT(format(id, '000'),name)) id,tag_name_parent  from tag_name_audit group by tag_name_parent ) AS tna
            ON tna.tag_name_parent = tn.id
            AND tn.state = 'P'
            AND tn.language = ?
            AND t.version IS NOT null
            AND tn.tag_parent IN (".$planIn.")

for 100 records your query rendered 13000700 NS for my query and 295016900 for yours :O quite remarkable difference !

You can check my test cases and see by yourself:

declare @tag table(id int NOT NULL IDENTITY(1,1), version varchar(1) NULL)
declare @tag_name table(tag_parent int,id int NOT NULL IDENTITY(1,1),name varchar(5),state varchar(1))
declare @tag_name_audit table(tag_name_parent int,id int NOT NULL IDENTITY(1,1),name varchar(5))

declare @cnt int
declare @max int

set @cnt=1
set @max=100

WHILE @cnt <= @max
BEGIN
   insert @tag(version) values (char(cast(DATEPART(nanosecond,SYSDATETIME())/1050 as int)%10+97))
   WAITFOR DELAY '00:00:00.015';
   insert @tag_name(name,tag_parent,state) values ('tn_'+char(cast(DATEPART(nanosecond,SYSDATETIME())/1050 as int)%10+97), cast( DATEDIFF(nanosecond,cast(GETDATE () as datetime2),SYSDATETIME())/90 as int)%@max+1,char(cast(DATEPART(nanosecond,SYSDATETIME())/990 as int)%2+ascii('P')))
   WAITFOR DELAY '00:00:00.015';
   insert @tag_name_audit(name,tag_name_parent) values ('tna_'+char(cast(DATEPART(nanosecond,SYSDATETIME())/1050 as int)%10+97), cast( DATEDIFF(nanosecond,cast(GETDATE () as datetime2),SYSDATETIME())/90 as int)%@max+1)
   SET @cnt = @cnt + 1;
   WAITFOR DELAY '00:00:00.015';
END;

select * from @tag order by id;
select * from @tag_name order by tag_parent;
select * from @tag_name_audit order by tag_name_parent;

DECLARE @t1 time;
DECLARE @t2 time;
DECLARE @t1_ time;
DECLARE @t2_ time;

SET @t1 = SYSUTCDATETIME() ;

Select
            t.id as u_id,
            (case tn.state when 'P' then SUBSTRING(tna.id,4,100) else tn.name end) as name,
            t.version,
            tn.state,
            tn.id tn_id,
            tna.id tna_id
            FROM @tag AS t
            INNER JOIN @tag_name AS tn ON tn.tag_parent = t.id
            LEFT JOIN
            ( select max(CONCAT(format(id, '000'),name)) id,tag_name_parent from @tag_name_audit group by tag_name_parent) AS tna
            ON tna.tag_name_parent = tn.id
            AND tn.state = 'P'
            AND t.version IS NOT null

SET @t2 = SYSUTCDATETIME () ;
SELECT DATEDIFF(NANOSECOND,@t1,@t2) AS elapsed_ms;

SET @t1_ = SYSUTCDATETIME()  ;

SELECT
            t.id as u_id,
            (case tn.state when 'P' then tna.name else tn.name end) as name,
            t.version,
            tn.state,
            tn.id tn_id,
            tna.id tna_id
            FROM @tag AS t
            INNER JOIN @tag_name AS tn ON tn.tag_parent = t.id
            LEFT JOIN @tag_name_audit AS tna ON tna.tag_name_parent = tn.id AND tn.state = 'P'
            AND  tna.id = (
                            SELECT MAX(id)
                            FROM @tag_name_audit
                            WHERE tag_name_parent = tn.id
                        )
            AND t.version IS NOT null;

SET @t2_ = SYSUTCDATETIME () ;
SELECT DATEDIFF(NANOSECOND,@t1_,@t2_) AS elapsed_ms;


Return a key when a function returns true I've no idea what to call this, so I called it Classify since that's how I use it.

Basically, the idea is to pass a Dictionary<TResult, Func<TValue, bool>> which is evaluated for an input, and the first Func<TValue, bool> returning true will return the TResult key.

The intention is to make this a generic extension method.

public TResult Classify<TValue, TResult>(TValue input, Dictionary<TResult, Func<TValue, bool>> classifications)
{
    foreach (var classification in classifications)
    {
        if (classification.Value.Invoke(input))
        {
            return classification.Key;
        }
    }

    return default(TResult);
}

Pretty simple, but pretty useful (for me at least) as well.

Usage:

<td class='<%#Classify(((ApiBatchLog)Container.DataItem).RequestsPerSecond, new Dictionary<string, Func<double, bool>> { ["good"] = x => x >= 3, ["neutral"] = x => x > 2, ["bad"] = x => true }) %>'>
    <%#((ApiBatchLog)Container.DataItem).RequestsPerSecond.ToString("0.00") %>
</td>

Needs more LINQ:

return classifications.FirstOrDefault(item => item.Value.Invoke(input)).Key;

This has the advantage (IMO) of highlighting the fact that you're discarding all but the first match (note that iteration order is not guaranteed!).

Classify doesn't really convey the fact that you're returning a Key from a dictionary. Seems FindKey would be a much better name.

A generic extension method seems like a good idea -  call your source source, and stick to TKey and TValue:

public static TKey FindKey<TKey, TValue>(this IDictionary<TKey, Func<TValue, bool>> source, TValue input)
{
    return source.FirstOrDefault(item => item.Value.Invoke(input)).Key;
}

That one-liner makes use of the fact that KeyValuePair<TKey,TValue> is a struct (and therefore can't be null), so you can just return .Key and that will be default(TKey).


Will this kind of codes leak memory? I am reading lot of articles regarding memory leaks. But i am not sure about below code. I usually do coding in this way. Can anyone tell me, will they leak memory?

I have doubts in following 3 scenarios (marked them in code).

I got these suspects after reading this post

var orderDetails = $("<root/>");

function GetOrderDetais()
{
    $.ajax(
    {
     url: url,
     dataType: 'xml',
     data: someThing,
     dataType: 'text',
     type: 'POST',
     contentType: "text/xml; charset=utf-8",
     success:function(value) //1 - closure function refrencing global variable
     {
        orderDetails = $(value);

        orderDetails.find("header").each(function() //2 - closure function refrencing global variable
        {
            //loop through order header details and do some DOM appends here.
        });

        ShowOrderDetails();
     },
     error:function(jqXHR, textStatus, errorThrown)
     {
        alert("Error" + textStatus + " ==> " + errorThrown);
     },
    });

}

function ShowOrderDetails()
{
    var orderNum = orderDetails.find("header oNum").text();

    $(orderDetails).find("item").each(function() //3 - closure function refrencing outer variable
    {
        //loop through item details and do some DOM appends here to form table.

        //one intresting statement.
        $("div").append($(this).find("itemNum").text()  + " - " + orderNum);
    });
}

  Update:

  See this Video to learn about memory
  leake.

No, there is nothing you're doing in that code that would result in memory leaks.

I found an article written by Jack Slocum (creator of the ExtJS framework) that details some basic steps to follow to avoid memory leaks. The original article is a bit dated and browsers have gotten a lot better at cleaning up memory, so I don't know how much of it still applies to the latest browsers (it certainly applies to older versions of IE and Firefox).

Here is the article: http://maxthonuser.blogspot.com/2008/02/easy-steps-to-avoid-javascript-memory.html

Relevant text (in case the link ever dies):

  It's as simple as these 3 steps:

  Never put anything in a DOM expando or property other than a primitive value unless you plan on cleaning it up.
  This is the most important rule of all. It may seem convenient to put your JS object in a DOM expando, so you can $() and get it, but don't do it. Sure, I know what you are thinking, I am being a little paranoid. There are lots of instances where putting a JS Object in a DOM expando won't cause a leak. That's true, but there are also many that will... some which are not so easy to detect (i.e. closures). So to avoid the possibility all together, I follow this simple rule.
  Clean up all your DOM event handlers on unload if there's a chance they could reference a DOM object.
  There's no reason to manually do this when there are libraries that do it automatically. I use YAHOO.util.Event for all my event handlers, it handles this for me automagically. Other libraries (prototype, dojo, etc) have some sort of mechanism to do the same thing, although I'm not sure how effective they are. If you look at the leak images above once again, you will notice almost all of them are in event related code of those libraries.
  Set your XMLHttpRequest onreadystatechange handlers to null when you are done with them.
  I use YAHOO.util.Connect for all my XHR connections and it uses a polling mechanism instead of readstate, so I don't need to do this anymore. If you can switch to YAHOO.util.Connect (or YAHOO.ext.UpdateManager built on top of it), I'd recommend it.


Move a line in a file In Groovy 1.8.6 I need to move a line just before another one. I'm interested in any improvements.

fileToChange = new File('.classpath')
linesToKeep = []
fileToChange.eachLine {
    if (it.contains('line to move')) {
        lineToMove = it
    } else {
        linesToKeep.add(it)
    }
}
fileToChange.withWriter { out ->
    linesToKeep.each {
        if (it.contains('line to recognize')) {
            out.writeLine(lineToMove)
        }
        out.writeLine(it)
    }
}

As you figured out, moving a line requires two passes:

one to determine what  to move and where to move it to,
and another to re-write the file with the lines moved.

With some meta-programming you can add a method to the InputStream class that would allow you to move a line ahead of another using concise code like this:

def source = /line to move/
def dest = /line to recognize/

input.moveLine(source, dest).writeTo(writer)

The meta-programming

You can start with a Groovy Category that adds the method Writable moveLine(Object, Object) to a class, such as an InputStream.

@groovy.lang.Category(InputStream)
class MoveLineCategory {

    Writable moveLine(Object sourceMatcher, Object destinationMatcher) {
        def (sourceLine, lines) = this.readLines().inject([null, []]) {output, line ->
            switch(line) {
                case ~sourceMatcher:
                    if(!output[0]) output[0] = line
                    break
                case ~destinationMatcher:
                    output[1] << null
                    output[1] << line
                    break;
                default:
                    output[1] << line
            }

            return output
        }

        {Writer writer ->
            lines.each {line ->
                if(line == null) writer << "$sourceLine\n"
                else writer << "$line\n"
            }
        }.asWritable()
    }
}

The first pass is similar to your File.eachLine() loop except it's done with Collection.inject(). The inject() method Closure returns a List containing two items: the line which matched the sourceMatcher regular expression, and a List of the entire file with a null inserted as a place holder for the destination of the matched line.

The second pass is actually not executed right way. Instead, a Closure coerced into a Writable is returned for later use. The returned Writable contains the code needed to write out the changed file.

Using InputStream.moveLine()

With the Writable on hand, it's now possible to write the file with the line moved. But to even use the InputStream.moveLine() method the Category in which it's defined needs to be made available. the Object.use(Class) method does this.

def source = /line to move/
def dest = /line to recognize/

use(MoveLineCategory) {
    input.moveLine(source, dest).writeTo(writer)
}

Complete (self-contained) example

Here's the entire thing in action:

@groovy.lang.Category(InputStream)
class MoveLineCategory {

    Writable moveLine(Object sourceMatcher, Object destinationMatcher) {
        def (sourceLine, lines) = this.readLines().inject([null, []]) {output, line ->
            switch(line) {
                case ~sourceMatcher:
                    if(!output[0]) output[0] = line
                    break
                case ~destinationMatcher:
                    output[1] << null
                    output[1] << line
                    break;
                default:
                    output[1] << line
            }

            return output
        }

        {Writer writer ->
            lines.each {line ->
                if(line == null) writer << "$sourceLine\n"
                else writer << "$line\n"
            }
        }.asWritable()
    }
}

def input = ['''This is a line.
And this is another line.
And this is yet another line.
You guessed it, another line.'''] as StringBufferInputStream

def writer = new StringWriter()

use(MoveLineCategory) {
    input
        .moveLine(/And this is another line\./, /You guessed it, another line\./)
        .writeTo(writer)
}

assert writer.toString().split(/\n/) == [
    'This is a line.',
    'And this is yet another line.',
    'And this is another line.',
    'You guessed it, another line.'
]

Disclaimer

These examples were written with Groovy 2.4.4.


Make website full responsive by using javascript Currently I am using javascript to make my website full responsive,
but this is extremly painful...

I do it like this and control the behaviour of all the elements from window width 50px till 2000px. Basically I change classes with removeClass and addClass.

function responsiveDesign()
{
  var wh = $(window).height();
  var ww = $(window).width();

  if (ww <= 50) {

  } else if (ww <= 100) {
    ...
  } else if (ww <= 150) {
    ...
  } else if (ww <= 200) {
    ...
  } else if (ww ....
    ...
  //and so on, until 2000
}

$(window).resize(function() {
    responsiveDesign();
});

responsiveDesign();

I have multiple thousands lines of javascript already...
I know I could also use @media but then I would also need thousands of code lines if I want a perfect responsive website.

e.g.

@media only screen and (max-width: 767px) {

@media only screen and (min-width: 710px) and (max-width: 768px) {

}

Is there a better way, or is it normal that we need so much lines of code for full responsive websites?

No, this is not the "normal" way to build a responsive website. "Normally" people use frameworks that are built specifically to remedy this problem.

Bootstrap, made by the guys who run Twitter, is the most popular responsive framework. Basically it's a set of styles and (opionally) javascript that you include in your sites and then you just add class names to your HTML depending on how you want things arranged. It's really easy to learn and use. Much easier than writing 1000s of lines of JS or CSS.


Playing pure sinusoidal tones I wrote a simple sound player to use as part of my MorseString class. I am new to using sounds (in fact, I was helped a lot by Google) and I don't know if this is the correct way to play a sound.

The method takes a specific frequency and a duration as parameters, and plays the frequency for the specified duration.

private static final int SAMPLING_FREQUENCY_IN_HZ = 8000;
private static final int NUMBER_OF_FADE_SAMPLES = 80;

private static void sound(int hz, int msecs) throws LineUnavailableException {
    byte[] buf = new byte[msecs * Byte.SIZE];
    for (int i = 0; i < buf.length; i++) {
        double angle = i / (SAMPLING_FREQUENCY_IN_HZ / hz) * 2 * Math.PI;
        buf[i] = (byte) (Math.sin(angle) * Byte.MAX_VALUE);
    }
    for (int i = 0; i < NUMBER_OF_FADE_SAMPLES && i < buf.length / 2; i++) {
        buf[i] = (byte) (buf[i] * i / NUMBER_OF_FADE_SAMPLES);
        buf[buf.length - 1 - i] = (byte) (buf[buf.length - 1 - i] * i / NUMBER_OF_FADE_SAMPLES);
    }
    AudioFormat af = new AudioFormat(SAMPLING_FREQUENCY_IN_HZ, Byte.SIZE, 1, true, false);
    SourceDataLine sdl = AudioSystem.getSourceDataLine(af);
    sdl.open(af);
    sdl.start();
    sdl.write(buf, 0, buf.length);
    sdl.drain();
    sdl.close();
}

The sound sounds like it is low-quality. Is there a way to improve that?
Did I do my math wrong?

Please explain in as much detail as possible, as I'm new and might not understand.

In a sense, your math is wrong.  In

double angle = i / (SAMPLING_FREQUENCY_IN_HZ / hz) * 2 * Math.PI;

… both of the divisions are done using integer arithmetic.  As a result, I get nothing more than a clicking noise.  To get the intended signal, you have to cast i to a double.  Better yet, write it instead as

double angle = 2 * Math.PI * i * hz / SAMPLING_FREQUENCY_IN_HZ;

By the Nyquist criterion, a sample rate of 8000 Hz means that you can play a frequency no higher than 4000 Hz.  Therefore, for robustness, you should do one of three things:

Throw an exception if hz >= SAMPLING_FREQUENCY_IN_HZ / 2.
Let the sampling frequency vary, by setting it to 16 * hz, for example.
Set SAMPLING_FREQUENCY_IN_HZ high enough so that it covers the range of human hearing (44.1 kHz for CD audio, 48 kHz for DVD video soundtracks).

It would also be wise to define the fade-in / fade-out period as a multiple of the sampling rate.


Switching to next select field by class of parent This emulates behavior of tab and shifttab with → and ←.

I.e., switch focus to the next/previous select field with parent class .tab (which are not siblings in the DOM.)

It works, but is there any way to make it more elegant? Especially the if/then and the limit to range. I'm fairly new to JavaScript.

$(document).on('keydown', '.tab select', function (e) {
    let tabs = $(".tab select");                        //get all tabs
    let tab = tabs.index($($(document.activeElement))); //focused tab
    if (e.which == 37) {                                //left arrow
        tab -= 1;
    } else {
        if (e.which == 39) {                            //right arrow
            tab += 1;
        }
    };
    tab = Math.min(tabs.length, Math.max(tab, 0));      //stay in range
    tabs.eq(tab).focus()
});

One thing you could consider is making this a little more generic and thus reusable. Currently you seem to assume, that there is only one .tab element in your document. If you had multiple (independent) .tab elements, each with their own set of select elements and you did't want to allow "tabbing" between then, then $(".tab select") will select all selects and not only the ones in the "current" .tab. So instead of

let tabs = $(".tab select");

I'd use

let tabs = $(this).closest(".tab").find("select");

Furthermore instead of hardcoding the selectors, consider making them variable, so that the script could be used with other class names and form elements:

function initArrowKeyTabbing(parentSelector, formElementSelector) {
  $(document).on('keydown', parentSelector + " " + formElementSelector, function (e) {
    $(this).closest(parentSelector).find(formElementSelector);
    // ...
  }
}

initArrowKeyTabbing(".tab", "select");

In the line

let tab = tabs.index($($(document.activeElement)));

you are calling jQuery twice. tabs.index($(document.activeElement)) would suffice. For the matter of the fact, .index() alternatively takes a DOM element as its parameter, so you even only need tabs.index(document.activeElement).

With an if ... else if ... construct you don't need the brackets around the second if:

if (e.which == 37) {
    tab -= 1;
} else if (e.which == 39) {
    tab += 1;
}

In this case instead of using if, a switch statement would be more flexible:

switch (e.which) {
    case 37:
       tab--; // Short form of tab -= 1;
       break;
    case 39:
       tab++;
       break;
}

The which event property is deprecated. The "proper" replacement is the key property, which however requires concessions towards IE and Edge, which use non-standard key names:

switch (e.key) {
  case "Left": // IE/Edge specific value
  case "ArrowLeft":
    tab--;
    break;
  case "Right": // IE/Edge specific value
  case "ArrowRight":
    tab++;
    break;
}


Implementing pthread barrier for Mac OS/X I have written this little thingie to fix a problem of missing pthread_barrier_t in Mac OS/X pthreads. Are there any issues with this code?

The header:

#ifndef PTHREAD_BARRIER_H
#define PTHREAD_BARRIER_H

#include <pthread.h>

#ifdef __APPLE__

#ifdef __cplusplus
extern "C" {
#endif

#if !defined(PTHREAD_BARRIER_SERIAL_THREAD)
# define PTHREAD_BARRIER_SERIAL_THREAD  (1)
#endif

#if !defined(PTHREAD_PROCESS_PRIVATE)
# define PTHREAD_PROCESS_PRIVATE    (42)
#endif
#if !defined(PTHREAD_PROCESS_SHARED)
# define PTHREAD_PROCESS_SHARED     (43)
#endif

typedef struct {
} pthread_barrierattr_t;

typedef struct {
    pthread_mutex_t mutex;
    pthread_cond_t cond;
    unsigned int limit;
    unsigned int count;
    unsigned int phase;
} pthread_barrier_t;

int pthread_barrierattr_init(pthread_barrierattr_t *attr);
int pthread_barrierattr_destroy(pthread_barrierattr_t *attr);

int pthread_barrierattr_getpshared(const pthread_barrierattr_t *restrict attr,
                   int *restrict pshared);
int pthread_barrierattr_setpshared(pthread_barrierattr_t *attr,
                   int pshared);

int pthread_barrier_init(pthread_barrier_t *restrict barrier,
             const pthread_barrierattr_t *restrict attr,
             unsigned int count);
int pthread_barrier_destroy(pthread_barrier_t *barrier);

int pthread_barrier_wait(pthread_barrier_t *barrier);

#ifdef  __cplusplus
}
#endif

#endif /* __APPLE__ */

#endif /* PTHREAD_BARRIER_H */

The source file:

#include "pthread_barrier.h"

#include <errno.h>

#ifdef __APPLE__

#define __unused __attribute__((unused))

int
pthread_barrierattr_init(pthread_barrierattr_t *attr __unused)
{
    return 0;
}

int
pthread_barrierattr_destroy(pthread_barrierattr_t *attr __unused)
{
    return 0;
}

int
pthread_barrierattr_getpshared(const pthread_barrierattr_t *restrict attr __unused,
                   int *restrict pshared)
{
    *pshared = PTHREAD_PROCESS_PRIVATE;
    return 0;
}

int
pthread_barrierattr_setpshared(pthread_barrierattr_t *attr __unused,
                   int pshared)
{
    if (pshared != PTHREAD_PROCESS_PRIVATE) {
        errno = EINVAL;
        return -1;
    }
    return 0;
}

int
pthread_barrier_init(pthread_barrier_t *restrict barrier,
             const pthread_barrierattr_t *restrict attr __unused,
             unsigned count)
{
    if (count == 0) {
        errno = EINVAL;
        return -1;
    }

    if (pthread_mutex_init(&barrier->mutex, 0) < 0) {
        return -1;
    }
    if (pthread_cond_init(&barrier->cond, 0) < 0) {
        int errno_save = errno;
        pthread_mutex_destroy(&barrier->mutex);
        errno = errno_save;
        return -1;
    }

    barrier->limit = count;
    barrier->count = 0;
    barrier->phase = 0;

    return 0;
}

int
pthread_barrier_destroy(pthread_barrier_t *barrier)
{
    pthread_mutex_destroy(&barrier->mutex);
    pthread_cond_destroy(&barrier->cond);
    return 0;
}

int
pthread_barrier_wait(pthread_barrier_t *barrier)
{
    pthread_mutex_lock(&barrier->mutex);
    barrier->count++;
    if (barrier->count >= barrier->limit) {
        barrier->phase++;
        barrier->count = 0;
        pthread_cond_broadcast(&barrier->cond);
        pthread_mutex_unlock(&barrier->mutex);
        return PTHREAD_BARRIER_SERIAL_THREAD;
    } else {
        unsigned phase = barrier->phase;
        do
            pthread_cond_wait(&barrier->cond, &barrier->mutex);
        while (phase == barrier->phase);
        pthread_mutex_unlock(&barrier->mutex);
        return 0;
    }
}

#endif /* __APPLE__ */

No problems found
As they say, "no news is good news".  I read your code all the way through with an eye towards finding any possible errors, but I didn't find anything to criticize.
Instead I'll point out the things I liked about your code.
Use of restrict keyword
I must admit, I don't use restrict but I should learn to, as it can only help the compiler.  I already advocate using const when appropriate, and this is similar.
Proper error checking
You check for errors and set errno appropriately.
Correct barrier operation
I tried to find any concurrency flaw in your barrier wait function but it looked correct.
The one weird thing
The one thing I thought was strange was that you used a do {} while loop without the curly braces:
    do
        pthread_cond_wait(&barrier->cond, &barrier->mutex);
    while (phase == barrier->phase);

I've never seen that before but I guess if you like to do that it's fine.  I think people throw in the curlies because it doesn't waste any lines, whereas with a while loop or an if block it would add an extra line:
    while (condition) {
        foo();
    }                       // <-- Wasted line

    do {
        foo();
    } while (condition);    // <-- No waste