Simple ring/circular buffer c++ class V2 The Original Post (v1)

I'm looking for feedback on the updated version of the code posted at the link above.

#pragma once

#include <memory>
#include <cassert>
#include <type_traits>

namespace datastructures {

    template<class _Ty, size_t _Size>
    class CircularBufferv2 {

        // uses char as type to prevent default _Ty initialization.
        alignas(alignof(_Ty)) char buffer[_Size * sizeof(_Ty)];
        size_t head;
        size_t tail;
        bool isFull;

    public:
        constexpr CircularBufferv2() noexcept :
            buffer{0},
            head{0},
            tail{0},
            isFull{false} {
        }

        void push(const _Ty& item) noexcept {
            assert(!isFull && "Attempting to push item into full buffer!");

            new (&buffer[head * sizeof(_Ty)]) _Ty(std::move(item));

            head = ++head % _Size;
            isFull = head == tail;
        }

        _Ty pop() noexcept {
            assert(!is_empty() && "Attempting to pop item from empty buffer!");

            auto location = reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
            auto result = std::move(*location);
            std::destroy_at(location);

            tail = ++tail % _Size;
            isFull = false;

            return result;
        }

        _NODISCARD constexpr _Ty& peek() noexcept {
            assert(!is_empty() && "Attempting to peek in empty buffer!");

            return *reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
        }

        _NODISCARD constexpr const _Ty& peek() const noexcept {
            assert(!is_empty() && "Attempting to peek in empty buffer!");

            return *reinterpret_cast<_Ty*>(&buffer[tail * sizeof(_Ty)]);
        }

        _NODISCARD constexpr bool is_empty() const noexcept {
            return !isFull && tail == head;
        }

        _NODISCARD constexpr size_t get_capacity() const noexcept {
            return _Size;
        }

        _NODISCARD constexpr size_t get_size() const noexcept {
            if (isFull)
                return _Size;

            if (head >= tail)
                return head - tail;

            return _Size + head - tail;
        }

        _NODISCARD _CONSTEXPR17 _Ty* data() noexcept {
            return buffer;
        }

        _NODISCARD _CONSTEXPR17 const _Ty* data() const noexcept {
            return buffer;
        }
    };
}

I want to take advantage of all of the new features(c++17) while also supporting older compilers(preferably all older compilers but c++11 is probably as old as i'll actually be compiling for). Any suggestions welcome. (I'm attempting to use this class as an example class to follow when constructing other classes.)

Also, the use of _CONSTEXPR17 on the data functions I was wondering why use the macro vs just a constexpr? (I based the use of macros around the std::array struct, its data function uses _CONSTEXPR17 rather than just constexpr.)

Instead of a char array you can use std::aligned_storage.

You aren't consistent with your macros you have both constexpr and _CONSTEXPR17. Pick one and stick to it.

destroy_at is standard only from C++17 onward. The equivalent syntax is location->~_Ty(); However it may be worth to put this and the placement new into a helper function to move the ugly syntax elsewhere and clarify the function.

You don't have a move variant of push or a bulk add/remove using iterators.


Word-Ladder solver in Python 3 Follow-up to Python program to find a word ladder transforming "four" to "five"

I'm trying to write a program to solve a word ladder puzzle to brush up on my Python. For those who don't know:

  A word ladder puzzle begins with two words, and to solve the puzzle one must find a chain of other words to link the two, in which two adjacent words (that is, words in successive steps) differ by one letter. - Wikipedia

I've implemented breadth first search as opposed to depth first search for a big performance boost since my last question, as suggested in the answers.
I've also added a new mode, that can add and remove letters as well as just swapping them.
Additionally, the program now takes user input, and can take more than two words to link together (given a b c, it runs a b, and then runs b c).

The program now takes ~6 seconds on my machine to run with input of five four and 1. I'm still interested in making this faster, and also would like to know how to make it more Pythonic.

# -------- Data Structures --------
class Queue():
    """ FIFO Queue """
    def __init__(self, parent = None):
        self.list = []
        self.parent = parent
    def append(self, value):
        self.list.append(value)
    def pop(self):
        return self.list.pop(0)

class Node():
    """ Node of a Tree """
    def __init__(self, value, parent = None):
        self.value = value
        self.parent = parent

# -------- Distance Functions --------
def hamming(s1, s2):
    return sum(ch1 != ch2 for ch1, ch2 in zip(s1, s2))

def levenshtein(s1, s2):
    if len(s1) < len(s2):
        return levenshtein(s2, s1)
    if len(s2) == 0:
        return len(s1)
    previous_row = range(len(s2) + 1)
    for i, c1 in enumerate(s1):
        current_row = [i + 1]
        for j, c2 in enumerate(s2):
            insertions = previous_row[j + 1] + 1
            deletions = current_row[j] + 1
            substitutions = previous_row[j] + (c1 != c2)
            current_row.append(min(insertions, deletions, substitutions))
        previous_row = current_row
    return previous_row[-1]

# -------- IO --------

# Print sequence from root to node
def print_words(node):
    values = []
    while isinstance(node, Node):
        values.append(node.value)
        node = node.parent
    print(list(reversed(values)))

# Get all input
all_words = open("/usr/share/dict/words", "r").read().lower().splitlines()
input_words = input("Enter list of words, seperated by spaces: ").split()
input_mode = int(input("Enter 1 for swap-only mode, or 2 for swap-add-rem mode: "))

# Validate user input
if not 1 <= input_mode <= 2:
    raise ValueError("Invalid mode: " + input_mode)

for word in input_words:
    if word not in all_words:
        raise ValueError("Invalid word: " + word)

# Adapt to mode
distance = [hamming, levenshtein][input_mode - 1]
if input_mode == 1:
    all_words = [word for word in all_words if len(word) == len(input_words[0])]

# -------- Breadth First Search --------
def fill(node, to_check, checked):
    checked.add(node.value)
    for word in all_words:
        if 1 >= len(word) - len(node.value) >= -1 and distance(word, node.value) == 1:
            to_check.append(Node(word, node))

for i in range(len(input_words) - 1):
    root = Node(input_words[i])
    checked = set()
    to_check = Queue()

    fill(root, to_check, checked)
    while to_check.list:
        node = to_check.pop()
        if node.value == input_words[i + 1]:
            print_words(node)
            break
        if node.value not in checked:
            fill(node, to_check, checked)

I'm a little bit short on time at the moment, but I'ld like to share a few minor observations with you. Maybe I will also find time to look at performance related optimizations and expand it later.

You said you would like your code to be Pythonic. With that in mind you could replace print(list(reversed(values))) by print(values[::-1]) which uses slicing (nice explanation on Stack Overflow) to revert the list. Since you need the complete list nevertheless, there is no real advantage in using a reverse iterator instead of a list directly.

Also since "Explicit is better than implicit." (from Zen of Python) I would prefer to see

distance = hamming if input_mode == 1 else levenshtein

or something similar instead of

distance = [hamming, levenshtein][input_mode - 1]

The proposed version would also allow you to drop the string to int conversion. It would even work if the user entered something else than the two values presented by the prompt. levenshtein would be assumed to be the default then. Change the if to your liking if you prefer hamming as default. Although this might not be something to think about, since you have an input validation in place (good). However, the way it is written could be improved since you have a really small range (read: two) values that are valid. That makes it possible to list them explicitly:

# Validate user input
if input_mode not in (1, 2):
    raise ValueError("Invalid mode: " + input_mode)

This almost reads like normal language.


Python decorator for optional arguments decorator I want my Python decorators to have optional arguments and not be called when not necessary.

The accepted answer in here doesn't accept named arguments, and I don't want to add boilerplate code inside decorators, so I came up with an alternative decorator:

import inspect

def decorator_defaults(**defined_defaults):
    def decorator(f):
        args_names = inspect.getargspec(f)[0]

        def wrapper(*new_args, **new_kwargs):
            defaults = dict(defined_defaults, **new_kwargs)
            if len(new_args) == 0:
                return f(**defaults)
            elif len(new_args) == 1 and callable(new_args[0]):
                return f(**defaults)(new_args[0])
            else:
                too_many_args = False
                if len(new_args) > len(args_names):
                    too_many_args = True
                else:
                    for i in range(len(new_args)):
                        arg = new_args[i]
                        arg_name = args_names[i]
                        defaults[arg_name] = arg
                if len(defaults) > len(args_names):
                    too_many_args = True
                if not too_many_args:
                    final_defaults = []
                    for name in args_names:
                        final_defaults.append(defaults[name])
                    return f(*final_defaults)
                if too_many_args:
                    raise TypeError("{0}() takes {1} argument(s) "
                                    "but {2} were given".
                                    format(f.__name__,
                                           len(args_names),
                                           len(defaults)))
        return wrapper
    return decorator

Two sample decorators:

from functools import wraps

@decorator_defaults(start_val="-=[", end_val="]=-")
def my_text_decorator(start_val, end_val):
    def decorator(f):
        @wraps(f)
        def wrapper(*args, **kwargs):
            return "".join([f.__name__, ' ', start_val,
                            f(*args, **kwargs), end_val])
        return wrapper
    return decorator

@decorator_defaults(end_val="]=-")
def my_text_decorator2(start_val, end_val):
    def decorator(f):
        @wraps(f)
        def wrapper(*args, **kwargs):
            return "".join([f.__name__, ' ', start_val,
                            f(*args, **kwargs), end_val])
        return wrapper
    return decorator

And usage of sample text decorators:

@my_text_decorator
def func1a(value):
    return value

@my_text_decorator()
def func2a(value):
    return value

@my_text_decorator2("-=[")
def func2b(value):
    return value

@my_text_decorator(end_val=" ...")
def func3a(value):
    return value

@my_text_decorator2("-=[", end_val=" ...")
def func3b(value):
    return value

@my_text_decorator("|> ", " <|")
def func4a(value):
    return value

@my_text_decorator2("|> ", " <|")
def func4b(value):
    return value

@my_text_decorator(end_val=" ...", start_val="|> ")
def func5a(value):
    return value

@my_text_decorator2("|> ", end_val=" ...")
def func5b(value):
    return value

print(func1a('My sample text'))  # func1a -=[My sample text]=-
print(func2a('My sample text'))  # func2a -=[My sample text]=-
print(func2b('My sample text'))  # func2b -=[My sample text]=-
print(func3a('My sample text'))  # func3a -=[My sample text ...
print(func3b('My sample text'))  # func3b -=[My sample text ...
print(func4a('My sample text'))  # func4a |> My sample text <|
print(func4b('My sample text'))  # func4b |> My sample text <|
print(func5a('My sample text'))  # func5a |> My sample text ...
print(func5b('My sample text'))  # func5b |> My sample text ...

decorator_defaults works, but I believe it could be written better.
I'm not that experienced in Python, so I would like to hear some ideas/comments on how to improve it.

The code looks rather complicated. Instead of trying to understand it, I'd just like to point to NickC's answer to the linked SO question.

If I add **kwargs to his optional_arg_decorator like this...

def optional_arg_decorator(fn):
    def wrapped_decorator(*args, **kwargs):
        if len(args) == 1 and len(kwargs) == 0 and callable(args[0]):
            return fn(args[0])

        else:
            def real_decorator(decoratee):
                return fn(decoratee, *args, **kwargs)

            return real_decorator

    return wrapped_decorator

...and adapt your decorators like this, I'm getting the same output from the test cases.

from functools import wraps

@optional_arg_decorator
def my_text_decorator(f, start_val="-=[", end_val="]=-"):
    @wraps(f)
    def wrapper(*args, **kwargs):
        return "".join([f.__name__, ' ', start_val,
                        f(*args, **kwargs), end_val])
    return wrapper

@optional_arg_decorator
def my_text_decorator2(f, start_val, end_val="]=-"):
    @wraps(f)
    def wrapper(*args, **kwargs):
        return "".join([f.__name__, ' ', start_val,
                        f(*args, **kwargs), end_val])
    return wrapper


Spring MVC controller My code has a nasty smell that looks like this:

@RequestMapping(value = { "/updateGraph" }, method = RequestMethod.POST)
public ResponseEntity<ArrowsGraph> updateGraph(@RequestBody ArrowsGraph g){
    try{
        g = arrowsBo.updateGraph(g);
        return new ResponseEntity<ArrowsGraph>(g, HttpStatus.OK);
    } catch (Exception e) {
        errorLogging.logFatalException(e,  sessionState);
        return new ResponseEntity<ArrowsGraph>(HttpStatus.INTERNAL_SERVER_ERROR);
    }
}

@RequestMapping(value = { "/deleteGraph" }, method = RequestMethod.POST)
public ResponseEntity<ArrowsGraph>  deleteGraph(@RequestBody ArrowsGraph g){
    try{
        g = arrowsBo.deleteGraph(g);
        return new ResponseEntity<ArrowsGraph>(g, HttpStatus.OK);
    } catch (Exception e) {
        errorLogging.logFatalException(e,  sessionState);
        return new ResponseEntity<ArrowsGraph>(HttpStatus.INTERNAL_SERVER_ERROR);
    }
}

Things to mention:

The exceptions being caught are runtime exceptions, likely thrown if the database mapping is configured incorrectly. If the code is working correctly, exceptions should never be thrown, but in the case there is an error, I want to display an error the client side, instead of having unintended behaviour.
The reason I'm using ResponseEntity<..> instead of @ResponseBody is so I can return a 500 in the case that an exception is thrown.
The purpose of errorLogging.logFatalException(...) is to log the error, and notify admins.

Before I refactored to use ResponseEntity<...> and catch the runtime exceptions, the methods looked a lot cleaner:

@RequestMapping(value = { "/deleteGraph" }, method = RequestMethod.POST)
public ArrowsGraph  deleteGraph(@RequestBody ArrowsGraph )
     return arrowsBo.deleteGraph(g);
}

Is there a nice way to avoid this repetition/boilerplate?

Is there a nice way to avoid this repetition/boilerplate?

Yes, there is a cleaner way.

Since you are using Spring, please check the annotation @ControllerAdvice and the class ResponseEntityExceptionHandler.

To handle exceptions, you'll need to extend ResponseEntityExceptionHandler and manage the returned ResponseEntity in a method annotated with @ExceptionHandler, for example:

@ControllerAdvice(basePackageClasses = MyControllerClass.class)
public class ErrorResponseHandler extends ResponseEntityExceptionHandler {

  @ExceptionHandler(MyExceptionType.class)
  @ResponseBody
  ResponseEntity<Object> handleControllerException(HttpServletRequest request, Throwable ex) {
    // process the exception and wrap the response to return in ResponseEntity
  }

}

Using this approach, you service methods will remain one-liners, returning non-wrapped ArrowsGraph instances.


Character creator for a role-playing game Goal:

  Write a character creator program for a role-playing game. The player
  should be given a pool of 30 points to spend on four attributes:
  strength, health, wisdom, and dexterity. The player should be able to
  spend points from the pool on any attribute and should also be able to
  take points from an attribute and put them back into the pool.

  Python Programming for the Absolute Beginner by Michael Dawson

My attempt:

print "create a character! you have points to assign to strength, health, wisdom, and dexterity."
name=raw_input("what's your character's name? ")
points=30
attributes=("health", "strength", "wisdom", "dexterity")
strength=0
health=0
wisdom=0
dexterity=0
while True:
    print
    print "you have", points, "points left."
    print \
    """
    1-add points
    2-take points
    3-see points per attribute
    4-exit
    """
    choice=raw_input("choice: ")
    if choice=="1":
        attribute=raw_input("which attribute? strength, health, wisdom, or dexterity? ")
        if attribute in attributes:
            add=int(raw_input("how many points? "))
            if add<=points and add>0:
                if attribute=="strength":
                    strength+=add
                    print name, "now has", strength, "strength points."
                elif attribute=="health":
                    health+=add
                    print name, "now has", health, "health points."
                elif attribute=="wisdom":
                    wisdom+=add
                    print name, "now has", wisdom, "wisdom points."
                elif attribute=="dexterity":
                    dexterity+=add
                    print name, "now has", dexterity, "dexterity points."
                points-=add
            else:
                print "invalid number of points."
        else:
            print "invalid attribute."
    elif choice=="2":
        attribute=raw_input("which attribute? strength, health, wisdom, or dexterity? ")
        if attribute in attributes:
            take=int(raw_input("how many points? "))
            if attribute=="strength" and take<=strength and take>0:
                strength-=take
                print name, "now has", strength, "strength points."
                points+=take
            elif attribute=="health" and take<=health and take>0:
                health-=take
                print name, "now has", health, "health points."
                points+=take
            elif attribute=="wisdom" and take<=wisdom and take>0:
                wisdom-=take
                print name, "now has", wisdom, "wisdom points."
                points+=take
            elif attribute=="dexterity" and take<=dexterity and take>0:
                dexterity-=take
                print name, "now has", dexterity, "dexterity points."
                points+=take
            else:
                print "invalid number of points."
        else:
            print "invalid attribute."
    elif choice=="3":
        print "strength -", strength
        print "health -", health
        print "wisdom -", wisdom
        print "dexterity -", dexterity
    elif choice=="4":
        if points==0:
            break
        else:
            print "use all your points!"
    else:
        print "invalid choice."
print "congrats! you're done designing "+name+'.'
print name, "has", strength, "strength points,", health, "health points,", wisdom, "wisdom points, and", dexterity, "dexterity points."

I'm learning my first language, Python, and would really appreciate it if anyone could tell me if I'm writing the most efficient code possible.

One of the first things I would do is try and group your information into neater packages, rather than having a bunch of free variables. I assume you're not too familiar with classes, but try putting your character's attributes into a data structure like a List, or even better - a Dictionary:

attributes = {'health': 0, 'strength': 0, 'wisdom': 0, 'dexterity': 0 }

If you want to change attributes, you can then do

attributes['health'] = some_value

or to increment,

attributes['health'] += some_value

Secondly, the best way to improve your program is to make it more readable by splitting your code up into functions. For instance, the code below is an example of what it may look like if you took some of the stat changing logic out of the main program:

if choice=="1":
    stat = input("Which stat? ")
    modifier = input("By how much? ")
    add_stats(modifier, stat)
elif choice=="2":
    stat = input("Which stat? ")
    modifier = input("By how much? ")
    take_stats(modifier, stat)
elif choice=="3":
    print(show_player_stats())

In general, any where you find yourself getting "too deep" in nests of if statements and loops, or you find yourself repeating your code, try and break it out into a function. Bear in mind this is a crude example, and you'll have to develop your own solution, but in terms of readability it's a vast improvement.

I've made a start on a version of your game using my own approach, although I haven't implemented all of it (you can't subtract points for instance). However, you can probably already see where the program has improved on things.

Things to note about my version compared to yours:

There is commenting (although basic). Commenting code is a MUST, even if it's for yourself. It will help you understand your own code and help anyone else who uses it, even if it's just to show different parts of the program.
The code is broken down into functions, which improves readability and allows you to re-use bits of code in the future. For instance, every time you want to print your character's info, you just call print_character()!
The code is neater - I'm packing information into a data structure, strings are formatted with linebreaks, logic is compartmented into smaller, manageable chunks.

However, I have used some Python you may not be familiar with, like the keys() method. But it's important to go through the code and try and work out what's happening. This will help to expose you to the "Python" way of doing things.

##### GAME FUNCTIONS #####

def add_character_points():
    attribute = raw_input("\nWhich attribute? Strength, Health, Wisdom or Dexterity?\n")

    if attribute in my_character.keys():
        amount = int(raw_input("By how much?"))

        if (amount > my_character['points']) or (my_character['points'] <= 0):
            print "Not enough points!"
        else:
            my_character[attribute] += amount
            my_character['points'] -= amount
    else:
        print "\nThat attribute doesn't exist!\n"

def print_character():
    for attribute in my_character.keys():
        print attribute, " : ", my_character[attribute]

##### MAIN FUNCTION #####

my_character = {'name': '', 'strength': 0, 'health': 0, 'wisdom': 0, 'dexterity': 0, 'points': 20}
running = True

print "Create a character! You have points to assign to strength, health, wisdom, and dexterity."

my_character['name'] = raw_input("What is your character's name? ")

while running:
    print "\nYou have ", my_character['points'], " points left.\n"
    print "1. Add points\n2. Remove points\n3. See current attributes\n4. Exit\n"

    choice = raw_input("Choice:")

    if choice == "1":
        add_character_points()
    elif choice == "3":
        print_character()
    elif choice == "4":
        running = False
    else:
        pass


Python Tic Tac Toe Game I made this as a personal challenge. It seems to work fine as I can't find any bugs and am happy with how it runs, but I am interested in what I should do to make the code more professional.

I think the draw() function can be replaced with two for loops but I could never get it working. The code for the input by each of the players and the check_board() function seems messy and I think it could be reduced by using arguments. I also think the while loop could be reduced somehow.

def tic_tac_toe():
    board = [1, 2, 3, 4, 5, 6, 7, 8, 9]
    end = False
    win_commbinations = ((0, 1, 2), (3, 4, 5), (6, 7, 8), (0, 3, 6), (1, 4, 7), (2, 5, 8), (0, 4, 8), (2, 4, 6))

    def draw():
        print(board[0], board[1], board[2])
        print(board[3], board[4], board[5])
        print(board[6], board[7], board[8])
        print()

    def p1():
        n = choose_number()
        if board[n] == "X" or board[n] == "O":
            print("\nYou can't go there. Try again")
            p1()
        else:
            board[n] = "X"

    def p2():
        n = choose_number()
        if board[n] == "X" or board[n] == "O":
            print("\nYou can't go there. Try again")
            p2()
        else:
            board[n] = "O"

    def choose_number():
        while True:
            while True:
                a = input()
                try:
                    a  = int(a)
                    a -= 1
                    if a in range(0, 9):
                        return a
                    else:
                        print("\nThat's not on the board. Try again")
                        continue
                except ValueError:
                   print("\nThat's not a number. Try again")
                   continue

    def check_board():
        count = 0
        for a in win_commbinations:
            if board[a[0]] == board[a[1]] == board[a[2]] == "X":
                print("Player 1 Wins!\n")
                print("Congratulations!\n")
                return True

            if board[a[0]] == board[a[1]] == board[a[2]] == "O":
                print("Player 2 Wins!\n")
                print("Congratulations!\n")
                return True
        for a in range(9):
            if board[a] == "X" or board[a] == "O":
                count += 1
            if count == 9:
                print("The game ends in a Tie\n")
                return True

    while not end:
        draw()
        end = check_board()
        if end == True:
            break
        print("Player 1 choose where to place a cross")
        p1()
        print()
        draw()
        end = check_board()
        if end == True:
            break
        print("Player 2 choose where to place a nought")
        p2()
        print()

    if input("Play again (y/n)\n") == "y":
        print()
        tic_tac_toe()

tic_tac_toe()

My strongest objection is to your use of recursion as a kind of goto:

if input("Play again (y/n)\n") == "y":
    print()
    tic_tac_toe()

I suggest changing the tic_tac_toe() function to play just one game, then calling it in a loop.

You do some similar recursive calls in p1() and p2().

Board layout

Your board layout is

1 2 3
4 5 6
7 8 9

For a better user experience, I suggest flipping it upside-down to mimic a computer number keypad.

I think that the code would be less confusing if the board labels matched the list indices.

Handling two players

The p1() function resembles p2(), and the main loop contains two copies of code that is mostly the same.  check_board() also has some repetition.  The root cause is the need to associate Player 1 with the symbol X and the verbal description "cross", and Player 2 with the symbol O and the verbal description "nought".  The code would be easier to generalize if you just called them "Player X" and "Player O", and I think that the user experience would be enhanced as well.  (You could also generalize the code without such a simplification by using, say, a namedtuple to represent each player.)

p1() and p2() could be simplified by moving the validation into choose_number(), especially if you didn't care to distinguish between the "You can't go there" and "That's not on the board" error messages.

Implementation details

In choose_number(), there is no need for a nested while True.  You also don't need to write continue.
Renaming check_board() to is_game_over() would make its purpose more obvious
win_commbinations contains a misspelling, and would be better as WIN_COMBINATIONS to indicate that it is a constant.  board[a[0]] == board[a[1]] == board[a[2]] is ugly, and would be better expressed as

for a, b, c in WIN_COMBINATIONS:
    if board[a] == board[b] == board[c] …

In check_board(), you used the variable a for two different purposes, which is confusing.  I would just rewrite the second half of the function entirely as

if 9 == sum((pos == 'X' or pos == 'O') for pos in board):
    print("The game ends in a tie\n")
    return True

There is no need for the end variable.  (In general, such flag variables should be eliminated.)

Suggested solution

def tic_tac_toe():
    board = [None] + list(range(1, 10))
    WIN_COMBINATIONS = [
       (1, 2, 3),
       (4, 5, 6),
       (7, 8, 9),
       (1, 4, 7),
       (2, 5, 8),
       (3, 6, 9),
       (1, 5, 9),
       (3, 5, 7),
    ]

    def draw():
        print(board[7], board[8], board[9])
        print(board[4], board[5], board[6])
        print(board[1], board[2], board[3])
        print()

    def choose_number():
        while True:
            try:
                a = int(input())
                if a in board:
                    return a
                else:
                    print("\nInvalid move. Try again")
            except ValueError:
               print("\nThat's not a number. Try again")

    def is_game_over():
        for a, b, c in WIN_COMBINATIONS:
            if board[a] == board[b] == board[c]:
                print("Player {0} wins!\n".format(board[a]))
                print("Congratulations!\n")
                return True
        if 9 == sum((pos == 'X' or pos == 'O') for pos in board):
            print("The game ends in a tie\n")
            return True

    for player in 'XO' * 9:
        draw()
        if is_game_over():
            break
        print("Player {0} pick your move".format(player))
        board[choose_number()] = player
        print()

while True:
    tic_tac_toe()
    if input("Play again (y/n)\n") != "y":
        break


Python word counter for all files in the current directory I wrote a simple command-line script to count all words in all documents (of the supported formats) in the current directory. Currently it supports TXT, DOCX, XLSX, and PDF formats and has been satisfactorily tested with them. As a freelance translator and content writer, this script provides me with an excellent tool, to quickly evaluate the scope of large projects by simply "dropping" the script into a directory and running it from PowerShell/Terminal.

Currently tested in Windows 10 only.

What do you think of this script? What should I improve?

import os
import openpyxl
import platform
import docx2txt
import PyPDF2

def current_dir():
    if platform.system() == "Windows":
        directory = os.listdir(".\\")
    else:
        directory = os.getcwd()
    return directory

def excel_counter(filename):
    count = 0
    wb = openpyxl.load_workbook(filename)
    for sheet in wb:
        for row in sheet:
            for cell in row:
                text = str(cell.value)
                if text != "None":
                    word_list = text.split()
                    count += len(word_list)
    return count

def pdf_counter(filename):
    pdf_word_count = 0
    pdfFileObj = open(filename, "rb")
    pdfReader = PyPDF2.PdfFileReader(pdfFileObj)
    number_of_pages = pdfReader.getNumPages() - 1
    for page in range(0, number_of_pages + 1):
        page_contents = pdfReader.getPage(page - 1)
        raw_text = page_contents.extractText()
        text = raw_text.encode('utf-8')
        page_word_count = len(text.split())
        pdf_word_count += page_word_count
    return pdf_word_count

def main():
    word_count = 0
    print(f"Current Directory: {os.getcwd()}")
    for file in current_dir():
        file_name_list = os.path.splitext(file)
        extension = file_name_list[1]
        if extension == ".xlsx":
            current_count = excel_counter(file)
            print(f"{file} {current_count}")
            word_count += current_count
        if extension == ".docx":
            text = docx2txt.process(file)
            current_count = len(text.split())
            print(f"{file} {current_count}")
            word_count += current_count
        if extension == ".txt":
            f = open(file, "r")
            text = f.read()
            current_count = len(text.split())
            print(f"{file} {current_count}")
            word_count += current_count
        if extension == ".pdf":
            pdf_word_count = pdf_counter(file)
            print(f"{file} {pdf_word_count}")
            word_count += pdf_word_count
        else:
            pass
    print(f"Total: {word_count}")

main()

I recommend using pathlib and Path objects instead of os, and you should use a context manager when manipulating files (e.g. with open("file.txt", "r") as file: ...). You also have a lot of repeating code when you're checking extensions, and you keep checking the rest of the if statements even if it's matched an earlier one. And the final else: pass does literally nothing so just remove that.

You could also do something about your nested for loops for sheet, row and cell (you'd typically use zip or itertools.product then) but this is kind of readable and nice so I'm not sure it's worth the conversion.


Linq-to-entities query too slow I am fairly new to Entity Framework and I am not sure I am really using it to it's max potential.  I have this query, that IMHO seems to take a little longer than I feel it should.  I have tried various combinations to retrieve all the data I need making as few connections as possible and so far this query yeilds the best results with an average of about 320 ms.  Is there anything I can do to make this query faster?

Something to note is that the include for Awe_PageContainerBlurb is a many-to-many relationship based on 3 tables.

var roles = from role in db.aspnet_Roles
            join userRole in db.aspnet_UsersInRoles on role.RoleId equals userRole.RoleId
            where userRole.UserId == userID
            select role;

var awePageData = ((ObjectQuery<Awe_Page>)(from page in db.AweBjects.OfType<Awe_Page>()
                                           join app in db.aspnet_Applications on page.ApplicationId equals app.ApplicationId
                                           where app.ApplicationName == user.ApplicationName
                                               && page.Active && page.Name == pageName
                                           from pageRole in page.aspnet_Roles
                                           where roles.Contains(pageRole)
                                           select page))
                                           .Include("Awe_PageContainerBlurb.Awe_Container")
                                           .Include("Awe_PageContainerBlurb.Awe_Blurb");

var menuItems = from menu in db.AweBjects.OfType<Awe_Menu>()
                join app in db.aspnet_Applications on menu.ApplicationId equals app.ApplicationId
                where app.ApplicationName == user.ApplicationName
                    && menu.Active
                from menuRole in menu.aspnet_Roles
                where roles.Contains(menuRole)
                select menu;

First things first. Since we are talking about an SQL backend (unless I'm mistaken) - you need to profile SQL and take each (sub)query seperately, then determine which query takes the longest. It would simply be pure guess work, taking answers out of thin air otherwise, since we have no reference for what the bottleneck actually is.

Next, what are you SQL indexes? You have many joins and where clauses. If you don't have your indexes set up correctly, then - there's your problem!

Finally compare your c# query to the actual SQL query. Sometimes your LINQ can look beautiful, but the actual code generated for the query is utter BS. I deal with LINQ2SQL myself and have many times been forced to make a list (ToList()) from the sub-queries, forcing the data into the client, into the realm of c# and doing the final query on top of those lists instead. I may use more memory for a brief period of time, but I gain orders of magnitude more speed...


How to turn this working A/B optimization program into more pythonic code? I have a working solution for the given problem of A/B Optimization, but you as an experienced Python (3) developer will agree with me, that this solution is not very pythonic at all.
My Question here is: How do I make this code more pythonic?
It's a very procedural process that one would expect from a Basic program, but not from a Python program. Please help me to become a better coder and make pythonic code by working through this example with me.
Here's the problem and my solution.
Problem

In this test you will be given a CSV containing the results of a website's A/B homepage test. The site is simultaneously testing 5 design variables (A, B, C, D, and E) and tracking if users clicked on the signup button (Z). Each line of the CSV represents a visitor to the homepage, what they saw, and if they clicked or not. The A-E variables will have values of 1-5, representing which version of that design element was shown to the user. The Z variable will contain either a 1 to represent that the user clicked signup, or a 0 to represent that the user did not.
A B C D E Z
1 3 1 2 4 1
3 2 1 5 5 0
4 3 2 2 5 1
5 5 2 3 4 0
2 4 1 3 4 0
...

Your task will be to determine the single combination of A-E values that will make users most likely to click the signup button. Assume the effects of each A-E value are mutually exclusive. If two values of a variable are equally optimal, choose the lesser value. You will enter the answer in the form of a 5-digit number, the first digit being the optimal A value, the second being the optimal B value, the third being the optimal C value, etc.

Solution
from urllib.request import urlopen

def getBestDesign(url):
    data = urlopen(url).readlines()
    A = {}; B = {}; C = {}; D = {}; E = {};
    A['1'] = 0
    A['2'] = 0
    A['3'] = 0
    A['4'] = 0
    A['5'] = 0
    B['1'] = 0
    B['2'] = 0
    B['3'] = 0
    B['4'] = 0
    B['5'] = 0
    C['1'] = 0
    C['2'] = 0
    C['3'] = 0
    C['4'] = 0
    C['5'] = 0
    D['1'] = 0
    D['2'] = 0
    D['3'] = 0
    D['4'] = 0
    D['5'] = 0
    E['1'] = 0
    E['2'] = 0
    E['3'] = 0
    E['4'] = 0
    E['5'] = 0
    for row in data:
        row = row.decode(encoding='UTF-8').split('\t')
        if row[5].startswith('1'):
            A[str(row[0])] += 1
            B[str(row[1])] += 1
            C[str(row[2])] += 1
            D[str(row[3])] += 1
            E[str(row[4])] += 1
    maxclicks_A = max(A.values())
    maxclicks_B = max(B.values())
    maxclicks_C = max(C.values())
    maxclicks_D = max(D.values())
    maxclicks_E = max(E.values())
    for k, v in A.items():
        if v == maxclicks_A:
            print('A: ', k)
    for k, v in B.items():
        if v == maxclicks_B:
            print('B: ', k)
    for k, v in C.items():
        if v == maxclicks_C:
            print('C: ', k)
    for k, v in D.items():
        if v == maxclicks_D:
            print('D: ', k)
    for k, v in E.items():
        if v == maxclicks_E:
            print('E: ', k)

if __name__ == "__main__":
    url = input('Url: ')
    getBestDesign(url)

There is massive code duplication in your script. Basically you are doing the same things for A through E, so you could just put them in a list of dictionaries and use loops. Also, by using defaultdict you do not have to initialize them per hand (and even if not you could use a loop for this, too). Also, I'd recommend removing the code for transforming the URL to a 2D array from the function, and using just split() without '\t' the code works as well, and is somewhat more robust to other input formats.

from urllib.request import urlopen
from collections import defaultdict

def getBestDesign(data):
    # this list holds all your A, B, C, ... dicts, with defaultvalues
    ABCDE = [defaultdict(int) for i in range(5)]
    for row in data:
        if row[5] == '1':
            for i in range(5):
                ABCDE[i][row[i]] += 1
    for i in range(5):
        maxclicks = max(ABCDE[i].values())
        for k, v in ABCDE[i].items():
            if v == maxclicks:
                print('ABCDE'[i], ": ", k) # note the ''!
                # break # optional, stop after first "max" value

if __name__ == "__main__":
    url = input('Url: ')
    lines = urlopen(url).readlines()
    data = [line.decode(encoding='UTF-8').split() for line in lines]
    getBestDesign(data)

The algorithms is still basically the same as yours, and I think that it can still be improved. Probably you could use numpy to treat the input data as a matrix and do all sorts of useful matrix operations on the data. Further, you could probably use collections.Counter or itertools.groupby for counting the good combinations, reducing the whole thing to two or three lines.

Here's another version, even shorter, doing some preprocessing and using collections.Counter:

def getBestDesign(data):
    filtered = [d[:-1] for d in data if d[5] == '1']
    for i, d in enumerate(zip(*filtered)):
        key, maxclicks = Counter(d).most_common(1)[0]
        print("{0}: {1} ({2})".format("ABCDE"[i], key, maxclicks))

Let's explain this line by line:

the filtered list comprehension holds just the lines from data where the user has clicked; as this is already filtered, we can drop the last column with [:-1]
in the loop, we iterate the data columns -- zip(*...) basically transposes the matrix -- using enumerate, which yields the index i and the actual data d at that index
the Counter class is used to count each variant and to return the most_common one
finally, we print the result using string.format, using the index i to get the respective letter from the string "ABCDE"


setTimeout and setInterval Problem statement
After 10 sec of execution of a program, start reading the property descriptors of each property of an object and continue reading all remaining properties each, every 3 seconds until done.

Solution
function Timer(obj, startTime, interval) {
    var PDO ={
        obj:{value: obj, enumerable: true, writable:false, configurable:false},
        startTime: {value:startTime, enumerable:true, writable:false, configurable:false},
        interval: {value:interval, enumerable:true, writable:false, configurable:false}
    };
    Object.defineProperties(this, PDO);
}

Timer.prototype.readProperties = function(){
    var props = Object.getOwnPropertyNames(this.obj);
    var self = this;
    function readPDO(){
        console.log(Object.getOwnPropertyDescriptor(self.obj, props[0]));
        props.shift();
        if (props.length === 0) {
            clearInterval(self.clearID);
        }
    }
    setTimeout(function(){
        self.clearID = setInterval(function(){
                                        readPDO();
                                    },
                                    self.interval
                                  );
                        },
                        self.startTime
              );
}

var t = new Timer({x:1, y:2}, 10000, 3000);
t.readProperties();

Can we improve the solution?
Are the naming conventions looking fine?

Describe your problem better

While this may seem not be a thing to be pointing out in a code review it is important to describe your problem in a nice way.
I would describe it as follows:

I want to yield each property of an object every three seconds. However the first one should be delayed by 10 seconds.

Separation of concerns

You are mixing the time repeating problem with the property yielding.
You also don't allow a way to specify a different strategy for what to do whenever another property comes (in this case that would be the console.log).

function setIntervalDelayed(delay, interval, cb){
    setTimeout(function(){
        if(cb()){
            setIntervalDelayed(interval, interval, cb);
        }
    }, delay);
}

function yieldProperties(obj, delay, interval, cb){
    var props = Object.getOwnPropertyNames(obj);
    setIntervalDelayed(delay, interval, function(){
        if(props.length == 0) return false;
        cb(obj[props[0]]);
        props.shift();
        return props.length > 0;
    })
}

yieldProperties({x:0, y:1, z:2}, 1000 * 10, 1000 * 3, console.log);

EDIT

While my approach is sligthly better then yours there is still mixed code. Again yieldProperties is doing more then it should, it still has that depedency of the delay and interval arguments, that don't really fit there.
Solution: implement iterator pattern.

    function setIntervalDelayed(delay, interval, cb){
        setTimeout(function(){
            if(cb()){
                setIntervalDelayed(interval, interval, cb);
            }
        }, delay);
    }

    function iterate(arr, cb){
        var i = 0;
        return function(){
            if(i >= arr.length) return false;
            cb.apply(arr[i]);
            ++i;
            return true;
        };
    }

    function yieldProperties(obj, cb){
        return iterate(Object.getOwnPropertyNames(obj), function(){
            cb({
                name: this.toString(),
                value: obj[this]
            });
        });
    }

    setIntervalDelayed(10 * 1000, 3 * 1000, yieldProperties({x:0, y:1, z:2}, console.log.bind(console)))


Comma Code (Python) (Automate The Boring Stuff) This is an exercise in the Automate The Boring Stuff book. I am supposed to create a function that takes a list value as an argument and returns a string with all the items separated by a comma and a space, with 'and' inserted before the last item. My code also includes a loop for the user to create their list.
def add_and(alist):
    if ((alist != []) & (len(alist) > 2)):
        for item in alist[:-1]:
            print(item + ", ",end="")
        print("and", alist[-1])
    elif ((alist != []) & (len(alist) == 2)):
        print(alist[0], "and", alist[1])
    elif ((alist != []) & (len(alist) == 1)):
        print(alist[0])
    else:
        print("")

your_list = []
while True:
    print("Enter An Item or nothing")
    item = input()
    if ((item) == ""):
        print("")
        break
    else:
        your_list.append(item)
        continue

add_and(your_list)

I know that the code works, but I was wondering if there were any faux-pas that I am implementing, or if there is anything I could obviously do to make it cleaner. Thanks!

Don't surround sub-predicates or predicates with parens when that isn't needed
Use logical and rather than binary &
Use join rather than a for-and-concatenate
Factor out common sub-predicates for your list emptiness checks into an outer if
Don't else-after-break
No need to continue
Gather your input in a separate function
Do not leave the input prompt blank

Suggested:
def and_add(alist):
    if len(alist) > 1:
        print(', '.join(alist[:-1]), 'and', alist[-1])
    elif len(alist) == 1:
        print(alist[0])
    else:
        print('')

def get_input():
    while True:
        item = input('Enter an item or nothing: ')
        if not item:
            return
        yield item

and_add(list(get_input()))


Processing eBay category list for database import Although I've been programming a fair while I'm learning Python and would like to know how I can make my code more "Pythonic".

I'm have a CSV file of eBay categories, containing the name and numeric code which I want to import in to my database. Unfortunately the file does not include a 'parent' category ID for each row, instead it uses the ordinal position of the name in row to indicate the nesting level:

L1,L2,L3,L4,L5,L6,Category ID
Antiques,,,,,,20081
,Antique Clocks,,,,,13851
,,Bracket Clocks,,,,100904
,,,Pre-1900,,,66840
,,,Post-1900,,,96762
,,Cuckoo Clocks,,,,13850
,,Longcase/ Grandfather Clocks,,,,100905
,,,Pre-1900,,,60249
,,,Post-1900,,,96763
,Antique Furniture,,,,,20091
,,Armoires/ Wardrobes,,,,98436

I've written a Python script to process the file, which is working correclty and generating the output I need, but I would like to know how it can be improved.

The script below is slightly changed from the one I am using, with simplified output (making it less verbose) but if anyone is interested in the other version which generates a JSON file for importing in to a Django site database, let me know.

I wanted to process the file line-by-line and avoid loading all the data in to memory, in case the file being processed is huge. So it uses a stack to record the parent category ID, other than that very little information is stored.

import csv

class Stack:
    def __init__(self):
        self.items = []

    def push(self, o):
        self.items.append(o)

    def pop(self, n=1):
        while n > 1:
            self.items.pop()
            n -= 1
        return self.items.pop()

    def peek(self, n=1):
        return self.items[-n] if len(self.items) >= n else 'null'

def extract_info(row):
    """
    :param row: data extracted from the csv file
    :return: level (number), the name of the category (string), category ID (number)

    Steps through the csv row until the first populated field (category name)
    and then returns the nesting level, category name and ID (which is always in
    the last field of the row.
    """
    level = 0
    for field in row:
        if field:
            return level, field, row[-1]
        level += 1

id_stack = Stack()

with open('UK_New_Structure_(Oct2015).csv', 'rb') as csvfile:
    rdr = csv.reader(csvfile, delimiter=',', quotechar='"')
    # Skip three header rows
    next(rdr)
    next(rdr)
    next(rdr)
    first_row = next(rdr)
    parent_id = 'null'  # top level categories do not have a parent

    level, name, cat_id = extract_info(first_row)
    print level, name, cat_id, parent_id
    id_stack.push(cat_id)  # Save first parent ID
    last_level = -1

    for row in rdr:
        level, name, cat_id = extract_info(row)

        if level == last_level:  # remove the newly added parent id, last item was not a parent
            id_stack.pop()
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
        elif level < last_level:
            id_stack.pop(last_level - level + 1)
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
            last_level = level
        elif level > last_level:
            parent_id = id_stack.peek()
            id_stack.push(cat_id)
            last_level = level

        print level, '..' * level, name, cat_id, parent_id

I am using Python 2.7.10, but if there are better ways to do this in Python 3, then I'm interested in those as well.

I don't have many comments regarding your style, as it seems to follow PEP8 (to my knowledge), so lets review the actual code:

Implementation of Stack – A straight forward implementation without to much fuzz. You don't have any error checking related to popping something of an empty stack, which might give you a problem at some point
Implementation of extract_info – No error handling if faulty input, which could let it return nothing at all if nothing matches on the if field: line
rdr is not a good name – It is not clear what next(rdr) actually does, try finding a better name here
Simplify if structure – You repeat statements within the if structure, and these could be moved outside of the if structure, which would help readability. In addition you pop/push too many times. Try the following:

if level <= last_level:
    id_stack.pop(last_level - level + 1)

parent_id = id_stack.peek()

id_stack.push(cat_id)
last_level = level

Change to newer print – The currently recommended standard for printing seems to be of the form: print('{} {} {} {} {}'.format(level, '..'*level, name, cat_id, parent_id)). In your case it complicates stuff, but in the long run you can pretty print much better using the newer form and variation thereover

Edit: Original code had a little flaw regarding the parent_id. The now corrected code gives the same output when using the provided CSV file as the output from original code.


Implement numbering scheme like A,B,C... AA,AB,... AAA..., similar to converting a number to radix26 I want to implement numbering scheme like Microsoft Word uses for numbering.
first one gets = A,next is  B, next is C, .... then AA, then AB,....and so on.
as shown below

 A
 B
 C
 .
 .
 AA
 AB
 AC
 .
 .
 AAA
 AAB
 ....

'=>' here means converted to.

some examples:

1 => A
26 => Z
27 => AA
52 => AZ
53 => BA

and heres the code for it:

   var convertToNumberingScheme = function(n){
        var x = n-1,
            r26 = x.toString(26),
            baseCharCode = "A".charCodeAt(0);

        var arr = r26.split(''),
            len = arr.length;

        var newArr =arr.map(function(val,i){
            val = parseInt(val,26);

            if( (i === 0) && ( len > 1)){
                val = val-1;
            }

            return String.fromCharCode(baseCharCode + val);
        });
        return newArr.join('');
    }

It seems to work fine, but any ideas if there are some potential bugs or ways to  optimize this.

The function in the question converts to base 26, then splits the resulting string, and converts each digit back to decimal - and then to a letter. That seems roundabout.

Here's a simpler one:

function convertToNumberingScheme(number) {
  var baseChar = ("A").charCodeAt(0),
      letters  = "";

  do {
    number -= 1;
    letters = String.fromCharCode(baseChar + (number % 26)) + letters;
    number = (number / 26) >> 0; // quick `floor`
  } while(number > 0);

  return letters;
}

This function basically does a repeated "divmod" of the input number; getting the modulus 26 and the quotient (floor of n divided by 26). The modulus is converted to a A-Z character that's prepended to the output string, and the quotient is used as the input for the next iteration of the loop.


Creating the design matrices for a canonical correlation analysis I have this code to generate symmetric matrices for testing how the design of the canonical correlation analysis I am performing works out. This is a extension of this solution

Each row of the matrix represent a dataset (it is symmetric), and if the value is 0 it means no interaction between the datasets, if it is higher that there is an interaction. The end goal of this code is just to make a grid search of the design that explains better the data I have.
However as I need to come up with different designs adding more datasets or less I would like to know how to improve this into a function that would be more general, specially the nested for loops part (If I test it with 5 datasets I add more for loops and then more unlist at the end).

Initial matrix (I usually work with 4 datasets):

C <-  matrix(0,ncol = 4, nrow = 4)

Weights for the interactions of each dataset (4 to avoid to many combinations):

nweight <- 4
weight <- seq(from = 0, to = 1, length.out = nweight)

Initiate the list that will contain the matrices

C_list <- vector("list", nweight)
cweight <- as.character(weight)
names(C_list) <- cweight

Loop for each position I want to change to obtain all the combinations of weights I want to test.

for(i1 in cweight) {
  C_list[[as.character(i1)]] <- vector("list", nweight)
  names(C_list[[(i1)]]) <- cweight
  for (i2 in cweight) {
    C_list[[(i1)]][[(i2)]] <- vector("list", nweight)
    names(C_list[[(i1)]][[(i2)]]) <- cweight
    for (i3 in cweight) {
      C_list[[(i1)]][[(i2)]][[(i3)]] <- vector("list", nweight)
      names(C_list[[(i1)]][[(i2)]][[(i3)]]) <- cweight
      for(i4 in cweight) {
        C_list[[(i1)]][[(i2)]][[(i3)]][[(i4)]] <- vector("list", nweight)
        names(C_list[[(i1)]][[(i2)]][[(i3)]][[(i4)]]) <- cweight
        for (i5 in cweight) {
          C_list[[(i1)]][[(i2)]][[(i3)]][[(i4)]][[(i5)]] <- vector("list", nweight)
          names(C_list[[(i1)]][[(i2)]][[(i3)]][[(i4)]][[(i5)]]) <- cweight
          for (i6 in cweight) {
            C[1, 2] <- as.numeric(i1)
            C[2, 1] <- as.numeric(i2)

            C[1, 3] <- as.numeric(i2)
            C[3, 1] <- as.numeric(i2)

            C[1, 4] <- as.numeric(i3)
            C[4, 1] <- as.numeric(i3)

            C[2, 3] <- as.numeric(i4)
            C[3, 2] <- as.numeric(i4)

            C[2, 4] <- as.numeric(i5)
            C[4, 2] <- as.numeric(i5)

            C[4, 3] <- as.numeric(i6)
            C[3, 4] <- as.numeric(i6)
            C_list[[i1]][[i2]][[i3]][[i4]][[i5]][[i6]] <- C
          }
        }
      }
    }
  }
}

Unlist the list of list of list of ... nested matrices to end up with a long list of matrices with the weights for each dataset

C_list2 <- unlist(unlist(unlist(unlist(unlist(C_list, FALSE, FALSE),
                                       FALSE, FALSE), FALSE, FALSE),
                         FALSE, FALSE), FALSE, FALSE)

Here you want to move away from for loops for two reasons:

your number of for loops depends on your number of datasets, so using for loops prevents you from generalizing your code to any number of datasets.
many for loops will likely slow down your code execution when working with a larger number of datasets.

I think the key to vectorizing your for loops is to use the expand.grid function. If you have

n <- 4

datasets, then you have

p <- n * (n - 1) / 2    # 6

degrees of freedom (the number of for loops in your code, or the number of items on the lower triangle of each matrix). If for each of these you can pick among

w <- seq(from = 0, to = 1, length.out = n)

then you can build the matrix of all possible combinations by doing:

W <- as.matrix(expand.grid(rep(list(w), p)))

Here W is a big matrix with 4096 rows, each row representing a different combination of your (i1, i2, i3, i4, i5, i6) variables:

> head(W)
          Var1      Var2 Var3 Var4 Var5 Var6
[1,] 0.0000000 0.0000000    0    0    0    0
[2,] 0.3333333 0.0000000    0    0    0    0
[3,] 0.6666667 0.0000000    0    0    0    0
[4,] 1.0000000 0.0000000    0    0    0    0
[5,] 0.0000000 0.3333333    0    0    0    0
[6,] 0.3333333 0.3333333    0    0    0    0

These 6 columns are only part of the n * n = 16 values needed in each matrix. We can expand using the following:

X <- matrix(1:(n*n), n, n) # pattern matrix of indices
A <- matrix(0, nrow(W), n * n)
A[,    X[lower.tri(X)]] <- W
A[, t(X)[lower.tri(X)]] <- W

A is similar to W in that it is a matrix with 4096 rows, but each row now has the n * n = 16 values of a symmetric matrix.

From there, you can reshape A into a 3D array:

dim(A) <- c(nrow(W), n, n)

and your 4096 matrices can be accessed as follows:

A[1, , ]
#      [,1] [,2] [,3] [,4]
# [1,]    0    0    0    0
# [2,]    0    0    0    0
# [3,]    0    0    0    0
# [4,]    0    0    0    0

A[10, , ]
#           [,1]      [,2]      [,3] [,4]
# [1,] 0.0000000 0.3333333 0.6666667    0
# [2,] 0.3333333 0.0000000 0.0000000    0
# [3,] 0.6666667 0.0000000 0.0000000    0
# [4,] 0.0000000 0.0000000 0.0000000    0

A[4096, , ]
#      [,1] [,2] [,3] [,4]
# [1,]    0    1    1    1
# [2,]    1    0    1    1
# [3,]    1    1    0    1
# [4,]    1    1    1    0

If I were you I would probably stop here, i.e., keep the data in this form. A 3d array might allow you to continue writing vectorized code if the rest of your analysis allows for it. However if you absolutely want a list of matrices, you can do:

C_list2 <- lapply(seq(nrow(A)), function(i) A[i, , ])

(note that the order of the matrices in my data and yours do not match. Let me know if this is a concern, it's probably a matter of reorganizing the rows and/or columns of the W matrix.)


Evaluate mathematical expression in reverse Polish notation in C This is a program exercise from The C Programming Language by Kernighan and Ritchie (Chap 5).

It is based on my earlier question here on Stack Overflow.

One user suggested to submit my code here, saying there's a whole
lot of bad coding practice in this snippet. Since most of the harmful
practice here originates from K&R, you might want to consider a better
source for learning C.

What are instances of bad coding practice in this program? How can it be improved or written in more compact form?

If The C Programming Language by Kernighan & Ritchie is not good way to start learning C programming, I'm open to suggestions on an alternative good read.

//Exercise 5-10. Write the program expr, which evaluates a reverse Polish
//expression from the command line, where each operator or operand is a
//separate argument. For example, expr 2 3 4 + *
//evaluates 2 x C+4).

//For multiplication character '*' is not working

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

#define MAXLINE 1000
#define NUMBER 0

int sign = 1;
char s[MAXLINE];

void calc (int type);

int main(int argc, char *argv[])
{
    if (argc < 4)
        printf("Usage: ./<programName> op1 op2 operator\n");
    else
    {
        int i, d;
        int c;

        while (--argc > 0 && (c = **++argv) != '\n')
        {
            i = 0;
            if (c == '+' || c == '-' || c == '*' || c == '/' || c == '=' || c == '\n')
            {
                if ((c == '+' || c == '-') && isdigit(d = *++(argv[0])))
                {
                    sign = (c == '-') ? -1 : 1;
                    c = d;
                    goto DOWN1;
                }
                else
                {
                    calc(c);
                    goto DOWN2; //To avoid re-executing calc(Number) which
                                //is outside any loop in main when operator
                                //is read and operation is performed.
                }
            }

DOWN1:      while (isdigit(c = *argv[0]))
            {
                s[i++] = c;
                c = *++(argv[0]);

                if (**argv == '.')
                {
                    s[i++] =  **argv;
                    while (isdigit(*++(argv[0])))
                        s[i++] = **argv;
                }
                s[i] = '\0';
            }
            calc(NUMBER);   //Outside while to get single push of s[]
                            //after reading the complete number
DOWN2:      ;
        }
    }
    return 0;
}

void push (double f);
double pop(void);

void calc (int type)
{
    double op2, res;
    switch(type)
    {
        case NUMBER:
            push(sign*atof(s));
            sign = 1;
            break;
        case '+':
            push(pop() + pop());
            break;
        case '-':
            op2 = pop();
            push(pop() - op2);
            break;
        case '*':
            push(pop() * pop());
            break;
        case '/':
            op2 = pop();
            push(pop() / op2);
            break;
        case '=':
            res = pop();
            push(res);
            printf("\t\t\t||Result = %lg||\n", res);
            break;
        case '\n':
            break;
        default:
            printf("\nError: Invalid Operator!\n");
            break;
    }
}

#define STACKSIZE 1000
double val[STACKSIZE];
int sp = 0;

void push(double f)
{
    if (sp >= STACKSIZE)
        printf("\nError: Stack Overflow!\n");
    else
        val[sp++] = f;
}

double pop(void)
{
    if (sp != 0)
    {
        double ret = val[--sp];
        return ret;
    }
    else
    {
        printf("\nError: Stack Empty!\n");
        return 0.0;
    }
}

Early returns are OK.

if (args < 4) {
    printf(....);
    return;
}
....

emphasizes where the business logic is.
The condition (c = **++argv) != '\n' looks sort of strange. It is indeed possible to embed a newline in an argument, but it doesn't warrant a special case. It is just one way to malform an argument, and there are plenty of them.
c = d; does nothing. The very first statement after goto DOWN1 overrides c.
Avoid gotos. I don't see the compelling reason to have them here. Just move the code under DOWN1 label to where it belongs, and see the gotos disappearing. Better yet, factor it out into a function.
There is no reason to copy the rest of the argument into s. You may directly pass it to atof. I understand the desire to sanitize the argument, but the way you do it is incorrect. It allows multiple dots, and misinterprets some well-formed floats (those with exponents, like 1e2). Let atof do its job correctly. Better yet, use strtod, and check where it stopped parsing.
Avoid globals. The bullet above eliminates s. To eliminate global sign, don't cramp everything to calc. Just compute the number, and push it. Let calc only deal with operators.
All error messages should go to stderr.


Handling thread interruptions in a directory iterator This code is code written on a whim to help $someindividual asking for such a thing on $somechatmedium; what is needed is a way, in Java, to provide an Iterator over Path objects which must only be directories, recursively, and where the number of returned instance number in some sufficient quantities to trigger an OutOfMemoryError if eagerly populated.

So, I came up with this (imports omitted for "brevity"):

public final class LazyDirectoryIterator
    implements Iterator<Path>, Closeable
{
    private static final int QUEUE_SIZE = 5;
    private static final long TIMEOUT_QUANTITY = 100L;
    private static final TimeUnit TIMEOUT_UNIT = TimeUnit.MILLISECONDS;

    private final ExecutorService executor
        = Executors.newSingleThreadExecutor();
    private final BlockingQueue<Path> queue
        = new ArrayBlockingQueue<>(QUEUE_SIZE);

    private Path nextPath;
    private boolean needNext = true;

    public LazyDirectoryIterator(final Path baseDir)
    {
        executor.submit(new Runnable()
        {
            @Override
            public void run()
            {
                try {
                    Files.walkFileTree(baseDir, new CustomVisitor());
                } catch (IOException ignored) {
                    Thread.currentThread().interrupt();
                }
            }
        });
    }

    @Override
    public boolean hasNext()
    {
        try {
            if (needNext) {
                nextPath = queue.poll(TIMEOUT_QUANTITY, TIMEOUT_UNIT);
                needNext = false;
            }
            return nextPath != null;
        } catch (InterruptedException ignored) {
            Thread.currentThread().interrupt();
            return false;
        }
    }

    @Override
    public Path next()
    {
        if (!hasNext())
            throw new NoSuchElementException();
        needNext = true;
        return nextPath;
    }

    @Override
    public void remove()
    {
        throw new UnsupportedOperationException();
    }

    @Override
    public void close()
        throws IOException
    {
        executor.shutdownNow();
    }

    private final class CustomVisitor
        extends SimpleFileVisitor<Path>
    {
        @Override
        public FileVisitResult preVisitDirectory(final Path dir,
            final BasicFileAttributes attrs)
            throws IOException
        {
            try {
                queue.put(dir);
                return FileVisitResult.CONTINUE;
            } catch (InterruptedException e) {
                Thread.currentThread().interrupt();
                throw new IOException("timeout", e);
            }
        }
    }
}

There are three places here where I purposely choose to set the interrupt flag:

in the Runnable walking the file tree, if I encounter an I/O exception (I'm really not sure about this one; but a Runnable can't throw);
in the FileVisitor itself, if I can't push onto the queue;
in .hasNext() if trying and retrieving an element from the queue times out.

It is expected that this Iterator will never run in more than one thread, which is why needNext and nextPath are not volatile. Note that in the three usages above, the two first are in another thread which is spawned from the constructor.

Note: up to Java 7, cannot use Java 8.

So, is my handling of InterruptedExceptions, and my uses of Thread.currentThread().interrupt() correct?

Overall your code looks fine to me, however some remarks:

I would log the IOExceptions that may happen during walkFileTree. Otherwise, if there are problems with the tree walk, it's hard to find out the reason later (e.g., if the baseDir does not exist). Perhaps you should perform also some sanity checks (i.e, baseDir is not null and baseDir exists) before submitting the Runnable.
When catching the IOException, it would be also possible to just throw a RuntimeException that has the IOException as its cause. But this is just a matter of taste.
However, in both cases if an IOException is thrown you need to be aware that the ExecutorService has still an idle thread (that may prevent the shutdown of your app) until you call the close method on LayzDirectoryIterator. Maybe using a custom ThreadFactory that creates daemon threads (and neat thread names) is an option here? I like to use Guava's ThreadFactoryBuilder for this purpose.
I'm generally not a fan of spawning a new Thread within the constructor, I would add an additional start method to submit the tree walk Runnable.
Your Thread.currentThread().interrupt() within the CustomVisitor is fine.


Tic-Tac-Toe code in JavaScript I have developed a tic-tac-toe game in native JavaScript.
Can you tell me how to optimize more and what are the missing things? JSBIN

The logic, modular approach, testable and scalable?

(function() {
  var playerOneData = {
    name: null,
    arr: []
  };
  var playerTwoData = {
    name: null,
    arr: []
  };
  winnerFound = false;
  var clickCount = 2;

  var start = document.getElementById('start');
  var resultsList = [
    [0, 1, 2],
    [3, 4, 5],
    [6, 7, 8],
    [0, 3, 6],
    [1, 4, 7],
    [2, 5, 8],
    [0, 4, 8],
    [2, 4, 6],
  ];
  var setApplicationView = function(screenId, display) {
    //     hide user infor form
    document.getElementById(screenId).style.display = display;
  };
  start.addEventListener('click', function() {
    //     fetch username
    playerOneData.name = document.getElementById('player-one-name').value;
    playerTwoData.name = document.getElementById('player-two-name').value;
    //     hide user info form
    setApplicationView('user', 'none');
    //     show game board
    setApplicationView('game-board', 'block');
    //     set the player's name
    document.querySelector('#p1-info span').innerHTML = playerOneData.name;
    document.querySelector('#p2-info span').innerHTML = playerTwoData.name;
  });

  var ifClicked = function(index, tile) {
    var i = 0;
    while (i < tile.length) {
      if (index == tile[i])
        return false;
      i++;
    }
    return true
  };
  var selectedTiles = [];

  var winnerFinder = function(playersData, resultsList) {
    if(playersData.arr.length<2) return true;
    var player = playersData.arr;
    var i = 0;
    resultsList.forEach(function(element) {
      i = 0;
      player.forEach(function(ele) {
        if (element.indexOf(ele) >= 0) {
          i++;
        }
      });
      if (i == element.length) {
        matrix.removeEventListener('click', startMatrix);
        document.getElementById('result').innerHTML = "Winner is " + playersData.name;
        winnerFound = true;
        return false;
      }
    });
  };
  var matrix = document.getElementById('matrix');
  var startMatrix = function($event) {
    var target = $event.target;
    var tileIndex = +target.getAttribute('tile-index');
    var alreadyClickedStatus = ifClicked(tileIndex, selectedTiles);
    if (target.className === 'tile' && alreadyClickedStatus === true) {
      selectedTiles.push(tileIndex);

      if (clickCount % 2 === 0) {
        playerOneData.arr.push(tileIndex);
        target.className = 'tile red';

        if (winnerFinder(playerOneData, resultsList) === false) {
          matrix.removeEventListener('click', startMatrix);
        }
      } else {
        playerTwoData.arr.push(tileIndex);
        target.className = 'tile blue';
        if (winnerFinder(playerTwoData, resultsList) === false) {
          matrix.removeEventListener('click', startMatrix);
        }
      }

      clickCount++;
      if (clickCount === 11 && winnerFound === false) {
        document.getElementById('result').innerHTML = 'Match drawn';
      }
    }
  }
  matrix.addEventListener('click', startMatrix);
  var resetBoard = function() {
    winnerFound = false;
    clickCount = 2;
    matrix.addEventListener('click', startMatrix);
    document.getElementById('result').innerHTML = '';
    selectedTiles = [];
    var tile = document.querySelectorAll('.tile')
    tile.forEach(function(ele, index) {
      tile[index].className = 'tile';
    });
    playerOneData.arr = [];
    playerTwoData.arr = [];
  };
  var restart = document.getElementById('restart');
  restart.addEventListener('click', function() {

    resetBoard();
  });
  var newGame = document.getElementById('new-game');
  newGame.addEventListener('click', function() {
    //     show user info form
    setApplicationView('user', 'block');
    //     hide game board
    setApplicationView('game-board', 'none');
    resetBoard();
  });
})();

Note, some of these require a build process

the iife should be added in the build process, it's distracting
The code doesn't expose anything. It's untestable.
var should be let or const
You can use arrow functions to turn some of the functions into one liners
Inconsistent formatting - I recommend getting a linter
setApplicationView - Should probably be replaced with show/hide functions. The current name is not very descriptive to me.
//     hide game board - You can make this a function and get rid of the comment. Several other similar issues.
ifClicked - A function name should generally describe what the functions does, not what happened when it was called. You could also just inline it.
winnerFinder - It would be so much nicer if this was called findWinner, and returned the result instead of storing it globally. Then you could test it as well.
As it stands, it actually returns false when winnerFound is true. Even more confusing.
winnerFinder - You can use player.filter here to get rid of the index variable
playerData.arr - What is arr?
var player = playersData.arr; - How can a player be an array?
document.getElementById - You can run all these calls at the top to shorten the code and get rid of unnecessary dom queries.
innerHTML - It's quite hard to tell how this whole thing works with view logic and game logic all mixed in together.
In general I would recommend separating the view logic from the game logic as much as possible, and even better would be to make the view logic declarative.


Querying MySQL from an external application I have a database that I need to query over and over as fast as possible. My queries execute pretty quickly, but there seems to be some additional lag. I have a feeling that this lag is due to the fact that I am initiating and de-initiating a connection the connection each time. Is there a way to avoid this?

I am not using libmysql (at least, not directly). I am using the "mysql50" package in Lazarus/FreePascal (similar to Delphi), which in turn uses libmysql (I think).

The purpose of this library is to pass along a query sent from MQL4 (a propitiatory C-like language for the financial exchange market), and return a single row from my MySQL database (to which it connects through a pipe).

I would really appreciate if someone took a look at my code and pointed out (or maybe even fixed) some inefficiencies.

{$CALLING STDCALL}

library D1Query;

{$mode objfpc}{$H+}

uses
  cmem,
  Windows,
  SysUtils,
  profs_win32exceptiontrap,
  mysql50;

var

  sock: PMYSQL;
  qmysql: st_mysql;

type
  VArray = array[0..100] of Double;
  PArray = ^VArray;

  procedure InitSQL; stdcall;
  begin

    mysql_init(PMySQL(@qmysql));
    sock :=
      mysql_real_connect(PMysql(@qmysql), '.', 'root', 'password', 'data', 3306, 'mysql', CLIENT_MULTI_STATEMENTS);
    if sock = nil then
    begin
      OutputDebugString(PChar('  Couldn''t connect to MySQL.'));
      OutputDebugString(PChar(mysql_error(@qmysql)));
      halt(1);
    end;

  end;

  procedure DeInitSQL; stdcall;
  begin

    mysql_close(sock);
  end;

  function SQL_Query(QRY: PChar; output: PArray): integer; stdcall;
  var
    rowbuf: MYSQL_ROW;
    recbuf: PMYSQL_RES;
    i: integer;
    nfields: LongWord;

  begin
    InitSQL();

    if (mysql_query(sock, QRY) < 0) then
    begin
      OutputDebugString(PChar('  Query failed '));
      OutputDebugString(PChar('   ' + mysql_error(sock)));
    end;

    recbuf := mysql_store_result(sock);
    nfields :=  mysql_num_fields(recbuf);
    rowbuf := mysql_fetch_row(recbuf);

    if (rowbuf <> nil) then
    begin
      for i:=0 to nfields-1 do
          output^[i] := StrToFloatDef(rowbuf[i], -666);
    end;

    mysql_free_result(recbuf);
    DeInitSQL();
    Result := i;

  end;

exports
  SQL_Query,
  InitSQL,
  DeInitSQL;

begin
end.

The most important thing here is not to create and destroy the connection as part of executing your query.  Force the calling code to create the connection first (or create it yourself if its not done) but dont destroy the connection until the library is unloaded.  That should improve speed.


Form validation for user account creation What would be a cleaner way to write the following?

// Check if add user was attempted
if ($params['submit']) {
    // Verify all fields are filled in
    if (Utilities::checkAllFieldsNotEmpty($params)) {
        // Make sure username is not taken
        if (Admin::checkUsername($params['username'])) {
            // Check if directory is created
            if (Admin::createUsersDirectory($params['username'])) {
                if (Admin::createNewUser($params['name'], $params['username'], $params['password'], $params['admin'])) {
                    Utilities::setMessage("Excellent!", "User was created successfully.", "admin_modal");
                }
            } else {
                Utilities::setMessage("Whoa!", "Directory creation failed.", "admin_modal");
            }
        } else {
            Utilities::setMessage("Whoa!", "Username in use.", "admin_modal");
        }
    } else {
        Utilities::setMessage("Whoa!", "Please fill in all fields", "admin_modal");
    }

    // Return admin view with message
    return $this->view->render($response, 'admin/admin.twig', [
        'name' => $user['name'],
        'message' => $_SESSION['message'],
        'form' => [
            'name' => $params['name'],
            'username' => $params['username'],
            'admin' => $params['admin']
        ]
    ]);
}

Don't nest your conditions like this.  Validate the input up front and fail out if need be.

For example that might look like this:

if(empty($params['submit'])) {
   // no parameters set
   // log error, throw exception, etc. as approrpiate
}
if(Utilities::checkAllFieldsNotEmpty($params) === false) {
   // fail out
   // not sure if you really need this condition if you validate each field
   // individually
}
// etc.

Then I would suggest that your Admin class should abstract away the details of user creation away from this script.  Ideally this script could just have something like:

try {
    $user = Admin::createNewUser(...);
} catch (Exception $e) {
    // do something to handle exception
}

And createNewUser() would go through all the logic of for validating user name, setting up directories, etc.  This code should know nothing about what is required to create a "user" outside of whatever information needs to be passed (i.e. user name).


LeetCode 894: All Possible Full Binary Trees I'm posting my code for a LeetCode problem copied here. If you would like to review, please do so. Thank you for your time!
Problem

A full binary tree is a binary tree where each node has exactly 0 or 2
children.
Return a list of all possible full binary trees with N nodes.  Each
element of the answer is the root node of one possible tree.
Each node of each tree in the answer must have node.val = 0.
You may return the final list of trees in any order.
Example 1:
Input: 7
Output: [
            [0,0,0,null,null,0,0,null,null,0,0],
            [0,0,0,null,null,0,0,0,0],
            [0,0,0,0,0,0,0],
            [0,0,0,0,0,null,null,null,null,0,0],
            [0,0,0,0,0,null,null,0,0]
        ]

Note:
1 <= N <= 20

Code
#include <vector>

class Solution {
public:
    static TreeNode *clone_tree(const TreeNode *node) {
        TreeNode *root = new TreeNode(0);
        root->left = (node->left) ? clone_tree(node->left) : nullptr;
        root->right = (node->right) ? clone_tree(node->right) : nullptr;
        return root;
    }

    static std::vector<TreeNode *> allPossibleFBT(const size_t n) {
        std::vector<TreeNode *> full_binary_trees;

        if (n == 1) {
            // full_binary_trees.push_back(new TreeNode(0));
            full_binary_trees.emplace_back(new TreeNode(0));

        } else if (n & 1) {
            for (size_t index = 2; index <= n; index += 2) {
                const auto left = allPossibleFBT(index - 1);
                const auto right = allPossibleFBT(n - index);

                for (size_t left_index = 0; left_index < left.size(); left_index++) {
                    for (size_t right_index = 0; right_index < right.size(); right_index++) {
                        full_binary_trees.emplace_back(new TreeNode(0));

                        if (right_index == right.size() - 1) {
                            full_binary_trees.back()->left = left[left_index];
                        } else {
                            full_binary_trees.back()->left = clone_tree(left[left_index]);
                        }

                        if (left_index == left.size() - 1) {
                            full_binary_trees.back()->right = right[right_index];
                        } else {
                            full_binary_trees.back()->right = clone_tree(right[right_index]);
                        }
                    }
                }
            }

        }

        return full_binary_trees;
    }
};

Problem

Solution

Discuss

Make clone_tree() private
Make helper functions that are not part of the public API private.
Avoid unnecessary nesting of statements if possible
In allPossibleFBT() there's a lot of indentation. There's a risk of the code running off the right hand side of the screen, making it hard to read. Try to reduce nesting if possible (but only if it improves readability and maintainability). In this case, you can get rid of the outermost if-statements by doing an early return if n is even, and then unconditionally adding the single-node tree:
static std::vector<TreeNode *> allPossibleFBT(const size_t n) {
    // Exit early if n is even
    if ((n & 1) == 0)
        return {};

    // Start with the one-node tree
    std::vector<TreeNode *> full_binary_trees{new TreeNode(0)};

    // Add more complex trees
    for (size_t index = 2; ...) {
        ...
    }

    return full_binary_trees;
}

Take a reference of an object if you are using it a lot
When we see a common expression being reused a lot, we can create a new variable holding the result of that expression. When the expression is actually an object, and we don't want to copy the object, we can still avoid repeating the expression by creating a reference to that object. So instead of explicitly writing full_binary_trees.back() many times, write:
auto &root = full_binary_trees.emplace_back(new TreeNode(0));

if (right_index == right_size() - 1) {
    root->left = ...

Since C++17, emplace_back() returns a reference to the emplaced element. In case you are writing code for older standards, you could write:
full_binary_trees.emplace_back(new TreeNode(0));
auto &root = full_binary_trees.back();
...

But perhaps it would even be better to first created the new tree, and then append it to the vector at the end:
auto root = new TreeNode(0);

if ...

full_binary_trees.emplace_back(root);

Other possible improvements
It would be nice to get rid of the inner-most if-statements, which is possible if you don't care about memory leaks:
for (auto left: allPossibleFBT(index - 1)) {
    for (auto right: allPossibleFBT(n - index)) {
        auto root = new TreeNode(0);
        root->left = clone_tree(left);
        root->right = clone_tree(right);
        full_binary_trees.emplace_back(root);
    }
}

In the context of this LeetCode problem, which doesn't state whether or not you are allowed to link a node into multiple trees, you might even get away with not calling clone_tree() at all, but just writing root->left = left; root->right = right.
Also note that this problem might benefit from memoization.


Find the longest path in a matrix where each step has entries that differ by 1 Given an N*N matrix that all numbers are distinct in it, the function should find the maximum length path (starting from any cell) such that all the cells along the path are in increasing order with a difference of 1.

It can move in 4 directions from the given cell (i, j), It can move to (i-1, j) or (i, j-1) or (i+1, j) or (i, j+1) with the condition that the adjacent cells have a difference of 1.

I wrote the following code that works in all the cases that I have checked.
I know that this is not optimal writing, with your permission, I would love to hear insights and ways to shorten the code. The program must be pure recursion without loops at all.

I'm not satisfied with the form and structure of my code because I went through the recursion step by step over all the options. I'm trying to write this in the form of backtracking with markings cells and stop conditions.

In the original question, the complexity of place, time, and the code are irrelevant.
At the moment, the discussion deals with the complexity of the code. I want to write it in a shorter and more elegant way while implementing backtracking.

public static int longestWorm(int[][] mat){
    return longestWorm(mat, 0,0,0);
}

private static int longestWorm(int[][] mat,int i, int j, int max){
    if (i == mat.length) return max;
    if (j == mat[i].length-1)
        return longestWorm(mat, i + 1, 0, max);
    if (wormCount(mat,i,j,0) > max)
        max = wormCount(mat,i,j,0);
    return longestWorm(mat, i, j + 1, max);
}
private static int wormCount(int[][] mat, int i, int j,int count){
    if(i < mat.length-1 && mat[i][j] == mat[i+1][j]+1)
        return 1+ wormCount(mat, i+1, j, count+1);
    if(j < mat[i].length-1 && mat[i][j] == mat[i][j+1]+1)
        return 1+ wormCount(mat, i, j+1, count+1);
    if(i > 0 && mat[i][j] == mat[i-1][j]+1)
        return 1+ wormCount(mat, i-1, j, count+1);
    if(j > 0 && mat[i][j] == mat[i][j-1]+1)
        return 1+ wormCount(mat, i, j-1, count+1);
    return count;
}

So a couple of remarks:

If I understand the required solution, you seem to have a bug, e.g. longestWorm(new int[][]{{0, 1, 3}, {9, 8 ,7}, {4, 5, 6}}) returns 10, although there are only 9 numbers. The wormcount calculation is then wrong because you increment the counter twice instead of once. Recursively defining the problem, you can say that the length of a path is the length of the current cell (which is one), summed by the length of the remaining path. Which would make the length function something like:

private static int wormCount(int[][] mat, int i, int j){
    if (i < mat.length - 1 && mat[i][j] == mat[i + 1][j] + 1) {
        return 1 + wormCount(mat, i + 1, j);
    }
    ....
    return 1;
}

This also makes the base case very clear. You also do not necessarily need to pass the count variable. Only perhaps if you would want to make it a tail-recursive function, but AFAIK in Java it will not help you eliminate a stack overflow.

The problem is defined as finding a path of increasing order, but your code seems to calculate it the other way around. Not wrong, but slightly confusing.
In the longestWorm function, you calculate the same function twice. This is not necessary:

max = Math.max(max, wormCount(mat, i, j, 0));

You could save it in a local variable if it helps the readeability.

I do not know the optimal way to solve this problem. My guess would be you could at each index calculate the path length, and save it in a table. Because each value is unique, it could simply be a map between the value and the length of the path starting at that value (else, it could simply be a key generated using the indices i and j). Before calculating the length for a particular value (or starting at a particular index), you first look if you already calculated it, which you could then reuse.


RegEx sentence-splitting madness. Mad genius? I inherited the following regex:
    /[^.!?\s][^.!?\n]*(?:[.!?](?!['"]?\s|$)[^.!?]*)*[.!?]?['"]?(?=\s|$)/g

This regex is used to split sentences, and is dealing with things like quotes after periods at the end of a sentence (He said "I don't care.") correctly. I've been playing with this regex, but I'm not sure yet if everything that's in here is needed. It seems to me that a lot of "groups" could be removed to achieve the same result.
Obviously documentation on this regex (one of the core aspects of the product) is missing.
This regex is madness. Is it just madness or is it mad genius?

Enlightenment.
Don't be impressed by a pattern. Format it to something readable:
[^.!?\s] [^.!?\n]*

(?:
    [.!?] (?! ['"]? \s | $ )
    [^.!?]*
)*

[.!?]? ['"]? (?= \s | $ )

There are three simple parts. No madness here, this pattern isn't complicated, but
before assuming parts are useless you have to fully understand what they do.

The first part [^.!?\s] [^.!?\n]* search a character that isn't a white-space or a punctuation mark, and then it consumes greedily all eventual following characters that aren't punctuation marks too or newlines.
Why newlines? This last point may seem surprising but is eventually related to the context: the way the text is formatted. In other words, sentences are perhaps on one line only.
Note that [^.!?\s] is the only thing that isn't optional in the pattern. Note also that since [^.!?\n]* is greedy, the next character after it can only be a punctuation mark, a newline or the end of the string.

The second part is an optional non-capturing group eventually repeated. Its goal is to accept punctuation marks that don't end the sentence, for instance things like:3.14, bestcatpics.maowmaow, bad enc?ding.
[.!?] should not be followed by a space or a "closing" quote and a space or the end of the string as the negative lookahead (?! ['"]? \s | $ ) expresses it. [^.!?]* will only reach the last position before the next punctuation mark (or the end of the string). Note that this time the newline isn't excluded: it's inconsistent with the first part.
The non-capturing group is repeated until there's no more "fake" punctuation mark.

The third and last part finishes the job consuming the punctuation mark that ends the sentence and the "closing" quote before a white-space or the end of the string. Punctuation mark and quote are optional.

Pattern Review.
From a technical point of view.
The pattern is well-designed to reduce the backtracking as possible: (A|B)* is unrolled to A* (BA*)*, where A is [^.!?] (with or without \n) and B is [.!?] with its lookahead assertion (?! ['"]? \s | $ ).
Writing ( [^.!?] | [.!?] (?! ['"]? \s | $ ) )* is shorter than [^.!?]* (?: [.!?] (?! ['"]? \s | $ ) [^.!?]* )* (about 9 chars) but is from far less efficient for a backtracking regex engine.
Other good point, all quantifiers are greedy and applied over restricted sets of characters, that is also more efficient than using reluctant quantifiers over permissive character classes (like the dot, [\s\S] or [^] in Javascript).
The only thing to deplore is the inconsistency between [^.!?\n] in the first part and [^.!?] in the second part. I really doubt there's a good reason behind. Also, (?=\s|$) is IMO better written like that: (?!\S) even if it looks less explicit.
About readability.
Most of regex engine provides a verbose mode that ignores white-spaces and inline comments in the pattern. Javascript doesn't have this feature yet, but for a long pattern like this one, nothing forbids to do that:
const pat = String.raw`
    [^.!?\s] [^.!?\n]* # start of the sentence

    (?: # "fake" punctuation mark
        [.!?] (?! ['"]? \s | $ )
        [^.!?]*
    )*

    [.!?]? ['"]? (?!\S) # end of the sentence`;

const re = RegExp.compile(pat.replace(/\s+|#.*/g, ''), 'g');

The approach.
This pattern may be the right solution in the specific context in which it is used. Nevertheless, and under no circumstances can it be considered a viable solution for isolating sentences from a plain text in natural language.
The analysis of natural language (NLP: Natural Language Processing) is a whole part of computing with these own tools like NLTK. While sentence breaking may seem like a basic task, it may require tools such as dictionaries, such as statistics (Machine Learning), and even, who would have thought, regex.


Stylistic exception and bang usage I'm not sure whether I should be throwing exceptions in the below model code (and rescuing in the controller - rescues not implemented below) or returning nil (in both or one of the exception cases). Also, should the model method be identify, or is identify proper?

My line of thinking is if the title is not set when identify is called, something is wrong or it is being used incorrectly.  Regarding the second exception, if there are no results, then the movie in question can not be scraped, thus making all operations later (which depend on the scraped data) useless.

I guess this comes down to return nil vs throw exception - I have been reading quite a few arguments on this subject and to me it seems proper to throw in both cases, thoughts?  Perhaps i'm missing a "rails way" of doing this too?  I'm fairly new to Rails.  If it matters, I am using Rails 3.2.

Movie model

validates :title, :presence => true

def identify
  raise "No Title" if self.title.nil?

  search_content = Nokogiri::HTML open("http://akas.imdb.com/find?q=#{CGI::escape(self.title)};s=tt")

  top_result = search_content.xpath("//div[@id='main']/p[1]/b/a").first

  raise "No Results" if top_result.nil?

  self.title = top_result.content

  self.imdb_id = top_result["href"].split("/")[2][2..-1].to_i

end

def populate_data
  identify if self.imdb_id.nil?

  # ...scrape imdb and save content to model attributes...
end

MovieController

def create
  @movie = Movie.new :title => params[:movie][:title]

  @movie.populate_data

  if @movie.save
    # ...the usual set flash, else recover kinda thing...

should the model method be identify! or is identify proper?

You should only end the method name with a bang (!) if the method modifies the object it is called on.  Furthermore, only if there is also another method of the same name without the bang which does not modify the object.  So no, you should not use a bang in your method name.

  return nil vs throw exception

I would take a cue from either ActiveRecord#find or #find_by_*.  The first will throw an exception when given an ID that doesn't exist.  The later will return nil.  Think of it as entering a URL in your browser as opposed to doing a Google search.  With the first, it's expected that you know the exact URL, with the later, it's not and you should get some sort of result or lack of result.  According to what you say, you should be going with throwing exceptions.

However, I would rethink how you're doing things.  I would do this with initialize instead of having to explicitly call populate_data.


Multilingual greeter object After coming across Douglas Crockford's views on class-free OOP, and doing away with new and this in Javascript I tried defining an object with its own variable and function, and returning it in JSON, as shown below:

var newObj = function (name, lang){
    var obj = {
            "name":name,
            "greeter":greeter
    };
    return obj;
};

function greeter (lang){
        switch (lang){
                case "en":return "hi";
                case "es":return "hola";
                default: return "hello";
        }
}

//and finally, use the object
var name = "john", lang = "es";
var obj = newObj(name, lang);

console.log(obj.name);
console.log(obj.greeter(lang));

I know this code works in NodeJS, but I wonder if it is actually a good idea to do it this way. More specifically, are there any implications/consequences to returning functions within a javascript object that I should be aware of?

This is not JSON. However JSON was inspired from Javascript Native way of creating object. This is why they're really similar.

However JSON handles only data, not functions.

Furthermore you can rewrite your function like this :

function newObj(name){
    this.name = name;
    return this;
};
newObj.prototype.greeter = function(lang){
    switch (lang){
            case "en":return "hi "+this.name;
            case "es":return "hola "+this.name;
            default: return "hello "+this.name;
    }
}

var obj = new newObj("name");
obj.greeter("de");

As you can see

I got rid of the 2nd parameter in the newObj function since it was not used.
I bind greeter to the Object so now I can use this.name in greeter to greet you by your name.
If you try to JSONify this, everything under prototype will be ignored so you won't have any error.