Find primes faster (Java) I'm new here, and relatively new to coding. I wrote a method that returns an array of the n first primes. Even though my intended goal was just to find the nth prime, I ended up using an array (int[] primes) in order to minimize calculations.

My method goes iterates upwards by 2, since even numbers greater than 2 are not primes, and testing if the candidate number is a multiple of the previous primes (of course not including 2) that have been found up until the test.

Actually it does not do that test with all the previous primes, only up to the square root of the last prime found. You can take a look at my method.

public int[] primes(int n){

    int[] primes = new int[n];
    primes[0] = 2;

    int count = 1;
    int number = 3;

    boolean numberIsPrime;

    while (count < n){

        numberIsPrime = true;

        for (int i = 1; i < (int)(Math.sqrt(number)); i++) {

            if (number % primes[i] == 0){
                numberIsPrime = false;
                break;
            }

        }

        if (numberIsPrime){

            primes[count] = number;
            count++;
        }

        number = number + 2;

    }

    return primes;

}

I tested it in a main method through System.out.println(instance.primes(n)[n-1]); for n = 1 million and it took not more than 10 seconds to find the correct answer, and for n = 10 million it took no more than 4 and a half minutes to find the correct answer.

I let it run for n = 100 million, since I started writing this question, and it doesn't look like it is going to finish for an hour or so. I think mathematically I reduced calculations a lot.

Just by decreasing the test to the square root of the last prime did reduce computing time quite much.

But can it be improved, not only mathematically but also by means of code?

Watch for mixed idioms

        for (int i = 1; i < (int)(Math.sqrt(number)); i++) {

This is wasteful.  All you need to do is

        for (int i = 1; primes[i] <= (int)(Math.sqrt(number)); i++) {

This only works in that it always checks too many values.  i and number are not actually related here.  The comparative relation is between primes[i] and number.  This relies on i < primes[i] for all i.

And as noted in a comment, this puts a lot of reliance on the compiler to figure out that the square root will be constant.  Consider

        int largestSmallerFactor = (int)(Math.sqrt(number));
        for (int i = 1; primes[i] <= largestSmallerFactor; i++) {

Now it's clear that we only take the square root once.

Helper method

        if (numberIsPrime){

Consider

        if (isOddPrime(number, primes)) {

Then you just need a helper method and can get rid of numberIsPrime altogether.

private boolean isOddPrime(int candidate, int[] primes) {
    int largestSmallerFactor = (int)(Math.sqrt(candidate));
    for (int i = 1; primes[i] <= largestSmallerFactor; i++) {
        if (candidate % primes[i] == 0) {
            return false;
        }
    }

    return true;
}

Optimization

Perhaps you are concerned that a method call has too much overhead.  If so, consider

    nextCandidate:
    while (count < n) {
        int largestSmallerFactor = (int)(Math.sqrt(number));
        for (int i = 1; primes[i] <= largestSmallerFactor; i++) {
            if (number % primes[i] == 0) {
                continue nextCandidate;
            }
        }

        primes[count] = number;
        count++;
    }

No method overhead.

People will point out that the method is a more readable and robust solution, which it is.  But if you're trying to wring out every possible efficiency, this might be better.  You'd have to compile and compare both versions to see.  Remember that the compiler might have an even better optimization for the method version.

Sieve of Eratosthenes

A comment suggested using the Sieve of Eratosthenes.  While it is true that that's an efficient way to find primes, it is not well suited to finding the nth prime.  The problem is that it checks a certain number of numbers, so there is a tendency to either overshoot or undershoot.

You can use a segmented sieve to process the sieve partially.  So if you undershoot, you can continue.  This also allows you to make smaller jumps, so you are less likely to overshoot.

It was asserted that a sieve avoids repeated division.  That's true but it replaces it with redundant multiplication.  For example, 210 is marked as not prime by the sieving operations for 2, 3, 5, and 7.  Or more generally, a number divisible by 6 will be sieved by both 2 and 3.  By contrast, trial division can stop as soon as it finds the first factor.

It may be that the sieve is faster on average.  After all, a square root is an expensive operation.  But it can do more work for a particular number than trial division does.

Here is an example of a windowed sieve which you could try to modify to this purpose.  Don't forget to time it both ways.


Re-implementing memcpy I have written an implementation of memcpy function and I want to know how good is it and if it gets the most out of the processor capabilities or not.

The reason for this is because I am writing my operating system (which will be of course  reviewed on parts here on Code Review) it is supposed to work efficiently on both x86_64 and x86 machines

#include <stddef.h>
#include <stdint.h>
void *memcpy(void *dest, const void *src,size_t n)
{
        if (n==0)
                return dest;
        #if defined(__x86_64__) || defined(_M_X64)
        size_t i=0;
        if(n>=8)
                while(i<n/8*8){
                        *(uint64_t*)(dest + i) = *(uint64_t*)(src+i);
                        i+=8;
                }
        if(n-i>=4){
                *(uint32_t*)(dest + i) = *(uint32_t*)(src+i);
                i+=4;
        }
        if(n-i>=2){
                *(uint16_t*)(dest+i) = *(uint16_t*)(src+i);
                i+=2;
        }
        if(n-i>=1)
                *(uint8_t*)(dest+i) = *(uint8_t*)(src+i);

        #elif defined(__i386) || defined(_M_IX86)
        size_t i=0;
        if(n>=4)
                while(i<n/4*4){
                        *(uint32_t*)(dest + i) = *(uint32_t*)(src+i);
                        i+=4;
                }
        if(n-i>=2){
                *(uint16_t*)(dest+i) = *(uint16_t*)(src+i);
                i+=2;
        }
        if(n-i>=1)
                *(uint8_t*)(dest+i) = *(uint8_t*)(src+i);

        #endif
        return dest;
}

I made some aggressive testing with memcmp() to check for correct data transmission and valgrind to check for memory leaks and It passed all the tests. I didn't post the testing code because I think it could be useless since I don't want it to be reviewed.

Not very fast
Your memcpy() implementation is not really better than a standard byte by byte copy.  Even though you attempt to copy more bytes at a time, the limiting factor isn't actually the number of bytes you copy per instruction.
If you research the various memcpy() implementations there are for x86 targets, you will find a wealth of information about how to get faster speeds.  I think the simplest thing for you to do is to just use the simple "rep movsb" implementation.
My own benchmarks
I ran your version against the following two versions.  One is a straightforward byte by byte copy, and the other is just using "rep movsb", which on modern processors is highly optimized:
void *memcpy2(void *dst, const void *src,size_t n)
{
    size_t i;

    for (i=0;i<n;i++)
        *(char *) dst++ = *(char *) src++;
    return dst;
}

void *memcpy3(void *dst, const void *src, size_t n)
{
    void *ret = dst;
    asm volatile("rep movsb" : "+D" (dst) : "c"(n), "S"(src) : "cc", "memory");
    return ret;
}

My results for copying 2GB (32-bit host):
OP's function  : 3.74 sec
memcpy2 (naive): 3.74 sec
memcpy3 (movsb): 2.96 sec


Dice Roll Program in C I've made a dice game that rolls three die, calculates the sum of their results, and asks the user whether he/she thinks the next roll's total will be higher, the same, or lower than the current roll. It works but I want to know if there is a better/more efficient way to do this. I just started C programming a couple days ago.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int main()
{
    int i;
    int runTime;
    int die1;
    int die2;
    int die3;
    int temp;
    int total;
    char guess;
    int count = 0;
    srand(time(NULL));

    printf("How many times do you want to play?\n");
    scanf(" %d", &runTime);
    printf("");

    die1 = ( (rand() % 6) + 1);
    die2 = ( (rand() % 6) + 1);
    die3 = ( (rand() % 6) + 1);
    total = (die1 + die2 + die3);

    if(runTime != 0){

        printf("Die 1: %d\n", die1);
        printf("Die 2: %d\n", die2);
        printf("Die 3: %d\n", die3);
        printf("-------------------\n");
        printf("Total: %d\n\n", total);
        temp = total;

        printf("Do you think the next total will be higher, the same, or lower than the previous total? (h, s, l) \n\n");
        scanf(" %c", &guess);

        for(i = 0; i < runTime - 1; i++){
            die1 = ( (rand() % 6) + 1);
            die2 = ( (rand() % 6) + 1);
            die3 = ( (rand() % 6) + 1);
            total = (die1 + die2 + die3);

            printf("Die 1: %d\n", die1);
            printf("Die 2: %d\n", die2);
            printf("Die 3: %d\n", die3);
            printf("-------------------");
            printf("Total: %d\n", total);

            if( (total > temp) && (guess == 'h') ){
                printf("You guessed correctly!\n");
                count++;
            }else if( (total == temp) && (guess == 's') ){
                printf("You guessed correctly!\n");
                count++;
            }else if( (total < temp) && (guess == 'l') ){
                printf("You guessed correctly!\n");
                count++;
            }else{
                printf("You guessed incorrectly :(\n");
            }
            temp = total;
            printf("Do you think the next total will be higher, the same, or lower than the previous total? (h, s, l) \n");
            scanf(" %c", &guess);

        }
    printf("You got a total of %d guesses correct!", count);
     }else{
    printf("Goodbye.");
    }
    return 0;
    }

I wouldn't recommend C or C++ as a starter language.
Try to reimplement the game in Python or Java or some other high level language.

You can use a loop instead of repeating yourself with die 1 die 2 and die 3.
Use a function for rolling and reporting.
The main loop can be a for loop that scanf()s the number of guesses and counts down.
A switch statement on the guess token could come in handy.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define N_DICE 3

/* Conveniently return the sum. */
int roll_dice(int *dice)
{
    int i, sum;
    for (i = sum = 0; i < N_DICE; i++)
    {
        dice[i] = rand()%6 + 1;
        sum += dice[i];
    }
    return sum;
}

void print_dice(int *dice, int sum)
{
    int i;
    printf("\n\n");
    for (i = 0; i < N_DICE; i++)
        printf("Die %d: %d\n", i + 1, dice[i]);
    printf("-------------------\nTotal: %d\n\n", sum);
}

int main()
{
    int runs, dice[N_DICE], sum, old_sum, correct, correct_guesses;
    char guess;

    srand(time(NULL));
    correct_guesses = 0;
    sum = roll_dice(dice);

    printf("How many times do you want to play? ");
    for (scanf("%d", &runs); runs; runs--)
    {
        print_dice(dice, sum);
        old_sum = sum;
        sum = roll_dice(dice);

        printf(
            "Do you think the next total will be higher, "
            "the same, or lower than the previous total? "
            "(h, s, l)\n"
        );
        scanf(" %c", &guess);
        switch (guess)
        {
            case 'h': correct = sum > old_sum; break;
            case 's': correct = sum == old_sum; break;
            case 'l': correct = sum < old_sum; break;
            default: correct = 0; printf("Not h, s or l.\n");
        }
        if (correct)
        {
            printf("You guessed correctly!\n");
            correct_guesses++;
        }
        else
            printf("You guessed incorrectly!\n");
    }
    printf("You got a total of %d guesses correct!\n", correct_guesses);
}


Adding rows to LinearLayouts which serve as lists I have a fragment with a ScrollView and, inside of it, I need maybe 5 or 6 lists. To make things simpler, let's say there are three lists - list of names, list of places and list of events.

The problem is, I can't use lists inside of ScrollViews so I need to use LinearLayouts and do the adding to the list thing by myself. This causes me all sorts of problems.

I'm using index variables to keep track of where in my layout to put the new row when the user clicks "Add".

int currIndNames;
int currIndPlaces;
int currIndEvents;

Now this is my onCreate function:

public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    if (savedInstanceState != null){
        names = savedInstanceState.getStringArrayList("names");
        places = savedInstanceState.getStringArrayList("places");
        events = savedInstanceState.getStringArrayList("events");
    } else {
        if (names == null){
            names = new ArrayList<String>();
        };
        if (places == null){
            places = new ArrayList<String>();
        }
        if (events == null){
            events = new ArrayList<String>();
        }
    }

    realNames = new LinkedHashSet<>(names);
    realPlaces = new LinkedHashSet<>(places);
    realEvents = new LinkedHashSet<>(events);
}

The "real" variables are there because I didn't find a way to put sets into savedInstanceState, so I keep each group of items both in a set and in a list.

Now, in my onViewCreated method, I'm calling prepareNames, preparePlaces and prepareEvents. The names are the first list in the layout, so the function looks something like this:

private void prepareNames(){

  edtAddName = (EditText)getActivity().findViewById(R.id.edtName);

  LinearLayout linearLayoutNames = (LinearLayout)getActivity().
            findViewById(R.id.linearLayoutNamesTitle);

    currIndNames = ((ViewGroup)linearLayout).indexOfChild(linearLayoutNames)+1;

    for (String name: names){
        addNamesRow(name, currIndNames++);
        currIndPlaces++;
        currIndEvents++;
    }

    prepareToggleButtonNames();
    prepareButtonAddName();
    prepareButtonNames();

}

Now, as you see, after adding the names row, I also have to increase the indexes for places and events, because they're further down in the linear layout.

Furthermore, users can remove rows, and I have to do something similar when removing rows. This is my addNamesRow function:

private void addNamesRow(final String text, int position){
    final LinearLayout rowLayout = (LinearLayout)LinearLayout.inflate(
            getActivity().getBaseContext(), R.layout.item,null);

    rowLayout.setTag(NAMES_TAG);

    TextView tvItem = (TextView)rowLayout.findViewById(R.id.tv_item);
    tvItem.setText(text);

    Spinner spItem = (Spinner)rowLayout.findViewById(R.id.sp_item);
    spItem.setAdapter(ArrayAdapter.createFromResource(getActivity().getBaseContext(),
            R.array.ratings,android.R.layout.simple_spinner_item));
    spItem.setSelection(2);

    Button btnItem = (Button)rowLayout.findViewById(R.id.btn_item);
    btnItem.setText("X");
    btnItem.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            currIndNames--;
            currIndPlaces--;
            currIndEvents--;
            linearLayout.removeView(rowLayout);
            realNames.remove(text);
        }
    });

    linearLayout.addView(rowLayout, position);

    linearLayout.invalidate();
    edtAddName.setText("");
}

My code works but I have a lot of duplication and it's very messy. I have some ideas on how to avoid some of the repetition but I'm new to Android development and I feel like I have some deeper issues in my code, and I would like to do things the right way.

Your problems in code seem to be a manifestation of problematic design. If I were you I would think about improving my design first.
Currently you're displaying a list of lists of variable lengths to your user. From a UX point of view that seems like a bad decision.
There's a reason why you can't use lists in a ScrollView ;)
Have you considered for example splitting up the lists to different fragments and putting them in a view pager so you have a separate tab for each list? Then you can use ListView or, probably even better, RecyclerView to display your items.
Again a lot of times when you have to write awkward, messy code to implement your design it is worth revisiting your design and improving it. Usually that clears up convoluted code as well.


Lottery number generator def getRandomNumbers(count, min, max)
  array = []

    while array.length < count
      tmp = rand(max) + min
      if array.include?(tmp)
        next
      else
        array.push(tmp)
      end
    end

  return array
end

def prettyPrint(someArray)
  string = ""
  someArray.each do |item|
    string += item.to_s + "\t"
  end
  puts string
end

array = getRandomNumbers(6, 1, 49)
prettyPrint(array)

I'm just learning Ruby and wrote this little two functions just to play around and get a feel for Ruby syntax etc.
I wanted a function that generates a given amount of random numbers within certain limits. In this example for 6 lottery numbers, therefore each number should only occur once. The second method is pretty self explanatory.

The code runs and does what he is supposed to do. However, what are nice tweaks and tricks to make this code nicer and utilizing the abilities of Ruby?

A min-max pair would be slightly better represented using a Range.  In fact, starting with Ruby 1.9, rand can accept a range.

The loop can be written a bit more succinctly.

The pretty-printing function should also be written in one line instead.

lower_case is a more conventional naming convention.  An explicit return is not necessary, as the code returns the last value anyway.

def get_random_numbers(range, count=1)
  array = []
  while array.length < count
    n = rand(range)
    array.push(n) unless array.include?(n)
  end
  array
end

def pretty_print(array)
  puts array.join("\t")
end

pretty_print(get_random_numbers(1..49, 6)


Translating month number into month name Time is very important from times immemorial. Doing great things and making it happen in less time is more important as it can mean less effort. So I really like to know which way is faster and more efficient in the following codes, and I would happy to know any other way we can achieve the same code:

$month=date('m');
//Using If..else
 if($month==1){
    echo "January";
}elseif($month==2){
    echo "February";
}elseif($month==3){
    echo "March";
}elseif($month==4){
    echo "April";
}elseif($month==5){
    echo "May";
}elseif($month==6){
    echo "June";
}elseif($month==7){
    echo "July";
}elseif($month==8){
    echo "August";
}elseif($month==9){
    echo "September";
}elseif($month==10){
    echo "October";
}elseif($month==11){
    echo "November";
}else{
    echo "December";
}

//Using Switch
switch($month){
case 1:
    echo "January";
break;
case 2:
    echo "February";
break;
case 3:
    echo "March";
break;
case 4:
    echo "April";
break;
case 5:
    echo "May";
break;
case 6:
    echo "June";
break;
case 7:
    echo "July";
break;
case 8:
    echo "August";
break;
case 9:
    echo "September";
break;
case 10:
    echo "October";
break;
case 11:
    echo "November";
break;
default:
    echo "December";
break;
}

//Using Array
$month_s=array(1=>"January",2=>"February",3=>"March",4=>"April",5=>"May",6=>"June",
7=>"July",8=>"August",9=>"September",10=>"October",11=>"November",12=>"December");
echo isset($month_s[$month]) ? $month_s[$month] : '';

Like I said in my comment: If you want to know which is faster, write a benchmarking script, it's that easy:

$start = microtime(true);
//if-hell approach
$ifs = microtime(true) - $start;
$start = microtime(true);
//switch
$switch = microtime(true) - $start;
$start = microtime(true);
//array
$array = microtime(true) - $start;

Then compare and echo the results in any way you like.
My gut tells me that in this case, the switch will probably be the faster option. Considering a switch construction is sort of like labeling blocks of code, and then do some sort of eval-go-to thingy.
Branching (if) is faster in terms of compilation, perhaps, but for the month of december, you'll end up performing all if statements.
The array approach, for this example, doesn't look like the fastest approach you can take in this case, because it's constructing an array, performing a HashTable lookup, evaluate the result of that lookup, then perform the same lookup again, or return an empty string. HashTable lookups are as fast as it gets, of course, but still, you're having to do 2 of them, just after you've constructed the HashTable. The other approaches don't construct to then perform a lookup.

Mind you, This is micro-optimization of the worst kind. It's completely pointless and the speed difference won't be noticeable. Especially if you use some form of OP-Code caching (APC, for example). That should be the first port of call if you're trying to optimize some code.

What code should you use?:
This is the better question to ask. The efficiency of some snippet of code isn't just determined by the time it takes to execute. Efficient code is good code (is stable, performant, error and notice-free) and is easy to read, update and maintain.
If your determining the name of the month then it's pretty obvious the data-set is limited to 12 valid strings. But in most cases, you might end up with an ever growing switch or if-elseif-else tree. That's not efficient coding. The moment you have to scroll to see a single block of code, you can safely say it's time to think about refactoring your code.

That's why I'd either use the array approach (works just as well when data is coming from a query, so you can create a function that you can reuse). Or, if you are working on dates, I'd give way to normalization, and use the DateTime class. when I see new DateTime or $d = DateTime::createFromFormat, I know what to exepct. Even more so when I see:

[public] function doStuff(DateTime $date)
{
}


Convert a formatted file size to bytes I needed to convert a formatted file size back into its length in bytes. I came up with the following code but want it reviewed:

/**
 * Converts a formatted size to its equivalent in bytes.
 *
 * @param size
 *            The formatted file size. Examples: "125.5 MB", "1KiB", "0.8 GB"
 * @return
 * @throws NumberFormatException
 */
public static long getBytesFromFormattedSize(final String size) throws NumberFormatException {
    final String[] arr = size.toUpperCase().split(" ");
    if (arr.length != 2) {
        if (arr.length == 1 && Character.isDigit(size.charAt(0))) {
            // Treat this as a formatted size missing the space.
            return getBytesFromSize(size);
        }
        throw new IllegalArgumentException("Expected '<size> <unit>', got '" + size + "'");
    }
    final char unit = arr[1].charAt(0);
    if (unit == 'B') {
        return Integer.parseInt(arr[0]);
    }
    final int bytes = arr[1].length() == 3 && arr[1].charAt(1) == 'I' ? 1000 : 1024;
    final char[] units = {
            'K', 'M', 'G', 'T', 'P', 'E'
    };
    int exp = 1;
    for (final char ch : units) {
        if (unit == ch) {
            break;
        }
        exp++;
    }
    return (long) (Double.parseDouble(arr[0]) * Math.pow(bytes, exp));
}

private static long getBytesFromSize(final String size) {
    String fixedSize = "";
    for (final char c : size.toCharArray()) {
        if (Character.isLetter(c) && !fixedSize.contains(" ")) {
            fixedSize += ' ';
        }
        fixedSize += c;
    }
    return getBytesFromFormattedSize(fixedSize);
}

Some tests:

String[] sizes = {
        "1 KB", "100 MB", "100MiB", "1 GB", "100B", "1.5 MB"
};
for (String size : sizes) {
    System.out.println(String.format("%s = %d bytes", size, getBytesFromFormattedSize(size)));
}

Output:

1 KB = 1024 bytes
100 MB = 104857600 bytes
100MiB = 100000000 bytes
1 GB = 1073741824 bytes
100B = 100 bytes
1.5 MB = 1572864 bytes

The function throws NumberFormatException and IllegalArgumentException.
Your implementation caters to your edge cases, take these for example:

1Kb (Do you wish to handle bits). This gets converted to 1KB (unwantedly).
1M iB (Incorrect position of space)
1 (No units).

Not sure if the input is manual or system feeded.
Why not have a function that validates the input and if it can be tailored (1MiB), it returns a operable string.
A Map can be used instead of the units' char array, for loop and Math.pow for each input.
getBytesFromFormattedSize() calls getBytesFromSize() which again calls getBytesFromFormattedSize(). Instead a flow as this would look clean.

 getBytesFromFormattedSize(){
     //Get a valid string (if possible) (in two forms value and unit)
     // Proceed thereof.
 }

fixedSize.contains()  has an O(n) time complexity. Instead a boolean would easily suffice the otherwise redundant call once we insert a space.
Names such as arr, size, bytes are ignorant to their puprose.
Too much of hard coded values to actually understand the flow.
Should I return 1000? http://en.wikipedia.org/wiki/Mebibyte
return Integer.parseInt(arr[0]); assumes that the value would not be greater than INT_MAX? Does the input guarantee that?
If size is an empty string, charAt(0) could give exception IndexOutOfBoundsException().


Python parallelization using Popen I frequently run a script similar to the one below to analyze an arbitrary number of files in parallel on a computer with 8 cores.

I use Popen to control each thread, but sometimes run into problems when there is much stdout or stderr, as the buffer fills up. I solve this by frequently reading from the streams. I also print the streams from one of the threads to help me follow the progress of the analysis.

I'm curious on alternative methods to thread using Python, and general comments about the implementation, which, as always, has room for improvement. Thanks!

import os, sys
import time
import subprocess

def parallelize(analysis_program_path, filenames, N_CORES):
    '''
    Function that parallelizes an analysis on a list of files on N_CORES number of cores
    '''
    running = []
    sys.stderr.write('Starting analyses\n')
    while filenames or running:
        while filenames and len(running) < N_CORES:
            # Submit new analysis
            filename = filenames.pop(0)
            cmd = '%s %s' % (analysis_program_path, filename)
            p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
            sys.stderr.write('Analyzing %s\n' % filename)
            running.append((cmd, p))
        i = 0
        while i < len(running):
            (cmd, p) = running[i]
            returncode = p.poll()
            st_out = p.stdout.read()
            st_err = p.stderr.read()  # Read the buffer! Otherwise it fills up and blocks the script
            if i == 0:  # Just print one of the processes
                sys.stderr.write(st_err)
            if returncode is not None:
                st_out = p.stdout.read()
                st_err = p.stderr.read()
                sys.stderr.write(st_err)
                running.remove((cmd, p))
            else:
                i += 1
        time.sleep(1)
    sys.stderr.write('Completely done!\n')

Python has what you want built into the standard library: see the multiprocessing module, and in particular the map method of the Pool class.

So you can implement what you want in one line, perhaps like this:

from multiprocessing import Pool

def parallelize(analysis, filenames, processes):
    '''
    Call `analysis` for each file in the sequence `filenames`, using
    up to `processes` parallel processes. Wait for them all to complete
    and then return a list of results.
    '''
    return Pool(processes).map(analysis, filenames, chunksize = 1)


Android music-streaming app This is my first time building an Android app or even using Java in the first place. All of this code does work, but I'm hoping to optimize it or find out if things could be done more easily than how I've done them.

radio.java

package com.example.jacob.wutk;

import android.media.AudioManager;
import android.media.MediaPlayer;
import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.view.View;
import android.widget.ImageButton;

public class radio extends AppCompatActivity {

    private MediaPlayer mediaPlayer;
    private boolean isMediaPlayerStarted = false;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_radio);
        setMediaPlayer();
    }

    public void setMediaPlayer() {
        try {
            mediaPlayer = new MediaPlayer();
            String url = "http://streamer.cci.utk.edu:8000/wutk-vorbis"; // your URL here

            final ImageButton imb = (ImageButton) findViewById(R.id.playPause);

            imb.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                    if (!isMediaPlayerStarted) {
                        mediaPlayer.prepareAsync();
                        isMediaPlayerStarted = true;
                    } else {
                        if (mediaPlayer.isPlaying()) {
                            imb.setImageResource(R.drawable.play1);
                            mediaPlayer.pause();
                        } else {
                            imb.setImageResource(R.drawable.pause1);
                            mediaPlayer.start();
                        }
                    }
                }
            });

            mediaPlayer.setOnPreparedListener(new MediaPlayer.OnPreparedListener() {
                public void onPrepared(MediaPlayer mediaPlayer){
                    mediaPlayer.start();
                }
            });

            mediaPlayer.setAudioStreamType(AudioManager.STREAM_MUSIC);
            mediaPlayer.setDataSource(url);
        } catch (Exception e) {
            e.printStackTrace();
        }

    }}

activity_radio.xml

<?xml version="1.0" encoding="utf-8"?>
<FrameLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:layout_alignParentTop="true"
    android:layout_centerHorizontal="true"
    tools:context="com.example.jacob.wutk.radio">
    <ImageView
        android:id="@+id/imageView"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:layout_gravity="left|center_vertical"
        android:scaleType="centerCrop"
        android:src="@drawable/background_mic1"/>

    <LinearLayout
        android:layout_width="fill_parent"
        android:layout_height="wrap_content"
        android:orientation="horizontal"
        android:paddingBottom="1.0dip"
        android:paddingLeft="4.0dip"
        android:paddingRight="4.0dip"
        android:paddingTop="5.0dip">
       <ImageButton
           android:id="@+id/playPause"
           android:layout_width="0.0dip"
           android:layout_height="wrap_content"
           android:layout_weight="1.0"
           android:background="?android:selectableItemBackground"
           android:clickable="true"
           android:scaleType="fitCenter"
           android:onClick="playPauseMusic"
           android:src="@drawable/play1"/>
       <ImageView
           android:layout_width="0.0dip"
           android:layout_height="fill_parent"
           android:layout_marginRight="5dp"
           android:layout_weight="1.0"
           android:background="?android:selectableItemBackground"
           android:scaleType="fitCenter"
           android:src="@drawable/logo"/>

    </LinearLayout>

</FrameLayout>

Debug version of app

There's really not a lot going on in what you've posted, so there's not much to pick apart.  Really the only functional change I'd mention is that there's a MediaController class available that you can use that provides play, pause, and seek functionality (including a standard UI that Android users are familiar with) that you might want to check out.  That said, it's 100% find to use your own UI if that's a design decision and not just the result of being unaware.  Also, you don't update the play/pause UI the first time the `MediaPlayer is started.  Other than that, it's mostly code style and convention issues that I see.

Classes are usually named using PascalCase (you have lowercase).  Also, convention is to use the word "Activity" when extending an Activity, so your radio class should probably be RadioActivity.
The prefix set is generally reserved for setter methods - methods that update a variable value and often do some pre- or post-processing work around that update.  I would probably call setMediaPlayer something like initializeMediaPlayer or startMediaPlayer.
You could break setMediaPlayer into multiple smaller methods.
You probably don't want that whole method body in the try block - figure out what's going to potentially throw and move the rest out.  This is a bit of a nitpick and isn't critical for a fairly small function like this, but it's a good habit to get into.
imb is a bad variable name - it's not clear what kind of object is being referenced.
Your code might be a little easier to read with less nesting.  I'd probably move the functionality in the onClick to its own method, and call that method in the onClick.
Why is the imb variable final and not a member variable?  It's OK to do this, but there doesn't seem to be clear reason - you're using member variables elsewhere.
Again, not a critical issue, but Android convention is to prefix private variables with m, so mediaPlayer would become mMediaPlayer.
Why is setMediaPlayer public?  Always be as restrictive as possible.
isMediaPlayerStarted will default to false so you don't need to set it.
If an error is thrown, do you really want to print the stack trace?  This might be fine if that's what you want, but I suspect that's either the IDE's decision or the result of a copy/paste...  What error might be thrown here?  What could be a possible source?  What do you really want to do if that happens?
If you're not using any ImageButton methods, you don't need to cast it - findViewById returns a View, which has a setOnClickListener method, so the cast doesn't do anything.


Shortening JPA criteria query boilerplate I've used hibernate for a long time. Recently I started using JPA, but I can't find a short way to write a simple select in less than these seven lines (the use of criteria is a must in this project).

Is there a shorter way to build this query?

public List<Transaction> findDevolucionesByOriginalOperationId(String originalOperationId) {
    CriteriaBuilder cb = em.getCriteriaBuilder();
    CriteriaQuery<Transaction> cq = cb.createQuery(Transaction.class);
    Root<Transaction> root = cq.from(Transaction.class);
    Collection<Predicate>  predicates = new ArrayList<Predicate>();

    predicates.add(cb.equal(root.get("originalOperationId"), originalOperationId));

    cq.where(predicates.toArray(new Predicate[predicates.size()]));
    List<Transaction> resultado = em.createQuery(cq).getResultList();
    return resultado;
}

JPA Criteria API is horrific, even the creators admitted that it was designed for tooling support rather than for developers.
My personal favorite approach is to use a fluent query API like QueryDSL.

The query in QueryDSL would look more or less as follows:

new JPAQuery().from(transaction)
    .where(transaction.originalOperationId.eq(originalOperationId))
   .list();

The difference is staggering. And the query is fully typesafe.


Extracting filenames from a URL by splitting a string I am used to accessing items from an array, returned by the Split function, directly. This is lazy, I know. Usually, I know the element I want and I say something like:

Debug.Print Split(href, "/")(0)

Questions:

1) Can I do something similar to access the UBound of the array returned?

2) What is the "best practice" way of doing things and why?

Code:

I wrote the following but it looks messy.

Sub Testing()

    Dim href As String
    Dim fileName As String

    href = "https://www.england.nhs.uk/statistics/wp-content/uploads/sites/2/2018/01/20180111-AmbSYS-post-ARP-month-of-December-2017v2.xlsx"

    fileName = Trim$(Split(href, "/")(UBound(Split(href, "/"))))

End Sub

I saw from here that I can also do:

Debug.Print Split(href, "/")(Len(href) - Len(Replace(href, "/", "")))

Again, messy.

I know that I can assign to an array variable and then access the UBound that way. It looks tidier but is essentially the same thing; i.e.

Dim myArr() As String

myArr() = Split(href, "/")
fileName = myArr(UBound(myArr))

Abstract the implementation into a function. This makes it easy to catch mistakes if you want to use this capability in more than one location. GetUpperBoundElemenentFromDelimitedString takes the required arguments you give it and returns the UBound result. The name may be cumbersome but typing getup and then pressing Ctrl+J will 'List Properties/Methods', the same going to Edit and choosing that option.

Public Function GetUpperBoundElemenentFromDelimitedString(ByVal inputValue As String, ByVal delimiter As String, Optional ByVal compare As VbCompareMethod = VbCompareMethod.vbTextCompare) As String
    Dim temp As Variant
    temp = Split(inputValue, delimiter, compare:=compare)
    GetUpperBoundElemenentFromDelimitedString = temp(UBound(temp))
End Function

Now when you are using it the descriptive name lets you immediately know what's happening.

Public Sub Foo()
    Dim bar As String
    bar = "This,is,going,to,return,an,element,at,a,specific,position."
    Debug.Print GetUpperBoundElemenentFromDelimitedString(bar, ",")
End Sub

The same could be done for a specific position

Public Function GetArrayElemenentFromDelimitedString(ByVal inputValue As String, ByVal delimiter As String, ByVal zeroBasedPosition As Long, Optional ByVal compare As VbCompareMethod = VbCompareMethod.vbTextCompare) As String
    GetArrayElemenentFromDelimitedString = Split(inputValue, delimiter, compare:=compare)(zeroBasedPosition)
End Function


Is calling a function only twice a good use of recursion? Is this, technically, a recursive function? It only gets called once by itself, but it cleaned up my code quite a bit, I think.  Or, does it needlessly complicate the code?

sub check_backlog {
    my $topic        = shift;
    my $prev_backlog = shift;
    my $backlog      = undef;
    my @status = `$CMD -topicstat $topic`;
    foreach (@status) {
        if (/Message count: (\d+)/) {
            $backlog = $LAST_PAREN_MATCH;
            if ( !$prev_backlog && $backlog > 1 ) {
                sleep $topic_wait_time;
                check_backlog( $topic, $backlog );
            }
            elsif ( $backlog > $prev_backlog ) {
                check_over_thresholds($backlog);
            }
        }
    }
    return $backlog;
}

Yes, every function which calls itself is considered a recursive function.

Recursion is really useful when traversing trees and such.

As I don't fully understand Perl, I can't really tell whether this case is proper use of recursion, perhaps also provide the intent of your code.

UPDATE

If you simply want to see whether data is passing (or not passing) through a queue, I wouldn't find recursion to be a suitable implemention.

I would expect a function which returns either true or false after a given amount of time, or immediately when data is passing through.

Consider the following pseudo code:

function isDataPassing( queue, time )
{
   headOfQueue = queue.peek();
   dataPassing = false;
   while ( timePassed < time )
   {
       if ( headOfQueue != queue.peek() )
       {
           dataPassing = true;
           break;
       }
   }

   return dataPassing;
}

I can't help you with how you would go about implementing this in Perl. StackOverflow is a better location to ask such a question.


Optimize “layering” in jQuery I am building an web application that is going to work in a similar way like photoshop or illustrator regarding layers. Here are my code that is searching the ".case" div for images, and then put these images id's to an array. When that is done we are looping the array to find out if the received image is the same as the one we loop through from the array.

When finding this image we wan't to move it one step up or down in the hierarchy. But this image have a div with controllers associated, so we have to move this in the same way.

So do you think this is the best possible code we can create to get this work? It does work, I only want to find out if there are any better solutions for it.

var arrangeLayers = function(e,direction) {

    var timestampId = e.attr('id');
        timestampId = timestampId.replace('name', '');

    var elem = [];
    $('.case img').each(function(){
        elem.push($(this).attr('id'));
    });

    for (i=0;i<elem.length;i++) {
        if (timestampId == elem[i] && direction == 'down') {
            if (i != 0) {
                $('#' + timestampId).insertBefore($('#' + elem[(i-1)]));
                $('#name' + timestampId).insertBefore($('#' + timestampId));
            }
        } else if (timestampId == elem[i] && direction == 'up') {
            if (i != (elem.length) -1) {
                $('#' + timestampId).insertAfter($('#' + elem[(i+1)]));
                $('#name' + timestampId).insertBefore($('#' + timestampId));
            }
        }
    }
}

Not if I understand what you're doing correctly.

So essentially each image has itself, and then some other element that should be before it as a buddy? Assuming they are all siblings, you should utilize some of jQuery's functions that support working with siblings.

Here's an example, though I'm sure it could still be better:

var arrangeLayers = function (e, direction) {
    var $target = $(e);
    var $targetBuddy = $target.prev();
    if (direction === 'down') {
        if($targetBuddy.index() > 1) {
            $target.insertBefore($targetBuddy.prev().prev());
        }
    } else {
        if ($target.index() !=$target.siblings().length) {
            $target.insertAfter($target.next().next());
        }
    }
    $targetBuddy.insertBefore($target);
}

Or if you don't care about index error checking (which in actual practice appears to not matter):

var arrangeLayers = function (e, direction) {
    var $target = $(e);
    var $targetBuddy = $target.prev();
    if (direction === 'down') {
        $target.insertBefore($targetBuddy.prev().prev());
    } else {
        $target.insertAfter($target.next().next());
    }
    $targetBuddy.insertBefore($target);
}

Here's a jsfiddle without the checking:


VBA Copy and Delete Rows loop optimisation I'm trying to identify which cells in Column C in a spreadsheet of data are dates and if they are, then check if that date is older than 2 years old. If the second condition is met then the below code should copy the entire row from sheet "DUP_ALL" to a new sheet (Archive) and then delete the copied row from the "DUP_ALL" sheet.

I believe it works but I was wondering if there are any better ways to optimise my code.

Is it more efficient to stay in the first loop and copy and delete the rows at the same time or is it better to copy all the rows, then loop through a second time deleting all the rows that meet the same criteria.

I tried doing a copy and then delete in the same for loop to begin with but it kept producing an error "Run Time 1004: Method 'Range' of Object '_worksheet' failed"

Any help with optimising the below is greatly appreciated

Sub TestArchive()
Dim sh As Worksheet, lr As Long, rng As Range, sh2 As Worksheet, lr2 As Long, c As Range
Set sh = Sheets("DUP_ALL") 'Edit sheet name
Set sh2 = Sheets("Archive") 'Edit Sheet name
lr = sh.Cells(Rows.Count, 1).End(xlUp).Row
Set rng = sh.Range("C2:C" & lr)

For Each c In rng
    If IsDate(c.Value) Then
        If c.Value < Date - 456 Then
            lr2 = sh2.Cells(Rows.Count, 1).End(xlUp).Row + 1
            c.EntireRow.Copy sh2.Range("A" & lr2)
            End If
        End If
    Next
For Each c In rng
    If IsDate(c.Value) Then
        If c.Value < Date - 456 Then
            lr1 = sh.Cells(Rows.Count, 1).End(xlUp).Row + 1
            c.EntireRow.Delete sh.Range("A" & lr1)
            End If
        End If
    Next
End Sub

I've found that if you are deleting rows in a range you need to create a for loop that starts at the bottom and works your way up.  When you are using the for each loop and try deleting an item out of that range and keep looping it destroys the reference to that range.  The next iteration will throw an error.

Here's an example, (I'm just pulling this out of my head and haven't tested it, but it should get you in the right direction)

...
dim sh1Rows as integer
dim myCell as range

sh1Rows = sh.["A1"].end(xlDown).row

for i = sh1Rows to 1 step -1
    set myCell = sh["A1"].offset(0,i)
    if IsDate(myCell.value) and myCell.value < Date - 456 Then
        sh2.cells(1,sh2.["A1"].end(xlDown) +1).entirerow = myCell.entirerow.value
        myCell.entirerow.delete
    end if
next


Determining if two Strings have common subtrings of a given length in Java Problem:

  Given 2 strings, consider all the substrings within them of length
  len. Len will be 1 or more. Returns true if there are any such
  substrings which appear in both strings. Compute this in linear time
  using a HashSet.

I know as a matter of fact that there are many areas upon which my code could be improved. However, rather than asking other people to correct my code, I would much prefer to be given hints or general principles as to how my code could be improved. I think this way, me,  and everyone that read this post could get more out of this exercise this way.

So could someone please provide me with some comments or suggestions? Critics of any level and kind are welcomed! Feel free to pick apart my code!

Class:

import java.util.HashSet;
import java.util.Set;

// CS108 HW1 -- String static methods
public class StringCode {

    public static boolean stringIntersect(String firstString, String secondString, int lengthOfSubstring) {

        boolean sameSubstringInBothStrings = false;

        Set<String> setOne = StringCode.getSubstringsSet(firstString, lengthOfSubstring);
        Set<String> setTwo = StringCode.getSubstringsSet(secondString, lengthOfSubstring);

        //compare setOne and setTwo to find out if there is any matching elements
        outerForLoop:
        for(String aSubstringInSetOne : setOne){
            for(String aSubstringInSetTwo : setTwo){
                if(aSubstringInSetOne.equals(aSubstringInSetTwo)){
                    sameSubstringInBothStrings = true;
                    break outerForLoop;
                }
            }
        }

        return sameSubstringInBothStrings;
    }

    static Set<String> getSubstringsSet(String aString, int lengthOfSubstring){
        Set<String> setOfSubstrings = new HashSet<>();

        if(aString.length() < lengthOfSubstring){
            return setOfSubstrings;
        }

        if(aString.length() == lengthOfSubstring){
            setOfSubstrings.add(aString);
            return setOfSubstrings;
        }

        char[] charArray = aString.toCharArray();

        //starting from the first index, going through aString
        for(int i = 0; i <= aString.length() - lengthOfSubstring; i++){

            StringBuilder sb = new StringBuilder();

            //add each substring of length (lengthOfSubstring) to the setOfSubstrings.
            for(int j = 0; j < lengthOfSubstring; j++){
                sb.append(charArray[i + j]);
            }

            setOfSubstrings.add(sb.toString());
        }

        return setOfSubstrings;
    }

}

Test:

import assign1.StringCode;
import static org.junit.Assert.*;
import org.junit.Test;

public class StringCodeTest {

    @Test
    public void testStringIntersectNormalStrings(){
        assertEquals(true, StringCode.stringIntersect("abcdickefg" , "zyxqdick", 4));
    }

    @Test
    public void testStringIntersectNormalStrings2(){
        assertEquals(false, StringCode.stringIntersect("abcdef", "zzzzzz", 1));
    }

    @Test
    public void testEmptyString(){
        assertEquals(false, StringCode.stringIntersect("abc", "", 2));
    }

    @Test
    public void testLengthOfSubstringGreaterThanTheStringItself(){
        assertEquals(false, StringCode.stringIntersect("abc", "xyz", 5));
    }

    @Test
    public void testTwoEmptyStrings(){
        assertEquals(false, StringCode.stringIntersect("", "", 2));
    }

}

Don't add unnecessary pre-checks
Your method getSubstringsSet starts with two early returns:
if(aString.length() < lengthOfSubstring){
    return setOfSubstrings;
}

if(aString.length() == lengthOfSubstring){
    setOfSubstrings.add(aString);
    return setOfSubstrings;
}

You actually don't need them. The rest of the code handles both cases just fine and it doesn't add to clarity or improve performance. Early-returns should be used when the rest of the code can then assume a condition that was checked, making its code simpler; but it is not the case here.
Use built-in methods
To add the substrings, you are currently doing:
StringBuilder sb = new StringBuilder();

//add each substring of length (lengthOfSubstring) to the setOfSubstrings.
for(int j = 0; j < lengthOfSubstring; j++){
    sb.append(charArray[i + j]);
}

setOfSubstrings.add(sb.toString());

So you're creating a new StringBuilder and then appending each character. This is a complicated way of retrieving the substring between two indices. It also allocates a new StringBuilder each time which isn't really needed.
You can just have
setOfSubstrings.add(aString.substring(i, i + lengthOfSubstring));

In the same way, stringIntersect is a lot more complicated than it should be.
First of all, you are using a label outerForLoop and then using it to break out of the inner loop with break outerForLoop;. This is generally not a good practice. What this shows is a missing method: what you really want is to make a method isIntersection(set1, set2) that would determine if the two sets have elements in common.
However, you don't even need to write one, since it is already built-in: Collections#disjoint. This method returns true if the two given collections are disjoint, i.e. have no elements in common. Therefore, you can remove your double for loop and just have:
boolean sameSubstringInBothStrings = !Collections.disjoint(setOne, setTwo);


Getting information from URL efficiently I wrote a dirty and ugly code to show in console how many bugs are left until the release of elementary codename Isis.
The code works but it's pretty inefficient. What it does is:

downloads the entire Launchpad site
writes it down to a temp file
scans for the appropriate lines in said file
stores the number of bugs still open
deletes the temp file
shows the total number of bugs left

How could I make this code more efficient? For example, is there a way to get that data without having to download the entire site each time? Or to scan the downloaded data for the still opened bugs, without having to write it down to a temp file first?

I'm running python 2.7.3.

import urllib
import os

"""
Retrieves the number of bugs left until the arrival of elementary OS Isis.
"""

# Get Launchpad data.
f = urllib.urlopen("https://launchpad.net/elementary/+milestone/isis-beta1")
s = f.read()
f.close()

# Write data to temp file.
ff = open("temp.del", "w")
ff.write(s)
ff.close()

# Find number of bugs left.
temp = open("temp.del", "r")
for i, line in enumerate(temp):

    # Incomplete bugs.
    if 'span class="statusINCOMPLETE">' in line and i < 450:
        a = temp.next()
        b = a.split('')
        c = b[1].split('')
        b_incomp = int(c[0])

    # Confirmed bugs.
    if 'span class="statusCONFIRMED">' in line and i < 450:
        a = temp.next()
        b = a.split('')
        c = b[1].split('')
        b_conf = int(c[0])

    # In progress bugs.
    if 'span class="statusINPROGRESS">' in line and i < 450:
        a = temp.next()
        b = a.split('')
        c = b[1].split('')
        b_inprog = int(c[0])

# Delete temp file.
os.remove('temp.del')

# Print to console.
bugs = b_incomp + b_conf + b_inprog
print '\n%d bugs left until Isis. Quit moaning.\n' % bugs

To start off, I will say what I say on every question dealing with files: always use with syntax.

By using this syntax you lessen the change for bugs; especially the bug that happens when you forget to call close() on an open file pointer. Your code shows this bug. You open the temp.del file, but never call close on it. Because of this, when you program finishes running, you have an active file pointer that the OS needs to clean up.

To fix this, simply do this:

with open('temp.del', 'r') as file:
    # Do stuff.

Once this block of code finishes, file will automatically get closed.

To answer you question whether or not its necessary to write the temporary file, take a look a the urllib.urlopen documentation. It says:

  Except for the info(), getcode() and geturl() methods, these methods have the same interface as for file objects — see section File Objects in this manual. (It is not a built-in file object, however, so it can’t be used at those few places where a true built-in file object is required.)

This means that you should be able to iterate through this just like you would a file. Which, in your case, means you do not have to save the temporary file. Unfortunately, in Python 2.7, the return object from urllib.urlopen is not a valid context manager (i.e doesn't implement __enter__ and __exit__). So we have to use open and close:

file = urllib.urlopen("https://launchpad.net/elementary/+milestone/isis-beta1")
for line in file:

Now onto improvements:

In each of your if statements, you have the condition and i < 450. You can bring this out into its own statement, and because you don't do anything after line 450, you can simply break if it evaluates to true:

for index, line in enumerate(file):
    if index >= 450:
        break

Getting the bug counts can be done easier using a regex:

import re

# Grabs all the numerical data, converts them to ints, then sums them together.
bugs += sum(map(int, re.findall('\d+', next_line)))

Each if statement holds the same code: you get the next line, split it, and then assign a value to a certain variable. After you finish the bugs, you add them all together. You don't differentiate after you parse them. You can combine these into a single if statement and simply keep a single running total like so:

if any(span in line for span in ['span class="statusINCOMPLETE">',
                                 'span class="statusCONFIRMED">',
                                 'span class="statusINPROGRESS">']):

    bugs += sum(map(int, re.findall('\d+', next_line)))

If you need to differentiate the different kind of bugs, you can use a dict:

bug_types = {'span class="statusINCOMPLETE">': 0,
             'span class="statusCONFIRMED">': 0,
             'span class="statusINPROGRESS">': 0}

for key in bug_types:
    if key in line:
        bug_types[key] += sum(map(int, re.findall('\d+', next_line))))

When formatting string output, convention says to use the str.format() function. The syntax look like this:

print '{} {}!'.format('Hello', 'World')

If you take all of my suggestions into account, here is your new code:

import urllib
import os
import re

bug_types = {'span class="statusINCOMPLETE">': 0,
    'span class="statusCONFIRMED">': 0,
    'span class="statusINPROGRESS">': 0}

# Even though we have to use `open` and `close` and can simply enclose the next
# for loop with the calls, I still would load the file into a temporary array.
# Then iterate over that.
file = urllib.urlopen("https://launchpad.net/elementary/+milestone/isis-beta1")
lines = [str(line) for line in file]
file.close()

# Because of the way `urlopen` streams the data, we cannot use the `next` command.
# Therefore, we need to use a flag to remember what kind of bug we found on the
# previous line.
bug_type = ''
for index, line in enumerate(lines):
    if index >= 450:
        break

    if bug_type:
        bug_types[bug_type] += sum(map(int, re.findall('\d+', line)))
        bug_type = ''
    else:
        for key in bug_types:
            if key in line:
                bug_type = key
                break

print('\n{} bugs left until Isis. Quit moaning.\n'.format(sum(bug_types.values())))


Array duplicate removal for duplicates exceeding `N`-number I recently encountered a front-end interview coding challenge question that required one to create a function which returned an array that excluded numbers that occurred more than n-number of times (i.e. duplicate-removal).  In this example it was all numbers repeated >= three (3) times.  I managed to submit an answer, but admittedly struggled solving it.
I am looking for any feedback / suggestions on how to make it more concise and efficient (the for loop inside the forEach method is especially repulsive).
Here is what I came up with:

// Remove duplicates that occur 3 or more times in an array
// keeping unique values and those with less than 3
function removeMany(arr) {
  const newArr = Array.from(arr).sort()
  let count = 0;
  let result = []

  newArr.forEach((value, index, ar) => {
    count += 1;
    // refactored afterwards from (ar[index + 1] !== value)
    if (ar.lastIndexOf(value) <= index && count <= 2) {
      for (var i = 0; i < arr.length; i++) {
        if (arr[i] === value) {
          result.push(arr[i])
        }
      }
      count = 0
    } else if (ar[index + 1] !== value) {
      count = 0;
    }
  });

  // +1 is there anyway to return a result that mimicks the original order of `numbers`?
  return result;  // [1, 2, 2, 3, 4, 4]
}

// Ex. 1
const numbers = [1, 2, 3, 2, 4, 4, 5, 5, 5, 5];
console.log(removeMany(numbers));

As you expected, there is a much simpler way to do this.

Create a map where the keys are the numbers in the array and the value is the number of times each number appears in the array.
Filter the input array for any numbers that appear less than X times.

function removeMany(numbers, max) {
    const numberMap = numbers.reduce((map, num) => {
        map[num] = map[num] ? map[num] + 1 : 1;
        return map;
    }, []);

    return numbers.filter(num => numberMap[num] < max);
}

const numbers = [1, 2, 3, 2, 4, 4, 5, 5, 5, 5];
console.log(removeMany(numbers, 3));


Finding terminal server user is connected on I'm trying to find out which terminal server a user is connected to.
Currently I'm trying to cycle through each server and check if he is connected.

$servers = @("server01","server02","server03","server04","server05","server06","server07","server08")

foreach ($server in $servers ) {
    $WMI = Get-WmiObject -Class Win32_Process -ComputerName $server -ErrorAction Stop
    $ProcessUsers = $WMI.getowner().user | Select-Object  -Unique
    if ( $ProcessUsers.Contains("user")) {
        $userServer = $server
        break
    }
}
echo $userServer

This can take some time depending on the server the user is connected too. Is there any better way to check this with PowerShell?

A couple things here... I would use the Win32_LoggedOnUser class to query, that'll get you the user name and "domain" for anybody with a connection to the computer. I have quotes on domain, because it considers the computer itself to be the domain for any local accounts. You just have to do a little string parsing to get the info you want into a nice, readable format.

Get-WmiObject -Class Win32_LoggedOnUser -ComputerName 'Server01' |
    Select PSComputerName,@{l='Domain';e={$_.Antecedent -replace '^.+?Domain="(.+?)".*$','$1'}},@{l='Name';e={$_.Antecedent -replace '^.+?Name="(.+?)".*$','$1'}} -Unique

Then, you can pass multiple computer names to the Get-WmiObject cmdlet at once, and it'll get the info from all of them. Plus, if you run it as a background job you can set a throttle limit, and it will try to process as many connections as you set the limit to.

$servers = @("server01","server02","server03","server04","server05","server06","server07","server08")

$Job = Get-WmiObject Win32_LoggedOnUser -ComputerName $servers -AsJob -ThrottleLimit 10
Wait-Job $Job
$Results = Receive-Job $Job
$Results | Select PSComputerName,@{l='Domain';e={$_.Antecedent -replace '^.+?Domain="(.+?)".*$','$1'}},@{l='Name';e={$_.Antecedent -replace '^.+?Name="(.+?)".*$','$1'}} -Unique | Sort PSComputerName,Domain,Name


Rake task to send users a reminder to post with conditions I am implementing a feature that reminds users to make a post via email if

the user has set daily reminders to true
he has not posted yet today
and if the current hour matches when he would like to receive the daily reminder in his time zone.

I am going to write a Rake task for it and then schedule it to run every hour.

Currently, the rake tasks looks like this:

namespace :mail do
  desc "Send daily reminder to users"
  task :daily_reminder => :environment do
    User.all.each do |user|
      Time.zone = user.time_zone
      if user.reminder == true && Time.current.strftime("%H").to_i == user.reminder_time && user.reminded == false
        UserMailer.test_email(user).deliver_now
        user.update_attribute(:reminded, true)

      elsif Time.current.strftime("%H") == user.reminder_time + 1 && user.reminded == true
        user.update_attribute(:reminded, false)
        puts "Update to false"
      end
    end
  end
end

I am resetting the field reminded back to false the hour after they were reminded.

Since I am rather new to Ruby and Ruby on Rails I would appreciate feedback on the setup of this feature a lot.

Please critique away on anything you think is broken or needs improvement.

Also, right now it is not working and it's giving me back a undefined method + for nil:NilClass which tells me that the user hasn't an ActiveRecord assigned to it? Because the reminder_time column exists and is set (to 16:00).

Follow-up question:

The if clause looks kind of large and unclean. Is that the proper way to go about this?
Shall I make the whole logic more clean by putting some of the conditions in methods?

Never User.all.each in the real world. That loads your entire table into memory and instantiates ActiveRecord models for every record. You want User.find_each do |user|, which uses find_in_batches to load 1000 records at a time.
Don't do simple filtering in Ruby, do it in the database. This...

if user.reminder == true

should be a simple scope that prevents these records from ever being loaded. Something like this:

class User < ActiveRecord::Base
  scope :with_reminder, -> { where(reminder: true) }
end

Now, your top-level loop can use the scope:

User.with_reminder.find_each do |user|

This filters out a huge number of users who would otherwise be loaded from the database needlessly.
Your if/else aren't actually correct, since your intent is to load all users with reminder = true. You load all users, and then your if looks for user.reminder == true, but your else doesn't. It affects all users with reminder of true or false, and then sets their reminded to false.

  ... which tells me that the user hasn't an ActiveRecord assigned to it? Because the reminder_time column exists and is set (to 16:00).

Err, no, that's not at all what that tells you. Nothing has "an ActiveRecord" assigned to it, that isn't at all how ActiveRecord works. Every record in the database is wrapped in an ActiveRecord object as you read it from the database, it's impossible for some users to "have" ActiveRecords and others not to.

See point 3. Your else currently checks all records, including those with reminder == false, and presumably those records don't have a reminder_time set.
It's really not clear what you're trying to do with Time.current.strftime("%H") == user.reminder_time + 1. What is "1"? You have no unit, so it means "1 second". Your probably never going to hit that.


Is nesting grids a good idea? I find myself nesting a lot of grids inside grids in WPF. I just found myself 3 Grids deep, and stopped to think: "Should I be doing this?" Is there some kind of performance cost? Is it unmaintainable? (Kind of like heavily nested ifs maybe.)

I guess the alternative is to have one grid, and use a whole lot of column spans.

(You may ignore the use of Events, I am aware commands are prob a better idea)

<Grid Name="grid">
<Grid.ColumnDefinitions>
    <ColumnDefinition Width="*"/>
    <ColumnDefinition Width="*"/>
    <ColumnDefinition Width="*"/>
</Grid.ColumnDefinitions>

<Grid Grid.Column="1">
    <Grid.RowDefinitions>
        <RowDefinition Height="20"/>
        <RowDefinition/>
        <RowDefinition Height="20"/>
    </Grid.RowDefinitions>
    <Grid.ColumnDefinitions>
        <ColumnDefinition/>
        <ColumnDefinition/>
        <ColumnDefinition/>
        <ColumnDefinition/>
    </Grid.ColumnDefinitions>

    <Button Grid.Row="0" Grid.Column="0" VerticalAlignment="Top"
        HorizontalAlignment="Left" Click="btnStart_Click"
        Content="New Game"/>
    <Button Grid.Row="0" Grid.Column="1" VerticalAlignment="Top"
        HorizontalAlignment="Center" Click="btnRun_Click"
        Content="Run"/>
    <Button Grid.Row="0" Grid.Column="2" VerticalAlignment="Top"
        HorizontalAlignment="Center" Click="btnUndo_Click"
        Content="Undo"/>
    <Button Grid.Row="0" Grid.Column="3" VerticalAlignment="Top"
        HorizontalAlignment="Right" Click="btnStep_Click"
        Content="Step"/>

    <Grid Grid.Row="1" Grid.ColumnSpan="4">
        <Grid.RowDefinitions>
            <RowDefinition/>
            <RowDefinition/>
        </Grid.RowDefinitions>
        <board:BoardView x:Name="boardView" Grid.Row="0"  Background="Firebrick"/>
        <GridSplitter  ResizeDirection="Rows" HorizontalAlignment="Stretch"
            VerticalAlignment="Bottom" />
        <TextBox Name="txtLog" Grid.Row="1"
            Text="{Binding Path=GameLog.Log, Mode=TwoWay}"
            VerticalScrollBarVisibility="Visible" FontFamily="Global Monospace"
            AcceptsReturn="True" Height="260" VerticalAlignment="Stretch"/>

    </Grid>

    <CheckBox Grid.Row="2" Grid.Column="0" Grid.ColumnSpan="3"
        Content="Use Algebraic Notation"
        IsChecked="{Binding Path=UseAlgebraicNotation}"/>

</Grid>

<ContentControl Grid.Column="0"
                Content="{Binding Path=Game.BlackPlayer}"
                Margin="3"
                />

<ContentControl Grid.Column="3"
                Content="{Binding Path=Game.WhitePlayer}"
                Margin="3"
                />

</Grid>

It's better to use nested grids that are easy to read, than complex spanning.  I try to avoid spanning in most scenarios, unless it's just 1 simple span.  But when in doubt, nest the grids, because that way future layout changes won't break everything where as spanning is directly tied to the number of columns you have.

A great example of this is headers and footers, you wouldn't want them to not fill the width of an app,


Reading complete contact information in Android My goal is to read complete contact information on an Android application. This is very slow and my understanding is that since the contacts are stored locally in a phone database, it shouldn't take long to fetch complete contact details.

public void readContacts() {
    Log.d("TAG", " -> Into readContacts");
    // Get the ContentResolver
    ContentResolver cr = getContentResolver();
    // Get the Cursor of all the contacts
    Cursor cursor = cr.query(ContactsContract.Contacts.CONTENT_URI, null, null, null, null);

    // Move the cursor to first. Also check whether the cursor is empty or not.
    if (cursor.moveToFirst()) {
        // Iterate through the cursor
        do {
            // Get the contacts name
            Contact contactToSave = new Contact();
            contactToSave.setIsUploaded(false);
            String id = cursor.getString(cursor.getColumnIndex(
                    ContactsContract.Contacts._ID));
            Log.d(TAG, " ID " + id);

            String name = cursor.getString(cursor.getColumnIndex(ContactsContract.Contacts.DISPLAY_NAME));
            contactToSave.setFirstName(name);
            Log.d(TAG, " Complete Name " + name);

            String orgWhere = ContactsContract.Data.CONTACT_ID + " = ? AND " + ContactsContract.Data.MIMETYPE + " = ?";
            String[] orgWhereParams = new String[]{id,
                    ContactsContract.CommonDataKinds.Organization.CONTENT_ITEM_TYPE};
            Cursor orgCur = cr.query(ContactsContract.Data.CONTENT_URI, null, orgWhere, orgWhereParams, null);

            if (orgCur.moveToFirst()) {
                String orgName = orgCur.getString(orgCur.getColumnIndex(ContactsContract.CommonDataKinds.Organization.DATA));
                String title = orgCur.getString(orgCur.getColumnIndex(ContactsContract.CommonDataKinds.Organization.TITLE));
                contactToSave.setCompany(orgName);
            }
            orgCur.close();

            Cursor emailCur = cr.query(
                    ContactsContract.CommonDataKinds.Email.CONTENT_URI,
                    null,
                    ContactsContract.CommonDataKinds.Email.CONTACT_ID + " = ?",
                    new String[]{id}, null);
            while (emailCur.moveToNext()) {
                // This would allow you get several email addresses
                // if the email addresses were stored in an array
                String email = emailCur.getString(
                        emailCur.getColumnIndex(ContactsContract.CommonDataKinds.Email.DATA));
                contactToSave.setEmail(email);
            }
            emailCur.close();

            if (Integer.parseInt(cursor.getString(cursor.getColumnIndex(
                    ContactsContract.Contacts.HAS_PHONE_NUMBER))) > 0) {
                Cursor pCur = cr.query(
                        ContactsContract.CommonDataKinds.Phone.CONTENT_URI,
                        null,
                        ContactsContract.CommonDataKinds.Phone.CONTACT_ID + " = ?",
                        new String[]{id}, null);
                while (pCur.moveToNext()) {
                    String phoneNumber = pCur.getString(pCur.getColumnIndex(
                            ContactsContract.CommonDataKinds.Phone.NUMBER));
                    contactToSave.setPhone(phoneNumber);
                }
                pCur.close();
            }
            mAdapter.add(contactToSave);
        } while (cursor.moveToNext());
        mAdapter.sort(new Comparator<Contact>() {
            @Override
            public int compare(Contact o1, Contact o2) {
                return o1.getFirstName().compareTo(o2.getFirstName());
            }
        });
    }
    // Close the cursor
    cursor.close();
}

I think creating multiple cursors for different data may be a performance overhead. Is there any way those can be combined?
Is there any other way to fetch the cursor than running multiple queries using multiple cursors?
If the answer for the above two questions are negative, then is this the best way possible for such a simple requirement?

Use AsyncQueryHandler, a helper class to help make handling asynchronous ContentResolver queries easier.

It basically wraps the ContentResolver object and handles background execution of its operations (CRUD) as well as passing messages (result) from the between threads (background and main/UI).

Just use to call the queryHandler.startQuery method and you will get callback in onQueryComplete method.

Example

class QueryHandler extends AsyncQueryHandler {

        public QueryHandler(ContentResolver cr) {
            super(cr);
        }

        @Override
        protected void onQueryComplete(int token, Object cookie, Cursor cursor) {
            switch (token) {

                case CONTACT_QUERY:
//Do your stuff here
                    if (cursor.moveToFirst()) {
                        // Iterate through the cursor
                        do {

                            String id = cursor.getString(cursor.getColumnIndex(
                                    ContactsContract.Contacts._ID));
                            Log.e(TAG, " ID " + id);

                        } while (cursor.moveToNext());
                    }
                    break;
}
}

Use below method to fetch contact and add projection, selection according to your requirment.

public void fetchContact(){
        queryHandler.startQuery(CONTACT_QUERY, null, ContactsContract.Contacts.CONTENT_URI, null, null, null, null);
    }


Setting output parameter inside LINQ select Is there a way to reduce the number of lines by setting the output variable inside LINQ select statement? How would you do it?

public void GetSomeValue(int bonusID, out decimal? maxGiving, out int? isActive)
{
    maxGiving = 0;
    isActive = 0;
    try
    {
        var con = new BonusModelDataContext();
        var a = con.Bonus
            .Where(c => c.ID == bonusID)
            .Select(c => new { maxGiving = c.maxGiving, isActive = c.isActive })
            .SingleOrDefault();
        maxGiving = a.maxGiving;
        isActive = a.isActive;

        // return iovation;
    }
    catch(Exception ex)
    {
        //return -1;
    }
}

No, you can't do that, you can't use out parameters in a lambda.

But even if you could, I don't think it would be a good idea to do so, because it would make the code less readable.

There are also some suspicious practices in your code:

You're using SingleOrDefault(), but you don't check the result for null. If you don't want to check for null, use Single() which will throw an exception if the query returns an empty collection.
You're catching all exceptions. You should catch only the exceptions you know about. And you certainly shouldn't use something like return -1, it's much better to just let the exception bubble up and catch it somewhere up the call stack. Another option might be catching the exception, wrapping it in some more specific exception and throwing that.
Why are you setting the output parameters to 0 by default? They are nullable, so setting them to null makes more sense to me.


Select the one table before my link I need to select the <table> before the table where the link I clicked. I can't use IDs, because I don't want to edit my jQuery every time I add a new table.

but this looks kinda bloated for me:

$(this).closest("table").prevAll("table").first().css( "background", "yellow" );

take a look here: http://jsfiddle.net/syddmxfn/13/

I think there's no silver bullet for what you're trying to do.
To find the previous table,
it's necessary to go up the DOM tree until the first table,
and then from there find the previous table.

.closest("table") is optimal and necessary.

On the other hand, it's true that .prevAll("table").first() is a bit awkward.
Something like .closestPrevious("table") would be the best.
I searched around and couldn't find such.
So you're stuck with .prevAll("table").first().

In short, bloated it may be, I think it's the best possible way.


Minimize array and output the remainder with respect to 1 second time limit This is my code:

n = int(raw_input())
seq = raw_input().split()
def minimize_list(lst):
    new_lst = []
    k = len(lst)
    for i in range(k-1):
        new_lst.append(lst[i+1]-lst[i])
    return new_lst
for i in range(len(seq)):
    seq[i] = int(seq[i])
final_list = []
for num in seq:
    final_list.append(num)
while len(final_list) != 1:
    final_list = minimize_list(final_list)
print (final_list[0])%(1000000000+7)

It takes a number and a sequence of numbers. (The first number is the number of numbers in the sequence). It then calculates a shorter sequence with the difference of adjacent numbers until there is just 1 number. Unfortunately, The time complexity is not good enough. I have to make it better to meet the time limit which is 1 second.

Any idea?

Note: I cannot see the input. I can just pass my code to a server which has 10 examples and runs my code on them. The server then tells me that this code has exceeded the time limit for some examples.

Update: I want to print (final_list[0])%(1000000000+7)

One easy way is to use numpy, which is implemented in C (if it is available on the machine your online judge is running on). It provides fast ways to do numerical calculations. In this case, you can use numpy.diff, which calculates the difference between elements. It even has a n argument, so it applies it n times. We also know how often we need to apply it to get only a single element, len(seq) - 1 times:

import numpy as np

n = int(raw_input())
seq = map(int, raw_input().split())
print np.diff(seq, len(seq) -1)[0]

If you need to stick to built-in methods, you could at least make the minimize_list method a bit shorter using a list comprehension (and maybe rename it to diff):

def diff(x):
    return [b - a for a, b in zip(x, x[1:])]

def reduce_diff(seq):
    while len(seq) > 1:
        seq = diff(seq)
    return seq[0]

if __name__ == "__main__":
    n = int(raw_input())
    seq = map(int, raw_input().split())
    print reduce_diff(seq)

I also removed the redundant copying from seq to final_list and instead of the for loop to convert all values to integers I used map.

Be aware that if you want to use this with Python 3 (as you should, since Python 2 will be obsolete soon), you need to surround that map call with a call to list and replace raw_input with input.

In addition I added a if __name__ == "__main__": guard to allow importing from this script from another script.

And finally, one can notice that this approach is \$\mathcal{O}(n^2)\$. When looking at a simple example of successive application of diff, a pattern becomes obvious:

\$
a, b, c, d, e\\
b-a, c-b, d-c, e-d\\
c-2b+a,d-2c+b, e-2d+c\\
d-3c+3b-a, e-3d+3c-b\\
e-4d+6c-4b+a
\$

The coefficients are the ones from Pascals triangle, together with an alternating sign:

\$
\begin{pmatrix}
 & & & & 1 & & & &\\
 & & & 1 & & 1 & & & \\
 & & 1 & & 2 & & 1 & & \\
 & 1 & & 3 & & 3 & & 1 & \\
1 & & 4 & & 6 & & 4 & & 1
\end{pmatrix}
\$

So, if you find a fast way to calculate the n-th row of Pascal's triangle, you can solve this problem in \$\mathcal{O}(n)\$, by adding the terms with a plus, modulo p and the ones with a minus, modulo p and then subtract them:

from itertools import cycle

def pascal_line(n):
    line = [1]
    mid, even = divmod(n, 2)
    line_append = line.append
    for k in xrange(1, mid + 1):
        num = line[k-1]*(n + 1 - k)//(k)
        line_append(num)
    reverse_it = reversed(line)
    if not even:
        next(reverse_it)
    line.extend(reverse_it)
    return line

def reduce_pascal(seq):
    return sum(sign * c * x
               for sign, c, x in zip(cycle([1, -1]),
                                     pascal_line(len(seq)-1),
                                     reversed(seq)))

if __name__ == "__main__":
    n = int(raw_input())
    seq = map(int, raw_input().split())
    p = 1000000000+7
    print reduce_pascal(seq) % p

One could make this also modular p at each step by doing the sum manually and enforcing the mod p.

Beware: This seems to break down for large sequences (since the terms in the triangle get big).


Make WCF Service testable I have WCF Service.  It works fine and I want to have test coverage for it. Unit tests and acceptance.

The problem is the static class in the code.  How is it possible to avoid it?

If it will be refactored to the code without static classes - I can use mocks for example Moq.

I have also used Ninject as DI Framework so far.

    [ServiceContract]
    public interface IWorkingService
    {
        [OperationContract]
        Collection<Result> UpdateEntity(int entityID);
    }

    public class WorkingService : IWorkingService
    {
        private static readonly WorkingService Logger = ProviderLog4Net.GiveLogger(typeof(SomeService));

        /// <inheritdoc />
        public Collection<Result> UpdateEntity(int entityID)
        {
            Logger.Info("------- UpdateEntity call ------------");

            try
            {
                return CoreFacade.UpdateEntity(entityID);
            }
            catch (Exception exception)
            {
                Logger.Fatal(exception);
                throw;
            }
        }
    }

    public static class CoreFacade
    {
       ...
    }

You are doing it wrong.

Why are you using WCF if you are only using it as an old timer ASMX service without using the extensibility points?

First things first - clean up

[ServiceContract]
public interface IWorkingService
{
    [OperationContract]
    Collection<Ergebnis> UpdateEntity(int entityId);
}

The service interface is the same as yours, but:

Why are you using Collection<T> as the return type? Use someting more generic.
Do not use primitives as service parameters, ever! Using primitives as parameters means that you cannot versionize your service, such as adding extra non-required properties.

public class WorkingService : IWorkingService
{
    private readonly NotStaticFacade _notStaticFacade;

    public WorkingService(NotStaticFacade notStaticFacade)
    {
        _notStaticFacade = notStaticFacade;
    }

    public Collection<Ergebnis> UpdateEntity(int entityId)
    {
        return _notStaticFacade.UpdateEntity(entityId);
    }
}

The service implementation is much more slimmer then yours because it's missing the logging stuff (be patient!). The important thing is that it isn't containing a parameter-less constructor because the service has a dependency: the NotStaticService (in your code this is the static CoreFacade). What does this mean? We will need a custom ServiceHostFactory because the default one cannot handle these kinds of situation, but don't worry; Ninject is with us!

Install-Package Ninject.Extensions.Wcf

The Ninject has an extension for WCF so install it via NuGet (remove App_Start directory after installation) and after that, create our own ServiceHostFactory:

public class WorkingServiceHostFactoryWithNinject :    NinjectServiceHostFactory
{
    private readonly IKernel _kernel;

    public WorkingServiceHostFactoryWithNinject()
    {
        _kernel = new StandardKernel(new WorkingServiceNinjectModule());

        // SetKernel is a static method in Ninject WCF!
        SetKernel(_kernel);
    }
}

Nothing fancy; we only telling the factory which IKernel instance we want to use. As you see, you will have a NinjectModule prepared to map the bindings.

public class WorkingServiceNinjectModule : NinjectModule
{
    public override void Load()
    {
        // what ever other binding is need
        Kernel.Bind<NotStaticFacade>().ToSelf();
    }
}

I'm only telling the kernel how to resolve the NotStaticFacade but this isn't necessary this way. We haven't finished yet because we haven't told the system to use our factory to build up our service. To do this, we need to open the *.svc markup (right-click on the file then View markup) and write into the directive the Factory attribute:

<%@ ServiceHost Language="C#" Debug="true" Service="CodeReview.WorkingService" CodeBehind="WorkingService.svc.cs"
        Factory="CodeReview.WorkingServiceHostFactoryWithNinject" %>

Save and close this.

Now we have a service with dependency injection and the only one real facade is our service.

WCF extensibility - IServiceBehavior, IErrorHandler

The WCF infrastructure has great possibilities for extending its capabilities, such as handling an error!

To do that we will create a new service behavior and we will use it for adding our error handlers to the service:

public class ErrorHandlerBehaviorWithNinjectKernel : IServiceBehavior
{
    private readonly IKernel _kernel;

    public ErrorHandlerBehaviorWithNinjectKernel(IKernel kernel)
    {
        _kernel = kernel;
    }

    public void Validate(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
    {
    }

    public void AddBindingParameters(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase, Collection<ServiceEndpoint> endpoints,
        BindingParameterCollection bindingParameters)
    {
    }

    public void ApplyDispatchBehavior(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
    {
        var errorHandlers = _kernel.GetAll<IErrorHandler>().ToArray();

        if (!errorHandlers.Any())
        {
            throw new Exception("No errorhandler was found");
        }

        foreach (var channelDispatcher in serviceHostBase.ChannelDispatchers.Select(channelDispatcherBase => channelDispatcherBase as ChannelDispatcher))
        {
            foreach (var errorHandler in errorHandlers)
            {
                channelDispatcher.ErrorHandlers.Add(errorHandler);
            }
        }
    }
}

What can we see here? We are adding all IErrorHandler instance to all possible dispatchers to use it. IErrorHandler is coming from the WCF infrastructure. You need to create your own implementation of it like this:

public class ErrorLogger : IErrorHandler
{
    private readonly IAmLogger _logger;

    public ErrorLogger(IAmLogger logger)
    {
        _logger = logger;
    }

    public void ProvideFault(Exception error, MessageVersion version, ref Message fault)
    {
        //nothing to do here
    }

    public bool HandleError(Exception error)
    {
        _logger.Fatal(error);

        return true;
    }
}

I have created the IAmLogger interface to fake your Log4Net stuff, nothing more:

public interface IAmLogger
{
    void Info(string info);
    void Fatal(Exception exception);
}

If you have created your IErrorHandler implementation, register it in your NinjectModule:

Kernel.Bind<IErrorHandler>().To<ErrorLogger>();

There are two ways to add an IServiceBehavior to a service: view attributes (then our class have to derive from Attribute) and programatically.
We will use the last one because of the IKernel dependency.

Let's bring up the factory again and extend it with overriding the CreateServiceHost method:

public class WorkingServiceHostFactoryWithNinject : NinjectServiceHostFactory
{
    private readonly IKernel _kernel;

    public WorkingServiceHostFactoryWithNinject()
    {
        _kernel = new StandardKernel(new WorkingServiceNinjectModule());

        // SetKernel is a static method in Ninject WCF!
        SetKernel(_kernel);
    }

    protected override ServiceHost CreateServiceHost(Type serviceType, Uri[] baseAddresses)
    {
        var serviceHost = base.CreateServiceHost(serviceType, baseAddresses);

        serviceHost.Description.Behaviors.Add(new ErrorHandlerBehaviorWithNinjectKernel(_kernel));

        return serviceHost;
    }
}

Now we are creating the ServiceHost and after that we are adding our behavior (ErrorHandlerBehaviorWithNinjectKernel) to it. That is how every exception in the system will be logged and we don't need to write anything into our service.

Logging operation calls - IOperationInvoker, IOperationBehavior

In WCF we can specify our own Operation invoker by implementing the IOperationInvoker interface and applying it to our operations via an IOperationBehavior instance inside a service behavior instance. (That's all, really!)

First create an IOperationLogger implementation:

public class OperationInvokerWithLogging : IOperationInvoker
{
    private readonly IAmLogger _logger;
    private readonly IOperationInvoker _invoker;
    private readonly DispatchOperation _dispatchOperation;

    public OperationInvokerWithLogging(IAmLogger logger, IOperationInvoker invoker, DispatchOperation dispatchOperation)
    {
        _logger = logger;
        _invoker = invoker;
        _dispatchOperation = dispatchOperation;
    }

    public object[] AllocateInputs()
    {
        return _invoker.AllocateInputs();
    }

    public object Invoke(object instance, object[] inputs, out object[] outputs)
    {
        _logger.Info(string.Format("Operation {0} called (Invoke)", _dispatchOperation.Name));
        return _invoker.Invoke(instance, inputs, out outputs);
    }

    public IAsyncResult InvokeBegin(object instance, object[] inputs, AsyncCallback callback, object state)
    {
        _logger.Info(string.Format("Operation {0} called (InvokeBegin)", _dispatchOperation.Name));
        return _invoker.InvokeBegin(instance, inputs, callback, state);
    }

    public object InvokeEnd(object instance, out object[] outputs, IAsyncResult result)
    {
        _logger.Info(string.Format("Operation {0} called (InvokeEnd)", _dispatchOperation.Name));
        return _invoker.InvokeEnd(instance, out outputs, result);
    }

    public bool IsSynchronous { get { return _invoker.IsSynchronous; } }
}

This is mostly a stub for the real IOperationInvoker what comes from the WCF infrastructure (we will see it soon). The only additional mechism is the logging parts (with out IAmLogger instance!).

Let's create our IOperationBehavior implementation; this will map the IOperationInvoker to all of our operations:

public class LoggingOperationBehaviorWithNinjectKernel : IOperationBehavior
{
    private readonly IKernel _kernel;

    public LoggingOperationBehaviorWithNinjectKernel(IKernel kernel)
    {
        _kernel = kernel;
    }

    public void Validate(OperationDescription operationDescription)
    {

    }

    public void ApplyDispatchBehavior(OperationDescription operationDescription, DispatchOperation dispatchOperation)
    {
        dispatchOperation.Invoker = new OperationInvokerWithLogging(_kernel.Get<IAmLogger>(), dispatchOperation.Invoker, dispatchOperation);
    }

    public void ApplyClientBehavior(OperationDescription operationDescription, ClientOperation clientOperation)
    {

    }

    public void AddBindingParameters(OperationDescription operationDescription, BindingParameterCollection bindingParameters)
    {

    }
}

Only mapping the invoker, nothing more except that we are resolving the IAmInvoker with Ninject.

Now we need a service behavior to apply the operation behavior to our service like the error handling stuff:

public class ServiceLoggingBehavior : IServiceBehavior
{
    private readonly IKernel _kernel;

    public ServiceLoggingBehavior(IKernel kernel)
    {
        _kernel = kernel;
    }

    public void Validate(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
    {
    }

    public void AddBindingParameters(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase, Collection<ServiceEndpoint> endpoints,
        BindingParameterCollection bindingParameters)
    {
    }

    public void ApplyDispatchBehavior(ServiceDescription serviceDescription, ServiceHostBase serviceHostBase)
    {
        foreach (var operation in serviceDescription.Endpoints.SelectMany(endpoint => endpoint.Contract.Operations))
        {
            operation.Behaviors.Add(new LoggingOperationBehaviorWithNinjectKernel(_kernel));
        }
    }
}

More or less the same as above nothing new here, map it in our factory and lets see what we have got:

public class WorkingServiceHostFactoryWithNinject : NinjectServiceHostFactory
{
    private readonly IKernel _kernel;

    public WorkingServiceHostFactoryWithNinject()
    {
        _kernel = new StandardKernel(new WorkingServiceNinjectModule());

        // SetKernel is a static method in Ninject WCF!
        SetKernel(_kernel);
    }

    protected override ServiceHost CreateServiceHost(Type serviceType, Uri[] baseAddresses)
    {
        var serviceHost = base.CreateServiceHost(serviceType, baseAddresses);

        serviceHost.Description.Behaviors.Add(new ErrorHandlerBehaviorWithNinjectKernel(_kernel));
        serviceHost.Description.Behaviors.Add(new ServiceLoggingBehavior(_kernel));

        return serviceHost;
    }
}

Pretty clear stuff and still no extra code in our service implementation!

About unit testing

As the others mentioned above, we are not really unit testing service implementations because, as you see, it contains a lot of infrastructure stuff in it. These are small parts which can be tested but mostly it isn't worth it.

Now you only have to test your NotStaticFacade class and not the service itself because it's just a stub. Of course it can contain code parts which are needed to be tested (for example the service using EF entities and transforms them to DTO instances) but now you can test these thing simpler:

Create an interface for the NotStaticFacade so you can fake it out with a simple implementation and modify your service constructor to accept the interface (ICoreStuff or whatever) not the concrete implementation. If you created a stub for the interface you can have unit tests to check your service output.

And of course you can still have integration tests to check the whole service behavior and in those tests you can set up different or no logger for example.

I'm not sure the things above will work as the way they are because I haven't tested it, but the main implementation steps are covered in the code examples.