Reading groups of files and concatenating them I have made some adjustments to some code that you can see on this thread:

Read daily files and concatenate them

I would like to make some further refinements and make sure I am on the right track organize some of my files better. I added some functions and made some other changes to prevent errors and make my code easier to reuse (and to be more Pythonic).

#got rid of import *
import pandas as pd
import numpy as np
import datetime as dt

ftploc = r'C:\Users\FTP\\'
loc = r'C:\Users\\'
splitsname = 'Splits'
fcrname = 'fcr_report_'
npsname = 'csat_report_'
ahtname = 'aht_report_'
rostername = 'Daily_Roster'
vasname = 'vas_report_'
ext ='.csv'

#had to create some periods and date format parameters
start_period = '13 day'
end_period = '1 day'
fcr_period = '3 day'
date_format1 = '%m_%d_%Y'
date_format2 = '%Y_%m_%d'
start_date = dt.date.today() - pd.Timedelta(start_period)
end_date = dt.date.today() - pd.Timedelta(end_period)
fcr_end_date = end_date - pd.Timedelta(fcr_period)
daterange1 = pd.Timestamp(end_date) - pd.Timestamp(start_date)
daterange2 = pd.Timestamp(fcr_end_date) - pd.Timestamp(start_date)
daterange1 = (daterange1 / np.timedelta64(1, 'D')).astype(int)
daterange2 = (daterange2 / np.timedelta64(1, 'D')).astype(int)
print('Starting scrubbing file...')

#AHT files have a different date format in the filename so I made this function
def dateFormat(filename):
    if filename == ahtname:
        return date_format2
    else:
        return date_format1

#FCR is 3 days delayed (72 hour window) so I needed to create some logic to adjust for it
def dateRange(filename):
    if filename == fcrname:
        return daterange2
    else:
        return daterange1

#this function works on all of my files now. I just wonder if there is a better way to refer to the other functions? Is having a separate function for the date range and format ideal?
def readAndConcatFile(filename, daterange):
    df_list = []
    try:
        for date_range in (pd.Timestamp(startdate) + dt.timedelta(n) for n in range(dateRange(filename))):
            df = pd.read_csv(ftploc + filename + date_range.strftime(dateFormat(filename)) + ext, parse_dates = True)
            df_list.append(df)
        return pd.concat(df_list)
    except IOError:
        print('File does not exist: ', filename + date_range.strftime(dateFormat(filename)) + ext)

#this appears to work great. I lose the ability to read certain columns though or to specify dtypes for specific columns
nps = readAndConcatFile(npsname, daterange)
vas = readAndConcatFile(vasname, daterange)
fcr = readAndConcatFile(fcrname, daterange)
aht = readAndConcatFile(ahtname, daterange)

Please note that I'm not sure if I'm reading your script correctly. I compared this to the original to see what's lost from how you called the functions. If I've misunderstood entirely please let me know.

In your original script one of your read_csv calls passed in a 'date_completed' key that you left out here to use one function for all files, but you can still get that information using a default value. Default values can be included in a function parameter list so even if they're not supplied, the variable will exist. In your case it would be good to have a value for parse_dates.

def readAndConcatFile(filename, daterange, parse_dates=True):
    df_list = []
    try:
        for date_range in (pd.Timestamp(startdate) + dt.timedelta(n) for n in range(dateRange(filename))):
            df = pd.read_csv(ftploc + filename + date_range.strftime(dateFormat(filename)) + ext, parse_dates = parse_dates)
            df_list.append(df)
        return pd.concat(df_list)
    except IOError:
        print('File does not exist: ', filename + date_range.strftime(dateFormat(filename)) + ext)

This means that in the absence of any value True will be passed to parse_dates as you're currently doing. However you could also pass specific parameters like you did previously.

nps = readAndConcatFile(npsname, daterange, ['call_date','date_completed'])
vas = readAndConcatFile(vasname, daterange, ['Call_date'])
fcr = readAndConcatFile(fcrname, daterange, ['call_time'])
aht = readAndConcatFile(ahtname, daterange)

However I noticed that you previously passed nothing at all to your call for aht whereas you're now passing True. If that's something you'd like to avoid, that's easy with a slight modification. When you want to use a default to make a parameter optional, set the default as None and then you can have a line where you test whether there was a parameter passed or not.

def readAndConcatFile(filename, daterange, parse_dates=None):
    df_list = []
    try:
        for date_range in (pd.Timestamp(startdate) + dt.timedelta(n) for n in range(dateRange(filename))):
            if parse_dates is None:
                df = pd.read_csv(ftploc + filename + date_range.strftime(dateFormat(filename)) + ext)
            else:
                df = pd.read_csv(ftploc + filename + date_range.strftime(dateFormat(filename)) + ext, parse_dates = parse_dates)
            df_list.append(df)
        return pd.concat(df_list)
    except IOError:
        print('File does not exist: ', filename + date_range.strftime(dateFormat(filename)) + ext)

This means you no longer have to pass True to parse_dates for aht just to have the function work.


Python - Simple wrapper class for a bot API Is the following in poor taste?

This would be contained within a groupme module. Does it make sense to re-define a RequestException class within this module? If I didn't, wouldn't consumers of the module need to be aware of the underlying requests.exceptions.RequestException being thrown:

...

class Bot(object):

    API_BASE_URL = 'http://api.groupme.com/v3/bots/{route}'
    API_POST_URL = API_BASE_URL.format(route='post')

    def __init__(self, bot_id):
        self.bot_id = bot_id

    def post(self, message):
        try:
            data = {'bot_id': self.bot_id, 'message': message}
            response = requests.post(Bot.API_POST_URL, data)
            response.raise_for_status()
        except requests.exceptions.RequestException as ex:
            raise RequestException('Failed to post bot message') from ex

class RequestException(Exception):

    ...

And the groupme module / above code would then be used in the following manner:

import groupme

bot = groupme.Bot('abcdef123')
try:
    bot.post('Hello, world!')
except groupme.RequestException as ex:  # this seems cleaner as opposed to requests.exceptions.RequestException
    print(ex)

Does it make sense to re-define a RequestException class within this module?

Yes, that makes total sense. Library users should never be expected to catch dependency exceptions. If there's even the slightest chance a user needs to interact with a dependency exception, you should 'copy' it.

If you wish to create a custom exception, it's nearly always better to derive it from an exception category (type error, value error, lookup error, ...) than directly from Exception. ConnectionError would suffice.

The from ex is unnecessary. The previous exception is already preserved as context.

You should also be asking yourself 'does the user really need to see the full stack trace?'. For trivial problems, things like HTTP 4XX or 5XX status codes, you could adjust the exception message to include the code.


Implementation similar to boost::optional for modelling function failure The idea being you can have a simple type called maybe that wraps another type, which provides the ability to possibly not return a value from a function.  I'm sure it's not perfect yet, but it's got the broad strokes I'm looking for.  I'm mostly looking for corner cases I've missed and anything else I've overlooked.

#ifndef MAYBE_H_
#define MAYBE_H_

#include <exception>

namespace {
    // implementation enabled for non-reference types
    template <class T>
    struct maybe {
        maybe()             : valid(false) {}       // default constructor

        maybe(const T& val) : valid(true)  {        // contructor from wrapped value
            new ((void*)storage) T(val);
        }

        maybe(const maybe &other) {                 // copy constructor
            valid = other.valid;
            if (valid) {
                new ((void*)storage) T(other.get_ref());
            }
        }

        maybe(maybe &&other) : maybe() {            // move constructor
            std::swap(valid,   other.valid);
            std::swap(storage, other.storage);
        }

        ~maybe() {                                  // destructor
            if (valid) {
                get_ref().~T();
            }
        }

        // assignment operator, uses copy-and-swap trick
        maybe& operator =(maybe other) {
            swap(*this, other);
            return *this;
        }

        // type-coercion operator to get back to wrapped value
        // calling without checking validity causes bad_exception
        operator T&() {
            if (!valid) {
                throw std::bad_exception();
            }
            return get_ref();
        }

        // for checking validity of container
        explicit operator bool() { return valid; }
    private:
        bool valid;
        alignas(T) char storage[sizeof(T)];

        T& get_ref() const { return *static_cast<T*>((void*)storage); }
    };

    // implementation enabled for reference types
    template <class T>
    struct maybe<T&> {
        maybe()                   : valid(false)        value(nullptr)     {}
        maybe(T& val)             : valid(true),        value(&val)        {}
        maybe(const maybe &other) : valid(other.valid), value(other.value) {}

        operator T&() {
            if (!valid) {
                throw std::bad_exception();
            }
            return *value;
        }

        explicit operator bool() { return valid; }
    private:
        bool valid;
        T*   value;
    };
} // namespace {

#endif//MAYBE_H_

Let's go over it bit-by-bit:

You should sprinkle around quite a lot of constexpr and noexcept-markers.
If at least one instantiation of a template is potentially constexpr, marking the template thus is not an error. Not marking it means none is.
union{T value;}; better shows the intent and is simpler than alignas(T) char storage[sizeof(T)];.
Do not values with byte-for-byte unless they are trivial types. Doing so is simply wrong.
The move-ctor should move, not swap. That might be far less expensive anyway.
Your base variant should gain a specialization for trivially copyable types.
Your base-variant should have a variadic-template ctor for constructing the value directly.
Your references-variant does not need a separate valid-flag: *NULL is not valid, thus a null pointer serves adequately.
Only include the minimal headers you need in your include-file.
Do not use an unnamed namespace, or every translation-unit will have its own unique template (which is different from all the others).

As a side-note, boost::optional should serve for the case you describe.


How to use recursion to turn object into string? EDIT
To me JSON.stringify is not the solution here, as it is much slower than recursion
    JSON.stringify(fakeData)
        .replace(/,/g, ';')
        .replace(/:(?={)/g, '')
        .replace(/"/g, '')

//Result
Recursion with reduce x 816,321 ops/sec ±7.38% (80 runs sampled)
JSON.stringify x 578,221 ops/sec ±1.72% (92 runs sampled)
Fastest is Recursion with reduce

I am making a css-in-js library, and I need to use recursion to turn object into string like this:
Input:
const fakeData = {
  a: {
    b: 'c',
    d: 'e'
  }
}

Output:
a{b:c;d:e;}

My recursion function:
const buildKeyframe = (obj) => {
    return Object.entries(obj).reduce((acc, [prop, value]) => {
        if (typeof value === 'string') {
            return `${acc}${prop}:${value};`
        }

        return `${acc}${prop}:{${buildKeyframe(value)}}`
    }, '')
}

This function works, but I think there is room for improvement(e.g. use TCO, avoid using reduce)... How can I write a better function to recurse this data structure?

I agree that JSON.stringify isn't the most appropriate for this problem, not just because it's slower, but also because it's a bit harder to understand. And you're also right in assuming that reduce() isn't the best tool for the job. Using a .map() reads in a much more natural way.

const convert = obj => (
  Object.entries(obj)
    .map(([key, value]) => (
      typeof value === 'string'
        ? `${key}:${value};`
        : `${key}{${convert(value)}}`
    ))
    .join('')
)

console.log(convert({
  a: {
    b: 'c',
    d: 'e'
  },
}))

When optimizing this, think about the use case. While tail-call recursion may or may not be possible, it will make the code much more difficult to read, and it really won't help. The main advantage to tail-call recursion is to save memory on the call stack, and to let a recursive function call into itself an unlimited amount of times while still using just one call-frame worth of memory on the stack. There's no practical chunk of extremely nested CSS that can be written to make tail-call optimization worth it.
In your question, speed seems to be one of your primary concerns, but I'll play a bit of devil's advocate here and ask if it really should be for this use case. How much CSS-in-JS would you need to write before any of these micro-performance improvements even become noticeable? Tens of thousands of lines? Hundreds of thousands? I would suggest just building out the library first, run some benchmarks afterwards, and find out what parts of it actually run slow. Then only worry about fixing the slower areas.


Project Euler #12 - first triangle number with more than 500 divisors I tried to solve Project Euler 12 with a first very naive solution by myself. It took nearly 30 minutes to run until it found the solution. Then I made a change in the function getDivisorCount which should have made the run time to about the square root of the original code, about 5 minutes. At least this was my opinion because the complexity should have changed from \$O(n^2)\$ to \$O(n\sqrt n)\$. But it went down to less than a second which surprised me and I could not find a reason.

Here is my code for review, a second time:

#include <iostream>
#include <math.h>

using namespace std;

int getDivisorCount(unsigned int number)
{
    unsigned int count = 0;
    unsigned int sqrt_ = sqrt(number);
    for(unsigned int i = 1; i <= sqrt_; i++)
    {
        if((number % i) == 0)
            count+=2;
    }
    if (sqrt_ * sqrt_ == number)
    {
        count--;
    }
    return count;
}

int main()
{
    unsigned int number = 0;
    for (unsigned int i = 1; ; i++)
    {
        number+=i;
        if(getDivisorCount(number)>500)
            break;
    }
    cout << number;
    return 0;
}

Note that my first version used the method:

int getDivisorCount(unsigned int number)
{
    unsigned int count = 0;
    for(unsigned int i = 1; i <= number; i++)
    {
        if((number % i) == 0)
            count++;
    }
    return count;
}

Both compiled with g++ -O2.

I'm looking for a fresh code review, but an explanation of why the current version is so much faster would also be appreciated.

Nitpicks

using namespace std;

This can be a bad habit to start.  See Why is using namespace std bad practice?

unsigned int sqrt_ = sqrt(number);

I don't like the name sqrt_.  Something like sqrt_number would be better in my opinion.

unsigned int number = 0;
for (unsigned int i = 1; ; i++)
{
    number+=i;
    if(getDivisorCount(number)>500)
        break;
}

That's an odd way to write a loop.  You are iterating i but never checking it.  I would prefer something like

for ( unsigned int number = 1, i = 2; getDivisorCount(number) <= 500; number += i, i++ ) ;

That's rather dense -- it might be easier in a while loop.  But at least this has a variable declaration, a loop check, and an increment.  Also, it avoids having to break to exit the loop.

unsigned int number = 1;
unsigned int i = 2;
while ( getDivisorCount(number) <= 500 ) {
    number += i;
    i++;
}

Analysis

You refer to your original code as quadratic, but quadratic in what?  The outer loop in main runs i times.  The inner loop runs number times.  What's number in terms of i?  The answer is that number is quadratic in i.  I.e. \$O(i^2)\$.  So we actually run the inner loop in getDivisorCount \$O(i^3)\$ times.

The optimized loop only runs the square root of number times.  But we know that the square root of number is about i.  So overall, it runs \$O(i^2)\$ times.

So the difference between the two is about i in magnitude.  What's i in this case?  Well, without giving the exact answer, I'll tell you that it's over 10,000.  You can get the exact answer by replacing

cout << number;

with

std::cout << i;

Note that you may have to move the declaration of i outside the loop.

Anyway, that's why your revised code is more than 10,000 times faster than your original code.  That's how big i was.

More generally, when going from \$O(n^2)\$ to \$O(n)\$ in an otherwise similar algorithm, you don't take the square root of the time.  You divide by n.  In this case, i or number is n.  This is why \$O(n)\$ is so much better than \$O(n^2)\$.


One-time pad encryption using letter shifting This program asks the user to input a List of words. It then returns a list of encrypted words, and a second list which specifies the numerical shifts used to encrypt those words. The idea was obtained from https://www.khanacademy.org/computing/computer-science/cryptography/crypt/v/one-time-pad

import random

def rotate_letter(letter, number):
    if letter == ' ':
        return letter
    if letter.islower():
        start = ord('a')
    elif not letter.islower():
        start = ord('A')

    normal_shift = ord(letter) - start
    new_letter = start + (normal_shift + number) % 26
    return chr(new_letter)

def avoid(letter):
    avoid = ' ,!?¿,*+-%/@1234567890'
    for i in avoid:
        if i == letter:
            return False
    return True

def rotate_word(word):
    new_word = ''
    random_series = []
    for letter in word:
        if avoid(letter):
            random_number = random.randint(1,26)
            new_letter = rotate_letter(letter, random_number)
            new_word += new_letter
            random_series.append(random_number)
        else:
            new_word += letter
    return (new_word, random_series)

def shift(words):
    index = 0
    encrypted_words = []
    random_shifts = []
    while index < len(words):
        (new_word, random_series) = rotate_word(words[index])
        encrypted_words.append(new_word)
        random_shifts.append(random_series)
        index += 1
    return (encrypted_words, random_shifts)

def get_input():
    words = []
    while True:
        user = (input('Input words to encrypt. Done when ready \n' ))
        if user == 'Done' or user == 'done':
            return words
        words.append(user)

def start():
    words = get_input()
    encrypted_words = []
    random_shifts = []
    (encrypted_words, random_shifts) = shift(words)
    print('UNENCRYPTED WORDS', words)
    print('ENCRYPTED WORDS', encrypted_words)
    print('RANDOM SHIFTS', random_shifts)

start()

Your function avoid is quite unnecessary and also confusing. I would expect avoid('@') to return True. When you store the letters as a global constant and as set, you should gain some speed (both because it does not have to be reconstructed again and again and because membership testing is very fast for a set. I replaced your function with letter not in letters_avoid.

Whenever you are doing string addition in Python, you are probably doing something wrong. This is because strings are immutable so for every addition a new string has to be created and the content of the strings to be added needs to be copied. This is quite slow. Rather, use a list to accumulate the letters of new_word and str.join them at the end.

When returning a tuple, the () are implicit. You can just write return a, b.

When iterating over words, you should just directly iterate over it, instead of its indices.

Using str.lower cuts down the double comparison if user == 'Done' or user == 'done' to if user.lower() == 'done':.

You should call your code with a if __name__ == '__main__': guard to allow importing your functions from another script without executing the whole script.

Final code:

import random

letters_avoid = set(' ,!?¿,*+-%/@1234567890')

def rotate_letter(letter, number):
    if letter == ' ':
        return letter
    if letter.islower():
        start = ord('a')
    elif not letter.islower():
        start = ord('A')

    normal_shift = ord(letter) - start
    new_letter = start + (normal_shift + number) % 26
    return chr(new_letter)

def rotate_word(word):
    new_word = []
    random_series = []
    for letter in word:
        if letter not in letters_avoid:
            random_number = random.randint(1, 26)
            new_word.append(rotate_letter(letter, random_number))
            random_series.append(random_number)
        else:
            new_word.append(letter)
    return "".join(new_word), random_series

def shift(words):
    encrypted_words = []
    random_shifts = []
    for word in words:
        new_word, random_series = rotate_word(word)
        encrypted_words.append(new_word)
        random_shifts.append(random_series)
    return encrypted_words, random_shifts

def get_input():
    words = []
    while True:
        user = input('Input words to encrypt. Done when ready \n')
        if user.lower() == 'done':
            return words
        words.append(user)

def start():
    words = get_input()
    encrypted_words, random_shifts = shift(words)
    print('UNENCRYPTED WORDS', words)
    print('ENCRYPTED WORDS', encrypted_words)
    print('RANDOM SHIFTS', random_shifts)

if __name__ == "__main__":
    start()


Creating object Nodes an easier way I am required to create a bunch of object nodes to an object:

this.selectedText = e.text;
this.clipNo = e.clipNo;

if(!this.clipCollection.hasOwnProperty(this.category)) {
    this.clipCollection[this.category] = {};
}

if(!this.clipCollection.hasOwnProperty("grandClipCount")) {
    this.clipCollection.grandClipCount = 0;
}

if(!this.clipCollection[this.category].hasOwnProperty(['pageNo'+this.pageNumber])) {
    this.clipCollection[this.category]['pageNo'+this.pageNumber] = {};
    this.clipCollection[this.category]['pageNo'+this.pageNumber].pageServerInfo = this.pageServerInfo;
}

if(!this.clipCollection[this.category][this.category+"ClipCount"]) {
    this.clipCollection[this.category][this.category+"ClipCount"] = 0;
}

if(!this.clipCollection[this.category]['pageNo'+this.pageNumber].hasOwnProperty("clips")) {
    this.clipCollection[this.category]['pageNo'+this.pageNumber].clips = {};
    this.clipCollection[this.category]['pageNo'+this.pageNumber].clips['clip'+this.clipNo] = this.selectedText;
    this.clipCollection[this.category]['pageNo'+this.pageNumber].pageClipCount = 0;
}

this.clipCollection[this.category]['pageNo'+this.pageNumber].clips['clip'+this.clipNo] = this.selectedText;
this.clipCollection[this.category]['pageNo'+this.pageNumber].pageClipCount += 1;
this.clipCollection[this.category][this.category+"ClipCount"] += 1;
this.clipCollection.grandClipCount += 1;

Is there a short approach to minimize this code?

Well for starters maybe get rid of this with a few local variables and
give names to often used constants, e.g.

var pageNo = 'pageNo'+this.pageNumber;

or so.

The blocks with hasOwnProperty checks can be replaced with a
conditional:

this.clipCollection[this.category] = this.clipCollection[this.category] || {};

this.clipCollection.grandClipCount = (this.clipCollection.grandClipCount || 0) + 1;

And so on.


Server-client data transfer I coded a server-client (kind of) chat, and I need your review as I'm sure it is a mess. I used lots of tutorials and tips form different websites and forums which were posted at different times (maybe now it's a lot easier to write what I want) and slapped them on Visual Studio. It's not like I wanted it to be a chat, but a way to communicate with a server as a client.

Server-Side

using System;
using System.Text;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace First_Server
{
    class Program
    {

    private static void SocketThread(object obj)
    {
        var ClientSocket = (TcpClient)obj;
        while (true)
        {
            try
            {
                NetworkStream NetworkStream = ClientSocket.GetStream();
                byte[] BytesFromClient = new byte[10025];
                NetworkStream.Read(BytesFromClient, 0, ClientSocket.ReceiveBufferSize);
                string DataFromClient = Encoding.ASCII.GetString(BytesFromClient);
                DataFromClient = DataFromClient.Substring(0, DataFromClient.IndexOf("#"));
                Console.WriteLine(DataFromClient);
                string ServerResponse = DataFromClient;
                Byte[] SendBytes = Encoding.ASCII.GetBytes(ServerResponse);
                NetworkStream.Write(SendBytes, 0, SendBytes.Length);
            }
            catch (Exception StreamException)
            {
                Console.WriteLine("Connection from a client closed.");
                ClientSocket.Close();
            }
        }
    }

    static void Main(string[] args)
    {
        int Port = 42069;
        IPAddress LocalIPAddress = IPAddress.Parse("192.168.1.10");
        TcpListener ServerSocket = new TcpListener(LocalIPAddress, Port);
        TcpClient ClientSocket;
        ServerSocket.Start(); Console.WriteLine("Server started. Waiting for requests...");

        while (true)
        {
            ClientSocket = ServerSocket.AcceptTcpClient(); Console.WriteLine("Connection from a client accepted.");
            ThreadPool.QueueUserWorkItem(SocketThread, ClientSocket);
        }
    }
}
}

Client-Side

using System;
using System.Text;
using System.Threading;
using System.Net.Sockets;
using System.Windows.Forms;

namespace First_Client
{
  public partial class Form1 : Form
  {

    TcpClient ClientSocket = new TcpClient();

    public void msg(string Message)
    {
        textBox1.Text = textBox1.Text + Message + Environment.NewLine;
    }

    public void ServerListener(object obj)
    {
        loop:
        var NetworkStream = (NetworkStream)obj;
        byte[] FromTheServer = new byte[10025];

        try
        {
            NetworkStream.Read(FromTheServer, 0, ClientSocket.ReceiveBufferSize);
        }
        catch (Exception StreamError)
        {
            goto loop;
        }

        string StringFromServer = Encoding.ASCII.GetString(FromTheServer);
        textBox1.Text = textBox1.Text + StringFromServer + Environment.NewLine;
    }

    public Form1()
    {
        InitializeComponent();
        ClientSocket.Connect("95.76.40.237", 42069); label1.Text = "Client status: connected to server";
        ThreadPool.QueueUserWorkItem(ServerListener);
    }

    private void button1_Click(object sender, EventArgs e)
    {
        NetworkStream NetworkStream = ClientSocket.GetStream();
        byte[] ToTheServer = Encoding.ASCII.GetBytes(textBox2.Text + "#");
        NetworkStream.Write(ToTheServer, 0, ToTheServer.Length);
        byte[] FromTheServer = new byte[10025];
        NetworkStream.Read(FromTheServer, 0, ClientSocket.ReceiveBufferSize);
        string StringFromServer = Encoding.ASCII.GetString(FromTheServer);
        msg(StringFromServer);
        textBox2.Text = "";
    }

    private void Form1_Load(object sender, EventArgs e)
    {

    }

    private void label1_Click(object sender, EventArgs e)
    {

    }
}
}

Naming
You code's naming conventions are very inconsistent (which possibly reflects the fact that you've taken bits of it from different sources).  Generally speaking, I expect local variables to follow a camelCasing naming convention.  Your current approach ends up with lines like this:
catch (Exception StreamException)

Which honestly at first glance made me ask the question 'have they done something funky with exception filters'.
Exceptions
You don't actually do anything with any of the exceptions that you catch.  If you don't need to access the variable, then you can simply catch the exception:
catch (Exception)

Typically you'd also want to try to avoid catching base exception.  It suggests that you don't really know what exceptions can be thrown.  Sometimes it's the right thing to do, but more often than not you should be catching more specific exceptions so that you can take more appropriate action.
When you're done, you're done
Your thread function in your server contains an endless loop.  This works whilst the client is behaving, but falls apart once you encounter an error.  Within the loop, you're catching all exceptions and responding by closing the socket:
catch (Exception StreamException)
{
    Console.WriteLine("Connection from a client closed.");
    ClientSocket.Close();
}

Best case, the second time you call Close, it throws an exception and the thread function crashes.  Worst case, the Close doesn't throw an exception and you don't notice that the thread function is in an endless loop, eating processor time.
Once you've closed the connection, you should return from the thread function to prevent the pointless spinning.


Using a Dictionary to retrieve BOOL values I have some values pulled from a web service giving me a bool value for a key.  I did it this way, but feel like it is repeating without need.  How can I refactor this?

Please note that the key on the dictionary has the same name as the object property.

for (NSDictionary *itemDicto in switchesArray) {
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindBookMeeting"]) {
        currentAppswitches.FindBookMeeting = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindCapabilityLocate"]) {
        currentAppswitches.FindCapabilityLocate = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindDesk"]) {
        currentAppswitches.FindDesk = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"FindEmployeeLocate"]) {
        currentAppswitches.FindEmployeeLocate = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"Help"]) {
        currentAppswitches.Help = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"ReportProblemIT"]) {
        currentAppswitches.ReportProblemIT = [[itemDicto objectForKey:@"value"]boolValue];
        //test ok
        //currentAppswitches.ReportProblemIT = NO;
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"ReportProblemProperty"]) {
        currentAppswitches.ReportProblemProperty = [[itemDicto objectForKey:@"value"]boolValue];
    }
    if ([[itemDicto objectForKey:@"id"]isEqualToString:@"Settings"]) {
        currentAppswitches.Settings = [[itemDicto objectForKey:@"value"]boolValue];
    }
}

Assuming the values under the id keys always match properties of currentAppswitches.

If you can change currentAppswitches's class to accept an NSNumber object rather than a BOOL you have some convenient options.

If currentAppswitches is KVO compliant you could write:

for (NSDictionary *item in switchesArray) {
  id value = [item objectForKey:@"value"];
  id key = [item objectForKey:@"id"];
  [currentAppswitches setValue:value forKeyPath:key];
}

Alternately you could use:

for (NSDictionary *item in switchesArray) {
  BOOL value = [[item objectForKey:@"value"] boolValue];

  //construct a selector for the property setter
  NSMutableString *selectorName = [item objectForKey:@"id"];
  NSString *firstCharacter = [selectorName substringToIndex:1];
  [selectorName stringByReplacingCharactersInRange:NSMakeRange(0,1) withString:[firstCharacter uppercaseString]];
  [selectorName insertString:@"set" atIndex:0];
  [selectorName appendString:@":"];

  SEL selector = NSSelectorFromString(selectorName);
  [currentAppswitches performSelector:selector withObject:value];
}

If currentAppswitches must continue to accept BOOLs then you could create an invocation:

for (NSDictionary *item in switchesArray) {
  id value = [item objectForKey:@"value"];

  //construct a selector for the property setter
  NSMutableString *selectorName = [item objectForKey:@"id"];
  NSString *firstCharacter = [selectorName substringToIndex:1];
  [selectorName stringByReplacingCharactersInRange:NSMakeRange(0,1) withString:[firstCharacter uppercaseString]];
  [selectorName insertString:@"set" atIndex:0];
  [selectorName appendString:@":"];

  SEL selector = NSSelectorFromString(selectorName);
  NSMethodSignature* signature = [[currentAppswitches class] instanceMethodSignatureForSelector:selector];
  NSInvocation* invocation = [NSInvocation invocationWithMethodSignature:signature];
  [invocation setTarget:currentAppswitches];
  [invocation setSelector:selector ];
  [invocation setArgument:&myBoolValue atIndex:2];
  [invocation invoke];
}


Bootstrap show many move one carousel modification I've been steadily improving a show many move one carousel sample that I found a few weeks ago. I want to post my improvements for others to use but don't want to spread bad code. I'm not an expert in JavaScript but I always like to produce clean code.

Working demo

(function(){
  $('.carousel-showmanymoveone .item').each(function(){
    var itemToClone = $(this);

    for (var i=1;i<4;i++) {
      itemToClone = itemToClone.next();

      // wrap around if at end of item collection
      if (!itemToClone.length) {
        itemToClone = $(this).siblings(':first');
      }

      // grab item, clone, add marker class, add to collection
      itemToClone.children(':first-child').clone()
        .addClass("cloneditem-"+(i))
        .appendTo($(this));
    }
  });
}());

Thoughts

I've done all the improvements I can think of but:

I'm wondering about seeing $(this) appear twice.
Are the siblings, children, :first-child, :first usages the best options?
Any other improvements or best practices I've missed?

Is there something I could do better?

Thanks for looking at this. I have concluded that I am happy with the code.

To incorporate the valid suggest by @vmariano, I have split this off into its own variable. This does make sense from a best-practices viewpoint as once I clone the item it is no longer itemToClone it is a clonedItem.

Final code:

(function(){
  $('.carousel-showmanymoveone .item').each(function(){
    var itemToClone = $(this);

    for (var i=1;i<4;i++) {
      itemToClone = itemToClone.next();

      // wrap around if at end of item collection
      if (!itemToClone.length) {
        itemToClone = $(this).siblings(':first');
      }

      // grab item, clone, add marker class, add to collection
      var clonedItem = itemToClone.children(':first-child').clone();
      clonedItem.addClass("cloneditem-"+(i))
         .appendTo($(this));
    }
  });
}());


Efficient way to efficiently merge arrays of objects in JavaScript I have the below scenario where I am trying to merge two arrays of objects.

The current code works, but I am looking for a more efficient way.

var constList = [
  { name: "jack", id: "1", designation: "hr" },
  { name: "mary", id: "2", designation: "it" },
  { name: "john", id: "3", designation: "fin" }
]

var apiList = [
  { name: "jack", id: "1", height: "10" },
  { name: "mary", id: "2", height: "20" }
]

var temp = [];

constList.forEach(x => {
  apiList.forEach(y => {
    if (x.id === y.id) {
      temp.push({ ...x,
        ...y
      });
    }
  });
});

console.log(temp);

You can use reduce for this purpose. Group the objects into an aggregate object by some key which we can make a parameter to a function mergeByKey. This function takes any number of object arrays which are then flattened (change flat to [].concat(...objArrs) for better browser compatibility) then reduced into a single object. During grouping, object spreading updates the value for each unique key as you did. Finally, extract the array of values from the grouping object using Object.values which is the final result.

Time complexity of the original version was O((n*longest_obj_length)^2), but the updated version is O(n*longest_obj_length).

const mergeByKey = (key, ...objArrs) =>
  Object.values(objArrs.flat().reduce((a, e) => {
    a[e[key]] = {...a[e[key]], ...e};
    return a;
  }, {}))
;

const constList = [
  { name: "jack", id: "1", designation: "hr" },
  { name: "mary", id: "2", designation: "it" },
  { name: "john", id: "3", designation: "fin" }
];

const apiList = [
  { name: "jack", id: "1", height: "10" },
  { name: "mary", id: "2", height: "20" }
];

console.log(mergeByKey("id", constList, apiList));


Persistent Cookie Store So my app needed a cookie store that persists over the app closing, device reboot etc. I decided to use the sharedPreferences for this. The class works pretty well, in both my tests and on the device. However there are couple of things I think can be improved. First is error handling. The problem is I cannot throw the errors further up in the app as the cookiestore interface doesn't not allow it. Secondly I have made a persistantcookie so I have a concrete class to serialise. I then convert that to the cookie interface. It seems an ugly way to do it. So any pointers would be very much appreciated.

public class BetterPersistantCookieStore implements CookieStore {

    private static final String TAG = BetterPersistantCookieStore.class.getSimpleName();
    private SharedPreferences sharedPreferences;
    private ObjectMapper objectMapper = new ObjectMapper();//TODO pass serializer and deserialiser into constructor
    private static final String COOKIES_KEY = "cookiesKey";

    @Inject
    public BetterPersistantCookieStore(SharedPreferences sharedPreferences) {
        this.sharedPreferences = sharedPreferences;
    }

    @Override
    public void addCookie(Cookie cookie) {
        List<PersistantCookie> cookieList = getPersistantCookies();
        int len = cookieList.size();
        for (int i = 0; i < len; ++i) {
            Cookie tempCookie = cookieList.get(i);
            if (cookie.getName().equals(tempCookie.getName())) {
                cookieList.remove(i);
            }
        }
        cookieList.add(new PersistantCookie(cookie));
        sharedPreferences.edit().putString(COOKIES_KEY, listToString(cookieList)).commit();
    }

    @Override
    public List<Cookie> getCookies() {
            return convertPersistantCookiesToCookies(getPersistantCookies());
    }

    @Override
    public boolean clearExpired(Date date) {
        List<PersistantCookie> cookieList = getPersistantCookies();
        int len = cookieList.size();
        boolean returnValue = false;
        for (int i = 0; i < len; ++i) {
            Cookie cookie = cookieList.get(i);
            if (cookie.isExpired(date)) {
                cookieList.remove(i);
                returnValue = true;
            }
        }
        sharedPreferences.edit().putString(COOKIES_KEY, listToString(cookieList)).commit();
        return returnValue;
    }

    @Override
    public void clear() {
        sharedPreferences.edit().remove(COOKIES_KEY).commit();
    }

    private List<PersistantCookie> getPersistantCookies()  {
            return stringToList(sharedPreferences.getString(COOKIES_KEY, ""));
    }

    private List<PersistantCookie> stringToList(String value) {
        try {
            return objectMapper.readValue(value, TypeFactory.collectionType(ArrayList.class, PersistantCookie.class));
        } catch (IOException e) {
            e.printStackTrace();
            return new ArrayList<PersistantCookie>(0);
        }
    }

    private String listToString(List<PersistantCookie> cookies)  {
        try {
            return objectMapper.writeValueAsString(cookies);
        } catch (IOException e) {
            e.printStackTrace();
            return "";
        }
    }

    private List<Cookie> convertPersistantCookiesToCookies(List<PersistantCookie> persistantCookies) {
        List<Cookie> cookies = new ArrayList<Cookie>();
        for (Cookie cookie : persistantCookies) {
            cookies.add(cookie);
        }
        return cookies;
    }

}

First is error handling. The problem is I cannot throw the errors further up in the app as the cookiestore interface doesn't not allow it.

You can define a custom unchecked exception and throw that.  (Or throw an existing unchecked exception if you can find one that is suitable.)

  Secondly I have made a persistantcookie so I have a concrete class to serialise. I then convert that to the cookie interface. It seems an ugly way to do it.

I don't think you have any sensible alternatives.  (And besides, I disagree that it is "ugly".)

And please correct the spelling of (at least) the public class and method names.  It is like chalk screeching on a blackboard ...


Simple POST function in ClojureScript and Node.js Node's HTTP library provides a handy shorthand function for GET requests. Something similar doesn't exist for POST requests. I'm trying to write one in ClojureScript that compiles to node-readable JavaScript.

In JavaScript (+ underscore.js) it would look like so:

var _ = require("underscore");
var url = require("url");
var http = require("http");

function postData (url_str, data, cb) {
   var parsed_url = url.parse(url_str);
   var options = _.extend(parsed_url, {method: "POST"});
   var req = http.request(options, cb);
   req.write(data);
   req.end();
}

postData("http://myurl:55555", "the data", function(res) {
   console.log(res.statusCode);
});

(Note: error handling should be added + passing the whole parsed_url object, extended by the method is maybe not necessary, but doesn't hurt either).

Now this should be transfered to ClojureScript. Here's an approach which technically works, however I'm asking myself if this is really the best solution:

(def url (node/require "url"))
(def http (node/require "http"))

(defn post-data [url_str data cb]
  (let [parsed_url (.parse url url_str)]
    (aset parsed_url "method" "POST")
    (let [req (.request http parsed_url cb)]
      (doto req
        (.write data)
        (.end)))))

(post-data "http://localhost:55555" "my data" #(println (.-statusCode %1)))

Does anybody have some comments/improvements for this?

Looks great! Some minor suggestions; pull the parsed-url aset into a doto, and that naming the request req isn't very useful. Prefer dash to underscore.

(let [parsed-url (doto (.parse url url_str)
                   (aset "method" "POST"))]
  (doto (.request http parsed-url cb)
    (.write data)
    (.end)))


A fixed max size container class containing unique elements with functions like push_back and pop_back and is iterate-able template <class T, int max_size>
class FixedSizedUniqueStack
{
    std::vector<T> m_vec;
    std::unordered_set<T> m_uset;
public:
    FixedSizedUniqueStack():m_vec(max_size){}
    bool push(T& x)
    {
        bool success = true;
        if(m_uset.insert(x)) m_vec.push_back(x);
        else success = false;
        return success;
    }
    void pop()
    {
        if(m_vec.size() > 0)
        {
            m_uset.erase(m_vec.back());
            m_vec.pop_back();
        }
    }
};

Duplicate value storage:

One disadvantage of your approach is that it stores a duplicate of each value inserted. The cost can become prohibitive for large objects, such as class instances stored by value.

A possible approach to avoid the duplicate would be just doing a linear search over the vector every time a new insertion takes place. For small arrays, this is usually quite fast.

Another possible implementation would be to keep the vector always sorted, then a fast binary search could be performed before insertions to ensure uniqueness.

Proper way of testing set/map insertion:

This code doesn't seem correct:

    bool success = true;
    if(m_uset.insert(x)) m_vec.push_back(x);
    else success = false;
    return success;

The insert method of sets and maps of the Standard Library will return you a pair consisting of an iterator to the inserted element (or to the element that prevented the insertion) and a bool denoting whether the insertion took place (from here).

So the correct way to test if the insertion was successful would be:

if (m_uset.insert(value).second == true)
{
    ...
}

max_size is never checked:

You've stated in the title that the container should have a fixed maximum size, however, you never check to see if the limit size was reached. push_back in a vector will resize the container if its initial capacity is exceeded. Same is true for set::insert.

push() is taking a mutable reference, which can cause confusion:

The signature of bool push(T& x), taking a mutable reference to T, shows the called that this method has the intention or possibility of altering the input parameter. That would be a pretty strange behaviour for a container. When storing some data, you don't want it to be changed by the container. The proper signature here would be to pass the value by const reference:

bool push(const T& x);

That will prevent a copy and will pass a clear message to the caller that the input value is not changed, only stored.

As you polish this interface, you should consider move semantics (C++11), and add a move reference (T&&) overload for push().

empty() is a more idiomatic way of checking for empty containers:

The traditional way of testing if a Standard container is empty is by calling the empty() method, as opposed to if (c.size() > 0).


Recursive right-to-left segmenting (tokenizing) of strings in Python I want to segment a list of strings based on a custom list of words/forms, from right-to-left. Here I've illustrated it with made-up English examples, but the point is to segment/tokenize using a custom list of forms. (I was also wondering whether I could/should use NLTK tokenize for this, but it looked like that might be too complex for this use case).

What would be a good Pythonic way to do this (readable and efficient)? I managed to get something working, but I don't have much experience and I'm curious how this can be improved. The code below uses a recursive function to keep splitting the left element where possible, and a $ anchor in the regular expression to match the morphs only in string-final position.

phrases = ['greenhouse', 'flowering plants', 'ecosystemcollapse']
morphs = ['house', 'flower', 'system', 'collapse']

def segment(text):
    segmented_text = [match for match in re.split('(' + '|'.join(morphs) + ')' + '$', text) if match]
    if len(segmented_text) == 2:
        segmented_morphs.insert(0, segmented_text[1])
        segment(segmented_text[0])
    else:
        segmented_morphs.insert(0, segmented_text[0])

for phrase in phrases:
    segmented_morphs = []
    words = phrase.split()
    for word in reversed(words):
        segment(word)
    print(segmented_morphs)

Result (as desired):

['green', 'house']
['flowering', 'plants'] # here 'flower' should not be segmented
['eco', 'system', 'collapse']

Here are some general comments:

Having a function mutate a global variable is a bad practice. In this case, it may seem easy enough to reason about what is going on, but I guarantee that at some point it will lead to an insidious bug that is hard to track down because you have no idea who is changing the global variable and when.
Recursion is not necessary here; it over-complicates the solution and may make it difficult to change/update the code.

The code itself is rather concise, but the weird data-flow out of the function makes it difficult to understand what's going on. This is complicated because the function is recursive and so you then have to keep track of the calling stack when figuring out how segmented_morphs is being changed.

I don't think the re module is really necessary here, I bet that it is heavier that just searching for strings and doesn't seem to offer any benefit. I've kept it for now, but I might offer another solution without it.

Here's how I would approach the problem:

import re

def segment_word(word, morphs):
    regex = re.compile('(' + '|'.join(morphs) + ')' + '$')
    splitter = lambda s: (regex.split(s) + [None])[:2]

    segments = []
    word, end = splitter(word)
    while end is not None:
        segments.append(end)
        word, end = splitter(word)

    if word:
        segments.append(word)

    return segments[::-1]

def segment_phrase(phrase, morphs):
    return [s for w in phrase.split() for s in segment_word(w, morphs)]

Here's a version without re, I'm not super happy about either. Neither version seems particularly elegant.

def segment_word(word, morphs):
    segments = []
    previous_length = -1
    while len(word) != previous_length:
        previous_length = len(word)
        for morph in morphs:
            if word.endswith(morph):
                segments.append(morph)
                word = word[:-len(morph)]

    if word:
        segments.append(word)

    return segments[::-1]


JPEG extraction script Here is a program that I've wrote to extract JPEGs from a file.  It reads a file that contains the image data and separates it into individual images.

import hashlib

inputfile = 'data.txt'
marker = chr(0xFF)+chr(0xD8)

# Input data
imagedump = file(inputfile, "rb").read()

imagedump = imagedump.split(marker)

count=0
for photo in imagedump:
    name = hashlib.sha256(photo).hexdigest()[0:16]+".jpg"
    file(name, "wb").write(marker+photo)
    count=count+1
    print count

The script names the identified images with their SHA256 digest, and all of the photos that it finds will be dumped into the current directory.

Here's how I test the script to see if it is working correctly:

Type cd ~/images/
create the folder mkdir test
dump some JPEGs into a singe file in the directory cat *.jpg > ./test/data.txt
cd test and put the script into the current directory
run the script python extract.py, and the JPEGs will be dumped into the current folder

How can I improve my script's performance?  Are there any problems with its operation?  The script does not know when an image ends; just when a new one starts.  Could this cause problems?

There is an End-of-data marker FF D9, but you can't scan for it blindly, because those bytes can also appear within a JPEG image. For example, if the JPEG contains a thumbnail, then FF D9 could mark the end of the thumbnail rather than of the whole image.  In fact, the FF D8 start-of-image marker can also appear within a JPEG image for the same reason.  Therefore, your technique is invalid.

To do a proper job, you must look for JPEG markers, most of which are followed by two bytes indicating the payload size, and advance the indicated number of bytes, until you hit the FF D9 marker.  It might even be faster, since you can advance in chunks rather than scanning every byte sequentially.


Reduce similar methods via pattern? I've put together a simple application in order for me to learn sqlite and OOP in PHP. The application basically outputs two random words, e.g. "Angry Badger", from two database tables, one called Animals & the other named Adjectives. At present both tables contain the same columns (but could change in future).

The code uses an interface, iWord with two methods to be implemented by their concrete classes (Animal, Adjective). The database object has been passed in on the constructors so I can create some unit tests later.

If I were to introduce a new class, fruit, with the same methods: getRandomItem() & listRandomItems($numerOfItems) I would need to create the another DB table, class, implement the same DB error handling and processing of data. This doesn't feel correct.

An alternative may be to combine the database tables into one and have one class that I pass in an item to lookup, e.g. animal, fruit, adjective, & dynamically build the SQL.

Any recommendations or patterns that I should be following?

Please note: this is test code for learning & not for production use ;)

<?php

interface iWord
{
    public function getRandomItem();
    public function listRandomItems($numerOfItems);
}

class Animal implements iWord
{
    private $database;

    public function _construct(PDO $pdo)
    {
        $this->database = $pdo;
    }

    public function getRandomItem()
    {
        $rAnimal = NULL;
        try
        {
            $query = $this->database->prepare("SELECT animalName FROM animal ORDER BY RANDOM() LIMIT 1;");
            $query->execute();
            $sqlres1 = $query->fetch(PDO::FETCH_OBJ);
            $rAnimal = $sqlres1->animalName;
        }
        catch (PDOException $ex)
        {
            echo $ex->getMessage();
        }
        return $rAnimal;
    }

    public function listRandomItems($numerOfItems)
    {
        // not yet implemented
        throw new Exception('Not Implemented');
    }
}

class Adjective implements iWord
{
    private $database;

    public function _construct(PDO $pdo)
    {
        $this->database = $pdo;
    }

    public function getRandomItem()
    {
         // exception handling required
         $query = $this->database->prepare("SELECT adjective FROM adjectives ORDER BY RANDOM() LIMIT 1;");
         $query->execute();
         $sqlres2 = $query->fetch(PDO::FETCH_OBJ);
         return $sqlres2->adjective;
     }

    public function listRandomItems($numerOfItems)
    {
        throw new Exception('Not Implemented');
    }
}

try {
    $pdo = new PDO("sqlite:c:/htdocs/mydb.sqlite");
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e) {
    echo $e->getMessage();
}

$adj = new Adjective($pdo);
$rAdjective = $adj->getRandomItem();
$anm = new Animal($pdo);
$rAnimal = $anm->getRandomItem();

echo $rAdjective . " " . $rAnimal;

unset($pdo);

Extract an abstract base class AbstractWordTable that implements the better-named interface WordTable by moving all code that's duplicated between Adjective and Animal (all but the table and column names). This is a major step in the DRY direction that will simplify your code.

I won't attempt to produce this code on my phone, but the key is for those concrete implementations to contain only unique code. The end result will be with a simple one-line constructor.

Another improvement would be to decouple reading the table contents from picking random words by splitting these functions between WordTable that reads words from a database table into an array and WordList that provides various ways to consume it, including choosing random words and subsets.


Declaring multiple arrays in PHP with one line Is it possible to declare multiple arrays in PHP in one line?

$userAnswers  = array();
$questionIDs = array ();
$sqlAnswers = array();
$sqlAnswersQuery = array();
$correctAnswers = array();

Any cleaner ways of doing this?

NOTE: The contents of these arrays are all DIFFERENT. I don't think setting them equal to each other would work.

This is good as it is.
It's recommended to have one statement per line, for better readability:
code is easier to read when the logic flows from top to bottom,
with no distractions sideways.
So even if there was a way to do this on one line, you really shouldn't.


Redirecting all pages under a certain directory to a single file I want to redirect all pages under a certain directory to a single file, like so:

/dir/a → /dir/new
/dir/b →  /dir/new
/dir/new →  /dir/new

With the straightforward

RedirectMatch 301 ^/dir/ http://example.com/dir/new

I get a redirect loop, since /dir/new redirects to itself. I replaced it with a regular expression

RedirectMatch 301 ^/dir/($|[^n]|n$|n[^e]|ne$|ne[^w]|new.) http://example.com/dir/new

which matches all URLs except the desired one. This works, but it's obviously the 'wrong' way to do it. What's a better way?

It seems you're looking for a negative lookahead:
path starting with /dir/ and not followed by new$:

RedirectMatch 301 ^/dir/(?!new$) http://example.com/dir/new


Reddit bot to gather listening suggestions import praw
import time
import database

# Create a user agent and log into Reddit
UA = 'OZBOZZ v0.1'
r = praw.Reddit(UA)
# Log UA into reddit with my personal user name and password
r.login('', '', disable_warning='True')
subreddit = r.get_subreddit('listentothis')
current_time = int(time.time())
user = ''
submission_list = []
results = []
count = 0
genres = ["Big Band Funk", "progressive Rock", "rock", "progressive metal"]

def botstuff():
# while condition is true...
while True:
    # Print Working so That I know the Next iteration has begun
    print("Working...")

    # For submission in subreddit /r/listen to this get up to 20 new posts
    for submission in subreddit.get_new(limit=20):
        # Create a variable called Sub_age that is current time minus the submission creation time divided by 60
        # divided by 60  divided by 12
        sub_age = (current_time - submission.created_utc) / 60 / 60 / 24
        # If the submission age is less than one (posted in last 24 hours)...
        if sub_age < 1:
            # Add that submission to the submission list. Each entry contains the title and Link to post
            submission_list.append([submission.title, submission.permalink])

    # Print the submission list, so I know I got something. Debugging
    print(submission_list)

    # For each list entry in submission_list
    for x in submission_list:
        # any of element in the list genre are found in the 1st... element property(?)
        if any(a in x[1] for a in genres):
            # Append the element with matching genre tags in to the results list.
            results.append(x)

    # Print the submission list, so I know I got something. Debugging
    print(results)

    # if the results list is empty
    if not results:
        # Send the user a message  letting them know I havent found something
        r.send_message(user, 'Nothing Yet', 'Hey, I didn\'t find anything')
    # Otherwise...
    else:
        # Create a message variable from the bot with the list of results.
        msg = 'Hey, Check these out.  -oz \n' + "\n" +'[%s]'.strip('[]') % ''.join(map(str, results)).strip('[]')
        # Sent the message to the user.
        r.send_message(user, 'Suggestions', msg)
        print("Message Sent")

    count = count + 1
    print("Sleep cycle " + str(count))
    #wait 60 minutes
    time.sleep(3600)

So this is the code completely working code so far. What this bot accomplishes is scanning new posts on /r/listentothis that are less than 24 hours old and parsing the text of the submission post that matches my list of genres called genres. If a match is found, it appends it to a results list. If the results list is empty it sends me a message stating that it didn't find anything, otherwise, it sends me a message with a suggestsion to check out some results and then prints out the results list.

I'm looking for ways to multithread this bot.
Make this a modular function because the goal is to make it so that a reddit user can send this bot account a message with genres and it will put the uname and genres into a database and do a search for them every hour.
Clean up results output so each result is printed on a new line.
Improve the string search efficiency if possible.
Search Specifically for only the strings providied. So for example, "rock" will not return "Glam rock"

1. General issues

r.login ('','', disable_warning=True) #notice True is boolean and not string

if you did disable_warning='False' it would still disable warning. Every non-empty string in python when cast to boolean will return True.

Too many global objects. Reddit API client can stay global r, having current_time as global is horrific. Stuff like submission_list results count genres shouldn't be global either but I can understand that your intent was to get those things from database so this is only temporary. All of those things should be returned by their functions at the moment when they are needed.

You shouldn't use login. It is being depreciated soon. What it means is that if not for the fact that praw devs don't want to break every single bot there is they give you a little time to remove login() from old bots. New bots shouldn't use this at all, because by the time you have it up and working it might be broken again cause login() will no longer be there. Check OAuth2 - it's bit more that single call to login() but still can be done in 2 minutes (and it's one time).

2. Multithreading

So there are multiple issues with multithreading here. First off you don't need it here. Your bot has only one functionality. Grab data, parse data, give output, repeat. Second one is that multiprocessing/multithreading in python is generally not too common unless there is actual need for that due to massive overhead, that usually makes things slower in the long run. So it would only be feasible if you want your program to extremely long stuff in parallel.

Lastly reddit bot is extremely bad example to try to multithread. The reason are reddit API restrictions. It will not respond to request more often than 0.5 sec. This is how developers have set it and also praw behaves by it. If you make 20 requests you will have to wait 10 seconds no matter how many threads try to do this. This is why it is always done in sequences with those.

3. Modularity

First off your program should have more functions, just for the sake of readability. Like

def get_submission_list():
    submission_list = []
    for submission in sub.....
    ...
    return submission_list

def send_messsages(results):
   .....

def main_loop():
    while True:
        check_inbox_for_new_bot_subscribers()
        s_list = get_submission_list()
        subscribed_users = get_subscribed_users_and_genres_from_db()
        results = prepare_results(subscribed_users,s_list)
        send_messages(results)

submission_list is kinda obvious, subscribed_users could be a list of tuples or a dictionary with 'username':'genres,separated,by comma' or w/e you feel comfortable working with. results in this case would be (username,[list of links to send to this user])

For the sake of checking multiple users it would be beneficial to also reverse username-genres dictionary, so that we also have fast lookup dictionary of 'genre':[list of usernames subscribed to this genre]'. This would be fun to implement, but benefit is obvious since we have now \$\mathcal{O}(n\cdot{}m)\$ time instead of \$\mathcal{O}(n^2\cdot{}m^2)\$.

Simple database model for this would be:

class BotSubscriber(Model):
    redditor_id = CharField()
    genres = CharField()

You obviously could save other stuff like time of subscription, how many songs you recommended to this user and w/e you feel like.

4. Clean out output

I believe '\n'.join(map(str,results)).strip('[]') or some variation of that would work as you want?

5. String search efficiency and better matching

It would be beneficial to parse submission.title before searching. Band - song [genre] (optional year) is pretty much standard format. Move Band - Song and save it as title and [genre] as genre (stripping starting braces as well).

On top of that you definitely use genre.lower() as well as save submitted user genres as lowercase. No need to be case sensitive in this scenario.

Once you have your genre saved as genre you can do any([1 for a in user.genres if a==genre.lower()]) or use reversed dictionary idea that I've posted in 3.


Calculate admission prices of different days Children are admitted free on Sundays, and pay only $6 each, any other day
Adults admission is $12 any day, except on Tuesdays, when it is only $8
Monday, Wednesday, Thursday, Friday, Saturday are same prices.
How do I shorten Mon/Wed-Sat so the code isn't repeated and can be single-line?
Sunday and Tuesday prices vary.

#include <iostream>
#include <iomanip>
using namespace std;

int main()
{
int Children, Adults, tickets, persons, pricea, pricec, total_price;
char day;

cout << "Enter the number of Adults attending? ";
cin >> Adults;
cout << "Enter the number of Children attending? ";
cin >> Children;
cout << "What is the day of the week? ";
cin >> day;

cout << "The day entered is: " << day << endl;
switch (day)
{
case 'm': case 'M':
    cout << "Monday" << endl;
    pricea = 12;
    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 't': case 'T':
    cout << "Tuesday" << endl;
    pricea = 8;
    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 'w': case 'W':
    cout << "Wednesday" << endl;
    pricea = 12;
    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 'r': case 'R':
    cout << "Thursday" << endl;

    pricea = 12;

    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 'f': case 'F':
    cout << "Friday" << endl;

    pricea = 12;

    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 's': case 'S':
    cout << "Saturday" << endl;

    pricea = 12;

    pricec = 6;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
case 'u': case 'U':
    cout << "Sunday" << endl;
    pricea = 12;
    pricec = 0;
    total_price = (Adults*pricea) + (Children*pricec);
    cout << "Total price is: " << total_price << endl;
    break;
default:
    cout << "Not a valid day" << endl;
}
system("pause");
return 0;
}

Several things that come to mind:

Don't use using namespace std; as it leads to namespace pollution (e.g. name collision)
Don't use std::endl unless you need to flush the buffer
As you already noticed there is a lot of duplicate code and just like DRY states we don't want to repeat ourselves

Going for a simple rewrite while keeping mostly to your original code we could do the following:

#include <iostream>

int main()
{
    unsigned int kids, adults;
    char day;

    std::cout << "Enter the number of Adults attending? ";
    std::cin >> adults;

    std::cout << "Enter the number of Children attending? ";
    std::cin >> kids;

    std::cout << "What is the day of the week? ";
    std::cin >> day;
    std::cout << "The day entered is: " << day << "\n";

    switch (day)
    {
        case 'u': case 'U':
            std::cout << "Total price is: " << adults * 12 << "\n";
            break;

        case 't': case 'T':
            std::cout << "Total price is: " << adults * 8 + kids * 6 << "\n";
            break;

        case 'm': case 'M':
        case 'w': case 'W':
        case 'r': case 'R':
        case 'f': case 'F':
        case 's': case 'S':
            std::cout << "Total price is: " << adults * 12 + kids * 6 << "\n";
            break;

        default:
            std::cout << "Not a valid day" << "\n";
    }
}

This makes use of Fall through cases in the switch construct thus simplifying your code without overengineering it.


Arduino Code for Juggling Machine (Stepper Motors) I have built a machine that juggles. An Arduino sends signals to stepper motor drivers that drive NEMA 23 size stepper motors.

Video of the machine: https://youtu.be/-9zD_8erkck
Currently, each arm makes one revolution and then pauses while the other arm makes one revolution. One arm starts before the other arm comes to a complete stop, the amount of overlap is captured by the variable stepover.
I would like to make the machine more consistent and run quieter with less vibration. How can I make it run smoother? Is there a way to do this where the hands will never come to a complete stop?
The stepper motors are running at 800 steps per revolution. I have tried to using more steps/rev for smoother operation, but the accelstepper library will not run the motors fast enough.
I have tried altering the values max_acceleration, max_speed, and stepover. I have not found a way to optimize these values.
#include <AccelStepper.h>
AccelStepper stepper1(AccelStepper::DRIVER, 9, 8);
AccelStepper stepper2(AccelStepper::DRIVER, 4, 3);

int start = 0;
int side = 0;
int max_speed = 6000;
int max_acceleration = 12000;
int stepover = 300;
int step_distance = 800;

void setup()
{
  stepper1.setMaxSpeed(max_speed);
  stepper2.setMaxSpeed(max_speed);
  stepper1.setAcceleration(1234);
  stepper2.setAcceleration(1234);
  stepper1.moveTo(600);
  stepper2.moveTo(-200);
}

void loop()
{
  // This code gets the arms in the right place before startup
  if (start == 0) {
    while (abs(stepper1.distanceToGo()) != 0) {
      stepper1.run();
    }
    while (abs(stepper2.distanceToGo()) != 0) {
      stepper2.run();
    }
    start = 1;
    stepper1.setAcceleration(max_acceleration);
    stepper2.setAcceleration(max_acceleration);
    delay(500);
  }

  // This code moves the left arm
  if (abs(stepper1.distanceToGo()) == 0
      and abs(stepper2.distanceToGo()) < stepover
      and side == 0) {
    stepper1.moveTo(step_distance + stepper1.currentPosition());
    side = 1;
  }

  // This code moves the right arm
  if (abs(stepper2.distanceToGo()) == 0
      and abs(stepper1.distanceToGo()) < stepover
      and side == 1) {
    stepper2.setCurrentPosition(0);
    stepper2.moveTo(step_distance);
    side = 0;
  }

  if (start == 1) {
    stepper2.run();
    stepper1.run();
  }
}

Inside loop, you're checking if some code should be run before startup. You could simply move the corresponding code to setup, and be sure it's only run once:
#include <AccelStepper.h>
AccelStepper stepper1(AccelStepper::DRIVER, 9, 8);
AccelStepper stepper2(AccelStepper::DRIVER, 4, 3);

int side = 0;
int max_speed = 6000;
int max_acceleration = 12000;
int stepover = 300;
int step_distance = 800;

void setup() {
  stepper1.setMaxSpeed(max_speed);
  stepper2.setMaxSpeed(max_speed);
  stepper1.setAcceleration(1234);
  stepper2.setAcceleration(1234);
  stepper1.moveTo(600);
  stepper2.moveTo(-200);

  while (abs(stepper1.distanceToGo()) > 0) {
    stepper1.run();
  }
  while (abs(stepper2.distanceToGo()) > 0) {
    stepper2.run();
  }
  stepper1.setAcceleration(max_acceleration);
  stepper2.setAcceleration(max_acceleration);
  delay(500);
}

void loop() {
  // This code moves the left arm
  if (abs(stepper1.distanceToGo()) == 0 and abs(stepper2.distanceToGo()) < stepover and side == 0) {
    stepper1.moveTo(step_distance + stepper1.currentPosition());
    side = 1;
  }

  // This code moves the right arm
  if (abs(stepper2.distanceToGo()) == 0 and abs(stepper1.distanceToGo()) < stepover and side == 1) {
    stepper2.setCurrentPosition(0);
    stepper2.moveTo(step_distance);
    side = 0;
  }

  stepper2.run();
  stepper1.run();
}

abs cannot be negative, so it might be clearer to replace abs(...) != 0 by abs(...) > 0
AccelStepper::move(long relative) is defined as moveTo(_currentPos + relative); so you can probably replace your moveTo by move.


Calculate 6 business days I wrote a little class that will calculate 6 business days from today. The reason it's in a class is because it will be used as a part of a bigger program.

I would like a way to refactor this class and make it a little more object oriented along with a little prettier.

from datetime import datetime, timedelta

class BusinessDay(object):
    """
    Calculate a business day of 6 days skipping Saturday and Sunday
    """
    return_date = []  # Array to store the calculated day

    def __init__(self, start_day):
        """
        :type start_day: Object, datetime object
        """
        self.start_day = start_day

    def calc_bus_day(self):
        """Calculate 6 business days from the self.start_day example:
        >>> import BusinessDay
        >>> test = BusinessDay(datetime(2016, 9, 14))
        >>> test.calc_bus_day()
        >>> #2016-09-22
        """
        days_skipped = self.start_day + timedelta(8)

        if datetime.isoweekday(days_skipped) in range(2, 6):
            print("Date {} is a weekday.".format(self.start_day))
            self.return_date = days_skipped
            print("Six days from today will be {}".format(self.return_date))
        else:
            print("Date {} is a weekend.".format(self.start_day))
            self.return_date = days_skipped + timedelta(2)
            print("Six days from the weekend will be {}".format(self.return_date.strftime("%m-%d-%Y")))

        if self.return_date.isoweekday() is 1 or 7:
            if self.return_date == 1:
                self.return_date = self.return_date + timedelta(2)
                return self.return_date - timedelta(2)
            else:
                self.return_date = self.return_date + timedelta(1)
                return self.return_date - timedelta(2)

Bug

if self.return_date.isoweekday() is 1 or 7:

Is always true because 7 is true, adding parenthesis will help you understand:

 if  ( self.return_date.isoweekday() is 1 ) or 7:

To get proper behaviour use:

 if self.return_date.isoweekday() in (1, 7):

Repetition

self.return_date = self.return_date + timedelta(2)
return self.return_date - timedelta(2)

And:

self.return_date = self.return_date + timedelta(1)
return self.return_date - timedelta(2)

Use a if/ternary for the first timedelta arguments and flatten these lines.

No class

You have a single function with a single argument, simplify and avoid a class. (You can re-use your function without a class, just put it in a separate module and import it.)


Graphs processing in Scala I've just started to learn Scala and decided to implement small graph library to train.
Here is the basic version, that describes graph and nodes structure and also provides implementation of DFS to find some path from one node to other.

class Graph[A] {

  var nodes = Set.empty[Node[A]]

  def addNode(n: Node[A]) = nodes = nodes + n

  def DFS(from: Node[A], to: Node[A]): List[Node[A]] = {
    var visited = Set.empty[Node[A]]
    @tailrec
    def _DFS(from: Node[A], to: Node[A], path: List[Node[A]]): List[Node[A]] = {
      visited = visited + from
      if (from.equals(to)) path
      else {
        from.outgoing.find(n => !visited.contains(n)) match {
          case Some(n) => _DFS(n, to, n :: path)
          case None => path match {
            case n :: Nil => List.empty[Node[A]]
            case _ :: n :: ns => _DFS(n, to, n :: ns)
          }
        }
      }
    }
    _DFS(from, to, List(from)).reverse
  }

}

class Node[A](val i: A) {
  var incoming = Set.empty[Node[A]]
  var outgoing = Set.empty[Node[A]]

  def addIncoming(n: Node[A]) = incoming = incoming + n
  def addOutgoing(n: Node[A]) = outgoing = outgoing + n
}

I'm actively writing code in Java, so my primary goal is to not write Java code in Scala syntax. Hence, first and main question is: does my code looks like typical Scala code? If not, what should I change?
Next thing bothering me, is that my Node is too loosely coupled with Graph. I could invoke DFS only on instance of Graph, but apart from this, I don't really use Graph abstraction: I could just create instances of Node and link them.
That leads me to another problem. Suppose now I want to define weighted graph. For Graph itself I could define Weighted trait, but then I need to replace inner Node with NodeWithWeightedEdges, and I don't understand how.

One of the important traits of Scala is that it very well supports the functional programming, and of the advantages of functional programming is that is makes the reasoning of the functions easier. This is easier if the functions do not have side effects and if the used data structures are immutable.

Immutable data structures and pure functions are not always the best option, but in a graph library I thing they might be suitable.

So in your code I'd change the Graph to be an immutable class (see sample below).

Another problem with your code you have notices yourself. It is the strange coupling of the graph and the node classes. A graph is a structure that contains the nodes and the edges. In your code however the graph does not have any information about the edges itself, the nodes are responsible for holding the information about the edges. This could become problematic if you add only the node a to the graph and the node a has edges to other nodes, not known to the graph. Also, with the current design, you cannot reuse the same node object in multiple graphs. Also in your code there is no guarantee that when the node a has an outgoing edge to b that there is a corresponding edge on b incoming from a.

Here a sample of immutable classes for the Node and the Graph. Note that this structure is not optimal at all, as the information about the incoming and outgoing edges is not stored efficiently, but this can be done easily with some maps, depending on the algorithms you want to develop. Another note: The graph in my sample stores the nodes and the edges a bit redundantly. It would be enough to store just the edges, if isolated nodes are not needed.

case class Node[+T](i: T)

class Graph[N <: Node[T], +T](someNodes:Set[N], val edges:Set[(N,N)]) {

  // someNodes does not necessarily contains all nodes from the edges, nodes does
  val nodes: Set[N] = someNodes ++ edges.map(_._1) ++ edges.map(_._2)

  def addVortex(node:N) = new Graph(nodes + node, edges)

  def addEdge(edge: (N, N)) = new Graph(nodes + edge._1 + edge._2, edges + edge)

  // Redundant, but more efficient data structure for the outgoing edges
  lazy val outgoingEdges: Map[N, Set[N]] = edges.groupBy(_._1).mapValues(s => s.map(_._2))

  // Here come your algorithms
}


Wrap string at certain delimiters The method receives an unformatted string and it needs to put line breaks after ';' or before 'i)' and 'i.', where i - integer.
If received a string like '; 1)', then it should add only one line breaker after ';' and before '1)'.

// This is ugly - needs to be refined and refactored
private static String format(String unformattedStr) {
    Pattern p1 = Pattern.compile("(\\;\\s*)");
    Matcher matcher1 = p1.matcher(unformattedStr);

    String formattedStr1;
    if (matcher1.find()) {
        formattedV1 = matcher1.replaceAll("$1\\\\n");
    } else {
        formattedV1 = unformattedStr;
    }

    Pattern p2 = Pattern.compile("(\\d+\\)|\\d+\\.)");
    Matcher matcher2 = p2.matcher(formattedV1);
    String formattedV2;

    if (matcher2.find()) {
        formattedV2 = matcher2.replaceAll("\\\\n$1");
    }
    else {
        formattedV2 = formattedV1;
    }

    String formattedV3 = formattedV2.replaceAll("\\\\n\\s*(\\\\n)+", "\\\\n");
    return formattedV3;
}

It looks like you have three distinct cases, two of which can be covered with one regex as @EmilyL pointed out already.

So unless I am missing something can't you just do the following?

private static String format(String unformattedStr) {
    return unformattedStr.replaceAll("(;?\\s)(\\d+[).])", "$1\\\\n$2");
}


Multiple independent or layered constructors for TicTacToe? At first it was like this:

public abstract class TicTacToeViewAbstract implements TicTacToeView {
    private final List<OnCellClickListener> onCellClickListeners;
    private boolean movesBlocked;
    private boolean gameFinished;
    private TicTacToeModel model;

    protected TicTacToeViewAbstract(TicTacToeModel model) {
        onCellClickListeners = new ArrayList<OnCellClickListener>();
        gameFinished = false;
        movesBlocked = false;
        plugModel(model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model, TicTacToeViewAbstract toRestore) {
        this.onCellClickListeners = toRestore.onCellClickListeners;
        this.gameFinished = toRestore.gameFinished;
        this.movesBlocked = toRestore.movesBlocked;
        this.plugModel(model);
        toRestore.unplugModel();
    }
    // other methods ...
}

Then I tried to remove duplication in both constructors:

public abstract class TicTacToeViewAbstract implements TicTacToeView {
    private final List<OnCellClickListener> onCellClickListeners;
    private boolean gameFinished;
    private boolean movesBlocked;
    private TicTacToeModel model;

    private TicTacToeViewAbstract(List<OnCellClickListener> onCellClickListeners,
                    boolean gameFinished, boolean movesBlocked, TicTacToeModel model) {
        this.onCellClickListeners = onCellClickListeners;
        this.gameFinished = gameFinished;
        this.movesBlocked = movesBlocked;
        this.plugModel(model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model) {
        this(new ArrayList<OnCellClickListener>(),
             false, false, model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model, TicTacToeViewAbstract toRestore) {
        this(toRestore.onCellClickListeners,
             toRestore.gameFinished, toRestore.movesBlocked, model);
        toRestore.unplugModel();
    }
    // ...
}

Which one looks better, and why?

I would vote for the second, i.e. the one with a basic constructor called by others.

But what is important is how to decide. The criteria that come to mind are:

what if you should refactor later?
which is the most clear when using this class?

In your case, you want to offer different constructors for different usages, with default values in some case.

There are two types of refactoring impacting a constructor:

fields change
usage changes

Refactoring fields

If the fields change, then you do not have to update 2 (or more) constructors by hand. You first update the lowest level constructor. You can handle the default setting of this field once. If you use an argument and forget to update other constructors, the compiler will let you know.

While if you picked the first option, you will notice you forgot to update all constructors at execution (or test if you have good tests) time.

Refactoring usage

If the usage change, you want to add a new constructor with different arguments. Then you will call another constructor, and all fields will be set since your constructors were correct at first implementation.

Again, if you picked the first option, you would have to set the fields one by one, and there are chances you would not set a field correctly, the compiler would not help you on that.

Clarity

Now let's consider how clear it looks to a user. If you set correctly the visibility (as you did), no option is particularly preferable. Some javadoc would help though, so that the user does not need to open your file. But when reading your file, unless the javadoc is clear, the second option is more confusing, as one should read the whole code to figure what best to use, and what each constructor does, jumping to another method, ...

There might be other criteria that I missed. But from a basic check, the second option seems easier to maintain and be clear enough if some documentation is added. But based on those two criteria, you might conclude differently (you might find clarity more important than later refactoring)


Summing the first 1 to n powers of 2 At a JavaScript test for a job, I was asked the question below. I believe there has to be a better way to accomplish the same result, but I couldn't find any. If some JavaScript geek would take a look and suggest a better approach I would really appreciate it:

  Given a positive integer greater or equal to 1, write a function f(n) to calculate as efficiently as possible the sum of the first 1 to n powers of 2. Do no use any buit-in function to calculate the power of a number.
  Example: if n = 3:

  $$ f(3)=2^1+2^2+2^3=2+4+8=14 $$

jsFiddle

function solution (n) {
    var t='';
    var q='';
    for (var i=1;i<=n;i++) {
        q='';
        for (var j=1;j<=i;j++) {
            q=q+'2*';
        }
        q=q+1;
        t=t+'('+q+')+';
    }
    t=t.substr(0,t.length-1);
    return eval(t);
}

EVAL, WHY!?

I really can't understand why you're using String concatenation and eval to calculate the result.

The most efficient way is to calculate \$2^{n+1}-2\$.

Additionally, your variable names are all one character long, and your code lacks spacing.

How can you tell what is t and what is q? Your variable names make no sense.

function solution (n) {
    var result = 2;
    for (var i = 1; i <= n; i++) {
        result *= 2;
    }
    return result - 2;
}

Another solution is to use bit shifting: (only works if \$n <= 29\$)

function solution(n) {
    return (2 << n) - 2;
}


Minimum difference between numbers in a file I'm learning Java and I created a simple solution for the following problem:

  Given a file [filename].txt such that each line has the format string, double, find the minimum possible difference between any two values and identify if it is less than an arbitrary value mindDist

package com.company;

import java.io.*;
import java.util.*;

public class Main {
    public static void main(String[] args) { try {
        Scanner sc = new Scanner(System.in);
        System.out.print("Please enter the name of the input file: ");
        Scanner inFile = new Scanner(new File(sc.nextLine()));
        sc.close();

        List<Double> l = new ArrayList<>();
        while (inFile.hasNextLine()) {
            String[] parts = inFile.nextLine().split(",");
            l.add(Double.parseDouble(parts[1]));
        }

        Collections.sort(l);
        double minDist = 5;
        double diff;
        double minDiff = Double.MAX_VALUE;

        for (int i = 1; i < l.size(); i++) {
            diff = l.get(i) - l.get(i - 1);
            if (diff < minDiff) minDiff = diff;
        }

        if (minDiff < minDist) System.out.println("The satellites are not in safe orbits.");
        else System.out.println("The satellites are in safe orbits.");
        if (l.size()!=1) System.out.println("The minimum distance between orbits (km): " + minDiff);
        inFile.close();

        } catch (NumberFormatException | FileNotFoundException | ArrayIndexOutOfBoundsException e) {
            System.err.println(e);
        }
    }
}

I am looking on advice on if there are any other possible errors / exceptions I may have missed, in addition to any way of increasing the efficiency / simplicity of my code.

You need to make sure you always close resources that can be closed. That means either a try-with-resources block or a try-finally block. You can’t just leave close outside one of these constructs because if an exception gets thrown the close method might not get called.
Arguably, parsing the distances is easier to read using the stream API. You might not yet have learned that part of the language yet, in which case there’s nothing wrong with your loop except that inFile is never closed. Oh, no, it is, but waaaay too late. Keep the scope of the scanner as tight as possible.
Likewise, the code for finding the minimum distance can be written as a stream. Credit to @AnkitSont.
Variable names are really, really important. They should describe the information being pointed to. l is a terrible variable name because it doesn’t tell us anything. list would be a slight improvement, but distances would be much better.
Along the same lines, there’s no reason to cut short variable name lengths. sc instead of scanner doesn’t save you anything, and it makes it harder on the reader because they have to go dig up what an sc is. minDist, diff and minDiffcan and should all be expanded out, and minDist might be better named safeDistance.
Declare variables where they’re first used, and limit their scope as much as possible. This reduces the cognitive load on the reader. minDist can be declared right before your output. It could arguably also be a constant in your class (private static final double declared at the class level). diff can be declared inside the for loop.
Even though the language allows curly braces to be optional for one-line blocks, it is widely regarded best practice that you always include curly braces. To do otherwise is inviting problems later when your code is modified.
Be careful with conditional checks. All of your code will work correctly if there are zero distances in the file until you hit if (l.size() != 1). You really want to check if (l.size() > 1), right?
Your catch block isn’t really doing anything. Uncaught exceptions would effectively be handled the same way - execution would terminate and the stack trace would be logged to System.err.
It's nice to use final when you can to indicate that a variable isn't going to be reassigned. This also reduces the cognitive load of the reader.

If you were to apply all these changes to your code, it might look something like:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Scanner;
import java.util.stream.IntStream;

public final class Main {

    public static void main(final String[] args)
            throws IOException {

        final String filename;
        System.out.print("Please enter the name of the input file: ");
        try (final Scanner scanner = new Scanner(System.in)) {
            filename = scanner.nextLine();
        }

        final double[] distances =
                Files
                    .lines(Paths.get(filename))
                    .mapToDouble((line) -> Double.parseDouble(line.split(",")[1]))
                    .sorted()
                    .toArray();

        final double minimumDifference =
                IntStream
                    .range(1, distances.length)
                    .mapToDouble(i -> distances[i] - distances[i - 1])
                    .min()
                    .getAsDouble();

        final double safeDistance = 5;
        if (minimumDifference < safeDistance) {
            System.out.println("The satellites are not in safe orbits.");
        } else {
            System.out.println("The satellites are in safe orbits.");
        }

        if (distances.length > 1) {
            System.out.println("The minimum distance between orbits (km): " + minimumDifference);
        }
    }

}


Strongly connected components algorithm In my Python application, I am using Tarjan's algorithm to compute strongly connected components. Unfortunately, it shows up under profiling as one of the top functions in my application (at least under cPython, I haven't figured out how to profile under Pypy yet).

I've looked at the code, but I don't see any obvious performance mistakes, and I can't figure out how to make it faster. Any suggestions?

Note: The profiler reports a lot of time spent inside the function, so it's not just a matter of slow callbacks.

def tarjanSCC(roots, getChildren):
    """Return a list of strongly connected components in a graph. If getParents is passed instead of getChildren, the result will be topologically sorted.

    roots - list of root nodes to search from
    getChildren - function which returns children of a given node
    """

    sccs = []
    indexCounter = itertools.count()
    index = {}
    lowlink = {}
    removed = set()
    subtree = []

    #Use iterative version to avoid stack limits for large datasets
    stack = [(node, 0) for node in roots]
    while stack:
        current, state = stack.pop()
        if state == 0: #before recursing
            if current not in index: #if it's in index, it was already visited (possibly earlier on the current search stack)
                lowlink[current] = index[current] = next(indexCounter)
                children = [child for child in getChildren(current) if child not in removed]
                subtree.append(current)

                stack.append((current, 1))
                stack.extend((child, 0) for child in children)
        else: #after recursing
            children = [child for child in getChildren(current) if child not in removed]
            for child in children:
                if index[child] <= index[current]: #backedge (or selfedge)
                    lowlink[current] = min(lowlink[current], index[child])
                else:
                    lowlink[current] = min(lowlink[current], lowlink[child])
                assert(lowlink[current] <= index[current])

            if index[current] == lowlink[current]:
                scc = []
                while not scc or scc[-1] != current:
                    scc.append(subtree.pop())

                sccs.append(tuple(scc))
                removed.update(scc)
    return sccs

You iterate twice over the children in both branches. Avoid that by combining these lines

children = [child for child in getChildren(current) if child not in removed]
stack.extend((child, 0) for child in children)

to

stack.extend((child, 0) for child in getChildren(current) if child not in removed)

and these

children = [child for child in getChildren(current) if child not in removed]
for child in children:

to

for child in getChildren(current):
    if child not in removed:

Instead of looping in Python and moving one item at a time here

    scc = []
    while not scc or scc[-1] != current:
        scc.append(subtree.pop())
    sccs.append(tuple(scc))

try to use slicing like this. A possible disadvantage is that index searches from the beginning.

    i = subtree.index(current)
    scc = tuple(reversed(subtree[i:]))
    del subtree[i:]
    sccs.append(scc)

Use local variables to reduce dictionary lookups. You can also bind methods such as stack.pop to local variables to avoid attribute lookup inside the loop.
Also to reduce dictionary lookups, you could combine index and lowlink dictionaries. Put the pair of values in as a tuple or a list to take advantage of unpacking into local variables:

next_index = next(indexCounter)
index[current] = [next_index, next_index]
...
index_current, lowlink_current = index[current]


Exposing configuration elements to program I have a config file stored in a java properties file:

output_path=/some/path/somewhere
num_threads=42
# ad infinitum

I have an singleton enum which controls the building of the config:

public enum ConfigFactory {
    INSTANCE;

    public static final String OUTPUT_PATH = "output_path";
    public static final String NUM_THREADS = "num_threads";

    private Map<String, ConfigParameter> mSettings = new HashMap<String, ConfigParameter>();

    public void build(String path) {
        InputStream inputStream = getClass().getResourceAsStream(path);
        Properties properties = new Properties();
        try {
            properties.load(inputStream);
            inputStream.close();
            for(Map.Entry<Object, Object> entry : properties.entrySet()) {
            // for brevity I only look for ints
            if(StringUtils.isNumeric(entry.getValue().toString())) {
                    mSettings.put(entry.getKey().toString(), new ConfigParameter<Integer>((Integer) entry.getValue()));
                } else {
                    mSettings.put(entry.getKey().toString(), new ConfigParameter<String>(entry.getValue().toString()));
                }
            }
        } catch(IOException e) {
            // intentionally left empty
        }
    }

    public ConfigParameter getValue(String s) {
        return mSettings.get(s);
    }

    private class ConfigParameter<T> {
        private T mConfigParameter;

        public ConfigParameter(T var) {
            mConfigParameter = var;
        }

        /**
         * Set the value of the parameter
         * @param param Generic object to be stored
         */
        public void set(T param) {
            mConfigParameter = param;
        }

        /**
         * Retrieve the object
         * @return Generic object stored
         */
        public T get() {
            return mConfigParameter;
        }
    }
}

Elsewhere in my program, when I want to obtain a config element, I reference it this way:

int numberOfThreads = ConfigFactory.INSTANCE.getValue(ConfigFactory.NUM_THREADS).get();

This method can be seen as untenable as I need to have static string literals for each config value defined in the properties file.  Is there a better way to expose these config values to the program and have only one place to change them if the properties file changes?  Is this considered best practices for exposing properties values to the program?

Encapsulation

It's not really necessary that you expose these at all:

public static final String OUTPUT_PATH = "output_path";
public static final String NUM_THREADS = "num_threads";

Let the ConfigFactory be responsible for that, and save the actual values as instance variables, instead of using public ConfigParameter getValue(String s).

For example, instead of exposing getValue(String s), expose these:

public int getNumThreads() {
    ...
}

public String getOutputPath() {
    ...
}

Where the implementation of those methods could either be return getValue(SOME_KEY); or return this.numThreads; , if you add such private fields that you initialize after you've read the config file.

Testability, Dependency Injection

Why is this even a singleton? It's good that you have a method that takes a path so that you can use different configs, but your code leaves some things to be desired.

Immutability / Concurrency: It is currently possible that one thread reads a value, while another thread calls the build method for the Nth time.
Generics: ConfigParameter is using generics, but your getValue method simply returns ConfigParameter, you could change that to:

public <T> ConfigParameter<T> getValue(String s)

Mocking / Testing: If you would want to write unit tests (which you of course want to do), you would have to actually read a configuration file to be able to use the config. However, if you extract an interface, that exposes the important methods, such as getNumThreads() then you could easily create other implementations of this interface, which doesn't even have to read from a config file!

Additionally, instead of a enum I would use a static factory method.

Here's how to use it as an static factory method, although I hope you will take my other advice as well about extracting an interface etc:

public class MyConfig { // it's not a factory

    private Map<String, ConfigParameter> mSettings = new HashMap<String, ConfigParameter>();

    public static MyConfig build(String path) {
        MyConfig config = new MyConfig();
        InputStream inputStream = getClass().getResourceAsStream(path);
        Properties properties = new Properties();
        try {
            properties.load(inputStream);
            inputStream.close();
            for (Map.Entry<Object, Object> entry : properties.entrySet()) {
            // for brevity I only look for ints
            if(StringUtils.isNumeric(entry.getValue().toString())) {
                    config.mSettings.put(entry.getKey().toString(), new ConfigParameter<Integer>((Integer) entry.getValue()));
                } else {
                    config.mSettings.put(entry.getKey().toString(), new ConfigParameter<String>(entry.getValue().toString()));
                }
            }
            return config;
        } catch(IOException e) {
            return null; // return null to indicate that a problem occurred
            // although the best thing would be to throw an exception here
        }
    }

    // ... ConfigParameter and other stuff
}