Easy Print PHP From Data array or objects

Readability issues

After staring at your code for about 5 minutes, I wasn't able to figure out what it was doing. So I copied it over to a PHP file and started testing it out. After some time I found out that it is replacing [name] in the HTML with the name key in an array provided as the $data parameter.

Your function names single() and multiple() are very non-descriptive. If I saw a piece of code doing PrintEasy::single($html, $data); I'd have no clue what is was doing other than printing some stuff. You want a line of code like that to be as descriptive as possible. This starts at naming your class and functions better.

On another note, I do not find the $count parameter on your second function very useful. Why wouldn't I just pass in a smaller array? The parameter most likely will only cause confusion. Furthermore, why not use the splat operator from PHP 5.6 to combine both of these methods into a single method with the same flexible functionality? This would result in a function like this:

function insert($html_string, ...$data) { // code

The name of your class is somewhat descriptive, as it does state that it will be printing something. However, we will be removing the echo with a return in a bit, so the class itself will no longer print something. So we need to come up with a better name for it. In this short period of time (good names can take time) I came up with VariableInserter, TemplateVariableInserter and PlaceholderReplacer.

I think I will be going with PlaceholderReplacer, as that is the only name that clearly indicates that the original text contains some placeholder strings that are going to be replaced.

Functionality

Your code does seem to work pretty well though. One tiny error that I noticed is that when your HTML contains for example [name], but there is no data object/array that contains this key, an undefined index/property NOTICE is raised.

Also, your PrintEasy class already decides for me what I want to do with the result. What if I want to use var_dump() instead, of if I want to use this functionality for an entirely different purpose? That is currently not possible with your implementation. Why not create a class that simply returns the replaced HTML. This makes it way more flexible. And if you do actually want to echo it, you just place echo in front of the function call.

Something else that struck me while looking at your code is that the implementation of your class is way more general than just for HTML. It can replace variables in any string. So why name it in such a specific way?

Rework of the class

Now that we have some better names for our class and functions, we can start working on improving the current code.

class PlaceholderReplacer {
    /**
     * Replaces placeholder strings such as "hello [name]" with
     * the 'name' key/property from an array/object
     *
     * @param string $string
     * @param array|object ...$data_sources
     * @return string
     */
    public static function replaceWithData($string, ...$data_sources)
    {
        $new_string = preg_replace_callback(
            '/\[([^\]]+)\]/',
            function ($match) use ($data_sources)
            {
                return self::retrieveMatchValueFromDataSources($match, $data_sources) ?: $match[0];
            },
            $string
        );

        return $new_string;
    }

    /**
     * @param array $match
     * @param array $data_sources
     * @return mixed|null
     */
    private static function retrieveMatchValueFromDataSources(array $match, array $data_sources)
    {
        $key = $match[1];

        foreach ($data_sources as $data_source)
        {
            if (is_object($data_source) && isset($data_source->$key))
            {
                return $data_source->$key;
            }

            if (is_array($data_source) && isset($data_source[$key]))
            {
                return $data_source[$key];
            }
        }

        return NULL;
    }
}

As you can see, I have extracted another private method from your original method. This method is responsible for trying to retrieve a key from any of the given data sources. By splitting up this functionality into a separate function your code becomes more readable and maintainable.

I have also added isset() checks before attempting to retrieve data from the data sources to prevent a NOTICE from being raised.

Readability of class usage

Now compare the use of your class to that of the class above. Assume these variables exist:

$html = "
 Hello [name]!
";
$data_source = ["name" => "Chuck Norris"];

Now consider

PrintEasy::single($html, $data_source);

versus

echo PlaceholderReplacer::replaceWithData($html, $data_source);

Now you might argue that the functionality of your PrintEasy::single() method could be derived from looking at the $html and $data_source variables and I would somewhat agree. But keep in mind that often you don't directly see the contents of a string like this (think of a piece of text retrieved from a database). This would make it much harder to figure out what PrintEasy::single() is actually doing.

Reusability

Furthermore, without echoing the new string directly in the function but returning it, the functionality has become much more versatile. For example, I could rebuild your class using the new PlaceholderReplacer class:

class PrintEasy {
    function single($html, $data)
    {
        echo PlaceholderReplacer($html, $data);
    }

This example shows that there was some functionality in your class that could be extraced to a class of its own, with a single responsibility (SRP). Now all that is left of the printEasy class is to echo the result of another class. Now that just seems silly, so a dedicated PrintEasy class is no longer needed since we can just use echo outside of the class.

Testing

Another great aspect of returning instead of echoing is that it makes testing your code much easier.

Consider the following test:

class PlaceholderReplacerTest {
    testSimpleExample() {
        $html = "
 Hello [name]!
";
        $data_source = ["name" => "Chuck Norris"];

        $new_html = PlaceholderReplacer::replaceWithData($html, $data_source);
        $this->assertEquals("
 Hello Chuch Norris!
", $new_html);
    }
}

Your class did not return anything, but echos it directly, making it harder to test. (Not impossible however, since you could use ob_start() and ob_get_clean() to buffer the output and get the return value, but that is generally not something you want to be working with when testing other functionality).


More of a golf question than a CodeReview question really,

I would first consider

return (c
  ? someFunc.apply(this, arguments) || undefined
  : undefined)

The interesting part is that if otherFunc is not a function, then you would have executed it as a function anyway..
Furthemore,

I like better someFunc instanceof Function than typeof someFunc === "function"
For something this fancy I would go for the Spartan f
I tend to avoid void except for bookmarklets

so perhaps something like this:

//Unless f is a function and returns a non false value, return undefined
return (f instanceof Function ? f.apply(this, arguments) : undefined )
       || alternativeFunction.apply(this,arguments );


odd' seems like a useless function, you never use its result, only its monadic context. Use a guard on fermat instead.

fermat n | odd n     = Just $ -- ...
         | otherwise = Nothing

Don't use snake_case in Haskell (get_perfect_sq). We're a camelCase language.

Instead of writing your recursive functions explicitly, try to use higher order functions out of Prelude. They more clearly express intent to readers of your code by highlighting common patterns. In this case, having read Wikipedia's page on Fermat's factorization method, the pseudo-code given in the Basic Method section shows that the primary operation of the function iterates on changing values of \$a\$ and \$b2\$. In Haskell we can use iterate to produce an infinite lazy stream of values, and find to grab the first that meets a condition.

-- given odd n
let a  = ceiling . sqrt $ fromIntegral n
    b2 = a * a - n
in find squareb2 $ iterate step (a, b2)

I'd also be sure to strap a comment to the top with the pseudo-code or algorithm you're trying to implement. It aids understanding and error recognition far easier.

{-
From https://en.wikipedia.org/wiki/Fermat%27s_factorization_method

FermatFactor(N): // N should be odd
  a ← ceil(sqrt(N))
  b2 ← a*a - N
  while b2 isn't a square:
    a ← a + 1 // equivalently: b2 ← b2 + 2*a + 1
    b2 ← a*a - N // a ← a + 1
  endwhile
  return a - sqrt(b2) // or a + sqrt(b2)
-}
fermat :: Integer -> Maybe (Integer, Integer)
fermat n | odd n     = Just . solutions $ find squareb2 (iterate step (a_0, b2_0))
         | otherwise = Nothing
  where
    a_0  = ceiling . sqrt $ fromIntegral n
    b2_0 = a_0 * a_0 - n

    step (a, b2) = (a + 1, b2 + 2 * a + 1)

    squareb2 = isSquare . snd

    solutions (a, b2) = let b = floor . sqrt $ fromIntegral b2 in (a - b, a + b)


Truncating text with jQuery but keep the HTML formatting

First of all, I would just like to say that this is a really good and useful function. From a Code Review standpoint, there are almost no errors in it that I know of. Here are a few things I found from examining it:

Keep spacing uniform
Line 8 doesn't have a space between parameters, where your other function calls do. This is most likely due to quick typing and not any major issue other than a cleanliness nitpick.

An invalid input for if
Lines 19-21 where you have an if like so:

if (missCount == 0) {
  // We got text enough, stop looping.
  return false;
}

You should never use == over === due to it being possible that something like "0" would match the same as 0. This is because the double equals signs finds if it matches an exact value, where triple equals signs tests for exact value and type. So your final code should look like this for the if statement:

if (missCount === 0) {
  // We got text enough, stop looping.
  return false;
}

Optional - JSHint
If you use JSHint in JSFiddle, then you'll run into errors when trying to run this:

elem.outerHTML.match(/^<[^>]+>/m)[0]
  + grabText
  + '</' + elem.localName + '>',

If you're worried about that then you just have to write it all on one line. But for being short and concise, that might not be what you want.


Using an Angular factory to run data through logic across controllers

You could define your 'service' on the service provider instead of the factory, or put one in between. Any property exposed on the service can be referenced in the controller, and whatever the service does with that property will be reflected in the controller. That's one of the differences between factory and service.

Take a look at this plunker for an example.


ASP.NET MVC using an N-Tier Model and DDD

I agree that exposing IRepository<> to the presentation layer is bad.

The reason it's bad is that your Business Logic Tier (BLT) might want to enforce business-y rules on the access of data. That's what the business layer is for, after all.

Your heart is in the right place when you restrict use of the repository to .Where in the presentation layer. However, you still have that entries field exposed for another programmer to come along and abuse later. To prevent this from happening, you could expose only an Expression in the presentation layer. Pass that Expression to the BLT, which would then instantiate the IRepository<> and use the Expression. That would also provide a hook for the BLT to add additional conditions to the Expression for access control, etc.


SICP - exercise 2.69 - generate a huffman tree from a set of ordered leaves

Instead of using (= (length leaves) 1) to measure the list, consider
just using (null? (cdr leaves)) instead, that way it's a constant
operation.

Otherwise, if you're not using destructive operations (to improve
adjoin-set), this looks as good as it can get.

Also make sure to have the correct indentation (if that wasn't caused by
pasting), e.g.:

(define (successive-merge leaves)
  (if (null? (cdr leaves))
      (car leaves)
      (successive-merge
       (adjoin-set
        (make-code-tree (car leaves)
                        (cadr leaves))
        (cddr leaves)))))

Also take a look at this post maybe, one of the posts regarding the same exercise.


Does scipy.spatial.distance.cdist run multiple cores in parallel on your machine ?
On my mac with Accelerate framework, it runs all 4 cores,
but equivalent numpy seems to run only 1.
(This is regardless of VECLIB_MAXIMUM_THREADS, which I don't understand.)

If the big matrix doesn't change very often, normalize it once outside the loop:

A /= np.linalg.norm( A, axis=1 )  # <-- once

def cosdist( A, b, dtype=np.float32 ):
    """ (A . b) / |b| -- A normalized """
    Adotb = A.dot(b) /
    Adotb /= np.linalg.norm(b)
    return Adotb

Since A.dot(b) and norm(A) take roughly the same time,
this runs about twice as fast -- on 1 core.

How much memory do you have ? 1M x 300 x 8 bytes is 2.4 Gbytes,
which may be a reasonable chunk size; 4.5M, 11Gbytes, will be memory-bound. Can you monitor memory usage / swapping ?

Using np.float32 instead of the default np.float64
might be faster, or allow bigger chunks.
But scipy cdist seems to convert float32 to float64, slower.

Numpy and scipy link to libraries for
BLAS,
Basic Linear Algebra Subprograms.
These libs are usually vendor-tuned, faster than C or Cython loops.
To see what numpy / scipy link to:

from numpy import __config__
print "numpy blas:\n", __config__.get_info( "blas_opt" )

from scipy import __config__
print "scipy blas:\n", __config__.get_info( "blas_opt" )

Even though these are the same (Accelerate framework on my mac),
numpy and scipy have different wrappers around BLAS
(numpy/linalg/linalg/lapack_lite.so, scipy/linalg/cython_blas.so)
with different overheads.

Summary: numpy / scipy dot and norm to BLAS
to multiple cores is a messy business.
Don't trust any runtimes that you haven't run yourself.


Finding the second largest element of large input sets

So your code seems to work fine, but you get a non-zero exit code for some unknown inputs... What could go wrong? If I call out to you 100 numbers, do you need to remember all of them to tell me the second highest? (inside an array in your head)
You don't. And the program doesn't either.
Most probably the program crashes with an out of memory error.

Here's one way to solve this using constant storage:

    while (testcase-- > 0) {
        long num;
        long largest = Long.MIN_VALUE;
        long secondLargest = Long.MIN_VALUE;
        while ((num = sc.nextLong()) != -1) {

                  if (num > largest) {
                     secondLargest = largest;
                     largest = num;
                } else if (num > secondLargest) {
                     secondLargest = num;
                }

        }
        System.out.println(secondLargest);
    }

Code review

Your code had serious formatting issues. Cleaned up, your code would look like this:

    while (testcase-- > 0) {
        List<Long> numbers = new ArrayList<>();
        long num;
        while ((num = sc.nextLong()) != -1) {
            numbers.add(num);
        }
        Collections.sort(numbers);
        if (numbers.size() == 0) {
            System.err.println("");
        } else if (numbers.size() == 1) {
            System.out.println(numbers.get(0));
        } else {
            System.out.println(numbers.get(numbers.size() - 2));
        }
    }

What changed?

Formatting fixed
Use interface type instead of implementation (List)
Removed redundant variables
Removed redundant initializations
Removed redundant parentheses
Improved variable names


Function to determine the appropriate dimensions for a close button

First, a few general notes about types.

Don't use String and Boolean. Use string and boolean instead. See the Typescript Handbook
If you know what mediaType must be, type it to require this.

type Media = 'VIDEO' | 'BANNER';

let m1: Media = 'VIDEO'; // OK
let m2: Media = 'video'; // Error: Type '"video"' is not assignable to type 'Media'.

Now on to your function. I believe your biggest problem here is that you are trying to do too much in a single function. As it appears you are using this in a class which can have two different types -- VIDEO and BANNER. You likely have other similar methods where the behavior is slightly different for each case. An abstract parent class for common functionality with a separate class for BANNER and VIDEO elements might a good idea.

To avoid assuming too much, I have left all of the logic in a single method for now, though splitting this up would greatly help with the method length.

The way I would do this is to avoid if statements as much as possible to make it simpler to extend the configuration in the future. With your current approach, what happens if the customer says they want to add a video ad with the dimensions of 300x300 on desktop? With this in mind, here is the approach I would take. It is a bit longer than your approach, but the cyclomatic complexity is 6, and adding further configurations does not increase the complexity. Splitting this up into two separate methods getCloseButtonBannerDims and getCloseButtonVideoDims is also a trivial change to shorten the method.

get CloseButtonDimensions(): Size {
    const IS_MOBILE = isMobile();

    let mediaType: Media = this.options.tag.media_type;
    let adWidth: number = this.options.tag.size.width;
    let adHeight: number = this.options.tag.size.height;

    // Sizes must be listed in decreasing order for this to work
    let sizes: Array<Size & {adWidth: number, adHeight: number}>;

    if (mediaType == "VIDEO") {
        if (!IS_MOBILE) {
            // Desktop:- "640x480", "480x360", "300x250"
            sizes = [
                { adWidth: 640, adHeight: 480, width: 15, height: 15 },
                { adWidth: 480, adHeight: 360, width: 15, height: 15 },
                { adWidth: 300, adHeight: 250, width: 11, height: 11 },
            ];
        } else {
            // Mobile:-  "300x250", "320x240"
            sizes = [
                { adWidth: 300, adHeight: 250, width: 11, height: 11 },
                { adWidth: 320, adHeight: 240, width: 13, height: 13 },
            ];
        }

        // Find the first ad where both the width and the height are
        // Greater than or equal to the expected size
        let {width, height} = sizes.find((size) => adWidth >= size.adWidth && adHeight >= size.adHeight);

        return { width, height };
    }

    // We know mediaType must be BANNER as we handled all VIDEO types above
    if (!IS_MOBILE) {
        // Desktop:- "728x90", "468x60", "300x250"
        sizes = [
            { adWidth: 728, adHeight: 90, width: 13, height: 13 },
            { adWidth: 468, adHeight: 60, width: 11, height: 11 },
            { adWidth: 300, adHeight: 250, width: 11, height: 11 },
        ]
    } else {
        // Mobile:-  "300x250", "320x50", "300x50"
        sizes = [
            { adWidth: 300, adHeight: 250, width: 11, height: 11 },
            { adWidth: 320, adHeight: 50, width: 8, height: 8 },
            { adWidth: 300, adHeight: 50, width: 8, height: 8 },
        ];
    }

    let {width, height} = sizes.find((size) => adWidth == size.adWidth && adHeight == size.adHeight);

    return { width, height };
}

For your more general questions, if you have a chain of if statements where all (or even several) check the same thing (!IS_MOBILE and mediaType == "BANNER"), it is probably a good idea to pull that check out and test it by itself before checking the other conditions (dimensions in this case). It might even be worth pulling all of the checks after that condition into their own method. The guiding principle here is to avoid doing more than one thing at once.

As for making code more readable for junior developers, trying to avoid doing more than one thing at once will help a lot. Try to make it so that when you go back in 6 months and revisit the code, there is a clear path through it. The fewer things you have to remember to understand a line of code, or a method, the better.


Finding all plurals in an array of Strings

The code is well readable as it is. As long as it solves the problem (it does), it is fine.

For a real-world plural detection, I would add to the class:

  public static boolean isPlural(String s)

to detect if a single word is a proper plural, because it is more complicated than 'ends with "S"'.


Using getopt parse and validate command line arguments

I think you have too many try/excepts.  More than one of them raise and catch their own exceptions. I've made some changes that improves readability.

For clarity the new code created a new exception class and raises it when appropriate.

Also using getopt's own handler for bogus input. The else in the main loop is redundant but it can also serve as a fallback in case you make a mistake by allowing an arg but forget to process it.

Pay attention to the wording:
if target_year not in map( ...

reads better than:
if not target_year in map( ...

#!/usr/bin/python
'''\nMailing Script
Usage: python script.py -y <year> -c <type_user>"
Arguments:
 -y, --year       any value from 2008 to 2013
 -c, --campaign   any value in:
                    '80c', '80d', 'allowances', 'late_filing',
                    'exempt_income', 'other_sources'
Flags:
 -h      help
'''
import sys
import getopt
first_yr, last_yr = (2008, 2013)
campaign_type = (
    '80c', '80d', 'allowances', 'late_filing',
    'exempt_income', 'other_sources',
)

class ArgumentError(Exception):
    pass

def usage():
    print sys.exit(__doc__)

def parse_arguments(argv):
    target_year = None
    campaign = None

    try:
        opts, args = getopt.getopt(argv[1:], "y:c:", ["year=", "campaign="])
    except getopt.GetoptError as err:
        # print help information and exit:
        print str(err)  # will print something like "option -a not recognized"
        usage()

    for opt, arg in opts:
        if opt == '-h':
            usage()

        elif opt in ("-y", "--year"):
            target_year = arg
            if target_year not in map(str, range(first_yr, last_yr + 1)):
                raise ArgumentError("Argument Error: Invalid year passed. \n "
                                    "One valid year is within (%s, %s)" % (first_yr, last_yr))

        elif opt in ("-c", "--campaign"):
            campaign = arg.lower()
            if campaign not in campaign_type:
                raise ArgumentError("Argument Error: Invalid 'Campaign type' passed. "
                                    "A valid value can be any of %s" % str(campaign_type))
        else:
            raise ArgumentError("Bad argument: I don't know what %s is" % arg)

    if target_year is None or campaign is None:
        raise ArgumentError("You need to supply both -y and -c")

    print 'Year : ', target_year
    print 'campaign: ', campaign

    # return argument values
    return target_year, campaign

if __name__ == "__main__":
    target_year, campaign = parse_arguments(sys.argv)


Benchmarking inline vs normal functions in C++

Inlining

First of all, your inline function calls the non-inline function, so if the compiler is really paying attention to the inline specification, it's only affecting a single "layer" of invocation.

inline long fib(unsigned n) {
    if (n < 2) return n;
    return fibonacci(n - 1) + fibonacci(n - 2);
}

To stand a chance of getting some good out of the inline specification, you probably want to have this call itself instead of the other function:

inline long fib(unsigned n) {
    if (n < 2) return n;
    return fib(n - 1) + fib(n - 2);
}

In general, recursive functions are a problem for inlining in any case, so many compilers completely disable inline code generation for recursive functions.

Add in the fact that most reasonably current compilers basically ignore the inline specifier anyway, and you get a high likelihood that you won't see any difference between these functions at all.

Timing

I'd do the timing somewhat differently. The single responsibility principle applies here, just like everything else. That means the timer should deal only with timing. I usually use something on this general order:

template <typename F, typename ...Args>
auto timer(F f, std::string const &label, Args && ...args) {
    using namespace std::chrono;

    auto start = high_resolution_clock::now();
    auto holder = f(std::forward<Args>(args)...);
    auto stop = high_resolution_clock::now();
    std::cout << label << " time: " << duration_cast<microseconds>(stop - start).count() << "\n";

    return holder;
}

Arguably this already does more than it should (printing out results in addition to the actual timing) but at least it's quite a bit closer to a single responsibility.

With that, we invoke each Fibonacci generator separately, something on this order:

auto foo = timer(fib, "inline", max);

The result will then look something like this:

inline time: 1062

Just a minor note: this is a somewhat more general timing function that most people usually need. In particular, it doesn't require the function being timed to take a specific number of arguments--the number and types of arguments you pass after the label have to match those needed by the function you're timing, but as far as the timer itself cares, it's essentially wide open.

Efficiency

Ignoring, for the moment, the issue of inlining (which is probably pretty much a red herring anyway), I'd consider other ways of computing Fibonacci numbers. In the absence of memoization, a recursive function is horribly inefficient. An iterative function is many times faster. That can be written with a for loop, something on this general order:

unsigned long long fib(unsigned limit) {
    unsigned long long a = 1;
    unsigned long long b = 1;
    unsigned long long c;

    if (limit < 2)
        return 1;

    for (auto i=1ULL; i<limit; i++) {
        c = a + b;
        a = b;
        b = c;
    }
    return c;
}

This is somewhat longer, but the difference in speed is...fairly substantial. Computing fib(43) with each, I get timings like this:

out of line time: 2807407170
     inline time: 2794785991
  iterative time: 321
Sum (ignore): 1568397607

I had to switch to showing the time in nanoseconds to get a non-zero time for the iterative solution. Even so, for fib(43) and highest resolution-timing I can get, it still shows a time of 0 about one run out of every 3 or 4. The recursive versions are approximately 7 orders of magnitude slower (and although inline code generation did seem to help a tiny bit, it's still minuscule compared to the improvement from a better algorithm. Oh, and for what it's worth, it looks like the inline code generation really did make a difference. It is pretty small, but at least in my testing, the inline version does finish just a tiny bit faster every time.

Final Code

For what it's worth, here's the code I ran to get the timing comparison:

#include <iostream>
#include <string>
#include <chrono>

template <typename F, typename ...Args>
auto timer(F f, std::string const &label, Args && ...args) {
    using namespace std::chrono;

    auto start = high_resolution_clock::now();
    auto holder = f(std::forward<Args>(args)...);
    auto stop = high_resolution_clock::now();
    std::cout << label << " time: " << duration_cast<nanoseconds>(stop - start).count() << "\n";

    return holder;
}

long fibonacci(unsigned n) {
    if (n < 2) return n;
    return fibonacci(n - 1) + fibonacci(n - 2);
}

inline long fib(unsigned n) {
    if (n < 2) return n;
    return fib(n - 1) + fib(n - 2);
}

unsigned long long iter_fib(unsigned limit) {
    unsigned long long a = 1;
    unsigned long long b = 1;
    unsigned long long c;

    if (limit < 2)
        return 1;

    for (auto i = 1ULL; i < limit; i++) {
        c = a + b;
        a = b;
        b = c;
    }
    return c;
}

int main() {
    static const int max = 43;
    auto a = timer(fibonacci, "out of line", max);
    auto b = timer(fib,       "     inline", max);
    auto c = timer(iter_fib,  "  iterative", max);
    std::cout << "Sum (ignore): " << a + b + c;
}


If the parameters are required then they need to be checked and their invalidity must be reported to the caller. There is a HTTP status code for it: 400/BAD REQUEST.

The file transfer is done one byte at a time. This is very inefficient. This kind of transer is usually done in 4096 byte blocks. There are libraries for this task too. For example Apache Commons IO IOUtil.copy(InputStream, OutputStream).

The content type mappings are inefficient. A Map would be a natural fit for the use case instead of a 2D array. But this is something you don't need to do yourself: https://stackoverflow.com/questions/51438/getting-a-files-mime-type-in-java

Calling the servlet repeatedly is called polling. A polling service should be quick and lightweight to not cause excessive load. The client should implement waiting between the polling. The servlet doesn't really need to know about the polling interval. If the servlet should implement throttling of "too eager clients", then you need to implement authentication to identify clients. Depending on the infrastructure you have, you might want to use a firewall or an API gateway for throttling.

By creating the file path by concatenating input from a URL parameter to a path constant you allow the caller to add directories to the path name. E.g. a file name "../../some/sensitive/file.txt" might reveal data you want to keep hidden. It is safer to get a file list from the directory with File.list(FileNameFilter) and use the filter find the file that matches the requested file. Other approach is to sanitize the input, but in mny opinion this is a much more robust solution. If there are a lot of files in the directory and there are a lot of requiests, you might want to cache the file list for a few seconds.

Closing the servlet output stream may have unwanted side effects, so you should just leave it to the servlet container.


Would you utilise "Using" on a member which is going to be added to a list?

No, you shouldn't use using on the object that you add to the list.

When you add the object to the list, it's just the reference to the object that gets added. There is no copy of the object created for the list.

If you use using on the object, it will be disposed at the end of the code block, which is a single statement in your example. Right after you have added the object to the list it will be disposed.

Although nothing happens automatically when you dispose an object, it's customary that the object should be unusable after the Dispose method has been called. The IDisposable interface is intended for controlling the lifespan of objects, usually to free unmanaged resources when you are done using the object.

After you are done using the objects in the list, you should dispose them.

However, from what I can tell there isn't going to be any unmanaged resources in your object, so there wouldn't be any need for it to implement IDisposable. The (supposedly) database related objects are only used during the creation of the object, and so far there are nothing unmanaged in the class.


Using synchronized blocks and volatile variables to ask a Java thread to terminate gracefully

Find a copy of Java Concurrency in Practice, it's got very good chapter on Cancellation and Shutdown.

You've made cancelProcess() private, so your outside threads aren't going to do anything to prevent process.waitForResponse() from blocking.  That's no good - let's make it public.

In addition to that, trying to cancel a running process by setting a volatile variable is an anti-pattern.  For your specific example, consider what happens if the external thread tries to cancel while process.waitForResponse() is running.  It doesn't do much good, does it -- ExecuteProcess can't check processKilled until the waitForReponse returns, and by then there is no point, you've already been forced to wait.

  Interruption is usually the most sensible way to implement cancellation.

As noted above, you can write an entire chapter on that problem; I'll spare us all that.  What that quotation does mean, though, is that task cancellation is a lousy example for learning about volatile variables.

Code Review

There's already a standard interface called Runnable; it's appropriate to use it here.  The runnable method returns a void though, so you should consider other ways to signal success and failure.  An alternative to Runnable would be a Callable<T>, which supports returning values.

  The Callable interface is similar to Runnable, in that both are designed for classes whose instances are potentially executed by another thread. A Runnable, however, does not return a result and cannot throw a checked exception.

Creating a process is a separate responsibility from running a process.  You should probably inject the process to be run, via the constructor.  The class might then look like:

public class ExecuteProcess implements Callable<Boolean> {
    private final Process process;

    public ExecuteProcess(Process process) {
        this.process = process;
    }

    Boolean call () {
        ...
    }
}

The advantage of Runnable and Callable: you can now take advantage of ExecutorService.submit(), which returns a Future.  Future.cancel(), under the covers, arranges for Thread.interrupt() to be called the right way.

Having done that, you can now clean up ExecuteProcess.  Instead of checking synchronized state, your process code is responsible for checking to see if the thread has been interrupted...

Executor Service and Future are part of the java.util.concurrent library; a library that gets the little details right so that you don't have to.  You can find sources on line if you want to look at the details for yourself: FutureTask.Sync.innerCancel might be a good starting point.

Also, it's really poor form to return null in a function that promises a boolean.


The first thing to do before making any changes to this code, is to improve its readability. I saw .Cells and thought "oh we're in a With block" ...and then had to look three times to find the With statement.

The keyword here, is indentation.

Sub DoSomething()
....
....With SomeObject
....|...
....|...If SomeCondition Then
....|...|...DoActionOne
....|...Else
....|...|...DoActionTwo
....|...End If
....|...
....|...Do
....|...|...DoActionThree
....|...Loop
....|...
....End With
....
....For i = 1 To 10
....|....
....|....DoActionFour
....|....
....Next
....
End Sub

When the Else blocks don't line up with the corresponding If statement, or when a Loop keyword doesn't line up with its corresponding Do keyword, or when nested blocks line up in column 1, you basically set yourself up for making a change that introduces a bug.

Proper indentation cannot be underestimated.

You have redundant object references:

If WB.Sheets("RawData") has CodeName Sheet1, name it RawDataSheet and use that reference instead.
If WB.Sheets("Conso") has CodeName Sheet2, name it ConsoSheet and use that reference instead.

"CodeName" is a property of all sheet objects in Excel VBA; there's a global object reference pointing to these, readily available for you to use - no need to fetch it from WB.Sheets collection, which by the way could give you non-worksheet objects, since the Sheets collection includes Charts, among other sheet types. You probably meant to use the WB.Worksheets collection instead. But then again, you don't need it - just use the global object VBA gives you for free instead.

Application.ScreenUpdating = False

Whenever you set that value to False, you need to handle runtime errors and make sure that whatever happens, the method can't exit without setting it back to True. Otherwise Excel will look "frozen" when it's actually completely responsive, just not redrawing itself... because you told it not to.

Don't assume things won't blow up. They always do.

Sub DoSomething()

    On Error GoTo CleanFail

    Application.ScreenUpdating = False

    '...

CleanExit:
    Application.ScreenUpdating = True
    Exit Sub
CleanFail:
    'handle errors
    Resume CleanExit
End Sub

Now that's great, but you're deleting rows, which should trigger a recalculation if calculation is set to xlAutomatic. Consider not only turning off ScreenUpdating, but also setting Calculation to xlManual while you're doing your thing.


I would use simply HasEmployees.

In case of Boolean, I always try to use hasSomething or isSomething. In Java, many libraries use this strategy (it's just a convention). And I like it, because it's so easy to read when you see:

if(annoyingEmployee.isRemovable() && !annoyingEmployee.hasBirthday()) {
  manager.fireEmployee(annoyingEmployee);
}


What you have written, is not exactly the Sieve of Erathostenes.
When doing the sieve, you don't do any divisions; you just step through all the numbers and cross off multiples.
The sieve doesn't find the n'th prime, but rather all primes up to a limit.
This is an example of how to do the sieve. It's a very basic version.

public static IEnumerable<int> SieveOfErathostenes(int upperLimit)
{
    //BitArray works just like a bool[] but takes up a lot less space.
    BitArray composite = new BitArray(upperLimit);

    //Only need to cross off numbers up to sqrt.
    int sqrt= (int)Math.Sqrt(upperLimit);
    for (int p = 2; p <= sqrt; ++p) {
        if (composite[p]) continue; //The number is crossed off; skip it

        yield return p; //Not crossed off means it's prime. Return it.

        //Cross off each multiple of this prime
        //Start at the prime squared, because lower numbers will
        //have been crossed off already. No need to check them.
        for (int i = p * p; i < upperLimit; i += p)
            composite[i] = true;
    }
    //The remaining numbers not crossed off are also prime.
    for (int p = sqrt + 1; p < upperLimit; ++p) {
        if (!composite[p]) yield return p;
    }
}


Batch/PowerShell script that toggles the minimized state of a window

It took a bit to wrap me head around what you have here. It certainly is a piece of work. Rather clever what you have done in order to have a file that runs batch and PowerShell. Bravo sir! It also seems you like doing this with other languages as well!
I will first try to address your questions first and then some other points I feel worth mentioning.

In practice there are of course better options until you put in the minimized exception. In my minimal practice of you code and the suggestions you tried I think, in general, your approach is required. This is the weakest part of the review as I do not have much to offer here.

Is there any situation where FindWindowEx() would not be able to find a window that could be found with FindWindow()? Refering to the documentation for FindWindow() I did find some evidence which could convince you to stick with FindWindowEx()

Retrieves a handle to the top-level window whose class name and window name match the specified strings. This function does not search child windows. This function does not perform a case-sensitive search.

That might not change anything but depending on the target of the script it might not find the window you are looking for if you use FindWindow().

There is a few things to touch on. Most are just FYI. For the ones that are more than that I will try and draw a little more attention to them.
Invoke-Expression
The hybrid-ness of your script relies on this. I am sure that you do it so that it is self contained. However, in case no one mentioned it: Invoke-Expression can be evil. While I don't take that to heart it is potentially dangerous to use as someone can inject code into that file and it will execute blindly.
If you are willing to have a .ps1 file that gets called it would be safer. That is my tactful way of addressing "Why not just write the whole thing in PowerShell?"
Don't really expect you to adhere to the advice. Just wanted you to be aware.
Comments
This is a big one since you are giving this to someone else. This script is lacking of comments. While I do not code much in batch the PowerShell is lacking and some of the idiosyncrasies of this script were lost on me until I looked up the documentation. While you might know what is going on now you might not months/years from now.
For instance I had to look up to documentation for the line $hwnd = (ps $env:prog)[0].MainWindowHandle. Something simple like
# MainWindowHandle will return 0 if the process in hidden/minimized.

Again that could just be me but in general this is lacking context comments.
Get-Content
I see that you are reading the file in as one string using a -join. I would think that you have at least PowerShell v3.0. Either way there are cleaner options for reading the file in as one string.
# PowerShell v3.0+
"iex (gc \"%~f0\" -Raw)"
# PowerShell 2.0
"iex (gc \"%~f0\" | Out-String)"

Determining Title
Parsing cmd output is a specialty of mine. From the looks of it you are banking on Window Title being the last line of the output. While I doubt there will ever be a problem I can not say that nothing would go wrong with that logic.
With what little it would take to get more reliable results I would go so far as to change that.
$titles = tasklist /v /fi "imagename eq Keepass*" /fo list |
        Where-Object{$_ -match "^Window Title:\s+?(.*)"} |
        ForEach-Object{$Matches[1]}

Notice I also used $titles since you could have multiple matches. The way you had it before would just take the last one. Could do just the same with a Select -First 1

Again, I am impressed with the script you have set up here.


I don't see anything wrong with the generated query. EF does generate verbose T-SQL, but that doesn't mean it's inefficient. If the generated query generates a decent execution plan there is no need to try to get EF to generate T-SQL the way you would have written it.

However the code would be easier to read like this:

var orders = await db.Orders.AsNoTracking()
                            .GroupBy(x => x.Status)
                            .Select(g => new { Status = g.Key, Count = g.Count() })
                            .ToDictionaryAsync(x => x.Status, x => x.Count);


Toggle four relays on/off based on bitmap stored in uint8_t

Use a loop to eliminate all that copy-pasta and get rid of some superfluous tests, branches and hard-coded constants:

void relayToggle(uint8_t relayBitmap)
{
    for (int r = 1; r <= 4; ++r)
    {
         if (relayBitmap & 1)
             relayOn(r);
         else
             relayOff(r);
         relayBitmap >>= 1;
    }
}


C++ Trie using std::map and std::unique_ptr

I don't see the need for unique_ptr for the root node.

spNode root;

I would just make this a node.

Node root;

The use of std::map is fine. But it does have O(log(n)) lookup. If you switch back to an array its O(1). Its a time for space thing. Pick the one you want.

I don't like the two line creation of nodes.

        std::unique_ptr<Node> node(new Node());
        temp->children[c] = std::move(node);

Just use reset:

        temp->children[c].reset(new Node());

Or if you have C++14 use std::make_unique()

        temp->children[c] = std::make_unique<Node>();

Personally search() does not seem quite the correct verb.

bool search(const std::string& str);

What is wrong with find()?


Joining a product-application-customer relationship using Java 8 Streams

Using the appropriate types

Your are repeatedly casting searchResult into a Map, the first suggestion I will make is to cast it once first to make subsequent references easier to read:

// use Map<String, Object> if you want to be less strict on the value type,
// at the expense of having to do Object.toString() later
List<Map<String, String>> searchResultDetails =
                                (List<Map<String, String>>) searchResults.getResult();

Rationale: Avoid code repetition and makes the one-time cast clearer to understand

Grouping the search results into a usable Map

Instead of traversing your searchResultMap for each product, consider generating an intermediary Map that gives you the list of CustomerDetails grouped by the product ID:

Map<String, List<CustomerDetails>> customerDetailsByProduct =
    searchResultDetails.stream()
        .collect(Collectors.groupingBy(map -> map.get("productID"),
                Collectors.mapping(map -> new CustomerDetails(map.get("customerID"),
                                            map.get("appId")), Collectors.toList())));

Rationale: Consider when you have \$n\$ products and a result for each of them. Your current solution will effectively be iterating through them, and iterate through the \$n\$ results for the one matching product. Therefore, you should start with grouping your results first by the product ID, so that the retrieval later via the Map interface will potentially be more efficient.

Process the product list as the final step

With the above map, it's just a matter of looping through your products and creating an instance of ProductInformationWithAssociatedCustomer with the customer details, if present:

// BTW do you really need such long class names?
List<ProductInformationWithAssociatedCustomer> results =
    products.stream()
            .map(product -> new ProductInformationWithAssociatedCustomer(product,
                                    customerDetailsByProduct.getOrDefault(product.getId(),
                                            Collections.emptyList())))
            .collect(Collectors.toList());


Validating region/state/province codes for different countries

There's an intrinsic problem in using a scalar UDF to validate your country and region. Your scalar function essentially answers the "is it valid?" question for just a single pair of values - that might be fine in an "as-the-insert-happens" sense of a live system that only occasionally has a need for validation of a single row, but SQLServer design principles have a particular emphasis on thinking in terms of sets of data, rather than thinking of rows of data in a line-by-line fashion, requiring individual attention.

If you're performing this on a large table/lot of rows (let's say 100,000) then you could conceive that SQLServer will run your select query 600,000 times. It's going to be considerably slower to run more than half a million queries that pick out one item of data at a time, than it is going to be to run a query that joins together 100,000 rows (SQLServer is good at joining data efficiently) and then loops over it, checking some conditions and making changes.

There's a process you can follow that will allow SQLServer to more effectively pick apart your function and incorporate/integrate it with the main query more effectively, particularly in terms of processing the query in parallel - You can read more about it in blogs and stackexchange answers discussing swapping scalar UDFs for inline TVFs. Some relatively minor performance improvements can be had by compiling the function. There is also work afoot to improve SQLServer's ability to process scalar UDFs inline with the rest of the query, but at the moment it's potentially an absolute showstopper for the performance of your query

You also have the option of doing away with the function (or not using  it in this context) and doing the cleanup all in one go (and you could encapsulate this logic in a stored procedure if you wanted) :

    UPDATE t
    SET
       a_region = ra.region_code,
       b_region = rb.region_code,
       c_region = rc.region_code,
       d_region = rd.region_code,
       e_region = re.region_code,
       f_region = rf.region_code
    FROM
       table_region t
       LEFT OUTER JOIN mapping.region ra WHERE t.country = ra.country AND t.a_region = ra.region_code
       LEFT OUTER JOIN mapping.region rb WHERE t.country = rb.country AND t.b_region = rb.region_code
       LEFT OUTER JOIN mapping.region rc WHERE t.country = rc.country AND t.c_region = rc.region_code
       LEFT OUTER JOIN mapping.region rd WHERE t.country = rd.country AND t.d_region = rd.region_code
       LEFT OUTER JOIN mapping.region re WHERE t.country = re.country AND t.e_region = re.region_code
       LEFT OUTER JOIN mapping.region rf WHERE t.country = rf.country AND t.f_region = rf.region_code

It basically works by left joining the region table 6 times, once for each of the *_region columns.

If the relation works out, then the r*.region_code will be populated with a value (which means the *_region column is set to the same value it currently is, i.e. a non-op).
If it doesn't work out, then the join fails, null is present in r*.region_code

You could run this regularly as a periodic fix. You could also turn this into a SELECT query and run it, to identify the ones that are failed. If you were then to move those out (to another table) so your table only contains valid related records, you could then consider making table_region(country,*_region) have multiple foreign keys to mapping.region(country,region_code) to ensure that records cannot be inserted into table_region that don't have a valid country/region mapping. This devolves responsibility to maintaining a valid country/region pairing to the constraints mechanism of the database and it will become impossible to insert data that has an invalid country/region pairing


This code style is not very good, becouse u declare functions inside a function. The problem with this is that you recreate all this functions every time u run "shoppingCart" function.

Instead try using this pattern;

(function(){

    //my private functions delare

    function recalculateCart(){
        ...
    }

    // my module public methods
    var myModule = {
        init: ...,
        shoppingCart: ...
    }

    return myModule;

})()


Scrolling event being fired too many times

Bug!
You have a small bug in your code:
$.throttle(100, sectionScroll));

See it? You have one-too-many parentheses. This line should be:
$.throttle(100, sectionScroll);

Debounce
You say in your title that the scrolling event is being fired too many times.
Well, this makes perfect sense: as long as the user has their mouse over the scroll bar and is moving the bar ever-so-slightly, the scroll event is going to try and fire as quick as it can.
Even for the tiniest movement, this event could fire over 50 times.
However, there is a simple way to prevent your code from trying to run that many times: a debounce.
A debounce is when you setup a variable to prevent code (typically an event) from running while another piece of code (often the same code) is still running.
Every time this scroll event fires, a new thread is going to be created with the event code running in it; a debounce would prevent these from running at the same time.
It is very simple to create a debounce; you just need a variable and a conditional.
Here is what the basic design would look like:
var debounce = true;
function foo() {
    if(debounce) {
        debounce = false;
        [code]
        debounce = true;
    }
}

The first line in the conditional would stop any other thread from running because they are all sharing a single debounce variable.
Now, all you need to do to apply this to your code would be to treat foo in the example as your sectionScroll function, create the debounce variable, and then encase the function code in the if statement.
Then, after adding the lines for setting and un-setting the debounce, you should be good to go.

The above is really the meat of my review, everything else is just miscellaneous.

Indentation
You have none! Which, in my opinion, is better than having bad indentation.
Luckily for you there is a handy-dandy tool that you probably already know about: JSFiddle. At the very top of the page is a button labeled "TidyUp".

Paste your code into the "JavaScript" code box.
Give that "TidyUp" button a smack.
???
Profit!

It's really as simple as that: using that "TidyUp" button will fully and properly indent your code.
However, don't get too used to this: you want to be able to tell right from wrong without having to consult something else.
Study the changes that that button made to your code. You may want to go back and forth a couple of times. And, read other JavaScript code that has proper indentation. Over time, you should be able to develop a strong sense of what is right, and what is wrong.
Note: I actually had to use this tool in order to review your code; I couldn't read it otherwise!


Firstly, a negative remark. The power of C++ means that it is not uncommon for new C++ developers to try and make C++ look like a language they feel more familiar with - sometimes English, sometimes a programming language they already know. This is almost always a bad thing - you need to speak like a native.

However, whilst C++ used for mainstream development does have generally understood idioms, one of its capabilities and original design aims is to try to support different styles of programming.

As well as trying to create an application syntax which is more English like, you are externalizing functionality into proxy objects rather than using the OOP paradigm of member functions. IMHO, this is perfectly legitimate as an experiment.

What you'll likely find is that as you pursue this you gave to make various compromises that finally result in you deciding that maybe this is not he way to go. Or maybe not. See what you find.

If I were to go down this path, I'd probably play with using templates to try and get some generic behaviour, given that the OOP approach us not being used. How exactly this goes will depend on whatever other functionality is needed.

For example:

template<class T>
class Move
{
public:
    Move(T& thing)
        : thing_(thing)
    {}
    virtual void to(const Point& position) = 0;
    T& thing() { return thing_; }
private:
    T& thing_;
};

class MoveSquare : public Move<Square>
{
public:
    MoveSquare(Square &squareToMove)
        : Move<Square>(squareToMove)
    {}

    virtual void to(const Point &target)
    {
        thing().topLeft = offsetHalfWidth(target);
    }

private:
    Point offsetHalfWidth(const Point& p)
    {
        const float halfWidth(thing().width / 2.0f);
        return Point{ p.x - halfWidth, p.y - halfWidth };
    }
};

class MoveCircle : public  Move<Circle>
{
public:
    MoveCircle(Circle &circle)
        : Move<Circle>(circle)
    {}

    virtual void to(const Point& position)
    {
        thing().middle = position;
    }

};


Display class containing a Canvas in a JFrame

one minor issue - no redundant code...

canvas = new Canvas();
canvas.setPreferredSize(new Dimension(width, height));
canvas.setMaximumSize(new Dimension(width, height));
canvas.setMinimumSize(new Dimension(width, height));
canvas.setFocusable(false);

you should use

canvas.setPreferredSize(dim); //see below
canvas.setMaximumSize(dim);
canvas.setMinimumSize(dim);

that leads to another issue:
create a default constructor that doesn't require a size

private static final int DEFAULT_WIDTH = 100;
private static final int DEFAULT_HEIGHT = 100;
public Display(String title){
    this(title, DEFAULT_WIDTH, DEFAULT_HEIGHT);
}

and use this parametrized constructor:

//private int width, height; TODO remove: no more required
private static final Dimension dim;
public Display(String title, int width, int height){
    this.dim = new Dimension(width, height);
    createDisplay();
}

another issue would be removing unrequired attributes - whats the sense of keeping the title? just forward it...

//private String title; TODO remove: no more required
public Display(String title, int width, int height){
    this.dim = new Dimension(width, height);
    createDisplay(title);
}

private void createDisplay(final String title){
    frame = new JFrame(title);
    ...
}


Filtering a large (50gb+) JSON lines file matching CIDR's

Is there a faster way of accomplishing this? Would Multithreading/Multiprocessing speed things up? Any help or other feedback would be much appreciated!

No, for the most part, multi-threading will make no difference for you. At some point the bottleneck should be the IO speed of reading 50GB of file content, and not the speed of the processing. You also need to read the file sequentially (I presume) to get the output in the same order as the input.

But, fundamentally, the solution should not need to have multi-threaded execution in order to improve the performance.

Learning how to measure performance of various parts of your code is an important skill. For the moment, it may be as simple as timing this code:

# Load JSON line by line
with open(filename) as f:
    for line in f:
        output = json.loads(line)
        rowcount+=1

i.e. convert each line from JSON, and count the lines.... how fast is that? I would expect that the whole program should be just as fast when the IP CIDR lookups work fast too.

Your performance issue is almost certainly related to this line here:

if all_matching_cidrs(output['ip'], cidr):

Your timings already support this.... it takes 1 second to search all records in 1 CIRD, but significantly longer for 2000 CIDRs...

So, you have a performance problem in the order of \$O(mn)\$ where \$m\$ is the number of rows in the file, and \$n\$ is the number of CIDRs.

You can't improve the performance related to the number of rows in the files, but you can improve the cost of the CIDR lookups. What if it was a fixed-cost to check all CIDR matches? Then your overall performance becomes \$O(m)\$ and does not depend on the number of CIDR records.

You can do this by preprocessing the CIDR data in to a structure that allows a fixed-cost lookup.

The structure I would use is a binary tree consisting of nodes representing each bit in the CIDR specs. Each leaf node represents a CIDR to include. I.e. you preprocess the CIDRs in to the tree that at most has 32 levels (for a /32 CIDR).

Then, for the lookup, you take your IP from the JSON, convert it in to an integer, and start shifting bits from the most significant. For each bit, you start descending the CIDR tree, and if you can descend the tree until you hit a leaf node, then you have found a matching CIDR. At most, this will be 32 iterations down the tree, but for the most part, CIDR's seldom are that specific. So, let's assume at most a /24 CIDR, meaning that you reduce your lookups to at most 24 descents, instead of as many as 2000 complete checks.

It comes down to the algorithm.

Update - example lookup

Note, I hacked together this tree for supporting faster lookups of IPs in a number of CIDR ranges. Python is not my primary language, so inspect it carefully, and adjust as needed. Specifically, I have used some naive mecheanisms for parsing IP addresses in to integers. Use dedicated libraries to do that instead.

You can see it running on ideone: https://ideone.com/cd0O2I

def parseIPPart(ipx, shift):
    try:
        return int(ipx) << shift
    except TypeError:
        return 0

def parseIP(ipString):
    ips_shifts = zip(ipString.split("."), range(24, -1, -8))

    addr = [parseIPPart(ip, shift) for ip, shift in ips_shifts]
    return sum(addr)

def parseCIDR(cidr):
    addrString, bitsString = cidr.split('/')
    try:
        bits = int(bitsString)
    except TypeError:
        bits = 32
    addr = parseIP(addrString)
    return addr, bits

class CIDRTree:
    class CIDRNode:
        def __init__(self, depth):
            self.depth = depth
            self.isset = None
            self.unset = None
            self.leaf = False

    def __init__(self):
        self.root = CIDRTree.CIDRNode(-1)

    def addCIDR(self, cidr):
        ip, bits = parseCIDR(cidr)
        node = self.root
        for b in range(bits):
            if node.leaf:
                # Previous bigger CIDR Covers this subnet
                return
            mask = 1 << (31 - b)

            if (ip & mask) != 0:
                if node.isset is None:
                    node.isset = CIDRTree.CIDRNode(b)
                kid = node.isset
            else:
                if node.unset is None:
                    node.unset = CIDRTree.CIDRNode(b)
                kid = node.unset
            node = kid
        # node is now a representation of the leaf that comes from this CIDR.
        # Clear out any more specific CIDRs that are no longer relevant (this CIDR includes a previous CIDR)
        node.isset = None
        node.unset = None
        node.leaf = True
        #print("Added CIDR ", ip, " and bits ", bits)

    def matches(self, ipString):

        ip = parseIP(ipString)
        node = self.root
        shift = 0
        while node is not None and not node.leaf:
            shift += 1
            mask = 1 << (32 - shift)
            val = (ip & mask) != 0
            node = node.isset if val else node.unset

        return node is not None and node.leaf

if __name__ == "__main__":
    cidrTree = CIDRTree()
    cidrTree.addCIDR("8.0.0.0/8")
    cidrTree.addCIDR("9.8.7.0/24")

    print ("Tree matches 8.8.8.8:", cidrTree.matches("8.8.8.8"))
    print ("Tree matches 9.9.9.9:", cidrTree.matches("9.9.9.9"))
    print ("Tree matches 9.8.7.6:", cidrTree.matches("9.8.7.6"))


Find fibonacci series upto a given maximum in javascript

Your loop can be terminated in a few different ways. You're using a boolean loop variable, but there are cleaner ways.

One is to simply move the if condition to the while condition (Malachi's answer beat me to this):

while (fib[i-1] + fib[i-2] <= max) {
  i = fib.push(fib[i-1] + fib[i-2]);
}

push helpfully returns the array's new length, so you don't need the i++.

Another is to keep the if branching, but use it to break the loop instead:

while (true) {
  var next = fib[i-1] + fib[i-2];
  if (next > max) {
    break; // exit the loop
  }
  i = fib.push(next);
}

Here we avoid calculating the new fibonacci number twice, but still: the previous code block is probably still the cleanest.

You also initialize i to 2, which is a bit redundant: It's the length of the fib array. Rather than manually initializing the array, and manually recording its length, I'd just do var i = fib.length.

Lastly, it might be good to consider "bad" max values. I.e. what if the maximum is set to zero? Or 1? Or -2324359? For zero you could return an empty array; for 1 you'd need to return a shorter array than what you start with, and for the negative max you could throw an error or return null. Not saying you need to any of this, but it's always to good to consider how you handle such cases.


Asynchronous logging using custom log4j appender and ExecutorService

The UpperCamelCase format is used to name types. You should rename your classes (and constructors) to EmailAppender and EmailSender.

The ExecutorService has a shutdown method that you can use to reject new tasks and terminate the current ones. You may shutdown your pool form the close method. And there is a nice code sample in the official documentation : https://docs.oracle.com/javase/9/docs/api/java/util/concurrent/ExecutorService.html

I guess you should also avoid to use System.out.println and e.printStackTrace(). They will both print unexpected messages on the console. Imagine that your code will be shipped in a library, once a dev will use your library he will suddenly got things printed on his console.

Aside of that I don't see anything wrong.

On a more personal point of view, I would have tried to extract the "configuration" (Creation of Properties and creation of the instance) to another class so that you make a clear distinction between the conversion of the LoggingEvent to an email and the sending of an email message.


You may not be able to get away from chaining a ton of ContinueWith invocations together. However, you might be able to create a builder class with methods that you pass parameters to so you don't have to call ContinueWith from calling code.

It looks like you might be able to remove a lot of duplication by extracting your anonymous methods into one or more stateless (probably static) methods that you can reuse by passing the method signature in, e.g.

private static void SetFlattenedException(Task stoppedTask, TaskContinuationOptions options)

You can also start going down the route of wrapping continuations in method calls with additional Action parameters to "inject" code to execute, but given that it tends to lead to a lot of "inside-out" code that is hard to follow, I'd use sparingly.


A few things:

I'm not sure why you need to map over the top level object? Are there top-level keys other than dataset that
you're interested in? From your example, it looks like you only need the array of records (guests), in which case you don't need mapObject.
I usually try to keep JSX expressions as slim as possible. Your JSX is quite long, and I can see 3 opportunities to shorten/simplify it:

Push more of the transformation into componentDidMount, making the data easier to work with during render
If some transformation has to be done during render (e.g. the search filtering), then consider assigning it to an intermediate local variable rather than doing it inline in JSX
Extract smaller components for logical sections

I'm guessing that those commented out lines were meant to be || (or) clauses?
Try naming variables according to the domain model. I.e., guest rather than item.
There's a bit of duplication that can be reduced, like lower-casing the search string.

Here's a complete example (haven't tested it!):

class Guests extends React.Component {
  constructor(props) {
    super(props);
    this.state = { guests: [] };
  }

  componentDidMount() {
    axios.get('https://gist.githubusercontent.com/eMatsiyana/e315b60a2930bb79e869b37a6ddf8ef1/raw/10c057b39a4dccbe39d3151be78c686dcd1101aa/guestlist.xml')
      .then(res => {
        const xml = XMLMapping.load(res.data);
        const json = XMLMapping.tojson(xml);
        this.setState({ guests: json.dataset.record });
      });
  }

  render() {
    const search = this.state.search.toLowerCase();
    const guests = this.state.guests.filter(guest => (
      guest.first_name.$t.toLowerCase().includes(search) ||
        guest.last_name.$t.toLowerCase().includes(search) ||
        guest.company.$t.toLowerCase().includes(search)
    ));

    return (

        {guests.map((guest, index) => (

            <Guest first_name={guest.first_name.$t} last_name={guest.last_name.$t} company={guest.company.$t} />

{guest.company.$t}

              <EmailFormdisplay guestid={index} />

              <PhoneFormdisplay guestid={index} />

            <GuestDropdown index={index} />

        ))}

    );
  }
}

const Guest = ({ first_name, last_name, company }) => (

    {first_name} {last_name}

{company}

);

const GuestDropdown = ({ index }) => (

    <Dropdown>
      <DropdownTrigger />
      <DropdownContent>

            <EmailFormdisplay guestid={index} />

            <PhoneFormdisplay guestid={index} />

      </DropdownContent>
    </Dropdown>

);

Hope that helps :)


Inserting and updating time spans in XML

Your code seems very procedural - you're worrying about implementing how things will be done before you think about what those things are and how you'd like to use them.  You don't have any properties, just methods, which I don't particularly like.

From what I understand you want to write a reusable wrapper from some XML, that's going to fit in a consistent format.  Do you have a class that represents that object?

public TagElement
{
    private XElement tagXml
    public string Name
    public DateTime LastClocked
    public DateTime TotalClocked
}

Now I'd simply make each of those properties point into the private XML object get / set their values.

public TagElement(string name, DateTime initialValue)
{
    tagXml = new XElement("Tag",
                 new XAttribute("Name", name),
                 new XElement("LastClocked", initialValue),
                 new XElement("TotalClocked", initialValue));
}

public DateTime LastClocked
{
    get
    {
        return tagXml.Descendents("LastClocked").Single().Value;
    }
    set
    {
        tagXml.Descendents("LastClocked").Single() = value;
    }
}

You can still have a Save method that persists the XElement to a file.  You probably also want a collection class that wraps the individual TagElements.

public TagCollection
{
    public List<TagElement> TagElements { get; set; }
}

So you can then use

var collection = new TagCollection("myFile.xml");
// constructor for TagCollection needs to parse the XML, and create TagElements

var fooElement = collection.TagElements.Single(tag => tag.Name == "foo");
fooElement.LastClocked = DateTime.Now;

collection.TagElements.ForEach(tag => tag.TotalClocked = DateTime.Now);

In summary, prefer to use properties to get and set values than using methods.  Abstract away the XML so that when you're using the class you do not know or care that it is being persisted to XML.


So the simplest solutions is to use the available stl function. However there is obviously less to learn there.

#include <algorithm>
#include <vector>

int removeDuplicates(std::vector<int>& nums) {
    nums.erase(std::unique(nums.begin(), nums.end()), nums.end()); ;
    return nums.size();
}


Assign multiple values to variables with a function in a class

Use parens:

(self.katAsk, self.katBid, self.promAsk, self.promBid, self.montAsk,
 self.montBid, self.katProm, self.katMont, self.promKat, self.promMont,
 self.montKat, self.montProm, self.ticker) = self.get_row()


Position.Set

This

long delta = value - position;
if (delta == 0)
    return;

isn't understandable at first glance. You should switch it to

if (position == value) { return; }
long delta = value - position;

You declare long delta for the above substraction but later on you will at least 2 times cast it to int. It would be better to do it the other way.

if (position == value) { return;}
int delta = (int)(value - position);

Potential "bug"

Assume the user of this class will call either Seek() or Position.Set() your code will throw an exception because stream == null.

You should extract the initializing/opening of the stream to a separate method which you call from the constructor, and if needed (cacheRemaining == 0) from the Read() method.

private void OpenStream(int count = 0)
{
    if (stream != null) { resp.Close(); }

    HttpWebRequest req = HttpWebRequest.CreateHttp(Url);

    cacheRemaining = (int)Math.Min(Length - Position, Math.Max(count, cachelen));
    req.AddRange(Position, Position + cacheRemaining - 1);

    resp = req.GetResponse();
    stream = resp.GetResponseStream();

}

Stream.Close()

The call to Stream.Close is not neccessary if you are closing the WebResponse as this is automatically closing the underlaying stream.

Naming

Shortening variable names to resp does not help for readability. Simply name it response.

Variables should be named using camelCase casing which you do except for cachelen which should be cacheLen or cacheLength.

General

What I don't see in your code is the caching. You are using cacheRemaining and cachelen but you don't cache anything.

The Read() method shows unexpected behaviour. Passing in an array with Length == 10 and an offset == 5 then with count == 6 this should throw an exception.

Using more of the built-in features of .NET

For sure that can be done, but as there is no seekable Network or HTTP stream you need to write some amount of code, but I don't see where this should matter, as you only do this once.

What would I do different to your implementation ?

copy the responsestream to a MemoryStream to take advantage of using the Seek() method.
overwrite the Close() method instead of implementing IDisposable as the Stream's Dispose() method only calls the Close() method.
make the cacheLength adjustable, at least by using a constructor parameter.
Ensure that the behaviour of my implementation is the same as of the abstract Stream class

Each method/property should throw an ObjectDisposedException if the stream is disposed
Read() method

If buffer is null throw an ArgumentNullException
If offset or count is negativ throw an ArgumentOutOfRangeException
If buffer.Length - offset < count throw an ArgumentException

But as always code does tell more than just words

class PartialHTTPStream : Stream
{
    private readonly Int64 cacheLength = 1024;
    private const Int32 noDataAvaiable = 0;
    private MemoryStream stream = null;
    private Int64 currentChunkNumber = -1;
    private Int64? length;
    private Boolean isDisposed = false;

    public PartialHTTPStream(String url)
        : this(url, 1024) { }

    public PartialHTTPStream(String url, Int64 cacheLength)
    {
        if (cacheLength > 0) { this.cacheLength = cacheLength; }
        Url = url;
    }

    public String Url { get; private set; }

    public override Boolean CanRead
    {
        get
        {
            EnsureNotDisposed();
            return true;
        }
    }

    public override Boolean CanWrite
    {
        get
        {
            EnsureNotDisposed();
            return false;
        }
    }

    public override Boolean CanSeek
    {
        get
        {
            EnsureNotDisposed();
            return true;
        }
    }

    public override Int64 Length
    {
        get
        {
            EnsureNotDisposed();
            if (length == null)
            {
                HttpWebRequest request = HttpWebRequest.CreateHttp(Url);
                request.Method = "HEAD";
                length = request.GetResponse().ContentLength;
            }
            return length.Value;
        }
    }

    public override Int64 Position
    {
        get
        {
            EnsureNotDisposed();
            Int64 streamPosition = (stream != null) ? stream.Position : 0;
            Int64 position = (currentChunkNumber != -1) ? currentChunkNumber * cacheLength : 0;

            return position + streamPosition;
        }
        set
        {
            EnsureNotDisposed();
            EnsurePositiv(value, "Position");
            Seek(value);
        }
    }

    public override Int64 Seek(Int64 offset, SeekOrigin origin)
    {
        EnsureNotDisposed();
        switch (origin)
        {
            case SeekOrigin.Begin:
                break;
            case SeekOrigin.Current:
                offset = Position + offset;
                break;
            default:
                offset = Length + offset;
                break;
        }

        return Seek(offset);
    }

    private Int64 Seek(Int64 offset)
    {
        Int64 chunkNumber = offset / cacheLength;

        if (currentChunkNumber != chunkNumber)
        {
            ReadChunk(chunkNumber);
            currentChunkNumber = chunkNumber;
        }

        offset = offset - currentChunkNumber * cacheLength;

        stream.Seek(offset, SeekOrigin.Begin);

        return Position;
    }

    private void ReadNextChunk()
    {
        currentChunkNumber += 1;
        ReadChunk(currentChunkNumber);
    }

    private void ReadChunk(Int64 chunkNumberToRead)
    {
        Int64 rangeStart = chunkNumberToRead * cacheLength;

        if (rangeStart > Length) { return; }

        Int64 rangeEnd = rangeStart + cacheLength - 1;
        if (rangeStart + cacheLength > Length)
        {
            rangeEnd = Length - 1;
        }

        if (stream != null) { stream.Close(); }
        stream = new MemoryStream((int)cacheLength);

        HttpWebRequest request = HttpWebRequest.CreateHttp(Url);
        request.AddRange(rangeStart, rangeEnd);

        using (WebResponse response = request.GetResponse())
        {
            response.GetResponseStream().CopyTo(stream);
        }

        stream.Position = 0;
    }

    public override void Close()
    {
        EnsureNotDisposed();

        base.Close();
        if (stream != null) { stream.Close(); }
        isDisposed = true;
    }

    public override Int32 Read(Byte[] buffer, Int32 offset, Int32 count)
    {
        EnsureNotDisposed();

        EnsureNotNull(buffer, "buffer");
        EnsurePositiv(offset, "offset");
        EnsurePositiv(count, "count");

        if (buffer.Length - offset < count) { throw new ArgumentException("count"); }

        if (stream == null) { ReadNextChunk(); }

        if (Position >= Length) { return noDataAvaiable; }

        if (Position + count > Length)
        {
            count = (Int32)(Length - Position);
        }

        Int32 bytesRead = stream.Read(buffer, offset, count);
        Int32 totalBytesRead = bytesRead;
        count -= bytesRead;

        while (count > noDataAvaiable)
        {
            ReadNextChunk();
            offset = offset + bytesRead;
            bytesRead = stream.Read(buffer, offset, count);
            count -= bytesRead;
            totalBytesRead = totalBytesRead + bytesRead;
        }

        return totalBytesRead;

    }

    public override void SetLength(Int64 value)
    {
        EnsureNotDisposed();
        throw new NotImplementedException();
    }

    public override void Write(Byte[] buffer, Int32 offset, Int32 count)
    {
        EnsureNotDisposed();
        throw new NotImplementedException();
    }

    public override void Flush()
    {
        EnsureNotDisposed();
    }

    private void EnsureNotNull(Object obj, String name)
    {
        if (obj != null) { return; }
        throw new ArgumentNullException(name);
    }
    private void EnsureNotDisposed()
    {
        if (!isDisposed) { return; }
        throw new ObjectDisposedException("PartialHTTPStream");
    }
    private void EnsurePositiv(Int32 value, String name)
    {
        if (value > -1) { return; }
        throw new ArgumentOutOfRangeException(name);
    }
    private void EnsurePositiv(Int64 value, String name)
    {
        if (value > -1) { return; }
        throw new ArgumentOutOfRangeException(name);
    }
    private void EnsureNegativ(Int64 value, String name)
    {
        if (value < 0) { return; }
        throw new ArgumentOutOfRangeException(name);
    }
}


Solution to 99 lisp problems, p09 in ruby

Here is solution that aims to solve the described problem.

Note that your initial solutions is doing more that the problem requires, you are handling string instead of lists, are ordering them, and the output isn't the one desired by the problem.

require 'minitest/autorun'

class L99
  # P09 (**) Pack consecutive duplicates of list elements into sublists.
  # If a list contains repeated elements they should be placed in separate sublists.
  def self.pack(list)
    list.slice_when { |i, j| i != j }.to_a
  end
end

class PackTest < Minitest::Test
  def test_pack
    list = %i[a a a a b c c a a d e e e e]
    expected = [%i[a a a a], [:b], %i[c c], %i[a a], [:d], %i[e e e e]]

    assert_equal expected, L99.pack(list)
  end

  def test_pack_string
    str = 'putanytexthere'
    expected = [['a'], %w[e e e], ['h'], ['n'], ['p'], ['r'], %w[t t t], ['u'], ['x'], ['y']]

    assert_equal expected, L99.pack(str.each_char.sort)
  end
end


Questions

  Is the reverse() method efficient as it is, or can it be improved?

You cannot reverse a standard linked list without touching all its nodes, which has a lower-bound asymptotic complexity of o(N) operations.  Your reverse() method has O(N) complexity and uses O(1) extra space, so you cannot improve its asymptotic complexity in either of those dimensions.  Note that the usual recursive solution consumes o(N) overhead, all on the stack, so an iterative implementation is clearly a superior choice.

You could, however, make it slightly more efficient by updating head just once, at the end, instead of at every iteration.  In particular, instead of head = current; inside the loop, you could use head = previous; immediately after the loop.  No other changes would be required.

Consider, however, that if speed of the reverse() method were of paramount importance, such that it was reasonable to sacrifice a bit of list-traversal performance, then you could create a List class for which reverse() is O(1).  I would do that with an internal circular, doubly-linked list of Nodes, with node traversal performed via a Node method that relies on an instance variable of the host List to determine which direction to consider as forward.  With such a structure, (logically) reversing the list would be achieved simply by changing the value of the single variable that controls list direction.  Example:

int direction = 0;

class Node {
    T value;
    Node[] links = new Node[2];
    public Node next() {
        assert ((direction >= 0) && (direction < links.length));
        return links[direction];
    }
}

public void reverse() {
    direction = 1 - direction;
}

  Are the inline comments inside the reverse() method clear or are they confusing and/or superfluous?

They are both clear and largely superfluous.  You've chosen reasonably good names for your variables, so the meaning of the code is fairly well readable from the code itself.  More generally, I rarely think it appropriate to comment every single one of a series of simple statements.  It is more effective to document at a somewhat higher level: if each statement reads well in and of itself then document the purpose of groups of lines.

Also, I prefer trailing comments to be very short.  If you have something longer to say then put it on its own line or lines.

Additional comments

Clearly your LinkedList class is constructed for the express purpose of demonstrating reverse(), as it is missing methods that one would expect a general-purpose list class to provide.  That might or might not be an issue if you were to present such an answer in an interview.  Note that you could provide a full-featured implementation of java.util.List by extending AbstractSequentialList and implementing its listIterator() method.

On general principles, the inner Node class should have a constructor with which its value field and maybe its next field can be initialized.  It may be appropriate to make the value final, too, as @forsvarir also suggested -- this is possible only if you provide a constructor.


Better implementation of checking function

I don't have time to check all of this, but here are some ideas ...
 - place long strings in parenthesis and break them up into multiple lines
 - if those strings are used more than once, use variables for them
 - break out the for loops as a separate function

type_error_msg = ("AssertionError: 'x' failed annotation check(wrong type):"
                  " value = {}, was type {} ...should be type {}")
list_error_msg = ("AssertionError: 'x' failed annotation check(wrong type):"
        " value = {}, was type {} ...should be type {} \nlist[{}] check: {}")
elem_number_error_msg = ("AssertionError: 'x' failed annotation check(wrong "
         "number of elements): value = {} \nannotation had {} elements[{}]")

def _check_values(value, annot, index):
    for x in value:
        check_history = ''
        check_history += str(annot1)
        assert type(x) == annot, self.list_error_msg.format(x,
             type_as_str(x), annot, index,  check_history)

def _check_list_or_tuple (self, param, annot, value, check_history):
    assert isinstance(value, type(annot)), self.type_error_msg.format(value,
                                     type_as_str(value), type_as_str(annot))
    if (len(annot) == 1 and type(annot[0]) != list and
        type(annot[0])!= tuple and inspect.isfunction(annot[0]) == False):
        self._check_values(value, annot[0], 1)
    elif (type(annot[0]) == list or type(annot[0]) == tuple or
          inspect.isfunction(annot[0]) == True):
        for i in range(len(value)):
            self.check(param, annot[0], value[i])
    elif len(annot) > 1:
        check_history = ''
        check_history += str(annot[0])
        assert type (value[0]) == annot[0], self.list_error_msg.format(
            value[0], type_as_str(value[0]), annot[0],
            annot.index(annot[0])+1, check_history)
        check_history = ''
        check_history += str(annot)
        assert len(annot) == len(value), self.elem_number_error_msg.format(
                                        value, len(annot), check_history)
        self._check_values(value[1:], annot[1], 2)


Python string clean up function with optional args

Substitution in multiple passes

Your function is slightly buggy in a non-deterministic way, due to the way you perform substitutions in multiple passes.  Performing string substitutions in multiple passes is usually a bad idea.  Here is another instance of such a bug.  The problem is that the result could vary depending on the iteration order through replace_extras.

Take this example:

>>> clean_up('acetone', replace_extras={
...     'one': '1',
...     'acetone': '(CH3)2CO',
...     'CO': 'carbon monoxide',
...     'C': 'carbon',
...     'CH3': 'methane'
... })

What will be the result?  If the 'one' substitution is done first, then it becomes 'acet1'.  If the 'acetone' substitution is done first, followed by the 'CO' substitution and the 'C' substitution, then it becomes '(carbonH3)2carbon monoxide'.  If it's 'acetone', then 'C', then 'CO', then it becomes '(carbonH3)2carbonO'.  Another possible outcome is '(methane)2carbon monoxide'.  All kinds of results are possible!

Therefore, I recommend doing substitutions in a single pass wherever possible, using regular expressions.  Regular expressions will always look for the leftmost longest match.1  With a regex substitution, the result will not be fed back for further processing.

Documentation and function design

You wrote a very long docstring, which is nice, but nowhere did you actually state the purpose of the function.

What is the purpose of the function?  There is a strip_chars phase, followed by a replace_extras phase.  By the single-responsibility principle, consider splitting the function into two functions, or at least writing it as a composition of two helper functions.

If you have specific examples of inputs and their corresponding outputs, it would be a good idea to write them as doctests instead.

Suggested solution

import re

def clean_up(text, strip_chars=[], replace_extras={}):
    r"""
    Remove all occurrences of strip_chars and whitespace at the beginning
    and end of each line, then perform string substitutions specified by
    the replace_extras dictionary (as well as normalizing all whitespace
    to a single space character), and then strip whitespace from the
    beginning and end.

    >>> clean_up('       ,  ,      , .,.,.,.,,,......test, \t  this\n'
    ...     '.is.a\n'
    ...     '.test...,,,         , .', strip_chars=',.')
    'test, this .is.a .test'

    Any consecutive whitespace is normalized to a single space, but
    you can override these implicit substitutions in replace_extras.

    >>> clean_up(' this is one test\n!\n', replace_extras={'\n': '', 'one': '1'})
    'this is 1 test!'
    """
    # Handle strip_chars
    strip_items = '|'.join(re.escape(s) for s in strip_chars)
    strip_re = r'^(?:{}|\s)+|(?:{}|\s)+$'.format(strip_items, strip_items)
    text = re.sub(strip_re, '', text, re.MULTILINE)

    # Normalize whitespace and handle replace_extras
    replace_keys = list(replace_extras.keys())
    replace_keys.sort(key=len, reverse=True)
    replace_re = '|'.join([re.escape(s) for s in replace_keys] + [r'\s+'])
    return re.sub(
        replace_re,
        lambda match: replace_extras.get(match.group(), ' '),
        text
    ).strip()


You have cached the selectors beforehand,

// we need to cache selectors to avoid re-reading the DOM
 var amount = $("#amount"),
     duration = $("#duration"),
     interest = $("#interest"),
     income = $("#incomeNeeded"),
     monthlyRate = $("#monthlyRate");

So why not use them here,

$("#amount").val($("#slider-range-min").slider("value") + " $$$");

and

slide: function (event, ui) {
     $("#amount").val(ui.value + " $$$");

to

amount.val($("#slider-range-min").slider("value") + " $$$");

and

slide: function (event, ui) {
     amount.val(ui.value + " $$$");

Repeat similarly for #duraton and #interest.

Also, instead of using .slice(x,y) where x and y are static values, why not use:

.slice(0,amount.val().indexOf(' '))

or use split()  - Fiddle

JSPerf test- split vs slice vs substring


shared_ptr alternative with no dependencies

You didn't post a complete header file (no include-guard/#pragma once), so it's unclear whether you also omitted a namespace Taylor { somewhere in there. But you should definitely use a namespace around a name as common as ref.

  virtual ~refcount() { }

Nit: You could use = default; instead of { } here, and you might get slightly better codegen on some compilers. (Virtual destructors are never actually "trivial," unfortunately, but getting the compiler to write this quasi-trivial code for you can't hurt.)

I notice that refcount->rc is public, not private. Was that a design decision?

It occurs to me that the proper name for what you're calling refcount is actually refcounted. The refcount, physically, is the data member rc. A type which derives from your base class is a refcounted type.

ref(T* obj) : obj(obj) {
  if(obj) {
    obj->rc++;
  }
}

I would write this as

explicit ref(T *obj) : obj_(obj) {
    if (obj_ != nullptr) {
        obj_->rc += 1;
    }
}

Notice the whitespace edits, the use of some kind of sigil for data members (to avoid having two different variables named obj in scope), my preference for += 1 when the increment is a stand-alone statement, the explicit comparison against nullptr in place of contextual-conversion-to-bool, and the addition of explicit. Non-explicit constructors permit implicit conversions, e.g.

struct Widget : public refcount { int data = 42; };

void print_data_of(ref<Widget> x) { std::cout << x->data << "\n"; }

int main() {
    Widget *p = new Widget;
    print_data_of(p);
    delete p;  // OOPS! Double delete!
}

It would be better (IMO of course) if this code did not compile.

template<class T2>
ref(const ref<T2>& r) {
  if(r.get()) {
    obj = r.get();
    obj->rc++;
  }
}

This is extremely sketchy. Consider:

struct Widget : public refcount { int data = 42; };
struct Gadget : public refcount { int x = -1; int data = 42; };
int main() {
    ref<Widget> w = new Widget;  // OK...
    ref<Gadget> v = w;           // ...sketchy...
    std::cout << v->data << "\n";  // OOPS! Prints "-1", not "42"
}

This constructor should be either completely removed, or else constrained (using enable_if, C++2a requires, or some other trickery) so that it participates in overload resolution only when T2* would be convertible to T*.

One easy way to mostly-fix this would be to simply add an assertion:

template<class T2>
ref(const ref<T2>& r) {
    static_assert(std::is_convertible_v<T2*, T*>);
    if (r != nullptr) {
        obj_ = r.get();
        obj_->rc += 1;
    }
}

Here we're lying to the library (e.g. std::is_constructible_v<ref<Widget>, ref<Gadget>> will still be true), but at least we prevent the client programmer from accidentally writing a program like the test case above.

Another way to fix the issue would be to rely on pointer-assignment to do the check for us. Instead of refcount *obj_;, let's make our data member look like T *obj_;. Then we can write

template<class T2>
ref(const ref<T2>& r) {
    if (r != nullptr) {
        obj_ = r.obj_;  //HERE
        obj_->rc += 1;
    }
}

And then if T2* isn't convertible to T*, we'll get an error on the line marked //HERE.

Incidentally, this also solves your problem with ref<T>::get() and incomplete types.

T& operator*() { return *get(); }
T& operator*() const { return *get(); }

You don't need both versions of the function, since they do exactly the same thing. Just write the const version. (Dereferencing a pointer doesn't need to modify the pointer, remember. Const is a contract.)

T* operator->() { return static_cast<T*>(obj); }
const T* operator->() const { return static_cast<const T*>(obj); }

And in this case you've got the two versions doing different things, but that's still wrong, because they shouldn't be doing different things! Dereferencing a pointer doesn't need to modify the pointer. What you meant in both cases was simply

T* get() const { return static_cast<T*>(obj); }
T* operator->() const { return get(); }
T& operator*() const { return *get(); }

Your void swap(ref& p) should be noexcept — just like your move-constructor, which I guess you didn't write. (You should do some move semantics here!)

I recommend implementing your swap as a one-liner: std::swap(obj_, rhs.obj_);.

A member swap function will not be picked up by any standard library algorithms. If you want your swap to be actually used, you'll need to provide an ADL swap, like this:

friend void swap(ref& a, ref& b) noexcept { a.swap(b); }

Your queued_delete is interesting. It's misnamed, in that its deletions are stacked (LIFO), not queued (FIFO). I don't know if that makes a difference to performance or anything like that, in practice.

It's also thread-unsafe, which is not clear from your description/documentation. In standard C++, we can write

int main() {
    std::shared_ptr<Widget> p(new Widget);
    std::thread([q = p]() {
        q = nullptr;
    }).detach();
    p = nullptr;
}

and be guaranteed that the writes to the refcount shared by p and q won't race with each other. Your documentation clearly states that we can't do that with your ref<Widget>. But what is surprising to me is that we also can't do the following!

int main() {
    ref<Widget> p(new Widget);
    std::thread([]() {
        ref<Gadget> q(new Gadget);
        q = nullptr;
    }).detach();
    p = nullptr;
}

Here, p.obj->rc and q.obj->rc are completely different objects, so there's no race there; but then they each call into queued_delete and try to write to stack, and those writes race with each other. So your ref is completely unsafe for use within a multi-threaded environment, even if you never share any objects between threads.

In theory I guess you could "fix" this by replacing the storage class static with thread_local everywhere it appears; but, please don't do that.


overwrite would be better as True or False.

Since each file is treated independently, you should define a function to make the independence obvious.

To split and join path components, use os.path.split() and os.path.join() instead of doing your own string analysis.  The interface is slightly nicer, and it also ensures that there won't be missing or extra / separators.

If you reorder the conditionals, you could reduce the number of os.path.exists() tests in the don't-overwrite case, where the destination file already exists.

Print 'Copying filename' before copying the file, so that if the copying fails, you'll see which file it failed on.  Print "Finished." instead of "Finished....", because ellipses imply that the job is incomplete.

import os
from shutil import copyfile
import urllib
from xml.etree.ElementTree import ElementTree

def copy_song(url, dest, overwrite=False):
    if not url.startswith('file://'):
        return

    orig_path = urllib.unquote(url[len('file://'):])
    orig_dirname, basename = os.path.split(orig_path)
    dest_dirname = os.path.join(dest, orig_dirname)
    dest_path = os.path.join(dest_dirname, basename)

    if not overwrite and os.path.exists(dest_path):
        print 'Skipping ' + orig_path
    else:
        if not os.path.exists(dest_dirname):
            os.mkdir(dest_dirname)
        print 'Copying ' + orig_path
        copyfile(orig_path, dest_path)

playlist = "/path/to/playlist.xml"
dest = '/path/to/destination';

playlist_xml = ElementTree.parse(playlist)
for song in playlist_xml.findall('/dict/dict/dict/string'):
    copy_song(song.text, dest)
print 'Finished.'


Create URL using data obtained from filters

Your code looks pretty good! Here are a few comments:

Prefer to use .data('dept') over .attr('data-dept').
Prefer to use .on('change', ...) over .bind('change', ...).
rel is an invalid attribute on input elements. You can use data-rel instead.
It's a inefficient to create URLs for every FilteredNav.wrapper whenever any checkbox is changed, since it should only affect one of them. Consider having changeUrl only affect the enclosing .dept-filters. (This is as simple as just removing the .each() and setting self = $(this).closest(FilteredNav.wrapper);.) Then in init, use $.each(FilteredNav.wrapper, changeUrl);.
This might look like:

var FilteredNav = {
  wrapper: $(".dept-filters")
  , init: function() {
    FilteredNav.changeUrl();
    $.each(FilteredNav.wrapper, FilteredNav.createUrl);
  }
  , createUrl: function() {
    var urlItems = [];

    var self = $(this).closest(FilteredNav.wrapper)
    , checkedInput = self.find('input[type="checkbox"]:checked')
    , submitButton = self.find('.refinar-btn');

    // select only the relevant items
    $.each(checkedInput, function(i, item) {
      urlItems.push($(item).data("rel"));
    });

    if (checkedInput.length) {
      // create the url
      var url = document.location.protocol
      + "//"
      + document.location.host
      + "/"
      + self.data("dept")
      + "/?"
      + urlItems.join("&");

      // add new url as href of 'refine' button
      submitButton.removeClass("hide").attr("href", url);
    } else {
      submitButton.addClass("hide");
    }

    // Just to demonstrate
    submitButton.siblings('output').text(submitButton.attr('href'));
  }
  , changeUrl: function() {
    FilteredNav.wrapper.find('input[type="checkbox"]').on("change", FilteredNav.createUrl);
  }
};

// uncomment this line when testing on the console
// FilteredNav.init();

// comment this line when testing on the console
$(document).ready(function() {
  FilteredNav.init();

  // don't leave my Stack Snippet! :)
  $('a').on('click', function() { return false; });
});
a {
  text-decoration: none;
  color: inherit;
}

.refinar-btn {
  border: 1px solid black;
  display: inline-block;
  padding: 1ex;
}

.refinar-btn.hide {
  background: #eee;
  color: #777;
}

        <input type="checkbox" data-rel="filter1" />
        Filter 1

        <input type="checkbox" data-rel="filter2" />
        Filter 2

        <input type="checkbox" data-rel="filter3" />
        Filter 3

    Refine
    <output></output>

        <input type="checkbox" data-rel="filterA" />
        Filter A

        <input type="checkbox" data-rel="filterB" />
        Filter B

        <input type="checkbox" data-rel="filterC" />
        Filter C

    Refine
    <output></output>

You can take this even further and only generate the URL when the submit button is actually clicked. You would then have to add or remove the .hide class in a different way. One solution is to have a very simple change handler for the checkboxes that just sets .hide if any are checked. Alternatively, you could move it entirely out of the script and use CSS similar to the following, which would be pretty dependent on your structure. For example, it wouldn't work in my snippet above.

.refinar-btn {
    /* hide styles, e.g.: */
    opacity: 0.5;
    box-shadow: 0;
}

input[type="checkbox"]:checked ~ .refinar-btn {
    /* show styles, e.g.: */
    opacity: 1;
    box-shadow: ...;
}