Do CriteriaQuery code more readable and elegant

In general this looks okay to me.  In fact I have a similar pattern in
an application with lots of optional filters.  Newer Java 8 features
which can make this a bit nicer (e.g. using Optional and method
references).

Throwing/catching RuntimeException like that looks iffy.  At least
add a message to the exception so the reader knows how to interpret
this.
Returning a List only to immediately convert it to an array doesn't
sound that useful, then again, if it's being used from other places it
might be okay.  List also has isEmpty, better than having to call
yet another static function for it.
CriteriaQuery.where
is already using the conjunction, no need to add the and there.
Note that the array can be passed for the
varargs arguments:

if (!CollectionUtils.isEmpty(predicates)) {
    cq.where(predicates.toArray(new Predicate[predicates.size()]));
}

For the the string matching queries additional helper methods might be
nice:

private static startsWithLower(Expression<String> expression, String pattern) {
    return cb.like(cb.lower(expression), pattern.toLowerCase() + "%");
}


Adding two different group of objects to a list of groups

The Name() class has what I would call a half-assed constructor: it creates an object in a not-quite-initialized state, making it temporarily useless until the setters are called.  If you had a Name(String name, String attrib) constructor, then the code would be a lot less cumbersome.

You can also avoid using indexed loops.

for (String city : list1) {
    namesList.add(new Name(city, "City"));
}
for (String country : list2) {
    namesList.add(new Name(country, "Country"));
}


Rock Paper Scissors without arrays and if-statements, how to reduce

This will do

int a = _random.Next(0, 3);
int b = _random.Next(0, 3);
int winner = (a - b + 4) % 3 - 1;

There is no need to subtract angles in radians or degrees as you can simply use integers. Technically a - b is sufficient to determine the winner and + 4 % 3 - 1 wraps it to the -1/0/+1 interval.


The good:

Well I was actually struggling a little to find places in your code, where you have made no mistakes whatsoever (when seen as whole), but actually there's a lot of good things in your code:

Correct use of the right data structures
Extraction of "magic numbers" to properly named constants
Correct implementation of the singleton pattern
Clean and simple initialization

But wherever there's light there's also shadows.

The bad:

You are overcommenting. Comments that merely restate what happens in the line right below them are useless noise. Comments like that don't explain anything new.
You can actually assume, that people reading your code can program, there's no necessity to spell things out for them. After all they are intelligent ;)

Instead your comments should shed light on complicated algorithms, that are hard to follow, even when written cleanly. Also important to comment is the why of code that's "different" to the standard. Further reading
You're violating naming conventions. Fields, no matter whether they are static or not, should be named in camelCase instead of pascal case. This was the reason Vladimir thought your code didn't compile.
Overly long names: MyThreadPoolInstance could just be instance. This also applies to MyPoolExecutor and MyBlockingQueue, which should be executor and queue respectively.
And while we're at the queue. It's never used. If you need it use it, if you don't need it: get rid of it.

The ugly:

There's two kinda ugly things about your code. The first being that you are misusing a static initializer block where it's completely unnecessary. Instead you could just do the eager initialization directly. Your executor and KEEP_ALIVE_TIME_UNIT do not depend on any ordering.

The second thing is that you're using the Singleton pattern. The Singleton pattern is often a misinformed choice. The thought of using a Singleton often comes from needing "a central ... for ..."

That is in 90% of the cases possible to solve without resorting to the Singleton pattern. This is especially true in your case. Often singletons get misused as a "Utility class". Enforcing Singleton use is problematic, because it can easily become a bottleneck in your application if done wrongly.

Additionally in your case (as in most) it's completely unnecessary. Instead you should strive to make your code just use the built-ins of java, as demonstrated in Vladimir's answer


This solution is a bit too much for such a task

Not even close. Welcome to the Natural Language Processing space :)

In order to properly compare the English texts we at least need to apply the following things:

make the comparison case-insensitive
remove punctuation and retain alphabetic words only
remove stop words
lemmatize words

We can make use of the awesome nltk Python library to help us tokenize, remove the stop words and lemmatize. Here is something more or less generic that works for HTML documents (you can modify the "download content" part if you are working with markdown or other document types):

from bs4 import BeautifulSoup
from nltk import word_tokenize
from nltk.stem import WordNetLemmatizer
from nltk.corpus import stopwords
import requests

ENGLISH_STOPS = set(stopwords.words('english'))

def download_content(url):
    response = requests.get(url)
    soup = BeautifulSoup(response.content, "html.parser")

    return soup.get_text()

def preprocess(text):
    tokens = word_tokenize(text)

    # lowering
    lower_tokens = [token.lower() for token in tokens]

    # Retain alphabetic words: alpha_only
    alpha_only = [t for t in lower_tokens if t.isalpha()]

    # Remove all stop words: no_stops
    no_stops = [t for t in alpha_only if t not in ENGLISH_STOPS]

    # Lemmatize all tokens into a new list: lemmatized
    wordnet_lemmatizer = WordNetLemmatizer()
    lemmatized = [wordnet_lemmatizer.lemmatize(t) for t in no_stops]

    return set(lemmatized)

def compare_documents(*urls):
    contents = [download_content(url) for url in urls]

    return set.intersection(*[set(preprocess(content)) for content in contents])

if __name__ == '__main__':
    print(compare_documents(*[
        "http://greenteapress.com/thinkpython2/html/index.html",
        "http://www.diveintopython3.net"
    ]))

Prints:

{'object', 'generator', 'string', 'first', 'study', 'book', 'name', 'refactoring', 'new', 'case', 'program', 'expression', 'file', 'python', 'method', 'class', 'common', 'comprehension'}

These are, of course, like in your example, not the full books that were compared - just the contents pages of the two.

By the way, here are the common words between your question and my answer (computed using the proposed code):

{
    'task', 'python', 'content', 'following', 'two', 'welcome',
    'natural', 'much', 'return', 'new', 'list', 'book', 'print', 'like',
    'compared', 'def', 'word', 'http', 'punctuation', 'solution', 'bit', 'set'
}


Pretty neat. I think this:

Label label = name.isEmpty() ? new Label("Hello user!") : new Label("How's it going, " + name + "?");

Could be neater like this:

Label label = name.isEmpty()
    ? new Label("Hello user!")
    : new Label("How's it going, " + name + "?");

Actually... what changes depending on the result name.isEmpty() isn't the label itself, but its content.

String greeting = name.isEmpty()
    ? "Hello user!"
    : "How's it going, " + name + "?";
Label label = new Label(greeting);

I'd probably rename label to greetingLabel, too.


I'm not sure why you think that forcing this into one function will make it a "best practice," and that doesn't seem right anyways.

I suppose one way you could do it is:

 WHERE id = :id AND otherid LIKE :otherid

If it were to be in the second function, :otherid would be a wildcard (%). Otherwise just have your value. Check out this.


Updating height of the rows in tables with jQuery

The only think I can think of to make it perform better is to store the jQuery objects and heights in variables:

var rightTr = $(".superBlock div.right tr:visible");

$(".superBlock div.left tr:visible").each(function(i){
    var leftRow = $(this),
        leftRowHeight = leftRow.height(),
        rightRow = rightTr.eq(i);
        rightRowHeight = rightRow.height();

    if (leftRowHeight != rightRowHeight){
        if (leftRowHeight > rightRowHeight){
            rightRow.height(leftRowHeight);
        } else {
            leftRow.height(rightRowHeight);
        }
    }
});

You could simplify the if-else statement a little, though I don't think you'll realistically get a performance boost:

$(".superBlock div.left tr:visible").each(function(i){
    var leftRow = $(this),
        leftRowHeight = leftRow.height(),
        rightRow = rightTr.eq(i);
        rightRowHeight = rightRow.height();

    if (leftRowHeight > rightRowHeight){
        rightRow.height(leftRowHeight);
    } else if (leftRowHeight < rightRowHeight) {
        leftRow.height(rightRowHeight);
    }
});

The big hit to performance is the repainting and reflowing of the page that results from changing the row heights. If there really is a performance problem with resizing the table rows, then you could dive a little deeper into some enhancements.

Figure out the height of the right table
In the left column, insert a DIV before the table. Give it the same height as the right table
Set the width attribute on the left table to it's current width in pixels
Change the position of the table to absolute so that the table is in its own document flow
Resize the rows
Remove the absolute positioning and width attribute
Hide the placeholder DIV

You should get a slight speed boost by making the table position: absolute while the row resizing gets done because the browser only has to reflow the TABLE element on screen, and not any surrounding elements, but really, this might not buy you very much unless the table is big.


You should be aware of Python's "magic methods" to implement class behaviours, including numerical methods like __add__ and __sub__. This allows you to write e.g. note3 = note1 + note2, rather than note3 = note1.add(note2). There are also methods for display, including __repr__, so you can easily view the instance. Additionally:

the class name should be singular, as this only holds one note, and constants should be UPPERCASE;
it's easier to handle either uppercase or lowercase and convert all input accordingly;
you can access class attributes via self too, which makes inheritance easier;
you check note in __init__, but there's nothing to protect it afterwards; and
number could be calculated based on note as required, rather than stored as duplicate information.

Here's an alternative implementation with those ideas in.

#!/usr/bin/env python3

from string import ascii_uppercase

class Note:

    NOTE_COUNT = 7
    NUMBERS = dict(enumerate(ascii_uppercase[:NOTE_COUNT]))
    NOTES = {v: k for k, v in NUMBERS.items()}

    def __init__(self, note=None, number=None):
        if note is not None and note.upper() in self.NOTES:
            note = note.upper()
        elif number is not None and number in self.NUMBERS:
            note = self.NUMBERS[number]
        else:
            raise Exception("Not a valid note")
        self._note = note

    def __repr__(self):
        return "Note({._note!r})".format(self)

    @property
    def number(self):
        return self.NOTES[self._note]

    @property
    def note(self):
        return self._note

    @note.setter
    def note(self, new_note):
        new_note = new_note.upper()
        if new_note not in self.NOTES:
            raise Exception("Not a valid note")
        self._note = new_note

    def __add__(self, other):
        try:
            num = other.number
        except AttributeError:
            num = other
        return Note(number=((self.number+num)%self.NOTE_COUNT))

    def __sub__(self, other):
        try:
            num = other.number
        except AttributeError:
            num = other
        return Note(number=((self.number-num)%self.NOTE_COUNT))

In use:

>>> B = Note("B")
>>> C = Note("C")
>>> B + C
Note('D') # not sure this addition makes sense, but it works!
>>> B.note = "X"
Traceback (most recent call last):
  File "<pyshell#38>", line 1, in <module>
    B.note = "X"
  File "<pyshell#34>", line 30, in note
    raise Exception("Not a valid note")
Exception: Not a valid note
>>> B.number
1

Now all you need is octaves and accidentals! You should also add some docstrings explaining what the class does and how to use it.


I'm not sure why you'd want to do this, however your code is really hard to understand.

You merge your functions together better.

name_of finds the parent frame, and then also goes on to handle creating and caching scope, along with finding the next name. This is defiantly breaking single responsibility.

I'd also merge all your one/two line functions into _cache. This is so we can tell what your code is actually doing.

What I'd do use:

the class for _Scopes,
a _scope function to build the scope,
a _get_frame function to get the current frame, and
the name_of function, to use _get_frame, and then call _Scopes.

To create _get_frame I'd move the try into it's own function.
To create _scope I'd:

Merge most of your code into _cache.
I manually perform the dropwhile and takewhile loops.
Move the code from _is_ident and _all_attr into the function.
Merge _all_attr with the takewhile loop. This is as you raise if i.opname in _LOAD. Unless it's LOAD_ATTR.
Remove the instructions list. It's not needed with the above, and you only want the last value anyway.
Don't mutate _Scopes from here.
Make this a generator of names, don't mutate anything.

To create _Scopes I'd:

Create a class, that inherits from collections.UserDict.
Move the rest of your name_of code into a next_name method.
You also can change the code so that it's better too.

Get the scope by using self.get. This means if it's not None then you can return without a second look-up.
Try to get the next(scope, None) if it's none, then you can implicitly remove the scope, by falling through to the creating code. Otherwise you can return the name.
If you wrap the call to _scope in a try, then you can perform the _Scopes.clear() function in the except clause.

And so I'd change your code to (untested):

import dis as _dis
from collections import UserDict

__all__ = ['name_of']

_ATTR = 'LOAD_ATTR'
_LOAD = set(
    'LOAD_CONST',
    'LOAD_NAME',
    'LOAD_GLOBAL',
    'LOAD_FAST',
    'LOAD_CLOSURE',
    'LOAD_DEREF',
    'LOAD_CLASSDEREF'
)

def _scope(frame):
    bytecode = iter(_dis.Bytecode(frame.f_code))
    while True:
        for instruction in bytecode:
            if instruction == 'name_of':
                break
        else:
            break

        name = None
        for instruction in bytecode:
            if instruction.opname != _ATTR:
                if instruction.opname not in _LOAD:
                    break
                raise SyntaxError('Invalid: {!r}'.format(name))
            name = instruction

        if name is None or not name.argrepr.isidentifier():
            raise SyntaxError('Invalid: {!r}'.format(name))
        yield name.argrepr

class _Scopes(UserDict):
    def next_name(self, frame):
        frame_id = id(frame)
        scope = self.get(frame_id, None)
        if scope is not None:
            name = next(scope, None)
            if name is not None:
                return name
        try:
            scope = self.data[frame_id] = _scope(frame)
        except SyntaxError:
            self.clear()
            raise
        return next(scope)

_scopes = _Scopes()

def _get_frame():
    try:
        raise Exception
    except Exception as e:
        return exc.__traceback__.tb_frame.f_back

def name_of(_):
    frame = _get_frame().f_back
    return _scopes.next_name(frame)


Method that fetches rows from database and maps it to a Model class with SqlDataReader

To quote Mark Seemann from the following article Small methods are easy to troubleshoot

  Write small methods. How small? Small enough that any unhandled exception is easy to troubleshoot.

Break the function up into smaller more manageable chunks that focus on a single concern as much as possible.

Yes, the net effect of this is a lot of additional functions. They are however easier to read and maintain.

Another benefit of the many small methods it that it would help identify areas that can be generalized into reusable services that help reduce repeated code, allowing it (the code) to be more DRY (Don't Repeat Yourself)

Lets start with getting the connection

private SqlConnection createConnection() {
    var connection = (SqlConnection) sqlHelper.CreateConnection(); //sqlHelper is a nugget and not in my
    connection.Open();
    return connection;
}

This can now be wrapped in a using block that will dispose of the connection once out of scope

using(SqlConnection connection = createConnection()) {

    //...
}

While there is nothing wrong with creating the command manually, I personal prefer to let the connection do it rather than newing one. Again wrapping in a using block, and populating.

//...

using(SqlCommand command = connection.CreateCommand()) {
    command.CommandText = Constants.GetAllParkingSlips;
    command.CommandType = System.Data.CommandType.StoredProcedure;
    command.Parameters.Add("@CreatedBy", SqlDbType.NVarChar).Value = gepservice.GetUserContext().UserId;
    command.Parameters.Add("@OffsetRows", SqlDbType.NVarChar).Value = offset;
    command.Parameters.Add("@FetchRows", SqlDbType.NVarChar).Value = fetch;

    //...
}

That could have also been refactored into its own method, but in my opinion it would have ended up with too many arguments.

This could be fixed with an aggregated value object.

public class ParkingSlipQuery {
    public int FetchRows { get; set; }
    public int OffsetRows  { get; set; }
    public string CreatedBy { get; set; }
}

for example

private SqlCommand createCommand(SqlConnection connection, ParkingSlipQuery query) {
    SqlCommand command = connection.CreateCommand();
    command.CommandText = Constants.GetAllParkingSlips;
    command.CommandType = System.Data.CommandType.StoredProcedure;
    command.Parameters.Add("@CreatedBy", SqlDbType.NVarChar).Value = query.CreatedBy;
    command.Parameters.Add("@OffsetRows", SqlDbType.NVarChar).Value = query.OffsetRows;
    command.Parameters.Add("@FetchRows", SqlDbType.NVarChar).Value = query.FetchRows;
    return command;
}

and implemented

var query = new ParkingSlipQuery {
    CreatedBy = gepservice.GetUserContext().UserId,
    OffsetRows = offset,
    FetchRows = fetch
};
using(SqlCommand command = createCommand(connection, query)) {
    //...
}

The execution of the command and subsequent reader result however can be moved

private SqlDataReader executeReader(SqlCommand command) {
    var refCountdr = (RefCountingDataReader) sqlHelper.ExecuteReader(cmd);
    return (SqlDataReader) refCountdr.InnerReader;
}

As you have realized by now, you should know what is coming next about building the model to populate the collection.

private ParkingSlipDetails getDetails(SqlDataReader reader) {
    ParkingSlipDetails details = new ParkingSlipDetails();
    details.Id = Convert.ToInt32(reader[FileUploadConstants.Id]);
    details.FileName = Convert.ToString(reader[FileUploadConstants.FileName]);
    details.FileUri = Convert.ToString(reader[FileUploadConstants.FileURI]);
    details.PONumber = Convert.ToString(reader[FileUploadConstants.DocumentNumber]);
    details.UploadDate = Convert.IsDBNull(reader[FileUploadConstants.UploadDate]) ? DateTime.UtcNow: Convert.ToDateTime(reader[FileUploadConstants.UploadDate]);
    details.FileStatus = (Status)(Convert.ToInt16(reader[FileUploadConstants.FileStatus]));
    details.ReceiptNumber = Convert.ToString(reader[FileUploadConstants.RecieptNumber].ToString());
    details.TotalRows = Convert.ToInt32(reader[FileUploadConstants.TotalRows]);
    details.CreatedBy = gepservice.GetUserContext().UserId;

    return details;
}

private List<ParkingSlipDetails> getDetailsList(SqlDataReader reader, string userId) {
    List<ParkingSlipDetails> detailsList = new List<ParkingSlipDetails>();
    if (reader != null) {
        while (reader.Read()) {
            ParkingSlipDetails details = getDetails(reader);
            details.CreatedBy = userId;
            detailsList.Add(details);
        }
    }
    return detailsList;
}

Finally, to avoid null reference errors, the method should try to avoid return null. An empty collection is safer to check than a null collection.

This results in refactor looking like

public CustomReturn<List<ParkingSlipDetails>> GetAllParkingSlips(int offset, int fetch) {
    List<ParkingSlipDetails> detailsList = new List<ParkingSlipDetails>();
    try {
        using(SqlConnection connection = createConnection()) {
            string userId = gepservice.GetUserContext().UserId;
            var query = new ParkingSlipQuery {
                CreatedBy = userId,
                OffsetRows = offset,
                FetchRows = fetch
            };
            using(SqlCommand command = createCommand(collection, query)) {
                using(SqlDataReader reader = executeReader(command)) {
                    detailsList = getDetailsList(reader, userId);
                }
            }
        }
    } catch(Exception ex) {
        LogError("ManageParkingSlipDataAccess", "GetAllParkingSlips", "Error", ex);
        detailsList = new List<ParkingSlipDetails>();
    }
    return new CustomReturn<List<ParkingSlipDetails>>(detailsList);
}

The using block will handle the closing and disposal of the connect and the other disposable members.

If following explicit dependency principle, it could be refactored further down by delegating the argument values to the caller.

public CustomReturn<List<ParkingSlipDetails>> GetAllParkingSlips(ParkingSlipQuery query) {
    List<ParkingSlipDetails> detailsList = new List<ParkingSlipDetails>();
    try {
        using(SqlConnection connection = createConnection()) {
            using(SqlCommand command = createCommand(collection, query)) {
                using(SqlDataReader reader = executeReader(command)) {
                    detailsList = getDetailsList(reader, query.CreatedBy);
                }
            }
        }
    } catch(Exception ex) {
        LogError("ManageParkingSlipDataAccess", "GetAllParkingSlips", "Error", ex);
        detailsList = new List<ParkingSlipDetails>();
    }
    return new CustomReturn<List<ParkingSlipDetails>>(detailsList);
}

with the following support methods

private SqlConnection createConnection() {
    var connection = (SqlConnection) sqlHelper.CreateConnection(); //sqlHelper is a nugget and not in my
    connection.Open();
    return connection;
}

private SqlCommand createCommand(SqlConnection connection, ParkingSlipQuery query) {
    SqlCommand command = connection.CreateCommand();
    command.CommandText = Constants.GetAllParkingSlips;
    command.CommandType = System.Data.CommandType.StoredProcedure;
    command.Parameters.Add("@CreatedBy", SqlDbType.NVarChar).Value = query.CreatedBy;
    command.Parameters.Add("@OffsetRows", SqlDbType.NVarChar).Value = query.OffsetRows;
    command.Parameters.Add("@FetchRows", SqlDbType.NVarChar).Value = query.FetchRows;
    return command;
}

private SqlDataReader executeReader(SqlCommand command) {
    var refCountdr = (RefCountingDataReader) sqlHelper.ExecuteReader(cmd);
    return (SqlDataReader) refCountdr.InnerReader;
}

private List<ParkingSlipDetails> getDetailsList(SqlDataReader reader, string userId) {
    List<ParkingSlipDetails> detailsList = new List<ParkingSlipDetails>();
    if (reader != null) {
        while (reader.Read()) {
            ParkingSlipDetails details = getDetails(reader);
            details.CreatedBy = userId;
            detailsList.Add(details);
        }
    }
    return detailsList;
}

private ParkingSlipDetails getDetails(SqlDataReader reader) {
    ParkingSlipDetails details = new ParkingSlipDetails();

    details.Id = Convert.ToInt32(reader[FileUploadConstants.Id]);
    details.FileName = Convert.ToString(reader[FileUploadConstants.FileName]);
    details.FileUri = Convert.ToString(reader[FileUploadConstants.FileURI]);
    details.PONumber = Convert.ToString(reader[FileUploadConstants.DocumentNumber]);
    details.UploadDate = Convert.IsDBNull(reader[FileUploadConstants.UploadDate]) ? DateTime.UtcNow: Convert.ToDateTime(reader[FileUploadConstants.UploadDate]);
    details.FileStatus = (Status)(Convert.ToInt16(reader[FileUploadConstants.FileStatus]));
    details.ReceiptNumber = Convert.ToString(reader[FileUploadConstants.RecieptNumber].ToString());
    details.TotalRows = Convert.ToInt32(reader[FileUploadConstants.TotalRows]);

    return details;
}


Sieve of Eratosthenes implementation is running slowly

There is some room for improvement in your code, and I will try to explain that in
several steps. But please note that I am not a C# person, so my review refers only
to the algorithm and performance itself and not to the style or any language
specifics. And most probably I am making a lot of C# errors here.

The following tests were done on a MacBook Pro using
Mono.

To show the performance improvement gained in each step, I have rewritten your code a bit:

The prime numbers are not printed because printing many lines also takes time. Instead, only the number of primes is counted.
The time for the computation is taken and printed with StopWatch.
Instead of a fixed upper bound (10,000 in your code) the limit is read from
the command line arguments. I used an upper bound of 50,000.

So your code is equivalent to this:

using System;
using System.Collections.Generic;
using System.Diagnostics; // (for Stopwatch)

namespace Primenumbers
{
    class MainClass
    {
        public static void Main (string[] args)
        {
            int maxPrime = int.Parse(args[0]);
            Stopwatch stopWatch = Stopwatch.StartNew();
            int numPrimes = sieve(maxPrime);
            stopWatch.Stop();
            Console.WriteLine("{0} primes in the range 2..{1}, time taken: {2}ms", numPrimes, maxPrime, stopWatch.Elapsed.TotalMilliseconds);
        }

        static int sieve(int maxPrime)
        {
            List<int> NoPrime = new List<int>();
            for (int x = 2; x <= maxPrime; x++)
            {
                for (int y = x * 2; y <= maxPrime; y = y + x)
                {
                    if (!NoPrime.Contains(y))
                    {
                        NoPrime.Add(y);
                    }
                }
            }

            int numPrimes = 0;
            for (int z = 2; z <= maxPrime; z++)
            {
                if (!NoPrime.Contains(z))
                {
                    numPrimes++;
                }
            }
            return numPrimes;
        }
    }
}

and the output is:

5133 primes in the range 2..50000, time taken: 5744.7922ms

Step 1: Jeroen Vannevel suggested to use HashSet instead of List,
and indeed this reduces the runtime considerably:

5133 primes in the range 2..50000, time taken: 17.7239ms

But an array of booleans is much faster because it allows direct access to all elements
without any lookup. Note that this does need not much more memory. The number of
primes below a number N is approximately N/log(N), so "most" numbers are
not primes. For example, for our upper limit of 50,000 we need an array with
50,000 elements instead of a hash set with 44,867 = 50,000-5,133 elements.

With an array of booleans the sieve method would be (thanks to @abligh for suggesting
a better variable name):

static int sieve(int maxPrime)
{
    bool[] isComposite = new bool[maxPrime + 1];
    for (int x = 2; x <= maxPrime; x++)
    {
        for (int y = x * 2; y <= maxPrime; y = y + x)
        {
            if (!isComposite[y])
            {
                isComposite[y] = true;
            }
        }
    }

    // Count primes ...
    return numPrimes;
}

and now the output is

5133 primes in the range 2..50000, time taken: 1.1336ms

Step 2: If x in the for (int x = ...) loop has already marked as "not a
prime" then all the multiples of x are also not primes, which means that
the inner y-loop needs not to be executed at all. On the other hand, the
multiples y can simply be marked without checking them before:

static int sieve(int maxPrime)
{
    bool[] isComposite = new bool[maxPrime + 1];
    for (int x = 2; x <= maxPrime; x++)
    {
        if (!isComposite[x])
        {
            for (int y = x * 2; y <= maxPrime; y = y + x)
            {
                isComposite[y] = true;
            }
        }
    }

    // Count primes ...
    return numPrimes;
}

Output:

5133 primes in the range 2..50000, time taken: 0.6805ms

Step 3: If a prime x has been found, all lower primes have already been
tested and their multiples marked. Therefore y in the inner loop can start
as x * x instead of x * 2. As a consequence, x needs only to run
from 2 to sqrt(maxPrime):

static int sieve(int maxPrime)
{
    bool[] isComposite = new bool[maxPrime + 1];
    for (int x = 2; x * x <= maxPrime; x++)
    {
        if (!isComposite[x])
        {
            for (int y = x * x; y <= maxPrime; y = y + x)
            {
                isComposite[y] = true;
            }
        }
    }

    // Count primes ...
    return numPrimes;
}

Output:

5133 primes in the range 2..50000, time taken: 0.4782ms

More improvements are possible, e.g. test only 2 and then every odd number,
but I hope this is sufficient for a start.

Finally, here are my measurements for an upper limit of 2,000,000:

Your method: Don't know, did not wait so long
Using a hash set: 2601.3797ms
Using a bool array: 55.2455ms
After applying "step 2": 19.2888ms
After applying "step 3": 11.6878ms


It should be a function. ExitApplication shouldn't need to exist, it ought to be the function's return value.

Function Userlogin(Password, ExitApplication) As Boolean

If the function returns True, we proceed; if the function returns False, the calling code can act accordingly, and exit the application.

Kudos for declaring all the variables that you're using. If the module doesn't specify Option Explicit at the top, add it, and make it a habit: that will make VBA refuse to compile code that contains typos (/undeclared variables).

This is a bad, misleading comment:

' Loop until Password is correct.
Do While Password = ""

The comment says one thing, the code says another. Remove the comment, the only truth here is that of the code. Same with any comment that narrates what the code does: useful comments explain why the code does what it does, and let the code speak for itself about what it's doing.

Your members are implicitly Public. If MainSub is meant to be called from the outside, or as a macro, then make it explicitly Public, and then if UserLogin is meant to only ever be called from MainSub, then make it Private.

Your parameters are implicitly passed ByRef. In many languages including VB.NET, parameters are passed by value by default; it's a good idea to specify ByRef explicitly when you mean to pass them by reference.

The parameters are also implicitly Variant, but Password is meant to be a String and ExitApplication is meant to be a Boolean; it's also a good idea to consistently specify a type, even when that type is Variant, and especially when that type is NOT Variant: that way you'll allocate only the memory space you need to allocate; no more, no less.

The Call keyword / explicit Call syntax is obsolete, and only supported for backward compatibility. Instead of this:

Call UserLogin(Password, ExitApplication)

You can do:

UserLogin Password, ExitApplication

You're not using explicit Call for MsgBox and InputBox function calls; there's no need to use an explicit Call syntax for your own functions either.

You use a lot of "" empty string literals; the vbNullString built-in constant is more efficient, since it's a null string pointer - as the following code in immediate pane demonstrate:

?StrPtr(""), StrPtr(vbNullString)
 456810912     0

See, VBA allocates a memory space for that empty string, but not for vbNullString. And if you call it 5 times...

?StrPtr(""), StrPtr(vbNullString)
 458095304     0
 458097512     0
 167896456     0
 241904984     0
 456810912     0

...you get a new address for it every time.

The InputBox result length-check is fine in this case, because an empty string is deemed invalid input - but the correct way to determine whether the user actually cancelled-out of the input box or meant to supply an empty string, is to verify the StrPtr string pointer value of the result: if that value is 0, the user cancelled the InputBox. If that value isn't 0, the user meant to supply an empty string.

This is interesting:

    If Confirm = vbYes Then
        ExitApplication = True
        Exit Do
        Exit Sub
    End If

    If Confirm = vbNo Then
        Password = ""
        PasswordLength = 8
        Confirm = ""
    End If

The two conditions are mutually exclusive, and the Exit Sub is heuristically unreachable code. It should be one conditional:

If Confirm = vbYes Then
    '...
Else
    '...
End If

It's not clear why Confirm is assigned to an empty string; it's a Variant/VbMsgBoxResult variable (implicitly declared as a Variant), so if anything it should be reset to 0, since VbMsgBoxResult is an enum type, and enum values are really just Long constants.

It looks particularly weird to see Password = "" immediately followed by PasswordLength = 8 - seems you're giving different meanings to PasswordLength. At one point it stands for the expected length, and at another it stands for the user input length. This is confusing.

Remove it. All you really need is this:

Const RequiredLength As Integer = 8

And then:

If Len(Password) <> RequiredLength Then

It reads much clearer, and removes the magic value 8 from your code, too; the string message should also be using that RequiredLength constant, so that if it ever needs to change to 12 (it probably won't, but don't write code with the assumption that requirements never change - that's a terrible mistake to make), the message isn't going to be misleading.


You could create another helper to create your navigation links:

def navigation_link_to(text, path)
  link_to text, path, class: "btn #{active_page(path)}"
end

Then in your view:

= navigation_link_to 'Home', root_path
= navigation_link_to 'Sign in', sign_in_path
= navigation_link_to 'Account', account_path


You use a temporary variable:

st="food | dairy | milk"
keys=st.split(" | ")

new_cat="eco" # should be added
d = t = {} # t is my temporary dictionary

for i in keys:
    t[i] = {}
    t = t[i]
t[new_cat] = {}

print d


Dynamically adding rows to an accessible HTML form

Most of this code is due to the fact that you're increasing the numbers in your attributes. I think that's a mistake. Your rows should all have identical name attributes, stored in an array (e.g. name="task[]").

The only problem would then be the identical IDs. Frankly, you shouldn't be using IDs in the first place. The only advantage of using IDs in this case is to associate the labels to the form fields (so that clicking the label focuses on the form field). This could easily be accomplished by wrapping the label around the form element.

So, to summarize, here are some points to consider:

Get rid of the IDs
Wrap the labels around the form elements to associate them with one another
Get rid of the numbers from the names
Use array like notation (e.g. name="task[]") for your names

Once you've incorporated all this, you can simply clone the row when needed.

HTML:

<fieldset id="timesheet-rows">
    <legend>Add Entries</legend>

    <div class="timesheet-row">

        <label>Project:
            <select name="project[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Department:
            <select name="department[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Task:
            <select name="task[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Hours:
            <input type="number" step="0.25" name="hours[]" width="1" placeholder="2.0" required />
        </label>

        <label>Comment:
            <input type="text" name="comment[]" width="50" />
        </label>

    </div>

    <input type="button" id="add-row" name="add-row" value="Add row" />
</fieldset>

Javascript:

jQuery(function($) {
    var $button = $('#add-row'),
        $row = $('.timesheet-row').clone();

    $button.click(function() {
        $row.clone().insertBefore( $button );
    });
});

Here's the fiddle: http://jsfiddle.net/wd5y9/


Sending templatized e-mail to a million contacts

The big picture

First of all, consider the possibility that this is a trick question.  When sending bulk e-mail, the performance bottleneck is likely to be with the Mail Transfer Agent or the database query that fetches the contacts rather than with the text generation.  I would raise a concern with the interviewer that perhaps we should gather some evidence before making a premature effort to optimize the wrong part of the process.  Optimizing the code in this question (which takes about 2 seconds to run) will not make any noticeable difference to the time necessary to conduct the e-mail campaign.

Review

That said, if you had to optimize the text generation process, your changes would be a mild improvement.  The HashMap is more lightweight than the synchronized Hashtable, and the StringBuilder does save a bit of copying, though not by as much as you think, since you are still making four global substitution passes through the entire emailBody.

Strictly speaking, both the original code and your version of it are buggy.  Because string substitution is being done in multiple passes, it is possible that the output from one round of substitution could introduce a string that looks like a placeholder that ends up being inappropriately substituted in a subsequent round.  (Another example of this class of bug.)  Here, it's unlikely to be a problem unless someone decided to deliberately exploit the bug.

Alternative approach

If I really had to optimize this code, I would take a different approach entirely.  The same emailBody template is likely to used for a million emails.  Therefore, it would make sense to "pre-compile" the template…

Dear {{USER_NAME}},

According to our records, your phone number is {{USER_PHONE}} and
your e-mail address is {{USER_EMAIL}}.  If this is incorrect, please
go to {{LOGIN_URL}} and update your contact information.

… into code that looks like this:

public class C implements Template {
    public void write(Writer out, Map<String, String> params) throws IOException {
        out.write("Dear ");
        out.write(params.get("USER_NAME"));
        out.write("\n\nAccording to our records, your phone number is ");
        out.write(params.get("USER_PHONE"));
        out.write(" and \nyour e-mail address is ");
        out.write(params.get("USER_EMAIL"));
        out.write(".  If this is incorrect, please \ngo to ");
        out.write(params.get("LOGIN_URL"));
        out.write(" and update your contact information.\n");
        out.flush();
    }
}

The performance is likely to be unbeatable, since there is no longer any string substitution going on.  This is, in fact, very similar to the code that a JSP engine generates, and you could try to integrate a JSP implementation instead of reinventing the wheel.

As a bonus, you can stack layers of writers.  For example, you might pass it a subclass of FilterWriter that performs Format=Flowed line wrapping as per RFC 2646 to keep line lengths to a reasonable length as required by SMTP.  You could also stack that directly on top of a Writer that feeds the text directly to an SMTP socket.  Or, to obtain the resulting text as a string as in the original code, just use a StringWriter.

When I tried generating a million messages with a pre-compiled template, it completed the task in half the time of your code.  If you include the time to compile the template into executable form, it's still better than break-even for one million messages.  For a hundred million messages, it's about four times faster, compilation overhead included.


GenBank to FASTA format using regular expressions without Biopython

Line iteration
    gb = f.readlines()
    locus = re.search('NC_\d+\.\d+', gb[3]).group()
    region = re.search('(\d+)?\.+(\d+)', gb[2])
    definition = re.search('\w.+', gb[1][10:]).group()

    for line in (gb):
        # ...
    end_pattern = re.search('[a,t,g,c]{1,9}', gb[-3])

can be
next(f)
definition = re.search('\w.+', next(f)[10:]).group()
region = re.search('(\d+)?\.+(\d+)', next(f))
locus = re.search('NC_\d+\.\d+', next(f)).group()

gb = tuple(f)

for line in gb:

Since you need gb[-3], you can't get away with a purely "streamed" iteration. You could be clever and keep a small queue of the last few entries you've read, if you're deeply worried about memory consumption.
Debugging statements
Remove this:
    print(len(sequence))

or convert it to a logging call at the debug level.
Compilation
Do not do this:
        pattern = re.compile('[a,t,g,c]{10}')

in an inner loop. The whole point of compile is that it can be done once to save the cost of re-compilation; so a safe option is to do this at the global level instead.


Simple atomic pseudo-random number generation

Primarily I'm looking for confirmation that it actually is atomic.

Yes, it appears atomic.

Other issues

Range and precision

randomicDouble() needs more functional explanation about  range and precision.  It seems to attempt an about linear distribution between [a...b]   (b included).

A range of [a...b)  (b excluded) is quite common.  So detailing the coding goal is important.

The code fails to provide a full precision random number.  Simple example: randomicDouble(1.0, 2.0) typically has about 253 different values in the [1.0...2.0) range and this code only supplies 232 different values.

With randomicDouble(), perhaps sometimes 2 calls to randomicNext() are needed.

To provide a linear distribution with randomicDouble(1.0, 4.0) is trickier given the change in absolute precision between  [1.0...2.0) and  [2.0...4.0). randomicDouble(0.0, 1.0) is even tricker.

Initialization

Code passes in only a 32-bit initialization whereas the state is 128-bit.  I'd expect at least a 64-bit initialization if not a full 128-bit one.

Mnior

20 is a magic number in for (int i = 0; i < 20; i++) and deserves explanation.


I don't know what this is called, so I haven't been able to Google a real implementation, so I've had to make my best guess at it. [...] If anyone can tell me what the bit flagging thing used in MsgBox is called, that would be really useful too.

They're called Flag enums in .NET (see this SO question), and apparently the naming is also appropriate for VB6 enums.

That If block definitely smells, because all branches result in lControl being added to mpCollection. Hence, it's not really an If...Else If logic you need here, rather something like:

If CanAddThisControl(ipControlType, lControl) Then mpCollection.Add lControl

This effectively eliminates/replaces the entire If block, but leaves you with a CanAddThisControl method to implement. Let's see...

Private Function CanAddThisControl(ipControlType As ControlTypes, lControl As Control) As Boolean

    'return true if the enum value matches the control's type

End Function

This is where VB6/VBA's lack of structures really hurts. What you need is really some kind of KeyValuePair that associates an enum value with a control type. What if we created a class to do just that?

Private Type tKeyValuePair
    key As Variant
    value As Variant
End Type

Private this As tKeyValuePair
Option Explicit

Public Property Get key() As Variant
    If IsObject(this.key) Then
        Set key = this.key
    Else
        key = this.key
    End If
End Property

Public Property Let key(k As Variant)
    If IsEmpty(k) Then Err.Raise 5
    this.key = k
End Property

Public Property Set key(k As Variant)
    If IsEmpty(k) Then Err.Raise 5
    Set this.key = k
End Property

Public Property Get value() As Variant
    If IsObject(this.value) Then
        Set value = this.value
    Else
        value = this.value
    End If
End Property

Public Property Let value(v As Variant)
    this.value = v
End Property

Public Property Set value(v As Variant)
    Set this.value = v
End Property

Public Function ToString() As String
    ToString = TypeName(Me) & "<" & TypeName(this.key) & "," & TypeName(this.value) & ">"
End Function

(damn VB6 case insensitivity!)

So now we have a way of associating enum values with a string:

Private Function CreateKeyValuePair(key As ControlTypes, value As String) As KeyValuePair
    Dim result As New KeyValuePair
    result.key = key
    result.value = value
    Set CreateKeyValuePair = result
End Function

Private Function GetControlTypesAsKeyValuePairs As Collection
    Dim result As New Collection
    result.Add CreateKeyValuePair(ControlTypes.eButton, "Button")
    result.Add CreateKeyValuePair(ControlTypes.eChart, "ObjectFrame")
    result.Add CreateKeyValuePair(ControlTypes.eCheckBox, "CheckBox")
    result.Add CreateKeyValuePair(ControlTypes.eComboBox, "ComboBox")
    result.Add CreateKeyValuePair(ControlTypes.eFrame, "Frame")
    result.Add CreateKeyValuePair(ControlTypes.eLabel, "Label")
    result.Add CreateKeyValuePair(ControlTypes.eLine, "Line")
    result.Add CreateKeyValuePair(ControlTypes.eListBox, "ListBox")
    result.Add CreateKeyValuePair(ControlTypes.eRadioButton, "RadioButton")
    result.Add CreateKeyValuePair(ControlTypes.eRectangle, "Rectangle")
    result.Add CreateKeyValuePair(ControlTypes.eTextBox, "TextBox")
    Set GetControlTypesAsKeyValuePairs = result
End Function

The above code could be simplified to a one-liner if you implemented a List to wrap the poorly tooled Collection class; see this CR post:

  Private Function GetControlTypesAsKeyValuePairs() As List
      Dim result As New List
      result.Add CreateKeyValuePair(ControlTypes.eButton, "Button"), _
                 CreateKeyValuePair(ControlTypes.eChart, "ObjectFrame"), _
                 CreateKeyValuePair(ControlTypes.eCheckBox, "CheckBox"), _
                 CreateKeyValuePair(ControlTypes.eComboBox, "ComboBox"), _
                 CreateKeyValuePair(ControlTypes.eFrame, "Frame"), _
                 CreateKeyValuePair(ControlTypes.eLabel, "Label"), _
                 CreateKeyValuePair(ControlTypes.eLine, "Line"), _
                 CreateKeyValuePair(ControlTypes.eListBox, "ListBox"), _
                 CreateKeyValuePair(ControlTypes.eRadioButton, "RadioButton"), _
                 CreateKeyValuePair(ControlTypes.eRectangle, "Rectangle"), _
                 CreateKeyValuePair(ControlTypes.eTextBox, "TextBox")
      Set GetControlTypesAsKeyValuePairs = result
  End Function

Now that we have a way of associating each enum value with a specific string, we're equipped to implement CanAddThisControl - I'll assume you went with a Collection, but the code would be pretty much identical if you used the List class I've mentioned above (just swap Collection for List):

Private Function CanAddThisControl(ipControlType As ControlTypes, lControl As Control) As Boolean

    Dim enums As Collection
    Set enums = GetControlTypesAsKeyValuePairs

    Dim kvp As KeyValuePair
    For Each kvp In enums
        If (ipControlType And kvp.Key) And TypeName(lControl) = kvp.Value Then
           CanAddThisControl = True
           Exit For
        End If
    Next

End Function

This should enable you to make the loop in BuildControlCollection as simple as this:

For Each lControl In ipForm.Controls

    If (ipControlType And eAll) Or CanAddThisControl(lControl) Then
        mpCollection.Add lControl
    End If

Next

Now this is a little inefficient, because CanAddThisControl is rebuilding the KeyValuePair collection every time it's called. But it's a fair start I think.


Where best to declare a counter variable to track number of guesses?

As a general rule, you want to use the narrowest possible scope for all variables. We do this so that:

The declaration is close to where we use it, making it easier to reason about why the variable exists.
It minimizes the chance of unintended consequences where the variable is accidentally updated somewhere else.

In this case, it’s so trivial that it doesn’t matter that much, but as your code increases in complexity, this becomes increasingly important. This practice of restricting variables to the narrowest possible scope becomes a central precept in writing safe code: It’s much harder to have unintended consequences or accidentally state changes if variables have narrow scopes.

So, if numberOfGuesses represents how many guesses it took you to guess a  particular random number, then it should be inside the keepPlaying loop. But if you also wanted to keep track of the total number of guesses for all the times you played, then that (probably best kept track in another variable, perhaps, totalNumberOfGuesses) would be outside this outer loop.

By the way, while you’re at it, the continueGuessing and randomNumber, belong inside this keepPlaying loop, too.

By the way,

If you define variables only used within the scope of a loop, it saves you from having to reset that variable for the next iteration. For example, right now you are resetting continueGuessing back to true as the last step in the keepPlaying loop; if you define continueGuessing within the loop, then you wouldn’t have to reset it again at the end of the loop, resulting in simpler code.

In your code, you make reference to “globals”. As a general rule, we try to minimize the use of globals because as as apps scale, globals can make it hard to test, harder to reason about state changes, etc. See Why is Global State so Evil or google “globals are evil”.

Some unrelated observations:

Rather than

let randomNumber = Int(arc4random_uniform(101))

I’d instead recommend:

let randomNumber = Int.random(in: 0...100)

It is equivalent, but arguably is more intuitive. (This would be especially true if you wanted to go from 1 to 100 instead, the arc4random_uniform code would become even less intuitive, whereas random(in: 1...100) is perfectly natural and trivial to reason about.
When you have a loop that you are always going to do at least once, rather than a while loop, we’d often consider repeat-while. E.g. rather than

var shouldPlayAgain = true
while shouldPlayAgain {
    ...

    shouldPlayAgain = ...
}

You might do:

var shouldPlayAgain: Bool
repeat {
    ...

    shouldPlayAgain = ...
} while shouldPlayAgain

Technically the latter is a tad more efficient, but the main benefit is that it makes your intent (to check at the end) absolutely clear.
At the end, you appear to want to be printing number of times the game was played, but you’re printing the number of guesses.

E.g. if you played twice, the first time requiring 3 guesses and the second time requiring 4 guesses, do you want to say “you played 2 times” or do you want to say “you guessed a total of 7 times”?


I don't see a way to improve the algorithm, but here are some general coding tips:

A number can either be greater than 1, or less than 2. So you can use 'else' here instead.

if(list.length < 2){
    return list
}else if(list.length > 1){

Sometimes people create extra variables to improve readability, but here 'n' is less readable than list.length.

let n = list.length

A little bit of commenting can go a long way, for example if you could explain why you add 'Infinity' to the lists in a comment:

listA.push(Infinity)
listB.push(Infinity)


Your code doesn't actually work for some tricky corner cases. Take e.g. [1, 10, 1, 1, 9]. It should be pretty obvious that 19 is the proper answer, but your code returns 11. To build a working solution, let's leave the circularity out, you have already figured how to get the result for a circular array running twice over a linear array.

As ususal with DP we will consider the array incrementally, and in an array dp[j] we will store the value of the maximum donation from the first j neighbors when the j-th neighbor is one of the donors. It is relatively straightforward to see that then:

dp[j+1] = max(dp[:j]) + lst[j+1]

Note that the max excludes the value over dp[j], as Python slices are right-exclusive.

We can turn this idea into code doing something like:

def mncs(lst):
    # MaximumNonContiguousSum
    dp = [0, 0]
    for item in lst:
        dp.append(max(dp[:-1]) + item)
    return max(dp)

This has a dreadful performance, \$O(n)\$ space and \$O(n^2)\$ time, because we are continuously rescanning the full dp array for the maximum. If we are only after the value of the maximum, and not the indices of the neighbors that produce it, we can minimize the overhead by keeping three numbers only, the previous maximum plus the last two entries:

def mncs(lst):
    dp = [0, 0, 0]
    for item in lst:
        m = max(dp[:-1])
        dp = [m, dp[-1], m + item]
    return max(dp)

Since we do constant work per entry, and keep a constant number of values as state, this solution uses \$O(1)\$ space and takes \$O(n)\$ time.

For completeness:

def mnccs(lst):
    # MaximumNonContiguousCircularSum
    return max(mncs(lst[1:]), mncs(lst[:-1]))


I would apply the following improvements:

use collections.defaultdict to count down the number of added and deleted lines separately. This would help to improve the counting logic and avoid having to check for the old date at all
use any() to check for the excluded wildcards
unpack args into added, deleted lines and a filename
switch to gitpython from subprocess

The new version of the code:

from collections import defaultdict
from datetime import datetime
from fnmatch import fnmatch

import git

EXCLUDED_WILDCARDS = ['*.eps', '*.CSV', '*jquery*']

def chomp_int(val):
    try:
        return int(val)
    except ValueError:
        return 0

repo = git.Repo(".")
git_log = repo.git.log(numstat=True, pretty="%at").encode("utf-8")

added, deleted = defaultdict(int), defaultdict(int)

for line in git_log.splitlines():
    args = line.decode('utf8').rstrip().split()

    if len(args) == 1:
        date = datetime.fromtimestamp(int(args[0])).date()
    elif len(args) >= 3:
        added_lines, deleted_lines, filename = args

        # Don't count changesets for excluded file types
        if any(fnmatch(filename, wildcard) for wildcard in EXCLUDED_WILDCARDS):
            continue

        added[date] += chomp_int(added_lines)
        deleted[date] += chomp_int(deleted_lines)

for date, added_lines in added.items():
    print(date, added_lines, deleted[date])


At several places in your code the explicit type annotation is not
needed, for example

var from: Int = start.column
var to: Int = move.end.column + 1

can be simplified because the compiler infers the type automatically:

var from = start.column
var to = move.end.column + 1

Checking for empty fields along the move can be simplified if you iterate
over a stride(), that allows stepping both in increasing and decreasing
order. In func isValidHorizontal() that would be

let direction = (move.end.column - start.column).signum()
for column in stride(from: start.column + direction,
                     to: move.end.column,
                     by: direction) {
    if board.tileType(at: GameBoardPosition(row: start.row, column: column)) != .empty {
        return false
    }
}
return true

This can further be shortened to

return stride(from: start.column + direction,
                to: move.end.column,
                by: direction).allSatisfy {
    board.tileType(at: GameBoardPosition(row: start.row, column: $0)) == .empty
}

using the allSatisfy(_:)
method introduced in Swift 4.2.

This approach can be applied to the other methods as well.

In func isValidDiagonal() you can check if a destination position can
be reached before walking along the diagonal. For example, if the start position
is (0, 0) then an end position (x, y) can be reached only if x == y.

Further remarks:

You have defined the start property in GameMove as optional
(in contrast to the end property). Does it really make sense to specify
only the end position in a move? If not then make that property non-optional,
that would also make several guard statements like

guard let start = move.start else { return false }

obsolete.


Binary search to identify duplicate user names

The description you are providing and your implementation do correspond to binary search indeed. As such, it would probably make sense to perform a few cosmetic changes to your function to make this cleaner and easier to (re)use:

call it binary_search (or something like that)
makes the signature (parameter names and documentation) more generic and not based on the concept of name. The parameters could be called item/element/x/needle and sorted_list/sorted_array.
make the return value clearer, for instance a boolean value telling whether the value was found or not (or maybe an index giving the position)
move all the input/output logic (except maybe for the debug statements) out of the function.

As for the performances, I am a bit concerned by the fact that you are actually performing copies by using the [:] sublist notation. Thus, each step of the algorithm, instead of operating in constant times will be bigger if the sublist you are copying gets bigger.
Also, you are re-sorting the whole array everytime the function is called (and even more that this apparently). The point of using binary search is that you pay the cost of sorting (O(n*log(n))) upfront assuming you'll amortize it by performing many searches in O(log(n)) times. If you have to re-sort everytime, you might as well use a linear search iterating through the array and stopping when the element is found.


if (k > 0 && head != null) {
            ...
            return RotatedList;

        }

        return head;

would be clearer (and more consistent with the other special case) as

        if (k <= 0 || head == null) {
            return head;
        }

        ...
        return RotatedList;

(although really if k < 0 I think it should throw new ArgumentOutOfRangeException(nameof(k))).

            ListNode tail = head,
            RotatedList = null,
            kthnode = head,
            kthPrevNode = head;

Most of these don't need to be declared so early. Declaring variables as late as possible (and, more generally, in the narrowest scope possible) helps to reduce cognitive load when maintaining the code.

            int listLength = 0;
            while (tail != null) {
                listLength += 1;
                tail = tail.next;
            }

This would be worth factoring out as a separate method Length(ListNode head).

            k = k % listLength;
            if (k == 0 || listLength == 0) {
                return head;
            }

This is sort-of buggy. If listLength == 0 then k % listLength will throw an ArithmeticException. However, you can never reach here in that case, because listLength == 0 is equivalent to head == null, and that was already handled in the first special case. To remove confusion, delete || listLength == 0. If you want to validate that condition, insert System.Diagnostics.Debug.Assert(listLength > 0); before the % line.

            for (int i = 0; i < listLength - k; i++) {

                kthPrevNode = kthnode;

                kthnode = kthnode.next;

            }

It's not actually necessary to track two nodes here. The invariant is that kthPrevNode.next == kthnode, so it suffices to track kthPrevNode.

Also, the blank lines seem excessive to me.

            RotatedList = kthnode;

It's conventional in C# that local variable names start with a lower case letter, so to avoid confusion I would rename RotatedList to rotatedList (or perhaps rotatedHead).

There are only two things where I can see that the efficiency can be improved: one is eliminating kthnode in the loop, as commented above; the other would be to hang on to the last node when finding the length, so that you don't need to find it a second time in the last few lines. (That would imply changing the factored out method to return an (int, ListNode) tuple).

As for time complexity, it's already optimal: linear time. Good job!


The sort is not in the loop, so your algorithm is "just" O(n^2) because the loop dominates, the O(n * log(n)) of the sort is neglegable (for big n).
You can solve the problem in O(n * log(n)) and without sorting or destroying the input.

Walk through the trips and build a Map location => {liftUps, dropOffs} (O(n) in time, O(n) in memory)

Iterate the map and build an array of {location, liftUps, dropOffs} (O(n) in time, O(n) in memory)

sort the array by location (this is now the slowest part of the algorithm: O(n * log(n)) in time, O(1) in memory)

iterate the array adding liftUps and subtracting dropOffs for each location to an aggregate integer that starts at zero. (O(n) in time, O(1) in memory)

whenever you surpass capacity return false; return true if the loop finishes

const carPooling = (trips, capacity) => {
    let i = 0;
  const map = new Map();
  for (i = 0; i < trips.length; ++i) {
    const passangers = trips[i][0];
    const start = trips[i][1];
    const end = trips[i][2];

    if (map.has(start)) {
        map.set(start, map.get(start) + passangers);
    } else {
        map.set(start, passangers);
    }

    if (map.has(end)) {
        map.set(end, map.get(end) - passangers);
    } else {
        map.set(end, -passangers);
    }
  }

  const list = new Array(map.size);
  i = 0;
  for (const [location, exchange] of map) {
    list[i++] = {location: location, exchange: exchange}
  }

  list.sort((a, b) => a.location - b.location);

  let occupied = 0;
  for (i = 0; i < list.length; ++i) {
    occupied += list[i].exchange;
    if (occupied > capacity) {
        return false;
    }
  }

  return true;
}

console.log(carPooling([[2,1,5],[3,3,7]], 4));
console.log(carPooling([[3,2,7],[3,7,9],[8,3,9]], 11));


Another way to shuffle a deck is to start of with your full deck (oldDeck) and an empty deck (newDeck). This uses the same random generator function in the OP.  I haven't tried to properly code this because the algorithm is the important message.

While size of oldDeck > 0
  chosenCard = getrandom(0,size of oldDeck-1)
  remove chosenCard from oldDeck ' reduces the size of oldDeck by 1
  append chosenCard to newDeck
'Wend
return newDeck ' or reassign newDeck to oldDeck, depending on the broader context

This algorithm would be particularly useful if a class of Deck existed, and it could be a member function as noted in the previous reply. You could then consider having a class of Card which includes a suit and value. A Deck would then be a collection of Cards, and the built-in collection functions become your friends. Of course, this amount of effort depends on what you are going to do with your Deck.


Find largest palindrome from the product of two 3-digit numbers

The main thing I'd suggest is to use functions more to break up your code into more clearly defined subtasks - especially if those subtasks are reusable.

For example, the lambda in your find_if() is:

[lhs = *it](const auto rhs){
    const auto input = to_string(lhs * rhs);

    return equal(cbegin(input), next(cbegin(input), size(input) / 2U), crbegin(input));
}

That would be clearer - and more maintainable and reusable - as:

[lhs](auto rhs){ return is_numeric_palindrome(lhs * rhs); }

Where is_numeric_palindrome() could be defined as:

auto is_numeric_palindrome(int value)
{
    auto const s = std::to_string(value);

    return is_palindrome(s.begin(), s.end());
}

The way you do the test right now can be neither constexpr nor noexcept, because you use to_string(), but we'll fix that shortly.

is_palindrome() is also an operation that could be reusable, and can be defined as:

// Need C++20 for constexpr std::equal()
// could possibly be noexcept, if only conditionally
template <typename BiDiIterator>
constexpr auto is_palindrome(BiDiIterator first, BiDiIterator last)
{
    return std::equal(//...
}

Once you have these functions written you can optimize them at your leisure. For example, you could go back to is_numeric_palindrome() and rewrite it as:

auto is_numeric_palindrome(int value, int base = 10) noexcept
{
    constexpr auto MAX_SIZE = // determine the max buffer size you need

    auto buffer = std::array<char, MAX_SIZE>{};

    auto res = std::to_chars(buffer.data(), buffer.data() + buffer.size(), value, base);

    return is_palindrome(buffer.data(), res.ptr);
}

That should make the whole algorithm quite a bit faster. And as a free side-effect/benefit, you can now check for palindromes in bases other than 10. (Technically you could do that with to_string(), too, I believe.)

And then after that you could devise a way to test if a number is a palindrome in any base without converting it to a string. You'd just need to rewrite is_numeric_palindrome() again. That might make your code even faster; you'd have to profile to be sure.

Moving on....

I think it would make your code more clear to use some better-named variables in the mix. For example, obviously-named variables like lhs and it_end could be defined right at the top of the loop like:

for(auto it = crbegin(inputs); it != crend(inputs) && *it * *it > max_product; ++it) {
    const auto lhs = *it;
    const auto it_end = crend(inputs);

    const auto it2 = find_if(it, it_end, [lhs](auto rhs){ return is_numeric_palindrome(lhs * rhs); });

    if(it2 != it_end) {
        const auto rhs = *it2;
        const auto product = lhs * rhs;

        if(product > max_product) {
            max_product = product;
            max[0] = lhs;
            max[1] = rhs;
        }
    }
}

I think that's much easier to read and mentally parse.

Now onto some more technical concerns....

You never bother to check that your multiplication doesn't overflow... and in fact, in your demo code it does overflow the minimum size of an int. An int is only guaranteed to hold from -32,767 to 32,767 inclusive... your final palindrome is 906,609. Everything probably works on your system because your ints are 32 or 64 bits, but you really should check these things in general. One easy way to do that is just to get the max value from your inputs, and divide numeric_limits<int>::max() by that, and make sure the result is greater than the divisor.

If you want to generalize this algorithm into its own function - which is a good idea - be sure to check whether the input data is sorted, and be sure to handle the situation where the input data sequence is empty (maybe by returning optional<tuple<int, int>>. And do checks to make sure the data is what you expect it to be (for example, no negative values).


There are methods that scream louder for improvement that the three you mentioned.

For example these:

public bool GetBoolFromEnum(string EnumValue)
{
  return Convert.ToBoolean(EnumValue);
}

public bool GetBoolFromEnum(int EnumValue)
{
  return Convert.ToBoolean(EnumValue);
}

public int GetEnumFromBool(bool BoolValue)
{
  return BoolValue ? 1 : 2;
}

You are converting some enum to bool or int and back but you are not using this enum here, why?

I guess the enum look probably like this (or rather should):

enum State
{
    Disabled,
    Enabled,
};

If you used the enum instead of just calling it like that, your methods might have be implemented as:

public bool ToState(State state)
{
    return Convert.ToBoolean(state);
}

public bool ToState(int state)
{
    return ToBoolean((State)state);
}

public int ToState(bool state)
{
    return Convert.ToInt32(state));
}

I find it very weird that you use 1 & 2 as boolean values instead of 1 & 0 that can be much easier converted to int/bool/enum.


Scraping a table from Texas Dept. of Criminal Justice website

In addition to PEP 20, the Python Zen, which you seem to be already familiar with,
another must read (and follow) for Python coders is PEP 8, the style guide.
Many of those recommendations apply to your code,
please read it carefully, I won't repeat them here.

Working with resources

In counties,
you open a file for reading but you never close it.
You should always close the resources, file handles you open.
But actually, in Python, you should use the with syntax, like this:

with open("counties.txt") as fh:
    for line in fh.readlines():
        countyArg[line.replace('\n', '').rstrip()] = 0

In this form, you don't actually need to close:
Python will automatically close the file handle after leaving the with block.

Why out-parameters?

It's not clear why you opted for initializing an empty county dictionary and then filling it in the counties method.
It would be more natural to let counties create the dictionary and return it.

Avoid global variables

county is declared at the outermost scope,
effectively making it a global variable.
Avoid global variables as much as possible.

As hinted earlier,
it would be better to create the dictionary inside a function,
and pass it around to the functions that need it.

For example:

load_counties(path):
    counties = {}
    with open(path) as fh:
        for line in fh.readlines():
            counties[line.replace('\n', '').rstrip()] = 0

    return counties

# ....

def main():
    counties = load_counties("counties.txt")
    scrape(url, counties)
    # ....

Avoid code duplication

Avoid repeated logic like this:

for row in table.findAll('tr')[1:]:
    if row.findAll('td')[8].text == 'Black':
        races['black'] += 1
    elif row.findAll('td')[8].text == 'White':
        races['white'] += 1
    elif row.findAll('td')[8].text == 'Hispanic':
        races['hispanic'] += 1

Not only it is error-prone to repeat row.findAll('td')[8].text many times,
it's also inefficient: beautiful soup has to find all td elements multiple times.

Refactor to do this only once:

for row in table.findAll('tr')[1:]:
    race = row.findAll('td')[8].text
    if race == 'Black':
        races['black'] += 1
    elif race == 'White':
        races['white'] += 1
    elif race == 'Hispanic':
        races['hispanic'] += 1

We can also go one step further:

for row in table.findAll('tr')[1:]:
    race = row.findAll('td')[8].text
    if race in ('Black', 'White', 'Hispanic'):
        races[race.lower()] += 1

Consider using collections.Counter

The collections package has a class that can help you here: Counter.
Instead of doing the counting by yourself,
this handy little utility can do it for you,
it goes something like this:

from collections import Counter

# ...

    races = []
    for row in table.findAll('tr')[1:]:
        race = row.findAll('td')[8].text
        if race in ('Black', 'White', 'Hispanic'):
            races.append(race.lower())

    return Counter(races)

That is,
from a list like ['black', 'white', 'black', 'white', 'black', 'hispanic'] it will create Counter({'black': 3, 'white': 2, 'hispanic': 1})

Use max to find the maximum entry of a dictionary

find_largest can be simplified using max:

def find_largest(counties):
    return max(counties.items(), key=lambda pair: pair[1]))


You can definitely simplify this, as the only part of each element of $langs that changes is the two-letter language code. Something like this would work:

<?php
$langs = array(
    "en",
    "de",
    "fr"
);
$self    = $_SERVER['REQUEST_URI'];
$pattern = "{^.*/}i";
$links   = array();
foreach ($langs as $code) {
    $url  = preg_replace($pattern, "$code/", $self);
    $link = "

";
    array_push($links, $link);
}
echo implode($links) . "\n";
?>

You just use a non-associative array and append / or .png where needed; much cleaner.


Getting rid of an inordinate use of the `instanceof` function

I will assume that you really, really want to use the classes defined in the library and not make your own. Presumably the library classes offer other benefits that were not otherwise relevant to the question.

One thought is that you could define an interface that offers a function with the same signature as getPoints, though with a different function name.

public interface PointListGettable {
    public Point[] getPointList();
}

(You'd probably want to come up with a better name than PointListGettable, however.)

Extend the offending classes so they implement this interface. For example:

public MyDestination extends Destination implements PointListGettable {
    public Point[] getPointList() {
        return getPoints();
    }
}

Now you can implement a function with a signature like

public PointListGettable getCurrentMapObjectWithPoint(Point point, PointListGettable [] bases)

but rather than define a function like your getpointsArrayFromMapObject, within the body of the for(PointListGettable model: bases) loop you can just write model.getPointList().

If it's just a matter of getting around one perceived defect in the inheritance scheme without using instanceof, however, I'm not sure all this is worth it.


The function does not handle end-of-file correctly. stdin is just a stream, and could be closed like any other stream. For example, type Ctrl-D. Or
  echo -n a | ./a.out
  a????????

Keep in mind that reading beyond end-of-file doesn't set errno.

There is no reason to special-case count == 0.
An IO error doesn't break the loop. If it happened, all the subsequent invocations of fgets would return EOF, and upon the loop termination the count will be n.

Pass the buffer size as size_t. This is what sizeof yields, and the buffers could be large enough for their size to overflow an integer.


I think you are on the right track, there are only a few things I would change.

I would create a static function in the controller which inflates and returns a root node to be added to the stage (or even launches a new stage).
I would make a separate controller for each window/fragment you want to create. So you would have a LoginController and a RegisterController, each inflating and binding to their respective fxml files.

Depending on how your app is going to to be structured, you'll likely want a MainController which provides a container to swap sub-controllers in and out of. In this way, each controller is a single entity that can be used ad-hoc; only the main controller will be aware of their existence and be responsible for displaying them. MainController would also provide the ideal place for global menus, titles, toolbar, tabs, etc.

UPDATE

public static LoginUiController initScene(Stage stage) throws IOException
{
    LoginUiController controller = new LoginUiController();

    // Inflate FXML
    FXMLLoader loader = new FXMLLoader(LoginUiController.class.getResource("controller/login/Login.fxml"));
    loader.setController(controller);
    controller.root = loader.load(); // Good to have a pointer to the root node so the controller can be nested

    // Create scene
    Scene scene = new Scene(controller.root);
    stage.setScene(scene);
    controller.onCreated(); // Method to let the controller know it has been inflated and added to a scene

    return controller;
}

I generally make use of an abstract base controller that exposes the different methods I use (inflateFxml, initScene, onCreate) as well as a property binder lookup table, so the controller can unbind all listeners when closed.


Menu-based Caesar Cipher with encryption and decryption

I'd initialize with something closer to this (although I might inline the two arrays):

plain_alphabet = ('a'..'z').to_a
cipher_alphabet = ('A'..'Z').to_a.rotate KEY
$caesar_map = Hash[plain_alphabet.zip cipher_alphabet]

The en/decryption (which could likely be handled by the built-in tr method) might look more like:

plaintext.split("").inject("") {|s, c| s << $caesar_map[c]}

I'd put the whole thing in a class, too; skip the global.


class IMessge - returns user message types | loop similar items

Well, you are doing this a little redundantly...

$keys = array(
    'name',
    'email',
    //etc...
);

$messages = array(
    'PASS' => $this->PASS,
);

foreach( $keys AS $key ) { $messages[ $key ] = $this->FAIL . $key; }

$this->messages = $messages;

Also, your get function has a fatal error. You currently have a check to see if a key exists, and if it doesn't it will output an error message. This is incomplete, if that key doesn't exist it will output the error message then PHP will throw an undefined index error because $key does not exist. To avoid this you should have it return that error message when you set it or have it return FALSE. I think such a check is good, since it seems you are dealing with user supplied information. If this was a private method and you knew the information you were passing it was reliable, then I would say its unnecessary.


Shortest path algorithm in 0-1-unoriented graph

You should extract reading the data and the Dijkstra to their own method. It helps debugging. You don't have enough information to make a meaningful heuristic for A* anyway.

When filling the adjacency matrix you do it as if it is directional while your post says it's not:

for (size_t vert = 0; vert < mEdges; ++vert) {
    cin >> edStart >> edEnd >> weight;
    ajList.at(edStart - 1).push_back(pair<size_t, size_t>(edEnd, weight));
    ajList.at(edEnd - 1).push_back(pair<size_t, size_t>(edStart, weight));
}

I suggest you use a priority queue of some sort; it lets you remove the current node in \$O(\log n)\$ instead of \$O(n)\$ (the first inner for loop in your code).


Print diagonal numbers of matrix in ascending order

It's bizarre that create_matrix() calls print_matrix().  According the Single Responsibility Principle, each function should do just one thing.  And if the function is called create_matrix(), then it should just return a matrix, and do nothing else.

If your create_matrix() function already implements the boustrophedon arrangement, then I see no reason why print_matrix() should also have a toggling reverse_flag.  That is, I consider the output

1 4
6 7
11 10
16 13

… to be wrong, as it is neither sorted in ascending order (1, 4, 6, 7, 10, 11, 13, 16) nor alternating (1, 4, 7, 6, 10, 11, 16, 13).

You don't actually have to create the matrix to print the diagonal entries.  Just use arithmetic.

def matrix_diagonals(n):
    for row_num in xrange(n):
        row_min = n * row_num + 1
        row_max = n * row_num + n
        diag1 = row_min + row_num
        diag2 = row_max - row_num
        yield max(diag1, diag2) if row_num % 2 else min(diag1, diag2)
        if diag1 != diag2:
            yield min(diag1, diag2) if row_num % 2 else max(diag1, diag2)

print list(matrix_diagonals(4))

You could even simplify the arithmetic a bit:

def matrix_diagonals(n):
    for row_num in xrange(n):
        diag1 = (n + 1) * row_num + 1
        diag2 = (n - 1) * row_num + n
        yield max(diag1, diag2) if row_num % 2 else min(diag1, diag2)
        if diag1 != diag2:
            yield min(diag1, diag2) if row_num % 2 else max(diag1, diag2)

Since you are using Python 2, xrange() would be more efficient than range().


Determining if an answer can be generated

This is an interesting problem! You kept our chatroom quite occupied with discussing this question for quite a while :)
Your approach is interesting but it has one big flaw:
You're starting from the wrong direction!
Instead of starting from a and b, start from c and d. Ask yourself this question: What are the ways to reach that number?
Consider this problem: (I will use the notation a, b --> c, d)
4, 5 --> 15, 19

What has to be the step before 15, 19? Which number was added to which?
Was it 15, x or was it x, 19? x, 19 would be nuts as 15 is less than 19. Therefore, we know that it was 15, x. And x must be 4, as 15 + 4 = 19.
By continuing to subtract the biggest number with the lowest number, we can find out all the possible inputs that can produce these two numbers.
15, 19
15, 4
11, 4
7, 4
3, 4
3, 1
2, 1
1, 1

As a, b was 4, 5, we can see already at 15, 4, that this is not possible because 4 < targetB.
Doing this for your example of 1, 2 --> 3, 8:
3, 8 <--- 8 is bigger, so decrease it by 3
3, 5 <--- 5 is bigger, so decrease it by 3
3, 2 <--- 3 is bigger, so decrease it by 2
1, 2 <--- 2 is bigger, so decrease it by 1
1, 1

Wait, what's that!? 1, 2 is in the list!? It's our lucky day! We've found a matching a, b! So this will return true.
I will leave the fun part of implementing the code up to you :) I hope your understand this approach.
As for the time complexity: Significantly faster than your current approach. As for the real time complexity, ask @rolfl.


100DaysofSwiftUI - Project 1 - Challenge 1: Convert temperature units: Celsius, Fahrenheit and Kelvin

The conversion function convertedDegreeC2F() handles only four of the nine possible conversions between the three units. In particular, the conversion between identical input/output unit is not handled. Also the function name is misleading because the function is not about conversion from Celsius to Fahrenheit only.

The conversion between temperature units can be greatly simplified using  Measurement from the Foundation framework:

  A numeric quantity labeled with a unit of measure, with support for unit conversion and unit-aware calculations.

Here is a standalone example for demonstration:

let t = Measurement(value: 10.0, unit: UnitTemperature.celsius)
print(t.converted(to: .fahrenheit).value) // 49.99999999999585
print(t.converted(to: .kelvin).value)     // 283.15

The Fahrenheit value is not “exact” due to floating point rounding issues. But that does not matter, because displaying the value should be done with a formatter anyway. Here we can use a MeasurementFormatter:

  A formatter that provides localized representations of units and measurements.

Again a standalone example for demonstration:

let formatter = MeasurementFormatter()
formatter.unitOptions = .providedUnit

let t = Measurement(value: 10.0, unit: UnitTemperature.celsius)
print(formatter.string(from: t)) //10°C
print(formatter.string(from: t.converted(to: .fahrenheit))) // 50°F
print(formatter.string(from: t.converted(to: .kelvin))) // 283.15 K

The measurement formatter adds the correct unit symbol automatically, and also localizes the output correctly (e.g. “283,15 K” with a comma as fraction separator, where appropriate).

The conversion from the input field value to a floating point number should be done with a NumberFormatter() to handle localized input correctly.

With these preparations, the correct conversion between arbitrary temperature units becomes simple:

func convert(temperature: String, from inputUnit: UnitTemperature, to outputUnit: UnitTemperature) -> String? {
    let inputFormatter = NumberFormatter()
    guard let value = inputFormatter.number(from: temperature) else { return nil }

    let inputTemp = Measurement(value: value.doubleValue, unit: inputUnit)
    let outputTemp = inputTemp.converted(to: outputUnit)

    let outputformatter = MeasurementFormatter()
    outputformatter.unitOptions = .providedUnit
    return outputformatter.string(from: outputTemp)
}

In your ContentView you can then define the list of available units as

let units: [UnitTemperature] = [.celsius, .fahrenheit, .kelvin]

instead of using a list of strings. Both pickers can be created from this list (with the correct symbol for each unit), e.g.

Picker("Input Unit", selection: $inputUnit) {
    ForEach(0..<units.count) {
        Text(self.units[$0].symbol)
    }
}
.pickerStyle(SegmentedPickerStyle())

Finally, converting the input value and displaying it is now simply done with

Text(convert(temperature: inputTemp, from: units[inputUnit],
            to: units[outputUnit]) ?? "Invalid input")

A small performance improvement might be to create both formatters only once, e.g. by storing them as private variables in the compilation unit:

import SwiftUI

fileprivate let inputFormatter = NumberFormatter()
fileprivate let outputformatter: MeasurementFormatter = {
    let fmt = MeasurementFormatter()
    fmt.unitOptions = .providedUnit
    return fmt
}()

func convert(...)
// ...


Copying a lot of code to filter four PHP arrays

comments should explain what the code does, at least the complicated blocks, such as the array merging, and/or a comment to describe the whole thing
Be consistent with naming things: you use camelCase sometimes (e.g. userArray) and underscores other times (e.g. field_value_gender). IIRC, underscores is the PHP style, so you could go with that, but the important thing is to be consistent.
Same goes for your filter parameter names: service_filter, region_filter, postal_code_filter, gender_filter
When you have places where you repeat code, use a function:

function build_array(post_field, user_field) {
    $post_value = $_POST[post_field];
    $user_value = get_cimyFieldValue(false, user_field, $post_value);
    $arr = array();

    foreach ($user_value as $value) {
        $arr[] = $val['user_id'];
    }
    return $arr;
}

build_array('service_filter', 'service');

But really, you probably don't need that. Are you trying to get a unique list of user IDs? Can you use one array and call array_unique on that? Or, are you trying to get duplicates? Or both? Either way, I really doubt you need all that merge/diff/unique'ing. For example, you could keep an array of user_id => times_seen and then loop through that to get entries with times_seen > 1 or whatever.


using namespace std;

You may have already seen Why is “using namespace std;” considered bad practice?

Personally, I just find it easier and more readable to write out std:: where necessary.

Declare where used

    int convertFromCopy, digit, convertTo;
    unsigned int convertFrom;
    string stringDigit, answer;

In old C, you used to have to define every variable at the beginning of a block.  In modern C and all C++, you can declare them anywhere.

    int convertTo;
    unsigned int convertFrom;

    std::cout << "Num (base 10): ";
    std::cin >> convertFrom;
    std::cout << "Base: ";
    std::cin >> convertTo;

    std::string answer;
    int convertFromCopy = convertFrom;

    while (convertFrom != 0){
        int digit = convertFrom % convertTo;

        std::string stringDigit;
        if (digit < 10) {
            stringDigit = '0' + digit;
        } else {
            stringDigit = digit - 10 + 'A';
        }

        answer = stringDigit + answer;

        convertFrom /= convertTo;
    }

    std::cout << convertFromCopy << " written in base " << convertTo << " is: " << answer;

I greatly prefer to use the block form for control statements (i.e. add {} to if and else here).  I find it both easier to read and less subject to editing errors.

Abstract into functions

    int convertTo;
    unsigned int convertFrom;

    std::cout << "Num (base 10): ";
    std::cin >> convertFrom;
    std::cout << "Base: ";
    std::cin >> convertTo;

    std::string answer = convertFromBaseTo(convertFrom, convertTo);

    std::cout << convertFrom << " written in base " << convertTo << " is: " << answer;

Note that the function removes the need for the convertFromCopy variable.

std::string convertFromBaseTo(unsigned int convertFrom, int convertTo) {
    std::string answer;

    while (convertFrom != 0){
        int digit = convertFrom % convertTo;

        char stringDigit;
        if (digit < 10) {
            stringDigit = '0' + digit;
        } else {
            stringDigit = digit - 10 + 'A';
        }

        answer.insert(answer.begin(), stringDigit);

        convertFrom /= convertTo;
    }

    return answer;
}

I changed stringDigit to a char because it's just a single character.  Adding to answer changed a bit as well.


Project Euler # 22 names scores in Python

The code looks quite good in general. Without a deeper look at the problem, there are twothree things that can easily be improved:

Building the look-up table

You use

letter_scores = {letter: ord(letter) - 64 for letter in string.ascii_uppercase}

which makes use of the ASCII encoding to determine the value of each letter. The following could be considered a little bit more straightforward:

letter_scores = {letter: index for index, letter in enumerate(string.ascii_uppercase, 1)}

This uses enumerate with a starting value of 1 to assign the position in the alphabet.

Iterating over the sorted names

When iterating over the sorted names, you already use enumerate to get index, but since you only use index+1, you can reuse the trick from above to simplify this even further (as @RootTwo rightfully pointed out in his comment).

Computing the score

To compute the score you use a for loop with an intermediate variable:

temp = 0
for letter in name:
    temp += letter_scores[letter]
total += temp * (index + 1)

This can be rewritten using a generator expression and sum:

total += sum(letter_scores[letter] for letter in name) * (index + 1)

It basically allows you to get rid of the additional counter variable.

Going nuts

You can also reapply that generator expression idea to the first loop and end with something like:

def calculate_score(filename):
    """Reads names and returns total scores."""
    letter_scores = {letter: index for index, letter in enumerate(string.ascii_uppercase, 1)}
    with open(filename) as names:
        names = sorted(names.read().replace('"', '').split(','))
        return sum(
            sum(letter_scores[letter] for letter in name) * index
            for index, name in enumerate(names, 1)
        )


Is there any convension which says when should I use it for my custom objects? In model layer or in view?

If your question is 'When should I override the default toString() method and use create my own?' then there is one set of answers. If your question is 'when should I call the toString() method (implicitly like "This is my object: " + myobj or explicitly)?' then there is a different set of answers.

So, when should you override the toString()? From the Javadoc for Object:

  In general, the toString method returns a string that "textually represents" this object. The result should be a concise but informative representation that is easy for a person to read. It is recommended that all subclasses override this method.

I strongly agree with this statement. Every custom class you create should override the toString method, and should find a way to textually represent the class content in a meaningful (but not necessarily exhaustive) way. For example, a 'Point' class representing a co-ordinate x and y where x==3 and y==4 should return something like Point(3,4). On the other hand, I believe that some classes are over-exhaustive with toString, like ArrayList does too much. I would prefer ArrayList(size 10: [sample0 .... sample9]). Edit: The Java tutorial for Object also says the same thing about toString():

  The Object's toString() method returns a String representation of the object, which is very useful for debugging

The reasons for making every class have a toString is because of the times you should use the toString() method.... and, as far as I am concerned, there are only three places in general programming where using toString() is legitimate (and two 'special' cases):

In exception handling: throw new IllegalStateException("Failed to process " + value);
for logging program progress using things like log4j.
when the JavaDoc for the toString() Method for the class you are using defines a specific, and non-alterable format for the result. e.g. String.toString() or Integer.toString(). If the class you are using does not specify what the toString() returns, and does not guarantee that it won't change, then create your own way to present the data.

There are two special cases for using toString():

I think, even though this is a special case, that this is the most important reason: when running your Java program through a debugger like the one built in to Eclipse. When you are stepping through your program the debugger will use the toString() method to show you the state of all your variables. This is where, in many cases, you will value the toString() more than anything else.
when developing or debugging a program it is often convenient for you to add 'println' statements, and they can be things like: System.out.println("Processing customer " + customer);`

In summary, the toString() method should produce enough data to debug problems with the object content. It is used occasionally for a 'contractual' representation of a value (like String.toString()).

But, in general, toString() Is a tool for the PROGRAMMER, not for the user. It is there to assist you when things go wrong, not when they go right.

In my real Job I file bugs against code (and pursue the bugs relentlessly) which throws exceptions where the exception has an Object.toString() in the message. I also mentally +1 people who's programs produce good detail during exceptional conditions.

Now, in your particular situation, I think that you should use the toString() method to represent the state of your Customer, and, because the display-conditions for your customer are different to the internal state (e.g. display needs to be UPPERCASE,) I think that you should have a different mechanism for displaying the Customer than the toString() method.

In other words, the information you will need to debug a situation involving a Customer (something printed in a stack trace or debug session) is different than the information you want to display on the screen.


It has been suggested in docs to not pass IConfiguration around for injection. Try creating a IConnectionFactory or just have a connection injected directly into the repo

Something like

public interface IDbConnectionFactory {
   IDbConnection CreateConnection();
}

and its implementation for your class would look something like this

public class SqlConnectionFactory : IDbConnectionFactory {
    private readonly string dbConnectionString;

    public SqlConnectionFactory(string dbConnectionString) {
        this.dbConnectionString = dbConnectionString;
    }

    public IDbConnection CreateConnection() {
        return new SqlConnection(this.dbConnectionString);
    }
}

it would be registered in startup

var connectionString = Configuration.GetConnectionString("MyConnString");
services.AddScoped<IDbConnectionFactory, SqlConnectionFactory>(_ => new SqlConnectionFactory(connectionString));

//...

and injected

public class TrainerRepo : IRepoTrainer {
    private readonly IDbConnectionFactory factory;

    public TrainerRepo(IDbConnectionFactory factory) {
        this.factory = factory;
    }

    public List<Trainer> GetTrainers() {
        string sql = "select trainer_id,trainer_name from trainer";
        using(IDbConnection connection = factory.CreateConnection()) {
            connection.Open();
            var res = connection.Query<Trainer>(sql).ToList<Trainer>();
            connection.Close();
            return res;
        }
    }

    //...

The repeated code can also be generalized

T Invoke<T>(Func<IDbConnection, T> func) {
    using(IDbConnection connection = factory.CreateConnection()) {
        connection.Open();
        var res = func(connection);
        connection.Close();
        return res;
    }
}

async Task<TResult> InvokeAsync<TResult>(Func<IDbConnection, Task<TResult>> func) {
    using(IDbConnection connection = factory.CreateConnection()) {
        connection.Open();
        var res = await func(connection);
        connection.Close();
        return res;
    }
}

Which changes the previous examples to

public List<Trainer> GetTrainers() {
    string sql = "select trainer_id,trainer_name from trainer";
    return Invoke(connection => connection.Query<Trainer>(sql).ToList<Trainer>());
}

public Task<int> updateTrainer(Trainer trainer) {
    string sql = "update trainer set trainer_name = @tname where trainer_id = @tid";
    return  InvokeAsync(connection => connection.ExecuteAsync(sql, param: new { tname = trainer.trainer_name, tid = trainer.trainer_id }));
}


What about seperating your sorting from your criteria?  I'm like you, I'm sure there is another way so am interested to see other opinions. I'm not exactly sure if this would scale for multiple orderBys either but I think it might (NOTE:  this code is untested and some types are not defined)

Something along the lines of:

public ActionResult List(string sorting = "Name", string sortingMethod = "ASC")
{
   var sortDirection = GetSortDirection(sortingMethod);
   var criteria = GetSortCriteria(sorting);

   var products = GetProductsOrderBy(criteria, sortDirection);

   return View(products.ToList());
}

// I'm not actually sure of the return type here.  Will attempt to test when I
// get in front of my development pc
Func<Product, object> GetSortCriteria(string sortOn)
{
   switch(sortOn)
   {
      case "Price":
      case "Category":
         return x => x.Price;
      default:
         return x.Name;
   }
}

IEnumerable<ProductList> GetProductsOrderBy(Action sortOn, SortDirection sortDirection)
{
    var productList = productRepository.Get();

    return sortDirection == SortDirection.Ascending ?
           productList.OrderBy(sortOn) :
           productList.OrderByDescending(sortOn);
}

SortDirection GetSortDirection(string sortBy)
{
   return sortBy == "ASC" ? SortDirection.Ascending : SortDirection.Descending;
}


If you are trying to make it fast, the last thing to do is drop into Python for-loops.  Keep it all in numpy as much as possible.

The code below maps the particles to the mesh and calculates the mean v for all the particles in each mesh square.  With 400M particles having random x,y, and v on a 1600x900 size mesh, it runs in about 100 seconds on my laptop.

xi = np.digitize(x, np.linspace(xmin, xmax, nx, endpoint=False)) - 1
yi = np.digitize(y, np.linspace(ymin, ymax, ny, endpoint=False)) - 1

flat_index = nx*yi + xi

counts = np.bincount(flat_index)

vsums = np.bincount(flat_index, weights=v)

means = (vsums / counts).reshape(nx,ny)

np.digitize(x, bins) maps values the values in the array x, to ints based on the values in bins.  x < bin[0] maps to 0; bin[0] <= x < bin[1] maps to 1; and so on.  The endpoints=False makes sure the indices go from 1 to 1440000 instead of 1 to 143999; and the -1 shifts it to 0 to 1439999.

np.linspace(start, end, nsteps) divides the range [start, end) in to nstep even intervals.  This is used to define the bins for np.digitize().

nx*yi + xi basically converts the x and y indices to a 1D index from 0 to 1439999, because bincount only works on 1D arrays

np.bincount(z, weights) if weights is omitted, this counts the number of occurrences of each integer in z (like a histogram).  If provided, weights is an array of the same dim as z, the values of the corresponding weights are summed instead of just counting the integers.  So setting weights to v, sums the velocity of each particle in a mesh square.

(sums / counts).reshape(nx,ny) calculates the means and reshapes the result into the 1600x900 mesh.


I would experiment with something like the following:

import Data.Typeable

withEntitySql
  :: ( PersistEntityBackend a ~ SqlBackend
     , ToBackendKey SqlBackend a
     , Typeable a )
  => Int64
  -> (Entity a -> ExceptT Text (HandlerT App IO) b)
  -> ExceptT Text (HandlerT App IO) b
withEntitySql key f = withEntity (show $ typeRep (Proxy :: Proxy a)) (toSqlKey key) f