Mean by properties in array of objects I have this array of objects:

const array = [
  {continent: 'Africa', country:'Algeria', year:'2018', value:10},
  {continent: 'Africa', country:'Algeria', year:'2017', value:15},
  {continent: 'Africa', country:'Algeria', year:'2016', value:2},
  {continent: 'Africa', country:'Egypt', year:'2018', value:20},
  {continent: 'Africa', country:'Egypt', year:'2017', value:1},
  {continent: 'Africa', country:'Egypt', year:'2016', value:20},
]

I want a new array with the mean by year:

const result = [
  {continent: 'Africa', country:'', year:'2018', value:15},
  {continent: 'Africa', country:'', year:'2017', value:8},
  {continent: 'Africa', country:'', year:'2016', value:11},
]

This is my code:

const yearsTemp = Array.from(new Set(dataset.map((datum: any) => datum.year)).values())
const years = yearsTemp.map(year => year.toString())
const means = years.map(year => {
  const filtered = filter(dataset, { continent: continent, year: year })
  const mean = filtered.reduce((acc, item) => acc + item.value, 0) / filtered.length
  return {
    continent: dataset[0].continent,
    country: '',
    year: year,
    value: mean,
  }
})

Is there a better way?

Since this is labelled TypeScript, I'm going to review it as such. First of all, you aren't using types, which defeats the purpose of using TypeScript. You should define what your objects look like

interface Datum { //not sure what to call it
    continent: string,
    country: string,
    year: string,
    value: number
}

I'm not a TypeScript expert or anything, but even as a beginner, I'd expect something at least like this. With this, we get that the year is apparently a string. Maybe it's not an issue but it's something to note. Before defining the type I just glanced at the object and assumed it was a number.

Which means that your array declaration should now look like this

const array : Array<Datum> = [
  {continent: 'Africa', country:'Algeria', year:'2018', value:10},
  {continent: 'Africa', country:'Algeria', year:'2017', value:15},
  {continent: 'Africa', country:'Algeria', year:'2016', value:2},
  {continent: 'Africa', country:'Egypt', year:'2018', value:20},
  {continent: 'Africa', country:'Egypt', year:'2017', value:1},
  {continent: 'Africa', country:'Egypt', year:'2016', value:20},
]

and the compiler would know what you are working with providing type safety for future operations.

Now, let's take a look at some other declarations

const yearsTemp : Array<string> = Array.from(new Set(dataset.map((datum: Datum) => datum.year)).values())

Assuming dataset has a content similar to array in the beginning, the types we annotate are Datum for each variable. This means that we are mapping each year value and get an array of string. However, calling .values() on the Set is superfluous - Array.from() works fine with just being passed a Set. So, you can drop that.

But what about the next line

const years: Array<string> = yearsTemp.map(year => year.toString())

So, we take an array of strings and produce an array of strings. That's fine normally, but to do that you call .toString() on each value. That's a pointless operation "some value".toString() is the same thing. It's also dangerous, as null.toString() would cause an error. For future reference, a way that wouldn't throw a null pointer exception is to pass the value through the String constructor function but don't use the keyword new.

let numericValue = 42;

let stringValue = String(numericValue);

console.log("typeof stringValue", typeof stringValue);
console.log('stringValue === "42"', stringValue === "42");

//this creates a new object string, not a string primitive
let dontDoThis = new String(numericValue);

console.log("typeof dontDoThis", typeof dontDoThis);
console.log('dontDoThis === "42"', dontDoThis === "42"); //not the same type
console.log('dontDoThis === new String("42")', dontDoThis === new String("42")); //not the same object

Passing the value null or undefined through String will not cause an error but it will produce the string "null" or "undefined" respectively. You can filter those out, if you don't want them

let objectsWithNumericValues = [{year: 2018}, {year: 2019}, {year: 2019}, {year: 2020}, {year: null}];

let extractedYears = objectsWithNumericValues.map(obj => obj.year);
let resultWithNull = Array.from(new Set(extractedYears))
  .map(String); //<-- convert to string

let resultWithoutNull = Array.from(new Set(extractedYears))
  .filter(Boolean) //<-- filter anything falsey
  .map(String);

console.log(resultWithNull);
console.log(resultWithoutNull);

On a separate note, the above code shows how you can avoid temporary variables by chaining operations. It saves some typing and it's clearer by not having clutter like throw-away variable names. But it also depends on the coding style.

Anyway, back on the topic of getting the years and turning them into strings - just drop the line that does toString and rename const yearsTemp to const years

As for the rest it seems OK, as long as you add the types. It's not clear where the continent variable is coming from when you call filter(dataset, { continent: continent, year: year }) but I'd assume you have that in context. The other strange this is dataset[0].continent. I'm not sure if that's correct or not, as it would take the same continent for each new produced value and it's not necessarily going to be the correct one unless dataset[0].continent === continent. But that might be a typo when transcribing the code here. Perhaps the same is true for country: ''

The only change I'd make is separate the functionality instead of put all the processing into .map(). The chaining of array operation makes the separation of intentions clearer and if you need to extract functionality, it's cleaner.

years
    .map(year => filter(dataset, { continent: continent, year: year }))
    .map(filtered => {
        const mean = filtered.reduce((acc, item) => acc + item.value, 0) / filtered.length
        return {
            continent: dataset[0].continent,
            country: '',
            year: year,
            value: mean
        }
    })

this way if you decide that either or both functionalities are reusable, you can separate them and your logic turns into

years
    .map(getResultsWithTheSameYear)
    .map(calculateMean)

and you can use these functions elsewhere. Perhaps you will have some other functionality where you do

countries
    .map(getResultsWithSameCountry)
    .map(calculateMean)


Exporting a UserForm from the VBE... and working around API bugs Rubberduck has been using an extension method to export code files, notably for source control purposes.

While testing source control, I ran into a weird bug in the VBIDE API, which caused UserForm modules to perpetually be treated as "modified", because every time we exported them, the VBIDE API would add an empty line at the beginning of the code-behind.

So I had to modify our ExportAsSourceFile extension method to work around that glitch:

Export the form to a temporary file
Read the contents of the file
Locate the last line of the file heading (attributes that are now showing in the VBE)
Determine if we have an extraneous empty line, and remove it
Write the modified content into the intended export folder

Modifying the file directly in the export folder wasn't an option, because then Git would have considered it as modified anyway.

For context/reference, here's what a UserForm code file looks like:

VERSION 5.00
Begin {C62A69F0-16DC-11CE-9E98-00AA00574A4F} UserForm1
   Caption         =   "UserForm1"
   ClientHeight    =   3165
   ClientLeft      =   45
   ClientTop       =   390
   ClientWidth     =   4710
   OleObjectBlob   =   "UserForm1.frx":0000
   StartUpPosition =   1  'CenterOwner
End
Attribute VB_Name = "UserForm1"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Option Explicit 'first line of actual user code visible in the VBE

Notice the VB_PredeclaredId attribute is always the last one specified: my fix relies on that detail.

Here's the extension method in question:

/// <summary>
/// Exports the component to the directoryPath. The file is name matches the component name and file extension is based on the component's type.
/// </summary>
/// <param name="component">The component to be exported to the file system.</param>
/// <param name="directoryPath">Destination Path for the resulting source file.</param>
public static string ExportAsSourceFile(this VBComponent component, string directoryPath)
{
    var path = Path.Combine(directoryPath, component.Name + component.Type.FileExtension());
    if (component.Type == vbext_ComponentType.vbext_ct_Document)
    {
        var lineCount = component.CodeModule.CountOfLines;
        if (lineCount > 0)
        {
            var text = component.CodeModule.get_Lines(1, lineCount);
            File.WriteAllText(path, text);
        }
    }
    else if(component.Type == vbext_ComponentType.vbext_ct_MSForm)
    {
        // VBIDE API *sometimes* inserts an extra newline when exporting a UserForm module.
        // this issue causes forms to always be treated as "modified" in source control, which causes conflicts.
        // we need to remove the extra newline before the file gets written to its output location.
        var tempFile = component.ExportToTempFile();
        var contents = File.ReadAllLines(tempFile);
        var vbExposedAttributeLine = 0;
        for (var i = 0; i < contents.Length; i++)
        {
            if (contents[i].StartsWith("Attribute VB_Exposed = "))
            {
                vbExposedAttributeLine = i;
                break;
            }
        }
        if (contents.Length > vbExposedAttributeLine && contents[vbExposedAttributeLine + 1].Replace(Environment.NewLine, string.Empty).Trim() == string.Empty)
        {
            // first line in module is empty - assume it wasn't in the VBE
            var edited = contents.Take(vbExposedAttributeLine).Union(contents.Skip(vbExposedAttributeLine + 1)).ToArray();
            File.WriteAllLines(path, edited);
        }
    }
    else
    {
        // other module types can just be exported directly
        component.Export(path);
    }

    return path;
}

public static string ExportToTempFile(this VBComponent component)
{
    var path = Path.Combine(Path.GetTempPath(), component.Name + component.Type.FileExtension());
    component.Export(path);
    return path;
}

Is there a better way to do this? It works.. but is there anything that could go wrong with it?

Well, here's a form saved from Excel 2013.

Notice the VB_Exposed, VB_HelpID and VB_Description attributes. I set the Description and Help attributes using the Object Browser Context Menu ("Properties"), but a user might have manually edited the attributes in a previous export/import, and so they might be in a different order

VERSION 5.00
Begin {C62A69F0-16DC-11CE-9E98-00AA00574A4F} UserForm1
   Caption         =   "UserForm1"
   ClientHeight    =   3165
   ClientLeft      =   45
   ClientTop       =   390
   ClientWidth     =   4710
   OleObjectBlob   =   "UserForm1.frx":0000
   StartUpPosition =   1  'CenterOwner
End
Attribute VB_Name = "UserForm1"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Attribute VB_Exposed = False
Attribute VB_HelpID = 55
Attribute VB_Description = "This is a description"
Private Sub UserForm_Click()
  Debug.Print "Test"
End Sub

There is a trailing blank line, but CR MarkDown isn't showing it.

Edit:
With Excel working again, I imported the file, and I do get the extra header line, and if it is exported with a header line, and reimported, there are then 2 header lines, so I'm getting the same behaviour as you.

You won't be able to rely on the position/order of VB_Attributes, so you'll have to parse the file.

You're also assuming a user won't have intentionally placed a blank line at the top of the module. And what happens if the user has more than one blank line at the top of the module?

The safest approach would probably be to count the blank lines at the top of the code module, before the export, and then adjust the number of blank lines found after the last VB_Attribute but before the actual code begins.

That would potentially deal with any fix that might come to VBE.


Checking for Roman numeral validity I am working on a Roman numerals calculator. For an input validity test I'm using this function:

bool isValidRomanNumeral(string& roman){
    // no test for one-digit Roman numeral
    if (roman.size() == 1) return true;
    for(size_t i=0; i<roman.size()-1; i++){
        // each more significant digit is either >= previous digit (II, VI, etc)
        if(toArab(to_string(roman[i])) >= toArab(to_string(roman[i+1])) ||
           // or 5 times smaller than previous digit (IV, XL, CD, etc)
           5*toArab(to_string(roman[i])) == toArab(to_string(roman[i+1])) ||
           // or 10 times smaller than previous digit (IX, XC, CM, etc)
           10*toArab(to_string(roman[i])) == toArab(to_string(roman[i+1])));
        else return false;
    }
    return true;}

Function: int toArab(string roman)

int toArab(string s){
 map<char, int> roman;
 roman['M'] = 1000;
 roman['D'] = 500;
 roman['C'] = 100;
 roman['L'] = 50;
 roman['X'] = 10;
 roman['V'] = 5;
 roman['I'] = 1;
 int res = 0;
 for(int i=0; i<s.size()-1; ++i){
    if(roman[s[i]] < roman[s[i+1]]) res -= roman[s[i]];
    else res += roman[s[i]];
 }
 res += roman[s[s.size()-1]];
 return res;}

Is there something else that can be done for optimization? Is there a better way to check for Roman numeral validity?

Bugs

This function will accept invalid strings as valid Roman numerals, e.g. IXC, XCX, IIIII, DD, VL, or A.

You always advance by one character.  If you're going to do that, you should use a state machine so that you know what you expect next.  Then you'd be able to check that D can't follow D and other invalid entries.  Part of the problem is that you can't compare just adjacent characters, e.g. IX and XC are valid substrings but IXC is not.

Repeated initialization

You initialize your roman map each time.  It would be better to initialize it once.  Consider a pattern like

static map<char, int> value_of;
if ( 0 == value_of.size() ) {
    initialize_digits_map(value_of);
}

Note that I used snake_case for the function name, as that's the standard for C++.  I also prefer it in general as it's easier to tell where each word ends and begins for people who may not use the English alphabet natively.

Even better would be to create a conversion class.  Your toArab function could then use the conversion class.  As could your isValidRomanNumeral function.  That would get rid of the unnecessary conversion to string.

Don't repeatedly process an expression with a static value

    for(size_t i=0; i<roman.size()-1; i++){

It's not a big difference, but it can be slightly more efficient to say

    for (size_t i = 0, n = roman.size() - 1; i < n; i++) {

That way you only call the function once and only perform the subtraction once.  In this case, the function call will likely get inlined, but the subtraction might not be.

It's generally easier for humans to read if you add more spaces.  This won't matter to the compiler but can help people read your code more quickly.


Stack with "getMiddle" and "deleteMiddle" operation Looking for code review, optimizations and best practices.

public class FindMiddleStack<T> {

    private Node<T> top;
    private Node<T> middle;
    private int size = 0;

    private static class Node<T> {
        Node<T> left;
        T item;
        Node<T> right;

        Node(Node<T> left, T item, Node<T> next) {
            this.left =  left;
            this.item = item;
            this.right = next;
        }
    }

    public void push(T item) {
        final Node<T> node = new Node<T>(null, item, top);
        if (top == null) {
            top = node;
            middle = node;
        } else {
            top.left = node;
            top = node;
        }

        size++;
        if (size % 2 == 0) {
            middle = middle.left;
        }
    }

    public T pop() {
        if (top == null) {
            throw new EmptyStackException();
        }
        T item = top.item;
        top = top.right;
        // important, to not leak references.
        if (top != null) {
            top.left = null;
        }

        if (top == null) {
            middle = null;
        }

        if (size % 2 == 0) {
            middle = middle.right;
        }

        size--;
        return item;
    }

    public T peek() {
        if (top == null) {
            throw new EmptyStackException();
        }
        return top.item;
    }

    public T getMiddle() {
        if (top == null) {
            throw new EmptyStackException();
        }
        return middle.item;
    }

    public void deleteMiddle() {
        if (top == null) {
            throw new EmptyStackException();
        }

        if (middle.left != null) {
            middle.left.right = middle.right;
        }
        if (middle.right != null) {
            middle.right.left = middle.left;
        }

        if (size % 2 == 1) {
            // deleted middle.
            middle = middle.left;
        } else {
            middle = middle.right;
        }

        if (middle == null) {
            top = null;
        }
        size--;
    }

    public int size() {
        return size;
    }

    public boolean isEmpty() {
        return size == 0;
    }
}

public class FindMiddleStackTest {

    @Test
    public void testPushPop() {
        FindMiddleStack<Integer> fm = new FindMiddleStack<Integer>();
        fm.push(10);
        fm.push(20);
        fm.push(30);
        fm.push(40);
        fm.push(50);
        fm.push(60);
        fm.push(70);
        int[] expected1 = {70, 60, 50, 40, 30, 20, 10};
        int[] actual1 = new int[7];
        int i1 = 0;
        while (!fm.isEmpty()) {
            actual1[i1++] = fm.pop();
        }
        assertTrue(Arrays.equals(expected1, actual1));
    }

    @Test
    public void testPopAndMiddle( ) {
        FindMiddleStack<Integer> fm = new FindMiddleStack<Integer>();
        fm.push(10);
        fm.push(20);
        fm.push(30);
        fm.push(40);
        fm.push(50);
        fm.push(60);
        fm.push(70);
        int[] expected2 = {40, 40, 30, 30, 20, 20, 10};
        int[] actual2 = new int[7];
        int i2 = 0;
        while (!fm.isEmpty()) {
            actual2[i2++] = fm.getMiddle();
            fm.pop();
        }
        assertTrue(Arrays.equals(expected2, actual2));
    }

    @Test
    public void testDeleteMiddleAndMiddle() {
        FindMiddleStack<Integer> fm = new FindMiddleStack<Integer>();
        fm.push(10);
        fm.push(20);
        fm.push(30);
        fm.push(40);
        fm.push(50);
        fm.push(60);
        fm.push(70);
        int[] expected3 = {40, 50, 30, 60, 20, 70, 10};
        int[] actual3 = new int[7];
        int i3 = 0;
        while (!fm.isEmpty()) {
            actual3[i3++] =  fm.getMiddle();
            fm.deleteMiddle();
        }
        assertTrue(Arrays.equals(expected3, actual3));
    }
}

Implementing "correctly":

Your Classname says, you have implemented a Stack with additional operations. Why do I not see implements or extends in your code then?

public class FindMiddleStack<T> {

should have been:

public class FindMiddleStack<T> extends Stack<T> {

scratch that, Stack<T> has been deprecated for so long, that there should be some annotation @Antique. Even the official oracle documentation recommends the use of Deque<T>!
Mind that Deque can also be used as a FIFO Queue

That said, your class could start with the following:

public class FindMiddleDeque<T> implements Deque<T> {

Design:

Your node class design is "flawed". You make it way to complicated, if you only want a LIFO (last in first out) behavior. Usually a Node in a stack knows about it's value and has a reference to the next Node.

private static class Node<T> {
    Node<T> next;
    T value;

    Node(T value, Node<T> next) {
        this.value = value;
        this.next = next;
    }
}

This also simplifies your pop and push methods:

@Override
public T pop() {
    if(size == 0 || top == null){
       throw new EmptyStackException();
    }

    size--;
    Node<T> destroy = top;
    top = destroy.next;

    return destroy.value;
    //destroy should be destroyed by falling out of scope
}

@Override
public void push(T value) {
    top = new Node(value, top);
    size++;
}

@Override
public T peek() {
    if(size == 0 || top == null){
       throw new EmptyStackException();
    }

    return top.value;
}

Finding the middle.

Your code is a little complicated concerning finding the middle of the stack. In fact we can assume, that the middle element is that at "index" size / 2.

How can we use that to our advantage?

public void deleteMiddle() {
    if(size == 0 || top == null){
        throw new EmptyStackException();
    }
    int middleIndex = size / 2;

    Node<T> current = top;
    Node<T> previous = null;
    for (int i = 0; i < middleIndex; i++){
        previous = current;
        current = current.next;
    }
    // we can assume we are in the middle now.
    previous.next = current.next; //current is lost
    size--;
}

We can also get the middle element in a similar fashion:

public T getMiddle() {
    if(size == 0 || top == null){
        throw new EmptyStackException();
    }

    int middleIndex = size / 2;

    Node<T> current = top;
    for (int i = 0; i < middleIndex; i++){
        current = current.next;
    }
    return current.value;
}


A simple parcel checker Python script My current student accommodation uses a manual paper and pen system for checking if students have mail so I made this simple Python script to help mitigate the problem of manually searching through spreadsheets just to see if you have mail or not, which I have been asked to demo soon. It's quite basic but I figured I should probably get it code reviewed since it may get real world usage.

GitHub

import json
import os

if not os.path.isfile('parcels.json'):
    open('parcels.json', 'w').write(json.dumps({}))
parcels = json.loads(open('parcels.json').read())

def add(student_id, new_letters, new_parcels):
    if student_id in parcels:
        parcels[student_id][0] += new_letters
        parcels[student_id][1] += new_parcels
    else:
        parcels[student_id] = [new_letters, new_parcels]

def collect(student_id):
    if student_id in parcels:
        parcel_info = 'Letter: ' + str(parcels[student_id][0]) + ' Parcel: ' + str(parcels[student_id][1])
        del parcels[student_id]
        return parcel_info
    else:
        return 'Letter: 0 Parcel: 0'

print('1. Add\n2. Collect')
option = input('Option: ')
student_id = input('Student ID: ')

if len(student_id) != 7:
    print('Invalid input! (student ID must be 7 digits long)')
    raise SystemExit(1)
try:
    if int(student_id) < 0:
        print('Invalid input! (student ID must be a non-negative integer)')
        raise SystemExit(1)
except ValueError:
    print('Invalid input! (student ID must be an integer)')
    raise SystemExit(1)

if option == '1':
    try:
        new_letters = int(input('Letters: '))
        new_parcels = int(input('Parcels: '))
    except ValueError:
        print('Invalid input! (only integers allowed)')
        raise SystemExit(1)

    if new_letters < 0 or new_parcels < 0:
        print('Invalid input! (only non-negative integers allowed)')
        raise SystemExit(1)

    add(student_id, new_letters, new_parcels)
elif option == '2':
    print(collect(student_id))
else:
    print('Invalid option!')

open('parcels.json', 'w').write(json.dumps(parcels))

Code organization

Defining the add() and collect() functions is a good start, but it could be better, as there is still a lot of free-floating code outside those functions.  Part of the problem is that prompting for the inputs to add() requires intimate knowledge of what add() does, so you might as well make add() responsible for reading its own input.

Prompting for the Student ID is common to options 1 and 2, but is not appropriate for any other invalid option.  Therefore, the Student ID prompting code needs to be moved.

parcels.json is hard-coded four times.  The loading code is at the top, far from the saving code at the bottom.  I also find it odd that you feel compelled to write an empty dictionary to parcels.json if the file does not exist.

I'd use the following outline:

import json
import re

DATABASE_FILENAME = 'parcels.json'

def load():
    try:
        with open(DATABASE_FILENAME) as f:
            return json.loads(f.read())
    except IOError as e:
        return {}

def save(data):
    …

def input_student_id(prompt='Student ID: '):
    while True:
        student_id = input(prompt)
        if re.match(r'^\d[7]$', student_id):
            return student_id
        print('Invalid input! Student ID must be 7 digits')

def input_nonnegative_int(prompt):
    …

def add(data):
    student_id = input_student_id()
    new_letters = input_nonnegative_int('Letters: ')
    new_parcels = input_nonnegative_int('Parcels: ')
    record = data.get(student_id, [0, 0])
    data[student_id] = [record[0] + new_letters, record[1] + new_parcels]

def collect(data):
    student_id = input_student_id()
    record = data.get(student_id, [0, 0])
    print('Letter: {0} Parcel: {1}'.format(*record))
    del data[student_id]

def main():
    data = load()
    while True:
        option = input('1. Add\n2.Collect\nOption: ')
        if option == '1':
            add(data)
            break
        elif option == '2':
            collect(data)
            break
        else:
            print('Invalid option!')
    save(data)

if __name__ == '__main__':
    main()

Besides reorganizing all of the code into functions, I've also changed the input validation.  It's more forgiving — retrying instead of aborting.  Also, using a regular expression to validate the student ID simplifies the code while forbidding "000.000" as input (which your program would have accepted).

Data format

Using a two-element list to store the letter and parcel count is hard to read and a headache for future expansion.  A dictionary with two keys would be a bit better.

I'm concerned about using a JSON file as a database.  It's not a storage mechanism that is designed for concurrent access, so you might end up with dropped add/collect transactions or even a corrupted file.  If you don't want to install a full SQL database server, then consider using SQLite as a storage mechanism — it takes care of the concurrency concerns for you.


Connect 4 Game Design in C++ I am working on designing a simple version of a Connect 4 game.
Here is my first draft code.
I would like some feedback on it, and also required modification for these follow up questions:

Add some intelligence in the players to select the column optimally.
How will you make the game generic so that it can check for 16 points in a row/column/diagonal
How will you handle a huge grid?

Regarding item3, I think since I am not storing the whole grid, my approach is scalable.
I appreciate for any recommendations or feedback.
class Connect4 {

vector <pair<int, map< pair<int,int>,int > > > rows;
vector <pair<int, map< pair<int,int>,int > > > cols;
vector <pair<int, map< pair<int,int>,int > > > diag;
int r;
int c;
int player;
public:
  Connect4 (int rr, int cc):r(rr), c(cc){
     rows = vector <pair<int, map< pair<int,int>,int > > >  (rr);
     cols = vector <pair<int, map< pair<int,int>,int > > > (cc);
     diag = vector <pair<int, map< pair<int,int>,int > > > (min (rr,cc));
  }

  int move(int row, int col, int player){
      int score = (player ==1)?1:-1;   // count player-1 socres with positive and player-2 with negative numbers

      rows[row].first +=score;
      rows[row].second[{row,col}]=score;
      if (abs(rows[row].first) ==4 && isValid(rows[row].second) )
            return player;

      cols[col].first += score;
      cols[col].second[{row,col}]=score;
      if (abs(cols[col].first) ==4 && isValid(cols[col].second) )
            return player;

      if(row ==col+2){
        diag[0].first +=score;
        if (abs(diag[0].first) ==4 ) //if the score is 4 no extra checking is needed
            return player;

      } else if (row ==col+1) {
        diag[1].first += score;
        if (abs(diag[1].first) ==4 && isValid(diag[1].second) )
            return player;

      } else if (row ==col) {
         diag[2].first += score;
         if (abs(diag[2].first) ==4 && isValid(diag[2].second) )
            return player;

      } else if (row+1 ==col) {
         diag[3].first += score;
         if (abs(diag[3].first) ==4 && isValid(diag[3].second) )
            return player;

      } else if (row+2 ==col) {
         diag[4].first += score;
         if (abs(diag[4].first) ==4 && isValid(diag[4].second) )
            return player;

      } else if (row+3 ==col) {
        diag[5].first += score;
        if (abs(diag[5].first) ==4 )
            return player;
      }
  }

  bool isValid(map<pair<int,int>, int> verify) {
      int cnt=1;
      map<pair<int,int>, int>::iterator itr = verify.begin();
      int prev= itr->second;
      itr++;
      for (;itr!= verify.end(); ++itr){
        int score = itr->second ;
        if(prev == score){
            cnt++;
            if(cnt ==4)
              return true;
        } else {
           cnt=0;
        }
        prev = score;
      }
      return (cnt>=4);
  }
};

The logic behind calculating the diagonal is discussed here:
in a grid of 6x7, we can have diagonal score of >=4 only in following matrix indices:
If we have same scores in following (X,Y):

(2,0), (3,1), (4,2), (5,3)                ->  diag[0]
(1,0), (2,1), (3,2), (4,3), (5,4)
(0,0), (1,1), (2,2), (3,3), (4,4), (5,5)
(0,1), (1,2), (2,3), (3,4), (4,5), (5,6)
(0,2), (1,3), (2,4), (3,5), (4,6)
(0,3), (1,4), (2,5), (3,6)                ->  diag[6]

Naming things
Avoid single-character variable names, unless it's something that is very commonly used, like i for a loop index. Consider r and c, even if you guess it has something to do with coordinates, are those positions or sizes? I would personally use width and height here as unambiguous names for the size of the grid.
Note that in the constructor's initializer list, you can use the same name for the parameter as for the member variable, like so:
int width;
int height;

public:
Connect4(int width, int height): width(width), height(height) {...}

Not everyone likes this style; some people want to disambiguate between parameter and member variable by writing:
Connect4(int width_, int height_): width(width_), height(height_) {...}

And you also see projects where all member variables are prefixed with m_, so it will look like:
int m_width;
int m_height;

public:
Connect4(int width, int height): m_width(width), m_height(height) {...}

Other variable names that could be improved:

cnt -> count
itr -> it (the latter is more idiomatic)
diag -> diagonal
verify -> line

Create an enum class for the possibly contents of a grid cell
An enum, and even better an enum class, allow you to specify the possible values of a grid cell up front, with names instead of integer values. I suggest you write the following:
class Connect4 {
    enum class Cell: char {
       EMPTY,
       PLAYER1,
       PLAYER2,
    };
};

The above also explicitly sets the type to char, so only one byte is necessary to store it.
Storing the grid
There are several things wrong with how you store the grid. The data structure you are using is very inefficient, both for large and small grids, and for checking for a winning condition. The best way is to use a single vector to store all the grid cells:
std::vector<Cell> grid;

This ensures things are laid out very compactly in memory, and to look up an arbitrary grid cell, the CPU only has to do one multiplication and two additions, which is very cheap on contemporary processors, and as Toby Speight already mentioned, to check horizontal, vertical and diagonal lines is very easy as well; just calculate the start point, end point and stride to use, and then every step along the line only a single addition has to be done to find out the grid cell to check.
I would add a function to get a reference to a grid cell given its coordinates, like so:
Cell &at(int row, int col) {
    return grid[row * c + col];
}

Handling huge grids
The first thing to consider when dealing with huge grids/arrays/matrices is whether the data structure is going to be densely or sparsely populated. It's easy to think that it should be sparse, after all you start with the grid being completely empty. But consider that during the end game of connect 4, you will probably have used more than half of the grid cells, so a dense grid is likely the best way to store the state.
With the above Cell type, you use one byte per grid cell. Since a cell only has three possible states, you could pack up to five cells in a byte (since 3⁵ = 243) relatively easily, at the cost of some extra calculations when reading/writing a given cell. With this, a 100,000 by 50,000 grid fits in a little less than a gigabyte.
You might also think about compressing the grid somehow, but again consider what a connect 4 game looks like near the end. For sure there won't be runs of more than 3 cells with the same color in a row, otherwise there would already have been a winner. And you rarely see a repetitive pattern appearing. So it basically looks like noise, which is not compressible. Perhaps predictive coding could be used to compress the sequence of moves made by the players, but the compressed data would most likely not be in a form suitable for displaying the grid and checking winning conditions.
The only option I see for significantly reducing the amount of data is to consider that the bulk of the grid cells where players have already played will never be part of the 4 connected cells of the same color. For example, you could mark all cells that are more than 3 positions away (both horizontally and vertically) from any empty cells as being "dead". This way, that in principle, instead of having to store \$\mathcal{O}(W \cdot H)\$, you only have to store \$\mathcal{O}(W + H)\$ grid cells. But that would mean that you cannot show exactly all the moves made so far anymore.
Checking for a winning condition
Your idea of keeping a per-line score is unfortunately not really helpful. It is quite easy to create a line where the total score would be four, but there are no 4 consecutive cells with the same player, for example, representing players 1 and 2 with O and X respectively:
X O O X O O X O O X O O

There are 8 pieces of player 1, and 4 of player 2, so your way of counting score would result in 4 or -4, but clearly no player has won yet. However, if player 1 adds two more O's to this line:
X O O X O O X O O X O O O O

Then player 1 has 4 in a row, but the "line score" would be 6 or -6. So the line score is not useful.
The simple solution is to just always check the 9x9 area surrounding the latest move for a horizontal, vertical or diagonal line with 4 in a row. This might seem a bit wasteful in the very early game when only few moves have been made, it doesn't really matter in the long run.


Place integers into a vector, sum each adjacent pair, refill vector with only the sums of each pair i.e remove all the original data from the vector Is there a quicker/better way of doing this, mine works great for small vectors but if i get a vector with an arbitrarily large size this would probably take forever to run. Thanks.(P.S. its been a while since I've written any code so please excuse me for any sloppiness).

/*Fill a vector with integers,
sum each pair of adjacent elements,
and remove the pair of elements and place sum in the vector*/

#include <iostream>
#include <vector>

int main() {
    std::vector<int> numbers;
    std::cout << "Please enter a list of integers:\n";

    int num1;
    while (std::cin >> num1)
        numbers.push_back(num1);

    // starting from back of the container with integer k representing last element
    // i representing the second to last
    for (int i = numbers.size() - 2, k = numbers.size() - 1; i > -1; i-=2, k-=2) {
        numbers[i] += numbers[k];
        numbers.erase(numbers.begin() + k);
    }
}

It seems to me that a tiny bit of calculation can simplify the code quite a bit:

auto dest = numbers.size() % 2; // 0 if even, 1 if odd

for (auto src = dest; src < numbers.size(); src += 2, ++dest)
    numbers[dest] = numbers[src] + numbers[src + 1];

numbers.resize(dest);

Although it has the same big-O complexity (linear in both cases) I'd expect this to typically be at least a little faster than the solution @larkey posted, for a couple of reasons. First, it avoids conditional execution by simply computing the starting source/destination positions. Second, when it resizes the array down to the correct size after the additions, it does so in one fell swoop, so to speak, instead of doing a pop_back individually on each item being removed.

If the vector contained objects with non-trivial destructors, the destructor for each object being removed would have to be invoked, so the resize would be linear on the number of items being removed. Given that these are ints, there's really no dtor to run, so the resize can be done with constant complexity (and I'd expect most current implementations of std::vector to include the specializations necessary to actually achieve constant complexity too).

More importantly (at least in my opinion), this seems to me to simplify the code so it's considerably easier to read and understand the intent and result.


Add variable columns to INSERT statement I'm learning SQL using python library pymysql.

I wanted to make an INSERT statement that will update 3, 5 or 10 columns using the same code by just passing the Database name, the Columns name and the Values.

Thus I made this:

def insert_statement(db,cols,values):
    separator = ","
    separator.join(cols)
    statement = "INSERT INTO " + db + " (" + separator.join(cols) + ") VALUES ("
    aux = []
    for i in range(0,len(values)):
        aux.append("%s")

    statement = statement + separator.join(aux) + ")"
    print (statement)
    return statement

Passing the values, the function produces:

>>>INSERT INTO Publicaciones (item_id,title,price) VALUES (%s,%s,%s)

Which works but, is it there a more pythonic way?

You’re not using values except for its length. But this should be the same length than cols, so use that instead.

You also don't need a for-loop to build a list with the same element N times, list multiplication can handle that just fine.

Lastly, I would use f-strings or at least str.format instead of string concatenation, it is prettyier.

def insert_statement(db, columns):
    column_names = ', '.join(columns)
    placeholders = ', '.join(['%s'] * len(columns))
    return f'INSERT INTO {db} ({column_names}) VALUES ({placeholders})'

Usage:

>>> insert_statement('Publicationes', ['item_id', 'title', 'price'])
'INSERT INTO Publicationes (item_id,title,price) VALUES (%s,%s,%s)'

Depending on your calling site, you can also make columns a variable length argument, it may be easier to use:

def insert_statement(db, *columns):
    column_names = ', '.join(columns)
    placeholders = ', '.join(['%s'] * len(columns))
    return f'INSERT INTO {db} ({column_names}) VALUES ({placeholders})'

Usage:

>>> insert_statement('Publicationes', 'item_id', 'title', 'price')
'INSERT INTO Publicationes (item_id,title,price) VALUES (%s,%s,%s)'

But you should limit yourself to only use this function using trusted input. If anything comming from a user enters here, this is a vulnerability waiting to happen:

>>> user_input = "price) VALUES (42, 'foobar', 0.00); DROP TABLE Publicationes; -- Now this a vulnerability in disguise :"
>>> insert_statement('Publicationes', 'item_id', 'title', user_input)
"INSERT INTO Publicationes (item_id, title, price) VALUES (42, 'foobar', 0.00); DROP TABLE Publicationes; -- Now this a vulnerability in disguise :) VALUES (%s, %s, %s)"


Traversing un-ordered list and appending attibute values to the list item I have written this jQuery code to traverse the HTML markup. Basically, I am selecting data-icount attribute of each anchor tag and appending it to anchor text as (28) e.g. the ALL becomes ALL (38) in the example. 38 here is the sum of all the data-icounts under that li. Similarly, summers become summers (2) because it has two data-icount 0 and 2 so 0+2 =2.

Working example here.

I am sure there must be an improved process to do this - any comments/suggestions would be appreciated.

HTML

        ALL

                Fruits

                        Summers

Mango

Water melon

                        Winters

Tangerines

Pomegranate

Others

                Furniture

Home

Garden

Games

Playstation

Clothes

Here are some tips:

1) Don't create variables if they're only used once or are a don't perform complex operations.

Old Code:

var num_sub_uls = $sub_uls.length;
var count_icount = 0;
if(num_sub_uls > 0) {

New Code:

var count_icount = 0;
if($sub_uls.length) {

2) Provide the base when using parseInt().

Old Code:

count_icount = parseInt($child_a.attr('data-icount'));

New Code:

count_icount = parseInt($child_a.attr('data-icount'), 10);

3) Eliminate commented code sections.

Old code:

var sub_lis = $sub_uls.find('a');
//console.log('Total anchors: '+sub_lis.length);
sub_lis.each(function(index2) {

New Code:

var sub_lis = $sub_uls.find('a');
sub_lis.each(function(index2) {

4) Simplify if conditions by using Truthy & Falsey .

Boolean(undefined); // => false
Boolean(null); // => false
Boolean(false); // => false
Boolean(0); // => false
Boolean(""); // => false
Boolean(NaN); // => false

Boolean(1); // => true
Boolean([1,2,3]); // => true
Boolean(function(){}); // => true

More here

Old Code:

if(num_sub_uls > 0) {

New Code:

if(num_sub_uls) {

5) Use .append() to append text to an element in jQuery.

Old Code:

var a_txt = $child_a.text();
$child_a.text(a_txt+' ('+count_icount+') ');

New Code:

$child_a.children('a').append( ' (' + count_icount + ') ' );

6) Simplify your if and else conditions.

Check if count_icount is greater than 0, instead of checking for sub_list.length;

Old Code:

if(num_sub_uls > 0) {
    var sub_lis = $sub_uls.find('a');
    sub_lis.each(function(index2) {
        count_icount += parseInt($(this).attr('data-icount'));
    });
} else {
    count_icount = parseInt($child_a.attr('data-icount'));
}

New Code:

$sub_uls.find('a').each(function(index2) {
    count_icount += parseInt($(this).attr('data-icount'));
});
if( !count_icount ){
    count_icount = parseInt($this.children('a').attr('data-icount'), 10);
}

7) Use .data() instead of .attr() to access properties that begin with data-.

.data() Documentation

Example:

Fruits

Use

$("#thing2").data( "icount" ) == "9";

Instead of

$("#thing2").attr( "data-icount" ) == "9";

Final code:

$(function () {
    $("#tree li").each(function () {
        var $this = $(this),
        count_icount = $this.find("a").data('icount');

        $this.children('ul').find('a').each(function () {
            count_icount += parseInt($(this).data('icount'), 10);
        });
        if (!count_icount) {
            count_icount = parseInt($this.children('a').data('icount'), 10);
        }
        $this.children('a').append(' (' + count_icount + ') ');
    });
});

Demo: http://jsfiddle.net/GQkB8/2/


Using return value vs out parameters It's very popular to see code like this:

bool DoSomething([some arguments]);

You can't understand the result of this method by looking at its name (it could actually be a void).

What's the best approach to refactor such kind of methods?  What I have in mind is something like this:

void DoSomething([some argumens],out bool everythingIsOk);

What do you think?

EDIT

It seems that so many people prefer

bool TryDoSomething();

rather than

void DoSomething(out bool success);

But what if a method is going to have a result instead of being void?  Can we use int.TryParse pattern or we should return the result and throw an exception if something goes wrong?

You’re asking the wrong question. In reality, both methods are bad. If you need the success message, either throw an exception on failure (but only if this is really an exceptional scenario, or return the success and indicate this in the method name. For instance, by calling the method TrySomething.

In general, your example sounds like the method is actually doing two distinct things, otherwise there would be no confusion in the first place. But this is code smell anyway: every method should have only one purpose. If that is the case, then the meaning of the return value is always clear, no need to give it a name.


Amazon S3 pass-through controller I am building a website that uses Amazon S3 to host its image (and other) files.

I require that images displayed on the website look like they come from my site and not from Amazon (so we are talking on the request/response level).  What I have done is created a controller which all these files will pass through.  The controller should request the file from Amazon and pass it on, as if it's from the local website.

My concern is my usage of the response stream and error-handling.  Basically, if anything happens, I want it to simply return a 404.

public class RequestState
{
    public RequestState()
    {
        BufferSize = 1024;
        BufferRead = new byte[BufferSize];
        Request = null;
        ResponseStream = null;
    }

    public int BufferSize { get; private set; }
    public byte[] BufferRead { get; set; }
    public HttpWebRequest Request { get; set; }
    public HttpWebResponse Response { get; set; }
    public Stream ResponseStream { get; set; }
}

public class MediaController : AsyncController
{
    public void IndexAsync(string key)
    {
        if (string.IsNullOrWhiteSpace(key))
            throw new HttpException((int)HttpStatusCode.NotFound, "File not found.");

        AsyncManager.OutstandingOperations.Increment();

        var request = (HttpWebRequest)WebRequest.Create(string.Format(Constants.Urls.AmazonFileGetUrlFormat, AppSettings.AmazonS3BucketName, key));
        request.Method = "GET";
        request.ContentType = "application/x-www-form-urlencoded";

        var state = new RequestState { Request = request };

        request.BeginGetResponse(ResponseCallback, state);
    }

    private void ResponseCallback(IAsyncResult result)
    {
        try
        {
            var state = (RequestState)result.AsyncState;
            HttpWebRequest request = state.Request;

            state.Response = (HttpWebResponse)request.EndGetResponse(result);
            Stream responseStream = state.Response.GetResponseStream();
            state.ResponseStream = responseStream;

            AsyncManager.Parameters["imageInBytes"] = ReadFully(responseStream);
            AsyncManager.Parameters["contentType"] = state.Response.ContentType;
        }
        catch (WebException ex)
        {
            var state = (RequestState)result.AsyncState;

            if (state.Response != null)
                state.Response.Close();

            AsyncManager.Parameters["webException"] = ex;
        }
        catch (Exception ex)
        {
            var state = (RequestState)result.AsyncState;

            if (state.Response != null)
                state.Response.Close();

            throw;
        }
        finally
        {
            AsyncManager.Parameters["key"] = "key";
            AsyncManager.OutstandingOperations.Decrement();
        }
    }

    public static byte[] ReadFully(Stream input)
    {
        var buffer = new byte[16 * 1024];

        using (var stream = new MemoryStream())
        {
            int read;
            while ((read = input.Read(buffer, 0, buffer.Length)) > 0)
            {
                stream.Write(buffer, 0, read);
            }
            return stream.ToArray();
        }
    }

    [OutputCache(Duration = 600, VaryByParam = "key")]
    public FileContentResult IndexCompleted(string key, byte[] imageInBytes, string contentType, WebException webException)
    {
        if (webException != null && webException.Status == WebExceptionStatus.ProtocolError)
        {
            var response = webException.Response as HttpWebResponse;

            if (response != null)
            {
                throw new HttpException((int)response.StatusCode, response.StatusDescription);
            }

            throw new HttpException((int)HttpStatusCode.NotFound, "File not found.");
        }

        if (imageInBytes == null || imageInBytes.Length == 0)
        {
            throw new HttpException((int)HttpStatusCode.NotFound, "File not found.");
        }

        return File(imageInBytes, contentType);
    }
}

I don't understand the point in using this pattern:

    catch (WebException ex)
    {
        var state = (RequestState)result.AsyncState;

        if (state.Response != null)
            state.Response.Close();

        AsyncManager.Parameters["webException"] = ex;
    }
    catch (Exception ex)
    {
        var state = (RequestState)result.AsyncState;

        if (state.Response != null)
            state.Response.Close();

        throw;
    }
    finally
    {
        AsyncManager.Parameters["key"] = "key";
        AsyncManager.OutstandingOperations.Decrement();
    }

When it looks like it could be simplified to this:

    catch (WebException ex)
    {
        AsyncManager.Parameters["webException"] = ex;
    }
    finally
    {
        var state = (RequestState)result.AsyncState;

        if (state.Response != null)
            state.Response.Close();

        AsyncManager.Parameters["key"] = "key";
        AsyncManager.OutstandingOperations.Decrement();
    }

The following may stem from my lack of familiarity with Amazon S3:

I'm not sure what functionality your ReadFully method offers over Stream.Read(Stream.Length) or, more safely, Stream.Read(Stream.Length - Stream.Position) except potentially causing issues if the stream does not have a length of multiplies of 16x1024.

Speaking of which, you've hard-coded that 16x1024 in a magic number:

var buffer = new byte[16 * 1024];

I would recommend converting these values to consts so you can change them later with ease.


Linked list with deep copy constructor I've created a linked list template class which contains a copy constructor that performs a deep copy of the list.

LinkedList.hpp

template <class T>
class LinkedList {
protected:
    struct Node {
        T value;
        Node *next;
        Node(T val, Node *nxt=NULL) {
            value = val;
            next = nxt;
        }
    };
    Node *head;
public:
    LinkedList();
    LinkedList(LinkedList &obj);
    ~LinkedList();

};

LinkedList.cpp

template <class T>
LinkedList<T>::LinkedList() {
    head = NULL;
}

template <class T>
LinkedList<T>::LinkedList(LinkedList &obj) {
    if (obj.head == NULL) {
        head = NULL;
    }
    else {
        head = new Node(obj.head->value);
        Node *current = head;
        Node *objHead = obj.head;
        Node *currentObj = objHead;
        while (currentObj->next != NULL) {
            current->next = new Node(currentObj->next->value);
            currentObj = currentObj->next;
            current = current->next;
        }
    }
}

template <class T>
LinkedList<T>::~LinkedList() {
    Node *current = head;
    while (current != NULL) {
        Node *garbage = current;
        current = current->next;
        delete garbage;
    }
}

template class LinkedList<int>;
template class LinkedList<double>;
template class LinkedList<bool>;
template class LinkedList<string>;

From what I can tell, the default copy constructor implements this functionality naturally, but I wanted to see if I could create a deep copy of a linked list myself using a self-defined copy constructor.

When I test this using:

LinkedList<int> ll;
ll.add(1);
ll.add(2);
ll.add(3);
ll.add(4);

LinkedList<int> ll2 = ll;
ll.add(5);

ll.displayList();
ll2.displayList();

It prints 1,2,3,4,5 and then 1,2,3,4.  I believe this is correct.  Are there any improvements I can make to the implementation of this?

A few fairly obvious points:
Prefer nullptr over NULL
Unless you really need to support older compilers, you're generally better off using nullptr in new code. If you do use nullptr, you can still support older compilers if really necessary--it's basically just a class that provides conversions for pointer to T and pointer to member.
Prefer member initializer lists over assignment in a constructor
The most obvious example here would be to replace your node constructor:
Node(T val, Node *nxt=NULL) {
    value = val;
    next = nxt;
}

...with one using a member initializer list:
Node(T value, Node *next = nullptr) :
    value(value),
    next(next)
{}

Note that yes, in this case, although the parameter being used to initialize it have the same name, but the compiler can keep track of which is which anyway.
Rule of three/five/zero
Note that in virtually every case, if you provide a destructor and a copy ctor, you also want a copy assignment operator. If you want the better efficiency you can get from move construction/move assignment available in C++11 and later, you'll probably want to add overloads of those that accept rvalue references. I consider it open to debate whether the rule of zero really applies well in this case--I've written a linked list that used smart pointers, but I was never entirely happy with it. It's clumsy but workable in the case of a singly linked list. For a doubly linked list, it's...basically unmanageable.
Side Note
Contrary to your statement, if you don't include a copy ctor, you probably won't get correct results with this sort of class. In particular, the compiler will generate code that does a shallow copy, so only the head pointer in the LinkedList object will be copied. This means if you copy a linked list, you'll end up with both pointing to a single head node. In itself that's not necessarily a problem, but when either of those is destroyed, all the nodes in the list will be destroyed, so in essence you've just destroyed both linked lists, not just the one you intended to. Attempting to use or destroy the one that wasn't supposed to have been destroyed yet will give undefined behavior.


Project Euler #1 Sum of multiples of 3 and 5 python implementation Given:

  If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.
  Find the sum of all the multiples of 3 or 5 below 1000.

My Solution:

def sum_multiples(num, limit):
    """ Calculates the sum of multiples of a given number.
    Args:
        num: The multiple.
        limit: The upper limit.
    Returns:
        The sum of the terms of a given multiple.
    """
    sum = 0
    for i in xrange(num, limit, num):
        sum += i
    return sum

def sum(limit):
    return (sum_multiples(3, limit) +
            sum_multiples(5, limit) -
            sum_multiples(15, limit))

print sum(1000)

Is there any better or more pythonic way? I have used generators for a very large calculation. Also, how can I get the running time for a given N?

It'd be more pythonic to use the built-in sum function instead of writing one yourself:

sum(xrange(num, limit, num))

However, this is still doing way too much work -- you don't need to do a for-loop for a series sum, there's a closed-form solution:

def sum_multiples(n, lim):
    last = (lim - 1) // n
    return n * (last) * (last+1) // 2

EDIT: Also, don't call your own function sum, since you hide the built-in one that way.

def sum35(limit):
    return (sum_multiples(3, limit) +
            sum_multiples(5, limit) -
            sum_multiples(15, limit))

print sum35(10)   # 23
print sum35(1000) # 233168


Find first unique number from array I was asked to create a function that return first UNIQUE integer from given array of integers. In a case of success the function return the unique number in a case of failure return -1. The complexity should be \$O(n \log(n))\$. How can this be improved?

function solution(A) {

    var returnNumber = -1;
    var tempArray = new Array();;

   for(i=0; i < A.length; i++){

      var newIndex = A[i];
     if( tempArray[newIndex] === undefined){
        tempArray[newIndex] = 1;
     }
     else{
        tempArray[newIndex] += 1;
     }
    }
   for(j = 0; j < A.length; j++){
     if(tempArray[A[j]] == 1){
       returnNumber = A[j];
       break;
     }
   }
    return returnNumber;
}

//var A =  [1, 4, 3, 3, 1, 2, 0, 0]; //returns 4
var A =  [22, 25, 3, 3, 1, 2, 0, 0,100,22,25,1,2]; // returns 100
solution(A)

I like your answer. It looks a bit like a counting sort, which is very fast. I expect it's faster than a few of the other answers, although Joseph's answer is cleaner and more readable for sure. If I'm not mistaken your implementation is O(n) (which is excellent).

A couple (minor) things:

Using a dictionary/ hashtable would make more sense than using an array to store the frequency of each number. I'm not sure if creating an array then setting the 10,000th index (eg, if var A=[10000, 1, 1, 5]) as something takes a lot of memory (I don't use js a lot).

if( tempArray[newIndex] === undefined){

can just be

if(!tempArray[newIndex]){

Formatting. Your indentation is all over the place. This is probably due to the copy paste into the text box, but if not this is the biggest thing to make your code more readable.

You have 2 semi-colons:

var tempArray = new Array();;

Lastly, a few comments and better variable names could really help. "A" should be something more descriptive. Ps: The variable A in

function solution(A) {

doesn't have to be the same name as

var A =  [22, 25, 3, 3, 1, 2, 0, 0,100,22,25,1,2]; // returns 100
solution(A)

Though it works fine if it's the same name. I wasn't sure if this was a coincidence or because you didn't know this, but I thought I'd mention it.

That's about all from me... I don't code a lot in js, so any comments are welcome if I've made mistakes above (but I think most of it's pretty applicable).


Calculating speed from a Pandas Dataframe with Time, X, and Y columns I'm trying to calculate speed between consecutive timepoints given data that is in a '.csv' file with the following columns: "Time Elapsed", "x", and "y". The ultimate goal is to get the data into a format where I can plot "Time Elapsed" vs. "speed"

I'm fairly sure that my implementation is doing what I want, but it's certainly possible (and likely) that I overlooked something. I'm also wondering whether there are faster/more efficient ways (in Python) to perform these calculations?

import pandas as pd
import numpy as np

def calculate_speeds(path_to_csv):
    data_df = pd.read_csv(path_to_csv)

    xy = data_df[['x', 'y']]

    b = np.roll(xy, -1, axis=0)[:-1]
    a = xy[:-1]
    dxy = np.linalg.norm(a - b, axis=1)

    dt = (np.roll(data_df['Time Elapsed'], -1) - data_df['Time Elapsed'])[:-1]

    speeds = np.divide(dxy, dt)

    speed_df = pd.DataFrame(data={'Time Elapsed':data_df['Time Elapsed'][:-1],'Speed':speeds})

    return speed_df

The code reads good enough and is pretty straightforward; you should consider improving its quality by adding documentation in the form of a docstring explaining what kind of data it expects. Speaking as such, you should extract the I/O part out of the function and have it being fed the required data, for better reusability and testing.

One other thing to note is that you perform the same roll twice on different columns of the same dataframe. You can combine these operations and write the equation in a more ... equationy-ish way:

import pandas as pd
import numpy as np

def calculate_speeds(positions_over_time):
    time = 'Time Elapsed'

    movements_over_timesteps = (
        np.roll(positions_over_time, -1, axis=0)
        - positions_over_time)[:-1]

    speeds = np.sqrt(
        movements_over_timesteps.x ** 2 +
        movements_over_timesteps.y ** 2
    ) / movements_over_timesteps[time]

    return pd.DataFrame({
        time: positions_over_time[time][:-1],
        'Speed': speeds,
    })

if __name__ == '__main__':
    data_df = pd.read_csv(path_to_csv)
    calculate_speeds(data_df)


Function to get specified key from /proc/cpuinfo On Linux, the file /proc/cpuinfo returns a set of key-value pairs, where the key and value are separated by a colon and each pair has its own line. It's a bit more complicated than that in reality, but for my needs that's as complicated as it needs to be. (I have only one processor core.)

My goal is to write a function in bash that takes the key for one of those items and returns the value corresponding to that key. Here's my current function:

function get_cpuinfo_prop () {
    TARGET_LINE=$(cat /proc/cpuinfo | grep ^$1)
    IFS=':'
    read -ra PARTS <<< "$TARGET_LINE"
    PROP_VALUE=${PARTS[1]}
    echo $PROP_VALUE
}

And you'd call it like this:

$(get_cpuinfo_prop 'key_name')

Is there a better way of doing it? How could I improve this code? Additionally, I think this might be returning extra whitespace before the actual value. How can I trim those characters off the beginning/end?

Useless use of cat
Grep can open files all by itself, cat isn't necessary in most circumstances. Also, quote everything - that's save you some debugging when you need to handle spaces in them - like for the "model name" property.
target_line=$(grep -m1 ^"$1" /proc/cpuinfo)

-m1 to stop at the first match, so you won't get weird results when you run your script on a machine with more CPUs. That's not portable unfortunately (not in POSIX), so if you don't have that, the usual options are: pipe to head -n 1, use awk instead and exit early, sed and exit early. The latter two can do it all in one go too.
Use your shells string manipulations for simple things
Assuming you want everything after the :, you can simply do:
value="${target_line##*: }"

See How do I do string manipulations in bash? for more like this, or the POSIX parameter expansion reference.
function keyword not necessary
It's less portable and doesn't buy you anything, so just omit it. See difference between “function foo() {}” and “foo() {}” for more details.
Variable names don't need to be in all caps
Personal preference of course, but especially for locals, lowercase is good - keep uppercase for globals/environment variables.
So:
get_cpuinfo_prop () {
  target_line=$(grep -m1 ^"$1" /proc/cpuinfo)
  echo "${target_line##*: }"
}


Normalise list of N dimensional numpy arrays I have a list of N dimensional NumPy arrays.

num_vecs = 10
dims = 2

vecs = np.random.normal(size=(num_vecs, dims))

I want to normalize them, so the magnitude/length of each vector is 1. I can easily do this with a for-loop.

norm = np.linalg.norm(vecs, axis=1)

for dd in range(dims):
    vecs[:, dd] /= norm

assert np.allclose(np.linalg.norm(vecs, axis=1), 1.)

But how do I get rid of the for-loop?

The trick is to use the keepdims parameter.

import numpy as np

num_vecs = 10
dims = 2

vecs = np.random.normal(size=(num_vecs, dims))

vecs /= np.linalg.norm(vecs, axis=1, keepdims=True)

assert np.allclose(np.linalg.norm(vecs, axis=1), 1.)


Reimplementation of call() method in JavaScript I was try to reimplement the native call method in JavaScript.
Here we need to handle the object which we pass in the call function and additional params. I handle the additional params using eval() method. Is there any more efficient way to write it?

var name = {
    name: "JavaScript",
  version: "6",
}

function printName(location, district){
    alert(this.name + ", " + this.version + ", " + location + ", " + district);
}

Function.prototype.myCall = function(...args){
  var param = args.slice(1),
      paramLength = param.length,
      paramString = "JSONarg.myFun(";
  for(var i = 1; i <= paramLength; i++){
    paramString += "args["+i+"],";
  }
  paramString += ")";

  if(typeof this != 'function'){
    throw new Error(this + " is not a Function");
  }

  var JSONarg =  {
    ...args[0],
    myFun: this
  }
  return eval(paramString);
}

printName.myCall(name, "Chrome", "browser");

Review

To be quick I will just add comments to the code with some alternatives

// if you add to a built in prototype use Object.defineProperty or Object.defineProperties
// using the default settings
// Function.prototype.myCall = function(...args){ // use 2 parameters
Function.prototype.myCall = function(name, ...args){

  // var param = args.slice(1),  // is now second parameter

  // Should be a constant
  //    paramLength = param.length,
  const  paramLength = args.length;

  // not really a param string. Rather its a eval function
  //  paramString = "JSONarg.myFun(";
  var evaledFunc = "JSONarg.myFun(";

  // The variable if defined in the loop should be let
  // Now starts at index 0
  //for(var i = 1; i <= paramLength; i++){
  for (let i = 0; i < paramLength; i++) {  // add space for ( and ) {

    evaledFunc += "args[" + i + "],";  // use spaces between operators
  }
  evaledFunc += ")";

  // This function is a prototype of Function. You would have to go hard
  // to have the next statement be true. Just let the native exception
  // generate the error
  /*if(typeof this != 'function'){
    throw new Error(this + " is not a Function");
  }*/

  // Does not change, thus should be a const
  // var JSONarg =  {
  const JSONarg =  {  // would be best defined at the top of the function

  // args[0] is now the param name
  //  ...args[0],
      ...name,
    myFun: this
  // }  // there should be a ; here
  };
  return eval(evaledFunc);
}

Removing the comments and some minor mods we get...

function myCaller(name, ...args) {
    const JSONarg =  {...name, myFun: this};
    var func = "JSONarg.myFun(";
    for (let i = 0; i < args.length; i++) {
        func += "args[" + i + "],";
    }
    func += ")";
    return eval(func);
}

Rather than eval you could also have use new Function

Object.defineProperty

To prevent problems (myCall showing up in strange places) and as it is in a very often used native type.( All functions get the new property). enumerable = true can slow every function call just a little depending on JS engine and version.

Add to prototype safely using Object.defineProperty. Default accessor configurable, enumerable, writable are false

if (! Function.prototype.myCall) {
    Object.defineProperty(Function.prototype, myCall, {value: myCaller});
}

Function.bind

With all that said there is a much easier way to do the same. Function.bind will bind a functions with an object (sets the functions this)

function printName(location, district){
    alert(this.name + ", " + this.version + ", " + location + ", " + district);
}
printName.bind({...name})( "Chrome", "browser");

// or just use original name obj

printName.bind(name)( "Chrome", "browser");

The bound function is a new reference, the original function remains unbound

const printNameBound = printName.bind(name);

printNameBound("Chrome", "browser");

printName("Chrome", "browser"); // will fail as not bound to name object

Function.call or Function.apply

You can also use Function.call or Function.apply to do the same. In this case there is no new reference to the function created. The binding is temporary.

printName.call(name, "Chrome", "browser");

// or

printName.apply(name, ["Chrome", "browser"]); // args as array


Replace outside comma with newline I'm looking for a better alternative to replace insert + gsub.

hobbies = "Hobbies: \nsports (basketball, gym), foods (hamburger, steak, pasta), reading, movie, justfortesting((test1, test2), test3)"
print "\n\nBefore: \n\n" + hobbies
counter = 0
for i in 0...hobbies.length do
    case hobbies[i]
    when '('; counter += 1
    when ')'; counter -= 1
    when ','; hobbies.insert i + 1, "\n" if counter == 0
    end
end
counter = nil
hobbies.gsub!(",\n", "\n")
hobbies.gsub!("\n ", "\n")
print "\n\nAfter: \n\n" + hobbies

Output:

Before:

Hobbies:
sports (basketball, gym), foods (hamburger, steak, pasta), reading, movie, justfortesting((test1, test2), test3)

After:

Hobbies:
sports (basketball, gym)
foods (hamburger, steak, pasta)
reading
movie
justfortesting((test1, test2), test3)

How about:

hobbies = "Hobbies: \nsports (basketball, gym), foods (hamburger, steak, pasta), reading, movie, justfortesting((test1, test2), test3)"
print "\n\nBefore: \n\n" + hobbies

counter = 0
hobbies = hobbies.chars.map do |char|
  next "\n" if char == ',' and counter == 0
  counter += 1 if char == '('
  counter -= 1 if char == ')'
  char
end.join
counter = nil

print "\n\nAfter: \n\n"
hobbies.each_line do |line|
  puts line.lstrip
end


Cracking the coding interview 1.7 - Rotate matrix by 90 degrees The question asks to rotate an image/matrix by 90 degrees. Ideally it asks to do it in place but before that I decided to do it using extra space because this is my first time working with matrices.

question:
while using extra space, could this have been any better?

def rotate_m1(matrix, d):
    """
    with extra space
    for each row in the orginal matrix, the items position in that row
    will be its row in the new matrix. Its position in the new row will be determined
    by 'l' (matrix row length), which decrements after each loop.

    e.g.
    1,2
    3,4

    the loop goes through and maps the top row to the outside of a new matrix. i.e.

    i = 0 # row
    j = 0 # col // its postion in the row
    l = d-1 # d = the dimensions of the matrix
    matrix[i][j] = 1

    in the new matrix the postion of matrix[i][j] (1) will be:

    new_matrix[j][l]; j being its postion in its original row, so in this case 1 was in postion 0 in
    the original matrix, and l is the outermost row of the new matrix becuse (in this example) during
    the first loop l = 2-1.

    after the first row is mapped, we take one away from l to map the next row to the row before
    the outer most row in the new matrix (d-2)
    """

    m2 = [[0 for x in range(d)] for y in range(d)] # construct new matrix
    l = d-1

    for i in range(len(matrix)):
        for j in range(len(matrix[i])):
            m2[j][l] = matrix[i][j]

        l -= 1
    print m2

input/ouptut:

input:
[[1,2,3],
[4,5,6],
[7,8,9]]

output:
[[7, 4, 1],
[8, 5, 2],
[9, 6, 3]]

I've added some extra info about how I came to the solution in the code comment.

Most of your code looks good. I have a few remarks, though.

The name of your function rotate_m1 is misleading. What does the m1 mean at all? Additionally, your function prints the result. It should not do that. Instead, it should either rotate the matrix in-place, or alternatively return the rotated matrix. That way, you can use the result further:

print(rotate(rotate(matrix)))

With your current code, thisis not possible.

The docstring of your function is very long. This docstring should be written for a specific target audience, which is the users of the function. The users only need to know:

"""Returns a copy of the given matrix, rotated clockwise by 90 degrees.

Example: [[1, 2, 3], [4, 5, 6]] is rotated to [[4, 1], [5, 2], [6, 3]]
"""

The implementation details do not belong in the behavioral description of the function.

Note that I chose a non-square example on purpose. If you rotate the matrix in-place, the matrix has to be square, but when you create a copy, you can also handle arbitrary rectangular matrices. Your code currently doesn't do this, but it should.

My standard disclaimer about CtCI: Don't trust that book. It contains many typos and inefficient, English-America-centric code. The "solution" code is often not indented properly, variables are not named carefully, edge cases are forgotten, and so on. I would not hire someone who learned from that book alone.


The finest integers are palindromes Challenge:

  Reverse the digits of a number and add it to the original
  until the number is a palindrome

Specifications:

  Your program should accept as its first argument a path to a filename.
  Each line in this file is one test case containing an integer n < 10,000.

  Assume each test case will always have an answer and
  that it is computable with less than 100 iterations.

  For each line of input, print a line of output
  containing the number of iterations to compute the palindrome
  and the resulting palindrome, on one line separated by a single space.

Solution:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;

public class ReverseAndAdd {
    public static void main(String[] args) throws FileNotFoundException {
        Scanner input = new Scanner(new File(args[0]));

        while (input.hasNextLine()) {
            findAndPrintPalindrome(input.nextLine());
        }
    }

    private static void findAndPrintPalindrome(String line){
        System.out.println(getPalindrome(line));
    }

    private static String getPalindrome(String line) {
        int reverseCount = 0,
            currentNum = Integer.parseInt(line)
        ;

        while (!isPalindromic(currentNum)) {
            currentNum += reverse(currentNum);
            reverseCount++;
        }

        return reverseCount + " " + currentNum;
    }

    private static boolean isPalindromic(int s) {
        return s == reverse(s);
    }

    private static int reverse(int n) {
        return Integer.parseInt(
            new StringBuilder(Integer.toString(n)).reverse().toString()
        );
    }
}

Sample Input:

  8
  125
  195

Sample Output:

  0 8
  1 646
  4 9339

I'm looking to pinpoint:

Anything I could alter to optimize this program.
General feedback on good vs poor practices in my code.

Indentation and formatting
    currentNum = Integer.parseInt(line)
    ;
Is there a chance of an MAX_INT overflow for reverse(). Seems unlikely for the given contraints though.
Since n is an integer, so negatives are allowed. In that case .reverse() will cause problem. Please check the constraints.
getPalindrome() should return a palindrome instead of an output you wish to have. Instead either have the function name changed. (Though not a big concern but ensure that when I see a function name, it shoul dbe intuitive so that I do not have to check the code for the actual output.)
Test cases:

Include 0, 9998 (max non palindromic n).
Negatives (if constraints dont mention so).
Invalid input. (Not handled by your program and may not be required but would help for a production code and not a programming challenge).

Optimisation:

reverse() though takes negligible time (in nanoseconds) but can be improved to handle all above cases along with speed in calculation. Ideone link

private static int reverseInt(int n) {
    int rev = 0;
    while(n>0){
        rev = (rev*10)+n%10;
        n=n/10;
   }
   return rev;
}


Generating random numbers I'm trying to refactor the following code and could use some suggestions.  Basically, three functions generating random numbers in different ways are called:

import java.util.Random;
public final class JP_RandomNumbers {

    /**
     * @param aArgs
     */

    public static void main(String[] aArgs) {
        genRandomNumbers1();
        genRandomNumbers2();
        genRandomNumbers3();
    }

    private static void genRandomNumbers1() {
        log("Generating 10 random integers in range 0..99.");

        //note that a single Random object is reused here
        Random randomGenerator = new Random();
        for (int idx = 0; idx < RANGE ; ++idx) {
            int randomInt = randomGenerator.nextInt(100);
            log("Generated :" + randomInt);
        }
        log("Done.");

    }

    private static void genRandomNumbers2() {
        //Generating random numbers in a specific range
        int START = 1;
        int END = 10;

        Random random = new Random();
        for (int idx = 1 ; idx <= 10 ; ++idx) {
            showRandomInteger(START, END, random);
        }
        log ("Done");

    }

    private static void showRandomInteger(int aStart, int aEnd, Random aRandom) {
        if(aStart > aEnd) throw new IllegalArgumentException("Start cannot exceed End.");

        //get the range, casting to avoid overflow problems
        long range = (long)aEnd - (long) aStart + 1;
        //compute a fraction of the range , 0 <= frac <= range
        long fraction = (long) (range * aRandom.nextDouble());
        int randomNumber = (int)(fraction + aStart);
        log("Generated : " + randomNumber);
    }

    private  void genRandomNumbers3() {
        double MEAN = 100.0f;
        double VARIANCE = 5.0f;
        for(int idx = 1; idx <= 10; ++idx){
            log("Generated : " + String.valueOf(getGaussean(MEAN,VARIANCE)));
        }
        log ("Done");

    }

    private double getGaussean(double aMean, double aVariance){
        return aMean + fRandom.nextGaussian() * aVariance;
    }

    //Private
    private static final int RANGE = 10;
    private Random fRandom = new Random();
    private static void log( String aString) {
        System.out.println(aString);
    }

}

Output:

Generating 10 random integers in range 0..99.
Generated :74
Generated :99
Generated :75
Generated :15
Generated :32
Generated :31
Generated :96
Generated :61
Generated :80
Generated :89
Done.
Generated : 2
Generated : 4
Generated : 1
Generated : 9
Generated : 9
Generated : 5
Generated : 9
Generated : 4
Generated : 3
Generated : 7
Done
Generated : 96.16324820042756
Generated : 97.4298343909747
Generated : 105.18654272922421
Generated : 90.56523225378042
Generated : 100.26474837892167
Generated : 99.13288502577953
Generated : 105.99193834986883
Generated : 101.21081536019025
Generated : 99.48291677422242
Generated : 100.55524156214658
Done

The number generation technique is generally sound.  I just have the following minor suggestions:

The idiomatic way to repeat something 10 times is

for (int idx = 0 ; idx < 10 ; ++idx) { … }

Use the fRandom instance variable; you shouldn't need to create new Random() in each function.
You hard-coded values in each function; those constants should probably be pulled out to the class level.
showRandomInteger() should be changed to return a random value instead of logging it.
The compiler will implicitly do String.valueOf() for you, so you can just write

log("Generated : " + getGaussean(MEAN,VARIANCE));

getGaussean() should be spelled getGaussian().


Custom Error Classes in OOP I have a class which parses a .csv file and converts it into successful rows and errors. Currently I have 12 different kind of errors (mail is not valid, missing column x, row is duplicate etc.).

Each error has a custom message. I also need to know in other classes what kind of error a row was, so a simple string is not enough. So far I am using this error class:

<?php

namespace App\Registration\GroupUploads;

/**
 * Represents all possible errors that may occur during a group upload.
 */
class Error
{

  const USEREXISTS  = 7;
  const MAILINVALID = 1;
  const SEALUSED    = 2;
  const MIXEDUP     = 3;
  // ..

  protected $id;
  protected $options;

  /**
   * Each error has a unique ID
   * @param int $id
   * @param array $options optional
   */
  protected function __construct($id, $options = null)
  {
    $this->id      = $id;
    $this->options = $options;
  }

  /**
   * Get Message of current object
   * @return string
   */
  public function getMessage()
  {
    switch ($this->id) {
      case static::USEREXISTS:
        return 'User already exists';
        break;

      case static::MAILINVALID:
        return 'Mail is not valid';
        break;

      case static::SEALUSED:
        return 'Sealnr is already used by other user.';
        break;

      case static::SEALUSED:
        return 'They messed up and mixed the seals inbetween orgas.';
        break;

      // ...

      default:
        return 'No message provided for error code';
        break;
    }
  }

  /**
   * Create error for an existing user
   * @return bool
   */
  public static function getDuplicateUserError()
  {
    return new static(static::USEREXISTS);
  }

  /**
   * Check if class is duplicate user error
   * @return bool
   */
  public function isDuplicateUserError()
  {
    return $this->id == static::USEREXISTS;
  }

  // ...

}

There are 12 constants, each representing the id of a specific error. There is one method getMessage() which uses a switch with 12 cases. Then I have 12 messages of the kind getErrorMessageX and another 12 methods isThisErrorNameX.

I feel that this is kind of messy and redundant. Also their might be me more error cases in the future, which will bloat up the class even more.

I thought about creating 12 separate classes, each class named by the error name. Something like that:

<?php

namespace App\Registration\GroupUploads;

/**
 * Error for existing user
 */
class UserExistsAlreadyError
{

  public function getMessage()
  {
    return 'User already exists';
  }

}

So instead of if($error->isDuplicateUserError()) I would write if($error instanceof UserExistsAlreadyError) .

However, I think creating 12 different classes, where each consists only of 12 lines, is a bit too much. Is there a better solution then those two extremes?

This is how I ended up doing it now:

<?php

namespace App\Registration\GroupUploads\Errors;

/**
 * Error that appear during parsing csv file
 * @var [type]
 */
abstract class Error
{
  const USEREXISTS  = 7;
  const MAILINVALID = 1;
  const SEALUSED    = 2;
  const MIXEDUP     = 3;
  const QUALMISSING = 20;

  public $id;
  protected $msg;

  /**
   * Get error message
   * @return [type] [description]
   */
  public function getMsg()
  {
    return $this->msg;
  }

  /**
   * Error if user existed before registration
   * @param  [type] $name [description]
   * @return [type]       [description]
   */
  public static function userAlreadyExists($name)
  {
    $error = new Error();
    $error->id   = static::USEREXISTS;
    $error->msg  = "User " . $name .  " already exists";
    return $error;
  }

  /**
   * Mail is not valid
   * @param  [type] $mail [description]
   * @return [type]       [description]
   */
  public static function mailInvalid($mail)
  {
    $error = new Error();
    $error->id   = static::MAILINVALID;
    $error->msg  = "The mail " . $mail .  " is not valid";
    return $error;
  }

  //..
}

This way the getMsg method is short. Also adding a new error only requires to add one method instead of two (and no modification of the getMsg function).

To identify an error I call if($error->id == Error::USEREXISTS){...}.

Having the id was quite important, because I wanted to store the errors in DB.


"The Love Letter Mystery" - storing and comparison of strings The problem goes like this.

Change the strings provided by user to form palindromes. To do this, 2
rules are:
(a) Reduce the value of a letter, e.g. Change 'd' to 'c',
but cannot change 'c' to 'd'.
(b) In order to form a palindrome, you
can repeatedly reduce the value of a letter until the letter becomes
'a'. Once a letter has been changed to 'a', it can no longer be
changed.
Each reduction in the value of any letter is counted as a single
operation. Find the minimum number of operations required to convert a
given string into a palindrome.

This is a HackerRank problem: the-love-letter-mystery
While I was solving it, I made a lot of mistakes. I was using char *str[n] and gets() to input the string. I thought it would work, but I realised after long time that I am not actually allocating any space at all. Later I came up with 2-d array char[n][1000] for storing the strings and scanf seemed to work for me.
Here is my solution:
int main() {

    int i, n, len, pos, ops, end;
    scanf("%d",&n);
    char str[n][10000];
    char a,b;
    for (i=0;i<n;i++) {
        scanf("%s",str[i]);
    }
    for (i=0;i<n;i++) {
        len = strlen(str[i]);
        pos = 0;
        end = len - 1;
        ops = 0;
        while ( pos < end) {
               a = str[i][pos];
               b = str[i][end];
               if ( a != b ) {
                   if ( b > a )
                       ops = ops + (b -a);
                   else
                       ops = ops + (a -b);
               }
               pos++;
               end--;
        }
        printf("%d\n", ops);
    }

return 0;
}

This works perfectly. I also have to assume in the program that the strings can be of max length 1000 as that is one of the constraint.
How can I improve this solution without making it too complex?

Honestly, this seems pretty good. There might be a faster way to do it, but it's not coming to mind.

I'd maybe not declare your int i that early on, and use the standard format instead, as that's what's expected.

for(int i = 0; i < n; i++)

As well as this, I'd give n a more descriptive name, such as num_lines or something similar. The same can be said for the other variables. ops and pos could give more information about what they're doing.


Setting up radio button click ActionListeners Instead of writing four almost identical setOnClickListener method calls, I opted to iterate over an array, but I'm wondering if this was the best way to do it. Do you have any suggestions for improving it?

/* Set up the radio button click listeners so two categories are not selected
   at the same time. When one of them is clicked it clears the others.
*/
final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};
for (int i = 0; i < 4; i++) {
    final int k = i;
    buttons[i].setOnClickListener(new OnClickListener() {
        @Override
        public void onClick(View v) {
            for (int j = 0; j < 4; j++) {
                if (buttons[j] != buttons[k]) {
                    buttons[j].setChecked(false);
                }
            }
        }
    });
}

What GUI framework is this? Is the View v being passed to the OnClick method actually the clicked RadioButton? If so, here are some changes to consider

final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};

final OnClickListener onClickHandler =
    new OnClickListener() {
        @Override
        public void onClick(final View v) {
            final RadioButton checkedButton = (RadioButton) v;
            for (final RadioButton button : buttons) {
                if (button != checkedButton) {
                    button.setChecked(false);
                }
            }
        }
    };

for (final RadioButton button : buttons) {
    button.setOnClickListener(onClickHandler);
}

If View v != the checked radio button, then:

final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};
for (final RadioButton thisButton: buttons) {
    thisButton.setOnClickListener(
        new OnClickListener() {
            @Override
            public void onClick(final View v) {
                for (final RadioButton button : buttons) {
                    if (button != thisButton) {
                        button.setChecked(false);
                    }
                }
            }
        }
    );
}


Calculation of the distance to the next space station I've made a solution for this problem:

  Task:

  Write a program for calculation of the distance to the next space station.

  Algorithm:

  First base is located at the distance equal to
  SMK from the beginning of the path (Sirius in our case). The next
  station is located at the distance of F(SMK) from the first. Third
  station — at the distance of F(F(SMK)) and so on. Here \$F\$ — is the Top
  Secret Mars Function (TSMF).  Its value is the sum of the cubes of
  digits its argument in decimal notation (for example \$F(12) = 1^3 + 2^3
> = 9\$). So if the distance from the (\$I − 1\$)-th to \$I\$-th stations is \$X\$, then the distance between \$I\$-th and (\$I + 1\$)-th stations is \$F(X)\$. Your
  cruiser is located between (\$N − 1\$)-th and \$N\$-th space stations at the
  distance of \$L\$ from (\$N − 1\$)-th station. Taking \$N\$, \$K\$ (Secret Mars Key)
  and \$L\$ as input your program should output the distance \$M\$ between your
  cruiser and \$N\$-th station. Oh, by the way! The value of SMK is always
  divisible by 3.

  Input:

  Number \$T\$ (\$2 ≤ T ≤ 33333\$) is placed in the first line of input — it is the number of tests for your program. It followed by the next \$T\$ lines.

  Each of these \$T\$ lines contains 3 integer numbers:

  \$N\$ (\$2 ≤ N ≤ 33333\$), \$K\$ (\$3 ≤ K ≤ 33333\$) and \$L\$ (\$L ≥ 1\$)

  Output:

  \$T\$ lines. \$I\$-th line contains the calculated value of \$M\$ for \$I\$-th test case.

  It has to run in less than one second and take up less than 16 megabytes of memory.

  Sample:

  http://img33.imageshack.us/img33/502/81219587.jpg

Here is the compiled code:

static void Main()
{
    int T = Convert.ToInt32(Console.ReadLine());
    for (int k = 0; k < T; k++)
    {
        string[] split = (Console.ReadLine()).Split(new Char[] { ' ' });
        int N = Convert.ToInt32(split[0]);
        double K = 0;
        string Kst = split[1];
        double POST = 0;

        for (int i = 0; i < N - 1; i++)
        {
            for (int j = 0; j <= Kst.Length - 1; j++)
            {
                int Kindex = int.Parse(Convert.ToString(Kst[j]));
                K = K + Kindex*Kindex*Kindex;
            }
           POST = K;
            Kst = Convert.ToString(K);
            K = 0;
        }
        Console.WriteLine(POST - Convert.ToDouble(split[2]));
    }
}

But this code doesn't go through the time limit of 1.0 second.  How can I improve the speed of this solution?

Here are a few tips:

Instead of getting each character, creating a new string from it, and then parse the string into a number, just get the character and convert the character code into the number.
Alternatively, parse the whole number once, then get the digits numerically using modulo.
There are only ten digits, so you can easily set up an array of precalculated cube values.
K is a double, but there are no floating point operations here. Use an int, or a long if needed.

Example:

int T = Convert.ToInt32(Console.ReadLine());
int[] cube = { 0, 1, 4, 9, 16, 25, 36, 49, 64, 81 };
for (int k = 0; k < T; k++) {
  string[] split = Console.ReadLine().Split(' ');
  int N = Convert.ToInt32(split[0]);
  int Kst = Convert.ToInt32(split[1]);
  int K = 0;
  for (int i = 0; i < N - 1; i++) {
    K = 0;
    while (Kst > 0) {
      K += cube[Kst % 10];
      Kst /= 10;
    }
    Kst = K;
  }
  Console.WriteLine(K - Convert.ToInt32(split[2]));
}


Showing all possible permutations from a set I wrote a small program which shows all possible permutations of choosing m integers from the set \${1, 2, ..., n}\$, as follows:

def perm(nlist, m):
    """
    Find all possible permutations of choosing m integers from a len(n)-list.

    INPUT:
    @para nlist: a list of length n.
    @para m: a integer.

    OUTPUT:
    All permutations arranged in ascending order lexicographically.
    """
    if m > len(nlist):
        return
    if m == 0 or m == len(nlist):
        return [[]]
    results = []
    for list_i in nlist:
        temp = list(nlist)
        temp.remove(list_i)
        results.extend(map(lambda x: [list_i] + x, perm(temp, m-1)))
    return results

However, this code is inefficient. Perhaps the recursive structure is not well-designed. How can I improve this code?

As noted by 200_success in the comments, the m == len(nlist) check is a bug that can be fixed by simply removing the condition.

Also, if m > len(nlist): return is almost redundant because you would get an empty list as the result anyway in that case. (The deepest levels of recursion give empty results when the list runs out and the emptiness propagates all the way up)

Turning the function into a generator simplifies it and speeds it up at the same time: from 10.9 ms to 6.6 ms for the case perm(range(6), 6). (Of course I wrapped that in list() when timing the generator version)

def perm(nlist, m):
    if m == 0:
        yield []
        return

    for list_i in nlist:
        temp = list(nlist)
        temp.remove(list_i)
        for p in perm(temp, m-1):
            yield [list_i] + p


Haskell Listing Directories in Working Directory I've got this fairly simple program to print out each subdirectory of the working dir.

import System.Directory (listDirectory, doesDirectoryExist)

main = do
    dirs <- getDirectories
    mapM_ putStrLn dirs

-- Get a list of directories within the working directory
getDirectories :: IO [FilePath]
getDirectories = do
    contents <- listDirectory "."
    fIO doesDirectoryExist contents

-- Filter a list with an IO predicate.
-- (In this use case, queries file system for directory status.)
fIO :: (a -> IO Bool) -> [a] -> IO [a]
fIO f [] = return []
fIO f (x:xs) = do
    matches <- f x
    rest <- fIO f xs
    return $ (if matches then [x] else []) ++ rest

I also attempted this implementation of the fIO function, and I'm not sure if it's legitimately less clear or if it only appears that way to me because I'm not used to reading Haskell-style syntax yet.

fIO f (x:xs) = do
    matches <- f x
    fIO f xs >>= return . ((if matches then [x] else []) ++)

Critique Request

Being a Haskell newbie, I'm sure a professional would change some things about this implementation. I'm curious what sticks out as awkward, non-idiomatic, or opaque. I'm also curious which implementation of the two fIOs (if either) you would select, and why.

Instead of fIO f xs >>= return . ((if matches then [x] else []) ++), you can use <$>:

([x | matches] ++) <$> fIO f xs

Applicative style would allow you to compress all this yet further:

fIO f (x:xs) = (\matches -> ([x | matches] ++)) <$> f xs <*> fIO f xs

But in the end:

fIO = filterM


Convert an integer to its Roman numeral equivalent I got sent a challenge from a friend;

  You are given a file that contains numbers, one number per line. Convert the numbers from the file into roman numerals.

Test input:
45
65
23
98
123
7754

Expected Output:
XLV
LXV
XXIII
XCVIII
CXXIII
MMMMMMMDCCLIV

I think I've succeeded in doing this successfully, and would like some critque on what I've done so far:

import sys

ROMAN_NUMERAL_TABLE = [
    ("M", 1000), ("CM", 900), ("D", 500),
    ("CD", 400), ("C", 100),  ("XC", 90),
    ("L", 50),   ("XL", 40),  ("X", 10),
    ("IX", 9),   ("V", 5),    ("IV", 4),
    ("I", 1)
]

def convert_to_roman(number):
    """ Convert an integer to Roman
    >>> print(convert_to_roman(45))
    XLV """
    roman_numerals = []
    for numeral, value in ROMAN_NUMERAL_TABLE:
        while value <= number:
            number -= value
            roman_numerals.append(numeral)

    return ''.join(roman_numerals)

if __name__ == '__main__':
    with open(sys.argv[1]) as data:
        for line in data.readlines():
            print("{} to roman numerals: {}".format(line.rstrip(), convert_to_roman(int(line.rstrip()))))

Key points that I would like to focus on (obviously feel free to critique the entire program)

Using the tuples for the list, is this good practice, or would a dict have worked better?
While converting the integers to the roman numerals is there a easier way to do this, right now my program will take a long time, for an integer that is higher than 12345679990 is there a way to speed up the processing for extremely high integers?

Using the tuples for the list, is this good practice, or would a dict have worked better?

Yes, in this case I think it's ok. Alternative is using orderdict or dict but sorting it by value which is not that good.

Now to make it work faster you can use division and multiplication. So it will look like this:

def convert_to_roman(number):
    """ Convert an integer to Roman
    >>> print(convert_to_roman(45))
    XLV """
    roman_numerals = []
    for numeral, value in ROMAN_NUMERAL_TABLE:
        count = number // value
        number -= count * value
        roman_numerals.append(numeral * count)

    return ''.join(roman_numerals)