Generating a random maze using disjoint sets I've had a go at writing something that generates a random maze using disjoint sets using the grouping class I found here.

The way I'm generating the maze is by generating all the cells, with each cell having a wall to the right of it and beneath it. Each wall can either be active or not.

I pick wall at random and look at the pair of cells on either side of it to see if they are in the same group (connected), if they are, I leave the wall, if they aren't, I destroy the wall, and join the groups the neighbouring cells belonged to.

This will continue until all the cells are connected. I'm then trying to print the maze, probably in a really horrible way.

I'd really like to get some feedback on anything I could be doing better. Is it possible to make the code more efficient? If I do a 1000*1000 maze, it takes a very long time to calculate.

import random
from grouper import Grouper
import sys

X = 20
Y = 20

class Cell():
    """Represents a cell in the maze, with an x and y coordinate and its
    right hand wall and downwards wall.

    """
    def __init__(self, x, y):
        self.x, self.y = x, y
        self.right_wall = self.down_wall = None

class Wall():
    """Represents a wall in the maze with its two neighbouring cells.
    """
    def __init__(self):
        self.neighbours = None
        self.active = True

def popchoice(seq):
    """Takes an iterable and pops a random item from it.
    """
    return seq.pop(random.randrange(len(seq)))

# A mapping of coord tuple to Cell object
cells = {}
# A list of all the non-edge walls
walls = []

# Generate cells
for y in range(Y):
    for x in range(X):
        cells[(x, y)] = Cell(x, y)

# Generate walls and add to the neighbouring cells
for y in range(Y):
    for x in range(X):
        current_cell = cells[(x,y)]
        down_wall = Wall()
        current_cell.down_wall = down_wall
        right_wall = Wall()
        current_cell.right_wall = right_wall
        if y != Y-1:
            down_wall.neighbours = (current_cell, cells[(x,y+1)])
            walls.append(down_wall)

        if x != X-1:
            right_wall.neighbours = (current_cell, cells[(x+1,y)])
            walls.append(right_wall)

# Get a list of all the cell objects to give to the Grouper
cell_list = [cells[key] for key in cells]

maze = Grouper(cell_list)

for _ in range(len(walls)):
    # Pop a random wall from the list and get its neighbours
    wall = popchoice(walls)
    cell_1, cell_2 = wall.neighbours
    # If the cells on either side of the wall aren't already connected,
    # destroy the wall
    if not maze.joined(cell_1, cell_2):
        wall.active = False
        maze.join(cell_1, cell_2)

# Draw the maze

maze_map = []

x_max = (X*2)+1
y_max = (Y*2)+1

# Make an empty maze map with True for wall and False for space
# Make top wall
maze_map.append([True for _ in range(x_max)])
for y in range(1, y_max):
    # Make rows with left side wall
    maze_map.append([True]+[False for _ in range(1, x_max)])

# Add the down and right walls from each cell to the map
for coords, cell in cells.items():
    x, y = coords
    # Add the intersection wall for each cell (down 1 right 1)
    maze_map[(y*2)+2][(x*2)+2] = True
    if cell.right_wall.active:
        maze_map[(y*2)+1][(x*2)+2] = True
    if cell.down_wall.active:
        maze_map[(y*2)+2][(x*2)+1] = True

def output(string):
    sys.stdout.write(string)

# Print the map
for row in maze_map:
    for tick in row:
        if tick: output('X'),
        else: output('.'),
    output('\n')

It gives a maze like this:

XXXXXXXXXXXXXXXXXXXXX
X.......X...X.X.X...X
XXX.XXXXXXX.X.X.X.XXX
X.X...............X.X
X.XXXXXXXXX.XXXXX.X.X
X...X...X.X.X.......X
X.XXXXX.X.X.XXXXXXXXX
X.X.X.....X...X...X.X
X.X.XXX.XXX.XXXXX.X.X
X.X...X.........X...X
X.XXX.XXX.X.XXX.X.XXX
X...X.X.X.X.X.X.X.X.X
XXX.X.X.XXX.X.XXX.X.X
X.................X.X
XXXXX.XXX.XXXXX.X.X.X
X...X.X.X.X...X.X.X.X
XXX.XXX.X.XXX.X.XXX.X
X.X...X...X.....X...X
X.X.X.XXX.X.X.XXX.X.X
X...X.....X.X.....X.X
XXXXXXXXXXXXXXXXXXXXX

import random
from grouper import Grouper
import sys

X = 20
Y = 20

class Cell():

Either put object as the base class, or don't include the (). It doesn't really matter, but I think this is ugly.

    """Represents a cell in the maze, with an x and y coordinate and its
    right hand wall and downwards wall.

    """
    def __init__(self, x, y):
        self.x, self.y = x, y
        self.right_wall = self.down_wall = None

class Wall():
    """Represents a wall in the maze with its two neighbouring cells.
    """
    def __init__(self):
        self.neighbours = None
        self.active = True

def popchoice(seq):
    """Takes an iterable and pops a random item from it.
    """
    return seq.pop(random.randrange(len(seq)))

Call it pop_choice so that you don't have guess where one word begins and the other exists. Also it takes a sequence not an iterable

# A mapping of coord tuple to Cell object
cells = {}

I recommend looking at numpy. It has an array type. Basically, you can create an array like: cells = numpy.array( (Y,X), object) which will provide an efficent 2d array structure. That would be nicer to work with and faster then the dictionary you are using.

# A list of all the non-edge walls
walls = []

# Generate cells
for y in range(Y):
    for x in range(X):
        cells[(x, y)] = Cell(x, y)

You shouldn't do logic and loops in module scope. They are slower there then in functions.

# Generate walls and add to the neighbouring cells
for y in range(Y):
    for x in range(X):
        current_cell = cells[(x,y)]
        down_wall = Wall()
        current_cell.down_wall = down_wall
        right_wall = Wall()
        current_cell.right_wall = right_wall
        if y != Y-1:
            down_wall.neighbours = (current_cell, cells[(x,y+1)])
            walls.append(down_wall)

        if x != X-1:
            right_wall.neighbours = (current_cell, cells[(x+1,y)])
            walls.append(right_wall)

You create Walls in every case, but only sometimes add them to walls. This suggests either a bug or a misnamed walls variable.

# Get a list of all the cell objects to give to the Grouper
cell_list = [cells[key] for key in cells]

If using dict, this is the same as cell_list = cells.values() If you use numpy arrays like I suggest its cell_list = cells.flatten()

maze = Grouper(cell_list)

This variable really isn't holding the maze. Calling it maze is misleading

for _ in range(len(walls)):
    # Pop a random wall from the list and get its neighbours
    wall = popchoice(walls)
    cell_1, cell_2 = wall.neighbours
    # If the cells on either side of the wall aren't already connected,
    # destroy the wall
    if not maze.joined(cell_1, cell_2):
        wall.active = False
        maze.join(cell_1, cell_2)

Do it like this instead:

random.shuffle(walls)
for wall in walls:
    cell_1, cell_2 = wall.neighbours
    ...

That'll be more efficient and is clearer.

# Draw the maze

maze_map = []

x_max = (X*2)+1
y_max = (Y*2)+1

Its not clear why you need x_max, a comment would be helpful explaining that

# Make an empty maze map with True for wall and False for space
# Make top wall
maze_map.append([True for _ in range(x_max)])
for y in range(1, y_max):
    # Make rows with left side wall
    maze_map.append([True]+[False for _ in range(1, x_max)])

Here is another case that can really use numpy's ndarrays:

maze_map = numpy.zeros( (x_max, y_max), bool)
maze_map[0,:] = True
maze_map[:,0] = True

That does the same as all your maze_map code so far. Also, calling maze_map, blocked or something would make it clearer why you are putting True/False in it.

# Add the down and right walls from each cell to the map
for coords, cell in cells.items():
    x, y = coords
    # Add the intersection wall for each cell (down 1 right 1)
    maze_map[(y*2)+2][(x*2)+2] = True
    if cell.right_wall.active:
        maze_map[(y*2)+1][(x*2)+2] = True
    if cell.down_wall.active:
        maze_map[(y*2)+2][(x*2)+1] = True

The formula being used to figure out which cells are being reference is kinda hard to figure out.

def output(string):
    sys.stdout.write(string)

# Print the map
for row in maze_map:
    for tick in row:
        if tick: output('X'),
        else: output('.'),
    output('\n')

The following might be a clearer version for drawing:

def output(blocked):
    sys.stdout.write(".*"[blocked])

for x in range(2*X+1):
    output(True)
sys.stdout.write('\n')

for y in range(Y):
    output(True)
    for x in range(X):
        cell = cells[(x,y)]
        output(False)
        output(cell.right_wall.active)
    sys.stdout.write('\n')
    output(True)
    for x in range(X):
        cell = cells[(x,y)]
        output(cell.down_wall.active)
        output(True)
    sys.stdout.write('\n')

I think its a bit clearer because it doesn't have formulas like 2*x+1. It needs more comment but I trust you can handle that.

EDIT

For numpy, use it like this:

cells = numpy.empty( (X,Y), object)
# object is the python object type, not the object you want to put into it
# python's default is to use a number types which are more efficient
# but we aren't storing numbers
for x, y in numpy.ndindex(X, Y):
    cell[index] = Cell(x,y)

Speed:

The biggest time-consumer in the code is the grouper code. That code has opted to implemented the simpler rather then more efficient algorithm for the task. You should really implememt the union-find algorithm. That algorithm is crazy efficient, running in for what is all intents and purposes constant time.

Memory:

If memory is a concern, add slots to your classes. Look up the documentation for how to do it. However, I wouldn't bother unless you are really running out of memory.


Handling dialog closure in a VBA user form I already asked a Code Review question about Working with a new form instance every time.

As far as I have decided, that the most important part of the code is to work with the instance of the form and to learn how to handle them properly, I have remade the code (roughly using the code from @Mat's Mug) in a way that I am looking for a type of a boilerplate for forms, that I will probably use later.

Thus, let's start. I have a form, that looks like this:

Buttons are called btnStart, btnExit and lblInfo.

This is the code behind the frmMain:

Option Explicit

Public Event OnRunReport()
Public Event OnExit()

Private Sub btnRun_Click()
    RaiseEvent OnRunReport
End Sub

Private Sub btnExit_Click()
    RaiseEvent OnExit
End Sub

Private Sub UserForm_QueryClose(CloseMode As Integer, Cancel As Integer)

    End

End Sub

This is the code behind the modMain:

Option Explicit

Private objPresenter   As clsSummaryPresenter

Public Sub MainGenerateReport()

    Call objPresenter.ChangeLabelAndCaption("Starting and running...", "Running...")
    GenerateNumbers

End Sub

Public Sub GenerateNumbers()

    Dim lngLong         As Long
    Dim lngLong2        As Long

    tblMain.Cells.Clear

    For lngLong = 1 To 3000
        For lngLong2 = 1 To 10
            tblMain.Cells(lngLong, lngLong2) = lngLong * lngLong2
        Next lngLong2
    Next lngLong

End Sub

Public Sub ShowMainForm() 'CTRL+E

    If (objPresenter Is Nothing) Then Set objPresenter = New clsSummaryPresenter
    objPresenter.Show

End Sub

This is the code behind the clsSummaryPresenter:

   Option Explicit

    Private WithEvents objSummaryForm As frmMain

    Private Sub Class_Initialize()

        Set objSummaryForm = New frmMain

    End Sub

    Private Sub Class_Terminate()

        Set objSummaryForm = Nothing

    End Sub

    Public Sub Show()

        If Not objSummaryForm.Visible Then
            objSummaryForm.Show vbModeless
            objSummaryForm.lblInfo = "Press Run to Start"
            objSummaryForm.Caption = "Starting"
        End If

    End Sub

    Public Sub Hide()

        If objSummaryForm.Visible Then objSummaryForm.Hide

    End Sub

  Public Sub ChangeLabelAndCaption(strLabelInfo As String, strCaption As String)

    objSummaryForm.lblInfo = strLabelInfo
    objSummaryForm.Caption = strCaption
    objSummaryForm.Repaint

  End Sub

  Private Sub objSummaryForm_OnRunReport()

    MainGenerateReport
    Refresh

End Sub

Private Sub objSummaryForm_OnExit()

    Hide

End Sub

Public Sub Refresh()

    With objSummaryForm
        .lblInfo = "Ready"
        .Caption = "Task performed"
    End With

End Sub

What I am not sure (and I did not understand) was how to handle the red X in a way that there is no error after it. Thus, I have decided to use the End, which seems to work fine.

I have put the Excel file on GitHub.  It contains the same VBA code as shown above, I encourage you to first inspect it with macros disabled, as a good security practice with Excel files from the Internet.

I like how your form is no longer concerned with anything other than being a form.

One More Abstraction Level

I'd go one step further, and encapsulate the controls, so that the presenter class doesn't need to know about the implementation details of the form. For example the ChangeLabelAndCaption method knows that there's a Label control on the form, named lblInfo.

The form could expose a property for it instead:

Public Property Get InformationText() As String
    InformationText = lblInfo.Text
End Property

Public Property Let InformationText(ByVal value As String)
    lblInfo.Text = value
End Property

This adds a little layer of abstraction, so that the presenter no longer needs to care what actual controls are on the form, so if you decide to change the Label control for some Frame instead, nothing breaks - your code is now independent of the form's implementation details:

objSummaryForm.InformationText = strLabelInfo

For Closure's Sake

Private Sub UserForm_QueryClose(CloseMode As Integer, Cancel As Integer)

    End

End Sub

The End keyword is essentially analoguous to this:

Sure, that will get your form closed, and everything the VBA runtime has been doing will get released. But it's a tiny little wee bit excessive: the End keyword literally ends execution of all VBA code, right there and then.

The QueryClose event is fired just before the form closes, and its parameters give you a handle on the process.

It's just unfortunate that their types are Integer.

The CloseMode parameter tells you how the form is closing; its value is one of the VbQueryClose enum values:

vbAppTaskManager
vbAppWindows
vbFormCode
vbFormControlMenu
vbFormMDIForm

The interesting value - the one that means "user clicked the red X button", is vbFormControlMenu. The integer value for it is 0. So you can do this:

If CloseMode = 0 Then

Or better, this:

If CloseMode = vbFormControlMenu Then

And then handle form close yourself.

When the "red X button" is clicked, the form instance is normally destroyed. If the form contains state that the calling code wants to use, if you don't handle QueryClose and the calling code looks like this:

myForm = New MainForm
myForm.Show vbModal
Debug.Print myForm.SomeProperty

Then the last statement will be a runtime error, because myForm is Nothing at that point.

The Golden Rule

  He who creates an object, shall be responsible for its destruction.

If the form can destroy itself, the calling code is left needing awkward If myForm Is Nothing post-conditions to handle the case where it created an object that decided to hara-kiri.

A cleaner approach is to prevent the form from being able to destroy itself.

Private Sub UserForm_QueryClose(CloseMode As Integer, Cancel As Integer)
    If CloseMode = vbFormControlMenu Then
        Cancel = True
        Hide
    End If
End Sub

We set the Cancel parameter to True, thus preventing the destruction of the object, but only when the CloseMode flag indicates that the form was closed from the control menu. If we didn't call the Hide method, the form would simply refuse to close with the "red X button". This is useful for the rare circumstances where you don't want a form to be closable by the user.

The QueryClose handler signature would arguably be much clearer like this:

Private Sub UserForm_QueryClose(ByRef CloseMode As VbQueryClose, ByRef Cancel As Boolean)

Unfortunately that signature mismatches the interface declaration, and wouldn't compile. But handling the QueryClose event works exactly as if that's what the signature was.

Hungarian Notation

Your code suffers from the "bad" form of Hungarian Notation. While Systems Hungarian is traditionally acceptable for form controls (e.g. lblInformation, txtUserInput, btnOk, etc.; the prefix tells us what type of control we're looking at), using it in actual code for everyday variables, makes things messy:

Dim lngLong         As Long
Private WithEvents objSummaryForm As frmMain
Public Sub ChangeLabelAndCaption(strLabelInfo As String, strCaption As String)

Hungarian Notation was never intended to turn out this way (<~ must read!).

The guy that invented Hungarian Notation meant prefixes to describe the kind of variable, not their type. Apps Hungarian actually does make the code easier to read and understand, and makes wrong code, look wrong:

't: measured in twips
'p: measured in pixels

tArea = tHeight * tWidth 'obviously correct
tArea = pHeight * tWidth 'obviously wrong

Assuming the same context (twips vs pixels), compare to:

'int: integer
'lng: long

lngArea = intHeight * intWidth 'correct?
lngArea = lngHeight * intWidth 'correct?

When you prefix an object variable with obj, or a string with str, an integer with int, lng, or any other type-prefix, you add zero useful information to its identifier, and make the names read awkwardly.

Dim oRange As Range

Stop doing that.


Edit database content with a web page I'm new to PDO, and I want to make sure I made this in the correct way. This way is working, but I have a feeling it's unclean.

I have this input:

$query = $handler->query('SELECT * FROM content WHERE id=1');
while($r = $query->fetch()) {
?>

    <form action="<?php echo BASE_URL ?>admin/edit_process.php?edit_about" method="post" autocomplete="off" id="contact_form" name="contact_form" role="form">

          Izmeni sadržaj sa početne strane:
          <textarea class="form-control" id="message" name="message" required="" rows="6"><?php echo $r['edit_about']; ?></textarea>

          <button class="btn btn-success float-right" type="submit" name="submit">Sačuvaj</button>

    </form>

<?php
}

Then it goes to this:

if(isset($_REQUEST['edit_about'])){
    if(!$user->isLoggedIn()){ Redirect::to(''.BASE_URL.''); }
  if(!$user->hasPermission('admin')) { Redirect::to(''.BASE_URL.''); }
    if(Input::exists()){
        try{
      $message = $_POST['message'];
      $sql = "UPDATE `content` SET `edit_about` = :message WHERE `id` = 1";
      $query = $handler->prepare($sql);
      $query->execute(array(
          ':message' => $message
      ));
            Redirect::to(''.BASE_URL.'index');
        }catch(Exception $e){
            die($e->getMessage());
        }
    }
}

And this is the output:

<?php
$query = $handler->query('SELECT * FROM content WHERE id=1');
while($r = $query->fetch()) {
    echo $r['edit_about'];
}
?>

The biggest problem with your PDO code is error reporting. die($e->getMessage()); is the worst method possible. Citing from my article on error reporting (which I highly recommend to read):

for the casual user the error message is pretty cryptic and confusing
it it essentially useless for the site programmer/admin as they likely won't be browsing the site at the moment and will have no idea that error has been occurred
the information provided in the error message is extremely useful for a potential hacker, as it provides them with the feedback for their actions and may leak some sensitive information about your site

So never write a try..catch operator unless you have a certain scenario to be executed in the catch block as it will be useless and harmful.

Another suggestion is just a matter of taste. Given you have only one variable in the query, you can use a positional placeholder instead of a named one, sparing yourself writing some extra code:

if(Input::exists()){
  $sql = "UPDATE `content` SET `edit_about` = ? WHERE `id` = 1";
  $query = $handler->prepare($sql);
  $query->execute([$_POST['message']]);
  Redirect::to(BASE_URL.'index');
}

As you may notice, the PDO code looks a bit different from the other code you are using. With a simple PDO wrapper could make this code even simpler and uniform:

if(Input::exists()) {
    $sql = "UPDATE `content` SET `edit_about` = ? WHERE `id` = 1";
    DB::run($sql, [$_POST['message']]);
    Redirect::to(BASE_URL.'index');
}

The example is taken from my PDO wrapper usage examples but you can use any wrapper you like.

Also, I don't know which framework you are using, but $_POST['message'] seems a bit alien to me. I would expect something like Input::get('message') instead.


Checking the GridColumnType using a switch statement I am trying to refactor a switch statement. I know that you can replace a switch with a Dictionary, or a strategy pattern but it kind of feels like overkill for what I have.

Switch

var filters = new List<ColumnFilter>();
switch (gridColumnDataType)
                {
                    case "System.String":
                        if (filterColumnType != typeof(RuleTextNode))
                            filters.Add(columnFilter);
                        break;
                    case "System.DateTime":
                        if (filterColumnType != typeof(RuleDateNode))
                            filters.Add(columnFilter);
                        break;
                    case "System.Boolean":
                        if (filterColumnType != typeof(RuleBoolNode))
                            filters.Add(columnFilter);
                        break;
                    default:
                        if (gridColumnType.IsNumericType())
                            if (filterColumnType != typeof(RuleNumberNode))
                                filters.Add(columnFilter);
                        break;
                }
            }

Explanation

I have a grid and each grid column has its own datatype column. Like DataType = "String.DateTime" And there is a filter on each column. Now the problem is every now and again the filter an column does not have the same DataType. Thank you Infragistics.

What the switch is doing is checking what the GridColumnType is and if it's, let's say, a System.String, the FilterColumnType should be of type RuleTextNode. If it is not, I am just adding it in the ColumnFilter List.

All the RuleTypes have the same underlying type FilteringNode.

Having said all of that, what would be the best way the rewrite the switch to avoid the code duplication and the switch?

The only possible solution I can come up with, to shorten/simplify your code, is to extract the redundant code and place it in a separate method TryAddFilter for example:

private void TryAddFilter(FilteringNode node)
{
    if (filterColumnType != typeof(node))
        filters.Add(columnFilter);
}

And then call this method from the switch-statement:

switch (gridColumnDataType)
{
    case "System.String": TryAddFilter(RuleTextNode); break;
    case "System.DateTime": TryAddFilter(RuleDateNode); break;
    case "System.Boolean": TryAddFilter(RuleBoolNode); break;
    default:
        if (gridColumnType.IsNumericType())
            TryAddFilter(RuleNumberNode); break;
}


First non-repeated character Challenge description is here need to find the first non repeated character from the string:

Write a program which finds the first non-repeated character in a string.
Input sample:
The first argument is a path to a file. The file contains strings.
For example:
yellow
tooth

Output sample:
Print to stdout the first non-repeated character, one per line.
For example:
y
t

Please let me know can I improve my solution.
#include <iostream>
#include <fstream>
#include <string>
#include <map>
#include <cstring>

void findNonRepeatedChar( const std::string& record)
{
    auto pred= [&record] ( char a, char b){
                  return strchr( record.c_str(), a ) < strchr( record.c_str(), b );
                };
    std::map< char, int, decltype(pred) > stringMap(pred);
    for( auto elem : record )
    {
        auto it = stringMap.find( elem );
        if( it != std::end(stringMap) )
        {
            ++(it->second);
        }
        else
        {
            stringMap.insert( std::make_pair<char&,int>(elem, 1));
        }
    }
    for( auto elem : stringMap)
    {
        if(elem.second == 1)
        {
            std::cout<< elem.first << "\n";
            break;
        }
    }
}

void readInputFile( const char * fileName )
{
    std::ifstream inFile( fileName, std::ifstream::in );
    std::string record="tooth";

    if( !inFile.is_open())
    {
        std::cout<<"File open failed \n";
        return;
    }
    while( getline( inFile, record ) )
    {
        findNonRepeatedChar(record);
    }
}

int main( int argc, char* argv[] )
{
    if( argc < 2 )
    {
        std::cout<<"Usage:program_name input_file_name \n";
    }
    readInputFile(argv[1]);
}

I've reviewed your code and here's what I've found.

Reconsider your algorithm

It's not really necessary to iterate through all of the characters in the string, as the current code does.  Since the task is to find only the first non-repeated character, the algorithm can terminate as soon as it finds a non-repeated character.  A simple way to do that is this:

void findNonRepeatedChar( const std::string& record )
{
    for (std::string::size_type i = 0; i < record.size(); ++i) {
        if (record.rfind(record[i]) == i) {
            if (record.find(record[i]) == i) {
                std::cout << record[i] << '\n';
                return;
            }
        }
    }
}

Reconsider the interface

It is odd that a function named findNonRepeatedChar() does not actually return anything.  The code could be clearer if it actually returned the non-repeated letter and left the printing to the calling routine.

Don't instantiate an object that is not needed

There is no reason to instantiate the record to "tooth" (or any other value) within readInputFile.

Use more descriptive function names

The function named readInputFile does more than simply read the file and as mentioned before, the findNonRepeatedChar() function doesn't return anything but prints as a side effect.  I'd be inclined to omit readInputFile entirely, and put its contents within main and to change the interface to as previously mentioned.

Give the user useful help

The program is probably not really named program_name so the string that's printed if there aren't enough commmand line arguments should probably instead be:

std::cout << "Usage: " << argv[0] << " input_file_name\n";


Simplify : find if element is in tuples stored in list Is there a way to simplify this script using in keyword or anything like this:
searched_value = 3
data = [("a", 1), ("b",2), ("c",3)]

### can be simplified ?
test = None
for i in data:
    if i[1] == searched_value:
        test = i[0]
        break
###

print(test)

I don't think there is, except maybe unpacking the pairs:
test = None
for k, v in data:
    if v == searched_value:
        test = k
        break

You could use next, but I wouldn't call it simpler:
test = next((k for k, v in data if v == searched_value), None)


Installing "mods" with Python A little disclaimer: you're going to have a field day with this. It's horrible. Looking at it makes me want to vomit, and don't ask me to explain it, because I've long forgotten what I was thinking when I wrote it. That being said, I can't really find a way to shorten it of make it make sense. Perhaps someone could help me out?

A bit of a background:

This program is meant to install mods for a game called Supreme Commander 2. It is meant to install ANY mod. The following is true of all the mods:

The only archive type is the .scd. SCD's have the same encoding as .zip's, but with a 0% compression rate.
The way to install the mod is simple: There will always be at least one scd inside the first, and there may be more scd's inside that one. Any scd with folders inside it should be moved to

  C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata

There may be more than one scd to be moved per mod.

And that's it. This is something of a "programming challenge," but I am really looking for improvements upon the following code:

import os, zipfile
def installfromdownload(filename):
    global scdlist
    targetfolder = r"C:\Program Files (x86)\Mod Manager\Mods\f_" + filename[:-4]
    lentf = len(targetfolder)
    unzip(r"C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd", targetfolder)
    if checkdirs(targetfolder) == False:
        os.system("copy C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd" " C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata")
    else:
        newfolderonelist = []
        getscds(targetfolder)
        for file in scdlist:
            newfolderone = targetfolder + "\f_" + file[:-4]
            filepath = targetfolder + "\\" + file
            unzip(filepath, newfolderone)
            newfolderonelist.append(newfolderone)
        for newfolders in newfolderonelist:
            if os.checkdirs(newfolders) == False:
                newfolders = newfolders[lentf + 1:]
                for scds in scdlist:
                    if newfolder in scds == True:
                        scdpath = targetfolder + "\\" + scds
                        os.system("copy " + scdpath + " C:\Program Files (x86)\Steam\steamapps\common\supreme commander 2\gamedata")

Note that there is no error handling here, that is done when I call the function. The function unzip() is here:

def unzip(file, target):
    modscd = zipfile.ZipFile(file)
    makepath(target)
    modscd.extractall(target)

The function checkdirs() is here (note this is pretty bad, it only checks for periods in the file name, so please submit suggestions):

def checkdirs(target):
    isfile = 0
    targetlist = os.listdir(target)
    for files in targetlist:
        for letters in files:
            if letters == ".":
                isfile = isfile + 1
    if isfile == len(os.listdir(target)):
        return False
    else:
        return True

The getscds() function just checks a filename for ".scd".

Firstly, your installfromdownload function should be install_from_download (and have a docstring to go with it explaining what it does).

This line:

targetfolder = r"C:\Program Files (x86)\Mod Manager\Mods\f_" + filename[:-4]

should probably check and make sure that the file actually ends with .zip (or whatever format you're actually expecting it to come in):

if filename.endswith('.zip'):
    target_folder = ...

Something doesn't seem quite right with:

unzip(r"C:\Program Files (x86)\Mod Manager\Mods\z_" + filename + ".scd", targetfolder)

You're stripping the last 4 characters off filename before, and here you're appending '.scd'. I assumed you were stripping .scd off above, but then realized you're unzipping it, so it's actually a .zip (or something like that). Either way, it's confusing and something like:

target_folder = r"..." + filename.split('.zip')[0]

would make it much more clear what you're actually doing (commenting would help a bit too).

Don't explicitly check against False:

if checkdirs(targetfolder) == False:

This should be:

if not checkdirs(targetfolder):

Don't join paths together with "\\" (which is OS specific anyway):

 filepath = targetfolder + "\\" + file

Use os.path.join instead:

 filepath = os.path.join(target_folder, file)

Some of the variable names here make your code really, really hard to follow (newfolderonelist, newfolderone).

This line should make your script crash:

if os.checkdirs(newfolders) == False:

As you're trying to call your checkdirs function (os.checkdirs doesn't exist).

Let's look at the checkdirs function:

def checkdirs(target):
    isfile = 0
    targetlist = os.listdir(target)
    for files in targetlist:
        for letters in files:
            if letters == ".":
                isfile = isfile + 1
if isfile == len(os.listdir(target)):
    return False
else:
    return True

Again following Python naming convention, this should be check_dirs (although the name needs a lot of work - what's it checking them for?). Further, a lot of the work here can replaced with os.path.isfile. From reading it, it looks like you're checking that everything in a given directory is not a file, so I'm going to call it not_all_files:

def not_all_files(directory):
    '''Checks the given directory, returning False if it contains only files,
    and True otherwise. If the argument given is not a directory,
    raises IOError.
    '''

    if not os.path.isdir(directory)
        # Oops, passed in something that wasn't a directory
        # Handle this error
        raise IOError('....')
    return not all((os.path.isfile(f) for f in os.listdir(directory)))

Given how you're using it above, I'd rather swap around what it returns: have it return True if everything in the directory is a file, False otherwise. This would then be:

def all_files(directory):
    '''Checks the given directory, returning True if it contains only files,
    and False otherwise. If the argument given is not a directory,
    raises IOError.
    '''

    if not os.path.isdir(directory)
        # Oops, passed in something that wasn't a directory
        # Handle this error
        raise IOError('....')
    return all((os.path.isfile(f) for f in os.listdir(directory)))

Your code in install_from_download would then become:

 if all_files(target_folder):

which I think is much easier to read.


Creating a loading sequence of an application I am using Webpack to create a sort of loading sequence of the application. I can simply call prepareInfosite().render() and I don't need to care about the hidden async nature of it.

export function prepareInfosite() {
    return setupRenderer((resolve) => {
        require.ensure(['./infosite'], (require) => resolve(require('./infosite')), 'infosite');
    });
}

export function prepareLoading() {...}
export function prepareGame() {...}

const uiContainerElement = document.createElement('div');
document.body.appendChild(uiContainerElement);

function setupRenderer(resolver) {
    let uiModule = null;
    let rendered = false;
    const renderArgs = [uiContainerElement];

    function executeRender() {
        if (rendered === true && uiModule !== null) {
            uiModule.render(...renderArgs);
        }
    }

    resolver((resolvedUiModule) => {
        uiModule = resolvedUiModule;
        executeRender();
    });

    return {
        render(...args) {
            rendered = true;
            renderArgs.push(...args);
            executeRender();
        },
    };
}

Originally, the setupRenderer looked like this, but since I am using Babel, it has polyfilled the Promise and size has grown from like 6kB to 22kB (after uglify2!), which isn't very nice.

function setupRenderer(resolver) {
    const promise = new Promise(resolver);
    return {
        render(...args) {
            promise.then((uiModule) => uiModule.render(uiContainerElement, ...args));
        },
    };
}

Do you think there is a nicer way how to express this without any external dependency?

Additionally, I would like to DRY up the lines with require.ensure. I am not entirely sure how this works, but is there some way how to specify name of loaded file dynamically, perhaps using context?

Do you think there is a nicer way how to express this without any external dependency?

Use Babel preset to only include browsers from the last two years. In that case, the promise polyfill is not needed. For example

{
  "presets": [
    ["env", {
      "targets": {
        "browsers": ["since 2016"]
      }
    }]
  ]
}

  Additionally, I would like to DRY up the lines with require.ensure. I am not entirely sure how this works, but is there some way how to specify name of loaded file dynamically, perhaps using context?

Use a wrapper function to pass the strings as arguments:

return setupRenderer((resolve) => { ensurance('./infosite', 'infosite')});

And define the function before the call:

function ensurance(path, name){
  require.ensure([path], (require) => resolve(require(path)), name);
}

References

Babel: env preset
caniuse: promises


Implementation of Symbol Table in C Following is my code for implementing basic symbol table in C. Please review my code and tell where I can improve my program.

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

#define BUFFER_SIZE 1024
#define TOKEN_SIZE 32

typedef struct symbol
{
    char* identifierName;
    char* datatype;
    int offset;
    struct symbol* next;
}symbol;

void insert(symbol** headRef,char* identifier,char* type)
{
    symbol* newnode = (symbol*) malloc(sizeof(symbol));
    static symbol* prevnode;
    newnode->identifierName = (char*) malloc((strlen(identifier)+1)*sizeof(char));
    newnode->datatype = (char*) malloc((strlen(type)+1)*sizeof(char));

    if(*headRef==NULL)
    {
        newnode->offset=0;
        prevnode = newnode;
    }

    strcpy(newnode->identifierName,identifier);
    strcpy(newnode->datatype,type);

    if(strcmp(newnode->datatype,"int") == 0)
        (newnode->offset) = (prevnode->offset) + sizeof(int);
    else if(strcmp("float",newnode->datatype) == 0)
        (newnode->offset) = (prevnode->offset) + sizeof(float);
    else if(strcmp("char",newnode->datatype) == 0)
        (newnode->offset) = (prevnode->offset) + sizeof(char);
    else if(strcmp("double",newnode->datatype) == 0)
        (newnode->offset) = (prevnode->offset) + sizeof(double);

    prevnode = newnode;
    newnode->next = (*headRef);
    (*headRef) = newnode;
}

void display(symbol* node)
{
    while(node!=NULL)
    {
        printf("%s %s %d \n",node->identifierName,node->datatype,node->offset);
        node = node->next;
    }
}

bool isIdentifier(const char* token)
{
    if(isalpha(token[0]) || token[0]=='_')
    {
        for(int i=1;token[i]!=(char)NULL;i++)
        {
            if(!isalnum(token[i]))
                return false;
        }
        return true;
    }
    return false;
}

bool isDatatype(const char* token)
{
    const char *datatype[]={"char","double","float","int"};
    for(size_t i=0;i<(sizeof(datatype)/sizeof(char*));i++)
    {
        if(strcmp(token,datatype[i])==0)
            return true;
    }
    return false;
}

void isDeclaration(char* statement,symbol** head)
{
    char *delimiters = " ,;\t";
    char *token = strtok(statement,delimiters);
    if(!isDatatype(token))
        return;
    char *datatype = token;
    while((token=strtok(NULL,delimiters)))
        if(isIdentifier(token))
            insert(head,token,datatype);
}

FILE* openFile(int param1,char* param2[])
{
    if(param1<2)
    {
        fprintf(stderr,"No file specified to read. \n");
        exit( EXIT_FAILURE );
    }

    if(param1>2)
    {
        fprintf(stderr,"Too many arguments for read to perform. \n");
        exit( EXIT_FAILURE );
    }

    FILE *fp = fopen(param2[1],"r");

    if(fp==NULL)
    {
        fprintf(stderr,"File access denied on read. \n");
        exit( EXIT_FAILURE );
    }
    return fp;
}

int main(int argc, char *argv[])
{
    FILE *fp = openFile(argc,argv);

    char string[BUFFER_SIZE];
    symbol* head = NULL;

    while(fgets(string,sizeof string, fp))
        isDeclaration(string,&head);

    display(head);
    return 0;
}

I transcribed the comments from @user3629249 into this community wiki answer:

Regarding this line:

symbol* newnode = (symbol*) malloc(sizeof(symbol));

In C, do not cast the returned value from malloc and family of functions. This is because the returned value is a void*, so can be assigned to any other pointer and maintenance becomes much easier as there is no need to fix the casting.
Always check (!=NULL) the returned value to assure the operation was successful.
The expression sizeof(char) is always 1, so adds nothing to the value passed to malloc()

Regarding this line:

const char *datatype[]={"char","double","float","int"};

To avoid having the execution re-create this every time the function is called, add a static modifier. I.E. static const char *datatype[]={"char","double","float","int"};
Regarding:

char *token = strtok(statement,delimiters)

On every call to strtok(), always check (!=NULL) the returned value to assure the operation was successful. A failure of the call to strtok() will result in isIdentifier() trying to read from address 0. This is undefined behaviour and can lead to a seg fault event.
In general, when checking the parameters (as in the openfile() function) When the code finds a problem with the command line parameters, it should display a usage statement, so the user is informed of exactly what they should have done.
When a system function returns an error indication, such as the returned value from fopen(), the next line of code should be perror(); so the related system error message gets output to stderr.
When #defineing numeric values, always wrap the numeric value with parens so there are no 'text replacement' errors.
Due to each new token being inserted at the beginning of the linked list, when the list is displayed, the tokens will be displayed in the reverse order. Is this what you want?
A symbol table needs to contain info on any modifiers being applied to a symbol. including when a modifier is a function name, a returned value, a returned type, etc etc linked values, such as struct's enum's, union's, etc need links between the type, the name, and the associated fields with their types, names, and order. The current code handles none of that.
The isDeclaration() function will treat parens, braces, and other similar items incorrectly. The parsing needs to keep a 'state' history, so as to know how to treat the next token. (all part of lexical and syntax analysis) The posted code does not properly handle strings in double quotes and single quotes. The posted code does not handle a '\' at the end of an input line.
The max token length is set to 31 characters+string delimiter. what is this code was reading a C source file? while only the first 32 characters may be significant, an identifier can be much longer. the code will overrun the token buffer in such an instance, resulting in undefined behaviour, leading to a seg fault event.


Compare string with wildcard string I have the following function to compare a string with a wildcard string (containing ? and *), as C# doesn't seem to have a builtin function to do it.

    /// <summary>
    /// Compares wildcard to string
    /// </summary>
    /// <param name="WildString">String to compare</param>
    /// <param name="Mask">Wildcard mask (ex: *.jpg)</param>
    /// <returns>True if match found</returns>
    public static bool CompareWildcard(string WildString, string Mask, bool IgnoreCase = true)
    {
        int i = 0, k = 0;

        // Cannot continue with Mask empty
        if (string.IsNullOrEmpty(Mask))
            return false;

        // If WildString is null -> make it an empty string
        if (WildString == null)
            WildString = string.Empty;

        // If Mask is * and WildString isn't empty -> return true
        if (string.Compare(Mask, "*") == 0 && !string.IsNullOrEmpty(WildString))
            return true;

        // If Mask is ? and WildString length is 1 -> return true
        if (string.Compare(Mask, "?") == 0 && WildString.Length == 1)
            return true;

        // If WildString and Mask match -> no need to go any further
        if (string.Compare(WildString, Mask, IgnoreCase) == 0)
            return true;

        while (k != WildString.Length)
        {
            switch (Mask[i])
            {
                case '*':

                    if ((i + 1) == Mask.Length)
                        return true;

                    while (k != WildString.Length)
                    {
                        if (CompareWildcard(WildString.Substring(k + 1), Mask.Substring(i + 1), IgnoreCase))
                            return true;

                        k += 1;
                    }

                    return false;

                case '?':

                    break;

                default:

                    if (IgnoreCase == false && WildString[k] != Mask[i])
                        return false;

                    if (IgnoreCase && Char.ToLower(WildString[k]) != Char.ToLower(Mask[i]))
                        return false;

                    break;
            }

            i += 1;
            k += 1;
        }

        if (k == WildString.Length)
        {
            if (i == Mask.Length || Mask[i] == '*')
                return true;
        }

        return false;
    }

I added the check to make sure Mask isn't null/empty because if it is then switch (Mask[i]) will throw a IndexOutOfRangeException. I'm wondering if there are any other checks I should make or change (specifically during the while loop)?

What you want to do is called "globbing". It's actually quite easy to do with a Regex.

There are a few code smells here. The recursion is the obvious one. The other is the special-cases. When you special-case the single-character wildcard, it suggests that you're thinking about the problem wrong. There's no reason why they would need to -- or even should -- be specialcased.

If you really want to reinvent the wheel, the "right way" is to build a state machine and backtrack. I combined @NotPro's answer and @svick's hint to get http://ideone.com/VUDFIw but that uses backtracking. Check out http://swtch.com/~rsc/regexp/regexp1.html to see why backtracking is a bad idea, and how we can get an O(n) state machine.


Hello World Node application with error handling I wrote this simple server in node. Actually my goal is to better understand exception handling best practices in Node.
So my question is more specific.
Is this code bullet proof to exceptions? Are all errors or exceptions propertly handled?
How would you make it more safe? Where would you put try/catch?
Is there a place where try/catch is missing?

var http = require('http');

// Register handler for incoming requests.
var server = http.createServer(function(request, response) {

    var body = "";

    request.setEncoding("utf8");

    request.on('error', function(err) {
        console.error(err.stack);
    });

    // Listen for incoming data in the request.
    request.on('data', function(chunk) {
        body += chunk
    });

    // Has all data arrived?
    request.on('end', function() {

        // We should send response only when the whole request has been received, isn't it?
        response.writeHead(200, {
            'Content-Type': 'text/html',
            'X-Powered-By': 'bacon'
        });

        response.on('error', function(err) {
            console.error(err);
        });

        response.write('');
        response.write('');
        response.write('
Hello, World!
');
        response.write('
');
        response.write(body);
        response.write('
');
        response.write('');
        response.write('');
        response.end();

    });

});

// Listen on some port
server.listen(8085);

If you're going to continue using raw, low level, http then you've done good enough (although you should probably return a 500 response code back to the client). In reality though, you're unlikely to continue working with Node at such a low level. Most Node applications are going to use a framework like Express, which has the concept of Middleware. Typically, you'd use some middleware to handle runtime errors.

RisingStack has a great tutorial for creating a simple Express server with middleware error handling.


Turning an array of words into a random license plate This is an assignment question from school:

  Write a method called licencePlate that takes an array of objectionable words and returns a random licence plate that does not have any of those words in it. Your plate should consist of four letters, a space, then three numbers.

import java.util.*;

class MethodAssign5{
static String licensePlate(String[] a){
    char[] lchars={'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z'};
    char[] nchars={'0','1','2','3','4','5','6','7','8','9'};
    boolean contain = true;
    char[] license = new char[8];
    license[4]=' ';
    Random generator = new Random();
    do{
        for(int i=0;i<4;i++){
            license[i]=lchars[generator.nextInt(26)];
        }
        for(int i=0;i<3;i++){
            license[i+5]=nchars[generator.nextInt(10)];
        }
        contain = false;
        for(String s:a){
            boolean same = true;
            char[] wchars = s.toCharArray();
            for(int i=0;i<4;i++){
                if(license[i]!=wchars[i]){
                    same = false;
                }
            }
            if(same==true){
                contain = false;
            }
        }
    }while(contain==true);
    String ans = new String(license);
    return ans;
}
public static void main(String[]args){
    String[] words ={"HAHA","GORD"};
    System.out.println(licensePlate(words));
}
}

As you can see, I have no idea if my solution is right. It's such a low probability.

What do you think about my solution? What should I do if this was an in-class test and had the same problem where the probability of getting the error is low?

A few suggestions, and then I'll get to how to test it:

Don't be afraid of whitespace -- your code is a little hard to read due to the lack of new lines.  Also, it's fairly standard to indent method names inside of classes.
Always use descriptive names: String[] a tells me nothing about what that parameter is.
When x is a bool, x == true is equivalent to x.

On an extremely picky note, contains seems like a more natural variable name for contain (grammatically anyway)

It doesn't really matter when there's only 4 characters (so 4 * A.length runs), but when doing a linear comparison, you should typically bail out of it with break or continue

Also, you might want to look into indexOf, contains and equalsIgnoreCase

Obviously this is just a little homework assignment, so it would be a bit overkill, but I would put the license plate generation in its own class in a real application.
I would also pass the banned words as a parameter to the constructor instead of to a method -- that allows your instance to carry around the words without any farther down consumers having to know what the words are

Imagine that you make a machine that's used in the DMV.  Imagine that this machine has a "generate license plate" button.  When pressed, a screen simply shows the plate.

Passing the words to the method is like having to key in the banned words every time the button is pressed -- the DMV employee shouldn't be responsible for knowing/controlling those words
Storing the words inside of an instance (whether set with a setBannedWords method or passed to the constructor) is like configuring the machine at creation.  The DMV employee no longer has to know what the words are, or be responsible for entering them.  Years could go by before an employee realized "Oh my, I just noticed that the machine has never output a cuss word!"
For completeness: A third option would be to not filter the words -- it would then be up to the employee to go "Hrmmm, 'FART 743' is probably a bad license plate."

Ok, so does this work?  Probably.  It looks correct to my rusty-with-Java-eye anyway.

The easiest way to test it would probably be to whittle down the set of possible characters from A-Z to A-D and then make ABC a bad word.  After a lot of test runs, you could be fairly certain that it was throwing out any ABCX or XABC.

This just made me realize: did your professor specify that the bad words would always be four characters?  If not, your code is wrong.

So how could this be tested more properly-ish?  Unfortunately it requires that the code become a lot longer.  Your professor would probably think you got a bit carried away if you truly made this testable.  Despite a fairly simple requirement, to test this with granularity would require breaking it down into quite a few pieces.

I can code this up if you want, but for the sake of brevity and time, I'll just describe it here.

The main problem with testing this is that all of the concerns are mixed together.  You have a few concerns:

Generating a license plate

Generating the text part
Generating the number part

Filtering a license plate

Note that separating this concerns allows you to test them independently.

"Does my class properly generate and filter license plates?" becomes "Does class X properly generate license plates?  Does class Y properly filter license plates?"

The second option is easier to test because it allows you to pass in a license plate generator that is rigged to generate bad words.

The text and number generation should arguably be separated.  That would allow for easily reusing generated numbers when a bad word is in the word part, but other than that, the added complexity would probably not be worth it.

One option would be to use an interface:

public interface LicensePlateGenerator {
    public String generateLicensePlate();
}

You could then have your classes implement it:

public class LicensePlateGeneratorRandom implements LicensePlateGenerator {

    private final lchar[] = {'A', 'B', 'C', ...};

    private final nchar[] = {'0', '1', '2', ...};

    public String generateLicensePlate() {

        //Randomly grab 4 lchars
        //Randomly grab 3 nchars

    }

}

public class LicensePlateGeneratorFiltered implements LicensePlateGenerator {

    private final LicensePlateGenerator gen;
    private final List<String> badWords;

    public LicensePlateGeneratorFiltered(LicensePlateGenerator generator, List<String> badWords) {

        this.gen = generator;
        this.badWords = badWords;

    }

    public String generateLicensePlate() {

        //It might be a good idea to test how many plates have already been generated
        //An infinite loop of generating could happen depending on the underlying generator
        //and what kind of badWords are defined

        String lp;

        do {
            lp = gen.generateLicensePlate();
        } while (isBad(lp));

        return lp;

    }

    private boolean isBad(String lp) {
        //Return false if lp contains any badWords
        //and true otherwise
    }

}

Note how this is transparent to any consuming class.  A consuming class doesn't need to know whether it has a LicensePlateGeneratorRandom or LicensePlateGeneratorFiltered; it just needs to know it has a LicensePlateGenerator.

Note that this allows you to test very easily.  Testing your random generator means just checking a few outputs.  Checking your filter could be done by rigging a generator:

(I would probably define this as an anonymous class in actual testing code, but my Java is too rusty to remember the syntax for that :p)

public class LicensePlateGeneratorFake implements LicensePlateGenerator {

    private final List<String> words;
    private Iterator<String> currentPos;

    public LicensePlateGeneratorFake(List<String> words) {
        this.words = words;
        currentPos = this.words.iterator();
    }

    public function generateLicensePlate() {
        if (!currentPos.hasNext()) {
            //Something should happen here...
            //could always just loop back around, or it
            //might also be worth considering having the interface
            //declare a certain exception as being possible in this method.
            //(That would also allow a way out of the infinite loop in the
            //(filter generator)
            //throw new LicensePlateGeneratorException("...");
            //(that could be the base class, and then a LicensePlateGeneratorInfiniteLoopException
            //could be thrown in the filter class)
        }
        //Obviously the end of the iterator should be checked, but I'm lazy
        return currentPos.next() + " 123";
    }

}

You then configure this fake generator to generate a certain stream of words ("ABCD", "DCBA", etc).  Then, you pass this fake generator to the filter, and give the filter a list of words you know that the fake generator will generate.

If you tell the fake generator to generate "ABCD" and you tell the filter to reject "ABCD", you can then test success by whether or not the filter generator returns "ABCD" in any of the generations.

An alternative approach would be to have the filter not be a generator.  Instead, your consuming code would be responsible for configuring its own filters and then generating plates until a suitable one is found.

Both designs have fairly strong pros and cons.

(Note: I completely bastardized the class names in my examples.  In a real application, namespaces should be used.  The code formatting on here makes namespaces a bit clumsy though, so I didn't use them.)


Idiomatic Scala Generics I'm working on some Scala problems given here, specifically:

sealed abstract class Tree[+T]
case class Node[+T](value: T, left: Tree[T], right: Tree[T]) extends Tree[T] {
  override def toString = "T(" + value.toString + " " + left.toString + " " + right.toString + ")"
}
case object End extends Tree[Nothing] {
  override def toString = "."
}
object Node {
  def apply[T](value: T): Node[T] = Node(value, End, End)
}

While implementing an insert into a height balanced tree I have the following function:

def insert(node: Tree[Char], value: Char): Tree[Char] = {
    if(node.eq(End)){
      Node(value,End,End)
    }
    else{
      val internalNode = node.asInstanceOf[Node[Char]]
      if(internalNode.left.height == internalNode.right.height){

          if(Random.nextBoolean()){ //choose left
            internalNode.left = insert(internalNode.left,value)
          }else{
            internalNode.right = insert(internalNode.right,value)
          }

      }
      else if(internalNode.left.height > internalNode.right.height){ //insert into right
          internalNode.right = insert(internalNode.right,value)
      }
      else{ //insert into left
          internalNode.left = insert(internalNode.left,value)
      }
      internalNode
    }

  }

Could you please let me know how I could improve this code.

Is there a way I can avoid doing the casting in asInstanceOf?
How can I improve the if else checks?

General feedback is also much appreciated.

When you feel yourself going to use a cast, especially when combined with an if on the save variable, then you most likely want to use a match.

def insert(node: Tree[Char], value: Char): Tree[Char] = {
  node match {
    case End => // hand end case here
    case n: Node[Char] => // handle the node case here
    case _ => // throw an exception, or do nothing if this case can't happen
  }
}

Now the node case is complicated enough that you might extract a method for it. Otherwise, it looked fine to me.


Is it acceptable to pass an Exception into a function that then may or may not throw it? I am still getting the hang of Exception Handling in PHP; Here is the scenario where I am using the convention:
There are times where I have a SQL query that I need to turn into an object, so I wrote this utility class/method.

Class A {
    public static function getThatOneInstance(){
        $sql = "SELECT id FROM table WHERE condition=met LIMIT 1";
        $O = UtilsMysql::fetchIdIntoInstance($sql, __CLASS__, new Exception('Not Found!?'));
        return $O;
    }
}

Class UtilsMysql {
    public static function fetchIdIntoInstance($sql, $Class, Exception $NotFoundE = NULL){
        $O = NULL;
        $r = mysql_query($sql);
        if (!$r) throw new ExceptionMySQL($sql);
        if (mysql_num_rows($r) > 0) {
            list($id) = mysql_fetch_row($r);
            $O = new $Class($id);
        } elseif (!is_null($NotFoundE)) {
            throw $NotFoundE;
        }
        return $O;
    }
}

There are times, though, where if there is no result returned from the query it represents an Exceptional Circumstance, and so an Exception needs to be thrown. Because this is not uncommon, rather than always testing the returned value from the utility method, I have started passing in an Exception, that will be thrown if no results are found. This way, I have a contract with the function, if I pass in an exception, I can rely on the fact that it returns an Object (don't need to test it).

Is it acceptable to pass an Exception into a function that then may or may not throw it?

As Winston said, use full names for your variables.  Also keep the case consistent - then when you write $Object instead of $object it will be more obvious.

Static should almost never be used if you are writing object oriented code.  You basically have namespaced functions with this code.  Rather than write a static utils class for SQL I would write an SQL class (or use an existing one).  If you want objects of a class to be created PDO can do this for you.

As far as the exception goes:

No, you shouldn't do that.  Your code creates an object that it doesn't need to every time the function is called.  At the worst you would have passed in the exception string and instantiated with new Exception in fetchIdIntoInstance.

Main Recommendation

You are better of dealing with an exception after it has happened at a level where you know why it happened.

Keep your lower level functions simple:  If something strange happens throw an exception.  In your case I would assume that your sql should work in fetchIdIntoInstance and throw an exception if you can't make the new instance.

At a higher level database operations will have a meaning.  You will be doing something specific that is easily understood.  Maybe you will be adding a record to a table or reading an object that you expect to be in the database.  At this point you can handle the exception and write meaningful information to a log file or something similar.

You could catch exceptions in getThatOneInstance but it does not seem like the right place.  I think you should catch it higher up where you know what to do with it.


Singleton Pattern + unavoidable Public Constructor We have an object that needs to be converted to a different type before storing it in database. For that purpose, the library provides an annotation over the field that specifies the appropriate class[given as ClassName.class] to use for converting back & forth.

The class implements an interface of the library that has the marshall & unmarshall methods.

public class ObjectConverter implements Marshaller<Instant> {

    public static final ObjectConverter CONVERTER = new ObjectConverter();

    @Override
    public String marshall(Instant obj) {
        return obj.toString();
    }

    @Override
    public Instant unmarshall(Class<Instant> clazz, String obj) {
        return Instant.parse(obj);
    }
}

Now, we need to manually convert Instant to appropriate type at certain places. A straight-forward approach would be to create a new instance everywhere and invoke those public methods.

My colleague was suggesting to have a static final field of itself and use it wherever needed in order to avoid creating it each and every time. But, my argument was that nobody is stopping from creating a new instance, thus defeating the purpose of the static instance.

A better way to handle this, in case object creation is really costly would be to have a utility class that has static methods for converting back & forth. These can be invoked in both the marshaller and during manual conversions. Is there any better approach?

Is it a good practice to have both static instance and public constructor at the same time?

The question is: what do you want to achieve.

As you store the singleton in the database it seems to me that your singleton is semantic and not technical.

Technical singletons have that unified structure of static instance field, maybe lazy initialization and private constructor to ensure only ONE instance per JVM.

Semantical singletons do not have the assertion to be unique per object identity in the JVM. Their assertion is they BEHAVE the same EVERYWHERE and they have semantic identity (equals) in ANY JVM to ensure identity beyond JVM boundaries.

Often this kind of singleton is used to have a central entry point to a domain/module.

To avoid the public constructor in the semantic singleton I have separated the requirement "central entry point" from "object instantiation". In my design I have a SingletonX class that adresses "central entry point". Furthermore I have a class X that is more or less a POJO Finally I have an interface InterfaceX to redefine the public interface of class X. Class X should be resolved at runtime and is mostly out of scope to be instantiated through client code. Class X is the class to be serialized.


Can I jump like this? In my nested for loops below, it is sometimes necessary to exit the outer loop and then continue program execution. Since loop labels and other goto-like constructions are heavily discouraged, I'm a bit afraid of raptors. Is this usage OK? Is there a better way to structure the program?

my @objects = load_objects();
OUTER: for my $obj ( @objects ) {
    for my $element ( @{$obj->elements} ) {
        if ( is_case_1( $element ) ) {
            ...
        } elsif ( is_case_2( $element ) ) {
            ...
        } else {
            warn "invalid element: $element";
            last OUTER;
        }
        stuff_that_assumes_valid_element( $element );
    }
    stuff_that_assumes_valid_object( $obj );
}
store_objects( @objects );

In general, the code is OK, i.e. I don't think it must be changed. However, to increase clarity, I would do a couple of things differently:

It's probably better to write an is_valid() method for your objects, so that whole inner loop is a single function call. Then, you can use grep to shorten the outer loop like this:

    my @objects = load_objects();
    my @valid_objects = grep { $_->is_valid() } @objects;
    stuff_that_assumes_valid_object($_) for @valid_objects;
    store_objects(@valid_objects);

Also, depending on what stuff_that_assumes_valid_element does, you can either put that call into is_valid() or into stuff_that_assumes_valid_object. Either way, your top-level code works with objects and thus shouldn't know too much about single elements inside those objects, that's something the object itself should know and do (see Law of Demeter for reasoning).


Is this PHP Email Form Secure from injections or other abuses? I am new to PHP and put this contact page together using the w3Schools.com example as a guide along with a couple other sources. As far as I can tell, it works fine, but before I put it up on my page I would like to know if there is any potential for abuse. Any feedback is appreciated.

<!DOCTYPE html>

<?php include 'header.php'; ?>

<?php
require_once "Mail.php";
function spamCheck($field) {
    $field = filter_var($field, FILTER_SANITIZE_EMAIL);
    if (filter_var($field, FILTER_VALIDATE_EMAIL)) {
        return true;
    } else {
        return false;
    }
}

if (isset($_REQUEST['email'])) {
    $mailcheck = spamCheck($_REQUEST['email']);
    if ($mailcheck == false) {
        echo "Invalid Input";
    } else {
        $from = $_REQUEST['email'];
        $body = "From\n" . $from . "\n\n"  .  $_REQUEST['message'];
        $to = "<REMOVED>";
        $subject = "$_REQUEST['subject'];
        $host = "<REMOVED>";
        $port = "<REMOVED>";
        $username = "<REMOVED>";
        $password = "<REMOVED>";
        $headers = array('From' => $from, 'To' => $to, 'Subject' => $subject);
        $smtp = Mail::factory('smtp',array ('host' => $host,
             'port' => $port,
             'auth' => true,
             'username' => $username,
             'password' => $password));
        $mail = $smtp->send($to, $headers, $body);
        if (PEAR::isError($mail)) {
            echo("
" . $mail->getMessage() . "
");
        } else {
            echo("
Message successfully sent!
");
        }
}

} else {
    echo "
Send me an email if you like.

<form method='post' action='contact.php'>

Your Email: <input name='email' type='text'>
    Subject: <input name='subject' type='text'>

Message:

    <textarea name='message' rows='15' cols='40'>
    </textarea>

    <input type='submit'>

    </form>";
}
?>

<?php include "footer.php"?>

I use a blank input field in the form and hide it with css.  If the field exists upon form submit, I know it's spam and just ignore it.

<form method="post">
<input type="text" name="email" />
<input type="text" name="email2" style="display:none;" />
<input type="submit" />
</form>

<?php if (!$_POST['email2']) { // not spam } ?>

Of course this only works with bots and spammers who scrape sites to get all the input fields, but since I've implemented it, it's worked great.


Random string generator I decided to make a random string generator for fun, and it works for the most part. Since I'm an amateur at programming, I thought I would post the code to get some feedback on it, and hopefully improve any bad habits.

#include <iostream>
#include <string>
#include <ctime>

typedef unsigned int uint;

std::string randomString(uint l = 15, std::string charIndex = "abcdefghijklmnaoqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")
{
    // l and charIndex can be customized, but I've also initialized them.

    uint length = rand() % l + 1;
    // length of the string is a random value that can be up to 'l' characters.

    uint ri[15];
    /* array of random values that will be used to iterate through random
      indexes of 'charIndex' */

    for (uint i = 0; i < length; ++i)
        ri[i] = rand() % charIndex.length();
    // assigns a random number to each index of "ri"

    std::string rs = "";
    // random string that will be returned by this function

    for (uint i = 0; i < length; ++i)
        rs += charIndex[ri[i]];
    // appends a random amount of random characters to "rs"

    if (rs.empty()) randomString(l, charIndex);
    // if the outcome is empty, then redo the generation (this doesn't work, help?)
    else return rs;
}

int main()
{
    srand(time(NULL));
    std::cout << randomString() << std::endl;
}

Your comments are too long (line length).
Your code is also also seems untidy because of them. I would reduce the comments and have use self documenting code.

This seems to short. What happens if l is 2048?

uint ri[15];

I would use a std::vector<uint> here. You can initalize it with length so that it will be the correct size.

You do the iteration twice. Once to generate the random numbers. The second time to use the random number to access your set of characters. Why not do this in a single loop.

Here is your bug:

    if (rs.empty()) randomString(l, charIndex);
    else return rs;

Can't see it! Add braces. and use more vertical space. You should use {} around all sub blocks. It will help avoid issues in the long run.

    if (rs.empty()) {
        randomString(l, charIndex);
    }
    else {
        return rs;
    }

Do you see it now? You forgot the return in the first branch of the if statement. You only return a value in the else section. This means your class has undefined behavior.


Simulating MixIns with Java I am using the following technique for simulating mixins (or Traits) using Java and would like to know whether this is reasonable or just confusing to future developers.  Also, I believe a good naming scheme is the best documentation, so if it would help to change the nomenclature, please provide suggestions.

/** A mixin interface which your class should implement */
public interface IsResizableMixin {

    /**
     * The implementation, which your class should
     * instance and delegate calls to
     */
    public static class IsResizableMixinImpl implements IsResizableMixin {

        // A widget is e.g. a Swing Component, a GWT Widget, etc.
        private Widget w;

        pubic IsResizableMixinImpl(Widget w) {
            this.w = w;
        }

        public void resizeTo(int width, int height);
            /* for example */
            w.setWidth(width);
            w.setHeight(height);
            w.getParent().redraw();
        }
    }

    void resizeTo(int width, int height);
}

Then an example implementation of a Widget, one that is resizable:

public class Foo extends Widget implements IsResizableMixin {
    private IsResizableMixin mixin;
    /* ... */

    public Foo() {
        super();
        this.mixin = new IsResizableMixinImpl(this);
        /* ... */
    }

    /* ... */

    public void resizeTo(int w, int h) {
        mixin.resizeTo(w, h);
    }
}

A richer example would be a Widget that can be set "busy", that is, overlaid with a spinner and disabled.

public interface IsBusyMixin {
    public class IsBusyMixinImpl implements IsBusyMixin {
        private Widget w;
        private boolean isBusy = false;

        pubic IsBusyMixinImpl(Widget w) {
            this.w = w;
        }

        public void toggleBusy();
            isBusy = !isBusy();
            if (isBusy) {
                /* overlay the component with a spinner */
            } else {
                /* remove the spinner overlay */
            }
        }
    }

    void toggleBusy();
}

public class Foo extends Widget implements IsBusyMixin {
    // Other than the fact that this method exists,
    // Foo is unaware that it can be toggled to a busy state.
    public void toggleBusy() {
        this.mixin.toggleBusy();
    }
}

The benefits of this approach are:

Code reuse.  In the past, if we needed Foo functionality, we copy-pasted boilerplate code, but if a bug is discovered, we've lost track of everywhere it's been duplicated.
Delegate implementations can vary; we can have an implementation for a Swing Component and one for a GWT component, but their API is the same.
Field names don't overlap, which is one of the problems with multiple inheritance.

The problems with this approach are:

Possibly confusing.  My solution is to document and hope the developer sees it.
Method name overlapping, which is why multiple inheritance is bad (i.e., Widget may have a getSize() method, and IsResizeableMixin may also declare a getSize()).  The solution is to give mixin methods distinctive names.

This is just a simple example.  Putting the boolean in the implementing class would work just as well, but I am trying to factor that out of Foo.  If we decide to change how that state is stored, all IsBusyMixins would need to be updated.

Any pros/cons I'm missing?

Note that Java 8's default interface implementations don't cover this use-case since mixins must hold some state.

There's always a choice, even if you need some additional state:

You can add the class directly to the class and provide methods for it in the interface or go for your solution.

Adding to the class directly would look like this:

public interface IsBusyMixin {
    boolean isBusy();
    void dumbToggleBusy();

    void overlayTheComponentWithASpinner(); // implement here or not

    void removeTheSpinnerOverlay(); // implement here or not

    public void toggleBusy();
        dumbToggleBusy();
        if (isBusy) {
            overlayTheComponentWithASpinner();
        } else {
            removeTheSpinnerOverlay();
        }
    }
}

public class Foo extends Widget implements IsBusyMixin {
    @Getter private boolean isBusy;
    public void toggleBusy() {
        isBusy = !isBusy();
    }
}

I'd say, it's always somehow possible. However, it may get way uglier then your solution. Moreover, all interface methods must be public and this exposes too much of the inner state.

So when things get a bit more complicated, I wouldn't misuse default methods that hard and would go with your approach instead.

Then you may consider to to use a default method exposing the mixin and maybe another one like

default void toggleBusy() {
    this.mixin().toggleBusy();
}

  Method name overlapping, which is why multiple inheritance is bad (i.e., Widget may have a getSize() method, and IsResizeableMixin may also declare a getSize()). The solution is to give mixin methods distinctive names.

In case one of the methods always delegates to the other, you should probably keep the names the same. Otherwise it can get confusing whatever you do.

The name IsResizeableMixin should probably be just Resizeable or ResizeableMixin.


Shunting-yard algorithm for expression parser After reading about the Shunting-yard algorithm, I decided to try to make a expression parser, before trying to make a actual language parser, using it. The way i translated the algorithm into C++ code seems pretty compact, so  there is really not much code to post:

Shunting-yard algorithm(in C++):

#include <iostream>
#include <string>
#include<vector>
#include<map>
#include "list.h"

bool isInteger(char &c) {
    return (c >= '0') && (c <= '9');
}

int main() {
    std::vector<char> output;
    List<char> stack;
    List<char>::iterator it = stack.begin();

    std::string expr("2-2*3/6");
    std::map<char, int> op_precedence;
    op_precedence['+'] = 10;
    op_precedence['-'] = 10;
    op_precedence['*'] = 20;
    op_precedence['/'] = 20;

    for (char &c : expr) {
        if (isInteger(c)) {
            output.push_back(c);
        } else {
            if ((stack.size() > 0)) {
                if ((op_precedence[stack.top()] >= op_precedence[c])) {
                    output.push_back(stack.top());
                    stack.pop();
                    stack.push(c);
                } else if ((op_precedence[stack.top()] < op_precedence[c])) {
                    stack.push(c);
                }
            } else {
                stack.push(c);
            }
        }
    }
    for (it = stack.begin(); it != stack.end(); it++) {
        output.push_back(*it);
    }

    for (auto &i : output) {
        std::cout << i << ' ';
    }

    return 0;
}

The only thing I should note, is that the list.h I include, is not part of the standard library. That is a linked list class I finished up a few hours ago. If the code for the linked list is really necessary, I'll post it, but I don't think it will. I pretty much behaves like a normal linked list. In fact, you could exchange it with the standard linked list. Just replace pop() with pop_front(), push() with push_back(), and stack.top() with stack.front().

I also should mention that I have not included parenthesis yet, as I'm just trying to get the basics down first.

isInteger is not needed. Use std::isdigit.
Trust the logic. An else if condition is mutually exclusive with if condition. There is no need to test for it - you already know it is true. A simple else is enough. But see also the next point.
Notice that all code paths in the else clause necessarily push(c). Factor it out:

if ((stack.size() > 0) {
    if ((op_precedence[stack_top()] >= op_precedence[c])) {
        output.push_back(stack.top());
        stack.pop();
    }
}
stack.push(c);


Another dynamic array implementation in C I saw quite a few posts about making a dynamic array so I tried making my own for learning purposes.
Here it is:
#ifndef VECTOR_H
#define VECTOR_H

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

#ifndef VECTOR_RESERVE_SIZE
#define VECTOR_RESERVE_SIZE 4
#endif

//Declaration

typedef struct
{
        int size_of_each;
        int reserved;
        int used;
        void* data;
} vector_t;

bool vector_init(vector_t *, int);
void vector_append(vector_t *, void *);
void* vector_get(vector_t *, int );
void vector_set(vector_t *, int, void *);
void vector_free(vector_t *);

//Implementation

bool vector_init(vector_t *p, int s)
{
        p->size_of_each = s;
        p->reserved = VECTOR_RESERVE_SIZE;
        p->used = 0;
        p->data = malloc(VECTOR_RESERVE_SIZE * p->size_of_each);

        if (p->data != NULL) { return false; }
        else { return true; }
}

void vector_append(vector_t *p, void *d)
{
        if (p->used == p->reserved)
        {
                p->data = realloc(p->data, VECTOR_RESERVE_SIZE);
                //printf("reallocated \n");
                p->reserved += VECTOR_RESERVE_SIZE;
        }
        memcpy((void*)((char*)p->data + p->size_of_each * p->used),  // copy to back of the vector
                        d, p->size_of_each);

        p->used += 1;
}

void* vector_get(vector_t *p, int i)
{
        if (!(i > p->used))
        {
                return (void*)((char*)p->data + p->size_of_each * i);
        }
        else { return NULL; }
}

void vector_set(vector_t *p, int i, void *d)
{
        if (!(i > p->used))
        {
                memcpy((void*)((char*)p->data + p->size_of_each * i), d, p->size_of_each);
        }
}

void vector_free(vector_t *p)
{
        free(p->data);
}

#endif

Here is an example of its usage:
#include <stdio.h>

#define VECTOR_RESERVE_SIZE 3
#include "vector.h"

int main()
{
        int a = 321;
        int b = 45;
        int c = 21;
        int d = 31;
        int e = 71;

        vector_t v;
        vector_init(&v, sizeof (int));

        vector_append(&v, &a);
        vector_append(&v, &b);
        vector_append(&v, &c);
        vector_append(&v, &d);
        vector_append(&v, &e);

        printf("1st element is  %d \n", *(int*)vector_get(&v, 0));
        printf("2nd element is  %d \n", *(int*)vector_get(&v, 1));
        printf("3th element is  %d \n", *(int*)vector_get(&v, 2));
        printf("4th element is  %d \n", *(int*)vector_get(&v, 3));
        printf("5th element is  %d \n\n", *(int*)vector_get(&v, 4));

        vector_set(&v, 4, &a);
        printf("4th element after setting is  %d \n\n", *(int*)vector_get(&v, 4));

        //contiguous
        printf("size of int is: %d \n", sizeof (int));
        printf("address of 1st element: %p \n", vector_get(&v, 0));
        printf("address of 2nd element: %p \n", vector_get(&v, 1));
        printf("address of 3rd element: %p \n", vector_get(&v, 2));
        printf("address of 4th element: %p \n", vector_get(&v, 3));
        printf("address of 5th element: %p \n", vector_get(&v, 4));

        vector_free(&v);
}

I've written some C++ before and this was one of my first few projects in C.
I'd like to know overall how good my code is and whether I'm using good practices / conventions.
Edit:
this:
p->data = realloc(p->data, VECTOR_RESERVE_SIZE);
should be this:
p->data = realloc(p->data, (p->reserved  + VECTOR_RSERVE_SIZE) * P->size_of_each);
Edit: Maybe it's too late but, if people are still reviewing this I would really like to know how does this approach compare to std::vector.

In C, as in C++, object pointers can be freely converted to void*, so quite a few casts can be removed.
It's probably convenient to store data as a char*, since that's how we use it.  We should still use void* as external interface, of course.
This is a dangerous anti-pattern:

            p->data = realloc(p->data, VECTOR_RESERVE_SIZE);

If the realloc() fails, it returns a null pointer.  By overwriting p->data before we check for null, we leak the old value of p->data, which is no longer accessible.  And the null-check is also missing, so subsequent access is Undefined Behaviour.
Style-wise, I think I'd find (!(i > p->used)) easier to read as (i <= p->used).  And if (p->data != NULL) { return false; } else { return true; } as return !p->data;.
The example code should set a good example, and check the return value of vector_init() (and also vector_append() once you realise it needs to indicate success/failure) and react appropriately.  Otherwise, you'll find you're encouraging slapdash usage.


getint-like function You probably know the following problem: You have written a small program that runs in your console and want the user to enter something. of course, it is very inconvenient when the user types the wrong things, so we have to prepare for any small child hacking like a madman on the keyboard.
Most functions wait for a valid input, which results in the problem that nasty line breaks occur when the user ONLY presses the enter key.
the problem is also normally easy to solve under Linux: read a character with getchar, determined the position of the cursor with the ANSI escape sequences and provided the whole thing with a do while loop.
BUT: I was not able to find a simple function to read in numbers in the same elegant way. The function must be able to read in numbers, but also be able to prevent line breaks. to achieve this, I used getchar and store the characters in a char array, whose indices I then convert to the corresponding numbers, which I then add to the actual number.
But maybe there is an easier way? or you have ideas how to optimize the function?
#include <iostream>
#include "TheGameHeaders.h"

#define CLEAR_LINE printf("\033[K")
#define POSITION(Ze, Sp) printf("\033[%d;%dH", Ze, Sp)
#define CLEAR printf("\033[2J")

int getint(int pos_ze, int pos_sp, int qntty_of_incs)
{
    int i = 0;
    char fake_nbr;
    char fake_nbrs[qntty_of_incs];

    int real_nbr = 0;
    int real_nbrs[qntty_of_incs];

    do
    {
        POSITION(pos_ze, pos_sp);
        while((fake_nbr = getchar()) != '\n')
        {
            if(fake_nbr == '0' || fake_nbr == '1' || fake_nbr == '2' || fake_nbr == '3' || fake_nbr == '4' || fake_nbr == '5' || fake_nbr == '6' || fake_nbr == '7' || fake_nbr == '8' || fake_nbr == '9')
            {
                if(i <= (qntty_of_incs-=1))
                {
                    fake_nbrs[i] = fake_nbr;
                    i++;
                    qntty_of_incs++;
                }
                else
                {
                    return 1;
                }
            }
            else
            {
                return 0;
            }
        }
    }
    while(i < 1);

    for(int a = 0; a < i; a++)
    {
        fake_nbrs[a] -= '0';
        real_nbrs[a] = fake_nbrs[a];
    }

    int i_fix = i;
    int i_fix_2 = i-=1;

    for(int a = 0; a < i_fix; a++)
    {
        i = i_fix_2;
        real_nbrs[a] *= pow(10, i-=a);
    }

    for(int a = 0; a < i_fix; a++)
    {
        real_nbr += real_nbrs[a];
    }

    return real_nbr;
}

oh and pow is a simple function:
int pow(int base, int exponent)
{

    if(exponent == 0)
    {
        return 1;
    }

    exponent -= 1;
    int fixBase = base;

    for(int i = 0; i < exponent; i++)
    {
        fixBase *= base;
    }

    return fixBase;
}

Overview
Looks like you are reading data from the standard input and converting to a number. There is already code for that:
int val;
std::cin >> val;

You use C functionality throughout this code (printf/arrays/getchar) where there are better C++ alternatives (std::cout/vector/std::cin). A lot of this work can be delegated to standard functions.
Code Review
#include <iostream>

What are these headers and what do they define?
#include "TheGameHeaders.h"

This is very terminal specific.
#define CLEAR_LINE printf("\033[K")
#define POSITION(Ze, Sp) printf("\033[%d;%dH", Ze, Sp)
#define CLEAR printf("\033[2J")

A lot of terms might support this but it is not universal. Have a look at ncurses library; that will provide a terminal-neutral way to achieve all this. ncurses, and curses before it, look up these control characters from their terminal databases and retrieve the terminal-specific escape codes.

This is not standard C++:
    char fake_nbrs[qntty_of_incs];

If you want dynamic (run time sized) arrays, then you should be using std::vector<>

We have standard functions for checking for numbers:
        while((fake_nbr = getchar()) != '\n')
        {
            if(fake_nbr == '0' || fake_nbr == '1' || fake_nbr == '2' || fake_nbr == '3' || fake_nbr == '4' || fake_nbr == '5' || fake_nbr == '6' || fake_nbr == '7' || fake_nbr == '8' || fake_nbr == '9')

Have a look at:
   std::isdigit()

Returning valid integer numbers to indicate failure conditions is probably not a good idea.
                else
                {
                    return 1;
                }
            }
            else
            {
                return 0;
            }

How can the user of your code tell if this is real user input or an error?

You use several loops to convert the each number to its correct value. Why not do this in a single loop?
    for(int a = 0; a < i; a++)
    {
        fake_nbrs[a] -= '0';
        real_nbrs[a] = fake_nbrs[a];
    }

    int i_fix = i;
    int i_fix_2 = i-=1;

    for(int a = 0; a < i_fix; a++)
    {
        i = i_fix_2;
        real_nbrs[a] *= pow(10, i-=a);
    }

    for(int a = 0; a < i_fix; a++)
    {
        real_nbr += real_nbrs[a];
    }

Seems simpler like this:
   int result = 0
   for(int a = j; a > 0; a--) {
       result = (result * 10) + (fake_nbrs[a - 1] - '0');
   }

There already exists a standard power function in the maths library.
int pow(int base, int exponent)

Nice touch. Most people forget this part.
    if(exponent == 0)
    {
        return 1;
    }


Multi or n-armed bandit reinforcement learning in R I am learning about reinforcement learning and came across the first and simplest form of reinforcement learning system called multi-armed reinforcement learning (also called as n-armed bandit). I have read Sutton's book and an outlace.com tutorial regarding this learning. Outlace.com has written a nice Python implementation of this learning which I rewrote as an R program.

#multi/n-armed bandit algorithm with greedy search
#original code http://outlace.com/Reinforcement-Learning-Part-1/

iter=500
n=10
arms=runif(n)
eps=0.1
av=rep.int(1,n)
counts=rep.int(0,n)

#reward function
reward<-function(prob){
  reward=0
  for (i in 1:100){
    if(runif(1)<prob){
      reward=reward+1
    }
    return(reward)
  }
}

bestArm<-function(a){
  return(max(a))
}

runMean=NULL
for (i in 1:iter){
  if(runif(1) > eps){
    # exploitation!(use best arm)
    choice=bestArm(av)
    counts[choice]=counts[choice]+1
    k = counts[choice]
    #reward for n arms
    rwd =  reward(arms[choice])
    old_avg = av[choice]
    new_avg = old_avg + (1/k)*(rwd - old_avg) #update running avg
    av[choice] = new_avg

  }else{
    # exploration!(test all arm)
    # choose a random lever 10% of the time.

    #randomly chose an arm
    choice=sample(arms,1)
    counts[choice]=counts[choice]+1
    old_avg = av[choice]
    new_avg = old_avg + (1/k)*(rwd - old_avg) #update running avg
    av[choice] = new_avg
  }
  runMean[i]=mean(av)
}
plot(runMean)

I don't know whether the output is correct because I don't understand some parts of the code and just copy pasted it.

Python

choice = bestArm(av)
counts[choice] += 1
k = counts[choice]
rwd =  reward(arms[choice])
old_avg = av[choice]
new_avg = old_avg + (1/k)*(rwd - old_avg) #update running avg
av[choice] = new_avg

R

choice=bestArm(av)
counts[choice]=counts[choice]+1
k = counts[choice]
#reward for n arms
rwd =  reward(arms[choice])
old_avg = av[choice]
new_avg = old_avg + (1/k)*(rwd - old_avg) #update running avg
av[choice] = new_avg

It runs and doesn't show any errors even after using most of the original Python code.

There were quite a few things wrong with your implementation:

In both the if and else parts, choice ended up being a numeric (a non-integer, e.g. 0.6532) when it should be an integer. The problem was that a) you where using max instead of which.max inside bestArm and b) randomly picking a value from arms instead of its index.
Inside the else part, you were using k and rwd without setting them.
your computation of the running mean was not using weights as the python implementation does
This won't have a significant impact, but I think the python implementation had an issue: the observed mean rewards av should be initialized to zero, not one.

Here are other improvements I would suggest:

The if/else should be limited to the picking of the choice index. Everything else should be moved after the if/else as to not repeat code. This was a problem from the original python implementation you pointed to.
Instead of writing a script, you should write a function. It is a good way to isolate the inputs and outputs of your program, it gives you an easy way to test with different input parameters, to share your code, etc.
use spaces to make your code easier on the eyes. Spaces also avoid ambiguous situations for people who are not experts in R's syntax rules; for example, when someone sees a<-b, is it assigning b to a, or is it asking if a is less than -b? Maybe another way to convince you is to make you notice that R's compiled code does contain spaces (run print(lapply) for example); it shows that R's authors agreed that code readability was more important than compactness.
If you are slowly building a vector (runMean) and know in advance its final length, initialize it with that exact length: runMean <- rep(NA, iter). Otherwise, at each iteration R will have to create a longer object and this will significantly slow down your code for any large number of iterations.

So the code could look like this:

n.armed.bandit.sim <- function(iter = 500, n = 10, eps = 0.1, max.reward = 100) {

  arms <- runif(n)
  av <- rep(0, n)
  counts <- rep(0L, n)

  reward  <- function(prob) sum(runif(max.reward) < prob)
  bestArm <- which.max

  runMean <- rep(NA, iter)
  for (i in seq_len(iter)) {
    choice <- if(runif(1) > eps) bestArm(av) else sample(n, 1)
    counts[choice] <- counts[choice] + 1L
    k <- counts[choice]
    rwd <- reward(arms[choice])
    old_avg <- av[choice]
    new_avg <- old_avg + (1 / k) * (rwd - old_avg)
    av[choice] <- new_avg
    runMean[i] <- weighted.mean(av, counts / sum(counts))
  }
  plot(runMean)
}


Finding the length of the number of paths in a matrix I found this problem from leetcode.com. We are asked to find the longest increasing path in a matrix. I finally came up with a recursion to achieve this but I found that when I increase the rows and columns to a large size then my program takes a long time to run.

I am wondering if there is any way I can optimize my code for speed, or perhaps if there is a better approach I can consider. I know that some have used dynamic programming and depth first search to achieve good speeds but to be honest I haven't really gotten there yet in my programming level.

#include <algorithm>
#include <cmath>
#include <vector>
#include <iostream>
#include <random>

using namespace std;

#define MATRIX_SIZE 1000

void printPath() {

}

int check(int i, int j, vector<vector<int> > matrix, vector<vector<int> > length) {
    if (length[i][j] == -1) {
        int currentValue = matrix[i][j];

        int len = 0;
        if ((i > 0) && (matrix[i-1][j] > currentValue)) { // check up
            len = max(len, check(i-1, j, matrix, length));
        }
        if ((i + 1 < matrix.size()) && (matrix[i+1][j] > currentValue)) { // check left
            len = max(len, check(i+1, j, matrix, length));
        }
        if ((j > 0) && (matrix[i][j-1] > currentValue)) { // check down
            len = max(len, check(i, j-1, matrix, length));
        }
        if ((j+1 < matrix[i].size()) && (matrix[i][j+1] > currentValue)) { // check right
            len = max(len, check(i, j+1, matrix, length));
        }
        length[i][j] = len + 1; // Include current cell
    }
    return length[i][j];
}

int longestPath(vector<vector<int> > matrix) {
    int n = matrix[0].size();
    int m = matrix.size();
    vector<vector<int> > length(m, vector<int>(n,-1));

    int len = 0;
    for (int i = 0; i < m; i++) {
        for (int j = 0; j < n; j++) {
            len = max(len, check(i, j, matrix, length));
        }
    }
    return len;
}

int main() {
    // Specify the number of rows and columns of the matrix
    int n = MATRIX_SIZE;
    int m = MATRIX_SIZE;

    // Declare random number generator
    mt19937 gen(10);
    uniform_int_distribution<> dis(1, 1000000);

    // Fill matrix
    vector<vector<int> > matrix;
    for(int i = 0; i < m; i++) {
            vector<int> row;
        for(int j = 0; j < n; j++) {
            row.push_back(0);
        }
        matrix.push_back(row);
    }

    // Apply random number generator to create matrix
    for(int i = 0; i < m; i++) {
        for(int j = 0; j < n; j++) {
            matrix[i][j] = dis(gen);
        }
    }

    // Print matrix to see contents
//  for(int i = 0; i < m; i++) {
//      for(int j = 0; j < n; j++) {
//          cout << matrix[i][j] << " ";
//      }
//      cout << endl;
//  }
//  cout << endl;

    int result = longestPath(matrix);
    cout << result << endl;

}

Don't do using namespace std;.

void printPath() {

}

This empty function does not do anything, so better remove it to make the code cleaner.

// Print matrix to see contents
//  for(int i = 0; i < m; i++) {
//      for(int j = 0; j < n; j++) {
//          cout << matrix[i][j] << " ";
//      }
//      cout << endl;
//  }
//  cout << endl;

Commented code clutters the code and can confuse the reader. "Is it still needed? Then why is it commented out? If not, why is it still there?". Apparently it is not used anymore, and it also tells the reader nothing about the code, so better delete it.

// Specify the number of rows and columns of the matrix
int n = MATRIX_SIZE;
int m = MATRIX_SIZE;

The comment here should not be needed, because the code explains itself. Maybe it would be even more explicit if you named the variables rows and columns. If n and m do not change, and you have MATRIX_SIZE defined anyways, you could just use MATRIX_SIZE instead of the variables.

// Declare random number generator
mt19937 gen(10);
uniform_int_distribution<> dis(1, 1000000);

A more explicit variable name would make this comment obsolete. Less comments means less to read and makes the code easier to understand. Comments should tell the reader why the code does something, but the code should explain what it does itself. rng would be short and is a well known acronym for random number generator.