Attempt at checking whether two words are permutations This is my first algorithm in C++ ever to check if two strings are permutations of each other. Please provide guidance on correctness of algorithm + ways to improve + thoughts on my use of the language itself.

The idea is two create two dictionaries which count the number of occurrences of each character using two for loops, and then we use a final for loop to check if the counts are equal.

void checkPermut(std::string &sentence1, std::string &sentence2)
{
    std::map<char, int> dict1;
    std::map<char, int> dict2;

    for (char & c : sentence1)
    {
        if (dict1.find(c) == dict1.end())
        {
            dict1.insert(std::pair<char, int>(c, 1));
        }
        else
        {
            dict1[c] == dict1[c] + 1;
        }
    }

    for (char & c : sentence2)
    {
        if (dict2.find(c) == dict2.end())
        {
            dict2.insert(std::pair<char, int>(c, 1));
        }
        else
        {
            dict2[c] == dict2[c] + 1;
        }
    }

    bool arePermut = true;

    for (char & c : sentence2)
    {
        if (dict1.find(c) == dict1.end())
        {
            arePermut = false;
            std::cout << "Not permutations." << std::endl;
            break;
        }
        else if (dict1[c] != dict2[c])
        {
            arePermut = false;
            std::cout << "Not permutations." << std::endl;
            break;
        }
    }

    if (arePermut)
    {
        std::cout << "Are permutations" << std::endl;
    }

}

int main()
{
    std::string somestring1;
    std::string somestring2;

    std::cin >> somestring1 >> somestring2;
    checkPermut(somestring1, somestring2);

    return 0;
}

Basic Review

Look below for algorithm ideas:

You are not modifying the input strings. So they should be passed by const reference to prevent accidents:

void checkPermut(std::string const& sentence1, std::string const& sentence2)
                             ^^^^^                         ^^^^^

You don't need to check if a value exists in a map like this:

        if (dict1.find(c) == dict1.end())
        {
            dict1.insert(std::pair<char, int>(c, 1));
        }
        else
        {
            dict1[c] == dict1[c] + 1;
        }

Use operator[] If it does not exist the item will be created (the value is "zero" initialized" and returned as a reference. You can replace the above code with the line:

        ++dict1[c];

Be careful when getting things by reference (accidental modification can happen). Unless you need to modify something, get by const reference.

    for (char const& c : sentence2)
              ^^^^^

But in this case I would get by value:

    for (char c : sentence2)

There is an easy way to make pairs that does not require all the type info:

            std::pair<char, int>(c, 1);

Simpler to write:

            std::make_pair(c, 1); // The function gets the type from
                                  // the parameters and creates the
                                  // correct type of underlying pair.

As a side note. technically you may not want to make the assumption that std::map uses a pair internally. This is why std::map has an internal type value_type so you can use that and not make any assumptions about the type used internally.

 using Map      = std::map<char, int>;
 using MapValue = typename Map::value_type;
 Map  dict1;
 ...
 dict1.insert(MapValue(c, 1));

There is a subtle bug here:

    for (char & c : sentence2)
    {
        if (dict1.find(c) == dict1.end())
        {
            arePermut = false;
            std::cout << "Not permutations." << std::endl;
            break;
        }
        else if (dict1[c] != dict2[c])
        {
            arePermut = false;
            std::cout << "Not permutations." << std::endl;
            break;
        }
    }

You check all the letters from sentence2 are exactly the same as in sentence1. But that may not be enough. You need to check the other way.

    // Assume:
    Sentence1 = "BigZ"
    Sentence2 = "Big";  // Your algorithm will match.

Note most containers are comparable.

There is a subtle bug here:

        if (dict1 != dict2)
        {
            std::cout << "Not permutations." << "\n";
        }
        else
        {
            std::cout << "Not permutations." << "\n";

Note: The operator>> reads a single space separated word. Your code above suggests sentences. Is this what you want?

    std::cin >> somestring1 >> somestring2;

Use std::getline(std::cin, somestring1) to read a line.

This is redundant in main():

    return 0;

Algorithm

We know you can simply compare the containers. But you can optimize this. By subtracting characters from sentence two from the container. If any become negative then you can exit early.

Also do you want to compare space characters? This makes it really hard to have anagram sentences if the word count has to be the same.

bool checkPermut(std::string const& sentence1, std::string const& sentence2)
{
    using Map = std::map<char, int>;
    Map count1;
    Map zero;

    for(auto c: sentence1) {
        if (std::isspace(c) {
            continue;
        }
        ++count1[c];
        zero[c] = 0;
    }
    for(auto c: sentence2) {
        if (std::isspace(c) {
            continue;
        }
        if (--count1[c] < 0) {
            return false; // exit early if we go negative.
        }
        zero[c] = 0;
    }
    return count1 == zero;  // Make sure all letters have reached zero.
}


Calculate class to practice encapsulation I realize this is very beginner code, but I just today grasped the basic concepts of private variables, constructors, and such. I wrote this very simple class, and a class to test it. Any and all input welcome.

Calculate.java

import java.lang.Math.*;

public class Calculate {
    private final double num1, num2;

    Calculate(double num1, double num2) {
        this.num1 = num1;
        this.num2 = num2;
    }

    public double Add() {
        return num1 + num2;
    }
    public double Subtract() {
        return num1 - num2;
    }
    public double Multiply() {
        return num1 * num2;
    }
    public double Divide() {
        return num1 / num2;
    }
    public double Exponent() {
        return Math.pow(num1, num2);
    }
}

TestCalculate.java

class TestCalculate {
    public static void main(String[] args) {
        double num1 = 5, num2 =7;
        Calculate calc = new Calculate(num1, num2);
        // show outputs from calculations
        System.out.println("Number 1: " + num1);
        System.out.println("Number 2: " + num2);
        System.out.println("Addition: " + calc.Add());
        System.out.println("Subtraction: " + calc.Subtract());
        System.out.println("Multiplication: " + calc.Multiply());
        System.out.println("Division: " + calc.Divide());
        System.out.println("Exponent: " + calc.Exponent());
    }
}

Sample output using given variables:

  Success   time: 0.1 memory: 320256 signal:0 Number 1: 5.0 Number
  2: 7.0 Addition: 12.0 Subtraction: -2.0 Multiplication:
  35.0 Division: 0.7142857142857143 Exponent: 78125.0

On Formatting

You can format your output using System.out.printf , or String.format()

For example, if you wanted your division test result to only include 2 digits after the decimal you could instead call System.out.printf("Division: %.2f%n", calc.divide());

The % is a special character; when the printf method reaches it it replaces that location with the specified formatted version of the proceeding arguments (You can have several in one line as long as they are in order). The '.2' that proceeds is what enforces only 2 digits after the decimal. The %n that immediately follows is so you can still have a new line after calling printf. You can read more about formatting and find some additional examples here.

On Exception Handling

I'll also echo the previous answer in saying that handling edge cases would do you some good, this could be done with a simple conditional within the method to ensure the divisor is not 0, which would otherwise cause an error or report NaN/infinity when it is undefined.

You could throw an exception if the divisor is 0 within the divide method itself, like so:

public double divide() {
        if (num2 == 0) {
            throw new ArithmeticException("Dividing by 0 is undefined");
        }

        return num1 / num2;
    }

The message could be  a simple "Cannot divide by 0." Which, if you catch it, would allow a 0 divisor to exist, and you don't have to constrict the possible num2 values and calculations possible.

On Java Convention & Readability

variables and methods are camelCase
Class and interface name are PascalCase
CONSTANTS are UNDERSCORE_CAPITALIZED

Also, try to have a line break between your methods, it'll aid in readability once method body increases. Though I personally omit this when the methods are closely linked, like getters and setters for the same value, as @Rhuarc13 suggest you have.

Although I disagree with adding anything not explicitly necessary(any user trying to pass a no arg constructor would encounter a compilation error) since this is an exercise on encapsulation I think a getter has some merit. Setters don't make sense in this case given your variables' final state, but if you instantiated several of these Calculate objects with different values it would be helpful to have a method to keep track of them without providing users with access to the variable itself, which would break encapsulation*.

Access to the variable directly is arguably inconsequential since it's final, but the point of encapsulation is that the external users should not concern themselves with the fields or implementation.

If you go in your calculate and added methods like so:

public double getNum1() {
    return this.num1;
}

public double getNum2() {
    return this.num2; // this keyword refers to that particular object
}

Then when you're interested in what value is actually contained within the object for example in your test case you could call

System.out.println("Number 1: " + calc.getNum1());
System.out.println("Number 2: " + calc.getNum2());

Alternative Implementation

If you were interested in setters as well, if you had private instead of final values you could use something like this:

public class Calculator {
    private double num1;
    private double num2;

    public Calculator setNum1(double num1) {
        this.num1 = num1;
        return this;
    }

    public Calculator setNum2(double num2) {
        this.num2 = num2;
        return this; // return the current state of Calculator to allow for chaining
    }

    public double divide() {
        return num1 / num2;
    }

    public double add() {
        return num1 + num2;
    }

    public static void main(String[] args) {
        // You can chain call them
        Calculator example = new Calculator().setNum1(3).setNum2(5);

        System.out.println(example.add());
        // Or use it separately.
        example.setNum1(45);
        System.out.println(example.divide());
    }
}

On User Input + Further input validation

Your revision seems concerned with user input, if you actually want to get and use input, a simple way to do that is to make use of Scanner.

Make sure to retrieve the resource at the top: import java.util.Scanner and then declare an instance: Scanner input = new Scanner(System.in)

The nice thing about that is you could ensure a non-0 divisor with a while loop.

for instance if in your main method you did:

System.out.print("Enter second value: ");
double val2 = input.nextDouble();

You could proceed with this:

while (val2 == 0) {
    System.out.print(
        "Second value cannot be 0" +
        "\nEnter a non-0 value: ");

    val2 = input.nextDouble();
}

and that would make sure that your user provided a non 0 divisor.


check for null before or after casting I have a list of BasicDBObject that I fetch from my database. This is basically a JSON document converted into a java Object.

Let's assume that we have a BasicDBObject named result

This JSON document may or may not contain the key name. If the document doesn't contain the key, result.get("name") will return null.

Is there a difference in terms of performance between this :

BasicDBObject result = ...
Object name = result.get("name");
String nameString = (name == null ? "." : name.toString());

and

BasicDBObject result = ...
String name = (String) result.get("name");
String nameString = (name == null ? "." : name);

Even if this part of the code is called 100 000+ times within a function, I know this wil never be the bottleneck, I'm just interested in the answer.

The first snippet runs on any type returned by result.get, the second snippet needs the method to return a String. So it makes only sense to compare the snippets if result.get returns  a String. I assume so.

The type cast is a noOp in byte code. So the real difference is that the first snippet calls name.toString(), but the second uses name directly. The result is the same but the method call may be slower than the direct access to name.

If the first is really slower depends on the optimizing that will be done by the compiler and runtime environment. You should benchmark it with the version of java currently in use, if you are interested in the result.


Synchronizing two scrollbars to work in unison This synchronizes the document’s vertical scrollbar and a DIV’s horizontal scrollbar so they can work in unison controlling the content simultaneously.

The code is operable and behaves how I want. I want to know whether or not this code can be optimized any further.

Heads-up: the jsScroll0 and jsScroll1 flags stop either scrollbar from locking due to an infinite loop scenario upon each scrollbar trying to set each other’s values.

(function() {
    var jsScroll0, jsScroll1;
    var divScroller = document.getElementById('scrollP');

    document.onscroll = function () {
        if (jsScroll1) {
            jsScroll1 = false;
            return;
        }
        jsScroll0 = true;
        divScroller.scrollLeft = document.documentElement.scrollTop;
    };
    divScroller.onscroll = function () {
        if (jsScroll0) {
            jsScroll0 = false;
            return;
        }
        jsScroll1 = true;
        document.documentElement.scrollTop = divScroller.scrollLeft;
    };
})();

This looks perfectly good to me.  Everything has a unique purpose, everything is either clearly explained or self-explanatory.  Unless you have tested this and found a performance issue, and you are sure that it is happening here, do not do any optimization to this.

There is no loop and no recursion, the code itself can't properly be an optimization target.  If you are observing that this is running slowly, the hidden costs of setting the scrollbars must be the cause.  To address this you will need to change your entire strategy to keeping the div contents in the right place using CSS only.  I have to say that while I find using the position of some element (scrollP?) to be a pretty elegant way to duck issues of messing around with coordinates that will be inconsistent otherwise, it is technically the best practice to do all positioning with CSS.


Find all starting indices in a string which are anagrams of a word The task:

  Given a word W and a string S, find all starting indices in S which
  are anagrams of W.

  For example, given that W is "ab", and S is "abxaba", return 0, 3, and
  4.

My solution:

    const anagramOccurrencesOf = (w, s) => {
      const occurrencesCopy = [];
      let sCopy = s.slice(0);

      let index = 0;
      while (sCopy.length && index !== -1) {
        index = sCopy.indexOf(w);
        if (index === -1) { break; }
        occurrencesCopy.push((occurrencesCopy[occurrencesCopy.length - 1] + index + 1 || 0));
        sCopy = sCopy.slice(index + 1);
      }
      const occurrencesReverse = [];
      let sReverse = s.split('').reverse().join('');

      index = 0;
      while (sReverse.length && index !== -1) {
        index = sReverse.indexOf(w);
        if (index === -1) { break; }
        occurrencesReverse.push( (occurrencesReverse[occurrencesReverse.length - 1] - w.length - index + 1) || s.length - w.length);
        sReverse = sReverse.slice(index + 1);
      }

      return [...occurrencesCopy, ...occurrencesReverse].sort((a,b) => a -b);
    };

    console.log(anagramOccurrencesOf("ab", "abxaba"));

EDIT: The solution above would find indexes of palindromes. The solution below should find indexes of anagrams:

const sortAlphabetically = x => x.toLowerCase().split('').sort().join('')
const getIndexOfAnagramIn = (s, w) => {
  const wSorted = sortAlphabetically(w);
  const result = []
  for (let i = 0; i < s.length + 1 - w.length; i++) {
    if (sortAlphabetically(s.slice(i, i + w.length)) === wSorted) {
      result.push(i);
    }
  }
  return result;
}

console.log(getIndexOfAnagramIn('abxaba', 'ab'));

This code looks pretty good. I don't see much I would change. This code makes good use of const and let where appropriate.

The only thing that stands out is that splitting a string into an array can be done with the spread syntax instead of calling split().

The first instance:

let sReverse = s.split('').reverse().join('');

Could be changed to

let sReverse = [...s].reverse().join('');

And the other instance:

const sortAlphabetically = x => x.toLowerCase().split('').sort().join('')

Could be changed to:

const sortAlphabetically = x => [...x.toLowerCase()].sort().join('')


Nesting elements in a shallow hierarchy I wrote some code to generate list items for a menu in Clojurescript. The intention is that the code takes parsed Markdown (Hiccup) that has been filtered down to h2 and h3 elements, and then converts them into unordered lists where the h3 elements are nested inside their immediate h2 predecessor.

It turned out to be surprisingly difficult in CLJS, and I'm curious if there's a more Lispy and efficient way to do it.

Code

(defn listify-element [element]
  "Replaces element type with :li."
  (vec (concat [:li (last element))]))

(defn listify-headings [headings-list]
  "Takes subitems (in :h2 :h3) and creates sub :uls out of the :h3 lists."
  (vec
   (concat
    [:ul]
    (map-indexed
     (fn [ind headings]
       (if (= 0 (mod ind 2))
         (map listify-element headings)
         (vec (concat [:ul] (map listify-element headings)))))
     (partition-by #(= :h2 (first %)) headings-list)))))

(defn nest-listified-headings [vector-list]
  "Nests sub-:uls inside their preceding :lis."
  (vec (concat [:ul]
          (reduce
           (fn [acc el] (if (= (first el) :ul)
                          (conj (pop (vec acc)) (conj (last acc) el))
                          (concat acc el)))
           vector-list))))

Test case

(nest-listified-headings
  (listify-headings [[:h2 "Foo"] [:h2 "Bar"] [:h3 "Baz"] [:h3 "Bat"]])

[:ul
  [:li "Foo"]
  [:li "Bar"
  [:ul
    [:li "Baz"]
    [:li "Bat"]]]]

Sorry the late reply,
Below is a lispy-er way to get to the same result :)

The main thing is to define a way to get from 2 ul's to a single ul.
(so we can use that as a step in a reduce)

basically what we want is, given two ul, to append the one of them to the last item of the other.

The algorithm would be something like this

take the last li from the parent.
append to it the whole second list.
pop it back in the parent

we can make a separate function that does exactly that.

let's call it append-li

(defn append-li
  "Appends given child to the last element with in the given parrent"
  [child parent]
  (let [ul-without-last-li (vec (butlast parent)) ;; take the parent with out the last element
        last-li (last parent)]) ;;take the last item from the parent
    (conj ul-without-last-li (conj last-li child))) ;; pop it back with the whole second list as a child

Using this append-li function ( could be named better but oh well :P )
we can implement our transformation algorithm like so

sort all elements in reverse order of h count (so the larger numbered headers are first (some thing like h3, h2, h1)
partition the list of headers by the type of header. so now we have a list of lists of headers, where each list only contains the same kind of headers
convert all headers to lis, so that now we have a list of groups of lis and the close to the front the list a group is, the deeper level it has
make every group a ul
flip the order of the lists (could have been done before), to make it so that the futher away the item
reduce with out append-li function.
profit :)

All this reversing is a bit annoying, but it ultimately maintains the original order of the items.

(small edit)
The reason we want to reverse the thing is that we want to build the tree of ul's in reverse order from the bottom up.
We could do it from the top down, but that would require a bit of recursion.

below is the full code with some (hopefully) helpful comments

Full Code:

(defn tag->li
  "Converts a header tag (or any tag) to a li"
  [[type & content]]
  (into [:li] content))

(defn append-li
  "Appends given child to the last element with in the given parrent"
  [child parent]
  (let [ul-without-last-li (vec (butlast parent))
        last-li (last parent)])
    (conj ul-without-last-li (conj last-li child)))

(defn listify-headings
  "Convers a list header tags to a nested unordered list"
  [nodes]
  (let [rev-sorted (sort-by first #(> %2 %1) nodes) ;;sorts by 'h' number, result is a sorted list where all h3 are before h2 etc
        types      (partition-by first rev-sorted)  ;;makes a list of lists, so that first list is only h3, second is only h2 etc...
        lis        (map #(map tag->li %) types)     ;;make the h's to li, we have order now so we don't care about what kind of h's it is, this gives a list of lists with 'lis in them'
        uls        (map #(into [:ul] %) lis)]       ;;raps each makes each list of 'li' to be a 'ul' with li's inside
    (reduce append-li (reverse uls))))              ;;concat each 'ul' to the previous (that's why we reverse) one to produce the tree of uls al lis

P.S.

while both your code and mine work and get the job done, i'm not sure if it's logically ok to do this.

from a semantic standpoint this does not make to much sens, as the desired output means that only the last header on the page has any sub-headers.

basically it would look like all chapters don't have any titles, and the last one has all of them.

i'm not sure whether or not that's the desired thing, but oh well :P


Program to top-up phone with conditions in Python I am fairly new to Python. I wrote a program to take the user inputs which are service, the top-up time, the top-up amount, and exit or stay on the program.
Please review my code. And, can my code be organized? I think it looks a bit messy, maybe it can be shorter?

First, I wrote everything under function, so I can restart the program later on.
Second, I want to take user input about the service that they want to top up by limiting the choice of service to only 3 which are Green, Blue, or Red. If none of it, display "Try Again!, Wrong Service" otherwise, "Welcome to Top-up Service".
Third, after passed service versification, the user can top up but limit to only 50,100,300,500,1000. Also, the user can only choose "How many times they want to top-up".
Forth, after top-up, the list of top-up and sum of top-up is presented.
Last, the user can choose to stay or exit the program. If stay, the program is restarted, if exit, the program is terminated.

Here is the code that I wrote
  def main():
# Letting user choose a service
    service = None
    while service not in {"Green", "Blue", "Red"}:
        service = input("Please enter Green, Blue or Red: ")
    # Now service is either Green, Blue or Red
        if service in {"Green", "Blue", "Red"}:
            print("Welcome to Top-up Service")
        else:
            print("Try Again!, Wrong Service")

# Doing Top up(s)
    top_list = []
    n=int(input("How many times do you want to top up? "))
    for i in range(1,n+1):
        top_up = None
        while top_up not in {50,100,300,500,1000}:
            top_up = int(input("Please enter 50,100,300,500 or 1000: "))
    # Now top_up is either 50,100,300,500 or 1000"
            if top_up in {50,100,300,500,1000}:
                top_list.append(top_up)
            else:
                print("Try Again!, Wrong Top-up")

# Show all top up(s) information & Sum all top up(s)
    print("Top up(s) information : ", top_list)
    print("Total top up(s): ", sum(top_list))

# Letting user choose to stay or exit
    restart = input("Do you want to start again?,Y or else ").upper()
    if restart == "Y":
        main()
    else:
        print("Thank you for using our service")
        exit()
main()

This is the example of output
Please enter Green, Blue or Red: o
Try Again!, Wrong Service
Please enter Green, Blue or Red: Blue
Welcome to Top-up Service
How many times do you want to top up? 2
Please enter 50,100,300,500 or 1000: 20
Try Again!, Wrong Top-up
Please enter 50,100,300,500 or 1000: 50
Please enter 50,100,300,500 or 1000: 500
Top up(s) information :  [50, 500]
Total top up(s):  550
Do you want to start again?,Y or else n
Thank you for using our service

You can divide your whole program into parts, this way it becomes easy while debugging and increases modularity in the code.
You can divide into multiple functions like

Selecting service.
Add top-up
Restart logic

Refractored code
AVAILABLE_SERVICES = {"Green", "Blue", "Red"}
AVAILABLE_TOPUPS = {50, 100, 300, 500, 1000}

def select_service():
    while True:
        service = input("Please enter Green, Blue or Red: ").title()
        if service in AVAILABLE_SERVICES:
            print("Welcome to Top-up Service")
            break
    return service

def top_up(service):
    times_topup = int(input("How many times do you want to top up? "))
    topups_selected = []
    for _ in range(times_topup):
        while True:
            topup = int(input("Please enter 50,100,300,500 or 1000: "))
            if topup in AVAILABLE_TOPUPS:
                topups_selected.append(topup)
                break
            else:
                print("Try Again!, Wrong Top-up")

    print("Top up(s) information : ", topups_selected)
    print("Total top up(s): ", sum(topups_selected))

def process():
    restart = "Y"
    while restart == "Y":
        service = select_service()
        top_up(service)
        restart = input("Do you want to start again?,Y or else: ").upper()
    print("Thank you for using our service")

if __name__ == "__main__":
    process()

Output:
Please enter Green, Blue or Red: Blue
Welcome to Top-up Service
How many times do you want to top up? 2
Please enter 50,100,300,500 or 1000: 7
Try Again!, Wrong Top-up
Please enter 50,100,300,500 or 1000: 50
Please enter 50,100,300,500 or 1000: 10
Try Again!, Wrong Top-up
Please enter 50,100,300,500 or 1000: 100
Top up(s) information :  [50, 100]
Total top up(s):  150
Do you want to start again?,Y or else: Y
Please enter Green, Blue or Red: Red
...
...
...
Do you want to start again?,Y or else: n
Thank you for using our service

Code Review

PEP-8 violations:

E231 - missing whitespace after ,

range(1,n+1) ⟶ range(1, n+1)
{50,100,300,500,1000} ⟶ {50, 100, 300, 500, 1000}

Declare constants for services and top-ups instead of writing {"Blue", ...} and {50, ...} everywhere.

{"Blue", ...} ⟶ AVAILABLE_SERVICES = {"Green", "Blue", "Red"} Now you can use AVAILABLE_SERVICES instead of writing {"Blue", ...}
{50, ...} ⟶ AVAILABLE_TOPUPS = {50, 100, 300, 500, 1000}

Use str.title when taking input for service, so "BLUE" would become "Blue".
Use _ here for i in range(1, n+1) when you are not using i in the loop.

for i in range(1, n+1) ⟶ for _ in range(n)
What's the purpose of _ in python

You can make AVAILABLE_SERVICES as frozenset to make immutable.

AVAILABLE_SERVICES = frozenset({"Green", "Blue", "Red"})
AVAILABLE_TOPUPS = frozenset({50, 100, 300, 500, 1000})

You can use pep8online* to check if your code is in accordance with PEP-8 standard.
You can use black* (code formatting tool)
black --line-length 79 your_file.py

* I'm not affiliated to pep8online and black.


Prime factorization of a number I'm trying to learn Python through the excellent online book Dive Into Python 3. I have just finished chapter 2 about data types and felt like trying to write a program on my own.
The program takes an integer as input and factorizes it into prime numbers:
$ python main.py 6144
6144 -> (2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3)

$ python main.py 253789134856
253789134856 -> (2, 2, 2, 523, 60657059)

I'd like to know if I could improve it in any way. Or if I've used any bad practice and so on.
#!/usr/bin/python

import sys
import math

def prime_factorize(n):
    factors = []
    number = math.fabs(n)

    while number > 1:
        factor = get_next_prime_factor(number)
        factors.append(factor)
        number /= factor

    if n < -1: # If we'd check for < 0, -1 would give us trouble
        factors[0] = -factors[0]

    return tuple(factors)

def get_next_prime_factor(n):
    if n % 2 == 0:
        return 2

    # Not 'good' [also] checking non-prime numbers I guess?
    # But the alternative, creating a list of prime numbers,
    # wouldn't it be more demanding? Process of creating it.
    for x in range(3, int(math.ceil(math.sqrt(n)) + 1), 2):
        if n % x == 0:
            return x

    return int(n)

if __name__ == "__main__":
    if len(sys.argv) != 2:
        print("Usage: %s <integer>" % sys.argv[0])
        exit()

    try:
        number = int(sys.argv[1])
    except ValueError:
        print("'%s' is not an integer!" % sys.argv[1])
    else:
        print("%d -> %s" % (number, prime_factorize(number)))

Edit:
@JeffMercado: Interesting link as I haven't encountered this before. And yes, the memoization technique should be easy to implement in my case since it is basically the same as the Fibonacci example.
In the Fibonacci example, they have a map (dictionary in Python?) outside the function, but should I do that? Global variables “are bad”? If that's the case, where does it belong? In the function prime_factorize() and I pass the map as an argument? I'm having a hard time deciding things like this, but I guess it gets easier with experience...
@WinstonEwert:

Prime factorization really only makes sense for integers. So you really use abs not fabs.

I couldn't find abs when I ran help(math), only fabs. I thought fabs was my only choice but I just found out that abs doesn't even reside in the math module. Fixed.

Tuples are for heterogeneous data. Keep this a list. It is conceptually a list of numbers, and it so it should be stored in a python list not a tuple.

You write that tuples are for heterogeneous data. I searched SO and many seems to be of this opinion. However, in the book I'm following, the author gives a few points for using a tuple:

Tuples are faster than lists. If you’re defining a constant set of values and all you’re ever going to do with it is iterate through it, use a tuple instead of a list.
It makes your code safer if you “write-protect” data that doesn’t need to be changed. Using a tuple instead of a list is like having an implied assert statement that shows this data is constant, and that special thought (and a specific function) is required to override that.

That is why a returned a tuple. Is the author's points valid or just not in this case?

Your expression, int(math.ceil(math.sqrt(n)) + 1) seems to be more complicated that it needs to be. Couldn't you get by with int(math.sqrt(n) + 1).

The upper value of range was unnecessarily complicated. Fixed.

Again, why are you trying to support something that isn't an int?

I was returning int(n) from get_next_prime_factor(n) since the number that is passed in to the function becomes a float when I divide it (in prime_factorize), so if I return just n from the function, I return a float which gets added to the list 'factors'. When I then print the factors, I get e.g. '11.0' as the last factor for the number 88.
Any other way to fix that?

I'd pass an exit code to indicate failure

Should I just exit(1) or something like that?

#!/usr/bin/python

import sys
import math

def prime_factorize(n):
    factors = []
    number = math.fabs(n)

Prime factorization really only makes sense for integers. So you really use abs not fabs.

    while number > 1:
        factor = get_next_prime_factor(number)
        factors.append(factor)
        number /= factor

    if n < -1: # If we'd check for < 0, -1 would give us trouble
        factors[0] = -factors[0]

    return tuple(factors)

Tuples are for heterogeneous data. Keep this a list. It is conceptually a list of numbers, and it so it should be stored in a python list not a tuple.

def get_next_prime_factor(n):
    if n % 2 == 0:
        return 2

    # Not 'good' [also] checking non-prime numbers I guess?
    # But the alternative, creating a list of prime numbers,
    # wouldn't it be more demanding? Process of creating it.
    for x in range(3, int(math.ceil(math.sqrt(n)) + 1), 2):

Your expression, int(math.ceil(math.sqrt(n)) + 1) seems to be more complicated that it needs to be. Couldn't you get by with int(math.sqrt(n) + 1).

        if n % x == 0:
            return x

    return int(n)

Again, why are you trying to support something that isn't an int?

if __name__ == "__main__":
    if len(sys.argv) != 2:
        print("Usage: %s <integer>" % sys.argv[0])
        exit()

I'd pass an exit code to indicate failure

    try:
        number = int(sys.argv[1])
    except ValueError:
        print("'%s' is not an integer!" % sys.argv[1])
    else:
        print("%d -> %s" % (number, prime_factorize(number)))

Algorithmwise, it will be more efficient to keep a list. They key is to keep information about the primes used between calls. In particular, you want a list that indicates the smallest prime factor for example

factors[9] = 3, factors[15] = 3, factors[42] = 21

A simple modification of the Sieve of Eratosthenes should suffice to fill that in. Then easy lookups into the list should tell you exactly which prime is next.


Fetching weather data from the OpenWeatherMap API I am trying to fetch weather data from the OpenWeatherMap API. The responded JSON will be deserialized into a object which has the same structure of the JSON, then we need to transform this object into another class to meet our needs.

Following is the class which the JSON returned by OpenWeatherMap will be deserialized:

public class OpenWeatherMapResponse implements Serializable {

    private static final long serialVersionUID = 8841521123253407726L;

    private City             city;
    private int              cod;
    private Item[]           list;

    // lots of accessors

    public class static Item {
        // members of inner class Item
    }

    public class static Weather {
        // members of inner class Weather
    }
}

The following is a snippet of the second class. This class has a constructor which takes a OpenWeatherMapResponse and use the value in the response to construct the object will actually be used by my system:

public class CityWeather implements Serializable {

    private static final long serialVersionUID = -4883661697465418806L;

    private static final String ISO8601_FORMAT = "yyyy-MM-dd HH:mm:ss";

    private double            temp;
    private double            minTemp;
    private double            maxTemp;
    private long              weatherId;
    private String            weatherName;
    private String            weatherDesc;

    public CityWeather() {}

    public CityWeather(OpenWeatherMapResponse openWeatherResponse) {

        Item[] list = openWeatherResponse.getList();
        Weather weather = findWeather(list);  // Get the first element of the Weather list

        // OpenWeatherMap doesn't provide API for fetching a specific day's forecast, so we have to build a day's weather data by assembling ourselves.
        Date tomorrow = getDateOfTomorrow();
        double tempMax = 0.0, tempMin = 99.0;
        List<Double> temps = new ArrayList<Double>();
        for (Item item : list) {
            Date dt = null;
            if((dt = parseDtTxt(item.getDt_txt())) == null) {
                continue;
            }

            // We only use weather data in recent 24 hours.
            if (dt.after(tomorrow)) {
                break;
            }

            Main main = item.getMain();
            temps.add(main.getTemp());
            tempMax = Math.max(tempMax, main.getTemp_max());
            tempMin = Math.min(tempMin, main.getTemp_min());
        }

        this.weatherId = weather.getId();
        this.weatherName = WeatherType.fromId((int) weather.getId()).name();
        this.weatherDesc = weather.getDescription();
        this.temp = calculateMean(temps);
        this.maxTemp = tempMax;
        this.minTemp = tempMin;
    }

    // lots of accessors

    // Parse the datetime string
    private Date parseDtTxt(String dtTxt) {
        try {
            return new SimpleDateFormat(ISO8601_FORMAT).parse(dtTxt);
        } catch (ParseException ex) {
            return null;
        }
    }

    private Date getDateOfTomorrow() {
        Calendar temp = Calendar.getInstance();
        temp.add(Calendar.DATE, 1);
        return temp.getTime();
    }

    private Weather findWeather(Item[] list) {
        for (Item item : list) {
            if (item.getWeather() !=null && item.getWeather().length > 0) {
                return item.getWeather()[0];
            }
        }
        return null;
    }

    private double calculateMean(List<Double> numbers) {
        if (isEmpty(numbers)) return 0;

        double sum = 0;
        for (double d : numbers) {
            sum += d;
        }
        return sum/numbers.size();
    }
}

As you can see, although this CityWeather looks like a data object, there is quite a little logic in this class to initialize its attributes.

Is this a good practice? If not, how should I modify my code?

I would suggest splitting this into two classes, one being your CityWeather POJO and a separate class for performing the logic currently in CityWeather.  The primary reason for this is to separate the business logic here (the parsing of the response from OpenWeatherMap) and your representation of the response (CityWeather).  In doing so, whichever class you have that is currently invoking the call to OpenWeatherMap should then pass the OpenWeatherMapResponse response that is returned from the call to this new class, which we'll call OpenWeatherMapParser.  OpenWeatherMapParser should take this response, and instantiate CityWeather objects based on the logic you currently have in CityWeather by calling it's setter methods


Project Euler Problem 33 - digit canceling fractions Problem:

  The fraction \$(\dfrac{49}{98})\$ is a curious fraction, as an inexperienced
  mathematician in attempting to simplify it may incorrectly believe
  that \$(\dfrac{49}{98}) = (\dfrac{4}{8})\$, which is correct, is obtained by cancelling the 9s.

  We shall consider fractions like, \$(\dfrac{30}{50}) = (\dfrac{3}{5})\$, to be trivial examples.

  There are exactly four non-trivial examples of this type of fraction,
  less than one in value, and containing two digits in the numerator and
  denominator.

  If the product of these four fractions is given in its lowest common
  terms, find the value of the denominator.

This is some rough code -- this one was a lot harder than I expected it to be. Please help me optimize.

from timeit import default_timer as timer

start = timer()
fractions_that_work = []

def a_common_element(list_a, list_b):
    count = 0
    common = 0
    for m in list_a:
        for n in list_b:
            if m == n:
                count += 1
                common = m
    if count == 1:
        return common
    return False

for numerator in range(10, 100):
    for denominator in range(numerator + 1, 100): # denom must be > num
        n_digits = sorted([int(x) for x in str(numerator)])
        d_digits = sorted([int(x) for x in str(denominator)])
        common = a_common_element(n_digits, d_digits)
        if common:
            n_digits.remove(common)
            d_digits.remove(common)
            n_rem = n_digits[0]
            d_rem = d_digits[0]
            if 0 not in n_digits and 0 not in d_digits:
                if float(numerator) / denominator == float(n_rem) / d_rem:
                    fractions_that_work.append([numerator, denominator])
product_of_fractions = [1, 1]
for frac in fractions_that_work:
    product_of_fractions[0] *= frac[0]
    product_of_fractions[1] *= frac[1]

start = timer()
ans = product_of_fractions[1] / product_of_fractions[0]
elapsed_time = (timer() - start) * 1000 # s --> ms

print "Found %d in %r ms." % (ans, elapsed_time)

Firstly, you set start at the top of the script (which is OK). However, you reset start right before your final calulation:

# This will only time the execution of one statement!
start = timer()
ans = product_of_fractions[1] / product_of_fractions[0]
elapsed_time = (timer() - start) * 1000 # s --> ms

Remove the above start = timer() to get a more accurate timing. Also, I would recommend moving the first start declaration to just before the nested for loops. That was the start time is more spatially related to what you are actually timing.

Secondly, your a_common_element function's name is a little misleading. Currently, it insinuates it will return positively if the two lists have any number of common elements. Changing the function name to something like has_single_common_element gives a better description of what the function actually does.

Also, a_common_element can be simplified a tad. By using list comprehension we can combine the nested for loops into one comprehension:

def a_common_element(list_a, list_b):
    common_list = [value for value in list_a if value in list_b]

    if len(common_list) == 1:
        return common_list[0]
    return False

Your next section is actually quite interesting: you used a neat way to separate the digits. However, the downfall (I believe) to your method is the back-and-forth casting that it requires. I would, instead, recommend doing to calculations to get your digits:

for numerator in range(10, 100):
    for denomerator in range(numerator+1, 100):
        # Division will strip off the first digit
        # Modulus will strip off the 0th digit
        n_digits = sorted([int(numerator/10), numerator % 10])
        d_digits = sorted([int(denomerator/10), denomerator % 10])

NOTE: This recommendation is based on the project description you provided, that is all numbers will have exactly 2 digits. If you wanted to extend this to greater sets of numbers (3-digits, 4-digits, etc) I would keep your current implementation.

This next recommendation, like the previous, is based off of 2-digit numbers only.

Once you remove the common digit from the lists, you assign the remaining digits to variables; which is good. However, you then go check if the lists contain 0 instead of comparing the variables:

# I imagine this is what Python does in the backgroud when you say:
#    if 0 not in list
if n_rem != 0 and d_rem != 0:

Your next section is nice. Its simple and intuitive. If you wanted to make the code more Pythonic, you could instead do it this way:

product_of_fractions = [1, 1]
for frac in fractions_that_work:
    product_of_fractions = [i*j for i, j in zip(product_of_fractions, frac)]

If you wanted to go a bit more towards functional programming you could use this single line of code:

from functools import reduce   # Needed for Python 3, works in Python 2.6+
import operator

product_of_fractions = [reduce(operator.mul, value) for value in zip(*fractions_that_work)]

Even though it may be more complex (and thus slightly less readable), I prefer this last way because of its brevity.


VBA - If error return '0' else execute code I am converting below excel formula to VBA code and it works as required.

=IFERROR(IF(A2="NULL",B2*C2,A2*B2*C2),0)

Could someone please review and suggest if any changes are required in terms of handling errors.

What the code does : If there is an error it returns '0' else executes the code.

Sub MultiplyValues()
'
' MultiplyValues Macro
'
' =IFERROR(IF(A2="NULL",B2*C2,A2*B2*C2),0)
' Forumala is in D

Dim rCell As Range
Dim rRng As Range
Set rRng = Range("A2", Range("A2").End(xlDown))
    For Each rCell In rRng.Cells
        On Error GoTo ErrHandler:
            If rCell.Value = "NULL" Then
                rCell.Offset(0, 3) = rCell.Offset(0, 1) * rCell.Offset(0, 2)
            Else
                rCell.Offset(0, 3) = rCell.Offset(0, 0) * rCell.Offset(0, 1) * rCell.Offset(0, 2)
            End If
            Next rCell
         Exit Sub
         ErrHandler:
            rCell.Offset(0, 3) = 0
    Resume Next
End Sub

This is easily handled with a simple function that checks for a potential error instead of triggering an error. Code first, and then an explanation:

Public Sub MultiplyValues()
    '
    ' MultiplyValues Macro
    '
    ' =IFERROR(IF(A2="NULL",B2*C2,A2*B2*C2),0)
    ' Forumala is in D

    Dim Target As Range

    ' The line below implicitly uses the ActiveSheet. If this is intended, qualify your range with `ActiveSheet.Range`, otherwise
    ' you will want to identify the proper sheet.
    Set Target = Range("A2", Range("A2").End(xlDown))
    Dim Cell As Range
    For Each Cell In Target.Cells
        ' No need to use an error handler. Avoiding errors is always preferred over relying on the error handler. In this case, let's use a function.
        Cell.Offset(0, 3).Value = GetFormulaValue(Cell)
    Next Cell
End Sub

Private Function GetFormulaValue(ByVal InputCell As Range) As Double
    If Not IsNumeric(InputCell.Value) Then
        ' Be sure to explicitly access the value property of the range object.
        GetFormulaValue = InputCell.Offset(0, 1).Value * InputCell.Offset(0, 2).Value

    ' I am adding this in case you want to run a separate calculation if the cell-value is 0).
    ' ElseIf InputCell.Value = 0 Then
        ' Your code here
    Else
        ' While order of operations would automatically calculate X * Y and then XY * Z, I prefer using parentheses
        ' to make my code even clearer.
        GetFormulaValue = (InputCell.Offset(0, 0) * InputCell.Offset(0, 1)) * InputCell.Offset(0, 2)
    End If
End Function

Public vs Private

First I would like to note the use of a Public subroutine along with a Private function. This basically just means that the MultiplyValues subroutine is accessible by other routines/the macro's list. On the other hand, the function is only seen by other code in the same module (so we don't risk the wrong code using it, and potentially returning a bad value). This is a little bit more advanced compared to where you are, but it is a good practice to get into early.

Variable Declarations

I noticed that you were using Hungarian Notation (HN). I would venture to assume we all go through this phase in the SO VBA community, but the sooner you break that habit, the better. It is far better to properly name variables than to use HN to denote the types. For this purpose, the names were easy. For larger projects it can be a bit more complicated. The sooner you avoid the habit though, the better.

The Good Stuff

Avoiding Errors Before They Happen

Error handling in VBA is not very robust. It works, but oftentimes it is misused. I am of the mindset that error handling (in the line label sense) should be avoided at all costs. The best way to accomplish this is to use logic to your advantage, and to wrap repeated operations in a single function.

In this case, the appropriate method was easy. The function takes an input cell, and returns a value based on the contents of the cell. While you initially checked for Null the function checks for IsNumeric since this will catch Null as well as anything else that isnt a number (since XYZ isn't Null but will throw an error if multiplied by a number).

Why Use Functions?

I like to think of functions as magical creatures that really are under-appreciated by newer programmers. Maybe I am hyping them up a bit. Maybe not. What I can say for sure is I used to avoid them. Then, someone came along (cough ThunderFrame cough) and lightly-reprimanded my avoidance of using single-functions for repeated steps (I had the same Format(X, Y) command in numerous places. He reminded me that, should that command ever change, I would then have to find every single place it lived and fix it. Simple advice, simple reminder, but it changed my approach to using Functions.

I tend to ramble, so the point here is this: When you have some operation that needs to be done in multiple places (or on multiple conditions) you should strongly consider a function. That way, should the operation change, you can fix it in the function, and not in all of the places that you are calculating the operation manually. Functions also allow you to return different types of values. Declare a Function as a Variant and you can return a vbNullString a 0 a "ThisIsABadValuePleaseIgnoreIt". Whatever you want to do. They allow greater control over the flow of your code.

Final Tidbits

As I do in many of my CR posts, I recommend using RubberDuck, especially when you're starting out. You can find it here: http://rubberduckvba.com/. It helps for finding those little things you do without knowing it (Implicit reference to ActiveWorksheet and .Value for example) and help you learn better habits. Over time, these small changes in habits can make or break a programmer.

Best of luck on this.


SslStream Class Review I have created an SslStream Abstract Class for my Client and i want you to review and enhance it for me if needed

SecureStream.cs

using System;
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Security.Cryptography.X509Certificates;
using System.Threading.Tasks;
using Program.Connections.Packets;
using Program.Core.Misc;

namespace Program.Connections.Network
{
    public abstract class SecureStream
    {
        private static readonly byte[] Buffer = new byte[0x1f80];
        private const string IpAddress = "192.168.1.65";
        private readonly int _port;
        private readonly Socket _socket;
        private SslStream _sslStream;

        public abstract void Receive(byte[] buffer);
        public abstract void Error(Exception e);

        protected SecureStream(int port)
        {
            _socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            _socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true);
            _socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.DontLinger, true);
            _port = port;
        }

        public void Connect()
        {
            _socket.BeginConnect(new IPEndPoint(IPAddress.Parse(IpAddress), _port), Authenticate, null);
        }

        private async void Authenticate(IAsyncResult ar)
        {
            try
            {
                var onCertificateValidationCallBack =
                    new RemoteCertificateValidationCallback(
                        OnCertificateValidation);
                _sslStream = new SslStream(new NetworkStream(_socket, true), false, onCertificateValidationCallBack);

                await _sslStream.AuthenticateAsClientAsync(IpAddress);
                if (!_sslStream.IsAuthenticated) throw new Exception("not^authenticated");

                do
                {
                    var size = await _sslStream.ReadAsync(Buffer, 0, Buffer.Length);
                    if (size == 0) throw new Exception("invalid^size");
                    var buffer = new byte[size];
                    Native.MemoryCopy(Buffer, buffer, (uint)buffer.Length);
                    Receive(buffer);
                    Array.Clear(Buffer, 0, Buffer.Length);

                } while (_sslStream.CanRead);
            }
            catch (Exception e)
            {
                Error(e);
            }
        }

        internal async Task Send(PacketWriter writer)
        {
            var buffer = writer.GetWorkspace();
            await _sslStream.WriteAsync(buffer, 0, buffer.Length);
        }

        private static bool OnCertificateValidation(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslpolicyerrors)
        {
            return true;
        }
    }
}

CoreServer.cs

using System;
using System.Windows.Forms;

namespace Program.Connections.Network
{
    public sealed class CoreServer : SecureStream
    {
        public CoreServer(int p) : base(p) { }

        public override void Receive(byte[] buffer)
        {
            //TODO: Handle The Buffer
        }

        public override void Error(Exception e)
        {
            //TODO: Add Error Writer.
            MessageBox.Show(
                string.Format("Disconnected from the server."),
                string.Format("Connection Error"), MessageBoxButtons.OK,
                MessageBoxIcon.Error);
            Environment.Exit(1);
        }
    }
}

And i Simply Execute it using

var coreServer = new CoreServer(/*port*/);
coreServer.Connect();

new byte[0x1f80]

I think using decimal would be more readable here. And I don't understand why is your buffer 8096 bytes, usual sizes are powers of two, like 8192.

IpAddress = "192.168.1.65"

This looks like a perfect example of something that should be configurable, not set in stone. You should accept the IP address in the constructor, like you already do with port number.

public abstract void Receive(byte[] buffer);
public abstract void Error(Exception e);

I'm not sure I like the use of inheritance here. I think using events, or something like that would make more sense here.

_socket.BeginConnect(…)

I don't like that you're using the old Asynchronous Programming Model when you could (and do in other parts of your code) use async-await. Unfortunately, Socket doesn't expose awaitable methods, but there are ways around that.

private async void Authenticate(IAsyncResult ar)

I think Authenticate is not a very good name for this method, it does much more than just authentication.

throw new Exception("not^authenticated")

You shouldn't throw Exception, you should instead create a custom class that derives from Exception and use that. And the exception message should be human-readable, not some error code like it seems in your case. If you need to differentiate different errors, either have different exception types for them, or add a property for that to your custom exception.

if (size == 0) throw new Exception("invalid^size");

When ReadAsync() returns 0, it's not invalid size, it means the end of the stream was reached, so you should act accordingly.

Native.MemoryCopy(Buffer, buffer, (uint)buffer.Length);

Why are using some custom memory copying method here? Do you have some reason for not using Buffer.BlockCopy()? Also, why are you even doing the copying here? It seems you class is intended only for limited use (since you have a constant IP address), so couldn't you make sure the buffer isn't used after Receive() returns, so you didn't have to copy at all?

return true;

I'm no expert on security, but I think you should perform at least some checks of the certificate here. Otherwise, your class isn't really a “secure stream”.

MessageBox.Show(…);

You shouldn't mix code like this with GUI code, they should be separated.


Effective method to sort list in Java I am a beginner in Java and am learning collections. I have written the following code to sort the elements in lists.

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;

class descendingOrder implements Comparator<Integer>{
    @Override
    public int compare(Integer numberOne, Integer numberTwo) {
        return -numberOne.compareTo(numberTwo);
    }
}

public class SortsTests {

    public static void main(String[] args) {
        List<Integer> listOne = new ArrayList<>();
        listOne.add(1);
        listOne.add(5);
        listOne.add(10);
        listOne.add(6);
        System.out.println(listOne);
        Collections.sort(listOne, new descendingOrder());
        System.out.println(listOne);
    }
}

Is this the most effective way to sort the lists or are there any better methods?

There is already a reverse order Comparator built-in to Java. You could also use Arrays.asList(T...); like,

List<Integer> listOne = Arrays.asList((Integer) 1, 5, 10, 6);
System.out.println(listOne);
Collections.sort(listOne, Comparator.reverseOrder());
System.out.println(listOne);

And if you're using Java 8+, you could do the same with a Stream on one line (assuming you don't need to preserve the reverse sorted List) like

System.out.println(IntStream.of(1, 5, 10, 6).boxed().sorted(Comparator
            .reverseOrder()).collect(Collectors.toList()));


Implementation of Dijkstra's algorithm in Python I have implemented Dijkstra's algorithm for my research on an Economic model, using Python.
In my research I am investigating two functions and the differences between them. Every functions takes as input two parameters:
F(a,b) and Z(a,b).
Every cell of the matrix is defined as: $$M[a][b]=|F(a,b)-Z(a,b)|$$
The purpose of this is to find the path of minimal difference between the equations that will be correct for every input a
Online implementations of Dijkstra's algorithm were all using weighted edges whereas I have weighted vertices.
Pseudo-code:
function Dijkstra(Graph, source):

    create vertex set Q

    for each vertex v in Graph:
        dist[v] ← INFINITY
        prev[v] ← UNDEFINED
        add v to Q
    dist[source] ← 0

    while Q is not empty:
        u ← vertex in Q with min dist[u]

        remove u from Q

        for each neighbor v of u:           // only v that are still in Q
            alt ← dist[u] + length(u, v)
            if alt < dist[v]:
                dist[v] ← alt
                prev[v] ← u

    return dist[], prev[]

Input:

2d array where each cells value is its weight
source tuple (x, y)

Output:

distance matrix where each cell contains distance from source to vertex (i, j)

prev matrix where each cell contains its parent. By tracebacking from (98,98) I can find the shortest path.

Implementation:
MAX_DISTANCE = 99999
RANGE_ARR = [x for x in range(1, 1001)]

def dijkstra_get_min(Q, dist):
    min = MAX_DISTANCE + 1
    u = None
    for vertex in Q:
        if dist[vertex[0], vertex[1]] <= min:
            min = dist[vertex[0], vertex[1]]
            u = vertex
    return u

def dijkstra(graph, src=(0, 0)):
    dist = np.array([np.array([0 for x in RANGE_ARR], dtype=float) for y in RANGE_ARR])
    prev = np.array([np.array([(0, 0) for x in RANGE_ARR], dtype='i,i') for y in RANGE_ARR])
    Q = []

    for i in RANGE_ARR_0:
        for j in RANGE_ARR_0:
            dist[i, j] = MAX_DISTANCE
            prev[i, j] = (0, 0)
            Q.append((i, j))

    dist[0][0] = 0

    while Q:
        u = dijkstra_get_min(Q, dist)
        Q.remove(u)
        moves = [x for x in ( (u[0], u[1] + 1), (u[0] + 1, u[1]), (u[0] + 1, u[1] + 1) ) if x in Q]
        for v in moves:
            alt = dist[u[0]][u[1]] + graph[v[0]][v[1]]
            if alt < dist[v[0]][v[1]]:
                dist[v[0], v[1]] = alt
                prev[v[0], v[1]] = u
    return dist, prev

Any opinions about its correctness?

[x for x in range(1, 1001)] can be written as just list(range(1, 1001)).
It would be good to give that 1001 a name too.

Similarly, [0 for x in RANGE_ARR] can be written as [0] * len(RANGE_ARR). Multiplying any sequence type by an integer repeats the elements within the sequence. As a bonus, from some quick benchmarking that I just did, it's also more than 10x faster:
from timeit import timeit

N = int(1e6)
TRIALS = int(1e3)

print(timeit(lambda: [0] * N, number=TRIALS), "seconds")

print(timeit(lambda: [0 for x in range(N)], number=TRIALS), "seconds")

2.9889957 seconds
38.1463017 seconds

Be aware though that you should not use this when the element type is mutable (like [[0]] * 5). Multiplying a sequence creates multiple references to the same object; it doesn't make copies.

It looks like Q should be a set. You don't care about order, and the only thing you use it for is to track membership of a set. Sets will be significantly faster here. The only two changes needed are:
Q = set()

. . .

Q.add((i, j))

The only change I can see this making is dijkstra_get_min technically does rely on the order of Q. If two elements with the same minimum values are in Q, your algorithm picks the last instance. Since sets may use a different order, this may change what vertex gets returned.

It looks like MAX_DISTANCE is meant to be some arbitrarily large number that everything else will be less than. You may want to try using np.inf for that. By hardcoding the upper limit, you risk the problem "growing" later and potentially exceeding that max; causing erroneous behavior.


Java file analyzer byte-by-byte This is part of this topic.
Problem:
I need to read a file with BigIntegers and make some analysis "on-the-fly" with each read number (get prime numbers count, get armstrong numbers count). Right now I have this code, but it works very slow:

package ee.raintree.test.numbers;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.math.BigInteger;

import ee.raintree.test.numbers.utils.MathUtils;

public class FileAnalyzer {
    private static final char SEPARATOR = ' ';
    private File file;
    private int armstrongNumbersCount;
    private int primeNumbersCount;

    public FileAnalyzer(File file) {
        this.file = file;
    }

    public void analyze() throws FileNotFoundException, IOException {
        countNumbers();
    }

    public int getArmstrongNumbersCount() {
        return armstrongNumbersCount;
    }

    public int getPrimeNumbersCount() {
        return primeNumbersCount;
    }

    private void countNumbers() throws FileNotFoundException, IOException {
        StringBuilder numberSb = new StringBuilder();
        try (FileInputStream fis = new FileInputStream(file)) {
            char currentChar;
            while (fis.available() > 0) {
                currentChar = (char) fis.read();
                if (currentChar == SEPARATOR) {
                    analyzeNumber(new BigInteger(numberSb.toString()));
                    numberSb = new StringBuilder();
                    continue;
                }
                numberSb.append(currentChar);
            }
            if (numberSb.length() > 0) {
                analyzeNumber(new BigInteger(numberSb.toString()));
            }
        }
    }

    private void analyzeNumber(BigInteger number) {
        if(MathUtils.isArmstrongNumber(number)) {
            armstrongNumbersCount++;
        }
        if(MathUtils.isPrime(number)) {
            primeNumbersCount++;
        }
    }
}

How can I speed up this process? As I was informed - it is lack of buffering.

Replace

try (FileInputStream fis = new FileInputStream(file)) {

with

try (InputStream fis = new BufferedInputStream(new FileInputStream(file))) {

See also Why is using BufferedInputStream to read a file byte by byte faster than using FileInputStream?


Insertion sort revised with output to console I took the question Performing insertion sort in C# and tidied it up a bit, then I thought about the use of the arrays and was wondering if it worked, and it does.

So now I have 3 static methods in a simple Program class to run from the console, just to check 15 numbers quickly.  It's not very efficient.

What can be done to make it more efficient? I am assuming taking out the PrintIntegerArray from inside the PerformInsertionSort method for larger arrays would help with that.

class Program
{
    static void Main(string[] args)
    {
        int[] numbers = new int[15] {9,4,13,42,56,21,1,19,42,42,40,109,3,8,99};

        Console.WriteLine("Final OutPut");
        PrintIntegerArray(PerformInsertionSort(numbers));
        Console.ReadLine();
    }

    static int[] PerformInsertionSort(int[] inputArray)
    {
        for (int i = 0; i < inputArray.Length - 1; i++)
        {
            for (int j = i + 1; j > 0; j--)
            {
                if (inputArray[j - 1] > inputArray[j])
                {
                    int temp = inputArray[j - 1];
                    inputArray[j - 1] = inputArray[j];
                    inputArray[j] = temp;
                }
                PrintIntegerArray(inputArray);
            }
        }
        return inputArray;
    }

    public static void PrintIntegerArray(int[] array)
    {
        foreach (int i in array)
        {
            Console.Write(i.ToString() + ", ");
        }
        Console.WriteLine("End of Array");
        Console.ReadLine();
    }
}

One obvious optimization is to avoid many unnecessary swaps:

        int j;
        value_to_insert = inputArray[i];
        for (j = i; j > 0; j--)
        {
            if (inputArray[j - 1] > value_to_insert)
            {
                inputArray[j] = inputArray[j - 1];
            }
        }
        inputArray[j] = value_to_insert;

Just one assignment instead of 3.

Besides (sorry for repeating my mantra on a naked loops), now it becomes obvious that an inner loop implements some sort of a shift algorithm. Better to factor it out.

It is also very well possible that separating find and shift phases may also speed it up:

        value_to_insert = inputArray[i];
        int j = upper_bound(inputArray, inputArray + i, value_to_insert);
        shift(inputArray + j, input_array + i, 1);
        inputArray[j] = value_to_insert;

because both methods could be implemented as intrinsics.


Delta encoding a list of numbers Given a list of numbers, e.g.:

25626 25757 24367 24267 16 100 2 7277

Output a delta encoding for the sequence. In a delta encoding, the first
element is reproduced as is. Each subsequent element is represented as the
numeric difference from the element before it. E.g. for the sequence above,
the delta encoding would be:

25626 131 -1390 -100 -24251 84 -98 7275

However, if a difference value does not fit in a single signed byte,
i.e. -127 <= x <= 127, then, instead of the difference, we would like
to use an escape token, printing it.

This will denote that the value following the escape token is a full
four-byte difference value, rather than a one-byte different value.

For this exercise, we'll declare -128 as the escape token.

Following the same example above, the final result would be:

25626 -128 131 -128 -1390 -100 -128 -24251 84 -98 -128 7275

function getDelta(arr) {
  let diffs = [];
  for (let i = 1, N = arr.length -1; i <= N; ++i) {
    diffs.push(arr[i] - arr[i - 1]);
  }
  return diffs;
}

function encode(arr) {
  if (arr.length === 0) {
    return arr;
  }
  let encoding = [];
  let deltas = getDelta(arr);

  for (let delta of deltas) {
    if (127 < delta || delta < -127) {
      encoding.push(-128);
    }
    encoding.push(delta);
  }
  encoding.unshift(arr[0]);
  return encoding;
}

console.log(encode('25626'.split(' ')).join(' '));
console.log(encode('25626 25757 24367 24267 16 100 2 7277'.split(' ')).join(' '));

Another option is to iterate over the array of values using Array.reduce.  For me this makes its purpose more apparent.  Also it naturally handles a zero length array.

function encode(values) {
  return values.reduce(_computeDiff, {
    encoded: []
  }).encoded;
}

Here the initial accumulator value is a state value that contains the encoded output.  It will also be used to hold the lastValue.  The _computeDiff function can then update the state for each value.

function _computeDiff(state, value) {
  // lastValue initially undefined will produce diff NaN
  let diff = value - state.lastValue;

  // insert escape code for large differences
  if (Math.abs(diff) > 127) {
    state.encoded.push(-128);
  }

  state.encoded.push(isNaN(diff) ? value : diff);
  state.lastValue = value;
  return state;
}

This does rely on the fact that a math operation on an undefined value is NaN. Also it uses Math.abs to more succinctly describe the escape threshold.


Faster method for loading a hash table I'm looking to make my code faster for loading a hash table.  Any pointers would be appreciated.  Below are my includes, the node def and the loading code.

The dictionary loaded could be any *.txt file.  Its pre processed to contains just lower case words sans punctuation.

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

// define hash table structure
typedef struct node
{
    char word[LENGTH + 1];
    struct node* next;
}
node;

bool load(const char* dictionary)
{

    // open dictionary and check for NULL
    FILE* fpdict = fopen(dictionary, "r");

    if (fpdict == NULL)
    {
        printf("Could not open %s.\n", dictionary);
        unload();
        return false;
    }

    // read in txt file
    else
    {
        node* new_node = malloc(sizeof(node));
        for ( ; fscanf(fpdict, "%s", new_node->word) != EOF;
            new_node = malloc(sizeof(node)) )
        {
            // call hash function to get key
            key = hash(new_node->word, strlen(new_node->word), TABLESIZE);

            // check if hashtable key is NULL
            if (hashtable[key] == NULL)
            {
                hashtable[key] = new_node;
                new_node->next = NULL;
            }

            // if linked list exists, insert it in order
            else if (hashtable[key] != NULL)
            {
                // point to first hashtable entry
                node* head = hashtable[key];
                // set crawler to head
                node* crawler = head;
                // check where to insert

                // insert word at end and set pointer to NULL
                while (crawler->next != NULL)
                {
                    crawler = crawler->next;
                }
                new_node->next = NULL;
                crawler->next = new_node;

            }
            wordCount += 1;
        }
        fclose(fpdict);
        free(new_node);
        return true;
    }

    return false;
}

An ultimate speedup is achieved by preprocessing the dictionary one step further, into the binary format. Group the words of the same hashcode together, and prepare a TOC as an array of file offsets of each such group at the beginning of the file. This way the dictionary loads in a single fread() (you may also want to account for endianness with bit more sophisticated format).

I also found strange that you add nodes at the end of the list. Unless you have a very strong reason of doing so, crawling lists is a pure waste of time. Inserting node at the head unconditionally may - or may not - gain some performance, depending of the quality of the hash.

That said, I may only reiterate on the @Edward comment: profile the code.


Fibonacci number function: convert to tail-recursion? I've been implementing a function for calculating the nth Fibonacci number in F#.  So far the best implementation I could come up with is this:

let fib n =
    let rec fib =
        function
        | 0 -> 0I, 1I
        | n ->
            let f1, f2 = fib (n / 2)
            let f1' = f1 * (2I * f2 - f1)
            let f2' = f2 * f2 + f1 * f1
            if n % 2 = 0 then
                (f1', f2')
            else
                (f2', f1' + f2')
    fib n |> fst

Can it be improved or written in a more F#-idiomatic way?

Also, as a separate question, can it be rewritten to be tail-recursive?

A concise and idiomatic (I think) implementation (which happens to be tail recursive) but without your / 2 optimisation would be:

let fib n =
    let rec tail n1 n2 = function
    | 0 -> n1
    | n -> tail n2 (n2 + n1) (n - 1)
    tail 0I 1I n


Web app that splits a URL graphically I wrote this web app that splits a URL graphically so that its parts can be customized to create a new URL. https://github.com/ibrahim-islam/urlpad. The app currently works as expected for usual URls but I am not so sure about the code structure.

module UrlPad {

    const map = (o: OmniboxFull): CustomOmniboxMap => {
        return {
            protocol: o.protocol,
            host: o.host,
            port: o.port,
            pathnames: o.pathname.split('/').filter(char => char),
            hash: o.hash,
            query: o.query
        };
    }

    class FormBuilder {

        private template: string[] = [];
        private for: string = '';

        constructor(forClass?: string) {
            this.for = forClass;
            this.template.push('
');
        }

        label(title: string) {
            this.template.push(`<label class="col-sm-2">${title}</label>`);
            return this;
        }

        input(value: string, id?: string, size: number = 10) {
            let id_att = id ? 'id="'+ id +'"' : '';
            this.template.push(`

            <input ${id_att} spellcheck="false" type="text" class="form-control ${this.for}" value="${value}">
`);
            return this;
        }

        build() {
            this.template.push('
')
            return this.template.join('');
        }
    }

    export function parse(url: string): string {
        let o = map(omnibox.parse(url));

        return Object.keys(o)
            .filter((key, index) => o[key])
            .map((key, index) => {
                let value = o[key];
                switch (key) {
                    case 'query':
                        return Object.keys(value)
                            .map((key, index) => new FormBuilder('query').label(`Querystring #${index}`).input(key, '',  5).input(value[key], '', 5).build())
                            .join('');

                    case 'pathnames':
                        return value
                            .map((value, index) => new FormBuilder('path').label(`Pathname #${index}`).input(value).build())
                            .join('');

                    default:
                        return new FormBuilder().label(key).input(value, key).build();
                }
            })
            .join('');
    }

    export function make(o: CustomOmniboxMap): string {
        let querystrings = Object.keys(o.query)
            .map((key, index) => `${key}=${o.query[key]}`)
            .join('&'),
            paths = o.pathnames.join('/');

        return `${o.protocol}://${o.host}${o.port ? ':' + o.port : ''}/${paths}${querystrings ? '?' + querystrings : ''}${o.hash ? o.hash : ''}`;
    }

}

window["UrlPad"] = UrlPad;

I made the app related functionality into a module, should it have been a class?
map function is only used inside parse function, should it have been a local function?
When testing this app should I test only the exported functions or any class or function I wrote as well?

Any other observations or suggestions to improve the code is also welcomed.

For the most part, this looks good. However, there are a few things I'd recommend changing.

Don't use module. namespace should be used instead. From the Typescript Handbook:

  A note about terminology: It’s important to note that in TypeScript 1.5, the nomenclature has changed. “Internal modules” are now “namespaces”. “External modules” are now simply “modules”, as to align with ECMAScript 2015’s terminology, (namely that module X { is equivalent to the now-preferred namespace X {).

You do not properly escape special characters. If a double quote appears anywhere in the URL, the form will break. Try the url https://example.com/?a=" >Broke to see an example.
Related to the above, building HTML with strings is an incredibly error prone process. If at all possible, avoid it. It is a better idea to use the <template> tag.
You don't need omnibox browsers have a built in URL api which covers everything that omnibox does and more. If you need to support IE, you can use the url-polyfill package - and gain the advantage that newer browsers don't need to load it.
Consider using Object.entries instead of Object.keys as it can slightly simplify some of your code.

const querystrings = Object.entries(o.query)
    .map(([key, value]) => `${key}=${value}`)
    .join('&')

For your specific questions:

A module namespace is a reasonable choice for the app. I personally never use them, preferring to use ES Modules. A larger concern is that you export the module to the global scope unnecessarily. Instead of using onclick="parse()" in your HTML, it would be better to add the event handler in your index.ts file and have this module in it's own file.
I don't believe you even need the map function, it wouldn't be that big of a deal to check for pathname instead of pathnames in the switch statement and split the string there. If you do keep the map function, it should be renamed to something more descriptive. When I saw map I expected it to be roughly equivalent to Array.prototype.map.
For tests, it depends on who you ask. Since you aren't parsing the URL yourself (good!) there isn't really too much that can go wrong here. I'd probably only write a few integration tests for this app as a whole.


Scraping HTML via async controller & classes + HTML agility pack I've developed a simple application to grab golfer index scores from a website that has no API. The application works but is very slow, with 6 users that require updating takes 60 seconds. I've tried to make my web requests asyncronous to offset some of this lag but it only resulted in a 15% increase in performance.

Description of the code:

On my view I have anchor tag that when clicked hides all the elements in the DOM and loads a preloader, after the preloader is appended to the DOM a AJAX call is executed that calls the UpdatedHandicap method on a controller in my project. From there we await a static method GrabIndexValue. All the code works, it's just very slow.

Possible solution:

This website allows me to input multiple GHIN #s however the result set is in a table with strangely generated xpaths:

//*[@id="ctl00_bodyMP_gvLookupResults_ctl02_lblHI"] which returns index of: 10.5
//*[@id="ctl00_bodyMP_gvLookupResults_ctl03_lblHI"] which returns index of: 9
//*[@id="ctl00_bodyMP_gvLookupResults_ctl04_lblHI"] which returns index of: 13.5

I don't know how to dynamically grab those result sets and parse them properly. So I feel like I'm stuck doing 1 web request per index value.

Async controller method:

public async Task<ActionResult> UpdateHandicap()
{
    //Fetch all the golfers
    var results = db.Users.ToList();
    //Iterate through the golfers and update their index value based off their GHIN #. We store this
    //value in the database to make our handicap calculation
    foreach (Users user in results)
    {
        user.Index = await Calculations.GrabIndexValue(user.GHID);
        db.Entry(user).State = EntityState.Modified;
        db.SaveChanges();
    }
    return RedirectToAction("Index", "Users");
}

Method to actually grab the data:

public static async Task<double?> GrabIndexValue(int ghin)
{
    string url = $"http://xxxxxxxxx/Widgets/HandicapLookupResults.aspx?entry=1&ghinno="+ghin+"&css=default&dynamic=&small=0&mode=&tab=0";
    HtmlWeb web = new HtmlWeb();
    HtmlDocument htmlDoc  = await Task.Run(()=>web.Load(url));

    string index = null;
    try
    {
        index = htmlDoc.DocumentNode.SelectNodes("//*[@id=\"ctl00_bodyMP_grdClubs\"]/tbody/tr/td[2]")[0].InnerText;
    }
    catch
    { }

    if (index != null)
    {
        var indexResult = Convert.ToDouble(index);
        return indexResult;
    }
    else
    {
        return null;
    }
}

Any insight into this predicament is greatly appreciated

Thank you

public async Task<ActionResult> UpdateHandicap()

We add the Async suffix to methods that are marked with the async keyword. This is a naming convention that as you'll see in a moment Entity Framework follows too.

var results = db.Users.ToList();

You can use the Users directly in the loop, you don't have to call ToList first.

foreach (Users user in results)
{
    user.Index = await Calculations.GrabIndexValue(user.GHID);
    db.Entry(user).State = EntityState.Modified;
    db.SaveChanges();
}

The async calls are incomplete. SaveChanges would still block so you also want to use the

await db.SaveChangesAsync();

but do you really need to call SaveChanges in a loop? This could be bad for the performance. I think you should do it after the loop:

foreach (var user in db.Users)
{
    user.Index = await Calculations.GrabIndexValue(user.GHID);
    db.Entry(user).State = EntityState.Modified;
}
await db.SaveChangesAsync();


Filtering a list of tuples I have a list of tuples which may vary in size quite significantly. Typical number of tuples in the list would be somewhere around 100.000. I am trying to perform a down-selection on these tuples to end up with a more workable amount of them.

Each tuple contains (among other insignificant for this example data) an integer and 3 floats. These are of interest to me (shown bold below, positions 0, 2, 3, 4).

Example:

(100213, 'foo',-23.412, -277.11, 12.567, 12.24, 23.25, 150.14, 'bar', 'foobar')

And the criterion to implement is the following in pseudocode:

Assume tuples i and j:

if i[2] < j[2] and i[3] < j[3] and abs(i[4]) > abs(j[2]) then j leaves the list.

I wrote a couple algorithms for the task, timed them and improved them and finally I arrived here:

def clean3(lst):
    lst = sorted(lst, key=lambda x: abs(x[4]), reverse=True)  # the abs comparison is taken care off.
    remove = set()  # for quicker membership checks
    for i, forces_i in enumerate(lst[:-1]):
        if forces_i not in remove:  # the current entry might have already been removed
            for forces_j in lst[i+1:]:  # forward check only - progressive gains
                if forces_j not in remove: # the current entry might have already been removed
                    if all(forces_i[x] < forces_j[x] for x in range(2, 4)):  # check with `all` now possible
                        remove.add(forces_j)
    return list(set(lst) - remove)

To my eyes, this looks as streamlined as it could be, but:

Can it be further improved, and if yes, how?
Would an entirely different approach be better here?

The pattern for i in range(n): for j in range(i+1, n): ... is better handled using itertools.combinations:

from itertools import combinations

def clean3(lst):
    lst = sorted(lst, key=lambda x: abs(x[4]), reverse=True) # the abs comparison is taken care off.
    remove = set() # for quicker membership checks
    for forces_i, forces_j in combinations(lst, 2):
        if forces_j not in remove: # the current entry might have already been removed
            if all(forces_i[x] < forces_j[x] for x in range(2, 4)):
                remove.add(forces_j)
    return list(set(lst) - remove)

I also removed the test for forces_i in removed since I feel you could have cases where it would fail to remove some forces_j. But if I'm wrong it's very easy to put back on the same line than the test for forces_j.


Merge two sorted arrays together Please review my answer for this interview question:

#include <iostream>
#include <vector>

std::vector<int> merge2Sorted ( std::vector<int> left, std::vector<int> right )
{
  //finger matching algo

  auto itLeft = left.begin();
  auto itRight = right.begin();

  auto itLeftEnd = left.end();
  auto itRightEnd = right.end();

  std::vector<int> result;
  result.reserve(std::max(left.size(),right.size()));

  while ( itLeft != itLeftEnd &&
          itRight != itRightEnd )
  {
    if ( *itLeft < *itRight )
      result.push_back( *itLeft++ );
    else
      result.push_back( *itRight++ );
  }

  // copy rest of left array
  while ( itLeft != itLeftEnd )
    result.push_back(*itLeft++);

  // copy rest of right array
  while ( itRight != itRightEnd )
    result.push_back(*itRight++);

  return result;
}

int main ()
{
  std::vector<int> v1 = { 1,2,3,4,5,6,7,8,9,10 };
  std::vector<int> v2 = { -1,-2,0,3,7,9,11,12 };

  std::vector<int> v3 ( merge2Sorted ( v1, v2 ) );

  for ( auto& i: v3 )
    std::cout << i << std::endl;
}

Pass your parameters by const reference to avoid a copy:

std::vector<int> merge2Sorted(std::vector<int> const& left, std::vector<int> const& right)
//                                             ^^^^^^                        ^^^^^^

You are not reservng enough

result.reserve(std::max(left.size(),right.size()));

The result size will eventually be the size of the sum of the two input arrays.

result.reserve(left.size() + right.size());

Your test for left and right can be simplified:

if ( *itLeft < *itRight )
  result.push_back( *itLeft++ );
else
  result.push_back( *itRight++ );
}

// Simplified
// Though I am 50/50 on this one.
result.push_back( ( *itLeft < *itRight ) ? *itLeft++ : *itRight++);

No point in flushing the stream after every print:

std::cout << i << std::endl;

                  ^^^^^^^^^^  prefer '\n' unless you really want to force a flush.

std::cout << i << '\n';


Checking if a Chess space has been attacked I have this list of at least seven else-if. When their condition is met, I change the same boolean is_attacked to true. This should be simple but I can't figure out in what way I can achieve the same in a more readable way.

if(request_is_attacked){
    if(as_knight){
        if(piece_abs_val==2){//knight
            is_attacked=true;
        }
    }else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }
    }else if(piece_abs_val==5){//queen
        is_attacked=true;
    }else if(piece_direction%2){
        if(piece_abs_val==4){//rook
            is_attacked=true;
        }
    }else if(piece_abs_val==3){//bishop
        is_attacked=true;
    }else if(i==0 && piece_abs_val==1){
        if(piece_val!=-1){//w_pawn
            if(piece_direction==4 || piece_direction==6){
                is_attacked=true;
            }
        }else{//b_pawn
            if(piece_direction==2 || piece_direction==8){
                is_attacked=true;
            }
        }
    }
}

For more context, this is happening inside a loop (hence the i), when the i is the first it means it is one square apart (i==0), so Pawns and Kings use this, but this information shouldn't matter much.

Also, when piece_direction%2 is true, we are moving in a + (that's why I check for Rooks), else it means we are in moving x direction, so I proceed to check for Bishops and Pawns.

Switch your code up

We can make this much more readable with a switch statement.

switch(piece_abs_val) {
    case 1:
        if(piece_val != -1){ //w_pawn
            if(piece_direction == 4 || piece_direction == 6){
                is_attacked = true;
            }
        } else { //b_pawn
            if(piece_direction == 2 || piece_direction == 8){
                is_attacked = true;
            }
        }
        break;
    case 2:
        is_attacked = as_knight;
        break;
    case 3:
        is_attacked = true;
        break;
    case 4:
        is_attacked = piece_direction % 2;
        break;
    case 5:
        is_attacked = true;
        break;
    case 6:
        is_attacked = i == 0;
        break;
}

Same exact code only much cleaner. I also did some refactoring. Take 6 for example; it used to be this:

else if(piece_abs_val==6){//king
        if(i==0){
            is_attacked=true;
        }

In the switch, piece_abs_val is checked for 6. Then, is_attacked is set to whatever i==0 evaluates to. However, instead of using a conditional, I just simply wrote the statement out in the variable setting.

Magic!

Your code is littered with magic numbers. These are bad.

What you need here is a makeshift enumerated type. In case you don't know, an enumerated type is basically a type that assigns plain texts words (not strings) to integer values. I say "makeshift" because JavaScript does not have native support for these values.

Judging from your comments, I was able to make this:

var Piece = {
    KNIGHT: 2,
    KING:   6,
    ...
}

You can fill in the rest. Then, when in need of checking the value of piece, don't use the number; use this "enum". For example:

if(piece_abs_val==Piece.KNIGHT){

This also removes the need for those annoying comments.

You should also do this for your piece_directions.

This is what your code now looks like:

var Piece = {
    PAWN: {
        BLACK: -1,
        WHITE: 1
    },
    KNIGHT: 2,
    BISHOP: 3,
    ROOK:   4,
    QUEEN:  5,
    KING:   6
}

switch(piece_abs_val) {
    case Piece.PAWN:
        if(piece_val != Piece.PAWN.BLACK){
            if(piece_direction == 4 || piece_direction == 6){
                is_attacked = true;
            }
        } else {
            if(piece_direction == 2 || piece_direction == 8){
                is_attacked = true;
            }
        }
    case Piece.KNIGHT:
            is_attacked = as_knight
    case Piece.BISHOP:
            is_attacked = true;
    case Piece.ROOK:
            is_attacked = piece_direction % 2;
    case Piece.QUEEN:
            is_attacked = true;
    case Piece.KING:
            is_attacked = i == 0;
}


Same records between two tables I have the following tables:

                 player
| id         | name        | surname      |
|:-----------|------------:|:------------:|
| 1001       |    Mike     |    Tyson     |
| 2001       |    John     |    Carlin    |
| 3001       |    Dan      |    StanH     |
| 4001       |    Alen     |     Derl     |

                  person
| id         | name        | surname      |
|:-----------|------------:|:------------:|
| 1001       |    Mike     |    Tyson     |
| 2001       |    Pirlon   |    Austin    |
| 6001       |    Danly    |    StanH     |
| 4001       |    Alen     |     Derl     |

I want create an SQL query in order to get the same records of the above schemes and print first and last name.

The query is

SELECT player.name, player.surname
   FROM  player INNER JOIN person
     on  player.id =  person.id
    AND  player.name = person.name
    AND  player.surname = person.surname;

Is there any way to create a more efficient query?

SQL is made to function on Joins, I wouldn't say that they are "kind of "expensive" operation" RDMS databases are made to relate to other tables.  if you had Primary and Foreign Keys set up and/or even indexes then it shouldn't be an "expensive operation".
edit
here is the best query for what you are trying to accomplish.
SELECT player.name, player.surname
FROM  player INNER JOIN person ON player.id = person.id
WHERE player.id = person.id
    AND player.name = person.name
    AND player.surname = person.surname

This is the write way to write the code, it's clean and straight forward to what is going on. your code essentially is doing the exact same thing, but is bad coding practice from my point of view.
SQL Fiddle Statistics and running code
SQL Fiddle Statistics for your code
(it actually looks like the execution is the same.)
I need to Note that both this query and the query that you are using join the two tables on their id columns.  this is an issue because there is 1 id that holds different values in either table. so those id columns mean two totally separate things and shouldn't be joined on in all reality.
example:

player table
| 2001       |    John     |    Carlin    |

AND

person table
| 2001       |    Pirlon   |    Austin    |

in this specific instance the where statement covers the id being different, in this one case.  BUT YOU NEED TO BE AWARE THIS IS BAD DATA  you should fix the database.
Sorry to say but the INTERSECT query that has been suggested actually takes longer and has more operations attached to it then both your original code and the code I posted.
SQL Fiddle from jmoreno's answer


Adding fade to jquery background position animate I've just implemented a background animate on some social media icons where the image goes from grey to color on :hover.

I wanted to know if there's a better way to write the following but also implement a fade, so as the background animates, it's also fading in on hover.

    <script type="text/javascript">

$(function(){
    $('#facebook')
        .css( {backgroundPosition: "0 0"} )
        .mouseover(function(){
            $(this).stop().animate({backgroundPosition:"(-63px 0px)"}, {duration:150})
        })
        .mouseout(function(){
            $(this).stop().animate({backgroundPosition:"(0 0)"}, {duration:150})
        })
    $('#twitter')
        .css( {backgroundPosition: "0 0"} )
        .mouseover(function(){
            $(this).stop().animate({backgroundPosition:"(-63px 0)"}, {duration:150})
        })
        .mouseout(function(){
            $(this).stop().animate({backgroundPosition:"(0 0)"}, {duration:150})
        })
});
</script>

I wanted to know if there's firstly a better way to write the following

Well, not much to be improved but you can simplify things a little bit using hover()

$('#facebook')
.css('background-position', '0 0')
.hover(function () {
    $(this).stop().animate({
        'background-position' : '(-63px 0px)'
    }, 150);
}, function () {
    $(this).stop().animate({
        'background-position' : '(0 0)'
    }, 150);
});

  implement a fade so as the background animates it's also fading in on hover

Your best bet is to implement it with two separate images instead of a sprite. You absolute position each image on top of each other, animate their top for the moving effect, and then fadeIn()|fadeOut().


Create a calendar application to check the scheduling conflicts of meetings The purpose here is to find if there is a conflict in the given set of start and end times of the meeting time slots listed.

Input: Start and end times of meetings

Output: True / False for conflicts

package code.Misc;

import java.util.ArrayList;

public class Calendar {

static int time [] = new int[1440];

public static void main(String args[]){

    String s[] =   {"15:30","1:30","1:15","15:00"};
    String e[] = {"18:35","13:28","1:30","15:29"};

    System.out.println(scheduler(s, e));
}

public static int calculateMinute(int hour, int minute){
        return hour*60+minute;
}

public static boolean timeClashChecker(int start, int end){
        for(int i=start;i<=end;i++){
            if(i==end){
                time[i] =  -1;
            }
            else if(time[i]==1){
                return false;
            }
            else if(time[i]==-1){
                time[i]=1;
            }
            else{
                time[i]=1;
            }
        }
    return true;
}
public static int parser(String time){
    String [] timeParts = time.split(":");
    int hour = Integer.parseInt(timeParts[0].trim());
    int minute = Integer.parseInt(timeParts[1].trim());

    return calculateMinute(hour,minute);
}
public static boolean scheduler(String start[], String end[]){

    for(int i=0;i<start.length;i++){
        int startMeeting = parser(start[i]);
        int endMeeting = parser(end[i]);
        boolean result = timeClashChecker(startMeeting,endMeeting);
        if(result == false){ //clash check
            return false;
        }
    }
    return true;
}
}

A couple of things that would be good to point out:

It appears that your code only checks the clash between the start and end time. If the start time is always bigger than the end time, in theory it should never clash.

for(int i=0;i<start.length;i++){
    int startMeeting = parser(start[i]);
    int endMeeting = parser(end[i]);
    boolean result = timeClashChecker(startMeeting,endMeeting);
    if(result == false){ //clash check
        return false;
    }
}

might be replaced with:

for(int i=0;i<start.length;i++){
    int startMeeting = parser(start[i]);
    int endMeeting = parser(end[i]);
    for(int e = 0; e < end.length; e++){
        if(e == i) continue;
        int meetingEnd = parser(end[e]);
        if(meetingEnd > startMeeting && meetingEnd < endMeeting) return true;
    }

    for(int s = 0; s < start.length; s++){
        if(s == i) continue;
        int meetingStart = parser(start[s]);
        if(meetingStart > startMeeting && meetingStart < endMeeting) return true;
    }

}

This way the end of each meeting is checked with each starting time. If there is a clash with any of them, it should detect it. It also checks if each starting time of another is also between a meeting.

If you make the change above, your timeClashChecker method is unnecessary.
Some of the method names are not entirely helpful.parser could become parseTime, and scheduler could be changed to doesClash, especially seeing it doesn't do any of the scheduling itself.