Implementing factory design pattern with generics In this new project I'm working on I need to create objects on runtime by data from the DB, right now I have two groups of classes, each group implementing a different interface.

I started working on a factory class, which will map id to a type, an abstract one so I can extend with a factory for each group.

The constructor parameter is the type of the interface common to all implementors of the group.

abstract class Factory
   {
    private Dictionary<int, Type> idsToTypes;
    private Type type;

    public Factory(Type _type)
    {
        idsToTypes = new Dictionary<int, Type>();
        type = _type;
    }

    protected Object GetProduct(int id, params object[] args)
    {
        if (idsToTypes.ContainsKey(id))
        {
            return Activator.CreateInstance(idsToTypes[id], args);
        }
        else
        {
            return null;
        }
    }

    public void RegisterProductType(int id, Type _type)
    {
        if (_type.IsInterface || _type.IsAbstract)
            throw new ArgumentException(String.Format("Registered type, {0}, is interface or abstract and cannot be registered",_type));

        if (type.IsAssignableFrom(_type))
        {
            idsToTypes.Add(id, _type);
        }
        else
        {
            throw new ArgumentException(String.Format("Registered type, {0}, was not assignable from {1}",_type,type));
        }

    }

Then I noticed both extending factories look the same, and in the end with the use of generics I got to this class:

 class GenericSingletonFactory<T> : Factory
    {
        static public GenericSingletonFactory<T> Instance = new GenericSingletonFactory<T>();

    private GenericSingletonFactory()
        : base(typeof(T)) { }

    public T GetObject(int id, params object[] args)
    {
        Object obj = base.GetProduct(id, args);
        if (obj != null)
        {
            T product = (T)obj;
            return product;
        }
        else return default(T);
    }
}

So I can just use like so:

GenericSingletonFactory<ISomething>.Instance.RegisterProductType(1, typeof(ImplementingISomething));
ISomething something = GenericSingletonFactory<ISomething>.Instance.GetObject(1);

It seems ok... but am I missing something here? is this a good way to do this kind of things?
I'm a little weary that this will fall apart on runtime somehow...

Several considerations:

Base Factory class does all the work, so I would simply merge it into the generic method.
Your singleton instance is stored in a public field. This means any part of your program can easily replace it. You should make it readonly, or (even better) expose it through a readonly property.
If you are not abstracting the factory (i.e. exposing the Instance property as an interface), it means there is no way to use any other factory in your program. This has two possible consequences:

you can either skip accessing Instance every time and use a plain static method (Factory<T>.Instance.DoStuff(...) becomes Factory<T>.DoStuff(...), which is slightly shorter)
or, you can decide that you want to hide the implementation of the factory behind an interface (IFactory<T>, for example), in which case a concrete factory will be stored in an instance of a separate class, which would completely change the call to something like DAL.GetFactory<T>().DoStuff(...) (DoStuff becomes an instance method). This leaves allows greater flexibility, since you can pass an IFactory<T> to a part of code which doesn't need to know about your static DAL classes. But it would require additional redesign.

If you add a new generic parameter to the RegisterProductType method, you can use the where clause to limit the type to derived types at compile time. Getting a compile error is much better than getting a run-time one.
Are you sure that you need to pass the constructor parameters to the GetProduct method? This part might be slightly unusual (although there might be cases where you would want to instantiate your classes by passing a parameter). What it there is a specific derived type which accepts different parameters than other types? Activator.CreateInstance indicates that you are forcing all your callers to know which constructor your method will use.

First four points (simplified) would result in something like:

public class Factory<T>
{
    private Factory() { }

    static readonly Dictionary<int, Type> _dict = new Dictionary<int, Type>();

    public static T Create(int id, params object[] args)
    {
        Type type = null;
        if (_dict.TryGetValue(id, out type))
            return (T)Activator.CreateInstance(type, args);

        throw new ArgumentException("No type registered for this id");
    }

    public static void Register<Tderived>(int id) where Tderived : T
    {
        var type = typeof(Tderived);

        if (type.IsInterface || type.IsAbstract)
            throw new ArgumentException("...");

        _dict.Add(id, type);
    }
}

Usage:

Factory<IAnimal>.Register<Dog>(1);
Factory<IAnimal>.Register<Cat>(2);

// this is the "suspicious" part.
// some instances may require different parameters?
Factory<IAnimal>.Create(2, "Garfield");

Fifth point is worth considering. If you change your factory to this:

public class Factory<T>
{
    private Factory() { }

    static readonly Dictionary<int, Func<T>> _dict
         = new Dictionary<int, Func<T>>();

    public static T Create(int id)
    {
        Func<T> constructor = null;
        if (_dict.TryGetValue(id, out constructor))
            return constructor();

        throw new ArgumentException("No type registered for this id");
    }

    public static void Register(int id, Func<T> ctor)
    {
        _dict.Add(id, ctor);
    }
}

Then you can use it like this:

Factory<IAnimal>.Register(1, () => new Dog("Fido"));
Factory<IAnimal>.Register(2, () => new Cat("Garfield", something_else));

// no need to pass parameters now:
IAnimal animal = Factory<IAnimal>.Create(1);

This delegates all construction logic to init time which allows complex scenarios for different object types. You can, for example, make it return a singleton on each call to Create:

// each call to Factory<IAnimal>.Create(3) should return the same monkey
Factory<IAnimal>.Register(3, () => Monkey.Instance);


Base64 encoder and decoder I'd like to have some feedback on this small snippet I wrote to implement a base64 encoder and decoder.

Specifically, I'm not sure I'm handling padding in the best way possible.

import string

values = string.ascii_uppercase + string.ascii_lowercase + string.digits + '+/'
ALPHABET = dict(zip(range(64), values))

def encode(unpadded_buf):
    padded_buf = unpadded_buf[:]
    if len(padded_buf) % 3 != 0:
        padded_buf += [0] * (3 - len(unpadded_buf) % 3)

    encoded_padded_buf = []

    for i in range(0, len(unpadded_buf), 3):
        encoded_padded_buf += [ padded_buf[i] >> 2 ]
        encoded_padded_buf += [ (padded_buf[i] & 3) << 4 | padded_buf[i + 1] >> 4 ]
        encoded_padded_buf += [ (padded_buf[i + 1] & 15) << 2 | padded_buf[i + 2] >> 6 ]
        encoded_padded_buf += [ padded_buf[i + 2] & 63]

    asciied = ''.join(map(lambda n: ALPHABET[n], encoded_padded_buf))

    if len(padded_buf) - len(unpadded_buf) == 2:
        return asciied[:-2] + '=='
    elif len(padded_buf) - len(unpadded_buf) == 1:
        return asciied[:-1] + '='

    return asciied[:]

def decode(asciied):
    _alphabet = dict(map(lambda p: (p[1], p[0]), ALPHABET.items()))
    encoded_buf = list(map(lambda c: _alphabet.get(c, 0), asciied))

    decoded_buf = []

    for i in range(0, len(encoded_buf), 4):
        decoded_buf += [ encoded_buf[i] << 2 | encoded_buf[i + 1] >> 4 ]
        decoded_buf += [ (encoded_buf[i + 1] & 15) << 4 | encoded_buf[i + 2] >> 2 ]
        decoded_buf += [ (encoded_buf[i + 2] & 3) << 6 | encoded_buf[i + 3] ]

    return decoded_buf

if __name__ == '__main__':
    assert bytes(decode(encode(list('foo'.encode())))).decode() == 'foo'

Interface

assert bytes(decode(encode(list('foo'.encode())))).decode() == 'foo'

To avoid using encode on strings or decode on bytes, you can use byte literals: b'foo'. This simplifies the writting to:

assert bytes(decode(encode(list(b'foo')))) == b'foo'

But then this let me wonder, why do the caller have to convert the arguments themselves? Why the need for list and bytes. A proper usage would be

assert decode(encode(b'foo')) == b'foo'

Which might let someone wonder decode to/encode from what? But assuming the module is properly named, that should not be an issue.

Now onto building functions that accept/returns bytes

Iteration and padding

When it comes to iterating over something in Python, it is deemed inelegant (and somewhat slower) to use indices rather than directly accessing content. Meaning for element in array: is way better than for i in range(len(array)): element = array[i]. Here you can argue that you can't easily iterate over groups of 3 or 4 elements at a time, but its forgetting about the itertools module and its variety of recipes.

grouper is the one you need:

from itertools import zip_longest

def grouper(iterable, n, fillvalue=0):
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

There is thus no need to manually handle the padding in encode and you can simplify iteration:

for first, second, third in grouper(unpadded_buf, 3):
    encoded_padded_buf += [ first >> 2 ]
    encoded_padded_buf += [ (first & 3) << 4 | second >> 4 ]
    encoded_padded_buf += [ (second & 15) << 2 | third >> 6 ]
    encoded_padded_buf += [ third & 63]

It also has the advantage to handle any iterable the same way. Meaning you don't need to convert your bytes to list beforehand, it will do just fine.

Extra characters

You don't need to differentiate the case where len(unpadded_buf) % 3 is 1 or 2. You can perform the same task using:

extra = len(unpadded_buf) % 3
if extra:
    return asciied[-extra:] + '='*extra
return asciied

You can also note that there is no reason to make a copy of asciied, like you don't make a copy of decoded_buf in decode.

ALPHABET?

It feels like this variable is misnamed, it is not an alphabet, it is a convertion table TO_BASE64. Same for _alphabet, I would have named it FROM_BASE64 and computed it once, as a constant, as its content would never be different.

I would also incorporate {'=': 0} to simplify the map call.

So far, the code could be:

import string
from itertools import zip_longest

TO_BASE64 = dict(zip(range(64),
                     string.ascii_uppercase + string.ascii_lowercase +
                     string.digits + '+/'))
FROM_BASE64 = {v: k for k, v in TO_BASE64.items()}
FROM_BASE64['='] = 0

def grouper(iterable, n, fillvalue=0):
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

def encode(unpadded_buf):
    encoded_buf = []
    for first, second, third in grouper(unpadded_buf, 3):
        encoded_buf += [ first >> 2 ]
        encoded_buf += [ (first & 3) << 4 | second >> 4 ]
        encoded_buf += [ (second & 15) << 2 | third >> 6 ]
        encoded_buf += [ third & 63]

    asciied = ''.join(TO_BASE64[n] for n in encoded_buf)
    extra = len(unpadded_buf) % 3
    if extra:
        return asciied[-extra:] + '='*extra

    return asciied

def decode(asciied):
    encoded_buf = (FROM_BASE64[n] for n in asciied)
    decoded_buf = []
    for first, second, third, fourth in grouper(encoded_buf, 4):
        decoded_buf += [ first << 2 | second >> 4 ]
        decoded_buf += [ (second & 15) << 4 | third >> 2 ]
        decoded_buf += [ (third & 3) << 6 | fourth ]

    return bytes(decoded_buf)

if __name__ == '__main__':
    assert decode(encode(b'foo')) == b'foo'

Adding elements to a list

You extend your "result" lists using other lists. It is rather inefficient as the list you use to extend the first one will be discarded right away. You’d be better of appending values to the "result" instead. Or use helper generator functions to build results as they are computed without having to populate a list that will be discarded right after:

import string
from itertools import zip_longest

TO_BASE64 = dict(zip(range(64),
                     string.ascii_uppercase + string.ascii_lowercase +
                     string.digits + '+/'))
FROM_BASE64 = {v: k for k, v in TO_BASE64.items()}
FROM_BASE64['='] = 0

def grouper(iterable, n, fillvalue=0):
    "Collect data into fixed-length chunks or blocks"
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

def _encoder(stream):
    for first, second, third in grouper(stream, 3):
        yield first >> 2
        yield (first & 3) << 4 | second >> 4
        yield (second & 15) << 2 | third >> 6
        yield third & 63

def encode(unpadded_buf):
    asciied = ''.join(TO_BASE64[n] for n in _encoder(unpadded_buf))
    extra = len(unpadded_buf) % 3
    if extra:
        return asciied[-extra:] + '='*extra

    return asciied

def _decoder(stream):
    for first, second, third, fourth in grouper(stream, 4):
        yield first << 2 | second >> 4
        yield (second & 15) << 4 | third >> 2
        yield (third & 3) << 6 | fourth

def decode(asciied):
    return bytes(_decoder(FROM_BASE64[n] for n in asciied))

if __name__ == '__main__':
    assert decode(encode(b'foo')) == b'foo'


Finding duplicate files on a disk using MD5 I started learning Python a month ago (used to write programs in Delphi before). Can you please take a look at my code and give me some tips what is good and what is bad in it?

The aim is to find duplicate files on a disk and saves this information. I'm using md5 hash. Is it right to use global variable like I did?

import os
import time
import hashlib

def md5_sum(path, block_size=256*128, hr=False):
    md5 = hashlib.md5()
    with open(path, 'rb') as f:
        for chunk in iter(lambda: f.read(block_size), b''):
            md5.update(chunk)

    if hr:
        return md5.hexdigest()
    return md5.digest()

def md5_simple(path, chunk):
    if chunk:
        return md5_sum(path, 8192)
    else:
        return hashlib.md5(open(path, 'rb').read()).digest()

def walking(path, chunk=False):
    for dirpath, dirnames, filenames in os.walk(path):
        for filename in filenames:
            global dupl_string
            try:
                if len(found_files) % 500 == 0:
                    print(len(found_files))

                fname = os.path.join(dirpath, filename)
                file_size = os.path.getsize(fname)
                found_files[fname] = file_size

                if chunk:
                    file_hash = md5_sum(fname, 8192)
                else:
                    file_hash = md5_simple(fname, file_size > 1000 * 1024 * 1024)

                if file_hash in hash_files:
                    dupl_string += 'Src: ' + hash_files[file_hash] + ', dupl: ' + fname + '\n'
                else:
                    hash_files[file_hash] = fname

            except OSError:
                print('OSError: ' + fname)

my_dir = "C:"
dupl_string = ''
found_files = dict()
hash_files = dict()

time1 = time.time()

try:
    walking(my_dir)
finally:
    if dupl_string:
        with open('dupl.txt', 'w') as f3:
            f3.write(dupl_string)

    with open('files.csv', 'w') as f:
        for key, value in sorted(found_files.items(), key=lambda item: item[1], reverse=True):
            f.write("%s, %s\n" % (key, value))

    print("Elapsed: " + str(time.time() - time1) + ' sec')

Formatting

The code formatting is good. The only major gripe I have is that there are some long lines (117 chars). Part of this is due to the high level of indenation, which we'll solve in another way, but even with heavily indented code, lines can be shorter. I use the black code formatter (with a linelength of 79) to format my code, so I don't have to worry about spacing, long lines or inconsistent quote characters any more

Try-except

You should surround the part that can fail (reading the filesize and file_hash) as closely as possible with the try-except clause. If you want to print a debug message and prevent the rest of that iteration to be executed, you can do that by adding a continue after the print

globals

as you question it, globals are not the correct way to pass information here. A different approach would be to have parts that

generate the paths to the files to check
return the hash
look for duplicates in the hashes
report the duplicates

Each part can receive the result of the previous step as input

generate the paths

Since python 3.4 there is a more convenient way to handle file paths: pathlib.Path

from pathlib import Path
def walking(path):
    return Path(path).glob("**/*")

This returns a generator that yields Paths to all of the files under path. Since it is so simple, it can be inlined.

hashing

def hashing(paths, chunk=False):
    for path in paths:
        filesize = path.stat().st_size
        if chunk:
            file_hash = md5_sum(fname, 8192)
        else:
            file_hash = md5_simple(fname, file_size > 1000 * 1024 * 1024)
        yield path, file_hash

magic numbers

This previous function has some magic numbers 8192 and 1000 * 1024 * 1024.
Better would be to define those a module level constants:

CHUNKSIZE_DEFAULT = 8192
MAX_SIZE_BEFORE_CHUNK = 1000 * 1024 * 1024 # files larger than this need to be hashed in chuncks

def hashing(paths, chunk=False):
    for path in paths:
        filesize = path.stat().st_size
        if chunk or file_size > MAX_SIZE_BEFORE_CHUNK:
            file_hash = md5_sum(fname, CHUNKSIZE_DEFAULT )
        else:
            file_hash = md5_simple(fname)
        yield path, file_hash

I'm not 100% happy with MAX_SIZE_BEFORE_CHUNK as variable name, but can't immediately think of a better on

hoist the OI

If you look at this talk by Brandon Rhodes, it makes sense not to open the file in the method that calculates the hash, but have it accept a filehandle. This is also the approach taken in this code review answer
So to reuse the slightly adapted code from this answer

def read_chunks(file_handle, chunk_size=CHUNKSIZE_DEFAULT):
    while True:
        data = file_handle.read(chunk_size)
        if not data:
            break
        yield data

def md5(file_handle, chunk_size=None, hex_representation=False):
    if chunk_size is None:
        hasher = hashlib.md5(file_handle.read())
    else:
        hasher = hashlib.md5()
        for chunk in read_chunks(file_handle, chunk_size):
            hasher.update(chunk)

    return hasher.digest() if not hex_representation else hasher.hexdigest()

This gets called like this:

def hashing(paths, chunk=False, hex_representation=False):
    for path in paths:
        file_size = path.stat().st_size
        with path.open("rb") as file_handle:
            chunk_size = (
                CHUNKSIZE_DEFAULT
                if chunk or file_size > MAX_SIZE_BEFORE_CHUNK
                else None
            )
            file_hash = md5(
                file_handle,
                chunk_size=chunk_size,
                hex_representation=hex_representation,
            )
        yield path, file_hash

looking for duplicates

Instead of keeping a long string with all the duplicates, you can keep a set of the Paths for each file_hash. A collections.defaultdict(set) is the suited container for this. You add each path to the set at the in the dict. At the end of the function, you filter the keys that have more than 1 entry:

def duplicates(hashings):
    duplicates = defaultdict(set)
    for path, file_hash in hashings:
        duplicates[file_hash].add(path)

    return {
        filehash: paths
        for filehash, paths in duplicates.items()
        if len(paths) > 1
    }

report the filesizes

instead of formating the csv-file yourself, you can use the csv module. It even has a writerows. Instead of defining the lambda function to srt by value, you can use operator.itemgetter

import csv
from operator import itemgetter

def report_results(file_handle, filesizes):
    writer = csv.writer(file_handle, delimiter=",", lineterminator="\n")
    sorted_sizes = sorted(filesizes.items(), key=itemgetter(1))
    writer.writerows(sorted_sizes)

This method can also be used to save the file_hashes

reporting the duplicates

def report_duplicates(file_handle, duplicates):
    writer = csv.writer(file_handle, delimiter=",", lineterminator="\n")
    for file_hash, duplicate_files in duplicates.items():
        for file_name in duplicate_files:
            writer.writerow((file_hash, str(file)))

Here I exported a csv file with the hash and filenames of the duplicates. If you want it in another format, you can easily change this method

bringing it together

def main(path, chunk=False):
    files = [file for file in Path(path).glob("**/*") if file.is_file()]

    filesizes = {str(path): path.stat().st_size for path in files}
    with open("test_filesizes.csv", "w") as file_handle:
        report_results(file_handle, filesizes)
    filehashes = dict(hashing(files, chunk=chunk))

    with open("test_hashes.csv", "w") as file_handle:
        report_results(file_handle, filehashes)

    duplicates = find_duplicates(filehashes.items())
    with open("test_duplicates.csv", "w") as file_handle:
        report_duplicates(file_handle, duplicates)

And then we put everything behind a __main__ guard:

if __name__ == "__main__":
    main(<path>)


Validation for registration password Could someone tell me if this is a bad idea for coding my validation for a password and it is bad? This is for registration for my site. Could you please tell me why and how you would go about it (I am a beginner and at PHP)?

$username = $_POST['username'];
$password = $_POST['password'];
$confirm_password = $_POST['confirm_password'];

$password_filter1 = '/(?=.*[a-zA-Z0-9]{6})/m';  //Password contains more then 6 char
$password_filter2 = '/(?=.*[A-Z])/m';           //Capital letter
$password_filter3 = '/(?=.*\d)/m';              //A digit

if (!empty($password & $confirm_password)) {
 if ($password === $confirm_password) {
  if (preg_match($password_filter1, $password)) {
   if (preg_match($password_filter2, $password)) {
    if (preg_match($password_filter3, $password)) {
     //If everything is true do this
    } else {
     $password_error = "Your password must contain at least 1 number";
    }
   } else {
    $password_error = "You're password must have at least 1 capital letter";
   }
  } else {
   $password_error = "You're password must be at least 6 characters long";
  }
 } else {
  $password_error = "The passwords do not match";
 }
} else {
 $password_error = "You must enter a password";
}

The only reason why I am not doing all the filters in 1 preg_match is because I would like specific errors.

First a stylistic note: you should always use at least 2 spaces of indentation for code; 1 space makes it very difficult to read nested statements in particular. PHP most often uses 4 spaces for indent.

Your regular expressions wouldn't need comments to explain them if you gave them meaningful names.

$password_filter1 = '/(?=.*[a-zA-Z0-9]{6})/m';  //Password contains more then 6 char
$password_filter2 = '/(?=.*[A-Z])/m';           //Capital letter
$password_filter3 = '/(?=.*\d)/m';              //A digit

There is also no need for m multiline flag for a password, since passwords shouldn't accept any newline characters.

Compare to:

$has_at_least_6_chars = '/(?=.*[a-zA-Z0-9]{6})/';
$has_a_capital_letter = '/(?=.*[A-Z])/';
$has_a_digit = '/(?=.*\d)/';

You may also consider making them into small functions so they are reusable elsewhere in the code, and if you needed to change them later you would just change them in one place.

function has_a_capital_letter($str) {
    return preg_match('/(?=.*[A-Z])/', $str);
}

//...

if (!has_a_capital_letter($password)) {
    $password_error = "Your password must have at least 1 capital letter";
}

There is no real value in nesting multiple if/else statements inside each other like this, when a simple sequence of if/elseif/else will work just as good. Nesting like that with even simple conditions makes the code quite hard to read, in fact. Nesting like this with more complex operations could lead to serious performance bottlenecks as it cannot break early as easily.

It's also simpler to test for the existence of conditions instead of the lack of them, and just break out of the conditionals as soon as a condition is met.

This is much simpler and cleaner:

if (empty($password & $confirm_password)) {
    $password_error = "You must enter a password";
}
elseif ($password !== $confirm_password) {
    $password_error = "The passwords do not match";
}
elseif (!preg_match($has_at_least_6_chars, $password)) {
    $password_error = "Your password must be at least 6 characters long";
}
elseif (!preg_match($has_a_capital_letter, $password)) {
    $password_error = "Your password must have at least 1 capital letter";
}
elseif (!preg_match($has_a_digit, $password)) {
    $password_error = "Your password must contain at least 1 number";
}
else {
    //If everything is true do this
}

By the way, I think there is a problem with $has_at_least_6_chars = '/(?=.*[a-zA-Z0-9]{6})/m' in that it doesn't truly check for 6 characters, it checks for 6 letters and/or numbers. According to the way your error messages are written, this should be a perfectly valid entry meeting all conditions:

$password = "Hell0!";
$confirm_password = "Hell0!";

Yet your code returns "Your password must be at least 6 characters long" (which of course it is, except that your regular expression removes the ! and all other non-alphanumeric characters). It would be much simpler (and more accurate) to use the built-in strlen() PHP function.

elseif (strlen($password) < 6) {
    $password_error = "Your password must be at least 6 characters long";
}

That is, unless you have requirements that forbid non-alphanumeric characters, which I think would be a terrible idea, since many people use them to make for stronger passwords that are harder to break through brute force.


Logic for an init function in email parser I am writing an email parser and cannot decide how much logic should be included in the __init__ function. I know that a constructor should make the object ready for use, but are there best practices?

I've added several statements to my constructor:

def __init__(self, message):
    assert isinstance(message, basestring)

    parsed_message = email.message_from_string(message)

    if self._is_email(parsed_message):
        self.email_obj = parsed_message
        email_payload = parsed_message.get_payload()
        self.valid_urls = self._find_valid_urls(email_payload)
        # ... some other stuff like looking for keywords etc.

What do you consider the most pythonic way:

Should I create a simple email object which possesses several methods that extract and return keywords, included URLs etc?
Do you consider it better to process all this information in the __init__ method and putting all results in object variables?

Notes:

Personally I prefer to keep the initialization method simple and lean. Just setup the minimum state of the object, let subsequent calls to methods do the real job when it's needed. Otherwise, why use a class? Maybe you don't need a class at all and just a function would do?
assert isinstance(message, basestring): Python is a dynamic language, embrace the fact and don't lose a second checking whether every argument has the type you expect (instead, write good docstrigs so the user knows how to call it). Another question is checking that given values satisfy some precondition (i.e assert len(s) >= 10).


Prototype for sending commands to an Arduino Uno This is a prototype for sending commands to an Arduino Uno. The program will allow the user to receive all the logged data such as GPS data and stuff like that. Since I haven't written the Arduino code to get actual data, the random_log function creates random data.

I have not yet written all the code for returning to home and all that jazz above, but I would first like feedback on what I have so far and how I can better organize or simplify it. (FYI the code works perfectly for me.)

import random
import time

print ('PROTOTYPE COMMAND PROMPT FOR TRANS COM')
print ('')

def random_log():
  for i in range(1, 50):
    right = random.randint(-10, 10)
    left = random.randint(-10, 10)
    r_time = random.randint(1, 15)
    l_time = random.randint(1, 15)
    log = [left, l_time, right, r_time]
    print (log)

def rth_dir():
  print ('RETURNING TO HOME POINT VIA DIRECT PATH')
  return trans_com()

def rth_log():
  print ('RETURNING TO HOME VIA PATH LOGS')
  return trans_com()

def abort():
  print ('EMERGENCY RTH HAS BEEN AUTHORIZED.')
  print ('URGENT: "u"   LOW BATTERY: "low bat"')
  a_type = input('SELECT ABORT TYPE')

  if a_type == 'u':
    print ('RETURNING ASAP')
    return trans_com

  elif a_type == "low bat":
    print ('RETURNING BECAUSE OF LOW BATTERY')
    return trans_com()

  else:
    print ('ERROR! NOT VALID ABORT TYPE')
    return abort()

def trans_com():
  trans_com_in = input('COMMAND COM: ')
  if trans_com_in == 'get log':
    random_log()
    return trans_com()

  elif trans_com_in == 'rth dir':
    rth_dir()
    return trans_com()

  elif trans_com_in == 'rth log':
    rth_log()
    return trans_com()

  elif trans_com_in == 'a':
    abort()
    return trans_com()

  elif trans_com_in == 'coms':
    print ('=================================================================')
    print ('rth_dir ---> Return To Home by direct path')
    print ('rth_log ---> Return To Home by logged data')
    print ('a ---> imediatly returns to home medium speed: EMERGENCY ONLY!')
    print ('get log ---> prints all travel log since home point')
    print ('=================================================================')
    print ('')
    return trans_com()

  else:
    print ('Unknown command, type "coms" for list of all commands')
    print ('')
    return trans_com()
trans_com()

trans_com is the user input for executing the different functions

Fixing minor errors

There are 2 minor errors you should fix and they are related to the commands to type:

print ('=================================================================')
print ('rth_dir ---> Return To Home by direct path')
print ('rth_log ---> Return To Home by logged data')
print ('a ---> imediatly returns to home medium speed: EMERGENCY ONLY')
print ('get log ---> prints all travel log since home point')
print ('=================================================================')

Actually typing rth_dir and rth_log lead to errors. What you actually meant was rth dir and rth log respectively (removed the underscore).

Handling the keyboard interrupt exception

When the user aims to exit your application and presses Ctrl + C, this exception is thrown but not handled:

    ...
    trans_com_in = input('COMMAND COM: ')
KeyboardInterrupt

You can handle the KeyboardInterrupt exception by running a sys.exit() this way:

def trans_com():
    try:
        # The previous function body here
    except KeyboardInterrupt:
       sys.exit()

Better command line approach

You can make your code much cleaner and more efficient if you rely on a parser for command-line options such as the one that comes with Python itself and called argparse. With that in mind, your in addition to cleanness and efficiency of the code mentioned before, this module will help you to get rid of several other lines of code. I

What I mean is that you can substitute these lines:

elif trans_com_in == 'coms':
    print ('=================================================================')
    print ('rth_dir ---> Return To Home by direct path')
    print ('rth_log ---> Return To Home by logged data')
    print ('a ---> imediatly returns to home medium speed: EMERGENCY ONLY!')
    print ('get log ---> prints all travel log since home point')
    print ('=================================================================')
    print ('')
    return trans_com()

  else:
    print ('Unknown command, type "coms" for list of all commands')
    print ('')
    return trans_com()

by using argparse accordingly; for example:

import argparse
# ...
# ...
parser = argparse.ArgumentParser(description="Prototype for sending commands to an Arduino Uno.")
group = parser.add_mutually_exclusive_group(required=True)
group.add_argument("--rth dir", help="Return To Home by direct path.")
group.add_argument("--rth log", help="Return To Home by logged data.")
group.add_argument("--a", help="Imediatly returns to home medium speed: EMERGENCY ONLY!")
group.add_argument("-- get log", help="Prints all travel log since home point.")
args = parser.parse_args()

This approach will  automatically lead several other lines of your code , such as these  ones, to be useless:

else:
    print ('Unknown command, type "coms" for list of all commands')
    print ('')

and:

trans_com_in = input('COMMAND COM: ')

Because when the user tries to run your code this way: python3 solution.py, the Python interpreter will trigger this exception that is self expressing:
    python3 solution.py
    usage: solution.py [-h]
                       (--rth dir RTH DIR | --rth log RTH LOG | --a A | -- get log  GET LOG)
    solution.py: error: one of the arguments --rth dir --rth log --a -- get log is required

By typing the --help (or -h) option, the user can see the information you already wanted to display in your current code:

python3 solution.py -h
usage: solution.py [-h]
                   (--rth dir RTH DIR | --rth log RTH LOG | --a A | -- get log  GET LOG)

Prototype for sending commands to an Arduino Uno.

optional arguments:
  -h, --help           show this help message and exit
  --rth dir RTH DIR    Return To Home by direct path.
  --rth log RTH LOG    Return To Home by logged data.
  --a A                Imediatly returns to home medium speed: EMERGENCY ONLY!
  -- get log  GET LOG  Prints all travel log since home point.

Now with this approach, you need to change your code accordingly, and I think that will be easy for you to do by checking the content of args variable instead of that of trans_com_in.

Other notes

You can type:

print ('PROTOTYPE COMMAND PROMPT FOR TRANS COM\n')

instead of:

print ('PROTOTYPE COMMAND PROMPT FOR TRANS COM')
print ('')

Choose better names: Functions' names like rth_dir(), trans_com() and log_dir() do not tell much about their purpose unless the maintainer of your code digs into their bodies. You should choose names that express right away the purpose of those functions. The same observation is valid when it comes to other variables (such as trans_com_in, r_time, l_time, left, right.
Leave only one blank line between the definitions of your functions. Please check PEP8 for more details.


Aggregation of files I have millions of files in a folder. File types will be with the name PES_timestamp and DOM_timestamp. I have to aggregate all PES and DOM filenames into one file which has the same filename.

The Aggregator program picks up only matching PES and DOM file pairs based on the timestamp of the file name from the incoming folder.

Example:

pes234343440
dom234343440

I thought of this idea to create 3 lists:

Holds all files in the folder
Holds only PES files
Holds all DOM files

Finally, I will read all the file name from list and aggregate them from this program:

public void mergeFiles(File output,List<String> list) throws IOException {
        BufferedWriter writer=null;
        BufferedReader br = null;
        BufferedReader r = null;
        try {
        writer = new BufferedWriter(new FileWriter(output));
        for(int i=0;i<list.size();i++){
              br = new BufferedReader(new FileReader("INCOMINGFOLDER/"+list.get(0)));
              AggregateNj obj=new AggregateNj();
              String s1 =null;
                while ((s1 = br.readLine()) != null) {
                    writer.write(s1);
                    writer.write("\n");
                }
            }
        } catch (IOException e){
            e.printStackTrace();
        }finally{
            if(br != null)br.close();
            if(r != null)r.close();
            if(writer != null)writer.close();
        }
    }

Is this approach correct, or can you suggest a better approach?

Spaces. You need spaces around things. For example, keywords should always have whitespace on either side. Brackets, too. Parentheses should have spaces on the outside -- that is, to the right of a closing, to the left of an opening one. Also, on either side of binary operators. In general, your use of whitespace is inconsistent and unconventional.

Aside from that:

You need to work on your variable names. Things like r, list, s1, and obj don't say anything about what they do or for what purpose they exist. In short, they're meaningless.

You always open list.get(0), so you're always opening the same file. I strongly suspect you mean list.get(i).

With that change, you can change the for from a normal for to an advanced one, which makes it much simpler semantically.

Still focusing on that line: Why are you assuming that the files to be merged are in INCOMINGFOLDER? That's a pretty risky assumption for a method that has no context other than being called mergeFiles, which suggests that it should merge any files, without modifying the paths.

Rather than hardcoding a line separator which doesn't work on all OSs -- which, incidentally, sorta breaks the biggest reason for using Java at all -- use BufferedWriter#newLine().

Instead of manually checking for null in a finally, just use a try with resources. It's much cleaner.

It's generally bad to swallow up errors without telling the use of the function. Rather than printing the stacktrace of possible IOExceptions, I'd recomment rethrowing it. I suggest removing the catch block -- that is, passing it up the chain.

You never use the instance of AggregateNj, so it's useless, so I got rid of it. You also never use BufferedReader r, so I removed that as well.

With all those put in, here's your (much nicer IMO) code:

public void mergeFiles(File outputFile, List<String> inputFiles) throws IOException {
    try (BufferedWriter output = new BufferedWriter(new FileWriter(outputFile))) {
        for(String fileName : inputFiles){
            try (BufferedReader in = new BufferedReader(new FileReader(fileName))) {
                String line;
                while ((line = in.readLine()) != null) {
                    output.write(line);
                    output.newLine();
                }
            }
        }
    }
}

You could also return a boolean depending on whether or not it completed successfully, and the method would look like this:

public boolean mergeFiles(File outputFile, List<String> inputFiles) throws IOException {
    try (BufferedWriter output = new BufferedWriter(new FileWriter(outputFile))) {
        for(String fileName : inputFiles){
            try (BufferedReader in = new BufferedReader(new FileReader(fileName))) {
                String line;
                while ((line = in.readLine()) != null) {
                    output.write(line);
                    output.newLine();
                }
            }
        }
        return true;
    } catch (IOException e) {
        return false;
    }
}

However, I'd recommend throwing an error instead of returning a boolean, as it gives a lot more information about what went wrong.

Note that I didn't review your design, since this is Code Review. If you want the design reviewed, please provide the code.


Converting a list of lists to a list of objects I have some code that does what I want - it just doesn't feel correct for being done in JavaScript.

What it does is take data that is like this:

   var data = [['Marker', 'sampleName1', 'sampleName2'],
     ['CCR4', 71.6, 83.4],
     ['CD27', 42.3, 76.2]];

And create a list of objects that looks like this (to use the plot.ly JavaScript library):

[{x:['CCR4', 'CD27'], y:[71.6,42.3], name: 'sampleName1'},
{x:['CCR4', 'CD27'], y:[83.4,76.2], name: 'sampleName2'}]

What I have is this:

var bData = [];
for (var i=0; i < data.length; i++) {
        var row = data[i];
        if (i ==0) {
                //Handle creation of objects
                for (var k = 1; k < row.length; k++) {
                        var obj = {name : row[k], type:'bar', x:[], y:[]};
                        bData.push(obj);
                }
        } else {
                var marker = row[0];
                for (var k=1; k < row.length; k++) {
                        var obj = bData[k -1];
                        obj.x.push(marker);
                        obj.y.push(row[k]);
                }
        }
}

What would be a better, more legible approach to this?

Here's a functional solution which I think is cleaner and makes what you are actually doing (essentially extracting columns from the data) more clear.

I put the explanation inline as comments:

function plotlyFormat(data) {
  var data = data.slice(0),         // make a copy
      headers = data.shift(),       // remove header row and save it
      x = column(data, 0);          // calculate the x property, same for all objects

  // Create the objects, each one's "y" data is just a column
  // of the original data

  return headers.map(function(name, i) {
    return {x: x, y: column(data,i), name: name}
  }).slice(1);

  // ^^^ the final "slice(1) above removes the first record,
  // since it's the "Marker" column, not an actual sample

  // simple utility function to extract a column from a nested array (matrix)
  function column(matrix, i) { return matrix.map(function(row) { return row[i] }) }
}

Just call the function on your input data, and it will return it in the format you want.

And an uncommented version, to see the character savings :)

function plotlyFormat(data) {
  var data = data.slice(0),
      headers = data.shift(),
      x = column(data, 0);

  return headers.map(function(name, i) {
    return {x: x, y: column(data,i), name: name}
  }).slice(1);

  function column(matrix, i) { return matrix.map(function(row) { return row[i] }) }
}


What would be the optimal way to address these arrays I think the arrays here are slowing down my code due to incorrect usage.

There is a lot of stuff going on here, it's understandable if nobody responds. Just looking for some pointers. Thanks.

for MN in xrange(RowModel,TotalModels):

    if(MN%5000==0):print 'Examining models', MN,'to', (MN+5000), '...'

    fobs=fobstemp
    fobserr=fobstemp*0.1 #10% error
    y=np.array([])
    fmod=np.array([])

I was reading that it can be memory exhaustive to run for loops and range functions over numpy arrays and it is better instead to run np.arange[]. Can anyone validate this statement? I'm not sure how to implement it here where RowModel and TotalModels are separate arrays. And assuming they are the same dimensions.

Here's the section of that code (which starts off with the above snippet). Ultimately the values at the end are used in a statistical calculation. The code works but it's insanely slow at this portion.

for MN in xrange(RowModel,TotalModels):

    if(MN%5000==0):print 'Examining models', MN,'to', (MN+5000), '...'

    fobs=fobstemp
    fobserr=fobstemp*0.1 #10% error
    y=np.array([])
    fmod=np.array([])

#Set the scale factor
#units are already embedded in scalemult [mJy]

    if(modcalibrate<=13):
     scale= (10**(convertStr(data[RowStar][calibrate])/-2.5)*10**(0.4*tau_mag[choice-1])*scalemult)/((convertStr(model[MN][modcalibrate])*1e+03*Fbol*1e+23)/(c/wavelengthmodel[modcalibrate-10]))
    else:
     scale= (convertStr(data[RowStar][calibrate])*10**(0.4*tau_mag[choice-1]))/(convertStr(model[MN][modcalibrate])*Fbol*1e+029/(c/wavelengthmodel[modcalibrate-10]))

#Convert the model data into fluxes and insert into array fmod
#
    for i in xrange(10+len(wavelength)-2):#2 repeats
     y=np.append(y,model[MN][i])
    for i in xrange(10,10+len(wavelength)-2): #2 repeats
     Alambda=Ak/wavelength[i-10]
     fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))
     if i== 11:
      fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))
     if i== 13:
      fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))

#Delete the components that are not shared between fobs and fmod
#
    j=0

    for i in xrange(len(wavelength)):
     if j<=len(fobs)-1:
      if -10<fobs[j]<-9:
       fobs=np.delete(fobs,j,None)
       fobserr=np.delete(fobserr,j,None)
       fmod=np.delete(fmod,j,None)
      else:
       j=j+1

#Calculate Chi^2 for the model and insert into an array
#

    scalefactor=np.append(scalefactor,scale)
    chi2_new=np.sum(((fobscaled-fmod)/(fobserr))**2)/(len(fobscaled)-1)
    chi2=np.append(chi2,chi2_new)
    if chi2_new<ChiThresh: #User input required here for Chi threshold maximum.
     BestFitsModel=np.append(BestFitsModel,MN)
     BestFitsChi=np.append(BestFitsChi,chi2_new)
     BestFitsScale=np.append(BestFitsScale,scale)
     counter=counter+1
    temp=np.sort(chi2)

wavelengthmodel is defined as an array of size 8. (8 entries, all floats).

wavelength is defined as an array of size 11 (11 entries, all floats).

modcalibrate is an index array which corresponds to a list imported elsewhere.

MN is a numerical array starting at 0 and ending at some large number. This corresponds to TopModels and RowModel which starts at 0 (RowModel value) and ends at n (TopModels value).

Anything else undefined in this script-section is an empty array until filled here, except 'ChiThresh' which is an integer of user input.

You should almost never be using np.append, numpy arrays are designed to be used as fixed size arrays. np.append creates a whole new array each time and will thus fairly expensive.

The first thing is that you should create the array at its full size. Then assign to the elements:

y = np.empty(10+len(wavelength)-2, int) # I don't know the type, guessing here
for i in xrange(10+len(wavelength)-2):#2 repeats
    y[i] = model[MN][i]

The second thing is that you should look for numpy operations to do the same thing your loop does. In this case, I can actually replace that code by:

y = model[MN, :10 + len(wavelength) - 2]

This assumes that model is a numpy array. If it is isn't, you should make it one.

Let's look at the code for fmod

for i in xrange(10,10+len(wavelength)-2): #2 repeats
     Alambda=Ak/wavelength[i-10]
     fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))
     if i== 11:
      fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))
     if i== 13:
      fmod=np.append(fmod,convertStr(y[i])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i-10]))

It is not as obvious how to vectorize this code. Firstly, we can clean up the code a bit. Eliminate the unused Alambda, eliminate duplicate pieces of code, and change the loop to start with zero. (When trying to vectorize you pretty much always want to start by changing your loops to start with 0).

for i in xrange(len(wavelength)-2): #2 repeats
     value = convertStr(y[i+ 10])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i])
     fmod=np.append(fmod, value)
     if i == 11 or i == 13:
          fmod = np.append(fmod, value)

It is still not clear how to break up this loop, so we apply a general strategy of breaking up a loop. If a loop does multiple things, we break the loop up so that the loop only does one thing. Here is the first loop.

value = numpy.empty( len(wavelength) - 2)
for i in xrange(len(wavelength) - 2):
    value[i] = convertStr(y[i+10])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel[i])

Once we have the loop down to the simple level of assigning to each element, we want to vectorize it. I don't know what convertStr does, but I'll assume it can be adapted to vectorize whatever I pass into it. So I can replace this loop by:

value = convertStr(y[10:])*Fbol*scale*1e+023*1e+03/(c/wavelengthmodel)

Let's move onto the second loop

for i in xrange(len(wavelength)-2): #2 repeats
     fmod=np.append(fmod, value[i])
     if i == 11 or i == 13:
          fmod = np.append(fmod, value[i])

This essentially copies value, but introduces repeats. There are  few ways this could be accomplished. Probability the simplest would be:

fmod = np.empty( len(wavelength), int)
fmod[0:11] = value[0:11]
fmod[11:13] = value[11]
fmod[13] = value[12]
fmod[14:17] = value[13]
fmod[17:] = value[14:]

Hopefully that gives you an idea of how to approach improving the performance.


find leading zeros: performance I am trying to find a N number of leading zeros from the output of the sha1 hash function. I would like N to go up to 10 or 9. Currently I can get to 7 in about 7 minutes (even though is not always that fast), but already 8 takes forever. The input to the sha1 must be a combination between an input_str and a random generated string.
Here is my code:
import os
import base64
import hashlib
import time

def gen_keys_06(_urandom=os.urandom, _encode=base64.b64encode):
    while True:
        yield _encode(_urandom(4)).decode('ascii')

def search_matching_random_str(input_str, zeroes, _sha1=hashlib.sha1):
    leading_zeros = "0"*zeroes
    for my_random_str in gen_keys_06():
        input_str_my_random_str = "".join([input_str, my_random_str])
        hashval = _sha1(input_str_my_random_str.encode('utf-8')).hexdigest()
        if hashval[:zeroes] == leading_zeros:
            return hashval, my_random_str

def get_time(input_data, zeroes):
    start = time.perf_counter()
    val, random_str = search_matching_random_str(input_data, zeroes)
    print(f'hash: {val}')
    print(f'random_str: {random_str}')
    finish = time.perf_counter()
    print(f'Finished in {round(finish-start, 2)} seconds(s)')

if __name__ == '__main__':

    first_str = "eTSlASZYlLNgKJuYeIQvGVbiAcLEEOVgAQPzSrtCOIwQxQHyFHcfjgRQJBJDlojx"
    get_time(first_str, 4)

On what should I work to make it faster?

zeros or zeroes? Better make up your mind and stick with one.
str.join is good when you want to join an iterable. For two strings, just use +.
Instead of always reencoding and rehashing the input string, you can do it once and then copy the resulting hasher (state) for different extensions.
Instead of creating random bytes and then elaborately turning them into a string and back to bytes, just use the bytes. Since that's also just a single function call, you can ditch your gen_keys_06 and its overhead.
str.startswith is at least simpler, don't know about speed.
For measuring performance, create a benchmark that runs the function much more than just once, as a single time is rather random. Or change the function so it tries a fixed number of random extensions (let's say a million) instead of stopping at the first successful one.
You ask us "On what should I work to make it faster?". That's a question to ask a profiler.
Producing random values takes time. Check the profiler's results (or leave out the randomness when doing the million-extensions thing) to see whether it's significant here. If it is, maybe try a different randomness source (I think I've seen someone say that os.urandom is slow on Linux). Or if you don't actually need randomness, try increasing bytes instead.
Do you really want zero-nibbles? Or would zero-bytes work as well, i.e., are you maybe really only interested in even numbers of zero-nibbles? Then you could use digest instead of hexdigest, which is probably faster because bytes are probably what the hasher actually works with and because it makes digests half as large and you'd check for fewer zeros.

A version incorporating some of those points:
def search_matching_random_str(input_str, zeros, _sha1=hashlib.sha1, _urandom=os.urandom):
    leading_zeros = "0" * zeros
    hasher_copy = _sha1(input_str.encode()).copy
    while True:
        my_random_bytes = _urandom(4)
        hasher = hasher_copy()
        hasher.update(my_random_bytes)
        hashval = hasher.hexdigest()
        if hashval.startswith(leading_zeros):
            return hashval, my_random_bytes


Remove pixel patch in image which is stored as array I have an array I which stores N images of size P (number of pixels). Every image is of size P = q*q.

Now I want to delete patches of size ps around a selected index IDX (set all values to zero).

My approach was to reshape every single image using reshape(q,q) and delete the pixels around IDX. I also have to check if the index is not outside the image.

Here is an example:

BEFORE:

AFTER:

My code is a real bottleneck and I would like to know if there is a way to improve the performance of my approach.

import numpy as np
import matplotlib.pyplot as plt
import time

def myplot(I):
    imgs = 5
    for i in range(imgs**2):
        plt.subplot(imgs,imgs,(i+1))
        plt.imshow(I[i].reshape(q,q), cmap="viridis", interpolation="none")
        plt.axis("off")
    plt.show()

N = 10000
q = 28
P = q*q
I = np.ones((N,P))
myplot(I)

ps = 5
IDX = np.random.randint(0,P,(N,1))
x0, y0 = np.unravel_index(IDX,(q,q))

t0 = time.time()

# HOW TO IMPROVE THIS PART ? #
for i in range(N):
    img = I[i].reshape(q,q)
    for x in range(ps):
        for y in range(ps):
            if (x0[i]+x < q) and (y0[i]+y < q):
                img[x0[i]+x,y0[i]+y] = 0.0
    I[i] = img.reshape(1,q*q)

print(time.time()-t0)
myplot(I)

I call this code (without the plotting procedure) about one million times from another code. Every call takes about 1 second on my system. This makes the code so far quite useless.

Any advice?

On my computer it takes 1.745 seconds to run the code in the post.
There's no need for the array of random indexes to be two-dimensional:

IDX = np.random.randint(0,P,(N,1))

In fact this is harmful for performance, because it means that x0[i] is an array of length 1 (not a scalar) and so img[x0[i]+x,y0[i]+y] requires "fancy indexing" which is slower than normal indexing.

It would be simpler to make the array of indexes one-dimensional:

IDX = np.random.randint(P, size=N)

This reduces the runtime to about 0.459 seconds (26.3% of the original).
There is no need to reassign I[i] at the end of the loop. When you call the reshape method on a NumPy array, what you get is a view onto the original array (not a copy) if possible. (And it is possible in this case.) So updating the view also updates the original.

This reduces the runtime to about 0.449 seconds (25.8%).
Instead of looping over range(N) and then looking up I[i] and x0[i] and y0[i], use zip to loop over all the arrays simultaneously:

for img, xx, yy in zip(I, x0, y0):
    img = img.reshape(q,q)
    for x in range(ps):
        for y in range(ps):
            if xx + x < q and yy + y < q:
                img[xy + x, yy + y] = 0.0

This reduces the runtime to about 0.358 seconds (20.5%).
Instead of looping over all the pixels in the patch and updating each pixel individually, use slices to update the whole region in one step:

for image, x, y in zip(I, x0, y0):
    image.reshape(q, q)[x:x + ps, y:y + ps] = 0.0

This works because NumPy (and Python generally) ensures that the bounds of a slice do not go beyond the end of the array. See the slicing documentation:

  The slice of \$s\$ from \$i\$ to \$j\$ is defined as the sequence of items with index \$k\$ such that \$i \le k < j\$. If \$i\$ or \$j\$ is greater than len(s), use len(s).

This reduces the runtime to about 0.025 seconds (1.4%).
We can vectorize the additions x + ps and y + ps:

for image, x, y, x1, y1 in zip(I, x0, y0, x0 + ps, y0 + ps):
    image.reshape(q, q)[x:x1, y:y1] = 0.0

This reduces the runtime to about 0.021 seconds (1.2%).
We could avoid the reshape inside the loop by doing a single reshape of the whole I array:

images = I.reshape(N, q, q)

and then:

for image, x, y, x1, y1 in zip(images, x0, y0, x0 + ps, y0 + ps):
    image[x:x1, y:y1] = 0.0

This reduces the runtime to about 0.018 seconds (1.0%).
We can halve the number of indexing operations by indexing the images array just once on each loop iteration:

for i, x, y, x1, y1 in zip(range(N), x0, y0, x0 + ps, y0 + ps):
    images[i, x:x1, y:y1] = 0.0

This reduces the runtime to about 0.011 seconds (0.6%).

That's about 150 times speedup overall, so calling this a million times will still take about 3 hours on my computer. There may be other improvements to be had if only we could see more of your code, but you'll need to make a new post for that.


Custom Dynamic Array Pretty simple class but just want to make sure I've approached it correctly.

public class DynamicArray<T> : IEnumerable<T>, ICollection<T>
    {
        private T[] _items;
        private int _size;
        private const int _growFactor = 2;

        public DynamicArray()
        {
            _items = new T[4];
        }

        public int Count
        {
            get
            {
                return _size;
            }
        }

        public bool IsReadOnly
        {
            get
            {
                return false;
            }
        }

        public void Add(T item)
        {
            if (_size + 1 > _items.Length)
            {
                var newArray = new T[_items.Length * _growFactor];
                _items.CopyTo(newArray, 0);
                _items = newArray;
            }
            _items[_size] = item;
            _size++;
        }

        public void Clear()
        {
            _items = new T[2];
            _size = 0;
        }

        public bool Contains(T item)
        {
            foreach(var value in _items)
            {
                if (value.Equals(item))
                {
                    return true;
                }
            }
            return false;
        }

        public void CopyTo(T[] array, int arrayIndex)
        {
            _items.CopyTo(array, arrayIndex);
        }

        public IEnumerator<T> GetEnumerator()
        {
            for(var i = 0; i < _size; i++)
            {
                yield return _items[i];
            }
        }

        public bool Remove(T item)
        {
            for(var i = 0; i < _items.Length; i++)
            {
                if (_items[i].Equals(item))
                {
                    _items[i] = default(T);
                    _size--;
                    return true;
                }
            }
            return false;
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }

Just a couple of minor points:

In your constructor, you create an the array with an initial size of 4:

_items = new T[4];

However in your Clear method you set it to an size of 2:

_items = new T[2];

It's odd that you set it to a smaller number.  If you had a constant for initial size, or called Clear from the constructor this could be avoided.
That said, do you really want clear to discard _items?  If you're reusing the array in a similar section of code, it seems reasonable to assume that after Clear has been called, there is a good chance the the array would grow again to a similar size.  It may be more efficient to simply reset _size, but leave _items intact.  Obviously this depends how you're using the array as to whether or not it's a good idea...
You don't limit the size of the array.  Dynamic doubling whilst efficient, can get big pretty quick.  You might want to consider adding a maximum size to your array, again, this would depend on what you're using it for as to whether or not it's worth while.  List limits it's growth for example, albeit to a rather large number.


HTML and CSS markup to achieve desired result I am new to HTML and CSS. I was wondering if anyone could give me some advice on my use of markup and CSS to achieve this result. I am trying to get a fixed full height sidebar with a list with sprite image for each item.

This is the code I have which achieves this.

HTML

User

Vacancies

Company

Create New

Title

Create new list

        <form name="input">
            <input type="text" name="Title">
            <input type="text" name="Location">
            <input type="text" name="Description">
            <input type="text" name="Closing date">
            <input type="submit" value="Add">
        </form>

CSS

* {
    margin: 0px;
    padding: 0px;
}
li {
    list-style-type: none;
}
.container {
    display: block;
    position:absolute;
    height:auto;
    bottom:0;
    top:0;
    left:0;
    right:0;
    background-color:white;
}
.column_wrap {
    position:relative;
}
.col_left {
    background-color:#f2f2f2;
    width:250px;
    position:absolute;
    left:0;
    top:0;
    bottom:0;
    height:auto;
    display: block;
}
.col_left img {
    float:right;
    display: block;
    line-height:40px;
    margin-left: auto;
    margin-right: auto;
}
.col_left li a {
    color: gray;
    display: block;
    line-height: 26x;
}
.col_left li a:hover {
    color: green;
}
.col_right {
    margin-left:250px;
}
.col_right li {
    float: right;
}
.col_right a {
    float: right;
    background: green;
    padding: 10px;
    color: white;
    font-weight:bold;
}
.col_right a:hover {
    background: blue;
}
input {
    display: block;
    border: 2px solid gray;
    padding: 10px;
    margin: 10px
}
input:hover {
    border: 2px solid blue;
}
input:focus {
    border: 2px solid blue;
}
.navimg {
    background:url('http://www.otlayi.com/web_images/content/free-doc-type-sprite-icons.jpg');
width: 0px;
    height: 20px;
    padding-left: 30px;
}
#user {
    background-position: -10px -6px;
}
#vacancy {
    background-position: -48px -6px;
}
#company {
    background-position: -88px -6px;
}

However, I think there might be some areas where my code is inefficient, messy or non symantic. Can anyone give me any advice please? I would really appreciate it.

A few suggestions

You were using this to pass classes to your main logo

.col_left img {
    float:right;
    display: block;
    line-height:40px;
    margin-left: auto;
    margin-right: auto;
}

But that will create problems if you introduce any other images there. So instead try giving that element a class. For example, a class of logo or anything that you want to name it. This way there will not be any unexpected conflicts caused by CSS inheritance

HTML

CSS

.logo {
  float:right;
  display: block;
  line-height:40px;
  margin-left: auto;
  margin-right: auto;
}

In testing, can use image placeholders. In my example, I used Placehold.it which allows you to just give a desired height

All of your
s on the left have the same rules. So you might as well give that
 a class and pass rules that way. However some rules may not apply. Also you gave both an id and class to those
s. this is what I did

HTML

    User

    Vacancies

    Company

CSS

.user, .vacancy, .company {
  background:url('http://www.otlayi.com/web_images/content/free-doc-type-sprite-icons.jpg');
  width: 0px;
  height: 20px;
  padding-left: 30px;
}

.user { background-position: -10px -6px; }
.vacancy { background-position: -48px -6px; }
.company { background-position: -88px -6px; }

I did it that way so that you can choose to add in what you want to accept what CSS rules.

in testing. When you have links, it is common to just use href="#"

Full code available at this CodePen.


Very simple calculator with four basic operations using only if/else statements I created a simple calculator with multiplication, division, addition, and subtraction. Is there a way to write it shorter? I am a beginner so I do not know much about programming other than if...else statements and different data types.

        Console.WriteLine("Welcome to the simple calculator! The four operators of this calculator are +, -, *(multiply), and /(divide)");
        Console.Write("Number: ");
        string number = Console.ReadLine();
        Console.Write("Second Number: ");
        string secondNumber = Console.ReadLine();
        Console.Write("Operator: ");
        string operatorType = Console.ReadLine();

        var numberConversion = Convert.ToInt64(number);
        var secondNumberConversion = Convert.ToInt64(secondNumber);
        var sum = numberConversion + secondNumberConversion; //Result of addition
        var difference = numberConversion - secondNumberConversion; //Result of subtraction
        var product = numberConversion * secondNumberConversion; //Result of multiplication
        var quotient = numberConversion / secondNumberConversion; //Result of division

        if (operatorType == "+")
        {
            Console.WriteLine(sum);
        } else if (operatorType == "-")
        {
            Console.WriteLine(difference);
        } else if (operatorType == "*")
        {
            Console.WriteLine(product);
        } else if (operatorType == "/")
        {
            Console.WriteLine(quotient);
        } else
        {
            Console.WriteLine("Please enter the correct operator");
        }

You need to check for input validity and have a mechanism for retry. Users can enter all sorts of things
Your variable names aren't consistent. number and secondNumber. I would go with firstNumber and secondNumber in your scheme, but I would not use your scheme.
Your variable names are too verbose: secondNumberConversion. How about n1 and n2?
A simple, elegant, maintainable and extensible solution is to use a Dictionary.

Not addressing 1. here is my version:

Console.WriteLine("Welcome to the simple calculator!"
                  + " The four operators of this calculator are "
                  + "+, -, *(multiply), and /(divide)");

Console.Write("First Number: ");
Int64 n1 = Int64.Parse(Console.ReadLine());

Console.Write("Second Number: ");
Int64 n2 = Int64.Parse(Console.ReadLine());

Console.Write("Operator: ");
string operatorType = Console.ReadLine();

var operatorsFuncMap = new Dictionary<string, Func<Int64, Int64, Int64>>
{
    {"+", (a, b) => a + b},
    {"-", (a, b) => a - b},
    {"*", (a, b) => a * b},
    {"/", (a, b) => a / b},
};

Console.WriteLine(operatorsFuncMap[operatorType](n1, n2));


Web-scraping investment fund data into Pandas Just starting out trying to do some python for data analysis. I am a total beginner and have figure out how to brute force somethings, but I know it is inefficient but don't know how to do it without messing up what i have.

I have multiple webpages to scrape and then store data in a data frame. The code is identical for all of the pages. How do I set it up as a routine instead just repeating the same code over and over again.

As an example the two urls are: https://etfdb.com/etf/IWD/ https://etfdb.com/etf/IWF/

The html is identical so the web scraping working exactly the same for both.

Once scraped, I want them in a single data frame.

The below works, but is me taking the least sophisticated approach since I know so little. The actual code is likely not pretty but it works.

Any help appreciated on how this should be improved.

import bs4
from urllib.request import urlopen as uReq
from bs4 import BeautifulSoup as soup
import pandas as pd
import numpy as np
from IPython.display import display

iwd_url = 'https://etfdb.com/etf/IWD/'
uClient = uReq(iwd_url)
page_html = uClient.read()
uClient.close()
page_soup = soup(page_html, "html.parser")

#Isolate header to get name and symbol
h1 = page_soup.h1

#Isolate stock symbol
title = h1.findAll("span",{"class":"label-primary"})
titlet = title[0].text

#print(titlet)

#strip space and line break
strip1 = h1.text.strip()

#strip stock symbol
strip2 = strip1.strip(titlet)

#strip remaining line break
strip3 = strip2.strip()

#print(strip3)

IWD = page_soup.findAll("table",{"class":"chart base-table"})[1]
#Create lists to fill
sectordata=[]
sectorname=[]
sectorweight=[]
for row in IWD.findAll("td"):

    sectordata.append(row.text)
#list created

#Assign every other value to proper list to get 2 columns
sectorname = sectordata[::2]
sectorweight = sectordata[1::2]

#Insert name/symbol for clarification/validation
sectorweight.insert(0,titlet)
sectorname.insert(0,strip3)

# create empty data frame in pandas
df = pd.DataFrame()
#Add the first column to the empty dataframe.
df['Sector']  = sectorname

#Now add the second column.
df['Weight'] = sectorweight
##display(df)

### NEXT

iwf_url = 'https://etfdb.com/etf/IWF/'
uClient = uReq(iwf_url)
page_html = uClient.read()
uClient.close()
page_soup = soup(page_html, "html.parser")

#Isolate header to get name and symbol
h1 = page_soup.h1

#Isolate stock symbol
title = h1.findAll("span",{"class":"label-primary"})
titlet = title[0].text

#print(titlet)

#strip space and line break
strip1 = h1.text.strip()

#strip stock symbol
strip2 = strip1.strip(titlet)

#strip remaining line break
strip3 = strip2.strip()

#print(strip3)

IWD = page_soup.findAll("table",{"class":"chart base-table"})[1]
#Create lists to fill
sectordata=[]
sectorname=[]
sectorweight=[]
for row in IWD.findAll("td"):

    sectordata.append(row.text)
#list created

#Assign every other value to proper list to get 2 columns
sectorname = sectordata[::2]
sectorweight = sectordata[1::2]

#Insert name/symbol for clarification/validation
sectorweight.insert(0,titlet)
sectorname.insert(0,strip3)

# create empty data frame in pandas
df2 = pd.DataFrame()
#Add the first column to the empty dataframe.
df2['Sector']  = sectorname

#Now add the second column.
df2['Weight'] = sectorweight
#display(df2)

results = df.merge(df2, on = "Sector")
results.columns = ['Sector', 'IWD', 'IWF']
display(results)

Like I said, this works, but it isn't automated and its ham-handed way of getting there. Please help me to get better!

I created function get_soup because this code is offen used many times in code. In this code I could put all in get_data.

get_data gets url and uses get_soup with this url. Later it scrapes data from html, creates DataFrame and returns it

Main part uses get_data with two urls and gets two dataframes.

I put some other comments in code.

# <-- remove not used modules
# <-- use more popular names
from urllib.request import urlopen
from bs4 import BeautifulSoup as BS
import pandas as pd
from IPython.display import display

# --- functions ---
# <-- all functions before main part

def get_soup(url):
    # <-- more readable names (and more popular)
    response = urlopen(url)
    html = response.read()
    response.close()
    soup = BS(html, "html.parser")
    return soup

def get_data(url):

    soup = get_soup(url)

    #Isolate header to get name and symbol
    h1 = soup.h1

    #Isolate stock symbol
    # <-- find() to get only first item
    title = h1.find("span",{"class":"label-primary"}).text

    #print(title)

    #strip space and line break
    # <-- use the same variable instead strip,strip2, strip3
    # <-- maybe too much comments
    header = h1.text.strip()

    #strip stock symbol
    header = header.strip(title)

    #strip remaining line break
    header = header.strip()

    #print(strip)

    # <-- use better name 'table'
    table = soup.find_all("table",{"class":"chart base-table"})[1]

    #Create lists to fill
    #sector_data = [row.text for row in table.find_all("td")]
    sector_data = []
    # <-- remove lists which will be created later

    for row in table.find_all("td"):
        sector_data.append(row.text)

    #Assign every other value to proper list to get 2 columns
    sector_name = sector_data[::2]
    sector_weight = sector_data[1::2]

    #Insert name/symbol for clarification/validation
    sector_weight.insert(0, title)
    sector_name.insert(0, header)

    # create dataframe in pandas
    # <-- create DF directly with data
    df = pd.DataFrame({
       'Sector': sector_name,
       'Weight': sector_weight,
    })

    #display(df)

    return df

# --- main ---

# <-- the same variable `url` because it keep the same type of data
#     and I will no need this value later - so I can resuse this name.
url = 'https://etfdb.com/etf/IWD/'
df1 = get_data(url)

url = 'https://etfdb.com/etf/IWF/'
df2 = get_data(url)

results = df1.merge(df2, on="Sector")
results.columns = ['Sector', 'IWD', 'IWF']
display(results)


Periodically sliding images I wrote this code using jQuery, but I think it can be improved. Can you provide some feedback?

    var $i = 1,
    totalImg = $('.home-featured-bg .absolute div').length,
    $theWidth = $(window).width(),
    $theHeight = $(window).height();
    $('.home-featured-bg .absolute').css('width',$theWidth * totalImg+'px');
    $('.home-featured-bg .absolute .slide,.home-featured-bg .absolute .slide img').css({'width':$theWidth+'px','height' : $theHeight+'px'});

window.setInterval(function(){
    $('.events-links a').hide();
    if($i == 1)
    {

        var $el = $theWidth;
    }else{
        var $el = $theWidth * $i;
    }
    if($i < totalImg)
    {
        $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
        $i++;
        $('.events-links a').eq($i - 1).css('display','block');
    }else{
        $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
        $i = 1;
        $('.events-links a').eq($i - 1).css('display','block');
    }

},3000);

$(window).resize(function(){
    totalImg = $('.home-featured-bg .absolute div').length,
    $theWidth = $(window).width(),
    $theHeight = $(window).height();
    $('.home-featured-bg .absolute').css('width',$theWidth * totalImg+'px');
    $('.home-featured-bg .absolute .slide,.home-featured-bg .absolute .slide img').css({'width':$theWidth+'px','height' : $theHeight});
});

Here few suggestion:

in the function  you have :

if($i == 1)
{

    var $el = $theWidth;
}else{
    var $el = $theWidth * $i;
}

could be replaced by a single line:

var $el = $theWidth * $i;

and you could modify the following if :

if($i < totalImg)
{
    $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
    $i++;
    $('.events-links a').eq($i - 1).css('display','block');
}else{
    $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
    $i = 1;
    $('.events-links a').eq($i - 1).css('display','block');
}

by extract the $('.events-links a').eq($i - 1).css('display','block');   like this:

if($i < totalImg)
{
    $('.home-featured-bg .absolute').animate({'margin-left':'-'+$el+'px'},2000);
    $i++;
}else{
    $('.home-featured-bg .absolute').animate({'margin-left':'0px'},2000);
    $i = 1;
}
$('.events-links a').eq($i - 1).css('display','block');


Dual rewrite rules (WWW prefix and HTTPS) I want to configure my website to force a www. prefix on the name, and the HTTPS protocol.

<system.webServer>
  <rewrite xdt:Transform="Insert">
    <rules>
      <clear/>
      <rule name="Add WWW prefix" enabled="true">
        <match url="(.*)" ignoreCase="true" />
        <conditions>
          <add input="{HTTP_HOST}" negate="true" pattern="^www\.(.*)$" /> <!--pattern="^www\.([.a-zA-Z0-9]+)$"/>-->
        </conditions>
        <action type="Redirect" url="http://www.{HTTP_HOST}/{R:0}" appendQueryString="true" redirectType="Permanent"/>
      </rule>
      <rule name="Redirect to HTTPS" enabled="true" stopProcessing="true">
        <match url="(.*)" ignoreCase="true" />
        <conditions>
          <add input="{HTTPS}" pattern="^OFF$" />
        </conditions>
        <action type="Redirect" url="https://{HTTP_HOST}/{R:1}" redirectType="Permanent" />
      </rule>-->
    </rules>
  </rewrite>
</system.webServer>

Considerations:

The first rule appears to be working fine.
The second rule is so far untested on the live site. (Still configuring SSL.)
Are there any performance considerations when applying two rules together like this? That is, must they both be applied on each request.

Redirecting twice does cause a slight performance impact, but if you are looking to use HSTS then you should use two redirects, as there are some situations where going direct from http://example.com to https://www.example.com would cause issues.

An example is the HSTS preload list, where you must:

2) Redirect from HTTP to HTTPS on the same host, if you are listening on port 80.
https://hstspreload.org/

So you will need to make sure the redirect chain goes:

http://example.com -> https://example.com -> https://www.example.com


AES GCM Python implementation I've recently started looking into encryption in python and I came across with pycryptodome library. I made this simple program that encrypt/decrypt a file using AES-GCM. Could someone take a look if I implemented the encryption and authentication correctly? For sake of simplicity, I'm posting only the encryption and makeKey functions.

import os
import struct
import secrets
import hahslib
from Cryptodome.Cipher import AES

def makeKey(password, salt):
    key = hashlib.pbkdf2_hmac('sha512',password.encode('ascii'), salt, 1000000)
    return key[:32]

def encrypt(password, inputFileName):
    #creates the needed variables for encryption
    salt = secrets.token_bytes(16)
    key = makeKey(password, salt)
    nonce = secrets.token_bytes(AES.block_size)
    cipher = AES.new(key, AES.MODE_GCM, nonce=nonce)
    #put the file size, salt and nonce in outputfile
    fileSize = os.path.getsize(inputFileName)
    with open("output", "wb") as outputFile:
        outputFile.write(struct.pack('<Q', fileSize))
        outputFile.write(salt)
        outputFile.write(nonce)
        #beginning of encryption itself
        chunkSize = 64 * 1024
        with open(inputFileName, "rb") as inputFile:
            while True:
                chunk = inputFile.read(chunkSize)
                if len(chunk) == 0:
                    outputFile.write(cipher.digest())
                    break
                outputFile.write(cipher.encrypt(chunk))

Handling files - the Python way

Without being able to test run your code at the momemt, there is one thing I can already recommend to you, which is to make use of Python's powerful with statement. Using with will make sure that files will be closed no matter what happens (e.g. unexpected exceptions). with also works with more than one file as described in this SO post.

Also consider adding the output filename as parameter to your function. You can even give your momentary value of "output" as default value.

Minor notes on style

I'm not sure if you're aware of the official Style Guide for Python, often just called PEP8. The official recommendation is to use snake_case for variable and function names. This would create a more unified code appearance when working with other Python libraries which often stick to that convention. As always, exceptions apply.

Below is your code including these changes.

def make_key(password, salt):
    ...

def encrypt(password, input_filename, output_filename="output"):
    #creates the needed variables for encryption
    salt = secrets.token_bytes(16)
    key = make_key(password, salt)
    nonce = secrets.token_bytes(AES.block_size)
    cipher = AES.new(key, AES.MODE_GCM, nonce=nonce)
    # put the file size, salt and nonce in outputfile
    filesize = os.path.getsize(input_filename)
    with open(output_filename, "wb") as output_file,\
         open(input_filename, "rb") as input_file:
        output_file.write(struct.pack('<Q', filesize))
        output_file.write(salt)
        output_file.write(nonce)
        #beginning of encryption itself
        chunkSize = 64 * 1024
        while True:
            chunk = input_file.read(chunkSize)
            if len(chunk) == 0:
                output_file.write(cipher.digest())
                break
            output_file.write(cipher.encrypt(chunk))
        print("File encrypted successfully!")


Credit card validation I started following Harvard's CS50 (Introduction to Computer Science) on edX, and as part of their Hacker edition set 1 was the following assignment:

I am supposed to write a program (in C), that takes as input a long long int from the user and proceeds to check whether the entered number is a valid credit card number. For the purposes of this assignment, I am only interested in Visa, American Express and MasterCard number formats. The validation is done with Luhn's algorithm.

/* This program checks whether an entered number is a valid
 * credit card number (only American Express, Visa and
 * MasterCard formats are supported).
 */

#include <cs50.h>
#include <stdio.h>
#include <string.h>
#include <math.h>

int main(void)
{
    long long int credit_card;
    int digits = 0;
    string bank = "";

    printf("Number: ");
    credit_card = GetLongLong();

    // calculate number of digits
    for (long long int temporary = credit_card; temporary > 0;
                    temporary /= 10, digits++);

    // check if number length corresponds to valid format
    switch (digits)
    {
        case 13:
            if ( (credit_card / (long long int) pow(10, digits - 1) ) == 4)
            {
                bank = "VISA";
                break;
            }

        case 15:
            if ( ( (credit_card / (long long int) pow(10, digits - 2)) == 34 ) ||
                   (credit_card / (long long int) pow(10, digits - 2)) == 37)
            {
                bank = "AMEX";
                break;
            }

        case 16:
            if ( ( (credit_card / (long long int) pow(10, digits - 2)) >= 51) &&
                   (credit_card / (long long int) pow(10, digits - 2)) <= 55)
            {
                bank = "MASTERCARD";
                break;

            }
            else if ( (credit_card / (long long int) pow(10, digits - 1) ) == 4)
            {
                bank = "VISA";
                break;
            }
        // sets bank to invalid if it doesn't satisfy any of the standards.
        default:
            bank = "INVALID";
    }

    if (strcmp(bank, "INVALID") != 0)
    {
        // used for summing up the digits of the credit card number.
        int odd_sum = 0, even_sum = 0, total = 0;

        // looping through all the digits, decreasing credit_card on every iteration
        for (int i = 1; i <= digits; i++, credit_card /= 10)
        {
            // gets the last digit of credit_card
            int digit = credit_card % 10;

            // performs the necessary tasks on every other digit, starting from the next-to-last one.
            if (i % 2 == 0)
            {
                int prod_dig_sum = 0;

                digit *= 2;
                prod_dig_sum += (digit % 10) + (digit / 10);
                even_sum += prod_dig_sum;
            }

            else
            {
                odd_sum += digit;
            }
        }

        total = odd_sum + even_sum;

        if (total % 10 == 0)
        {
            printf("%s\n", bank);
            return 0;
        }

        else
        {
            printf("INVALID\n");
            return 0;
        }
    }

    printf("INVALID\n");
    return 0;
}

Notes:

The cs50.h library is custom-made by the Harvard staff
the function GetLongLong() is part of that library, and I currently don't know all the details of how this function works
This assignment is fairly early in the course, so we are only supposed to use the most basic tools of C. Basically, only conditions and loops.
I focused mainly on code readability, sometimes on expense of exactness

Having said all that, I am wondering if there is maybe a more elegant way to do all the checking involved in my program. The way it is now, I think I may have over-complicated the process a bit.

Avoid floating point
Actually I would advise the opposite of what @Xean said.  I would completely remove all floating point from your program, including any calls to pow and log.   I prefer your original loop to compute the number of digits.
The reason I don't like using floating point is that you might mysteriously get erroneous results due to roundoff errors.  For example, on my computer, when I do this:
val = pow(9, 17);

I get this result, which is off by one:
16677181699666568 (actual value should be one higher)

For your program, you could either write a simple function to compute the powers of ten you need, or even hardcode the four values you need since you are only using powers 12, 13, 14, and 15.


Trie Tree match string containing ? and * Using Python 2.7 and here is my code which implement trie match for a whole word, and also support if the whole word contains ? (match any one character) or * (any zero or more characters).

My question is wondering if any improvement I can make (in terms of performance) and also if any code functional bugs, please feel free to point out.

from collections import defaultdict

class TrieNode:
    def __init__(self):
        self.children = defaultdict(TrieNode)
        self.isEnd = False

    def insert(self, source):
        if not source:
            return
        node = self
        for s in source:
            node = node.children[s]
        node.isEnd = True

    def search(self, source):
        if not source:
            return self.isEnd == True
        if source[0] == '?':
            for (ch,node) in self.children.items():
                if node.search(source[1:]) == True:
                    return True
            return False
        elif source[0] == '*':
            for (ch, node) in self.children.items():
                if ((node.search(source[1:]) == True) or
                    (node.search(source) == True)):
                    return True
            return False
        else:
            if source[0] in self.children:
                node = self.children[source[0]]
                return node.search(source[1:])
            else:
                return False
if __name__ == "__main__":
    root = TrieNode()
    root.insert('abc')
    root.insert('acd')
    root.insert('bcd')
    print root.search('acd') # True
    print root.search('acdd') # False
    print root.search('aaa') # False
    print root.search('a?d') # True
    print root.search('a*d') # True
    print root.search('ad*') # False
    print root.search('*c*') # True
    print root.search('*c?') # True
    print root.search('*a?') # False

1. Review

The implementation with defaultdict is very elegant. I like it.
The code is not portable to Python 3 because of the use of the print statement. It would be easy to make it portable using from __future__ import print_function.
Set-like data structures contain elements, so element would be a better variable name than source.
It's inconvenient to create a trie from some other data structure like a list of strings: you have to call the insert method for each element you want to add. It would be better if the constructor took an optional iterator and inserted all the elements, just like the constructors for other collections in Python.
Instead of self.isEnd == True, just write self.isEnd, and similarly for other comparisons against True.
The attributes children and isEnd are only intended for use by the TrieNode class itself. It's conventional to give such attributes names starting with _.
The code for insert starts like this:

if not source:
    return

which means that if you try to add the empty string to the trie, it apparently succeeds, but in fact the empty string was not added. This is misleading. If you really want to prevent the caller adding the empty string, then raise an exception:

if not source:
    raise ValueError("empty string not supported")

But this seems inconvenient to me, so instead I suggest removing these two lines and allowing the empty string as an element. But this reveals a problem with the search method:

>>> root = TrieNode()
>>> root.insert('')
>>> root.search('*')
False

To fix this, revise the search method as follows:

elif source[0] == '*':
    if node.search(source[1:]):
        return True
    for (ch, node) in self.children.items():
        if node.search(source) == True:
            return True
    return False

(but see below for further improvements to this code).
In this code:

for (ch,node) in self.children.items():
    if node.search(source[1:]) == True:
        return True
return False

you don't make any use of ch, so it would be simpler to write:

for node in self.children.values():
    if node.search(source[1:]):
        return True
return False

which is the same as:

return any(node.search(source[1:]) for node in self.children.values())

Instead of computing source[1:] on each iteration of the loop, compute it once and remember it in a local variable:

rest = source[1:]
return any(node.search(rest) for node in self.children.values())

But better still, reorganize the search so that it remembers the current index into the search term, then you don't have to construct all these substrings. See below for how this might be implemented.
The interface to the search method makes it impossible to tell if strings containing wildcards are elements of the trie. Consider:

>>> import random
>>> root = TrieNode()
>>> root.insert(random.choice(['?', 'x']))
>>> root.search('?')
True

Is '?' a member of the trie or not? It would make sense to provide an alternative search method that doesn't use wildcards: for example the caller could write 'element' in trie instead of trie.search('element'). This requires a __contains__ method, which is straightforward to implement since it doesn't need to consider wildcards:

def __contains__(self, element):
    node = self
    for k in element:
        if k not in node._children:
            return False
        node = node._children[k]
    return node._end

Now you can accurately determine membership for strings containing wildcards:

>>> '?' in root
False

The search method only tells you whether some element in the trie matched the search term, and doesn't tell you which element or elements match. When the search term contains wildcards, it would be more useful if the search generated the matching elements. For example, if you have added the words in a dictionary to a trie, then you'd like to be able to query 'p?t' and get out 'pat', 'pet', 'pit', 'pot', 'put'. See the revised code in §2 below, and further discussion in §3.
The data structure presents a set-like interface (it represents a collection of distinct strings where you can add new elements and test elements for membership). It would therefore make sense to design the interface so that it uses the same method names as Python's built-in sets, that is, add instead of insert, and to implement more of the set interface, for example __iter__, __len__, __or__, __and__ and so on.

The easiest way to do this is to inherit from collections.abc.Set. The idea is that your class implements the __contains__, __iter__ and __len__ methods, and collections.abc.Set class implements the other set methods in terms of these. See the revised code in §2 below.
The code has test cases, but it's hard to check that they are correct: you have to carefully compare the output against the expected output. It would be better to get the computer to do the work here, using the features in the unittest module.

2. Revised code

from __future__ import print_function
from collections import defaultdict
from collections.abc import Set

class TrieNode(Set):
    """A set of strings implemented using a trie."""

    def __init__(self, iterable=()):
        self._children = defaultdict(TrieNode)
        self._end = False
        for element in iterable:
            self.add(element)

    def add(self, element):
        node = self
        for s in element:
            node = node._children[s]
        node._end = True

    def __contains__(self, element):
        node = self
        for k in element:
            if k not in node._children:
                return False
            node = node._children[k]
        return node._end

    def search(self, term):
        """Return the elements of the set matching the search term, which may
        include wildcards ? (matching exactly one character) and *
        (matching zero or more characters).

        """
        results = set() # Set of elements matching search term.
        element = []    # Current element reached in search.
        def _search(m, node, i):
            # Having just matched m, search for term[i:] starting at node.
            element.append(m)
            if i == len(term):
                if node._end:
                    results.add(''.join(element))
            elif term[i] == '?':
                for k, child in node._children.items():
                    _search(k, child, i + 1)
            elif term[i] == '*':
                _search('', node, i + 1)
                for k, child in node._children.items():
                    _search(k, child, i)
            elif term[i] in node._children:
                _search(term[i], node._children[term[i]], i + 1)
            element.pop()
        _search('', self, 0)
        return results

    def __iter__(self):
        return iter(self.search('*'))

    def __len__(self):
        return sum(1 for _ in self)

3. Efficiency

The Set class requires implementations of __iter__ and __len__ so in the code above I implemented them as simply as possible to avoid making the answer too complicated. But if this is going to be used for anything serious then it would be a good idea to provide specialized implementations with better performance. The __iter__ method could be implemented like this, using the stack of iterators pattern:

def __iter__(self):
    element = ['']
    stack = [iter([('', self)])]
    while stack:
        for k, node in stack[-1]:
            element.append(k)
            if node._end:
                yield ''.join(element)
            stack.append(iter(node._children.items()))
            break
        else:
            element.pop()
            stack.pop()

And the __len__ method can be made \$O(1)\$ if you maintained in each node a count of the number of elements in the trie rooted at that node:

def __init__(self, iterable=()):
    self._children = defaultdict(TrieNode)
    self._end = False
    self._len = 0
    for element in iterable:
        self.add(element)

def add(self, element):
    node = self
    for s in element:
        node = node._children[s]
    if not node._end:
        node._end = True
        node._len += 1
        node = self
        for s in element:
            node._len += 1
            node = node._children[s]

def __len__(self):
    return self._len

I made the search method return a set of elements matching the search term. It would be more general if this method generated the matching elements one at a time. This would more efficiently support use cases such as finding any element matching a search term (by stopping the iteration after the first result), or determining if there is a unique element matching a search term (by stopping the iteration after the second result).

In Python 3 it's easy to implement this using the yield from statement, but in Python 2 you have to write:

for result in _search(...):
    yield result

which is rather verbose. But see below for an alternative.
The implementation of the search method is inefficient in the way it handles the * wildcard. The problem is that when the current character in the search term is *, the search descends the trie twice at each node (once having consumed the *, once not). This means that each node in the trie may be visited many times in the course of the search.

It is possible to revise the code so that each node in the trie is visited at most once, by modifying the search method so that instead of considering a single index i in the search term, it considers a set of indexes.

Here's one way to implement this, again using the stack of iterators pattern:

def search(self, term):
    """Generate the elements of the set matching the search term, which
    may include wildcards.

    """
    def _add(indexes, i):
        # Add i to set of indexes into term, taking account of wildcards.
        while i < len(term) and term[i] == '*':
            indexes.add(i)
            i += 1
        indexes.add(i)

    indexes = set()
    _add(indexes, 0)
    if self._end and len(term) in indexes:
        yield ''

    indexes_stack = [indexes] # Stack of sets of indexes into term.
    element = ['']            # Current element reached in search.
    iter_stack = [iter(self._children.items())] # Stack of iterators.

    while iter_stack:
        for k, node in iter_stack[-1]:
            new_indexes = set()
            for i in indexes_stack[-1]:
                if i >= len(term):
                    continue
                elif term[i] == '*':
                    _add(new_indexes, i)
                elif term[i] == '?' or term[i] == k:
                    _add(new_indexes, i + 1)
            if new_indexes:
                element.append(k)
                if node._end and len(term) in new_indexes:
                    yield ''.join(element)
                indexes_stack.append(new_indexes)
                iter_stack.append(iter(node._children.items()))
                break
        else:
            element.pop()
            indexes_stack.pop()
            iter_stack.pop()