Reversing each word of a string while keeping the case index I was given a simple requirement for reversing a string. The requirement is as follows:

In any programming language, create an input that accepts "My name is Albert McDonald." and outputs the reversed value "Ym eman si Trebla DlAnodcm."

I got it to work like this:

string expectedInput = "My name is Albert McDonald.";

//Prompt the user to enter "My name is Albert McDonald."
Console.WriteLine("Please enter the sentence \"" + expectedInput + "\"");
Console.WriteLine();

//Wait for the users input.
string inputText = Console.ReadLine();

//Convert the input to a char array.
List<char> characters = inputText.ToList<char>();

//Get the list of uppercase indexes
List<int> upperIndexes = new List<int>();
for (int index = 0; index < characters.Count; index++)
{
    if (Char.IsUpper(characters[index]))
        upperIndexes.Add(index);
}

//Remove the period at the end
inputText = inputText.Substring(0, inputText.Length - 1);

//Get the reversed array of words.
string[] words = inputText.Split(' ').Reverse().ToArray();

//Join the array back to a string.
string revesedWords = string.Join(" ", words);

//Reverse the entire string and make it all lower case.
string result = new string(revesedWords.Reverse().ToArray()).ToLower();
StringBuilder sb = new StringBuilder(result);

//Change the case of the original index
foreach (int index in upperIndexes)
{
    char newCharacter = char.Parse(result.Substring(index, 1).ToUpper());
    sb[index] = newCharacter;
}

//Output the resutlt
Console.WriteLine("Your name in reverse by word is: \"" + sb.ToString() + ".\"");
Console.WriteLine();

//Inform the user how to close the screen.
Console.WriteLine("Press any key to close the screen.");
Console.ReadLine();

Is there a better way in terms of lines of code and performance-wise?

If you aim for fewer lines of code, you could of course put everything into a single line, but that would not helpful at all.

However, you could shorten your code a bit by using a simple regular expression and some linq-fu, something like:

var s = yourInputString;

// split string into words
var splitted = Regex.Split(s, @"(\W)").Select(r => r.AsEnumerable()).ToArray();

// reverse each word
var reversed = splitted.Select(sp => sp.Reverse().ToArray());

// adjust case
var zipped = splitted.Zip(reversed, (a, b) => string.Join("", a.Select((c, i) => char.IsUpper(c) ?
                                                                                char.ToUpper(b[i]) :
                                                                                char.ToLower(b[i]))));

// join back to string
var result = string.Join("", zipped);

I would not care about performance in this case, because it would be pointless.

Also, reversing a string with Reverse().ToArray() will corrupt your string if it contains surrogate characters, but this is probably out of scope of your task.


All tangled up in IF and ELIF and TRY I'm working on a small program to add or remove certain entries from the Windows registry, and I'm getting all tangled up in IF and TRY conditions. I've spent hours chasing bugs, usually logical ones, trying to accomplish what is conceptually very simple. The more I work on and "improve" the code so that it works in more and more conditions, the more tortured and complicated the IF tests get, until I can't tell which is up and down anymore. Surely there must be a better way! (even for my little brain)

This is all I'm trying to do:

When asked to install:

If our python version and installpath is in registry: do nothing.
If our python version and different installpath: do nothing.
If our python is not there, add it.

When asked to remove:

If our python is not there: do nothing
If our python version and different installpath: do nothing.
If our python version and installpath is in registry: remove.

But my code for just the removal part looks like the below (the whole thing is here). It works, more or less, but it just doesn't feel right. I don't know what to do to make it cleaner. Your viewpoints are appreciated. Thanks.

def remove():
    ''' see if any existing registrations match our python version and register ours if not '''

    if CurrentUser:
        match = True if our_version in CurrentUser else False
        versions = CurrentUser
    elif AllUsers:
        match = True if our_version in AllUsers else False
        versions = AllUsers
    else:
        print '\nOur version (%s) not registered to "%s", skipping...' % (our_version, versions[our_version])

    try:
        if match:
            print '\nVersion matches ours, calling deRegisterPy...'
            deRegisterPy(pycore_regpath,our_version)
    except:
        raise

def deRegisterPy(pycore_regpath, version):
    ''' remove this python install from registry '''
    pycore_regpath = pycore_regpath + version   # e.g. 'SOFTWARE\Python\Pythoncore\2.7'
    try:
        reg = OpenKey(HKEY_LOCAL_MACHINE, pycore_regpath)
        installpath = QueryValue(reg, installkey) # win32
        if installpath == our_installpath:
            print '\nexisting python matches ours, removing...\n'
            # print '(%s vs %s)' % (installpath, our_installpath)
            for subkey in ['\\InstallPath', '\\PythonPath']:
                DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath + subkey)
            DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath)
            print "--- Python %s, %s is now removed!" % (our_version, our_installpath)
        CloseKey(reg)
    except EnvironmentError:
        print 'EnvironmentError', EnvironmentError()
        raise
        return
    except WindowsError:
        print "Strange, we've hit an exception, perhaps the following will say why:"
        print WindowsError()
        raise
        return
    CloseKey(reg)

# main
if args['action']=='install':
    install()
elif args['action']=='remove':
    remove()

def remove():
    ''' see if any existing registrations match our python version and register ours if not '''

    if CurrentUser:

The python style guide reserves CamelCase for class names. If this is a global constant it should be CURRENT_USER. If its not a global constant, it shouldn't be accessed as a global variable.

        match = True if our_version in CurrentUser else False

Just use match = our_version in CurrentUser

        versions = CurrentUser
    elif AllUsers:
        match = True if our_version in AllUsers else False
        versions = AllUsers

    else:
        print '\nOur version (%s) not registered to "%s", skipping...' % (our_version, versions[our_version])

    try:
        if match:
            print '\nVersion matches ours, calling deRegisterPy...'
            deRegisterPy(pycore_regpath,our_version)
    except:
        raise

What??? the try..except does nothing if you just raise the error right away

def deRegisterPy(pycore_regpath, version):

Python style guide recommends lower_case_with_underscores for functions names

    ''' remove this python install from registry '''
    pycore_regpath = pycore_regpath + version   # e.g. 'SOFTWARE\Python\Pythoncore\2.7'
    try:
        reg = OpenKey(HKEY_LOCAL_MACHINE, pycore_regpath)
        installpath = QueryValue(reg, installkey) # win32

Why are you taking the time to point out that its win32 here?

        if installpath == our_installpath:
            print '\nexisting python matches ours, removing...\n'
            # print '(%s vs %s)' % (installpath, our_installpath)

Don't leave dead code in comments

            for subkey in ['\\InstallPath', '\\PythonPath']:
                DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath + subkey)
            DeleteKey(HKEY_LOCAL_MACHINE, pycore_regpath)
            print "--- Python %s, %s is now removed!" % (our_version, our_installpath)
        CloseKey(reg)
    except EnvironmentError:
        print 'EnvironmentError', EnvironmentError()

What are you trying to do here? You construct a new EnviromentError object, and then print it. But that's a pointless thing to do

        raise

If you are just going to raise the error again, not a whole lot of point in printing it anyways
            return

Do you realize that nothing after the raise will be executed, so this return will have no effect?

    except WindowsError:
        print "Strange, we've hit an exception, perhaps the following will say why:"
        print WindowsError()

You seem to be confused about how to catch errors, you want:

except WindowsError as error:
    print error

But really you don't need to do that: just let the exceptions fly and python will print them when it exits due to the error

        raise
        return

Again, the return will never be reached

    CloseKey(reg)

If an exception occurs, this will never be called. You should put it in a finally block or something

# main
if args['action']=='install':
    install()
elif args['action']=='remove':
    remove()

You should really put logic like this in a main function.

Any my rewrite of your whole linked file. No testing has been done. But it should give you something of an idea of what you can do.

#!/usr/bin/env python

import sys
from _winreg import *

import argparse, sys
import os.path

PYTHON_VERSION = "%d.%d" % sys.version_info[0:2]

# the registry key paths we'll be looking at & using
PYCORE_REGISTRY_PATH = ("SOFTWARE","Python","Pythoncore")
INSTALL_KEY = "InstallPath"
PYTHON_KEY = "PythonPath"
PYTHONPATH = ";".join( os.path.join(sys.prefix, sub) for sub in ["", "Lib", "DLLs"])

class RegisteryKeyNotFound(Exception):
    pass

class RegisteryKey(object):
    def __init__(self, hive):
        self._key = key

    def _path_from(self, segments):
        if not isinstance(segments, tuple):
            segments = (segments,)

        return "\\".join(segments)

    def __getitem__(self, segments):
        try:
            subkey = _winreg.OpenKey(self._key, self._path_from(segments))
        except WindowsError:
            # should really check the error
            # to make sure that key not found is the real problem
            raise RegisteryKeyNotFound(path)
        return RegisteryKey(subkey)

    def get_or_create(self, segments):
        subkey = _winreg.CreateKey(self._key, self._path_from(segments))
        return RegisteryKey(subkey)

    def value(self, segments):
        return _winreg.QueryValue(self._key, self._path_from(segments))

    def delete(self, segments):
        _winreg.DeleteKey(self._key, self._path_from(segments))

    def set_value(self, segments, value):
        _winreg.SetValue(self._key, self._path_from(segments), REG_SZ, value)

    def __iter__(self):
        index = 0
        while True:
            try:
                yield RegisteryKey(EnumKey(key, index))
            except WindowsError:
                # add check to make sure correct here was gotten
                break
            else:
                index += 1

    def __del__(self):
        _winreg.CloseKey(self._key)

HIVE_LOCAL_MACHINE = RegisteryKey(HKEY_LOCAL_MACHINE)
HIVE_CURRENT_USER = RegisteryKey(HKEY_CURRENT_USER)

def get_existing(hive):
    ''' retrieve existing python registrations '''

    try:
        key = hive[PYCORE_REGISTRY_PATH]
    except RegisteryKeyNotFound:
        return {}

    versions = {}
    for version in key:
        versions[version] = version.value(INSTALL_KEY)                           # e.g. {'2.7' = 'C:\\Python27'}

    return versions

def register_python(version):
    ''' put this python install into registry '''
    try:
        reg = HIVE_LOCAL_MACHINE[PYCORE_REGISTRY_PATH][version]
    except RegisteryKeyNotFound:
        reg = HIVE_LOCAL_MACHINE.get_or_create(PYCORE_REGISTRY_PATH).get_or_create(version)
        reg.set_value(INSTALL_KEY, sys.prefix)
        reg.set_value(PYTHON_KEY, PYTHONPATH)
        print "--- Python %s is now registered to %s!" % (PYTHON_VERSION, sys.prefix)
    else:
        print reg.value(INSTALL_KEY)

    if reg.value(INSTALL_KEY) == sys.prefix and reg.value(PYTHON_KEY) == PYTHONPATH:
        print "=== Python %s is already registered!" % (PYTHON_VERSION)
    else:
        print "*** Unable to register!"
        print "*** You probably have another Python installation!"

def deregister_python(version):
    ''' remove this python install from registry '''
    reg = HIVE_LOCAL_MACHINE[PYCORE_REGISTRY_PATH][version]
    install_path = reg.value(INSTALL_KEY)
    if install_path == sys.prefix:
        print '\nexisting python matches ours, removing...\n'
        for subkey in ['InstallPath', 'PythonPath']:
            reg.delete(subkey)
        reg.delete()
        print "--- Python %s, %s is now removed!" % (PYTHON_VERSION, sys.prefix)

def install(installed_versions):
    ''' see if any existing registrations match our python version, and if not, register ours '''

    print '\n...installing'

    if PYTHON_VERSION in installed_versions:
        print '\nOur version (%s) already registered to "%s", skipping...' % (PYTHON_VERSION, installed_versions[PYTHON_VERSION])
    else:
        print '\nPutting python from environment into registry...\n'
        register_python()

def remove(installed_versions):
    ''' see if any existing registrations match our python version and register ours if not '''
    #print args
    if PYTHON_VERSION in installed_versions:
        deregister_python()
    else:
        print '\nOur version (%s) not registered, skipping...' % (PYTHON_VERSION)

# @url http://stackoverflow.com/questions/4042452/display-help-message-with-python-argparse-when-script-is-called-without-any-argum
# display the usage message when it is called with no arguments
class MyParser(argparse.ArgumentParser):
    def error(self, message):
        sys.stderr.write('error: %s\n' % message)
        self.print_help()
        sys.exit(2)

def main():
    parser=MyParser()
    parser.add_argument('action', help='one of "install" or "remove" ')
    args = parser.parse_args()

    current_user_versions = get_existing(HIVE_CURRENT_USER)
    local_machine_versions = get_existing(HIVE_LOCAL_MACHINE)

    print '\nFound in Current User:'
    for key, value in current_user_versions.items():
        print "\t%s - %s" % (key, value)
    print '\nFound in All Users:'
    for key, value in local_machine_version.items():
        print "\t%s - %s" % (key, value)

    all_versions = {}
    all_versions.update(current_user_versions)
    all_versions.update(local_machine_versions)

    if args.action == 'install':
        install(all_versions)
    elif args.action == 'remove':
        remove(all_versions)
    else:
        print '\nInvalid action specified. I only understand "install" and "remove". '

if __name__ == '__main__':
    main()

Some notes:

The _winreg interface is rather low level. You can make code a lot clearer by wrapping it in something more pythonic. I've only taken a first stab at an interface, and it probably could be done a lot better.
You split out CurrentUser and LocalMachine but treated them pretty much exactly the same. The code can be simplified by merging them into a giant dictionary of versions
you remove python from LocalMachine but not CURRENT_USER. That seems odd but I've not changed it.


Convert Tail-Call Recursive Code to Loop So, I'm in the midst of writing a programming language interpreter in Python, and one of the problems I'm concerned about is that since I've been using a lot of recursions to implement the interpreter, I'll get a stack overflow if the size of the code is too large. For example, I have a piece of code that evaluates an and operation. Since the language I'm implementing is a logic programming language (like Prolog), it needs to try out all the options, so it consumes a generator and loops over it looking at the rest of the and operation. Behold:

def evaluate_and(and_clauses, binds={}):
    if and_clauses == []:
        yield binds
    else:
        head, *tail = and_clauses
        possible = evaluate(head, binds)
        for p in possible:
            yield from evaluate_and(tail, p)

The issue with this is that if the and operation is buried deep within the program, and the call stack is already large, then if the and has too many clauses the interpreter will crash.

If anyone could suggest a way to make this not possible, that would be amazing!

PS: I have an even bigger problem, which is that since recursion is the only way to do looping in a logic programming language if you loop more than a 100 times in my language, it creates a stack overflow in my interpreter.

A quick search for "python tail call optimization" results in this SO answer which has a PyPI package, tco. Converting your code to return lists rather than generators should be all you need.

  recursion is the only way to do looping in a logic programming language

This is wrong.

If you want it to stay as a generator, then you'll have to manually handle the stack. The following doesn't account for order so if you need that then you'll have to figure that out yourself.

def evaluate_and(and_clauses, binds={}):
    stack = [(clauses, binds)]
    while stack:
        and_clauses, binds = stack.pop()
        if and_clauses == []:
            yield binds
        else:
            head, *tail = and_clauses
            possible = evaluate(head, binds)
            for p in possible:
                stack.append((tail, p))


Segment tree implementation I'm learning segment trees and their implementation. For the purpose, I tried solving the problem on CodeChef.

Full code here

My tree implementation is as follows:

#include <bits/stdc++.h>

#define _MAX (1<<17)
#define MAX (_MAX<<1)
using namespace std;

struct node
{
    int tails;
    int heads;
    void Merge(node &A,node &B) // Merge two nodes
    {
        tails=A.tails+B.tails;
        heads=A.heads+B.heads;
    }
    void CreateLeaf() // Creates a leaf
    {
        tails=1;
        heads=0;
    }
};

node ST[MAX];   // Segment tree ST

/* res is the node storing the result
   Information for range [l,r] is present in ST[idx]
   Query is required for range [u,v]  */

void Query(node &res,int l,int r,int u,int v,int idx)
{
    if(l==u && r==v)
        {
            res=ST[idx];
            return;
        }
    else
    {
        int mid=(l+r)/2;
        if(mid>=v)
            Query(res,l,mid,u,v,2*idx);
        else if(mid<u)
            Query(res,mid+1,r,u,v,2*idx+1);
        else{
            node left,right;
            Query(left,l,mid,u,mid,2*idx);    // left child is 2*idx
            Query(right,mid+1,r,mid+1,v,2*idx+1);   // right child is 2*idx+1
            res.Merge(left,right);
        }
    }
}

void init(int idx,int l,int r) // initialises the tree
{
    if(l==r)
        {
            ST[idx].CreateLeaf();
            return;
        }
    else
    {
        int mid=(l+r)/2;
        init(2*idx,l,mid);
        init(2*idx+1,mid+1,r);
        ST[idx].Merge(ST[2*idx],ST[2*idx+1]);
    }
}

/* Updates the value of a single node.
    When we flip the coins, no. of heads and tails get swapped */

void updatevalue(node &A)
{ swap(A.tails,A.heads); }

/* Updates the node ST[idx] and all the nodes descending the node ST[idx]
    ST[idx] stores information for range [l,r]  */

void update(int l,int r,int idx)
{
    if(l==r) // returns as soon as we reach a leaf node
        {
            updatevalue(ST[idx]);
            return;
        }
    else
    {
        updatevalue(ST[idx]);
        int mid=(l+r)/2;
        update(l,mid,2*idx); // Updates left child
        update(mid+1,r,2*idx+1); // Updates right child
    }
}

/* Range to be updated is [u,v]
    ST[idx] stores information for range [l,r] */

void UpdateRange(int l,int r,int u,int v,int idx)
{
   if(u<=l && r<=v)
         {
            update(l,r,idx);
                // As soon as required node is found, update the node and all its descendant
            return;
         }
    else
    {
        int mid=(l+r)/2;
        if(mid>=v)
            UpdateRange(l,mid,u,v,2*idx);
        else if(mid<u)
            UpdateRange(mid+1,r,u,v,2*idx+1);
        else
        {
            UpdateRange(l,mid,u,mid,2*idx);
            UpdateRange(mid+1,r,mid+1,v,2*idx+1);
        }
            ST[idx].Merge(ST[2*idx],ST[2*idx+1]);
    }
}

int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    int n,q,x,y,z;
    node res;
    cin>>n>>q;
    init(1,0,n-1); /* ST[1] stores information for [0,n-1] */
    while(q--)
    {
        cin>>x>>y>>z;
        if(x==1)
        {
            Query(res,0,n-1,y,z,1);
            cout<<res.heads<<"\n";
        }
        else
        {
           UpdateRange(0,n-1,y,z,1);
        }
    }
    return 0;
}

I've read many online sources and applied the segmented tree to the best of my knowledge, however my implementation is shown slow in accordance with time constraints for the problem. Please tell where my code is lacking. I feel my range update is a slow function but I can't find any method to speed it up as when we flip a coin, we have to update all ranges having that coin. Any better implementation for Range update or query is welcomed.

In your if-else logic :

you can get rid of a few returns
if you have something like if (condition) { some_code(); some_more_code1(); } else { some_code(); some_more_code2(); }, you can write it by moving the common code : some_code() if (condition) { some_more_code1(); } else { some_more_code2(); } (assuming that the code doesn't affect the evaluation of the condition).
if you have a test with an empty then block, you can rewrite it with the negation to have a test with no else block.

Your CreateLeaf() seems to be something that could be done in a constructor.

Your updatevalue() seems to be something that could be done in the node class.

You can move the declaration of int x, y, z to a smaller scope.

Query should probably return a value instead of taking a reference.

Merge could take const references.

Using using namespace std is usually frowned upon.

Taking (most of) these comments (and some others) into account, I have :

#include <bits/stdc++.h>

#define _MAX (1<<17)
#define MAX (_MAX<<1)
using namespace std;

class Node
{
    public:
    Node(): tails(1), heads(0) {}

    void Merge(const Node &A,const Node &B) // Merge two Nodes
    {
        tails=A.tails+B.tails;
        heads=A.heads+B.heads;
    }
    /* Updates the value of a single Node.
   When we flip the coins, no. of heads and tails get swapped */
    void Swap()
    {
        swap(tails,heads);
    }
    int getNbHeads() { return heads; }
    private:
    int tails;
    int heads;
};

Node ST[MAX];   // Segment tree ST

/* res is the Node storing the result
   Information for range [l,r] is present in ST[idx]
   Query is required for range [u,v]  */

Node Query(int l,int r,int u,int v,int idx)
{
    if(l==u && r==v)
    {
        return ST[idx];
    }
    else
    {
        int mid=(l+r)/2;
        if(mid>=v)
            return Query(l,    mid,u,v,2*idx);
        else if(mid<u)
            return Query(mid+1,r,  u,v,2*idx+1);
        else{
            Node res;
            res.Merge(
                Query(l,    mid,u,    mid,2*idx), // left child is 2*idx
                Query(mid+1,r,  mid+1,v,  2*idx+1) // right child is 2*idx+1
            );
            return res;
        }
    }
}

void init(int idx,int l,int r) // initialises the tree
{
    if(l!=r)
    {
        int mid=(l+r)/2;
        init(2*idx,l,mid);
        init(2*idx+1,mid+1,r);
        ST[idx].Merge(ST[2*idx],ST[2*idx+1]);
    }
}

/* Updates the Node ST[idx] and all the Nodes descending the Node ST[idx]
   ST[idx] stores information for range [l,r]  */

void update(int l,int r,int idx)
{
    ST[idx].Swap();
    if(l!=r)
    {
        int mid=(l+r)/2;
        update(l,mid,2*idx); // Updates left child
        update(mid+1,r,2*idx+1); // Updates right child
    }
}

/* Range to be updated is [u,v]
   ST[idx] stores information for range [l,r] */

void UpdateRange(int l,int r,int u,int v,int idx)
{
    if(u<=l && r<=v)
    {
        // As soon as required Node is found, update the Node and all its descendant
        update(l,r,idx);
    }
    else
    {
        int mid=(l+r)/2;
        if(mid>=v)
            UpdateRange(l,    mid,u,v,2*idx);
        else if(mid<u)
            UpdateRange(mid+1,r,  u,v,2*idx+1);
        else
        {
            UpdateRange(l,    mid,u,    mid,2*idx);
            UpdateRange(mid+1,r,  mid+1,v,  2*idx+1);
        }
        ST[idx].Merge(ST[2*idx],ST[2*idx+1]);
    }
}

int main()
{
    ios_base::sync_with_stdio(false);
    cin.tie(NULL);
    int n,q;
    cin>>n>>q;
    init(1,0,n-1); /* ST[1] stores information for [0,n-1] */
    while(q--)
    {
        int x, y, z;
        cin>>x>>y>>z;
        if(x==1)
        {
            cout<< Query(0,n-1,y,z,1).getNbHeads()<<"\n";
        }
        else
        {
            UpdateRange(0,n-1,y,z,1);
        }
    }
    return 0;
}

Also, I forgot to tell it but it would probably be worth encapsulating the logic around ST in some kind of class or at least pass the array as an argument as global variable can make things hard to track/maintain.


Pausing script while waiting for Docker container and printing updates I'm looking to improve it in brevity, readability, and simplicity. Basically, I'm just looking for a more elegant solution. What improvements can I make?

i=0
while true; do
    OUTPUT="$(docker inspect --format='{{json .State.Health.Status}}' su-apache)"
    if [ $OUTPUT = '"healthy"' ]; then
      echo
      break
    fi
    echo -en "\r\033[KWaiting $i seconds for su-apache to finish starting... Current health: $OUTPUT"
    let "i++"
    sleep 1
done

echo 'Environment started successfully.'

If you don't mind using Bash,
then it will be more elegant to replace the while loop with a counting for loop:

for ((i = 0; ; i++)); do
    # ...
done

I'm guessing that you're using Bash anyway from the let "i++" syntax,
which will not work with simpler shells like Dash.

Note that the various flags of echo are not portable.
For a more portable solution, replace it with printf.

Lastly, the indentation is inconsistent: some blocks are indented with 4 spaces, others with 2. It would be better to indent consistently (I like 4 spaces).


Finding reversed word pairs I wrote this code to find opposites of words (i.e., words backwards). Input is a list of words. If this list contains a word + its opposite, then it should form a pair. The output should be a list of tuples (pairs).
The output should:

contain every pair of words only once - it should contain either ('reed', 'deer') or ('deer', 'reed'), but not both.
not contain palindromes (words which are opposite to themselves) - e.g. 'refer'

def find_opposites(lst):
    if lst == [] or len(lst) == 1:
        return []
    else:
        opposites = [] # contains all words backwards (except palindromes)
        result = []
        for word in lst:
            if word != word[::-1]: # avoid palindromes
                opposites.append(word[::-1])

        for word in opposites:
            if word in lst:
            # if word in both opposites list and original list,
            # then it should form a pair with its opposite

                tup = (word[::-1], word) # create tuple
                if tup[::-1] not in result: # append tuple only if its opposite in not already in result
                    result.append(tup)

        return result

I have tried to substitute the list opposites with a dictionary as I know lists are slower to process, but it still takes a very long time.
Please let me know if you have any ideas on how to make it run faster, as it takes ages when I run on my corpora which contains over 11.000 words. Thanks a lot :)

As others have mentioned, your program runs slowly because it is doing a lot of membership tests against fairly large lists, which is expensive. If you replace your lists with sets, that alone should improve performance by quite a bit.
Creating a set from the input list of words is also important to ensure the correctness of your algorithm. As @Tweakimp pointed out, we only want to find pairs of opposites within a collection of unique words, to avoid the possibility of duplicates of the same pair from showing up in the results.
Below is a simple algorithm which calculates all the pairs in one pass.

Create unique_words: set[str] from the input list of words.
Create results: list[tuple[str, str]] for our results. This will be our return value.
Create words_seen: set[str] to track which words from unique_words we've seen so far.
Iterate over unique_words; for each word, also calculate reversed_word.

If we haven't seen reversed_word yet, then we don't know if word has a matching pair (yet). So we add word to words_seen.
If we have seen reversed_word, then we just found a matching pair, so we append (word, reversed_word) to results.

Return results.

def find_opposites(words):
    unique_words = set(words)

    results = []
    words_seen = set()

    for word in unique_words:
        reversed_word = word[::-1]

        # if we have already seen reversed_word,
        # then we found a new pair of opposites: (word, reversed_word)
        if reversed_word in words_seen:
            results.append((word, reversed_word))
        else:
            words_seen.add(word)

    return results

Because we start with the set unique_words, we actually don't need to check explicitly for the palindrome case. This is because in order for a palindrome to end up in results, there would need to be at least two instances of the same palindrome in unique_words, which is impossible.


Pattern for storing object of varying type I have a situation where I need to store fields of differing types of some data structures along with some similar metadata (The application takes data from one of many sources, some data possibly coming from more than one source, and then does stuff with it).

The solution I came up with was to create a class that would hold each piece of data which I could then add to each object. The DataParam object can be loaded with any data type that's determined when it's first constructed. When we get or set it later, those methods are passed a class type similar to what we are getting or setting. Since we always check against the value we originally "typed" our object with, we can never accidentally set it to some other type of value (and mess everything up).

The reason these are all generic methods and the entire class isn't a generic is because I need to store DataParam in a container and that wouldn't be possible if the class itself was a generic (I can't store a DataParam<String>, and a DataParam<int> in one container).

public class DataParam
{
    public String name
    public boolean wasUpdated
    private String type
    private boolean isArray
    private Object data;

    public <T> DataParam(String name, Class<T> typeClass)
    {
        this.name = name;
        type = typeClass.getName();
        isArray = typeClass.isArray();
        data = null;
        wasUpdate = false;
    }
    public <T> T get(Class<T> typeClass)
    {
        if(type.equals(typeClass.getName()) && isArray == typeClass.isArray())
            return typeClass.cast(data);
        else
            return null;
    }
    public <T> T set(T obj, Class<T> typeClass)
    {
        if(type.equals(typeClass.getName()) && isArray == typeClass.isArray())
            data = (Object) obj;
    }
    public String doSomething()
    {
        //An example of doing different things based on type
        switch(type)
        {
        case "java.lang.String":
            if(isArray)
                return TakesAStringArray(get(String[].class));
            else
                return TakesAString(get(String.class));
        break;
        case "java.lang.Integer":
            return TakesAnInteger(get(Integer.class));
        break;
        case "myPackage.DataParam":
            return TakesADataParam(get(myPackage.DataParam.class));
        break;
        //default: Do nothing
    }
}
public DataObj
{
    protected HashMap<String,DataParam> fields;
    //Other functions
}
public SpecificObj extends DataObj
{
    public SpecificObj()
    {
        fields.add(new DataParam("name", String.class));
        fields.add(new DataParam("obj", myPackage.DataParam.class));
        fields.add(new DataParam("count", Integer.class));
    }
}

My questions are, is there anything blatantly obviously wrong with this, like I'm gonna screw myself over later? Is there a better way that I should consider?

This should be type safe, right? (I'm checking the class every time I do anything with that piece of data, but it just seems weird that I have to do it myself versus like, the compiler raising an error that I'm doing it wrong).

What you are doing appears to be sane. The use of generics is primarily as a compile-time validator (and it makes some code simpler, like not having to cast things).

If you don't know the types of your data at compile time, then you can't use generics.

So, you have run-time specified data, and that's OK. Do you handle that as well as it can be handled? That is the real question.

Items I can see that raise question-marks:

Why are you saving away the Class name (as type) when you can save away the actual Class?
wasUpdated is not used.
type is a bad name for a Java variable. It conflicts with java.lang.Type, and you don't need it anyway if you are going to store the Class instead of its name.
if you do keep the class, then you can use the Class.isInstance(...) method to ensure the data types are correct, instead of specifying the class as part of the retrieval in the get(Class<T> clazz) method... or, you can confirm they are the same with a simple .equals() call instead of the name/isArray check.
the methods like return TakesAnInteger(...) should have a lower-case t to start with.... this is the Java convention for method names.
Similarly, Java convention is to put the opening brace { for a block on the same line as the method declaration.

Edit: with Class saved instead of name and isArray

Also, I note now that you are missing some semi-colons in your data declarations.... and also, you are missing return values from some of your methods.

public class DataParam {
    public String name;
    public Class<?> dataClass;
    public boolean wasUpdated;
    private Object data;

    public DataParam(String name, Class<?> typeClass) {
        this.name = name;
        dataClass = typeClass;
        data = null;
        wasUpdated = false;
    }

    public <T> T get(Class<T> typeClass) {
        if(dataClass.equals(typeClass)) {
            return typeClass.cast(data);
        }
        return null;
    }

    public <T> T set(T obj, Class<T> typeClass) {
        if(dataClass.equals(typeClass)) {
            T toreturn = typeClass.cast(data);
            data = obj;
            return toreturn;
        }
        return null;
    }

    public String doSomething() {
        //An example of doing different things based on type
        //.......
        return null;
    }
}


Input, display and search company data I have just learned classes and was practicing some examples from the textbook when I came to a screeching halt.

Here's the program I have written, which is complete but not optimal.  What this program does is that it inputs information about the company and then lets you display the data or search for a specific set of data.

This is a self-learning exercise, not homework and not for work either.

#include<iostream>
#include<cstring>
#include <conio.h>
using namespace std;

class company
{
    public:
    char companyname[20],companytype[10],companycode[5];
    int yearlyturnover;

    void displaydata()
    {
        cout<<companyname[20];
        cout<<companytype[10];
        cout<<companycode[5];
        cout<<yearlyturnover;
    }

};

int main()
{
 int count,num,option;

cout<<"How many companies do you want to input?:";
    cin>>num;

for(count=0;count<num;count++)
{
    company com1[count];
    cout<<"\nEnter the name of the company:";
        cin.getline(com1[count].companyname,20);
    cout<<"\nEnter the code of the company";
        cin.getline(com1[count].companycode,5);
    cout<<"\nEnter the Type of company: Public Ltd. or Private Ltd.";
        cin.getline(com1[count].companytype,10);
    cout<<"\nInput the yearly turnover of the company:";
        cin>>com1[count].yearlyturnover;
}

cout<<"\nPress 1 to display the data:";
cout<<"\nPress 2 to search for a company";
    cin>>option;
switch(option)
{

case 1: for(count=0;count<num;count++)
            {
            company com1[count];
            com1[count].displaydata();
            }
        break;

case 2: for(count=0;count<num;count++)
        {
            char input[10];
            company com1[count];
        cout<<"\nPlease enter the company type:";
            cin.getline(input,10);
        if(strcmp(input,com1[count].companytype)==0)
        {
            com1[count].displaydata();
        }

        }
        break;

default:cout<<"\nError: terminating program...";
};

 getch();
}

First, Welcome to code review, and a very nice question for a beginning self learner in C++.

user1118321 is correct about a everything he says, I'm going to emphasis one or two items and provide some references before I mention a few other things.

Use Functions:
Functions allow one to break up their code into smaller, easier to understand logical blocks. When
designing software it is often easier to break a complex problem into smaller problems that are
easier to program. Some people use flow charts to map out their program, and high level flow
charts will only contain the functions. These will be followed by lower level flow charts that
map out the logic of each function.

Using functions allows one to decrease the amount of code written in some cases.

The main() function should be primarily used to set up the environment for the rest of the program,
call a function to execute the rest of the program and catch any errors.

If you find yourself repeating code move that code to a function so it only needs to be written and
debugged once. This is actually a programming principle generally referred to as Don't Repeat Yourself
and you may see it referred to here on code review as DRY.

Both functions and classes should follow the Single Responsibility Principle (SRP). The Single Responsibility
Principle is that a function should do one thing, and one thing only. This makes code easier to write,
debug and maintain. It also makes the function easier to reuse.
The Single Responsibility Principle may also be a reference to Demeter's Law (I may be wrong about them being the same principle).

Another programming principle sometimes mentioned is Keep It Simple (KISS), although this isn't just for
programming.

You may want to look into SOLID programming as well.

A more comprehensive list of programming principles can be found here.

Use Existing Classes
A long time ago when I started using C++ the Standard Template Library (STL), sometimes called the
Standard Library, had not yet been defined or implemented. We had to create our own strings, queues, vectors,
stacks and other container classes. Using the C++ container classes and the standard library requires
A Lot Less Code and makes it much simpler to write correct software. Two great website to become familar
with if you are going to write C++ code are CPlusPlus and the C++ reference.

These websites are better
than using a book as a reference because it remains up to date, and documents the C++11, C++14 and C++17
standards all in one place.

The string container class provides one with much more functionality then cstrings.

The follow is new content not previously mentioned.

Magic Numbers
The term magic number is sometimes used for numeric constants in code that are not obvious. By Not Obvious I mean that it's not obvious what the number represents. Non-symbolic references to numbers
should be replaced either by named constants, or enums (enumerator types). An example of a named
constant is:

constexpr double PI = 3.1415926;

Now in your code below, it is somewhat obvious what 1 and 2 are, but generally switch statements use enums or
symbolic constants.

    std::cout << "\nPress 1 to display the data:";
    std::cout << "\nPress 2 to search for a company";
    cin >> option;

    switch(option)
    {
        case 1:
           for(count = 0; count < num; count++)
           {
               company com1[count];
               com1[count].displaydata();
           }
           break;

        case 2:
            for(count = 0; count < num; count++)
            {
                char input[10];
                company com1[count];

                std::cout << "\nPlease enter the company type:";
                cin.getline(input, 10);

                if(strcmp(input,com1[count].companytype) == 0)
                {
                    com1[count].displaydata();
                }

            }
            break;

        default:
            std::cout<<"\nError: terminating program...";
    };

Please note that the default case should probably have a break; statement as well, especially if is
embedded within another control statement such as a loop.

Readability and Maintainability
Making the code more readable makes it more maintainable by you or anyone else that needs to add functionality.

The code would be much more readable if it was consistently indented as I show above in the included section
of the code. Blocks of code within { and } should be indented.

Operators in control statements such as for() loops, while loops and if statement should be separated by
spaces.

Use vertical spacing to make the code easier to read as well.

In C++ and more modern languages it is more common to create variables as you need them. I would change the above
code to

    std::cout << "\nPress 1 to display the data:";
    std::cout << "\nPress 2 to search for a company";

    int option;
    cin >> option;

This allows the maintainer to know the type of variable without jumping all over the code.

BUGS
This code may work due to the specific compiler you are using, but the fact is that you create the
company array at least 3 times. Once in every loop that I see. The variable com1 should be created
once at the top of main where count, num and option are created.

company com1[count];

The variable com1 should probably be created as
    vector com1;

This would provide a variable sized array of companies. The code for inputing the companies wouldn't change,
but the code for displaying a company or searching for a company can change:

for (auto thiscompany: com1)
{
    thiscompany.displaydata();
}

You could use the find_if() function instead of the current search.


Refactor VB.NET to C#.Net using Linq I am rewriting a VB.NET application in C#. I will not subject you to the original code because it's pretty messy. Below is the converted C# code:

   public IEnumerable<WebQuery> GetQueries(Request request)
    {
        var queries = new List<WebQuery>();
        foreach (var report in request.Reports)
        {
            queries.AddRange((from query in _context.WebQueries
                join qg in _context.WebQueryGroups on query.QueryKey equals qg.QueryKey
                join wp in _context.WebPermissions on qg.QueryGroupNameKey equals wp.TaskGroupNameKey
                join wugn in _context.WebUserGroupNames on wp.UserGroupNameKey equals wugn.UserGroupNameKey
                join wug in _context.WebUserGroups on wugn.UserGroupNameKey equals wug.UserGroupNameKey
                join wt in _context.WebTasks on query.QueryCategory equals wt.TaskReportCategory
                where wt.TaskKey == request.Key && wp.ResourceKey == 4 && wt.TaskKey == request.Key && wug.UserKey == report.UserKey
                select query).OrderBy(x => x.QueryTitle));
        }
        return queries;
    }

As you can see, I am committing a grievous error in making a call to the database many times.

I am not sure how to append the loop to my query so that I only call the context one time.

Have you tried:

    public IEnumerable<WebQuery> GetQueries(Request request)
    {
        List<string> keys = request.Reports.Select(i => i.UserKey).ToList();
        var queries = new List<WebQuery>();
        queries.AddRange((from query in _context.WebQueries
            join qg in _context.WebQueryGroups on query.QueryKey equals qg.QueryKey
            join wp in _context.WebPermissions on qg.QueryGroupNameKey equals wp.TaskGroupNameKey
            join wugn in _context.WebUserGroupNames on wp.UserGroupNameKey equals wugn.UserGroupNameKey
            join wug in _context.WebUserGroups on wugn.UserGroupNameKey equals wug.UserGroupNameKey
            join wt in _context.WebTasks on query.QueryCategory equals wt.TaskReportCategory
            where wt.TaskKey == request.Key
                && wp.ResourceKey == 4
                && wt.TaskKey == request.Key
                && keys.Contains(wug.UserKey)
            select query).OrderBy(x => x.QueryTitle));
        return queries;
    }

?


Reversing every word in a string Examples:

char test1[] = "               ";
char test2[] = "   hello  z";
char test3[] = "hello world   ";
char test4[] = "x y z ";

Results:

"               "
"   olleh  z"
"olleh dlrow   "
"x y z "

The problem:

  Reverse every world in a string, ignore the spaces.

  The following is my code. The basic idea is to scan the string, when
  finding a word, then reverse it. The complexity of the algorithm is
  O(n), where n is the length of the string.

Could anyone help me verify it? Is there any better solution?

void reverse_word(char* s, char* e)
{
    while(s < e)
    {
        char tmp = *s;
        *s = *e;
        *e = tmp;
        ++s;
        --e;
    }
}

char* word_start_index(char* p)
{
    while((*p != '\0') && (*p == ' '))
    {
        ++p;
    }

    if(*p == '\0')
        return NULL;
    else
        return p;
}

char* word_end_index(char* p)
{
    while((*p != '\0') && (*p != ' '))
    {
        ++p;
    }

    return p-1;
}

void reverse_string(char* s)
{
    char* s_w = NULL;
    char* e_w = NULL;
    char* runner = s;

    while(*runner != '\0')
    {
        char* cur_word_s = word_start_index(runner);
        if(cur_word_s == NULL)
            break;
        char* cur_word_e = word_end_index(cur_word_s);
        reverse_word(cur_word_s, cur_word_e);
        runner = cur_word_e+1;
    }
}

If using C++ is an option for you (which your tags suggest), then you might consider using std::swap for swapping the characters. You might also want to use isspace to check for whitespace other than ' '.

You could also write it all in a single function:

void reverse_every_word(char* s)
{
    char* front;
    char* back;
    while(*s != '\0')
    {
        // handle whitespace
        while(*s != '\0' && isspace(*s))
            s++;

        // skip to the end of the current word
        front = s;
        while(*s != '\0' && !isspace(*s))
            s++;

        // reverse
        back = s-1;
        while (front < back)
            std::swap(*front++, *back--);
    }
}


Record parsing to Scala object with some unknown fields I am getting some customer data but don't know the complete schema beforehand. I want to parse this data and create a Scala object from it. But due to unknown fields in the record, I fill only the important fields and remaining record data is dumped in a remData placeholder field. This remData is used so that we can still see the customer data in output and can add more fields in future.

The customer provides 3-4 types of such feeds with each feed having 15-20 fields. I want to generate a generic framework for such parsing. I tried this code and it works but I wonder if there are any common patterns for such type of parsing or a cleaner approach.

class Record{
    var imptField1 =""
    var imptField2 =""
    var remData =""
    def this(data:String){
      this()
      val fieldKeyValues = data.split(",")
      for( fieldKeyValue <- fieldKeyValues){
        val tokens = fieldKeyValue.split("=")
        val key = tokens(0)
        key match{
          case "imptField1"=> this.imptField1 = tokens(1)
          case "imptField2"=> this.imptField2 = tokens(1)
          case _ => this.remData +=fieldKeyValue+","
        }
      }
   }
 }

I also tried using Jackson objectMapper for that but it doesn't provide the functionality of dumping unknown fields in a placeholder. Please suggest a more efficient/cleaner way of doing this.

I would parse the row into a Map[String,String] first (by collecting the key-value pairs that you already know how to split), and then do

this.imptField1 = map.get("imptField1").getOrElse("")
this.imptField2 = map.get("imptField2").getOrElse("")
this.remData = map  // just the whole Map, or maybe remove the above two keys

I would also change the class to be immutable and move the parsing code into the companion object (it would do the parsing, then pass the map to the constructor of the class which can directly assign to its val fields).


Project Euler, Challenge #12 in Swift The sequence of triangle numbers is generated by adding the natural numbers. So the 7th triangle number would be 1 + 2 + 3 + 4 + 5 + 6 + 7 = 28. The first ten terms would be:
1, 3, 6, 10, 15, 21, 28, 36, 45, 55, ...
Let us list the factors of the first seven triangle numbers:
1: 1
3: 1,3
6: 1,2,3,6
10: 1,2,5,10
15: 1,3,5,15
21: 1,3,7,21
28: 1,2,4,7,14,28
We can see that 28 is the first triangle number to have over five divisors.
What is the value of the first triangle number to have over five hundred divisors?

Swift Code:
import Foundation

let numberOfRequiredFactors = 500

func nthTriangularNumber( n:Int ) -> Int {
    var number = 0

    for iterator in 0...n {

        number += iterator

    }

    return number
}

func findNumberOfFactors( number:Int ) -> Int {
    var numberOfFactors = 0

    for iterator in 1...number {
        if number % iterator == 0 {
            numberOfFactors++
        }
    }

    return numberOfFactors
}

var countOfTriangularNumbers = 1
var found = false
var foundNumber = 0

while !found {

    var numberOfFactors = findNumberOfFactors(nthTriangularNumber(countOfTriangularNumbers))

    if numberOfFactors > numberOfRequiredFactors {
        found = true
        foundNumber = nthTriangularNumber(countOfTriangularNumbers)
    }

    countOfTriangularNumbers++

}

println("The number is: \(foundNumber)")

Comments on making this code more efficient, making it more cleaner or violations of best practices would be highly appreciated. However, it did work for smaller numbers, which is how I tested if the logic is right.

The spacing on your parenthesis in your function declarations is odd and distracting.  Try not to fight what autocomplete will give you.  It should look like this:
func nameOfFunction(firstArgumentName: Int) -> Int

Notice the lack of spaces inside the parenthesis but the addition of the space between the argument name and its type?  This is the expect formatting style for Swift function declaration.

Memoization

func nthTriangularNumber( n:Int ) -> Int {
    var number = 0

    for iterator in 0...n {

        number += iterator

    }

    return number
}

We can immediately improve upon this approach.  We know that for any number n, the nth triangular number will be the n-1th triangular number + n, right?
So we can improve this approach by using some memoization.
Without changing the rules of this function (although, I think the name needs some work), we can use a local struct to hold an array of the previously calculated triangular numbers.
Since Swift doesn't have the same sort of static function variables as Objective-C and other languages have, we have to consult this Stack Overflow question to figure out how to achieve something similar.
So... we're going to want a nested struct that looks like this:
struct Memoizer {
    static var triangularNumbers: [Int] = []
}

Now we'll use this Memoizer.triangularNumbers array to memoize triangular numbers so nothing is ever calculated more than once.  Perhaps this makes sense as an extension to Int though?
extension Int {
    func triangularNumber() -> Int {
        struct Memoizer {
            static var triangularNumbers: [Int] = [0]
        }

        let lastIndex = Memoizer.triangularNumbers.count - 1
        for n in lastIndex...self {
            let next = Memoizer.triangularNumbers[n] + n
            Memoizer.triangularNumbers.append(next)
        }

        return Memoizer.triangularNumbers[self]
    }
}

So, if we try to find 500's triangular number, it still takes just as long as it would have taken using your approach (assuming we've not previously found any other triangular numbers) (and maybe slightly longer since we have to allocate memory for the array).  But once we've calculated the 500th triangular number, anything less than 500 is a simply array look up.  And calculating the 501st triangular number is a matter of grabbing the 500th and adding 501 to it.
That last point is the most important part here though.
In your current implementation, each iteration of your while loop takes progressively longer than the previous one.  Your countOfTriangularNumbers is also the count of addition operations you have to do per loop to calculate that particular triangular numbers.
Using the implementation I just suggested, your number of addition operations to calculate the triangular number stays at a constant one.
The difference between one addition operation and two addition operations may not be measurable.  But by the 500th or 1000th or more iteration of your loop, the difference between one addition operation and 500 addition operations starts to become noticeable.  (And this is minor addition to the time it takes... but when we do find the number, we have to then recalculate it inside your if branch).

There is still a lot to work on for this problem, but I think this answer sets you down a very good path.  Think about what this memoization is doing for us, and try to see where we can apply it in other places (especially as you work through Project Euler).


DataGrid Filter Method Very Slow I'm having a lot of trouble in my Database program with trying to implement an effective DataGrid filtering method. After taking advice from a previous code review I'm trying to implement a DataModel method of coding. My previous method of searching my DataGrid was this;

SearchGrid (PREVIOUS)

private void SearchGrid(object sender, TextChangedEventArgs e)
    {
        DataView dv = dataGrid.ItemsSource as DataView;
        if (compNameRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyName LIKE '%" + searchBox.Text + "%'";
        }
        if (compTownRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyTown LIKE '%" + searchBox.Text + "%'";
        }
        if (compPcodeRad.IsChecked == true)
        {
            dv.RowFilter = "CompanyPcode LIKE '%" + searchBox.Text + "%'";
        }
    }

This worked fine with the way I bound my DataGrid before. I now bind my DataGrid using a Model class and the previous filter method does not work. I have written a new filter method which is this;

SearchGrid (NEW)

private void SearchGrid(object sender, TextChangedEventArgs e)
        {
            if (!string.IsNullOrEmpty(searchBox.Text))
            {
                ICollectionView view =      CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
                view.Filter += (obj) =>
                {
                    CompanyModel model = obj as CompanyModel;
                    if (model == null)
                        return true;

                    if (compNameRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
                    }
                    if (compTownRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
                     }
                    if (compPcodeRad.IsChecked == true)
                    {
                        return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
                    }
                    return false;
                };
            }
        }

Now this does work OK, however there are two issues:

is that it is extremely slow when I type text to search the DataGrid, and
it doesn't deal with the most simple of cases, for example if the name of the Company is "A1" then searching for "a1" will not find the company.

I have been recommended to use Entity Framework however as I am using an OleDB connection (this is a very old Database I am rewriting) I have been unable to get this to work.

Is there a more efficient search method I can use that is quicker and would also find all companies (i.e case insensitive)?

private void SearchGrid(object sender, TextChangedEventArgs e)
    {
        if (!string.IsNullOrEmpty(searchBox.Text))
        {
            ICollectionView view =      CollectionViewSource.GetDefaultView(dataGrid.ItemsSource);
            view.Filter += (obj) =>
            {
                CompanyModel model = obj as CompanyModel;
                if (model == null)
                    return true;

                if (compNameRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyName) && model.CompanyName.Contains(searchBox.Text);
                }
                if (compTownRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyTown) && model.CompanyTown.Contains(searchBox.Text);
                }
                if (compPcodeRad.IsChecked == true)
                {
                    return !string.IsNullOrEmpty(model.CompanyPcode) && model.CompanyPcode.Contains(searchBox.Text);
                }
                return false;
            };
        }
    }

The problem with your code is that each time the text changed you are adding a new Filter to the ICollectionView by using

view.Filter += (obj) =>

this little + is doing all the harm. For each raise of the TextChanged event a new filter is added and never removed.

If you remove the + you will see that your filtering will become faster.

The problem with A1 vs a1 is that the Contains() method is case sensitive. A better way would be to use IndexOf(string, StringComparison) like so

return !string.IsNullOrEmpty(model.CompanyName)
       && model.CompanyName.IndexOf(searchBox.Text, StringComparison.OrdinalIgnoreCase) > -1;


Third project Euler solution The things I'm interested the most from the review are:

The performance of the code
Overall review of the code structure, styling rules and naming conventions.

  What is the largest prime factor of the number 600851475143 ?

import math
#----------------------------------------------------------------------------------
def prime_factorization(num_to_factor):
    """Calculates the prime factors of the input number based on
    Pollard's rho algorithm for integer factorization.

    Arguments:
        type:<int> - An number to factor.
    Return:
        type:<arr> - An array holding the prime factors of input number.
    """

    #------------------------------------------------------------------------------
    def calc_factor(num_to_factor):

        x,x_fixed = 2,2
        cycles = 2
        factor = 1

        while factor == 1:
            for counter in range(cycles):
                if factor > 1: break

                x = (x * x - 1) % num_to_factor
                factor = math.gcd(x - x_fixed, num_to_factor)

            x_fixed = x
            cycles *= 2
        return factor

    factors = []
    while num_to_factor > 1:

        # Gets the smallest factor of (num_to_factor) and appends it to the array.
        # Divides the (num_to_factor) with it, and starts searching for the next factor.
        factor = calc_factor(num_to_factor)
        factors.append(factor)
        num_to_factor //= factor

    return factors

#----------------------------------------------------------------------------------
def main():
    n = 600851475143
    print(' Prime factorization of {} is: {}'.format(n, prime_factorization(n)))
#----------------------------------------------------------------------------------
if __name__ == "__main__":
    main()

Disclaimer: I have never used or implemented Pollard's rho algorithm.

calc_factor

I think this function does not need to be a nested function. It could be a normal function on its own (and thus be documented accordingly).

Maybe, its name and the name for its parameter could be updated for something that sounds more usual, maybe get_factor(n)...

Instead of checking if factor > 1 at the beginning of the loop, you could check if just after updating the value of factor.

Also, instead of trying to break out of 2 loops, you could simply return the value directly if factor > 1 and you realise you don't need to check the value of factor in other places.

You'd get something like:

def get_factor(n):
    x,x_fixed = 2,2
    cycles = 2

    while True:
        for counter in range(cycles):
            x = (x * x - 1) % n
            factor = math.gcd(x - x_fixed, n)
            if factor > 1:
                return factor
        x_fixed = x
        cycles *= 2

prime_factorization

Here again, you could probably rename the parameter for something shorter.

I am not sure what the conventional way to do is but once you get a factor with the other functions, it may be worth checking how many times it divides.

Potential issues

Finally, I have the feeling that the algorithm does not provide an real prime factorisation because the factors returned by calc_factor are not always prime factors, they are just "random" factors.

If it may help you, here is the function I've used many times for Project Euler factorisations:

def prime_factors(n):
    """Yields prime factors of a positive number."""
    assert n > 0
    d = 2
    while d * d <= n:
        while n % d == 0:
            n //= d
            yield d
        d += 1
    if n > 1:  # to avoid 1 as a factor
        assert d <= n
    yield n


Multiple FizzBuzz with input from file I made a fizzbuzz program in Java 7 I'd like reviewed.

Given an input file with three numbers in each line, fizz, buzz, and count:

3 5 10
2 7 15

The output, for each line, should be the numbers from 1 to count, except that multiples of fizz are replaced with "F", multiples of buzz are replaced by "B", and multiples of both with "FB":

1 2 F 4 B F 7 8 F B
1 F 3 F 5 F B F 9 F 11 F 13 FB 15

The code, in Java 7:

package com.codeeval.fizzbuzz;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public class Main {
    public static void main(String[] args) throws IOException {
        String path = args[0];
        List<String> lines = getFileLines(path);

        StringBuilder sb = new StringBuilder();
        for (String line : lines){
            int[] arguments = getFizzBuzzArguments(line);
            int x = arguments[0];
            int y = arguments[1];
            int n = arguments[2];

            String result = fizzBuzz(x, y, n);
            sb.append(result).append('\n');
        }

        System.out.println(sb);
    }

    public static String fizzBuzz(int x, int y, int n) {
        StringBuilder sb = new StringBuilder();
        for (int i = 1; i <= n; i++) {
            boolean isX = (i % x == 0);
            boolean isY = (i % y == 0);
            String append;

            if (isX || isY) {
                if (isX && isY) { append = "FB"; }
                else if (isX) { append = "F"; }
                else { append = "B"; }
            } else { append = Integer.toString(i); }

            sb.append(append);
            if (i != n){ sb.append(' '); }
        }
        return sb.toString();
    }

    private static List<String> getFileLines(String path) throws IOException {
        File file = new File(path);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        List<String> lines = new ArrayList<>();
        String line;
        while ((line = buffer.readLine()) != null) {
            line = line.trim();
            lines.add(line);
        }

        return lines;
    }

    private static int[] getFizzBuzzArguments(String line){
        String[] arguments = line.split(" ");
        return getArrayOfInts(arguments);
    }

    private static int[] getArrayOfInts(String[] strings){
        int[] ints = new int[strings.length];

        for (int i = 0; i < strings.length; i++){
            ints[i] = Integer.parseInt(strings[i]);
        }

        return ints;
    }
}

Your code is well structured and quite easy to read and understand: this is a very good point about it.

However, there are some things that may be improved, but very few of them seem to be really critical.

main(arg)

Variables int x, int y and int n are treated separately, but indeed they are semantically related, because they describe a "FizzBuzz" to produce. A dedicated class may be created, for example:

public class FizzBuzz {

  private final int x;
  private final int y;
  private final int n;

  public FizzBuzz(int x, int y, int n) {
    this.x = x;
    this.y = y;
    this.n = n;
  }
  // getters, if needed
}

Moreover, your fizzBuzz method may then go directly into this class, named something like processInput(). The loop in main() would look like this:

for (String line : lines) {
  int[] arguments = getFizzBuzzArguments(line);
  FizzBuzz fb = new FizzBuzz(arguments[0], arguments[1], arguments[2]);
  sb.append(fb.processInput()).append('\n');
}

FizzBuzz may also have a constructor taking directly the array of input data as argument.

fizzBuzz(args)

Three points concerning the modulo divisions and the booleans isX and isY.

First, the divisors (x or y) must be validated to be different from 0, if you want to avoid ArithmeticException .

Second, they should be extracted into a dedicated method, which is easier to read:

private static boolean isMultipleOf(int dividend, int divisor) {
  return dividend % divisor == 0;
}

Third, the booleans are named poorly. They'd better sound as isFizz and isBuzz: these are their meanings.

Finally, the conditionals can be simplified:

if (isFizz || isBuzz) {
  if (isFizz) {
    sb.append("F");
  }
  if (isBuzz) {
    sb.append("B");
  }
} else {
  sb.append(i);
}

getFileLines()

Since you underline that the code in Java 7, then its facilities should be used.

Replace File with java.nio.file.Path.
Use a try-with-resources block in order to close automatically the streams opened with BufferedReader and FileReader.


Nearest point using 2D tree, static variable for the closest point so far This is a problem from a Coursera class:

  Write a data type to represent a set of points in the unit square (all points have x- and y-coordinates between 0 and 1) using a 2D tree to support efficient nearest neighbor search (find a closest point to a query point).

I implemented the class with pre-setup API. The problem for me was to efficiently realize the pruning rule:

  To find a closest point to a given query point, start at the root and recursively search in both subtrees using the following pruning rule: if the closest point discovered so far is closer than the distance between the query point and the rectangle corresponding to a node, there is no need to explore that node (or its subtrees).

So I need to store statically best tuple (closest point, minimum distance) so far. For this purpose I think static variable of class is not the best choice (it is needed only for one this method). I also don't like my solution for this: I made nested static class, pointDist, with static variables. What is a more optimal/elegant decision?

public class KdTree {

    private Node root;
    private int size;

    public KdTree() {
        root = null;
        size = 0;
    }

    // ... other class methods

    private static final class pointDist {
        private static Point2D p; // closest point
        private static double d; // min distance

        private static void begin() {
            p = null;
            d = Double.POSITIVE_INFINITY;
        }
    }

    private void nearest(Point2D p, Node x) {
        if (x == null) return;
        // code with p, x, pointDist.p and pointDist.d variables
        // ...
    }

    public Point2D nearest(Point2D p) {
        assert size() > 0;
        pointDist.begin();
        nearest(p, root);
        return pointDist.p;
    }

    private static class Node {

        private Point2D p; // the point
        private RectHV rect; // the axis-aligned rectangle corresponding to this node
        private Node lb; // the left/bottom subtree
        private Node rt; // the right/top subtree

        public Node(Point2D pp, Node prevN, int caseCut) {
            p = pp;
            lb = null;
            rt = null;
            // root case
            if (caseCut == -1) rect = new RectHV(0, 0, 1, 1);
            else {
                RectHV prevR = prevN.rect;
                // the rest of the code here is to define rect based on values of
                // caseCut variable: 0,1,2,4 - cuts from left, right, bottom and top
                // ...
            }
        }
    }
}

You're right in sensing there has to be a better way to do this.  Right now, you're setting static fields from within instance methods, which is rarely a good idea.  Luckily, your code is already sufficiently structured that fixing this requires very little work.

Explicitly pass an instance containing a Point2D and double as a parameter in the nearest method.  This keeps your algorithm as it is, but it limits the scope of changes so that they stay within local execution:

private static final class PointDistance {
    // note that these are not static
    Point2D p;
    double d;
}

private void nearest(Point2D p, Node x, PointDistance closest) {
    // replace pointDist with closest here
}

public Point2D nearest(Point2D p) {
    final PointDistance closest = new PointDistance();
    nearest(p, root, closest);
    return closest.p;
}


Simple functions class I wrote this code a while back and I have recently been rewriting most of my code. I just to know if these methods can be improved. The ones that pop out to me are listed below; others don't matter as much.

Methods that need improving:

FilterInjectionChars
IsValidAlphaNumeric
FilterFigure

namespace Apple.Application.Core
{
    using System;
    using System.Collections.Generic;
    using System.Linq;

    class AppleFunctions
    {
        private readonly HashSet<char> _allowedCharacters;

        public AppleFunctions()
        {
            _allowedCharacters = new HashSet<char>(new[] { '-', '.', ' ', 'Ã', '©', '¡', '', 'º', '³', 'Ã', '‰', '_' });
        }

        public bool GetBoolFromEnum(string EnumValue)
        {
            return Convert.ToBoolean(EnumValue);
        }

        public bool GetBoolFromEnum(int EnumValue)
        {
            return Convert.ToBoolean(EnumValue);
        }

        public int GetEnumFromBool(bool BoolValue)
        {
            return BoolValue ? 1 : 2;
        }

        public bool IsNumber(string StringValue)
        {
            int Number;
            return int.TryParse(StringValue, out Number);
        }

        public int CompareMilleseconds(DateTime time, DateTime from)
        {
            DateTime DateTime = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
            return Convert.ToInt32((from.Subtract(DateTime).TotalMilliseconds - time.Subtract(DateTime).TotalMilliseconds));
        }

        public int GetUnixTimestamp()
        {
            return (int)DateTime.UtcNow.Subtract(new DateTime(1970, 1, 1)).TotalSeconds;
        }

        public DateTime GetUnixToDate(int unixTimeStamp)
        {
            DateTime Datetime = new DateTime(1970, 1, 1, 0, 0, 0, 0, System.DateTimeKind.Utc);
            return Datetime.AddSeconds(unixTimeStamp).ToLocalTime();
        }

        public DateTime GetUnixToDate(double unixTimeStamp)
        {
            DateTime Datetime = new DateTime(1970, 1, 1, 0, 0, 0, 0, System.DateTimeKind.Utc);
            return Datetime.AddSeconds(unixTimeStamp).ToLocalTime();
        }

        public int GetDateToUnix(DateTime time)
        {
            DateTime Datetime = new DateTime(1970, 1, 1, 0, 0, 0, 0, System.DateTimeKind.Utc);
            return Convert.ToInt32((time - Datetime).TotalSeconds);
        }

        public string GetDateJoinString(long timeStamp) // MOVE THIS TO GROUP
        {
            string[] time = GetUnixToDate(timeStamp).ToString("MMMM/dd/yyyy").Split('/');
            return String.Format("{0} {1}, {2}", time[0].Substring(0, 3), time[1], time[2]);
        }

        public long GetNow()
        {
            double totalMilliseconds = (DateTime.UtcNow - new DateTime(1970, 1, 1, 0, 0, 0)).TotalMilliseconds;
            return ((long)totalMilliseconds);
        }

        private bool IsValidCharacter(char c)
        {
            return char.IsLetterOrDigit(c) || _allowedCharacters.Contains(c);
        }

        /* METHODS TO IMPROOVE ARE BELOW */

        public string FilterFigure(string FigureCode)
        {
            return FigureCode.Any(character => !IsValidCharacter(character)) ? "lg-3023-1335.hr-828-45.sh-295-1332.hd-180-4.ea-3168-89.ca-1813-62.ch-235-1332" : FigureCode;
        }

        public bool IsValidAlphaNumeric(string inputStr)
        {
            inputStr = inputStr.ToLower();
            if (string.IsNullOrEmpty(inputStr))
                return false;
            return inputStr.All(IsValidCharacter);
        }

        public string FilterInjectionChars(string input)
        {
            input = input.Replace('\u0001', ' ');
            input = input.Replace('\u0002', ' ');
            input = input.Replace('\u0003', ' ');
            input = input.Replace('\t', ' ');
            return input;
        }
    }
}

There are methods that scream louder for improvement that the three you mentioned.

For example these:

public bool GetBoolFromEnum(string EnumValue)
{
  return Convert.ToBoolean(EnumValue);
}

public bool GetBoolFromEnum(int EnumValue)
{
  return Convert.ToBoolean(EnumValue);
}

public int GetEnumFromBool(bool BoolValue)
{
  return BoolValue ? 1 : 2;
}

You are converting some enum to bool or int and back but you are not using this enum here, why?

I guess the enum look probably like this (or rather should):

enum State
{
    Disabled,
    Enabled,
};

If you used the enum instead of just calling it like that, your methods might have be implemented as:

public bool ToState(State state)
{
    return Convert.ToBoolean(state);
}

public bool ToState(int state)
{
    return ToBoolean((State)state);
}

public int ToState(bool state)
{
    return Convert.ToInt32(state));
}

I find it very weird that you use 1 & 2 as boolean values instead of 1 & 0 that can be much easier converted to int/bool/enum.


Check if specific key exists in 2-dimensional array I am writing a "collision detection" in PHP and want to check if a specific "area" is still free in the two-dimensional target array.

I use the following code to do that, but I think there might be a better (= faster) solution than looping over every possible position in the array and check whether it's set or not.

public function CheckForCollision($top, $left, $size, $collision_array) {

       $possible_x = range($left, $left+$size-1);
       $possible_y = range($top, $top+$size-1);

       // Slow solution, improve this: Check every possible value in the $collision_array with our "might be"-values
       foreach($possible_x as $x)
       {
          foreach($possible_y as $y)
          {
             if($collision_array[$x][$y] == 1) { return false; break; }
          }
       }

       return true;
    }

Any ideas on this?

If I have understood your question correctly you are seeking a function which can check if values of a 2-dimensional array equals FALSE. An example of such array structure could be:

$array = [
    0 => [0, 0, 0, 0, 0],
    1 => [0, 0, 0, 0, 0],
    2 => [0, 0, 0, 0, 0],
    3 => [0, 0, 0, 0, 0],
    4 => [0, 0, 0, 0, 0],
];

I have attempted to write an implementation from the context you have given. I have reduced your code down to a single for loop and opened the possibility to having any default value that evaluates to FALSE, such as false, null, 0 or an empty array. This is done through the native array_filter() function. I have also included some error checking. An example usage could be.

/*
 * Using array structure from previous example.
 *
 * Check if array elements starting at index 1 and 2
 * elements forward only contains values that equals
 * to FALSE.
 */
$valid = collisions($array, 1, 2);

Along with inline comments the code of the function should be pretty self explanatory.

/**
 * Check if the provided array has any values that does not
 * evaluate to FALSE in the given array range.
 *
 * @param array $array A two dimensional array.
 * @param int   $start An integer specifying an array index to start the check.
 * @param int   $size  An integer specifying how many array elements to check values for.
 *
 * @return bool Returns FALSE if there are collisions, TRUE otherwise.
 * @throws \OutOfRangeException
 * @throws \DomainException
 */
function collisions(array $array, $start, $size)
{
    /*
     * We ensure the outer array has only
     * numeric array keys.
     */
    $array = array_values($array);

    /*
     * Check if starting index is out of range of the provided array.
     */
    if(!array_key_exists($start, $array)) {
        throw new OutOfRangeException('The specified starting index is out of range of the provided array.');
    }

    /*
     * Compute the upper boundary
     */
    $boundary = (int)$start + (int)$size;

    /*
     * Check if the amount of elements to check AFTER the specified starting
     * index is out of range of the provided array.
     */
    if(!array_key_exists($boundary, $array)) {
        throw new OutOfRangeException('The specified amount of elements to check exhausts the provided array.');
    }

    for($i = (int)$start; $i <= $boundary; $i++) {

        /*
         * Check if the current array value is an array.
         */
        if(!is_array($array[$i])) {
            throw new DomainException("Invalid type of array value at index {$i}. Expected an array, type of " . gettype($array[$i]) . ' given.');
        }

        /*
         * If the native function array_filter() doesn't have a second
         * argument, it will filter out any values corresponding to FALSE. I assume
         * an accepted value is 0, which evaluates to FALSE.
         *
         * Therefore of the resulting array is NOT empty there are
         * values which does not equal to 0 (zero).
         */
        if(array_filter($array[$i])) {
            return false;
        }

    }

    return true;
}

The error checking and the first line $array = array_values($array); can be removed if you are sure the provided array always has the correct structure and has numerical indexes. But I would strongly recommend to not doing so, as good error checking can help you when bugs occur.

Performance wise this shouldn't be too bad. The many if statements when checking for errors are somewhat slow, but should be negligible. As far as I know the for loop doesn't operate on a copy of the provided array as foreach otherwise would. This is only a concern if you are having huge arrays. If this is the case I would strongly recommend using an iterator.

Regarding performance. Premature optimization is almost always not the answer. I would suggest you write SOLID code. Then when you are done you can profile your application and look for the performance heavy blocks. Remember that if your application relies on a database the biggest performance gains are properly in writing highly optimized SQL queries. Of course there are exceptions. If this function is used extensively throughout your code it's always good to think about optimization.

Happy coding!


Bridge Transport - contest practice I'm practicing for a contest by doing some of the older problems, and I was wondering if there are more efficient ways of doing this or if I'm not following any coding conventions. Most importantly, I would like to know if I am using ArrayList correctly in this code, since I learned how to use it off of a brief tutorial on another website.

  Problem S2: Bridge transport

    A train of railway cars attempts to cross a bridge. The length of each car is 10m but their weights can differ. The bridge is 40m long. The cars are numbered starting at 1, going up to N, and they cross the bridge in that order. Your job is to find the largest number of railway cars that can safely cross the bridge.

      Sample Input

      100

      6

      50

      30

      10

      10

      40

      50

      Sample Output

      5

    Explanation for input/output

    The first number indicates the maximum weight that the bridge can hold (100), the second number tells us how many railway cars there are in total (6), and the next N (6) lines are the weights of those respective railway cars (in order, so car 1 has a weight of 50 and so on). The first four railway cars have total weight of 100, which is not greater than what the bridge can hold. When the first railway car leaves, and the next comes on, we have a total weight of $$30 + 10 + 10 + 40 = 90$$ This is also fine because it's not greater than what the bridge can hold. The last four cars would cause the bridge to break, since $$10 + 10 + 40 + 50 = 110$$ which is greater than the bridge can hold. Therefore, to get the maximum number of railway cars, you would need to disengage the last car, and let the first five go.

My solution:

public class BridgeTransport {

public static void main(String args[]) {

    try {
        BufferedReader input = new BufferedReader(new InputStreamReader(
                System.in));

        int maxWeight = Integer.parseInt(input.readLine());
        int totalCars = Integer.parseInt(input.readLine());
        int crossedCars = 0;
        int massOnBridge = 0;

        ArrayList<Integer> cars = new ArrayList<Integer>();
        ArrayList<Integer> carsOnBridge = new ArrayList<Integer>();

        // to get a list of all the weights of cars
        for (int i = 0; i < totalCars; i++) {
            cars.add(Integer.parseInt(input.readLine()));
        }

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

            // put the cars on the bridge one by one, if there are more than 4
            // cars, then remove the first car
            carsOnBridge.add(cars.get(i));
            if (carsOnBridge.size() > 4) {
                carsOnBridge.remove(0);
            }

            // gets the sum of the masses on the bridge
            for (int j = 0; j < carsOnBridge.size(); j++) {
                massOnBridge += carsOnBridge.get(j);
            }

            // if the masses are less than the max weight for the bridge,
            // then they can successfully cross
            if (massOnBridge <= maxWeight) {
                crossedCars++;
            } else {
                break;
            }

            //reset the sum of the masses on the bridge back to zero
            massOnBridge = 0;

        }

        System.out.println(crossedCars);

    } catch (IOException e) {
        e.printStackTrace();
    }
}
}

Am I using ArrayList correctly here?
Are there better ways to accomplish this task?
I feel guilty for resetting the mass on the bridge to 0 each iteration. Is that okay, or is there a better way to do it?

Instead of resetting the massOnBridge, you can declare it inside the body of the for loop:

for (int i = 0; i < totalCars; i++) {
    int massOnBridge = 0;
    ...

It is a good practice to keep a scope of a variable as narrow as possible.
There are a lot of comments in your code that tell what the code does. Instead, you should try to write a self-documenting code. You can increase modularity and clarity of your code by using separate methods for separate tasks. For instance,

// gets the sum of the masses on the bridge
for (int j = 0; j < carsOnBridge.size(); j++) {
     massOnBridge += carsOnBridge.get(j);
}

looks like a very good candidate for a separate method:

/**
  * Computes the sum of elements of {@code aList}.
  * @param aList A list of integers. Must not contain {@code null} elements.
  *              Must be non-{@code null}.
  * @return The sum of elements of the given list.
  */
private static int getSum(List<Integer> aList) {
    int sum = 0;
    for (int value : aList) {
         sum += value;
    }
    return sum;
 }

Having one long method(main in your case) that does everything is a bad practice.


nth Factorial of n 1s in a row In what way can I improve this? Be that performance, readability, etc.
I'm thinking implementing NumPy would improve performance.
Usage: Calculate* the nth factorial of n 1s in a row.
*No need for a precise calculation. Magnitude and leading digits is enough.
Examples:
2nd factorial of 11. (11 x 9 x 7 x 5 x 3 x 1)
3rd factorial of 111. (111 x 108 x 105....)
...
import multiprocessing
import argparse
import datetime
import math

parser = argparse.ArgumentParser(
    formatter_class=argparse.HelpFormatter,
    description="Calcs x factorial",
    usage=""
)

parser.add_argument("-n", "--number", type=int)

args = parser.parse_args()

def first_n_digits(num, n):
    return num // 10 ** (int(math.log(num, 10)) - n + 1)

def getones():
    if args.number == 1 :
        print(1)
        exit()

    num = ""
    for _ in range(0, args.number) :
        num += "".join("1")
    return int(num)

def getlog(send_end, i, threads, num):
    inc = int(num/threads)
    inc -= int(inc%args.number)
    start = num-inc*i

    end = num-inc*(i+1) if i < threads-1 else 0

    output = 0
    for j in range(start, end, -args.number):
        if j > 0:
            output += math.log10(j)

    send_end.send(output)

def main():
    num = getones()
    threads = multiprocessing.cpu_count() if getones()/multiprocessing.cpu_count() > multiprocessing.cpu_count() else 1
    jobs = []
    pipe_list = []
    for i in range(threads):
        recv_end, send_end = multiprocessing.Pipe(False)
        p = multiprocessing.Process(target=getlog, args=(send_end, i, threads, num))
        jobs.append(p)
        pipe_list.append(recv_end)
        p.start()
    i = 0
    for proc in jobs:
        proc.join()
        i+=1
    result_list = [output.recv() for output in pipe_list]

    magnitude = int(sum(result_list))
    normalized = 10**(sum(result_list)-magnitude)

    fnd = str(first_n_digits(normalized, 3))
    print("{}.{}{}e{}".format(fnd[0], fnd[1], fnd[2], magnitude))

if __name__ == '__main__':
    start = datetime.datetime.now()
    main()
    end = datetime.datetime.now()
    print(end-start)
```

One thing I'd change is to not use args everywhere. You use the name "n" in the problem description, so I'd use that in the program as well. Reading args only once, to store the value in n.
Your getones function is rather lengthy and inefficient, taking O(n2) time to build the string. Although the rest of the program can't handle large n anyway, so efficiency doesn't really matter here. (And maybe the conversion to int takes quadratic time, not sure.)
Here's a shorter solution:
e = sum(map(log10, range(int('1' * n), 0, -n)))
print('%.2fe%d' % (10**(e % 1), e // 1))

Demo:
>>> n = 9
>>> from math import log10
>>> e = sum(map(log10, range(int('1' * n), 0, -n)))
>>> print('%.2fe%d' % (10**(e % 1), e // 1))
9.22e93968682

Comparison with yours (at repl.it) for n=9, same result and mine's about three times as fast:
n = 9
yours:
9.22e93968682
0:00:11.911943
mine:
9.22e93968682
0:00:03.964518

We have a different result for example for n=2:
n = 2
yours:
1.03e4
0:00:00.011884
mine:
1.04e4
0:00:00.000085

The exact result is:
>>> math.prod(range(11, 0, -2))
10395

So I'd say yours isn't rounding correctly.


Transform snake_case to camelCase with sed I want to convert my_long_variable to myLongVariable in sed.

This works:

echo "my_long_variable" | sed -r 's/(^|_)([a-z])/\U\2/g' | sed -r 's/^(.)/\l\1/g'

Is there a more elegant way to do that with sed?

Why try to match for (^|_)? You are anyway modifying it at the end, so just skip the first character entirely:

echo "my_long_variable" | sed -r 's/_([a-z])/\U\1/gi' | sed -r 's/^([A-Z])/\l\1/'

The above pattern will take care of cases where you start with:

Some_var_Iable


Musical Note Length Calculator in C This is my first ever program in C and I was just tinkering around with this idea, as I'm on a music based course and the synth plugins all use milliseconds as its time.

Any improvements to make it more efficient or less dependant with a little explanation would be really appreciated.

/* declarations */
int crotchet(int);
int semibreve(int);
int minim(int);
int quaver(int);
int semiquaver(int);
int demisemiquaver(int);

int main() {

int bpm;

/* Ask for note bpm */
printf("Enter bpm value:\n");
scanf("%d", &bpm);

/* returns note lengths from functions */
printf("Semibreve would last for... ");
printf("%d", semibreve(bpm));
printf(" miliseconds! \n");

return 0;
}

int crotchet(int bpm)
{
/*  1 second = 1000
 1 min = 60 seconds = 6000 miliseconds
 */

int crotchet;
crotchet = 6000 / bpm;

return crotchet;
}

int semibreve(int bpm)
{
int crot;
crot = crotchet(bpm);
int semibreve;
semibreve = crot * 4;

return semibreve;
}

int minim(int bpm)
{
int crot;
crot = crotchet(bpm);
int minim;
minim = crot * 2;

return minim;
}

Same for quaver (dividing crot by 2), semiquaver (dividing crot by 4) and demi semiquaver (dividing crot by 8).

Any additional explanation towards the music theory I will also add if requested.

Your calculations are all off by a factor of 10: one minute is 60000 milliseconds, not 6000.  Instead of explaining your 6000 using a comment, write it out in the code, and the compiler will figure it out for you. That calculation will not cost you any runtime performance at all.

I don't recommend that design, defining one function per type of note.  One function should cover everything, I think.

Since you use printf() and scanf(), you should #include <stdio.h>.

It is customary to put main() at the end, so that you don't have to declare the functions it uses in advance.

#include <stdio.h>

#define SEMIBREVE       (1.0)
#define MINIM           (1.0/2)
#define CROTCHET        (1.0/4)
#define QUAVER          (1.0/8)
#define SEMIQUAVER      (1.0/16)
#define DEMISEMIQUAVER  (1.0/32)

int millisec(int bpm, double note) {
    return (int)(
        60      /* seconds */
        * 1000  /* milliseconds per second */
        * 4     /* crotchets per semibreve */
        * note
        / bpm
    );
}

int main() {
    int bpm;
    printf("Enter tempo (crotchets per minute): ");
    scanf("%d", &bpm);

    printf("Semibreve would last for... %d milliseconds!\n", millisec(bpm, SEMIBREVE));
    printf("Dotted crotchet would last for... %d milliseconds!\n", millisec(bpm, 1.5 * CROTCHET));
    printf("Demisemiquaver would last for... %d milliseconds!\n", millisec(bpm, DEMISEMIQUAVER));
}


Implementing an algorithm that walks the DOM without recursion Here's a simple algorithm that walks the DOM given a node:

function walkDOM(n) {
    do {
        console.log(n);
        if (n.hasChildNodes()) {
            walkDOM(n.firstChild)
        }
    } while (n = n.nextSibling)
}

I wanted to implement it iteratively as an exercise, and came up with this:

function walkDOM2(n) {
    var recStack = [];
    // First get the parent of the given node, so that
    // you can get the siblings of the given node too
    // (starting from the last sibling),
    // rather than just start with the children of the
    // given node.
    // (This is to make this behave the
    // same way as the recursive one.)
    recStack.push(n.parentNode);

    while (recStack.length > 0) {
        var current = recStack.pop();
        // Log only if the current node is
        // the given node or a node below it.
        // (This is to make this behave the
        // same way as the recursive one.)
        if (current != n.parentNode)
            console.log(current);
        if (!current.hasChildNodes())
            continue;

        current = current.lastChild;
        do {
            recStack.push(current);
            // Skip the sibling nodes
            // before the given node.
            // (This is to make this behave the
            // same way as the recursive one.)
            if (current === n)
                break;
        } while (current = current && current.previousSibling);
    }
}

I have used a couple tricks to make it behave the same way as the first recursive version. Is there a more concise way of writing this without recursion?

I know you are doing this as an exercise, and personally I like the recursive function. But just as an alternative, there is also the much forgotten TreeWalker API.

  Browser compatibility

  Supported by IE9+, FF2+, Chrome 1+, Safari 3+, Opera 9+

Javascript

var treeWalker = document.createTreeWalker(document.getElementById("list"), NodeFilter.SHOW_ALL, {
    acceptNode: function (node) {
        return NodeFilter.FILTER_ACCEPT;
    }
}, false);

do {
    console.log(treeWalker.currentNode);
} while (treeWalker.nextNode());

On jsfiddle


Async telnet connection over StreamSocket My team and I have a very poor understanding of best practices in relation to telnet. We have Task.Delay and Task.Wait in the code, including async voids and from what I understand those are potential areas that can cause deadlocks and other issues.

I'm trying to understand async in relation to running a telnet client. Is this a safe implementation of cancellation tokens and telnet connections?

public enum TelnetClientStatus : int
{
    Unset = 0,
    Connected= 1,
    Disconnected  = 2,
    Failed = 3
}

public TelnetClientStatus Status { get; set; } = TelnetClientStatus.Unset;

public async Task OpenConnection()
{
    //Don't allow the session to keep trying to open after the 5 seconds
    var cancellationTokenSource = new CancellationTokenSource();
    cancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(5));
    Status = await Task.Run(MakeConnection, cancellationTokenSource.Token);
}

private async Task<TelnetClientStatus> MakeConnection()
{
    _streamSocket = new StreamSocket();

    _dataWriter = new DataWriter(_streamSocket.OutputStream);
    _dataReader = new DataReader(_streamSocket.InputStream)
    {
        InputStreamOptions = InputStreamOptions.Partial
    };

    try
    {
        await _streamSocket.ConnectAsync(new HostName(HostName), Port.ToString());
        return TelnetClientStatus.Connected;
    }
    catch (TaskCanceledException) //All other exceptions need to be investigated not ignored
    {
        //I understand a connection failed due to timeout, give the app some flag to work with
        //Maybe retry the connection?
        return TelnetClientStatus.Failed;
    }
    catch (Exception ex)
    {
        throw ex;
    }
}

I see at least two points.

if you have a catch to catch a sytem Exception (wich is not good btw., it would be better to catch specific exceptions) and the only action is to throw that exception then its useless to have. If you need to rethrow an exception you should always omit the ex so just throw;, otherwise the StackTrace will be broken.
Having the public setter of the Status property is breaking encapsulation. Why should a user of the object be able to set this property? You should make it private.