PrintLn header for C++ I've always wondered what's the most elegant way of implementing PrintLn in C++. I have not yet come with perfect conclusion. This is my shot.

Shortcut for std::cout, std::cerr and std::clog "print line" version. So you can type
CoutLn << "Hello world!"; instead of std::cout << "Hello world!" << '\n';

CoutLn, CerrLn, ClogLn are implemented.

#include <iostream>
#include <typeinfo>
#include <mutex>

/* Template magic: determine if our type is printable. */
template<typename S, typename T, typename = void>
struct is_to_stream_writable : std::false_type {};
template<typename S, typename T>
struct is_to_stream_writable<S, T, std::void_t<decltype(std::declval<S&>()<<std::declval<T>())>> : std::true_type {};

/* Since std::cout, std::cerr etc. are all different instances of the same type,
 * let's define dummy classes for detecting different instances. */
class Cout{};
class Cerr{};
class Clog{};

template<class Stream = Cout>
class PrintLn {
  private:
    static std::ostream* stream;
    static std::mutex m;
  public:
    PrintLn() {
      if constexpr (std::is_same<Stream, Cout>::value) stream = &std::cout;
      else if constexpr (std::is_same<Stream, Cerr>::value) stream = &std::cerr;
      else if constexpr (std::is_same<Stream, Clog>::value) stream = &std::clog;
    }
    template<class T>
    PrintLn& operator<<(const T& msg) {
      static_assert(is_to_stream_writable<std::ostream, T>::value, "your type is not printable");
      std::lock_guard<std::mutex>(PrintLn<Stream>::m);
      *stream << msg;
      return *this;
    }
    ~PrintLn() {
      std::lock_guard<std::mutex>(PrintLn<Stream>::m);
      *stream << '\n';
    }
};

/* Declare static variables. */
template<class Stream>
typename::std::mutex PrintLn<Stream>::m;

template<class Stream>
typename::std::ostream* PrintLn<Stream>::stream;

using CoutLn = PrintLn<Cout>;
using CerrLn = PrintLn<Cerr>;
using ClogLn = PrintLn<Clog>;

/* Shorter syntax, for example: `CerrLn{} << "error";` -> `CerrLn << "error";`. */
#define CoutLn CoutLn{}
#define CerrLn CerrLn{}
#define ClogLn ClogLn{}

Have you looked at C++20 osyncstream?  It seems to have a better interface.

Multithreading

std::lock_guard<std::mutex>(PrintLn<Stream>::m);

This line is useless, because temporary objects are destroyed at the end of the full-expression.  (Does this even have a temporary in C++17?)  You need a named variable instead.  Also make use of class template argument deduction:

std::lock_guard lock{m};

Also,

Streams

Your approach is unnecessarily restricted because only three hardcoded streams std::cout, std::cerr, and std::clog are supported.  And I think {} is OK and these don't really help a lot:

#define CoutLn CoutLn{}
#define CerrLn CerrLn{}
#define ClogLn ClogLn{}

Make the function object have regular semantics instead.  You may use a hash map internally to store the mutexes, as syncbuf does.

Also SFINAE on operator<<.

Here's the same thing implemented with osyncstream:

template <
    class CharT,
    class Traits = std::char_traits<CharT>,
    class Allocator = std::allocator<CharT>
> class PrintLn : public std::basic_osyncstream<CharT, Traits, Allocator> {
    using Base = std::basic_osyncstream<CharT, Traits, Allocator>;
public:
    using Base::Base;

    PrintLn(PrintLn&&) = default;
    PrintLn& operator=(PrintLn&&) = default;
    ~PrintLn()
    {
        if (this->get_wrapped()) {
            *static_cast<Base*>(this) << '\n';
        }
    }
};

inline auto cout_ln()
{
    return PrintLn{std::cout};
}
inline auto cerr_ln()
{
    return PrintLn{std::cerr};
}
inline auto clog_ln()
{
    return PrintLn{std::clog};
}

But anyway, why would you wanna do this when you can simply print a \n?


Create a random string with a set prefix I was curious on how I could create a random string and have it have a set prefix.  Lets say, as an example, 3456.  This question is more referring to how I would join two strings together, my prefix and my random string.
To give a more comprehensive exmaple of what i'm looking for, let's say I generate the string "ab8897fjs".  I would like to add my prefix to that string to make it "3456ab8897fjs".
Thank you!
Add: This is my code for string generation.
char *randstring(size_t length) {

    static char charset[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789,.-#'?!";
    char *randomString = NULL;

    if (length) {
        randomString = malloc(sizeof(char) * (length +1));

        if (randomString) {
            for (int n = 0;n < length;n++) {
                int key = rand() % (int)(sizeof(charset) -1);
                randomString[n] = charset[key];
            }

            randomString[length] = '\0';
        }
    }

    return randomString;
}

When you do rand()%n you will get a random number in the range [0, n-1] but the distribution will not be uniform. Some values will be more likely than others. This is more easily illustrated if we drop the number of random bits from 32 to 4, i.e. RAND_MAX=15. And we want a number between 0 and 10 so we do rand()%11. Now for returns values of 0-10 from rand() well get the wanted values, but what about the rest of the range? Well from 11 to RAND_MAX well also get 0-4... This means that values 0-4 are twice as likely to occur as values 5-10, this isn't what we wanted!
I explain this in another answer here: https://codereview.stackexchange.com/a/159760/36120


Counting GUAG introns in chromosomes I have this code that is working fine but it's taking pretty much 100% of my cpu to run and it takes around 25min. I'd really like to optimize it but don't know what parts I could improve.

The main problem I guess is that I'm using coordinates of a file to be compared with a 3,5GB file.
So I tried putting the information of this 3,5GB file in a hash to make it faster but it's not what I thought it would be.
I want to compare exon coordinates to the whole human genome (hg38), see which two letters are before the exon position and which two are in the end, so that would count as an intron. Then, for each intron I want to see if those two letters are compatible with GUAG or not.

#!/usr/bin/perl
use warnings;
use strict;

print "Enter file path (refGene)\n";
my $ref_gene = <>;
chomp ($ref_gene);
my $local1 = "./$ref_gene";

print "Enter file path (hg38)\n";
my $filename = <>;
chomp ($filename);
my $local2 = "./$filename";

open (HG, $local2) or die "error opening $filename\n";

my %chromosome;
my $key;
while (my $line = <HG>) {
    chomp ($line);
    if ($line =~ /^\>/) {
        $key = $line;
        $key =~ s/\>//;          # / stop editor red coloring
        $chromosome{$key} = "";
    }
    else {
        $chromosome{$key} .= uc $line;
    }}

close (HG);

open (REFGENE, $local1) or die "error opening $ref_gene\n";

my $total_introns = 0;
my $GUAG_introns = 0;
while (my $line = <REFGENE>) {
    my @column = split ("\t", $line);
    my $chr = $column[2];
    my $sense = $column[3];
    my $number_exons = $column[8];
    my $beginning_of_exon = $column[9];
    my $end_of_exon = $column[10];

    my @beginning_exons = split (",", $beginning_of_exon);
    my @end_exons = split (",", $end_of_exon);
    foreach my $element (@end_exons) { #subtract 1 from end of exon because its 1-based index
        $element -= 1;
    }

    my $introns = ($number_exons - 1); #amount of introns = exons - 1
    $total_introns += $introns;

    my $current_seq = $chromosome{$chr};

    for (my $i=0; $i<@beginning_exons-1; $i++) {
        my $end = substr ($current_seq, $end_exons[$i]+1, 2); #get two letters after end of exon
        my $beginning = substr ($current_seq, $beginning_exons[$i+1]-2, 2); #get two letters before beggining of exon

        if ($sense eq "+") {
            if ( ($end eq "GT") && ($beginning eq "AG") ) {
            $GUAG_introns += 1;
            }}

        elsif ($sense eq "-") {

            if ( ($end eq "CT") && ($beginning eq "AC") ) {
                $GUAG_introns += 1;
            }}}
}

close (REFGENE);

my $GUAG = (($GUAG_introns/$total_introns)*100); #percentage of GUAG introns
print "number of GUAG introns = $GUAG_introns \n total introns = $total_introns \n";
print " $GUAG% of GT-AG introns.\n\n";

exit;

input

717     NM_000525       chr11   -       17385248        17388659        17386918        17388091        1       17385248,       17388659,
987     NM_000242       chr10   -       52765379        52771700        52768136        52771635        4      52765379,52769246,52770669,52771448,    52768510,52769315,52770786,52771700,

The two letters before the exon position would be AG or the last two letters of an intron

column 9 has the coordinates to the start of an exon and column 10 to the end, I'm trying to get the things in the middle and see if they start with GT and end with AG. Column 3 is the sense of the sequence, so if it's negative (-) instead of looking for GUAG (U cause it's RNA in the end but the reference(hg38) is T) I'll be looking for the reverse complement (AC instead of GT, and CT instead of AG). Column 2 is the chromosome, so for chromosome 6 I'd look the reference in hg38 for chromosome 6 and count the introns. The other file is 15MB

I know it's a lot of code and that nobody will download 3,5GB of human genome to run this but I'd like to know which kind of improvements I can do to optimize the code and not use 100% of cpu.

I put a small part of the input file and the hg38.

I have a program that seems to work and produce the same results as your own code. However I have a problem in that the intron doesn't seem to be defined properly

Suppose my sequence is the alphabet and the exons we have are defined by

@beginning_exons = (5, 17);
@end_exons       = (12, 21);

                  1                   2
1 2 3 4|5 6 7 8 9 0 1 2|3 4 5 6|7 8 9 0 1|2 3 4 5 6
A B C D|E F G H I J K L|M N O P|Q R S T U|V W X Y Z
        [=============] <-----> [=======]

so they are EFGHIJKL and QRSTU, forming an intron MNOP. The calculation in your program, and in my code that produces the same result, gives $end = 'MN', which is fine, but $beginning = 'PQ', which I don't understand. It is the last character of the intron and the first character of the exon

Can this be right? I have put exactly the values above into your code and mine and get the same result

Update

As usual, the solution was staring me in the face. I am confident that the values you extract to @beginning_exons and @end_exons are not character positions but offsets, that may be passed directly to substr. It was your comment "subtract 1 from end of exon because its 1-based index" that threw me

So it is the gaps between the letters that are numbers from zero at the start of the string. Like this

                    1                   2
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6
 A B C D|E F G H I J K L|M N O P|Q R S T U|V W X Y Z
         [=============] <-----> [=======]

Now the "exon" EFGHIJKL starts at 4 and ends at 12, and QRSTU starts at 16 and ends at 21, making the data look like this

@beginning_exons = (4, 16);
@end_exons       = (12, 21);

Assuming this makes the arithmetic much easier, because there is no off-by-one error to account for, and the start and end of the intron are 12 and 16—the end of the first exon and the beginning of the next. Furthermore, numbers can be fed directly into substr without any offset, except that the last two characters of the intron are clearly at (6-2)

I've modified my code below to take this into account, and it makes it much clearer to read

Here's my solution

The main reason yours is so slow is that it's reading the HG38 file line by line, which means it has to upper case each line and append it to the sequence up to 4 million times per sequence

That's very slow because the repeated appending will mean that the string will frequently become too big for the space allocated to it, and it has to be copied to a larger space before it can be expanded. Copying such a huge string thousands of times takes a lot of processor work

I have written it so that a whole chromosome is read at a cctime. Then all that has to be done is to remove the newlines and set it to upper case, just once

I've also printed the each chromosome's name when it is encountered, to give some confirmation that the process is progressing. (You can stop this by removing the print "    $key\n" statement.) The time taken to test the introns at the end is minimal

As I said, this code produces the results 492784 /499504 that you expect, but I'm still concerned about the end pair of bases. Please let me know if you have an explanation

use strict;
use warnings FATAL => 'all';

STDOUT->autoflush;

my ( $ref_gene_file, $hg38_file ) = qw/ refGene.txt hg38.fa /;

my %chromosome;

{
    print "Processing HG38:\n";

    open my $hg38_fh, '<', $hg38_file or die qq{Error opening "$hg38_file" for input: $!};

    local $/ = '>';

    my $key;

    while ( <$hg38_fh> ) {

        next unless /[^>\s]/;

        s/(.+)// and $key = $1;
        printf "    %s\n", $key;

        $chromosome{$key} = uc tr/A-Za-z//cdr;
    }

    print "\n";
}

my $total_introns = 0;
my $GUAG_introns  = 0;

{
    open my $ref_gene_fh, '<', $ref_gene_file
            or die qq{Error opening "$ref_gene_file" for input: $!};

    while ( <$ref_gene_fh> ) {

        my @column = split;

        my ( $chr, $sense, $n, $exon_starts, $exon_ends ) = @column[ 2, 3, 8, 9, 10 ];
        my @exon_starts = $exon_starts =~ /\d+/g;
        my @exon_ends   = $exon_ends   =~ /\d+/g;

        die "Data discrepancy" unless @exon_starts == $n and @exon_ends == $n;

        my $introns = $n - 1;    #amount of introns = exons - 1
        $total_introns += $introns;

        my $sequence = $chromosome{$chr} or die "No such chromosome";

        for my $i ( 0 .. $introns-1 ) {

            my ($intron_start, $intron_end) = ( $exon_ends[$i], $exon_starts[$i+1] );

            my $intron_prefix = substr( $sequence, $intron_start, 2 );
            my $intron_suffix = substr( $sequence, $intron_end-2, 2 );

            if ( $sense eq '+' ) {
                ++$GUAG_introns if $intron_prefix eq 'GT' and $intron_suffix eq 'AG';
            }
            elsif ( $sense eq '-' ) {
                ++$GUAG_introns if $intron_prefix eq 'CT' and $intron_suffix eq 'AC';
            }
        }
    }
}

printf "Number of GUAG introns = %d\n", $GUAG_introns;
printf "Total introns = %d\n",          $total_introns;
printf "%.1f%% of GT-AG introns.\n\n",  $GUAG_introns / $total_introns * 100;

output

Processing HG38:
    chr1
    chr10
    chr11
    chr11_KI270721v1_random
    chr12
    chr13
    chr14
    ...

Number of GUAG introns = 492784
Total introns = 499504
98.7% of GT-AG introns.

[Finished in 39.1s]


Count the number of vowelgroups in a string I've written a function that can count the vowel groups in a string.

def count_vowelgroups(woord):
    if woord[0].lower() in 'aeiouy':
        output = 1
    else:
        output = 0

    for i in range(1, len(woord)):
        if woord[i].lower() in 'aeiouy' and woord[i-1].lower() not in 'aeiouy':
            output += 1
    return output

It works fine, but not really pythonic! Who can help?

For example:

print(count_vowelgroups('Hello everyone!'))
6
print(count_vowelgroups('Alright'))
2
print(count_vowelgroups('Whaaaaaat!?'))
1
print(count_vowelgroups('Je pense donc je suis.'))
6

Using the itertools recipe pairwise you can directly use sum to calculate the number of vowel groups by iterating over pairs of values.

In addition I would make the vowels a global constant and a set, because building the string every time is going to cost some time. Also, sets have very fast membership testing.

With these changes you would get:

from itertools import tee, izip

VOWELS = set("aeiouy")

def pairwise(iterable):
    """s -> (s0,s1), (s1,s2), (s2, s3), ..."""
    a, b = tee(iterable)
    next(b, None)
    return izip(a, b)

def count_vowelgroups(words):
    words = words.lower()
    return sum(c_1 not in VOWELS and c in VOWELS for c_1, c in pairwise(words))


Orthodox Easter calculator I have code for calculating the date for orthodox Easter and it makes me cry when I look at the if-else part. Please give your suggestions on how to make it shine (if possible).

I guess I can format day/month to DateTime format but I'm afraid it'll bring just more mess.

It works just fine though this.

Source

puts "- Hello, I'm EasterEvaluator. I can tell you when will be(or was) Easter."
puts "- Please enter the year which you'r interested in:"
year = gets.chomp.to_i
a = year % 19
b = year % 4
c = year % 7
d = (19*a+15) % 30
e = (2*b + 4*c + 6*d + 6) % 7
f = d + e
if f<=9
    easter = 22+f
    easter = easter + 13 if year > 1918
    if easter > 31
        easter = easter - 31
        month = "April"
    else
        month = "March"
    end
else
    easter = f-9
    easter = easter + 13 if year > 1918
    if easter > 30
        easter = easter-30
        month = "May"
    else
        month = "April"
    end
end

puts "- In #{year}, Easter is going to be on #{easter} of #{month}."

Completely untested psuedo-code, but the key is to realize f = number of days from March 22 to Orthodox Easter.

(* f gives Easter as the number of days since March 22 *)

easter = f+22+(year<1918)*13

month = "March"; # default
if f > 31 {month="April"; day=f-31}
if f > 61 {month="May"; day=f-61}

You might also post to http://codegolf.stackexchange.com


Finding the subsets of a set I am trying to write a procedure that finds the power set of a list, that is, the set of 2n subsets (where n is the number of elements in the list). I simply quite find all the subsets. I feel my code is a lot more work than it should be.

(define (element-of-set? x set)
  (cond ((null? set) false)
        ((not(pair? set)) (display "not a set\n") false)
        ((equal? x (car set)) true)
        (else (element-of-set? x (cdr set)))))

(define (length set)
  (define (length-iter a len )
    (if (null? a)
        len
        (length-iter (cdr a) (+ 1 len))))
  (length-iter set 0))

(define (adjoin-set x set)
  (if (element-of-set? x set)
      set
      (cons x set)))

(define (conS x w )
  (cond ((null? w) '())
        (else(cons (cons (car x) (cons (car w) '())) (conS x (cdr w))))))

(define (helper s)
  (define (help s newS len)

    (if (<= len 0) newS
        (help (cdr s)(append (adjoin-set s newS) (conS s (cdr s)))(- len 1))))
  (help s '() (length s)))

(define (trim s)
  (let helper((s s) (newS '()) (len (length s)))
    (cond ((<= len 0) newS)
          ((null?(cdr(car s)))
                 (helper (cdr s) newS(- len 1)))
          (else (helper (cdr s) (adjoin-set (car s) newS) (- len 1))))))

(define (subsets x)
  (define (sub x newSet)
    (if (null? x) newSet
        (sub (cdr x) (adjoin-set (cons (car x) '())
                                 (adjoin-set '() newSet)))))
  (sub x (trim (helper x))))

I think in general it's okay, if you need to define all of those yourself (in particular length and element-of-set? that is. trim looks a bit complicated for what it does: If you want to skip single element lists (why?), write it like that, no need for the accumulator and the length call.

The input set should be cleaned up once before the subset generation though, then you won't have to filter the result for duplicates.

conS, helper and trim are bad names. The first one is a compatibility issue since you'll find Lisps that are case-insensitive, which will then overwrite the regular cons. Both conS and helper don't say anything at all about what they do. trim essentially the same, a little better maybe. In general, stick with scheme-names-with-dashes-in-between, descriptive names and no camel case.
You sometimes skip the space between )_( and add spaces where they don't belong, like _) which looks messy. Indentation is sometimes a bit off, e.g. the if in subsets has the first case on the same line as the if.


Javascript objects listening to their own events I have been playing with event driven javascript lately, and have wondered if it's a good/bad thing for objects listening for their own events as a way of abstracting and simplifying internal logic.

Consider the following:

function MyModule(options) {
    this.options = options;
    this.data    = {};

    this.setListeners();
    //...
}

MyModule.prototype = new EventEmitter;
MyModule.prototype.constructor = MyModule;

MyModule.prototype.updateBasket = function() {
    var self = this,
        url  = this.options.url;

    $.ajax(url)
        .done(function(data) {
            self.emit('basketupdate:success', data);
        })
        .fail(function(jqxhr, textStatus, err) {
            self.emit('basketupdate:error', err);
        })
        .complete(function() {
            self.emit('basketupdate:complete');
        });
};

MyModule.prototype.setListeners = function() {
    var self = this;

    this.on('basketupdate:success', function(data) {
        // ... do something on success
        self.data = data;
    });

    this.on('basketupdate:success', function(data) {
        // ... do something else on success
        console.dir(data);
    });

    this.on('basketupdate:error', function() {
        // ... do something to handle the error
    });
};

var module = new MyModule({
    url: '/path/to/request'
});

module.updateBasket();

A simple module with an ajax request. I could quite easily put all the logic in the corresponding callback, or even map callbacks to internal methods. However I'm quite liking this approach to simplifying the code.

Are there downsides or potential problems to structuring the code in the way that I haven't considered. Or would this be seen as an anti-pattern with events only intended to be listened to from elsewhere?

You could do this, but it's not really useful.

This code is just easier to read:

MyModule.prototype.updateBasket = function() {
    var self = this,
        url  = this.options.url;

    $.ajax(url)
        .done(this.done)
        .fail(this.fail)
        .complete(this.complete);
};

MyModule.prototype.done = function(data) {
    // Do something when done
};

MyModule.prototype.fail = function(data) {
    // Do something when request failed
};

MyModule.prototype.complete = function(data) {
    // Do something no matter what
};

And you could give more meaningful names to your methods.

Event emitters are useful when you're playing with different objects that you don't want to link together.


Pollard p - 1 implementation I'm trying to implement the Pollard p - 1 algorithm seen here, in Java.
It has a C implementation here. I am using it for numbers without any smaller factors so trial factorisation isn't needed.  My implementation can factor numbers such as 15236506168104630133 but it takes around ten minutes. I also would like it to factor number of up to 30 digits with it. Is there any improvements I can make?

public static void factorise(BigInteger number){

    BigInteger b = BigInteger.valueOf(10);
    //b!
    BigInteger k = Utils.factorial(b);

    while(true){

        //random number a from number 0 to n-2
        BigInteger a = Utils.generateRandomNumber(number).add(BigInteger.valueOf(-1));

        BigInteger tmp = a.modPow(k, number);
        //a^k - 1 (mod n)
        tmp.add(BigInteger.valueOf(-1));
        //gcd(a^k - 1 (mod n), number);
        BigInteger gcd = tmp.gcd(number);

        if(gcd.compareTo(BigInteger.ONE) > 0){

            if(gcd.isProbablePrime(100)){
                System.out.println("Factor:d " + gcd);

                number = number.divide(gcd);

            }

            if(number.compareTo(BigInteger.ONE) == 0){
               System.out.println("done");
                return;
            }

            if(number.isProbablePrime(10)){
                 System.out.println("Factor: " + number);
                 return;
            }

        }

    }

}

public static BigInteger generateRandomNumber(BigInteger number){

    BigInteger random;
    do {
        random = new BigInteger(number.bitLength(), new SecureRandom());
    } while (random.compareTo(number) >= 0);

    return random;

}

public static BigInteger factorial(BigInteger number){

    BigInteger mul = number.add(BigInteger.valueOf(-1));

    while(mul.compareTo(BigInteger.valueOf(1)) != 0){

        number = number.multiply(mul);
        mul = mul.add(BigInteger.valueOf(-1));

    }

    return number;
}

I really really hate when the comments (which are supposed to help) don't agree with the code. Because now you don't know which is correct (the code or the comments).
    //random number a from number 0 to n-2
    BigInteger a = Utils.generateRandomNumber(number).add(BigInteger.valueOf(-1));

When writing comments. DO NOT put a comment for each obvious line of code. The code describes itself I don't need you to tell me what the line of code does. I can actually read code (as can everybody that will read your code).
When you write comments describe the algorithm that your code is trying to implement (preferably with a link to a static page that will never move (wikipidia) that not only describes the algorithm but what its trying to do (or put it in the comments at the top of a chunk of code). Describe WHY you are using the algorithm and what the final result will be.
Code:
    //random number a from number 0 to n-2
    BigInteger a = Utils.generateRandomNumber(number).add(BigInteger.valueOf(-1));

If this actually generates a random number in that range then I would seriously kick the implementer of generateRandomNumber for generating an initial range of [1..(number-1))
This comment is even stranger:
    BigInteger tmp = a.modPow(k, number);
    //a^k - 1 (mod n)
    tmp.add(BigInteger.valueOf(-1));

Really. Not sure what the heck is happening here.
What is it with all the usless white space.
            }

        }

    }

}

Maybe I should kick the implementor of generateRandomNumber
public static BigInteger generateRandomNumber(BigInteger number){

That is not doing what the name suggests it is doing.
Yep sure the loop is going to work:
do {
    random = new BigInteger(number.bitLength(), new SecureRandom());
} while (random.compareTo(number) >= 0);

But why have the loop. Why not generate a random one smaller than the max than add one.
Also why are you creating a new random number generator on each iteration new SecureRandom(). You just create than once (in the entire life cycle of the program) then use the instance every time.


Persist relation to a class (and not a class instance) in Ruby on Rails with Mongoid Assume my users can subscribe to several plan_types. I want to define those plans as classes, and be able to keep, for each user, a reference to a subscription_plan and also a presubscription_plan. This is like a State pattern :

class Payment::SubscriptionPlan
  def self.max_subscription_quantity
    120
  end
end

class Payment::ConcretePlan1 < Payment::SubscriptionPlan
  def self.commitment_period
    2.months
  end
end

class Payment::ConcretePlan2 < Payment::SubscriptionPlan
  def self.max_subscription_quantity
    50
  end
end

What is the best way to "persist" the relation to a class in the database ? Currently I was doing the following using the as_enum gem

class Settings
  setting :subscription_plans, type: Array, default: [
    Payment::ConcretePlan1,
    Payment::ConcretePlan2
  ]
end

class User
  as_enum :subscription_plan, Settings.subscription_plans, map: :string
  as_enum :presubscription_plan, Settings.subscription_plans, map: :string

  def subscription_plan
    return nil if super.nil?
    Payment.const_get(super.to_s.demodulize)
  end

  def presubscription_plan
    return nil if super.nil?
    Payment.const_get(super.to_s.demodulize)
  end
end

The current implementation works but I'm not sure this is the best way to tackle the problem

It works that's for sure.

But enums could just as easily be defined in the database. Now that you've already got it working with enums, I would say stick with enums untill you have a good reason to change it.

anyways, here's how i probably would have done it:

e.g.

user has_many subscription_plans
subscription_plan can belong to many users.
Limit the ability for a user to have a max of two, one of each type. Or the user must have two at any one time etc.

You get the picture. You can change the subscription plan table to suit your needs.

Hope this helps.


Base class for singletons Even though singletons are bad practice and often unnecessary, I still see many developers using this pattern over and over. Since implementation of this pattern usually requires some common code, I thought we could just make a base class to be extended.

I came up with this so far.

<?php
namespace wl;

/**
 * @dosc allows only one instance for each extending class
 * @example use it for database connection, config setup...
 * Be aware, the singleton pattern is consider to be an anti-pattern
 * because it can be hard to debug.
 * In most cases you do not need to use singleton pattern
 * so take a longer moment to think about it before you use it.
 */
class Singleton
{
    /**
     *  holds an single instance of a class
     *
     *  @var array of objects
     */
    protected static $instance = [];

    /**
     *  @desc provides a single slot to hold an instance interchanble between all child classes.
     *  @return object
     */
    public static function getInstance(){
        $class = get_called_class();
        if(!isset(self::$instance[$class]) || !self::$instance[$class] instanceof $class){
            self::$instance[$class] = new static(); // create and instance of child class which extends Singleton super class
            echo "new ". $class . PHP_EOL; // remove this line after testing
            return  self::$instance[$class]; // remove this line after testing
        }
        echo "old ". $class . PHP_EOL; // remove this line after testing
        return static::$instance[$class];
    }

    /**
     *  do not allow create new instance by new keyword
     *
     */
    protected function __construct(){}

    /**
     *  Do not clone the object
     */
    protected function __clone(){}

    /**
     *  Do not allow reserialization of this object
     */
    protected function __wakeup(){}

}

/**
 * ----------------------------------------------USE EXAMPLE---------------------------------------------------
 *  @docs example database class by extending singleton class implements singleton pattern
 */
class Database extends Singleton
{
    public function __construct(){

    }
}

/**
 *  @docs Config class by extending singleton class implements singleton pattern
 */
class Config extends Singleton
{
    public function __construct(){

    }
}

/**
 *  @example create new Database
 */
$bd1 = Database::getInstance(); // new
$bd2 = Database::getInstance(); // old

/**
 *  @example create new Config
 */
$bd1 = Config::getInstance(); // new
$bd2 = Config::getInstance(); // old

$bd3 = Config::getInstance(); // old
$bd4 = Database::getInstance(); // old

$bd5 = Database::getInstance(); // old
$bd6 = Config::getInstance(); // old

Any suggestions are very welcome.
github

Not a Singleton

class Database extends Singleton
{
    public function __construct(){

    }
}

This is not a singleton.  Add the following code:

$bd7 = new Config();

You have a new Config object.

class Config extends Singleton {

    protected function __construct() {

    }

}

This is a singleton.  It will give an error if you try to create a Config object directly.  Or

class Database extends Singleton {

}

Is a singleton that gives an error if you create a Database object directly as it inherits its constructor from its parent.

Registry

You don't need to use inheritance to get this functionality.

    public static function getInstance(){
        $class = get_called_class();
        if(!isset(self::$instance[$class]) || !self::$instance[$class] instanceof $class){
            self::$instance[$class] = new static(); // create and instance of child class which extends Singleton super class
            echo "new ". $class . PHP_EOL; // remove this line after testing
            return  self::$instance[$class]; // remove this line after testing
        }
        echo "old ". $class . PHP_EOL; // remove this line after testing
        return static::$instance[$class];
    }

Change to

    public static function getInstance($class, $object = null) {
        if (!isset(self::$instance[$class]) && $object != null) {
            self::$instance[$class] = $object;
        }

        return static::$instance[$class];
    }

or just

    public static function getInstance($key) {
        return static::$instance[$key];
    }

with

    public static function setInstance($key, $object) {
        static::$instance[$key] = $object;
    }

And change

$bd1 = Database::getInstance(); // new

to

Registry::setInstance('database', new Database());
$bd1 = Registry::getInstance('database');

Now, it's possible that in a real application you would make more use of what inheritance gives you.  But in this toy application, you don't.

I changed the name from Singleton to Registry, as better fitting this pattern.

You lose the just-in-time instantiation of the original, but you gain control over how it is instantiated.  An advantage of this form is that you can also do things like

Registry::setInstance('database', new MockDatabase());

Your original form prevented this, exacerbating the Singleton pattern's problems with user testing.

Nitpicks

/**
 * @dosc allows only one instance for each extending class
 * @example use it for database connection, config setup...
 * Be aware, singleton pattern is consider to be an antipatern and becaouse of it build it is hard to debug.
 * In most cases you do not need to use singleton patern so make a longer moment to think about it befor you use it.
 */

Without the spelling errors:

/**
 * @dosc allows only one instance for each extending class
 * @example use it for database connection, config setup...
 * Be aware, the singleton pattern is consider to be an anti-pattern
 * because it can be hard to debug.
 * In most cases you do not need to use singleton pattern
 * so take a longer moment to think about it before you use it.
 */

I also changed the wording a bit and broke up two lines so that they don't cause scrolling (at least in my browser).


Nearest color algorithm efficiency Ok so my name is qqqiiiiiiiifdff and I made this code that simulates a perceptual color picker algorithm that selects the nearest color by iterating over the color array and selecting the minimum error, processing the calculations in a perceptual manner.
This is an example:
#include <stdint.h>
#include <math.h>

inline double square(double x){return x*x;}

inline double sRGBtolinear(double input){
    if(input < 0) return -sRGBtolinear(-input);
    else if(input > 0.0404482362771082) return pow((input+0.055)/1.055, 2.4);
    else return input/12.92;
}

inline double lineartosRGB(double input){
    if(input < 0) return -lineartosRGB(-input);
    else if(input > 0.00313066844250063) return pow(input, 0.4166666666666667)*1.055-0.055;
    else return input*12.92;
}

inline double lineartosRGBderivative(double input){
    if(input < 0) return lineartosRGBderivative(-input);
    else if(input > 0.00313066844250063) return pow(input, -0.5833333333333334)*0.4166666666666667*1.055;
    else return 12.92;
}

uint64_t selectnearest(uint32_t* palette, uint64_t palettelength, uint32_t color){
    uint64_t index = 0;
    double maxerror = 1.0/0.0;
    for(uint64_t i=0; i<palettelength; i++){
        double color1[3];
        double color2[3];
        color1[0] = sRGBtolinear((color/65536%256)/255.0);
        color1[1] = sRGBtolinear((color/256%256)/255.0);
        color1[2] = sRGBtolinear((color%256)/255.0);
        color2[0] = sRGBtolinear((palette[i]/65536%256)/255.0);
        color2[1] = sRGBtolinear((palette[i]/256%256)/255.0);
        color2[2] = sRGBtolinear((palette[i]%256)/255.0);
        double initdistance = sqrt((square(212671.0*(color1[0]-color2[0]))+square(715160.0*(color1[1]-color2[1]))+square(72169.0*(color1[2]-color2[2])))/561891144402.0);
        double brightness1 = ((212671.0*color1[0])+(715160.0*color1[1])+(72169.0*color1[2]))/1000000.0;
        double brightness2 = ((212671.0*color2[0])+(715160.0*color2[1])+(72169.0*color2[2]))/1000000.0;
        double sRGBbrightness1 = lineartosRGB(brightness1);
        double sRGBbrightness2 = lineartosRGB(brightness2);
        double brightnessdifference = brightness2 - brightness1;
        double sRGBbrightnessdifference = sRGBbrightness2 - sRGBbrightness1;
        if(brightness1 == brightness2){
            brightnessdifference = 1;
            sRGBbrightnessdifference = lineartosRGBderivative(brightness1);
        }
        double result = initdistance*(sRGBbrightnessdifference/brightnessdifference);
        if(result < maxerror){
            index = i;
            maxerror = result;
        }
    }
    return index;
}

int main(){
    const uint64_t amountofcolors = 216;
    uint32_t palette[amountofcolors];
    for(uint64_t i=0; i<216; i++){
        palette[i] = i%6*0x000033 + i/6%6*0x003300 + i/36%6*0x330000;
    } // sample palette
    volatile uint32_t color;
    for(uint64_t i=0; i<65536; i++){
        color = (rand()%4096)+(rand()%4096*4096);
        color = palette[selectnearest(palette, amountofcolors, color)];
    }
}

To a person who does not know any code, this does not appear inefficient until it is run. What happened here? The process returned zero, and the execution time is 31.141 seconds. Now, it might seem like this is quite fast for 65536 colors to quantize to the palette, however you could imagine how a dynamic script to play a 256×256 movie in a reduced palette will play in 0.03 fps. Could this be a processing error and I have to try again? No, because the next time it is not much better at 31.085 seconds, and again at 31.110 seconds. Now, you could argue that a different color picker algorithm would be faster, however that is not the point because they don't pick colors the same way, the point is to make this specific algorithm faster, but still pick the colors about the same way. What should be done in order to improve the efficiency of this method?

There are a lot of performance aspects that can be improved in your code. I will cover general suggestions assuming that this code is for Win32 target. I will use minGw compiler with c11 standard.
Compiler Optimizations:
Without optimization code runs about 12.8 seconds on my PC. With -O3 optimization flag it takes about 10.6 seconds (about 17% faster).
Data types and arithmetic operations:
double type and constants with high precision are used in the code. Do you really need this precision? Moreover infinity is used in maxerror variable: double maxerror = 1.0/0.0; Consider to use DBL_MAX instead of inf. If performance is important then general algorythm of data types selection is following:

Can I use integer arithmetics? Prefereably int/uint types with logical/shift operations.
If not, then can I use float?
If not, then chose double or more precise types.
If floating point data types are used then consider NOT using floating point exceptions, NaN, Inf and denormal values. All these things can be very slow.

For example, you extract color components in the following way:
color1[0] = sRGBtolinear((color/65536%256)/255.0);

Here integer division can be replaced by bitwise/shift operations which is much faster. Assembly code will be almost 3 times shorter.
color1[0] = sRGBtolinear(((color >> 16) & 0xff)/255.0);

Inlining:
As it has been already mentioned inline keyword is confusing nowadays. Compilers have attributes or pragmas to always_inline/force_inline or noinline functions, because it is very important in some cases. E.g. if you want to insert assembly code with label in function wich compiler decided to inline. In this case compilation will fail. Only prohibiting inlining can help. In our case functions lineartosRGBOrig and lineartosRGBderivativeOrig can't be inlined because they are recursive. By the way, this recursion is not needed because input to these functions is always positive in current algorithm.
Algorithm changes and code rearrangment:

It is better to move variables declarations from the loop.
Values related to color in function selectnearest can be calculated only once istead of recalculation palettelength times.
RGBtolinear data can be precalculated as a table with 255 elements to significantly improve performance.
initdistance and brightness values are used only in comparison which allows us to remove sqrt operation and some divisions because we don't need exact values. Only relation is important.

Some brief optimizations:
I have made some optimizations that improved performance by 86% to original code compiled with -O3 flag and by 89% to code compiled without optimization. The code is not perfect but it is much faster.
#pragma GCC push_options
#pragma GCC optimize ("O3")

double gRgbToLinearTable[255];

void InitRgbToLinearTable(){
    for (size_t i = 0u; i < 255u; ++i)
    {
        double val = i / 255.0;
        gRgbToLinearTable[i] = (val > 0.0404482362771082 ? pow((val+0.055)/1.055, 2.4) : val/12.92);
    }
}

[[gnu::always_inline]] inline double square(double x) { return x*x; }

[[gnu::always_inline, gnu::flatten]] inline void sRGBtolinear(double* outComponents, uint32_t color){
    outComponents[0] = gRgbToLinearTable[(color >> 16) & 0xff];
    outComponents[1] = gRgbToLinearTable[(color >> 8) & 0xff];
    outComponents[2] = gRgbToLinearTable[color & 0xff];
}

[[gnu::always_inline, gnu::flatten]] inline double lineartosRGB(double input){
    if (input > 0.00313066844250063)
        return (pow(input, 0.4166666666666667)*1.055-0.055);
    else
        return input*12.92;
}

[[gnu::always_inline, gnu::flatten]] inline double lineartosRGBderivative(double input){
    if(std::abs(input) > 0.00313066844250063) return pow(input, -0.5833333333333334)*0.4166666666666667*1.055;
    else return 12.92;
}

size_t selectnearest(const uint32_t* palette, size_t palettelength, uint32_t color)
{
    size_t index = 0;
    double maxerror = DBL_MAX;
    double colors[2][3];
    double initdistance, brightness[2], rgbBrightness[2], brightnessdifference, sRGBbrightnessdifference;
    sRGBtolinear(colors[0], color);
    brightness[0] = (0.212671*colors[0][0])+(0.715160*colors[0][1])+(0.072169*colors[0][2]);
    rgbBrightness[0] = lineartosRGB(brightness[0]);

    for (size_t i = 0; i < palettelength; i++)
    {
        if (palette[i] != color)
        {
            sRGBtolinear(colors[1], palette[i]);

            initdistance = (square(0.212671*(colors[0][0]-colors[1][0]))+square(0.212671*(colors[0][1]-colors[1][1]))+square(0.072169*(colors[0][2]-colors[1][2])));
            brightness[1] = (0.212671*colors[1][0])+(0.715160*colors[1][1])+(0.072169*colors[1][2]);
            rgbBrightness[1] = lineartosRGB(brightness[1]);

            if(brightness[0] != brightness[1])
            {
                brightnessdifference = brightness[1] - brightness[0];
                sRGBbrightnessdifference = rgbBrightness[1] - rgbBrightness[0];
            }
            else
            {
                brightnessdifference = 1.0 ;
                sRGBbrightnessdifference = lineartosRGBderivative(brightness[0]);
            }

            double result = initdistance*(sRGBbrightnessdifference/brightnessdifference);
            if(result < maxerror){
                index = i;
                maxerror = result;
            }
        }
        else
        {
            return i;
        }
    }

    return index;
}

uint32_t* MakeSamplePalette(size_t amountOfColors)
{
    uint32_t* pResult = new uint32_t[amountOfColors];
    for(size_t i = 0; i < amountOfColors; i++)
    {
        pResult[i] = i%6*0x000033 + i/6%6*0x003300 + i/36%6*0x330000;
    }

    return pResult;
}

int main()
{
    const size_t amountofcolors = 216u;
    uint32_t* palette = MakeSamplePalette(amountofcolors);
    volatile uint32_t color;

    InitRgbToLinearTable();
    for(size_t i = 0; i < 65536u; i++){
        color = (static_cast<unsigned int>(rand()) & 0xfff)+(static_cast<unsigned int>((rand()) & 0xfff) << 12);
        color = palette[selectnearest(palette, amountofcolors, color)];
    }

    delete[] palette;
    return color;
}
#pragma GCC pop_options


Quickly filter an object by keys I am trying to optimize an object filter function. Given an array of keys, I need to filter an object. I feel that creating a new object each time may degrade performance. Is there a way to mock the Array.prototype.filter function for objects?

function main() {
    var Types = {
        INTEGER:   { value: 'int'    },
        CHARACTER: { value: 'char'   },
        FLOAT:     { value: 'float'  },
        DOUBLE :   { value: 'double' },
        STRING:    { value: 'str'    },
        BOOLEAN:   { value: 'bool'   }
    };

    var Numbers = filterByKeys(Types, [ 'INTEGER', 'FLOAT', 'DOUBLE' ]);

    console.log(Numbers);
};

function listToSet(list) {
    var _set = {};
    if (Ext.isArray(list)) {
        for (var i = 0; i < list.length; i++) {
            _set[list[i]] = true;
        }
    }
    return _set;
};

function filterByKeys(obj, keep) {
    var result = {},
        unfiltered = keep === undefined,
        keys = listToSet(keep);
    if (Ext.isObject(obj)) {
        Ext.Array.each(Object.keys(obj), function(key) {
            if (unfiltered ||  (!unfiltered && keys[key])) {
                result[key] = obj[key];
            }
        });
    }
    return result;
};

Ext.onReady(function () {
    main();
});
.as-console-wrapper { top: 0; max-height: 100% !important; }
<script src="https://cdnjs.cloudflare.com/ajax/libs/extjs/4.2.1/ext-all.js"></script>

Without ExtJS Dependency

function main() {
    var Types = {
        INTEGER:   { value: 'int'    },
        CHARACTER: { value: 'char'   },
        FLOAT:     { value: 'float'  },
        DOUBLE :   { value: 'double' },
        STRING:    { value: 'str'    },
        BOOLEAN:   { value: 'bool'   }
    };

    var Numbers = filterByKeys(Types, [ 'INTEGER', 'FLOAT', 'DOUBLE' ]);

    console.log(Numbers);
};

function listToSet(list) {
    var _set = {};
    if (isArr(list)) {
        for (var i = 0; i < list.length; i++) {
            _set[list[i]] = true;
        }
    }
    return _set;
};

function filterByKeys(obj, keep) {
    var result = {},
        unfiltered = keep === undefined,
        keys = listToSet(keep);
    if (isObj(obj)) {
        for (var key in obj) {
            if (unfiltered ||  (!unfiltered && keys[key])) {
                result[key] = obj[key];
            }
        };
    }
    return result;
};

var tObj='[object Object]',tArr='[object Array]',str=Object.prototype.toString;
var isArr='isArray'in Array?Array.isArray:function(v){return str.call(v)===tArr};
var isObj=str.call(null)===tObj?function(v){return v!==null&&v!==void 0&&str.call(v)===tObj&&v.ownerDocument===void 0}:function(v){return str.call(v)===tObj
};

$(document).ready(function () {
    main();
});
.as-console-wrapper { top: 0; max-height: 100% !important; }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

Use var myKeys = Object.keys(myObject) to get the keys.
Check if a myString exist in the array myKeys using native

var matchingKey = myKeys.indexOf(myString) !== -1

Processing an array? Use this:

var matchingKeys = myKeys.filter(function(key){ return key.indexOf(myString) !== -1 });

Get the value using myObject[matchingKey].

Processing an array? Use this:

var matchingValues = matchingKeys .map(function(key){ return myObject[key] });

No need to check array-boundaries or sizes. It's 100% boundary-safe.


Getting a transpose of a matrix of size of n*n where n=2^m using multithreading The following is a c++ source code to get a matrix (std::vector<std::vector<T>>) transpose in parallel.

The span is \$\Theta(\lg^2(n))\$ while the work is \$\Theta(n^2)\$

Any suggestion to improve will be appreciated.

#include<vector>
#include<iostream>
#include<future>
#include<iomanip>
template <typename T>
void parallelForColsTrans(std::vector<std::vector<T>>& A, size_t fCol, size_t lCol, size_t i, size_t n)
{
    if (fCol == lCol)
    {
        T temp = A[i][fCol+n];
        A[i][fCol+n] = A[i+n][fCol];
        A[i+n][fCol] = temp;

        return;
    }

    std::async(parallelForColsTrans<T>, std::ref(A), fCol, (fCol + lCol) / 2, i, n);
    std::async(parallelForColsTrans<T>, std::ref(A), (fCol + lCol) / 2 + 1, lCol, i, n);

}

template <typename T>
void parallelForRowsTrans(std::vector<std::vector<T>>& A, size_t fRow, size_t lRow, size_t fCol, size_t lCol, size_t n)
{
    if (fRow == lRow) {
        parallelForColsTrans(A, fCol, lCol, fRow, n);
        return;
    }

    std::async(parallelForRowsTrans<T>, std::ref(A), fRow, (fRow+lRow) / 2, fCol, lCol, n);
    std::async(parallelForRowsTrans<T>, std::ref(A), (fRow + lRow) / 2 + 1, lRow, fCol, lCol, n);
}

template <typename T>
void pMatTransposeRecursive(std::vector<std::vector<T>>& A, size_t firstRow, size_t lastRow, size_t firstColumn, size_t lastColumn)
{
    if (firstRow == lastRow)    return;

    auto t1 =std::async(pMatTransposeRecursive<T>, std::ref(A), firstRow, (firstRow +lastRow)/2, firstColumn, (firstColumn+lastColumn)/2);
    auto t2 =std::async(pMatTransposeRecursive<T>, std::ref(A), (firstRow +lastRow)/2+1, lastRow, firstColumn, (firstColumn+lastColumn)/2);
    auto t3 =std::async(pMatTransposeRecursive<T>, std::ref(A), firstRow, (firstRow +lastRow)/2, (firstColumn+lastColumn)/2+1, lastColumn);
    pMatTransposeRecursive<T>(std::ref(A), (firstRow +lastRow)/2+1, lastRow, (firstColumn+lastColumn)/2+1, lastColumn);
    t1.get();
    t2.get();
    t3.get();
    size_t n = (lastColumn-firstColumn+1)/2;
    parallelForRowsTrans<T>(std::ref(A), firstRow, firstRow+n-1, firstColumn, firstColumn+n-1, n);

}

template <typename T>
void transpose(std::vector<std::vector<T>>& A){
    pMatTransposeRecursive( A, 0, A.size()-1, 0, A[0].size()-1);
}
int main(){

    std::vector<std::vector<int>> A = {{1,2,3,4,7,7,7,8}, {5,6,7,8,4,5,1,1}, {9,10,5,5,11,12,4,79}, {7,8,13,14,15,16,44,6}, {13,-14,7,-7,15,-16,-44,6}, {13,-14,105,106,404,6,9,9}, {13,-14,7,-7,15,-16,-44,6}, {13,-14,105,106,404,6,9,9}};
    transpose(A);
    for(auto & el:A){
        for(auto& ele:el) std::cout << ele << std::setw(4) ;
        std::cout << "\n";

    }
}

Is it worth parallelising?

When you are parallelizing code you have to ask yourself if it's worth doing that. Parallel code doesn't magically give a speedup, because there are various things that can actually slow you down when using parallelism, for example:

Spawning and waiting for threads itself costs some time.
Threads might be fighting for resources, like locks, but even just memory access might cause contention.
Code that is not trivially parallelizable might require more computations when parallelized than when running single-threaded.

In the case of transposing matrices, there are almost no computations involved, the work is completely dominated by reading and writing to memory. A good single-threaded implementation, which reads sequentially as much as possible, will likely be able to saturate the memory bus on typical desktop machines.

In your implementation, you have a huge overhead just from starting all the async work items: you call async() more often than there are elements in the matrix. So that alone will make this horribly slow. Most good parallel algorithms try to limit the number of threads to the number of CPU cores or hardware threads that are available.

Even if you didn't parallelize, but kept the structure of the code the same (just not use async()), then the recursive divide-and-conquer approach will cause a non-sequential memory access pattern.

Code review

Nested std::vector<>s are not efficient containers for matrices

While it's convenient to declare a 2D matrix using std::vector<std::vector<T>>, it is not very efficient, since there is a lot of indirection. It is more efficient to declare a single std::vector<T> of size n * n, and index the vector using [row * n + col], or use a C++ library that provides proper matrix classes.

Use std::swap()

The standard library provides std::swap(), which swaps two variables for you.


Coin tossing simulation to get x heads in a row, ten times - timely at higher amount of heads I have created a program to simulate a coin being tossed to land on heads/tails x amount of times in a row, where x can be defined by the user. The program then gets x amount of heads in a row, a fixed ten times. This is to generate an average.

using System;

namespace HeadsStreak
{
    class MainClass
    {
        public static void Main(string[] args)
        {
            Console.WriteLine("Enter a head streak to get. This will be executed ten times to generate an average.");
            int desiredStreak = Convert.ToInt32(Console.ReadLine());
            int grandTotalAttempts = 0;
            int executionCount = 1;
            int currentStreak = 0;
            int totalAttempts = 0;
            Random rnd = new Random();
            do
            {
                while (currentStreak != desiredStreak)
                {
                    if (rnd.Next(0, 2) == 0)
                    {
                        currentStreak += 1;
                    }
                    else
                    {
                        currentStreak = 0;
                    }
                    totalAttempts += 1;
                }
                Console.WriteLine("Attempt Number: {0}", executionCount);
                Console.WriteLine("Total Attempts: {0}", totalAttempts);
                Console.WriteLine("");
                grandTotalAttempts += totalAttempts;
                executionCount += 1;
                currentStreak = 0;
                totalAttempts = 0;
            } while (executionCount <= 10);
            Console.WriteLine("Average Attempts: {0}", grandTotalAttempts/10);
        }
    }
}

Let's say I ask for fifteen heads. The console may look something like this:

Enter a head streak to get. This will be executed five times to generate an average.
15
Attempt Number: 1
Total Attempts: 82761

Attempt Number: 2
Total Attempts: 42464

Attempt Number: 3
Total Attempts: 157531

Attempt Number: 4
Total Attempts: 79750

Attempt Number: 5
Total Attempts: 19214

Attempt Number: 6
Total Attempts: 4573

Attempt Number: 7
Total Attempts: 8161

Attempt Number: 8
Total Attempts: 99553

Attempt Number: 9
Total Attempts: 41036

Attempt Number: 10
Total Attempts: 84431

Average Attempts: 61947

This can execute fairly quickly within a reasonable timeframe. Time issues begin at 24, I assume this is because it takes 2^24 attempts to get 24 heads in a row. However, I am wondering if there is anything to improve in the code in order to improve performance so that higher streaks can be achieved within a reasonable timeframe?

Fails if they don't enter an integer
You are doing integer math on the average
I would prefer ++; over += 1;
I would rather see a foreach
Odd but rnd.Next(2) is faster
Need to get into the millions for a decent sample

public static void CoinToss()
{
    int sample = 1000000;
    Debug.WriteLine($"Enter a head streak to achieve. This will be executed {sample.ToString("N0")} times to generate an average.");
    int desiredStreak = 10;
    int grandTotalAttempts = 0;
    int currentStreak;
    int totalAttempts;
    Random rnd = new Random();
    for (int i = 0; i < sample; i++)
    {
        currentStreak = 0;
        totalAttempts = 0;
        while (currentStreak != desiredStreak)
        {
            if (rnd.Next(2) == 0)
            {
                currentStreak ++;
            }
            else
            {
                currentStreak = 0;
            }
            totalAttempts ++;
        }
        if ((i + 1) % (sample / 20) == 0)
        {
            Debug.WriteLine($"Attempt Number: {(i + 1).ToString("N0")}");
            Debug.WriteLine($"Total Attempts: {totalAttempts.ToString("N0")}");
            Debug.WriteLine("");
        }
        grandTotalAttempts += totalAttempts;
    }
    Debug.WriteLine($"Average Attempts: {((decimal)grandTotalAttempts / sample).ToString("N2")}");
}


Condense repetitive jQuery .click function I'm looking for a better way of writing this repetitive jQuery function:

(function($) {
  var mobileAmount = 300;

  $('#_100').click(function() {
    $('*').removeClass('-amount-active');
    $('#_100').addClass('-amount-active');
    mobileAmount = 100;
  });
  $('#_200').click(function() {
    $('*').removeClass('-amount-active');
    $('#_200').addClass('-amount-active');
    mobileAmount = 200;
  });
  $('#_300').click(function() {
    $('*').removeClass('-amount-active');
    $('#_300').addClass('-amount-active');
    mobileAmount = 300;
  });

  ...

})( jQuery );

There is a number of amount buttons and each time one is clicked it receives the active class for styling and updates the mobileAmount variable.

Steps to fix this:

Add class 'amount-button' to all buttons
Add attribute data-mobileamount to all buttons and assign values like
100, 200, .. to each button.

For example:
<button class="amount-button" data-mobileamount="100" >Mobile Amount 100</button >
<button class="amount-button" data-mobileamount="200">Mobile Amount 200</button>

 (function($) {
  var mobileAmount = 300;
  $('.amount-button').click(function() {
    $('*').removeClass('-amount-active');
    $(this).addClass('-amount-active');
    mobileAmount = $(this).data('mobileamount');
  });
})( jQuery );


EF query for calculating monthly trends My application has a fairly important query that is used in a lot of places. Unfortunately it takes about 14 seconds to run, so I'd like to find a way of possibly improving it.

The application receives incidents (right now there's about 70,000 total incidents, and about 16,000 incidents in this query), with each incident having a range of different flags. The purpose of the query is to get a monthly trend of the incidents and their flags.

For example:

Month |  Flagged  |  Falls  |  FallsWithInjury  | ....
-------------------------------------------------------
Jan        10          3            1
Feb        15          6            0
Mar        14          2            2
....

which I can then graph client side.

In the following query, FacilityIds are a list of facilities I want to load incidents for (about 80 IDs in the array). The start and end filters are 6 months apart.

var query = from i in Db.Incidents.Where(x => FacilityIds.Contains(x.FacilityId))
            where i.IncidentDate >= start && i.IncidentDate <= end
            group i by new { i.IncidentDate.Year, i.IncidentDate.Month } into incidentsByMonth
            let date = incidentsByMonth.Key
            select new MonthlyStat
                        {
                            Year = date.Year,
                            Month = date.Month,
                            AllIncidents = incidentsByMonth.Count(),
                            Flagged = incidentsByMonth.Count(ibm => ibm.IsSerious),
                            SentToHospital = incidentsByMonth.Count(x => x.Treatments.Any(it => it.TreatmentTypeId == (int)TreatmentTypes.Senttohospital)),
                            FallsWithInjuries = incidentsByMonth.Count(ibm => ibm.IncidentTypes.Any(it => it.IncidentTypeId == (int)IncidentTypes.Fall) && ibm.InjuryTypes.Any(inj => inj.InjuryTypeId != (int)InjuryTypes.None)),
                            FallsWithFracture = incidentsByMonth.Count(ibm => ibm.IncidentTypes.Any(it => it.IncidentTypeId == (int)IncidentTypes.Fall) && ibm.InjuryTypes.Any(inj => inj.InjuryTypeId == (int)InjuryTypes.Fracture)),
                            IncidentTypes = incidentsByMonth.SelectMany(ibm => ibm.IncidentTypes).GroupBy(it => new { it.IncidentTypeId, it.Description }).Select(it => new IncidentProperty<IncidentTypes>
                            {
                                Key = (IncidentTypes)it.Key.IncidentTypeId,
                                Count = it.Count()
                            }),
                            InjuryTypes = incidentsByMonth.SelectMany(ibm => ibm.InjuryTypes).GroupBy(it => new { it.InjuryTypeId, it.Description }).Select(it => new IncidentProperty<InjuryTypes>
                            {
                                Key = (InjuryTypes)it.Key.InjuryTypeId,
                                Count = it.Count()
                            }),
                            DayOfWeek = incidentsByMonth.GroupBy(ibm => SqlFunctions.DatePart("weekday", ibm.IncidentDate)).Select(ibm => new Aggregate<DayOfWeek> { Key = (DayOfWeek)ibm.Key.Value, Count = ibm.Count() }),
                            IncidentsByHour = incidentsByMonth.GroupBy(ibm => ibm.IncidentDate.Hour).Select(h => new Aggregate<int> { Key = h.Key, Count = h.Count() }),
                            PotentialIncurredMedian = incidentsByMonth.Select(x => x.PotentialIncurredMedian * .4).DefaultIfEmpty(0).Sum(),
                        };

Profiling different components, it looks like the IncidentTypes and InjuryTypes properties account for about 6 seconds of the 14 seconds totals. I've tried rewriting it a few different ways, but all seem to be worse in terms of performance.

I've started writing the query in native T-SQL, but I think it's a bit complex for my abilities right now.

Any suggestions would be greatly appreciated.

Native T-SQL
I don't know enough about linq to do a proper code review for that, however as far as performance is concerned I think T-SQL would take care of the expensive part of your query more efficiently.
The following query will not get you all the information from your entire query, but it would take care of at least this section:
Year = date.Year,
Month = date.Month,
AllIncidents = incidentsByMonth.Count(),
Flagged = incidentsByMonth.Count(ibm => ibm.IsSerious),
SentToHospital = incidentsByMonth.Count(x => x.Treatments.Any(it => it.TreatmentTypeId == (int)TreatmentTypes.Senttohospital)),
FallsWithInjuries = incidentsByMonth.Count(ibm => ibm.IncidentTypes.Any(it => it.IncidentTypeId == (int)IncidentTypes.Fall) && ibm.InjuryTypes.Any(inj => inj.InjuryTypeId != (int)InjuryTypes.None)),
FallsWithFracture = incidentsByMonth.Count(ibm => ibm.IncidentTypes.Any(it => it.IncidentTypeId == (int)IncidentTypes.Fall) && ibm.InjuryTypes.Any(inj => inj.InjuryTypeId == (int)InjuryTypes.Fracture)),

Note that I made assumptions about your schema design and you may need to make some adjustments. Also please note that I used more descriptive table aliases as well to make it easier for Mr. Maintainer.
DECLARE @dateStart DATE = '2014-01-01'; -- or whichever date you want
DECLARE @dateEnd DATE = DATEADD(MONTH, 6, @dateStart); -- 6 months later e.g. July 1 2014
SELECT
    DATEPART(yyyy, incd.date) AS Year,
    DATEPART(mm, incd.date) AS Month,
    COUNT(incd.IncidentId) AS AllIncidents,
    COUNT(incd.IsSerious) AS Flagged,
    (SELECT COUNT incd.IncidentId
        FROM incd
        WHERE treat.TreatmentType = 'SenttoHospital') AS SentToHospital,
    (SELECT COUNT incd.IncidentId
        FROM incd
        WHERE incdType = 'Fall'
        AND injuType.InjuryType <> 'None') AS FallsWithInjuries,
    (SELECT COUNT incd.IncidentId
        FROM incd
        WHERE incdType = 'Fall'
        AND injuType.InjuryType = 'Fracture') AS FallsWithFracture,
FROM Incidents AS incd
LEFT JOIN IncidentTypes AS incdType ON inc.IncidenTypeId = incdType.IncidentTypeId
LEFT JOIN InjuryTypes AS injuType ON inc.InjuryTypeId = injuType.InjuryTypeId
LEFT JOIN Treatments AS treat ON inc.TreatmentTypeId = treat.TreatmentTypeId
WHERE incd.date >= @dateStart
AND incd.date < @dateEnd;


Optimizing JSON append template I am fetching a list of messages from some JSON and appending each message as an LI within a UL. I have created the message template within jQuery to append each message to the document. My code works well but I can't help but feel it could be improved (I am a beginner JS programmer). From what I have seen online moving the templating to handlebars.js would be a better solution but I can't be sure.

$(document).ready(function() {
  var broadcastMessagesJsonURL = "https://api.myjson.com/bins/sadap";
  $.getJSON(broadcastMessagesJsonURL, function(data) {
  $.each(data.broadcastMessages, function(i, item) {

  //All items from the json
  var broadcastMessageID = (item.ID);
  var checkBroadcastMessageRead = (item.read ? " broadcast__message__read__state--unread" : "");
  var broadcastMessageSubject = (item.subject);
  var broadcastMessageGroup = (item.group);
  var broadcastMessageDateSent = (item.dateSent);
  var checkBroadcastFeatureImage = (item.featureImage ? " broadcast__message__image--active" : "");
  var checkBroadcastForm = (item.form ? " broadcast__message__form--active" : "");
  var checkBroadcastAttachments = (item.attachments ? " broadcast__message__attachment--active" : "");
  var broadcastMessageContent = (item.content);

  var broadcastMessageTemplate = ('
 \

 \

 \

' + broadcastMessageSubject + '
 \

' + broadcastMessageGroup + '
 \

' + broadcastMessageDateSent + '
 \

' + broadcastMessageContent + '
 \

 \

');
  $(".broadcast__messages__list").append(broadcastMessageTemplate);
});
  }).fail(function() {
    console.log("broadcastMessages json cannot be loaded");
  });
});

CODEPEN LINK

Rewrite

Below is a rewrite with all my further suggestions applied.

const messagesURL = 'https://api.myjson.com/bins/sadap';

const messagesLoadError = () => console.error('broadcastMessages json cannot be loaded');

const forEachMessage = (_, message) => {
  const messageRead  = message.read         ? ' broadcast__message__read__state--unread' : '',
        featureImage = message.featureImage ? ' broadcast__message__image--active'       : '',
        form         = message.form         ? ' broadcast__message__form--active'        : '',
        attachments  = message.attachments  ? ' broadcast__message__attachment--active'  : '';

  const template = `

${message.subject}

${message.group}

${message.dateSent}

${message.content}

`;

  $('.broadcast__messages__list').append(template);
};

$.getJSON(messagesURL, data => $.each(data.broadcastMessages, forEachMessage)).fail(messagesLoadError);

Remarks

API security

Your API endpoint (https://api.myjson.com/bins/sadap) uses insecure and archaic SSL configuration. Actually, I couldn't connect to it at all, since I have tweaked up browser configuration. Few key points about your API's security:

Vulnerable to the OpenSSL CCS vulnerability (CVE-2014-0224),
Relies fully on insecure RC4 or legacy CBC mode of ciphers it uses,
Doesn't support Forward Secrecy.

See Qualys' test result.

Unused callback's parameter

You have the following line in your code:

$.each(data.broadcastMessages, function(i, item) {

but i is never used. In such cases, it's good to replace it with underscore (_).

Naming issues

As Leon Bambrick added on top of Phil Karlton's quote:

  There are only two hard things in Computer Science: cache
  invalidation, naming things, and off-by-one errors.

„Some” of your names are overly verbose (word broadcast seem to be everywhere) and other are not descriptive enough, like item.

Wall of text

This

var broadcastMessageID = (item.ID);
var checkBroadcastMessageRead = (item.read ? " broadcast__message__read__state--unread" : "");
var broadcastMessageSubject = (item.subject);
var broadcastMessageGroup = (item.group);
var broadcastMessageDateSent = (item.dateSent);
var checkBroadcastFeatureImage = (item.featureImage ? " broadcast__message__image--active" : "");
var checkBroadcastForm = (item.form ? " broadcast__message__form--active" : "");
var checkBroadcastAttachments = (item.attachments ? " broadcast__message__attachment--active" : "");
var broadcastMessageContent = (item.content);

would be much easier to read, if you would combine vars and align equality signs, question marks and colons:

var broadcastMessageID         = item.ID,
    broadcastMessageSubject    = item.subject,
    broadcastMessageGroup      = item.group,
    broadcastMessageDateSent   = item.dateSent,
    broadcastMessageContent    = item.content,
    checkBroadcastMessageRead  = item.read         ? ' broadcast__message__read__state--unread' : '',
    checkBroadcastFeatureImage = item.featureImage ? ' broadcast__message__image--active'       : '',
    checkBroadcastForm         = item.form         ? ' broadcast__message__form--active'        : '',
    checkBroadcastAttachments  = item.attachments  ? ' broadcast__message__attachment--active'  : '';

Variables

Most of your variables are unnecessary. If you would rename ambiguous item to message, you could use message.ID instead of doing var broadcastMessageID = (item.ID).

Also, bear in mind that the following three variables are unused: checkBroadcastFeatureImage, checkBroadcastForm, checkBroadcastAttachments.

Use template literals

var broadcastMessageTemplate could become much more easily readable if you used template literals to declare it. Mind however, that this is still not the „most vanilla” method to create DOM elements either. Take a look at createElement().

Fail notification

You should use console.error() instead of console.log() in your fail's callback.


PHP password-resetting page I am producing a PHP code and I want to implement it in my server to do its function. My question is, are there any vulnerabilities that I should correct?

I tried to establish the session first then I will initiate a SQL connection and I will ask the user to enter his email. Also I added the reset function of his email.

<?php
if ($_SESSION['admin'] !== true) {
    header('Location: /login.php?');
}

$conn = new mysqli("localhost","admin","password","db");

if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

$msg = '';
if (isset($_POST['submit'])) {
    $email = $_POST['email'];
    $sql = "SELECT * FROM users WHERE email = $email;";
    $result = @$conn->query($sql);
    if ($result->num_rows == 0) {
        die("This email: $email is incorrect");
    }
    $row = $result->fetch_assoc();
    $id = $row["id"];
    $surename = $row["surename"];

    $sql = "SELECT * from users JOIN password_resets ON (password_resets.user_id=users.id)  WHERE id=$id;";
    $result = @$conn->query($query);
    if ($result->num_rows == 0) {
        $insert_sql = "INSERT INTO password_resets (user_id, reset_timestamp) VALUES ($id, unix_timestamp());";
        if ($conn->query($insert_sql) !== TRUE) {
            die("Error: " . $insert_sql . "
" . $conn->error);
        }
    }
    $result = $conn->query($query);
    $row = $result->fetch_assoc();

    $link = "https://".$_SERVER['SERVER_NAME']."/reset.php?id=".$row['id'] ."&timestamp=".$row['reset_timestamp'];
    $message = "Hi, $surename,

    Here's your password reset link: $link";
    $headers = "From: ".$_SESSION['first_name']." ".$_SESSION['last_name']." <".$_SESSION['email'].">";
    mail($email,"Your password reset link",$message,$headers);
    $msg = "Password reset for $email sent!";
?>

<title>Reset password for a user</title>

  <form>

<?=$msg ?>

    <input type="text" width=80 name="email">
    <input type="submit" name="submit" value="Send">
  </form>

<?php
}

Your code is wide open to SQL Injection which is the most dangerous web vulnerability. This occurs when you execute SQL queries with unsanitized user data. By placing the raw $_POST variable directly in your query you are allowing an attacker to inject their own SQL into your query and execute it. They can do anything from stealing data to deleting your database.

To combat this you must use parameterized queries. Stack Overflow covers this very well but here's what your code would like if you use mysqli with prepared statements:

$stmt = $dbConnection->prepare('SELECT * FROM users WHERE email = ?');
$stmt->bind_param('s', $email); // 's' specifies the variable type => 'string'

$stmt->execute();

$result = $stmt->get_result();
$row = $result->fetch_assoc();

You also are wide open to Cross-site scripting (XSS) attacks which is the #7 web vulnerability. This occurs because you take raw $_POST data and output it directly into your HTML. An attacker can place malicious code in this value and attack your users and site once it is rendered by the browser.

When outputting user data, always escape that data. Stack Overflow covers this as well. In PHP you can do this by using htmlspecialchars().

<?= htmlspecialchars($msg, ENT_QUOTES, 'UTF-8'); ?>

You will notice that the one $_POST variable, $_POST['email'], has left your site wide open to attack. Before you even attempt to use it you should validate it indeed is a valid email address. If it is not, you should report an error and not attempt to use it as it obviously is invalid and useless anyway.

PHP offers an easy way to validate an email addresses. filter_var() with the FILTER_VALIDATE_EMAIL flag will validate if an email address is valid.

$email = $_POST['email'];
if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
    // email is invalid. report error and abort.
}

On a different note, you start your script off by checking to see if a user is an admin. If they are not you use header() to do a redirect away from that page. That's usually okay but you should follow it with a call to exit() to ensure the script stops executing. If not, the code below may still execute and, combined with other vulnerabilities in the page, leave you open to attack.

if ($_SESSION['admin'] !== true) {
    header('Location: /login.php?');
    exit;
}


Yet another smart pointer implementation for learning I based this implementation on CountedPtr in The C++ Standard Library book by Nicolai Josuttis (page 222) and also available online here.

I know that I can use the C++11 smart pointers but this is a learning exercise and also for some projects an earlier compiler must be used.

I didn't understand the need for the actual reference counter to be a pointer so I just used a standard integer.  Maybe that introduces a bug, not sure.

My main concern is about the object lifecycle.  Does the reference counter as a straight integer cause a bug?  Any feedback will be much appreciated.

Here is the smart pointer class:

#ifndef SMARTPOINTER_HPP__
#define SMARTPOINTER_HPP__

#include <iostream>

template<typename T>
class sp {
public:
   sp(T* ptr) : ptr_(ptr), ref_cnt_(1) {
      std::cout << "sp ctor: " << ptr_ << ", ref_cnt_: " << ref_cnt_ << std::endl;
   }

   T& operator*() { return *ptr_; }

   T* operator->() { return ptr_; }

   sp(const sp<T>& rhs) : ptr_(rhs.ptr_), ref_cnt_(rhs.ref_cnt_) {
      ++ref_cnt_;
      std::cout << "sp copy ctor: " << ptr_ << ", ref_cnt_: " << ref_cnt_ << std::endl;
   }

   sp& operator=(const sp& rhs) {
      if(this != &rhs) {
         *this = rhs;
         ++ref_cnt_;
         std::cout << "sp assignment: " << ptr_ << ", ref_cnt_: " << ref_cnt_ << std::endl;
      }
      return *this;
   }

   ~sp() {
     if(ptr_)
       std::cout << "~sp: " << ptr_ << ", ref count: " << ref_cnt_ << std::endl;
       --ref_cnt_;
       if(ref_cnt_ == 0)
         delete ptr_;
   }

   bool operator==(const T& that) const { return ptr_ == that.ptr_; }
   bool operator==(T* that) const { return ptr_ == that; }

   bool operator!=(const sp& that) const { return ptr_ != that.ptr_; }
   bool operator!=(sp* that) const { return ptr_ != that; }

private:
   T* ptr_;
   unsigned ref_cnt_;
};

#endif //SMARTPOINTER_HPP__

Here is my code to exercise:

#include <iostream>
#include <string.h>

#include "smart_pointer.hpp"

class name {
public:
   name(const char* label) : label_(label) {}

   const char* get_name() const { return label_; }
   unsigned length() const { return strlen(label_); }

private:
  const char* label_;
};

int main() {

   {
      std::cout << "Test with ints\n";
      sp<int> myptr1 = new int(3);
      *myptr1 = 4;
      std::cout << "number: " << *myptr1 << std::endl;

      sp<int> myp2(myptr1);

      sp<int> myp3(myptr1);

      sp<int> myp4 = myptr1;
   }

   {
     std::cout << "Test with name object\n";
     sp<name> myname = new name("Andrew");
     std::cout << myname->get_name() << " has length: " << myname->length() << std::endl;
     sp<name> myname2(myname);
   }

   return 0;
}

I get this output:

Test with ints
sp ctor: 004C7A80, ref_cnt_: 1
number: 4
sp copy ctor: 004C7A80, ref_cnt_: 2
sp copy ctor: 004C7A80, ref_cnt_: 2
sp copy ctor: 004C7A80, ref_cnt_: 2
~sp: 004C7A80, ref count: 2
~sp: 004C7A80, ref count: 2
~sp: 004C7A80, ref count: 2
~sp: 004C7A80, ref count: 1
Test with name object
sp ctor: 004C7A80, ref_cnt_: 1
Andrew has length: 6
sp copy ctor: 004C7A80, ref_cnt_: 2
~sp: 004C7A80, ref count: 2
~sp: 004C7A80, ref count: 1

Main problem with the reference counter:

As it stands right now, your code will eventually leak or try to delete the same pointer more than once. You are keeping a diferent reference count inside each smart pointer, which is not synchronised, so smart pointers sharing the same object can have different counts for the same memory. Take this simple example, similar to your second one:

sp<MyObject> ptr1 = new MyObject(); // ptr1.ref_cnt = 1
sp<MyObject> ptr2 = ptr1; // ptr1.ref_cnt = 1, ptr2.ref_cnt = 2
// Now we have two different counts for the same object!

Once you pass the copy constructor or assignment operator, the reference count of the original pointer is left unaltered. This cannot possibly end well. That's why in the original example you've linked the author used a pointer for the reference counter as well, so that any smart pointer sharing ownership of an object can also point to the same counter. Once the owned object is to be freed, the last pointer also frees the counter.

One cleaver trick to avoid allocating the counter in a separate new call is to allocate both the object and the counter as the same block of raw bytes, then adjust the pointers and construct the object on the remaining memory. This is how some implementations of std::make_shared() work. Once you fix the counter issue, you might want to try this optimization.

Other issues with the code:

This destructor seems broken:

   ~sp() {
     if(ptr_)
       std::cout << "~sp: " << ptr_ << ", ref count: " << ref_cnt_ << std::endl;
       --ref_cnt_;
       if(ref_cnt_ == 0)
         delete ptr_;
   }

Didn't you forget to wrap the first if with curly braces? This seems like a case where the indenting of the code tries to tell the reader one thing, while the code does a different thing. The first if only applied to the cout call. To avoid this confusion, it is a good idea to always provide { } even for single line control statements. So this is how I believe you intended to write, since it wouldn't make sense decrementing the counter if the pointer is null:

~sp() {
    if(ptr_) {
        std::cout << "~sp: " << ptr_ << ", ref count: " << ref_cnt_ << std::endl;
        --ref_cnt_;
        if(ref_cnt_ == 0) {
            delete ptr_;
        }
    }
}

The comparison operators that take a raw pointer should take that pointer by const T*, to make clear that they do not attempt to modify the pointed object.
You'll probably also want to provide overloads of operator * and -> that are const and return const pointer and reference. Otherwise you aren't able to call those on a const sp<T>.
Your class still requires a lot of tuning and polishing to be anywhere near usable. If you plan on taking it a step further, I recommend reading Modern C++ Design. The book has a dedicated chapter on Smart Pointers. It is very detailed and well written and I think it is still one of the best references out there on SPs, even though it was released before C++11.


C++ implementation of merge sort I am studying the merge sort algorithm and implemented it. In the mergeSort function, I am allocating two new vectors. Can I do it without allocating extra memory? Are there any other suggestions?

#ifndef MERGE_SORT
#define MERGE_SORT
#include <vector>

template<typename T>
void merge(std::vector<T>& array, std::vector<T> left, std::vector<T> right) {
    array.clear();
    auto lsize = left.size();
    auto rsize = right.size();
    unsigned int i=0, j=0;
    while (i < lsize && j < rsize) {
        if (left[i] < right[j]) {
            array.push_back(left[i]);
            ++i;
        }else {
            array.push_back(right[j]);
            ++j;
        }
    }
    for(; i<lsize; ++i)
        array.push_back(left[i]);
    for(; j<rsize; ++j)
        array.push_back(right[j]);
}

template<typename T>
void mergeSort(std::vector<T>& seq) {
    auto n = seq.size();
    if (n > 1) {
        auto start = seq.begin();
        auto end = seq.end();
        auto middle = start + n / 2;
        std::vector<T> left(start, middle);
        std::vector<T> right(middle, end);
        mergeSort(left);
        mergeSort(right);
        merge(seq, left, right);
    }
}

#endif // MERGE_SORT

Can I do it without allocating extra memory?

In fact, yes.  The trick is to merge recursively (technically, the \$O(\log N)\$ extra memory is still necessary for recursion). Here is a pseudo-code for the classic in-place merge:

// Prerequisite: [begin, mid) and [mid, end) ranges are sorted
inplace_merge(begin, mid, end)
{
    if (end - begin) < 2) return;

    left = begin + (mid - begin)/2;
    right = lower_bound(mid, end, *left);

    mid = rotate(left, mid, right);

    inplace_merge(begin, left, mid);
    inplace_merge(mid, right, end);
}

Stability

The most important feature of merge sort is stability: the elements compared equal retain their original order (not very important for integers, but immensely important when sorting structured data). Your implementation loses it:

if (left[i] < right[j]) {
    array.push_back(left[i]);
    ++i;
}else {
    array.push_back(right[j]);
    ++j;
}

In case of equality it is an element from right to be pushed first. Easy to fix with

if (right[j] < left[i])

or

if (left[i] <= right[j])

First way is preferred, because it doesn't assume existence of operator<=.


Entity Framework Core DbFunction for square root I'm porting my site over from .NET Framework / MVC 5 / EF6 to .NET Core 3.1 MVC / EF Core. MSSQL Server for the database.
EF6 has SqlFunctions of which one method is SquareRoot, which translates to using SQRT in sql queries against MSSQL Server.
EFCore does not have the SqlFunctions class. It does have an equivalent DbFunctions class but this is missing lots of methods.
After some googling about I've come up with the following for translating the LINQ query to sql:
MyDbFunctions.cs
public static class MyDbFunctions
{
    public static double? SquareRoot(double? arg) => throw new Exception();
}

OnModelCreating method in DbContext
var sqrtMethodInfo = typeof(MyDbFunctions).GetMethod(nameof(MyDbFunctions.SquareRoot));
modelBuilder
    .HasDbFunction(sqrtMethodInfo)
    .HasTranslation(args => SqlFunctionExpression.Create("SQRT", args, typeof(double?), null));

Usage:
from a in context.Posts
let sqrt = MyDbFunctions.SquareRoot(a.someColumnWithDoubleValue)
where sqrt < 1337
select new MyViewModel
{
   Sqrt = sqrt
    ...
};

This seems to work nicely. I can see the use of SQRT within the generated query (using SQL Server Profiler) and the where clause being applied. It also looks near identical to what EF6 generates.
I'm wondering if any EF Core pros can see anything wrong/pitfalls with the implementation (or improve on it)?

Here I capture the main essence of the comments.
Facts

As the OP has mentioned the Sqrt functionality is no exposed on the EF.Functions in EFCore

EF 6 exposed it through SqlFunctions

Math.XYZ functions can be used in Linq2Entities

Not all of the functions have translator to the corresponding SQL function, current state
As a part of this issue more function translators will in v5

Solutions

Math.Pow can be used as well, like this: Math.Pow(a.someColumnWithDoubleValue, 0.5)

This will be translated into POWER

Math.Sqrt can be used as well, like this: Math.Pow(a.someColumnWithDoubleValue)

This will be translated into SQRT
If the data type would be decimal then explicit cast is needed:

Math.Pow((double)a.someColumnWithDecimalValue)
Translated into SQRT(CONVERT(Float, ...)


Safety of globalizing config data I have some framework in PHP/MySQL for developing web services. Finally my code works on two servers - production and developer. My idea is to create one code without differences, which will work different on both servers, depend of config. I created table [ini], with two rows of data, where I can specifically change settings of many parameters like:

email address of administrator, which is used to inform him about registration new user,
IDs of advertising banners to display on some pages,
page name to redirect after successful logging,
ID of default role of new user
etc.

I can of course prepare a class for read one specify setting when it is necessary, but it means that I will read data from my [ini] table dozen times in many PHP files.

Is it safe to read it once and globalize by following code?

//on developer machine my INI_ID == 0, on production server INI_ID == 1
if($_SERVER['SERVER_NAME'] == 'www.developer_domain_name.com'){
 define('INI_ID',0);
}
else{
 define('INI_ID',1);
}

$row = function_SQLToRow('SELECT * FROM ini WHERE ini_id = ' . INI_ID . '');
$GLOBALS['ini'] = $row;

I'm open for other solutions, maybe storing config in file and read specific file depend on server, or prepare list of constants depend on server are safer solutions?

Yes, it is safe. No, it is not a good idea.

Don't mix application logic and configuration with the database, the database is used for permanent storage of data.

Use a file (json, ini, xml, whatever) which is named the same but has different contents in production/dev environments. Then, it's as simple as:

$config = json_decode(file_get_contents('config.json')); //I'm assuming JSON here

Also, please don't globalize configuration. Pass whichever config option you need (just that config option, not the whole configuration array) into whatever function/object that needs it.


Calculate the rates at which users get stuck at each stage I completed an algorithm problem just as a personal study, and I can't help but feel like my solution is a bit needlessly complicated, although I can't figure out a better way to do it. Here's the problem :

  Given a number N, and a list of users currently on a stage, find out the failure rate of each stage up to N, and list the stages in descending order of failure rate. The failure rate is the percentage of users currently stuck on a stage, out of all the users who are currently playing the current stage or higher.

Example :
        stages = [2, 1, 2, 6, 2, 4, 3, 3], N = 5

There is 1 user currently playing stage 1, therefore out of the list of 8 users playing stage 1 or higher the failure rate of stage 1 is 1/8.
There are 3 users currently playing stage 2, therefore out of 7 users playing stage 2 or higher the failure rate is 3/7.
There are 2 users currently playing stage 3, therefore out of 4 users playing stage 3 or higher the failure rate is 2/4.
There is 1 user currently playing stage 4, therefore out of 2 users playing stage 4 or higher the failure rate is 1/2.
There are 0 users currently playing stage 5, therefore out of 1 users playing stage 5 or higher the failure rate is 0/1.

Therefore the result is : [3, 4, 2, 1, 5]

My solution :

public class StageFailureRate {
    private ArrayList<Integer> result;

    public StageFailureRate(int N, Integer[] stages) {
        result = new ArrayList<>();
        Map<Integer, Integer> usersOnStage = new HashMap<>();

        //count how many users are on each stage
        for (int stage : stages) {
            if (!usersOnStage.containsKey(stage)) {
                usersOnStage.put(stage, 1);
            } else {
                usersOnStage.put(stage, usersOnStage.get(stage) + 1);
            }
        }

        //compute the failure rate for each stage, up to N stages
        int total = stages.length;
        Map<Integer, Float> failureRates = new HashMap<Integer, Float>();
        float failureRate = 0;
        for (int i = 1; i <= N; ++i) {
            if (usersOnStage.containsKey(i)) {
                failureRate = (float) usersOnStage.get(i) / (float) total;
                total = total - usersOnStage.get(i);
            } else {
                failureRate = 0 / total;
            }
            failureRates.put(i, failureRate);
        }

        //Sort into descending order and get the result.
        Map<Integer, Float> sortedMap = sortByValue(failureRates);

        for (Map.Entry<Integer, Float> entry : sortedMap.entrySet()) {
            result.add(entry.getKey());
        }
    }

    public ArrayList<Integer> getResult() {
        return result;
    }

    private static Map<Integer, Float> sortByValue(Map<Integer, Float> unsortedMap) {
        List<Map.Entry<Integer, Float>> list =
                new LinkedList<Map.Entry<Integer, Float>>(unsortedMap.entrySet());

        Collections.sort(list, new Comparator<Map.Entry<Integer, Float>>() {
            public int compare(Map.Entry<Integer, Float> o1,
                               Map.Entry<Integer, Float> o2) {
                return (o2.getValue()).compareTo(o1.getValue());
            }
        });

        Map<Integer, Float> sortedMap = new LinkedHashMap<Integer, Float>();
        for (Map.Entry<Integer, Float> entry : list) {
            sortedMap.put(entry.getKey(), entry.getValue());
       }
        return sortedMap;
    }
}

This solution is correct. Do you think it's the most optimal solution or could this be optimized to be much faster?

you should define float failureRate = 0; inside the for (int i = 1; i <= N; ++i) loop, because you are not using it outside the loop. See this for more info.
When you calculate failureRate, just cast one of usersOnStage.get(i) or total to float. You don't need to cast both.
else {failureRate = 0 / total;} is redundant because you'll always set failureRate to 0, which is its default value.

As for optimization to make it faster, since this piece of code is likely to be used in a video game, stages should be a small list, where efficiency of the algorithm doesn't matter much. You'd probably want to use simpler data structures, for a smaller overhead.


Concisely handling both arrays and Backbone.Collection as parameter I've got a Backbone.Collection which looks like this:

define([
    'background/mixin/collectionMultiSelect',
    'background/model/searchResult'
], function (CollectionMultiSelect, SearchResult) {
    'use strict';

    var SearchResults = Backbone.Collection.extend({
        model: SearchResult,

        mixins: [CollectionMultiSelect],

        //  Reset the collection's values by mapping a single or list of Song objects into
        //  JSON objects which will be instantiated by Backbone.
        setFromSongs: function (songs) {
            var searchResults = [];

            if (songs instanceof Backbone.Collection) {
                searchResults = songs.map(function (song) {
                    return {
                        song: song,
                        title: song.get('title')
                    };
                });
            } else if (_.isArray(songs)) {
                searchResults = _.map(songs, function (song) {
                    return {
                        song: song,
                        title: song.get('title')
                    };
                });
            } else {
                searchResults.push({
                    song: songs,
                    title: songs.get('title')
                });
            }

            this.reset(searchResults);
        },

        //  Returns the underlying Songs of the selected SearchResults.
        getSelectedSongs: function() {
            return _.map(this.selected(), function (searchResult) {
                return searchResult.get('song');
            });
        }
    });

    return SearchResults;
});

I feel like I'm being overly verbose with my setFromSongs method. The intent of the method is to allow outside objects to dumbly give a single Song object, an array of Song objects, or a Backbone.Collection of Songs to the method and have it handle them appropriately.

I thought that this made sense for a few reasons:

Backbone already anticipates singular versus multiple objects with its add and reset methods. I would like my methods to do so, as well.
When working with a Collection it often becomes an array through use of underscore utility methods such as map, pluck, etc.

Is this as concise as this method can be while still making its intent clear? How would you approach this?

setFromSongs initialises the searchResults variable and then in 2-of-3 cases it immediately overwrites it. You can rewrite the singular case to create the array containing only the single object then you don't need to initialise the variable immediately.

You are repeating the code { song: x, title: x.get('title') } in three different places - you can make the code more DRY if you create a function defined outside the class to handle the mapping of a song to its association with its title:

var associateSongWithTitle = function( song ){
  return {
    song:  song,
    title: song.get('title')
  };
};

You can then refactor the code to:

var SearchResults = Backbone.Collection.extend({
  ...
  setFromSongs: function (songs) {
      var searchResults;
      if (songs instanceof Backbone.Collection) {
          searchResults = songs.map( associateSongWithTitle );
      } else if (_.isArray(songs)) {
          searchResults = _.map(songs, associateSongWithTitle );
      } else {
          searchResults = [ associateSongWithTitle( songs ) ];
      }
      this.reset(searchResults);
  },
  ...
});


Find all permutations of a string in C++ I need a review of the following code for finding all permutations of a string in C++:

List Permute(const String& string)
{
    if (string.length == 0)
        return EmptyStrings(1);
    List prevList = Permute(SubString(string,1,string.length));
    List nextList = EmptyStrings(string.length*prevList.length);
    for (int i=0; i<prevList.length; i++)
    {
        for (int j=0; j<string.length; j++)
        {
            nextList[i*string.length+j] += SubString(prevList[i],0,j);
            nextList[i*string.length+j] += string[0];
            nextList[i*string.length+j] += SubString(prevList[i],j,string.length-1);
        }
    }
    return nextList;
}

You may assume the correctness of the following:

class List
class String
List EmptyStrings(int n); // returns a list of 'n' empty strings
String SubString(const String& string,int i,int j); // returns the string 'string[i...j)'

You may need to remove duplicates. Consider what your output will be if, for example, your input string is "aaa".

Code like this is unusual:

// nextList contains an array of empty strings
nextList[i*string.length+j] += SubString(prevList[i],0,j);

IMO this would be more usual:

// nextList is empty
// calculate new string
string nextString = SubString(prevList[i],0,j)
    + string[0]
    + SubString(prevList[i],j,string.length-1);
nextList.Add(nextString);

Returning by value is sometimes necessary but can be inefficient. Your algorithm creates many temporary List objects. A better API might be ...

void Permute(const String& inputString, List& outputList) { ... }

... where one empty List is allocated by the caller, and you add to it.

This statement may be incorrect ...

List prevList = Permute(SubString(string,1,string.length));

... I don't know how returns the string 'string[i...j)' is defined but perhaps this statement should be ...

List prevList = Permute(SubString(string,1,string.length - 1));

You may run out of memory very quickly. For a string of length n there are n! ('n factorial') permutations. "what's up doc?" has 14 characters => 87178291200 (or 0x144C3B2800) i.e. bigger than 232 combinations.


Is there a better way to insert an enum into a set without macros? I created an enum for a class and in the constructor I inserted all the enum values into a set.  I am wondering if there is a better way anyone can recommend.  I feel like there should be, but have been unable to think of one or find one online.  I wrote the code in C++ and am using the Boost libraries.

Here is the enum:

class CreateAndUseIniFile {

    std::set<CreateAndUseIniFile::iniFileValues> m_modules;
    enum iniFileValues {
        FIXED_VOLTAGE1,
        FIXED_VOLTAGE2,
        FIXED_VOLTAGE3,
        FIXED_VOLTAGE4
    }
}

and this is the constructor where I add the values

CreateAndUseIniFile::CreateAndUseIniFile() {
    m_modules.insert(CreateAndUseIniFile::FIXED_VOLTAGE1);
    m_modules.insert(CreateAndUseIniFile::FIXED_VOLTAGE2);
    m_modules.insert(CreateAndUseIniFile::FIXED_VOLTAGE3);
    m_modules.insert(CreateAndUseIniFile::FIXED_VOLTAGE4);
}

You can iterate over the enum and insert each one individually.

for ( int i = FIXED_VOLTAGE1; i != FIXED_VOLTAGE4; i++ )
{
    m_modules.insert(CreateAndUseIniFile::static_cast<iniFileValues>(i));
}

Note: This will work with your specific case... don't do this if you have enums with set values that contain gaps between enum values.