CareerCup (Bloomberg): Check if string is valid based on brackets The goal of this problem is to determine if a string with brackets is valid. Below is a summary of the question from careercup.

Check if string s is valid based on brackets

"(({{}}))" is a valid s
"{[]}" is a valid s
"[{[}]]" is not valid

I know this is a rather basic exercise. I am looking for feedback on whether my solution is sound and efficient. I would also like to know if my code is clean. Also is there too much for a 45 minute interview? The solution is shown below.

public class Question_ValidStringWithBrackets
{
    public bool IsValidBracketString(String s, BracketValidator validator = null)
    {
        if (s == null)
            throw new ArgumentNullException("A string is required");

        if (validator == null)
            validator = BracketValidator.CreateDefaultValidator();

        Stack<char> openStack = new Stack<char>();

        foreach (var curBracket in s)
        {
            if (validator.IsOpen(curBracket))
            {
                openStack.Push(curBracket);
            }
            else if (openStack.Count == 0 || validator.IsMatchingPair(openStack.Pop(), curBracket))
            {
                return false;
            }
        }
        return openStack.Count == 0;
    }

    public class BracketValidator
    {
        private HashSet<char> openBrackets = new HashSet<char>();
        private Dictionary<char, char> closedOpenedPair = new Dictionary<char, char>();

        public void AddPair(char open, char close)
        {
            if (char.IsWhiteSpace(open) || char.IsWhiteSpace(close))
                throw new ArgumentException("A bracket must be specified. An empty character is not allowed.");

            if (openBrackets.Contains(open) || openBrackets.Contains(close))
                throw new ArgumentException("Brackets exist already.");

            if (closedOpenedPair.ContainsKey(open) || closedOpenedPair.ContainsKey(close))
                throw new ArgumentException("Brackets exist already.");

            openBrackets.Add(open);
            closedOpenedPair.Add(close, open);
        }

        public Boolean IsOpen(char open)
        {
            return openBrackets.Contains(open);
        }

        public bool IsMatchingPair(char open, char close)
        {
            return closedOpenedPair.TryGetValue(open, out char actualClose) && actualClose == close;
        }

        public static BracketValidator CreateDefaultValidator()
        {
            var validator = new BracketValidator();
            validator.AddPair('{', '}');
            validator.AddPair('[', ']');
            validator.AddPair('(', ')');

            return validator;
        }
    }
}

Guard conditions

ArgumentNullException is typically thrown on incorrect usage of the API. It is not an end-user exception, so I would not bother with an exception message.

  throw new ArgumentNullException("A string is required");

throw new ArgumentNullException(nameof(s)); // perhaps rename 's' to 'input' or 'value'

For the ArgumentException instances you throw, I would also add the nameof(parameterName) to the constructor. A decent error message is relevant here, so keep the message.

You have a guard on whitespace. I don't see this in the spec. Is this a requirement? If not, allow white space as either an open or close bracket.

  if (char.IsWhiteSpace(open) || char.IsWhiteSpace(close))

The next guard checks on duplicate registrations. An alternative policy is to ignore a registration if already registered. This depends on whether you want your API to be error-prone or act as a sandbox for consumers.

  if (openBrackets.Contains(open) || openBrackets.Contains(close))

I'm missing a guard condition. I doubt open and close can be the same.

if (open == close)  // throw ..

Readability

You could substitute some if statements with a ternary operator.

s = s ?? throw new ArgumentNullException(nameof(s));
validator = validator ?? BracketValidator.CreateDefaultValidator();

Redundant type declarations can be replaced by var. Unlike javascript, var is ok in C#.

  Stack<char> openStack = new Stack<char>();

var openStack = new Stack<char>();


Read file with over 300k words and filter through appropriate filter to return list of matching words As a puzzle we were asked to bring out the possibilities in breaking a 4 letter wordlock combination. It was to be a four letter word from the English dictionary. There were 10 possible characters in each place. So \$10^4\$ possibilities of random words. I brute forced my code to return a list of possible words by first reading a file of 300k English words and storing it in a list. Then I applied a filter at each place to narrow down the list of possible words that could work on the combo.

I know I am brute forcing the code down to 1089 possibilities, did I overlook other possibilities? I am a beginner and if I wanted to learn how to improve the efficiency of my code and make it simpler/functioning, what steps can I take?

filterlist2 = ['a','r','t','h','i','v','o','y','l','e',]
filterlist3 = ['a','l','t','o','i','n','s','r','m','f',]
filterlist4 = ['a','m','d','k','e','s','x','p','l','y',]

#filterlist1 basically before I realized I can store parameters in a list
lines = tuple(open("words.txt", 'r'))
char1filter=[]
for element in lines:
    if len(element)==5:
        if element.startswith('b') is True:
            char1filter.append(element)
        elif element.startswith('p') is True:
            char1filter.append(element)
        elif element.startswith('t') is True:
            char1filter.append(element)
        elif element.startswith('s') is True:
            char1filter.append(element)
        elif element.startswith('m') is True:
            char1filter.append(element)
        elif element.startswith('d') is True:
            char1filter.append(element)
        elif element.startswith('c') is True:
            char1filter.append(element)
        elif element.startswith('g') is True:
            char1filter.append(element)
        elif element.startswith('f') is True:
            char1filter.append(element)
        elif element.startswith('l') is True:
            char1filter.append(element)

#print(char1filter)

print(len(char1filter))
#returned 3935 results
#filter 2 for second character
char2filter =[]
for element1 in char1filter:
    if element1[1] in filterlist2:
        char2filter.append(element1)
print('char2 filter applied')
print(char2filter)
print(len(char2filter))
#filter 2 returned 3260 words
#filter 3

char3filter =[]
for element2 in char2filter:
    if element2[2] in filterlist3:
        char3filter.append(element2)
print(char3filter)
print(len(char3filter))
#filter 3 returned 1991 words
#filter 4
char4filter =[]
for element3 in char3filter:
    if element3[3] in filterlist4:
        char4filter.append(element3)
print(char4filter)
print(len(char4filter))

#filter 4 returned 1089 words

First off I'd change how you are making char1filter.
Here's what I'd do differently:

Remove the is True as it's implied.
Change the element.startswith('') to element[0] == 'a'.
Merge all the element checks into one, using in. element[0] in 'bptsmdcgfl'.
Use a list comprehension.

Merging all this together we should get:

char1filter = [
    element
    for element in lines
    if len(element) == 5 and element[0] in 'bptsmdcgfl'
]

Id however store 'bptsmdcgfl' with filterlist2, filterlist3 and filterlist4.
And instead just remove all element that aren't 5 long.

five_long = [
    element
    for element in lines
    if len(element) == 5
]

This leaves you with four roughly identical pieces of code.
And so I'd change all the filterlists to be a single list to loop through.
And then perform the transformations inside this.

data = five_long
for to_filter in filter_lists:
    data = [
        element
        for element in data
        if element[0] in to_filter
    ]
    print(data)
    print(len(data))

If however you just want the last filter list, then you can merge all the filters into one, and then do a single filter on that.

filter_list = [e for l in filter_list for e in l]
data = [
    element
    for element in five_long
    if element[0] in filter_list
]
print(data)
print(len(data))

You should also read PEP8 to style your code correctly, like I have above, which makes your code much easier to read.
You should also use with when opening files, tuple(open('')) is bad and should instead be:

with open('words.txt', 'r') as f:
    lines = tuple(f)

And so the code could be:

FILTER_LISTS = [
    ['b', 'p', 't', 's', 'm', 'd', 'c', 'g', 'f', 'l'],
    ['a', 'r', 't', 'h', 'i', 'v', 'o', 'y', 'l', 'e'],
    ['a', 'l', 't', 'o', 'i', 'n', 's', 'r', 'm', 'f'],
    ['a', 'm', 'd', 'k', 'e', 's', 'x', 'p', 'l', 'y'],
]

with open('words.txt', 'r') as f:
    lines = tuple(f)

data = [
    element
    for element in lines
    if len(element) == 5
]
for to_filter in FILTER_LISTS:
    data = [
        element
        for element in data
        if element[0] in to_filter
    ]
    print(data)
    print(len(data))


Updating an Nginx config file for SSL This script updates a settings in ssl config file for Nginx. I generate DHEC key and then update the Nginx SSL file to that location.  I also give an option for the user through arguments if they want to change the bit size of the key and if they want more conservative or hardened SSL cipher suites.

I don't know how to deal with the long wait on a large key because the user might think the program froze because it takes a long time generate. Its dependent on the environment of course how long you wait. How would you deal with this, through a percentage output? I'm using a spinner at the moment which does the same thing as the 'openssl' command does. So I will probably not use it if I can't find something uses a line or two at most instead of continuous outputting and pushing everything off screen.

#!/bin/bash

source /etc/global.conf # load global vars
source setup/functions.sh #functions

apt_install openssl

nginx_ssl_conf=/etc/nginx/nginx-ssl.conf
DEFAULT_BIT_SIZE=2048
isHardened="false"
hardened_ciphers="'ECDH+AESGCM256:DH+AESGCM256:ECDH+AES256:SH+AES256:RSA+AESGCM256:RSA+AES256:!aNULL:!MD5:!kEDH';"
hardened_protocol="TLSv1.2;"

DHEC_path=$STORAGE_ROOT/ssl/dhparam.pem

# Functions
update_config()
{
    lineNUM=$(grep -n "$2" $1 | tail -n1 | sed 's/:.*//')
    [ "$lineNUM" ] || lineNUM="$"
    sed -i -r "$lineNUM s|#?(.*)|#\1\n$4\n$2 $3|" "$1"
}

ok()
{
    echo -e '\e[32m'$1'\e[m';
}

spinner()
{
    local pid=$1
    local delay=0.75
    local spinstr='|/-\'
    while [ "$(ps a | awk '{print $1}' | grep $pid)" ]; do
        local temp=${spinstr#?}
        printf " [%c]  " "$spinstr"
        local spinstr=$temp${spinstr%"$temp"}
        sleep $delay
        printf "\b\b\b\b\b\b"
    done
    printf "    \b\b\b\b"
}

# Usage info
usage()
{
    cat << EOF
    Usage: ${0##*/} [-h] [-p DIR_DHEC_KEY] [-b BIT_SIZE]  [-c DIR_NGINX_SSL]...
    This script generates and enables DHEC for Nginx.  Defaulted to 2048 key.
    Hardened mode will generate 4096 key and the following cipher suites:
    'ECDH+AESGCM256:DH+AESGCM256:ECDH+AES256:SH+AES256:RSA+AESGCM256:RSA+AES256:!aNULL:!MD5:!kEDH'

    -h          Enable hardened ciphers and 4096 bit key.
    -p          Specify dir to generate the DHEC key.
    -c          Specify dir nginx ssl conf is.
    -b          Specify the bit size to generate which will override any other default.
EOF
    exit 1
}

while getopts "hb:c:p:" opt ; do
    case "${opt}" in
        b)
            BIT_SIZE=${OPTARG}
            if [ -z "${OPTARG}" ]; then
               usage
            fi
            ;;
        h)
            isHardened=true
            BIT_SIZE=4096
            ;;
    p)
            DHEC_path=${OPTARG}
        if [ -z "${OPTARG}" ]; then
               usage
            fi
            ;;
        c)
            nginx_ssl_conf=${OPTARG}
            if [ -z "${OPTARG}" ]; then
               usage
            fi
            ;;
        *)
            usage
            ;;
    esac
done
shift $((OPTIND-1))

if [ -z "${BIT_SIZE}" -a "true" == ${isHardened} ]; then
    BIT_SIZE=4096
elif [ -z "${BIT_SIZE}" ]; then
    BIT_SIZE=$DEFAULT_BIT_SIZE
fi

ok "â¯â¯â¯ Generating DH elliptic curve key.  It might take a while, grab a coffee!"
ok "Seriously, $BIT_SIZE takes forever!"

if [ ! -f $DHEC_path ]; then
  # Generate a 4096 bit random parameter for DH elliptic curves.
  # Generated by OpenSSL with the following command:
  #   openssl dhparam -outform pem -out dhparam.pem 2048
  #   openssl dhparam -outform pem -out dhparam.pem 4096
   ( openssl dhparam -outform pem -out $DHEC_path $BIT_SIZE) &
    spinner $!
fi
        update_config $nginx_ssl_conf ssl_dhparam $DHEC_path';' "#Path to DHEC key"

if [ $isHardened == "true" ]; then
    update_config $nginx_ssl_conf ssl_ciphers $hardened_ciphers "#Hardened SSL Ciphers DHEC"
        update_config $nginx_ssl_conf ssl_protocols $hardened_protocol "#Hardened SSL Protocol"
fi

service nginx reload

How would you deal with this, through a percentage output?

I don't know a better way. It seems fine the way you did it, using a spinner.

Quoting

The quoting is unnecessary here:

isHardened="false"

You can safely write this simply as:

isHardened=false

Here, on the other hand, you should quote $1:

lineNUM=$(grep -n "$2" $1 | tail -n1 | sed 's/:.*//')

And you could use sed to include what tail does:

lineNUM=$(grep -n "$2" "$1" | sed -ne '$s/:.*//p')

Prefer [[ ... ]] instead of [ ... ]

[ "$lineNUM" ] || lineNUM="$"

[ ... ] is obsoleted, unless you require POSIX compatibility. Using [[ ... ]] is recommended, and it let's you simplify the quoting:

[[ $lineNUM ]] || lineNUM="$"

Don't use echo for non-trivial things

Avoid using echo with any of it's flags, like -e, because it's not portable. You also don't need the trailing ; here:

echo -e '\e[32m'$1'\e[m';

It's better to rewrite using printf:

printf '\e[32m%s\e[m\n' $1

Simplify logic

You evaluate -z "${BIT_SIZE}" twice, unnecessarily:

if [ -z "${BIT_SIZE}" -a "true" == ${isHardened} ]; then
    BIT_SIZE=4096
elif [ -z "${BIT_SIZE}" ]; then
    BIT_SIZE=$DEFAULT_BIT_SIZE
fi

You can simplify the logic, and using [[ ... ]] also helps:

if [[ ! $BIT_SIZE ]]; then
    if [[ $isHardened = true ]]; then
        BIT_SIZE=4096
    else
        BIT_SIZE=$DEFAULT_BIT_SIZE
    fi
fi

Formatting

The formatting is strange here, and it makes your script harder to read:

if [ ! -f $DHEC_path ]; then
  # Generate a 4096 bit random parameter for DH elliptic curves.
  # Generated by OpenSSL with the following command:
  #   openssl dhparam -outform pem -out dhparam.pem 2048
  #   openssl dhparam -outform pem -out dhparam.pem 4096
   ( openssl dhparam -outform pem -out $DHEC_path $BIT_SIZE) &
    spinner $!
fi
        update_config $nginx_ssl_conf ssl_dhparam $DHEC_path';' "#Path to DHEC key"

Write like this instead:

if [[ ! -f $DHEC_path ]]; then
    # Generate a 4096 bit random parameter for DH elliptic curves.
    # Generated by OpenSSL with the following command:
    #   openssl dhparam -outform pem -out dhparam.pem 2048
    #   openssl dhparam -outform pem -out dhparam.pem 4096
    (openssl dhparam -outform pem -out $DHEC_path $BIT_SIZE) &
    spinner $!
fi
update_config $nginx_ssl_conf ssl_dhparam $DHEC_path';' "#Path to DHEC key"


Repurposing UITableView infinite scroll configuration/properties Had my first go at implementing infinite scrolling for UITableView's in my iOS app last night, and I noticed I ended up copying/pasting quite a bit of boilerplate from one VC to the next (if I wanted infinite scroll on it).

By the way, I'm fairly new to iOS, so this question is probably more broad than just specifically keeping infinite scroll config DRY...

I have properties like these:

@property(nonatomic) int checkins_page;
@property(nonatomic) int checkins_total;
@property(nonatomic) int checkins_total_pages;
@property(nonatomic) BOOL checkins_load_more;
@property(nonatomic) int watchlist_page;
@property(nonatomic) int watchlist_total;
@property(nonatomic) int watchlist_total_pages;
@property(nonatomic) BOOL watchlist_load_more;

I have two UITableViews in this VC, so that's fun.

Then I have a delegate method:

-(void)scrollViewDidScroll:(UIScrollView *)scrollView
{

    if(scrollView == self.checkinsTableView && !self.checkins_load_more){
        return;
    }
    if(scrollView == self.watchlistTableView && !self.watchlist_load_more){
        return;
    }
    CGFloat height = scrollView.frame.size.height;

    CGFloat contentYoffset = scrollView.contentOffset.y;

    CGFloat distanceFromBottom = scrollView.contentSize.height - contentYoffset;

    if(distanceFromBottom < height)
    {
        if(scrollView == self.checkinsTableView){
            self.checkins_load_more = NO;
            if(self.checkins_page < self.checkins_total_pages){
                self.checkins_page++;
                [self downloadCheckins];
            }
        }
        if(scrollView == self.watchlistTableView){
            self.watchlist_load_more = NO;
            if(self.watchlist_page < self.watchlist_total_pages){
                self.watchlist_page++;
                [self downloadWatchlist];
            }
        }
    }
}

Now I'm seeing a lot of this stuff could be abstracted...somewhere...somehow, maybe into a more general purpose infinite scroll VC, but like I'm said, quite new to iOS, so I'm not sure the best way to go about reducing this boilerplate code.

Any advice/pointers would be appreciated on how to clean it up slightly?

I do not see a huge problem with the structure of this code.  It is not a huge number of lines repeated, and the contents of the if structures are obviously different as far as which variables they act upon.  That being said, I do believe that it can be made more clear and concise by using objects to contain some of the data.

Without seeing the whole context of the code, it looks to me like self.checkinsTableView and self.watchlistTableView are basically the same.  If their contents can be converted into a class (possibly a subclass of the Apple class, possibly not, sometimes this can be tricky) then you would be able to more easily break things down into smaller methods.

You would make the following things properties of the class:

@property(nonatomic) int page;
@property(nonatomic) int total;
@property(nonatomic) int total_pages;
@property(nonatomic) BOOL load_more;

Then you would change the if statements to look like this:

if(distanceFromBottom < height)
{
    if(scrollView == self.checkinsTableView){
        [self processThing:checkinsClass]
    }
    if(scrollView == self.watchlistTableView){
        [self processThing:watchListclass];
    }
}

-(void) processThing:(YourClass *)object {
    object.load_more = NO;
    if(class.page < object.total_pages){
        object.page++;
        [self downloadList:object];
    }
}

This class could possibly be integrated into the self.watchListTableView class itself to shorten the code even further.  I haven't subclassed TableView (I'm assuming that's what this is based on the name) so I don't know if it is easy/possible or not.

It might make sense to move these checks into another method for simplicity, but it is hard to say based on the context:

if(scrollView == self.checkinsTableView && !self.checkins_load_more){
    return;
}
if(scrollView == self.watchlistTableView && !self.watchlist_load_more){
    return;
}

I tried to write an example method for this, but I think it would only work easily if you subclassed the TableView, and even if you did you would have to pass in the scrollView also to perform the check, so the way that you have it might be better.

Finally I would like to talk about a couple formatting issues.  Variables in Objective-C should be camelCase, so watchlist_load_more should be watchlistLoadMore.

Also I recommend more white space in a few areas.  @property(nonatomic) BOOL watchlist_load_more; would look better with a space after property.  Your if statements are a bit crowded. if(scrollView == self.checkinsTableView){ would read easier as if (scrollView == self.checkinsTableView) {.

Finally, I would choose one format for {} brackets and stick with it.  I believe that it is fine to have the bracket either on the same line, or on the next line, but you alternate between these two options in the code.  I would recommend that you pick one and be consistent with it. These are minor nitpicks, but I think they would help readability.


Reading a binary file containing periodic samples I have the following code for reading HTK feature files. The code below is working completely correct (verified it with unit tests and the output of the original HTK toolkit).

from HTK_model import FLOAT_TYPE
from numpy import array
from struct import unpack

def feature_reader(file_name):
    with open(file_name, 'rb') as in_f:
        #There are four standard headers. Sample period is not used
        num_samples = unpack('>i', in_f.read(4))[0]
        sample_period = unpack('>i', in_f.read(4))[0]
        sample_size = unpack('>h', in_f.read(2))[0]
        param_kind = unpack('>h', in_f.read(2))[0]

        compressed = bool(param_kind & 02000)

        #If compression is used, two matrices are defined. In that case the values are shorts, and the real values are:
        # (x+B)/A
        A = B = 0
        if compressed:
            A = array([unpack('>f',in_f.read(4))[0] for _ in xrange(sample_size/2)], dtype=FLOAT_TYPE)
            B = array([unpack('>f',in_f.read(4))[0] for _ in xrange(sample_size/2)], dtype=FLOAT_TYPE)
            #The first 4 samples were the matrices
            num_samples -= 4

        for _ in xrange(0,num_samples):
            if compressed:
                yield ((array( unpack('>' + ('h' * (sample_size//2)),in_f.read(sample_size)) ,dtype=FLOAT_TYPE) + B) / A)
            else:
                yield (array( unpack('>' + ('f' * (sample_size//4)),in_f.read(sample_size)), dtype=FLOAT_TYPE))

How can I speed up this code, are there things I should improve in the code?

data = in_f.read(12)
    num_samples, sample_period, sample_size, param_kind = unpack('>iihh', data)
    A = B = 0
    if compressed:
        A = array('f')
        A.fromfile(in_f, sample_size/2)
        B = array('f')
        B.fromfile(in_f, sample_size/2)
        #The first 4 samples were the matrices
        num_samples -= 4

And so on


Handling multiple dimensions of arrays with speed I'm looking to learn how to write efficient Haskell code. This snippet works, but the run time is slow. How can I write faster Haskell code?

Input: maxPrice = 7

Input: vendorsDelivery = [5, 4, 2, 3]

Input: vendorsProducts = [[1, 1, 1],[3, -1, 3],[-1, 2, 2],[5, -1, -1]]

Output: [1,2] (fastest delivery time for a given order)

import Data.List
import Data.List.Split
import Control.Monad

data Vendor a = Vendor { vendorNbr :: Int
                       , deliveryTime :: Int
                       , itemPrice :: Int
                       } deriving (Show)

populate d v = map (filter (\x -> itemPrice x /= (-1))) $ transpose $ map (\(x,y,z) -> map (\xs -> Vendor x y xs) z) $ zip3 [0..] d v

minimalBasketPrice mP vD vP = nub . get1 $ foldr (\x xs -> if get2 x < (get2 xs) then x else xs) (possible !! 0) possible
    where combos = sequence (populate vD vP)
          possible = filter (\(_,_,x) -> x <= mP) [((accu vendorNbr y), (total deliveryTime y), (total itemPrice y)) | y <- combos]

-- | Helper Functions

get1 (x,_,_) = x
get2 (_,x,_) = x

total f = foldr (\x y -> (f x) + y) 0
accu f = foldr (\x y -> (f x) : y) []

Have some rewrite rules applied, combinators used and helpers inlined while I think about your algorithmic question. And make Vendor a newtype.

minimalBasketPrice mP vD
  = nub
  . map vendorNbr
  . minimumOn (sum . map deliveryTime)
  . filter ((<= mP) . sum . map itemPrice)
  . sequence
  . (map . filter) ((/= -1) . itemPrice)
  . transpose
  . zipWith3 (\x y -> map (Vendor x y)) [0..] vD

This is the backpack problem and NP-hard. We won't be scaling well in the worst-case whatever we do, but we can do a little better by, for example, only keeping track of suffixes of vendor selections that can't be undercut in time and price at the same time:

newtype Candidate a = Candidate
  { vendors :: [Int]
  , time :: Int
  , price :: Int
  }

minimalBasketPrice mP vD
  = nub . vendors . head
  . foldr (foo mP) [Candidate [] 0 0]
  . map (filter ((/= -1) . itemPrice))
  . transpose
  . zipWith3 (\x y -> map (Vendor x y)) [0..] vD

foo :: Int -> [Vendor] -> [Candidate] -> [Candidate]
foo mP choices
  = catMaybes . snd
  . mapAccumL bar (mP+1)
  . sortOn time
  . liftA2 addVendor choices

bar :: Int -> Candidate -> (Int, Maybe Candidate)
bar bestprice candidate = if price candidate < bestprice
  then (price candidate, Just candidate)
  else (bestprice, Nothing)

addVendor (Vendor v t p) (Candidate vs ts ps) = Candidate (v:vs) (t+ts) (p+ps)


Keeping an observable collection up to date I'm using an ObservableCollection in a WPF app.  The source of the data doesn't provide changes. It provides snapshots instead.  The method I wrote to update the ObservableCollection works but it seems inelegant. Is there a better way to do this?

    public static void UpdateMarketOrderList(
                          ObservableCollection<MarketOrder> oldList,
                          List<MarketOrder> newList)
    {
        int oi = 0; // old list iterator
        int ni = 0; // new list iterator

        while (ni < newList.Count)
        {
            if (oi == oldList.Count)
            {
                oldList.Insert(oi, newList[ni]);
            }
            else if (oldList[oi].Price == newList[ni].Price)
            {
                if (oldList[oi].Quantity != newList[ni].Quantity)
                {
                    oldList[oi].Quantity = newList[ni].Quantity;
                }
                ++oi;
                ++ni;
            }
            else if (oldList[oi].Price < newList[ni].Price)
            {
                oldList.RemoveAt(oi);
            }
            else if (oldList[oi].Price > newList[ni].Price)
            {
                oldList.Insert(oi, newList[ni]);
            }
        }

        while (oldList.Count > ni)
        {
            oldList.RemoveAt(ni);
        }
    }

If it matters, the definition MarketOrder can be considered to be this:

public class MarketOrder
{
    public decimal Price { get; set; }
    public decimal Quantity { get; set; }
}

After studying your code snippet for a bit here's what I came up with.

I wouldn't call them iterators. They're really acting more as indexing operators that doubles as a loop counter. Furthermore, they're always incremented in sync and they have the exact same initial value.

int oi = 0; // old list iterator
int ni = 0; // new list iterator

So I would replace with simply:

int i = 0;

The second loop for trimming the extra elements:

while (oldList.Count > ni)
{
  oldList.RemoveAt(ni);
}

The post-condition for i after the first loop has to be equal to newList.Count otherwise we would still be looping the first while. Thusly, changing the condition to this is clearer:

while (oldList.Count > newList.Count)

After tracing your UpdateMarketOrderList method, the discernible end effect, from what I can tell, just seems to be a replacement of the oldList with the newList. If that's really the desired end consequence of running this function, why not simplify it, succinctly to this instead?

public static void UpdateMarketOrderList( ObservableCollection<MarketOrder> oldList,
                                          List<MarketOrder> newList )
{
    oldList.Clear();

    for(int i = 0; i < newList.Count; ++i)
    {
        oldList.Add(newList[i]);
    }
}

Then again you didn't specify in your question what the pre/post-conditions are for UpdateMarketOrderList, which is something that does matter.

After reading OP's comment that fired change events are important, I would refactor the main loop like this:

    while (i < newList.Count)
    {
        if (i == oldList.Count ||
            oldList[i].Price > newList[i].Price)
        {
            oldList.Insert(i, newList[ni]);
        }
        else if (oldList[i].Price < newList[i].Price)
        {
            oldList.RemoveAt(i);
            continue;
        }
        else if (oldList[i].Quantity != newList[i].Quantity)
              // && oldList[i].Price == newList[i].Price
        {
            Debug.Assert(oldList[i].Price == newList[i].Price);
            oldList[i].Quantity = newList[i].Quantity;
        }

        ++i;
    }

With this refactor you decrease the indent level and tighten the loop a bit while preserving existing behavior. The key thing to realize is that RemoveAt doesn't increment the loop counter. Your previous code did that in a slightly less direct manner.

My gut feeling is that this could be refactored further still but without a good idea of the expected side-effects and other requirements for this method, it will be difficult to refactor this safely.


USACOs Arithmetic Progression I was working on the USACO problem arithmetic progression.

You can see the problem (and a solution) here - http://massivealgorithms.blogspot.com/2015/08/arithmeticprogressions-codetrick.html

My code successfully runs for the first 7 test cases. However, once it reaches test case 8 (22,250) it times out.
What modifications should I make to make my code more efficent? Most solutions I found online were also o(m^2*n) complexity, so I'm not sure what I need to change.

Here is my code:

 //#include "stdafx.h"
    #include <iostream>
    #include <fstream>
    #include <string>
    #include <algorithm>
    #include <vector>
    using namespace std;

int main()
{
    ofstream fout("ariprog.out");
    ifstream fin("ariprog.in");
    int n, m;
    fin >> n >> m;
    int bisquare[125001] = {};

    for (int a = 0; a <= m; a++) {
        for (int b = a; b <= m; b++) {
            bisquare[(a*a) + (b*b)] = 1;
        }
    }
    int max = 2 * m*m;
    int value;
    bool tb = false;
    pair<int, int> all[125001];
    int yeah = 0;
    for (int s = 0; s <= max/2; s++) {
        for (int i = 1;i<=max/2; i++) {
            for (int x = 0; x < n; x++) {
                value = s + x*i;

                if (value > max) {
                    tb = true;
                    break;
                }
                if (!bisquare[value]) break;
                if (bisquare[value] && x==n-1)  {
                    all[yeah] = make_pair(i,s);
                    yeah++;
                }
            }
            if (tb) {
                tb = false;
                break;
            }
        }
    }
    sort(all,all+yeah);
    for (int i = 0; i < yeah; i++) {
        fout << all[i].second << " " << all[i].first << endl;
    }
    if (all[0].first == 0) {
        fout << "NONE" << endl;
    }
}

Faster searching

Suppose you have an array filled with either 1 and 0, and you are trying to search for a consecutive sequence of twenty 1's in a row.  The way you are doing it is like this:

111111111111111100001111111110111011111111
^               ^
|...............|
start here      end here  length = 15, not long enough, so move on

Next iteration:

111111111111111100001111111110111011111111
 ^              ^
 |..............|
 start here     end here  length = 14, not long enough, so move on

Next iteration:

111111111111111100001111111110111011111111
  ^             ^
  |.............|
  start here    end here  length = 13, not long enough, so move on

As you can imagine, the above algorithm is not very efficient because you are wasting time looking at the same array values over and over again.  The above can take up to \$O(M*N)\$ time, where \$M\$ is the length of the string and \$N\$ is the length of the sequence you are searching for.

Instead of restarting your search from the next index, you should be restarting your search from just past the zero you found:

111111111111111100001111111110111011111111
^               ^
|...............|
start here      end here  length = 15, not long enough, so move on

Next iteration:

111111111111111100001111111110111011111111
                 ^
                 |
                 Restart here, not back there

By doing this, the search now only takes \$O(M)\$ time.  You could do even better by mimicking the Boyer Moore string search algorithm and skipping to the end and searching backwards, like this:

111111111111111101111111111110111011111111
^
|
start here

111111111111111101111111111110111011111111
                   ^
                   |
                   jump 19 spots ahead and search backwards

111111111111111101111111111110111011111111
                ^
                |
                Found a zero here

111111111111111101111111111110111011111111
                 ^
                 |
                 Restart here

This is still an \$O(M)\$ algorithm in the worst case, but it can be as fast as \$O(M/N)\$ in the best case (an array filled with all 0s).

Loop rearrangement

The way you have set up your loops does not lend itself to doing the faster searching methods.  You will need to rearrange your loops in order to be able to restart your searches properly.  I suggest you move your x loop from being the innermost loop to being the outermost loop.  Then you may find that the two other loops collapse into a single search loop.


Computing word-frequency-table I got a word count assignment and the requirements are shown below:

  INPUT

  The input contains words in several lines and in each line
  there could be more than one word. The input only contains English
  words, separated by space or line breaks. Words are case-sensitive.

  OUTPUT

  Print the words in alphabetic order with the corresponding appearance
  count in the text. For each word, please print the result in a single line
  (separate the word and the number with space).

Sample Input:

Computer system
computer design
algorithm design and analysis
quantum computer
computer science Department

Sample Output:

Computer 1
algorithm 1
analysis 1
and 1
computer 3
department 1
design 2
quantum 1
science 1
system 1

I tried something shown below and actually got the same output as the sample but still got marked as wrong answer by the assignment judge system.
Is there anything I did wrong?

#include <iostream>
#include <algorithm>
#include <string>
#include <sstream>
#include <iterator>
#include <vector>
using namespace std;

int main() {

    string whole;
    string line;
    string word;

    do {
        getline(cin, line);
        whole += line + ' ';
    } while (!line.empty());

    if (!whole.empty()) {
        istringstream iss(whole);
        vector<string> results((istream_iterator<string>(iss)), istream_iterator<string>());

        int count = 1;
        int size = results.size();
        if (size == 1) cout << results[0] << ' ' << count;
        else {
            sort(results.begin(), results.end());
            for (int j = 0; j < size; j++) {
                if (j != size - 1) {
                    if (results[j] != results[j + 1]) {
                        string word = results[j];
                        cout << word << ' ' << count << endl;
                        count = 1;
                    }
                    else if (results[j] == results[j + 1]) count++;
                }
                else if (j == size - 1) {
                    if (results[j] != results[j - 1]) {
                        string word = results[j];
                        cout << word << ' ' << count << endl;
                    }
                    else if (results[j] == results[j - 1]) cout << results[j] << ' ' << count << endl;
                }
            }
        }
    }
    return 0;
}

@Deduplicator said almost everything, I will only add that it's really important that you know algorithms and data structures from the STL and using the proper in each case.
Here, you do so much work for nothing:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <string>
#include <map>

int main() {
    auto in = std::istream_iterator<std::string>{std::cin};
    auto out = std::map<std::string, int>{};

    std::for_each(in, {}, [&out](const std::string& w) { out[w]++; });
    for (auto& w : out) { std::cout << w.first << ' ' << w.second << '\n'; }
}


Similar methods to update today's sales, revenue, and profit Please help me to refactor. I have two models, conversion & statistic.
Methods sales_today, revenue_today, profit_today in conversion model only gets specific info from db.

Statistic model

class Statistic < ActiveRecord::Base

class << self

def create_and_update_today(agregate_name, value, network)
  today = find_or_create_by(agregateName: agregate_name, networkName: network)
  today.update(agregateName: agregate_name, date: Date.today,
                value: value, networkName: network)
end

def sales_today(network)
  total = Conversion.sales_today(network)
  create_and_update_today('salesToday', total, network)
end

def revenue_today(network)
  total = Conversion.revenue_today(network)
  create_and_update_today('revenueToday', total, network)
end

def profit_today(network)
  total = Conversion.profit_today(network)
  create_and_update_today('profitToday', total, network)
end

end
end

Conversion model

class Conversion < ActiveRecord::Base

  class << self

def sales_today(network)
  where('DATE(created_at)= ?  AND networkName = ?', Date.today, network).size
end
etc...

end

Is there any way to refactor almost similar pieces of code?

Instead of dynamically defining a bunch of methods, you could just have one method that takes an argument that tells it what kind of work to do:

def self.create_or_update_today(kind, network)
  meth = :"#{kind}_today"
  total = Conversion.send(meth, network)

  aggregate_name = "#{kind}Today"

  today = find_or_initialize_by(agregateName: aggregate_name, networkName: network)
  today.update(date: Date.today, value: total)
end

You would call it like this:

Statistic.create_or_update_today(:sales, network)

(You'll notice I used find_or_initialize_by instead of find_or_create_by. This way the code will always perform two queries instead of performing three if it's a new record. update will create the record of it doesn't exist.)

If you wanted you could still define individual methods, but this way it's much cleaner:

def self.sales_today(network)
  self.create_or_update_today(:sales, network)
end

def self.revenue_today(network)
  self.create_or_update_today(:revenue, network)
end

# ...

You could also do this with define_method in a loop as Devon does, but it would save very few lines while sacrificing a lot of readability and, as britishtea points out, introducing documentation issues.

P.S. Just a tip. Don't use size for this:

where('DATE(created_at) = ?  AND networkName = ?',
      Date.today, network).size

You only want to know how many records there are, but when you do this it fetches all of the data in all of the records and initializes a Conversion object for each, which is really wasteful. Instead of size, use Rails' built-in count method:

where('DATE(created_at) = ?  AND networkName = ?',
      Date.today, network).count

This will let the database perform the calculation so ActiveRecord only needs to fetch a single number.


Select top 2 employees based on Grade and Alphabetical Order Given a list of Employees ( name and grade). Output the names of the top 2 employees selected for a task(sorted based on grade and then alphabetically).

I am using a maxHeap and override the compareTo method of the Employee class
to define the sorting order while initializing the heap .I would like to know if there is a more efficient way(space/time).

The Code:

//Employee class
class Employee implements Comparable<Employee>{
    String name;
    int grade;

    public Employee (String name,int grade){
        this.name = name;
        this.grade = grade;
    }

    @Override
    public int compareTo(Employee e) {
        if(this.grade == e.grade) {
            // if the grades are the same then alphabetical priority
            return e.name.compareTo(this.name);
        }

        if(this.grade < e.grade) return -1;
            return 1;
        }
    }

Class to build max heap and print out two names

class EmployeePicker {
    public static void main(String[] args) {

        PriorityQueue<Employee> employeeHeap = new PriorityQueue<>(5,Collections.reverseOrder());

        // add 5 employees
        employeeHeap.add(new Employee("Adam", 5));
        employeeHeap.add(new Employee("Annie",7));
        employeeHeap.add(new Employee("Amit", 5));
        employeeHeap.add(new Employee("Carl", 5));
        employeeHeap.add(new Employee("Tom", 4));

        // print first two names
        System.out.println(employeeHeap.poll().name);
        System.out.println(employeeHeap.poll().name);
    }
}

About performance

As a forewords, I'm not sure you should be obsessed with performance as a beginner (actually I'm pretty sure you should not but surely others will disagree).

Consider reading : https://softwareengineering.stackexchange.com/questions/80084/is-premature-optimization-really-the-root-of-all-evil

With that said...

In the Employee class :

As said by coderodde, your compareTo can't be made faster.

In the EmployeePicker class :

PriorityQueue isn't made with performance in mind.
Seeing your code there is no point in using this class over a TreeSet and I'd really recommend using a TreeSet over anything else.

If you really are obsessed with performance, PriorityQueue may outperform a TreeSet when you have over 20_000 elements for your specific use case.

However, if you are really really obsessed with performance and you know all the employees as soon as the program starts (and then only have to retrieve some of them after that) then you should consider using an array that you sort once... it'll also be the most space-efficient solution of the three.

With less than a hundred elements, I'm pretty sure you can consider all solutions equals in performance.

Conclusion : IMO use TreeSet unless you have good reasons not to :)

About code format

You should make the fields of Employee private and final and create getters for them. When a method have multiple parameters, you should always follow the comma with a space (makes code more readable).

The indentation in compareTo is really confusing.


Logging the name of recipes using ingredients Given two objects with some food data, log the name of the recipe and ingredients names separated by conjunction "and". Please run the code I have attached to see the expected output.

condition - match RECIPES.rules.id with INGREDIENT.id and use INGREDIENT.name in the final output.

I was wondering if my solution can be improved? The fact that I am looping array inside an existing array loop makes me think this solution is not good.

const INGREDIENTS = [
  {
    id: 'chocolate',
    name: 'Chocolate Ice Cream',
    type: 'ice_cream',
    contains: ['cream', 'milk_chocolate']
  },
  {
    id: 'm_and_ms',
    name: 'M&Ms',
    type: 'topping',
    contains: ['milk_chocolate']
  },
  {
    id: 'lemon',
    name: 'Lemon Ice Cream',
    type: 'ice_cream',
    contains: ['cream', 'lemon']
  },
  {
    id: 'peanuts',
    name: 'Peanuts',
    type: 'topping',
    contains: ['peanuts']
  },
  {
    id: 'vanilla',
    name: 'Vanilla Ice Cream',
    type: 'ice_cream',
    contains: ['cream', 'vanilla_extract']
  },
  {
    id: 'strawberry',
    name: 'Strawberry Ice Cream',
    type: 'ice_cream',
    contains: ['cream', 'strawberries']
  },
  {
    id: 'caramel',
    name: 'Caramel Sauce',
    type: 'topping',
    contains: ['caramel']
  },
  {
    id: 'raspberry',
    name: 'Raspberry Sauce',
    type: 'topping',
    contains: ['raspberries']
  }
];

// DOCSTRING
const RECIPES = [
  {
    id: 'the_basic',
    name: 'The Basic',
    rules: ['chocolate', 'vanilla']
  },
  {
    id: 'chocolate_lovers',
    name: 'Chocolate Lovers',
    rules: ['chocolate', 'm_and_ms', 'caramel']
  },
  {
    id: 'tooty_fruity',
    name: 'Tooty Fruity',
    rules: ['strawberry', 'raspberry', 'm_and_ms']
  }
];

const printRecipe = () => {
  RECIPES.forEach(recipe => {
    let recipeName = recipe.name;

    let listOfItems = recipe.rules.reduce((acc, currentIngredient, index) => {
      let result;
      let items = INGREDIENTS.find(ingrd => {
        return ingrd.id === currentIngredient;
      });
      if (acc.length === 0) {
        result = items.name;
      } else {
        result = `${acc} and ${items.name}`;
      }
      return result;
    }, '');
    console.log(`${recipeName} - ${listOfItems}`);
  });
};

printRecipe();

I would suggest two improvements:

Use a Map to have a faster lookup in your ingredients. find needs to iterate through an array every time, while if you create a Map for it, you can get each result in constant time
Use the map array method (unrelated to the above mentioned Map object) in combination with join instead of reduce.

Here is how that would look:

const INGREDIENTS = [{id: 'chocolate',name: 'Chocolate Ice Cream',type: 'ice_cream',contains: ['cream', 'milk_chocolate']},{id: 'm_and_ms',name: 'M&Ms',type: 'topping',contains: ['milk_chocolate']},{id: 'lemon',name: 'Lemon Ice Cream',type: 'ice_cream',contains: ['cream', 'lemon']},{id: 'peanuts',name: 'Peanuts',type: 'topping',contains: ['peanuts']},{id: 'vanilla',name: 'Vanilla Ice Cream',type: 'ice_cream',contains: ['cream', 'vanilla_extract']},{id: 'strawberry',name: 'Strawberry Ice Cream',type: 'ice_cream',contains: ['cream', 'strawberries']},{id: 'caramel',name: 'Caramel Sauce',type: 'topping',contains: ['caramel']},{id: 'raspberry',name: 'Raspberry Sauce',type: 'topping',contains: ['raspberries']}];
const RECIPES = [{id: 'the_basic',name: 'The Basic',rules: ['chocolate', 'vanilla']},{id: 'chocolate_lovers',name: 'Chocolate Lovers',rules: ['chocolate', 'm_and_ms', 'caramel']},{id: 'tooty_fruity',name: 'Tooty Fruity',rules: ['strawberry', 'raspberry', 'm_and_ms']}];

const keyedIngredients = new Map(INGREDIENTS.map(o => [o.id, o.name]));
const printRecipe = () => {
  RECIPES.forEach(recipe => {
    let listOfItems = recipe.rules.map(
        ingredient => keyedIngredients.get(ingredient)
    ).join(" and ");
    console.log(`${recipe.name} - ${listOfItems}`);
  });
};

printRecipe();


Python functions to save JSON and H5 to local files and S3 I'm trying to copy files to S3 after writing them locally:
I have 2 functions which I need to do this. What is the best way to refactor the code that exists in both.

    s3 = False
    if data_fp.lower().startswith('s3://'):
        s3_data_fp = data_fp
        data_fp = os.path.join(_TMP, os.path.basename(data_fp))
        s3 = True

def save_json(data_fp, data, sort_keys=True, indent=4):
    s3 = False
    if data_fp.lower().startswith('s3://'):
        s3_data_fp = data_fp
        data_fp = os.path.join(_TMP, os.path.basename(data_fp))
        s3 = True

    with open(data_fp, 'w') as output_file:
        json.dump(data, output_file, cls=NumpyEncoder, sort_keys=sort_keys,
                  indent=indent)
    if s3:
        save_in_s3(data_fp, s3_data_fp)

# def save_hdf5(data_fp: str, data: Dict[str, object]):
def save_hdf5(data_fp, data, metadata=None):
    s3 = False
    if data_fp.lower().startswith('s3://'):
        s3_data_fp = data_fp
        data_fp = os.path.join(_TMP, os.path.basename(data_fp))
        s3 = True

    if metadata is None:
        metadata = {}
    mode = 'w'
    if os.path.isfile(data_fp):
        mode = 'r+'
    with h5py.File(data_fp, mode) as h5_file:
        for key, value in data.items():
            dataset = h5_file.create_dataset(key, data=value)

    if s3:
        save_in_s3(data_fp, s3_data_fp)

The S3-functionality wraps the specific code, which does not change the variables. So how about a decorator?

from functools import wraps

def handle_s3_upload(fcn):
    @wraps(fcn)
    def inner(data_fp, data, *args, **kwargs):
        # handle S3 path
        s3 = False
        if data_fp.lower().startswith('s3://'):
            s3_data_fp = data_fp
            data_fp = os.path.join(_TMP, os.path.basename(data_fp))
            s3 = True
        # call decorated function
        fcn(data_fp, data, *args, **kwargs)
        # S3 upload
        if s3:
            save_in_s3(data_fp, s3_data_fp)
    return inner

@handle_s3_upload
def save_json(data_fp, data, sort_keys=True, indent=4):
    with open(data_fp, 'w') as output_file:
        json.dump(data, output_file, cls=NumpyEncoder, sort_keys=sort_keys,
                  indent=indent)

@handle_s3_upload
def save_hdf5(data_fp, data, metadata=None):
    if metadata is None:
        metadata = {}
    mode = 'w'
    if os.path.isfile(data_fp):
        mode = 'r+'
    with h5py.File(data_fp, mode) as h5_file:
        for key, value in data.items():
            dataset = h5_file.create_dataset(key, data=value)


CSS Animation Card Flip I've made a card flip with CSS keyframes but instead of flipping clockwise and then flip counterclockwise to it's original position, I've two separate keyframes that will cause it to rotate in the same direction each time I rotate.

I have jQuery checking if either class exists and adding the first one if neither exists and then I toggle the classes to keep this effect going indefinitely.

Is there a better way of writing this CSS to condense this into one class and have JavaScript just toggle that one class instead of two? If not, is there a better approach to handling the JavaScript?

$(".click").click(function () {
    $flipper = $(".flipper");

    if (!$flipper.hasClass("flipped") && !$flipper.hasClass("second-flip")) {
        $flipper.addClass("flipped");
    } else {
        $flipper.toggleClass("flipped");
        $flipper.toggleClass("second-flip");
    }
});
.flip-container {
    height: 150px;
    width: 150px;

    perspective: 600;
    position: relative;
}
.flipper {
    height: 100%;
    width: 100%;

    position: absolute;
    transform-style: preserve-3d;
    transition: transform 1s;
}
.back, .front {
    height: 100%;
    width: 100%;

    display: block;
    position: absolute;
    backface-visibility: hidden;
}
.front {
    background-color: red;
}
.back {
    background-color: blue;
    transform: rotateY(180deg);
}
.flipped {
    animation: spin180 1s 1 forwards;
}
.second-flip {
    animation: spin360 1s 1 forwards;
}

@keyframes spin180 {
    0% { transform: rotateY(0deg); }
    100% { transform: rotateY(180deg); }
}

@keyframes spin360 {
    0% { transform: rotateY(180deg); }
    100% { transform: rotateY(360deg); }
}

Front

Back

<button class="click">Click me</button>

jQuery's .toggleClass() function allows you to determine the class name to be toggled by passing a function as the first argument:

$(".click").click(function() {
  $(".flipper").toggleClass(function() {
    if ($(this).is(".flipped")) {
      return "second-flip";
    } else {
      return "flipped";
    }
  });
});

Making your Javascript code cleaner and more usable. :)

$(".click").click(function() {
  $(".flipper").toggleClass(function() {
    if ($(this).is(".flipped")) {
      return "second-flip";
    } else {
      return "flipped";
    }
  });
});
.flip-container {
  height: 150px;
  width: 150px;
  perspective: 600;
  position: relative;
}
.flipper {
  height: 100%;
  width: 100%;
  position: absolute;
  transform-style: preserve-3d;
  transition: transform 1s;
}
.back,
.front {
  height: 100%;
  width: 100%;
  display: block;
  position: absolute;
  backface-visibility: hidden;
}
.front {
  background-color: red;
}
.back {
  background-color: blue;
  transform: rotateY(180deg);
}
.flipped {
  animation: spin180 1s 1 forwards;
}
.second-flip {
  animation: spin360 1s 1 forwards;
}
@keyframes spin180 {
  from {
    transform: rotateY(0deg);
  }
  to {
    transform: rotateY(180deg);
  }
}
@keyframes spin360 {
  from {
    transform: rotateY(180deg);
  }
  to {
    transform: rotateY(360deg);
  }
}

Front

Back

<button class="click">Click me</button>


Spliting PDF and get thumbnail while adding it to a ListView I'm doing a application to work with PDF and TIFF files.
To divide them, the user opens the file or folder (if it is a folder, it gets all the files inside) and the app splits the file, getting a tiny image (thumbnail) in JPG format. Then it creates a ListViewItem adding the thumbnail, the name and the size of the file.

But it takes too long (1:40min) with a PDF with 68 pages, so if a larger PDF is opened, it will take too long.

So I wish someone could say where can I improve my code so it doesn't take that long.

CODE

public void SplitPDF2(string input)
{
    try
    {
        iTextSharp.text.pdf.PdfReader reader = new iTextSharp.text.pdf.PdfReader(input);
        iTextSharp.text.pdf.PdfReader.unethicalreading = true;
        string name = Path.GetFileNameWithoutExtension(input);
        for (int i = 1; i <= reader.NumberOfPages; i++)
        {
            using (MemoryStream ms = new MemoryStream())
            {
                //get thumbnail from 1st page-----------------------------------------------------
                using (var document = PdfiumViewer.PdfDocument.Load(input))
                {
                    var image = document.Render(i - 1, 60, 84, 300, 300, true);
                    image.Save(ms, ImageFormat.Jpeg);
                    thumbnails.Images.Add(j.ToString(), image);
                    image.Dispose();
                }
                //split pdf-----------------------------------------------------------------------
                string p = AppDomain.CurrentDomain.BaseDirectory + "temporario\\" + name + " - Pagina " + i + ".PDF";
                using (Stream outputStream = new FileStream(p, FileMode.Create))
                {
                    Document doc = new Document();
                    PdfWriter pdfWriter = PdfWriter.GetInstance(doc, outputStream);
                    doc.Open();
                    PdfContentByte pdfContentByte = pdfWriter.DirectContent;
                    PdfImportedPage importedpage = pdfWriter.GetImportedPage(reader, i);
                    iTextSharp.text.Rectangle mediabox = reader.GetPageSize(i);

                    doc.SetPageSize(mediabox);
                    doc.NewPage();
                    pdfContentByte.AddTemplate(importedpage, 0, 0);

                    doc.Close();
                    doc.Dispose();
                    pdfWriter.Dispose();
                }
                //in the end it will add the thumbnail, the name and the size of the file to the ListView
                FileInfo f = new FileInfo(p);
                ListViewItem _item1 = new ListViewItem();
                _item1.ImageKey = j.ToString(); //image
                _item1.SubItems.Add(name + " - Pagina " + i + ".PDF"); //name
                _item1.SubItems.Add(f.Length.ToString()); //size

                lista2.Items.Add(_item1);
                j++;
            }
        }
        reader.Dispose();
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.ToString(), "Erro", MessageBoxButtons.OK);
    }
}

This code leaves a bunch of open questions, minor ones:
- SplitPDF2(string input) what has been/happened to SplitPDF() or SplitPDF1()?
- what is lista2?

Non-minor:

Document/comment your source code. In the code.
Preparing thumbnails and splitting a PDF into separate-file-per-page do neither sound nor look related:
Do not put/implement both in a single procedure
I can't seem to see a purpose in saving each thumbnail rendered into a MemoryStream ms that doesn't get used otherwise.
You instantiate one and only one iTextSharp.text.pdf.PdfReader(input) for each input:
Do the same for PdfiumViewer.PdfDocument
it will add the thumbnail I don't see that.

Overall, I'm left mistrusting writing file-per-page PDF to be a useful step in accomplishing the larger task at hand.
wish someone could say where [to improve the] code so it doesn't take that long my crystal ball is dull, but profiling/timeit() might help.


Simple Caesar Cipher in Java I am a recent beginner in Java and have attempted a simple Caesar cipher (using only lowercase):

public class RunEncryption
{
    public static void main(String[] args)
    {
        SecretMessage test1 = new SecretMessage();
        test1.setMessage("hello");
        test1.display();
        test1.encrypt(1);
    }

}

This is my class with the main method in it:

public class SecretMessage
{
    String message;

    void setMessage(String newMessage) {
        message=newMessage;
    }

    void display() {
        System.out.println(message);
    }

    void encrypt(int key) {
        int len = message.length();
        for(int i=0; i<len; i++) {
            char c = message.charAt(i);
            c = (char)(c+key);
            System.out.print(c);
        }
    }

}

This works thankfully. I will get to wrapping the letters around. But I want to ask if there is anything maybe unconventional about this or if it is inefficient code. Any help would be appreciated as I am still a beginner. Also, how long does it take to make this code? It took me far too long.

IMHO separating computation and printing is the most important improvement to do.

A code which can't do the computation separately is just a throw-away junk; you can't ever use it for anything else.
A code which can't do the printing separately is useless for the same reason. You can't use it for anything else, not even for the decryption.

In s simple snippet it doesn't matter much, but learning it later with complicated algorithms is much harder.

Note an asymmetry: Your SecretMessage stores the plaintext, but not the ciphertext. Ask yourself, if it needs to store anything.

Think about naming. Is the SecretMessage really a message? Or is it maybe an Encryptor? The way you implemented it, your naming isn't bad, but when you change it, you should reconsider it.

Does encrypt really encrypt its argument? Or would you expect it to do something else? Maybe being more verbose with encryptWithKey(char key) makes more sense.

Efficiency is usually overrated. You must think about using efficient algorithms and data structures for the job, but you can ignore nearly everything else. Anyway, before you start optimizing, make sure you optimize the real bottleneck, i.e., profile you code.

Anyway, using System.out.print for single characters may slow you down as it's synchronized and it flushes the output.

First building the result (using StringBuilder) would be faster. Moreover, it's a step in the right direction (Separation of concerns).


Mocking an F# primality tester I would like to ask 2 questions about the modularization of an F# file. The code is just a working example of prime factorization, but that's not the main topic. I have a main file (PrimeFactors.fs), a different file for its mocked version (PrimeFactorsMock.fs), and a test file (PrimeFactorsTests.fs):

PrimeFactors.fs

namespace PrimeFactors

module PrimeFactors =

    type IPrimeChecker =
        abstract member IsPrime : int64 -> int64 -> bool

    type PrimeFactors() =
        interface IPrimeChecker with
            member this.IsPrime number i =
                number % i = 0L

        member this.CheckPrimes number i acc =
            match i with
                | _ when i > (number |> float |> sqrt |> int64)   -> number::acc
                | _ when (this :> IPrimeChecker).IsPrime number i -> this.CheckPrimes (number / i) i (i::acc)
                | _                                               -> this.CheckPrimes number (i + 1L) acc

        member this.Of number =
            if number = 1L then
                [||]
            else
                this.CheckPrimes number 2L [] |> List.toArray

PrimeFactorsMock.fs

namespace PrimeFactors
open PrimeFactors

module PrimeFactorsMock =
    type PrimeFactorsMock() =
        inherit PrimeFactors()

        let mutable stepCount = 0L

        interface IPrimeChecker with
          member this.IsPrime number i =
            stepCount <- stepCount + 1L
            number % i = 0L

        member this.StepCount number =
            stepCount <- 0L
            this.Of number |> ignore
            stepCount

PrimeFactorsTests.fs

namespace PrimeFactors

open PrimeFactors
open NUnit.Framework
open FsUnit
open PrimeFactorsMock

[<TestFixture>]
module PrimeFactorTests =

    [<TestCase(1L, [||])>]
    [<TestCase(2L, [|2L|])>]
    [<TestCase(3L, [|3L|])>]
    [<TestCase(4L, [|2L; 2L|])>]
    [<TestCase(6L, [|3L; 2L|])>]
    [<TestCase(8L, [|2L; 2L; 2L|])>]
    [<TestCase(16L, [|2L; 2L; 2L; 2L|])>]
    [<TestCase(773L, [|773L|])>]
    [<TestCase(8L, [|2L; 2L; 2L|])>]
    let prime_factors_of_number number expect =
        PrimeFactors().Of number |> should equal expect

    [<TestCase(1L, 0L)>]
    [<TestCase(4L, 1L)>]
    [<TestCase(773L, 26L)>]
    let step_count_of_number number stepCount =
        PrimeFactorsMock().StepCount number |> should equal stepCount

What I like in that solution it's mock version is separated rightly from the initial class. So I only have to redefine IPrimeChecker.IsPrime, not the this.CheckPrimes, and possible all the inner functions. So I made IsPrime, and only IsPrime, virtual. It's a poor man's mocking.

My question is about mocking with object expression if that could be achieved without creating the PrimeFactorsMock.fs, and less coding. If we redefine the whole PrimeFactorsMock in the test file, it doesn't count (at least make it smaller). The goal is to make the IsPrime function interchangeable in the test file without much hassle.

The other question is I would like to use active patterns for IsPrime. So I can use pattern matching with (|Prime|NonPrime|) in the this.CheckPrimes part. Restriction is that have to be same as interchangeable as the already implemented virtual IsPrime.

The first thing I notice is the somewhat misleading method name IsPrime. I would call it IsFactor, because it only checks if i is a factor of number - not if it is a prime factor.

Secondly I think it is a problem that PrimeFactorsMock.IsPrime has its own implementation of number % i = 0L because you then don't evaluate the base class implementation which could be invalid.

A solution to that problem could be as shown below. Be aware that I have dropped the IPrimeChecker, because it is unnecessary.

module PrimeFactors =

    type PrimeFactors() =

        // I've renamed IsPrime to IsFactor because it is only checking if i is a factor of number not if it is a prime factor
        // You have to tuple the arguments to make it possible to be called from a derived class by base.IsFactor
        abstract member IsFactor: uint64 * uint64 -> bool
        default this.IsFactor (number, i) = number % i = 0UL

        member this.CheckPrimes number i acc =
            match i with
                | _ when i > (number |> float |> sqrt |> uint64)   -> number::acc
                | _ when this.IsFactor (number, i) -> this.CheckPrimes (number / i) i (i::acc)
                | _  -> this.CheckPrimes number (i + 1UL) acc

        member this.Of number =
            if number = 1UL then
                [||]
            else
                this.CheckPrimes number 2UL [] |> List.toArray

module PrimeFactorsMock =
    open PrimeFactors

    type PrimeFactorsMock() =
        inherit PrimeFactors()

        let mutable stepCount = 0L

        override this.IsFactor (number, i) =
            stepCount <- stepCount + 1L
            base.IsFactor (number, i) // Her the base class implementation is called

        member this.StepCount number =
            stepCount <- 0L
            this.Of number |> ignore
            stepCount

When it comes to Object Expressions as a mock for testing, I haven't found a valid approach in this case, because you can't extend an OE beyond the type you "derive" from. For instance the below straight forward solution is not an option:

let mock = {
    new PrimeFactors.PrimeFactors()
    with
        member this.IsFactor (number, i) =
            stepCount <- stepCount + 1L
            base.IsFactor (number, i)

    // This is not allowed
    let mutable stepCount = 0L
    member this.StepCount number =
        stepCount <- 0L
        this.Of number |> ignore
        stepCount

}

According to Active Patterns for Prime|NoPrime (or is it Factor?) I would use a union type instead defined as follows:

type FactorType =
    | Factors of uint64 * uint64
    | NonFactor

- used in a way like this:

let getFactorType a b =
            match a % b with
            | r when r = 0UL -> Factors(b, a / b)
            | _ -> NonFactor

- and matched like this:

match getFactorType num i with
| Factors(a, b) -> getPrimes b a (a::acc)
| NonFactor -> getPrimes num (i + 1UL) acc

An entire solution that uses a discriminated union type for factor/nonfactor and minimize the Mock derive could be as follows - not claiming it to be best practice or anything like that.

module PF =

    // Instead of an active pattern, I use this discriminated union type
    type FactorType =
        | Factors of uint64 * uint64
        | NonFactor

    // Function prototype for functions testing if a number is a factor of another
    // I've changed the domain type from int64 to uint64
    type FactorChecker = uint64 * uint64 -> FactorType

    type PrimeFactors() =

        // This function is made static because it has nothing to do with the class itself
        // and because I want to reuse it in the derived Mock class (se below)
        // I renamed it to getFactorType from IsPrime,
        // because IsPrime is somewhat misleading
        // as it only checks if b is a factor of a not if it is a prime factor.
        static member getFactorType (a, b) =
            match a % b with
            | r when r = 0UL -> Factors(b, a / b)
            | _ -> NonFactor

        // This could be static as well but here I made it a member.
        // It would be nice to declare it protected, but that's not an option in F#
        // In order to let the client provide the function to check for factors, the last argument is a function of type FactorChecker
        member this.getFactors number (factorChecker: FactorChecker) =
            if number < 2UL then
                []
            else
                // The stop condition is the (uint64 sqrt) of n
                let max n = n |> float |> sqrt |> uint64

                let rec getPrimes num i acc =
                    match i with
                    | _ when i > max num -> num::acc
                    | _ ->
                        match factorChecker (num, i) with
                        | Factors(a, b) -> getPrimes b a (a::acc)
                        | NonFactor -> getPrimes num (i + 1UL) acc

                getPrimes number 2UL []

        member this.Of (number: uint64) : uint64 list = this.getFactors number PrimeFactors.getFactorType

module PFMocks =
    open PF

    // This is a simplified version of PrimeFactorsMock without the IsPrime override
    type PrimeFactorsMock() =
        inherit PrimeFactors()

        member this.StepCount number =
            let mutable stepCount = 0L
            let facChecker (a, b) =
                stepCount <- stepCount + 1L
                PrimeFactors.getFactorType (a, b)
            this.getFactors number facChecker |> ignore
            stepCount


Solidworks save as PDF macro This just opens all drawing files in solidworks and saves as a *.pdf file. I just wanted to to get a second set of eyes on this before I let it go on the actual drawings database (i.e. all our files) and hopefully make sure it won't encounter any errors.

using SolidWorks.Interop.sldworks;
using SolidWorks.Interop.swconst;
using System.Runtime.InteropServices;
using System;
using System.IO;

namespace PDF.csproj
{
    public partial class SolidWorksMacro
    {

        public void Main()
        {
            string pdfPath;
            string Path = "C:\\Users\\SAFEROADS201\\Desktop\\01-VMS Trailer";
            string txtPath = Path + "\\UpdatedPDFs.txt";
            string Date = DateTime.Now.ToShortDateString();
            string[] drwPaths = System.IO.Directory.GetFiles(Path, "*.slddrw", SearchOption.AllDirectories);
            foreach (string s in drwPaths)
            {
                if (!s.Contains("~$"))
                {
                    ModelDoc2 swDoc = null;
                    int longstatus = 0;
                    int longwarnings = 0;
                    pdfPath = s;
                    pdfPath = pdfPath.Replace("Drawings", "PDFs").Replace("SLDDRW", "PDF");
                    (new FileInfo(pdfPath)).Directory.Create();
                    swDoc = ((ModelDoc2)(swApp.OpenDoc6(s, 3, 0, "", ref longstatus, ref longwarnings)));
                    longstatus = swDoc.SaveAs3(pdfPath, 0, 0);
                    swDoc = null;
                    swApp.CloseAllDocuments(true);
                }
            }
            System.IO.File.WriteAllText(txtPath, Date);
         }

        /// <summary>
        ///  The SldWorks swApp variable is pre-assigned for you.
        /// </summary>
        public SldWorks swApp;
    }
}

Naming

Also the naming guidlines doesn't explictly mention variable local to a method, you should name them using camelCase casing.
Naming methods, properties and variables in a meaningful way will help you or Mr.Maintainer to faster grasp what a variable/method/class is about.
Shortening of names like foreach (string s in drwPaths) should be avoided.

General

If the right side type of an assignment is obvious one should use var like

var path = "C:\\Users\\SAFEROADS201\\Desktop\\01-VMS Trailer";

By using guard conditions you can save horizontal spacing which improves readability of the code.
By extracting the algorithm to a method in a separate class you can reuse it.
Filepaths should be composed using the System.IO.Path.Combine() method.
If you are dealing with a large amount of files you should prefer Directory.EnumerateFiles() over Directory.GetFiles().
From the documentation of GetFiles():

  The EnumerateFiles and GetFiles methods differ as follows: When you use EnumerateFiles, you can start enumerating the collection of names before the whole collection is returned; when you use GetFiles, you must wait for the whole array of names to be returned before you can access the array. Therefore, when you are working with many files and directories, EnumerateFiles can be more efficient.

variables should be declared as near as possible to their usage.
strings, especially strings which represents filepaths should be marked as verbatim string literal to ignore escape sequences. See this SO answer

Refactoring

Let us start by introducing a new class called SolidWorkFileConverter, add a constructor and add a method named ToPdf()

public class SolidWorkFileConverter
{
    private SldWorks swApplication;
    public SolidWorkFileConverter(SldWorks swApp)
    {
        swApplication = swApp;
    }

    public void ToPdf(string searchPath, string logFileName)
    {
        var logFileDestionation = System.IO.Path.Combine(searchPath, logFileName);
        var currentDate = DateTime.Now;
        var drawingFiles = System.IO.Directory.EnumerateFiles(Path, "*.slddrw", SearchOption.AllDirectories)
                                              .Where(file => !file.Contains("~$"));
        foreach (var drawingFile in drawingFiles)
        {
            ToPdf(drawingFile);
        }
        WriteToLog(logFileDestination, currentDate);
    }

    public void ToPdf(string drawingFile)
    {
        string destinationFileName = ComposeDestinationFileName(drawingFile);
        CreateDestinationDirectory(destinationFileName);
        int state = 0;
        int warningState = 0;
        ModelDoc2 swDoc = (ModelDoc2)(swApplication.OpenDoc6(s, 3, 0, "", ref state, ref warningState));
        state = swDoc.SaveAs3(pdfPath, 0, 0);
        swDoc = null;
        swApplication.CloseAllDocuments(true);
    }

    private string ComposeDestinationFileName(string sourceFileName)
    {
        return sourceFileName.Replace("Drawings", "PDFs").Replace("SLDDRW", "PDF");
    }

    private void CreateDestinationDirectory(string destinationFileName)
    {
        string directoryName = System.IO.Path.GetDirectoryName(destinationFileName);
        System.IO.Directory.CreateDirectory(directoryName);
    }

    private void WriteToLog(string logFileDestination, DateTime currentDate)
    {
        System.IO.File.WriteAllText(logFileDestination, currentDate.ToShortDateString());
    }
}

this can now be called by your main() method (or used anywhere) like

string searchPath = @"C:\Users\SAFEROADS201\Desktop\01-VMS Trailer";
string logFileName = System.IO.Path.Combine(searchPath , "UpdatedPDFs.txt");
SolidWorkFileConverter converter = new SolidWorkFileConverter(swApp);
converter.ToPdf(searchPath, logFileName);


Solving the Reve's puzzle I'm trying to solve the Reve's puzzle (a variant of the Tower of Hanoi puzzle with four pegs). This is what I've come up with so far:

def solve(disk, source, temppeg1, temppeg2, destination):
    if disk == 1:
        print((source, destination))
    elif disk == 2:
        print((source, temppeg1))
        print((source, destination))
        print((temppeg1, destination))
    else:
        solve(disk - 2, source, temppeg2, destination, temppeg1)
        print((source, temppeg2))
        print((source, destination))
        print((temppeg2, destination))
        solve(disk - 2,  temppeg1, source, temppeg2, destination)

>>> solve(16, 'a', 'b', 'c', 'd')
('a', 'c')
... 763 lines deleted ...
('a', 'd')

While this works fine, I was wondering if there's a better solution (more optimal) than what I have so far. Currently with what I have, I can solve 16 disk in 765 moves.

if you are just trying to find the minimum number of moves and not necessarily a solution you can use the Frame–Stewart algorithm that you linked to earlier

this builds up a solution to the number of moves to achieve a solution.

def FrameStewart(ndisks,npegs):
    if ndisks ==0: #zero disks require zero moves
        return 0
    if  ndisks == 1 and npegs > 1: #if there is only 1 disk it will only take one move
        return 1
    if npegs == 3:#3 pegs is well defined optimal solution of 2^n-1
        return 2**ndisks - 1
    if npegs >= 3 and ndisks > 0:
        potential_solutions = (2*FrameStewart(kdisks,npegs) + FrameStewart(ndisks-kdisks,npegs-1) for kdisks in range(1,ndisks))
        return min(potential_solutions) #the best solution
    #all other cases where there is no solution (namely one peg, or 2 pegs and more than 1 disk)
    return float("inf")

print FrameStewart(16,4) #prints 161

this tells us that the optimal solution for 16 disks and 4 pegs is 161 moves, note that it does not tell us what those moves are

if you actually need the moves you will have to heavily modify this solution.

start by solving the towersofhanoi with 3 pegs as that is the traditional layout and has well defined algorithms to solve

def towers3(ndisks,start=1,target=3,peg_set=set([1,2,3])):
   if ndisks == 0 or start == target: #if there are no disks, or no move to make
      return [] #no moves
   my_move = "move(%s,%s)"%(start,target)
   if ndisks == 1: #trivial case if there is only one disk, just move it
      return [my_move]
   helper_peg = peg_set.difference([start,target]).pop()
   moves_to_my_move = towers3(ndisks-1,start,helper_peg)
   moves_after_my_move = towers3(ndisks-1,helper_peg,target)
   return moves_to_my_move + [my_move] + moves_after_my_move

you can easily verify that this is returning optimal solutions by knowing that the optimal solution to the towers of hanoi with 3 pegs is 2ndisks - 1

all thats left is to change FrameStewart to also return moves

def FrameStewartSolution(ndisks,start=1,end=4,pegs=set([1,2,3,4])):
    if ndisks ==0 or start == end: #zero disks require zero moves
        return []
    if  ndisks == 1 and len(pegs) > 1: #if there is only 1 disk it will only take one move
        return ["move(%s,%s)"%(start,end)]
    if len(pegs) == 3:#3 pegs is well defined optimal solution of 2^n-1
        return towers3(ndisks,start,end,pegs)
    if len(pegs) >= 3 and ndisks > 0:
        best_solution = float("inf")
        best_score = float("inf")
        for kdisks in range(1,ndisks):
            helper_pegs = list(pegs.difference([start,end]))
            LHSMoves = FrameStewartSolution(kdisks,start,helper_pegs[0],pegs)
            pegs_for_my_moves = pegs.difference([helper_pegs[0]]) # cant use the peg our LHS stack is sitting on
            MyMoves = FrameStewartSolution(ndisks-kdisks,start,end,pegs_for_my_moves) #misleading variable name but meh
            RHSMoves = FrameStewartSolution(kdisks,helper_pegs[0],end,pegs)#move the intermediat stack to
            if any(move is None for move in [LHSMoves,MyMoves,RHSMoves]):continue #bad path :(
            move_list = LHSMoves + MyMoves + RHSMoves
            if(len(move_list) < best_score):
                best_solution = move_list
                best_score = len(move_list)
        if best_score < float("inf"):
            return best_solution
    #all other cases where there is no solution (namely one peg, or 2 pegs and more than 1 disk)
    return None

note that this is going to be much slower than the version that does not need to find the actual solution (this being codereview maybe some folks have suggestions to make it run faster)
Timings from this experiment

towers3(16)  # 0.09 secs
FrameStewart(16) #0.04 secs
FrameStewartSolution(16) #67.04 secs!!!

really slow as you can see

you can speed it up alot by memoizing it

import json

def fsMemoizer(f): #just a junky quick memoizer
    cx = {}
    def f2(*args):
        try:
            key= json.dumps(args)
        except:
            key =json.dumps(args[:-1] + (sorted(list(args[-1])),))
        if key not in cx:
            cx[key] = f(*args)
        return cx.get(key)
    return f2
@fsMemoizer
def FrameStewartSolution(ndisks,start=1,end=4,pegs=set([1,2,3,4])):
    ...

after memoization the time to calculate became much faster (less than a second)


How to reduce execution time for this clustering computation? Is there any way to reduce total execution time for the function getSilhouetteIndex? P.S. I am using weka SimpleKMeans for getting kmeans and ceval.

private double getSilhouetteIndex(List<ITSPoI> _POIs, SimpleKMeans kmeans, ClusterEvaluation ceval)
{
    double si_index = 0;
    double[] ca = ceval.getClusterAssignments();
    double[] d_arr = new double[ca.length];
    List<Double> si_indexes = new ArrayList<Double>();

    for (int i=0; i<ca.length; i++)
    {
        // STEP 1. Compute the average distance between the i-th PoI and all other points of a given cluster
        double a = averageDist(_POIs,ca,i,1);

        // STEP 2. Compute the average distance between the i-th PoI and all PoIs of other clusters
        for (int j=0; j<ca.length; j++)
        {
            double d = averageDist(_POIs,ca,j,2);
            d_arr[j] = d;
        }

        // STEP 3. Compute the the distance from the i-th PoI to its nearest cluster to which it does not belong
        double b = d_arr[0];
        for (Double _d : d_arr)
        {
            if (_d < b)
                b = _d;
        }

        // STEP 4. Compute the Silhouette index for the i-th PoI
        double si = (b - a)/Math.max(a,b);
        si_indexes.add(si);
    }

    // STEP 5. Compute the average index over all observations
    double sum = 0;
    for(Double _si : si_indexes)
    {
         sum += _si;
    }
    si_index = sum/si_indexes.size();

    return si_index;
}

private double averageDist(List<ITSPoI> _POIs, double[] ca, int id, int calc)
{
    double avgDist = 0;
    List<ITSPoI> clusterPOIs = new ArrayList<ITSPoI>();

    // Distances inside the cluster
    if (calc == 1)
    {
        for (int i = 0; i<ca.length; i++)
        {
            if (ca[i] == ca[id])
                clusterPOIs.add(_POIs.get(i));
        }
    }
    // Distances outside the cluster
    else
    {
        for (int i = 0; i<ca.length; i++)
        {
            if (ca[i] != ca[id])
                clusterPOIs.add(_POIs.get(i));
        }
    }

    double latx, lonx, laty, lony;
    double[] dist = new double[clusterPOIs.size()];
    latx = _POIs.get(id).getLat();
    lonx = _POIs.get(id).getLon();
    for (int i=0; i<clusterPOIs.size(); i++)
    {
        laty = clusterPOIs.get(i).getLat();
        lony = clusterPOIs.get(i).getLon();
        dist[i] = distanceGeo(latx,lonx,laty,lony);
    }

    double sum = 0;
    for(Double d : dist)
    {
         sum += d;
    }
    avgDist = sum/dist.length;

    return avgDist;
}

private double distanceGeo(double lat1, double lon1, double lat2, double lon2)
{
    if (lat1 == lat2 && lon1 == lon2)
    {
        return 0;
    }
    else
    {
        double theta = lon1 - lon2;
        double dist = Math.sin(deg2rad(lat1)) * Math.sin(deg2rad(lat2)) + Math.cos(deg2rad(lat1)) * Math.cos(deg2rad(lat2)) * Math.cos(deg2rad(theta));
        dist = Math.acos(dist);
        dist = rad2deg(dist);
        dist = dist * 60 * 1.1515;
        dist = dist * 1.609344;
        return dist;
    }
}

There are three methods for computing a great circle distance between two points that are specified by latitude and longitude:

You used the spherical law of cosines, which is the most straightforward method based on mathematical principles.
The haversine formula yields better accuracy for small distances, though at a greater computational cost.
An approximation can be obtained by using the Pythagorean theorem on an equirectangular projection.

For the purposes of clustering, an approximation of the great circle distance might be good enough.  I would also declare this function private static final as a very strong hint that it can be inlined — as it should, since it's a pure mathematical function.

private static final double approxDistanceGeo(double lat1, double lon1, double lat2, double lon2)
{
    if (lat1 == lat2 && lon1 == lon2)
    {
        return 0;
    }
    else
    {
        double x = deg2rad(lon1 - lon2) * Math.cos(deg2rad((lat1 + lat2) / 2));
        double y = deg2rad(lat1 - lat2);
        double dist = Math.sqrt(x * x + y * y);
        dist = dist * 60 * 1.1515 * 1.609344;
        return dist;
    }
}

Working directly in radians could save a few deg2rad() conversions.


Implementing linked list from scratch - Follow up A follow up for this question:
Implementing Linked lists in Python from scratch

Node:

class Node():
    def __init__(self, key):
        self.key = key
        self.next = None

Linked list:

from node import Node

class LinkedList:
    link_connection = ' -> '

    ''' init, getters, setters '''
    def __init__(self, key):
        self.head = Node(key)
        self.tail = self.head
        self.length = 1

    @property
    def head(self):
        return self.__head

    @head.setter
    def head(self, node):
        if node is not None:
            self.__head = node

    @property
    def tail(self):
        return self.__tail

    @tail.setter
    def tail(self, node):
        if node is not None:
            try:
                if node.next is None:
                    self.__tail.next = node
                    self.__tail = self.__tail.next
            except AttributeError:
                self.__tail = node

    @property
    def length(self):
        return self.__length

    @length.setter
    def length(self, value):
        self.__length = value

    ''' generator for nodes '''
    ''' iterating, search etc. '''
    def node_generator(self, target=None):
        node = self.head
        yield node
        while node.next is not None and node.next.key != target:
            node = node.next
            yield node

    def __iter__(self):
        return map(lambda node: node.key, self.node_generator())

    def __str__(self):
        return self.link_connection.join(str(key) for key in self)

    def go_forward(self, steps):
        for node in self.node_generator():
            if steps == 0:
                return node
            steps -= 1

    def delete(self, target):
        for node in self.node_generator(target):
            pass
        node.next = node.next.next
        self.length -= 1

    def __len__(self):
        return self.__length

    def append(self, key):
        node = Node(key)
        self.tail = node
        self.length += 1

    def find(self, value):
        for node in self:
            if node == value:
                return node

Is this new implementation better?

Specification and Unit Tests

You are lacking a clear specification for your methods. We have to guess what these methods do exactly, specially for edge cases.
You should provide unit tests when providing an API class. You would have found some bugs if you did.

Review

 @head.setter
    def head(self, node):
        if node is not None:
            self.__head = node

There is no link from the new head to the previous one. Also, what would you do if node.next is already filled?

 @head.setter
    def head(self, node):
        if node is not None:
            node.next = self.__head
            self.__head = node

@tail.setter
    def tail(self, node):
        if node is not None:
            try:
                if node.next is None:
                    self.__tail.next = node
                    self.__tail = self.__tail.next
            except AttributeError:
                self.__tail = node

Why would you expect an AttributeError here? You are only setting next. And why would you ignore a tail that has a next node? Why not include the tail chain?

''' generator for nodes '''
''' iterating, search etc. '''
def node_generator(self, target=None):
    node = self.head
    yield node
    while node.next is not None and node.next.key != target:
        node = node.next
        yield node

The spec is unclear here. You always include the head, and then continue as long as a next node's key does not match the specified target. Why not check head for the same condition?

def delete(self, target):
    for node in self.node_generator(target):
        pass
    node.next = node.next.next
    self.length -= 1

What if the node you want to delete is the tail? This edge case is not foreseen.


Building a table from the result of an AJAX call I am looping through content coming from the database using an ajax call. Then I'm building a table using a for loop. Is there a faster way to build the table using the data from the database or is this way efficient enough?

var getPages = '

Page Num	Type	Month	Name	Reg No.	Rrc District

';

for(var i=0; i<getPagesResponse.length; i++) {
    getPages += '
' + getPagesResponse["PageNum"][i] + '	' + getPagesResponse["PageType"][i] + '	' + getPagesResponse["ReportingMonth"][i] + '	' + getPagesResponse["FilerName"][i] + '	' + getPagesResponse["FilerRegNo"][i] + '	' + getPagesResponse["RrcDistrict"][i] + '
';
}
getPages += '

';
//add table to div
$("#pagesTable").html(getPages);

i think it is good approach, if data return from database is in small size (i.e limited record say 10,20 etc).


Batch rename files with Mac Bash,while the dir index should be recorded I need to batch rename images with Mac Bash,while the directory index should be recorded.

To rename files ,turn

Root---A----0.png
|      |
|      ----1.png
|
-------B----0.png
       |
       ----1.png

to

Root---A----0_0.png
|      |
|      ----0_1.png
|
-------B----1_0.png
       |
       ----1_1.png

Here is my code, it is ok to deal with the situation that dir A has a name of spacing.

Base=$(pwd)
num=0
IFS='
' # split on newline only
for path in *
    do
    if [[ -d $path ]]; then
        NewPath="${Base}/${path}"
        for f in "$NewPath"/*
            do
            dir=`dirname "$f"`
            base=`basename "$f"`
            name="${dir}/${num}_${base}"
            mv ${f} "$name"
        done
        num=$((num + 1))
    fi
done

Any way to do it more brilliantly? find is a good option to handle files recursively.

Limit globbing to directories only by appending a slash:  for dir in */

If you cd into subdirectories, you don't need to construct a new path for each file.  If you cd inside a subshell with ( cd … ), the original directory will be restored when subshell exits.  Make sure to increment n outside of the subshell, or the new value will be lost!

The IFS= is not needed; bash will split the filenames properly.  You just need to quote the variable when you refer to it.

n=0
for d in */ ; do
    ( cd "$d" && for f in * ; do mv  "$f" $n"_$f" ; done )
    (( n++ ))
done


Evaluating given numbers and operations The program reads in an operation, and then numbers, and does stuff to them based on that.

Exercise number 1 at this linl

For example, ./this_file sum 1 2 3 would yield 6

I'm fairly certain I don't need all the #includes, but I had added them at some point to deal with a problem, some ideas I kept others I didn't.

I've also started reading Accelerated C++ and I realize that I should have probably tried to use things like Array<> if it's to really be C++.

/*
 * Write a program that takes as its first argument one of the words ‘sum,’ ‘product,’ ‘mean,’ or ‘sqrt’  and for further arguments a series of numbers.
 * The program applies the appropriate function to  the series.
 */

#include <stdio.h>
#include <math.h>
#include <stdlib.h>
#include <iostream>
#include <string.h>

using namespace std;

// Prototypes
double sum(int argc,const char* argv[]);
double product(int argc, const char* argv[]);
double mean(int argc, const char* argv[]);

int main(int argc, const char* argv[])
{
    double return_value;
    if(argc == 3)
    {
        //This means we're in sqrt, as we don't support sqrt of > 1 number
        //Nor sum, product, etc of less than one. 'cause that's dumb

        double operand;
        operand = strtod(argv[2], NULL );
        return_value = sqrt(operand);

    }
    else
    {
        if(strcmp(argv[1],"sum") == 0)
        {
            return_value = sum(argc, argv);
        }
        if(strcmp(argv[1],"product") == 0)
        {
            return_value = product(argc, argv);
        }
        if(strcmp(argv[1],"mean") == 0)
        {
            return_value = mean(argc, argv);
        }
    }

    cout << return_value << endl;
}

double sum(int argc,const char* argv[])
{

    double return_value = 0.0;
    for(int i = 2; i < argc; i++)
    {
        double x = strtod(argv[i], NULL);
        return_value += x;
    }
    return return_value;
}

double product(int argc, const char* argv[])
{

    double return_value = 1;
    for(int i = 2; i < argc; i++)
    {
        return_value *= strtod(argv[i],NULL);
    }
    return return_value;
}

double mean(int argc,const char* argv[])
{
    double summation = sum(argc, argv);
    return summation/((double) argc-2);
}

You're using C headers (which end in .h), not the C++ ones.  This will also ensure that they're put into the std namespace.  Also, you can remove <string.h> since it's not currently in use.
All of your operations rely on command line arguments, yet the program is still allowed to continue even if there aren't any (excluding the filename).  You can fix this by terminating the program right away if argc is too low.

if (argc == 1)
{
    // print this to the standard error stream
    std::cerr << "Additional arguments are required!";
    // same as return 1, also used for failure
    return EXIT_FAILURE;
}

With this in place, you should also redo the other argc checks.  Add this given check first, then have the others.  I'd also consider replacing else with an else if that includes a specific number of command line arguments.  With the else there, that code block will be executed with any other value not already given in an if.
Consider changing the parameter names of the functions for clarity.  Even if you're trying to pass the command line arguments to them, the functions themselves aren't very self-documenting.

You could also assign the various argv[] values to separate variables in main() and passing them to the other functions.  This again helps with self-documentation.
There's no need to declare and then initialize:

double operand;
operand = strtod(argv[2], NULL );

If you just need to initialize, then do that only:

double operand = strtod(argv[2], NULL );

The contents of mean():

double summation = sum(argc, argv);
return summation/((double) argc-2);

can just be a single line:

return sum(argc, argv) / ((double) argc-2);

The sum() function still reveals the intent, so it can be put together.