Minimax Tic-Tac-Toe implementation Notes:

I know \ are against PEP8; they are for readibility
I created this working through a tutorial
I forgot to add blank lines, so it may be a little squished

''' Tic-Tac-Toe Minimax Program by Peter'''

#Tutorial found at: http://giocc.com/concise-implementation-of-minimax-through-higher-order-functions.html'''
class GameState:
    def __init__(self,board):
        self.board = board
        self.winning_combos = [[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 is_gameover(self):
        '''returns if a game_state has been won or filled up'''
        if self.board.count('X') + self.board.count('O') == 9:
            return True
        for combo in self.winning_combos:
            if (self.board[combo[0]] == 'X' and self.board[combo[1]] == 'X' and self.board[combo[2]] == 'X') or \
            (self.board[combo[0]] == 'O' and self.board[combo[1]] == 'O' and self.board[combo[2]] == 'O'):
                return True
        return False
    def get_possible_moves(self):
        '''returns all possible squares to place a character'''
        squares = []
        for index, square in enumerate(self.board):
            if square != 'X' and square != 'O':
                squares.append(index)
        return squares
    def get_next_state(self, move, our_turn):
        '''returns the gamestate with the move filled in'''
        copy = self.board[:]
        if our_turn:
            copy[move] = 'X'
        else:
            copy[move] = 'O'
        return GameState(copy)
'''Named evals instead of eval to respect the eval function. http://stackoverflow.com/questions/9383740/what-does-pythons-eval-do'''
def evals(game_state):
    '''score a game_state from the computers point of view, 1 = win, 0 = tie, -1 = lose'''
    for combo in game_state.winning_combos:
        if game_state.board[combo[0]] == 'X' and game_state.board[combo[1]] == 'X' and game_state.board[combo[2]] == 'X':
            return 1
        elif game_state.board[combo[0]] == 'O' and game_state.board[combo[1]] == 'O' and game_state.board[combo[2]] == 'O':
            return -1
    else:
        return 0

'''max and min will call on each other recusively, until a terminal state'''
def max_play(game_state):
    '''if the game is over returns score, otherwise calls min_play on it's childen (possible moves from the state) and returns the maximum'''
    if game_state.is_gameover():
        return evals(game_state)
    return max(map(lambda move: min_play(game_state.get_next_state(move, True)), game_state.get_possible_moves()))
def min_play(game_state):
    '''if the game is over returns score, otherwise calls max_play on it's childen (possible moves from the state) and returns the minimum'''
    if game_state.is_gameover():
        return evals(game_state)
    return min(map(lambda move: max_play(game_state.get_next_state(move, False)), game_state.get_possible_moves()))

def minimax(game_state):
    '''returns the max of mapping the (move, score) tuple to the possible move using [1] of the tuple the (score)'''
    return max(map(lambda move: (move, min_play(game_state.get_next_state(move, True))), game_state.get_possible_moves()), key = lambda x: x[1])

def pretty_print(board):
    '''prints a list by 3 chars, joined by spaces'''
    print(' '.join(board[:3]))
    print(' '.join(board[3:6]))
    print(' '.join(board[6:9]))

'''starting board, assuming it is X's turn.'''
start_board = '_ _ _ ' + '_ O _ ' + '_ _ _'

'''Interpreting and printing start board'''
start_game_state = GameState(start_board.split(' '))
pretty_print(start_game_state.board)
'''Finding best possible move and score'''
move, score = minimax(start_game_state)
'''Displaying move and outcome'''
if score == 0:
    word = 'TIE'
elif score == 1:
    word = 'WIN'
else:
    word = 'LOSS, who rigged the board?!?'
print('X should go at index #',move, 'Which will always result in a ' + word)
start_game_state.board[move] = 'X'
pretty_print(start_game_state.board)

A couple minor comments:

start_board is defined as a string with spaces, then only used once with the spaces split. This means that you can completely remove the variable and do start_game_state = GameState(['_','_','_','O','_','_','_']), which the would be better formatted over multiple lines to make the layout clear:

GameState(['_','_','_',
           '_','O','_',
           '_','_','_'
         ])

The functions min_play, max_play, and minmax are near duplicates of eachother and should be merged into one with a second argument.
All three of those functions could be made clearer with a generator expression/list comprehension
Your comments should be written using the comment operator rather than the multi-line string operator, which should be used only for multiline comments and docstrings
The function get_next_state could be simplified using a ternary operator.
Get_possible_moves could also be simplified with a list comprehension.
Instead of checking if all nine squares are filled in is_gameover, you could check if no squares are free, which is faster as it only does one pass through the loop.
lambda x:x[1] should be replaced with operator.itemgetter(1), which is faster.
is_gameover and evals are near duplicates of eachother and should be merged

Fixed code:

#Tic-Tac-Toe Minimax Program by Peter

#Tutorial found at: http://giocc.com/concise-implementation-of-minimax-through-higher-order-functions.html
from operator import itemgetter

class GameState:
    def __init__(self,board):
        self.board = board
        self.winning_combos = [[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 get_winner(self):
        '''returns None if the game is still going, otherwise scores from computer's point of view (1=win, 0=tie, -1=win)'''
        if self.board.count('_') == 0:
            return 0
        for combo in self.winning_combos:
            if (self.board[combo[0]] == 'X' and self.board[combo[1]] == 'X' and self.board[combo[2]] == 'X'):
                return 1
            elif self.board[combo[0]] == 'O' and self.board[combo[1]] == 'O' and self.board[combo[2]] == 'O':
                return 0
        return None
    def get_possible_moves(self):
        '''returns all possible squares to place a character'''
        return [index for index, square in enumerate(self.board) if square == '_']
    def get_next_state(self, move, our_turn):
        '''returns the gamestate with the move filled in'''
        copy = self.board[:]
        copy[move] = 'X' if our_turn else 'O'
        return GameState(copy)

def play(game_state, our_turn):
    '''if the game is over returns (None, score), otherwise recurses to find the best move and returns it and the score.'''
    score = game_state.get_winner()
    if score != None:
        return None, score
    moves = ((move, play(game_state.get_next_state(move, our_turn), not our_turn)[1]) for move in game_state.get_possible_moves())
    return (max if our_turn else min)(moves, key=itemgetter(1))

def pretty_print(board):
    '''prints a list by 3 chars, joined by spaces'''
    print(' '.join(board[:3]))
    print(' '.join(board[3:6]))
    print(' '.join(board[6:9]))

#Interpreting and printing board
start_game_state = GameState(['_','_','_',
                              '_','O','_',
                              '_','_','_'
                            ])
pretty_print(start_game_state.board)
#Finding best possible move and score
move, score = play(start_game_state, True)
#Displaying move and outcome
if score == 0:
    word = 'TIE'
elif score == 1:
    word = 'WIN'
else:
    word = 'LOSS, who rigged the board?!?'
print('X should go at index #',move, 'Which will always result in a ' + word)
start_game_state.board[move] = 'X'
pretty_print(start_game_state.board)


Chat room using GenServer It seems odd that state must be passed around all the time. Can't State module hold itself, so I could do something like

state = %State{size: size}
case state.is_full? do
    ...

Here state is always passed. I suppose this is how it is in FP?

defmodule Chatter.Room do
  use GenServer

  defmodule State do
    defstruct size: 3, players: []

    def is_full?(state) do
      Enum.count(state.players) >= state.size
    end

    def add_player(state, player) do
      case State.is_full?(state) do
        true -> {:room_is_full, state}
        _    -> {:ok, append_player(state, player)}
      end
    end

    defp append_player(state, player) do
      %{state | players: state.players ++ [player]}
    end
  end

  def open(size) do
    {:ok, pid} = GenServer.start_link(__MODULE__, %State{size: size})
    pid
  end

  def join(room, user) do
    GenServer.call(room, {:join, user})
  end

  def players(room) do
    GenServer.call(room, :players)
  end

  def init(state) do
    {:ok, state}
  end

  def handle_call(:players, _from, state) do
    {:reply, state.players, state}
  end

  def handle_call({:join, player}, _from, state) do
    case State.add_player(state, player) do
      {:ok, state} -> {:reply, :ok, state}
      {error, state} -> {:reply, error, state}
    end
  end
end

Short answer -- yes, data including state is immutable.

In traditional functional programming idiom data is immutable.  With this in mind, GenServer stores the current state internally.  When you implement the callback functions (handle_call, handle_cast, ...) the current state is passed to your callback function.  You then return a new state which then replaces the existing state inside the GenServer.  Depending on the data structure used for maintaining your state, only a portion of the new data may be "deeply" copied.

If I relate this directly to your question, I would not implement state as you have.  The GenServer wraps the state for you.  A more erlang/elixirish outline would be something like:

defmodule ModuleName do
  # api functions
  # place calls to cast, call, here
  def is_full?(server) do
    GenServer.call(server, {:check_full})
  end

  # callbacks
  # match ":check_full" call
  def handle_call({:check_full}, _from, state) do
    # perform your full check actions here
  end

  # match an ":add_user" call
  def handle_call({:add_user, {user_data}}, _from, state) do
    # perform your add_user actions here
  end

  # catch all
  def handle_call(request, _from, state) do
    # do something here, if you want, or omit as suits your style
  end
end

Certainly, you can also use guards instead of matching or use one giant handle_call function with a case or cond to separate out the pieces.  The method used above is clean and easy to understand.

I may have missed the point of the question --- the short answer was, yes, we pass the state around.  The longer answer was the GenServer can keep track of it for you.


Java Swing Panel layout I am working on a Java project for college that involves us setting up a TCP Server and Client. I have that part working and now to add more of a feel to my project I want to add a GUI.

We have not begun learning about GUI's in Java yet. However I want to try as I think it would be a useful exercise. I have a very basic GUI set up and the appropriate ActionListener set for the button. My next problem is positioning my panels so they look neat and tidy on the Frame...

At the moment I have all the components in one panel as seen below:

public ClientGUI(){

    //Initialise Frame
    frame = new JFrame("TCP Client");

    //Initialise Panel 1 & Components
    p1 = new JPanel();

    //Set Layout
    p1.setLayout(new GridLayout(1,2));

    //Label 1 - For TextArea
    l1 = new JLabel("Chat Log");
    p1.add(l1);

    //TextArea - To display conversation
    t1 = new JTextArea(10,10);
    p1.add(t1);

    //Label 2 - For TextField
    l2 = new JLabel("Message");
    p1.add(l2);

    //Message Box - For user input
    t2 = new JTextField(10);
    p1.add(t2);

    //Button 1 - To send message
    b1 = new JButton("Send");
    p1.add(b1);

    //Add panels to frame
    frame.add(p1);

    //Frame properties...
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setSize(400,400);
    frame.setVisible(true);

    //Add Event listener to button
    b1.addActionListener(new ActionListener(){
        public void actionPerformed(ActionEvent ev){
            //do something
            t1.setText(t2.getText());
        }
    });

I have a wireframe drawn up of what I'd like it to look like but unfortunately I cannot upload as my reputation is not high enough. Basically something like this...

Label on top of a TextArea.
Label and TextField below.
Button below.

I'd appreciate any feedback anyone might have! Thanks very much.

What you want is called the BoxLayout, which feeds UI elements in columns or rows. And then you can nest them one inside another, e.g. have one horizontal box layout panel as an element in another that is vertical (kind of like nested HTML tables). Here is a pretty decent tutorial about layout managers and it includes the BoxLayout.


Criticize my Linked List implementation I'd appreciate any and all criticism about this code.  I didn't have a particular use case in mind when I wrote it; I just wanted to try implementing linked lists.  My comments are not very useful, I think.  I'm not a very experienced programmer, so there's probably plenty of things I could improve or fix here.

Here is the header file:

// llist.h
#ifndef LLIST_H_
#define LLIST_H_

struct LList;
typedef struct LList LList;

struct LLNode;
typedef struct LLNode LLNode;

struct LList {
    LLNode *front, *back;
};

struct LLNode {
    void *data;
    LLNode *next;
};

LList* llist_new(LList **self);
void llist_add(LList *self, void *v);
void llist_remove(LList *self, LLNode *node);
void llist_insert(LLNode *node, void *v);
void llist_destroy(LList *self);

#endif

Here is the .c file:

// llist.c
#include <assert.h>
#include <stdlib.h>

#include "llist.h"

// Initialize a new, empty LList, in dynamically allocated memory.
LList *llist_new(LList **self) {
    *self = malloc(sizeof (LList));
    if (self == NULL) { return NULL; }

    (*self)->front = NULL;
    (*self)->back = NULL;

    return *self;
}

// Append a value to the LList.  Creates a new node.
void llist_add(LList *self, void *v) {
    LLNode *node = malloc(sizeof (LLNode));
    assert(node != NULL);

    node->data = v;
    node->next = NULL;

    // Put this node at the back of the list.
    if (self->front != NULL) {
        self->back->next = node;
        self->back = node;
    } else {
        self->back = node;
        self->front = node;
    }
}

// Iterate over the LList to locate a node, and then remove it from the list.
void llist_remove(LList *self, LLNode *node) {
    LLNode *iter_node = self->front;
    LLNode *prev_node = NULL;

    while (iter_node != NULL) {
        if (iter_node == node) {
            if (prev_node != NULL) { prev_node->next = iter_node->next; }
            else { self->front = NULL; }
            free(iter_node);
            return;
        }
        prev_node = iter_node;
        iter_node = iter_node->next;
    }

}

// Insert a value after a particular node.
void llist_insert(LLNode *node, void *v) {
    LLNode *next = node->next;
    LLNode *new_node = malloc(sizeof (LLNode));

    new_node->data = v;
    new_node->next = next;

    node->next = new_node;
}

void llist_destroy(LList *self) {
    LLNode *iter_node = self->front;

    while (iter_node != NULL) {
        LLNode *next = iter_node->next;
        free(iter_node);
        iter_node = next;
    }

    free(self);
}

// Initialize a new, empty LList, in dynamically allocated memory.
LList *llist_new(LList **self) {

Its odd to return the new list as a return value and by reference.

    *self = malloc(sizeof (LList));
    if (self == NULL) { return NULL; }

Doing this check after you've already dereferenced self seems kinda pointless. Maybe you meant *self?

    (*self)->front = NULL;
    (*self)->back = NULL;

    return *self;
}

// Append a value to the LList.  Creates a new node.
void llist_add(LList *self, void *v) {
    LLNode *node = malloc(sizeof (LLNode));
    assert(node != NULL);

    node->data = v;
    node->next = NULL;

    // Put this node at the back of the list.
    if (self->front != NULL) {

I'd add a comment to explain what having self->front NULL means.

        self->back->next = node;
        self->back = node;
    } else {
        self->back = node;
        self->front = node;
    }
}

// Iterate over the LList to locate a node, and then remove it from the list.
void llist_remove(LList *self, LLNode *node) {
    LLNode *iter_node = self->front;
    LLNode *prev_node = NULL;

    while (iter_node != NULL) {
        if (iter_node == node) {
            if (prev_node != NULL) { prev_node->next = iter_node->next; }
            else { self->front = NULL; }

I think putting the blocks on the same line like this makes it harder to read. I also think it fail if you delete the first node.

            free(iter_node);
            return;
        }
        prev_node = iter_node;
        iter_node = iter_node->next;
    }

}

// Insert a value after a particular node.
void llist_insert(LLNode *node, void *v) {
    LLNode *next = node->next;
    LLNode *new_node = malloc(sizeof (LLNode));

    new_node->data = v;
    new_node->next = next;

Why did you have the next local variable. It doesn't seem to have helped make the code clearer.

    node->next = new_node;
}

void llist_destroy(LList *self) {
    LLNode *iter_node = self->front;

    while (iter_node != NULL) {
        LLNode *next = iter_node->next;
        free(iter_node);
        iter_node = next;
    }

    free(self);
}


Creates a class that searches for the elements in a list that contain a sequence Not sure what I should write here. The code should be self-explanatory.

"""This is a data definition class--Searchable_list.
Searchable_list takes a list of strings and makes it searchable.
Searchable meaning you can find which elements in the list have a pattern.
"""
class Searchable_list(object):
    """this will make your word list searchable.
Note, It will also loose the original order of the list."""
    def __init__(self, lis):
        assert hasattr(lis,"__iter__")
        self.search_dict=dict()
        for word in set(lis):self.add_word(word)

    def add_word(self,word):
        """this will add a word to the search_dict
search dict is of the form: {letter:{nextletter:{(index,word)}}}
"""
        assert type(word) is str#or isinstance(word,str)
        for index,val in enumerate(word[:-1]):
            next_letter=self.search_dict.setdefault(val,dict())
            words_list=next_letter.setdefault(word[index+1],set())#object modification
            words_list.add((index,word))#object modifification

    def find_matches(self,seq):
        """finds all the words in the list with this sequence.
Uses '.' as wildcard.
"""
        s_d=self.search_dict
        assert len(seq)>1
        #could put a try catch to catch key errors
        for index,letter in enumerate(seq[:-1]):
            if not(letter=="."and seq[index+1]=="."):
                #no point if they all match...
                if letter==".":
                    L_m=set.union(*(i.get(seq[index+1],set()) for i in s_d.values()))
                    #.get is important here. not all is have i[seq[index+1]]
                elif seq[index+1]==".":
                    L_m=set.union(*(i for i in s_d[letter].values()))
                else:
                    L_m=s_d[letter].get(seq[index+1],{})#this is a set.
                #L_m==letter_matches
                if index>0:
                    m_m=((i-index,word) for i,word in L_m)
                    #m_m=matches_matches. These words still have the pattern.
                    #your matching all indexes to the original m_s
                    m_s.intersection_update(m_m)
                    #m_s=matches_set
                else:
                    m_s=L_m.copy()
                    #http://stackoverflow.com/questions/23200969/how-to-clone-or-copy-a-set-in-python
        return m_s

EDIT: Because this post was bumped, and I've added some pretty major improvements to it here's a link to the final version of this. It's not very clean, but it has optimizations this doesn't have. If there's an interest I can try to put some comments explaining the optimizations in the github or post something here about it (probably in the form of another answer). https://github.com/user-name-is-taken/words-with-friends/blob/master/WWF_DDC.py (note, the scrabble stuff is just adapting this code for scrabble). For now, the basic idea behind the optimizations is that python's set.intersection is faster than set.union

naming

Based on reading your comment, I propose this name:

class SearchableCollection(object):

Rely on duck typing, rather than trying to write java code in python:

def __init__(self, words):
    # (delete this line) assert hasattr(lis,"__iter__")
    self.words = {}
    for word in words:
        self._add_word(word)

It appears that add_word is not part of your public API. Mark it so with a leading underscore, or make it a nested def. Do not assert that type is str.

    for index,val in enumerate(word[:-1]):

Please name it letter rather than the very vague val. Or cur_letter, parallel with next_letter.

You are using setdefault() in a sensible way. But you might be happier using defaultdict.

You named it words_list, but apparently you meant words_set.

style

Run $ flake8 WWF_DDC.py and follow its advice, please.


Fizzbuzz in COBOL, Verbose dinosaur style I was long a professional in COBOL, but noone ever dared comment my style. As the FizzBuzz seems fashion here, I made one. Verbose, of course, we're speaking about COBOL. I know I could have made in 15 lines, but decided instead to provide the full structure for a production program. Result makes 87 lines.

I'll be happy to read your thoughts, comments, critics, or executions. Made on OpenCOBOLIDE.

  ******************************************************************
  * Author:Gazzz0X2Z                                              *
  * Date:2015, Dec the 12th                                      *
  * Purpose:FizzBuzz, what else?                                 *
  * Tectonics: cobc                                               *
  ******************************************************************
   IDENTIFICATION DIVISION.
  *-*-*-*-*-*-*-*-*-*-*-*-*-
   PROGRAM-ID. FIZZBUZZ.
  *-*-*-*-*-*-*-*-*-*-*--
   ENVIRONMENT DIVISION.
  *-*-*-*-*-*-*-*-*-*-*--
   CONFIGURATION SECTION.
  *----------------------
   INPUT-OUTPUT SECTION.
  *----------------------
   DATA DIVISION.
  *-*-*-*-*-*-*-*-
   FILE SECTION.
  *-------------------------
   WORKING-STORAGE SECTION.
  *-------------------------
   01  WS-DATA.
       05  WS-COUNTS.
           10  WS-FIRST        PIC 9(4)    VALUE ZERO.
           10  WS-LAST         PIC 9(4)    VALUE 20.
           10  WS-POSN         PIC 9(4).
       05  WS-RESULTS.
           10  WS-RS00         PIC 9(4).
           10  WS-RM05         PIC 9(4).
               88  WS-ML05                 VALUE ZERO.
           10  WS-RM03         PIC 9(4).
               88  WS-ML03                 VALUE ZERO.
       05  WS-DISPLAY-STRING   PIC X(10).
       05  WS-DISPLAY-NUM REDEFINES WS-DISPLAY-STRING.
           10  WS-FIZZBUZZ-PRI PIC X(3).
           10  WS-FIZZBUZZ-INT PIC Z(3)9.
           10  WS-FIZZBUZZ-SUI PIC X(3).
       05  WS-DISPLAY-ALPHA REDEFINES WS-DISPLAY-STRING.
           10  WS-FIZZBUZZ-STR PIC X(10).
               88  WS-IS-OTHER             VALUE "*  0000  *".
               88  WS-IS-FIZZ              VALUE "*  FIZZ  *".
               88  WS-IS-BUZZ              VALUE "*  BUZZ  *".
               88  WS-IS-FIZZBUZZ          VALUE "*FIZZBUZZ*".

  *-*-*-*-*-*-*-*-*-*-*
   PROCEDURE DIVISION.
  *-*-*-*-*-*-*-*-*-*-*
   MAIN-PROCEDURE.
        PERFORM    10000-BEGIN
        PERFORM    20000-MAIN
           VARYING WS-POSN FROM WS-FIRST BY 1
             UNTIL WS-POSN > WS-LAST
        PERFORM    30000-END
        .
        STOP RUN.
  *
   10000-BEGIN.
       DISPLAY "**********"
       .
  *
   20000-MAIN.
       PERFORM 40000-COMPUTE-FIZZBUZZ
       DISPLAY WS-DISPLAY-STRING
       .
  *
   30000-END.
       DISPLAY "**********"
       .
  *
   40000-COMPUTE-FIZZBUZZ.
       DIVIDE WS-POSN BY 5 GIVING WS-RS00 REMAINDER WS-RM05
       DIVIDE WS-POSN BY 3 GIVING WS-RS00 REMAINDER WS-RM03
       EVALUATE TRUE
           WHEN (WS-ML05 AND WS-ML03)
               SET WS-IS-FIZZBUZZ  TO TRUE
           WHEN WS-ML05
               SET WS-IS-BUZZ      TO TRUE
           WHEN WS-ML03
               SET WS-IS-FIZZ      TO TRUE
           WHEN OTHER
               SET WS-IS-OTHER     TO TRUE
               MOVE WS-POSN        TO WS-FIZZBUZZ-INT
       END-EVALUATE
       .
  *
   END PROGRAM FIZZBUZZ.

Result :

**********
*FIZZBUZZ*
*     1  *
*     2  *
*  FIZZ  *
*     4  *
*  BUZZ  *
*  FIZZ  *
*     7  *
*     8  *
*  FIZZ  *
*  BUZZ  *
*    11  *
*  FIZZ  *
*    13  *
*    14  *
*FIZZBUZZ*
*    16  *
*    17  *
*  FIZZ  *
*    19  *
*  BUZZ  *
**********

As I'm sure you are aware, COBOL tends to be written one way at one site, another way at another.

Often things like name prefixes, and paragraph/SECTION prefixes cannot be other than dictated. Same with case of identifiers.

I dislike WSnn- (drives people nuts trying to insert numbers so that new data can be located rationally rather than in the order of arrival in specification) and WS- (similar for the LINKAGE SECTION) but am happy with W-/L-/LS- (or nothing). Absolutely hate single-word identifiers. Even these days with forewarning of what may appear later, it is plain dumb to have them there waiting for some compiler-change to make them invalid.

Also, since COBOL has no concept of "main", it is something to avoid pretending exists.

For paragraphs, or SECTIONS, I go with two numerics. Then with each subsequent level, append an alpha, A, D, G, J, M etc (allowing for insertions), and keep all related paragraphs/SECTIONS together, physically. Your program only has one level, so it doesn't really matter what you do in that one.

Nicely laid-out. I tend to be more extreme, but the effort you've invested impresses the reader (if they are a programmer, rather than a 9-5-er).

Hate the comments.

I'd remove all the redundant code (you don't need an ENVIRONMENT DIVISION, for instance).

Ah, for these last two, they have been generated for you, so I'd suggest copying an existing model program rather than using the OpenCOBOLIDE bolierplate.

I only use VARYING with things that, well, vary. So I'd use TIMES. Describe the situation better. You have to do more typing, but in addition to the better description of the data, your code will be faster than using VARYING.

Forget ideas like using REM. COBOL has done remainders since day one, a considerable time ago, so why (I don't know the answer to this) is there a function for it? In Enterprise COBOL (IBM Mainframe) REM would return a floating-point item, then converted to what you define in your program. Wow, do we need that?

I would work on the data-names a bit, but they are better than VAR1, VAR2 (or WS13-VAR1 and WS34-VAR2).

Here's an example of how I like to format a program:

http://ibmmainframes.com/viewtopic.php?p=278927#278927

And since that is a link, here's the code:

   IDENTIFICATION DIVISION.
   PROGRAM-ID. BBHEXD.
   ENVIRONMENT DIVISION.
   DATA DIVISION.
   WORKING-STORAGE SECTION.
   01  W-WHEN-COMPILED                      PIC X(8)BX(8).

   01  W-DISPLAY-HEX-MAX-UIND      USAGE IS INDEX.
   01  W-FIRST-TIME-FLAG                    PIC X VALUE "Y".
       88  W-FIRST-TIME-IN-PROGRAM          VALUE "Y".
       88  W-NOT-FIRST-TIME-FLAG            VALUE "N".

   01  W-WORK-BYTE-FOR-CALC         COMP    PIC S9(4) VALUE ZERO.
   01  FILLER REDEFINES W-WORK-BYTE-FOR-CALC.
       05  FILLER                           PIC X.
       05  W-WORK-BYTE                      PIC X.

   01  W-LEFT-HALF-BYTE-NUM         COMP    PIC S9(4).
   01  W-RIGHT-HALF-BYTE-NUM        COMP    PIC S9(4).
   01  W-BYTES-TO-OUTPUT            COMP    PIC S9(4).
   01  W-BYTES-MOVED                COMP    PIC S9(4).

   01  W-HEX-ALPHA-SUB              COMP    PIC S9(4).
   01  W-HEX-ALPHA-TABLE VALUE "ABCDEF".
       05  FILLER OCCURS 6 TIMES
            INDEXED BY W-HEX-ALPHA-IDX.
           10  W-HEX-ALPHA                  PIC X.

   01  W-DISPLAY-HEX.
       05  W-LEFT-HEX                       PIC X.
       05  W-RIGHT-HEX                      PIC X.

   01  W-HEX-TO-DISPLAY-LEFT-TABLE VALUE SPACE.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-BYTES-TO-OUTPUT
           INDEXED BY W-DISPLAY-HEX-LEFT-IND.
           10  W-HEX-TO-DISPLAY-LEFT        PIC X.
               88  W-HEX-TO-DISPLAY-LEFT-IS-EMPTY
                                            VALUE SPACE.
   01  W-HEX-TO-DISPLAY-RIGHT-TABLE VALUE SPACE.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-BYTES-TO-OUTPUT
           INDEXED BY W-DISPLAY-HEX-RIGHT-IND.
           10  W-HEX-TO-DISPLAY-RIGHT       PIC X.

   01  W-BYTE-TO-DISPLAY-TABLE VALUE SPACE.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-BYTES-TO-OUTPUT
           INDEXED BY W-DISPLAY-BYTE-IND.
           10  W-BYTE-TO-DISPLAY            PIC X.
   01  W-CONVERT-TO-HEX-FROM-DIGIT   COMP   PIC S9(4).

   01  W-CONVERTED-HEX-DIGIT                PIC X.
   01  W-CONVERTED-HEX-DIGIT-NUM
         REDEFINES W-CONVERTED-HEX-DIGIT    PIC 9.

   01  W-CONVERTED-HEX-DIGIT-NUM4           PIC 9(4).
   01  FILLER REDEFINES W-CONVERTED-HEX-DIGIT-NUM4.
       05  FILLER                           PIC XXX.
       05  W-CONVERTED-HEX-DIGIT-NUM4L      PIC 9.

   01  W-COLS-LINE.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-BYTES-TO-OUTPUT.
           10  FILLER                       PIC X.

   01  W-COLS-TABLE.
       05  FILLER                           PIC X(10) VALUE
           "----+----1".
       05  FILLER                           PIC X(10) VALUE
           "----+----2".
       05  FILLER                           PIC X(10) VALUE
           "----+----3".
       05  FILLER                           PIC X(10) VALUE
           "----+----4".
       05  FILLER                           PIC X(10) VALUE
           "----+----5".
       05  FILLER                           PIC X(10) VALUE
           "----+----6".
       05  FILLER                           PIC X(10) VALUE
           "----+----7".
       05  FILLER                           PIC X(10) VALUE
           "----+----8".
       05  FILLER                           PIC X(10) VALUE
           "----+----9".
       05  FILLER                           PIC X(10) VALUE
           "----+----0".
       05  FILLER                           PIC X(10) VALUE
           "----+----1".
       05  FILLER                           PIC X(10) VALUE
           "----+----2".
       05  FILLER                           PIC X(10) VALUE
           "----+----3".
       05  FILLER                           PIC X(02) VALUE
           "--".

   LINKAGE SECTION.

   01  L-DATA-LENGTH COMP PIC S9(8).

   01  L-BBHEXD-CONTROL-BLOCK.
       05  L-BBHEXD-CB-MAX-LINE         COMP PIC S9(4) VALUE +80.
           88  L-BBHEXD-CB-MAX-LINE-VALID    VALUE +80 +132.
       05  L-BBHEXD-CB-CHUNK-LENGTH     COMP PIC S9(4) VALUE ZERO.
           88  L-BBHEXD-CB-CHUNK-LENGTH-ZERO VALUE ZERO.
       05  FILLER                            PIC X(96) VALUE SPACE.

   01  L-DATA-PICX1-FOR-USING PIC X.

   01  L-DATA-TO-DISPLAY REDEFINES L-DATA-PICX1-FOR-USING.
       05  FILLER OCCURS 99999999 TIMES
             INDEXED BY L-DTD-BYTE-FOR-C-AND-C-IDX.
           10  L-DTD-BYTE-FOR-CONV-AND-COPY PIC X.

   PROCEDURE DIVISION USING
                                    L-DATA-PICX1-FOR-USING
                                    L-DATA-LENGTH
                                    L-BBHEXD-CONTROL-BLOCK
                                    .
   00-CONTROL SECTION.

       PERFORM                      10-FIRST-TIME
       PERFORM                      30-CNV-LK-DATA-TO-HEX-AND-DISP
       GOBACK
       .
   10-FIRST-TIME SECTION.

       IF W-FIRST-TIME-IN-PROGRAM
           PERFORM                  10A-SAY-WHO-WE-ARE
           SET W-NOT-FIRST-TIME-FLAG
                                    TO TRUE
       END-IF

       PERFORM                      10D-CHECK-OUTPUT-LENGTH
       PERFORM                      10G-SET-OUTPUT-LNGTH-AND-CHUNK
       .
   10A-SAY-WHO-WE-ARE SECTION.

       MOVE WHEN-COMPILED           TO W-WHEN-COMPILED
       DISPLAY "BBHEXD This program compiled on "
                                    W-WHEN-COMPILED
       .
   10D-CHECK-OUTPUT-LENGTH SECTION.

       IF ( NOT L-BBHEXD-CB-MAX-LINE-VALID )
           DISPLAY "BBHEXD maximum line length should be 80 or 132"
           DISPLAY "BBHEXD found>" L-BBHEXD-CB-MAX-LINE "<"
           CALL "FOODUMP"
       END-IF
       .
   10G-SET-OUTPUT-LNGTH-AND-CHUNK SECTION.

       IF L-BBHEXD-CB-CHUNK-LENGTH-ZERO
           MOVE L-BBHEXD-CB-MAX-LINE
                                    TO W-BYTES-TO-OUTPUT
       ELSE
           IF ( L-BBHEXD-CB-CHUNK-LENGTH
               GREATER THAN L-BBHEXD-CB-MAX-LINE )
               DISPLAY "BBHEXD Sorry, chunks ignored, too big"
               DISPLAY "BBHEXD Chunks>" L-BBHEXD-CB-CHUNK-LENGTH "<"
               DISPLAY "BBHEXD Maxlen>" L-BBHEXD-CB-MAX-LINE "<"
               MOVE L-BBHEXD-CB-MAX-LINE
                                    TO W-BYTES-TO-OUTPUT
           ELSE
               MOVE L-BBHEXD-CB-CHUNK-LENGTH
                                    TO W-BYTES-TO-OUTPUT
           END-IF
       END-IF

       SET W-DISPLAY-BYTE-IND       TO W-BYTES-TO-OUTPUT
       SET W-DISPLAY-HEX-MAX-UIND   TO W-DISPLAY-BYTE-IND

       IF L-DATA-LENGTH EQUAL TO ZERO
           DISPLAY "BBHEXD You want to display zero bytes?"
           DISPLAY "BBHEXD Do not expect to see them"
       END-IF
       .
   30-CNV-LK-DATA-TO-HEX-AND-DISP SECTION.

       SET W-DISPLAY-HEX-LEFT-IND
           W-DISPLAY-HEX-RIGHT-IND
           W-DISPLAY-BYTE-IND       TO +1
       MOVE ZERO                    TO W-BYTES-MOVED

       PERFORM                      30A-BYTE-BY-BYTE-CONVERT
         VARYING                    L-DTD-BYTE-FOR-C-AND-C-IDX
         FROM                       1
         BY                         1
         UNTIL                      L-DTD-BYTE-FOR-C-AND-C-IDX
           GREATER THAN             L-DATA-LENGTH

       IF ( W-BYTES-MOVED NOT EQUAL TO ZERO )
           MOVE W-BYTES-MOVED       TO W-BYTES-TO-OUTPUT
           PERFORM                  99A-DISPLAY-PART-OF-OUTPUT
       END-IF

       .
   30A-BYTE-BY-BYTE-CONVERT SECTION.

       MOVE L-DTD-BYTE-FOR-CONV-AND-COPY
             ( L-DTD-BYTE-FOR-C-AND-C-IDX )
                                    TO W-WORK-BYTE

       DIVIDE W-WORK-BYTE-FOR-CALC  BY 16
         GIVING                     W-LEFT-HALF-BYTE-NUM
         REMAINDER                  W-RIGHT-HALF-BYTE-NUM

       MOVE W-LEFT-HALF-BYTE-NUM    TO W-CONVERT-TO-HEX-FROM-DIGIT
       PERFORM                      30AA-CONVERT-HALF-BYTE
       MOVE W-CONVERTED-HEX-DIGIT   TO W-LEFT-HEX
       MOVE W-RIGHT-HALF-BYTE-NUM   TO W-CONVERT-TO-HEX-FROM-DIGIT
       PERFORM                      30AA-CONVERT-HALF-BYTE
       MOVE W-CONVERTED-HEX-DIGIT   TO W-RIGHT-HEX

       IF ( W-DISPLAY-HEX-LEFT-IND
           GREATER THAN W-DISPLAY-HEX-MAX-UIND )
           PERFORM                  99A-DISPLAY-PART-OF-OUTPUT
           SET W-DISPLAY-HEX-LEFT-IND
               W-DISPLAY-HEX-RIGHT-IND
               W-DISPLAY-BYTE-IND
                                    TO +1
       END-IF

       MOVE W-LEFT-HEX              TO W-HEX-TO-DISPLAY-LEFT
                                        ( W-DISPLAY-HEX-LEFT-IND )
       MOVE W-RIGHT-HEX             TO W-HEX-TO-DISPLAY-RIGHT
                                        ( W-DISPLAY-HEX-RIGHT-IND )
       MOVE W-WORK-BYTE             TO W-BYTE-TO-DISPLAY
                                        ( W-DISPLAY-BYTE-IND )
       ADD +1                       TO W-BYTES-MOVED
       SET W-DISPLAY-HEX-LEFT-IND
           W-DISPLAY-HEX-RIGHT-IND
           W-DISPLAY-BYTE-IND       UP BY +1
       .
   30AA-CONVERT-HALF-BYTE SECTION.

       IF ( W-CONVERT-TO-HEX-FROM-DIGIT GREATER THAN 9 )
           SUBTRACT 9               FROM W-CONVERT-TO-HEX-FROM-DIGIT
             GIVING                 W-HEX-ALPHA-SUB
           SET W-HEX-ALPHA-IDX      TO W-HEX-ALPHA-SUB
           MOVE W-HEX-ALPHA ( W-HEX-ALPHA-IDX )
                                    TO W-CONVERTED-HEX-DIGIT
       ELSE
           MOVE W-CONVERT-TO-HEX-FROM-DIGIT
                                    TO W-CONVERTED-HEX-DIGIT-NUM4
           MOVE W-CONVERTED-HEX-DIGIT-NUM4L
                                    TO W-CONVERTED-HEX-DIGIT-NUM
       END-IF

       .
   99A-DISPLAY-PART-OF-OUTPUT SECTION.

       MOVE W-COLS-TABLE            TO W-COLS-LINE
       DISPLAY                      W-COLS-LINE

       DISPLAY                      W-BYTE-TO-DISPLAY-TABLE
       DISPLAY                      W-HEX-TO-DISPLAY-LEFT-TABLE
       DISPLAY                      W-HEX-TO-DISPLAY-RIGHT-TABLE
       MOVE SPACE                   TO W-BYTE-TO-DISPLAY-TABLE
                                       W-HEX-TO-DISPLAY-LEFT-TABLE
                                       W-HEX-TO-DISPLAY-RIGHT-TABLE
       MOVE ZERO                    TO W-BYTES-MOVED

       .

To demonstrate the usefulness of meaningful names, here's a version with the names rubbished:

   IDENTIFICATION DIVISION.
   PROGRAM-ID. UGLY.
   ENVIRONMENT DIVISION.
   DATA DIVISION.
   WORKING-STORAGE SECTION.
   01  W-A                      PIC X(8)BX(8).

   01  W-B            USAGE IS INDEX.
   01  W-C          USAGE IS INDEX.

   01  W-D              PIC S9(9).
   01  FILLER REDEFINES W-D.
       05  FILLER                           PIC X(5).
       05  W-E         PIC 9(4).
   01  W-F PIC X VALUE "Y".
       88  W-F-Y VALUE "Y".
       88  W-F-N VALUE "N".

   01  W-G         COMP    PIC S9(4) VALUE ZERO.
   01  FILLER REDEFINES W-G.
       05  FILLER                           PIC X.
       05  W-H                      PIC X.

   01  W-I         COMP    PIC S9(4).
   01  W-J        COMP    PIC S9(4).
   01  W-K            COMP    PIC S9(4).

   01  W-L              COMP    PIC S9(4).
   01  W-M VALUE "ABCDEF".
       05  FILLER OCCURS 6 TIMES
            INDEXED BY I.
           10  W-N                  PIC X.

   01  W-O.
       05  W-P                       PIC X.
       05  W-Q                      PIC X.

   01  W-R.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-K
           INDEXED BY J.
           10  W-S        PIC X.
               88  W-S-Y
                                            VALUE SPACE.
   01  W-T.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-K
           INDEXED BY K.
           10  W-U       PIC X.

   01  W-V.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-K
           INDEXED BY L.
           10  W-W            PIC X.
   01  W-X   COMP   PIC S9(4).

   01  W-Y                PIC X.
   01  W-Z
         REDEFINES W-Y    PIC 9.

   01  W-AA           PIC 9(4).
   01  FILLER REDEFINES W-AA.
       05  FILLER                           PIC XXX.
       05  W-AB      PIC 9.

   01  W-AC.
       05  FILLER OCCURS 1 TO 132 TIMES
           DEPENDING ON W-K.
           10  FILLER                       PIC X.

   01  W-AD.
       05  FILLER                           PIC X(10) VALUE
           "----+----1".
       05  FILLER                           PIC X(10) VALUE
           "----+----2".
       05  FILLER                           PIC X(10) VALUE
           "----+----3".
       05  FILLER                           PIC X(10) VALUE
           "----+----4".
       05  FILLER                           PIC X(10) VALUE
           "----+----5".
       05  FILLER                           PIC X(10) VALUE
           "----+----6".
       05  FILLER                           PIC X(10) VALUE
           "----+----7".
       05  FILLER                           PIC X(10) VALUE
           "----+----8".
       05  FILLER                           PIC X(10) VALUE
           "----+----9".
       05  FILLER                           PIC X(10) VALUE
           "----+----0".
       05  FILLER                           PIC X(10) VALUE
           "----+----1".
       05  FILLER                           PIC X(10) VALUE
           "----+----2".
       05  FILLER                           PIC X(10) VALUE
           "----+----3".
       05  FILLER                           PIC X(02) VALUE
           "--".

   LINKAGE SECTION.

   01  L-A PIC X.

   01  L-B REDEFINES L-A.
       05  FILLER OCCURS 99999999 TIMES
             INDEXED BY M.
           10  L-C PIC X.

   01  L-D COMP PIC S9(8).

   01  L-E.
       05  L-F         COMP PIC S9(4) VALUE +80.
           88  L-F-VALID    VALUE +80 +132.
       05  L-G     COMP PIC S9(4) VALUE ZERO.
           88  L-G-Y VALUE ZERO.
       05  FILLER                            PIC X(96) VALUE SPACE.

   PROCEDURE DIVISION USING         L-A
                                    L-D
                                    L-E
                                    .
   00-A SECTION.
       IF W-F-Y
           MOVE WHEN-COMPILED       TO W-A
           DISPLAY                  "UGLY program compiled on "
                                    W-A
           SET W-F-N                TO TRUE
       END-IF

       IF ( NOT L-F-VALID )
           DISPLAY "UGLY maximum line length should be 80 or 132"
           DISPLAY "UGLY found>" L-F "<"
           CALL "FOODUMP"
       END-IF
       IF L-G-Y
           MOVE L-F                 TO W-K
       ELSE
           IF ( L-G
               GREATER THAN L-F )
               DISPLAY "UGLY Sorry, chunks ignored, too big"
               DISPLAY "UGLY Chunks>" L-G "<"
               DISPLAY "UGLY Maxlen>" L-F "<"
               MOVE L-F             TO W-K
           ELSE
               MOVE L-G             TO W-K
           END-IF
       END-IF
       SET L                        TO W-K
       SET W-B                      TO L
       SET J
           K
           L                       TO ZERO

       PERFORM                      00-B
         VARYING                    M
         FROM                       1
         BY                         1
         UNTIL                      M
           GREATER THAN             L-D

       IF ( NOT W-S-Y )
           SET W-C                  TO J
           MOVE W-C                 TO W-D
           MOVE W-E                 TO W-K
           PERFORM                  00-D
       END-IF

       GOBACK
       .
   00-B SECTION.

       MOVE L-C
             ( M )
                                    TO W-H

       DIVIDE W-G                   BY 16
         GIVING                     W-I
         REMAINDER                  W-J

       MOVE W-I                     TO W-X
       PERFORM                      00-C
       MOVE W-Y                     TO W-P
       MOVE W-J                     TO W-X
       PERFORM                      00-C
       MOVE W-Y                     TO W-Q

       SET J
           K
           L                        UP BY +1

       IF ( J GREATER THAN W-B )
           PERFORM                  00-D
           MOVE SPACE               TO W-R
                                       W-U
           SET J
               K
               L
                                    TO +1
       END-IF

       MOVE W-P                     TO W-S
                                        ( J )
       MOVE W-Q                     TO W-U
                                        ( K )
       MOVE W-H                     TO W-W
                                        ( L )
       .
   00-C SECTION.

       IF ( W-X GREATER THAN 9 )
           SUBTRACT 9               FROM W-X
             GIVING                 W-L
           SET I                    TO W-L
           MOVE W-N ( I )           TO W-Y
       ELSE
           MOVE W-X                 TO W-AA
           MOVE W-AB                TO W-Z
       END-IF

       .
   00-D SECTION.

       MOVE W-AD                    TO W-AC
       DISPLAY                      W-AC

       DISPLAY                      W-V
       DISPLAY                      W-R
       DISPLAY                      W-T
       .

Now, there are a couple of bugs in this version. Enjoy finding them, as you first have to work out, with no aid, what the program does.

Since you have GnuCOBOL, track down the discussion area at SourceForge.Net, the current home of GnuCOBOL.


Instantiate and display a ClassifiedAd Create an application named TestNewspaperAd that instantiates and displays a ClassifiedAd object. A ClassifiedAd has fields for number of words and a price. Include get and set properties for the number of words but only a get property for the price--the price should be calculated at nine cents per word. You can create either a console or GUI app.

namespace TestNewsPaperAd
{
    class Newspaper
    {
        static void Main(string[] args)
        {
            const double rate = 0.09;
            double total;
            ClassifiedAd Article = new ClassifiedAd();
            Article.Words = 583;

            total = Article.Words * rate;

            WriteLine("The price of the {0} word article is {1}\n", Article.Words, total);
        }
    }
}
class ClassifiedAd
{
    private int words;
    private double price;

    public int Words
    {
        get
        {
            return words;
        }
        set
        {
            words = value;
        }
    }
    public double Price
    {
        get
        {
            return price;
        }
    }
}

There is an error in how you implement the Price property: according to the description your get for the price should return the number of words times 0.09. As it is, the Price will always return 0 as there is never a value assigned to it.

Try adding Console.WriteLine(Article.Price); inside your Main method for demonstration.

You can add this inside your class:

private const decimal pricePerWord = 0.09M;

Notice the use of decimal (including the M suffix) instead of double, which is a more appropriate type for currency/money values.

Then your get becomes:

public decimal Price
{
    get
    {
        return Words * pricePerWord;
    }
}

I would also recommend to make a constructor for ClassifiedAd that takes the number of words, since a classified ad without words would be nonsensical.

public ClassifiedAd(int words)
{
    this.words = words;
}

Also as suggested by Roman Gräf, a ToString override method would be a nice addition:

override public string ToString()
{
    return string.Format("The price of a {0} word classified ad is {1}\n", Words, Price);
}

Then your Main method can do everything by only using what your ClassifiedAd class provides:

static void Main(string[] args)
{
    ClassifiedAd ad = new ClassifiedAd(583);
    Console.WriteLine(ad.ToString());
    // The price of a 583 word classified ad is 52.47

    ad.Words = 900;
    Console.WriteLine(ad.ToString());
    // The price of a 900 word classified ad is 81.00
}


Building Weather Forecast from JSON with optionals I am looking for the best way of parsing JSON in Swift 2.0.

I have this JSON

{
  "latitude": 37.8267,
  "longitude": -122.423,
  "currently": {
    "time": 1440110395,
    "summary": "Clear"
  }
}

and field summary is NOT always visible in response.

My model object and parsing method:

enum JSONErrorType: ErrorType {
  case InvalidJson
}

struct WeatherForecast {
  let date: NSDate
  let summary: String?
  let latitude: Double
  let longitude: Double

  init(date: NSDate, summary: String?, latitude: Double, longitude: Double) {
    self.latitude = latitude
    self.longitude = longitude
    self.date = date
    self.summary = summary
  }

  static func fromJSON(json: AnyObject) throws -> WeatherForecast {
    guard let dict = json as? NSDictionary,
      let latitude = dict["latitude"] as? Double,
      let longitude = dict["longitude"] as? Double,
      let curr_dict = dict["currently"] as? NSDictionary,
      let timestamp = curr_dict["time"] as? NSTimeInterval else {
      throw JSONErrorType.InvalidJson
    }
    let date = NSDate(timeIntervalSince1970: timestamp)
    if let summary = curr_dict["summary"] as? String {
      return WeatherForecast(date: date, summary: summary, latitude: latitude, longitude: longitude)
    } else {
      return WeatherForecast(date: date, summary: nil, latitude: latitude, longitude: longitude)

    }
  }
}

I can't use guard because the summary property is optional. if let works here but I have two returns which doesn't look very good. Also if I will have more optionals fields, the code gets messy.

Remember optionals.
Let's start with answering your primary concern: what do I do with the summary field?
You've already appropriately set up a constructor which takes a String? argument for summary.  But your particular code never takes advantage of this fact, does it?

if let summary = curr_dict["summary"] as? String

Let's break down what's going on here piece by piece.

curr_dict["summary"] returns an AnyObject?.  It makes no assumptions about the type, since curr_dict is an NSDictionary.  It either returns whatever value is at that key, or nil if the key doesn't exist.

as? String tries to cast that value as a String.  If the value was nil or if it can't be cast as a string, it returns nil, otherwise it returns String.

let summary = declares the variable called summary, whose type is determined by the return of everything to the right.  Since the top of the right-side's stack is as? String, which can return nil or a String, then the return type is String?.

if let casts an optional type (String?) as a non-optional type (String) if it can, otherwise it enters the else block.

So... we can get rid of the if let part, and just make ourselves an optional string, right?
let summary = curr_dict["summary"] as? String
return WeatherForecast(date: date, summary: summary, latitude: latitude, longitude: longitude)

But wait, there's more!

guard let dict = json as? NSDictionary, // ...

We should prefer to use Swift types over Objective-C types when that makes sense.  And here is a place where it does.  What we want is a [String:Any].
guard let dictionary = json as? [String:Any], // ...

Encapsulate.
We should also prefer encapsulation.  It doesn't make particular sense to have a latitude and longitude property.  What we need is a type that encapsulates these, as they're just two parts of describing one thing: location.
The easiest solution would be simply importing CoreLocation and either using CLLocationCooridinate2D or CLLocation.  The downside to this, however, is that any class that uses our struct has to import CoreLocation in order to use this property, but we can easily get around this with a typealias:
typealias WeatherForecastLocation = CLLocationCoordinate2D

And now we replace our latitude and longitude properties with a property of type WeatherForecastLocation.

Informative Errors
So, JSONErrorType.InvalidJSON... how useful is this?  It's an enum with a single case, just so we have something to throw.  We can be so much more informative.
First of all, two types of valid top-level JSON objects.  Dictionary or array.  So any method we write to parse JSON will expect one of these two types, so the first error will be the JSON not being that type.  That gives use two errors right off the bat (and let's grab some better names here):
enum JSONParseError: ErrorType {
    case ExpectedArrayNotFound
    case ExpectedDictionaryNotFound
}

If we are looking for JSON with a top-level object of dictionary, and we don't find it, we throw ExpectedDictionaryNotFound.  It doesn't matter what it looked like.  But this is already more informative... we couldn't even manage to get past the first level.
Now, what's another sort of JSON parsing error?  Missing keys.  But we don't want to simply be that generic.  Let's specify what key is missing, shall we?  Swift enums can have associated values.  With our MissingKey error, we should use a String associated value to identify the missing key.  Or enum now looks like this:
enum JSONParseError: ErrorType {
    case ExpectedArrayNotFound
    case ExpectedDictionaryNotFound
    case MissingKey(String)
}

We also have an error case for when the type of the value doesn't match what we expect.  For this, we want to specify the key that's problematic and specify the type that we expect it to be, so let's associate a tuple of two strings:
enum JSONParseError: ErrorType {
    case ExpectedArrayNotFound
    case ExpectedDictionaryNotFound
    case MissingKey(String)
    case InvalidTypeForKey(String,String)
}

So, the first string specifies the key, and the second string is a message something like "Expected Int", "Expected Array".
Now... the last thing we can do here that's probably quite helpful... return the JSON that was passed in with error.
enum JSONParseError: ErrorType {
    case ExpectedArrayNotFound(Any)
    case ExpectedDictionaryNotFound(Any)
    case MissingKey(String,Any)
    case InvalidTypeForKey(String,String,Any)
}


Speeding up and shortening a loop I'm still relatively new to C++ and I've written a program that could do with being a bit shorter and faster.

I use this same set of for loops many times to cycle through the whole tree of values. I'll use the JSON format to show you a much shortened version of the tree (in reality the tree has another layer of depth with many more values):

{
  "WorldOne": {
    "Earth": {
      "Japan": "Tokyo",
      "America": "New York"
    },
    "Moon": {
      "Dark Side": "Crater One"
    }
  }
}

I then parse this and fill a large vector with these values. The format of the vectors can be seen in the loop below.

My program then, depending on the parameters I set, cycles through to grab the data contained within.

This is the loop I am using over and over again:

for ( auto cWorld : fWorldVec ) //vector of world pointers
        {
            for ( auto cPlanet : ( cWorld )->fPlanetVector )
            {
                for ( auto cCity : cPlanet.fCityVector )
                {
                     for (auto cValue : cCity.fValueVector)
                     {
                          // Then I do stuff
                          // e.g:
                          ConstructSomething( ( cWorld )->getWorldId(), cPlanet.getPlanetId(), cCity.getCityId(), cValue.getValueId() );
                          // I know the above JSON didn't contain values,
                          // but it does in my code for reasons that aren't worth mentioning
                          // This loop is used so many times throughout my code
                          // It would be wonderful to simply be able to grab the data much faster
                     }
                }
            }
        }

I apologise if this question is too vague, but I really need a speed boost or even to make this loop shorter.  I'd like any improvements on the format of my tree or how to condense these loops.

I was thinking along the lines of making some kind of address table with pointers - but I can't quite think of a way to do it.

There is no improvement which can be done on the code you show us. If the function constructSomething really needs to be called for each combination of the parameters, there is nothing better you can do.

You should try to optimizie the function, maybe it is possible to cache some part of the construction which is common if one of the parameters is fixed.

For example you can pop the computation cWord->getWorldId() in the outer loop, so that you make it once for every Word (instead of once for every City). But I think this is a slight improvement.


Vectorization for temperature simulation I'm new to Matlab and I would like to know tips in order to reduce the processing time of such program. The scheme of the actual code is very similar to typical C++ code and I would like to vectorize the for and if loops in order to decrease the processing time. The code is added just to make you know the overall idea of what I would like to obtain.

close all             %Tanca totes les finestres obertes
clear all             %Removes variables from memory
clc                   %Clears command window
tic                   %inicia comptador temps simulació

trans=fopen('trans_matlab.txt','wt')

%Physical properties and geometrical data input
Tair=25;
Tinitial=100;
h=10;
k=5;
qv=500;
thick1=0.05;
thick2=0.1;
density=1000;
Cp=800;

%Parameters imput
max_error=0.000001;   %Error màxim a partir del cual no hi ha canvis en els resultats
max_iter=1000000;
r=0.9;
N=10;                 %Nodes paret1
M=10;                 %Nodes paret2
Inc_x1=thick1/N;
Inc_x2=thick2/M;
num_t=23000;          %temps de realització
Inc_t=20;

%Initial map temperatures
t=1;
for i=1:(N+M+1)       %Hipòtesi inicial: tots els nodes tenen T i Ta = Tair
    T(i,t)=Tinitial;      %(N+M+1) pq no deixa començar a 0
    Ta(i,t)=Tinitial;
end

%GAUS SEIDEL
while t<num_t
    %New instant
    t0=t;
    t=t+1;
    for i=1:(N+M+1)
        Ta(i,t)=T(i,t0);
    end
    %Temperature calculation
    for j=0:max_iter
        for i=1:(N+M+1)
            if i==1
            b=0;
            c=k/Inc_x1;
            d=qv*Inc_x1/2;
            e=density*Cp*Inc_x1/(2*Inc_t);
            a=b+c+e;
            T(i,t)=(c.*Ta(i+1,t)+d+e.*T(i,t0))/a;
            end
            if (i>1)&(i<(N+1))
            b=k/Inc_x1;
            c=k/Inc_x1;
            d=qv*Inc_x1;
            e=density*Cp*Inc_x1/(Inc_t);
            a=b+c+e;
            T(i,t)=(b.*Ta(i-1,t)+c.*Ta(i+1,t)+d+e.*T(i,t0))/a;
            end
            if i==(N+1)
            b=k/Inc_x1;
            c=k/Inc_x2;
            d=qv*Inc_x1/2;
            e=density*Cp*Inc_x1/(2*Inc_t)+density*Cp*Inc_x2/(2*Inc_t);
            a=b+c+e;
            T(i,t)=(b.*Ta(i-1,t)+c.*Ta(i+1,t)+d+e.*T(i,t0))/a;
            end
            if (i>(N+1))&(i<(N+M+1))
            b=k/Inc_x2;
            c=k/Inc_x2;
            e=density*Cp*Inc_x2/(Inc_t);
            a=b+c+e;
            T(i,t)=(b.*Ta(i-1,t)+c.*Ta(i+1,t)+e.*T(i,t0))/a;
            end
            if i==(N+M+1)
            b=k/Inc_x2;
            c=h;
            e=density*Cp*Inc_x2/(Inc_t*2);
            a=b+c+e;
            T(i,t)=(b.*Ta(i-1,t)+h*Tair+e.*T(i,t0))/a;
            end
            %Error calculation-trobar l'error maxim de la matriu error(i,t)
            errort=0;
            error(i,t)=abs(T(i,t)-Ta(i,t));
            if error(i,t)>errort
            errort=error(i,t);
            else
            errort=errort;
            end
        end
        if errort>max_error
            for i=1:(N+M+1)
                Ta(i,t)=T(i,t).*r+Ta(i,t).*(1-r);
            end
        else
            break
        end
    end
    iter(t)=j;
end

toc
%Print Results
t=1;
fprintf(trans,'time\t T1\t TN+1\t TN+M+1\t iter\n');

while(t<=num_t)
time=t*Inc_t;
fprintf(trans,'%.2f\t %.2f\t %.2f\t %.2f\t %.2f\n',time,T(1,t),T(N+1,t),T(N+N+1,t),iter(t));
t=t+1;
end
fclose(trans);

Besides the hints in comments, I would like to add the following:

Replace the abbreviated variables by useful names. This will increase the performance of your code reviewer.
Try to keep all your comments in English (see 1.).
Preallocate variables if you really want to write their value inside a loop. Even in C++ you would not write Ta(i,t)=Tinitial;.
Do you really need all N*M*num_t values? If not, you should only store the current and the last iteration.
Now try to begin to replace the repetitive statements like Ta(i,t)=T(i,t0); and the sourounding for-loop with a single statement. You don't need to replace all at once.
Split your code into a main function and one or more subfunctions. A recent Matlab itself may then increase execution speed of the loops.
You calculate c=k/Inc_x1; very often but neither k nor Inc_x1 changes. Try to identify lines which can be computed outside of the loop. Maybe you need to introduce more variables (with good names!).
Replace the while-loop by a for-loop. It will be more readable.
Correct the indentation of your code. Press Ctrl-A and Ctrl-I.
If you have a 2D object, then why not use a 2D array for this. If you really need the results for all times, then you should have a 3D-array. You may then initialize it with temperature = zeros(M, N, T_max);

If you made all this changes, feel free to present your new readable and beautiful code.


LeetCode gas station implementation There are N gas stations along a circular route, where the amount of
  gas at station i is gas[i].

  You have a car with an unlimited gas tank and it costs cost[i] of gas
  to travel from station i to its next station (i+1). You begin the
  journey with an empty tank at one of the gas stations.

  Return the starting gas station's index if you can travel around the
  circuit once in the clockwise direction, otherwise return -1.

  Note:

  If there exists a solution, it is guaranteed to be unique. Both input
  arrays are non-empty and have the same length. Each element in the
  input arrays is a non-negative integer. Example 1:

  Input:  gas  = [1,2,3,4,5] cost = [3,4,5,1,2]

  Output: 3

  Explanation: Start at station 3 (index 3) and fill up with 4 unit of
  gas. Your tank = 0 + 4 = 4 Travel to station 4. Your tank = 4 - 1 + 5
  = 8 Travel to station 0. Your tank = 8 - 2 + 1 = 7 Travel to station 1. Your tank = 7 - 3 + 2 = 6 Travel to station 2. Your tank = 6 - 4 + 3 = 5 Travel to station 3. The cost is 5. Your gas is just enough to
  travel back to station 3. Therefore, return 3 as the starting index.
  Example 2:

  Input:  gas  = [2,3,4] cost = [3,4,3]

  Output: -1

  Explanation: You can't start at station 0 or 1, as there is not enough
  gas to travel to the next station. Let's start at station 2 and fill
  up with 4 unit of gas. Your tank = 0 + 4 = 4 Travel to station 0. Your
  tank = 4 - 3 + 2 = 3 Travel to station 1. Your tank = 3 - 3 + 3 = 3
  You cannot travel back to station 2, as it requires 4 unit of gas but
  you only have 3. Therefore, you can't travel around the circuit once
  no matter where you start.

Here is my solution:

public class Solution {
    public int CanCompleteCircuit(int[] gas, int[] cost) {
        int startIndex = 0;
        int tank = 0;
        int diff= 0;
        int sum =0;
        for(int i=0;i < gas.Length; i++)
        {
            diff = gas[i] - cost[i];
            sum += diff;
            if(tank + diff < 0)
            {
                startIndex = i+1;
                tank = 0;
            }
            else
            {
                tank += diff;
            }
        }
        if(sum < 0)
        {
            return -1;
        }
        else
        {
            return startIndex;
        }
    }
}

Please review code runtime and memory. Any other comments are welcome!

Your code nicely exploits all the constraints from the instructions.

It runs in linear time, which is much better than the naive approach, which would run in quadratic time.

It doesn't need any space other than a few local variables. Perfect.

Without further explanation the code is a bit hard to follow because of the many variables, and it is not entirely clear what sum and diff are exactly. Sure, sum holds a sum, but of what?

The diff variable should move inside the for loop since it is only needed there.

Instead of tank + diff < 0, you could also write tank < diff, but since you later compute tank + diff, having this common subexpression in the code is actually useful. You could also combine the expressions: just execute tank += diff, and if there's a negative amount of gas in the tank after that, reset it to zero. It's impossible in the real world to have negative amounts of gas in the tank, but it would make the code shorter.

For understanding the algorithm I would have preferred a variant that has two for loops. In the first loop, just compute the sum. In the second loop, determine the starting index. That way, there's fewer variables who can confuse me, the human reader. But if you're going for execution speed, you approach is perfect, again.

I would have preferred a short introduction about how you constructed your algorithm. Learning the ideas and then going from ideas to code is often easier than going backwards from the code to the underlying ideas.


Finding all possible paths between two points in a grid using recursion I'm trying to find all possible paths made of 0 from bottom left to top right, only by moving either up or to the right. The input file contains a grid of characters, where 1 represents an obstacle and 0 represents a vacant cell.

The problem I have is the code not finishing/ taking too long. The code works correctly with smaller inputs like this, but on a large file with 5000 rows and 5000 characters in each row it never finishes even after 20 minutes.

import sys
sys.setrecursionlimit(900000000)

file_e = open('file.txt','r')
file = file_e.readlines()
file_e.close()
file.reverse()
for index,line in enumerate(file):
    file[index] = line.split(' ')
print(len(file)-1, len(file[0])-1)
def end(file,y,x):

    if y == len(file)-1 and x == len(file[0])-1: return 1

    try:
        if file[y+1][x] == '0': up = True
    except: pass
    try:
        if file[y][x+1] == '0': right = True
    except: pass

    try:
        if up == True and right == True: return end(file,y+1,x) + end(file,y,x+1)
    except: pass

    try:
        if up == True: return end(file,y+1,x)
    except: pass

    try:
        if right == True: return end(file,y,x+1)
    except: pass

    if file[y+1][x] == '1' or file[y][x+1] == '1':
        return 0

print(end(file,0,0))

No optimization will make this run effectively for large inputs because fundamentally the "all paths problem" is hard. In short, as the size of the input increases incrementally, the number of paths increases exponentially. Your best option may be to ask "what is a similar question I can ask that will give me a useful answer for less computational expense."

In terms of code quality, using try-catch structures for branching is often considered bad form. In many languages this would be a performance issue, but from my 30 seconds of googling it appears the performance impact isn't huge in python.

One adjustment you could make is to parse the file into an adjacency matrix and then using a known algorithm to solve for your lists of paths.


Get minimum of each group based on hour criteria using pandas I have a requirement which I posted in SO. However, I have already written code and works fine. I request your review to know whether it is fool proof/elegant and efficient. Please find the details here in SO post Question in SO

my code

df['time_1']= pd.to_datetime(df['time_1'])
s=pd.to_timedelta(24,unit='h')-(df.time_1-df.time_1.dt.normalize())
df['tdiff'] = df.groupby(df.time_1.dt.date).time_1.diff().shift(-1).fillna(s)
df['t_d'] = df['tdiff'].dt.total_seconds()/3600
df['hr'] = df['time_1'].dt.hour
df['date'] = df['time_1'].dt.date
df['day'] = pd.DatetimeIndex(df['time_1']).day

# here I get the freq and cumsum of each val for each day and each hour.Since sort = 'False', timeorder is retained as is

temp_1 = pd.DataFrame(df.groupby(['subject_id','date','hr','val'], sort=False)['t_d'].agg({'cumduration':sum,'freq':'count'}).reset_index())

# here i remove the `hour` component and sum the value duration in same day but different hours (for example `5` was in 12th hour and 13th hour. we sum them)

temp_2 = pd.DataFrame(temp_1.groupby(['subject_id','date','val'], sort=False)['cumduration'].agg({'sum_of_cumduration':sum,'freq':'count'}).reset_index())

# Later, I create a mask for `> 1` hr criteria

mask = temp_2.groupby(['subject_id','date'])['sum_of_cumduration'].apply(lambda x: x > 1)
output_1 = pd.DataFrame(temp_2[mask].groupby(['subject_id','date'])['val'].min()).reset_index()

# I check for `< 1 ` hr records here

output_2 = pd.DataFrame(temp_2[~mask].groupby(['subject_id','date'])['val'].min()).reset_index()

# I finally check for `subject_id` and `date` and then append

output = output_1.append(output_2[~output_2['subject_id'].isin(output_1['subject_id'])])

output

leave the original data intact

Since df is the original data, adding columns to it can have a strange side effect in another part of the calculation. Best is not to touch the original data, and do all the calculations etc in separate series or copied DataFrames

pd.grouper

You can use pd.Grouper to group per day and hour

You can group the date per subject and hour with:

df.groupby(["subject_id", pd.Grouper(key="time_1", freq="h"), "val"])

Then you don't need all those extra columns "hr", "date" and "day" anymore.
There is no reason for the column "t_d" either. since timedelta's can be counted, summed etc as well.

aggregation function

Apart from that,  you can design a specific function to get the requested minimum from one such group.

If you supply it data grouped per subject and date, dropping rows with the same val and checking whether more than 1hr has passed is a lot easier

def day_grouper(group):
    """
    Returns the minimum in `val` that lasted more than 1 hour
    """
    # drop consecutive same values.
    # The first value is retained because it becomes NaN and `NaN != 0`
    group = group.loc[group["val"].diff() != 0]
    # drop those that take less than 1hr
    more_than_1h = group["time_1"].diff().shift(-1) > pd.Timedelta("1h")
    # Check whether the last value started before 23:00
    more_than_1h.iloc[-1] = group["time_1"].iloc[-1].time() < datetime.time(23)
    return group.loc[more_than_1h, "val"].min()

This can then be called like this:

(
    df.groupby(["subject_id", pd.Grouper(key="time_1", freq="d")])
    .apply(day_grouper)
    .rename("val")
    .reset_index()
)

The 2 last rows are to get the format you want

code formatting

Try to follow Pep-8.

£some of your lines are really long, and convention is a space after a comma.

I use black to format my source code, so I don't have to worry about long lines, spaces, etc any more


Read in parameters with links from textfile I'm trying to read in a few lines of parameters from an textfile and format these paths.

How can I improve this code to work with less lists? I just need the final paths.

import os
config = open("C:\\Users\\config.txt")
content = config.readlines()
removedEnds = []
removedStarts = []
finalPaths = []

for string in content:
    removedEnds.append(string.rstrip())

for string in removedEnds:
    print(string)
    if string.find('='):
        removedStarts.append(string[(string.find('=')+1):])

for string in removedStarts:
    finalPaths.append(string.split("\\"))

for string in finalPaths:
    print(os.path.join(*string))

The textfile looks like this:

Source=C:\Users\workspace\OMDB\WebContent\app
Target=C:\Apache24\htdocs

Rather than creating a series of lists, you should just iterate over the lines of the file directly. You don't need to maintain state between any two lines – just process them as you get them.

A few idioms you should also note:

Use the with open(...) constructions. This ensures the file is always closed, even if the script throws an exception. (Your script never closes the file.)
Rather than using if string.find('='), use if '=' in string.
Rather than using string[(string.find('=')+1):] to get the portion after the equals sign, I think it's neater and more Pythonic to use string.split('=', maxsplit=1)[1]

Here's a version of the script that doesn't use any lists:

import os

with open('C:\\Users\\config.txt') as f:
    for line in f:
        if '=' in line:
            path = line.split('=', maxsplit=1)[1].strip()
            components = path.split('\\')
            print(os.path.join(*components))


Multiple forms for accessing records in database I have a main form for adding and deleting records and a button that opens a child form that displays records from an access database.  Child form will have buttons for moving though the rows like first, last, next, previous and search.  When user selects a record the child form will close and the selected record will be displayed in the main form, from which it can be changed, deleted, etc.  This is the logic behind what I’m doing.  So far I have written a class for accessing the data and is called from the child form to display the records.  Before I move on, I want to get advice on what I have done so far and if it follows best practices.  Any advice would be greatly appreciated.

Main Form:

namespace Test
{
public partial class frmTest : Form
{
    public frmTest()
    {
        InitializeComponent();
    }

    public class myClass
    {
        private static OleDbConnection myConn = new OleDbConnection();
        private static DataSet myDS;
        private static DataRow myDR;
        public static int MaxRows = 0;
        public static int increment = 0;
        public static string name;

        public static void GetConnection()
        {
            myClass.myConn.ConnectionString = @"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Temp\Testing\TestDatabase.accdb";
            try
            {
                myConn.Open();
                myDS = new DataSet();
                string SQL = "SELECT * From Test";
                OleDbDataAdapter myDA = new OleDbDataAdapter(SQL, myConn);
                myDA.Fill(myDS, "People");
                NavigateRecords();
                MaxRows = myDS.Tables["People"].Rows.Count;
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
            finally
            {
                myConn.Close();
            }
        }

        public static void NavigateRecords()
        {
            myDR = myDS.Tables["People"].Rows[increment];
            name = myDR.ItemArray.GetValue(1).ToString();
        }
    }

    private void frmNewForm_Click(object sender, EventArgs e)
    {
        frmNewForm fNew = new frmNewForm();
        fNew.ShowDialog();
    }
}
}

Child form:

namespace Test
{
public partial class frmNewForm : Form
{
    public frmNewForm()
    {
        InitializeComponent();
    }

    private void frmNewForm_Load(object sender, EventArgs e)
    {
        MyAlias.GetConnection();
        txtName.Text = MyAlias.name;
    }

    private void FillTextBox()
    {
        txtName.Text = MyAlias.name;
    }

    private void btnNext_Click(object sender, EventArgs e)
    {
        if (MyAlias.increment != MyAlias.MaxRows - 1)
        {
            MyAlias.increment++;
            MyAlias.NavigateRecords();
            FillTextBox();
        }
        else
        {
            MessageBox.Show("No more rows");
        }
    }
}
}

Learn about Data Binding

Here is one tutorial.  My main point is that with the Visual Studio designer linking  back end data and the UI is pretty straight forward. And you can get navigation automagically; and along with "master / detail" synchronization it is so "thank goodness I didn't have to write this."

Separate UI from Data Fetching

I would pass in a MyClass object to the Form. This dependency injection is generally superior to "newing up" objects internally; making the design far more flexible and testable. I.E. we can inject mock (fake) stuff for the sake of testing.

One Form

Once the data fetching is factored out of frmTest then it becomes clear that a second Form is not needed. However, if you want that child form look/feel/behavior, then the data binding above makes it easy peasy.

Code Convention for Data Fetching

Don't leave open / unused connections lying about
Always wrap DB calls in try/catch. There's so much that can go wrong.

Here is a generally accepted good way of writing the code, taken right out of MSDN.

public void GetPeopleData() {
    // I put these outside of "using" because they are class variables.
     myClass.myConn.ConnectionString = @"Provider=Microsoft.ACE.OLEDB.12.0;Data Source=C:\Temp\Testing\TestDatabase.accdb";
 myDS = new DataSet();

    // everything created in "using" gets destroyed/goes out of scope at end
    // of "using" block. And in particular "myConn" gets disposed / destroyed
    // at the end because it is the "using" parameter. We really want to make
    // sure all the resources - and in particular the OleDbConnection - go
    // away.
    using (OleDbConnection myConn= new OleDbConnection(connectionString))
        {
            string SQL = "SELECT * From Test";
            OleDbDataAdapter myDA = new OleDbDataAdapter(SQL, myConn);

            // Open the connection and execute the insert command.
            try
            {
                connection.Open();
                myDA.Fill(myDS, "People");
                NavigateRecords();
                MaxRows = myDS.Tables["People"].Rows.Count;
            } catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
      } //using

        // The connection is automatically closed when the
        // code exits the using block.
} //GetPeopleData()

Rename GetConnection()

This method gets the data. "Getting the connection" is merely a detail in that process.

EDIT

Changed the general example to be specific to the question code.

Renamed GetConnection() to GetPeopleData()

Refactor GetPeopleData()

GetPeopleData() should do that and only that. I do not understand NavigateRecords() in there. I should think navigating records is done in the child form ... or does NavigateRecoreds() even navigate the records?


Find existing session I have this method in my rails app:

def find_or_create_session(fbid)
    @sessions = Session.all
    if @sessions.find_by facebook_id: fbid
      @session = @sessions.find_by facebook_id: fbid
      if ((Time.now - @session.last_exchange).fdiv(60)).to_i > 5
        @session = Session.create(facebook_id: fbid, context: {})
      end
    else
       @session = Session.create(facebook_id: fbid, context: {})
    end
    @session
end

It retrieves all the Sessions, and tries to find one with a given facebook_id. If there is one, it checks if it's a recent one (last_exchange was less than 5 minutes ago). If it's not, it creates a new one.

If it can't find a session for this facebook_id, it will create one.

This is pretty easy, but it seems to be a lot of if/else for such a straightforward thing.

Some notes:

@session: A method called find_or_create_session should not change the state of instance variables.
@sessions.find_by facebook_id: fbid is written twice. Use variables to avoid repetition. Also, the consensus in the Ruby community is to always write parens (except for DSL-like code).
Time.now - @session.last_exchange: You can perform that check in the  SQL query.
Your method is basically doing find_or_create_session = existing_session || new_session, let the code mimic it.

I'd write:

def find_or_create_session(fbid, max_age: 5.minutes)
  Session.find_by(["facebook_id = ? AND last_exchange >= ?", fbid, max_age.ago]) ||
    Session.create(facebook_id: fbid, context: {})
end


Molecule and ligation finding program I wrote this code so I could figure out the number of molecules within a plasmid and an insert of DNA. I then get the amount of insert DNA I would need to use (in uL) for a ligation with the DNA at different insert ratios and dilutions.

How can I clean up this code? It works correctly. However, I feel like I typed this out all the extremely long way and I would love to know how to make this more performant.

In my next project I would like to not have to type all of these out. I imagine that I am missing some functions or loops. I am still a very new beginner and this is my first project outside of a classroom.

print "Please put the amount of BPs: "
number = int(raw_input())
solution = (number * 660)
Avo = (6.022 * (10 ** 23))
Grams_Avo = ( solution / Avo)
print "Put in Concentration (ng/ul): "
conc = float(raw_input())
multiple = (10 ** -9)
amount = (conc * multiple)
total = float(amount / Grams_Avo)
print "Amount of molecules:  %s" % total

print "Amount of ul used by the plasmid: "
plasmid = float(raw_input())
particles = float(plasmid * total)
print "What ratio amount will you use?: "
ratio = int(raw_input())
molecules_needed = float(particles * ratio)
print molecules_needed

print "Conc. of your oligo?: "
oligo = float(raw_input())
oligo_num = float(oligo * multiple) # amount
print "Enter number of G in Oligo: "
G = int(raw_input())
print "Enter number of A in Oligo: "
A = int(raw_input())
print "Enter number of T in Oligo: "
T = int(raw_input())
print "Enter number of C in Oligo: "
C = int(raw_input())
sumG = (G * 329)
sumA = (A * 313)
sumT = (T * 304)
sumC = (C * 289)
daoligo = (sumG + sumA + sumT + sumC) # g/mol

print "Total amount of your insert %s" % daoligo

oligototal = float(oligo_num / (daoligo/Avo)) # molecules for insert
complete = float(molecules_needed/oligototal) #amount of insert needed from stock ul

print "What dilution will you use?"
print "1. Stock"
print "2. 1:10"
print "3. 1:20"
print "4. 1:30"
print "5. 1:50"
code = float(raw_input())
if code == 1:
    print complete #stock
elif code == 2:
    code2 = (oligototal/10)
    complete1 = float(molecules_needed/code2)
    print complete1 #1:10
elif code == 3:
    code3 (oligototal/20)
    complete2 = float(molecules_needed/code3)
    print complete2 #1:20
elif code == 4:
    code4 = (oligototal/30)
    complete3 = float(molecules_needed/code4)
    print complete3 #1:30
elif code == 5:
    code_5 = (oligototal/50)
    complete4 = float(molecules_needed/code_5)
    print complete4 #1:50

For the starter, you can create small function which will get input since your code requires too much user input.

Just making that one function will reduce repetitive code of raw_input() and printing it's msg.

Also, your code will be more readable.

Code:

def get_input(msg):
    print msg
    val = int(raw_input())
    return val

number = get_input("Please put the amount of BPs: ")

solution = (number * 660)
Avo = (6.022 * (10 ** 23))
Grams_Avo = ( solution / Avo)

conc = float(get_input("Put in Concentration (ng/ul): "))

multiple = (10 ** -9)
amount = (conc * multiple)
total = float(amount / Grams_Avo)
print "Amount of molecules:  %s" % total

plasmid = float(get_input("Amount of ul used by the plasmid: "))
particles = float(plasmid * total)

ratio = get_input("What ratio amount will you use?: ")

molecules_needed = float(particles * ratio)
print molecules_needed

oligo = float(get_input("Conc. of your oligo?: "))
oligo_num = float(oligo * multiple) # amount

G = get_input("Enter number of G in Oligo: ")

A = get_input("Enter number of A in Oligo: ")

T = get_input("Enter number of T in Oligo: ")

C = get_input("Enter number of C in Oligo: ")

sumG = (G * 329)
sumA = (A * 313)
sumT = (T * 304)
sumC = (C * 289)
daoligo = (sumG + sumA + sumT + sumC) # g/mol

print "Total amount of your insert %s" % daoligo

oligototal = float(oligo_num / (daoligo/Avo)) # molecules for insert
complete = float(molecules_needed/oligototal) #amount of insert needed from stock ul

m = """
    What dilution will you use?
    1. Stock
    2. 1:10
    3. 1:20
    4. 1:30
    5. 1:50
    """

code = float(raw_input())
code =  get_input(m)

if code == 1:
    print complete #stock
elif code == 2:
    code2 = (oligototal/10)
    complete1 = float(molecules_needed/code2)
    print complete1 #1:10
elif code == 3:
    code3 = (oligototal/20)
    complete2 = float(molecules_needed/code3)
    print complete2 #1:20
elif code == 4:
    code4 = (oligototal/30)
    complete3 = float(molecules_needed/code4)
    print complete3 #1:30
elif code == 5:
    code_5 = (oligototal/50)
    complete4 = float(molecules_needed/code_5)
    print complete4

Output

Please put the amount of BPs:
12
Put in Concentration (ng/ul):
1
Amount of molecules:  76035353535.4
Amount of ul used by the plasmid:
2
What ratio amount will you use?:
3
4.56212121212e+11
Conc. of your oligo?:
4
Enter number of G in Oligo:
4
Enter number of A in Oligo:
6
Enter number of T in Oligo:
7
Enter number of C in Oligo:
8
Total amount of your insert 7634
9

    What dilution will you use?
    1. Stock
    2. 1:10
    3. 1:20
    4. 1:30
    5. 1:50

1
1.44583333333


Python program to find the substring with concatenation of all words This is a Leetcode problem:

  You are given a string, S, and a list of words, L that are all of the same lengths. Find all starting indices of substring(s) in S that is a concatenation of each word in L exactly once and without any intervening characters.

Here is my solution to the problem:

class Solution:

    def __init__(self, S, L):
        self.S = S
        self.L = L

    def findSubstring(self, S, L):
        res = []      # result list
        num = len(L)  # length of the str list
        ls = len(S)
        if num == 0:
            return []
        str_len = len(L[0]) # length of each str

        #creating the map: counting the occurrence of each string

        map_str = dict((x,L.count(x)) for x in set(L))
        i = 0
        while i + num * str_len - 1 < ls:
            map_str2 = {}
            j = 0
            while j < num:
                subs = S[i + j * str_len:i + j * str_len + str_len ]
                if not subs in map_str:
                    break
                else:
                    map_str2[subs] = map_str2.get(subs, 0) + 1
                    if map_str2[subs]>map_str[subs]:
                        break
                    j = j + 1
            if j == num:
                res.append(i)
            i = i + 1

        return res

S = "barfoothefoobarman"
L = ["bar","foo"]

index = Solution(S, L)
print(index.findSubstring(S, L))

Here are some example inputs/outputs:

S = "barfoothefoobarman"

L = ["bar", "foo"]

>>> [0, 9]

-------------------------------------------------------------------

S = "lingmindraboofooowingdingbarrwingmonkeypoundcake"

L = ["fooo", "barr", "wing", "ding", "wing"]

>>> [13]

So I would like to know whether I could make this program shorter and more efficient.

Any help would be highly appreciated.

As I told in another answers, your code is very C/C++ styled, it is not Pythonic. Try to avoid manual iteration with indices as much as possible. Python has an enormous standard library that contains many useful modules. I already recommended you an itertools module. It contains pair of dozens generic functions to work with iterators. One of them - permutations - do 90% of your work:

Return successive r length permutations of elements in the iterable.
permutations('ABCD', 2) --> AB AC AD BA BC BD CA CB CD DA DB DC
permutations(range(3)) --> 012 021 102 120 201 210

For list L, it returns all possible permutations iterator. If we will join each permutation, we will get the needed search substring.
But next we will have a problem. Python has no built-in function or easy method to find indices of all found substrings in the string (it is strange, I hope they will add it to future Python versions). So you have to do it manually. You can run string.find(substring) in the loop and, after each found substring, to shorten the search range.
Here is the code for my solution:
from itertools import permutations

class Solution(object):
    """
    Leetcode-XXXXX solution
    """

    def find_chained_substrings(self, S, L):
        result = []
        for perm in permutations(L):
            chained_string = ''.join(perm)
            i = S.find(chained_string)
            while i != -1:
                result.append((i, perm))
                i = S[i+len(chained_string):].find(chained_string)
        return result

waka = 'foobarwakatttpingpong-ongpingpt'
lst = ['foo', 'bar', 'waka']
lst2 = ['t', 'pingp', 'ong']
S = Solution()
S.find_chained_substrings(waka, lst2)

It will return:
[(12, ('t', 'pingp', 'ong')), (22, ('ong', 'pingp', 't'))]
Note, that your Leetcode task needs only indices. I construct index-substring tuple for readability and simplier testing. If you want to make a Leetcode solution, replace this string:
result.append((i, perm))
with this:
result.append(i)

P.S. I strongly recommend you to read the Python Standard Library documentation. It will greatly improve your Python programming skill and will shift you from the C/C++ish style to Pythonic style.


Enums with different methods, useful or abuse? I'm wondering if the way I wrote our TextHelper class is a really a proper way to do it.
We have products that are interlinked in different ways and every product has multiple text fields that might be empty.

Now depending on where the text is supposed to show up, we want different texts.
A simple example would be:
On the product detail page it should show the shop specific text, if not available, the main description, if that is not available, the main description of it's parent product and the short description of the parents' subproducts.

So much for the motivation. I tried to shorten and simplify the code as much as I could. Is this a good way to use enums or should this be done completely different?

public class ProductTextHelper {
    public static String getTextInOrder(TextType[] order, IProduct p) {
        for (TextType t : order) {
            if (!t.getText(p).equals(""))
                return t.getText(p);
        }
    }

    public enum TextType {
        text1 {
            public String getText(IProduct p) {
                return p.getText1;
            }
        },
        text2 {
            public String getText(IProduct p) {
                String t = p.getText2;
                return t.equals("") ? "" : t + TextType.text1.getText(p);
            }
        },
        text3 {
            public String getText(IProduct p) {
                StringBuilder sb = new StringBuilder();
                sb.append(p.getText3());
                for (IProduct subP : p.getRelatedProducts()) {
                    sb.append(TextType.text2.getText(subP));
                }
            }
        },
        // more enums

        public static final TextType[] useCase1 = {text1, text2};
        public static final TextType[] useCase2 = {text4, text1, text3};
        public static final TextType[] useCase3 = {text3, text2};
        // a few more arrays

        public abstract String getText(IProduct p);
    }
}

And this gets called like that:

ProductTextHelper.getTextInOrder(TextType.usecase2, product);

My answer to your main question is you gotta do what you gotta do. Just so you feel more comfortable; this pattern has a name strategy enum. It is even in Effective Java.

But I have a criticism for this bit:

    public static final TextType[] useCase1 = {text1, text2};

Arrays are mutable. You should not implement class constants as final arrays. Use unmodifiable collections instead:

    public static final List<TextType> useCase1 = Collections.unmodifiableList(
         Arrays.asList(text1, text2));

Also each time you add another use case you should not need to modify this enum, that is by adding another useCase. Those should be moved to wherever they are used.


Edge cases for simulated malloc function I'm trying to simulate the malloc function in C by using mmap and having 2 linked lists to act as memory blocks. The program works by reserving a region of memory and then keeps track of the free and allocated memory address with size in the linked lists. I have tested the program with normal cases like

free memory > needed to be allocate
free memory < needed to be allocate
free memory == needed to be allocate

… but couldn't think of special cases.  Are there cases where the program fails?

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/mman.h>

struct block {
void *addr; /*start address of memory for this block */
int size;
struct block *next;
};

void *mem;
struct block *freelist;
struct block *allocated_list;

void *smalloc(unsigned int nbytes) {
struct block *cur = freelist;

 while(cur!=NULL){

        if (cur ->size >= nbytes){
            struct block *tempBlock;
            tempBlock= malloc(sizeof(struct block));
            tempBlock->addr = cur ->addr;
            tempBlock->size = nbytes;
            tempBlock->next = NULL;

            cur->addr = cur->addr + nbytes;
            cur ->size = cur ->size - nbytes;

                if(allocated_list == NULL){
                    allocated_list = tempBlock;
                }
                else{
                    cur = allocated_list;
                    while(cur->next!=NULL){
                        cur =cur->next;
                    }
                    cur->next=tempBlock;
                }

            return (tempBlock->addr);

        }
        cur = cur->next;

}

return NULL;
}

int sfree(void *addr) {
struct block *cur = allocated_list;
struct block *tempBlock;
while(cur!=NULL){
        if (cur ->next->addr == addr){
            tempBlock=cur->next;        //disjoint the matched block from the allocated_list
            cur->next = cur->next->next;
            tempBlock->next = NULL;     // set the matched block pointing to null

            cur = freelist;
            while(cur->next!=NULL){
                cur =cur->next;
                }
            cur->next=tempBlock;

            return(0);
        }
        cur= cur->next;

}
return -1;
}

void mem_init(int size) {
mem = mmap(NULL, size,  PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0);
if(mem == MAP_FAILED) {
     perror("mmap");
     exit(1);
}
else {
    freelist = malloc(sizeof(struct block));
    freelist ->size = size;
    freelist ->next = NULL;
    freelist ->addr = mem;

    allocated_list= NULL;

}

/* NOTE: this function is incomplete */

}

void mem_clean(){
struct block *cur = freelist;
struct block *tempBlock;
while(cur!=NULL){
    tempBlock = cur->next;
    munmap(cur->addr,cur->size);
    free(cur);
    cur = tempBlock;
}
cur = allocated_list;
while(cur!=NULL){
    tempBlock = cur->next;
    munmap(cur->addr,cur->size);
    free(cur);
    cur = tempBlock;
}

}

Not sure if it's just a result from copy-and-pasting the code here but the code formatting needs improvement.

Inconsistent indentation
Inconsistent use of spaces
Random empty lines

Clearly formatted code is a lot easier to read, maintain and is less prone to contain bugs.
Naming is inconsistent as well, e.g. allocated_list vs freelist vs tempBlock.
There is no need to place braces around the return value like this: return(0);. It should simply be return 0;. return is not a function call.
In order to append blocks to the end of each of the lists you currently iterate to find the end. It would pay off to keep a tail pointer pointing to the last element of each of the lists which avoids that.
I think there is a bug when calling sfree on the last block allocated. Example:

First call to smalloc allocates a new struct block and assigns it to allocated_list

This means allocated_list->next == NULL.

Call to sfree

cur points to allocated_list
cur != NULL
But cur->next->addr is invalid because cur->next == NULL

I think the cleaning up is technically broken. When initializing the structure you perform a single call to mmap to obtain a set of mapped memory pages. However on clean up you munmap every single block allocated by the user of smalloc. As per munmap man page:

  The implementation  will require that addr be a multiple of the page size {PAGESIZE}.

This can't be guaranteed because the passed in addr is not aligned at all as it is purely dependent on what was allocated by the user.

However I suspect it "simply works" because the first call to munmap unmaps the entire space mapped in originally with mmap because that's what the first addr of freelist is. Nevertheless the implementation is incorrect and works by pure coincidence.
You should check the return values of malloc and munmap as well.
Multiple calls to mem_init will leak memory. While it would certainly count as user error detecting this and notifying the user is good defensive programming.


Saving player name throughout script-launches I would like to use this in a program to save users' names even after they relaunch the script. I was just wondering if there were some ways I could clean up this code.

Note: this currently was made to not support name changing, but could possibly be implemented in the future.

if os.path.isfile('names.json'): # if there is a file with names...
    text_file = open("names.json") # only open it in normal mode
else: # if not...
    text_file = open("names.json", "w+") # make one

if os.stat("names.json").st_size > 0:
    for line in text_file:
        if line.istitle():
            player_name = line
        else:
            player_name = console.input_alert('What is your name? ').title()
            text_file = open("names.json", "w")
            text_file.write(player_name)
            text_file.close()
else:
    player_name = console.input_alert('What is your name? ').title()
    text_file = open("names.json", "w")
    text_file.write(player_name)
    text_file.close()

I'm not sure why you are reading all of the file and checking for istitle(). Here I make the assumption that if the file exists, it will always include the player name on the first line.

if os.path.isfile('names.json'):
  with open('names.json') as f:
    player_name = f.readline()
else:
  player_name = console.input_alert('What is your name? ').title()

with open('names.json', 'w') as f:
    f.write(player_name)

If you want to read the entire file and check for istitle():

player_name = None
if os.path.isfile('names.json'):
  with open('names.json') as f:
      for line in f.readlines():
          if line.istitle():
              player_name = line

with open('names.json', 'w') as f:
    if player_name is None:
      player_name = console.input_alert('What is your name? ').title()
    f.write(player_name)

You may notice that I do not close the files in either case. That is because the with statement takes care of that for us. See Understanding Python's "with" statement.