HackerRank "Nested Lists" Code I completed the "Nested Lists" challenge on HackerRank, and would love any feedback on my code.

My program is fed text with the number of students in a classroom, the name of a student, and their grade. For example:

4
Shadab
8
Varun
8.9
Sarvesh
9.5
Harsh
10

The code should return the student with the second lowest grade. Here's the output using the above input:

Varun

Here is my code:

def secondlow(students):
    grades = []
    for student in students:
        grades.append(student[1])
    sort_grades = sorted(grades)
    seclow_grade = sort_grades[0]
    for grade in sort_grades:
        if grade != seclow_grade:
            seclow_grade = grade
            break
    seclow_stud = []
    for student in students:
        if student[1] == seclow_grade:
            seclow_stud.append(student[0])
    for name in sorted(seclow_stud):
        print(name)

students = []
for pupil in range(int(input())):
    new_stud = [input(), float(input())]
    students.append(new_stud)
secondlow(students)

The code mostly follows usual coding style and reads good. I would just change secondlow to second_low and avoid using abreviations in variable names.

Now for the improvements:

Print vs return

Do not code too much behaviour into your functions. Let them compute their stuff and return the result so the caller can do whatever they want with it. It doesn't add much here, but it is a good habit to have.

Getting the value you are looking for

seclow_stud = []
for student in students:
    if student[1] == seclow_grade:
        seclow_stud.append(student[0])

You do not need to store the only value you are looking for into a list. Especialy when using a for loop after that to extract the element out of the list:

for student in students:
    if student[1] == seclow_grade:
        return student[0]

Sorted

sort_grades = sorted(grades)
seclow_grade = sort_grades[0]
for grade in sort_grades:
    if grade != seclow_grade:
        seclow_grade = grade
        break

The sorted builtin returns a list of values in increasing order. What is important to note here is that sort_grades is a list. So sort_grade[0] is the lowest grade and sort_grade[1] is the second lowest. No need to search for the value yourself, sorted already did the work for you.

seclow_grade = sorted(grades)[1]

List-comprehensions

grades = []
for student in students:
    grades.append(student[1])

Python have these construct to easily build lists out of other iterables:

grades = [student[1] for student in students]

You can also use the same construct when building lists out of the input. Overall code would look like:

def secondlow(students):
    grades = [student[1] for student in students]
    seclow_grade = sorted(grades)[1]
    for student in students:
        if student[1] == seclow_grade:
            return student[0]

students = [[input(), float(input())] for _ in range(int(input()))]
print(secondlow(students))

Dictionaries

I know that the challenge explicitly mentionned lists of lists, but Python have dictionaries that can map a value to another. You can use them as a replacement:

def secondlow(students):
    seclow_grade = sorted(students.values())[1]
    for student in students.items():
        if student[1] == seclow_grade:
            return student[0]

students = {}
for _ in range(int(input())):
    name = input()
    grade = float(input())
    students[name] = value
print(secondlow(students))

Or you can take advantage of the way you can retrieve items associated to others by switching the way you store your data:

def secondlow(students):
    second_low = sorted(students)[1]
    return students[second_low]

students = {}
for _ in range(int(input())):
    name = input()
    grade = float(input())
    students[value] = name  # Note the switch here
print(secondlow(students))

Edit

After reading the challenge description again, I realised that there can be multiple students with the second lowest grade. And in fact, there might be cases where there are multiple student with the lowest grade. Making my point about having the second lowest grade as sorted(grades)[1] irrelevant… Unless you remove duplicates using a set:

def second_low(students):
    second_low_grade = sorted(set(students.values()))[1]
    for student in students.items():
        if student[1] == second_low_grade:
            yield student[0]

students = {}
for _ in range(int(input())):
    name = input()
    grade = float(input())
    students[name] = value

for second_lowest_student in second_low(students):
    print(second_lowest_student)

Here I used the yield keyword to avoid building a list in second_low. What it does is allowing the function to return several results: one each time there is a match (if student[1] == second_low_grade). Here is the version using list of lists:

def second_low(students):
    grades = set(student[1] for student in students)
    seclow_grade = sorted(grades)[1]
    for student in students:
        if student[1] == seclow_grade:
            yield student[0]

students = [[input(), float(input())] for _ in range(int(input()))]
for second_lowest_student in second_low(students):
    print(second_lowest_student)

Unfortunately, such requirements prevent you from using dictionaries the other way around.


LiveTime Object Literal I have a website that retrieves time data from any given location on the world live.

I wanted to be able to automatically change the time and not deal with the hassle of local client times.

It's fairly simple, but since I'm not that good with JS, I'd like to know if there is anything I could improve about it?

Suggestions are welcome!

(function() {
    "use strict";

    var LiveTime = {
        rawOffset: null,
        dstOffset: null,
        timeoutId: null,

        setOffset: function(raw, dst) {
            LiveTime.rawOffset = raw * 1000;

            if (typeof dst !== "undefined") {
                LiveTime.dstOffset = dst * 1000;
            }
        },

        start: function() {
            LiveTime.timeoutId = (function increment() {
                var now = new Date();
                var now = new Date(
                    now.getUTCFullYear(),
                    now.getUTCMonth(),
                    now.getUTCDate(),
                    now.getUTCHours(),
                    now.getUTCMinutes(),
                    now.getUTCSeconds(),
                    now.getUTCMilliseconds()
                );

                now.setTime(now.valueOf() + LiveTime.rawOffset + LiveTime.dstOffset);

                var element = document.getElementById("current-time");
                element.innerHTML = ("0" + now.getHours()).slice(-2) + ":"
                                  + ("0" + now.getMinutes()).slice(-2) + ":"
                                  + ("0" + now.getSeconds()).slice(-2);

                LiveTime.timeoutId = setTimeout(increment, 50);
            })();
        },

        stop: function() {
            clearTimeout(LiveTime.timeoutId);
        }
    }

    //usage example
    LiveTime.start();

    //simulate new location after 5 seconds
    setTimeout(function() {
        LiveTime.setOffset(-9 * 3600, 3600); //alaska
   }, 5000);

    //stop the time after 5 more seconds
    setTimeout(function() {
        LiveTime.stop();
   }, 10000);
})();

From a once over:

dst stands for daylight saving time, you should comment that
offset I assume is the offset to the UTC (Greenwich time), you should comment that as well
I am ambiguous about declaring rawOffset, dstOffset, timeoutId. I can see the value of self documenting code, but is it worth it ? Not sure. The code works fine if you delete those 3 statements.
Consider falsy evaluations

if (typeof dst !== "undefined") {
  LiveTime.dstOffset = dst * 1000;
}

compared to

if (dst) {
  LiveTime.dstOffset = dst * 1000;
}

or even (a bit hacky)

LiveTime.dstOffset = dst ? dst * 1000 : 0;

Do not declare variables twice like here:

var now = new Date();
var now = new Date(

You can get the timestring this way:

now.toTimeString().substring(0,8);

This basically keeps only the first 8 (hh:mm:ss) characters of the time string
You can get the UTC date value this way:

new Date((new Date()).toUTCString().slice(0,-4) )

This gets the UTC date, strips off the timezone part and re-creates the date
document.getElementById("current-time"); <- This should be a parameter
As @Pinoniq mentioned, ideally you should write your class/code so that you can run multiple instances.
LiveTime is not a constructor (yet), so it should be called liveTime

This would be my counter proposal:

function liveTime(elementId, offsetSeconds, dayLightTimeSavingSeconds) {
    "use strict";

    var timeZoneOffset = 0,
        daylightTimeSavingOffset = 0,
        element = document.getElementById(elementId),
        timeoutId;

    if( offsetSeconds ){
      setTimeOffset( offsetSeconds, dayLightTimeSavingSeconds );
    }

    function setTimeOffset(offsetSeconds, dayLightTimeSavingSeconds) {
        //Convert to milliseconds
        timeZoneOffset = offsetSeconds * 1000;
        daylightTimeSavingOffset = (dayLightTimeSavingSeconds || 0) * 1000;
    }

    function start() {
        if( timeoutId ){
          return; //We already started
        }
        (function increment() {
            //Set the time to UTC time
            var now = new Date((new Date()).toUTCString().slice(0, -4));
            //Add the configured offsets
            now.setTime(now.valueOf() + timeZoneOffset + daylightTimeSavingOffset);
            //Update the HTML
            element.innerHTML = now.toTimeString().substring(0, 8);
            //Come back again
            timeoutId = setTimeout(increment, 50);
        })();
    }

    function stop() {
        clearTimeout(timeoutId);
        timeoutId = 0;
    }
    return {
        setTimeOffset: setTimeOffset,
        start: start,
        stop: stop
    };
}

//usage example
liveTime('current-time',-9 * 3600, 3600).start();
liveTime('current-time2', 4 * 3600 ).start();
Alaska time:

Moscow time:


Code Refactoring for Dynamic List in Angular 8 With a specific requirement of showing/hiding the pages, the following implementation has been carried out.

The pages which needs to be hidden will be provided through an array

this.unshowPages = ['menuPage', 'aboutusPage','linkedInPage'];

The corresponding HTML file,

 <ng-container *ngIf="showaboutPages">

            About us Page

      </ng-container>
       <ng-container *ngIf="showlkdPages">

            LinkedIN Page

      </ng-container>

The ts file will be

unshowPages= [];

showMenuPages: boolean = true;
showaboutPages: boolean = true;
showlkdPages: boolean = true;

ngOnInit() {
//this.unshowPages = ['menuPage', 'aboutusPage','linkedInPage'];
this.unshowPages = ['menuPage'];

this.unshowPages.forEach((x:string)=> {

  console.log(x)

  switch(x){
    case 'menuPage': this.showMenuPages = false; break;
    case 'aboutusPage': this.showaboutPages = false; break;
    case 'linkedInPage': this.showlkdPages = false; break;

  }

  });

The problem comes here is when pages gets updated, the switch case should be modified to adapt new pages. Is there any optimized way to achieve this.

The minimal reproducible working code : https://stackblitz.com/edit/angular-ivy-1antde

To handle such dynamic cases, you need to have dynamic code in both ts and html side.Take a look at this demo code which can be refactored furthur.

Here we have changed HTML

        <ng-container *ngFor="let page of pages">

                    {{page.title}}

        </ng-container>

and similarly we created a dynamic array

pages = [
  {
    id: 'menuPage',
    routing: '#',
    title: 'Menu Pages',
    showDOM: true
  },
  {
    id: 'aboutusPage',
    routing: '#',
    title: 'About US Pages',
    showDOM: true
  },
  {
    id: 'linkedInPage',
    routing: '#',
    title: 'LinkedIn Pages',
    showDOM: true
  }
]

It's still in a very crude format because your case will surely require more things from it. But this would help you to get the idea on how you should proceed.


Implement a queue using a linked list Implement a queue using a linked list.

I'm looking for a code review, optimizations and best practices.

public class QueueAsLinkedList<T> {

    private Node<T> front;
    private Node<T> rear;
    private int size;

    public QueueAsLinkedList() {}

    private static class Node<T> {
        T item;
        Node<T> next;

        Node (T item, Node<T> next) {
            this.item = item;
            this.next = next;
        }
    }

    public void add(T item) {
        Node<T> node = new Node<T>(item, null);
        if (rear == null) {
            rear = node;
            front = node;
        } else {
            rear.next = node;
        }

        rear = node;
        size++;
    }

    public T remove() {
        if (front == null) {
            throw new NoSuchElementException();
        }
        T item = front.item;
        front = front.next;
        if (front == null) {
            rear = null;
        }
        size--;
        return item;
    }

    public T peek() {
        if (front == null) {
            throw new NoSuchElementException();
        }
        return front.item;
    }

    public int getCount() {
        return size;
    }

    public boolean isEmpty() {
        return size == 0;
    }

    public static void main(String[] args) {
        QueueAsLinkedList<Integer> ll = new QueueAsLinkedList<Integer>();
        ll.add(1);   ll.add(2); ll.add(3);
        while (!ll.isEmpty()) {
            System.out.println(ll.remove());
        }
    }
}

There is no point in having the constructor for Node require the 'next' node. It is never part of the constructor, in your code.
The default constructor is useless code here. You may as well remove it, it does nothing.
the method getCount should be called size to be conformant with other classes.
I prefer when people use just one signal for the system state. In your code, you use size == 0 in some places to determine if the queue is empty, and in other places you use front == null

Otherwise the code looks functional, and neat.


Counting number of `1` digits in the value of \$11^n\$ I am trying to improve the efficiency of my code,

The Question is:

Given a value n, where n is in the range [0 .. 500], return the number of digits equal to 1 in the decimal representation of the number \$11^n\$.

Naturally when dealing with a larger power (where n is a large number), this reduces the efficiency drastically.

I managed to produce a quick but inefficient rough solution, that seems to work, but what could I do to improve the code and its efficiency?

import java.util.ArrayList;

public class Solution {

private static int num = 11;

public static int solution(int n) {

    int counter = 0;
    String compare = "1";
    double answer = Math.round(Math.pow(num, n));
    System.out.println(answer);

    String s = String.valueOf(answer);
    String[] parts = s.split("");

    for (int i=0; i<parts.length; i++){
        if (parts[i].equals(compare)){
            counter++;
        }
    }
    System.out.println(counter);
    return counter;
}

public static void main(String[] args) {

    solution(8);
    solution(3);

    }
}

Any help in improving my understanding of the techniques of writing more efficient code would be much appreciated as well as improvements to the code example above.

You should put your programming skills aside for a bit and think. Think mathematically, what it actually means.

$$ 11^n = (10+1)^n $$

Lets say a=10, b=1 => \$(a+b)^n\$, this can be expanded using pascal triangle:

   1
  1 1
 1 2 1
1 3 3 1
...

I suppose everybody knows how this continues...

It then goes like this:

$$ (a+b)^n = {n \choose 0} a^n b^0 + {n \choose 1} a^{n-1} b^1 + \dots + {n \choose n} a^0 b^n $$

You can notice that since b=1 we can erase all those terms \$b^x\$

And we are left with
$$ (a+b)^n = {n \choose 0} a^n + {n \choose 1} a^{n-1} + \dots + {n \choose n} a^0 $$

Since a=10, we know \$a^n\$ is basically a 1 followed by n zeroes.

Simple add and carry of the coefficients should now show you when a digit is 1 and when it is anything else.


Is there a better SQLite update / insert pattern than this? I've got a function which updates an existing record or inserts a new one, but it looks very clunky to me.

using (SQLiteConnection db = new SQLiteConnection(AppSettings.DbPath))
{
    db.RunInTransaction(() =>
    {
        var y = db.Table<MyClass>().Where(x => x.GUID == this.GUID && x.VersionNumber == this.VersionNumber).FirstOrDefault();

        bool insert = false;

        if (y == null)
        {
            y = new MyClass();
            y.GUID = this.GUID;
            insert = true;
        }

        y.VersionNumber = this.VersionNumber;
        y.WordCount = this.WordCount;
        y.Updated = DateTime.Now;
        y.Checksum = this.Checksum;
        y.Deleted = this.Deleted;
        y.Source = this.Source;

        if (insert)
        {
            db.Insert(y);
        }
        else
        {
            db.Update(y);
        }
    });
}

The only alternative I can see is a bigger if else, which looks neater but has some argument repitition

using (SQLiteConnection db = new SQLiteConnection(AppSettings.DbPath))
{
    db.RunInTransaction(() =>
    {
        var y = db.Table<PoeFileVersion>().Where(x => x.GUID == this.GUID && x.VersionNumber == this.VersionNumber).FirstOrDefault();

        if (y == null)
        {
            y = new PoeFileVersion();
            y.GUID = this.GUID;

            y.VersionNumber = this.VersionNumber;
            y.WordCount = this.WordCount;
            y.Updated = DateTime.Now;
            y.Checksum = this.Checksum;
            y.Deleted = this.Deleted;
            y.Source = this.Source;

            db.Insert(y);
        }
        else
        {
            y.VersionNumber = this.VersionNumber;
            y.WordCount = this.WordCount;
            y.Updated = DateTime.Now;
            y.Checksum = this.Checksum;
            y.Deleted = this.Deleted;
            y.Source = this.Source;

            db.Update(y);
        }
    });
}

This is not unique to SQLite of course - I've come up against the same ugly code in Linq-to-SQL. Is there a better pattern?

The second code block is the clunky one, it has repetition which the first one doesn't. If you wanted to refactor the first example you would need to go higher up and do the check for existing record before you make the call to save or update then have a method for save and one for update, but this would lead to some duplicate code.

From what you have posted I'd stick with the first one.


Scraping GitHub for security vulnerabilities I haven't done much Python programming, but I really like the language so I've been using it for side projects that I work on.  The problem with this is that I don't have the opportunity to have my peers review my code and give suggestions.

I've been working on a project that will be scraping GitHub looking for security vulnerabilities.  I've created a separate file in the project that contains all functions that interact with GitHub's API.

import requests
import re
import base64
import os

def getRepos(since=0):
  url = 'http://api.github.com/repositories'
  data = """{
    since: %s
  }""" % since
  response = requests.get(url, data=data)
  if response.status_code == 403:
    print "Problem making request!", response.status_code
    print response.headers
  matches = re.match(r'<.+?>', response.headers['Link'])
  next = matches.group(0)[1:-1]
  return response.json(), next

def getRepo(url):
  response = requests.get(url)
  return response.json()

def getReadMe(url):
  url = url + "/readme"
  response = requests.get(url)
  return response.json()

# todo: return array of all commits so we can examine each one
def getRepoSHA(url):
  # /repos/:owner/:repo/commits
  commits = requests.get(url + "/commits").json()
  return commits[0]['sha']

def getFileContent(item):
  ignoreExtensions = ['jpg']
  filename, extension = os.path.splitext(item['path'])
  if extension in ignoreExtensions:
    return []
  content = requests.get(item['url']).json()
  lines = content['content'].split('\n')
  lines = map(base64.b64decode, lines)
  print 'path', item['path']
  print 'lines', "".join(lines[:5])
  return "".join(lines)

def getRepoContents(url, sha):
  # /repos/:owner/:repo/git/trees/:sha?recursive=1
  url = url + ('/git/trees/%s?recursive=1' % sha)
  # print 'url', url
  response = requests.get(url)
  return response.json()

The code is run from here:

import github
import json

def processRepoContents(repoContents):
  # for each entry in the repo
  for tree in repoContents['tree']:
    contentType = tree['type']
    print 'contentType --- ', contentType
    # if type is "blob" get the content
    if contentType == 'blob':
      github.getFileContent(tree)
      print '***blob***'
    elif contentType == 'tree':
      print '***tree***'
      # if type is "tree" get the subtree

if __name__ == '__main__':
  repos, next = github.getRepos()
  for repo in repos[0:10]:
    # repoJson = github.getRepo(repo['url'])
    sha = github.getRepoSHA(repo['url'])
    repoJson = github.getRepoContents(repo['url'], sha)
    processRepoContents(repoJson)

I was hoping to get some feedback as to whether or not I am doing anything that would be considered not a best practice.

Also - I have all these functions in a file called github.py and I then include it by using import github wherever I need it.  I'm assuming that it would not make sense to create a class to wrap these functions, since there is never any state for the class to keep track of that the functions would require.  Does this reasoning make sense or should I wrap these functions in a class?

If anyone is interested they can see all of the code in the repo here - I would love to get feed back on the rest of the code (there isn't much more), but didn't think it would all fit in this question.

PEP8 - click, read, apply

PEP8 gives coding conventions for the Python code comprising the
standard library in the main Python distribution

you should use 4 spaces per indentation level
function names should be lowercase, with words separated by underscores as necessary to improve readability. mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility
variable names should also stick to the above rule
there should be two newlines between each method
use augmented assigments where possible (url = url + ('/git/trees/%s?recursive=1' % sha) -> url += '/git/trees/%s?recursive=1' % sha)
you're sometimes using double-quotes around strings, and sometimes single-quotes. Choose one and stick to that.
use format() instead of the old %
I'd rather suggest you use the print() function even if you're using python 2.7. It will make you code easily portable.
you're lacking docstrings. Write docstrings for all public modules, functions, classes, and methods. They are not necessary for non-public methods, but you should have a comment that describes what the method does. This comment should appear after the def line.)

Code:
import base64
import os
import re
import requests

def get_repos(since=0):
    """
    Left as an exercise for OP
    """
    url = 'http://api.github.com/repositories'
    data = '{{since: {}}}'.format(since)
    response = requests.get(url, data=data)

    if response.status_code == 403:
        print('Problem making request! {}'.format(response.status_code))
        print(response.headers)

    matches = re.match(r'<.+?>', response.headers['Link'])
    next = matches.group(0)[1:-1]

    return response.json(), next

def get_repo(url):
    """
    Left as an exercise for OP
    """
    return requests.get(url).json()

def get_readme(url):
    """
    Left as an exercise for OP
    """
    url += '/readme'
    return requests.get(url).json()

# todo: return array of all commits so we can examine each one
def get_repo_sha(url):
    """
    Left as an exercise for OP
    """
    commits = requests.get(url + '/commits').json()
    return commits[0]['sha']

def get_file_content(item):
    """
    Left as an exercise for OP
    """
    ignore_extensions = ['jpg']
    filename, extension = os.path.splitext(item['path'])

    if extension in ignore_extensions:
        return []

    content = requests.get(item['url']).json()
    lines = content['content'].split('\n')
    lines = map(base64.b64decode, lines)

    print('Path: '.format(item['path']))
    print('Lines: '.format(''.join(lines[:5])))

    return ''.join(lines)

def get_repo_contents(url, sha):
    """
    Left as an exercise for OP
    """
    url += '/git/trees/{}?recursive=1'.format(sha)

    return requests.get(url).json()

For the second .py file:

the above rules apply
don't import modules you're not using (import json)

Code
import github

def process_repo_contents(repo_contents):
    """
    Left as an exercise for OP
    """
    for tree in repo_contents['tree']:
        content_type = tree['type']
        print('content_type --- {}'.format(content_type))

        if content_type == 'blob':
            github.get_file_content(tree)
            print('***blob***')
        elif content_type == 'tree':
            print('***tree***')

if __name__ == '__main__':
    repos, next = github.get_repos()
    for repo in repos[0:10]:
        sha = github.get_repo_sha(repo['url'])
        repo_json = github.get_repo_contents(repo['url'], sha)
        process_repo_contents(repo_json)

Pay more attention to variable names and data types

next is a builtin keyword so I'd recommend you to change it to something else
ignore_extensions is a list in your case. And it only keeps one string. Don't you think it would be more appropriate to make it a string from the beginning ?


Path finder for network link wiring I have created a Path finder.
The aim is to find ALL paths that join a Point A to a Point B.
I have defined an object Segment which is a link between two assets.
It is readable in both directions.
==> Segment(A->B) === Segment(B<-A)

Here is my code:
# Exemple Path finder

class Segment:
  # A segment is a link (physical, radio, logical) beetween two assets
  def __init__(self, sku, e1, e2, re1, re2):
    # The SKU is the unique ID of the segment
    # A segment as two endpoint that can be inversable
    self.sku = sku
    self.sku_endpoint_1 = e1
    self.sku_endpoint_2 = e2
    self.reverse_sku_endpoint_1 = re1
    self.reverse_sku_endpoint_2 = re2

# We can instanciante a segment like that

seg1 = Segment("SEGMENT1","A", "B", "B", "A")
seg2 = Segment("SEGMENT2","B", "C", "C", "B")
seg3 = Segment("SEGMENT3","C", "D", "D", "C")
seg4 = Segment("SEGMENT4","C", "E", "E", "C")
seg5 = Segment("SEGMENT5","E", "A", "A", "E")
seg6 = Segment("SEGMENT6","E", "D", "D", "E")

# Then create a liste of segment
liste_segments = [seg1, seg2, seg3, seg4, seg5, seg6]

--
# The function below allow to find neibhors of a segment
def neighbors(liste_segments, node):
  for segment in liste_segments:
    if node == segment.sku_endpoint_1:
      yield segment

--
# And the function below solve all paths existing beetween an start et end.
def solve(liste_segments, s, e, current_path=None, nb_call=0):
  nb_call += 1
  if nb_call == 1:
    # We create a new liste with the inversable values to have all possibility of linking
    liste_segments = clone(liste_segments)
  if current_path is None:
    current_path = []
  if s == e:
    # Here, we end the function because we found the end.
    yield current_path
  else:
    # Else we call recursively the solve function on each neibhors
    for node in neighbors(liste_segments, s):
      print(node.sku_endpoint_1, node.sku_endpoint_2)
      if node not in current_path:
        for result in solve(liste_segments, node.sku_endpoint_2 , e, current_path + [node], nb_call):
          yield result

--
# The yield function return a generator
# The function here create a dictionnary of all paths found.
# On all paths we check if the paths is meaningful.
# Ex: if a segment is found twice on the same path...this is not meaningful.
# So we delete the entry from the dict to keep only understanding paths.
def list_to_dict(paths):
  d = {}
  index = 0
  for path in paths:
    index+=1
    d[index] = []
    for segment in path:
      d[index].append(segment.sku)

  to_del = []
  for k, v in d.items():
    temp = []
    for seg in v:
      if seg not in temp:
        temp.append(seg)
      else:
        if k not in to_del:
          to_del.append(k)
  for i in to_del:
    del d[i]
  return d

--
# The clone function create a new list of segment with all possibility.

def clone(liste_segments):
  new_list_segments = []
  for seg in liste_segments:
    new_list_segments.append(Segment(seg.sku, seg.sku_endpoint_1, seg.sku_endpoint_2, "", ""))
    new_list_segments.append(Segment(seg.sku, seg.reverse_sku_endpoint_1, seg.reverse_sku_endpoint_2, "", ""))
  return new_list_segments

Is my code pythonic? What could be changed?

If you want to work with graphs in Python, you should have a look at the excellent module networkx. It has (usually fast) implementations of the most common (and many not so common) operations on directed and undirected graphs.
Your case in particular, finding all simple paths (i.e. paths without repeating nodes) between two nodes, is already implemented:
import networkx as nx

g = nx.Graph()  # bi-directional edges. Use nx.DiGraph() if you want to explicitly specify directions
g.add_edges_from([("A", "B"), ("B", "C"), ("C", "D"), ("C", "E"), ("E", "A")])
g.add_edge("E", "D")  # can also add individual edges

for path in nx.all_simple_paths(g, source="A", target="B"):
    print(path)

# ['A', 'B']
# ['A', 'E', 'C', 'B']
# ['A', 'E', 'D', 'C', 'B']

If you want to associate data with each edge and get back only this data, this is also possible:
g = nx.Graph()  # bi-directional edges. Use nx.DiGraph() if you want to explicitly specify directions
g.add_edges_from([("A", "B", {"sku": "SEGMENT1"}),
                  ("B", "C", {"sku": "SEGMENT2"}),
                  ("C", "D", {"sku": "SEGMENT3"}),
                  ("C", "E", {"sku": "SEGMENT4"}),
                  ("E", "A", {"sku": "SEGMENT5"})])
g.add_edge("E", "D", sku="SEGMENT6")  # can also add individual edges

paths = nx.all_simple_paths(g, source="A", target="B")
for path in map(nx.utils.pairwise, paths):
    for edge in path:
        print(edge, g.edges[edge]["sku"])
    print()

# ('A', 'B') SEGMENT1
#
# ('A', 'E') SEGMENT5
# ('E', 'C') SEGMENT4
# ('C', 'B') SEGMENT2
#
# ('A', 'E') SEGMENT5
# ('E', 'D') SEGMENT6
# ('D', 'C') SEGMENT3
# ('C', 'B') SEGMENT2

It even allows you to easily visualize the graph:
import matplotlib.pyplot as plt

nx.draw(g, with_labels=True, node_color="white")
plt.show()

Or, more fancy:
pos = nx.spring_layout(g)
labels = dict(zip(g.edges, [g.edges[e]["sku"] for e in g.edges]))
nx.draw(g, with_labels=True, node_color="white", pos=pos)
nx.draw_networkx_edge_labels(g, pos=pos, edge_labels=labels)
plt.show()


Creating chat commands properly with Socket.IO I am developing a chat program (mountreus-chat and GitHub) in Node.js using Socket.io and my code looks awful. I'm starting to use commands and now it's even worse.

Here's a code snippet (you can find every detail on the Github repo):

io.on('connection', function(socket){
      if(socketConnections().length <= 1024){
        socket.username = socket.handshake.query.username;
        socket.toString = function(){return this.name};
        socket.join(socket.handshake.query.room);
        socket.on('postName', function(username){
                socket.username = username;
                });
        var socketsConnected = socketConnections(socket.handshake.query.room, "/");
        io.emit('connections', socketsConnected.length + 1);
        socket.on('chat message', function(msg){
            var markedMessage =  markdown.renderInline(msg.message);
            var messageDate = moment(msg.date).format("LT, D/M");
            var firstWord = markedMessage.substr(0, markedMessage.indexOf(" "));
            if(msg.message !== "/help"){
            if(firstWord === "/broadcast"){
            var finalMessage = markedMessage.substr(markedMessage.indexOf(" ") + 1);
             var messageToBeSent = '
 BROADCAST: ' + finalMessage + '

' + messageDate + '
';
            }else if(firstWord === "/bot-say"){
                var finalMessage = markedMessage.substr(markedMessage.indexOf(" ") + 1);
                var messageToBeSent = '
 Chat bot: ' + finalMessage + '

' + messageDate + '
';
            }else{
            var messageToBeSent = '
' + escapeHTML(msg.username) + ': ' + markedMessage + '

' + messageDate + '
';
            }
            if(messageToBeSent.length <= 8192){
                  if(!verifyEmptyness(msg.message)){
                    io.to(socket.handshake.query.room).emit('chat message', messageToBeSent);
                  }else{
                  socket.emit('chat message', 'PM: Sorry, you cannot send empty messages.');
                  }
                }else{
                  socket.emit('chat message', 'PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
                }
            }else{
                socket.emit('chat message', 'Montreus Chat - v1.3.3
Available commands:
/help - Display help commands
/bot-say &lt;message&gt; - Give something for the bot to say!
/broadcast &lt;message&gt; - Broadcast a message');
            }
        });
      socket.on('users', function(){
                var socketsConnected = socketConnections(socket.handshake.query.room, "/");
                io.to(socket.handshake.query.room).emit('connections', socketsConnected.length + 1);
                });
      socket.on('disconnect', function(){
                var socketsConnected = socketConnections(socket.handshake.query.room, "/");
                io.to(socket.handshake.query.room).emit('connections', socketsConnected.length + 1);
                });
      }else{
      socket.emit('chat message', 'PM: Sorry, we cannot allow more than 1024 connections in the server');
      socket.emit('chat message', 'PM: Disconnecting! Try again later.');
      socket.emit('connections', 'You are not connected.');
      socket.disconnect();
      }
      });

You can gain a lot of readability by formatting the code (every decent editor will do a passable job at this with a single keystroke) and extracting several small functions (refactoring). I'll start with this but only extract the reusable functions.

  Note: I assumed you can emit the initial connection count after binding the commands. If not, move the first call to emitConnectionCount inside setupSocket.

io.on('connection', function (socket) {
    if (socketConnections().length <= 1024) {
        setupSocket();
        emitConnectionCount();
    } else {
        disconnect();
    }

    function setupSocket() {
        socket.username = socket.handshake.query.username;
        socket.toString = function () {
            return this.name
        };
        socket.join(socket.handshake.query.room);
        socket.on('postName', handleUsername);
        socket.on('chat message', handleMessage);
        socket.on('users', emitConnectionCount);
        socket.on('disconnect', emitConnectionCount);
    }

    function disconnect() {
        emitToUser('PM: Sorry, we cannot allow more than 1024 connections in the server');
        emitToUser('PM: Disconnecting! Try again later.');
        emitToUser('You are not connected.', 'connections');
        socket.disconnect();
    }

    function handleUsername(username) {
        socket.username = username;
    }

    function handleMessage(msg) {
        var markedMessage = markdown.renderInline(msg.message);
        var messageDate = moment(msg.date).format("LT, D/M");
        var firstWord = markedMessage.substr(0, markedMessage.indexOf(" "));
        var messageToBeSent;
        var finalMessage;
        if (msg.message === "/help") {
            emitToUser('Montreus Chat - v1.3.3
Available commands:
/help - Display help commands
/bot-say &lt;message&gt; - Give something for the bot to say!
/broadcast &lt;message&gt; - Broadcast a message');
        } else if (firstWord === "/broadcast") {
            finalMessage = markedMessage.substr(markedMessage.indexOf(" ") + 1);
            messageToBeSent = '
 BROADCAST: ' + finalMessage + '

' + messageDate + '
';
        } else if (firstWord === "/bot-say") {
            finalMessage = markedMessage.substr(markedMessage.indexOf(" ") + 1);
            messageToBeSent = '
 Chat bot: ' + finalMessage + '

' + messageDate + '
';
        } else {
            messageToBeSent = '
' + escapeHTML(msg.username) + ': ' + markedMessage + '

' + messageDate + '
';
        }
        if (messageToBeSent.length <= 8192) {
            if (!verifyEmptyness(msg.message)) {
                emitToRoom(messageToBeSent);
            } else {
                emitToUser('PM: Sorry, you cannot send empty messages.');
            }
        } else {
            emitToUser('PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
        }
    }

    function emitToRoom(message, command) {
        io.to(socket.handshake.query.room).emit(command || 'chat message', message);
    }

    function emitToUser(message, command) {
        socket.emit(command || 'chat message', message);
    }

    function emitConnectionCount() {
        var socketsConnected = socketConnections(socket.handshake.query.room, "/");
        emitToRoom(socketsConnected.length + 1, 'connections');
    }
});

That's much easier to grok already, but let's tackle that complicated handleMessage function next. What are its responsibilities?

Parse the message to extract an optional command (/help, /broadcast, and /bot-say).
Handle the help command.
Fail if the message is empty.
Format the message using Markdown.
Embed it into an HTML snippet.
Fail if the formatted message is too long.
Emit the formatted message.

Wow, that's a lot of work--too much for a single function. Let's simplify it by extracting functions for emitting the help text and formatting the message.

function handleMessage(chat) {
    var message = chat.message;
    if (message === "/help") {
        handleHelp();
    } else {
        var split = message.indexOf(" ");
        var command;
        if (split === -1) {
            command = '/chat';
        } else {
            command = message.substr(0, split);
            message = message.substr(split);
        }
        if (verifyEmptyness(message)) {
            emitToUser('PM: Sorry, you cannot send empty messages.');
        } else {
            var realMessage = formatMessage(command, message, chat.username, moment(chat.date).format("LT, D/M"));
            if (realMessage.length > 8192) {
                emitToUser('PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
            } else {
                emitToRoom(realMessage);
            }
        }
    }
}

function formatMessage(command, message, user, date) {
    var intro;
    switch (command) {
        case '/broadcast':
            intro = 'BROADCAST';
            break;
        case '/bot-say':
            intro = 'Chat bot';
            break;
        case '/chat':
            intro = escapeHTML(user);
            break;
        default:
            intro = 'UNKNOWN COMMAND';
            message = command;
            break;
    }
    return '
 ' + intro + ': ' + message + '

' + date + '
';
}

function handleHelp() {
    sendToUser(
        'Montreus Chat - v1.3.3'
        + '
Available commands:'
        + '
/help - Display help commands'
        + '
/bot-say &lt;message&gt; - Give something for the bot to say!'
        + '
/broadcast &lt;message&gt; - Broadcast a message'
    );
}

That's better, but handleMessage is still doing too much. I don't like that it has to parse the original message, check for an empty chat message, and send the formatted message if it's not too long. Let's extract the parsing first.

function handleMessage(chat) {
    if (chat.message === '/help') {
        handleHelp();
    } else {
        var parsed = parseMessage(chat.message);
        if (verifyEmptyness(parsed.message)) {
            emitToUser('PM: Sorry, you cannot send empty messages.');
        } else {
            var message = formatMessage(parsed.command, parsed.message, chat.username, moment(chat.date).format("LT, D/M"));
            if (message.length > 8192) {
                emitToUser('PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
            } else {
                emitToRoom(chat, command, message);
            }
        }
    }
}

function parseMessage(message) {
    var split = message.indexOf(' ');
    if (split === -1 || message.charAt(0) !== '/') {
        return {command: '/chat', message: message};
    } else {
        return {command: message.substr(0, split), message: message.substr(split)};
    }
}

If you don't mind formatting an empty message that won't be sent, you can extract the format/check/send logic to yet another function.

function handleMessage(chat) {
    if (chat.message === '/help') {
        handleHelp();
    } else {
        handleChat(chat);
    }
}

function handleChat(chat) {
    var parsed = parseMessage(chat.message);
    var formatted = formatMessage(parsed.command, parsed.message, chat.username, moment(chat.date).format("LT, D/M"));
    if (verifyEmptyness(parsed.message)) {
        emitToUser('PM: Sorry, you cannot send empty messages.');
    } else if (formatted.length > 8192) {
        emitToUser('PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
    } else {
        emitToRoom(chat, command, formatted);
    }
}

Here's the final product (after a little reorganization) for ease of understanding and discussion:

io.on('connection', function (socket) {
    if (socketConnections().length > 1024) {
        disconnect();
    } else {
        setupSocket();
        emitConnectionCount();
    }

    function disconnect() {
        emitToUser('PM: Sorry, we cannot allow more than 1024 connections in the server');
        emitToUser('PM: Disconnecting! Try again later.');
        emitToUser('You are not connected.', 'connections');
        socket.disconnect();
    }

    function setupSocket() {
        socket.username = socket.handshake.query.username;
        socket.toString = function () {
            return this.name
        };
        socket.join(socket.handshake.query.room);
        socket.on('postName', handleUsername);
        socket.on('chat message', handleMessage);
        socket.on('users', emitConnectionCount);
        socket.on('disconnect', emitConnectionCount);
    }

    function handleUsername(username) {
        socket.username = username;
    }

    function handleMessage(chat) {
        if (chat.message === '/help') {
            handleHelp();
        } else {
            handleChat(chat);
        }
    }

    function handleHelp() {
        sendToUser(
            'Montreus Chat - v1.3.3'
            + '
Available commands:'
            + '
/help - Display help commands'
            + '
/bot-say &lt;message&gt; - Give something for the bot to say!'
            + '
/broadcast &lt;message&gt; - Broadcast a message'
        );
    }

    function handleChat(chat) {
        var parsed = parseMessage(chat.message);
        var formatted = formatMessage(parsed.command, parsed.message, chat.username, moment(chat.date).format("LT, D/M"));
        if (verifyEmptyness(parsed.message)) {
            emitToUser('PM: Sorry, you cannot send empty messages.');
        } else if (formatted.length > 8192) {
            emitToUser('PM: Oops! You cannot send messages longer than 8192 characters. Sorry!');
        } else {
            emitToRoom(chat, command, formatted);
        }
    }

    function parseMessage(message) {
        var split = message.indexOf(' ');
        if (split === -1 || message.charAt(0) !== '/') {
            return {command: '/chat', message: message};
        } else {
            return {command: message.substr(0, split), message: message.substr(split)};
        }
    }

    function formatMessage(command, message, user, date) {
        var intro;
        switch (command) {
            case '/broadcast':
                intro = 'BROADCAST';
                break;
            case '/bot-say':
                intro = 'Chat bot';
                break;
            case '/chat':
                intro = escapeHTML(user);
                break;
            default:
                intro = 'UNKNOWN COMMAND';
                message = command;
                break;
        }
        return '
 ' + intro + ': ' + message + '

' + date + '
';
    }

    function emitConnectionCount() {
        emitToRoom(socketConnections(socket.handshake.query.room, "/").length + 1, 'connections');
    }

    function emitToRoom(message, command) {
        io.to(socket.handshake.query.room).emit(command || 'chat message', message);
    }

    function emitToUser(message, command) {
        socket.emit(command || 'chat message', message);
    }
});

The main benefits of this version over the original are

Short functions with descriptive names make it easier to understand the program in small pieces and verify its correctness.
Repeated code has been deduplicated with reusable functions with sane defaults.
Separating responsibilities into functions allows each function to live at the same abstraction level. For example, handleMessage is only concerned with detecting /help or passing off to handleChat while the latter is only concerned with checking for a valid message. Likewise for parseMessage and formatMessage: they are both at even lower abstraction levels.
Splitting the functions also allowed reusing variable names across functions to avoid wordy repetition such as markedMessage, finalMessage, and messageToBeSent.

Here are some suggested improvements:

Add JSDoc documentation to each function that requires further explanation, especially those that take parameters. It would also be good to document the chat object passed to handleMessage.
Find better names for handleChat and handleMessage. I would reserve handle for functions bound to commands, though handleHelp does make sense. Naming can be tough at times, but a good name goes a long way. Don't be afraid of longer names like formatAndSendMessage to remove confusion.
Consider extracting sendMessage(original, formatted) from handleChat so the latter doesn't do any parsing, formatting, length-checking, or sending but merely ties those all together.


Emulating "DROP USER IF EXISTS" I wrote this MySQL stored procedure to emulate DROP USER IF EXISTS (not available in MySQL). Any comments or identified bugs?

DROP PROCEDURE IF EXISTS DropUserIfExists;
DELIMITER $$
    CREATE PROCEDURE DropUserIfExists(MyUserName VARCHAR(100))
    BEGIN
      DECLARE foo BIGINT DEFAULT 0 ;
      SELECT COUNT(*)
      INTO foo
        FROM mysql.user
          WHERE User = MyUserName ;
       IF foo > 0 THEN
             SET @A = (SELECT Result FROM (SELECT GROUP_CONCAT("DROP USER"," ",MyUserName,"@'%'") AS Result) AS Q LIMIT 1);
             PREPARE STMT FROM @A;
             EXECUTE STMT;
             FLUSH PRIVILEGES;
       END IF;
    END ;$$
DELIMITER ;

I would say you are wrong!!!

Let me explain...

If there is a user named 'tempuser'@'localhost' and you are trying to call DropUserIfExists('tempuser'), the above procedure may produce error something like:

Error Code: 1396 Operation DROP USER failed for 'tempuser'@'%', since there is no user 'tempuser'@'%'

A workaround for this problem is to pass both username and host name to the SP (pass a null host name if you want to delete all users named tempuser)

The following SP will help you to remove user 'tempuser'@'%' by executing CALL DropUserIfExistsAdvanced('tempuser', '%');

If you want to remove all users (say 'tempuser'@'%', 'tempuser'@'localhost' and 'tempuser'@'192.168.1.101') execute SP like CALL DropUserIfExistsAdvanced('tempuser', NULL); This will delete all users named tempuser!!! seriously...

Now please have a look on mentioned SP DropUserIfExistsAdvanced:

DELIMITER $$

DROP PROCEDURE IF EXISTS `DropUserIfExistsAdvanced`$$

CREATE DEFINER=`root`@`localhost` PROCEDURE `DropUserIfExistsAdvanced`(
    MyUserName VARCHAR(100)
    , MyHostName VARCHAR(100)
)
BEGIN
DECLARE pDone INT DEFAULT 0;
DECLARE mUser VARCHAR(100);
DECLARE mHost VARCHAR(100);
DECLARE recUserCursor CURSOR FOR
    SELECT `User`, `Host` FROM `mysql`.`user` WHERE `User` = MyUserName;
DECLARE CONTINUE HANDLER FOR NOT FOUND SET pDone = 1;

IF (MyHostName IS NOT NULL) THEN
    -- 'username'@'hostname' exists
    IF (EXISTS(SELECT NULL FROM `mysql`.`user` WHERE `User` = MyUserName AND `Host` = MyHostName)) THEN
        SET @SQL = (SELECT mResult FROM (SELECT GROUP_CONCAT("DROP USER ", "'", MyUserName, "'@'", MyHostName, "'") AS mResult) AS Q LIMIT 1);
        PREPARE STMT FROM @SQL;
        EXECUTE STMT;
        DEALLOCATE PREPARE STMT;
    END IF;
ELSE
    -- check whether MyUserName exists (MyUserName@'%' , MyUserName@'localhost' etc)
    OPEN recUserCursor;
    REPEAT
        FETCH recUserCursor INTO mUser, mHost;
        IF NOT pDone THEN
            SET @SQL = (SELECT mResult FROM (SELECT GROUP_CONCAT("DROP USER ", "'", mUser, "'@'", mHost, "'") AS mResult) AS Q LIMIT 1);
            PREPARE STMT FROM @SQL;
            EXECUTE STMT;
            DEALLOCATE PREPARE STMT;
        END IF;
    UNTIL pDone END REPEAT;
END IF;
FLUSH PRIVILEGES;
END$$

DELIMITER ;


Fluent interface and polymorphism for building a scene with shapes I would like to improve the interfaces of some polymorphic classes going from positional to named parameters and I came up with the fluent interface.

The following is the most clean, compact and compilable (-std=c++11 required) example that I have been able to come up with:

#include <iostream>
#include <cmath>
#include <vector>
#include <string>

using namespace std;

struct Figure {
  string _name;
  Figure * name(const string & str) { _name=str; return this; }
  virtual double area() const=0;
};

struct Circle: Figure {
  double _radius;
  Circle * radius(double r) { _radius=r; return this;}
  double area() const override {return M_PI*_radius*_radius;}
};

struct Square: Figure {
  double _side;
  Square * side(double s) { _side=s; return this;}
  double area() const override {return _side*_side;}
};

struct Scene {
  vector<Figure*> v;
  ~Scene() { for (auto & f : v) delete f; }
  Scene & add(Figure * f) {v.push_back(f); return *this;}
  double total_area() const {
    double total=0;
    for (auto f : v) total += f->area();
    return total;
  }
};

int main() {
  Scene scene;
  scene.add((new Circle)->radius(1.)->name("c1"))
       .add((new Square)->  side(1.)->name("s1"));
  cout << "Total area: " << scene.total_area() << endl;
  return 0;
}

I have a couple of issues with this:

That is an ugly place to have a new operator, could it be avoided somehow?
After having called the method name("name") the type is lost so there is still an ordering to respect: add((new Square)->name("s1")->side(1.)) will not compile. You should imagine to have many levels of inheritance and lots of parameters to be setted!

How would you address these issues and improve the code? C++1y is allowed and preferred to Boost?

For further improvements see the related question: Variadic templates and pointers to member functions to achieve a named-parameters interface in C++

Another thing to consider in your current design:

~Scene() { for (auto & f : v) delete f; }

You are calling the destructor of the base here, not the derived classes' destructors. Remember to make a virtual destructor in your class to solve this problem.


Helper method to check if 3 numbers match some of the valid results I have this helper method to check if 3 numbers match some of the valid results (XXX+bool, XXX, -XX, X-X, XX-, --X).

const categoryNumber = (numbers, myBool, resultNumbers) => {
  const matches = numbers.map((val, index) => val === resultNumbers[index]);
  if (matches[0] && matches[1] && matches[2] && myBool) return 1;
  if (matches[0] && matches[1] && matches[2]) return 2;
  if (matches[0] && matches[1]) return 3;
  if (matches[1] && matches[2]) return 4;
  if (matches[0] && matches[2]) return 5;
  if (matches[2]) return 6;
  return 0;
};

Type vars?:

numbers:       numeric array size 3
myBool:        boolean
resultNumbers: numeric array size 3

You could use a look up table, as follows:

var obj = { 'XXX': 2, '-XX': 3, 'X-X': 4, 'XX-': 5, '--X': 6}
var match = ""+ matches[0] + matches[1] + matches[2];

and then

if(match === 'XXX' && myBool){ return 1; } else { return obj[match]; }

This assumes that each position in your matches will be either an X or a -.


concatenate string, with different first and last iteration Good day. My code works but I think there is a better way to do this. Like string pattern or string manipulation. I'm not yet familiar with both terms.
The goal is to get "=A1-A2-B3-D4-WHATEVER STRING=". From array of strings.
The code is :
string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING"}; // can be any string value
string newString = "=";
for (int i = 0; i < arr.Length; i++) {
    if (i == arr.Length-1) {
        newString  += arr[i].ToString() + "=";
    } else {
        newString += arr[i].ToString() + "-";
    }
}

You should use String.Join() here. Its easier to read and shorter as well.
Like
string[] arr = { "A1", "A2", "B3", "D4", "WHATEVER STRING" }; // can be any string value
string result = "=" + string.Join("-", arr) + "=";


Lambda to collect properties in a child object I have a class:

public class ProductInvoice {
     //Some other properties.
    private List<ProductEntry> productEntries = newArrayList();
//otherMethods
}

And:

public class ProductEntry {
//other properties
    private Long quantity;
    private Long productId;
}

Now  I need to prepare a map of productId vs quantity by iterating over a collection of ProductInvoice objects:

List<ProductInvoice> entries = productInvoiceRepository.findByProductEntries_ProductIdIn(productIds);
        Map<Long, Long> productIdVsQuantity = newHashMap();
        entries.forEach( pi -> pi.getProductEntries().forEach(pe -> {
            if (productIdVsQuantity.containsKey(pe.getProductId())) {
                productIdVsQuantity.put(pe.getProductId(), productIdVsQuantity.get(pe.getProductId()) + pe.getQuantity());
            } else {
                productIdVsQuantity.put(pe.getProductId(), pe.getQuantity());
            }
        }));

Could there be a better way of doing this? I think this can be optimized using better lambda usage.

First, you can use Stream.flatMap() to convert a Stream of ProductInvoice to ProductEntries.

Next, you just need suitable Collector implementations to sum all the quantities per product ID, and group by that...

Putting it all together:

Map<Long, Long> result = productInvoiceRepository
                            .findByProductEntries_ProductIdIn(productIds)
                            .stream().flatMap(v -> v.getProductEntries().stream())
                            .collect(Collectors.groupingBy(ProductEntry::getProductId,
                                    Collectors.summingLong(ProductEntry::getQuantity)));

BTW, you may want to consider a better method name for findByProductEntries_ProductIdIn() to remove the underscore...


FizzBuzz vs. FortRan In keeping with using FizzBuzz as an introduction to a language, I present FizzBuzz with a Fortran95 twist:

For multiples of 3, print Fort, and for 5's multiples print Ran. For multiples of both, print FortRan.

Looking especially for style and common practice criticisms. A hint on how to left-justify the values in a standard way would be appreciated too.

The following code is presented in Ideone too:

program fizzbuzz
implicit none
integer ::i

! Loop through integers 1 through 100
! Multiples of 3 print Fort
! Multiples of 5 print Ran
! Multiples of both print FortRan
! 15 is the lowest common multiple of 3, and 5
!    and thus is a shortcut to FortRan

do i=1,100
    if (MODULO(i, 15) == 0) then
        write (*,'(A8)') 'FortRan'
    else if (MODULO(i, 3) == 0) then
        write (*,'(A8)') 'Fort'
    else if (MODULO(i, 5) == 0) then
        write (*,'(A8)') 'Ran'
    else
        write (*,'(I8)') i
    end if
end do

end program fizzbuzz

I find MODULO() to be jarring, when everything else is written in lowercase.  Be consistent.

To left-justify strings in the output, write it using a * format.  Since you want to write to standard output, you can simplify it by using print instead.

To left-justify numbers in the output, first convert it to a string, then strip leading spaces.

program fizzbuzz
implicit none
integer ::i
character(len=8) :: str

! Loop through integers 1 through 100
! Multiples of 3 print Fort
! Multiples of 5 print Ran
! Multiples of both print FortRan
! 15 is the lowest common multiple of 3, and 5
!    and thus is a shortcut to FortRan

do i=1,100
    if (modulo(i, 15) == 0) then
        print *, 'FortRan'
    else if (modulo(i, 3) == 0) then
        print *, 'Fort'
    else if (modulo(i, 5) == 0) then
        print *, 'Ran'
    else
        write (str, '(i8)') i
        str = adjustl(str)
        print *, str
    end if
end do

end program fizzbuzz


Classifying the user into an age bracket In this program, I have to check the range of a given age and then print a message as per the age. I want to optimize the if else  logic.

/* Sample code to read in test cases:*/

import java.io.*;
public class Main {
 public static void main (String[] args) throws IOException {
    File file = new File(args[0]);
    BufferedReader buffer = new BufferedReader(new FileReader(file));
    String line;
    while ((line = buffer.readLine()) != null) {
        line = line.trim();
        // Process line of input Here

        int age = Integer.parseInt(line);

          if(age>=0 && age<=2){
              System.out.println("Still in Mama's arms");
             // break;
          }else if(age>=3 && age<=4){
               System.out.println("Preschool Maniac");
              // break;
          }else if(age>=5 && age<=11){
               System.out.println("Elementary school");
             // break;
          }else if(age>=12 && age<=14){
               System.out.println("Middle school");
             // break;
          }else if(age>=15 && age<=18){
               System.out.println("High school");
             // break;
          }else if(age>=19 && age<=22){
               System.out.println("College");
             // break;
          }else if(age>=23 && age<=65){
               System.out.println("Working for the man");
             // break;
          }else if(age>=66 && age<=100){
               System.out.println("The Golden Years");
              //break;
          }else{
               System.out.println("This program is for humans");

          }
         }
      }
    }

Is there any other way to do this program? Using a switch, it becomes more complex to type with so many cases for the age.

A TreeMap<Integer, String> with a for loop and stop on first number greater than or equaling the number to find will solve your problem.

treemap.put(-1, "This program is for humans");
treemap.put(2, "Still in Mama's arms");
//etc...
treemap.put(Integer.MAX_VALUE, "This program is for humans");

Then, loop through entries...

for(Map.Entry<Integer, String> entry : treemap.entrySet()){
    if(entry.getKey() >= age){
        System.out.println(entry.getValue());
        break;
    }
}

The default behaviour for a TreeMap is that the contents are sorted based on the key, which explains why this works. Use Integer.MAX_VALUE for the last case - it'll be equal to or greater than any other number.

You can even use a built-in function for this:

System.out.println(treemap.ceilingEntry(age).getValue());

Making the final code...

import java.io.*;
import java.util.TreeMap;

public class Main {
    private static TreeMap<Integer, String> treemap = new TreeMap<>();

    private static void initializeAgeDescriptions() {
        treemap.put(-1, "This program is for humans");
        treemap.put(2, "Still in Mama's arms");
        treemap.put(4, "Preschool Maniac");
        treemap.put(11, "Elementary school");
        treemap.put(14, "Middle school");
        treemap.put(18, "High school");
        treemap.put(22, "College");
        treemap.put(65, "Working for the man");
        treemap.put(100, "The Golden Years");
        treemap.put(Integer.MAX_VALUE, "This program is for humans");
    }

    public static void main (String[] args) throws IOException {
        initializeAgeDescriptions();
        File file = new File(args[0]);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        String line;
        while ((line = buffer.readLine()) != null) {
            line = line.trim();

            int age = Integer.parseInt(line);
            System.out.println(treemap.ceilingEntry(age).getValue());
        }
    }
}


Count number of occurrences of each word of text, display as circles of varying size My focus was to have clean, structured code. It has to be efficient. The display doesn't have to look that good. The circles do overlap and sometimes the word doesn't fit inside the circle, but those are problems aren't really my concern right now and I will fix later.

circle.py

import pygame
import random

# Define the colors and screen sizes, define center

# Colors
black = (0, 0, 0)
blue = (0, 128, 255)
red = (255, 102, 102)
green = (102, 255, 178)
purple = (178, 102, 255)
yellow = (255, 255, 102)
colors = [blue, red, green, purple, yellow]
white = (255, 255, 255)

# Radius and font size
radius = [30, 40, 50, 60, 70, 80, 90, 100]

# Screen dimensions and title
SCREEN_WIDTH, SCREEN_HEIGHT = 640, 480
CENTER = (SCREEN_WIDTH/2, SCREEN_HEIGHT/2)
TITLE = "Occurences"

# Actual screen
screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))

# Initialize screen
pygame.display.set_caption(TITLE)
screen.fill(white)

class Circle:
    def __init__(self, size, text):
        rand_color = random.randint(0, 4)
        rand_x = random.randint(size+5, SCREEN_WIDTH-size)
        rand_y = random.randint(size+5, SCREEN_HEIGHT-size)

        self.x = rand_x
        self.y = rand_y
        self.size = size
        self.color = colors[rand_color]
        self.screen = screen
        self.text = text

    def display(self):
        pygame.draw.circle(self.screen, self.color, (self.x, self.y), self.size)

        myfont = pygame.font.SysFont("monospace", self.size-15, bold=True)
        label = myfont.render(self.text, 1, black)
        screen.blit(label, (self.x-self.size+10, self.y-20))

occurences.py

import pygame
from pygame.locals import *
import circle
import time
import re
from operator import itemgetter
from collections import Counter

# Intro
pygame.init()

intro_text = ["Hi!", "Welcome to 'occurences'!", "A program (with a misspelt name) that counts the number",
        "of occurrences in a piece of text", "and displays circles with sizes based on", "the number of occurrences of each word."]

for sentence in intro_text:
    print sentence
    time.sleep(2)

text =  raw_input("Enter some text:\n") # Prompt the user for text

# Action!!!

#word_count = Counter(words) # Count them
#sorted_count = word_count.most_common() # Sort the count
#sorted_copy = sorted_count[:] # A copy of the sorted count, just in case

# Here's a one liner for the code above.
words = Counter(re.findall(r"[\w']+", text.lower())).most_common() # Split words

while words: # While the list of words isn't empty,
    max_occ = max(words, key=itemgetter(1))[1] # Take current max occurrence of the list

    for word in words:

        if word[1] == max_occ: # If the current word's occurrence is same as the max occurrence
            curr_word = word

            if len(circle.radius) != 1:
                c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
                c.display() # Displaying the word on the circle
                circle.radius.pop() # Remove that current largest size
            else:
                c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
                c.display()

            words = [x for x in words if x != curr_word] # Change the list so that the words already dealt with are gone, making a new max_occ

pygame.display.flip()

# Event loop
running = 1
while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = 0

pygame.display.update()

Needless work to get in sorted order.

I'd like to take a look at the following section:

words = Counter(re.findall(r"[\w']+", text.lower())).most_common() # Split words

while words: # While the list of words isn't empty,
    max_occ = max(words, key=itemgetter(1))[1] # Take current max occurrence of the list

    for word in words:

        if word[1] == max_occ: # If the current word's occurrence is same as the max occurrence
            curr_word = word

            if len(circle.radius) != 1:
                c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
                c.display() # Displaying the word on the circle
                circle.radius.pop() # Remove that current largest size
            else:
                c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
                c.display()

            words = [x for x in words if x != curr_word] # Change the list so that the words already dealt with are gone, making a new max_occ

First, let's look at the help of most_common:

most_common(self, n=None) unbound collections.Counter method
    List the n most common elements and their counts from the most
    common to the least.  If n is None, then list all element counts.

So, they are in exactly the order you want. Thus, we can replace some code

words = Counter(re.findall(r"[\w']+", text.lower())).most_common()
for word in words:
    if len(circle.radius) != 1:
        c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
        c.display() # Displaying the word on the circle
        circle.radius.pop() # Remove that current largest size
    else:
        c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
        c.display()

Notice how the while and if are now both gone? All because of how .most_common already guarantees they are ordered by max-occurence.

Code duplication in getting the radius

Let's take a look at the inner if:

if len(circle.radius) != 1:
    c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
    c.display() # Displaying the word on the circle
    circle.radius.pop() # Remove that current largest size
else:
    c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
    c.display()

I'm going to do something sneaky: I see how the circle.radius.pop() and c.display() are unrelated, and that we can swap them.

if len(circle.radius) != 1:
    c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
    circle.radius.pop() # Remove that current largest size
    c.display() # Displaying the word on the circle
else:
    c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
    c.display()

Now, I see how both the branches of the if end with the same: the c.display() (and a comment which I'll ignore ;) ).

if len(circle.radius) != 1:
    c = circle.Circle(circle.radius[-1], word[0]) # Draw a circle with the current largest size of circle.radius
    circle.radius.pop() # Remove that current largest size
else:
    c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
c.display()

Another thing I'd like to note. circle.radius.pop() actually returns a value. What value? The last value in the list, that is circle.radius[-1]. Let's inline it.

if len(circle.radius) != 1:
    c = circle.Circle(circle.radius.pop(), word[0]) # Draw a circle with the current largest size of circle.radius
else:
    c = circle.Circle(30, word[0]) # When there's only one size left (the smallest one), make all other words that size.
c.display()

Ooh, see how they now have almost the same body? All that differs is the calculation of the radius. Let's move getting the radius outside of the construction of the circle.

if len(circle.radius) != 1:
    radius = circe.radius.pop()
    c = circle.Circle(radius, word[0]) # Draw a circle with the current largest size of circle.radius
else:
    radius = 30
    c = circle.Circle(radius, word[0]) # When there's only one size left (the smallest one), make all other words that size.
c.display()

and now we do the same trick as before: moving shared code outside of a branch

if len(circle.radius) != 1:
    radius = circe.radius.pop()
else:
    radius = 30
c = circle.Circle(radius, word[0])
c.display()

Combining above results

words = Counter(re.findall(r"[\w']+", text.lower())).most_common()
for word in words:
    if len(circle.radius) != 1:
        radius = circle.radius.pop()
    else:
        radius = 30
    c = circle.Circle(radius, word[0])
    c.display()


remove if's for stream filter I've the following code which check for duplicate property and provide and warning when the key is exist and provide error if the key and the value have the same values.
The code have done some iteration and its working
my question is there is a way to avoid the if's statement's  ?

      Map<String, Object> existProp = requires.get(reqidx).getProperties();
        Optional<Map.Entry<String, Object>> foundProperty = properties.entrySet().stream().filter(property -> existProp.containsKey(property.getKey())).findFirst();
        if (foundProperty.isPresent()) {
            // Same key already exists
            LOG.warn("Property with key {} already exists in requires {}", foundProperty.get().getKey(), requires.get(reqidx).getName());
            // in case the existing property has the same value (check only for value of type String)
            if (foundProperty.get().getValue() instanceof String
                    && existProp.get(foundProperty.get().getKey()) instanceof String
                    && (foundProperty.get().getValue()).equals(existProp.get(foundProperty.get().getKey()))) {
                throw new Exception("Property " + foundProperty.get().getKey() + "  exists already with value " + foundProperty.get().getValue() + " in requires " + requiresName);
            }
        }
        return reqidx;
    }

Every Optional.isPresent() can be replaced with the fluent Optional.ifPresent() which takes a Consumer function arg (that can be lambda expr) which accepts the value of the Optional.get()

properties.entrySet().stream().filter(property -> existProp.containsKey(property.getKey()))
.findFirst().ifPresent(foundPropertyValue -> {
  // Same key already exists
  LOG.warn("Property with key {} already exists in requires {}", foundPropertyValue.getKey(), requires.get(reqidx).getName());
  // in case the existing property has the same value (check only for value of type String)
  if (foundPropertyValue.getValue() instanceof String
      && existProp.get(foundPropertyValue.getKey()) instanceof String
      && (foundPropertyValue.getValue()).equals(existProp.get(foundPropertyValue.getKey()))) {
        throw new Exception("Property " + foundPropertyValue.getKey() + "  exists already with value " + foundPropertyValue.getValue() + " in requires " + requiresName);
  }
});


Binary convert to string in Ruby This was a challenge for reddit #202 Easy /u/learnprogramming ...I wish that someone reviews my code and let me know your opinion about my way of thinking beacause I am a total beginner in Ruby:

class BinaryChar
  attr_accessor :binaryString

  def convertToChar
    binaryString.scan(/.{1,8}/) { |part| print part.to_i(2).chr }
    puts
  end

end

bc = BinaryChar.new
speak = 0
loop do
  print "Enter string: "
  speak = gets
  bc.binaryString=(speak)
  bc.convertToChar
end

BinaryChar is a class with one method. You continually change the state of the object so you can call the method with the right "argument". That's not what classes are for, but module functions or class methods. I rewrote the code twice: once using a class in the right way and once using a module function (or class method).

with a class

class BinaryChar
  def initialize(string)
    @binary = string.scan(/.{1,8}/).map do |character|
      character.to_i(2).chr
    end
  end

  def to_s
    @binary.join
  end
end

loop do
  print "Enter string: "
  bc = BinaryChar.new(gets)

  puts bc # to_s is called automatically by puts.
end

with a module function

module Binary
  def self.to_binary(string)
    binary = string.scan(/.{1,8}/).map do |character|
      character.to_i(2).chr
    end

    binary.join
  end
end

loop do
  print "Enter string: "

  puts Binary.to_binary
end

A couple of minor nitpicks:

In ruby the convention is to use snake_case for anything but ClassNames and CONSTANTS.
When you get input from stdin (gets) a newline is appended to it ("\n"). In most cases you want to strip the newline of using String#chomp.
Accessor methods can look just like variable assignments: bc.binary_string = speak.
Conversion methods are usually named to_<something>. To convert to a String, for example, most classes implement a to_s method.


Create own implementation of vector which has moving operations and push_back for rvalue template<typename T>
class Vector
{
private:
    size_t size;
    size_t capacity;
    T* buffer;
public:
    Vector(size_t s) {
        size = 0;
        capacity = s;
        buffer = new T[capacity];
    }
    //Move constructor
    Vector(Vector&& rhs): size(rhs.size),
                          capacity(rhs.capacity),
                          buffer(tmp.buffer)
    {
        rhs.buffer = nullptr;
    }
    T& operator=(Vector&& tmp) {
        size = tmp.size;
        capacity = tmp.capacity;
        buffer = std::move(tmp.buffer);
        return *this;
    }
    void push_back(const T& v) {
        if (size >= capacity)
            reserve(capacity + 5);
        buffer[size++] = v;
    }
    void push_back(T&& v) {
        if (size >= capacity)
            reserve(capacity + 5);
        buffer[size++] = v;
    }

Have I implemented all move operations in my class Vector?
Is it correct implementation of method push_back for rvalue ?

Have I implemented all move operations in my class Vector?

Yes. And no. You have a bug. You are not resetting the size of the moved Vector, but resetting its buffer, which can break code like this:

Vector<int> vec;
vec.push_back(1);
Vector<int> v = std::move(vec);
if (vec.size() == 1)
    vec[0] = 8; // undefined behavior!

Also, what is tmp? Did you mean to use rhs?

  Is it correct implementation of method push_back for rvalue ?

No. In

buffer[size++] = v;

you are not taking advantage that v is a rvalue reference. You are copying v into the buffer, but that is not needed, as v points to a rvalue. You can move it to the buffer:

buffer[size++] = std::move(v);

Although you have implemented all move operations for Vector, you forgot the copy operations. But maybe you just hide them because you didn't want any feedback on them.

Nonetheless, you could have written your move constructor like this, which is more readable IMO (and doesn't include the bug mentioned above):

Vector(Vector&& rhs) : size(std::exchange(rhs.size, 0)),
                       capacity(std::exchange(rhs.capacity, 0)),
                       buffer(std::echange(rhs.buffer, nullptr))
{}

You can do the same thing for operator=(Vector&&).

However, consider using the copy-swap idiom, for a exception safe way of implementing the Rule of Five.


Displaying an item from an array on page load This piece of Angular code is used to add/remove an item to a favourite array and display it on page load. Firstly I check if I have an array and then create one if not. It also checks the localStorage on page load.

How can this be improved?

$scope.favourites = $localStorage.favourites;

$scope.addExerciseToFavourites = function(exercise) {
  if($scope.favourites === undefined) {
    $scope.favourites = [];
    favouriteToggle(exercise)
  } else {
    favouriteToggle(exercise);
  };
};

function favouriteToggle(exercise) {
  if($scope.favourites.indexOf(exercise.exerciseName) == -1) {
      $scope.favourites.push(exercise.exerciseName);
  } else {
      $scope.favourites.splice($scope.favourites.indexOf(exercise.exerciseName),1);
  };
   $localStorage.favourites = $scope.favourites;
};

It's a good start, but there are some improvements that can be made.

For instance, this chunk:

if($scope.favourites === undefined) {
  $scope.favourites = [];
  favouriteToggle(exercise)
} else {
  favouriteToggle(exercise);
};

Could be written as:

if($scope.favourites === undefined) {
  $scope.favourites = [];
}

favouriteToggle(exercise);

Since you always want to call the favouriteToggle function.

But you can go further, and treat undefined as simply another false'y value like false, null, empty string, and numeric zero:

if(!$scope.favourites) {
  $scope.favourites = [];
}
// ...

Which is synonymous with a common set-default-value-if-false'y idiom:

$scope.favourites = $scope.favourites || [];
// ...

You might also see it written as:

$scope.favourites || ($scope.favourites = []);
// ...

The three code blocks above are all equivalent: If $scope.favourites is false'y, set it to [].

Since it's a simple one-liner, you can just move it out of the function entirely, and then just skip the entire function:

$scope.favourites = $localStorage.favourites || [];

function $scope.favouriteToggle(exercise) {
  if($scope.favourites.indexOf(exercise.exerciseName) == -1) {
      $scope.favourites.push(exercise.exerciseName);
  } else {
      $scope.favourites.splice($scope.favourites.indexOf(exercise.exerciseName),1);
  };
   $localStorage.favourites = $scope.favourites;
};

As for favouriteToggle, I'd name it toggleFavourite instead, just because it sounds more natural. But let's make it little more descriptive, and call it toggleExerciseFavourite.

Secondly, instead of finding the index twice with indexOf, it'd be easier to find it once, and store the result:

$scope.toggleExerciseFavourite = function(exercise) {
  var index = $scope.favourites.indexOf(exercise.exerciseName);

  if(index === -1) {
    $scope.favourites.push(exercise);
  } else {
    $scope.favourites.splice(index, 1);
  }

  $localStorage.favourites = $scope.favourites;
};

Lastly, I don't know Angular, I'm afraid, so I don't know how manipulating the $localStorage object works. But if $scope.favourites is just a reference to it, we can skip the last line, since we're manipulating the same object already:

$scope.toggleExerciseFavourite = function(exercise) {
  var index = $scope.favourites.indexOf(exercise.exerciseName);

  if(index === -1) {
    $scope.favourites.push(exercise);
  } else {
    $scope.favourites.splice(index, 1);
  }
};


To-do list using React Would this todo list benefit from breaking it down into more components?  What would be the advantages?  Am I breaking any best practices from what is seen here?

var TodoList = React.createClass({
    getInitialState: function(){
        return {
            todos:data,
            text:""
        };
    },
    handleChange: function(event){
        this.setState({text:event.target.value });
    },
    submitHandler: function(event){
        event.preventDefault();
        var todo = this.state.text;
        data.push({task:todo});
        this.setState({text:""});
        $('#input').val("")
    },
    render: function(){
        var rows = [];
        this.state.todos.forEach( function(todo, index){
            rows.push(
{todo.task}
)
        });

        return(

                <form onSubmit={this.submitHandler}>

                        {rows}

                    <input id="input" key={this.state.timestamp} onChange={this.handleChange} type="text" />

                    <button type="submit" className="btn btn-primary" disabled={this.state.text.length === 0}>Submit</button>
                </form>

        )
    }
});

var data = [
    {task:"eat cheese"},
    {task:"go to gym"},
    {task:"do some yoga"},
    {task:"go to sleep"}
];

ReactDOM.render(<TodoList/>, document.getElementById('app') );

I think data should just be contained in the component state, because it belongs to the TodoList:

getInitialState: function() {
    return {
        todos: [{task: "eat cheese"}, ...],
        text: ""
    };
},

which would simplify submitHandler into

submitHandler: function(event) {
    event.preventDefault();
    var todo = this.state.text;
    this.setState({
        text: "",
        todos: this.state.todos.concat([todo])
    });
},

The jQuery $('#input').val("") call is unnecessary because React should be handling that state for us. Just make the value of the <input> reflect state.text and all will flow:

<input id="input" key={this.state.timestamp} onChange={this.handleChange}
       value={this.state.text} type="text" />

In render(), the declaration of rows and the forEach can be combined into a map:

var rows = this.state.todos.map(function(todo, index) {
    return (
{todo.task}
);
});

--

In general your code is very clear. Right now your app is simple and I don't think there's much need to break it down into smaller parts.

However, as your app inevitably grows and you add more features, separating, say, TodoItem and TodoList into distinct components would be useful. When a function gets long and starts doing five or six conceptually distinct things, you separate it into smaller functions and compose them together. Same thing when classes start representing many things at once.

So some advantages of breaking things down into smaller components are modularity and composability. Your app becomes easier to test and think about.


Optimize narrow phase contact detection for phyiscs simulation code I'm trying to optimize the narrow phase of the contact detection in my code. I'm using OpenMP for multithreading.
That is the starting code:
// Narrow Phase
        const unsigned int NbPairs = pair_manager_->get_n_pairs();
        delete[] sum_rad_;
        sum_rad_ = new double[NbPairs];
        delete[] distance_vector_;
        distance_vector_ = new Vector3d[NbPairs];
        delete[] sq_dist_;
        sq_dist_ = new double[NbPairs];
        const Pair * Pairs = pair_manager_->get_pairs();
        int n_active = 0;
        auto * active = new bool[NbPairs];
#pragma omp parallel default(none) shared(active, Pairs, n_active) firstprivate(NbPairs) num_threads(current_num_threads_)
        {
#pragma omp for schedule(static)
            for (int i = 0; i<NbPairs; i++) {
                const int id_0 = Pairs[i].id_0_;
                const int id_1 = Pairs[i].id_1_;
                sum_rad_[i] = radius_[id_0]+radius_[id_1];
                distance_vector_[i] = position_[id_1]-position_[id_0];
                sq_dist_[i] = distance_vector_[i].squaredNorm();
            }
            Pair * PairsNonConst = pair_manager_->get_pairs_non_const();
#pragma omp for schedule(static) reduction(+:n_active)
            for (int i = 0; i<NbPairs; i++) {
                if (sq_dist_[i]<sum_rad_[i]*sum_rad_[i]) {
                    active[i] = true;
                    n_active++;
                } else {
                    PairsNonConst[i].data_id = -1;
                    active[i] = false;
                }
            }
        }
        delete[] active_pairs_;
        active_pairs_ = new int[n_active];
        n_active_pairs_ = n_active;
        int j = 0;
        // FIXME: is it possible to include it in the parallel loop? -> I guess so, then do merge reduction
        for (int i = 0; i<NbPairs; i++) {
            if (active[i]) {
                active_pairs_[j] = i;
                j++;
            }
        }
        assert(j==n_active_pairs_);
        delete[] active;

The pair manager contains the candidate contact pairs, which has to be checked in the narrow phase. I only have spherical objects, so this is fairly easy (if squared sum of radii is larger than the squared distance of the objects, then we have a contact).
In a previous developing phase, I splitted the two for loops to get performance benefits, so that the first one does not contain any if statement. So, in the first one I calculate all the relevant quantities for all contact candidates, in the second one I have to check every candidate for actual contact.
After that, I have to loop over the active array and save the indexes of each active contact pair. In another part of the code, I'll then loop over this array active_pairs_ and calculate the contact forces, i.e. by indirectly accessing sq_dist_ and distance_vector_ in this way:
#pragma omp for schedule(static)
for (int i=0; i<n_active_pairs_; i++) {
   int id_pair = active_pairs_[i];
   const Vector3d distance = distance_vector_[id_pair];
   // ...
}

Ideally, I would like to save only the quantities for the actual contact pairs in sum_rad_, distance_vector_, and sq_dist_ so that in this for loop I can only loop over those arrays.
Also, this would save me the last serial loop in the narrow phase. On the other hand it would mean merging the two loops in the parallel region (unless someone has a better idea on how to do that).
Last but not least, with my approach, I cannot have a proper first touch (important on NUMA systems). It would be nice to get this somehow as well.
What are your thoughts?

delete[] sum_rad_;
    sum_rad_ = new double[NbPairs];
    delete[] distance_vector_;
    distance_vector_ = new Vector3d[NbPairs];
    delete[] sq_dist_;
    sq_dist_ = new double[NbPairs];

Why all these bare array-pointers that you have to manage yourself?  You'd have much more maintainable code with std::vector or even just std::unique_ptr<T[]>.


Resolving IP to hostname in Python 2 This piece of script will take one argument (file which contain list of ip, one per line) and convert it to its hostname. For now this will only work with IPv4.

This definitely doesn't look good and I want experts to review it and suggest some good ways of coding it in Python.

#!/usr/bin/python

import socket
import sys

BLUE='\033[34m'
RED='\033[31m'
NORMAL='\033[0m'

def check_ip():
        try:
                socket.inet_aton(ip)
        except Exception, e:
                print RED+"Invalid ip addresss = "+NORMAL + str(ip)

file = sys.argv[1]
with open(file) as ipfile:
        for iplist in ipfile:
                for IP in iplist.split():
                        ip = IP.strip(' \t\n\r')
                        if ip == '':
                                pass
                        else:
                                check_ip()
                                host = socket.getfqdn(ip)
                                print BLUE+ip+NORMAL + " hostname is " + BLUE+host+NORMAL

The standard indentation in Python is 4 spaces, as specified in PEP 8.  This is a pretty strong convention, since Python is very sensitive to indentation.

To process a file specified on the command line, use the fileinput module.  It supports multiple filenames, and it also falls back to reading standard input if there is no filename given.

Instead of for iplist in ipfile, I would say for line in ipfile — because it's not a list until you have split it.  It's also odd that you wrote the code to expect a "list", when in your question, you specified one IP address per line.

str.split(), with no separator specified, splits on whitespace and automatically discards whitespace from the results for you.

Calling check_ip() with the ip as a global variable is bad practice.  You should pass it as a parameter.  However, I wouldn't bother defining a function for it, especially since that function has a side-effect of printing its output.

The except Exception, e syntax is obsolescent.  Prefer writing except Exception as e.  (In this case, you don't need to name the exception at all.)

Since all of the color-changing codes should reset the color back to normal, I would define a function for that.

import fileinput
import socket

RED = '31'
BLUE = '34'

def color(color_code, text):
    return '\033[{}m{}\033[0m'.format(color_code, text)

for line in fileinput.input():
    for ip in line.split():
        try:
            socket.inet_aton(ip)
        except Exception as invalid_ip:
            print color(RED, 'Invalid IP address = ') + ip
        else:
            hostname = socket.getfqdn(ip)
            print color(BLUE, ip) + ' hostname is ' + color(BLUE, hostname)


Encapsulated unit testing logic I have some C# unit tests which each basically perform the same steps but supply different arguments to the unit under test.

I wanted to encapsulate the logic inside some "helper methods" but found that the method names, while descriptive, were cumbersome. For example, the code inside a test method would look like this contrived example:

var expected = 1;
var set = new [] { 1, 2, 3, 4, 5 };
AssertThatCallToGetSmallestReturnsExpectedValueFromGivenSet(expected, set);

I simplified the method names and took up a convention of using C#'s named arguments feature:

AssertThatCallToGetSmallest(
    returnsValue: 1,
    fromSet: new [] { 1, 2, 3, 4, 5 }
);

The latter approach seems cleaner and more readable to me. I've been working with objective-c for a few months now so invoking methods in this manner has become less of an issue for me aesthetically. The other advantage is that this convention scales better with an increase in the number of parameters (if that should ever really be of value).

One thing I'm particularly concerned with is the lack of enforced readability because named arguments are optional. There is the potential for programmers to later come along and "misuse" the helper methods I've created by failing to specify the parameter names:

AssertThatCallToGetSmallest(1, new [] { 1, 2, 3, 4, 5 }); // what does this even mean?

So, which way is "better"?

I'm using the Visual Studio Unit Testing Framework with MSTest.

The most canonical way of accomplishing what you want is to use the testing framework's functionality for writing data-driven tests, rather than writing a common helper method and then a number of separate tests which pass arguments into it.

Different frameworks do this differently, so I will focus on MSTest first, as that is the framework you use.

In MSTest, there is a DataSource attribute you can supply, which defines how the framework can locate your test data.  Additionally, you supply a TestContext property of type TestContext.

When the tests execute, TestContext will be populated with the data for a single test case, and you can access it by using named indexing into the TestContext.DataRow property.

(Note: I haven't done MSTest in some time, so feel free chime in if there's a better way nowadays)

Example:
(Embedded resource: foo.xml)

<?xml version="1.0" encoding="utf-8" ?>
<Tests>
   <Test>
      1
      2
      <result>3<result>
   </Test>
   <Test>
      2
      2
      <result>4<result>
   </Test>
</Tests>

(test file)

  public TestContext TestContext { get; set; }

  [TestMethod]
  [DeploymentItem ("foo.xml")]
  [DataSource ("Microsoft.VisualStudio.TestTools.DataSource.XML",
               "|DataDirectory|\\foo.xml",
               "Test",
               DataAccessMethod.Sequential)]
  public void TestAdd ()
  {
     var first = (int) TestContext.DataRow ["a"];
     var second = (int) TestContext.DataRow ["b"];
     var expected = (int) TestContext.DataRow ["result"];
     var result = testObject.Add(first,second);
     Assert.AreEqual(expected, result);
  }

Adding more test cases is just a matter of adding  elements with new data and re-building/re-running.

The How-To MSDN page can be found here, and some examples of DataSource configuration strings can be found here.

Just to give you a taste of this in other frameworks:

NUnit TestCase

  [TestCase(1,2,3)]
  [TestCase(2,2,4)]
  public void TestAdd(int first, int second, int expected)
  {
     var result = testObject.Add(first, second);
     Assert.That(result, Is.EqualTo(expected));
  }

NUnit TestCaseSource

  public static IEnumerable<TestCaseData> TestData
  {
     yield return new TestCaseData(1, 2, 3);
     yield return new TestCaseData(2, 2, 4);
  }

  [TestCaseSource("TestData")]
  public void TestAdd(int first, int second, int expected)
  {
     var result = testObject.Add(first, second);
     Assert.That(result, Is.EqualTo(expected));
  }

MbUnit Row

  [RowTest]
  [Row(1,2,3)]
  [Row(2,2,4)]
  public void TestAdd(int first, int second, int expected)
  {
     var result = testObject.Add(first, second);
     Assert.AreEqual(result, expected);
  }


CodingBat: without_end Link To Question

Given a string, return a version without the first and last char, so
"Hello" yields "ell". The string length will be at least 2.
without_end('Hello') → 'ell'
without_end('java') → 'av'
without_end('coding') → 'odin'

This is the code I wrote as a solution to this problem. I feel that this simple of a problem can be written in one line, but I can baffled as to how I can do so; after many attempts. Any and all feedback is appreciated and considered so I can further improve my python practices.
def without_end(str):

  if len(str) <= 2:
    return ""

  no_start = str[1:]
  no_end = no_start[:len(no_start) - 1]

  return no_end

You were right on track with

no_start = str[1:]
no_end = no_start[:len(no_start) - 1]

In a first step, they can be combined:

return str_[1:len(str_)-1]

Note: I changed str to str_ because str is actually a datatype in Python.

The next thing to know about slicing, is that you can use negative indices to index from the back without specifically using the length of the string like so:

return str_[1:-1]

And since this also works for strings with a length equal to two, you can also get rid of the earlier check.

So your are now down to basically:

def without_start_end(input_):
    return input_[1:-1]

Again take note: input_ also has the trailing _, since, you guessed it, input is already taken.

A non-directly code relate side-note: the official Style Guide for Python Code (aka PEP8) recommends to use 4 spaces per indentation level, and most people seem to follow that recommendation. There are however exceptions, e.g. Google's TensorFlow framework uses 2 spaces per indentation level (see their style guide), although Google's own "general purpose" style guide for Python code sticks with 4 spaces.


Counting Lines and Sum of Lines I have a small function that when passed a str that names a file that contains a program; it returns a 2-tuple with the number of the non-empty lines in that program, and the sum of the lengths of all those lines. Here is my current functioning code:

def code_metric(file_name):
    line_count = char_count = 0
    with open(file_name) as fin:
        stripped = (line.rstrip() for line in fin)
        for line_count, line in enumerate(filter(None, stripped), 1):
            char_count += len(line)
    return line_count, char_count

Is there a way to implement this function using functionals such as map, filter, and reduce and small lambdas to pass to these functionals? I could make it work conventionally but having some issue with using functionals. Any help would be great.

line_count can be obtained with len(lines) instead of an enumerate.
You use rstrip (rightstrip), I think that strip is better as you (probably) don't want to count the blanks of indentation as characters.
f is the standard name for files in Python not fin
Using list comprehension and a for loop as you do is much more readable than a functional alternative, anyway, down here I show you a functional version:

:

def code_metric(file_name):
    with open(file_name) as f:
        lines = f.read().splitlines()
    char_count = sum(map(len,(map(str.strip,filter(None,lines)))))
    return len(filter(None,lines)), char_count


Reading a text file, need help cleaning exception handling Which of the following is cleaner / more adopted standard of handling exceptions while coding a stream ? I would also appreciate a reason why one of the following trumps over other ?  In my opinion I find option 1 better but nested try where first try does nothing more than a wrapper is making me not satisfied with approach.

NavigableSet<String> dictionary = new TreeSet<String>();
BufferedReader br = null;
try {
    try {
        br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"));
        String line;
        while ((line = br.readLine()) != null) {
            dictionary.add(line.split(":")[0]);
        }

    } finally {
        br.close();
    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
}

VS

NavigableSet<String> dictionary = new TreeSet<String>();
BufferedReader br = null;

    try {
        br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"));
        String line;
        while ((line = br.readLine()) != null) {
            dictionary.add(line.split(":")[0]);
        }

    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
} finally {
     try{
        if(br != null) br.close();
        } catch (Exception ex){

        }
 }

First off, in Java 7 all of this is obsolete if you use 'try with resources'

NavigableSet<String> dictionary = new TreeSet<>();
try (BufferedReader br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"))) {
    String line;
    while ((line = br.readLine()) != null) {
        dictionary.add(line.split(":")[0]);
    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
}

If you have to work on an earlier JDK consider these points :

Both snippets you supply are not exactly equivalent. i.e. if closing the file fails, the first snippet will throw a RuntimeException wrapping the exception of the close(). The second snippet will throw a RuntimeException wrapping the original exception.

Again in SE 7 using the try with resources you'll be able to see both exceptions :  the original exception is thrown and it has the exception upon close() attached as a suppressed exception.

So I think style is irrelevant, you want the original exception (which may even not be related to I/O). If you can use SE 7, use that, otherwise go for snippet 2 (add logging for the exception on close())