Numeric selfmade CAPTCHA in Windows application Nowadays many sites introduce CAPTCHAs (Completely Automated Public Turing test to tell Computers and Humans Apart) for securing their sites. I wish to introduce the technique  in windows forms, as expression evaluation CAPTCHA.

Approach:

Two randomly generated integers are loaded into two different labels
Place a random operator (+,-,*) in between them
Users are requested to provide the calculated result in a text box
Compare the user input and expression evaluation result for confirmation.

Private Sub Generate_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Generate.Click
    ' This function will generate the Numeric Captcha
    ' with two random number and one operator
    Dim Rand As New System.Random
    op = Rand.Next(0, 3)
    lblfirst.Text = Rand.Next(0, 10) 'first random number
    lblsecond.Text = Rand.Next(0, 10) 'second random number
    If op = 0 Then ' Operator selection
        lblop.Text = "+"
    ElseIf op = 1 Then
        lblop.Text = "-"
    ElseIf op = 2 Then
        lblop.Text = "*"
    End If
End Sub

Private Sub check_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles check.Click
    'Validating the user input with Random CAPTCHA
    Dim output As Integer
    If op = 0 Then ' choosing operator based on op
        output = CInt(lblfirst.Text) + CInt(lblsecond.Text)
    ElseIf op = 1 Then
        output = CInt(lblfirst.Text) - CInt(lblsecond.Text)
    ElseIf op = 2 Then
        output = CInt(lblfirst.Text) * CInt(lblsecond.Text)
    End If
    If output = CInt(txtResult.Text) Then
        MsgBox("Successfully varified")
    Else
        MsgBox("Varification Faild")
    End If
End Sub

Are there any suggestions on improving the technique?

Option Explicit On
From what's an option strict and explicit?

Option Strict "restricts implicit data type conversions to only widening conversions". See here. With this option enabled, you can't accidentally convert one data type to another that is less precise (e.g. from an Integer to a Byte). Again, an option that should be turned on by default.

So after turning Option Strict On you will get 2 errors at these lines
lblfirst.Text = Rand.Next(0, 10) 'first random number
lblsecond.Text = Rand.Next(0, 10) 'second random number

stating "Option Strict On prohibits implicit conversion of Integer to String" (my translation from german).
Now you might think that's a bad thing, but it isn't. It will just show you where you need some work to do.
And for this 2 errors it is also quite easy, you need only to call the ToString() method of the Integer which is returned by calling the Rand.Next() method and there is the next problem.
Variables should use camelCase for their names and also the names should be descriptive so you won't need any comments for these variables anymore, so let us do it right and rename op to operandType and also Rand to randomizer
Dim randomizer As New System.Random
Dim operandType as Integer = 0

operandType = randomizer.Next(0, 3)
lblfirst.Text = randomizer.Next(0, 10).ToString() 'first random number
lblsecond.Text = randomizer.Next(0, 10).ToString() 'second random number

But wait, you are creating Integers convert them to Strings which you assign to the Text property of the labels which you are converting to Integers again inside the check_Click() method. That can be done better. Let declare some variables on class level to do this.
Private firstNumber As Integer = 0
Private secondNumber As Integer = 0

and change the lines above to
firstNumber = randomizer.Next(0, 10)
lblfirst.Text = firstNumber.ToString() 'first random number

secondNumber = randomizer.Next(0, 10)
lblsecond.Text = secondNumber.ToString() 'second random number

now we can use firstNumber and secondNumber in the check_Click method without using the String to Integer conversation (I will later talk about this also).
But wait, we can do better... Why don't we calculate the desired result at the time we generate the captcha ? Let us declare another Integer variable to hold the result
Private internalResult As Integer = 0

and use it
If operandType = 0 Then ' Operator selection
    internalResult = firstNumber + secondNumber
ElseIf operandType = 1 Then
    internalResult = firstNumber - secondNumber
ElseIf operandType = 2 Then
    internalResult = firstNumber * secondNumber
End If

So looking at this If..ElseIf..ElseIf I see 2 things, operandType is in the Range[0..2] which should be expressed as an enum (which I will lend from ckuhn203's answer) and the If..ElseIf.. should be a Select Case . We need also to change
Private operandType As Integer

to
Private operandType As Operand

next we add the enum
Enum Operand
    Add
    Substract
    Multiply
End Enum

and implement the Select..Case
Select Case operandType
    Case Operand.Add
        internalResult = firstNumber + secondNumber
    Case Operand.Substract
        internalResult = firstNumber - secondNumber
    Case Operand.Multiply
        internalResult = firstNumber * secondNumber
End Select

Now let us talk about conversion from String to Integer by taking a look at the last few lines of your check_Click() method
If output = CInt(txtResult.Text) Then
    MsgBox("Successfully varified")
Else
    MsgBox("Varification Faild")
End If

You are calling CInt(txtResult.Text). Assume the user will enter "lala" in the textbox. This will result in an InvalidCastException. A better way would be to use Integer.TryParse() which as the name implies tries to parse a String to an Integer. If this succeeds the method returns true otherwise it will return false. So let us use it
If (Integer.TryParse(txtResult.Text, output) AndAlso output = internalResult) Then
    MsgBox("Successfully verified")
Else
    MsgBox("Verification failed")
End If

Wait, what is this AndAlso ? AndAlso performs a short-circuiting logical conjunction on two expressions. So this means, that the right part of the AndAlso will only be evaluated, if the left part is true.
As we have, putting aside the last part, only code which belongs to the captcha generating and checking, so it would be a good thing to create a class of it
Public Class Captcha

    Private Enum Operand
        Add
        Substract
        Multiply
    End Enum

    Private firstNumber As Integer = 0
    Private secondNumber As Integer = 0
    Private internalResult As Integer = 0
    Private operandType As Operand
    Private result As Integer = 0
    Private randomizer As New Random()

    Public Function IsValid(answer As String) As Boolean
        internalResult = CalculateResult()
        Return (Integer.TryParse(answer, result) AndAlso result = internalResult)
    End Function

    Private Function CalculateResult() As Integer
        Dim result As Integer = 0

        Select Case operandType
            Case Operand.Add
                result = firstNumber + secondNumber
            Case Operand.Substract
                result = firstNumber - secondNumber
            Case Operand.Multiply
                result = firstNumber * secondNumber
        End Select

        Return result
    End Function

    Public Function Generate() As String
        Dim generatedCaptcha As String = String.Empty

        operandType = CType(randomizer.Next(0, 3), Operand)

        Select Case operandType
            Case Operand.Add
                generatedCaptcha = " + "
            Case Operand.Substract
                generatedCaptcha = " - "
            Case Operand.Multiply
                generatedCaptcha = " * "
        End Select

        firstNumber = randomizer.Next(0, 10)
        secondNumber = randomizer.Next(0, 10)

        Return firstNumber.ToString() & generatedCaptcha & secondNumber.ToString()
    End Function
End Class

Now you need only one label to show the captcha which I have named captchaLabel and your code looks fine and tidy
Dim _captcha As New Captcha()

Private Sub Generate_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Generate.Click

    captchaLabel.Text = _captcha.Generate()

End Sub

Private Sub check_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles check.Click

    If _captcha.IsValid(txtResult.Text) Then
        MessageBox.Show("Successfully verified")
    Else
        MessageBox.Show("Verification Failed")
    End If

End Sub

Side Note
As I see that you are comming from VB6, I want to encourage you to learn the .NET way of these things. So skip using MsgBox(),CInt(), Mid() etc, as these are only kept to stay backward compatible.


Calculating the number of zeroes at the end of a factorial The code seems to work, but, I always get the "Time Limit Exceeded" error (the time limit defined is 6 seconds). I even tried reducing the number of iterations but am still running short on time. I am basically trying to find the number of 5's and 2's, to obtain 10.

#include <iostream>

int number_counter(long int x, int base)
{
long int val = x;
unsigned int counter = 0;
for(long int i = x; i >= 1;(!(i%base)?(i=i-base):i--))
{
    val =i;
    while((val % base) == 0)
    {
        counter++;
        val = val/base;
    }
}
return counter;

}

int Zfact(long int num)
{
    unsigned int fives, twos;
    fives = number_counter(num, 5);
    twos = number_counter(num, 2);
    return (fives>twos) ? twos : fives;
}
int main() {
    int T;                                  //T = Number of test cases.
    cin>>T;
    unsigned long int n;
    for(int i=0;i < T;i++)
    {
        cin>>n;
        int zeroes = Zfact(n);
        cout<<zeroes<<endl;
    }

    return 0;
    }

This challenge looks familiar - I think I answered a similar challenge on our sister site, Programming Puzzles and Code Golf.  Those answers may not be as readable, though.

There's a couple of pieces of good news:

You don't need to count 2's in the prime factorization.  There will always be more 2's than 5's, so you can never have an unpaired 5.
You don't need to iterate over every number up to num if you see that we're just computing n/5 + n/25 + n/125 + ... (integer division).  If we define f(n) = n/5 + f(n/5) and f(0) = 0, we have a much faster version.  That can be implemented recursively, or fairly easily converted to iterative form.


Prefix all lines on standard error Is this the best way to prefix all lines written by a Bash script to standard error?

#! /bin/bash

exec 3>&2
exec 2> >(sed 's/^/ERROR: /' >&3)

echo stdout
echo stderr >&2

I am wondering, if I have to close file descriptor 3.

There's not a lot to review here.  It's Valgrind clean, though that's not difficult to achieve.

Certainly consider closing stream 3, unless you want to keep a means to emit unprefixed stderr messages.

One shortcoming is that the shell won't wait for sed to finish when it terminates.  If the last process's output is still being filtered at this point, then you may fail to capture it - I observed this (repeatably) when running the program in Emacs compilation-mode.

One way you can improve on this if you're using GNU sed is by passing -u ("unbuffered") flag to sed.  If you don't have GNU sed but do have GNU coreutils, then the stdbuf utility could make sed line-buffered for you:

stdbuf -oL -iL sed 's/^/ERROR: /'

With any sed, we could (and should) close stderr when we terminate and wait for sed to finish to be sure of catching all error output.

Modified code

#!/bin/bash

exec 3>&2
exec 2> >(sed -u 's/^/ERROR: /' >&3)
exec 3>&-
# shellcheck disable=SC2064
trap "exec 2>&-; wait $!" EXIT

echo stdout
echo stderr >&2


Simple netwire program I've written a simple program to test out the capabilities of netwire (or rather, how to write programs using netwire). The goal is to have an application that shows a window, starting out as completely filled with red. Pressing space should switch from red to blue and vice versa. Pressing escape or closing the window should terminate the program. I've been able to write something that works (using Allegro, but that doesn't matter too much - the Allegro part can't be simplified, at least not by much), but it's gotten a bit lengthy for something that sounds so simple.

{-# LANGUAGE OverloadedStrings #-}

module Main(main) where

import Control.Wire hiding (when)
import Prelude hiding ((.),id)
import qualified Graphics.UI.Allegro.Raw as Al
import Data.Monoid
import Control.Monad.Reader
import qualified Data.Set as S
import Data.Set (Set)

iff True  a _  = a
iff False _ b  = b

data Game = Game {
    keyState :: Set Al.Key,
    isClosed :: Bool
}

data Result = GameColor Al.Color | Quit

colRed = Al.Color {Al.colorR = 1.0, Al.colorG = 0.0, Al.colorB = 0.0, Al.colorA = 1.0}
colBlue = Al.Color {Al.colorR = 0.0, Al.colorG = 0.0, Al.colorB = 1.0, Al.colorA = 1.0}

initG = Game { keyState = S.empty, isClosed = False }
addKey (g@ Game {keyState = s}) k = g {keyState = S.insert k s}

-- Produce when event occurs, inhibit otherwise
gameEvent :: Monoid e => (Game -> Bool) -> Event e (ReaderT Game IO) a
gameEvent f = mkFixM $ \_ a ->
  iff <$> asks f <*> (return $ Right a) <*> (return $ Left mempty)

windowClosed = gameEvent isClosed

keyPressed k = gameEvent (S.member k . keyState)

doQuit = pure Quit . (keyPressed Al.KeyEscape <|> windowClosed)
color = doColorRed

-- The once prevents infinite recursion in case space was pressed
doColorRed = pure (GameColor colRed) . (once <|> notE (keyPressed Al.KeySpace)) -->
              doColorBlue
doColorBlue = pure (GameColor colBlue) . (once <|> notE (keyPressed Al.KeySpace)) -->
                doColorRed

colorApp :: Wire () (ReaderT Game IO) a Result
colorApp = doQuit <|>
            color

main = do
  Al.initialize
  Al.installKeyboard
  Just window <- Al.createDisplay 800 600
  Al.setTargetBackbuffer window
  Just q <- Al.createEventQueue
  t <- Al.createTimer $ 1/60
  Al.registerEventSource q =<< Al.getKeyboardEventSource
  Al.registerEventSource q =<< Al.getDisplayEventSource window
  Al.registerEventSource q =<< Al.getTimerEventSource t
  Al.startTimer t
  loop colorApp clockSession q
  Al.destroyEventQueue q
  Al.destroyDisplay window where
    loop w session q = do
      game <- handleEvents initG q
      (mx, w', session') <- runReaderT (stepSession w session ()) game
      case mx of
         Left ex -> putStrLn $ "Inhibited: " ++ show ex -- this should never happen
         Right (GameColor c) -> do
           Al.clearToColor c
           Al.flipDisplay
           loop w' session' q
         Right Quit -> return ()
    handleEvents g q = do
      ev <- Al.waitForEvent q
      case ev of
        Al.KeyDownEvent{Al.eventKeycode = k} -> handleEvents (addKey g k) q
        Al.Timer{}                           -> return g
        Al.DisplayClose{}                    -> return g{isClosed = True}
        _                                    -> handleEvents g q

Your iff function would be better expressed with <|> or msum. They're for making a choice, so one of them ought be used in place of iff, I'm not sure where the True/False values are feeding out into iff, but I would maybe change them to feed out something that's a Monoid type or MonadPlus instead of Bool then use the appropriate choice technique.

You named a data type which appears to be the purpose of being a state Result, was this supposed to be a Command, Instruction, NextState, or Update instead perhaps? Result seems like a confusing name to me for what I (think?) that is.

data Update = Color (Float, Float, Float) | Quit | Reset | etc | etc | whatever
type Command = (Game -> Update -> IO Game) -- Presuming those functions which execute changes like doColorRed return an IO monad

I think you're missing a cohesive set of data models to present your system and it's possible states for which you could write functions to alter... When working with any UI presentation, it's usually best to create a model of your system and a separate view which responds to your model.

I would change:

  case mx of
     Left ex -> putStrLn $ "Inhibited: " ++ show ex -- this should never happen
     Right (GameColor c) -> do
       Al.clearToColor c
       Al.flipDisplay
       loop w' session' q
     Right Quit -> return ()

To:

toColor (r,g,b) = Al.Color {Al.colorR = r, Al.colorG = g, Al.colorB = b, Al.colorA = 1.0}
ProcessCommand g (Color c) = Al.clearToColor (toColor c) >> Al.flipDisplay >> (gameLoop g)
ProcessCommand g Quit = return ()

then where you do your case, make sure your Game data type (represented as g above) has those w', session', and q variables as a part of your Game data type so the gameLoop function can execute that loop given any Game

and your case can take advantage of the exception monad instead of doing a case...

fmap (processCommand g) (mx <|> Right $ someFunctionThatTakesMxAndCreatesACommandToPrintTheError mx)

Give some of these techniques a try and see how it cleans up for ya. After that perhaps you'll see a higher level design appear that will make the domain a little easier to model and approach.


Deleting rooms using nested promises I have a piece of code that has to deal with some nested promises and shared variables between them. This code is complicated by itself and because of promises syntax is even more difficult to read for those that haven't written it.

The code right now is something like this:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return function() {
    return User.findOne({ _id: userId })
    .then((userDeleted) => {
      return Promise.all(teamMembers.map((member) => {
        return User.findOne({ _id: member }).then((user) => {
          return {
            id: user._id,
            commonTeams: Utils.arrayIntersect(user.teams, userDeleted.teams).length !== 0
          };
        });
      }))
      .then((usersNoCommonTeam) => {
        return Promise.all(usersNoCommonTeam.map((user) => {
          if(user.commonTeams) {
            return Promise.resolve();
          }
          return Room.findOneAndRemove(
            {
              $and: [
                { type: 'one-to-one' },
                { owners: { $all: [user.id, userDeleted] } }
              ]
            });
        }));
      });
    })
    .then((roomsToDelete) => {
      return Promise.all(roomsToDelete.map((room) => {
        if(room) {
          return Membership.remove({ room: room._id });
        }
        return Promise.resolve();
      }));
    });
  };
};

someDatabaseLookups.then(_deleteOneToOneRooms(userId, teamMembers))

I have considered to split each then into functions so the overall code is simpler to read:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return function() {
     return User.findOne({ _id: userId })
     .then((deletedUser) => {
       return _getUsersWithNoCommonTeams(deletedUser, teamMembers);
     })
     .then((usersNoCommonTeam) => {
       return _getOneToOneRooms(userId, usersNoCommonTeam);
     })
     .then(_removeRooms);
  };
};

But I don't like having one-line functions only to give them names and at the end if you are reading the code for the first time, you will have to scroll up and down to know what is being done, which from my point of view makes it harder to understand.

So my question is, is there a convention on how to write the cleanest code when dealing with complicated nested promises that use shared variables?

If you have the possibility to use it, you could try the async / await pattern. It is however only part of the standard since ecmascript 2017.

The pattern allows you to wait for promise before continuing the execution path. This means you could rewrite your current code in the following syntax

// mock
const User = {
  findOne: async function( param ) {
    return new Promise(resolve => setTimeout(() => resolve( param ), 500));
  }
};

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return async function() {
    let user = await User.findOne({ _id: userId });
    for (const member of teamMembers) {
      let foundMember = await User.findOne({ _id: member });
      // do something with that member
    }
  };
};

const removeUser = async function() {
  return _deleteOneToOneRooms( 5, [10, 15, 16] )().then( () => console.log('done') );
};

// an async function returns a promise in itself, so from outside an async function, you can use the `then` syntax
removeUser().then(() => console.log('async also done'));
// this will be logged first
console.log('remove user was called, rest is async');

This only gives you an idea from how you could reformat your code using this pattern.

The last piece of code can then also be rewritten like:

const _deleteOneToOneRooms = function(userId, teamMembers) {
  return async function() {
     let deletedUser = await User.findOne({ _id: userId });
     let usersNoCommonTeam = await _getUsersWithNoCommonTeams(deletedUser, teamMembers);
     return await _getOneToOneRooms(userId, usersNoCommonTeam);
  };
};


Extracting first name, last name, phone number, and address from a form I have this:

public void InfoAssignment()
{
    //MessageBox.Show(firstName = firstNameBox.Text);
    firstName = firstNameBox.Text;
    lastName = lastNameBox.Text;
    phoneNumber = phoneBox.Text;
    address = addressBox.Text;

    //Check for text in firstNameBox
    if (!String.IsNullOrWhiteSpace(firstName))
    {
        firstName = firstNameBox.Text;
    }
    else
    {
        MessageBox.Show("Please enter a first name");
        return;
    }

    if (!String.IsNullOrWhiteSpace(lastName))
    {
        lastName = lastNameBox.Text;
    }
    else
    {
        MessageBox.Show("Please enter a last name");
        return;
    }

    if(!String.IsNullOrWhiteSpace(phoneNumber))
    {
        phoneNumber = phoneBox.Text;
    }
    else
    {
        MessageBox.Show("Please enter a phone number");
        return;
    }

    if (!String.IsNullOrWhiteSpace(address))
    {
        address = addressBox.Text;
    }
    else
    {
        MessageBox.Show("Please enter a phone number");
        return;
    }
    //firstName = firstNameBox.Text;
    //lastName = lastNameBox.Text;
}

It does what I need it to do but obviously this is basically repeating the same thing with only minor variations. I'm thinking there has to be a better way of which I'm unaware. Is there?

You could create a method for it!

bool Validate(string val, TextBox textBox, string description)
{
    if (!string.IsNullOrWhiteSpace(val))
    {
        val = textBox.Text;
    }
    else
    {
        MessageBox.Show("Please enter " + description");
        return false;
    }

    return true;
}

Then you could use it like so:

if (!Validate(firstName, firstNameBox, "a first name"))
   return;
if (!Validate(lastName, lastNameBox, "a last name"))
   return;


Update machine settings at various moments over time period I have the actions and the times in an instructions.js file:

module.exports = {
  temperature : {
    "00:00" : 100,
    "01:00" : 110,
    "02:00" : 120,
    "03:00" : 130
  },
  ventilation : {
    "00:00" : 0,
    "00:30" : 25,
    "01:30" : 75,
    "02:30" : 100
  },
  end : "03:30"
}

Then after importing the instructions,

function adjustMachineParams() {
  if (instructions.temperature[machineTime()]) {
    var temp = instructions.temperature[machineTime()];
    machine.setTemperature(temp);
  }
  if (instructions. ventilation[machineTime()]) {
    var vent = instructions.ventilation[machineTime()];
    machine.setVentilation(vent);
  }
  if (machineTime() == instructions.end) {
    machine.stop();
  }
}

function machineTime() {
  var date = new Date(null);
  date.setSeconds(machineTimer/10); // machineTimer is in deciseconds
  return date.toISOString().substr(14, 5);
}

And finally, the machine loop...

var machineLoop = setInterval(function() {
    adjustMachineParams()
    machineTimer += 10;
  }
}, 1000)

Looking for feedback on the way that I have set up the instructions and also the general strategy and elegance (or lack of it) of the code.

To be honest: Not a very elegant approach.

Let's have a look at a different hypothetical sample data to see where it breaks:

module.exports = {
  temperature : {
    "00:00" : 100,
    "01:05" : 110,
  },
  ventilation : {
    "00:00" : 0,
    "00:30" : 25,
  },
  end : "01:35"
}

And the machine did not stop. Why?

Because you are only handling multiples of 10 seconds, and everything in between is never matched by your string comparisons.

Specifying the times as strings for readability in the config module is reasonable, so let's keep this as a requirement for now.

This leaves us with either increasing the sample rate on the timer, or with refactoring the logic which handles the state at any given point in time. Since the sample rate can't be increased arbitrarily, let's go with refactoring the logic.

Tracking the machines state is the key here. Right now, your state only contains machineTimer. However, it should also contain some information which allows to decide which instructions have been executed yet, and which don't.

For simplicity, let's just call that one machineLastUpdate, and assume that everything up to the time stamp recorded in that variable has already run.

With that, simply reverse the logic. Instead of converting machineTimer to string, instead convert the times from the configuration into seconds. For every converted time stamp, execute the correlating command exactly when timestamp > machineLastUpdate && timestamp <= machineTimer.

This approach ensures that all defined commands are executed exactly once - regardless of whether your timer can provide that resolution.

  // machineTimer is in deciseconds

This is somewhat pointless in Javascript, it's a double precision float anyway.

Unless you have a good reason to specifically use deciseconds, just stick with the established standard of using seconds as the unit for all time stamps.

machineTimer += 10;

Beware that setInterval can have a drift, or may even skip invocations when missing the time window. If you need accuracy for longer runs, instead better sample the system time at every invocation instead to determine the real time offset since the machine was started.


JavaScript dynamic  creation I've been focusing on my PHP skills lately but have been shifting to JavaScript.  I'm familiar with the bare-bone basics of jQuery.  I'm not as familiar with JavaScript as I'd like to be.  I'm a solo-developer so I'd just like somebody to take a look at this and point out any mistakes or things I could be doing better.

What the code does

Creates an arbitrary number of <script> elements, assigns a type and src attribute to each one and then inserts that <script> element before the </body>.

The code

See Better Code below!

function async_init() {

    var element, type, src;
    var parent = document.getElementsByTagName('body');
    var cdn = new Array;

    cdn[0] = 'http://code.jquery.com/jquery-1.6.2.js';
    cdn[1] = 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jquery-ui.js';

    for (var i in cdn) {
        element = document.createElement('script');
        type    = document.createAttribute('type');
        src     = document.createAttribute('src');

        type.nodeValue = 'text/javascript';
        src.nodeValue  = cdn[i];

        element.setAttributeNode(type);
        element.setAttributeNode(src);
        document.body.insertBefore(element, parent);
    }

}

<body onload="async_init()">

Right now the code is working, which is great.  But my questions:

Is this the "best" way to do this?  I define best as optimum user experience and efficient code.
Are there any obvious "noobie" flaws in my JavaScript?
Is there anyway I could get the onload attribute out of the body?

I used Mozilla Developer Network as my JavaScript reference.  If there are any other references that are accurate, documented & useful I would love to have more information.

Thanks for checking the code out and your feedback, even if its pointing out how my code sucks!

Better code after critique

function async_init() {
    var element;
    var cdn = new Array;

    cdn[0] = 'http://code.jquery.com/jquery-1.6.2.js';
    cdn[1] = 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jquery-ui.js';

    for (var i in cdn) {
        element = document.createElement('script');
        element.setAttribute('type', 'text/javascript');
        element.setAttribute('src', cdn[i]);
        document.body.appendChild(element);
    }

}

You've erred many times in the above code. However, that means you get to learn a lot about how to properly interact with the DOM. In many instances, there are built-ins that quickly and efficiently get the job done.

First off, you're incorrectly accessing the body. document.body is a well-supported reference to the body that's been around since at least DOM Level 1.

Secondly, you're taking the hard route towards creating an array. In his book, "JavaScript: The Good Parts", Douglas Crockford insists on using literals instead of constructors. In this case, an array literal is []. This is an easier way of calling new Array().

Thirdly, you're incorrectly iterating over the cdn array with a for...in loop instead of a for loop. A for...in loop is meant for objects as it iterates over the properties of an object. Not only is it slower than a normal for loop, but it also is very susceptible to prototype modification. For example, try running this code with a build of MooTools (jsFiddle has one enabled by default):

for(var key in [])
{
    console.log(key);
}

You'll get a ton of hits because MooTools modifies the Array prototype. You'd have to do a hasOwnProperty check for each property just to avoid those pitfalls. Avoid for...in on anything but objects.

Next, declaring the i variable inside of the for...in loop gives the illusion of block scope. This isn't the case, since i is available anywhere in the function after it's been defined. Only functions have scope in JavaScript. Here are Crockford's thoughts via "...The Good Parts" (page 102):

  In most languages, it is generally best to declare variables at the first site of use. That turns out to be a bad practice in JavaScript because it does not have block scope. It is better to declare all variables at the top of each function.

Finally, setting the attributes for each script tag isn't necessary. Since you're already interacting with the DOM, setting their cousin, the DOM property is a better idea. The DOM provides shortcuts to node properties and is more stable than setting attributes. get/set/removeAttribute all have the unfortunate shortcoming of being very weird in various Internet Explorer builds. They're best avoided unless completely necessary.

Fixed + optimized:

function async_init() {
    var element;
    var parent = document.body;
    var cdn = ["http://code.jquery.com/jquery-1.6.2.js", "https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jquery-ui.js"];
    var i = 0, file;
    for (i;i<cdn.length;i++) {
        file = cdn[i];
        element = document.createElement("script");
        element.type = "text/javascript";
        element.src  = file;
        parent.appendChild(element);
        //free file's value
        file = null;
    }
    /*
    * Cleanup at end of function, which isn't necessary unless lots of memory is being
    * used up.
    * No point nulling a Number, however. In some ECMAScript implementations
    * (ActionScript 3), this throws a warning.
    */
    //empty array
    cdn.splice(0, cdn.length);
    //clear variable values
    element = null;
    parent = null;
    cdn = null;
}

Also note that since you're adding these scripts asynchronously, there's no stable way to detect when the file has loaded, short of a script loader or a timer (my opinion is neither are stable).

An alternative is to use document.write to append these scripts synchonously. However, you'll have to use a DOMString and escape the forward slash in the script's end tag. This will append it to the end of the body tag.

Example: document.write("<script type=\"text/javascript\" src=\"\"><\/script>");

Note that the backslash character (\) escapes characters in JavaScript. I've used it to escape the double quotes for each attribute along with the forward slash in the end tag.

If you have any more questions regarding JavaScript or the DOM, feel free to ask in the comments.

-Matt


Anagram palindrome checking I need to figure out if any anagram of the string can be a palindrome or not.

Ideas:

It relies on two observations:

Frequency of every character in the string is even, if length of the string is even.
Frequency of every character in the string is even except for one character, where frequency must be odd if the string length is odd.

#include <iostream>
#include <algorithm>
#include <string>
using namespace std;

bool sortAsc(char c, char d) { return c<d; }
int main() {
    string s;
    cin>>s;
    sort(s.begin(), s.end(), sortAsc);
    int flag = 0;
    string::size_type i=0;

    //Even - number of characters whose frequencies are even
    //Odd - number of characters whose frequencies are even
    int even=0, odd=0;

    //The number of times the loop runs is size-1
    //because of the reason below
    while(i<s.size()-1) {
        int temp=0;

        //checking if the next char is same as current
        //if i will be size-1 then s[i+1] will be out of bound
        while(s[i]==s[i+1]) {
            ++temp;
            ++i;
        }

        //if the last element has only one occurrence
        //then it should be counted as a character with odd occurrence
        if(i==s.size()-2) {
            //don't increment temp if
            //i is the second last element, just skip everything outside
            if(s[i]!=s[i+1]) { ++odd; ++i; continue;}
        }

        ++temp;
        if(temp%2==0) { ++even; } else { ++odd; }
        ++i;
    }

    if(s.size()%2==0 && odd == 0 || s.size()%2 != 0 && odd == 1)
        flag=1;
    else
        flag=0;

    //I could have directly echoed them to console
    //but its just the way here
    if(flag==0)
        cout<<"NO";
    else
        cout<<"YES";

    return 0;
}

As you can see the following block is only used once but condition is checked in every iteration.

if(i==s.size()-2) {

    if(s[i]!=s[i+1]) { ++odd; ++i; continue;}

}

It just just there because s[i+1] if i==size will return error.

Question:

Is there any way I can loop until the last index and can still check if the last element has any occurrences?
Can it be done better without the initial sorting?

I think you might be overcomplicating the whole thing.

How do you define a palindrome? No matter which direction you're reading it, it's the same word. As you mentioned already, this means that all occurences of characters have to be even, except for one, which might be odd. However, the string length doesn't matter for this (it's just a logical consequence):

All characters with even occurences don't matter and can be ignored. You only have to count the number of characters who've got an odd count.

With no odd count: abccba - this is a palindrome.
With one odd count: abcba - this is a palindrome as well.
With two odd counts: abcdba - this isn't a palindrome.

So the task boils down to one simple task: Count the number of odd character counts.

Some code things I've noticed:

Try to avoid using using namespace. It might bite you in the back once you've got ambigious code and accidently call (or pass) the wrong function and you're unable to find the reason for some bug.
Try to be more consistent with your whitespace characters. In some places you've put them there, in other cases you've omitted them. Overall this makes your code quite a bit harder to read (at least for me).
Don't use single-char variables (a, i, k, etc.) unless they're iterators. This isn't JavaScript. You usually don't have to optimize code size. Instead use talking variable names, even if it's just "index" or "offset".
I'd also try to use a constant value rather than calling specific members over and over again, if you don't expect them to change. In your case this would be the check inside your loop: while(i<s.size()-1) { You don't know what's happening inside std::string::size(). While highly unlikely, this might actually iterate over your whole string every loop to determine the current length, making your code even more inefficient. Just ensure your compiler knows it's going to be constant:

  const std::string::size_type length = s.size();
  while(i < length - 1) {
      // ...
  }

In your solution you start by sorting the whole string (which is most likely O(n²)). After this you're iterating over all elements a second time to do the actual counting (O(n)).

To me this is quite suboptimal, especially if you're able to use some memory for this.

I'd try something like the following code, which will iterate over the input string and count the occurences of every character (O(n)). In a second loop I count the number of even and odd occurences (O(1)).

#include <iostream>

int main(int argc, char **argv) {
    // Let's just assume argv[1] holds the string

    // This will hold all character counts, initialized to 0
    unsigned char counts[256] = {0};

    // Iterate over the string...
    for (unsigned char *pos = reinterpret_cast<unsigned char *>(argv[1]); *pos; ++pos)
        ++counts[*pos]; // ...and count

    // Counts for even/odd:
    // - Even -> index 0
    // - Odd -> index 1
    unsigned int eo[2] = {0};

    // Now iterate over all characters
    for (unsigned int i = 0; i < 256; ++i)
        ++eo[counts[i] % 2];

    if (eo[1] > 1) { // More than one odd count - can't be an anagram
        std::cout << "No anagram can be a palindrome, since there are multiple unique characters." << std::endl;
        return 0;
    }

    // At most one odd count; this can be an anagram
    std::cout << "There might be an anagram that is a palindrome." << std::endl;
    return 0;
}

The big advantage here in my opinion is the fact that there's no branching outside any loop construct (except that one final check).

You could however, just skip the counting of even/odd counts and use a single bool to determine whether you've got a second occurence of an odd count (which would mean that you don't have any palindrome).


Alternative std::function implementation for lambdas I was trying to implement a faster, alternative implementation of std::function. I came up with the code below:
template <typename R, typename ...Args>
struct Function {
    template <typename Lambda>
    Function(Lambda f) {
    static auto function = f;
    func = + [] (Args... args) -> R {
        return function(args...);
    };
    };
    Function(const Function& other) : func(other.func) {}
    inline auto operator() (Args... args) {
        return func(args...);
    }
    R (*func) (Args... args);
};

Now you can instantiate a Function like below:
int y = 100;
int x = 56;
auto f1 = Function<int , int>([&] (int z) {return x + y + z});
// If you need to store a generic custom functor class:
auto functor = SomeCustomFunctorClass<int, int>(); // Assuming functor takes an int as an argument and returns an int
auto f2 = Function<int , int>([functor] (int x) {return functor(x);});

Note that my implementation Function can only store lambdas, but storing a generic lambda can be achieved through the example above.
The idea here is to erase the type of the lambda in the constructor by storing it statically, which means that I can use it the function pointer func. Note that, a different static variable is instantiated each time the constructor is called because lambdas all have a unique type, so each time a new version of the constructor is instantiated with a different constructor.
I've benchmarked this implementation, and it runs significantly faster than std::function (with optimisations enabled). It is up to 6 - 7 times faster for construction, 15 times faster for copying, and calling it is as fast as calling a function pointer (though significantly slower than calling a lambda which is always inlined).
However, this almost seems too easy, with this implementation being only 15 lines of code. Are there any drawbacks to my implementation compared to the traditional implementation of std::function using heap allocation and virtual calls? Perhaps statically allocating the lambda is not a good idea?

Questions & answers
“Are there any drawbacks to my implementation compared to the traditional implementation of std::function using heap allocation and virtual calls? Perhaps statically allocating the lambda is not a good idea?”
Yes, definitely a bad idea, with numerous drawbacks.
Because there are no compile-time checks to ensure that you’re actually using a lambda, this type is very brittle and very dangerous. Today I may write code like this:
auto x = 0;
auto y = 0;

auto func_x = [&x] { ++(*x); };
auto func_y = [&y] { ++(*y); };

// ... later:
auto function_x = Function<void>{func_x};
auto function_y = Function<void>{func_y};

No problems there. (Well, I mean, yes, problems, but not for this specific presumed use case. But we’ll get back to that.)
But then, later in the development process, someone refactors those two lambdas into this:
struct indirect_incrementer
{
    int* p_val = nullptr;

    auto operator()() { ++(*p_val); }
};

// then the following two lines:
//auto func_x = [&x] { ++(*x); };
//auto func_y = [&y] { ++(*y); };
// become:
auto func_x = indirect_incrementer{&x};
auto func_y = indirect_incrementer{&y};

It’s an innocent and perfectly logical refactor… but now everything blows up, because your assumption that every function object type Function is constructed with no longer holds. The static variable function is initialized with a copy of func_x, and so calling function_y does not call func_y, it calls func_x.
This isn’t just a problem with using function objects. Even if you somehow completely banned the use of function objects with Function—like, say, with some kind of linter or something—and made 100% sure that Function is always called with a legit lambda object, the problem still exists. Because despite your thinking, it is possible for a lambda type to be non-unique. It’s even quite trivial to do… just call the function that contains the lambda more than once:
auto render()
{
    // ... [snip] ...

    // somewhere in your render function, you use Function:
    auto func = Function<int, int>{[&] (int i) { return ++i; }};

    // ... [snip] ...
}

// elsewhere, in your main game loop:
while (not done)
{
    input();
    update();
    render(); // <- !!!
}

render() is called in a loop, but only the first loop will initialize the static function variable. Meaning every other time you use func, it will be with dangling references to the locals in that first render() call.
Now you could “fix” that problem with various hacks, basically detecting whether function was previously initialized, or counting the number of times it was initialized, or something like that. But it won’t fix the other problems.
The other big problem with using a static variable to hold a copy of the lambda is that you’re quietly cheating C++’s scope rules. This could lead to frustrating and nasty surprises. For example:
auto p_weak = std::weak_ptr<int>{};

// in some more restricted scope:
{
    auto p = std::make_shared<int>();
    p_weak = p;

    auto func = Function<void>{[p] { if (p) ++(*p); }};

    // use func somehow

    // scope is ending, so func and p are being destroyed, right?
    //
    // ... *right*?
}

if (auto p = p_weak.lock(); p)
    std::cerr << "memory leak!";

Turns out, p is never actually destructed; the lambda takes a copy, which is then copied into the static function variable, which then holds on to it… forever (well, until after main() returns, at least). This is not just a shared_ptr problem; I just used shared_ptr to be able to access otherwise lost memory. Any value that gets captured by the lambda is never destructed during the lifetime of the program. In other words, the more you use Function—even when every use avoids the other problems mentioned—the more you leak memory.
That’s probably why you’re seeing such large increases in speed: the function object is only being copy-constructed once—even when that’s incorrect—and it’s never being destructed.
(Also, std::function has a ton of very important and useful other abilities and benefits Function doesn’t offer, like the ability to be null-constructed, and reseated. Mere type-erasure is useful, I suppose, but probably not useful enough on its own… especially when simple function pointers can do that (and more!).)
So yes, using a static variable to get around dynamic allocation is not a good idea.
Code review
template <typename Lambda>
Function(Lambda f) {
static auto function = f;
func = + [] (Args... args) -> R {
    return function(args...);
};
};

Proper indentation would definitely help make this more readable. It would also highlight the stray semicolon you have at the end.
The fact that you’re copying f into function makes the whole class unusable with move-only lambda types:
auto p = std::make_unique<int>();

auto lambda_p = [p = std::move(p)] { if (p) { *p = 42; } };

auto func_p = Function<void>{lambda_p}; // won't compile

// (also, as mentioned previously, p won't ever be freed)

You should also use forwarding references in all the function calls. Right now every function argument will be taken by-value, which will be at least slow, and probably won’t compile in a lot of cases.
Function(const Function& other) : func(other.func) {}

This is entirely unnecessary.
inline auto operator() (Args... args) {
    return func(args...);
}

The inline keyword does nothing here.
Again, you should really be using forwarding references.
The whole interface of Function is also rather clunky. It seems specious to have to give the return and argument types, when they’re obvious and deduce-able from the lambda itself. It would be much nicer to be able to write:
auto f1 = Function{[&] (int z) {return x + y + z}};
// f1 is deduced as Function<int, int>

This is a situation where deduction guides would be useful.
Summary
You can’t get away with avoiding having function object wrappers actually wrap the function objects. Put that way, it should seem pretty obvious.
Your type-erasing wrapper either needs to dynamically allocate the memory to copy a function object, or use the small object optimization, or both. Trying to hide the wrapped object in a static variable is cheating. You may get away with it for a little while… but you will eventually be caught, either via memory leaks, reading/writing dangling references, or simply reading/writing to the wrong objects.


Convert a matrix ("DataFrame") to printable string While I understand that "V is a simple language," I find myself repeating a lot of stuff. For instance the whole first block, "measure column widths," could beautifully be accomplished by two simple list comprehension lines in Python. Is there some way to simplify the allocate array, initialize the index pointer, loop-assign, increase index pointer code? Any equivalent to Python's enumerate() would go a long way...
What do I do about the pad thing? Sure, it will work in 99.9% instances, but would be nice with an arbitrary length solution (such as ' '*n in Python).
Performance-wise I'm not concerned for this function in particular, but I'm curious if there are any obvious blunders? It looks close to C to me, and it's probably only cache handling that could be a problem? I'm assuming the three mutable arrays end up on the heap? If so: anything I can do about that?
I'm left with the sense that "simple language" in this case is equivalent to "hard-to-read implementation." With just a few more features it could be both simple and easy to read? What is your impression?
pub fn (df DataFrame) str() string {
    // measure column widths
    mut width := []int{len: 1 + df.cols.len}
    mut str_sers := []Series{len: 1 + df.cols.len}
    mut sers := [df.index]
    sers << df.cols
    mut i := 0
    for ser in sers {
        str_sers[i] = ser.as_str()
        mut row_strs := str_sers[i].get_str().clone()
        row_strs << [ser.name]
        width[i] = maxlen(row_strs)
        i++
    }

    // columns
    pad := '                                        '
    mut row_strs := []string{len: sers.len}
    i = 0
    for ser in sers {
        w := width[i]
        row_strs[i] = pad[0..(w - ser.name.len)] + ser.name
        i++
    }
    mut s := row_strs.join('  ')

    // cell data
    l := df.len()
    if l == 0 {
        s += '\n[empty DataFrame]'
    }
    for r in 0 .. l {
        i = 0
        for ser in str_sers {
            w := width[i]
            row_strs[i] = pad[0..(w - ser.get_str()[r].len)] + ser.get_str()[r]
            i++
        }
        s += '\n' + row_strs.join('  ')
    }
    return s
}

V already has a built-in enumeration for plain arrays:
for i, a in arr {
}

Which removes six unnecessary lines of code.
List comprehension would remove another dozen lines, and what little I understand about V's map/reduce, it seems very limited (think lambda without closure) so that is not an option. Python's list comprehension with zip is an efficient way to reduce dumb code, but perhaps it defeats V's "only one way to do things." And perhaps this is a rare scenario.


Continuously receive messages async from Azure Service Bus Queues I'd like to get input on my approach of continuously receiving messages from an Azure Service Bus queue using the async part of the library.
My main concern being whether its "safe" to use Task.Factory.StartNew to process the incoming messages and then continue to call the Action's to continue receiving new messages.

The processor class is started like this:

public void Start()
{
    ThrowIfDisposed();
    lock (_lockObject)
    {
        Trace.WriteLine(string.Format("Started '{0}' allocation.", _projectSettings.Name));

        _cancellationSource = new CancellationTokenSource();
        Task.Factory.StartNew(ReceiveAllocationMessages, _cancellationSource.Token);
    }
}

And then the actual continuous receival of messages from the (Azure Service Bus) QueueClient (from the Microsoft.ServiceBus.Messaging namespace):

private void ReceiveMessage()
{
    Task.Run(async () =>
    {
        //Receive a new message async within 1 minute
        return await _queueClient.ReceiveAsync(TimeSpan.FromMinutes(1));
    })
    .ContinueWith(t =>
    {
        if (t.Result != null)
        {
            //The result is not null so we process the Brokered Message
            Task.Factory.StartNew(() =>
            {
                ProcessMessage(t.Result)
                ReceiveNextMessage();
            }, _cancellationSource.Token);
        }
        else
        {
            // Continue receiving and processing
            // new messages until told to stop.
            ReceiveNextMessage();
        }
    });
}

private void ReceiveNextMessage()
{
    if (_cancellationSource.IsCancellationRequested == false)
    {
        //Continue to receive new messages every 1 minute
        Task.Delay(TimeSpan.FromMinutes(1))
            .ContinueWith(t => ReceiveMessage());
    }
}

When the process is stopped or disposed the Stop method is called in order to cancel the CancellationTokenSource as shown here:

public void Stop()
{
    lock (_lockObject)
    {
        using (_cancellationSource)
        {
            if (_cancellationSource != null)
            {
                _cancellationSource.Cancel();
                _cancellationSource = null;
            }
        }

        Trace.WriteLine(string.Format("Stopped '{0}' allocation.", _projectSettings.Name));
    }
}

Does this seem like a reasonable way to go about implementing continuous receival of messages in an async fashion?

UPDATE: I posted a gist with the code from above polished and put into a class. Hopefully improves readability Gist of the queue Processor class

First, some notes:

You shouldn't be using Task.Factory.StartNew() unless you have to, Task.Run() is better.

I think you're using Task.Run() (and Task.Factory.StartNew()) and ContinueWith() too much. Only use Task.Run() when you actually need to run the code on another thread. And ContinueWith() is almost always better written using await.
If you're using it to support cancellation, you can just use ThrowIfCancellationRequested() instead.

Don't use == false, ! is much clearer.

Your your code, the processing can be Started two times. You should probably guard against that.

Your Start() and Stop() methods seems to be written with support for restarting in mind. It might be simpler to forbid that.

Now, about the structure of your code: I think a much clearer way to write your code would be to use a loop, instead of recursion. That way, your top level method can be made very clear:
while (!_cancellationSource.IsCancellationRequested)
{
    await ReceiveMessageAsync();
    await Task.Delay(TimeSpan.FromMinutes(1));
}

The ReceiveMessageAsync() method could then look like this, which is a lot shorter and much more readable:
private async Task ReceiveMessageAsync()
{
    var message = await _queueClient.ReceiveAsync(TimeSpan.FromMinutes(1));

    if (message != null)
    {
        _cancellationSource.ThrowIfCancellationRequested();
        ProcessMessage(message);
    }
}

Note that this version behaves differently than yours with regards to exceptions. In your code, if an exception happens, it's either silently ignored (if it's in ReceiveAsync()) or it just silently stops receiving (if it's in ProcessMessage()). Both are very bad and make errors hard to debug. With my code, any exception stops receiving and is reported to the top-level Task.
You should then devise some way of reporting any exceptions on that Task.
A simple solution would be to store the Task in Start() and then rethrow any exceptions in Stop(). A cleaner solution might be to change void Start() into something like Task RunAsync() and report exceptions using the returned Task.
You could also consider not stopping if some specific exception happens (e.g the server is temporarily unavailable). But that should happen only for some specific exceptions and those exceptions should be logged anyway, to help with debugging.


My PHP function that reads names and attempts to fix the suffixes I am reading a text file that has some name inconsistencies. For example I have names like:

MILLERS, WALTER M IV
DUPONTE, THOMAS B. II
HARDIWAY, GRANT U. SR.
GUIDRY, PAUL JOHN

As you can see I can split the name by first name and last name however the suffixes are placed incorrectly. Ideally the names should be:

WALTER M MILLERS IV
THOMAS B. DUPONTE II
GRANT U. HARDIWAY SR.
PAUL JHN GUIDRY

That being said this is what I have but I feel Im over complicating something simple. Any tips? Also my logic does not account for the suffix of V since that could be a middle initial also.

private function extractFullName($name){

        $suffix = collect(['II', 'III', 'IV', 'JR.', 'SR.']);

        // lets first assemble a full name from the row
        $fullNameArray = explode(",", $name);
        $firstName=trim($fullNameArray[1]);
        $lastName=trim($fullNameArray[0]);

        //lets look at the FIRST NAME for II/III/IV/JR./SR.
        $suffix->each(function ($item, $key) use ($firstName, $lastName) {

            if (strpos($firstName, " ".$item)){
                $firstName = preg_replace('/ '.$item.'$/', '', $firstName);
                $lastName.=' '.$item;
                return false;
            }
        });

        //Basically arranging it in a 'firstname lastname II/III/IV/JR./SR.' format
        $name = $firstName." ".$lastName;
        $name = ucwords(strtolower($name));
        return $name;
    }

If it can be assumed that a suffix will always have a space before it this is easier.

Loop through suffixes and see if one is in the first name.  Strip out the suffix if is is there.  Any suffix that is found is added to a suffix variable, s.

Treating the suffix separately is nice because it can be added, separated, lower cased, modified etc.  For example, suffixes could be standardized; "SR." and "SR" could all be "SR." in the final name. This is easier to do if suffix is a separate variable.  It could be an array as well.

function extractFullName($name) {

    // $suffixes = ['II', 'III', 'IV', 'JR.', 'SR.'];
    // Suffixes would always have a space before them
    $suffixes = [' II', ' III', ' IV', ' JR.', ' SR.'];

    // lets first assemble a full name from the row
    $fullNameArray = explode(",", $name);
    $firstName = trim($fullNameArray[1]);
    $lastName = trim($fullNameArray[0]);
    $s = ''; // default zero length string.
    //
    //lets look at the FIRST NAME for II/III/IV/JR./SR.
    foreach ($suffixes as $suffix) {
        if (strpos($firstName, $suffix)) {
            // Suffix was found, remove it from first name
            $firstName = str_replace($suffix, "", $firstName);
            $s .= $suffix;  // keep ths suffixes stored separately.
        }
    }

    //Basically arranging it in a 'firstname lastname II/III/IV/JR./SR.' format
    $fullName = $firstName . " " . $lastName;
    $finalName = ucwords(strtolower($fullName)) . " " . $s;
    return $finalName;
}

$name1 = 'MILLERS, WALTER M IV';
$name2 = 'MIVERS, LIVEY M IV';  // test for IV in the middle of a name
$name3 = 'DUPONTE, THOMAS B. II';
$name4 = 'DUPONTE, THOMAS B. II SR.';  // test for multiple suffixes.
$name5 = 'HARDIWAY, GRANT U. SR.';
$name6 = 'GUIDRY, PAUL JOHN';
echo extractFullName($name1) . "
";
echo extractFullName($name2) . "
";
echo extractFullName($name3) . "
";
echo extractFullName($name4) . "
";
echo extractFullName($name5) . "
";
echo extractFullName($name6) . "
";


Pairwise combinations of an array in Javascript I wrote a Javascript function that seems to successfully generate the unique pairwise combinations of a list:

function pairwise(list) {
  var pairs = [];
  list
    .slice(0, list.length - 1)
    .forEach(function (first, n) {
      var tail = list.slice(n + 1, list.length);
      tail.forEach(function (item) {
        pairs.push([first, item])
      });
    })
  return pairs;
}

pairwise('abcd'.split(''))
// [["a","b"],["a","c"],["a","d"],["b","c"],["b","d"],["c","d"]]

I checked it by using the combinatorics formula:

$$
combinations = \frac{n(n-1)}{2}
$$

var alpha = [ "a", "b", "c", "d", "e", "f", "h", "i", "j" ];

for(var n=1;n<7;n++){
  console.log(
    n,                                 // number of items
    pairwise(alpha.slice(0,n)).length, // how many pairs from function?
    n*(n-1)/2                          // double check with formula above
  );
}

// 1 0 0
// 2 1 1
// 3 3 3
// 4 6 6
// 5 10 10
// 6 15 15

Which looks right: there are 6 ways to chose pairs from 4 items, and so on.

It works, but I find it hard to read, and I'm not sure how to make it more readable. I'd welcome suggestions.

Pairing combinations like this is a very common thing to do in any language. There's a very 'idiomatic' way to do this type of operation:

for (int i = 0; i < length; i++) {
    for (int j = i + 1; j < length; j++) {
        // do something with pair (i,j)
    }
}

You will see, and recognize this pattern anywhere.

Because that pattern is so recognizable, I prefer seeing it rather than the slicing and dicing your code does.

Additionally, since the formula for the number of iterations is so readily available, it makes sense to pre-size the output array you use. Pre-sizing the output array will make a significant performance difference.

Further, even though the 'classic' for loop is used here, the performance will be fine. Other systems requiring slices or maps of list subsets will require additional work which, despite being 'idiomatic', will not necessarily be faster:

function pairwise(list) {

    var pairs = new Array((list.length * (list.length - 1)) / 2),
        pos = 0;

    for (var i = 0; i < list.length; i++) {
        for (var j = i + 1; j < list.length; j++) {
            pairs[pos++] = [list[i], list[j]];
        }
    }
    return pairs;
}

var result = pairwise(['a', 'b', 'c', 'd', 'e']);
document.getElementById('output').innerHTML = JSON.stringify(result);


Interop: Return Parameter Method as Void For interop purpose, this is something that I always do (C#):

 public static extern BigObject InteropWithCPlusPlus();

where BigObject is ( you guess it) a big object, it's not something small like int, or double.

Now, is this a good practice? Or is it better to do the interop in this way:

public static extern void InteropWithCPlusPlus(ref BigObject bigObject);

I prefer the first one as it is easier to read and understand, but I afraid that it comes with performance penalty when the return object is a large, consuming memory one.

I don't think there would be too much of a difference. You have to use a reference anyway, because non-native types are stored as references. On x86 processors, returns are placed in register A, which is limited length. So, your C++ program would be either returning a reference to BigObject or modifying an existing instance of it.

The area where I can see a performance hit is in memory allocation. If you are making that call, using the object, and then discarding it, it would be faster to create on single instance in your C# code and use that rather than allocating memory every time. You'll get diminishing returns for small numbers of calls, though; unless you have a couple thousand in a given program run, you probably won't notice the hit either way.


Attempt at solution for Dining Philosopher algorithm I know there are a lot of different solutions to solve the "Dining Philosophers" problem, but I wanted to get some feedback on a solution that I have so far.

My motivation for working on this is to prepare for future job interviews and to get more familiar with threads and C++.

#define P 5
#include <iostream>
#include <mutex>
#include <thread>
#include <Windows.h>

std::thread *threads[P]; //my philosophers
std::mutex mtx[P]; //forks
std::mutex cout;

bool pickUp(int left, int right) {
    if (mtx[left].try_lock()) {
        if (mtx[right].try_lock()) {
            return true;
        }
        else {
            mtx[left].unlock();
        }
    }
    return false;
}

void putDown(int left, int right) {
    mtx[left].unlock();
    mtx[right].unlock();
}

void run(int philID) {
    int leftIndex = philID-1;
    int rightIndex = (philID) % (P-1);
    while (1) {
        if (pickUp(leftIndex, rightIndex)) {
            cout.lock();
            std::cout << "Philosopher " << philID << " eats.\n";
            cout.unlock();
            putDown(leftIndex, rightIndex);
            Sleep(50); //cannot acquire again right away
        }
        else {
            cout.lock();
            std::cout << "Philosopher " << philID << " thinks.\n";
            cout.unlock();
        }
    }
}
int main() {

    for (int i = 1; i <= P; i++) {
        threads[i-1] = new std::thread(run, i);

    }
    for (int i = 1; i <= P; i++) {
        if (threads[i - 1]->joinable())
            threads[i - 1]->join();
    }
    return 0;
}

My pickUp() function takes the indices of the philosophers "fork" in the array of mutexes and tries to get first the left and then the right fork. If it cannot get the right immediately, it simply puts the left fork down and returns false. If this happens then the philosopher "thinks" before trying to pickup his forks again.

If both forks are able to be picked up, then he eats and then puts down both his forks. This alone was enough to avoid deadlock, but there was the problem of starvation for other philosophers. To solve this, I added a Sleep() call of 50 milliseconds so that a philosopher cannot instantly grab his forks again.

Is this enough to be considered an adequate answer to this problem? It works fine, but is there a "better" solution that I am missing?

Overall
Wow. Correct implementation.
Your the first person to post what I would call a proper implementation.
Good Job.
Code Review
Prefer NOT to use #define
#define P 5

Nearly every use of #define has a better alternative in C++ (unless you are really abstracting OS specific details).
In this case case you are  defining a constant integer. There is an explicit version of that for C++.
constexpr int globalConstantP = 5;  // More than 1 letter

While we are on P. A variable of length 1 is not very meaningful. Your code should be self documenting. This really means your variable names should be meaningful.
Avoid Platform specific includes.
#include <Windows.h>

This is the one place where #define comes in useful. Abstracting platfrom specific details. The Windows and Unix versions of sleep are different.
#if defiend(WIN32)

#include <Windows.h>
#define  doSleep(X)     Sleep(X)

#elif defined(__linux__)

#include <unistd.h>
#define  doSleep(X)     sleep(X * 1000)

#elif defined (__APPLE__) && defined (__MACH__)

#include <unistd.h>
#define  doSleep(X)     sleep(X * 1000)

#else

#error "Unsupported Platform"

#endif

Avoid using pointers
std::thread *threads[P];

In modern C++ you should never (or practically never) be calling new and delete. You should use automatic variables for all situations so that you can make sure the constructor/destructor and thus RAII kicks in to do the resource management. When you do need dynamically allocated objects they should be wrapped in smart pointers so that they can not leak.
Here you don't need pointers at all. Just use std::thread.
Avoid global variables
std::thread *threads[P];
std::mutex mtx[P]; //forks

Global mutable state is the bane of many problems in program. But especially testing. It is best to pass objects to functions (by reference) and manipulate them or even better to just manipulate the object by calling the member functions.
I would have created a class called Table that had the array of std::thread and std::mutex objects. Then your functions below become methods than manipulate the object.
cout!!!
std::mutex cout;

That is too easy to be confused with std::cout. Please put it in a namespace and the very least. I would actually create your own stream class (one that can be locked) and pass that as the output object to the constructor of your Table class.
Thinking should sleep
It take times to think.
It should also take time to think.
       if (pickUp(leftIndex, rightIndex)) {

            std::cout << "Philosopher " << philID << " eats.\n";

            doSleep(rand() % 20);

            // Don't put Down the forks until you have had time
            // time to do some eating. Remeber you are simulating work
            // after having obtained the resources. The work should
            // take none zero time.
            putDown(leftIndex, rightIndex);

        else {
            std::cout << "Philosopher " << philID << " thinks.\n";

            // When the philosopher is thinking the time should
            // also be none zero as you are simulating doing work
            // that is not related to the resource.
            doSleep(rand() % 20);
        }

Joinable
        if (threads[i - 1]->joinable())
            threads[i - 1]->join();

No need to test for joinable if you have never called detach on your thread.
Return 0
    return 0;

No need to return 0 at the end of main(). It does that automatically.
Normally you use return 0 to indicate there was a success BUT also to indicate that it could fail and there is another way to exit that could return a non zero value. When I see a return 0 I immediately start looking for alternative returns paths to see under what conditions the application will fail.
So if your application can not fail don't return anything (this is an indication that it can't fail). The compiler will add the return 0 if one was not provided.


Simple async/await progress bar In researching the new .NET Async and Await keywords in .NET 4.5, I created a simple example showing how to update a progress bar and a label while an async function is running. I also included how to cancel the process while it is taking place.

To use the code below, create a Windows form and include two buttons:

Button1 = start button
Button2 = stop button

Also add a progress bar called ProgressBar1 and a label called Label1.

A few things to note:

The functions use ASYNC and AWAIT "all the way down" from the initial button click to the final VB function that supports asynchronous support;
Note the use of the cancellationtokensource to signal that we want to cancel things;
The final asynchronous VB function we use is WebClient.DownloadStringTaskAsync(). This is arbitrary, and is used for example purposes only.

Please let me know if you have any questions or problems with this code.

' Simple progress bar example with cancellation and label updating
' Using the ASYNC and AWAIT keywords available through .Net 4.5

' Helpful sources include:
' - http://msdn.microsoft.com/en-us/library/hh191443.aspx
' - http://blogs.msdn.com/b/dotnet/archive/2012/06/06/async-in-4-5-enabling-progress-and-cancellation-in-async-apis.aspx

Imports System.Net
Imports System.Threading

Public Class Form1

    Dim test As WebClient = New WebClient ' This is specific to the type of async function we're doing. Yours may be different.
    Dim myToken As CancellationTokenSource ' This is used to notify the async function that we want to cancel it.
    Dim myProgress As Progress(Of Integer) ' We use this as the vehicle for reporting progress back to our GUI thread

    Dim Counter As Integer = 0 ' This is a variable that we use to track our progress (specific to this example)

    Private Async Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        ProgressBar1.Value = 0
        Counter = 0
        myToken = New CancellationTokenSource
        Await startProgressBar(myToken) ' Note that the signature of our startprogressbar method includes the cancellationtokensource
    End Sub

    Private Sub Button2_Click(sender As Object, e As EventArgs) Handles Button2.Click
        If Not myToken Is Nothing Then
            myToken.Cancel() ' We tell our token to cancel
        End If
    End Sub

    Public Async Function startProgressBar(ct As CancellationTokenSource) As Task

        Dim progressTarget As Action(Of Integer) ' We have to create an event handler for the function we use to report progress
        progressTarget = AddressOf ReportProgress ' We assign the handler to our specific function (see below)
        myProgress = New Progress(Of Integer)(progressTarget) ' When we initialize our progress reporter, we pass our handler to it

        For I = 0 To 9

            If ct.IsCancellationRequested = True Then
                Exit For ' We then break out of the loop
            End If

            Dim myString As String = Await RetrieveString(myProgress) ' We don't do anything with this string, just so we can run our async function

        Next

    End Function

    Public Async Function RetrieveString(ByVal myProgress As IProgress(Of Integer)) As Task(Of String)

        Dim myString As String = ""

        If Not myProgress Is Nothing Then 'If we have a valid progress object, report our progress using it
            Counter = Counter + 10
            myProgress.Report(Counter)
        End If

        myString = Await test.DownloadStringTaskAsync("http://www.cnn.com") ' This is a throwaway function - just something that uses async
        Return myString ' We are really not doing anything with the results of this function, just returning it

    End Function

    Private Sub ReportProgress(ByVal myInt As Integer)
        Label1.Text = "Step Number: " & myInt.ToString
        ProgressBar1.Value = myInt
    End Sub

End Class

The final asynchronous VB function we use is WebClient.DownloadStringTaskAsync(). This is arbitrary, and is used for example purposes only.

This indicates to me that you should think about making this reusable, so that when you need to use the progress bar somewhere else, you don't need to copy & paste the code.

Dim progressTarget As Action(Of Integer) ' We have to create an event handler for the function we use to report progress
progressTarget = AddressOf ReportProgress ' We assign the handler to our specific function (see below)
myProgress = New Progress(Of Integer)(progressTarget) ' When we initialize our progress reporter, we pass our handler to it

This is way too verbose. What's wrong with just the following line?

myProgress = New Progress(Of Integer)(AddressOf ReportProgress)

You don't need to comment every single thing you do, that's obvious from the code.

You should disable the button that doesn't make sense at the moment. This is especially important for the StartButton, because right now, the user can start the download multiple times.

Dim myString As String = ""

' code that doesn't use myString

myString  = …

Return myString

There is no need to declare the variable early or to set it to "". Actually, there is no need to declare the variable at all, you can just return the expression directly.

Names like Label1 and myInt are very unhelpful. Use descriptive names like ProgressLabel (though that's not great either) and stepNumber.


Single inheritance polymorphism I am learning about polymorphism in C++ and decided to implement an example. Is there anything I need to fix in my code? This compiles and runs.

#include <iostream>

using std::cout;
using std::endl;
using std::cin;

class Employee
{
public:
    Employee():salary(1000){}
    virtual ~Employee(){}
    virtual void Talk() const {cout << "Employee says: ";}

protected:
    int salary;
};

class Programmer : public Employee
{
public:
    void Talk() const {cout << "I am a Programmer.\n";}
    void Compiling() const {cout << "My code is compiling...\n";}
};

class SalesPerson : public Employee
{
public:
    void Talk() const {cout << "I am a SalesRep.\n";}
    void Selling() const {};
};

int main()
{
    const int numEmployees = 3;
    Employee* Company[numEmployees];
    Employee* pEmployee;

    int choice,i;

    for(i=0; i<numEmployees; i++)
    {
        std::cout << "(1) Programmer (2) SalesRep: ";
        cin >> choice;
        if(choice == 1)
            pEmployee = new Programmer;
        else
            pEmployee = new SalesPerson;

        Company[i] = pEmployee;
    }

    cout << endl;

    for(i = 0; i<numEmployees;i++)
    {
        Company[i]->Talk();

        Programmer *pCoder = dynamic_cast<Programmer *> (Company[i]);

        if(pCoder)
            pCoder->Compiling();

        cout << endl;
    }

    return 0;
}

Problems with your interface design:

The Employee interface you've designed is clearly lacking
abstraction, since you have a dynamic_cast in the program to
test if an employee is of a specific type.

Employee should instead have a virtual method such as DoWork()
which in turn will call Compiling() for Programmer,
Selling for a SalesPerson and so forth:

class Employee
{
public:
    ....

    // Virtual function with no default implementation (= 0)
    virtual void DoWork() const = 0;
};

class Programmer
{
public:
    void DoWork() const { Compiling(); }
};

class SalesPerson
{
public:
    void DoWork() const { Selling(); }
};

The constructor of Employee sets salary to 1000 unconditionally.
Perhaps it would be better to take salary as a constructor parameter
so you can easily specify different salaries:

Employee(double salary) : salary(salary) { }

BTW, double seems more appropriate for a monetary value, as it
is likely to include decimal places. int is for whole numbers only.

The destructor of Employee is empty, so it should be made a default:

virtual ~Employee() = default;

Memory management tidbits:

It is generally better to avoid manual memory management, as it is
error prone, not exception safe, and slipping a memory leak is very easy.
In fact, in your current example, the objects you've allocated
with new are not being destroyed. Remember that C++ is not garbage collected. You would have to delete them somewhere.

You should ideally be using an automatic pointer to manage
your dynamic instances. main() could be reworked in the following way:

#include <array>
#include <memory>

int main()
{
    std::array<std::unique_ptr<Employee>, 3> company;

    int choice = 0;
    for (int i = 0; i < company.size(); i++)
    {
        std::cout << "(1) Programmer (2) SalesRep: ";
        cin >> choice;

        if (choice == 1)
        {
            company[i].reset( new Programmer );
        }
        else
        {
            company[i].reset( new SalesPerson );
        }
    }

    ....
}

You should note in the example above the use of std::unique_ptr and
std::array. The unique_ptr is a managed pointer that will delete
the object automatically when you leave the function scope. This way
you don't have to worry about memory leaks. std::array is a fixed size
array which behaves just like a plain C array, but with some extra member
methods and debug bounds checking. It also knows its length, so we no longer
need to keep that numEmployees constant around.


Using generics to find a View by Class This works but feels clunky - any suggestions for improvement?

public static <T extends View> T findViewByClassReference(View rootView, Class<T> classReference){
  if(classReference.isInstance(rootView)){
    return classReference.cast(rootView);
  }
  if(rootView instanceof ViewGroup){
    ViewGroup viewGroup = (ViewGroup) rootView;
    for(int i = 0; i < viewGroup.getChildCount(); i++){
      View child = viewGroup.getChildAt(i);
      View match = findViewByClassReference(child, classReference);
      if(match != null){
        return classReference.cast(match);
      }
    }
  }
  return null;
}

This works but feels clunky

This seems fine. It does the job well. I don't see how this can be done simpler, and I'm not aware of other existing method in the SDK that can help simplify this.

  any suggestions for improvement?

I have only minor suggestions:

The method uses depth first search to find a view. Depending on the actual tree of views in the application, this might be slow, for example when you have very deep trees but the class you're looking for is at a shallow level. In such case a breadth first search would perform better. This is not something to "fix", just something to keep in mind, and adjust to your actual application as necessary
Another point to keep in mind is that the method will return the first view with matching class. When there are multiple views of the same class, a user might not get the one expected
As with any utility method, it's good to add a nice javadoc. That will be a good place to mention about the previous points (search algorithm used, returns first match).
A common name for Class instances is clazz. I suggest to use that name to make the code look more familiar to other developers, and it's shorter too than classReference
The "ByClassReference" in the method name is redundant, because this is implied by the method signature
The formatting could be slightly more readable if you apply auto reformat in Android Studio


Working with a new form instance every time I have a form that looks like this:

It is initialized with a Shortcut from the keyboard. It is in a module:

Public Sub ShowMainForm()
    With frmMain
        .Show vbModeless
    End With
End Sub

The form has a button:

Private Sub btnRun_Click()
    Call MainGenerateReport
End Sub

The button runs a procedure, called MainGenerateReport in a module.

Public Sub MainGenerateReport()
    ' other code;
    Call frmMain.MakeLabel
    ' other code;
End Sub

frmMain.MakeLabel changes a label in the form with some information:

Public Sub MakeLabel()
    Dim c           As Long
    Dim r           As Long
    Me.lbInfo.Visible = checkNumbers
    Me.lbInfo.Clear

    If checkNumbers Then
        With Me.lbInfo
            .ColumnCount = 2
            .ColumnWidths = CStr(Me.lbInfo.Width / 1.8 & ";" & Me.lbInfo.Width / 4)

            For r = 0 To 7
                .AddItem
                For c = 0 To 1
                    .List(r, c) = tblInfo.Cells(1 + r, 3 + c)
                Next c
            Next r
        End With
    End If
End Sub

At the end, if I want to close the form, I use the Esc key. I have a button on the form, btnExit and its cancel property is set to True:

Private Sub btnExit_Click()
    Unload Me
End Sub

The problem: According to VBA best practices, I should initialize the form like this (and rewrite my MakeLabel and the btnExit_Click accordingly):

Public Sub ShowMainForm()
    With new frmMain
        .Show vbModeless
    End With
End Sub

Is that recommended?

Public Sub ShowMainForm()
    With frmMain
        .Show vbModeless
    End With
End Sub

You don't want that With block, it's redundant.

Public Sub ShowMainForm()
    With New frmMain
        .Show vbModeless
    End With
End Sub

You don't want to do that with a vbModeless form either - the instance will be destroyed quite immediately after being created.

For an object-oriented approach that uses a modeless form, I'd suggest you make the form a member of a dedicated "presenter" class:

Option Explicit
Private WithEvents summaryForm As frmMain ' <~ notice WithEvents ..I'll get to it.

Private Sub Class_Initialize()
    Set summaryForm = New frmMain
End Sub

Private Sub Class_Terminate()
    Set summaryForm = Nothing
End Sub

Public Sub Show()
    If Not summaryForm.Visible Then summaryForm.Show vbModeless
End Sub

Public Sub Hide()
    If summaryForm.Visible Then summaryForm.Hide
End Sub

Now, this "presenter" class shall be responsible for accessing the form; notice how it already hides the implementation detail of the modeless-ness to the outside world.

When the user clicks the btnRun button, the form itself shouldn't be responsible for anything - so instead of calling MainGenerateReport directly, we'll fire an event to tell the presenter class that it needs to do something about it:

Option Explicit
Public Event OnRunReport()
Public Event OnExit()

Private Sub btnRun_Click()
    RaiseEvent OnRunReport
End Sub

Private Sub btnExit_Click()
    RaiseEvent OnExit
End Sub

That way the form has no dependencies to other modules, and pretty much zero responsibilities.

Back to the presenter, we can handle these events:

Private Sub summaryForm_OnRunReport()
    MainGenerateReport
    Refresh
End Sub

Private Sub summaryForm_OnExit()
    Hide
End Sub

Public Sub Refresh()
    'todo
End Sub

This leaves the problem of MakeLabel/Refresh. I don't know what tblInfo is, but I'm pretty sure it's not the form's concern. Really all it needs is some Range or ListObject that contains whatever information needs to go into that list - and instead of looping to .AddItem and explicitly set each .ListItem, you could use the control's .RowSource property and avoid looping altogether; this CR Q&A shows how.

Once the presenter class is capable of providing a data source for the form's ListBox (you'll have to verify it it actually binds to the source; if that's the case then you won't even need a refresh button, the list would just update itself).

That would be the Refresh implementation.

The last step is to instantiate the presenter; as long as the presenter instance is alive, the encapsulated form instance lives - there's no need to explicitly Unload Me anywhere.

Say you named the class SummaryPresenter, then you could have it exist in global scope:

Option Explicit
Private presenter As SummaryPresenter

Public Sub ShowMainForm() ' macro attached to shortcut key
    If presenter Is Nothing Then Set presenter = New SummaryPresenter
    presenter.Show
End Sub

Since we made the OnRunReport handler call Refresh after MainGenerateReport runs, the procedure no longer needs to call it explicitly. Or, if it does need to (hard to tell with just a little 'other code; comment to work with), then it can do so by calling the Presenter object's Refresh method:

Public Sub MainGenerateReport()
    ' other code;
    presenter.Refresh
    ' other code;
End Sub

This makes your code use the same instance of frmMain all the time, while separating responsibilities into different [class] modules... which isn't much different from working against the default instance in the first place.

But then, everything boils down to why you would want a new instance every time - IMO if the form is a modeless "toolwindow" that you can show/hide while working in Excel, then it's objectively better (more efficient) to avoid initializing it everytime you want to show it... just like it's more efficient to avoid initializing the listbox columns everytime you refresh it ;-)


Using setTimeout to get scrolling chat window to work, but doesn't feel like the ideal solution My friend and I are working on a bare-bones chat web app, using Angular on the front end. He's using Swampdragon for some of the real-time stuff.

My task that I set out to achieve was to get the chat window to scroll to the bottom when a chat room is loaded (most relevant bit is $dragon.onChannelMessage):

app.controller('ChatRoomCtrl',
        ['$scope', '$dragon', 'ChatStatus', 'Users', function (
            $scope, $dragon, ChatStatus, Users) {
    $scope.channel = 'messages';
    $scope.ChatStatus = ChatStatus;

    $scope.idToUser = function(id) {
        var user;
        user = $scope.users.filter(function(obj) {
            return obj.pk == id;
        });
        return user[0].display_name;
    };
    Users.getList().then(function(users) {
        $scope.users = users;
    });

    $dragon.onReady(function() {
        $dragon.subscribe('messages', $scope.channel).then(function(response) {
            $scope.dataMapper = new DataMapper(response.data);
        });
    });

    $dragon.onChannelMessage(function(channels, message) {
        if (indexOf.call(channels, $scope.channel) > -1) {
            if (ChatStatus.messages[message.data.room].indexOf(message.data) == -1) {
                message.data.posted = new Date(message.data.posted);
                $scope.$apply(function() {
                    ChatStatus.messages[message.data.room].push(message.data);

                    setTimeout(function() {
                        scrollToBottom();
                    }, 30);
                });
            }
        }
    });
}]);

Or, when a new message is pushed:

app.controller('RoomCtrl', ['$scope', 'Rooms', 'Messages', 'ChatStatus', function($scope, Rooms, Messages, ChatStatus) {
    $scope.changeRoom = function(room) {
        ChatStatus.selectedRoom = room;
        Messages.getList({'room': room.id}).then(function(messages) {
            angular.forEach(messages, function(message, key) {
                message.posted = new Date(message.posted);
            });
            ChatStatus.messages[room.id] = messages;

            setTimeout(function() {
                scrollToBottom();
            }, 30);
        });
    }
    $scope.rooms = Rooms.getList()
    .then(function(rooms) {
        ChatStatus.rooms = rooms;
        ChatStatus.selectedRoom = rooms[0];
        $scope.rooms = rooms;
    })
}]);

In both controllers, I refer to the scrollToBottom function:

function scrollToBottom() {
    var chatWindow = document.querySelector('.the-chats');
    var mostRecent = chatWindow.querySelector('li:last-child');
    var mostRecentDimensions = mostRecent.getBoundingClientRect();
    var chatWindowScrollY = chatWindow.scrollTop;

    chatWindow.scrollTop = mostRecentDimensions.bottom + chatWindowScrollY;
}

If I remove the setTimeout from the first controller, it'll scroll to what is the last item in the list before the new message is pushed, while the second controller will error out.

If the setTimeout is in place, this does what I want it to do. However, it feels like a bad solution; it certainly doesn't feel like an 'Angular' way.

I've read a bit about promises, deferred objects, $q, etc., but the examples always seem to use it in the context of AJAX-types of calls, so I don't know if that applies here. But that's really what I'm looking for, right? Push the new message, then do the scroll?

If you find yourself using setTimeout and want to be more angular, check out the $timeout service in the AngularJS API.

In addition, for your purposes, you might be better off using $evalAsync.

$evalAsync waits for at least one digest cycle before executing the expression, which might be enough for the DOM to have the new elements added (though not yet rendered). This can prevent the "flicker" that you otherwise get from waiting for elements to render before manipulating them. The only reason to not use $evalAsync would be if you're dependent on the DOM rendering for accuracy.


Longest Peak without backtracking The goal is to find the longest peak in a sequence of numbers. A peak is defined to be a strictly increasing elements until a tip is found and then a strictly decreasing elements.

e.g. (1): 3, 2, 1, 2, 3, 4, 3, 0, 10, 0, 1, 2, 2, 1, 0

1, 2, 3, 4, 3, 0 is a peak;
0, 10, 0 is a peak;
0, 1, 2, 2, 1, 0 is NOT a peak (there is no tip);

e.g. (2): 1, 3, 2 is a peak

e.g. (3): 1, 2, 3 is NOT a peak

I'm trying to do so without backtracking - i.e. without finding a peak and then going outwards from it both back and front.

Here's a code I came up with

def longestPeak(array):
    longest = peakStart = peakEnd = 0
    peakArrived = False
    for i in range(1, len(array) - 1):
        # Check for peak
        if array[i-1] < array[i] and array[i] > array[i+1]:
            peakArrived = True
            if len(array) == 3:  # special case
                return 3
        # Check for end
        if array[i] <= array[i+1] and array[i] < array[i-1]:
            peakEnd = i
        elif array[i+1] < array[i] and i+2 == len(array):
            peakEnd = i+1
        # Check for longest
        if peakArrived and peakEnd > peakStart and peakEnd - peakStart + 1 > longest:
            longest = peakEnd - peakStart + 1
            peakArrived = False
        # Check for start
        if array[i] < array[i+1] and array[i] <= array[i-1]:
            peakStart = i
        elif array[i] > array[i-1] and i == 1:
            peakStart = 0

    return longest

Are you sure that solution is correct? By your description of the problem, I would have expected the result for an input [1, 6, 4, 7, 6, 6, 5] to be 3, but your solution produces 5.

Your solution is interesting -- that drew me to this question. I find that code looking at adjacent elements when iterating and accumulating an optimal value is a code smell in algorithms. It typically makes a solution significantly more difficult to read and debug. Usually, you should only look at as few values in each iteration as possible -- and I believe it's sufficient to look only at the current value whilst keeping the previous handy.

        if array[i-1] < array[i] and array[i] > array[i+1]:

This confers the additional benefit of removing the random access requirement for the input; that is, removing the __getitem__ dependency (array[i] is actioned as type( array ).__getitem__( array, i )). Whilst you've indicated that the input should be an array by the parameter name, one of Python's strengths is its duck-typing. There's no fundamental reason why the algorithm could not, or should not, apply more generically to any Iterable[int]. My solutions below will additionally work with input (contrived, I grant you) ( x for x in ( 0, 1, 0 ) ). If your minimum Python version requirement is sufficiently high, I would personally add a type hint to the argument, but that's usually deemed overkill.

            if len(array) == 3:  # special case
                return 3

A special case that isn't an edge-value of a data-type (e.g. 0 for an unsigned integer) is an immediate code smell.

    for i in range(1, len(array) - 1):

Skipping the end (and usually the start) of a sequence in an accumulation/optimisation problem is a smell. Avoid if possible.

The problem can be modeled by a state-machine. On each iteration of the array elements, you begin in one of four states: starting, floor, climbing, descending. You begin in the starting state and once the first element is seen immediately transition to floor. The floor represents a state in which you're neither climbing nor descending. This occurs when you've reached some plateau and haven't climbed out of it. The climbing state is self-descriptive. You begin descending once you have passed a peak having made a climb. The longest seen peak can only end whilst in a descending state. You seemed in your solution to try and defer the recalculations of the longest seen peak until you've left the peak. This isn't necessary -- if you descend further downwards, you can always carry on increasing the value of the longest seen peak.

Here is a descriptive algorithm:

import enum

class TrekState( enum.Enum ):
    Begin   = 0
    Floor   = 1
    Climb   = 2
    Descend = 3

class Trek:
    def __init__( self ):
        self.state = TrekState.Begin
        self.counter = 0
        self.longest = 0

    def advance( self, i ):
        # update new state of our trek
        if self.state == TrekState.Begin:
            self.state = TrekState.Floor
            self.counter = 1
        else:
            previous = self.previous

            if self.state == TrekState.Floor:
                if previous < i:
                    self.state = TrekState.Climb
                    self.counter += 1
            elif self.state == TrekState.Climb:
                if previous < i:
                    self.counter += 1
                elif previous > i:
                    self.state = TrekState.Descend
                    self.counter += 1
                else: # previous == i
                    self.state = TrekState.Floor
                    self.counter = 1
            elif self.state == TrekState.Descend:
                if previous < i:
                    self.state = TrekState.Climb
                    self.counter = 2
                elif previous > i:
                    self.counter += 1
                else: # previous == i
                    self.state = TrekState.Floor
                    self.counter = 1

        # update longest and previous
        if self.state == TrekState.Descend:
            self.longest = max( self.counter, self.longest )
        self.previous = i

def updatedLongestPeak( seq ):
    trek = Trek()
    for i in seq:
        trek.advance( i )
    return trek.longest

. A more concise version of the above, replacing object constructs with a more unstructured style, and some manual optimisations:

def verboseLongestPeak( seq ):
    state = counter = longest = 0

    for i in seq:
        counter += 1
        if state == 0:
            state = 1
        elif state == 1:
            if previous < i:
                state = counter = 2
        elif state == 2:
            if previous > i:
                state = 3
            elif previous == i:
                state = 1
        elif state == 3:
            if previous < i:
                state = counter = 2
            elif previous == i:
                state = 1

        if state == 3:
            longest = max( counter, longest )
        previous = i

    return longest

. If you were to go for a solution of the latter's style, it would be essential to provide comments on the meaning of each state value.

Neither solution is verified to be correct.


Converting Procedural JS Code to Object I'm new to JS and trying to improve my code with the use of more organised objects.  I have the following procedural code which I have converted to an object. It does an ajax load within main. Aside from grouping the methods (functions) for better organisation, what other advantages does this approach have?  Is there any way the object version of the code can be further improved?

Procedural approach

var main = $("main"),

    init = function() {

    },

    ajaxLoad = function(html) {
      document.title = html.match(/<h1>(.*?)<\/h1>/)[1].trim();
      main.fadeIn(200);
      init();
    },

    loadPage = function(href) {
      main.fadeOut(200, function() {
        main.load(href + " main>*", ajaxLoad);
      });
    };

    init();

    $(document).on('click', 'header a', function() {
      var href = $(this).attr('href');

      if (href.indexOf(document.domain) > -1 || href.indexOf(':') === -1) {
        history.pushState({}, '', href);
        loadPage(href);
        return false;
      }
    });

Object approach

var pageLoader = {

      main: $('main'),

      init: function() {},

      ajaxLoad: function(html) {
        document.title = html.match(/<h1>(.*?)<\/h1>/)[1].trim();
        pageLoader.main.fadeIn(200);
        pageLoader.init();
      },

      loadPage: function(href) {
        pageLoader.main.fadeOut(200, function() {
          pageLoader.main.load(href + " main>*", pageLoader.ajaxLoad);
        });
      },

      ajaxLink: function(href) {
        if (href.indexOf(document.domain) > -1 || href.indexOf(':') === -1) {
          history.pushState({}, '', href);
          pageLoader.loadPage(href);
        }
      }

    };

    pageLoader.init();

    $(document).on('click', 'header a', function() {

      var href = $(this).attr('href');
      pageLoader.ajaxLink(href);
      return false;

    });

First, I'm going to go through and do a little nitpicking.

In the procedural version, why is everything indented except for main?

When you are closing the object in the object version, the closing brace should be in the same column as the beginning line of the object.

So this:

var pageLoader = {

      ...

};                     <---

Rather than this:

var pageLoader = {

      ...

    };                 <---

Once you change this, you'll notice that the things inside the object are indented two indents, so I recommend knocking off one indent.

How much you are indenting is all messed up:

You are indented 8 spaces (2 indents) in the object.
You indented everything else 4 spaces (1 indent).
You indented the code inside functions 2 spaces (1/2 indent).

I recommend that you choose one and stick with it (I use 1 indent).

You don't need a return at the end of the function event listener (onclick); where is that false going to get returned to?

There is nothing inside your init functions... why are these here?

Other than that, your "translation" from procedural to object seems fine; all you basically did was stick the methods and variables in an object and add pageLoader. to the beginning of a few things.

I think that the code is good in an object because, as you said, it aids organization. However, since I don't know the full/rest of the code, I can't be sure if this is the best approach for your situation.