Bouncing ball - for swing event handling tutorial This is a sample program that I intend to post as part of a series of beginner level Java tutorials.  Please provide any feedback on improvements that would make example more clear or illustrate/emphasize best practices.  The example drawns a ball object to a panel on a mouse click and then moves it randomly with another mouse click.

Class BouncingBall

import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;

/*
 * this class will add a red ball to a canvas or play area, when a button is clicked and
 * then move about randomly when another button is clicked
 *
 * jmergenthaler 10/1/2011
*/

public class BouncingBall {

private JFrame frame = new JFrame();
private JPanel actionPanel = new JPanel();
private JPanel playarea = new JPanel();
private JButton btnNew = new JButton("Add Red Ball");
private JButton movebtn = new JButton("Move it");

//constructor
BouncingBall(){
    buildTheGUI();
}

public void buildTheGUI(){
    frame.setLayout( new BorderLayout());
    btnNew.addActionListener( new ButtonClickHandler() );
    movebtn.addActionListener( new MoveButtonClickHandler() );
    actionPanel.add(btnNew);
    actionPanel.add(movebtn);
    frame.add(BorderLayout.NORTH,actionPanel);
    frame.add(BorderLayout.SOUTH,playarea);
    frame.setSize(500, 500);
    frame.setVisible(true);
}

public static void main(String args[]){
    new BouncingBall();
}

class ButtonClickHandler implements ActionListener{
    public void actionPerformed(ActionEvent e){

        //create initial Ball object add to the frame
        frame.add(new Ball() );
        //draw
        frame.validate();
    }
}

class MoveButtonClickHandler implements ActionListener{
    public void actionPerformed(ActionEvent e){

        //polymorphic behavior, calling the Ball constructor differently
        frame.add(new Ball(1) );
        //redraw
        frame.validate();
    }
}// end class MoveButtonClickHandler
}//end class BouncingBall

class 2 - the Ball class

import java.awt.Color;
import java.awt.Graphics;
import java.util.Random;
import javax.swing.JPanel;

class Ball extends JPanel{

//private instance variables avail. only avail to methods in this class
private int x,y,w,h;

//constructor
Ball(){
    this.x = 200;
    this.y = 200;
    this.w = 100;
    this.h = 100;
}

//constructor with different behavior
Ball(int a){
    Random rand = new Random();

    this.w = 100;
    this.h = 100;
    this.x = rand.nextInt(300);
    this.y = rand.nextInt(300);
}

//draw the ball
//@override
public void paintComponent(Graphics g){
    super.paintComponent(g);
    g.setColor(Color.RED);
    g.fillOval(x, y, h, w);
}

}//end class Ball

Swing GUIs should be built inside the EDT, e.g. using SwingWorker.invokeLater(). See http://leepoint.net/JavaBasics/gui/gui-commentary/guicom-main-thread.html for details.
I think calling a JPanel descendant "Ball" is just confusing. If you really want that a panel can handle only one Ball, call it BallPanel or so. But it would be more flexible if you had a DrawPanel, which accepts a number of objects to draw, and the objects themself. That makes it much easier to extend the system later:

.

public class DrawPanel {
   private List<Drawable> drawables = new ArrayList<Drawable>();
   ...
   public void addDrawable(Drawable d) { drawables.add(d); }

   public void paintComponent(Graphics g){
     super.paintComponent(g);
     for(Drawable d : drawables) {
        d.draw(g, getWidth(), getHeight());
     }
   }
   ...
   //for animation a timer task, calling Drawable.update
}

interface Drawable {
   public void draw(Graphics g, int width, int height);

   //when you need animation
   public void update(long ms);
}

public class Ball implements Drawable {
  ...
}

This is only one possibility to split view and model, the "right" way depends on your needs. But keeping both model and view in one class is a receipt for trouble.


Nesting of paragraph with headings and cite with dynamic content in a VueJS app Background

I added a few items to a codepen that displayed a list of beers. One of them was the citation of the brewer who gave tips. Previously the tips were just generated into a
 but now I have wrapped it inside a blockquote

Code

    Tips from the brewer:
    "{{ beer.tips }}"

      &ndash; {{ beer.contributor }}
      <a style="color: white" :href="`https://www.twitter.com/${beer.twitter}`" target="_blank">{{beer.twitter}}

Demo

My working demo is available on CodePen

I don't know Vue.js then I won't comment about that part.

Overall structure seems OK but maybe few things may be improved. The most obvious one is usage of CSS classes to apply styling without conveying any extra semantic. In your code bright class is applied to text to make it visually distinguishable but:

Name describes the applied style instead of its meaning. If you later decide to use dimmed text then you also need to change class name (or to keep them out-of-sync).
It does not help screen readers to understand what its content is: effectively just a sub-title.
It's applied both to
,  and...
 reinforcing its usage as mere styling instead of carrying any semantic value.

For the tagline I'd use a subheader or a specific class:

{{ beer.name }}

{{ beer.tagline }}

Alternatively:

{{ beer.name }}

{{ beer.tagline }}

Note that you can use mixins to avoid CSS duplication then there is no reason to re-use the same class multiple times. Note that you're using
 for the outer container and there it has little meaning, you may refactor your HTML code to be more HTML5ish:

  ...

Loading...

      <hgroup>

{{ beer.name }}

{{ beer.tagline }}

      </hgroup>

Note that I'm using  as outer container and
 with its own <hgroup>. What's for? Screen readers (and search engines) now know what each item is, the element conveys a semantic value and you can, also, directly use it in your CSS. Image is actually a side content of the
 then you can use .

Refactored this we should look at the other bright elements. Your paragraphs are some text with a title. Exactly what
 is for:

Description:

{{ beer.desc }}

Tips from the brewer:

...

We finally arrived to the
. In this case you're quoting a small inline tweet, also note that usually
 does not contain any extraneous content (besides a  with the  element). What I think is more appropriate for this citations (which are not a separate standalone block of text) is the <q> element:

    <q cite="...">{{ beer.tips }}</q>

Few important things here: quotation marks should not be part of your HTML, browser already does it (using the appropriate symbols for the current language - which BTW you should add to your HTML). Also note the cite attribute to reference the source (if it's a twitter than you have an URL to link).  element can follow inside
.

You can (and probably should) style
 directly without using bright. You may also like (if required by your CSS) to wrap each section with
 (in this case you may have <hgroup> instead of the
 list).

You may consider to use microdata for some of your content (beer name and author of the tweet).

JS local variables in addBeer() can be const instead of let.

I see absolutely no reason (for this kind of content) to do not support accessibility. It's more complex than one paragraph in this questionthen just to refer to official W3C documentation.


Converting Date object in CoffeeScript to string (with a weird format) I'm trying to convert a Date object into a weird format, like so:

03/30/2017 11:37:14:082 -0500

The issue that really trips me up is the timezone offset. There's no way that screams at me as easy to do, so here's my implementation (in coffeescript, hopefully it's easy enough to understand without intimate knowledge of coffeescript):

  timeStamp = new Date()
  offsetHours = timeStamp.getTimezoneOffset() / 60 * 100
  if offsetHours <= -1000
    offsetString = "+" + Math.abs(offsetHours)
  else if offsetHours == 0
    offsetString = "+0000"
  else if offsetHours < 0 and offsetHours > -1000
    offsetString = "+0" + Math.abs(offsetHours)
  else if offsetHours >= 1000
    offsetString = "-" + offsetHours
  else
    offsetString = "-0" + offsetHours
  timeStampString = (timeStamp.getMonth() + 1) + '/' + timeStamp.getDay() +
    '/' + timeStamp.getFullYear() + " " + timeStamp.getHours() + ":" +
    timeStamp.getMinutes() + ":" + timeStamp.getSeconds() + ":" +
    timeStamp.getMilliseconds() + " " + offsetString

I realize my implementation isn't handling time-zones that have 30 minute offsets (which will get fixed) but I am looking to see if anyone can poke any other holes they see here. This code kind of smells to me and I feel like there should be an easier way to accomplish this.

I've spoofed the date prototype for JS to test a variety of time-zones and haven't found anything weird yet. This is to support interop with another application so "use a different scheme" unfortunately isn't an option. I'd like to avoid using libraries as well.

First of all, your given sample features zero-padded 2-digit months and milliseconds which you currently don't reproduce.

The mentioned 'smell' is probably caused by

the 'manual' zero-padding of the timezone offset and its many if-else branches
the many hardcoded numbers and strings
the long illegible string concatenation

We can get rid of the self-made zero-padding by using String.padStart() or - if compatibility is a concern - one of its many alternative implementations.

We can't really get rid of the many hardcoded constants, but we can name them to make them meaningful and less magic in appearance:

const seperators = ['/', '/', ' ', ':', ':', ':', ' ', '', '', ''];

We can then replace the large string concatenation expression by leveraging one of those array's reduce or map method:

function formatDate(date) {
  const values = [
    date.getMonth() + 1,
    date.getDate(),
    date.getFullYear(),
    date.getHours(),
    date.getMinutes(),
    date.getSeconds(),
    date.getMilliseconds(),
    date.getTimezoneOffset() > 0 ? '-' : '+',
    Math.abs(date.getTimezoneOffset() / 60),
    Math.abs(date.getTimezoneOffset() % 60)
  ];
  const digits = [2, 2, 4, 2, 2, 2, 3, 0, 2, 2];
  const seperators = ['/', '/', ' ', ':', ':', ':', ' ', '', '', ''];

  return values.map((value, i) =>
    value.toString().padStart(digits[i], '0') + seperators[i]
  ).join('');
}

console.log(formatDate(new Date()));

That's it. I don't see how we can leverage any other built-in method to shorten above task. Using JavaScript's built-in Date.toLocaleTimeString('en-US', options) gets us pretty far, but it is missing timezone offset and millisecond options and introduces unwanted separators:

const options = {
  year: 'numeric',
  month: '2-digit',
  day: '2-digit',
  hour: '2-digit',
  minute: '2-digit',
  second: '2-digit',
  hour12: false
};

console.log(new Date().toLocaleString('en-US', options));

PS: Above code should translate pretty easily to coffeescript, but as I am not very well versed in that language, I chose to give code samples in vanilla JS.

Edit: Instead of getDay() you probably want to use getDate().


Constructing a cumulative distribution function from a histogram During some running process, a histogram of values shall be build up. When done, the CDF shall be derived from it and used to get the quantiles for some values.

My current implementation looks as follows:

import java.util.*
import kotlin.math.max
import kotlin.math.min
import kotlin.math.roundToInt
import kotlin.random.Random

// 10 buckets for interval [0, 1000], outliers will be clamped
class Histogram(
    private val data: MutableMap<Int, Int> = mutableMapOf()
) {
    fun add(value: Int) {
        val bucket = max(0, min(1000, value)) / 100
        data[bucket] = data.getOrDefault(bucket, 0) + 1
    }

    fun deriveCDF(): CDF {
        val sum = data.values.sum()
        val pdfData = data.toSortedMap().mapValues { it.value.toDouble() / sum }
        val cdfData: MutableMap<Int, Double> = mutableMapOf()
        var acc = 0.0
        pdfData.forEach {
            acc += pdfData.getOrDefault(it.key, 0.0)
            cdfData[it.key] = acc
        }
        return CDF(cdfData.toSortedMap())
    }
}

// cumulative distribution function
class CDF(
    private val data: SortedMap<Int, Double>
) {
    fun getQuantile(value: Double): Double? {
        val bucket = (10 * value).roundToInt()
        return data[bucket]
    }
}

fun main() {
    val hist = Histogram()
    List(10000) {
        Random.nextInt(0, 1000)
    }.forEach {
        hist.add(it)
    }
    val cdf = hist.deriveCDF()
    println(cdf.getQuantile(0.3))
}

My problem with this is, that is it possible to construct instances of CDF with SortedMaps as data that don't make sense. I would like to prevent this, already at compile time.

The obvious alternative approach is to not have the construction logic (Histogram to CDF) in a Histogram::deriveCDF, but instead have a constructor of CDF take a Histogram instance and do the math there. This however forces Histogram to expose its internals data: MutableMap<Int, Int>, i.e., make them public, for the constructor of CDF to access them. But I would like to keep this implementation detail of Histogram hidden.

How can I escape this dilemma?

Your problem starts here:

val cdfData: MutableMap<Int, Double> = mutableMapOf()

This is the data that you want to your CDF class, but anyone can make a mutable map anywhere and use it for tons of things. So this is what you want to hide. So put this inside a CDFBuilder

class CDFBuilder {
     private val cdfData: MutableMap = mutableMapOf<Int, Double>()

     fun withData(a: Int, b: Double): CDFBuilder {
          // verification and stuff
          cdfData[a] = b
          return this
     }
}

Now, the problem is to build the CDF class without letting anyone else do it, right? There are a few options here:

Make a copy of the cdfData public from the CDFBuilder and read it in the CDF constructor, passing a CDFBuilder to the constructor.

Or 2, make a private constructor on the CDF-class, allowing it to only be constructed from the builder:

class CDF private constructor(val map: SortedMap<Int, Double>) {

    class CDFBuilder {
        private val cdfData: MutableMap = mutableMapOf<Int, Double>()

        fun withData(a: Int, b: Double): CDFBuilder {
            // verification and stuff
            cdfData[a] = b
            return this
        }

        fun build(): CDF {
            return CDF(map.toSortedMap())
        }
    }
}


Disco Dancefloor Mini-Game I was told to have my code reviewed here as I am a total beginner to Python.
I made a small Tkinter script that generates a 4x4 field of buttons and randomly recolors them when clicking on any button, but I was told that the code is not clean. If someone could point out a few flaws and some possible enhancements that would relly help a lot.

from tkinter import *
from random import randint

window = Tk()
window.title("Test")
window.geometry('200x200')
color = ["red","blue","green","yellow","black","purple","orange"]
RandInt = 0
j = 0
h = 0

def ButtonDef(xvar = 0,yvar = 0):
    btn = Button(window,command =lambda:[RandomColor()])
    btn.grid()
    btn.place(x = xvar*50, y = yvar*50, width = 50, height = 50)

    def RandomColor():
        for child in window.winfo_children():
            child.configure(bg=color[randint(0,6)])

while j in range (4):
    i = 0
    j += 1
    while i in range (4):
        ButtonDef(i,h)
        i += 1
        if i == 4:
            h += 1
window.mainloop()

One of the first useful things to take in consideration when developing in Python (whether you are a beginner or expert) is the PEP 8 that provides general rules how to write a code that can help other developers to read your code.

For example, from the naming conventions section you can read that variable names are written in snake_camel case. This means RandInt should be re-written as rand_int and ButtonDef() RandomColor() should be written button_def() and random_color() respectively.
In the imports section, you can read that we should avoid wild card imports. This means we should write import tkinter as tk instead of from tkinter import *

Names should be meaningful and express their purpose. This means ButtonDef(), which is rather a misleading name, could be named create_buttons() instead because you are creating 16 button widgets with that function. I would also rename RandomColor() by something like get_random_color() or pick_random_color() because a function does something, so it is better the name gets prefixed with a verb. Also, color is actually referencing several colors, so it should be named colors instead or, why not, colors_list because it consists of a list of colors.
General code formatting rules:

When affecting a value to a variable, we should leave one space character around both sides of the affectation symbol =. But this does not apply to tkinter when it comes to a widget's argument. Let me take an example to clarify this: btn = Button(window,command =lambda:[RandomColor()]) should be written btn = Button(window,command=lambda:[RandomColor()]) (changed ...command =lambda:... to ...command=lambda:...)
In this line: while j in range (4): we should remove the space in range (4): and write range(4): instead.

There are two main suggestions:

I would write your code in an OOP approach. No wonder that OOP and GUI coincided in birth historically.
Ideally, whenever we create widgets, we would keep a reference to them for later reuse or code invocation.  In your code, btn is overridden several 16 times so, by the end, it refers only to the last created button widget; this means you can not access easily ([winfo_children()][3] offers a way to access the child widgets of window but it is hard to have a friendly control on them)

When you have to deal with several widgets and manage them through different lines and columns, grid()  will make your life much easier than place().

Based on all the elements previously mentioned, and based on tkinter best practices article, I would re-rewrite your code as follows:

import tkinter as tk
import random

class ChangeButtonsColor(tk.Frame):

   def __init__(self, master):
       self.master = master
       tk.Frame.__init__(self, self.master)
       self.__colors = ["red","blue","green","yellow","black","purple","orange"]
       self.configure_gui()
       self.create_widgets()

   def configure_gui(self):
       self.master.title('Test')
       self.master.geometry('200x200')

   def create_widgets(self):
       self.create_buttons()

   def create_buttons(self):
       self.buttons = {}
       for i in range(4):
           for j in range(4):
               self.buttons['button{}'.format(4*i + j)] = tk.Button(self.master, bg=self.get_random_color())
               self.buttons['button{}'.format(4*i + j)].grid(row=i, column=j)
               self.buttons['button{}'.format(4*i + j)].config(width=3, height=3)
               self.buttons['button{}'.format(4*i + j)].config(command=self.change_buttons_color)

   def get_random_color(self):
       return random.choice(self.__colors)

   def change_buttons_color(self):
       for i in range(16):
           self.buttons['button{}'.format(i)].config(bg=self.get_random_color())

if __name__ == '__main__':
   root = tk.Tk()
   main_app =  ChangeButtonsColor(root)
   root.mainloop()

Note that in I created a dictionary to keep references to each button I created in create_buttons() function. The way to access each button is demonstrated in change_buttons_color()


Printing concentric squares of numbers I wrote a code which prints the following pattern :

5 5 5 5 5 5 5 5 5
5 4 4 4 4 4 4 4 5
5 4 3 3 3 3 3 4 5
5 4 3 2 2 2 3 4 5
5 4 3 2 1 2 3 4 5
5 4 3 2 2 2 3 4 5
5 4 3 3 3 3 3 4 5
5 4 4 4 4 4 4 4 5
5 5 5 5 5 5 5 5 5

The code is as follows :

#include <stdio.h>

void main(){
   int n,i,k,b,a;
   printf("ENTER OUTER NUMBER:");
   scanf("%d",&n);
   for(i=0;i<2*n-1;i++){
     printf("%d",n);
   }
   printf("\n");
   int p=n;
   for(i=0;i<n-1;i++){
     int a=0;
     for(k=0;k<=i;k++){
        printf("%d",n-a);
        a++;
     }
     for(b=0;b>i;b++){
        printf(" ");
     }
     for(k=1;k<=2*p-3;k++){
        printf("%d",n-i-1);
     }
     p--;
     for(k=i;k>=0;k--){
        printf("%d",n-k);
     }
     printf("\n");
   }
    int q=1;
    for(i=0;i<n-2;i++){
     for(k=n;k>i+1;k--){
        printf("%d",k);
     }
     for(k=1;k<=2*q-1;k++){
        printf("%d",q+1);
     }
     q++;
     for(k=2+i;k<=n;k++){
        printf("%d",k);
     }
     printf("\n");
   }
   for(i=0;i<2*n-1;i++){
     printf("%d",n);
   }
   getchar();
}

I printed the first and last line independently.Then,I divided the pattern into two halves and considered some common triangular patterns as follows:

5                 5    2 3 4 5
5 4             4 5      3 4 5
5 4 3         3 4 5        4 5
5 4 3 2     2 3 4 5          5

The code ran well in the CODE BLOCKS IDE.But, I think it's too long and I complicated the coding.My query is:

Can I shorten the code with the similar logic of breaking the pattern into parts or mine is ok ?
Is there any alternative to this ?

Use a bit more spaces. It will make your code more readable.
Try to be coherent with your indents
You should sanitize input. What happen if user enter a number greater than 9 ? Or 0, 1, or 2? Or negative number? Or not a number? Never trust user, They all try to broke your program.
When you just print on char, prefer putchar() instead of printf().

Here's my solution, maybe not the most optimal, but clean and short. I compute the distance from the current coordinate to nearest side and then remove it to the base number.

#include <stdio.h>
int main(void) {
    int n = 5;
    int a;
    int b;
    const int m = 2*n-1;
    for (int i = 0; i < m; ++i) {
        for (int j = 0; j < m; ++j) {
            a = (i >= n) ? m - i - 1: i;
            b = (j >= n) ? m - j - 1 : j;
            putchar('0' + n - ((a < b) ? a : b));
        }
        putchar ('\n');
    }
    return 0;
}

To improve this, you could split the inner loop to avoid the two conditional assignments. I let you trying to do it yourself.


'100' is a magic number Magic numbers are bad... I totally agree. But there's one magic number I find hard to fix:

  '100' is a magic number.

Consider this code:

public double getPercent(double rate) {
    return rate * 100;
}

public double getRate(double percent) {
    return percent / 100;
}

SonarQube will raise 2 violations, one for each use of 100. I could replace 100 with a constant variable, but what would be a good name for it? Is that really a good idea? If somebody ever changes its value that will be a disaster. I could also just add // NOSONAR to all the lines that use 100.

What is the best practice for dealing with the magic number 100?

UPDATE

From the answers, the most useful bit for me was this:

  Practical Magic Number rule: A literal is a not a magic number if the most meaningful variable name for it is the same as the spoken name of the literal.

So by this logic, 100 is NOT a magic number.

However, to make the violation go away, I decided to replace the 100 with a constant:

public static final double HUNDRED = 100;  // change it and I'll kill you

In my real project there are many lines using 100, and if I put // NOSONAR on all those lines that might cover up other potential problems that other developers might inadvertently add later.

Not sure if there will be any real benefits using this constant. A small one may be that when I do git grep 100 I see a lot of matches from resource files in the project, while git grep HUNDRED turns up just the the Java code that uses this.

Speaking as a human programmer (i.e. I am not Lint software), your use of "100" there looks fine to me.

Wikipedia has an article (without citations) titled Accepted limited use of magic numbers: IMO your "100" is in the same category as these other "accepted" magic numbers.

This Wiki describing magic numbers says two things.

Firstly,

  Practical Magic Number rule: A literal is a not a magic number if the most meaningful variable name for it is the same as the spoken name of the literal.

That's applicable here: you're looking for a named constant like HUNDRED or CENTUM.

Secondly, it also suggests loading "magic" numbers (e.g. a "discount rate") from a configuration file:

static final double DISCOUNT_PERCENT = getProperty( "sales.discount_percent" );
static final double DISCOUNT_FACTOR = 1 - (DISCOUNT_PERCENT / 100);

// ...

salePrice = DISCOUNT_FACTOR * regularPrice;

Note that though this example code carefully loaded DISCOUNT_PERCENT from a configuration, the "100" used to calculate the DISCOUNT_FACTOR is hard-coded.

If you use "100" instead of HUNDRED, it's easier for a programmer to understand, and to verify that it's correct.

IMO the only benefit to using HUNDRED is to find the several methods which use the same magic number (in your example it's used by getPercent and getRate).


Harvard CS50 Problem Set 1: greedy change-making algorithm The goal of this code is to take dollar or cents input from the user and give out minimum number of coins needed to pay that between quarters, dimes, nickels and pennies. If this code can be shortened, how would one do it?

#include <stdio.h>
#include <cs50.h>
#include <ctype.h>
#include <math.h>

int main(void)
{
float b;
do
    {
    printf("please give balance owing: ");
    b = GetFloat();
    }
while (b<0);

int v;
int q = 25;
int d = 10;
int n = 5;
int p = 1;
int count=0;

b=b*100;
int c=round(b);
int balance = c;
printf("%i\n", balance);
//printf("%.2f\n", round(c));

while(balance>=q)
    {
    v=balance/q;
    count=count+v;
    balance=balance-(q*v);
    }
while(balance>=d)
    {
    v=balance/d;
    count=count+v;
    balance=balance-(d*v);
    }
while(balance>=n)
    {
    v=balance/n;
    count=count+v;
    balance=balance-(n*v);
    }
while(balance>=p)
    {
    v=balance/p;
    count=count+v;
    balance=balance-(p*v);
    }

printf("the number of coins needed is: %i\n", count);
printf("balance after coins paid: %i\n", balance);

}

The first thing that sticks out to me is indentation.  But I'm going to recommend some other changes, so I'll address indentation last.

Let's give our variables some better, more descriptive names, and let's declare our constants as constants.  And since we're taking in a floating point number for input from the user, we need to compare that with other floating point numbers.  Or we need to use it as an integer.

int const kVALUE_QUARTER = 25;
int const kVALUE_DIME = 10;
int const kVALUE_NICKEL = 5;
int const kVALUE_PENNY = 1;

float userInput = -1.0;
int balance = 0;
int coinCount = 0;

do {
    printf("please give balance owing: ");
    userInput = GetFloat();
} while (userInput < 0);

balance = (int)(userInput * 100.0);

Now, we can eliminate all of our loops using the modulo operator (%) and integer division.

// number of quarters:
coinCount += balance / kVALUE_QUARTER;
balance %= kVALUE_QUARTER;

// number of dimes:
coinCount += balance / kVALUE_DIME;
balance %= kVALUE_DIME;

// number of nickels:
coinCount += balance / kVALUE_NICKEL;
balance %= kVALUE_NICKEL;

// number of pennies:
coinCount += balance / kVALUE_PENNY;
balance %= kVALUE_PENNY;

Now let's put it all together and take a look at our indentation:

#include <stdio.h>
#include <cs50.h>
#include <ctype.h>
#include <math.h>

int main(void) {
    int const kVALUE_QUARTER = 25;
    int const kVALUE_DIME = 10;
    int const kVALUE_NICKEL = 5;
    int const kVALUE_PENNY = 1;

    float userInput = -1.0;
    int balance = 0;
    int coinCount = 0;

    do {
        printf("please give balance owing: ");
        userInput = GetFloat();
    } while (userInput < 0);

    balance = (int)(userInput * 100.0);

    // number of quarters:
    coinCount += balance / kVALUE_QUARTER;
    balance %= kVALUE_QUARTER;

    // number of dimes:
    coinCount += balance / kVALUE_DIME;
    balance %= kVALUE_DIME;

    // number of nickels:
    coinCount += balance / kVALUE_NICKEL;
    balance %= kVALUE_NICKEL;

    // number of pennies:
    coinCount += balance / kVALUE_PENNY;
    balance %= kVALUE_PENNY;

    printf("the number of coins needed is: %i\n", count);
    printf("balance after coins paid: %i\n", balance);
}


Implementing `create` and `destroy` functions to replace `new` and `delete` operators The environment I'm working in requires that all allocation is done through a special set of malloc/calloc functions and that a user ID is provided when allocating. The environment has no implementation of exceptions, so errors are signaled by returning NULL.

For this reason, I have tried to write a couple of C++ functions which make it easier to create and destroy C++ objects:

namespace details {
    template<typename T>
    typename enable_if<is_pod<T>::value, T>::type *
    create(uint32_t module_id) {
        // use module_id
        return (T *)calloc(1, sizeof(T));
    }

    template<typename T, typename ...Args>
    typename enable_if<!is_pod<T>::value, T>::type *
    create(uint32_t module_id, Args&&... args) {
        // use module_id
        T *p = (T *)calloc(1, sizeof(T));
        if (p)
            new (p) T(forward<Args>(args)...);
        return p;
    }

    template<typename T>
    void destroy(typename enable_if<is_pod<T>::value, T>::type *t) {
        if (t)
            free(t);
    }

    template<typename T>
    void destroy(typename enable_if<!is_pod<T>::value, T>::type *t) {
        if (!t)
            return;

        t->~T();
        free(t);
    }
};

template<typename T, typename ...Args>
T *create(uint32_t module_id, Args&&... args) {
    return details::create<T>(module_id, forward<Args>(args)...);
}

template<typename T>
void destroy(T *t) {
    details::destroy<T>(t);
}

Can this be done smarter?  And are there any cases where my create/destroy functions are not handling as one would expect?

After some research, I'm convinced Christopher Creutzig is right. If you want all your allocations to go through customized functions, you're much better off replacing the global operator new. This will catch all calls to new for the translation unit, whether you replace them with create or not. That means when you create a std::vector<T> myvec(3), it will use your operator new for any heap allocations it needs to do, instead of sidestepping your global policy.

If you need to pass a module id, operator new can handle that as well by accepting extra parameters. I'm unclear how that mixes with examples such as the std::vector<T> heap allocations; it may require using a custom allocator for each STL container. Setting that aside, here's an example like yours following the lead on cppreference.com:

void* operator new(std::size_t cb, uint32_t module_id)
{
    // use module_id
    return calloc(1, cb);
}
void operator delete(void* ptr)
{
    free(ptr);   // I'm unsure if this requires a null check
}

type *p = new(module_id) type;

As an aside, if you're particularly tied to calling functions, I'd be tempted to remove the SFINAE specialization via is_pod<T> and just make it an inline check via is_trivially_constructible<T> and is_trivially_destructible<T>...that is if checking is necessary at all.

template<typename T>
T* create(uint32_t module_id)
{
    // use module_id
    T* p = calloc(1, sizeof(T));
    if (!is_trivially_constructable<T>::value && p)
    {
        new (p) T(forward<Args>(args)...);
    }
    return p;
}


JavaScript accordion I'm trying to refactor some code I wrote months ago when I used to use lots of anonymous functions. The way I'm trying to do it now is by taking all those functions and rewrite them within Object Literal Pattern.

I'm new to this kind of pattern and I would like to know if I'm doing it right, what should I improve or if there's a better way to write the code for easier maintainability.

The first piece of code I refactored is in the following rows and is about an accordion view.

I've also created a JsFiddle for this where you can see the whole code including the former way of writing the js.

var mainAccordion = {
  init: function() {
    this.cacheDom();
    this.bindEvents();
  },

  cacheDom: function() {
    this.$elTrigger = $('.js-accordion-trigger');
  },

  bindEvents: function() {
    this.$elTrigger.on('click', this.elExpand);
  },

  elExpand: function(event) {
    var $el = $(this),
        $elContent = $el.next('.accordion__content'),
        $elParent = $el.closest('li'),
        $elSiblings = $el.parent().siblings(),
        $groupSiblings = $el.parents('.list-with-header').siblings();

    if (!$elParent.hasClass('is-expanded')) {
      event.preventDefault();

      $elContent.show();
      $elParent.addClass('is-expanded');

      $elSiblings.removeClass('is-expanded');
      $elSiblings.find('.accordion__content').hide();

      if ($groupSiblings.find('li').hasClass('is-expanded')) {
        $groupSiblings.find('li').removeClass('is-expanded');
        $groupSiblings.find('.accordion__content').hide();
      }

      // animate accordion element to top of page
      $('html, body').stop().animate({
        'scrollTop': $elParent.offset().top
      });
    } else {
      event.preventDefault();

      $elContent.slideUp('fast');
      $elParent.removeClass('is-expanded');
    }
  }
};

mainAccordion.init();

Some small changes I'd recommend listed below:

D.R.Y.

Why don't you call event.preventDefault before your control statement since it will be called in both the if and the else case?

Avoid doubling up on lookups

I am not very experienced in jQuery, but can't you make some if checks not double up on lookups?

var liGroup = $groupSiblings.find('li')
if (liGroup.hasClass('is-expanded')) {
     liGroup.removeClass('is-expanded');

Also check the specs on removeClass()..perhaps you can call removeClass() without checking hasClass?

Formatting/Readability

Your code is difficult to read without more formatting. For example, it's not clear which if matches up with the else from skimming the code. It shouldn't be necessary for a reader to parse parens/brackets to figure out what the if...else pairing is.

Also a few more comments would be helpful. For example, comments describing your control statement branching in a few words would be welcome.


Increment a text field value So let's take this code which increments a Javascript text field value:

1

var counter = $(".counter")
counter.html( parseInt(counter.html()) + 1 )

It can be made shorter by using the Unary operator + which behaves like Number():

1

var counter = $(".counter")
counter.html( +counter.html() + 1 )

is there a shorter and better way to do this?

I tried combinations of the ones below, neither work:

1

$(".counter").html( +this.innerHTML + 1 )

and

1

$(".counter").html( +this.value + 1 )

and

1

$(".counter").html( +$this.html() + 1 )

Not a question that fits CR well. This is clearly short, untested sample code. You need $("#counter"), not $(".counter").

var counter = $("#counter")
counter.html( parseInt(counter.html()) + 1 )

1

If you truly wish to increase the content of a field that you access by id you could try this:

document.getElementById('counter').textContent++;

1

This works as of IE9.. Personally I would always make sure that there is a number in textContent since otherwise the value will become NaN which looks like amateur hour.


Many onClick handlers that all append different text to one textarea The script basically just appends the words based on the user's click/s on the textarea. I just need some help improving my code further using the .on("click", function{}) and making it shorter.

$(document).ready(function() {

        $("#add_user").on("click",function() {
            $("#textareaFixed").append("Lorem");
        });

        $("#add_disallow").on("click",function() {
            $("#textareaFixed").append("Ipsum");
        });

        $("#add_wild").on("click",function() {
            $("#textareaFixed").append("Dolor");
        });

        $("#add_comment").on("click",function() {
            $("#textareaFixed").append("Hey");
        });

        $("#add_bb").on("click",function() {
            $("#textareaFixed").append("World");
        });

        $("#add_gdrive").on("click",function() {
            $("#textareaFixed").append("Its Time");
        });

        $("#add_gimage").on("click",function() {
            $("#textareaFixed").append("To Move");
        });

        $("#add_gnews").on("click",function() {
            $("#textareaFixed").append("Again");
        });

        $("#add_bash").on("click",function() {
            $("#textareaFixed").append("Thats it");
        });
    })

We can reduce duplication by using an object as a dictionary, and look up an output value based on an element's ID.

var messageMap = {
    "add_user": "Lorem",
    "add_disallow": "Ipsum",
    "add_wild": "Dolor",
    "add_comment": "Hey",
    "add_bb": "World",
    "add_gdrive": "Its Time",
    "add_gimage": "To Move",
    "add_gnews": "Again",
    "add_bash": "Thats it"
};

$(document).ready(function() {
    $.each(messageMap, function(key, value){
        $("#" + key).on("click",function() {
            $("#textareaFixed").append(messageMap[$(this).attr("id")]);
        });
    });
});


Toggling images in a table Toggling images in a table. The example would be a user using a checklist program. She can click on a cell next to task to toggle available status images. Like a checkmark, fail, question.

Other design suggestions are welcomed

<!DOCTYPE html>

    <title>Test 123</title>

You use a lot of jQuery, where plain javascript will suffice and is actually more efficient. A small example: el.attr('id') loops through the available attributes until it finds ID1. you can simply do el[0].id and select it without anything complex. The same fors for .src and some other values. I will add comment to the coding below to guide you through it:

1 This isnt 100% true, they've improved but you get the gist of it

Round 1:

$(function () {
    var lastImageIdClicked = "";
    var timer;

    $('.imgX').on('click', function () {
        // Was: el = $(this);
        // - Placed [var] before it to make it local scope
        // - We dont yet need it als a jQuery element
        // - el is very short and not descriptive. [elem] is more common:
        var elem = this ;
        // Was: el.attr('id');
        // - update to [elem] -> elem.id
        // - removed variable because we no longer use it (see a few lines below)
        //~ elId =  elem.id
        // Was: el.attr('src');
        // - update to [elem] -> elem.id
        // - removed variable because we no longer use it (see a few lines below)
        //~ elSrc = elem.scr
        var clicks = 0;

        // Was: if (elId === lastImageIdClicked) {
        // - just use elem.id here. It's not more complex and actually tells me more
        if (elem.id === lastImageIdClicked) {
            clicks++;
            clearTimeout(timer);
        }
        // This whole else is irrelevant as you set [var clicks = 0;] a few lines higher
        // else {
        //      clicks = 0;
        //}

        // Was: var fileName = elSrc;
        // - Simply use elem.src
        // We use this only once a few lines below, just use [elem.src] there, no need to use a variable for that

        // Was: lastImageIdClicked = elId;
        // - Simply use elem.id
        lastImageIdClicked = elem.id;

        var re = /(pic)(.*)(.png)/;
        var imgNumber = elem.src.match(re)[2]; // todo: why array
        clicks = Number(imgNumber) + 1;

        // Was: if (clicks > 3) clicks = 0;
        // IMO you should _always_ use brackets: if (clicks > 3){ clicks = 0;}
        if (clicks > 3){ clicks = 0; }

        // Was: var eleImage = $(this).attr('src', 'pic{0}.png'.replace('{0}', clicks));
        // - we now just use this.src = /* ... */
        // - You place {0} in your string, then replace it? Just concat it directly: ['pic'+click+'.png']
        var eleImage = this.src = 'pic'+clicks+'.png';

        //Save image if 5 seconds have passed
        timer = setTimeout(function () {
            alert('saved');
        }, 5000);
    });
});

Clean that up:

$(function () {
    var lastImageIdClicked = "",
        timer;

        $('.imgX').on('click', function () {
            var elem = this ;
            var clicks = 0;

            if (elem.id === lastImageIdClicked) {
                clicks++;
                clearTimeout(timer);
            }
            lastImageIdClicked = elem.id;

            var re = /(pic)(.*)(.png)/;
            var imgNumber = elem.src.match(re)[2]; // todo: why array
            clicks = Number(imgNumber) + 1;

            if (clicks > 3){ clicks = 0; }

            var eleImage = this.src = 'pic'+clicks+'.png';

            //Save image if 5 seconds have passed
            timer = setTimeout(function () {
                alert('saved');
            }, 5000);
    });
});

Round 2

$(function () {
    var lastImageIdClicked = "",
        timer;

        $('.imgX').on('click', function () {
            var elem = this ;
            var clicks = 0;

            if (elem.id === lastImageIdClicked) {
                clicks++;
                clearTimeout(timer);
            }
            lastImageIdClicked = elem.id;

            // Was: var re = /(pic)(.*)(.png)/;
            // Was: var imgNumber = elem.src.match(re)[2]; // todo: why array
            // - This is a simple regex, no need to use a variable for it if you only use it once
            // - you select multiple groups now, you don't want that, only the number
            var imgNumber = elem.src.match(/pic(.*)\.png/)[1]; // todo: why array
            clicks = Number(imgNumber) + 1;

            if (clicks > 3){ clicks = 0; }

            // Was: var eleImage = this.src = 'pic'+clicks+'.png';
            // - Seemingly does nothing, assuming you want to change the src
            this.src = 'pic'+clicks+'.png';

            //Save image if 5 seconds have passed
            timer = setTimeout(function () {
                alert('saved');
            }, 5000);
    });
});

Clean that up

$(function () {
    var lastImageIdClicked = "",
        timer;

        $('.imgX').on('click', function () {
            var elem = this ;
            var clicks = 0;

            if (elem.id === lastImageIdClicked) {
                clicks++;
                clearTimeout(timer);
            }
            lastImageIdClicked = elem.id;

            var imgNumber = elem.src.match(/pic(.*)\.png/)[1]; // todo: why array
            clicks = Number(imgNumber) + 1;

            if (clicks > 3){ clicks = 0; }
            this.src = 'pic'+clicks+'.png';

            //Save image if 5 seconds have passed
            timer = setTimeout(function () {
                alert('saved');
            }, 5000);
    });
});

Round 3

This is for you do to yourself:
- Instead if hardcoding max-images = 3, make an array with images imgs and do imgs.length
- Instead of using a regex, add a data-current_img="1" to your images and use that to count
- If you use the data method, you can update the image to elem.src = imgs[current+1] to move to the next img, instead of the pricy regex

Now you  don't have to manually limit to 3, or rename the images to their number, nor do you need to update our code if an extra state has to be build.


Using a custom comparison function for BTreeSet and a foreign type As detailed in this SO question, I'm using custom logic in a BTreeSet to automatically sort a foreign data type as elements are added to the container.  Note that the Data type is used from another module in the actual implementation, so I can't just implement Ord for it, necessitating this approach.

I'm concerned that the iterator pipeline I'm using in main() is too complex and that the various impls are just boilerplate for something that really boils down to a single line of actual desired logic.

Here's a Rust Playground link.

use std::collections::BTreeSet;
use std::convert::{From, Into};
use std::cmp::Ordering;
use std::ops::Deref;

// Here's the original struct...
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
struct Data(usize);

impl From<usize> for Data {
    fn from(i: usize) -> Self {
        Data(i)
    }
}

// And here's the wrapper type we'll use to sort it.
#[derive(PartialEq, Eq, Debug)]
struct SortableData(Data);

impl Deref for SortableData {
    type Target = Data;
    fn deref<'a>(&'a self) -> &'a Data {
        &self.0
    }
}

impl From<Data> for SortableData {
    fn from(data: Data) -> Self {
        SortableData(data)
    }
}

impl PartialOrd for SortableData {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl Ord for SortableData {
    fn cmp(&self, other: &Self) -> Ordering {
        (self.0).0.cmp(&(other.0).0)
    }
}

fn main() {
    let source: [Data; 3] = [Data(9), Data(4), Data(2)];
    let sorted: BTreeSet<SortableData> = source.iter().cloned().map(|i| i.into()).collect();
    for d in sorted.iter() {
        println!("{:?}", **d);
    }
}

Your code looks basically how I would write it, very nice! I do have a few tiny nits.

I strongly dislike using Deref except when you have a smart pointer. I fear that using Deref to compose items is going to be the Rust equivalent of using inheritance for code reuse.

Using Deref also can be a bit surprising, as every method is now available. Let's say that your Data struct has some more methods:

impl Data {
    fn wow(&self) { println!("Wow") }
}

Now, your SortableData has those methods as well!

for d in sorted.iter() {
    d.wow();
}

I much prefer to be explicit about it; I would implement Borrow instead.

I don't believe this opinion to be 100% supported by the community, but I'm going to make my case where I can ^_^
When possible, I'd implement From / Into in both directions. This doesn't always make sense, but here it does.
There's no need to specify the type for source — type inference handles that just fine.
You could just map the into function directly: .map(Into::into).
Instead of calling iter in the for loop expression, you can just take a reference.
From and Into are both part of the prelude and thus don't need to be used again.

use std::borrow::Borrow;
use std::cmp::Ordering;
use std::collections::BTreeSet;

// Here's the original struct...
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
struct Data(usize);

impl From<usize> for Data {
    fn from(i: usize) -> Self {
        Data(i)
    }
}

// And here's the wrapper type we'll use to sort it.
#[derive(PartialEq, Eq, Debug)]
struct SortableData(Data);

impl From<Data> for SortableData {
    fn from(data: Data) -> Self {
        SortableData(data)
    }
}

impl From<SortableData> for Data {
    fn from(data: SortableData) -> Self {
        data.0
    }
}

impl Borrow<Data> for SortableData {
    fn borrow(&self) -> &Data {
        &self.0
    }
}

impl PartialOrd for SortableData {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl Ord for SortableData {
    fn cmp(&self, other: &Self) -> Ordering {
        (self.0).0.cmp(&(other.0).0)
    }
}

fn main() {
    let source = [Data(9), Data(4), Data(2)];
    let sorted: BTreeSet<SortableData> = source.iter().cloned().map(Into::into).collect();
    for d in &sorted {
        let d: &Data = d.borrow();
        println!("{:?}", d);
    }
}


Update record with CSV I'm using Rails 4.2.5, and I'd like to update existing records with a csv file with two fields: id,category. Where if the category field is blank, it defaults to misc.

Here's an example csv file:

id,category
123-123-123,0
123-123-124,1
123-123-125,
123-123-126,2

So, I've created some tasks, update_item_categories, and set_item_categories:

# lib/tasks/update_item_categories.rake
require 'csv'

namespace :db do
  task :update_item_categories => :environment do
  categories = ['misc', 'food', 'drink']
  CSV.foreach('public/update/item_categories.csv', headers: true) do |row|
    row_hash = row.to_hash
    category = categories[row_hash['category'].to_i]
    unless Item.where(id: row_hash['id']).empty?
      item = Item.find(row_hash['id'])
      item.update_attributes(
        :category => category
      )
      item.save!
    end
  end

  task :set_item_categories => :environment do
    Item.where(category: nil).each do |item|
      item.update_attributes(category: 'misc')
      item.save!
    end
  end
end

I will definitely appreciate any advice on how this could be better written, and also whether this is even a good way to go about solving this problem.

Regarding the first task, you could save a couple of queries by modifying the code to look like this:

# ....

CSV.foreach('public/update/item_categories.csv', headers: true) do |row|
  row_hash = row.to_hash
  category = categories[row_hash['category'].to_i]

  item = Item.where(id: row_hash['id'])
  item.update_attributes!(category: category) unless item.nil?
end

# ....

You can retrieve the item once and perform update_attributes! without save!. update_attributes! saves the changes to the db using save! internally. Here's the source code (update_attributes! it's an alias for update!):

def update!(attributes)
  with_transaction_returning_status do
    assign_attributes(attributes)
    save!
  end
end

Regarding the second task, since you want to raise an exception in case of invalid record, you have to save each record individually but you can save some queries by finding records in batches. The code could look like this:

Item.find_each(category: nil) do |item|
  item.update_attributes!(category: 'misc')
end


Form a loop with previous and next I have a class Edge which looks something like this:

class Edge {

    Edge previous;
    Edge Next;

}

I have edges in an ArrayList. They have to form a loop, as in the last one is connected to the first one.

This is what I have:

    Edge previous, current, next;

    previous = edges.get(edges.size()-1);

    for (int i = 0; i < edges.size(); i++) {
        current = edges.get(i);
        next = i+1 == edges.size() ? edges.get(0) : edges.get(i+1);

        current.previousEdge = previous;
        current.nextEdge = next;

        previous = current;
    }

I only have the idea that this could be written clearer than it is now.

You can use a for-each iterator instead of using indexes.
It would be somewhat simpler, like this:

Edge previous = edges.get(nodeList.size() - 1);
for (Edge edge : edges) {
    edge.previous = previous;
    previous.next = edge;
    previous = edge;
}

Suppose the initial list of edges is not in an ArrayList which allows quick random access but another kind of iterable. Then, you could rewrite this way without accessing any element by index:

Iterator<Edge> iter = edges.iterator();

Edge first = iter.next();
Edge previous = first;
Edge edge = null;

while (iter.hasNext()) {
    edge = iter.next();
    edge.previous = previous;
    previous.next = edge;
    previous = edge;
}

if (edge != null) {
    edge.next = first;
    first.previous = edge;
}


Append to compressed tar file with performance As you might know, Python's tarfile does not have the ability to append to the files that compressed with e.g. gz or bz2. I tried to implement this functionality which works fine but operates slowly. The following function accepts a string or bytes object and appends it as a file to an existing tarfile. I'm not sure if this code has the best performance, and it may have issues. I also tried writing files to memory instead of to a temporary directory, but this didn't impact performance.
import os
import tarfile
import tempfile
import time
from pathlib import Path

def append_tar_file(buffer, file_name, output_path, replace=True):
    """
    append a buffer to an existing tar file
    """
    # extract files
    # check for existing file and overwrite if need to
    # compress files
    if not os.path.isfile(output_path):
        return
    buffer = buffer.encode("utf-8") if isinstance(buffer, str) else buffer

    with tempfile.TemporaryDirectory() as tempdir:
        tempdirp = Path(tempdir)
        with tarfile.open(output_path, "r:bz2") as tar:
            try:
                tar.extractall(os.path.abspath(tempdirp))
            except Exception as err:   #tar file is empty
                print(err)
        buffer_path = os.path.join(tempdir, os.path.basename(file_name))
        if replace or (buffer_path not in list(os.path.abspath(f) for f in tempdirp.iterdir())):
            with open(buffer_path, "wb") as f:
                f.write(buffer)

        with tarfile.open(output_path, "w:bz2") as tar:
            for file in tempdirp.iterdir():
                try:
                    tar.add(file, arcname=os.path.basename(os.path.normpath(file)))
                except Exception as err:
                    print(err)

if __name__ == "__main__":
    path = "./test.tar.gz"
    buffer = "Test String"
    filename = "somefile"
    for i in range(1, 100):
        print(time.time())
        append_tar_file(buffer, filename+str(i), path)

Indeed, the tarfile package doesn't support appending to a compressed tar.
But I think you can do better than your current attempt.
Instead of extracting the content to disk, you could keep it in memory, write and append to a new compressed file, and finally rename the compressed file.
That way you will write a single temporary file,
instead of many.
The reduced disk I/O should improve the performance.

def append_tar_file(buf, file_name, output_path, replace=True):
    """
    append a buf to an existing tar file if not already there, or if replace=True
    """
    if not os.path.isfile(output_path):
        return

    with tempfile.TemporaryDirectory() as tempdir:
        tmp_path = os.path.join(tempdir, 'tmp.tar.bz2')

        with tarfile.open(output_path, "r:bz2") as tar:
            if not replace:
                if file_name in (member.name for member in tar):
                    return

            if isinstance(buf, str):
                buf = buf.encode("utf-8")

            fileobj = BytesIO(buf)
            tarinfo = tarfile.TarInfo(file_name)
            tarinfo.size = len(fileobj.getvalue())

            with tarfile.open(tmp_path, "w:bz2") as tmp:
                for member in tar:
                    if member.name != file_name:
                        tmp.addfile(member, tar.extractfile(member.name))
                tmp.addfile(tarinfo, fileobj)

        os.rename(tmp_path, output_path)


Optimizing Code for Project Euler Problem 14 For Project Euler problem 14 I wrote code that runs for longer than a minute to give the answer. After I studied about memoization, I wrote this code which runs for nearly 10 seconds on Cpython and nearly 3 seconds on PyPy. Can anyone suggest some optimization tips?

import time
d={}
c=0
def main():
    global c
    t=time.time()
    for x in range(2,1000000):
        c=0
        do(x,x)
    k=max(d.values())
    for a,b in d.items():
        if b==k:
            print(a,b)
            break

    print(time.time()-t)

def do(num,rnum):
    global d
    global c
    c+=1
    try:
        c+=d[num]-1
        d[rnum]=c
        return
    except:
        if num==1:
            d[rnum]=c
            return
        if num%2==0:
            num=num/2
            do(num,rnum)
        else:
            num=3*num+1
            do(num,rnum)

if __name__ == '__main__':
    main()

I think you're over complicating your solution, my approach would be something along these lines:

def recursive_collatz(n):
        if n in collatz_map:
                return collatz_map[n]
        if n % 2 == 0:
                x = 1 + recursive_collatz(int(n/2))
        else:
                x = 1 + recursive_collatz(int(3*n+1))
        collatz_map[n] = x
        return x

Basically define a memoization map (collatz_map), initialized to {1:1}, and use it to save each calculated value, if it's been seen before, simply return.

Then you just have to iterate from 1 to 1000000 and store two values, the largest Collatz value you've seen so far, and the number that gave you that value.

Something like:

largest_so_far = 1
highest = 0
for i in range(1,1000000):
    temp = recursive_collatz(i)
    if temp > largest_so_far:
        highest = i
        largest_so_far = temp

Using this approach I got:

Problem 14's answer is: 837799.
Took 1.70620799065 seconds to calculate.


Challenge: phone keypad to letters - return all possible strings The challenge:

Given a mapping of digits to letters (as in a phone number), and a digit string, return all possible letters the number could represent. You can assume each valid number in the mapping is a single digit.

Example:

If {"2": ["a", "b", "c"], 3: ["d", "e", "f"], …}
then "23" should return ["ad", "ae", "af", "bd", "be", "bf", "cd", "ce", "cf"].

My iterative solution:

const digitMap = (digiStr) => {

  const digitMapping = {
    2: ['a', 'b', 'c'],
    3: ['d', 'e', 'f'],
    4: ['g', 'h', 'i'],
    5: ['j', 'k', 'l'],
    6: ['m', 'n', 'o'],
    7: ['p', 'q', 'r', 's'],
    8: ['t', 'u', 'v'],
    9: ['w', 'x', 'y', 'z']
  }
  const length = digiStr.length;
  if (length === 1) return digitMapping[digiStr]; // in case of single digit strings
  const indexes = []; // index addresses of charsets, in order
  for (let i = 0; i < length; i++) {
    indexes.push(0);
  }
  let nextI = length === 2 ? 0 : 1; // current spot of nextI in indexes
  const possibleStrs = []; // for final answer of possible permutations
  let str = '';
  let endCount = 0; // if all addresses in indexes are at their end, then break while loop
  while (true) {
    console.log(indexes);
    for (let i = 0; i < length; i++) {
      str += digitMapping[digiStr[i]][indexes[i]];
      if (indexes[i] === digitMapping[digiStr[i]].length - 1) {
        endCount += 1;
      }
    }
    possibleStrs.push(str);
    str = '';
    if (endCount === length) {
      break;
    }
    endCount = 0;
    // increment last item in indexes as long as it < length of corresponding inner char array
    if (indexes[indexes.length - 1] < digitMapping[digiStr[digiStr.length - 1]].length - 1) {
      indexes[indexes.length - 1] += 1;
    }
    else {
      indexes[indexes.length - 1] = 0;
      // increment nextI as long as < length of corresponding inner array
      if (indexes[nextI] < digitMapping[digiStr[nextI]].length - 1) {
        indexes[nextI] += 1;
      }
      else {
      // otherwise shift position of nextI in indexes array
        // if next position of nextI not end of indexes array
          // shift nextI 1 spot forward in indexes
        if (nextI + 1 < length - 1) {
          nextI += 1;
          indexes[nextI] += 1;
        }
        else {
          // else reset all indexes to 0, starting from position 1
          for (let i = 1; i < indexes.length; i++) {
            indexes[i] = 0;
          }
          // increment first item in indexes
          indexes[0] += 1;
          nextI = 1; // place nextI at position 1
        }
      }
    }
  }
  return possibleStrs;
}

console.log(digitMap("2"));
console.log(digitMap("23"));
console.log(digitMap("234"));
console.log(digitMap("2345"));
console.log(digitMap("23456"));

Produces the correct output for all test cases. Any alternative approaches--recursive or iterative--to cleaning up the conditional logic would be greatly appreciated.

Too long for a comment...

The problem as presented seems to have much in common with the string permutations problem in terms of solution structure.

You should be able to do something similar to: https://stackoverflow.com/questions/4240080/generating-all-permutations-of-a-given-string with slight modification to get a compact recursive formulation.

Pseudo code below:

solution(string digits, string prefix="", array<string> outputs)
    if digits empty then
        add prefix to outputs
    else
        currentDigit := digits.get(0)
        remainingDigits := digits.remove(0);
        for each character in listOfCharsForDigit[currentDigit] do
            solution(remainingDigits, prefix+character, outputs)
        endfor
    endif

Hope that helps...


Argument order with default value I have a library with two methods: one takes an object and vararg strings, the other takes a default value as well.

They are currently implemented as:

export default class koalaesce {
    static get(base, ...steps) {
        return koalaesce.getDefault(base, null, ...steps);
    }

    static getOrThrow(base, ...steps) {
        return // long impl;
    }

    static getDefault(base, def, ...steps) {
        try {
            return koalaesce.getOrThrow(base, ...steps);
        } catch (e) {
            if (e.constructor === MissingLinkError || e.constructor === NullLinkError) {
                return def;
            } else {
                throw e;
            }
        }
    }
}

and would be called like:

get(obj, "foo", "bar", "baz")
getDefault(obj, 9, "foo", "bar", "baz")

I feel weird splitting up the base and args with the default value, but I'm not sure if putting the default value first is appropriate:

getDefault(9, obj, "foo", "bar", "baz")

Since the base object and args are related, this seems more reasonable, but I don't think I've seen the default value come first in a case like this (most often, they come last, but that and varargs seems like lousy API design).

Is there an idiomatic way to do this, for Javascript or in general? If so, what is the reasoning behind that?

Yes, varargs and default args obviously conflict, so choose either.  I think the API as it is, with two functions, is fine and it's a good way to distinguish the two cases.  And I wouldn't expect the default value to go first in any case.

You could also consider not using varargs, right?  So getOrThrow(base, steps, default), using an array for steps instead.


Looping Array object with conditions and comparison the performance with ECMA VS Vanilla JS The Plain vanilla JS needs to converted in to advanced ECMA. The below 3 snippets shown yields the correct result. Need to identify which among them is performance oriented
this.allUserDetails.forEach((el, indx) => {
    if (type === 0 && flagResumeAll === false) {
        if (el.extensionTime === time && el.userProctorSignal === 0) {
            this.toggleButtonDisplayType = 0;
        } else {
            flagResumeAll = true;
        }
    } else if (type === 1 && flagPauseAll === false) {
        if (el.extensionTime === time && el.userProctorSignal === 1) {
            this.toggleButtonDisplayType = 1;
        } else {
            flagPauseAll = true;
        }
    }
});

The above code could be repaced with Code I and Code II
Code I
if (type === 0 && flagResumeAll === false) {
    this.allUserDetails.filter(el => {
        if(el.extensionTime === time && el.userProctorSignal === 0) {
        this.toggleButtonDisplayType = 0;
        } else {
        flagResumeAll = true;
        }
    })
} else if (type === 1 && flagPauseAll === false) {
    this.allUserDetails.filter(el => {
        if(el.extensionTime === time && el.userProctorSignal === 1) {
        this.toggleButtonDisplayType = 1;
        } else {
            flagPauseAll = true;
        }
})

Code II
this.allUserDetails.filter(el => {
    if(type === 0 && flagResumeAll === false && el.extensionTime === time && el.userProctorSignal === 0) {
        this.toggleButtonDisplayType = 0;
    } else if (type === 0 && flagResumeAll === false){
        flagResumeAll = true;
    } else if (type === 1 && flagPauseAll === false && el.extensionTime === time && el.userProctorSignal === 1) {
        this.toggleButtonDisplayType = 1;
    } else if(type === 1 && flagPauseAll === false) {
        flagPauseAll = true;
    }
})

Which among them is yields better in performance replacing from plain/vanilla JS to Code I/II.
Will this be further simplified..

It looks like the main difference between your original code snippet and the two revisions is that you're using .filter() instead of .forEach() - both of which came out around the same time, and both of which came out a while ago (IE 9 supports both).
It looks like you aren't doing any significant changes to the code that would improve speed, so you will only see marginal improvements to performance, if any. But, you can always benchmark each of them and find out for yourself how fast each version is.
The original solution is actually the best one among the three shown. There seems to be a big misunderstanding about what the filter() function is and how to use it. filter() is supposed to take an array and return a new array with some values filtered out. The values that get filtered out is decided by the function passed in. For example, if you want the even numbers in a list of numbers, you could do this:
const numbers = [1, 3, 4, 6, 7, 9]
const isEven = n => n % 2 === 0
console.log(numbers.filter(isEven)) // [ 4, 6 ]

You are not using filter() this way - you're just using is like an alternative forEach() function, which is a big no-no. You can just change the .filter() for .forEach() in each of your revisions. Once you have done that, both your original code and your first revision would work just fine. I wouldn't go with the second one though, there's too much inside the if conditions.
I don't really have any newer javascript feature to suggest for these code snippets, except that I generally recommend using the newer for of loop instead of forEach() - it's more powerful than .forEach(), and it fixes the issues that for-in loops had.


Spin-lock implementation I'm working on a project where a spinlock is more appropriate than a mutex, and after few tries I came up with:

type SpinLock uint32

func (sl *SpinLock) Lock() {
    for !atomic.CompareAndSwapUint32((*uint32)(sl), 0, 1) {
        runtime.Gosched() //without this it locks up on GOMAXPROCS > 1
    }
}

func (sl *SpinLock) Unlock() {
    atomic.StoreUint32((*uint32)(sl), 0)
}

It works fine, and it's even a little bit faster than sync.Mutex, and 2x the speed of sync.RWMutex.

➜ go test -bench=. -benchmem -v -cpu 4
BenchmarkSpinL-4            2000           1078798 ns/op           33923 B/op       2006 allocs/op
BenchmarkMutex-4            2000           1195814 ns/op           32781 B/op       2002 allocs/o
BenchmarkRWMutex-4          1000           2352117 ns/op           78253 B/op       2147 allocs/op

The test uses multi readers / writers to a map[int]*struct{int, int}. Running it with -race doesn't detect any data races, either.

But I have that nagging feeling that I forgot something, so I'm wondering if my implementation is correct?

The only weak point is that the implementation is not copy safe nor there exist mechanism for ensuring copy protection. I would hide its underlying type and return as sync.Locker, so it can't be mis-used:

type spinLock uint32

func (sl *spinLock) Lock() {
    for !atomic.CompareAndSwapUint32((*uint32)(sl), 0, 1) {
        runtime.Gosched() //without this it locks up on GOMAXPROCS > 1
    }
}

func (sl *spinLock) Unlock() {
    atomic.StoreUint32((*uint32)(sl), 0)
}

func SpinLock() sync.Locker {
    return &spinLock{}
}

An alternative I've seen in sync.Cond is to embed an auxiliary type for copy-protection, although it would complicate the implementation unnecessary.


Solving the rod-cutting problem using dynamic programming I've written what I believe is a valid dynamic programming solution to a variation of the rod cutting problem. In this problem, the goal is to make as few cuts as possible on a rod of length n.

I have two questions:

Is this a valid dynamic programming solution?
How can it be improved?

(This is not a school assignment. I simply want to understand dynamic programming better)

public class RodCutting {

    RodCutting(int[] cuts, int length) {
        List<Integer> cutLengths = new ArrayList<Integer>();
        for (int i = 0; i < cuts.length; i++) {
            cutLengths.add(cuts[i]);
        }
        Collections.sort(cutLengths);
        List<List<Integer>> optimalCuts = new ArrayList<List<Integer>>();

        // initialize list
        for (int i = 0; i <= length; i++) {
            optimalCuts.add(new ArrayList<Integer>());
        }

        for (int i = 1; i <= length; i++) {
            // assume 1 is always a valid cut length
            if (cutLengths.contains(i)) {
                for (int j = 0; j < cutLengths.size(); j++) {
                    if (i == cutLengths.get(j)) {
                        optimalCuts.get(i).add(cutLengths.get(j));
                        // nothing larger than the current cut
                    }
                }
            } else {
                for (int j = 0; j < cutLengths.size(); j++) {
                    if (i > cutLengths.get(j)) {
                        List<Integer> newCuts = union(
                                optimalCuts.get(i - cutLengths.get(j)),
                                cutLengths.get(j));
                        if (optimalCuts.get(i).size() == 0
                                || newCuts.size() < optimalCuts.get(i).size()) {
                            optimalCuts.remove(i);
                            optimalCuts.add(i, newCuts);
                        }
                    }
                }
            }
        }

        for (int i = 0; i < optimalCuts.size(); i++) {
            System.out.println(i + ": " + optimalCuts.get(i));
        }

    }

    List<Integer> union(List<Integer> listOfCuts, int additionalCut) {
        List<Integer> returnList = new ArrayList<Integer>(listOfCuts);
        returnList.add(additionalCut);
        return returnList;
    }

    public static void main(String[] args) {
        int[] cuts = { 1, 3, 4, 7 };
        int rodLength = 100;
        RodCutting cut = new RodCutting(cuts, rodLength);
    }
}

I'm not familiar with the problem of rod-cutting, so I cannot reflect on the correctness of the algorithtm.
However, I can give you suggestions for some slight improvements:

union method does not depend on any instance variables --> consider making it static
consider splitting the algorithm into two parts: the constructor could have just some code to store the parameters, and then you could have a separate method to perform the actual calculations (this would make the code more readable)

So, I suggest something like this (not tested):

public class RodCutting {
    private int [] cutLengths;
    private int length;

    RodCutting(int[] cuts, int length) {
        cutLengths = new ArrayList<Integer>();
        for (int i = 0; i < cuts.length; i++) {
            cutLengths.add(cuts[i]);
        }
        Collections.sort(cutLengths);

        this.length = length;
   }

   public void calc() {
        List<List<Integer>> optimalCuts = new ArrayList<List<Integer>>();

        // initialize list
        for (int i = 0; i <= length; i++) {
            optimalCuts.add(new ArrayList<Integer>());
        }

        for (int i = 1; i <= length; i++) {
            // assume 1 is always a valid cut length
            if (cutLengths.contains(i)) {
                for (int j = 0; j < cutLengths.size(); j++) {
                    if (i == cutLengths.get(j)) {
                        optimalCuts.get(i).add(cutLengths.get(j));
                        // nothing larger than the current cut
                    }
                }
            } else {
                for (int j = 0; j < cutLengths.size(); j++) {
                    if (i > cutLengths.get(j)) {
                        List<Integer> newCuts = union(
                                optimalCuts.get(i - cutLengths.get(j)),
                                cutLengths.get(j));
                        if (optimalCuts.get(i).size() == 0
                                || newCuts.size() < optimalCuts.get(i).size()) {
                            optimalCuts.remove(i);
                            optimalCuts.add(i, newCuts);
                        }
                    }
                }
            }
        }

        for (int i = 0; i < optimalCuts.size(); i++) {
            System.out.println(i + ": " + optimalCuts.get(i));
        }

    }

// ... rest of the code


TapeEquilibrium implementation does not satisfy all requirements The Codility's TapeEquilibrium problem asks:

  Given an array \$A\$ with length \$N\$, indexed starting from 0, find an equilibrium index \$P\$, such that

  $$\left| \sum_{i=0}^{P-1} a_i - \sum_{i=P}^{N-1} a_i \right|$$

  is minimized.  Assume that \$2 \le N \le 100000\$ and that \$-1000 \le a_i \le 1000\$.  Elements of input arrays can be modified.

Here is my solution:

 public int solution(int[] A)
 {
        // write your code in C# 5.0 with .NET 4.5 (Mono)
        List<int> list = A.Cast<int>().ToList();
        int temp = 0;
        int sum = list.Sum();
        List<int> result = new List<int>();
        foreach (int i in list)
        {
             temp = temp + i;
             result.Add(Math.Abs(  ( sum - temp) - temp));

        }
        return  result.Min();
    }

How can I make it better?  I only got 83%.

I coded up my own solution, and mine was pretty close to yours, but mine received 100%.

Here are the problems:

List<int> list = A.Cast<int>().ToList();

Both the Cast and the ToList are redundant here. Arrays implement IEnumerable, which is enough for LINQ. The ToList is iterating the entire array, which is costing you some runtime.

int temp = 0;
int sum = list.Sum();
List<int> result = new List<int>();
foreach (int i in list)
{
    temp = temp + i;
    result.Add(Math.Abs(  ( sum - temp) - temp));
}
return  result.Min();

This is hurting you on the space requirements -- you're generating a list of every subtraction. Then it's hurting you on time requirement again because it has to iterate the entire result list to find the lowest number.

You can do the entire thing with two passes of the input. You're doing 4.

I'll also say that you should almost never have a variable named temp. Give it a name that describes what it actually is. I used the names "left" and "right".

Left starts at 0, Right starts at the Sum. For each number, add it to Left, subtract it from Right. Much easier to read.

Update
I actually realized that in addition to everything I said, there's a major logic flaw in your version, which is probably what's hurting you more than the other stuff:

What happens if your code tries to handle {3, -1}?

First pass:

temp = 3
sum = 2

|(2 - 3) - 3| = 4

There should be no second pass, but you do one anyway:
Second pass:

temp = 2
sum = 2

|(2 - 2) - 2| = 2


Return a minimum number of ranges from a collection of ranges I'm new to Python and SWE in general so excuse the simple questions. I was given the following coding challenge by an interviewer. And I came up with the following solution. But I was passed over because it didn't meet their performance criteria. I was wondering if anyone could give me pointers on how I can do better on this question and in general for questions like this. I've found other answers solving the question, but I wanted specific answers to my implementation.

Here is the feedback I received:

The while (zip_range_list):line sticks out: you don't see a lot of
while loops in Python, you don't have to put parentheses around the
test expression, and solving this problem with a while loop is a
weird thing to do. Why are while loops a bad idea?
Adding a range to reduced_zip_ranges before it's reduced, and then
continually referring to the element you just added as
reduced_zip_ranges[-1] instead of having a separate binding for it
reads awkwardly. Why is this awkward?
The construct range_check = range(low-1, high+2) may be correct, but it's both strange to look at and ridiculously space-wasteful: instead of comparing endpoints he builds a list of the entire range of numbers just to check
membership in that range. He builds these over and over again in a
loop within a loop. I see the point here. I was trying to avoid a
long if-statement. Wasn't a good idea.
Speaking of "loop within a loop", this is an O(N-squared) algorithm when it could have been O(N) after the sort. I guess I overlooked this, I see 0(n^2) now. How can I avoid this?
The routine has two different non-exceptional return points; the one within the loop is unnecessary (the code works as well with it commented out).

PROBLEM
Given a collection of zip code ranges (each range includes both
their upper and lower bounds),
provide an algorithm that produces the minimum number of ranges required
to represent the same coverage as the input.

Input: [[14,17], [4,7], [2,5], [10,12] , [15,16], [4,9], [11,13]]
Output: [[2,17]]

# Implementation
def zip_range_reducer(zip_range_list):

    if not zip_range_list:
        raise Exception("Empty list of ranges provided!")

    reduced_zip_ranges = []

    zip_range_list.sort()

    while (zip_range_list):
        no_overlap_ranges = []
        reduced_zip_ranges.append(zip_range_list[0])
        if len(zip_range_list) == 1:
            return reduced_zip_ranges
        zip_range_list.pop(0)
        for zip_range in zip_range_list:
            low, high = reduced_zip_ranges[-1][0], reduced_zip_ranges[-1][1]
            range_check = range(low-1, high+2)
            if zip_range[0] in range_check or zip_range[1] in range_check:
                reduced_zip_ranges[-1][0] = min(reduced_zip_ranges[-1][0], zip_range[0])
                reduced_zip_ranges[-1][1] = max(reduced_zip_ranges[-1][1], zip_range[1])
            else:
                no_overlap_ranges.append(zip_range)
        zip_range_list = no_overlap_ranges

    return reduced_zip_ranges

I’ll try to go over your five points. Keep in mind that I’m not your interviewer and my interpretation might not be what they really meant.

While loops are not bad per se, but they are not the preferred way to iterate over a datastructure. I started solving the exercise also using a while loop to reduce a partially reduced list until reaching a fixed point. But I realized that, once the original list is sorted, you only need a single pass (more on that later). This is why solving this problem with a while loop is a weird thing to do.
Compare these toy examples:

counts = []
running_total = 0
for amount, should_separate in some_input:
    running_total += amount
    if should_separate:
        counts.append(running_total)
        count = 0

and

counts = [0]
for amount, should_separate in some_input:
    counts[-1] = counts[-1] + amount
    if should_separate:
        counts.append(0)

even though the second one is shorter, the core of the computation (counts[-1] = counts[-1] + amount) loose its readability due to the indexing. Plus, using one less variable, you lose the opportunity to document the code using an expressive name.
I’m assuming Python 2 here, as in Python 3 range produces a range object, pretty much the same as xrange in Python 2. So yes, use xrange instead, you’ll save both on memory and time as checking if an element is in a list is \$\mathcal{O}(n)\$. But for such simple checks where you already have the bounds up-front, you can use the shorthand comparisons: if low - 1 <= value <= high + 1:. Especially given the fact that:
Once the list is sorted, only two consecutive ranges can be part of the same "mega"range. If they are not, then you know your output contains another "mega"range that starts at this moment because no other element will start before this new range. This also simplifies the check for range inclusion as you only need to check for the lower bound of the current element to be in the current range; and since items are sorted, you know that that lower bound won't be lower than the lower bound of the current range either.
The ….pop(0) will make the for loop a no-op when initial length is 1, so the loop will stop itself on the next iteration, so the len(…) == 1 check is really redundant with the rest of your code. Drop code that serve no purpose as it is error-prone.

Others remarks I can make:

Why generate an exception on empty input? Simply return an empty list as output: no range in, no range out, I don't see any problem. You won't even enter the while so return an empty list anyway, again, this code is somewhat redundant with the rest.
Why modify the original list by .sort()ing in place? Since you return a different result, you should not modify the input.

Proposed improvements:

def zip_range_reducer(ranges):
    if not ranges:
        return []

    ranges = sorted(ranges)
    reduced = []

    lowest, highest = ranges[0]
    for low, high in ranges:
        # Since we sorted ranges, we know of overlapping
        # ranges by this only check.
        if low <= highest + 1:
            lowest = min(lowest, low)
            highest = max(highest, high)
        else:
            reduced.append([lowest, highest])
            lowest = low
            highest = high
    reduced.append([lowest, highest])

    return reduced

Another interesting way to put this code and remove the boilerplate (empty check, first item extraction) is to use an helper generator function that will trigger interesting behaviours of how iteration works in Python:

def zip_range_reducer(ranges):
    ranges = iter(sorted(ranges))
    return list(_zip_range_helper(ranges))

def _zip_range_helper(sorted_ranges_iterator):
    lowest, highest = next(sorted_ranges_iterator)

    for low, high in sorted_ranges_iterator:
        # Since we have sorted ranges, we know
        # of overlaping ranges by this only check.
        if low <= highest + 1:
            lowest = min(lowest, low)
            highest = max(highest, high)
        else:
            yield [lowest, highest]
            lowest = low
            highest = high

    yield [lowest, highest]

Here the empty check is implicit by the fact that next will raise StopIteration when ranges is empty. But this StopIteration will be interpreted by list() as "there is no more elements to put in here" and an empty list will be returned. Otherwise the first element will be consumed to populate lowest and highest; and each element of the resulting list will be yielded one by one.


Martix of Counts of Regex Hits Over a List of Strings I have:

A dataframe with Identifiers (TermId) and Search Terms (SearchTerm).
A list of text strings (MyText).  This likely would be a column (series) in a dataframe.

I for every string in MyText I want to get a count of hits for every regex SearchTerm, producing a table of counts.  This is a scaled down example.  I'd have 10,000 of Mytext and 100s of SearchTerms.  I am newer to Python (from R) and would like to know where can I optimize the code to make it more performant?  I'm open to what ever feedback people have to offer on all aspects of the code.
## Dependencies
import re
import pandas as pd

## Data
MyText = ['I saw xx.yy and also xx.yyy', 'FireFly* this is xx_yy there', 'I xx$YY', 'I see x.yy now .NET', 'now xx.yyxx.yyxx.yy']

MyDictionary = pd.DataFrame({
    'TermId': ['SK_{}'.format(x) for x in list(range(0, 6))],
    'SearchTerm': ['xx[.]yy', '[.]NET', 'A[+]', 'FireFly[*]', 'NetFlix[$]',  'Adobe[*]']
})

## Convert Search Term to Compiled Regex Object
MyDictionary['Regexes'] = [re.compile(s) for s in MyDictionary['SearchTerm']]

## List comprehension to double loop over the regexes &
## elements of MyText to count hits
out = {id: [len(regex.findall(s)) for s in MyText] for regex, id in zip(MyDictionary['Regexes'], MyDictionary['TermId'])}
out = pd.DataFrame(out)
out['MyText'] = MyText

## Results
print(out)
print(MyDictionary)

Which yields:
   SK_0  SK_1  SK_2  SK_3  SK_4  SK_5                        MyText
0     2     0     0     0     0     0   I saw xx.yy and also xx.yyy
1     0     0     0     1     0     0  FireFly* this is xx_yy there
2     0     0     0     0     0     0                       I xx$YY
3     0     1     0     0     0     0           I see x.yy now .NET
4     3     0     0     0     0     0           now xx.yyxx.yyxx.yy

An idiomatic approach would be to put MyText in a dataframe (if not already) and assign() the results of str.count().
I'd also suggest using an actual dict for the regex mapping. It's not strictly necessary, but the syntax is just cleaner given that this use case aligns with dict. (Here I created terms from MyDictionary just for continuity with the existing code.)
terms = dict(zip(MyDictionary.TermId, MyDictionary.SearchTerm))
df = pd.DataFrame({'MyText': MyText})

df = df.assign(**{key: df.MyText.str.count(val) for key, val in terms.items()})

#                          MyText  SK_0  SK_1  SK_2  SK_3  SK_4  SK_5
# 0   I saw xx.yy and also xx.yyy     2     0     0     0     0     0
# 1  FireFly* this is xx_yy there     0     0     0     1     0     0
# 2                       I xx$YY     0     0     0     0     0     0
# 3           I see x.yy now .NET     0     1     0     0     0     0
# 4           now xx.yyxx.yyxx.yy     3     0     0     0     0     0
```


Parse YAML file with nested parameters as a Python class object I would like to use a YAML file to store parameters used by computational models developed in Python. An example of such a file is below:

params.yaml

reactor:
  diameter_inner: 2.89 cm
  temperature: 773 kelvin
  gas_mass_flow: 1.89 kg/s

biomass:
  diameter: 2.5 mm                # mean Sauter diameter (1)
  density: 540 kg/m^3             # source unknown
  sphericity: 0.89 unitless       # assumed value
  thermal_conductivity: 1.4 W/mK  # based on value for pine (2)

catalyst:
  density: 1200 kg/m^3                            # from MSDS sheet
  sphericity: 0.65 unitless                       # assumed value
  diameters: [[86.1, 124, 159.03, 201], microns]  # sieve screen diameters
  surface_areas:
    values:
      - 12.9
      - 15
      - 18
      - 24.01
      - 31.8
      - 38.51
      - 42.6
    units: square micron

Parameters for the Python model are organized based on the type of computations they apply to. For example, parameters used by the reactor model are listed in the reactor section. Units are important for the calculations so the YAML file needs to convey that information too.

I'm using the PyYAML package to read the YAML file into a Python dictionary. To allow easier access to the nested parameters, I use an intermediate Python class to parse the dictionary values into class attributes. The class attributers are then used to obtain the values associated with the parameters. Below is an example of how I envision using the approach for a much larger project:

params.py

import yaml

class Reactor:

    def __init__(self, rdict):
        self.diameter_inner = float(rdict['diameter_inner'].split()[0])
        self.temperature = float(rdict['temperature'].split()[0])
        self.gas_mass_flow = float(rdict['gas_mass_flow'].split()[0])

class Biomass:

    def __init__(self, bdict):
        self.diameter = float(bdict['diameter'].split()[0])
        self.density = float(bdict['density'].split()[0])
        self.sphericity = float(bdict['sphericity'].split()[0])

class Catalyst:

    def __init__(self, cdict):
        self.diameters = cdict['diameters'][0]
        self.density = float(cdict['density'].split()[0])
        self.sphericity = float(cdict['sphericity'].split()[0])
        self.surface_areas = cdict['surface_areas']['values']

class Parameters:

    def __init__(self, file):

        with open(file, 'r') as f:
            params = yaml.safe_load(f)

        # reactor parameters
        rdict = params['reactor']
        self.reactor = Reactor(rdict)

        # biomass parameters
        bdict = params['biomass']
        self.biomass = Biomass(bdict)

        # catalyst parameters
        cdict = params['catalyst']
        self.catalyst = Catalyst(cdict)

example.py

from params import Parameters

pm = Parameters('params.yaml')

# reactor
d_inner = pm.reactor.diameter_inner
temp = pm.reactor.temperature
mf_gas = pm.reactor.gas_mass_flow

# biomass
d_bio = pm.biomass.diameter
rho_bio = pm.biomass.density

# catalyst
rho_cat = pm.catalyst.density
sp_cat = pm.catalyst.sphericity
d_cat = pm.catalyst.diameters
sa_cat = pm.catalyst.surface_areas

print('\n--- Reactor Parameters ---')
print(f'd_inner = {d_inner}')
print(f'temp = {temp}')
print(f'mf_gas = {mf_gas}')

print('\n--- Biomass Parameters ---')
print(f'd_bio = {d_bio}')
print(f'rho_bio = {rho_bio}')

print('\n--- Catalyst Parameters ---')
print(f'rho_cat = {rho_cat}')
print(f'sp_cat = {sp_cat}')
print(f'd_cat = {d_cat}')
print(f'sa_cat = {sa_cat}')

This approach works fine but when more parameters are added to the YAML file it requires additional code to be added to the class objects. I could just use the dictionary returned from the YAML package but I find it easier and cleaner to get the parameter values with a class interface.

So I would like to know if there is a better approach that I should use to parse the YAML file? Or should I organize the YAML file with a different structure to more easily parse it?

If you split the configuration fields into magnitude and unit (as you've already done for surface_areas) you won't have to split and parse them in code.
If you then convert your configuration to JSON you won't need to convert strings to numbers. JSON strings must be quoted, and numbers must be unquoted, so the json module will simply do those conversions for you.

Other than that:

Configuration handling should be separate from building other objects - that way it's easy to use your code whether the configuration comes from a file or from command-line parameters.
Accessing properties two levels deep (such as pm.biomass.diameter) violates the Law of Demeter. You could write for example an as_parameter_list for each class to get a representation like f'rho_cat = {rho_cat}' etc.


Meet Results Displayer I decided to dive into the world of JS/JQuery by trying to build a simple web app to display all of my various meet results in an easy-to-view manner. For this stage of the review, all it does is generate a table of all the meets and log some stuff to the console when you click on it.

The app gets its data as JSON in a form like:

[{"Date": "MM/DD/YYYY", "Name": "Some Event", "Sport": "Run", "Events": [...]},
 {"Date": "MM/DD/YYYY", "Name": "Some Other Event", "Sport": "Swim", "Events": [...]},
 ...
 ]

and displays them in a table like:

Additionally, I'm storing the "Events" part in each cell so that when I click on a cell, I can eventually do something interesting with the data.

Is there anything I'm doing that violates some key language principles? Is what I'm doing with events too hacky? Is there a simpler way to build up the table? All comments welcome.

Date	Run	Swim

High Level Notes

My most substantial suggestion is to avoid mixing data logic and view code.  Transform your data, as pure data, into the form you would like it to be so that your view code can treat it uniformly.  Then your view will be more like a "dumb" mail-merge template, even if you decide to keep using jquery to build it up programmatically.  A rewrite should clarify what I mean:

This will take each event that the ajax returns and transform it into data for both of the row cells:

function eventToRowData(event) {
    var nullCell = {"Name": '', "Sport":null, "Events": null},
        isRun = event.Sport == 'Run';
    return {
        "Date": event.Date,
        "RunCell": isRun ? event : nullCell,
        "SwimCell": isRun ? nullCell : event
    };
}

Now all the if...else logic vanishes from your view code:

function buildTable(data) {
    var rowData = data.map(eventToRowData),
        tableBody = document.createElement("tbody");

    rowData.forEach(makeRow);
    $("#my_events").append(tableBody);

    function makeRow(rowDatum) {
        var row = tableBody.insertRow();
        row.insertCell().appendChild(document.createTextNode(rowDatum.Date));
        addEventCell(rowDatum.RunCell);
        addEventCell(rowDatum.SwimCell);

        function addEventCell(cellData) {
            var cell = row.insertCell();
            cell.className = cellData.Sport ? 'Event' : '';
            cell.Events = cellData.Events;
            cell.appendChild(document.createTextNode(cellData.Name));
        }
    }
}

See below for full working snippet.

Other Notes

To mitigate nesting, avoid defining things like success callbacks inline, unless they are so short they can fit on a single line.  This will make both the callback itself and whatever is calling it (in your case, an ajax options object) more readable.
Table row striping and hover highlighting belong in css.  As a rule, most view things that can be done in css should.
Consider ditching jquery.  The only good reason to have it in this code is simplifying your ajax call.  You could use zepto instead, or do it raw, and save some KB. See this link for more.
Consider using a light template library to clean up the view code which builds the table.
Consider using mithril.js (an extremely light library with the same philosophy as React) if this becomes more complex, or if you are interested in taking the separation of data view even further.
I didn't fix all the trivial things like naming conventions.  js is camelCase, css is dash-case by convention.
Finally, this may not be relevant, since I get the sense you are just doing this to learn, but unless you have more complex, interactive features, I think server side code rendering a static site is more appropriate for this use case.

var sampleData = [{
    "Date": "MM/DD/YYYY",
    "Name": "Some Event",
    "Sport": "Run",
    "Events": [1, 2]
}, {
    "Date": "MM/DD/YYYY",
    "Name": "Some Other Event",
    "Sport": "Swim",
    "Events": [1, 2]
}, {
    "Date": "MM/DD/YYYY",
    "Name": "Another Event",
    "Sport": "Swim",
    "Events": [1, 2]
}, ]

// Rather than work with data in an inconvenient format, change
// it to a format that suits our view code.
function eventToRowData(event) {
    var nullCell = {"Name": '', "Sport":null, "Events": null},
        isRun = event.Sport == 'Run';
    return {
        "Date": event.Date,
        "RunCell": isRun ? event : nullCell,
        "SwimCell": isRun ? nullCell : event
    };
}

function buildTable(data) {
    var rowData = data.map(eventToRowData),
     tableBody = document.createElement("tbody");

    rowData.forEach(makeRow);
    $("#my_events").append(tableBody);

    function makeRow(rowDatum) {
        var row = tableBody.insertRow();
        row.insertCell().appendChild(document.createTextNode(rowDatum.Date));
        addEventCell(rowDatum.RunCell);
        addEventCell(rowDatum.SwimCell);

        function addEventCell(cellData) {
         var cell = row.insertCell();
         cell.className = cellData.Sport ? 'Event' : '';
            cell.Events = cellData.Events;
            cell.appendChild(document.createTextNode(cellData.Name));
     }
    }
}

$(document).ready(buildTable(sampleData))
table, th, td {
    border: 1px solid black;
}
td {
    text-align: center;
    vertical-align: middle;
    padding: 5px;
}
#my_events tbody tr:nth-child(even) {
   background-color: #ccc;
}
td.Event:hover { background: lime }

            Date
            Run
            Swim