Asynchronous SSLSTREAM Recently I have written an SslStream class asynchronously authenticate clients and receive message from them. I still would like anyone to suggest improvements for my code.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Net;
using System.Net.Sockets;
using System.Net.Security;
using System.Security.Authentication;
using System.Security.Cryptography.X509Certificates;
using System.IO;

public class Wrapper
{
    public byte[] buffer;
    public SslStream sslStream;
    public object connector;
}

public class Sock
{
    private Dictionary<string, byte> Connections;
    public event Action<Wrapper> AnnounceNewConnection;
    public event Action<Wrapper> AnnounceDisconnection;
    public event Action<byte[], Wrapper> AnnounceReceive;
    private Socket _sock;

    private X509Certificate certificate = X509Certificate.CreateFromCertFile("exportedcertificate.cer");

    public Sock(int port)
    {
        try
        {
            Connections = new Dictionary<string, byte>();
            _sock = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            _sock.Bind(new IPEndPoint(IPAddress.Any, port));
            _sock.Listen(500);
            _sock.BeginAccept(AcceptConnections, new Wrapper());
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
        }
    }

    private void AcceptConnections(IAsyncResult result)
    {
        Wrapper wr = (Wrapper)result.AsyncState;
        try
        {
            wr.sslStream = new SslStream(new NetworkStream(_sock.EndAccept(result), true));
            wr.sslStream.BeginAuthenticateAsServer(certificate, EndAuthenticate, wr);

            _sock.BeginAccept(AcceptConnections, new Wrapper());
        }
        catch (Exception e) { Console.WriteLine(e); AnnounceDisconnection.Invoke(wr); wr.sslStream.Close(); wr.sslStream.Dispose(); }
    }

    private void EndAuthenticate(IAsyncResult result)
    {
        Wrapper wr = (Wrapper)result.AsyncState;
        try
        {
            try { wr.sslStream.EndAuthenticateAsServer(result); }
            catch { }

            if (wr.sslStream.IsAuthenticated)
            {
                AnnounceNewConnection.Invoke(wr);
                if (wr.sslStream.CanRead)
                {
                    wr.buffer = new byte[5];
                    wr.sslStream.BeginRead(wr.buffer, 0, wr.buffer.Length, ReceiveData, wr);
                }
            }
            else
            {
                AnnounceDisconnection.Invoke(wr); wr.sslStream.Close(); wr.sslStream.Dispose();
            }
        }
        catch (Exception e) { Console.WriteLine(e); AnnounceDisconnection.Invoke(wr); wr.sslStream.Close(); wr.sslStream.Dispose(); }
    }

    private void ReceiveData(IAsyncResult result)
    {
        Wrapper wr = (Wrapper)result.AsyncState;
        try
        {
            AnnounceReceive.Invoke(wr.buffer, wr);

            SocketError error = SocketError.Disconnecting;
            int size = wr.sslStream.EndRead(result);
            if (error == SocketError.Success && size != 0)
            {
                wr.buffer = new byte[size];
                wr.sslStream.BeginRead(wr.buffer, 0, wr.buffer.Length, ReceiveData, wr);
            }
            else
            {
                wr.sslStream.Close();
                wr.sslStream.Dispose();
                AnnounceDisconnection.Invoke(wr);
            }
        }
        catch (Exception e) { Console.WriteLine(e); AnnounceDisconnection.Invoke(wr); wr.sslStream.Close(); wr.sslStream.Dispose(); }
    }
}

Also, how can I verify server authentication?

I can't comment on how secure your code is, however there are a couple of other problems that I would like to point out.

Clutter

Your using directives can be cut down to:

using System;
using System.Net;
using System.Net.Sockets;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;

Naming

Let me show you what I think of when I see a class called Sock:

You need to give that class a better name, for instance SslSocketMonitor. Actually, I don't know enough about your class to find a good name; use your brain to find a better one.
Let me show you what I think of when I see a class called Wrapper:

Because you are using it as a parameter to an event, this ought to be called something like SslSocketEventArgs, but certainly not Wrapper.
Decide on one naming convention and stick to it. You are using three different conventions for naming private fields:

private Dictionary<string, byte> Connections; // UpperCase
private Socket _sock;                         // _lowerCasePrefixed
private X509Certificate certificate;          // lowerCase

Your event names are nonstandard and not particularly clear.
Instead of AnnounceNewConnection try Connected,
instead of AnnounceDisconnection try Disconnected,
and instead of AnnounceReceive use ReceivedData.

API problems

.NET programmers are familiar with events. Therefore, it makes sense to implement the pattern properly:

SslSocketEventArgs (formerly Wrapper) needs to be derived from System.EventArgs
Instead of using Action:

public event Action<Wrapper> AnnounceNewConnection;
public event Action<Wrapper> AnnounceDisconnection;
public event Action<byte[], Wrapper> AnnounceReceive;

use the more expressive and conventional EventHandler<T>:

public event EventHandler<SslSocketEventArgs> Connected;
public event EventHandler<SslSocketEventArgs> Disconnected;
public event EventHandler<SslSocketEventArgs> ReceivedData;

It's not safe to directly invoke event handlers as they could easily be null. So define the following extension method to safely invoke the event handlers:

public static class EventHandlerExtensions
{
    public static void InvokeSafely<T>(this EventHandler<T> eventHandler,
                       object sender, T eventArgs) where T : EventArgs
    {
        if (eventHandler != null)
        {
            eventHandler(sender, eventArgs); // syntactic sugar for .Invoke
        }
    }
}

and replace all the usages:

Disconnected.InvokeSafely(this, wr);
Received.InvokeSafely(this, wr); // etc...

I'm not a fan of beginning listening in the constructor. It would be better to invoke that explicitly in a separate method.
There should be a way to cancel listening. Write a public method to enable this.

Programming errors

Your field Dictionary<string, byte> Connections is initialized but never used. Delete it.
You have lines like these spread throughout your code:

wr.sslStream.Close();
wr.sslStream.Dispose();

That scares me. What if you forgot to close it somewhere? It would be best to enclose all uses of the class in using statements where possible.
In ReceiveData, you make the following (constant) assignment:

SocketError error = SocketError.Disconnecting;

So the following condition will always be false:

if (error == SocketError.Success && size != 0)

What you are probably trying to do is instead of catch (Exception):

catch (SocketException e)
{
    // handle properly!
    Console.WriteLine(e.SocketErrorCode);
}


Random name picker for a contest I have an array of 4 names, and was wondering what the best best way to reliably pick 1 name for a contest would be. I've read online that javascripts Math.Random is only pseudo random. Which makes me wonder, would this be a fair enough function to put a prize on the line?

The picking process won't be externally/customer facing, so I'm not worried about how it looks, more just about how fair it would be.

$(".randomize").click(function() {
  var contestents = [{
    person: "Kareem"
  }, {
    person: "Charlie"
  }, {
    person: "Hobie"
  }, {
    person: "Hoder"
  }];

  var winner = Math.floor(Math.random() * contestents.length)
  $('div').html(contestents[winner].person);
});

jsfiddle here

If not, how could I randomize this even more to be fair enough to put a prize on the line.

You are correct. Math.random is indeed pseudo-random. However, its algorithm is actually quite good. In this SO post, a concern is brought up that the randomly generated numbers tend to all have the same number of digits. While this was statistically proven to make sense, you won't run into a similar problem since you are only dealing with single digit numbers.

Math.random should be random enough for your purposes. However, if you want to get truly random numbers to be absolutely fair, check out RANDOM.ORG. This, however, would obviously be a lot more unwieldy than just using Math.random.


Iterating 2D array in Swift I have a situation where I needed to convert NSNumber 2D array to Float 2D array, so I did it like this:

var numArr:[[NSNumber]]()
//fill values
let floatArr = numArr.map { $0.map { $0.floatValue} }

It's working fine, no issues!

But I wondering if there's any better way to handle this?

Your code is fine, an equivalent method in Swift 3 would be

let floatArr = numArr.map { $0.map { Float($0) } }

You get the same result with less code by using the
bridging cast from NSNumber to Float, which works for (nested)
arrays as well:

if let floatArr = numArr as [[Float]] { ... }

Actually I don't know of any case (in Swift 3) where this cast can fail.

Things change a bit in Swift 4, as a consequence of
SE 0170 - NSNumber bridging and Numeric types. Your code still works fine, and is equivalent
to

let floatArr = numArr.map { $0.map { Float(truncating: $0) } }

The truncating initializers where introduced because not every
Number can be converted to a scalar value without loosing information,
there are also failable init?(exactly:) initializers.

Now the bridged cast can fail

for numbers which are not exactly representable as a Float,
such as Double(1.1),
for numbers which exceed the range of Float.

(The first case is currently discussed at https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20170612/037499.html and may change in the future.)

Example:

let numArr = [
    [ NSNumber(value: 1.1), NSNumber(value: 2.0) ],
    [ NSNumber(value: Double.greatestFiniteMagnitude) ],
]

if let floatArr = numArr as? [[Float]] {
    print(floatArr)
} else {
    print("failed")
}

This produces [[1.10000002, 2.0], [inf]] in Swift 3, but fails
in Swift 4.

To summarize: Your code is fine.

If you don't care about "loosing
precision" or overflows then there is no need to change anything. In Swift 4
you can write it as Float(truncating: $0) to emphasize
that the result might not be exactly identical to the given
numbers.
If you care then you can use the new "exactly" initializers,
or the bridged cast as? [[Float]] in Swift 4.


Rails 4.0 api_key implementation I have a concept of the user having many API keys, but the API key belongs to the user, so I did the following:

config/routes.rb

  resources :users do
    resources :api_keys, path: '/developmentcenter'
  end

app/models/api_key.rb

class ApiKey < ActiveRecord::Base
  belongs_to :user
end

app/controllers/api_keys.rb

Note: I have separate some of the logic up because I don't like messy methods, so I split them into lib/aisis_planner/api_keys_controller_mixins.rb.

class ApiKeysController < ApplicationController
  before_action :authenticate_user!

  def index
    @user = User.find(params[:user_id])
    @api_key = User.apikeys
  end

  def create
    @user = User.find(params[:user_id])
    @api_key = ApiKey.new(create_new_api_key)
    create_api_key(@api_key, @user)
    end

  def destroy
    @user = User.find(params[:user_id])
    destroy_api_key(@user)
  end

  private

  def create_new_api_key
    params.permit(:api_key, user_attributes: [:id])
  end
end

lib/aisis_planner/api_keys_controller_mixins.rb

module AisisPlanner
  module ApiKeysControllerMixins
    def create_api_key(api_key, user)
      if api_key.save
        flash[:notice] = 'Created a new API Key'
        redirect_to user_path(user.id)
      else
        flash[:alert] = 'Could not create API Key please try again'
        redirect_to user_path(user.id)
      end
    end

    def destroy_api_key(user)
      ApiKey.find(params[:id]).destroy
      flash[:notice] = 'Api Key has been deleted.'
      redirect_to user_path(user.id)
    end
  end
end

Now for the tests. There are no integration tests at this time (I use capybara).

require 'spec_helper'

describe ApiKeysController do
  let(:user) { FactoryGirl.create(:user) }

  describe "#create" do
    it "should not create an api key for those not logged in" do
      post :create, :user_id => '', :id => ''
      expect(response).to redirect_to login_path
    end

    it "should create an api key" do
      sign_in user
      cookies[:auth_token] = user.auth_token
      post :create, :user_id => user.id.to_param
      expect(response).to redirect_to user_path(user.id)
    end
  end

  describe "#destroy" do
    it "should not delete an api for a user not logged in" do
      post :destroy, :user_id => '', :id => ''
      expect(response).to redirect_to login_path
    end

    it "should delete an api key for a logged in user" do
      sign_in user
      cookies[:auth_token] = user.auth_token
      key = FactoryGirl.create(:api_key, user_id: user.id)
      post :destroy, :user_id => user.id.to_param, :id => key.id.to_param
    end
  end
end

What are your thoughts?

Quite a few things sprang to mind immediately, but as I delved deeper I realized that, well, there are a lot of issues. So here goes.

Outright bugs, I think

User.apikeys - what? Shouldn't that be @user.api_keys? But I notice you don't test the index action, which would have caught that.

Security Issues

You're not really using params filtering correctly. You're permitting (rather than requiring) some top-level params, but never actually filter the "sub-params". The point is usually to filter the nested params, e.g. params.require(:api_key).permit(:foo, :bar).
But judging by your tests, it seems that creating an API key doesn't need any params at all, which makes things simpler.

Secondly, create new API key records through the has_many association on User (which I assume you have); don't set the user_id directly. I.e. @user.api_keys.new(...). But read on first.

If the idea is that a user can only create API keys for himself/herself (which the tests seem to indicate), then don't find the user by params[:user_id]. Right now, it's very easy to craft a request with a different user_id param, and destroy API keys that belong to other users in the system.

So if you only want the current user to create keys for himself/herself, the route doesn't need to be nested at all, since you only ever need the currently logged in user. I'm assuming you're using Devise, so the current user is accessible via the aptly named current_user helper method. (You can also dispense with the nested routing, if you only want the current user).

Even so, any user would still be able to delete other people's keys, because you don't check whether a given id param actually belongs to the current user (or the @user for that matter). Basically, if someone's logged in, they can just call the destroy action with completely arbitrary numbers and destroy everything.

Heck, you can do that from the JS console in a browser. Try typing $.ajax({url: "/users/x/developmentcenter/y", type: "DELETE"}) where x is your own user ID (which you can see in a URL somewhere) and y is any number between 1 and whatever the maximum for your database's ID column is. If y happens to a key id - any key - it'll get destroyed.

The mixins could also be a source of security issues. See the section below about that.

Consistency and Naming

In your index action you have @api_key = User.apikeys.
Why is one singular, and the other one plural? @api_key should be called @api_keys (since it's a collection), and the has_many relation on User should be api_keys with an underscore (since that's how you write it everywhere else). Again, I'm thinking that last part is a straight-up bug.

Your create_new_api_key doesn't do what it says; it doesn't create anything. Call it api_key_params instead, since that's what it returns. Besides, it's pretty confusing to have two methods called create_new_api_key and create_api_key... which one do you think actually creates an API key? But again, judging by your tests, you don't need create_new_api_key at all, since no params filtering should be needed.

Pointless Mixins

I understand that you don't like long methods, but making a mixin module is excessive. Do you intend to create/destroy API keys (and redirect) in other controllers? I'm guessing that the answer is "no" (if it's "yes", you've got other problems). So your mixins can only be mixed into 1 specific class, which completely negates the point of having "mixins".
The only thing you're "gaining" is that you've split your code into two files. Now you've got two places to look whenever you have to work with the code. Personally, I had to look back and forth quite a few times to figure stuff out. Since this is supposed to be a very straight-forward controller, that's not a good sign.

Besides, responsibility is all over the map now. In create_api_key you pass in both the user and the new API key instance. So there, your controller code handles params, and object instantiation. But in destroy_api_key you only pass only the user, and let the mixin method mess around with params to find the API key to destroy.

Optimally, a mixin has zero dependencies on its use context (your code depends on at least params, redirect_to which are only relevant for controllers), and is of general use (your code simply isn't). What you're doing is moving some lines of code and making things worse.

I can't actually see where you include the module... if you're doing it in ApplicationController you're polluting every controller with completely irrelevant methods.

And all those methods become public methods on every controller they're included in, meaning they function as actions. So worst case scenario, if you've got a too-liberal routing scheme, all of those methods can be called as actions simply by putting their names in the URL.

Odds and Ends

Don't pass the id attribute to path/url helpers; pass the record itself. If you ever want to override to_param in your models to make nicer URLs, it won't have any effect if all your code explicitly passes only the id attribute. I.e. redirect_to user_path(user.id) → redirect_to user_path(user)

Furthermore, for link_to and redirect_to you don't even need to use a path/url helper if it's a resourceful route; you can just pass the record. Rails will figure the rest out. I.e.
So redirect_to user_path(user) → redirect_to user

If you've only got index, create and destroy actions, then make that explicit in the routes. And there's no need for a slash in the path argument.
resources :api_keys, only: [:index, :create, :destroy], path: 'developmentcenter'

Your last test is missing an expectation. It just calls some stuff, but doesn't actually test anything. I'd suggest at least an expect {...}.to change(ApiKeys, :count).by(-1).

More tests in general - for instance, tests of all the security stuff mentioned above.

Proof-read your flash messages (and test 'em while you're at it).

Here's how I'd write it. I'm assuming you only want users to create/delete their own keys and no params are required to do so

class ApiKeysController < ApplicationController
  before_action :authenticate_user!

  def index; end # just access current_user.api_keys directly in the view

  def create
    api_key = current_user.api_keys.new
    if api_key.save
      redirect_to current_user, notice: 'Created a new API key'
    else
      redirect_to current_user, alert: 'Could not create a new API key. Please try again'
    end
  end

  def destroy
    api_key = current_user.api_keys.find params[:id]
    api_key.destroy
    redirect_to current_user, notice: 'API key has been deleted.'
  end
end

No mixins, yet shorter than the original, and a lot more secure


Ruby if-statement, possible to shorten it further? Is it possible to shorten this, or to improve the code in any way? I know it's just a simple if-statement, but it's always fun to learn a shorter way of doing things.

if self.citizenship == 'ES'
  @identification_type = IDENTIFICATION_TYPES[0]
elsif self.country == 'ES'
  @identification_type = IDENTIFICATION_TYPES[1]
end

You could remove a little duplication like this:

@identification_type = if self.citizenship == 'ES'
  IDENTIFICATION_TYPES[0]
elsif self.country == 'ES'
  IDENTIFICATION_TYPES[1]
end


Finding the first non-repeating character in a string Below is a problem I encountered during an online course. There are a lot of similar questions asked on this, but I couldn't find a solution there as well as I believe my specific issue is a bit different from the rest.

  Given a string s, find the first non-repeating character in the string
  If there is no such character print ‘@’.  You are required to complete the function

  Input Format:    String s

  Output Format: First non-repeating character or @

  You are supposed to implement char
  char FirstNonRepeatedchar(char* str, int len): which takes a pointer to input string str and length of the string(including ‘\0’). The function should return either the first non repeating character or ‘@’

  Input Constraint: All input characters will be either lower or upper case english alphabets.

   str[i]  [a,z][A,Z]

  There will not be any space in the input string.

 /************ Prefix Code Begins*******************/
 #include <iostream>
 #include <string>
using namespace std;

//function finds the first non-repeating character
 char FirstNonRepeatedchar(const char* , int);

int main(){

int len;
string line;
getline (cin, line);

len = int(line.length());

cout << FirstNonRepeatedchar(line.c_str(), len);

return 0;
}
/******************* Prefix Code Ends******************/

Also, note I cannot change anything in the prefix code above. I wrote 2 solutions: first one below is giving correct output.

char FirstNonRepeatedchar(const char* str, int len){
    int count[26]={0};
    for(int i=0;i<len;i++)
    {
        count[*(str+i)]++;
    }
    for(int i=0;i<len;i++)
    {
        if(count[*(str+i)]==1)
        return *(str+i);
    }
return '@';
}

The above solution iterates the string twice. So, I tried to improve the complexity by implementing it using a hash.

char FirstNonRepeatedchar(const char* str, int len){
int count[26]={0};
int j;

char c;
if(isupper(*str)==1)
{ c='A';
}
else
c='a';
for(int i=0;i<len;i++)
{   j=*(str+i)-c;
    if(count[j]<0)
    ;
    else if(count[j]==0)
    count[j]=i+1;

    else if(count[j]>0)
    count[j]=-i;

}
int min=len+1;
for(int i=0;i<26;i++)
{
    if(count[i]>0 && count[i]<min)
    min=count[i];

}
if(min==len+1)
return '@';
else
return *(str+min-1);
}

Is the second solution I gave the most optimized solution possible for this problem?

Let's try a solution that doesn't use an array with all the required bookkeeping.  The following code works, and bypasses the problems others have pointed out.

First, a quick template function that searches for duplicates given a char:

template <typename T>
inline bool repeated(char c, T first, T last) {
    size_t count = 0;
    while (first != last) {
        if (*first == c) ++count;
        if (count > 1) return true;
        ++first;
    }
    return false;
}

Now we create a set of unique characters and then scan the string to see if there are any duplicates.  If there are, we remove the char from the set and continue to the next one.

char FirstNonRepeatedchar(const char* str, int len) {
    auto uniq = std::set<char>(str, str + len);
    auto first = str;
    auto last = str + len;
    while (first != last) {
        if (auto it = uniq.find(*first) != uniq.end()) {
            if (!repeated(*first, str, str + len)) return *first;
            uniq.erase(it);
        }
        ++first;
    }
    return '@';
}


Determine if a section is in view i am currently learning to do things with vanilla Javascript. No plugins, libraries or code snippets. I want to do things on my own, so that I understand 100% of what is happening.

I wrote a little script which checks, if a section of the page is in view.

Now, the script works as expected, but I was wondering, if I had accomplished the task in a good way. I am especially unsure about looping over the sections in a scroll event. I would love to get some feedback on this. Example page

Here is the code

(function () {
    // Store all panels in variable
    const panels = document.querySelectorAll(".panel");

    // Window on load event
    window.addEventListener("load", () => {
        let scrollTop = window.scrollY;
        let windowHeight = window.innerHeight;
        let documentHeight = document.body.offsetHeight;

        panels.forEach( panel => {
            if(scrollTop == panel.offsetTop ){
                panel.classList.add("is-visible");
            }
        });
    }); // end of load event

    // Window on scroll event
    window.addEventListener("scroll", () => {
        let scrollTop = window.scrollY;
        let windowHeight = window.innerHeight;
        let documentHeight = document.body.offsetHeight;

        panels.forEach( panel => {
            if(scrollTop >= panel.offsetTop - ( windowHeight / 2)){
                panel.classList.add("is-visible");
            } else {
                panel.classList.remove("is-visible");
            }
            if(scrollTop >= (panel.offsetTop + panel.clientHeight) - ( windowHeight / 2) ) {
                panel.classList.remove("is-visible");
            }
        });

    }); // end of scroll event
})();

APPENDIX

So I refactored the code as follows. I am still not sure about things like the IIFs.

(function(){

            const panels = document.querySelectorAll(".panel");
            let scrolling = false;

            (function windowOnLoad () {

                // Window on load event
                window.addEventListener("load", () => {
                    let scrollTop = window.scrollY;
                    let windowHeight = window.innerHeight;
                    let documentHeight = document.body.offsetHeight;

                    panels.forEach( panel => {
                        if(scrollTop == panel.offsetTop ){
                            panel.classList.add("is-visible");
                        }
                    });
                }); // end of load event
            })();

            (function windowOnSCroll(){
                window.addEventListener("scroll", () => {
                    scrolling = true;
                });
            })();

            setInterval( function() {
              if (scrolling) {
                scrolling = false;
                let scrollTop = window.scrollY;
                let windowHeight = window.innerHeight;
                let documentHeight = document.body.offsetHeight;

                panels.forEach( panel => {
                    if(scrollTop >= panel.offsetTop - ( windowHeight / 2)){
                        panel.classList.add("is-visible");
                    } else {
                        panel.classList.remove("is-visible");
                    }
                    if(scrollTop >= (panel.offsetTop + panel.clientHeight) - ( windowHeight / 2) ) {
                        panel.classList.remove("is-visible");
                    }
                });

              }
            }, 250 );

        })();

I am especially unsure about looping over the sections in a scroll event.

There doesn't seem to be much of an alternative. One could select all elements with class name is-visible:

const visible = document.getElementsByClassName('is-visible');

Note that that collection is live, meaning it will be updated dynamically as the DOM changes, so there isn't any reason to re-query the DOM for those.

In the version that attempts to debounce the DOM updates, the IIFEs seem a bit excessive. Is the goal to limit scope of variables?

The function passed to panels.forEach() could be simplified:

panel => {
    if(scrollTop >= panel.offsetTop - ( windowHeight / 2)){
        panel.classList.add("is-visible");
    } else {
        panel.classList.remove("is-visible");
    }
    if(scrollTop >= (panel.offsetTop + panel.clientHeight) - ( windowHeight / 2) ) {
        panel.classList.remove("is-visible");
    }
}

This can be simplified by calling Element.classList.toggle() with two arguments.

panel => {
    let addClass = scrollTop >= panel.offsetTop - ( windowHeight / 2);

    if(scrollTop >= (panel.offsetTop + panel.clientHeight) - ( windowHeight / 2) ) {
        addClass = false;
    }
    panel.classList.toggle("is-visible", addClass);
}

const panels = document.querySelectorAll(".panel");

In most browsers it would generally be quicker to fetch the game squares using document.getElementsByClassName(), but that would return a live collection (refer to this SO answer for an explanation).

 const panels = [...document.getElementsByClassName("panel")];

The debounced version looks a little bit un-efficient because it uses an interval so that function gets executed more than it needs to.

There are also many arguments not to use setInterval and setTimeout - e.g. blindman67's SO Post Why is requestAnimationFrame better than setInterval or setTimeout

The functionality from the setInterval callback could be put into a function called checkVisibility. Then the scroll handler could be simplified to use requestAnimationFrame()

let timeout;
window.addEventListener("scroll", _ => {
    if (timeout) {
        window.cancelAnimationFrame(timeout);
    }
    timeout = window.requestAnimationFrame(checkVisibility);
});


House-coloring optimization challenge I have an interview coming up in the next few weeks, and I'm choosing to be interviewed in Python. I began programming in Python (it was about four years ago), so the syntax is natural to me, but I only learned about some useful tools of Python just this past year, when I took a class that recommended using it.

The Problem

We'll say there are \$N\$ houses, where \$N\$ is some integer. Each house can be painted in either Red, Green or Blue. The cost of coloring each house in each of the colors is different.

Figure out how to color each house so no two adjacent houses have the same color and the total cost of coloring all the houses is as low as possible.

My Solution

# Assume the prices of painting each house in a specific color is
# given by an array of size 3xN, giving the price to paint each house
# for each specific color
def find_cheap_house_painters(N, prices):
    # initialize the totals array
    totals = [[0 for i in range(N)] for j in range(3)]

    # the first houses are special case
    # we just paint them, that's it.
    for color in range(3):
        totals[color][0] = prices[color][0]

    # after that, dynamic programming does its magic.
    # at each step, add the price of painting this house
    # to the minimum of the total painting of the other two colors
    colors = set(range(3))
    for house in range(1, N):
        for color in colors:
            previous_totals = [totals[other_color][house-1] for other_color in (colors - {color})]
            totals[color][house] = prices[color][house] + min(previous_totals)

    # so now we have the totals to paint up to each house
    # we'll just work backwards to find the coloring of the houses.
    colorings = [3 for i in range(N)]
    colorings[-1] = min(colors, key=lambda x : totals[x][-1])
    for house in range(N-2, -1, -1):
        # take the index of the smaller option
        other_colors = colors - {colorings[house+1]}
        colorings[house] = min(other_colors, key=lambda x : totals[x][house])

    # convert to a human-readable string
    color_names = "RGB"
    house_colorings = "".join([color_names[colorings[house]] for house in range(N)])
    return house_colorings

My Questions

Are there specifically Pythonic structures, idioms, or "things" that I should be aware of that would help in this specific problem? If you're so inclined, I'm also looking for a list of generally useful and Pythonic "things". I became aware of list comprehension, any(), all(), and lambdas only recently, so tools at about that level are helpful.
I have yet to take my university's Programming Studio class, in which I'd learn a lot about writing readable, sensible, and maintainable code. How is my variable naming, commenting, etc? Again, if you're so inclined, if you want to provide a list of common errors and bad habits, I'd appreciate it.
Anything that strikes you. Just because I didn't specifically ask for it here doesn't mean it'll won't help me write better code.

I do have a lot of comments, but I find your solution pretty good.  The algorithm makes sense, and your comments make it easy to follow.

The function is closer to a computation than a search, so I wouldn't call it find….  The N parameter is redundant, since it can be deduced from the dimensions of prices.  On the other hand, color_names is hard-coded (and worse, 3 is hard-coded in several places); I would make it parameterized.  The comment on the function should be a docstring.

I would prefer to return a list; the caller can join them into a string if desired.

I can understand crashing if there are 0 colors (because the problem would be ill-defined in that case), but the function should not crash if there are 0 houses.

The algorithm works by "walking up and down the street", evaluating the color choice at each house.  Therefore, the code would be more natural if you transposed the matrices,1 such that the primary axis is the house number, and the secondary axis is the color.  You can observe the benefit when writing the totals for the first house: your solution slices prices; my solution below just copies an element.

You initialize totals to a matrix of 0s and colorings to a list of 3s.  I prefer to allocate empty lists using [None] * n.

You perform set operations on colors.  I prefer to use list comprehensions with an if filter.  I suspect that that the filtered list comprehension should be faster, though your set operations might be easier to understand.

def cheapest_house_coloring(prices, color_names='ROYGBIV'):
    """
    Finds the cheapest way to color a list of N houses using C colors such that
    no two consecutive houses have the same color.

    prices is an N-by-C array containing the price to paint the house n with
    color c.

    color_names is a sequence containing the names of the colors.
    """
    num_houses = len(prices)
    if not num_houses:
        return []
    num_colors = len(prices[0])

    totals = [None] * num_houses
    totals[0] = prices[0][:]

    # After the first house, dynamic programming does its magic.
    # At each step, add the price of painting this house
    # to the minimum of the total painting of the other colors.
    for house in range(1, num_houses):
        totals[house] = [
            prices[house][color] +
            min(totals[house - 1][c] for c in range(num_colors) if c != color)
            for color in range(num_colors)
        ]

    # Now we have the totals to paint up to each house.
    # We'll just work backwards to find the coloring of the houses.
    colorings = [None] * num_houses
    colorings[-1] = min(range(num_colors), key=lambda c: totals[-1][c])
    for house in range(num_houses - 2, -1, -1):
        # Take the index of the cheapest option (infinite cost
        # prohibits consecutive houses with the same color)
        colorings[house] = min(range(num_colors),
            key=lambda c: totals[house][c] if c != colorings[house + 1]
                else float('Infinity'))

    # Convert color indexes to names
    return [color_names[colorings[house]] for house in range(num_houses)]

1 If you need to transpose a matrix, use list(zip(*matrix)).


Coordinating Threads with Reactive Streams I just started learning reactive streams with RxJava. After reading a couple of books and a lot of articles our there I am still having trouble understanding how to coordinate multiple threads.

I would appreciate a critique of the following code, particularly if anyone knows a better way to do it.

I basically want to execute multiple tasks in the background, and as results start to get back I want to get immediately notified of the results. I want the background tasks to run in individual threads, but I want the notification thread to run in a single thread.

My idea is that the notification code does not need to worry about synchronization and I can achieve that by making sure the notification code only runs in one thread.

This was my best attempt of that:

public class Question {

    public static void main(String[] args) {

        //A hypothetical list of tasks to run asynchronously
        List<Callable<String>> tasks = Arrays.asList(
            () -> "One: " + Thread.currentThread().getName(),
            () -> "Two: " + Thread.currentThread().getName(),
            () -> "Three: " + Thread.currentThread().getName()
        );

        //A blocking queue to hold the results when ready
        BlockingQueue<String> queue = new ArrayBlockingQueue<>(3);

        //when a task is done, this observer puts it in the queue.
        //observer code then will run in the currently processing thread
        Observer<String> observer = Observers.create(queue::offer);
        tasks.stream().map(Question::async).forEach(result -> result.subscribe(observer));

        //as tasks get resolve and enter the queue, this other observer process the results
        //in my current thread, not in any of the task threads.
        consumer(queue, 3).forEach(item -> {
                System.out.println("Received " + item + " at " + Thread.currentThread().getName());
            });
    }

    static <T> Observable<T> async(Callable<T> supplier) {
        return Observable.<T>create(subscriber -> {
            try {
                subscriber.onNext(supplier.call());
                subscriber.onCompleted();
            }
            catch (Exception ex) {
                if (!subscriber.isUnsubscribed()) {
                    subscriber.onError(ex);
                }
            }
        }).subscribeOn(Schedulers.computation());
    }

    static <T> Observable<T> consumer(BlockingQueue<T> queue, int count) {
        return Observable.<T>create(subscriber -> {
            for (int i = 0; i < count; i++) {
                try {
                    T text = queue.take();
                    subscriber.onNext(text);
                }
                catch (InterruptedException ex) {
                    subscriber.onError(ex);
                }
            }
            subscriber.onCompleted();
        }).observeOn(Schedulers.immediate());
    }
}

And I get:

Received One: RxComputationThreadPool-1 at main
Received Two: RxComputationThreadPool-2 at main
Received Three: RxComputationThreadPool-3 at main

So, this works like a charm, but I have the feeling it is still too verbose and that maybe there is a way to make it simpler with RxJava somehow.

You are jumping through a lot of hoops to get your queue, consumer, and observer strategies aligned. I look at your code, and I don't really follow it all. Part of the reason is that I don't know RxJava at all, but, normally, I can follow these things without much research.

So, your code streams the tasks, for each one, it creates an Observable for it which is subscribed on the Reactive computation engine. The observer for the Observable is set to add the result to the queue.

Now, for the queue, you create an Observer that registers one action for each expected result. You then loop over those events, and print the result.

I can understand why you may want to use RxJava for some situations, but, that last part makes no sense to me... Let me show you your code:

//as tasks get resolve and enter the queue, this other observer process the results
//in my current thread, not in any of the task threads.
consumer(queue, 3).forEach(item -> {
        System.out.println("Received " + item + " at " + Thread.currentThread().getName());
    });

where consumer is:

static <T> Observable<T> consumer(BlockingQueue<T> queue, int count) {
    return Observable.<T>create(subscriber -> {
        for (int i = 0; i < count; i++) {
            try {
                T text = queue.take();
                subscriber.onNext(text);
            }
            catch (InterruptedException ex) {
                subscriber.onError(ex);
            }
        }
        subscriber.onCompleted();
    }).observeOn(Schedulers.immediate());
}

What does that code do? It looks for three strings on a queue, and prints them.

for (int i = 0; i < 3; i++) {
    System.out.println("Received " + queue.take() + " at " + Thread.currentThread().getName());
}

What's wrong with that (apart from the 3 constant)? ^^^^

Having figured out all the loops you are going through, I struggled to understand the advantage that RxJava is giving you, especially when Java has already got the "right tool" in the java.util.concurrent.* toolbox.

Let me explain, the CompletionService concept says: Submit a bunch of jobs to the service, and the service will tell you when they complete. It has a submit(...) method to add jobs to the service, and a take(...) method which will wait until the next-completed task, and return it.

Creating a Thread Factory that matches the names you have in RxJava quickly, I have this code:

private static final AtomicInteger threadId = new AtomicInteger(0);

private static Thread threadFactory(Runnable r) {
    Thread t = new Thread(r, "RxComputationThreadPool-" + threadId.incrementAndGet());
    t.setDaemon(true);
    return t;
}

private static ExecutorService pool = Executors.newCachedThreadPool(Reactive::threadFactory);

That code creates an ExecutorService which has threads named things like your code's results.

Now, with that code available, I can have a really simple problem space:

public static void main(String[] args) throws InterruptedException, ExecutionException {
    List<Callable<String>> tasks = Arrays.asList(
            () -> "One: " + Thread.currentThread().getName(),
            () -> "Two: " + Thread.currentThread().getName(),
            () -> "Three: " + Thread.currentThread().getName()
        );

    int count = tasks.size();

    CompletionService<String> completor = new ExecutorCompletionService<>(pool);

    tasks.stream().forEach(completor::submit);
    for (int i = 0; i < count; i++) {
        System.out.println("Received: " + completor.take().get() + " at " + Thread.currentThread().getName());
    }

}

Note that the CompletionService returns a completed Future<String>, not the actual String, so you have to get() the string from the Future<String>.

I have put this code up on to ideone so you can see it in action.

Note, the solution I provided has no need for RxJava, and that's because, if you want to execute a bunch of things asynchronously, and retrieve the results in a single thread, then perhaps the observable pattern is not the right one....


Making operations more dynamic I am trying to build something that will allow us to configure "custom" validation for each customer.  So if a customer wants to see a value contain a specific char, be specific length, or equal something else they just add a pre-created option.  I wrote something that seems to be very primitive, but works.  I would like it to be more dynamic.  For instance, I can pass in actual C# code and it will perform that operation on the value input and being compared.

public class Operation
{
    public object value { get; set; }
    public OMagic perform { get; set; }
}

public class Magic
{

    public bool Validate(object val, Operation operation)
    {
        switch (operation.perform)
        {
            case OMagic.Equal:
                return (string)val == (string)operation.value;
            case OMagic.NotEqual:
                return (string)val != (string)operation.value;
            case OMagic.LengthGreaterThan:
                return (int)val < (int)operation.value;
            case OMagic.LengthEqual:
                return (int)val == (int)operation.value;
            case OMagic.LengthLessThan:
                return (int)val > (int)operation.value;
            default:
                return false;
        }
    }
}

public enum OMagic
{
    Equal,
    NotEqual,
    LengthGreaterThan,
    LengthEqual,
    LengthLessThan

}

That is the primitive code, and here are the tests I wrote (using nunit)

[TestFixtureSetUp]
    public void SetupTest()
    {
        //CurrentMessage = getMessage(WebIZ.Test.UnitTest.Properties.Resources.TestResponse1);
        Magic = new Magic();
    }

    private Magic Magic;

    [Test]
    public void Text_Compare_Works()
    {
        Assert.IsTrue(Magic.Validate("yes", new Operation() { value = "yes", perform = OMagic.Equal }));
        Assert.IsFalse(Magic.Validate("yes", new Operation() { value = "no", perform = OMagic.Equal }));
        Assert.IsTrue(Magic.Validate("yes", new Operation() { value = "no", perform = OMagic.NotEqual }));
        Assert.IsFalse(Magic.Validate("yes", new Operation() { value = "yes", perform = OMagic.NotEqual }));
    }

    [Test]
    public void Int_Compare_Works()
    {
        Assert.IsFalse(Magic.Validate(7, new Operation(){value = 7, perform=OMagic.LengthGreaterThan}));
        Assert.IsTrue(Magic.Validate(7, new Operation() { value = 8, perform = OMagic.LengthGreaterThan }));
        Assert.IsFalse(Magic.Validate(7, new Operation() { value = 7, perform = OMagic.LengthLessThan }));
        Assert.IsTrue(Magic.Validate(7, new Operation() { value = 6, perform = OMagic.LengthLessThan }));
        Assert.IsFalse(Magic.Validate(7, new Operation() { value = 8, perform = OMagic.LengthEqual }));
        Assert.IsTrue(Magic.Validate(7, new Operation() { value = 7, perform = OMagic.LengthEqual }));
    }

Well there is multiple solution to this problem ,

Using delegate:

public bool Validate<T1,T2>(T1 value, T2 compareValue, Func<T1,T2,bool> operation)
{
    return operation(value, compareValue);
}
magic.Validate(1, 2, (a, b) => a > b);
magic.Validate("Testing", "Testing", (a, b) => a.Equals(b));

but twp problems with this code is : first your input can be limited , right now you cannot compare more than two values , so you might need to see what can be done to extend this code.

second you can make a switch case to return the right method for comparision.

Using the Expression which could be more compact and will be cleaner code. I have not tried this yet.
Avoid using private variable naming with capital letters.
Avoid giving a name where two capital letters are at start (OMagic), you could write this like OperationMagic.
Avoid boxing and unboxing (if your operations are small dont worry).


Parallel Reduction method with C++AMP I am writing a C++AMP library, and as one of my utility methods I am implementing a parallel reduction algorithm based on the cascade method documented on this blog post with slight improvements by utilizing asynchronous CPU execution.
Does anyone have any performance/correctness/language improvements to offer?
template <typename T, std::size_t TileSize = DEFAULT_TILE_SIZE, std::size_t NumTiles = DEFAULT_NUM_TILES>
auto ParallelAccumulate( const concurrency::array_view<const T, 1>& avData ) -> decltype(std::declval<T>() + std::declval<T>())
{
    static_assert(IsPowerOfTwo( TileSize ), "Tile Size must be a power of two");
    static_assert(is_amp_compatible<T>::value, "The internal type of the array_view must be amp comptible");
    static_assert(NumTiles > 0, "There must be a non-zero number of tiles");

    std::size_t sArrayLength = avData.extent.size();
    const std::size_t sStrideLength = TileSize * NumTiles * 2U;

    // Accumulate tail (if necessary):
    const std::size_t sTailLength = sArrayLength % sStrideLength;
    std::future<decltype(std::declval<T>() + std::declval<T>())> futTailSum;
    if( sTailLength != 0 )
    {
        std::vector<T> vecTail( sTailLength );
        concurrency::copy( avData.section( concurrency::index<1>( sArrayLength - sTailLength ) ), vecTail.begin() );
        futTailSum = std::async( std::launch::async, [&vecTail]{ return concurrency::parallel_reduce( vecTail.begin(), vecTail.end(), static_cast<T>(0) ); } );

        if( (sArrayLength -= sTailLength) == 0 )
        {
            return futTailSum.get();
        }
    }

    concurrency::array<decltype(std::declval<T>() + std::declval<T>()), 1> arrPartialResult( NumTiles );
    concurrency::parallel_for_each( concurrency::extent<1>( TileSize * NumTiles ).tile<TileSize>(), [=, &arrPartialResult]( concurrency::tiled_index<1> tIndex ) restrict( amp ) {
        tile_static decltype(std::declval<T>() + std::declval<T>()) tTileData[TileSize];
        std::size_t sLocalIndex = tIndex.local[0];

        std::size_t sInputIndex = (tIndex.tile[0] * 2U * TileSize) + sLocalIndex;
        tTileData[sLocalIndex] = static_cast<decltype(std::declval<T>() + std::declval<T>())>(0);

        do {
            tTileData[sLocalIndex] += avData[sInputIndex] + avData[sInputIndex + TileSize];
            sInputIndex += sStrideLength;
        } while( sInputIndex < sArrayLength );

        tIndex.barrier.wait();

        for( std::size_t sStride = TileSize / 2U; sStride > 0; sStride /= 2U )
        {
            if( sLocalIndex < sStride )
            {
                tTileData[sLocalIndex] += tTileData[sLocalIndex + sStride];
            }

            tIndex.barrier.wait();
        }

        if( sLocalIndex == 0 )
        {
            arrPartialResult[tIndex.tile[0]] = tTileData[0];
        }
    } );

    std::vector<decltype(std::declval<T>() + std::declval<T>())> vecPartialResult( NumTiles );
    concurrency::copy( arrPartialResult, vecPartialResult );
    return concurrency::parallel_reduce( vecPartialResult.begin(), vecPartialResult.end(), static_cast<T>(0) ) + futTailSum.get();
}

I only see some possible stylistic/reusability improvements:

You use decltype(std::declval<T>() + std::declval<T>()) five times in the function (plus the one in the return type). You could probably typedef that to improve readability. I don't know how you would call it, but I am pretty sure that you can find a meaningful name.
By the way, are there many types for which decltype(std::declval<T>() + std::declval<T>) is different from T while still being amp compatible? If it is the case, that may break some other pieces of your code anyway (I didn't check though; that's a wild guess).
Whenever possible, try to use std::begin and std::end instead of the member functions. It helps to write reusable code; you might someday want to extract parts of your function to write smaller and more generic functions that could work on old C arrays.
By default, if you don't specify the rank of an array_view, it is one. Therefore, you actually don't have to specify it since you always use 1.
Your lines are really long. As I said in the comments, you might want to split some of them to improve readability:

futTailSum = std::async( std::launch::async, [&vecTail]
    {
        return concurrency::parallel_reduce( std::begin(vecTail),
                                             std::end(vecTail),
                                             static_cast<T>(0) );
    } );


The base-ten value of n-base number with Python To start off I'd like to note I am aware of int(val, base). My goal was to write a short function capable of taking in a base int, and a string defining the value to be summed.

The function rests upon the equation:

  represented_value = value * base ^ placeholder

>>> base_v = lambda base_v, num_a: sum(int(num_a[len(num_a) - ph - 1]) * base_v ** ph for ph in range(len(num_a)))
>>> base_v(10, "123")
123
>>> base_v(100, "123")
10203

I can even do something silly like:

>>> base_v(-10, "123")
83
>>>
>>> base_v(-10, ["123", "123", "123"])
11193

I am curious how this function can be made more succinct and what pitfalls it may have.

A few quick comments:

Use a proper function here, not a lambda. It’s generally more readable, and should be preferred unless you’re writing a ~very short function. And then use different variable names for the function and its arguments, otherwise things get confusing.

Trying to make code more concise is good, but be careful not to make it too short.
This function probably needs a docstring (especially because the argument order is the opposite of that used by int()) and a comment. It took me a minute to work out how the code that’s written corresponds to your equation – and indeed, the equation itself isn’t entirely obvious.
Your function allows me to pass inputs that don’t make sense. For example:

>>> print base_v(2, "3")
3

But 3 isn’t a valid base-2 number: you should throw a ValueError here (like int() does).
Rather than indexing from the end of the string with num_a[len(num_a) - ph - 1], you can use negative indexing just to do num_a[-ph-1], which is cleaner and neater.
ETA: The other problem is that it can only deal with the digits 1–10. It doesn’t deal with numeric strings that contain letters, e.g. base_v(16, "a") throws a ValueError, even though a is a valid base-16 number.


Uploading multiple images to Google Cloud Storage using Node.js I am no JavaScript expert, and I have managed to hack together a node.js script that does exactly what I want it to do: upload images contained within a folder to Google Cloud Storage for a Firebase project, and then return the public access URLs. Given that I have limited experience with node.js combined with GC Storage, please advise me on any issues that running this code over approximately 200 images in the folder may cause.

var fs = require('fs');
const {Storage} = require('@google-cloud/storage');

const projectId = 'XXXXXXX';

//Creates a client
const storage = new Storage({
    projectId: projectId,
    keyFilename: 'auth.json'
  });

// Reference the bucket
var bucket = storage.bucket('XXXXXXX.appspot.com');

//This reads the folder where the images are stored
fs.readdir('ImagesToUpload', (err, files) => {

    if( err ) {
        console.error( "Could not read the directory.", err );
        process.exit( 1 );
    }

    files.forEach(function( file, index ) {

        var filePath = 'ImagesToUpload/'

        console.log(file)

        // Upload a local file to a new file to be created in the bucket
        bucket.upload(filePath += file, (err, file) => {
            if (err) { return console.error(err); }
            let publicUrl = `https://firebasestorage.googleapis.com/v0/b/${projectId}.appspot.com/o/${file.metadata.name}?alt=media`;
            console.log(publicUrl);
        });
    })
})

Looks good. A few nits:

You might as well use const where you can, and let when that doesn't work. You can avoid var completely.
Instead of var filePath = 'ImagesToUpload/'-- and then modifying filePath with +=, just set filePath to the correct path immediately, ie. const filePath = 'ImagesToUpload' + file. This is just simpler and easier to read.
A subtler point: The logs are providing you your only reference to the file mapping onto the server. I'm concerned that these async functions won't necessarily run in order. Consider a huge file followed by a small file. Would the loop of files allow bucket.upload calls to be running at the same time? I would assume so. The small one would complete before the large one. If this happens, it's possible that your log console.log(file) will happen out of sequence with the console.log(publicUrl). This can be solved by moving the first console log into the loop right before the publicUrl log.
Unused arguments at the end of the list can be omitted safely in JS, so function( file, index ) can be simply function( file), or even file => if you use the new ES syntax (which you are)


Get count of credit cards that are expired Below is a sample implementation for getting the expired credit cards in the credit card list.I wrote a property and initialized it in the constructor. Need an opinion on if this is a proper way to implement it.

   public class User
    {
        private readonly List<CreditCard> creditCards;

        public int CountOfCreditCards
        {
            get
            {
                return counter;
            }
        }

        private static int counter;

        private User()
        {
            creditCards = new List<CreditCard>();
            counter = GetCountOfExpiredCreditCards();
        }

        public int GetCountOfExpiredCreditCards()
        {
            return creditCards.Count(x => x.ExpirationMonth <= DateTime.Now.Month);
        }
    }

If a class has one or more private constructors and no public constructors, other classes (except nested classes) cannot create instances of this class.

https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/private-constructors

Make your constructor public.

There is an obvious bug in GetCountOfExpiredCreditCards  A card that expires in 01/19(January, 2019) will show up as expired if checked today.  Check for both year and month.


Comparing triangles using Java 8 streams I'm trying to get more familiar with Java 8 streams as they seem to be very powerful (and shorter), so I rewrote a method to use streams. However I'm not very satisfied with it and would like some suggestions on how to make better use of streams. The code itself has multiple nested loops, so it's not very easy. Also I'm forced to use final arrays to write the variables inside the lambda loops.

// t1, t2 are List<Triangle> passed as parameter
Triangle original = t1.get(0);
t2.forEach(transformed ->
{

    for (int[] permutation : permutations)
    {
        Matrix trial = calculateMatrix(original.getVertices(), applyPermutation(transformed.getVertices(), permutation));

        final double[] sum = {0};
        points1.stream().map(v -> v.extend(1)).map(trial::multiply).forEach(v -> sum[0] += points2.stream().mapToDouble(v2 -> distanceCheated(v, v2)).min().orElse(0));

        if (bestDeviation[0] == null || sum[0] < bestDeviation[0])
        {
            second[0] = transformed;
            bestDeviation[0] = sum[0];
            bestMatrix[0] = trial;
        }
    }
});

EDIT: So as I aparently left out many important information, i try to provide them here:

The first obvious missing information is the connection between points1/2 and t1/2. points1/2 are plain two lists of points contained in two images (original and transformed). t1/2 are lists of triangles built with delauney triangulation using the points from points1/2. The goal is to calculate the transformation matrix used, to create the second set of points.

I posted the whole class here: https://gist.github.com/anonymous/8f21597d8a51c8960de7
Vertex is used as a Vector (mathematical)
Triangle consists of 3 Vertices

Since (as mentioned in my comment) the question is very tightly scoped, this review will be mostly style-focused:
Names
Some of your names are good. Some of your names are bad. I didn't see anything really ugly though, so you got that going for you ;)
Some examples of the good names include:

original, permutation, bestDeviation, trial

These names are speaking for themselves, it's almost instantly clear what they  mean.
On the other hand there's things like:

t1, t2, v, v2

These names are completely meaningless. I can't tell what they stand for, I have no idea (not in the slightest) what is in the variables with these names. It would tremendously help to understand your code's purpose, if I (or Mr. Maintainer) knew what the smallest units the code works with are.
I'd guesstimate you're operating on Vectors in a graphical representation of bodies, but I simply cannot be sure.
Anyways, from naming to:
Spacing and Linebreaks:
Thge code could use some additional linebreaks here and there, if only just to reduce the "verical complexity" of your code.
Normally I try to portion my code so you can read it one line at a time, and almost instantly understand what happens on it.
That's extremely hard when I need to scroll and there's multiple (low-level) transformations and operations. Try to write code, which doesn't take long to skim over and get the gist of.

points1.stream().map(v -> v.extend(1)).map(trial::multiply).forEach(v -> sum[0] += points2.stream().mapToDouble(v2 -> distanceCheated(v, v2)).min().orElse(0));

then becomes:
points1.stream().map(v -> v.extend(1))
        .map(trial::multiply)
        .forEach(v -> sum[0] += points2.stream()
                .mapToDouble(v2 -> distanceCheated(v, v2))
                .min().orElse(0)
        );

On the other hand there's one place where I feel you left too much space:

t2.forEach(transformed ->
{

    for (int[] permutation : permutations) {

I'd write it like this:
t2.forEach(transformed -> {
    for (int[] permutation : permutations) {

Other than this, there's no stylistic points to make here, and as mentioned I don't want to review the functionality without more information and context.


Random "programming language" I decided to write my own programming language, of sorts.  Essentially, you write code into a text file and the program will run the code inside the text file. The file is provided through a script argument.

# Basic programming language
from operator import eq, ne, lt, gt, le, ge
from random import random
from sys import exit, argv

# Specify file to be run
code = argv([1])

"""
>> - Specify to the user that something will be outputted.
<< - Specify to the user that something will be inputted.
"""

# Dict storing operators
OPERATORS = {
    "==": eq,
    "!=": ne,
    "<=": le,
    ">=": ge,
    "<": lt,
    ">": gt
}

# Dict storing program data
data = {
    "vars": [0] * 100000,
    "rand": random()
}

# Interpret the code
def interpreter(separator):
    for line in open(code, "r").readlines():

        # Split each line
        s_command = line.split(separator)

        # Basic input and output
        if s_command[0] == "$out" and s_command[1] == ">>":
            if s_command[2] in data:
                print data[s_command[2]]
            if s_command[2] not in data:
                print s_command[2]
        if s_command[0] == "$inp" and s_command[1] == "<<":
            inp = raw_input(s_command[2])

        # Assign integer variables
        if s_command[0] == "$assignint" and s_command[1] == "<<":
            data["vars"][int(s_command[2])] = int(s_command[3])

        # Assign string variables
        if s_command[0] == "$assignstr" and s_command[1] == "<<":
            data["vars"][int(s_command[2])] = s_command[3]

        # Return variable values
        if s_command[0] == "$return" and s_command[1] == ">>":
            print data["vars"][int(s_command[2])]

        # Test integer values
        if s_command[0] == "$testint" and s_command[1] == ">>":
            if s_command[3] in OPERATORS:
                print OPERATORS[s_command[3]](
                    int(s_command[2]),
                    int(s_command[4]))

        # Test string values
        if s_command[0] == "$teststr" and s_command[1] == ">>":
            if s_command[3] in OPERATORS:
                print OPERATORS[s_command[3]](
                    s_command[2],
                    s_command[4])

        # Test variable values
        if s_command[0] == "$testvar" and s_command[1] == ">>":
            if s_command[3] in OPERATORS:
                print OPERATORS[s_command[3]](
                    data["vars"][int(s_command[2])],
                    data["vars"][int(s_command[4])])

        # Terminate the program
        if s_command[0] == "$exit" and s_command[1] == ">>":
            exit(int(s_command[2]))

# Run the interpreter
if __name__ == "__main__":
    while True:
        interpreter(":")

Here's an example of a code file:

$out:>>:Hello world.
$inp:<<:[input]>

$assignint:<<:0:10
$assignint:<<:1:11

$return:>>:0
$return:>>:1

$testvar:>>:0:==:1
$testvar:>>:0:!=:1

$exit:>>:0

All I'm really looking for is a way to turn that ugly if/else chain into something prettier without changing the original syntax.

It looks like all your tests follow the pattern

if s_command[0] == "$xxxxx" and s_command[1] == "yyyyy":

So what you can do is create a tuple of the first two elements of s_command:

instr = tuple(s_command[:2])

Then, you can do

if instr == ("$xxxxx", "yyyyy"):

Next, you can use a dictionary to collect all your instructions in one place:

Instructions = {
    ("$out", ">>"): do_out,
    ("$inp", "<<"): do_inp,
    ("$assignint", "<<"): do_assignint,
    ...
}

Then you would define a number of functions, one for each instruction, such as

def do_inp(s_command):
    inp = raw_input(s_command[2])

and call your methods with

Instructions[instr](s_command)


New Spreadsheet Row to YouTube Playlist I have the following code below that takes a new row added to my Google Spreadsheet (has a header row) and extracts the video ID. If the ID does not exist in the playlist, it adds the video to the playlist. If it doesn't, it skips it. Not really comfortable with Google Apps Script, so I would like some constructive feedback.
function addVideoToYouTubePlaylist() {
  // retrieve video ID list from the playlist.
  var playlistId = "PL6bCFcS8yqQxSPjwZ9IXFMfVm6kaNGLfi";
  var list = [];
  var pageToken = "";
  do {
    var res = YouTube.PlaylistItems.list(["snippet"], {playlistId: playlistId, maxResults: 50, pageToken: pageToken});
    if (res.items.length > 0) list = list.concat(res.items);
    pageToken = res.nextPageToken || "";
  } while (pageToken);
  var obj = list.reduce((o, e) => Object.assign(o, {[e.snippet.resourceId.videoId]: e.snippet.resourceId.videoId}), {});

  // read the source videos from Google Sheet
  var sheet = SpreadsheetApp.getActiveSheet();
  var data = sheet.getDataRange().getValues();

  // iterate through all rows in the sheet
  for (var d=1,l=data.length; d<l; d++) {
    var videoId = extractVideoID(data[d][0]);
    if (!obj[videoId]) {
    YouTube.PlaylistItems.insert({
      snippet: {
        playlistId: playlistId,
        resourceId: {
          kind: "youtube#video",
          videoId: extractVideoID(videoId)
          }
        }
      }, "snippet");
    }
    else{
      console.log("Already in playlist");
    }
    sheet.deleteRow(d+1); // delete row after video is checked
  }
}

function extractVideoID(url){
    var regExp = /^.*((youtu.be\/)|(v\/)|(\/u\/\w\/)|(embed\/)|(watch\?))\??v?=?([^#\&\?]*).*/;
    var match = url.match(regExp);
    if ( match && match[7].length == 11 ){
        return match[7];
    }
    else if (!( match && match[7].length == 11 )){
        console.log("Could not extract video ID.");
        var trimmedVideoID = url.replace("https://youtu.be/", "");
        trimmedVideoID = trimmedVideoID.replace('https://www.youtube.com/watch?v=', "");
        trimmedVideoID = trimmedVideoID.replace('https://youtube.com/watch?v=', "");
        trimmedVideoID = trimmedVideoID.replace("&feature=youtu.be", "");
        trimmedVideoID = trimmedVideoID.replace("&feature=share", "");
        console.log(trimmedVideoID);
        return trimmedVideoID;
    }
}

Let's start with extractVideoID().
Your regex /^.*((youtu.be\/)|(v\/)|(\/u\/\w\/)|(embed\/)|(watch\?))\??v?=?([^#\&\?]*).*/ doesn't guarantee the URL has a YouTube format. For example, the URL www.google.com/watch?v=01$@T$89h does match the regex and has the following groups: ["www.google.com/watch?v=0123456", "watch?", undefined, undefined, undefined, undefined, "watch?", "01$@T$89h"], where the 7th group 01$@T$89h isn't a valid YouTube video ID.
I suggest you replace your regex with /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w\-]+\?v=|embed\/|v\/)?)([\w\-]+)(\S+)?$/ (source). This regex only matches valid YouTube URLs format, and the 5th group is the ID of the video. Notice that a valid YouTube URL along with a video ID doesn't guarantee that the video exist. You can use YouTube.Videos.list() to test if the video for the given ID exists.
  if ( match && match[7].length == 11 ){
    return match[7];
  }
  else if (!( match && match[7].length == 11 )){ // equivalent of else {}
    // multiple attempts to replace
    return trimmedVideoID;
  }

The if..else if block is bad. If you find a match, you return the ID. Done. So extractVideoID() has only one purpose: if the URL has a valid format, return the ID. Otherwise, i.e., if it doesn't find a match, then extractVideoID() should return null.
if (match && match[5])
  return match[5];

In your code, the else if block is opposite of the first if, so it is actually an else block. This also means that, altought you couldn't find any match, you are returning anything anyway, even if url is garbage like Lorem ipsum dolor sit amet, consectetur adipiscing elit..
Now, with addVideoToYouTubePlaylist().
pageToken = res.nextPageToken || "";
The nextPageToken is a string for the next page, so unless YouTube.PlaylistItems.list() fails - and thow an error, stopping everything - it won't evaluate to false. And if it does, you are restarting all over again, possibly heading to the same falsy value thus creating an infinite loop.
for (var d=1,l=data.length; d<l; d++) {
You can test d < data.length directly for improved reading.
  var videoId = extractVideoID(data[d][0]);
  if (!obj[videoId]) {
  YouTube.PlaylistItems.insert({

You may want to test if the video with the given ID exists before of inserting it in the playlist.
  resourceId: {
    kind: "youtube#video",
    videoId: extractVideoID(videoId)
  }

Without chaning extractVideoID() and passing a valid ID to it returns null.
sheet.deleteRow(d+1); // delete row after video is checked
When you delete row number d+1, what was in row (d+1)+1 now is in row d+1. Because of that, you need a counter of deleted rows, say r, and delete the row number d+1-r to compensate the upward "shift".


Update the idx values if vals element match between consecutive rows I want to update the numbers in idx field if there is any matching letter between the vals from two consecutive row.

Input data = '''pos\tidx\tvals
    23\t4\tabc
    25\t7\tatg
    29\t8\tctb
    35\t1\txyz
    37\t2\tmno
    39\t3\tpqr
    41\t6\trtu
    45\t5\tlfg'''

Explantion: Since there is letteramatching between idx 4 and 7 the idx at pos 25 will be updated to 4, but again there istmatching between vals at pos 25 and 29 we update the idx at 29 also to 4 instead of just 7.

#Expected output to a file (tab separated):
pos    idx    vals
23    4    abc
25    4    atg
29    4    ctb
35    1    xyz
37    2    mno
39    3    pqr
41    3    rtu
45    5    lfg

I have written the given workable code (below) so far, and would also like to

write the expected output to a file
optimize the code for the work I am doing.
the answer has to follow my method of reading two consecutive rows (as keys, values) pairs at a time in ordered way. The reason is this question is just a trial of other problem I am trying to solve. Other parts of the code can be optimized in any pythonic way.

Code:

import csv
import itertools
import collections
import io
from itertools import islice

data = '''pos\tidx\tvals
23\t4\tabc
25\t7\tatg
29\t8\tctb
35\t1\txyz
37\t2\tmno
39\t3\tpqr
41\t6\trtu
45\t5\tlfg'''

data_As_Dict = csv.DictReader(io.StringIO(data), delimiter='\t')
grouped = itertools.groupby(data_As_Dict, key=lambda x: x['idx'])

''' Function to read the data as key, val pairs in Ordered way.'''
def accumulate(data):
    acc = collections.OrderedDict()
    for d in data:
        for k, v in d.items():
            acc.setdefault(k, []).append(v)
    return acc

''' Store data as keys,values '''
grouped_data = collections.OrderedDict()
for k, g in grouped:
    grouped_data[k] = accumulate(g)

''' Print the very first k1. After this we only need to print k2 and update the idx '''
header_with_1stK1 = io.StringIO(data).read().split('\n')[0:2]
print('\n'.join(header_with_1stK1))

''' make an empty new_k2 value. This k2 value is updated and carried on base on match between vals from two different rows. '''
k2_new = ''

for n in range(2):
    if n > 0:
        break. # just to run the loop one time and to prevent resetting of k2_new values to ‘’

    ''' Now, read as keys, values pairs for two consecutive keys '''
    for (k1, v1), (k2, v2) in zip(grouped_data.items(), islice(grouped_data.items(), 1, None)):

        v1_vals = ''.join(v1['vals'])
        v2_vals = ''.join(v2['vals'])

        v1_list = list(v1_vals)
        v2_list = list(v2_vals)

        ''' to check if there is any matching element '''
        commons = [x for x in v1_list if x in v2_list]

        v2_pos = ''.join(v2['pos'])

        ''' start updating the idx values '''
        if k2_new == '':
            if len(commons) > 0:
                k2_new = k1
                print('\t'.join([v2_pos, k2_new, v2_vals]))

            else:
                k2_new = ''
                print('\t'.join([v2_pos, k2, v2_vals]))

        elif k2_new != '':
            if len(commons) > 0:
                k2_new = k2_new
                print('\t'.join([v2_pos, k2_new, v2_vals]))

            else:
                k2_new = ''
                print('\t'.join([v2_pos, k2, v2_vals]))

print('\nUpdated the idx values')

I may contribute by first eliminating the itertools and collections module.

import csv
import io

Set the data

    data = '''pos\tidx\tvals
    23\t4\tabc
    25\t7\tatg
    29\t8\tctb
    35\t1\txyz
    37\t2\tmno
    39\t3\tpqr
    41\t6\trtu
    45\t5\tlfg''';
print('INPUT:\n'+data);

Create a "set" of OrderedDictionary object based on the data, and then save it as a list. This is because data_As_dict is not subscriptable, relatively not flexible for work in iteration, and each dictionary in it will be erased after being reused.

data_As_Dict = csv.DictReader(io.StringIO(data), delimiter='\t');
list_Of_Dict = [i for i in data_As_Dict];

Now it is saved as a list, we can access each row. We can check the condition by simply checking v1+v2. For example, if v1='abc' and v2='atg', then v1+v2='abcatg'. We have a pattern, 'a' occurs twice, and the others are unique. So here is your condition : len(set(v1+v2)) != len(v1+v2)

Here is the code for creating the output,

for i in range(0, len(list_Of_Dict)-1):
    v1 = list_Of_Dict[i]['vals'];
    v2 = list_Of_Dict[i+1]['vals'];
    if len(set(v1+v2)) != len(v1+v2):
        list_Of_Dict[i+1]['idx'] = list_Of_Dict[i]['idx'];

output_data = 'pos\tidx\tvals\n';
for i in list_Of_Dict:
    output_data += i['pos']+'\t'+i['idx']+'\t'+i['vals']+'\n';
print('OUTPUT:\n'+output_data);

Full code:

import csv
import io

data = '''pos\tidx\tvals
23\t4\tabc
25\t7\tatg
29\t8\tctb
35\t1\txyz
37\t2\tmno
39\t3\tpqr
41\t6\trtu
45\t5\tlfg''';

print('INPUT:\n'+data);
data_As_Dict = csv.DictReader(io.StringIO(data), delimiter='\t');

list_Of_Dict = [i for i in data_As_Dict];

for i in range(0, len(list_Of_Dict)-1):
    v1 = list_Of_Dict[i]['vals'];
    v2 = list_Of_Dict[i+1]['vals'];
    if len(set(v1+v2)) != len(v1+v2):
        list_Of_Dict[i+1]['idx'] = list_Of_Dict[i]['idx'];

output_data = 'pos\tidx\tvals\n';
for i in list_Of_Dict:
    output_data += i['pos']+'\t'+i['idx']+'\t'+i['vals']+'\n';
print('OUTPUT:\n'+output_data);


A 'flexible' VBA approach to lookups using arrays, scripting dictionary and user input In my previous post Optimise compare and match method using scripting.dictionary in VBA I wanted to address optimising the scripting.dictionary approach I was using and I feel I have achieved that (feel free to correct me though!). I am now implementing user input boxes which have also increased the complexity and now have duplicated code. I'm wondering if there might be a better way of doing this?

This is all one sub procedure and it is working well as far as I have been able to test it, I have just broken it up with comments.

Public Sub arrayMatchv3()

    Dim workingArray As Variant
    Dim workingCell As Range
    Dim workingLastRow As Long
    Set workingCell = Application.InputBox("Select the first cell of your comparison column.", Type:=8)
    workingLastRow = workingCell.End(xlDown).Row
    With workingCell
        workingArray = Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow)
    End With

    Dim targetArray As Variant
    Dim targetCell As Range
    Set targetCell = Application.InputBox("Select the first cell of your target column.", Type:=8)
    With targetCell
        targetArray = Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow)
    End With

    Dim i As Long

With the below ReDim section, I am unsure of any method to write a single column from the worksheet to a single column within a 2d array, so have resorted to looping to populate the second column. Perhaps it isn't really needed to add them both into a single array?

    ReDim Preserve workingArray(1 To UBound(workingArray, 1), 1 To UBound(workingArray, 2) + 1)
    For i = LBound(workingArray, 1) To UBound(workingArray, 1)
        workingArray(i, UBound(workingArray, 2)) = targetArray(i, 1)
    Next i

The below three code sections are duplicates of the three above but using different variables, not sure how to address this?

    Dim comparisonArray As Variant
    Dim comparisonCell As Range
    Dim comparisonLastRow As Long
    Set comparisonCell = Application.InputBox("Select the first cell of the column to match values with.", Type:=8)
    comparisonLastRow = comparisonCell.End(xlDown).Row
    With comparisonCell
        comparisonArray = Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow)
    End With

    Dim returnArray As Variant
    Dim returnCell As Range
    Set returnCell = Application.InputBox("Select the first cell from the column you wish to return values from.", Type:=8)
    With returnCell
        returnArray = Range(.Address, Left(.Address, Len(.Address) - 1) & comparisonLastRow)
    End With

    ReDim Preserve comparisonArray(1 To UBound(comparisonArray, 1), 1 To UBound(comparisonArray, 2) + 1)
    For i = LBound(comparisonArray, 1) To UBound(comparisonArray, 1)
        comparisonArray(i, UBound(comparisonArray, 2)) = returnArray(i, 1)
    Next i

I'm quite happy with the remaining code which includes a lot of feedback from my previous posts, but critique is still welcome, especially if it can address any potential performance issues.

    With New Scripting.Dictionary
        .CompareMode = vbTextCompare

        For i = LBound(comparisonArray, 1) To UBound(comparisonArray, 1)
            If Not .Exists(comparisonArray(i, 1)) Then
                .Add comparisonArray(i, 1), comparisonArray(i, 2)
            End If
        Next i

        Dim arrayToPaste As Variant
        ReDim arrayToPaste(1 To UBound(workingArray, 1), 1 To 1)
        For i = LBound(workingArray, 1) To UBound(workingArray, 1)
            If Len(workingArray(i, 2)) = 0 Then
                If .Exists(workingArray(i, 1)) Then
                    arrayToPaste(i, 1) = .Item(workingArray(i, 1))
                End If
            Else
                arrayToPaste(i, 1) = workingArray(i, 2)
            End If
        Next i
    End With

    With targetCell
        Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow) = arrayToPaste
    End With

End Sub

Signature

Public Sub arrayMatchv3()

I like that the procedure is explicitly Public. Unlike a lot of other languages - including VB.NET, in VBA a member is public unless specified otherwise; being explicit is a good thing. I don't see Option Explicit atop your procedure, but I have it in my IDE and it compiles with it, so kudos for declaring all your variables!

If you're not specifying Option Explicit, you have to know that the slightest typo in a variable's name will make VBA "declare" an implicit Variant variable on-the-spot to receive the value you're assigning to what you intended to be that local variable you just declared: VBA happily compiles with undeclared identifiers, which inevitably will cause bugs that will be hard to find. With Option Explicit specified, the compiler will choke on a typo, finding them for you in a split second.

Member names in VBA should be PascalCase, so that the API you're writing feels like the API you're using when you write VBA code: the entire Excel object model, the VBA standard library, the scripting runtime library - pretty much every single VBA API out there, they all expose modules, classes, procedures, functions, properties, constants, enums, events, etc. - all in PascalCase.

I like that you're using camelCase for your locals though; I find it makes the code feel a bit easier to read, and I do it myself.

The name isn't ideal: "array match v3" is a noun - it's an "array match"... nouns make excellent module/class names: Worksheet, Workbook, Application. Procedures do something, they're verbs. If the procedure matches arrays then MatchArray would be a better name.

I see you're having a version number in the name: this supposes there's an arrayMatchv2 somewhere, and then an arrayMatchv1 or arrayMatch procedure not too far. If not, then just remove the v3 suffix.

Otherwise, you're storing versioning metadata in the code base itself - which is clutter: if arrayMatch usages have been replaced by references to arrayMatchv2, you now have as many places to revisit in the code to make sure it's now calling arrayMatchv3. If v3 fixed a bug in v2, your code base still has the bug even though v3 fixed it.

If you need versioning, use source control. You can create a repository on GitHub, and then use a tool such as Rubberduck to commit, revert, push, pull, create branches and merge them, straight from a docked toolwindow in the IDE. And then you can use GitHub to view any version, or git to fetch any specific version.

I guess that covers the method's signature =)

User input and responsibilities

  it is working well as far as I have been able to test it

To the outside world, the procedure is a complete black box: it takes no input and returns no value. Beyond the name and without peeking at the source code, we have no idea what's happening in there when we read the calling code.

Application.InputBox is called 4 times: there are 4 user inputs happening. By telling the outside world "I need 4 Range parameters to do my thing", the procedure's signature would already be telling us a lot more.

Collecting user input is a concern in its own right. All kinds of wild things can happen when a user gets involved. They can ESC out, or X-out of your dialogs, make typos, enter illegal values - many things can go wrong when a user needs to get involved.

Set workingCell = Application.InputBox("Select the first cell of your comparison column.", Type:=8)
workingLastRow = workingCell.End(xlDown).Row

Accessing workingCell.End here means the program dies with a runtime error if you (/the user) X-out or otherwise cancel the InputBox.

workingCell.End(xlDown) will stop at the first blank cell it finds: if the row can contain blank cells, your workingLastRow will not be the actual last row in the selected column. A more reliable way would be to start at the last row in workingCell's column, and go xlUp from there instead:

With workingCell.Parent
    workingLastRow = .Cells(.Rows.Count, workingCell.Column).End(xlUp).Row
End With

Take the collect user input concern out of the procedure: use parameters, and make it the caller's responsibility to figure out how to get that input - with an InputBox... or perhaps just any arbitrary set of Range objects. Don't trust the input: either raise a descriptive error, or exit early if the input isn't valid (e.g. if any parameter is Nothing).

Except... but we aren't actually working with Range objects - what we really want is a workingArray, a targetArray, a comparisonArray, a returnArray... and we return a Dictionary. 4 parameters and a return value: the Sub becomes a Function, and the inputs and outputs become much clearer already. The fact that you felt the need to split it up in the OP speaks a lot about this concern, too.

At this point things are starting to shape up:

A function to collect user input; validates and returns a Range object
A function that takes a Range and returns an array
A function that takes input arrays and returns a Dictionary
A procedure that takes a Dictionary and writes to a specified Range
A procedure that coordinates all those

By simply splitting things up and tightening variable lifetimes and scopes, and passing and returning objects and arrays, we've made it much easier to write code that can test various valid and invalid inputs and document how things behave... and automated tests are much more reliable than ad-hoc F5-debug testing.

Bugs & Assumptions

Your code is making a bad assumption here, can you spot it?

With workingCell
    workingArray = Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow)
End With

Range is implicitly referring to the active worksheet: the unqualified call will throw an error if the active sheet is a chart! You should be using .Parent.Range to qualify the Range method call with a specific worksheet, so that the active sheet isn't an implicit dependency.

But there's another bug: that -1 is assuming .Row is less than 10, i.e. that the row number in the .Address string is 1 single digit. If the user selects anything between rows 10 and 19, the workingArray will be 100 rows too tall; if the user selects anything between rows 20 and 29, it's 200 rows you don't need, that enter the working array, and become useless iterations here:

For i = LBound(workingArray, 1) To UBound(workingArray, 1)

It's not clear from just reading the code, but I also suspect the code assumes workingCell and targetCell are on the same row, on the same sheet:

With targetCell
    targetArray = Range(.Address, Left(.Address, Len(.Address) - 1) & workingLastRow)
End With

If the two user selections aren't on the same row, then the targetArray can be missing values: it would probably be a good idea to validate that, or make the procedure fail instead of producing wrong results; that could be caught and handled in the function responsible for collecting user input. If you know that an input value is already validated, you can Debug.Assert a precondition, and the code will only ever come to a halt if the assertion fails:

Debug.Assert Not workingCell Is Nothing
Debug.Assert Not targetCell Is Nothing
Debug.Assert workingCell.Parent = targetCell.Parent
Debug.Assert workingCell.Row = targetCell.Row

Seeing that at the top of a procedure that takes workingCell As Range and  targetCell As Range parameters makes it pretty crystal-clear what the assumptions are, and allows the reader/maintainer to say "mismatching working and target rows? impossible!"


Optimize boolean statements I have the following logic:

bool a;
bool b;
int v = 0;

//code here to set the values of a and b...

if(b)
{
    v = a ? 1 : 2;
}
else
{
     v = a ? 0 : 1;
}

I believe that there may be a way to minimize this logic (or rewrite it in a more clever way) that I am not thinking of.

int v = b ? (a ? 1 : 2) : (a ? 0 : 1);

EDIT #1:

As per svick, parsed out conditional operator on separate lines:

int v = b
    ? (a ? 1 : 2)
    : (a ? 0 : 1);

EDIT #2:

Here's one I don't like, but it has no branching whatsoever:

int v = Convert.ToInt32(b) + Convert.ToInt32(!a);


Search a big (13 GB) Outlook PST file I have an Outlook PST file of a pretty big size (13 GB). For some indexing purpose I needed to scan through the full PST file to get all the PR_SEARCH_KEY and then write into a text file. This recursive procedure is used to loop through all folders and subfolders to get the PR_SEARCH_KEY from each email and non-email item. It took nearly 30 minutes to complete the full scan and writing into the text file.

How can I make this faster, such as for completing the process within 5 minutes?
  Is there any efficiency I'm missing here? I'm very new to Outlook and VB.NET.

Dim dict = New Dictionary(Of String, String)

Sub PrepareIndexing()
    Dim oApp As Outlook.Application
    Dim objName As Outlook.NameSpace
    Dim sFolder As Outlook.Folder
    oApp = CreateObject("Outlook.Application")
    objName = oApp.GetNamespace("MAPI")
    sFolder = objName.Folders.Item("pstname")

    dict.Clear()

    Call InitIndexing(sFolder)

    For Each d As KeyValuePair(Of String, String) In dict
        Using outputFile As New StreamWriter("E:\" & Convert.ToString("file.txt"), True)
            outputFile.WriteLine(d.Key & "," & d.Value)
        End Using
    Next
    dict.Clear()
End Sub

Sub InitIndexing(f As Outlook.Folder)
    Dim PropName, skey As String
    Dim oPA As Outlook.PropertyAccessor
    PropName = "http://schemas.microsoft.com/mapi/proptag/0x300B0102" 'Pr_Search_Key

    If f.Folders.Count > 0 Then
        For c = 1 To f.Folders.Count
            Dim Folder As Outlook.Folder = f.Folders.Item(c)
            Dim r As String = Folder.FolderPath
            For Each ml In Folder.Items
                oPA = ml.PropertyAccessor
                skey = oPA.BinaryToString(oPA.GetProperty(PropName))
                If Not dict.ContainsKey(skey) Then
                    dict.Add(skey, r)
                End If
            Next
            Call InitIndexing(Folder)
        Next
    End If
End Sub

I would assume that this
For Each d As KeyValuePair(Of String, String) In dict
    Using outputFile As New StreamWriter("E:\" & Convert.ToString("file.txt"), True)
        outputFile.WriteLine(d.Key & "," & d.Value)
    End Using
Next

is the bottleneck of your code. You are opening, writing to and closeing the file for each KeyValuePair of the dictionary.
Change it like
Using outputFile As New StreamWriter("E:\" & Convert.ToString("file.txt"), True)
    For Each d As KeyValuePair(Of String, String) In dict
            outputFile.WriteLine(d.Key & "," & d.Value)
    Next
End Using

and it should be running much faster.
Btw, why do you use Convert.ToString("file.txt") ?

InitIndexing()
by changing the condition of the f.Folders.Count to check if Count = 0 you could return early from the Sub and therefor save one level of indentation like so
If f.Folders.Count = 0 Then Exit Sub

For c = 1 To f.Folders.Count
    Dim Folder As Outlook.Folder = f.Folders.Item(c)
    Dim r As String = Folder.FolderPath
    For Each ml In Folder.Items
        oPA = ml.PropertyAccessor
        skey = oPA.BinaryToString(oPA.GetProperty(PropName))
        If Not dict.ContainsKey(skey) Then
            dict.Add(skey, r)
        End If
    Next
    Call InitIndexing(Folder)
Next

but you have bigger problems here: Naming ! You shouldn't use abbreviations to name things. If you come back to this code in 3 months you won't know the meaning of oPA, r or objName. You should always use meaningful names to name things.


Evaluator for adding and subtracting algebraic expressions I wrote a little program that adds and subtracts some algebraic expressions.

In my code (2 x 3) represents \$2x^3\$.

The purpose of this post is to get some feedback; moreover, I would like to know how to make it perform better and improve it.

I ran this program on a Mac with SBCL + Emacs.

(defun compute (exp)
  (cond ((term? exp) exp)
    ((expression? exp)
     (make (compute (first-expression exp))
           (compute (second-expression exp))
           (operator exp)))
    (t (error "Unknown Expression Type --COMPUTE" exp))))

(defun term? (exp)
  (or (numberp exp)
      (monomial? exp)))

(defun monomial? (exp)
  (and (consp exp)
       (numberp (car exp))
       (symbolp (car (cdr exp)))
       (numberp (car (cdr (cdr exp))))))

(defun expression? (exp)
  (or (eql (car exp) '+)
      (eql (car exp) '*)
      (eql (car exp) '-)))

(defun first-expression (exp) (car (cdr exp)))

(defun second-expression (exp) (car (cdr (cdr exp))))

(defun operator (exp) (car exp))

(defun make (e1 e2 op)
  (cond ((eql op '+)
     (make-addition e1 e2))
        ((eql op '-)
     (make-subtraction e1 e2))
    (t (make-product e1 e2))))

I wrote the following higher order procedures because version 1 had
a lot patterns.

(defun make-addition (e1 e2)
  (make-exp e1
        e2
        '+
        #'add-monomials))

(defun make-subtraction (e1 e2)
  (make-exp e1
        e2
        '-
        #'subtract-monomials))

(defun make-exp (e1 e2 op term)
  (cond ((eql e1 0) e2)
    ((eql e2 0) e1)
    ((&number? e1 e2) (+ e1 e2))
    ((&monomial? e1 e2)
     (funcall term e1 e2))
    (t (list op e1 e2))))

;; make-product
(defun make-product (e1 e2) (list '* e1 e2))

(defun &number? (e1 e2)
  (and (numberp e1)
       (numberp e1)))

(defun &monomial? (e1 e2)
  (and (monomial? e1)
       (monomial? e2)))

(defun add-monomials (e1 e2)
  (compute-monomials e1
             e2
             #'+
             '+))

(defun subtract-monomials (e1 e2)
  (compute-monomials e1
             e2
             #'-
             '-))

(defun compute-monomials (e1 e2 term op)
  (if (same-variable? e1 e2)
      (make-monomial (funcall term (coeff e1) (coeff e2))
             (variablex e1)
             (exponent e1))
      (list op e1 e2p)))

(defun variablex (exp) (car (cdr exp)))

(defun same-variable? (e1 e2)
  (eql (variablex e1) (variablex e2)))

(defun make-monomial (coeff var exp)
  (list coeff var exp))

(defun coeff (exp) (car exp))

(defun exponent (exp) (car (cdr (cdr exp))))

Make sure your code is indented correctly here. It isn't.

Then document your code at least with some minimum amount of explanations and structure.

Common Lisp has ways to comment code:

;;;; program comment

;;; section comment

(defun bar ()

  "This is a retrievable function documentation
string, which
can be spread over many lines."

  (let ((foo (random 100)))
   ;;  indented code comment
   (+ foo 3)   ; inline code comment
   ))

#|
     Block comments
|#

Then one can retrieve the function documentation with some IDE keystrokes/mechanisms and:

(documentation 'bar 'function)

Some hints:

(defun expression? (exp)
  (or (eql (car exp) '+)
      (eql (car exp) '*)
      (eql (car exp) '-)))

(case exp
  ((+ * -) t)
  (otherwise nil))

(member (car exp) '(+ * -))

(defun first-expression (exp) (car (cdr exp)))

(cadr exp)
(second exp)

(defun second-expression (exp) (car (cdr (cdr exp))))

(caddr exp)
(third exp)

(defun make (e1 e2 op)
  (cond ((eql op '+)
         (make-addition e1 e2))
        ((eql op '-)
         (make-subtraction e1 e2))
        (t (make-product e1 e2))))

(case op
  (+ ...)
  (- ...)
  (otherwise ...))


FizzBuzz: A PowerShell Story After reading way too much into what fizzbuzz was I though PowerShell could easily have its way with this.

Function Get-FizzBuzz{
    param(
        [Parameter(Position=0,ValueFromPipeline=$true)]
        [int]$Number
    )

    process{
       $result = switch($Number){
            {$_ % 3 -eq 0}{"Fizz"}
            {$_ % 5 -eq 0}{"Buzz"}
            default{$_}
       }
       -join $result

    }
}

What we have is a function that accepts pipeline input. For each number that is passed we run it through a switch. Be default it will check against both clauses. So when it hit 15 it will run for Fizz and Buzz. If neither are matched then the number just continues on without being converted. Since FizzBuzz must appear on the same line we capture the $result and use -join to concatenate the elements. This only has an effect on the $result when it has more than element.

Sample Output

PS C:\Windows\system32> Get-FizzBuzz -Number 15
FizzBuzz

PS C:\Windows\system32> 1..15 | Get-FizzBuzz
1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzz

Not sure what could be done beyond this. I suppose you could argue that a filter as supposed to a function would be more terse. I leave that to a reviewer to chew on.

Your code looks good to me. I like how it takes pipelined input. That is pretty elegant.

You don't need the Position=0 here:

[Parameter(Position=0,ValueFromPipeline=$true)]

This is what the help says about that:

  By default, all function parameters are positional. Windows PowerShell        assigns position numbers to parameters in the order in which the        parameters are declared in the function.

The following comments are just my personal preferences. If this were a real code review for production code, I wouldn't insist on any of it. Feel free to ignore!

I think the switch statement there is elegant but a little confusing. In most languages, switch will only ever select a single branch. When I see "switch", I expect it to select one branch only. That's just a habit of thought. Your code took me by surprise. I had to stop and think about how it managed to get both "Fizz" and "Buzz" for the number 15.

I'm almost tempted to say that multiple selection in switch statements is a misfeature of PowerShell because it violates the principle of least surprise. I'm torn on that question however because it is a cool feature.

Personally I would have used two if statements. Even though that would have been clunklier, it would have been clearer and more explicit in my opinion, and therefore better.

Regarding this line:

-join $result

That is correct according to the help. -join can indeed be a unary operator. It looks weird though. I'd never seen it like that before, and it took me by surprise. Personally I would have used it as a binary operator in the usual fashion:

$result -join ""