Recursive function to find the number of ways a number can be generated out of a set of numbers I had a job interview test and the question I got was about making a function which would return the number of ways a number could be generated by adding numbers from a certain set, where any number in the set can be used N times.

It is like if I have the number 10 and I want to find out how many ways 10 can be generated using [2,3,5]

2+2+2+2+2 = 10
5+3+2 = 10
2+2+3+3 = 10
5+5 = 10

to solve it I made this function:

function getNumberOfWays($money, $coins) {
    static $level = 0;
    if (!$level) {
        sort($coins);
    }

    if ($level && !$money) {
        return 1;
    } elseif (!$level && !$money) {
        return 0;
    }

    if ($money === 1 && array_search(1, $coins) !== false) {
        return 1;
    } elseif ($money === 1 && array_search(1, $coins) === false) {
        return 0;
    }

    $r = 0;
    $tmpCoins = $coins;
    foreach ($coins as $index => $coin) {
        if (!$coin || $coin > $money) {
            continue;
        }

        $tmpCoins[$index] = 0;
        $tmpMoney = $money;
        do {
            $tmpMoney -= $coin;
            if ($tmpMoney >= 0) {
                $level++;
                $r += getNumberOfWays($tmpMoney, $tmpCoins);
                $level--;
            } elseif (!$tmpMoney) {
                $r++;
            }
        } while ($tmpMoney >= 0);
    }

    return $r;
}

This function works correctly: it returns the right value.

Can I improve it in any way?

I'm assuming in this answer that you have infinite quantities of the designated numbers at your disposal, since that appears to be the way your code works. With that condition, this problem is the coin sums problem on Project Euler: given a set of coin denominations, how many ways can change be made for a number?

Looking at your code in terms of efficiency, the top-down recursive function is doing a lot of repeated work because of overlapping subproblems, similar to the recursive Fibonacci problem and is the hallmark of dynamic programming.

The biggest improvement available to you is using a memo or cache in the form of an associative array. Memoization makes it possible to keep track of what's already been computed for a given recursive call and avoid recomputations, pruning off huge chunks of the call tree. This effectively turns the exponential O(nk) problem into O(nk) [source] where k is the number of coins.

There's also a bottom-up iterative version which is an improvement still over the "recursion with cache" version. I recommend adding a cache to your algorithm as above, solving the Project Euler problem and spending some time perusing the solution thread that you'll gain access to which contains many imaginative and efficient solutions including these bottom-up versions.

In terms of coding style, there are variables and loops that can be removed. Moving "set up" code out of the recursive call and into a wrapper can save a few conditional checks and eliminate the level variable. Doing an array_search through your coins array and setting used elements to 0 is a bit awkward and can be handled easily by passing an index variable to determine which coins have been used already. The full function header I used has 4 parameters (including the index and a reference to the memo), which makes it nice to wrap.

Lastly, your base case checks need only happen for being out of money. When that happens, either you've made change successfully, or the change you tried to make wasn't valid (you went into negative numbers). No need to test anything else. All told, the whole program can be 30 lines or less.

I'd like to post my PHP code here, but in the spirit of Project Euler, you'll have the most fun if you solve it yourself instead of reading a spoiler! We can compare notes afterwards if you want (although the solutions you'll see in the forum will be far superior).


Runge-Kutta fourth order integration I have written a simple code for Runge-Kutta fourth order integration to solve a system of ordinary differential equations and parallelized it using OpenMP. I don't know if it is the best we can do for maximum performance of the code with little effort.

I need all values of to be returned, so I kept values in all steps.
I also create threads in each time step and paralleled in position, i.e.
pragma opm parallel is inside the loop over time.

Here is my try: (link to gitlab repository)

//RHS for a system of equations
void xprsys(const int n,const vector<double>& x, vector<double>& f)
{
    /*
     * n : number of equations
     * x : value in each time step
     * f : RHS of equtions dx/dt = f
     */
    double sum1=0;
    #pragma omp parallel for reduction(+:sum1)
    for (int i=0; i<n; i++){
        sum1 = 0;
        for(int j=0; j<n; j++)
            sum1 += sin(x[j]-x[i]);
        f[i] = M_PI + 2.0 * sum1;
    }
}

void SolveRK4(const int n, double h,vector<double> x,
     vector<vector<double>>& x_vec,
          int nstep, vector<double>& times)
{
    /*
     * times : vector contains the time 0 : t_final step dt
     * x_vec : [nstep by N] 2 Dimensional vector
     * dim1  : defined as typedef vector<double> dim1
     */

    times[0] = 0.0;
    dim1 y(n);
    dim1 f1(n),f2(n),f3(n),f4(n);
    double half_h = 0.5 * h;
    double h_sixth = h/6.0;
    // x_vec[nstepxN]

    for (int i=0; i<n; i++)
        x_vec[0][i] = x[i];

    for (int k=1; k<nstep; k++){
        times[k] = k*h;
        xprsys(n,x,f1);

        #pragma omp parallel for
        for(int i=0; i<n; i++)
            y[i] = x[i] + half_h * f1[i];

        #pragma omp master
        xprsys(n,y,f2);
        #pragma omp barrier

        #pragma omp for
        for(int i=0; i<n; i++)
            y[i] = x[i] + half_h * f2[i];

        #pragma omp master
        xprsys(n,y,f3);
        #pragma omp barrier

        #pragma omp for
        for(int i=0; i<n; i++)
            y[i] = x[i] + h * f3[i];

        #pragma omp master
        xprsys(n,y,f4);
        #pragma omp barrier

        #pragma omp for
        for(int j=0; j<n; j++) {
            x[j] = x[j] + h_sixth * (f1[j] + f4[j] + 2.0 * (f2[j] + f3[j]));
            x_vec[k][j] = x[j];
        }
    }
}

I would really suggest that you better structure your code. Runge Kutta methods are quite straightforward, so you should have a function that iterates a single step in parallel for every variable.

Furthermore, you can slightly deviate from the standard Runge Kutta scheme if you directly store \$x + h_if_i\$ in the \$i\$th component of that vector. You only have to adopt the summation at the end to account for that.

I did so here https://github.com/miscco/Neural_Network_TC/blob/master/Iterate_ODE.h

static void add_RK(std::vector<double>& var) {
    var[0] = (-3*var[0] + 2*var[1] + 4*var[2] + 2*var[3] + var[4])/6;
}

A minimal improvement that I would do is changing this

for (int i=0; i<n; i++)
    x_vec[0][i] = x[i];

To

x_vec[0] = x;

Where you utilize the underlying copy/move constructor, which might be more efficient depending on the compiler.


Window Factory and Manager using MVVM pattern I am fairly new to Factories and Dependency Injection in MVVM.

I've build my View(Window) factory with help from SO, and I would like to know if I had implemented everything properly(code works just fine).

I'm sorry for the length, but there was no other way for me to show my complete example.

First, there is the WindowFactory class. Its purpose is to delegate the creation of windows and coupling it with their ViewModels, so that I can avoid referencing View in its respective ViewModel (following MVVM pattern).

public class WindowFactory : IWindowFactory
{
    private readonly IWindowManager _windowManager;

    public WindowFactory(IWindowManager windowManager)
    {
        this._windowManager = windowManager;
    }

    public void CreatePhoneWindow()
    {
        var view = new PhoneWindow();
        var viewAdapter = new ViewAdapter(view, _windowManager);
        var viewModel = new PhoneWindowViewModel(viewAdapter);

        _windowManager.addWindow(view);
        view.DataContext = viewModel;
        view.Show();
    }

    // Here are copies of the above method, one for each different window that I create
}

WindowManager class. Its purpose is to keep track of current open windows, and allow certain operations on them (like checking if any window is open at a time, so that I can prevent other windows from being created)

class WindowManager : IWindowManager
{
    private ObservableCollection<Window> _windows;

    public WindowManager()
    {
        _windows = new ObservableCollection<Window>();
    }

    public void addWindow(Window window)
    {
        if (_windows.Contains(window) == false) { _windows.Add(window); }
    }

    public void removeWindow(Window window)
    {
        if (_windows.Contains(window) == true) { _windows.Remove(window); }
    }

    public bool isAnyWindowOpen()
    {
        if (_windows.Count == 0) { return true; }
        else { return false; }
    }
}

ViewAdapter class. Its purpose is to handle View close commands, that are comming from inside the View's ViewModel (again, to decouple).

class ViewAdapter : ICloseable
{
    private readonly Window _view;
    private readonly IWindowManager _windowManager;

    public ViewAdapter(Window view, IWindowManager windowManager)
    {
        this._view = view;
        this._windowManager = windowManager;
    }

    public void Close()
    {
        _windowManager.removeWindow(_view);
        _view.Close();
    }
}

Finally, I instantiate my WindowFactory and WindowManager in my MainWindow (Main, startup/default View) code-behind:

public MainWindow()
{
    InitializeComponent();

    var windowManager = new WindowManager();
    var windowFactory = new WindowFactory(windowManager);
    MenuItemsStackPanel.DataContext = new SideMenuViewModel(windowFactory, windowManager);
}

  I'm not including the Interfaces, since they contain simply the mocks of the used functions, nothing else.

I would really appreciate any feedback on my Factory design especially, since I might have made mistakes that I am unaware of, OR there could be a much better, cleaner way to achieve this.

Disclaimer: I haven't really used MVVM though am familiar with the pattern.

WindowFactory

The layers in MVVM, from upper to lower in terms of direction of dependencies (dependencies flow down), are View, ViewModel, and Model. Therefore it seems wrong to me that the ViewModel contains a View (by way of containing a ViewAdapter).

Another issue is that CreatePhoneWindow() actually displays the window. I would expect a factory to return a window, which the client can then display (or not) as and when desired. This isn't to say it is wrong, but it feels wrong (to me).

WindowManager

if (_windows.Contains(window) == false) is unnatural; use if (!_windows.Contains(window)) instead. Same for the comparison against true in the other method.

if (_windows.Count == 0) { return true; }
else { return false; }

This also can be simplified to return _windows.Count == 0;. (But is it correct that when the _windows collection is empty it means windows are open?)

Also why are you using an ObservableCollection? It appears you're not binding to it, being as it's a private field, so why not just use a standard List<Window> instead?

ViewAdapter

  Its purpose is to handle View close commands, that are coming from
  inside the View's ViewModel...

I don't understand why this class exists. If a ViewModel wants to say "time to close", it can raise an event. The View (Window) - which should have a dependency on the ViewModel - can subscribe to this event, and close itself. No need for an extra class to do its own closing. If you still want to have a WindowManager, then the WindowManager can subscribe to the Window's OnClose event and remove that object from its collection.


Genetic algorithm for Clever Algorithms project I wrote a bunch of Ruby code for a book project I've just finished. One criticism is that it is fine code but not very "ruby like". I agree my style was simplified for communication reasons, and although it's procedural code, it still feels "ruby-like" to me.

For the representative example below, any ideas on making it more "Ruby like"?

# Genetic Algorithm in the Ruby Programming Language

# The Clever Algorithms Project: http://www.CleverAlgorithms.com
# (c) Copyright 2010 Jason Brownlee. Some Rights Reserved.
# This work is licensed under a Creative Commons Attribution-Noncommercial-Share Alike 2.5 Australia License.

def onemax(bitstring)
  sum = 0
  bitstring.size.times {|i| sum+=1 if bitstring[i].chr=='1'}
  return sum
end

def random_bitstring(num_bits)
  return (0...num_bits).inject(""){|s,i| s<<((rand<0.5) ? "1" : "0")}
end

def binary_tournament(pop)
  i, j = rand(pop.size), rand(pop.size)
  j = rand(pop.size) while j==i
  return (pop[i][:fitness] > pop[j][:fitness]) ? pop[i] : pop[j]
end

def point_mutation(bitstring, rate=1.0/bitstring.size)
  child = ""
   bitstring.size.times do |i|
     bit = bitstring[i].chr
     child << ((rand()<rate) ? ((bit=='1') ? "0" : "1") : bit)
  end
  return child
end

def crossover(parent1, parent2, rate)
  return ""+parent1 if rand()>=rate
  point = 1 + rand(parent1.size-2)
  return parent1[0...point]+parent2[point...(parent1.size)]
end

def reproduce(selected, pop_size, p_cross, p_mutation)
  children = []
  selected.each_with_index do |p1, i|
    p2 = (i.modulo(2)==0) ? selected[i+1] : selected[i-1]
    p2 = selected[0] if i == selected.size-1
    child = {}
    child[:bitstring] = crossover(p1[:bitstring], p2[:bitstring], p_cross)
    child[:bitstring] = point_mutation(child[:bitstring], p_mutation)
    children << child
    break if children.size >= pop_size
  end
  return children
end

def search(max_gens, num_bits, pop_size, p_crossover, p_mutation)
  population = Array.new(pop_size) do |i|
    {:bitstring=>random_bitstring(num_bits)}
  end
  population.each{|c| c[:fitness] = onemax(c[:bitstring])}
  best = population.sort{|x,y| y[:fitness] <=> x[:fitness]}.first
  max_gens.times do |gen|
    selected = Array.new(pop_size){|i| binary_tournament(population)}
    children = reproduce(selected, pop_size, p_crossover, p_mutation)
    children.each{|c| c[:fitness] = onemax(c[:bitstring])}
    children.sort!{|x,y| y[:fitness] <=> x[:fitness]}
    best = children.first if children.first[:fitness] >= best[:fitness]
    population = children
    puts " > gen #{gen}, best: #{best[:fitness]}, #{best[:bitstring]}"
    break if best[:fitness] == num_bits
  end
  return best
end

if __FILE__ == $0
  # problem configuration
  num_bits = 64
  # algorithm configuration
  max_gens = 100
  pop_size = 100
  p_crossover = 0.98
  p_mutation = 1.0/num_bits
  # execute the algorithm
  best = search(max_gens, num_bits, pop_size, p_crossover, p_mutation)
  puts "done! Solution: f=#{best[:fitness]}, s=#{best[:bitstring]}"
end

First a note on your general design: You use hashmaps to represent members of the population. I'd rather recommend you create a little class to represent them:

class Individual
  include Comparable

  attr_reader :bitstring, :fitness

  def initialize(bitstring)
    @bit_string = bitstring
    @fitness = onemax(bitstring)
  end

  def <=>(other)
    fitness <=> other.fitness
  end
end

You'll now use Individual.new(bitstring) to create a new individual (which will automatically calculate its own fittness) and individual.bitstring and individual.fitness to get an individual's bitstring or fitness respectively.

This has the benefit that now the logic of how to calculate an individuals fitness lives in the Individual class instead of in multiple place in the search method. This is a more natural place for it and makes it easier to find and change should you ever decide to use a different fitness function.

Also note that I included Comparable, implementing <=> based on fitness. This means that you can now compare individuals to each other such that an individual is greater than another if and only if its fitness is greater. This allows you to use > on two individuals as well as methods like max to get the individual with the greatest fitness out of an array of individuals. It also allows you to call sort on array of individuals without passing a block. This will simplify your code in some places.

In a comment you mentioned that you don't want to use classes. However I still think that it would greatly improve your design to have the logic for creating new individuals and calculating their fitness in one single place, so you should at least factor it out into a method like this:

def create_individual(bitstring)
    {:bitstring => bitstring, :fitness => onemax(bitstring)}
end

Now some notes on particular pieces of code:

bitstring.size.times {|i| sum+=1 if bitstring[i].chr=='1'}

As has already been mentioned, this can easily be replaced with a call to count. However as a general note I want to point out that it's good practice to avoid index-based loops wherever possible, so if count did not exist, I'd write the above like this:

bitstring.each_char {|c| sum+=1 if c == '1'}

Or for 1.8.6 compability:

bitstring.each_byte {|b| sum+=1 if b.chr == '1'}

(0...num_bits).inject(""){|s,i| s<<((rand<0.5) ? "1" : "0")}

When dealing with inject, it's usually a good idea to keep the function simple, so it's more easily understandable. Often you can separate the logic of generating the elements from the logic of combining them, by using map before inject. This would make your code look like this:

(0...num_bits).map {|i| (rand<0.5) ? "1" : "0"}.inject("") {|s,i| s<<i}

Now there's two things to notice here:

Whenever you call map on a range from 0 to some size, you might rather want to use Array.new(size) {block} which creates an array of the given size by calling the block size times.
The smaller inject is now much easier to understand, but happens to be equivalent to calling join, so let's just do that.

So your code becomes:

Array.new(num_bits) { (rand<0.5) ? "1" : "0" }.join

This way you create a temporary array, which you didn't before, but on the other hand your code became much more understandable, so that little inefficiency should be worth it.

(pop[i][:fitness] > pop[j][:fitness]) ? pop[i] : pop[j]

Thanks to the Individual class this now looks like this:

(pop[i] > pop[j]) ? pop[i] : pop[j]

This isn't much different, but note that it now has the form (x > y) ? x : y, which incidentally means "the maximum of two objects". In other words we can simplify this code by using the max method:

[ pop[i], pop[j] ].max

bitstring.size.times do |i|
  bit = bitstring[i].chr

Again this should be bitstring.each_char do |bit| or bitstring.each_byte do |b| bit = b.chr.

""+parent1 if rand()>=rate

I'm not sure what the ""+ is supposed to do here. Remove it. Note that ""+x is not a way to turn x into a string in ruby (like it would be in e.g. Java). Calling ""+x when x is not a string (or "duck-string"), will cause a type error.

The fact that you're using ""+ to create a copy is less than obvious (thus the above paragraph). If you use the dup method, whose sole purpose it is to create a copy, instead, it will be much more understandable.

I'd also change the crossover method to return an individual rather than a string, which makes the code a bit simpler.


Boolean expression complexity and bitwise operators Given this kind of code:

public static final int DEFAULT_MASK = Mask.BASE
    | Mask.SECTOR
    | Mask.GROUP
    | Mask.INDUSTRY
    | Mask.NAME
    | Mask.COUNTRY;

Sonar raises a warning:

  Boolean expression complexity is 5 (max allowed is 3).

What would be a good way to deal with this?

I suppose the warning might go away if I rewrite like this:

public static final int DEFAULT_MASK;
static {
    int tmp = Mask.BASE;
    tmp |= Mask.SECTOR;
    tmp |= Mask.GROUP;
    tmp |= Mask.INDUSTRY;
    tmp |= Mask.NAME;
    tmp |= Mask.COUNTRY;
    DEFAULT_MASK = tmp;
}

... but this smells (too many mutations, ugly).

UPDATE

In addition to the default mask above, I have a few other masks as well, using different sets of bits, for different purposes. Every bitmask with more than 3 bits would raise a Sonar violation.

If it’s a complex bitwise operation, like:

result = a & b | c | d & x & y

then it would make perfect sense to raise a violation. On the other hand, this looks like an "innocent" bitmask:

mask = a | b | c | d

An automated tool cannot judge if this is dangerous or not, so it's understandable to raise a flag for this. But in this particular case, it's clearly a false positive.

I put // NOSONAR on the offending line of code to make the warning go away, like this:

public static final int DEFAULT_MASK = Mask.BASE  // NOSONAR
    | Mask.SECTOR
    | Mask.GROUP
    | Mask.INDUSTRY
    | Mask.NAME
    | Mask.COUNTRY;


Select first or last object from id I found this code in our project and it just feels like the wrong way to do what it seems to be doing

id<NSFastEnumeration> results = [info objectForKey: ZBarReaderControllerResults];
ZBarSymbol *symbol = nil;
for (symbol in results) {
    // this just selects the first symbol in the results
}

Presumably it actually doesn't select the first symbol - instead it selects the last one!

But there must be a better way. I don't quite know what

[info objectForKey: ZBarReaderControllerResults]

returns, but I'm not too bothered either. Is there a way that I can do the above code without having to loop through all possible results?

ZBarSDK is the only place I've ever seen NSFastEnumeration.  I don't know a whole lot about it, nor do I know exactly why ZBarSDK was designed to be used in this way.

NSFastEnumeration isn't any faster than using a forin loop.  It is faster than a regular for or while (or do-while) loop, but not faster than a forin.

In a forin loop, we're using regular Objective-C collections, and NSArray has a firstObject and lastObject.

But if the real question is (and should be) how do I improve this code (more than just specifically grabbing a single object out of an NSFastEnumeration), then the answer is to use AVCaptureMetadataOutput (official documentation), which was introduced in iOS7.

ZBarSDK has an iOS7 memory leak.  Also, I don't know about compiled on-device size, but when I removed it from my project, it saved about 1.4mb from the project size.

Meanwhile, AVCaptureMetadataOutput has many advantages over ZBarSDK.

It's slightly easier to use... and it's simple to use if you're already familiar with video capture in iOS.
It leaves a smaller footprint in terms of storage space your app takes up on a device.
Even without ZBarSDK's memory leak, AVCaptureMetadataOutput has a smaller memory footprint.
AVCaptureMetadataOutput seems to scan barcodes faster and a farther distances than what I could manage with ZBarSDK.
ZBarSDK doesn't support 64-bit processors at all.  AVCaptureMetadataOutput however does.


Sending test emails to a list of email addresses I've made the following program that takes a list of email addresses from a table in MS Access and sends each a test email.

'Send email to mailing list
Function SendNotificationEmail()

 'Handle any errrors
 On Error GoTo ErrHandler:

    'Create Outlook object
    Dim olApp As Outlook.Application

    'Namespace
    Dim olNS As Outlook.NameSpace
    Dim olFolder As Outlook.MAPIFolder

    'Create a reference to the email item you will use to send the email
    Dim olMailItem As Outlook.MailItem

    Set olApp = CreateObject("Outlook.Application")
    Set olNS = olApp.GetNamespace("MAPI")

    Set olFolder = olNS.GetDefaultFolder(olFolderInbox)
    Set olMailItem = olFolder.Items.Add("IPM.Note")

    'Create the body of the message
    Dim strBodyText As String
    strBodyText = "Test Email - DELETE ME :D"

    'Open record set on
        Dim dbC As DAO.Database
        Dim rcdSet As DAO.Recordset
        Dim fld As DAO.field

        'Used to update the table
        Dim strSQL As String
        strSQL = "SELECT DISTINCT [Email Address] FROM tblMailingList"

        'Connect to current database
        Set dbC = CurrentDb
        Set rcdSet = dbC.OpenRecordset(strSQL)

        'Count the number of records
        Dim intNumRecords As Integer
        'Move to the last record in order to count all the records
        rcdSet.MoveLast

        intNumRecords = rcdSet.RecordCount - 1
        'Move back
        rcdSet.MoveFirst

        'Get data back from field birth date
        Set fld = rcdSet.Fields("Email Address")

        'Subject
        olMailItem.Subject = "Mailing List Test"

        'Loop through the records
        For i = 0 To intNumRecords
            'Recipient/s
            olMailItem.To = fld.Value

            'Body of email
            olMailItem.Body = strBodyText

            'Automatically send the email
            olMailItem.Send

           'Reset email item otherwise it won't work
            Set olMailItem = olFolder.Items.Add("IPM.Note")

            'Move to the next record
            rcdSet.MoveNext
        Next

    'Close record set and connection
    rcdSet.Close

    'Empty variables
    Set fld = Nothing
    Set rcdSet = Nothing
    Set dbC = Nothing

    'Relase all the object variables
    Set olMailItem = Nothing
    Set olFolder = Nothing
    Set olNS = Nothing
    Set olApp = Nothing

ErrHandler:

'If error occurs
If Err.Number <> 0 Then
  MsgBox "Error Number: " + Err.Number + ": Description: " + Err.Description
End If

End Function

Is my approach above correct? I had to reset the olMailItem in the loop otherwise it would return a Type Error. Is there a better approach to sending multiple emails?

Set olMailItem = Nothing
Set olFolder = Nothing
Set olNS = Nothing
Set olApp = Nothing

is unnecessary. VBA is garbage collected, there's no need for this, or any of the other = Nothing's you have. Here's an article from one of the MS developers about this practice. It refers to VBScript, but VBA's garbage collection algorithm is similar, and probably substantially better than VBScript's.
Around
'Open record set on
        Dim dbC As DAO.Database

You start a new level of indentation. Why? I thought when I first read this that it was a part of the loop below.
Finally, you might want to think more about your error handling (although VB's error handling isn't very fun to work with). For example, if you get an exception on olMailItem.Send, you might not care -- you want the script to keep going and try to send the next email, in case that was a fluke. However, if something goes wrong on Set rcdSet = dbC.OpenRecordset(strSQL), you probably want the script to quit entirely, since nothing afterwards is going to be useful if you don' have anything to work on.


Java queue with dynamic priority flag I need to build a queue where the elements will be added and removed in chronological order by default. But if the client sets the priority flag for the queue I need to be able to pull the elements based on the priority order of the elements.

I currently have this implemented using two priority queues, however I do not like the approach as it seems like an overkill.

Please let me know if there is a better way of doing this or if there is an existing implementation that exists.

import javax.naming.OperationNotSupportedException;
import java.util.Comparator;
import java.util.PriorityQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;

public class DynamicPriorityQueue<ComparableQueueElement> implements IQueue<ComparableQueueElement> {

    private static final int CONSTANT_HUNDRED = 100;
    private boolean fetchByCustomPriority = false;
    private final ReentrantLock lock;
    private final PriorityQueue<ComparableQueueElement> queue;
    private final PriorityQueue<ComparableQueueElement> customPriorityQueue;

    public DynamicPriorityQueue() {
        this(null);
    }

    public DynamicPriorityQueue(Comparator<ComparableQueueElement> comparator) {
        this.lock = new ReentrantLock();
        this.queue = new PriorityQueue<>(CONSTANT_HUNDRED);
        if (comparator != null)
            this.customPriorityQueue = new PriorityQueue<ComparableQueueElement>(CONSTANT_HUNDRED, comparator);
        else
            this.customPriorityQueue = null;
    }

    public void setFetchByCustomPriority(boolean fetchByCustomPriority) throws OperationNotSupportedException {
        if (this.customPriorityQueue == null)
            throw new OperationNotSupportedException("Object was created without a custom comparator.");

        this.fetchByCustomPriority = fetchByCustomPriority;
    }

    public void push(ComparableQueueElement t) throws InterruptedException {
        if (this.lock.tryLock(CONSTANT_HUNDRED, TimeUnit.MILLISECONDS)) {
            try {
                this.queue.offer(t);
                if (this.customPriorityQueue != null)
                    this.customPriorityQueue.offer(t);
            } finally {
                this.lock.unlock();
            }
        }
    }

    public ComparableQueueElement peek() {
        return this.fetchByCustomPriority ? this.queue.peek()
                : (this.customPriorityQueue != null ? this.customPriorityQueue.peek() : null);
    }

    public ComparableQueueElement pop() throws InterruptedException {
        ComparableQueueElement returnElement = null;
        if (this.lock.tryLock(CONSTANT_HUNDRED, TimeUnit.MILLISECONDS)) {
            try {
                if (this.fetchByCustomPriority && this.customPriorityQueue != null) {
                    returnElement = this.customPriorityQueue.poll();
                    this.queue.remove(returnElement);
                }
                else {
                    returnElement = this.queue.poll();
                    if (this.customPriorityQueue != null) {
                        this.customPriorityQueue.remove(returnElement);
                    }
                }
            } finally {
                this.lock.unlock();
            }
        }
        return returnElement;
    }
}

Constants naming

CONSTANT_HUNDRED is a bad name. It tells us its value, but not its role. Its value could be changed, making the whole thing useless, and in addition it is used in completely different contexts (queue size, and time out delay) so a user wishing to change one, might unknowingly alter the other.

You should declare two distinct constants:

private static final int QUEUE_DEFAULT_SIZE = 100;
private static final int LOCK_TIMEOUT_MILLIS = 100;

Usage:

    this.queue = new PriorityQueue<>(QUEUE_DEFAULT_SIZE);
    ...
    if (this.lock.tryLock(LOCK_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)) {

Dubious Exception

When setting fetchByCustomPriority to false on a Queue which does not support custom priority, there will be an exception. There shouldn't be, because the user isn't asking for the unavailable functionality

Timing out breaks Queue contract

When timing out on pop(), you would return null. according to the Queue contract this null value means, "there are no Objects in the Queue now", but that may be wrong. It might just be that too many Threads are waiting to get their turn and this one got timed out.

You should Throw a custom TimedoutException to differentate between timing out, and having an empty queue.

Same goes for push().

Wrong usage of ternary operator ?

In peek:

return this.fetchByCustomPriority ? this.queue.peek() : (this.customPriorityQueue != null ? this.customPriorityQueue.peek() : null)

if the condition is true, the first member is returned, otherwise the second is returned. I suspect if fetchByCustomPriority is true, you'll want to use the customPriorityQueue, so you need to invert arguments 2 and 3.

Uneven popping

In push, you push messages on both queues.

In pop(), you either pop the queue, or the customPriorityQueue. This means both queues act differently depending on if fetchByCustomPriority is true or false. This makes your object inconsistent and breaks Queue interface contract:

ComparableQueueElement comparableQueue = new ComparableQueueElement(aComparator);
comparableQueue.push(element);
comparableQueue.setFetchByCustomPriority(true);
comparableQueue.pop(); // returns element
comparableQueue.setFetchByCustomPriority(false);
comparableQueue.pop(); // returns element AGAIN!!

Your objects should stay consistent.

You could fix this by popping both queues in pop() method.

Or you could design your functionality in a way that it cannot fail...

Infallible DynamicPriorityQueue

Your fields are too rich for your object. The two queues could become out of sync. The best way to fix this is to have your objects only allow states which make sense.

Since you're describing an Object wich is a queue, let the only field by a queue.
I've changed to class to make it a bit more dynamic even, in that the comparator is no longer a fixed one. So now your can setFetchByCustomPriority(null) to fetch in-order, or setFetchByCustomPriority(comparatorA) and switch to setFetchByCustomPriority(comparatorB) on the fly.

import javax.naming.OperationNotSupportedException;
import java.util.Comparator;
import java.util.PriorityQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;

public class DynamicPriorityQueue<ComparableQueueElement> implements IQueue<ComparableQueueElement> {
    private static final int CONSTANT_HUNDRED = 100;
    private final ReentrantLock lock;
    private PriorityQueue<ComparableQueueElement> queue;

    public DynamicPriorityQueue() {
        this(null);
    }

    public DynamicPriorityQueue(Comparator<ComparableQueueElement> comparator) {
        this.lock = new ReentrantLock();
        if (comparator != null) {
            this.queue = new PriorityQueue<ComparableQueueElement>(CONSTANT_HUNDRED, comparator);
        } else {
            this.queue = ew PriorityQueue<ComparableQueueElement>(CONSTANT_HUNDRED);
        }
    }

    public void setFetchByCustomPriority(Comparator<ComparableQueueElement> comparator) {
        // Just replace the existing queue
        PriorityQueue<ComparableQueueElement> newQueue = comparator == null ? new PriorityQueue(queue.size()) : new PriorityQueue(queue.size(), comparator);
        ComparableQueueElement element = queue.poll();
        while(element != null){
            newQueue.push(element);
            element = queue.poll();
        }
        queue = newQueue;
    }

    public void push(ComparableQueueElement t) throws InterruptedException, TimedOutException {
        if (this.lock.tryLock(CONSTANT_HUNDRED, TimeUnit.MILLISECONDS)) {
            try {
                this.queue.offer(t);
            } finally {
                this.lock.unlock();
            }
        } else {
            throw new TimedOutException();
        }
    }

    public ComparableQueueElement peek() {
        return this.queue.peek();
    }

    public ComparableQueueElement pop() throws InterruptedException, TimedOutException {
        ComparableQueueElement returnElement = null;
        if (this.lock.tryLock(CONSTANT_HUNDRED, TimeUnit.MILLISECONDS)) {
            try {
                return this.queue.pop();
            } finally {
                this.lock.unlock();
            }
        }
        throw new TimedOutException();
    }
}


Batch processing large text files for sentiment analysis I have written this script in Python 2.7 and I was wondering where I could improve it or efficient. This is my first script so I don't now much. The script takes a text file from a folder removes blank spaces with list comprehension and then breaks it into smaller 20 elements and saves it each 20 elements as a definition into a dictionary with iterative numbered keys. Each key is then used to fetch the definition from the dictionary and sends each through the API one by one saving the output into another dictionary. The result dictionary is then converted to text and saved into a text file.

import glob
import os
import json
import paralleldots

api_key = "iyuDsKbgZSsCxVj6Moe37MzdqCUuvkpf33t6qS3X3cH"
paralleldots.set_api_key(api_key)

output = input(r"C:\Users\User\Desktop\trial")
txt_files = os.path.join("D:\\english\\stou\\check", '*.txt')

dic = {}                                #dictionary for storing list elements
res = {}                                #results dictionary for api output

def for_high (list_e):                  #defining a function for taking list elements and putting them into dic
    no_keys = list_e / 20
    dic = {
        1: l[1:21]
    }
    i = 1
    start_key_value = 21
    while i <= no_keys:
        i += 1
        end_key_value = start_key_value + 20
        dic.update({i: ''.join(l[start_key_value:end_key_value])})
        start_key_value += 20
    for x in dic:                       #creating a for loop for getting and saving the output and creating a new file
        output = paralleldots.emotion(dic.get(x))
        res.update({x: output})
        with open(os.path.join("C:\\Users\\User\\Desktop\\trial", filename), 'w') as text_file:
            text_file.write(json.dumps(res))

for txt_file in glob.glob(txt_files):     # for loop for going through all the text files in the input directory
    with open(txt_file, "r") as input_file:
        filename = os.path.splitext(os.path.basename(txt_file))[0] + '.txt'
        l = [l for l in input_file.readlines() if l.strip()]
        list_e = int(len(l))            #no. of list elements variable

    if list_e > 20:                     #checking if list has more than 20 elements
        if list_e % 2 != 0:             #checking if list has an odd no. of elements
            list_e += 1
            for_high(list_e)

        else:
            for_high(list_e)
    else:
        in_txt = paralleldots.emotion(l)
        filename = os.path.splitext(os.path.basename(txt_file))[0] + '.txt'
        with open(os.path.join("C:\\Users\\User\\Desktop\\trial", filename), 'w') as text_file:
            text_file.write(str(in_txt))

I understand you are a beginner and you had the guts to post your code. You deserve some feedback and I will try.

Your code is hard to read

This is most probably why you didn't get answers till now. There is code repetition, global and reused variables, no clear control flow, and bad variable names. It is really hard to find the core functionality.
So I start in no particular order.

Bad names

dic = {}                                #dictionary for storing list elements

Well I can see this is a dict. Bad name, bad comment. Use names describing the meaning, not the type. Comments shoud describe the hidden, not so obvious parts, that could not be expressed by names. Your whole comment #dictionary for storing list elements can be split into  #dictionary for storing ... elements and ... lists ....
the first one is completely obvious, the other is just some type information again. No hints to the sort of content (should be in the name), no hints as to the key.
You could safely replace the whole line with guesswhat={}.

Do not use globals or nonlocals for no reason

def for_high (list_e):                  #defining a function for taking list elements and putting them into dic

Uses filename, res, and l. While one could expect that on res and as it was declared at global scope, it is a complete no-go to use variables that look like temporaries in their scope. This renders your function absolutely untestable and your function will easily break if you alter the outer code. Pass required variables, so the reader/maintainer is aware they are used by the function.
Unlike what the comment suggests, your function currently only takes a list length. And unless for_high is a well known / defined term in your community, it is a bad name again.

Do not repeat yourself (DNRY)

Your function has some code:

output = paralleldots.emotion(dic.get(x))
res.update({x: output})
with open(os.path.join("C:\\Users\\User\\Desktop\\trial", filename), 'w') as text_file:
    text_file.write(json.dumps(res))

which is duplicated outside (slightly modified)

in_txt = paralleldots.emotion(l)
filename = os.path.splitext(os.path.basename(txt_file))[0] + '.txt'
with open(os.path.join("C:\\Users\\User\\Desktop\\trial", filename), 'w') as text_file:
    text_file.write(str(in_txt))

How can something be either output or in_txt? Of course a function has a return value or output or result. None of these make a good name. What is returned?
For one code snippet filename is explicitly set, for the other it magically pre-exists? So the function accesses a value from outside and the outer code overwrites the existing value with the very same value before using it.
This is pure horror. Also one of this file writes is done in a loop, the file is overwritten in each iteration. It shall either be written once or appended.
Create a function handling one element.

Simplify control flow

If both execution paths of an if clause nearly do the same, check whether you can simplify it. Your

if list_e % 2 != 0:             #checking if list has an odd no. of elements
    list_e += 1
    for_high(list_e)

else:
    for_high(list_e)

can be simplified to (again add a useful comment):

if list_e % 2 != 0:             # list length may not be odd because ...
    list_e += 1
for_high(list_e)

but: are you sure it is a good idea to increase a list-length variable while the list itself stays odd?

Define numeric constants once

you have

no_keys = list_e / 20
dic = {1: l[1:21]}
start_key_value = 21
end_key_value = start_key_value + 20
start_key_value += 20
if list_e > 20:                     #checking if list has more than 20 elements

I do not think these are independent constants. So define this constant once and use that name throughout the code (again change the comment to have some meaning)

max_emoticon_len = 20
no_keys = list_e / max_emoticon_len
dic = {1: l[1:max_emoticon_len + 1]}
start_key_value = max_emoticon_len + 1
end_key_value = start_key_value + max_emoticon_len
start_key_value += max_emoticon_len
if list_e > max_emoticon_len:       #checking if list has more than elements than desired to fit for ...

Other

Also list_e is a bad name. e mostly is used as name/pre-/postfix for elements for e in l:. for no. of elements the term length as in len() is used. stick to those conventions to avoid irritation.

Also dic = {1: l[1:21]} - are you sure you want to skip the first element? In Python, indexing starts at 0.

Also is it desired, that when having a list of length 40, the first sequence has length 20, and the last one has length 19?

Also dic = {1: l[1:21]} hides the outer dic which therefor is never used and may be deleted

Also dic = {1: l[1:21]} - why do you set this explicitly? Why not inside the loop?

Also dic.update({i: 'some'}) may be done by dic[i] = 'some'. The same goes for res.

Also you handle lists with less than 20 elements in your main code. However, if the list is bigger than 20 elements, you do batches of 20 and drop the last ones. Desired?


Find all words that can create a pyramid I wanted to write something that would find all pyramid words within a supplied text file or list of strings. An example of a pyramid word is the word "deadheaded". It has one of the letter "h", two of "a", three of "e", and four of "d".
    This would form a pyramid if you were to stack the letters:

   h
  a a
 e e e
d d d d

I'm looking for input regarding various things, such as design (is this constructed well for a user and easy to understand?), extensibility, using the right data structures, dealing with input/output...

Would this be better designed with more OOP in it? For example, I could have a base class with sub classes finding words with different shapes than a pyramid. But I'm not sure how much it would help in this design, as it seems fairly script-like with all the string manipulation.

public class PyramidWordsFinder
{
    public PyramidWordsFinder()
    {
        this.pyramidWordList = new List<string>();
    }

    public IList<string> pyramidWordList { get; set; }

    public IReadOnlyList<string> LoadDictionaryFile(string filePath)
    {
        List<string> wordsDictionary = new List<string>();
        Encoding enc = Encoding.GetEncoding(1250);
        string[] lines = File.ReadAllLines(filePath, enc);
        wordsDictionary.AddRange(lines.Where(x => x.Length > 2));
        return wordsDictionary.AsReadOnly();
    }

    public void WriteWordListToFile(IEnumerable<string> words, string fullFileName)
    {
        if (String.IsNullOrWhiteSpace(fullFileName))
        {
            throw new ArgumentException("The file name is not correctly formed");
        }

        File.WriteAllLines(fullFileName, words);
    }

    public IList<string> FindPyramidWords(IReadOnlyList<string> wordList)
    {
        foreach (var word in wordList)
        {
            // Quick checks before seeing if pyramid word
            if (this.WordIsCorrectLength(word) && (word.ToLower().Distinct().Count() == this.GetMaximumLetterFrequency(word)))
                {
                if (this.DetermineIfWordIsPyramid(word))
                {
                    this.pyramidWordList.Add(word);
                }
            }
        }
        return this.pyramidWordList;
    }

    private int GetMaximumLetterFrequency(string word)
    {
        int total = 0;
        for (int i = 1; i <= word.Length; i++)
        {
            total += i;
            if (total == word.Length) return i;
        }

        return 0;
    }

    private bool WordIsCorrectLength(string word)
    {
        var correctLengths = new List<int> {6, 10, 15, 21, 28, 36, 45, 55 };
        if (correctLengths.Any(item=> item == word.Length))
        {
            return true;
        }

        return false;
    }

    /// <summary>
    /// Determines if the given word is a pyramid word
    /// </summary>
    /// <param name="word"></param>
    /// <returns></returns>
    public bool DetermineIfWordIsPyramid(string word)
    {
        var lowerCaseWord = word.ToLower();
        List<int> letterCounts = new List<int>();
        var distinctLetters = lowerCaseWord.ToLower().Distinct();

        // Adds up how many times each distinct letter is seen in the word
        foreach (var letter in distinctLetters)
        {
            var numLetters = lowerCaseWord.Count(x => x == letter);
            letterCounts.Add(numLetters);
        }

        // determines if the frequency the distinct letters descends consecutively
        var maximumLetterFrequency = this.GetMaximumLetterFrequency(lowerCaseWord);
        for (var i = maximumLetterFrequency; i >= 1; i--)
        {
            if (!letterCounts.Contains(i)) return false;
        }

        return true;
    }
}

SRP

You should start by splitting this class into smaller pieces to satisfy the Single Responsibility Priciple. One class should do only one thing and yours is doing at least two:

it reads and writes files
it finds the pyramid words

This means you should:

Create one module for reading and writing words into a file
Let the second module only find pyramid words

The first module could be the DictionaryFile

class DictionaryFile
{
    public IReadOnlyList<string> LoadDictionary(string filePath)
    {
        // ...
    }

    public void SaveDictionary(IEnumerable<string> words, string fileName)
    {
        // ...
    }
}

Keep the names clear but not too long if this is not necessary.

The second module is the PyramidWordFinder:

class PyramidWordFinder
{
    private readonly HashSet<int> _lengths = new HashSet<int>();

    public PyramidWordFinder(int maxLength)
    {
        var length = 3;
        var i = 1;
        while (length <= maxLength)
        {
            _lengths.Add(length);
            length = ++i + length + 1;
        }
    }

    public bool IsPyramidWord(string value)
    {
        if (!_lengths.Contains(value.Length)) { return false; }

        var groups = value
            .GroupBy(x => x.ToString(), StringComparer.OrdinalIgnoreCase)
            .Select(x => x.Count())
            .OrderBy(x => x);
        return groups.SequenceEqual(Enumerable.Range(1, groups.Length));
    }
}

HashSet

Instead of having a list with predefined lengths, you can calculate them based on the max length and save them in a HashSet<int> for faster lookup which is O(1) in comparison to the list where it's O(n).

LINQ

To determine whether a word is a pyramid you can use LINQ where you first group each letter and sort it in the ascending order. Then you check if the count is increasing by one:

var groups = value
    .GroupBy(x => x.ToString(), StringComparer.OrdinalIgnoreCase)
    .Select(x => x.Count())
    .OrderBy(x => x);
return groups.SequenceEqual(Enumerable.Range(1, groups.Length));

StringComparer

With the StringComparer.OrdinalIgnoreCase you don't need to call ToLower or ToUpper.

IEnumerable

public IList<string> FindPyramidWords(IReadOnlyList<string> wordList)

Don't return a complete list of words if this isn't necessary. Use an iterator instead - by using LINQ or the yield return. What if you wanted just the first three words? You would need to calculate them all anyway. With deferred execution you can stop whereever you want to.

Example

You can now easily find all pyramid words with linq:

var pwf = new PyramidWordFinder(maxLength: 55); // or use constant
var words = new string[] { /* words */ }; // from the file
var pyramidWords = words.Where(pwf.IsPyramidWord).ToList();

I'm aware of the fact that the LINQ solution might not be the fastest one but it's definitely the esiest one. If you don't notice any performance hit then why make it so complex? Premature optimization is the root of all evil.

The point is that you should separate each feature so that you can work on it and test it without affecting the others. If you can focus on one thing at a time you can better optimize it.


Beginner Python User Login System I started Python 3 days ago. This user login code is the first project I completed, not following along with a tutorial. The idea is that you can create a user and login with it. I am wondering if there are ways to make this code much more efficient or shorter. If it needs anything, I'd love to learn new things.
def choices():
    print("Please choose what you would like to do.")
    choice = int(input("For Signing Up Type 1 and For Signing In Type 2: "))
    if choice == 1:
        print("Please Provide")
        username = str(input("Username: "))
        f = open("usernames.txt", 'r')
        info = f.read()
        if username in info:
            print("Username unavailable. Please Try Again.")
            return print(choices())
        f.close()
        password = str(input("Password: "))
        f = open("passwords.txt", 'r')
        info1 = f.read()
        f.close()
        f = open("usernames.txt", 'w')
        info = info + " " + username
        f.write(info)
        f.close()
        f = open("passwords.txt", 'w')
        info1 = info1 + " " + password
        f.write(info1)
        f.close()
        print("Congratulations! You have successfully created an account. You may now login in.")
        return print(choices())
    elif choice == 2:
        print("Please Provide")
        username = str(input("Username: "))
        f = open("usernames.txt", 'r')
        info = f.read()
        if username in info:
            password = str(input("Password: "))
            f = open("passwords.txt", 'r')
            info1 = f.read()
            if password in info1:
                print("Welcome Back " + username + "!")
            else:
                print("Your password is incorrect. Please try again.")
                return print(choices())
        else:
            print("Your username is either incorrect or nonexistent. You can either try again or create a new account.")
            return print(choices())
    else:
        raise TypeError

print(choices())

Use with to handle file operations
A common way to handle files is by creating a file instance with open statement and closing it when you are done with the handler, while this would work, it is prone to error. What if you forget to close the file? It leads to either the file not saving or the handler might be corrupted with garbage. with automatically closes the file once operations on it are done. A simple example is as follow
with open('usernames.txt', 'r') as f:
    f.read()

Recursion is expensive
Your choices function uses recursion which might become expensive. Recursion creates temporaries in the stack frame and as it recur more and more, your stack frame might eventually be filled, though it can be elegant to solve a problem with recursion, limit it usage. Any function that can be written recursively can also be written with an iterative approach. Iterative approach tends to be faster and does not create temporaries
Consider reducing the complexity of choices
Right now choice does way too much, signs up a user and also login a user. Your functions should do just one thing and do it very well. The name choices is very misleading, from the name, I would expect it to make a choice or rather I make a choice among a list of values, rather is suprisingly signs up and login a user. Avoid function side-effect. Function side-effect is when a function promises one thing and does another.
Use a dictionary pair
I really think a dictionary is needed in this scenario. Dictionaries as we all know are very fast. Storing a key-value pair of username and password makes the code clean and easy to reason about.
Detour

You might want to use open('filename.txt', 'a') when you want to append to a file, using 'w' overwrites the content of the file and is similar to calling truncate on the file.

When you want to recieve string input, no need for this statement str(input("Enter")), input automatically recieves strings.

Avoid variable names suchh as f

This is an editted version of the code
def signup():
    with open("usernames.txt", 'r') as user:
        usernames = user.read().splitlines()
    with open("passwords.txt", 'r') as pwd:
        passwords = pwd.read().splitlines()

    info = dict(zip(usernames, passwords))

    print("Please Provide")
    username = input("Username: ")
    while username in info.keys():
        print("Username unavailable. Please Try Again.")
        username = input("Username: ")
    password = input("Password: ")

    with open("usernames.txt", 'a') as user:
        user.write(username + '\n')
    with open("passwords.txt", 'a') as pwd:
        pwd.write(password + '\n')

    print("Congratulations! You have successfully created an account. You may now login in.")

def login():
    with open("usernames.txt", 'r') as user:
        usernames = user.read().splitlines()
    with open("passwords.txt", 'r') as pwd:
        passwords = pwd.read().splitlines()

    info = dict(zip(usernames, passwords))
    username = input("Username: "))
    password = input("Password: "))

    if info.get(username, None) is None:
        print("Your username is either incorrect or nonexistent. You can either try again or create a new account.")
        signup()
        return

    if password != info[username]:
        print("Your password is incorrect. Please try again.")
        return

    print("Welcome Back " + username + "!")

if __name__ == '__main__':
    print("Please choose what you would like to do.")
    choice = int(input("For Signing Up Type 1 and For Signing In Type 2: "))
    if choice == 1:
        signup()
    elif choice == 2:
        login()


Automate booking on a travel site Given below is my task

  Write a selenium test that makes a booking from Bangalore to New Delhi
  for today's date with a return of tomorrow using makemytrip.com.
  Select the cheapest flight and make sure that you come to the booking page.

I'm pretty new to Selenium and I would like to know if my attempt at achieving the aforementioned task is as good as it can get or if there are certain aspects of my code that could be improved.

I've relied on some simple element selectors to achieve the task and one concern I have with automating a test case (such as the one mentioned above) for a web site is to do it in such a way that it remains change proof i.e. a test case written in such a way that it validates certain functions of the site without relying on the structure of the web page itself especially when the "id" attribute isn't available. Are there any general recommendations/practices as far as automation of web sites is concerned ?

from selenium import webdriver,common
from selenium.webdriver.common.keys import Keys
from datetime import datetime
import time,re

def type_and_enter(element,text):
    element.clear()
    element.send_keys(text)
    time.sleep(1)
    element.send_keys(Keys.ENTER)

source = "Bangalore"
destination = "New Delhi"

# CREATE A NEW GOOGLE CHROME OBJECT & LOGIN TO makemytrip.com
chrome_browser = webdriver.Chrome()
chrome_browser.get("https://www.makemytrip.com/")

# ASSERT WE ARE ON THE CORRECT PAGE
assert "makemytrip" in chrome_browser.title.lower()

# ENTER VALUE FOR "FROM"
from_field = chrome_browser.find_element_by_id("hp-widget__sfrom")
type_and_enter(from_field,source)

# ENTER VALUE FOR "TO"
to_field = chrome_browser.find_element_by_id("hp-widget__sTo")
type_and_enter(to_field,destination)

# SET DEPART DATE TO PRESENT DAY
depart_field = chrome_browser.find_element_by_id("hp-widget__depart")
depart_field.click()

I think the biggest thing you have to take into account is Separation of Concerns. You should definitely separate page and element definitions from the actual logic of the test. Put your element locators into Page Objects and reference page object attributes in your tests. You can, for instance, have a Search page, a Search Results page, a Booking page on a high level which, in turn, may consist of inner page objects defining blocks/parts of a page.

Another way to abstract things away is Behavior Driven Development which would, at least, make you think of the actions and conditions on the target site pages and describe them in a natural language. There are a few Python packages for BDD, in particular - behave which can be, of course, used with selenium: Using the behave Framework for Selenium BDD Testing: A Tutorial


Database schema for a school Here is the database query needed to create the database:

create table Area
(
AreaId int primary key identity(1,1),
Name nvarchar(64) not null
)

create table Level
(
LevelId int primary key identity(1,1),
Name nvarchar(32) not null,
Principle nvarchar(512) not null
)

create table Subject
(
SubjectId int primary key identity(1,1),
AreaId int foreign key references Area(AreaId),
LevelId int foreign key references Level(LevelId),
Name nvarchar(32) not null,
Abbreviation nvarchar(16)
)

create table StaffType
(
StaffTypeId int primary key identity(1,1),
Name nvarchar(64) not null
)

create table Staff
(
StaffId int primary key identity(1,1),
StaffTypeId int foreign key references StaffType(StaffTypeId),
Name nvarchar(128) not null,
LastNameFather nvarchar(256) not null,
LastNameMother nvarchar(256) not null,
DateOfBirth datetime,
PlaceOfBirth nvarchar(256),
Sex nvarchar(8) not null,
Carnet nvarchar(64),
Telephone nvarchar(64),
MobilePhone nvarchar(64),
Address nvarchar(256),
FatherName nvarchar(256),
MotherName nvarchar(256),
FatherContactNumber nvarchar(64),
MotherContactNumber nvarchar(64),
FatherPlaceOfWork nvarchar(64),
MotherPlaceOfWork nvarchar(64),
DateOfHiring datetime,
YearsOfService int,
Formation nvarchar(128)
)

create table Grade
(
GradeId int primary key identity(1,1),
Name nvarchar(32) not null,
LevelId int foreign key references Level(LevelId),
Observation nvarchar(256)
)

create table GradeInstance
(
GradeInstanceId int primary key identity(1,1),
StaffId int foreign key references Staff(StaffId),
GradeId int foreign key references Grade(GradeId),
Name nvarchar(32) not null,
Year datetime
)

create table Student
(
StudentId int primary key identity(1,1),
RUDE int,
Name nvarchar(64) not null,
LastNameFather nvarchar(256) not null,
LastNameMother nvarchar(256) not null,
DateOfBirth datetime not null,
PlaceOfBirth nvarchar(128),
Sex nvarchar(8),
Carnet nvarchar(32),
Telephone nvarchar(64),
MobilePhone nvarchar(64),
Address nvarchar(256),
FatherName nvarchar(512),
MotherName nvarchar(512),
FatherMobilePhone nvarchar(64),
MotherMobilePhone nvarchar(64),
FatherProfession nvarchar(64),
MotherProfession nvarchar(64),
FatherPlaceOfWork nvarchar(256),
MotherPlaceOfWork nvarchar(256),
Observations nvarchar(3000)
)

create table StudentInstance
(
StudentInstanceId int primary key identity(1,1),
GradeInstanceId int foreign key references GradeInstance(GradeInstanceId),
StudentId int foreign key references Student(StudentId)
)

create table StudentGradeReport
(
StudentGradeReportId int primary key identity(1,1),
StudentInstanceId int foreign key references StudentInstance(StudentInstanceId),
SubjectId int foreign key references Subject(SubjectId),
FirstTrimester int,
SecondTrimester int,
ThirdTrimester int,
FinalGrade int
)

If you find an attribute that should be null checked, please disregard it. I've gone over this with the client and they want certain things to left blank if so chosen by the end user.

My main concern when designing this database was how to associate a student with a grade here and now, yet keep a record for previous years and manage to see what grade he got back in 2009. See?

I think I've done a good job but you never know - the hivemind here will probably find a flaw and I'd love some feedback.

Your data needs to be more

ATOMIC AND HOMOGENEOUS

This table

create table StudentGradeReport
(
    StudentGradeReportId int primary key identity(1,1),
    StudentInstanceId int foreign key references StudentInstance(StudentInstanceId),
    SubjectId int foreign key references Subject(SubjectId),
    FirstTrimester int,
    SecondTrimester int,
    ThirdTrimester int,
    FinalGrade int
)

doesn't look right to me.

I think that you want something more like this

CREATE TABLE StudentGradeReport
(
    TableID INT PRIMARY KEY IDENTITY(1,1),
    StudentInstanceID INT FOREIGN KEY REFERENCES, StudentInstance(StudentInstanceID),
    SubjectID INT FOREIGN KEY REFERENCES Subject(SubjectID)
    Term NVARCH(2), -- this will tell us T1, T2, T3 or F
    SchoolYear INT,
    UpdateDate DATETIME,
    UserChangeID INT NOT NULL
)
-- the UserChangeID would be a foreign key to the Staff table(or User Table),
-- assuming they are the only people that can change these things.
-- this way you can keep track of who is changing things.

This seems like the right way to create this table.

You want your data to be atomic in all of your tables, but not redundant.

In your version of this table you had redundant data, you would fill in FirstTrimester with a 1 (true) and then the other 3 with a 0 (false) that is very redundant.

The data that would go into that table would have to be updated or overwritten if they wanted to change something or there was a mistake.  This isn't good for keeping track of changes.  In the table that I gave you have the ability to add a new row of data and then keeping track of who made the change, this way you can keep track of who changed what and when.

Having your data in the table that I provided it is going to be a whole lot easier to pull reports from. you can pull data from

a specific student
a specific subject
a Specific school year
a range of school years
all changes made by a specific user

I hope that looking at my answer helps you to see how the rest of your tables need to be more ATOMIC.


Business rule updater I'm trying to increase the speed of my application at the moment. My inserting/updating methods were opening every time a new connection which took too much time. I read that it's better to create a connection object and pass it to different methods. It worked fine but I'm just curious if this is a good practice.

This is a part of my code I wrote:

public class BusinessRuleUpdaterDaoImplV2 extends OracleBaseDao implements BusinessRuleUpdaterDaoV2 {

    @Override
    public boolean updateBusinessRule(BusinessRule businessRule) {
        try(Connection con = super.getConnectionConfigDb()) {
            con.setAutoCommit(false);
            if (updateTableBusinessRule(con,businessRule)){
                return true;
            } else {
                con.rollback();
                return false;
            }
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return false;
    }

    private boolean updateTableBusinessRule(Connection con, BusinessRule businessRule){
        String queryBr = "UPDATE BUSINESSRULE SET ERRORMESSAGE = ?, SQLCODE = ?, CUSTOMNAME = ?, OPERATOR_ID = ? WHERE BUSINESSRULE_ID = ?";
        try(PreparedStatement pstmtBr = con.prepareStatement(queryBr)) {
            pstmtBr.setString(1,businessRule.getErrorMessage());
            pstmtBr.setString(2,businessRule.getSqlQuery());
            pstmtBr.setString(3,businessRule.getName());
            pstmtBr.setInt(4,businessRule.getOperatorID());
            pstmtBr.setInt(5,businessRule.getBusinessRuleID());
            pstmtBr.executeUpdate();
            pstmtBr.close();
            return updateTableTargetTable(con,businessRule);
        } catch (SQLException e) {
            e.printStackTrace();
            return false;
        }
    }

    private boolean updateTableTargetTable(Connection con, BusinessRule businessRule){
        String queryTt = "UPDATE TARGETTABLE SET NAME = ? WHERE TABLE_ID = ?";
        for (int i = 0 ; i < businessRule.getTableListSize() ; i++) {
            try (PreparedStatement pstmtTt = con.prepareStatement(queryTt)) {
                pstmtTt.setString(1,businessRule.getListOfTables().get(i).getName());
                pstmtTt.setInt(1, businessRule.getListOfTables().get(i).getId());
                pstmtTt.executeUpdate();
                pstmtTt.close();
            } catch (SQLException e) {
                e.printStackTrace();
                return false;
            }
        }

        return false;
    }

    private boolean updateTableAttribute(Connection con, BusinessRule businessRule){
        // return updateTableValue(con, businessRule)
        return false;
    }

    private boolean updateTableValue(Connection con, BusinessRule businessRule){
        //return updateTableStack(con, businessRule)
        return false;
    }

    private boolean updateTableStack(Connection con, BusinessRule businessRule){
        return false;
    }
}

As you can see in the first method, updateBusinessRule(), I'm opening a connection and passing it to the next method. If one of the methods returns false, it will make a roll back in the first method.

I'm also curious about why intellij is telling me that pstmtTt.close() is redundant.

I think it is ok to re-use the Connection.

I would not try-catch all the SqlExceptions in the update* methods. Just let them throw. Then you don't need all the boolean logic either. Makes it simpler I guess.


regex string extension I wrote some string-extensions:

public static class RegexStringExtensions
{
    public static string PatternReplace(
        this string seed, string pattern,
        Func<string, string> outPattern)
    {
        return Regex.Replace(seed, pattern, s => outPattern(s.Groups[1].Value));
    }

    public static string PatternReplace(
        this string seed, string pattern,
        Func<string, string, string> outPattern)
    {
        return Regex.Replace(seed, pattern, s => outPattern(s.Groups[1].Value, s.Groups[2].Value));
    }
    public static string PatternReplace(
        this string seed, string pattern,
        Func<string, string, string, string> outPattern)
    {
        return Regex.Replace(seed, pattern, s => outPattern(s.Groups[1].Value, s.Groups[2].Value, s.Groups[3].Value));
    }
    public static string PatternReplace(
        this string seed, string pattern,
        Func<string, string, string, string, string> outPattern)
    {
        return Regex.Replace(seed, pattern, s => outPattern(s.Groups[1].Value, s.Groups[2].Value, s.Groups[3].Value, s.Groups[4].Value));
    }
}

with those, I could handle a replacement for regex-pattern and work with the groups more in C#-way like (just an easy example):

string val =
    "abcdef".PatternReplace(@"(ab)(cd)(ef)", (s,t,u) => u + string.Concat(s.Reverse()));
// result is "efba"

I think it's not that nice, to have so many overloads since I could have like 10 groups or something.
Any idea how I could make this a bit nicer?
For now, it'd work with up to 4 parameters, but with also 4 overloads.

I'm not saying I like this but you could reuse the longest overload for the shorter ones. Here I took the one that takes three parameters.

public static string PatternReplace(
    this string seed,
    string pattern,
    Func<string, string> outPattern
)
{
    return seed.PatternReplace(pattern, (x, y, z) => outPattern(x));
}

public static string PatternReplace(
    this string seed,
    string pattern,
    Func<string, string, string> outPattern
)
{
    return seed.PatternReplace(pattern, (x, y, z) => outPattern(x, y));
}

Here's another small experiment with C# 7 where you could use anonymous tuples with only one method. But actually you could return a tuple in you current solution too.

public static class RegexStringExtensions
{
    public static string PatternReplace(
        this string seed,
        string pattern,
        Func<(string, string, string), string> outPattern
    )
    {
        return Regex.Replace(
            seed,
            pattern,
            m => outPattern((
                m.Groups[1].Value,
                m.Groups[2].Value,
                m.Groups[3].Value)
            )
        );
    }
}

Usage:

string val = "abcdef".PatternReplace(
    @"(ab)(cd)(ef)",
    t => t.Item1 + string.Concat(t.Item2.Reverse())
);


Implement a range behaviour in Python using iterators I've been assigned the following Python homework:

Implement myrange that acts like range using iterators. Define a function myrangeand  define a class MyRange.

We've just seen iterators, so I think I'm asked to use them. I am not required to write a sophisticated code, but only something that allows the range-for loop syntax.
Professor said that, roughly, an iterator is an object for which the dunder method __next__ is provided. I've seen there are similar questions here. However, none of them is defining the __next__ method in a class.

So, here's what I did: first I implemented MyRange class and then my range. After that, I did two tests.
Question: It works, but I'd like to be sure from you if I solved correctly the question, I mean, if this is what I was supposed to do :-) As I said, I've just seen what is an iterator.
class MyRange():

    def __init__(self,data):
        self.data = data
        self.check()
        self.index = -1

    def check(self):
        assert len(self.data)>0, "Not enough data"

    def __iter__(self):
        return self

    def __next__(self):
        if self.index == len(self.data)-1:
            raise StopIteration
        self.index= self.index+1
        return self.data[self.index]

def my_range(*args):
    return MyRange((args))

print("Tests using MyRange class \n")

for i in MyRange((1,2,3,4)):
    print(i)

print("Tests with range for loop \n")
for i in my_range(1,2,3,4,5,"Hello"):
    print(i)

r = MyRange((1,2,3,4,"Last Value"))
print(next(r))
print(next(r))
print(next(r))
print(next(r))
print(next(r))
print(next(r))

I'll show here the output, which seems the correct one:
1
2
3
4
Tests with range for loop

1
2
3
4
5
Hello
1
2
3
4
Last Value
Traceback (most recent call last):
  File "Untitled3.py", line 45, in <module>
    print(next(r)) #<-- Raises StopIteration
  File "Untitled3.py", line 16, in __next__
    raise StopIteration
StopIteration

You have an iterator
Your code implements an iterator, you are right. You have defined a __next__ method and it works well.
I would tweak your code slightly, though:

remove the check because iterators can be empty and that is okay;
fix minor spacing issues around operators, etc;
use augmented assignment to increment;
change order of statements in __next__ to be more idiomatic.

All in all, MyRange would end up looking like this:
class MyRange():
    def __init__(self,data):
        self.data = data
        self.index = -1

    def __iter__(self):
        return self

    def __next__(self):
        self.index += 1
        if self.index == len(self.data):
            raise StopIteration
        return self.data[self.index]

In particular, the changes to __next__ are because you start by setting self.index to the value of the index you would like to read. Then, just before reading the value from data, you ensure you can actually use that index and raise StopIteration if you can't.
range is not any iterator
However, the "problem statement" says to implement an iterator that behaves like range, and as brought to your attention in the comments, range is a very specific iterator that can take up to 3 arguments: start, stop, and step.
Take a look at the docs for the range function here.
Having said that, solving this "issue" does not need to be very difficult.
You could do something as simple as having my_range take the three arguments,
generate the data you need, and then feed it into MyRange to iterate, but that seems like a waste.
What I think is more or less the intended approach, is to define MyRange to expect the start, stop, and step values, and the my_range function is what the user calls, which then fills in the start, stop, and step values that are used by default and calls MyRange.
E.g.,

my_range(10) would call MyRange(0, 10, 1);
my_range(4, 12) would call MyRange(4, 12, 1);
my_range(0, 13, -3) would call MyRange(0, 13, -3).

(In the examples above, I assumed you changed MyRange to do what I described.)
Therefore, just for the sake of clarity: what would be happening is that the __next__ method would use the three arguments start, stop, and step (and possibly other auxiliary variables you create) to compute the successive values, instead of generating all of them at once and then returning one by one.


Random integer range with exceptions I have to generate a random number from a range with exceptions held in a bool array, is there a way to do it without a loop depending on probability?
There is also a potential infinite loop here if all values are false.
Any suggestions?

    bool[] paths = InnateChannels(channelID, channelOrientation);

    bool done = false;
    int iDirection = 0;
    while (!done)
    {
        iDirection = Random.Range(0, 4);
        if (paths[iDirection])
            done = true;
    }
    direction = DirectionToVector(iDirection);

As you already noticed, your code will loop forever if all array elements
are false, so you have to check first if at least one element is true.

If the array has \$ N \$ elements with \$ t \$ of them being true, then
the probability that a random number in the range \$ 0 ... N-1 \$ is
"valid" is \$ t/N \$. This means that the expected number of iterations
is \$ N/t \$. For example, if half of the possible directions are valid,
you'll need 2 iterations in average. If only 1% are valid, you'll need
100 iterations in average.

Here is a possible alternative which needs only a single random number.
As I am not so familiar with C#, I'm
describing the algorithm only.

Count the number \$ t \$ of true elements in the array.
Terminate if \$ t = 0 \$.
Compute a random number \$ i \$ in the range \$ 1 ... t \$.
Traverse the array once to find the \$i^\text{th} \$ entry which is true.
The index of that element is your result.


Check if a numpy array contains numerical data This function serves me as a testing-utility to check if the result is really numeric and sometimes as input-validation if there are a lot of operations before I would find out (with an Exception) if it's not-numerical.

But I feel like the .dtype.kind in ... is too complex. I've written this function a while back and tried using some better approach but I couldn't find any solution that works for python 2.7 and 3.x and different numpy version 1.7+.

The code:

import numpy as np

def is_numeric_array(array):
    """Checks if the dtype of the array is numeric.

    Booleans, unsigned integer, signed integer, floats and complex are
    considered numeric.

    Parameters
    ----------
    array : `numpy.ndarray`-like
        The array to check.

    Returns
    -------
    is_numeric : `bool`
        True if it is a recognized numerical and False if object or
        string.
    """
    numerical_dtype_kinds = {'b', # boolean
                             'u', # unsigned integer
                             'i', # signed integer
                             'f', # floats
                             'c'} # complex
    try:
        return array.dtype.kind in numerical_dtype_kinds
    except AttributeError:
        # in case it's not a numpy array it will probably have no dtype.
        return np.asarray(array).dtype.kind in numerical_dtype_kinds

and I have the following tests:

def test_not_array():
    assert is_numeric_array(1)
    assert is_numeric_array(1.)
    assert is_numeric_array(1+1j)
    assert not is_numeric_array('a')
    assert not is_numeric_array(None)
    assert is_numeric_array([1, 2, 3])

def test_array():
    assert is_numeric_array(np.array(1))
    assert is_numeric_array(np.array(1.))
    assert is_numeric_array(np.array(1+1j))
    assert is_numeric_array(np.array([1]))
    assert is_numeric_array(np.array([1.]))
    assert is_numeric_array(np.array([1+1j]))
    assert not is_numeric_array(np.array('a'))
    assert not is_numeric_array(np.array(['a']))

test_not_array()
test_array()

1. Question

You write, "I feel like the .dtype.kind in ... is too complex". You're probably right about that: I've never needed anything like this in NumPy code. Normally I know what the datatypes are, or I rely on the caller setting them up correctly. But I don't think I can help unless you can explain what you are using this for. Why do you need to know whether an array is numeric or not?

Update: you say in comments that you are trying validate data read in from files. For this use case, consider using np.genfromtxt, passing loose=False. For example:

>>> from io import BytesIO
>>> np.genfromtxt(BytesIO(b'1,2,x,3'), dtype=float, delimiter=',', loose=False)
Traceback (most recent call last):
  File "numpy/lib/_iotools.py", line 688, in _strict_call
    new_value = self.func(value)
ValueError: could not convert string to float: b'x'

2. Review

You've written a docstring! That's excellent.
But I think the docstring could be clearer about what happens when array is not a NumPy array. I would write something like, "Determine whether the argument has a numeric datatype, when converted to a NumPy array."
The docstring says, "False if object or string" but those are not the only non-numeric kinds (there's also unicode and void), so I would write something like, "True if the array has a numeric datatype, False if not."
np.asarray is cheap if the argument is already an array: "No copy is performed if the input is already an ndarray" so you might as well call it in all cases, and avoid the try: ... except: and the duplicated code.
The set numerical_dtype_kinds is always the same, and so it ought to be a global variable.
The test cases would be more convenient to run if you used the features in the unittest module.
There's a lot of repetition in the test cases. Since all the tests are of the form assert is_numeric_array(x) or assert not is_numeric_array(x), it would make sense to put the test cases in a couple of lists, and iterate over them. There's duplication between test_not_array and test_array that could easily be removed.
There are no test cases checking that Booleans are numeric, or that objects are not.

3. Revised code

import numpy as np

# Boolean, unsigned integer, signed integer, float, complex.
_NUMERIC_KINDS = set('buifc')

def is_numeric(array):
    """Determine whether the argument has a numeric datatype, when
    converted to a NumPy array.

    Booleans, unsigned integers, signed integers, floats and complex
    numbers are the kinds of numeric datatype.

    Parameters
    ----------
    array : array-like
        The array to check.

    Returns
    -------
    is_numeric : `bool`
        True if the array has a numeric datatype, False if not.

    """
    return np.asarray(array).dtype.kind in _NUMERIC_KINDS

from unittest import TestCase

class TestIsNumeric(TestCase):
    NUMERIC = [True, 1, -1, 1.0, 1+1j]
    NOT_NUMERIC = [object(), 'string', u'unicode', None]

    def test_is_numeric(self):
        for x in self.NUMERIC:
            for y in (x, [x], [x] * 2):
                for z in (y, np.array(y)):
                    self.assertTrue(is_numeric(z))
        for x in self.NOT_NUMERIC:
            for y in (x, [x], [x] * 2):
                for z in (y, np.array(y)):
                    self.assertFalse(is_numeric(z))
        for kind, dtypes in np.sctypes.items():
            if kind != 'others':
                for dtype in dtypes:
                    self.assertTrue(is_numeric(np.array([0], dtype=dtype)))


Stack implementation using a growing array I'm working through Sedgewick's Algorithms book for fun and educational purposes. I am implementing a simple Stack<T>, using System.Collections.Generic.Stack<T> as a guide:

[System.Diagnostics.DebuggerDisplay("Count = {Count}, Size = {_values.Length}")]
public class Stack<T>
{
    // Constructors
    public Stack(int capacity)
    {
        _values = new T[capacity];
        _count = 0;
    }

    public Stack() :
        this(DEFAULT_SIZE)
    {
    }

    // Static variables
    private static readonly int DEFAULT_SIZE = 4;
    private static readonly float DEFAULT_TRIM_THRESHOLD = 0.90f;

    // Variables
    private T[] _values;
    private int _count;

    // Properties
    public int Count
    {
        get { return _count; }
    }

    // Methods
    public void Push(T value)
    {
        if (_count == _values.Length)
            Array.Resize(ref _values, _values.Length << 1);
        _values[_count++] = value;
    }

    public T Pop()
    {
        T result = _values[--_count];
        _values[_count] = default(T);
        return result;
    }

    public T Peek()
    {
        return _values[_count - 1];
    }

    public void Clear()
    {
        for (int i = 0; i < _count; i++)
            _values[i] = default(T);
        _count = 0;
    }

    public void Trim()
    {
        if (_count / (float)_values.Length < DEFAULT_TRIM_THRESHOLD)
            Array.Resize(ref _values, Math.Max(_count, DEFAULT_SIZE));
    }
}

A few things:

There were #region and <summary> tags on everything, but I removed them to post here, since it seemed like it added a lot of visual noise and made the code otherwise harder to read.
The class does not currently implement IEnumerable<T> or IEnumerable, but will.
I don't want to implement ICollection<T>, since I think Add() and Remove() would be inappropriate methods for this class. Doing something like hiding them behind an explicit interface implementation would be misrepresenting the class's intent.
Performance. Premature optimization should be avoided obviously, but I would like the classes I implement from the book to be useful in future projects. It doesn't need to outperform the default implementation, but it should be comparable.
I compared my Push() method against the default implementation by adding 1,000,000 items to the stack. It takes ~68,000 ticks for mine, and ~25,000 ticks for the default. Even if I strip Push() down to just the array assignment, using compile on Release with optimized code enabled, the default is still running at twice the speed mine is, and I have no idea why.

Any feedback or insight is appreciated!

Edit: As requested, here is the code I am using to benchmark the speed. This is what I usually do to get a sense of which of two things is faster; not sure how hackish it's considered:

int n = 10000000;
long r1, r2;

var stack1 = new Stack<int>();
{
    Stopwatch w = new Stopwatch();
    w.Start();
    {
        for (int i = 0; i < n; i++)
            stack1.Push(i);
    }
    w.Stop();
    r1 = w.ElapsedTicks;
}

var stack2 = new System.Collections.Generic.Stack<int>();
{
    Stopwatch w = new Stopwatch();
    w.Start();
    {
        for (int i = 0; i < n; i++)
            stack2.Push(i);
    }
    w.Stop();
    r2 = w.ElapsedTicks;
}

As a baseline, it takes r2 = ~37,015 ticks for the default implementation to add 1,000,000 elements.

r1 = ~76,892 ticks (compile to debug, w/ resizing)
r1 = ~66,638 ticks (compile to debug, no resize, keep resize logic)
r1 = ~57,357 ticks (compile to debug, no resize, resize logic omitted)
r1 = ~51,420 ticks (compile to release, w/ resizing)
r1 = ~40,909 ticks (compile to release, no resize, keep resize logic)
r1 = ~33,821 ticks (compile to release, no resize, resize logic omitted)

So, looking at the different results, it seems that I can achieve the same speed as the default implementation, but at the cost of making the class brittle and inflexible.

Your code looks very good, except for one bug in the constructor:

public Stack(int capacity)

You should really do a bounds check on capacity. It should not be allowed to be negative, of course.
But also, if capacity is zero, you will get an exception when you call Push. It will try to resize to count * 2, which is zero.

_values.Length << 1

Code should be clear. If you mean to multiply by 2, say * 2, not << 1

I would really implement a GetEnumerator() method, so you can iterate over the Stack items, even if you're not going to implement any collection interface. Implementing IEnumerable<T> is handy if you want to use LINQ.


Comparing unordered list in java I want to compare two unordered list and currently I am doing it in the following way. Is there a better way to do this as I am repeating my code twice (for null check & then converting to a Set). Maybe there exists a library in Guava which does this?

public class CompareUnorderedList implements BiPredicate<Object, Object> {
    private Function<Object, Object> mapping;
    public CompareUnorderedList(Function<Object, Object> mapping) {
       this.mapping = mapping;
    }
    @Override
    public boolean test(Object o1, Object o2) {
        o1 = o1 != null ? o1 : Collections.EMPTY_LIST;
        o2 = o2 != null ? o2 : Collections.EMPTY_LIST;

        Set<Object> firstSet = (List<Object>o1).stream().map(mapping).collect(Collectors.toSet());
        Set<Object> secondSet = (List<Object>o2).stream().map(mapping).collect(Collectors.toSet());
        return firstSet.equals(secondSet);
    }
}

I think it would be better to not convert the List to a Set, but just check if they are the same size and if one contains all of the other (as it is done in HashSet) like so:

public static boolean test(Object o1, Object o2){
    try{
        Collection<?> c1 = (Collection<?>)o1,
                      c2 = (Collection<?>)o2;
        return (c1.size()==c2.size())&&c1.containsAll(c2);
    }
    catch(Exception e){
        return false;
    }
}


A recursive_count_if Function with Automatic Type Deducing from Lambda for Various Type Arbitrary Nested Iterable Implementation in C++ This is a follow-up question for A recursive_count_if Function For Various Type Arbitrary Nested Iterable Implementation in C++ and A recursive_count_if Function with Specified value_type for Various Type Arbitrary Nested Iterable Implementation in C++. After digging into the thing that detecting the argument type of a function, I found that it is possible to simplify the T1 parameter in last implementation with boost::callable_traits::args_t syntax in Boost.CallableTraits library. As the result, recursive_count_if template function can be used exactly as the following code.
std::vector<std::vector<std::string>> v = {{"hello"}, {"world"}};
auto size5 = [](std::string s) { return s.size() == 5; };
auto n = recursive_count_if(v, size5);

The implementation of recursive_count_if function with automatic type deducing:
#include <boost/callable_traits.hpp>
//  recursive_count_if implementation
template<class T1, class T2> requires (is_iterable<T1> && std::same_as<std::tuple<std::iter_value_t<T1>>, boost::callable_traits::args_t<T2>>)
auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::count_if(input.begin(), input.end(), predicate);
}

//  transform_reduce version
template<class T1, class T2> requires (is_iterable<T1> && !std::same_as<std::tuple<std::iter_value_t<T1>>, boost::callable_traits::args_t<T2>>)
auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [predicate](auto& element) {
        return recursive_count_if(element, predicate);
        });
}

The used is_iterable concept:
template<typename T>
concept is_iterable = requires(T x)
{
    *std::begin(x);
    std::end(x);
};

The constraints of usage
Because the type in the input lambda function plays the role of termination condition, you can not use auto keyword as generic lambdas here. If the lambda function like [](auto element) { } is passed in, compiling errors will pop up. If you want to use generic lambdas, maybe you can choose the previous version recursive_count_if function due to the termination condition is separated.
A Godbolt link is here.
All suggestions are welcome.
The summary information:

Which question it is a follow-up to?
A recursive_count_if Function For Various Type Arbitrary Nested Iterable Implementation in C++ and
A recursive_count_if Function with Specified value_type for Various Type Arbitrary Nested Iterable Implementation in C++

What changes has been made in the code since last question?
This version of recursive_count_if template function is boost-dependent, and the type of termination condition can be deduced automatically from input lambda parameter.

Why a new review is being asked for?
In my opinion, the requires-clause of recursive_count_if template function is a little complex and it's boost-dependent. If there is any simpler way to do this, please let me know.

Don't try to deduce the predicate's parameter type
As Quuxplusone mentioned in the comments of the predecessor question, you should not try to determine the type of the predicate function's parameter, this is bound to fail. And indeed the problem immediately starts when you look at your predicate:
auto size5 = [](std::string s) { return s.size() == 5; };

And think: hey, that's making a copy of s, I should pass that by const reference instead:
auto size5 = [](const std::string &s) { return s.size() == 5; };

Now it fails the requires clauses of your functions. You might be able to add some more tricks to cast away cv-qualifiers and references before comparing the types with std::same_as, but that does not cover all possible situations either. For example, I could write a lambda which allows any type of std::basic_string to be checked:
auto size5 = []<typename T>(std::basic_string<T> s) { return s.size() == 5; };

As I suggested in the comments, you just want to write a concept that checks if the predicate can be applied to the argument it is fed, you don't need to check the types themselves. Here is a possible implementation:
template<typename Pred, typename T>
concept is_applicable_to_elements = requires(Pred predicate, const T &container)
{
    predicate(*container.begin());
};

template<class T1, class T2> requires is_applicable_to_elements<T2, T1>
auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::count_if(input.begin(), input.end(), predicate);
}

template<class T1, class T2>
auto recursive_count_if(const T1& input, const T2 predicate)
{
    return std::transform_reduce(std::begin(input), std::end(input), std::size_t{}, std::plus<std::size_t>(), [predicate](auto& element) {
        return recursive_count_if(element, predicate);
    });
}

Note that since all the templates used is_iterable(), you can remove that requirement, although if you keep it you get nicer error messages if you accidentily try to apply recursive_count_if() on something that does not support iterating over.


Optimize this path location script? Example Script is located in /home/insp/public_html/deploy/

I want to return the /home/insp/ section

$path = realpath(__DIR__);

        $parts = explode('/', $path);

        $newPath = array(
            $parts[0],
            $parts[1],
            $parts[2],
        );

        $realPath = implode('/', $newPath);

Is there a better/more effeicent way to acheve this?

A different way:

preg_match(':^/[^/]*/[^/]*/:', realpath(__DIR__), $matches);
$realPath = $matches[0];

This matches the first and second files in path, and stores them in $matches.


Javascript coding wager I have two scripts written by my co-workers for auto-filling a form with information from the Facebook graph-api. I am trying to figure out which has been coded in a more efficient style?

This is the first one. http://jsfiddle.net/TcGGZ/30/

$('#account_get').click(function(){
    $.getJSON('http://graph.facebook.com/'+$('#facebook').val(), function(fd) {
        $('#id').val(fd.id);
        $('#name').val(fd.name);
        $('#first_name').val(fd.first_name);
        $('#last_name').val(fd.last_name);
        $('#link').val(fd.link);
        $('#username').val(fd.username);
        $('#gender').val(fd.gender);
        $('#locale').val(fd.locale);
    });
});

This is the second one I am trying to decide about. http://jsfiddle.net/VuY2b/46/

var facebook_account = '';

function fillForm() {
    facebook_account = $('#facebook').val();

    $.getJSON('http://graph.facebook.com/' + facebook_account, function(data) {
        var facebook_data = data;
        console.log(facebook_data);
        var inputs = $('form > input');
        for (var key in facebook_data) {
            for (var l = 0; l < inputs.length; l++) {
                if (inputs[l].getAttribute('name') === key) {
                    inputs[l].value = facebook_data[key];
                }
            }
        }
    });

}

var x = document.getElementById('account_get');
x.addEventListener("click", fillForm, false);

The first one will be slow because of all the jQuery selection.

The second one will be slow because of the two loops. It keeps looping over what was already found. BUT it is going to be faster than the first one.

I made a basic jSPerf with a basic object: http://jsperf.com/some-random-beef

I also added two more cases to show you faster ways. NOW, if the object contains more junk in it, the results may differ.

var inputs = document.forms[0];
for (var key in fd) {
    var elem = inputs[key];
    if (elem) {
        elem.value = fd[key];
    }
}

Now what version of the code would I want to read? The first one.


Optimization on Sieve of Eratosthenes using vector I am solving this prime generator problem from SPOJ:

Peter wants to generate some prime numbers for his cryptosystem. Help him! Your task is to generate all prime numbers between two given numbers!
Input
The input begins with the number t of test cases in a single line (t<=10). In each of the next t lines there are two numbers m and n (1 <= m <= n <= 1000000000, n-m<=100000) separated by a space.
Output
For every test case print all prime numbers p such that m <= p <= n, one number per line, test cases separated by an empty line.

I’m using the Sieve of Eratosthenes. Whenever I submit the following code it gives Time Limit Exceeded. Can anyone please give me some optimizations that can make my code run faster?
#include <iostream>
#include <cmath>
#include <vector>
using namespace std;
using ll = long long;

int main() {

    ios_base::sync_with_stdio(false);

    int t = 0;
    cin >> t;

    while (t--) {
        ll first = 0, n = 0;
        cin >> first >> n;
        vector<bool> primes(n+1, 0);

        for (ll i = 2; i <= sqrt(n); ++i) {
            if (primes[i] == 0) {
                for (ll j = 2; (i*j) <= n; ++j) {
                    primes[i*j] = 1;
                }
            }
        }

        for (ll i = 2; i <= n; ++i) {
            if (primes[i] == 0 && i >= first) {
                cout << i << '\n';
            }
        }

    }

    return 0;
}

Repeating work already done
The biggest problem with your solution is that for each test case, you start over from scratch.  This causes you to do up to 10x the work that you need to do.  What you should do instead is:

Read all test cases first (into a vector for example).
Find the maximum n of all the test cases.
Use your sieve once to find all primes up to the maximum number.
Using the vector of primes that you produced with the sieve, solve all test cases.

Don't use expensive operations in loop iterator
Depending on your compiler, this line could be costly:

    for (ll i = 2; i <= sqrt(n); ++i) {

You could instead do:
     for (ll i = 2, sqrt_n = sqrt(n); i <= sqrt_n; ++i) {

to make sure that you don't call sqrt() once per loop iteration.
Sieve loop improvements
This sieve loop:

            for (ll j = 2; (i*j) <= n; ++j) {
                primes[i*j] = 1;
            }

Could be written more optimally as:
            for (ll j = i*i, dj = i+i; j <= n; j += dj) {
                primes[j] = 1;
            }

Notice, the loop starts at i*i instead of 2*i, and it iterates by 2*i instead of by i.  It also avoids multiplication within the main loop.  However, this loop assumes that all even numbers have already been marked from the primes array, so you will have to do that separately when you create the primes array.
Printing loop
The printing loop is not optimal because it starts at 2, even though first may be very large:

    for (ll i = 2; i <= n; ++i) {
        if (primes[i] == 0 && i >= first) {
            cout << i << '\n';
        }
    }

You should just start the loop at first:
    for (ll i = first; i <= n; ++i) {
        if (primes[i] == 0) {
            cout << i << '\n';
        }
    }