Finding the most frequent words in Pandas dataframe I am new in Python coding. I think the code could be written in a better and more compact form. It compiles quite slowly due to the method of removing stop-words.
I wanted to find the top 10 most frequent words from the column excluding the URL links, special characters, punctuations... and stop-words.
Any criticisms and suggestions to improve the efficiency & readability of my code would be greatly appreciated. Also, I want to know if there exists any dedicated python module to get the desired result easily.
I have a dataframe df such that:
print(df['text'])

0         If I smelled the scent of hand sanitizers toda...
1         Hey @Yankees @YankeesPR and @MLB - wouldn't it...
2         @diane3443 @wdunlap @realDonaldTrump Trump nev...
3         @brookbanktv The one gift #COVID19 has give me...
4         25 July : Media Bulletin on Novel #CoronaVirus...
                                ...
179103    Thanks @IamOhmai for nominating me for the @WH...
179104    2020! The year of insanity! Lol! #COVID19 http...
179105    @CTVNews A powerful painting by Juan Lucena. I...
179106    More than 1,200 students test positive for #CO...
179107    I stop when I see a Stop\n\n@SABCNews\n@Izinda...
Name: text, Length: 179108, dtype: object

I have done it in the following way:
import pandas as pd
import nltk
import re
import string
from nltk.corpus import stopwords
nltk.download('punkt')
nltk.download('stopwords')
from nltk.tokenize import word_tokenize

stop_words = stopwords.words()

def cleaning(text):
    # converting to lowercase, removing URL links, special characters, punctuations...
    text = text.lower()
    text = re.sub('https?://\S+|www\.\S+', '', text)
    text = re.sub('<.*?>+', '', text)
    text = re.sub('[%s]' % re.escape(string.punctuation), '', text)
    text = re.sub('\n', '', text)
    text = re.sub('[’“”…]', '', text)

    # removing the emojies               # https://www.kaggle.com/alankritamishra/covid-19-tweet-sentiment-analysis#Sentiment-analysis
    emoji_pattern = re.compile("["
                           u"\U0001F600-\U0001F64F"  # emoticons
                           u"\U0001F300-\U0001F5FF"  # symbols & pictographs
                           u"\U0001F680-\U0001F6FF"  # transport & map symbols
                           u"\U0001F1E0-\U0001F1FF"  # flags (iOS)
                           u"\U00002702-\U000027B0"
                           u"\U000024C2-\U0001F251"
                           "]+", flags=re.UNICODE)
    text = emoji_pattern.sub(r'', text)

    # removing the stop-words
    text_tokens = word_tokenize(text)
    tokens_without_sw = [word for word in text_tokens if not word in stop_words]
    filtered_sentence = (" ").join(tokens_without_sw)
    text = filtered_sentence

    return text

dt = df['text'].apply(cleaning)

from collections import Counter
p = Counter(" ".join(dt).split()).most_common(10)
rslt = pd.DataFrame(p, columns=['Word', 'Frequency'])
print(rslt)

          Word  Frequency
0      covid19     104546
1        cases      18150
2          new      14585
3  coronavirus      14189
4          amp      12227
5       people       9079
6     pandemic       7944
7           us       7223
8       deaths       7088
9       health       5231

An example IO of my function cleaning():
inp = 'If I smelled the scent of hand sanitizers today on someone in the past, I would think they were so intoxicated that… https://t.co/QZvYbrOgb0'
outp = cleaning(inp)
print('Input:\n', inp)
print('Output:\n', outp)

Input:
 If I smelled the scent of hand sanitizers today on someone in the past, I would think they were so intoxicated that… https://t.co/QZvYbrOgb0
Output:
 smelled scent hand sanitizers today someone past would think intoxicated

Note: The data you're going through is 370k+ lines. Because I tend to run different versions of code a lot during review, I've limited my version to 1000 lines.

Your code goes all over the place. Imports, downloads, another import, a variable being loaded, a function definition, the function being called and oh, another import. In that order. Would you agree it's helpful to sort those, so we can easily find what we're looking for?
The revised head of the file would look like this:
import re
import string
import nltk

import pandas as pd

from collections import Counter
from nltk.tokenize import word_tokenize
from nltk.corpus import stopwords

nltk.download('punkt')
nltk.download('stopwords')

After that, we'd ordinarily put the function definition. However, there's part of the program that doesn't have to be in the function itself. It only has to be executed once, even if multiple files are handled.
# removing the emojies
# https://www.kaggle.com/alankritamishra/covid-19-tweet-sentiment-analysis#Sentiment-analysis
EMOJI_PATTERN = re.compile("["
                           u"\U0001F600-\U0001F64F"  # emoticons
                           u"\U0001F300-\U0001F5FF"  # symbols & pictographs
                           u"\U0001F680-\U0001F6FF"  # transport & map symbols
                           u"\U0001F1E0-\U0001F1FF"  # flags (iOS)
                           u"\U00002702-\U000027B0"
                           u"\U000024C2-\U0001F251"
                           "]+", flags=re.UNICODE)

The variable is in UPPER_CASE now, because it's a pseudo-constant (Python doesn't really have constants, but it's a reminder to you and other developers that the variable should be set once and only once). It's customary to put the pseudo-constants between the imports and function definitions so you know where to look for them.
Now, the rest of the program is mostly fine already. You could use more functions, but with a program this size that would mostly be an exercise. I'd rename some of the variables, cut up the lines, use a proper docstring (you had a great start already with the comment at the start of the cleaning function) and prepare the program for re-use. After all, it would be nice to simply import from this file instead of having to copy code to the next few projects using this, wouldn't it? And we don't want to run the specifics of this program every time it's imported, so we explicitly only run it if it's not imported.
import re
import string
import nltk

import pandas as pd

from collections import Counter
from nltk.tokenize import word_tokenize
from nltk.corpus import stopwords

nltk.download('punkt')
nltk.download('stopwords')

STOP_WORDS = stopwords.words()

# removing the emojies
# https://www.kaggle.com/alankritamishra/covid-19-tweet-sentiment-analysis#Sentiment-analysis
EMOJI_PATTERN = re.compile("["
                           u"\U0001F600-\U0001F64F"  # emoticons
                           u"\U0001F300-\U0001F5FF"  # symbols & pictographs
                           u"\U0001F680-\U0001F6FF"  # transport & map symbols
                           u"\U0001F1E0-\U0001F1FF"  # flags (iOS)
                           u"\U00002702-\U000027B0"
                           u"\U000024C2-\U0001F251"
                           "]+", flags=re.UNICODE)

def cleaning(text):
    """
    Convert to lowercase.
    Rremove URL links, special characters and punctuation.
    Tokenize and remove stop words.
    """
    text = text.lower()
    text = re.sub('https?://\S+|www\.\S+', '', text)
    text = re.sub('<.*?>+', '', text)
    text = re.sub('[%s]' % re.escape(string.punctuation), '', text)
    text = re.sub('\n', '', text)
    text = re.sub('[’“”…]', '', text)

    text = EMOJI_PATTERN.sub(r'', text)

    # removing the stop-words
    text_tokens = word_tokenize(text)
    tokens_without_sw = [
        word for word in text_tokens if not word in STOP_WORDS]
    filtered_sentence = (" ").join(tokens_without_sw)
    text = filtered_sentence

    return text

if __name__ == "__main__":
    max_rows = 1000  # 'None' to read whole file
    input_file = 'covid19_tweets.csv'
    df = pd.read_csv(input_file,
                     delimiter = ',',
                     nrows = max_rows,
                     engine = "python")

    dt = df['text'].apply(cleaning)

    word_count = Counter(" ".join(dt).split()).most_common(10)
    word_frequency = pd.DataFrame(word_count, columns = ['Word', 'Frequency'])
    print(word_frequency)

Naturally, if you'd want a more memory-efficient version, you could cut-out all intermediate variables in those last few lines. That would make it a little harder to read though. As long as you're not reading multiple large files into memory in the same program, it should be fine.
Some of the advice I've provided comes from the PEP8, the official Python style guide. I can highly recommend taking a look at it.


Preventing keydown delay I created a class to prevent the default delay which happens when holding down the key on Windows Forms. It seems like the class is working correctly but I don't know if I'm using the best practice.

KeyboardController class;

class KeyboardController
{
    public event EventHandler kbEvent;
    private DispatcherTimer timer;
    private HashSet<Key> keyPressed;

    public KeyboardController(Window c, EventHandler function)
    {
        c.KeyDown += WinKeyDown;
        c.KeyUp += WinKeyUp;
        kbEvent = function;
        keyPressed = new HashSet<Key>();

        timer = new DispatcherTimer();
        timer.Tick += new EventHandler(kbTimer_Tick);
        timer.Interval = new TimeSpan(0, 0, 0, 0, 10);
        timer.Start();
    }

    public bool KeyDown(Key key)
    {
        if (keyPressed.Contains(key))
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    private void WinKeyDown(object sender, KeyEventArgs e)
    {
        keyPressed.Add(e.Key);
    }

    private void WinKeyUp(object sender, KeyEventArgs e)
    {
        keyPressed.Remove(e.Key);
    }

    private void kbTimer_Tick(object sender, EventArgs e)
    {
        kbEvent(null, null);
    }
}

MainWindow.xaml.cs;

public MainWindow()
{
    EventHandler eh = new EventHandler(KeyboardFunction);
    kbc = new KeyboardController(this, eh);
}

private void KeyboardFunction(object sender, EventArgs e)
{
    if (kbc.KeyDown(Key.Right))
    {
        // do something.
    }
    else
    {
        // do something.
    }
}

The main issue is that the key event handler and the timer tick will run on different threads which means that someone could press a key while the timer tick fires - in which case the HashSet could be modified while you are trying to read it from another thread. You will need to safeguard against that by protecting the access to the HashSet with a lock You can probably get away with it because the DispatcherTimer runs on the UI thread where the KeyDown/KeyUp events are also being raised.
The idiomatic way of subscribing to event handler is to let the user subscribe to the event rather than passing the event handler via the constructor. What happens if the user has shorter lifetime than the event provider? In your case it would keep it alive because you can't unsubscribe.
When raising the event you need to check if the vent handler is null - subscriptions to an event are considered optional which means no subscriber might exist. Also you should not pass null as sender or EventArgs. The idiomatic pattern in C# goes like this:

private void kbTimer_Tick(object sender, EventArgs e)
{
    var handler = kbEvent;
    if (handler != null)
        handler(this, EventArgs.Empty);
}

The KeyDown method can be reduced to 1 line of code:

return keyPressed.Contains(key);

The name for the event is a bit non-descriptive. How about keyboardTick?
Standard naming convention in C# for public fields/properties/methods is PascalCase (so the event should be KeyboardTick).

In summary the final code could look like this:

class KeyboardController
{
    public event EventHandler KeyboardTick;
    private DispatcherTimer timer;
    private HashSet<Key> pressedKeys;
    private readonly object pressedKeysLock = new object();

    public KeyboardController(Window c)
    {
        c.KeyDown += WinKeyDown;
        c.KeyUp += WinKeyUp;
        pressedKeys = new HashSet<Key>();

        timer = new DispatcherTimer();
        timer.Tick += kbTimer_Tick;
        timer.Interval = new TimeSpan(0, 0, 0, 0, 10);
        timer.Start();
    }

    public bool KeyDown(Key key)
    {
        lock (pressedKeysLock)
        {
            return pressedKeys.Contains(key);
        }
    }

    private void WinKeyDown(object sender, KeyEventArgs e)
    {
        lock (pressedKeysLock)
        {
            keyPressed.Add(e.Key);
        }
    }

    private void WinKeyUp(object sender, KeyEventArgs e)
    {
        lock (pressedKeysLock)
        {
            keyPressed.Remove(e.Key);
        }
    }

    private void kbTimer_Tick(object sender, EventArgs e)
    {
        var handler = keyboardTick;
        if (handler != null)
        {
            handler(this, EventArgs.Empty);
        }
    }
}

and the usage:

public MainWindow()
{
    kbc = new KeyboardController(this);
    kbc.KeyboardTick += HandleKeyboardTick;
}

private void HandleKeyboardTick(object sender, EventArgs e)
{
    if (kbc.KeyDown(Key.Right))
    {
        // do something.
    }
    else
    {
        // do something.
    }
}


Collection of XmlNode control I have implemented the structure below at least 5 times, and it is getting cumbersome.  Is there a better approach?
Current code-segment implemented
public XmlNodeList Templates {
  get {
    return this.GetElementsByTagName( "Templates" );
  }
  set {
    foreach( UITemplateAssoc nd in value ) {
      this.AddTemplate( nd );
    }
  }
}
public UITemplateAssoc GetTemplate (string name) {
  foreach( UITemplateAssoc nd in this.Templates ) {
    if( nd.Name == name ) {
      return nd;
    }
  }

  return null;
}
public void AddTemplate ( UITemplateAssoc val ) {
  for( int i = this.Templates.Count - 1 ; i >= 0 ; i-- ) {
    if( this.Templates[i].Name == val.Name ) {
      this.ReplaceChild( val , this.Templates[i] );
      return;
    }
  }

  this.AppendChild( val );
}

There's a couple of nice alternatives for you.

Extensions

If you don't need to encapsulate the XmlNodeList instances, you could make your methods extensions for XmlDocument or whatever you've extended:

public static class TemplateExtensions
{
    public static XmlNodeList GetTemplates(this XmlDocument document)
    {
        return document.GetElementsByTagName("Templates");
    }

    public static UITemplateAssoc GetTemplate(this XmlNodeList list, string name)
    {
        foreach(var nd in list)
        {
            if (nd.Name == name) return nd;
        }
    }

    // ...
}

// ...
var templates = yourDocument.GetTemplates();
var template = templates.GetTemplate("abc");

More on extensions methods here:
http://msdn.microsoft.com/en-us/library/bb383977.aspx

Inheritance

Put these methods in a base class and have the behavior that differs in inherited classes.

[edit]Updated to show how the base class can be generic[/edit]

public abstract class TemplateListBase<T>
    where T : XmlNode
{
    private XmlNodeList templates;

    public XmlNodeList Templates
    {
        get
        {
            return templates.GetElementsByTagName( "Templates" );
        }
    }

    protected TemplateListBase(XmlNodeList initialList)
    {
        templates = initialList;
    }

    public void AddRange(XmlNodeList list)
    {
        foreach( T nd in list )
        {
            AddTemplate( nd );
        }
    }

    public T GetTemplate (string name)
    {
        foreach( T nd in templates )
        {
            if( nd.Name == name ) return nd;
        }
        return null;
    }

    public void AddTemplate ( T val )
    {
        for( int i = templates.Count - 1 ; i >= 0 ; i-- )
        {
            if( templates[i].Name == val.Name )
            {
                templates.ReplaceChild( val , templates[i] );
                return;
            }
        }
        templates.AppendChild( val );
    }
}

public class UITemplateAssocList : TemplateListBase<UITemplateAssoc>
{
    public UITemplateAssocList(XmlNodeList listOfUITemplates)
        : base(listOfUITemplates)
    {
    }

    public void FancyLogicA(string data)
    {
        foreach(var node in Templates)
        {
            // differing logic
        }
    }
}

public class ConcreteTemplateB : TemplateListBase<SomeOtherKindOfNode>
{
    // ..

    public void FancyLogicB(XmlNodeList list)
    {
        for (var node in list)
        {
            if (LikeThisNode(node)) AddTemplate(node);
        }
    }
}

Notice that I replaced your setter with an AddRange method.
It's ususally good practice not to replace an entire encapsulated collection using a setter.
Although you can validate the entries in the setter, a property should not do much logic other than returning or setting privates.
If you have a look at the collections in the BCL, they all expose an AddRange method and a method Clear to empty the collection. The AddRange method also gives you the option of adding multiple sets in turn.

If you need different logic when stuff is added etc, have a look at the template method pattern:
http://en.wikipedia.org/wiki/Template_method_pattern

If you need multiple base classes with differing logic, you could combine the inheritance with the extension methods, or another base class. :)


Plotting quadratic equations with varying parameters On Wikipedia, there is a graph shown:

It shows the quadratic \$y = ax2 + bx + c\$, varying each coefficient separately while the other coefficients are fixed (at values a = 1, b = 0, c = 0)

As a learning experience I decided to replicate these plots in Matplotlib as follows:

import numpy as np
import matplotlib.pyplot as plt
import math

#Plot the quadratic function y = ax2 + bx + c
#Varying each coefficient [a, b, c] separately while the other coefficients are fixed (at values a = 1, b = 0, c = 0)

#Iterate these 5 coeficients and plot each line
coefs = [-2, -1, 0, 1, 2]

#set up the plot and 3 subplots (to show the effect of varying each coefficient)
f, (ax1, ax2, ax3) = plt.subplots(1, 3, sharey=True, figsize=(18, 6))

#some x values to plot
x = np.linspace(-2, 2, 30)

for idx, val in enumerate([ax1, ax2, ax3]):

    for v in coefs:
        a, b, c = 1, 0, 0

        if idx == 0:
            a = v
        elif idx == 1:
            b = v
        else:
            c = v

        y = a * (x**2) + (b * x) + c

        val.plot(x, y, label="Coeficient is " + str(coefs[i]))

        val.axhline(y=0, color='k')
        val.axvline(x=0, color='k')
        val.grid()
        val.legend(loc='lower center')

plt.show()

It works fine:

but I am new to programming and I have an uneasy feeling that using if statements isn't optimal. It feels like I should be iterating over something rather than using ifs.

What other ways could I iterate over the coefficients a, b & c to produce the 3 different subplots?

tl;dr: Use numpy's broadcasting features:

coefs = np.array([-2, -1, 0, 1, 2])

#set up the plot and 3 subplots (to show the effect of varying each coefficient)
f, axes = plt.subplots(1, 3, sharey=True, figsize=(18, 6))

#some x values to plot
x = np.linspace(-2, 2, 30)

# calculate the y-values, varying each coefficient separately
y_values = [coefs[:, None] * x[None, :] ** 2,                 # vary a
            x[None, :] ** 2 + coefs[:, None] * x[None, :],    # vary b
            x[None, :] ** 2 + coefs[:, None]]                 # vary c

for ax, ys in zip(axes, y_values):
    for c, y in zip(coefs, ys):
        ax.plot(x, y, label="Coefficient is {}".format(c))
    ax.axhline(y=0, color='k')
    ax.axvline(x=0, color='k')
    ax.grid()
    ax.legend()
plt.show()

Calculating all necessary y-values would make this a 3D array (one dimension being x, one being the different plots and one being the different values of the coefficients).

To make it slightly easier than that we can handle each plot as a 2D array and make a list of the plots. This way we don't have to deal with the fact that the quadratic term should always appear.

If you really wanted to you could probably make that into a vectorized computation as well, but three dimensions was a bit too much to get straight in my head right now.

Explanation of broadcasting:

You can use the fact that numpy has something called broadcasting. It automatically adapts the shapes of vectors you do some operations on, if they are consistent. It can make things look very confusing but allows you to vectorize operations along multiple dimensions (using the fact that doing so loops at C speed, instead of at Python speed).

A small example: You want to multiply two arrays, both of shape (2, 2). numpy then simply does an element-wise multiplication (so \$c_{ij} = a_{ij} \cdot b_{ij}\$).

But if you multiply two arrays of shape (1, 2) and (2, 2), this is not directly possible. So instead it repeats the first array along its first dimension (since it is only of lenth one) and then does an element-wise multiplication. In other words \$c_{ij} = a_{0j} \cdot b_{ij}\$.

To make this abstract example concrete:

>>> a = np.arange(2).reshape(1,2)
array([[0, 1]])
>>> b = np.arange(4).reshape(2,2)
array([[0, 1],
       [2, 3]])
>>> a * b
array([[0, 1],
       [0, 3]])

Now, the final piece is the ability to cast arrays to different shapes (as I already did in the above example using numpy.reshape. Apart from that command you can also use fancy slicing to achieve the same effect:

>>> np.arange(2)[None, :].shape
(1, 2)
>>> np.arange(2)[:, None].shape
(2, 1)

So it repeats it along the axis where I put None (and puts all values in the axis where I put :).


Creating new CSS element everytime database updates I have created a function using jQuery Ajax which create a new CSS element whenever I update the database using a form.

Is it ok?  If not, hat can be the best method to do it?

$(document).ready(function(){
    $('#statusupdater').submit(function(e) {
        updatestatus();
        e.preventDefault();
    });
});

function updatestatus()
{
$.ajax({
    type: "POST",
    url: "statusupdate.php",
    data: $('#statusupdater').serialize(),
    dataType: "dataString"
});
$('#statuscontainer').prepend( $('
').load('statusupdate.php'));
$("#statusupdater")[0].reset();
    $('.statuspop').fadeOut(300);
            $('#opacify').hide();
}

statusupdate.php

<?php include "dbcon.php";

 $status=$_POST['status'];
 $photo=$_POST['photoupload'];

 if ($status || $photo)
 {
    $date = date('m/d/Y h:i:s');
    $statusinsert = mysqli_query($con,"INSERT INTO status (st_id,statu,date_created,userid) VALUES ('','$status','$date','$id')" );
 }

 $selectstatus = mysqli_query($con,"select * from status where userid = '".$id."' order by st_id desc limit 1");
 $getstatus=mysqli_fetch_array($selectstatus);
 ?>

<?php echo $getstatus["userid"];?>

<?php echo $getstatus["statu"];?>

Set the return datatype to html and update the DOM in the success callback:

function updatestatus()
{
    $.ajax({
        type: "POST",
        url: "statusupdate.php",
        data: $('#statusupdater').serialize(),
        dataType: "html",
        success: function(data, status, xhrObj){
            $('#statuscontainer').prepend($('<div id="stat">').html(data));
            $("#statusupdater")[0].reset();
            $('.statuspop').fadeOut(300);
            $('#opacify').hide();
        }
    });
}

You could also update the DOM using the error and statusCode callbacks if it fails.


Genetic algorithm to find the minimum of a three-variable function I've implemented a simple genetic algorithm for continuous floating point parameter spaces and without recombination. However, the way I'm passing in parameters and acquiring results from multiprocessing feels wrong. Should I be using concurrent.futures?

The code below uses the genetic algorithm to find the minimum of the equation x^2+y^2+z/10 over the parameter space -2 < x < 0, 0 < y < 2 and 10 < z < 11, but I'd like to keep the code easy to modify for various parameter spaces and evaluation functions.

import numpy as np

import multiprocessing
from collections import OrderedDict
import os
import time

def eval_iter(arg_lst, l_lst):
    for c_i, args in enumerate(arg_lst):
        yield c_i, args, l_lst

def eval_func(c_i, args, l_lst):
    assert len(args) == 3
    x = args[0]
    y = args[1]
    z = args[2]
    res = x**2 + y**2 + z/10
    print(f"Eval {x}, {y}, {z}: {res}")
    l_lst[c_i] = res

if __name__ == '__main__':

    generation_num = 10
    child_num = 5

    space = OrderedDict((
        ('x', (-2., 0.)),
        ('y', (0., 2.)),
        ('z', (10., 11.))
    ))

    params = OrderedDict([(nm, []) for nm in space.keys()])
    for nm, v_range in space.items():
        params[nm] = np.random.uniform(v_range[0], v_range[1], size=child_num)

    arg_list = []
    for c_n in range(child_num):
        arg_list.append([val[c_n] for val in params.values()])

    manager = multiprocessing.Manager()
    loss_lst = manager.list([np.inf for i in range(child_num)])

    for r_n in range(generation_num):
        with multiprocessing.Pool(os.cpu_count()) as pool:
            pool.starmap(eval_func, eval_iter(arg_list, loss_lst))

        fittest_idx = int(np.argmin(loss_lst))
        base_args = arg_list[fittest_idx]
        print(f"Best {base_args}\n")

        # mutate offspring from fittest individual
        params = OrderedDict([(nm, []) for nm in space.keys()])
        for s_i, (nm, v_range) in enumerate(space.items()):
            std = (v_range[1] - v_range[0]) / 2
            noise = np.random.normal(0, std, size=child_num)
            new_param = base_args[s_i] + noise
            params[nm] = np.clip(new_param, v_range[0], v_range[1])

        arg_list = []
        for c_n in range(child_num):
            arg_list.append([val[c_n] for val in params.values()])

        loss_lst = manager.list([np.inf for i in range(child_num)])

Better organisation

You program in its current state is one huge chunk of snippet with all logic inside it. You should consider splitting it into separate smaller functions. The limits on \$ x, y, z \$ are preset. Consider putting them as a GLOBAL_CONSTANT.
Ideal import order (according to PEP8) is

standard library imports
related third party imports
local application/library specific imports

os/collections etc. are standard library, and should be imported first, followed by numpy.
pythonify your code

x = args[0]
y = args[1]
z = args[2]

can be expressed as:

x, y, z = args

similarly

manager.list([np.inf for i in range(child_num)])

can become

manager.list([np.inf] * child_num)

I am not sure about how numpy.argmin works, but I think using None instead of np.inf could be better in the sense that you might extend your program to also find local/global maxima along with minima points.


Dynamic association generation in ActiveRecord I'm developing a Rails-based web interface/API for a large, vendor-supplied, legacy Oracle database. One of the constraints is that I can't change the schema other than adding views, because it still has to support the vendor's application.

The table for customer data has a limited number of columns, and extended attributes are stored in customer_def_field_value. These attributes are defined in customer_def_field_def.

so I end up with the classes:

class FieldDef < ActiveRecord::Base

  set_table_name 'customer_def_field_def'
  set_primary_key :customer_def_field_def_id
  set_sequence_name 's_customer_def_field_def'

  alias_attribute :field_def_id, :customer_def_field_def_id

  has_many :field_values

  def symbol
    self.title.parameterize('_').to_sym
  end
end

class FieldValue < ActiveRecord::Base

  set_table_name 'customer_def_field_value'
  set_primary_keys :cust_id, :customer_def_field_def_id
  set_sequence_name :autogenerated

  alias_attribute :field_def_id, :customer_def_field_def_id

  belongs_to :customer, :foreign_key => :cust_id
  belongs_to :field_def, :foreign_key => :customer_def_field_def_id
end

In order to get associations on Customer for each of the 'custom fields', I have this code in the Customer class:

FieldDef.find_each do |field|
  has_one field.symbol, :class_name => 'FieldValue', :foreign_key => :cust_id, :conditions => proc{ "customer_def_field_def_id = #{field.id}"}
  accepts_nested_attributes_for field.symbol
  delegate :field_value, :to => field.symbol, :prefix => true
  delegate :field_value=, :to => field.symbol, :prefix => true
  attr_accessor ('set_' + field.symbol.to_s).to_sym
end

This works like a charm, but it queries the DB every time a Customer is instantiated in order to generate the method definitions.

It hasn't been a performance issue... yet.

New custom field types aren't added very often, so I think I might be able to do this in an initializer rather than in my model, and just restart the application if new fields have to be added... but I wanted to get some feedback. Thanks!

I think the query of the database would only happen once in production mode (the class would get cached, in development mode, the class gets reloaded every request that references a Customer object). You can confirm this by setting:

config.log_level = :debug

in your production.rb file and seeing if the query runs more than once. It might be worth having an after_save callback for FieldDef that does the same thing as the "FieldDef.find_each do |field|" block you have so you don't have to restart the server when new FieldDefs are created.


Enemy damaging with regen system Small project i made to practice my c++, im a beginner and want to know all about optimizing code.
Enemy starts off with 10 HP and we can damage them for 1 HP by pressing F1,
If their HP is below their max they regenerate 1 point each second.
#include <time.h>
#include <Windows.h>
#include <iostream>
using namespace std;

int main()
{
    int maxHP = 10;
    int enemyHp = maxHP;
    cout << "Enemy HP: " << enemyHp << '\n';
    while (enemyHp > 0)
    {
        if (GetAsyncKeyState(VK_F1) & 1)
        {
            enemyHp--;
            system("CLS");
            cout << enemyHp << '\n';
        }
        static int sec = time(NULL);
        int secNew = time(NULL);
        int* secPtr = &sec;

        if (sec != secNew)
        {
            if (enemyHp < maxHP)
            {
                enemyHp++;
                system("CLS");
                cout << enemyHp << '\n';
            }
            *secPtr = secNew;
        }

        if (enemyHp == 0)
        {
            system("CLS");
            cout << "Enemy has died.\n";
        }
    }
    return 0;
}
```

Don't write using namespace std;.
maxHP should be constexpr or at least const.  In general, use const where you can.


MS Fakes Shims. Assert "was called" (by closures) Is it a good or bad practice to use closures for asserting the method was called in such a way?

The method set a shim and return an Action wich both capture local mutable variable:

private static Action<bool> ShimCopy(FileInfo source, FileInfo destination)
{
    var wasCalled = false;
    ShimFileInfo.AllInstances.CopyToString = (info, destinationPath) =>
    {
        Assert.AreEqual(source.FullName, info.FullName, "Copy wrong file");
        Assert.AreEqual(destination.FullName, destinationPath, "Copy to wrong destination");
        Assert.IsFalse(wasCalled, "Redundant file copy");
        wasCalled = true;
        return destination;
    };
    return expected => Assert.AreEqual(expected, wasCalled);
}

So it could be used like this:

...
var assertWasCalled = ShimCopy(source, destination);
act();
assertWasCalled(true);
...

It looks convinient but I don't like impure funtions and here I have exactly the case with side effects and indeterministic results. But I've heard "Closures is a poor man's object"...

In addition returned action has no strict human readable signature describing parameter and function meaning. That could be solved by delegate, but delegate is an additional piece of complexity for the project.

Generally, I see no problem with using closures with unit testing, as long as the intent is clear and concise. Functional programming style can be difficult for others to grasp if they're not familiar with it. So, best practices here may vary from person to person and company to company. I use the pattern a lot for testing if my events are firing correctly.

bool eventCalled = false;
obj.PropertyChanged += (sender,args)=>
 {
      Assert.IsTrue(args.PropertyName.Equals("Change");
      eventCalled=true;
 };
obj.Change = true;

Assert::IsTrue(eventCalled, "PropertyChanged was not called");


12 hours to 24 hours conversion function I have made a time format converter which converts 12 hour format to 24 hour format. My convert_to_24() takes a string in format of HH:MM:SS followed by AM or PM. It returns a converted string to 24 hours format without the AM/PM string.

Note: Let us assume that input will always have no space between them e.g. 07:05:45PM, not 07:05:45 PM

def convert_to_24(time):
    """str -> str
    converts 12 hours time format to 24 hours
    """
    return time[:-2] if time[-2:] == "AM" else str(int(time[:2]) + 12) + time[2:8]

Sample usage:

>>> convert_to_24("07:05:45PM")
'19:05:45'
>>> convert_to_24("10:47:00PM")
'22:47:00'
>>> convert_to_24("02:35:36AM")
'02:35:36'
>>>

Can my function be made even better? By better I mean better in way of performance, design etc.

This function is not just cryptic — it's also wrong.  I would expect convert_to_24('12:00:00AM') (midnight) to be '00:00:00', but it returns '12:00:00' (noon).


String concatenation comparisons For some time, JavaScript string concatenation could be optimised by join()ing an array of strings, instead of +ing them together.

Recently, browsers have been optimised to handle strings better, and +ing is back in favour.

The various posts I've read on the subject have cited many jsperf.com tests, which are invariably regular, and nothing much like the dynamic irregularity typically encountered in most web apps that need to concatenate on the fly.

So, I built a new test, the idea of which is to subject each method of concatenation to a relatively equal degree of randomness.

The results show a similar performance difference as other tests of arr.join("") vs. str + str.

Question: Is this jsperf.com test code good jsperf.com test code; is there method in my madness, or am I just mad?

Familiararity with jsperf.com testing is assumed.

"Where a script is concatenating arbitrary numbers of strings dynamically and often, is it 'better' to define a function to handle it, arr.join() inline, or perform simple inline concatenations?"

Preparation code:

Here I aim simply to create a large string from which random substrings can be extracted and stored to global vars for concatenation during the test runs. Also create a couple of functions, one for programmatic concatenation of an arbitrary number of strings using the arr.join("") method, and another for the random selection of the global substrings.

var base = "so many ways to skin a cat",
    c = 0, bl, hbl,
    rand = function( s, e ) {
        return Math.floor( Math.random() * ( e - s ) ) + s;
    },
    concatenateThese = function() {
        return [].slice.call( arguments ).join( "" );
    };
while ( ++c < 10 ) {
    base += base;
}
bl = base.length;
hbl = Math.floor( bl / 2 );
c = 0;
while ( ++c < 100 ) {
    window[ "randStr" + c ] = base.substr( rand( 0, hbl ), rand( hbl, bl ) );
}

My intention is that all the following test snippets are subject to a realistically equal amount of computational randomness where e.g. gluing an identical string to itself for 10000 iterations, can't be itself optimised. Each value is unknowable until runtime.

Test Snippet 1:

This should concatenate 7 randomly selected global strings from those created by the preparation code, by use of the + operator.

var rslt = window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ] +
    window[ "randStr" + rand( 0, 100 ) ];

Test Snippet 2:

This should concatenate 7 randomly selected global strings from those created by the preparation code, by use of the join() method.

var rslt = [ window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ] ].join( "" );

Test Snippet 3:

This should concatenate 7 randomly selected global strings from those created by the preparation code, by use of the join() method via a handler function created by the preparation code.

var rslt = concatenateThese( window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ],
    window[ "randStr" + rand( 0, 100 ) ] );

Is this jsperf.com test code good jsperf.com test code; is there method in my madness, or am I just mad?

Probably. This is a micro-optimization whose performance is really negligible in recent browsers. The only browsers I remember choking when it comes to strings were IE8 and older.

With that said, out of all your methods, concatenation with + is an obvious winner, all because it operates solely on strings. The other methods use an array as glue.

Test #2 is at a disadvantage as it has to wrap the strings into an array before they get concatenated by join. Test #3 is in even more disadvantage as you are creating an array twice, first on [] and second on slice.

And even with that, in terms of readability, all three look horrible in the presence of template strings which was designed to improve concatenation and interpolation in strings. It should be present on Firefox, Chrome and Edge as well as transpiler setups. Babel uses the + method when transpiling template strings, so there must be a reason for doing so.

So I have skimmed over the actual test code, and focused more on the approaches. By the way, I believe jsperf is just a front and persistent service that uses Benchmark.js under the hood. So if you're doing any actual testing, best use the library directly and locally.

var base = "so many ways to skin a cat",
  c = 0, bl, hbl,
  rand = function( s, e ) {
    return Math.floor( Math.random() * ( e - s ) ) + s;
  },
  concatenateThese = function() {
    return [].slice.call( arguments ).join( "" );
  };

A lot of vaguely named variables. What's c, bl, hbl, s, e and everything else? Tests are made to verify code. If I can't read the test, how do I know it works? How do I know what it's supposed to do?

rand appears to be a ranged random number generator. Name it as such. concatenateThese... concatenate what exactly? Sure, there's arguments, but the thing about code is that if I cannot know what the code does without having to dig up the source or pull up a debugger, then that's terrible code.

var base = "so many ways to skin a cat",

/* ton of code between */

while ( ++c < 10 ) {
    base += base;
}

Why are these two pieces of code separate? If I were reading code and the other part was 100 lines or more apart, I would wonder how base was 10 times the size. I'd have to (gasp) dig up the source... again.

By the way, another way of multiplying a string is

Array(10).join('the sentence you want multiplied');

You happen to use window. That's... bad. Nobody wants to pollute the global namespace. Suggesting you store those random strings in an array or object, not on window. While jsperf tests are isolated from each other, your prep code and tests aren't. There's a chance you might introduce something in either code that can infect the other, causing erroneous test results. Sure, you namespaced with randStr, but that doesn't guarantee that you won't mess it up in the long run. Writing tests is tedious, refactoring tests is a nightmare. Best done right always.


Generate a random numbers class I have made class to setup a random number bases on <random>. It works fine, but I have doubt in std::default_random_engine mEngine{ std::random_device()() }; Did i implement it correctly? What is the best practice for random?

#include <random>
#include <iostream>

// for some compilers that's don't support nested templates with the same parameter
template <typename T>
auto dist() -> typename std::enable_if<std::is_integral<T>::value, std::uniform_int_distribution<T> >::type;

template <typename T>
auto dist() -> typename std::enable_if<std::is_floating_point<T>::value, std::uniform_real_distribution<T> >::type;

template<typename T>
class Random
{
public:
    Random(const T& min, const T& max)
        : mUnifomDistribution(min, max)
    {}
    Random(const Random<T>&) = delete;
    Random(const Random<T>&&) = delete;
    Random<T>& operator = (const Random<T>&) = delete;
    T operator()()
    {
        return mUnifomDistribution(mEngine);
    }

private:
    std::default_random_engine mEngine{ std::random_device()() }; // <-- here the doubt - is it seeding?
    //template <typename T>
    //static auto dist() -> typename std::enable_if<std::is_integral<T>::value, std::uniform_int_distribution<T> >::type;

    //template <typename T>
    //static auto dist() -> typename std::enable_if<std::is_floating_point<T>::value, std::uniform_real_distribution<T> >::type;

    using type = decltype(dist<T>());
    type mUnifomDistribution;
};

int main()
{
    ::Random<int> getRandom(0, 9);
    for (int i = 0; i<9; ++i)
        std::cout << getRandom() << '\n';
}

I have a few comments on this.
Don't attempt to nest templates with the same parameter
First, as you've already discovered, nested templates with the same parameter are a violation of the C++ language standard, whether or not your compiler complains.  (In fact, I would say it's broken if it does not complain.)
C++ standard, section 14.6.1:

A template-parameter shall not be redeclared within its scope (including nested scopes). A template-parameter shall not have the same name as the template name.

The definition of mUniformDistribution can be simpler
The code doesn't need a using statement.  The declaration can be made directly:
decltype(dist<T>()) mUniformDistribution;

(Also note that the existing code misspells "Uniform")
The use of Random does not require a scope
The code in main refers to ::Random but that's really not necessary here.  It should just be Random.
Consider repeatability
To answer the question in your code, yes,
std::default_random_engine mEngine{ std::random_device()() };

that line of code does seed mEngine however, I'd suggest rethinking that feature.  If you were using this random number generator for, say a simulation, you might want to be able to duplicate the simulation exactly and not simply always run it with new random numbers.  To get the best of both worlds, you could keep your existing code, but eliminate the initialization of mEngine and add it instead to the constructors:
Random(const T& min, const T& max)
    : mEngine{},
      mUniformDistribution(min, max)
{}
Random(const T& min, const T& max, bool )
    : mEngine{ std::random_device()() },
      mUniformDistribution(min, max)
{}
// ...
std::default_random_engine mEngine;

Now if you construct using the first version, you'll get the default seed each time.  If you use the second constructor (with a dummy bool parameter) it will use a new seed each time.


Synchronization of remote files download Preamble: it's a self-assigned and pure syntetic task to learn (and remember what I already knew) C# threads and synchronization and data structures.

The original question was here https://stackoverflow.com/questions/16458937/synchronization-of-remote-files-download

The short task explanation:

Let's say I have a dictionary <string, string> that represents a path (http) to a file by some key, ie:

foo => http://domain.tld/file1
bar => http://domain2.tld/file2

And I'd like to implement a class that will implement an interface with 2 methods:

String Rand();
String Get(String key);

The first method would pick the file randomly from all the available, and the Get would return a particular file, or to be precise - local path to a downloaded file.

The class should be thread-safe, so that if several threads request the same key with Get() or the Rand() picks the same item - then only one thread should actually download a file to a local drive, or the path should be retrieved immediately if a file has already been downloaded.

My proof of concept solution is:

class Downloader
{
    private readonly IDictionary<string, string> _map;
    private IDictionary<string, string> _storage = new Dictionary<string, string>();
    private ConcurrentDictionary<string, Task<string>> _progress = new ConcurrentDictionary<string,Task<string>>();

    public Downloader(IDictionary<string, string> map)
    {
        _map = map ?? new Dictionary<string, string>();
    }

    public string Get(string key)
    {
        if (_storage.ContainsKey(key))
        {
            return _storage[key];
        }

        Task<string> task;
        if (_progress.TryGetValue(key, out task))
        {
            return task.Result;
        }

        task = _retrieveFile(key);

        if (!_progress.TryAdd(key, task))
        {
            return Get(key);
        }

        task.Start();
        _storage[key] = task.Result;
        return task.Result;
    }

    private Task<string> _retrieveFile(string key)
    {
        string path;

        if (!_map.TryGetValue(key, out path))
        {
            throw new ArgumentException("The specified key wasn't found");
        }

        return new Task<string>(k =>
        {
            Console.WriteLine("Started retrieving {0}", k.ToString());
            System.Threading.Thread.Sleep(3000);
            Console.WriteLine("Finished retrieving {0}", k.ToString());
            return k.ToString() + " local path";
        }, path);
    }
}

The copy is at http://pastebin.com/LJnK7FB6

How could I make it better? Any particular problems with the code?

In the comments @I4V pointed out to some problems at https://stackoverflow.com/questions/16458937/synchronization-of-remote-files-download#comment23617420_16460673 but seems like I'm missing the point

If you change _retrieveFile to return a Task that's already started, you could simplify the whole Get() into just:

return _progress.GetOrAdd(key, _retrieveFile).Result;

Also, it seems that Downloader with empty map doesn't make sense, so you should check for that in your constructor, instead of using ??.


CodeEval solutions to find trailing strings This is a problem in CodeEval, moderate level. The problem is very easy for this level. But I am not able to get more than 60 points for any solution I submit. (Moderate levels highest score is 65 and they take speed and memory into consideration.)

The problem statement is:

  There are two strings: A and B. Print 1 if string B occurs at the end
  of string A. Otherwise, print 0.

  Input

  Hello World,World
  Hello CodeEval,CodeEval
  San Francisco,San Jose

  Output

  1
  1
  0

import java.io.*;

public class Main {
    public static void main (String[] args) throws IOException {
        File file = new File(args[0]);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        String line;
        String[] words;
        while ((line = buffer.readLine()) != null) {
            System.out.println(trailingStrings(line));
        }
    }

    public static int trailingStrings(String line) {
        int indexOfComma = line.indexOf(",");

        int secondStringLength = line.length()-indexOfComma-1;

        int firstStringLength = indexOfComma;

        if(firstStringLength < secondStringLength) {
            return 0;
        }
        int i = indexOfComma+1;
        int j = indexOfComma - secondStringLength;
        while(j<indexOfComma) {
            if(line.charAt(j) != line.charAt(i)) {
                return 0;
            }
            j++; i++;
        }
        return 1;
    }
}

Memory Usage: 4784128 (bytes)
Speed: 644 ms

If I instead split the line with "," and get the words and use String.endsWith(), I am getting better memory usage. This surprises me because in my first solution I am not creating new String objects, I just use one String.

import java.io.*;
public class Main {
    public static void main (String[] args) throws IOException {
        File file = new File(args[0]);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        String line;
        String[] words;
        while ((line = buffer.readLine()) != null) {
            words = line.split(",");
            String s1 = words[0];
            String s2 = words[1];
            if(s1.endsWith(s2)){
                System.out.println("1");
            } else {
                System.out.println("0");
            }
        }
    }
}

Memory Usage: 413696 (bytes)
Speed: 1047 ms

Why is my first solution taking up so much memory? Is it because of the int declarations? If I remove int declarations and use the values directly, memory usage reduced by half to 2412544 bytes but still way higher compared to second solution.

I suggest a functional answer for this problem (In ruby but should easily be translatable into Java):

def endswith(text, appendix)
    text.last(appendix.length) == appendix
end

I did not benchmark but this approach is likely to be faster as you are only using using 2 functions and a comparison instead of all the loops and ifs that you used.


Asking the user to select a test subject and difficulty level I'm currently in the early stages of a Python project, and the program (so far) asks the user what test they would like to take and what difficulty they would like it to be, and I'd like to know if there're any optimisations that I could make.

class NotWork(Exception): pass
class DoWork(Exception): pass

def which_test(real_dif, real_test, give_test):
    if difficulty == real_dif and test == real_test:
        give_test()

def is_it_correct(variable, subdif1, subdif2, subdif3, message):
    if variable != subdif1 and variable != subdif2 and variable != subdif3:
        print(message)
        raise NotWork
    else:
        raise DoWork

while True:
    try:
        test = str(input("What test do you want to take? Computer Science, History or Music? ").strip().lower())
        is_it_correct(test, "computer science", "history", "music", """\nSorry, you didn’t input valid settings. Please enter (without the quotes) either "Computer Science", "History", or "Music".\n""")
    except NotWork:
        continue
    except DoWork:
        break

while True:
    try:
        difficulty = str(input("Do you want to take the test in easy, medium or hard? ").strip().lower())
        is_it_correct(difficulty, "easy", "medium", "hard", """\nSorry, you didn't input valid settings. Please enter (without the quotes) either "Easy", "Medium", or "Hard".\n""")
    except NotWork:
        continue
    except DoWork:
        break

which_test("easy", "computer science", easy_CS)
which_test("medium", "computer science", medium_CS)
which_test("hard", "computer science", hard_CS)
which_test("easy", "history", easy_history)
which_test("medium", "history", medium_history)
which_test("hard", "history", hard_history)
which_test("easy", "music", easy_music)
which_test("medium", "music", medium_music)
which_test("hard", "music", hard_music)

Use of Exceptions/Errors

Raising the DoWork error when input validation succeeds seems strange. Errors are usually raised by a function when, for some reason, it could not do what it was supposed to. Raising an error from is_it_correct if the input was not correct seems like fair game - in fact, there is a built-in ValueError that is meant to be raised when a function is called with values that are somehow malformed.

However, since your method is called is_it_correct already, why not just return a boolean that answers the question that is the method name?

Call signature

Right now, is_it_correct takes one argument per valid choice. What if, at some point, you want to add another test to your program? You would have to create a method that takes an additional valid choice as argument. But then, you would run into trouble with the difficulty selection - it still only has two valid choices.

I recommend using an array for the valid choices for extensibility's sake.

Separation of concerns

Right now, the is_it_correct method is in charge of validating the correctness of the input as well as printing out an error message. I recommend moving the error message to the point in your code where other user interaction takes place already - namely close to the input calls.

With all of the above recommendations considered, is_it_correct and its usage could look like this:

def is_it_correct(choice, valid_choices):
    return choice in valid_choices

while True:
    test = input()
    test_choices = ["computer science", "history", "music"]
    if is_it_correct(test, test_choices):
        break
    else:
        print("Sorry, you didn't input valid settings. Available choices: " + ", ".join(test_choices))

Deduplication

You have two while True loops doing something very similar - prompting the user for one of a given set of choices and displaying an error message if he fails to enter a valid choice. Why not wrap that up in a method?

def require_input(prompt, choices):
  # generate a readable list of choices
  choices_string = "{} or {}".format(", ".join(choices[:-1]), choices[-1]).capitalize()

  # build the prompt message dynamically
  msg = "{} {}?\n".format(prompt, choices_string)

  while True:
    user_choice = input(msg)
    if is_it_correct(user_choice, choices)
        return user_choice
    print("Please input either " + choices_string + ".")

tests = ["computer science", "history", "music"]
test = require_input("What test do you want to take?", tests)

difficulty_levels = ["easy", "medium", "hard"]
difficulty = require_input("How difficult do you want the test to be?", difficulty_levels)


Lo Shu Magic Square (Python) I wrote a python program to find if a matrix is a magic square or not. It works, but I can't help feeling like I may have overcomplicated the solution. I have seen other implementations that were a lot shorter, but I was wondering how efficient/inefficient my code is and how I could improve it and/or shorten it to achieve the result I am looking for.
def main():

    matrix = [[4,9,2],
              [3,5,7],
              [8,1,6]]

    result = loShu(matrix)

    print(result)

def loShu(matrix):

    i = 0
    j = 0

    for i in range(0, len(matrix)):
        for j in range(0, len(matrix[j])):
            if ((matrix[i][j] < 1) or (matrix[i][j] > 9)):
                return ("This is not a Lo Shu Magic Square - one of the numbers is invalid")

    row1 = matrix[0][0] + matrix[0][1] + matrix[0][2]
    row2 = matrix[1][0] + matrix[1][1] + matrix[1][2]
    row3 = matrix[2][0] + matrix[2][1] + matrix[2][2]

    ver1 = matrix[0][0] + matrix[1][0] + matrix[2][0]
    ver2 = matrix[0][1] + matrix[1][1] + matrix[2][1]
    ver3 = matrix[0][2] + matrix[1][2] + matrix[2][2]

    diag1 = matrix[0][0] + matrix[1][1] + matrix[2][2]
    diag2 = matrix[0][2] + matrix[1][1] + matrix[2][0]

    checkList = [row1,row2,row3,ver1,ver2,ver3,diag1,diag2]

    temp = checkList[0]

    for x in range (0, len(checkList)):
        if checkList[x] != temp:
            return ("This is not a Lo Shu Magic Square")

    return ("This is a Lo Shu Magic Square")

main()

Your solution is hardcoded for matrices of size 3x3 - why not support matrices of arbitrary size as well? It's also difficult to maintain and debug your code, i.e., it's easy to mess up somewhere when you are indexing your rows and columns. Also, imagine you wanted to consider larger matrices: what a nightmare to now modify your existing logic!
A more robust solution could turn to numpy. Specifically, you can check row and column sums by doing m.sum() and m.sum(axis=1), and compute the sums of the diagonals via m.trace() or by sum(np.diag(m)) and sum(np.diag(np.fliplr(m)).
As a general comment, just return a boolean value from your function. Let the main program decide what do with this value, i.e., whether to print "this is good", "This is a Lo Shu Magic Square", or whatever. In this way, your verifier function stays clean and more generic.


Are the returns within my session function actually needed? I have the following object oriented class revolving about the PHP super global $_SESSION  I shown the script to a fellow developer and he mentioned that all my returns are not necessary needed, whereas I believe that they are?

My code is as followed:

/*
*
*
*   This class will allow control of the super global $_SESSION within PHP
*       It's possible to import this class within websites which are already working with session variables.
*   Currently the functionality provides:
*       Invoking a new session
*       Getting the current session stats
*       Setting a session key/value
*       Getting the session value
*       displaying the session array
*
*
*
*/

class Session {
    protected $Session_Started = false;

    public function __construct(){
        /*
        *
        *   If session is already invoked,  the constructor
        *   will switch the protected variable to true, and the session_start function will not be called
        *       If session is not already invoked, the protected variable will remain false and provide the ability
        *           To invoke a session_start() by calling $Class->init();
        */
            if (session_status() === 1){
                $this->Session_Started = true;
                return false;
            }
            return true;
    }
    public function init(){
        /*
            Invoke a session

        */
        if ($this->Session_Started === false){
            session_start();
            $this->Session_Started = true;
            return true;
        }
        return false;
    }
    public function Status_Session(){
        /*
            Getting the current session status, and return readable information on the current status
        */
        $Return_Switch = false;
        if (session_status() === 1){
            $Return_Switch = "Session Disabled";
        }elseif (session_status() === 2){
            $Return_Switch = "Session Enabled, but no sessions exist";
        }elseif (session_status() === 3){
            $Return_Switch = "Session Enabled, and Sessions exist";
        }
        return $Return_Switch;
    }
    public function Set ($Key = false, $Value){
        /*
            Set a value within the $_SESSION global
                this function will return true, if a sucessfull addition has been made
                and return false if a problem is enountered
        */
        if ($this->Session_Started === true){
            $_SESSION[$Key] = $Value;
            return true;
        }
        return false;
    }
    public function Get ($Key){
        /*
            Invoking this function will return the current value of the session key, else return false is an error is encountered
        */
        if (isset($_SESSION[$Key])){
            return $_SESSION[$Key];
        }
        return false;
    }
    public function Display(){
        /*
            If session is started, this function will return a readable and formatted array
        */
        if ($this->Session_Started === true){
            return "<pre>".$_SESSION."</pre>";
        }
    }

} // Close class

Now, for example:

$Class = new Session();

if ($Class->Set("Key","Value")){
  echo "Created Value/key within the Session array, and returned true";
}

I understand that people like to validate a successful/fail boolean based on true and false, so I have included the correct return, but my overall question is just to confirm which I believe is correct.. Which is regarding the returns... Is it a perfectly acceptable thing to do, over having no returns? and within best preference?

As for your question, at least Get should not return false if the value is not set, now the caller will not know whether the actual value is false or whether the value is not set.

Other than that,

I would suggest you use PHP_SESSION_DISABLED , PHP_SESSION_NONE and PHP_SESSION_ACTIVE instead of 1, 2 and 3
To have Status_Session call session_status is confusing, maybe call it session_status_string ? Also, this should not return false, but maybe "Session status could not be determined"
I dont understand this : if (session_status() === 1){ $this->Session_Started = true; if session_status() == 1, then sessions are disabled, why would you set Session_Started to true ?
session_start() will return a boolean that indicates whether the session actually started, you should check that boolean instead of assuming success with $this->Session_Started = true;
session_start() also can resume a session, I am not a PHP master, but it seems to me you could replace init() with

public function init(){
    /* Start or Resume a session */
    $this->Session_Started = session_start();
    return $this->Session_Started;
}

Finally, if you are adamant about checking whether the session is started or not in set, would it not make sense to start the session for the caller, so that your code works auto-magically ?

public function Set ($Key = false, $Value){
  if (!isset($_SESSION)) {
    init();
  }
  $_SESSION[$Key] = $Value;
}

Also, as mini-tech commented, if you expect a session, and it is not there, then you might want to throw an exception instead of returning false.


Event emitter in JavaScript without using Node's built in class or any additional libraries I need to create this eventEmitter class with the functions listed below. I think I could clean the code a little but don't really know where to start.

var eventEmitter =function  (){
  this.listeners = 0;
  this.events = {};
  return this;
};

eventEmitter.prototype.on = function(ev, cb) {
  if (typeof ev !== 'string') throw new TypeError("Event should be type string", "index.js", 6);
  if (typeof cb !== 'function' || cb === null || cb === undefined) throw new TypeError("callback should be type function", "index.js", 7);

  if (this.events[ev]){
    this.events[ev].push(cb);
  } else {
    this.events[ev] = [cb];
  }

  this.listeners ++;
  return this;
};

eventEmitter.prototype.emit = function(eventType) {

  if (typeof eventType !== 'string') throw new TypeError("Event type should be type string", "index.js", 6);
  var handlerFunctions = this.events[eventType];

  if (handlerFunctions) {
    var self = this;

    for (var i = 0; i < handlerFunctions.length; i++) {
      var handler = handlerFunctions[i];

      if (arguments.length > 0) {
        var args = Array.prototype.slice.call(arguments).slice(1, arguments.length);
        handler.apply(self, args);
      } else{
        handler.call(self);
      }
    }
  }
  return this;
};

eventEmitter.prototype.off = function(eventType, handlers) {

  if ( arguments.length > 0 && (eventType === 'undefined' || typeof eventType !== 'string')) throw TypeError('listener must be a function');

  if (arguments.length > 1) {
    if ( typeof handlers !== 'function' || handlers === 'undefined') throw TypeError('handler must be a function string or object');
  }

  switch(arguments.length) {
    case 0:
      this.listeners = 0;
      this.events = {};
      break;
    case 1:
      if (this.events[eventType]) {
        this.listeners = this.listeners - this.events[eventType].length;
        delete this.events[eventType];
      }
      break;

    case 2:

      if (this.events[eventType]) {
        for (var i = 0; i < this.events[eventType].length; i++) {
           if (handlers.toString() == this.events[eventType][i].toString()){
             this.events[eventType].splice( i, 1 );
             this.listeners --;
             i --;
           }
        }
      }
      break;
  }
  return this;
};

module.exports = eventEmitter;

Constructors should be PascalCase, i.e. EventEmitter - not eventEmitter. Of course, EventEmitter is the exact name of Node's own implementation, so I'd pick something else.
Don't hardcode file and line number in errors. Those get added automatically, which is the whole point. And besides, if you hardcode them, you'll have to keep them up to date (line numbers are already misleading), etc.. It's interpreter-generated metadata, not data.
Also, this a minor thing but don't use "should" in an error description. You're throwing an error because, something must be something - not just because it ought to. So event names must be strings, and listeners must be functions.
Don't bother with maintaining the listeners count manually. You can just do this (ES6 syntax, but you can translate it):

Object.keys(this.events).reduce((count, key) => count + this.events[key].length, 0);
// => number of listener functions

You can add that as a getter function with Object.defineProperty or just have a getListenerCount method. If you add it as a method you can even choose to only get the count for a named event, i.e. getListenerCount('someEventName').
This:

if (typeof cb !== 'function' || cb === null || cb === undefined)

is very redundant. The only thing that matters is whether cb is a function. Doesn't matter if it's null or undefined - it's still not a function.
You don't need to return this if you intend to use a function as a constructor. You can, but you don't need to.

Here's a quick refactoring based on the points above:

function EventBase() {
  this.events = {};
};

EventBase.prototype = {
  on: function (event, listener) {
    if (typeof event !== 'string') throw new TypeError("Event must be a string");
    if (typeof event !== 'string') throw new TypeError("Listener must be a function");
    this.events[event] || (this.events[event] = []);
    this.events[event].push(listener);
  },

  off: function (event, listener) {
    if (arguments.length === 0) {
      // remove all listeners
      this.events = {};
      return;
    }

    if (!this.events[event]) {
      // return if there's no event by the given name
      return;
    }

    if (arguments.length === 1) {
      // remove all listeners for the given event
      delete this.events[event];
      return;
    }

    // remove specific listener
    this.events[event] = this.events[event].filter(function (func) {
      return func !== listener;
    });
  },

  emit: function (event) {
    if (!this.events[event]) {
      // return if there's no event by the given name
      return;
    }

    // get args
    var args = [].slice.call(arguments, 1);

    // invoke listeners
    this.events[event].forEach(listener => listener.apply(this, args));
  },

  getListenerCount: function (event) {
    // get total number of listeners
    if (arguments.length === 0) {
      return Object.keys(this.events).reduce((count, key) => count + this.getListenerCount(key), 0);
    }

    // return zero for non-existing events
    if (!this.events[event]) {
      return 0;
    }

    // return count for specific event
    return this.events[event].length;
  }
};


Wrapping IMemoryCache with SemaphoreSlim I need simple caching for one of my web-services so I use the Microsoft.Extensions.Caching.Memory package. I wrapped it with SemaphoreSlim to make sure reading and writing are safe. I'm not sure what I'm doing here is entirely correct. What do you think? Is this wrapper OK or can I turn GetOrCreateAsync into an extension for IMemoryCache and it'll handle everything?

public class CacheService
{
    private static readonly SemaphoreSlim Locker = new SemaphoreSlim(1, 1);

    private readonly IMemoryCache _cache;

    public CacheService(IMemoryCache cache)
    {
        _cache = cache;
    }

    public async Task<T> GetOrCreateAsync<T>(object key, Func<Task<T>> create)
    {
        await Locker.WaitAsync();
        try
        {
            if (_cache.TryGetValue(key, out var entry))
            {
                return (T)entry;
            }
            else
            {
                return _cache.Set(key, await create(), new MemoryCacheEntryOptions
                {
                    AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(17)
                });
            }
        }
        finally
        {
            Locker.Release();
        }
    }
}

Sorry this is a bit of a ramble, but a few things jump out at me:

17?!?! I don't have to tell you about magic numbers! This should probably be configurable... somehow...
Why is Locker static? It doesn't make sense to restrict access to one cache because a totally different cache is currently in use.
Locking the cache while you create an item isn't necessarily going to end well: it would be all too easy for create to try to access the cache, and now you have a deadlock. The 'easiest' way to resolve this would be to slightly abuse the cache, and instead of storing the values themselves store some wrapper which indicates whether the value is written yet with a different concurrency mechanism. This way, you can add a 'pending' value to the cache immediately, which can be initialised outside the lock.

This increases the complexity of dealing with create failures, because you have to communicate between threads with some other object, but now you can only dead-lock yourself by trying to access the object your are currently creating. It would be nice to detect this, but I think it might be impossible, so you probably want to give whatever mechanism awaits the pending entry a time-out. I don't think you can do better than this without passing the complexity off to the caller, which of course would ruin the nice API.

A better solution would probably be to provide a completely separate concurrency mechanism for handling 'pending' values, so that there is less overhead and complexity accessing the cache (presumably the more common operation). It wouldn't store the values, but rather just provide a mechanism to wait for them to appear in the proper cache (on a key-by-key basis).
I'd prefer that the cast to (T) was checked first, so that you can throw a highly-specific exception explaining that whatever was in the cache was not what the caller was expecting.
Since you are providing the access control, your class is probably also responsible for disposing the cache: it should provide this facility. Disposing Locker is also a concern. This adds to the opportunity for a consumer to get cryptic error messages if they are trying to access the cache as it is disposed.
Public APIs should have inline documentation (///), so that the maintainer knows what it's meant to do, and consumers can find out how to use them correctly without consulting the source-code.


Enforcing numeric inputs and sanitizing different formats for IE11 input fields I'm working on a web form that functions with numeric inputs. I have <input type="number"> input fields and with Chrome and Firefox things work nicely since they only allow numeric data to be filled in.

However, I need to support IE11 and it does not enforce the number type when inputting data (but does when submitting data).

Since I want to make it as easy as possible for users (who often copy-paste data from different sources), I wanna sanitize the numbers both on paste as well as on submit.

Things I'm achieving here:

Replacing spaces and commas as thousand-separators (100 000.25 -> 100000.25 and 100,000.25 -> 100000.25
Replacing commas a decimal pointer with a dot (100000,25 -> 100000.25)

How would you improve the code and are there some edge cases I'm forgetting?

function sanitizeValue(value) {
    return value.replace(/ /g, '')
                .replace(/\./g, ',')
                .replace(/[,](?=.*[,])/g, "")
                .replace(/,/g, '.')
}

var form = document.getElementById('form')
var inputs = document.getElementsByTagName('input')

form.onsubmit = function(event) {
    Object.values(inputs).forEach(function(input) {
        input.value = sanitizeValue(input.value)
    })
}

Object.values(inputs).forEach(input => {
    input.onpaste = ev => {
        ev.preventDefault()
        let pasteValue = ev.clipboardData.getData('text/plain')
        ev.target.value = sanitizeValue(pasteValue)
    }
})

Don't guess!
You should not attempt to correct client input unless you make sure that the client knows (via a click) that a change has been made.
Converting a value like 100,1.0 to 1001.0 is a guess. The client may have intended only the 100, or the 1.0 and your guess may not be noticed.
You can vet input as it is entered or on paste, removing non numeric characters (which you don't do) eg "a100a" will become 100, but when there is ambiguity like "a100a00" it should be considered not a number.
If the result is not a number use standard validation warnings rather than guessing.


Quadratic Primes - Project Euler 27 Project Euler Problem 27

  Euler discovered the remarkable quadratic formula:

  \$n^2 + n + 41\$

  It turns out that the formula will produce \$40\$ primes for the
  consecutive values \$n = 0\$ to \$39\$. However, when \$n = 40, 40^2 + 40 + 41 =
> 40(40 + 1) + 41\$ is divisible by \$41\$, and certainly when \$n = 41, 41^2 +
 41 + 41\$ is clearly divisible by \$41\$.

  The incredible formula \$n^2 − 79n + 1601\$ was discovered, which produces
  \$80\$ primes for the consecutive values \$n = 0\$ to \$79\$. The product of the
  coefficients, \$−79\$ and \$1601\$, is \$−126479\$.

  Considering quadratics of the form:

  \$n^2 + an + b\$, where \$|a| < 1000\$ and \$|b| < 1000\$

  where \$|n|\$ is the modulus/absolute value of \$n\$ e.g. \$|11| = 11\$ and \$|−4| = 4\$
  Find the product of the coefficients, \$a\$ and \$b\$, for the quadratic
  expression that produces the maximum number of primes for consecutive
  values of \$n\$, starting with \$n = 0\$.

My Implementation:

I used Sieve of Eratosthenes for finding primes. I also used a HashSet for fast searching.

static void Main()
{
    HashSet<int> primes = Primes(10000000);

    Func<int,int,int,int> f = (a,b,n) => n * n + a * n + b;

    int maxCount = int.MinValue;
    int x = 0;
    int y = 0;
    for(int a = -999;a < 1000;a++)
    {
        for(int b = -999;b < 1000;b++)
        {
            int count = 0;
            int n = 0;
            while(primes.Contains(f(a,b,n++))) count++;
            if(count > maxCount)
            {
                maxCount = count;
                x = a;
                y = b;
            }
        }
    }

    Console.WriteLine("x * y = {0}",x * y);
}

static HashSet<int> Primes(int n)
{
    HashSet<int> primes = new HashSet<int>();
    BitArray a = new BitArray(n+1,true);

    for(int i = 2;i <= Math.Sqrt(n);i++)
    {
        if(a[i])
        {
            for(int j = i * i;j <= n;j += i)
            {
                a[j] = false;
            }
        }
    }

    for(int i = 2;i < a.Length;i++)
    {
        if(a[i]) primes.Add(i);
    }
    return primes;
}

How can I improve this? Can you offer a more efficient implementation?

There is one relatively simple optimization you can do, for a start, and it will likely make a big difference...

If you analyze the equation:

n² + an + b, where |a| < 1000 and |b| < 1000

you need to solve it for where n² + an + b is prime.

The first n value to use is 0. 0² +a0 +b is b, thus, in order to process any values, it is clear that b needs to be a prime number (and not negative either...).

Thus, you should rearrange your loops, and your outer loop should be on the first k prime numbers in your set, where the k'th prime is < 1000.

There are 168 primes less than 1000, so that will reduce your loops from 2000 'b' loops to just 168.

Similarly, when you loop on only those 168 primes, you are guaranteed that the n == 0 result is prime, and you only need to test on the n > 0 condition.

Further, when n is 1,  the equation becomes:

1 + a + b = prime

or, since you have primes already from your sieve, you can significantly restrict the data-set by doing:

a = prime - b - 1

for primes in the range of b - 998 through to b + 1000 (which will keep a within the limits of -999 to +999). Note that there are no primes less than 2, and only a few hundred primes less than the 'worst case' (b ==999) + 1000

Only if you satisfy those two conditions would I consider even calling the prime function f(a,b,n++)


Suppress console output from React in Jest testing output but not in browser output I have a React app that I am testing with the built-in jest testing tools. In the process of debugging, I sometimes send data to the console (e.g. console.log, console.error, etc.) from my code-under-test. This shows up both in the browser console as well as in the testing output in the terminal which is what I sometimes want.

However, sometimes I want to send such debugging messages to either:

the testing terminal or
the browser,

but not both.

I know how to do #1: just include the console.log in the jest test code rather than in the code-under-test. However, if I want to do #2, I'm not quite sure what to do.

I tried the following in the code-under-test:

if (!jest) {
  console.log('some browser-only output');
}

That does suppress the output in the test, which otherwise behaves normally, i.e. no error is thrown during testing. However, it does not output anything in the browser but instead throws an error: "Uncaught ReferenceError: jest is not defined", i.e. there is no let jest = ... or const jest = ... anywhere in the code.

I did come up with a solution:

try { jest; }
catch(e) {
    console.log('some browser-only output');
}

This gives me the desired results. However, the solution feels very hack-y: It requires throwing an error in order to work, which just "smells" wrong, even for a debugging tool.

So, is there a better way for me to check whether jest exists without throwing an error? Or another way to see if testing is underway, again without throwing an error?

Hope this helps someone else:
jest --silent

http://facebook.github.io/jest/docs/en/cli.html#silent


Is there a better "Go" way to implement LC Add Two Numbers solution Problem Statement:

You are given two non-empty linked lists representing two non-negative integers. The digits are stored in reverse order, and each of their nodes contains a single digit. Add the two numbers and return the sum as a linked list.
You may assume the two numbers do not contain any leading zero, except the number 0 itself.

Implementation
func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode {
    return helper(l1, l2, 0)
}

func helper(left *ListNode, right *ListNode, carry int) *ListNode {

    if left == nil && right == nil {
        if carry > 0 {
            return &ListNode{
                Val: carry,
            }
        }
        return nil
    }

    if left == nil {
        r := right.Val + carry
        return &ListNode{
            Val:  r % 10,
            Next: helper(nil, right.Next, r / 10),
        }
    } else if right == nil {
        r := left.Val + carry
        return &ListNode{
            Val:  r % 10,
            Next: helper(left.Next, nil, r / 10),
        }
    }

    r := left.Val + right.Val  + carry
    return &ListNode{
        Val: r % 10,
        Next: helper(left.Next, right.Next, r / 10),
    }
}

```

Go does not have tail recursion: Wikipedia: Tail call. Use iteration rather than recursion.
Simplify your sprawling, hard-to-read code.

type ListNode struct {
    Val  int
    Next *ListNode
}

// Add Two Numbers
// https://leetcode.com/problems/add-two-numbers/
func addTwoNumbers(l1 *ListNode, l2 *ListNode) *ListNode {
    root := &ListNode{} // at end, start = root.Next
    end := root

    c := 0
    for {
        if l1 == nil && l2 == nil && c == 0 {
            break
        }

        v := c
        if l1 != nil {
            v += l1.Val
            l1 = l1.Next
        }
        if l2 != nil {
            v += l2.Val
            l2 = l2.Next
        }
        c = v / 10
        v = v % 10

        end.Next = &ListNode{
            Val:  v,
            Next: nil,
        }
        end = end.Next
    }

    return root.Next // start
}


Abstractness of Shape class Shape represents an abstract class and the other two classes inherits from it. This looks insufficiently abstract to me. How could I improve it?

import math
class Point:

    def __init__(self,x,y):
        self.x = x
        self.y = y

    def move(self,x,y):
        self.x += x
        self.y += y

    def __str__(self):
        return "<"+ str(self.x) + "," + str(self.y) + ">"

class Shape:
    def __init__(self, centrePoint, colour, width, height):
        self.centrePoint = centrePoint
        self.colour = colour
        self.width = width
        self.height = height
        self.type = "Square"

    def __init__(self, centrePoint, radius, colour):
        self.type = "Circle"
        self.radius = radius
        self.colour = colour
        self.centrePoint = centrePoint

    def move(self,x,y):
        self.centrePoint.move(x,y)

    def getArea(self):
        if (self.type == "Square"):
            return self.width * self.height
        elif (self.type == "Circle"):
            return math.pi*(self.radius**2)

    def __str__(self):
        return "Center Point: " + str(self.centrePoint) + "\nColour: "+ self.Colour + "\nType: " + self.type + "\nArea: " + self.getArea()

class Rectangle (Shape):

    def scale(self, factor):
        self.scaleVertically(factor)
        self.scaleHorizontally(factor)

    def scaleVertically(self, factor):
        self.height *= factor

    def scaleHorizontally(self, factor):
        self.width *= factor

class Circle (Shape):

    def scale(self, factor):
        self.radius * factor

Your class Shape knows about what kind of shape it is (circle/square). This makes it tied to the two child classes. It should not know any thing about the composition. Rather just provide methods with empty implementations that can be overridden by child classes.

The only thing I can see that is common across all classes is the concept of a center point and a color. So

import abc

class Shape(metaclass=abc.ABCMeta):
    def __init__(self, centrePoint, colour):
        self.centrePoint = centrePoint
        self.colour = colour

Since move depends only on center point, it is ok to define it in base class.

    def move(self,x,y):
        self.centrePoint.move(x,y)

This is certainly one that should be implemented by the child classes because it requires knowledge about what kind of a shape it is.

    @abc.abstractmethod
    def getArea(self):
        return

or if you dont want to use it

    def getArea(self):
        raise NotImplementedError('Shape.getArea is not overridden')

see
abc in
so

And the to string method is another that should be done in child because it holds details of what shape it is.

    @abc.abstractmethod
    def __str__(self):
        return

However, if you are going to use the same format for all shapes, then you can provide the default implementation (as below.)

    def __str__(self):
        return "Center Point: " + str(self.centrePoint) + "\nColour: "+ self.Colour + "\nType: " + self.type() + "\nArea: " + self.getArea()

I would also recommend scale as part of the base class because all shapes can be scaled.

    def scale(self, factor):
        return

Basically all operations that make sense on any shapes should have an abstract method on the base class.

Now for child classes

class Rectangle (Shape):
    def __init__(self, cp, color, width, height):
        super(Rectangle, self).__init__(cp, color)
        self.width = width
        self.height = height

    def scale(self, factor):
        self.scaleVertically(factor)
        self.scaleHorizontally(factor)

    def scaleVertically(self, factor):
        self.height *= factor

    def scaleHorizontally(self, factor):
        self.width *= factor

    def getArea(self):
        return self.width * self.height

    def type(self):
        return "Rectangle"

class Circle (Shape):
    def __init__(self, cp, color, radius):
        super(Rectangle, self).__init__(cp, color)
        self.radius = radius

    def scale(self, factor):
        self.radius * factor

    def getArea(self):
        return math.pi*(self.radius**2)

    def type(self):
        return "Circle"


Finding the common iterator category in C++ I created a template trait which finds the common iterator category between a set of given iterator categories. More specifically, it's a class with a member type alias to one of the iterator tags which is the most efficient for the specific requirement set that is implied by both categories given as template arguments (the least "powerful" in the iterator hierarchy).

For example, given std::forward_iterator_tag, std::random_access_iterator_tag and std::bidirectional_iterator_tag, it provide std::forward_iterator_tag.

If all the given types are std::output_iterator_tag then it will provide std::output_iterator_tag. Otherwise, there is no member type alias. (std::output_iterator_tag must be treated differently because it is not in the input iterator hierarchy.)

#include <algorithm>
#include <iterator>
#include <tuple>
#include <type_traits>

namespace detail {
    // Gets the index of the first occurrence of a type in a tuple (inverse of std::tuple_element_t).
    template <typename T, class Tuple>
    struct tuple_type_index;

    // Specialisation for match on parameter pack's first type.
    template<typename T, typename... Ts>
    struct tuple_type_index<T, std::tuple<T, Ts...>> : std::integral_constant<std::size_t, 0> {};

    // Specialisation for no match on parameter pack's first type.
    template<typename T, typename U, typename... Ts>
    struct tuple_type_index<T, std::tuple<U, Ts...>> : std::integral_constant<std::size_t, 1 + tuple_type_index<T, std::tuple<Ts...>>::value> {};

    // Helper variable template.
    template<typename T, class Tuple>
    constexpr std::size_t tuple_type_index_v = tuple_type_index<T, Tuple>::value;

    // Hierarchy of input iterator categories (output iterator category is handled separately) from least to most powerful.
    using iterator_categories = std::tuple<std::input_iterator_tag, std::forward_iterator_tag, std::bidirectional_iterator_tag, std::random_access_iterator_tag>;
}

// Primary template (for 0 categories).
template<class... IteratorCategories>
struct common_iterator_category {};

// Helper type alias.
template<class... IteratorCategories>
using common_iterator_category_t = typename common_iterator_category<IteratorCategories...>::type;

// Specialisation for 1 category.
template<class IteratorCategory>
struct common_iterator_category<IteratorCategory> {
    using type = IteratorCategory;
};

// Specialisation for 2 non-output-iterator categories.
template<class IteratorCategory1, class IteratorCategory2>
struct common_iterator_category<IteratorCategory1, IteratorCategory2> {
    using type = std::tuple_element_t<
        std::min(
            detail::tuple_type_index_v<IteratorCategory1, detail::iterator_categories>,
            detail::tuple_type_index_v<IteratorCategory2, detail::iterator_categories>),
        detail::iterator_categories>;
};

// Specialisation for 2 output iterator categories.
template<>
struct common_iterator_category<std::output_iterator_tag, std::output_iterator_tag> {
    using type = std::output_iterator_tag;
};

// Specialisation for 3+ categories.
template<class IteratorCategory1, class IteratorCategory2, class... IteratorCategories>
struct common_iterator_category<IteratorCategory1, IteratorCategory2, IteratorCategories...> :
    common_iterator_category<common_iterator_category_t<IteratorCategory1, IteratorCategory2>, common_iterator_category_t<IteratorCategories...>> {};

The code with tests/examples is live here.

I am specifically interested in feedback with regards to the conciseness/efficiency of the design, and also whether it is well formed (I am worried about UB but "happens to work" or no diagnostic required issues). I'm not so much concerned with the tuple_type_index utility (it's just there because it happens to be required). But feedback of any sort is of course welcome.

If you have a typelist of iterator tag types, then std::common_type will find the common base type.

using InTag = std::istream_iterator<int>::iterator_category;
using BdTag = std::list<int>::iterator::iterator_category;
using RaTag = std::vector<int>::iterator::iterator_category;

static_assert(std::is_same_v<std::common_type_t<InTag, BdTag>, std::input_iterator_tag>);
static_assert(std::is_same_v<std::common_type_t<InTag, RaTag>, std::input_iterator_tag>);
static_assert(std::is_same_v<std::common_type_t<BdTag, RaTag>, std::bidirectional_iterator_tag>);

If you are unwrapping the tag type at the call site, just have the trait do it.

template <class... Iterators>
struct common_iterator_tag
: std::common_type<typename std::iterator_traits<Iterators>::iterator_category...>
{
};

template <class... Iterators>
using common_iterator_tag_t = typename common_iterator_tag<Iterators...>::type;

using RaIter = std::vector<int>::iterator;
using InIter = std::istream_iterator<int>::iterator;

static_assert(std::is_same_v<std::input_iterator_tag, common_iterator_tag_t<RaIter, InIter>>);

using BackIter = std::back_insert_iterator<std::vector<int>>;
using InsertIter = std::insert_iterator<std::list<int>>;

static_assert(std::is_same_v<std::output_iterator_tag, common_iterator_tag_t<BackIter, InsertIter>>);


Vertical Bar Graph Generator I have created a python program that prints a vertical bar graph made from fields and values inputted by the user. It works by first creating a horizontal bar graph and then going through each layer and determining whether to print a letter, a "|", a "_", or a space. I would appreciate any feedback on the program and things I could do to improve it.

import re

feilds = input("Input| ex: dogs,10 cats,23 birds,67\n").strip()
while not re.match("^(\w+,\d+ ?)+$", feilds):
  feilds = input("invalid input, use this format:\nfeild,value feild,value feild,value...\n").strip()
width = int(input("How wide is your output window in characters? "))
scale = float(input("Scale: "))
vals = dict([feild.split(",") for feild in feilds.split(" ")])
for k, v in vals.items():
  vals[k] = int(v)
spacing = int(width / len(vals.keys()))
horizontal = ["-" * int(v * scale) + "|" + k[::-1] for k, v in vals.items()]
vertical = []
for x in reversed(range(len(max(horizontal, key=len)))):
  layer = []
  for val in horizontal:
    try:
      if val[x] == "-":
        layer.append("|")
      elif val[x] == "|":
        layer.append("_")
      else:
        layer.append(val[x])
    except IndexError:
      layer.append(" ")
  vertical.append(layer)
print("\n")
print("\n".join((" " * (spacing - 1)).join(layer) for layer in vertical))

ex:

input:

dogs,20 cats,18 fish,25 birds,10
80
.5

output:

                                        f
                                        i
d                                       s
o                   c                   h
g                   a                   _
s                   t                   |
_                   s                   |                   b
|                   _                   |                   i
|                   |                   |                   r
|                   |                   |                   d
|                   |                   |                   s
|                   |                   |                   _
|                   |                   |                   |
|                   |                   |                   |
|                   |                   |                   |
|                   |                   |                   |
|                   |                   |                   |

NOTE: output might be messed up if the width is off by a lot. Make sure to input a width close to what it is.

feilds is spelled fields. IDEs like PyCharm will identify spelling mistakes in variables.

Why is there a ? after the space in your regex? Currently I believe it will pass input looking like

key,1key,2key,3

So you have to fix that edge case, probably by adding a final key-value pair with no space, and making the space in the first group mandatory.

This:

vals = dict([feild.split(",") for feild in feilds.split(" ")])
for k, v in vals.items():
    vals[k] = int(v)

should be

vals = {k: int(v)
        for field in fields.split(' ')
        for k, v in field.split(',')}

i.e. don't construct a list in memory only to throw it away.

This:

spacing = int(width / len(vals.keys()))

can be

spacing = width // len(vals)

This:

for x in reversed(range(len(max(horizontal, key=len)))):

doesn't actually need an index at all, since you don't use it. Also, lumping everything into one line is confusing. Instead:

longest_horz = max(len(h) for h in horz)
for v in val[longest_horz-1::-1]:

Then use v instead of val[x].


LeetCode: Binary Tree Zigzag Level Order Traversal C# https://leetcode.com/explore/interview/card/top-interview-questions-medium/108/trees-and-graphs/787/

  Given a binary tree, return the zigzag level order traversal of its
  nodes' values. (ie, from left to right, then right to left for the
  next level and alternate between).

  For example:

Given binary tree [3,9,20,null,null,15,7],
    3
   / \
  9  20
    /  \
   15   7

  return its zigzag level order traversal as:

[
  [3],
  [20,9],
  [15,7]
]

/// <summary>
/// https://leetcode.com/explore/interview/card/top-interview-questions-medium/108/trees-and-graphs/787/
/// </summary>

[TestClass]
public class ZigzagLevelOrderTest
{
    [TestMethod]
    public void TestZigZag()
    {
        /*     3
              / \
             9  20
                /  \
               15   7
          */
        TreeNode root = new TreeNode(3);
        root.left = new TreeNode(9);
        root.right = new TreeNode(20);
        root.right.left = new TreeNode(15);
        root.right.right = new TreeNode(7);
        IList<IList<int>> res = ZigzagLevelOrderClass.ZigzagLevelOrder(root);
        CollectionAssert.AreEqual(new List<int> { 3 }, res[0].ToList());
        CollectionAssert.AreEqual(new List<int> { 20, 9 }, res[1].ToList());
        CollectionAssert.AreEqual(new List<int> { 15, 7 }, res[2].ToList());
    }

    [TestMethod]
    public void FailedTest()
    {
        /*     1
              / \
             2   3
            /     \
            4      5
             */

        TreeNode root = new TreeNode(1);
        root.left = new TreeNode(2);
        root.right = new TreeNode(3);
        root.left.left = new TreeNode(4);
        root.right.right = new TreeNode(5);
        IList<IList<int>> res = ZigzagLevelOrderClass.ZigzagLevelOrder(root);
        CollectionAssert.AreEqual(new List<int> { 1 }, res[0].ToList());
        CollectionAssert.AreEqual(new List<int> { 3, 2 }, res[1].ToList());
        CollectionAssert.AreEqual(new List<int> { 4, 5 }, res[2].ToList());
    }

}

public class ZigzagLevelOrderClass
{
    public static IList<IList<int>> ZigzagLevelOrder(TreeNode root)
    {
        List<IList<int>> result = new List<IList<int>>();
        if (root == null)
        {
            return result;
        }
        Stack<TreeNode> currentLeveL = new Stack<TreeNode>();
        Stack<TreeNode> nextLevel = new Stack<TreeNode>();
        currentLeveL.Push(root);
        while (currentLeveL.Count > 0 || nextLevel.Count > 0)
        {
            var nodes = new List<int>();
            while (currentLeveL.Count > 0)
            {
                var curr = currentLeveL.Pop();
                if (curr.left != null)
                {
                    nextLevel.Push(curr.left);
                }

                if (curr.right != null)
                {
                    nextLevel.Push(curr.right);
                }
                nodes.Add(curr.val);
            }

            if (nodes.Count > 0)
            {
                result.Add(nodes);
            }
            nodes = new List<int>();
            while (nextLevel.Count > 0)
            {
                var curr = nextLevel.Pop();
                if (curr.right != null)
                {
                    currentLeveL.Push(curr.right);
                }
                if (curr.left != null)
                {
                    currentLeveL.Push(curr.left);
                }
                nodes.Add(curr.val);
            }

            if (nodes.Count > 0)
            {
                result.Add(nodes);
            }
        }

        return result;
    }
}

Please review for performance. this is the second time I solved this question.
I think I did a better job now.

Your implementation is close, but it can be a bit shorter. You correctly use two stacks, but you duplicate the code alternating between using the two. That's a bit of a waste of space. Instead, at the end of the first while loop, you can just assign nextLevel to currentLevel, create a new stack to nextLevel, and repeat:

    while (currentLeveL.Count > 0 || nextLevel.Count > 0)
    {
        var nodes = new List<int>();
        while (currentLeveL.Count > 0)
        {
            var curr = currentLeveL.Pop();
            if (curr.left != null)
            {
                nextLevel.Push(curr.left);
            }

            if (curr.right != null)
            {
                nextLevel.Push(curr.right);
            }
            nodes.Add(curr.val);
        }

        if (nodes.Count > 0)
        {
            result.Add(nodes);
        }
        currentLevel = nextLevel;
        nextLevel = new Stack<TreeNode>();
    }

The while condition can also be made easier, nextLevel is known to be empty when it is evaluated:

while (currentLeveL.Count > 0)

Since you're looping over the stack, only removing each item, not adding items back or anything, you can just replace the while for a foreach:

public static IList<IList<int>> ZigzagLevelOrder(TreeNode root)
{
    List<IList<int>> result = new List<IList<int>>();
    if (root == null)
    {
        return result;
    }
    Stack<TreeNode> currentLeveL = new Stack<TreeNode>();
    Stack<TreeNode> nextLevel = new Stack<TreeNode>();
    currentLeveL.Push(root);
    while (currentLeveL.Count > 0)
    {
        var nodes = new List<int>();
        foreach(var curr in currentLeveL)
        {
            if (curr.left != null)
            {
                nextLevel.Push(curr.left);
            }

            if (curr.right != null)
            {
                nextLevel.Push(curr.right);
            }
            nodes.Add(curr.val);
        }

        if (nodes.Count > 0)
        {
            result.Add(nodes);
        }
        currentLeveL = nextLevel;
        nextLevel = new Stack<TreeNode>();
    }

    return result;
}