Laravel controller to update appointment requests My question/refactor is similar to what is found here:
Laravel controller for form validation skinny

However, I feel that the answer is for 5.0 and not 5.2. This question does not pertain only to one controller but considering that I am making a RESTful API I believe 1 example is sufficient so I can propagate this:

class UpdateAppointmentRequest extends Request
{
    /**
     * Determine if the user is authorized to make this request.
     *
     * @return bool
     */
    public function authorize()
    {
        return false;
    }

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array
     */
    public function rules()
    {
        return [
            'id' => 'required|integer',
            'user_id' => 'required|integer',
            'status' => 'integer',
            'note' => 'alpha'
        ];
    }
}

public function update(UpdateAppointmentRequest $request)
{
    $appointment = $this->appointment->find($request->get('id'));
    if ($appointment === null) {
        return $this->notFound('Appointment Not Found');
    }
    if($request->has('user_id') === true) {
        $appointment->user_id = $request->get('user_id');
    }
    if($request->has('status') === true) {
        $appointment->status = $request->get('status');
    }
    if($request->has('note') === true) {
        $appointment->note = $request->get('note');
    }
    try {
        return response()->json(['created' => $appointment->save()]);
    } catch(\PDOException $e) {
        return $this->internalIssues($e->getMessage());
    }
}

My issue is the if statements. Now considering that only 2 items from the validation are required and the rest are not - I have to validate if the present of the variable is there in order to update. It works well, but it will eventually make controllers very bloated (depending on which restful resources might have more parameters).

Does anyone know of a better way to refactor this within Laravel 5.2+?

public function update(UpdateAppointmentRequest $request)
{
    try {
        $data = array_filter($request->only(['user_id','status','note']));
        $appointment = $this->appointment->findOrFail($request->input('id'));
        $result = $appointment->update($data);
        return response()->json(['created' => $result]);
    } catch(\PDOException $e) {
        return $this->internalIssues($e->getMessage());
    }
}

What is wrong with this simple solution? If request does not have a field, say user_id, the only would not return that. And if there is an error while updating, the catch would catch that and return error. Why all those manual checking?


Animal Shelter in Java I have been working my way through the problems in the book Cracking the Coding Interview.  The instructions were to maintain an animal shelter that operates on a first in, first out basis (i.e. it is a queue). People must adopt the "oldest" (based on arrival time) of all animals at the shelter, or if they would prefer a dog or a cat (and will receive the oldest animal of that type). They cannot choose which specific animal they would like.

AnimalShelter.java

package problem_2_9;

import java.util.LinkedList;
import java.util.NoSuchElementException;

public class AnimalShelter {

    public enum AnimalType{
        DOG, CAT
    }

    private int animalId;
    LinkedList<Integer> cats;
    LinkedList<Integer> dogs;

    public AnimalShelter(){
        cats = new LinkedList<Integer>();
        dogs = new LinkedList<Integer>();
    }

    public void enqueue(AnimalType type){
        switch(type){
        case DOG:
            dogs.add(animalId);
            animalId++;
            break;
        case CAT:
            cats.add(animalId);
            animalId++;
            break;
        }
    }
    public Integer dequeueCat(){
        if(cats.isEmpty()) throw new NoSuchElementException("There are no cats in the animal shelter.");
        return cats.pop();
    }

    public Integer dequeueDog(){
        if(dogs.isEmpty()) throw new NoSuchElementException("There are no dogs in the animal shelter");
        return dogs.pop();
    }

    public Integer dequeueAny(){
        if(dogs.isEmpty() && cats.isEmpty()) throw new NoSuchElementException("There are no animals in the animal shelter.");
        if(dogs.isEmpty()){
            return cats.pop();
        }
        else if(cats.isEmpty()){
            return dogs.pop();
        } else{
            if(cats.peek() < dogs.peek()){
                return cats.pop();
            }
            else{
                return dogs.pop();
            }
        }
    }
}

I've tested the AnimalShelter class using this code:

Main.java

package problem_2_9;

import problem_2_9.AnimalShelter.AnimalType;

    public class Main {

        public static void main(String[] args){
            // dogs = 0 2 4 6
            // cats = 1 3 5 7
            AnimalShelter shelter = new AnimalShelter();
            for(int i = 0; i < 4; i++){
                shelter.enqueue(AnimalType.DOG);
                shelter.enqueue(AnimalType.CAT);
            }

            for(int i = 0; i < 4; i++){
                System.out.println(shelter.dequeueCat());
                System.out.println(shelter.dequeueDog());
            }
        }
    }

The model has failed

I would expect from an animal shelter to actually store animals.
(Well, in programming terms, a representation of animals.)
Your implementation is not an animal shelter, it's an animal-relative-arrival-time shelter. I cannot add animals to it, and cannot retrieve animals from it. I can only add animal types, and retrieve meaningless sequence numbers. This is not an acceptable model of an animal shelter.

Failure to generalize

It's important to read the problem description attentively.
Notice that dogs and cats are not part of the essence of the problem,
they are casually mentioned examples. The target system should support animals of all types, and that seems to be a reasonable expectation.

I think the expected interface is something like this:

interface AnimalShelter {

    add(Animal animal);

    Animal get(AnimalType type);

    Animal get();
}

Notice that I didn't call these methods enqueue and dequeue.
Those are implementation terms,
not the terms of the problem domain.
(Probably they could be improved, if I knew anything about the real-world domain of animal shelters.)
Different animal shelters may have different policies for adding and getting animals.
Your task is to implement one with a specific policy,
which could be reflected in the name of the class in your implementation,
but this interface can stay as is.

Testing

What you called "testing" is "demonstrating".
It's not testing anything.
Testing should involve assertions (of truths) about the implementation.
Here's an example:

@Test
public void should_return_the_right_type_in_fifo_order() {
    AnimalShelter shelter = new FifoAnimalShelter();

    Animal fluffy = new Cat("fluffy");
    shelter.add(fluffy);

    Animal sparky = new Dog("sparky");
    shelter.add(sparky);

    Animal smelly = new Cat("smelly");
    shelter.add(smelly);

    assertThat(shelter.get(AnimalTypes.CAT)).isEqualTo(fluffy);
    assertThat(shelter.get(AnimalTypes.CAT)).isEqualTo(smelly);
}

Discussion

For all problems in this book, it's important to carefully consider the time and space complexity of the implementation. In your current implementation, this was trivial: all the dequeue* methods have \$O(1)\$ complexity. That should have been a warning sign, indicating that there's more to this exercise. Indeed, if you extend to support all animal types, the complexity analysis becomes more interesting.

Take for example the get() and get(AnimalType) methods. If you use one queue per type, then get(AnimalType) will be \$O(1)\$, and get() will be \$O(k)\$, where \$k\$ is the number of types. If you use one queue for all animals, then get(AnimalType) will be \$O(n)\$, and get() will be \$O(1)\$. Further variations and optimizations will be possible, it's good to think about them and practice calculating their time and space complexity tradeoffs.


Representing a long regular expression in a Polynomial class I wrote a simple Polynomial class:

class Polynomial
  def initialize(coefficients)
    @coefficients = coefficients.reverse
  end

  def to_s
    return '0' if @coefficients.all?(&:zero?)
    @coefficients.map.with_index do |k, index|
      " #{Polynomial.sign(k)} #{k.abs}x^#{index}"
    end.reverse.join.strip          # removes whitespace at the front
    .gsub(/\A\+\s/, '')             # removes + if first coefficient is positive
    .gsub(/x\^0/, '')               # removes x to the power of 0
    .gsub(/\s(\+|-)\s0x\^\d/, '')   # removes 0 coefficients
    .gsub(/\s(\+|-)\s0/, '')        # removes 0 at the end of the expression
    .gsub(/\^1/, '')                # removes power of 1
    .gsub(/1x/, 'x')
  end

  def evaluate(x)
    @coefficients.map.with_index { |k, index| k * (x**index) }.reduce(0, :+)
  end

  private

  def self.sign(integer)
    integer >= 0 ? '+' : '-'
  end
end

There are a few things that bother me in the to_s method:

Is it bad that I wrote the short comments explaining what each gsub does? Bad in terms of "Would you do this in production code?".
Since all but one of the gsubs do the same, I could replace them with a single gsub, containing a long regexp with lots of ors (|):

.gsub(/\A\+\s|x\^0|\s(\+|-)\s0x\^\d|\s(\+|-)\s0|\^1/, '')

In that case though the regexp become quite unreadable. Which of the two is better?
Regarding the map.with_index part:

I use do/end, but than the chaining of methods on the end doesn't look good.
If I use curly braces on multiple lines, I break the convention curly braces for single-line blocks, do/end for multi-line blocks.
If I use curly braces with a single-line block, the line will be more than 80 characters long. So which of the three variants would you use?

Rather than construct the polynomial string incorrectly, then fix it with regex's, it may be easier to contruct it directly.  This is one way you could do that.

class Polynomial
  def initialize(coefficients)
    @coefficients = coefficients
  end

  def to_s
    coeffs = @coefficients.each_with_index.select { |c,k| c.nonzero? || k==0 }.reverse
    coeffs.each_with_object('') do |(c,k), s|
      cneg = (c.to_f < 0.0)
      if [c,k] == coeffs.first
        s << '-' if cneg
      else
        s << (cneg ? ' - ' : ' + ')
      end
      s << "#{c.abs}" unless (c.abs.to_f == 1.0 && k > 0)
      s << "x" if k > 0
      s << "^#{k}" if k > 1
    end
  end
end

coefficients = [2.1, 3, 1.0, 0, -6.1, 1, -5.3, 0.2]
p Polynomial.new(coefficients).to_s
  # => 0.2x^7 - 5.3x^6 + x^5 - 6.1x^4 + x^2 + 3x + 2.1

For this example value of coefficients, this is what's happening:

First save each coeffient's index with its value:

a = @coefficients.each_with_index
  # => [[2.1,0], [3,1], [1.0,2], [0,3], [-6.1,4], [1,5], [-5.3,6], [0.2,7]]

Next, select only pairs for which the value is non-zero or the exponent is zero:

b = a.select { |c,k| c.nonzero? || k==0 }
  # => [[2.1, 0], [3, 1], [1.0, 2], [-6.1, 4], [1, 5], [-5.3, 6], [0.2, 7]]

Notice that [0,3] has been removed.  Now reverse the elements of the array:

coeffs = b.reverse
  # => [[0.2, 7], [-5.3, 6], [1, 5], [-6.1, 4], [1.0, 2], [3, 1], [2.1, 0]]

each_with_object('') creates an empty string, designated s in the block.  It is then a simple matter to consider each element of each term in the polynomial to build up the string.

I'm not certain if I followed the formatting rules exactly, but if I did not, it should be easy to modify the code to conform.

(Edited to fix boo-boos spotted by @200_success.)


Optimizing C++ equivalent of Matlab `filter` function Minimum working example below. The Matlab filter function filters the input data x using a rational transfer function defined by the numerator and denominator coefficients b and a and initial conditions z. My C++ version of the code is way slower than the Matlab one. My signal x is a huge valarray, a and b are three elements long each and the initial conditions z is just three zeros.

#include <iostream>
#include<ctime>
#include <cstdlib>
using std::rand;
#include <valarray>
using std::valarray;
#include <ctime>

#define D_SCL_SECURE_NO_WARNINGS 1

struct FilterResults
{
    valarray<double> filteredSignal;
    valarray<double> finalConditions;
};

FilterResults Filter(const valarray<double> &b,
    const valarray<double> &a, const valarray<double> &x, const valarray<double> &z)
{
    valarray<double> y(x.size());
    // Pad z
    valarray<double> zOut(a.size());
    std::copy(std::begin(z), std::end(z), std::begin(zOut));

    double Xm, Ym;
    size_t i, n = a.size();
    for (size_t m = 0; m < y.size(); m++)
    {
        Xm = x[m];
        y[m] = b[0] * Xm + zOut[0];
        Ym = y[m];
        for (i = 1; i < n; i++)
        {
            zOut[i - 1] = b[i] * Xm + zOut[i] - a[i] * Ym;
        }
    }
    valarray<double> s = zOut[std::slice(0, zOut.size() - 1, 1)];
    FilterResults r;
    r.filteredSignal = std::move(y);
    r.finalConditions = std::move(s);
    return r;
}

int main() {
    std::srand(std::time(0));
    valarray<double> b{ { (double)rand(), (double)rand(), (double)rand() } };
    valarray<double> a{ { (double)rand(), (double)rand(), (double)rand() } };
    valarray<double> z{ { 0, 0, 0 } };
    valarray<double> x( 500000 );
    for (size_t i = 0; i < 500000; i++)
    {
        x[i] = (double)rand();
    }

    clock_t begin = clock();
    auto r = Filter(b, a, x, z);
    clock_t end = clock();
    double elapsed_secs = double(end - begin) / CLOCKS_PER_SEC;
    std::cout << "elapsed_secs: " << elapsed_secs << std::endl;
    system("pause"); // C++ is 0.184 seconds on my computer, Matlab is 0.006060.
}

Any idea on how to speed it up?

Few highlights:

std::copy(std::begin(z), std::end(z), std::begin(zOut));

To copy input values to overwrite them is just a waste of time. You will overwrite them then you do not need to copy old values, use z instead of zOut inside loops.

y.size() is possibly evaluated multiple times. It's a minor performance issue (and it may be inlined) but it depends on your actual implementation.

I don't know about valarray<T> implementation but if this is performance critical you may want to use a plain array.

I hope FilterResults constructor doesn't allocate anything (what valarray<double> default constructor does?) That said, this is C++, filter needs a state then you should design this using a Filter class with its private instance fields. You return just filtered data and you do not create/copy/move status. I imagine a Filter class like this:

class Filter
{
public:
    Filter(const valarray<double>& b, const valarray<double>& b)
    {
        // Validate parameters...
        this->a = a;
        this->b = b;

        // I omit z, I think there are very few chances you want
        // to set initial filter state...
    }

    valarray<double>& Process(const valarray<double>& input)
    {
        // Your code here, when you're done just return computed
        // result. "Final conditions" (filter status?) has not to
        // be returned, it's a private field.
    }

private:
    valarray<double> b, a, z;
};

As you can note some calculations/allocations will be performed in constructor. It won't change anything if each time you filter a different chunk of samples, using different filter, but in the other cases it will allow a big speed-up. Next step in design (to do something in C++) is to abstract away filter and its design: you may have a pure abstract base class Filter, an IirFilter implementation and one or more classes derived from FilterDesigner (let's say ChebyshevIirFilterDesigner to calculate filter coefficients). In general to mimic MATLAB approach may result in a sub-optimal implementation (because languages are different and because it will lead to a less than perfect OOP design.) If this class is a use-once function and you want to keep a function instead of a class you may drop finalConditions, do you actually use it? If you're implementing an on-line filter (for a continuous stream) then you'd better save it as private field (it's the z parameter you're using now)

You should never have using namespace std; You already use std:: consistently then you just add a possible source of conflicts.

Few more notes about performance:

In this case (I guess they do not use any esoteric math theory to perform an optimization) C++ performance should be pretty similar or even better than MATLAB. MATLAB may (I do not know what current implementation does) use SIMD/AVX instructions. Do not think you can have same throughput if you don't.

To perform a benchmark is more complex than this! At very least you should setup your machine for that and to perform calculations few thousands times to have an average and a distribution then inspect results to understand caching issues...


Filtering and adding additional properties to array of objects Hi I am working on a complex application and currently adding two additional properties to objects that match the condition and returning it back. I was able to achieve the expected output, but just wanted to know if there is any other way which is more efficient than what I have tried.

Step 1 : Filtering the array to get only car objects.

Step 2 : Add firstlayer and lastlayer prop to car objects

Step 3 :

If it is first car object in the filtered array then mark firstlayer
as true.     If it is last car object in the filtered array then mark
lastlayer as true.

Step 4 : concat it with the other non-car objects

Original array:

[{
  id: 10,
  name: "Truck",
  status: "Cancelled"
}, {
  id: 11,
  name: "Bus",
  status: "Approved"
}, {
  id: 12,
  name: "Car1",
  status: "Approved"
}, {
  id: 19,
  name: "Car2",
  status: "Cancelled"
}, {
  id: 13,
  name: "Car3",
  status: "Cancelled"
}]

Expected array:

[{
      id: 10,
      name: "Truck",
      status: "Cancelled"
    }, {
      id: 11,
      name: "Bus",
      status: "Approved"
    }, {
      id: 12,
      isFirstLayer: true,
      isLastLayer: false,
      name: "Car1",
      status: "Approved"
    }, {
      id: 19,
      isFirstLayer: false,
      isLastLayer: false,
      name: "Car2",
      status: "Cancelled"
    }, {
      id: 13,
      isFirstLayer: false,
      isLastLayer: true,
      name: "Car3",
      status: "Cancelled"
    }]

This is the code I have tried

var array = [
  {
    "name": "Truck",
    "status": "Cancelled",
    "id": 10
  },
  {
    "name": "Bus",
    "status": "Approved",
    "id": 11
  },
  {
    "name": "Car1",
    "status": "Approved",
    "id": 12
  },
  {
    "name": "Car2",
    "status": "Cancelled",
    "id": 19
  },
  {
    "name": "Car3",
    "status": "Cancelled",
    "id": 13
  }
];

var i=0;
var arrayLength=array.length;
var newArray = array.filter(function(a) {
  if((a.name !='Bus'&&a.name!='Truck'))
  {

      if(i==0){
    a.isFirstLayer=true;
          a.isLastLayer=false;
      }else if(i==arrayLength-i-1){
         a.isFirstLayer=false;
          a.isLastLayer=true;
      } else{
          a.isFirstLayer=false;
          a.isLastLayer=false;
      }
      i++;
  };
  return a
});

console.log(newArray);

https://jsfiddle.net/fierce_trailblazer/q2uvf06r/24/

Any suggestions to improve the code would be helpful

It may be a better idea to use a reducer to initially add the properties to a subset of the elements of the array. Finally you can change the last element from the new Array with a property isLastLayer. I used Array.find within the reducer lambda and a custom function findLast for finding the last array-element with property isLastLayer.

const result = document.querySelector("#result");
const log = (...txt) => result.textContent += txt.join("\n") + "\n";

// moved to function, so the relevant code is more visible
const array = getTestArray();

// find the last element using the predicate lambda
const findLast = (array, predicate) => {
  for (let i = array.length - 1; i >= 0; --i) {
    if (predicate(array[i])) {
      return array[i];
    }
  }
};

// reduce the values, adding the layer props to
// all elements where the name does not start
// with 'bus' or 'truck'. Use Array.find within the
// accumulated array to determine if isFirstLayer
// is already in the set of elements
let newArray = array.reduce((acc, value) => {
  if (/^(bus|truck)/i.test(value.name)) {
    return [...acc, value];
  }
  const isFirst = !acc.find(el => "isFirstLayer" in el);
  value = { ...value,
    isFirstLayer: isFirst,
    isLastLayer: false
  };
  return [...acc, value];
}, []);

// use findLast to find the last element with
// a property 'isLastLayer' in the array, and
// change its value
findLast(newArray, el => "isLastLayer" in el).isLastLayer = true;

// let's see if it worked
log(JSON.stringify(newArray, null, 2));

function getTestArray() {
  return [{
      "name": "Truck",
      "status": "Cancelled",
      "id": 10
    },
    {
      "name": "Bus",
      "status": "Approved",
      "id": 11
    },
    {
      "name": "Car1",
      "status": "Approved",
      "id": 12
    },
    {
      "name": "Car2",
      "status": "Cancelled",
      "id": 19
    },
    {
      "name": "Car3",
      "status": "Cancelled",
      "id": 13
    }
  ];
}

It can also be done without the extra findLast function

const result = document.querySelector("#result");
const log = (...txt) => result.textContent += txt.join("\n") + "\n";
const array = getTestArray();
let accumulator = {
  newArr: [],
  // track first/last
  firstLayerSet: false,
  lastLastLayer: null
};
const addPropsReducer = (acc, value, i) => {
  if (/^(bus|truck)$/i.test(value.name)) {
    return {
      newArr: [...acc.newArr, value]
    };
  }
  value = { ...value,
    isFirstLayer: !acc.firstLayerSet,
    isLastLayer: false
  };
  return {
    ...acc,
    newArr: [...acc.newArr, value],
    firstLayerSet: true,
    lastLayerIndex: i
  };
};

let newArray = array.reduce(addPropsReducer, accumulator);
newArray.newArr[newArray.lastLayerIndex].isLastLayer = true;

// ditch tracking values
newArray = newArray.newArr;

log(JSON.stringify(newArray, null, 2));

function getTestArray() {
  return [{
      "name": "Truck",
      "status": "Cancelled",
      "id": 10
    },
    {
      "name": "Bus",
      "status": "Approved",
      "id": 11
    },
    {
      "name": "Car1",
      "status": "Approved",
      "id": 12
    },
    {
      "name": "Car2",
      "status": "Cancelled",
      "id": 19
    },
    {
      "name": "Car3",
      "status": "Cancelled",
      "id": 13
    }
  ];
}

Play with this code in jsFiddle.


New Year Chaos (HackerRank problem) - find minimum swap count The goal of the function is to find and print the minimum number of swaps (bribes) in what was initially a sorted array of ints - https://www.hackerrank.com/challenges/new-year-chaos/problem

I have passed this problem with the following code:
def minimumBribes(q):
    bribeCount = 0
    simulated = list(range(1, len(q) + 1))

    for i in range(0, len(q)):
        if q[i] > i+3:
            print("Too chaotic")
            return
    for i in range(0, len(simulated)):
        if simulated[i] == q[i]:
            continue
        while(simulated[i] != q[i]):
            # do 2 bribes
            if i + 3 == q[i]:
                simulated[i + 2], simulated[i + 1] = simulated[i + 1], simulated[i + 2]
                simulated[i + 1], simulated[i] = simulated[i], simulated[i + 1]
                bribeCount += 2
            # do 1 bribe
            else:
                simulated[i + 1], simulated[i] = simulated[i], simulated[i + 1]
                bribeCount += 1

    print(bribeCount)

My approach was to first scan through the array to determine whether it is valid, as 1 person can only bribe (swap) twice, hence the first for loop.
Then I go through each entry in my initial state array(simulated) and do swaps until the simulated entry at position i matches that in the final array at the same position.
I'm wondering if there is a better way to do this or whether something can be improved in my approach? I rarely do while loops like this as it seems it could go infinite, but I guess it's okay for this problem as we know only 2 swaps are possible for any entry.

Some minor stuff:
This if:
    if simulated[i] == q[i]:
        continue

is redundant and can be removed, due to the predicate on your while. The while would execute zero times and have the same effect as if you continued.
The while itself:
    while(simulated[i] != q[i]):

should drop the outer parens.
The range here:
for i in range(0, len(simulated)):

should drop the 0,  because that's the default.
Since you use both i and simulated[i], you should iterate using enumerate instead of range (though this won't be the case if you delete the if; continue). Getting a value from enumerate won't be helpful during the value swaps that you do later.
About the value swaps, this block:
        if i + 3 == q[i]:
            simulated[i + 2], simulated[i + 1] = simulated[i + 1], simulated[i + 2]
            simulated[i + 1], simulated[i] = simulated[i], simulated[i + 1]
            bribeCount += 2
        # do 1 bribe
        else:
            simulated[i + 1], simulated[i] = simulated[i], simulated[i + 1]
            bribeCount += 1

should collapse to
        if i + 3 == q[i]:
            simulated[i + 2], simulated[i + 1] = simulated[i + 1], simulated[i + 2]
            bribeCount += 1

        simulated[i + 1], simulated[i] = simulated[i], simulated[i + 1]
        bribeCount += 1


List elements digit difference sort I am doing a code challenge of codesignal.com which ask for following things:
Given an array of integers, sort its elements by the difference of their largest and smallest digits. In the case of a tie, that with the larger index in the array should come first.

Example

For a = [152, 23, 7, 887, 243], the output should be
digitDifferenceSort(a) = [7, 887, 23, 243, 152].

Here are the differences of all the numbers:

152: difference = 5 - 1 = 4;
23: difference = 3 - 2 = 1;
7: difference = 7 - 7 = 0;
887: difference = 8 - 7 = 1;
243: difference = 4 - 2 = 2.

23 and 887 have the same difference, but 887 goes after 23 in a, so in the sorted array it comes first.
I wrote following code using python3 and it passes all normal tests but it cannot pass execution time tests. How can I improve my code to decrease it's execution time for list with considerable amount of elements?

def digitDifferenceSort(a):
    diff = []
    for i in a:
        i = list(str(i))
        diff.append(i)

    for i in range(len(diff)):
        for j in range(i+1, len(diff)):
            if int(max(diff[i])) - int(min(diff[i])) > int(max(diff[j])) - int(min(diff[j])):
                diff[j], diff[i] = diff[i], diff[j]
            elif int(max(diff[i])) - int(min(diff[i])) == int(max(diff[j])) - int(min(diff[j])):
                diff[i], diff[j] = diff[j], diff[i]
    new_list = []
    for i in diff:
        b = ''
        for j in i:
            b = b + j

        new_list.append(int(b))

    return new_list

Python has a built-in sorted function, you should use it. What it needs to sort according to some special criteria is a key function:

def max_digit_diff(n):
    n_str = str(n)
    return int(max(n_str)) - int(min(n_str))

This uses the fact that "0" < "1" < ... < "9".

However, the sorted function uses a stable sorting algorithm, so if two elements compare equal, the original order is preserved. But here we want the opposite order (later elements come first), so we just reverse the list first:

def digit_difference_sort(a):
    return sorted(reversed(a), key=max_digit_diff)

This should be vastly easier to read than your convoluted function. Note that the function names also follow Python's official style-guide, PEP8.

Like all (good) sorting functions, this is \$\mathcal{O}(n \log n)\$. Here is a timing comparison to your function with arrays up to length 10k (at which point your function takes more than a minute...).

Here is an implementation of the radix sort suggested by @JollyJoker in their answer:

from itertools import chain

def radix_sort(a):
    sub_a = [[] for _ in range(10)]
    for x in a:
        sub_a[max_digit_diff(x)].append(x)
    return list(chain.from_iterable(reversed(x) for x in sub_a))

This seems to have the same complexity as my approach, probably the implementation of max_digit_diff actually dominates this:


Remove row when everything in the row is same I have a code that will delete the row if the entire row is exactly the same. So if in row 1 I have 100,200,300,400,500,600 in column A,B,C,D,E,F respectively, and in row 3 I have the same thing, it will remove row 3.

Sub remove()
    Dim a As Long
    For a = Cells(Rows.Count, 1).End(xlUp).row To 1 Step -1
        If WorksheetFunction.CountIf(Range("A1:A" & a), Cells(a, 1)) > 1 Then
            If WorksheetFunction.CountIf(Range("B1:B" & a), Cells(a, 2)) > 1 Then
                If WorksheetFunction.CountIf(Range("C1:C" & a), Cells(a, 3)) > 1 Then
                    If WorksheetFunction.CountIf(Range("D1:D" & a), Cells(a, 4)) > 1 Then
                        If WorksheetFunction.CountIf(Range("E1:E" & a), Cells(a, 5)) > 1 Then
                            If WorksheetFunction.CountIf(Range("F1:F" & a), Cells(a, 6)) > 1 Then Rows(a).Delete
                        End If
                    End If
                End If
            End If
        End If
    Next
End Sub

However this code looks a little long and I was wondering if I could use a for loop but I have no idea on how to start. I am new to VBA coding which is why my methods could be a little hard coded. Any help is appreciated, Thank you!

(Edit) Here is a sample workbook that I have created.
In this workbook I have a few names on the first column and the next few columns are the points they obtain on different days. However, there are entries that have the exact same entry row, meaning, the points and the name is exactly the same and this is what I want to remove. in the sample workbook, the name james have the 2 exact same entries and therefore, it will remove 1 of it. However Lan and Denise does not have the same entries hence it remains after running the code.

The proper way to do this is use the Range.RemoveDuplicates method.

Here is the refactored macro created by clicking the Remove Duplicates button on the Data tab of the Ribbon.  Array(1, 2, 3, 4, 5, 6) contains all the column numbers that need matching data for a row to be deleted.

Application.ScreenUpdating = False
Range("A1").CurrentRegion.RemoveDuplicates Columns:=Array(1, 2, 3, 4, 5, 6), Header:=xlYes


Refactoring IEnumerable I have the below code which increases the code complexity. Any way to refactor it?

I am trying to refactor the below code so that the CCM can be reduced for the code. i.e code complexity matrix.

//// TODO: Needs refactoring
    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        var memberNames = new List<string>();

        if (this.DateOfBirth == DateTime.MaxValue)
        {
            memberNames.Add("DateOfBirth");
            yield return new ValidationResult("Invalid date, please enter your date of birth", memberNames);
        }
        else if (this.DateOfBirth == DateTime.MinValue)
        {
            memberNames.Add("DateOfBirth");
            yield return new ValidationResult("Invalid date, please enter your date of birth", memberNames);
        }

        if (!this.Age.HasValue)
        {
            memberNames.Add("DateOfBirth");
            yield return new ValidationResult("Invalid date, please enter your date of birth", memberNames);
        }
        else
        {
            if (this.Age < 18 || this.Age > 75)
            {
                memberNames.Add("DateOfBirth");
                yield return new ValidationResult("Age must be between 18 and 75 years", memberNames);
            }
        }

        if (!this.Gender.HasValue)
        {
            memberNames.Add("Gender");
            yield return new ValidationResult("This field is required", memberNames);
        }

        if (!(this.Age >= 60))
        {
            yield break;
        }

        // similarly many more validation checks - if-else conditions
    }

I don't know the other conditions but I would do it like this:

public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
{
    var memberNames = new List<string>();

    bool isValidDateOfBirth = DateOfBirth > DateTime.MinValue && DateOfBirth < DateTime.MaxValue && Age.HasValue;
    if (!isValidDateOfBirth)
    {
        memberNames.Add("DateOfBirth");
        yield return new ValidationResult("Invalid date, please enter your date of birth", memberNames);
    }

    bool isValidAge = Age.HasValue && Age.Value >= 18 && Age <= 75;
    if (!isValidAge)
    {
        memberNames.Add("DateOfBirth");
        yield return new ValidationResult("Age must be between 18 and 75 years", memberNames);
    }

    bool isValidGender = Gender.HasValue;
    if (!isValidGender)
    {
        memberNames.Add("Gender");
        yield return new ValidationResult("This field is required", memberNames);
    }
}


Spring MVC login controller I am making a small project in Spring MVC. My login controller looks like this:

@Controller
public class LoginController
{
   @Autowired
   private Environment env;

   private static final Logger LOGGER = LoggerFactory.getLogger(LoginController.class);

   private AuthenticationService authenticationService = AuthenticationService.getAuthenticationInstance();

   @RequestMapping(value = "/login.jsp" , method = RequestMethod.GET)
   public ModelAndView userLoginPage(){
    return new ModelAndView("Login");
   }

   @RequestMapping(value = "/login", method = RequestMethod.POST)
   public ModelAndView userLogin(@ModelAttribute LoginModel loginModel) {
    LOGGER.info("Employee trying to login [{}] ", loginModel.toString());
    ModelAndView loginView = new ModelAndView();
    loginView.addObject("loginModel", loginModel);
    String login = authenticationService.authenticateUser(loginModel,env);
    switch (login) {
    case "Authenticated":
        loginView.setViewName("dashboard");
        break;
    case "Authenticated First time Login":
        loginView.setViewName("changepassword");
        break;
    case "Not Authenticated":
        loginView.setViewName("Login");
        loginView.addObject("invalid", "invalid user/password");
        break;
    }

    return loginView;
 }

     @RequestMapping(value = "/changePassword" , method = RequestMethod.POST)
     public ModelAndView userPasswordChange(@ModelAttribute
       PasswordChangeModel passwordChangeModel){
     LOGGER.info("Employee trying to change password [{}]
       ",passwordChangeModel.getUserid());
     authenticationService.changePassword(passwordChangeModel, env);
     return new ModelAndView("passwordchangesuccess");
   }

 }

I am using switch case to delegate to different views. Is this the right approach or should I do something else to delegate response to views?

The switch has a bad reputation and is usually seen as a code smell.

In your case, I would first propose to replace the strings by one enum so that you have strong typing, you avoid typos errors and your Ide can warn you if you miss a case in your switch. By the way, it is a good practice to have a default case to handle and spot the unexpected cases (I usually throw a UnsupportedOperationException or IllegalArgumentException). You should of course avoid to have some MVC part into your AuthenticationService but that is a job for the adapters and his friends.

If you really want to remove this switch (I wish) then the idea is to use polymorphism. Basically, your AuthenticationService should return a subtype of a basic result class (LoginSuccess, LoginFailed) where the view name will be defined or computed.

About your controller itself, you should use @Autowired for Environment and AuthenticationService. Also note that Spring discourage the usage of autowired fields outside of tests classes.


Bruteforce hashes using Python 3 I've written a CLI tool in Python, for bruteforcing hashes (only md5 with this version). Apart from the fact that Python is not exactly suitable for bruteforcing (performance), what else can I do to improve my code (performance -otherwise-, styling, naming, argument handling etc.)?

from sys import argv, exit
from time import time
from hashlib import md5
from itertools import product
from datetime import datetime
from string import ascii_lowercase, ascii_uppercase, digits

hash_ = argv[1]
charset_ = argv[2]
minlength = argv[3]
maxlength = argv[4]

charset = {"1":  ascii_lowercase,
           "2":  ascii_uppercase,
           "3":  digits,
           "4":  ascii_lowercase
               + ascii_uppercase,
           "5":  ascii_lowercase
               + digits,
           "6":  ascii_uppercase
              + digits,
           "7":  ascii_lowercase
              + ascii_uppercase
              + digits
         }

 charset = charset[charset_]

 colors = {"R":"\033[91m",
           "G":"\033[92m",
           "~":"\033[00m"
         }

def bruteforce(hash_, characters, min_length, max_length):

    start = datetime.now()
    start_time = time()

    for length in range(int(min_length), int(max_length) + 1):
        products = product(characters, repeat=length)
        for attempt in products:
            hashed = "".join(attempt).encode("utf-8")
            hashed = md5(hashed).hexdigest()

            if hashed != hash_:
                print(f"{colors['R']}{''.join(attempt)}")

            else:
                diff = int(time()-start_time)
                print(f"{colors['G']}{''.join(attempt)}")
                print(colors["~"])
                print(" Statistics")
                print("~~~~~~~~~~~~~~~~~")
                print(f"Started:          {start}")
                print(f"Calculation time: {diff} seconds")
                print(f"Original hash:    {hash_}")
                print(f"Found string:     {''.join(attempt)}")
                print("~~~~~~~~~~~~~~~~~")

               return True

def main():
    print(colors["~"])
    bruteforce(hash_, charset, minlength, maxlength)
    print(colors["~"])

if __name__ == "__main__":
    main()
    exit()

1) You don't need to pass minlength and maxlength in your bruteforce function since you making it available at module level.

2) Also there are unwanted new-lines.

3) Also dict formatting can be improved a lot, this will help in readability of your code. Try using any online dict-formatter.

4) You can use datetime.now().time() instead of using a separate library time.

5) If possible avoid using too many print statements or atleast do that with proper messaging rather than any random string like ~~~~~~ you are using.


RAND() with a generated link I have a project from my college. I'm making a site which will display different quotations each time the page is loaded. I use the following code:

$con = mysql_connect("localhost","root","");
if (!$con){
die('Could not connect: ' . mysql_error());
}
mysql_select_db("xlsx_db", $con);

$result = mysql_query("SELECT * FROM sheet1 ORDER BY RAND() LIMIT 1");

if (!$result) {
echo 'Could not run query: ' . mysql_error();
exit;
}

$row = mysql_fetch_array($result);
echo "Quote: ";
echo $row['quote']."
";
echo $row['by']."
";
?>

All works fine. But my teacher told me to make the text link-able. So that if anyone copy that link of the quote, that case, he can pest the code on the browser and can see that quote again.

Could anyone please help me with this task? Any kind of help is really appreciated.

Your link should look like this:

Permalink

In your php code, you can grab the selected quote id like this:

$q_id = $_GET['q_id'];

To check if the q_id has been set:

if (array_key_exists('q_id', $_GET)) {
    // the q_id was specified
}

To check that the q_id is numeric (important to do before plugging it into a mysql query):

if (is_numeric($_GET['q_id'])) {
    // q_id is a number
}

To fetch that single quote:

$result = mysql_query("SELECT * FROM sheet1 WHERE q_id = " . $q_id);

Deprecation of mysql_ functions

  PHP functions that start with mysql_ have been deprecated as of PHP 5.5.0. If you are in a position to do so, please consider updating your code to use the MySQLi or PDO extensions instead.


Complex reporting query suggestion needed for different approach as per latest Tsql format We have recently migrated from SQL Server 2008 R2 to SQL Server 2017 web edition. I am not a SQL developer but enthuses to learn.
Since many of our scrips are of old SQL format, I am searching for some alternative to leverage new T-SQL formats to help optimize the codes.
Below is one example which I believe can be modified using some different approach. It will be helpful for me if someone suggest me some approaches so that I can learn in detail about them.
Below query is for report it uses UNION ALL, the first part fetches data of ID for which payments are done incorrectly marked as 'recoup' and second part fetched data for the same ID it amount of payment required to be done.
SELECT TOP 200 'Recoup',ERH_DOC_DATE,ERH_DOCNO,ERH_RECV_FROM,ERD_BLH_ID,BLD_CODE,A.ERD_ALLOWED_AMT,A.ERD_PAID_INS_AMT
FROM TRN_ERA_HEAD,TRN_ERA_DET A,TRN_BILLING_DET
WHERE ERH_STATUS = 'P' AND ERH_BOOL_INACTIVE = 0 AND A.ERD_BOOL_RECALLED=1 AND
A.ERD_ALLOWED_AMT <> 0 AND A.ERD_PAID_INS_AMT <> 0 AND
ERH_ID = A.ERD_ERH_ID AND
A.ERD_BLD_ID = BLD_ID AND
NOT EXISTS (SELECT B.ERD_ID FROM TRN_ERA_DET B WHERE A.ERD_ERH_ID = B.ERD_ERH_ID AND A.ERD_ID <> B.ERD_ID AND B.ERD_BLD_ID = A.ERD_BLD_ID AND B.ERD_BOOL_RECALLED = 0 AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT)
AND ERH_DOC_DATE>='01-01-2021' AND ERH_DOC_DATE<='05-31-2021'
UNION ALL
SELECT TOP 200 'Payment',EH1.ERH_DOC_DATE,EH1.ERH_DOCNO, EH1.ERH_RECV_FROM, ED1.ERD_BLH_ID,BD1.BLD_CODE,ED1.ERD_ALLOWED_AMT, ED1.ERD_PAID_INS_AMT FROM
( SELECT ERD_ERH_ID ERDERH_ID, ERD_BLH_ID erdblh_id, a.erd_bld_id bld, A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT FROM TRN_ERA_HEAD,TRN_ERA_DET A,TRN_BILLING_DET WHERE ERH_STATUS = 'P' AND ERH_BOOL_INACTIVE = 0 AND A.ERD_BOOL_RECALLED=1 AND A.ERD_ALLOWED_AMT <> 0 AND A.ERD_PAID_INS_AMT <> 0 AND ERH_ID = A.ERD_ERH_ID AND A.ERD_BLD_ID = BLD_ID AND NOT EXISTS
(SELECT B.ERD_ID FROM TRN_ERA_DET B WHERE A.ERD_ERH_ID = B.ERD_ERH_ID AND A.ERD_ID <> B.ERD_ID AND B.ERD_BLD_ID = A.ERD_BLD_ID AND B.ERD_BOOL_RECALLED = 0 AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT) AND ERH_DOC_DATE>='01-01-2021'
AND ERH_DOC_DATE<='05-31-2021') AS RECOUPS,
TRN_ERA_HEAD EH1,TRN_ERA_DET ED1, TRN_BILLING_DET BD1 WHERE RECOUPS.ERDERH_ID < ED1.ERD_ERH_ID AND ED1.ERD_BLD_ID = RECOUPS.BLD AND EH1.ERH_ID=ED1.ERD_ERH_ID AND ED1.ERD_BLD_ID=BD1.BLD_ID AND EH1.ERH_TRNTYPE='IN' AND EH1.ERH_STATUS = 'P' AND EH1.ERH_BOOL_INACTIVE = 0 AND ED1.ERD_BOOL_RECALLED=0 AND ED1.ERD_ALLOWED_AMT <> 0 AND ED1.ERD_PAID_INS_AMT <> 0;

Please suggest me some different approaches. Thank you in advance.

To start, here is a potential formatting that makes the query less of a tragedy to read:
SELECT TOP 200
    'Recoup', ERH_DOC_DATE, ERH_DOCNO, ERH_RECV_FROM, ERD_BLH_ID,
    BLD_CODE, A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT
FROM TRN_ERA_HEAD, TRN_ERA_DET A, TRN_BILLING_DET
WHERE ERH_STATUS = 'P'
    AND ERH_BOOL_INACTIVE = 0
    AND A.ERD_BOOL_RECALLED = 1
    AND A.ERD_ALLOWED_AMT <> 0
    AND A.ERD_PAID_INS_AMT <> 0
    AND ERH_ID = A.ERD_ERH_ID
    AND A.ERD_BLD_ID = BLD_ID
    AND NOT EXISTS (
        SELECT B.ERD_ID
        FROM TRN_ERA_DET B
        WHERE A.ERD_ERH_ID = B.ERD_ERH_ID
            AND A.ERD_ID <> B.ERD_ID
            AND B.ERD_BLD_ID = A.ERD_BLD_ID
            AND B.ERD_BOOL_RECALLED = 0
            AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT
            AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
    )
    AND ERH_DOC_DATE >= '01-01-2021'
    AND ERH_DOC_DATE <= '05-31-2021'
UNION ALL
SELECT TOP 200
    'Payment', EH1.ERH_DOC_DATE, EH1.ERH_DOCNO, EH1.ERH_RECV_FROM,
    ED1.ERD_BLH_ID, BD1.BLD_CODE, ED1.ERD_ALLOWED_AMT, ED1.ERD_PAID_INS_AMT
FROM (
    SELECT
        ERD_ERH_ID ERDERH_ID, ERD_BLH_ID erdblh_id, a.erd_bld_id bld,
        A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT
    FROM TRN_ERA_HEAD, TRN_ERA_DET A,TRN_BILLING_DET
    WHERE ERH_STATUS = 'P'
        AND ERH_BOOL_INACTIVE = 0
        AND A.ERD_BOOL_RECALLED = 1
        AND A.ERD_ALLOWED_AMT <> 0
        AND A.ERD_PAID_INS_AMT <> 0
        AND ERH_ID = A.ERD_ERH_ID
        AND A.ERD_BLD_ID = BLD_ID
        AND NOT EXISTS (
            SELECT B.ERD_ID
            FROM TRN_ERA_DET B
            WHERE A.ERD_ERH_ID = B.ERD_ERH_ID
                AND A.ERD_ID <> B.ERD_ID
                AND B.ERD_BLD_ID = A.ERD_BLD_ID
                AND B.ERD_BOOL_RECALLED = 0
                AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT
                AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
        )
        AND ERH_DOC_DATE >= '01-01-2021'
        AND ERH_DOC_DATE <= '05-31-2021'
    ) AS RECOUPS,
    TRN_ERA_HEAD EH1,
    TRN_ERA_DET ED1,
    TRN_BILLING_DET BD1
WHERE RECOUPS.ERDERH_ID < ED1.ERD_ERH_ID
    AND ED1.ERD_BLD_ID = RECOUPS.BLD
    AND EH1.ERH_ID = ED1.ERD_ERH_ID
    AND ED1.ERD_BLD_ID = BD1.BLD_ID
    AND EH1.ERH_TRNTYPE = 'IN'
    AND EH1.ERH_STATUS = 'P'
    AND EH1.ERH_BOOL_INACTIVE = 0
    AND ED1.ERD_BOOL_RECALLED = 0
    AND ED1.ERD_ALLOWED_AMT <> 0
    AND ED1.ERD_PAID_INS_AMT <> 0;

Aside from the valid points that @Squirrel has already raised:

You'll want to change your date literal format to something that is non-locale-dependent; read https://blog.greglow.com/2020/04/27/t-sql-101-67-literal-date-and-time-values-in-sql-server-t-sql/ for instance
Choose a consistent case; right now yours isn't. My personal preference is lowercase-everything (keywords and symbols) to avoid shouting, but even if you upper-case everything it will be more consistent than what you have now.


Count cars passing in opposite directions The problem comes from codility, whose coding problems I'm starting to enjoy as their evaluation criteria really looks at runtime.

Task description

  A non-empty zero-indexed array A consisting of N integers is
  given. The consecutive elements of array A represent consecutive
  cars on a road.

  Array A contains only 0s and/or 1s:

  0 represents a car traveling east,
  1 represents a car traveling west.

  The goal is to count passing cars. We say that a pair of cars (P, Q),
  where 0 ≤ P < Q < N, is passing when P is traveling to the east and
  Q is traveling to the west.

  For example, consider array A such that:

  A[0] = 0
  A[1] = 1
  A[2] = 0
  A[3] = 1
  A[4] = 1

  We have five pairs of passing cars: (0, 1), (0, 3), (0, 4), (2, 3),
  (2, 4).

  Write a function:

def solution(a)

  that, given a non-empty zero-indexed array A of N integers, returns the number of passing cars.

  The function should return −1 if the number of passing cars exceeds
  1,000,000,000.

  For example, given:

  A[0] = 0
  A[1] = 1
  A[2] = 0
  A[3] = 1
  A[4] = 1

  the function should return 5, as explained above.

  Assume that:

  N is an integer within the range [1..100,000];
  each element of array A is an integer that can have one of the following values: 0, 1.

  Complexity:

  expected worst-case time complexity is \$O(N)\$;
  expected worst-case space complexity is \$O(1)\$, beyond input storage (not counting the storage required for input arguments).

  Elements of input arrays can be modified.

Solution

I've tried my hand at 2 different solutions, one where I delete elements from the array but that was slower than my first pass like this:

def solution(a)
  results = 0
  a.each_with_index do |el, i|
    if el == 0
      j = i
      while j <= a.size-1
        results += 1 if a[j] != 0
        j += 1
      end
    end
    return -1 if results > 1000000000
  end
  results
end

I understand why this is \$O(n^2)\$, but I'm unsure how to make this faster.

def solution(a)
  results = 0
  east_count = 0
  a.each do |el|
    if el == 0
      east_count += 1
    else
      results += east_count
    end
    return -1 if results > 1000000000
  end
  results
end

Note that you don't have to compare each car to every other car.  You just have to track how many are traveling east and each time you find someone going west, increment your results by the eastbound count.  I find this easier to understand than tracking the westbound cars backwards.

r | c
--+--
0 | 0
0 | 1
1 | 1 (0,1)
1 | 2
3 | 2 (0,3), (2,3)
5 | 2 (0,4), (2,4)

In the table, r stands for results and c stands for east_count.  The pairs found at any point are to the right.

This will not find the pairs that pass, but it does count them.  It maintains the required \$P < Q\$ by order of iteration.

I don't know much about Ruby, so I won't try to make syntactic comments.  Apologies if my own syntax is weak.

This iterates through the array once, so it's \$O(n)\$ in time.  It defines a constant number of scalar variables, so it's \$O(1)\$ memory beyond the input array.


Variadic template class that stores a vector of each type given I'm exploring the possibility to generalise a class of mine that stores some entities. I like it to be able to keep entities of several type, given as template arguments. To explore the possibilities I wrote the following class, that does exactly that. I'm however unsure if anything that I do here is a) save and b) as easy as it gets. I'm not very deep into metaprograming. I'm looking for tips how to improve and pitfalls.

#include <iostream>
#include <tuple>
#include <vector>

template<typename... Ts>
class A
{
    template<typename T>
    using container = std::vector<T>;

public:
    template<typename T>
    void add(T e) {
        std::get<container<T>>(m_tupel).emplace_back(e);
    }

    template<typename T>
    container<T>& get() {
        return std::get<container<T>>(m_tupel);
    }

private:
    std::tuple<container<Ts>...> m_tupel;
};

int main()
{
    A<int, bool, double, float> a;
    a.add(3);
    a.add(33);
    a.add(3.14);
    a.add(333);
    a.add(31.4);

    const auto& cDouble = a.get<double>();
    const auto& cInt = a.get<int>();

    for(auto i : cDouble)
        std::cout<<i<<" ";
    std::cout<<"\n";
    for(auto i : cInt)
        std::cout<<i<<" ";
}

A few points of criticism of your approach:

Prefer protected to private

This isn't really about your overall design, but - unless you have very good reason, use protected rather than private member. In few cases is it useful to hide data from your subclasses.

"Where did my data go?!"

Suppose you have vectors types T_1 and T_2 in your tuple, and you add() an element of type T_3, which may be implicitly cast into both a T_1 and a T_2. Where will it go? The answer is: It depends on which casting is preferred, which in turns depends both on language defaults and on your own choices of constructors and casting operations for T_1, T_2 and T_3.

Do you remember with certainty which implicit casts are preferred over which? I know I don't, and I write C++ all the time. Now, ok, maybe you're a smart guy and have that all memorized. What about the next person reading your code? And what if it's in code which itself is templated? And what if somebody adds or removes some T_1, T_2 or T_3 code? It seems to me this is just asking for trouble.

If your data goes in a vector other than the one you expect, you'll have to start searching vectors you have trouble even referring to, for that data.

So, you asked about what's "safe" - I would say this is not too safe.

Motivation

This is not a problem with the code, it's a problem with your presentation. Why would you need a tuple of vectors of arbitrary types? Are you really sure that's what you need? Not, say, an array of std::variants like one of the comments suggests? Or a class with named vector, e.g.

struct A {
    std::vector<Foo> foos;
    std::vector<Bar> bars;
}

Now, this is much less "fancy" and "hi-tech" and modern-C++'ish, but it is simpler and easier to debug. There are also other points in the design space. I just find it a bit hard to believe that you would really need a completely generic tuple of typed vectors.


Async Queue Processing I have a Queue that needs to try to process when anything updates in it (add, remove or update).

However, I don't want any of the callers to wait while the processing is happening (either for the processing to happen or while the processing is happening).

This is what I came up with:

private static readonly SemaphoreSlim asyncLock = new SemaphoreSlim(1);
private async void ProcessQueue()
{
    // Lock this up so that one thread at a time can get through here.
    // Others will do an async await until it is their turn.
    await asyncLock.WaitAsync();
    try
    {
        // Offload this to a background thread (so that the UI is not affected)
        var queueProcessingTask = Task.Run( () =>
        {
            var processingStuck = false;
            while (myQueue.Count >= 1 && !processingStuck)
            {
                // Get the next item
                var queueItem = myQueue.Peek();
                // Try to process this one. (ie DoStuff)
                processingStuck = ProcessQueueItem(queueItem);
                // If we processed successfully, then we can dequeue the item
                if (!processingStuck)
                    myQueue.Dequeue();
            }
        });
        Task.WaitAll(queueProcessingTask);
    }
    finally
    {
        asyncLock.Release();
    }
}

Is my Thread and async handling going to ensure that only 1 queueItem at a time can ever be "in the works"?

And will this avoid using resources from the UI thread?

The thread that calls it will likely be the UI Thread.

You're invoking Task.WaitAll which waits for the processing to finish.
However you said, "... I don't want any of the callers to wait while the processing is happening (either for the processing to happen or while the processing is happening)."
Is this a contradiction?
Instead of creating short-lived tasks as a local variable inside a semaphore, perhaps it would be better to:

Create a single long-lived task (e.g. as a data member of your class)
Wake up the task when you enqueue something or fix a ProcessQueueItems problem (perhaps by setting a event which the task is waiting on)
Don't invoke Task.WaitAll until your program is shutting down
Put an exception handler inside the task so that the (single, supposedly long-lived) task can't permanently fail if one ProcessQueueItem call throws an exception

It is a custom type of mine. Has a few native types and a dictionary.

It needs to be a type like ConcurrentQueue<T> because you have one thread (the task) dequeueing from it while (presumably) other threads are enqueueing on it.


Drop-down selectors for state and city I have this old code and I want it to be easier to understand. Originally, it loaded using an onload on the opening body tag, so I managed to change that to a window.onload, but now redundant code is getting elements by ID, since it first finds the form and after that the actual select box.

var localidades = new Array(
  new Array("Baja California","Tijuana","Rosarito","Ensenada","Tecate","Mexicali","San Felipe"),
  new Array("Sonora","San Luis Río Colorado","Sonoita","Sásabe","Nogales","Naco","Agua Prieta"),
  new Array("Chihuahua","El Berrendo","Puerto Palomas","San Jerónimo","Ciudad Juárez","El Porvenir","Ojinaga"),
  new Array("Coahuila","Ciudad Acuña","Piedras Negras"),
  new Array("Nuevo León","Colombia"),
  new Array("Tamaulipas","Nuevo Laredo","Reynosa","Río Bravo","Nuevo Progreso","Matamoros")
);
function seleccionLocal(selection){
  var cajon = document.getElementById("forma1").city;
  for(var x=0;x<localidades.length;x++){
    if(selection==localidades[x][0]){
      var cities = localidades[x];
      cajon.length = cities.length;
      for(var n=1;n<cities.length;n++){
        cajon[n] = new Option(cities[n],cities[n]);
      }
      break;
    }
  }
}
window.onload = function StartIt(){
  var cajon = document.getElementById("forma1").country;
  cajon.length = localidades.length+1;
  for(var x=0;x<localidades.length;x++){
    cajon[x+1] = new Option(localidades[x][0],localidades[x][0]);
  }
}
<form id="forma1" action="" onsubmit="" style="margin:auto;">
    <select name="country" onchange="seleccionLocal(this.options[this.selectedIndex].value)">
    <option value="-1" >Estado</option>
  </select>
  <select name="city">
    <option value="-1" >Ciudad</option>
  </select>
</form>

It's working, but I guess it can be lighter. What say You?

Mixing English and Spanish in code is always awkward.  I don't recommend it.  If you must do it, though, try to be consistent, and avoid mixing in English identifiers like var cities.

The localidades array, in which the first string of each member array is special and represents a state, is weird.  An associative array, with the state names as keys, would be better.

In modern practice, the HTML should be free of all JavaScript, and the event hooks are added from the JavaScript.  addEventListener(), which allows the code to add multiple handlers, is preferable to window.onload = …, which only allows one handler.

The "Estado" and "Ciudad" placeholder options should be disabled to prevent them from being selected or submitted.

Below, I've defined a function, selectorDeEstadoYCiudad, which acts as a controller for the two drop-downs.

var localidades = {
  "Baja California": ["Tijuana","Rosarito","Ensenada","Tecate","Mexicali","San Felipe"],
  "Sonora": ["San Luis Río Colorado","Sonoita","Sásabe","Nogales","Naco","Agua Prieta"],
  "Chihuahua": ["El Berrendo","Puerto Palomas","San Jerónimo","Ciudad Juárez","El Porvenir","Ojinaga"],
  "Coahuila": ["Ciudad Acuña","Piedras Negras"],
  "Nuevo León": ["Colombia"],
  "Tamaulipas": ["Nuevo Laredo","Reynosa","Río Bravo","Nuevo Progreso","Matamoros"],
};

function selectorDeEstadoYCiudad(localidades, forma) {
  forma.state.addEventListener('change', function (e) {
    var cajon = forma.city;
    var ciudades = localidades[e.target.value];
    for (var i = 0; i < ciudades.length; i++){
      cajon[i+1] = new Option(ciudades[i], ciudades[i]);
    }
    cajon.length = ciudades.length + 1;
    cajon.value = cajon[0].value;
  });

  var cajon = forma.state;
  for (var estado in localidades) {
    cajon.appendChild(new Option(estado, estado));
  }
}

window.addEventListener('DOMContentLoaded', function() {
  selectorDeEstadoYCiudad(localidades, document.getElementById('forma1'));
});
<form id="forma1" action="">
  <select name="state">
    <option disabled>Estado</option>
  </select>
  <select name="city">
    <option disabled>Ciudad</option>
  </select>
</form>


Object Oriented PHP Url Router I just finished rewriting my Router class. Any input would be much appreciated.

The class routes URLs based on a site.com/class/method/param/param/etc syntax.

One thing I've done differently is created a mechanism for basic templates to be loaded directly without the need for a controller. I often find pages such as /about/ do not require additional methodology, and setting up really basic controllers just seems like a waste of time. Therefore, I have created a check for templates such as _about.php in the views folder, with the underscore denoting that it can be loaded directly.

The class also currently assumes an error class or template.

class Router{

    public static function getPath($uri = null){
        // set default uri
        $uri = is_null($uri) ? $_SERVER['REQUEST_URI'] : $uri;

        // remove possible query string
        $uri = preg_replace('/\?(.*)/', '', $uri);

        return explode('/',trim($uri,'/'));
    }

    public static function getQuery($uri = null){

        // set default uri
        $uri = is_null($uri) ? $_SERVER['REQUEST_URI'] : $uri;

        // get query string
        $query = substr($uri, strpos($uri,'?') + 1);

        // parse into array
        $q = array();
        parse_str($query,$q);

        return $q;
    }

    public static function follow($uri = null){

        $path = self::getPath($uri);

        // get controller and method, or set defaults of root and index
        $controller = ($path[0] == "") ? 'root' : $path[0];
        $method = ((count($path) > 1) && ($path[1] != "")) ? $path[1] : null;

        // see if controller is a static page, only possible if no method specified
        if(is_null($method) && file_exists(APPLICATION_DIR . '/views/_' . $controller . '.php')){
            include APPLICATION_DIR . '/views/_' . $controller . '.php';
        }
        else if(file_exists(APPLICATION_DIR . '/controllers/' . $controller . '.php')){ // see if controller exists
            // load controller class
            include_once APPLICATION_DIR . '/controllers/' . $controller . '.php';

            // if method is empty or doesn't exist, it defaults to "index"

            if(is_null($method) || !method_exists($controller,$method)) {
                $method = "index";
            }

            if(method_exists($controller, $method)){
                $c = new $controller();
                $c->$method();
            }
            else{
                self::follow('e404');
            }
        }
        else{
            self::follow('e404');
        }

    }

}

This is not OOP.

Observe how your class contains only static functions.  There are no class properties.  This can be written equivalently:

namespace Router;

function getPath($uri = null){
    // Same implementation (I have just cut it out for readability).
}

function getQuery($uri = null){
    // Same implementation.
}

function follow($uri = null){
    // Same implementation.
}

Usage from anywhere can be compared (First your code):

Router::getPath($uri);
Router::getQuery($uri);
Router::follow($uri);

Then the equivalent:

Router\getPath($uri);
Router\getQuery($uri);
Router\follow($uri);

Note how no object is required to be used (that is a good hint that its not OO).

The following question would be worth reading:

User class design

I have an answer to that question which covers in more detail some of the problems with using static in OOP.


Scala: Are assertions acceptable outside of test context? In the case that Type is not an option, data must exist so I just use an assert to clarify and throw an exception if data is missing. Is there a better way to do this? I think in Java this would be acceptable but I know the scala crowd tends to have a different idea of what idiomatic use of the language is.

private def convertData(dataOption: Option[ByteString], tpe: Type): Any = {

    if(tpe.<:<(typeOf[Option[Any]])){
      dataOption match{
        case Some(x) => ???
        case None => None
      }
    } else{
      assert(dataOption.isDefined)
      ???
    }
}

In short: yes, assertions make perfect sense outside testing and can raise the confidence that the code is actually correct.

Design by contract

What you like to assert, namely, that "dataOption must be defined if tpe does not represent a subtype of Option[Any]" can be considered a precondition of your method. That is, a condition over the parameters (and fields and global state) that needs to hold in order for your method to run successfully. The method implementer states such conditions and can then rely on them, whereas the caller is responsible for ensuring that they hold. The dual concept are postconditions, where the caller gets the guarantee which the implementer must ensure.

Languages such as Eiffel and Spec# natively support this so-called design by contract approach to programming. Natively, because contract elements such as preconditions, postconditions and invariants are built into the language. There is a significant difference, however, when the contracts are looked at. Eiffel checks contracts at run-time, which is quite similar to testing in the sense that only concrete executions are considered. Spec#, on the other hand, verifies contracts statically, which means that it tries to assert that the contract holds for all possible executions. The latter is very challenging and not always possible, but it yields much stronger guarantees in case it is. The former is easier, but gives you weaker guarantees.

Other languages lack native support for such contracts, for example Java or C#. In order to design by contract, Java developers can use JML and C# developers can use Code Contracts. JML is based on Java annotations and can be used for static verification and run-time checking, as can Code Contracts be, but they are implemented as a regular library.

Scala

Scala also lacks built-in support for contracts, but there are quite a few projects that try to work around this. Amongst others, there are:

Very simple run-time checked pre- and postconditions described in the paper Contracts for Scala
Static verification of functional Scala code with Leon
Run-time checking of Scala with a Code-Contracts-like library (project report)
Static verification of possibly non-functional Scala code (project description, the report will be only soon)

The answer

Back to your question: You use assertions (and the control flow) to check the precondition of your method, hence, you kind of do design by contract - which is good, because it makes it more likely that errors are detected close to where they are caused. These checks are basically like small unit tests and thus can increase the confidence in your code.

In order to make the method's precondition as explicit as possible, I would move the assertion to the begin of the method

private def convertData(dataOption: Option[ByteString], tpe: Type): Any = {
  assert(tpe.<:<(typeOf[Option[Any]]) || dataOption.isDefined,
         "<put helpful error message here>")

  ...
}

You could also put assertions at all return points of the method in order to mimic postconditions.

In case you worry about performance, you can use the fact the assert is annotated as @elidable and thus can be removed by the compiler, for example, for a production build.


Prototype Design Pattern C++ I have just started reading the GO4 book to learn the OOD concepts.
In order to practice the Prototype pattern, I implemented a small example (the idea for colored shapes was taken from "refactoring.guru"). Following is my code with some questions beneath.
Prototype definition:
enum class Shape
{
    Circle,
    Rectangle,
};

class ColoredShapePrototype
{
protected:
    std::string color_;
public:
    ColoredShapePrototype() {}
    ColoredShapePrototype(std::string color) : color_(color) {}
    virtual ~ColoredShapePrototype() {}
    virtual ColoredShapePrototype* Clone() const = 0;
    virtual void ShapeDetails() { std::cout << "Color: " << color_ << "\n"; }
    virtual void UpdateColor(int color) { color_ = color; }
};

class ColoredCirclePrototype : public ColoredShapePrototype
{
private:
    int radius_;
public:
    ColoredCirclePrototype(std::string color, int raduis) : ColoredShapePrototype(color), radius_(raduis) {}
    ColoredShapePrototype* Clone() const override { return new ColoredCirclePrototype(*this); }
    void ShapeDetails() { ColoredShapePrototype::ShapeDetails(); std::cout << "Radius: " << radius_ << "\n"; }
};

class ColoredRectanglePrototype : public ColoredShapePrototype
{
private:
    int height_;
    int width_;
public:
    ColoredRectanglePrototype(std::string color, int height, int width) : ColoredShapePrototype(color), height_(height), width_(width) {}
    ColoredShapePrototype* Clone() const override { return new ColoredRectanglePrototype(*this); }
    void ShapeDetails() { ColoredShapePrototype::ShapeDetails(); std::cout << "Height: " << height_ << "\nWidth:" << width_ << "\n"; }
};

class ShapesPrototypeFactory
{
private:
    std::unordered_map<Shape, ColoredShapePrototype*> prototypes_;
public:
    ShapesPrototypeFactory() {
        prototypes_[Shape::Circle] = new ColoredCirclePrototype("White", 5);
        prototypes_[Shape::Rectangle] = new ColoredRectanglePrototype("White", 2, 3);
    }
    ~ShapesPrototypeFactory() {
        delete prototypes_[Shape::Circle];
        delete prototypes_[Shape::Rectangle];
    }
    ColoredShapePrototype* CreatePrototype(Shape shape) { return prototypes_[shape]->Clone(); }
};

Usage in main:
ShapesPrototypeFactory prototype_factory;
ColoredShapePrototype* circ = prototype_factory.CreatePrototype(Shape::Circle);
circ->ShapeDetails();
ColoredShapePrototype* rect = prototype_factory.CreatePrototype(Shape::Rectangle);
rect->ShapeDetails();
delete circ;
delete rect;

Questions:

Interfaces in general - pure virtual functions and other variables are declared, and derived classes should implement those. Suppose I want to add a specific capability (with unique data member and member function) to the derived - Does it contradict the interface idea? (Because, if I have a pointer from base to derived, this pointer will not recognize the specific method...). If it is OK, Is there a way to execute a derived function through the interface(who doesnt have this function..)?

while debugging and watching circ variable, I can see the variable in the memory window, and I see "white" color, but I cant find the radius there. Is there a way i can see specific data member address in the memory? (I have tried circ->radius_ etc. none was working..)

I have seen that most implementations are using "unique_ptr". I know its basic usage, but how critical it is? and why not use it all the times rather than a regular pointer actually? and where are the places I should use it in the prototype pattern?

Implementing Copy Constructor in case of non-stack variables (shallow/deep copying) for the clone operation - Is a CCtor in the base class is sufficient or the derived is required as well?

Code review - better syntax/design/ideas/improvements?

Thanks in advance, I will highly appreciate your help!

Let's go through your questions.

No, it does not contradict the idea.
The base class should define the common functionality of the derived classes, that's needed by some system to operate on such objects. However, part of the code of that system, or a different part of the code base may be dedicated to working only with some of the derived classes, in which case the functions and variables should not be defined in the base class.
Using your example, a rendering system may require a Render() method and variables  defining the visual characteristics of a shape, which should be part of the colored shape interface. On the other hand, a system that works with shapes that have corners will not be expected to work on circles. In which case you would dynamic_cast a shape to the derived type in that system and call the methods specific to the derived class.

Depending on the debugger you're using, you are probably using a watch window to view the values of the circ variable. The watch window will display circ as a variable of type ColoredShapePrototype*, which only has a color variable.
In Visual studio, for example, watch window will allow you to cast the variable to a ColoredCirclePrototype*, by typing (ColoredCirclePrototype*)circ. You will then see the radius member of circ.

Smart pointers avoid common problems with managing dynamically allocated memory. unique_ptr specifically avoids leaks by deallocating the memory at the end of the scope, unless it is moved to another unique_ptr. If the clone() function returns a unique_ptr, this means the user of your prototype pattern code won't need to bother remembering to dealocate the memory. However, you are enforcing him to use a specific implementation of a smart pointer, in this case the standard library's unique_ptr. If this prototype pattern is part of a library and used in a project that has a different implementation of unique_ptr, it can become more complicated to manage this difference than if you return a plain raw pointer.

If a derived class has members that need to be deep copied, then that class needs to define a copy constructor that performs the deep copy.

My feedback

The most significant issue I see is that someone else can further derive a class from your derived classes, and there's nothing to remind them that the clone method needs to be overriden. Let's consider creating a ColoredSquarePrototype that inherits ColoredRectanglePrototype. If we create a square object and try to clone it, we would call the ColoredRectanglePrototype::clone() which copy constructs a rectangle, not a square. This is not easy to solve, one option is to declare your derived classes final, so they cannot be further derived. If you wish to allow derivations, you could read further about the issue in this blog post: https://katyscode.wordpress.com/2013/08/22/c-polymorphic-cloning-and-the-crtp-curiously-recurring-template-pattern/
I don't see the benefit of providing a default constructor in
ColoredShapePrototype, it just allows the usage of shapes with an
unitialized color ("").
You should pass a const string& to the constructors to avoid copying.
Removing "Prototype" from the name of the shape classes will convey better that these classes represent actual shapes instead of some auxiliary implementation.
The virtual void UpdateColor(int color) incorrectly takes an integer parameter instead of a string. This compiles, because the integer is converted to a char for std::string::operator=(const char) to be invoked.


Floating point representation in IEEE 754 format The following code takes a decimal number and gives its IEEE 754 floating point representation. However, the code is not complete and I am totally fine because it does the job for me:

It does not consider negative numbers.
The input number is not a fraction. Its always in the form xx.yy
As of now, it does not give representation in 32-bit or 64-bit format, it just gives exponent and significant value.

function toBinary (decimal) {
    return decimal.toString(2);
}

function getIntegerPart (binary){
    return binary.slice(0, binary.indexOf('.'));
}

function getFractionalPart (binary){
    return binary.slice(binary.indexOf('.')+1);
}

function toNormalize (binary) {
    var normalizedRep = new Object();
    exponentValue = binary.indexOf('.')-1;
    floatPosition = binary.indexOf('.');
    normalizedRep.floatRep = binary[0]+ '.' + binary.slice(1, floatPosition) + binary.slice(floatPosition+1);
    normalizedRep.exponentValue = exponentValue;
    normalizedRep.floatPosition = floatPosition;
    return normalizedRep;
}

function getSignificand (normalizedRep) {
    return normalizedRep.floatRep.slice(normalizedRep.floatRep.indexOf('.')+1);
}

function twosComp() {
    var decimal = 15.22;
    var binary = toBinary(decimal);
    var integerPart = getIntegerPart(binary);
    var fractionalPart = getFractionalPart(binary);
    var floatPosition = binary.indexOf('.');
    var normalizedRep = toNormalize(binary);
    var significand = getSignificand(normalizedRep);
    var exponentValue = normalizedRep.exponentValue;

    console.log(binary);
    console.log(floatPosition);
    console.log(normalizedRep.floatRep);
    console.log(significand);
    console.log(exponentValue);
}

I just don't get what you're trying to do. JavaScript does not know of integers, floats or any other numeric type, except for Number.
This data-type just happens to be a 64bit IEEE754 floating point value (or double in most strong-typed languages). This is clearly described in the official language specs

primitive value corresponding to a double-precision 64-bit binary format IEEE 754 value
NOTE A Number value is a member of the Number type and is a direct representation of a number.

So a float representation of a number is the number itself. If you want n number of significant digits, then there's a handy method for that:
var num = 123.321;
console.log(num.toFixed(2));//123.32

If you want the integer part of a non-integer number:
parseInt(num, 10);//123 (best to specify the radix)

If you want the remainder, either one of the following approaches will do:
num - parseInt(num, 10);
//or
num%1//modulo || remainder of division by 1

Note that in this last case, you will see the rounding errors of the IEE754 spec show up, so:
num = 123.321;
//get length of string rep for total number - int length,  -1 for .
var fix = (""+num).length - (""+parseInt(num, 10)).length - 1;
(num%1).toFixed(fix);

Might be required.
As far as the fraction representation of a number is concerned, that is going to take some work. Thankfully, it's all been done before and the code is available on github.
I don't know what you mean by: "It does not consider negative numbers", because, like I said before: there is but one type for numbers in JS, and so it's always signed. Just use the absolute value, and keep track of the what the number was initially (prositive or negative) like so:
function processNumber(num)
{
    var sign = !(num > 0);//bool, true if num is negative
    var workVal = Math.abs(num);//get absolute value
    var returnObj = {};
    returnObj.intVal = parseInt(workVal, 10);
    returnObj.floatPart = (workVal%1).toFixed((""+workVal).length - (""+returnObj.intVal).length - 1);
    returnObj.fixed2 = workVal.toFixed(2);
    //and so on...
    if (sign === true)
    {
        for (prop in returnObj)
        {
            if (returnObj.hasOwnProperty(prop))
            {//they're all strings, just prepend the sign...
                returnObj[prop] = '-' + returnObj[prop];
            }
        }
    }
    return returnObj;


Tarjan's algorithm for Strongly Connected Components without keeping SCC in stack I've been looking for an implementation of Tarjan's algorithm in C#/C++ for a strongly-connected components in graphs, and couldn't find any implementation without use of additional Stack - most of them just follow wikipedia pseudocode. I've decided to create one in case if someone is looking for it. Please take a look and codereview. The main point is that mark the SCC as completed once I'm done with descendand processing, and that any cross-edge towards completed SCC will be ignored.

using System;
using System.Collections.Generic;
using System.Linq;

public class Graph
{
    private int _verticesCount;
    private List<int>[] _vertexAdjancedVertices; // i-th element contains info about all adjanced vertices of vertex #i

    public Graph(int[,] edges)
    {
        _verticesCount = edges.Cast<int>().Max() + 1;
        _vertexAdjancedVertices = new List<int>[_verticesCount];
        for (int i = 0; i < _verticesCount; ++i)
            _vertexAdjancedVertices[i] = new List<int>();

        for(int i = 0; i < edges.GetLength(0); ++i)
            AddDirectedEdge(edges[i, 0], edges[i, 1]);
    }

    public void AddEdge(int vertex1, int vertex2, bool directed = false)
    {
        AddDirectedEdge(vertex1, vertex2);
        if (!directed)
            AddDirectedEdge(vertex2, vertex1);
    }

    public void AddDirectedEdge(int vertex1, int vertex2)
    {
        _vertexAdjancedVertices[vertex1].Add(vertex2);
    }

    public List<List<int>> GetStronglyConnectedComponents()
    {
        //DFS
        var processed = new bool[_verticesCount];
        var minConnectedValue = new int[_verticesCount];
        var sccCompleted = new bool[_verticesCount];
        int currentTime = 0;

        for(int startingVertex = 0; startingVertex < _verticesCount; ++startingVertex)
            if (!processed[startingVertex])
                GetStronglyConnectedComponents(startingVertex, ref currentTime, processed, minConnectedValue, sccCompleted);

        var res = minConnectedValue.Select((mcv, i) => new {Vertex = i, MinConnectedValue = mcv})
            .GroupBy(vmcv => vmcv.MinConnectedValue)
            .Select(g => g.Select(vmcv => vmcv.Vertex).ToList()).ToList();
        return res;
    }

    private void GetStronglyConnectedComponents(int vertex, ref int currentTime, bool[] processed, int[] minConnectedValue, bool[] sccCompleted)
    {
        processed[vertex] = true;
        ++currentTime;
        //var currentDiscoveryTime = currentTime;
        minConnectedValue[vertex] = currentTime; // initialize to current time
        sccCompleted[vertex] = false;
        foreach (var neighbour in _vertexAdjancedVertices[vertex])
        {
            if (!processed[neighbour])
            {
                GetStronglyConnectedComponents(neighbour, ref currentTime, processed, minConnectedValue, sccCompleted);
                minConnectedValue[vertex] = Math.Min(minConnectedValue[vertex], minConnectedValue[neighbour]); // if we will ever find cycle
            }
            else if (!sccCompleted[minConnectedValue[neighbour]]) // ignore references to completed sccs
            {
                minConnectedValue[vertex] = Math.Min(minConnectedValue[vertex], minConnectedValue[neighbour]); // we've reached processed vertex - use it as a minConnectedValue we could reach to (if smaller)
            }
        }
        if (minConnectedValue[vertex] == vertex) // we are going up to the stack, meaning that we are done with all the descendands
            sccCompleted[vertex] = true; // mark as completed in case if we are the root of current scc
    }
};

Usage:

    var g = new Graph(new[,] {
        {1, 2}, {2, 3}, {2, 4}, {2, 5}, {3, 1}, {4, 1}, {4, 6}, {4, 8}, {5, 6}, {6, 7}, {7, 5}, {8, 6}
    });

    var sccs = g.GetStronglyConnectedComponents();

    Console.WriteLine("g1");
    foreach (var scc in sccs)
        Console.WriteLine(String.Join(",",scc)); // print out

Brace Usage

It is generally considered best practice to always use braces with control statements (such as "if" and "for" statements). If you are concerned with whitespace, consider that placing the opening brace on the same line as the control statement means that the total increase in code size is only 1 line (for the closing brace). It has the added benefit of causing the compiler to complain if you later delete the statement, without deleting the entire block of code attached to it.

Preincrement vs. Postincrement

While both preincrement (++i) and postincrement (i++) do the same thing in isolation, I generally prefer to read the latter. The latter also happens to behave the way most people expect when refactoring the code (render the value, then increment), whereas the former tends to be a very special case (increment first, then render the value) which can cause errors and confusion during refactoring and maintenance. As such, I would recommend using preincrement operators only in the rare occasions they are actually needed (they're a flag that you need to slow down and read that chunk of code carefully).

Algorithm Behavior

The algorithm's output varies based on the order that the edges are presented to the class. Either the class needs to ensure that its internal edge lists are always in the appropriate order assumed/required by the algorithm, or the algorithm needs to be modified to handle edges in an arbitrary order (which the stack-based algorithm does).

The original question's test vector results in:

0
1,2,3,4
5,6,7
8

Whereas reordering the edges to be:

{4, 8}, {1, 2}, {8, 6}, {2, 5}, {3, 1}, {7, 5}, {4, 1}, {2, 4}, {4, 6}, {5, 6}, {6, 7}, {2, 3}

Results in an output of:

0
1,2,3,4
5,6,7,8

There may be additional deficiencies/broken cases surrounding this particular implementation. I ceased investigation upon finding this problem, but I would strongly suggest extensive verification against a known-working stack-based implementation after taking any corrective actions on your stackless algorithm.

Container Types

As a general rule of thumb, when returning a container, you should return the most-generic container interface type appropriate. It is almost never appropriate to return a concrete container, such as a List<T> or Dictionary<T>. In the case of public List<List<int>> GetStronglyConnectedComponents(), it should probably return ICollection<ICollection<int>> (since knowing how many groups and how many vertices are in each group makes sense enough to not use IEnumerable<T>, but all of the IList<T> functionality seems overkill).

Comments

Comments are far easier to read when they are above the line they are commenting, rather than to the right. Comments on the right of lines tend to contribute to overly-long lines, they tend to not be well-aligned (and come out of alignment upon refactoring), and tend to get lost/pushed off the side of the screen. All of this makes it much more difficult to do a comments-only skim of the code to get the gist of what it's supposed to be doing, prior to analyzing the code proper, or when quickly trying to find the part of the code you need to modify. The only benefit of comments on the right is that they take up less horizontal space -- but the hit to readability, and therefore usefulness, makes this placement unacceptable to me in most cases.

Some of the comments could also use improvement. "If we will ever find cycle" is confusing; it's either an incomplete thought, or placed on the wrong line. "Initialize to current time" is self-evident; it would be more useful to know why initializing to the current time is important (and possibly comment what currentTime is/does). "DFS" I assume means "depth-first search," and if so, should go into the recursive GetStronglyConnectedComponents call (right before the if statement that kicks off the actual depth-first searching), not the root call.

The "var currentDiscoveryTime[...]" comment is also dead code, and should be removed.

Structures

It would be nice from an aesthetic standpoint to, e.g., pass around a single array of a structure containing your working data, rather than three same-sized arrays.

Speed

If you're simply aiming to get to an \$O(1)\$ sidestep of the stack lookup, consider the easier alternative of keeping a secondary data structure (such as your List<bool>) that you can modify to mirror the stack, allowing you \$O(1)\$ insertion and lookup in \$2N\$ space. This would eliminate your need to get clever with edge orderings (which will take time to sort) or subtle algorithm alterations (translating the stack to effectively an array), while getting you the speedup you want. It's not as cool or elegant as the algorithmic gymnastics of doing away with the stack altogether, but it's much more likely to be maintainable, fast, and correct in the future. ;)


Producer / Consumer implementation for Parallel Processing in the TPL I've stripped this down to its bare bones removing stopping/cancellation logic etc... to keep it simple.

The Producer is a very simple class containing a timer. At regular intervals  the TimerOnElapsed will let the host now that it has another batch of items available. The onus is on the host to pull that next batch using GetNextBatch().

public class Producer
{
    public event EventHandler BatchAvailable;
    private readonly Timer timer;
    private int i;

    public Producer()
    {
        i = 1;
        timer = new Timer(5000);
        timer.Elapsed += TimerOnElapsed;
    }

    public void Start()
    {
        timer.Enabled = true;
        timer.Start();
    }

    private void TimerOnElapsed(object sender, ElapsedEventArgs e)
    {
        if (BatchAvailable != null)
            BatchAvailable(sender , e);
    }

    public IEnumerable<int> GetNextBatch()
    {
        var range = Enumerable.Range(i, i + 50).ToList();
        i = i + 50;
        return range;
    }
}

The Consumer class uses a Concurrent.BlockingCollection<T> to pass objects to a Parallel ForEach loop. The Consumer is READY for the next batch each time it empties the blocking collection of the existing batch.

public class Consumer
{
    private TaskFactory _factory;
    private readonly BlockingCollection<int> _entries;

    public Consumer()
    {
        _entries = new BlockingCollection<int>();
    }

    public void Start()
    {

        _factory = new TaskFactory();

        try
        {
            _factory.StartNew(() =>
            {
                Parallel.ForEach(
                    _entries.GetConsumingEnumerable(),
                    new ParallelOptions() { MaxDegreeOfParallelism = 5 },
                    ProcessEntry
                    );
            });
        }
        catch (OperationCanceledException oce) { }
    }

    public void Add(int entry)
    {
        _entries.Add(entry);
    }

    public bool Ready
    {
        get { return (_entries.Count == 0); }
    }

    private void ProcessEntry(int entry)
    {
        Console.WriteLine("Processing {0}", entry);
        Thread.Sleep(3000);
    }
}

The Host is a class containing each of the above. It orchestrates communication between the two. Each time the Producer says it has another batch available, the host checks to see if the Consumer is ready, and if so, retrieves the batch and passes it on.

public class Host
{
    private Producer _producer;
    private Consumer _consumer;

    public Host()
    {
        _producer = new Producer();
        _producer.BatchAvailable += (s,e) => ProducerOnBatchAvailable();
        _consumer = new Consumer();
    }

    public void Start()
    {
        _producer.Start();
        _consumer.Start();
    }

    private void ProducerOnBatchAvailable()
    {
        if (!_consumer.Ready) return;
        Console.WriteLine("Producer is ready for another Batch...");
        var batch = _producer.GetNextBatch().ToList();
        batch.ForEach(_consumer.Add);
    }
}

I've tested this a couple of times and it behaves as I'd expect. The throttling of batch sizes and the max parallelism in the P-ForEach loop also works. And in my larger example I have a number of other Players that hand entry along to the next step in the pipeline by way of more events and blocking collections.

However, I'm a bit irked by the fact that I never need to call the BlockCollection<T>.CompletedAdding(). Is this bad practice? Do I have any potential problems leaving the blocking collection in the WAIT state for long periods of time?

In the real example the producer will be querying a DB queue which could potentially have no work in it, so the BlockingCollection could sit there for hours overnight.

As a matter of being thread-safe, you should replace this:

    private void TimerOnElapsed(object sender, ElapsedEventArgs e)
    {
        if (BatchAvailable != null)
            BatchAvailable(sender , e);
    }

with this:

    private void TimerOnElapsed(object sender, ElapsedEventArgs e)
    {
        var batchAvailable = this.BatchAvailable;

        if (batchAvailable != null)
        {
            batchAvailable(sender, e);
        }
    }

Reason being, accessing the event field "raw", you may wind up with it going null between the if and the invocation itself.


Hibernate Save method practice We are new into hibernate, in our project for saving an entry the below given code is used, please have a look.

public void save(Object obj) {
    Session session = null;
    Transaction transaction = null;
    try {
        session = sessionFactory.getCurrentSession();
        transaction = session.beginTransaction();
        session.save(obj);
        session.flush();
        transaction.commit();
    } catch (JDBCException jde) {
        logger.fatal("Error occured in database communication", jde);
        transaction.rollback();
        throw new RuntimeException(jde);
    } finally {
        if (session.isOpen()) {
            session.close();
        }
    }
}

Is this the right way to do (Considering performance and Security) ?
Is this thread safe? as there may be number of users at the same time.

Bonus Question:is there any way to make this code better in Java 8 ?

ps:This method is written in our DatabaseUtil class.

Your finally should strive to be as safe as it can, since you may have already had some error occur. To that end, you could run into trouble with a null pointer:

} finally {
    if (session.isOpen()) {
        session.close();
    }
}

If session is null then you've just caused another error, so test session before you call on it:

} finally {
    if (session != null && session.isOpen()) {
        session.close();
    }
}