Social network evolution I am writing a piece of code which models the evolution of a social network. The idea is that each person is assigned to a node and relationships between people (edges on the network) are given a weight of +1 or -1 depending on whether the relationship is friendly or unfriendly.

Using this simple model you can say that a triad of three people is either "balanced" or "unbalanced" depending on whether the product of the edges of the triad is positive or negative.

Finally, what I am trying to do is implement an ising type model.  In other words, random edges are flipped and the new relationship is kept if the new network has more balanced triangles (a lower energy) than the network before the flip, if that is not the case then the new relationship is  only kept with a certain probability.

I have written the following code, however the dataset I have contains ~120k triads, and as a result it will take 4 days to run!

Could anyone offer any tips on how I might optimise the code?

#Importing required librarys

try:
    import matplotlib.pyplot as plt
except:
    raise

import networkx as nx
import csv
import random
import math

def prod(iterable):
    p= 1
    for n in iterable:
        p *= n
    return p

def Sum(iterable):
    p= 0
    for n in iterable:
        p += n[3]
    return p

def CalcTriads(n):
    firstgen=G.neighbors(n)
    Edges=[]
    Triads=[]

    for i in firstgen:
        Edges.append(G.edges(i))

    for i in xrange(len(Edges)):
        for j in range(len(Edges[i])):# For node n go through the list of edges (j) for the neighboring nodes (i)
            if set([Edges[i][j][1]]).issubset(firstgen):# If the second node on the edge is also a neighbor of n (its in firstgen) then keep the edge.
                t=[n,Edges[i][j][0],Edges[i][j][1]]
                t.sort()
                Triads.append(t)# Add found nodes to Triads.

    new_Triads = []# Delete duplicate triads.
    for elem in Triads:
        if elem not in new_Triads:
            new_Triads.append(elem)
    Triads = new_Triads

    for i in xrange(len(Triads)):# Go through list of all Triads finding the weights of their edges using G[node1][node2]. Multiply the three weights and append value to each triad.
            a=G[Triads[i][0]][Triads[i][1]].values()
            b=G[Triads[i][1]][Triads[i][2]].values()
            c=G[Triads[i][2]][Triads[i][0]].values()
            Q=prod(a+b+c)
            Triads[i].append(Q)

    return Triads

###### Import sorted edge data ######
li=[]
with open('Sorted Data.csv', 'rU') as f:
    reader = csv.reader(f)
    for row in reader:
        li.append([float(row[0]),float(row[1]),float(row[2])])
G=nx.Graph()
G.add_weighted_edges_from(li)

for i in xrange(800000):
    e = random.choice(li)   # Choose random edge

    TriNei=[]
    a=CalcTriads(e[0]) # Find triads of first node in the chosen edge
    for i in xrange(0,len(a)):
        if set([e[1]]).issubset(a[i]): # Keep triads which contain the whole edge (i.e. both nodes on the edge)
            TriNei.append(a[i])
    preH=-Sum(TriNei) # Save the "energy" of all the triads of which the edge is a member

    e[2]=-1*e[2]# Flip the weight of the random edge and create a new graph with the flipped edge
    G.clear()
    G.add_weighted_edges_from(li)

    TriNei=[]
    a=CalcTriads(e[0])
    for i in xrange(0,len(a)):
        if set([e[1]]).issubset(a[i]):
            TriNei.append(a[i])
    postH=-Sum(TriNei)# Calculate the post flip "energy".

    if postH<preH:# If the post flip energy is lower then the pre flip energy keep the change
        continue

    elif random.random() < 0.92: # If the post flip energy is higher then only keep the change with some small probability. (0.92 is an approximate placeholder for exp(-DeltaH)/exp(1) at the moment)
        e[2]=-1*e[2]

#Importing required librarys

I can see that. Don't comment on what's obvious from the syntax

try:
    import matplotlib.pyplot as plt
except:
    raise

Catching an exception only to rethrow it? That's a waste

import networkx as nx
import csv
import random
import math

def prod(iterable):
    p= 1
    for n in iterable:
        p *= n
    return p

You may want to look at numpy, it has function like this written in efficient C.

def Sum(iterable):
    p= 0

p? why p?

    for n in iterable:
        p += n[3]
    return p

The python style guide recommends lowercase_with_underscores for global function names. The name also suggests its a generic summing function which its not. No indication is given for what kind of data it expects. A docstring should at least explain that.

def CalcTriads(n):
    firstgen=G.neighbors(n)

Don't use global variables. Instead pass G in as a parameter. Also, I recommend against calling it G.

    Edges=[]
    Triads=[]

Python style guide recommemnds lowercase_with_underscores for local variables names

    for i in firstgen:
        Edges.append(G.edges(i))

I recommend a list comprehnsion

Edges = [G.edges(i) for i in firstgen]

    for i in xrange(len(Edges)):

Don't iterate over the integers, iterate over the edges. Use for edge in Edges:

        for j in range(len(Edges[i])):# For node n go through the list of edges (j) for the neighboring nodes (i)

I recommend putting the comment on a seperate line from the if. Otherwise it tends to wrap off the side of the screen and that's annoying. Also, iterate over the container, not the indexes. Use something like for source, dest in edges:

            if set([Edges[i][j][1]]).issubset(firstgen):# If the second node on the edge is also a neighbor of n (its in firstgen) then keep the edge.

Create a list with a single element, putting that in a set, simply to call issubet is silly. Use Edges[i][j][1] in firstgen.

                t=[n,Edges[i][j][0],Edges[i][j][1]]
                t.sort()
                Triads.append(t)# Add found nodes to Triads.

    new_Triads = []# Delete duplicate triads.
    for elem in Triads:
        if elem not in new_Triads:
            new_Triads.append(elem)
    Triads = new_Triads

That is an inefficient way of doing that. Instead, make Triads a set and put tuples rather then lists in it. That'll be faster and easier to read.

    for i in xrange(len(Triads)):# Go through list of all Triads finding the weights of their edges using G[node1][node2]. Multiply the three weights and append value to each triad.

This code would be way cleaner if you used for triad in Triads:

            a=G[Triads[i][0]][Triads[i][1]].values()
            b=G[Triads[i][1]][Triads[i][2]].values()
            c=G[Triads[i][2]][Triads[i][0]].values()
            Q=prod(a+b+c)
            Triads[i].append(Q)

    return Triads

###### Import sorted edge data ######
li=[]

Bad name, gives no idea what it intends

with open('Sorted Data.csv', 'rU') as f:

I think you are supposed to read csv files in a binary mode.

    reader = csv.reader(f)
    for row in reader:
        li.append([float(row[0]),float(row[1]),float(row[2])])

Use for source, dest, value in row: li.append(float(source), float(dest), float(value))

Do you really want floats? Are the source and destination really floats or are they ints?

G=nx.Graph()
G.add_weighted_edges_from(li)

for i in xrange(800000):

Don't execute loop at the module level. Code written inside  a function will run faster.
        e = random.choice(li)   # Choose random edge

    TriNei=[]
    a=CalcTriads(e[0]) # Find triads of first node in the chosen edge

Don't use variable names like a, a one letter variable name is terrible waste of information capacity.

    for i in xrange(0,len(a)):

Don't use xrange to iterate over containers.

        if set([e[1]]).issubset(a[i]): # Keep triads which contain the whole edge (i.e. both nodes on the edge)
            TriNei.append(a[i])
    preH=-Sum(TriNei) # Save the "energy" of all the triads of which the edge is a member

    e[2]=-1*e[2]# Flip the weight of the random edge and create a new graph with the flipped edge
    G.clear()
    G.add_weighted_edges_from(li)

You should be able to find and change the edge weight directly. Reconstructing the whole graph is going to be kinda slow.

    TriNei=[]
    a=CalcTriads(e[0])
    for i in xrange(0,len(a)):
        if set([e[1]]).issubset(a[i]):
            TriNei.append(a[i])
    postH=-Sum(TriNei)# Calculate the post flip "energy".

Seeing the same piece of code in two place is a big hint. Write a function!

    if postH<preH:# If the post flip energy is lower then the pre flip energy keep the change
        continue

    elif random.random() < 0.92: # If the post flip energy is higher then only keep the change with some small probability. (0.92 is an approximate placeholder for exp(-DeltaH)/exp(1) at the moment)
        e[2]=-1*e[2]

What you should consider doing is writing a function which given a node calculates the change in "energy" that would occour if it was flipped. Right now you calculate before and after, but you should be able to simply calculate the delta without changing it. If you can effeciently calculate the delta, that's all you need.


Emitting event on close of other event (event chaining) I have a software that does the following in sequence:

download text files from s3
reads the files
collect the data from files

The program is asynchronous-synchronous - each phase is asynchronous, but it heavily depends on the output of the previous phase, so I'm using Promises for build the chaining.

The problem of this architecture is that is using to much CPU since I'm spawning child processes in the extract and collect phase. So I decide to change to events:

How I'm doing:

I have 3 eventEmitters: downloadFile, extract, collect:

downloadfile.emit("copy", s3file);
downloadfile.on("error", callback);
downloadfile.on("close", (file) => {
 extract.emit("file", file);
 extract.on("error", callback);
 extract.on("close", (file) => {
   collect.emit("data", file.data);
   collect.on("error", callback);
   collect.on("close", (data) => {
      data is here... i can use without waiting all the other files.
   });
 });
});

But this isn't very readable... I prefer the Promise way:

Promise.resolve(options)
  .then(downloadFiles)
  .then(extract)
  .then(collect);

So I was thinking... is it possible to chain the events? I mean, the output of an event will be the input of other, just like the Promises? If so, how could I achieve that?

You should note that there's a significant difference between events and Promises. Events listen and fire multiple times. However, Promises are a one-time deal. Once it resolves/rejects, the callback won't fire again. You'll have to recreate the whole sequence for another series. With that:

  is it possible to chain the events?

Yes and no. It depends on the event source. In your case, assuming downloadfile, extract and collect are event sources for a single operations, you can simply wrap that event source in a promise that resolves/rejects.

  If so, how could i achieve that?

Assuming emit is a trigger to start the operation, error as the error listener and close as success, you can simply wrap them in promises, where error calls rejectand close calls resolve.

function downloadFiles(downloader, options){
  return new Promise((resolve, reject) => {
    downloader.emit('copy', file);
    downloader.on('error', reject);
    downloader.on('close', resolve);
  });
}

function extract(extractor, file){
  return new Promise((resolve, reject) => {
    extractor.emit('copy', file);
    extractor.on('error', reject);
    extractor.on('close', resolve);
  });
}

function collect(collector, data){
  return new Promise((resolve, reject) => {
    collector.emit('copy', data);
    collector.on('error', reject);
    collector.on('close', resolve);
  });
}

function createSequence(){
  var downloader = createDownloadFileThingy();
  var extractor = createExtractThingy();
  var collector = createCollectThingy();

  return Promise.resolve(options)
    .then(options => downloadFiles(downloader, options))
    .then(file => extract(extractor, file))
    .then(file => collect(collector, file.data));
}


Shorten dict comprehension with repeated operation dc = {line.split('=')[0]: line.split('=')[1] for line in txt}

Below avoids duplication but is even longer:
dc = {k: v for line in txt for k, v in
      zip(*map(lambda x: [x], line.split('=')))}

Any better way? Just without any imports.

Context: with open('config.txt', 'r') as f: txt = f.read().split('\n')
a=1
bc=sea

>>> {'a': '1', 'bc': 'sea'}  # desired output; values should be string

dc = dict(line.split('=') for line in txt)


Similar but slightly different JavaScript functions I have two very similar functions that exist mainly for syntactic sugar.

deactivate = (el, seek) -> el.find(seek).removeClass("active")
activate = (el, seek) -> el.find(seek).addClass("active")

This allows me to write, for example:

activate $("#something"), ".child"

Using the above purely as an example, ignoring any logical errors with the JavaScript itself… how could this be refactored to be more dry?

You can use js' bracket notation for this task

alter = (el, seek, method) -> el.find(seek)[method + "Class"]("active")
#use like this:
alter $("#something"), ".child", "add"

However, your situation doesn't really call for DRYness. Your method names make more sense than alter, or whichever name you may choose.


Prompting user for connection parameters to SQL Server I try to avoid keeping passwords etc. in memory or in plain text anywhere. But I am on a huge time crunch and this will only be used internally this week then probably won't get touched again. I just want to make sure this is secure and if not, what the risk is.

I'm mostly concerned about the password sitting in memory. I'm not sanitizing data but for this particular case it isn't needed.

My specific questions are:

From a security perspective, would this code be acceptable in a SQL Importer?
Besides data sanitation, is there a best practice somewhere that I'm missing in this?

Console.WriteLine("Enter the Server Name. Ex: SQLMASTER"); //Gets the server to connect to, an IP address is also acceptable.
ServerName = Console.ReadLine();
Console.WriteLine("Enter the Database. Ex: Accounts"); //The actual database to use.
DatabaseName = Console.ReadLine();
Console.WriteLine("Use Windows authentication? Y/N"); //you will usually use this.
YesNo = Console.ReadLine();

if (YesNo.ToLower() != "y")
{
    Console.WriteLine("Enter your Username (Domain name may be required). Ex: MyName");
    UserName = Console.ReadLine();
    Console.WriteLine("Enter your Password. Ex: ********");
    Password = ReadPassword();

    //Builds the connection string with the data we collected above. We're going to send this to the SQL Connection. (Username and Password)
    ConnectionString = "Data Source = " + ServerName + "; Initial Catalog = " + DatabaseName + "; User ID = " + UserName + "; Password = " + Password;

    Password = ""; //We do this to clear the password from memory
    UserName = ""; //Clearing username from memory
}
else
{
    //Builds the connection string with the data we collected above. We're going to send this to the SQL Connection. (Windows Authentication)
    ConnectionString = "Data Source = " + ServerName + "; Initial Catalog = " + DatabaseName + "; Integrated Security=SSPI"; //User ID = " + UserName + "; Password = " + Password;
}

//Create a new connection with above connection string
SQLCon = new SqlConnection(ConnectionString);
ConnectionString = ""; //Again, doing this to purge the credentials from memory

try
{
   SQLCon.Open();
   Console.WriteLine("\nConnected!\n");
   Console.WriteLine("Working...\n");
   .... etc
}
catch (Exception ex)
{
   MessageBox.Show("Failed! See console for details");
   Console.WriteLine(ex.Message);
}

It doesn't matter what you do with this code, the ConnectionString can always be accessed in the SqlConnection.

As a test, here's what you should do:

Launch your application outside of Visual Studio. Take a new instance of Visual Studio and go to 'Debug' -> 'Attach to Process...' and then locate the process for your programme.

Once you have done this, run your programme until the SQL point, then hit the 'Pause debugging' button in Visual Studio.

Once you have done this, go to the Diagnostic Tools and take a memory snapshot. While it's paused you can view the memory dump of the snapshot.

You should see a record appear in the 'Memory Usage' tab of the diagnostic tools, click the 'Objects (Diff)' link.

A new tab will open with a lot of fun stuff, order everything by 'Object Type' and find SqlConnection.

Double click your SqlConnection line.

Click the first line (assuming you have one) in the new portion of the window, then click 'Referenced Objects' at the bottom section of the window.

It should show you a bunch of objects, find SqlConnectionString, and drill down into it.

You should see several string objects. Go ahead and hover each one, one will be your password.

Of course, the string object on the SqlConnection itself will also be a connection string and have your password in it.

Bottom line: the client computer is always compromised in your eyes. You can do no more to secure it at this point, with an instance of Visual Studio (which this can be done more effectively with a separate programme that hides on the client computer) I can view your connection string and extract it with little to no effort required. (This whole process takes ~10 seconds once you have used the 'Break All' on the programme.)

My advice to you: don't take unnecessary security precautions to ensure your work is 'safe in memory' -- there's no such thing. All memory can be dumped, and even if it's only in memory for ~0.00001ms, an attacker can still find it. Just worry about building your application, let the memory security be handled by the user. :)

On to the code review:

YesNo is a bad variable name; C# rules are camelCase for local members. It should also be named something related to what it does: windowsCredentials.
You say data sanitization isn't important, but what happens if the user enters a semicolon then another ConnectionString property? You should always validate input.
You should be using the SQLCon, not creating it like you are. (using (var connection = new SqlConnection(ConnectionString))), same with SqlCommand and SqlDataReader if/when/where you use them.

Why?

The SqlConnection type is an IDisposable - that means, it's capable of being automatically closed and deallocated.

In the case of SqlConnection, IIRC there are unmanaged objects that need disposed, cleared and cleaned up before the connection is done. There are resources it uses that are not automatically freed.

If you look at the API of SqlConnection, it has a Close() method which indicates that it will disconnect from the database and free it's resources. This is important because if you forget the Close() call, then your database connection will hang open. This has performance implications for your programme, database and network.

So, we use using (...) construct for it which is a fancy way of writing:

var connection = new SqlConnection();
try
{
    /* Inner code here */
}
finally
{
    ((IDisposable)connection).Dispose();
}

The Dispose() method of SqlConnection then handles closing and flushing all the connection information.

On to your requests:

  From a security perspective, would this code be acceptable in a SQL Importer?

As far as security goes - maybe. I'm not a security expert, but I would be wary of having no input validation here. At the very least, do not allow input with a semicolon (;), as that can totally mess with your connection string.

  Besides data sanitation, is there a best practice somewhere that I'm missing in this?

I mentioned them above, but generally speaking you're trying to solve a problem that we have accepted as a non-issue. Worry about the features of the application, then about the security. Your job as a programmer is not to secure the information in RAM, but to secure it:

On disk / file system
In the database
Over the network

Don't worry so much about the password sitting in memory. Yes, it's generally safer to keep it in memory as shortly as possible, but that goes along with Single-Responsibility Principle, the Principle of Least Astonishment, etc. Your password (and all variables) should be as short-lived as possible. (I.e. Don't do like the old C code used to with 100 variables defined at the top of the file.)

I have turned this answer into a guide on my blog for further explanation.


Iterate over large list of lists and replace its elements I have a large list of lists, where each list contains words/characters as elements, and each list is of different length and may contain same words/characters more than once, as for example:

list_ = [['black', 'window', 'says', 'window'], ['dog', 'great'], ['after', 'end', 'explains', 'income', '.', '?']]

I also have a large dictionary of words/characters like this:

dictionary = {'word1': 'RARE', 'word2': 'RARE', 'word3': 'RARE', '%': 'RARE'} etc.

The length of list_ is around 40 000 (the total number of words in all lists is about 1 mln) and the length of dictionary is around 31 000. For each word in dictionary, I need to check if it's in any of the lists of list_ and if it is, I need to replace it with 'RARE'.

For example, if 'black'== 'word1' and 'after' == word3, the final list_ will be:

list_ = [['RARE', 'window', 'says'], ['dog', 'says'], ['RARE', 'end', 'explains', 'income']]

Because of the large length of list_ and dictionary, the code that I currently have below takes almost 40min to run. I'm new to Python and I would appreciate if someone could suggest whether there's a way to do this task much faster/more efficiently.

def replace_all(word_list, dictionary):
   for i, j in dictionary.iteritems():
      for lst in word_list:
         if i in set(lst):
            lst[lst.index(i)] = j

replace_all(list_, dictionary)

You're looping in incorrect order, take the advantage of the fact that dictionary provides O(1) lookups. So, you should loop over the list and replace its items with the value present in the dictionary. This way you will loop over the list only once:

We can either use dict.get here and avoid an if condition:

def replace_matched_items(word_list, dictionary):
   for lst in word_list:
      for ind, item in enumerate(lst):
          lst[ind] = dictionary.get(item, item)

Or check for the key first and modify only matched item(LBYL):

def replace_matched_items(word_list, dictionary):
   for lst in word_list:
      for ind, item in enumerate(lst):
          if item in dictionary:
              lst[ind] = dictionary[item]

or if you prefer EAFP then try to catch the KeyError:

def replace_matched_items(word_list, dictionary):
   for lst in word_list:
      for ind, item in enumerate(lst):
          try:
              lst[ind] = dictionary[item]
          except KeyError:
              pass

I generally find functions that modify a mutable item in-place and don't return anything confusing, it's usually a better idea to return the list object from the function and re-assign to the variable from where it was called.

def replace_matched_items(word_list, dictionary):
   for lst in word_list:
      for ind, item in enumerate(lst):
          lst[ind] = dictionary.get(item, item)

list_ = replace_matched_items(list_, dictionary)

But the above approach is still flawed(at least when our object has multiple references), with this even though we re-assigned only a single variable all the other references to the list object will still be affected, which can be very confusing to users. For example:

def func(seq):
    for lst in seq:
        for i, x in enumerate(lst):
            if x == 1:
                lst[i] = 100
    return seq

foo = [[1, 1, 2], [1, 2, 3]]
dct = {'key': foo}

foo = func(foo)
print foo
print dct

Outputs:

[[100, 100, 2], [100, 2, 3]]
{'key': [[100, 100, 2], [100, 2, 3]]}

To fix such such cases we can either pass a deepcopy of the list to the function(or simple shallow copy for list containing only immutable items) or we can create a new list inside of the function. But as we are dealing with 1 million items creating another list will surely result in extra memory consumption. Here's an example of how to do this by creating a new list using dict.get:

def replace_matched_items(word_list, dictionary):
    new_list = [[dictionary.get(item, item) for item in lst] for lst in word_list]
    return new_list

list_ = replace_matched_items(list_, dictionary)

Few other questions on the same topic:

Is making in-place operations return the object a bad idea?
Why does django ORM's save method not return the saved object?


"P1 attacks P2" and "P2 attacks P1" This is just my simple structure. I want to know if there is a better way than this because I have so many lines of code already.

    <?php
$p1speed = 5;
$p2speed = 3;

//way for next turn: 1 for turn
$p1 = 0;
$p2 = 0;

if($p1speed>$p2speed) {

    $p1 = 1;
    $p2 = 0;

}
else if($p2speed>$p1speed) {

    $p1 = 0;
    $p2 = 1;

}

$count=0;

//begin
while($count<5){

    if($p1==1) {

        echo 'P1 turns
';

        //adjust
        $p1 = 0;
        $p2 = 1;

    }
    else if($p2==1) {

        echo 'P2 turns
';

        //adjust
        $p1 = 1;
        $p2 = 0;

    }

    $count++;

}
?>

This is a refactoring of the original code. It picks the starting player and then uses a simple toggle ($currentPlayer = ($currentPlayer == 1) ? 2 : 1;) to determine which player goes next for 5 rounds.

$currentPlayer = 1;

if ($player2speed > $player1speed) {

    $currentPlayer = 2;

}

$count = 0;

while ($count < 5) {

    echo $currentPlayer . " turns<br/>";

    $currentPlayer = ($currentPlayer == 1) ? 2 : 1;

    $count ++;
}

Note, the current code fails if both player's speeds are the same.


Linq-like functions in JavaScript Today was my first day working with JavaScript and I tried to implement some Linq-like (C#) functions as extension methods.

I'm pretty new to JavaScript and wanted to ask you for checking my functions and tell me, whether they're good and really usable in productive applications. Or maybe, what I could do better. I don't know whether there is already a library for this; it's primarily for learning purposes.

Array.prototype.where = function * (predicate){
    for(var i=0; i < this.length; i++){
        if (predicate(this[i])) yield this[i];
    }
}

Array.prototype.all = function (predicate){
    for(var i=0; i < this.length; i++){
        if (!predicate(this[i])) return false;
    }
    return true;
}

Array.prototype.any = function (predicate){
    if (predicate == null) return this.length >0;
    for(var i=0; i < this.length; i++){
        if (predicate(this[i])) return true;
    }
    return false;
}

Array.prototype.select = function * (predicate){
    for(var i=0; i < this.length; i++){
        yield predicate(this[i], i);
    }
}

Array.prototype.groupby = function (predicate){
    var result = {};
    for(var i=0; i < this.length; i++){
        var key = predicate(this[i]);
        if (result[key] == null)
            result[key] = Array();
        result[key].push(this[i]);
    }
    return result;
}

Array.prototype.foreach = function (predicate){
    for(var i=0; i < this.length; i++){
        predicate(this[i]);
    }
}

Array.prototype.first_or_default = function (predicate){
    if (predicate == null){
        return this.length > 0 ? this[0] : null;
    }
    for(var i=0; i < this.length; i++){
        if (predicate(this[i]))
            return this[i];
    }
    return null;
}

Array.prototype.last_or_default = function (predicate){
    return this.reverse().first_or_default();
}

Array.prototype.count = function (predicate){
    var count = this.length;
    if (predicate == null) return count;
    for(var i=0; i < this.length; i++){
        if (!predicate(this[i])) count-=1;
    }
    return count;
}

Array.prototype.distinct = function * (predicate){
    var already_yielded = []
    var has_predicate = predicate != null;

    for(var i=0; i < this.length; i++){
        var current_element = this[i];

        if (!has_predicate){
            if (already_yielded.indexOf(current_element) >= 0) continue;
            already_yielded.push(current_element);
        }else{
            if (already_yielded.indexOf(predicate(current_element)) >= 0) continue;
            already_yielded.push(predicate(current_element));
        }
        yield current_element;
    }
}

This might not really be an answer, but I suggest you to check out linq.js.
So yes, there is already an implementation of this and you can look at the code to compare and it will help the "learning purposes" you're aiming for.

Example implementation of any :

 // Overload:function()
    // Overload:function(predicate)
    Enumerable.prototype.any = function (predicate) {
        predicate = Utils.createLambda(predicate);

        var enumerator = this.getEnumerator();
        try {
            if (arguments.length == 0) return enumerator.moveNext(); // case:function()

            while (enumerator.moveNext()) // case:function(predicate)
            {
                if (predicate(enumerator.current())) return true;
            }
            return false;
        }
        finally {
            Utils.dispose(enumerator);
        }
    };

I personally don't have the required experience in JavaScript to give you my opinion on your implementation. Good luck !


Binary Tree Sort Algorithm (Python) A Binary Tree Sort is an algorithm that builds a binary search tree from the elements to be sorted, and then traverses the tree (in-order) so that the elements come out in sorted order.

Average Case Time Complexity : O(N log N) adding one element to a Binary Search Tree on average takes O(log N) time (height of a tree). Therefore, adding n elements will take O(N log N) time.

Worst Case Time Complexity : O(N^2). The worst case can be improved by using a self-balancing binary search tree, which will take O(N log N) time to sort the array in worst case.

Just for practicing, I've implemented the Binary Tree Sort algorithm, and if you'd like to review the code and provide any change/improvement recommendations, please do so, and I appreciate that.

Code

"""
This module creates a Binary Sort Tree ascendingly using In Order Tree Traverse;
Stable Sort;
Worst Case Time Complexity (For if the tree would be a chain Tree): O(N^2)
Average Case Time Complexity: O(N^Log N)
Memory Complexity: Not in place O(N)
"""

import random
from typing import List, TypeVar

T = TypeVar('T')

# Not sure how to design and handle the exceptions here yet
class ExceptionHandling(Exception):
    pass

class Node(object):
    def __init__(self, node_value: int) -> None:
        """
        Initializes a node with three attributes;
        `node_value` (Node Value): Must be an integer/float;
        `right_child` (Right Child): Initializes to `None`; Its value must be larger than the Root Node;
        `left_child` (Left Child): Initializes to `None`; Its value must be smaller than the Root Node;
        """
        self.node_value = node_value
        self.right_child = None
        self.left_child = None

class BinarySearchTree(object):
    def __init__(self) -> None:
        """
        Initializes the Root Node of the Binary Tree to None;
        """
        self.root = None

    def is_empty(self) -> bool:
        """
        Returns True if the tree doesn't have a node;
        """
        return self.root == None

    def insert(self, new_node: int) -> None:
        """
        Inserts an integer-value Node in the Tree;
        """
        self.root = self._insert(self.root, new_node)

    def _insert(self, parent: int, new_node: int) -> int:
        """
        Inserts an integer value in the left or right Node;
        Returns the parent Node;
        """
        # If tree is empty
        if parent is None:
            parent = Node(new_node)
        # If the new Node value is smaller than its parent Node value
        elif new_node < parent.node_value:
            parent.left_child = self._insert(parent.left_child, new_node)
        # If the new Node value is bigger than its parent Node value
        else:
            parent.right_child = self._insert(parent.right_child, new_node)

        return parent

    def inorder(self) -> None:
        """
        Calls the _inorder traversing method;
        """
        self._inorder(self.root)

    def _inorder(self, parent: int) -> None:
        """
        Traverses the tree inorder (Left Node, Root Node, Right Node)
        """
        if parent is None:
            return
        self._inorder(parent.left_child)
        print(f'{parent.node_value}')
        self._inorder(parent.right_child)

if __name__ == '__main__':

    tree = BinarySearchTree()

    for i in range(random.randint(20, 30)):
        tree.insert(random.uniform(-10, 10))

    tree.inorder()

Sources

Tree Sort - Wiki
Tree Sort - Geeks for Geeks

Type Hints

From these lines:

from typing import List, TypeVar

T = TypeVar('T')

it looks like you intend to add type-hints for a type T to you code.  But nowhere are you using T as a type hint.  You probably wanted to actually use T, such as like:

def __init__(self, node_value: T) -> None

Either that, or delete the typing code.

Exception Handling

You have:

class ExceptionHandling(Exception):
    pass

but nowhere are you actually executing raise ExceptionHandling("Your error message").  Moreover, nowhere do I actually see a need to raise an exception; you aren't doing anything that could fail.  Until you have a need for raising your own custom exception, you could remove this code.

class Node(object):

Since you are using f-strings, it is clear, you are using Python 3.  In Python 3+, you don't need to inherit from object; it is automatically implied.

Names & Types

def insert(self, new_node: int) -> None:

Is new_node a Node or an int?  The variable name suggests it would be a Node, but it is typed as an int.  When you use it, you are passing in an int.  Maybe new_value would be clearer?

def _insert(self, parent: int, new_node: int) -> int:

Now we're definitely confused.  Is parent an int?  Does this function return an int?  Both seem to be a definite "no".  Those types should be Node.  And again, new_node should be renamed, because it isn't a Node.

Method naming (and docstrings)

What does inorder do?  I'd expect it to return the contents of the tree "in order".  But the type-hint says it returns None, so that is not correct.  Let's consult help:

>>> help(BinarySearchTree.inorder)
Help on function inorder in module __main__:

inorder(self) -> None
    Calls the _inorder traversing method;

>>>

Well, that's entirely unhelpful.  It calls a private _inorder traversing method.  Ok, but what does it do???

Name functions based on what they do.  Your inorder function prints the tree in order, so name it with print in its name, such as print_inorder.

Also, write docstrings to tell a caller what the function does.  Use a high-level description.  Include any requirements, preconditions and/or postconditions.  Do not include implementation details.  The caller cares only about what it does and how to use the function properly; they only care the function works, not how it works.

def print_inorder(self) -> None:
    """
    Print the tree in ascending order, one element per line, to sys.stdout
    """
    self._inorder(self.root)

Finally, your class is named BinarySearchTree, yet it provides no methods for searching.  Hmm.

Pointless f-string

The statement:

print(f'{parent.node_value}')

is a complicated way of writing:

print(parent.node_value)

Algorithm

Your insert / _insert functions are overly complicated.  There is no need to use recursion.  Moreover, returning the parent from _insert is mostly useless, because the caller is passing parent to the function; it only matters when the caller passes None in as the parent.

A non-recursive approach:

def insert(self, new_value: int) -> None:

    new_node = Node(new_value)

    if self.root:
        parent = self.root
        while True:
            if new_value < parent.node_value:
                if parent.left_child:
                    parent = parent.left_child
                else:
                    parent.left_child = new_node
                    break
            else:
                if parent.right_child:
                    parent = parent.right_child
                else:
                    parent.right_child = new_node
                    break
    else:
        self.root = new_node

Possible Improvements

Encapsulation

Node is an internal detail of the BinarySearchTree.  You could make it an internal class:

class BinarySearchTree:

    class Node:
        def __init__(self, node_value:T ) -> None:
            self.node_value = node_value
            self.right_child = None
            self.left_child = None

    def __init__(self) -> None
        self.root = None

    ...

       parent = BinarySearchTree.Node(new_node)

    ...

Iterator

Instead of the method inorder printing the contents of the tree, perhaps it could return an iterator which would traverse the tree returning elements one at a time.  The caller could then print them, or perform whatever operation they desired.

for value in tree.inorder():
    print(value)

Instead of naming this iterator creating function inorder, it could be named __iter__, in which case, the caller would just need to write:

for value in tree:
    print(value)

You'd create this iterator in one of two ways.  The "hard way", returning a new iterator object, with a __next__ method.  Or the "easy way" using a generator function, using yield and yield from statements.


MongoDB database connector on NodeJS What my code does

I am building an Express API server with mongoDB as my database.

I have a list of players which must be added to 2 mongoDB collections (teamList and countryList).

The data schema is similar. My code is as follows.

Data Model model.js

var mongoose = require('mongoose');
var Schema = mongoose.Schema;

var teamSchema = new Schema({
  teamid: {
    type: String,
    required: true
  },
  created: {
    type: Date
  },
  lastUpdated: {
    type: Date,
    default: Date.now,
    required: true
  },
  playerList: [{
    name: {
      type: String,
      required: true,
      unique: true
    },
    dateAdded: {
      type: Date,
      default: Date.now
    }
  }]
});

module.exports = mongoose.model('teamModel', teamSchema);

var countrySchema = new Schema({
  countryid: {
    type: String,
    required: true
  },
  created: {
    type: Date
  },
  lastUpdated: {
    type: Date,
    default: Date.now,
    required: true
  },
  playerList: [{
    name: {
      type: String,
      required: true,
      unique:true
    },
    dateAdded: {
      type: Date,
      required: true
    }
  }]
});
module.exports = mongoose.model('countryModel', countrySchema);

Router router.js

module.exports = function(app) {
  const teamControllerv1 = require('../v1/controller/teamController');
  const countryControllerv1 = require('../v1/controller/countryController');
  app.route('/v1/team').post(teamControllerv1.addPlayerToTeam);
  app.route('/v1/country').post(countryControllerv1.addPlayerToCountry);
};

Controllers
countryController.js

var mongoose = require('mongoose');
const countryModel = mongoose.model('countryModel');

exports.addPlayerToCountry = function (req, res) {
    countryModel.update({
        countryid: req.body.countryID
      }, {
        $addToSet: {
          countryList: {
            $each: req.body.playerList
          }
        }
      }, {
        upsert: true
      },
      function (err, data) {
        if (!err && data) {
          util.successResponder(res, successText);
        } else {
          util.serverErrorResponder(res, errorOccured);
        }
      });
}

teamController.js

var mongoose = require('mongoose');
const TeamModel = mongoose.model('teamModel');

exports.addPlayerToTeam = function (req, res) {
    teamModel.update({
        teamid: req.body.teamID
      }, {
        $addToSet: {
          teamList: {
            $each: req.body.playerList
          }
        }
      }, {
        upsert: true
      },
      function (err, data) {
        if (!err && data) {
          util.successResponder(res, successText);
        } else {
          util.serverErrorResponder(res, errorOccured);
        }
      });
}

Problem I am trying to solve

As you can see this code clearly violates DRY. Except the mongoose database connector, everything else in the 2 files is exactly the same. How can I write this in a cleaner way such that the db connector is abstracted away?

So I kind of figured this out. The db connector can be passed around like just another variable. Silly I didn't realise this before. I created a new file to abstract the model.

dbOperations.js

exports.addToDB = async function (model, dataObj, listName, res) {
  model.update({
      userid: dataObj.userID
    }, {
      $addToSet: {
        listName: {
          $each: dataObj.playerID
        }
      }
    }, {
      upsert: true
    },
    function (err, data) {
      if (!err && data) {
        util.successResponder(res, successText);
      } else {
        util.serverErrorResponder(res, errorOccured);
      }
    });
};

teamController.js

var mongoose = require('mongoose');
const teamModel = mongoose.model('teamModel');
const dbOperations = require('../../helper/dbOperations');

exports.addPlayerToTeam = function (req, res) {
  let dataObj = {teamID: req.body.teamID, playerID: req.body.playerList};
  dbOperations.addToDB(teamModel,dataObj,'teamList',res);
}

And similarly for the other file as well. Any other files which follow a similar schema can use it. There is probably a better way to do this so that this can be generalised further to accommodate other schema types.


Matrix struct with random values allocation and deallocation I've written this code in 2013 or 2014, I think. I've rediscovered it today in one of my folders, made some changes and now I'm left (and my 2013/2014 me as well) wondering how good it looks for other C programmers.

It's a simple dummy program that reads N and M from stdin, then creates a matrix with those values as rows and columns with dynamic memory allocation, prints them, and then frees the memory. A rollback is made if malloc fails.

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

#define ERROR -1
#define SUCCESS 0

char welcome[] = "\tThis program \
    gets the size of a matrix from stdin, messes around \
    with it and then frees it.\n";

struct Matrix
{
    int N;
    int M;
    int **values;
};

/**
* allocates memory for the matrix
*/
int allocateMatrix(struct Matrix *matrix, int n, int m)
{
    printf("Allocating memory for matrix... (%d x %d)\n", n, m);
    int i, j;

    /* allocate rows */
    matrix->values = malloc(n * sizeof(int*));

    if (detectError(matrix->values)) {
        return ERROR;
    }

    /* allocate columns */
    for(i = 0; i < matrix->N; i++)
    {
        matrix->values[i] = malloc(m * sizeof(int));
        /* handles errors */
        if (detectError(matrix->values[i])) {
              /* frees already allocated memory */
            j = i-1;
            for(; j >= 0; j--)
            {
                free(matrix->values[i]);
            }
            free(matrix->values);
            return ERROR;
        }
    }
    printf("%lu bytes of memory allocated! \n", (n * m * sizeof(int)));
}

/**
* gets the properties of the matrix
*/
int getMatrixProps(struct Matrix *matrix)
{
    int n, m;
    printf("\nWhat will be the size of our matrix? (N x M)\n");
    scanf("%d %d", &n, &m);
    matrix->N = n;
    matrix->M = m;
    return allocateMatrix(&(*matrix), n, m);
}

/**
* frees the memory requested
*/
void freeMatrixMem(struct Matrix *myMatrix)
{
    int i;
    for(i = 0; i < myMatrix->N; i++)
    {
        free(myMatrix->values[i]);
    }
    free(myMatrix->values);
}

/**
* populates and prints the matrix
*/
void dummyHandleMatrix(struct Matrix *myMatrix)
{
    srand(time(NULL)); /* seeds random function */
    int n, m;
    printf("Populating matrix with random data... ");
    for(n = 0; n < myMatrix->N; n++)
    {
        for(m = 0; m < myMatrix->M; m++)
        {
            int value = rand() % 50;
            printf("%d ", value);
            myMatrix->values[n][m] = value;
        }
    }
    printf("\n");
    printf("Matrix populated. Values are:\n[");
    for(n = 0; n < myMatrix->N; n++)
    {
        for(m = 0; m < myMatrix->M; m++)
        {
            printf("%d, ", myMatrix->values[n][m]);
        }
        printf("\n");
    }
    printf("]\n");
}

int detectError(void *ptr) {
     if (ptr == NULL)
    {
            printf("operation failed\n");
            printf("%s\n", strerror(errno));
            return ERROR;
    }
    return SUCCESS;
}

int main(int argc, char *argv[])
{
    printf("%s", welcome);
    struct Matrix myMatrix;

    /* get size of the matrix and allocate memory for it */
    if (getMatrixProps(&myMatrix) == ERROR) {
        return ERROR;
    }

    /* populates it with random data and print to the screen */
    dummyHandleMatrix(&myMatrix);

    /* frees allocated memory */
    printf("Freeing matrix memory!\n");
    freeMatrixMem(&myMatrix);
    printf("Memory freed.\n");
}

Improve your error reporting:

Your detectError function is not very useful. The name is also not good. It doesn't "detect errors", but rather it detects an out-of-memory condition if called after malloc.

int detectError(void *ptr) {
     if (ptr == NULL)
    {
            printf("operation failed\n");
            printf("%s\n", strerror(errno));
            return ERROR;
    }
    return SUCCESS;
}

If malloc fails, there is only one possible reason, it can't allocate any more memory. On a POSIX implementation if that happens malloc will set errno to ENOMEM. So by calling strerror you are just inventing a very complicated way of printing "out-of-memory" to stdout. Your code would be simpler and more idiomatic if instead of calling detectError you would just check the return of malloc for NULL:

matrix->values = malloc(n * sizeof(int*));
if (matrix->values == NULL) {
    fprintf(stderr, "Out-of-memory");
    return ERROR;
}

Also note that on some implementations malloc might not set errno at all or set it to EAGAIN, which would print an unclear or wrong message if using errno with strerror.

Still looking at the error reporting, it is more conventional to print error messages to stderr, so that users can filter normal program output from errors more easily.

Use booleans whenever possible instead of error codes:

#define ERROR -1
#define SUCCESS 0

I would not define those constants and instead use bool. If there are only two possible outcomes, success or failure, then just returning true or false is simpler and avoids confusion. It is very common for libraries to mix the use of integers as booleans and as error codes, which complicates things and leads to error if you misinterpret a return value. Keep it simple.

Awkward pointer manipulation:

From getMatrixProps, which already takes matrix by pointer:

return allocateMatrix(&(*matrix), n, m);

That dereference followed by taking the address is nonsensical. Pass it directly as is.

Allocate a flat array:

Lastly, you could simplify a lot if you'd allocate the whole matrix as a sequential memory block. Instead of having to call a malloc per column, you can allocate a flat array of columns.

Since we are dealing with a 2D matrix, in the following example I'll call
the columns width and rows height, using x and y as the respective indexes.

typedef struct {
    int width;
    int height;
    int * values;
} Matrix;

bool allocMatrix(Matrix * mat, int width, int height)
{
    // The total number of elements in the matrix is
    // the number of columns times the number of rows
    //
    mat->values = malloc(width * height * sizeof(int));
    if (mat->values == NULL) {
        fprintf(stderr, "Out-of-memory");
        return false;
    }

    mat->width  = width;
    mat->height = height;
    return true;
}

//
// To access a given 2D index, now that the matrix is flat, you'll need to
// calculate the proper offsets with the formula: 'x + y * width'
//
// To simplify element access, I suggest defining a couple helper functions:
//

int getMatrixElement(const Matrix * mat, int x, int y)
{
    return mat->values[x + (y * mat->width)];
}

void setMatrixElement(Matrix * mat, int x, int y, int value)
{
    mat->values[x + (y * mat->width)] = value;
}


Calculate someone's pay, including overtime In this problem, you need to calculate someone's pay by multiplying their salary (double)by the hours (int) worked. If they worked for more than 8 hours, you pay them 1.5 times their salary for however many hours more they worked.
Here is my solution.

public static double pay(double a, int b) {
if (b > 8) {
    return (a * 8) + ((b - 8)*(a * 1.5));
}
else {
    return (a * b);
}}

Wondering if there is any way to do it without if else statements since the course hasn't taught those yet.

You should use better variable names, for a start.... consider:

public static double pay(double hourlyWage, int hours) {
    ...
}

Then, your code is not that bad (with some indentation fixes too):

public static double pay(double hourlyWage, int hours) {
    if (hours > 8) {
        return (hourlyWage * 8) + ((hours - 8)*(hourlyWage * 1.5));
    }
    else {
        return (hourlyWage * hours);
    }
}

There is no good way to handle the overtime rate other than with a conditional somewhere. Ternary conditions may help a bit.....

public static double pay(double hourlyWage, int hours) {
    int regular = hours > 8 ? 8 : hours;
    int overtime = hours - regular;
    return hourlyWage * (regular + (1.5 * overtime));
}


Handling response from another microservice I have this piece of code placed in ESI class. There is switch statement nested in a try-catch block and it feels simply wrong for me (I have a suspicion that I'm close to exception driven development). I'm looking forward to read your suggestions on how could I potentially improve this code.

@PostConstruct
public void initialize() {
    this.webTarget = this.client.target("http://demo1173642.mockable.io/");
}

public Message getMockMessage() {
    Response response = null;

    try {
        response = this.webTarget.request().get();

        switch (response.getStatusInfo().getFamily()) {
            case SUCCESSFUL:
                return response.readEntity(Message.class);

            case SERVER_ERROR:
                throw new WebApplicationException(ErrorCode.EXTERNAL_SERVICE_BAD_GATEWAY.getMessage(),
                        Response.Status.BAD_GATEWAY);

            default:
                throw new InternalServerErrorException(ErrorCode.EXTERNAL_SERVICE_EXCEPTION.getMessage());
        }

    } catch (Exception ex) {
        logger.log(Level.WARNING, ErrorCode.UNEXPECTED_EXECPTION.getMessage(), ex);
        if (response != null) {
            response.close();
        }
        throw new InternalServerErrorException();
    }
}

In fact, your error handling is quite messed up: in the cases for server error and default you throw an exception, which you immediately replace with a different exception in your own catch block. I don't think that's what you meant to do. Furthermore, you only close() the response object in the exception case, but not in the success branch...

Nevertheless, the throwing of a WebApplicationException leads me to believing that we are in a RESTful application here. Thus, the best way would be to declare an excepton mapper to handle the underlying ProcessingException (which is the only one your catch-all-block should encounter) and do not do the handling internally. For exception mappers, here's a tutorial to start with: https://dennis-xlc.gitbooks.io/restful-java-with-jax-rs-2-0-en/cn/part1/chapter7/exception_handling.html  (but you will find a lot more resources using your favourite search engine.)

Having this out of the way, the only thing for your method to do is using a try-with-resources block for the response:

public Message getMockMessage() {
    try(Response response = this.webTarget.request().get()) {
        switch (response.getStatusInfo().getFamily()) {
            case SUCCESSFUL:
                return response.readEntity(Message.class);
            case SERVER_ERROR:
                throw new WebApplicationException(ErrorCode.EXTERNAL_SERVICE_BAD_GATEWAY.getMessage(),
                        Response.Status.BAD_GATEWAY);
            default:
                throw new InternalServerErrorException(ErrorCode.EXTERNAL_SERVICE_EXCEPTION.getMessage());
    }
}


Function launching default editor I have written a function that launches the default editor set in git config, right now I have managed to get it working for Sublime, nano and Vim.

def launchEditor(editor):
    """ this function launches the default editor
        for user to compose message to be sent
        along with git diff.

        Args:
            editor(str): name or path of editor
        Returns:
            msg(str): html formatted message
    """

    filePath = os.path.join(os.getcwd(), "compose.txt")
    wfh = open(filePath, 'w')
    wfh.close()

    if os.path.exists(filePath):
        # using sublime
        if re.search(r'ubl', editor):
            diff = subprocess.Popen(['cat',filePath], stdout=subprocess.PIPE)
            pr = subprocess.Popen(
                editor,
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                shell=True,
                stdin=diff.stdout
                )
            pr.wait()
            if pr.returncode == 0:
                msg = pr.stdout.read()
        else:
            # using vim or nano
            pr = subprocess.Popen([editor, filePath], stdin=open('/dev/tty', 'r'))
            pr.wait()
            if pr.returncode == 0:
                with open(filePath, 'r') as fh:
                    msg = fh.readlines()
    os.remove(filePath)
    return "".join(msg).replace("\n","<br>")

Any suggestion on improvement and adding support to other text editors is welcome!!

Revision update:

 Traceback (most recent call last):
  File "/Users/san/Development/executables//git-ipush", line 268, in <module>
    sys.exit(main())
  File "/Users/san/Development/executables//git-ipush", line 46, in main
    preCheck(args)
  File "/Users/sanjeevkumar/Development/executables//git-ipush", line 156, in preCheck
    message = launchEditor(editor)
  File "/Users/san/Development/executables//git-ipush", line 77, in launchEditor
    if subprocess.call([editor, f.name]) != 0:
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 524, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1308, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

1. Comments on your code

The function is poorly specified in that it combines two tasks: (i) it gets input from the user via an editor; (ii) it replaces newlines in the input with <br>. But what I just want the input and don't want any HTML conversion (especially not a half-baked conversion like this one)?

It would be better to decompose this function into two pieces. In what follows I'm just going to discuss piece (i), launching the editor and capturing the input.
There are missing imports (os and subprocess).
The function always uses the file compose.txt in the current directory. This means that if I already had a file with that name, it would be erased and then deleted, which would be really annoying.

It would be better to create a temporary file for this purpose, using Python's tempfile.NamedTemporaryFile.
This code:

diff = subprocess.Popen(['cat',filePath], stdout=subprocess.PIPE)
pr = subprocess.Popen(..., stdin=diff.stdout)

is a classic "useless use of cat". If you read the documentation for subprocess.Popen, you'll see:

  stdin, stdout and stderr specify the executed program’s standard input, standard output and standard error file handles, respectively. Valid values are PIPE, DEVNULL, an existing file descriptor (a positive integer), an existing file object, and None.

(my emphasis) so you can write:

pr = subprocess.Popen(..., stdin=open(filePath))

and save a process. (But actually this is unnecessary, as explained below.)
This code:

if re.search(r'ubl', editor):

doesn't seem like a very robust way to ensure that editor is Sublime Text. I mean, for all you know I could have just run:

$ ln /usr/bin/vi ubl

It's best to treat all the editors the same. After all, git commit doesn't have any special case for Sublime Text, so neither should you.

In particular, you don't need to go through all that subprocess.Popen shenanigans to open a file in Sublime. I find that

subprocess.call(['subl', filename])

works fine.
If all you are going to do with a subprocess is wait for it to exit:

pr = subprocess.Popen([editor, filePath], stdin=open('/dev/tty', 'r'))
pr.wait()
if pr.returncode == 0:

then use subprocess.call instead of subprocess.Popen:

if subprocess.call([editor, filePath]) == 0:

Specifying stdin=open('/dev/tty', 'r') is unnecessary. Let the editor decide how it wants to get input from the user.
If the editor returned with a non-zero code, your function doesn't report an error, it just continues running (but with nothing assigned to msg) until it reaches "".join(msg) which fails with a mysterious TypeError. Better to raise an exception if the editor returned an error code.

(I submitted a bug report for the mysterious TypeError: see Python issue 20507.)
Your function splits the input into lines by calling msg = fh.readlines(), and then it joins these lines back together again with "".join(msg). This is pointless: if you just want the contents of the file as a string, write msg = fh.read() instead.

2. Revised code

import subprocess
import tempfile

def input_via_editor(editor):
    """Launch editor on an empty temporary file, wait for it to exit, and
    if it exited successfully, return the contents of the file.

    """
    with tempfile.NamedTemporaryFile() as f:
        f.close()
        try:
            subprocess.check_call([editor, f.name])
        except subprocess.CalledProcessError as e:
            raise IOError("{} exited with code {}.".format(editor, e.returncode))
        with open(f.name) as g:
            return g.read()


Checks if all items are negative This piece of code checks to see if all the values within an array are all negative and returns true if they all are.

public static boolean allNegatives(int[] a) {

  if(a == null) {

      return false;

   }

    for(int i = 0; i < a.length; i++) {
        if(a[i] > 0)
            return false;
    } return true;
}

With java 8 you can reduce your code using a more functional programming. It would be something like next. You could even reduce it to one line:

public static boolean allNegatives(int[] a) {
     //@bowmore suggestion edit
     return a != null && Arrays.stream(a).allMatch(i -> i < 0);
}


Circle Summation (30 Points) InterviewStree Puzzle cont Further to the question
I solved it in following way.

#include <iostream>
#include <algorithm>
#include <iterator>
#include <stdio.h>

int main ( int argc, char **argv) {
    int T; // number of test cases
    std::cin >> T;
    for ( int i = 0 ; i < T; i++) {
        int N; // number of childrens
        std::cin >> N;
        int M; // number of rounds
        std::cin >> M;
        long long childrens[50];
        for ( int j = 0; j < N; j++ )
            std::cin >> childrens[j];

        int pos = 0;
        for ( int i = 0; i < N ; i++) {
            long long childs[50];
            for ( int j = 0; j < N; j++ )
                childs[j] = childrens[j];
            for ( int j = 0; j < M; j++) {
                int left  = ( pos     == 0)  ? N - 1  : pos - 1;
                int right = ( pos + 1 >= N ) ? 0      : pos + 1;
                childs[pos] = (childs[pos] + childs[left] + childs[right])% 1000000007;
                pos++;
                pos   = ( pos >= N ) ? 0 : pos;
            }
            std::copy(childs ,childs + N ,std::ostream_iterator<long long>(std::cout," "));
            std::cout << std::endl;
        }
        std::cout << std::endl;
    }
    return 0;
}

but the M can be 10^9, My code is going to take lot of time to finish, What are the various ways I can optize this.

#include <iostream>
#include <algorithm>
#include <iterator>
#include <stdio.h>

int main ( int argc, char **argv) {
    int T; // number of test cases
    std::cin >> T;
    for ( int i = 0 ; i < T; i++) {
        int N; // number of childrens
        std::cin >> N;
        int M; // number of rounds
        std::cin >> M;

Why split this across two lines instead of std::cin >> N >> M;?

        long long childrens[50];

I missed this in my first submissions and tried using 32 bit integers. It didn't work so well...

        for ( int j = 0; j < N; j++ )
            std::cin >> childrens[j];

        int pos = 0;
        for ( int i = 0; i < N ; i++) {
            long long childs[50];
            for ( int j = 0; j < N; j++ )
                childs[j] = childrens[j];
            for ( int j = 0; j < M; j++) {
                int left  = ( pos     == 0)  ? N - 1  : pos - 1;

You can use left = (pos + N - 1) % N which works thanks to modular math.

                int right = ( pos + 1 >= N ) ? 0      : pos + 1;

You can use right = (pos + 1) % N

                childs[pos] = (childs[pos] + childs[left] + childs[right])% 1000000007;
                pos++;
                pos   = ( pos >= N ) ? 0 : pos;

You can use pos = (pos + 1) % N

            }
            std::copy(childs ,childs + N ,std::ostream_iterator<long long>(std::cout," "));
            std::cout << std::endl;
        }
        std::cout << std::endl;
    }
    return 0;
}

Generally, writing brute force solutions for a site like Interviewstreet is a waste of time.   Its not like you can write your brute force solution, and then optimize it so that it will run fast enough. You need to come up with a completely different approach.

Since Interviewstreet is trying to test your abilities I'm not going to share my solution. (Unless you have a solution for the Meeting Point problem, then I'll trade :P) But I will give a hint: think back to linear algebra.


Universal number comparer I wrote some code which compares numbers of all datatypes with each other.
The standard Comparer which is implemented by every number Type only compares numbers of the same type.

I cast to decimal because this is the largest possible number type and should cover everything.

What do you think about the code?
Is there any room for improvement?
How can I handle NaN the best way?
Do you think BigInt and Real numbers should be a part of it?

public class UniversalNumberComparer: IComparer
{
    public int Compare(object x, object y)
    {
        if(!IsNumber(x) || !IsNumber(y))
            throw new InvalidOperationException();

        if(x == null)
        {
            return y == null
                ? 0
                : -1;
        }
        else
        {
            if(y == null)
                return 1;
        }

        return Decimal.Compare(Convert.ToDecimal(x), Convert.ToDecimal(y));
    }

    private bool IsNumber(object value)
    {
        if(value is sbyte || value is sbyte?)
            return true;
        if(value is byte || value is byte?)
            return true;
        if(value is short || value is short?)
            return true;
        if(value is ushort || value is ushort?)
            return true;
        if(value is int || value is int?)
            return true;
        if(value is uint || value is uint?)
            return true;
        if(value is long || value is long?)
            return true;
        if(value is ulong || value is ulong?)
            return true;
        if(value is float || value is float?)
            return true;
        if(value is double || value is double?)
            return true;
        if(value is decimal || value is decimal?)
            return true;
        return false;
    }
}

As for your question about NaN: this is also a situation where conversion to decimal is likely to be problematic. Assuming that you resolve that problem, the question then is what to do about it in the comparison.

It depends on the goals of your comparison. The semantics of NaN are "this thing isn't even a number, so asking which one is bigger is a nonsensical question", and therefore any operation on NaN is false. That means that NaN is not greater than any number, not smaller than any number and not equal to any number, including itself. If that's acceptable for your comparison purposes then that's the semantics you should implement.

Those semantics are not acceptable for sorting. If the comparison is being used in a comparison sort then the comparison is required to produce a total consistent order. It must be reflexive -- every number must equal itself. It must be antisymmetric -- if a < b is true then b < a must be false. And it must be transitive -- if a < b is true and b < c is true then a < c must be true. The usual way to deal with NaNs is to say that all NaNs are equal to each other -- achieving reflexivity, and that all NaNs are smaller than every non-NaN -- achieving antisymmetry and transitivity.

And don't forget that you'll need to deal with positive and negative infinities.


Sum of squares of two largest of three numbers Given the following problem (SICP Exercise 1.3):

  Define a procedure that takes three
  numbers as arguments and returns the
  sum of squares of the two largest
  numbers.

I wrote the following (somewhat clumsy) solution in Scheme.  How can I make it better?

(define (greatest-two a b c)
  (cond ((> a b) (cond ((> b c) (list a b))
                       (else (list a c))))
        ((> a c) (cond ((> c b) (list a c))
                       (else (list a b))))
        (else (list b c))))

(define (square x) (* x x))
(define (sum-of-squares a b) (+ (square a) (square b)))
(define (f a b c)
  (apply sum-of-squares (greatest-two a b c)))

Scheme is a lot more functional than Common Lisp. The way you can apply that to this situation is by making more use of passing functions around (to the point that this problem is almost a one-liner). For the puzzle as written, I'd do something like

(define (big-squares a b c)
   (apply + (map (lambda (n) (* n n))
                 (take (sort (list a b c) >) 2))))

If you wanted to decompose it properly into named functions

(define (square num) (expt num 2))
(define (sum num-list) (apply + num-list))
(define (two-biggest num-list) (take (sort num-list >) 2))

(define (big-squares a b c) (sum (map square (two-biggest (list a b c)))))

If you wanted to go completely overboard, also toss in

(define (squares num-list) (map square num-list))

which would let you define big-squares as

(sum (squares (two-biggest (list a b c))))

(code above in mzscheme)


Search function for books I was making a library program for trying to practice C++. I wanted to use a vector to store all the books, the books being a struct to hold the general things the books would have.

struct Book
{
    string  bookName; // book's name
    string  bookAuthor; // book's author
    string  bookID; // a random generated string
    int     bookType; // what type of book
};

The books in the library would be stored in a vector. To find one of these books I used a function that takes in a bookId parameter and uses it to find the book.

Book LibrarySystem::LocateBook(string bookId)
{
    /* May not be the fastest search algorithm */

    for (int i = 0; i <= mBooks.size(); i++)
    {
        Book result = mBooks[i];
        if (bookId == mBooks[i].bookID)
        {
            return result;
        }

        else
        {
            cout << "FAILED TO FIND " + bookId << endl;
        }
    }
}

This works but I was wondering if it could be faster. Looking at it, I am going through it one by one checking if the bookId is equal to the one provide in the parameter which seems slow.

I did do a small test to check if it all worked. I created a function:

string LibrarySystem::BookDataOutput(string bookId)
{
    Book book = LocateBook(bookId);
    string data =
        "Book author: " + book.bookAuthor + "\n" +
        "Book name: " + book.bookName + "\n" +
        "Book type: " + to_string(book.bookType) + "\n" +
        "Book Id: " + book.bookID;
    return data;
}

Which outputs a single string with all the book's information. It uses LocateBook to find the the book with the bookId.

I run in main:

int main(int args, char* argv[])
{
    /* Seed the random generator */
    srand(time(NULL));

    LibrarySystem library;

    library.RegisterBook("Test1", "Test1", BOOK_TYPE::Adult, "test1");

    cout << library.BookDataOutput("test1") << endl;

    cin.get();
}

and the output is:

Book author: Test1
Book name: Test1
Book type: 5
Book Id: test1

Which matches all the information I provided.

Since your search is only by 1 parameter, a map<string,Book> would work a lot better than a vector.

map<string,Book> mBooks;
string LibrarySystem::BookDataOutput(string bookId)
{
    Book book = mBooks[bookId];
    string data =
        "Book author: " + book.bookAuthor + "\n" +
        "Book name: " + book.bookName + "\n" +
        "Book type: " + to_string(book.bookType) + "\n" +
        "Book Id: " + book.bookID;
    return data;
}


Python script to renew key from Vault every 30s A small Python lib to renew key from Vault every 30s to always get the latest key.

from __future__ import print_function

import hvac
import time
import threading
import os

VAULT_URL = os.environ['VAULT_ADDR']
VAULT_TOKEN = os.environ['VAULT_TOKEN']

class Client:
    def __init__(self, *keys):
        self.keys = keys
        self.data_dict = {}
        self.client = hvac.Client(
            url=VAULT_URL,
            token=VAULT_TOKEN)

        self.__renew()

    def read(self):
        for key in self.keys:
            self.data_dict[key] = self.client.read(key)
        return self.data_dict

    def __renew(self):
        self.client.renew_token()
        threading.Timer(30, self.__renew).start()
        self.read()

if __name__ == '__main__':
    client = Client('secret/key')
    print(client.read())

You don't need a class here. Neither you need a Timer.

import hvac
import time
import threading
import os
import sys

VAULT_URL, VAULT_TOKEN = sys.argv[1:]

def read(client, *keys):
    client.renew_token()
    return {k: client.read(key) for key in keys}

if __name__ == '__main__':
    client = hvac.Client(url=VAULT_URL, token=VAULT_TOKEN)
    while True:
        print(read(client, 'secret/key'))
        time.sleep(30)


Inputting 10 numbers in an array I would like some tips on how I could simplify my code. Basically what I am trying to do is to input 10 numbers in an array but this looks messy so I am trying to change it to look less messy.

Imports System

Public Module Module1

Public Sub Main()
    Dim score(10) as decimal
    Console.WriteLine("Enter the score")
    score(0) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(1) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(2) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(3) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(4) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(5) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(6) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(7) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(8) = Console.ReadLine
    Console.WriteLine("Enter the score")
    score(9) = Console.ReadLine
    Console.WriteLine(score(0) + score(1) + score(2) + score(3) + score(4) + score(5) + score(6) + score(7) + score(8) + score(9))

End Sub

Repetition. Always a sign that something could be improved. DRY (Don't repeat yourself) is the mantra here.

Magic Numbers. Passable for one-off code, but can get in the way if you wish to maintain or extend the code.

I will deal with the magic numbers first (and you'll see why when I address the repetition)

You have Dim score(10) as decimal where there is no real reason for 10 except that it is a nice number. We can fix this (may seem superfluous at first, but it is a good habit) by the following:

Const numOfInputs as Long = 10 ' You can always call it what you like, but make it sensible and descriptive
Dim score(numOfInputs) as decimal

Now, if you wanted to work with 12 numbers you only have to change the assignment of numOfInputs. You have two areas of repetition. Addressing the main block:

For iterator as Long = 0 to numOfInputs - 1 ' 0 based array.
    Console.WriteLine("Enter the score (input number " & iterator & ")")
    score(iterator) = Console.ReadLine
Next iterator

See how I was also able to "personalise" the prompt string! Now if you want to do something else in here, you only have to fix the code once, and not ten times.

Addressing the output. There are a couple of ways to do this. One of those is to set up another loop, loop through the array and then use the output. But we already have the exact same loop that we would use to add them all up.

Const numOfInputs as Long = 10 ' You can always call it what you like, but make it sensible and descriptive
Dim score(numOfInputs) as decimal
Dim finalCount as decimal = 0 'OK, the default value is 0 but I like to be explicit
For iterator as Long = 0 to numOfInputs - 1 ' 0 based array.
    Console.WriteLine("Enter the score (input number " & iterator & ")")
    score(iterator) = Console.ReadLine
    finalCount += score(iterator) ' simply add up the items as we go along.
Next iterator

As you can see, I have now put in place finalCount so that we can add up as we go along - which is what I would have done if I had it in a separate loop anyway.

Oh, and now you can print it out through Console.WriteLine(finalCount).

Key lessons

Loop rather than repeat. Much easier to maintain
Reuse loops where you can - it is much more efficient. Think about your program logic so you can be efficient with loops.
Replace magic numbers with variables (constants) that are defined somewhere logical. Much easier to replace a well-named constant only once than trying to find that 10 and then working out if that needs to change to 12 or if it really is 10.


Designing a simple Battleship game in Python I am trying to code Battleship. It should be a text one-player game against computer where computer and human player take turns in shooting at opponent's ships. I decided to start implementation with this part: A human types target coordinates and computer answers if a ship has been hit or even if it has been sunk. Ship positions are fixed (predefined in the program). For some time I wondered how to represent game plan and ships to get this working as smoothly as possible. This is what I have put together:

class GamePlan:

    DIMX = 10
    DIMY = 10

    SHIP_MISS = 0
    SHIP_HIT = 1
    SHIP_DEAD = 2

    SHIP_COUNTS = {
        2: 1,
        3: 1,
    }

    def __init__(self):
        self.ships = [
            [(1,1), (1,2)],
            [(5,6), (5,7), (5,8)],
        ]

    def hit(self, target):
        hit_ship = None
        hit_ship_index = None
        for i, ship in enumerate(self.ships):
            if target in ship:
                ship.pop(ship.index(target))
                hit_ship = ship
                hit_ship_index = i
        if hit_ship == []:
           self.ships.pop(hit_ship_index)
           return self.SHIP_DEAD
        if hit_ship:
            return self.SHIP_HIT
        return self.SHIP_MISS

def main():
    game_plan = GamePlan()

    while True:
        raw_coords = raw_input('Enter coords: ')
        str_coords = raw_coords.split()
        coords = tuple([int(c) for c in str_coords])
        if len(coords) != 2:
            print 'Bad input'
            continue

        result = game_plan.hit(coords)
        if result == GamePlan.SHIP_DEAD:
            print 'Ship dead'
        if result == GamePlan.SHIP_HIT:
            print 'Ship hit'
        if result == GamePlan.SHIP_MISS:
            print 'Missed'

if __name__ == "__main__":
    main()

EDIT: GamePlan should be probably called Board as answer of Janne Karila suggests. Just to clarify what I meant by that name with my flawed English.

There are a few things I am unsure about:

Is it correct that GamePlan processes shooting (in the method hit)?
Is hit a good name for that method or should it be something like process_hit? Because GamePlan is being hit, it is not hitting anything. Is hit still good in such case? This is probably my biggest concern.
Should ships be represented as objects of a class Ship instead?
Is GamePlan a good thing to have or is it useless? I mean I could make class AI that directly owns ships but I am not sure where stuff like SHIP_COUNTS would go then. I have also planned that I will use GamePlan to generate ship positions (hence SHIP_COUNTS and DIMX, DIMY which are unused atm) but plan generation could also easily go to AI, I guess.
Is there anything else that is wrong?

Perhaps GamePlan would better be named eg. Board? English is not my mother tongue, but AFAIK game plan == strategy.

1,3,4: The ships are static, they just occupy some space on the board. However, a ship object would be aware which squares belong to the ship, and perhaps should be responsible to determine if it has been sunk.

2: hit is perfect; in OOP the object is often the object also in grammatical sense. Think of file.close().

5: If the player hits the same coordinates twice, you'll report a miss, which is wrong.


Sweep & prune broadphase algorithm My game engine uses the following broadphase collision detection algorithm:
internal void SweepAndPrune()
{
    // First: order all objects from left to right on the x-axis:
    IOrderedEnumerable<GameObject> axisList = _gameObjects.OrderBy(x => x.LeftRightMost.X);

    // loop through all objects:
    for(int i = 0; i < axisList.Count(); i++)
    {
        // For each object, iterate over all the subsequent objects in the list
        // and find out if there are overlaps on the x-axis:
        for(int j = i+1; j < axisList.Count(); j++)
        {
            GameObject a = axisList.ElementAt(i);
            GameObject b = axisList.ElementAt(j);
            if(b.Left > a.Right)
            {
                // if their is no overlap, then the rest will not overlap as well.
                // might as well stop the inner loop here:
                break;
            }
            // if there is an overlap, add A to B's list
            // and B to A's list of potential collisions.
            // [every object's list of collision candidates is
            // cleared before the next frame]
            a._collisionCandidates.Add(b);
            b._collisionCandidates.Add(a);
        }
    }
}

The problem is, this algorithm slows down the performance by a lot if there are > 50 objects on screen.
Also, the list of GameObject instances (_gameObjects) needs to get sorted another time per frame because I need to order the objects by their distances from the camera (background objects need to get rendered first because of transparency!).
Is there anything I can do to speed things up?

An IEnumerable (and IOrderedEnumerable) represents a question, not an answer. axisList is not really a list -- it is a question asking "What is the sequence of elements from _gameObjects ordered by X value?". So every time you do this:
            GameObject a = axisList.ElementAt(i);
            GameObject b = axisList.ElementAt(j);

You are answering the question "What is the ith element in that ordering". Which means it needs to re-order the sequence on every iteration! Your outer loop ends up being hugely expensive (O(n^3 log(n)) if my math checks out); you certainly don't need to do that.
You should cache the result of the ordering and re-use it in your loop. Change this
IOrderedEnumerable<GameObject> axisList = _gameObjects.OrderBy(x => x.LeftRightMost.X);

to this:
var orderedByX = _gameObjects.OrderBy(x => x.LeftRightMost.X).ToList();

.ToList() will evaluate the OrderBy query and return a flattened "answer" that you can keep reusing, and as a bonus it has O(1) lookup so ElementAt is fast -- you could even change ElementAt(i) to [i] to make that more clear.


StackList implementation I needed a structure that works really similar to the predefined Stack<T> but with a few extra functionalities. Examples include getting the index of value in the collection and being able to index the structure itself.

Any flaws or improvement notes are welcome!

public class StackList<T> : IEnumerable<T>
{
    private readonly List<T> _stackList = new List<T>();

    public T Pop()
    {
        if (_stackList.Count == 0)
        {
            throw new InvalidOperationException("The stack is empty");
        }
        T value = _stackList[0];
        _stackList.Remove(value);
        return value;
    }

    public void Push(T item)
    {
        _stackList.Insert(0, item);
    }

    public T Peek()
    {
        if (_stackList.Count == 0)
        {
            throw new InvalidOperationException("The stack is empty");
        }
        return _stackList[0];
    }

    public IEnumerator<T> GetEnumerator()
    {
        return _stackList.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public T this[int index]
    {
        get { return _stackList[index]; }
        set { _stackList[index] = value; }
    }

    public int Count
    {
        get { return _stackList.Count; }
    }

    public void Clear()
    {
        _stackList.Clear();
    }

    public bool Contains(T item)
    {
        return _stackList.Contains(item);
    }

    public int IndexOf(T item)
    {
        return _stackList.IndexOf(item);
    }
}

Issues

public void Push(T item)
{
    _stackList.Insert(0, item);
}

The hurts the stack list.

It would be much easier if you just added new items at the end of the list. The Insert needs to rebuild the entire list:

  This method is an O(n) operation, where n is Count.

For an addition this is a real bottleneck.

The same applies to the Pop method which internally uses List.Remove:

  This method performs a linear search; therefore, this method is an O(n) operation, where n is Count.

With these features the StackList<T> is unusable - as far as performance is concerned.

I saw this comment of you:

  I've made it the way it is because I wanted to be able visually see how the stack is sorted while debugging but I guess that will do.

There is a much simpler solution for debug helpers without sacrificing performance.

You can decorate your class with the DebuggerDisplay attribute and build what you need to see in debug.

Here's the pattern I use:

[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class StackList<T> : IEnumerable<T>
{
    private string DebuggerDisplay => $"[{string.Join(", ", this)}]";
}

It's not a mistake that the property is private. The debugger will find it anyway and by being private it doesn't pollute the public API.

Challenging the purpose of the StackList<T>

Finally I'd like to undermine the purpose of the StackList<T>.

You write:

  I needed a structure that works really similar to the predefined Stack but with a few extra functionalities for example getting the index of value in the collection and being able to index the structure itself.

I doubt you need a new type for this. With a few extensions you can achieve the same with less effort and based on the original Stack.

Let's try...

  getting the index of value in the collection

We can build a new extension for this by utilizing two other extensions:

static class StackExtension
{
    public static int IndexOf<T>(this Stack<T> stack, T value)
    {
        var result = stack.Select((x, i) => new { x, i }).FirstOrDefault(y => y.x.Equals(value));
        return result == null ? -1 : result.i;
    }
}

or with C# 6

public static int IndexOf<T>(this Stack<T> stack, T value)
    => stack.Select((x, i) => new { x, i }).FirstOrDefault(y => y.x.Equals(value))?.i ?? -1;

Example:

var s = new Stack<int>();
s.Push(1);
s.Push(2);
s.Push(3);
s.Push(4);

s.Dump();

s.IndexOf(2).Dump(); // 2
s.IndexOf(4).Dump(); // 0
s.IndexOf(5).Dump(); // -1

  being able to index the structure itself

For this you don't even need to write an extension because there is already one: ElementAt and its safe equivalent ElementAtOrDefault.


Optimal custom operator with generic type constraint for numerical type I'm implementing (for an article) two custom infix operators:

¿% - to calculate percent of total
%? - to calculate the percent that represents a segment of total

After debugging some errors and looking for information I finally found a way to get my code working:

protocol NumericType {

    static func *(lhs: Self, rhs: Self) -> Self
    static func *(lhs: Self, rhs: Int) -> Self
    static func /(lhs: Self, rhs: Self) -> Self
    static func /(lhs: Self, rhs: Int) -> Self

} // NumericType

extension Double : NumericType {

    internal static func *(lhs: Double, rhs: Int) -> Double {
        return lhs * Double(rhs)
    }

    internal static func /(lhs: Double, rhs: Int) -> Double {
        return lhs / Double(rhs)
    }

 } // extension Double : NumericType

extension Float : NumericType {

    internal static func *(lhs: Float, rhs: Int) -> Float {
        return lhs * Float(rhs)
    }

    internal static func /(lhs: Float, rhs: Int) -> Float {
        return lhs / Float(rhs)
    }

 } // extension Float : NumericType

extension Int : NumericType { }

infix operator ¿%

func ¿% <T: NumericType>(percentage: T, ofThisTotalValue: T) -> T {

    return (percentage * ofThisTotalValue) / 100

} // infix operator ¿%

infix operator %?

func %? <T: NumericType>(segment: T, ofThisTotalValue: T) -> T {

    return (segment * 100) / ofThisTotalValue

} // infix operator %?

let percentage: Double = 8
let price: Double = 45

let save = percentage ¿% price

print("\(percentage) % of \(price) $ = \(save) $")

print("\(save) $ of \(price) $ = \(save %? price) %")

The output:

8.0 % of 45.0 $ = 3.6 $
3.6 $ of 45.0 $ = 8.0 %

Do you thinks there could be a more optimal and readable approach? Could you give some advice or share an example?

I imagine that you started out with something like this:

infix operator ¿%

func ¿% <T: NumericType>(percentage: T, ofTotal: Double) -> Double {
    return percentage * ofTotal / 100.0
}

And received the error:

  binary operator '*' cannot be applied to operands of type 'T' and 'Double'

You've solved this by creating a function with signature NumericType * Double -> Double... which works, but as you've found requires a bunch of boilerplate.

The alternative approach is to to find a way to convert NumericType to a double so that in the end you're doing the multiplication on two doubles:

(NumericType -> Double) * Double - > Double

An implementation of this would look something like this:

protocol NumericType {
    var doubleValue: Double { get }
}

infix operator ¿%
infix operator %?

func ¿% <T: NumericType>(percentage: T, ofTotal: Double) -> Double {
    return percentage.doubleValue * ofTotal / 100.0
}

func %? <T: NumericType>(segment: T, ofTotal: Double) -> Double {
    return segment.doubleValue * 100 / ofTotal
}

extension Double: NumericType {
    var doubleValue: Double { return self }
}

extension Int: NumericType {
    var doubleValue: Double { return Double(self) }
}

However, please please please reconsider using custom operators like this.

I don't think that it's improving the readability or maintainability of your code, and unless you're using it every single day, I feel like it's something that will cause headaches in the future.


A package for the DOM - Events Please provide feedback on the correctness of this code.  It should handle older versions of IE but how far back it goes I have not determined yet.

/**************************************************************************************************
EVENTS
*/

    // ... snip

    Priv.functionNull = function () {
        return undefined;
    };

    // createEvent
    Priv.createEvent = function () {
        if (doc.createEvent) {
            return function (type) {
                var event = doc.createEvent("HTMLEvents");
                event.initEvent(type, true, false);
                $A.someKey(this, function (val) {
                    val.dispatchEvent(event);
                });
            };
        }
        if (doc.createEventObject) {
            return function (type) {
                var event = doc.createEventObject();
                event.eventType = type;
                $A.someKey(this, function (val) {
                    val.fireEvent('on' + type, event);
                });
            };
        }
        return Priv.functionNull;
    };

    Priv.proto.createEvent = function (type) {
        return Priv.createEvent.call(this, type);
    };

    Pub.createEvent = (function () {
        return function (element, type) {
            var temp = [];
            temp[0] = element;
            Priv.createEvent.call(temp, type);
        };
    }());

    // addEvent
    Priv.addEvent = (function () {
        if (win.addEventListener) {
            return function (type, callback) {
                $A.someKey(this, function (val) {
                    val.addEventListener(type, callback);
                });
            };
        }
        if (win.attachEvent) {
            return function (type, callback) {
                $A.someKey(this, function (val) {
                    val.attachEvent('on' + type, callback);
                });
            };
        }
        return Priv.functionNull;
    }());

    Priv.proto.addEvent = function (type, callback) {
        return Priv.addEvent.call(this, type, callback);
    };

    Pub.addEvent = (function () {
        return function (element, type, callback) {
            var temp = [];
            temp[0] = element;
            Priv.addEvent.call(temp, type, callback);
        };
    }());

    //remove event
    Priv.proto.removeEvent = (function () {
        if (win.removeEventListener) {
            return function (type, callback) {
                $A.someKey(this, function (val) {
                    val.removeEventListener(type, callback);
                });
            };
        }
        if (win.detachEvent) {
            return function (type, callback) {
                $A.someKey(this, function (val) {
                    val.detachEvent('on' + type, callback);
                });
            };
        }
        return Priv.functionNull;
    }());

    Priv.proto.removeEvent = function (type, callback) {
        return Priv.removeEvent.call(this, type, callback);
    };

    Pub.removeEvent = (function () {
        return function (element, type, callback) {
            var temp = [];
            temp[0] = element;
            Priv.removeEvent.call(temp, type, callback);
        };
    }());

The good

Your code will handle any type of browser, is pretty easy to follow.

The bad

Pub.removeEvent will not work. That's because you define Priv.proto.removeEvent twice, whereas you probably want to define Priv.proto.removeEvent and Priv.removeEvent.

The ugly

You are using doc.createEvent which is deprecated, you should look into event constructors.

Furthermore

Because IE decided to do things differently for many event related things, a lot of people check as you do for the existence of a function and go from there, however your approach could be a little DRYer.

Priv.addEvent = (function () {
    if (win.addEventListener) {
        return function (type, callback) {
            $A.someKey(this, function (val) {
                val.addEventListener(type, callback);
            });
        };
    }
    if (win.attachEvent) {
        return function (type, callback) {
            $A.someKey(this, function (val) {
                val.attachEvent('on' + type, callback);
            });
        };
    }
    return Priv.functionNull;
}());

can be done as

Priv.addEvent = (function () {
  var addFunction = win.addEventListener ? 'addEventListener' : 'attachEvent',
      prefix      = win.addEventListener ? '' : 'on';

  return function (type, callback) {
    $A.someKey(this, function (val) {
      val[addFunction]( prefix + type, callback);
    });
  };
  return Priv.functionNull;
}());

This approach can be applied to addEvent, removeEvent and perhaps to 'createEvent'.

Finally, as always, you should steer away from someKey, just stick to the standard Array.


First attempt incorporating backbone.js in my code I have a feeling I might be doing it wrong, what major things would you improve in this code?

The code is basically handling a tour on first login, showing it only for first login users (activated externally via calling activate) and preventing the step to apear again if the user has already seen it.

I intentionally used only the basic building blocks of backbone.js to a bare minimum, e.g. I didn't use templates as the DOM at this moment is given as is, and the HTML elements are not "backbone" friendly

var tourModel, tourView;

(function($){
    var VISITED = "visited";
    function cookieName(step){
        return "tour_step" + step;
    }
    function isStepVisited(step){
        return $.cookie(cookieName(step)) === VISITED;
    }
    function setStepVisited(step){
        $.cookie(cookieName(step.number),VISITED);
    }
    var TourModel = Backbone.Model.extend({
        active: false, //private flag
        defaults: {
            step: {
                number:1,
                display:false
            }
        },
        activate:function(){
            this.active = true;
        },
        close:function(){
            this.toggleStep(this.get("step").number, false);
        },
        toggleStep: function(step, show){
            //if this is active (e.g. first time login) and the user haven't seen this step yet (e.g. cookie isn't set) or if this is closing the step (!show) then continue, else ignore
            if(!show || this.active && !isStepVisited(step)){
                this.set("step",{number:step, display:show});
            }
        },
        initialize: function(){
            this.on("change:step",function(model,step){
                if(step.display){
                    setStepVisited(step);
                }
            });
        }
    });

    tourModel = new TourModel();
})(jQuery);

(function($){

    var closeStepOne = function(){
        $("#step1").hide();
        $("#rss, #gorss, #mySitesTab").glowToggle(false);
    };
    var closeStepTwo = function(){
        $("#step2").hide();
        $(".help-text-drag, #gallery").glowToggle(false);
    };

    var stepFunctions = {
            1:function (show){
                if(show) {
                    $("#step1").show();
                    $("#rss, #gorss, #mySitesTab").glowToggle(true);
                    $("#rssInputs, #mySitesTab").expose({
                        closeOnEscape : true,
                        closeOnClick : false,
                        close:function(){
                            closeStepOne();
                        }
                    });
                } else {
                    closeStepOne();
                    $("#mySitesTab").expose("remove");
                }
            },
            2:function (show){
                if(show) {

                    $("#step2").show();
                    $(".help-text-drag, #gallery").glowToggle(true);

                    $(".help-text-drag, #step2, #gallery").expose({
                        closeOnEscape : true,
                        closeOnClick : false,
                        close:function(){
                            closeStepTwo();
                            //$("#rss, #gorss, #mySitesTab").glowToggle(false);
                        }
                    });
                    $("#step2").css("z-index","1003");
                } else {
                    closeStepTwo();
                     $("#gallery").expose("remove");
                }
            }
         };

    var TourView = Backbone.View.extend({

        model: tourModel,
        initialize: function() {
            this.model.on( 'change:step', this.render, this );
        },
        render: function(model,step) {
            //alert(JSON.stringify(step,4));
            stepFunctions[step.number](step.display);
        }
    });

    tourView = new TourView();
    //view private functions

})(jQuery);

$(function(){

    $(".tourCloseButton").click(function(){
       tourModel.close();
    });

});

Alternative Approach:

In your case, I believe that the use of a model is not really necessary for this task.  I would construct this using a master view with child views for each step:

A TourView that contains a variable tracking which step is currently active and listenting for an event from it's child views for navigation.

A TourStep1View .. TourStepNView for each step of the tutorial whose render function contains corresponding show and close function contains the hide.

Launch the TourView and have it render a child view for the first step.  The child view should throw an event to signal the parent view to swap it out for the next child view.


is my add method in binary search tree correct I want to implement code for inserting a new Node in a binary search tree in java  so please check if my code is correct ,,, Here is my method :

public class BinarySearchTree {
      …

      public boolean add(int value) {
            if (root == null) {
                  root = new BSTNode(value);
                  return true;
            } else
                  return root.add(value);
      }
}
        public class BSTNode {
         …

      public boolean add(int value) {
            if (value == root.data)
                  return false;
            else if (value <root.data) {
                  if (root.left == null) {
                        root.left = new BSTNode(value);
                        return true;
                  } else
                        return root.left.add(value);
            } else if (value > root.data) {
                  if (root.right == null) {
                        root.right = new BSTNode(value);
                        return true;
                  } else
                        return root.right.add(value);
            }
            return false;
      }
}

It looks to be correct.

However this code can result in unbalanced trees (like if you add all numbers in increasing order).

To keep it balanced you can use the techniques described for self-balancing trees