Testing whether today is the second or fourth Saturday of the month This is working. Is it ok?

import calendar
from datetime import datetime

def second_fourth_saturday(year):
    holidays = {}
    for month in range(1, 13):
        cal = calendar.monthcalendar(year, month)
        if cal[0][calendar.SATURDAY]:
            holidays[month] = (
                cal[1][calendar.SATURDAY],
                cal[3][calendar.SATURDAY]
            )
        else:
            holidays[month] = (
                cal[2][calendar.SATURDAY],
                cal[4][calendar.SATURDAY]
            )
    return holidays

if __name__ == "__main__":
    today = datetime.today().day
    tomonth = datetime.today().month
    if today in second_fourth_saturday(2019)[tomonth]:
        print("Enjoy")
    else:
        print("Start Working")

A few suggestions

collections.defaultdict

holidays = {}
for month in range(1, 13):
    holidays[month] = []
    ...

There is a module for dictionaries starting with a basic datatype

from collections import defaultdict
holidays = defaultdict(tuple)

Secondly you first init it as a list, and afterwards you make it a tuple this is odd. Pick one and stick with it
You don't have to calculate all the months only the specific month

Since you already know which month it is now, just calculate only the month you are interested in

To do this you would have to give the month as a second parameter

def second_fourth_saturday(year, month):
    cal = calendar.monthcalendar(year, month)
    ...

Don't Repeat Yourself

if cal[0][calendar.SATURDAY]:
    holidays[month] = (
        cal[1][calendar.SATURDAY],
        cal[3][calendar.SATURDAY]
    )
else:
    holidays[month] = (
        cal[2][calendar.SATURDAY],
        cal[4][calendar.SATURDAY]
    )

If you calculate the weeks beforehand you don't have to repeat yourself

second_fourth_saturday = (1, 3) if cal[0][calendar.SATURDAY] else (2, 4)

Return what is asked

Instead of return a dict of month with second/fourth saturdays, I think it would be more clear if the function returns a boolean value if the day is a second or fourth saturday

Code

from calendar import monthcalendar, SATURDAY
from datetime import datetime

def second_fourth_saturday(date):
    month_calender = monthcalendar(date.year, date.month)
    second_fourth_saturday = (1, 3) if month_calender[0][SATURDAY] else (2, 4)
    return any(date.day == month_calender[i][SATURDAY] for i in second_fourth_saturday)

if __name__ == "__main__":
    is_second_fourth_saturday = second_fourth_saturday(datetime.today())
    print("Enjoy" if is_second_fourth_saturday else "Start working")


Project Euler #12 very long runtime Project Euler Problem 12 asks to find the value of the first triangular number to have over 500 divisors, where the \$n\$th triangular number is \$\sum_{i=1}^{n} i\$.

  The sequence of triangle numbers is generated by adding the natural
  numbers. So the 7th triangle number would be \$1 + 2 + 3 + 4 + 5 + 6 + 7 = 28\$. The first ten terms would be:

1, 3, 6, 10, 15, 21, 28, 36, 45, 55, ...

  Let us list the factors of the first seven triangle numbers:

 1: 1
 3: 1,3
 6: 1,2,3,6
10: 1,2,5,10
15: 1,3,5,15
21: 1,3,7,21
28: 1,2,4,7,14,28

  We can see that 28 is the first triangle number to have over five
  divisors.

I have the following code written in C#. I never had the patience to wait for the program to finish but the code shows the right answer because I tried  the example on their website. My code has a very long runtime and I don't know what to do to optimize it. I've waited for like 5-6 minutes on a AMD 4x cores 3.10 GHz and nothing...

static void Main(string[] args) {

        Stopwatch time = new Stopwatch();
        time.Start();

        int trianglenumber = 0;
        int divizori = 0;

            for (int i = 1; i < Int32.MaxValue; i++) {
                int tempnumber = 0;
                for (int j = 1; j < i; j++) {
                    tempnumber += j;
                }
                for (int k = 1; k < tempnumber+1; k++) {
                    if (tempnumber % k == 0) {
                        divizori++;
                    }
                }
                if (5 < divizori) {
                    trianglenumber = tempnumber;
                    break;
                }
                divizori = 0;
            }

            time.Stop();

            double timp = time.ElapsedMilliseconds ;

            Console.WriteLine(trianglenumber);
            Console.Write("Runtime: " + timp/1000+ " seconds");
            Console.ReadKey();

    }

You should change your algorithm. First note that triangle numbers can be written as:
\$
T_n = \frac{n(n+1)}{2}
\$
then notice that \$n\$ and \$n+1\$ cannot share a common divisor. So you can find all the divisors of \$T_n\$ just by finding the divisors of \$n\$ and \$n+1\$ (and removing a factor 2) and multiplying them together. This alone should greatly improve your algorithm turning it (the part inside the main loop) from \$O(n^2)\$ to \$O(n)\$.

Then you could make a table of prime number to quickly find the factorization of \$n\$ and \$n+1\$. Once you know the prime factors of \$n\$ you can find the number of all divisors of \$n\$. See, for example, here: http://www.marksmath.com/math/problems/count-div.html

addendum

First advice is actually enough. Here is the code (I don't know C#, hope it compiles):

static int count_divisors(int n) {
  int count = 0;
  for (int k=1;k<=n;++k) {
    // this loop can be reduced further by noticing that
    // apart from n itself there is no divisor with k*2>n
    if (n % k == 0) count++;
  }
  return count;
}

static void Main(string[] args) {
  int last_d = 0; // cache the computation for previous n
  for (int n=0;;++n) {
    int triangular = n*(n+1)/2; // only used for debugging and clarity
    int d; // number of divisors of (n+1) (or (n+1)/2 if (n+1) is even)
    if (n%2==0)
      d = count_divisors(n+1);
    else
      d = count_divisors((n+1)/2); // n+1 is even
    // # of divisors of n * (n+1)/2 is the product of divisors
    // since two consecutive numbers do not share any divisor (apart from 1)
    int total = d*last_d;
    if (total>500) {
      Console.WriteLine("found: " + triangular);
      break;
    }
    last_d = d; // cache the computation
  }
}


Python graphical user interface test In my programming class I am learning how to make graphical user interfaces, and so I decided to make one.

import tkinter as t
from random import choice

COLORS = ["white", "black", "red", "green", "blue", "cyan", "yellow", "magenta"]

def button_clicked():
    display_area.config(text = "You clicked the button!")
    screen.itemconfig(epilepsy_man, fill = choice(COLORS))

def epilepsy_man_movment(event):
    key = event.keysym
    if key == "Right":
        screen.move(epilepsy_man, 10, 0)
        screen.itemconfig(epilepsy_man, fill = choice(COLORS))
    elif key == "Left":
        screen.move(epilepsy_man, -10, 0)
        screen.itemconfig(epilepsy_man, fill = choice(COLORS))
    elif key == "Up":
        screen.move(epilepsy_man, 0, -10)
        screen.itemconfig(epilepsy_man, fill = choice(COLORS))
    elif key == "Down":
        screen.move(epilepsy_man, 0, 10)
        screen.itemconfig(epilepsy_man, fill = choice(COLORS))

window = t.Tk()
window.title("Your reading this right now. Please stop.")

button = t.Button(window, text = "Can you click me?", command = button_clicked)
button.pack()

display_area = t.Label(window, text = "Go on! Click the button!")
display_area.pack()

screen = t.Canvas(window, width = 500, height = 500)
screen.pack()
epilepsy_man = screen.create_oval(200, 200, 300, 300, fill = choice(COLORS))
screen.bind_all("<Key>", epilepsy_man_movment)

window.mainloop()

Please note "epilepsy". If you run this there will be flashing images.

I want to know if I properly followed any conventions, as well as if there is any better way of doing what I have done.

What this program does is make a button that changes the color of the circle and the text on a textbox, as well as creating said textbox and said circle which moves with the arrow keys and changes with each key press.

You misspelled "movement" as "movment".

Typically (as per PEP-8), indentations are specified by one hard tab or four spaces, so you're good in that respect.

To be sure about formatting you can read up on PEP-8.

I am also wondering about _man. This doesn't seem to be displaying a man (but I haven't run it) so why name it that way? Also, epilepsy doesn't seem like a suitable name. Perhaps colored_box would be more formal?


Codeigniter 3 - Set Flashdata Array issue I need help about CodeIgniter $this->session->set_flashdata array.

I think my code is so long, is there any way to simplify my code?

Script at my views

<?php
for($i=0; $i<$max; $i++) {
   if($this->session->flashdata("'warning".$i."'"))
      echo $this->session->flashdata("'warning".$i."'");
   if($this->session->flashdata("'info".$i."'"))
      echo $this->session->flashdata("'info".$i."'");
}
?>
<form action="xxxx/insertMember" method="post">
   <?php for($i=0; $i<$max; $i++) { ?>
      <label>Username <?php echo $i; ?></label>
      <input type="text" placeholder="username" name="username[<?php echo $i; ?>]>
   <?php } ?>
   <button type="submit">Submit</button>
</form>

Script at my controller

...
parent::__construct();
$max = 5;

public function insertMember() {
....form was validated & return true...
   $name = $this->input->post('username',TRUE);
   $a = 1;
   $b = 1;
   for($i=0; $i<$max; $i++) {
      if($name[$i] != '') {
         //check username was registered on database or not.
         $doCheck[$i] = $this->model->check($name[$i]);

         //if already registered
         if(!empty($doCheck[$i])) {
            $this->session->set_flashdata("'warning".$a."'",$name[$i].' was registered');
            $a++;
         }

         //if not registered
         else {
            $this->model->insert($name[$i]);
            $this->session->set_flashdata("'warning".$b."'",$name[$i].' sucessfully registered');
            $b++;
         }

         redirect('registerpage');
}

This seems more like a code review question (like Alive to Die comments), so I'm not entirely sure what you are needing.  For the most part, your code looks fine.  I have a few things to consider, but they are mainly preferential.

You could adjust the View output within the form.

<?php
    for($i=0; $i<$max; $i++) {
        echo '<label>Username ' . $i . ' </label><input type="text" placeholder="username" name="username[' . $i . ']>';
    }
?>

In your Controller, you could also initiate $a and $b differently, as mentioned in this Stack Overflow article: Assign same value to multiple variables.

$a = $b = 1;


Determining if a relation is reflexive A binary relation \$R\$ on a set \$A\$ is called reflexive if and only if \$R(a, a)\$ for every element \$a \in A\$.

I want to know if there can be any improvements made on the function below to make it more efficient.

def reflexive(R):
    '''
    @param R : set containing homogenous elements
    '''
    result = []
    a = []
    y = []

    for a1, a2 in R:
        if (a1 == a2):
            result.append((a1,a2))
            y.append(a1)

        if (a1 not in a):
            a.append(a1) #contains list of a0

    if (set(a) == set(y)):
        print("Reflexive")
        return (result, True)

    print("Not Reflexive")
    return ([] , False)

Test case:

R2 = reflexive([(1, 3), (1, 5), (3, 1), (3, 5), (7, 5)])

Not Reflexive
[]

Regardless of whether the function is actually correct or not, there are some issues that could be improved:

I would expect a function that is supposed to determine whether a relation \$R\$ over a set \$A\$ is reflexive to take two arguments:

The relation \$R\$,
and the set \$A\$.

Providing only the set \$\{(a, b) | a, b \in A, R(a,b)\}\$ and deducing \$A\$ from it is insufficient, because there could be elements in \$A\$ which are not related to any other element. This could lead to a false positive result.
I would expect the function to only return a boolean value, and to have no side effects like printing its result. Leave that to the caller.
The docstring of the function does not explain well enough what it expects as its argument, and does not explain at all what the function does.
a and y are poorly named. What is their meaning?
Instead of creating a and y a as lists to which is appended (needlessly checking if elements are already contained), only to convert them to sets at the end, why don't you just create them as sets in the first place and add to them?
Parentheses are not needed around conditions in if statements:

if a1 == a2:
if a1 not in a:
if set(a) == set(y):

(also technically not around the returned tuples)

return result, True
return [], False

This comment confuses me: What is a0?

if (a1 not in a):
    a.append(a1) #contains list of a0

You better just remove the comment.


Using Bluebird with fs to read a file within each of an (variable length) array of directories I'm reading an array (of unknown length) of modules package.json files in order to extract the "style" property and store it in an array of its own:

var gulp = require('gulp')
  , path = require('path')
  , Promise = require('bluebird')
  , fs = Promise.promisifyAll(require('fs-extra'));

let components = [
    'comp1',
    'comp2',
    'comp3'
];
let npmScope = '@myscope';
gulp.task('getstyle', () => {
    let componentsBaseLocation = path.join(__dirname, 'node_modules', npmScope);
    Promise.map(components, (componentName) => {
        return fs.readFileAsync(path.join(componentsBaseLocation, componentName, 'package.json'), 'utf8')
    })
    .spread((...packageData) => {
        return packageData.map((packageDatum, i) => {
            packageDatum = JSON.parse(packageDatum);
            tempName = packageDatum.name.match(new RegExp(`${NpmScope}\/(.*?)$`))[1]
            tempStyle = path.join(componentsBaseLocation, tempName, packageDatum.style);
            return {
                name: tempName,
                style: tempStyle
            };
        });
    })
    .then(components => {
        console.log(JSON.stringify(components, null, 2));
    });
});

This works, but I wonder if there are smarter, better, more appropriate ways to use bluebird or promises in general.

Several things you could simplify:

You don't need to use .spread() since the data is already in an array.
If you just process each file as it is read, you can avoid the .map() iteration entirely.
You can avoid some temporary variables.
You should declare all local variables with const, let or var as appropriate.

Here's modified code:

var gulp = require('gulp')
  , path = require('path')
  , Promise = require('bluebird')
  , fs = Promise.promisifyAll(require('fs-extra'));

let components = [
    'comp1',
    'comp2',
    'comp3'
];
let npmScope = '@myscope';
gulp.task('getstyle', () => {
    const componentsBaseLocation = path.join(__dirname, 'node_modules', npmScope);
    Promise.map(components, (componentName) => {
        return fs.readFileAsync(path.join(componentsBaseLocation, componentName, 'package.json'), 'utf8').then(data => {
            const packageDatum = JSON.parse(data);
            const tempName = packageDatum.name.match(new RegExp(`${NpmScope}\/(.*?)$`))[1];
            return {
                name: tempName,
                style: path.join(componentsBaseLocation, tempName, packageDatum.style)
            };
        });
    }).then(components => {
        console.log(JSON.stringify(components, null, 2));
    });
});

You may also want to handle a failure in JSON.parse() with a local try/catch handler so rather than end the entire operation, you can handle an individual error and continue with the rest of the processing, but that depends entirely upon how you're using this code and what you want to happen if a parse failure occurs.

You may also want to handle a failure in packageDatum.name.match().


K-means clustering in Python The following code uses scikit-learn to carry out K-means clustering where \$K = 4\$, on an example related to wine marketing from the book DataSmart. That book uses excel but I wanted to learn Python (including numPy and sciPy) so I implemented this example in that language (of course the K-means clustering is done by the scikit-learn package, I'm first interested in just getting the data in to my program and getting the answer out).

I'm new to Python so any advice on style or ways to write my code in a more idiomatic way would be appreciated.

The csv files needed (in the same directory as the program code) can be produced from downloading "Chapter 2" from the book link above and saving the first and second sheets of the resulting excel file as csv.

# -*- coding: utf-8 -*-
"""
A program to carry out Kmeans clustering where K=4
on data relating to wine marketing from book
"Data Smart: Using Data Science to Transform Information into Insight"

Requires csv input file OfferInfo.csv with headings
'Campaign', 'Varietal', 'Minimum Qty (kg)', 'Discount (%)', 'Origin', 'Past Peak'
and input file Transactions.csv with headings
'Customer Last Name', 'Offer #'
"""

#make more similar to Python 3
from __future__ import print_function, division, absolute_import, unicode_literals

#other stuff we need to import
import csv
import numpy as np
from sklearn.cluster import KMeans

#beginning of main program

#read in OfferInfo.csv
csvf = open('OfferInfo.csv','rU')
rows = csv.reader(csvf)
offer_sheet = [row for row in rows]
csvf.close()

#read in Transactions.csv
csvf = open('Transactions.csv','rU')
rows = csv.reader(csvf)
transaction_sheet = [row for row in rows]
csvf.close()

#first row of each spreadsheet is column headings, so we remove them
offer_sheet_data = offer_sheet[1:]
transaction_sheet_data = transaction_sheet[1:]

K=4 #four clusters
num_deals = len(offer_sheet_data) #assume listed offers are distinct

#find the sorted list of customer last names
customer_names = []
for row in transaction_sheet_data:
    customer_names.append(row[0])
customer_names = list(set(customer_names))
customer_names.sort()
num_customers = len(customer_names)

#create a num_deals x num_customers matrix of which customer took which deal
deal_customer_matrix = np.zeros((num_deals,num_customers))
for row in transaction_sheet_data:
    cust_number = customer_names.index(row[0])
    deal_number = int(row[1])
    deal_customer_matrix[deal_number-1,cust_number] = 1
customer_deal_matrix = deal_customer_matrix.transpose()

#initialize and carry out clustering
km = KMeans(n_clusters = K)
km.fit(customer_deal_matrix)

#find center of clusters
centers = km.cluster_centers_
centers[centers<0] = 0 #the minimization function may find very small negative numbers, we threshold them to 0
centers = centers.round(2)
print('\n--------Centers of the four different clusters--------')
print('Deal\t Cent1\t Cent2\t Cent3\t Cent4')
for i in range(num_deals):
    print(i+1,'\t',centers[0,i],'\t',centers[1,i],'\t',centers[2,i],'\t',centers[3,i])

#find which cluster each customer is in
prediction = km.predict(customer_deal_matrix)
print('\n--------Which cluster each customer is in--------')
print('{:<15}\t{}'.format('Customer','Cluster'))
for i in range(len(prediction)):
    print('{:<15}\t{}'.format(customer_names[i],prediction[i]+1))

#determine which deals are most often in each cluster
deal_cluster_matrix = np.zeros((num_deals,K),dtype=np.int)
print('\n-----How many of each deal involve a customer in each cluster-----')
print('Deal\t Clust1\t Clust2\t Clust3\t Clust4')
for i in range(deal_number):
    for j in range(cust_number):
        if deal_customer_matrix[i,j] == 1:
            deal_cluster_matrix[i,prediction[j]] += 1

for i in range(deal_number):
    print(i+1,'\t',end='')
    for j in range(K):
        print(deal_cluster_matrix[i,j],'\t',end='')
    print()
print()

print('The total distance of the solution found is',sum((km.transform(customer_deal_matrix)).min(axis=1)))

One obvious improvement would be to break the code up a bit more - identify standalone pieces of functionality and put them into functions, e.g.:

def read_data(filename):
    csvf = open(filename,'rU')
    rows = csv.reader(csvf)
    data = [row for row in rows]
    csvf.close()
    return data

offer_sheet = read_data('OfferInfo.csv')
transaction_sheet = read_data('Transactions.csv')

This reduces duplication and, therefore, possibilities for errors. It allows easier development, as you can create and test each function separately before connecting it all together. It also makes it easier to improve the functionality, in this case by adopting the with context manager:

def read_data(filename):
    with open(filename, 'rU') as csvf:
         return [row for row in csv.reader(csvf)]

You make that change in only one place and everywhere that calls it benefits.

I would also have as little code as possible at the top level. Instead, move it inside an enclosing function, and only call that function if we're running the file directly:

def analyse(offer_file, transaction_file):
    offer_sheet = read_data(offer_file)
    transaction_sheet = read_data(transaction_file)
    ...

if __name__ == "__main__":
    analyse('OfferInfo.csv', 'Transactions.csv')

This makes it easier to import the code you develop elsewhere without running the test/demo code.


`std::move_as_tuple` and `std::when_all` Working with futures and .then, I keep running into the problem of needing to "capture by move" in a lambda. In fact, not only that, but when implementing e.g. when_all, I find myself needing to capture a whole parameter-pack. Neither of these things are supported out of the box in C++17.

However, we can capture a whole parameter pack "by forward", using std::forward_as_tuple and std::apply. So I decided to write a std::move_as_tuple that would do the same thing as forward_as_tuple but for moving instead of forwarding.

How does this code look? And is there a simpler way to accomplish my goal?

template<class... Types>
constexpr auto move_as_tuple(Types&&... args)
{
    return std::forward_as_tuple(std::move(args)...);
}

Sample usage (which is also fair game for critique):

future<std::tuple<>> when_all()
{
    promise<std::tuple<>> p;
    p.set_value({});
    return p.get_future();
}

template<class F, class... Rest>
future<std::tuple<F, Rest...>> when_all(F f, Rest... rest)
{
    return f.then([rest = move_as_tuple(rest...)](auto f) {
        auto w = [](auto&&... as) { return when_all(std::forward<decltype(as)>(as)...); };
        return std::apply(w, rest).then([f = std::move(f)](auto future_of_tuple_of_rest) {
            return std::tuple_cat(std::make_tuple(std::move(f)), future_of_tuple_of_rest.get());
        }).get();
    });
}

(Self-answering for posterity.)

There's a major flaw in my above implementation of when_all: It calls .get()! This can cause the thread to block indefinitely. We need to eliminate that .get somehow, or at least make sure that we only ever call .get on a future that has just been passed in as the argument of a .then callback.

The reason I was calling .get was to work around a type incompatibility: the type of

auto SUBEXPRESSION =
    std::apply(w, rest).then([f = std::move(f)](auto future_of_tuple_of_rest) {
        return std::tuple_cat(std::make_tuple(std::move(f)), future_of_tuple_of_rest.get());
    })

is std::future<std::tuple<F, Rest...>>, which means that the type of

auto EXPRESSION =
    f.then([...](auto f) {
        return SUBEXPRESSION;
    });
});

is std::future<decltype(SUBEXPRESSION)>, which is to say std::future<std::future<std::tuple<F, Rest...>>>. That's too many futures! So I was trying to work around this by calling .get right before returning the "inner" future.

Turns out that boost::future and std::experimental::future (in the Concurrency TS) already have a solution for this problem. It's called .unwrap, and the idea is that if decltype(f) is future<future<T>>, then decltype(f.unwrap()) is future<T>.

In Boost's case, this is currently provided as a constructor explicit future(future<future>&&) instead of as an .unwrap member function, but there's a bug open to provide .unwrap directly.

In the Concurrency TS, .then also implicitly calls .unwrap on the callback's return value if that return value is seen to be of type future<future<T>>.

So my solution was simply to add a call to .unwrap (well, the Boost explicit constructor), and then everything was happy.

There's another showstopping problem with the above code: it doesn't compile for more than one argument, because I'm missing the mutable keyword on both of the lambdas involved. You can't "move out of" the captured f unless f is non-const, which means you need mutable on the inner lambda; and at least GCC is unhappy with std::apply(w, rest) unless rest is non-const, which means you need mutable on the outer lambda too.

Finally, I decided to stop trying to capture references to things, and just replaced everything with capture-by-value. This simplified the code to the point where I didn't see any reason to keep move_as_tuple as its own function at all.

My final code:

auto when_all() {
    promise<tuple<>> p;
    p.set_value({});
    return p.get_future();
}

template<class F, class... Rest>
auto when_all(F f, Rest... rest) {
    return future<tuple<F, Rest...>>(
        f.then([rest = make_tuple(std::move(rest)...)](auto f) mutable {
            auto w = [](auto... as) { return when_all(std::move(as)...); };
            return apply(w, std::move(rest)).then(
                [f = std::move(f)](auto future_of_tuple_of_rest) mutable {
                    return tuple_cat(make_tuple(std::move(f)), future_of_tuple_of_rest.get());
                }
            );
        })
    );
}


Flexible generic event system that binds to objects I have been trying to create an event queue system using SDL. I've been trying to use this for Pong. The game is not even finished yet, but I stopped work to examine how my event system is organized.

Event_Queue - This is the heart of the system. All events go through here and all listeners come through here.
Event - This holds a name string, a type string, and a Boost.Any object to be able to package any type of data along with it. Boost.Any will act as a wrapper for another general data structure, but in the case that it isn't general enough, another can be easily adopted.
Event_Receivable - Interface between Event_Receiver and object; I think it's unclean but I don't know of a better way than inheritance to do this.
Event_Receiver - The object that is tacked onto an object to be able to receive objects. It is also a type of interface--logic will be handled within the object, preferably through a Logic module, and is intended to be hooked up to a scripting API later.
Event_Sender - This is supposed to send events onto the Event_Queue. This will probably be improved so that I can just use std::move or something, but this is supposed to always be the outlet from where Events are fired.

Does this constitute a complete event system? Are there any flaws besides the glaring ignorance regarding copy, references, and move?

Event_Queue.h:

class Event_Queue
{
private:

    SDL_Event input;

    std::queue<Event> event_queue;
    std::vector<Event_Receiver*> receiver_list;
public:
    void add_receiver(Event_Receiver* receiver_ptr);
    bool remove_receiver(Event_Receiver* receiver_ptr);

    void push_event(Event e);

    void poll_input();
    void poll_event();

};

Event_Receivable.h:

class Event_Receivable
{
public:
    virtual ~Event_Receivable() = 0;
    virtual void on_notify(Event* e) = 0;

};

Event_Receiver.h:

class Event_Receiver
{
public:
    Event_Receiver(Event_Receivable* owner_obj);
    ~Event_Receiver();

    void receive_event(Event* e);
    void notify_owner(Event* e);
    void set_owner(Event_Receivable* owner_obj) {owner = owner_obj;}

    Event_Receivable* get_owner_obj() {return owner;}

private:
    Event_Receivable* owner;

};

Event.h:

#include "Common.h"
#include "boost/any.hpp"

/*
 * Event is used extensively within the program.
 * It is the basis for all messages in the event system.
 * Objects will send and receive events.
 *
 * It uses boost::any in order to be able to
 * attach any kind of data to be read.
 *
 * Event is meant to be extremely flexible in order to
 * be extensible to many different types of games.
 */
class Event

    {
    private:
        std::string name;
        std::string type;
        std::vector<boost::any> info;
    public:
        Event(std::string name_str, std::string type_str)
        {
            name = name_str;
            type = type_str;
        }

        void set_name(std::string str) {name = str;}
        void set_type(std::string str) {type = str;}
        void attach_data(boost::any data) {info.push_back(data);}

        std::string get_name() {return name;}
        std::string get_type() {return type;}
        boost::any get_info() {return info;}
    };

Event_Sender.h:

class Event_Sender
{
public:
    void send_event(Event e);
    void send_event(Event e, Event_Queue q);

    void set_event_queue(Event_Queue e_queue) {event_queue = &e_queue;}

    Event_Queue* get_event_queue() {return event_queue;}

private:
    Event_Queue* event_queue;
};

GitHub

This code already does compile correctly with dependencies sorted out, but I need feedback on the system.

It looks good to me. The Event_Receivable thing looks like a vanilla observer pattern, which involves inheritance. I don't really have comments on the architecture, but I recommend a look at this Game Programming Patterns chapter if you haven't already.

Code review

A few considerations on the design:

The first thing that jumps to the eye here is the heavy use of raw pointers. The event queue, for instance, has a vector of Event_Receiver pointers which it does not own. It would be a disaster if one of those objects were to be deleted while the pointer is still in the queue's vector. Event_Receiver also keeps a raw pointer to its owner. I think you should take a step back and rethink this. It can save you a lot of headache on pointer debugging in the future, not to mention the potential for memory leaks, which can be a serious problem in a game. If you can use C++11, then consider the standard smart pointers. It seems that a combination of shared_ptrs and weak_ptrs would be suitable here.
You already know this, but when you do decide how to handle copy/assignment, take a look at the Rule of Three. The safest course of action until then would be to make the types that hold naked pointers, or arrays of pointers, non-copyable, by deleting the copy constructor and assignment operator. That might actually suit your needs and it is very easy to do.
You are using strings to define the event type and name/id; not sure if this is a good approach. The only reason I would chose to represent those as strings would be if I had the need to create event types dynamically during runtime. The two main issues with using strings are 1) performance: it is much faster comparing an integer/enum-constant than it is comparing a string; 2) when declaring/referencing an event in the code, if you mistype a string, you will only find out about the error after a little runtime debugging. That problem goes away with compile-time constants. Your choice should really be based on how flexible you need the system to be, then choose your trade-off.

Miscellaneous:

Your get* methods are missing const at the end, which would make it impossible to call one of them on a const object. Since they don't mutate member data, make them const correct. E.g.:

boost::any get_info() const { return info; }
                      ^^^^^

Still talking about the getters, I'm not sure how costly it is to copy, for instance, a boost::any, but if the caller will mostly just be inspecting the value, then consider returning complex objects by const reference. E.g.:

const boost::any & get_info() const { return info; }
^^^^^            ^            ^^^^^

Looking in the repository link, I see that you have some very long-winded for loops in Event_Queue, such as this one from Event_Queue::poll_input():

for(std::vector<Event_Receiver*>::iterator it = receiver_list.begin(); it != receiver_list.end(); ++it)

Such huge line can be made a lot simpler by using a range-based for loop (C++11), or at least the use of auto for the iterators.
Event_Sender seems to be missing a constructor, which would leave its member pointer uninitialized until it is set somewhere else. This is very dangerous and can be a source of bugs. Always initialize your variables to a known value. Don't leave room for chance.
Looking at the implementation of Event_Receiver, the destructor is empty, so you can just remove it and let the compiler supply a default.
You have a mixed style of public/private class sections. Some have private at the start, others have it at the end. I'd suggest that you make it uniform through all classes. My personal preference is to place the public section of a class first in the header file. The rationale is that people using and reading my code will care much more about the public methods and data, so that's what they want to see first when reading the header file. Private/protected sections of a class are of interest to the programmer maintaining the code, so it makes sense that they don't need that much visibility.
Use the constructor initializer list to init member data. Using the assignment operator might result in redundant initialization if the compiler is not able to optimize the code (calls the default constructor of a type + operator =).

Event(std::string name_str, std::string type_str)
    : name(std::move(name_str))
    , type(std::move(type_str))
{ }

That's also a perfect place for a move, to avoid copying things over and over. Without move the best would have been taking name_str and type_str by const reference.


Password maker in C# I need to generate semi-complex throw-able passwords (just one use) in my application. I want it to be:

at least 8 characters long;
contains at least 1 digit, 1 [a-z] char, 1 [A-Z] char;

I generate a few passwords a day, and I don't mind about speed. I'm more looking for suggestions about code being maintainable (or not) and possible huge security flaws. Here's the code:

public static string GenerateSimplePassword(int length = 8)
{
    length = Math.Max(length, 8);

    var allowedLower = new[] { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'j', 'k', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z' };
    var allowedDigits = new[] { '1', '2', '3', '4', '5', '6', '7', '8', '9' };
    var allowedUpper = allowedLower.Select(x => char.ToUpper(x)).ToArray();

    var r = new Random();
    var generatedCode = "";
    generatedCode += allowedLower[r.Next(0, allowedLower.Length)];

    generatedCode += allowedDigits[r.Next(0, allowedDigits.Length)];

    generatedCode += allowedUpper[r.Next(0, allowedUpper.Length)];

    var arrays = new[] { allowedLower, allowedUpper, allowedDigits };
    for (var i = 0; i < length - 3; i++)
    {
        var array = arrays[r.Next(0, arrays.Length)];
        generatedCode += array[r.Next(0, array.Length)];
    }

    return String.Join("", generatedCode.OrderBy(x => r.Next()));
}

Fully Refactored Code: see this Gist

Example Usage:

static void Main(string[] args) {
    var generator = new PasswordGenerator(new PasswordGeneratorOptions {
        MinimumNumberOfLowerCaseCharacters = 1,
        MinimumNumberOfNumericCharacters = 1,
        MinimumNumberOfUpperCaseCharacters = 1,
        OutputLength = 8,
        SpecialCharacters = new char[0] { },
    });

    Console.WriteLine(generator.Next());
}

Review Comments:

First off, using Random as a source of entropy probably isn't the best idea since it is possible (however unlikely) that an attacker could gain the underlying seed; with it they would trivially be able to deduce all of the outputs used to generate our passwords. Let's implement a generator that uses a static instance of the RNGCryptoServiceProvider class instead:

// makes it easier to replace the implementation I demonstrate with something better
public interface ISecureRandom
{
    uint Next(uint x, uint y);
}

// a possible implementation of a secure RNG, not exhaustively tested...
public sealed class SecureRandom : ISecureRandom
{
    private static readonly RandomNumberGenerator DefaultRandomNumberGenerator = new RNGCryptoServiceProvider();

    public static SecureRandom DefaultInstance => new SecureRandom(DefaultRandomNumberGenerator);

    private readonly RandomNumberGenerator m_randomNumberGenerator;

    public SecureRandom(RandomNumberGenerator randomNumberGenerator) {
        if (null == randomNumberGenerator) {
            throw new ArgumentNullException(paramName: nameof(randomNumberGenerator));
        }

        m_randomNumberGenerator = randomNumberGenerator;
    }

    public byte[] GetBytes(byte[] buffer) {
        m_randomNumberGenerator.GetBytes(buffer);

        return buffer;
    }
    public byte[] GetBytes(int count) => GetBytes(new byte[count]);
    public uint Next() => BitConverter.ToUInt32(GetBytes(sizeof(uint)), 0);
    public uint Next(uint x, uint y) {
        if (x > y) {
            var z = x;

            x = y;
            y = z;
        }

        var range = (y - x);

        if (range == 0) {
            return x;
        }
        else if (range == uint.MaxValue) {
            return Next();
        }
        else {
            return (Next(exclusiveHigh: range) + x);
        }
    }

    private uint Next(uint exclusiveHigh) {
        var range = (uint.MaxValue - (((uint.MaxValue % exclusiveHigh) + 1) % exclusiveHigh));
        var result = 0U;

        do {
            result = Next();
        } while (result > range);

        return (result % exclusiveHigh);
    }
}

Regarding maintainability, we could start by creating a proper class to encapsulate all of our settings:

public sealed class PasswordGeneratorOptions
{
    private static readonly char[] DefaultSpecialChars = new[] { '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '-', '=', '`', '~', '_', '+', ',', '.', '\'', '"', ';', ':', '?', '|', '/', '\\', '[', ']', '{', '}', '<', '>' };

    public int MinimumNumberOfNumericCharacters { get; set; }
    public int MinimumNumberOfLowerCaseCharacters { get; set; }
    public int MinimumNumberOfSpecialCharacters { get; set; }
    public int MinimumNumberOfUpperCaseCharacters { get; set; }
    public int OutputLength { get; set; }
    public ISecureRandom RandomNumberGenerator { get; set; }
    public IReadOnlyList<char> SpecialCharacters { get; set; }

    public PasswordGeneratorOptions() {
        MinimumNumberOfLowerCaseCharacters = 0;
        MinimumNumberOfNumericCharacters = 0;
        MinimumNumberOfSpecialCharacters = 0;
        MinimumNumberOfUpperCaseCharacters = 0;
        RandomNumberGenerator = SecureRandom.DefaultInstance;
        SpecialCharacters = DefaultSpecialChars;
    }
}

Then we can declare a PasswordGenerator class that consumes our options:

public sealed class PasswordGenerator
{
    private readonly PasswordGeneratorOptions m_options;

    public PasswordGenerator(PasswordGeneratorOptions options) {
        if (options.OutputLength < (options.MinimumNumberOfLowerCaseCharacters + options.MinimumNumberOfNumericCharacters + options.MinimumNumberOfSpecialCharacters + options.MinimumNumberOfUpperCaseCharacters)) {
            throw new ArgumentOutOfRangeException(message: "output length must be greater than or equal to the sum of all MinimumNumber* properties", actualValue: options.OutputLength, paramName: nameof(options.OutputLength));
        }

        m_options = options;
    }
}

Now we can start thinking about implementation details, I personally like the idea of breaking things up into various categories so that we can provide more complex configuration options later. Let's try classifying your characters into lower-case, upper-case, number, and special. The ASCII encoding has largely taken care of all of this for us "for free" since three of these four classes have contiguous regions in the specification; we can write a simple generator for each like so:

var randomNumberGenerator = new SecureRandom();

Func<char> GetAsciiLetterLowerCase = () => ((char)randomNumberGenerator.Next(97, 123));
Func<char> GetAsciiLetterUpperCase = () => ((char)randomNumberGenerator.Next(65, 91));
Func<char> GetAsciiNumber = () => ((char)randomNumberGenerator.Next(48, 58));

Special characters aren't as easy so we'll resort to using a table of values:

var specialCharacters = new char[] { '!', '@', '#', };

Func<char> GetAsciiSpecial = () => specialCharacters[randomNumberGenerator.Next(0U, ((uint)specialCharacters.Count))];

The only remaining thing left to do is implement the generator function. This can be accomplished by more or less following the same strategy you already have in place. I chose to implement Fisher–Yates shuffle instead of using LINQ for the final randomization step:

public string Next() {
    var index = 0;
    var length = m_options.OutputLength;
    var randomNumberGenerator = m_options.RandomNumberGenerator;
    var result = new char[length];
    var useSpecial = (0 < m_options.SpecialCharacters.Count);

    for (var i = 0; (i < m_options.MinimumNumberOfLowerCaseCharacters); i++) {
        result[index++] = m_getAsciiLetterLowerCase();
    }

    for (var i = 0; (i < m_options.MinimumNumberOfNumericCharacters); i++) {
        result[index++] = m_getAsciiNumeric();
    }

    for (var i = 0; (i < m_options.MinimumNumberOfSpecialCharacters); i++) {
        result[index++] = m_getAsciiSpecial();
    }

    for (var i = 0; (i < m_options.MinimumNumberOfUpperCaseCharacters); i++) {
        result[index++] = m_getAsciiLetterUpperCase();
    }

    for (var i = index; (i < length); i++) {
        char c;

        switch (randomNumberGenerator.Next(0U, (useSpecial ? 4U : 3U))) {
            case 3U:
                c = m_getAsciiSpecial();
                break;
            case 2U:
                c = m_getAsciiNumeric();
                break;
            case 1U:
                c = m_getAsciiLetterLowerCase();
                break;
            case 0U:
                c = m_getAsciiLetterUpperCase();
                break;
            default:
                throw new InvalidOperationException();
        }

        result[i] = c;
    }

    FisherYatesShuffle(randomNumberGenerator, result);

    return new string(result);
}

private static void SwapRandom<T>(ISecureRandom randomNumberGenerator, IList<T> list, uint indexLowerBound, uint indexUpperBound) {
    var randomIndex = randomNumberGenerator.Next(indexLowerBound, indexUpperBound);
    var tempValue = list[(int)randomIndex];

    list[(int)randomIndex] = list[(int)indexUpperBound];
    list[(int)indexUpperBound] = tempValue;
}
private static void FisherYatesShuffle<T>(ISecureRandom randomNumberGenerator, IList<T> list) {
    var length = list.Count;
    var offset = 0U;

    while (offset < length) {
        SwapRandom(randomNumberGenerator, list, 0U, offset++);
    }
}


Modify XML attribute based on element value I have some xml files that have some nodes like (among other nodes)

<disp-formula id="deqn*">
...\tag{1}
</disp-formula>
<disp-formula id="deqn*">
...\tag{2}
</disp-formula>
<disp-formula id="deqnxyz">
...\tag{3}
...
...\tag{4}
...\tag{5}...
...
......\tag{6}
</disp-formula>

I'm trying to get the values inside the string/strings \tag{} which is inside the nodes  with an attribute named id and modify it with the value/values inside its respective tag{} i.e. the output should be

<disp-formula id="deqn1">
...\tag{1}
</disp-formula>
<disp-formula id="deqn2">
...\tag{2}
</disp-formula>
<disp-formula id="deqn3-6">
...\tag{3}
...
...\tag{4}
...\tag{5}...
...
......\tag{6}
</disp-formula>

I've done

//for nodes containing single \tag{}
Regex regex = new Regex(@"(?<=\\tag\{)(\w+)(?=\})");
var xml = File.ReadAllText(@"D:\Test\sample.xml");
var xdoc = Xdocument.Parse(xml, LoadOptions.PreserveWhitespace);
var _descriptions = xdoc.Descendants("disp-formula")
    .Where(x => regex.Match(x.Value).Success);
foreach (var description in _descriptions)
{
    var _Result = regex.Match(description.Value).Value;
    description.Attribute("id").Value = "deqn" + _Result;
    xdoc.Save(@"D:\Test\sample.xml", SaveOptions.DisableFormatting);
}
//for nodes containing multiple \tag{}'s
var descriptions = xdoc.Descendants("disp-formula")
    .Where(x => regex.Matches(x.Value).Count > 1);
foreach (var description in descriptions)
{
    var p = regex.Matches(description.Value).Cast<Match>().Select(m => m.Value).ToArray();
    var x = p[0];
    var y = p[p.Count() - 1];
    var Result = x + "-" + y;
    description.Attribute("id").Value = "deqn" + Result;
    xdoc.Save(@"D:\Test\sample.xml", SaveOptions.DisableFormatting);
}

How do I make this code more efficient?

Performance

The most obvious improvement would be to only save the file once, after you've made all the necessary changes.
You're also doing a lot of duplicate regex matching. The Where calls do matching, and then the foreach loops have to look for the same matches again. There's also some overlap between the single and multiple match logic

All of that can be simplified into a single foreach loop:

// read file content

foreach (var formulaNode in xdoc.Descendants("disp-formula"))
{
    var matches = regex.Matches(formulaNode.Value);
    if (matches.Count == 0)
        continue;

    var id = "deqn" + matches[0].Value;
    if (matches.Count > 1)
        id += "-" + matches[matches.Count - 1].Value;

    formulaNode.Attribute("id").Value = id;
}

// save file

Do you have a specific reason for using a lookbehind and lookahead in your regex? \\tag\{(\w+)\} will be faster. The match value no longer contains just the tag ID, but because you're using a capture group it's still easy to obtain that ID: match.Groups[1].Value.
Instead of using Count(), you can use an array's Length property directly. Count() is a Linq method, and for arrays it'll just return their Length, but it does have to do some type checking, so it takes a tiny bit of extra work.

Code quality

The filename is duplicated several times. Duplication makes code harder to maintain. Store the filename in a variable, or better: make it a method parameter.
Variable naming is a little inconsistent (camelCase, _leadingUnderscore, _PascalCase). camelCase is normally used for parameters and local variables, PascalCase for type, property and method names. Some people use leading underscores for private fields, others don't - but whatever approach you pick, being consistent will make your code easier to read and understand.
Some variable names are rather undescriptive: p, x, y. Something like tagIDs, firstTagID and lastTagID would make the code easier to understand.

Other

The multi-match logic only looks at the first and last tag IDs. Does your input always contain sequential tag IDs, or could they come in a different order or contain 'gaps'? You may want to document this assumption - if you ever need to modify this code later on then at least you'll know what you were thinking back then.


Log4net helper class for unit tests The class helps to determine the exact output log message, taking in account the conversionPattern that log4net uses for layout. Current usage is you have to attach the listener, then call your BL or a wrapper method which your code uses to write to log, then call the GetRenderedMessage() method. The purpose is to make sure that even if I modify my wrapper method (I have a wrapper method which adds some properties to log4net's ThreadContext.Properties and also formats the message in a certain way) the rendered log would remain the same. Are there any improvements or suggestions to make it more "friendly" so someone would know how to use it without reading this intro?

using log4net;
using log4net.Appender;
using log4net.Config;
using System;
using System.IO;
using System.Linq;

namespace TestUtils
{
    public class Log4netListener
    {
        MemoryAppender memAppender;

        /// <summary>
        /// Attaches the listener so it would capture any log messages written.
        /// </summary>
        /// <param name="appenderName"></param>
        public void AttachLogListener(string appenderName)
        {
            var appender = LogManager.GetRepository().GetAppenders().Where(a => a.Name == appenderName);
            var patternLayout = ((log4net.Layout.PatternLayout)((AppenderSkeleton)(appender.First())).Layout);
            memAppender = new MemoryAppender();
            memAppender.Layout = patternLayout;
            BasicConfigurator.Configure(memAppender);
        }

        /// <summary>
        /// Gets the message written to log. Use this method only after AttachLogListener() was called
        /// and a message was written to log4net afterwards.
        /// </summary>
        public string GetRenderedMessage()
        {
            if (memAppender == null)
            {
                throw new InvalidOperationException("The listener was not attached. Make sure you call AttachLogListener() before writing to log.");
            }
            var events = memAppender.GetEvents();
            string logMessage = "";
            using (StringWriter writer = new StringWriter())
            {
                foreach (var ev in events)
                {
                    memAppender.Layout.Format(writer, ev);
                }
                logMessage = writer.ToString();
            }
            return logMessage;
        }
    }
}

Usage:

[TestClass]
public class LoggingTests
{
    [AssemblyInitialize]
    public static void Configure(TestContext tc)
    {
        XmlConfigurator.Configure(); // must use this line to make log4net work from test  if "configSource" is used in app.config
    }

    [TestMethod]
    public void LogLayout()
    {
        var listener = new Log4netListener();
        listener.AttachLogListener("MyLogFileAppender");

        // Call business logic here that writes to log...

        string logMessage = listener.GetRenderedMessage();
        // Assert...
    }
}

Are there any improvements or suggestions to make it more "friendly" so someone would know how to use it without reading this intro?

The most obvious thing is high-quality XML docs. You've already started, but you need to fill in the param name where it's blank, and the return value for the method where it's missing.

On top of that, adding a summary tag to the class itself containing most of the verbiage above would be helpful.

Reading a little further into your implementation - is it possible to move the code of the AttachLogListener method into a constructor? That would guarantee your order of execution/initialization and make usage more sane and obvious, I think.


Tag manipulation using DOMDocument On our main symfony2 application at work I have had to manipulate some outputted grid fields by wrapping them in a hyperlink tag.  This happens in quite a few different controllers, so I made a method to do this to prevent code repetition.

I decided to do this using DOMDocument and construct the new hyperlink with that.

public function linkWrap($string, $route, array $route_params = [], $prefilter = null)
  {
    if(strlen($string) > 0)
    {
      $dom = new \DOMDocument();
      $new_hyper = $dom->createElement('a');
      $new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.htmlspecialchars($prefilter) : ''));
      $new_hyper->nodeValue = $string;

      return $dom->saveHTML($new_hyper);
    }

    return $string;
  }

The method takes a number of options the last of which is used only to append a prefilter query string for use with APYGridBundle.

My question is, does this cause a significantly larger overhead than just constructing the link in a more 'conventional' way?

As follows:

$link = "generateUrl('a_route', array('id' => 23)) . '?'.$prefilter">some text";

In addition, any tips on how any of could be improved would be appreciated.

Creating links using DOMDocument makes no sense to me.

I have created a simple benchmark for both methods.

Simple string concatenation is around 7 times faster on my computer:

Method Name          Iterations    Average Time      Ops/second
------------------  ------------  --------------    -------------
simpleLink        : [10,000    ] [0.0000013786793] [725,331.85764]
linkUseDomDocument: [10,000    ] [0.0000098103762] [101,932.89054]

You could reuse objects in (for example) static variables like this:

public function linkWrap2($string, $route, array $route_params = [], $prefilter = null)
{
    static $dom = null, $new_hyper = null;

    if(strlen($string) > 0)
    {
        if ($dom === null) {
            $dom = new \DOMDocument();
            $new_hyper = $dom->createElement('a');
        }

        $new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.htmlspecialchars($prefilter) : ''));
        $new_hyper->nodeValue = $string;

        return $dom->saveHTML($new_hyper);
    }

    return $string;
}

but it's only 30% faster.

My observations

Don't use htmlspecialchars in setAttribute value, because it will be encoded twice:

$new_hyper->setAttribute('href', $this->generateUrl($route, $route_params).(isset($prefilter) ? '?'.($prefilter) : ''));

However, you must use htmlspecialchars in simple concatenated strings:

$link = "generateUrl('a_route', array('id' => 23)) . '?'.htmlspecialchars($prefilter).'">some text';

Try to avoid custom query strings after a generated route URL. Instead of place value of $prefilter after the URL, you could place additional route params:

$this->generateUrl($route, array_merge($route_params, [ 'prefilter' => $prefilter ]));


Health Test App, It Returns a Score Based on the Answers Given Here is my parent component: App.vue. I was wondering what improvements I could make to the code here.

Application Description

  This application was built off the Vue.js framework and utilizes a
  Laravel REST API I created to fetch custom data. It stores and tracks
  your ‘health score’, which is determined by the answers you submit in
  the test. Each answer has a value. So, the unhealthiest answer has a
  value of 0, the second-least healthiest 1, second-most healthiest 2,
  and the healthiest answer 3. The values of the answers you submit are
  aggregated throughout the test via local storage. After the final
  question has been answered, your total health score is returned. Your
  final score will determine the message you receive.

<template>

      <router-view
        v-if="testData.length"
        :currentTestData = testData[testDataIndex]
        :testDataIndex = testDataIndex
        :nextAndResetIndices="nextAndResetIndices"
        :selectAnswer="selectAnswer"
        :selectedIndex="selectedIndex"
        :submitAnswer="submitAnswer"
        :submittedIndex="submittedIndex"
        :healthScore="healthScore"
        :setStorage="setStorage"
        :getStorage="getStorage"
        :resetStorage="resetStorage"
        :message="message"
        :returnMessage="returnMessage"

      />

</template>

If you wish to keep your implementation scheme, I only have one large critique:

Names are important

Your naming scheme should allow a 3rd party to understand your code from a cursory glance. You have a lot of sneaky implementations tucked into your functions that aren't expressed in your naming convention.

All of your operations are stateful, which is technically ok but you use a lot of verbage that makes it sound like they're functional operations with actual return values. All of your functions act on stateful variables. Your naming convention should reflect this through the use of verb phrases, as opposed to nouns.

resetStorage: A good variable name. It is a verb that acts on localStorage

getStorage: Misleading. The traditional usage of the verb get is to retrieve a value or perform a computation (and return it). You are assigning this.healthScore. getStorage doesn't even have a return value. A more apt name would be something more like setHealthScoreFromLocalStorage.

toZero: What exactly is being set to zero? From the rest of your code (looking at resetStorage) I can infer that the value of 0 is the default value. It should be more along the lines of resetHealthScore or clearHealthScore.

returnMessage: It is updating this.message. So something more like updateHealthMessage?


Presenting collections for code generation I've never been too bothered by the lack of present (and thus print) support for Factor collections and other objects, but I've never needed to generate Factor programs from JSON until now.

To that end, I use these methods to turn arrays, hashtables and assocs into roundtrippable strings:

M: f present
  drop "f" ;

M: array present
  [
    [
      dup string? [
        "\"" dup surround
      ] when
      present
    ] map
  ]
  [ length ]
  bi
  [ "%s " ] replicate "" join
  vsprintf "{ " " }"  surround ;

M: assoc present
  { } assoc-clone-like present ;

M: hashtable present
  { } assoc-clone-like present "H" prepend ;

The array method was written rather haphazardly as I tried to find something that seemed robust. The way the elements are finally formatted, using replicate and vsprintf may seem sloppy but it looks better than an until, and sprintf can't be used on non-constant values.

The recursive nature allows it to be cleverly repurposed for other collection types and on arbitrarily deeply nested arrays. Moreover, string literals in arrays preserve their "quotes" so they roundtrip too, which I initially struggled with.

But, can it be improved? The goal is that any collection converts into a string and evals back into its original value (called roundtripping), which it does:

IN: scratchpad { 1 { 2 3 } "4" 5 } present ( -- x ) eval

--- Data stack:
{ 1 ~array~ "4" 5 }
IN: scratchpad H{ { 1 "a" } { 2 3 } { "4" 5 } } present ( -- x ) eval

--- Data stack:
{ 1 ~array~ "4" 5 }
H{ { 1 "a" } { 2 3 } { "4" 5 } }

can it be improved?

Looks good to me.

Ship it.

The program accomplishes its goals, and it is clear. The length seems about right for what it does.


SQL to match string value against multiple columns I have a table called tblStudent having data as

I want to retrieve records having single value compared against multiple columns. For example -
find all students having Grade 'C' in any of the subjects.
One way could be -

SELECT * FROM tblStudent
WHERE Grade_Maths = 'C' OR
Grade_Science = 'C' OR
Grade_English = 'C'

Another (cleaner/shorter) way of achieving same is

SELECT * FROM tblStudent
WHERE 'C' IN (Grade_Maths, Grade_Science, Grade_English)

Because the table data can be considerably huge in real-time, I am not sure whether second method will always return correct result.
Can please suggest if there could be any potential issue with using second approach.

Thanks a lot!

The queries are the same except potentially the second won't use index if any exists on grade_* columns. And this flipped approach Is rather uncommon (at least i see it for first time). Possibly because you wouldnt need such approach if you had a better tables structure.

What if suddently students can study history or biology? You Will need to create new columns And Alter all related queries.

Instead there should be a "subjects" table and an M:N relation table between subjects and students. Your query wont contain any ORs or INs the way it does now. And adding new subjects Is matter of  merely inserting new row to subjects table.


Refactor two methods with similar logic but handling different entities We have three methods, the first calls the second one. How would you refactor so the third one can call the second one too as they have similar code?
public async Task UnpublishPersonAsync(string id)
{
    await QueueAndUpdateStatusAsync(id, PersonStatus.Unpublishing, QueueNames.Unpublish);
}

private async Task QueueAndUpdateStatusAsync(string id, PersonStatus status, string queueName)
{
    var personDto = await _cosmosDbService.GetItemAsync<PersonDto>(id, id);
    var operationIsValid = ValidateOperation(status, personDto);

    if (operationIsValid)
    {
        personDto.Status = status;
        personDto = await _cosmosDbService.UpdateItemAsync(id, id, personDto);
        var person = _mappingService.MapToPerson(personDto);

        await _queue.PublishToStorageQueueAsync(person, queueName);
    }
    else
    {
        throw new InvalidOperationException();
    }
}

public async Task DeletePersonAsync(string id, bool deleteAssociatedData)
{
    var personDto = await _cosmosDbService.GetItemAsync<PersonDto>(id, id);
    var operationIsValid = ValidateOperation(PersonStatus.Deleting, personDto);

    if (operationIsValid)
    {
        personDto.Status = PersonStatus.Deleting;
        personDto = await _cosmosDbService.UpdateItemAsync(id, id, personDto);
        var person = _mappingService.MapToPerson(personDto);

        var deleteCommand = new DeletePersonCommand()
        {
            Person = person,
            DeleteAssociatedData = deleteAssociatedData
        };

        await _queue.PublishToStorageQueueAsync(deleteCommand, QueueNames.Delete);
    }
    else
    {
        throw new InvalidOperationException();
    }
}

I thought about using generics and make it optional, but it seems that's not possible. By the way, PublishToStorageQueueAsync is already generic for the first argument.

Disclaimer

From the comments:

the problem is that I don't have enough experience nor knowledge to improve it

I will stop you right there. If you don't have enough experience nor knowledge then I suggest you don't do refactoring on this specific code, not because you can't but because you endanger yourself of making undesired behaviour or making actually harder to read code.
The sole purpose of refactoring is making a code easier to understand. Now this can be tricky because sometimes some sections of your application have to be very performant, other sections need to be very readable as most of the business logic is there, some parts need to be very flexible since they are changing often there are many cases. The key to refactoring is first realizing what are you refactoring for - performance (very hard, if you don't have experience I would suggest not to do it), readability, extensibility and so on.
Back to your case

Playing with tasks if you are inexperienced often leads to some code running either slower or harder to reason about.
First (and probably the last one given the fact I don't know the business logic) change I would do to this code is postponing Task's awaiter.
// instead of this
public async Task UnpublishPersonAsync(string id)
{
    await QueueAndUpdateStatusAsync(id, PersonStatus.Unpublishing, QueueNames.Unpublish);
}

// strive to do this
public Task UnpublishPersonAsync(string id)
{
    return QueueAndUpdateStatusAsync(id, PersonStatus.Unpublishing, QueueNames.Unpublish);
}

// or arrow notation if your team likes it
public Task UnpublishPersonAsync(string id)
    => QueueAndUpdateStatusAsync(id, PersonStatus.Unpublishing, QueueNames.Unpublish);

Why is this?
I am always trying to explain the guys in the team that for a change that is so insignificant can give us little tiny bit of improvement. What I mean is that

it does not create async state machine class for your method

it does not introduce unnecessary code that has to be run (goto statements) really really small improvement but if the change is so little.

it leaves the consumer of your method to decide when he wants to await or if he wants to await.

Both methods(QueueAndUpdateStatusAsync and DeletePersonAsync) look the same, alas, they use different overloads of PublishToStorageQueueAsync methods and it will make the code harder to read if you try to abstract them.
What I don't like is throwing exceptions - I use them as a last resort really really last resort. In your case I will refactor both of the methods to follow the same pattern (not that they don't now) like so:
public async Task<bool> DeletePersonAsync(string id, bool deleteAssociatedData)
{
    var personDto = await _cosmosDbService.GetItemAsync<PersonDto>(id, id);
    var operationIsValid = ValidateOperation(PersonStatus.Deleting, personDto);

    // personal preference you can do it with brackets and newlines
    if (!operationIsValid) return false;

    personDto.Status = PersonStatus.Deleting;
    personDto = await _cosmosDbService.UpdateItemAsync(id, id, personDto);
    var person = _mappingService.MapToPerson(personDto);

    var deleteCommand = new DeletePersonCommand()
    {
        Person = person,
        DeleteAssociatedData = deleteAssociatedData
    };

    await _queue.PublishToStorageQueueAsync(deleteCommand, QueueNames.Delete);
    return true;
}

As you can see now you return if the whole transaction is successful or not simply by changing return type to Task<bool> let consumers handle errors the way they want don't force them to handle your exception especially for something so easy to fix.
If you have decided to go with exceptions nevertheless, then I strongly suggest you encode some information inside it. Say some message like "Validation of X failed." anything that can help you give meaning to it later when you happen to debug it. Also if this is some kind of library code, creating custom specific exception is also an option if you like exceptions so much, but as a rule of thumb I strongly suggest you to stay away from exceptions at all cost no matter the language or framework.
P.S. They also incur performance drawback for unwinding the stack later. Your clients need to catch it and probably re-throw it also if necessary, overall it gets messy IMO.
Conclusion

Other than that I think its relatively easy code to follow, don't refactor it unless you find problems with it - whether that will be performance problems, readability problems and etc.


Finding the shortest excerpt that contains all search terms Write a function called answer(document, searchTerms) which
  returns the shortest snippet of the document, containing all of the
  given search terms. The search terms can appear in any order.

Inputs:
(string) document = "many google employees can program"
(string list) searchTerms = ["google", "program"]
Output:
(string) "google employees can program"

 Inputs:
(string) document = "a b c d a"
(string list) searchTerms = ["a", "c", "d"]
 Output:
(string) "c d a"

My program below is giving the correct answer but the time complexity is very high since I am doing the Cartesian product. If the input is very high then I am not able to clear to those test cases. I am not able to reduce the complexity of this program, and any help will be greatly appreciated.

import itertools

import sys

def answer(document, searchTerms):

    min = sys.maxint

    matchedString = ""

    stringList = document.split(" ")

    d = dict()

    for j in range(len(searchTerms)):

        for i in range(len(stringList)):

            if searchTerms[j] == stringList[i]:

                d.setdefault(searchTerms[j], []).append(i)

    for element in itertools.product(*d.values()):

        sortedList = sorted(list(element))

        differenceList = [t - s for s, t in zip(sortedList, sortedList[1:])]

       if min > sum(differenceList):

          min = sum(differenceList)
          sortedElement = sortedList

          if sum(differenceList) == len(sortedElement) - 1:
            break

    try:
        for i in range(sortedElement[0], sortedElement[len(sortedElement)-1]+1):

            matchedString += "".join(stringList[i]) + " "

    except:
        pass

    return matchedString

If anyone wants to clone my program here is code

Pythonicity

Your solution feels very procedural and not really Pythonic.  I believe that this function accomplishes the same task using roughly the same approach:

from itertools import product

def answer(document, search_terms):
    words = document.split(" ")

    search_term_positions = [
        [i for i, word in enumerate(words) if word == term]
        for term in search_terms
    ]

    combos = [sorted(combo) for combo in product(*search_term_positions)]
    if not combos:
        return None    # Could not find all search terms
    best_combo = min(combos, key=lambda combo: combo[-1] - combo[0])

    return " ".join(words[best_combo[0] : best_combo[-1] + 1])

Observations:

d is a variable name that suggests that it's a dict, but is not much more helpful than that.  Instead of d.values(), I've defined search_term_positions, which is a much more descriptive variable name.  You don't really even need a dictionary, since you only care about the values, not the keys.
You can simplify the second for loop by taking full advantage of the built-in min() function.
You wrote sum(differenceList) three times!  sum(differenceList) is just sortedList[-1] - sortedList[0], because

$$(n_1 - n_0) + (n_2 - n_1) + (n_3 - n_2) + \ldots + (n_N - n_{N - 1})$$

is a telescoping series where all of the terms cancel except \$-n_0\$ and \$n_N\$.
sortedElement[len(sortedElement)-1] can be written as sortedElement[-1].
Swallowing all exceptions with except: pass is a no-no.

Bugs

This function only recognizes words that are delimited by spaces.  Any other punctuation could cause words to fail to be recognized.

Your algorithm works based on word count rather than by character positions.  Therefore, you end up finding the excerpt with the fewest intervening words, which is not necessarily what I would consider to be the "shortest snippet".  Example:

>>> answer('many google supercalifragilistic program to google a program', ['google', 'program'])
'google supercalifragilistic program'

I've also written a different solution to solve the same problem.  There, as here, I've made no attempt to choose a speedy algorithm, but rather aimed for clarity.


Implementation of Python's re.split in Clojure (with capturing parentheses) If you use capturing parenthesis in the regular expression pattern in Python's re.split() function, it will include the matching groups in the result (Python's documentation).

I need this in my Clojure code and I didn't find an implementation of this, nor a Java method for achieving the same result.

(use '[clojure.string :as string :only [blank?]])

(defn re-tokenize [re text]
  (let [matcher (re-matcher re text)]
    (defn inner [last-index result]
      (if (.find matcher)
        (let [start-index (.start matcher)
              end-index (.end matcher)
              match (.group matcher)
              insert (subs text last-index start-index)]
          (if (string/blank? insert)
            (recur end-index (conj result match))
            (recur end-index (conj result insert match))))
        (conj result (subs text last-index))))
    (inner 0 [])))

Example:

(re-tokenize #"(\W+)" "...words, words...")
  => ["..." "words" ", " "words" "..." ""]

How could I make this simpler and / or more efficient (maybe also more Clojure-ish)?

You can adjust you implementation to be a lazy-seq for some added performance:

(use '[clojure.string :as string :only [blank?]])

(defn re-tokenizer [re text]
  (let [matcher (re-matcher re text)]
    ((fn step [last-index]
       (when (re-find matcher)
         (let [start-index (.start matcher)
               end-index (.end matcher)
               match (.group matcher)
               insert (subs text last-index start-index)]
           (if (string/blank? insert)
             (cons match (lazy-seq (step end-index)))
             (cons insert (cons match (lazy-seq (step end-index))))))))
     0)))

This implementation will be more efficient as the results will only be calculated as needed. For instance if you only needed the first 10 results from a really long string you can use:

(take 10 (re-tokenize #"(\W+)" really-long-string)

and only the first 10 elements will be computed.


Program that converts a number to a letter of the alphabet #include <iostream>

int main() {
    int letter = 0;
    std::cout << "Please type in a number: ";

    while (1) {
        std::cin >> letter;
        //less than 1 or greater than 26
        if (!(letter < 1 || 26 < letter)) {
            std::cout << "The letter that corosponds to that value is '"
                      << char(64+letter) << "'\n";
            return 0;
        }
        std::cout <<"The English Alphabet only has 26 letters. Try again\n";
    }
}

Gets a integer and prints it as a character. I had thought about using stdio instead of stream but I didn't want it to look scary to a beginner.

C++ does not mandate a particular character coding, so while your code is functionally correct on ASCII and ISO-8859 systems, it won't work in EBCDIC environments, for example.  Even with the obvious fix to add 'A'-1 in place of the magic constant 64, the program assumes that English letters have consecutive values, which just isn't the case in BCD encodings.

The portable approach is to index a string literal:

    static const std::string_view alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

    if (letter > 0 && letter <= alphabet.size()) {
        std::cout << "Letter " << letter << " is '"
                  << alphabet[letter-1]
                  << "'\n";
    }

Other issues:

Don't use letter before checking whether std::cin >> letter succeeded.
Instead of an infinite loop, prefer a finite loop for reading inputs.
Consider accepting input as command-line argument(s) as a more convenient alternative to using standard input.
Spelling: "corresponds".


Copy and paste to different worksheets This code:

Copies a value from "Sheet1"
Pastes special (as a number) in another sheet "Sheet2"
Jumps to the next line

I already:

Disabled screen refresh
Wrapped the code with Application.EnableEvents = False  and Application.Calculation = xlCalculationManual

1 made the calculation a little faster, but 2 didn't seem to make any effect.

Dim iterations, counter As Integer
iterations= 10000
counter = 1

Do While counter <= iterations

' copy value from Sheet1
Sheets("Sheet1").Range("B4").Copy

' paste value in sheet2
Sheets("Sheet2").Select

Selection.PasteSpecial Paste:=xlPasteValues
ActiveCell.Offset(1, 0).Range("A1").Select

counter = counter + 1

Loop

I usually run 50,000+ iterations (counter = 50000).

How can I improve this code speed?

if your aim is to:

copy the value in cell "B4" of worksheet "Sheet1"
and paste it in worksheets "Sheet2" from cell "A1" downwards in iterations cells

then simply use:

Dim iterations: iterations = 1000
Worksheets("Sheet2").range("A1").Resize(iterations).value = Worksheets("Sheet01").range("B4").value

if your aim is to:

copy the value from cell "B4" of worksheet "Sheet1" and down to iterations cells
and paste them in worksheets "Sheet2" from cell "A1" downwards in iterations cells

then simply use:

Dim iterations: iterations = 1000
Worksheets("Sheet2").range("A1").Resize(iterations).value = Worksheets("Sheet01").range("B4").Resize(iterations).value


Wrote a package in Go. What did I do wrong? What did I do right? I wrote a Player package in Go. It builds correctly, but I would love a review on what I did and did not do "the Go way". I'm new to coding in the language, although I've gotten about halfway through Ivo Balbaert's "The Way To Go".

The code consists of a struct and a bunch of setters and getters for the struct.

package Player

import (
    "math/rand"
    "time"
)

type Player struct {
    name   string
    race   string
    id     int64
    pos    Position
    facing int64
    health int64
    wpn_id int64   // Weapon id
    is_buf bool    // Has buff
    buf_mt float64 // Buff multiplier
}

type Position map[int64]int64

func (P *Player) SetPlayer(name string, race string) *Player {
    rand := rand.New(rand.NewSource(time.Now().UnixNano()))

    P.name = name
    P.race = race
    P.id = rand.Int63()
    P.pos = Position{0: 0}
    P.facing = 0
    P.health = 100
    P.wpn_id = 0
    P.is_buf = false
    P.buf_mt = 0.0

    return P
}

func (P *Player) GetPlayer() *Player {
    return P
}

func (P *Player) SetPosition(x int64, y int64) {
    P.pos = Position{x: y}
}

func (P *Player) GetPosition() Position {
    return P.pos
}

func (P *Player) SetFacing(direction int64) {
    P.facing = direction
}

func (P *Player) GetFacing() int64 {
    return P.facing
}

func (P *Player) SetHealth(amount int64, direction string) {
    if direction == "down" {
        P.health = P.health - amount
    } else {
        P.health = P.health + amount
    }
}

func (P *Player) GetHealth() int64 {
    return P.health
}

func (P *Player) SetWeaponId(weapon_id int64) {
    P.wpn_id = weapon_id
}

func (P *Player) GetWeaponId() int64 {
    return P.wpn_id
}

func (P *Player) SetIsBuffed(is_buff bool) {
    P.is_buf = is_buff
}

func (P *Player) GetIsBuffed() bool {
    return P.is_buf
}

func (P *Player) SetBuffMultiplier(multiplier float64) {
    P.buf_mt = multiplier
}

func (P *Player) GetBuffMultiplier() float64 {
    return P.buf_mt
}

This is fairly bare-bones as far as packages go, but there are thing that can be improved here.

Firstly, to create a player object, someone is going to need to do the equivalent of:

player := new(Player)
player.SetPlayer(...)

This is a bit annoying. I'd much rather create a function that constructs a fully initialised Player:

func NewPlayer(name string, race string) *Player {
    ...
}

We can also use some syntactic sugar to shorten the actual initialisation:

func NewPlayer(name_ string, race_ string) *Player {
    rand := rand.New(rand.NewSource(time.Now().UnixNano()))
    return &Player{name: name_, race: race_, id: rand.Int63(), pos: Position{0: 0},
                   facing: 0, health: 100, wpn_id: 0, is_buf: false, buf_mt: 0.0}
}

I don't really understand what the point of the function:

func (P *Player) GetPlayer() *Player {
    return P
}

is supposed to be. It takes a player, and returns the same player - a no-op.

I'm also not sure why you use a map for position, when 2 integers will do:

type Position struct {
    x int64
    y int64
}

In the following function:

func (P *Player) SetHealth(amount int64, direction string) {
    if direction == "down" {
        P.health = P.health - amount
    } else {
        P.health = P.health + amount
    }
}

direction could be an enum-like type:

type Direction uint8

const (
    DOWN Direction = iota,
    UP   Direction = iota
)

func (P *Player) SetHealth(amount int64, direction Direction) {
    switch direction {
    case DOWN: P.health -= amount
    case UP: P.health += amount
    default: panic("Unknown direction type!")
    }
}

Although this is overkill here, and I'd go with @WinstonEwert's suggestion and just pass in either a positive or negative number that can be used to modify the player health.

A type with a lot of getters and setters doesn't really fit with the Go "mantra". In fact, I'd consider this a bit of an anti-pattern in most languages - if you're going to write both Get and Set functions, without really checking arguments or enforcing constraints, you may as well just give direct access to the underlying variable. This is debatable, however, and I know some people will disagree, so I'll won't belabour the point.


C++ Reading Data in a File I'm not sure if I should ask here. The program worked it's just that I'm not satisfied how I implemented it because I used two while loop to read a specific data and I have no idea what am I doing. How can I make it more efficient and instead of using two while loop?

spritesheet.txt

int main() {

fstream file("spritesheet.txt");

int x, y, width, height;
string num1, num2;
string line;
string data;

if (file.is_open())
{
    while (getline(file, line))
    {
        if (line == "enemy")
        {
            while (file >> data >> num1 >> num2)
            {
                if (data == "xy:")
                {
                    x = std::stoi(num1);
                    y = std::stoi(num2);
                    cout << "x: " << x << endl << "y: " << y << endl;
                }
                if (data == "size:")
                {
                    width = std::stoi(num1);
                    height = std::stoi(num2);
                    cout << "width: " << width << endl << "height: " << height << endl;
                }
            }
            file.close();
            break;
        }
    }
}
system("pause");

}

You could add state to the reader: e.g. have a std::string variable target outside of the loop, read the lines in a while loop and...

while (getline(file, line)) {
  if (line=="enemy" || line=="player") {
    target=line; // remember where to put things
  }
  else {
    const std::string::size_type p=line.find(":");
    if (p!=std::string::npos) {
      assert(!target.empty());
      const std::string tag=line.substr(0, p);
      const std::string val=line.substr(p+1);

      // parse val (maybe with istringstream),
      // then put values into appropriate destination,
      // by using target and tag
    }
  }
}


Lazy split and semi-lazy split Sometimes I need to be able to split data into chunks, and so something like str.split would be helpful. This comes with two downsides:

Input has to be strings
You consume all input when generating the output.

I have a couple of requirements:

It needs to work with any iterable / iterator. Where the items have the != comparator.
I don't want to consume the chunk of data when returning it.
Rather than returning a tuple, I need to return a generator.

And so this left me with two ways to implement the code. A fully lazy version isplit. And one that is semi-lazy where it consumes some of the generator, when moving to the next chunk, without fully consuming it.

And so I created:

from __future__ import generator_stop
import itertools

def _takewhile(predicate, iterator, has_data):
    """
    Return successive entries from an iterable as long as the
    predicate evaluates to true for each entry.

    has_data outputs if the iterator has been consumed in the process.
    """
    for item in iterator:
        if predicate(item):
            yield item
        else:
            break
    else:
        has_data[0] = False

def isplit(iterator, value):
    """Return a lazy generator of items in an iterator, seperating by value."""
    iterator = iter(iterator)
    has_data = [True]
    while has_data[0]:
        yield _takewhile(value.__ne__, iterator, has_data)

def split(iterator, value):
    """Return a semi-lazy generator of items in an iterator, seperating by value."""
    iterator = iter(iterator)
    has_data = [True]
    while True:
        carry = []
        d = _takewhile(value.__ne__, iterator, has_data)
        try:
            first = next(d)
        except StopIteration:
            if not has_data[0]:
                break
            yield iter([])
        else:
            yield itertools.chain([first], d, carry)
            carry.extend(d)

An example of these working are below. There is an edge case with isplit, which is as far as I know inherent from the code being fully lazy. This is shown below too.

print('isplit')
print([list(i) for i in isplit('abc def ghi', ' ')])
print([list(i) for i in isplit(' abc def ghi', ' ')])
s = isplit('abc def ghi', ' ')
print(list(itertools.zip_longest(*itertools.islice(s, 4))))

print('\nsplit')
print([list(i) for i in split('abc def ghi', ' ')])
print([list(i) for i in split(' abc def ghi', ' ')])
s = split('abc def ghi', ' ')
print(list(itertools.zip_longest(*itertools.islice(s, 4))))

Which outputs:

isplit
[['a', 'b', 'c'], ['d', 'e', 'f'], ['g', 'h', 'i']]
[[], ['a', 'b', 'c'], ['d', 'e', 'f'], ['g', 'h', 'i']]
[('a', 'b', 'c', None), ('d', 'e', 'f', None), (None, 'g', 'h', None), (None, 'i', None, None)]

split
[['a', 'b', 'c'], ['d', 'e', 'f'], ['g', 'h', 'i']]
[[], ['a', 'b', 'c'], ['d', 'e', 'f'], ['g', 'h', 'i']]
[('a', 'd', 'g'), ('b', 'e', 'h'), ('c', 'f', 'i')]

I would prefer the name iterable for the iterable argument (compare the documentation for the itertools module), and sep for the seperator argument (compare the documentation for str.split).
isplit has the unsatisfactory feature that you cannot ignore any of the returned iterators — you have to consume each one fully before moving on to the next, otherwise the iteration goes wrong.

For example, suppose we want to select words starting with a capital letter. We might try:

for word in isplit('Abc def Ghi', ' '):
    first = next(word)
    if first == first.upper():
        print(first + ''.join(word))

But this produces the output:

Abc
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
StopIteration

Instead, we have to ensure that we consume each word iterator fully, even if we don't care about it:

for word in isplit('Abc def Ghi', ' '):
    first = next(word)
    if first == first.upper():
        print(first + ''.join(word))
    else:
        for _ in word:
            pass

The same issue arises with the standard library function itertools.groupby, where calling code might move on to the next group before it has finished iterating over the previous group. groupby solves this problem for us by fully consuming the previous group as soon as the caller moves on to the next group. It would be helpful for isplit to do the same.
The similarity with itertools.groupby suggests that we could implement isplit very simply in terms of groupby, like this:

from itertools import groupby

def isplit(iterable, sep):
    """Generate the contiguous groups of items from the iterable that are
    not equal to sep.

    The returned groups are themselves iterators that share the
    underlying iterable with isplit(). Because the source is shared,
    when the isplit() object is advanced, the previous group is no
    longer visible. So, if that data is needed later, it should be
    stored as a list.

    """
    for key, group in groupby(iterable, sep.__ne__):
        if key:
            yield group

Note that this code behaves like plain str.split() in that it coalesces adjacent separators. If you need the behaviour to be more like str.split(' '), with empty groups when there are adjacent separators, then it should be straightforward to add an else: clause to generate the necessary empty iterators, like this:

for key, group in groupby(chain((sep,), iterable, (sep,)), sep.__ne__):
    if key:
        yield group
    else:
        for _ in islice(group, 1, None):
            yield iter(())

This uses itertools.chain and itertools.islice.

(There are a couple of minor optimizations you could make here: the 1-element tuple (sep,) could be stored in a variable and used twice, and iter(()) could be a global constant since you don't need a new empty iterator each time.)


Continuous median in Ruby I have written a program in Ruby which calculates the median of a streaming set of numbers dynamically. For each number that arrives, the median is calculated for all the numbers that have arrived so far.

The issue I am facing is that the program times out on some inputs, and, most likely, the reason is that I return a variable that is passed to a routine. I need to do this because this variable changes inside the routine and I can't pass by reference in Ruby.

Is there any workaround for this inability to pass by reference? I have seen the code involving "binding" but I found it clumsy. Why does Ruby not have a simple, straightforward pass-by-reference as do C++, C# and Java? I have pasted my code below.

I maintain two heaps max and min. The value in any node in the max (min) heap is at least as large (small) as the value in any node below it. Any arriving number is inserted into the max heap or min heap with the proviso that the difference in the number of nodes in the two heaps does not exceed 1, and that the heap property be preserved for both heaps. With this proviso, the median can be calculated from the root elements in the two heaps.

Node=Struct.new(:v,:l,:r)do
end

max=0
min=0

def count(m)
        m==0 ? 0 : 1 + count(m.l) + count(m.r)
end

def delete(d,type)

      if d==0
          return 0
      end
      if type=="max"
          if d.l==0 && d.r==0
              return 0
          elsif d.l!=0 && d.r!=0
              if  d.l.v > d.r.v
                  d.v= d.l.v
                  d.l=delete(d.l,type)
              else
                  d.v= d.r.v
                  d.r= delete(d.r,type)
              end
          elsif d.l == 0
                 d.v= d.r.v
                 d.r=delete(d.r,type)
              else
                 d.v= d.l.v
                 d.l=delete(d.l,type)
              end
     else
          if d.l==0 && d.r==0
              return 0
          elsif d.l!=0 && d.r!=0
              if  d.l.v < d.r.v
                  d.v= d.l.v
                  d.l=delete(d.l,type)
              else
                  d.v= d.r.v
                  d.r= delete(d.r,type)
              end
          elsif d.l == 0
                 d.v= d.r.v
                 d.r=delete(d.r,type)
              else
                 d.v= d.l.v
                 d.l=delete(d.l,type)
              end
     end
return d
    end

def insert(v,type,t)

        if t==0
            return t=Node.new(v,0,0)
        end
            if type=="max"
                    if v>t.v
                    tmp=t.v
                    t.v=v
                                t.l=insert(tmp, type,t.l)
            else
                                t.l=insert(v,type,t.l)
            end
            else
                        if v<t.v
                    tmp=t.v
                    t.v=v
                                t.l=insert(tmp, type,t.l)
            else
                                t.l=insert(v,type,t.l)
            end
        end
                return t

end

n=gets.chomp.to_i

n.times do
    v=gets.chomp.to_i
     if (k=count(max)-count(min))==1
        if v<max.v
            tmp=max.v
            max=delete(max,"max")
            min=insert(tmp,"min",min)
            max=insert(v,"max",max)
        else
                        min=insert(v,"min",min)
        end
        p (max.v+min.v)/2.0
     elsif  k==0
        if max==0 || v<=max.v
                        max=insert(v,"max",max)
            p max.v/1.0
        else
                        min=insert(v,"min",min)
            p min.v/1.0
        end
     else
                if v>min.v
            tmp=min.v
            min=delete(min,"min")
            max=insert(tmp,"max",max)
            min=insert(v,"min",min)
        else
                        max=insert(v,"max",max)
        end
        p (max.v+min.v)/2.0
     end
end

Median vs. Mean

mean in math is commonly referred to as 'average'. Median means 'the number in the middle'.

The mean for (1, 4, 11) would be (1+4+11)/3.0 == 5.3333...
The median for (1, 4, 11) would be [1,4,11][(3/2.0).floor] == 4

Using your code, when I enter 2 2 3 ( take a list of 2 entries. Give me the value for 2 and 3) I get back 2.5. That's a mean, not a median. I guess there really is no median for 2 and 3, but for the sake of conversation let's say we use the logic above, it'd be 2.

I'm going to assume you want a mean instead of a median, because implementations speak more to intent than technical words.

Return Value Causing Timeout?

  The issue I am facing is that the program times out on some inputs,
  and, most likely, the reason is that I return a variable that is
  passed to a routine. I need to do this because this variable changes
  inside the routine and I can't pass by reference in Ruby.

It's not clear to me why you think a timeout is caused when you "return a variable that is passed to a routine." I'm pretty sure that is not the reason.

Passing by Reference doesn't include Immutables

  Is there any workaround for this inability to pass by reference? I
  have seen the code involving "binding" but I found it clumsy. Why does
  Ruby not have a simple, straightforward pass-by-reference as do C++,
  C# and Java?

Ruby uses pass-by-reference. However, if you "throw away" the referenced object, it will appear that Ruby used pass-by-value. Immutable items like numbers, symbols, strings, etc will always act this way because if they are changed, their reference will always be lost. If you increment or reassign a number, you aren't changing an object. You are point to a different object.

For a trivial example, just drop into IRB:

>> x = 8
=> 8
>> x.object_id
=> 17
>> x+=1
=> 9
>> x.object_id
=> 19

Without going to far afield of your question, this is because when you change from 8 to 9, you aren't changing the internal state of some object. You're actually pointing to a new object. And if you replaced a mutable object reference in ruby, you'd lose that reference, too.

 def change_number(num)
   num = num + 1
 end

 def change_object(obj)
   obj = {}
 end

 def add_to_object(obj)
   obj[:bar] = :baz
 end

>> score = 8

>> score.object_id
=> 17

>> out = change_number(score)
=> 9

>> out.object_id
=> 19

>> score.object_id
=> 17  # number 9 is object 19. A new object.

>> mutable = {fu: 'bar'}
=> {:fu=>"bar"}

>> mutable.object_id
=> 6553100

>> out = change_object(mutable) # this is pass-by-reference
=> {}

>> mutable.object_id
=> 6553100 # same object we passed in

>> out.object_id
=> 7488280 # a new object was created, despite pass-by-reference.

>> out = add_to_object(mutable)
=> :baz

>> mutable.object_id
=> 6553100 # same object

>> mutable
=> {:fu=>"bar", :bar=>:baz}  # object was passed by reference!

Mean Value

This is Code Review. If this were posted on Stack Overflow, I would say, "Don't do it that way. Just keep a single sorted list of the values you've seen. Then use the sum and count."

But since this is a code review site, I'll give you some thoughts on your code.

Where are your unit tests?
I highly recommend using full variable names instead of v, k, m, d, etc. We live in a time where computers have more than enough memory to remember an entire word. And there's typically autocomplete. So just type out the full word so when you go back 3 months later you won't have to figure out what each letter meant.
This is an object-oriented language, so using classes would be helpful. For instance, let's create a NumberList class.

Refactor using OO

(I'm assuming that insert(v,"max",max) should be insert(value,"max",max))

Node=Struct.new(:v,:l,:r)do
end

class NumberList
  def initialize()
    @max = 0
    @min = 0
  end

  def count(m)
    m==0 ? 0 : 1 + count(m.l) + count(m.r)
  end

  def delete(d,type)
    if d==0
      return 0
    end
    if type=="max"
      if d.l==0 && d.r==0
        return 0
      elsif d.l!=0 && d.r!=0
        if  d.l.v > d.r.v
          d.v= d.l.v
          d.l=delete(d.l,type)
        else
          d.v= d.r.v
          d.r= delete(d.r,type)
        end
      elsif d.l == 0
        d.v= d.r.v
        d.r=delete(d.r,type)
      else
        d.v= d.l.v
        d.l=delete(d.l,type)
      end
    else
      if d.l==0 && d.r==0
        return 0
      elsif d.l!=0 && d.r!=0
        if  d.l.v < d.r.v
            d.v= d.l.v
            d.l=delete(d.l,type)
        else
            d.v= d.r.v
            d.r= delete(d.r,type)
        end
      elsif d.l == 0
        d.v= d.r.v
        d.r=delete(d.r,type)
      else
        d.v= d.l.v
        d.l=delete(d.l,type)
      end
    end

    return d
  end

  def insert(v,type,t)
    if t==0
      return t=Node.new(v,0,0)
    end
      if type=="max"
        if v>t.v
        tmp=t.v
        t.v=v
        t.l=insert(tmp, type,t.l)
      else
        t.l=insert(v,type,t.l)
      end
    else
      if v<t.v
        tmp=t.v
        t.v=v
        t.l=insert(tmp, type,t.l)
      else
        t.l=insert(v,type,t.l)
      end
    end

    return t
  end

  def add_value( value )
    if (k=count(@max)-count(@min))==1
      if value<@max.v
        tmp=@max.v
        @max=delete(@max,"max")
        @min=insert(tmp,"min",@min)
        @max=insert(value,"max",@max)
      else
        @min=insert(value,"min",@min)
      end

      p (@max.v+@min.v)/2.0
    elsif  k==0
      if @max==0 || value<=@max.v
        @max=insert(value,"max",@max)
        p @max.v/1.0
      else
        @min=insert(value,"min",@min)
        p @min.v/1.0
      end
    else
      if value>@min.v
        tmp=@min.v
        @min=delete(@min,"min")
        @max=insert(tmp,"max",@max)
        @min=insert(value,"min",@min)
     else
       @max=insert(value,"max",@max)
     end

     p (@max.v+@min.v)/2.0
    end
  end
end

keeper = NumberList.new

n=gets.chomp.to_i

n.times do
  keeper.add_value( gets.chomp.to_i )
end

Now that we have it in an object, I could write some tests. But since you posted this 3 months ago, I'm not sure you're even going to read it.

Other things you should change

This is more complicated than necessary. Think harder about your problem rather than jumping in!
Ruby has loose typing, so it is possible to put either a number or an object in a variable. But if you're going to do that, your code shouldn't have to know about it. The object should have basically the same interface as a number. In this case, you initialize with an integer 0 and then later use a Node, which response to v, l and r. Not at all the same. I'm a little confused about how this works but I'd think an empty Node would be your best choice.
Your methods are hard to read. Method names should be "executable comments" ... in other words, you should know on a granular level what's happening just by reading the method name. And they should be written with the idea that a stranger should understand them.
Use more, shorter methods

Getting the Mean, the Ruby Way

Finally, here's how I'd implement a mean value in Ruby. Since Ruby allows you to add methods to core classes like Array, it makes some sense to add mean and sum to those classes. Of course, this will break if you try to do ['Jim', 'Spock', 'McCoy'].sum. So don't do that.

require 'bigdecimal'

class Array
  def sum
    self.inject(0){|sum,x| sum + x }
  end

  def mean
    BigDecimal.new(self.sum) / self.count
  end
end

in the console, you'd do this:

>> [2,4,66,32,3].mean.to_f
=> 21.4