Parsing MWNumericArray to C# code optimization

As usual with performance problems the only way of knowing for sure is to use a profiler and check which piece of code is taking the longest time. However looking at the code you are copying the matrix element by element which is typically the slowest way to go. Often there is a method which can do that as a bulk operation for you.

I don't know much about the Matlab stuff as I've never used it but I found this question on Stackoverflow which has a couple of answers showing a single line statement of copying it. Based on one of these answers your code for the 2D case could look like this:

double[,] data = (double[,])matrix.ToArray(MWArrayComponent.Real);

I assume in the 3D case you'll get a double[,,] back which you might have to take apart afterwards so it fits your List<double[]> type.

Apart from that:

This exception has a misleading message in case the dimensions are less than 2

if (dimensions.Length > 3 || dimensions.Length < 2)
{
    throw new ArgumentException("Input matrix dimensions are too long", "Three is the max dimensions");
}


I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file.  That's a violation of the Single Responsibility Principle.  What if I want to perform multiple color substitutions before writing out the result?  What if I want a different output filename, or a format other than BMP?

Avoid empty declarations, when you can declare and initialize values at the same time.  It's more readable and less error-prone.

You don't care about the x-y coordinates of each pixel — only the color matters.  So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}


Edit: Only after posting did I see that the question was tagged c++14. This code relies on std::void_t from c++17 which is trivial to implement in c++14.

You can extract some of the functionality to a reusable detect type-trait that only needs to be supplied a type alias to work.

namespace detail {

template <template <class...> class CheckTemplate, class = void, class... Object>
struct detect_impl : std::false_type {};

template <template <class...> class CheckTemplate, class... Object>
struct detect_impl<CheckTemplate, std::void_t<CheckTemplate<Object...>>, Object...> : std::true_type {};
}  // namespace detail

template <template <class...> class CheckTemplate, class... Object>
constexpr bool detect_v = detail::detect_impl<CheckTemplate, void, Object...>{};

With that in place all we need is to use it as such.

template <class T>
using is_valid = decltype(std::declval<T>().method1(0, 0, 0));

template <typename T>
constexpr bool is_valid_v = detect_v<is_valid, T>;

static_assert(is_valid_v<ValidType<int>>)

The Object pack let's us make a more flexible is_valid that takes the argument types as parameters if we want.

template <class T, class... Args>
using is_valid_flex = decltype(std::declval<T>().method1(std::declval<Args>()...));

template <class... Args>
constexpr bool is_valid_flex_v = detect_v<is_valid, Args...>;

static_assert(is_valid_flex_v<ValidType<int>, int, int int>);

Additional modification is possible if we want to involve the return-type of the method.


Brainf**k to Ruby converter, written in Ruby -- v1.0

You should open the input file before the output file, so that if the input file is unreadable, no file is created at all.  File opening should be done using blocks for automatic closure and cleanup.

This is too complicated:

replacements.each do |data|
  if data[:replacing] == char
    output.puts(data[:replacement])
  end
end

replacements would be better as a Hash:

replacements = {
    '+' => 'data[pointer] += 1',
    '-' => 'data[pointer] -= 1',
    '>' => 'pointer += 1',
    '<' => 'pointer -= 1',
    '.' => 'putc data[pointer]',
    ',' => 'data[pointer] = $stdin.readbyte',
    '[' => 'until data[pointer] == 0',
    ']' => 'end',
}

open(input_file, File::RDONLY) do |input|
  open(output_file, File::CREAT | File::WRONLY) do |output|
    output.puts(start)
    output.puts(input.each_char.map { |c| replacements[c] }.compact.join("\n"))
  end
end

One big puts should be faster than many indivual puts.


Well, in principle having non-nullable pointers, smart or not, is a good thing. It's just a shame that nullability is not an optional non-default trait (not only for pointers, but for all types; for numbers non-zeroability might be an interesting companion-trait, which incidentally makes the former more efficient).

Your implementation of it leaves something to be desired though, because you completely block conversion from "normal" nullable pointers to your non-nullable one and back. That severely limits your types utility.
Also, one expects a lot more functionality from a pointer-like type, like more conversions, more operators, and the rest of the smart-pointer-interface, at least as far as compatible with the design-goals.

Now let's look at the code of your prototype:

Separate your new smart-pointer from the test-code, and make it a self-contained header-only-library.
Never use using namespace std; in an include-file, not that it is much better in a source-file. Read "Why is “using namespace std;” considered bad practice?" for the reasons.
As said above, you need loads more accessors, and all the other functons, member and free, which comprise the interface for a smart-pointer.
If you use private inheritance, you have an easier time passing through the needed interface with using than writing it from scratch.

And the code of your test:

Only explicitly flush a stream if you really need to. Doing so frivolously is quote costly.
If you want to output a single character to a stream, consider doing so explicitly instead of with a length-1 C-string. It's potentially slightly more efficient.


Handling request errors in snap with haskell

What's "bad" about the request?

Giving a generic failure response isn't terribly useful to the poor soul who has to deal with handling the errors when interfacing with your API.  Why did it fail?  You're only catching the case where the response either is or isn't valid JSON, but what about other kinds of failure?

One thing you might want to look at is Digestive Functors, if you haven't already.  You can use it to do some of your error handling (eg. the value for FieldB is out of the specified range, etc.).  There's a Digestive Functors Aeson library that will let you send your JSON result through the form.  I don't do much with passing around JSON data myself, so this might be a bit more than you need.

The database can fail, too

You've only handled the case where decoding your JSON request can fail, not the database portion.  When it comes to the database, there are 4 basic outcomes to a query:

Success: Results
Success: No results
Failure: Constraint Violations (errors caused by the user)
Failure: SQL errors (errors caused by the developer)

So for queries that can fail (any time you run an insert/update/delete query), you'll want to make sure you're catching the Constraint Violations.  PostgreSQL Simple has a convenient function in Database.PostgreSQL.Simple.Errors (catchViolation) specifically for that (though if you're using the PostgreSQL Simple snaplet rather than using PostgreSQL Simple directly, you'll have to roll your own because the type signatures are incompatible).

Snap Extras might be right for you

There's another package called snap-extras that contains useful functions like reading the response as JSON or responding with JSON.  I know some people aren't too fond of it since it has a lot of dependencies, but I tend to use some of the other modules in it, so it's convenient for me.

If not, you can easily copy/paste just the parts you need into your own project.


Python lets you set default values for parameters in the method signature in the way you have done above with minLength=''. This means that if the caller of the function doesn't provide a value for that parameter the default of an empty string will be used instead. Essentially doing the job of your if minLength == '': minLength = 4 line. Here is what you might write instead:

def restriction_place(string, min_length=4, max_length=12):
    pass

I also took the liberty of using snake_case as that's the convention in Python for variable and function names (in most cases).

There is a caveat to default values when using mutables, however. see: http://docs.python-guide.org/en/latest/writing/gotchas/


Displaying a letter grade based on a percentage

It would be good to split the different sub-tasks to different functions,
for example:

int promptScore(): a method to prompt for score
String getGrade(int score): a method to return the grade from the score

The promptScore method could use an infinite loop with two steps:

Ask for a number
If the number is within the range, return it

The getGrade method could use a sorted map that stores the limits and the corresponding grades.
You could iterate over the entries in this map,
and when you find a value that is below the limit, then you return the corresponding grade.

Something like this:

class Grades {

    private static final Map<Integer, String> limits;

    static {
        limits = new TreeMap<>();
        limits.put(50, "B");
        limits.put(90, "B+");
        limits.put(92, "A-");
        limits.put(96, "A");
        limits.put(101, "A+");
    }

    public static int promptScore() {
        Scanner scanner = new Scanner(System.in);

        while (true) {
            System.out.print("What is the percentage of the student: ");
            int x = scanner.nextInt();

            if (0 <= x && x <= 100) {
                return x;
            }
            System.out.println("Please put in a percent from 0 - 100!\n");
        }
    }

    public static String getGrade(int score) {
        for (Map.Entry<Integer, String> entry : limits.entrySet()) {
            int limit = entry.getKey();
            if (score < limit) {
                return entry.getValue();
            }
        }
        throw new AssertionError("Impossible case");
    }

    public static void main(String[] args) {
        System.out.println(getGrade(promptScore()));
    }
}

The benefit of splitting up the functions like this is that writing unit tests becomes easy:

@Test
public void testB() {
    assertEquals("B", Grades.getGrade(0));
    assertEquals("B", Grades.getGrade(49));
}

@Test
public void testB_Plus() {
    assertEquals("B+", Grades.getGrade(50));
    assertEquals("B+", Grades.getGrade(89));
}

@Test
public void testA_Minus() {
    assertEquals("A-", Grades.getGrade(90));
    assertEquals("A-", Grades.getGrade(91));
}

@Test
public void testA() {
    assertEquals("A", Grades.getGrade(92));
    assertEquals("A", Grades.getGrade(95));
}

@Test
public void testA_Plus() {
    assertEquals("A+", Grades.getGrade(96));
    assertEquals("A+", Grades.getGrade(100));
}


The definition of the winningMap is very confusing.

I suggest a different form of input:

var winningMap = {Rock: "Scissors", Paper: "Rock", Scissors: "Paper"};

It defines which choice can be beaten by a specific selection.
If both inputs are the same, it's obvious a tie:

var getWinner = function (playerChoice, computerChoice) {
    if(playerChoice === computerChoice){
        return "Tie";
    }
    if(computerChoice === winningMap[playerChoice]){
        return "Player Wins";
    }
    return "Computer Wins";
};

Notice, that I didn't use the string-definitions. I don't think that it's necessary, and it only makes the code a bit more complicated.

In order to convert the computer's choice to an actual string (Rock, Scissors, Paper), you can use an Array:

var stringMapping = ["Rock", "Scissors", "Paper"];

var getComputerChoice = function () {
    return stringMapping[ Math.floor(Math.random() * 3) ];
};

Maybe a new-Operator would be understood easier

The closure might be confusing for a beginner, maybe you should consider to write the Object this way:

function RockPaperScissors (){
     var getComputerChoice = function(){ ... };    //= private variable
     this.getWinner = function(){ ... };           //= public variable
};

var actualObject = new RockPaperScissors();
actualObject.method();
actualObject['method']();

If your colleagues are used to object oriented programming, this style might be more familiar.

Always use brackets

if ($input.val() === '-1') { return; }

I personally don't mind conditions within one line, but dropping the brackets is bad practice - and can lead to strange behaviours, especially in JavaScript.

Furthermore, I wonder why u used the dollar-sign at the beginning of the $input-variable?

Edit:

I share @Barts's opinion from the comments - it's not neccessary to use jQuery in this example. You should show them jQuery once they know the basic principles of JavaScript.


Create a struct that encapsulates all the details of a stack
The problem is that your stack just looks like a pointer to an int, indistinguishable from other pointers to ints. And the first element it points to is treated differently from the other elements. In this case, it is better to create a struct that keeps track of the allocated memory and the size of it, like so:
struct Stack {
    size_t size;
    int *data;
};

You initialize it as follows:
struct Stack stack = {0, NULL};

Now you should change push() and pop() to take a pointer to a struct stack:
void push(struct Stack *stack, int val) {
    stack->size++;
    int *new_data = realloc(stack->data, stack->size * sizeof *stack->data);

    if (!new_data) {
        // error handling here, or just
        abort();
    }

    stack->data = stack->new_data;
    stack->data[stack->size - 1] = val;
}

And similar for pop(). Note that it is common to have functions that operate on an object take the pointer to that object as the first parameter. Also, I made the function return void, there is no need to return the size of the stack size that information is already available to the caller.
Avoid using global variables
You should avoid using global variables if possible. My example code above no longer requires there to be a global stack. This change allows the code to manage multiple stacks without conflicts.
Add functions to create and destroy stacks
Instead of requiring the caller to know how to properly initialize a struct Stack and to free it after use, create functions that do this for you. That allows you to change the internals of struct Stack later, without having to change all the places where a stack is used.
Use a common prefix to avoid name conflicts
push() and pop() are very generic names. There are many more things that can have push and pop operations, such as FIFO queues. I recommend you use a common prefix for all data structures and functions for your stack. This can simply be Stack or stack if you think that is unlikely to conflict with anything else.


Generating a table of contents with collapsible subsections, PhP or JQuery?

I find your php script incredibly hard to read.  Maintaining this script would surely involve substantial time investment.

You are conducting far too much preg_ surgery on input that can be prepared to be valid html.  Processing html is far more reliable via a proper DOM parsing tool.  My preferred tool is DOMDocument and I often pair it with XPath when it makes node targeting clearer/simpler.

I don't have the time to completely rewrite your script, but that is precisely what I recommend.

You shouldn't ever need to perform "mopping up" of your generated html by throwing some more </div> in at the end.


DATE *create_date(unsigned char month, unsigned char day, unsigned short year)

What if I want to create a DATE on the stack?

    if((month > 12) || (month < 1))
        return NULL;
    if((day > 31) || (day < 1))
        return NULL;
    if((year > 9999) || (year < 0))
        return NULL;

There are many date libraries out there, and they're not consistent on their offsets for months. I would consider creating an enum for the months.

Also, either don't validate the values at all or validate them fully. If I can't create 2017-13-01 then why can I create 2017-02-31? If I can't create a date in 753 BC then why can I create a date in year 0 (whatever that means – 1 BC?)?

Obviously that comes with some caveats, particularly with respect to the transition from the Julian to the Gregorian calendar; the documentation should make it clear what the transition date is and what days don't exist in consequence.

void printf_date(DATE *input, DFORMAT format)

Wouldn't sprintf_date be more useful? You can print to stdout given sprintf, but you can't print to anything other than stdout given only printf.

        printf("%hhu-%hhu-%hu\n",input->month,input->day,input->year);

        printf("%hhu/%hhu/%hu\n",input->month,input->day,input->year);

No. Just no. ISO 8601 is the only date format that should be used unless a locale parameter is supported.


Use constructors to initialize your objects.

        root = new Node;
        root->val = n;
        root->left = nullptr;
        root->right = nullptr;

This should be replaced by:

        root = new Node(n);

Design

I think you will find that your code becomes a lot simpler if you move your test for null out of the public members. Then make the test for null the first thing you do in the private member (Don't test before you call the private member test as the first action in the call). This will simplify all the situations were you currently have to test for null.

For Example:
I would write addValue like this:

void BTree::addValue(int n) {
    root = addValue(n, root);
}
Node* BTree::addValue(int n, Node* current) {
    if (current == nullptr) {
        return new Node(n);
    }
    if (n < current->val) {
        current->left  = addValue(n, current->left);
    else if (n > current->val) {
        current->right = addValue(n, current->right);
    }
    return current;
}


I would remove the:

if( size > 0 )

and replace the

do {...} while

with a

while {...}

to simplify the structure of the code.

You can also redesign your algorithm to simplify it further. You can use an Array instead of ArrayList and use the following shuffle algorithm which avoids the use of a supplementary array. Here is the complete code:

import java.util.Random;

public class ShuffleGenerator {
    private int indices[];
    private int nextIndex;

    ShuffleGenerator(int size) {
        indices = new int[size];
        nextIndex = 0;
        for( int i = 0; i < size; i++ ){
            indices[i]=i;
        }
    }

    public void shuffle(Random rnd){
        for( int i = 0; i < indices.length; i++){
            // place a random element j>=i into position i
            int j = i + rnd.nextInt(indices.length-i);
            // swap i and j
            int c = indices[i];
            indices[i] = indices[j];
            indices[j] = c;
        }
    }

    /**
     * Get the next index.
     * @return -1 if at the end of the sequence.
     */
    public int getNext(){
        if( nextIndex >= indices.length){
            return -1;
        }
        return indices[nextIndex++];
    }

    /**
     * Get the previous index.
     * @return -1 if at the start of the sequence.
     */
    public int getPrevious(){
        if( nextIndex <= 0){
            return -1;
        }
        return indices[--nextIndex];
    }
}

BTW I think it makes more sense to pass a Random object instead of a seed. How do you choose the seed to pass to the function?

added thanks to @Pimgd I noticed that there was a problem in your code: the first call to getNext() should return the first element of the sequence not the second...

Actually the interface getNext,getPrevious is not very good in my opinion (image how you would use it). I think it would be best to have getCurrent, next, prev where next and prev return void. Or you could simply define length and at methods and remove the cursor from the class.


This is nowhere near the full answer, but I just want to touch on the initialize method of Merge real quick since it was the first thing to jump out to me.

def initialize(destination_id, *source_ids)
  @source_ids = source_ids
  @destination_id = destination_id
  @sources = Modelname.where(id: @source_ids).all
  @destination = Modelname.where(id: @destination_id).first
end

This code is functionally equivalent. Using the splat operator at the end I guarantee the source ids will be in an array.  I'll keep looking at the rest of the code and go from there, but that was the first thing I wanted to point out.


Retrieving a list, by iterating through a list

You're looking for the LINQ-equivalent of an IN clause in SQL. So something like this:

SELECT value FROM tblEquipment_Attributes
WHERE Category_Attribute_Identifier IN (<list of integers>)
AND Unique_Item_ID = itemID;

So what you could do is write your LINQ statement to see if the Category_Attribute_Identifier is in the list. Then your function will look something like this:

Function getValues(catAttIDs As List(Of Integer), itemID As Integer) As List(Of String)
    Dim db As New Model1

    Dim currentValues as List(Of String) = (From row In db.tblEquipment_Attributes
                     Where catAttIDs.Contains(row.Category_Attribute_Identifier) _
                     And row.Unique_Item_ID = itemID
                     Select row.Value).ToList()

    Return currentValues
End Function

Note that ToList will create a List<T>, where T is the type of the elements. As long as Value in your db is a varchar, it'll be a List.


Modern HTTP-Requests for login/register system

Declaring a class makes sense if it can be reused, but less so if the logic and element ids are hardcoded to just one case, it only adds the unnecessary this. to variables.
It's unclear why the click listener is handled outside of the form class as it's logically and conceptually a part of the form.
JS naming convention is camelCase, not snake_case.
Consider URLSearchParams API so the HTTP headers are set automatically and values are properly escaped.
Consider making the login routes more declarative e.g. routes in the code below.
If your goal was to use a class as a namespace then it might be better to switch to modules. For example using the built-in ES modules the inner contents of the function will be the entire contents of the module.

function handleLogin() {
  const request = new XMLHttpRequest();
  const loader = document.getElementById('loader');
  const btn = document.getElementById('right');
  let step = 0;
  const routes = [{
    path: 'one',
    field: 'username',
    onload() {
      step++;
      getDataElement().focus();
    },
    onerror(e) { /* show error */ },
  }, {
    path: 'two',
    field: 'password',
    onload() { /* show home page */ },
    onerror(e) { /* show error */ },
  }];

  btn.addEventListener('click', () => {
    loader.style.opacity = 1;
    const {path, field} = routes[step];
    request.open('post', `/login/step_${path}`);
    request.send(new URLSearchParams({[field]: getDataElement().value}));
  });

  request.addEventListener('loadend', e => {
    loader.style.opacity = 0;
    routes[step][request.response === 'true' ? 'onload' : 'onerror'](e);
  });

  function getDataElement() {
    return document.getElementById(routes[step].field);
  }
}


IsInRole()
I don't like how the method parameters are validated. IMO the null-propagation operator shouldn't be used for everything because it reduces the readability. Having a null-propagation operator for "safely" "accessing" a property is OK if the object is the first on the right hand of an assignment like in checking for var ident = principal?.Identity; but nevertheless I personally would stick to a combined null check like so
if (principal == null || principal.Identity == null) { return false; }

But why areyou checking this at the beginning of the method ? I would check simple types first like departmentId <= 0.
The null-propagation operator here
var roleDeparments = ((ClaimsIdentity)ident).FindFirst($"{role} departments")?.Value;

could be missed at first glance making a maintainer of the code wonder what this is about, at least if he/she has not that good eyes like me. He/she can't grasp at first glance that you "safely" access the Value property here.
Can you spot the error here ?
var inInRole = roleDeparments
    .Split(',')
    .Any(d => d == departmentId.ToString() || d == "all");

Aside from spelling error in inInRole you could store the result of departmentId.ToString() in a variable to check in the Any(), but maybe the compiler is smart enough to do this for you.
Omitting braces {} for single line if statemenst is your responsibility. I wouldn't do it. If I would do it then only like if (departmentId <= 0) return false;
Using abbreviations leads to less readable code. Why don't you rename ident  to identity ?
Implementing the mentioned points leads to
public static bool IsInRole(this IPrincipal principal, string role, int departmentId)
{
    if (departmentId <= 0) { return false; }
    if (principal == null || principal.Identity == null) { return false; }

    var identity = principal.Identity;

    var claim = ((ClaimsIdentity)identity).FindFirst($"{role} departments");
    if (claim == null) { return false; }

    var roleDeparments = claim.Value;
    if (string.IsNullOrWhiteSpace(roleDeparments)) { return false; }

    var isInRole = roleDeparments
        .Split(',')
        .Any(d => d == departmentId.ToString() || d == "all");

    return isInRole;
}

CreateIdentity()
This made me wonder
bool isAll = departments.Any(d => d == null);

reading isAll and Any. After rereading your question I stumbled over

if the departmentId is set to NULL then it means this role is general to all departments

This is screaming for a comment because a maintainer of the code should know this as well. Together with a better name it would be clear for a maintainer what is happening here like so
// If one of the user.UserRoles has a DepartmentId which is null,
// the role is general for all departments
bool isGeneralForAllDepartments = departments.Any(d => d == null);

string departmentIds = isGeneralForAllDepartments
    ? "all"
    : string.Join(",", departments);


Program to find megaPrime numbers between 2 given integers

I think John Coleman did a good job explaining speeding up the testing for mega-primality, but you're still using trial division to test for primality. The Miller-Rabin test is much faster, and can be made deterministic given the size of your range.

The algorithm starts by checking if \$n\$ is 1 or even. These cases are trivial, and should return 0 unless \$n = 2\$.

Next we take n-1 and remove the largest power of 2. Put another way we want to find s and d such that \$2^sd = n - 1\$ and d is odd. For example, if \$n = 49\$, \$n - 1 = 2^43\$, so \$s = 4\$ and \$d = 3\$.

Next we pick a random number for \$a\$ such that \$1 \lt a \lt n - 1\$. Now we make a loop on \$r\$ where \$r\$ goes from 0 to \$s - 1\$ inclusively. If for all values of r:

\$a^{2^rd} \% n \ne n - 1\$ and \$a^d\%n\ne1 % n != 1\$, then \$n\$ is composite. Otherwise, \$n\$ has a large probability of being prime. If you test every \$a\$ where \$a = {2, 3, 5, 7, 11, 13, 17, 19, 23}\$ and \$n\$ comes up as a likely prime for every time, it is prime.

If you are unfamiliar with this you may want to do \$a^b \% c\$ one step at a time, but computing \$a^b\$ may take a long time, and will quite likely result in an overflow error.

Use this trick:

\$a^b \%c =\$
If \$b = 0\$ then 1
If \$b\$ is odd then \$(a^{b-1} \% c ) b \% c\$
If \$b\$ is even then \$(a^{\frac{b}{2}} \% c)^2 \% c\$

This way, the number is constantly being reduced, and as long as recursion is used, the time will scale logarithmically with the size of \$b\$.


Casting const pointer to non-const pointer when using struct iovec

I think you should keep it. const is not only meant for the compiler, but also for anybody reading your code. Even with the void* cast, you're still saying to readers "Trust me, I'm not touching your buffer!" and this is a valuable information that should not be discarded.


To add to Quill's answer, use heredoc whenever you want to use multiline strings.

$contents = <<<CON
New order!

Name: {$name}
Phone number: {$phone}
City: {$city}
CON;

$content = htmlspecialchars($contents);

EDIT

Why & how it improves your code,

Much cleaners and improves readability
When you need double quotes inside the string you don't need to escape them
You don't have to worry about which characters to use when you need a line break.

A common use of this is when you write SQL queries, HTML or Email message bodies in your application. Basically multiline strings.


There is really quite a lot wrong here.

As a very simple improvement, why aren't you using ActiveRecord's associations? Why do you constantly do this...

Weight.where(:user_id => current_user.id).each ...
# ...
@current_weight = Weight.where(:user_id => @user.id).order(:day).last

Instead of this?

current_user.weights.each ...
# ...
current_weight = @user.weights.order(:day).last

That alone would drastically improve the readability of your code.

You can also make the loop in your index action more idiomatic:

@result = current_user.weights.map do
  [weight.day, weight.kilograms]
end.to_h

You also do some weird things in WeightCalculator with instance vairables. Your methods all seem to both return a value, and assign that same value to an instance vairable that is never used. There is no reason to do this:

def current_weight
  # ...
  @current_weight = <calculation>
end

Instead, you should just return the calculation, or memoize it via ||=:

def current_weight
  # ...
  @current_weight ||= <calculation>
end

If I were working on this code, I would refactor it in the following way to take advantage of the above few points:

class WeightCalculator
  def initialize(profile, user)
     @profile = profile
     @user = user
  end

  def weights
    Weight.where(:user_id => @user.id)
    # or this, doesn't matter:
    @weights ||= @user.weights
  end

  def start_weight
    @start_weight ||= weights.order(:day).first
  end

  def current_weight
    @current_weight ||= weights.order(:day).last
  end

  def weight_loss
    current_weight.kilograms - start_weight.kilograms
  end

  def timeframe
    (current_weight.day - start_weight.day).to_i / 7
  end
end

Your index action is also pretty broken.

First, You try to access @profile to pass it to your WeightCalculator before you set it, and then later you set it to Profile.where(...), which returns an association, not a specific profile. Again, using associations, you probably just want

@profile = current_user.profile

Second, you query Weights all over the place, over and over. You constantly go through the effort of typing out Weight.where(...) instead of just doing it once and reusing the returned set of weights.  You even make multiple calculators for absolutely no reason, and inside the calculator you don't even reuse weights! Just do things once, there is no reason to continually run the same queries over and over, even if they are being cached by ActiveRecord.

calculator = WeightCalculator.new(@profile, current_user)
@weights = calculator.weights

@result = @weights.map do |weight|
  [weight.day, weight.kilograms]
end.to_h


Python: Loop through folders, convert XLSX files to CSV removing non ASCII

I'd have a couple of suggestions which you may want to follow:

Use openpyxl module to read xlsx documents, and the csv module to write.

An example might look like this:

import openpyxl
import csv

wb = openpyxl.load_workbook('test.xlsx')
sh = wb.get_active_sheet()
with open('test.csv', 'wb') as f:
    c = csv.writer(f)
    for r in sh.rows:
        # here, you can also filter non-ascii characters
        c.writerow([cell.value for cell in r])

Don't use os.sep.

From docs:

  [...] Note that knowing this is not sufficient to be able to parse or
  concatenate path names...

Instead, you could use normpath from os.path to normalize the path.

From the docs:

  Normalize a pathname by collapsing redundant separators and up-level
  references so that A//B, A/B/, A/./B and A/foo/../B all become A/B.
  This string manipulation may change the meaning of a path that
  contains symbolic links. On Windows, it converts forward slashes to
  backward slashes.

Don't use + separator to join certain paths. Instead, just use os.path.join().

As for the speed improvement, you'll need to go through all the subfolders in your parent folder and find all the .xslx files. Perhaps skipping non-xlsx files will bring a bit of speed: str.endswith('.xslx').


Smallest job first algorithm implementation

I think this can be done with far less code. Like this:

function calculateTotalClockCycles(jobs, index) {
  jobs.sort((a, b) => a - b);
  return jobs.slice(0, index).reduce((a, b) => a + b, 0);
}

alert(calculateTotalClockCycles([10, 3, 15, 8, 7], 3));

In the first line of code inside the function, the jobs are sorted, small to large. The second line is more complicated. The reduce() method is used to sum all the values of the array it applies to. That array is the wanted part (slice) of the whole sorted jobs array.

So here reduce() is used as an alternative for a 'for' loop.


Bloom filter implementation using a BitArray

You're using assert_equal to compare something with true - which is exactly what the basic assert method already does. Just like you use refute to check that something's false, assert checks that something's true.
You might want to check that \$k ≤ m\$. Doesn't really make sense otherwise. (Of course it doesn't really make sense for \$k\$ to equal \$m\$ either, as adding just 1 item would set every bit right away, but at least that's technically valid.)
In this case, I'm OK with using single-letter variables like k and m, since that appears to be conventional enough, and they're hard to give simple, yet descriptive names. However, I'd also add copious comments to explain exactly what the heck they mean. I had to go to Wikipedia to find out. (Edit: Sorry, comments already there, just not by the #initialize method, where I expected them to be.)
p fp_rate - don't leave print calls in final code (unless that code's purpose is to print, of course).
Why does #false_positive_rate round its output? Rounding is a presentation concern; the rate just is what it is, decimals and all. I'm thinking you're maybe rounding so it passes your tests, but in that case you're writing your code backwards.
Your #include? method is broken. It'll return (from the method; not just the block) at the first bit it checks - it won't check all of them. So if the first bit it checks is zero, you get false - which is correct! But if the first bit is 1, you immediate get true even though none of the other k-1 bits have been checked.
This also means that your false positive rate is at best

$$1 - (1 - \frac{1}{m})^n$$

because \$k\$ is effectively always 1 when checking. But if \$k\$ is higher, then we'd be setting more bits than we're checking, so the bit array fills up faster.
For instance, if \$k = 2\$ and \$m = 4\$ (extreme values; just for illustration), then the first item (call it \$A\$) we insert should set half of the bits in the bit array. That should (if my math is correct) give a false positive rate of ~19% after adding that one item: There's a 19% chance that another item, \$B\$, will match the same 2 bits. But if we only check 1 bit, then the false positive rate is essentially 50%, since half the bits in the array are set.

Making only only minimal changes, you could do this:

@k.times do |n|
  hash_val = Digest::MD5.hexdigest( item.to_s + n.to_s ).to_i(16)
  position = hash_val % @m
  return false if @b.get(position).zero?
end
true

Now, it'll only return if a bit is zero, otherwise it'll keep going.
But your hashing logic is flawed too, I'm afraid. There's no guarantee that it'll return \$k\$ unique bit positions (for \$k > 1\$). It's possible (though unlikely) that you'll calculate \$k\$ MD5 hashes that are all cleanly divisible by (i.e. are multiples of) \$m\$. In that case, you'd just get the bit position zero \$k\$ number of times and end up setting a single bit. It's a similar situation to the one above, since again, the false positive rate rises.
If, for example, \$k = 2\$ and \$m = 4\$ again, and we've added item \$A\$ which (correctly) set 2 bits, we should have that 19% false positive rate when checking \$B\$. But if the hash for \$B\$ works out so that it's just 1 bit instead of 2, then we're back to a 50% false positive rate since it's only that 1 bit that would have to match, rather than 2 separate bits.

Frankly, I don't know enough about creating robust hashing functions to offer a good solution to this. I'll continue and I'll provide alternative code for different things, but it all includes this basic flaw (except in the addendum at the end). The code I provide here won't fix that; it's there to illustrate other aspects of the code.
You've duplicated the logic for hashing items. Duplication is always bad, but this is a prime example of why: If the two pieces of code aren't exactly identical, the whole thing stops functioning. I'd extract the hashing logic into a separate, private method that returns an array of bit positions

 def item_bit_positions item
   @k.times.map do |n|
     hash = Digest::MD5.hexdigest("#{item}#{n}").to_i(16)
     hash % @m
   end
 end

You'll note that I'm using string interpolation; IMO it's clearer than manually calling to_s and concatenating. The results the same, though, but that result may not be what you want (see below)
Using MD5 and then discarding a bunch of stuff really feels like overkill. Ruby already maintains a hash for anything that inherits from Object (i.e. everything). It's what's used to check for object equality, and it's hexadecimal number you see printed if you print an object that doesn't have a #to_s method.
#hash is just a (signed) integer. You could do something like:

 def item_bit_positions item
   @k.times.map { |n| item.hash % (@m-n) }
 end

(Again, this is just as flawed [edit: Actually, it's more flawed, see below] as your current implementation (it can return fewer than \$k\$ positions); it's just here to illustrate how #hash can be used.)

The above does not require digest. It does however require that \$k ≤ m - 1\$, or you'll get a divide-by-zero error.

Note, though, that unlike MD5 which will always produce the same output for a given input, #hash is simply a number generated at runtime, so it'll be different each time you run the code. This could make testing tricky, since you're also dealing with something that can produce false positives. (One solution to this might be to stub out #hash on the items you're inserting.)

Using #hash also changes the behavior somewhat since it's now about object-wise equality - not string-wise equality. In your implementation, the string "foo" and the symbol :foo would be treated as one and the same, since both have the same string representation, and you're converting everything to a string to calculate the MD5 hash. But using #hash there's no such collision.
You're using your BitArray class, but I'd be tempted to just re-roll some of the same logic here, since it doesn't take much, and the implementation can be more specific. You can do everything with integers and bitwise manipulation.

class BloomFilter
  # This reader is mostly here for test purposes,
  # which isn't too pretty, I'm afraid. At least
  # it's a pretty harmless addition
  attr_reader :bit_array

  # TODO: Explain what k and m mean!
  def initialize(k = 1, m = 10)
    k, m = k.to_i, m.to_i
    raise RangeError, "k must be smaller than m" if k >= m
    raise ArgumentError, "k and m must be positive integers" if k < 1
    @k = k
    @m = m
    @bit_array = 0
  end

  def insert(item)
    @bit_array |= digest(item)
    self
  end

  def include?(item)
    hash = digest(item)
    @bit_array & hash == hash
  end
  alias_method :member?, :include?

  private

  # WARNING: This method is flawed! See answer
  def digest(item)
    hash = item.hash
    @k.times.map do |n|
      bit = (hash % (@m-n))
      2**bit
    end.inject(&:|)
  end
end

gem 'minitest'
require 'minitest/autorun'

class BloomFilterTest < Minitest::Test
  def test_initialize_with_valid_args
    assert BloomFilter.new(1, 2)
  end

  def test_initialize_with_invalid_args
    assert_raises RangeError do
      BloomFilter.new(2, 2)
    end
  end

  def test_initialize_with_negative_args
    assert_raises ArgumentError do
      BloomFilter.new(-2, 2)
    end
  end

  def test_insert_uses_the_items_hash
    item = mock_item(123)
    instance = BloomFilter.new
    instance.insert(item)
    item.verify
  end

  def test_insert_changes_the_bit_array
    instance = BloomFilter.new
    previous = instance.bit_array
    instance.insert(Object.new)
    refute_equal previous, instance.bit_array
  end

  def test_include_uses_the_items_hash
    item = mock_item(123)
    instance = BloomFilter.new
    instance.include?(item)
    item.verify
  end

  def test_include_returns_true_for_member
    item = mock_item(123, 2)
    instance = BloomFilter.new
    instance.insert(item)
    assert instance.include?(item)
  end

  def test_include_returns_false_for_nonmember
    member = mock_item(123)
    non_member = mock_item(122)
    instance = BloomFilter.new(3, 10)
    instance.insert(member)
    refute instance.include?(non_member)
  end

  # An intentionally failing test
  def test_digest_always_sets_k_bits
    k = 3
    item = mock_item(720) # multiple of 10, 8 and 9
    instance = BloomFilter.new(k, 10)
    hash = instance.send(:digest, item) # testing private method
    assert_equal k, count_bits(hash), "Note: This test intentionally fails!"
  end

  # A helper method (mock object factory)
  def mock_item(hash, calls = 1)
    mock = Minitest::Mock.new
    calls.times { mock.expect(:hash, hash) }
    mock
  end

  def count_bits(number)
    count = 0
    until number == 0
      count += (number & 1)
      number = number >> 1
    end
    count
  end
end

A thing I've noticed in you past few questions: You're writing tests which is great, but you're writing the barest minimum of tests. For instance, you're not testing #insert at all, and your test for #include? doesn't include an edge case as the one mentioned above where 2 items shared the same first bit - but not all of them.

Basically, you should have a higher test-to-code ratio. It's not uncommon to have way more test code than implementation code; in fact that's something to aim for.

Addendum: This is an interesting topic, actually. Not usually my cup of tea (I suck at theoretical stuff like this), but I got nerd sniped.

I've also realized that my hash implementation is even more flawed than I thought: Even if it manages to set \$k\$ bits (which isn't guaranteed), their distribution isn't random. Since I'm using modulo @m - n, only the first iteration has a chance of setting the highest bit. Boo, me, boo.

Anyway, as I said, I don't know of a way to construct a hash function that will always set exactly \$k\$ bits with random distribution, and I'm not smart enough to work one out - but I can think of a hack.

If \$m\$ is simply defined to be some multiple of \$k\$, then we can do something like this:

If we say that \$k = 4\$ and our multiplier \$f = 8\$ (just as an example), then \$m = k \cdot f = 32\$. So we have an array that's 32 bits long, or - to put it another way - we have 4 arrays that are each 8 bits long.

We can then do something like this (using #hash for simplicity):

mask = 0
k.times do |n|
  hash = calc_hash(n, item) # calculate a big 'ol hash (assume this method exists)
  mask = mask << f          # shift our current mask f places
  mask |= 2**(hash % f)     # set a bit somewhere in the lower f places
end

mask will now contain exactly 4 "set" bits - one in each of its 4 "sections". Even if 2**(hash % f) is the same for each iteration, it'll still end up in different places, since we shift the mask.

This will not have the same false positive rate as a proper hashing function, but at least it'll be predictable. But it will be slightly worse overall.

In essence, what we have are four Bloom filters, each set to \$k = 1\$ and \$m = f\$. So (if my math is correct, which is not something to rely on) our false positive rate should be the regular rate for one such filter, but to the power of 4. Or, in mathematical terms:

$$(1 - (1 - \frac{1}{f})^{n})^{k}$$

(I think that's correct but I may be completely wrong)


Too many null checks in an XML-to-XML transformation

Your problem isn't null checks, it's a lack of encapsulation. This code is rifling through the metaphorical pockets of myPatientCareReport, like an overly familiar paperboy [PDF].

Let's look at what it knows:

myPatientCareReport has a field eHistory
eHistory has a field eHistoryCurrentMedsGroup
eHistoryCurrentMedsGroup is an array (or at least has an indexer)
the order of elements in eHistoryCurrentMedsGroup is meaningful
the elements of eHistoryCurrentMedsGroup have a field eHistory12
eHistory12 has a field PN
PN is a string with some opaque meaning (see also: stringly typed)

If any of these implementation details were to change, you would end up rewriting a lot of code.

  In object-oriented programming, encapsulation is the term for not
  only grouping data and behavior, but also hiding data and behavior
  implementation details within a class ... so that the inner workings
  of a class are not exposed. This reduces the chances that callers will
  modify the data inappropriately or program according to the
  implementation, only to have it change in the future. -- Essential C#
  5.0

Perhaps it's time for a Demeter Transmogrifier?

  Code that violates the Law of Demeter is a candidate for Hide
  Delegate, e.g. manager = john.getDepartment().getManager() can be
  refactored to manager = john.getManager(), where the Employee class
  gets a new getManager() method. However, not all such refactorings
  make as much sense. Consider, for example, someone who’s trying to
  kiss up to his boss: sendFlowers(john.getManager().getSpouse()).
  Applying Hide Delegate here would yield a getManagersSpouse() method
  in Employee. Yuck.

Now it's hard to suggest what this could look like because I'm not familiar with the problem domain, and a lot of the names here are meaningless to me (eHistory12? is there an eHistory13?). But you would probably want the code to end up looking something like this:

if (patientReport == null)
{
    ...
    return;
}

var currentMedicationGroup = patientReport.GetCurrentMedicationGroup(0);
currentOnMedicationObservation.Value.Add(GetBoolean(currentMedicationGroup));

private static BooleanValue GetBoolean(MedicationGroup medicationGroup)
{
    if (medicationGroup != null && medicationGroup.HasPertinentNegatives)
    {
        var pertinentNegatives = medicationGroup.GetPertinentNegatives();
        if (pertinentNegatives == PertinentNegatives.Refused ||
            pertinentNegatives == PertinentNegatives.UnableToComplete)
        {
            return new BooleanValue { NullFlavor = NullFlavor.NoInformation };
        }
        else
        {
            return BooleanValue.True;
        }
    }
    else
    {
        return BooleanValue.False;
    }
}


I can't comment on JS in particular too much because I'm no expert. I'll leave that for others more qualified. I can offer some advice on general programming principles though.

Code Styling

Code styling is a preference and yours is fairly consistent at least but since you asked about readability, I will talk a bit about it. Because code styling is generally considered to be opinionated, I will try not to be too specific and stick to the generally accepted norms.

Place opening braces on the same line as the block statement they belong to. There is no need to take up a whole line for a single character.  Use white space to format for neatness where appropriate instead. Same for else blocks. They belong to the if and cannot exist on their own.

if ( foo == bar ) {
} else if ( foo == foobar ) {
} else {
}

Also, space out your syntax appropriately. You should have a space before the opening brace.

window.RNG = function( min, max ) {

'if', 'for', 'while', etc are not functions. place a space between the keyword and the conditional (). See the 'if' example above. Do not place a space between function identifiers and the argument list (you are already doing this).

If you mix variable declarations and assignments, do not use a list of declarations. Create a separate declaration for each.

var dif = max - min, refill=function(){
    for (var i = min, result={length:0}; i <= max; i++) {
        result[i]=true;
        result.length++;
    }
    return result;
}, avail = refill();

... is better written as ...

var dif = max - min;

var refill = function() {
    for ( var i = min, result = { length:0 }; i <= max; i++ ) {
        result[i] = true;
        result.length++;
    }
    return result;
};

var avail = refill();

Programming Habits

Please don't do this. Mixing assignments in expressions is almost always a bad idea.

else if( ( min = min >> 0 ) >= ( max = max >> 0 ) )

You are essentially doing 5 different things in a single statement. This is very confusing and exceptionally prone to error. Without detailed inspection, it is not apparent that you are calculating and assigning min and max at the same time you are comparing them. Not good. Figure out how to split it into 3 statements: 2 assignments and the if statement with a simple conditional.

Why are you calculating dif inside of this expression and inside of a loop? The value of dif never changes.

result = ( window.Math.random() * ( dif + 1 ) + min ) << 0;

Precalculate whenever possible:

var dif = ( max - min ) + 1;

What is this? What's wrong with isNaN()? Try to avoid trickery. It's difficult to know what you intend to happen here. I only know what it's supposed to do by reading the error message.

if( min / 1 != min )

Also use parentheses to make your precedence explicit. Try not to rely on operator precedence. It requires deeper knowledge and is prone to mistakes. Also, the reader can't know if you made a logical error. In the above case we can assume what the precedence is supposed to be because it generally doesn't make sense to return a value to a conditional. That is not always the case though and you make the reader think harder than they should need to.

It has been raised that isNan() is not always available. From my own link, the comments also indicate that isNaN() will not work in every case. If these issues are a concern, then I would suggest creating a new function to self-document what is being done. Since the same test is used in two different places, it is a good candidate for it's own function anyway.


Finding an object in a nested object/array structure

I usually have methods to normalize data in the format I want it so you don't have to iterate again.
If you want you can have Schema that provides you methods or access to the data using custom methods.

var questions = new QuestionsSchema(data); // Normalizes to preferred format    questions.getOriginal(); // => Access to the original object
questions.getQuestion('0.1');    // =>      {"key": "0.0","title": "Question one","type": "integer"}
questions.getSection('section_1'); // => whole section

I like more the composed functions you build, is a more like functional way.


Objective C implementation of Swift's if-let

My suggestion would be: Don't do it. The main disadvantages
(in my opinion) are:

As you noticed, commas inside the macro argument are not handled,
leading to "strange" error messages:

for (SKNode *node in touchedNodes) {
    IF_LET(Tile, tile, node, {
        // Use of undeclared identifier 'IF_LET'
        SKAction *scale = [SKAction scaleBy: 1.2 duration: 0.15];
        SKAction *action = [SKAction sequence:@[scale, scale.reversedAction]];
        // Too many arguments provided to function-like macro invocation
        [tile runAction:action];
    })
}

It is not obvious how this must be fixed, and enclosing all Objective-C
message calls feels unnatural.
Macros are not known to the debugger. As a consequence, you cannot
set a breakpoint at a certain line inside the block – the debugger will
stop at the end of the block instead.
For the same reason you cannot single-step through the block statements.

Some of your intended applications have alternatives in pure
Objective-C, which are (almost) as good.

Example 1: Your code

for (SKNode *node in touchedNodes) {
    IF_LET(Tile, tile, node, {
        SKAction *scale = [SKAction scaleBy: 1.2 duration: 0.15];
        SKAction *action = ([SKAction sequence:@[scale, scale.reversedAction]]);
        [tile runAction:action];
    })
}

can be replaced by

for (Tile *tile in touchedNodes) {
    if ([tile isKindOfClass:Tile.class]) {
        SKAction *scale = [SKAction scaleBy: 1.2 duration: 0.15];
        SKAction *action = ([SKAction sequence:@[scale, scale.reversedAction]]);
        [tile runAction:action];
    }
}

The fast-enumeration for statement does not care how about the
type of the loop variable, you only have to verify its class before
accessing it.

Example 2: Your code

SKLabelNode *t = nil;
IF_LET(SKLabelNode, temp, t, {
    // doesn't run: t was nil
    [temp setText:@"text"];
})

can be replaced by (the even shorter)

SKLabelNode *t = nil;
[t setText:@"text"];

because sending Objective-C message to nil is a no-op.

Example 3: Your code

t = [SKLabelNode labelNodeWithText:@"some text"];
IF_LET(NSObject, temp, t, {
    // this block runs, letting you do stuff with the local scope variable
    // NSObject *temp = (NSObject*)t;
}) else {
    // else block available for use, but
    // this one doesn't run because t is not nil and is a kind of NSObject
}

is equivalent to

t = [SKLabelNode labelNodeWithText:@"some text"];
if (t != nil) {
    // Local scope for if-case
} else {
    // Local scope for else-case
}


Generic Singly-Linked List implementation

A few things that stuck out to me:

You don't gain anything by having the Node class public.  The user of your linked list should only need to be concerned about the data.

It seems very illogical to me to have the Node class printing the LinkedList.  This should more properly be only part of the LinkedList itself.  It would probably be advantageous though for the Node class to override the ToString() method to make it easier to print the data.

From a quick perusal, I couldn't see any use for the previous property except in the RemoveNode method.  I would suggest keeping it local to that method.

A comment explaining the purpose of the AddLastTwo method.  To me it appears to iterate over the list to find the last node, instead of using the last property of the class, like the AddLast.

To me names like AddLast and RemoveNode are over complicated.  If a node isn't being added to the end then it would be an insert operation not an add.  It shouldn't be necessary to tell the user that it's being added to the end.  Likewise the user doesn't need to know that the data is stored in a node, they are simply concerned with removing the data.

When you are using default get and set methods it's much easier to use the default syntax:

public Node<T> next{get;set;}


Joining couples, red candidates, and blue candidates

I'm afraid there's not much we can do to help with this kind of question. There's not much obviously wrong. You likely need to run the query analyzer and add some missing indices and keys. Regardless, there's still an opportunity for improvement, albeit not much performance wise...

 SELECT  *

Do you really need every column? Even with proper keys/indices in place, this will typically force a table scan. Explicitly state only the fields you need to return. It will result in less I/O and could possibly turn a scan operation into a seek. If it doesn't, you'll need to find the missing index to be added.

The next thing you can do is remove the duplication.

LEFT JOIN candidates_blue as cb
    ON cb.save_id = 3
        AND cb.user_id = c.id_candidate_blue

LEFT JOIN candidates_red as cr
    ON cr.save_id = 3
        AND cr.user_id = c.id_candidate_red

WHERE c.save_id = 3
    AND cb.enter_match = 1
    AND cr.enter_match = 1

I suppose you added all the save_id = 3s in an attempt to speed up the query? Don't bother. It won't help. Specifying it once for the couples table is sufficient. Which actually brings me to an optimization. Your query is equivalent to using INNER JOIN, but without any of the benefits. You're not currently getting the unmatched records anyway, so you might as well switch. You might see a significant performance increase.

SELECT  * /* don't forget to specify the fields you actually want */
FROM  couples as c

INNER JOIN candidates_blue as cb
    ON cb.user_id = c.id_candidate_blue

INNER JOIN candidates_red as cr
    ON cr.user_id = c.id_candidate_red

WHERE c.save_id = 3
    AND cb.enter_match = 1
    AND cr.enter_match = 1

One last thing: Very nice formatting! It's extremely easy on the eyes.


Like I wrote in my comment, the function you've got isn't really a parser. Yes, it parses the strings you pass it, but that's not its purpose, as far as I can tell. Parsing is a byproduct of the actual purpose, which seems to be to sanitize the strings by removing the domain key/value pair.

Now, if you're just looking to remove that key/value pair, you could simply do

function sanitizeCookies(strings) {
  return strings.map(function (cookieString) {
     return cookieString.replace(/\bdomain\s*=[^;]*;?/ig, "");
  });
}

Like @tomdemuyt, I'm using Array.map() so you won't get null back if you pass it an empty array; you'll just get another empty array (and, frankly, that makes more sense, I think).


Calculation of Energy Spectrum using C++

Avoid using namespace std;
If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well. This stack overflow question discusses this in more detail.
Include Files
You are currently including math.h 3 times, it is only necessary to do this once. Since you are using C++, stick with #include <cmath>.
Declare the Variables as Needed
The code contains this line:
    int i,j,k;

These are the variables you are using as loop control variables, they can therefore be declared in the loops themselves:
    for (int i = 0; i < np-1; i++){
        for (int j = i+1; j < np; j++){
        ...
        }
    }

Break the Code Into Functions to Make it Easier to Implement and Debug
Currently the code is all in main(), as programs grow and features get added the code would be to complex to easily update if it is all in main(). Also to achieve the ultimate goal of multi-threading for this program you will need to break the code up into multiple functions that can run on a separate core.
void FUNCTION_NAME(int i, int j, int np, double E[])
{
    double x = sin(double(2*i+j)/(3.0*np));//x,y,z,d1 for test.
    double y = sin(double(i+j)/(2.0*np));//x,y,z are velocity
    double z = sin(double(i*j)/np/np); //d1 velocity magnitude
    double d1 = sqrt(x*x + y*y + z*z);

    for (k = 1; k <= kmax; k++){
        double d2 = k * d1;
        E[k] += x + y + sin(d2) / d2;
    }
}

int main()
{
    int np = 1000000;  // number of fluid particle
    int kmax = 100;   // wavenumber
    double E[kmax+1] = {0.0};  //energy

    // need to be parallelized
    for (i = 0; i < np-1; i++){
        for (j = i+1; j < np; j++){
            FUNCTION_NAME(i, j, np, E[]);
        }
    }
}


Intercept read access to a certain object attribute

Something to watch out for with your approach is that Python calls certain special methods without looking them up via __getattribute__. For example, the __iter__ method:

>>> w = InterceptAttrWrapper(open('test'), 'attr', 'value')
>>> for line in w: print line
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'InterceptAttrWrapper' object is not iterable

See the documentation on "Special method lookup for new-style classes". If this affects you, then you'll need to add to your wrapper class all the special methods that you need to support (in my example, __iter__, but when wrapping other objects one might need to implement __getitem__, __repr__, __hash__, or others).

Here's a slightly more readable way to get at the wrapper object's own data, together with support for intercepting multiple attributes:

class InterceptAttrWrapper(object):
    """
    Wrap an object and override some of its attributes.

    >>> import sys
    >>> stdin = InterceptAttrWrapper(sys.stdin, name = 'input')
    >>> stdin.mode # Pass through to original object.
    'r'
    >>> stdin.name # Intercept attribute and substitute value.
    'input'
    """
    def __init__(self, obj, **attrs):
        self._data = obj, attrs

    def __getattribute__(self, name):
        obj, attrs = object.__getattribute__(self, '_data')
        try:
            return attrs[name]
        except KeyError:
            return getattr(obj, name)


Color string from object (random if empty, deterministic else)

From PEP 8:

  Comparisons to singletons like None should always be done with is or
  is not, never the equality operators.

  Also, beware of writing if x when you really mean if x is not None --
  e.g. when testing whether a variable or argument that defaults to None
  was set to some other value. The other value might have a type (such
  as a container) that could be false in a boolean context!

What you should do in your case is : obj = obj if obj is not None else random().

Then, even though I like the ternary operator, I reckon its usage is not required/idiomatic here. The usual way to handle default parameter is more like

if arg is None:
    arg = default()

As far as I can tell, using hash like you did is ok. However, I guess you should handle values that could be out of the range you are expecting (I couldn't find easily something telling which range of values can be returned).


Don't need :as resp if only body is ever used.
I'd also drop the :keys destructuring if it's not reused.
While couple of those maps make sense, this sequence is also pretty
repetitive, I'd suggest condensing repeated invocations into a single
call at least.
The (not (nil? ...)) is probably the same as the value itself, so
just (filter #(%1 "tim")) should suffice.
The function should most likely return the list instead of printing
it so the files can be examine further in another part of the code.
Maybe use mapcat instead of the map/reduce/concat combination?

The deref is fine I think, it also depends on how the calls should be
batched together (or not).

All in all:

(ns fourchan.core
  (:require [clojure.data.json :as json])
  (:require [org.httpkit.client :as http]))

(defn scrape []
  (->>
   (json/read-str (@(http/get "http://a.4cdn.org/wg/threads.json") :body))
   (mapcat #(%1 "threads"))
   (map #(http/get (str "http://a.4cdn.org/wg/thread/" (%1 "no") ".json")))
   (mapcat #((json/read-str ((deref %1) :body)) "posts"))
   (filter #(%1 "tim"))
   (map #(str (%1 "tim") (%1 "ext")))))


__version__ = '0.1'
__author__ = 'antmancoder'

# Importing of modules required for the script to run successfully

Comments should be used for the non-obvious parts of your code. Explaining that we have imports is obvious. It doesn't need a comment.

import scraperwiki
import lxml.html
import urlparse
import urllib2
import dateutil.parser
from geopy import geocoders

# Introduction of various global variables required throughout the running of the code
urlstem = "http://planecrashinfo.com"
urlyeardb = "database.htm"

Global constants should be in ALL_CAPS, by convention

yearsource = urlparse.urljoin(urlstem, urlyeardb)
yearlist = []
sourcepageurl = []

You shouldn't have global variables. It is better to return and pass parameters.

def parenturlscraper():
    """Function scrapes all of the parent URLs from 'planecrashinfo.com/database'"""
    html = scraperwiki.scrape(yearsource)
    root = lxml.html.fromstring(html)

    hrefs = root.cssselect('td a')

    for href in hrefs:
        link = href.attrib['href']
        url = urlparse.urljoin(urlstem, link)
        yearlist.append(url)

Instead of appending to a global variable, you should append them into a local list and return it.

def childurlscraper():
    """Function scrapes all of the child URLs from those scraped in the parenturlscraper module"""
    for url in yearlist:
        html = scraperwiki.scrape(url)
        root = lxml.html.fromstring(html)

Just did this, you should write a function that that combines the above two lines

        hrefs = root.cssselect('td a')
        url = url[0:34]

Why? This is a good place to have a comment explaining why you just did that.

        for href in hrefs:
            linkurl = href.attrib['href']
            url = urlparse.urljoin(url, linkurl)

You should store in  child_url. As it is you are reusing the variable from before. Your code will actually reeatedly join the elements onto each other. So I'm pretty sure that's not what you wanted.

            sourcepageurl.append(url)

Again, append onto a local list, and then return that.

def sourcepagescraper():
    """Function scrapes respective data for each accident and placed it into DB"""
    for url in sourcepageurl:
        try:
            html = scraperwiki.scrape(url)
            root = lxml.html.fromstring(html)
            for tr in root.cssselect("body"):

Okay, you call it tr but its actually the body. Also, isn't there only ever one body? Do you really need a lloop?

                tds = tr.cssselect("td")
                location = coorlookup(tds[7].text_content())
                for td in tds:

This doesn't seem right. You wnat to iterate over each cell? But then you look at all the cells to fetch the data.

                    crashinfo = {}
                    crashinfo['url'] = url
                    crashinfo['date'] = dateutil.parser.parse(tds[3].text_content()).date()
                    crashinfo['time'] = tds[5].text_content()
                    crashinfo['location'] = tds[7].text_content()
                    crashinfo['latitude'] = location[1][0]
                    crashinfo['longitude'] = location[1][1]
                    crashinfo['operator'] = tds[9].text_content()
                    crashinfo['flight no'] = tds[11].text_content()
                    crashinfo['route'] = tds[13].text_content()
                    crashinfo['aircraft type'] = tds[15].text_content()
                    crashinfo['registration'] = tds[17].text_content()
                    crashinfo['cn ln'] = tds[19].text_content()
                    crashinfo['aboard'] = tds[21].text_content()
                    crashinfo['fatalities'] = tds[23].text_content()
                    crashinfo['ground'] = tds[25].text_content()
                    crashinfo['summary'] = tds[27].text_content()

You can use something like:

crashinfo = {
        'url' : url,
        'date' : dateutil.parser.parse(tds[3].text_content())).date()
        ...
}

which will probably be a bit better. Also, I'd avoid reference the cells by the index. That'll break if new cells are added. Are there classes on the cells you can use? I'd also probably make a big list like:

DATA = [
    ('time', 5),
    ('operator', 9)
]

And then loop over the list to fill in the dict.

                scraperwiki.sqlite.save(unique_keys=['url'], data=crashinfo)
        except urllib2.HTTPError, err:
            if err.code == 404:
                continue

Firstly, you want to put as little as possible in the try block. This is helped by the fact that python has an else block. Everything after .scrape should be in the else block. But the error handling also is a bit off. I think you want to ignore scrapings when the page cannot be found. But you should really reraise the exception when its not that. As it stands, you ignore it. Also, continue is exactly the same as pass here.

def coorlookup(location):
    """Function is called from the 'sourcepagescraper' function to geolocate locations listed on website for each accident"""
    g = geocoders.Google()

Avoid single letter variables names, they are harder to follow

    try:
        loc = g.geocode(location, exactly_one=True)
        return loc

Combine those two lines

    except:

Don't ever (well almost ever) use bare excepts. Catch the specif error you want to handle.
            return ("",("",""))

parenturlscraper()
childurlscraper()
sourcepagescraper()


(let ((numbers (range 3 (+ limit 1) 2))

Above defines a list of numbers. Later you set various numbers to zero. That's not good for a list structure. Use a vector, an one-dimensional array.

(dolist (step (range 3 (+ limit 1) 2))

Creating the list and then iterating over it makes no sense. Use a LOOP statement directly.

  (dolist (i (range start half step))

Creating the list and then iterating over it makes no sense. Use a LOOP statement directly.

Example:

(defun sundaram-sieve (limit)
  (let ((numbers (coerce (cons 2 (range 3 (+ limit 1) 2)) 'vector))
        (half (/ limit 2))
        (start 4))
    (loop for step from 3 upto limit by 2 do
          (loop for i from start below half by step do
                (setf (aref numbers i) 0))
          (incf start (* 2 (1+ step)))
          (when (> start half)
            (return (remove 0 numbers))))))

Runs fast.

CL-USER 36 > (time (sundaram-sieve 100000))
Timing the evaluation of (SUNDARAM-SIEVE 100000)

User time    =        0.007
System time  =        0.000
Elapsed time =        0.004
Allocation   = 1646136 bytes
0 Page faults
#(2 3 5 7 11 13 17 ... 99971 99989 99991)


string manipulation, which version of those 2 functions is better?

I like the more the first, because it works at an higher level of abstraction and it makes it clear that you are interested in comparing a part of the string with the postfix.

I'd probably introduce another variable to clearly represent that you are comparing the postfix with the latest strlen(postfix) chars of the original string.


I'm not seeing anything wrong with your SQL or your view. The controller can be cleaned up a lot. In fact, you could reduce your controller method to two lines.

The Product.find_by_sql call should be encapsulated in the Product class as its own static method
The SQL should be stored in a constant
The logic for finding products by product type, or defaulting to active products should also be a static method on the Product class

Basically, you should be able to do this in your controller:

@products = Product.find_by_product_types params[:product_type_id]
@sizes = Product.sizes

The Product class could be amended like below:

class Product < ActiveRecord::Base
  SQL_FIND_SIZES = "SELECT products.id,
                           LEFT(variant_properties.description,1) AS short_desc,
                           variants.price
                    FROM products
                    INNER JOIN product_properties ON product_properties.product_id = products.id
                    INNER JOIN variant_properties ON product_properties.property_id = variant_properties.property_id
                    INNER JOIN properties ON     properties.id           = product_properties.property_id
                                             AND properties.id           = variant_properties.property_id
                                             AND properties.display_name = 'Size'
                    INNER JOIN variants ON variants.product_id = products.id AND variants.id = variant_properties.variant_id"
  def self.sizes
    self.find_by_sql SQL_FIND_SIZES
  end

  def self.find_by_product_types(product_type_id)
    return [] if product_type_id.nil?

    product_type = ProductType.find_by_id product_type_id
    product_types = product_type.self_and_descendants.map(&:id) if product_type

    return if product_types.nil?
             self.active
           else
             self.where product_type_id: product_types
           end
  end
end


Repeated DB connection at max user connections

You code has several problems. Specificaly it breaks at least two of the SOLID principles, namely single responsibility and dependency inversion principles.
Single responsibility is violated because the class is responsible for:

connecting to db
retrying connection
whatever other methods provided by the class you did not show us

Dependency inversion principle is violated because the class uses hardcoded values (although stored in instance properties).
Let me first separate out the first responsibility. We don't really need OOP for that, so let me do it in FP style.
function createMyPdo(): \PDO
{
  $dsn = ...;
  $user = ...;
  $password = ...;
  $options = ...;
  return new \PDO($dsn, $user, $password, $options);
}

This function now opens a connection with hardcoded credentials. Although the credentials are hardocded and we would better pull those values off of maybe environment variables. It at least has just one responsibility and that is to open the specific database connection.
Now lets implement the retry logic as a separate thing, that only depends on something that creates PDO instance.
function createPdoWithRetry(callable $factory, int $attempts): \PDO
{
  $attemptsMade = 0;
  while ($attempts > $attemptsMade) {
    try {
      return $factory();
    } catch (\PDOException $e) {
      if ($e->getCode() === 1040) {
        \usleep(\pow(2, $attempsMade) * 10000);
        ++$attemptsMade;
      } else {
        throw $e;
      }
    }
  }
  throw new \RuntimeException("All $attemptsMade retry attempts exhausted.");
}

Now I can easily open the connection with retry logic
$pdo = createPdoWithRetry(fn () => createMyPdo(), 20);

And I can also open a different connection using the same thing:
$pdo2 = createPdoWithRetry(fn () => createOtherPdo(), 20);

This approach still has some caveats:

where createMyPdo could only throw PDOException, createPdoWithRetry can also throw a generic RuntimeException.
the type of the $factory parameter has a very vague type, that we have to describe in a docblock and cannot be enforced by PHP runtime.
createPdoWithRetry knows error code 1040 which is mysql specific, but the knowledge that mysql is used is only known to the createMyPdo function.

Let's make it better by going back to OOP style and introducing an interface for the connection factory, including specific exceptions to be thrown (which will abstract away the 1040 speific mysql error code).
class DatabaseConnectionException extends \Exception
{
}

class TooManyConnectionsException extends DatabaseConnectionException
{
}

interface DatabaseConnector
{
  /**
   * @return \PDO
   * @throws DatabaseConnectionException
   * @throws TooManyConnectionsException
   */
  public function createPdo(): \PDO;
}

As you can see I added some @throws annotations to:

inform implementors of the interface which exception they can throw
informs consumers of the interface which exceptions they can catch

Although PHP cannot enforce the actual type of exceptions thrown from the implementations, documenting the exceptations makes it less likely that someone will implement it wrong.
Now lets make an implementaton for our specific connection:
class MyDatabaseConnector implements DatabaseConnector
{
  public function createPdo(): \PDO
  {
    $dsn = ...;
    $user = ...;
    $password = ...;
    $options = ...;
    try {
      return new \PDO($dsn, $user, $password, $options);
    } catch (\PDOException $e) {
      if ($e->getCode() === 1040) {
        throw new TooManyConnectionsException($e->getMessage(), (int) $e->getCode(), $e);
      } else {
        throw new DatabaseConnectionException($e->getMessage(), (int) $e->getCode(), $e);
      }
    }
  }
}

Now maybe you see why the credentials should not be hardocded, because the same code would work for any credentials. Hardcoding the credentials, we are doomed to repeat some code if we want to open a different connection.
But I leave that up to you to find your way to the dependency inversion principle. Basically constructor of a class should accept things that the class instances need to know, rather then having the instances deciding on their own what those values should be (ie. pass the credentials through constructor, rather then hardocing the credentials withing the class).
Anyway, to implement the retry logic we will use the same interface:
class RetryingDatabaseConnector implements DatabaseConnector
{
  private DatabaseConnector $factory;
  private RetryDelayStrategy $delayStrategy;
  private int $maxAttempts;

  public function __construct(DatabaseConnector $factory, RetryDelayStrategy $delayStrategy, int $maxAttempts)
  {
    $this->factory = $factory;
    $this->delayStrategy = $delayStrategy;
    $this->maxAttempts = $maxAttempts;
  }

  public function createPdo(): \PDO
  {
    $attemptsMade = 0;
    while ($this->maxAttempts > $attemptsMade) {
      try {
        return $this->factory->createPdo();
      } catch (TooManyConnectionsException $e) {
          ++$attemptsMade;
          $this->delayStrategy->wait($attemptsMade)
      }
    }
    throw new TooManyConnectionsException("All $attemptsMade retry attempts exhausted.");
  }
}

As you can see I have also extracted the delay logic to a separate interface, because exponentialy growing gaps between attempts is just one of many possible strategies. And again, you don't want to implement a new connector that would repeat a lot of the logic just to change to delay strategy.
interface RetryDelayStrategy
{
  public function wait(int $attemptsMade): void;
}

class ExponentialRetryDelayStrategy implements RetryDelayStrategy
{
  private int $coefficient;
  private int $base;

  public function __construct(int $coefficient, int $base = 2)
  {
    $this->coefficient = $coefficient;
    $this->base = $base;
  }

  public function wait(int $attemptsMade): void
  {
    \usleep(\pow($this->base, $attempsMade) * $this->coefficient);
  }
}

And finaly some consumer code. A controller that opens a database connection (to do something with it) and gives a user friendly messages if the connection fails and a more specific one if the connection fails specificaly because of too many open connections.
class MyController extends BaseController
{
  private DatabaseConnector $connector;

  public function __construct(DatabaseConnector $connector)
  {
    $this->connector = $connector;
  }

  public function myControllerAction(Request $request): Response
  {
    try {
      $pdo = $this->connector->createPdo();
    } catch (TooManyConnectionsException $e) {
      return $this->send(503, "Too many database connections.");
    } catch (DatabaseConnectionException $e) {
      return $this->send(503, "Database unavailable.");
    }

    // do something with PDO here

    return $this->send(200, "All done");
  }
}

Now, you see the controller just depends on a DatabaseConnector. It doesn't really care if and how many times the connector will retry. It just cares that it can fail in a specific (TooManyConnectionsException) or an further unspecified (DatabaseConnectionException) way or, if it succeeds, it returns a PDO instance.
And it will work no matter what connector you pass to it, as long as the interface is implemented correctly. And so you can change the implementation without touching the controller code. Again, thanks to single responsibility a dependency inversion principles.
No matter which is used in the DI setup
$myConnector = new MyDatabaseConnector();
// or maybe if we pull it to envs
// $myConnector = new MyDatabaseConnector($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASS']);
$connector = $myConnector;

or
$delayStrategy = new ExponentialRetryDelayStrategy(10000, 2);
$retryingConnector = new RetryingDatabaseConnector($myConnector, $delayStrategy, 20);
$connector = $retryingConnector;

the controller code stays untouched
$controller = new MyController($connector);
...
$controller->run();

Also notice, how I avoided setting an errorCode and checking it in the controller. Instead we just throw exceptions. This forces the caller to actually handle the error or let it bubble up the stack and eventually stop execution of the program.
This is the prefered behaviour because if you would forget to check the error code, it really does not make sense to try to send sql queries over a "not opened connection".
Btw, if you are interested in design patterns, you can notice that:

DatabaseConnector and its implementations are a factory method pattern
RetryingDatabaseConnector and the interface follow the decorator pattern
RetryDelayStrategy and its implementations are the strategy of the strategy design pattern and RetryingDatabaseConnector is the strategy consumer


As this is a quit nice approach, it is missing an important part.

Validation of input parameter

You should check at least the step parameter if it is a positive number greater 0.

InclusiveRange(1d, 5d,-1);

the above call is by your definition a valid call, but can lead to a System.OutOfMemoryException if combined with the .ToList() extension method.

You need to decide if you want to enable negative step. If you don't want to allow negative values for step a simple guard clause can handle this.

if (step <= 0) { throw new ArgumentOutOfRangeException("step", "The value has to be greater than 0"); }

But if you want to enable negative step values you need to add more checks and a different implementation.

A negative step means that start > end so you would need to introduce 2 more methods which you then call based on step beeing positive or negative.

private static IEnumerable<double> InclusiveRangeAscending(double start, double end, double step)
{

}

private static IEnumerable<double> InclusiveRangeDescending(double start, double end, double step)
{

}

public static IEnumerable<double> InclusiveRange(double start, double end, double step = 1)
{
    if (step == 0) { throw new ArgumentOutOfRangeException("step", "The value has to be unequal to 0"); }
    if (step > 0)
    {
        return InclusiveRangeAscending(start, end, step);
    }
    else
    {
        return InclusiveRangeDescending(start, end, step);
    }
}


Both your and janos' implementations are broken:

data = [1, 0] * 10000
consecutive_one(data)
#>>> 140

This is because you don't always reset after seeing a 0. Going from janos', you should have

longest = 0
current = 0
for num in data:
    if num == 1:
        current += 1
    else:
        longest = max(longest, current)
        current = 0

return max(longest, current)

and equivalent for the original.

You'll find that this functionality is largely provided by itertools.groupby, though:

from itertools import groupby

def len_iter(items):
    return sum(1 for _ in items)

def consecutive_one(data):
    return max(len_iter(run) for val, run in groupby(data) if val)


I am surprised this compiles and works (it does I test it)

 global_handler();

As global handler is a pointer to a function  I was expecting to see this:

 (*global_handler)();

Comments:

Throw specifiers (except no throw) have been deprecated in the latest standard.
So you should probably follow their lead in that

void* operator new(std::size_t n) throw(std::bad_alloc)

The array version of new calls the no-throw version of new.

void* operator new[](size_t n, std::nothrow_t const&) throw()

Thus you are not getting the benefits of any registered out of memory handlers. Maybe you should just call the normal version of new so that the out of memory handlers are called for arrays as-well.

  Do I need any special alignment handling?

No. malloc() provides memory that is aligned for any type.
http://pubs.opengroup.org/onlinepubs/009695399/functions/malloc.html

  Should there be any other differences between operator new and operator new[]?

No. Apart from not using the out of memory handlers it looks fine.


Your code is inefficient because each iteration within the sapply has to search through the 100k rows of data to find the relevant ones. Without changing your code at all, you could get a significant speed improvement by first pre-aggregating your input data:

dat <- aggregate(lab ~ index, data = dat, FUN = sum)

This way, each iteration will only have to search through $M = 2154$ rows instead of the 100k. But it is not fixing everything and I prefer rewriting your code with your more general goal in mind, where

indx.vec <- c(1:10, 14:20)

First, aggregate your data using tapply:

agg.lab <- with(dat, tapply(lab, list(index), FUN = sum))
length(agg.lab)
# [1] 2154
head(agg.lab)
#  1  2  3  4  5  6
# 25 23 26 21 22 21

It returns a vector containing sum(lab) for each distinct value of index. In the event that the data has no value for a given index, you could overwrite the output-ed NA with a 0 so as to not affect the upcoming roll sums:

agg.lab[is.na(agg.lab)] <- 0

Next, we subset by only keeping the indices in indx.vec

agg.lab   <- agg.lab[indx.vec]
#  1  2  3  4  5  6  7  8  9 10 14 15 16 17 18 19 20
# 25 23 26 21 22 21 19 21 24 28 21 18 24 22 23 21 20

Next, we map each value of indx.vec to the "group number" it belongs to (here, 1:14 belong to group #1 then 14:20 belong to group #2):

indx.grp  <- cumsum(c(TRUE, diff(indx.vec) != 1))
# [1] 1 1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2

Using this vector of group indices, we break agg.lab into a list of vectors, one for each group number:

agg.split <- split(agg.lab, indx.grp)
# $`1`
#  1  2  3  4  5  6  7  8  9 10
# 25 23 26 21 22 21 19 21 24 28
#
# $`2`
# 14 15 16 17 18 19 20
# 21 18 24 22 23 21 20

Finally, we rollsum within each vector (where I used width = 4 for illustration)

out <- lapply(agg.split, zoo::rollsum, k = width)
# $`1`
#  2  3  4  5  6  7  8
# 95 92 90 83 83 85 92
#
# $`2`
# 15 16 17 18
# 85 87 90 86

With indx.vec <- 1:M, this code runs in 0.015 seconds on my machine, versus 12+ seconds for your count.ones. It also scales pretty well as it
takes less than 0.4 sec for n1 = n2 = 1e6.


I think your Insert function is about as tight as you can make it without getting into strictly-organizational functions to tuck the list details (head, tail) and node details (next, prev) out of sight. Not a big deal for this one function and probably not for the class overall.

If I could make one suggestion, though, it would be that you reconsider having the Find function serve as both a match-finding function (returning true or false accordingly) and a state-modifying function that updates member currPos.

If I saw the definition of Find in the header, I would wonder why it was not a const function since the name and return value give me no reason to think it's modifying the object itself -- surely it just returns true if a match is found and nothing else. Also, since the list is ordered, it would seem that currPos must always be updated before any meaningful modification is performed: it determines where the current insert/delete occurs (not an arbitrary insert/delete like against head or tail) so currPos has no value to insertion/deletion before or after the call and thus no value as a member. So currPos as a member seems like unnecessary temporary storage at best and error-prone storage at worst.

Assuming my assessment of Find and currPos are fair, consider removing member currPos and adding a function to manage the list search without storing the node or the match flag in the object, like so:

node* OrdListClass::FindNode(/*IN*/ItemType searchKey, bool& matchFound) const
{
    matchFound = false;

    //Skip operations if empty.
    if(IsEmpty()) return nullptr;

    node* currPos = head;

    while(currPos != nullptr)
    {
        if(currPos->data == searchKey)
        {
            matchFound = true;
            return currPos;
        }
        else if (currPos->data > searchKey)
        {
            currPos = currPos->prev;
            return currPos;
        }
    }
}

Find then becomes:

bool OrdListClass::Find(/*IN*/ItemType searchKey) const
{
    bool matchFound = false;
    //FindNode is only called to determine if there is an exact match.
    FindNode(searchKey, matchFound);
    return matchFound;
}

And Insert becomes:

void OrdListClass::Insert(ItemType item)
{
    bool matchFound = false;
    node* currPos = FindNode(item, matchFound);

    if (matchFound)
    {
      //Nothing to insert.
      return;
    }

    //currPos points to the node that we're inserting AFTER
    node* newNode = new node;
    newNode->data = item;

    if(currPos == nullptr)
    {
        //We're inserting at the head.
        newNode->prev = nullptr;
        newNode->next = head;
        head->prev = newNode;
        head = newNode;
    }
    else if(currPos->next == nullptr)
    {
        //We're inserting at the tail
        newNode->next = nullptr;
        newNode->prev = tail;
        tail->next = newNode;
        tail = newNode;
    }
    else
    {
        //We're inserting in the middle of the list.
        newNode->prev = currPos;
        newNode->next = currPos->next;
        currPos->next->prev = newNode;
        currPos->next = newNode;
    }
}//End Insert

There are alternative ways to implement FindNode rather than passing a bool reference for the matching flag (e.g., returning something like a NodeSearchResult object that contains the flag and the node together), but the gist is that FindNode is a function that manages the insertion/deletion point without resorting to using a member for temporary storage.

Whether or not you choose to go with something like this, I hope it gives you something to think about. Otherwise, like I said, Insert itself seems fine.


My main recommendation is that your variables have more descriptive names. While !x! may seem like a reasonable variable name for a counter that you're never going to use again, calling it something like !counter! makes your code easier for other people to maintain when there's a lot more of it.

I also got rid of ENABLEEXTENSIONS because that's enabled by default, and I moved cd users above :login_menu because if somebody enters an invalid user number and you're already in the users folder, your code will try to go into the users folder that's inside of the users folder and since there isn't one, you'll get an error.

To cut out that nested for loop, I stored the username map in an array. From there, you can determine whether or not a username is valid by if the variable exists.

Finally, I replaced the !s that you echoed with ^^!s so that they would be escaped. Because you have delayed expansion enabled, the last bit of your code would be displayed as HeyuserN because batch would consider ! You're logged in as ! to be a variable (because that's a valid variable name in batch).

If you could guarantee that you would never have more than ten users, you could build a string for a choice list to guarantee that the user would never enter an invalid user number, but I assumed this would be used by a large number of people.

@echo off
setlocal enabledelayedexpansion

cd users

:login_menu

set "user_counter=1"
for %%A in (*.user) do (
    echo !user_counter!. %%~nA
    set "users[!user_counter!]=%%~nA"
    set /a user_counter+=1
)
echo.

set /p "user_selection=Select user: "
if "%user_selection%"=="" goto login_menu
if not defined users[%user_selection%] (
    echo That user does not exist^^!
    pause
    goto login_menu
)
set "user_name=!users[%user_selection%]!"

:enter_password
for /f "tokens=1,2 delims=:" %%E in (!user_name!.user) do set "stored_password=%%F"
echo Enter your password, !user_name!
echo.
set /p "entered_password=Password: "

if "%entered_password%"=="%stored_password%" goto menu
echo.
echo That password is invalid^^!
pause
goto enter_password

:menu
echo Hey^^! You're logged in as !user_name!^^!
pause


Is there a cleaner way to add DEBUG comments?

I think it would be much cleaner to utilize partial methods to create your logging statements.  That way you can log wherever you need it and can disable the code my omitting it the logging function definition.  By using partial methods, if the definition is omitted, no IL is generated for the method and calls to the partial method are ignored as if it was never there.

Just mark the class partial, define the signature of the partial method and call it like normal.  Then wrap the actual implementation in the conditional compilation blocks.

partial class MyClass
{
    // declare the partial method
    static partial void Log(string format, params object[] arguments);

    static void SomeMethod()
    {
        // call the log method like usual
        Log("Source Site Set to: {0}", archiveQueueEntity.SourceSite);
        Log("Source List Set to: {0}", archiveQueueEntity.SourceUrl);
        Log("Destination Site Set to {0}", archiveQueueEntity.DestinationSite);
        Log("Destination List Set to: {0}", archiveQueueEntity.DestinationUrl);
    }

#if DEBUG
    static partial void Log(string format, params object[] arguments)
    {
        Console.WriteLine(format, arguments);
    }
#endif
}

Otherwise if you're not able to change it, you should still create a separate logging method and disable to actual printing in the method there.  That way your method is called but does nothing.

static void Log(string format, params object[] arguments)
{
#if DEBUG
    Console.WriteLine(format, arguments);
#endif
}

Or alternatively, use the Trace class to do your logging.  As long as you have no listeners registered, you will not see any of the logging messages.  When debugging, add a ConsoleTraceListener to the Listeners collection.

#if DEBUG
    Trace.Listeners.Add(new ConsoleTraceListener());
#endif

Trace.WriteLine("Source Site Set to: {0}", archiveQueueEntity.SourceSite);
Trace.WriteLine("Source List Set to: {0}", archiveQueueEntity.SourceUrl);
Trace.WriteLine("Destination Site Set to {0}", archiveQueueEntity.DestinationSite);
Trace.WriteLine("Destination List Set to: {0}", archiveQueueEntity.DestinationUrl);

Something that eluded me until now, use the ConditionalAttribute attribute on the log function for both cases to achieve the same effect.

[Conditional("DEBUG")]
static void Log(string format, params object[] arguments)
{
    Console.WriteLine(format, arguments);
}