Hash Map Library for any type (Void Pointers) This is a hash map library for C that stores void* pointers with char* strings (null terminated) for keys

//  ColiisionHashMap
//  Copyright © 2015 Leo Dastur.
//

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

typedef struct HashMapNode {
    char* key;
    void* data;
} HashMapNode;

typedef struct HashLocation {
    HashMapNode** nodes;
    int numberNodes;
} HashLocation;

typedef struct HashMap {
    int mapSize;
    struct HashLocation** hasharray;

} HashMap;

HashMap* newHashMap(int mapSize) {
    if (mapSize<=0) {
        abort(); //Zero Mapsize is bad
    }
    HashLocation** array= calloc(mapSize, sizeof(HashLocation*));

    for (int i = 0; i<mapSize; i++) {
        array[i] = malloc(sizeof(HashLocation));
        array[i]->nodes=NULL;
        array[i]->numberNodes=0;
    }

    HashMap* aHashMap = calloc(1, sizeof(HashMap));
    aHashMap->mapSize = mapSize;
    aHashMap->hasharray = array;
    return aHashMap;
}

int keyHash (HashMap* aHashMap, char* key, int key_len) {
    int charSum=0;
    int i;
    for (i=0; i<key_len; i++) {
        charSum+=key[i];
    }
    return charSum % aHashMap->mapSize;
}

int getStringlen(char* astring) {
    strlen(astring);
/*
    int counter=0;
    while (astring[counter]!='\0') {
        counter++;
    }
 */
    return (int) strlen(astring); /*counter;*/

}

void mapAdd(HashMap* aHashMap, char* key, char* data) {
    int key_len = getStringlen(key);
    int hashed_key = keyHash(aHashMap, key, key_len);

    HashMapNode* dataNode = malloc(sizeof(HashMapNode));
    dataNode->key=key;
    dataNode->data=data;

    HashLocation* theHashlocation = aHashMap->hasharray[hashed_key];
    if (theHashlocation->nodes==NULL) {
        theHashlocation->nodes = calloc(1, sizeof(HashMapNode*));
        theHashlocation->nodes[0]=dataNode;
        theHashlocation->numberNodes=1;
    } else {
        realloc(theHashlocation->nodes, theHashlocation->numberNodes+1);
        theHashlocation->nodes[theHashlocation->numberNodes] = dataNode;
        theHashlocation->numberNodes++;
    }

}

void* mapGet(HashMap* aHashMap, char* key) {
    int key_len = getStringlen(key);
    int hashed_key = keyHash(aHashMap, key, key_len);

    HashLocation* theHashLocation = aHashMap->hasharray[hashed_key];
    if (theHashLocation->nodes==NULL) {
        return NULL;
    }
    if (theHashLocation->numberNodes==1) {
        HashMapNode* dataNode = theHashLocation->nodes[0];
        return dataNode->data;
    }
    else {
        //Get Node Array, Search Node Array for key, return that node's data. Return NULL if key is not found.
        HashMapNode** nodeArray = theHashLocation->nodes;
        int array_len = theHashLocation->numberNodes;
        HashMapNode* dataNode = NULL;

        for (int i = 0; i < array_len; i++) {
            if (!strcmp(nodeArray[i]->key, key) ) {
                dataNode = nodeArray[i];
                // printf("Found Key");
                break;
            }
            //else printf("Didn't Find it yet");
        }
        if (dataNode==NULL) {
            return NULL;
        }
        else return dataNode->data;
    }
}

Is there a more efficient way of handling collisions, and
what else could I do to make this a real-world usable map?

Bug

Suppose there are 2 keys that hash to the same bucket (i.e. HashLocation).  If you add key #1 and then search for key #2, mapGet() will return the data for key #1 instead of returning NULL.  The problem is here:

if (theHashLocation->numberNodes==1) {
    HashMapNode* dataNode = theHashLocation->nodes[0];
    return dataNode->data;
}

Here, you assume that if there is only one node in the bucket, that it must match the key you are looking for.  But you need to still compare the key to see if it matches.

Unnecessary array of pointers

Your hasharray inside HashMap is an array of pointers to HashLocations.  There's no need for hasharray to be an array of pointers.  It should simply be an array of HashLocations instead.  Using one contiguous array is simpler (less allocations) and better for cache locality.

Bad reallocation strategy

The way you resize each bucket when it grows is by reallocating by one element larger.  This will use \$O(n^2)\$ time because of all the recopying required.  You should use a better reallocation strategy such as doubling the size every time and keeping track of how many elements are being used.

Poor hash function

Your hash function simply adds each character of the string together.  This is a poor hash function which will result in many hash collisions.  You should try something different such as rotating the hash by 5 bits every time you add the next character in.  Here is a StackOverflow question about string hashes, and a short webpage describing a few string hashes, linked from that question.


PHP code to show a PHP page I'm trying to avoid manually updating a whitelist of pages, so I wrote the following that I might use, but I'd like my fellow, more experienced, developers to let me know if this is safe. By safe, I mean that it won't allow the inclusion or execution of arbitrary code. Is it safe from directory traversal and other similar vectors?

<?php
$current_page = 'default_page';
//Get list of acceptable pagenames:
$found_pages = [];
if ($handle = opendir('pages')) {
    while (false !== ($entry = readdir($handle))) {
        if ($entry != "." && $entry != "..") {
            $found_pages[]= $entry;
        }
    }
    closedir($handle);
}

if(isset($_GET)){
  $requested_page=reset($_GET); //just need value of first GET element, regardless of name.
  if(in_array($requested_page.'.php',$found_pages)){
    $current_page=$requested_page;
  }
}

include('pages/' . $current_page . '.php');

Yes, I believe that will accomplish what you are trying to do.  However, I think it can also be streamlined pretty substantially.  Some general pointers:

Looping through the directory to get all files on all page requests is going to be pretty inefficient in the long-run.  File operations are relatively expensive, because they require disk-seeks.  Obviously this is less critical for a low-traffic system with a small number of files.
You are returning the default page if an invalid page is requested.  I think it would be better to either show an access-denied error (as such a request might signify someone trying to circumvent restrictions in your system) or at least a 404 (which would be more friendly towards legitimate users).  Either way, you should stop executing when an invalid page is found.
You could fix the first issues if you have a database in your system. Instead of directly checking the file path, you could record the list of allowed pages in your database, check against the DB, and then load the page in much the same way you currently are.  The advantage of such a solution is that you could separate your public paths from your filenames: the database could record the "url" of the file which you lookup against, and then fetch the actual filename out of the database.  Of course if you do that, you pretty much just made a basic routing system.
Is there anything that prevents users from going directly to the pages?  From the looks of it your pages are just a sub-directory of your public system.  That may suggest room for possible security vulnerabilities.

As an alternate way of accomplishing exactly what you are doing without reading the full directory, you can use realpath.  It returns the normalized absolute URL to a file, or nothing if the file doesn't exist.  You can then check and see if what is returned by realpath actually lives in your application directory.  That will make for fewer disk seeks and will allow your performance to remain (relatively) constant even if the number of files grows.  Here is a quick example:

if(isset($_GET)){
    $pages_dir='/path/to/pages/folder/pages/';
    $requested=reset($_GET);
    $path=realpath($pages_dir . $requested);
    if(!$path || substr($path, 0, strlen($pages_dir)) != $pages_dir){
        show_404();
        exit;
    }
}

include($path);

To reiterate, you build the path assuming the file lives in the destination directory.  realpath will then resolve the path, accounting for directory traversals.  I.e. if $_GET contained ../../../../../../../etc/passwd then you would pass this to realpath: /path/to/pages/dir/pages/../../../../../../../etc.passwd and realpath would return simply /etc/passwd.  You then check what realpath returned to see if your results are still in the proper directory.  If it is, then you're good.


Check if a game of Tic-Tac-Toe has a winner I've implemented a Tic Tac Toe game with a public method checkWin() to check if the game has a winner. This is my code.

public class TicTacToe {
    int size;
    int [][] board;

    public TicTacToe(int size){
        this.size = size;
        board = new int[size][size];
        for (int i = 0; i <size ; i++) {
            for (int j = 0; j <size ; j++) {
                board[i][j] = -1;
            }
        }

    }

    public void markCross(int row, int col){
        if(row >= size || col >= size) {
            throw new IllegalStateException("Invalid grid point");
        }
        board[row][col] = 1;

    }

    public void markZero(int row, int col){
        if(row >= size || col >= size){
            throw new IllegalStateException("Invalid grid location");
        }
        board[row][col] = 0;
    }

    public boolean checkWin(){
       int countOnes = 0;
        int countZeros = 0;
        for (int i = 0; i < size; i++) {
            for (int j = 0; j < size; j++) {

                if(board[i][j] == 0) countZeros++;
                else if(board[i][j] == 1)countOnes++;
            }
            if(countZeros == size || countOnes == size) return true;
            break;
        }
        countOnes = 0;
        countZeros = 0;

        //check diagonal right

        for (int i = 0, j= 0; i <size ; i++, j++) {
            if (board[i][j] == 0) countZeros++;
            else if(board[i][j] == 1) countOnes++;
        }
        if(countOnes == size || countZeros == size) return true;

        //check diagonal left
        for (int i = size-1, j= size-1; i >= 0 ; i--, j--) {
            if (board[i][j] == 0) countZeros++;
            else if(board[i][j] == 1) countOnes++;
        }
        if(countOnes == size || countZeros == size) return true;

        return false;

    }
}

I would suggest this instead :

public boolean checkWin(){
    boolean diagLTRHasWinner = true;
    boolean diagRTLHasWinner = true;
    for(int j = 1; j < this.size; j++){
        boolean columHasWinner = true;
        boolean rowHasWinner = true;
        for(int i =0; i < this.size; i++){
            columHasWinner = columHasWinner && (board[i][j] == board[i][j-1]);
            rowHasWinner = rowHasWinner && (board[j][i] == board[j-1][i]);
        }
        if ((rowHasWinner && board[j][0] != -1) || (columHasWinner && board[0][j] != -1)){
            return true;
        }
        diagLTRHasWinner = diagLTRHasWinner  && (board[j][j] == board[j-1][j-1]);
        diagRTLHasWinner = diagLTRHasWinner  && (board[this.size - j][j] == board[this.size - j+1][j-1]);
    }
    return (diagRTLHasWinner && board[0][0] != -1) || (diagLTRHasWinner && board[this.size-1][0] != -1);
}

In order to reduce the number of loops, and objects allocated.
Besides the new code does not even count the element in the board, it just checks if we have a valid sequence of X or O, no matter which one, neither how many! It simply detects the end of the game !


Handling Null values (and equivalents) routinely in Python I've found the following code invaluable in helping me 'handle' None values including "whitespace" characters that should be treated as None based on the situation.   I have been using this code for quite some time now:

class _MyUtils:

    def __init__(self):
        pass

    def _mynull(self, myval, myalt, mystrip=True, mynullstrings=["", "None"], mynuminstances=(int, float)):
        # if the value is None, return the alternative immediately.

        if myval is None:
            return myalt

        # if the value is a number, it is not None - so return the original
        elif isinstance(myval, mynuminstances):
            return myval

        # if the mystrip parameter is true, strip the original and test that
        else:
            if mystrip:
                testval = myval.strip()
            else:
                testval = myval

            # if mynullstrings are populated, check if the upper case of the
            # original value matches the upper case of any item in the list.
            # return the alternative if so.
            if len(mynullstrings) > 0:
                i = 0
                for ns in mynullstrings:
                    if ns.upper() == testval.upper():
                        i = i + 1
                        break
                if i > 0:
                    return myalt
                else:
                    return myval
            else:
                return myval

def main():
    x = _MyUtils()
    print(x._mynull(None, "alternative_value", True, [""]))

if __name__ == '__main__':
    main()

The code requires an input, an alternative to provide if input is found to be Null, whether to 'strip' the input during testing (if not a number), values to treat as 'equivalent' to None and types of number instances to determine if the input is numeric (and hence not none).

Essentially, too many processes that we run depend upon not having None values in the data being processed—whether that be lambda functions, custom table toolsets, etc.  This code gives me the ability to handle None values predictably, but I am sure there is a better approach here. Is there a more Pythonic way of doing this? How can this code be improved? How would others approach this problem?

Generally I don't think you should have a class for this functionality. There's no state and no particular meaning to MyUtils object here. You can make this into a long function in whatever module you deem appropriate in your codebase.

I think this function as written is a code smell. It 1) doesn't cover a whole lot of types and 2) implies that where you're using it you're not going to have even a rough idea of what type of data you're expecting. In most cases you will have some idea, and even then it's not usually a good idea to do explicit type checking.

Where you're using this for numbers you can replace it with myval if myval is not None else mydefault.

A function like this may be more useful for strings, for which there are a wider range of essentially empty values. Perhaps something like this

def safe_string(s, default="", blacklist=["None"]):
    if s is None or len(s.strip()) == 0:
        return default
    if s.upper() in [b.upper() for b in blacklist]:
        return default
    return s


Combining List.ForEach and List.Zip I have three lists, and I need to operate on the ith element of each list simultaneously.

private void TripleForEach<T1, T2, T3>(IEnumerable<T1> a1, IEnumerable<T2> a2, IEnumerable<T3> a3, Action<T1, T2, T3> x)
{
    a3.Zip(a2, (t3, t2) => Tuple.Create(t2, t3)).Zip(a1, (t23, t1) => Tuple.Create(t1, t23.Item1, t23.Item2)).ToList().ForEach(z => x(z.Item1, z.Item2, z.Item3));
}

Any better way to do this? Creating and pulling apart those temporary Tuples smells bad to me.

I agree, the temporary tuples are a code smell. They carry a performance overhead, but worse, they obscure the purpose of the code. Here is a way of achieving the goal by working directly on the enumerators.

private static void TripleForEach<TFirst, TSecond, TThird>(
        IEnumerable<TFirst> first,
        IEnumerable<TSecond> second,
        IEnumerable<TThird> third,
        Action<TFirst, TSecond, TThird> action)
{
    using (IEnumerator<TFirst> e1 = first.GetEnumerator())
    using (IEnumerator<TSecond> e2 = second.GetEnumerator())
    using (IEnumerator<TThird> e3 = third.GetEnumerator())
    {
        while (e1.MoveNext() && e2.MoveNext() && e3.MoveNext())
        {
            action(e1.Current, e2.Current, e3.Current);
        }
    }
}


Python - Hexadecimal to Decimal conversions I have written a python 2 program for a school project to convert from hexadecimal to decimal without using built-in functions and vice versa in python. I would like to know how I can make my code more concise - specifically, the "d_func" function.

d_dict = {"0" : 0, "1" : 1, "2" : 2, "3" : 3, "4" : 4, "5" : 5, "6" : 6, "7" : 7, "8" : 8, "9" : 9, "A" : 10, "B" : 11, "C" : 12, "D" : 13, "E" : 14, "F": 15}

def d_func(digit, mode):
    if mode == 1:
        for x in range(len(d_dict.keys())):
            if digit == list(d_dict.keys())[x]:
                return x
    else:
        for y in range(len(d_dict.values())):
            if digit == list(d_dict.values())[y]:
                return list(d_dict.keys())[y]

def hd_func(h_num):
    d_num, p = 0, 0

    for digit in range(len(h_num), 0, -1):
        d_num = d_num + 16 ** p * d_func(h_num[digit - 1], 1)
        p += 1

    return str(d_num)

def dh_func(d_num):
    f_list, h_num, p = [], "", 0

    while d_num > 0:
        f_list.append(d_num % 16)
        d_num //= 16

    for f in f_list[::-1]:
        h_num += d_func(f, 2)

    return h_num

func_dict = {"h": hd_func, "d": dh_func}
u_choice, u_num = input("Enter [H] for Hexadecimal to Decimal, [D] for Decimal to Hexadecimal"), input("Enter the number: ")
print(func_dict[u_choice.lower()](u_num))

First -- document your code!  I read through your d_func function and here's my attempt at writing a docstring for what it does, with Python-2-compatible type hints.  Hopefully I got it right.  :)

def d_func(digit, mode):
    # type: (Union[str, int], int) -> Union[int, str, None]
    """Mode 1: give the int value of the hex digit.
    Other modes: give the hex digit of the given value.
    Returns None if you give it a bad value, I guess?
    """
    if mode == 1:
        for x in range(len(d_dict.keys())):
            if digit == list(d_dict.keys())[x]:
                return x
    else:
        for y in range(len(d_dict.values())):
            if digit == list(d_dict.values())[y]:
                return list(d_dict.keys())[y]
    return None  # stumblebum

Right off the bat: this should not be one function.  (The fact that it can return either an int or a str depending on its input type is a good clue!)  You should have one function that converts value to digit and another that converts digit to value; there's no value at all to having one function that does completely different things depending on a flag you pass it.

Second: yes, this can be a lot simpler.  I think you could implement d_func as follows:

d_dict = {"0" : 0, "1" : 1, "2" : 2, "3" : 3, "4" : 4, "5" : 5, "6" : 6, "7" : 7, "8" : 8, "9" : 9, "A" : 10, "B" : 11, "C" : 12, "D" : 13, "E" : 14, "F": 15}
d_dict_reverse = {v: d for d, v in d_dict.iteritems()}  # this is just building a new dictionary with the keys and values swapped

def d_func(digit, mode):
    # type: (Union[str, int], int) -> Union[int, str]
    if mode == 1:
        return d_dict[digit]
    else:
        return d_dict_reverse[digit]

At that point, it doesn't need to be a function at all, because you're just doing a simple dictionary lookup.  Give your dictionaries reasonable names that say what they do:

digit_to_value = {
    "0" : 0, "1" : 1, "2" : 2, "3" : 3, "4" : 4, "5" : 5, "6" : 6, "7" : 7,
    "8" : 8, "9" : 9, "A" : 10, "B" : 11, "C" : 12, "D" : 13, "E" : 14, "F": 15
}
value_to_digit = {v: d for d, v in digit_to_value.iteritems()}

and then instead of:

d_num = d_num + 16 ** p * d_func(h_num[digit - 1], 1)

do:

d_num = d_num + 16 ** p * digit_to_value[h_num[digit - 1]]

and instead of:

    for f in f_list[::-1]:
        h_num += d_func(f, 2)

do:

    for f in f_list[::-1]:
        h_num += value_to_digit[f]

With this approach, not only do you not have to write a function, but unlike your function the dictionary will automatically raise a KeyError if you provide the wrong kind of input, e.g. if you do value_to_digit[100] or digit_to_value[1].  Raising an error ASAP when you mess up (aka "fail fast") is good, because it makes it easier to figure out exactly where your bug is.  :)


Am I dynamic filtering correctly in Django? I needed a way to get URL parameters and put them and their values into a query set's .filter() if they exist. After a day of piecing together information from the Interwebs, I have a working solution to my problem. However, is it the best way to do this? I feel it can be improved or streamlined, but my tiny brain is failing me:

In utilities.py:

def get_filters(url, allowed):
    query = parse.urlsplit(url).query
    pack = dict(parse.parse_qsl(query))
    translated = {}

    for pk, pv in pack.items():
        for ak, av in allowed.items():
            if pk == ak:
                translated[av] = pv

    return translated

In views.py:

from utilities import get_filters

people = Person.objects.all()
url = request.get_full_path()

allowed_filters = {
    'location': 'person_location',
    'role': 'person_role',
}

filters = get_filters(url, allowed_filters)

if filters:
    filtered_query = people.filter(**filters)
else:
    filtered_query = people

if ('search' in request.GET) and request.GET['search'].strip():
    query_string = request.GET['search']
    query_search = get_query(query_string, ['person_first_name', 'person_last_name'])
    query_results = filtered_query.filter(query_search)
else:
    query_results = filtered_query

It does everything I need it to: if a URL parameter is in the allowed list, it takes that parameter and value, packs it into a dictionary, and shoves it into filter() to be unpacked by the interpreter.

This way whenever I want to add a new filter, I just add it in the allowed list and I don't have to touch anything else. I did it this way so I could use an alias like special_id=3 in the URL to target a relationship like person__special__special_id=3 without the URL getting cluttered like the Django admin side does.

Any suggestions for improving this?

You can change the first method to

def get_filters(url, allowed):
    query = parse.urlsplit(url).query
    pack = dict(parse.parse_qsl(query))
    translated = {}

    for pk, pv in pack.items():
        if pk in allowed:
            translated[allowed[pk]] = pv

    return translated

Remove this condition, it's useless.

if filters:
    filtered_query = people.filter(**filters)
else:
    filtered_query = people

and just use

filtered_query = people.filter(**filters)

Maybe add search to the allowed filters? and improve the method to handle multiple values for a specific key.

allowed_filters = {
    'location': 'person_location',
    'role': 'person_role',
    'search': ['person_first_name', 'person_last_name']
}

```


Vampiric text-based adventure game Here is my vampire based text adventure game.
I recently started learning Python and Googled for some good projects to practice with. I wrote all this code myself after checking out some examples on the internet.
import time #added to enable pause

#A text based dracula survival game

#possible answers
answer_A = ['A', 'a']
answer_B = ['B', 'b']
answer_C = ['C', 'c']
yes = ['yes', 'Yes', 'y', 'Y']
no = ['no', 'No', 'n', 'N']

#story begins here
def intro():
    print('You wake up in front of a castle, you have no idea where you are or why you are here.'
    'You suddenly see a colony of bats fly away.' 'You turn back and see a vampire figure appear out of nowhere. You will:')
    time.sleep(1)
    print('''    A. Ask for direction.
    B. Run inside the castle.
    C. Run to the cave.''')
    choice = input('>>>  ')
    if choice in answer_A:
        print('\n Well you dont ask vampires for directions. \n\nRip')
    elif choice in answer_B:
        option_castle()
    elif choice in answer_C:
        option_cave()
    else:
        print("That's not an option idiot")
        intro()

def option_castle():
    print('You ran inside the castle and see a glass front cubboard with garlic inside.You hear the Vampire coming,''You will: ')
    time.sleep(1)
    print('''    A. Take the garllic to scare the vampire.
    B. Hide
    C. Escape from backdoor.''')
    choice = input('>>>  ')
    if choice in answer_A:
        print('This is not a story book, what are you doing? Making salad? \n\n RIP')
        option_death()
    elif choice in answer_B:
        print("This is not hide n'seek \n\n RIP" )
        option_death()
    elif choice in answer_C:
        option_abdvilllage()
    else:
        print('Not an option idiot')
        option_castle()

def option_cave():
    print('You ran inside a dark cave, you were not sure if its a good idea or not but in there you see a shiny silver dagger.' 'Hurry bats are coming: ')
    time.sleep(1)
    print('''    A. You pick up the dagger and fight.
    B. You pick up the dagger and hide.
    C. You run.''')
    choice = input('>>>  ')
    if choice in answer_A:
        print('You picked the silver dagger and stood there like a fearsome warrior. The vampire attacked you but you were cunning and avoiding its attack stabbed the vampire right in its heart. Well done vampire slayer, you may live.')
    elif choice in answer_B:
        print("Cowards don't get to fight and live. \n\n RIP")
        option_death()
    elif choice in answer_C:
        option_abdvilllage()
    else:
        print('not an option idiot')
        option_cave()

def option_abdvilllage():
    print('You ran towards an abandoned village in the open. The bats are coming faster than before, you will: ')
    time.sleep(1)
    print('''    A. Hide
    B. Pick a wood to stab the vampire
    C. Enter the cave''')
    choice = input('>>>  ')
    if choice in answer_A:
        print('You hid in a hut and well it worked, you were lucky and the sun rose killing the vampire. You were a coward but a lucky one.')
    elif choice in answer_B:
        print("For real? How can a piece of wood kill an immortal blood sucking human size bat? \n\n RIP")
        option_death()
    elif choice in answer_C:
        option_cave()
    else:
        print('not an option idiot')
        option_abdvilllage()

def option_death():
    choice = input('Do you want to play again? Yes or No  ')
    if choice in yes:
        intro()
    else:
        print('very well')

play = input('Do you want to play? Y or N  ')
if play == 'Y'.lower():
    intro()
else:
    print('very well')

Don't ask if the user wants to play the first time
If the user started your game, then of course they want to play it, otherwise they wouldn't have started it to begin with. So don't ask and go to the intro immediately. If they started your game in error, they can always quit it by closing the window, pressing control-C or something like that.
You will overflow the stack, eventually
For every action that does not end the game, you just call another function, but you don't return from a function. This means your call stack will grow indefinitely. With the gigabytes of RAM we have in our computers nowadays, you might not notice this mistake, but on the eight-bitters of the previous century, your game would run out of memory very quickly because of this.
Generally, you want to have a main loop that handles the input, and that advances the state based on your input. For example, you could write it like so:
def intro():
    print('You wake up...')
    ...
    choice = input('>>> ')
    if choice in answer_A:
        print("...");
        return "game_over"
    elif choice in answer_B:
        return "castle"
    ...

def main_loop():
    state = "intro"

    while state != "end":
        if state == "intro":
            state = intro()
        elif state == "castle":
            state = option_castle()
        ...
        elif state == "game_over":
            again = input('Do you want to play again? Y or N  ')
            if again in yes:
                state = "intro"

main_loop()

The above is just an illustration of the idea. To do it properly, you would probably use an enum for the state, and possibly have a map of states to functions so you can simplify the whole loop. For example:
from enum import Enum

class State(Enum):
    INTRO = 1
    CASTLE = 2
    ...
    GAME_OVER = -1
    END = -2

def intro():
    ...

def option_castle():
    ...

def game_over():
    print('Game over.')
    again = input('Do you want to play again? Y or N  ')
    if again in yes:
        return State.INTRO
    else
        return State.QUIT

scenarios = {
    State.INTRO: intro,
    State.CASTLE: option_castle,
    ...
    State.GAME_OVER: game_over,
}

def main_loop():
    state = State.INTRO

    while state != State.END:
        state = scenarios[state]()

main_loop();

Consider not sleeping before showing possible choices
The calls to time.sleep(1) are not needed for the game, and just make it so the player has to wait unncessarily before being able to read the choices. I would just avoid this.
The game is quite rude
It might seem funny to you, but if I was a player, and accidentally typed in the wrong character, and got told that I was an idiot, my appreciation of the game would drop significantly. Also, being told that garlic doesn't work because that's just a story book thing is very annoying, since the whole concept of vampires comes from a story book to begin with.


Extract from typelist by runtime value I combined the ideas from this answer and a comment to this question to create a new instance of a type extracted from a type list. The index of the desired type within the type list is provided at run time, only.

#include <iostream>
#include <memory>

class Base
{
public:
    virtual void foo() = 0;
};

class A : public Base
{
public:
    void foo() override
    {
        std::cout << "A" << std::endl;
    }
};

class B : public Base
{
public:
    void foo() override
    {
        std::cout << "B" << std::endl;
    }
};

template <class... Types>
class type_list {};

using list_t = type_list<A,B>;

template <class... Types>
static constexpr std::size_t length = sizeof...(Types);

template <std::size_t idx, class... Types>
class extract
{
    static_assert(idx < sizeof...(Types), "index out of bounds");

    template <std::size_t i, std::size_t n, class... Rest>
    struct extract_impl;

    template <std::size_t i, std::size_t n, class T, class... Rest>
    struct extract_impl<i, n, T, Rest...>
    {
        using type = typename extract_impl<i + 1, n, Rest...>::type;
    };

    template <std::size_t n, class T, class... Rest>
    struct extract_impl<n, n, T, Rest...>
    {
        using type = T;
    };

public:
    using type = typename extract_impl<0, idx, Types...>::type;
};

template <std::size_t idx, class TypeList>
struct type_list_extract;

template <std::size_t idx, template <class...> class TypeList, class... Types>
struct type_list_extract<idx, TypeList<Types...>>
{
    using type = typename extract<idx, Types...>::type;
};

template <std::size_t idx, class TypeList>
using type_list_extract_t = typename type_list_extract<idx, TypeList>::type;

template<std::size_t idx, bool done = false, class... Types>
struct action_wrapper
{
    static std::unique_ptr<Base> get_instance_by_id(std::size_t id)
    {
        if (id == idx)
        {
            using type = type_list_extract_t<idx, Types...>;
            return std::make_unique<type>();
        }

        static constexpr auto cont = length<Types...> < idx + 1;
        return action_wrapper<idx + 1, cont, Types...>::get_instance_by_id(id);
    }
};

template<std::size_t idx, class... Types>
struct action_wrapper<idx, true, Types...>
{
    static std::unique_ptr<Base> get_instance_by_id(std::size_t id)
    {
        return nullptr;
    }
};

static std::unique_ptr<Base> get_instance_by_id(std::size_t id)
{
    return action_wrapper<0, false, list_t>::get_instance_by_id(id);
}

int main()
{
    std::unique_ptr<Base> ptr = get_instance_by_id(1);
    if (ptr)
    {
        ptr->foo();
    }
}

Prefer using \n over std::endl

Other than that, it looks like your code is rather clean and up-to-date style-wise.  I might suggest that the get_instance_by_id return the actual type, not the Base in case it gets used in a context where a specific one is known at compile time (or within other template code).


Position logo in middle of nav I've recently migrated away from floats and have started using flexbox to build my grids.

My navigation has 4 menu items. I'd like my logo to sit in the middle of these. To maintain a good page structure for SEO, I don't like the idea of putting my logo within a list item inside the nav.

Instead, I've built a solution that uses flexbox and a negative margin.

/*------------------------
Default grid
-------------------------*/
.flex {
  display: flex;
  flex-flow: row wrap;
  justify-content: space-between;
}

.col {
  flex: 1;
}

/*------------------------
Columns
-------------------------*/
.has-2-columns .col {
  flex: none;
}

.has-2-columns .col {
  width: 49%;
}

/*------------------------
Logo
-------------------------*/
.site-title {
  position: relative; /* Enables z-index */
  z-index: 1; /* Positions logo above .main-navigation */
  width: 80px;
  margin: 0 auto;
  background: #ccc;
}

/*------------------------
Main navigation
-------------------------*/
.main-navigation {
  position: relative;
  margin-top: -24px; /* Brings nav inline with .site-branding */
  z-index: 0; /* Ensures nav is below .site-branding */
}

.main-navigation ul {
  list-style: none;
  padding: 0;
  margin: 0;
}

.main-navigation ul li {
  display: inline-block;
}

.left-menu {
  text-align: left;
}

.left-menu li {
  margin-right: 24px;
}

.right-menu {
  text-align: right;
}

.right-menu li {
  margin-left: 24px;
}

Demo

Home

About

Contact

Store

Is there a better way? Would this solution cause issues across different devices when it comes to clicking / tapping the menu items or logo to go back home?

If you're going to use flex, use flex! Flex inherently helps with margin control.

Margins are great, but you're essentially saying, I want to learn flex, except when it's easier/convenient to go back to what I've been doing.

Some key topics of interest regarding your concerns:

  To maintain a good page structure for SEO...

Great! That's a good attitude to have. You should always follow the W3C specs for best practices of what content goes where. If you don't like reading long-winded documentation.. MDN always has a great synopsis of elements/attributes/rules/principals/best practices (usually in a blue box).

Buttttt having a header inside  tags isn't necessarily against standards, in fact, all flow content is permitted. ref

Additionally, in your circumstance, it acts as a main content link, so I would argue it belongs there.

However, keeping in mind the importance of a semantic elements purpose is a very good outlook imo.

Furthermore on this point, I can not tell you how many developers overlook accessibility. Not only does it help you with SEO, it absolutely is critical to how disabled persons rely on their devices. It often takes trivial amount of time to make a website accessible friendly, but developers generally don't consider it because either they themselves aren't disabled and it's in the back of their mind, they haven't had any complaints, they don't work for someone who cares to implement it. But that is beyond the scope of the question, either way you are on the right track with that mentality.

  I've recently migrated away from floats and have started using flexbox

Not a bad thing, will definitely make it easier for you, but try not to consider them synonyms. Float properties apply to all elements, even flex, but flex properties do not apply to all elements. With the following caveats:

float applies but no effect is taken if display = none
float applies but no effect is taken if element is flex

An example of why they are not synonymous is float:left for an  tag around text will allow that text to flow around the content, whereas flexbox is ... well a box and the text will maintain in it's rect bounds instead of wrapping around the box next to it.

An important thing to embrace with flex is that as soon as you set an element to display: flex the direct children of that container become flex items. So note the hierarchy there.

Take this example where 'C' is not a direct descendant of .wrapper, therefor it doesn't inherit properties.

.wrapper {
  display: flex;
  flex-flow: column nowrap;

  height: 175px; padding: 10px; border-color: red;
}

.b, .c {
   float: right;
   border-color: green;
}
.c {
  background-color: lightgrey;
}

/** irrelevant **/
div {
  height: 50px;
  border: 1px solid;
}
.a { border-color: blue; }

:::::: A :::::::

:::::: B ::::::

:::::: C ::::::

I bring that up, because navigation items are generally top-level children, so we can consider that in circumstances like margin-top: -24px. You can take the following snippet (unchanged from OP, other than color for effect) and see why it can be problematic real fast, and worse, it can easily create a domino effect where you will have to add margins to a lot of other things just to create synergy:

/*------------------------
Default grid
-------------------------*/

.flex {
  display: flex;
  flex-flow: row wrap;
  justify-content: space-between;
}

.col {
  flex: 1;
}

/*------------------------
Columns
-------------------------*/

.has-2-columns .col {
  flex: none;
}

.has-2-columns .col {
  width: 49%;
}

/*------------------------
Logo
-------------------------*/

.site-title {
  position: relative;
  /* Enables z-index */
  z-index: 1;
  /* Positions logo above .main-navigation */
  width: 80px;
  margin: 0 auto;
  background: #ccc;
}

/*------------------------
Main navigation
-------------------------*/

.main-navigation {
  position: relative;
  margin-top: -24px;
  /* Brings nav inline with .site-branding */
  z-index: 0;
  /* Ensures nav is below .site-branding */
}

.main-navigation ul {
  list-style: none;
  padding: 0;
  margin: 0;
}

.main-navigation ul li {
  display: inline-block;
}

.left-menu {
  text-align: left;
}

.left-menu li {
  margin-right: 24px;
}

.right-menu {
  text-align: right;
}

.right-menu li {
  margin-left: 24px;
}

.site-header {
  background: pink;
}

Demo

Home

About

Contact

Store

So that is why I say, if you're going to use flex, then use flex! Take advantage of it. Let it do all this work and calculations for you. Additionally, I don't know if you have begun plans for this or not, but this isn't really responsive friendly as it stands now.

TLDR;

Revisiting requirements

  My navigation has 4 menu items. I'd like my logo to sit in the middle of these

The way you have set it up, as you can see get's the job done. But they are not apart of the same parent, therefor they can't take advantage of it's inherited properties, and more importantly don't recognize each other as siblings. So I think that's the first thing to address.

Let's assume the following here forward (no margins, no positioning):

/*------------------------
Default grid
-------------------------*/

/*------------------------
Columns
-------------------------*/

.has-2-columns .col {
  flex: none;
}

.has-2-columns .col {
  width: 49%;
}

/*------------------------
Logo
-------------------------*/

.site-title {
  background: #ccc;
}

/*------------------------
Main navigation
-------------------------*/

.main-navigation { background-color: lightgrey;}

.main-navigation ul {
  display: inline-flex;
  list-style: none;
  padding: 0;
  margin: 0;
}

.main-navigation ul li { }

.left-menu {
  text-align: left;
}

.left-menu li {}

.right-menu {
  text-align: right;
}

.right-menu li {}

.site-header {
  display: flex;
  flex-flow: row wrap;
  background-color: pink;
}

Demo

Home

About

Contact

Store

Making them siblings can assistant in limiting any crutches we have on margins and positioning by granting them situational awareness to one another now.

This is also a good time to practice good standards when it comes to classes. Since we stripped all the classes we no longer need, we aren't styling multiple elements anymore, so we can change their selector to an id.

Additionally, let's take advantage of what we learned about direct descendants and apply that now by making the
 a direct sibling to both lists as follows:

/*------------------------
    Main navigation
    -------------------------*/

#main-navigation {
  background-color: lightgrey;
}

#main-navigation ul {
  display: inline-flex;
  list-style: none;
  padding: 0;
  margin: 0;
}

#site-header {
  display: flex;
  flex-flow: row wrap;
  background-color: pink;
}

Home

About

Demo

Contact

Store

Your needs will vary depending on the project, but this particular example we don't have anything else but the navigation so we are going to promote the  to be the flex parent. In doing this we can take advantage of align properties, justify properties and of course the other flex benefits. The 2 key properties are align-items which (on flex-direction: row) it centers the children vertically in our case, and justify-content which you seem to be familiar with. These 2 properties alone handle everything you've used margins for as intended.

#main-navigation {
 display: flex;
 flex-flow: row wrap;
 align-items: center;  /* center the elements (vertically for this axis) in the parent space */
 justify-content: space-between;
}

#main-navigation ul {
  display: inline-flex;
  list-style: none;
  padding: 0;
  margin: 0;
}
#main-navigation li {
  padding: 0 10px;
}

/* irrelevant */
#site-header {
  background-color: pink;
}

Home

About

Demo

Contact

Store

So that's one way to get your desired outcome on a fundamental level

..... but wait, mobile first right? Naturally, a simple media query can help you adjust things as you wish, that will be arbitrary to your styling needs, but here is a brief example that makes the previous example responsive friendly:

#main-navigation {
 display: flex;
 flex-flow: row wrap;
 align-items: center;  /* center the elements (vertically for this axis) in the parent space */
 justify-content: space-between;
}

#main-navigation ul {
  display: inline-flex;
  list-style: none;
  padding: 0;
  margin: 0;
}
#main-navigation li {
  padding: 0 10px;
}

@media only screen and (max-width: 396px) {
  #main-navigation {
    flex-flow: column nowrap;
  }

  #main-navigation h1 {
    order: -1;
  }
}

/* irrelevant */
#site-header {
  background-color: pink;
}

Home

About

Demo

Contact

Store

Notice how in that example, the
 element has an order of -1. Flex assigns '0' to an element as the default value. Therefor -1 gives specificity to those elements without needing to change every single one to [..1,2,3 etc]

And if you REALLY REALLY don't feel well about including it there after all that, we can kind of hack it there:

#main-navigation {
  padding: 10px;
}

#main-navigation ul {
  display: flex;
  flex-flow: row wrap;
  align-items: center;
  /* center the elements (vertically for this axis) in the parent space */
  justify-content: space-evenly;
  list-style: none;
  padding: 0;
  margin: 0;
}

#branding {
  font-size: 1.75rem;
}

@media only screen and (max-width: 396px) {
  #main-navigation ul {
    flex-flow: column nowrap;
  }
  #branding {
    order: -1;
  }
}

/* irrelevant */

#site-header {
  background-color: pink;
}

Home

About

Demo

Contact

Store


Long expression to sum data in a multi-dimensional model I am porting a linear optimization model for power plants from GAMS to Pyomo. Models in both frameworks are a collection of sets (both elementary or tuple sets), parameters (fixed values, defined over sets), variables (unknowns, defined over sets, value to be determined by optimization) and equations (defining relationships between variables and parameters).

In the following example, I am asking for ideas on how to make the following inequality more readable:

def res_stock_total_rule(m, co, co_type):
    if co in m.co_stock:
        return sum(m.e_pro_in[(tm,)+ p] for tm in m.tm for p in m.pro_tuples if p[1] == co) + \
               sum(m.e_pro_out[(tm,)+ p] for tm in m.tm for p in m.pro_tuples if p[2] == co) + \
               sum(m.e_sto_out[(tm,)+ s] for tm in m.tm for s in m.sto_tuples if s[1] == co) - \
               sum(m.e_sto_in[(tm,)+ s] for tm in m.tm for s in m.sto_tuples if s[1] == co) <= \
               m.commodity.loc[co, co_type]['max']
    else:
        return Constraint.Skip

Context:

m is a model object, which contains all of the above elements (sets, params, variables, equations) as attributes.
m.e_pro_in for example is a 4-dimensional variable defined over the tuple set (time, process name, input commodity, output commodity).
m.tm is a set of timesteps t = {1, 2, ...}, m.co_stock the set of stock commodity, for which this rule will apply only (otherwise, no Constraint is generated via Skip).
m.pro_tuples is a set of all valid (i.e. realisable) tuples (process name, input commodity, output commodity).
m.commodity is a Pandas DataFrame that effectively acts as a model parameter.

My question now is this:

Can you give me some hints on how to improve the readability of this fragment? The combination of tuple concatenation, two nested list comprehensions with conditional clause, Pandas DataFrame indexing, and a multiline expression with line breaks all make it less than easy to read for someone who might just be learning Python while using this model.

First of all, use helper functions or explicit for loops.

E.g. you're looping over m.tm four times. This can be done in a single loop. It might need more lines of code, but it will get much more readable.

E.g.

def res_stock_total_rule(m, co, co_type):
    if not co in m.co_stock:
        return Constraint.Skip

    val = 0
    for tm in m.tm: # one single loop instead of four
        for p in m.pro_tuples:
            if p[1] == co:
                val += m.e_pro_in[(tm,) + p)]
            if p[2] == co:
                val += m.e_pro_out[(tm,) + p)]
        for s in m.sto_tuples: # one single loop instead of two
            if s[1] == co:
                val += m.e_sto_out[(tm,) + p)] - m.e_sto_in[(tm,) + p)]
    return val <= m.commodity.loc[co, co_type]['max']


Login and User Information Requests This code basically connects to a database, sets login success and failure pages, queries the database for user details, checks if user is active, sets session value and redirects accordingly.

Can you have a look? What do you think of it? Any suggestions?

<?php

session_start();

// Connect to the database
try {
    $db = new PDO('mysql:host=localhost; dbname=database', 'username', 'password');
    $db->setAttribute(PDO::ATTR_ERRMODE,PDO::ERRMODE_EXCEPTION);
    $db->exec("SET NAMES 'utf8'");
} catch(Exception $e) {
    exit;
}

// Set URL for successful login and unsuccessful login
if($_POST['page']) {
    $success = $_POST['page'];
} else {
    $success = 'http://website.com/members';
}
$fail = 'http://website.com/login';

// Check if login came from my server
if($_SERVER['SERVER_NAME'] != 'website.com') {
    header('Location: ' . $fail);
}

// Check if a user is trying to login
if($_POST) {

    // Query the users details
    try {
        $user_query = $db->prepare('SELECT * FROM users LEFT JOIN zones ON user_timezone = zone_id WHERE user_email = ? AND user_pass = ?');
        $user_query->bindParam(1, $_POST['username'], PDO::PARAM_STR, 50);
        $user_query->bindParam(2, $_POST['password'], PDO::PARAM_STR, 15);
        $user = $user_query->execute();
        $user = $user_query->fetch(PDO::FETCH_ASSOC);
    } catch(Exception $e) {
        exit;
    }

    // Make sure account is active
    if($user['user_active'] != 1) {
        header('Location: ' . $fail . '?error=2');
        exit;
    }

    // Make sure user exists
    if($user != FALSE) {
        $_SESSION['uid'] = $user['user_id'];
        $_SESSION['utz'] = $user['zone_name'];
        header('Location: ' . $success);
    } else {
        header('Location: ' . $fail . '?error=1');
        exit;
    }
} else {
    header('Location: ' . $fail . '?error=4');
    exit;
}

?>

Just few quick things come to my mind:

The way try/catch is used is to exit on error, which would happen anyway. Maybe you need to send some info to the user here instead.
Are you passing raw user input to the database, without validation? This would be clear security issue.
Why not if($user) instead of if($user != FALSE)?
What is the purpose to set headers and then exit without any request?


Implementing a faster inbound neighbour similarity in Networkx Given a directed graph G of 20+ million edges and approximately 2 million unique nodes, can this function/process be made faster on a dataset of 40k edges?

The current speed is at ~1.8 seconds per edge.

import pandas as pd
import networkx as nx

# main feature calculator
def jacard_inbound(u,v):
    preds = G.predecessors(v)
    result = [jacard_coef(u,x) for x in preds]
    return sum(result), max(result), sum(result)/len(result)

# jacquard's coefficient & helper functions
def jacard_coef(u,v):
    return common_friends(u,v)/total_friends(u,v)

def total_friends(u,v):
    return len(set(Gamma(u)).union(Gamma(v)))

def common_friends(u,v):
    return len(set(Gamma(u)).intersection(Gamma(v)))

# to cache calculated results
gamma_dict = dict()
def Gamma(u):
    s = gamma_dict.get(u)
    if s is None:
        s = set(G.successors(u)).union(G.predecessors(u))
        gamma_dict[u]=s
    return s

Running example:

# sample graph
G = nx.DiGraph()
G.add_edges_from([(1,2), (1,3), (2,3), (3,4), (3,5), (7,1), (5,6), (8,2), (9,0)])

# sample edges
dataset = pd.DataFrame(columns = ['source','target'])
dataset['target'] = [3, 6, 2, 3]
dataset['source'] = [2, 1, 8, 1]

t = time.time()
dataset['jacard_inbound'] = dataset[['source','target']].apply(lambda x: jacard_inbound(x['source'],x['target']), axis=1)
print(time.time() - t)

"Jaccard coefficient", not "Jacard" or "Jacquard": it's named after Paul Jaccard.
There are no docstrings. What do these functions do? What do they return?
The results of these functions depend on the global variable G. If you ever needed to generalize your program to operate on more than one graph at a time this dependency would become very inconvenient. The functions ought to either take the graph as an argument, or be methods on the graph class.
Is Gamma standard graph terminology for this function? I've seen it called 'neighbourhood' (or 'open neighbourhood' when it needs to be clear that u is not included).
Python has a built-in decorator functools.lru_cache for caching the results of a function. So you can write:

@lru_cache(maxsize=None)
def neighbourhood(g, u):
    "Return the set of vertices in the graph g that are adjacent to u."
    return set(g.successors(u)).union(g.predecessors(u))

Reuse the set of successors instead of discarding it:

@lru_cache(maxsize=None)
def neighbourhood(g, u):
    "Return the set of vertices in the graph g that are adjacent to u."
    result = set(g.successors(u))
    result.update(g.predecessors(u))
    return result

In common_friends and total_friends, there is no need to make a copy of the neighbourhood before taking the union/intersection. You can take the union/intersection directly:

def total_friends(g, u, v):
    """Return the number of vertices in the graph g that are in the
    neighbourhoods of u or v (or both).

    """
    return len(neighbourhood(g, u) | neighbourhood(g, v))

def common_friends(g, u, v):
    """Return the number of vertices in the graph g that are in the
    neighbourhoods of both u and v.

    """
    return len(neighbourhood(g, u) & neighbourhood(g, v))


Set Dates on Spreadsheet Rows I'm trying to get this to run faster, even marginally so. Right now to update 470 rows it takes about 90 seconds. It works, but I know there are better ways. I understand calls to Spreadsheet API are slow, and batching getRanges and setValues would speed things up, but doesn't seem obvious given the brief bit I know.
function setDates() {

var app = SpreadsheetApp;
var ssData = app.getActiveSpreadsheet().getSheetByName("Data");

//Calculates due dates based on entered dates in column D
  for (var i=4; i<=ssData.getLastRow(); i++)  {                                     //cycles through all 18 pilots
    for(var j=0; j<25; j++) {                                                       //cycles through an individuals' 25 training items
    var dateCompleted = new Date(ssData.getRange(i+j,4).getValue());                //date of training completed
    var interval = ssData.getRange(i+j,5).getValue();                               //interval in months between date complete and date due
    var monthCompleted = dateCompleted.getMonth();
    var yearCompleted = dateCompleted.getFullYear();

      if (!isNaN(parseFloat(yearCompleted)) && !isNaN(interval)) {                  //checks to see if vars year and interval are numbers
      var dateDue = new Date(dateCompleted.setMonth((monthCompleted + interval)));  //date of training due
      var yearDue = dateDue.getFullYear();
      var monthDue = dateDue.getMonth();
      dateDue = new Date(yearDue, monthDue+1, 0);
      ssData.getRange(i+j,6).setValue(new Date(dateDue));                           //sets dateDue in column F
      }
    }
    i = i + 25;
  }
}

General review
Generally the code is OK and problems are due to lack of experience rather than bad style or habit.
I am unfamiliar with SpreadsheetApp so you should wait in case someone that is adds another answer before you accept this answer, if so inclined.
Rather than submit this answer I took a quick look at the DOC for SpreadsheetApp (I hope I got the correct one) and added to this answer at bottom under heading Performance.
Review points

Take care with indentation. all code blocks should be indented (If indentation mess is due to snippets poor formatting ignore this line)

When using var declare them at the top of the function.

Use constants const for variables that do not change.

Use i, j only for indexes or counters, if you are accessing coordinates, row, column, or x, y use names to match the axis name.
In this case it looks like you are only indexing by row index. In the rewrite I use the variable name row. Then a counter called i to count 25 and allow for the skipped row every 26 rows.
BTW When using i, j in loops the convention is i for the inner loop and j is the outer.

Don't calculate the same value many times. Eg you calculate i + j 3 times.

Avoid single use variables.

Use while loops in preference to for loops especially if you are incrementing the counter manually in the loops block.
While loops are generally syntactically cleaner and are less complex under the hood (if you use let to declare the counter).
Note that when using while loops it is easy to forget to add the incrementation so take care to not forget to increment / decrement

Use arithmetic assignment operators in this case += (Addition assignment) i += 25 rather than i = i + 25;

The is no need to create a new Date object every time you make a change.

Date object
The Date objects call Date.getFullYear always returns a Number as an integer.
There is no need to use parseFloat and the result of !isNaN(parseFloat(yearCompleted)) will always be true and is thus not required.
Further the variable yearCompleted is only used in the unneeded predicate and as such is not required at all.
To set the date to the first day of the month you don't need to use year, month + 1, 0 just set it to the 1st year, month, 1
The inner if statement
It is unclear if the call to get interval = ssData.getRange(i + j, 5).getValue() will return a number or not.
Because the use of monthCompleted is dependent on the valid numerosity of interval you can combine the two in one expression. However I suspect that interval will always be a number and as such there is no need for the isNaN test.
Rewrite
Using the above points the rewrite greatly reduces the code complexity. Without data there is no way to know if there is a need to vet the interval value. I added the test but it may not be needed.
function setDates() {
    var row = 4, i;
    const data = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Data");
    while (row < data.getLastRow()) {
        i = 25;
        while (i--) {
            const interval = data.getRange(row, 5).getValue();
            if (!isNaN(interval)) {
                const completed = new Date(data.getRange(row, 4).getValue());
                completed.setMonth(interval + completed.getMonth());
                completed.setDate(1);
                data.getRange(row, 6).setValue(completed);
            }
            row ++;
        }
        row ++;
    }
}

Performance
I have taken a quick look at the SpreadsheetApp docs and thus will address the performance problems you are concerned about.
The problem I think is the use of getRange(row, column) which you use to get each cell.
Rather...

Use getRange(row, column, numRows, numColumns) to get all the rows and columns in one call before you iterate the cells.

Then get an array of values from the range using range.getValues() make the modifications needed.

Then use range.setvalues() to set the changes made

This may get a significant performance increase.
Rewrite based on Docs
As I have no data or access to the API the rewrite is only a guess based on a quick read of the DOC's
The only problem I can foresee is why you skip every 26th row and why you start from the 5th row (index as row 4)
function setDates() {
    var rowIdx = 0;
    const sheet = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Data");
    const range = sheet.getRange(4, 4, sheet.getLastRow() - 4, 3);
    const data = range.getValues(); // as 2D array of rows by columns eg...
                                    // data[0][0] is row 4 column 4
                                    // data[10][2] is row 14 column 6
    for (const row of data) {
        if (rowIdx > 0 && (rowIdx % 26)) { // skip every 26th row
            const interval = row[1];
            if (!isNaN(interval)) {
                row[2] = new Date(row[0]);
                row[2].setMonth(interval + row[2].getMonth());
                row[2].setDate(1);
            }
        }
        rowIdx++;
    }
    range.setValues(data);
}


Run multiple prerequisite methods in parallel before continuing I have an application in which I have multiple slow running prerequisite methods I need to call in order to have the data I need to continue.

To try to speed this up I am doing this:

Dictionary<string, string> processOptions = null;
Dictionary<string, string> businessAreas = null;
Dictionary<string, string> requirementLevels = null;
List<TemplateSection> sections = null;
List<TemplateVersion> versions = null;

var prerequisiteTasks = new List<System.Threading.Tasks.Task>();

prerequisiteTasks.Add(new System.Threading.Tasks.Task(() => sections = GetSectionsInTreeForm(latestVersionGuid)));
prerequisiteTasks.Add(new System.Threading.Tasks.Task(() => versions = GetTemplateVersions(templateId)));
prerequisiteTasks.Add(new System.Threading.Tasks.Task(() => processOptions = GetOptionSetProcessPhaseTypes()));
prerequisiteTasks.Add(new System.Threading.Tasks.Task(() => businessAreas = GetOptionSetBusinessAreaTypes()));
prerequisiteTasks.Add(new System.Threading.Tasks.Task(() => requirementLevels = GetOptionSetRequirementLevelTypes()));

prerequisiteTasks.ForEach(x => x.Start());
System.Threading.Tasks.Task.WaitAll(prerequisiteTasks.ToArray());

Each of the methods above can take anything from 2-5 seconds to complete.
Is there a nicer way of doing this, or is this something I shouldent do?

P.S. the code as is works great and decreases the time used by around 30%.

Another option is Parallel.Invoke:

Dictionary<string, string> processOptions = null;
Dictionary<string, string> businessAreas = null;
Dictionary<string, string> requirementLevels = null;
List<TemplateSection> sections = null;
List<TemplateVersion> versions = null;

Parallel.Invoke(() => sections = GetSectionsInTreeForm(latestVersionGuid),
                () => versions = GetTemplateVersions(templateId),
                () => processOptions = GetOptionSetProcessPhaseTypes(),
                () => businessAreas = GetOptionSetBusinessAreaTypes(),
                () => requirementLevels = GetOptionSetRequirementLevelTypes());

It should be pointed out that these tasks run in threads from the thread pool (in both cases: Task and Parallel.Invoke). That's OK for few tasks with a run time from 2-5 seconds, but for long running tasks it is better to use a Task object with TaskCreationOptions.LongRunning to run the task in it's own thread. Otherwise the threadpool threads will be blocked for other short running tasks.

Tasks provide a more granular control but that is not required in your case so I would prefer the simpler Parallel.Invoke variant.


building a search functionality I am trying to search account table by providing a search string. I am building a query based in different conditions and then adding the returned records in a list.

The functionality is working correctly but I want to know how an below method be further optimized.

    public PageReference search(){
    clear();
    runningUserId = Userinfo.getUserId();
    String searchquery;
    Boolean existUser = AccSearchAccess__c.getValues(runningUserId) !=null? true : false;
    if(existUser){
        searchquery='Select  Name,Id,AccessLevel__c FROM Account where Name like \'%'+accountName+'%\'';
    }
    else{
        set<id> accountIds = new set<id>();
        List<WMAccess__c> wmaLst = [Select id,Account__c from WMAccess__c where WM_User__c = : UserInfo.getUserId()];
        for(WMAccess__c wm : wmaLst){
            accountIds.add(wm.Account__c);
        }
        if(!accountIds.isEmpty()){
            searchquery='Select  Name,Id,AccessLevel__c FROM Account where Name like \'%'+accountName+'%\' AND id IN:accountIds';
        }
    }

    accList= Database.query(searchquery);
    if(accList.size()>0)
        for(Account acc : accList){
            AccountWrapper aw = new AccountWrapper(acc, false);
            aw.accountId = acc.id;
            aw.accountName = acc.Name;
            aw.accountAccess = acc.AccessLevel__c;
            accountsList.add(aw);
        }
    return null;
}

You have exposed an injection vulnerability.

'WHERE Name like \'%'+accountName+'%\''

Any time you merge user input into your query, you should call String.escapeSingleQuotes, or better yet, use a dynamic binding.

// OK
searchQuery += 'WHERE Name LIKE \'%' + String.escapeSingleQuotes(accountName) + '%\'';

// BETTER
String fuzzySearchTerm = '%' + accountName + '%';
searchQuery += 'WHERE Name LIKE :fuzzySearchTerm';


Finding unique top sums from multiple lists My question arises from this post on MSE where I have provided an answer to solve the question :

There are multiple lists given. The number of lists is arbitrary.
Each list contains numbers and is sorted descendingly.
We shall take exactly \$1\$ element from each list and calculate the sum of elements.How would you go about finding the top \$N\$ sums?

Here the top sums need not to be different, just the indices of them. I wanted to write an algorithm to find the top \$N\$ unique sums, namely where the sum of elements is different. What I have done is using the same approach described in the linked post. The problem is that for some inputs there are lots of duplicate sums, so the research for the unique ones gets slower and slower. I post the implementation in python
import time

def top_solutions_2(N,lists):

    N_best = []
    k, len_k_lists = len(lists), [len(x) for x in lists]
    init_sol = (sum(x[0] for x in lists),tuple(0 for x in range(k)))
    comp_list, new_vals = [[init_sol]], []
    seen = {init_sol[1]}

    for _ in range(N) :

        curr_best = [float('-inf')]
        for x in comp_list :
            if x and x[-1][0] > curr_best[0] : curr_best = x[-1]

        N_best.append(curr_best)

        inds = []

        for arr in comp_list :

            while arr :

                comp_val = arr.pop()

                if curr_best[0] > comp_val[0] : arr.append(comp_val); break

                inds.append(comp_val[1])

        comp_list.append([])

        for ind in inds :

            for x in range(k) :

                if len_k_lists[x] > ind[x]+1 : r = tuple(c if i != x else c+1 for i,c in enumerate(ind))
                else : continue

                if r not in seen :
                    curr_sum = curr_best[0]+lists[x][r[x]]-lists[x][r[x]-1]
                    comp_list[-1].append((curr_sum,r))
                    seen.add(r)

        comp_list[-1].sort()

    return N_best

for N in range(10,60,10) :

    lists = [ [23,5,3,2,1],
              [19,9,8,7,0],
              [17,12,4,2,1],
              [15,13,11,9,2],
              [21,17,13,9,4],
              [16,13,12,11,1],
              [27,23,21,18,4],
              [31,25,24,12,1],
              [27,22,14,7,3],
              [9,8,7,6,5]]

    a = time.time()
    top_solutions_2(N,lists)
    b = time.time()
    print("Top {} in {} sec".format(N,b-a))

where the output is
Top 10 in 0.0 sec
Top 20 in 0.07787561416625977 sec
Top 30 in 0.5308513641357422 sec
Top 40 in 2.2048890590667725 sec
Top 50 in 7.203002452850342 sec

How can a more efficient algorithm with a lower complexity and/or a lower running time be made?And also, how can my approach be improved in efficiency?
Thank you in advance


My own snprintf implementation in C I decided to make my own version of snprintf in C. I intentionally changed some things though. My version guarantees the buffer printed to will be null-terminated, and it returns the number of characters printed to the buffer, not the number that would have been printed if the buffer's size was not limited. And I only worried about some of the main formatting features, like %s, %c, %d, %h, and %H.

I would love to know what I could do better in this and improve, or what aspects of it I did or did not implement well.

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

int INT_TO_STR_DIGITS_L[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
int INT_TO_STR_DIGITS_U[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };

int int_to_str(int x, char *buf, size_t size, int base, int uppercase) {
    int length = (int)ceil(log((double)x)/log((double)base));
    int r, i = 0;
    char c;

    if (size < length) {
        x /= (int)pow(base, (float)(length - size));
        length = size;
    }

    do {
        if (i >= size) break;
        r = x % base;
        if (uppercase) {
            c = INT_TO_STR_DIGITS_U[r];
        } else {
            c = INT_TO_STR_DIGITS_L[r];
        }
        buf[length-i-1] = c;
        x /= base;
        i++;
    } while (x != 0);

    return i;
}

int my_snprintf(char *str, size_t max_size, const char *fmt, ...) {

    va_list arg_list;
    va_start(arg_list, fmt);

    int chars_printed = 0;
    char *start_str = str;

    char c, *str_arg;
    int num, len;
    int uppercase = 0, base = 10;

    for (int i = 0; fmt[i] != 0; i++) {
        if (max_size - chars_printed <= 0) {
            break;
        } else if (fmt[i] == '%') {
            i++;
            switch (fmt[i]) {
            case 'c':
                c = va_arg(arg_list, int);
                str[chars_printed++] = c;
                break;
            case '%':
                str[chars_printed++] = '%';
                break;
            case 's':
                str_arg = va_arg(arg_list, char *);
                len = strnlen(str_arg, max_size - chars_printed);
                strncpy(str+chars_printed, str_arg, len);
                chars_printed += len;
                break;
            case 'H':
                uppercase = 1;
            case 'h':
                base = 16;
            case 'd':
                num = va_arg(arg_list, int);
                len = int_to_str(num, str+chars_printed, max_size - chars_printed, base, uppercase);
                chars_printed += len;
                break;
            default:
                printf("Invalid format.\n");
                va_end(arg_list);
                return -1;
            }
        } else {
            str[chars_printed++] = fmt[i];
        }
    }

    if (chars_printed == max_size) chars_printed--;
    str[chars_printed] = 0;

    va_end(arg_list);
    return chars_printed;
}

Bug
Your int to string conversion isn't working correctly when the number being printed is an exact power of the base.  Here is a program that demonstrates the bug:
int main(void)
{
    char buf[256];

    memset(buf, 'z', 256);
    my_snprintf(buf, 256, "abc%ddef", 1000);
    printf("%s\n", buf);
}

Expected output:
abc1000def

Actual output:
ab1000zdef

As you can see, the 1000 portion was written one too far to the left.  The problem is that your number length computation is off by one for exact powers of the base.
Unnecessary and unsafe floating point operations
The Floating Point Police™ would like to point out that the use of floating point in int_to_str() is both unnecessary and dangerous.  First of all, this line:
int length = (int)ceil(log((double)x)/log((double)base));

could be rewritten to use a loop to count the number of digits.  By using floating point, you open yourself up to rounding errors.  For example, if x were 125 and base were 5, you would expect length to be 3.  However, when I ran the above code using 125 and 5 on my x86 machine, I got a length of 4 instead.  This is because the division evaluated to something like 3.00000001 and ceil rounded it up to 4.  (Of course there is already an unrelated off by one bug mentioned in the previous section.  This floating point use is a separate cause of concern).
The same thing applies to this line:
    x /= (int)pow(base, (float)(length - size));

This could be rewritten to be a loop where you divide by base once per loop iteration.  By using pow() and casting to int, you run the risk of the result of pow erroneously rounding down to the previous int.


Security of a "contact us" form I have a form, which uses AJAX to send POST data to the following controller method:

public function action_sendmail() {
        $jsonHelper=Loader::helper('json');
        $answer = (int) $_POST['security'];
        if($answer <= 0)
            $answer = 1;

        if($answer == 4){

            $name = filter_var($_POST['name'], FILTER_SANITIZE_STRING);
            $email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL);
            $message = filter_var($_POST['message'], FILTER_SANITIZE_STRING);

            $name = strip_tags($name);
            $email = strip_tags($email);
            $message = strip_tags($message);

            $name = str_replace(array("\r", "\n", "%0a", "%0d"), '', stripslashes($name));
            $email = str_replace(array("\r", "\n", "%0a", "%0d"), '', stripslashes($email));
            $message = str_replace(array("\r", "\n", "%0a", "%0d"), '', stripslashes($message));

            if(!filter_var($email, FILTER_VALIDATE_EMAIL)){
                echo $jsonHelper->encode(t("Wrong email address."));
                die;
            }
            if(preg_match("/^[+]?[0-9]*$/", $_POST['phone'])) {
                $phone = strip_tags($_POST['phone']);
            }
            else {
                echo $jsonHelper->encode(t("Wrong phone number format."));
                die;
            }

            $message = t('Phone number').": ".$phone."\n\n".$message;

            $mail = Loader::helper('mail');
            $mail->setBody($message);
            $mail->from($email, $name);
            $mail->to("test@example.com");
            $mail->to($email);
            $mail->setSubject(t('Example subject'));

            $sent = true;

            $mail->sendMail();

            if($sent) {
                echo $jsonHelper->encode("sent");
                die;
            }
            else {
                echo $jsonHelper->encode(t("Could not send mail. Please try again."));
                die;
            }

        }
        else{
            echo $jsonHelper->encode(t("Wrong security answer!"));
            die;
        }
    }

The script sanitizes different attributes (name, email and the message) and sends an email to a specified address and the one from the HTML form.
My question: Is the form secure?

Secure: Not yet, but it's a start. Your validation is too strict in some places, and may result in the wrong email address being used. In particular here:

$email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL);
//a bit later on:
$email = str_replace(array("\r", "\n", "%0a", "%0d"), '', stripslashes($email));

If the value of $email is one of these examples of valid email addresses:

!#$%&'*+-/=?^_`{}|~@example.org
"()<>[]:,;@\\\"!#$%&'*+-/=?^_`{}| ~.a"@example.org
"very.(),:;<>[]\".VERY.\"very@\\ \"very\".unusual"@strange.example.com

you're stripping the slashes, which'll yield:

!#$%&'*+-=?^_`{}|~@example.org
"()<>[]:,;@"!#$%&'*+-=?^_`{}| ~.a"@example.org
"very.(),:;<>[]".VERY."very@ "very".unusual"@strange.example.com

Effectively changing the actual email address, and this may in turn turn a valid, though outlandish email address into an invalid one.
BTW: source for these email addresses is the email wiki

When it comes to email addresses: use filter_validate($emai, FILTER_VALIDATE_EMAIL) and rely on that function's ability to validate the string. Don't do any processing of your own, especially not after having sanitized and validated the email address.
Bottom line, if you want to validate an email address, while allowing users to pass an (accidental) \r char, use this:

$email = filter_var($_POST['email'], FILTER+_SANITIZE_EMAIL);
if (!filter_var($email, FILTER_VALIDATE_EMAIL))
{
    echo $jsonHelper->encode($email.' is an invalid address'));
}

On the other hand, you're not sanitizing the message and name as well as maybe you should. It's not unlikely your script is vulnerable to mail injection. A must-read on this subject can be found here.

Since we're all in the business of programming, I hope you'll understand I'm not going to repeat myself here, but I've dealt with mail injection previously on this site: read my answer to this question, which focusses on injection a bit more.

Some other niggles:

Please, as ever, try to stick to the coding standards as described by PHP-FIG. Most, if not all, major players subscribe to this standard, so should you. Check the standard here.
Things like public function foobar(){ are not accepted, the opening brace should go on the next line. It may seem silly to stumble over this, but really: it isn't.
don't use die in a method, and avoid using echo. I take it you're using a framework of sorts. Check if there's an ajax helper that outputs, or use the view to echo JSON content. And from within an action (controller method), don't call die, but rather return, signaling an uneventfull end of the controllers' job. die breaks the normal way of processing a request, which may cause problems down the road.


Better way to iterate through a dictionary and comparing its keys with itself I have a dictionary with author names as keys and values of None. I want to use fuzzywuzzy to compare these author names to see if there are similar ones that I can combine.

So far I am using two dictionaries (they have the same data in both of them, and ideally I would just use one) and then using a double for loop. I want to optimize it further, and I was thinking of using Dictionary Composition, but I couldn't figure out how to get that working with my fuzzy compare logic.

import uuid
from fuzzywuzzy import fuzz

authorToDelete = {}
dictOfAllData1 = {'Trevor Jacobs': None, 'Josh Francis': None, 'Marcie Lank': None, 'Marcie H. Lank': None}
dictOfAllData2 = {'Trevor Jacobs': None, 'Josh Francis': None, 'Marcie Lank': None, 'Marcie H. Lank': None}

for key in dictOfAllData1:
    for key2 in dictOfAllData2:
        str1 = ' '.join(key.split()) #some spaces are different so I handle those here
        str2 = ' '.join(key2.split())
        ratio = fuzz.ratio(str1, str2)
        if fuzz.ratio(str1, str2) > 85 and dictOfAllData1[key] == None:
            dictOfAllData1[key] = str(uuid.uuid1())
        elif ratio > 85:
            if str1 != str2:
                authorToDelete[key2] = None
            else:
                dictOfAllData1[key] = str(uuid.uuid1())

for deleteMe in authorToDelete:
    dictOfAllData1.pop(deleteMe)

print(dictOfAllData1)

#Output is: {'Trevor Jacobs': 'edb6e3d8-3898-11ea-9b62-784f4397bfaf', 'Josh Francis': 'edb6e892-3898-11ea-9b62-784f4397bfaf', 'Marcie Lank': 'edb6eaea-3898-11ea-9b62-784f4397bfaf'}

This is the code I have so far. It works, but it takes longer than I think it should (about 4 seconds with a dictionary of ~700 keys, and about 80 seconds with a dictionary of ~2,700 keys)

My question is how can I make this more efficient? Is there a way I can use if in dict or something like that instead of my second for loop?

I originally posted this on StackOverflow

Before speaking about the actual algorithm, let me hint you at the official Style Guide for Python Code (often just called PEP 8), a set of guidelines to write idiomatic-looking Python code. A core takeaway of the read should be, that in Python lower_case_with_underscores is the preferred way to name variables and functions. Fortunately you don't have to remember all those rules. There is good tool support for style and also static code checking in Python. A non-exhaustive list can be found here on this post here on Code Review Meta. I personally most often use flake8 and pylint from that list.

With that out of the way, let's have a look at the code. First, you don't need to have two copies of your dict. Also, the whitespace normalization inside the loop is run more often than needed, because it is calculated several times for every key. Fortunately, it's easy to solve both of those problems:

keys = list(dict_of_all_data.keys())
authors = {key: ' '.join(key.split()) for key in keys}

keys now holds all author names from the original dict, whereas authors holds their normalized versions. Maybe you should also consider to convert all the names to upper-/lowercase in order to make it even more robust, but you'll have to see yourself if that's worth it.

Looking at the fuzzywuzzy documentation of fuzzywuzzy.fuzz reveals that it basically seems to be a wrapper around difflib.SequenceMatcher. From a few quick experiments I got the impression that the following is true:

fuzzywuzzy.fuzz(str1, str2) == fuzzywuzzy.fuzz(str2, str1)

With that in mind, the amount of computations can be cut down even more, by avoiding duplicate comparisons. Since you gave no special reason for using UUIDs, I skipped them in the reworked code for the moment.

dict_of_all_data = {'Trevor Jacobs': None, 'Josh Francis': None, 'Marcie Lank': None, 'Marcie H. Lank': None}

keys = list(dict_of_all_data.keys())
authors = {key: ' '.join(key.split()) for key in keys}
authors_to_delete = []

for i, key1 in enumerate(keys, 1):
    author1 = authors[key1]
    for key2 in keys[i:]:   # this helps to avoid duplicate comparison
        author2 = authors[key2]
        ratio = fuzz.ratio(author1, author2)
        if ratio > 85 and dict_of_all_data[key1] is None:
            dict_of_all_data[key1] = True   # likely not even necessary
            authors_to_delete.append(key2)

for delete_me in authors_to_delete:
    dict_of_all_data.pop(delete_me)

print(dict_of_all_data)

As a bonus, one should wrap the code into a function to nicely separate it from the rest of the code. Whether or not you want to convert this code into a function, it would be a good idea to replace the magic value 85 with a parameter/constant with a meaningful name.

Grajdeanu Alex presents a similar idea in his answer, but uses itertools.combinations instead of doing the de-duplication manually as I suggested above.


Simple Freelock collection I wrote simple lock-free collection class for saving items from multithreading code. I did it just for fun and experience. Can you check my code for potentially problems please?

    public class InputBox<T>
    {
        private BoxItem<T> _current;

        public InputBox()
        {
            _current = null;
        }

        public void Add(T value)
        {
            var nextItem = new BoxItem<T>(value);
            BoxItem<T> initialItem = null;
            do
            {
                initialItem = _current;
                nextItem.Next = _current;
            } while (initialItem != Interlocked.CompareExchange(ref _current,
                                                                nextItem,
                                                                initialItem));
        }

        public bool TryAdd(T value)
        {
            var initialItem = _current;
            return initialItem ==
                   Interlocked.CompareExchange(ref _current,
                                               new BoxItem<T>(value, _current),
                                               initialItem);
        }

        public BoxResultCollection<T> TakeAll()
        {
            return new BoxResultCollection<T>(_current);
        }
    }

    internal sealed class BoxItem<T>
    {
        private readonly T _value;
        private BoxItem<T> _next;

        internal BoxItem(T value)
            :this(value, null)
        {
            // empty
        }

        internal BoxItem(T value, BoxItem<T> next)
        {
            _value = value;
            _next = next;
        }

        internal T Value
        {
            get { return _value; }
        }

        internal BoxItem<T> Next
        {
            get { return _next; }
            set { _next = value; }
        }
    }

    public class BoxResultCollection<T> : IEnumerable<T>
    {
        private readonly BoxItem<T> _root;

        internal BoxResultCollection(BoxItem<T> root)
        {
            _root = root;
        }

        public IEnumerator<T> GetEnumerator()
        {
            var current = _root;

            while (current != null)
            {
                yield return current.Value;
                current = current.Next;
            }

            yield break;
        }

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

        public bool IsEmpty
        {
            get { return _root == null; }
        }

        public IEnumerable<T> ToEnumerable()
        {
            return this;
        }
    }

public bool TryAdd(T value)

A method that intentionally doesn't work when there is contention? I don't see how would that be ever useful.

internal BoxItem<T> Next
{
    get { return _next; }
    set { _next = value; }
}

You could have used an autoproperty here.

yield break;

Adding yield break; at the end of iterator method is the same as adding return; at te end of a normal void method: the method would work exactly the same without it.

public IEnumerable<T> ToEnumerable()

I don't see how would this method be useful. Especially since there already is AsEnumerable() extension method that does exactly the same thing.


Code style: passing a reference to subroutine Is the subroutine test1 ugly?/uglier than the others?

#!/usr/bin/env perl
use warnings;
use strict;
use 5.10.1;
use Data::Dumper;

my $ref = { one => 1, two => 2, three => 3 };

sub test1 {
    my ( $ref ) = @_;
    $ref->{three} = 4;
}
test1( $ref );

sub test2 {
    $ref->{three} = 4;
}
test2( $ref );

sub test3 {
    my ( $ref ) = @_;
    $ref->{three} = 4;
    return $ref;
}
$ref = test3( $ref );

say Dumper $ref;

Here are some considerations when writing such functions:

By passing the function all of the variables it needs (test1), you don't need to rely on global variables. This makes the function easier to test, more predictable (no side effects) and easier to refactor as you can move it to another module or file easily. If it relied on global variables, it would be broken if the variable is moved or its data structures changed.
Returning the original reference is not necessary for test3, because the original $ref is changed. Returning is useful if you want to chain function calls.
Modifying the arguments passed your function directly can be surprising to the function's caller. If you do this, make it clear in the documentation. If you actually want to make a new copy of the object, modify it and return it, use something like Clone. Of course, this can be slow.
Just in case you are writing object accessors/mutators: please consider using a module such as Class::Accessor or the more heavyweight but extensive Moose which can generate such functions for you automatically.


Computing the subtotal of all items on their property criteria: qty * price I have an array which represent the item purchases of a customer, and computed the items subtotal in this logic:

function getSubtotal(arr) {
  let total = 0.00;
  arr.forEach(function(i) {
    total += (i.qty * i.price)
  });
  return total;  // 1274.21
}

And yes of course I get the correct result, but is there any efficient way to do this in JavaScript?

let data = [
  {
    name : 'Item 1', qty: 2, price: 15.50 // 31
  },
  {
    name : 'Item 2', qty: 17, price: 25.13 // 427.21
  },
  {
    name : 'Item 3', qty: 102, price: 8.00 // 816
  }
];

function getSubtotal(arr) {
  let total = 0.00;
  arr.forEach(function(i) {
    total += (i.qty * i.price)
  });
  return total; // 1274.21
}

document.write(getSubtotal(data));

You are computing the sum of an array, with a twist. Use Array.reduce() to compute sums.

Furthermore, if you are going to use the ECMAScript let language feature, you should also use arrow function notation.

function subtotal(items) {
    return items.reduce(
        ((total, item) => total + item.qty * item.price),
        0.00
    );
}

Better naming would help. "Get" implies that you are retrieving something that already exists, which is not the case here.  Also, i has a connotation of being an index counter, which makes its use in your forEach() confusing.


MIPS assembly program to read integer byte by byte I try to learn assembly by writing small programs. This MIPS program is supposed to read an integer from the terminal and print it byte by byte in hexadecimal. I think it works but I didn't check everything.

.data
prompt:     .asciiz "Enter an integer (positive or negative): "
s1:     .asciiz "B1:
s2:     .asciiz " B2:
s3:     .asciiz " B3:
s4:     .asciiz " B4:
MyNumber:       .word 0

.text
main:
    addi $v0, $zero, 4  #code 4 is to print string
    la $a0, prompt      #loads string into register
    syscall

    addi $v0, $zero, 5  #code 5 is to read an integer
    syscall

    sw $v0, MyNumber        #stores value from $v0 to input

    addi $v0, $zero, 4  #code 4 is to print string
    la $a0, s1      #loads string into register
    syscall

    la $t1, MyNumber
    lb $a0, 0($t1)
    lb $t0, MyNumber
    addi $v0, $zero, 34
    syscall

    addi $v0, $zero, 4  #code 4 is to print string
    la $a0, s2      #loads string into register
    syscall

    lb $a0, 1($t1)
    addi $v0, $zero, 34 #print in hexadecimal
    syscall

    addi $v0, $zero, 4  #code 4 is to print string
    la $a0, s3      #loads string into register
    syscall

    lb $a0, 2($t1)
    addi $v0, $zero, 34
    syscall

    addi $v0, $zero, 4  #code 4 is to print string
    la $a0, s4      #loads string into register
    syscall

    lb $a0, 3($t1)
    addi $v0, $zero, 34
    syscall

Firstly, you're missing some closing quotes on the data declarations at the top of the file. And you should specify that you make use of MARS specific calls.

However, when you read an integer using the "read integer" call, you read an 8-bit integer, that is, ranging from 0 to 255.

In your program you attempt to print four bytes out of this single byte, and therefore your program will print out garbage to the console.

You should simply remove the code that prints B2, B3 and B4.


Displaying all permutations of a string I am using this code to display all the permutations of a given input string. Any advice/solution will be very helpful.

#include<iostream>
using namespace std;

/* Function to swap two characters */
void swap(char& a, char& b)
{
 char temp;
temp = a;
a = b;
b = temp;
}
/* Function to obtain permutations of string characters */
void permutation(string s,int i,int n)
{
int j;
if (i == n)
    cout << s << "\t";
else
  {
    for (j = i; j < s.length(); j++)
    {
        swap(s[i],s[j]);
        permutation(s, i + 1, n);
        swap(s[i],s[j]);
    }
  }
}
int main()
{
 string s;
 cout << "Enter the amino acid string : ";
 cin >> s;
 cout << endl << "The permutations of the given string : " << endl;
 permutation(s, 0, s.length() - 1);
 cout << endl;
}

Note that using the standard library function std::next_permutation from the header <algorithm>, your code can more or less be reduced to:

#include <algorithm>
#include <iostream>
#include <string>

int main()
{
    std::string s;
    std::cout << "Enter the amino acid string: ";
    std::cin >> s;
    std::cout << "\nThe permutations of the given string:\n";

    std::sort(s.begin(), s.end());
    do
    {
        std::cout << s << '\n';
    } while(std::next_permutation(s.begin(), s.end()));
}

Note that the iterable (the string in our case) needs to be sorted for the call to std::next_permutation to work, hence the call to std::sort beforehand.

That said, solving your problem in a more idiomatic manner might be interesting, but there are some more things you may want to know if you want to write idiomatic C++ in the future:

You don't need to write your own swap function. The standard already provides std::swap to swap any two values.
using namespace std; is often considered bad practice. While it shouldn't matter in your case, using it in a header file will drag every name from the std:: namespace into the global namespace in every file including this header file. Generally speaking, unless you know what you're doing, avoid this construct and fully qualify every name from the standard library with std::.
You could use some additional spacing. Don't hesitate to separate your functions by empty lines and to add some more spaces between your functions parameters. It will make your code easier to read. Also, try to be consistent when you indent stuff; use the same number of spaces for indentation everywhere.


Get owner of ~5 million files and folders using Powershell and Robocopy I've written a Powershell script that uses Robocopy to get the full path of all files on a drive and then run it through a foreach loop to get the owner for each file and generate it in a CSV.  The code works, but is very slow when I run it against so many items.  Can someone help me make it as efficient as possible?  I also use "File System Security PowerShell Module 4.2.3" to use Get-NTFSOwner which gets me around the 256 character limit, which is also why I use Robocopy to generate the paths.

Import-Module NTFSSecurity

#Gets initial file and folder list
 $Folderpath = "G:\DEPT"
 $Folders = Get-Childitem $Folderpath

 ForEach ($Folder in $Folders) {Start-Job -ScriptBlock {
          Param($Folder,$Folderpath)
          robocopy "$FolderPath\$Folder" NULL /L /S /NJH /FP /NC /XJ /NS /NJH /NC /NJS /LOG:D:\AutoAssign\Data\RoboLogs\$Folder.txt

           } -ArgumentList $Folder,$Folderpath}

#Wait unitl all jobs are finished
Get-Job | Wait-Job

#Trims out white space and empty lines
  $Folderpath = "D:\AutoAssign\Data\RoboLogs"
  $Files = Get-Childitem $Folderpath

ForEach ($File in $Files) {Start-Job -ScriptBlock {
          Param($File,$Folderpath,$Files)
          $Trim = Get-Content "$Folderpath\$File"
          $Trim.trim() | Where { $_ } | Out-File "$Folderpath\$File"
           } -ArgumentList $File,$Folderpath,$Files }

#Wait unitl all jobs are finished
Get-Job | Wait-Job

#Gets Owner for each file And Create CSV

$Dir = "D:\AutoAssign\Data\RoboLogs"
$Files = Get-ChildItem $Dir
ForEach ($File in $Files){ Start-Job -ScriptBlock {
    Param($Dir,$Files,$File)

$OutputFile = "D:\AutoAssign\Data\Final\$File.csv"
$Results = @()

$Paths = Get-Content $Dir\$File
ForEach ($Path in $Paths){
 $Owner = get-ntfsowner $Path | Select Owner | ft -hidetableheaders | Out-String
 $Owner = $Owner.Trim()

      $Properties = @{

      Path = $Path
      Owner = $Owner

      }

If ($Owner -ne "BUILTIN\Administrators" -and $Owner -ne $null-and $Owner -ne "Domain\Domain Admins" -and $Owner -notlike "S-1*"){
    $Results += New-Object psobject -Property $properties}

    }

$Results | Export-Csv -notypeinformation -Path $OutputFile

} -ArgumentList $Dir,$Files,$File #Ends Initial ForEach

}  #Ends Job Script Block

#Wait unitl all jobs are finished
Get-Job | Wait-Job

#Merge all files into one CSV
Get-ChildItem  D:\AutoAssign\Data\Final | Select-Object -ExpandProperty FullName | Import-Csv | Export-Csv D:\AutoAssign\Data\G.csv -NoTypeInformation -Append

#Delete all original files
Get-ChildItem D:\AutoAssign\Data\RoboLogs | Remove-Item
Get-ChildItem D:\AutoAssign\Data\Final | Remove-Item

Your robocopy command lists /NC and /NJH twice.

$Folders and $Folder contain objects with properties, use these instead of treating them as strings (which implicit casts them as such) or pipe to Select-Object -ExpandProperty FullName
Same with $Files and $File

I'd not reuse variable names but define neccessary ones from begin.
Proper indenting eases reading/understanding the code IMO better than excess white space.

The following script isn't tested at all, see it as rough template:

## Q:\Test\2018\08\12\cr_201527.ps1
Import-Module NTFSSecurity

$Logs  = 'D:\AutoAssign\Data\RoboLogs\'
$Final = 'D:\AutoAssign\Data\Final\'
$Folderpath = "G:\DEPT"

#Gets initial file and folder list
$Folders = Get-Childitem $Folderpath

ForEach ($Folder in $Folders) {
    Start-Job -ScriptBlock { Param($Folder,$Logs)
        robocopy "$($Folder.FullName)" NULL /L /S /NJH /FP /NC /XJ /NS /NJH /NC /NJS /LOG:"$Logs$($Folder.Name).txt"
    } -ArgumentList $Folder,$Logs
}

#Wait until all jobs are finished
Get-Job | Wait-Job

<# functionality integrated into next sction
    #Trims out white space and empty lines
#>

#Gets Owner for each file And Create CSV

ForEach ($File in (Get-ChildItem $Logs)){
    Start-Job -ScriptBlock { Param($Logs,$File,$Final)
        $OutputFile = Join-Path $Final "$($File.Name).csv"
        $Results = ForEach ($Path in ((Get-Content $File.FullName).Trim() -ne '')){
            $Owner = (Get-NtfsOwner $Path).Owner.Trim()
            If ($Owner -ne "BUILTIN\Administrators" -and
                $Owner -ne $null                    -and
                $Owner -ne "Domain\Domain Admins"   -and
                $Owner -notlike "S-1*"){
                [PSCustomObject]@{
                    Path = $Path
                    Owner = $Owner
                }
            }
        }
        $Results | Export-Csv -NoTypeInformation -Path $OutputFile
    } -ArgumentList $Logs,$File,$Final #Ends Initial ForEach

}  #Ends Job Script Block

#Wait unitl all jobs are finished
Get-Job | Wait-Job

#Merge all files into one CSV
Get-ChildItem  D:\AutoAssign\Data\Final | Select-Object -ExpandProperty FullName |
    Import-Csv | Export-Csv D:\AutoAssign\Data\G.csv -NoTypeInformation -Append

#Delete all original files
Get-ChildItem D:\AutoAssign\Data\RoboLogs | Remove-Item
Get-ChildItem D:\AutoAssign\Data\Final | Remove-Item


Simple ticketing system Here is my simple ticketing system:

 <?php
        session_start();
        session_id();
        ob_start();
        require("../configuration/config.php");
        $GetTickets = $con->query("SELECT * FROM tickets WHERE open='true'");
        if(!$_SESSION['Admin']) {
        header('Location: login.php'); exit();
        }
        ?>
        <!DOCTYPE HTML>

            <title> ticketExpress | Admin </title>
            <link rel='stylesheet' href='../assets/css/style.css'>

  Logout

 Open Tickets

        <?php
        while($TicketInfo = $GetTickets->fetch_object()) {
        $Subject = $TicketInfo->Subject;
        if(strlen($Subject)>50) {
            $Subject = substr($Subject,0,50)."...";
        } else {
        $Subject = $TicketInfo->Subject;
        }
        echo "
".$Subject ."ID'>ID'> Close
";
        }
        if(isset($_GET['delete'])) {
        $ID = $_GET['delete'];
        echo "

            ";
        }
        if(isset($_GET['logout'])) {
        session_destroy();
        header('Location: login.php');
        }
        if(isset($_GET['close'])) {
        $ID = $_GET['close'];
        echo "

            ";
        }
        ?>

Could you give me tips on improving the performance of my script? I've noticed that it takes some time to receive a response, so performance can definitely be further improved. I've been doing PHP for the last 8 months or so and am still a newbie.

A few quick tips:

premature optimization is the root of all evil
use a profiling tool
premature optimization is the root of all evil
Don't echo markup, close php, write markup, open php tag again
premature optimization is the root of all evil
Dont query the DB, if you might have to change header('Location:...') a few lines further down. If you're going to have to redirect, do so ASAP.
premature optimization is the root of all evil
Avoid globals, they're error prone, hellish to debug and generally evil, but not as evil as premature optimization
premature optimization is the root of all evil

All in all, there's not enough code yet to squeeze any meaningful performance gain out of it. Not by altering the code alone, anyway.
If you do decide to use a profiling tool, also look into caching (google APC), and try your hand at bytecode caching. See how many ms you can shave off that way, but always remind yourself of one thing:

premature optimization is the root of all evil