Rigidness and verbose nature of XML parsing I've been developing a simple console application in C++ using the TinyXML2 library, and I honestly can't help but feel what I'm doing is not very robust. Maybe that's just the nature of parsing XML (this is my first foray into the topic, so I've no idea.)

The following code works to the best of my knowledge, but it feels like there has to be a better way than manually navigating to each element, checking for null pointers, then grabbing the information. Is there a more elegant way to handle this? I have to be missing something within the documentation, because I feel like I'm writing a lot of redundant code here.

#include <iostream>
#include "tinyxml2.h"

int main() {
    tinyxml2::XMLDocument document;

    if (document.LoadFile("games.xml") == tinyxml2::XML_SUCCESS) {
        std::cout << "'games.xml' successfully loaded." << std::endl;

        tinyxml2::XMLElement *root = document.RootElement();

        std::cout << root->Value() << std::endl;

        tinyxml2::XMLElement *child;

        for (child = root->FirstChildElement(); child != nullptr; child = child->NextSiblingElement()) {
            std::cout << "\t" << child->Value() << std::endl;

            if (child->FirstChildElement("name") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            if (child->FirstChildElement("system") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            if (child->FirstChildElement("status") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            const char* game_name = child->FirstChildElement("name")->GetText();
            const char* game_system = child->FirstChildElement("system")->GetText();
            const char* game_status = child->FirstChildElement("status")->GetText();

            if (game_name == nullptr || game_system == nullptr || game_status == nullptr) {
                std::cout << "Error: null pointer." << std::endl;
                return 1;
            }
            std::cout << "\t\t" << game_name << std::endl;
            std::cout << "\t\t" << game_system << std::endl;
            std::cout << "\t\t" << game_status << std::endl;
        }
    } else {
        std::cerr << "Unable to load 'games.xml'. Error code: " << document.ErrorID() << std::endl;
        return 1;
    }
    return 0;
}

In your code I do agree there more redundancy than I like to see. I would suggest writing some helper functions, and perhaps leveraging exceptions instead of return codes (I won't show that here).

const char* FirstChildElementText(tinyxml2::XMLElement *parent, const char *child_name)
{
    auto child = parent->FirstChildElement(child_name);
    return (child != nullptr) ? child->GetText() : nullptr;
}
: : :
for (child = root->FirstChildElement(); child != nullptr; child = child->NextSiblingElement()) {
    auto game_name = FirstChildElementText(child, "name");
    auto game_system = FirstChildElementText(child, "system");
    auto game_status = FirstChildElementText(child, "status");

    if (game_name == nullptr || ...)
    : : :
}

This helps a bit. Always be on the lookout for a good refactoring. Even if you already found a better XML library for your needs.

(I have things I dislike about both DOM and SAX style XML parsing, either being too lenient or too much work. Really, all in all, I suspect that directly working with XML is approaching things from the wrong level. But I don't have a great solution, so I'll get off my soap box.)


OpenCV Mat processing time I'd like to know whether having different variables for the src (source) and dst (destination) of an OpenCV function will have an effect on the processing time.  I have two functions below
that does the same thing.

public static Mat getY(Mat m){
    Mat mMattemp = new Mat();
    Imgproc.cvtColor(m,mMattemp,Imgproc.COLOR_YUV420sp2RGB);
    Imgproc.cvtColor(mMattemp,mMattemp, Imgproc.COLOR_RGB2HSV);
    Core.inRange(mMattemp, new Scalar(20, 100, 100), new Scalar(30, 255, 255), mMattemp);
    return mMattemp;
}

VERSUS

public static Mat getY(Mat m){
    Mat mMattemp_rgb = new Mat();
    Mat mMattemp_hsv = new Mat();
    Mat mMattemp_ir = new Mat();
    Imgproc.cvtColor(m,mMattemp_rgb,Imgproc.COLOR_YUV420sp2RGB);
    Imgproc.cvtColor(mMattemp_rgb,mMattemp_hsv, Imgproc.COLOR_RGB2HSV);
    Core.inRange(mMattemp_hsv, new Scalar(20, 100, 100), new Scalar(30, 255, 255), mMattemp_ir);
    return mMattemp_ir;
}

Which of the two is considered best-practice? What is the advantage of one over the other?

Just looking over the two functions, I would probably say the first function is a better practice.  It's shorter (meaning it's more maintainable), it uses fewer variables (which can cause less confusion), and does the same thing as the second function. Overall the advantage is that the first function is just shorter.

In terms of speed/efficiency (if I had to guess), it looks like the first function would also be a bit faster as well (though not by very much at all). To know for sure which one is faster, test the function calls with OpenCV's methods double getTickCount() and double getTickFrequency().

getTickCount gives you the number of clock cycles after a certain event, e.g., after machine is switched on.

A = getTickCount()  // number of clock cycles from beginning (for example, 100)
getY(image)      // do whatever process you want
B = getTickCount()  // number of clock cycles from beginning (for example, 150)

C = B - A           // number of clock cycles for processing (150-100 = 50)

Now you want to know how many seconds are these clock cycles. For that, you want to know how many seconds a single clock takes, i.e. clock_time_period. If you find that, simply multiply by 50 to get total time taken.

For that, OpenCV gives second function, getTickFrequency(). It gives you frequency, i.e. how many clock cycles per second. You take its reciprocal to get time period of clock.

time_period = 1/frequency.

Now you have time_period of one clock cycle, multiply it with 50 to get total time taken in seconds.


CSV parsers and OrderedDictionary I wrote the following (static) class to read files easily in C#. However, it is a static class, so I wonder if there would be a gain to make it a non static method (and therefore, passing arguments as the separator and the path as attributes of the class).

Besides, the OrderedDictionary class may not be a good choice (it is not generic but it preserves the order of the columns), and I don't know if there is a nice workaround for this.

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.IO;

namespace MySolution.FileUtils
{
    /// <summary>
    /// Various helpers to read through the lines of a file.
    /// </summary>
    public static class LinesEnumerator
    {
        public static string ReadFirstLine(string path)
        {
            using (FileStream fs = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
            using (StreamReader sr = new StreamReader(fs))
            {
                return sr.ReadLine();
            }
        }

        /// <summary>
        /// Enumerates the lines of a file.
        /// </summary>
        /// <param name="path">The path of the file.</param>
        /// <param name="maxLines">The maximum number of lines to read.</param>
        /// <returns>The lines of a file, as a IEnumerable</returns>
        public static IEnumerable<string> LinesReader(string path, int maxLines = Int32.MaxValue)
        {
            string line;
            int lineNumber = 0;
            using (FileStream fs = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
            using (StreamReader sr = new StreamReader(fs))
            {
                while ((line = sr.ReadLine()) != null && lineNumber < maxLines)
                {
                    lineNumber++;
                    yield return line;
                }
            }
        }

        /// <summary>
        /// Reads the lines of a CSV file and enumerates them through a csv file
        /// </summary>
        /// <param name="path">Path to the CSV file</param>
        /// <param name="separator">Separator used by the CSV file</param>
        /// <returns>A collection of ordered dictionaries, corresponding to the lines of the file</returns>
        public static IEnumerable<OrderedDictionary> DictReaderCSV(string path, char separator)
        {
            using (FileStream fs = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
            using (StreamReader sr = new StreamReader(fs))
            {
                string[] header = sr.ReadLine().Split(separator);
                string line = "";
                while ((line = sr.ReadLine()) != null)
                {
                    string[] splittedLine = line.Split(separator);
                    OrderedDictionary od = new OrderedDictionary();
                    for (int i = 0; i < header.Length; i++)
                        od.Add(header[i], splittedLine[i]);
                    yield return od;
                }
            }
        }
    }
}

Naming Items

You should avoid abbreviations like Dict in DictReaderCSV, spelling it Dictionary is cleaner.

Next, it's much more natural to use nouns for class names and verbs for methods. As such, I would name your class CSVDictionaryReader and your main method EnumerateEntries(). LinesEnumerator didn't give any hint that the class is also responsible for parsing CSV.

Unused code

ReadFirstLine and ReadFirstLine don't seem to be used in the current code.

Streams

The StreamReader class already provides constructors that receive a disk path, so no need to manually create a FileStream just to pass it onto the StreamReader.

What would however give much more flexibility is for the class to allow the user to provide it with an already constructed Stream or TextReader. Perhaps the user wants to parse a 100 MB CSV as it's arriving over the wire, without writing it to the disk.

Parsing CSV

CSV files allow for quotes to be used so that entries can contain the separator. A parser should be prepared to correctly deal with this.

Id,Name,Age
1,John,20
2,"Jack, the second", 21


Knapsack class in Ruby This is a simple implementation of a knapsack written in Ruby.  I've included all the relevant code here, but my question is concerning the contents setter .contents=

## item.rb
class Item
  attr_accessor :weight
  attr_accessor :data

  def initialize(weight:, data: nil)
    @weight = weight
    @data   = data
  end

end

 ## knapsack.rb
class KnapsackCapacityExceededError < StandardError; end
class KnapsackWeightExceededError < StandardError; end
class KnapsackContentError < StandardError; end

require_relative('./item')

class Knapsack
  attr_reader :capacity
  attr_reader :weight
  attr_reader :contents

  def initialize(capacity:, weight:)
    @capacity = capacity
    @weight   = weight
    @contents = Array.new(@capacity) { nil }
  end

  def contents=(new_contents)
    raise KnapsackCapacityExceededError if self.exceeds_capacity? new_contents
    raise KnapsackWeightExceededError   if self.exceeds_weight? new_contents
    raise KnapsackContentError          unless new_contents.all? { |e| e.is_a? Item }

    @contents = new_contents
  end

  def fit?(contents)
    return false if self.exceeds_weight?(contents) || self.exceeds_capacity?(contents)
    true
  end
  alias_method :fits?, :fit?

  def fits_weight?(contents)
    new_weight = contents.map { |item| item.weight }.sum
    return true if new_weight <= self.weight
    false
  end

  def exceeds_weight?(contents)
    return true if !fits_weight? contents
    false
  end

  def fits_capacity?(contents)
    return true if contents.length <= self.capacity
    false
  end

  def exceeds_capacity?(contents)
    return true if !fits_capacity? contents
    false
  end

end

# k = Knapsack.new(capacity: 10, weight: 50)

In this method there are three conditions where an exception is raised, is this a code smell?

Any other feedback also appreciated.

So there are multiple things. First of all I'll share the code refactoring related suggestions and then we can look at the performance improvements of code.

Refactoring

You don't really need to pass a block to Array.new to initialize the array with nil values.

def initialize(capacity:, weight:)
  @capacity = capacity
  @weight   = weight
  @contents = Array.new(@capacity) { nil }
end

can be refactored to:

def initialize(capacity:, weight:)
  @capacity = capacity
  @weight   = weight
  @contents = Array.new(@capacity)
end

def contents=(new_contents)
  raise KnapsackCapacityExceededError if self.exceeds_capacity? new_contents
  raise KnapsackWeightExceededError   if self.exceeds_weight? new_contents
  raise KnapsackContentError          unless new_contents.all? { |e| e.is_a? Item }

  @contents = new_contents
end

can be refactored to:

def contents=(new_contents)
  raise KnapsackCapacityExceededError if exceeds_capacity? new_contents
  raise KnapsackWeightExceededError   if exceeds_weight? new_contents
  raise KnapsackContentError          if has_non_items? new_contents

  @contents = new_contents
end

def has_non_items?(contents)
  contents.any? { |content| !content.is_a?(Item) }
end

-> Notice that usage of self keyword has been removed.

-> Moved the logic of checking any non-items in contents to a separate method. That makes the if conditions more readable and it replaces the unless with if to make it consistent with rest of the conditions.
Method fit? can be refactored to:

def fit?(contents)
  fits_weight?(contents) && fits_capacity?(contents)
end

Method fits_weight? can be refactored as well.

def fits_weight?(contents)
  contents.map(&:weight).sum <= weight
end

Performance Tuning

You don't need to initialize the @contents array with nil elements. You can just write @contents = [] to save some extra used memory.


Implementing a Singleton pattern in C# Is this the best way to implement this pattern in C#?

public sealed class Singleton
{
    private static readonly Singleton instance = new Singleton();
    public static Singleton Instance { get { return instance; } }

    static Singleton() {}
    private Singleton() {}
}

I use Jon Skeet's version of a thread safe Singleton with fully lazy instantiation in C#:

public sealed class Singleton
{
    // Thread safe Singleton with fully lazy instantiation á la Jon Skeet:
    // http://csharpindepth.com/Articles/General/Singleton.aspx
    Singleton()
    {
    }

    public static Singleton Instance
    {
        get
        {
            return Nested.instance;
        }
    }

    class Nested
    {
        // Explicit static constructor to tell C# compiler
        // not to mark type as beforefieldinit
        static Nested()
        {
        }

        internal static readonly Singleton instance = new Singleton();
    }
}

It works wonders for me! It's really easy to use, just get the instance from the Instance property like so; SingletonName instance = SingletonName.Instance;


Converting the comma seprated numeric string into int array //convert the comma separated numeric string into the  array of int.
public class HelloWorld
{
  public static void main(String[] args)
   {
     // line is the input  which have the comma separated number
     String line = "1,2,3,1,2,2,1,2,3,";
     // 1 > split
     String[] inputNumber =  line.split(",");
    // 1.1 > declear int array
     int number []= new int[10];
     // 2 > convert the String into  int  and save it in int array.
 for(int i=0; i<inputNumber.length;i++){
       number[i]=Integer.parseInt(inputNumber[i]);
  }
 }

}

Is there a more efficient solution to achieve the same result?

Basic improvements

Instead of setting the size of the int array to 10, it would be better to derive the right size from the size of String array
Instead of int number[] the more conventional way to write is int[] number
For structures that contain multiple values it's more natural to give plural names, for example "number" -> "numbers" for an array of numbers
The variable names are very poor in general, and should be improved to better reflect their purpose, making the code easier to understand

Something like this:

String line = "1,2,3,1,2,2,1,2,3,";
String[] parts = line.split(",");
int[] ints = new int[parts.length];
for (int i = 0; i < parts.length; i++) {
    ints[i] = Integer.parseInt(parts[i]);
}

Split to logical steps

It's good to get into the habit of decomposing tasks to their small steps. That is, instead of having all the logical steps in a single main method, it would be better to split to multiple functions, for example:

static int[] toIntArray(String[] arr) {
    int[] ints = new int[arr.length];
    for (int i = 0; i < arr.length; i++) {
        ints[i] = Integer.parseInt(arr[i]);
    }
    return ints;
}

static int[] parseLineToIntArray(String line) {
    return toIntArray(line.split(","));
}

public static void main(String[] args) {
    String line = "1,2,3,1,2,2,1,2,3,";
    System.out.println(Arrays.toString(parseLineToIntArray(line)));
}


Counting number of pairs in an integer array So, I wanted to have the count for number of pairs of same integer there are in an array for which I used a HashMap

Input contains two lines:

  Integer n:

  Signifies the size of input
  Input numbers separated by white space character:
  E.g 4 6 6 7 7 8 7 6 4

For the same input,

  Expected Output:

  3

import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;

public class SockMerchant {

    public int[] returnsockArr() {
        Scanner sc = new Scanner(System.in);
        int arraySize = sc.nextInt();
        int[] sockArr = new int[arraySize];
        for(int i = 0;i <= arraySize-1; i++) {
            sockArr[i] = sc.nextInt();
        }
        sc.close();
        return sockArr;
    }

    public int returnCount(Map<String, Object> mapCount) {
        int count = 0;
        for(String keyName : mapCount.keySet()) {
            int value = (int)mapCount.get(keyName);
            count = count+(value/2);
        }
        return count;
    }

    public int setMapAndReturnCount(int[] sockArr) {
        Map<String, Object> mapCount = new HashMap<String, Object>();
        for(int j = 0;j<= sockArr.length-1;j++) {
            if(mapCount.containsKey(Integer.toString(sockArr[j]))) {
                mapCount.put(Integer.toString(sockArr[j]), (int)mapCount.get(Integer.toString(sockArr[j]))+1);
            }
            else {
                mapCount.put(Integer.toString(sockArr[j]), 1);
            }
        }
        return returnCount(mapCount);
    }

    public static void main(String args[]) {
        SockMerchant sm = new SockMerchant();
        int[] sockArr = sm.returnsockArr();
        int finalCount = sm.setMapAndReturnCount(sockArr);
        System.out.println(finalCount);
    }
}

I think that I have complicated the solution a bit too much

Is there a better approach to do the same thing?

There are couple of improvement possible in your code.

In this method setMapAndReturnCount you should change the declaration of map from,

Map<String, Object> mapCount = new HashMap<String, Object>();

to

Map<Integer, Integer> mapCount = new HashMap<Integer, Integer>();

As you know, you want to store the input array numbers as key in Map and value is the count of occurence of a particular number in the array.

By using Object class as value in Map, you are defeating one of the main purpose of having generics implementation in Java. If you use Object then you need to uselessly cast, and otherwise don't have to.

Another point in same method, you can write your for loop in a better way like this, instead of your current code which does double work of first checking through containsKey and then uses mapCount.get to again pick the value of key.

public int setMapAndReturnCount(int[] sockArr) {
    Map<Integer, Integer> mapCount = new HashMap<Integer, Integer>();
    for (int j = 0; j < sockArr.length; j++) {
        Integer count = mapCount.get(sockArr[j]);
        if (count == null) {
            mapCount.put(sockArr[j], 1);
        } else {
            mapCount.put(sockArr[j], count + 1);
        }
    }
    return returnCount(mapCount);
}

Similarly, you can change returnCount method to this,

public int returnCount(Map<Integer, Integer> mapCount) {
    int count = 0;
    for (Integer value : mapCount.values()) {
        count += (value / 2);
    }
    return count;
}

As you don't need to first iterate on keySet and then retrieve the values with get which will be slower, and instead just iterate it on values as that is what you need for calculating number of pairs, which will be relatively faster.

Also, if you are using Java-8 and above,

You can change your returnCount to one liner like this,

public int returnCount(Map<Integer, Integer> mapCount) {
    return mapCount.values().stream().mapToInt(x -> x / 2).sum();
}


Repeated Strings Hacker Rank challenge I'm doing a hacker rank challenge with this one called "Repeated String". I attempted this challenge in my weaker language java. The goal is to print out the occurrences of the letter a. First you're given a string "aba" then a number 10. The new string generated would be abaabaabaa and you need to count the a. The solution below works but is it as simple as it should be? I've looked around and my solution looks good but still I'm terrified I'm over complicating this.

// Complete the repeatedString function below.
static long repeatedString(String s, long n) {
    final long quotient = n/s.length();
    final long remainder = n%s.length();
    long aCount = 0;

    if((s.length() == 1) && (s.charAt(0) == 'a')){
        return n;
    }
    for(int i=0; i<s.length(); i++){
        if(s.charAt(i) == 'a'){
            aCount++;
        }
    }
    aCount = aCount * quotient;
    for(int i=0; i<remainder; i++){
        if(s.charAt(i) == 'a'){
            aCount++;
        }
    }
    return aCount;
}

If your goal is to optimize efficiency, you can do a little better by only looking at each character in the string once. The first remainder characters of the string will occur quotient + 1 times. The remaining characters will occur quotient times. So you can loop once from 0 to remainder and do some math, then loop from remainder + 1 to s.length() and do some math, and you'll have your result. It's unclear without testing whether it's faster to do the multiplication outside or to add the correct amount in your loop. Also, your short-circuit for inputs of length one is not helpful unless you expect that most of the inputs will be a single-character. It's probably safe to pull it.

If your goal is to avoid overcomplicating things, websites like HackerRank may not be ideal. They tend to provide algorithmic challenges that reward hyper-optimized code. By their nature, those kinds of solutions will be complicated. It turns out it's hard to write an AI that will grade readable code, but easy to write one that will grade performant code. So even though readable code is much more desirable, performant code is what scores well.


Random alphanumeric password generator with GOTOs I was inspired after this thread on Stack Overflow to create a random 8 character alphanumeric password generator.

Sadly, it is closed, so I cannot provide an answer there. Anyway, here is the code. Let me know if there are any bugs/bias going on.

using System.Collections.Generic;
using System.Security.Cryptography;

public class AlphanumericGen
{
    public virtual string Generate(byte length)
    {
        ICollection<char> chars = new LinkedList<char>();
        var buffer = new byte[1];
        short counter = 0;
        using (var rng = new RNGCryptoServiceProvider())
        {
            while (counter < length)
            {
                rng.GetBytes(buffer);
                var nextChar = (char)buffer[0];

                if (nextChar >= '0' && nextChar <= '9')
                {
                    goto addChar;
                }

                if (nextChar >= 'A' && nextChar <= 'Z')
                {
                    goto addChar;
                }

                if (nextChar >= 'a' && nextChar <= 'z')
                {
                    goto addChar;
                }

                continue;

            addChar:
                chars.Add(nextChar);
                ++counter;
            }
        }

        return string.Concat(chars);
    }
}

As a general rule of thumb, any time you feel the need to use goto, take a couple of aspirin and lay down until the feeling passes.  They probably should have been deprecated decades ago.

In this particular case, using a string of allowed characters and randomly picking an index in that string would do away with your goto's.

I think too a StringBuilder would do a lot better for storing the characters than a LinkedList.


Aggregating clients and client types The code I have put together achieves what I want, but I don't know if it is the right way to go about tackling the problem.

I am dealing with 2 tables (CLIENT, CLIENTTYPE) each of which I have an auto-generated entity for in my models folder. As I don't actually want to send all the data from each entity back to the user, I have created a viewmodel for each in a ViewModels folder.

A CLIENT can have multiple CLIENTTYPE, so I have created the a List property of ClientTypeViewModels in my ClientViewModel

    public ClientsWithTypesViewModel getClientAndTypesByCode(string code)
    {
        var x =
            from c in _context.Cases
            where c.ClientID == code
            select new ClientsWithTypesViewModel
            {
                ClientID = c.ClientID,
                Title = c.Title,
                Name = c.FullName,
                ClientTypes = (from w in _context.ClientTypes where w.ClientType == c.ClientType select new ClientTypeViewModel { ClientType = w.ClientType, Description = w.Description }).ToList()
            };
        return x.FirstOrDefault();
    }

You should not need to populate the ClientTypes property with your own query. Usually it is handled by a navigation property that already knows how to handle related tables but for this to work you first need to set the foreign key properly. Since you did not post the models I cannot say if it would be possible in your case.

You can read this article explaining how it should work: Don’t use Linq’s Join. Navigate!

Then you should be able to load the relations with Include:

var x =
    from c in _context.Cases.Include("ClientTypes")
    where c.ClientID == code
    select c;

See also Loading Related Objects


Velocity verlet implementation My code is meant to update the positions and velocities of planets. There are a few methods in this class, but updatePosition and updateVelocity are the most important, as they implement the velocity verlet algorithm. Could it be shorter/more efficient?

public class Particle{

private double mass;
private PhysicsVector initialPosition = new PhysicsVector();
private PhysicsVector initialVelocity = new PhysicsVector();

public PhysicsVector updatePosition(PhysicsVector initialPosition, PhysicsVector initialVelocity, double timeStep, PhysicsVector aDueToGravity){
    PhysicsVector x = new PhysicsVector(initialVelocity);
    x.scale(timeStep);
    PhysicsVector z = new PhysicsVector(aDueToGravity);
    z.scale(0.5*timeStep*timeStep);

    initialPosition.increaseBy(x);
    initialPosition.increaseBy(z);
    return initialPosition;
}

public PhysicsVector updateVelocity(PhysicsVector initialVelocity, double timeStep, PhysicsVector a, PhysicsVector newA){
    PhysicsVector z = new PhysicsVector(newA);
    z.increaseBy(a);
    z.scale(0.5*timeStep);
    initialVelocity.increaseBy(z);
    return initialVelocity;
}

public PhysicsVector[] centreOfMass(double[] mass, PhysicsVector[] positions){
    PhysicsVector centreOfMass = new PhysicsVector();
    centreOfMass.setVector(0,0);
    double sum = ArrayLogic.sumArray(mass);
    for(int i=0;i<positions.length;i++){
        centreOfMass.increaseBy(positions[i].scale(mass[i],positions[i]));
    }
    centreOfMass.scale(1/sum);
    positions = ArrayLogic.subtract(positions, centreOfMass);
    return positions;
}

public PhysicsVector[] cOMVel(double[] mass, PhysicsVector[] velocity){
    PhysicsVector cOMV = new PhysicsVector();
    double total = ArrayLogic.sumArray(mass);
    for(int ae=0;ae<velocity.length;ae++){
        cOMV.increaseBy(velocity[ae].scale(mass[ae],velocity[ae]));
    }
    cOMV.scale(1/total);
    velocity = ArrayLogic.subtract(velocity, cOMV);
    return velocity;
}

public static PhysicsVector[] momentum(double[] planetMass, PhysicsVector[] velocity, PhysicsVector[] positions){
    PhysicsVector[] momentum = new PhysicsVector[planetMass.length];
    PhysicsVector[] angularMomentum = new PhysicsVector[planetMass.length];
    PhysicsVector a = new PhysicsVector(0,0);
    for(int i=0; i<velocity.length; i++){
        a = a.scale(planetMass[i], velocity[i]);
        momentum[i] = a;
    }

    for(int i=0; i<velocity.length; i++){
        angularMomentum[i] = angularMomentum[i].vectorProduct(positions[i], momentum[i]);
    }

    return angularMomentum;
   }

}

At the moment, increasing the timestep seems to improve my results. This seems odd, which is partly why I suspect my implementation is incorrect.

In writing a class model, it can be useful to consider whether a class represents a value (something that doesn't change, but can be useful in multiple contexts), or an entity (something with an identity - a sense of self - that doesn't change even though the values associated with that identity do change).

In your fragment here, I would expect Particle to be an entity (Earth is still Earth even while it moves through its orbit), and PhysicsVector to be a value.

What does this mean for your implementations?

Entities are usually an encapsulation of some rules describing how their current state becomes the next state.  You don't change the state of an entity directly - instead you command it to change, giving it the arguments that it needs to do the work, and the entity determines its own next state.

public class Particle{

    private double mass;
    private PhysicsVector initialPosition = new PhysicsVector();
    private PhysicsVector initialVelocity = new PhysicsVector();

Quick review here - these describe the state of the particle, and you've made them private, which is consistent with the idea that the Particle should control its own state.  mass should probably be final -- you could name it restMass to be more clear.  (It might not be final, depending on how you want to model inelastic collisions).

initialPosition and initialVelocity are probably incorrectly named -- the "initial" state of an object shouldn't change later? so currentPosition/currentVelocity would make things clearer, although since the entity is always holding its current state, position and velocity would probably suffice.

My guess is that your model will want to be able to specify the starting configuration of particles, so you should have a constructor that allows them to do so -- in other words, we are constructing the entity in its initial state.

public class Particle{
    // ...
    public Particle(double restMass, PhysicsVector initialPosition, PhysicsVector initialVelocity) {
        this.mass = restMass;
        this.position = initialPosition;
        this.velocity = initialVelocity;

One constructor can reference another, so if you intend to support the creation of particles in an arbitrary initial state, you create a second contructor that calls this first:

    public Particle(double restMass) {
        this(restMass, new PhysicsVector(), new PhysicsVector());

Now, as we said we send commands to an entity to tell it to change its state.  The entity already knows its own state, so we don't need to pass that in.  So this code doesn't really make sense

public PhysicsVector updatePosition(PhysicsVector initialPosition, PhysicsVector initialVelocity, double timeStep, PhysicsVector aDueToGravity){
    PhysicsVector x = new PhysicsVector(initialVelocity);
    x.scale(timeStep);
    PhysicsVector z = new PhysicsVector(aDueToGravity);
    z.scale(0.5*timeStep*timeStep);

    initialPosition.increaseBy(x);
    initialPosition.increaseBy(z);

    return initialPosition;
}

Either this is a member function (meaning, being run against a specific entity), in which case it should be using the state of the entity at the time it is run:

public void updatePosition(double timeStep, PhysicsVector aDueToGravity){
    PhysicsVector x = new PhysicsVector(velocity);
    x.scale(timeStep);
    PhysicsVector z = new PhysicsVector(aDueToGravity);
    z.scale(0.5*timeStep*timeStep);

    position.increaseBy(x);
    position.increaseBy(z);

    return initialPosition;
}

or instead it's more a convenient calculation service; you can make it clear that it isn't really scoped to the entity by declaring the method to be static (class scope)

public static PhysicsVector updatePosition(PhysicsVector initialPosition, PhysicsVector initialVelocity, double timeStep, PhysicsVector aDueToGravity){
    // ...
}

Or alternatively you can perhaps see in it a time step calculator that is independent of the Particle

public class Accelerator {
    private final double timeStep;
    private final PhysicsVector acceleration;

    public PhysicsVector nextPosition(PhysicsVector currentPosition, PhysicsVector currentVelocity) {
        // ...
    }

public class Particle {
    public void updatePosition(Accelerator accelerator) {
        position = accelerator.nextPosition(position, velocity);

I strongly prefer this approach -- gravity, in non uniform fields, depends on where you are, so you are going to need the position to calculate the force (and then the mass to scale it to the right acceleration).

Your calculator for momentum definitely looks more like a service that should be operating on a collection of particles, than it does a member function of a single particle

public static PhysicsVector[] momentum(Particle [] particles)

ValueTypes are typically immutable - subtracting the position of the earth from the position of the sun doesn't move either particle, but instead gives you a new vector describing the relative position between the two.

We don't send commands to valueTypes, we send them queries, "what do we get if?" kinds of things.  Running the query doesn't change the value (it is immutable), it just returns a result.  So if we want to use vector addition to update the position of the particle, we should expect to use a replace with query result pattern

position = position.increaseBy(x);

Given that what we are doing is vector addition, I'd probably prefer that increaseBy was instead using the spelling add.

Other modeling considerations - PhysicsVector, as a type, is a bit vague; it would be very easy to initialize a particle the wrong way round by ordering your position and velocity incorrectly.  You could make PositionVector distinct from VelocityVector; this would also guard against bugs where you try to add two different vectors that have different units.  Depending on the model, you might also want to distinguish PolarVectors and AxialVectors,


Toggling a class in JS I'm toggling the class of an element with this code but I'm pretty sure it can be written more efficiently.

$('.expandable_header').click(function() {
    toggle_expandable_icon($(this).find('i')[0]);
    $(this).nextUntil('tr.expandable_header').slideToggle(100, function(){
    });
});

function toggle_expandable_icon(elem) {
    var toOpen = 'fa-plus-circle';
    var toClose = 'fa-minus-circle';
    if (elem.classList.contains(toOpen)) {
        elem.classList.replace(toOpen, toClose);
    } else {
        elem.classList.replace(toClose, toOpen);
}};

Not pointless to say js is not my mother tongue.

Use toggle methods

Presuming that you are just toggling those two classes, you could just use classList.toggle() with those two class names:

function toggle_expandable_icon(elem) {
    elem.classList.toggle('fa-plus-circle');
    elem.classList.toggle('fa-minus-circle');
}

And it appears jQuery is used, so the .toggleClass() method could be used on a jQuery collection (i.e. remove the [0] in toggle_expandable_icon($(this).find('i')[0]);):

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle').toggleClass('fa-minus-circle');
}

Those class names could also be added together in a single toggleClass() call:

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle fa-minus-circle');
}

Demo

Without knowing the HTML structure it was challenging to know exactly how the toggles operated, but it wasn't very difficult to generate something.

$('.expandable_header').click(function() {
    toggle_expandable_icon($(this).find('i'));
    $(this).nextUntil('tr.expandable_header').slideToggle("slow");
});

function toggle_expandable_icon(collection) {
    collection.toggleClass('fa-plus-circle fa-minus-circle');
}

<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.8.2/css/all.css" rel="stylesheet" />

                                                                                Zombies exist!


Rust program, which reads, processes, and writes a file I'm making a small program to benchmark Rust's performance compared to some other languages. The idea I came up with, was to take a file (with the numbers 1 to 10000000 written to it in separate lines) as a command line argument:

1
2
3
...
9999999
10000000

read the file, and then multiple every line in that file by two and write those back to the same file, so the end result would look like this:

2
4
6
...
19999998
20000000

My code works, and does exactly what it should do. I am especially looking for ways how I could make the code cleaner while also making it run faster. I'm currently running it with:

$ cargo run --release numbers.txt

The program:

use std::env;
use std::fs::OpenOptions;
use std::io::Read;
use std::io::Write;

fn main() {
    let args: Vec<String> = env::args().collect();
    let file_path = &args[1];

    let mut file = OpenOptions::new().read(true).open(file_path).unwrap();
    let mut contents = String::new();
    file.read_to_string(&mut contents).unwrap();
    let vec: Vec<i32> = contents.lines()
        .map(|line| line.trim().parse::<i32>().unwrap() * 2)
        .collect();

    let mut file = OpenOptions::new().write(true).open(file_path).unwrap();
    for line in vec {
        write!(file, "{}\n", line).unwrap();
    }
}

This is my first post here, but I thought I might give this one a shot.

Your code could be made more user friendly by providing custom error messages.

When you get the filename from the command line arguments, you assume that the user will provide at least one command line argument. You could provide a helpful error message if they don't.
If opening a file fails for some reason (probably that the file doesn't exist at all), you could provide a custom print-out.
My example uses expect() as an example, but in code where user-friendliness is actually a goal, you would probably want to use eprintln!("error message here") and std::process::exit(1) to quit the program. That way, you don't get rust's extra panic messages which a user shouldn't have to understand.

You don't actually have to allocate a Vec to read the command line arguments, you can use the .nth method on the Args iterator.
You don't need the trim() call, because the lines() iterator already removes the newlines.
If you are looking to make the code cleaner, you could open the file for reading with File::open, and open it for writing with File::create.
I don't know how to significantly improve the performance of your code.

The biggest thing that I notice is that when you write the processed sequence of numbers to the file, you call write! on every line. This means that you talk to the OS every time you write a number. This could be made faster by using BufWriter, which would let you talk to the file only once. Alternatively, you could construct your output string while you are reading the file.

use std::env;
use std::fs::File;
use std::io::{BufReader, BufRead};
use std::io::Write;

fn main() {
    let file_path = &env::args().nth(1).expect("must provide an input file");

    let file = File::open(file_path).expect("failed to open input file");

    let doubled_contents: String = BufReader::new(file).lines()
        .map(|line| {
            let number = line.unwrap().parse::<i32>().expect("failed to parse file");
            format!("{}\n", number*2)
        })
        .collect();

    let mut output_file = File::create(file_path).expect("failed to open output file");
    write!(output_file, "{}", doubled_contents).expect("could not write output");
}


LeetCode 124: Binary Tree Maximum Path Sum 2 I'm posting my code for a LeetCode problem. If you'd like to review, please do so. Thank you for your time!
Problem
Given a non-empty binary tree, find the maximum path sum.
For this problem, a path is defined as any sequence of nodes from some starting node to any node in the tree along the parent-child connections. The path must contain at least one node and does not need to go through the root.
Example 1:
Input: [1,2,3]

       1
      / \
     2   3

Output: 6

Example 2:
Input: [-10,9,20,null,null,15,7]

   -10
   / \
  9  20
    /  \
   15   7

Output: 42

Inputs
[1,2,3]
[-10,9,20,null,null,15,7]
[-10,9,20,null,null,15,7,9,20,null,null,15,7]
[-10,9,20,null,null,15,7,9,20,null,null,15,720,null,null,15,7,9,20,null,null,15,7]
[-10,9,20,null,null,15,7,9,20,null,null,15,720,null,null,15,7,9,20,null,null,15,7999999,20,null,null,15,7,9,20,null,null,15,720,null,null,15,7,9,20,null,null,15,7]

Outputs
6
42
66
791
8001552

Code
# Definition for a binary tree node.
# class TreeNode:
#     def __init__(self, val=0, left=None, right=None):
#         self.val = val
#         self.left = left
#         self.right = right

class Solution:
    def maxPathSum(self, root: TreeNode) -> int:
        def depth_first_search(node: TreeNode) -> int:
            if not node:
                return 0
            left_sum = depth_first_search(node.left)
            right_sum = depth_first_search(node.right)
            if not left_sum or left_sum < 0:
                left_sum = 0
            if not right_sum or right_sum < 0:
                right_sum = 0
            self.sum = max(self.sum, left_sum + right_sum + node.val)
            return max(left_sum, right_sum) + node.val

        self.sum = float('-inf')
        depth_first_search(root)
        return self.sum

References

Problem

Discuss

Solution

Unnecessary checks in depth_first_search()
The function depth_first_search() always returns an integer value, never None. So the check for a partial sum being None or < 0 can be rewritten using max():
left_sum = max(0, depth_first_search(node.left))
right_sum = max(0, depth_first_search(node.right))


Search a unique list and search a larger list to set dictionary with keys of binary values I would like to search a list of unique values and create a dictionary with those unique values as keys, and a binary list for whether those values occur.

I have successfully solved this, but would like a more efficient solution. This solution performs a high amount of operations for a search list of size 1000, 10000, and 1000000.

This code is fully reproducible.

import numpy as np

# Create random array, len(10), of integers 1-5

array_to_search = np.array(np.random.randint(0,5,(1,10)))
array_to_search = array_to_search.flatten()

#array_to_search = [0 3 0 0 0 2 1 3 3 0]

# Extract each unique value from array_to_search

array_of_unique_values = np.unique(array_to_search)

# array_of_unique_values = [0 1 2 3]

# Create a dictionary of unique values
output = dict.fromkeys(array_of_unique_values)

# Declare lists as values to the dictionary

output = { k : [] for k in array_of_unique_values}

# For each value in unique values, search the larger list and set the dictionary
# value to a 1 if found, 0 if not found.

for i in array_of_unique_values:
    for j in array_to_search:
        if i == j:
            output[i].append(1)
        else:
            output[i].append(0)

output = {0: [1, 0, 1, 1, 1, 0, 0, 0, 0, 1],
          1: [0, 0, 0, 0, 0, 0, 1, 0, 0, 0],
          2: [0, 0, 0, 0, 0, 1, 0, 0, 0, 0],
          3: [0, 1, 0, 0, 0, 0, 0, 1, 1, 0]}

== with broadcasting is the most straightforward solution:

array_to_search = np.array(np.random.randint(0,5,(1,10))).ravel()
array_of_unique_values = np.unique(array_to_search)

# Creates the binary vectors
matches = array_of_unique_values[:, None] == array_to_search

# Converts to dict
output = dict(zip(array_of_unique_values, matches))

You can also set return_indices flag to True in np.unique and use these to create the table:

array_to_search = np.array(np.random.randint(0,5,(1,10))).ravel()
array_of_unique_values, indices = np.unique(array_to_search, return_inverse=True)
matches = np.zeros((array_of_unique_values.size, array_to_search.size), dtype=bool)
matches[indices, np.arange(array_to_search.size)] = True
output = dict(zip(array_of_unique_values, matches))


Too slow JSON data processing using SQL I have a case study where I need to take data from a REST API do some analysis on the data using an aggregate function, joins etc and use the response data in JSON format to plot some retail graphs.

Approaches being followed till now:

Read the data from JSON store these in python variable and use insert to hit the SQL query. Obviously, it is a costly operation because for every JSON line read it is inserting into the database. For 33k rows it is taking more than 20 mins which is inefficient.
This can be handled in elastic search for faster processing but complex operation like joins are not present in elastic search.

If anybody can suggest what would be the best approach (like preprocessing or post-processing in python) to follow for handling such scenarios it would be helpful.

SQL Script

def store_data(AccountNo)

        db=MySQLdb.connect(host=HOST, user=USER, passwd=PASSWD, db=DATABASE, charset="utf8")
        cursor = db.cursor()
        insert_query = "INSERT INTO cstore (AccountNo) VALUES (%s)"
        cursor.execute(insert_query, (AccountNo))
        db.commit()
        cursor.close()
        db.close()
        return

def on_data(file_path):
        #This is the meat of the script...it connects to your mongoDB and stores the tweet
        try:
           # Decode the JSON from Twitter
            testFile = open(file_path)

            datajson = json.load(testFile)
            #print (len(datajson))

            #grab the wanted data from the Tweet
            for i in range(len(datajson)):
                for cosponsor in datajson[i]:
                    AccountNo=cosponsor['AccountNo']
                    store_data( AccountNo)

Here are a few things you can do to improve the performance of the data loading:

convert the JSON file to CSV and use LOAD DATA to load from a file (sample). It is probably the fastest way to do what you are trying to do.
use .executemany() instead of .execute():

datajson = json.load(testFile)

insert_query = """
    INSERT INTO
        cstore (AccountNo)
    VALUES (%(AccountNo)s)
"""
cursor.executemany(insert_query, datajson)

look into disabling/removing existing indexes during the insertion and then re-creating them after the insertion is done
make sure you are not doing the data-load "over the internet" and there is no network-latency and bandwidth impact, be "closer" to the database server
loading the JSON file is probably not a bottleneck, but you may also look into faster JSON parsers like ujson
I remember we also had some data-loading-performance-related success with the ultra-fast umysql Python database driver, but it looks like the package has not been maintained for quite a while


Bash script for an ABC organization fulfilling the following requirements Write a bash script for an ABC organization fulfilling the following requirements:

Takes an Employee's Name, designation, age, years of service and salary, extra working hours as an input.
Calculates remaining years of service. An Employee retires at the age of 60.
Check if the Employee is valid for salary appraisal. If an employee has been working for more than an year he deserves to be given an appraisal of 10%.
If an employee has given extra working hours within a month to the organization calculate bonus amount.
Per hour amount varies depending upon the designation. See the given table to calculate bonus according to designations.
Designation Per hour-amount
Higher-Management   1K
Lower-management    800PKR
Other staff 500 PKR
Print the Employee’s information along with bonus and appraisal amount

read -p "Enter employee name: " ename # printing message and scanning variable

read -p "Enter designation: " designation # printing message and scanning variable

read -p "Enter age: " age # printing message and scanning variable

read -p "Enter years of service: " years # printing message and scanning variable

read -p "Enter salary: " salary # printing message and scanning variable

read -p "Enter extra working hours: " hours # printing message and scanning variable

years_remaining=$(( 60-age )) # calculating remaining years

#code to calculate appraisal

if [ "$years" -gt 1 ] # if condition

then

appraisal=$(( salary/10 )) # appraisal value

else

appraisal=0 # appraisal value

fi # end of if statement

#code to calculate the bonus

if [ "$designation" = "Higher-Management" ] # if condition

then

bonus=$(( 1000*hours )) # calculating bonus

elif [ "$designation" = "Lower-management" ] # else if condition

then

bonus=$(( 800*hours )) # calculating bonus

elif [ "$designation" = "Other staff" ] # else if condition

then

bonus=$(( 500*hours )) # calculating bonus

else

bonus=0

fi # end of if statement

echo "Employee name: $ename" # printing message

echo "Age: $age" # printing message

echo "Designation: $designation" # printing message

echo "Years of service: $years" # printing message

echo "Salary: $salary" # printing message

echo "Extra working hours: $hours" # printing message

echo "Bonus: $bonus" # printing message

echo "Appraisal: $appraisal" # printing message

echo "years_remaining:$years_remaining " # printing message

Better comments
You have put a comment on almost every line that just restates what the code already says and does not add any information or clarification.
This is a bad practice!
A comment that states "this is an if statement" makes the code less readable, by doubling the length of a line with no added value.
When adding a comment, you should only do so if it provides information that is not in the code, or is difficult to extract from the code given average knowledge of the programing language.
Input validation
It is customary to check all user input before using it in your code.
Unless you have been explicitly instructed to assume that the input is valid, you should add some code, for example to make sure age is actually a number.
Also, you may want to check for cases where age >= 60 i.e. the employee is about to retire, or has retired already.
Better flow control
The long if elif else tree where you calculate the bonus should be replace with a case statement.
This will make the code shorter, more readable, and make adding or removing bonus options easier.
Also, store bonus multiplier in a variable to avoid repeating code:
case $designation in
    "Higher-Management")
        hour_bonus=1000
        ;;

    "Lower-management")
        hour_bonus=800
        ;;

    "Other staff")
        hour_bonus=500
        ;;

     *)
        hour_bonus=0
        ;;
esac

bonus=$((hours * hour_bonus))

Note, that you could also add shopt -s nocasematch before the case block to make the comparison case insensitive, so that if your user forgets to type capital letters (or forgets the caps lock on) your code can still match the designation to the proper bonus value.


C# 5 Async Await .Task.Factory.StartNew cancellation I have the async code that implements cancellation token. It's working but Im not pretty sure if this is the right way to do it so I just want feedback about it.

Here is the actual code:

/// <summary>
///
/// </summary>
private async void SaveData() {

    if (GetActiveServiceRequest() != null)
    {
        var tokenSource = new System.Threading.CancellationTokenSource();

        this.ShowWizardPleaseWait("Saving data...");

        var someTask = System.Threading.Tasks.Task<bool>.Factory.StartNew(() =>
        {

            bool returnVal = false;

            // Set sleep of 7 seconds to test the 5 seconds timeout.
            System.Threading.Thread.Sleep(7000);

            if (!tokenSource.IsCancellationRequested)
            {
                // if not cancelled then save data

                App.Data.EmployeeWCF ws = new App.Data.EmployeeWCF ();
                returnVal = ws.UpdateData(_employee.Data);
                ws.Dispose();
            }

            return returnVal;

        }, tokenSource.Token);

        if (await System.Threading.Tasks.Task.WhenAny(someTask, System.Threading.Tasks.Task.Delay(5000)) == someTask)
        {
            // Completed
            this.HideWizardPleaseWait();
            if (someTask.Result)
            {
                this.DialogResult = System.Windows.Forms.DialogResult.OK;
            }
            else
            {
                this.DialogResult = System.Windows.Forms.DialogResult.Abort;
            }
            btnOK.Enabled = true;
            this.Close();
        }
        else
        {
            tokenSource.Cancel();

            // Timeout logic
            this.HideWizardPleaseWait();
            MessageBox.Show("Timeout. Please try again.")
        }

    }
}

Does async / await / cancellation code is well implemented?

It's better to use CancellationTokenSource(TimeSpan) constructor to set the cancellation after 5 seconds.

Also, Task.Run method is a recommended way to run compute-bound tasks (see remark here).

Other issues worth noting:

by conventions asynchronous methods should have a suffix Async
using keyword is a recommended way to correctly dispose IDisposable objects
if a task to be cancelled cannot return correct results (according to business requirements) it's recommended to throw OperationCanceledException by calling token.ThrowIfCancellationRequested() method.

private async void SaveDataAsync()
{
    if (GetActiveServiceRequest() == null)
        return;

    var cancellationToken = new CancellationTokenSource(TimeSpan.FromSeconds(5)).Token;
    this.ShowWizardPleaseWait("Saving data...");

    var someTask = Task.Run<bool>(() =>
    {
        // Set sleep of 7 seconds to test the 5 seconds timeout.
        System.Threading.Thread.Sleep(7000);

        cancellationToken.ThrowIfCancellationRequested();

        // if not cancelled then save data
        using (App.Data.EmployeeWCF ws = new App.Data.EmployeeWCF())
        {
            return ws.UpdateData(_employee.Data);
        }
    }, cancellationToken);

    try
    {
        this.DialogResult = await someTask
            ? DialogResult.OK
            : DialogResult.Abort;

        btnOK.Enabled = true;
        this.Close();
    }
    catch(OperationCanceledException)
    {
        MessageBox.Show("Timeout. Please try again.")
    }
    finally
    {
        this.HideWizardPleaseWait();
    }
}


Number of Google search results over a period of time, saved to database I am writing a Python script that scrapes data from Google search results and stores it in a database. I couldn't find any Google API for this, so I am just sending a HTTP GET request on Google's main site (and also Google News site). Then, using Beautiful Soup, I extract the number of search results found. I store this number in a mongo database using pymongo.

I want to collect the number of search results for specific keywords for next few months and then visualize the data. I would want something like this in the end:

Here is my code and I will run this following script four times a day using cron:

#! /bin/python

import re
import datetime

import pymongo
from pymongo import MongoClient
import requests
from bs4 import BeautifulSoup

REGEX = r'About (.*) results'
keywords = ['Barack Obama', 'Gandhi', 'Putin']

def number_of_search_results(key):
    def extract_results_stat(url):
        headers = {
            'User-Agent': 'Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20100101 Firefox/21.0'
        }
        search_results = requests.get(url, headers=headers, allow_redirects=True)
        soup = BeautifulSoup(search_results.text)
        result_stats = soup.find(id='resultStats')
        m = re.match(REGEX, result_stats.text)
        # print m.group(1)
        return int(m.group(1).replace(',',''))

    google_main_url = 'https://www.google.co.in/search?q=' + key
    google_news_url = 'https://www.google.co.in/search?hl=en&gl=in&tbm=nws&authuser=0&q=' + key
    return (extract_results_stat(google_main_url), extract_results_stat(google_news_url))

if __name__ == '__main__':
    conn = MongoClient()
    db = conn['search_results']
    current_time = datetime.datetime.utcnow()
    for key in keywords:
        google_main, google_news = number_of_search_results(key)
        # print key, google_main, google_news
        db.search_results.insert({'time': current_time, 'name': key, 'google_main': google_main, 'google_news': google_news})

I am new to databases, so is my mongo schema good for my project?
Any general improvements?

If you just need the number of search results, you can surely use what you're using now and grep the total result number, otherwise you may use Google Search API, or find an open source scrawling tool like Scrappy.


Correct semantic html5 I would be thankful for feedback for following html5 markup.
The outer div (class articles) has no more meaning as setting flex for it's child elements. Is a div the semantically correct choice in this case?

.articles {
  display: flex;
}

.articles__article {
  flex: 1;
}

Headline

Text Text Text

Headline

Text Text Text

Yes, in this case it is. As you stated yourself it has "no more meaning". It only exists for the purpose of styling.

This is what the MDN has to say about div:

  The HTML Content Division element () is the generic container for
  flow content. It has no effect on the content or layout until styled
  using CSS.

  As a "pure" container, the  element does not inherently represent
  anything. Instead, it's used to group content so it can be easily
  styled using the class or id attributes, marking a section of a
  document as being written in a different language (using the lang
  attribute), and so on.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div


Toggle chart series in clojurescript I'm toggling on/off series in chart and was wondering if I was doing anything crazy.

Is the atom really a good idea here?

;; ================ Graph data ================

(def serie1 {:label "Bar" :data [[11 13] [19 11] [30 -7]]})
(def serie2 {:label "Foo" :data [[10 1] [17 -14] [30 5]]})
(def serie3 {:label "Zon" :data [[8 2] [18 7] [30 15]]})

(def series (for [serie [serie1 serie2 serie3]]
              {:label (:label serie) :action "toggle-serie" :param serie}))

;; ============ Chart state ================

(def !chart
  "Chart state"
  (atom []))

(defn toggle-serie! [serie]
  "Return the series collection minus/plus the given serie"
  (swap! !chart #(if (some #{serie} %)
                   (remove #{serie} %)
                   (conj % serie))))

(defn reset-chart []
  "Reset chart state"
  (reset! !chart []))

;;================ Page population ================

; almost identical to ibdknox overtone example
(defpartial button [{:keys [label action param]}]
  [:a.button {:href "#" :data-action action :data-param param} label])

(defn populate [container buttons]
  (doseq [b buttons]
    (append container (button b))))

(populate ($ :#series) series)

;; ================ Button functions ================

(defn plot
  "Plot the given series and return the chart as an object."
  [placeholder coll-data & options]
  (js/jQuery.plot placeholder (clj->js coll-data) (clj->js options)))

(def chart ($ :#chart))

(defn toggle-serie-and-update-chart!
  "Add or remove a serie form the chart atom and redraw the chart."
  [serie]
  (toggle-serie! serie) ;update the chart atom
  (plot chart @!chart)) ; redraw the chart

(delegate ($ :#series) button :click
          (fn [e]
            (.preventDefault e)
            (this-as me
                     (let [$me ($ me)
                           param (cljs.reader/read-string (data $me :param))]
                       (toggle-serie-and-update-chart! param)))))

If your dataset were coming from an outside source, like an API call, I'd say the atom would make sense.  In this case, your dataset is static.  In that case, I think I might find it more intuitive to use a var for the series and then select the items out that you want to plot.   In that case, your state is really just a list of which series (you are selecting).

So, if you had all your series:

(def all-series
  {:series1 :...series1
   :series2 :...series2
   :series3 :...series3})

!chart might contain #{:series1 :series3} and you could get all the values out using (select-keys all-series @!chart).

You might find you can avoid the atom altogether, but at the very least I'd always look for ways to keep that state as simple as


Find diagonal positions for bishop movement In a chess board, I need to find the diagonals a bishop can move on, and more specifically, the coordinates of those squares. So, given a grid of any size and a position in that grid (expressed in coordinates within the grid), I have to compute the coordinates of the diagonals of that initial position.

I'm using zero-based indexing, and the (row, column) notation for coordinates.

For example, on a 8x8 grid, with starting position of (0, 0), the returned list should be [(0, 0), (1, 1), (2, 2), (3, 3), (4, 4), (5, 5), (6, 6), (7, 7)].

On a 8x8 grid, with starting position of (3, 4), the returned list should be [(3, 4), (2, 3), (1, 2), (0, 1), (4, 5), (5, 6), (6, 7), (4, 3), (5, 2), (6, 1), (7, 0), (2, 5), (1, 6), (0, 7)]

This is my working program in Python 3:

def diagonals(coord, size):
    limit = size - 1
    coords = [coord]
    row = coord[0]
    col = coord[1]

    while row > 0 and col > 0:
        row -= 1
        col -= 1
        coords.append((row, col))

    row = coord[0]
    col = coord[1]

    while row < limit and col < limit:
        row += 1
        col += 1
        coords.append((row, col))

    row = coord[0]
    col = coord[1]

    while row < limit and col > 0:
        row += 1
        col -= 1
        coords.append((row, col))

    row = coord[0]
    col = coord[1]

    while row > 0 and col < limit:
        row -= 1
        col += 1
        coords.append((row, col))

    return coords

coord = (3, 4)
size = 8
print(diagonals(coord, size))

Depending on the diagonal (4 cases), row and column are added or subtracted by one until the last square is reached, and everything is kept in a list, which in the end is returned.

It works, but it left me wondering if there's a simpler, different, better way of doing this, probably using linear algebra or something? And what about idiomatically, how can this be more pythonic?

The biggest problem you have is you have five lines that are nearly identical, four times.

row = coord[0]
col = coord[1]

while row > 0 and col > 0:
    row -= 1
    col -= 1

There is two ways to come at this. (1) make a function that yield the rows and columns. (2) use zip and range.

The first way you need to enter a start, limit and step for both the x and y axis.
You then need to limit all inputs by 0 < x < x_limit, and the same for the y axis too.
This is as we don't really know what direction we're going.
We then add the step to x each iteration.
This can make:

def _diagonal(x, y, x_limit, y_limit, x_diff, y_diff):
    while 0 < x < x_limit and 0 < y < y_limit:
        x += x_diff
        y += y_diff
        yield (x, y)

Where you'd use it like:

for x, y in _diagonal(row, col, limit, limit, 1, 1):
    coords.append((x, y))

However this is quite alike to range, we enter a start, limit and step in both.
Take range(row + 1, limit, 1).
To then get both x and y you need to use zip which works perfectly as it stops iterating when any of the inputs stop.
And so you can get:

for x, y in zip(range(row + 1, limit, 1), range(col + 1, limit, 1)):
    coords.append((x, y))

This I would say is better than creating your own function as you don't really have to implement anything yourself.

Either way you'll want to remove the duplicate code to append to coords.
To do this you can use itertools.chain.
This lets you chain iterators to become one iterator.
So since you want a list as output you'll have to 'wrap' the call in a list call.

This can get you:

from itertools import chain

def diagonals(coord, size):
    x, y = coord
    return list(chain(
        [(x, y)],
        zip(range(x - 1, -1, -1), range(y - 1, -1, -1)),
        zip(range(x + 1, size, 1), range(y + 1, size, 1)),
        zip(range(x + 1, size, 1), range(y - 1, -1, -1)),
        zip(range(x - 1, -1, -1), range(y + 1, size, 1)),
    ))

print(diagonals((3, 4), 8))


Marking loans as paid if the payments exceed the loan amount I have a Rails app where Loan has paid attribute. Loan should be marked as paid if total sum of payments is equal or higher than loan amount_to_pay. Problem is that it is obligatory to have paid attribute and not checking it dynamically by joining payments to loans and selecting whether it is paid or not (long story...). So I have several options.

Option 1

Creating payments via Loan class. It helps to wrap it into transaction to be sure, that create and update would be executed. But as I understand I'm breaking single-responsibility principle here and loan have to worry about payment creation.

class Loan
  has_many :payments

  def pay!(payment_params)
    transaction do
      payments.create(payment_params)
      update(paid: true) if payments.total >= amount_to_pay
    end
  end
end

class Payment
  belongs_to :loan

  def self.total
    pluck(:amount).reduce(:+)
  end
end

loan = Loan.create(amount_to_pay: 1000)
loan.pay!({ amount: 500 })
loan.pay!({ amount: 500 })
loan.paid? #=> true

Option 2

Payments here are created directly, but there is no 100% guarantee that after_create will be executed. More over callbacks are difficult to test.

class Loan
  has_many :payments
end

class Payment
  belongs_to :loan

  after_create :mark_loan_as_paid

  def mark_loan_as_paid
    if loan.payments.total >= loan.amount_to_pay
      loan.update(paid: true)
    end
  end
end

loan = Loan.create(amount_to_pay: 1000)
Payment.create(amount: 500, loan_id: loan.id)
Payment.create(amount: 500, loan_id: loan.id)
loan.paid? #=> true

So, what is the best option to handle such cases (when some action in one class should change something in another).

Your current approach isn't terrible. Yes, there's some coupling between the two models, but that isn't "illegal". Records in Rails, by virtue of being relational data models, typically have conceptual coupling already. And while functional coupling is different, it's sometimes the best solution. Heck, Rails even has a few features that rely on such coupling like delegating methods to an associated record, or the (similar to your setup) accepts_nested_attributes_for method, and the counter_cache option for associations, which does indeed update the "owner" record after create and destroy operations.

So for what it's worth, I'd say option 2 makes the most sense. If a Payment is created - however it's created - update its associated Loan. I wouldn't say it's difficult to test either: Create a loan, create sufficient payments, then reload the loan and check that it's been paid. Transactions are trickier only if your models a distributed across multiple databases, but even then they can be added.

I'd just make sure to also run mark_loan_as_paid after an update or destroy operation - if a payment is cancelled or changed, it's no good if the loan still appears as paid. And you can run periodic sanity checks on the data (e.g. with a rake task), if need be. For anything financial, that's probably a good idea no matter what.

But all that being said, it is of course nice to avoid coupling when you can. Again, in this case, I wouldn't bother. But if I was forced to, I'd say it's a matter of thinking beyond the Loan and Payment classes, because if the logic doesn't quite fit in either of those, then it's only logical that it fits somewhere else entirely.

One option is simply to do everything in the controller. It's precisely the sort of thing controllers are meant to do: Control access to and modification of data. In Rails you typically end up with a 1:1 structure of controllers and models (e.g. a PostsController for Post records), but that isn't a law. A user account system might have a SessionsController and a SignupsController and so on that all work on the same User model.

So you could easily have your PaymentsController both create a Payment record and update the Loan record. Nothing wrong with that. In fact, your controller is probably already loading the Loan anyway in order to do @loan.payments.create(payment_params).

Last option is to make Downpayment a "model" (just a plain class, not an ActiveRecord model) or a service object. Its responsibility would be the same as the controller in the previous description: Create a Payment record, and update the Loan record. So it's essentially the same logic, just extracted out of the controller.

In either case, you're back to a single responsibility: Pay down a loan. There are two models involved, but conceptually it's one task.

The tradeoff with either, however, is that you could technically create Payment without the Loan being updated by simply not using the controller/service object. But on the other hand you can do that too by just writing to the database without going through Rails.

An example:

def downpay(loan, amount)
  loan.transaction do
    loan.payments.create!(amount: amount)
    loan.update_attributes!(paid: loan.payments.total >= loan.amount)
  end
end


Calculating the nth prime number for huge BigIntegers I wrote this simple class for calculating the nth prime:

package generator;

import java.math.BigInteger;

public class SimplePrimeNumberGenerator implements PrimeNumberGenerator {

    private static final BigInteger MINUS_ONE = BigInteger.valueOf(-1);
    private static final BigInteger ZERO = BigInteger.ZERO;
    private static final BigInteger ONE = BigInteger.ONE;
    private static final BigInteger TWO = BigInteger.valueOf(2);
    private static final BigInteger THREE = BigInteger.valueOf(3);
    private static final BigInteger SIX = BigInteger.valueOf(6);
    private static final BigInteger EIGHT = BigInteger.valueOf(8);

    @Override
    public BigInteger calculateNthPrimeNumber(BigInteger n) {
        if (n.compareTo(ZERO) <= 0) {
            throw new NumberFormatException(
                    "Can not calculate nth prime number for n=" + n);
        }
        BigInteger candidate, count;
        for (candidate = TWO, count = ZERO; count.compareTo(n) == -1; candidate = candidate
                .add(ONE)) {
            if (isPrime(candidate)) {
                count = count.add(ONE);
            }
        }
        return candidate.add(MINUS_ONE);
    }

    /*
     * Performs a simple primality test on the input parameter.
     */
    private static boolean isPrime(BigInteger candidate) {

        if (candidate.isProbablePrime(2)) {
            if (candidate.compareTo(TWO) == -1) {
                return false;
            }
            if (candidate.equals(TWO) || candidate.equals(THREE)) {
                return true;
            }
            if (candidate.mod(TWO).equals(ZERO)
                    || candidate.mod(THREE).equals(ZERO)) {
                return false;
            }
            BigInteger sqrtCandidate = sqrt(candidate).add(ONE);
            for (BigInteger i = SIX; i.compareTo(sqrtCandidate) <= 0; i = i
                    .add(SIX)) {
                if (candidate.mod(i.add(MINUS_ONE)).equals(ZERO)
                        || candidate.mod(i.add(ONE)).equals(ZERO)) {
                    return false;
                }
            }
            return true;
        } else {
            return false;
        }
    }

    /*
     * Derived from:
     * http://faruk.akgul.org/blog/javas-missing-algorithm-biginteger-sqrt/
     */
    private static BigInteger sqrt(BigInteger n) {
        BigInteger a = ONE;
        BigInteger b = n.shiftRight(5).add(EIGHT);
        while (b.compareTo(a) >= 0) {
            BigInteger mid = new BigInteger(a.add(b).shiftRight(1).toString());
            if (mid.multiply(mid).compareTo(n) > 0) {
                b = mid.subtract(ONE);
            } else {
                a = mid.add(ONE);
            }
        }
        return a.subtract(ONE);
    }
}

It scales reasonably well for five-digit-numbers n, but I need really large prime numbers (which is why I went with BigInteger in the first place). How can I achieve reasonable performance (one prime in less than five seconds) for calculating, say, the 10.000.000.000th prime number?

Small nit:

if (candidate.compareTo(TWO) == -1)
    return false;

This is implementation dependence on BigInteger - better to use generic way:

if (candidate.compareTo(TWO) < 0) {
    return false;
}

Also - this is in general more readable and less error-prone to wrap the if statements in {}-brackets even for 1-liners.

Some tricks to possibly improve performance:

Precalculate Erathosphenes sieve till some number you can handle sieve completely in memory (don't use BigInteger for storing those) and check up to sqrt but iterating through primes only and when you exhausted sieve - switch back to your method.
Did you try BigInteger#isProbablePrime here? This will tell you much faster if current number is composite - so you will do less checks.


Analog clock in React.js, rendered using CSS rotation I am new to ReactJS and I am trying to write a component which emulates an analog clock. The second, minutes and hour div elements are transformed every second using an interval function. However, I am wondering if there is a better way of transforming the styles for them in the setDate() function?

It seems that what I have written directly modifies the DOM rather than using the Virtual DOM that React provides.

import React from 'react';

class Clock extends React.Component {
    constructor(props) {
        super(props);

        this.state = {
            timer: setInterval(this.setDate, 1000)
        }
    }

    componentWillUnmount() {
        clearInterval(this.state.timer);
    }

    setDate() {
        const secondHand = document.querySelector('.second-hand');
        const minutesHand = document.querySelector('.min-hand');
        const hourHand = document.querySelector('.hour-hand');

        const now = new Date();
        const second = now.getSeconds();
        const minute = now.getMinutes();
        const hour = now.getHours();

        const secondDegrees = (second/60) * 360 + 90;
        const minutesDegrees = (minute/60) * 360 + 90;
        const hourDegrees = (hour/12) * 360 + 90;

        secondHand.style.transform = `rotate(${secondDegrees}deg)`;
        minutesHand.style.transform = `rotate(${minutesDegrees}deg)`;
        hourHand.style.transform = `rotate(${hourDegrees}deg)`;
    }

    render() {
        return (

        )
    }
}

export default Clock;

Yes - whenever the DOM is being queried (e.g. calls to document.querySelector(), etc.), that might likely be a sign that the Virtual DOM is not being utilized properly.

Use the React component state for storing values for degree values

The styles can be added inline such that the degree values of the transform-rotate styles reference values in the state:

<div className="hand hour-hand" style={{transform: `rotate(${this.state.hourDegrees}deg)`}}>

<div className="hand min-hand" style={{transform: `rotate(${this.state.minuteDegrees}deg)`}}>

<div className="hand second-hand" style={{transform: `rotate(${this.state.secondDegrees}deg)`}}>

To set those values in the state, update setDate() to call setState() with those values:

setDate() {
    const now = new Date();
    const second = now.getSeconds();
    const minute = now.getMinutes();
    const hour = now.getHours();
    this.setState({
        secondDegrees:  (second/60) * 360 + 90,
        minutesDegrees: (minute/60) * 360 + 90,
        hourDegrees: (hour/12) * 360 + 90
    });
}

And in order for the closure to reference this of the component properly in the interval callback, it can be bound using Function.bind()- use that when calling setInterval():

constructor(props) {
    super(props);
    this.state = {
        timer: setInterval(this.setDate.bind(this), 1000)
    }
}

The style attributes could also be bound to plain-old Javascript objects- this would be useful in case there were multiple in-line styles to attach. For example:

render() {
    const hourStyle = {
        transform: `rotate(${this.state.hourDegrees}deg)`
    };
    return (

a

Click here for a demonstration in JSFiddle, or expand the snippet below.

var logged = false;
class Clock extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      timer: setInterval(this.setDate.bind(this), 1000),
      secondDegrees: 0,
      minuteDegress: 0,
      hourDegrees: 0
    }
  }
  componentWillUnmount() {
    clearInterval(this.state.timer);
  }
  setDate() {

    const now = new Date();
    const second = now.getSeconds();
    const minute = now.getMinutes();
    const hour = now.getHours();

    const secondDegrees = (second / 60) * 360 + 90;
    const minutesDegrees = (minute / 60) * 360 + 90;
    const hourDegrees = (hour / 12) * 360 + 90;;
    this.setState({
      secondDegrees: (second / 60) * 360 + 90,
      minutesDegrees: (minute / 60) * 360 + 90,
      hourDegrees: (hour / 12) * 360 + 90
    });
  }
  render() {
    const secondStyle = {
      transform: `rotate(${this.state.secondDegrees}deg)`
    };
    const minuteStyle = {
      transform: `rotate(${this.state.minuteDegrees}deg)`
    };
    const hourStyle = {
      transform: `rotate(${this.state.hourDegrees}deg)`
    };
    return ( <
      div className = "col-md-6" >
      <
      div className = "clock" >
      <
      div className = "clock-face" >
      <
      div className = "hand hour-hand"
      style = {
        hourStyle
      } >
 <
      div className = "hand min-hand"
      style = {
        minuteStyle
      } >
 <
      div className = "hand second-hand"
      style = {
        secondStyle
      } >
 <
      /div> <
      /div> <
      /div>
    );
  }
}

ReactDOM.render( <
  Clock / > ,
  document.getElementById('container')
);
/*Matt Litzinger - http://codepen.io/mlitzinger/pen/ruEyz*/

.clock {
  width: 200px;
  height: 200px;
  background: -webkit-radial-gradient(#3b3b3b, #000);
  background: radial-gradient(#3b3b3b, #000);
  box-shadow: inset 0px 0px 30px #777, 0px 2px 18px rgba(0, 0, 0, 0.5), 0px 20px 40px #bbb;
  border: 6px solid #222;
  border-radius: 106px;
  margin: auto;
  position: absolute;
  top: 0;
  bottom: 0;
  left: 0;
  right: 0;
}
.hand {
  background: #fff;
  position: absolute;
  -webkit-transform-origin: 50% 100%;
  -moz-transform-origin: 50% 100%;
  -o-transform-origin: 50% 100%;
  -ms-transform-origin: 50% 100%;
  transform-origin: 50% 100%;
}
.hour-hand {
  width: 4px;
  height: 55px;
  box-shadow: 0px 0px 7px #000;
  top: 45px;
  left: 98px;
}

.min-hand {
  width: 4px;
  height: 80px;
  box-shadow: 0px 0px 4px #000;
  top: 20px;
  left: 98px;
}

.second-hand {
  width: 2px;
  height: 80px;
  background: #bbb;
  box-shadow: 0px 0px 7px #000;
  top: 20px;
  left: 99px;
}

.pin {
  width: 10px;
  height: 10px;
  background: #111;
  border-radius: 10px;
  margin: auto;
  position: absolute;
  top: 0;
  bottom: 0;
  left: 0;
  right: 0;
}

.hour-hand,
.minute-hand,
.second-hand {
}


SPOJ Adding reversed number in Java I just started to practice for competitive programming and just started to solve the most solved problems on SPOJ.
This is what I made for ADDREV: Adding reversed numbers in Java (https://www.spoj.com/problems/ADDREV/)

The Antique Comedians of Malidinesia prefer comedies to tragedies. Unfortunately, most of the ancient plays are tragedies. Therefore the dramatic advisor of ACM has decided to transfigure some tragedies into comedies. Obviously, this work is very hard because the basic sense of the play must be kept intact, although all the things change to their opposites. For example the numbers: if any number appears in the tragedy, it must be converted to its reversed form before being accepted into the comedy play.
Reversed number is a number written in arabic numerals but the order of digits is reversed. The first digit becomes last and vice versa. For example, if the main hero had 1245 strawberries in the tragedy, he has 5421 of them now. Note that all the leading zeros are omitted. That means if the number ends with a zero, the zero is lost by reversing (e.g. 1200 gives 21). Also note that the reversed number never has any trailing zeros.
ACM needs to calculate with reversed numbers. Your task is to add two reversed numbers and output their reversed sum. Of course, the result is not unique because any particular number is a reversed form of several numbers (e.g. 21 could be 12, 120 or 1200 before reversing). Thus we must assume that no zeros were lost by reversing (e.g. assume that the original number was 12).
Input
The input consists of N cases (equal to about 10000). The first line of the input contains only positive integer N. Then follow the cases. Each case consists of exactly one line with two positive integers separated by space. These are the reversed numbers you are to add.
Output
For each case, print exactly one line containing only one integer - the reversed sum of two reversed numbers. Omit any leading zeros in the output.

import java.util.*;
import java.lang.*;

class Main
{

public static void main (String[] args) throws java.lang.Exception
{
    Scanner sc = new Scanner(System.in);
    sc.nextLine();
    while(sc.hasNextInt())
    {
        printNextNumber(sc);
    }

}

public static void printNextNumber(Scanner sc)
{
    int a = sc.nextInt();
    int b = sc.nextInt();
    int reversedA = reverseNumber(a);
    int reversedB = reverseNumber(b);
    int reversedSum = reverseNumber(reversedA+reversedB);
    System.out.println(reversedSum);
}

public static int reverseNumber(int number)
{
    return listToNumberMadeOfDigits(numberToListOfDigits(number));

}

public static List<Integer> numberToListOfDigits(int number)
{
    List<Integer> list = new ArrayList<>();
    while(number>0){
        list.add(number%10);
        number /= 10;
    }

    Collections.reverse(list);
    return list;
}

public static int listToNumberMadeOfDigits(List<Integer> digits)
{
    int number = 0;
    for(int i = 0; i < digits.size();i++){
        number += Math.pow(10,i)*digits.get(i);
    }
    return number;
}

 }

How is this code?
What should I avoid in the future?

First, note that this review is of general good coding practices for production code, and not all suggestions will be appropriate for the sphere of "competitive programming", in which I have no significant experience.

Some thoughts:

For readability, use whitespace consistently and more frequently. There should be whitespace (0) after control statements (if, while, for), around operators (+, %, ..), and before curly braces ({).

In Java, the convention is that open curly braces go on the same line, not on a newline.

Use final aggressively when variable declarations will not change to reduce cognitive load on readers.

AutoCloseable resources such as Scanner should be used in a try-with-resources block to ensure they get closed.

a and b are poor choices for variable names because they're meaningless. Variable names should describe the value they hold. You probably don't even need the intermediate variables.

Methods not intended for use outside their class should be private. Always prefer the most restrictive access you can reasonably apply.

In a real application, it would be highly preferable to having the reversed sum pushed back to the main() method to handle printing. This gives you flexibility, rather than pushing your output choice through your code - if you want to change later, it changes in fewer, higher-level places.

It might be nice to use the number of cases they provide you with.

Don't throw Exception ever - throw the most specific checked exception possible. Don't declare thrown checked exceptions if you don't actually throw them.

Your algorithm is much more complex than it needs to be. Lists are extraneous. The problem can be solved with math.

If you were to apply all the changes I suggested, your code might look more like:

import java.util.Scanner;

class Main {

    public static void main(final String[] args) {
        try (final Scanner sc = new Scanner(System.in)) {
            final int numberOfCases = sc.nextInt();
            for (int i = 0; i < numberOfCases; i++) {
                final int firstNumber = sc.nextInt();
                final int secondNumber = sc.nextInt();
                System.out.println(addReversedNumbers(firstNumber, secondNumber));
            }
        }
    }

    private static int addReversedNumbers(final int firstNumber, final int secondNumber) {
        return reverseNumber(reverseNumber(firstNumber) + reverseNumber(secondNumber));
    }

    private static int reverseNumber(final int number) {
        int numberToReverse = number;
        int reversedNumber = 0;

        while (numberToReverse > 0) {
            final int digit = numberToReverse % 10;
            numberToReverse /= 10;
            reversedNumber = 10 * reversedNumber + digit;
        }
        return reversedNumber;

    }

}


SQL-injection mitigation script I'll gladly appreciate it if you could review my code below and let me know if they are sufficiently secure.

My main website and these scripts will use same database, so I need to make sure they are sufficiently secure before I put them live.

Since I cannot post more than two links, I'll post my Anti-SQL Injection script, and below are the actual two scripts.

<?php
if (!function_exists('sql_inject_chec')) {
function sql_inject_chec(){
 // Anti-SQL Injection
 $badwords = array("--",";","'",'"',"DROP",
                   "SELECT", "UPDATE", "DELETE",
                   "drop", "select", "update", "delete",
                   "WHERE", "where","exec","EXEC",
                   "procedure","PROCEDURE");
 foreach($_REQUEST as $value)  {
  foreach($badwords as $word)
   if(substr_count($value, $word) > 0) {
    die("SQL Эnjection Denemesi Yaptэnэz... ama Baюarэsэz Oldunuz ;)");
    }
  }
 }
}
sql_inject_chec();
?>

Ingame registration - SCRIPT

Score display - SCRIPT

This is not a constructive way to protect your scripts from SQL injection — the objective of SQL-injection prevention should not be to disallow characters or words, but to escape characters. What you have now, prevents your users from submitting perfectly valid English words such as select, update, etc. and/or perfectly valid characters such as --, " and '.

Consider this fictional comment:

  When I select "where" from the menu, it updates the procedure -- strange.

This would be rejected by your code.

Now, I'm sure, for your application, this might (currently) be irrelevant, but your code is not very flexible or portable. What happens if your current application, or another future application needs to accept these words? You'd have to rewrite your code, and for a possible new application you wouldn't be able to copy existing code.

More importantly though, your current code is probably not elaborate enough to mitigate all possible attack vectors. You'd really be wise to implement prepared, parameterized  statements by using PDO, instead of trying to prevent your users from entering possible SQL keywords and characters. With prepared, parameterized statements you don't have to worry about escaping the correct characters anymore — PDO (or, if the driver supports it natively, the database driver) will escape the parameter values you supply for you.

Let me give you a short introductory example of how you could implement SQL-injection prevention with PDO:

// create a database connection handle
$driver   = 'odbc';
$database = 'kn_online';
$username = '*redacted*';
$password = '*redacted*';
$dbh = new PDO( "$driver:$database", $username, $password );

// create a parameterized statement to select a record from CURRENTUSER
// uses a named parameter (denoted by :), called accountId
// that will be properly escaped by PDO
$sql = 'SELECT * FROM CURRENTUSER WHERE StrAccountID = :accountId';

// prepare the statement; returns a PDOStatement object
$stmt = $dbh->prepare( $sql );

// execute the statement by passing the parameter values in an associative array
$executionResult = $stmt->execute( array(
    ':accountId' => $acc
) );

// if the execution succeeded
if( $executionResult )
{
    // fetch the record as an associative array
    $record = $stmt->fetch( PDO::FETCH_ASSOC );
}

The beauty of prepared statements is that you can also easily execute them repeatedly, with new parameter values. Consider this example (connection handle creation omitted for brevity):

// insert into some table
$sql = 'INSERT INTO SOMETABLE VALUES( :id, :name )';

$stmt = $dbh->prepare( $sql );

// some fictional data
// parameter value keys don't need : prepended
$data = array(
    array( 'id' => 1, 'name' => 'Name One' ),
    array( 'id' => 2, 'name' => 'Name Two' ),
    array( 'id' => 3, 'name' => 'Name Three' )
);

foreach( $data as $datum )
{
    // using the same prepared statement
    // to execute for each datum
    $stmt->execute( $datum );
}


Necessary singleton in DAO I am using a singleton in my DAO. Would I be better off with static methods? Also, should I be worried about synchronization of the singleton and my data structures?

public class RAMUserDAO implements GenericDAO<User, String, Boolean>
    {
        private static RAMUserDAO   userDAO = null;
        private Map<String, User>   userList;
        private Map<String, User>   banList;
        private static Logger       log     = LOG.PRODUCTION;

        private RAMUserDAO()
            {
                userList = new ConcurrentHashMap<String, User>();
                banList = new ConcurrentHashMap<String, User>();
            }

        public static RAMUserDAO getRAMUserDAO()
            {
                if (userDAO == null)
                    {
                        synchronized (RAMUserDAO.class)
                            {
                                if (userDAO == null)
                                    {
                                        userDAO = new RAMUserDAO();
                                    }
                            }
                    }
                return userDAO;
            }
    }

Your spacing is all over the place, and the double null check against userDAO seems redundant.

I'm not making a comment on the big picture on whether or not the singleton is necessary, but I'd certainly fix getRAMUserDAO() up to look more like this:

public static RAMUserDAO getRAMUserDAO() {
    synchronized (RAMUserDAO.class) {
        if (userDAO == null) {
            userDAO = new RAMUserDAO();
        }
        return userDAO;
    }
}

The Egyptian-style braces are just my general preference.  I don't know if there's a Java standard for how to do the braces.