A baby monitor using a Raspberry Pi to send push notifications when triggered by noise I have a Raspberry Pi 3B with a USB microphone. I have used a combination of Bash and Python scripts to detect noise levels above a certain threshold that trigger a notification on my phone. The actual sound is not streamed.

The scripts are called using a command line alias for the following:

bash /home/pi/babymonitor.sh | /home/pi/monitor.py

Bash script - babymonitor.sh

This uses arecord to repeatedly record 2-second snippets via a temporary file, sox and grep to get the RMS (root mean square) amplitude (see this example for sox stat output) and then tail to get only the numerical data at the end of the line. This is then piped to the Python script.

#!/bin/bash
trap break INT
while true; do
    arecord --device=hw:1,0 --format S16_LE --rate 44100 -d 2 /dev/shm/tmp_rec.wav ; sox -t .wav /dev/shm/tmp_rec.wav -n stat 2>&1 | grep "RMS     amplitude" | tail -c 9
done

Python processing script - monitor.py

This receives the volume data and compares it to an arbitrarily defined threshold that I chose based on testing. If a noise is detected in one of the 2-second snippets, a push notification is sent and it suppresses further notifications for 10 seconds (5 cycles of 2-second snippets).

#!/usr/bin/env python

import sys
import push

THRESHOLD = 0.01

count = 0
suppress = False

while True:
    try:
        line = sys.stdin.readline().strip() # e.g. "0.006543"
        number = float(line)
        if number > THRESHOLD and not suppress:
            p = push.PushoverSender("user_key", "api_token")
            p.send_notification("There's a noise in the nursery!")
            count = 0
            suppress = True
        elif suppress:
            print("Suppressing output")
        else:
            print("All quiet")

        # Count 5 cycles after a trigger
        if suppress:
            count += 1
        if count >= 5:
            count = 0
            suppress = False

    except ValueError:
        # Cannot coerce to float or error in stdin
        print("Value error: " + line)
        break

    except KeyboardInterrupt:
        # Cancelled by user
        print("Baby monitor script ending")
        break

sys.exit()

Script for push notification service - push.py

This sends an http request to the Pushover service, which results in a push notification on my phone.

#!/usr/bin/env python

import httplib
import urllib

class PushoverSender:
    def __init__(self, user_key, api_key):
        self.user_key = user_key
        self.api_key = api_key

    def send_notification(self, text):
        conn = httplib.HTTPSConnection("api.pushover.net:443")
        post_data = {'user': self.user_key, 'token': self.api_key, 'message': text}
        conn.request("POST", "/1/messages.json", urllib.urlencode(post_data), {"Content-type": "application/x-www-form-urlencoded"})
        print(conn.getresponse().read())

I would be grateful for any feedback on how I can improve my code or the conceptual aspects of this project. I am not very familiar with Bash scripting so feedback is particularly welcome on this.

Is it acceptable to have the Bash script and Python script called
together with a pipe between them and both running while True
loops? It certainly seems to work.
Is there a better way to exit? Current pressing
ctrl+c will terminate both the Bash script
(due to trap break INT) and the Python script (due to except
KeyboardInterrupt).

Bash

    arecord --device=hw:1,0 --format S16_LE --rate 44100 -d 2 /dev/shm/tmp_rec.wav ; sox -t .wav /dev/shm/tmp_rec.wav -n stat 2>&1 | grep "RMS     amplitude" | tail -c 9

You can use newlines instead of ; and after | to make it easier to see the separate commands:

    arecord --device=hw:1,0 --format S16_LE --rate 44100 -d 2 /dev/shm/tmp_rec.wav
    sox -t .wav /dev/shm/tmp_rec.wav -n stat 2>&1 |
        grep "RMS     amplitude" |
        tail -c 9

It's not clear to me why you're using /dev/shm rather than a pipe. Since you say you're not very familiar with bash, I wonder whether it's because you saw in the man page that sox requires a filename and didn't know that it's common for - to be used as a special filename to signify stdin. Untested, but on my reading of the man pages the following should work:

    arecord --device=hw:1,0 --format S16_LE --rate 44100 -d 2 |
        sox -t .wav - -n stat 2>&1 |
        grep "RMS     amplitude" |
        tail -c 9

I think the extraction of the amplitude could be more robust in two ways:

As a minor point, I suggest changing the regex to "RMS *amplitude" as a future-proofing precaution against the stats gaining a new stat with a longer name.
More importantly, tail -c 9 is a very bold assumption. The Python program uses strip(), so you don't care about leading whitespace. I propose replacing the tail with cut -d: -f2.

Final version of the bash script if you agree with all of my suggestions:

    arecord --device=hw:1,0 --format S16_LE --rate 44100 -d 2 |
        sox -t .wav - -n stat 2>&1 |
        grep "RMS *amplitude" |
        cut -d: -f2

Python

I don't have much to say here except that count and suppress looks to me to be at least one variable too many. If you replace with e.g. lines_to_suppress then if suppress becomes if lines_to_suppress <= 0 and instead of incrementing count you decrement lines_to_suppress.

However, it might be clearest to just eliminate them both and replace

            count = 0
            suppress = True

with

            for _ in range(0, 5):
                sys.stdin.readline()
                print("Suppressing output")


Displaying a paginated list of forum topics using PHP PDO Can someone tell me if this code is safe? Can be sql-injected or something else hacked? Code get some rows from db and show in pages with pagination... if i can improve let me know and show me how, thanks.

$conn = new PDO('mysql:host=localhost;dbname=admin_admin', 'admin_admin', 'password');
     $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
     $num_rows = $conn->query('SELECT COUNT(*) FROM a_topics')->fetchColumn();
     $pages = new Paginator($num_rows,25,array(25,50,100,250,'All'));
     $stmt = $conn->prepare('SELECT a_topics.pid, a_topics.title, a_topics.forum_id, b_forums.id, b_forums.name
        FROM a_topics INNER JOIN b_forums ON a_topics.forum_id = b_forums.id
        ORDER BY a_topics.pid DESC LIMIT :start,:end');
     $stmt->bindParam(':start', $pages->limit_start, PDO::PARAM_INT);
     $stmt->bindParam(':end', $pages->limit_end, PDO::PARAM_INT);
     $stmt->execute();
     $result = $stmt->fetchAll();
     echo $pages->display_jump_menu().$pages->display_items_per_page();
     echo $pages->display_pages();
        foreach($result as $row) {
        echo "$row[0] - $row[1] - $row[2]";
     } catch(PDOException $e) {
     echo 'ERROR: ' . $e->getMessage();
     }

This code is safe from the SQL injection standpoint but it is likely prone to XSS because of the untreated output, and also to possibly leak the sensitive information due to the error message unconditionally spat out.

Regarding other improvements I would suggest a more robust connection code and to avoid bindParam calls thanks to disabled emulation mode.

The rewritten code would be

include 'pdo.php';
$num_rows = $conn->query('SELECT COUNT(*) FROM a_topics')->fetchColumn();
$pages = new Paginator($num_rows,25,array(25,50,100,250,'All'));
$stmt = $conn->prepare('SELECT a_topics.pid, a_topics.title, a_topics.forum_id, b_forums.id, b_forums.name
    FROM a_topics INNER JOIN b_forums ON a_topics.forum_id = b_forums.id
    ORDER BY a_topics.pid DESC LIMIT :start,:end');
$stmt->execute(['start' => $pages->limit_start, 'end' => $pages->limit_end]);
$result = $stmt->fetchAll();

echo $pages->display_jump_menu().$pages->display_items_per_page();
echo $pages->display_pages();
foreach($result as $row) {
    echo htmlspecialchars($row[0]),
         " - ",
         htmlspecialchars($row[1]),
         " - ",
         htmlspecialchars($row[2])";
}


Adding two Integers in an Array I was wondering if I could get a second look at my program before I turn it in. The job was to code a program in java that adds two integers to an array gather the sum while displaying the integers that belong in the carry column section. I am curious if there is a simpler way to do what I am trying to do.
I hope the output is enough to give you the idea behind the program.

Here is a sample output.

Enter the first number : 9999
Enter the second number : 9999
Carry : 1110
Num1 : 9999
Num2 : 9999
Sum : 19998
Continue(Y/N)

import java.util.Arrays;
import java.util.Scanner;
import java.util.*;

public class Main {

public static void main(String args[]) {

 Integer[]  num1 = new Integer[10];
 Integer[]  num2 = new Integer[10];
 Integer[]  temp = new Integer[10];
 Integer[] carry = new Integer[10];
 Integer[]   sum = new Integer[10];

Scanner input = new Scanner(System.in);

do{

     int g;
     int a=0;
     int b=0;
     int t;
     int s1;
     int c=0;

     //Read two integer values from user input
     System.out.print("\n Enter the first number : ");
     int m = input.nextInt();

     System.out.print("\n Enter the second number : ");
     int n = input.nextInt();

     for(int i = 0; i < 10; i++){

         num1[i]=0;
         num2[i]=0;

     }

      //Placing the two integer numbers into array
     while(m > 0){

         g = m % 10;
         m = m / 10;
         num1[a] = g;
         temp[a] = g;
         a++;

     }

     while(n > 0){

         g = n % 10;
         n = n / 10;
         num2[b] = g;
         b++;
     }

     if(a > b) c = a;
     else   c = b;

     for(int i = 0; i < c; i++){

         carry[i]=0;
         sum[i]=0;

     }

     //finding sum and handling carry
     for(int i = 0; i < c; i++){

         sum[i] = num1[i]+num2[i];
         s1=sum[i];
         t=0;

         while(s1 > 0){

             s1= s1 / 10;
             t++;

         }

         if(t > 1 && ((i+1)<c)){

             carry[i+1] = 1;
             num1[i+1] = num1[i+1] + 1;
             sum[i] = sum[i] % 10;

         }

     }

      // Printing the sum value to user
     System.out.print("\n Carry : " );
     for(int x = c-1; x >= 0; x--)
         System.out.print(carry[x]);

     System.out.print("\n");
     System.out.print("\n Num1 : " );
     for(int x=c-1;x>=0;x--)
         System.out.print(temp[x]);
     System.out.print("\n");

     System.out.print("\n Num2 : " );
     for(int x=c-1;x>=0;x--)
         System.out.print(num2[x]);
     System.out.print("\n");

     System.out.print("\n Sum : " );
     for(int x=c-1;x>=0;x--)
         System.out.print(sum[x]);
     System.out.print("\n");

 } while(Continue());
}

 public static boolean Continue(){

  char choice;

   Scanner input = new Scanner(System.in);
   System.out.println("Continue(Y/N)");
   choice = input.next().charAt(0);

   return Character.toLowerCase(choice) == 'y';

  }

}

Code quality has 3 priorities:

the code works as expected
the code is easy to read and does not surprise the reader
the code is (almost) free from duplications

Please read the following with this 3 priorities in mind.

Naming

Finding good names is the hardest part in software development so always take your time to improve the names of your identifiers.

Single letter names

Do not use single letter names or abbreviations. There are only few exceptions like loop variables or very common  abbreviations. While reading this code your brain has to do additional work to resolve single letters to their meaning.

Always use expressive names for your identifiers taken from the problem domain, not from the technical solution.

Naming Conventions

Please keep to the Java Naming Conventions. This will make the life of your coworkers easier as it does for you when your coworkers follow this suggestion.

eg.:
You have this method:

public static boolean Continue()

In Java only class names start with an uppercase letter. Also the method name (if correctly spelled) would collide with a reserved Java key word (which is no technical problem since java does distinguish between keywords and method names). This may confuse the reader.

Also since the method returns a boolean its name should start with is or has.

Names should express intention

Your method Continue() reveals another problem: it does not what its name implies.

What it really does is to ask the user what to do.
So its name should be ie.:  isUserRepeating()

Comments

Comments should always describe why the code is like it is.

Your comments somehow structure the code into individual sections.
You should better extract those individual sections to separate methods with names derived from the comments you wrote.

When doing so this will support a very important principle of object oriented programming: separation of concerns.

Visibility scopes

All your variables are declared at the top of your
main method or the top of your main loop.

You should reduce the visibility to the least possible block which means that you should declare them at first use. This will help to refactor your code later.

Code duplications

You have lots of duplicated code which does merrily the same but  the names of the variables differ.
eg.:

 //Placing the two integer numbers into array
 while(m > 0){
     g = m % 10;
     m = m / 10;
     num1[a] = g;
     temp[a] = g;
     a++;
 }
 while(n > 0){
     g = n % 10;
     n = n / 10;
     num2[b] = g;
     b++;
 }

The only difference is that the first loop additionally sets the values in the temp array.

So the better approach would be to fill the temp array in its own loop (for now, but there are even better approaches):

 //Placing the two integer numbers into array
 while(m > 0){
     g = m % 10;
     m = m / 10;
     num1[a] = g;
     a++;
 }
 a=0; // reset the currentDigitIndex
 while(m > 0){
     g = m % 10;
     m = m / 10;
     temp[a] = g;
     a++;
 }
 while(n > 0){
     g = n % 10;
     n = n / 10;
     num2[b] = g;
     b++;
 }

now you can create a method that does the loop:

private static int placeIntegerIntoArray(int numberToSplit, Integer[] individualDigits){
   int currentDigitIndex=0;
   while(numberToSplit > 0){
         int currentDigit = numberToSplit % 10;
         numberToSplit = numberToSplit / 10;
         individualDigits[currentDigitIndex] = currentDigit;
         currentDigitIndex++;
     }
     return currentDigitIndex;
}

and in your main you change to:

a = placeIntegerIntoArray(m, num1);
placeIntegerIntoArray(m, temp); // initialite to same as num1
b = placeIntegerIntoArray(n, num2);

BTW: this removes variable g from the scope of the main method.

Magic numbers.

Your code uses some literal numbers. You should extract them to constants so that you can give them meaningful names.

eg.: the literal number 10 in

 sum[i] = sum[i] % 10;

might be the SINGLE_DIGIT_MAX_VALUE

whereas in

 Integer[]  num1 = new Integer[10];

it might be the MAXIMUM_DIGIT_LENGTH


Quant Trading Robot #1 - API Calls I'm (re)building a Quantitative Trading Robot in Python.

Description:

This is my module for API Calls. The API documentation can be found here. This is the direct interface layer between my code and the exchange server. Any abstraction on top of the direct API calls is in a separate module.

Classes:

GDAXExchangeAuth - Authentication for restricted API Calls

Functions:

get_server_time - Retrieve the curent server time

get_account - Retrieve Account balances

get_book - Retrieve the order book for a specified currency pair

get_fills_after_order - Retrieve all filled orders that have occurred after some specific order id.

post_limit_order - Post an order to the exchange

get_price_history - Retrieve the price history for a specific currency pair

Areas of interest:

The usual, am I following conventions, how's my naming, how's my documentation, how well thought-out is my structure?

import json
import hmac
import hashlib
import requests
import base64
from requests.auth import AuthBase

API_KEY = 'redacted'
API_SECRET = 'redacted'
API_PASSPHRASE = 'redacted'
API_URL = 'https://api.gdax.com/'

class GDAXExchangeAuth(AuthBase):
    SIGNATURE_HTTP_HEADER = 'CB-ACCESS-SIGN'
    TIMESTAMP_HTTP_HEADER = 'CB-ACCESS-TIMESTAMP'
    KEY_HTTP_HEADER = 'CB-ACCESS-KEY'
    PASSPHRASE_HTTP_HEADER = 'CB-ACCESS-PASSPHRASE'

    def __init__(self, api_key, secret_key, passphrase, server_time):
        self.api_key = api_key
        self.secret_key = secret_key
        self.passphrase = passphrase
        self.server_time = server_time

    def __call__(self, request):
        epoch = self.server_time['epoch']
        timestamp = str(epoch)
        request.headers.update(
            {
                'CB-ACCESS-TIMESTAMP': timestamp,
                'CB-ACCESS-KEY': self.api_key,
                'CB-ACCESS-PASSPHRASE': self.passphrase,
                'Content-Type': 'application/json'
            }
        )

        message = timestamp + request.method + request.path_url + (request.body or '')
        message = message.encode()
        hmac_key = base64.b64decode(self.secret_key)
        signature = hmac.new(hmac_key, message, hashlib.sha256)
        signature_b64 = base64.b64encode(signature.digest())

        request.headers.update(
            {
                'CB-ACCESS-SIGN': signature_b64,
                'Content-Type': 'application/json'
            }
        )

        return request

def get_server_time():
    url = API_URL + 'time'
    r = requests.get(url)
    return r

def get_account(server_time):
    auth = GDAXExchangeAuth(API_KEY, API_SECRET, API_PASSPHRASE, server_time)
    url = API_URL + 'accounts'
    r = requests.get(url, auth=auth)
    return r

def get_book(product, level):
    # level 1 = Bid/Ask
    url = API_URL + '/products/' + product + '/book?level=' + str(level)
    r = requests.get(url)
    return r

def get_fills_after_order(server_time, order_id):
    # get all filled orders after the order id.
    # max 100 results returned
    auth = GDAXExchangeAuth(API_KEY, API_SECRET, API_PASSPHRASE, server_time)
    url = API_URL + 'fills?before=' + str(order_id)
    r = requests.get(url, auth=auth)
    return r

def post_limit_order(server_time, side, price, size, product_id):
    order_type = 'limit'

    order = {}
    order['side'] = side
    order['price'] = price
    order['size'] = size
    order['product_id'] = product_id
    order['type'] = order_type

    auth = GDAXExchangeAuth(API_KEY, API_SECRET, API_PASSPHRASE, server_time)
    url = API_URL + 'orders'
    data = json.dumps(order)
    r = requests.post(url, data=data, auth=auth)
    return r

def get_price_history(product_id, start, end, granularity):
    params = {}
    params['start'] = start
    params['end'] = end
    params['granularity'] = granularity

    url = API_URL + '/products/' + product_id + '/candles'
    r = requests.get(url, params=params)
    return r

To check your code against coding standard and some error detection, I'd suggest you pylint, if you don't know it already.
Running pylint on your original code gives a list of things to check and a score 7.29/10 which is a good start.

What you should fix:

add docstrings at module, class and function level.
remove temporary variables when used just once.

r = requests.get(url, params=params)
return r

return requests.get(url, params=params)

prefer string formatting over concatenation.

url = API_URL + '/products/' + product_id + '/candles'
url = '%s/products/%s/candles' % (API_URL, product_id)

no need to initialize each dictionary's key separately.

order = {}
order['side'] = side
order['price'] = price
order['size'] = size
order['product_id'] = product_id
order['type'] = order_type

order = {
    'side': side,
    'price': price,
    'size': size,
    'product_id': product_id,
    'type': order_type
}

consider parameter injection for the GDAXExchangeAuth instance in all your functions, it will simplify testing or reusing of the code with a different Authentication method.

Pylint complains about GDAXExchangeAuth class having too few public methods, but in this case I like the usage of a function class and there is no real need of other methods, so for me it's fine.

I assume this is probably just an example, but in any case I suggest also getting sensitive information from environment variables, something like

    import os

    API_PASSPHRASE = os.environ['API_PASSPHRASE']


Simplifying finding neighbors in graph private static List<ConstrDirectedEdge> kruskalConstruct(ConstructionDigraph CG) {

    int current = CG.srcVertexIndex();
    boolean visited[] = new boolean[CG.V()];
    visited[current] = true;
    UF uf = new UF(CG.V());
    List<ConstrDirectedEdge> feasibleNeighbours = new ArrayList<ConstrDirectedEdge>();
    List<ConstrDirectedEdge> solution = new ArrayList<ConstrDirectedEdge>();

    do {
        /* clear neighbours from prev iteration */
        feasibleNeighbours.clear();

        /* build feasible neighbour list */
        for (ConstrDirectedEdge directedEdge : CG.adj(current)) {
            int v = CG.getVertex(directedEdge.to()).getSource();
            int w = CG.getVertex(directedEdge.to()).getDestination();

            if (!visited[directedEdge.to()] && !uf.connected(v, w)) {
                feasibleNeighbours.add(directedEdge);
            }
        }

        //TODO: code smell
        if (feasibleNeighbours.isEmpty()) {
            break;
        }

        /* calculate the probability for each neighbour */
        double R = calculateR(feasibleNeighbours);
        System.out.println("R for source is : " + R);
        for (ConstrDirectedEdge feasibleneighbour : feasibleNeighbours) {
            feasibleneighbour.calcProbability(R, alpha, beta);
        }

        /* pick a neighbour */
        ConstrDirectedEdge pickedUp = choiceEdgeAtRandom(feasibleNeighbours);
        visited[pickedUp.to()] = true;
        current = pickedUp.to();
        solution.add(pickedUp);
        uf.union(CG.getVertex(current).getSource(),   CG.getVertex(current).getDestination());
    } while (!feasibleNeighbours.isEmpty());

    return solution;
}

I want to eliminate the code smell that is the break in the middle of the loop. As can be seen I have chosen to use a do {} while() in order to do the initialization with the initial node being CG.srcVertexIndex(). What I was thinking about is make the following code:

        for (ConstrDirectedEdge directedEdge : CG.adj(current)) {
            int v = CG.getVertex(directedEdge.to()).getSource();
            int w = CG.getVertex(directedEdge.to()).getDestination();

            if (!visited[directedEdge.to()] && !uf.connected(v, w)) {
                feasibleNeighbours.add(directedEdge);
            }
        }

into a separate function that will be returning a List<ConstrDirectedEdge> and then the loop can turn into:

while(!(feasibleNeighbours = getFeasibleNeighbours(CG, visited,  uf)).isEmpty()) {
/* calculate the probability for each neighbour */
/* pick a neighbour */
}

But then I will have to pass visited and uf as parameter which in fact are going to be manipulated by getFeasibleNeighbours functions - in essence I will be using input parameters to store output state which is not a good idea.

Finally I could just make visited and uf static private vars and just reinitialize them when needed and use them directly, but again this seems kind of wrong.

The code is working fine, however I'd like to hear opinions on how this can be made more readable.

Your proposed getFeasibleNeighbours doesn't modify the two parameters. The visited array isn't modified at all. The cf is modified by the union-find process, but that's really more of a caching then a modification. So there isn't any concerns about the parameters of getFeasibleNeighbors.

} while (!feasibleNeighbours.isEmpty());

There doesn't seem to be any point in checking this here. It can't possibly have changed since you checked it in the middle of the loop.

while(!(feasibleNeighbours = getFeasibleNeighbours(CG, visited,  uf)).isEmpty()) {

You can do something like this, but I find it rather ugly.

In the general case of break-in-the-middle loops you have something like:

get_stuff_ready();
while(1)
{
     do_stuff_before();
     if( !some_test() )
           break;
     do_stuff_after();
}

This equivalent to:

get_stuff_ready();
do_stuff_before();
while( some_test() )
{
     do_stuff_after();
     do_stuff_before();
}

This does repeat do_stuff_before() twice. However, you can often combine get_stuff_ready() and do_stuff_before().

So you could something like:

private static List<ConstrDirectedEdge> kruskalConstruct(ConstructionDigraph CG) {

    int current = CG.srcVertexIndex();
    boolean visited[] = new boolean[CG.V()];
    visited[current] = true;
    UF uf = new UF(CG.V());
    List<ConstrDirectedEdge> solution = new ArrayList<ConstrDirectedEdge>();
    List<ConstrDirectedEdge> feasibleNeighbours = getFeasibleNeighbors(CG, current, uf);

    while( !feasibleNeighbours.isEmpty() )
    {

        ConstrDirectedEdge pickedUp = pickNeighbour(feasibleNeighbours);
        current = pickedUp.to();

        visited[current] = true;
        uf.union(CG.getVertex(current).getSource(),   CG.getVertex(current).getDestination());
        solution.add(pickedUp);

        feasibleNeighbours = getFeasibleNeighbors(CG, current, uf);
    }

    return solution;
}


Code to change the size of the fields based on that 2 fields are optional inside a contact form In my React app using Material UI, I wrote a code to manage the size of the fields of a contact form. Those fields are elements that can be added or removed depending on a configuration.
The code I wrote works as should be but looks ugly and I need help to make it better
let size = 12;
    if (useMediaQuery(theme.breakpoints.up('sm'))) {
      const hasTime = fieldsConfig.some(
        f => f.name === 'preferredContactHours',
      );
      const hasContactType = fieldsConfig.some(
        f => f.name === 'preferredContactWay',
      );
      if (name === 'phone' && (!hasTime || !hasContactType)) size = 7;
      else if (name === 'preferredContactWay' && hasTime) size = 6;
      else if (name === 'preferredContactWay' && !hasTime) size = 5;
      else if (name === 'preferredContactHours' && hasContactType) size = 6;
      else if (name === 'preferredContactHours' && !hasContactType) size = 5;
    }

The size by default is 12 so if any of the conditions are not applicable the field will be full size in the grid.
After the if the hasTime and hasContactType are about that those fields are optional so I need to see if they are present or not.
The rest is the sizing and the conditions about it regarding if I have or not the field then is a specific size.
What I need is to have it better and efficient then all that verbose way if that could be possible.

I think your current code looks pretty reasonable. All the logic you're typing out is required, and it's quite readable as-is. There are a few tweaks you can consider, but there could well be different opinions on whether it makes the code more elegant or not.
Array search You initially save into 2 variables whether a couple particular properties exist in the array of objects. You can make that a bit nicer by first mapping to an array of .names, then using .includes instead:
const configNames = fieldsConfig.map(config => config.name);
const hasTime = configNames.includes('preferredContactHours');
const hasContactType = configNames.includes('preferredContactWay');

Abstract into a function Your whole code snippet runs for the purpose of figuring out the size to use. Rather than having this block among whatever else may exist on that level, and rather than conditionally reassigning size in a bunch of places, you might consider a function that calculates the size instead:
const size = getSize();

(This also lets you define size with const, which is great)
Save the comparisons into variables Rather than performing the same test multiple times, you can save them into variables first, it'll help a bit:
const preferredContactWay = name === 'preferredContactWay';
const preferredContactHours = name === 'preferredContactHours';

Group the conditions that result in the same size (just an option, you may or may not like how it looks):
const getSize = () => {
  if (!useMediaQuery(theme.breakpoints.up('sm'))) {
    return 12;
  }
  const configNames = fieldsConfig.map(config => config.name);
  const hasTime = configNames.includes('preferredContactHours');
  const hasContactType = configNames.includes('preferredContactWay');

  if (name === 'phone' && (!hasTime || !hasContactType)) return 7;

  const preferredContactWay = name === 'preferredContactWay';
  const preferredContactHours = name === 'preferredContactHours';

  if (
    (preferredContactWay && hasTime) ||
    (preferredContactHours && hasContactType)
  ) {
    return 6;
  }
  else if (
    (preferredContactWay && !hasTime) ||
    (preferredContactHours && !hasContactType)
  ) {
    return 5;
  }
  return 12;
}

RE: comment, to call useMediaQuery in the caller of getSize, either do:
const size = useMediaQuery(theme.breakpoints.up('sm') ? 12 : getSize();

or
const size = getSize(useMediaQuery(theme.breakpoints.up('sm'));

altering the body of getSize as needed to check the parameter.


Opening TV episodes in a web browser I put this together and it seems to work.

1channel.py -breakingbad 1

The Output (1-10):

(1) - Currently 3.80/5
(2) - Currently 3.50/5

(1 opens the link in browser)

If anyone has any suggestions or wants to improve it:

from bs4 import BeautifulSoup
import urllib2, sys, webbrowser

def Work(tvshow):
    print "\n[*] Working...\n"

    try:

        f = urllib2.urlopen(tvshow)
        html = f.read()
        soup = BeautifulSoup(html)

        for table in soup.findAll('table',{"width":"100%"}):
            for a in table.findAll('a',{"target":"_blank"}):
                for li in table.findAll('li',{"class":"current-rating"}):

                    if a.text=="Version 2":
                        print "(1) - %s\n" % (li.text)
                        link1 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 3":
                        print "(2) - %s\n" % (li.text)
                        link2 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 4":
                        print "(3) - %s\n" % (li.text)
                        link3 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 5":
                        print "(4) - %s\n" % (li.text)
                        link4 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 6":
                        print "(5) - %s\n" % (li.text)
                        link5 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 7":
                        print "(6) - %s\n" % (li.text)
                        link6 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 8":
                        print "(7) - %s\n" % (li.text)
                        link7 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 9":
                        print "(8) - %s\n" % (li.text)
                        link8 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 10":
                        print "(9) - %s\n" % (li.text)
                        link9 = "http://www.1channel.ch" + a['href']

                    elif a.text=="Version 11":
                        print "(10) - %s\n" % (li.text)
                        link10 = "http://www.1channel.ch" + a['href']

        user = raw_input(">>> ")

        if user == '1':
            webbrowser.open(link1)
        elif user == '2':
            webbrowser.open(link2)
        elif user == '3':
            webbrowser.open(link3)
        elif user == '4':
            webbrowser.open(link4)
        elif user == '5':
            webbrowser.open(link5)
        elif user == '6':
            webbrowser.open(link6)
        elif user == '7':
            webbrowser.open(link7)
        elif user == '8':
            webbrowser.open(link8)
        elif user == '9':
            webbrowser.open(link9)
        elif user == '10':
            webbrowser.open(link10)
        else:
            exit(0)

    except urllib2.HTTPError:
            print "- HTTP Error!"
    except urllib2.URLError:
            print "- Connection Faliure!"
    except UnboundLocalError:
            print "Episode does not exist!"

def Main():
    if len(sys.argv) !=3:
        print "1channel.py -show episode#"
        sys.exit()

    elif sys.argv[1] == '-breakingbad':
        tvshow = 'http://www.1channel.ch/tv-4128-Breaking-Bad/season-4-episode-' + sys.argv[2]

    elif sys.argv[1] == '-walkingdead':
        tvshow = 'http://www.1channel.ch/tv-2490619-The-Walking-Dead/season-3-episode-' + sys.argv[2]

    elif sys.argv[1] == '-poi':
        tvshow = 'http://www.1channel.ch/tv-2727923-Person-of-Interest/season-2-episode-' + sys.argv[2]

    else:
        sys.exit()

    Work(tvshow)

if __name__ == '__main__':
    Main()

Here's what I would start with:

import sys
import re
import urllib2
import webbrowser

from bs4 import BeautifulSoup

def get_episodes(url):
    print '[*] Working...'

    soup = BeautifulSoup(urllib2.urlopen(url))
    links = {}

    for table in soup.find_all('table', width='100%'):
        for a in table.find_all('a', target='_blank'):
            for li in table.find_all('li', class_='current-rating'):
                match = re.search(r'Version (\d+)', a.get_text())

                if match:
                    number = match.group(1)
                    links[number] = 'http://www.1channel.ch' + a['href']

                    print '({}) - {}\n'.format(number, li.get_text())

    return links

if __name__ == '__main__':
    if len(sys.argv) != 3:
        print '1channel.py -show episode#'
        sys.exit()
    elif sys.argv[1] == '-breakingbad':
        url = 'tv-4128-Breaking-Bad/season-4-episode-'
    elif sys.argv[1] == '-walkingdead':
        url = 'tv-2490619-The-Walking-Dead/season-3-episode-'
    elif sys.argv[1] == '-poi':
        url = 'tv-2727923-Person-of-Interest/season-2-episode-'
    else:
        sys.exit()

    try:
        episodes = get_episodes('http://www.1channel.ch/' + url + sys.argv[2])
    except urllib2.HTTPError:
        print '- HTTP Error!'
        sys.exit(0)
    except urllib2.URLError:
        print '- Connection Faliure!'
        sys.exit(0)

    number = int(raw_input('>>> '))

    if number in episodes:
        webbrowser.open(episodes[number])
    else:
        print 'Episode does not exist!'
        sys.exit(0)

Keep functions simple and (generally) single-purpose. Have them do one simple task.
Keep your code DRY. If you catch yourself writing the same code over and over, make a function for the general case and just fill in the variables.
BeautifulSoup4 uses underscores instead of camel case, so findAll becomes find_all.

I haven't tested this code either, but it should work.


Simple TCP client in Rust Here's a little command-line utility written in Rust that connects to a Denon AV receiver and sends a command, then receives the response. It's a simple TCP client that takes its command from the first argument.

extern crate encoding;

use std::env;
use std::io::prelude::*;
use std::net::TcpStream;
use encoding::{Encoding, EncoderTrap};
use encoding::all::ASCII;

const HOST: &'static str = "10.21.39.30:23";

fn main() {
    let command = match env::args().nth(1) {
        Some(cmd) => cmd,
        None => {
            let my_name = env::args().nth(0).unwrap();
            panic!("Usage: {} [command]", my_name)
        }
    };
    let mut command_bytes = ASCII.encode(&command, EncoderTrap::Strict).unwrap();
    command_bytes.push('\r' as u8);
    let mut stream = TcpStream::connect(HOST).unwrap();
    stream.write_all(&command_bytes).unwrap();
    let mut response = String::new();
    stream.read_to_string(&mut response).unwrap();
    println!("{}", response);
}

Is there a better way to handle this stuff? My questions are mostly:

Is there an alternative to all of these unwrap() calls?
It seems better (to me) to add the '\r' to the end of the command, making command_bytes mutable, than to call stream.write() another time. Think so?
The response will be ASCII, so stream.read_to_string() seems appropriate. It also seems like a bad idea for a network client to read with no size limits. Any good way to deal with this?
Any other structural stuff here that is glaringly bad? I'm starting to understand coding in Rust, I think, but haven't yet developed my own style. Early corrections would be greatly appreciated.

Your code looks pretty great to me, especially for the current size of it. As it grows to do more things, you will probably want to start restructuring things a bit, so I'm looking at from that direction. It also depends a bit on your audience — if this is just a tool for you, then unwrapping is just fine as you will understand what the error message means and how to fix it.

  Is there an alternative to all of these unwrap() calls?

Absolutely. Check out The Rust Programming Language's section on error handling. In this case, it's probably simplest to use a Box<Error>, but in a larger program or a library you may want to investigate implementing your own error type. I almost put that version here, but it was so long that it would have been really inappropriate given the current size :-)

  It seems better (to me) to add the '\r' to the end of the command, making command_bytes mutable, than to call stream.write() another time. Think so?

I don't think there's going to be a major performance difference between the two, or at least it could be made negligible. When you call write_all, it may actually make multiple write calls. In a way, you are  implementing a simple BufWriter. I think it mostly comes down to semantics. In this case, I do like that you add the \r to the command packet, as it allows that logic to all be in one tidy spot.

  It also seems like a bad idea for a network client to read with no size limits. Any good way to deal with this?

You can use take, which will limit the number of bytes read. The amount you use is the tricky part. Are you guaranteed to always get responses less than some size? Maybe the protocol says that you will always get 4 bytes that then tell you how many bytes follow? In the example below, I just assumed a maximum of 1 KiB.

Following the error handling guides, I extracted the majority of your logic to a new function that uses Result. That way, main simply has to decide what to do on success or failure. This also means that you get nicer error messages like this:

  An error occurred: Unable to transmit: Network is unreachable (os error 51)

I also added a little bit of space between logically distinct parts of the method. They are a little small to be broken out for now, but they could easily lead to new methods in the future.

Some thoughts for the future:

It might be a good idea to parameterize the host value to the send_one_command method. You can then use the constant from within main.
Will you ever need to have multiple back-and-forth transmissions? If so, you may want to create a struct that holds the connection and keeps it open. Then you ask it to send a command.
The remaining unwrap in main is tricky. It seems very unlikely that the program name could be missing, so unwrap seems to be a good fit as it's a case you simply can't handle well. On the other hand, errors hiding other errors can be annoying to debug. You may want to use something like .unwrap_or_else(|| "unknown program name".into()).

extern crate encoding;

use std::{env, error};
use std::io::prelude::*;
use std::net::TcpStream;
use encoding::{Encoding, EncoderTrap};
use encoding::all::ASCII;

const HOST: &'static str = "10.21.39.30:23";

fn send_one_command(command: &str) -> Result<String, Box<error::Error + Send + Sync>> {
    let mut command_bytes = try!(ASCII.encode(command, EncoderTrap::Strict).map_err(|x| x.into_owned()));
    command_bytes.push('\r' as u8);

    let mut stream = try!(TcpStream::connect(HOST));
    try!(stream.write_all(&command_bytes));

    let mut response = String::new();
    let mut limited = stream.take(1024);
    try!(limited.read_to_string(&mut response));

    Ok(response)
}

fn main() {
    let command = match env::args().nth(1) {
        Some(cmd) => cmd,
        None => {
            let my_name = env::args().nth(0).unwrap();
            panic!("Usage: {} [command]", my_name)
        }
    };

    match send_one_command(&command) {
        Ok(response) => println!("{}", response),
        Err(err) => println!("An error occurred: {}", err),
    }
}


Scoreboard with score evolution plots using classes I've very recently started using OOP paradigms for the code in my work (academic research). As an exercise I tried coding this quick scoreboard.

I'm not interested in improving the 'display' part, I know there would be a lot to do there, but does my use of classes make any sense?

import os
from numpy import nan, nansum
from matplotlib import interactive
import matplotlib.pyplot as plt

class Player:
    def __init__(self, name):
        self.name = name
        self.score = []
        self.cumulative_score = []
        self.total_score = 0

    def add_score(self, score):
        self.score[-1] = score
        self.cumulative_score[-1] = self.total_score + score
        self.total_score = nansum(self.score)

class ScoreBoard:
    def __init__(self):
        plt.style.use('ggplot')
        interactive(True)
        player_name = True
        self.players = []
        while player_name:
            player_name = input('Player name ? ')
            if player_name:
                self.players += [Player(player_name)]
        self.round = 0
        self.figure = plt.figure()
        self.ax = self.figure.add_subplot(111)
        self.plots = {}
        for player in self.players:
            self.plots[player.name], = plt.plot(range(1, self.round + 1),
                                                player.cumulative_score,
                                                label=player.name, marker='*')
        plt.legend()

    def new_round(self):
        self.round += 1
        for player in self.players:
            player.score += [nan]
            player.cumulative_score += [nan]
        for i, player in enumerate(self.players):
            self.display()
            new_score = input('Score pour {}: '.format(player.name))
            if new_score:
                player.add_score(int(new_score))

    def display(self):
        self.ax.clear()
        for player in self.players:
            self.plots[player.name], = plt.plot(range(1, self.round + 1),
                                                player.cumulative_score,
                                                label=player.name, marker='*')
        plt.legend()
        os.system('cls' if os.name == 'nt' else 'clear')
        print(' \t| '.join(player.name for player in self.players))
        print(''.join(['-'] * 80))
        for round in range(self.round):
            print(' \t| '.join(str(player.score[round]) for player in
                               self.players))
        print(''.join(['-'] * 80))
        print(' \t| '.join(str(player.total_score)
                           for player in self.players))

score_board = ScoreBoard()

while True:
    score_board.new_round()

I think your use of classes mostly makes sense.

The key to object oriented programming, in my humble opinion, is relating how we think about objects in the real world to the way our code is structured. If you were playing a game, would you, as the player, add a score to the score board? Well, it might depend on the game. Sometimes, the game might determine your score and add it to the scoreboard (I'm thinking battleship). In that game, the player doesn't add their score to the scoreboard, it is behind the scenes in a way.

I like that you created a class player, most games revolve around players and encapsulating that information in a class makes sense.

When you play a game, does the scoreboard determine when a new round occurs? Not really, that is sort of inherent to the game. The scoreboard, which in the real world encapsulates information needed just for displaying information related to a game, doesn't actually make decisions about a game. It merely receives information and then displays it. Therefore, it might make sense to separate your scoreboard's display functionality from the functionality of actually playing the game.

This could happen by creating another class called game that would have as members of the class player's and a scoreboard. The game class could be responsible for starting a new round and be responsible for making sure the scoreboard is updated and the player's own score is updated.

Often in OOP, methods are like actions and take on verb-like names. I like that you called your method add_score inside player.

I hope this is constructive feedback and you find it helpful. If you have any questions, let me know.


Limited checkbox selection I need to select all checked checkboxes, except checkboxes having a class .checkall.

HTML:

<input type="checkbox" value="1961" class="checkbox">
<input type="checkbox" name="checkall" class="checkbox checkall">

Script:

$("input.checkbox:checkbox:checked:not(.checkall)").each(function () {
    var checkbox = $(this), machineId = checkbox.val()
}

I just posted on your other post about this, use:

$("input[type=checkbox]:checked:not(.checkall)")

This is the 'regular' way to check for input tags of a particular type, it's much clearer than your way. I'm not sure what the significance of the .checkbox class is but it seems like it's not necessary to my.

I also recommend you cache your check boxes in a variable and search that multiple times. That way you're only searching the check boxes each time instead of the whole DOM.

// in document ready
var checkboxes = $('input[type=checkbox]');

// When you need to find the checked check boxes
var checked = checkboxes.find(':checked:not(.checkall)');


Accepting a positive integer from either a command-line argument or System.in I've been doing a lot of programs for classes that require a prompt from the program. I thought it would be much faster for me to rapidly test inputs if I didn't have to execute the program and then type the input once inside the program (as opposed to just hitting ↑ and adjusting the input that way).

My concern is this code seems to have some duplication with the exception handling, and I'm also not sure if it's as efficient as it can be. I'm somewhat of a beginner, but I'm certainly interested in more advanced ideas/solutions as that would point in direction of where to self-study more.

import java.util.Scanner;

/*
 *  This program is to implement testing command line input or reverting to regular prompts if
 *  an argument isn't provided or is invalid. It tests to see if the input is a negative integer,
 *  and displays the result.
 *
 *  Algorithm:
 *  If the console argument is valid, treat it as user input, or otherwise ignore it
 *  Continue to ask for a positive integer until one is given.
 *  Display the integer given and confirm it's negative
 */
public class TestConsoleArgs
{
    public static void main( String[] args )
    {
        String arg;
        Scanner user_input = new Scanner(System.in);
        int num = 0;

        try {
            arg = args[0];
            num = Integer.parseInt(arg);
        } catch (ArrayIndexOutOfBoundsException e) {
            arg = "";
        } catch (NumberFormatException e2) {
            arg = "";
        }

        while (num <= 0)
        {
            System.out.print("Please type in a positive integer.  ");
            arg = user_input.next();

            try {
                num = Integer.parseInt(arg);
            } catch (NumberFormatException e) {
                System.out.print("That's not an integer! ");
                continue;
            }

            if (num <= 0)
            {
                System.out.print("That's a negative number. ");
            }
        }

        System.out.println("Yes, '" + num + "' is a positive integer.");
    }
}

user_input can be better renamed as userInput.

If you want to make the prompting 'self-contained', put it inside a method that will re-prompt the user for non-integer values:

private static int getIntegerInput(Scanner scanner) { ... }

I also don't think you need to catch ArrayIndexOutOfBoundsException, instead consider picking up from the command line only if args.length is at least 1:

// calls getIntegerInput(Scanner) with scanner if input is not an integer
private static int parseStringOrPrompt(String input, Scanner scanner) { ... }

Putting it altogether:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        int num = args.length > 0 ? parseStringOrPrompt(args[0], scanner)
                                    : getIntegerInput(scanner);
        while (num < 0) {
            System.out.print("That's a negative integer.");
            num = getIntegerInput(scanner);
        }
        System.out.println("Yes, '" + num + "' is a positive integer.");
    }
}


Palidrome checker in haskell I decided to avoid the trivial isPalindrome lst = lst == reverse lst and tried writing a method with pattern matching and recursion.

allButFirstAndLast :: [a] -> [a]
allButFirstAndLast = tail . init

isPalidrome :: [a] -> Bool
isPalidrome [] = True
isPalindrome [a] = True
isPalindrome [a,b] = a == b
isPalindrome lst = (isPalindrome (allButFirstAndLast lst)) &&
                   ((head lst) == (last lst))

The implementation is correct, but not efficient. The problem are the calls to last and init, which are both O(n), which makes the whole function O(n^2). On the other hand, lst == reverse lst is just O(n).

See Finding palindromes in a linked list.

Also the pattern

isPalindrome [a,b] = a == b

is redundant, you can omit it and let it be handled by the generic case.


Adding reversed numbers This is my solution to the competitive coding question on SPOJ here.  It gives a runtime of 0.24 but the best solutions posted have a 0.0 runtime. How could I possibly achieve better results than this?

#include<iostream>
#include<cstdio>

using namespace std;

int rev(int num)
{
    int rev_num=0;
    while(num)
    {
        rev_num= rev_num*10 + num%10;
        num/=10;
    }
    return rev_num;
}
int main()
{
    int n, n1, n2;
    scanf("%d",&n);
    while(n--)
    {
        cin>>n1>>n2;
        cout<<rev(rev(n1)+rev(n2))<<endl;

    }
    return 0;
}

A faster method would avoid the numeric divisions. Instead, it would load the number in a string (through the sprintf function), reverse the string, and finally recast the string into a number (with the help of the sscanf function).

I'm attaching a function named fastrev which in turn uses an auxilliary function reverse_string. Hopefully, this would improve runtime for large numbers.

char* reverse_string(char *s1, char* s2)
{
    char *p, *q;
    int len = strlen(s1);

    for (p = s1; *p; p++) {
        s2[len-1] = *p;
        len--;
    }
    return s2;
}

int fastrev(int num) {
    static char nbuff[32];
    static char rnbuff[32];
    int rnum;

    sprintf(nbuff, "%d", num);
    reverse_string(nbuff, rnbuff);
    sscanf(rnbuff, "%d", &rnum);
    return rnum;
}


matplotlib fixing axis scale and alignment There is few points In the script I don't like I think it can be improved, but I need a second opinion, here is the interactive plot I'm trying to build.

here is the code with thought in comments.

#!/usr/bin/python3

from functools import lru_cache

import numpy as np
import scipy.stats as ss
import matplotlib.pyplot as plt
import matplotlib.widgets as widgets

@lru_cache # beacuse it redraws each time
def get_pdf(mu, sigma=1, offset=4):
    o = sigma * offset
    x = np.linspace(mu - o, mu + o, 100)
    rv = ss.norm(mu, sigma)
    return x, rv.pdf(x)

fig, ax = plt.subplots()
# added the subplot for bottom margin and the slider, since its also the ax
plt.subplots_adjust(bottom=0.25)

ax.fill_between(*get_pdf(0), label='A', alpha=0.7)
ax.set_xlim(-10, 10)
ax.set_ylim(0, 1)

# my guess is this has to be integrated with the update rather than outside
# and using t variable as global which I don't like, and guess it can be improved
t = ax.fill_between(*get_pdf(2), label='B', color='crimson', alpha=0.7)

slider = widgets.Slider(
    # ax position are absolute, should be easy without subplot may be
    ax      = plt.axes([0.25, 0.1, 0.5, 0.03]),
    label   = "shift",
    valmin  = -5,
    valmax  = 5,
    valinit = 2,
    valstep = 0.5
)

def update(val):
    x, y = get_pdf(val)
    global t # really hate to do this
    t.remove()
    t = ax.fill_between(*get_pdf(val), color='crimson', alpha=0.7)
    fig.canvas.draw_idle()

slider.on_changed(update)
ax.legend()
plt.show()

If you want to get rid of the global t you could consider creating a class to manage the data and updating. I quickly whipped something together, although I'm not well versed enough in stats to have a good idea about what names would be more appropriate.

from functools import lru_cache

import numpy as np
import scipy.stats as ss
import matplotlib.pyplot as plt
import matplotlib.widgets as widgets

#probability density function generator: chached
@lru_cache # beacuse it redraws each time
def get_pdf(mu, sigma=1, offset=4):
    o = sigma * offset
    x = np.linspace(mu - o, mu + o, 100)
    rv = ss.norm(mu, sigma)
    return x, rv.pdf(x)

# This simple class will hold the reference to `t` so that it doesn't need to
# be a global
class Data():
    '''
    A simple class that plots data on an axis and contains a method for updating
    the plot.
    '''

    def __init__(self, ax, **properties):
        self.properties = properties
        self.t = ax.fill_between(*get_pdf(2), **properties)

    def update(self, val):
        x, y = get_pdf(val)
        self.t.remove()
        self.t = ax.fill_between(x, y, **self.properties)
        fig.canvas.draw_idle()

# Generate the figure and axis for our widget

fig, ax = plt.subplots()
# added the subplot for bottom margin and the slider, since its also the ax
plt.subplots_adjust(bottom=0.25)

# add a slider widget
slider = widgets.Slider(
    # ax position are absolute, should be easy without subplot may be
    ax      = plt.axes([0.25, 0.1, 0.5, 0.03]),
    label   = "shift",
    valmin  = -5,
    valmax  = 5,
    valinit = 2,
    valstep = 0.5
)

# add a reference distribution *A*
ax.fill_between(*get_pdf(0), label='A', alpha=0.7)
ax.set_xlim(-10, 10)
ax.set_ylim(0, 1)

# Create a data instance
properties = dict(label='B', color='crimson', alpha=0.7)
data = Data(ax=ax, **properties)

# link data update method to slider.on_changed
slider.on_changed(data.update)
ax.legend()
plt.show()

```


Translator for the English language I made a translator for the English language. As of now it only translates from Spanish, Russian, and Arabic (will be adding more languages).

The program pulls the input from a yaml file with a list of words and phrases inside of it. It searches the users input and if it finds a match it returns with the English translation of the word or phrase!

require 'yaml'

def welcome
    puts "Welcome to the Translation Center! Please enter the language you would like to translate to English or you can choose from the list below:
    1. Spanish
    2. Russian
    3. Arabic"
    input = language = gets.chomp!
    case language
    when /spanish/i
        spanish_translate
    when "1"
        spanish_translate
    when /russian/i
        russian_translate
    when "2"
        russian_translate
    when /arabic/i
        arabic_translate
    when "3"
        arabic_translate
    else
        puts "Language is not yet supported within Translation Center."
        exit
    end
end

def spanish_translate
    translations = YAML.load_file 'spanish.yml'
    puts "Enter word or phrase to be translated to English, press 'Q' to quit:"
    input = gets.chomp
    case
    when translations[input]
        puts "The translation from Spanish to English is: #{translations[input]}"
    when input =~ /q/i
        exit
    else
        puts "Invalid word or phrase redirecting..."
        spanish_translate
    end
end

def russian_translate
    translation = YAML.load_file 'russian.yml'
    puts "Enter word or phrase you would like translated to English, press 'Q' to quit:"
    input = gets.chomp!
    case
    when translations[input]
        puts "The translation from Russian to English is: #{translations[input]}"
    when input =~ /q/i
        exit
    else
        puts "Invalid word or phrase redirecting..."
        russian_translate
    end
end

def arabic_translate
    translation = YAML.load_file 'arabic.yml'
    puts "Enter word or phrase you woukd like translated to English, press 'Q' to quit:"
        input = gets.chomp!
        input = gets.chomp!
    case
    when translations[input]
        puts "The translation from Arabic to English is: #{translations[input]}"
    when input =~ /q/i
        exit
    else
        puts "Invalid word or phrase redirecting..."
        arabic_translate
    end
end
welcome

Here's the Spanish yaml file:

a: to
abajo: down
el abancio: fan
buenos dias: good morning
buenos tardes: good afternoon
como se llama usted: what is your name
como esta usted: how are you
estoy bien: i am fine
mucho gusto: nice to meet you
perdoname: excuse me
por favor: please
gracias: thank you
lo siento: i'm sorry
salud: bless you
de nada: you are welcome
hay muchos: there are many
si: 'yes'
no: 'no'
yo no comprendo: i do not understand
yo quiero una tortilla: i want a tortilla

Russian yaml file:

и: and, though
в: in, at
не: not
он: he
на: on, it, at, to
я: I
что: what, that, why
тот: that
быть: to be
с: with and, from, of
а: while, and, but
весь: all, everything
это: that, this, it
как: how, what, as, like

Arabic yaml file:

salam: hi
سلام: hi
Sabah el kheer: good morning
صباح الخير: good morning
massa el kheer: good evening
مساء الخير: good evening
marhaban: welcome
مرحبا: welcome
Kaifa haloka: how are you(male)
kaifa haloki: how are you(female)
كيف حالك؟: how are you
ana bekhair shokran: i'm fine thanks
أنا بخير شكر: im fine thanks
wa ant: and you(male)
wa anti: and you (female)
و أنت؟: and you
jayed: good
'aadee: so-so
عادي: good
جيد: so-so

It now has the ability to translate to three different languages.

How's this looking as an OOP language overall? What can I do differently next time?

How's this looking as an OOP language overall?

Ruby is actually multi-paradigm.
It allows you to use OO, and it gives you the option not to.
For example you didn't use any objects,
so you didn't actually employ OOP.

For improvements:

Your {lang}_translate code is very repetitive,
you can change it to a class to reduce repetitiveness.
You load the YAML file on each word translated (or incorrectly inputted),
and so you should load it once.
You have to add multiple things when you add a new language.

Both the index and language in the case.
Both the index and language in the puts
The language file in the new {lang}_translate function.

You can remove this all if you use a list and a class.

First, to remove the first two points,
I would make a class that lazily loads the translations.
And implements the translation.
Also I personally think your case statement in {lang}_translate is better as if elsif else.
exit seems like a bad design here,
if you were to want to allow the user to change language by entering 'q',
you will have to remove the exit.

class Translate
    def initialize(file_name)
        @file_name = file_name
    end

    def set_translation
        @translations = YAML.load_file @file_name
    end

    def translate
        puts "Enter word or phrase to be translated to English, press 'Q' to quit:"
        input = gets.chomp
        if (@translations || set_translation)[input]?
            puts "The translation from Spanish to English is: #{@translations[input]}"
        elsif input =~ /q/i
            return
        else
            puts "Invalid word or phrase redirecting..."
            translate
        end
    end
end

spanish = Translate.new('spanish.yaml')
spanish.translate

This can then be expanded easily to allow any language,
by just passing a different file path.

As for welcome,
I would recommend using an Array so you have control over the order.
And so you can specify languages by only name and file.

languages = [
    ["Spanish", Translate.new('spanish.yaml')],
    ["Russian", Translate.new('russian.yaml')],
    ["Arabic", Translate.new('arabic.yaml')]
]

Being able to use this in the output is a simple each_with_index.

For the program to automatically call translate
you need to get the index of the language in the languages Array.
Where you get the index either via a regex or .index.
Finally you then check if it's valid input, and output correctly.

def welcome
    languages = [
        ["Spanish", Translate.new('spanish.yaml')],
        ["Russian", Translate.new('russian.yaml')],
        ["Arabic", Translate.new('arabic.yaml')]
    ]

    puts "Welcome to the Translation Center! Please enter the language you would like to translate to English or you can choose from the list below:"
    languages.each_with_index { |item, index| puts String(index + 1) + '. ' + item[0] }
    language = gets.chomp!.downcase

    index = if /^\d+$/.match(language) then
        Integer(language) - 1
    else
        languages.index { |x| x[0].downcase == language }
    end

    if index.nil? or (language = languages[index][1]).nil?
        puts "Language is not yet supported within Translation Center."
    else
        language.translate
    end
end

Finally, I think that you should allow users to input more than one word or phrase at a time. This would be adding a simple infinite loop to Translate.translate, and changing it to not be recursive.


Count the number of inversions using Binary Indexed Tree in an array Original Problem: HackerRank

I am trying to count the number of inversions using Binary Indexed Tree in an array. I've tried to optimise my code as much as I can. However, I still got TLE for the last 3 test cases. Any ideas that I can further optimise my code?

# Enter your code here. Read input from STDIN. Print output to STDOUT
#!/usr/bin/python

from copy import deepcopy

class BinaryIndexedTree(object):
    """Binary Indexed Tree
    """
    def __init__(self, n):
        self.n = n + 1
        self.bit = [0] * self.n

    def update(self, i, k):
        """Adds k to element with index i
        """
        while  i <= self.n - 1:
            self.bit[i] += k
            i = i + (i & -i)

    def count(self, i):
        """Returns the sum from index 1 to i
        """
        total = 0
        while i > 0:
            total += self.bit[i]
            i = i - (i & -i)
        return total

def binary_search(arr, target):
    """Binary Search
    """
    left, right = 0, len(arr) - 1
    while left <= right:
        mid = left + ((right - left) >> 1)
        if target == arr[mid]:
            return mid
        elif target < arr[mid]:
            right = mid - 1
        else:
            left = mid + 1

    # If not found, return -1.
    return -1

T = input()
for iterate in xrange(T):
    n = input()
    q = [ int( i ) for i in raw_input().strip().split() ]
    answer = 0
    # Write code to compute answer using x, a and answer

    # Build a Binary Indexed Tree.
    bit = BinaryIndexedTree(n)

    # Copy q and sort it.
    arr = sorted(deepcopy(q))

    # index array.
    index = map(lambda t: binary_search(arr, t) + 1, q)

    # Loop.
    for i in xrange(n - 1, -1, -1):
        answer += bit.count(index[i])
        bit.update(index[i] + 1, 1)
    print answer

def __init__(self, n):
        self.n = n + 1
        self.bit = [0] * self.n

bit has a well-known interpretation in programming which distracts from the intended interpretation here. IMO even something as generic as data would be better.

    def update(self, i, k):
        """Adds k to element with index i
        """
        while  i <= self.n - 1:
            self.bit[i] += k
            i = i + (i & -i)

The loop condition would be more familiar as while i < self.n.

I don't see any reason to avoid +=.

A reference to explain the data structure would be helpful, because otherwise this is quite mysterious.

        mid = left + ((right - left) >> 1)

The reason for this particular formulation of the midpoint is to avoid overflow in languages with fixed-width integer types. Since that's not an issue in Python, I'd favour the more straightforward mid = (left + right) >> 1. It might even be a tiny bit faster.

    q = [ int( i ) for i in raw_input().strip().split() ]

    index = map(lambda t: binary_search(arr, t) + 1, q)

Why the inconsistency? I think it would be more Pythonic to use comprehensions for both.

    # Copy q and sort it.
    arr = sorted(deepcopy(q))

    # index array.
    index = map(lambda t: binary_search(arr, t) + 1, q)

This seems like a rather heavyweight approach. Why not just

    index = sorted((val, idx) for idx, val in enumerate(q))

?

Although asymptotically I don't see any reason why this would be \$\omega(n \lg n)\$, it has a few \$\Theta(n \lg n)\$ stages. The standard algorithm for this problem, which is essentially merge sort, has the same asymptotic complexity but probably hides a much smaller constant behind the \$\Theta\$ / \$O\$.


A1Notation conversion to row & column index This set of Google Apps Script helper functions are published in this gist, along with a bunch of other utilities. Background info is in this SO question. Since no Google services are used, this is pure JavaScript.

What I hope to get from a code review:

General feedback on clarity & maintainability.
Is the regex in cellA1ToIndex() too big a drag on performance? What alternatives would be as-or-more functional, with better performance?

cellA1ToIndex( string cellA1, number index )

Convert a cell reference from A1Notation to 0-based indices (for arrays) or 1-based indices (for Spreadsheet Service methods).

colA1ToIndex( colA1, index )

Return a 0-based array index corresponding to a spreadsheet column label, as in A1 notation.

rowA1ToIndex( string rowA1, number index )

Return a 0-based array index corresponding to a spreadsheet row number, as in A1 notation. Almost pointless, really, but maintains symmetry with colA1ToIndex().

/**
 * Convert a cell reference from A1Notation to 0-based indices (for arrays)
 * or 1-based indices (for Spreadsheet Service methods).
 *
 * @param {String}    cellA1   Cell reference to be converted.
 * @param {Number}    index    (optional, default 0) Indicate 0 or 1 indexing
 *
 * @return {object}            {row,col}, both 0-based array indices.
 *
 * @throws                     Error if invalid parameter
 */
function cellA1ToIndex( cellA1, index ) {
  // Ensure index is (default) 0 or 1, no other values accepted.
  index = index || 0;
  index = (index == 0) ? 0 : 1;

  // Use regex match to find column & row references.
  // Must start with letters, end with numbers.
  // This regex still allows induhviduals to provide illegal strings like "AB.#%123"
  var match = cellA1.match(/(^[A-Z]+)|([0-9]+$)/gm);

  if (match.length != 2) throw new Error( "Invalid cell reference" );

  var colA1 = match[0];
  var rowA1 = match[1];

  return { row: rowA1ToIndex( rowA1, index ),
           col: colA1ToIndex( colA1, index ) };
}

/**
 * Return a 0-based array index corresponding to a spreadsheet column
 * label, as in A1 notation.
 *
 * @param {String}    colA1    Column label to be converted.
 *
 * @return {Number}            0-based array index.
 * @param {Number}    index    (optional, default 0) Indicate 0 or 1 indexing
 *
 * @throws                     Error if invalid parameter
 */
function colA1ToIndex( colA1, index ) {
  if (typeof colA1 !== 'string' || colA1.length > 2)
    throw new Error( "Expected column label." );

  // Ensure index is (default) 0 or 1, no other values accepted.
  index = index || 0;
  index = (index == 0) ? 0 : 1;

  var A = "A".charCodeAt(0);

  var number = colA1.charCodeAt(colA1.length-1) - A;
  if (colA1.length == 2) {
    number += 26 * (colA1.charCodeAt(0) - A + 1);
  }
  return number + index;
}

/**
 * Return a 0-based array index corresponding to a spreadsheet row
 * number, as in A1 notation. Almost pointless, really, but maintains
 * symmetry with colA1ToIndex().
 *
 * @param {Number}    rowA1    Row number to be converted.
 * @param {Number}    index    (optional, default 0) Indicate 0 or 1 indexing
 *
 * @return {Number}            0-based array index.
 */
function rowA1ToIndex( rowA1, index ) {
  // Ensure index is (default) 0 or 1, no other values accepted.
  index = index || 0;
  index = (index == 0) ? 0 : 1;

  return rowA1 - 1 + index;
}

I see you fixed the index-checking/defaulting bug that gengkev pointed out in the comments. As SpiderPig mentioned the cleanest solution is probably index = index ? 1 : 0.

However, I don't know if I'd bother with the index parameter at all. So far, it's led to bugs, duplication across all three functions, and all it really does is add/subtract 1. That can be done anywhere.

So I'd recommend just translating to 1-based indices, and if you need zero-based indices, well, subtract 1.

I wouldn't worry about using a regex - but I'd probably make it stricter. I imagine that a cell reference isn't that flexible. I imagine the string may contain "$"s but that's about it. Correct me if I'm wrong.

However, cellA1ToIndex() also does too much. It checks things that the other two functions don't. So if you call the other two functions directly, your input isn't checked as strictly. For instance, colA1ToIndex() will accept a string like "%#", and return something wrong. Similarly, rowA1ToIndex() will gladly accept just about anything, and return NaN.

So I'd keep cellA1ToIndex as "dumb" as possible, and make sure the specialized functions are more robust.

Lastly, why a of maximum 2 letters for columns? Why not 3? Or 4?

I'd do something like this:

function cellA1ToIndex(cellA1) {
  var match = cellA1.match(/^\$?([A-Z]+)\$?(\d+)$/);

  if(!match) {
    throw new Error( "Invalid cell reference" );
  }

  return {
    row: rowA1ToIndex(match[2]),
    col: colA1ToIndex(match[1])
  };
}

function colA1ToIndex(colA1) {
  var i, l, chr,
      sum = 0,
      A = "A".charCodeAt(0),
      radix = "Z".charCodeAt(0) - A + 1;

  if(typeof colA1 !== 'string' || !/^[A-Z]+$/.test(colA1)) {
    throw new Error("Expected column label");
  }

  for(i = 0, l = colA1.length ; i < l ; i++) {
    chr = colA1.charCodeAt(i);
    sum = sum * radix + chr - A + 1
  }

  return sum;
}

function rowA1ToIndex(rowA1) {
  var index = parseInt(rowA1, 10)
  if(isNaN(index)) {
    throw new Error("Expected row number");
  }
  return index;
}


Refactor C# OR statement using Linq How can I write this OR statement better, allowing for growth in number of values?

var isNewFile = (progressID == 10 || progressID == 20 || progressID == 21);

I was thinking of this:

var isNewFile = progressID.Contains (10,20,21)

OR using Linq:

.Select or .Contains

Any thoughts?

For something this simple, I think using the || is fine, however, if you need to use the list of acceptable integers more generally or avoid hard-coding the set in a single expression1, you can use something like this:

var newFileProgressIds = new[] { 10, 20, 21 };

var isNewFile = newFileProgressIds.Contains(progressID);

Or possibly use a HashSet<T>:

var newFileProgressIds = new HashSet<int> { 10, 20, 21 };

var isNewFile = newFileProgressIds.Contains(progressID);

1: Of course, in these examples, the values are still hard coded in one place, but precisely how you generate the list is unimportant.


Implement total savings function on a record type the TotalSavings() function on the record below has a code smell that I can't exactly detail. Is there a more readable alternative for implementing this function on a record type?

type ProvidersCharged = ProvidersCharged of decimal
type InsuranceSavings = InsuranceSavings of decimal

type ClaimsSummary =
    { Claims:Claim seq
      ProvidersCharged:ProvidersCharged
      InsuranceSavings:InsuranceSavings
    } member this.TotalSavings() = match this.ProvidersCharged with
                                   | ProvidersCharged charged ->
                                        match this.InsuranceSavings with
                                        | InsuranceSavings savings -> charged - savings

Yes, there are, indeed, a couple of code smells.

The first one is that TotalSavings is a type member, and not a function ;)

The other code smell is that you have the beginning of arrow code in your implementation.

I'd write it like this instead:

let totalSavings summary =
    let (ProvidersCharged charged) = summary.ProvidersCharged
    let (InsuranceSavings savings) = summary.InsuranceSavings
    charged - savings

Notice that you don't have to use the match keyword when doing pattern match. With single-case discriminated unions, you can also pattern match directly into let-bound values.

Normally, the brackets aren't necessary, but in this case they are, because e.g. ProvidersCharged is both a case constructor and a record label. If the case constructor had been unambiguous, you could have omitted the brackets, like this:

let Foo foo = bar

You can also pattern match directly in function arguments, but in this case, it becomes too verbose:

let totalSavings' { ProvidersCharged = ProvidersCharged charged; InsuranceSavings = InsuranceSavings savings; Claims = _ } =
    charged - savings

Notice how the record labels are automatically destructured here, using nested pattern matching: matching is done first on the record labels, then on the case constructors inside of the labels.

You'll also notice that I used a wildcard match on Claims because I don't care about that value.


Greedy algorithms to find minimum number of coins (CS50) I'm taking CS50, an Introduction to CS, and we're asked to do the following task:

Suppose that a cashier owes a customer some change and in that cashier’s drawer are quarters (25¢), dimes (10¢), nickels (5¢), and pennies (1¢). The problem to be solved is to decide which coins and how many of each to hand to the customer. Think of a “greedy” cashier as one who wants to take the biggest bite out of this problem as possible with each coin they take out of the drawer. For instance, if some customer is owed 41¢, the biggest first (i.e., best immediate, or local) bite that can be taken is 25¢. (That bite is “best” inasmuch as it gets us closer to 0¢ faster than any other coin would.) Note that a bite of this size would whittle what was a 41¢ problem down to a 16¢ problem, since 41 - 25 = 16. That is, the remainder is a similar but smaller problem. Needless to say, another 25¢ bite would be too big (assuming the cashier prefers not to lose money), and so our greedy cashier would move on to a bite of size 10¢, leaving him or her with a 6¢ problem. At that point, greed calls for one 5¢ bite followed by one 1¢ bite, at which point the problem is solved. The customer receives one quarter, one dime, one nickel, and one penny: four coins in total

Here is my solution to the problem:
#include <stdio.h>
#include <cs50.h>
#include <math.h>

float cents, money;
int ask_money(void);
int dollars_to_cents(int dollars);
int get_max_coins(int _cents, int coin_type);
int min_coins_exchanged(int _money, int _coin_types[], int _arr_length);
int quarter = 25, dimes = 10, nickles = 5, pennies = 1;

int main(void)
{
    money = dollars_to_cents(ask_money());
    int arr_length = 4;
    int coin_types[] = {quarter, dimes, nickles, pennies};
    printf("%i\n", min_coins_exchanged(money, coin_types, arr_length));
}

// Given some amount of money, this returns the maximum amount of coins of a certain type possible
// such that the remaining money is less than the value of the coin type, then it subtracts our money by that amount
int get_max_coins(int _cents, int coin_type)
{
    int max_coins = floor((float)((_cents) / coin_type));
    money -= max_coins * coin_type;
    return max_coins;
}

int ask_money(void)
{
    do
    {
        money = get_float("Change owed: ");
    }
    while (money < 0);
    return money;
}

// Necessary to avoid problems with floating point imprecision
int dollars_to_cents(int dollars)
{
    return round(money * 100);
}

// Calculates the minimum amount of coins exchanged such that when these coins are added up they are equal to the change owed
int min_coins_exchanged(int _money, int _coin_types[], int _arr_length)
{
    int _min_coins_exchanged = 0;
    for(int i = 0; i < _arr_length; i++)
    {
        _money = money;
        _min_coins_exchanged += get_max_coins(_money, _coin_types[i]);
    }
    return _min_coins_exchanged;
}

I'm looking for feedback on my code. How do I make my code better and cleaner? I.e in terms of readability, performance, comments, design principles, etc. There's a lot of ways to approach this problem, and indeed when trying to solve it I've tried many different ways but this is my current favorite. What do you think about my solution? Perhaps there's a better way to approach the problem?

This code needs compiling with more warnings enabled:
gcc -std=c17 -fPIC -g -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds  -Wstrict-prototypes    -Wconversion    254841.c    -o 254841
254841.c: In function ‘main’:
254841.c:14:13: warning: conversion from ‘int’ to ‘float’ may change value [-Wconversion]
     money = dollars_to_cents(ask_money());
             ^~~~~~~~~~~~~~~~
254841.c:17:40: warning: conversion from ‘float’ to ‘int’ may change value [-Wfloat-conversion]
     printf("%i\n", min_coins_exchanged(money, coin_types, arr_length));
                                        ^~~~~
254841.c: In function ‘get_max_coins’:
254841.c:24:21: warning: conversion from ‘double’ to ‘int’ may change value [-Wfloat-conversion]
     int max_coins = floor((float)((_cents) / coin_type));
                     ^~~~~
254841.c:25:11: warning: conversion from ‘int’ to ‘float’ may change value [-Wconversion]
     money -= max_coins * coin_type;
           ^~
254841.c: In function ‘ask_money’:
254841.c:36:12: warning: conversion from ‘float’ to ‘int’ may change value [-Wfloat-conversion]
     return money;
            ^~~~~
254841.c: In function ‘dollars_to_cents’:
254841.c:42:12: warning: conversion from ‘double’ to ‘int’ may change value [-Wfloat-conversion]
     return round(money * 100);
            ^~~~~~~~~~~~~~~~~~
254841.c:40:26: warning: unused parameter ‘dollars’ [-Wunused-parameter]
 int dollars_to_cents(int dollars)
                      ~~~~^~~~~~~
254841.c: In function ‘min_coins_exchanged’:
254841.c:51:18: warning: conversion from ‘float’ to ‘int’ may change value [-Wfloat-conversion]
         _money = money;
                  ^~~~~

You have avoided some of the common traps, such as embedding the set of coins into the logic, although I wouldn't give those locale-specific names - here in Britain, we have 1, 2, 5, 10, 20, 50, 100 and 200 but they don't have nicknames.
I would instead make the set of coins be a global constant, without naming them individually:
const unsigned int coin_values = {
    25,  /* quarter */
    10,  /* dime    */
    5,   /* nickel  */
    1    /* penny   */
};
const size_t coin_values_len = sizeof coin_values / sizeof *coin_values;

Avoid using floating-point types for money.  The problem is that .01 is not an exact fraction in binary, so the code is prone to error due to rounding.
Instead, do something like
unsigned int dollars, cents = 0;
if (scanf("%u.%u") < 1) {
    fputs("Currency parse failure\n", stderr);
    return EXIT_FAILURE;
}
/* if we got here, we read the dollars, and perhaps the cents */
unsigned int total_cents = 100 * dollars + cents;  /* TODO: deal with overflow */

You're using some names that you shouldn't:

int get_max_coins(int _cents, int coin_type)

All identifiers beginning with _ followed by a letter are reserved for use by the implementation.  That means it's a bad idea to use these names in your own code.  There's no reason not to just call the argument cents without that prefix, so that's what I'd recommend.
There's some pointless conversion here:

int max_coins = floor((float)((_cents) / coin_type));

cents / coin_type is of type int.  We convert that to float (losing precision), truncate the fractional part (which will be zero, since we started with an integer) and then implicitly convert to int.  That code is a very inefficient and problematic way to write

int max_coins = _cents / coin_type;

There's some serious problems with global variables, and there seems to be confusion passing values by parameter or via globals.  Look at this:
float money;

int main(void)
{
    money = dollars_to_cents(ask_money());
}

int dollars_to_cents(int dollars)
{
    return round(money * 100);
}

int ask_money(void)
{
    do
    {
        money = get_float("Change owed: ");
    }
    while (money < 0);
    return money;
}

In main(), we assign to money, which we're also using in ask_money.  And dollars_to_cents() completely ignores its argument and shares the global variable.  There's no need for this variable to be shared - each function can have its own local variable:
int main(void)
{
    int money = dollars_to_cents(ask_money());
}

int dollars_to_cents(float dollars)
{
    /* don't actually use float - see earlier in review */
    return round(dollars * 100);
}

float ask_money(void)
{
    float money;
    do
    {
        money = get_float("Change owed: ");
    }
    while (money < 0);
    return money;
}

Summary

Eliminate the global variables
Use more appropriate numeric types


Compare a sequence with the reference frequency of hexamers I have written this function (and others similar to that one) But I am not sure I am using references on their full power.

My currently concerns is if I make a huge use of memory. The subroutine recieve a reference to two files, the subroutines, like these one return a hash (except &log_time, which returns a scalar). The subroutines expect a reference, thats why I use my %current_seq = ($id_name[0] => $seqs[$j]); my %freq_seq = &hexamer_freq(\%current_seq); on the subroutine, I don't think this is a good way to do it, but I can't imagine a better way to do it.

sub comparer{
    # Compare a sequence with the reference frequency of hexamers
    # First argument the file of to analyse, second argument the file of reference

    my %score;

    #Reading arguments
    my $seq = shift;
    my $ref_seq = shift;

    # Calculating the reference log2
    my %ref_seq = &read_fasta($$ref_seq);
    my %freq_ref = &hexamer_freq(\%ref_seq);

    # Counting hexamers and frequencies for each sequence
    my %seqs = &read_fasta($seq);
    while( my ($id,  $sequen) = each %seqs){

        my @id_seq = split(/\s+/, $id);
        my @id_name = split(/\./, $id_seq[0]);
        my $max = 0;
        my $min = 999;
        for (my $i = 0; $i < 3; $i++){
            last if length $sequen <= $i;
            my $sequ = substr($sequen, $i);
            next unless (defined $sequ);
            my $rev_sequ = reverse($sequ);
            my @seqs = ($sequ, $rev_sequ);
            for (my $j = 0; $j < scalar(@seqs); $j++){
                my %current_seq = ($id_name[0] => $seqs[$j]);
                my %freq_seq = &hexamer_freq(\%current_seq);

                # Handle the sequences that are too short to contain an hexamer
                if (scalar keys %freq_seq == 0){
                    print STDERR &log_time(), "Unable to calculate the Hexamer score of $id_name[0]\n";
                    next;
                };
                # Calculate the hexamer score
                my $score = 0;
                my $n_hexamers = scalar keys %freq_seq;
                foreach my $hex (keys %freq_seq){
                    if (defined $freq_ref{$hex}){
                        $score += log2($freq_seq{$hex}/$freq_ref{$hex});
                    }
                };
                # Store the two possible candidates of "best score"
                if ($score/$n_hexamers > $max){
                    $max = $score/$n_hexamers;
                };
                unless ($score/$n_hexamers > $min) {
                    $min = $score/$n_hexamers
                }
            # Store the data for each sequence
            my $key = $id_name[0] . " frame: $i";
            $key .= " FWD" if $j == 0; # The fwd + or - have the same hexamers
            $key .= " REV" if $j == 1;
            $score{$key} = [$max, $min];
            };
        };
    }
    return %score;
};

Besides, I would like to improve the way the $min is calculated. Now the 999 is an arbitrary number, I expect it won't be reached, because the $freq_ are numbers between 0 and 1, and it is unlikely to have so big numbers (but it may happen).

I removed your comments and inserted mine.

# Documentation goes to POD.

=item comparer

Compare a sequence with the reference frequency of hexamers.

First argument the file of to analyse, second argument the file of reference

=cut

sub comparer {
    my ($seq, $ref_seq) = @_; # I like arguments being processed as the first step in the sub. No need to shift twice.

    my %score;
    my %freq_ref = hexamer_freq({ read_fasta($$ref_seq) });
    my %seqs = read_fasta($seq);

    while (my ($id,  $sequen) = each %seqs) {
        my @id_seq = split ' ', $id;
        my @id_name = split /\./, $id_seq[0];
        my ($max, $min);
        for my $i (0 .. 2) {  # No need for a C-style for.
            last if length $sequen <= $i;

            my $sequ = substr $sequen, $i;
            next unless defined $sequ;

            my $rev_sequ = reverse $sequ;
            my @seqs = ($sequ, $rev_sequ);

            for my $j (0 .. $#seqs) {  # C-style eliminated again.
                my %freq_seq = hexamer_freq({ $id_name[0] => $seqs[$j] }); # Anonymous hash.

                if (keys %freq_seq == 0) {
                    print STDERR log_time(), "Unable to calculate the Hexamer score of $id_name[0]\n";
                    next
                }

                my $score = 0;
                my $n_hexamers = keys %freq_seq; # "scalar" not needed in scalar context.
                for my $hex (keys %freq_seq){
                    if (defined $freq_ref{$hex}){
                        $score += log2($freq_seq{$hex} / $freq_ref{$hex});
                    }
                }

                if (! defined $max || $score / $n_hexamers > $max) {
                    $max = $score / $n_hexamers;
                }
                if (!defined $min || $score / $n_hexamers <= $min) {
                    $min = $score / $n_hexamers
                }

                my $key = $id_name[0] . " frame: $i";
                $key .= (' FWD', ' REV')[$j] if $j < 2;  # Poor man's "switch".
                $score{$key} = [$max, $min];
            }
        }
    }
    return %score
}

There's no need to call subroutines with the & prepended.
Semicolons after blocks are not needed.
To avoid guessing the minimum, just use undef and check for that in the condition. You could also try 'INF', but it's not portable.
Why is the subroutine's second parameter a scalar reference? Is it a very long string?


Rotating lists in linear time and constant space (Java) (The next iteration.)

I have this small utility method for rotating java.util.Lists in linear time, constant space:

package net.coderodde.util;

import java.util.Arrays;
import java.util.Collections;
import static java.util.Collections.swap;
import java.util.List;
import java.util.Objects;
import java.util.Scanner;

/**
 * This class contains a static method for rotating lists in linear time and
 * constant space.
 *
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Oct 6, 2017)
 */
public final class ListReversal {

    private ListReversal() {}

    /**
     * Performs the list rotation.
     *
     * @param <T>           the list element type.
     * @param list          the list whose content to rotate.
     * @param rotationCount the number of steps to rotate to the right. If
     *                      negative, rotates to the left.
     */
    public static <T> void rotate(List<T> list, int rotationCount) {
        Objects.requireNonNull(list, "The input list is null.");

        if (rotationCount < 0) {
            rotateLeft(list, -rotationCount);
        } else if (rotationCount > 0) {
            rotateRight(list, rotationCount);
        }
    }

    private static <T> void rotateLeft(List<T> list, int rotationCount) {
        Collections.<T>reverse(list);
        rotationCount %= list.size();
        reverse(list, list.size() - rotationCount, list.size());
        reverse(list, 0, list.size() - rotationCount);
    }

    private static <T> void rotateRight(List<T> list, int rotationCount) {
        Collections.<T>reverse(list);
        rotationCount %= list.size();
        reverse(list, 0, rotationCount);
        reverse(list, rotationCount, list.size());
    }

    private static <T> void reverse(List<T> list, int fromIndex, int toIndex) {
        for (int i = fromIndex, j = toIndex - 1; i < j; ++i, --j) {
            swap(list, i, j);
        }
    }

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        List<Integer> list = Arrays.asList(1, 2, 3, 4, 5, 6);
        System.out.println(list);

        while (true) {
            int rotationCount = scanner.nextInt();
            rotate(list, rotationCount);
            System.out.println(list);
        }
    }
}

I would like to hear any comments regarding the above.

The time complexity of the rotate method is O(N^2) if the list doesn't support random access because each call to swap takes O(N) time. One can fix it by using two ListIterators (one going forward and one going backwards) to reverse a part of the list.
There's no need to create separate rotateLeft and rotateRight methods. You can always take the shift modulo the size of the list (in mathematical sense, without negative results, not in a sense of the operator %) instead.


Weight-loss calculator I have a table that holds a user's weight for specific days. In my weights_controller.rb index action I can show these weights - no problem. What I also want to show on the index is the user's current weight, his weight loss over time as well as the timeframe he achieved this weight loss.

Thus I created a weight_calculator.rb to do the needed calculations:

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

  def current_weight
    @current_weight = Weight.where(:user_id => @user.id).order(:day).last
  end

  def weight_loss
    @current_weight = Weight.where(:user_id => @user.id).order(:day).last
    @start_weight = Weight.where(:user_id => @user.id).order(:day).first
    @weight_loss = @current_weight.kilograms - @start_weight.kilograms
  end

  def timeframe
    @current_weight = Weight.where(:user_id => @user.id).order(:day).last
    @start_weight = Weight.where(:user_id => @user.id).order(:day).first
    @weight_loss_timeframe = @current_weight.day - @start_weight.day
    @weeks = @weight_loss_timeframe.to_i/7
  end
end

This is my weights_controller.rb index action:

  def index
    @weights = Weight.where(:user_id => current_user.id).order(day: :desc)
    @weight_loss = WeightCalculator.new(@profile, current_user).weight_loss
    @weeks = WeightCalculator.new(@profile, current_user).timeframe

    @result = {}
    Weight.where(:user_id => current_user.id).each do |weight|
      @result[weight.day] = weight.kilograms
    end

    @profile = Profile.where(:user_id => current_user.id)
  end

Everything works fine in this setup. However, there is clearly some repition in my weight_calculator.rb and I am asking myself what would be the best way to write this code to be DRY conform?

There is really quite a lot wrong here.

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

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

Instead of this?

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

That alone would drastically improve the readability of your code.

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

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

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

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

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

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

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

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

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

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

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

  def weight_loss
    current_weight.kilograms - start_weight.kilograms
  end

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

Your index action is also pretty broken.

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

@profile = current_user.profile

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

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

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


Pass in parameters instead of multiple functions I can't quite figure out the best way to approach this. I have two jQuery objects that both are used to set cookies. I then parse the JSON and set a cookie that is either the users height or the users weight.

This works fine, but I feel like there is an extreme amount of redundant code here that could be cleaned up but for the life of me I'm blanking on how to best do this. Ideally I'd like to be able to somehow condense this and have a function that I can pass in parameters if I want to receive the height cookie or the weight cookie.

Possibly something along the lines of: userInfo(heightCookie) or userInfo(weightCookie).

I'm a bit of a newbie, so code examples are ideal, please.

var height_cookie = {
        name: 'user-height',
        options: {
            path: '/',
            expires: 365
        }
    };
var weight_cookie = {
        name: 'user-weight',
        options: {
        path: '/',
        expires: 365
        }
    };

function userHeightCookie() {
            var userData = $.parseJSON($.cookie(height_cookie.name));
            return(userData);
    };

function userWeightCookie() {
            var userData = $.parseJSON($.cookie(weight_cookie.name));
            return(userData);
    };

function readHeightCookie(userInfo) {
        $.cookie(height_cookie.name, JSON.stringify(userInfo), height_cookie.options);
    };

function readWeightCookie(userInfo) {
        $.cookie(weight_cookie.name, JSON.stringify(userInfo), weight_cookie.options);
    };

var userInfo = readHeightCookie();
var userInfo2 = readWeightCookie();

I'd put the cookies into an object so they can be named.

var userCookies = {
    height: {
        name: 'user-height',
        options: {
            path: '/',
            expires: 365
        }
    },
    weight: {
        name: 'user-weight',
        options: {
            path: '/',
            expires: 365
        }
    }
};

Then you only need read/write cookie methods: (I've not used $.cookie so I'm only guessing I haven't broken it.)

function readCookie(cookieName) {
    var userData = $.parseJSON($.cookie(userCookies[cookieName].name));
    return(userData);
};

function writeCookie(userInfo, cookieName) {
    $.cookie(weight_cookie.name, JSON.stringify(userInfo),userCookies[cookieName].options);
};

If you want to be really modular (like all the cool kids) you might do something like:

var CookieMonster =(function ($) {
    var userCookies = {
        height: {
            name: 'user-height',
            options: {
                path: '/',
                expires: 365
            }
        },
        weight: {
            name: 'user-weight',
            options: {
                path: '/',
                expires: 365
            }
        }
    };
    return {
        readCookie: function readCookie(cookieName) {
            var userData = $.parseJSON($.cookie(userCookies[cookieName].name));
            return(userData);
        },
        writeCookie: function writeCookie(userInfo, cookieName) {
            $.cookie(weight_cookie.name, JSON.stringify(userInfo),userCookies[cookieName].options);
        }
    };
})(jQuery);

var userInfo = CookieMonster.readCookie('height');
var userInfo2 = CookieMonster.readCookie('weight');