Proper use of reduce, nested loops Below is an implementation of Dijkstra's shortest path algorithm. It's input graph is represented as an association list of source nodes to assoc of target node and arc weight.

My question is about the following, am I missing a way of simplifying min-path function used in reduce? Maybe the great and powerful loop could do reduce's job?

(defun dijkstra (source target graph)
  (labels ((min-path (&optional arc-1 arc-2)
             (let ((price-1 (cdr arc-1))
                   (price-2 (cdr arc-2)))
               (cond
                 ((and (not (null arc-1)) (not (null arc-2))) (if (< price-1 price-2)
                                                                arc-1 arc-2) )
                 ((and (not (null arc-1)) (null (arc-2))) arc-1)
                 ((and (not (null arc-2)) (null (arc-1))) arc-2))))
           (get-best-path (visited)
             (reduce #'min-path (loop for pick in visited
                                      for start = (car pick)
                                      for cost = (cdr pick)
                                      append (loop for arc in (cdr (assoc start graph))
                                                   when (null (assoc (car arc) visited))
                                                   collect (cons (car arc) (+ cost (cdr arc))))))))
    (loop named main-loop
          with visited = (list (cons source 0))
          for best-arc = (get-best-path visited)

          when (not (null best-arc))
          do (push best-arc visited)
          until (or (null best-arc)
                  (eql (car best-arc) target))
          finally
          (format t "Found shortest path from ~A to ~A: ~A~%" source target best-arc)
          (return-from main-loop (cdr best-arc)))))

I'm new to Lisp, and I would appreciate any advice, especially on bad Lisp habits and  bad Lisp style in my code, thanks.

UPD: new version with review comments applied - no warnings, unlike the original version.

(defun dijkstra (source target graph)
  (labels ((min-path (&optional arc-1 arc-2)
             (cond ((and arc-1 arc-2) (if (< (cdr arc-1) (cdr arc-2)) arc-1 arc-2))
                   (arc-1 arc-1)
                   (arc-2 arc-2)
                   (t nil)))
           (get-best-path (visited)
             (reduce #'min-path
                     (loop for pick in visited
                           for start = (car pick)
                           for cost = (cdr pick)
                           append (loop for arc in (cdr (assoc start graph))
                                        unless (assoc (car arc) visited)
                                        collect (cons (car arc) (+ cost (cdr arc))))))))
    (loop with visited = (list (cons source 0))
          for best-arc = (get-best-path visited)
          when best-arc do
          (push best-arc visited)
          until (or (null best-arc) (eql (car best-arc) target))
          finally (return (cdr best-arc)))))

First, (not (null x)) is not very idiomatic; just use x instead.

Next, you are not handling all the cased in your cond (and you obviously mean (null arc-2) and not (null (arc-2))). If fact, I would drop the let and use nested ifs instead.

Finally, to answer your original question: of course loop can do everything reduce can, but if your intent is clearer with the latter, use it!

EDIT: unless is more idiomatic than when (null ...).


Computing a prime table in Rust My objective is computing a table of primes up to n, where n is a natural number passed as an argument to the prime_table function.

Here is src/lib.rs:

#![feature(vec_remove_item)]

// Return a vector containing all the products between i and natural numbers beyond 2.
// The vector only includes products which are not greater than the maximum specified.
fn get_multiples(i: u32, max: u32) -> Vec<u32> {
    let mut results = Vec::new();
    let max_multiplier = max / i;
    for number in 2..=max_multiplier {
        results.push(i * number);
    }
    results
}

// Return a vector containg all the primes up to n.
pub fn prime_table(n: u32) -> Vec<u32> {
    let root_of_n = (n as f64).sqrt() as u32;
    let mut table: Vec<u32> = (2..=n).collect();
    for i in 2..=root_of_n {
        if !table.contains(&i) {
            continue;
        }
        for multiple in get_multiples(i, n).iter() {
            table.remove_item(&multiple);
        }
    }

    table
}

Something I'd like but I'm not sure is possible would be changing get_multiples to return an iterator of the results vector instead of the vector itself, but I'm not sure how. If there is a way, I suspect it necessitates lifetimes, something I haven't grasped yet

My main concern is avoiding unnecessarily allocating memory.

Returning an iterator is fairly easy with impl. Here's how you can do it. Note that you need to use move with the closure so that it takes ownership of i. Since u32 can be copied, this just copies it instead of taking a reference, which lets you avoid dealing with lifetimes on the return type.

fn get_multiples(i: u32, max: u32) -> impl Iterator<Item = u32> {
    let max_multiplier = max / i;
    (2..=max_multiplier).map(move |n| i * n)
}

As another note, I have a recommendation for improving performance on your sieve. Currently, every time you call remove_item, you have to search all the way through the vector, remove the item, then shift all items after it forward by one. This is awful for performance. Instead, you could have a Vec<bool> and simply flip the value based on index rather than remove it. Then when you're done sieving, you can go through this vector along with indices and create a Vec<u32> to return.


Convert a string into an R vector expression Problem: Given a arbitrary string, split by a given regex and format into an R vector expression. E.g. Convert "a..,b..,c,d" into 'c("a","b","c","d")', when the given regex is "[.,]+".

Solution in haskell:

module Lib
    ( toRVec
    ) where

import           Data.List  (intersperse)
import           Data.Maybe (fromMaybe)
import           Data.Text  ()
import           Data.Text  (pack, strip, unpack)
import           Text.Regex (mkRegex, splitRegex)

(//) :: Maybe a -> a -> a
(//) = flip fromMaybe

stripString = (unpack . strip . pack)

toRVec :: String -> Maybe String -> Maybe Bool ->  String
toRVec orig delim' toStrip' =
  let
      delim = delim' // "[ \n]+"
      toStrip = toStrip' // True
      ws = splitRegex (mkRegex delim) $ stripString orig
      ws1 = map (\w -> "\"" ++ w ++ "\"") $ if toStrip then
                                                      map stripString ws
                                                    else
                                                      ws
      ws2 = intersperse "," ws1
      res = foldl (++) "" ws2
   in "c(" ++ res ++ ")"

Any suggestion for improvements are welcome.

How I would do it:

import Data.List.Split
import Data.List
foo delim s = "c(\"" ++ intercalate "\",\"" (wordsBy (`elem` delim) s) ++ "\")"

reads yours

No brackets needed around unpack . strip . pack.

I would replace if toStrip then map stripString ws else ws with (if toStrip then map stripString else id) ws.

Some of your names are unneeded; I would inline ws, ws1 and ws2 into where they are used (once ws is only used once), perhaps even res.

Instead of foldl (++), use concat.


Counts the number of times a particular character, such as e, appears in a file This question came from a Java tutorial online:

  Write an example that counts the number of times a particular character, such as e, appears in a file.

I was just wondering if there is any way to improve this particular solution.

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

public class CountLetter{
    public static void main(String[] args) {
        Path file = Paths.get("/Users/justin/Desktop/Codes Netbean/JavaRandom/xanadu.txt");
        CountLetter cl1 = new CountLetter(file, 'e');
        System.out.println(cl1.count());
    }

    private Path file;
    private char lookFor;

    CountLetter(Path file, char lookFor){
        this.file = file;
        this.lookFor = lookFor;
    }

    private int count(){
        int count = 0;
        try(BufferedReader br = new BufferedReader(new InputStreamReader(Files.newInputStream(file)))){
            String line = null;
            while((line = br.readLine()) != null){
                for(int i = 0; i < line.length(); i++){
                    if(line.charAt(i) == lookFor){
                        count++;
                    }
                }
            }
        } catch (IOException x){
            System.err.println(x);
        }
        return count;
    }
}

If you are using java 8 you can use streams.

Also IMO for this kind of functionality a static method would me more appropriate.

 public static long countCharacter(Path path, char lookFor) throws IOException {

     try (Stream<String> stream = Files.lines(path)) {

         return stream.flatMapToInt(String::chars)
                      .filter(c -> c == (int) lookFor)
                      .count();
     }
 }


Beginner Tic-Tac-Toe in Python def draw_board(board):
    display_board = ''

    for number in list(range(len(board))):
        if (number + 1) % 3 == 0:
            if number < 6:
                display_board += '  ' + str(board[number]) + '  \n—————————————————\n'
            else:
                display_board += '  ' + str(board[number]) + '  '
        else:
            display_board += '  ' + str(board[number]) + '  |'

    return display_board

def check_win(board, letter):
    if all(item == letter for item in board[:3]):
        return True
    elif all(item == letter for item in board[3:6]):
        return True
    elif all(item == letter for item in board[6:9]):
        return True
    elif all(item == letter for item in board[::3]):
        return True
    elif all(item == letter for item in board[1::3]):
        return True
    elif all(item == letter for item in board[2::3]):
        return True
    elif all(item == letter for item in board[::4]):
        return True
    elif all(item == letter for item in board[2:8:2]):
        return True

x = input('Who is X?  > ')
o = input('Who is O?  > ')

squares = {
    'nw': 0, 'n': 1, 'ne': 2,
    'w': 3, 'c': 4, 'e': 5,
    'sw': 6, 's': 7, 'se': 8
}

directions = list(squares.keys())

tic_tac_toe_board = [
    '.', '.', '.',
    '.', '.', '.',
    '.', '.', '.'
]

valid_turns = 0
x_turn = True
print(draw_board(tic_tac_toe_board) + '\n(Xs first) ', end='')

while valid_turns < 9:
    square = input('Exactly type in the following available directions: ' + ', '.join(directions) + '  > ')

    if square.lower() in directions:
        directions.remove(square.lower())

        if x_turn:
            tic_tac_toe_board[squares[square.lower()]] = 'X'
            x_turn = False
        else:
            tic_tac_toe_board[squares[square.lower()]] = 'O'
            x_turn = True

        valid_turns += 1
    else:
        print('INVALID MOVE')

    print(draw_board(tic_tac_toe_board))

    if check_win(tic_tac_toe_board, 'X'):
        print(x + ' has won!')
        break
    elif check_win(tic_tac_toe_board, 'O'):
        print(o + ' has won!')
        break

if valid_turns == 9:
    print('DRAW!')

Ways of improving/optimizing:

draw_board function.

prefer enumerate(board) over redundant list(range(len(board)))
all conditional branches within for loop have the same common expression to append '  ' + str(board[number]) + '  '. Thus, it can be moved up to reduce conditional branches
prefer flexible f-string formatting over awkward string concatenation combined with casting to str

Now the optimized draw_board function would look as:

def draw_board(board):
    display_board = ''

    for i, v in enumerate(board):
        display_board += f'  {v}  '
        if (i + 1) % 3 == 0:
            if i < 6:
                display_board += '\n—————————————————\n'
        else:
            display_board += '|'

    return display_board

check_win function introduces many repetitive checks all(item == letter for item in board[...]) with the same returned value.
A more flexible way is to create an additional function to check crossed row:

def row_crossed(letter, row):
    return all(item == letter for item in row)

Then, check_win would call any function on generator expression that yields all the needed sub-checks:

def check_win(board, letter):
    return any((row_crossed(letter, board[:3]), row_crossed(letter, board[3:6]),
            row_crossed(letter, board[6:9]), row_crossed(letter, board[::3]),
            row_crossed(letter, board[1::3]), row_crossed(letter, board[2::3]),
            row_crossed(letter, board[::4]), row_crossed(letter, board[2:8:2])))

square dictionary. To reduce quotes mess it can be composed in concise way with zip and range functions:

squares = dict(zip(('nw', 'n', 'ne', 'w', 'c', 'e', 'sw', 's', 'se'), range(9)))

tic_tac_toe_board easily replaced with tic_tac_toe_board = ['.'] * 9
square.lower() repeats 4 times within main while loop. Instead, extract that expression into a variable at once and refer it:

square = input('Exactly type in the following available directions: {}  > '.format(', '.join(directions)))
square = square.lower()

the whole condition:

if x_turn:
    tic_tac_toe_board[squares[square.lower()]] = 'X'
    x_turn = False
else:
    tic_tac_toe_board[squares[square.lower()]] = 'O'
    x_turn = True

would be simplified as it relies on explicit negation of x_turn flag.Getting  2 lines against 6:

tic_tac_toe_board[squares[square]] = 'X' if x_turn else 'O'
x_turn = not x_turn


Using Python to create a JSON file of recipes from MySQL I needed to create a JSON file from a couple of MySQL tables to migrate some recipes from wordpress.
This was my first time writing a script in python and would like to get some feedback on what I could have done better. I come from a Javascript background.

import mysql.connector
import json

cnx = mysql.connector.connect(
    database='somedb', user='root', password='root', host='localhost'
)

cnx1 = mysql.connector.connect(
    database='somedb', user='root', password='root', host='localhost'
)

cursor = cnx.cursor();
cursor1 = cnx1.cursor();

query = "SELECT a.recipe_id, a.recipe_item_type, b.meta_key, b.meta_value, b.recipe_item_id FROM wp_simmer_recipe_items a, wp_simmer_recipe_itemmeta b WHERE a.recipe_item_id = b.recipe_item_id GROUP BY a.recipe_item_id"
query1 = "SELECT * FROM wp_simmer_recipe_itemmeta WHERE recipe_item_id=(%s)"

cursor.execute(query)
rs = cursor.fetchall()

data = {}

for row in rs:
    if row[1] == 'instruction':
        cursor1.execute(query1, (row[4],))
        insD = cursor1.fetchall()
        for instruction in insD:
            if instruction[2] != 'is_heading':
                data.setdefault(row[0], {'instructions':[], 'ingredients': []})['instructions'].append(instruction[3]);
    else:
        cursor1.execute(query1, (row[4],))
        rd = cursor1.fetchall()
        ingredient = {}
        for itemMeta in rd:
            ingredient[itemMeta[2]] = itemMeta[3]
        data.setdefault(row[0], {'ingredients': [], 'instructions': []})['ingredients'].append(ingredient)

with open('data.json', 'w') as outfile:
    json.dump(data, outfile, sort_keys=True, indent=4)

cursor1.close()
cnx1.close()
cursor.close()
cnx.close()

Preface

For this review, we shall need to refer to the database schema for the WordPress Simmer plugin:

if ( $items_table_name != $wpdb->get_var( "SHOW TABLES LIKE '$items_table_name'" ) ) {
        // The recipe items table.
        $query .= "CREATE TABLE $items_table_name (
               recipe_item_id bigint(20) NOT NULL auto_increment,
               recipe_item_type varchar(200) NOT NULL DEFAULT '',
               recipe_id bigint(20) NOT NULL,
               recipe_item_order int(11) NOT NULL DEFAULT '0',
               PRIMARY KEY  (recipe_item_id),
               KEY recipe_id (recipe_id)
               ) $charset_collate;";
}

if ( $item_meta_table_name != $wpdb->get_var( "SHOW TABLES LIKE '$item_meta_table_name'" ) ) {

        // The recipe item meta table.
        $query .= "CREATE TABLE $item_meta_table_name (
               meta_id bigint(20) NOT NULL auto_increment,
               recipe_item_id bigint(20) NOT NULL,
               meta_key varchar(255) NULL,
               meta_value longtext NULL,
               PRIMARY KEY  (meta_id),
               KEY recipe_item_id (recipe_item_id),
               KEY meta_key (meta_key)
               ) $charset_collate;";
}

Querying

To start, let's format your first query for readability:

query = """
    SELECT a.recipe_id, a.recipe_item_type
         , b.meta_key, b.meta_value, b.recipe_item_id
        FROM wp_simmer_recipe_items a, wp_simmer_recipe_itemmeta b
        WHERE a.recipe_item_id = b.recipe_item_id
        GROUP BY a.recipe_item_id
"""

Your query has a non-sensical GROUP BY clause.  The only way that this query could be legal is if the schema and query are formulated so as to guarantee that each recipe_item_id only appears at most once in the query result.  While the wp_simmer_recipe_items table does have a PRIMARY KEY (recipe_item_id) constraint, there is no such uniqueness guarantee in the wp_simmer_recipe_itemmeta table.  MySQL < 5.7.5 somehow executes the query despite the fact that it makes no sense; in MySQL ≥ 5.7.5, it should fail with an error, as it should in any sane SQL implementation:

  ERROR 1055 (42000): Expression #3 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'somedb.b.meta_key' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by

Based on the query results, you build the ingredients and instructions lists by appending entries.  However, you fail to ORDER BY recipe_item_order, so the ingredients and instructions may appear in any order.  (Try baking the bread before mixing the dough!)

When joining tables, write the query using JOIN. It's more readable than implicitly joining tables using the WHERE clause.

The worst sin in this code is executing query1 for each recipe found by query.  This is a bad idea for two reasons:

For good performance, you should never execute queries in a loop, especially when the number of secondary queries depends on the length of the results of the primary query.  Each additional query necessitates a round trip to the server, and takes time to interpret and execute.
The information returned by query1 is entirely redundant — all of the information you need from it (namely, the meta_key and meta_value for each recipe_item_id) is already contained within the results of query!

Furthermore, you don't need to make two connections to the same database.  All you need is two cursors, which can be created from just one connection.

Suggested solution

Execute two queries: one to get the instructions, one to get the ingredients. (Here, I've taken a shortcut and assumed that every recipe will have at least one instruction and at least one ingredient.)

Take advantage of itertools.groupby() to help construct the inner lists and dictionaries.

from itertools import groupby
import json
from operator import itemgetter
import mysql.connector

cnx = mysql.connector.connect(
    database='somedb', user='root', password='root', host='localhost'
)
cursor = cnx.cursor()
data = {}

cursor.execute("""
    SELECT item.recipe_id
         , meta.meta_value AS instruction_text
        FROM wp_simmer_recipe_items item
            INNER JOIN wp_simmer_recipe_itemmeta meta
                ON item.recipe_item_id = meta.recipe_item_id
        WHERE
            item.recipe_item_type = 'instruction'
            AND meta.meta_key <> 'is_heading'
        ORDER BY item.recipe_id, item.recipe_item_order
""")
for recipe_id, instructions in groupby(cursor.fetchall(), itemgetter(0)):
    data[recipe_id] = {
        'instructions': [row[1] for row in instructions],
        'ingredients': [],
    }

cursor.execute("""
    SELECT item.recipe_id
         , meta.recipe_item_id
         , meta.meta_key
         , meta.meta_value
        FROM wp_simmer_recipe_items item
            INNER JOIN wp_simmer_recipe_itemmeta meta
                ON item.recipe_item_id = meta.recipe_item_id
        WHERE
            item.recipe_item_type <> 'instruction'
        ORDER BY item.recipe_id, item.recipe_item_order, meta.meta_key
""")
for (recipe_id, item_id), item in groupby(cursor.fetchall(), itemgetter(0, 1)):
    data[recipe_id]['ingredients'].append({row[2]: row[3] for row in item})

cursor.close()
cnx.close()

with open('data.json', 'w') as outfile:
    json.dump(data, outfile, sort_keys=True, indent=4)

Further discussion

MySQL, since version 5.7.22, has JSON support.  It would be nice to rewrite the SQL such that the entire JSON result could be returned from just a single query.  Unfortunately, the JSON_ARRAYAGG() function does not let you specify the order of its results:

  JSON_ARRAYAGG(col_or_expr)

  Aggregates a result set as a single JSON array whose elements consist of the rows. The order of elements in this array is undefined.

That, to me, would be a deal-breaker for exporting a recipe.


Is there some way to simplify this loop I have a cycle which goes from -55 to -1, then from 1 to 55 before going back to -55 again. I want to make a loop going from an arbitrary point in this loop to another arbitrary point, but never as far as a full cycle.
Since the end value of the counter might be either a larger or smaller than the start value, I initially made the exit condition for the loop that fra equals til.
However, I also want to do something when fra = til, so in the end my loop ended up looking like what you can see below, with a helper-variable triggering the exit.
I am not all that happy with how this ended up looking though, feeling the logic is a bit clunky. Is there some way to improve the code below?
Option Explicit

Dim første_økt_anodeskift(1 To 10, 1 To 2) As Long

Const farge_anodeskift_1 As Long = 14395790 ' RGB(142, 169, 219)

Sub main()
    Dim syklus As Long

    syklus = 1

    Call legg_inn_verdier
    Call fargelegg(første_økt_anodeskift(syklus, 1), første_økt_anodeskift(syklus, 2), farge_anodeskift_1)
End Sub

Private Sub fargelegg(fra, til, farge)
    Dim exit_on_next As Boolean

    exit_on_next = False

    Do
        ' Do stuff

        If exit_on_next Then Exit Do

        fra = fra + 1

        If fra = 0 Then
            fra = 1
        ElseIf fra = 56 Then
            fra = -55
        End If

        If fra = til Then exit_on_next = True
    Loop While True
End Sub

Private Sub legg_inn_verdier()
    første_økt_anodeskift(1, 1) = -11: første_økt_anodeskift(1, 2) = 6
End Sub
```

Maybe this looks better to you.  Encapsulating a loop's contained logic in dedicated single-purpose procedures generally makes a loop easier to read and understand(IMO).  In this case, the loop conditions requiring the repeated code to run 'one last time' becomes a little more clear as well.
    Option Explicit

    Dim første_økt_anodeskift(1 To 10, 1 To 2) As Long

    Const farge_anodeskift_1 As Long = 14395790 ' RGB(142, 169, 219)

    Sub main()
        Dim syklus As Long

        syklus = 1

        Call legg_inn_verdier
        Call fargelegg(første_økt_anodeskift(syklus, 1), første_økt_anodeskift(syklus, 2), farge_anodeskift_1)
    End Sub

    Private Sub fargelegg(fra, til, farge)
        Do
            DoStuff fra, til, farge

            fra = GetNextFra(fra)

            If fra = til Then
                DoStuff fra, til, farge
                Exit Do
            End If

        Loop While True
    End Sub

    Private Sub legg_inn_verdier()
        første_økt_anodeskift(1, 1) = -11: første_økt_anodeskift(1, 2) = 6
    End Sub

    Private Sub DoStuff(fra, til, farge)
        ' Do stuff
    End Sub

    Private Function GetNextFra(fra) As Variant
        fra = fra + 1

        If fra = 0 Then
            fra = 1
        ElseIf fra = 56 Then
            fra = -55
        End If

        GetNextFra = fra
    End Function


Selecting function overload based on a type match with return value of passed callable On an std::list<A> or std::vector<A> or other collection container, the functor map (fmap) operation takes a function/callable of type B(A) and applies it elementwise to a list of As to get a list of Bs:

// Pseudo-code
container<B> fmap( B(A) f, container<A> xs)
{
  // Apply f elementwise to xs, and collect the results and...
  return result;
}

If A and B are the same, then fmap can take the list by copy, and mutate its copy and return it. Otherwise, I want to take the input container by const reference, and create a new container for the results. The following code works (using clang++4.0 -std=c++1z) but it's just so unspeakably ugly for such a simple thing.

#include <algorithm>
#include <functional>
#include <iostream>
#include <iterator>
#include <numeric>
#include <type_traits>
#include <vector>

// clang-format off
template <template <typename, typename> class Container, typename A, typename F,
  // Enables this overload if the container's element type matches the output
  //   type of the callable:
  typename = std::enable_if_t<std::is_same< A,
    decltype(std::invoke(std::declval<F>(), std::declval<A>()))>::value>>
auto fmap(F f, Container<A, std::allocator<A>> xs) {
  std::cout << "I/O type SYMMETRY DETECTED!!\n";
  std::transform(std::begin(xs), std::end(xs), std::begin(xs), f);
  return xs;
}
// clang-format on

// clang-format off
template <template <typename, typename> class Container, typename A, typename F,
typename = std::enable_if_t< ! std::is_same< A,
  decltype(std::invoke(std::declval<F>(), std::declval<A>()))>::value>>
auto fmap(F f, const Container<A, std::allocator<A>>& xs) {
  std::cout << "I/O type MISMATCH DETECTED!!\n";
  using B = decltype(f(std::declval<A>()));
  using ContainerOfB = Container<B, std::allocator<B>>;

  ContainerOfB result;

  if constexpr(std::is_same<ContainerOfB, std::vector<B>>::value) {
    result.reserve(xs.size());
  }
  std::transform(std::begin(xs), std::end(xs), std::back_inserter(result), f);

  return result;
}

template <typename T>
void printrange(const T r) {
  std::copy(std::cbegin(r), std::cend(r),
            std::ostream_iterator<decltype(*std::begin(r))>(std::cout, ", "));
}

int main() {
  std::vector<int> v(20);
  std::iota(v.begin(), v.end(), 0);

  // clang-format off
  printrange(
      fmap([](int i) { return '\'' + std::to_string(i) + '\''; },
      fmap([](int x) { return x * x; },
      fmap([](int x) { return x + 1; }, v) ))
  );
  // clang-format on

  return 0;
}

Forgive me, I'm new to to template metaprogramming, and I'm aware that I'm not doing anything difficult. In fact, everything I'm using is in <type_traits>, so I'm not really doing any metafunction programming, I'm just using metafunctions! But the code I just wrote is so heinously ugly that I'm sure it can be polished and prettied. I seek the greater wisdom of the community to help me clean up this mess.

Higher level overview:
Personally I think the architecture has limited use cases. It was quite hard to implement, and even then doesn't match the specification.
From what I understood, this is the problem definition:

Given a container with element type T, and a callable which has return type U, create a new container with element type U and populate it by calling callable on each element of the first container. If T and U are the same, take the container by copy and transform it, otherwise use back inserter to populate it.

Although it is already good, in realm of template metaprogramming this is insufficient.
Implementation overview:
The solution places superficial limits (as per the problem definition). There are two problems in C++ template metaprogramming: assuming too much and assuming too little. Both of those happened in this case. Most of the time it is hard to tell if too little is assumed or too much, so they are mostly interchangeable.
The most obvious superficial assumptions are made here:
Container<A, std::allocator<A>>

Very few hand-rolled containers will have allocator type, or even any types at all. Remapping from one type to another is quite hard, and is the most significant problem in this case.
Somewhere the other stuff except the element type is left out:
if constexpr(std::is_same<ContainerOfB, std::vector<B>>::value) {
   result.reserve(xs.size());
}

A vector which has non-standard allocator will probably benefit from reserving too.
I don't think the code itself is unreadable. After some time on codereview and around a year of template metaprogramming this code is still readable. Although I would note that the comments in the center of the template parameter declaration got me really confused. I would put that above or below parameter declaration part.
Another solution:
I gave it a shot. I believe I covered more cases, at the expense of code size, although I think readability increased a bit:
namespace detail
{
    template <typename Container>
    using value_type = typename Container::value_type;

    template <typename Callable, typename Container>
    using invoke_result = std::result_of_t<Callable(value_type<Container>)>;
}

template <typename Container, typename Callable>
struct mutable_in_place :
        public std::bool_constant<std::is_same_v<detail::value_type<Container>,
                                  detail::invoke_result<Callable, Container>>>
{};

namespace detail
{
    template <template <typename ...> typename Container, typename T, typename ... TArgs, typename Callable>
    auto realfmap(Container<T, TArgs...> container, Callable&& callable, std::true_type)
    {
        std::transform(container.begin(), container.end(),
                       container.begin(), std::forward<Callable>(callable));
        return container;
    }

    template <template <typename ...> typename Container, typename T, typename ... TArgs, typename Callable>
    auto realfmap(Container<T, TArgs...>&& container, Callable&& callable, std::false_type)
    {
        using parameter_type = decltype(*container.begin());
        using invoke_result = std::result_of_t<Callable(parameter_type)>;
        Container<invoke_result> mapped_container;

        if constexpr(std::is_same_v<decltype(mapped_container), std::vector<invoke_result>>)
        {
            mapped_container.reserve(container.size());
        }

        std::transform(container.begin(), container.end(), std::back_inserter(mapped_container),
                       std::forward<Callable>(callable));
        return mapped_container;
    }
}

template <typename Container, typename Callable>
auto realfmap(Callable&& callable, Container&& container)
{
    return detail::realfmap(std::forward<Container>(container),
                            std::forward<Callable>(callable),
                            mutable_in_place<std::decay_t<Container>, Callable>{});
}

Although it grabs the other template arguments, it cannot use them. For example, consider the case when container=std::vector<int> and callable=std::string(int). The allocator which should get instantiated for the returning vector is std::allocator<std::string>, but the grabbed one is std::allocator<int>, so static assert will fire or some other library error notification facility, since std::vector<T>::value_type and std::vector<T>::allocator::value_type must match.
The reasoning behind not putting mutable_in_place is to expose this to the user. Some people could do template metaprogramming on top of this, and sometimes people might run into template bugs, and exposing the variable template should aid them.
About using std::result_of_t<>, the reason is that it is much easier to convert the type trait compared to hand rolled version with decltype() and stuff. It might even be possible to write a python script or something similar to convert throughout the code base, since the syntax is trivial.
Don't get me wrong, but plain transform has clear advantage here, both semantics and implementation wise.
Change the problem:
It seems like solving the current problem will have a very big engineering cost, thus I propose the following problem definition:

An StdContainer: type which exactly matches declaration template <typename ElemType, typename Allocator = std::allocator<ElemType>> class container except identifier names (type parameters and class name itself) and is allocator-aware. Given an StdContainer and Callable, create and populate another StdContainer with return type of the Callable as element type. Creating and populating depends on the equality of element type of the original container and return type of the callable. If they match, a copy is taken and std::transform() is called. Otherwise, the original container is taken by reference, new one is default constructed and it is populated though calling std::transform() with std::back_inserter() as output iterator.

Your solution will solve the new problem and won't have any bugs. Also, for me it would be sufficient to pass into code base, with some cosmetic tweaking.


GET multiple data frames using external module's function that uses requests Purpose

The point of this mini project is to quickly gather data from a website's API and combine the collected DataFrames into a "master" DataFrame with all of the stocks I am interested in looking at. I do this using the iexfinance module. If needed, here is the code behind iexfinance.

Improvements?

While this module does a great job of dealing with the API, I'm not sure my code is as fast as it could be.  Because the iexfinance module uses requests to make GET requests, I believe there should be a faster way to asynchronously send GET requests instead of having to use multiprocessing. Below is my code:

import pandas as pd
import numpy as np
from iexfinance import Stock
from multiprocessing import Pool
import os

def iex_get_stat(batch):
    """
    :param batch: batch is a list of stock tickers (ex: ["AAPL", "MSFT", "TSLA"]
    Gets and returns DataFrames of stats on a list of stocks
    """
    frame = Stock(batch, output_format="pandas").get_key_stats().T
    return frame

def get_stats(symbols):
    """
    :param symbols: List of symbols (can only send 100 at a time,
    so these may need to be broken up)
    :return: Pandas DataFrame with stats
    """
    symbols = [symbols[i : i + 99] for i in range(0, len(symbols), 99)]
    frames = []
    pool = Pool(processes=os.cpu_count())
    frames.append(pool.starmap(iex_get_stat, [[batch] for batch in symbols]))
    pool.close()
    pool.join()
    stats = pd.concat(frames[0])
    return stats

if __name__ == "__main__":
    symbols = ["AAPL", "TSLA", "MSFT"]
    stats = get_stats(symbols)
    print(stats)

Your docstrings look a bit weird to me. The first line is usually a summary of what the function does. In your first function that line comes after the parameters and in the second one it does not exist at all. In get_stats when describing the parameter symbols there is no need to describe what the function internally does with it (i.e. split it up). This is an implementation detail (if it mutated the original list that might be worth mentioning but you don't do that).
I would put the chunking line into its own function, so you could change it if you want to at some time:

def chunks(it, n):
    """Splits `it` into chunks of length `n` (last chunk might be smaller)."""
   return [it[i : i + n] for i in range(0, len(it), n)]

The processing part is unnecessarily long. You could just write:

def get_stats(symbols):
    """
    :param symbols: List of symbols (can only send 100 at a time,
    so these may need to be broken up)
    :return: Pandas DataFrame with stats
    """
    with Pool() as pool:
        return pd.concat(pool.starmap(iex_get_stat, chunks(symbols, 99))))

Note that when exiting, this calls pool.terminate() instead of pool.close(); pool.join(). If you want to switch to the async methods this means that it will not wait for your workers to finish, but since you are not (I think), it should be fine.

Pool also automatically chooses os.cpu_count() when no number of processes is passed.
I would also move encapsulating each batch within another list into iex_get_stat, so you can just pass it a list instead (which is then actually what it says in the docstring):

def iex_get_stat(batch):
    """
    :param batch: batch is a list of stock tickers (ex: ["AAPL", "MSFT", "TSLA"]
    Gets and returns DataFrames of stats on a list of stocks
    """
    return Stock([batch], output_format="pandas").get_key_stats().T


PHP remote upload security I need to upload images to a remote HTTP server from a local script. The image name format is {32-char MD5}.jpg. Server directory format is 6c/d3/6cd3556deb0da54bca060b4c39479839.jpg. So, for example, if I upload the image called 6cd3556deb0da54bca060b4c39479839.jpg, it should be available at https://example.com/6c/d3/6cd3556deb0da54bca060b4c39479839.jpg.

The local script basically does a HTTPS multipart/form-data POST request with the "file" and "password" fields to a remote server (https://example.com/up.php).

Remote server has a PHP script (up.php) which checks if the "password" is correct and then either allows file uploading or disallows it.

Is this script secure? What can go wrong?

This is the code I use on the server. It checks password against SHA-512 hash and then checks if the filename extension is ".jpg".

<?php
// Upload Script
//
// You need to post "file" and "password", which is checked
// against using SHA-512

// Check if the password is correct
$password_hash = "128-characters long SHA-512 string here";
if (hash("sha512", $_POST["password"]) != $password_hash) {
    http_response_code(403);
    echo "403 Access Denied";
    exit(1);
}

if(!empty($_FILES["file"])) {
    // Check if the filename extension is ".jpg",
    // otherwise deny uploading
    $file_parts = pathinfo($_FILES["file"]["name"]);
    if ($file_parts["extension"] != "jpg") {
        http_response_code(500);
        echo "Incorrect File Extension";
        exit(1);
    }

    // Filename of the file being uploaded
    $filename = $_FILES["file"]["name"];

    // Build a directory path
    $tmp_a = mb_substr($filename, 0, 2); // First 2 chars of a filename
    $tmp_b = mb_substr($filename, 2, 2); // 3 to 4 chars of a filename
    $tmp_c = $filename;
    $directory = "$tmp_a/$tmp_b";
    // Full path to the file including the directory
    $full_path = "$directory/$filename";

    // If the directory doesn't exist, then create it
    if (!file_exists($directory)) {
        mkdir($directory, 0755, true);
    }

    // Upload the file
    if (move_uploaded_file($_FILES["file"]["tmp_name"], $full_path)) {
        echo "Success";
    } else {
        http_response_code(500);
        echo "Something went wrong";
        exit(1);
    }
}

Because you specifically ask about security:

For comparing hashes like SHA, you should use hash_equals().

If you are requiring the use of a password, you shouldn't use SHA-512. While it's certainly better than MD5, instead use bcrypt. Bcrypt is heavily recommended for password storage, especially long-term.
With bcrypt, instead of using hash() and hash_equals(), you would use password_hash() and password_verify().

Should new directories really have read and execute permissions for the group and other users? Likewise, I would check the permissions of the file itself.
I would personally have the files as 0600 and directories as 0700 unless other permissions are needed.

Checking the file extension is only helpful for preventing naive false uploads. On Linux and most UNIX-like operating systems, a file extension means very little (if anything). It could be any type of file with a JPEG extension, likewise anything could have a JPEG file extension.

I don't know about the rest of the code.


refactor function so same code is written only once I have this code

@objc func didClickActionButton(sender: UITapGestureRecognizer) {

    if let contents = contentList {

      if contents[currentScreenIndex].customMeta["permission-location"] != nil {
        //request Permission
        ApiHelper.shared.initLocationHelperWithBeacon()
      }
      if contents[currentScreenIndex].customMeta["permission-notification"] != nil {
//        PushNotificationHelper.requestNotificationAuthorization { }
        PushNotificationHelper.requestNotificationAuthorization {
          DispatchQueue.main.async {
            if self.currentScreenIndex == contents.count - 1 {
                self.finishOnboarding()
              return
            }
           self.currentScreenIndex+=1
           self.showScreenAtPosition(position: self.currentScreenIndex)
          }
        }
        return
      }
      if currentScreenIndex == contents.count - 1 {
        finishOnboarding()
        return
      }
      currentScreenIndex+=1
      showScreenAtPosition(position: currentScreenIndex)
    }
  }

As you can see I have the same code in the DispatchQueue.main.async as I have at the bottom at the func. Is there anyway to refactor this so I don't write the same code twice? Is it even worth it to refactor this?

Refactor the repeated code into its own private function:

@objc func didClickActionButton(sender: UITapGestureRecognizer) {
    guard let contents = contentList else { return }

    if contents[currentScreenIndex].customMeta["permission-location"] != nil {
        //request Permission
        ApiHelper.shared.initLocationHelperWithBeacon()
    }

    if contents[currentScreenIndex].customMeta["permission-notification"] != nil {
        PushNotificationHelper.requestNotificationAuthorization {
            DispatchQueue.main.async {
                self.advanceOnboarding()
            }
        }
        return
    }

    advanceOnboarding()
}

private func advanceOnboarding() {
    if currentScreenIndex == contents.count - 1 {
        finishOnboarding()
        return
    }

    currentScreenIndex += 1
    showScreenAtPosition(position: currentScreenIndex)
}

I’d also suggest the guard for the early exit, as shown above.

  Is it even worth it to refactor this?

Refactoring this code out into its own function not only avoids the unnecessary repetition (simplifying maintenance in the future), but it also makes this routine much easier to reason about. When first reading your code, it took a few seconds to figure out what you were trying to do, why that code was being repeated, etc. Giving that block of code a clear name makes it easier to grok what’s going on.


Efficiently convert 60 GB JSON file to a csv file Description
Simply take a JSON file as input and convert the data in it into a CSV file. I won't describe the functionality in too much detail since I have reasonable docstrings for that. As you can see, my solution is not memory efficient since I'm reading all the file into memory.
I'd like to improve the performance of my solution as much as possible. (perhaps not load everything at once into memory -- even if it's gonna be slower).
The JSON file that I'm trying to convert is 60 GB and I have 64GB of RAM.

Code
import csv
import json

CSV_PATH = 'file.csv'
JSON_PATH = 'file.json'

def flattenjson(json_data, delim):
    """
    Flatten a simple JSON by prepending a delimiter to nested children.

    Arguments:
        json_data (dict): JSON object
            e.g: {
                    "key1": "n1_value1",
                    "key2": "n1_value2",
                    "parent1": {
                        "child_key1": "n1_child_value1",
                        "child_key2": "n1_child_value2"
                    }
                }
        delim (str): Delimiter for nested children (e.g: '.')

    Returns:
        Flattened JSON object.
            e.g: {
                    'key1': 'n1_value1',
                    'key2': 'n1_value2',
                    'parent1.child_key1': 'n1_child_value1',
                    'parent1.child_key2': 'n1_child_value2'
                }
    """

    flattened_json = {}
    for i in json_data.keys():
        if isinstance(json_data[i], dict):
            get = flattenjson(json_data[i], delim)
            for j in get.keys():
                flattened_json[i + delim + j] = get[j]
        else:
            flattened_json[i] = json_data[i]

    return flattened_json

def write_json_to_csv(flattened_json, csv_path):
    """
    Write flattened json to a csv file. The keys of the json will be the header
    of the csv and the values..well, the values ^_^.

    Arguments:
        flattened_json (dict): Flattened JSON object.
            e.g: {
                    'key1': 'n1_value1',
                    'key2': 'n1_value2',
                    'parent1.child_key1': 'n1_child_value1',
                    'parent1.child_key2': 'n1_child_value2'
                }
        csv_path (str): path of the CSV file

    Returns:
        None
    """

    with open(csv_path, 'w') as out_file:
        w = csv.DictWriter(out_file, flattened_json.keys())
        w.writeheader()
        w.writerow(flattened_json)

def main():
    """
    Main entry to our program.
    """

    with open(JSON_PATH) as json_file:
        json_data = json.load(json_file)

    flattened_json = flattenjson(json_data, '.')
    write_json_to_csv(flattened_json, CSV_PATH)

if __name__ == '__main__':
    main()

More about input / output

I don't know where the JSON file comes from so I have to stick with it and process it as is.
I can't change the structure of the JSON file
As far I saw, the JSON data will be at most 7 level-nested so we could have something like:

{
    "a": "1",
    "b": "2",
    "c": {
        "c_1": "3",
        "c_2": "4"
    },
    "d": {
        "d_1": {
            "d_1_1": "5",
            "d_1_2": "6"
        },
        "d_2": {
            "d_2_1": "5",
            "d_2_2": "6"
        }
        ... and so on
    }
}

I have to write the data to the CSV file as described above.
The CSV for the above JSON will look like this:

I'm specifically looking for a review orientated on memory optimizations which probably comes with a cost of a slower running time (that's fine) but any other overall improvements are welcome!
PS: I've done the above in Python 3.8.2 so I'd like you to focus on a version of Python >= 3.6

It looks like the actual processing is quite simple, so I would recommend using a streaming JSON parser like jq --stream or (in Python) ijson.


Comparing screen captures using unsafe / API calls After a week of searching and testing each approach via Stopwatch,
I came to this method using the fastest way possible to capture screen into a bitmap  and then to a byte[].

Is it possible to make it any faster using parallel features or any idea I have not taken into account? (As I am a newbie, 4 months of self learning.)
I mixed two or three versions of the copying function (portion of screen to memory
then convert captured/crop into byte[]). I might have left unnecessary lines of code, I would like to refine it (if and where needed).

unsafe public static Bitmap NatUnsfBtmp(IntPtr hWnd, Size Ms)
{
    Stopwatch swCap2Byte = new Stopwatch();
    swCap2Byte.Start();
    WINDOWINFO winInfo = new WINDOWINFO();
    bool ret = GetWindowInfo(hWnd, ref winInfo);
    if (!ret)
    {
        return null;
    }

    int height = Ms.Height;
    int width = Ms.Width;
    if (height == 0 || width == 0) return null;

    Graphics frmGraphics = Graphics.FromHwnd(hWnd);
    IntPtr hDC = GetWindowDC(hWnd); //gets the entire window
    //IntPtr hDC = frmGraphics.GetHdc(); -- gets the client area, no menu bars, etc..

    System.Drawing.Bitmap tmpBitmap = new System.Drawing.Bitmap(width, height, frmGraphics);
    Bitmap bitmap = (Bitmap)Clipboard.GetDataObject().GetData(DataFormats.Bitmap);
    Graphics bmGraphics = Graphics.FromImage(tmpBitmap);
    IntPtr bmHdc = bmGraphics.GetHdc();
    BitBlt(bmHdc, 0, 0, width, height, hDC, 0, 0, TernaryRasterOperations.SRCCOPY);

    swCap2Byte.Stop();
    string swCopiedFF = swCap2Byte.Elapsed.ToString().Remove(0, 5);
    swCap2Byte.Restart();
    #region <<=========== CopytoMem->ByteArr ============>>

    BitmapData bData = tmpBitmap.LockBits(new Rectangle(new Point(), Ms),
    ImageLockMode.ReadOnly,
    PixelFormat.Format24bppRgb);
    MyForm1.MyT.Cap.TestBigCapturedBtmp = tmpBitmap;
    // number of bytes in the bitmap
    int byteCount = bData.Stride * tmpBitmap.Height;
    byte[] bmpBytes = new byte[byteCount];

    // Copy the locked bytes from memory
    Marshal.Copy(bData.Scan0, bmpBytes, 0, byteCount);
    byte[] OrgArr = bmpBytes;//File.ReadAllBytes("testFcompScr.bmp");
    // don't forget to unlock the bitmap!!
    swCap2Byte.Stop();
    string SwFCFscr = swCap2Byte.Elapsed.ToString().Remove(0, 5);
    System.IO.File.WriteAllBytes(MyForm1.AHItemsInitialDir + "testBig4BenchViaChaos.bar", OrgArr);
    System.Windows.Forms.MessageBox.Show("   Copied @  " +swCopiedFF + Environment.NewLine+"Converted @ " + SwFCFscr);
    btmp.UnlockBits(bData);

    if(System.IO.File.ReadAllBytes(MyForm1.AHItemsInitialDir + "testBig4BenchViaChaos.bar")== OrgArr)
        System.Windows.Forms.MessageBox.Show("OK");
    else System.Windows.Forms.MessageBox.Show("Not same");

    if (BigOrsmall == "Big")
    {

        MyT.Cap.TestBigCapturedBtmp = btmp;
        MyT.CapSave.TestBigCaptSavedAsBar = OrgArr;
        File.WriteAllBytes(AHItemsInitialDir + "testBig4BenchViaChaos.bar", OrgArr);
    }
    else if (BigOrsmall == "Small")
    {
        MyT.Cap.TestSmallCapturedBtmp = btmp;
        File.WriteAllBytes(AHItemsInitialDir + "testSmall4BenchViaChaos.bar", OrgArr);
        MyT.CapSave.TestSmallCaptSavedAsBar = OrgArr;
    }
    TestedCap_DoPutInPicBox(PicBox_CopiedFromScreen);

    #endregion

    bmGraphics.ReleaseHdc(bmHdc);
    ReleaseDC(hWnd, hDC);

    return tmpBitmap;
}

There are a few things I see at a glance:

If you have to enclose a section of code within a method in region
tags, you should be pulling that code out into a helper method.  You
could probably split this method into several.  Don't worry about
overhead of extra method calls -- the compiler will likely re-inline
them anyways.
Split the body of your if/else on "Big" and "Small" into a helper method that takes in a file name.  That's the only difference I see in each block.
There are a lot of lines where you are declaring and assigning variables.  When you do that, it is likely more readable to declare as var, because you indicate what type it is immediately to the right of the assignment operator.
Rather than converting your elapsed times with ToString ().Remove () calls, I would instead suggest storing them as TimeSpan types and using a string format in your message boxes.
Move "Big", "Small", "testBig4BenchViaChaos.bar", and "testSmall4BenchViaChaos.bar" into constants.
If this is just a utility that is not client-facing, move your other strings to constants.  Otherwise, move them into a resx so they can be localized.
Naming - you should use longer, more descriptive names.  You are not saving much by using a name like OrgArr instead of the longer OriginalArray.  You should also be consistent in capitalization.  Some of your local variables are camel case, and some are pascal case.  The standard for the framework is camel case for local variables.
You are overwriting testBig4BenchViaChaos.bar with OrgArr, but then comparing the bytes in testBig4BenchViaChaos.bar with OrgArr.  Am I missing something, or should this always return a match because the data is the same?  Though, on that note, you are doing == with byte arrays, so you should really always mis-match due to object reference comparison.
Your quickest way to compare the two byte arrays is probably going to be using a for loop (single-threaded) or possibly PLINQ using SequenceEqual.  As always, benchmark to be sure because test results are worth a thousand expert opinions.


My code changes a single RGB value by a set amount, then does the same changes over the other channels So I have the following code, that takes an image and modifies the RGB channels one at a time. It then creates a list of 9 images with the various modified channels.
The Red channel is modified by 0.1, then by 0.5, then by 0.9. This is then applied to the Green channel then the Blue Channel.
Resulting in a list of 9 images with the same modifications, but to different channels. The code works, but I just feel like there is a lot of repetition, and this really bugs me.
#%%

from PIL import Image

# Set the file and open it
file = "Final/charlie.png"

#Create an empty list to append the images to
images = []

#Image processing function, depending on the counter
def image_processing0(a):
    if count == 0 or count == 3 or count == 6:
        c = int(a * 0.1)
        return c
    elif count == 1 or count == 4 or count == 7:
        c = int(a * 0.5)
        return c
    elif count == 2 or count == 5 or count == 8:
        c = int(a * 0.9)
        return c

#Set the count to 0
count = 0

#Create PixelMap
while count <9:
#Creat Pic at the start of each loop
    pic = Image.open(file)
#Convert the image to RGB, so we dont have to deal with the alpha channel
    pic = pic.convert('RGB')

#Get the RGB value for every pixel
    for x in range(pic.size[0]):
            for y in range(pic.size[1]):

                #Split the RGB value into variables r, g and b
                r,g,b = pic.getpixel((x,y))

#Check the count, use logic to appy the processing to the correct channel
                if count <3:
                    r = image_processing0(r)
                    pic.putpixel((x,y),(r,g,b))
                elif count < 6:
                    g = image_processing0(g)
                    pic.putpixel((x,y),(r,g,b))
                elif count < 9:
                    b = image_processing0(b)
                    pic.putpixel((x,y),(r,g,b))

#Add the images to the list
    images.append(pic)
#Increment the counter
    count+=1

Both my logic statement, and my function seem a bit repetitive:
 #---------------------------------------------------
    #Image processing function, depending on the counter
    def image_processing0(a):
        if count == 0 or count == 3 or count == 6:
            c = int(a * 0.1)
            return c

        elif count == 1 or count == 4 or count == 7:
            c = int(a * 0.5)
            return c

        elif count == 2 or count == 5 or count == 8:
            c = int(a * 0.9)
            return c
#---------------------------------------------------

#Check the count, use logic to apply the processing to the correct channel
if count <3:
    r = image_processing0(r)
    pic.putpixel((x,y),(r,g,b))

elif count < 6:
    g = image_processing0(g)
    pic.putpixel((x,y),(r,g,b))

elif count < 9:
    b = image_processing0(b)
    pic.putpixel((x,y),(r,g,b))
#---------------------------------------------------

I tried to change my logic to the following, but soon realized that when I put the pixel back, i had no way to specify exactly what channel I was placing back in the  pic.putpixel((x,y),(r,g,b)) statement.
if count <3:
    channel = r
elif count < 6:
    channel = g
elif count < 9:
    channel = b

channel = image_processing0(channel)
pic.putpixel((x,y),(r,g,b))    #No way to define which channel is being changed

Some style advice and much simpler/shorter/faster solutions.
Style
Your comments break the structure, making the code harder to read. As PEP 8 says:

Block comments [...] are indented to the same level as that code. Each line of a block comment starts with a # and a single space

So
while count <9:
#Creat Pic at the start of each loop
    pic = Image.open(file)
#Convert the image to RGB, so we dont have to deal with the alpha channel
    pic = pic.convert('RGB')

becomes
while count <9:
    # Create pic at the start of each loop
    pic = Image.open(file)
    # Convert the image to RGB, so we dont have to deal with the alpha channel
    pic = pic.convert('RGB')

and I might remove the first comment, as it, unlike the second comment, doesn't contain any insight that's not easily seen in the code anyway.
NumPy
Taking tsh's solution further, to NumPy:
from PIL import Image
import numpy as np

file = "Final/charlie.png"

im = Image.open(file).convert('RGB')
images = []
for channel in range(3):
    for factor in 0.1, 0.5, 0.9:
        data = np.copy(im)
        ch = data[:, :, channel]
        np.multiply(ch, factor, ch, casting='unsafe')
        images.append(Image.fromarray(data))

Image.convert with matrix
Or using convert's matrix parameter:
from PIL import Image

file = "Final/charlie.png"

im = Image.open(file)
images = []
for channel in range(3):
    for factor in 0.1, 0.5, 0.9:
        matrix = [1, 0, 0, 0,
                  0, 1, 0, 0,
                  0, 0, 1, 0]
        matrix[channel * 5] = factor
        images.append(im.convert('RGB', matrix))

Speeds
Using some 575 x 575 sample image, I got roughly these times per new image:

Yours: 2.9 seconds
tsh's: 0.8 seconds
My NumPy one: 0.003 seconds
My convert one: 0.005 seconds


MySQL queries with try-except error handling I have this function that I have written, that gets passed a row of data from a mysql function. The function then uses a try-except block to get some a list of values depending on a value within the row of data.

It then loops through this data and completes another try-except block within the first try-except block's else command.

In fact, a lot of my code is styled this way. Some blocks go on for so long, or other blocks start once one has finished. Is this good practice of writing code? Or should I refactor my code and move the second try-except block into another separate function?

def select_uploaded_series(row):
        try:
            select_series_name = """SELECT track_ser_series
            FROM track_series
            WHERE track_ser_id = %s"""
            dbDets[1].execute(select_series_name,(row['track_id']),)
            series_names = dbDets[1].fetchall()
        except Exception as e:
            raise(e)
        else:

            names = []

            for series in series_names:
                try:
                    select_series_name_query = """SELECT title
                    FROM series
                    WHERE categoryId = %s"""
                    dbDets[1].execute(
                            select_series_name_query,
                            (series['track_ser_series']),)
                    track_series_names = dbDets[1].fetchall()
                except Exception as e:
                    raise(e)
                else:

                    for t_names in track_series_names:
                        names.append(t_names['title'])

            return(names)

If you're just going to re-raise the exception you caught, then there is no point in catching it in the first place. Instead of:

try:
    code block A
except Exception:
    raise
else:
    code block B

you can write:

code block A
code block B

and the behaviour is just the same.


Lychrel calculator (Project Euler 55) If we take 47, reverse and add, 47 + 74 = 121, which is palindromic.

  Not all numbers produce palindromes so quickly. For example,

349 + 943 = 1292,
1292 + 2921 = 4213
4213 + 3124 = 7337

  That is, 349 took three iterations to arrive at a palindrome.

  Although no one has proved it yet, it is thought that some numbers,
  like 196, never produce a palindrome. A number that never forms a
  palindrome through the reverse and add process is called a Lychrel
  number. Due to the theoretical nature of these numbers, and for the
  purpose of this problem, we shall assume that a number is Lychrel
  until proven otherwise. In addition you are given that for every
  number below ten-thousand, it will either (i) become a palindrome in
  less than fifty iterations, or, (ii) no one, with all the computing
  power that exists, has managed so far to map it to a palindrome. In
  fact, 10677 is the first number to be shown to require over fifty
  iterations before producing a palindrome: 4668731596684224866951378664
  (53 iterations, 28-digits).

  Surprisingly, there are palindromic numbers that are themselves
  Lychrel numbers; the first example is 4994.

  How many Lychrel numbers are there below ten-thousand?

This is my solution:

from datetime import datetime

candidates = []

def reverse(number):
    ''' Returns the reverse order of an integer,
        for example: reverse(123) returns the integer 321 '''
    number = str(number)
    return int(number[::-1])

def is_lychrel(number):
    ''' Returns True if 'number' is a lychrel candidate,
        but works only with numbers from 1 to 10000 '''
    iterated = number + reverse(number)
    for i in range(25):
        rev_iterated = reverse(iterated)
        if iterated != rev_iterated:
            iterated = iterated + rev_iterated
        else:
            return False
    return True

if __name__ == '__main__':

    start_time = datetime.now()

    for number in range(10001):
        if is_lychrel(number):
            candidates.append(number)

    time = datetime.now() - start_time # To get the time of the calculation

    print('In {} were found {} lychrel candidates'.format(time, len(candidates)))
    print(' '.join(map(str, candidates)), 'are candidates.')
    suml = input('Press ENTER to close ')

On my PC (with Intel Atom 2GHz), I got it with .4 seconds.  Could you offer suggestions on making it run faster?

1. Comments on your code

Python has a built-in module, timeit, for measuring the execution time of small code snippets. So there is no need for all that fussing about with datetime.
Project Euler problem 55 says:

  you are given that for every number below ten thousand, it will either (i) become a palindrome in less than fifty iterations

but your code only tries 25 iterations. It happens to be the case that 25 iterations are enough in this case, but how did you figure that out? If you had a mathematical proof that showed that 25 iterations were enough, then that would be fair enough, but I suspect that in fact you re-ran the program with different values for the count of iterations until you found that smallest one that produces the right answer. This seems like cheating to me: if you are willing to do that, why not just replace the whole program with:

print(answer)

which would be even faster?
The problem statement asks:

  How many Lychrel numbers are there below ten-thousand?

but you test range(10001) which is off by one. Luckily for you, 10,000 is not a Lychrel number, so this doesn't lead to an error.
You've made candidates into a global variable, and run some of your code at top level. This makes it hard to test your program from the interactive interpreter. You need something like this:

def problem55(n):
    """Return a list of Lychrel candidates below n."""
    candidates = []
    for number in range(n):
        if is_lychrel(number):
            candidates.append(number)
    return candidates

and then you can test it like this:

>>> from timeit import timeit
>>> timeit(lambda:problem55(10000), number=100)
8.279778157011606

2. Speeding it up

The problem statement only asks how many Lychrel numbers there are, so there is no need to construct a list of them.
Function calls in Python are moderately expensive, so you can save some time by inlining them. This leads to an implementation like this:

def problem55_1(n, m):
    """Return the count of numbers below n that do not lead to a
    palindrome after m iterations of reversal and addition.

    """
    count = 0
    for i in range(n):
        r = int(str(i)[::-1])
        for _ in range(m):
            i += r
            r = int(str(i)[::-1])
            if i == r:
                break
        else:
            count += 1
    return count

which is about 16% faster than yours:

>>> timeit(lambda:problem55_1(10000, 50), number=100)
6.93482669594232

Janne suggested that you could speed things up by caching the numbers already found to be Lychrel and non-Lychrel, and you replied:

  doing that is going to require more conditions and probably is going to be the same running time or even worse

which is a bit of a defeatist attitude. Certainly the code gets more complex when you add caching, but it's not a priori obvious whether this costs more than it saves, or saves more than it costs. The best way to find out is to knuckle down and try it:

def problem55_2(n, m):
    """Return the count of numbers below n that do not lead to a
    palindrome after m iterations of reversal and addition.

    """
    lychrel = set()
    nonlychrel = set()
    count = 0
    for i in range(n):
        j = i
        if i in lychrel:
            count += 1
            continue
        stack = [i]
        r = int(str(i)[::-1])
        for _ in range(m):
            i += r
            if i in lychrel:
                count += 1
                lychrel.update(stack)
                break
            if i in nonlychrel:
                nonlychrel.update(stack)
                break
            stack.append(i)
            r = int(str(i)[::-1])
            if i == r:
                nonlychrel.update(stack)
                break
        else:
            count += 1
            lychrel.update(stack)
    return count

I find that this is about twice as fast as your code:

>>> timeit(lambda:problem55_2(10000, 50), number=100)
3.935654202941805

One final tweak from me. Now that we are caching, the order in which we check numbers for the Lychrel property matters. As you iterate the reverse-and-add process, the numbers get bigger. So if we start with the larger numbers and proceed downwards, then we are more likely to get cache hits. So I tried replacing the line:

    for i in range(n):

with

    for i in range(n - 1, -1, -1):

and got a further small speedup:

>>> timeit(lambda:problem55_3(10000, 50), number=100)
3.800810826011002


Finding common elements, ignoring duplicates, in two given string arrays I am going through the CodingBat exercises for Java. Here is the one I have just completed:

  Start with two arrays of strings, a and b, each in alphabetical order, possibly with duplicates. Return the count of the number of strings which appear in both arrays. The best "linear" solution makes a single pass over both arrays, taking advantage of the fact that they are in alphabetical order.

Here is my code:

public int commonTwo(String[] a, String[] b){

    String[] shortArray;
    String[] longArray;

    if (a.length != b.length) {
        shortArray = a.length < b. length ? a : b;
        longArray = a.length > b. length ? a : b;
    } else {
        shortArray = a;
        longArray = b;
    }

    int count = 0;
    String letterToAvoid = "";

    for (int i = 0; i < shortArray.length; i++) {
        for (int j = 0; j < longArray.length; j++) {
            if (shortArray[i] == longArray[j] && longArray[j] != letterToAvoid){
                count++;
                letterToAvoid = longArray[j];
            }
        }
    }
    return count;
}

My questions are:

Is there a better - or more efficient - way to find the longest and shortest array lengths (and subsequently assign them separately if the lengths are equal)?
Is my letterToAvoid idea, used to 'skip' certain array elements, good for this kind of exercise?
How could I make this code more efficient?

There is one major flaw in your code: Testing strings for value
equality must be done with .equals() not with ==.
For example, for

commonTwo(new String[]{new String("a"),"b","c"},
          new String[]{"a","b","c"})

your code returns 2 instead of 3.
(Compare How do I compare strings in Java?.)

Your computation of the larger/shorter array can be simplified to

shortArray = a.length < b.length ? a : b;
longArray = a.length >= b.length ? a : b;

But actually your code does not take advantage of the fact that one array is shorter than the other, so you might as well loop over a
and b directly.

Your letterToAvoid idea works (almost) to skip duplicate elements, but I would call the variable differently, perhaps lastDuplicate.
And you could early-return from the inner loop if a duplicate is found.
Your idea does not work if the string arrays contain empty strings
as well which should also be counted.

Your solution is not efficient. It does not take advantage of the
fact that both arrays are sorted. Instead of a nested loop you
can perform a parallel single iteration over both arrays:

If current left element is smaller than current right element,
advance left position by one.
If current right element is smaller, advance right position.
If current left and right elements are equal, we have found
a duplicate. Skip following equal elements in both arrays.

Possible implementation:

public int commonTwo(String[] a, String[] b){

    int count = 0;

    int i = 0;  // Index into a
    int j = 0;  // Index into b

    while (i < a.length && j < b.length) {
        int cmp = a[i].compareTo(b[j]);
        if (cmp < 0) {
            // a[i] is smaller
            i++;
        } else if (cmp > 0) {
            // b[j] is smaller
            j++;
        } else {
            // a[i] == b[j]
            // Increment count and skip duplicates
            count++;
            String lastDuplicate = a[i];
            i++;
            j++;
            while (i < a.length && a[i].equals(lastDuplicate)) { i++; }
            while (j < b.length && b[j].equals(lastDuplicate)) { j++; }
        }
    }

    return count;
}


Converting string of hex values to numpy array of specific shape I am working on a project that needs to take a .bdf font file and turn it into a Numpy array. I am using the code below to turn the bitmap for the font into a numpy array. It currently works, I just feel that it's nearly impossible to read but I'm not sure if there's a better way to do it.

How the current code works:

It splits the string by line since each line of the string is a row.
It selects the first two values from the row since the last two never seem to matter.
Using int() I convert the string of hex to an integer.
Using bin() I turn the integer to binary.
Then I use zfill() because leading zeros in the binary are important. This returns a string of 1's and 0's
Then I use list() to break the string of binary into a list of single 1's and 0's
Then I convert that all to a numpy array with dtype=int

The process feels really messy and it takes a millisecond which I feel like is pretty long for a (15, 9) numpy array. Any ideas for improvements would be greatly appreciated.

def hexToNumpy(hexValues, columns):

    return np.array(
        [
            list(bin(int(row[0:2], 16))[2:].zfill(columns))
            for row in hexValues.split("\n")
        ],
        dtype=int,
    )

T = """0000
0000
7F00
0800
0800
0800
0800
0800
0800
0800
0800
0800
0000
0000
0000"""

hexToNumpy(T, 9)

You can simplify bin(...)[2:].zfill(columns) to f'{...:0>{columns}b}'.

This can use f'{int(row[:2], 16):0>{columns}b}'.
I'd recommend you convert to a NumPy array out of the function, as it's not really that important to be in there.
Your code isn't idiomatic, as Python uses snake_case not camelCase.

def from_hex(values, columns):
    return [
        list(f'{int(row[:2], 16):0>{columns}b}')
        for row in values.split("\n")
    ]


Should I be using a stored procedure for this function in my code behind? We typically do our saves and deletes for our asp.net pages (SQL Server as back-end) in the code behind. I've seen some reference the idea of creating a stored procedure for this purpose. Is this the best approach or is what I'm doing practical enough? Any suggestions? Here's an example of the delete button function (first half is for triggers set up in SQL):

protected void DeleteClick(object sender, EventArgs e)
{
    cookie = Request.Cookies["pAuthCookie"];
    if (txtKey.Text == "")
    {
        string scriptstring = "alert('User Error - Must Select Fromn the LIST first.');";
        ScriptManager.RegisterStartupScript(Tab1, typeof(string), "alertscript", scriptstring, true);
        if (TabContainer1.ActiveTabIndex == 1)
            TabContainer1.ActiveTabIndex = 0;
        return;
    }
    else
    {
        String fprescript = txtKey.Text;
        string empno = cookie["thenum"].ToString();

        string sqlstring1 = "UPDATE prescriptions set empfk = @empfk WHERE prescriptpk = @prescriptpk";
        con.Open();
        SqlCommand cmd1 = new SqlCommand(sqlstring1, con);
        cmd1.Parameters.Add("@prescriptpk", SqlDbType.Int);
        cmd1.Parameters.Add("@empfk", SqlDbType.Int);
        cmd1.Parameters["@prescriptpk"].Value = Convert.ToInt32(fprescript);
        cmd1.Parameters["@empfk"].Value = empno.Trim();
        cmd1.ExecuteNonQuery();

        con.Close();
        string sqlstring = "DELETE FROM prescriptions WHERE prescriptpk = @prescriptpk";
        con.Open();
        SqlCommand cmd = new SqlCommand(sqlstring, con);
        cmd.Parameters.Add("@prescriptpk", SqlDbType.Int);
        cmd.Parameters["@prescriptpk"].Value = Convert.ToInt32(fprescript);
        cmd.ExecuteNonQuery();

        con.Close();
        clearoutdata();
        TabContainer1.ActiveTabIndex = 0;
        GridView1.DataBind();
    }
}

This is overall comments on above code

1) Encapsulate what is repeating

In your method you are opening sql connection, adding parameter  and executing it one by one. Rather doing this please extract out it into one method.

 private void ExecuteNonQuery(string connectionstring, string query, IEnumerable<SqlParameter> parameters =null)
  {
            using (var connection = new SqlConnection(connectionstring))
            {
                connection.Open();
                using (var command = new SqlCommand(query, connection))
                {
                    if (parameters != null)
                    {
                        foreach (var parameter in parameters)
                        {
                            command.Parameters.Add(parameter);
                        }
                    }
                    command.ExecuteNonQuery();
                }
            }
   }

2) Always dispose the connection

In above code your opening connection and closing it, but this can lead into connection leak in case of errors. Please wrap your code into using statement or try clause like this

using (var connection = new SqlConnection())
{

}

3) Distribute your responsibility into layer

The above code is doing too much

script registering
Sql Connection stuff
Data Populating and clearing

Please separate out your code into at least two classes
1) Fetching data class
2) UI binding stuff class

4)inline SQL vs Stored Procedure

Stored Procedure is good as your code lies at one place in db but creating Stored Procedure for one line of SQL is not a good idea. If your logic is going to be complex and big ,Use a Stored procedure other wise inline sql.

PS: I have created a  gist named SQLHelper class for you to abstract out the sql connection and command things,

https://gist.github.com/paritoshmmmec/5c3855af7215ac87d787


Filter template meta function I wrote a simple C++ template meta function for filtering out types from a list not matching a predicate. This is similar to Filtering variadic template arguments but does pattern matching instead of std::conditional with the aim of being faster. To that end no empty typelists are concatenated which should incur less instantiations and be faster according to https://blog.galowicz.de/2016/06/25/cpp_template_type_list_performance

The below code works for all my test cases. Any hints how to make it faster (less template instantiations), smaller, better, ...? (Note: Assume appropriate *_t using-templates)

template <typename... Ts>
struct typelist;

template <typename... TLists>
struct concat;
template <typename... Args1, typename... Args2>
struct concat<typelist<Args1...>, typelist<Args2...>>
{
    using type = typelist<Args1..., Args2...>;
};
template <class Seq1, class Seq2, class... Seqs>
struct concat<Seq1, Seq2, Seqs...> : concat<Seq1, concat_t<Seq2, Seqs...>>
{
};

template <class TList, template <typename> class Cond>
struct filter;
template<bool matches, template <typename> class Cond, typename...>
struct filter_helper;

template<template <typename> class Cond, typename T, typename... Ts>
struct filter_helper<true, Cond, T, Ts...>
{
    using type = concat_t<typelist<T>, filter_t<typelist<Ts...>, Cond>>;
};

template<template <typename> class Cond, typename T, typename... Ts>
struct filter_helper<false, Cond, T, Ts...>
{
    using type = filter_t<typelist<Ts...>, Cond>;
};

template<template <typename> class Cond, typename T, typename... Ts>
struct filter<typelist<T, Ts...>, Cond>
{
    using type = typename filter_helper<Cond<T>::value, Cond, T, Ts...>::type;
};

template<template <typename> class Cond>
struct filter<typelist<>, Cond>
{
    using type = typelist<>;
};

Some tests (TMP_ASSERT is like static_assert):

using list1 = tmp::typelist<int, double>;
using list2 = tmp::typelist<float>;
using list3 = tmp::typelist<double>;
using list4 = tmp::typelist<float, double, double>;
using list5 = tmp::typelist<double, float, double>;
using list6 = tmp::typelist<double, double, float>;
using empty = tmp::typelist<>;

template<typename T>
struct is_not_double: std::true_type{};
template<>
struct is_not_double<double>: std::false_type{};

TMP_ASSERT_SAME((tmp::filter_t<list1, is_not_double>), (tmp::typelist<int>));
TMP_ASSERT_SAME((tmp::filter_t<list2, is_not_double>), (tmp::typelist<float>));
TMP_ASSERT_SAME((tmp::filter_t<list3, is_not_double>), (tmp::typelist<>));
TMP_ASSERT_SAME((tmp::filter_t<list4, is_not_double>), (tmp::typelist<float>));
TMP_ASSERT_SAME((tmp::filter_t<list5, is_not_double>), (tmp::typelist<float>));
TMP_ASSERT_SAME((tmp::filter_t<list6, is_not_double>), (tmp::typelist<float>));
TMP_ASSERT_SAME((tmp::filter_t<empty, is_not_double>), (tmp::typelist<>));

Note: Code is from a library licensed under BSD 3-Clause.

I find all those structs tricks a bit old-school now...

There is an alternative implementation using constexpr functions; I also recommend to use if constexpr to make things more readable, but it's doable without it; you just need another overload:

template <typename...>
struct Type_list{};

template <typename... T1s, typename... T2s>
constexpr auto concatenate(Type_list<T1s...>, Type_list<T2s...>) {
    return Type_list<T1s..., T2s...>{};
}

template <template <typename> typename Condition, typename Result>
constexpr auto filter_types(Result result, Type_list<>) {
    return result;
}

template <template <typename> typename Condition, typename Result, typename T, typename... Ts>
constexpr auto filter_types(Result result, Type_list<T, Ts...>) {
    if constexpr (Condition<T>{})
        return filter_types<Condition>(concatenate(result, Type_list<T>{}), Type_list<Ts...>{});
    else
        return filter_types<Condition>(result, Type_list<Ts...>{});
}

template <template <typename> typename Condition, typename... Types>
using filtered_types = std::decay_t<decltype(filter_types<Condition>(Type_list<>{}, Type_list<Types...>{}))>;

template<typename T>
struct is_not_double: std::true_type{};
template<>
struct is_not_double<double>: std::false_type{};

template <typename T>
void print_type() {
    puts(__PRETTY_FUNCTION__);
    }

int main() {
    print_type<filtered_types<is_not_double, double, int, char, float*, double, char*, double>>();
}

I'd rather have this sort of programming: template programming looks like normal programming, rather than have two subsets of the language apart from each other.

Edit: as per request, here's a rewriting of the code without C++ 17 features:

template <typename...>
struct Type_list{};

template <typename... T1s, typename... T2s>
constexpr auto concatenate(Type_list<T1s...>, Type_list<T2s...>) {
    return Type_list<T1s..., T2s...>{};
}

template <template <typename> typename Condition, typename Result, typename T, typename... Ts>
constexpr auto filter_types(Result result, Type_list<T, Ts...>, std::true_type)  {
    return filter_types<Condition>(concatenate(result, Type_list<T>{}), Type_list<Ts...>{});
}

template <template <typename> typename Condition, typename Result, typename T, typename... Ts>
constexpr auto filter_types(Result result, Type_list<T, Ts...>, std::false_type) {
    return filter_types<Condition>(result, Type_list<Ts...>{});
}

template <template <typename> typename Condition, typename Result>
constexpr auto filter_types(Result result, Type_list<>) {
    return result;
}

template <template <typename> typename Condition, typename T, typename... Ts, typename... Us>
constexpr auto filter_types(Type_list<Us...> result, Type_list<T, Ts...> lst) {
    return filter_types<Condition>(result, lst, Condition<T>{});
}

template <template <typename> typename Condition, typename... Types>
using type_filter = decltype(filter_types<Condition>(Type_list<>{}, Type_list<Types...>{}));

As for a variadic, container agnostic, concatenate, here's what you would do with fold expressions:

template <template <typename> typename Type_container, typename... T1s, typename... T2s>
constexpr auto operator+(Type_container<T1s...>, Type_container<T2s...>) {
    return Type_container<T1s..., T2s...>{};
}

template <typename... Type_lists>
constexpr auto concatenate(Type_lists... type_lists) {
    return (type_lists + ...);
}

Without fold expressions you need to provide overloads for calls with 0, 1 and more type lists, as you did with your structs.


Print all possible codes for a given string My question is to print all possible codes for the string.

Assume that value of a=1, b=2, c=3, d=4, .... z=26.

E.g. for “1123” possible codes are aabc, kbc, alc, aaw, kw

Here's what I could come up with.

   private static void printCodes(String str, String ans) {
    if (str.length() == 0) {
        System.out.println(ans);
        return;
    }

    String ch1 = str.substring(0, 1);
    String restOfTheString1 = str.substring(1);

    if (Integer.parseInt(ch1, 10) > 0) {
        char code = (char) (Integer.parseInt(ch1, 10) + 'a' - 1);
        printCodes(restOfTheString1, ans + code);
    }

    if (str.length() >= 2) {
        String ch12 = str.substring(0, 2);
        String ros2 = str.substring(2);

        if (Integer.parseInt(ch12, 10) > 0 && Integer.parseInt(ch12) <= 26) {
            char code = (char) (Integer.parseInt(ch12, 10) + 'a' - 1);
            printCodes(ros2, ans + code);
        }
        if (Integer.parseInt(ch12, 10) == 0) {
            printCodes(ros2, ans);
        }
    }
}

Let me know if I did something wrong or anything could have been better.
Thanks!

Your code is a reasonably neat use of recursion to solve the problem, and recursion is a good solution to this problem. I prefer it for people to make it obvious that a function is used recursively to prepare myself for what to expect, I tend to use "recursive" in the name. You don't show how you set up the recursive method, though, and in general it's poor form to embed print statements inside functions that do other things too (single responsibility principle: https://en.wikipedia.org/wiki/Single_responsibility_principle).

A better solution would be to return a collection of results that you can print in a separate place.

What really bothers me, though, is two things:

the hard-coded assumption that all codes are either 1 or 2 characters/digits. I realize that in your input problem it's true, but I feel there's a better way, or a more general way to solve this problem
the conversion to integers, and integer parsing of strings

Personally I am not too concerned about the string-concatenation you use for computing the ans value when recursing, but there may be a better way with using a pre-computed array, with tracking indices. I would consider the array/index solution a "phase 2" thing if performance is an actual, proven problem.

So, having said the above, it's best to explain it with code... I would do two significantly different things... I would set up a lookup table of string-based "digit" keys, and the corresponding character value. I would then loop through that table in the recursive function to determine the matches....

Setting up the table is relatively complicated, but it's a 1-time thing consisting of 2 correlated string arrays:

private static final String[] keys;
private static final String[] values;

static {
    String alphabet = "abcdefghijklmnopqrstuvwxyz";
    keys = new String[alphabet.length()];
    values = new String[alphabet.length()];
    for (int i = 0; i < alphabet.length(); i++) {
        keys[i] = String.valueOf(i + 1);
        values[i] = alphabet.substring(i, i + 1);
    }
}

Now, with those two arrays, we can decode the values "easily":

public static List<String> decode(String input) {
    List<String> results = new ArrayList<>();
    decodeRecursive(input, "", results);
    return results;
}

private static void decodeRecursive(String input, String current, List<String>results) {
    if (input.length() == 0) {
        results.add(current);
        return;
    }

    for (int i = 0; i < keys.length; i++) {
        String key = keys[i];
        if (input.startsWith(key)) {
            decodeRecursive(input.substring(key.length()), current + values[i], results);
        }
    }
}

Note that all the work is basically in the "string" space, and not parsing integers, etc. Also, the presentation/printing is done somewhere else.

I put this together in a program on ideone so you can see it working: https://ideone.com/sozgXb


Using Python to rename multiple csv files in Windows I need to rename a large list of csv files that are generated from 3 different servers. The files are produced with a date stamp as the extension, which I need to move in each file name to retain the date stamp.
The file name format before editing is as:

billing.pps-svr01.csv.2015-09-01
billing.pps-svr02.csv.2015-09-01
billing.pps-svr03.csv.2015-09-01

The file name format after editing should be:

billing.pps-svr01.2015-09-01.csv
billing.pps-svr02.2015-09-01.csv
billing.pps-svr03.2015-09-01.csv

My question is in regard to code efficiency and best practice. The following code seems to work in testing, however I'm very new to Python and programming in general and I'd like to know if there are any other ways to solve this problem that are more efficient...for example, could this be accomplished in a one-liner or something else? I should also note that I intend to incorporate this into a much larger script that parses these files after they've been renamed. Any feedback and/or suggestions would be great.
import os

os.chdir(r'C:\Users\Extract')

for filename in os.listdir('.'):
    if filename.startswith('billing.pps-svr'):
        name = filename.split('.')
        name = name+[name.pop(0-2)]
        new_name = '.'.join(name)
        os.rename(filename, new_name)

Here are my thoughts:

You don't do any error handling if any of the file operation fails. Not to be recommended
Your code will flip the latter two parts, all the time. It does not care if the filename has already been fixed or not!
The code name + [name.pop(0-2)] troubles me. You are concatenating the name list with the popped value, but in order for this work you need for the popping to happen before the first part to be joined. Scary stuff...
Here are some pythonic ways to do stuff with lists:

name[:-2] – Get everything but the two last elements
name[-2:] – Get only the last two elements
name[::-1] – Reverse the element list

Here is some coding displaying the flaw in your original rename code, and two options for how to handle it correctly.

for filename in (
                 'billing.pps-svr01.2014-09-01.csv',
                 'billing.pps-svr02.2014-09-01.csv',
                 'billing.pps-svr01.csv.2015-09-01',
                 'billing.pps-svr02.csv.2015-09-01',
                ):

    print('\nTesting {}:'.format(filename))

    name = filename.split('.')
    name = name + [name.pop(0-2)]
    new_name = '.'.join(name)

    print '    old rename: {} to {}'.format(filename, new_name)

    filename_parts = filename.split('.')
    first_part = filename_parts[:-2]
    last_part = filename_parts[-2:]

    if last_part[-1] != 'csv':
        new_name = '.'.join(first_part + last_part[::-1])
        print '    new rename: {} to {}'.format(filename, new_name)
    else:
        print '    no rename needed'

    if filename_parts[-2] == 'csv':
        new_name = '.'.join(filename_parts[:-2] + filename_parts[-2:][::-1])
        print '    alt rename: {} to {}'.format(filename, new_name)
    else:
        print '    no alternate rename needed'

The output from this are as follows:

Testing billing.pps-svr01.2014-09-01.csv:
    old rename: billing.pps-svr01.2014-09-01.csv to billing.pps-svr01.csv.2014-09-01
    no rename needed
    no alternate rename needed

Testing billing.pps-svr02.2014-09-01.csv:
    old rename: billing.pps-svr02.2014-09-01.csv to billing.pps-svr02.csv.2014-09-01
    no rename needed
    no alternate rename needed

Testing billing.pps-svr01.csv.2015-09-01:
    old rename: billing.pps-svr01.csv.2015-09-01 to billing.pps-svr01.2015-09-01.csv
    new rename: billing.pps-svr01.csv.2015-09-01 to billing.pps-svr01.2015-09-01.csv
    alt rename: billing.pps-svr01.csv.2015-09-01 to billing.pps-svr01.2015-09-01.csv

Testing billing.pps-svr02.csv.2015-09-01:
    old rename: billing.pps-svr02.csv.2015-09-01 to billing.pps-svr02.2015-09-01.csv
    new rename: billing.pps-svr02.csv.2015-09-01 to billing.pps-svr02.2015-09-01.csv
    alt rename: billing.pps-svr02.csv.2015-09-01 to billing.pps-svr02.2015-09-01.csv

Notice how the two first files would have gotten a wrongly rename use your original code.

Code refactor (added)

To accomodate for your question regarding building this into a larger script, and to give example of error handling, I've refactor your code into the following (using the tip from Janne Karila on using rsplit):

import os

def rename_csv_files(directory, required_start):
    """Rename files in <directory> starting with <required_start> to csv files

    Go to <directory> and read through all files, and for those
    starting with <required_start> and ending with something like
    *.csv.YYYY-MM-DD and rename these to *.YYYY-MM-DD.
    """
    try:
       os.chdir(directory)
    except OSError, exception:
       print('IOError when changing directory - {}'.format(exception))
       return

    try:
        for filename in os.listdir('.'):
            if filename.startswith(required_start):

                base, ext, date = filename.rsplit('.', 2)
                new_filename = '.'.join((base, date, ext))
                if ext == 'csv' and not os.path.exists(new_filename):
                    try:
                        os.rename(filename, new_filename)
                        print 'Renamed: {}'.format(new_filename)

                    except OSError, exception:
                        print('Failed renaming file - dir: {}, original file:  {}, new file: {} - {}'.format(
                              directory, filename, new_filename, exception))

                elif ext != 'csv':
                    print('Skipped: {}'.format(filename))

                else:
                    print('Skipped: {} - Renamed version already exists'.format(filename))

    except OSError, exception:
        print('Failed traversing directory - dir: {} - {}'.format(directory, exception))

def main():
    rename_csv_files('./test_data', 'billing.pps-svr')

if __name__ == '__main__':
    main()

Running this script against the following test-data:

$ ls -1d test_data/* | sort -n
test_data/billing.pps-svr01.2014-09-01.csv
test_data/billing.pps-svr01.csv.2015-09-01
test_data/billing.pps-svr02.2014-09-01.csv
test_data/billing.pps-svr02.2015-09-01.csv
test_data/billing.pps-svr02.csv.2015-09-01
test_data/original_files.tar

Gives the following output:

Skipped: billing.pps-svr01.2014-09-01.csv
Renamed: billing.pps-svr01.2015-09-01.csv
Skipped: billing.pps-svr02.2014-09-01.csv
Skipped: billing.pps-svr02.2015-09-01.csv
Skipped: billing.pps-svr02.csv.2015-09-01 - Renamed version already exists

This code now handles error handling for at least the following cases:

Directory not existing, or read or execution permission faults
Errors when traversing directory or renaming files
The logical error of renaming a file into an already existing file


Hacking Ruby Hash: Fastest #to_struct method I am trying to make the fastest #to_struct method in Ruby's Hash.

I am including a use case and benchmark so you can run and see if you have really improved the code.

This is my implementation and the benchmark is included. The time at the bottom is the time it takes on my machine. How can I make this faster?

require "json"
require 'benchmark'
require 'bigdecimal/math'

class Hash
  def to_struct
    k = self.keys
    klass = k.map(&:to_s).sort_by {|word| word.downcase}.join.capitalize
    begin
      Kernel.const_get("Struct::" + klass).new(*self.values_at(*k))
    rescue NameError
      Struct.new(klass, *(k)).new(*self.values_at(*k))
    end
  end
end

# You have a hash that you have built in your app
sample_hash = {
  foo_key: "foo_val",
  bar_key: "bar_val",
  baz_key: "baz_val",
  foo1_key: "foo_val",
  bar1_key: "bar_val",
  baz1_key: "baz_val",
  foo2_key: "foo_val",
  bar2_key: "bar_val",
  baz2_key: "baz_val",
  foo3_key: "foo_val",
  bar3_key: "bar_val",
  baz3_key: "baz_val",
  foo4_key: "foo_val",
  bar4_key: "bar_val",
  baz4_key: "baz_val",
  foo5_key: "foo_val",
  bar5_key: "bar_val",
  baz5_key: "baz_val",
  foo6_key: "foo_val",
  bar6_key: "bar_val",
  baz6_key: "baz_val",
  foo7_key: "foo_val",
  bar7_key: "bar_val",
  baz7_key: "baz_val",
}

# Then you have JSON coming from some external api
json_response = "{\"qux_key\":\"qux_val\",\"quux_key\":\"quux_val\",\"corge_key\":\"corge_val\"}"
hash_with_unknown_keys = JSON.parse(json_response)

# Merge these two together
sample_hash.merge!(hash_with_unknown_keys)

iterations = 100_000

Benchmark.bm do |bm|
  bm.report "#to_struct" do
    iterations.times do
      # Would be super nice if I could convert this to a struct with a method
      # Somehow a bit faster than the explicit example below and much faster than open struct
      sample_struct = sample_hash.to_struct
      unless sample_struct.foo_key == "foo_val"
        raise "Wrong value"
      end
    end
  end

  bm.report "Struct" do
    iterations.times do
      sample_struct = Struct.new(*sample_hash.keys)
        .new(*sample_hash.values)
      unless sample_struct.foo_key == "foo_val"
        raise "Wrong value"
      end
    end
  end

  bm.report "OpenStruct" do
    iterations.times do
      sample_open_struct = OpenStruct.new(sample_hash)
      unless sample_open_struct.foo_key  == "foo_val"
        raise "Wrong value"
      end
    end
  end
end

#       user     system      total        real
# #to_struct  4.030000   0.010000   4.040000 (  4.072031)
#     Struct  6.870000   0.290000   7.160000 (  7.320459)
# OpenStruct 23.550000   0.210000  23.760000 ( 23.895187)

Use OpenHash and Ruby >= 2.3.0

Starting with MRI 2.3.0, your benchmark using OpenHash gets fast.  Very fast:

ruby-2.2.5: ruby 2.2.5p319 (2016-04-26 revision 54774) [x86_64-linux]
       user     system      total        real
#to_struct  1.780000   0.000000   1.780000 (  1.774490)
Struct  9.100000   0.000000   9.100000 (  9.099619)
OpenStruct  7.910000   0.000000   7.910000 (  7.911342)

ruby-2.3.0: ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux]
       user     system      total        real
#to_struct  1.700000   0.000000   1.700000 (  1.695587)
Struct  7.660000   0.000000   7.660000 (  7.660869)
OpenStruct  0.650000   0.000000   0.650000 (  0.658817)

With the latest MRI, Your #to_struct method gets a bit of a speed boost as well.

ruby-2.4.1: ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
       user     system      total        real
#to_struct  1.460000   0.000000   1.460000 (  1.459063)
Struct  7.420000   0.000000   7.420000 (  7.416505)
OpenStruct  0.660000   0.000000   0.660000 (  0.658009)

So if you can, use Ruby >= ruby 2.3.0, and use OpenHash.

How to make #to_struct faster

I made the following changes for performance:

Eliminate the mapping of hash keys using #downcase.
Use #values instead of #values_at (values are always the same order as keys).  See https://stackoverflow.com/a/31425274/238886

and these for clarity:

Eliminate the temporary for self.keys
DRY the creation of the struct instance
Removed self references.

With these changes, the code is:

class Hash
  def new_to_struct
    klass_name = keys.map(&:to_s).sort.join.capitalize
    klass = begin
              Kernel.const_get("Struct::" + klass_name)
            rescue NameError
              Struct.new(klass_name, *keys)
            end
    klass.new(*values)
  end
end

and the benchmark (run against ruby-2.4.1):

       user     system      total        real
#to_struct  1.410000   0.000000   1.410000 (  1.403908)
#new_to_struct  0.760000   0.000000   0.760000 (  0.757548)
Struct  7.060000   0.010000   7.070000 (  7.075619)
OpenStruct  0.650000   0.000000   0.650000 (  0.649057)

These changes get to_struct close to OpenStruct, but still not as fast.