Systematically analyzing dice roll outcomes I created a generator function to iterate through all possible dice rolls, and then applied it the ability score generating methods in Pathfinder RPG. (Obviously, there are more efficient ways to approach probability outcome analysis from a number theoretic perspective, but this allows generic and quick analysis without too much mathematical understanding of the particular situation.)

My question is, using this iterator, is there any way to generically split the iterator into different numbers of dice? My current approach feels sloppy and is definitely not scalable to more varied numbers of dice. Obviously, I could just use separate dice iterators, but that would be a generally less efficient approach.

Also, as always, any other feedback is appreciated, even PEP 8 stylistic feedback.

math_supplement.py

from fractions import Fraction

def dice_outcomes(num, min=1, max=6):
    '''
    A generator function returning all outcomes of num dice as lists. Dice values are inclusively between min (default 1) and max (default 6).
    '''
    output = [min] * num
    try:
        while True:
            yield output
            i = 0
            while True:
                output[i] += 1
                if output[i] <= max:
                    break
                output[i] = min
                i += 1
    except IndexError:
        return

def sample_space_report(stats, size):
    '''
    Reports on stats relative to a sample space.

    stats
        a dict, with each key being an outcome and the
    size
        the sample space
    '''
    probsum = 0
    for outcome, occurences in stats.items():
        prob = Fraction(occurences, size)
        probsum += prob
        print(f'{outcome}: {prob} ({float(prob):.2%})')
    print(f'Validation sum = {probsum}')

Pathfinder_ability_score_probability.py

from collections import Counter

from math_supplement import dice_outcomes, sample_space_report

class AbilityScoreStats:
    def __init__(self, roll_handler, sample_space):
        self.roll_handler = roll_handler
        self.sample_space = sample_space
        self.outcomes = Counter()

    def add(self, l):
        self.outcomes[self.roll_handler(l)] += 1

MIN = 1
MAX = 6
DICE_FACES = MAX - MIN + 1
ability_score_methods = {'Standard':
                             AbilityScoreStats(lambda rolls: sum(rolls) - min(rolls),
                             DICE_FACES ** 4),
                         'Classic':
                             AbilityScoreStats(lambda rolls: sum(rolls),
                             DICE_FACES ** 3),
                         'Heroic':
                             AbilityScoreStats(lambda rolls: sum(rolls) + 6,
                             DICE_FACES ** 2)
                        }
for rolls in dice_outcomes(2, min=MIN, max=MAX):
    ability_score_methods['Heroic'].add(rolls)
    for dice_three in range(1,7):
        working_rolls = rolls + [dice_three]
        ability_score_methods['Classic'].add(working_rolls)
        for dice_four in range(1,7):
            working_rolls = rolls + [dice_three, dice_four]
            ability_score_methods['Standard'].add(working_rolls)
for name, method in ability_score_methods.items():
    print(f'    {name}:')
    sample_space_report(method.outcomes, method.sample_space)


`Maybe` handling in Matrix manipulation in Elm I'd like to learn if my rewrite is worse than the original, less clear or less "idiomatic".

I've found this code that handles updating two dimensional matrixes in Elm:

type alias Matrix a = Array.Array (Array.Array a)
type alias Location = (Int, Int)

get : Location -> Matrix a -> Maybe a
get location m =
  Array.get (row location) m `andThen` Array.get (col location)

set : Location -> a -> Matrix a -> Matrix a
set location value m =
  update location (always value) m

update : Location -> (a -> a) -> Matrix a -> Matrix a
update location f m =
  get location m
  |> Maybe.map (
    \current ->

      Array.get (row location) m
      |> Maybe.map (
          \oldRow ->
            Array.set (col location) (f current) oldRow
            |> (\newRow -> Array.set (row location) newRow m)
      )
      |> Maybe.withDefault m
  )
  |> Maybe.withDefault m

I couldn't understand update at first so I tried to write my own (using my own domain types):

type Player = PlayerOne | PlayerTwo
type Slot = Empty | PlayedBy Player
type alias Board = Array.Array (Array.Array (Slot))
type alias Position = (Int, Int)

get : Board -> Position -> Maybe Slot
get board (row, column) =
    Array.get row board `Maybe.andThen` Array.get column

setPosition : Board -> Position -> Player -> Board
setPosition board (row, column) player =
    case get board (row, column) of
        Nothing ->
            board
        Just Empty ->
            case Array.get row board of
                Nothing ->
                    board
                Just oldRow ->
                    let
                        newRow = Array.set column (PlayedBy player) oldRow
                    in
                        Array.set row newRow board
        Just s ->
            board

After rewriting I could understand what's going on in the original update function, but ended up feeling my version's clearer. This is my first day writing Elm so I'm no authority in Elm code clarity.

I like your code a lot more than the original! It is definitely cleaner and more obvious than the first example you gave. However, there still seems to be a process you’re repeating — get value from array, pattern match on it, update array — that you can still factor out. (The original code is guilty of this, too.)

I tried to rewrite your function in a matter that avoids this, and my code’s at the bottom; see if you find it more readable. But first, some stylistic advice on writing Elm:

Generally, instead of writing things like Array.Array, it's useful to expose the type name from the data structure package you're using. Also, type parameters don’t need parentheses around them (except for grouping).

So you can rewrite:

type alias Board = Array.Array (Array.Array (Slot))

to

import Array exposing (Array)

type alias Board = Array (Array Slot)

which I think is a bit more Elm-y.
A useful design guideline (for packages, but  it’s a good idea for code in general): the data structure is always the last argument. You’ve reordered them from your first example, but the original order was there for a good reason: if your types look like

f : (extra params) -> Container -> Container

you can partially apply your function and end up with:

f "extra" "arguments" : Container -> Container

which is very useful for function composition and piping: often you want to take a Container and pipe it through a bunch of Container -> Container functions, like so:

result =
    initialContainerValue
    |> f "hello" "world"
    |> someOtherStep
    |> f "oak" "fir"

The Elm style guide contains some more recommendations that could apply to your code, but they’re more subjective. The two big ones I could think of have to do with whitespace.

Always have 2 empty lines between top-level declarations.
The examples in the guide have single empty lines between case branches, too.

My version

{-| Apply a function at the given index in the array.

     applyAt 2 negate (fromList [50, 60, 70]) == fromList [50, 60, -70]
-}
applyAt : Int -> (a -> a) -> Array a -> Array a
applyAt i f array =
    case Array.get i array of
        Nothing ->
            array

        Just a ->
            Array.set i (f a) array

{-| Update a slot: the given player occupies it, but only if it's empty. -}
setSlot : Player -> Slot -> Slot
setSlot player slot =
    case slot of
        Empty ->
            PlayedBy player

        s ->
            s

{-| Update a slot somewhere in a board. -}
setPosition : Position -> Player -> Board -> Board
setPosition (row, column) player =
    applyAt row
        (applyAt column
            (setSlot player))


Code for showing installed python packages/modules Any comments about style (especially any "non-pythonic" code), performance, readability, etc would be much appreciated! I've tested this on CentOS and OSX 10.9. Works for Python 2.7 and 3.

#!/usr/bin/env python
#
# This script will run on Python 2.7.x and later
#
# By default, iter_modules looks in sys.path which is initialized from
# the environment variable PYTHONPATH, plus an installation-dependent default.
from pkgutil import iter_modules
import sys

def parse_command_line():
    import argparse # added to Python 2.7 to replace optparse
    python_version = sys.version
    parser = argparse.ArgumentParser(description='Use this script to view the installed packages and/or modules for Python version: '+python_version)
    parser.add_argument('--type',dest='list_type',type=str,default='packages',help='Can be \'packages\' (default), \'modules\', or \'both\'')
    parser.add_argument('--ncols',type=int,default=2,help='Number of columns in output (default: 2)')
    args = parser.parse_args()
    return args

def select_packages():
    return [ module[1:] for module in iter_modules() if module[2] ],'Packages'

def select_modules():
    return [ module[1:] for module in iter_modules() if not module[2] ],'Modules'

def select_both():
    return [ module[1:] for module in iter_modules() ],'Packages and Modules'

options = { 'packages' : select_packages,
            'modules' : select_modules,
            'both' : select_both,
}

def get_printed_names(pack_tuple):
    p_name = []
    for code in pack_tuple:
        if ( code[1] ):
            p_name.append(code[0]+' (Package)')
        else:
            p_name.append(code[0]+' (Module)')
    return p_name

def fmt_cols(my_list, cols):
    import math
    max_width = max(map(lambda x: len(x), my_list))
    col_length = int(math.ceil(len(my_list)/cols+1))
    justify_list = list(map(lambda x: x.ljust(max_width), my_list))
    lines = [ ' '.join(justify_list[i::col_length]) for i in range(col_length) ]
    print("\n".join(lines))

def sort_func(a_str):
    tmp_str = a_str.lower() # fine with ASCII subset of characters
    for index,letter in enumerate(a_str):
        if letter == '_':
            continue
        else:
            return tmp_str[index:]

# main()
arguments = parse_command_line()
detail_level = arguments.list_type
try:
    installed_code,tag = options[detail_level]()
except KeyError:
    print('Error: detail_level argument must be \'packages\', \'modules\', or \'both\'')
    sys.exit()

ncols = arguments.ncols
print('\nFor output options type \'python_pkginfo.py -h\'')
print('\nInstalled '+tag+' for Python version '+sys.version+':\n')
print('To see package versions try: pip freeze | sort')
print('after issuing the appropriate \"setpkgs -a python\" command\n')
print_list = get_printed_names(installed_code)
print_list.sort(key=sort_func)
fmt_cols(print_list,ncols)

#!/usr/bin/env python
#
# This script will run on Python 2.7.x and later
#
# By default, iter_modules looks in sys.path which is initialized from
# the environment variable PYTHONPATH, plus an installation-dependent default.
from pkgutil import iter_modules
import sys

def parse_command_line():
    import argparse # added to Python 2.7 to replace optparse

Its typically considered best to import all modules at the beginning. Avoid importing inside functions. Also, the python history lesson was pointless.

    python_version = sys.version

Why are you doing this? Why not just use sys.version when you need this?

    parser = argparse.ArgumentParser(description='Use this script to view the installed packages and/or modules for Python version: '+python_version)
    parser.add_argument('--type',dest='list_type',type=str,default='packages',help='Can be \'packages\' (default), \'modules\', or \'both\'')
    parser.add_argument('--ncols',type=int,default=2,help='Number of columns in output (default: 2)')
    args = parser.parse_args()
    return args

I'd combine these last two lines.

def select_packages():
    return [ module[1:] for module in iter_modules() if module[2] ],'Packages'

def select_modules():
    return [ module[1:] for module in iter_modules() if not module[2] ],'Modules'

def select_both():
    return [ module[1:] for module in iter_modules() ],'Packages and Modules'

These three functions are awkward. For one, when looping over tuples, its generally best to unpack the tuples. This will make the code easier to follow. They are also largely duplicate of each other.

options = { 'packages' : select_packages,
            'modules' : select_modules,
            'both' : select_both,
}

The python standard is to use ALL_CAPS for global constants.

def get_printed_names(pack_tuple):

pack_tuple is a bad name, I don't know what its in it.

    p_name = []

avoid abbreviations like p, it just makes it harder to see what's going on.

    for code in pack_tuple:
        if ( code[1] ):

You don't need the parens. It'd also be easier to follow if you used for name, ispkg in pack_tuple:

            p_name.append(code[0]+' (Package)')
        else:
            p_name.append(code[0]+' (Module)')
    return p_name

Why does this function operate on a list of packages?

def fmt_cols(my_list, cols):

my_list is a bad name. I have no idea what's in the list.

    import math
    max_width = max(map(lambda x: len(x), my_list))

lamba x: len(x) is just len(x). So you can do max(map(len, my_list))

    col_length = int(math.ceil(len(my_list)/cols+1))
    justify_list = list(map(lambda x: x.ljust(max_width), my_list))

In python 2.x, map produces a list already. (Perhaps you're aiming to work on Python 3 as well?) It's also probably better to use a list comprehension.

    lines = [ ' '.join(justify_list[i::col_length]) for i in range(col_length) ]
    print("\n".join(lines))

There's not much point in putting everything in a line just to print it. I'd just print it directly in a for loop.

def sort_func(a_str):
    tmp_str = a_str.lower() # fine with ASCII subset of characters
    for index,letter in enumerate(a_str):
        if letter == '_':
            continue
        else:
            return tmp_str[index:]

It seems to me that you are just stripping off the _ so instead, you can use return a_str.lower().lstrip('_')

# main()

Normal style is to put this in a main function.

arguments = parse_command_line()
detail_level = arguments.list_type
try:
    installed_code,tag = options[detail_level]()
except KeyError:
    print('Error: detail_level argument must be \'packages\', \'modules\', or \'both\'')
    sys.exit()

Why don't you have argument parser check this?

ncols = arguments.ncols

Why are you copying this in a local?

print('\nFor output options type \'python_pkginfo.py -h\'')
print('\nInstalled '+tag+' for Python version '+sys.version+':\n')
print('To see package versions try: pip freeze | sort')
print('after issuing the appropriate \"setpkgs -a python\" command\n')
print_list = get_printed_names(installed_code)
print_list.sort(key=sort_func)
fmt_cols(print_list,ncols)

My rewrite of your script:

#!/usr/bin/env python
#
# This script will run on Python 2.7.x and later
#
# By default, iter_modules looks in sys.path which is initialized from
# the environment variable PYTHONPATH, plus an installation-dependent default.
from pkgutil import iter_modules
import sys
import argparse
import math

def parse_command_line():
    parser = argparse.ArgumentParser(description='Use this script to view the installed packages and/or modules for Python version: ' + sys.version)
    parser.add_argument('--type',dest='list_type',type=str,default='packages',help='What to list', choices = ['packages', 'modules', 'both'])
    parser.add_argument('--ncols',type=int,default=2,help='Number of columns in output (default: 2)')
    args = parser.parse_args()
    return args

LIST_TYPE = {
    'packages': (True, False, 'Packages'),
    'modules': (False, True, 'Modules'),
    'both': (True, True, 'Packages and Modules')
}

def find_modules(include_packages, include_modules):
    for module_loader, module_name, is_package in iter_modules():
        if is_package and include_packages:
            yield module_name, is_package
        elif not is_package and include_modules:
            yield module_name, is_package

def formatted_name(module):
    name, is_package = module
    if is_package:
        return name + ' (Package)'
    else:
        return name + ' (Module)'

def print_cols(cells, cols):
    max_width = max(map(len, cells))
    rows = int(math.ceil(len(cells)/cols+1))
    for row in range(rows):
        print(' '.join(x.ljust(max_width) for x in cells[row::rows]))

def main():
    arguments = parse_command_line()
    include_packages, include_modules, tag = LIST_TYPE[arguments.list_type]
    installed_code = sorted(
        find_modules(include_packages, include_modules),
        key = lambda module: module[0].lower().lstrip('_'))

    print('\nFor output options type \'python_pkginfo.py -h\'')
    print('\nInstalled '+tag+' for Python version '+sys.version+':\n')
    print('To see package versions try: pip freeze | sort')
    print('after issuing the appropriate \"setpkgs -a python\" command\n')
    print_cols(list(map(formatted_name, installed_code)), arguments.ncols)

if __name__ == '__main__':
    main()


Euler Project #3 Largest prime factor timeout I have been recently trying my hands on Euler's Project through HackerRank and got stuck with project #3 where you have to find the Largest Prime Factor.

Question: The prime factors of 13195 are 5, 7, 13 and 29.
What is the largest prime factor of a given number N?

I have written two different codes which both work for the first four problems in HackerRank but timeout on the last two. Please let me know how I can optimize my code further so that the timeout doesn't occur.

Code #1

for a in range(int(input())):
    n = int(input())
    b = n//2
    i=1
    if b%2==1:
        i+=1
    max=1
    while b > 2:
        if n%b==0:
            isprime = True
            for c in range(2,int(b**0.5)+1):
                if b%c==0:
                    isprime = False
                    break;
            if isprime:
                max = b
                break;
        b-=i
    if max==1:
        max=n
    print(max)

This one tries to find the first largest numbers which can divide a particular number and then tries to find whether it is a prime.

Code #2

for a in range(int(input())):
    n = int(input())
    num = n
    b = 2
    max=1
    while n > 1 and b < (num//2)+1:
        if n%b==0:
            n//=b
            max=b
        b+=1
    if max==1:
        max=n
    print(max)

This basically tries to find the last largest factor by multiple division.

It seems to me your second code example is wrong. You divide out only one prime factor and moving on testing for divisibility. If you enter, say: 8, it finds 2, divides it out, which gives 4, and will find 4 as largest prime that divides 8.

You have a lot of redundant tests, stepping b by one.

Consider this:
Write any positive number n as n = m + 6*q (q may be 0). Except for the prime numbers 2 and 3: If m is divisible by 2, then so is n (because 6 is divisible by 2). If m is divisible by 3, then so is n (again, because 6 is divisible by 3). The only chances for n to be prime is, if m is 1 or 5. Not all of those numbers are prime, but if it is prime, it will be 1 or 5 modulo 6.

That will add a few lines of code, to explicitly test for primes 2 and 3. But after that, it should speed up your algorithm. Just remember to divide out all powers of a prime before moving on. A single test could look like this:

n = int(input())
num = n
max = 1

if n % 2 == 0:
    max = 2
    while n % 2 == 0:
        n //= 2

if n % 3 == 0:
    max = 3
    while n % 3 == 0:
        n //= 3

b = 5
inc = 2

while n != 1:
    if n % b==0:
        max = b
        while n % b == 0:
            n //= b

    b += inc
    if inc == 2:
        inc = 4
    else:
        inc = 2


copy_if_max() algorithm I need a copy_if_max() function, i.e. copy all the elements in an input range that are equal to the max in that range. The most obvious implementation does two passes over the data:

template<class InputIt, class OutputIt>
OutputIt copy_if_max2(InputIt first, InputIt last, OutputIt dst)
{
    using T = typename InputIt::value_type;
    auto m = *std::max_element(first, last);
    return std::copy_if(first, last, dst, [&](T const& elem) {
        return elem >= m; // NOTE: not elem == m, because we use equivalence, not equality
    });
}

This is all nice and dandy (\$O(N)\$ complexity). However, for true input ranges, the two passes over the input data are a no go.

Note also that the maximum of the input range is defined in terms of an operator< that defines an equivalence relation, and not necessarily an equality relation. The suggested single-pass answer by @Yuushi would therefore not work.

I've come up with what I call a clearable back_inserter, that will reset the output range whenever the max is updated. I do that by having an assignment operator that takes a nullptr_t argument (which takes precedence in overload resolution over all other pointers, so even if the actual data would contain a nullptr, this would not match!). Furthermore, there is an implicit conversion to std::size_t so I can detect the underlying output container size.

template<class Container>
class clearable_back_insert_iterator
:
    public std::iterator< std::output_iterator_tag, void, void, void, void >
{
private:
    Container* c_;

public:
    using self_t = clearable_back_insert_iterator;
    using value_type = typename Container::value_type;

    clearable_back_insert_iterator(Container& c): c_(&c) {}

    operator std::size_t() { return c_->size(); }

    self_t& operator=(value_type const& v)
    {
        std::cout << "push_back \n";
        c_->push_back(v);
        return *this;
    }

    self_t& operator=(std::nullptr_t)
    {
        std::cout << "clear \n";
        c_->clear();
        return *this;
    }

    self_t& operator*() { return *this; }
    self_t& operator++() { return *this; }
    self_t& operator++(int) { return *this; }
};

template<class Container>
clearable_back_insert_iterator<Container> clearable_back_inserter(Container& c)
{
    return clearable_back_insert_iterator<Container>(c);
}

This allows me to write a single-pass copy_if_max1() that uses a clearable_back_inserter and a std::transform():

template<class InputIt, class OutputIt>
OutputIt copy_if_max1(InputIt first, InputIt last, OutputIt dst)
{
    using T = typename InputIt::value_type;
    auto m = std::numeric_limits<T>::min();
    std::transform(first, last, dst, [&](T const& elem) {
        if (dst == 0) {             // size()
            m = elem;
            return dst = elem;      // push_back()
        }
        if (m <= elem) {
            if (m != elem) {
                m = elem;
                dst = nullptr;      // clear()
            }
            dst = elem;             // push_back()
        }
        return dst;
    });
    return dst;
}

Live example. I realize that this one-pass version does more copying because it will copy all elements of the input range as long as they match the intermediate maximum, only to be cleared whenever the max is updated.

My questions:

Which points in this design would you critique?
Do you think clearable back_inserters are a good and reusable concept?
Should I use named members size() and clear() instead of conversion to size_t and assignment from nullptr?
Does Boost have some other alternatives (filter_iterator perhaps?)
Is it better to avoid the complications altogether and use the two-pas version copy_if_max2() (perhaps with a full copy of the input to allow multi-passes)?

I think you're overcomplicating the solution a bit. Since you're making copies of what is the maximum in the range, all you need is something that keeps a value to store the current max, and another which keeps a count of the number of maximum values that have been found in the input data. For example:

#include <iterator>

template <typename InputIter, typename OutputIter>
void copy_if_max(InputIter begin, InputIter end, OutputIter out_begin)
{
    std::size_t count = 1;
    typedef typename std::iterator_traits<InputIter>::value_type T;

    if(begin == end) return;

    T current_max = *begin;
    ++begin;

    while(begin != end) {
        if(*begin > current_max) {
            current_max = *begin;
            count = 1;
        }
        else if(*begin == current_max) {
            ++count;
        }
        ++begin;
    }

    for(std::size_t i = 0; i < count; ++i) {
        *out_begin++ = current_max;
    }
}

Note that this solution also uses std::iterator_traits<T>, which you should be using instead of relying on the InputIterator passed having a nested value_type typedef (which will of course be false if it is passed a T*, for example).


Reading file in C++ until string I need to read HTML files and ignore the input until reaching the following tag     <a class="member-modal" then print the characters between that and the closing
 tag.

Below is the extracted line of the file of interest.

Phil Jankins

Below is my approach

#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>
#include <cstdlib>
using namespace std;
int main()
{
string temp;
string delimet = "member - modal";

ifstream file;
file.open("in.txt");

getline(file, temp);
cout << temp;
while (!file.eof())
{
    if (temp.find(delimet))
        cout << temp;
    getline(file, temp);
}
file.close();

}

Unnecessary #includes:

You don't need <cstdlib> and <iomanip> for this program. Those can be removed.

Avoid using namespace:

using namespace in a source file for such a tiny program is fine, but don't make it a habit. Prefer to qualify the names with the namespace prefix (std::) when practical. Read a longer discussion here.

Open the stream on declaration and let the destructor close it:

Instead of explicitly calling open() in your file, use the parameterized constructor to make your code more concise and direct:

std::ifstream file("in.txt");

Also, no need to explicitly close at the end of main(). The destructor will close the file automatically when the scope of declaration is exited.

Be sure to always check that the file was opened successfully! If it fails, you should at least print a message to let the user know about the problem.

Use the return value of getline() instead of eof():

The canonical line reading loop in C++ is:

while (std::getline(file, line))
{
}

That can simplify you program quite significantly. You can still test specific error states with the methods bad(), fail() and eof(). See the documentation for std::ifstream.

Incorrect use of string::find():

You have misinterpreted the return value of std::string::find(). This method returns the position of the first character of the first match. If no matches were found, the function returns std::string::npos.

Which means that testing it as in if (temp.find(delimet)) will probably not work as you intended, since the return value is not a boolean.

The correct usage is:

if (str.find(substring) != std::string::npos) { /* found the substring */ }

Also note that "member - modal" is not the same as "member-modal". find() will look for an exact match, including spaces, so your implementation will only find something in the input file if you replace the search string to match the contents of the file down to each space. A more flexible implementation could be achieved using C++ regular expressions.

Be more thoughtful about variable naming:

Names line temp and delimet convey little meaning.

temp should at the very least be named line, as that is what the variable holds (a line from the file).

delimet sounds like a misspelled "delimiter", so delimiter would be better. Other options include needle, for searching a needle in a haystack, target, for the target of the search, wanted_string, and the list goes on...

Properly indent main():

Code inside main() is not properly indented to convey nesting into the function. Proper indentation is very important for code readability, so that must be fixed.

Summing up:

This is my suggest implementation, based on the original code and applying the above changes:

#include <fstream>
#include <iostream>
#include <string>

int main()
{
    // Alway check after opening to make sure the
    // file existed and was opened successfully.
    //
    std::ifstream file("in.txt");
    if (!file.is_open())
    {
        std::cerr << "Failed to open file!\n";
        return -1;
    }

    // Since this is a read-only variable,
    // also mark is as `const` to make intentions
    // clear to the readers.
    //
    const std::string needle = "member-modal";

    // Main reading loop:
    //
    std::string haystack;
    while (std::getline(file, haystack))
    {
        if (haystack.find(needle) != std::string::npos)
        {
            std::cout << haystack << "\n";
            // Prints
            //  Phil Jankins
            // once
        }
    }
}


Download URL from clipboard This is a Python script I made to download a URL from the clipboard.

When a new URL is copied to a clipboard it adds the URL to a download queue, then on a separate thread it downloads the URL to a file.

I am new to multithreading.  Is my script properly using multithreading? It needs to download the data while still listening for changes in the content of the clipboard.

import time
import os
import urllib
import subprocess
import threading

download_number = 0
count=0
save_file = "links_list.txt"
old_url = ""
download_queue = []
download_delay = 5

data_file = open(save_file, "r")
past_links = data_file.read()
data_file.close()

def get_image():
    print download_queue
    url = download_queue.pop(0)

    data = urllib.urlopen(url).read()
    name = url.split("/")[-1:][0]
    extension = name.split(".")[-1:][0]
    basename = name.split(".")[:-1][0]

    # make a new file name
    if os.path.exists(name):
        new_name = basename+"-0."+extension
        try_count = 0
        while os.path.exists(new_name):
            try_count += 1
            new_name = basename+"-"+str(try_count)+"."+extension
        name = new_name

    data_file = open(name, "w")
    data_file.write(data)
    data_file.close()

    # goto the next link
    if len(download_queue) != 0:
        time.sleep(download_delay)
        get_image()

while(1):
    time.sleep(0.25)
    clipboard = subprocess.check_output('pbpaste', shell=True)
    # check if the clipbard contains a new url
    if clipboard[:4] == "http" and clipboard != old_url and clipboard not in past_links:
        past_links += clipboard
        old_url = clipboard

        # add url to queue
        download_queue.append(clipboard)
        print download_number,

        # start the download fuction
        t = threading.Thread(target=get_image)
        t.start()

        # links that have been downloaded
        data_file = open(save_file, "a")
        data_file.write(clipboard+"\n")
        data_file.close()

        download_number += 1

You maintain old_url as well as past_links. You probably don't need old_url at all because everything you process will be added to past_links.

You are reading past_links as one giant string with embedded newlines. That won't be very efficient when doing clipboard not in past_links because it will have to search through the whole string.

When you run past_links += clipboard, you are sticking the data on to the end of past_links with no newline at the end, so new links added there will be jammed together.

You should probably set up past_links as a set, and use set operations. You can still read and write it as a plain text file.

You create a new thread for each download request, which will download multiple files in parallel. It's not clear that is what you really want to do.

Your use of a recursive call to get_image() is a very strange way to implement a loop.

You are reading the entire download into memory in one giant string, and then writing it out to disk. If you were to download a very large file, your script would require a corresponding amount of memory during downloading. Use urllib.urlretrieve to read directly into a file.

Use clipboard.startswith("http") instead of clipboard[:4] == "http"

Finally, use the standard Python Queue module instead of implementing your own:

The Queue module implements multi-producer, multi-consumer queues. It is especially useful in threaded programming when information must be exchanged safely between multiple threads.


A scalable way to remove duplicates from Arrays without extensions in Swift 4 This method is an attempt at removing duplicates from arrays without the use of extensions.
func arrayWithDuplicatesRemoved<T: Equatable>(from array: [T]) -> [T] {
    var results = [T]()
    return array.compactMap { (element) -> T? in
        if results.contains(element) {
            return nil
        } else {
            results.append(element)
            return element
        }
    }
}

let dirty = ["apple", "kiwi", "grapefruit", "kiwi", "kiwi", "strawberry", "watermelon", "apple", "banana"]
let clean = arrayWithDuplicatesRemoved(from: dirty)
print(clean) // ["apple", "kiwi", "grapes", "strawberry", "watermelon", "banana"]

Your algorithm is O(n^2) so not very scalable... The immediate improvement I can think of is to use a Set for results (which means making T Hashable).

If you aren't worried about position, then just create a Set from an array and you are done.


Edge finding binary search Imagine a website that publishes the number of daily commuters in only a graphical form each day using a bar chart.  I want to determine the number by reading the bar chart after saving the graphic as an image (the image stuff is not important here).  The way I want to read the bar chart is by going to a pixel number (the #of commuters axis) and asking the question, "Is the pixel 'on' or 'off'?"  (On means that the bar is present and off means that this guess too high)  Note: that there is a lower bound of 0 and, technically, an infinite upper bound.  But, in reality, 10000 may be the realistic upper bound.  Also note, No Change from yesterday is a frequent finding.

Given a starting number from yesterday to guess, what's the most efficient way to find the number?  Does my function provide the most efficient method?  Efficient means fewest number of queries to ask if a pixel is on or off.  There will be multiple bars to measure and keeping track of the iteration number might add unwanted complexity if it's not truly needed.

My algorithm follows as a function.  Any advice is most welcome.
(brought to CR from SE, https://stackoverflow.com/questions/13782587/edge-finding-binary-search )

def EdgeFind(BlackOrWhite,oldtrial,high,low):
# BlackOrWhite is a 0 or 1 depending on if the bar is present or not.  A 1 indicates that you are below or equal to the true number.  A 0 indicates that you are above the true number
# the initial values for oldtrial, high, and low all equal yesterday's value

 factorIncrease = 4 #5
 finished = 0

 if BlackOrWhite == 1 and oldtrial==high:
    newtrial = oldtrial+factorIncrease*(high-low)+1
    high = newtrial
    low = oldtrial
 elif BlackOrWhite == 1 and high-oldtrial==1:
    finished = 1
    newtrial = oldtrial
 elif BlackOrWhite == 1:
    newtrial = oldtrial+(high-oldtrial)/2
    low = oldtrial

 if BlackOrWhite == 0 and oldtrial==low:
    newtrial = (oldtrial)/2
    high = oldtrial
    low = newtrial
 elif BlackOrWhite == 0 and oldtrial-low==1:
    finished = 1
    newtrial = oldtrial-1
 elif BlackOrWhite == 0:
    newtrial = oldtrial-(oldtrial-low+1)/2
    high = oldtrial

 if (oldtrial==1) and low!=1:
    finished = 1

 return finished,newtrial,high,low

How about

if BlackOrWhite == 1:
  if oldtrial == high:
    newtrial = oldtrial+factorIncrease*(high-low)+1
    high = newtrial
    low = oldtrial
  elif high-oldtrial==1:
    finished = 1
    newtrial = oldtrial
  else:
    newtrial = oldtrial+(high-oldtrial)/2
    low = oldtrial

For simplifying your ifs?


Accessing Models Directly Through AJAX I was thinking about how to handle AJAX requests in a MVC environment in PHP in a easy way.

What I've come up is (without showing you the backend code):

$.ajax({
    // url is empty so the request goes to the current route and controller
    // each controller (extended from base controller) checks every request to
    // see if it's an AJAX one. If it is it calls the Model/method specified bellow
    url:'',
    method:'POST',
    data: {
        toCall: 'User@create', // Calls model "User" with method "create"
        args: userData // arguments for method "create"
    },
    success: function(r) {
        // return JSON or true/false
    }
});

So basically the key is this "toCall: 'User@create'" line. which calls models directly. Should that be avoided because of security? Is it possible for a user to somehow change this "User@create" to access other models and methods?

Is my question clear?

Yes this raises concerns

You'll be skipping some controller logic which may involve;

Checking certain user conditions (ie: are they logged-in)
Can they perform this model action due to their ACL/permissions?
Collecting other data from other models/methods to pass into this model method

Depending on your design, accessing a model directly and skipping the controller logic can raise some unexpected errors (described above) and/or give users actions they wouldn't necessarily have. i.e: Admin@RemoveUser

Usually in an MVC pattern, the model and method names are guessable, depending on the action. Ie: http://example.com/user/view/1 = User model, viewAction method. So exposing this information isn't really a concern. What is a concern is users accessing model action that are restricted by controller logic.

So, what should I do?

Contact the controller directly, and have your view processor to detect it's an AJAX request (through the HTTP_X_REQUESTED_WITH=XMLHttpRequest header) to "disable" the default view with all the design, and just passback (usually) a JSON string that you can use with JavaScript (JSON.parse) and do as you want with the result.


Argparse with subparsers We have a main application made of a few separate parts - Unity and Clc.

I'm writing a new tool which will be used to manage them: deploy Unity, or run some checks and a lot of other stuff. As there will be a lot of options, I want split them in to separate namespaces.

Here is my implementation, it works, but I'm not sure if it's the best way.

import argparse

parser = argparse.ArgumentParser()
subparsers = parser.add_subparsers()

parser_unity = subparsers.add_parser('unity', help='Unity help')
parser_unity.add_argument('-t', '--tagcheck', dest='unity_tagcheck', action='store_true', help='Check tags in .csproj files')
parser_unity.add_argument('-d', '--deploy', dest='unity_deploy', action='store_true', help='Deploy Unity project to appropriate URL')

parser_clc = subparsers.add_parser('clc', help='Clc help')
parser_clc.add_argument('-d', '--deploy', dest='clc_deploy', action='store_true', help='Deploy Cloudlibrary project to appropriate environment')

res = parser.parse_args()

print('All %s' % res)

try:
    if res.unity_deploy:
        print('Unity deploy')

    if res.unity_tagcheck:
        print('Unity tagcheck')
except AttributeError:
    pass

try:
    if res.clc_deploy:
        print('Clc deploy')
except AttributeError:
    pass

And here is how is works:

d:\RDS\rdsmanager>tmp\test.py clc -d
All Namespace(clc_deploy=True)
Clc deploy

d:\RDS\rdsmanager>tmp\test.py unity -d -t
All Namespace(unity_deploy=True, unity_tagcheck=True)
Unity deploy
Unity tagcheck

d:\RDS\rdsmanager>tmp\test.py unity -d
All Namespace(unity_deploy=True, unity_tagcheck=False)
Unity deploy

If you take a look at the argparse documentation, they suggest to use
set_defaults on each of the parsers to associate a processing function
depending on which parser is selected.  So if you do that then you can
get rid of the try/except blocks, which would improve things quite a
lot.

E.g.:

def handler_unity(res):
    if res.unity_deploy:
        print('Unity deploy')

    if res.unity_tagcheck:
        print('Unity tagcheck')

def handler_clc(res):
    if res.clc_deploy:
        print('Clc deploy')
...
parser_unity.set_defaults(func=handler_unity)
...
parser_clc.set_defaults(func=handler_clc)
...
res.func(res)


How can I store the fibonacci sequence in a vector during each recursive call? #include <iostream>
#include <vector>

void getUserNum(int &);
int rFib(int n);
void dispSeq(std::vector<int> &vRef);

int main()
{
    int userNum;
    getUserNum(userNum);
    rFib(userNum);
    return 0;
}

void getUserNum(int &refVal)
{
    std::cout << "Enter a positive integer:\n";
    std::cin >> refVal;
}

int rFib(int n)
{
     static std::vector<int> fib;
     static bool baseReached = false;
     fib.push_back(n);

     if (baseReached)
     {
             dispSeq(fib);
     }

     if (n <= 1)
     {
        baseReached = true;
        return n;
     }
     else
     {
        return rFib(n-2) + rFib(n-1);
     }
 }

void dispSeq(std::vector<int> &v)
{
    for (size_t i = 0; i < v.size(); i++)
    {
        std::cout << v[i] << " ";
    }
    std::cout << std::endl;
 }

Its normal to put parameter names even in the declarations:

void getUserNum(int &);

We use this to help us identify what the function is doing.

In C++ the '*' and the '&' are part of the type information. So it is usual to push these left towards the type rather than the name. Note this is basically the opposite of C conventions. But C++ is much more rigorous on types and safety than C.

void dispSeq(std::vector<int> &vRef);

// Normally in C++ code I would expect this:
void dispSeq(std::vector<int>& vRef);

Be careful with your use of out parameters.

void getUserNum(int &refVal)

It seems more natural to return a value from a function then get sombody inside to set an external value via a reference (not unheard of and it will depend a lot on the context). But in this situation I would return a value. It will then make using the code eaiser.

 int userNum   = getUserNum();

Error Checking (especially input from a human) needs to be more rigorous.

void getUserNum(int &refVal)
{
    std::cout << "Enter a positive integer:\n";
    std::cin >> refVal;
}

If a user enters a non positive value things will go bad. If the user inputs a string the value of refVal I believe is left unaltered (which leaves your code with an uninitialized variable).

int getUserNum()
{
    int result;

    // user input is done by the line.
    // so read a line at a time and validate it.
    std::string  line;
    std::getline(std::cin, line);

    std::stringstream  linestream(line);

    int result;
    if (linestream >> result)
    {
         // The read worked.
         // Lets check the user did not type ofther crap on the line.
         //
         char x;
         if (!(linestream >> x))
         {
               // Reading more data worked. This means the input was good.
               // As the user typed a number followed by some other stuff
               // like  56.4   (the .4 is illegal)
               //       5Plop  (the Plop is illegal)
               //
               // The the above would have successfully read a value into
               // `x` this with the ! it would have failded and this
               // this condition would not have been entered.

               // We only return if:
               //    1. We read an integer.
               //    2. There is no other data on the line except white space.
               return result;
         }
    }
    // invalid input
    std::cout << "Stupid user input aborting\n";
    exit(1); // Reading failed.
             // Leave as an exercise on how to repeat the question.
             // You can also compress that very verbose if statement into a single
             // line without it looking too bad.
}

// Single line if
if ((linestream >> result) && !(linestream >> x))
{   return result;
}

In Fib() you should probably not use recursion. Prefer a while loop. Loop from 0 upto the point you want to reach. This makes using a vector easy. You can also start from where you left off last time rather than 0.

int fib(int n)
{
     static std::vector<int>   values = {1,1};
     if (n < values.size()-1) }return values[n];

     int start = values.size();
     for(int loop = start; loop < n; ++loop)
     {
          values[loop] = values[loop-2] + values[loop-1];
     }
     return values[n];
}


Find Users by Most Badges I very infrequently use SQL. I need more experience, so I have created a data explorer query to find those users with most of any particular badge (have to edit the query itself right now to alter it). I like following best practices, so please feel free to nit-pick and really do this right (indentation, capitalization, everything).

I have commented code that I cannot get to work (I would like it to work by user input), I hope my intent is quite obvious.

--declare @BadgeName nvarchar(50) = ##BadgeName:string##
--declare @MyTop int = ##MyTop:int##

select
--  top @MyTop
    top 100
    Badges.UserId as [User Link],
    Users.Reputation,
    --Badges.Name,
    count(Badges.Id) as NumberOfBadges
  from
      Badges
    inner join
      Users On Users.Id=Badges.UserId
  where
--    Badges.Name = @BadgeName and
      Badges.Name = 'Great Answer' and
      Users.Reputation > 10000
group by
    Badges.UserId, Users.Reputation
order by
    NumberOfBadges desc

Here's the actual query if you want to run it:

http://data.stackexchange.com/stackoverflow/query/271133/search-users-by-number-of-badges

DECLARE @BadgeName AS NVARCHAR(50) = ##BadgeName:string?Great Answer##;
DECLARE @MyTop AS INT = ##MyTop:int?100##;

SELECT TOP(@MyTop)
    Badges.UserId AS [User Link],
    Users.Reputation,
    @BadgeName AS BadgeName,
    COUNT(Badges.Id) AS NumberOfBadges
FROM  Badges
  INNER JOIN Users ON Users.Id=Badges.UserId
WHERE Badges.Name = @BadgeName
  AND Users.Reputation > 10000
GROUP BY Badges.UserId, Users.Reputation
ORDER BY NumberOfBadges DESC

The DECLARE statements should have semi-colons at the end.

Apparently you need the parentheses when using SELECT TOP with a variable.

As you noticed, you need to put columns in the GROUP BY to include them in the SELECT.  You can get around this by using the variable instead of the column.

I prefer capitalized keywords and a more compact formatting, but that's really up to you.


Pairing anagrams in a string list The task was to check for pairs of anagrams in a string list.

I first sorted all the words. and looped over the letters.
The interview continues with a follow-up, to do the same with limited memory.
I tried to implement a Hash function.

The dictionary return value is only for my convince in order to see all the pairs. I could just print it into the console instead.

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Security.Cryptography;
    using System.Text;
    using System.Threading.Tasks;

    namespace ConsoleApplication2
    {
    public class ReverseString
    {
        public ReverseString()
        {
            List<string> anagarms = new List<string>();
            anagarms.Add("Gilad");
            anagarms.Add("dilaG");
            anagarms.Add("bat");
            anagarms.Add("but");
            anagarms.Add("utb");
            pairs = PairAnagrams(anagarms);
            pairs = PairAnagramsHash(anagarms);
        }

        Dictionary<string, string> PairAnagrams(List<string> wordsList)
        {
            Dictionary<string, string> dictionary = new Dictionary<string, string>();
            foreach(var item in wordsList)
            {
                if (dictionary.ContainsValue(item))
                {
                    continue;
                }
                foreach(var item2 in wordsList)
                {
                    if(item.Length != item2.Length || item.Equals(item2))
                    {
                        continue;
                    }

                    var sortedWord1 = item.ToArray();
                    Array.Sort(sortedWord1);
                    var sortedWord2 = item2.ToArray();
                    Array.Sort<char>(sortedWord2);
                    int i;
                    for (i = 0; i < sortedWord2.Length; i++)
                    {
                        if (sortedWord2[i] != sortedWord1[i])
                        {
                            break;
                        }
                    }
                    if(i == sortedWord1.Length)
                    {
                        dictionary.Add(item, item2);
                    }
                }
            }
            return dictionary;
        }

        Dictionary<string, string> PairAnagramsHash(List<string> wordList)
        {
            Dictionary<string, string> dictionary = new Dictionary<string, string>();

            foreach (var item in wordList)
            {
                foreach (var item2 in wordList)
                {
                    if (item.Equals(item2) || dictionary.ContainsKey(item2))
                    {
                        continue;
                    }
                    if (HashFunction(item) == HashFunction(item2))
                    {
                        dictionary.Add(item, item2);
                    }
                }
            }
            return dictionary;
        }

        public int HashFunction(string word)
        {
              int result = 0;
              var charArray = word.ToArray();
              for (int i = 0; i < word.Length; i++)
              {
                  result += charArray[i] * 37;
              }
              return result;
        }
    }
}

Algorithm of PairAnagrams()

You are calling

var sortedWord1 = item.ToArray();
Array.Sort(sortedWord1);

inside the inner loop, but item won't change there.

One time you are using Array.Sort(sortedWord1); and two lines below Array.Sort<char>(sortedWord2); Be consistent in your style.

This

for (i = 0; i < sortedWord2.Length; i++)
{
    if (sortedWord2[i] != sortedWord1[i])
    {
        break;
    }
}
if (i == sortedWord1.Length)
{
    dictionary.Add(item, item2);
}

can be replaced by

if (sortedWord1.SequenceEqual(sortedWord2))
{
    dictionary.Add(item, item2);
}

By using .Where() on the wordList at the for each you can omit the check for length and equal.

By using the .OrderBy() on the chars of the words, there won't be a need to call .ToArray() and Array.Sort().

This reduces the code to

Dictionary<string, string> PairAnagrams(List<string> words)
{
    Dictionary<string, string> pairedAnagrams = new Dictionary<string, string>();
    foreach (var word in words)
    {
        if (pairedAnagrams.ContainsValue(word))
        {
            continue;
        }

        var sortedWord1 = word.OrderBy(c => c);
        int wordLength = word.Length;
        foreach (var word2 in words.Where(w => w.Length == wordLength && !word.Equals(w)))
        {
            if (sortedWord1.SequenceEqual(word2.OrderBy(c => c)))
            {
                pairedAnagrams.Add(word, word2);
            }
        }
    }
    return pairedAnagrams;
}

Constructor

The constructor of an object should be used only for constructing the object. You are misusing it to start the action.

Naming

variable names should be correctly spelled. anagarms vs anagrams
parameters should't be pre/post fixed by their types. List<string> wordsList-> List<string> words

General

no need for using System.Security.Cryptography


Registering Pokémon weaknesses I recently answered a question that had me wondering if I could simplify this below code without creating an extra function:

for object in objects {
    if let type = object["type"] where !type.isEmpty, let name = object["name"] {

        print(pokemonTypeDefenseChart[type])
        for weakness in pokemonTypeDefenseChart[type]! {
            if pokemonWeaknessChart[weakness] == nil {
                pokemonWeaknessChart[weakness] = []
            }
            pokemonWeaknessChart[weakness]?.append(name)
        }
    }

    if let typeTwo = object["typeTwo"] where !typeTwo.isEmpty, let name = object["name"] {
        for weakness in pokemonTypeDefenseChart[typeTwo]! {
            if pokemonWeaknessChart[weakness] == nil {
                pokemonWeaknessChart[weakness] = []
            }
            pokemonWeaknessChart[weakness]?.append(name)
        }
    }
}

Essentially the code loops through objects (list of pokemon) and is designed to add all of the pokemon weak to type (and optionally typeTwo).

I ended up just refactoring the for statements inside the if let statements into a function, but I was wondering if it was possible to compress these without an extra function. The only difference is the type.

I know I could simplify it a bit by using a map instead of for loops but I wanted to keep it more understandable for the question.

Any ideas?

You could do something like:

for x in ["type", "typeTwo"] {
    if let type = object[x] where !type.isEmpty,
        let name = object["name"],
        defenseChart = pokemonTypeDefenseChart[type] { /* ... */ }
}

Also, to simplify the rest of your code, you could extend Dictionary like this:

extension Dictionary {
    subscript(key: Key, fallback fallback: Value) -> Value {
        get { return self[key] ?? fallback }
        set { self[key] = newValue }
    }
}

This allows you to replace this:

for weakness in defenseChart {
    if pokemonWeaknessChart[weakness] == nil {
        pokemonWeaknessChart[weakness] = []
    }
    pokemonWeaknessChart[weakness]?.append(name)
}

by this:

for weakness in defenseChart {
    pokemonWeaknessChart[weakness, fallback: []].append(name)
}


Comparison function for strings I got a function that has as input 2 strings, computes the string similarity of all the combinations and give back and output the highest similarity. For example, "you are" and "are you" will have 1 as similarity value.

import itertools
from difflib import SequenceMatcher
import numpy as np

def _compare_names(a, b):
    return SequenceMatcher(None, a, b).ratio()

def _compare_names_with_combinations(name_1,name_2):
    name_1 = name_1.lower().split()
    name_2 = name_2.lower().split()
    combinations_name_1 = list(itertools.permutations(name_1))
    combinations_name_2 = list(itertools.permutations(name_2))
    combinations_name_joined_1 = [' '.join(list(name)) for name in combinations_name_1]
    combinations_name_joined_2 = [' '.join(list(name)) for name in combinations_name_2]
    distances = []
    for name1 in combinations_name_joined_1:
        for name2 in combinations_name_joined_2:
            distance = _compare_names(name1,name2)
            distances.append(distance)
    return max(distances)

examples:

_compare_names_with_combinations('you are','are you')
>> 1
_compare_names_with_combinations('you are','are yos')
>> 0.85714285

My concerns come when I have to compare a lot of texts and it seems that there should be around a more efficient way of computing this value. Do you think there is space in this function to decrease the computational time?

This will not reduce time complexity, just space complexity; as well as spending less time in the interpreter, most of the work being done in C.

The key idea is to make use of generators to avoid building lists that you only iterate once; reducing your need for memory management, thus increasing the overall speed.

Since you are already importing itertools, I’ll just import the relevant parts to save on typing a bit, feel free to use the long names if you see fit:

from itertools import product, permutations

in case you are using Python 2, I'd use:

from itertools import product, permutations, imap as map

Now we transform each list into a generator expression:

def _compare_names_with_combinations(name_1,name_2):
    join = ' '.join
    name_1 = name_1.lower().split()
    name_2 = name_2.lower().split()
    distances = []
    for name1 in map(join, permutations(name_1)):
        for name2 in map(join, permutations(name_2)):
            distance = _compare_names(name1,name2)
            distances.append(distance)
    return max(distances)

Then product lets us merge the two loops:

def _compare_names_with_combinations(name_1,name_2):
    join = ' '.join
    name_1 = name_1.lower().split()
    name_2 = name_2.lower().split()
    distances = []
    for name1, name2 in product(map(join, permutations(name_1)), map(join, permutations(name_2))):
        distance = _compare_names(name1,name2)
        distances.append(distance)
    return max(distances)

An other improvement is to not build the list of all distances since we are only interested in the maximum value: let's put this max call out of the function and turn it into a generator:

def _compare_names_with_combinations(name_1,name_2):
    join = ' '.join
    name_1 = name_1.lower().split()
    name_2 = name_2.lower().split()
    for name1, name2 in product(map(join, permutations(name_1)), map(join, permutations(name_2))):
        yield _compare_names(name1,name2)

def score_names(name_1, name_2):
    return max(_compare_names_with_combinations(name_1, name_2))

Well, we’re just mapping _compare_names over the product. Maybe we could have that max call in the function after all:

def _compare_names_with_combinations(name_1, name_2):
    join = ' '.join
    name_1 = name_1.lower().split()
    name_2 = name_2.lower().split()
    return max(
        map(
            _compare_names,
            product(
                map(join, permutations(name_1)),
                map(join, permutations(name_2))
            )
        )
    )

But this will require that we modify _compare_names to accept a tuple of two names as parameter instead of two names as two separate parameters:

def _compare_names(names):
    a, b = names
    return SequenceMatcher(None, a, b).ratio()


A custom String Builder implementation Two code files below : the actual logic & the test logic.

Core Logic

package stringBuilder;

public class NoksStringBuilder {

    //initial capacity
    private static final int INITIAL_SIZE = 3;

    String[] stringList = new String[INITIAL_SIZE];

    //number of strings in the builder currently
    int size = 0;

    //total number of chars in the builder , sum total of length of each string
    int characterCount = 0;

    public void add(String s){
        if(size < stringList.length){
            stringList[size++] = s;
            characterCount += s.length();
        }
        else{
            String[] temp = new String[stringList.length*2];

            for(int i =0 ; i< stringList.length; i++){
                temp[i] = stringList[i];
            }

            stringList = temp;
            add(s);
        }
    }

    public String toString(){
        char[] output = new char[characterCount];
        int outputIndex = 0;

        for(int i = 0; i < size; i++){
            for(int j = 0; j < stringList[i].length(); j++){
                output[outputIndex++] = stringList[i].charAt(j);
            }
        }

        return new String(output);

    }

}

Test Class

package stringBuilder;

import java.util.Scanner;

public class Test {

    public static void main(String[] args){
        NoksStringBuilder stringBuilder = new NoksStringBuilder();

        Scanner inputScanner = new Scanner(System.in);
        int i = 0;

        while(i != 3){

            printMenu();
            i = Integer.parseInt(inputScanner.nextLine());

            if(i == 1){
                System.out.println("Enter string");
                stringBuilder.add(inputScanner.nextLine());
            }
            else if( i == 2){
                System.out.println(stringBuilder.toString());
            }

        }

    }

    private static void printMenu() {
        System.out.println("StringBuilder Options :");
        System.out.println("1.Add");
        System.out.println("2.ToString");
        System.out.println("3.Quit");
    }

}

public class NoksStringBuilder {

When told that something implements StringBuilder, I would expect it to implement the same interfaces.

public class NoksStringBuilder implements CharSequence, Appendable, Serializable {

This way it can be used as a replacement for a StringBuilder used in more generic circumstances.

    String[] stringList = new String[INITIAL_SIZE];

You call this a stringList, but it's actually a String array.  If it were an actual List, you could quit mucking around managing capacity.

      List<String> strings = new ArrayList<>();

This would push all the memory management off onto the List.  But I actually don't think that anything involving String is correct here.  More on that later.

    public void add(String s){

Similarly, I would call this

    public Appendable append(CharSequence csq) {

Then you could use it the same way as with a real StringBuilder.  Or you could use it the way you do your method (realizing that a String is a CharSequence).

All this would make obvious that an array of String is not a good way to hold a StringBuilder.  For one thing, while it makes for a quick append of a String, it works less well with letters and you lose the random access aspect.  I'd probably try

    private char[] sequence = new char[DEFAULT_CAPACITY];

That puts more work into add but makes toString simpler.

    public String toString() {
        return new String(sequence, 0, length);
    }

Note that length is an object field that you'll have to maintain.  It's the equivalent of your characterCount.

Note that if you want to implement capacity(), you could just say

    public int capacity() {
        return sequence.length;
    }


A scalable lock-free channel Here's my lock-free channel impl lfchan,

I didn't want to copy/paste the code here since it is multiple files.

I'm trying to get an idea of what can be improved or if there are any bugs in it.

chan.go

type innerChan struct {
    q       []aValue
    sendIdx uint32
    recvIdx uint32
    slen    uint32
    rlen    uint32
    die     uint32
}

// Chan is a lock free channel that supports concurrent channel operations.
type Chan struct {
    *innerChan
}

// New returns a new channel with the buffer set to 1
func New() Chan {
    return NewSize(1)
}

// NewSize creates a buffered channel, with minimum length of 1
func NewSize(sz int) Chan {
    if sz < 1 {
        panic("sz < 1")
    }
    return Chan{&innerChan{
        q:       make([]aValue, sz),
        sendIdx: ^uint32(0),
        recvIdx: ^uint32(0),
    }}
}

// Send adds v to the buffer of the channel and returns true, if the channel is closed it returns false
func (ch Chan) Send(v interface{}, block bool) bool {
    if !block && ch.Len() == ch.Cap() {
        return false
    }
    ln, cnt := uint32(len(ch.q)), uint32(0)
    for !ch.Closed() {
        if ch.Len() == ch.Cap() {
            if !block {
                return false
            }
            runtime.Gosched()
            continue
        }
        i := atomic.AddUint32(&ch.sendIdx, 1)
        if ch.q[i%ln].CompareAndSwapIfNil(v) {
            atomic.AddUint32(&ch.slen, 1)
            return true
        }
        if block {
            if i%250 == 0 {
                pause(1)
            }
        } else if cnt++; cnt == ln {
            break
        }
        runtime.Gosched()
    }
    return false
}

// Recv blocks until a value is available and returns v, true, or if the channel is closed and
// the buffer is empty, it will return nil, false
func (ch Chan) Recv(block bool) (interface{}, bool) {
    if !block && ch.Len() == 0 { // fast path
        return nilValue, false
    }
    ln, cnt := uint32(len(ch.q)), uint32(0)
    for !ch.Closed() || ch.Len() > 0 {
        if ch.Len() == 0 {
            if !block {
                return nilValue, false
            }
            runtime.Gosched()
            continue
        }
        i := atomic.AddUint32(&ch.recvIdx, 1)
        if v, ok := ch.q[i%ln].SwapWithNil(); ok {
            atomic.AddUint32(&ch.rlen, 1)
            return v, true
        }
        if block {
            if i%250 == 0 {
                pause(1)
            }
        } else if cnt++; cnt == ln {
            break
        }
        runtime.Gosched()
    }
    return nilValue, false
}

Out of order bug
It appears that there could be a case where the items inserted into the channel can be read out of order.  From reading your test code,  it seems that you want to maintain a FIFO ordering, so I am guessing this is unwanted behavior.  Here's the sequence of events that can trigger the behavior:

Initially, sendIdx recvIdx are -1, and slen rlen are 0.
Thread 1 calls Send, and atomically increments sendIdx to 0.
Thread 1 is context switched out before filling in q[0].
Thread 2 calls Send, and atomically increments sendIdx to 1.
Thread 2 fills in ch.q[1] and atomically increments slen to 1.
Thread 3 calls Recv, and sees that there is 1 element in the queue because slen - rlen = 1.
Thread 3 atomically increments recvIdx to 0.
Thread 3 tries to do a read from q[0] but finds that it was empty.
Thread 3 atomically increments recvIdx to 1.
Thread 3 reads from q[1] and find the value placed there by Thread 2.
Thread 1 wakes up an inserts its element into q[0].

So at this point, the item added by Thread 2 in q[1] was read before the item added by Thread 1 in q[0], but it could be argued that Thread 2 inserted its element timewise before Thread 1 so that behavior might be OK.  However:

Thread 1 calls Send again, and inserts an element into q[2].
Thread 3 calls Recv and reads the element from q[2].
Thread 3 calls Recv and reads the element from q[0] (after wrapping around the queue once).

Now, it is clear that Thread 1 sent q[0] before q[2], but Thread 3 received q[2] before q[0], which is out of order.


Finding the number of ways to partition {1,2, ..., N} into p1 and p2 such that sum(p1) == sum(p2) I am trying to write a space and time efficient algorithm to calculate the number of ways to partition a set of integers {1, 2, ..., N} into two partitions such that the sums of integers in the two partitions are equal. I have started with a brute force approach that seems to give me correct results:

from itertools import combinations
def f(n):
    s = set([i+1 for i in range(n)])
    valid_partitions = []
    for i in range(1,n):
        combos = combinations(s,i)
        for c in combos:
            p1 = frozenset(c)
            p2 = set(range(1,n+1)) - p1
            p2 = frozenset(p2)
            if sum(p1) == sum(p2):
                valid_partitions.append(frozenset((p1,p2)))

    return len(set(valid_partitions))

Also, since I am checking every way to partition {1,2,...,N} into two non-empty sets, is it correct to say that the Big O time complexity of this approach is S(N,2) (Stirling Number of the Second Kind)? Also, how would I evaluate the Big O space complexity of this approach? Is there a better approach I can take?

Duplicated code/values

You have both:

s = set([i+1 for i in range(n)])

and

p2 = set(range(1,n+1)) - p1

This is bad because you have the same sets generated in 2 places with 2 different pieces of code:

it is not efficient
it is not easy to read and understand
it is easy to get wrong if you ever need to update the set definition.

You should write something like:

        p2 = frozenset(s - p1)

Various hints

If sum(p1) == sum(p2), then sum(p1) == sum(p2) == sum(s) // 2 and sum(p1) == sum(s) // 2 is a sufficient condition.
You could write something like:

def f(n):
    s = set([i+1 for i in range(n)])
    target_sum = sum(s) / 2
    valid_partitions = []
    for i in range(1,n):
        for c in combinations(s,i):
            if sum(c) == target_sum:
                p1 = frozenset(c)
                p2 = frozenset(s - p1)
                valid_partitions.append(frozenset((p1,p2)))
    return len(set(valid_partitions))

You could just consider only p1 without bothering about p2. Partitions would be counted twice but you can always divide the result at the end. Once you start this, you realise that all the logic about removing duplicates can be removed (all generation combinations are unique): you do not need sets and you do not need frozensets:

def f(n):
    s = set([i+1 for i in range(n)])
    target_sum = sum(s) / 2
    valid_partitions = []
    for i in range(1,n):
        for c in combinations(s,i):
            if sum(c) == target_sum:
                valid_partitions.append(c)
    return len(valid_partitions) // 2

Then, you do not need the have all partitions in a container, a simple counter will do the trick:

def f(n):
    s = set([i+1 for i in range(n)])
    target_sum = sum(s) / 2
    nb_partitions = 0
    for i in range(1,n):
        for c in combinations(s,i):
            if sum(c) == target_sum:
                nb_partitions += 1
    return nb_partitions // 2

Then, if you want to make things more concise, you could use the sum builtin and use the fact that True is 1 and False is 0 to write something like:

def f(n):
    s = set([i+1 for i in range(n)])
    target_sum = sum(s) / 2
    return sum(sum(c) == target_sum for i in range(1, n) for c in combinations(s, i)) // 2

Final optimisation

If you look at the value returned for the first 19 values, you get something like:

0 0
1 0
2 0
3 1
4 1
5 0
6 0
7 4
8 7
9 0
10 0
11 35
12 62
13 0
14 0
15 361
16 657
17 0
18 0
19 4110

We can see that the functions return 0 in 1 case out of 2. This corresponds to the fact that we cannot partition an odd sum. This can be taken into account to avoid computing combinations when this happens:

def f(n):
    s = set([i+1 for i in range(n)])
    target_sum, rem = divmod(sum(s), 2)
    if rem:
        return 0
    return sum(sum(c) == target_sum for i in range(1, n) for c in combinations(s, i)) // 2

Last detail

Your first call to set is useless because the elements are unique already and you just need an iterable. Also, you could get rid of the list comprehension by tweaking the parameters given to range: s = range(1, n+1).


Query using cross join or inner join Please consider the following three tables that I am playing with in the image below:

I want to come up with a SQL Query which shows first_name, last_name, and the total amount of all orders for customer_id 12345

Here is what I have come up with:

SELECT C.first_name , C.last_name,
       SUM(O.total_price) AS TotalAmount
FROM   Customer C , Order O
WHERE  C.customer_id = O.customer_id
AND    C.customer_id = 12345;

Does my query look good? If yes, then should I be using Inner Join instead of Cross Join from an efficiency point of view?

Your query looks OK, for the most part. There are two things you could do better.

C and O don't make for very good aliases. Picture if you had 20 tables instead of just 2. In programming it's better to give aliases, variables, etc. meaningful names. Aliases are useful for shortening long table names, but should not be short to the point it obfuscates your code. In your case something like Cust and Ord would be better.

You are using old-style join syntax which is deprecated and should be avoided. In your case, inner join would work good, so instead of this:

FROM   Customer C , Order O
WHERE  C.customer_id = O.customer_id
AND    C.customer_id = 12345;

You should do this:
FROM   Customer C
INNER JOIN Order O
  ON C.customer_id = O.customer_id
WHERE C.customer_id = 12345;

Cross join
Chances are, if you think you need a cross join, you probably don't need a cross join. First, they are insanely slow. Second, the cross join gives you a Cartesian product which is almost never what you want, and in your case your results would be way off. (Try it!)
Here is some more information about cross join in SQL, for further reading.


Check column values are in different sets I have a data frame called data with lots of columns, and Country is one of them.

I created a new logical column called train, which is true when the row belongs to the train set and false otherwise.  The data frame is already sampled.

Now I want to be sure that there are no countries in both train and test sets with this code:

nrow( data[ data[ data$train, ]$Country %in% data[! data$train, ]$Country, ] ) == 0

which evaluates to true, so I guess I sampled correctly.  But that line makes my eyes bleed.

I would do:

with(data, length(intersect(Country[train], Country[!train])) == 0)


The efficiency of looping over nested dictionaries in Python How to make the following code more efficient?

"""
Calculate the posterior distribution of p(pos|word) using Baye's rule:
p(pos|word) \propto p(word|pos)p(pos).
"""
word_pos_dict = defaultdict(float)  # Example, word|pos -> 0.0012
pos_dict = defaultdict(float)       # pos -> 0.005
pos_word_dict = defaultdict(float)  # pos|word -> 0.017

for word_pos, word_pos_prob in word_pos_dict.items():
    word, pos = word_pos.split('|')
    marginal_prob = 0
    for pos_prob in pos_dict.values():
        marginal_prob += word_pos_prob * pos_prob    # Marginal prob of p(word)
    pos_word_prob = word_pos_prob * pos_dict[pos]
    pos_word = pos + '|' + word
    pos_word_dict[pos_word] = pos_word_prob / marginal_prob

In practice, the length of word_pos_dict is 57,602, and pos_dict has 984 elements, which make this calculation much slower. Is there something wrong with the implementation, design or algorithm?

One obvious improvement:

for word_pos, word_pos_prob in word_pos_dict.items():
    marginal_prob = 0
    for pos_prob in pos_dict.values():
        marginal_prob += word_pos_prob * pos_prob

Simplifies to:

multiplier = 0
for pos_prob in pos_dict.values():
    multiplier += pos_prob
# or just 'multiplier = sum(pos_dict.values())'

for word_pos, word_pos_prob in word_pos_dict.items():
    marginal_prob = word_pos_prob * multiplier

Now the inner loop only runs once.


UNIX cp multithreaded implementation in C I recently finished a high school project of mine for a class revolving around creating my own multithreaded implementation of the "cp" terminal command in UNIX systems.
I am seeking for help by experienced people in C to help me make my code better, offering constructive criticism all while improving myself at the same time.
Specifically:

Can my code become faster than it currently is?
Are there any potential runtime errors in my code? ( I have tested it with multiple examples and all would work fine, I'm just trying to find the ones I didn't think of)
Does my code have a good overall structure?

Points of interest:
src/crawler.c
#include "../headers/crawler.h"

int dircrawler(char* path, char* dest, int pathsize)
{
   // dirent variables and structures
   DIR* dp;
   struct dirent* entry;
   // buffers
   size_t bufsize;
   char *buf1, *buf2;
   buf1 = buf2 = NULL;
   struct cp_thr_args* args = NULL;

   if ((dp = opendir(path)) != NULL) {
      while ((entry = readdir(dp)) != NULL) {
         // skip over . and ..
         if (!(strcmp(entry->d_name, ".")) || !(strcmp(entry->d_name, "..")))
            continue;

         if (buf1 != NULL)
            free(buf1);
         if (buf2 != NULL)
            free(buf2);

         // printf("%s\n",entry->d_name);

         // buf1: absolute new path, buf2: relative destination path
         bufsize = snprintf(NULL, 0, "%s/%s", path, entry->d_name) + 1;
         mmalloc(&buf1, bufsize); // buf1
         snprintf(buf1, bufsize, "%s/%s", path, entry->d_name);

         bufsize = snprintf(NULL, 0, "%s%s/%s", dest, (path + pathsize - 1), entry->d_name) + 1;
         mmalloc(&buf2, bufsize); // buf2
         snprintf(buf2, bufsize, "%s%s/%s", dest, (path + pathsize), entry->d_name);

         if (entry->d_type == DT_DIR) {
            // make new directory in dest
            mkdir(buf2, 0755);
            if (dircrawler(buf1, dest, pathsize))
               closedir(dp);

         } else {
            // thread copying section
            int found = 1;
            while (1) { // ((Point of interest))
               // trying to find a non occupied thread
               // using status[]
               for (int i = 0; i < THREADCOUNT; ++i) {
                  if (status[i] == 1) {
                     cp_thr_init(&args, buf1, buf2, &status[i]);
                     pthread_create(&cpthr[i], NULL, cp_thr, (void*)args);
                     pthread_barrier_wait(&can_free);
                     cp_thr_free(args);
                     found = 0;
                     break;
                  }
               }
               if (!found)
                  break;
            }
         }
      }
      if (buf1 != NULL)
         free(buf1);
      if (buf2 != NULL)
         free(buf2);
      closedir(dp);
      return 0;
   }
}

src/threads.c
#include "../headers/threads.h"

int status[THREADCOUNT] = { 1, 1, 1, 1 };
pthread_mutex_t cp_lock = PTHREAD_MUTEX_INITIALIZER;

void* cp_thr(void* args)
{
   // extract args data before it gets rewritten
   struct cp_thr_args* temp = args;
   char* from;
   mmalloc(&from, return_size(temp->from) + 1);
   char* dest;
   mmalloc(&dest, return_size(temp->dest) + 1);
   int* status = temp->status;
   strcpy(from, temp->from);
   strcpy(dest, temp->dest);
   // send signal to release barrier and free args from crawl
   pthread_barrier_wait(&can_free); // ((Point of interest))

   // assign status to 0 to mark thread as occupied
   pthread_mutex_lock(&cp_lock);
   *status = 0;
   pthread_mutex_unlock(&cp_lock);

   copy_file(from, dest);

   free(from);
   free(dest);

   // assign status to 1 to mark thread as free to use
   pthread_mutex_lock(&cp_lock);
   *status = 1;
   pthread_mutex_unlock(&cp_lock);

   return (void*)0;
}

Use ftw() to crawl directories
ftw() is a POSIX function that will recursively crawl directories, and calls a callback function for every directory entry found.
Consider using strdup() and asprintf()
Instead of copying strings using some kind of strlen()+malloc()+strcpy(), use the POSIX function strdup() if possible, which takes care of all this for you.
Linux and most BSDs also implement asprintf(), which takes care of allocating a correctly sized buffer for you. Be aware that this is definitely not cross-platform.
Don't throw away threads
While thread creation and destruction is quite fast on Linux, it still has an overhead. If you are copying many small files, that overhead might start dominating. It would be much better to have a fixed number of threads that are waiting for work to do.
A typical implementation would use a thread-safe queue for this. Have the directory crawler fill a queue of filenames to copy. The queue is guarded by a mutex, and has an associated condition variable. Whenever it pushes an item to the queue, it signals the condition variable. The threads wait for elements to be pushed, and when woken up, they will pop one entry from the queue and start copying it.
However, assuming the directory crawler is much faster than files can be copied, it would be a waste of memory to fill a queue with all filenames encountered, especially if the directory tree is large. You only need a few entries in the queue for worker threads to pick up work. You can make the crawler block itself if the queue gets too large. You should add a second condition variable then that gets signalled whenever a worker dequeues a filename to copy.
Find the right number of threads to use
It's hard to predict how many threads will be optimal. It could be that one thread is optimal, depending on how fast the storage is compared to the overhead of having multiple threads. Having as many threads as files to copy might also not be ideal; even if they don't use much CPU time, performing I/O on multiple files at the same time might result in more disk seeks (which is mostly an issue on harddisks, but some solid state drives also have some performance penalty for doing non-sequential access). You could benchmark your code and optimize the number of threads for your system, but the performance might be different on other systems.
Alternatives for threads
Threads are a solution to the problem that there is some implicit serialization when doing regular I/O operations on files in a single thread. However, there are also other techniques to have multiple I/O operations submitted to the kernel simultaneously. POSIX AIO is one way to do this, however it has its limitations. In particular, I don't think you can easily hand off a file copy operation to it. What is more promising is Linux's io_uring, although this is of course not a cross-platform solution to your problem. If you are interested in the latter, then also see this LWN article and this excellent guide. Most BSD flavors have something similar called kqueue.


Implementations and analysis of my python login system (~80 lines) I wrote this login system in the evening and i would like to know what should I fix / change, if should I use classes and, about readability, if this code is OK.
I'm a begginer in coding so made this small project only for exercising, but good to know wether if i'm things doing right or wrong.
import os.path

credentials = []
database_path = "C:\\Users\\filip\\PycharmProjects\\database"
database = os.listdir(database_path)

def initialize():
    choose_option = int(input('0. Exit\n1. Login\n2. Sign up\n\nOption: '))
    if choose_option == 1:
        login_func()
    elif choose_option == 2:
        get_username(), get_password(), add_credentials_to_database()
    elif choose_option == 0:
        exit('Exiting...')
    else:
        print("You should enter '0', '1', or '2'.")
        initialize()

def get_username():
    username = input('Enter username: ')
    if f'{username}.txt' not in database:
        if len(username) >= 8:
            credentials.append(username)
        else:
            print('Username too short (min 8 characters)\n\n')
            get_username()
    else:
        print('Username already exists')
        get_username()

def get_password():
    password = input('Enter password: ')
    if len(password) >= 8:
        confirm_password = input('Confirm password: ')
        if password == confirm_password:
            credentials.append(password)
        else:
            print('Password does not match with confirmation\n\n')
            get_password()
    else:
        print('Password too short (min 8 characters)\n\n')
        get_password()

def add_credentials_to_database():
    credentials_file = os.path.join(database_path, f'{credentials[0]}.txt')
    file = open(credentials_file, 'w')
    file.write(credentials[1])
    file.close()
    print('Account created successfully\n\n')
    initialize()

def login_func():
    database = os.listdir(database_path) #Refreshes the database, so if you create an account you can login without having to run the program again
    enter_login = input('Enter username: ')
    if f'{enter_login}.txt' in database:
        for file in database:
            if f'{enter_login}.txt' == file:
                enter_password = input('Enter password: ')
                file = open(f"C:\\Users\\filip\\PycharmProjects\\database\\{file}", 'r')
                if enter_password == file.read():
                    print('WELCOME!')
                else:
                    print('Invalid password\n\n')
                    login_func()
    else:
        print('Invalid username\n\n')
        login_func()

if __name__ == '__main__':
    initialize()

Also, if you remember, please tell me in the comments simple projects that you did when you were learning, so I can do it too.
ps: I know this login system is not secure at all but maybe later I could do some encryptation on those credencials...

When entering passwords, you may want to opt for using the getpass library. It allows you (if the console supports it) to hide passwords as they're being typed. You'd use getpass.getpass just as you are using input to ask for the password. The only difference is, the password won't show in the console as it's typed.

You're using recursion in get_password in order to ask the user again. This isn't a good idea. If the user fails too many times, your program can crash. Just use a loop:
def get_password():
    while True:
        password = input('Enter password: ')
        if len(password) >= 8:
            confirm_password = input('Confirm password: ')
            if password == confirm_password:
                credentials.append(password)
                return
            else:
                print('Password does not match with confirmation\n\n')
        else:
            print('Password too short (min 8 characters)\n\n')

I'm looping until correct input is entered, then I'm just returning from within the infinite loop.
And the same goes for the rest of your functions. Recursion is great when used properly, but it carries more problems than most other looping methods. Use it in cases where it's appropriate (like when the problem itself is recursive), not just for general looping.

add_credentials would be safer if it used a with block to close the file for you:
def add_credentials_to_database():
    credentials_file = os.path.join(database_path, f'{credentials[0]}.txt')
    with open(credentials_file, 'w') as file:
        file.write(credentials[1])
    print('Account created successfully\n\n')
    initialize()

Now, even if an error were to happen half-way through the function, there's a greater chance that the file would be closed. Making a practice of using with also prevents you from forgetting to add a call to file.close().

At the top you have:
credentials = []
database_path = "C:\\Users\\filip\\PycharmProjects\\database"
database = os.listdir(database_path)

A few things to note:

credentials shouldn't be a global here. Global mutable states (which credentials is) tend to complicate code and make it harder to understand. In this set up, I would probably make credentials a local variable of initialize, have get_password and the like to return the data instead of adding it to the list directly, and manually pass that data to add_credentials_to_database.

database_path is a module-level constant, and as such should be in upper-case like DATABASE_PATH according to PEP8.

Arguably database is also (effectively) constant, so it should also be in upper-case.

In initialize, I would get rid of the int conversion of choose_option. All the call to int is doing is allowing for a ValueError to be raised if the user enters in a dumb menu option like "a". Just use the raw string value and compare against it:
choose_option = input('0. Exit\n1. Login\n2. Sign up\n\nOption: ')
if choose_option == "1":
    . . .

If you actually needed it as an integer to do math on it or something, it would be different. It's simply an "indicator value" though, so the type of choose_option doesn't really matter, and keeping it as a string has less problems.

And my favorite projects that I still do whenever I learn a new language are:

A naïve, bruteforce prime number finder (and then maybe a sieve). Fairly basic.
Conway's Game of Life. Harder, and has the potential to turn into a large project.
A Mandelbrot Set image generator. Even harder, and usually becomes a larger project.


Retrieve generic contract names for Managed Extensibility Framework Edit
Found the original contract name resolution by .NET MEF, the code below is not useful anymore. See response!

My issue was to use existing code, to make a MEF ExportFactory, imported via [Import] attribute to a property, return instances of Moq mocks (for unit testing). Problem was, that the original code always used the Type.FullName property value as contract name. This worked fine for non-generic types, but not for generics.

Generic FullName is like:

My.Assembly.GenericType`1[My.Assembly.InnerType]

but MEF expects a name like:

My.Assembly.GenericType(My.Assembly.InnerType)

The complete unit test code actually extends the MEF ExportProvider class, to register ExportDefinition instances for an ExportFactory and it's ProductDefinition. The contract name goes to a metadata dictionary, using the key CompositionConstants.ExportTypeIdentityMetadataName.

This following code is only for the contract name conversion. It calls itself recursively, for generic arguments which are generic themselves. The regular expression matches by the `[digits] name extension, used by .NET for generic types. It would be nice to see a generally better way, or already existing functions from MEF, if such exists.

Edit: I added the incrementor and code for open (unspecified) generics, which have names with indexed curly brackets: instead of IMyType<string,int> IMyType<,>, resolved to contract name IMyType({0},{1}). Regex has also been changed to match, if no type names come after `[digits].

private static readonly Regex GenericRegex =
    new Regex(@"^(?<fullNameNonGeneric>\S+?)`\d+(\[.*\])*$",
        RegexOptions.Compiled);

public static string ResolveGenericMefContractName(this Type type)
{
    return ResolveGenericMefContractName(type, new IndexIncrementor());
}

private static string ResolveGenericMefContractName(Type type, IndexIncrementor incrementor)
{
    var fullName = type?.FullName;
    if (fullName == null)
    {
        return null;
    }
    var match = GenericRegex.Match(fullName);
    if (!match.Success)
    {
        return type.FullName;
    }

    var fullNameNonGeneric = match.Groups["fullNameNonGeneric"].Value;
    var genericArgs = type.GetGenericArguments();

    return fullNameNonGeneric + "(" +
        string.Join(",", genericArgs.Select(
            ga => ResolveGenericMefContractName(ga, incrementor)
            ?? "{" + incrementor.GetAndIncrementValue() + "}"))
        + ")";
}

private class IndexIncrementor
{
    private int _value;

    public int GetAndIncrementValue() => _value++;
}

The code in the question was an attempt to generate type contract names, from the way I saw how MEF created them. Especially the difference from Type.FullName, which was previously used, when getting a contract name. For this reason, the code is a probably inferior re-implementation of MEF's own name generation, and should not be used for this purpose.

This is the contract name resolution used by MEF in .NET framework (maybe different namespace in Core, Apps etc.):

Namespace:

System.ComponentModel.Composition

Static method:

AttributedModelServices.GetContractName(Type)

Unfortunately, it's often easier to re-invent the wheel, than to find out about existing implementations...


Improving a JavaScript filtering function I created a function that compares an array of letters to the alphabet, and returns the letters that are not in the original array.

While this works, it looks somewhat clumsy to me, but I don't know what I'm looking for to improve the code...Is there a way to make it more elegant? Perhaps getting rid of the notPresent array somehow?

getLettersNotInContent(['z', 'a', 'p']);

function getLettersNotInContent(letters) {
  // Filter dups if present
  var uniques = [];
  letters.filter(function(letter) {
    if (uniques.indexOf(letter) === -1) {
      uniques.push(letter);
    }
  });

  // Filter and return the letters that are not present in the content
  var alphabet = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o',
  'p','q','r','s','t','u','v','w','x','y','z'];
  var notPresent = [];
  alphabet.filter(function(letter) {
    if (uniques.indexOf(letter) === -1) notPresent.push(letter);
  });
  return notPresent;
}

You're mixing up two different ways of doing this. The old-fashioned way involves looping through the alphabet and pushing the letters not present into an array:

function getLettersNotInContent2(letters) {
    var alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
                    'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
    var notPresent = [];
    for (var i = 0; i < alphabet.length; i++) {
        if (letters.indexOf(alphabet[i]) === -1) notPresent.push(alphabet[i]);
    }
    return notPresent;
}

The newer way (more elegant but not supported in IE8) uses the filter method with a function that returns true or false:

function getLettersNotInContent3(letters) {
    var alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
                    'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
    return alphabet.filter(function (letter) {
        return letters.indexOf(letter) === -1
    });
}

Your code happens to work because, under the hood, filter also loops through the values of an array in turn. But you're not using it in the intended way.

(In both cases I removed the filtering out of duplicates as this seems unnecessary.) (jsfiddle)


Input string, output vowels and consonants to log, separately but in order I completed a short coding challenge and just want to know if I should use shorter methods of which I am unaware, or make it more readable.

 /*
 *
 * Complete the vowelsAndConsonants function.
 * Print your output using 'console.log()'.
 */
function vowelsAndConsonants(s) {
var strConsonants = "";
var strVowels = "";
var i;

for (i in s) {
    if (s.charAt(i) == "a" || s.charAt(i) == "e" || s.charAt(i) == "i" ||
s.charAt(i) == "o" || s.charAt(i) == "u") {
        strVowels += s.charAt(i);
    }
    else if (s.charAt(i) != "a" || s.charAt(i) != "e" || s.charAt(i) != "i"
|| s.charAt(i) != "o" || s.charAt(i) != "u") {
        strConsonants += s.charAt(i);
    }
}

// console.log(strVowels);

i = 0;
for (i in strVowels) {
    console.log(strVowels.charAt(i));
}

// console.log(strConsonants);

i = 0;
for (i in strConsonants) {
     console.log(strConsonants.charAt(i));
}

}

Instead of providing you a "better" solution, I will try to give you some feedback the code.

Block indentation

  function vowelsAndConsonants(s) {
var strConsonants = "";
var strVowels = "";
var i;
// ..

To work with code it is important that it is formatted in a readable way. There are some style guides out there for example from google

function vowelsAndConsonants(s) {
  var strConsonants = "";
  var strVowels = "";
  var i;
  // ..

Type Embedded in Name

var strConsonants = "";
var strVowels = "";

In this variable names the type is embedded in the name

  Avoid placing types in method names; it's not only redundant, but it forces you to change the name if the type changes.

Array iteration and for...in

For your task is the order important:

  Input string, output vowels and consonants to log, separately but in order

You can read on MDN about the for..in loop,that it do not guaranties a traversal in order:

  Note: for...in should not be used to iterate over an Array where the index order is important. [...]

  [...] iterating over an array may not visit elements in a consistent order. Therefore, it is better to use a for loop with a numeric index (or Array.prototype.forEach() or the for...of loop) when iterating over arrays where the order of access is important.

Make the Else-Statement Implicit

  if (s.charAt(i) == "a" ||
    s.charAt(i) == "e" ||
    s.charAt(i) == "i" ||
    s.charAt(i) == "o" ||
    s.charAt(i) == "u") {
  strVowels += s.charAt(i);
} else if (s.charAt(i) != "a" ||
           s.charAt(i) != "e" ||
           s.charAt(i) != "i" ||
           s.charAt(i) != "o" ||
           s.charAt(i) != "u") {
  strConsonants += s.charAt(i);
}

Currently the if-else statement tries to express: If you are a, e, i, o, u do something, else if you are not from a, e, i, o, u do something.

This is semanticly the same as: If you are a, e, i, o, u do something, else do something.

Additional we can wrap the condition of the vowels into its own method to make the code more readable.

function isVowel(letter) {
    return letter === "a" ||
           letter === "e" ||
           letter === "i" ||
           letter === "o" ||
           letter === "u"
}

The if-statement could now look like

var letter = s.charAt(i)
if (isVowel(letter)) {
  strVowels += letter;
} else {
  strConsonants += letter;
}

String Concatenation..

  strVowels += letter

  Every time Strings get merged by + a new String gets created, because Strings are immutable, that means that for each concatenation new memory space gets allocated.

Better would be to use an array instead of a string an push into it.

vowels.push(letter)

Example Refactoring

function isVowel(letter) {
    return letter === "a" ||
           letter === "e" ||
           letter === "i" ||
           letter === "o" ||
           letter === "u"
}

function vowelsAndConsonants(s) {
    var consonants = [];
    var vowels = [];

    for (var letter of s) {
        if (isVowel(letter)) {
            vowels.push(letter)
        } else {
            consonants.push(letter)
        }
    }

    for (var vowel of vowels) {
        console.log(vowel);
    }

    for (var constant of consonants) {
        console.log(constant);
    }
}

and from here you can still use some methods like forEach or using the ternary operator to shorten an if-else

function isVowel(letter) {
    return letter === "a" ||
           letter === "e" ||
           letter === "i" ||
           letter === "o" ||
           letter === "u"
}

function print(x) {
    console.log(x)
}

function vowelsAndConsonants(s) {
    var consonants = [];
    var vowels = [];

    for (var letter of s) {
        isVowel(letter)
            ? vowels.push(letter)
            : consonants.push(letter)
    }

    vowels.forEach(print)
    consonants.forEach(print)
}