Creating two output files from two formatted files I've been working on a project to take data from csv files, re-format it, and display it as charts.  I'm working for a company over the summer and when I leave, I would like my code to be readable/editable by whoever takes over in the future.

This code reads in two files, formats them, and creates two output files.  It takes in one command line argument as a filepath to the source folder for the data.  The formatting just counts the number of occurrences of a word and increments a corresponding value in a 2D array.

Would you understand what this is doing if I hadn't given an explanation?  Should I change anything about it to make it more readable?

#!/usr/bin/perl

use strict;
use warnings;

my $src = $ARGV[0];
my %files = (
    'LM' => [$src . '\raw\casesByCategory_LM.csv', $src . '\casesByCategoryLM.csv'],
    'TM' => [$src . '\raw\casesByCategory_TM.csv', $src . '\casesByCategoryTM.csv'],
);

foreach my $file (keys %files) {
    my @categories = (
        ['RMA Request', 0],
        ['Product Info Request', 0],
        ['Dealer Locator (New or Resupply)', 0],
        ['Order Request', 0],
        ['Order Status', 0],
        ['Wireless Activation/Deactivation', 0],
        ['Pricing', 0],
        ['Account Setup/Registration', 0],
        ['Other', 0],
    );
    my $total = 0;
    open (DATA, $files{$file}[0]) or die $!;

        # Remove header
        <DATA>;
        my $match = 0;
        while (my $line = <DATA>) {
            # Remove \n character
            chomp $line;
            # Search through list of categories to find a match
            if ($line =~ /^"(.+)"$/) {
                foreach my $cat (@categories) {
                    if ($1 eq $cat->[0]) {
                        $cat->[1]++;
                        $match = 1;
                    }
                }
            }
            # Add case to 'Other' if no match was found
            if (not $match) {
                $categories[8]->[1]++;
            } else {
                $match = 0;
            }
            $total++;
        }

    close (DATA);

    # Write organized data to new CSV file for chart
    open (OUTPUT,'>', $files{$file}[1]) or die $!;

        print OUTPUT "Total: $total\n";
        foreach my $cat (@categories) {
            print OUTPUT "$cat->[0],$cat->[1]\n";
        }

    close (OUTPUT);
}

All in all I can understand it OK, it reads fairly well. There are a few structures I would recommend you change.... for a start, I would separate the category definitions from the counters, as it would be easier to maintain:

Note, I am defining @categories as a simple array, not a nested array here.... so it is a different thing from yours:

my @categories = ('RMA Request', 'Product Info Request', 'Dealer Locator (New or Resupply)',
                  'Order Request', 'Order Status', 'Wireless Activation/Deactivation',
                  'Pricing', 'Account Setup/Registration', 'Other');

my %catcounts;
foreach (@categories) {
    $catcounts{$_} = 0;
}

OK, this does three things, it makes adding/changing the categories easy, and it separates the count setup from the category setup. It also sets up a hash fro the counter, so you don't need to loop through them all but can do it by key, so the code:

        if ($line =~ /^"(.+)"$/) {
            foreach my $cat (@categories) {
                if ($1 eq $cat->[0]) {
                    $cat->[1]++;
                    $match = 1;
                }
            }
        }
        # Add case to 'Other' if no match was found
        if (not $match) {
            $categories[8]->[1]++;
        } else {
            $match = 0;
        }

can become:

        if ($line =~ /^"(.+)"$/ && defined $catcount{$1}) {
            $catcount{$1}++;
        } else {
            $catcount{'Other'}++;
        }

This will be much faster because it uses the hash lookup instead of the array loop to find the category.

you will need to adapt the output to loop through the @categories, but print out the %catcount


Propagate a default parameter of a function among several files I am starting to write a CLI using argparse library. This is my first attempt to structure a large project, usually I write single script. The CLI auto is defined in a top directory, helper functions are in a subfolder named project and a main function (where I want to put the logic of my program) in sub-sub-folder named sync.

Here is the tree-structure:

├── top/
├── auto
├── __init__.py
  └── project/
  ├── helper.py
  ├── __init__.py
    └── sync/
    ├── __init__.py
    ├── sync_file.py

helper.py:

def play(device, verbose=False):
    if verbose:
        print device + "with verbose on"
    else:
        print device + "verbose off"

sync_file.py:

from .. import helper

def main(device, verbose=False):
    helper.play(device,verbose)

if __name__ == '__main__':
    #Test here the logic
    main('omv')
    main('omv',verbose=True)

auto:

#!/usr/bin/env python

import argparse
import project.sync.sync_file as logic_sync

def sync(args):
    if args.verbose:
        logic_sync.main(args.device,verbose=True)
    else:
        logic_sync.main(args.device)

parser = argparse.ArgumentParser(description='Info on my project')
subparsers = parser.add_subparsers()

sync_parser = subparsers.add_parser('sync', help='synchronize')
sync_parser.add_argument('--device', default='nas', help='specify the device name')
sync_parser.add_argument('--verbose', action='store_true', help='increase the output verbose')
sync_parser.set_defaults(func=sync)  # set the default function to sync

if __name__ == '__main__':
    args = parser.parse_args()
    args.func(args)

After I've made the auto file executable chmode +x auto I can use the CLI:

./auto sync
# nasverbose off
./auto sync --verbose
# naswith verbose on

I would like to keep this structure but I see a lot of repetition in propagating the verbose parameter from one file to another. Is there a better way to do it?

I would create a shared.py module somewhere (I'll let you handle the import directory tree). It would just contain

args = None

Import it in all your modules and assign args from argparser directly to it.
Now shared.args contains all the information (even device that you did not ask for :))

Of course this is not reentrant because of the shared global structure, but if you need reentrancy, you just have to pass 1 parameter: a fake args structure like argparse created (see the FakeArgs in syncfile.py)

And when you're ready to create one single python singleton object to avoid too much parameter passing, just yell :)

in auto code

#!/usr/bin/env python

import argparse
import shared
import project.sync.sync_file as logic_sync

def sync(args):
    shared.args = args
    logic_sync.main()

parser = argparse.ArgumentParser(description='Info on my project')
subparsers = parser.add_subparsers()

sync_parser = subparsers.add_parser('sync', help='synchronize')
sync_parser.add_argument('--device', default='nas', help='specify the device name')
sync_parser.add_argument('--verbose', action='store_true', help='increase the output verbose')
sync_parser.set_defaults(func=sync)  # set the default function to sync

if __name__ == '__main__':
    args = parser.parse_args()
    args.func(args)

sync_file.py code:

from .. import helper
import shared

def main():
    helper.play()

if __name__ == '__main__':
    #Test here the logic

    # we don't have argparse module creating the structure
    # for us ? what the heck? create a fake one

    class FakeArgs:
        def __init__(self):
           self.verbose = False
           self.device = 'omv'

    shared.args = FakeArgs()
    main()
    shared.args.verbose = True
    main()

helper.py code:

import shared
def play():
    if shared.args.verbose:
        print shared.args.device + " with verbose on"
    else:
        print shared.args.device + " verbose off"


Parsing of XML in Python using minidom, and inserting data into a database I have some XML that I am parsing in Python and I need to insert the data on the database. This is current implementation of my code that is working.

Actual XML:

<JobTechData xmlns="http://tempuri.org/JobTechData.xsd">
  <TechDataParams>
    <ProcessType>IONMILL1</ProcessType>
    <RunNumber>125229</RunNumber>
    <RunType>522</RunType>
    <ToolID>HC_VIM05</ToolID>
    <ProcessTS>2016-01-25-08.06.53.000000</ProcessTS>
    <Recipe>S1280</Recipe>
    <PalletPosition>1</PalletPosition>
    <Target>1280</Target>
    <CumulTarget>0</CumulTarget>
    <CTarget>0</CTarget>
    <SITarget>0</SITarget>
  </TechDataParams>
</JobTechData>

And below is the code im using to parse the XML above.

def split(iterable, where):

    def splitter(acc, item, where=where):
        if item == where:
            acc.append([])
        else:
            acc[-1].append(item)
        return acc
    return reduce(splitter, iterable, [[]])

def insert_runspec(spec):
    cnx = mysql.connector.connect(user = USER,
                                        password = PASSWORD,host = HOST,
                                      database = DB, port = DB_PORT)
    cursor = cnx.cursor()
    params = ['?' for item in spec]
    sql = 'INSERT INTO runspecdata(param_value,param_name,ics_runnum,recipe,batch,tool) VALUES (%s)' % ','.join(params)
    cursor.execute(sql,spec)
    cursor.close()
    cnx.close()

spec = []

rc = doc.getElementsByTagName('Recipe')[0].firstChild.nodeValue
rid = doc.getElementsByTagName('RunNumber')[0].firstChild.nodeValue
tid = doc.getElementsByTagName('ToolID')[0].firstChild.nodeValue

# append specification details to spec list, delimit by '|'
spec.append(int(doc.getElementsByTagName('Target')[0].firstChild.nodeValue))
spec.append('target')
spec.append('|')
spec.append(int(doc.getElementsByTagName('CumulTarget')[0].firstChild.nodeValue))
spec.append('cumul')
spec.append('|')
spec.append(int(doc.getElementsByTagName('CTarget')[0].firstChild.nodeValue))
spec.append('c-target')
spec.append('|')
spec.append('sio-target')
spec.append('|')
spec.append(int(doc.getElementsByTagName('SITarget')[0].firstChild.nodeValue))
spec.append('si-target')
splitspec = split(spec,'|')

ics = 1069

for i in splitspec:
    if any(i):
        i.append(ics)
        i.append(rc)
        i.append(rid)
        i.append(tid)
    t = tuple(i)
    insert_runspec(t)

This is the example data inserted to database.

This is working fine. But as you can see the use of the delimiter isn't so Pythonic. Is there a better way to parse the XML?

You're right, the delimiter doesn't seem pythonic. The whole logic behind your split function either.

Let's start by analyzing the problem: you want to extract parameters by name in your XML, associate it to an other name, add default values and insert it in a database.

There is nowhere a need to fetch every values before storing a row in the database, so we could write a function that does all that at once:

def connect(user, host, pwd, db, port):
    c = mysql.connector.connect(user=user, password=pwd, host=host, database=db, port=port)
    return c

def get_default_values(doc):
    rc = doc.getElementsByTagName('Recipe')[0].firstChild.nodeValue
    rid = doc.getElementsByTagName('RunNumber')[0].firstChild.nodeValue
    tid = doc.getElementsByTagName('ToolID')[0].firstChild.nodeValue
    return 1069, rc, rid, tid

def from_xml_to_db(doc, xml_name, db_name, connexion, default_values):
    param_value = int(doc.getElementsByTagName(xml_name)[0].firstChild.nodeValue)
    db_row = (param_value, param_name) + default_values
    param_binding = ', '.join('?' * len(db_row))
    sql_query = 'INSERT INTO runspecdata(param_value, param_name, ics_runnum, '\
                'recipe, batch, tool) VALUES ({})'.format(param_binding)
    cursor = connexion.cursor()
    cursor.execute(sql_query, db_row)
    cursor.close()

ics_rc_rid_tid = get_default_values(doc)
connexion = connect(USER, HOST, PASSWORD, DB, DB_PORT)
from_xml_to_db(doc, 'Target', 'target', connexion, ics_rc_rid_tid)
from_xml_to_db(doc, 'CumulTarget', 'cumul', connexion, ics_rc_rid_tid)
from_xml_to_db(doc, 'CTarget', 'c-target', connexion, ics_rc_rid_tid)
from_xml_to_db(doc, 'SITarget', 'si-target', connexion, ics_rc_rid_tid)
connexion.close()

I didn't considered the case of 'sio-target' as there is no value associated and it doesn't appear in the DB screenshot. If you want to support adding a row with no parameter associated in the XML, you may want to consider using a default value (say None for xml_name will lead to param_value = 0 or something along the lines).

However, this code is still not ideal. There are repetitions at several levels and, more importantly, there is a tight coupling between reading from XML and writing to DB. You can see that I moved the the DB connection out of the writing function to lessen coupling, let's do more of those.

To start with doc.getElementsByTagName(...)[0].firstChild.nodeValue need to be factorized. Let's add handling of default values when names does not exist:

def read_parameter(doc, name=None, default=0):
    if name is None:
        return default

    try:
        value = doc.getElementsByTagName(name)[0].firstChild.nodeValue
    except IndexError:
        value = default

    return int(value)

Here we handle the IndexError in case there is no tag named name in the document and the returned list is empty. If there is a possibility that a tag does not have elements within it, firstChild may be None and you might have to deal with an AttributeError as well (speculations, I’m not that versed into minidom).

This let us simplify:

def get_default_values(doc, ics=1069):
    return (
        ics,
        read_parameter(doc, 'Recipe'),
        read_parameter(doc, 'RunNumber'),
        read_parameter(doc, 'ToolID'),
    )

Next we need to separate reading from XML from writing to DB. Let's start with the writing:

def store_target(connexion, db_row):
    param_binding = ', '.join('?' * len(db_row))
    sql_query = 'INSERT INTO runspecdata(param_value, param_name, ics_runnum, '\
                    'recipe, batch, tool) VALUES ({})'.format(param_binding)
    with connexion.cursor() as cursor:
        cursor.execute(sql_query, db_row)

Much simpler. I also changed the cursor creation to ease its management, the context manager should be implemented as part of the DB API 2.0 but if it doesn't, just use connexion.cursor() and cursor.close() as previously.

Now the XML reading is already handled by read_parameter, so let's bring it all together:

ics_rc_rid_tid = get_default_values(doc)
connexion = connect(USER, HOST, PASSWORD, DB, DB_PORT)
row = read_parameter(doc, 'Target'), 'target'
store_target(connexion, row + ics_rc_rid_tid)
row = read_parameter(doc, 'CumulTarget'), 'cumul'
store_target(connexion, row + ics_rc_rid_tid)
row = read_parameter(doc, 'CTarget'), 'c-target'
store_target(connexion, row + ics_rc_rid_tid)
row = read_parameter(doc, None), 'sio-target'
store_target(connexion, row + ics_rc_rid_tid)
row = read_parameter(doc, 'SITarget'), 'si-target'
store_target(connexion, row + ics_rc_rid_tid)
connexion.close()

Well… no… this seems not right. Let's improve that as well.

XML_TO_DB_TABLE = (
    ('Target', 'target'),
    ('CumulTarget', 'cumul'),
    ('CTarget', 'c-target'),
    (None, 'sio-target'),
    ('SITarget', 'si-target'),
)

def read_parameter(doc, name=None, default=0):
    if name is None:
        return default

    try:
        value = doc.getElementsByTagName(name)[0].firstChild.nodeValue
    except IndexError:
        value = default

    return int(value)

def get_default_values(doc, ics=1069):
    return (
        ics,
        read_parameter(doc, 'Recipe'),
        read_parameter(doc, 'RunNumber'),
        read_parameter(doc, 'ToolID'),
    )

def connect(user, host, pwd, db, port):
    c = mysql.connector.connect(user=user, password=pwd, host=host, database=db, port=port)
    return c

def store_target(connexion, db_row):
    param_binding = ', '.join('?' for _ in db_row)
    sql_query = 'INSERT INTO runspecdata(param_value, param_name, ics_runnum, '\
                'recipe, batch, tool) VALUES ({})'.format(param_binding)
    with connexion.cursor() as cursor:
        cursor.execute(sql_query, db_row)

def xml_to_db(doc, connexion, table):
    common_values = get_default_values(doc)
    with connexion:
        for xml_name, db_name in table:
            specific_values = read_parameter(doc, xml_name), db_name
            store_target(connexion, specific_values + common_values)

if __name__ == '__main__':
    doc = #Initialize it somehow
    cnx = connect(USER, HOST, PASSWORD, DB, DB_PORT)
    xml_to_db(doc, cnx, XML_TO_DB_TABLE)
    cnx.close()

Here the with connexion part is supposed to commit or rollback the transactions depending if anything went well of if there was an error. Again, if it doesn't work for MySQL, just use the management method you're used to.

A few other comments to finish with:

Your indentation is very weird on your connect call;
Using global variables inside a function can impair reusability, try to pass them as parameters instead;
The if __name__ == '__main__' part lets you import your module and tests functions without having any code executed beforehand;
You're building the binding list ((?, ?, ?, ?, ?, ?)) based on the length of the row you want to insert but you are actually expecting 6 arguments (by looking at the name of the columns involved); why not enforce that anyway and let the DB handle wrong number of arguments itself?

That last point would lead to a much simpler store_target:

def store_target(connexion, db_row):
    with connexion.cursor() as cursor:
        cursor.execute(
            'INSERT INTO runspecdata(param_value, '
            'param_name, ics_runnum, recipe, batch, tool) '
            'VALUES (?, ?, ?, ?, ?, ?)', db_row)


Smooth sidebar toggle animation with vuejs and tailwind I'm making a slide sidebar with vuejs and tailwind. It works but feels kind of sluggish. Is there a way to make it smoother ?
working example: https://codepen.io/tuturu1014/pen/oNzRXeW
<button @click="isOpen = !isOpen" class="bg-blue-200 p-5">
  Open
  Close
</button>

  <transition name="slide">

sidebar

  </transition>

    content

You should use transition instead of animation and target the width property :
 .slide-enter-active, .slide-leave-active {
  transition: width 1s;
}
.slide-enter, .slide-leave-to{
  width:0;
}

LIVE DEMO
we could consider a transition as particular case of animation, the transition is applied when an element property changes (transition), but animation could be applied on any element infinitely or for some duration, you could watch the two first free videos on vue mastery that explain that clearly


"Guess the Number" game in Python I have been using Python 3 to learn programming, and this is a very basic task, but I want to gain good programming patterns/habits at the very beginning.

The program will first randomly generate a number unknown to the user. The user needs to guess what that number is. The program interacts with the user.

import random
randomNumber = random.randrange(0,100)
print("Random number has been generated")
guessed = False
while guessed==False:
    userInput = int(input("Your guess pleas: "))
    if userInput==randomNumber:
        guessed = True
        print("Well done!")
    elif userInput>100:
        print("Our guess range is between 0 and 100, please try a bit lower")
    elif userInput<0:
        print("Our guess range is between 0 and 100, please try a bit higher")
    elif userInput>randomNumber:
        print("Try one more time, a bit lower")
    elif userInput < randomNumber:
        print("Try one more time, a bit higher")

print("End of program")

I would probably combine these two if clauses:

elif userInput>100:
    print("Our guess range is between 0 and 100, please try a bit lower")
elif userInput<0:
    print("Our guess range is between 0 and 100, please try a bit higher")

To:

elif user_input > 100 or user_input < 0:
    print("Our guess range is between 0 and 100, please try again")

Trusting that the user is clever enough to realise that if he entered 1000 (which is still visible in the line just above this output), that he is too high for range(1,101).

Incidentally, there may be a small bug in your program:

random.randrange(1,100)

will never generate 100, just like range(1,100) will only give you the numbers from 1 to 99, inclusive. But you also allow the user to guess 0 or 100.

Use either of these two:

random.randrange(0,101)
random.randint(0,100)

The latter actually does return an integer N, such that a <= N <= b

Your program will crash if the user enters any non-number. Better use something like:

while True:
    try:
        user_input = int(input("Your guess please: "))
    except ValueError:
        print("Please enter a number")
        continue
    if user_input == random_number:
        print("You win")
        break
    elif user_input < 0 or user_input > 100:
        print("Our guess range is between 0 and 100, please try again")
    elif user_input < random_number:
        # Notify user
    elif user_input > random_number:
        # Notify user

Which will ask the user until a valid number is supplied. You can also put all the other checking code in there as well.


BigDecimal code critique I'm teaching myself BigDecimal and would appreciate a critique. In the code fragment below, I calculate a tip from user input: check and tip percentage (entered as integer):

 txt_check = txtcheck.getText().toString();
 tpercent = txttippct.getText().toString();

 BigDecimal hundred = new BigDecimal("100");
 BigDecimal pct = new BigDecimal(tpercent);

 BigDecimal bdpercent = pct.divide(hundred, 2,BigDecimal.ROUND_UNNECESSARY);
 BigDecimal bdcheck = new BigDecimal(txt_check);
 BigDecimal bdtip = new BigDecimal(txt_check);
 bdtip = bdtip.multiply(bdpercent);
 BigDecimal bdtotal = new BigDecimal(txt_check);
 bdtotal = bdtotal.add(bdtip);

The code above appears to function correctly. To display these values I convert to string:

 scheck = new DecimalFormat("####.00").format(bdcheck.doubleValue());
 scheck = StringUtils.leftPad(scheck,7," ");

Thanks for any input.

For such a simple calculation, the logic used for the bdtip and bdtotal variables wasnt straight forward. Instead of creating it with the value of one of the operands, then multiplying/adding it by the other operand, why not do something like the following:

txt_check = txtcheck.getText().toString();
tpercent = txttippct.getText().toString();

BigDecimal hundred = new BigDecimal("100");
BigDecimal pct = new BigDecimal(tpercent);

BigDecimal bdpercent = pct.divide(hundred, 2, BigDecimal.ROUND_UNNECESSARY);
BigDecimal bdcheck = new BigDecimal(txt_check);

BigDecimal bdtip = bdcheck.mulitply(bdpercent);
BigDecimal bdtotal = bdcheck.add(bdtip);


Private client-server chat I try to create a private chat with Python but I don't know best practice.

It's only for study purpose, I'd like understand the logic behind a simple private chat with socket and the best mode to implement it with no framework.

For now, It's not important to check for example if username has > or other, only socket procedure and send messages to specific client.

This is my code for client:

import socket
from thread import *
from time import sleep
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect(('127.0.0.1', 8889))
u = raw_input('Username: ')
print 'To send: recipient>message'
def se(s):
    while 1:
        s.send(u + '>' + raw_input())
        print s.recv(1024)
def re(s):
    while 1:
        s.send(u + '>show>')
        r = s.recv(1024)
        if r != 'No messages':
            print r
        sleep(0.05)
start_new_thread(se ,(s,))
start_new_thread(re ,(s,))
while 1:
    pass

and server:

import socket
from thread import *
import string
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.bind(('127.0.0.1', 8889))
s.listen(10)
c = {}
def clientthread(conn):
    while 1:
        data = conn.recv(1024)
        e = data.split('>')
        if len(e) == 3:
            if e[1] == 'show':
                try:
                    m = ''
                    for i in range(0, len(c[e[0]])):
                        m = m + c[e[0]][i]
                        if i != (len(c[e[0]])-1):
                            m = m + '\n'
                        if m == '':
                            data = 'No messages'
                        else:
                            data = m
                    del c[e[0]]
                except:
                    data = 'No messages'
            else:
                try:
                    c[e[1]]
                except:
                    c[e[1]] = []
                c[e[1]].append(e[0]+'>'+e[2])
                data = 'Ok'
        else:
            data = 'Error'
        conn.send(data)
    conn.close()
while 1:
    conn, addr = s.accept()
    start_new_thread(clientthread ,(conn,))
s.close()

With this code every user can send and recive messages.
For example if there are 2 users (2 clients with server active): max and paul; if max writes paul>hello paul recives max>hello.

Program uses dictionary and lists for send message to particular client.
I don't know if this way is correct.

First off, don't use 1 over True in while loops. While 1 works, using True is much clearer.
Secondly, you have inconsistent indentation. While in Python 2.x, this doesn't affect much, Python 3.x will throw an error.
Never use try-except without a specific error to catch. There could easily be an error that goes un-noticed.
Most of your names are okay, but there are some like c, or e that should be clearer. Variable and function names should also be in snake_case, and classes should be in PascalCase. Names like conn should be renamed to something else, like connection, for example.
You need more blank lines. There should be two blank lines in between each code block/function/class on the top level.
Finally, you should avoid wildcard imports like this: from thread import *. You should either import the entire module, or any classes/functions/variables that you need.

Here's Python's official style guide, PEP8


256 bit AES encryption in Android I am using the following code to encrypt a file in an SD card in Android:

I want the code to be reviewed for:

Security (prioritize this)
Coding Style
Performance

void encrypt() throws IOException, NoSuchAlgorithmException, NoSuchPaddingException,
        InvalidKeyException {
    String myString = getOutputFile();
    File myFile = new File(myString);
    FileInputStream inputStream = new FileInputStream(myFile);
    File encodedfile = new File(path,"filename" + ".mp4");
    FileOutputStream outputStream = new FileOutputStream(encodedfile);
    SecretKeySpec sks = new SecretKeySpec("MyDifficultPassw".getBytes(), "AES");
    Cipher cipher = Cipher.getInstance("AES");
    cipher.init(Cipher.ENCRYPT_MODE, sks);
    CipherOutputStream cos = new CipherOutputStream(outputStream, cipher);
    int b;
    byte[] d = new byte[8];
    while((b = inputStream.read(d)) != -1){
        cos.write(d, 0, b);
    }
    cos.flush();
    cos.close();
    inputStream.close();
}

Verbosity

String myString = getOutputFile();
File myFile = new File(myString);
FileInputStream inputStream = new FileInputStream(myFile);

FileInputStream can take String as argument. I'd do this instead:

FileInputStream inputStream = new FileInputStream(getOutputFile());

Whoa whoa whoa! We are reading from the output file? Did you make a mistake? I dunno, but by simply removing some lines I've suddenly exposed what could be a flaw!

Clarity

You're making a lot of objects and doing things with them. Try putting some blank space in between. It groups statements together and makes the code more logical.

void encrypt() throws IOException, NoSuchAlgorithmException, NoSuchPaddingException,
        InvalidKeyException {
    FileInputStream inputStream = new FileInputStream(getOutputFile());

    File encodedfile = new File(path,"filename" + ".mp4");
    FileOutputStream outputStream = new FileOutputStream(encodedfile);

    SecretKeySpec sks = new SecretKeySpec("MyDifficultPassw".getBytes(), "AES");
    Cipher cipher = Cipher.getInstance("AES");
    cipher.init(Cipher.ENCRYPT_MODE, sks);
    CipherOutputStream cos = new CipherOutputStream(outputStream, cipher);

    int b;
    byte[] d = new byte[8];
    while((b = inputStream.read(d)) != -1){
        cos.write(d, 0, b);
    }
    cos.flush();
    cos.close();
    inputStream.close();
}


Implement 'Foldable' on NonEmpty Given:

data NonEmpty a = NonEmpty a (Array a)

I completed the following exercise from PureScript by Example:

  Write a Foldable instance for NonEmpty. Hint: reuse the Foldable instance for arrays.

instance foldableNonEmpty :: Foldable NonEmpty where
  foldr f acc (NonEmpty a as) = foldr f (f a acc) as
  foldl f acc nonEmpty        = foldr (flip f) acc nonEmpty
  foldMap f (NonEmpty a as)   = f a <> (foldMap f as)

Please critique my implementation. Can I do better?

Yes, you can. Given that foldr is right-associative, your implementation is incorrect. Intuitively, foldr will recurse down a structure until it bottoms out and then will it start accumulating the result by going back up. So, this version would be more appropriate:

foldr f acc (NonEmpty a as) = f a (foldr f acc as)

In fact your version is a correct foldl:

foldl f acc (NonEmpty a as) = foldl f (f acc a) as

I think your version of foldMap is fine given the associativity of the monoidal operation.


JavaScript function to convert underscore-case to camel-case (and vice-versa) Actually I needed a function which converts underscore to camel-case.
But then I've had the idea: Why not extend the function so, that is works the other way around?

Finally I recalled that there is a third case which programmers use: Pascal-Case. So I included that too.

The function doesn't work perfect but as long as the different cases are used as expected it works alright.

Any hints and suggestions concerning improvements are welcomed.

/**
* Converts a string to either underscore-case or camel-case or
*   to pascal-case.
* @param {string} toChange
* @param {string} [toCase='toUnderscoreCase'] - Options:
*   'toUnderscoreCase' | 'toCamelCase' | 'toPascalCase'
* @returns {string} according toCase-param.
*   Returns an empty string in case of error.
*/

function changeCase(toChange, toCase) {
  var needle = null;
  var funct = null;
  var needleChangingUpperLower = /[a-z0-9]_[a-z0-9]/g;
  var needleToUnderscore = /(([a-zA-Z0-9]*?)(([a-z0-9][A-Z]))|([a-zA-Z0-9]*)$)/g;

  toCase = toCase || 'toUnderscoreCase'

  if ( !toChange ||
      typeof toChange !== 'string' ||
      toChange.length < 2 ) {
    return '';
  }

  if (toChange.search(/[^\w]/g) > -1) {
    return '';
  }

  function toChangingUpperLower(match) {
    var chars = match.split('_');

    return chars[0] + chars[1].toUpperCase();
  }

  function toUnderscore(match, wordComplete, wordTruncated, wordChange) {
    var ret = '';

    if (wordChange) {

      var chars = wordChange.split('');

      ret += wordTruncated.toLowerCase() + chars[0] + '_' + chars[1].toLowerCase();
    } else {
      ret = wordComplete.toLowerCase();
    }

    return ret;
  }

  switch (toCase) {
    case 'toCamelCase':
      needle = needleChangingUpperLower;

      toChange = toChange.slice(0, 1).toLowerCase() + toChange.slice(1);

      funct = toChangingUpperLower;

      break;
    case 'toPascalCase':
      needle = needleChangingUpperLower;

      toChange = toChange.slice(0, 1).toUpperCase() + toChange.slice(1);

      funct = toChangingUpperLower;

      break;
    case 'toUnderscoreCase':
      needle = needleToUnderscore;

      funct = toUnderscore;

      break;
    default:
      return '';
  }

  return toChange.replace(needle, funct);
}

// --- Usage-examples and test-cases --------
var tests = [ {
  toChange: 'get_amount_of_taxes',
  toCase: ''
},

{
  toChange: 'ThisIsTheSoCalledPascalCase',
  toCase: 'toCamelCase'
},
{
  toChange: 'thisIsTheSoCalledCamelCase',
  toCase: 'toPascalCase'
},
{
  toChange: 'set_sum_of_articles',
  toCase: 'toCamelCase'
},
{
  toChange: 'getAmountOfTaxes',
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'XMLHttpRequest',
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'theXMLconnector1ForABC',
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'aB',
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'c',
  toCase: 'toUnderscoreCase'
},
{
  toChange: [2, 3],
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'abc#123Spec',
  toCase: 'toUnderscoreCase'
},
{
  toChange: 'abc_890-spec',
  toCase: 'toCamelCase'
},
{
  toChange: 'get_amount_of_taxes',
  toCase: 'abc123'
} ];

tests.forEach(function(test) {
  var result = changeCase(test.toChange, test.toCase);
  var log = console.log;
  var err = console.error;

  !result
  ? err( '%s\n%s',
        'Something has gone wrong.',
        JSON.stringify(test, null, 2))
  : log( '%s - %s',
        result,
        JSON.stringify(test, null, 2));
});

Tests

Your test code isn't the clearest. Assuming the rest of the code works as it should, then you should look into making your tests clearer and more reliable.

As it stands, you don't give any expected value for the test results. Having that would enable you to throw an error if the result is wrong as well as if the method throws. Consider something like this:

var tests = [
    {
        'toChange': 'ThisIsPascalCase',
        'toCase': 'toUnderscoreCase',
        'expected': 'this_is_pascal_case'
    }
];

tests.forEach(function(test) {
    var result = changeCase(test.toChange, test.toCase);
    if(!result) {
        console.error("Something has gone wrong!");
        // log extra error details if you want them
    }
    else if(result !== test.expected) {
        console.error("Result doesn't match expected result!");
        // log extra error and environment/stack details
    }
    else {
        console.log("Test passed successfully.");
    }
});

That makes your test code clearer, and provides you with accuracy checking.


Lazy prime counting function I have written a simple snippet of code that implements a function that returns a list of primes up to a certain integer n. Assuming that this function could be called several times in the script for different values of n, I wanted to make it as "lazy" as possible—that is, able to reuse previously-calculated variables.

Here is the code (assume isPrime(n) returns True if n is prime and False otherwise):

primes = []

def pi(n):
    global primes

    if len(primes) == 0:
        # Full
        for i in range(2, n + 1):
            if isPrime(i):
                primes.append(i)
        return primes

    elif n <= primes[-1]:
        # Lazy
        return list(filter(lambda m: m <= n, primes))

    else:
        # Semi-lazy
        for i in range(primes[-1] + 1, n + 1):
            if isPrime(i):
                primes.append(i)
        return primes

The problem is, as you may have spotted, that I'm using a global variable (primes) to store the list of primes, and later modifying it inside the pi function. I've heard that relying on mutable global variables is a bad design choice. What are better ways to implement this?

This question should actually be generalized to the following: what's the best design pattern to implement a 'lazy' function (i.e. a function that reuses previously calculated values)?

The technique you're looking for is called memoization (or in other contexts it might have names like caching). Lazy evaluation is something else entirely.

Memoization is the canonical example for how decorators work in Python, but you don't need to learn the additional language construct if you don't like. Consider the following snippet. It could be more general, but that's beside the point.

def memoize(f):
    cache = {}
    def _f(n):
        if n not in cache:
            cache[n] = f(n)
        return cache[n]
    return _f

What we've created is a function that takes another function as an argument. How do we use it? Well, we pass a function into it.

def pi(n):
    # do stuff
pi = memoize(pi)

Take a close look at this.

First we make our normal function that deals with primes.
Next, we pass it into this memoize() monster that we just made.
When memoize() sees this function (now just called f), it makes a cache and defines a new function _f(). It's this new function _f() that's finally returned (i.e., at the end of all this, pi is the same as whatever _f happened to be), so let's look closely at what _f() does.

First, _f() takes a look in the cache to see if we've already seen the value n. If not, it stores the output f(n), i.e. the original pi(n) we wanted, in the cache.
Next, it returns the pre-computed value straight from the cache. That's it.

Now we've eliminated the use of globals (behind the scenes the cache thing we described is stored in what's called the closure of _f), we've taken the problem of caching away from pi() to give pi() a small, single purpose, and we've created a reusable memoization mechanism that we can apply to any other function that takes exactly one hashable input.

Back to the decorator thing I mentioned earlier, you could just as well write

@memoize
def pi(n):
    # do stuff

to memoize the pi function instead of applying the memoizer after the fact. It doesn't make much of a difference either way, but the latter is considered more pythonic.

To be able to take multiple arguments, you can start to do things with *args and **kwargs in your memoizer, but that's additional complexity without a lot of added benefits for the sake of this problem.


Java: Only log first error occurrence (with hourly reset) I have a redis caching server. If that caching server is down, my app will query the database directly.

I want to know from the logs if my caching server is not reachable. However, if I add a log error, it would completely spam my logfiles if a high amount of users would try to access data.

As a result, I would like to only log the very first error occurrence, resetting only if no errors occurred for an hour.

Is this a feasable approach at all? If so, please review my implementation:

public class CustomCacheErrorHandler implements CacheErrorHandler {

   private Logger logger = LoggerFactory.getLogger(this.getClass());
   private long errorOccurred;
   private final long ONEHOUR = 3600000;

   public void handleCacheGetError(RuntimeException exception, Cache cache, Object key) {
      if (errorOccurred < (System.currentTimeMillis() - ONEHOUR))
         logger.error("Error while getting cache " + cache.getName() + " for Key " + key);
      errorOccurred = System.currentTimeMillis()
   }
}

Using currentTimeMillis over System.nanoTime() and a final long ONEHOUR over TimeUnit.HOURS.toMillis(1) for performance.

Thanks in advance!

Your code currently checks when the last error occurred. This might not be what you actually want. Assuming that an error occurs every 30 minutes, I would expect to have a log entry every 60 minutes. Your current code produces one log entry at the beginning, and no more log entries after that.

I once wrote similar code, and I separated the code into two Java classes: the Throttler for the actual algorithm and the ThrottledLogger for using the algorithm to throttle log messages.

I'm still happy with that old code, and it had the following additional features:

It allows several events before throttling kicks in.
When throttling kicks in, that is logged as well ("further log messages will be suppressed").
When throttling has finished, that is logged as well ("suppressed {} messages").

With these changes, the log messages do not hide any important information. The throttled logger I used was created like this:

new ThrottledLogger(
    logger,
    5,  // number of allowed messages
    1, TimeUnit.HOUR  // 1 more message every hour
);

With this information you should be able to write the same code as I did. Be sure to write some unit tests for the throttling algorithm, to demonstrate that it works indeed like you want it to work.

If you don't need this additional complexity, you should at least move the errorOccurred = assignment into the body of the if statement.


Largest Palindrome Checker import java.util.ArrayList;

import java.util.Collections;

import java.util.HashSet;

public class LargestPalindrome{
    static ArrayList<String> palinList=new ArrayList<String>();

    public static void splitString(){
        String actualString="ABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOOD";
        for(int i=0 ; i<actualString.length() ; i++){
            for(int j=i ; j<actualString.length() ; j++){
                findPalindrome(actualString.substring(i,j));
            }
        }

    }
    public static void findPalindrome(String actualStr){
        String reversedStr="";
        //int palindromeCount=0;

        for(int i=actualStr.length()-1 ; i>=0 ; i--){
            reversedStr+=actualStr.charAt(i);
        }

        if(actualStr.equals(reversedStr)&&actualStr.length()>=3){
            //System.out.println(actualStr);
            palinList.add(actualStr);
        }
    }

    public static String largestPalindromeCheck(ArrayList<String> palList){
        int maxLength=palList.get(0).length();
        int index=0;
        for(int i=1 ; i<palList.size() ; i++){
            if(palList.get(i).length()>maxLength){
                maxLength=palList.get(i).length();
                index=i;
            }
        }
            return palList.get(index);
    }

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        splitString();
        //System.out.println(palinList);
        System.out.println(largestPalindromeCheck(palinList));
    }
}

Please review the code and provide feedback on best practices and code optimization.

Re-organising the code

You've tried to split the logic into small functions and that was a good idea. Unfortunately, because of the way it plays with a class member palinList, the flow is quite hard to follow. In order to avoid this, the best is to define small functions that perform a simple task and return a result and to try to avoid side-effects if we don't need them.

Also, your function and argument names are not really clear. In particular :

splitString() : one would expect this to get a string and to return a list of string
actualString : what is a "non-actual" string ?
largestPalindromeCheck(ArrayList<String> palList) : this returns the biggest string in the list, it doesn't really matter whether this is a list of palindromes or not.

You are importing useless packages.

You should remove useless code before submitting to code review.

The style for spacing and brackets is a bit unusual.

Taking this simple comments into account (except for the last one because I can't be bothered), here's the code I have rewritten :

import java.util.ArrayList;

public class LargestPalindrome{

    public static String findLargestPalindrome(String s){
        return getLargestString(findPalindromes(s));
    }

    public static String reverseString(String s){
        String reversedStr="";
        for(int i = s.length()-1 ; i >= 0 ; i--){
            reversedStr += s.charAt(i);
        }
        return reversedStr;
    }

    public static ArrayList<String> findPalindromes(String s){
        ArrayList<String> palinList=new ArrayList<String>();
        for(int i=0 ; i<s.length() ; i++){
            for(int j=i ; j<s.length() ; j++){
                String sub = s.substring(i,j);
                if(sub.equals(reverseString(sub))&&sub.length()>=3){
                    palinList.add(sub);
                }
            }
        }
        return palinList;
    }

    public static String getLargestString(ArrayList<String> list){
        int maxLength=list.get(0).length();
        int index=0;
        for(int i=1 ; i<list.size() ; i++){
            if(list.get(i).length()>maxLength){
                maxLength=list.get(i).length();
                index=i;
            }
        }
        return list.get(index);
    }

    public static void main(String[] args) {
        String s="JUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGERJUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGER";
        System.out.println(findLargestPalindrome(s));
    }
}

Making things faster

if(sub.equals(reverseString(sub))&&sub.length()>=3) can be rewritten if(sub.length()>=3 && sub.equals(reverseString(sub))) to avoid going through list construction if we don't need to.

Actually, this is not so much of an issue because we can make the check for palindromes more efficient : reusing this answer, one can define a check for palindromness that doesn't need to build a string and stops as soon as the string is indeed not a palindrom. With my examples, instead of going through 2420363 iterations in reverseString(), we now perform 31423 iterations in isPalindrome.

Now, for a more dramatic improvement we have to focus on what we really want to achieve, what we are really looking for : we don't care that much about populating a list of palindromes, we only care about the biggest one. What does this imply ? It implies that we can ignore whatever is smaller than what we have already found.

At this stage, the code looks like this :

public class LargestPalindrome{

    public static boolean isPalindrome(String str) {
        int n = str.length();
        for( int i = 0; i < n/2; i++ )
        {
            if (str.charAt(i) != str.charAt(n-i-1)) return false;
        }
        return true;
    }

    public static String findLargestPalindrome(String s){
        String biggestPalindrome = "";
        for(int i=0 ; i<s.length() ; i++){
            for(int j=i ; j<s.length() ; j++){
                String sub = s.substring(i,j);
                if (sub.length()>biggestPalindrome.length() && isPalindrome(sub)){
                    biggestPalindrome = sub;
                }
            }
        }
        return biggestPalindrome;
    }

    public static void main(String[] args) {
        String s="JUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGERJUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGER";
        System.out.println(findLargestPalindrome(s));
    }
}

Now, an additional trick : for a given i, j will go through all values from i to s.length() -1 and the order does not really matter (yet). Thus, the loop can be returned : for(int j=i ; j<s.length() ; j++) becomes for(int j=s.length()-1 ; j >= i ; j--). First cool thing : for a given i, we'll call length() only once : it doesn't look like much but as it comes for free, it's pretty good already but the best is still to come. Indeed, for a given i, the substrings we are considering will get smaller and smaller. Thus, we can stop iterating whenever the strings we would consider become smaller than what we have found already.

This can be written :

public static String findLargestPalindrome(String s){
    String biggestPalindrome = "";
    for(int i=0 ; i<s.length() ; i++){
        for(int j=s.length()-1 ; j >= i ; j--){
            nbIt++;
            String sub = s.substring(i,j);
            if (sub.length() < biggestPalindrome.length()) break;
            if (isPalindrome(sub)){
                biggestPalindrome = sub;
            }
        }
    }
    return biggestPalindrome;
}

Actually, we don't even need to build that substring to consider its length : because of the way we construct it, sub.length() == j-i and this optimisation can be included as part of the for syntax : for(int j=s.length()-1 ; j >= i + biggestPalindrome.length(); j--).

At the end, my final version of the code looks like :

public class LargestPalindrome{

    public static boolean isPalindrome(String str) {
        int n = str.length();
        for( int i = 0; i < n/2; i++ )
        {
            if (str.charAt(i) != str.charAt(n-i-1)) return false;
        }
        return true;
    }

    public static String findLargestPalindrome(String s){
        String biggestPalindrome = "";
        for(int i=0 ; i<s.length() ; i++){
            for(int j=s.length()-1 ; j >= i + biggestPalindrome.length(); j--){
                String sub = s.substring(i,j);
                if (isPalindrome(sub)){
                    biggestPalindrome = sub;
                }
            }
        }
        return biggestPalindrome;
    }

    public static void main(String[] args) {
        String s="JUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGERJUSTTOMAKETHESTRINGABITLONGERABCBAHELLOHOWRACECARAREYOUILOVEUEVOLIIAMAIDOINGGOODRISETOVOTESIRJUSTTOMAKETHESTRINGABITLONGER";
        System.out.println(findLargestPalindrome(s));
    }
}


Optimizing text drawing on screen I'm looking to speed up this method. Ideally, I would like to cut the time in 1/2 or more. At the moment I have to draw the screen line-by-line as the font is 16 pixels high, but it is being drawn with an extra blank pixel on the bottom. If I could figure out how to set the line-height to 16 pixels I would imagine that drawing 1 string over 24 strings would help speed it up. Baring that what else can I do to increase the speed?

- (void)drawRect:(NSRect)dirtyRect
{
    NSGraphicsContext *nsGraphicsContext = [NSGraphicsContext currentContext];
    CGContextRef context = (CGContextRef) [nsGraphicsContext graphicsPort];

    CGContextSetShouldSmoothFonts(context, false);

    NSMutableAttributedString *string = [[NSMutableAttributedString alloc] initWithString:@""];
    for (int y=24; y >= 0; y--) {
        [[string mutableString] setString:@""];;
        for (int x=0; x < 80; x++) {
            AnsiScreenChar *c = [self.screen objectAtIndex:(y*80)+x];
            [self.fontAttributes setValue:c.fgColor forKey:NSForegroundColorAttributeName];
            [self.fontAttributes setValue:c.bgColor forKey:NSBackgroundColorAttributeName];

            NSAttributedString *subString = [[NSAttributedString alloc] initWithString:c.data
                                                                            attributes:self.fontAttributes];
            [string appendAttributedString:subString];
        }
        [string drawAtPoint:NSMakePoint(0, self.frame.size.height - ((y+1)*16))];
    }
}

Profile

Running     Time  Self        Symbol Name
3276.0ms    9.5%  3276.0      -[AnsiView drawRect:]
1726.0ms    5.0%  1726.0      -[NSLayoutManager(NSTextViewSupport) showCGGlyphs:positions:count:font:matrix:attributes:inContext:]
1540.0ms    4.4%  1540.0      -[__NSCFDictionary setObject:forKey:]
1467.0ms    4.2%  1467.0      -[NSObject dealloc]
1118.0ms    3.2%  1118.0      -[NSView _drawRect:clip:]
1108.0ms    3.2%  1108.0      -[NSConcreteHashTable getItem:]
1106.0ms    3.2%  1106.0      -[NSLayoutManager(NSPrivate) _drawBackgroundForGlyphRange:atPoint:parameters:]
901.0ms     2.6%  901.0       -[NSLayoutManager(NSPrivate) _insertionPointHelperForGlyphAtIndex:]
779.0ms     2.2%  779.0       -[NSLayoutManager(NSPrivate) _drawGlyphsForGlyphRange:atPoint:]
704.0ms     2.0%  704.0       -[__NSCFString replaceCharactersInRange:withString:]
693.0ms     2.0%  693.0       -[NSATSGlyphStorage createCTTypesetter]
619.0ms     1.8%  619.0       -[NSAttributeDictionary objectForKey:]
599.0ms     1.7%  599.0       -[NSLayoutManager(NSPrivate) _rectArrayForRange:withinSelectionRange:rangeIsCharRange:singleRectOnly:fullLineRectsOnly:inTextContainer:rectCount:rangeWithinContainer:glyphsDrawOutsideLines:]
558.0ms     1.6%  558.0       -[NSLayoutManager locationForGlyphAtIndex:]
537.0ms     1.5%  537.0       +[NSObject allocWithZone:]

You want to avoid as much work inside drawRect: as possible. Instead of recalculating the attributed string for each line here do it whenever self.screen changes. That way it's done once instead of every time the some portion of the view is redrawn (which will happen a lot).

Maybe keep a self.lines array with the attributed string for each line in it. Or maybe it's a dictionary with the key being the line number and you just keep adding lines and drawRect just draws the last displayHeight lines.

Basically do everything you can to remove unnecessary work from drawRect.


Critique some Ruby + RSpec Code for a SERP checker I found a SERP checker written in PHP and I decided, in order to better learn to program, I would re-write it in Ruby.

All I have written so far takes a list of keywords that a user inputs and cleans the list and turns each keyword into a url for Google search.

Here is the code:

require 'sinatra'
require 'rspec'
get '/serp_checker' do
  "<form action='/ranked' method='post'>
  Keyword
  <textarea name='keyword' id='keyword' type='text' /></textarea>
  URL
  <input name='url' id='url' type='text' />
  <input type='submit' value='Go!' />
  </form>"
end

def clean_up_keywords(str)
  str.gsub("\n", ",").delete("\r").split(',')
end

def clean_up_list(arr)
  arr.reject(&:empty?).each(&:lstrip!)
end

def make_strings_url_friendly(arr)
  arr.each do |e|
    e.gsub!(" ", "+")
  end
end

def make_urls(arr)
  arr.map {|e| "http://www.google.com/search?num=100&q=" + e}
end

post '/ranked' do
  dirty_list = clean_up_keywords(params[:keyword])
  clean_list = clean_up_list(dirty_list)
  url_ready_list = make_strings_url_friendly(clean_list)
  url_list = make_urls(url_ready_list)
end

Here is the spec:

require_relative '../lib/rankypanky.rb'

describe "#clean_up_keywords" do
  it "should push items separated by a newline into array as separate items" do
    clean_up_keywords("apples\noranges").should == ["apples", "oranges"]
  end
  it "should delete all \r chars" do
    clean_up_keywords("Chat\r").should == ["Chat"]
  end
  it "should push items separated by commas into an array" do
    clean_up_keywords("Chat, Meta, About").should == ["Chat", " Meta", " About"]
  end
  it "should push items separated ONLY by a \s char into array as one item" do
    clean_up_keywords("New York, apples").should == ["New York", " apples"]
  end
end

describe "#clean_up_list" do
  it "should not include empty strings/items in array" do
    clean_up_list(["apples", "", "oranges"]).should == ["apples", "oranges"]
  end
  it "should remove any leading white space from items in array" do
    clean_up_list([" oranges", "apples"]).should == ["oranges", "apples"]
  end
end

describe "#make_strings_url_friendly" do
  it "should replace /s with a +" do
    make_strings_url_friendly(["oranges apples"]).should == ["oranges+apples"]
  end
end

describe "#make_urls" do
  it "should append array items to http://www.google.com/search?num=100&q=" do
    make_urls(["pie"]).should == ["http://www.google.com/search?num=100&q=pie"]
  end
end

Basically I'm looking for any better ways to do this as well as whether my code meets hmmm standards... just some general critique. Thank you so much!

I see you haven't got any response on this for a long while, so I'll give it a try.

I don't really see anything to pick on when it comes to style or technique. To me this looks very good. A few questions though:

why do you require "rspec" in the controller code? Shouldn't this be in the test code instead?
You don't do any attempt to sanitize or verify that the code works with non-english characters, or other characters that may cause trouble. What if someone type "tørris&message=<script>alert("Hoy there");</script>" in the input form? (I guess google will sanitize it, but it would probably be better to do it before passing the url to them.)


Print a random anagram of a given string I am given a task to create a function that prints a random anagram of a given string:

def anagram(value):
    '''Prints random anagram of given value'''
    import random
    newWord = ''
    for i in range(len(value)):
        pos = random.randint(0, len(value)-1)
        newWord += value[pos]
        value = value[:pos] + value[pos+1:]
    print newWord

anagram('12345')
anagram('should')

What all edge cases should this program cover?

When building a string you should build a list, and then use ''.join(). This is as strings are immutable, and so generating newWord takes \$O(n^2)\$ time.
You are mannually poping pos from value. If you change value to a list, you can just use list.pop.
You should import random at the top of your code. Never in a function.
Common Python style is to use snake_case for variables.
Common Python style is to use _ as a throw away variable.
You can use random.randrange, rather than randint
It's best if you return rather than print your anagram.

And so you could use:

def anagram(value):
    new_word = []
    value = list(value)
    for _ in range(len(value)):
        pos = random.randrange(len(value))
        new_word.append(value.pop(pos))
    return ''.join(new_word)

This however runs in \$O(n^2)\$ time. If you use a Fisher–Yates shuffle, you can do this in \$O(n)\$ time.

def anagram(value):
    value = list(value)
    for i in range(len(value)):
        pos = random.randrange(i, len(value))
        value[i], value[pos] = value[pos], value[i]
    return ''.join(value)

You can also use random.shuffle, which likely also uses the above algorithm. However you won't have to maintain it. Allowing:

def anagram(value):
    value = list(value)
    random.shuffle(value)
    return ''.join(value)


Merge sort algorithm implementation using C++ (redux) This is a follow up question to the one I asked a couple of days ago.

I reviewed the suggestions & did some research (specifically, read relevant info on CLRS). The new version is 3 times faster (210ms vs 71ms on 100,000 numbers) & looks cleaner.

One thing I still feel weird about is not using iterators except than to init the sub-vectors. However, any attempt in using them causes a drop in performance (down to 120ms).

I'm also curious if my code is good in terms of code style. Is this idiomatic C++?

void merge(std::vector<int>& a, int p, int q, int r)
{
    std::vector<int> l(a.begin() + p, a.begin() + q + 1);
    std::vector<int> ri(a.begin() + q + 1, a.begin() + r + 1);

    int n1 = q - p + 1;
    int n2 = r - q;

    int i, j; i = j = 0;

    while (i < n1 && j < n2) {
        if (l[i] < ri[j]) {
            a[p++] = l[i++];
        } else {
            a[p++] = ri[j++];
        }
    }

    while (i < n1) {
        a[p++] = l[i++];
    }

    while (j < n2) {
        a[p++] = ri[j++];
    }
}

void merge_sort(std::vector<int>& a, int p, int r)
{
    if (p < r) {
        int q = (p + r) / 2;
        merge_sort(a, p, q);
        merge_sort(a, q + 1, r);
        merge(a, p, q, r);
    }
}

The first thing I would suggest is using better names for your variables.  It's very hard for someone else looking at or using your code to determine what those variables are supposed to represent.


How to properly get values from multidimensional object Output should be an object ({cal: val}), with soonest possible time throughout all calendars recommended times from the data.

My question is: is this approach over complicated and if so, what should be the proper way to solve that task?

Incoming data is an object:

let data = [
  {
    name: "A",
    calendars: {
      "1_A@group.calendar.google.com": {
        busy: ["2019-08-19T14:30:00.000Z"],
        free: [
          {
            start: "2019-08-19T08:00:00.000Z",
            end: "2019-08-19T09:30:00.000Z"
          }
        ],
        recommended: "2019-08-19T01:00:00.000Z"
      },
      "2_A@group.calendar.google.com": {
        busy: ["2019-08-19T14:30:00.000Z"],
        free: [
          {
            start: "2019-08-19T12:30:00.000Z",
            end: "2019-08-19T14:00:00.000Z"
          }
        ],
        recommended: "2019-08-19T04:00:00.000Z"
      }
    }
  },
  {
    name: "B",
    calendars: {
      "1_B@group.calendar.google.com": {
        busy: ["2019-08-19T14:30:00.000Z"],
        free: [
          {
            start: "2019-08-19T12:30:00.000Z",
            end: "2019-08-19T14:00:00.000Z"
          }
        ],
        recommended: "2019-08-19T05:00:00.000Z"
      },
      "2_B@group.calendar.google.com": {
        busy: ["2019-08-19T14:30:00.000Z"],
        free: [
          {
            start: "2019-08-19T12:30:00.000Z",
            end: "2019-08-19T14:00:00.000Z"
          }
        ],
        recommended: "2019-08-19T02:00:00.000Z"
      }
    }
  }
];

Here is how I solved it:  https://codesandbox.io/s/purple-rain-ecmcu?fontsize=14

function reduce(data) {
  return data.reduce((acc, cur, i) => {
    if (!acc.length || cur[1] < acc[1]) {
      return [cur[0], new Date(cur[1])];
    } else {
      return acc;
    }
  }, []);
}

data = Object.fromEntries([
  reduce(
    data.map(provider => {
      let providerCalendars = Object.keys(provider.calendars);
      let providerRecommended = Object.values(provider.calendars).map(
        (calendar, i) => [
          providerCalendars[i],
          new Date(calendar.recommended).getTime()
        ]
      );
      return reduce(providerRecommended);
    })
  )
]);

Minor Changes

Method name reduce suggests a generic API method that would take a predicate and accumulator. Instead, it's very specific in returning a custom array and mapping a string to Date. I would call it getCalendarByMinimumRecommendedTime instead.
Prefer the use of const over let for immutable data: const providerCalendars and const providerRecommended.
The unused index parameter i can be omitted: return data.reduce((acc, cur) => {.


Simple webpage on React.js I just started to learn React.js and decided to build on of the simpler projects from freeCodeCamp with it. I'm really interested in ways in which this code can be improved.

I can see at least two problems - two components with the exact same logic and different states and not working feature with scrolling to different sections based on href of a link pressed. While I will be able to fix the second one eventually, the first one is more about logic of the code.

So, my main concerns are the logic of the code and style of it. What should be fixed and how?

Here is a link to working code:

Codepen

class Contact extends React.Component {
  constructor() {
    super();
    this.state = {
      links: [
        "https://www.vk.com",
        "https://www.github.io",
        "https://codepen.io"
      ],

      icons: [
        "http://www.iconninja.com/files/16/107/688/vk-com-icon.png",
        "http://www.freeiconspng.com/uploads/git-github-hub-icon-25.png",
        "http://www.iconninja.com/files/695/496/986/black-codepen-icon.png"
      ]
    };
  }
  render() {
    const { links, icons } = this.state;
    let social = links.map((href, i) => {
      return (

      );
    });
    return
{social}
;
  }
}
class Portfolio extends React.Component {
  constructor() {
    super();
    this.state = {
      links: [
        "http://kiraburova.github.io/ModernLook/",
        "http://kiraburova.github.io/passion/",
        "http://kiraburova.github.io/singolo/",
        "https://kiraburova.github.io/Kafeinate/"
      ],

      images: [
        "https://image.ibb.co/chE8xv/1.png",
        "https://image.ibb.co/kR9K4a/2.png",
        "https://image.ibb.co/n9niVF/3.png",
        "https://image.ibb.co/i3PVAF/4.png"
      ]
    };
  }
  render() {
    const { links, images } = this.state;
    let gallery = links.map((href, i) => {
      return (

      );
    });
    return
{gallery}
;
  }
}
class About extends React.Component {
  constructor() {
    super();
    this.state = {
      name: "Name",
      text: "Hello! This is just a sample text"
    };
  }
  render() {
    return (

{this.state.name}

{this.state.text}

    );
  }
}
class Navigation extends React.Component {
  constructor() {
    super();
    this.state = {
      nav: ["About", "Portfolio", "Contact"]
    };
  }
  render() {
    let navigation = this.state.nav.map(item => {
      return {item};
    });
    return
{navigation}
;
  }
}
class App extends React.Component {
  render() {
    return (

        <Navigation />
        <About />
        <Portfolio />
        <Contact />

    );
  }
}

ReactDOM.render(<App />, document.getElementById("app"));

Great choice, Allan and good effort! React is awesome and fun. You can improve a few things if you are interested:

Separate Presentational and Container Components. This way your app will be easier to scale, read and maintain
You can try to have your state in one parent component and pass it as props to its children
Extract all components in different files
Navigation can be done with React Router. It is very simple to set up and easy to work with
Consider learning and using Redux for your React App state when you are comfortable with the basics or React
Enjoy learning :)


Use parent class to declare a child object or how compiler allocates memory I'm working with C# and .NET Framework 4.0 and Windows Forms.

On my project I have three custom objects that inherits from MyControl class (MyControl class inherits from UserControl).

I have this code:

private void ShowControlForBatchStatus(BatchStatus batchStatus)
{
    MyControl control = null;

    panelContainer.Controls.Clear();

    if (batchStatus == BatchStatus.NoBatch)
    {
        control = new InicializeLevel();
    }
    else if ((batchStatus == BatchStatus.BatchRunning) ||
                (batchStatus == BatchStatus.BatchPaused))
    {
        control = new Stadistics();
    }
    else if (batchStatus == BatchStatus.BatchFinish)
    {
        control = new FinishBatch();
    }

    panelContainer.Controls.Add(control);
}

Now I think that if it is correct to declare a MyControl control variable when I'm going to use bigger control like FinishBatch. I think that if compiler allocate memory for a MyControl object, and now I use a bigger one for that variable, is that a problem?

Maybe, I'm confuse and the compiler only allocate memory when I do new FinishBatch();.

Does the compiler allocate memory when I declare a variable?

Kind of a strange question for CodeReview as I see nothing wrong with your code itself. Answering your questions:

... is that a problem?

  No.

Does the compiler allocate memory when I declare a variable?

  Here's a good answer from Eric Lippert on storage location when declaring variables.

The only thing you could change about your code is make it a switch statement (although this change has nothing to do with the memory question):

private void ShowControlForBatchStatus(BatchStatus batchStatus)
{
    MyControl control = null;
    panelContainer.Controls.Clear();

    switch(batchStatus)
    {
        case BatchStatus.NoBatch: control = new InicializeLevel(); break;
        case BatchStatus.BatchRunning:
        case BatchStatus.BatchPaused: control = new Stadistics(); break;
        case BatchStatus.BatchFinish: control = new FinishBatch(); break;
    }

    panelContainer.Controls.Add(control);
}


Framework constructor I'm working on my framework constructor. First I tried to look into jQuery and understand it, but it's so out of my league. Instead I googled and put this code together. Though there aren't many posts about creating a framework. Anyway, since I'm not a guru I might be doing it the wrong way or missing things. Can I take your opinions, suggestions to make it a little more stable?

FIDDLE

var optionalSelector = "$";

(function () {
    (this[arguments[0]] = function constructor(input) {
        if (!(this instanceof constructor)) {
            return new constructor(input);
        }

        var regExp_id = /^#/;
        var regExp_class = /^\./;
        var regExp_tag = /[^#]|[^.]/;

        var typeOfInput;

        function isElement(obj) {
            if (obj !== null && typeof obj === "object") {
                return obj;
            }
        }

        if (isElement(input)) {
            return input;
        } // this
        else {
            if (input.search(regExp_id) != -1) {
                typeOfInput = "id";
            } else if (input.search(regExp_class) != -1) {
                typeOfInput = "class";
            } else if (input.search(regExp_tag) != -1) {
                typeOfInput = "tag";
            }

            switch (typeOfInput) {

                case "id":
                    if (document.getElementById(input.replace(/#/, ""))) {
                        return document.getElementById(input.replace(/#/, ""));
                    } else {
                        console.log("Invalid id: " + input);
                        return false;
                    }
                    break;

                case "class":
                    if (document.getElementsByClassName(input.replace(/./, "")).length > 0) {
                        return document.getElementsByClassName(input.replace(/./, ""));
                    } else {
                        console.log("Invalid class: " + input);
                        return false;
                    }
                    break;

                case "tag":
                    if (document.getElementsByTagName(input).length > 0) {
                        return document.getElementsByTagName(input);
                    } else {
                        console.log("Invalid tag: " + input);
                        return false;
                    }
                    break;
            }
        }
    })
})(optionalSelector);

First of all, nice effort for the code. But I highly go against it because there's jQuery. And if you don't like jQuery because it's heavy, then I suggest going for Sizzle.

But anyways, back to your code. First I notice this one:

var optionalSelector = "$";

Never use $. It's being abused already by frameworks. Use another namespace to avoid collisions with mainstream libraries, something unique, like MyVeryAmazingDomLibrary or MVADL for short. Hehehe.

Also, I notice this namespace declaration happens to be outside the library. Do include it inside the library's module:

;(function(window){
  ...
  window.myNamespace = myLibraryStuffHere;
  ...
}(window));

So far, your code looks ok. Everything is pretty much covered. I also see that it works only with single selectors at the moment. However, I notice you are using replace to remove the # and .:

document.getElementById(input.replace(/#/, ""));

RegExp is heavy for trivial operations such as this. You can remove the first character using substr or substring. That way, you avoid RegExp.

document.getElementById(input.substr(1));

Now, in the demo, I see this:

a[0].onclick = function() { alert("you clicked the classes"); }
a[1].onclick = function() { alert("you clicked the classes"); }

If you had a hundred elements, would you do this still? I assume you'd do a loop, right? But wouldn't it be neat if the library took care of that? Yes?

So there's a reason they call it a "jQuery object". jQuery objects are basically arrays (array-like to be exact, normally called a set or collection) that applies operations to each item in the collection.

So to get you on the right track, think of this code:

a.on('click',someFunction);

a is the jQuery object, and on is a method of jQuery that attaches a click handler to each item in set a. on would look like:

myAmazingLibraryObject.prototype.on = function(event,handler){

  //assuming "this" represents the current collection and "elements"
  //is an array of elements, a result of getting by tag or by class.
  //Let's say we are standards compliant and ES5 compliant,
  //we use forEach to loop through the set, and addEventListener to
  //add the handlers.

  this.elements.forEach(function(element){
    element.addEventListener(event,handler,false);
  });
}

Now, you can do this:

var elements = myAmazingLibrary('a'); //get all <a>

elements.on('click',function(event){
  event.preventDefault();
  console.log('clicked');
});

An advantage of using a jQuery-like object, where the results are contained in an array, is that even when your collection is empty, the code does not break. If we used on on an empty collection, nothing will go wrong. Empty set, no loop, no element access, no error.

Lastly, don't just console.log or return false on errors. Throw meaningful errors instead. Use Error objects so that try-catch can collect them and inspect them. The last thing you would want from your code is to have it running broken and not know about it.


Computing exponential function by Taylor Series without overflow The original question is to write a Fortran program to compute the sum of the first 20 terms in the exponential equation (for x=1,2,3,4,5):
$$\sum_{n=0}^\infty \frac{x^n}{n!} = 1 + \frac{x^1}{1!} + \frac{x^2}{2!} + \frac{x^3}{3!} + \frac{x^4}{4!} + \frac{x^5}{5!} + \ldots$$

My original approach was:

PROGRAM Exponential_Function

IMPLICIT NONE

INTEGER :: x,n,i
REAL :: f=0.0,fact

! Print Header
PRINT *, "           x     Sum"

Do x = 1,5,1
    Do n = 0,19,1

        ! calculate factorial
        fact =1.0
        Do i=1,n,1
            fact = fact*i
        End Do

        f=f+(x**n)/(fact)
    END Do

    PRINT *, x, f
    f=0.0
End Do

END PROGRAM Exponential_Function

The code could run, but only displayed results up to x=3:

Then, I tried another approach:

IMPLICIT NONE

INTEGER :: x,n
REAL :: f=1.0,term=1.0

! Print Header
PRINT *, "           x     Sum"

Do x = 1,5,1
    Do n = 1,19,1
        term = term * (REAL(x)/REAL(n))
        f = f + term

    END Do

    PRINT *, x, f
    term = 1.0
    f = 1.0

End Do

and it gave the right results:

           x     Sum
           1     2.71828
           2     7.38906
           3     20.0855
           4     54.5982
           5     148.413

I wonder if the second solution is the only correct solution to this question?
Is there a way to rectify the first approach to avoid the arithmetic overflow problen in Fortran?

Well the exponential function is map from reals to reals (usually denoted f:R→R), so I would have expected the use of reals for all variables (i.e., x and n). However, I'll base my answer using integers, as that is what you've used.

Using larger integers

Fortran's basic integer precision has a largest integer value of 2147483647, which is exceeded for 4**16=4294967296. However, you certainly can specify a larger integer type (see the link) and easily fit 4**19=274877906944. This would be as easy as specifying the kind:

integer :: x,n

would become

integer, parameter :: i_8 = selected_int_kind(15)
integer(i_8) :: x, n

And you easily get the values you desire.

Alternative implementation

Instead of specifying larger precision, you could express the Taylor-expansion of the exponential as
$$
e^x = 1 + x\cdot \left(1 + \frac{x}{2}\cdot\left(1 + \frac{x}{3}\cdot \left(\cdots\right) \right) \right)
$$
and write your function as,

f=1.0
do i=n-1,1,-1
   f = 1.0 + x * f / i
end do

(here n is your number of terms you want). Fortran will implicitly cast i and x as reals (since they are multiplying/dividing a real in f), so you don't need to worry about larger precision in your integers. This version could also fit easily into a function where you can have the user specify the n and x values:

real function my_exp(n, x) result(f)
   integer, intent(in) :: n, x
   f=1.0
   do i=n-1,1,-1
      f = 1.0 + x * f / i
   end do
end function

which is small enough that a simple contains will be fine for your program (i.e., using a module isn't needed).

Other odds-and-ends

Modern Fortran (well, since Fortran 90) does not require the use of capitals for keywords, so you don't need to have your program yell at you that it is printing something or that you're going to specify the types of all variables (i.e., you can use implicit none, real and integer). Most IDEs I know have support for Fortran syntax, so you'll know when you're using keywords.

While initializing f=1.0 in the main will do what you expect, you may want to break the habit of doing so now because you may (accidentally) carry it over to functions where it will have different behaviour.


Python program to find a word ladder transforming "four" to "five" I saw this Puzzling problem and thought I would try to write a Python program to solve it. The task is to transform "four" to "five", forming a new four-letter word at each step, replacing one letter at each step, in as few steps as possible.

But turns out I don't know how to optimize recursion, so I'm posting here for help. I'm mostly just confused on why the code to change the past needs to be at the top of the function, but I would also like advice on how to speed this up in general. Right now it takes about 10x as long for each step up max_depth gets on my computer.

There won't be any matches until you change max_depth - I didn't want anyone copy-pasting and lagging out. There should be a solution at depth 5, according to Puzzling. However, my words file doesn't have the Foud or the word Fous, which that answer uses. Bumping up to max_depth six will take my computer ~10 minutes, which I don't want to try yet.

def hamming(string1, string2):
    assert len(string1) == len(string2)
    return sum(char1 != char2 for char1, char2 in zip(string1, string2))

max_depth = 3
start_word = "five"
end_word = "four"
all_words = open("/usr/share/dict/words", "r").read().lower().splitlines()
all_words = list(filter(lambda word: word.isalpha(), all_words))
all_words = list(filter(lambda word: len(word) == len(start_word), all_words))

sequences = []

def search(current_word, past = []):
    # Needs to be first to be fast for some reason
    past = past[:]
    past.append(current_word)

    if len(past) > max_depth:
        sequences.append(past)
        return

    for word in all_words:
        if hamming(word, current_word) == 1 and word not in past:
            search(word, past)

search(start_word)

sequences = [sequence[:sequence.index(end_word) + 1] for sequence in sequences if end_word in sequence]

if len(sequences) == 0:
    print("No matches")
else:
    print(min(sequences, key=len))

By performing recursion, you are performing a depth-first search of four-letter words. However, this task involves finding a shortest path, and shortest-path problems are generally better done using breadth-first search. With BFS, the first solution you encounter will be an optimal solution — which is not the case with DFS.


Finding all delivery orders with multiple purchase orders I have a SQL query to find all delivery orders with multiple purchase orders (one-to-many). Is there a better or shorter way to write this?

SELECT * FROM GR_data
WHERE doNo IN
(
    SELECT dt.doNo FROM
    (
        SELECT doNo, po_number FROM GR_data
        WHERE DATALENGTH(Mat_No)<=0
        GROUP BY doNo, po_number
    )AS dt
    GROUP BY dt.doNo
    HAVING COUNT(dt.doNo)>1
)
ORDER BY doNo, po_number

The subquery calculates a distinct count using two step, of course this can be simplified to:

SELECT *
FROM GR_data
WHERE doNo IN
 (
   SELECT doNo
   FROM GR_data
   WHERE DATALENGTH(Mat_No)<=0
   GROUP BY doNo
   HAVING COUNT(DISTINCT po_number)>1
 )
ORDER BY doNo, po_number

Depending on your actual data it might be more efficient to apply a Windowed Aggregate Function:

COUNT(DISTINCT po_number) OVER (PARTITION BY doNo)

Unfortunately using DISTINCT in Analytical Functions is not supported  by SQL Server.

But assuming that there are no NULLs in po_number COUNT DISTINCT OVER can be emulated using different ways, probably most efficient by applying two DENSE_RANKS:

SELECT *  -- if you don't want to show "dense_cnt" you  must list all other columns besides
FROM
 (
   SELECT *,
      DENSE_RANK() OVER (PARTITION BY doNo ORDER BY po_number)
     +DENSE_RANK() OVER (PARTITION BY doNo ORDER BY po_number DESC) - 1 AS dense_count
   FROM GR_data
   WHERE DATALENGTH(Mat_No)<=0
 ) AS dt
WHERE dense_count > 1
ORDER BY doNo, po_number

Check actual resource usage if those rewrites are more efficient.


Brute Force N Body Implementation in C++ I have wrote the following code in C++ for the n-body problem. This code is sequential as later on I am planning to parallelize it using OpenMP. I want to know whether I have wrote the correct implementation, if there are some errors or bugs, or if this is an invalid approach.

#include <iostream>
#include <ctime>
#include <math.h>
using namespace std;
#define N 100
#define G 6.673e-11
#define TIMESTAMP 1e11
struct Particle{
    double rx, ry;//position components
    double vx, vy;//velocity components
    double fx, fy;//force components
    double mass;//mass of the particle

};
Particle Update(Particle p, double timestamp)
{
    p.vx += timestamp*p.fx / p.mass;
    p.vy += timestamp*p.fy / p.mass;
    p.rx += timestamp*p.vx;
    p.ry += timestamp*p.vy;
    return p;
}
void PrintParticle(Particle p)
{
    printf("rx == %f ry == %f vx == %f vy == %f mass == %f\n", p.rx,p.ry,p.vx,p.vy,p.mass);
}
//Reset the forces on particle
Particle ResetForce(Particle p)
{
    p.fx = 0.0;
    p.fy = 0.0;
    return p;
}
//Add force to particle a by particle b
Particle AddForce(Particle a,Particle b)
{
    double EPS = 3E4;      // softening parameter (just to avoid infinities)
    double dx = b.rx - a.rx;
    double dy = b.ry - a.ry;
    double dist = sqrt(dx*dx + dy*dy);
    double F = (G * a.mass * b.mass) / (dist*dist + EPS*EPS);
    a.fx += F * dx / dist;
    a.fy += F * dy / dist;
    return a;

}

int main()
{
    Particle particles[N];
    srand(time(NULL));
    //randomly generating N Particles
    for (int i = 0; i < N; i++){
        double rx = 1e18*exp(-1.8)*(.5 - rand());
        particles[i].rx = rx;
        double ry = 1e18*exp(-1.8)*(.5 - rand());
        particles[i].ry = ry;
        double vx = 1e18*exp(-1.8)*(.5 - rand());
        particles[i].vx = vx;
        double vy = 1e18*exp(-1.8)*(.5 - rand());
        particles[i].vy = vy;
        double mass = 1.98892e30*rand()*10 + 1e20;
        particles[i].mass = mass;

    }

    int numberofiterations = 10;
    int count = 0;
    while (count < numberofiterations){
        for (int i = 0; i < N; i++)
        {
            particles[i] = ResetForce(particles[i]);
            for (int j = 0; j < N; j++)
            {
                if (i != j)
                {
                    particles[i] = AddForce(particles[i], particles[j]);
                }

            }
        }
        //loop again to update the time stamp here
        for (int i = 0; i < N; i++)
        {
            particles[i] = Update(particles[i], TIMESTAMP);
        }
        for (int i = 0; i < N; i++)
        {
            PrintParticle(particles[i]);
        }
        count++;
    }

    return 0;
}

Here are some observations that may help you improve your code.

Make sure to #include all required headers

This program calls printf and srand but does not include the corresponding headers.  Fix that by adding these lines:

#include <cstdio>
#include <cstdlib>

Use objects

You have a Particle structure and then separate functions that operate on Particle data. With only a slight syntax change, you would have a real object instead of C-style code written in C++.  For example, you have a loop right now that says this:

for (int i = 0; i < N; i++)
{
    particles[i] = Update(particles[i], TIMESTAMP);
}

With objects, and with C++11 or newer, you could instead write this:

for (auto &p : particles)
    p.update(TIMESTAMP);

All that would be required would be to make Update a member function.

Prefer stream I/O to printf

The printf function was a capable workhorse for many years, but the C++ iostream library is better in a number of regards.  Although it's often more typing for the programmer initially, it's better because it has better type checking, less possibility for runtime overhead, and it fits well with the rest of C++.  So I would probably change your PrintParticle to this:

friend ostream& operator<<(ostream& out, const Particle &p) {
    return out <<
        "rx == " << p.rx_ <<
        " ry == " << p.ry_ <<
        " vx == " << p.vx_ <<
        " vy == " << p.vy_ <<
        " mass == " << p.mass_ << '\n';
}

Use const where practical

There are a number of things in your code that look and act like constants, such as numberofiterations in main().  Declare them as const for a program more resistant to error. Also, move the constants, such as N into main rather than having them at file scope.

Create a function rather than repeating code

The functions which create the particle use the same rand()-based function several times.  Instead of repeating it, make it into a function:

double xrand() {
    return 1e18*exp(-1.8)*(.5 - rand());
}

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.  It isn't necessarily wrong to use, but be aware of when you absolutely shouldn't do it (such as in header files).

Consider using a better random number generator

If you are using a compiler that supports at least C++11, consider using a better random number generator.  In particular, instead of rand, you might want to look at std::uniform_real_distribution and friends in the <random> header.

Consider altering your algorithm

At the moment, the force on each particle from every other particle is calculated exhaustively, but Newton tells us that the force actually acts equally (but opposite) on both particles, so your loop could be simplified and calculations speeded if you take advantage of that fact to only calculate force vectors once and then apply them to both.

Double check your constant values

It seems to me that 3e4 is a somewhat large value for EPS.  Additionally, in the calculation of the random mass, the math is currently 1.98892e30*rand()*10 + 1e20, but why not simply write 1.98892e31*rand() + 1e20 instead?

Eliminate return 0 at the end of main

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.


Updating MySQL with shuffled numbers between 1 and 50 I need to assign a number to each userId.  I think this is too much of a query for updating.  Is it possible to reduce the number of queries, or is it okay?

def newRanking(groupId: String) = {
    DB.withConnection { implicit con =>
      var rank: List[Int] = util.Random.shuffle(List(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50))
      var i = 0
      val sql = SQL("SELECT userId FROM `groupInformation` WHERE groupId={groupId} limit 50").on("groupId" -> groupId)
      sql().map { row =>
        SQL("update groupInformation set rank={rank} where groupId={groupId} and userId={userId}").on("groupId" -> groupId, "rank" -> rank(i), "username" -> row[String]("userId")).executeUpdate()
        i += 1
      }.toList
    }
  }

As I'm sure you've noticed, executing any SQL query in a loop is a bad idea.  It would be much better to formulate a statement that accomplishes the entire batch of changes at once.

To achieve that goal, you'll need to combine several ideas:

Shuffling using ORDER BY RAND().  Admittedly, the documentation admits:

  RAND() is not meant to be a perfect random generator. It is a fast way to generate random numbers on demand that is portable between platforms for the same MySQL version."

So, you'll have to decide whether MySQL's idea of randomness is as good as Scala's.
A row number generator.  Unfortunately, MySQL has no row_number() or rank() function, like some other databases, so you'll need to use a hack.
UPDATE using multiple table references.

A statement that you could use is:

UPDATE `groupInformation`
    INNER JOIN (
            SELECT *
                FROM `groupInformation`
                ORDER BY RAND() LIMIT 50
        ) AS shuffled
            ON `groupInformation`.userId = shuffled.userId
    INNER JOIN (
            SELECT @row := 0
        ) AS row_var_decl
    SET `groupInformation`.rank = @row := @row + 1
    WHERE `groupInformation`.groupId = {groupId};

Here's an SQL Fiddle as a demonstration.

As you've noted, the behaviour of the query above is different from the original when there are fewer than 50 matching rows in groupInformation.